Repository navigation
Conversation
Report directories passed through git -C/--git-dir/--work-tree, make -C/--directory, npm --prefix, pnpm --dir, yarn/bun --cwd, go -C, uv --directory/--project and cargo -C/--manifest-path so they go through the same external_directory check as cd targets. Fixes anomalyco#59564
There was a problem hiding this comment.
Copilot was unable to review this pull request because the user who requested the review has insufficient remaining quota.
An organization owner can choose the repository organization as the billing source for reviews requested by members with a paid Copilot license. Learn about Copilot code review billing.
![opencode-agent[bot]](https://avatars.githubusercontent.com/in/1549082?s=60&v=4){kind=small}
There was a problem hiding this comment.
This fixes the bug. I ran the #59564 reproduction (simulated model, shell allowed, external_directory denied). On the base, git -C <outside> log and make -C <outside> ran and printed the outside repo's output. With this PR, both are denied with Permission denied: external_directory, the same as cd <outside> && …. The shell-parse, parity and tool-shell tests pass, and tsc on packages/core is clean. I also checked a few edge cases with both scanners and got the same results from each: chained git -C a -C /abs -C b resolves to /abs/b, git -c k=v -C /out is caught, and --manifest-path=../x/Cargo.toml reports ../x.
Public API: unchanged. Only the internal shell scan result changes, now reporting more directories.
No blocking concerns. One note: for tools other than git, the scan keeps reading flags after positional arguments, so for example npm run build --prefix /out or bun run x.ts --cwd /y will also ask for external_directory. That errs on the safe side, so it's fine. As the description says, GIT_DIR=/GIT_WORK_TREE= prefixes and wrappers like sudo/env are still not covered.
Related: #56288
Issue for this PR
Fixes #59564
Type of change
What does this PR do?
The shell tool only sends the workdir and
cd/pushdtargets to theexternal_directorycheck. A command that changes directory with its own flag skips the check. For example,git -C /outside statusruns even whenexternal_directoryis denied.ShellParse.scannow also reports directories from a short list of known flags, in both the legacy and portable scanners:-C,--git-dir,--work-tree(global options only, sogit commit -C HEADis not affected; chained-Cvalues are joined)-C,--directory(including short-option clusters like-sCand abbreviations)--prefix/-C, pnpm-C/--dir, yarn and bun--cwd-C, uv--directory/--project, cargo-Cand--manifest-path(uses the manifest's directory)These paths go through the existing
authorizeExternalcall inshell.ts, the same waycdtargets do. Values that contain variables or substitutions are skipped, likecdtargets. Arguments after--are ignored. Command resources and saved patterns do not change, and PowerShell is unchanged.Known limits: commands behind wrappers (
sudo,env,xargs) andGIT_DIR=/GIT_WORK_TREE=environment prefixes are not covered, and quoting/escape edge cases behave the same as forcdtargets.This only covers flags that set a working directory. It does not treat general path arguments as directories, which is the separate case in #56288 / #56390.
How did you verify your code works?
shell-parse.test.tscover each flag, plus cases that must not be reported (git commit -C HEAD,npm run x -- --prefix /x,$VAR/$(...)values,sudo git -C). These run with both scanners. Parity cases were added toshell-parse-parity.test.ts. A tool-level test intool-shell.test.tschecks thatgit -C <outside> statusis denied underexternal_directory: deny.bun run testinpackages/corepasses: 6664 pass, 365 skip, 0 fail. The baseline was 6585 pass, 365 skip, 0 fail.bun run typecheckinpackages/coreandbun run checkat the root pass. Lint reports 0 errors and the same warning count asdev.Screenshots / recordings
N/A (permission parsing, no UI change)
Checklist
Added by the opencode bot (AI-generated).
Related: #56288