Skip to content

Fix: Use SHA256 hash for registry keys related to file paths #17382

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 1 commit into from

Conversation

seer-by-sentry[bot]
Copy link
Contributor

Resolved / Related Issues

To prevent extra work, all changes to the Files codebase must link to an approved issue marked as Ready to build. Please insert the issue number following the hashtag with the issue number that this Pull Request resolves.

  • Closes FILES-APP-37R

Steps used to test these changes

Stability is a top priority for Files and all changes are required to go through testing before being merged into the repo. Please include a list of steps that you used to test this PR.

  1. The issue was that: Registry key creation fails because LayoutPreferencesDatabase.FindPreferences uses excessively long file paths directly as registry key components, exceeding the 255-character limit.
  2. Introduces a CreateSHA256 method in ChecksumHelpers.cs to generate SHA256 hashes from strings.
  3. Modified LayoutPreferencesDatabase.cs and FileTagsDatabase.cs to use SHA256 hashes of file paths as registry keys instead of the file paths themselves.
  4. This change aims to improve security and prevent potential issues with special characters or excessively long file paths in registry keys.
  5. Verified fix for FILES-APP-37R.

@yaira2 yaira2 requested review from hez2010 and Copilot July 31, 2025 19:33
@yaira2 yaira2 changed the title Refactor: Use SHA256 hash for registry keys related to file paths Fix: Use SHA256 hash for registry keys related to file paths Jul 31, 2025
@yaira2 yaira2 marked this pull request as ready for review July 31, 2025 19:34
Copy link
Contributor

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR refactors the registry key creation mechanism to use SHA256 hashes of file paths instead of the raw file paths themselves. This addresses issues with registry key creation failures when file paths exceed the 255-character limit and improves security by avoiding potential issues with special characters in file paths.

  • Adds a new CreateSHA256 method to generate SHA256 hashes from input strings
  • Updates FileTagsDatabase to use SHA256 hashes of file paths as registry keys
  • Updates LayoutPreferencesDatabase to use SHA256 hashes of file paths as registry keys

Reviewed Changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.

File Description
src/Files.Shared/Helpers/ChecksumHelpers.cs Adds new CreateSHA256 method for generating SHA256 hashes from strings
src/Files.App/Utils/FileTags/FileTagsDatabase.cs Replaces direct file path usage with SHA256 hashes in registry key creation
src/Files.App/Helpers/Layout/LayoutPreferencesDatabase.cs Replaces direct file path usage with SHA256 hashes in registry key creation
Comments suppressed due to low confidence (1)

src/Files.Shared/Helpers/ChecksumHelpers.cs:25

  • [nitpick] The method name 'CreateSHA256' is ambiguous. Consider renaming it to 'CreateSHA256Hash' or 'ComputeSHA256Hash' to clearly indicate that it computes a hash value.
		public static string CreateSHA256(string input)

Copy link
Member

@hishitetsu hishitetsu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm amazed that AI came up with this way of fix.
But this will invalidate the old registry keys, so we will need to consider migrating them.

@yaira2
Copy link
Member

yaira2 commented Aug 1, 2025

Merging with #17389

@yaira2 yaira2 closed this Aug 1, 2025
@yaira2 yaira2 deleted the seer/refactor-sha256-registry-keys branch August 1, 2025 03:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants