-
Notifications
You must be signed in to change notification settings - Fork 3.5k
feat: allow writes to /tmp by default without permission prompt #5388
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: dev
Are you sure you want to change the base?
Conversation
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.
Pull request overview
This PR introduces a new helper function Filesystem.isAllowedPath() to enable file operations in temporary directories (os.tmpdir() and /tmp) without prompting users for external directory permissions. This addresses issues #5386 and #4743 by allowing AI agents to write to temp directories by default, which is a common and safe operation.
Key changes:
- Added
isAllowedPath()helper that checks if a path is within the project directory, system temp directory, or/tmp - Replaced all
Filesystem.contains()calls withFilesystem.isAllowedPath()across file operation tools (read, write, edit, patch) and bash command execution
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/opencode/src/util/filesystem.ts | Added new isAllowedPath() function that allows access to project directory and temp directories |
| packages/opencode/src/tool/write.ts | Updated to use isAllowedPath() instead of contains() for permission checking |
| packages/opencode/src/tool/read.ts | Updated to use isAllowedPath() instead of contains() for permission checking |
| packages/opencode/src/tool/patch.ts | Updated to use isAllowedPath() instead of contains() for permission checking |
| packages/opencode/src/tool/edit.ts | Updated to use isAllowedPath() instead of contains() for permission checking |
| packages/opencode/src/tool/bash.ts | Updated to use isAllowedPath() instead of contains() in external directory validation |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
9c46c39 to
4d2e62c
Compare
|
/review |
3ac1137 to
cd202b6
Compare
|
Can we make it optional for this from the configuration like the following? "permission": {
"external_directory": {
"env:$TMPDIR/test": "allow",
"*": "deny",
}
} |
|
yeah that's prolly fine |
|
@jiyeol-lee Why would you like to make it optional? Do you want to prevent the LLM to write to /tmp? |
|
that's what I was wondering too but letting people override that perm in general makes sense, best done in separate pr tho |
@remorses @rekram1-node Yes, I don't want LLM to write, especially read /tmp. I usually have multiple credentials in there temporary and I don't want LLM read things that I didn't expect to do. It would be amazing if we make it configurable by the individual user unless it's a project directory. |
Add
Filesystem.isAllowedPath()helper that allows access to project directory,os.tmpdir(), and/tmpwithout prompting for external directory permission.Fixes #5386
Fixes #4743