Repository navigation
fix(O168): a script's arguments are not deletions (write-target false positives) - #156
Conversation
An interpreter run is uncertain, so every absolute mention went through the ancestor rule: python3 x.py $HOME, node x.js "$PWD", . venv/bin/ activate && python -V were reported as deleting home / the workspace root, and a non-git $PWD was labeled git internals. The exemption applies only while the path is named nowhere else (cd-exemption semantics; source ./evil.sh; ls <root> still hits), across both shell forms. o127 P2 splits accordingly (P2a documents the new contract).
…arg-writes # Conflicts: # CHANGELOG.md # README.en.md # README.md
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: be8108f457
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| for (const word of command.split(/[\s'"`;&|()<>=]+/)) { | ||
| if (!word || argTokens.has(word)) continue; |
There was a problem hiding this comment.
When another command repeats an exempt argument verbatim, argTokens.has(word) skips that occurrence too, so the promised “named nowhere else” check never cancels the exemption. For example, with the workspace at ../proj, node benign.js ../proj; xargs rm -rf ../proj </dev/null now returns no protected-path hits, whereas the parent commit blocks it. Locally, the headless CLI executed this command and deleted the workspace's README before the post-run check reported damage. The inspected xargs --help confirms it runs COMMAND with “arguments INITIAL-ARGS”; the second occurrence is therefore a deletion operand, not a script argument. Remove only the identified script-argument occurrences before checking the remaining command.
Useful? React with 👍 / 👎.
| if (rest.some((a) => INLINE_CODE_FLAGS.has(stripEdgeQuotes(a)))) continue; | ||
| const scriptIdx = rest.findIndex((a) => !stripEdgeQuotes(a).startsWith('-')); | ||
| if (scriptIdx < 0) continue; | ||
| for (const arg of rest.slice(scriptIdx + 1)) add(arg); |
There was a problem hiding this comment.
When a script prints its path argument into a mutating downstream command, the path is named only as a script argument but still becomes a deletion target. With echo.js containing console.log(process.argv[2]), node echo.js /absolute/workspace | xargs rm -rf now returns no protected-path hits; the parent commit blocks it. A local headless CLI reproduction deleted the workspace's README, and snapshot restoration did not recover it. The inspected xargs --help explicitly says it runs COMMAND with “more arguments read from input.” Apply downstream-pipeline safety checks before granting this exemption, as the existing read-only-root exemption does.
Useful? React with 👍 / 👎.
…arg-writes # Conflicts: # CHANGELOG.md # README.en.md # README.md
O168 (P1): O101/O151 write-target rules blocked normal commands
Repro on main: an interpreter run is "uncertain" (a script can write
anything), so every absolute mention went through the delete-ancestor rule —
python3 x.py $HOME→ reported as deleting~(ssh);node x.js "$PWD"→ reported as deleting the workspace root, and in aNON-git directory the root was labeled "git repository internals" (the
category itself was wrong);
python3 x.py /tmp,. venv/bin/activate && python -V(the bare.resolves to the workspace root) → same ancestor hit;
sourcewasinconsistent with
..Fix (
src/safety/protected-paths.ts)scriptArgumentMentionscollects the arguments of an invoked script — theinterpreter script form (
python3 x.py ARG…),sh x.sh ARG…, the./sourceoperand and the bare.— across both shell forms (pwshseparator form included). The mentions loop then skips only the
delete-ancestor rule for them, exactly like the cd exemption (O127) and the
read-only-root exemption (O151):
source ./evil.sh; ls <root>still hits (pinned by o151);(
python3 x.py $HOME/.ssh,.git/config);-c/-e) keep the old behavior;rm -rf $HOME,mv $HOME /tmp,rm -rf "$PWD"), and actual writes remain covered by the script scan(layer 2) and the post-run snapshot (layer 3, O152).
Tests
test/o168-interpreter-arg-writes.spec.mjs(22): policy GREEN for thefour QA commands +
sh/bash x.shforms + option-prefixed interpreter;policy RED for direct protected args, real deletes, and the inline form;
process e2e (
dist/cli.js -p, real benign scripts): the four commandsexit 0; control
python3 x.py $HOME/.sshstill exit 9.o127P2 split: P2a documents the new contract (root as script arg is nota deletion); P2b keeps rm-of-root / writes-under-.git / inline rmSync
blocked.
o151204/204 unchanged (incl.source ./evil.sh; ls <root>),o10124/24,o10374/74,o11751/51.Revert-red (local, macOS)
Neutralizing the exemption (
isScriptArg = false) → 23 red (all GREENpolicy cases + the three process e2e cases); with the fix 22/22. Two
iterations were needed to keep o151/o127 green (pwsh separator form union,
trailing-slash token normalization) — both covered by the suites above.
Head
be8108f4includes main at9d455d94(#155 / O169).npm run checkgreen.