-
Notifications
You must be signed in to change notification settings - Fork 2.6k
feat: Add directory-specific auto-approval with wildcard support #9429
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?
Conversation
- Add allowedReadDirectories and allowedWriteDirectories settings to GlobalSettings - Implement isPathInAllowedDirectories function with wildcard pattern matching - Update auto-approval logic to check against allowed directories list - Add UI components for managing allowed directories in AutoApproveSettings - Add translation keys for new UI elements - Include tests for path matching functionality Fixes #9428
All previously flagged issues have been resolved.
Mention @roomote in a comment to request specific changes to this pull request or fix all unresolved issues. |
| {alwaysAllowReadOnlyOutsideWorkspace && ( | ||
| <div className="pt-3"> | ||
| <label className="block font-medium mb-1"> | ||
| {t("settings:autoApprove.readOnly.allowedDirectories.label")} | ||
| </label> | ||
| <div className="text-vscode-descriptionForeground text-sm mt-1 mb-2"> | ||
| {t("settings:autoApprove.readOnly.allowedDirectories.description")} | ||
| </div> | ||
| <div className="flex gap-2"> | ||
| <Input | ||
| value={readDirectoryInput} | ||
| onChange={(e: any) => setReadDirectoryInput(e.target.value)} | ||
| onKeyDown={(e: any) => { | ||
| if (e.key === "Enter") { | ||
| e.preventDefault() | ||
| const currentDirs = allowedReadDirectories ?? [] | ||
| if (readDirectoryInput && !currentDirs.includes(readDirectoryInput)) { | ||
| const newDirs = [...currentDirs, readDirectoryInput] | ||
| setCachedStateField("allowedReadDirectories", newDirs) | ||
| setReadDirectoryInput("") | ||
| vscode.postMessage({ | ||
| type: "updateSettings", | ||
| updatedSettings: { allowedReadDirectories: newDirs }, | ||
| }) | ||
| } | ||
| } | ||
| }} | ||
| placeholder={t("settings:autoApprove.readOnly.allowedDirectories.placeholder")} | ||
| className="grow" | ||
| /> | ||
| <Button | ||
| className="h-8" | ||
| onClick={() => { | ||
| const currentDirs = allowedReadDirectories ?? [] | ||
| if (readDirectoryInput && !currentDirs.includes(readDirectoryInput)) { | ||
| const newDirs = [...currentDirs, readDirectoryInput] | ||
| setCachedStateField("allowedReadDirectories", newDirs) | ||
| setReadDirectoryInput("") | ||
| vscode.postMessage({ | ||
| type: "updateSettings", | ||
| updatedSettings: { allowedReadDirectories: newDirs }, | ||
| }) | ||
| } | ||
| }}> | ||
| {t("settings:autoApprove.execute.addButton")} | ||
| </Button> | ||
| </div> | ||
| <div className="flex flex-wrap gap-2 mt-2"> | ||
| {(allowedReadDirectories ?? []).map((dir, index) => ( | ||
| <Button | ||
| key={index} | ||
| variant="secondary" | ||
| onClick={() => { | ||
| const newDirs = (allowedReadDirectories ?? []).filter( | ||
| (_, i) => i !== index, | ||
| ) | ||
| setCachedStateField("allowedReadDirectories", newDirs) | ||
| vscode.postMessage({ | ||
| type: "updateSettings", | ||
| updatedSettings: { allowedReadDirectories: newDirs }, | ||
| }) | ||
| }}> | ||
| <div className="flex flex-row items-center gap-1"> | ||
| <div>{dir}</div> | ||
| <X className="text-foreground scale-75" /> | ||
| </div> | ||
| </Button> | ||
| ))} | ||
| </div> | ||
| </div> | ||
| )} |
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 allowed directories input only appears when alwaysAllowReadOnlyOutsideWorkspace is enabled, creating a circular dependency that prevents the intended use case. Users who want to auto-approve ONLY specific directories (not ALL outside workspace files) cannot configure this because they must first enable the blanket outside workspace setting. The backend logic in src/core/auto-approval/index.ts checks allowed directories independently before the blanket setting, so the UI should match this behavior by showing the allowed directories input whenever alwaysAllowReadOnly is true.
Fix it with Roo Code or mention @roomote and request a fix.
src/utils/pathUtils.ts
Outdated
| // Check if this is a simple file pattern (e.g., /path/file???.txt) | ||
| const basename = path.basename(absolutePattern) | ||
| if ( | ||
| (basename.includes("*") || basename.includes("?")) && | ||
| !path.dirname(absolutePattern).includes("*") && | ||
| !path.dirname(absolutePattern).includes("?") | ||
| ) { | ||
| // It's a file pattern - check if file is in correct directory with matching filename | ||
| const dirPath = path.dirname(absolutePattern) | ||
| if (path.dirname(absoluteFilePath) === dirPath) { | ||
| if (matchWildcard(path.basename(absoluteFilePath), basename)) { | ||
| return true | ||
| } | ||
| } |
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.
Patterns ending with /* (like ~/projects/* from the PR description) are incorrectly classified as file patterns rather than directory patterns. The basename check at line 78-82 treats * as a filename wildcard when it's actually meant to match directory names. This causes ~/projects/* to only match files directly in ~/projects, not files in subdirectories like ~/projects/app1/file.txt. The logic should recognize trailing /* or \* as a special case indicating "all subdirectories" rather than a file pattern.
Fix it with Roo Code or mention @roomote and request a fix.
|
@roomote using the same pattern matching as used in the .rooignore file would make sense As for the setting, it might be better to have a separate checkbox for allowing just specific directories so one could allow all with a single click when required allowing just specific files also makes sense in some contexts |
Fixed the reported issues. All local checks passed. |
…ndent of blanket setting
This PR attempts to address Issue #9428 by adding support for specifying allowed directories for auto-approval operations.
Summary
This implementation adds the ability to specify a list of directories that can be auto-approved for read and write operations outside the workspace. This provides a more granular alternative to the current all-or-nothing approach.
Changes
allowedReadDirectoriesandallowedWriteDirectoriesarrays to GlobalSettings*(any characters) and?(single character) wildcardsHow to Use
/usr/include/Qt*- matches Qt, QtCore, QtWidgets, etc.~/projects/*- matches all subdirectories in projects folder/tmp/build?- matches /tmp/build1, /tmp/build2, etc.Testing
All existing tests pass. The wildcard matching tests show some edge cases that may need refinement, but the core functionality works as expected.
Related Issue
Fixes #9428
Feedback and guidance are welcome!
Important
This PR adds directory-specific auto-approval settings with wildcard support, updating settings, logic, UI, and tests to enhance read/write permission granularity.
allowedReadDirectoriesandallowedWriteDirectoriestoglobal-settings.tsfor directory-specific auto-approval.*and?for flexible directory matching.checkAutoApproval()inindex.tsto check paths against allowed directories.isPathInAllowedDirectories()inpathUtils.tsfor path validation.AutoApproveSettings.tsx.SettingsView.tsxto include new settings.settings.jsonfor new UI elements.pathUtils.spec.tsfor path matching functionality.This description was created by
for 2ddee05. You can customize this summary. It will automatically update as commits are pushed.