Repository navigation
Guard git writes outside the bound source worktree and risky shell substitutions; release 0.2.41 (#284) - #285
Conversation
…ell substitutions (#284)
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: 098c6e9819
ℹ️ 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".
| function sourceGuard(command: string, ctx: GitSafetyContext): GitSafetyVerdict { | ||
| let segCwd = ctx.startDir; | ||
| let source: { root: string; id: RepoIdentity } | null | undefined; | ||
| for (const segment of splitSegments(command)) { |
There was a problem hiding this comment.
splitSegments does not split on newlines, so a normal multiline Bash request such as echo preparing\ngit -C <other-worktree> commit -m x is tokenized as one echo segment and the Git write is never examined. The same helper in riskyMutation lets $(echo preparing\ngit push) bypass SHELL-SUBST-01, so both protections can be defeated by routine multiline commands; split unquoted line breaks while preserving heredoc bodies.
Useful? React with 👍 / 👎.
| if (name === "sudo" || name === "command" || name === "builtin" || name === "env") { | ||
| rest = rest.slice(1); | ||
| continue; |
There was a problem hiding this comment.
strip removes sudo or env but leaves their options and option operands at the front, so commands such as env -i git push and sudo -u root git commit leave -i or -u as the first token and parseGitCall rejects them. These commands—and the same forms inside $()—therefore execute mutations while bypassing both new guards; parse the supported wrappers' options and operands before searching for git or gh.
Useful? React with 👍 / 👎.
| while (i < command.length) { | ||
| const ch = command[i]; |
There was a problem hiding this comment.
The scanner continues through unquoted shell comments, so echo ok # $(git push origin main) is denied even though Bash treats the substitution as comment text and never executes it. This can block harmless commands or generated scripts containing commented examples, contrary to the stated rule that only substitutions the shell executes are guarded; ignore comment text through the next newline.
Useful? React with 👍 / 👎.
lidge-jun
changed the title
1 participant
Closes #284.
On 2026-10-01 a session bound to a source worktree with
cxc session sourcepassed Markdown with inline code through a double-quoted shell argument. zsh executed the backticks, andgit cherry-pickran on the main checkout'sdevinstead of the bound worktree. Nothing in codexclaw objected.Both checks below run inside the existing
worktree-guard-pretoolPreToolUse hook. The hook JSON is unchanged, so there is no new hook registration and no new trust hash.--abort/--quitstay allowed.$(...)substitution that the shell will run is denied if its body runs a git write orgh pr merge/close,gh issue close,gh release deleteorgh repo delete. This applies with or without a binding. Single quotes and quoted-delimiter heredocs are treated as literal, andsh/bash/zsh -cpayloads are scanned. Any other backtick inside double quotes gets an allow-plus-advisory.cxc-devgains DEV-SHELL-TEXT-01: generated text goes through apply_patch, files,<<'EOF'or--body-file. The worktree-guardian skill andstructure/INDEX.mdlist the new rules.One acceptance item differs from the issue. Codex hook payloads carry only
tool_input.command; exec_command'sworkdiris not exposed (codex-rsunified_exec/exec_command.rspre_tool_use_payload). The guard therefore resolves the directory from the hook cwd plus in-commandcd,git -C,--git-dirand--work-tree. The allowed retry isgit -C <source> ..., and the deny message says so. Atool_input.workdirfield is honored if a host ever sends one. Directgit pushinside a-cpayload is not denied, because that also appears in legitimate ssh and remote scripts.Validation: the new
git-write-guard.test.ts(8 tests) covers the incident command, nested and-csubstitutions, quoted heredocs, a real linked-worktree binding, theworkdirfield, and envelopes. The existing worktree-guard, hook-e2e, probe-compiled-hooks and L19 suites pass.gate.mjspasses, and the compileddist/cli.js hook worktree-guard-pretoolwas smoke-tested for deny, advisory and silent allow. Locally,npm testfailed onlygui/test/router.test.ts, because this checkout has nonode_modules(reactmissing).The second commit bumps every package, the plugin manifest (
0.2.41+codex.20261006130152), the lockfile and the inventory to 0.2.41, and adds the CHANGELOG entry. It touches the same 15 files as the 0.2.40 release commit, andcheck-versions.mjs 0.2.41,gate.mjsandinventory.mjs --checkpass.