fix: close the v0.22.1 re-audit high-severity findings (HI-01..HI-10)

CHANGELOG.md

@@ -22,6 +22,52 @@ to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
   comments no longer claim strict mode "blocks": PostToolUse fires after the tool ran, so
   exit 2 is surfaced stderr feedback, not enforcement — blocking belongs to the
   PreToolUse guard. A wrapper-level regression test drives the real `secret-redact.sh`.
+- **Guard blocks shell writes to protected paths (HI-06).** The PreToolUse guard now blocks
+  writes/mutations to protected paths (`.env`, keys, `secrets/`, `.ssh/`) via redirections and
+  truncations (`> .env`, `: > .env`), `tee`, `sed -i`, `cp`/`mv`/`install`, and `dd of=`, not
+  just reads. Interpreter-driven writes (`python -c`, `node -e`) remain out of scope and are
+  documented as such — this is defence in depth, not a sandbox.
+- **Hardened git secret-reader detection (HI-07).** Reader detection now sees through wrappers
+  and global options (`env`/`command`/`VAR=val` prefixes, `/usr/bin/git`, `-C`/`--no-pager`/`-c`/
+  `--git-dir`/`--work-tree`) and covers `git blame`/`show-index`/`bundle`.
+- **Broader hook tool coverage (HI-08).** The protect-paths PreToolUse matcher now includes
+  `NotebookEdit`, and the secret-redact PostToolUse matcher includes `WebFetch`, `NotebookEdit`,
+  and MCP tools (`mcp__.*`); the settings template and plugin manifest stay in sync.
+
+### Fixed
+
+- **`forge verify` runs every detected suite (HI-01).** A polyglot repo where a passing Node
+  suite hid a failing or unexecuted pytest suite no longer reports PASS — every detected
+  executable suite runs, and a non-executable or missing one makes the overall result
+  INCOMPLETE. Per-suite `executed`/`notExecuted` detail is recorded.
+- **Verification is bound to the final code state (HI-02).** Provenance records a `codeState`
+  fingerprint (git HEAD + a hash of working/staged/untracked changes); the completion gate only
+  counts a `forge verify` PASS as evidence when that fingerprint still matches at Stop, so code
+  edited _after_ verification no longer passes on a stale stamp.
+- **Completion gate detects edits to pre-dirty files (HI-03).** The session baseline stores a
+  content fingerprint per already-dirty file, so further edits to it during the session are seen
+  instead of hidden by its start-of-session dirty state.
+- **A changed test file is no longer proof (HI-04).** A deleted or empty test file no longer
+  satisfies the code-change evidence requirement; the test leg needs a substantive (existing,
+  non-empty) test file or a code-state-bound verify PASS.
+- **Honest spawn and metrics classification (ME-01, ME-02).** Spawn failures (missing binary,
+  permission/exec-format errors, signal kills, timeout) are reported INCOMPLETE rather than a
+  false test FAIL — only a real non-zero exit counts as FAIL; deep-verify metrics no longer
+  count NOT_CONFIGURED/INCOMPLETE runs as a pass. Provenance records `gitAvailable` so a failure
+  to detect changed files is never reported as a clean tree (ME-04).
+- **Ownership-safe settings uninstall (HI-05).** The settings merge records an `_forgeOwned`
+  manifest of exactly what it added (permissions genuinely absent before, hook guard-identities,
+  statusLine, `$schema`); `forge init --remove-settings` reverses only those, never a user-owned
+  entry that predates Forge. Ownership is path-aware, so a user's custom hook that merely shares
+  a basename with a Forge hook is never claimed or removed; a legacy scan seeds the manifest for
+  pre-manifest installs.
+- **Init aborts on profile-persistence failure (HI-09).** When `forge init --profile` can't
+  persist because `.forge/forge.config.json` is corrupt, init now aborts before sync,
+  `.gitattributes`, and the settings merge — zero side effects — instead of continuing and
+  reporting the error afterward.
+- **Safer installer transactions (HI-10).** `install.sh` backs up a user-owned symlink at a
+  destination instead of silently replacing it, and stops (`Uninstall INCOMPLETE`, non-zero)
+  rather than removing assets that settings hooks still reference when settings cleanup fails.
 
 ### Changed
 

global/guards/protect-paths.sh

@@ -45,16 +45,43 @@ if [ -n "${cmd:-}" ]; then
   # message mentioning ".env") isn't a false positive, AND require a protected path token.
   # Best-effort defence in depth — a content scan like `rg TOKEN .` with no named path can't
   # be caught here; that's what secret-redact.sh is for.
-  # Git subcommands that can print file/history content (RA-05): show, log, diff,
-  # stash (show -p), cat-file, archive, grep. Subcommand may be followed by a space
-  # or end the command string.
-  reader='(^|[;&|])[[:space:]]*((cat|less|more|head|tail|nl|xxd|od|strings|base64|rg|grep|ag)[[:space:]]|git[[:space:]]+(show|log|diff|stash|cat-file|archive|grep)([[:space:]]|$))'
+  #
+  # SCOPE: this is a POSIX-ERE regex guard, not a sandbox. Regex cannot parse shell, so
+  # interpreter-driven access/writes — `python -c 'open(".env","w")…'`, `node -e …`, `perl -e`
+  # — are DELIBERATELY out of scope here. This layer sits behind the permission system and
+  # secret-redact.sh; treat every match/miss as best-effort hardening, never a boundary.
+  #
+  # Git subcommands that can print file/history content (RA-05, HI-07): show, log, diff,
+  # stash (show -p), cat-file, archive, grep, blame, show-index, bundle. Subcommand may be
+  # followed by a space or end the command string.
+  # HI-07 (best-effort hardening): tolerate an optional `env `/`command ` or `VAR=val `
+  # prefix and an optional absolute/relative path before `git` (e.g. `/usr/bin/git`), then
+  # skip git's own global options (`-C <dir>`, `--no-pager`, `-c k=v`, `--git-dir=…`,
+  # `--work-tree=…`) between `git` and the subcommand. A wrapper we don't model can still slip.
+  gitpfx='([[:alnum:]_]+=[^[:space:]]+[[:space:]]+|(env|command)[[:space:]]+)*([^[:space:]]*/)?git[[:space:]]+'
+  gitopt='(-C[[:space:]]+[^[:space:]]+[[:space:]]+|--no-pager[[:space:]]+|-c[[:space:]]+[^[:space:]]+[[:space:]]+|--git-dir=[^[:space:]]+[[:space:]]+|--work-tree=[^[:space:]]+[[:space:]]+)*'
+  gitsub='(show|log|diff|stash|cat-file|archive|grep|blame|show-index|bundle)([[:space:]]|$)'
+  reader="(^|[;&|])[[:space:]]*((cat|less|more|head|tail|nl|xxd|od|strings|base64|rg|grep|ag)[[:space:]]|${gitpfx}${gitopt}${gitsub})"
   # \b anchors the extensions so `.key` matches a real key file but NOT `Object.keys`,
   # and `.env` matches `.env`/`.env.prod` but NOT `.environment`.
   secret='(\.env(\.[A-Za-z0-9_-]+)?\b|id_rsa\b|id_ed25519\b|\.pem\b|\.key\b|/secrets/|/\.ssh/)'
   if printf '%s' "$cmd" | grep -qE "$reader" && printf '%s' "$cmd" | grep -qE "$secret"; then
     deny "reading a protected secret path via Bash is blocked. Read it yourself if intended."
   fi
+  # Close the Bash secret-WRITE bypass (HI-06): the Edit/Write guard covers tool writes, but a
+  # shell `echo x > .env`, `printf … >> .env`, `tee .env`, `sed -i … .env`, `cp/mv/install X
+  # .env`, `dd of=.env`, or a truncation (`> .env`, `: > .env`) mutates a protected file past
+  # it. Detect a protected-path token appearing (a) as a redirection target (`>`/`>>` then the
+  # path) or (b) as an argument to a known mutating command. Interpreter writes are out of
+  # scope (see SCOPE note above). Each alternative embeds "$secret", so a bare `echo hi >
+  # out.txt` (no protected token) is never blocked.
+  w_redir=">>?[[:space:]]*[\"']?[^[:space:]<>|;&]*${secret}"
+  w_cmd="(^|[;&|])[[:space:]]*(${gitpfx})?(tee([[:space:]]+-a)?|cp|mv|install)[[:space:]]+[^;&|]*${secret}"
+  w_sed="(^|[;&|])[[:space:]]*sed[[:space:]]+[^;&|]*-i[^;&|]*${secret}"
+  w_dd="(^|[;&|])[[:space:]]*dd[[:space:]]+([^;&|]*[[:space:]])?of=[^[:space:]]*${secret}"
+  if printf '%s' "$cmd" | grep -qE "${w_redir}|${w_cmd}|${w_sed}|${w_dd}"; then
+    deny "writing to a protected secret path via Bash is blocked. Edit it yourself if intended."
+  fi
   # Pipe-to-shell (e.g. curl … | sh). Boundary-aware so legit `… | shellcheck` is not caught.
   if [[ "$cmd" =~ \|[[:space:]]*(sh|bash|zsh)([[:space:]]|$) ]]; then
     deny "piping content to a shell is blocked."

global/settings.template.json

@@ -102,7 +102,7 @@
     ],
     "PreToolUse": [
       {
-        "matcher": "Edit|Write|MultiEdit|Bash",
+        "matcher": "Edit|Write|MultiEdit|NotebookEdit|Bash",
         "hooks": [
           {
             "type": "command",
@@ -143,7 +143,7 @@
         ]
       },
       {
-        "matcher": "Bash|Read|Grep",
+        "matcher": "Bash|Read|Grep|WebFetch|NotebookEdit|mcp__.*",
         "hooks": [
           {
             "type": "command",

hooks/hooks.json

@@ -31,7 +31,7 @@
     ],
     "PreToolUse": [
       {
-        "matcher": "Edit|Write|MultiEdit|Bash",
+        "matcher": "Edit|Write|MultiEdit|NotebookEdit|Bash",
         "hooks": [
           {
             "type": "command",
@@ -72,7 +72,7 @@
         ]
       },
       {
-        "matcher": "Bash|Read|Grep",
+        "matcher": "Bash|Read|Grep|WebFetch|NotebookEdit|mcp__.*",
         "hooks": [
           {
             "type": "command",

install.sh

@@ -35,11 +35,19 @@ say() { printf '  %s\n' "$*"; }
 # Run argv directly (no eval — S-02). In dry-run, print the argv instead of executing.
 act() { if [ "$DRY" = 1 ]; then say "[dry-run] $*"; else "$@"; fi; }
 
-# link SRC DEST — back up an existing real file/dir, then symlink.
+# link SRC DEST — back up an existing real file/dir OR a foreign symlink, then symlink.
+# A symlink already pointing into this repo (Forge's own) is refreshed silently; any other
+# existing symlink is a USER symlink and is backed up before being replaced, so `ln -sfn`
+# never silently destroys it (HI-10).
 link() {
   local src="$1" dest="$2"
   [ -e "$src" ] || { say "skip (missing in bundle): $src"; return; }
-  if [ -e "$dest" ] && [ ! -L "$dest" ]; then
+  if [ -L "$dest" ]; then
+    case "$(readlink "$dest")" in
+      "$REPO"/*) : ;;  # our own symlink — safe to refresh in place
+      *) act mv "$dest" "$dest.forge-bak-$STAMP"; say "backed up existing symlink $dest" ;;
+    esac
+  elif [ -e "$dest" ]; then
     act mv "$dest" "$dest.forge-bak-$STAMP"; say "backed up existing $dest"
   fi
   act mkdir -p "$(dirname "$dest")"
@@ -136,7 +144,14 @@ uninstall_forge() {
       printf '%s\n' "$REMOVE_OUT" | sed 's/^/  /'
     else
       [ -n "$REMOVE_OUT" ] && printf '%s\n' "$REMOVE_OUT" | sed 's/^/  /'
-      say "note: automatic settings cleanup failed — remove the Forge hook/permission/statusline entries from $CLAUDE_DIR/settings.json by hand, or fix the file and run: forge init --remove-settings"
+      # Removing the asset symlinks now would leave GLOBAL settings hooks firing scripts that
+      # no longer exist — the dangerous case (HI-10). Stop with the assets intact instead of
+      # printing a false "Done."
+      cat >&2 <<EOF
+
+  Uninstall INCOMPLETE — settings still reference Forge hooks; fix $CLAUDE_DIR/settings.json then re-run \`install.sh --uninstall\` (asset symlinks left in place)
+EOF
+      exit 1
     fi
   else
     say "note: node not found — remove the Forge hook/permission/statusline entries from $CLAUDE_DIR/settings.json by hand, or run \`forge init --remove-settings\` once node is available"

src/cli.js

@@ -1116,6 +1116,12 @@ async function run(argv) {
           !t.runner && t.detected?.length ? ` (detected: ${t.detected.join(", ")})` : ""
         }`,
       );
+      if (t.executed?.length)
+        console.log(
+          `  suites ran:       ${t.executed.map((s) => `${s.label}=${s.status}`).join(", ")}`,
+        );
+      if (t.notExecuted?.length)
+        console.log(`  suites skipped:   ${t.notExecuted.join(", ")} (no built-in executor)`);
       const detail = t.output || (t.detected ?? []).join(", ");
       const verdictLine =
         r.status === "PASS"
@@ -1150,6 +1156,14 @@ async function run(argv) {
             ? `— INCOMPLETE: ${t.output || (t.detected ?? []).join(", ") || "test run did not complete"}`
             : "— NOT CONFIGURED (no test runner detected)";
     console.log(`  tests:            ${testsLine}`);
+    // Per-suite honesty (HI-01): a polyglot repo runs every executable suite; name which
+    // ones ran with what verdict, and which were detected but never executed.
+    if (t.executed?.length)
+      console.log(
+        `  suites ran:       ${t.executed.map((s) => `${s.label}=${s.status}`).join(", ")}`,
+      );
+    if (t.notExecuted?.length)
+      console.log(`  suites skipped:   ${t.notExecuted.join(", ")} (no built-in executor)`);
     console.log(`  symbols checked:  ${r.provenance.symbolsChecked}`);
     if (r.unknown.length)
       console.log(

src/consensus.js

@@ -349,9 +349,16 @@ export function verifyDeep({
   } catch {}
   recordMetric(targetRoot, {
     stage: "verify",
-    // `outcome` keeps its historical block|pass vocabulary; the four-state verdict
-    // rides along additively as `status`.
-    outcome: verdict.block ? "block" : "pass",
+    // `outcome` is derived from the FINAL four-state status, not `verdict.block` alone
+    // (ME-01): an unverified run (NOT_CONFIGURED / INCOMPLETE) must never be counted as a
+    // "pass". PASS→pass, FAIL/block→block, and the unverified states carry their own
+    // lower-cased label. The four-state verdict still rides along additively as `status`.
+    outcome:
+      status === "PASS"
+        ? "pass"
+        : status === "FAIL" || verdict.block
+          ? "block"
+          : status.toLowerCase(),
     status,
     mode: "deep",
     lenses: deep.lenses.filter((l) => l.ran).length,