Skip to content

fix: grep external directory permission evaluation#26958

Merged
rekram1-node merged 1 commit into
devfrom
fix-grep-perms
May 12, 2026
Merged

fix: grep external directory permission evaluation#26958
rekram1-node merged 1 commit into
devfrom
fix-grep-perms

Conversation

@rekram1-node
Copy link
Copy Markdown
Collaborator

@rekram1-node rekram1-node commented May 12, 2026

Fixes grep external-directory permission checks when the requested path resolves through an alias or symlink, such as macOS /var -> /private/var.

Previously, grep canonicalized the requested path with AppFileSystem.resolve() before evaluating external_directory permissions. That meant allow rules written against the user-facing path could miss after realpath normalization, causing an approval prompt even when
the path should have been auto-approved.

This change evaluates external_directory permissions against the originally requested absolute path, while still using the resolved path for the actual ripgrep execution and result handling.

Validation:

  • Added an integration-level grep regression test covering an external symlinked directory allow rule.
  • Ran bun test test/tool/grep.test.ts from packages/opencode.
  • Result: 5 pass, 0 fail.

@rekram1-node
Copy link
Copy Markdown
Collaborator Author

/review

: path.join(ins.directory, params.path ?? ".")
yield* reference.ensure(requested)
const requestedInfo = yield* fs.stat(requested).pipe(Effect.catch(() => Effect.succeed(undefined)))
yield* assertExternalDirectoryEffect(ctx, requested, {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggestion only, so please use your judgment: moving the external_directory check before AppFileSystem.resolve means an in-workspace symlink to an external directory is treated as internal by containsPath, so grep can search outside the project without asking. Could we preserve the alias-based permission pattern while still checking the resolved path for the workspace-boundary bypass?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would expect this to not get flagged since file is in cwd

@rekram1-node rekram1-node merged commit 1a28924 into dev May 12, 2026
15 checks passed
@rekram1-node rekram1-node deleted the fix-grep-perms branch May 12, 2026 02:47
leohenon pushed a commit to leohenon/opencode-vim that referenced this pull request May 12, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant