fix(product): no model-typed value from untrusted input reaches a shell line before tested code validates it (6.30.0) - #292
Merged
Conversation
…ll line before tested code validates it A feature name derived from the user's description is now written with the Write tool to .pharn/feature-name/candidate.txt and printed back only as a FEATURE_SLUG_RE member by the new pharn/floor/feature-name.mjs, before any shell line carries it (/pharn-spec Step 0, /pharn-loop S1/S2). The seven name-taking commands ask for a missing name or resolve it only through the CLI. /pharn-loop Step 6d returns with the constant `git checkout - --`, and /pharn-ship --quick item 7 no longer takes a base ref from the description. SHELL-SINK pins in command-hygiene.test.mjs close the shell-value and name-origin tables both ways and execute the committed lines on hostile names. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
Important
This repository does not receive automatic reviews because it has fewer than 10 stars. ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Advanced Run ID: Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
PrzemekGalarowicz
added a commit
that referenced
this pull request
Sep 27, 2026
…released 6.30.0) Only lines this branch adds against origin/main move (plus SKILLS_VERSION and the README badge); main's ## [6.30.0] section stays byte-for-byte below ## [6.31.0]. The first renumber's history lines are kept, and the version parentheticals now name both renumbers. SHIP.md records the merge and this commit. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
PrzemekGalarowicz
added a commit
that referenced
this pull request
Sep 27, 2026
… it is judged by; unrelated test anomalies no longer void the record (6.31.0) (#291) * fix(floor): the AC gate — a PLAN cannot scope the test infrastructure it is judged by; unrelated test anomalies no longer void the record H2: check-ac-tests.mjs gains ac-artifact-in-plan and widens test-infra-in-plan to package-manager configs and script-named files; the lock pin (ac-tests-lock/4) covers chained scripts, script-named files, the jest key and .npmrc/.yarnrc via one closed literal token pass. M6: per-test anomalies no longer refuse the whole record; they decide only the ACs whose mapped files hold them and are reported otherwise. Lesson L65 promoted (accept delegated to the orchestrating model). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * chore: renumber ac-gate-plan-scope to 6.30.0 after merging main (#290 released 6.29.0) Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * chore(ac-gate-plan-scope): post-merge regress and verify records, and SHIP.md Regress on the merged tree (base c1bf663): no-regressions. Verify: every gate exits 0 but reconcile, whose 50 escapes are exactly files origin/main changed after the build's anchor (set difference empty; VERIFY.md). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * test(check-ac-tests): the measured hook table no longer assumes a case-insensitive volume On ext4 a case variant of the lock is a different, new file: the guard allows it and it opens nothing, so the APFS-measured 'opens' column read true on CI. The table now keeps the filesystem-independent fact (the build may write a file it is judged by, decided by dev+inode) apart from the raw guard reading, which follows a run-time case-sensitivity probe of the temp volume. The four case-variant rows are the probe-dependent ones; isRed stays unconditional (the fold is fail-closed). A pure test injects both probe results. Measured on APFS and on a case-sensitive APFS image: 60/60 on both. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * chore: renumber ac-gate-plan-scope to 6.31.0 after merging main (#292 released 6.30.0) Only lines this branch adds against origin/main move (plus SKILLS_VERSION and the README badge); main's ## [6.30.0] section stays byte-for-byte below ## [6.31.0]. The first renumber's history lines are kept, and the version parentheticals now name both renumbers. SHIP.md records the merge and this commit. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
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.
What
A read-only injection audit of the product commands' pinned lines found three places where a value the model derives
from untrusted input reached a shell before anything checked it. Each was reproduced against the 6.28.2 lines in a
throwaway directory.
/pharn-specStep 0, and so/pharn-ship;/pharn-loopS1): the only check ran inside a nodeprocess, after the shell had parsed the line carrying the candidate.
/pharn-loop's own validator ranx'$(touch PWNED)'and exited 0./pharn-loopStep 6d typedgit symbolic-refoutput intogit switch '<original branch>', so a hostile branchname ran its command on a failed commit.
/pharn-ship --quickitem 7 took a--base <ref>the command has no flag for, so the ref came from thedescription.
How
pharn/floor/feature-name.mjs(header = spec). The model writes the slug with the Write tool to.pharn/feature-name/candidate.txt. The CLI refuses a symlinked or non-directory parent, reads the file withoutfollowing it, removes it, and prints it only as a
FEATURE_SLUG_REmember.--freshreplaces the loop's S2 shellloop.
<name>shell line.through the CLI.
git checkout - --, with its bound stated.--model-approve.feature-name.test.mjs: 41 tests, 5/5 mutants killed.command-hygiene.test.mjs: both tables closed both ways, order and sentence pins (presenceonly), the committed lines executed over hostile names, and the 6.28.2 lines shown to run their payloads.
SKILLS_VERSION6.29.0 → 6.30.0 (minor, a new floor CLI; this PR merges ahead of #291).MIN_CLIstays0.5.0.
Floor verdicts
validate: GREEN./pharn-dev-regress:no-regressions. This was a re-run under low load; the first run's two wall-clock failures arerecorded in
REGRESSION.md./pharn-dev-verify:PASS(4,122/4,122 tests,reconcileCLEAN before the merge)./pharn-dev-review: GREEN, 0 floor findings. Advisory 1 and 2 are fixed here; 3 and 4 are recorded only.git merge origin/main, all checks below ran GREEN or exit 0:check:changelog-entryagainstc1bf663;check:changelog,check:badge,docs:check,validate;command-hygieneandfeature-nametests (312/312);format:check,lint,lint:md.reconcilereads ESCAPE on 32 files. By set difference, every one is a fileorigin/mainchanged: the expectedmerge case.
Bounds (advisory)
The pins read command text, never a run, so three things are advisory:
One candidate file serves the whole tree.
Gates were decided by the orchestrating model under the maintainer's delegation, not by a human. Audit trail:
.dev/features/shell-sink-validation/.🤖 Generated with Claude Code