-
Notifications
You must be signed in to change notification settings - Fork 9k
Address Fileserver's case insensitivity issue on Windows #2111
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
Conversation
Changed normalizePath() to convert all normalized path to lowercase on Windows.
| function normalizePath(p: string): string { | ||
| return path.normalize(p); | ||
| const normalized = path.normalize(p); | ||
| // On Windows, paths are case-insensitive. Normalize to lowercase for comparisons. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Errr, according to https://learn.microsoft.com/en-us/windows/wsl/case-sensitivity#change-the-case-sensitivity-of-files-and-directories files technically can be case sensitive on Windows...
Not sure what the best solution is here. Maybe assuming it's not case sensitive is okay, I mean even the Microsoft website seems to have a disclaimer saying most apps assume this - and it would fix the issues that people are reporting.
Are there any libraries for Node.js that handle this more nicely?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmm maybe fs.realpath.native (or its sync counterpart)
domdomegg
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I worry at the moment these changes open up this risk:
- I set up a case-sensitive directory in Windows,
/path/to - I set an allowlist of directories, with something like
/path/to/Hamburger, so that Claude can view and edit my hamburger recepies - Some other folder,
/path/to/HamburgERis in the same folder, which contains my medical records from visiting the emergency room in Hamburg. This data is now accidentally exposed
Appreciate this is fairly unlikely, but anywhere we're accidentally giving more access than we intend feels like a potential security vuln.
To move forwards here, I'd appreciate:
- Some reasoning about why this is okay, actually; or
- An alternative approach that eliminates or sufficiently mitigates this oversharing risk
|
Hey, thanks for the contribution! This hasn't been updated for a while, so I'm closing it to help us more easily track active contributions. If you're still interested in working on this, please feel free to reopen it. Thanks again for your contribution! |
Changed normalizePath() to convert all normalized path to lowercase on Windows. (Effectively treating Windows paths as case-insensitive)
Description
Address Fileserver's case insensitivity issues on Windows
Changed normalizePath() to convert all normalized path to lowercase on Windows.
I believe this should fix issues: #470, #1820, #1033, #1838 and #1987
Server Details
Motivation and Context
The filesystem MPC Server incorrectly treats paths with lowercase drive letters as outside the allowed directory scope, even though the path is valid on Windows. This results in an "access denied" error, despite the target path being inside the allowed directory.
Behavior defined in #1987 and other related issues. (#470, #1820, #1033, #1838)
How Has This Been Tested?
My local dev machine running Windsurf on Windows.
Breaking Changes
N/A
Types of changes
Checklist