fix: close the v0.22.1 re-audit high-severity findings (HI-01..HI-10)#87
Merged
Conversation
…eader detection; cover NotebookEdit/WebFetch/MCP (HI-06, HI-07, HI-08) HI-06: protect-paths.sh now blocks Bash WRITES/mutations to protected paths (env files, keys, secrets/, .ssh/) via redirection targets (truncations included), tee, in-place stream edits, copy/move/install into the path, and raw device writes. Each alternative embeds the existing protected-path matcher so benign writes are untouched. Interpreter-driven writes are explicitly out of scope for the regex guard and documented as such -- defence in depth, not a sandbox. HI-07: the git-reader matcher tolerates env/command/VAR=val prefixes and an absolute/relative path before the binary, skips git global options (-C, --no-pager, -c k=v, --git-dir, --work-tree), and adds the blame, show-index and bundle readers. Best-effort hardening, not a boundary. HI-08: PreToolUse protect-paths matcher gains NotebookEdit; PostToolUse secret-redact matcher gains WebFetch, NotebookEdit and the MCP wildcard. Both settings.template.json and hooks/hooks.json updated identically; a new test asserts coverage and cross-manifest consistency. Guard stays POSIX-ERE and passes bash -n (shellcheck unavailable in env). Co-Authored-By: Claude Fable 5 <[email protected]> Claude-Session: https://claude.ai/code/session_01KfCEnSJzSG914yyVesjjD6
…honest spawn/metrics classification (HI-01, HI-02, ME-01, ME-02, ME-04)
runTests now executes EVERY detected whitelisted suite (npm/pnpm/yarn/bun/pytest)
instead of the first, aggregating an honest four-state verdict: a PASS requires
every detected required suite to have run and passed. A passing Node suite can no
longer mask a second failing or unexecuted suite. Per-suite detail is exposed as
`executed: [{label,status,exitCode,...}]` and `notExecuted: [labels]`.
Spawn failures that mean the suite never ran (ENOENT, timeout/SIGTERM, EACCES,
ENOEXEC, other signal termination) are classified INCOMPLETE, never FAIL; only a
real non-zero exit code from a suite that actually ran is FAIL (ME-02).
verify() provenance now carries a code-state fingerprint via a new exported
`computeCodeState(cwd)` helper (git HEAD + sha256 of `git diff HEAD` + staged diff
+ sorted untracked file contents), so a completion gate can detect code that
changed after verification. gitAvailable records whether git/diff succeeded so a
detection failure is never reported as clean (HI-02, ME-04).
consensus verifyDeep metrics `outcome` is derived from the final four-state status
(PASS→pass, FAIL/block→block, else the lower-cased status) so an unverified
NOT_CONFIGURED/INCOMPLETE run is no longer counted as a pass (ME-01).
cli verify output lists executed vs skipped suites honestly.
Co-Authored-By: Claude Fable 5 <[email protected]>
Claude-Session: https://claude.ai/code/session_01KfCEnSJzSG914yyVesjjD6
… test-file movement is not proof (HI-02, HI-03, HI-04) HI-02: a fresh `verify` PASS now only counts as test-evidence when its stored codeState.dirtyHash still equals the code state recomputed at Stop. A PASS whose code changed afterward is stale and no longer satisfies the code-change test leg. verify.js gains an exported computeCodeState(cwd) (sha256 of `git diff HEAD` + `git diff --cached` + sorted untracked non-.forge file contents) and stamps provenance.codeState; gate.js imports it and compares, failing toward requiring a test-file change on any doubt (git unavailable / null hash / throw), never throwing. HI-03: the session-start dirty snapshot now stores a per-path content fingerprint (`<sha256>\t<path>`), so a pre-dirty file the agent edits further this session is seen as a real change instead of being hidden. Legacy path-only snapshots degrade gracefully (unknown fingerprint => treat as potentially-changed, never hidden). HI-04: a changed test file is an obligation signal, not proof. The weaker test-evidence leg now requires a test file that still exists and is non-empty (deleted/emptied tests are rejected); the strong leg is the code-state-bound verify PASS. gateDecision keeps back-compat for pure-table callers. The gate stays fail-open, preserves FORGE_STOPGATE=0, the block-once marker, and second-stop-proceeds. Normal "edit code + forge verify + handoff" still passes. Co-Authored-By: Claude Fable 5 <[email protected]> Claude-Session: https://claude.ai/code/session_01KfCEnSJzSG914yyVesjjD6
… profile persist failure; safer symlink/uninstall transactions (HI-05, HI-09, HI-10) - Settings merge records an _forgeOwned ownership manifest of the EXACT entries it added (permissions genuinely absent before, hook guard-identities, statusLine, $schema); removeForgeSettings reverses only those, never a user-owned entry that predates Forge. Ownership is path-aware so a user's custom hook at a different absolute path is never claimed or removed; a legacyOwnedScan seeds the manifest for pre-manifest installs. - init() aborts before sync/gitattributes/settings when writeProfile fails on a corrupt config (HI-09), matching the invalid-name abort — zero side effects. - install.sh backs up a user-owned symlink instead of silently replacing it, and stops (Uninstall INCOMPLETE, non-zero) rather than removing assets that settings hooks still reference when settings cleanup fails (HI-10). Co-Authored-By: Claude Fable 5 <[email protected]> Claude-Session: https://claude.ai/code/session_01KfCEnSJzSG914yyVesjjD6
Consolidated [Unreleased] entries for the verify, gate, guards, and settings/installer workstreams. Co-Authored-By: Claude Fable 5 <[email protected]> Claude-Session: https://claude.ai/code/session_01KfCEnSJzSG914yyVesjjD6
CodeWithJuber
marked this pull request as ready for review
July 19, 2026 09:02
12 tasks
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 & why
Follows the merged critical fixes (CR-01/CR-02, PR #85). This PR closes the ten high-severity findings from the v0.22.1 edge-case re-audit — the audit's remaining P0 plan (P0-2 through P0-6) — as four independently-green workstream commits plus a consolidated changelog.
Verification (HI-01, HI-02, ME-01/02/04) —
src/verify.js,src/stack.js,src/consensus.jsexecuted/notExecuteddetail.codeStatefingerprint (git HEAD + hash of working/staged/untracked changes) so verification is bound to the exact tree it validated.gitAvailableis recorded so a detection failure is never reported as clean.Completion gate (HI-02, HI-03, HI-04) —
src/gate.js,src/session.jsforge verifyPASS counts as evidence only when itscodeStatefingerprint still matches at Stop — code edited after verification no longer passes on a stale stamp.Guards (HI-06, HI-07, HI-08) —
global/guards/protect-paths.sh,global/settings.template.json,hooks/hooks.json> .env,tee,sed -i,cp/mv/install,dd of=).env/command/VAR=prefixes,/usr/bin/git,-C/--no-pager/-c/--git-dir/--work-tree, and addsblame/show-index/bundle.NotebookEdit(writes) andWebFetch/NotebookEdit/mcp__.*(output). Interpreter-driven writes (python -c) remain documented out of scope — defence in depth, not a sandbox.Settings / init / installer (HI-05, HI-09, HI-10) —
src/init.js,install.sh_forgeOwnedmanifest of exactly what it added;--remove-settingsreverses only those, never a user-owned entry that predates Forge. Ownership is path-aware, so a user's custom hook sharing a basename is never claimed/removed; a legacy scan seeds the manifest for old installs.forge init --profileaborts before any side effect when the config is corrupt (matching the invalid-name abort).install.shbacks up a user-owned destination symlink instead of replacing it, and stops (Uninstall INCOMPLETE, non-zero) rather than removing assets that settings hooks still reference when cleanup fails.The parallel worktrees for verify and gate both added
computeCodeState; I reconciled to a single robust implementation (null-delimited filenames, explicit.forge/exclusion so verify writing its own provenance can't perturb the fingerprint the gate rechecks).Integrated branch: 1010 tests / 0 failures, Biome + tsc +
forge docs checkgreen,forge verifyPASS.Checklist
npm testpasses (1010 tests, 0 fail; ~25 new regression tests)npm run checkpasses (Biome)computeCodeState, ownership manifest, multi-suite runner, guard write/reader detection)fix:/security:/docs:)CHANGELOG.mdupdated under## [Unreleased]forge docs checkgreen)Risk & rollback
FORGE_STOPGATE=0, block-once, second-stop-proceeds) preserved and tested.Extra checks (tick if applicable)
npm run typecheckpassestest/_fixtures.js)🤖 Generated with Claude Code
https://claude.ai/code/session_01KfCEnSJzSG914yyVesjjD6
Generated by Claude Code