-
Notifications
You must be signed in to change notification settings - Fork 6k
Files and paths can be disallowed inside of directories #1012
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
base: main
Are you sure you want to change the base?
Files and paths can be disallowed inside of directories #1012
Conversation
44f95b9
to
f403ae1
Compare
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.
Thanks for the PR! Had some minor comments. Also I was trying to compare this to this other PR: #623 to see if this approach would be consistent with that older one which I'm in the process of looking at, too.
normalizePath(path.resolve(expandHome(dir))) | ||
); | ||
// Default exclusion patterns that always apply | ||
const DEFAULT_EXCLUSIONS = ['.git', 'node_modules']; |
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.
const DEFAULT_EXCLUSIONS = ['.git', 'node_modules']; | |
const DEFAULT_EXCLUSIONS = [ | |
'.git', | |
'node_modules', | |
'.env*', | |
'id_rsa', | |
'id_dsa', | |
'*.pem' | |
]; |
(So this could also exclude common secrets/key files by default)
if (!exclusion.includes('/') && !exclusion.includes('*')) { | ||
// Compare filenames case-insensitively | ||
if (requestedFilename.toLowerCase() === exclusion.toLowerCase()) { | ||
throw new Error(`Access denied - path not accessible`); |
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 think we should make it clearer to the LLM that its not accessible on purpose, something like: Access denied - path matches exclusion pattern
// Check if the path matches any exclusion pattern | ||
if (isPathExcluded(config.path, relativePath, config.exclusions)) { | ||
// Use a generic message that doesn't reveal if the file exists | ||
throw new Error(`Access denied - path not accessible`); |
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.
Same here and with other new errors being added.
console.error("Usage: mcp-server-filesystem |
||
console.error("Usage: mcp-server-filesystem |
||
console.error(""); | ||
console.error("Examples:"); |
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 think as long as the tool descriptions have good descriptions for the LLM then you don't need to add this logging.
This code was entirely written by Claude, with incremental testing by the human.
Description
Added the ability to exclude specific paths from filesystem access using patterns in the arguments list.
Server Details
Motivation and Context
Users needed more control over which files and directories could be accessed within an allowed directory. This change allows specifying excluded paths (e.g.,
.env
files, build directories,.git
directories) that should never be accessible, even if they exist within an allowed directory.The feature provides enhanced security by allowing fine-grained access control. For instance, a user can allow access to a project directory while preventing access to sensitive files or directories within it.
How Has This Been Tested?
!.git
) can re-enable access to otherwise excluded pathsTSCONFIG.json
excludes both uppercase and lowercase variants)test/foo
excludes that specific subdirectory)Breaking Changes
Existing configurations without exclusion patterns will continue to work as before, except that now
.git
andnode_modules
are excluded by default.Types of changes
Checklist
Additional context
The implementation supports the following features:
Default Exclusions:
.git
andnode_modules
are excluded by default for security reasons.Custom Exclusions: Add exclusions with the syntax
/path/to/dir!file1,dir2,path/to/file3
.Negation Patterns: Override default or higher-level exclusions by prefixing with
!
, e.g.,!.git
to allow access to the.git
directory even though it's excluded by default.Case-Insensitive Matching: All exclusion patterns are case-insensitive for security on all operating systems.
Path-Based Exclusions: Support for excluding specific paths like
test/foo
to block access to subdirectories.Example usage: