chore(ci)(deps): bump actions/setup-node from 4.0.3 to 6.3.0#2
Closed
dependabot[bot] wants to merge 1 commit intomainfrom
Closed
chore(ci)(deps): bump actions/setup-node from 4.0.3 to 6.3.0#2dependabot[bot] wants to merge 1 commit intomainfrom
dependabot[bot] wants to merge 1 commit intomainfrom
Conversation
Author
LabelsThe following labels could not be found: Please fix the above issues or remove invalid values from |
dependabot
Bot
force-pushed
the
dependabot/github_actions/actions/setup-node-6.3.0
branch
from
75d27b7 to
eefb050
Compare
himerus
pushed a commit
that referenced
this pull request
Apr 18, 2026
- Replace +++ patch-header scrape with git diff --name-status so file DELETIONS under protected paths also require Codex review (#1). - Parse push refspecs correctly: split on ':', take destination, strip 'refs/heads/' / 'refs/for/', reject bare 'HEAD' as target. Prior regex let 'git push origin HEAD:main' collapse diff to empty (#2). - Replace two-grep audit scan with jq -e structural predicate enforcing tool_name == "codex.review" AND metadata.head_sha == $sha AND metadata.verdict not in {blocking, error}. Prior greps accepted any audit line with matching substrings inside arbitrary metadata (#3). - Fail-closed on every parse error. jq still guarded at hook entry. Updates codex-event.ts docstring to describe the jq predicate instead of the old substring match (which is now actively misleading). Signed-off-by: Jake Strawn <bandy.strawn@clarityhouse.press>
himerus
pushed a commit
that referenced
this pull request
Apr 18, 2026
- Replace +++ patch-header scrape with git diff --name-status so file DELETIONS under protected paths also require Codex review (#1). - Parse push refspecs correctly: split on ':', take destination, strip 'refs/heads/' / 'refs/for/', reject bare 'HEAD' as target. Prior regex let 'git push origin HEAD:main' collapse diff to empty (#2). - Replace two-grep audit scan with jq -e structural predicate enforcing tool_name == "codex.review" AND metadata.head_sha == $sha AND metadata.verdict not in {blocking, error}. Prior greps accepted any audit line with matching substrings inside arbitrary metadata (#3). - Fail-closed on every parse error. jq still guarded at hook entry. Updates codex-event.ts docstring to describe the jq predicate instead of the old substring match (which is now actively misleading). Signed-off-by: Jake Strawn <bandy.strawn@clarityhouse.press>
himerus
added a commit
that referenced
this pull request
Apr 18, 2026
#14) * feat(audit): add metadata field and public @bookedsolid/rea/audit helper Attach optional metadata to the AuditRecord schema and emit caller-supplied keys from ctx.metadata through the audit middleware (skipping the reserved autonomy_level key kept for internal bookkeeping). Add src/audit/append.ts as a standalone helper that reads the tail of .rea/audit.jsonl for prev_hash, computes the SHA-256 hash, and appends atomically with fsync. Exported as @bookedsolid/rea/audit so the codex-adversarial agent and downstream consumers (Helix helix.plan / helix.apply, future plugins) can emit structured events through the same hash chain. Add src/audit/codex-event.ts as the single source of truth for the codex.review event shape, shared between the TypeScript helper and the push-review-gate shell hook. Signed-off-by: Jake Strawn <bandy.strawn@clarityhouse.press> * feat(hooks): enforce Codex adversarial review on protected-path pushes Extend push-review-gate.sh to block git push when the diff touches any of src/gateway/middleware/, hooks/, src/policy/, or .github/workflows/ unless .rea/audit.jsonl contains a codex.review entry for the current HEAD. The grep pattern matches the constants in src/audit/codex-event.ts — keep both in lockstep if either changes. Document the audit-append responsibility in agents/codex-adversarial.md with a concrete example using the public @bookedsolid/rea/audit helper. Deliberate non-action on commit-review-gate: commit-side enforcement would double friction without adding safety, since nothing lands remote without passing the push gate. The rationale is captured in the push-gate header so a future reader does not 'fix' the missing commit-side check. Signed-off-by: Jake Strawn <bandy.strawn@clarityhouse.press> * feat(gateway): MCP server, downstream pool, registry loader, smoke tests Implement the rea serve gateway on top of @modelcontextprotocol/sdk 1.29: - src/registry/{types,loader}.ts — zod-validated RegistrySchema with the same TTL + mtime-invalidation cache pattern as src/policy/loader.ts. Server names constrained to lowercase-kebab. - src/gateway/downstream.ts — per-server DownstreamConnection wrapping a Client + StdioClientTransport pair. One reconnect on transport error, then mark unhealthy and let the circuit-breaker middleware take over. - src/gateway/downstream-pool.ts — Map<serverName, DownstreamConnection> with <serverName>__<toolName> prefix routing. Split on first __ so downstream tools that themselves contain __ still work. - src/gateway/server.ts — upstream Server bound to the full 10-layer middleware chain: audit → kill-switch → tier → policy → blocked-paths → rate-limit → circuit-breaker → injection → redact → result-size-cap → terminal. Zero-server mode boots cleanly with an empty catalog. - src/gateway/session.ts — per-process UUID session_id stable for the lifetime of rea serve. - server.test.ts — smoke tests via InMemoryTransport covering zero-server listTools, zero-server callTool denied, HALT denial, and tier classification. Signed-off-by: Jake Strawn <bandy.strawn@clarityhouse.press> * feat(cli): rewrite `rea serve` as real MCP gateway with graceful shutdown - `src/cli/serve.ts` loads `.rea/policy.yaml` + `.rea/registry.yaml`, creates the gateway, and connects StdioServerTransport. SIGTERM / SIGINT drain in-flight work and close the downstream pool before exit. - `src/cli/index.ts` adds `--force` and `--accept-dropped-fields` flags on `rea init` (consumed by the upcoming install pipeline). Zero-server registries boot cleanly and advertise an empty tool catalog so first-run does not crash. Signed-off-by: Jake Strawn <bandy.strawn@clarityhouse.press> * feat(policy): layered profile schema and five shipped profiles - `src/policy/profiles.ts` introduces a zod-strict `ProfileSchema` with all fields optional, the `HARD_DEFAULTS` layer, and `mergeProfiles` / `loadProfile` helpers. Merge order is `hardDefaults ← profile ← reagentTranslation ← wizardAnswers` so each later layer can only narrow the preceding one (autonomy ceilings always clamp). - Five profiles under `profiles/`: `minimal`, `bst-internal` (what this repo dogfoods), `open-source`, `client-engagement`, and `lit-wc`. Each is a literal fragment — no `extends` chains — so the materialized `.rea/policy.yaml` on disk is the full source of truth for what the middleware enforces. Signed-off-by: Jake Strawn <bandy.strawn@clarityhouse.press> * feat(cli): install pipeline — copy, settings merge, commit-msg, claude-md, reagent Makes `rea init` a real installer instead of a stub. New modules under `src/cli/install/`: - `copy.ts` — copies `hooks/**`, `commands/**`, `agents/**` into `.claude/`, chmods hooks `0o755`, conflict policy per flag (`--force` overwrites, `--yes` skips existing, otherwise interactive prompt). - `settings-merge.ts` — pure merge into `.claude/settings.json`; never silently overwrites consumer hooks; warns only when chaining onto a pre-existing matcher (novel-matcher additions on a fresh install produce exactly one informational notice per matcher, not per hook). - `commit-msg.ts` — belt-and-suspenders install of `.git/hooks/commit-msg` (and `.husky/commit-msg` when husky is present); respects `core.hooksPath`. - `claude-md.ts` — managed fragment inside `CLAUDE.md` delimited by `<!-- rea:managed:start v=1 -->` / `<!-- rea:managed:end -->`; content outside the markers is never touched. - `reagent.ts` — field-for-field translator with explicit copy / drop / ignore lists. Drop-list fields refuse translation without `--accept-dropped-fields` to prevent silent security downgrades; autonomy is clamped to the profile ceiling. Each module ships with vitest coverage (`copy.test.ts`, `settings-merge.test.ts`, `reagent.test.ts`). Signed-off-by: Jake Strawn <bandy.strawn@clarityhouse.press> * feat(cli): wire init, expand doctor to 9 checks, changeset for 0.2.0 - `src/cli/init.ts` is rewritten to drive the full install pipeline: load profile, optionally translate an existing reagent policy, merge wizard answers, materialize `.rea/policy.yaml` as a literal, copy artifacts via the new install modules, merge settings atomically, install the commit-msg hook, and update the CLAUDE.md managed fragment. - `src/cli/doctor.ts` grows to 9 checks (`.rea` dir, policy parses, registry parses, agents count, hook executability, settings matchers present, commit-msg hook installed, codex-adversarial agent + command, registry parse roundtrip). Exit code reflects the worst check. - `package.json` adds the `./audit` subpath export (public API for the adversarial-review helper) and includes `.husky/` in `files[]` so the husky source ships to consumers. - `.changeset/0.2.0-mvp.md` — minor bump documenting Tracks 1/2/3 and the explicit deferrals to the full 0.2.0 cycle. Signed-off-by: Jake Strawn <bandy.strawn@clarityhouse.press> * docs(codex): use colon-form slash commands (/codex:review, /codex:adversarial-review) The Codex plugin exposes commands as /codex:review and /codex:adversarial-review. Our docs were using space-form which would break invocation. Note: THREAT_MODEL.md has one remaining occurrence of the old form but is in blocked_paths and requires a direct maintainer edit. Signed-off-by: Jake Strawn <bandy.strawn@clarityhouse.press> * fix(hooks): close three push-review-gate bypasses surfaced by codex - Replace +++ patch-header scrape with git diff --name-status so file DELETIONS under protected paths also require Codex review (#1). - Parse push refspecs correctly: split on ':', take destination, strip 'refs/heads/' / 'refs/for/', reject bare 'HEAD' as target. Prior regex let 'git push origin HEAD:main' collapse diff to empty (#2). - Replace two-grep audit scan with jq -e structural predicate enforcing tool_name == "codex.review" AND metadata.head_sha == $sha AND metadata.verdict not in {blocking, error}. Prior greps accepted any audit line with matching substrings inside arbitrary metadata (#3). - Fail-closed on every parse error. jq still guarded at hook entry. Updates codex-event.ts docstring to describe the jq predicate instead of the old substring match (which is now actively misleading). Signed-off-by: Jake Strawn <bandy.strawn@clarityhouse.press> * fix(gateway): restrict downstream child env and reset reconnect per episode Codex finding #4 (HIGH): every MCP child spawned from .rea/registry.yaml inherited the operator's full process environment — OPENAI_API_KEY, GITHUB_TOKEN, customer secrets, everything. Registry is an attacker-writable surface in shared / CI contexts. - Default to a hardcoded allowlist of neutral env vars (PATH, HOME, LANG, NODE_*, TMPDIR, etc.). - New optional RegistryServer.env_passthrough: string[] opts specific additional names into the forwarded set. Names matching /(TOKEN|KEY|SECRET|PASSWORD|CREDENTIAL)/i are refused at schema-parse time — the explicit server.env is the escape hatch for operator-typed secrets. - Merge order: allowlist then passthrough then explicit. Undefined host vars are skipped (no "undefined" string serialization). Codex finding #7 (MEDIUM): reconnectAttempted never reset after success, so one reconnect was one-per-object-lifetime, not one-per-failure-episode as documented. - Reset reconnectAttempted on successful reconnect+retry. - 30s flap-guard: refuse to reconnect a second time within that window, mark unhealthy so circuit breaker takes over. - JSDoc updated to match actual semantics. THREAT_MODEL.md update (env-inheritance policy documentation) owed — file is in blocked_paths and needs a direct maintainer edit. Signed-off-by: Jake Strawn <bandy.strawn@clarityhouse.press> * fix(install): reject symlink destinations, portable atomic settings, correct git config read Codex finding #5 (HIGH): rea init followed destination symlinks via copyFile + chmod, so a malicious symlink at e.g. .claude/hooks/foo.sh could redirect a subsequent --force install to an arbitrary path and chmod its target 0o755. - Resolve install root once with realpath; assert every destination resolves inside it. - lstat every destination before writing. Any symlink raises UnsafeInstallPathError naming the offending path and its link target. - Use COPYFILE_EXCL on fresh creates. Intentional overwrites unlink first to defeat symlink-swap TOCTOU between lstat and copyFile. Codex finding #6 (MEDIUM): two callers in the same process calling appendAuditRecord with different-looking paths to the same directory used different writeQueues keys, breaking the per-process hash-chain serialization the file header promised. - Normalize baseDir with path.resolve + best-effort realpath at function entry; cache resolved keys at module scope. Codex finding #8 (MEDIUM): fs.rename on Windows throws EEXIST when the destination exists. rea init could not update an existing .claude/settings.json on Windows. - Try rename; on EEXIST/EPERM, unlink dest and retry. No runtime dep added. Codex finding #9 (MEDIUM): the hooksPath regex matched any hooksPath = X line anywhere in .git/config regardless of section, so a [worktree] or [alias] block with the key redirected the installer. - Shell out to git config --get core.hooksPath via execFile. Fall back to .git/hooks when unset or errored. Adds symlink-refusal tests, concurrent-append serialization tests, Windows rename-retry simulation, and section-aware hooksPath tests. All quality gates green: type-check, 64/64 tests, lint, build. Signed-off-by: Jake Strawn <bandy.strawn@clarityhouse.press> * fix(hooks): use pre-push stdin contract and allowlist verdict predicate Codex round-2 finding R2-1 (HIGH): the round-1 refspec parser extracted only the dst side and then diffed "$MERGE_BASE"...HEAD. A user on branch foo pushing "git push origin hotfix:main" had the gate review foo's commits against main, not hotfix's — protected-path changes on hotfix evaded the gate entirely. - Read git's real pre-push stdin contract: lines of <local_ref> <local_sha> <remote_ref> <remote_sha> - Use local_sha as the source commit for the diff. - Use remote_sha as the merge base when the remote already has the ref; fall back to merge-base with target / main for new branches. - Argv parser kept as fallback for manual testing; it also now resolves src^{commit} for src:dst, not HEAD. - Multi-refspec pushes iterate all refspecs and pick the one with the largest diff so a mixed push cannot hide large commits behind a trivial refspec. - All-zero local_sha (branch delete) refspecs are tracked separately; a delete-only push fails closed with an explicit block message. - macOS bash 3.2 compatible (no namerefs). Codex round-2 finding R2-2 (HIGH): the jq predicate used a blocklist (`.metadata.verdict != "blocking" and != "error"`). Missing verdict yielded jq null, which compares != to any string and passes — a forged record with just head_sha set satisfied the gate. - Flip to allowlist: `.metadata.verdict == "pass" or == "concerns"`. - null / missing / unknown verdicts all correctly fail. shellcheck clean, syntax-checked, parse tests passing. Signed-off-by: Jake Strawn <bandy.strawn@clarityhouse.press> * fix(audit,install): remove cwd-aware baseDir cache; anchor install writes against ancestor changes Codex round-2 finding R2-3 (HIGH): the round-1 fix for finding #6 added a resolvedBaseDirCache keyed by the raw baseDir string. path.resolve('.') reads process.cwd() at call time, but cache hits skipped re-resolution. A long-lived process calling appendAuditRecord('.', ...) before and after process.chdir() would append to the first cwd's audit log even after the chdir — audit events routed to the wrong hash chain. - Remove resolvedBaseDirCache entirely. path.resolve + fs.realpath are cheap; audit append is not a hot path. writeQueues (the actual correctness fix from round 1) stays, keyed by the resolved path. - Regression test: chdir between two appendAuditRecord('.') calls and assert each record lands in the correct directory. Codex round-2 finding R2-4 (MEDIUM): the round-1 symlink refusal fix validated paths but copyFile/unlink dereferenced strings later. A concurrent attacker with write access inside the install root could swap an ancestor directory for a symlink between validation and write. COPYFILE_EXCL anchored only the leaf. - Snapshot the ancestor chain with realpath + lstat mtime after assertSafeDestination. - Re-verify immediately before every unlink and before the terminal write; any ancestor change raises UnsafeInstallPathError with kind: 'ancestor-changed'. - Replace copyFile with openSync(O_WRONLY | O_CREAT | O_EXCL | O_NOFOLLOW) + fs.write. O_EXCL races safely; O_NOFOLLOW refuses any symlink that sneaks in at the leaf. - Deterministic tests for both the ancestor-change detection and the O_NOFOLLOW leaf refusal. The end-to-end race (attacker swaps during live install) is skipped with a documented reason: single-process vitest cannot deterministically drive such a race without real multi-process timing coordination. Residual risk: sub-millisecond window between ancestor re-verify and the open syscall. Documented in the copy.ts header comment. All quality gates green: 68/68 tests pass (1 intentional skip), type-check clean, lint clean, build clean. Signed-off-by: Jake Strawn <bandy.strawn@clarityhouse.press> * fix(install): reject ancestor symlinks and escapes in snapshotAncestors Codex round-3 finding R3-1 (HIGH): the round-2 snapshotAncestors recorded ancestor realpaths but did not assert they resolved under the install root. An attacker swapping .claude/hooks for a symlink to /tmp/decoy before the snapshot had that state accepted as baseline; verifyAncestorsUnchanged then passed, and writeFileExclusiveNoFollow wrote the file to /tmp/decoy. O_NOFOLLOW protects only the leaf, not ancestor components — a confirmed install-root escape primitive. - snapshotAncestors now lstats each ancestor and refuses symlinks with UnsafeInstallPathError { kind: 'symlink' }. - After realpath resolution, every ancestor must equal resolvedRoot or begin with resolvedRoot + path.sep. Otherwise UnsafeInstallPathError { kind: 'escape' }. - Walk must terminate at resolvedRoot; hitting filesystem root without matching raises escape (belt-and-suspenders against upstream bugs). - Two deterministic regression tests: ancestor-symlink-inside-root and ancestor-escape-outside-root. Both verify the decoy target stays untouched. All quality gates green: 70/70 tests pass (1 intentional skip), type-check clean, lint clean, build clean. Signed-off-by: Jake Strawn <bandy.strawn@clarityhouse.press> * fix(hooks): fail-closed on unresolvable remote refs and parse --delete correctly Codex round-3 finding R3-2 (HIGH): the round-2 merge-base fallback `git merge-base X Y 2>/dev/null || echo $remote_sha` wrote the literal SHA string to $mb when Y was not in the local object DB (the common stale-checkout case). Downstream `rev-list` and `git diff` failed on the invalid ref, errors swallowed by `|| echo` defaults, DIFF_FULL collapsed to empty, and the gate exited 0 with no review — full bypass for any push whose remote tip the local repo hadn't seen. - Probe `git cat-file -e "${remote_sha}^{commit}"` before merge-base when remote_sha != ZERO_SHA. Missing object → exit 2 with fetch remediation message. - Capture merge-base, rev-list --count, and git diff exit statuses explicitly. Non-zero → exit 2. Empty output only reaches the legitimate no-op push path when git exited 0. Codex round-3 finding R3-3 (MEDIUM): resolve_argv_refspecs treated `-*` tokens as flags to skip unconditionally, so `git push --delete origin doomed` skipped --delete, consumed origin as remote, and processed doomed as a normal push. HAS_DELETE never fired; a destructive deletion was reviewed as an ordinary commit. - Explicit --delete / -d / --delete=VALUE cases set delete_mode=1. - Subsequent bare refspecs tagged with __REA_DELETE__ sentinel in the same refspec array (no second array; macOS bash 3.2 compat). - Emission loop strips the sentinel and emits ZERO|ZERO|(delete)|refs/heads/<dst> matching the git pre-push stdin contract. Existing HAS_DELETE block fail-closes on delete-only pushes — no downstream change. shellcheck clean. All edge cases verified (normal push, src:dst, upstream inference, legacy :doomed syntax, HEAD-target block). All quality gates green: 70/71 tests pass (1 intentional skip), type-check clean, lint clean, build clean. Signed-off-by: Jake Strawn <bandy.strawn@clarityhouse.press> * feat(hooks): land G11.1 audited escape hatch for push-review-gate Pulled G11.1 from the 0.3.0 resilience plan into 0.2.0 after round-4 Codex review hit an account rate limit. Without an escape hatch, a Codex outage blocks every push that touches a protected path — turning an availability failure of an external service into a hard-stop on local development. The escape hatch preserves the audit contract while allowing a push to proceed when the reviewer is unavailable. Contract: set REA_SKIP_CODEX_REVIEW to a non-empty reason string. Empty / unset = no bypass, gate enforces as before. The reason is written verbatim into the audit record so every skip leaves a durable, hash-chained explanation that `git blame`, auditors, and future reviewers can find. Implementation details - New block runs inside the protected-path branch, before the existing Codex-audit grep. When REA_SKIP_CODEX_REVIEW is non-empty: 1. Emit a loud stderr banner so the skip is impossible to miss in terminal output. 2. Compute files_changed count from DIFF_FULL for the audit record. 3. Resolve actor from `git config user.email || user.name`. Fail closed (exit 2) if neither is set — an unattributable skip is worse than a blocked push. 4. Require `dist/audit/append.js` to exist. Fail closed if missing — we never want to "skip the audit skip record." 5. Invoke the existing append helper via `node --input-type=module` with tool_name "codex.review.skipped" (note the distinct name). 6. Exit 0 only on successful audit append; any non-zero node exit fails closed. - tool_name is deliberately "codex.review.skipped", NOT "codex.review". The push-review-gate grep / jq predicate for codex.review records must never match a skip record, or the gate would become a no-op for any future push on the same HEAD. Two distinct event names, one shared hash chain. - jq is used to serialize the audit-record fields, so reasons containing quotes, newlines, or shell metacharacters cannot break out of the JSON context. Fail closed if jq is missing or serialize returns non-zero. - Metadata recorded: head_sha, reason (verbatim REA_SKIP_CODEX_REVIEW value), actor, verdict "skipped", files_changed (integer count of changed files in the protected paths diff). Tests (__tests__/hooks/push-review-gate-escape-hatch.test.ts, 8 cases) - Fail-closed when dist/audit/append.js is missing - Fail-closed when no git user.email or user.name is configured - Happy path: exit 0, banner present, audit record has every expected field with correct values - Reason=="1" literal when caller sets REA_SKIP_CODEX_REVIEW=1 — no implicit "default" reason; whatever the caller types is what gets logged, including short values - Skip record does NOT satisfy the existing codex.review jq predicate — regression guard against collapsing the two event names - Sanity: gate still blocks (exit 2) when env var is unset - Sanity: gate still blocks when env var is empty string - Sanity: dist/audit/append.js present in-repo shellcheck clean. 78 tests pass (13 files, 1 pre-existing skip). Lint, type-check, build all clean. First use is this push itself: Codex is rate-limited until 2026-04-25, the round-3 fixes (R3-1, R3-2, R3-3) were applied per spec but not adversarially verified, and the escape hatch is the mechanism that audits its own introduction. Follow-ups still owed (0.3.0): - G11.2 pluggable Claude-self reviewer (fallback review path) - G11.3 startup probe for Codex availability - G11.5 telemetry on skip frequency - THREAT_MODEL.md: document the escape hatch as an audited gate weakening (requires direct maintainer edit — blocked_paths) - .claude/hooks/push-review-gate.sh dogfood mirror resync Signed-off-by: Jake Strawn <bandy.strawn@clarityhouse.press> * docs(changeset): add G11.1 escape hatch to 0.2.0 notes Signed-off-by: Jake Strawn <bandy.strawn@clarityhouse.press> * feat(reviewers): land AdversarialReviewer interface + Codex adapter stub G11.2 step 1 of 3 — introduces the pluggable adversarial-reviewer contract the push gate will eventually dispatch through. - src/gateway/reviewers/types.ts: ReviewVerdict / ReviewFinding / ReviewResult / ReviewRequest / AdversarialReviewer shared shapes - src/gateway/reviewers/codex.ts: CodexReviewer adapter. isAvailable() probes `codex --version` with a 2s timeout; review() throws by design because the real path is the codex-adversarial agent, not a TS call - Unit tests cover exec success/ENOENT/timeout/non-zero, version caching, and the documented review() throw No behavior change — nothing wires these in yet. G11.2 steps 2 and 3 add ClaudeSelfReviewer and the selector; G11.3/G11.4 adopt them. Signed-off-by: Jake Strawn <bandy.strawn@clarityhouse.press> * feat(reviewers): add ClaudeSelfReviewer fallback G11.2 step 2 of 3 — the real runtime fallback when Codex is unreachable. Not a cross-model check, so every result is flagged degraded=true so the audit log is honest about what actually ran. - src/gateway/reviewers/claude-self.ts: one-shot Opus call in a fresh context with a review-only system prompt. Parses STRICT JSON matching ReviewResult; verdict=error on parse failure, APIError (429/5xx), or network error. Caps diff at 200KB and notes truncation in the summary - Pin reviewer_version to claude-opus-4-7 so audit entries stay reproducible across future model bumps - Always pins degraded=true even if the model tries to overwrite it - Adds @anthropic-ai/sdk@^0.90.0 (verified via npm view). Only new dependency this task touches - 16 new unit tests exercising isAvailable, success path, malformed findings drop, error paths (missing key, unparseable, bad verdict, APIError, generic Error), and truncation Not yet wired into the selector or push gate — that lands in step 3. Signed-off-by: Jake Strawn <bandy.strawn@clarityhouse.press> * feat(reviewers): add selector + policy/registry schema hooks G11.2 step 3 of 3 — ties the interface, CodexReviewer, and ClaudeSelfReviewer together behind a single selectReviewer() entry point with audit-friendly (degraded, reason) signals. No caller wired yet — push-review-gate integration is G11.3/G11.4 work per the spec. - src/gateway/reviewers/select.ts: precedence is env REA_REVIEWER > registry.reviewer > policy.review.codex_required=false > default (Codex first, fall back to claude-self degraded=true) > throw NoReviewerAvailableError pointing at the G11.1 escape hatch - Policy schema: new optional review.codex_required boolean; strict on unknown nested fields so typos fail loudly - Registry schema: new optional top-level reviewer enum ('codex' | 'claude-self'); unknown values rejected at parse time - 16 new selector tests cover the full precedence table, the unknown- env-var rejection, the NoReviewerAvailableError path, and the policy-first no-Codex case (first-class, NOT degraded). Policy and registry loader tests gain a block for the new fields — all backwards-compatible Total delta: 78/1 skipped -> 122/1 skipped. Lint + type-check + build all green. @anthropic-ai/sdk@^0.90.0 is the only new dep. Signed-off-by: Jake Strawn <bandy.strawn@clarityhouse.press> * feat(hooks): honor review.codex_required in push-review-gate (G11.4) When .rea/policy.yaml sets review.codex_required: false, the protected-path Codex adversarial-review gate is skipped entirely. The REA_SKIP_CODEX_REVIEW escape hatch also becomes a no-op (skipping a review that isn't required is not meaningful). Adds src/scripts/read-policy-field.ts — a tiny standalone script that exposes a single scalar policy field to shell hooks without dragging in the full CLI surface. Exit codes distinguish missing (1) from malformed (2) so callers can pick different fail modes. Fail-closed semantics: if the helper can't parse the policy, the gate treats codex_required as true (safer default) and logs a warning. 7 new integration tests exercise the no-codex path alongside the existing 8 escape-hatch tests, including malformed-policy and missing-policy regressions. Signed-off-by: Jake Strawn <bandy.strawn@clarityhouse.press> * feat(doctor): conditional Codex checks under review.codex_required (G11.4) When policy.review.codex_required is false, the two Codex-specific doctor checks (codex-adversarial agent, /codex-review command) are replaced by a single info line explaining why they were skipped. In the default and explicit-true cases, the original behavior is preserved. The curated-agents roster still expects codex-adversarial.md so flipping codex_required back to true does not require a re-install. Extracts `collectChecks(baseDir)` as a testable seam and adds a `info` status kind for purely advisory lines that never contribute to exit code. 4 new unit tests cover both modes plus the absent-field regression. Signed-off-by: Jake Strawn <bandy.strawn@clarityhouse.press> * feat(profiles): add bst-internal-no-codex and open-source-no-codex (G11.4) Two new profile variants that carry every setting from their parent (bst-internal / open-source) but are designed to default review.codex_required: false at init time. The profile YAMLs themselves don't emit the review block — that's written by the init flow based on the profile name — but the leading comment documents the coupling. Each file explains when the variant is appropriate and how to re-enable Codex later (edit .rea/policy.yaml, flip codex_required to true). Profile registry discovery is file-based (loadProfile checks for profiles/<name>.yaml), so simply adding these files makes them available; the allowlist in src/cli/init.ts is updated in the accompanying init change. Signed-off-by: Jake Strawn <bandy.strawn@clarityhouse.press> * feat(init): add --codex / --no-codex flags and wizard prompt (G11.4) rea init now writes an explicit review.codex_required field into every .rea/policy.yaml it creates. The value is resolved in this order: 1. Explicit --codex / --no-codex flag (commander's boolean-with-negation pair) wins unconditionally. 2. Otherwise derive from the chosen profile name — profiles ending in `-no-codex` default to false, everything else defaults to true. 3. Interactive mode prompts for a final confirmation, seeded with the flag/profile default. Adds `bst-internal-no-codex` and `open-source-no-codex` to the profile allowlist (the YAMLs were added in the previous commit). When the resolved value is false, the CLI prints a durable notice after install pointing at the exact knob (`review.codex_required: true`) the operator would flip to re-enable Codex later. A TODO comment in the same block flags the coupling with a future G6-style Codex install assist. 7 new non-interactive init tests cover the flag combinations and confirm the written policy parses via the strict loader (catching any key typo in the emitted YAML). Signed-off-by: Jake Strawn <bandy.strawn@clarityhouse.press> * docs: document G11.4 + G11.2 in 0.2.0 changeset and CLAUDE.md Changeset picks up the actual surface that landed for G11.4 (push gate, doctor, init flow, two new profile variants, 18 new tests) and records G11.2 which was missing from the original draft. CLAUDE.md profile listing now enumerates the -no-codex variants and explains what they actually change at init time. Non-Negotiable Rules section is untouched. Signed-off-by: Jake Strawn <bandy.strawn@clarityhouse.press> * fix(kill-switch): enforce single-shot HALT read with fail-closed errno handling (G4) Close the TOCTOU gap between the middleware's HALT check and the downstream terminal. The previous implementation called `stat` → `lstat` → `open` as a three-syscall sequence, creating a window in which HALT state could change between the decision and the read. The rewrite issues exactly ONE syscall per invocation on the HALT file: `fs.open(path, O_RDONLY)`. The decision is derived entirely from the open outcome: * ENOENT → HALT absent → proceed with the chain. * open succeeds → HALT present → deny. A best-effort read populates the reason string (capped at 1024 bytes); the read does NOT influence the decision. * any other errno → unknown state → deny (fail-closed). Semantic guarantee codified in the module-level doc block: HALT is evaluated exactly once per invocation, at chain entry. A call that passes that check runs to completion; a call that fails it is denied. Creating .rea/HALT mid-flight does NOT cancel in-flight invocations — it blocks subsequent invocations only. This matches standard kill-switch semantics (SIGTERM after acceptance: the process continues). The decision is recorded on `ctx.metadata.halt_decision` (absent | present | unknown) and `ctx.metadata.halt_at_invocation` (ISO-8601 timestamp when present, null otherwise). The audit middleware already forwards arbitrary ctx.metadata keys into the hash-chained record, so every audit row now carries the HALT decision that governed it. THREAT_MODEL.md §5.7 needs a corresponding update to replace the "theoretical TOCTOU on shared filesystems" residual risk with the explicit semantic guarantee. THREAT_MODEL.md is in blocked_paths, so the proposed paragraph is drafted to /tmp/halt-semantic-update.md for the maintainer to apply. Signed-off-by: Jake Strawn <bandy.strawn@clarityhouse.press> * test(kill-switch): cover TOCTOU, concurrency, and errno fail-closed paths (G4) Six new tests exercise the single-shot HALT semantic from every angle: 1. HALT created between chain start and terminal — the test's "next()" writes .rea/HALT mid-flight, yields a tick, and asserts the invocation still completes. Proves the middleware never re-checks. 2. HALT removed mid-invocation — HALT present at entry → denied. Removing HALT after the middleware returns does NOT rescue the call; the terminal never runs. 3. Per-invocation decisions, never cached — invocation 1 sees HALT (denied), HALT is removed, invocation 2 sees it absent (allowed). Two separate decisions. 4. ENOENT regression — HALT absent → next() runs, status stays Allowed. 5. Non-ENOENT errno → fail-closed — HALT exists with mode 0o000. On a non-root user the open fails with EACCES → decision 'unknown' → denial. On root, open succeeds → decision 'present' → still denied. Terminal never runs in either case. 6. Concurrency matrix — 10 invocations across a HALT toggle. First batch of 5 runs with HALT absent (all allowed); HALT is then written; second batch of 5 (all denied). Each invocation's decision reflects the state at ITS own chain entry, not a shared snapshot. The existing "HALT is a directory" test updated to assert platform-invariant denial (Linux: open on a dir succeeds → 'present'; macOS: open returns EISDIR → 'unknown'; both deny). Existing "caps HALT read size" test updated to also assert halt_decision. Test count delta: +6 (140 → 146 pass, 1 skip unchanged). Signed-off-by: Jake Strawn <bandy.strawn@clarityhouse.press> * feat(redact-safe): add wrapRegex with worker-based timeout bound (G3) Adds `src/gateway/redact-safe/match-timeout.ts` — a synchronous `SafeRegex` wrapper that bounds every regex `test`/`replace` to a configurable wall-clock budget (default 100ms). Implementation: Option A — worker thread per exec. - No native dependency (vs `re2`, which would add a build step and a second regex dialect). - Hard timeout: on expiry the parent calls `worker.terminate()`, which reliably kills a catastrophic backtracker. - Overhead ~1ms per call. Acceptable for gateway payloads today; worker pooling is a future-proofing option for 0.3.0 that would not change the public `SafeRegex` surface. Synchronization: the parent blocks on `Atomics.wait` over a SharedArrayBuffer while the worker computes. The worker writes its reply to a MessageChannel port (transferred via `transferList`), then stores `1` into the SAB and calls `Atomics.notify`. The parent wakes, drains the reply port via `receiveMessageOnPort`, and terminates the worker. This keeps `.test()` / `.replace()` synchronous so they remain a drop-in replacement for `RegExp.prototype.test` / `.replace` inside the existing middleware tight loops. On timeout: `.test()` returns `{matched: false, timedOut: true}` and `.replace()` returns `{output: input (unchanged), timedOut: true}`. An optional `onTimeout` callback fires exactly once and its errors are swallowed so a bad logger cannot break middleware. The caller (redact / injection middleware — added in the next commit) is responsible for emitting the audit event with size+pattern-id only, never the input text. Tests cover: benign match/replace, catastrophic `(a+)+$` pattern against `"a".repeat(25) + "X"` timing out within 2× the budget, replace returning input unchanged on timeout, `onTimeout` fire-exactly-once, callback error swallowing, default 100ms timeout, and `.pattern` passthrough. Signed-off-by: Jake Strawn <bandy.strawn@clarityhouse.press> * feat(redact,injection): route default and user patterns through wrapRegex (G3) Every regex the redact and injection middleware layers run against untrusted MCP payloads now goes through the `SafeRegex` wrapper from G3. Per-call timeouts mean a catastrophic backtracker can no longer hang the gateway — the worker is terminated, the offending value is replaced with a sentinel, and an audit event is emitted. Changes: - `src/gateway/middleware/redact.ts`: - `SECRET_PATTERNS` is now exported (required by the CI lint:regex check added in the next commit). - New `createRedactMiddleware({ matchTimeoutMs?, userPatterns? })` factory. Defaults preserve current behavior (100ms budget). Users from policy-loaded patterns are a first-class input. - `redactSecrets` takes compiled patterns + optional `onTimeout` callback. On timeout the entire field is replaced with the sentinel `[REDACTED: pattern timeout]` — the scanner never lets an un-scanned value escape. Scanning short-circuits on the offending pattern. - Timeout audit events are pushed into `ctx.metadata` under the key `redact.regex_timeout` as an array of `{event, pattern_source, pattern_id, input_bytes, timeout_ms}`. The input text is NEVER written — only its UTF-8 byte length. - The exported `redactMiddleware` constant is preserved for back-compat; `createRedactMiddleware()` is the new canonical form. - `src/gateway/middleware/injection.ts`: - `INJECTION_PHRASES` now exported (for lint:regex). Added exported `INJECTION_BASE64_PATTERN` + `INJECTION_BASE64_SHAPE` constants — the two regexes this middleware runs. Both pass through `SafeRegex` now. - `scanForInjection` takes compiled SafeRegex bundle; patterns are built once per invocation via `compileInjectionPatterns`. - Timeout events land on `ctx.metadata` under `injection.regex_timeout`. Same size-only contract as redact. - Literal phrase matches continue to use `String.prototype.includes` (no regex, no ReDoS surface). - `src/gateway/redact-safe/match-timeout.ts`: - Added `matchAll` op to `SafeRegex` — bounded match enumeration, needed so the injection middleware can extract base64 tokens without falling back to unbounded `String.prototype.match`. The worker forces the global flag so matchAll is meaningful regardless of how the pattern was specified. - Removed the unused async runner + `wrapRegexAsync` export; the sync surface is sufficient and matches how middleware actually calls. Tests: - `src/gateway/middleware/redact.test.ts` (new, 7 tests): redaction + sentinel + audit-metadata shape + no-input-leakage + invocation-continues-after-timeout + nested-object preservation. - `scanForInjection` keeps its existing literal-phrase behavior; the base64 branch now uses `SafeRegex.matchAll`. Performance note: the middleware chain still walks every string in the result and runs N patterns × 1 worker-spawn per string. This is the defense-in-depth cost the threat model already accepts. Worker pooling is a 0.3.0 optimization that would not change the public surface. Signed-off-by: Jake Strawn <bandy.strawn@clarityhouse.press> * feat(ci,policy): add lint:regex, load-time safe-regex check on user patterns (G3) Completes the G3 defense-in-depth story. Two new enforcement points land around the runtime timeout from the first two commits: 1. Build-time static lint. `scripts/lint-safe-regex.mjs` imports the compiled `SECRET_PATTERNS` and the two injection-scan constants from `dist/`, passes each through `safe-regex`, and exits non-zero on any flagged offender. Wired into `pnpm lint` as `lint:regex` and chained in BEFORE eslint so a bad regex short-circuits the pipeline fast. Running it caught one offender already — the existing "Private Key" pattern with nested `\s+` inside optional alternation. Tightened to a single-space form that matches the canonical PEM armor header (`-----BEGIN [TYPE ]PRIVATE KEY-----`). Non-standard whitespace in PEMs is not in our threat model. 2. Load-time safe-regex check on user-supplied patterns. `src/policy/types.ts` gains a `RedactPolicy` interface with `match_timeout_ms?: number` and `patterns?: UserRedactPattern[]`. `src/policy/loader.ts` validates each pattern via `safe-regex` at load time — a flagged pattern rejects the entire policy load with an error that names the offender. The zod schema stays strict so typos fail loudly. Malformed-regex-source also fails load. Gateway wiring: - `src/gateway/server.ts` compiles user patterns via `wrapRegex` at gateway-create time and passes the configured `matchTimeoutMs` to both `createRedactMiddleware` and `createInjectionMiddleware`. User patterns are appended after defaults, preserving precedence. Tests (7 new in `src/policy/loader.test.ts`): - accepts `redact.match_timeout_ms` + `redact.patterns` round-trip - back-compat: `redact` undefined when not set - rejects `(a+)+$` (safe-regex flagged) - rejects malformed regex source (`(`) - rejects unknown fields at the `redact.` level (strict) - rejects unknown fields inside a `patterns` entry (strict) - accepts a bounded user pattern end-to-end Dev dependencies: `safe-regex@^2.1.1` + `@types/safe-regex@^1.1.6` (verified existence + license via `npm view`). Changeset `.changeset/0.2.0-mvp.md` gains a `## ReDoS safety (G3)` section and G3 is removed from the deferred list. Signed-off-by: Jake Strawn <bandy.strawn@clarityhouse.press> * feat(observability): land CodexProbe — availability polling and state (G11.3) CodexProbe polls `codex --version` and a best-effort catalog subcommand to expose whether the Codex CLI is reachable right now. The probe is intentionally decoupled from the reviewer selector — it reports state only, it never gates a review. Consumers (`rea serve` startup, `rea doctor`) read the state and decide what to do. Key behaviors: - Never throws from getState(); startup never fail-closes on a probe miss. - setInterval is .unref()'d so polling does not pin the event loop. - onStateChange listeners fire on transitions, not on every tick. - Concurrent probe() callers share a single in-flight exec. - Degraded-skip path for Codex builds that don't recognize `catalog --json`, documented inline so a false unauthenticated flag can't creep in. 18 unit tests cover exit codes, timeouts, ENOENT, version parsing, lifecycle, listener semantics, and the concurrency guarantee. Signed-off-by: Jake Strawn <bandy.strawn@clarityhouse.press> * feat(serve,doctor): wire CodexProbe lifecycle and doctor check (G11.3) On `rea serve` startup, run an initial probe when policy.review.codex_required is not explicitly false. A failed probe emits a stderr warn only; serving continues. The probe runs periodically via start() and is stopped on SIGTERM/SIGINT. `rea doctor` now runs a one-shot probe (when Codex is required) and adds two rows: `codex.cli_responsive` (pass/warn) and `codex.last_probe_at` (info). Probe failure does NOT fail the doctor — it surfaces as a warn consistent with the existing Codex-optional checks. Kept collectChecks() accepting an optional probe state so existing unit tests (which don't run a probe) still pass. 4 new doctor tests cover the pass/warn branches, no-codex isolation, and the pure checksFromProbeState helper. Signed-off-by: Jake Strawn <bandy.strawn@clarityhouse.press> * feat(observability): land codex-telemetry — append-only metrics.jsonl (G11.5) Observational telemetry for adversarial-review invocations. Each record captures invocation_type, estimated token counts (chars/4), duration, exit code, and whether stderr looked rate-limited. Appended to `<baseDir>/.rea/metrics.jsonl` as JSONL, fsync'd after each write. Explicit non-goals documented in the module header: - NOT the audit log — audit is hash-chained and authoritative; telemetry is free-form operator numbers. - NEVER stores input_text / output_text. The strings are consumed once for token estimation and then discarded. A test asserts absence of marker strings in the written file to enforce the contract. Fail-soft writes — any I/O error surfaces as a single stderr warning and resolves without throwing. Telemetry must never interfere with a review. `summarizeTelemetry` buckets records by local-tz day, most-recent first, and handles missing file / malformed lines / out-of-window records cleanly. 15 unit tests cover the shape, payload-absence invariant, rate-limit regex with 4 real-world stderr examples, day bucketing, and fail-soft behavior. Signed-off-by: Jake Strawn <bandy.strawn@clarityhouse.press> * feat(reviewers,doctor): instrument ClaudeSelfReviewer and add --metrics flag (G11.5) ClaudeSelfReviewer.review() now writes a single telemetry row per SDK call via an internal emitTelemetry helper that contains both sync throws and async rejections from a misbehaving injected telemetry fn. Three paths are instrumented — success, API error, unparseable output — with exit_code = 0 on success and 1 on any error. The 'no API key' short-circuit is deliberately NOT instrumented; there is no SDK call to measure. CodexReviewer.review() is left uninstrumented. It throws today (real path goes through the codex-adversarial agent); a TODO comment references the 0.3.0 work where Codex runs from TS and the same instrumentation will apply. rea doctor --metrics prints a compact 7-day telemetry summary after the existing checks. The flag never contributes to the exit code — purely observational. Test hygiene: ClaudeSelfReviewer test suite now redirects process.cwd() to a tmpdir in beforeAll so the default telemetry path (when baseDir/recordTelemetryFn are not injected) doesn't scribble into the repo's own .rea/metrics.jsonl. .rea/metrics.jsonl added to .gitignore as belt-and-suspenders for consumers. Changeset updated with G11.3 + G11.5 sections. Signed-off-by: Jake Strawn <bandy.strawn@clarityhouse.press> * chore(eslint): ignore .claude/worktrees/ to prevent sibling-agent bleed Parallel agent worktrees live under .claude/worktrees/. Without this exclusion, eslint walks into their src/ and flags all of the transient in-flight work — including any other agent's in-progress branch — as errors in this checkout. No behavior change in normal development. Signed-off-by: Jake Strawn <bandy.strawn@clarityhouse.press> * feat(install,cli,hooks): G12 upgrade + install hardening for 0.2.0-mvp G12 (install manifest + rea upgrade) lands together with a broad hardening pass driven by a local Opus code-reviewer adversarial read of the full surface. Install manifest + upgrade - install-manifest.json with per-file sha256 + source classification - rea upgrade --dry-run | -y | --force with bootstrap mode - synthetic entries for CLAUDE.md managed fragment and settings.json - drift classification: new/unmodified/drifted/removed-upstream Hardening (addresses B1-B7 from local review) - fs-safe.ts: resolveContained + atomicReplaceFile with O_WRONLY|O_CREAT|O_EXCL|O_NOFOLLOW, three-file Windows replace - zod path-traversal refinements run at parse time (control chars, absolute POSIX/Windows, drive letters, UNC, ..) - TOCTOU defenses: snapshotAncestors + verifyAncestorsUnchanged - upgrade.ts: all fs mutations routed through safe helpers, SHA recomputed from installed bytes, diff size cap 256KB - .husky/pre-push: here-doc loop fixes subshell scope bug, anchored protected-path regex, POSIX-portable awk for HALT reason, Codex audit grep matches tool_name AND head_sha - postinstall.mjs: fileURLToPath for Windows portability, package-manager-agnostic upgrade recommendation - shared START_MARKER/END_MARKER/extractFragment between install and upgrade to prevent marker drift Tests - fs-safe.test.ts (16): resolveContained, atomicReplaceFile, safeDeleteFile, safeReadFile including symlink refusal - manifest-schema.test.ts (19): strict parse, path rejection for absolute/UNC/traversal/control-chars, synthetic entries - Net +35 tests; 247 total passing Signed-off-by: Jake Strawn <bandy.strawn@clarityhouse.press> * chore(policy): set review.codex_required=false for this repo Codex is rate-limited in our environment. Local Opus code-reviewer substitutes for the adversarial-review leg of Plan → Build → Review per CLAUDE.md. The push-gate already honors this via G11.4, so with the flag set no env-var bypass is required on each push. Signed-off-by: Jake Strawn <bandy.strawn@clarityhouse.press> * fix(ci,hooks,tests): unblock Lint/Test CI on 0.2.0-mvp PR - ci.yml Lint job: build before lint so lint:regex can inspect dist/ - ci.yml Test job: build before test so no-codex hook integration tests resolve dist/scripts and dist/audit symlinks into the scratch repo - push-review-gate.sh: use [.] instead of \. in the protected-path ERE so GNU awk does not dirty stderr with escape-sequence warnings that made the no-codex tests brittle. Sync .claude/hooks/ copy. - redact.test.ts: bump worker-regex timeout from 30ms to 250ms. Under GitHub Actions worker-thread startup load, 30ms was below the noise floor for default patterns and they spuriously timed out on benign input. 250ms keeps per-test duration sub-second while clearing CI. Signed-off-by: Jake Strawn <bandy.strawn@clarityhouse.press> * fix(husky): honor review.codex_required in terminal pre-push hook (G11.4 parity) `.claude/hooks/push-review-gate.sh` (Claude-Code PreToolUse path) already short-circuits the protected-path Codex audit requirement when the policy sets `review.codex_required: false`. The terminal pre-push hook (`.husky/pre-push`) was missed during G11.4 and still demanded the audit entry for every protected-path diff, breaking the first-class no-Codex mode for anyone pushing from the terminal. Mirror the Claude-Code hook's policy read: invoke `dist/scripts/read-policy-field.js review.codex_required` once; if the field resolves to `false`, skip the audit requirement on this push. Every other path (HALT, protected-path regex, audit-log grep, REA_SKIP env-var escape hatch, fail-closed missing helper) is unchanged. Fail-closed: if the helper is missing (unbuilt rea) or errors, treat the field as true — safer default. Operator can `pnpm build` or set the escape-hatch env var for a one-off bypass. Signed-off-by: Jake Strawn <bandy.strawn@clarityhouse.press> * fix(tests): escape-hatch makeScratchRepo sets baseline identity unconditionally CI runners have no global git user.email / user.name, so the previous conditional-identity logic caused `git commit` to abort with "Author identity unknown" before the hook under test could ever run. makeScratchRepo now: 1. Sets a baseline identity before the initial commits so the commits always succeed. 2. Applies the caller's requested identity state AFTER the commits: - null → unset the config (fail-closed test path) - string → override with that value - undefined → leave the baseline in place This preserves the original test intent — each test still exercises the hook with its intended identity state — while making the suite robust against CI environments that lack a global git identity. Signed-off-by: Jake Strawn <bandy.strawn@clarityhouse.press> --------- Signed-off-by: Jake Strawn <bandy.strawn@clarityhouse.press> Co-authored-by: Jake Strawn <bandy.strawn@clarityhouse.press>
dependabot
Bot
changed the title
Bumps [actions/setup-node](https://github.com/actions/setup-node) from 4.0.3 to 6.3.0. - [Release notes](https://github.com/actions/setup-node/releases) - [Commits](actions/setup-node@1e60f62...53b8394) --- updated-dependencies: - dependency-name: actions/setup-node dependency-version: 6.3.0 dependency-type: direct:production update-type: version-update:semver-major ... Signed-off-by: dependabot[bot] <support@github.com>
dependabot
Bot
force-pushed
the
dependabot/github_actions/actions/setup-node-6.3.0
branch
from
eefb050 to
c7a33e0
Compare
This was referenced Apr 19, 2026
Merged
Author
|
Superseded by #39. |
dependabot
Bot
deleted the
dependabot/github_actions/actions/setup-node-6.3.0
branch
himerus
pushed a commit
that referenced
this pull request
Apr 20, 2026
Codex pass-4 surfaced two HIGH parity gaps between hooks/_lib/push-review-core.sh and .husky/pre-push, plus one missing regression test. Pass-3 closed the no-default-ref bootstrap fail-open; pass-4 closes the remaining two paths and rounds out test coverage. Finding #1 (HIGH): hooks/_lib/push-review-core.sh:805-807 silently continue'd when default_ref resolved (origin/HEAD, origin/main, or origin/master present) but git merge-base returned empty — the unrelated-histories / grafted-branch / transient-failure scenario. Combined with the longest-diff BEST_COUNT accumulator, a protected- path refspec with an empty merge-base could slip through whenever another refspec in the same push was selected as BEST. Fall through to the empty-tree baseline instead, matching the spirit of the pass-3 fix for the no-default-ref case — the protected-path check still runs against the full refspec content, so the gate remains effective. Finding #2 (HIGH): the protected-path Codex-audit check ran once on the BEST_COUNT-selected refspec, while .husky/pre-push's per-refspec loop checks every refspec. A push like big:big hotfix:main where big is 50 non-protected commits and hotfix is one commit touching hooks/foo.sh would be blocked by husky and silently passed by the shared core. Move the protected-path check INSIDE the main per-refspec loop and require the Codex audit entry to match each protected refspec's own local_sha. The post-loop section 7a reduces to a short comment noting the check moved up. Finding #3 (MEDIUM): the adapter suite exercised three new-branch zero-SHA permutations (origin/HEAD set, origin/main fallback, origin/master fallback) but not the fully-empty case — that was only covered by husky-e2e. Added an adapter-level test for all-three-refs- missing with a REA_SKIP_CODEX_REVIEW sanity bypass to prove the block fired on the protected-path gate rather than on a ref-resolution error. Findings #4/#5 (LOW): the action-required hint at the end of the shared core was still three-dot, which would fail verbatim in the bootstrap case where MERGE_BASE is the empty-tree SHA. Converted to two-dot to match the rest of the file. The stale docstring was refreshed to reflect pass-3's two-dot conversion and line numbers. Dogfood mirror .claude/hooks/_lib/push-review-core.sh synced byte-for-byte via cp. Tests: adapter suite 16 -> 17 (new bootstrap regression green), full hooks matrix 76 -> 77, full suite 1059/1060 (1 pre-existing skip). Signed-off-by: Jake Strawn <bandy.strawn@clarityhouse.press>
himerus
pushed a commit
that referenced
this pull request
Apr 20, 2026
Pass-4 moved the protected-path Codex-audit check inside the per-
refspec loop so that a push like `git push origin clean:clean
hotfix:main` cannot hide a small protected-path refspec behind a
larger non-protected one. Pass-5 review verified the code is
correct but flagged that every prepushLine in the adapter suite
is a single refspec — no direct regression test for the hide-
behind scenario. A future refactor that moves the check back
outside the loop or short-circuits after the BEST_COUNT winner
would silently reintroduce the HIGH parity gap.
Add a multi-refspec adapter test with the clean refspec first
(non-protected, touches README.md) and the protected refspec
second (touches hooks/). Asserts:
- gate blocks (exit 2)
- stderr matches /protected paths changed/
- stderr contains the PROTECTED refspec's sha (not the clean
one's), proving the per-refspec loop caught the right refspec
Uses the existing makeRepo fixture's cleanFeatureSha and
featureSha — no fixture changes required.
Adapter suite: 17 -> 18. Full hooks matrix stays green.
Signed-off-by: Jake Strawn <bandy.strawn@clarityhouse.press>
himerus
added a commit
that referenced
this pull request
Apr 20, 2026
…ipeline hardening (#44) * ci(release): BUG-013 defense-in-depth — rebuild + post-publish verify 0.6.1 shipped with dist/ byte-identical to 0.6.0. The pipeline did not enforce a fresh build from the shipping commit, and did not verify that the published tarball's dist/ matched what CI built. Two new steps: 1. Rebuild dist/ from HEAD before publish — records a SHA-256 tree hash of the rebuilt dist/ to .rea-dist-hash so the verify step can compare against what was built at release time. 2. Verify published tarball dist/ matches CI-built dist/ — downloads the just-published tarball from npm, hashes its dist/ tree, and fails the release if the hash differs. Also document the 0.6.2 BUG-012 trust boundary in THREAT_MODEL §5.2a: CLAUDE_PROJECT_DIR is advisory-only; the script-anchor idiom owns the trust decision for cross-repo guard evaluation. Signed-off-by: Jake Strawn <bandy.strawn@clarityhouse.press> * fix(hooks): REA_SKIP_CODEX_REVIEW now works on stale checkouts The whole-gate escape hatch (REA_SKIP_PUSH_REVIEW) worked correctly from 0.5.0 onward, but the narrower Codex-only bypass (REA_SKIP_CODEX_REVIEW) was positioned after ref-resolution. On a stale checkout where the remote ref had been force-pushed or pruned, `git rev-parse <sha>` would fail with exit 128 and the gate would hard-crash before the skip check ever ran, leaving operators unable to unblock a push that they knew was safe. Reorders section 5b/5c of push-review-gate.sh so CODEX_REQUIRED policy resolution and the REA_SKIP_CODEX_REVIEW consume both run *before* ref-resolution. Metadata recorded to the skip audit entry is now best-effort: HEAD SHA, target from upstream, files_changed from HEAD~1..HEAD. G11.4 semantics preserved — when `review.codex_required: false`, the env var remains a no-op (the gate already short-circuits upstream of the skip branch). Adds a dedicated regression test that feeds the hook a pre-push stdin with a bogus remote SHA and asserts REA_SKIP_CODEX_REVIEW still fires and exit is 0. Signed-off-by: Jake Strawn <bandy.strawn@clarityhouse.press> * fix(gateway): BUG-014 — bound lastErrorMessage at write, not at read 0.6.2 shipped `boundedDiagnosticString` at the getter (`get lastError`), which meant every assignment site was trusted to eventually flow through the read path. A future call site that forgot to use the getter, or any bug that leaked the raw field, would serialize the full unbounded string. Converts `lastErrorMessage` from a TS-private field to a true ES-private field (`#lastErrorBacking`) with a bounded accessor pair: #lastErrorBacking: string | null = null; get #lastErrorMessage(): string | null set #lastErrorMessage(msg: string | null) // applies the bound The invariant is now structural: every `this.#lastErrorMessage = x` write produces a bounded stored value, regardless of how many assignment sites exist or where they live. The getter keeps its bound-at-read as cheap defense-in-depth. Also adds a test-only seam `_testOnly_seedLastError` that routes through the bounded setter — the single existing `(conn as any) .lastErrorMessage = ...` test access site would silently create a public property on the new ES-private class, masking the clear-on- success behavior under test. Two new regression tests verify (a) the bound applies at assignment, and (b) the backing field is unreachable from `this` or `as any` casts. Ships together with the hook ordering fix and BUG-013 release-pipeline hardening as 0.7.0 minor — see .changeset/070-helix-blockers.md. Signed-off-by: Jake Strawn <bandy.strawn@clarityhouse.press> * fix: address Codex adversarial review findings (4 concerns) Codex review on 2026-04-20 returned verdict "concerns" with 3 substantive findings plus 1 semver nit. All fixed: Finding #1 — push-review-gate skip audit metadata Skip block runs before ref-resolution (0.7.0 ordering change) but was guessing head_sha from `git rev-parse HEAD` and target from `@{upstream}`. A skip on `git push origin hotfix:main` from a `feature` checkout recorded the feature SHA, not the hotfix SHA. Now: parse the first valid refspec line from pre-push stdin (`<local_ref> <local_sha> <remote_ref> <remote_sha>`) and record the authoritative `local_sha` as head_sha. Fall back to HEAD only when stdin isn't a pre-push contract. files_changed is now null in skip records (no authoritative push window pre-ref-resolution — don't lie with a local proxy). New metadata_source field tags the record as "prepush-stdin" or "local-fallback". Regression test exercises push-from-other-ref. Finding #2 — .rea-dist-hash leaked into release PR `changesets/action` runs `git add .` when creating/updating the Version Packages PR. `.rea-dist-hash` was written to the repo root, so the next release PR would commit it. Fix: write to $RUNNER_TEMP/rea-dist-hash (CI scratch); update verify step to read from the same path. Finding #3 — _testOnly_seedLastError was public API The seam added in the prior commit ended up in the shipped .d.ts, callable by any consumer. Removed the seam entirely. Rewrote both regression tests to use natural failure paths (reconnect-success for clear-on-success; flap-window fail for bound-at-write). Also hardened the setter to throw TypeError on non-string input. Finding #4 — semver nit: minor → patch Defense-in-depth + UX fix + CI hardening doesn't warrant minor. No new public API (seam removed). Renamed changeset file and reworded to reflect the patch scope. Signed-off-by: Jake Strawn <bandy.strawn@clarityhouse.press> * chore(gateway): address Codex N2/N3 nits from 0.7.0 review - docstring: correct rationale for read-side bound (ES-private fields are lexically scoped, so "buggy subclass" framing is inaccurate — the correct belt-and-suspenders rationale is intra-class writes that bypass the setter) - test name: "not reachable from \`this\` or \`as any\`" overstated; only the \`as any\` property-access path is actually verified by the test Signed-off-by: Jake Strawn <bandy.strawn@clarityhouse.press> * refactor(hooks): extract push-review-core shared library (BUG-008 cleanup) Splits the 1053-line `hooks/push-review-gate.sh` monolith into a thin adapter (~77 lines) plus a reusable core library at `hooks/_lib/push-review-core.sh` (~934 lines) exposing three functions: - pr_parse_prepush_stdin — detects `<ref> <sha> <ref> <sha>` stdin - pr_resolve_argv_refspecs — argv-based refspec parsing - pr_core_run — full gate pipeline (guard → HALT → CMD parse → push_review:false → skip-env → codex_required → protected paths → cache lookup → block) The adapter sources the core via `BASH_SOURCE`-anchored lookup (mirrors the BUG-012 script-anchor pattern), keeping the trust boundary intact. Behavior is preserved byte-for-byte — all 50 hook integration tests stay green. The `rea init` install pipeline (`src/cli/install/copy.ts` `walkAndCopy`) already recurses into `_lib/` and `package.json` lists `hooks/` in its `files` array, so consumers receive the library automatically on upgrade. Unblocks task #50 (native git pre-push adapter `hooks/push-review-gate-git.sh`), which can now consume the same core as a ~20-line wrapper instead of duplicating the monolith. Mirrors both files into `.claude/hooks/` for this repo's dogfood install. Test installer helpers updated in five test files so fixtures accurately mimic the real `rea init` topology. Signed-off-by: Jake Strawn <bandy.strawn@clarityhouse.press> * feat(hooks): ship native git pre-push adapter push-review-gate-git.sh (BUG-008 cleanup) Completes the BUG-008 cleanup started in 81230d7 (task #49) by shipping a dedicated native-husky adapter alongside the Claude Code PreToolUse one. Both adapters are thin shims over `pr_core_run` in `hooks/_lib/push-review-core.sh`; the -git variant exists so `.husky/pre-push` expresses its install intent clearly and future git- only behavior (e.g. remote-URL-scoped policy) has a natural home. Task #50. Pairs with #49. Changes - hooks/push-review-gate-git.sh — new 92-line adapter: captures stdin, anchors via BASH_SOURCE, sources the core, calls pr_core_run. Body is byte-for-byte the same shape as push-review-gate.sh; only the docstring differs (install banner + escape-hatch ordering caveat + link to task #85). - .claude/hooks/push-review-gate-git.sh — dogfood mirror, byte-identical. - hooks/_lib/push-review-core.sh — tracked-mode fixed to 100755 via update-index --chmod=+x (source was 100644, mirror 100755). Closes the pass-2 drift finding. - __tests__/hooks/push-review-gate-git-adapter.test.ts — 13-test suite: * Protected-path block via husky stdin+argv (exercises BUG-008 sniff) * REA_SKIP_PUSH_REVIEW bypass + audit-receipt check * push_review:false policy short-circuit * Missing _lib/ → exit 2 with diagnostic (fails closed, never no-op) * Parity matrix: every branch pr_core_run exposes (HALT, REA_SKIP_PUSH_REVIEW, REA_SKIP_CODEX_REVIEW, empty stdin, non-protected path, protected path) runs through BOTH adapters and asserts identical exit code + identical load-bearing stderr — load-bearing anti-drift guard * Byte+mode parity for all three source↔mirror pairs (adapter x2, core x1). Mode assertion is absolute '100755', not just equality, so symmetric chmod -x would still fail * Hard jq precondition at module load (throws) instead of per-test silent skip — the failure mode that hid BUG-008 through a full minor cycle Codex adversarial review: 4 passes, final verdict pass (0 findings). Earlier passes produced: - P1 F1/F2/F3: drift risk, coverage gap, docstring inaccuracy - P2 F1'/F2'/F3': mode-bit drift, missing byte-parity, test classifier - P3 F1/F2/F3/F4 (concerns, non-blocking): mode absolute, jq hard precondition, protected-vs-general branch separation, exit-code docstring - P4: pass, all pass-3 fixes verified load-bearing Quality gates: lint + type-check + 1045 tests + build all green. Follow-on: task #85 tracks narrowing REA_SKIP_CODEX_REVIEW from a whole-gate bypass to a true Codex-only waiver. Out of 0.7.0 scope; the matrix test documents the current semantics so the eventual change surfaces as a test delta rather than silent behavior drift. Signed-off-by: Jake Strawn <bandy.strawn@clarityhouse.press> * ci: class-level dist/ regression gate (generalizes BUG-013 beyond [security] marker) Adds a new `dist-regression` CI job that catches the 0.6.0 → 0.6.1 regression class on EVERY PR and EVERY push:main, not just on releases that happen to carry a `[security]` changeset marker. The narrow gate in scripts/tarball-smoke.sh only fires when a changeset is labeled `[security]`. That is bypass-resistant against an attacker who forgets the label but can't prevent a committer from omitting the label entirely. This gate closes that gap by keying on src/ diff vs the last published tag rather than on changeset content. Task #80. ## Algorithm 1. `npm view @bookedsolid/rea version` → PREV_VERSION 2. Resolve `v<version>` tag (fetch-depth:0 on the new job, with a `git fetch --depth=1 origin refs/tags/...` fallback) 3. `git diff --name-only <tag> HEAD -- src/` → if 0 files, skip 4. `npm pack <pkg>@<prev>` in a tempdir, extract 5. Hash both dist/ trees: find dist -type f -print0 | sort -z | xargs -0 shasum -a 256 | shasum -a 256 (recipe matches release.yml:82,130 exactly so this gate and the post-publish verify step speak the same digest) 6. Equal hashes + src changed → exit 2 REGRESSION ## Bypass resistance Verified by adversarial review (Codex pass 1 + pass 2): - package.json rename — visible in PR diff, caught elsewhere - Local tag shadowing — CI uses origin-fetched tag via fetch-depth:0 - Env-var registry override — workflow-level perms, attacker can't redirect install in an untrusted-PR sandbox - argv injection — script reads no positional args - Symlink farming — REPO_ROOT resolved via `pwd -P` - Hash recipe divergence — byte-for-byte identical to release.yml - Empty-dist bypass — preflight `if [ ! -d dist ]` hard-fails at start - TOCTOU — CI single-runner, no concurrent mutation - `latest` dist-tag manipulation — out of threat model (org compromise) ## Skip-on-infrastructure-failure surface The gate degrades to a clean skip (exit 0, specific log reason) when: - no previous published version exists - the `v<version>` tag is unreachable and cannot be fetched - src/ is byte-identical vs the tag - `npm pack` against the prev version fails - `npm pack` succeeds but produces no .tgz - the fetched tarball has no `package/dist/` Skipping on these does NOT reopen BUG-013: release.yml lines 78-84 (Rebuild dist/ from HEAD before publish) and 110-138 (Verify published tarball dist/ matches CI-built dist/) are the catching net at the moment it matters most. This gate is the common-case PR guard; those steps are the publish-time guarantee. ## Files - scripts/dist-regression-gate.sh (NEW, 220 lines, shellcheck clean) - .github/workflows/ci.yml (+45 lines, new `dist-regression` job) ## Local verification - shellcheck clean - PASS on current branch (src changed vs v0.6.2, dist/ hashes differ) - FAIL (exit 2) when dist/ is replaced with the published 0.6.2 tarball's dist/ - Full 1045-test suite still green Signed-off-by: Jake Strawn <bandy.strawn@clarityhouse.press> * test(hooks): add husky e2e — real git push → hook → exit propagation (task #81) Every existing test in __tests__/hooks/ synthesizes the husky stdin and invokes the hook with spawnSync('bash', ...). That validates the parse logic but not the real plumbing: - Does `git push` actually invoke `.husky/pre-push` when `core.hooksPath=.husky` is set? - Does the hook's non-zero exit actually abort the push? - Is the protected-path block observable in `git push` stderr? A pre-0.5 rea could pass every existing unit test and still fail this one, because a hook that silently exits 0 would let `git push` succeed — the push-to-remote outcome is the only thing that matters for the BUG-008 threat model. Six tests covering the full matrix: 1. protected paths changed → PUSH BLOCKED, remote ref untouched 2. non-protected paths → clean exit 0, remote ref advanced 3. .rea/HALT → REA HALT banner, remote ref untouched 4. REA_SKIP_CODEX_REVIEW waiver → exit 0, reason in stderr 5. review.codex_required: false (helper-returns-scalar branch) 6. counterfactual: noop hook → push succeeds (proves test #1 is load-bearing) The scratch setup uses a bare repo on disk as `origin`, symlinks dist/scripts + dist/audit + dist/policy into the scratch work dir, and uses the SHIPPED `.husky/pre-push` (not a modified copy) so hook drift would fail loudly. Codex adversarial review: 3 iterations, verdict pass on final pass. Findings addressed: - dist/policy symlink added so read-policy-field.js actually resolves (not falls back via ERR_MODULE_NOT_FOUND to fail-closed) - Async IIFE precondition replaced with sync accessSync at module load - Schema-valid validPolicyYaml helper so zod strict accepts the scratch policy and the helper-returns-"false" branch is genuinely exercised - Env hardening: REA_SKIP_CODEX_REVIEW explicitly stripped from child env in test #5 so the scalar-branch proof cannot spuriously pass via the waiver path Signed-off-by: Jake Strawn <bandy.strawn@clarityhouse.press> * chore: bump 0.7.0 changeset — minor (BUG-008 cleanup + regression guards) The earlier 063-defense-in-depth changeset was scoped for a 0.6.3 patch but the branch now includes: - BUG-008 structural cleanup (push-review-core + native git adapter) - Class-level dist/ regression gate (CI job + scripts/dist-regression-gate.sh) - Husky e2e regression guard These warrant a minor bump. Rename + expand the changeset so the "Version Packages" PR reflects the full 0.7.0 scope. Signed-off-by: Jake Strawn <bandy.strawn@clarityhouse.press> * fix(hooks): 0.7.0 Codex follow-ups — master fallback, zero-SHA coverage, bare-remote cleanup Addresses Codex adversarial review findings C1/C2/C3 on fix/0.4.0-tarball-smoke-blockers plus a fail-open miss in `.husky/pre-push` and master-default fork parity with the shared core. C1 — master-default fork support. New-branch push (remote SHA = zero) probes origin/HEAD → origin/main → origin/master via `git rev-parse --verify` in both `hooks/_lib/push-review-core.sh` and `.husky/pre-push`. Prior version hard-coded origin/main as the merge-base anchor, failing-closed noisily on master-default forks. C2 — explicit zero-SHA regression coverage. Three new tests in `push-review-gate-git-adapter.test.ts` exercise the new-branch zero-SHA path (`refs/heads/feature <sha> refs/heads/feature 0000...`) across all probe permutations: origin/HEAD set, origin/HEAD absent with origin/main present, and origin/HEAD + origin/main both absent with origin/master present (C1 fallback). C3 — bare-remote tempdir cleanup. Three push-review-gate suites (no-codex, escape-hatch, skip-push-review) now track both the scratch repo and its bare remote via a `track(repo)` helper. Prior versions only cleaned the scratch repo; the bare remote leaked across CI runs. .husky/pre-push fail-open fix. Earlier body silently `continue`d on an empty merge-base. A push with unrelated histories or transient git failure would pass through without the protected-path check. Now fail-closed with a diagnostic. The bootstrap scenario — first push to an empty remote with no remote-tracking ref at all — is distinguished from the failure path and skipped cleanly, since there is no baseline to diff against. Eight husky-e2e tests green including the policy-based codex_required=false branch and the new `.claude/hooks/` PROTECTED_RE alternative. Shared-core parity verified byte-for-byte between `hooks/_lib/push-review-core.sh` and the `.claude/hooks/_lib/` mirror. Changeset updated. Signed-off-by: Jake Strawn <bandy.strawn@clarityhouse.press> * fix(hooks): .husky/pre-push pass-2 — empty-tree baseline on bootstrap + honor $1 remote Addresses two findings from the second Codex adversarial review pass on 0.7.0: Finding 1 [HIGH]: bootstrap-scenario `continue` in .husky/pre-push was a real fail-open. A first push of protected-path changes to a remote with no resolved remote-tracking ref (no origin/HEAD, no origin/main, no origin/master) silently skipped the gate and shipped without a codex.review receipt. Fix: when no remote ref resolves, use the well-known empty-tree SHA `4b825dc642cb6eb9a060e54bf8d69288fbee4904` as the merge-base baseline. The protected-path diff then runs over the complete change set of the push, so a bootstrap push of a protected path is gated correctly. Finding 2 [MEDIUM]: hook hardcoded `refs/remotes/origin/*` in the fallback probe chain. git passes the remote name as $1 to pre-push; a push to `upstream` would probe stale `origin/*` refs even when `upstream/main` was the valid baseline. Fix: read `$1` as `REMOTE` (default `origin` for direct invocation) and parameterize all three fallback probes. Matches the shared core's `argv_remote="${1:-origin}"` convention at hooks/_lib/push-review-core.sh:214. Two new regression tests in husky-e2e.test.ts: - `blocks a bootstrap push of protected paths to a remote with no tracking refs` — builds a scratch with a truly-empty bare remote, commits a protected-path change, asserts the first push blocks AND that `REA_SKIP_CODEX_REVIEW` still lets it through (proves the block is on the intended gate, not a collateral breakage). - `uses the $1 remote name (not hardcoded origin) for the fallback probe` — renames `origin` to `upstream`, pushes to `upstream`, asserts the gate fires against `upstream/*` tracking refs (not missing `origin/*`). Full suite: 1058 pass / 1 skipped. Husky-e2e: 10/10 green. Signed-off-by: Jake Strawn <bandy.strawn@clarityhouse.press> * fix(hooks): close bootstrap fail-open in shared push-review core (Codex 0.7.0 pass-3) Codex pass-3 found the same bootstrap fail-open pattern in hooks/_lib/push-review-core.sh that 701b631 just removed from .husky/pre-push: when none of origin/HEAD, origin/main, or origin/master resolves as a remote-tracking ref, the shared core set default_ref to the literal "refs/remotes/${argv_remote}/main" — already proven absent one branch up, guaranteeing merge-base failure. The empty merge-base then combined with the longest-diff BEST_COUNT accumulator and allowed a protected-path refspec to bypass the gate on a fresh remote. Mirror the .husky/pre-push treatment: when no remote-tracking ref resolves, seed the merge-base directly with the well-known empty-tree SHA (4b825dc, the output of git hash-object -t tree /dev/null). Downstream git diff then emits the complete change set, so protected-path detection sees every file in the bootstrap push. Converted the two call sites that pipe this baseline (git diff at line 853 and git diff --name-status at line 898) from three-dot to two-dot form. Three-dot syntax computes an implicit merge-base, which git refuses to evaluate when the left side is a tree SHA ("error: object 4b825dc... is a tree, not a commit"). Two-dot is literal left-to-right diff and accepts tree or commit on either side, which is what we want here — the baseline is already the correct merge-base (either a true merge-base computed via git merge-base, or the empty-tree sentinel). Dogfood mirror synced byte-for-byte via cp. Tests: husky-e2e regression suite 10/10 green (includes bootstrap and $1-remote regression tests from 701b631). Full hooks matrix 76/76, full suite 1058/1059 (1 pre-existing skip). Signed-off-by: Jake Strawn <bandy.strawn@clarityhouse.press> * fix(hooks): shared-core parity hardening (Codex 0.7.0 pass-4) Codex pass-4 surfaced two HIGH parity gaps between hooks/_lib/push-review-core.sh and .husky/pre-push, plus one missing regression test. Pass-3 closed the no-default-ref bootstrap fail-open; pass-4 closes the remaining two paths and rounds out test coverage. Finding #1 (HIGH): hooks/_lib/push-review-core.sh:805-807 silently continue'd when default_ref resolved (origin/HEAD, origin/main, or origin/master present) but git merge-base returned empty — the unrelated-histories / grafted-branch / transient-failure scenario. Combined with the longest-diff BEST_COUNT accumulator, a protected- path refspec with an empty merge-base could slip through whenever another refspec in the same push was selected as BEST. Fall through to the empty-tree baseline instead, matching the spirit of the pass-3 fix for the no-default-ref case — the protected-path check still runs against the full refspec content, so the gate remains effective. Finding #2 (HIGH): the protected-path Codex-audit check ran once on the BEST_COUNT-selected refspec, while .husky/pre-push's per-refspec loop checks every refspec. A push like big:big hotfix:main where big is 50 non-protected commits and hotfix is one commit touching hooks/foo.sh would be blocked by husky and silently passed by the shared core. Move the protected-path check INSIDE the main per-refspec loop and require the Codex audit entry to match each protected refspec's own local_sha. The post-loop section 7a reduces to a short comment noting the check moved up. Finding #3 (MEDIUM): the adapter suite exercised three new-branch zero-SHA permutations (origin/HEAD set, origin/main fallback, origin/master fallback) but not the fully-empty case — that was only covered by husky-e2e. Added an adapter-level test for all-three-refs- missing with a REA_SKIP_CODEX_REVIEW sanity bypass to prove the block fired on the protected-path gate rather than on a ref-resolution error. Findings #4/#5 (LOW): the action-required hint at the end of the shared core was still three-dot, which would fail verbatim in the bootstrap case where MERGE_BASE is the empty-tree SHA. Converted to two-dot to match the rest of the file. The stale docstring was refreshed to reflect pass-3's two-dot conversion and line numbers. Dogfood mirror .claude/hooks/_lib/push-review-core.sh synced byte-for-byte via cp. Tests: adapter suite 16 -> 17 (new bootstrap regression green), full hooks matrix 76 -> 77, full suite 1059/1060 (1 pre-existing skip). Signed-off-by: Jake Strawn <bandy.strawn@clarityhouse.press> * test(hooks): multi-refspec regression for pass-4 finding #2 Pass-4 moved the protected-path Codex-audit check inside the per- refspec loop so that a push like `git push origin clean:clean hotfix:main` cannot hide a small protected-path refspec behind a larger non-protected one. Pass-5 review verified the code is correct but flagged that every prepushLine in the adapter suite is a single refspec — no direct regression test for the hide- behind scenario. A future refactor that moves the check back outside the loop or short-circuits after the BEST_COUNT winner would silently reintroduce the HIGH parity gap. Add a multi-refspec adapter test with the clean refspec first (non-protected, touches README.md) and the protected refspec second (touches hooks/). Asserts: - gate blocks (exit 2) - stderr matches /protected paths changed/ - stderr contains the PROTECTED refspec's sha (not the clean one's), proving the per-refspec loop caught the right refspec Uses the existing makeRepo fixture's cleanFeatureSha and featureSha — no fixture changes required. Adapter suite: 17 -> 18. Full hooks matrix stays green. Signed-off-by: Jake Strawn <bandy.strawn@clarityhouse.press> * fix(ci): isolate test env from runner flags + add 0.7.0 husky body to allowlist Two runner-only regressions surfaced in PR #44: 1) push-review-gate-git-adapter.test.ts inherits process.env in its spawnSync calls. GitHub Actions sets CI=true, which trips the Codex F2 CI-aware refusal of REA_SKIP_PUSH_REVIEW and flips the expected exit code from 0 to 2. Locally the flag is unset and the tests passed. Clear CI + GITHUB_ACTIONS on the two bypass-path tests so the non-runner branch executes deterministically. The parity-matrix "F2 positive case" (opt-in via review.allow_skip_in_ci=true) is unaffected. 2) src/cli/install/pre-push.test.ts reads the shipped .husky/pre-push body via `git show main:.husky/pre-push` with a working-tree fallback. The runner checks out at fetch-depth:1, so `git show main:` fails and the tests fall back to the modified working-tree body. Its SHA256 was not yet in KNOWN_LEGACY_HUSKY_SHA256, so the R24 F2 and R25 F3 byte-identical allowlist tests returned false. Add the 0.7.0 body hash (empty-tree baseline + $1 remote honoring + fail-closed on empty merge-base) to the allowlist. Signed-off-by: Jake Strawn <bandy.strawn@clarityhouse.press> --------- Signed-off-by: Jake Strawn <bandy.strawn@clarityhouse.press> Co-authored-by: Jake Strawn <bandy.strawn@clarityhouse.press>
himerus
added a commit
that referenced
this pull request
Apr 21, 2026
* fix(security): close mixed-push delete bypass + portable hasher + schema fixes (0.9.4)
Closes four push-review-core.sh defects from HELiX PR #1506 CodeRabbit review:
- J (CRITICAL, rea#61): hoist HAS_DELETE guard above `-z SOURCE_SHA` fallback.
Mixed push `git push origin safe:safe :main` was silently letting the
deletion through because a non-delete refspec pre-populated SOURCE_SHA.
- K (MEDIUM, rea#62): `grep -c ... || echo "0"` captured "0\n0" into
LINE_COUNT / FILE_COUNT; PUSH REVIEW GATE banner rendered "0\n0 files
changed". Replace with `|| true` + `\${VAR:-0}` default.
- L (HIGH, rea#63): `shasum` missing on Alpine / distroless silently
produced empty PUSH_SHA, disarming the cache. Portable sha256sum →
shasum → openssl chain with hex-64 validation and a WARN fallback.
openssl branch uses `awk '{print \$NF}'` (no -r) to stay compatible
with OpenSSL 1.1.x.
- M (MEDIUM, rea#64): SKIP_METADATA stringified os_pid / os_ppid via
`jq --arg`. Use `--argjson` so downstream auditors querying numeric
pid match. os_uid stays on --arg (id -u can be empty).
Regression suite: new `push-review-gate-portability-security.test.ts`
(9 cases covering all four defects). Existing pid/ppid type assertion
in `push-review-gate-skip-push-review.test.ts` flipped from string to
number per M.
Dogfood mirror at `.claude/hooks/_lib/push-review-core.sh` kept
byte-identical.
Signed-off-by: Jake Strawn <bandy.strawn@clarityhouse.press>
* fix(security): address 0.9.4 pass-1 Codex concerns
- pass-1 concern 1 (UX): on a hasher-less host, PUSH_SHA is empty and the
banner's step 3 was emitting `rea cache set pass --branch X --base Y`
(blank SHA, CLI rejects it). Branch the step-3 text so the no-hasher
path directs the operator to REA_SKIP_PUSH_REVIEW (the documented
audited escape hatch) and documents the hasher-install path to restore
the cache.
- pass-1 concern 2 (sibling Defect K): `hooks/commit-review-gate.sh:117`
and `hooks/_lib/common.sh:90` both carried the same `|| echo "0"` bug.
At those sites LINE_COUNT feeds `[[ -gt ]]` / `[[ -ge ]]` arithmetic,
where `0\n0` tripped "syntax error in expression" at runtime on any
rename-only / empty-file diff. Applied the same `|| true` + `${VAR:-0}`
pattern. Mirrors synced.
- pass-1 concern 3 (weak K test): original fixture added a file with
content, so grep matched 1 and the no-match branch was never
exercised — the assertions passed even against the pre-fix script.
Rewrote the K test to use an empty-file add (diff has zero
+content/-content lines, forces grep's no-match exit). Added a new
commit-review-gate K test that asserts stderr does NOT contain a bash
"syntax error in expression" on a zero-line staged diff.
Pass-1 concern 4 (wire-format note on pid/ppid) is acknowledged in the
changeset text; the patch bump stays (the prior emission was a bug).
Signed-off-by: Jake Strawn <bandy.strawn@clarityhouse.press>
* fix(security): extend pass-2 portability + cache-predicate fixes to commit-gate (0.9.4)
Codex pass-2 surfaced two high-severity findings in `hooks/commit-review-gate.sh`
that structurally mirror defects already fixed in `push-review-core.sh` but were
missed because the bug report only described the push path:
1. CLI cache predicate was `.hit == true` — a cached `fail` verdict satisfied
the gate and let the commit proceed. Tighten to `.hit == true and .result ==
"pass"` (matches the direct-file fallback two branches below, which already
enforces `result == "pass"`).
2. STAGED_SHA used the same `shasum -a 256 ... || echo ""` non-portable chain
that Defect L (rea#63) fixed in push-review-core.sh. On Alpine / distroless
the hasher was missing, STAGED_SHA ended up empty, and the section-11 banner
rendered `rea cache set pass` — a dead-end the CLI rejects. Apply the
portable hasher chain (sha256sum → shasum → openssl with `$NF` not `-r`) +
hex-64 validation, and branch the banner so the empty-STAGED_SHA path emits
an actionable "install a hasher or escalate" message instead of the invalid
cache-set line.
Regression coverage: two new cases in
`__tests__/hooks/push-review-gate-portability-security.test.ts` pin both fixes:
a `rea` shim returning `{"hit":true,"result":"fail"}` must still block, and a
PATH that excludes all three hashers must render "Cache is DISABLED" instead of
the dead-end cache-set line.
Dogfood mirror synced.
Signed-off-by: Jake Strawn <bandy.strawn@clarityhouse.press>
* fix(security): commit-gate must pass --base to rea cache CLI (0.9.4 pass-3)
Codex pass-3 blocked on a pre-existing contract bug in `hooks/commit-review-gate.sh`
that pass-2 sat next to without fixing. `rea cache check` and `rea cache set`
both declare `--base` as a `requiredOption` in src/cli/index.ts, but the commit
gate never passed it: the `cache check` call omitted it entirely (the CLI's
non-zero exit was silently swallowed by `|| echo '{"hit":false}'`), and the
section-11 banner instructed agents to run `rea cache set <sha> pass` — a
command the real CLI rejects on every invocation. The cache path was
structurally unreachable in any CLI-backed run.
Resolve BASE_BRANCH by the same preference order push-review-core.sh uses:
origin/HEAD → origin/main → origin/master → empty. If nothing resolves,
disable the cache and emit a clear WARN instead of a command the CLI rejects.
Pass --base through to the `cache check` call and to the banner's `rea cache
set` line.
Harden the cache-predicate regression test's `rea` shim to record every argv
it receives, and assert every `cache check` invocation includes both `--branch`
and `--base`. Pre-fix, the argv assertion fails (pass-3 #2); the happy-path
stub masked the contract mismatch until now. Also extend the test helper to
set up a bare origin + symbolic-ref origin/HEAD so the new BASE_BRANCH probe
can resolve in the sandbox.
Dogfood mirror synced.
Signed-off-by: Jake Strawn <bandy.strawn@clarityhouse.press>
* style(hooks): use git -C over cd && git for BASE_BRANCH fallback probes
Codex pass-4 LOW finding: the fallback origin/main / origin/master probes
used inline `cd "$REA_ROOT" && git rev-parse ...` which mutates the hook
shell's cwd for the rest of the process. Safe today (no downstream
relative-path usage) but breaks the file's dominant idiom — every other
git invocation uses either command-substitution `$(cd … && git …)` or
`git -C "$REA_ROOT" …`. Switched to `git -C` to match the cross-repo
guard at §1a.
Signed-off-by: Jake Strawn <bandy.strawn@clarityhouse.press>
---------
Signed-off-by: Jake Strawn <bandy.strawn@clarityhouse.press>
Co-authored-by: Jake Strawn <bandy.strawn@clarityhouse.press>
This was referenced Apr 22, 2026
Merged
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.
Bumps actions/setup-node from 4.0.3 to 6.3.0.
Release notes
Sourced from actions/setup-node's releases.
... (truncated)
Commits
53b8394Bump minimatch from 3.1.2 to 3.1.5 (#1498)54045abScope test lockfiles by package manager and update cache tests (#1495)c882bffReplace uuid with crypto.randomUUID() (#1378)774c1d6feat(node-version-file): support parsingdevEnginesfield (#1283)efcb663fix: remove hardcoded bearer (#1467)d02c89dFix npm audit issues (#1491)6044e13Docs: bump actions/checkout from v5 to v6 (#1468)8e49463Fix README typo (#1226)621ac41README.md: bump to latest released checkout version v6 (#1446)2951748Bump@actions/cacheto v5.0.1 (#1449)