v1.57.6.0 fix wave: 8 community bugs (4 security guards failing open)

CHANGELOG.md

@@ -1,5 +1,94 @@
 # Changelog
 
+## [1.57.6.0] - 2026-06-07
+
+## **Eight community-filed bugs fixed in one wave, four of them security guards that were quietly failing open.**
+## **Your redaction gate now catches modern OpenAI keys, and `/ship`'s adversarial review stops choking on your own security tests.**
+
+This is a fix wave. The throughline: guards that reported success while doing nothing.
+The secret-redaction gate that every `/spec`, `/ship`, `/cso`, and `/document-*` run
+passes through was blind to modern `sk-proj-`/`sk-svcacct-`/`sk-admin-` OpenAI keys and
+silently dropped its size cap on a bad flag. The cross-project learnings trust gate was
+an allowlist on paper and a denylist in code, so untrusted rows leaked between projects.
+The destructive-action classifier waved through "rotate the database password." Each one
+looked like it was protecting you. None of them were. All four now fail closed, with
+tests that pin the exact case that used to slip by. Three more fixes clear silent
+crashes and skipped reviewers, and `/ship`'s adversarial pass no longer trips Anthropic's
+usage policy when it reads your repo's own attack-payload fixtures.
+
+### The numbers that matter
+
+Reproduce with `bun test test/redact-engine.test.ts test/gstack-learnings-search.test.ts test/one-way-doors.test.ts test/diff-scope.test.ts test/brain-cache-roundtrip.test.ts`.
+
+| Guard / path | Before | After |
+|---|---|---|
+| `sk-proj-`/`sk-svcacct-`/`sk-admin-` OpenAI keys | zero findings (HIGH fails open) | blocked, with prose false-positive guards |
+| `gstack-redact --max-bytes <garbage>` | NaN silently disables the size cap | rejected at the CLI; engine backstop holds |
+| Cross-project learnings with no `trusted` field | imported (denylist bug) | excluded (true allowlist) |
+| "rotate the database password" | classified two-way (auto-approvable) | classified one-way (always asks) |
+| `.mjs/.cjs/.mts/.cts`-only PRs | backend reviewer skipped | backend reviewer runs |
+| `_meta.json` missing `last_refresh` | brain-cache crashes (TypeError) | degrades to a cold cache |
+| Safety-skill hooks on Claude Code 2.1.162 | every Edit/Write errored | hooks resolve and run |
+| `/ship` adversarial review over security fixtures | denied by usage policy | runs, fixtures read in summary mode |
+
+The redaction one is the sharpest: a project/service-account/admin OpenAI key pasted
+into a spec or PR body used to sail straight through the gate. Now it blocks, and the
+calibration is pinned so hyphenated prose like "the sk-learning-rate schedule" does not
+false-positive and wedge your ship.
+
+### What this means for you
+
+If you rely on the redaction guard or the cross-project learnings gate, they now do what
+the docs always said. If you run `/ship` on a repo that tests its own security guards,
+adversarial review stops dying on contact with your fixtures. And if you are on Claude
+Code 2.1.162, `/guard`, `/freeze`, and `/careful` work again instead of erroring on every
+edit. Upgrade and re-run anything that touched these paths.
+
+### Itemized changes
+
+#### Fixed
+- **Redaction misses modern OpenAI keys (#1868).** `openai.key` (HIGH/block) used a
+  contiguous-alphanumeric pattern that stopped at the first `-`/`_`, so base64url-bodied
+  `sk-proj-`/`sk-svcacct-`/`sk-admin-` keys produced no finding and failed open through
+  every redaction sink. Replaced with explicit bare-vs-prefixed alternation; added
+  positive and false-positive tests. Reported by @jbetala7.
+- **Redaction size cap fails open on a bad flag (#1824).** A malformed `--max-bytes`
+  parsed to `NaN`, and `byteLen > NaN` is always false, silently disabling the
+  fail-closed oversize guard; a negative value blocked everything. The CLI now rejects
+  non-integer / non-positive values, and the engine falls back to the default cap as a
+  backstop. Reported by @jbetala7.
+- **Cross-project learnings trust gate leaked (#1745).** `gstack-learnings-search
+  --cross-project` is documented as an allowlist but was coded as `trusted === false`,
+  admitting any row missing the `trusted` field. Flipped to `trusted !== true`. Reported
+  by @jbetala7.
+- **Destructive-action classifier missed "rotate ... password" (#1839).** The `rotate`
+  keyword pattern omitted `password` while its `revoke`/`reset` siblings included it, so
+  the most common credential-rotation phrasing classified as a reversible two-way
+  question. Added `password` to the alternation.
+- **Review Army skipped backend reviewer on ESM/CJS PRs (#1810).** `gstack-diff-scope`
+  matched only `*.ts|*.js`; a PR touching only `.mjs/.cjs/.mts/.cts` reported no backend
+  scope. Added the four module extensions. Reported by @jbetala7.
+- **Brain-cache crash on a partial `_meta.json` (#1879).** `loadMeta` returned parsed
+  JSON verbatim; a file missing `last_refresh` crashed three consumers with a TypeError.
+  Added an object-shape guard and map normalization; missing schema/endpoint identity now
+  forces a safe rebuild rather than trusting a stale file. Reported by @jbetala7.
+- **Safety-skill hooks broken on Claude Code 2.1.162 (#1871).** `guard`, `freeze`, and
+  `careful` frontmatter hooks used `${CLAUDE_SKILL_DIR}`, which CC 2.1.162 no longer
+  populates, so every Edit/Write/Bash errored. Anchored the hook commands to the
+  installed checkout path. Reported by @omariani-howdy.
+- **`/ship` adversarial review denied on own security fixtures (#1899).** The Claude
+  adversarial subagent reasoned "like an attacker" over the full diff; when the diff
+  included the repo's own attack-payload regression fixtures, Anthropic's real-time
+  usage-policy safeguards denied the call. The subagent now carries authorized-defensive
+  -testing framing and reads fixture/test files in summary mode (no raw payload bytes),
+  stating so explicitly. Reported by @bmajewski.
+
+#### For contributors
+- `#1882` (skills hardcode `~/.claude/skills/gstack/`, breaking non-`gstack` install
+  dirs) is filed as the top item in `TODOS.md`. It was scoped out of this wave once it
+  proved to be a host-config/preamble change touching all 52 skills, distinct from the
+  `#1871` hook fix it was originally paired with.
+
 ## [1.57.5.0] - 2026-06-07
 
 ## **Your agent now keeps its decisions, not just its code.**

TODOS.md

@@ -1,5 +1,48 @@
 # TODOS
 
+## NEXT PRIORITY
+
+### P1: #1882 — portable skill-install prefix (non-`gstack` install dirs break silently)
+
+**What:** Every generated SKILL.md hardcodes the literal `~/.claude/skills/gstack/...`
+for its `bin/`/asset calls (the per-invocation telemetry/config preamble plus ~9
+resolvers). `setup` wires the top-level skill symlinks for any directory name, so
+installing at `~/.claude/skills/<other>` leaves every internal `bin` reference
+pointing at a non-existent `~/.claude/skills/gstack/` path — failing **silently, at
+skill-invocation time**. Make the emitted references portable: resolve the install
+root at runtime (the preamble already defines `GSTACK_ROOT`/`GSTACK_BIN` in
+`scripts/resolvers/preamble/generate-preamble-bash.ts` but the literals don't use
+them) and emit `$GSTACK_BIN`-relative paths instead of the hardcoded prefix.
+
+**Why:** Filed as #1882. Split out of the June 2026 fix wave (decision A) once
+implementation showed it is a host-config/design change, not a fix-wave patch. The
+urgent half — the guard/freeze/careful frontmatter hooks broken on CC 2.1.162 — was
+already fixed in that wave (#1871) with a literal `$HOME`-anchored path, because
+frontmatter hooks run before any runtime variable exists and cannot use `$GSTACK_BIN`.
+So #1882 is now purely the body-preamble portability work.
+
+**Pros:** Unblocks installs at any directory name; removes a whole class of silent
+invocation-time failures.
+**Cons:** Touches the most load-bearing bash in the repo (every skill's preamble);
+a silent mistake breaks all 52 skills. High blast radius — needs its own focused PR.
+
+**Context / where to start:**
+- Rewire `ctx.paths.binDir` (and browse/design dir paths) + the ~9 resolvers that
+  emit the literal (`testing.ts`, `review.ts`, `design.ts`, `browse.ts`,
+  `redact-doc.ts`, `tasks-section.ts`, `preamble/generate-*.ts`) to use the
+  preamble-defined `$GSTACK_ROOT`/`$GSTACK_BIN`.
+- Ensure `GSTACK_ROOT`/`GSTACK_BIN` are defined before first use in EVERY skill's
+  preamble (verify the telemetry preamble's first bin call is after the definition).
+- **Test conflict (verified):** `test/gen-skill-docs.test.ts:1942` and the sibling
+  ship assertion currently *assert* generated Claude output `.toContain('~/.claude/skills/gstack')`
+  as a guardrail that Codex-host paths don't leak. These must be rewritten to match
+  the new portable scheme.
+- Regenerate all 52 SKILL.md (`bun run scripts/gen-skill-docs.ts --host all`); never
+  hand-edit generated files. Bisect: resolver/host-config change commit, then the
+  52-file regen commit.
+- Smoke-test a skill invocation from a non-`gstack` install dir to prove the fix.
+- Sibling of #349 (the `$CLAUDE_CONFIG_DIR` / `~/.claude` path issue).
+
 ## Test infrastructure
 
 ### ✅ DONE (v1.53.1.0): Rebaseline parity-suite (v1.44.1 → v1.53.0.0)

VERSION

@@ -1 +1 @@
-1.57.5.0
+1.57.6.0

bin/gstack-brain-cache

@@ -83,7 +83,23 @@ function loadMeta(scope: 'cross-project' | 'per-project', projectSlug: string |
     return { schema_version: GSTACK_SCHEMA_PACK_VERSION, endpoint_hash: detectEndpointHash(), last_refresh: {}, last_attempt: {} };
   }
   try {
-    return JSON.parse(readFileSync(path, 'utf-8')) as CacheMeta;
+    const parsed = JSON.parse(readFileSync(path, 'utf-8')) as unknown;
+    // #1879: a valid JSON file can still be the wrong shape. JSON.parse can return
+    // null/array/string/number, and a partial object can omit last_refresh — three
+    // consumers (isStale, cmdInvalidate, refreshEntity) dereference meta.last_refresh
+    // unguarded and crash with a TypeError.
+    if (typeof parsed !== 'object' || parsed === null || Array.isArray(parsed)) {
+      return { schema_version: GSTACK_SCHEMA_PACK_VERSION, endpoint_hash: detectEndpointHash(), last_refresh: {}, last_attempt: {} };
+    }
+    const meta = parsed as CacheMeta;
+    // Normalize ONLY the dereferenced maps. Do NOT default schema_version /
+    // endpoint_hash — leaving them absent makes schemaVersionMismatch() /
+    // endpointSwitched() correctly force a rebuild (missing identity = mismatch =
+    // safe). Defaulting them to current values would suppress invalidation and
+    // trust a stale file of unknown provenance.
+    meta.last_refresh = meta.last_refresh ?? {};
+    meta.last_attempt = meta.last_attempt ?? {};
+    return meta;
   } catch {
     // Corrupt _meta — start fresh (entries will refresh on next access).
     return { schema_version: GSTACK_SCHEMA_PACK_VERSION, endpoint_hash: detectEndpointHash(), last_refresh: {}, last_attempt: {} };

bin/gstack-diff-scope

@@ -75,7 +75,10 @@ while IFS= read -r f; do
 
     # Backend: everything else that's code (excluding views/components already matched)
     *.rb|*.py|*.go|*.rs|*.java|*.php|*.ex|*.exs) BACKEND=true ;;
-    *.ts|*.js) BACKEND=true ;;  # Non-component TS/JS is backend
+    # Non-component TS/JS is backend. Include ESM/CJS (.mjs/.cjs) and
+    # explicit-module TS (.mts/.cts) — #1810: these matched no category, so an
+    # ESM/CJS-only PR skipped the backend reviewer entirely.
+    *.ts|*.js|*.mjs|*.cjs|*.mts|*.cts) BACKEND=true ;;
   esac
 done <<< "$FILES"
 

bin/gstack-learnings-search

@@ -90,10 +90,16 @@ for (const taggedLine of lines) {
     const isCrossProject = sourceTag === 'cross';
     e._crossProject = isCrossProject;
 
-    // Trust gate: cross-project learnings only loaded if trusted (user-stated)
+    // Trust gate: cross-project learnings only loaded if trusted (user-stated).
     // This prevents prompt injection from one project's AI-generated learnings
     // silently influencing reviews in another project.
-    if (isCrossProject && e.trusted === false) continue;
+    // #1745: this is an ALLOWLIST, not a denylist. The old equals-false check
+    // admitted any row where trusted is missing/undefined (legacy rows written
+    // before the field existed, hand-edited rows, rows from other tools).
+    // Require trusted to be exactly true. NOTE: this whole block is a
+    // double-quoted bun -e string, so bash still does command substitution
+    // inside it. Keep backticks and dollar-paren out of these comments.
+    if (isCrossProject && e.trusted !== true) continue;
 
     entries.push(e);
   } catch {}

bin/gstack-redact

@@ -161,12 +161,25 @@ function readLines(path: string | undefined): string[] | undefined {
 function buildOpts(): ScanOptions {
   const vis = (arg("--repo-visibility") as RepoVisibility) || "unknown";
   const maxBytes = arg("--max-bytes");
+  // #1824: validate the RAW string, not the parse result. parseInt("123abc")
+  // is 123 and parseInt("foo") is NaN — both silently corrupt the fail-closed
+  // oversize guard. Require a clean positive integer or reject before scanning.
+  let maxBytesOpt: number | undefined;
+  if (maxBytes !== undefined) {
+    if (!/^\d+$/.test(maxBytes) || Number(maxBytes) <= 0) {
+      process.stderr.write(
+        `gstack-redact: --max-bytes must be a positive integer (got "${maxBytes}")\n`,
+      );
+      process.exit(1);
+    }
+    maxBytesOpt = Number(maxBytes);
+  }
   return {
     repoVisibility: ["public", "private", "unknown"].includes(vis) ? vis : "unknown",
     allowlist: readLines(arg("--allowlist")),
     selfEmail: arg("--self-email"),
     repoPublicEmails: readLines(arg("--repo-public-emails")),
-    ...(maxBytes ? { maxBytes: parseInt(maxBytes, 10) } : {}),
+    ...(maxBytesOpt !== undefined ? { maxBytes: maxBytesOpt } : {}),
   };
 }
 

careful/SKILL.md

@@ -14,7 +14,7 @@ hooks:
     - matcher: "Bash"
       hooks:
         - type: command
-          command: "bash ${CLAUDE_SKILL_DIR}/bin/check-careful.sh"
+          command: "bash $HOME/.claude/skills/gstack/careful/bin/check-careful.sh"
           statusMessage: "Checking for destructive commands..."
 ---
 <!-- AUTO-GENERATED from SKILL.md.tmpl — do not edit directly -->

careful/SKILL.md.tmpl

@@ -19,7 +19,7 @@ hooks:
     - matcher: "Bash"
       hooks:
         - type: command
-          command: "bash ${CLAUDE_SKILL_DIR}/bin/check-careful.sh"
+          command: "bash $HOME/.claude/skills/gstack/careful/bin/check-careful.sh"
           statusMessage: "Checking for destructive commands..."
 sensitive: true
 ---

freeze/SKILL.md

@@ -15,12 +15,12 @@ hooks:
     - matcher: "Edit"
       hooks:
         - type: command
-          command: "bash ${CLAUDE_SKILL_DIR}/bin/check-freeze.sh"
+          command: "bash $HOME/.claude/skills/gstack/freeze/bin/check-freeze.sh"
           statusMessage: "Checking freeze boundary..."
     - matcher: "Write"
       hooks:
         - type: command
-          command: "bash ${CLAUDE_SKILL_DIR}/bin/check-freeze.sh"
+          command: "bash $HOME/.claude/skills/gstack/freeze/bin/check-freeze.sh"
           statusMessage: "Checking freeze boundary..."
 ---
 <!-- AUTO-GENERATED from SKILL.md.tmpl — do not edit directly -->

freeze/SKILL.md.tmpl

@@ -20,12 +20,12 @@ hooks:
     - matcher: "Edit"
       hooks:
         - type: command
-          command: "bash ${CLAUDE_SKILL_DIR}/bin/check-freeze.sh"
+          command: "bash $HOME/.claude/skills/gstack/freeze/bin/check-freeze.sh"
           statusMessage: "Checking freeze boundary..."
     - matcher: "Write"
       hooks:
         - type: command
-          command: "bash ${CLAUDE_SKILL_DIR}/bin/check-freeze.sh"
+          command: "bash $HOME/.claude/skills/gstack/freeze/bin/check-freeze.sh"
           statusMessage: "Checking freeze boundary..."
 sensitive: true
 ---

guard/SKILL.md

@@ -15,17 +15,17 @@ hooks:
     - matcher: "Bash"
       hooks:
         - type: command
-          command: "bash ${CLAUDE_SKILL_DIR}/../careful/bin/check-careful.sh"
+          command: "bash $HOME/.claude/skills/gstack/careful/bin/check-careful.sh"
           statusMessage: "Checking for destructive commands..."
     - matcher: "Edit"
       hooks:
         - type: command
-          command: "bash ${CLAUDE_SKILL_DIR}/../freeze/bin/check-freeze.sh"
+          command: "bash $HOME/.claude/skills/gstack/freeze/bin/check-freeze.sh"
           statusMessage: "Checking freeze boundary..."
     - matcher: "Write"
       hooks:
         - type: command
-          command: "bash ${CLAUDE_SKILL_DIR}/../freeze/bin/check-freeze.sh"
+          command: "bash $HOME/.claude/skills/gstack/freeze/bin/check-freeze.sh"
           statusMessage: "Checking freeze boundary..."
 ---
 <!-- AUTO-GENERATED from SKILL.md.tmpl — do not edit directly -->

guard/SKILL.md.tmpl

@@ -20,17 +20,17 @@ hooks:
     - matcher: "Bash"
       hooks:
         - type: command
-          command: "bash ${CLAUDE_SKILL_DIR}/../careful/bin/check-careful.sh"
+          command: "bash $HOME/.claude/skills/gstack/careful/bin/check-careful.sh"
           statusMessage: "Checking for destructive commands..."
     - matcher: "Edit"
       hooks:
         - type: command
-          command: "bash ${CLAUDE_SKILL_DIR}/../freeze/bin/check-freeze.sh"
+          command: "bash $HOME/.claude/skills/gstack/freeze/bin/check-freeze.sh"
           statusMessage: "Checking freeze boundary..."
     - matcher: "Write"
       hooks:
         - type: command
-          command: "bash ${CLAUDE_SKILL_DIR}/../freeze/bin/check-freeze.sh"
+          command: "bash $HOME/.claude/skills/gstack/freeze/bin/check-freeze.sh"
           statusMessage: "Checking freeze boundary..."
 sensitive: true
 ---

lib/redact-engine.ts

@@ -253,7 +253,16 @@ function emailAllowed(email: string, opts: ScanOptions): boolean {
 
 export function scan(input: string, opts: ScanOptions = {}): ScanResult {
   const repoVisibility: RepoVisibility = opts.repoVisibility ?? "unknown";
-  const maxBytes = opts.maxBytes ?? DEFAULT_MAX_BYTES;
+  // #1824: ?? only catches null/undefined, not NaN or <= 0. A bad value
+  // (NaN from a malformed --max-bytes, or a negative) would make `byteLen >
+  // maxBytes` always false and silently disable the fail-closed oversize guard.
+  // Guardrail: any non-finite or non-positive value falls back to the default
+  // cap. The CLI is the layer that rejects bad args; this is belt-and-suspenders
+  // so the engine never silently runs uncapped.
+  const maxBytes =
+    Number.isFinite(opts.maxBytes) && (opts.maxBytes as number) > 0
+      ? (opts.maxBytes as number)
+      : DEFAULT_MAX_BYTES;
 
   // Fail CLOSED on oversize input. Check byte length BEFORE heavy work.
   const byteLen = Buffer.byteLength(input, "utf8");

lib/redact-patterns.ts

@@ -233,8 +233,13 @@ export const PATTERNS: RedactPattern[] = [
     id: "openai.key",
     tier: "HIGH",
     category: "secret",
-    description: "OpenAI API key (incl. sk-proj-)",
-    regex: /\b(sk-(?:proj-)?[A-Za-z0-9]{32,})\b/,
+    description: "OpenAI API key (incl. sk-proj-/sk-svcacct-/sk-admin-)",
+    // Two explicit shapes (NOT a globally-optional prefix, which would match
+    // malformed sk--... or separator-less sk-projabc...):
+    //   prefixed: sk-{proj,svcacct,admin}- + base64url-ish body (allows -_)
+    //   bare:     sk- + contiguous alphanumeric run (legacy), keeps {32,} floor
+    regex:
+      /\b(sk-(?:proj|svcacct|admin)-[A-Za-z0-9_-]{20,}|sk-[A-Za-z0-9]{32,})\b/,
   },
   {
     id: "sendgrid.key",

package.json

@@ -1,6 +1,6 @@
 {
   "name": "gstack",
-  "version": "1.57.5.0",
+  "version": "1.57.6.0",
   "description": "Garry's Stack — Claude Code skills + fast headless browser. One repo, one install, entire AI engineering workflow.",
   "license": "MIT",
   "type": "module",