fix(code-review): reject resolve-base.sh against repo top-level, not CWD#12
Open
fix(code-review): reject resolve-base.sh against repo top-level, not CWD#12
Conversation
The CWD-rejection guard for `${CLAUDE_SKILL_DIR}/scripts/resolve-base.sh`
compared `real_base` against `pwd -P`. In a monorepo where the reviewer is
launched from a subdirectory, a harness value like `<repo-root>/.evil`
sits outside the CWD subdir but is still repo-controlled and would be
accepted, allowing arbitrary code execution from the reviewed repo.
Compare against the repo top-level (`git rev-parse --show-toplevel`)
instead, falling back to `pwd -P` only when not inside a git repo. Add a
contract test that exercises the monorepo-subdirectory attack and pins
the new probe shape.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Follow-up to #11 / EveryInc#783. Codex review on EveryInc#783 flagged that the CWD-rejection guard for
${CLAUDE_SKILL_DIR}/scripts/resolve-base.shcomparesreal_baseagainstpwd -P— the caller's current directory — not the reviewed-repo top-level. The bypass:pwd -P=<repo-root>/services/foo.${CLAUDE_SKILL_DIR}to<repo-root>/.evil(a sibling of the subdir).real_baseis outside the CWD subdir, so thecaseclause does not match and the path is accepted.bash \"$RESOLVE_SCRIPT\"then executes a repo-controlled script.The valid (and intended) boundary is the reviewed repo's top-level, not the caller's CWD.
What changed
real_repofromgit rev-parse --show-toplevel, with apwd -Pfallback when the CWD is not inside a git repo. Thecasematch rejectsreal_baseequal to or nested underreal_repo.git rev-parse --show-toplevelmust appear in SKILL.md) and adds an end-to-end monorepo-subdirectory attack scenario:git inita fake repo, set CWD to a subdir, pointCLAUDE_SKILL_DIRat<root>/.evil, assert the probe fails closed and the decoy never runs.Verification
bun test tests/review-skill-contract.test.ts— 26 pass.bun test— full suite (1240 tests) passes.bun run release:validate— clean.Test plan