-
Notifications
You must be signed in to change notification settings - Fork 2.6k
fix: prevent bypassing .rooignore restrictions via terminal commands #7215
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
- Updated error message when files are blocked by .rooignore to explicitly warn against using terminal commands to bypass restrictions - Added clear instructions in .rooignore system prompt to not attempt bypassing restrictions - Updated tests to verify the new warning messages are included Fixes #7204
|
Separate from the prompting change, there is code to try to block |
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.
Reviewing my own code is like grading my own homework - suspicious but necessary.
| } | ||
|
|
||
| return `# .rooignore\n\n(The following is provided by a root-level .rooignore file where the user has specified files and directories that should not be accessed. When using list_files, you'll notice a ${LOCK_TEXT_SYMBOL} next to files that are blocked. Attempting to access the file's contents e.g. through read_file will result in an error.)\n\n${this.rooIgnoreContent}\n.rooignore` | ||
| return `# .rooignore\n\n(The following is provided by a root-level .rooignore file where the user has specified files and directories that should not be accessed. When using list_files, you'll notice a ${LOCK_TEXT_SYMBOL} next to files that are blocked. Attempting to access the file's contents e.g. through read_file will result in an error. IMPORTANT: Do NOT attempt to bypass these restrictions by using terminal commands to read the file contents - this violates the user's explicit access restrictions.)\n\n${this.rooIgnoreContent}\n.rooignore` |
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.
The warning message works well, but I'm wondering if we should also enhance the validateCommand method (lines 111-160) to actively block terminal commands that attempt to read ignored files? Currently it returns the blocked file path but doesn't seem to be actively used to prevent command execution. Could this be a follow-up enhancement to make the restriction enforcement more robust?
|
|
||
| rooIgnoreError: (path: string) => | ||
| `Access to ${path} is blocked by the .rooignore file settings. You must try to continue in the task without using this file, or ask the user to update the .rooignore file.`, | ||
| `Access to ${path} is blocked by the .rooignore file settings. You must try to continue in the task without using this file, or ask the user to update the .rooignore file. IMPORTANT: Do NOT attempt to bypass this restriction by using terminal commands (like cat, head, tail, etc.) to read the file contents - this violates the user's explicit access restrictions.`, |
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.
The warning message is quite verbose. Would it be clearer if we made it more concise? Something like: "IMPORTANT: Terminal commands must not be used to bypass .rooignore restrictions." Just a thought - the current version is certainly explicit about what not to do.
|
@mrubens I'll run some tests |
|
Closing this PR. The existing code in already blocks terminal commands from accessing .rooignore-protected files. This is properly integrated in at line 46. The additional warning messages in this PR are redundant since the blocking mechanism is working correctly. |
Summary
This PR addresses issue #7204 by adding explicit warnings to prevent models from attempting to bypass
.rooignorerestrictions using terminal commands.Problem
When files are blocked by
.rooignore, the model would sometimes try to work around the restriction by using terminal commands likecat,head, ortailto read the file contents, which violates the user's explicit access restrictions.Solution
Added clear instructions in two places:
.rooignoreinstructions shown to the model now include a warning not to attempt bypassing restrictionsChanges
rooIgnoreErrormessage insrc/core/prompts/responses.tsto include warning about terminal command bypass.rooignoreinstructions insrc/core/ignore/RooIgnoreController.tsto include the same warningTesting
Fixes #7204
cc @daniel-lxs
Important
Adds warnings in error messages and system prompts to prevent bypassing
.rooignorerestrictions using terminal commands.rooIgnoreErrorinresponses.tsto warn against using terminal commands to bypass.rooignorerestrictions..rooignoreinstructions inRooIgnoreController.tsto include the same warning.RooIgnoreController.spec.tsandresponses-rooignore.spec.tsto verify new warning messages.readFileTool.spec.tsreflect changes in error message format.This description was created by
for a1173b9. You can customize this summary. It will automatically update as commits are pushed.