diff --git a/.github/workflows/free-tests.yml b/.github/workflows/free-tests.yml index 12ff67a32..ea91ce010 100644 --- a/.github/workflows/free-tests.yml +++ b/.github/workflows/free-tests.yml @@ -79,6 +79,17 @@ jobs: run: bun run test:cso:macos env: GSTACK_CSO_MACOS_TESTS: "1" + - name: Build browser lifecycle prerequisites + run: | + bunx playwright install chromium + bun run build + - name: Exercise native agent ownership and linked settings + env: + TMPDIR: /tmp + run: | + files=(browse/test/terminal-agent-lifecycle.test.ts browse/test/terminal-agent-native-observation.test.ts browse/test/terminal-agent-watchdog.test.ts browse/test/server-embedder-terminal-port.test.ts browse/test/server-factory.test.ts test/gstack-settings-hook-symlink.test.ts test/gstack-settings-hook-schema-aware.test.ts) + for file in "${files[@]}"; do test -f "$file"; done + bun test "${files[@]}" cso-windows-launcher: runs-on: windows-latest diff --git a/CHANGELOG.md b/CHANGELOG.md index 585f56b6e..9e64410d6 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,21 @@ # Changelog +## [1.88.1.0] - 2026-09-22 + +Credential masking follows the exact detected source, and pre-push scans follow the actual destination. Browser agents and CSO operations retain precise ownership, while settings updates and artifact reinitialization preserve user-owned data. + +### Fixed +- Credential masking uses original spans mapped from normalized matches, so anchored assignments, repeated values, Unicode and entity input redact the flagged value without masking a later neighbor. Sanitizable CSO source stays readable as masked text; unmaskable payloads remain withheld. +- Pre-push scans bind their ranges to the destination's name and effective URL, including separate push URLs. Missing advertised objects use a conservative range, SHA-256 repositories get the correct empty tree, and bounded slices retain normalized proximity context without fabricating line anchors or duplicate findings. Unscannable long lines or context windows still block explicitly. +- Hermetic evaluation children reject credential-shaped variables admitted through broad CI prefixes, while preserving CI metadata, named provider authentication and explicit runner overrides. +- Browser agents belong to the persistent daemon and an exact process generation. Replacement requires confirmed exit, failed startup retains uncertain children, and publication and shutdown share a lock so older generations cannot delete a successor's discoverability files. Repeated failed respawns are bounded. Uncertain ownership and abandoned publication locks require manual recovery rather than a process sweep. +- CSO lease recovery preserves 64-bit filesystem identities and nanosecond timestamps instead of rounding them. Adjacent identities remain distinct, replaced files remain protected, and ambiguous legacy state is still refused. +- Settings mutations, backups and rollback resolve the selected symlink target and share its canonical lock. Links, private file modes and unrelated settings survive; changed or invalid targets are refused. +- Artifact reinitialization updates managed allowlist entries while retaining the user suffix byte-for-byte, including comments and a missing final newline. Ambiguous markers and failed reads or assembly leave the original allowlist intact. + +### Changed +- Required native macOS checks cover agent lifecycle, concurrent shutdown and linked settings. The native Windows CSO suite exercises repeated commands and exact high-ID lease recovery on NTFS. + ## [1.87.6.0] - 2026-09-18 **Review gates keep their proof.** diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index fc384d03b..a3f8bb966 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -286,8 +286,13 @@ runner, the Agent SDK runner, plus the codex and gemini runners) spawns its chil through `test/helpers/hermetic-env.ts`: an allowlist-scrubbed environment, a fresh seeded `CLAUDE_CONFIG_DIR`, a temp `GSTACK_HOME`, and `--strict-mcp-config`. Your operator `~/.claude` config, MCP servers (gbrain, Conductor), skills, `~/.gstack` -decision logs, and `CONDUCTOR_*` env never leak into the child, so local eval -signal matches CI instead of disagreeing for reasons unrelated to the code under +decision logs, and `CONDUCTOR_*` env never leak into the child. The `GITHUB_` +and `EVALS_` prefix rules preserve CI metadata but reject credential-shaped +names such as `GITHUB_TOKEN`, `GITHUB_PERSONAL_ACCESS_TOKEN`, and +`GITHUB_APP_PRIVATE_KEY`. Named provider auth, runner `extraAllow` entries, and +per-test overrides are deliberate exceptions; a name-based rule cannot identify +a secret assigned to an arbitrary metadata name. This keeps local eval signal +aligned with CI instead of disagreeing for reasons unrelated to the code under test. The hermetic `CLAUDE_CONFIG_DIR` seeds no skills by default; a PTY test that types a `/skill` slash command passes `seedSkills: true` to the PTY runner, which swaps in `hermeticSkillsConfigDir()` — a seeded skill registry that diff --git a/TODOS.md b/TODOS.md index 48e3758f3..4880439b9 100644 --- a/TODOS.md +++ b/TODOS.md @@ -249,9 +249,10 @@ global-path registration + re-point). Remaining: update the locale pin test. Filed via /ship review army (maintainability). **Priority:** P3. Effort S. - **Accepted threat-model notes (documented, no action planned):** - redact-prepush treats content pushed to ANY private remote as already-left - (accident-only threat model); a parcel-shaped twin within 400 chars can - suppress phone redaction (WARN-tier pattern, attacker-influence accepted); + redact-prepush's no-argv compatibility mode retains all-remotes exclusions; + installed hooks bind scans to the actual destination. A parcel-shaped twin + within 400 chars can suppress phone redaction (WARN-tier pattern, + attacker-influence accepted); codex-probe's 400-signature grep can misread a transient proxy 400 as MODEL_UNUSABLE (bounded by the 15-min negative-cache TTL). @@ -316,9 +317,11 @@ silent regression: by test/setup-playwright-best-effort.test.ts (fork-port Wave A). Still unpinned: `_clear_playwright_quarantine` (the P0 #2554 heal's shell half). Effort S. -- **redact-prepush `scanAddedLines` slicing** — the >1MiB catch-up-diff chunk - path (the reason the function exists) is unexercised; a regression - reintroduces blocking-while-unscanned. Effort S. +- **redact-prepush `scanAddedLines` slicing** — the >1MiB chunk path was + unexercised at v1.67. Installed-hook controls in + test/redact-prepush-target.test.ts now cover large clean diffs, seam + proximity/normalization, duplicate findings, and long-line refusal + (v1.88.1.0). - **supabase telemetry-ingest edge function** — zero tests; producer caps at 200 chars vs ingest's 500 (dead server cap); no column↔migration pin. - **gbrain-repo-policy-client** — no direct test file; the spawn-failed vs diff --git a/VERSION b/VERSION index c9885d180..b4a4a18ff 100644 --- a/VERSION +++ b/VERSION @@ -1 +1 @@ -1.87.6.0 +1.88.1.0 diff --git a/agents-digest/gstack-AGENTS.md b/agents-digest/gstack-AGENTS.md index 94572e83a..7293928f8 100644 --- a/agents-digest/gstack-AGENTS.md +++ b/agents-digest/gstack-AGENTS.md @@ -1,4 +1,4 @@ -# gstack digest v1.87.6.0 — regenerate/re-copy after upgrading gstack +# gstack digest v1.88.1.0 — regenerate/re-copy after upgrading gstack Behavioral rules from gstack (https://github.com/garrytan/gstack), compressed for agent hosts without a full skill install. The full skills add workflows, diff --git a/bin/gstack-artifacts-init b/bin/gstack-artifacts-init index 6ad2dcfb6..d2c1a8cf3 100755 --- a/bin/gstack-artifacts-init +++ b/bin/gstack-artifacts-init @@ -277,7 +277,13 @@ cat > "$GSTACK_HOME/.gitignore" <<'EOF' * EOF -cat > "$GSTACK_HOME/.brain-allowlist" <<'EOF' +ALLOWLIST="$GSTACK_HOME/.brain-allowlist" +ALLOWLIST_TMP=$(mktemp "$GSTACK_HOME/.brain-allowlist.XXXXXX") +ALLOWLIST_ASSEMBLED=$(mktemp "$GSTACK_HOME/.brain-allowlist.XXXXXX") +trap 'rm -f "$ALLOWLIST_TMP" "$ALLOWLIST_ASSEMBLED"' EXIT +ALLOWLIST_MARKER='# ---- USER ADDITIONS BELOW ---- (survives re-init; above is managed)' + +cat > "$ALLOWLIST_TMP" <<'EOF' # Canonical allowlist of paths that gstack-brain-sync will publish. # One glob per line. Anything not matching stays local. # Do not edit directly; managed by gstack-artifacts-init. User additions go @@ -325,6 +331,33 @@ transcripts/run-*/**/*.md # ---- USER ADDITIONS BELOW ---- (survives re-init; above is managed) EOF +if [ -s "$ALLOWLIST" ]; then + if marker_count=$(grep -Fxc "$ALLOWLIST_MARKER" "$ALLOWLIST"); then + : + else + grep_status=$? + if [ "$grep_status" -gt 1 ]; then + echo "gstack-artifacts-init: could not read $ALLOWLIST; refusing to replace it" >&2 + exit 1 + fi + marker_count=0 + fi + if [ "$marker_count" -ne 1 ]; then + if [ "$marker_count" -eq 0 ]; then reason="has no managed marker"; else reason="has multiple managed markers"; fi + echo "gstack-artifacts-init: $ALLOWLIST $reason; refusing to replace ambiguous user data" >&2 + exit 1 + fi + marker_line=$(grep -nFx "$ALLOWLIST_MARKER" "$ALLOWLIST" | cut -d: -f1) + suffix_start=$(head -n "$marker_line" "$ALLOWLIST" | wc -c) + cp -p "$ALLOWLIST" "$ALLOWLIST_ASSEMBLED" + : > "$ALLOWLIST_ASSEMBLED" + cat "$ALLOWLIST_TMP" >> "$ALLOWLIST_ASSEMBLED" + tail -c +"$((suffix_start + 1))" "$ALLOWLIST" >> "$ALLOWLIST_ASSEMBLED" +else + cat "$ALLOWLIST_TMP" > "$ALLOWLIST_ASSEMBLED" +fi +mv -f "$ALLOWLIST_ASSEMBLED" "$ALLOWLIST" + cat > "$GSTACK_HOME/.brain-privacy-map.json" <<'EOF' [ {"pattern": "projects/*/learnings.jsonl", "class": "artifact"}, diff --git a/bin/gstack-redact-prepush b/bin/gstack-redact-prepush index 3e678f3a3..8ca1fa0f5 100755 --- a/bin/gstack-redact-prepush +++ b/bin/gstack-redact-prepush @@ -29,12 +29,18 @@ import { spawnSync } from "child_process"; import * as fs from "fs"; import * as os from "os"; import * as path from "path"; -import { scan, type Finding } from "../lib/redact-engine"; +import { normalizeWithMap, scan, type Finding } from "../lib/redact-engine"; import { mkdirpSync } from "../lib/fs-utils"; const ZERO = /^0+$/; -// The canonical empty-tree object; diffing against it yields all content as added. -const EMPTY_TREE = "4b825dc642cb6eb9a060e54bf8d69288fbee4904"; +let emptyTree: string | undefined; +function emptyTreeOid(): string { + if (emptyTree) return emptyTree; + const oid = gitStrict(["hash-object", "-w", "-t", "tree", "--stdin"]).trim(); + if (!/^(?:[0-9a-f]{40}|[0-9a-f]{64})$/.test(oid)) throw new Error("git could not resolve the empty tree"); + emptyTree = oid; + return oid; +} /** * Permissive git for legitimately-fallible PROBES (symbolic-ref, rev-parse, @@ -81,46 +87,59 @@ function objectExists(sha: string): boolean { * scanned when later pushed to a PUBLIC remote — "already left this machine" * is not "already reached THIS remote". Git hands pre-push the push remote's * name as $1 (and its URL as $2); the installed hook wrapper forwards "$@". - * Fallbacks keep the historical all-remotes behavior when the name is - * unavailable (stdin/CLI invocation) or is not a configured remote (URL - * pushes have no remote-tracking namespace) — falling back scans LESS than - * the narrowed form would, but never less than the hook historically did. + * A URL push has no tracking namespace, even if origin happens to contain + * the same commits. Direct invocations without Git's argv keep the historical + * all-remotes behavior for compatibility. */ -let _remotesExclusion: string | undefined; -function remotesExclusion(): string { - if (_remotesExclusion === undefined) { - const name = process.argv[2]; - const configured = name - ? git(["remote"]).split("\n").map((s) => s.trim()).filter(Boolean).includes(name) - : false; - _remotesExclusion = configured ? `--remotes=${name}/*` : "--remotes"; - } - return _remotesExclusion; +type PushTarget = { kind: "remote"; name: string } | { kind: "url" } | { kind: "unknown" }; +let cachedPushTarget: PushTarget | undefined; +function pushTarget(): PushTarget { + if (cachedPushTarget) return cachedPushTarget; + const name = process.argv[2]; + const url = process.argv[3]; + if (!name && !url) return cachedPushTarget = { kind: "unknown" }; + if (!name || !url) return cachedPushTarget = { kind: "url" }; + const remotes = git(["remote"]).split("\n"); + if (!remotes.includes(name)) return cachedPushTarget = { kind: "url" }; + const pushUrls = git(["remote", "get-url", "--push", "--all", name]).trim().split("\n"); + const fetchUrls = git(["remote", "get-url", "--all", name]).trim().split("\n"); + return cachedPushTarget = pushUrls.includes(url) && fetchUrls[0] === url + ? { kind: "remote", name } + : { kind: "url" }; } -function defaultRemoteBranch(): string { - // origin/HEAD → origin/main, fall back to main/master. - const sym = git(["symbolic-ref", "refs/remotes/origin/HEAD"]).trim(); - if (sym) return sym.replace("refs/remotes/", ""); - for (const b of ["origin/main", "origin/master"]) { - if (git(["rev-parse", "--verify", b]).trim()) return b; +function remotesExclusionArgs(): string[] { + const target = pushTarget(); + if (target.kind === "remote") return [`--remotes=${target.name}/*`]; + return target.kind === "unknown" ? ["--remotes"] : []; +} + +function defaultRemoteBranch(): string | null { + const target = pushTarget(); + if (target.kind === "url") return null; + const remote = target.kind === "remote" ? target.name : "origin"; + const sym = git(["symbolic-ref", `refs/remotes/${remote}/HEAD`]).trim(); + if (sym.startsWith(`refs/remotes/${remote}/`) && git(["rev-parse", "--verify", `${sym}^{commit}`]).trim()) return sym; + for (const b of [`${remote}/main`, `${remote}/master`]) { + const qualified = `refs/remotes/${b}`; + if (git(["rev-parse", "--verify", `${qualified}^{commit}`]).trim()) return qualified; } - return "origin/main"; + return null; } /** - * Base commit for a push whose remote tip we cannot use directly, ordered from - * most precise to most conservative. Returns null when nothing can anchor the - * range, i.e. the whole history really is new content. + * Base commit for a new remote ref, ordered from most precise to most + * conservative. Returns null when nothing can anchor the range. */ function unknownRemoteTipBase(localSha: string): string | null { // 1. The common case: a merge-base with the remote's default branch. - const base = git(["merge-base", localSha, defaultRemoteBranch()]).trim(); + const defaultBranch = defaultRemoteBranch(); + const base = defaultBranch ? git(["merge-base", localSha, defaultBranch]).trim() : ""; if (base) return base; - // 2. No merge-base. defaultRemoteBranch() guessed a ref that does not exist - // (default branch named trunk/develop, origin/HEAD unset), or history is - // disjoint. Anything reachable from localSha but from NO remote-tracking + // 2. No merge-base. The target's default branch may be unavailable + // (named trunk/develop, remote/HEAD unset), or history may be disjoint. + // Anything reachable from localSha but from NO remote-tracking // branch is what this push actually adds; the parent of its oldest commit // is the real base. // @@ -133,11 +152,12 @@ function unknownRemoteTipBase(localSha: string): string | null { // NOTHING — "scans more, never less" inverted into "scans nothing". // // The exclusion is scoped to the PUSH TARGET's tracking refs (see - // remotesExclusion): content on some OTHER remote has left this machine, + // remotesExclusionArgs): content on some OTHER remote has left this machine, // but it has not reached the remote being pushed to — a secret that only // ever hit a private remote must still be scanned on its way to a public // one (S1). - const newCommits = git(["rev-list", "--reverse", localSha, "--not", remotesExclusion()]).trim(); + const exclusions = remotesExclusionArgs(); + const newCommits = git(["rev-list", "--reverse", localSha, ...(exclusions.length ? ["--not", ...exclusions] : [])]).trim(); if (newCommits) { const oldest = newCommits.split("\n")[0]; const parent = git(["rev-parse", "--verify", `${oldest}^`]).trim(); @@ -180,7 +200,7 @@ function unknownRemoteTipBase(localSha: string): string | null { * base excludes both. `rev-list --not --remotes=/*` is the * operation that does, and this file already reasons that way in * `unknownRemoteTipBase` step 2. The exclusion is scoped to the push target's - * tracking namespace (see remotesExclusion): the upstream commits a catch-up + * tracking namespace (see remotesExclusionArgs): the upstream commits a catch-up * merge brings in came from the SAME remote being pushed to, so scoping keeps * the #2592 fix intact while a secret known only to some OTHER (private) * remote is still scanned on its way to this one (S1). @@ -199,7 +219,7 @@ function addedLinesFromNewCommits(localSha: string, remoteSha: string): string | // direction. So it stays the base; `--remotes` only ADDS exclusions on top. if (ZERO.test(remoteSha) || !objectExists(remoteSha)) return null; - const narrowed = git(["rev-list", localSha, "--not", remoteSha, remotesExclusion()]).trim(); + const narrowed = git(["rev-list", localSha, "--not", remoteSha, ...remotesExclusionArgs()]).trim(); if (!narrowed) return null; // If excluding remote-tracking refs changes nothing, this push has no @@ -236,12 +256,8 @@ function addedLinesFor(localSha: string, remoteSha: string): string { let range: string; if (ZERO.test(remoteSha) || !objectExists(remoteSha)) { - // Either a new branch (zero remote sha), or the remote tip object is absent - // locally (shallow clone, force-push without a prior fetch, CI checkout) so - // remote..local cannot resolve. Both need a base derived locally; scan MORE - // rather than hard-blocking a legitimate push (adversarial review finding 8). - const base = unknownRemoteTipBase(localSha); - range = base ? `${base}..${localSha}` : `${EMPTY_TREE}..${localSha}`; + const base = ZERO.test(remoteSha) ? unknownRemoteTipBase(localSha) : null; + range = base ? `${base}..${localSha}` : `${emptyTreeOid()}..${localSha}`; } else { // Existing branch (incl. force-push): net new content remote..local. range = `${remoteSha}..${localSha}`; @@ -293,9 +309,12 @@ function collectAddedLines(diff: string): string { * DEFAULT_MAX_BYTES (1 MiB) so a slice never trips its oversize guard. */ const SCAN_CHUNK_BYTES = 768 * 1024; +const SCAN_OVERLAP_CHARS = 16 * 1024; +const MAX_SCAN_BYTES = 1024 * 1024; /** - * Scan added lines in line-aligned slices, unioning the findings. + * Scan complete-line core slices with bounded left and right context. Admit + * only findings whose original captured start lies inside that core. * * Why: the engine refuses input over its byte cap and fails closed, which is * right for one scan() call but wrong as a push policy — a feature branch @@ -306,11 +325,11 @@ const SCAN_CHUNK_BYTES = 768 * 1024; * people to reach for --no-verify, defeating the guardrail far more thoroughly * than a large diff does. * - * Slicing loses NO detection coverage, because every pattern is single-line: - * none in redact-patterns.ts carries the `m` or `s` flag, the - * BEGIN-PRIVATE-KEY patterns capture only the header line rather than the key - * body, and the engine itself iterates line by line. A line boundary therefore - * cannot bisect a detectable secret, so no inter-slice overlap is needed. + * A match may need context on either adjacent line (`nearWindow` reaches 300 + * normalized characters). The full core lines keep opaque spans intact; the + * engine's normalization map selects context in normalized units without + * changing the raw input. A partial context may create an artificial boundary, + * but its findings are never admitted as core findings. * * Fail-closed is preserved: a SINGLE line over the budget is still passed to * the engine intact, so a genuinely unscannable blob (minified bundle, @@ -322,26 +341,75 @@ const SCAN_CHUNK_BYTES = 768 * 1024; */ function scanAddedLines(added: string, opts: Parameters[1]): Finding[] { const findings: Finding[] = []; - let slice: string[] = []; - let sliceBytes = 0; + const cores: Array<{ start: number; end: number }> = []; + const lines = added.split("\n"); + let start = 0; + let end = 0; + let bytes = 0; + for (let i = 0; i < lines.length; i++) { + const line = lines[i]; + const newline = i < lines.length - 1 ? 1 : 0; + const lineBytes = Buffer.byteLength(line, "utf8") + newline; + if (bytes && bytes + lineBytes > SCAN_CHUNK_BYTES) { + cores.push({ start, end }); + start = end; + bytes = 0; + } + end += line.length + newline; + bytes += lineBytes; + if (lineBytes > SCAN_CHUNK_BYTES) { + cores.push({ start, end }); + start = end; + bytes = 0; + } + } + if (bytes) cores.push({ start, end }); - const flush = () => { - if (slice.length === 0) return; - findings.push(...scan(slice.join("\n"), opts).findings); - slice = []; - sliceBytes = 0; + const contextStart = (boundary: number): number => { + if (!boundary) return 0; + let length = Math.min(boundary, SCAN_OVERLAP_CHARS * 2); + while (true) { + const from = boundary - length; + const raw = added.slice(from, boundary); + if (Buffer.byteLength(raw, "utf8") > MAX_SCAN_BYTES) return from; + const { normalized, map } = normalizeWithMap(raw); + if (!from || normalized.length > SCAN_OVERLAP_CHARS) { + return from + (map[Math.max(0, normalized.length - SCAN_OVERLAP_CHARS - 1)] ?? 0); + } + length = Math.min(boundary, length * 2); + } }; - for (const line of added.split("\n")) { - // +1 for the newline that rejoins it. - const lineBytes = Buffer.byteLength(line, "utf8") + 1; - // Close the current slice BEFORE overflowing it. A single oversized line - // lands in a slice of its own and is handed to the engine as-is. - if (sliceBytes > 0 && sliceBytes + lineBytes > SCAN_CHUNK_BYTES) flush(); - slice.push(line); - sliceBytes += lineBytes; + const contextEnd = (boundary: number): number => { + if (boundary === added.length) return boundary; + let length = Math.min(added.length - boundary, SCAN_OVERLAP_CHARS * 2); + while (true) { + const to = boundary + length; + const raw = added.slice(boundary, to); + if (Buffer.byteLength(raw, "utf8") > MAX_SCAN_BYTES) return to; + const { normalized, map } = normalizeWithMap(raw); + if (to === added.length || normalized.length > SCAN_OVERLAP_CHARS) { + return boundary + (map[Math.min(normalized.length, SCAN_OVERLAP_CHARS + 1)] ?? length); + } + length = Math.min(added.length - boundary, length * 2); + } + }; + + for (const core of cores) { + const left = contextStart(core.start); + const text = added.slice(left, contextEnd(core.end)); + const result = scan(text, opts); + if (result.oversize) { + findings.push(...result.findings); + return findings; + } + const starts = [0]; + for (let i = 0; i < text.length; i++) if (text[i] === "\n") starts.push(i + 1); + for (const finding of result.findings) { + const offset = starts[finding.line - 1] + finding.col - 1 + left; + if (offset >= core.start && offset < core.end) findings.push(finding); + } } - flush(); return findings; } @@ -437,11 +505,8 @@ function main() { // detected, on a diff that was never read. Blocking is still right (fail // closed), but the reason must be the true one: a guardrail that cries wolf // is a guardrail that gets bypassed by reflex, which is worse than none. - // Seen live 2026-07-30: a diff of a few hundred bytes reported HIGH - // engine.input_too_large, because an unresolvable base branch made the hook - // fall back to EMPTY_TREE..local — i.e. the WHOLE repo (~7 MiB) as "added - // lines". The size the operator sees and the size the hook measures can - // therefore differ by four orders of magnitude. + // The computed range can be much larger than the visible tip diff, and a + // single over-cap line is deliberately withheld rather than partly scanned. const unscanned = allHigh.filter((f) => f.id.startsWith("engine.")); const secrets = allHigh.filter((f) => !f.id.startsWith("engine.")); @@ -466,9 +531,8 @@ function main() { process.stderr.write(` ${f.id}: ${f.description}\n`); } process.stderr.write( - "\nLikely cause: the base branch could not be resolved, so the whole repo was\n" + - "treated as added lines. Check `git rev-parse --abbrev-ref origin/HEAD` and\n" + - "`git merge-base HEAD origin/main`, then push again. Scan the diff yourself\n" + + "\nA long line, large first push, or unavailable remote tip may exceed the\n" + + "per-slice safety cap. Check the destination and range; scan the diff yourself\n" + "before bypassing: `git diff ..HEAD | grep -inE \'password|secret|token|api.?key\'`.\n", ); } diff --git a/bin/gstack-settings-hook b/bin/gstack-settings-hook index b755752f9..645616e61 100755 --- a/bin/gstack-settings-hook +++ b/bin/gstack-settings-hook @@ -110,6 +110,10 @@ function gsMain(fn) { try { fn(); } catch (e) { + if (e && e.gstackUnreadableSettings === true) { + process.stderr.write("gstack-settings-hook: " + e.message + " -- refusing to mutate\n"); + process.exit(3); + } process.stderr.write("gstack-settings-hook: internal error (" + (e && e.message) + ") -- refusing to mutate\n"); process.exit(4); } @@ -161,6 +165,49 @@ function gsWinPath(p) { } return p; } +function gsResolveSettingsPath(input) { + var fs = require("fs"); + var path = require("path"); + var absolute = path.resolve(gsWinPath(input)); + var resolved; + try { resolved = fs.realpathSync(absolute); } + catch (e) { + if (e && (e.code === "EACCES" || e.code === "EPERM")) { + throw Object.assign(new Error("cannot read " + absolute + " (" + e.code + ")"), { gstackUnreadableSettings: true }); + } + if (!e || e.code !== "ENOENT") throw e; + try { + if (fs.lstatSync(absolute).isSymbolicLink()) throw new Error("settings link has no target"); + } catch (missing) { + if (!missing || missing.code !== "ENOENT") throw missing; + } + resolved = path.join(gsResolveSettingsPath(path.dirname(absolute)), path.basename(absolute)); + } + return process.platform === "win32" ? resolved.replace(/\\/g, "/") : resolved; +} +function gsAssertSettingsTarget(settingsPath) { + if (gsResolveSettingsPath(process.env.GSTACK_SETTINGS_INPUT) !== settingsPath) { + throw new Error("settings target changed while waiting or writing"); + } +} +function gsSettingsIdentity(settingsPath) { + var fs = require("fs"); + try { + var stat = fs.lstatSync(settingsPath, { bigint: true }); + if (!stat.isFile()) throw new Error("settings target is not a regular file"); + return [stat.dev, stat.ino, stat.size, stat.mode, stat.mtimeNs, stat.ctimeNs].join(":"); + } catch (e) { + if (e && e.code === "ENOENT") return null; + throw e; + } +} +var gsLoadedIdentity; +function gsAssertSettingsUnchanged(settingsPath) { + gsAssertSettingsTarget(settingsPath); + if (gsSettingsIdentity(settingsPath) !== gsLoadedIdentity) { + throw new Error("settings changed during mutation"); + } +} function gsIsAlive(cmd) { var fs = require("fs"); var p = gsWinPath(gsStripWrap(cmd)); @@ -214,6 +261,8 @@ function gsRotateBackups(settingsPath, keep) { } function gsLoadSettings(path) { var fs = require("fs"); + gsAssertSettingsTarget(path); + gsLoadedIdentity = gsSettingsIdentity(path); var raw = null; try { raw = fs.readFileSync(path, "utf8"); } catch (e) { @@ -231,6 +280,7 @@ function gsWriteIfChanged(path, beforeText, settings, existed) { var fs = require("fs"); var afterText = JSON.stringify(settings, null, 2); if (afterText === beforeText) return false; + gsAssertSettingsUnchanged(path); // Preserve the live file mode across the tmp+rename (settings.json can // carry API keys in its env block -- a user-tightened 0600 must never be // silently broadened to the default 0644). Fresh files start 0600. @@ -242,13 +292,24 @@ function gsWriteIfChanged(path, beforeText, settings, existed) { gsRotateBackups(path, 10); } var tmp = process.env.GSTACK_TMP_PATH; - fs.writeFileSync(tmp, afterText + "\n"); - try { fs.chmodSync(tmp, mode); } catch (e) {} - fs.renameSync(tmp, path); + fs.writeFileSync(tmp, afterText + "\n", { mode: mode, flag: "wx" }); + try { + fs.chmodSync(tmp, mode); + gsAssertSettingsUnchanged(path); + fs.renameSync(tmp, path); + } finally { + try { fs.unlinkSync(tmp); } catch (e) { if (e.code !== "ENOENT") throw e; } + } return true; } ' +GSTACK_SETTINGS_INPUT="$SETTINGS_FILE" +export GSTACK_SETTINGS_INPUT +SETTINGS_FILE=$(bun -e "$_HOOK_JS_PRELUDE"'gsMain(function () { + process.stdout.write(gsResolveSettingsPath(process.env.GSTACK_SETTINGS_INPUT)); +});') || exit $? + # ─── Mutation lock ──────────────────────────────────────────────────── # Accepted tradeoffs (adversarial-reviewed): (1) the lock serializes gstack # writers only -- Claude Code rewrites settings.json without honoring it, so a @@ -805,6 +866,7 @@ case "$ACTION" in ;; rollback) + _acquire_lock || exit 1 if [ ! -f "$SETTINGS_FILE.bak-latest" ]; then echo "rollback: no backup pointer at $SETTINGS_FILE.bak-latest" >&2 exit 1 @@ -831,10 +893,23 @@ case "$ACTION" in echo "rollback: pointer references missing backup $LATEST" >&2 exit 1 fi - _acquire_lock || exit 1 - _RB_TMP="$SETTINGS_FILE.tmp.$$.$RANDOM" - cp "$LATEST" "$_RB_TMP" - mv "$_RB_TMP" "$SETTINGS_FILE" + _mutation_env + GSTACK_SETTINGS_PATH="$SETTINGS_FILE" GSTACK_RESTORE_PATH="$LATEST" \ + bun -e "$_HOOK_JS_PRELUDE"'gsMain(function () { + var fs = require("fs"); + var target = process.env.GSTACK_SETTINGS_PATH; + var backup = process.env.GSTACK_RESTORE_PATH; + var tmp = process.env.GSTACK_TMP_PATH; + gsAssertSettingsTarget(target); + gsLoadedIdentity = gsSettingsIdentity(target); + try { + fs.copyFileSync(backup, tmp, fs.constants.COPYFILE_EXCL); + gsAssertSettingsUnchanged(target); + fs.renameSync(tmp, target); + } finally { + try { fs.unlinkSync(tmp); } catch (e) { if (e.code !== "ENOENT") throw e; } + } + });' echo "OK: restored $SETTINGS_FILE from $LATEST" ;; diff --git a/browse/src/cli.ts b/browse/src/cli.ts index e36728189..7c32c7fd6 100644 --- a/browse/src/cli.ts +++ b/browse/src/cli.ts @@ -527,6 +527,7 @@ async function startServer(extraEnv?: Record): Promise): Promise): Promise readPidStartTime(pid) === startTime && readPidCmdline(pid).split(/\s+/).includes(SERVER_SCRIPT); + if (stillOurs()) { + safeKill(pid, 'SIGTERM'); + const deadline = Date.now() + 500; + while (Date.now() < deadline && stillOurs()) await Bun.sleep(50); + if (stillOurs()) safeKill(pid, 'SIGKILL'); + } + } + // Server didn't start in time — check the on-disk startup error log. // Both platforms now spawn with stdio: 'ignore', so the server writes // errors to disk for the CLI to read (see server.ts start().catch). @@ -1664,6 +1678,7 @@ Refs: After 'snapshot', use @e1, @e2... as selectors: const newPid = spawnTerminalAgent({ stateFile: config.stateFile, serverPort: newState.port, + ownerPid: newState.pid, cwd: config.projectDir, }); if (newPid) { @@ -1756,6 +1771,7 @@ Refs: After 'snapshot', use @e1, @e2... as selectors: spawnTerminalAgent({ stateFile: config.stateFile, serverPort: respawned.port, + ownerPid: respawned.pid, cwd: config.projectDir, }); } catch (err: any) { diff --git a/browse/src/server.ts b/browse/src/server.ts index e26450532..4176a6716 100644 --- a/browse/src/server.ts +++ b/browse/src/server.ts @@ -51,7 +51,7 @@ import { safeUnlink, safeUnlinkQuiet, safeKill } from './error-handling'; import { findAvailablePort, formatExplicitPortUnavailableError, formatRandomPortUnavailableError, } from './port-allocator'; -import { readAgentRecord, killAgentByRecord, agentRecordPath, spawnTerminalAgent } from './terminal-agent-control'; +import { acquireAgentStateLock, readAgentRecord, clearAgentRecord, isOurAgent, isAgentRecordLive, isAgentRecordGone, stopAgentByRecord, spawnTerminalAgent } from './terminal-agent-control'; import { isProcessAlive } from './error-handling'; import { sanitizeBody, stripLoneSurrogateEscapes, stripLoneSurrogates, sanitizeReplacer } from './sanitize'; import { startSocksBridge, testUpstream, type BridgeHandle } from './socks-bridge'; @@ -75,6 +75,18 @@ import * as net from 'net'; import * as path from 'path'; import * as crypto from 'crypto'; +const SERVER_INSTANCE_ID = crypto.randomUUID(); + +function removeOwnedDaemonStateQuiet(): void { + try { + const release = acquireAgentStateLock(path.dirname(config.stateFile), 0); + try { + const state = JSON.parse(fs.readFileSync(config.stateFile, 'utf8')); + if (state.pid === process.pid && state.instanceId === SERVER_INSTANCE_ID) safeUnlinkQuiet(config.stateFile); + } finally { release(); } + } catch {} +} + // ─── Unicode Sanitization ─────────────────────────────────────── // Unpaired UTF-16 surrogate halves (\uD800–\uDFFF) in page DOM text, OCR // output, and other CDP-sourced strings are rejected by JSON consumers @@ -466,11 +478,15 @@ async function startTunnel(opts: { console.log(`[browse] Tunnel listener bound on 127.0.0.1:${tunnelPort}, ngrok → ${tunnelUrl}`); // Update state file - const stateContent = JSON.parse(fs.readFileSync(config.stateFile, 'utf-8')); - stateContent.tunnel = { url: tunnelUrl, domain: domain || null, startedAt: new Date().toISOString() }; - const tmpState = tmpStatePath(); - fs.writeFileSync(tmpState, JSON.stringify(stateContent, null, 2), { mode: 0o600 }); - fs.renameSync(tmpState, config.stateFile); + const releaseStateLock = acquireAgentStateLock(config.stateDir); + try { + const stateContent = JSON.parse(fs.readFileSync(config.stateFile, 'utf-8')); + if (stateContent.pid !== process.pid || stateContent.instanceId !== SERVER_INSTANCE_ID) throw new Error('daemon state was replaced'); + stateContent.tunnel = { url: tunnelUrl, domain: domain || null, startedAt: new Date().toISOString() }; + const tmpState = tmpStatePath(); + fs.writeFileSync(tmpState, JSON.stringify(stateContent, null, 2), { mode: 0o600 }); + fs.renameSync(tmpState, config.stateFile); + } finally { releaseStateLock(); } return { ok: true, url: tunnelUrl! }; } catch (err: any) { @@ -735,6 +751,7 @@ const idleCheckInterval = setInterval(idleCheckTick, 60_000); // Production code must never import this — see `idle timer + onDisconnect // dual-instance fix` describe block for usage. export const __testInternals__ = { + serverInstanceId: SERVER_INSTANCE_ID, idleCheckTick, // Watchdog seams (watchdog.test.ts): drive the 15s poll against an // arbitrary (dead) PID, trigger the handoff-promotion suppression exactly @@ -1432,9 +1449,7 @@ if (import.meta.main) { // Windows: taskkill /F bypasses SIGTERM, but 'exit' fires for some shutdown paths. // Defense-in-depth — primary cleanup is the CLI's stale-state detection via health check. if (process.platform === 'win32') { - process.on('exit', () => { - safeUnlinkQuiet(config.stateFile); - }); + process.on('exit', removeOwnedDaemonStateQuiet); } } @@ -1452,6 +1467,7 @@ function emergencyCleanup() { if (fs.existsSync(config.stateFile)) { const raw = fs.readFileSync(config.stateFile, 'utf-8'); const state = JSON.parse(raw); + if (state.pid !== process.pid || state.instanceId !== SERVER_INSTANCE_ID) return; if (state.xvfbPid && state.xvfbStartTime) { // Lazy import — emergencyCleanup may run on platforms where // ./xvfb's Linux-specific helpers fail to load. Best effort. @@ -1472,7 +1488,7 @@ function emergencyCleanup() { if (activeBrowserManager.getConnectionMode() === 'headed' || process.env.BROWSE_HEADED === '1') { cleanSingletonLocks(resolveChromiumProfile()); } - safeUnlinkQuiet(config.stateFile); + removeOwnedDaemonStateQuiet(); } // Same import.meta.main gate as SIGINT/SIGTERM — embedders register their // own crash handlers. @@ -1582,6 +1598,7 @@ export function buildFetchHandler(cfg: ServerConfig): ServerHandle { AGENT_WATCHDOG_TICK_MS * (RESPAWN_GUARD_MAX + 2), ); let agentRespawnGuardTripped = false; + let consecutiveSpawnFailures = 0; if (ownsTerminalAgent) { agentWatchdogInterval = setInterval(() => { @@ -1594,7 +1611,7 @@ export function buildFetchHandler(cfg: ServerConfig): ServerHandle { // intentionally fall through here — split-brain is worse than // unresponsiveness, and slow recovery is handled by the user via // restart. - if (record && isProcessAlive(record.pid)) return; + if (record && !isAgentRecordGone(record)) return; // Either no record (never spawned, or cleaned up after crash) or // PID is dead. Try to respawn. const now = Date.now(); @@ -1617,13 +1634,20 @@ export function buildFetchHandler(cfg: ServerConfig): ServerHandle { cwd: cfg.config.projectDir, }); if (pid) { + consecutiveSpawnFailures = 0; console.log(`[browse] terminal-agent respawned by watchdog (PID: ${pid})`); } else { + consecutiveSpawnFailures++; console.warn('[browse] terminal-agent respawn skipped — script not found on disk'); } } catch (err: any) { + consecutiveSpawnFailures++; console.warn('[browse] terminal-agent respawn failed:', err?.message || err); } + if (consecutiveSpawnFailures >= RESPAWN_GUARD_MAX) { + agentRespawnGuardTripped = true; + console.error('[browse] terminal-agent respawn guard tripped after repeated failed starts — manual restart required'); + } }, AGENT_WATCHDOG_TICK_MS); // Detach the watchdog timer from Node's event-loop ref count so a // healthy idle process can still exit cleanly if everything else is @@ -1654,23 +1678,44 @@ export function buildFetchHandler(cfg: ServerConfig): ServerHandle { // State and terminal files belong to this instance, including embedders // whose cfg differs from the standalone daemon's module-level config. const config = cfg.config; + const stateOwner = (() => { + try { return JSON.parse(fs.readFileSync(config.stateFile, 'utf8')); } catch { return null; } + })(); + const foreignState = Number.isSafeInteger(stateOwner?.pid) && stateOwner.pid > 0 + && (stateOwner.pid !== process.pid || (stateOwner.instanceId && stateOwner.instanceId !== SERVER_INSTANCE_ID)); console.log('[browse] Shutting down...'); - if (ownsTerminalAgent) { + if (ownsTerminalAgent && !foreignState) { // Identity-based kill (v1.44+). Replaces the v1.43- `pkill -f // terminal-agent\.ts` regex teardown which matched sibling gstack // sessions on the same host. Only the PID recorded in // `/terminal-agent-pid` by THIS daemon's agent is signaled. try { const stateDir = path.dirname(config.stateFile); - const record = readAgentRecord(stateDir); - if (record) killAgentByRecord(record, 'SIGTERM'); + const releaseAgentLock = acquireAgentStateLock(stateDir); + try { + let currentState: { pid?: number; instanceId?: string } | null = null; + try { currentState = JSON.parse(fs.readFileSync(config.stateFile, 'utf8')); } catch {} + if (currentState?.pid && (currentState.pid !== process.pid + || (currentState.instanceId && currentState.instanceId !== SERVER_INSTANCE_ID))) { + console.warn('[browse] terminal-agent state now belongs to a successor; retaining its files'); + } else { + const record = readAgentRecord(stateDir); + const agentStopped = !record || (record.pid !== 0 + && (!isAgentRecordLive(record) || (isOurAgent(record, process.pid) && stopAgentByRecord(record)))); + const current = readAgentRecord(stateDir); + if (agentStopped && (!record || (current?.pid === record.pid && current.gen === record.gen))) { + safeUnlinkQuiet(path.join(stateDir, 'terminal-port')); + safeUnlinkQuiet(path.join(stateDir, 'terminal-internal-token')); + if (record) clearAgentRecord(stateDir, record); + } else if (!agentStopped) { + console.warn('[browse] terminal-agent identity or exit could not be confirmed; retaining its record'); + } + } + } finally { releaseAgentLock(); } } catch (err: any) { - console.warn('[browse] Failed to kill terminal-agent:', err.message); + console.warn('[browse] Failed to stop terminal-agent; retaining its state:', err.message); } - safeUnlinkQuiet(path.join(path.dirname(config.stateFile), 'terminal-port')); - safeUnlinkQuiet(path.join(path.dirname(config.stateFile), 'terminal-internal-token')); - safeUnlinkQuiet(agentRecordPath(path.dirname(config.stateFile))); } try { detachSession(); } catch (err: any) { console.warn('[browse] Failed to detach CDP session:', err.message); @@ -1712,7 +1757,17 @@ export function buildFetchHandler(cfg: ServerConfig): ServerHandle { if (cfgBrowserManager.getConnectionMode() === 'headed') { cleanSingletonLocks(resolveChromiumProfile()); } - safeUnlinkQuiet(config.stateFile); + if (!foreignState) { + try { + const releaseStateLock = acquireAgentStateLock(path.dirname(config.stateFile)); + try { + const currentState = JSON.parse(fs.readFileSync(config.stateFile, 'utf8')); + if (currentState.pid === process.pid && currentState.instanceId === SERVER_INSTANCE_ID) safeUnlinkQuiet(config.stateFile); + } finally { releaseStateLock(); } + } catch (err: any) { + if (fs.existsSync(config.stateFile)) console.warn('[browse] Daemon state cleanup could not confirm ownership:', err?.message || err); + } + } process.exit(exitCode); } @@ -3182,6 +3237,7 @@ export async function start() { // Write state file (atomic: write .tmp then rename) const state: Record = { pid: process.pid, + instanceId: SERVER_INSTANCE_ID, port, token: envCfg.authToken, startedAt: new Date().toISOString(), @@ -3206,7 +3262,28 @@ export async function start() { }; const tmpFile = tmpStatePath(); fs.writeFileSync(tmpFile, JSON.stringify(state, null, 2), { mode: 0o600 }); - fs.renameSync(tmpFile, config.stateFile); + try { + const releaseStateLock = acquireAgentStateLock(config.stateDir); + try { fs.renameSync(tmpFile, config.stateFile); } finally { releaseStateLock(); } + } catch (err) { + safeUnlinkQuiet(tmpFile); + throw err; + } + + const stateWatchMs = parseInt(process.env.GSTACK_STATE_WATCH_MS || '60000', 10); + if (stateWatchMs > 0) { + let missed = 0; + const stateWatch = setInterval(() => { + let owner: { pid?: number; instanceId?: string } | null = null; + try { owner = JSON.parse(fs.readFileSync(config.stateFile, 'utf8')); } catch {} + if (owner?.pid === process.pid && owner.instanceId === SERVER_INSTANCE_ID) { missed = 0; return; } + if (++missed < 2) return; + clearInterval(stateWatch); + console.warn('[browse] daemon state is no longer reachable; shutting down this instance'); + handle.shutdown(); + }, stateWatchMs); + (stateWatch as any).unref?.(); + } browserManager.serverPort = port; @@ -3338,11 +3415,15 @@ export async function start() { tunnelActive = true; const tunnelPort = boundTunnel.port; console.log(`[browse] Tunnel listener bound (local-only test mode) on 127.0.0.1:${tunnelPort}`); - const stateContent = JSON.parse(fs.readFileSync(config.stateFile, 'utf-8')); - stateContent.tunnelLocalPort = tunnelPort; - const tmpState = tmpStatePath(); - fs.writeFileSync(tmpState, JSON.stringify(stateContent, null, 2), { mode: 0o600 }); - fs.renameSync(tmpState, config.stateFile); + const releaseStateLock = acquireAgentStateLock(config.stateDir); + try { + const stateContent = JSON.parse(fs.readFileSync(config.stateFile, 'utf-8')); + if (stateContent.pid !== process.pid || stateContent.instanceId !== SERVER_INSTANCE_ID) throw new Error('daemon state was replaced'); + stateContent.tunnelLocalPort = tunnelPort; + const tmpState = tmpStatePath(); + fs.writeFileSync(tmpState, JSON.stringify(stateContent, null, 2), { mode: 0o600 }); + fs.renameSync(tmpState, config.stateFile); + } finally { releaseStateLock(); } } catch (err: any) { console.error(`[browse] BROWSE_TUNNEL_LOCAL_ONLY=1 listener bind failed: ${err.message}`); } diff --git a/browse/src/terminal-agent-control.ts b/browse/src/terminal-agent-control.ts index 786d5a05b..c5b70b96f 100644 --- a/browse/src/terminal-agent-control.ts +++ b/browse/src/terminal-agent-control.ts @@ -16,9 +16,57 @@ */ import * as fs from 'fs'; import * as path from 'path'; -import { safeUnlink, safeKill, isProcessAlive } from './error-handling'; +import * as crypto from 'crypto'; +import { spawnSync } from 'child_process'; +import { safeUnlink, isProcessAlive } from './error-handling'; import { restrictFilePermissions, mkdirSecure } from './file-permissions'; import { atomicWriteSync } from '../../lib/fs-atomic'; +import { readPidCmdline, readPidStartTime } from './xvfb'; + +function agentProcessInfo(pid: number): { startTime: string; commandLine: string } { + if (!Number.isSafeInteger(pid) || pid <= 0) return { startTime: '', commandLine: '' }; + if (process.platform !== 'win32') return { startTime: readPidStartTime(pid), commandLine: readPidCmdline(pid) }; + const script = `(Get-CimInstance Win32_Process -Filter 'ProcessId = ${pid}' | Select-Object CreationDate,CommandLine | ConvertTo-Json -Compress)`; + try { + const result = spawnSync('powershell.exe', ['-NoProfile', '-NonInteractive', '-Command', script], { encoding: 'utf8', windowsHide: true, timeout: 2000 }); + if (result.status !== 0 || !result.stdout) return { startTime: '', commandLine: '' }; + const processInfo = JSON.parse(result.stdout); + return { startTime: processInfo?.CreationDate || '', commandLine: processInfo?.CommandLine || '' }; + } catch { return { startTime: '', commandLine: '' }; } +} + +export function readAgentStartTime(pid: number): string { + return agentProcessInfo(pid).startTime; +} + +const pendingAgentExits = new Set(); + +export function acquireAgentStateLock(stateDir: string, waitMs = 5000): () => void { + mkdirSecure(stateDir); + const lockPath = path.join(stateDir, 'terminal-agent-pid.lock'); + const deadline = Date.now() + waitMs; + let fd: number; + while (true) { + try { + fd = fs.openSync(lockPath, 'wx', 0o600); + break; + } catch (err: any) { + if (err?.code !== 'EEXIST' || Date.now() >= deadline) { + throw new Error(`terminal-agent state lock unavailable at ${lockPath}: ${err?.code || err}; inspect the owning process before manual recovery`); + } + Atomics.wait(new Int32Array(new SharedArrayBuffer(4)), 0, 0, 25); + } + } + const owned = fs.fstatSync(fd, { bigint: true }); + return () => { + try { + const current = fs.statSync(lockPath, { bigint: true }); + if (current.dev === owned.dev && current.ino === owned.ino) fs.unlinkSync(lockPath); + } catch (err: any) { + if (err?.code !== 'ENOENT') throw err; + } finally { fs.closeSync(fd); } + }; +} /** * Locate the terminal-agent script on disk. In dev (cli.ts running via @@ -41,11 +89,8 @@ export function resolveTerminalAgentScript(searchHints: { metaDir?: string; exec } /** - * Spawn a fresh terminal-agent as a detached child. Handles the standard - * three steps: kill any prior agent recorded at `/terminal-agent-pid`, - * clear the stale record, then `Bun.spawn(['bun', 'run', script], ...)` with - * env wiring. Returns the PID of the new agent on success, null when the - * agent script can't be located. + * Spawn an owned terminal-agent. A prior record is retained until its exact + * process exits, and the new generation is recorded before it may bind. * * Used by both the CLI cold-start path (cli.ts) and the v1.44 watchdog in * server.ts. Centralizing here removes a copy-paste between them and means @@ -62,31 +107,82 @@ export function spawnTerminalAgent(opts: { /** Override script lookup for tests. */ scriptPath?: string; }): number | null { + if (!Number.isSafeInteger(opts.ownerPid) || opts.ownerPid <= 0) throw new Error('terminal-agent requires a daemon owner PID'); const stateDir = path.dirname(opts.stateFile); - const prior = readAgentRecord(stateDir); - if (prior) { - killAgentByRecord(prior, 'SIGTERM'); - clearAgentRecord(stateDir); - } const script = opts.scriptPath || resolveTerminalAgentScript(); if (!script || !fs.existsSync(script)) return null; - const proc = (Bun as any).spawn(['bun', 'run', script], { - cwd: opts.cwd || process.cwd(), - env: { - ...process.env, - BROWSE_STATE_FILE: opts.stateFile, - BROWSE_SERVER_PORT: String(opts.serverPort), - BROWSE_OWNER_PID: String(opts.ownerPid), - ...(opts.extraEnv || {}), - }, - stdio: ['ignore', 'ignore', 'ignore'], - // Explicit for the Node fallback path (dist/bun-polyfill.cjs), where the - // host default is the opposite of Bun's. A visible console window on every - // watchdog respawn is the symptom when this is missing. - windowsHide: true, - }); - proc.unref?.(); - return proc.pid ?? null; + const release = acquireAgentStateLock(stateDir); + try { + const prior = readAgentRecord(stateDir); + if (prior) { + if (prior.pid === 0) { + console.warn('[browse] terminal-agent startup or failed exit remains unconfirmed; retaining its reservation'); + return null; + } + if (isAgentRecordLive(prior) && !stopAgentByRecord(prior)) { + console.warn(`[browse] terminal-agent PID ${prior.pid} is still running or its identity cannot be confirmed; refusing a second agent`); + return null; + } + clearAgentRecord(stateDir, prior); + safeUnlink(path.join(stateDir, 'terminal-port')); + safeUnlink(path.join(stateDir, 'terminal-internal-token')); + } + const ownerStartTime = readAgentStartTime(opts.ownerPid); + if (!ownerStartTime) throw new Error('terminal-agent owner identity is unavailable'); + const gen = crypto.randomBytes(16).toString('base64url'); + const reservation: AgentRecord = { pid: 0, gen, startedAt: Date.now(), ownerPid: opts.ownerPid, ownerStartTime }; + writeAgentRecord(stateDir, reservation); + let proc: any; + try { + proc = (Bun as any).spawn(['bun', 'run', script, `--agent-gen=${gen}`], { + cwd: opts.cwd || process.cwd(), + env: { + ...process.env, + ...(opts.extraEnv || {}), + BROWSE_STATE_FILE: opts.stateFile, + BROWSE_SERVER_PORT: String(opts.serverPort), + BROWSE_OWNER_PID: String(opts.ownerPid), + BROWSE_OWNER_START_TIME: ownerStartTime, + BROWSE_AGENT_GEN: gen, + }, + stdio: ['ignore', 'ignore', 'ignore'], + windowsHide: true, + }); + } catch (err) { + clearAgentRecord(stateDir, reservation); + throw err; + } + const retainUntilExit = () => { + pendingAgentExits.add(proc); + proc.exited?.then(() => { + try { + const releasePending = acquireAgentStateLock(stateDir); + try { clearAgentRecord(stateDir, reservation); } finally { releasePending(); } + } catch (err) { console.warn('[browse] terminal-agent pending exit cleanup failed:', err); } + pendingAgentExits.delete(proc); + }, (err: unknown) => console.warn('[browse] terminal-agent exit remains unconfirmed:', err)); + }; + const pid = proc.pid; + const startTime = pid ? readAgentStartTime(pid) : ''; + if (!pid || !startTime) { + try { proc.kill('SIGTERM'); } catch {} + retainUntilExit(); + throw new Error('terminal-agent process identity is unavailable'); + } + const record: AgentRecord = { pid, gen, startedAt: Date.now(), startTime, ownerPid: opts.ownerPid, ownerStartTime }; + try { + writeAgentRecord(stateDir, record); + } catch (err) { + if (!stopAgentByRecord(record)) { + retainUntilExit(); + throw new Error(`terminal-agent record update failed and child ${pid} exit is unconfirmed: ${err}`); + } + clearAgentRecord(stateDir, reservation); + throw err; + } + proc.unref?.(); + return pid; + } finally { release(); } } export interface AgentRecord { @@ -95,6 +191,9 @@ export interface AgentRecord { gen: string; /** ms since epoch. Reserved for future PID-reuse guards. */ startedAt: number; + startTime?: string; + ownerPid?: number; + ownerStartTime?: string; } export function agentRecordPath(stateDir: string): string { @@ -124,27 +223,79 @@ export function writeAgentRecord(stateDir: string, record: AgentRecord): void { restrictFilePermissions(target); } -export function clearAgentRecord(stateDir: string): void { +export function clearAgentRecord(stateDir: string, expected?: AgentRecord): void { + if (expected) { + const current = readAgentRecord(stateDir); + if (!current || current.pid !== expected.pid || current.gen !== expected.gen) return; + } safeUnlink(agentRecordPath(stateDir)); } +export function isAgentRecordLive(record: AgentRecord): boolean { + return Number.isSafeInteger(record.pid) && record.pid > 0 && isProcessAlive(record.pid); +} + +function agentStatus(record: AgentRecord, ownerPid?: number): 'owned' | 'gone' | 'unknown' { + if (!isAgentRecordLive(record)) return 'gone'; + if (!record.startTime || !record.ownerPid || !record.ownerStartTime) return 'unknown'; + if (ownerPid !== undefined && (record.ownerPid !== ownerPid || record.ownerStartTime !== readAgentStartTime(ownerPid))) return 'unknown'; + const actual = agentProcessInfo(record.pid); + if (!actual.startTime) return isAgentRecordLive(record) ? 'unknown' : 'gone'; + if (actual.startTime !== record.startTime) return 'gone'; + try { + let state: string | undefined; + if (process.platform === 'linux') { + state = fs.readFileSync(`/proc/${record.pid}/stat`, 'utf8').match(/^\d+ \(.*\) ([A-Z])/u)?.[1]; + } else if (process.platform === 'darwin') { + const result = spawnSync('ps', ['-p', String(record.pid), '-o', 'stat='], { encoding: 'utf8', windowsHide: true, timeout: 2000 }); + if (result.status === 0) state = result.stdout?.trim()?.[0]; + } + if (state === 'Z') return 'gone'; + } catch {} + if (!isAgentRecordLive(record)) return 'gone'; + return actual.commandLine.split(/\s+/).some(arg => arg.replace(/^['"]|['"]$/g, '') === `--agent-gen=${record.gen}`) + ? 'owned' : 'unknown'; +} + +export function isOurAgent(record: AgentRecord, ownerPid?: number): boolean { + return agentStatus(record, ownerPid) === 'owned'; +} + +export function isAgentRecordGone(record: AgentRecord): boolean { + return agentStatus(record) === 'gone'; +} + /** * Kill the agent identified by `record`. Signal defaults to SIGTERM (give * the agent a chance to run its own SIGTERM cleanup). Returns true if a - * signal was actually sent to a live PID; false if the PID was already - * dead (no-op). Never throws — ESRCH is swallowed by safeKill. - * - * Validates liveness BEFORE signaling so a PID-reuse race (the recorded - * PID was reaped and a brand-new unrelated process now holds it) can't - * cause us to kill the wrong process. This is a best-effort defense: - * Linux/macOS don't expose process-start-time cheaply, and the gap - * between record-write and watchdog-tick is small (60s max). + * signal reached an exact-generation live process, false otherwise. */ export function killAgentByRecord( record: AgentRecord, signal: NodeJS.Signals = 'SIGTERM', ): boolean { - if (!isProcessAlive(record.pid)) return false; - safeKill(record.pid, signal); - return true; + if (!isOurAgent(record)) return false; + try { process.kill(record.pid, signal); return true; } catch { return false; } +} + +export function stopAgentByRecord(record: AgentRecord, graceMs = 1000): boolean { + const initial = agentStatus(record); + if (initial === 'gone') return true; + if (initial !== 'owned') return false; + const waitForExit = (ms: number) => { + const deadline = Date.now() + ms; + while (Date.now() < deadline) { + const status = agentStatus(record); + if (status === 'gone') return true; + if (status === 'unknown') return false; + Atomics.wait(new Int32Array(new SharedArrayBuffer(4)), 0, 0, 25); + } + return agentStatus(record) === 'gone'; + }; + if (!killAgentByRecord(record, 'SIGTERM')) return agentStatus(record) === 'gone'; + if (waitForExit(graceMs)) return true; + const afterGrace = agentStatus(record); + if (afterGrace !== 'owned') return afterGrace === 'gone'; + if (!killAgentByRecord(record, 'SIGKILL')) return agentStatus(record) === 'gone'; + return waitForExit(graceMs); } diff --git a/browse/src/terminal-agent.ts b/browse/src/terminal-agent.ts index 33d0c09f1..3f82bb332 100644 --- a/browse/src/terminal-agent.ts +++ b/browse/src/terminal-agent.ts @@ -26,7 +26,7 @@ import * as crypto from 'crypto'; import { writeSecureFile, restrictFilePermissions, mkdirSecure } from './file-permissions'; import { atomicWriteSync, atomicWriteQuiet } from '../../lib/fs-atomic'; import { safeUnlink } from './error-handling'; -import { writeAgentRecord, clearAgentRecord } from './terminal-agent-control'; +import { writeAgentRecord, readAgentRecord, clearAgentRecord, readAgentStartTime, acquireAgentStateLock } from './terminal-agent-control'; import { findAvailablePort } from './port-allocator'; import { extractPtyCookie } from './pty-session-cookie'; import { @@ -38,6 +38,7 @@ const STATE_FILE = process.env.BROWSE_STATE_FILE || path.join(process.env.HOME | const PORT_FILE = path.join(path.dirname(STATE_FILE), 'terminal-port'); const BROWSE_SERVER_PORT = parseInt(process.env.BROWSE_SERVER_PORT || '0', 10); const BROWSE_OWNER_PID = parseInt(process.env.BROWSE_OWNER_PID || '0', 10); +const BROWSE_OWNER_START_TIME = process.env.BROWSE_OWNER_START_TIME || (BROWSE_OWNER_PID > 0 ? readAgentStartTime(BROWSE_OWNER_PID) : ''); const OWNER_WATCHDOG_MS = parseInt( process.env.GSTACK_TERMINAL_OWNER_WATCHDOG_MS || '15000', 10, @@ -51,7 +52,7 @@ const INTERNAL_TOKEN = crypto.randomBytes(32).toString('base64url'); // shared w * header means "legacy caller" and is accepted (backward compat); a * present-but-mismatched header returns 409 stale generation. */ -const CURRENT_GEN = crypto.randomBytes(16).toString('base64url'); +const CURRENT_GEN = process.env.BROWSE_AGENT_GEN || crypto.randomBytes(16).toString('base64url'); // In-memory attach-token registry. Parent posts /internal/grant after // /pty-session; we validate WS upgrades against this map. @@ -1004,6 +1005,25 @@ function readBrowseToken(): string { // Boot. async function main() { + const dir = path.dirname(PORT_FILE); + if (process.env.BROWSE_AGENT_GEN) { + const deadline = Date.now() + 2000; + while (Date.now() < deadline) { + const pending = readAgentRecord(dir); + if (pending?.gen === CURRENT_GEN && pending.pid === process.pid) break; + if (pending && pending.gen !== CURRENT_GEN) throw new Error('terminal-agent startup record was replaced'); + await Bun.sleep(25); + } + const recorded = readAgentRecord(dir); + if (recorded?.pid !== process.pid || recorded.ownerPid !== BROWSE_OWNER_PID || recorded.ownerStartTime !== BROWSE_OWNER_START_TIME) { + throw new Error('terminal-agent startup record was not confirmed'); + } + } + const pauseFile = process.env.NODE_ENV === 'test' ? process.env.GSTACK_TERMINAL_TEST_PUBLISH_BARRIER : undefined; + if (pauseFile) { + fs.writeFileSync(`${pauseFile}.ready`, 'ready'); + while (!fs.existsSync(pauseFile)) await Bun.sleep(10); + } writeClaudeAvailable(); // #2314: allocate from the shared fixed scan range, then bind. Probe-then- // bind has a TOCTOU window — a concurrent process can take the port between @@ -1032,17 +1052,21 @@ async function main() { // Write port file atomically so the parent server can pick it up. // Throws on failure — a boot without a discoverable port file is broken. - const dir = path.dirname(PORT_FILE); - try { mkdirSecure(dir); } catch {} - atomicWriteSync(PORT_FILE, String(port), { mode: 0o600 }); - restrictFilePermissions(PORT_FILE); // Windows ACL hardening - - // Write identity-based agent record (pid + per-boot gen). Replaces the - // v1.43- `pkill -f terminal-agent\.ts` regex teardown that could kill - // sibling gstack sessions. Callers (cli.ts spawn site, server.ts - // shutdown, the v1.44 watchdog) now route through killAgentByRecord in - // terminal-agent-control.ts. - writeAgentRecord(dir, { pid: process.pid, gen: CURRENT_GEN, startedAt: Date.now() }); + const releasePublication = acquireAgentStateLock(dir); + let record; + try { + const current = readAgentRecord(dir); + if (current && current.pid !== process.pid && current.pid > 0) throw new Error('terminal-agent record was replaced before bind'); + record = process.env.BROWSE_AGENT_GEN ? current : { + pid: process.pid, gen: CURRENT_GEN, startedAt: Date.now(), startTime: readAgentStartTime(process.pid), + ownerPid: BROWSE_OWNER_PID, ownerStartTime: BROWSE_OWNER_START_TIME, + }; + if (!record || record.pid !== process.pid || record.gen !== CURRENT_GEN) throw new Error('terminal-agent record was replaced before bind'); + if (!process.env.BROWSE_AGENT_GEN) writeAgentRecord(dir, record); + writeSecureFile(INTERNAL_TOKEN_FILE, INTERNAL_TOKEN); + atomicWriteSync(PORT_FILE, String(port), { mode: 0o600 }); + restrictFilePermissions(PORT_FILE); + } finally { releasePublication(); } // Hand the parent the internal token so it can call /internal/grant. // Parent learns INTERNAL_TOKEN via env (TERMINAL_AGENT_INTERNAL_TOKEN below). @@ -1055,9 +1079,16 @@ async function main() { const cleanup = () => { if (cleaningUp) return; cleaningUp = true; - safeUnlink(PORT_FILE); - safeUnlink(INTERNAL_TOKEN_FILE); - clearAgentRecord(dir); + try { + const releaseCleanup = acquireAgentStateLock(dir, 25); + try { + if (readAgentRecord(dir)?.gen === CURRENT_GEN) { + safeUnlink(PORT_FILE); + safeUnlink(INTERNAL_TOKEN_FILE); + clearAgentRecord(dir, record); + } + } finally { releaseCleanup(); } + } catch {} process.exit(0); }; process.on('SIGTERM', cleanup); @@ -1070,11 +1101,8 @@ async function main() { // the same cleanup path as an intentional shutdown when it disappears. if (BROWSE_OWNER_PID > 0) { const ownerWatchdog = setInterval(() => { - try { - process.kill(BROWSE_OWNER_PID, 0); - } catch { - cleanup(); - } + if (!BROWSE_OWNER_START_TIME || readAgentStartTime(BROWSE_OWNER_PID) !== BROWSE_OWNER_START_TIME + || readAgentRecord(dir)?.gen !== CURRENT_GEN) cleanup(); }, OWNER_WATCHDOG_MS); (ownerWatchdog as any)?.unref?.(); } @@ -1087,10 +1115,6 @@ async function main() { // In practice, the agent generates INTERNAL_TOKEN once at boot and writes it // to a state file the parent reads. This avoids env-passing races. See main(). const INTERNAL_TOKEN_FILE = path.join(path.dirname(STATE_FILE), 'terminal-internal-token'); -try { - mkdirSecure(path.dirname(INTERNAL_TOKEN_FILE)); - writeSecureFile(INTERNAL_TOKEN_FILE, INTERNAL_TOKEN); -} catch {} main().catch((err) => { console.error(`[terminal-agent] boot failed: ${err instanceof Error ? err.message : String(err)}`); diff --git a/browse/test/process-liveness-windows.test.ts b/browse/test/process-liveness-windows.test.ts index de907af40..b2f5f7e93 100644 --- a/browse/test/process-liveness-windows.test.ts +++ b/browse/test/process-liveness-windows.test.ts @@ -1,4 +1,5 @@ -import { describe, test, expect } from 'bun:test'; +import { describe, test, expect, spyOn } from 'bun:test'; +import * as childProcess from 'node:child_process'; import * as fs from 'fs'; import * as os from 'os'; import * as path from 'path'; @@ -103,34 +104,54 @@ describe('process liveness probe (Windows terminal-agent leak)', () => { expect(offenders).toEqual([]); }); - test('5. spawnTerminalAgent passes windowsHide so no console is shown', () => { + test('5. spawnTerminalAgent passes windowsHide so no console is shown', async () => { const tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), 'gstack-hide-')); const script = path.join(tmpDir, 'fake-agent.ts'); fs.writeFileSync(script, '// no-op\n'); const origSpawn = (Bun as any).spawn; + const originalProbe = Bun.spawnSync; + const originalWindowsProbe = childProcess.spawnSync; + const probe = spyOn(Bun, 'spawnSync').mockImplementation(((command: string[], options: any) => { + if (command[0] === 'ps' && command[2] === String(process.pid)) { + return { exitCode: 0, stdout: Buffer.from('fixture-owner-start'), stderr: Buffer.alloc(0) }; + } + return originalProbe(command, options); + }) as typeof Bun.spawnSync); + const windowsProbe = spyOn(childProcess, 'spawnSync').mockImplementation(((command: string, args: string[], options: any) => { + if (command === 'powershell.exe') { + const owner = args.join(' ').includes(`ProcessId = ${process.pid}'`); + return { status: 0, stdout: JSON.stringify(owner ? { CreationDate: 'fixture-owner-start', CommandLine: 'test-owner' } : null), stderr: '' }; + } + return originalWindowsProbe(command, args, options); + }) as typeof childProcess.spawnSync); + const exited = Promise.resolve(0); let captured: any = null; (Bun as any).spawn = (_cmd: any, opts: any) => { captured = opts; - return { pid: 4242, unref() {} }; + return { pid: 2147483647, exited, kill() {}, unref() {} }; }; try { - const pid = spawnTerminalAgent({ + expect(() => spawnTerminalAgent({ stateFile: path.join(tmpDir, 'state.json'), serverPort: 12345, ownerPid: process.pid, cwd: tmpDir, scriptPath: script, - }); - expect(pid).toBe(4242); + })).toThrow('terminal-agent process identity is unavailable'); expect(captured).not.toBeNull(); expect(captured.windowsHide).toBe(true); // Owner-PID lifetime tie (#2019): the agent polls this and exits when // its owning browse server dies, so it can't be adopted by PID 1. expect(captured.env.BROWSE_OWNER_PID).toBe(String(process.pid)); + expect(captured.env.BROWSE_OWNER_START_TIME).toBe('fixture-owner-start'); // Detached background daemon — must not inherit a terminal either. expect(captured.stdio).toEqual(['ignore', 'ignore', 'ignore']); + await exited; + expect(fs.existsSync(path.join(tmpDir, 'terminal-agent-pid'))).toBe(false); } finally { (Bun as any).spawn = origSpawn; + windowsProbe.mockRestore(); + probe.mockRestore(); fs.rmSync(tmpDir, { recursive: true, force: true }); } }); diff --git a/browse/test/server-embedder-terminal-port.test.ts b/browse/test/server-embedder-terminal-port.test.ts index 53bf9f3b7..d0c8f3060 100644 --- a/browse/test/server-embedder-terminal-port.test.ts +++ b/browse/test/server-embedder-terminal-port.test.ts @@ -1,4 +1,4 @@ -import { describe, test, expect, beforeEach, beforeAll, afterAll } from 'bun:test'; +import { describe, test, expect, beforeEach, beforeAll, afterAll, spyOn } from 'bun:test'; import * as fs from 'fs'; import * as path from 'path'; import * as os from 'os'; @@ -219,4 +219,83 @@ describe('buildFetchHandler ownsTerminalAgent gate', () => { // match cannot be satisfied by the JSDoc reference earlier in the file. expect(source).toMatch(/ownsTerminalAgent:\s*true,\s*\/\/\s*CLI spawns terminal-agent\.ts/); }); + + test('5. shutdown cannot remove a successor published after its current-record read', async () => { + writeSentinels(); + const ready = path.join(fixtureDir, 'competitor-ready'); + const script = path.join(fixtureDir, 'competitor.ts'); + fs.writeFileSync(script, ` + import * as fs from 'fs'; + import * as path from 'path'; + import { acquireAgentStateLock } from ${JSON.stringify(path.resolve(import.meta.dir, '../src/terminal-agent-control.ts'))}; + const stateDir = process.argv[2]; + fs.writeFileSync(${JSON.stringify(ready)}, 'ready'); + const release = acquireAgentStateLock(stateDir); + try { + fs.writeFileSync(path.join(stateDir, 'terminal-port'), 'successor-port'); + fs.writeFileSync(path.join(stateDir, 'terminal-internal-token'), 'synthetic-successor-token'); + fs.writeFileSync(path.join(stateDir, 'terminal-agent-pid'), JSON.stringify({ pid: process.pid, gen: 'successor', startedAt: Date.now() })); + } finally { release(); } + `); + const originalRead = fs.readFileSync; + let recordReads = 0; + let actor: ReturnType | undefined; + const reader = spyOn(fs, 'readFileSync').mockImplementation(((file: fs.PathOrFileDescriptor, options?: any) => { + const result = originalRead(file as any, options); + if (String(file) === AGENT_RECORD_FILE && ++recordReads === 2) { + actor = Bun.spawn([process.execPath, script, stateDir], { stdio: ['ignore', 'ignore', 'ignore'] }); + const deadline = Date.now() + 3000; + while (!fs.existsSync(ready) && Date.now() < deadline) Atomics.wait(new Int32Array(new SharedArrayBuffer(4)), 0, 0, 10); + if (!fs.existsSync(ready)) throw new Error('competitor never reached publication'); + Atomics.wait(new Int32Array(new SharedArrayBuffer(4)), 0, 0, 150); + } + return result; + }) as typeof fs.readFileSync); + try { + const handle = buildFetchHandler(makeMinimalConfig({ ownsTerminalAgent: true })); + await withStubs(async () => runShutdown(handle)); + expect(recordReads).toBeGreaterThanOrEqual(2); + expect(actor).toBeDefined(); + expect(await Promise.race([actor!.exited.then(() => true), Bun.sleep(5000).then(() => false)])).toBe(true); + expect(readIfExists(PORT_FILE)).toBe('successor-port'); + expect(readIfExists(TOKEN_FILE)).toBe('synthetic-successor-token'); + expect(JSON.parse(readIfExists(AGENT_RECORD_FILE)!)).toMatchObject({ gen: 'successor' }); + } finally { + reader.mockRestore(); + try { actor?.kill('SIGKILL'); } catch {} + fs.rmSync(ready, { force: true }); + fs.rmSync(script, { force: true }); + } + }, 15000); + + test('6. unavailable state lock retains agent files rather than guessing ownership', async () => { + writeSentinels(); + const originalOpen = fs.openSync; + const opened = spyOn(fs, 'openSync').mockImplementation(((file: fs.PathLike, flags: string | number, mode?: number) => { + if (String(file) === path.join(stateDir, 'terminal-agent-pid.lock')) { + throw Object.assign(new Error('synthetic lock denial'), { code: 'EACCES' }); + } + return originalOpen(file, flags as any, mode); + }) as typeof fs.openSync); + try { + const handle = buildFetchHandler(makeMinimalConfig({ ownsTerminalAgent: true })); + await withStubs(async () => runShutdown(handle)); + expect(readIfExists(PORT_FILE)).toBe(SENTINEL_PORT); + expect(readIfExists(TOKEN_FILE)).toBe(SENTINEL_TOKEN); + expect(readIfExists(AGENT_RECORD_FILE)).not.toBeNull(); + } finally { opened.mockRestore(); } + }); + + test('7. late state takeover is not removed after browser close', async () => { + fs.mkdirSync(stateDir, { recursive: true }); + fs.writeFileSync(fixtureConfig.stateFile, JSON.stringify({ pid: process.pid })); + const successor = { pid: process.pid, instanceId: 'synthetic-late-successor' }; + const browserManager = new BrowserManager(); + browserManager.close = async () => { fs.writeFileSync(fixtureConfig.stateFile, JSON.stringify(successor)); }; + try { + const handle = buildFetchHandler(makeMinimalConfig({ browserManager, ownsTerminalAgent: false })); + await withStubs(async () => runShutdown(handle)); + expect(JSON.parse(fs.readFileSync(fixtureConfig.stateFile, 'utf8'))).toEqual(successor); + } finally { fs.rmSync(fixtureConfig.stateFile, { force: true }); } + }); }); diff --git a/browse/test/server-factory.test.ts b/browse/test/server-factory.test.ts index 58982b47f..deefed76f 100644 --- a/browse/test/server-factory.test.ts +++ b/browse/test/server-factory.test.ts @@ -246,11 +246,11 @@ describe('buildFetchHandler factory contract', () => { fs.mkdirSync(path.dirname(globalState), { recursive: true }); fs.mkdirSync(path.dirname(instanceState), { recursive: true }); fs.writeFileSync(globalState, 'unrelated daemon state'); - fs.writeFileSync(instanceState, 'owned instance state'); const script = ` import fs from 'node:fs'; - import { buildFetchHandler } from ${JSON.stringify(path.resolve(__dirname, '../src/server.ts'))}; + import { buildFetchHandler, __testInternals__ } from ${JSON.stringify(path.resolve(__dirname, '../src/server.ts'))}; import { resolveConfig } from ${JSON.stringify(path.resolve(__dirname, '../src/config.ts'))}; + fs.writeFileSync(${JSON.stringify(instanceState)}, JSON.stringify({ pid: process.pid, instanceId: __testInternals__.serverInstanceId })); const handle = buildFetchHandler({ authToken: 'factory-shutdown-ownership-test', browsePort: 34567, config: resolveConfig({ BROWSE_STATE_FILE: ${JSON.stringify(instanceState)} }), diff --git a/browse/test/sidebar-tabs.test.ts b/browse/test/sidebar-tabs.test.ts index 7f408d0ac..6dbc5e3c1 100644 --- a/browse/test/sidebar-tabs.test.ts +++ b/browse/test/sidebar-tabs.test.ts @@ -235,7 +235,8 @@ describe('cli.ts: sidebar-agent is no longer spawned', () => { 'utf-8', ); expect(CONTROL_SRC).toContain('terminal-agent.ts'); - expect(CONTROL_SRC).toMatch(/\.spawn\(\['bun',\s*'run',\s*script\]/); + expect(CONTROL_SRC).toMatch(/\.spawn\(\['bun',\s*'run',\s*script,\s*`--agent-gen=\$\{gen\}`\]/); + expect(CONTROL_SRC).toContain('BROWSE_OWNER_PID: String(opts.ownerPid)'); }); }); diff --git a/browse/test/sidebar-ux.test.ts b/browse/test/sidebar-ux.test.ts index c97412349..5d339f249 100644 --- a/browse/test/sidebar-ux.test.ts +++ b/browse/test/sidebar-ux.test.ts @@ -967,9 +967,10 @@ describe('shutdown cleanup (server.ts)', () => { // by browse/test/terminal-agent-pid-identity.test.ts). const shutdownFn = serverSrc.slice( serverSrc.indexOf('async function shutdown('), - serverSrc.indexOf('async function shutdown(') + 1200, + serverSrc.indexOf('try { detachSession()', serverSrc.indexOf('async function shutdown(')), ); - expect(shutdownFn).toContain('killAgentByRecord'); + expect(shutdownFn).toContain('stopAgentByRecord'); + expect(shutdownFn).toContain('isOurAgent(record, process.pid)'); expect(shutdownFn).toContain('readAgentRecord'); // No pkill CALL — the word may appear in the explanatory comment, so // match invocation shapes only. The repo-wide reintroduction tripwire @@ -994,4 +995,3 @@ describe('cookie import button (sidebar)', () => { expect(js).toContain('cookie-picker'); }); }); - diff --git a/browse/test/terminal-agent-lifecycle.test.ts b/browse/test/terminal-agent-lifecycle.test.ts new file mode 100644 index 000000000..8b2d47188 --- /dev/null +++ b/browse/test/terminal-agent-lifecycle.test.ts @@ -0,0 +1,363 @@ +import { afterEach, describe, expect, spyOn, test } from 'bun:test'; +import * as fs from 'fs'; +import * as path from 'path'; +import * as os from 'os'; +import { + acquireAgentStateLock, agentRecordPath, clearAgentRecord, isOurAgent, killAgentByRecord, readAgentRecord, + readAgentStartTime, spawnTerminalAgent, stopAgentByRecord, type AgentRecord, + writeAgentRecord, +} from '../src/terminal-agent-control'; + +const sourceDir = path.join(import.meta.dir, '..', 'src'); +const dirs: string[] = []; +const pids: number[] = []; +const dir = () => { + const value = fs.mkdtempSync(path.join(os.tmpdir(), 'g4-')); + dirs.push(value); + return value; +}; +const waitFor = async (check: () => boolean, timeout = 3000) => { + const deadline = Date.now() + timeout; + while (Date.now() < deadline) { + if (check()) return true; + await Bun.sleep(25); + } + return check(); +}; +const spawn = (stateDir: string, ownerPid = process.pid) => { + const pid = spawnTerminalAgent({ stateFile: path.join(stateDir, 'browse.json'), serverPort: 0, ownerPid, + extraEnv: { GSTACK_TERMINAL_OWNER_WATCHDOG_MS: '25' } }); + if (pid) pids.push(pid); + return pid; +}; + +afterEach(() => { + for (const pid of pids.splice(0)) { + const record = dirs.map(readAgentRecord).find(value => value?.pid === pid); + if (record) stopAgentByRecord(record, 200); + } + for (const stateDir of dirs.splice(0)) fs.rmSync(stateDir, { recursive: true, force: true }); +}); + +describe('terminal-agent owned lifecycle regression', () => { + for (const field of ['dev', 'ino'] as const) { + test(`lock release preserves a replacement with an adjacent 64-bit ${field}`, () => { + const stateDir = dir(); + const lockPath = path.join(stateDir, 'terminal-agent-pid.lock'); + const identity = 1n << 63n; + expect(Number(identity)).toBe(Number(identity + 1n)); + const originalFstat = fs.fstatSync; + const originalStat = fs.statSync; + const descriptor = spyOn(fs, 'fstatSync').mockImplementation(((fd: number, options?: any) => { + const stat = originalFstat(fd, options); + return Object.assign(stat, { [field]: options?.bigint ? identity : Number(identity) }); + }) as typeof fs.fstatSync); + const pathname = spyOn(fs, 'statSync').mockImplementation(((file: fs.PathLike, options?: any) => { + const stat = originalStat(file, options); + return String(file) === lockPath + ? Object.assign(stat, { [field]: options?.bigint ? identity + 1n : Number(identity + 1n) }) + : stat; + }) as typeof fs.statSync); + try { + acquireAgentStateLock(stateDir)(); + expect(fs.existsSync(lockPath)).toBe(true); + } finally { + descriptor.mockRestore(); + pathname.mockRestore(); + } + }); + } + + test('connect and supervisor pass the persistent daemon as owner', () => { + const cli = fs.readFileSync(path.join(sourceDir, 'cli.ts'), 'utf8'); + const connect = cli.slice(cli.indexOf('// Auto-start terminal agent'), cli.indexOf('// ─── Outer Supervisor')); + const supervisor = cli.slice(cli.indexOf('// ─── Outer Supervisor'), cli.indexOf('// ─── Headed Disconnect')); + expect(connect).toMatch(/spawnTerminalAgent\(\{[^}]*ownerPid:\s*newState\.pid/s); + expect(supervisor).toMatch(/spawnTerminalAgent\(\{[^}]*ownerPid:\s*respawned\.pid/s); + }); + + test('owned agent starts, is replaced only after exit, and leaves a live sibling alone', async () => { + const firstDir = dir(); + const siblingDir = dir(); + const first = spawn(firstDir); + const sibling = spawn(siblingDir); + expect(first).toBeGreaterThan(0); + expect(sibling).toBeGreaterThan(0); + expect(await waitFor(() => fs.existsSync(path.join(firstDir, 'terminal-port')))).toBe(true); + const firstRecord = readAgentRecord(firstDir)!; + const siblingRecord = readAgentRecord(siblingDir)!; + expect(isOurAgent(firstRecord, process.pid)).toBe(true); + const replacement = spawn(firstDir); + expect(replacement).toBeGreaterThan(0); + expect(replacement).not.toBe(first); + expect(isOurAgent(firstRecord)).toBe(false); + expect(isOurAgent(siblingRecord)).toBe(true); + expect(readAgentRecord(firstDir)?.pid).toBe(replacement); + }); + + test('child waits for its PID to replace the pre-spawn reservation', async () => { + const stateDir = dir(); + const originalSpawn = Bun.spawn; + (Bun as any).spawn = (...args: Parameters) => { + const child = originalSpawn(...args); + Atomics.wait(new Int32Array(new SharedArrayBuffer(4)), 0, 0, 800); + return child; + }; + try { + const pid = spawn(stateDir); + expect(pid).toBeGreaterThan(0); + expect(await waitFor(() => fs.existsSync(path.join(stateDir, 'terminal-port')))).toBe(true); + expect(readAgentRecord(stateDir)?.pid).toBe(pid); + } finally { (Bun as any).spawn = originalSpawn; } + }); + + test('failed signals retain the live record and prevent a duplicate spawn', () => { + const stateDir = dir(); + const first = spawn(stateDir)!; + const record = readAgentRecord(stateDir)!; + const original = process.kill; + (process as any).kill = ((pid: number, signal: NodeJS.Signals | number) => { + if (pid === first && signal !== 0) throw Object.assign(new Error('denied'), { code: 'EPERM' }); + return original(pid, signal); + }) as typeof process.kill; + try { + expect(spawn(stateDir)).toBeNull(); + expect(readAgentRecord(stateDir)).toEqual(record); + } finally { + (process as any).kill = original; + } + }); + + test('a transient identity lookup failure after a signal is not confirmed exit', () => { + const stateDir = dir(); + const first = spawn(stateDir)!; + const record = readAgentRecord(stateDir)!; + const originalKill = process.kill; + const originalSpawnSync = Bun.spawnSync; + let obscured = false; + (process as any).kill = ((pid: number, signal: NodeJS.Signals | number) => { + if (pid === first && signal !== 0) { obscured = true; return true; } + return originalKill(pid, signal); + }) as typeof process.kill; + (Bun as any).spawnSync = (...args: Parameters) => { + const command = args[0] as string[]; + if (obscured && command[0] === 'ps' && command[2] === String(first)) { + return { exitCode: 1, stdout: Buffer.alloc(0), stderr: Buffer.alloc(0) }; + } + return originalSpawnSync(...args); + }; + try { + expect(spawn(stateDir)).toBeNull(); + expect(readAgentRecord(stateDir)).toEqual(record); + } finally { + (process as any).kill = originalKill; + (Bun as any).spawnSync = originalSpawnSync; + } + }); + + test('PID reuse and foreign records are never signaled', () => { + const stateDir = dir(); + const forged: AgentRecord = { + pid: process.pid, gen: 'synthetic-foreign-generation', startedAt: Date.now(), + startTime: readAgentStartTime(process.pid), ownerPid: process.pid, ownerStartTime: readAgentStartTime(process.pid), + }; + fs.writeFileSync(agentRecordPath(stateDir), JSON.stringify(forged)); + expect(isOurAgent(forged)).toBe(false); + expect(killAgentByRecord(forged, 'SIGTERM')).toBe(false); + expect(spawn(stateDir)).toBeNull(); + expect(readAgentRecord(stateDir)).toEqual(forged); + clearAgentRecord(stateDir, { ...forged, gen: 'different' }); + expect(readAgentRecord(stateDir)).toEqual(forged); + }); + + test('unwritable record path rejects before spawning any agent', () => { + const stateDir = dir(); + const blocker = path.join(stateDir, 'blocker'); + fs.writeFileSync(blocker, 'block'); + const originalSpawn = Bun.spawn; + let child: ReturnType | undefined; + (Bun as any).spawn = (...args: Parameters) => { + child = originalSpawn(...args); + return child; + }; + try { + expect(() => spawnTerminalAgent({ stateFile: path.join(blocker, 'browse.json'), serverPort: 0, ownerPid: process.pid })) + .toThrow(); + expect(child).toBeUndefined(); + expect(fs.readdirSync(stateDir)).toEqual(['blocker']); + } finally { + (Bun as any).spawn = originalSpawn; + try { child?.kill('SIGKILL'); } catch {} + } + }); + + test('a leftover exclusive lock refuses recovery without stealing ownership', () => { + const stateDir = dir(); + const lock = path.join(stateDir, 'terminal-agent-pid.lock'); + fs.writeFileSync(lock, ''); + expect(() => acquireAgentStateLock(stateDir, 0)).toThrow('state lock unavailable'); + expect(fs.existsSync(lock)).toBe(true); + expect(readAgentRecord(stateDir)).toBeNull(); + }); + + test('record update failure after spawn confirms child exit before dropping its handle', async () => { + const stateDir = dir(); + const originalSpawn = Bun.spawn; + const originalRename = fs.renameSync; + let child: ReturnType | undefined; + let writes = 0; + const rename = spyOn(fs, 'renameSync').mockImplementation(((from: fs.PathLike, to: fs.PathLike) => { + if (String(to) === agentRecordPath(stateDir) && ++writes === 2) { + throw Object.assign(new Error('synthetic state write failure'), { code: 'EIO' }); + } + return originalRename(from, to); + }) as typeof fs.renameSync); + (Bun as any).spawn = (...args: Parameters) => { + child = originalSpawn(...args); + return child; + }; + try { + expect(() => spawn(stateDir)).toThrow('synthetic state write failure'); + expect(writes).toBe(2); + expect(child).toBeDefined(); + expect(await Promise.race([child!.exited.then(() => true), Bun.sleep(3000).then(() => false)])).toBe(true); + expect(readAgentRecord(stateDir)).toBeNull(); + } finally { + rename.mockRestore(); + (Bun as any).spawn = originalSpawn; + try { child?.kill('SIGKILL'); } catch {} + } + }); + + (process.platform === 'win32' ? test.skip : test)('unconfirmed post-write child keeps its reservation until it exits', async () => { + const stateDir = dir(); + const originalSpawn = Bun.spawn; + const originalRename = fs.renameSync; + const originalKill = process.kill; + let child: ReturnType | undefined; + let writes = 0; + let deniedSignals = 0; + const rename = spyOn(fs, 'renameSync').mockImplementation(((from: fs.PathLike, to: fs.PathLike) => { + if (String(to) === agentRecordPath(stateDir) && ++writes === 2) throw new Error('synthetic update refusal'); + return originalRename(from, to); + }) as typeof fs.renameSync); + (Bun as any).spawn = (...args: Parameters) => { + child = originalSpawn(...args); + originalKill(child.pid, 'SIGSTOP'); + return child; + }; + (process as any).kill = ((pid: number, signal: NodeJS.Signals | number) => { + if (child && pid === child.pid && signal !== 0) { + deniedSignals++; + throw Object.assign(new Error('signal denied'), { code: 'EPERM' }); + } + return originalKill(pid, signal); + }) as typeof process.kill; + try { + expect(() => spawn(stateDir)).toThrow('exit is unconfirmed'); + expect(deniedSignals).toBeGreaterThan(0); + expect(readAgentRecord(stateDir)?.pid).toBe(0); + expect(spawn(stateDir)).toBeNull(); + expect(readAgentRecord(stateDir)?.pid).toBe(0); + originalKill(child!.pid, 'SIGCONT'); + expect(await Promise.race([child!.exited.then(() => true), Bun.sleep(4000).then(() => false)])).toBe(true); + expect(await waitFor(() => readAgentRecord(stateDir) === null)).toBe(true); + } finally { + (process as any).kill = originalKill; + (Bun as any).spawn = originalSpawn; + rename.mockRestore(); + if (child) try { originalKill(child.pid, 'SIGCONT'); } catch {} + try { child?.kill('SIGKILL'); } catch {} + } + }, 6000); + + test('owner death and record takeover shut down the old generation without deleting its successor', async () => { + const stateDir = dir(); + const owner = Bun.spawn([process.execPath, '-e', 'process.stdin.resume()'], { stdio: ['pipe', 'ignore', 'ignore'] }); + try { + const pid = spawn(stateDir, owner.pid)!; + expect(await waitFor(() => fs.existsSync(path.join(stateDir, 'terminal-port')))).toBe(true); + const record = readAgentRecord(stateDir)!; + owner.kill('SIGTERM'); + await owner.exited; + expect(await waitFor(() => !isOurAgent(record), 5000)).toBe(true); + expect(readAgentRecord(stateDir)).toBeNull(); + } finally { try { owner.kill('SIGKILL'); } catch {} } + + const first = spawn(stateDir)!; + expect(await waitFor(() => fs.existsSync(path.join(stateDir, 'terminal-port')))).toBe(true); + const original = readAgentRecord(stateDir)!; + const successor = { ...original, pid: 2147483646, gen: 'synthetic-successor' }; + fs.writeFileSync(agentRecordPath(stateDir), JSON.stringify(successor)); + expect(await waitFor(() => !isOurAgent(original), 5000)).toBe(true); + expect(readAgentRecord(stateDir)).toEqual(successor); + expect(first).toBeGreaterThan(0); + }, 12000); + + test('losing concurrent startup cannot publish over the winning generation', async () => { + const stateDir = dir(); + const stateFile = path.join(stateDir, 'browse.json'); + const barrier = path.join(stateDir, 'go'); + const ownerStartTime = readAgentStartTime(process.pid); + const rawAgent = (gen: string, paused: boolean) => Bun.spawn(['bun', 'run', path.join(sourceDir, 'terminal-agent.ts'), `--agent-gen=${gen}`], { + env: { ...process.env, BROWSE_STATE_FILE: stateFile, BROWSE_OWNER_PID: String(process.pid), + BROWSE_OWNER_START_TIME: ownerStartTime, BROWSE_AGENT_GEN: gen, NODE_ENV: 'test', + GSTACK_TERMINAL_OWNER_WATCHDOG_MS: '25', + ...(paused ? { GSTACK_TERMINAL_TEST_PUBLISH_BARRIER: barrier } : {}) }, + stdio: ['ignore', 'ignore', 'ignore'], + }); + const old = rawAgent('synthetic-old-generation', true); + let winner: ReturnType | undefined; + try { + writeAgentRecord(stateDir, { pid: old.pid, gen: 'synthetic-old-generation', startedAt: Date.now(), + startTime: readAgentStartTime(old.pid), ownerPid: process.pid, ownerStartTime }); + expect(await waitFor(() => fs.existsSync(`${barrier}.ready`))).toBe(true); + winner = rawAgent('synthetic-new-generation', false); + writeAgentRecord(stateDir, { pid: winner.pid, gen: 'synthetic-new-generation', startedAt: Date.now(), + startTime: readAgentStartTime(winner.pid), ownerPid: process.pid, ownerStartTime }); + expect(await waitFor(() => fs.existsSync(path.join(stateDir, 'terminal-port')))).toBe(true); + const port = fs.readFileSync(path.join(stateDir, 'terminal-port'), 'utf8'); + const token = fs.readFileSync(path.join(stateDir, 'terminal-internal-token'), 'utf8'); + fs.writeFileSync(barrier, 'continue'); + expect(await Promise.race([old.exited.then(() => true), Bun.sleep(3000).then(() => false)])).toBe(true); + expect(readAgentRecord(stateDir)?.gen).toBe('synthetic-new-generation'); + expect(fs.readFileSync(path.join(stateDir, 'terminal-port'), 'utf8')).toBe(port); + expect(fs.readFileSync(path.join(stateDir, 'terminal-internal-token'), 'utf8')).toBe(token); + } finally { + fs.writeFileSync(barrier, 'continue'); + try { old.kill('SIGKILL'); } catch {} + try { winner?.kill('SIGKILL'); } catch {} + await old.exited; + if (winner) await winner.exited; + } + }, 10000); + + test('daemon respawns after agent crash, then exits without deleting a successor state', async () => { + const stateDir = dir(); + const stateFile = path.join(stateDir, 'browse.json'); + const daemon = Bun.spawn(['bun', 'run', path.join(sourceDir, 'server.ts')], { + env: { ...process.env, BROWSE_STATE_FILE: stateFile, BROWSE_HEADLESS_SKIP: '1', BROWSE_PARENT_PID: '0', + GSTACK_AGENT_WATCHDOG_TICK_MS: '50', GSTACK_STATE_WATCH_MS: '50' }, + stdio: ['ignore', 'ignore', 'ignore'], + }); + try { + expect(await waitFor(() => fs.existsSync(stateFile))).toBe(true); + expect(await waitFor(() => { + const record = readAgentRecord(stateDir); + return !!record && record.pid > 0 && isOurAgent(record, daemon.pid); + }, 5000)).toBe(true); + const old = readAgentRecord(stateDir)!; + expect(old.ownerPid).toBe(daemon.pid); + expect(isOurAgent(old, daemon.pid)).toBe(true); + expect(killAgentByRecord(old, 'SIGKILL')).toBe(true); + expect(await waitFor(() => !!readAgentRecord(stateDir) && readAgentRecord(stateDir)!.gen !== old.gen, 5000)).toBe(true); + const successor = { ...JSON.parse(fs.readFileSync(stateFile, 'utf8')), pid: process.pid, instanceId: 'synthetic-successor' }; + fs.writeFileSync(stateFile, JSON.stringify(successor)); + expect(await waitFor(() => daemon.exitCode !== null, 5000)).toBe(true); + expect(JSON.parse(fs.readFileSync(stateFile, 'utf8'))).toEqual(successor); + } finally { + try { daemon.kill('SIGKILL'); } catch {} + await daemon.exited; + } + }, 15000); +}); diff --git a/browse/test/terminal-agent-native-observation.test.ts b/browse/test/terminal-agent-native-observation.test.ts new file mode 100644 index 000000000..374970a34 --- /dev/null +++ b/browse/test/terminal-agent-native-observation.test.ts @@ -0,0 +1,66 @@ +import { describe, expect, spyOn, test } from 'bun:test'; +import * as childProcess from 'node:child_process'; +import { isAgentRecordGone, isOurAgent, stopAgentByRecord } from '../src/terminal-agent-control'; + +describe('terminal-agent native exit observations', () => { + const pid = 2147483645; + const startTime = 'Tue Sep 22 23:39:21 2026'; + const gen = 'test-observed-generation'; + const ownedCommand = `bun run terminal-agent.ts --agent-gen=${gen}`; + const cases = [ + { name: 'Darwin zombie retains nonempty command text', command: '(bun)', state: 'Z', gone: true }, + { name: 'zombie retains its generation argument', command: ownedCommand, state: 'Z', gone: true }, + { name: 'process exits during start-time lookup', command: '', state: '', missingStart: true, reap: true, gone: true }, + { name: 'process exits during command lookup', command: '', state: '', reap: true, gone: true }, + { name: 'live process has a failed start-time lookup', command: '', state: 'S', missingStart: true, gone: false }, + { name: 'live process has a failed command lookup', command: '', state: 'S', gone: false }, + { name: 'live foreign generation is not ours', command: 'bun unrelated.ts', state: 'S', gone: false }, + { name: 'live owned generation remains ours', command: ownedCommand, state: 'S', gone: false, owned: true }, + { name: 'failed state probe cannot certify a zombie', command: '', state: 'Z', stateStatus: 1, gone: false }, + { name: 'process exits before the owned signal', command: ownedCommand, state: 'S', gone: false, owned: true, reapOnSignal: true }, + ]; + + for (const scenario of cases) { + test(scenario.name, () => { + const platform = Object.getOwnPropertyDescriptor(process, 'platform')!; + let live = true; + const signals: unknown[] = []; + const kill = spyOn(process, 'kill').mockImplementation(((target: number, signal: unknown) => { + expect(target).toBe(pid); + if (signal !== 0) { + signals.push(signal); + if (scenario.reapOnSignal) { live = false; throw Object.assign(new Error('gone'), { code: 'ESRCH' }); } + throw new Error('unexpected signal'); + } + if (!live) throw Object.assign(new Error('gone'), { code: 'ESRCH' }); + return true; + }) as typeof process.kill); + const probe = spyOn(Bun, 'spawnSync').mockImplementation(((command: string[]) => { + expect(command.slice(0, 3)).toEqual(['ps', '-p', String(pid)]); + const start = command[4] === 'lstart='; + if (scenario.reap && (!start || scenario.missingStart)) live = false; + return { + exitCode: start && scenario.missingStart ? 1 : 0, + stdout: Buffer.from(start ? scenario.missingStart ? '' : startTime : scenario.command), + stderr: Buffer.alloc(0), + }; + }) as typeof Bun.spawnSync); + const state = spyOn(childProcess, 'spawnSync').mockReturnValue({ + status: scenario.stateStatus ?? 0, stdout: scenario.state, stderr: '', + } as any); + Object.defineProperty(process, 'platform', { ...platform, value: 'darwin' }); + try { + const record = { pid, gen, startTime, startedAt: 0, ownerPid: pid, ownerStartTime: startTime }; + expect(isAgentRecordGone(record)).toBe(scenario.gone); + expect(isOurAgent(record)).toBe(scenario.owned ?? false); + if (scenario.gone || scenario.reapOnSignal) expect(stopAgentByRecord(record, 0)).toBe(true); + expect(signals).toEqual(scenario.reapOnSignal ? ['SIGTERM'] : []); + } finally { + Object.defineProperty(process, 'platform', platform); + state.mockRestore(); + probe.mockRestore(); + kill.mockRestore(); + } + }); + } +}); diff --git a/browse/test/terminal-agent-watchdog.test.ts b/browse/test/terminal-agent-watchdog.test.ts index ff48b0bb3..2cabf130e 100644 --- a/browse/test/terminal-agent-watchdog.test.ts +++ b/browse/test/terminal-agent-watchdog.test.ts @@ -19,8 +19,8 @@ describe('terminal-agent watchdog (v1.44+)', () => { expect(src).toMatch(/export function spawnTerminalAgent\(/); // Must clean up prior PID before spawning (no zombies). expect(src).toContain('readAgentRecord(stateDir)'); - expect(src).toContain('killAgentByRecord(prior'); - expect(src).toContain('clearAgentRecord(stateDir)'); + expect(src).toContain('stopAgentByRecord(prior)'); + expect(src).toContain('clearAgentRecord(stateDir, prior)'); }); test('2. watchdog is gated on ownsTerminalAgent', () => { @@ -39,7 +39,11 @@ describe('terminal-agent watchdog (v1.44+)', () => { // identity-based liveness. Slow-but-alive agents must NOT trigger // respawn (split-brain defense). expect(block).toContain('readAgentRecord(stateDir)'); - expect(block).toContain('isProcessAlive(record.pid)'); + expect(block).toContain('isAgentRecordGone(record)'); + const control = fs.readFileSync(CONTROL_TS, 'utf-8'); + expect(control).toContain('if (result.status === 0) state = result.stdout?.trim()?.[0];'); + expect(control).toContain("if (state === 'Z') return 'gone'"); + expect(control.indexOf("if (state === 'Z') return 'gone'")).toBeLessThan(control.indexOf('return actual.commandLine.split')); // Negative: no executable name-based process lookup. Allow the strings // to appear in prose comments (the watchdog doc explains what it // replaces), reject only actual invocations. diff --git a/docs/BROWSER_INTERNALS.md b/docs/BROWSER_INTERNALS.md index 1d797a6c2..fb3b44035 100644 --- a/docs/BROWSER_INTERNALS.md +++ b/docs/BROWSER_INTERNALS.md @@ -21,11 +21,12 @@ here usually trace to not understanding the cross-component flow. **Embedder terminal-agent ownership** (v1.42.1.0+, identity-based kill v1.44.0.0+). `buildFetchHandler` in `browse/src/server.ts` accepts `ServerConfig.ownsTerminalAgent?: -boolean` (default `true`). When `true`, factory shutdown runs the full teardown: -identity-based kill via `killAgentByRecord(readAgentRecord(stateDir))` from -`browse/src/terminal-agent-control.ts` plus `safeUnlinkQuiet` on -`/terminal-port`, `/terminal-internal-token`, and -`/terminal-agent-pid` (the per-boot agent record introduced in v1.44). +boolean` (default `true`). When `true`, factory shutdown acquires +`acquireAgentStateLock(stateDir)` and checks daemon ownership. It removes +`terminal-port`, `terminal-internal-token`, and the matching `terminal-agent-pid` +record only when the recorded agent is absent, already dead, or confirmed stopped +by `stopAgentByRecord`. Uncertain identity or exit, an unavailable lock, or +successor-owned daemon state leaves those files intact. Embedders (e.g. the gbrowser phoenix overlay) that pre-launch their own PTY server must pass `false` so their discovery files survive gstack teardown cycles. The flag is the third caller-owned teardown gate in `ServerConfig` (alongside diff --git a/lib/cso/state.ts b/lib/cso/state.ts index 58743455c..39e889a22 100644 --- a/lib/cso/state.ts +++ b/lib/cso/state.ts @@ -6,51 +6,58 @@ import { CsoError, RunReportV3, canonical, completeness, fingerprint, renderRepo import { redact, sanitizeForJson, sanitizeHelperForJson } from './process'; const MAX_STATE_FILE=1024*1024; -type AtomicRecoveryIdentity={dev:number;ino:number;nlink:number;size:number;mode:number;uid:number;mtimeMs:number;ctimeMs:number}; +type ExactStats=Pick & Pick; +function exactStats(stat:fs.BigIntStats):ExactStats{ + for(const value of [stat.nlink,stat.size,stat.mode,stat.uid])if(value>BigInt(Number.MAX_SAFE_INTEGER)||value< -BigInt(Number.MAX_SAFE_INTEGER))throw new CsoError('UNSAFE_PATH','Filesystem metadata exceeds safe bounds'); + return{dev:stat.dev,ino:stat.ino,mtimeNs:stat.mtimeNs,ctimeNs:stat.ctimeNs,nlink:Number(stat.nlink),size:Number(stat.size),mode:Number(stat.mode),uid:Number(stat.uid),isFile:()=>stat.isFile(),isSymbolicLink:()=>stat.isSymbolicLink(),isDirectory:()=>stat.isDirectory()}; +} +function exactLstat(path:string):ExactStats{return exactStats(fs.lstatSync(path,{bigint:true}));} +function exactFstat(fd:number):ExactStats{return exactStats(fs.fstatSync(fd,{bigint:true}));} +type AtomicRecoveryIdentity={dev:bigint;ino:bigint;nlink:number;size:number;mode:number;uid:number;mtimeNs:bigint;ctimeNs:bigint}; export interface AtomicNoReplaceRecoveryOptions { label:string;maxBytes:number; validate?:(value:unknown,publisherPid:number)=>void; publisherAlive?:(value:unknown,publisherPid:number)=>boolean; } class AtomicPublicationTransition extends CsoError { constructor(message:string){super('SNAPSHOT_RACE',message);this.name='AtomicPublicationTransition';} } -function recoveryIdentity(stat:fs.Stats):AtomicRecoveryIdentity{return{dev:stat.dev,ino:stat.ino,nlink:stat.nlink,size:stat.size,mode:stat.mode,uid:stat.uid,mtimeMs:stat.mtimeMs,ctimeMs:stat.ctimeMs};} -function sameRecoveryIdentity(left:AtomicRecoveryIdentity,right:AtomicRecoveryIdentity):boolean{return left.dev===right.dev&&left.ino===right.ino&&left.nlink===right.nlink&&left.size===right.size&&left.mode===right.mode&&left.uid===right.uid&&left.mtimeMs===right.mtimeMs&&left.ctimeMs===right.ctimeMs;} +function recoveryIdentity(stat:ExactStats):AtomicRecoveryIdentity{return{dev:stat.dev,ino:stat.ino,nlink:stat.nlink,size:stat.size,mode:stat.mode,uid:stat.uid,mtimeNs:stat.mtimeNs,ctimeNs:stat.ctimeNs};} +function sameRecoveryIdentity(left:AtomicRecoveryIdentity,right:AtomicRecoveryIdentity):boolean{return left.dev===right.dev&&left.ino===right.ino&&left.nlink===right.nlink&&left.size===right.size&&left.mode===right.mode&&left.uid===right.uid&&left.mtimeNs===right.mtimeNs&&left.ctimeNs===right.ctimeNs;} function recoveryProcessAlive(pid:number):boolean{try{process.kill(pid,0);return true;}catch(error:any){return error?.code==='EPERM';}} function liveRecognizedPublication(temp:string,target:string,pid:number,options:AtomicNoReplaceRecoveryOptions):boolean{ if(!recoveryProcessAlive(pid))return false; - try{const temporary=fs.lstatSync(temp);if(temporary.isSymbolicLink()||!temporary.isFile()||temporary.nlink<1||temporary.nlink>2||(process.getuid&&temporary.uid!==process.getuid())||(process.platform!=='win32'&&(temporary.mode&0o077)!==0))return false;if(temporary.size===0)return temporary.nlink===1;if(temporary.nlink!==2||!privatePublicationFile(temporary,options))return false;const published=fs.lstatSync(target);return published.nlink===2&&samePublicationInode(temporary,published,options);}catch{return false;} + try{const temporary=exactLstat(temp);if(temporary.isSymbolicLink()||!temporary.isFile()||temporary.nlink<1||temporary.nlink>2||(process.getuid&&temporary.uid!==process.getuid())||(process.platform!=='win32'&&(temporary.mode&0o077)!==0))return false;if(temporary.size===0)return temporary.nlink===1;if(temporary.nlink!==2||!privatePublicationFile(temporary,options))return false;const published=exactLstat(target);return published.nlink===2&&samePublicationInode(temporary,published,options);}catch{return false;} } -function liveEmptyPublication(path:string,pid:number):boolean{if(!recoveryProcessAlive(pid))return false;try{const stat=fs.lstatSync(path);return stat.isFile()&&!stat.isSymbolicLink()&&stat.size===0&&stat.nlink===1&&(!process.getuid||stat.uid===process.getuid())&&(process.platform==='win32'||(stat.mode&0o077)===0);}catch{return false;}} -function privatePublicationObservation(stat:fs.Stats,options:AtomicNoReplaceRecoveryOptions):boolean{return stat.isFile()&&!stat.isSymbolicLink()&&stat.size>=0&&stat.size<=options.maxBytes&&stat.nlink>=1&&stat.nlink<=2&&(!process.getuid||stat.uid===process.getuid())&&(process.platform==='win32'||(stat.mode&0o077)===0);} -function livePublicationAdvanced(temp:string,pid:number,observed:fs.Stats|undefined,options:AtomicNoReplaceRecoveryOptions):boolean{ +function liveEmptyPublication(path:string,pid:number):boolean{if(!recoveryProcessAlive(pid))return false;try{const stat=exactLstat(path);return stat.isFile()&&!stat.isSymbolicLink()&&stat.size===0&&stat.nlink===1&&(!process.getuid||stat.uid===process.getuid())&&(process.platform==='win32'||(stat.mode&0o077)===0);}catch{return false;}} +function privatePublicationObservation(stat:ExactStats,options:AtomicNoReplaceRecoveryOptions):boolean{return stat.isFile()&&!stat.isSymbolicLink()&&stat.size>=0&&stat.size<=options.maxBytes&&stat.nlink>=1&&stat.nlink<=2&&(!process.getuid||stat.uid===process.getuid())&&(process.platform==='win32'||(stat.mode&0o077)===0);} +function livePublicationAdvanced(temp:string,pid:number,observed:ExactStats|undefined,options:AtomicNoReplaceRecoveryOptions):boolean{ if(!observed||!privatePublicationObservation(observed,options)||!recoveryProcessAlive(pid))return false; Atomics.wait(LEASE_ELECTION_WAIT,0,0,LEASE_ELECTION_POLL_MS); - let current:fs.Stats;try{current=fs.lstatSync(temp);}catch(error:any){return error?.code==='ENOENT';} + let current:ExactStats;try{current=exactLstat(temp);}catch(error:any){return error?.code==='ENOENT';} if(!privatePublicationObservation(current,options)||current.dev!==observed.dev||current.ino!==observed.ino)return false; if(current.nlink!==observed.nlink||current.size!==observed.size)return true; return false; } function publicationOwnerAlive(value:unknown,publisherPid:number,options:AtomicNoReplaceRecoveryOptions):boolean{return options.publisherAlive?.(value,publisherPid)??recoveryProcessAlive(publisherPid);} -function privatePublicationFile(stat:fs.Stats,options:AtomicNoReplaceRecoveryOptions):boolean{return stat.isFile()&&!stat.isSymbolicLink()&&stat.size>0&&stat.size<=options.maxBytes&& +function privatePublicationFile(stat:ExactStats,options:AtomicNoReplaceRecoveryOptions):boolean{return stat.isFile()&&!stat.isSymbolicLink()&&stat.size>0&&stat.size<=options.maxBytes&& (!process.getuid||stat.uid===process.getuid())&&(process.platform==='win32'||(stat.mode&0o077)===0);} -function samePublicationObject(left:fs.Stats,right:fs.Stats,options:AtomicNoReplaceRecoveryOptions):boolean{return privatePublicationFile(left,options)&&privatePublicationFile(right,options)&& +function samePublicationObject(left:ExactStats,right:ExactStats,options:AtomicNoReplaceRecoveryOptions):boolean{return privatePublicationFile(left,options)&&privatePublicationFile(right,options)&& left.dev===right.dev&&left.ino===right.ino&&left.size===right.size&&left.mode===right.mode&&left.uid===right.uid;} -function samePublicationInode(left:fs.Stats,right:fs.Stats,options:AtomicNoReplaceRecoveryOptions):boolean{return samePublicationObject(left,right,options)&&left.mtimeMs===right.mtimeMs;} -function publicationLinkTransition(observed:fs.Stats,current:fs.Stats,links:1|2,options:AtomicNoReplaceRecoveryOptions):boolean{ +function samePublicationInode(left:ExactStats,right:ExactStats,options:AtomicNoReplaceRecoveryOptions):boolean{return samePublicationObject(left,right,options)&&left.mtimeNs===right.mtimeNs;} +function publicationLinkTransition(observed:ExactStats,current:ExactStats,links:1|2,options:AtomicNoReplaceRecoveryOptions):boolean{ const from=links===1?1:2,to=links===1?2:1; return observed.nlink===from&¤t.nlink===to&&samePublicationInode(observed,current,options); } -function publicationPathRemoved(observed:fs.Stats,current:fs.Stats,options:AtomicNoReplaceRecoveryOptions):boolean{return observed.nlink>=1&&observed.nlink<=2&¤t.nlink>=0&¤t.nlink=0&&left.nlink<=2&&right.nlink>=0&&right.nlink<=2&&left.nlink!==right.nlink&&samePublicationInode(left,right,options);} +function publicationPathRemoved(observed:ExactStats,current:ExactStats,options:AtomicNoReplaceRecoveryOptions):boolean{return observed.nlink>=1&&observed.nlink<=2&¤t.nlink>=0&¤t.nlink=0&&left.nlink<=2&&right.nlink>=0&&right.nlink<=2&&left.nlink!==right.nlink&&samePublicationInode(left,right,options);} function atomicTempTarget(path:string,publisherPid?:number):{target:string;pid:number}|undefined{ const match=basename(path).match(/^(.*)\.tmp\.(\d{1,10})\.[a-f0-9]{8}$/),pid=match?Number(match[2]):0; return match&&match[1]&&Number.isSafeInteger(pid)&&pid>1&&(publisherPid===undefined||pid===publisherPid)?{target:join(dirname(path),match[1]),pid}:undefined; } -function settledAtomicTemp(path:string,observed:fs.Stats,options:AtomicNoReplaceRecoveryOptions):boolean{ +function settledAtomicTemp(path:string,observed:ExactStats,options:AtomicNoReplaceRecoveryOptions):boolean{ const publication=atomicTempTarget(path);if(!publication)return false; - let target:fs.Stats;try{target=fs.lstatSync(publication.target);}catch{return false;} + let target:ExactStats;try{target=exactLstat(publication.target);}catch{return false;} return observed.nlink>=1&&observed.nlink<=2&&target.nlink===1&&privatePublicationFile(observed,options)&&privatePublicationFile(target,options)&& - observed.dev===target.dev&&observed.ino===target.ino&&observed.size===target.size&&observed.mode===target.mode&&observed.uid===target.uid&&observed.mtimeMs===target.mtimeMs; + observed.dev===target.dev&&observed.ino===target.ino&&observed.size===target.size&&observed.mode===target.mode&&observed.uid===target.uid&&observed.mtimeNs===target.mtimeNs; } function readPublicationBytes(fd:number,size:number,label:string):string{ const bytes=Buffer.alloc(size);let offset=0; @@ -58,12 +65,12 @@ function readPublicationBytes(fd:number,size:number,label:string):string{ const extra=Buffer.alloc(1);if(fs.readSync(fd,extra,0,1,size)!==0)throw new CsoError('SNAPSHOT_RACE',`${label} interrupted publication changed while it was read`); return bytes.toString('utf8'); } -function recoveryJson(path:string,links:1|2,options:AtomicNoReplaceRecoveryOptions,observed?:fs.Stats):{identity:AtomicRecoveryIdentity;value:unknown}{ +function recoveryJson(path:string,links:1|2,options:AtomicNoReplaceRecoveryOptions,observed?:ExactStats):{identity:AtomicRecoveryIdentity;value:unknown}{ let fd:number|undefined; try{ - const before=fs.lstatSync(path); + const before=exactLstat(path); if(before.nlink===0){ - let current:fs.Stats;try{current=fs.lstatSync(path);}catch(error:any){if(error?.code==='ENOENT')throw new CsoError('INSUFFICIENT_CAPACITY',`${options.label} was removed while it was inspected`);throw error;} + let current:ExactStats;try{current=exactLstat(path);}catch(error:any){if(error?.code==='ENOENT')throw new CsoError('INSUFFICIENT_CAPACITY',`${options.label} was removed while it was inspected`);throw error;} if(samePublicationInode(before,current,options)&&(current.nlink===0||current.nlink===links))throw new AtomicPublicationTransition(`${options.label} changed link state while it was inspected`); throw new CsoError('UNSAFE_PATH',`${options.label} was replaced while it was inspected`); } @@ -72,14 +79,14 @@ function recoveryJson(path:string,links:1|2,options:AtomicNoReplaceRecoveryOptio if(!before.isFile()||before.isSymbolicLink()||before.nlink!==links||before.size<=0||before.size>options.maxBytes|| (process.getuid&&before.uid!==process.getuid())||(process.platform!=='win32'&&(before.mode&0o077)!==0)) throw new CsoError('UNSAFE_PATH',`${options.label} interrupted publication is not one private regular file`); - fd=fs.openSync(path,fs.constants.O_RDONLY|(fs.constants.O_NOFOLLOW??0));const opened=fs.fstatSync(fd); + fd=fs.openSync(path,fs.constants.O_RDONLY|(fs.constants.O_NOFOLLOW??0));const opened=exactFstat(fd); if(!sameRecoveryIdentity(recoveryIdentity(before),recoveryIdentity(opened))){ if(publicationLinkTransition(before,opened,links,options))throw new AtomicPublicationTransition(`${options.label} interrupted publication changed link state while it was opened`); if(publicationProgress(before,opened,options))throw new CsoError('INSUFFICIENT_CAPACITY',`${options.label} changed phase during concurrent recovery while it was opened`); throw new CsoError('SNAPSHOT_RACE',`${options.label} interrupted publication changed while it was opened`); } const serialized=readPublicationBytes(fd,opened.size,options.label);let value:unknown;try{value=JSON.parse(serialized);}catch{throw new CsoError('UNSAFE_PATH',`${options.label} interrupted publication is not valid JSON`);} - const final=fs.fstatSync(fd);if(readPublicationBytes(fd,opened.size,options.label)!==serialized)throw new CsoError('SNAPSHOT_RACE',`${options.label} interrupted publication changed while it was read`);let after:fs.Stats;try{after=fs.lstatSync(path);}catch(error:any){if(error?.code==='ENOENT'&&publicationPathRemoved(opened,final,options))throw new CsoError('INSUFFICIENT_CAPACITY',`${options.label} was removed by another recovery helper while it was read`);throw error;}const openedIdentity=recoveryIdentity(opened),finalIdentity=recoveryIdentity(final),afterIdentity=recoveryIdentity(after); + const final=exactFstat(fd);if(readPublicationBytes(fd,opened.size,options.label)!==serialized)throw new CsoError('SNAPSHOT_RACE',`${options.label} interrupted publication changed while it was read`);let after:ExactStats;try{after=exactLstat(path);}catch(error:any){if(error?.code==='ENOENT'&&publicationPathRemoved(opened,final,options))throw new CsoError('INSUFFICIENT_CAPACITY',`${options.label} was removed by another recovery helper while it was read`);throw error;}const openedIdentity=recoveryIdentity(opened),finalIdentity=recoveryIdentity(final),afterIdentity=recoveryIdentity(after); if(!sameRecoveryIdentity(openedIdentity,finalIdentity)||!sameRecoveryIdentity(openedIdentity,afterIdentity)){ const coherentTransition=(sameRecoveryIdentity(openedIdentity,finalIdentity)&&publicationLinkTransition(opened,after,links,options))|| (publicationLinkTransition(opened,final,links,options)&&sameRecoveryIdentity(finalIdentity,afterIdentity)); @@ -95,23 +102,23 @@ function atomicTempCandidates(target:string):Array<{path:string;pid:number}>{ const directory=dirname(target),name=basename(target),escaped=name.replace(/[.*+?^${}()|[\]\\]/g,'\\$&'),pattern=new RegExp(`^${escaped}\\.tmp\\.(\\d{1,10})\\.([a-f0-9]{8})$`); return fs.readdirSync(directory).flatMap(entry=>{const match=entry.match(pattern),pid=match?Number(match[1]):0;return match&&Number.isSafeInteger(pid)&&pid>1?[{path:join(directory,entry),pid}]:[];}); } -function matchesRecoveryInode(stat:fs.Stats,identity:AtomicRecoveryIdentity,options:AtomicNoReplaceRecoveryOptions):boolean{return privatePublicationFile(stat,options)&&stat.dev===identity.dev&&stat.ino===identity.ino&&stat.size===identity.size&&stat.mode===identity.mode&&stat.uid===identity.uid&&stat.mtimeMs===identity.mtimeMs;} +function matchesRecoveryInode(stat:ExactStats,identity:AtomicRecoveryIdentity,options:AtomicNoReplaceRecoveryOptions):boolean{return privatePublicationFile(stat,options)&&stat.dev===identity.dev&&stat.ino===identity.ino&&stat.size===identity.size&&stat.mode===identity.mode&&stat.uid===identity.uid&&stat.mtimeNs===identity.mtimeNs;} /** Recover only the hard-link publication window of atomicWriteSync(noReplace). */ export function recoverAtomicNoReplaceJson(target:string,options:AtomicNoReplaceRecoveryOptions):void{ - let targetStat:fs.Stats;try{targetStat=fs.lstatSync(target);}catch(error:any){if(error?.code==='ENOENT')return;throw new CsoError('UNSAFE_PATH',`${options.label} could not be inspected`);} + let targetStat:ExactStats;try{targetStat=exactLstat(target);}catch(error:any){if(error?.code==='ENOENT')return;throw new CsoError('UNSAFE_PATH',`${options.label} could not be inspected`);} // Callers own legacy-directory and special-file handling. Only a regular // file can be the no-replace hard-link publication this helper recognizes. if(!targetStat.isFile()||targetStat.isSymbolicLink())return; if(targetStat.nlink===1)return; if(targetStat.nlink===0){ - let current:fs.Stats;try{current=fs.lstatSync(target);}catch(error:any){if(error?.code==='ENOENT')throw new AtomicPublicationTransition(`${options.label} was removed while it was inspected`);throw new CsoError('UNSAFE_PATH',`${options.label} could not be reinspected`);} + let current:ExactStats;try{current=exactLstat(target);}catch(error:any){if(error?.code==='ENOENT')throw new AtomicPublicationTransition(`${options.label} was removed while it was inspected`);throw new CsoError('UNSAFE_PATH',`${options.label} could not be reinspected`);} if(samePublicationInode(targetStat,current,options)&¤t.nlink>=0&¤t.nlink<=2)throw new AtomicPublicationTransition(`${options.label} changed link state while it was inspected`); throw new CsoError('UNSAFE_PATH',`${options.label} was replaced while it was inspected`); } if(targetStat.nlink!==2)throw new CsoError('UNSAFE_PATH',`${options.label} has an unrecognized hard-link count`); - const canonical=recoveryJson(target,2,options,targetStat),matches=atomicTempCandidates(target).flatMap(candidate=>{try{const observed=fs.lstatSync(candidate.path);return observed.dev===canonical.identity.dev&&observed.ino===canonical.identity.ino?[{...candidate,observed}]:[];}catch{return[];}}); + const canonical=recoveryJson(target,2,options,targetStat),matches=atomicTempCandidates(target).flatMap(candidate=>{try{const observed=exactLstat(candidate.path);return observed.dev===canonical.identity.dev&&observed.ino===canonical.identity.ino?[{...candidate,observed}]:[];}catch{return[];}}); if(matches.length!==1){ - let settled:fs.Stats|undefined;try{settled=fs.lstatSync(target);}catch(error:any){ + let settled:ExactStats|undefined;try{settled=exactLstat(target);}catch(error:any){ if(matches.length===0&&error?.code==='ENOENT')throw new AtomicPublicationTransition(`${options.label} was removed during candidate enumeration`); } if(settled&&publicationLinkTransition(targetStat,settled,2,options))throw new AtomicPublicationTransition(`${options.label} interrupted publication settled during candidate enumeration`); @@ -121,10 +128,10 @@ export function recoverAtomicNoReplaceJson(target:string,options:AtomicNoReplace if(!sameRecoveryIdentity(canonical.identity,temporary.identity))throw new CsoError('UNSAFE_PATH',`${options.label} hard link changed identity`); options.validate?.(canonical.value,candidate.pid);options.validate?.(temporary.value,candidate.pid); if(publicationOwnerAlive(canonical.value,candidate.pid,options))throw new CsoError('INSUFFICIENT_CAPACITY',`${options.label} publication is still owned by a live helper`); - let finalTarget:fs.Stats,finalTemp:fs.Stats; - try{finalTarget=fs.lstatSync(target);finalTemp=fs.lstatSync(candidate.path);}catch(error:any){ + let finalTarget:ExactStats,finalTemp:ExactStats; + try{finalTarget=exactLstat(target);finalTemp=exactLstat(candidate.path);}catch(error:any){ if(error?.code!=='ENOENT')throw error; - for(const path of [target,candidate.path]){try{const stat=fs.lstatSync(path);if(!matchesRecoveryInode(stat,canonical.identity,options))throw new CsoError('UNSAFE_PATH',`${options.label} was replaced during concurrent recovery`);}catch(recoveryError:any){if(recoveryError instanceof CsoError)throw recoveryError;if(recoveryError?.code!=='ENOENT')throw recoveryError;}} + for(const path of [target,candidate.path]){try{const stat=exactLstat(path);if(!matchesRecoveryInode(stat,canonical.identity,options))throw new CsoError('UNSAFE_PATH',`${options.label} was replaced during concurrent recovery`);}catch(recoveryError:any){if(recoveryError instanceof CsoError)throw recoveryError;if(recoveryError?.code!=='ENOENT')throw recoveryError;}} throw new AtomicPublicationTransition(`${options.label} was settled by another recovery helper`); } if(!sameRecoveryIdentity(canonical.identity,recoveryIdentity(finalTarget))||!sameRecoveryIdentity(canonical.identity,recoveryIdentity(finalTemp))){ @@ -142,7 +149,7 @@ export function recoverAtomicNoReplaceJson(target:string,options:AtomicNoReplace /** Remove a never-published temp, or validate a temp that became published while observed. */ export function discardAtomicNoReplaceTemp(path:string,publisherPid:number,options:AtomicNoReplaceRecoveryOptions):void{ - let observed:fs.Stats;try{observed=fs.lstatSync(path);}catch(error:any){ + let observed:ExactStats;try{observed=exactLstat(path);}catch(error:any){ if(error?.code==='ENOENT'){ const publication=atomicTempTarget(path,publisherPid); if(publication){ @@ -154,7 +161,7 @@ export function discardAtomicNoReplaceTemp(path:string,publisherPid:number,optio } if(observed.nlink===2&&privatePublicationFile(observed,options)){ const target=atomicTempTarget(path,publisherPid)?.target; - let published:fs.Stats|undefined;try{if(target)published=fs.lstatSync(target);}catch{} + let published:ExactStats|undefined;try{if(target)published=exactLstat(target);}catch{} if(target&&published&&published.dev===observed.dev&&published.ino===observed.ino&&published.nlink===2&&privatePublicationFile(published,options)){ recoverAtomicNoReplaceJson(target,options); const settled=recoveryJson(target,1,options); @@ -166,7 +173,7 @@ export function discardAtomicNoReplaceTemp(path:string,publisherPid:number,optio } const temporary=recoveryJson(path,1,options,observed);options.validate?.(temporary.value,publisherPid); if(publicationOwnerAlive(temporary.value,publisherPid,options))throw new CsoError('INSUFFICIENT_CAPACITY',`${options.label} publication is still owned by a live helper`); - let final:fs.Stats;try{final=fs.lstatSync(path);}catch(error:any){if(error?.code==='ENOENT')throw new AtomicPublicationTransition(`${options.label} temp was removed by another recovery helper`);throw error;} + let final:ExactStats;try{final=exactLstat(path);}catch(error:any){if(error?.code==='ENOENT')throw new AtomicPublicationTransition(`${options.label} temp was removed by another recovery helper`);throw error;} if(!sameRecoveryIdentity(temporary.identity,recoveryIdentity(final)))throw new CsoError('SNAPSHOT_RACE',`${options.label} temp changed before recovery`); try{fs.unlinkSync(path);}catch(error:any){if(error?.code==='ENOENT')throw new AtomicPublicationTransition(`${options.label} temp was removed by another recovery helper`);throw new CsoError('PERSISTENCE_FAILED',`${options.label} unpublished temp could not be removed`);} } @@ -253,16 +260,16 @@ export function writeJsonExclusive(path:string,value:unknown):void{ function readPrivateJson(path:string):unknown{ let fd:number|undefined; try{ - const before=fs.lstatSync(path); + const before=exactLstat(path); if(!before.isFile()||before.isSymbolicLink()||before.nlink!==1||before.size<=0||before.size>MAX_STATE_FILE|| (process.getuid&&before.uid!==process.getuid())||(process.platform!=='win32'&&(before.mode&0o077)!==0)) throw new CsoError('UNSAFE_PATH','Invalid private state file'); fd=fs.openSync(path,fs.constants.O_RDONLY|(fs.constants.O_NOFOLLOW??0)); - const opened=fs.fstatSync(fd); + const opened=exactFstat(fd); if(!sameRecoveryIdentity(recoveryIdentity(before),recoveryIdentity(opened))) throw new CsoError('SNAPSHOT_RACE','Private state file changed while it was opened'); const raw=fs.readFileSync(fd,'utf8'); - const final=fs.fstatSync(fd),after=fs.lstatSync(path); + const final=exactFstat(fd),after=exactLstat(path); if(!sameRecoveryIdentity(recoveryIdentity(opened),recoveryIdentity(final))|| !sameRecoveryIdentity(recoveryIdentity(opened),recoveryIdentity(after))) throw new CsoError('SNAPSHOT_RACE','Private state file changed while it was read'); @@ -315,7 +322,7 @@ const LEASE_CANDIDATE=/^([a-f0-9]{32})\.json$/; const LEASE_DECISION=/^([a-f0-9]{32})\.decision$/; const LEASE_ACTIVE=/^([a-f0-9]{32})\.active\.([a-f0-9]{16})$/; type LockOwner={pid:number;processIdentity?:string;token:string;createdAt:number}; -type LockIdentity={dev:number;ino:number}; +type LockIdentity={dev:bigint;ino:bigint}; type LeaseLinks=1|2; type LeaseDecision={schemaVersion:1;token:string;kind:'ticket'|'withdraw';ticket?:string;candidateDev:string;candidateIno:string;ownerPid:number;ownerProcessIdentity?:string;ownerCreatedAt:number;publisherPid:number;publisherProcessIdentity?:string;createdAt:number}; function processAlive(pid:number):boolean{if(!Number.isInteger(pid)||pid<=1)return false;try{process.kill(pid,0);return true;}catch(error:any){return error?.code==='EPERM';}} @@ -334,8 +341,8 @@ function validateLeaseDecision(value:unknown,expectedToken?:string):LeaseDecisio const decision=value as Record,kind=decision.kind,ticket=decision.ticket; if(decision.schemaVersion!==1||typeof decision.token!=='string'||!LOCK_TOKEN.test(decision.token)||(expectedToken!==undefined&&decision.token!==expectedToken)|| (kind!=='ticket'&&kind!=='withdraw')||(kind==='ticket'&&(typeof ticket!=='string'||!/^[a-f0-9]{16}$/.test(ticket)||ticket==='0000000000000000'))||(kind==='withdraw'&&ticket!==undefined)|| - typeof decision.candidateDev!=='string'||!/^\d+$/.test(decision.candidateDev)||!Number.isSafeInteger(Number(decision.candidateDev))|| - typeof decision.candidateIno!=='string'||!/^\d+$/.test(decision.candidateIno)||!Number.isSafeInteger(Number(decision.candidateIno))|| + typeof decision.candidateDev!=='string'||!/^(0|[1-9]\d*)$/.test(decision.candidateDev)||BigInt(decision.candidateDev)>0xffffffffffffffffn|| + typeof decision.candidateIno!=='string'||!/^(0|[1-9]\d*)$/.test(decision.candidateIno)||BigInt(decision.candidateIno)>0xffffffffffffffffn|| !Number.isInteger(decision.ownerPid)||Number(decision.ownerPid)<=1||!Number.isFinite(decision.ownerCreatedAt)||Number(decision.ownerCreatedAt)<0|| !Number.isInteger(decision.publisherPid)||Number(decision.publisherPid)<=1||!Number.isFinite(decision.createdAt)||Number(decision.createdAt)<0|| (decision.ownerProcessIdentity!==undefined&&(typeof decision.ownerProcessIdentity!=='string'||!PROCESS_IDENTITY.test(decision.ownerProcessIdentity)))|| @@ -347,23 +354,23 @@ function decisionPublisher(decision:LeaseDecision):LockOwner{return{pid:decision function leaseDecisionRecoveryOptions(token:string):AtomicNoReplaceRecoveryOptions{return{label:'Run mutation lease decision',maxBytes:LOCK_OWNER_MAX_BYTES, validate:(value,pid)=>{const decision=validateLeaseDecision(value,token);if(decision.publisherPid!==pid)throw new CsoError('UNSAFE_PATH','Run mutation lease decision temp does not match its publisher');}, publisherAlive:(value,pid)=>{const decision=validateLeaseDecision(value,token);if(decision.publisherPid!==pid)throw new CsoError('UNSAFE_PATH','Run mutation lease decision temp does not match its publisher');return ownerIsAlive(decisionPublisher(decision));}};} -function ownerLinkTransition(left:fs.Stats,right:fs.Stats):boolean{return left.isFile()&&right.isFile()&&left.dev===right.dev&&left.ino===right.ino&&left.size===right.size&&left.mode===right.mode&&left.uid===right.uid&& +function ownerLinkTransition(left:ExactStats,right:ExactStats):boolean{return left.isFile()&&right.isFile()&&left.dev===right.dev&&left.ino===right.ino&&left.size===right.size&&left.mode===right.mode&&left.uid===right.uid&& left.nlink>=0&&left.nlink<=2&&right.nlink>=0&&right.nlink<=2&&left.nlink!==right.nlink;} -function readOwner(path:string,expectedToken?:string,expectedLinks:LeaseLinks=1,observed?:fs.Stats):{owner:LockOwner;identity:LockIdentity}{ +function readOwner(path:string,expectedToken?:string,expectedLinks:LeaseLinks=1,observed?:ExactStats):{owner:LockOwner;identity:LockIdentity}{ let fd:number|undefined; try{ - const before=fs.lstatSync(path); + const before=exactLstat(path); if(observed&&ownerLinkTransition(observed,before))throw new CsoError('INSUFFICIENT_CAPACITY','Run mutation lease changed phase while it was read'); if(before.isSymbolicLink()||!before.isFile()||before.nlink!==expectedLinks||before.size<=0||before.size>LOCK_OWNER_MAX_BYTES|| (process.getuid&&before.uid!==process.getuid())||(process.platform!=='win32'&&(before.mode&0o077)!==0)) throw new CsoError('UNSAFE_PATH','Run mutation lease is invalid'); fd=fs.openSync(path,fs.constants.O_RDONLY|(fs.constants.O_NOFOLLOW??0)); - const opened=fs.fstatSync(fd); + const opened=exactFstat(fd); if(ownerLinkTransition(before,opened))throw new CsoError('INSUFFICIENT_CAPACITY','Run mutation lease changed phase while it was read'); if(!opened.isFile()||opened.dev!==before.dev||opened.ino!==before.ino||opened.nlink!==expectedLinks||opened.size!==before.size) throw new CsoError('UNSAFE_PATH','Run mutation lease changed while it was read'); let parsed:unknown;try{parsed=JSON.parse(fs.readFileSync(fd,'utf8'));}catch{throw new CsoError('UNSAFE_PATH','Run mutation lease is malformed');} - const final=fs.fstatSync(fd),after=fs.lstatSync(path); + const final=exactFstat(fd),after=exactLstat(path); const coherentTransition=(ownerLinkTransition(opened,final)&&final.dev===after.dev&&final.ino===after.ino&&final.nlink===after.nlink)|| (opened.dev===final.dev&&opened.ino===final.ino&&opened.nlink===final.nlink&&ownerLinkTransition(opened,after)); if(coherentTransition)throw new CsoError('INSUFFICIENT_CAPACITY','Run mutation lease changed phase while it was read'); @@ -390,7 +397,7 @@ function recoverLeasePublications(leases:string):void{ validate:(value,pid)=>{const owner=validateOwner(value,token);if(owner.pid!==pid)throw new CsoError('UNSAFE_PATH','Run mutation lease temp does not match its publisher');}, publisherAlive:(value,pid)=>{const owner=validateOwner(value,token);if(owner.pid!==pid)throw new CsoError('UNSAFE_PATH','Run mutation lease temp does not match its publisher');return ownerIsAlive(owner);}}: leaseDecisionRecoveryOptions(token); - let publicationObserved:fs.Stats|undefined;try{publicationObserved=fs.lstatSync(temp);}catch{} + let publicationObserved:ExactStats|undefined;try{publicationObserved=exactLstat(temp);}catch{} if(liveEmptyPublication(temp,publisherPid))throw new CsoError('INSUFFICIENT_CAPACITY',`${options.label} publication is still changing under a live helper`); try{ if(fs.existsSync(target))recoverAtomicNoReplaceJson(target,options); @@ -415,14 +422,14 @@ function recoverLeasePublications(leases:string):void{ function readLegacyOwner(path:string):{pid:number;processIdentity?:string;token:string;createdAt:number}{ let fd:number|undefined; try{ - const before=fs.lstatSync(path); + const before=exactLstat(path); if(before.isSymbolicLink()||!before.isFile()||before.nlink!==1||before.size<=0||before.size>LOCK_OWNER_MAX_BYTES||(process.getuid&&before.uid!==process.getuid())) throw new CsoError('UNSAFE_PATH','Legacy run mutation lock owner is invalid'); fd=fs.openSync(path,fs.constants.O_RDONLY|(fs.constants.O_NOFOLLOW??0)); - const opened=fs.fstatSync(fd); + const opened=exactFstat(fd); if(opened.dev!==before.dev||opened.ino!==before.ino||opened.nlink!==1)throw new CsoError('UNSAFE_PATH','Legacy run mutation lock owner changed while it was read'); let value:unknown;try{value=JSON.parse(fs.readFileSync(fd,'utf8'));}catch{throw new CsoError('UNSAFE_PATH','Legacy run mutation lock owner is malformed');} - const after=fs.lstatSync(path),record=value as Record; + const after=exactLstat(path),record=value as Record; if(after.dev!==opened.dev||after.ino!==opened.ino||!record||typeof record!=='object'||Array.isArray(record)||!Number.isInteger(record.pid)||Number(record.pid)<=1|| typeof record.token!=='string'||record.token.length<1||record.token.length>256|| (record.processIdentity!==undefined&&(typeof record.processIdentity!=='string'||!PROCESS_IDENTITY.test(record.processIdentity)))) @@ -440,7 +447,7 @@ function exactUnlink(path:string,token:string,identity:LockIdentity,links:LeaseL if(current.identity.dev!==identity.dev||current.identity.ino!==identity.ino)throw new CsoError('PERSISTENCE_FAILED','Run mutation lease ownership changed before exact release'); // One final pathname check narrows lstat/read/unlink replacement races. Lease // names are immutable and never reused by cooperating helpers. - let final:fs.Stats;try{final=fs.lstatSync(path);}catch{throw new CsoError('PERSISTENCE_FAILED','Run mutation lease ownership changed before exact release');} + let final:ExactStats;try{final=exactLstat(path);}catch{throw new CsoError('PERSISTENCE_FAILED','Run mutation lease ownership changed before exact release');} if(final.isSymbolicLink()||final.dev!==identity.dev||final.ino!==identity.ino||final.nlink!==links)throw new CsoError('PERSISTENCE_FAILED','Run mutation lease ownership changed before exact release'); try{fs.unlinkSync(path);}catch{throw new CsoError('PERSISTENCE_FAILED','Run mutation lease ownership changed before exact release');} } @@ -471,7 +478,7 @@ function ensureLockProtocol(dir:string):string{ if(error?.code!=='EEXIST')throw new CsoError('PERSISTENCE_FAILED','Run mutation lock protocol could not be initialized'); recoverAtomicNoReplaceJson(lock,{label:'Run mutation lock protocol',maxBytes:LOCK_OWNER_MAX_BYTES, validate:value=>{if(!value||typeof value!=='object'||Array.isArray(value)||(value as any).protocol!==LOCK_PROTOCOL)throw new CsoError('UNSAFE_PATH','Run mutation lock protocol is invalid');}}); - const stat=fs.lstatSync(lock); + const stat=exactLstat(lock); if(stat.isSymbolicLink())throw new CsoError('UNSAFE_PATH','Run mutation lock is a symlink'); if(stat.isFile()){ let protocol='';try{if(stat.nlink!==1||stat.size<=0||stat.size>LOCK_OWNER_MAX_BYTES||(process.platform!=='win32'&&(stat.mode&0o077)!==0))throw new Error('invalid');protocol=JSON.parse(fs.readFileSync(lock,'utf8')).protocol;}catch{} @@ -482,7 +489,7 @@ function ensureLockProtocol(dir:string):string{ // so it is never age-reclaimed. Fully published dead owners can migrate. const owner=readLegacyOwner(join(lock,'owner.json')); if(ownerIsAlive(owner as LockOwner))throw new CsoError('INSUFFICIENT_CAPACITY','Another helper is updating this run'); - const migration=join(lock,'.v3-migration'),claim=acquireMigrationClaim(migration),current=fs.lstatSync(lock); + const migration=join(lock,'.v3-migration'),claim=acquireMigrationClaim(migration),current=exactLstat(lock); if(current.dev!==stat.dev||current.ino!==stat.ino){try{exactUnlink(migration,claim.owner.token,claim.identity);}catch{}throw new CsoError('INSUFFICIENT_CAPACITY','Another helper changed this run during recovery');} const tomb=join(dir,`.mutation-lock.legacy-${process.pid}-${randomBytes(4).toString('hex')}`); try{fs.renameSync(lock,tomb);atomicWriteSync(lock,marker,{mode:0o600,noReplace:true});fs.rmSync(tomb,{recursive:true,force:true});} @@ -491,13 +498,13 @@ function ensureLockProtocol(dir:string):string{ } const leases=join(dir,'.mutation-lock-leases'); if(!fs.existsSync(leases))try{fs.mkdirSync(leases,{mode:0o700});}catch(error:any){if(error?.code!=='EEXIST')throw error;} - const stat=fs.lstatSync(leases);if(stat.isSymbolicLink()||!stat.isDirectory()||(process.getuid&&stat.uid!==process.getuid()))throw new CsoError('UNSAFE_PATH','Run mutation lease directory is invalid'); + const stat=exactLstat(leases);if(stat.isSymbolicLink()||!stat.isDirectory()||(process.getuid&&stat.uid!==process.getuid()))throw new CsoError('UNSAFE_PATH','Run mutation lease directory is invalid'); if(process.platform!=='win32')fs.chmodSync(leases,0o700); return leases; } type LeaseState={token:string;owner:LockOwner;identity:LockIdentity;candidate?:string;decisionPath?:string;decision?:LeaseDecision;decisionIdentity?:LockIdentity;active?:string;number?:bigint}; type HeldRunLease={path:string;decision:string;decisionIdentity:LockIdentity;active:string;token:string;identity:LockIdentity}; -function privateLeaseArtifact(stat:fs.Stats):boolean{return stat.isFile()&&!stat.isSymbolicLink()&&stat.size>0&&stat.size<=LOCK_OWNER_MAX_BYTES&& +function privateLeaseArtifact(stat:ExactStats):boolean{return stat.isFile()&&!stat.isSymbolicLink()&&stat.size>0&&stat.size<=LOCK_OWNER_MAX_BYTES&& (!process.getuid||stat.uid===process.getuid())&&(process.platform==='win32'||(stat.mode&0o077)===0);} function readLeaseDecision(path:string,token:string):{decision:LeaseDecision;identity:LockIdentity}{ const options=leaseDecisionRecoveryOptions(token);recoverAtomicNoReplaceJson(path,options); @@ -529,9 +536,9 @@ function scanRunLeases(leases:string):LeaseState[]{ if(group.decision)try{decisionRecord=readLeaseDecision(group.decision,token);}catch(error){if(error instanceof CsoError&&(error.code==='SNAPSHOT_RACE'||error.code==='INSUFFICIENT_CAPACITY')){if(error.code==='INSUFFICIENT_CAPACITY'){if(Date.now()>=deadline)throw error;contention=error;}retry=true;break;}throw error;} if(group.actives[0]&&(!decisionRecord||decisionRecord.decision.kind!=='ticket'||decisionRecord.decision.ticket!==group.actives[0].encoded))throw new CsoError('UNSAFE_PATH','Run mutation lease active phase does not match its ticket decision'); const ownerPath=group.candidate??group.actives[0]?.path;let inspected:{owner:LockOwner;identity:LockIdentity}|undefined; - if(ownerPath){const expected=(group.candidate&&group.actives[0]?2:1) as LeaseLinks;let observed:fs.Stats;try{observed=fs.lstatSync(ownerPath);}catch(error:any){if(error?.code==='ENOENT'){retry=true;break;}throw error;}if(!privateLeaseArtifact(observed)){throw new CsoError('UNSAFE_PATH','Run mutation lease owner phase is not one private regular file');}if(observed.nlink!==expected){retry=true;break;}try{inspected=readOwner(ownerPath,token,expected,observed);}catch(error){if(error instanceof CsoError&&error.code==='INSUFFICIENT_CAPACITY'){if(Date.now()>=deadline)throw error;contention=error;retry=true;break;}throw error;}} - if(group.candidate&&group.actives[0]){let activeStat:fs.Stats;try{activeStat=fs.lstatSync(group.actives[0].path);}catch(error:any){if(error?.code==='ENOENT'){retry=true;break;}throw error;}if(!privateLeaseArtifact(activeStat)||activeStat.dev!==inspected!.identity.dev||activeStat.ino!==inspected!.identity.ino)throw new CsoError('UNSAFE_PATH','Run mutation lease active phase does not match its candidate inode');if(activeStat.nlink!==2){retry=true;break;}} - const identity=inspected?.identity??{dev:Number(decisionRecord!.decision.candidateDev),ino:Number(decisionRecord!.decision.candidateIno)},owner=inspected?.owner??decisionOwner(decisionRecord!.decision); + if(ownerPath){const expected=(group.candidate&&group.actives[0]?2:1) as LeaseLinks;let observed:ExactStats;try{observed=exactLstat(ownerPath);}catch(error:any){if(error?.code==='ENOENT'){retry=true;break;}throw error;}if(!privateLeaseArtifact(observed)){throw new CsoError('UNSAFE_PATH','Run mutation lease owner phase is not one private regular file');}if(observed.nlink!==expected){retry=true;break;}try{inspected=readOwner(ownerPath,token,expected,observed);}catch(error){if(error instanceof CsoError&&error.code==='INSUFFICIENT_CAPACITY'){if(Date.now()>=deadline)throw error;contention=error;retry=true;break;}throw error;}} + if(group.candidate&&group.actives[0]){let activeStat:ExactStats;try{activeStat=exactLstat(group.actives[0].path);}catch(error:any){if(error?.code==='ENOENT'){retry=true;break;}throw error;}if(!privateLeaseArtifact(activeStat)||activeStat.dev!==inspected!.identity.dev||activeStat.ino!==inspected!.identity.ino)throw new CsoError('UNSAFE_PATH','Run mutation lease active phase does not match its candidate inode');if(activeStat.nlink!==2){retry=true;break;}} + const identity=inspected?.identity??{dev:BigInt(decisionRecord!.decision.candidateDev),ino:BigInt(decisionRecord!.decision.candidateIno)},owner=inspected?.owner??decisionOwner(decisionRecord!.decision); if(decisionRecord&&(!decisionMatchesIdentity(decisionRecord.decision,identity)||!decisionMatchesOwner(decisionRecord.decision,owner)))throw new CsoError('UNSAFE_PATH','Run mutation lease decision does not match its candidate owner'); const number=decisionRecord?.decision.kind==='ticket'?BigInt(`0x${decisionRecord.decision.ticket}`):undefined; states.push({token,owner,identity,...(group.candidate?{candidate:group.candidate}:{}),...(group.decision&&decisionRecord?{decisionPath:group.decision,decision:decisionRecord.decision,decisionIdentity:decisionRecord.identity}:{}),...(group.actives[0]?{active:group.actives[0].path}:{}),...(number!==undefined?{number}:{})}); @@ -550,14 +557,14 @@ function releaseLeaseState(state:LeaseState):void{ function exactDecisionUnlink(path:string,token:string,identity:LockIdentity):void{ let current:{decision:LeaseDecision;identity:LockIdentity};try{current=readLeaseDecision(path,token);}catch{throw new CsoError('PERSISTENCE_FAILED','Run mutation lease decision changed before exact release');} if(current.identity.dev!==identity.dev||current.identity.ino!==identity.ino)throw new CsoError('PERSISTENCE_FAILED','Run mutation lease decision changed before exact release'); - let final:fs.Stats;try{final=fs.lstatSync(path);}catch{throw new CsoError('PERSISTENCE_FAILED','Run mutation lease decision changed before exact release');} + let final:ExactStats;try{final=exactLstat(path);}catch{throw new CsoError('PERSISTENCE_FAILED','Run mutation lease decision changed before exact release');} if(!privateLeaseArtifact(final)||final.nlink!==1||final.dev!==identity.dev||final.ino!==identity.ino)throw new CsoError('PERSISTENCE_FAILED','Run mutation lease decision changed before exact release'); try{fs.unlinkSync(path);}catch{throw new CsoError('PERSISTENCE_FAILED','Run mutation lease decision changed before exact release');} } function publishLeasePhase(candidate:string,target:string,token:string,identity:LockIdentity):void{ const before=readOwner(candidate,token,1);if(before.identity.dev!==identity.dev||before.identity.ino!==identity.ino)throw new CsoError('PERSISTENCE_FAILED','Run mutation lease changed before phase publication'); try{fs.linkSync(candidate,target);}catch{throw new CsoError('PERSISTENCE_FAILED','Run mutation lease phase could not be published');} - const source=fs.lstatSync(candidate),phase=fs.lstatSync(target);if(source.dev!==identity.dev||source.ino!==identity.ino||phase.dev!==identity.dev||phase.ino!==identity.ino||source.nlink!==2||phase.nlink!==2)throw new CsoError('PERSISTENCE_FAILED','Run mutation lease phase changed during publication'); + const source=exactLstat(candidate),phase=exactLstat(target);if(source.dev!==identity.dev||source.ino!==identity.ino||phase.dev!==identity.dev||phase.ino!==identity.ino||source.nlink!==2||phase.nlink!==2)throw new CsoError('PERSISTENCE_FAILED','Run mutation lease phase changed during publication'); } function makeLeaseDecision(owner:LockOwner,identity:LockIdentity,kind:'ticket'|'withdraw',ticket?:string):LeaseDecision{ const publisherIdentity=processIdentity(process.pid); @@ -567,9 +574,9 @@ function publishLeaseDecision(path:string,decision:LeaseDecision):void{ try{atomicWriteSync(path,JSON.stringify(decision)+'\n',{mode:0o600,noReplace:true});}catch(error:any){if(error?.code!=='EEXIST')throw new CsoError('PERSISTENCE_FAILED','Run mutation lease decision could not be published');} } function releaseKnownLease(candidate:string,decisionPath:string,active:string|undefined,token:string,identity:LockIdentity,expectedDecisionIdentity?:LockIdentity,requireActive=false):void{ - let candidateStat:fs.Stats;try{candidateStat=fs.lstatSync(candidate);}catch{throw new CsoError('PERSISTENCE_FAILED','Run mutation lease ownership changed before exact release');} + let candidateStat:ExactStats;try{candidateStat=exactLstat(candidate);}catch{throw new CsoError('PERSISTENCE_FAILED','Run mutation lease ownership changed before exact release');} if(!privateLeaseArtifact(candidateStat)||candidateStat.dev!==identity.dev||candidateStat.ino!==identity.ino)throw new CsoError('PERSISTENCE_FAILED','Run mutation lease ownership changed before exact release'); - let activeStat:fs.Stats|undefined;try{if(active)activeStat=fs.lstatSync(active);}catch(error:any){if(error?.code!=='ENOENT')throw new CsoError('PERSISTENCE_FAILED','Run mutation lease active phase changed before cleanup');} + let activeStat:ExactStats|undefined;try{if(active)activeStat=exactLstat(active);}catch(error:any){if(error?.code!=='ENOENT')throw new CsoError('PERSISTENCE_FAILED','Run mutation lease active phase changed before cleanup');} if(requireActive&&!activeStat)throw new CsoError('PERSISTENCE_FAILED','Run mutation lease active phase changed before exact release'); if(activeStat&&(!privateLeaseArtifact(activeStat)||activeStat.dev!==identity.dev||activeStat.ino!==identity.ino))throw new CsoError('PERSISTENCE_FAILED','Run mutation lease active phase changed before cleanup'); const expected=activeStat?2:1;if(candidateStat.nlink!==expected||activeStat&&activeStat.nlink!==2)throw new CsoError('PERSISTENCE_FAILED','Run mutation lease link state changed before cleanup'); @@ -583,8 +590,8 @@ function recoverDeadLease(state:LeaseState):boolean{ if(ownerIsAlive(state.owner))return false; try{releaseLeaseState(state);}catch(error){ if(!(error instanceof CsoError)||error.code!=='PERSISTENCE_FAILED')throw error; - for(const path of [state.candidate,state.active].filter((value):value is string=>Boolean(value))){try{const stat=fs.lstatSync(path);if(!privateLeaseArtifact(stat)||stat.dev!==state.identity.dev||stat.ino!==state.identity.ino)throw new CsoError('UNSAFE_PATH','Dead run mutation lease was replaced during recovery');}catch(recoveryError:any){if(recoveryError instanceof CsoError)throw recoveryError;if(recoveryError?.code!=='ENOENT')throw recoveryError;}} - if(state.decisionPath&&state.decisionIdentity)try{const stat=fs.lstatSync(state.decisionPath);if(!privateLeaseArtifact(stat)||stat.dev!==state.decisionIdentity.dev||stat.ino!==state.decisionIdentity.ino)throw new CsoError('UNSAFE_PATH','Dead run mutation lease decision was replaced during recovery');}catch(recoveryError:any){if(recoveryError instanceof CsoError)throw recoveryError;if(recoveryError?.code!=='ENOENT')throw recoveryError;} + for(const path of [state.candidate,state.active].filter((value):value is string=>Boolean(value))){try{const stat=exactLstat(path);if(!privateLeaseArtifact(stat)||stat.dev!==state.identity.dev||stat.ino!==state.identity.ino)throw new CsoError('UNSAFE_PATH','Dead run mutation lease was replaced during recovery');}catch(recoveryError:any){if(recoveryError instanceof CsoError)throw recoveryError;if(recoveryError?.code!=='ENOENT')throw recoveryError;}} + if(state.decisionPath&&state.decisionIdentity)try{const stat=exactLstat(state.decisionPath);if(!privateLeaseArtifact(stat)||stat.dev!==state.decisionIdentity.dev||stat.ino!==state.decisionIdentity.ino)throw new CsoError('UNSAFE_PATH','Dead run mutation lease decision was replaced during recovery');}catch(recoveryError:any){if(recoveryError instanceof CsoError)throw recoveryError;if(recoveryError?.code!=='ENOENT')throw recoveryError;} Atomics.wait(LEASE_ELECTION_WAIT,0,0,LEASE_ELECTION_POLL_MS); } return true; @@ -646,7 +653,7 @@ function acquireRunLease(dir:string):HeldRunLease{ const leases=ensureLockProtocol(dir);recoverLeasePublications(leases); const token=randomBytes(16).toString('hex'),lease=join(leases,`${token}.json`),owner:LockOwner={pid:process.pid,processIdentity:processIdentity(process.pid),token,createdAt:Date.now()}; atomicWriteSync(lease,JSON.stringify(owner)+'\n',{mode:0o600,noReplace:true}); - const ownStat=fs.lstatSync(lease),ownIdentity={dev:ownStat.dev,ino:ownStat.ino}; + const ownStat=exactLstat(lease),ownIdentity={dev:ownStat.dev,ino:ownStat.ino}; const decision=join(leases,`${token}.decision`);let decisionIdentity:LockIdentity|undefined,active:string|undefined; try{ const chosen=chooseRunLeaseTicket(leases,token,owner,ownIdentity);decisionIdentity=chosen.identity; @@ -763,7 +770,7 @@ function cleanupRun(dir:string,run:string,now:number,pinned:boolean,admit:()=>vo const created=Number(run.split('-')[0]),runExpired=now-created>30*86400_000,retainedBundle=repairBundleExpiry(dir,run,now,runExpired,admit); admit();const ephemeral=fs.existsSync(join(dir,EPHEMERAL_REPLAY)); if(ephemeral||(runExpired&&!pinned&&!retainedBundle)){ - admit();const before=fs.lstatSync(dir),tomb=join(dirname(dir),`.retired-${run}-${randomBytes(16).toString('hex')}`);fs.renameSync(dir,tomb);releasePath=join(tomb,'.mutation-lock-leases',basename(lease.path));admit();const after=fs.lstatSync(tomb); + admit();const before=exactLstat(dir),tomb=join(dirname(dir),`.retired-${run}-${randomBytes(16).toString('hex')}`);fs.renameSync(dir,tomb);releasePath=join(tomb,'.mutation-lock-leases',basename(lease.path));admit();const after=exactLstat(tomb); if(before.dev!==after.dev||before.ino!==after.ino)throw new CsoError('SNAPSHOT_RACE','Expired run changed while it was retired'); // The retired name is outside the public run namespace. Consume the // exclusive lease with the tree so no release/delete gap can admit a diff --git a/lib/redact-engine.ts b/lib/redact-engine.ts index 9d76c9024..273ba428e 100644 --- a/lib/redact-engine.ts +++ b/lib/redact-engine.ts @@ -121,6 +121,10 @@ export function normalizeWithMap(input: string): { normalized: string; map: number[]; } { + return normalizeOriginal(input); +} + +function normalizeOriginal(input: string, spanEnds?: number[]): { normalized: string; map: number[] } { const out: string[] = []; const map: number[] = []; let i = 0; @@ -133,6 +137,7 @@ export function normalizeWithMap(input: string): { for (const ch of rep) { out.push(ch); map.push(i); + spanEnds?.push(i + ent.length); } i += ent.length; matchedEntity = true; @@ -150,9 +155,10 @@ export function normalizeWithMap(input: string): { ZERO_WIDTH.lastIndex = 0; const norm = ch.normalize("NFKC"); - for (const nch of norm) { - out.push(nch); + for (let j = 0; j < norm.length; j++) { + out.push(norm[j]); map.push(i); + spanEnds?.push(i + 1); } i += 1; } @@ -327,6 +333,12 @@ function emailAllowed( // ── The scan ────────────────────────────────────────────────────────────────── export function scan(input: string, opts: ScanOptions = {}): ScanResult { + return scanInternal(input, opts); +} + +type OriginalSpan = { start: number; end: number }; + +function scanInternal(input: string, opts: ScanOptions, spans?: Map): ScanResult { const repoVisibility: RepoVisibility = opts.repoVisibility ?? "unknown"; let starts: number[] | null = null; // line index, built on the first finding // #1824: ?? only catches null/undefined, not NaN or <= 0. A bad value @@ -363,7 +375,8 @@ export function scan(input: string, opts: ScanOptions = {}): ScanResult { }; } - const { normalized, map } = normalizeWithMap(input); + const spanEnds: number[] | undefined = spans ? [] : undefined; + const { normalized, map } = normalizeOriginal(input, spanEnds); const fenceRanges = toolFenceRanges(normalized); const allow = new Set(opts.allowlist ?? []); @@ -379,8 +392,7 @@ export function scan(input: string, opts: ScanOptions = {}): ScanResult { if (m.index === re.lastIndex) re.lastIndex++; const span = m[1] ?? m[0]; - const spanStartInMatch = m[1] !== undefined ? m[0].indexOf(m[1]) : 0; - const normOffset = m.index + Math.max(0, spanStartInMatch); + const normOffset = m.indices?.[1]?.[0] ?? m.index; // Per-span placeholder suppression. if (isPlaceholderSpan(span)) continue; @@ -421,7 +433,7 @@ export function scan(input: string, opts: ScanOptions = {}): ScanResult { toolFenceDegraded = true; } - findings.push({ + const finding: Finding = { id: pat.id, tier: pat.tier, severity, @@ -433,7 +445,12 @@ export function scan(input: string, opts: ScanOptions = {}): ScanResult { autoRedactable: !!pat.autoRedactable, repoVisibility, ...(toolFenceDegraded ? { toolFenceDegraded } : {}), - }); + }; + findings.push(finding); + if (spans) { + const end = spanEnds?.[normOffset + span.length - 1]; + if (end !== undefined && end > origOffset) spans.set(finding, { start: origOffset, end }); + } } } @@ -450,6 +467,7 @@ function withFlags(flags: string): string { let f = flags; if (!f.includes("g")) f += "g"; if (!f.includes("m")) f += "m"; + if (!f.includes("d")) f += "d"; return f; } @@ -475,10 +493,11 @@ export function applyRedactions( opts: ScanOptions = {}, ): RedactResult { const ids = new Set(findingIds); - const { findings } = scan(input, opts); + const spans = new Map(); + const { findings } = scanInternal(input, opts, spans); const targets = findings .filter((f) => ids.has(f.id) && f.autoRedactable) - .map((f) => ({ f, ...locateSpan(input, f) })) + .map((f) => ({ f, ...(spans.get(f) ?? { start: -1, end: -1 }) })) .filter((t) => t.start >= 0); // Right-to-left so earlier offsets remain valid after splicing. @@ -528,9 +547,10 @@ const MARKER_ONLY_PATTERN_IDS = new Set(["pem.private_key", "gcp.service_account * structure-preserving path.) */ export function redactFindingSpans(input: string, opts: ScanOptions = {}): string | null { - const { findings } = scan(input, opts); + const spans = new Map(); + const { findings } = scanInternal(input, opts, spans); if (findings.some((f) => MARKER_ONLY_PATTERN_IDS.has(f.id))) return null; - const targets = findings.map((f) => ({ f, ...locateSpan(input, f) })); + const targets = findings.map((f) => ({ f, ...(spans.get(f) ?? { start: -1, end: -1 }) })); if (targets.some((t) => t.start < 0)) return null; // Coalesce overlapping/touching ranges — splicing two intersecting spans @@ -557,26 +577,6 @@ export function redactFindingSpans(input: string, opts: ScanOptions = {}): strin return body; } -function locateSpan(input: string, f: Finding): { start: number; end: number } { - // Re-derive the offset from line/col on the original text. - let offset = 0; - let line = 1; - while (line < f.line && offset < input.length) { - if (input[offset] === "\n") line++; - offset++; - } - offset += f.col - 1; - const pat = PATTERNS_BY_ID[f.id]; - if (!pat) return { start: -1, end: -1 }; - const re = new RegExp(pat.regex.source, withFlags(pat.regex.flags)); - re.lastIndex = Math.max(0, offset - 2); - const m = re.exec(input); - if (!m) return { start: -1, end: -1 }; - const span = m[1] ?? m[0]; - const start = m.index + (m[1] !== undefined ? m[0].indexOf(m[1]) : 0); - return { start, end: start + span.length }; -} - function inStructuralToken(body: string, start: number, end: number): boolean { // Markdown link target: [text](...span...). The span may sit anywhere inside // the parenthesized target (e.g. an email embedded in a URL). Walk backward diff --git a/package.json b/package.json index 6f28dce37..910d3a30d 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "gstack", - "version": "1.87.6", + "version": "1.88.1", "description": "Garry's Stack \u2014 Claude Code skills + fast headless browser. One repo, one install, entire AI engineering workflow.", "license": "MIT", "type": "module", diff --git a/test/artifacts-allowlist-decisions.test.ts b/test/artifacts-allowlist-decisions.test.ts index 7908cd28d..c196202c0 100644 --- a/test/artifacts-allowlist-decisions.test.ts +++ b/test/artifacts-allowlist-decisions.test.ts @@ -5,9 +5,9 @@ import * as path from "path"; const ROOT = path.resolve(import.meta.dir, ".."); const INIT = fs.readFileSync(path.join(ROOT, "bin", "gstack-artifacts-init"), "utf-8"); -/** Pull a quoted heredoc body out of gstack-artifacts-init by target filename. */ +/** Pull a quoted heredoc body out of gstack-artifacts-init by destination. */ function heredoc(target: string): string { - const re = new RegExp(`cat > "\\$GSTACK_HOME/${target}" <<'EOF'\\n([\\s\\S]*?)\\nEOF\\n`); + const re = new RegExp(`cat > "${target}" <<'EOF'\\n([\\s\\S]*?)\\nEOF\\n`); const m = INIT.match(re); if (!m) throw new Error(`heredoc for ${target} not found in gstack-artifacts-init`); return m[1]; @@ -39,7 +39,7 @@ const DECISION_PATHS = [ * Windows -- which is the platform where this bug bit. */ describe("the artifacts allowlist covers the decision store", () => { - const globs = heredoc("\\.brain-allowlist") + const globs = heredoc("\\$ALLOWLIST_TMP") .split("\n") .map((l) => l.trim()) .filter((l) => l && !l.startsWith("#")); @@ -51,7 +51,7 @@ describe("the artifacts allowlist covers the decision store", () => { }); test("decisions.* are class artifact, so they sync in artifacts-only mode too", () => { - const map = JSON.parse(heredoc("\\.brain-privacy-map\\.json")); + const map = JSON.parse(heredoc("\\$GSTACK_HOME/\\.brain-privacy-map\\.json")); for (const p of DECISION_PATHS) { const hit = map.find((e: { pattern: string; class: string }) => globToRe(e.pattern).test(p)); expect({ p, cls: hit?.class }).toEqual({ p, cls: "artifact" }); @@ -61,6 +61,6 @@ describe("the artifacts allowlist covers the decision store", () => { test("the allowlist still ends with the user-additions marker", () => { // Additions below it survive re-init; a glob added above would be silently // overwritten the next time gstack-artifacts-init runs. - expect(heredoc("\\.brain-allowlist").trimEnd()).toMatch(/# ---- USER ADDITIONS BELOW ----/); + expect(heredoc("\\$ALLOWLIST_TMP").trimEnd()).toMatch(/# ---- USER ADDITIONS BELOW ----/); }); }); diff --git a/test/cso-cli.test.ts b/test/cso-cli.test.ts index 41e56f72c..968a5de99 100644 --- a/test/cso-cli.test.ts +++ b/test/cso-cli.test.ts @@ -73,7 +73,33 @@ describe('compiled CSO command workflow',()=>{ test('wallet-shaped helper finding IDs survive submit, review, and verification admission',()=>{const started=command(['start','--repo',repo,'--comprehensive','--offline']);expect(started.status).toBe(0);const run=JSON.parse(started.stdout),base:any={title:'Wallet-shaped helper ID',rootCause:'Tenant query omits caller tenant predicate 0',location:{path:'src/users.ts',line:1,symbol:'tenantQuery'},advisoryIds:[],severity:'high',confidence:'high',confidenceRationale:'Caller-to-sink trace directly establishes the missing predicate',evidence:'supported',attackerControl:'Tenant chooses the record ID',impact:'Cross-tenant record disclosure',scenario:'A tenant requests another tenant record by ID',trace:['route','tenantQuery','database'],references:['src/users.ts:1'],recommendation:'Bind the query to the authenticated tenant',challenge:{reviewer:'reviewer-session',independent:true,mode:'independent_agent',callers:'route caller checked',controls:'authentication lacks authorization',counterevidence:'none found',conclusion:'tenant predicate is absent'}};let helperId='';for(let i=0;i<10_000;i++){base.rootCause=`Tenant query omits caller tenant predicate ${i}`;helperId=fingerprint(base);if(redact(helperId)!==helperId)break;}expect(redact(helperId)).not.toBe(helperId);const input=path.join(root,`wallet-id-${run.runId}.json`);fs.writeFileSync(input,JSON.stringify({findings:[base]}));expect(command(['submit',run.runId,input]).status).toBe(0);const reportPath=path.join(state,'security','cso',run.repoId,run.runId,'report.json'),stored=JSON.parse(fs.readFileSync(reportPath,'utf8'));expect(stored.findings[0].id).toBe(helperId);const before=fs.readFileSync(path.join(repo,'src','users.ts'),'utf8'),request:any={findingId:helperId,runtimeProfile:'node-v1',port:3456,start:{executable:'/usr/local/bin/node',args:['src/users.ts']},legitimate:[{name:'control',path:'/health',method:'GET',expected:{status:200}}],security:{name:'tenant boundary',path:'/users/other',method:'GET',expected:{status:403},vulnerable:{status:200}},existingTests:[{executable:'/usr/local/bin/node',args:['--test']}],testFiles:['src/control.test.ts'],fixtures:{},boundaryFiles:['src/users.ts'],changes:[{path:'src/users.ts',beforeSha256:sha256(before),after:before+'// tenant predicate\n',effect:'source'}],review:{reviewer:'reviewer-session',independent:true,rootCauseRepaired:true,featurePreserved:true,boundaryMocks:false,rationale:'The tenant predicate is added while the control remains intact',reviewedPatchHash:''}};request.review.reviewedPatchHash=patchHash(request);fs.writeFileSync(input,JSON.stringify(request));const reviewed=command(['record-review',run.runId,input,'--producer','producer-session']);expect(reviewed.status).toBe(0);request.review.artifactId=JSON.parse(reviewed.stdout).reviewArtifactId;fs.writeFileSync(input,JSON.stringify(request));const verified=command(['verify',run.runId,input]);expect(verified.status).not.toBe(0);expect(verified.stderr).toContain('PREREQUISITE');expect(verified.stderr).not.toContain('REDACTION_FAILED');const review=JSON.parse(fs.readFileSync(path.join(path.dirname(reportPath),'reviews',`${request.review.artifactId}.json`),'utf8'));expect(review.findingId).toBe(helperId);}); test('daily find, early submit, and finish persist a truthful report without changing source',()=>{const before=git('status','--porcelain=v1','-z'),started=command(['start','--repo',repo,'--scope','auth','--offline']);expect(started.status).toBe(0);const run=JSON.parse(started.stdout),runId=run.runId;expect(run.completeness).toBe('not assessed');const reportPath=path.join(state,'security','cso',run.repoId,runId,'report.json');expect(fs.existsSync(reportPath)).toBe(true);const current=JSON.parse(fs.readFileSync(reportPath,'utf8')); const wallet='0x1234567890abcdef1234567890abcdef12345678',submission={application:{actors:['authenticated tenant user'],assets:['tenant records'],entrypoints:['GET /users/:id'],tenantBoundaries:['record.tenant_id equals session.tenant_id'],sensitiveOperations:['read tenant record'],invariants:['a tenant cannot read another tenant record']},findings:[{title:'Cross-tenant user read',rootCause:`Tenant query ${wallet} omits caller tenant predicate`,location:{path:'src/users.ts',line:1,symbol:'tenantQuery'},advisoryIds:[],severity:'high',confidence:'high',confidenceRationale:'The captured caller-to-sink trace and control review directly support the finding',evidence:'supported',attackerControl:'Authenticated caller chooses the record ID',impact:'Another tenant record is returned',scenario:'A tenant supplies a known record ID owned by another tenant and receives that record',trace:['GET /users/:id','tenantQuery','findUnique by id'],references:['src/users.ts:1','OWASP API1:2023'],recommendation:'Bind the lookup predicate to the authenticated tenant identifier before returning the record',challenge:{reviewer:'independent-2',independent:true,mode:'independent_agent',callers:'Authenticated route forwards path ID',controls:'Authentication exists; authorization predicate does not',counterevidence:'Opaque IDs reduce guessing but do not authorize',conclusion:'The caller can provide a known cross-tenant ID'}}],coverage:current.coverage.filter((c:any)=>!['snapshot-inputs','history-inputs'].includes(c.domain)).map((c:any)=>({...c,status:'assessed',method:'caller and middleware trace',gaps:[],evidence:[`${c.domain} inspected against captured source`]})),gaps:[]};const file=path.join(root,'submission.json');fs.writeFileSync(file,JSON.stringify(submission));const submitted=command(['submit',runId,file]);expect(submitted.status).toBe(0);expect(JSON.parse(fs.readFileSync(reportPath,'utf8')).findings).toHaveLength(1);const finished=command(['finish',runId]);expect(finished.status).toBe(0);const final=JSON.parse(fs.readFileSync(reportPath,'utf8'));expect(final.completeness).toBe('complete');expect(final.findings[0]).toMatchObject({reproduction:'not_attempted',repair:'not_attempted',closure:'open'});expect(final.findings[0].id).toBe(fingerprint(final.findings[0]));expect(JSON.stringify(final)).not.toContain(wallet);const markdown=fs.readFileSync(path.join(path.dirname(reportPath),'report.md'),'utf8');expect(markdown.startsWith('complete — domain:auth')).toBe(true);expect(markdown).toContain('.env: excluded: credential or execution configuration');expect(markdown).not.toContain('ghp_');expect(git('status','--porcelain=v1','-z')).toBe(before);}); - test('credential originals are withheld rather than printed',()=>{const run=JSON.parse(command(['start','--repo',repo,'--scope','auth','--offline']).stdout);const result=command(['read',run.runId,'.env']);expect(result.status).not.toBe(0);expect(result.stderr).toContain('MISSING_INPUT');expect(result.stdout+result.stderr).not.toContain('ghp_');}); + test('credential originals are withheld rather than printed',()=>{const run=JSON.parse(command(['start','--repo',repo,'--scope','auth','--offline']).stdout);const result=command(['read',run.runId,'.env']);expect(result.status).toBe(0);expect(result.stdout.trim()).toBe('TOKEN=');expect(result.stderr).toBe('');expect(result.stdout+result.stderr).not.toContain('ghp_');expect(result.stdout+result.stderr).not.toContain(CREDENTIAL_CANARY);expect(fs.existsSync(path.join(state,'security','cso',run.repoId,run.runId,'snapshot','.env'))).toBe(false);}); + test('supplementary NFKC expansion cannot displace compiled reader redaction',()=>{ + const file=path.join(repo,'.env'); + expect(fs.realpathSync(file)).toBe(path.join(fs.realpathSync(repo),'.env')); + const original=fs.readFileSync(file),prefix='\uFA6C'.repeat(64)+'\n',suffix='\n'+'z'.repeat(256); + const value=['8Fk2pQ9vXz4wL7mN','3rT6yB1cD5eG0hJq'].join(''); + try{ + fs.writeFileSync(file,`${prefix}DB_PASSWORD=${value}${suffix}`); + const started=command(['start','--repo',repo,'--scope','auth','--offline']);expect(started.status).toBe(0); + const run=JSON.parse(started.stdout),result=command(['read',run.runId,'.env']); + expect(result.status).toBe(0);expect(result.stderr).toBe(''); + expect(result.stdout.trimEnd()).toBe(`${prefix}DB_PASSWORD=${suffix}`); + expect(result.stdout+result.stderr).not.toContain(value); + expect(fs.existsSync(path.join(state,'security','cso',run.repoId,run.runId,'snapshot','.env'))).toBe(false); + }finally{fs.writeFileSync(file,original);} + }); + test('marker-only credential payloads remain unavailable to the compiled reader',()=>{ + const file=path.join(repo,'.env'),original=fs.readFileSync(file),body=['-----BEGIN ','PRIVATE KEY-----\n','synthetic-key-body\n','-----END ','PRIVATE KEY-----\n'].join(''); + try{ + fs.writeFileSync(file,body); + const started=command(['start','--repo',repo,'--scope','auth','--offline']);expect(started.status).toBe(0); + const run=JSON.parse(started.stdout),result=command(['read',run.runId,'.env']); + expect(result.status).not.toBe(0);expect(result.stderr).toContain('MISSING_INPUT');expect(result.stdout+result.stderr).not.toContain('synthetic-key-body'); + const dir=path.join(state,'security','cso',run.repoId,run.runId); + expect(fs.existsSync(path.join(dir,'readable','.env'))).toBe(false);expect(fs.existsSync(path.join(dir,'snapshot','.env'))).toBe(false); + }finally{fs.writeFileSync(file,original);} + }); test('bounded submission and snapshot readers reject multiply-linked pathnames',()=>{const run=JSON.parse(command(['start','--repo',repo,'--scope','auth','--offline']).stdout),dir=path.join(state,'security','cso',run.repoId,run.runId),inputSource=path.join(root,`linked-input-source-${run.runId}.json`),input=path.join(root,`linked-input-${run.runId}.json`);fs.writeFileSync(inputSource,'{}');fs.linkSync(inputSource,input);const submitted=command(['submit',run.runId,input]);expect(submitted.status).not.toBe(0);expect(submitted.stderr).toContain('one bounded regular file');const readable=path.join(dir,'readable','src','users.ts'),outside=path.join(root,`linked-readable-${run.runId}`);fs.linkSync(readable,outside);const read=command(['read',run.runId,'src/users.ts']);expect(read.status).not.toBe(0);expect(read.stderr).toContain('one bounded regular file');}); test.skipIf(process.platform==='win32')('rejects a SARIF FIFO without blocking on open',()=>{const run=JSON.parse(command(['start','--repo',repo,'--scope','auth','--offline']).stdout),fifo=path.join(root,`sarif-${run.runId}.pipe`);expect(spawnSync('/usr/bin/mkfifo',[fifo],{encoding:'utf8',timeout:5_000}).status).toBe(0);const started=Date.now(),imported=command(['import-sarif',run.runId,fifo]);expect(Date.now()-started).toBeLessThan(2_000);expect(imported.status).not.toBe(0);expect(imported.stderr).toContain('bounded regular file');}); test('model submissions cannot overwrite helper-owned readiness or scanner coverage',()=>{const run=JSON.parse(command(['start','--repo',repo,'--scope','auth','--offline']).stdout),file=path.join(root,'helper-coverage.json');fs.writeFileSync(file,JSON.stringify({coverage:[{domain:'runtime-readiness',scope:'node',status:'assessed',method:'claimed by model',gaps:[],exclusions:[],evidence:['untrusted claim']}]}));const result=command(['submit',run.runId,file]);expect(result.status).not.toBe(0);expect(result.stderr).toContain('helper-owned');const report=JSON.parse(fs.readFileSync(path.join(state,'security','cso',run.repoId,run.runId,'report.json'),'utf8'));expect(report.coverage.some((x:any)=>x.domain==='runtime-readiness')).toBe(false);}); diff --git a/test/cso-distribution.test.ts b/test/cso-distribution.test.ts index d45547c30..feebec2b9 100644 --- a/test/cso-distribution.test.ts +++ b/test/cso-distribution.test.ts @@ -336,6 +336,27 @@ describe('CSO runtime staging gates', () => { expect(gate['continue-on-error']).not.toBe(true); }); + test('native macOS ownership and settings regressions are required by the free gate', () => { + const workflow = Bun.YAML.parse(readFileSync(join(ROOT, '.github/workflows/free-tests.yml'), 'utf8')) as any; + const job = workflow.jobs['cso-macos-launcher']; + const gate = job.steps.find((step: any) => step.name === 'Exercise native agent ownership and linked settings'); + expect(job['runs-on']).toBe('macos-latest'); + expect(gate.env.TMPDIR).toBe('/tmp'); + expect(gate.run).toContain('test -f "$file"'); + expect(gate.run).toContain('bun test "${files[@]}"'); + for (const file of [ + 'browse/test/terminal-agent-lifecycle.test.ts', + 'browse/test/terminal-agent-native-observation.test.ts', + 'browse/test/terminal-agent-watchdog.test.ts', + 'browse/test/server-embedder-terminal-port.test.ts', + 'browse/test/server-factory.test.ts', + 'test/gstack-settings-hook-symlink.test.ts', + 'test/gstack-settings-hook-schema-aware.test.ts', + ]) expect(gate.run).toContain(file); + expect(gate['continue-on-error']).not.toBe(true); + expect(workflow.jobs['free-tests'].needs).toContain('cso-macos-launcher'); + }); + test('publication requires manual protected-main review, signed evidence, and native containment checks', () => { const raw = readFileSync(join(ROOT, '.github/workflows/cso-runtime-images.yml'), 'utf8'); const workflow = Bun.YAML.parse(raw) as any; diff --git a/test/cso-lease-identity.test.ts b/test/cso-lease-identity.test.ts new file mode 100644 index 000000000..f368e5573 --- /dev/null +++ b/test/cso-lease-identity.test.ts @@ -0,0 +1,182 @@ +import { afterEach, describe, expect, spyOn, test } from 'bun:test'; +import * as fs from 'node:fs'; +import * as os from 'node:os'; +import * as path from 'node:path'; +import { recoverAtomicNoReplaceJson, withLock } from '../lib/cso/state'; + +const roots:string[]=[]; +const tmp=()=>{const root=fs.mkdtempSync(path.join(os.tmpdir(),'cso-lease-identity-'));roots.push(root);return root;}; +afterEach(()=>{for(const root of roots.splice(0))fs.rmSync(root,{recursive:true,force:true});}); + +function syntheticIds(root:string,first:bigint,step=1n):()=>void{ + const lstat=fs.lstatSync.bind(fs),fstat=fs.fstatSync.bind(fs),ids=new Map(); + const map=(stat:fs.Stats|fs.BigIntStats,bigint:boolean)=>{ + const key=`${stat.dev}:${stat.ino}`; + if(!ids.has(key))return stat; + const ino=ids.get(key)!; + return Object.assign(Object.create(Object.getPrototypeOf(stat)),stat,{dev:bigint?first:Number(first),ino:bigint?ino:Number(ino)}); + }; + const pathSpy=spyOn(fs,'lstatSync').mockImplementation(((file:any,options?:any)=>{ + const stat=lstat(file,options); + if(String(file).startsWith(root+path.sep)){ + const key=`${stat.dev}:${stat.ino}`; + if(!ids.has(key))ids.set(key,first+step*BigInt(ids.size)); + } + return map(stat,options?.bigint===true); + }) as typeof fs.lstatSync); + const fdSpy=spyOn(fs,'fstatSync').mockImplementation(((fd:any,options?:any)=>map(fstat(fd,options),options?.bigint===true)) as typeof fs.fstatSync); + return()=>{fdSpy.mockRestore();pathSpy.mockRestore();}; +} + +function leaseFixture(dir:string,ownerPid:number,token:string){ + withLock(dir,()=>0); + const leases=path.join(dir,'.mutation-lock-leases'),candidate=path.join(leases,`${token}.json`),decision=path.join(leases,`${token}.decision`); + fs.writeFileSync(candidate,JSON.stringify({pid:ownerPid,token,createdAt:0})+'\n',{mode:0o600}); + const stat=fs.lstatSync(candidate,{bigint:true}); + fs.writeFileSync(decision,JSON.stringify({schemaVersion:1,token,kind:'ticket',ticket:'0000000000000001',candidateDev:String(stat.dev),candidateIno:String(stat.ino),ownerPid,ownerCreatedAt:0,publisherPid:ownerPid,createdAt:0})+'\n',{mode:0o600}); + return {leases,candidate,decision,stat}; +} + +describe('CSO exact filesystem lease identity',()=>{ + test('the last safe integer and its next inode remain distinct',()=>{ + const dir=tmp(),restore=syntheticIds(dir,9007199254740991n); + try{ + const first=path.join(dir,'first'),second=path.join(dir,'second'); + fs.writeFileSync(first,'a');fs.writeFileSync(second,'b'); + expect(fs.lstatSync(first,{bigint:true}).ino).toBe(9007199254740991n); + expect(fs.lstatSync(second,{bigint:true}).ino).toBe(9007199254740992n); + expect(withLock(dir,()=>1)).toBe(1); + }finally{restore();} + }); + + test.each([9007199254740991n,9007199254740992n,9007199254740993n])('serializes and releases identity starting at %s across consecutive mutations',(first)=>{ + const dir=tmp(),restore=syntheticIds(dir,first); + try{ + for(let i=0;i<4;i++)expect(withLock(dir,()=>{ + const leases=path.join(dir,'.mutation-lock-leases'),files=fs.readdirSync(leases); + const candidate=path.join(leases,files.find(name=>name.endsWith('.json'))!),decision=path.join(leases,files.find(name=>name.endsWith('.decision'))!),record=JSON.parse(fs.readFileSync(decision,'utf8')); + expect(record.candidateDev).toBe(String(fs.lstatSync(candidate,{bigint:true}).dev)); + expect(record.candidateIno).toBe(String(fs.lstatSync(candidate,{bigint:true}).ino)); + expect(BigInt(record.candidateIno)).toBeGreaterThanOrEqual(first); + return i; + })).toBe(i); + expect(fs.readdirSync(path.join(dir,'.mutation-lock-leases'))).toEqual([]); + }finally{restore();} + }); + + test('recovers a dead owner and its exact high-ID decision, then admits the next operation',()=>{ + const dir=tmp(),restore=syntheticIds(dir,9007199254740992n); + try{ + const {leases,candidate,decision}=leaseFixture(dir,2147483647,'a'.repeat(32)); + expect(withLock(dir,()=>1)).toBe(1); + expect(fs.existsSync(candidate)).toBe(false);expect(fs.existsSync(decision)).toBe(false); + expect(withLock(dir,()=>2)).toBe(2);expect(fs.readdirSync(leases)).toEqual([]); + }finally{restore();} + }); + + test('a decision left after exact candidate removal keeps its high ID until recovery',()=>{ + const dir=tmp(),restore=syntheticIds(dir,9007199254740992n); + try{ + const {leases,candidate,decision,stat}=leaseFixture(dir,2147483647,'e'.repeat(32)); + expect(JSON.parse(fs.readFileSync(decision,'utf8')).candidateIno).toBe(String(stat.ino)); + fs.unlinkSync(candidate); + expect(withLock(dir,()=>3)).toBe(3); + expect(fs.readdirSync(leases)).toEqual([]); + }finally{restore();} + }); + + test('a live foreign lease is not stolen, even when its timestamp is old',()=>{ + const dir=tmp(),restore=syntheticIds(dir,9007199254740992n); + try{ + const {candidate,decision}=leaseFixture(dir,process.pid,'b'.repeat(32)); + expect(()=>withLock(dir,()=>1)).toThrow('Another operation in this helper'); + expect(fs.existsSync(candidate)).toBe(true);expect(fs.existsSync(decision)).toBe(true); + }finally{restore();} + }); + + test('a legacy rounded high-ID decision is ambiguous and remains blocked',()=>{ + const dir=tmp(),restore=syntheticIds(dir,9007199254740993n,2n); + try{ + const token='c'.repeat(32),{candidate,decision,stat}=leaseFixture(dir,2147483647,token); + expect(stat.ino%2n).toBe(1n); + const record=JSON.parse(fs.readFileSync(decision,'utf8')); + record.candidateIno=String(Number(stat.ino)); + fs.writeFileSync(decision,JSON.stringify(record)+'\n',{mode:0o600}); + expect(record.candidateIno).not.toBe(String(stat.ino)); + let failure:unknown;try{withLock(dir,()=>1);}catch(error){failure=error;} + expect(failure).toMatchObject({code:'UNSAFE_PATH'}); + expect(fs.existsSync(candidate)).toBe(true);expect(fs.existsSync(decision)).toBe(true); + }finally{restore();} + }); + + test('a neighboring replacement inode is rejected during exact release',()=>{ + const dir=tmp(),restore=syntheticIds(dir,9007199254740992n); + try{ + let replaced=''; + let failure:unknown;try{withLock(dir,()=>{ + const leases=path.join(dir,'.mutation-lock-leases'),active=fs.readdirSync(leases).find(name=>name.includes('.active.'))!; + replaced=path.join(leases,active);const bytes=fs.readFileSync(replaced); + fs.renameSync(replaced,`${replaced}.original`);fs.writeFileSync(replaced,bytes,{mode:0o600}); + });}catch(error){failure=error;} + expect(failure).toMatchObject({code:'PERSISTENCE_FAILED'}); + expect(fs.existsSync(replaced)).toBe(true); + }finally{restore();} + }); + + test.skipIf(process.platform==='win32')('a symlink substituted for a dead owner is never reclaimed',()=>{ + const dir=tmp(),restore=syntheticIds(dir,9007199254740992n); + try{ + const {candidate,decision}=leaseFixture(dir,2147483647,'f'.repeat(32)),original=`${candidate}.original`; + fs.renameSync(candidate,original);fs.symlinkSync(original,candidate); + let failure:unknown;try{withLock(dir,()=>1);}catch(error){failure=error;} + expect(failure).toMatchObject({code:'UNSAFE_PATH'}); + expect(fs.lstatSync(candidate).isSymbolicLink()).toBe(true); + expect(fs.existsSync(decision)).toBe(true); + }finally{restore();} + }); + + test('atomic recovery matches only the exact high-ID hard-link publication',()=>{ + const dir=tmp(),restore=syntheticIds(dir,9007199254740992n); + try{ + const target=path.join(dir,'artifact.json'),temporary=`${target}.tmp.2147483647.deadbeef`; + fs.writeFileSync(temporary,'{"value":"retained"}\n',{mode:0o600});fs.linkSync(temporary,target); + recoverAtomicNoReplaceJson(target,{label:'Synthetic publication',maxBytes:4096}); + expect(fs.existsSync(temporary)).toBe(false);expect(JSON.parse(fs.readFileSync(target,'utf8'))).toEqual({value:'retained'}); + }finally{restore();} + }); + + test('adjacent IDs that round to the same Number cannot impersonate a publication',()=>{ + const dir=tmp(),restore=syntheticIds(dir,9007199254740992n); + try{ + const target=path.join(dir,'artifact.json'),temporary=`${target}.tmp.2147483647.deadbeef`,time=new Date(1_700_000_000_000); + fs.writeFileSync(target,'{"value":"retained"}\n',{mode:0o600}); + fs.writeFileSync(temporary,'{"value":"retained"}\n',{mode:0o600}); + fs.utimesSync(target,time,time);fs.utimesSync(temporary,time,time); + fs.linkSync(target,path.join(dir,'other'));fs.linkSync(temporary,path.join(dir,'another')); + const original=fs.lstatSync(target,{bigint:true}),foreign=fs.lstatSync(temporary,{bigint:true}); + expect(foreign.ino-original.ino).toBe(1n); + expect(Number(foreign.ino)).toBe(Number(original.ino)); + let failure:unknown;try{recoverAtomicNoReplaceJson(target,{label:'Synthetic publication',maxBytes:4096});}catch(error){failure=error;} + expect(failure).toMatchObject({code:'UNSAFE_PATH'}); + expect(fs.existsSync(temporary)).toBe(true); + expect(fs.readFileSync(target,'utf8')).toBe('{"value":"retained"}\n'); + }finally{restore();} + }); + + test('atomic recovery rejects a same-millisecond timestamp change while the file is opened',()=>{ + const dir=tmp(),target=path.join(dir,'artifact.json'),temporary=`${target}.tmp.2147483647.deadbeef`,fstat=fs.fstatSync; + fs.writeFileSync(temporary,'{"value":"retained"}\n',{mode:0o600});fs.linkSync(temporary,target); + fs.utimesSync(target,1_700_000_000.0001,1_700_000_000.0001); + let injected=false,failure:unknown; + const reader=spyOn(fs,'fstatSync').mockImplementation(((fd:any,options?:any)=>{ + const stat=fstat(fd,options); + if(injected||options?.bigint!==true)return stat; + injected=true; + return Object.assign(Object.create(Object.getPrototypeOf(stat)),stat,{mtimeNs:stat.mtimeMs*1_000_000n+600_000n}); + }) as typeof fs.fstatSync); + try{recoverAtomicNoReplaceJson(target,{label:'Synthetic publication',maxBytes:4096});}catch(error){failure=error;}finally{reader.mockRestore();} + expect(injected).toBe(true); + expect(failure).toMatchObject({code:'SNAPSHOT_RACE'}); + expect(fs.existsSync(temporary)).toBe(true); + }); +}); diff --git a/test/cso-snapshot-state.test.ts b/test/cso-snapshot-state.test.ts index a7e982a7c..491a1f4a7 100644 --- a/test/cso-snapshot-state.test.ts +++ b/test/cso-snapshot-state.test.ts @@ -24,7 +24,7 @@ describe('CSO dirty snapshot boundary',()=>{ test('does not invoke configured clean filters, hooks, fsmonitor, or PATH shims',async()=>{const p=repo(),marker=path.join(p,'marker');fs.writeFileSync(path.join(p,'.gitattributes'),'*.txt filter=hostile\n');git(p,'config','filter.hostile.clean',`/bin/sh -c 'touch ${marker}; cat'`);git(p,'config','core.fsmonitor',`/bin/sh -c 'touch ${marker}'`);const fake=path.join(p,'bin');fs.mkdirSync(fake);fs.writeFileSync(path.join(fake,'git'),`#!/bin/sh\ntouch '${marker}'\nexit 99\n`,{mode:0o755});const old=process.env.PATH;process.env.PATH=`${fake}:${old}`;try{await capture(p,newRun(p).dir,'HEAD');}finally{process.env.PATH=old;}expect(fs.existsSync(marker)).toBe(false);}); test('rejects symlinks and hard links as execution inputs',async()=>{const p=repo();fs.symlinkSync('/etc/passwd',path.join(p,'escape'));expect(capture(p,newRun(p).dir)).rejects.toThrow();fs.unlinkSync(path.join(p,'escape'));fs.linkSync(path.join(p,'tracked.txt'),path.join(p,'hard'));expect(capture(p,newRun(p).dir)).rejects.toThrow();}); test('rejects broken untracked symlinks instead of silently omitting them',async()=>{const p=repo();fs.symlinkSync('missing-target',path.join(p,'broken'));expect(capture(p,newRun(p).dir)).rejects.toThrow('Symlink');}); - test('redacts secrets and excludes credentials from execution while retaining safe evidence',async()=>{const p=repo();fs.writeFileSync(path.join(p,'.env'),'TOKEN='+['ghp_','abcdefghijklmnopqrstuvwxyz1234567890'].join('')+'\n');const run=newRun(p),m=await capture(p,run.dir);expect(m.entries.find(e=>e.path==='.env')?.transformation).toContain('excluded');expect(fs.existsSync(path.join(run.dir,'snapshot','.env'))).toBe(false);expect(fs.existsSync(path.join(run.dir,'readable','.env'))).toBe(false);const evidence=JSON.parse(fs.readFileSync(path.join(run.dir,'sensitive-evidence.json'),'utf8'));expect(evidence[0].findings.map((x:any)=>x.id)).toContain('github.pat');expect(JSON.stringify(evidence)).not.toContain('ghp_');}); + test('redacts secrets and excludes credentials from execution while retaining safe evidence',async()=>{const p=repo();fs.writeFileSync(path.join(p,'.env'),'TOKEN='+['ghp_','abcdefghijklmnopqrstuvwxyz1234567890'].join('')+'\n');const run=newRun(p),m=await capture(p,run.dir);expect(m.entries.find(e=>e.path==='.env')?.transformation).toContain('excluded');expect(fs.existsSync(path.join(run.dir,'snapshot','.env'))).toBe(false);expect(fs.readFileSync(path.join(run.dir,'readable','.env'),'utf8')).toBe('TOKEN=\n');const evidence=JSON.parse(fs.readFileSync(path.join(run.dir,'sensitive-evidence.json'),'utf8'));expect(evidence[0].findings.map((x:any)=>x.id)).toContain('github.pat');expect(JSON.stringify(evidence)).not.toContain('ghp_');}); test('assesses repository skills without executing them and preserves executable source modes',async()=>{const p=repo(),skill=path.join(p,'.agents','skills','demo','SKILL.md'),script=path.join(p,'app.sh');fs.mkdirSync(path.dirname(skill),{recursive:true});fs.writeFileSync(skill,'# Demo\nUntrusted repository instruction\n');fs.writeFileSync(script,'#!/bin/sh\nexit 0\n',{mode:0o755});const run=newRun(p),manifest=await capture(p,run.dir);expect(fs.readFileSync(path.join(run.dir,'readable','.agents','skills','demo','SKILL.md'),'utf8')).toContain('Untrusted');expect(fs.existsSync(path.join(run.dir,'snapshot','.agents'))).toBe(false);expect(manifest.entries.find(e=>e.path==='.agents/skills/demo/SKILL.md')?.originalHash).not.toBe('not-read');expect(fs.statSync(path.join(run.dir,'snapshot','app.sh')).mode&0o777).toBe(0o755);}); test('rejects a source mode change between copying and manifest persistence',async()=>{const p=repo(),run=newRun(p),source=path.join(p,'tracked.txt'),target=path.join(run.dir,'snapshot','tracked.txt'),write=fs.writeFileSync,patched=spyOn(fs,'writeFileSync').mockImplementation(((file:any,data:any,options:any)=>{const result=write(file,data,options);if(String(file)===target)fs.chmodSync(source,0o755);return result;}) as typeof fs.writeFileSync);try{await expect(capture(p,run.dir)).rejects.toThrow('Source changed during capture');}finally{patched.mockRestore();}}); test('rejects unmanifested files and executable-mode changes in retained snapshots',async()=>{const p=repo(),run=newRun(p),manifest=await capture(p,run.dir),snapshot=path.join(run.dir,'snapshot');assertSnapshot(run.dir,manifest);fs.writeFileSync(path.join(snapshot,'injected.js'),'malicious\n');expect(()=>assertSnapshot(run.dir,manifest)).toThrow('membership');fs.unlinkSync(path.join(snapshot,'injected.js'));fs.chmodSync(path.join(snapshot,'tracked.txt'),0o755);expect(()=>assertSnapshot(run.dir,manifest)).toThrow('changed');}); diff --git a/test/cso-windows-launcher.test.ts b/test/cso-windows-launcher.test.ts index 3a62f2af1..41709df1a 100644 --- a/test/cso-windows-launcher.test.ts +++ b/test/cso-windows-launcher.test.ts @@ -310,6 +310,72 @@ describe('CSO native Windows build contract', () => { const started=spawnSync(actual,['start','--repo',repository,'--offline'],{cwd:repository,encoding:'utf8',env,timeout:30_000});expectSuccessfulProcess(started,'gstack-cso start');expect(JSON.parse(started.stdout).schemaVersion).toBe(3);expect(fs.existsSync(path.join(profile,'.gstack','security','cso'))).toBe(true); }, 120_000); + test('NTFS high file IDs survive repeated native commands and ambiguous old decisions remain blocked', () => { + const repository=path.join(temporary,'lease lifecycle repository'),profile=path.join(temporary,'lease lifecycle profile'); + fs.mkdirSync(repository);fs.mkdirSync(profile); + const git='C:\\Program Files\\Git\\cmd\\git.exe',gitEnv={...process.env,HOME:profile}; + for(const args of [['init','-q'],['config','user.email','fixture@example.test'],['config','user.name','Fixture']] as string[][]){const result=spawnSync(git,args,{cwd:repository,encoding:'utf8',env:gitEnv,timeout:10_000});expect(result.status).toBe(0);} + fs.writeFileSync(path.join(repository,'app.js'),'console.log("fixture")\n'); + for(const args of [['add','app.js'],['commit','-qm','fixture']] as string[][]){const result=spawnSync(git,args,{cwd:repository,encoding:'utf8',env:gitEnv,timeout:10_000});expect(result.status).toBe(0);} + const actual=path.join(ROOT,'bin','gstack-cso-launcher.exe'),env={...process.env,HOME:'',GSTACK_HOME:'',CLAUDE_PLUGIN_ROOT:'',CLAUDE_PLUGIN_DATA:'',USERPROFILE:profile,PATH:temporary}; + const command=(args:string[])=>spawnSync(actual,args,{cwd:repository,encoding:'utf8',env,timeout:30_000}),submission=path.join(profile,'submission.json'); + fs.writeFileSync(submission,'{}\n'); + for(let i=0;i<3;i++){ + const started=command(['start','--repo',repository,'--offline']);expectSuccessfulProcess(started,'gstack-cso start'); + const run=JSON.parse(started.stdout),dir=path.join(profile,'.gstack','security','cso',run.repoId,run.runId),leases=path.join(dir,'.mutation-lock-leases'); + const initialized=command(['resume',run.runId]);expectSuccessfulProcess(initialized,'gstack-cso initialize mutation lease'); + expect(fs.realpathSync(leases).startsWith(fs.realpathSync(profile)+path.sep)).toBe(true); + if(i===0){ + const churn=path.join(leases,'churn');let inode=0n; + for(let attempt=0;attempt<1024;attempt++){ + fs.writeFileSync(churn,'x');inode=fs.lstatSync(churn,{bigint:true}).ino;fs.unlinkSync(churn); + if(inode>BigInt(Number.MAX_SAFE_INTEGER))break; + } + expect(inode).toBeGreaterThan(BigInt(Number.MAX_SAFE_INTEGER)); + } + const inspected=command(['inspect',run.runId]);expectSuccessfulProcess(inspected,'gstack-cso inspect'); + expect(JSON.parse(inspected.stdout).report.status).toBe('running'); + const read=command(['read',run.runId,'app.js']);expectSuccessfulProcess(read,'gstack-cso read');expect(read.stdout).toContain('fixture'); + const history=command(['history',run.runId]);expectSuccessfulProcess(history,'gstack-cso history'); + const submitted=command(['submit',run.runId,submission]);expectSuccessfulProcess(submitted,'gstack-cso submit'); + const resumed=command(['resume',run.runId]);expectSuccessfulProcess(resumed,'gstack-cso resume'); + const finished=command(['finish',run.runId]);expectSuccessfulProcess(finished,'gstack-cso finish'); + expect(JSON.parse(finished.stdout).completeness).not.toBe('complete'); + const report=JSON.parse(fs.readFileSync(path.join(dir,'report.json'),'utf8')); + expect(report.coverage.some((entry:any)=>entry.status==='not_assessed')).toBe(true); + expect(fs.readdirSync(leases)).toEqual([]); + if(i!==2)continue; + const token='d'.repeat(32),candidate=path.join(leases,`${token}.json`),decision=path.join(leases,`${token}.decision`); + for(const rounded of [false,true]){ + let selected:string|undefined; + for(let batch=0;batch<16&&!selected;batch++){ + const paths:string[]=[]; + for(let index=0;index<64;index++){ + const file=path.join(leases,`fixture-${batch}-${index}`); + fs.writeFileSync(file,JSON.stringify({pid:2147483647,token,createdAt:0})+'\n');paths.push(file); + const inode=fs.lstatSync(file,{bigint:true}).ino; + if(!selected&&inode>BigInt(Number.MAX_SAFE_INTEGER)&&String(Number(inode))!==String(inode))selected=file; + } + for(const file of paths){if(file===selected)fs.renameSync(file,candidate);else fs.unlinkSync(file);} + } + expect(selected).toBeDefined(); + const stat=fs.lstatSync(candidate,{bigint:true}),record={schemaVersion:1,token,kind:'ticket',ticket:'0000000000000001',candidateDev:String(stat.dev),candidateIno:rounded?String(Number(stat.ino)):String(stat.ino),ownerPid:2147483647,ownerCreatedAt:0,publisherPid:2147483647,createdAt:0}; + fs.writeFileSync(decision,JSON.stringify(record)+'\n'); + const result=command(['resume',run.runId]);expect(result.status).not.toBe(0); + if(rounded){ + expect(record.candidateIno).not.toBe(String(stat.ino));expect(result.stderr).toContain('UNSAFE_PATH'); + expect(fs.existsSync(candidate)).toBe(true);expect(fs.existsSync(decision)).toBe(true); + fs.unlinkSync(decision);fs.unlinkSync(candidate); + }else{ + expect(result.stderr).toContain('INVALID_SCHEMA'); + expect(fs.existsSync(candidate)).toBe(false);expect(fs.existsSync(decision)).toBe(false); + } + } + const next=command(['start','--repo',repository,'--offline']);expectSuccessfulProcess(next,'gstack-cso start after legacy state'); + expect(JSON.parse(next.stdout).runId).not.toBe(run.runId); + } + }, 180_000); + test('the actual helper rejects source mutation during snapshot capture without certifying a report', async () => { const repository=path.join(temporary,'racing repository'),profile=path.join(temporary,'race profile'),padding=path.join(repository,'padding'),target=path.join(repository,'zzzz-race-target.js'); fs.mkdirSync(repository);fs.mkdirSync(profile);fs.mkdirSync(padding); diff --git a/test/gstack-artifacts-init.test.ts b/test/gstack-artifacts-init.test.ts index b1dd3c84b..b6adb3c1f 100644 --- a/test/gstack-artifacts-init.test.ts +++ b/test/gstack-artifacts-init.test.ts @@ -16,6 +16,7 @@ import * as fs from 'fs'; import * as os from 'os'; import * as path from 'path'; import { spawnSync } from 'child_process'; +import { canRevokeReads } from './helpers/fs-caps'; // Integration tests spawn real git/gh/glab subprocesses. The default 5s // per-test timeout is tight on developer machines; raise to 30s to match @@ -150,6 +151,7 @@ function run(argv: string[], opts: { env?: Record; input?: strin GSTACK_HOME: tmpHome, USER: 'testuser', HOME: tmpHome, + GIT_CONFIG_NOSYSTEM: '1', ...(opts.env || {}), }; const res = spawnSync(INIT_BIN, argv, { @@ -166,11 +168,23 @@ function run(argv: string[], opts: { env?: Record; input?: strin }; } +function gitInFixture(argv: string[]) { + return spawnSync('git', argv, { + encoding: 'utf-8', + timeout: 30_000, + env: { ...process.env, GIT_CONFIG_NOSYSTEM: '1' }, + }); +} + function readCalls(file: string): string[] { if (!fs.existsSync(file)) return []; return fs.readFileSync(file, 'utf-8').trim().split('\n').filter(Boolean); } +function expectNoAllowlistTemps() { + expect(fs.readdirSync(tmpHome).filter((name) => name.startsWith('.brain-allowlist.'))).toEqual([]); +} + beforeEach(() => { tmpHome = fs.mkdtempSync(path.join(os.tmpdir(), 'artifacts-init-')); bareRemote = fs.mkdtempSync(path.join(os.tmpdir(), 'artifacts-bare-')); @@ -239,6 +253,7 @@ describe('gstack-artifacts-init provider selection', () => { }); test('only glab authed → defaults to gitlab (no prompt)', () => { + makeFakeGh({ authStatus: 'fail' }); makeFakeGlab({}); const r = run([]); expect(r.status).toBe(0); @@ -246,7 +261,8 @@ describe('gstack-artifacts-init provider selection', () => { }); test('neither authed → falls through to manual URL paste', () => { - // No gh, no glab fakes. + makeFakeGh({ authStatus: 'fail' }); + makeFakeGlab({ authStatus: 'fail' }); const r = run([], { input: 'https://github.com/testuser/gstack-artifacts-testuser\n' }); expect(r.status).toBe(0); expect(r.stderr).toContain('Neither gh nor glab'); @@ -278,7 +294,7 @@ describe('gstack-artifacts-init canonical URL storage (codex Finding #10)', () = makeFakeGh({ webUrl: 'https://github.com/testuser/gstack-artifacts-testuser' }); const r = run(['--host', 'github']); expect(r.status).toBe(0); - const remote = spawnSync('git', ['-C', tmpHome, 'remote', 'get-url', 'origin'], { encoding: 'utf-8', timeout: 30_000 }); + const remote = gitInFixture(['-C', tmpHome, 'remote', 'get-url', 'origin']); expect(remote.stdout.trim()).toBe('https://github.com/testuser/gstack-artifacts-testuser'); }); @@ -289,7 +305,7 @@ describe('gstack-artifacts-init canonical URL storage (codex Finding #10)', () = }); const r = run(['--host', 'github']); expect(r.status).toBe(0); - const remote = spawnSync('git', ['-C', tmpHome, 'remote', 'get-url', 'origin'], { encoding: 'utf-8', timeout: 30_000 }); + const remote = gitInFixture(['-C', tmpHome, 'remote', 'get-url', 'origin']); expect(remote.stdout.trim()).toBe('git@github.com:testuser/gstack-artifacts-testuser.git'); }); @@ -297,7 +313,7 @@ describe('gstack-artifacts-init canonical URL storage (codex Finding #10)', () = makeFakeGh({ gitProtocol: 'unset' }); const r = run(['--host', 'github']); expect(r.status).toBe(0); - const remote = spawnSync('git', ['-C', tmpHome, 'remote', 'get-url', 'origin'], { encoding: 'utf-8', timeout: 30_000 }); + const remote = gitInFixture(['-C', tmpHome, 'remote', 'get-url', 'origin']); expect(remote.stdout.trim()).toBe('https://github.com/testuser/gstack-artifacts-testuser'); }); @@ -305,7 +321,7 @@ describe('gstack-artifacts-init canonical URL storage (codex Finding #10)', () = makeFakeGlab({ gitProtocol: 'ssh' }); const r = run(['--host', 'gitlab']); expect(r.status).toBe(0); - const remote = spawnSync('git', ['-C', tmpHome, 'remote', 'get-url', 'origin'], { encoding: 'utf-8', timeout: 30_000 }); + const remote = gitInFixture(['-C', tmpHome, 'remote', 'get-url', 'origin']); expect(remote.stdout.trim()).toBe('git@gitlab.com:testuser/gstack-artifacts-testuser.git'); }); }); @@ -360,7 +376,7 @@ describe('gstack-artifacts-init idempotency', () => { makeFakeGh({ gitProtocol: 'ssh' }); const r = run(['--remote', 'https://github.com/testuser/gstack-artifacts-testuser']); expect(r.status).toBe(0); - const remote = spawnSync('git', ['-C', tmpHome, 'remote', 'get-url', 'origin'], { encoding: 'utf-8', timeout: 30_000 }); + const remote = gitInFixture(['-C', tmpHome, 'remote', 'get-url', 'origin']); expect(remote.stdout.trim()).toBe('https://github.com/testuser/gstack-artifacts-testuser'); }); @@ -373,7 +389,7 @@ describe('gstack-artifacts-init idempotency', () => { 'ssh', ]); expect(r.status).toBe(0); - const remote = spawnSync('git', ['-C', tmpHome, 'remote', 'get-url', 'origin'], { encoding: 'utf-8', timeout: 30_000 }); + const remote = gitInFixture(['-C', tmpHome, 'remote', 'get-url', 'origin']); expect(remote.stdout.trim()).toBe('git@github.com:testuser/gstack-artifacts-testuser.git'); }); @@ -392,6 +408,164 @@ describe('gstack-artifacts-init idempotency', () => { expect(r2.status).toBe(0); }); + test('re-run preserves the user allowlist suffix byte-for-byte', () => { + makeFakeGh({}); + const url = 'https://github.com/testuser/gstack-artifacts-testuser'; + expect(run(['--remote', url]).status).toBe(0); + const allowlist = path.join(tmpHome, '.brain-allowlist'); + const marker = '# ---- USER ADDITIONS BELOW ---- (survives re-init; above is managed)\n'; + const suffix = '\n# user comment\n\ncustom/path-*.jsonl'; + const original = fs.readFileSync(allowlist, 'utf-8'); + fs.writeFileSync(allowlist, original.replace('projects/*/timeline.jsonl', 'projects/*/stale-managed-path.jsonl')); + fs.chmodSync(allowlist, 0o600); + fs.appendFileSync(allowlist, suffix); + + expect(run(['--remote', url]).status).toBe(0); + const updated = fs.readFileSync(allowlist); + expect(updated.subarray(updated.indexOf(marker) + Buffer.byteLength(marker))).toEqual(Buffer.from(suffix)); + expect(updated.toString().split(marker).length - 1).toBe(1); + expect(updated.toString()).toContain('projects/*/timeline.jsonl'); + expect(updated.toString()).not.toContain('projects/*/stale-managed-path.jsonl'); + expect(fs.statSync(allowlist).mode & 0o777).toBe(0o600); + expectNoAllowlistTemps(); + }); + + test('empty allowlist is initialized with only the current managed rules', () => { + makeFakeGh({}); + const url = 'https://github.com/testuser/gstack-artifacts-testuser'; + expect(run(['--remote', url]).status).toBe(0); + const allowlist = path.join(tmpHome, '.brain-allowlist'); + fs.writeFileSync(allowlist, ''); + + expect(run(['--remote', url]).status).toBe(0); + const updated = fs.readFileSync(allowlist, 'utf-8'); + expect(updated).toContain('projects/*/timeline.jsonl'); + expect(updated.split('# ---- USER ADDITIONS BELOW ----').length - 1).toBe(1); + expectNoAllowlistTemps(); + }); + + test('marker-only file without a final newline is refreshed without duplication', () => { + makeFakeGh({}); + const url = 'https://github.com/testuser/gstack-artifacts-testuser'; + expect(run(['--remote', url]).status).toBe(0); + const allowlist = path.join(tmpHome, '.brain-allowlist'); + const marker = '# ---- USER ADDITIONS BELOW ---- (survives re-init; above is managed)'; + fs.writeFileSync(allowlist, marker); + + expect(run(['--remote', url]).status).toBe(0); + const updated = fs.readFileSync(allowlist, 'utf-8'); + expect(updated.split(marker).length - 1).toBe(1); + expect(updated.endsWith('\n')).toBe(true); + expectNoAllowlistTemps(); + }); + + test('duplicate managed markers are refused without changing the file', () => { + makeFakeGh({}); + const url = 'https://github.com/testuser/gstack-artifacts-testuser'; + expect(run(['--remote', url]).status).toBe(0); + const allowlist = path.join(tmpHome, '.brain-allowlist'); + const original = fs.readFileSync(allowlist); + fs.appendFileSync(allowlist, original.subarray(original.indexOf(Buffer.from('# ---- USER ADDITIONS BELOW ----')))); + const ambiguous = fs.readFileSync(allowlist); + + const r = run(['--remote', url]); + expect(r.status).not.toBe(0); + expect(fs.readFileSync(allowlist)).toEqual(ambiguous); + expectNoAllowlistTemps(); + }); + + test('markerless legacy allowlist is retained and re-init refuses ambiguity', () => { + makeFakeGh({}); + const url = 'https://github.com/testuser/gstack-artifacts-testuser'; + expect(run(['--remote', url]).status).toBe(0); + const allowlist = path.join(tmpHome, '.brain-allowlist'); + const legacy = Buffer.from('# user rules\ncustom/legacy.jsonl'); + fs.writeFileSync(allowlist, legacy); + + const r = run(['--remote', url]); + expect(r.status).not.toBe(0); + expect(fs.readFileSync(allowlist)).toEqual(legacy); + expectNoAllowlistTemps(); + }); + + test('failed allowlist replacement leaves the old file intact', () => { + makeFakeGh({}); + const url = 'https://github.com/testuser/gstack-artifacts-testuser'; + expect(run(['--remote', url]).status).toBe(0); + const allowlist = path.join(tmpHome, '.brain-allowlist'); + fs.appendFileSync(allowlist, '\ncustom/failure-control.jsonl'); + const previous = fs.readFileSync(allowlist); + const fakeMv = path.join(fakeBinDir, 'mv'); + fs.writeFileSync(fakeMv, `#!/bin/bash\nlast=\"\"\nfor arg in \"$@\"; do last=\"$arg\"; done\nif [ \"$last\" = \"$GSTACK_HOME/.brain-allowlist\" ]; then exit 73; fi\nexec /bin/mv \"$@\"\n`, { mode: 0o755 }); + + const r = run(['--remote', url]); + expect(r.status).not.toBe(0); + expect(fs.readFileSync(allowlist)).toEqual(previous); + expectNoAllowlistTemps(); + }); + + test('allowlist read failure leaves the old file intact', () => { + makeFakeGh({}); + const url = 'https://github.com/testuser/gstack-artifacts-testuser'; + expect(run(['--remote', url]).status).toBe(0); + const allowlist = path.join(tmpHome, '.brain-allowlist'); + fs.appendFileSync(allowlist, '\ncustom/read-failure-control.jsonl'); + const previous = fs.readFileSync(allowlist); + const fakeGrep = path.join(fakeBinDir, 'grep'); + fs.writeFileSync(fakeGrep, `#!/bin/bash\nfor arg in \"$@\"; do if [ \"$arg\" = \"$GSTACK_HOME/.brain-allowlist\" ]; then exit 2; fi; done\nexec /bin/grep \"$@\"\n`, { mode: 0o755 }); + + const r = run(['--remote', url]); + expect(r.status).not.toBe(0); + expect(fs.readFileSync(allowlist)).toEqual(previous); + expectNoAllowlistTemps(); + }); + + _test.skipIf(!canRevokeReads())('real unreadable allowlist is preserved when chmod blocks reads', () => { + makeFakeGh({}); + const url = 'https://github.com/testuser/gstack-artifacts-testuser'; + expect(run(['--remote', url]).status).toBe(0); + const allowlist = path.join(tmpHome, '.brain-allowlist'); + fs.appendFileSync(allowlist, '\ncustom/unreadable-control.jsonl'); + const previous = fs.readFileSync(allowlist); + fs.chmodSync(allowlist, 0); + + try { + const r = run(['--remote', url]); + expect(r.status).not.toBe(0); + expectNoAllowlistTemps(); + } finally { + fs.chmodSync(allowlist, 0o600); + } + expect(fs.readFileSync(allowlist)).toEqual(previous); + }, 30_000); + + test('partial allowlist assembly failure preserves the original and cleans temporary files', () => { + makeFakeGh({}); + const url = 'https://github.com/testuser/gstack-artifacts-testuser'; + expect(run(['--remote', url]).status).toBe(0); + const allowlist = path.join(tmpHome, '.brain-allowlist'); + fs.appendFileSync(allowlist, '\ncustom/assembly-failure-control.jsonl'); + const previous = fs.readFileSync(allowlist); + const fakeCat = path.join(fakeBinDir, 'cat'); + fs.writeFileSync(fakeCat, [ + '#!/bin/bash', + 'for arg in "$@"; do', + ' case "$arg" in', + ' "$GSTACK_HOME"/.brain-allowlist.*)', + ' if /bin/grep -qF \'# Canonical allowlist of paths that gstack-brain-sync will publish.\' "$arg"; then /bin/head -c 32 "$arg"; exit 73; fi', + ' ;;', + ' esac', + 'done', + 'exec /bin/cat "$@"', + '', + ].join('\n'), { mode: 0o755 }); + + const r = run(['--remote', url]); + expect(r.status).not.toBe(0); + expect(fs.readFileSync(allowlist)).toEqual(previous); + expectNoAllowlistTemps(); + }); + test('re-run with DIFFERENT --remote exits 1 with conflict message', () => { makeFakeGh({}); run(['--remote', 'https://github.com/testuser/gstack-artifacts-testuser']); diff --git a/test/gstack-settings-hook-symlink.test.ts b/test/gstack-settings-hook-symlink.test.ts new file mode 100644 index 000000000..cff8e07a6 --- /dev/null +++ b/test/gstack-settings-hook-symlink.test.ts @@ -0,0 +1,268 @@ +import { afterEach, beforeEach, describe, expect, test } from 'bun:test'; +import * as fs from 'node:fs'; +import * as path from 'node:path'; +import * as os from 'node:os'; +import { spawn, spawnSync } from 'node:child_process'; +import { canRevokeReads } from './helpers/fs-caps'; + +const hook = path.resolve(import.meta.dir, '../bin/gstack-settings-hook'); +let root: string; +let target: string; +let settings: string; + +beforeEach(() => { + root = fs.realpathSync(fs.mkdtempSync(path.join(os.tmpdir(), 'gstack-link-'))); + fs.mkdirSync(path.join(root, 'config')); + fs.mkdirSync(path.join(root, 'dotfiles')); + target = path.join(root, 'dotfiles/settings.json'); + settings = path.join(root, 'config/settings.json'); + fs.writeFileSync(target, '{"theme":"dark","hooks":{}}\n', { mode: 0o600 }); + fs.symlinkSync('../dotfiles/settings.json', settings); + expect(fs.realpathSync(settings)).toBe(target); + expect(target.startsWith(root + path.sep)).toBe(true); +}); + +afterEach(() => fs.rmSync(root, { recursive: true, force: true })); + +function env(file = settings) { + return { ...process.env, HOME: root, GSTACK_HOME: path.join(root, 'state'), GSTACK_SETTINGS_FILE: file }; +} + +function addArgs(source = 'link-test') { + return ['add-event', '--event', 'PostToolUse', '--command', '/fixture/hook', '--source', source]; +} + +function run(args: string[], file = settings) { + return spawnSync('bash', [hook, ...args], { env: env(file), encoding: 'utf8', timeout: 10_000 }); +} + +function runWithRealpathFailure(args: string[], code: string, afterTemp = false) { + const fakeBin = path.join(root, 'bin'); + const preload = path.join(root, 'realpath-permission.ts'); + fs.mkdirSync(fakeBin); + fs.writeFileSync(preload, `const fs = require('fs'); +const original = fs.realpathSync; +fs.realpathSync = (file, ...args) => { + if (String(file) === process.env.GSTACK_SETTINGS_INPUT && (!${afterTemp} || (process.env.GSTACK_TMP_PATH && fs.existsSync(process.env.GSTACK_TMP_PATH)))) { + throw Object.assign(new Error('synthetic realpath permission failure'), { code: ${JSON.stringify(code)} }); + } + return original(file, ...args); +}; +`); + fs.writeFileSync(path.join(fakeBin, 'bun'), '#!/bin/sh\nexec "$ACTUAL_BUN" --preload "$FS_PROBE" "$@"\n', { mode: 0o755 }); + return spawnSync('bash', [hook, ...args], { + env: { ...env(), PATH: fakeBin + path.delimiter + process.env.PATH, ACTUAL_BUN: process.execPath, FS_PROBE: preload }, + encoding: 'utf8', timeout: 10_000, + }); +} + +describe('settings hook preserves the resolved settings target', () => { + test('add, ensure and remove preserve the link, real target and private mode', () => { + const add = run(addArgs()); + expect(add.status, add.stderr).toBe(0); + expect(fs.lstatSync(settings).isSymbolicLink()).toBe(true); + expect(fs.readlinkSync(settings)).toBe('../dotfiles/settings.json'); + expect(JSON.parse(fs.readFileSync(target, 'utf8')).hooks.PostToolUse[0]._gstack_source).toBe('link-test'); + expect(fs.statSync(target).mode & 0o777).toBe(0o600); + const before = fs.readFileSync(target, 'utf8'); + const ensure = run(['ensure-event', ...addArgs().slice(1)]); + expect(ensure.status, ensure.stderr).toBe(0); + expect(fs.readFileSync(target, 'utf8')).toBe(before); + const remove = run(['remove-source', '--source', 'link-test']); + expect(remove.status, remove.stderr).toBe(0); + expect(fs.lstatSync(settings).isSymbolicLink()).toBe(true); + expect(JSON.parse(fs.readFileSync(target, 'utf8')).theme).toBe('dark'); + expect(fs.readFileSync(target, 'utf8')).not.toContain('link-test'); + }); + + test('backup and rollback are siblings of the real target, not the link', () => { + const original = fs.readFileSync(target, 'utf8'); + const add = run(addArgs()); + expect(add.status, add.stderr).toBe(0); + expect(fs.existsSync(target + '.bak-latest')).toBe(true); + expect(fs.existsSync(settings + '.bak-latest')).toBe(false); + const rollback = run(['rollback']); + expect(rollback.status, rollback.stderr).toBe(0); + expect(fs.lstatSync(settings).isSymbolicLink()).toBe(true); + expect(fs.readFileSync(target, 'utf8')).toBe(original); + expect(fs.statSync(target).mode & 0o777).toBe(0o600); + }); + + test('different aliases contend for the same target lock', () => { + fs.mkdirSync(target + '.lock'); + fs.writeFileSync(target + '.lock/owner', 'fixture-owner'); + const result = spawnSync('bash', [hook, ...addArgs()], { + env: { ...env(), GSTACK_SETTINGS_LOCK_TIMEOUT_MS: '100' }, + encoding: 'utf8', timeout: 10_000, + }); + expect(result.status).toBe(5); + expect(result.stderr).toContain('could not acquire lock'); + expect(fs.lstatSync(settings).isSymbolicLink()).toBe(true); + expect(fs.readFileSync(target, 'utf8')).not.toContain('link-test'); + expect(fs.existsSync(settings + '.lock')).toBe(false); + }); + + test('dangling and cyclic links are rejected without replacing them', () => { + fs.unlinkSync(target); + const dangling = run(addArgs()); + expect(dangling.status).not.toBe(0); + expect(fs.lstatSync(settings).isSymbolicLink()).toBe(true); + expect(fs.existsSync(target)).toBe(false); + fs.symlinkSync('../config/settings.json', target); + const cyclic = run(addArgs()); + expect(cyclic.status).not.toBe(0); + expect(fs.lstatSync(settings).isSymbolicLink()).toBe(true); + expect(fs.lstatSync(target).isSymbolicLink()).toBe(true); + }); + + test('malformed linked settings stay unchanged', () => { + fs.writeFileSync(target, '{not json'); + const result = run(addArgs()); + expect(result.status).toBe(3); + expect(fs.lstatSync(settings).isSymbolicLink()).toBe(true); + expect(fs.readFileSync(target, 'utf8')).toBe('{not json'); + }); + + test('a missing ordinary settings file can still be created', () => { + const fresh = path.join(root, 'config/fresh.json'); + const result = run(addArgs(), fresh); + expect(result.status, result.stderr).toBe(0); + expect(fs.lstatSync(fresh).isFile()).toBe(true); + expect(JSON.parse(fs.readFileSync(fresh, 'utf8')).hooks.PostToolUse).toHaveLength(1); + expect(fs.statSync(fresh).mode & 0o777).toBe(0o600); + }); + + test('read-only commands and absent-parent no-ops do not create files', () => { + expect(run(addArgs()).status).toBe(0); + const original = fs.readFileSync(target, 'utf8'); + const siblings = fs.readdirSync(path.dirname(target)); + expect(run(['list-sources']).stdout).toContain('link-test'); + expect(run(['list-items', '--event', 'PostToolUse']).stdout).toContain('/fixture/hook'); + expect(run(['diff-event', ...addArgs().slice(1)]).status).toBe(0); + expect(fs.readFileSync(target, 'utf8')).toBe(original); + expect(fs.readdirSync(path.dirname(target))).toEqual(siblings); + expect(fs.lstatSync(settings).isSymbolicLink()).toBe(true); + const missing = path.join(root, 'absent/config/settings.json'); + expect(run(['list-sources'], missing).status).toBe(0); + expect(run(['remove-source', '--source', 'link-test'], missing).status).toBe(0); + expect(fs.existsSync(path.join(root, 'absent'))).toBe(false); + }); + + test('legacy add, remove and prune-stale use the real target', () => { + const command = path.join(root, 'missing/bin/gstack-session-update'); + expect(run(['add', command]).status).toBe(0); + expect(fs.readFileSync(target, 'utf8')).toContain(command); + expect(run(['remove', command]).status).toBe(0); + expect(fs.readFileSync(target, 'utf8')).not.toContain(command); + expect(run(['add', command]).status).toBe(0); + expect(run(['prune-stale']).status).toBe(0); + expect(fs.readFileSync(target, 'utf8')).not.toContain(command); + expect(fs.lstatSync(settings).isSymbolicLink()).toBe(true); + expect(JSON.parse(fs.readFileSync(target, 'utf8')).theme).toBe('dark'); + }); + + test.skipIf(!canRevokeReads())('an unreadable linked target stays untouched', () => { + const original = fs.readFileSync(target, 'utf8'); + fs.chmodSync(target, 0o000); + try { + const result = run(addArgs()); + expect(result.status, result.stderr).toBe(3); + expect(result.stderr).toContain('cannot read'); + expect(fs.lstatSync(settings).isSymbolicLink()).toBe(true); + expect(fs.readdirSync(path.dirname(target))).toEqual(['settings.json']); + } finally { + fs.chmodSync(target, 0o600); + } + expect(fs.readFileSync(target, 'utf8')).toBe(original); + }); + + for (const code of ['EACCES', 'EPERM']) { + test(`a ${code} realpath failure keeps the unreadable-settings exit contract`, () => { + const original = fs.readFileSync(target, 'utf8'); + const result = runWithRealpathFailure(addArgs(), code); + expect(result.status, result.stderr).toBe(3); + expect(result.stderr).toContain('cannot read'); + expect(fs.lstatSync(settings).isSymbolicLink()).toBe(true); + expect(fs.readdirSync(path.dirname(target))).toEqual(['settings.json']); + expect(fs.readFileSync(target, 'utf8')).toBe(original); + }); + } + + for (const action of ['add-event', 'rollback']) { + test(`a late permission failure during ${action} removes its temporary settings copy`, () => { + if (action === 'rollback') expect(run(addArgs()).status).toBe(0); + const original = fs.readFileSync(target, 'utf8'); + const result = runWithRealpathFailure(action === 'rollback' ? ['rollback'] : addArgs(), 'EACCES', true); + expect(result.status, result.stderr).toBe(3); + expect(result.stderr).toContain('cannot read'); + expect(fs.lstatSync(settings).isSymbolicLink()).toBe(true); + expect(fs.readFileSync(target, 'utf8')).toBe(original); + expect(fs.readdirSync(path.dirname(target)).filter(name => name.startsWith('settings.json.tmp.'))).toEqual([]); + }); + } + + test('concurrent writes through file and directory aliases both survive', async () => { + const directoryAlias = path.join(root, 'linked-directory'); + fs.symlinkSync('dotfiles', directoryAlias); + const directorySettings = path.join(directoryAlias, 'settings.json'); + expect(fs.realpathSync(directorySettings)).toBe(target); + const first = Bun.spawn(['bash', hook, ...addArgs()], { env: env(), stdout: 'pipe', stderr: 'pipe' }); + const second = Bun.spawn(['bash', hook, 'add-event', '--event', 'Stop', '--command', '/fixture/stop', '--source', 'second-source'], { + env: env(directorySettings), stdout: 'pipe', stderr: 'pipe', + }); + try { + const statuses = await Promise.all([first.exited, second.exited]); + expect(statuses).toEqual([0, 0]); + const stored = JSON.parse(fs.readFileSync(target, 'utf8')); + expect(stored.hooks.PostToolUse[0]._gstack_source).toBe('link-test'); + expect(stored.hooks.Stop[0]._gstack_source).toBe('second-source'); + expect(fs.lstatSync(settings).isSymbolicLink()).toBe(true); + expect(fs.lstatSync(directoryAlias).isSymbolicLink()).toBe(true); + } finally { + if (first.exitCode === null) first.kill(); + if (second.exitCode === null) second.kill(); + await Promise.all([first.exited, second.exited]); + } + }); + + test('a link retargeted while waiting for its lock cannot mutate either target', async () => { + const other = path.join(root, 'dotfiles/other.json'); + const original = fs.readFileSync(target, 'utf8'); + fs.writeFileSync(other, original); + fs.mkdirSync(target + '.lock'); + fs.writeFileSync(target + '.lock/owner', 'fixture-owner'); + const bin = path.join(root, 'bin'); + fs.mkdirSync(bin); + const realMkdir = spawnSync('which', ['mkdir'], { encoding: 'utf8', timeout: 5_000 }).stdout.trim(); + expect(realMkdir).not.toBe(''); + const observed = path.join(root, 'lock-observed'); + fs.writeFileSync(path.join(bin, 'mkdir'), '#!/bin/bash\nif [ "$1" = "$LOCK_EXPECTED" ]; then printf seen > "$LOCK_OBSERVED"; fi\nexec "$REAL_MKDIR" "$@"\n', { mode: 0o755 }); + const child = spawn('bash', [hook, ...addArgs()], { + env: { ...env(), PATH: `${bin}:${process.env.PATH}`, LOCK_EXPECTED: target + '.lock', LOCK_OBSERVED: observed, REAL_MKDIR: realMkdir }, + stdio: ['ignore', 'pipe', 'pipe'], timeout: 5_000, + }); + let stderr = ''; + child.stdout.resume(); + child.stderr.on('data', data => { stderr += data; }); + const finished = new Promise((resolve, reject) => { + child.once('error', reject); + child.once('close', resolve); + }); + try { + const deadline = Date.now() + 2_000; + while (!fs.existsSync(observed) && Date.now() < deadline) await Bun.sleep(10); + expect(fs.existsSync(observed)).toBe(true); + fs.unlinkSync(settings); + fs.symlinkSync('../dotfiles/other.json', settings); + expect(fs.realpathSync(settings)).toBe(other); + fs.rmSync(target + '.lock', { recursive: true }); + expect(await finished, stderr).not.toBe(0); + expect(fs.readFileSync(target, 'utf8')).toBe(original); + expect(fs.readFileSync(other, 'utf8')).toBe(original); + expect(fs.lstatSync(settings).isSymbolicLink()).toBe(true); + } finally { + if (child.exitCode === null) child.kill(); + await finished; + } + }); +}); diff --git a/test/helpers/hermetic-env.test.ts b/test/helpers/hermetic-env.test.ts index a92a810a6..74901ff83 100644 --- a/test/helpers/hermetic-env.test.ts +++ b/test/helpers/hermetic-env.test.ts @@ -116,6 +116,49 @@ describe('buildHermeticEnv allowlist', () => { expect(e.GH_TOKEN).toBeUndefined(); // not in extraAllow }); + test('prefixes keep CI metadata but do not admit credential-shaped operator names', () => { + const base = { + ...CONTAMINATED, + GITHUB_TOKEN: 'synthetic-token', + GITHUB_PERSONAL_ACCESS_TOKEN: 'synthetic-pat', + GITHUB_APP_PRIVATE_KEY: 'synthetic-private-key', + GITHUB_CLIENT_SECRET: 'synthetic-client-secret', + GITHUB_PAT: 'synthetic-pat-short', + EVALS_API_KEY: 'synthetic-eval-key', + GITHUB_SHA: 'abc123', + GITHUB_PATH: '/tmp/actions-path', + GITHUB_TOKENIZER: 'metadata-tokenizer', + GITHUB_KEYRING: 'metadata-keyring', + EVALS_RUN_ID: 'run-123', + EVALS_SELECTION_JSON: '{}', + }; + const result = buildHermeticEnv(base, HERMETIC_VARS); + for (const name of [ + 'GITHUB_TOKEN', 'GITHUB_PERSONAL_ACCESS_TOKEN', 'GITHUB_APP_PRIVATE_KEY', + 'GITHUB_CLIENT_SECRET', 'GITHUB_PAT', 'EVALS_API_KEY', 'GH_TOKEN', + ]) expect(result[name]).toBeUndefined(); + for (const name of [ + 'GITHUB_ACTIONS', 'GITHUB_SHA', 'GITHUB_PATH', 'GITHUB_TOKENIZER', + 'GITHUB_KEYRING', 'EVALS_MODEL', 'EVALS_RUN_ID', 'EVALS_SELECTION_JSON', + ]) expect(result[name]).toBe(base[name]); + }); + + test('explicit provider auth, runner admissions, and overrides still win', () => { + const base = { + ...CONTAMINATED, + GITHUB_TOKEN: 'synthetic-token', + GEMINI_API_KEY: 'synthetic-gemini', + }; + const result = buildHermeticEnv(base, HERMETIC_VARS, { + GITHUB_APP_PRIVATE_KEY: 'synthetic-override', + }, { extraAllow: ['GEMINI_*', 'GITHUB_TOKEN'] }); + expect(result.ANTHROPIC_API_KEY).toBe(base.ANTHROPIC_API_KEY); + expect(result.GEMINI_API_KEY).toBe(base.GEMINI_API_KEY); + expect(result.GITHUB_TOKEN).toBe(base.GITHUB_TOKEN); + expect(result.GITHUB_APP_PRIVATE_KEY).toBe('synthetic-override'); + expect(buildHermeticEnv(base, HERMETIC_VARS).GITHUB_TOKEN).toBeUndefined(); + }); + test('TERM falls back when base omits it', () => { const base = { ...CONTAMINATED } as NodeJS.ProcessEnv; delete base.TERM; diff --git a/test/helpers/hermetic-env.ts b/test/helpers/hermetic-env.ts index 815b4cfec..e1cf29289 100644 --- a/test/helpers/hermetic-env.ts +++ b/test/helpers/hermetic-env.ts @@ -63,12 +63,18 @@ const ALLOW_EXACT = new Set([ /** Prefix rules: eval-harness knobs + CI metadata. Deliberately NOT here: * CONDUCTOR_* / CLAUDE_* (incl. CLAUDECODE, CLAUDE_CODE_ENTRYPOINT) / * GSTACK_* / MCP_* / GBRAIN_* — session-context contamination; and operator - * credentials (GH_TOKEN, SSH_AUTH_SOCK, GIT_*, OPENAI_API_KEY, + * credentials (GH_TOKEN, GITHUB_*_TOKEN, SSH_AUTH_SOCK, GIT_*, OPENAI_API_KEY, * VOYAGE_API_KEY) — CI doesn't have them and eval children have no business * using them. A test that legitimately needs one opts in via its own env * override; a provider runner (codex/gemini) re-admits its auth vars via - * opts.extraAllow. */ + * opts.extraAllow. Prefix matches reject credential-shaped suffixes; exact + * and explicit runner admissions still win. */ const ALLOW_PREFIXES = ['EVALS_', 'GITHUB_']; +const CREDENTIAL_SUFFIXES = new Set([ + 'KEY', 'KEYS', 'TOKEN', 'TOKENS', 'SECRET', 'SECRETS', 'PASSWORD', 'PASSWD', + 'PASS', 'CREDENTIAL', 'CREDENTIALS', 'AUTH', 'PAT', 'DSN', 'COOKIE', + 'SESSION', 'PRIVATE', +]); export interface HermeticEnvOpts { /** Per-runner additional allowed names (exact match) or prefixes (entries @@ -115,7 +121,8 @@ export function buildHermeticEnv( const allowed = ALLOW_EXACT.has(k) || extraExact.has(k) || - ALLOW_PREFIXES.some((p) => k.startsWith(p)) || + (ALLOW_PREFIXES.some((p) => k.startsWith(p)) && + !CREDENTIAL_SUFFIXES.has(k.slice(k.lastIndexOf('_') + 1).toUpperCase())) || extraPrefixes.some((p) => k.startsWith(p)); if (allowed) out[k] = v; } diff --git a/test/helpers/session-runner.test.ts b/test/helpers/session-runner.test.ts index b7d221855..c91130337 100644 --- a/test/helpers/session-runner.test.ts +++ b/test/helpers/session-runner.test.ts @@ -1,6 +1,60 @@ import { describe, test, expect } from 'bun:test'; +import { spawnSync } from 'node:child_process'; +import * as fs from 'node:fs'; +import * as os from 'node:os'; +import * as path from 'node:path'; +import { pathToFileURL } from 'node:url'; import { parseNDJSON } from './session-runner'; +test('runSkillTest launches a child without operator credentials', () => { + const root = fs.mkdtempSync(path.join(os.tmpdir(), 'gstack-hermetic-session-')); + try { + const bin = path.join(root, 'claude'); + fs.writeFileSync(bin, `#!/usr/bin/env node +const names = ['GITHUB_TOKEN', 'GITHUB_PERSONAL_ACCESS_TOKEN', 'GITHUB_APP_PRIVATE_KEY', 'GH_TOKEN', 'GITHUB_ACTIONS', 'GITHUB_PATH', 'GITHUB_TOKENIZER', 'EVALS_RUN_ID']; +const present = Object.fromEntries(names.map(name => [name, Object.hasOwn(process.env, name)])); +console.log(JSON.stringify({type: 'result', subtype: 'success', result: JSON.stringify(present)})); +`, { mode: 0o700 }); + const script = `import { runSkillTest } from ${JSON.stringify(pathToFileURL(path.join(import.meta.dir, 'session-runner.ts')).href)}; +const result = await runSkillTest({prompt: 'synthetic fixture', workingDirectory: ${JSON.stringify(root)}, model: 'fixture', timeout: 5000, startupGraceMs: 5000, allowedTools: []}); +console.log(JSON.stringify({exitReason: result.exitReason, child: JSON.parse(result.output)}));`; + const result = spawnSync(process.execPath, ['-e', script], { + cwd: path.resolve(import.meta.dir, '..', '..'), + encoding: 'utf8', + timeout: 30_000, + env: { + PATH: `${root}${path.delimiter}${process.env.PATH ?? '/usr/bin:/bin'}`, + HOME: root, + TMPDIR: os.tmpdir(), + GITHUB_TOKEN: 'synthetic-token', + GITHUB_PERSONAL_ACCESS_TOKEN: 'synthetic-pat', + GITHUB_APP_PRIVATE_KEY: 'synthetic-private-key', + GH_TOKEN: 'synthetic-gh-token', + GITHUB_ACTIONS: 'true', + GITHUB_PATH: '/tmp/actions-path', + GITHUB_TOKENIZER: 'metadata-tokenizer', + EVALS_RUN_ID: 'synthetic-run', + }, + }); + expect(result.status, result.stderr).toBe(0); + expect(JSON.parse(result.stdout)).toEqual({ + exitReason: 'success', + child: { + GITHUB_TOKEN: false, + GITHUB_PERSONAL_ACCESS_TOKEN: false, + GITHUB_APP_PRIVATE_KEY: false, + GH_TOKEN: false, + GITHUB_ACTIONS: true, + GITHUB_PATH: true, + GITHUB_TOKENIZER: true, + EVALS_RUN_ID: true, + }, + }); + } finally { + fs.rmSync(root, { recursive: true, force: true }); + } +}); + // Fixture: minimal NDJSON session (system init, assistant with tool_use, tool result, assistant text, result) const FIXTURE_LINES = [ '{"type":"system","subtype":"init","session_id":"test-123"}', diff --git a/test/redact-engine.test.ts b/test/redact-engine.test.ts index 48ee9316f..db9e1c02b 100644 --- a/test/redact-engine.test.ts +++ b/test/redact-engine.test.ts @@ -573,20 +573,10 @@ describe("redactFindingSpans — machine-egress masking (#1947)", () => { expect(out).toBe("first then end"); }); - test("fails closed (null) when a span cannot be relocated — never raw passthrough", () => { - // env.kv's span (the value) starts well past the regex match start (the - // var name), so locateSpan's rewind-2 re-exec misses it. The contract is - // null → caller drops the whole payload. The one thing that must never - // happen is the secret surviving in the output. + test("masks an anchored env.kv value rather than withholding the whole payload", () => { const secret = "8Fk2pQ9vXz4wL7mN3rT6yB1cD5eG0hJq"; const out = redactFindingSpans(`API_KEY=${secret}`, { repoVisibility: "private" }); - if (out !== null) { - // If locateSpan ever learns to find context-prefixed spans, masking - // must actually mask. - expect(out).not.toContain(secret); - } else { - expect(out).toBeNull(); - } + expect(out).toBe("API_KEY="); }); test("line/col at boundaries: line start, after blank lines, first char, last unterminated line", () => { @@ -603,7 +593,7 @@ describe("redactFindingSpans — machine-egress masking (#1947)", () => { expect(redactFindingSpans(`a\nb\n${token} x`, { repoVisibility: "private" })).toBe("a\nb\n x"); }); - test("multiline input redacts a finding past the first line (locateSpan line/col path)", () => { + test("multiline input redacts a finding past the first line (original span map)", () => { const token = "ghp_" + "1234567890abcdefghijklmnopqrstuvwxyz"; const out = redactFindingSpans(`line one\nline two has ${token}\nline three`, { repoVisibility: "private", diff --git a/test/redact-prepush-scan-range.test.ts b/test/redact-prepush-scan-range.test.ts index 2f2d31ec2..06fd4ec87 100644 --- a/test/redact-prepush-scan-range.test.ts +++ b/test/redact-prepush-scan-range.test.ts @@ -224,7 +224,7 @@ describe("S1: exclusion scoped to the push-target remote", () => { const head = run(["rev-parse", "HEAD"]).trim(); const { code, stderr } = runHook( `refs/heads/feature ${head} refs/heads/feature ${originTip}\n`, - ["origin", "file:///ignored"], + ["origin", run(["remote", "get-url", "--push", "origin"]).trim()], ); expect(code).toBe(1); expect(stderr).toContain("BLOCKED"); @@ -242,7 +242,7 @@ describe("S1: exclusion scoped to the push-target remote", () => { const head = run(["rev-parse", "HEAD"]).trim(); const { code, stderr } = runHook( `refs/heads/feature ${head} refs/heads/feature ${originTip}\n`, - ["origin", "file:///ignored"], + ["origin", run(["remote", "get-url", "--push", "origin"]).trim()], ); expect(stderr).not.toContain("BLOCKED"); expect(code).toBe(0); @@ -263,7 +263,7 @@ describe("S1: exclusion scoped to the push-target remote", () => { expect(code).toBe(0); }); - test("an unconfigured name (URL push) also falls back rather than erroring", () => { + test("an unconfigured URL push cannot borrow another remote's published history", () => { const { originTip } = buildSecretOnSecondRemote(); const head = run(["rev-parse", "HEAD"]).trim(); const url = "file:///not-a-configured-remote"; @@ -271,7 +271,7 @@ describe("S1: exclusion scoped to the push-target remote", () => { `refs/heads/feature ${head} refs/heads/feature ${originTip}\n`, [url, url], ); - expect(stderr).not.toContain("could not"); - expect(code).toBe(0); + expect(stderr).toContain("aws.access_key"); + expect(code).toBe(1); }); }); diff --git a/test/redact-prepush-target.test.ts b/test/redact-prepush-target.test.ts new file mode 100644 index 000000000..4635722ee --- /dev/null +++ b/test/redact-prepush-target.test.ts @@ -0,0 +1,393 @@ +import { afterEach, describe, expect, test } from "bun:test"; +import { spawnSync } from "node:child_process"; +import * as fs from "node:fs"; +import * as os from "node:os"; +import * as path from "node:path"; +import { scan } from "../lib/redact-engine"; + +const REDACT = path.resolve(import.meta.dir, "../bin/gstack-redact"); +const roots: string[] = []; +const key = ["AKIA", "1234567890ABCDEF"].join(""); + +function git(cwd: string, ...args: string[]): string { + const result = spawnSync("git", args, { cwd, encoding: "utf8", timeout: 30_000 }); + if (result.status !== 0) throw new Error(`git ${args[0]} failed: ${result.stderr}`); + return result.stdout.trim(); +} + +function fixture(initialSecret = true, sha256 = false): { repo: string; origin: string; publish: string; head: string } { + const root = fs.mkdtempSync(path.join(os.tmpdir(), "prepush-target-")); + roots.push(root); + const repo = path.join(root, "repo"); + const origin = path.join(root, "origin.git"); + const publish = path.join(root, "publish.git"); + fs.mkdirSync(repo); + const format = sha256 ? ["--object-format=sha256"] : []; + git(root, "init", "--bare", "-q", "-b", "main", ...format, origin); + git(root, "init", "--bare", "-q", "-b", "main", ...format, publish); + git(repo, "init", "-q", "-b", "main", ...format); + git(repo, "config", "user.name", "Fixture"); + git(repo, "config", "user.email", "fixture@example.test"); + fs.writeFileSync(path.join(repo, "config.txt"), initialSecret ? `key ${key}\n` : "clean\n"); + git(repo, "add", "config.txt"); + git(repo, "commit", "-qm", "seed"); + const head = git(repo, "rev-parse", "HEAD"); + git(repo, "remote", "add", "origin", origin); + git(repo, "push", "-q", "-u", "origin", "main"); + git(repo, "remote", "add", "publish", publish); + const install = spawnSync("bun", [REDACT, "install-prepush-hook"], { cwd: repo, encoding: "utf8", timeout: 30_000 }); + expect(install.status).toBe(0); + return { repo, origin, publish, head }; +} + +function seedIndependentRemote(repo: string, publish: string): string { + const producer = path.join(path.dirname(repo), "producer"); + fs.mkdirSync(producer); + git(producer, "init", "-q", "-b", "main"); + git(producer, "config", "user.name", "Fixture"); + git(producer, "config", "user.email", "fixture@example.test"); + fs.writeFileSync(path.join(producer, "clean.txt"), "different history\n"); + git(producer, "add", "clean.txt"); + git(producer, "commit", "-qm", "remote tip"); + git(producer, "remote", "add", "publish", publish); + git(producer, "push", "-q", "publish", "main"); + return git(publish, "rev-parse", "refs/heads/main"); +} + +afterEach(() => { + for (const root of roots.splice(0)) fs.rmSync(root, { recursive: true, force: true }); +}); + +describe("installed pre-push guard uses the actual destination", () => { + test("origin tracking at HEAD cannot excuse the same credential on first push to publish", () => { + const { repo, publish, head } = fixture(); + expect(git(repo, "rev-parse", "origin/main")).toBe(head); + const push = spawnSync("git", ["push", "publish", "main"], { cwd: repo, encoding: "utf8", timeout: 30_000 }); + expect(push.status).toBe(1); + expect(push.stderr).toContain("aws.access_key"); + expect(git(publish, "for-each-ref", "--format=%(refname)")).toBe(""); + }); + + test("a configured remote's different push URL cannot reuse its fetch tracking tip", () => { + const { repo, publish } = fixture(); + git(repo, "remote", "set-url", "--push", "origin", publish); + const push = spawnSync("git", ["push", "origin", "main"], { cwd: repo, encoding: "utf8", timeout: 30_000 }); + expect(push.status).toBe(1); + expect(push.stderr).toContain("aws.access_key"); + expect(git(publish, "for-each-ref", "--format=%(refname)")).toBe(""); + }); + + for (const [name, symbolicHead] of [ + ["a local branch cannot shadow the target's qualified remote-tracking base", true], + ["the main fallback cannot resolve to a colliding local branch", false], + ] as const) { + test(name, () => { + const { repo, publish, head: base } = fixture(false); + fs.writeFileSync(path.join(repo, "leak.txt"), `key ${key}\n`); + git(repo, "add", "leak.txt"); + git(repo, "commit", "-qm", "secret after target base"); + const tip = git(repo, "rev-parse", "HEAD"); + git(repo, "update-ref", "refs/remotes/publish/main", base); + if (symbolicHead) git(repo, "symbolic-ref", "refs/remotes/publish/HEAD", "refs/remotes/publish/main"); + git(repo, "update-ref", "refs/heads/publish/main", tip); + expect(git(repo, "rev-parse", "publish/main")).toBe(tip); + expect(git(repo, "rev-parse", "refs/remotes/publish/main")).toBe(base); + const push = spawnSync("git", ["push", "publish", "main"], { cwd: repo, encoding: "utf8", timeout: 30_000 }); + expect(push.status).toBe(1); + expect(push.stderr).toContain("aws.access_key"); + expect(git(publish, "for-each-ref", "--format=%(refname)")).toBe(""); + }); + } + + test("the second fetch URL is not represented by the first URL's tracking ref", () => { + const { repo, origin, publish } = fixture(); + const second = path.join(path.dirname(repo), "second.git"); + git(path.dirname(repo), "init", "--bare", "-q", "-b", "main", second); + git(publish, "fetch", "-q", origin, "main:refs/heads/main"); + git(repo, "fetch", "-q", "publish"); + expect(git(repo, "rev-parse", "refs/remotes/publish/main")).toBe(git(repo, "rev-parse", "HEAD")); + git(repo, "remote", "set-url", "--add", "publish", second); + expect(git(repo, "remote", "get-url", "publish")).toBe(publish); + const push = spawnSync("git", ["push", "publish", "main"], { cwd: repo, encoding: "utf8", timeout: 30_000 }); + expect(push.status).toBe(1); + expect(push.stderr).toContain("aws.access_key"); + expect(git(second, "for-each-ref", "--format=%(refname)")).toBe(""); + }); + + test("direct URL push cannot borrow origin's tracking history", () => { + const { repo, publish } = fixture(); + const push = spawnSync("git", ["push", publish, "main"], { cwd: repo, encoding: "utf8", timeout: 30_000 }); + expect(push.status).toBe(1); + expect(push.stderr).toContain("aws.access_key"); + expect(git(publish, "for-each-ref", "--format=%(refname)")).toBe(""); + }); + + test("an advertised remote tip absent from local objects cannot justify a guessed base", () => { + const { repo, publish } = fixture(); + const remoteTip = seedIndependentRemote(repo, publish); + expect(spawnSync("git", ["cat-file", "-e", remoteTip], { cwd: repo, timeout: 30_000 }).status).not.toBe(0); + const push = spawnSync("git", ["push", "--force", "publish", "main"], { cwd: repo, encoding: "utf8", timeout: 30_000 }); + expect(push.status).toBe(1); + expect(push.stderr).toContain("aws.access_key"); + expect(git(publish, "rev-parse", "refs/heads/main")).toBe(remoteTip); + }); + + test("an absent advertised tip permits a clean forced update after conservative scanning", () => { + const { repo, publish, head } = fixture(false); + const remoteTip = seedIndependentRemote(repo, publish); + expect(spawnSync("git", ["cat-file", "-e", remoteTip], { cwd: repo, timeout: 30_000 }).status).not.toBe(0); + const push = spawnSync("git", ["push", "--force", "publish", "main"], { cwd: repo, encoding: "utf8", timeout: 30_000 }); + expect(push.status).toBe(0); + expect(git(publish, "rev-parse", "refs/heads/main")).toBe(head); + }); + + test("new SHA-256 branch scans the correct empty tree and allows a clean push", () => { + const { repo, publish } = fixture(false, true); + const push = spawnSync("git", ["push", "publish", "main"], { cwd: repo, encoding: "utf8", timeout: 30_000 }); + expect(push.status).toBe(0); + expect(git(publish, "rev-parse", "refs/heads/main")).toHaveLength(64); + fs.writeFileSync(path.join(repo, "leak.txt"), `key ${key}\n`); + git(repo, "add", "leak.txt"); + git(repo, "commit", "-qm", "new secret"); + const blocked = spawnSync("git", ["push", "origin", "main"], { cwd: repo, encoding: "utf8", timeout: 30_000 }); + expect(blocked.status).toBe(1); + expect(blocked.stderr).toContain("aws.access_key"); + }); + + test("one leaky ref blocks a multi-ref push without updating its clean neighbor", () => { + const { repo, origin, head } = fixture(false); + git(repo, "checkout", "-q", "-b", "leak"); + fs.writeFileSync(path.join(repo, "leak.txt"), `key ${key}\n`); + git(repo, "add", "leak.txt"); + git(repo, "commit", "-qm", "leaky branch"); + git(repo, "checkout", "-q", "main"); + fs.writeFileSync(path.join(repo, "clean.txt"), "safe update\n"); + git(repo, "add", "clean.txt"); + git(repo, "commit", "-qm", "clean branch"); + const push = spawnSync("git", ["push", "origin", "main", "leak"], { cwd: repo, encoding: "utf8", timeout: 30_000 }); + expect(push.status).toBe(1); + expect(push.stderr).toContain("aws.access_key"); + expect(git(origin, "rev-parse", "refs/heads/main")).toBe(head); + expect(git(origin, "for-each-ref", "--format=%(refname)", "refs/heads/leak")).toBe(""); + }); + + test("a deleted ref alongside a clean update is permitted", () => { + const { repo, origin } = fixture(false); + git(repo, "checkout", "-q", "-b", "disposable"); + fs.writeFileSync(path.join(repo, "disposable.txt"), "safe\n"); + git(repo, "add", "disposable.txt"); + git(repo, "commit", "-qm", "temporary branch"); + git(repo, "push", "-q", "origin", "disposable"); + git(repo, "checkout", "-q", "main"); + fs.writeFileSync(path.join(repo, "clean.txt"), "safe update\n"); + git(repo, "add", "clean.txt"); + git(repo, "commit", "-qm", "clean update"); + const head = git(repo, "rev-parse", "HEAD"); + const push = spawnSync("git", ["push", "origin", "main", ":disposable"], { cwd: repo, encoding: "utf8", timeout: 30_000 }); + expect(push.status).toBe(0); + expect(git(origin, "rev-parse", "refs/heads/main")).toBe(head); + expect(git(origin, "for-each-ref", "--format=%(refname)", "refs/heads/disposable")).toBe(""); + }); + + test("installed wrapper forwards refs and rejects malformed input instead of passing it on", () => { + const { repo, origin, head } = fixture(false); + const hook = git(repo, "rev-parse", "--git-path", "hooks/pre-push"); + const result = spawnSync("bash", [hook, "origin", origin], { + cwd: repo, input: "refs/heads/main malformed\n", encoding: "utf8", timeout: 30_000, + }); + expect(result.status, result.stderr || result.error?.message).toBe(1); + expect(result.stderr).toContain("could not parse a pre-push ref line"); + expect(git(origin, "rev-parse", "refs/heads/main")).toBe(head); + }); + + test("a HIGH proximity finding across line-aligned scan slices cannot pass", () => { + const { repo } = fixture(false); + const value = ["AbCdEfGhIjKlMnOpQrStU", "vWxYz0123456789AbCd"].join(""); + const payload = `${"x".repeat(768 * 1024 - 43)}\naws_secret_access_key=\n${value}\n`; + expect(scan(payload).findings.map((finding) => finding.id)).toContain("aws.secret_key"); + fs.writeFileSync(path.join(repo, "payload.txt"), payload); + git(repo, "add", "payload.txt"); + git(repo, "commit", "-qm", "near seam"); + const push = spawnSync("git", ["push", "origin", "main"], { cwd: repo, encoding: "utf8", timeout: 30_000 }); + expect(push.status).toBe(1); + expect(push.stderr).toContain("aws.secret_key"); + }); + + test("an unlabeled high-entropy value near a seam stays clean", () => { + const { repo } = fixture(false); + const value = ["AbCdEfGhIjKlMnOpQrStU", "vWxYz0123456789AbCd"].join(""); + const payload = `${"x".repeat(768 * 1024 - 43)}\nordinary description\n${value}\n`; + expect(scan(payload).findings.map((finding) => finding.id)).not.toContain("aws.secret_key"); + fs.writeFileSync(path.join(repo, "payload.txt"), payload); + git(repo, "add", "payload.txt"); + git(repo, "commit", "-qm", "unlabeled seam"); + const push = spawnSync("git", ["push", "origin", "main"], { cwd: repo, encoding: "utf8", timeout: 30_000 }); + expect(push.status).toBe(0); + expect(push.stderr).not.toContain("aws.secret_key"); + }); + + test("overlap does not count a single finding twice", () => { + const { repo } = fixture(false); + const payload = `${"x".repeat(768 * 1024 - 75)}\nkey ${key}\n${"x".repeat(100)}\n`; + fs.writeFileSync(path.join(repo, "payload.txt"), payload); + git(repo, "add", "payload.txt"); + git(repo, "commit", "-qm", "one finding at seam"); + const push = spawnSync("git", ["push", "origin", "main"], { cwd: repo, encoding: "utf8", timeout: 30_000 }); + expect(push.status).toBe(1); + expect(push.stderr.match(/HIGH aws\.access_key/g)).toHaveLength(1); + }); + + test("supplementary NFKC expansion cannot move a seam finding out of its owning core", () => { + const { repo, origin, head } = fixture(false); + const prefix = "\uFA6C".repeat(64); + const credential = `key ${key}\n`; + const padding = "x".repeat(768 * 1024 - Buffer.byteLength(prefix) - credential.length - 1); + const payload = `${prefix}${padding}\n${credential}${"z".repeat(100)}\n`; + expect(scan(payload).findings.map((finding) => finding.id)).toContain("aws.access_key"); + fs.writeFileSync(path.join(repo, "payload.txt"), payload); + git(repo, "add", "payload.txt"); + git(repo, "commit", "-qm", "supplementary normalization seam"); + const push = spawnSync("git", ["push", "origin", "main"], { cwd: repo, encoding: "utf8", timeout: 30_000 }); + expect(push.status).toBe(1); + expect(push.stderr.match(/HIGH aws\.access_key/g)).toHaveLength(1); + expect(git(origin, "rev-parse", "refs/heads/main")).toBe(head); + }); + + test("a partial-line overlap cannot manufacture an anchored assignment finding", () => { + const { repo } = fixture(false); + const value = ["8Fk2pQ9vXz4wL7mN", "3rT6yB1cD5eG0hJq"].join(""); + const assignment = `API_KEY=${value} `; + const suffix = "z".repeat(16 * 1024 - assignment.length - 6); + const first = "!" + "x".repeat(768 * 1024 - 61 - assignment.length - suffix.length) + assignment + suffix; + const payload = `${first}\nshort\n${"z".repeat(100)}\n`; + expect(scan(payload).findings.map((finding) => finding.id)).not.toContain("env.kv"); + fs.writeFileSync(path.join(repo, "payload.txt"), payload); + git(repo, "add", "payload.txt"); + git(repo, "commit", "-qm", "mid-line control"); + const push = spawnSync("git", ["push", "origin", "main"], { cwd: repo, encoding: "utf8", timeout: 30_000 }); + expect(push.status).toBe(0); + expect(push.stderr).not.toContain("MEDIUM finding"); + }); + + test("mid-line carry cannot create a HIGH word boundary around an embedded key", () => { + const { repo, origin, head } = fixture(false); + const first = "x".repeat(768 * 1024 - 1 - 16_384) + key + " ".repeat(16_384 - key.length); + const payload = `${first}\nordinary\n`; + expect(scan(payload).findings.map((finding) => finding.id)).not.toContain("aws.access_key"); + fs.writeFileSync(path.join(repo, "payload.txt"), payload); + git(repo, "add", "payload.txt"); + git(repo, "commit", "-qm", "embedded key control"); + const push = spawnSync("git", ["push", "origin", "main"], { cwd: repo, encoding: "utf8", timeout: 30_000 }); + expect(push.status).toBe(0); + expect(push.stderr).not.toContain("aws.access_key"); + expect(git(origin, "rev-parse", "refs/heads/main")).not.toBe(head); + }); + + test("a long Bearer span retains its later Authorization context across the seam", () => { + const { repo, origin } = fixture(false); + const alphabet = "7pFb4ZaCuG8wDsVk2EnHy6Qt9Jr5Lx0M"; + const token = Array.from({ length: 20_480 }, (_, i) => alphabet[(i * 7 + Math.floor(i / 31)) % 32]).join(""); + const bearerLine = `Bearer ${token}`; + const payload = `${"x".repeat(768 * 1024 - bearerLine.length - 2)}\n${bearerLine}\nAuthorization\n`; + expect(scan(payload).findings.map((finding) => finding.id)).toContain("auth.bearer"); + fs.writeFileSync(path.join(repo, "payload.txt"), payload); + git(repo, "add", "payload.txt"); + git(repo, "commit", "-qm", "long bearer advisory"); + const tip = git(repo, "rev-parse", "HEAD"); + const push = spawnSync("git", ["push", "origin", "main"], { cwd: repo, encoding: "utf8", timeout: 30_000 }); + expect(push.status).toBe(0); + expect(push.stderr).toContain("MEDIUM finding"); + expect(git(origin, "rev-parse", "refs/heads/main")).toBe(tip); + }); + + test("zero-width padding cannot move normalized proximity context out of the overlap", () => { + const { repo } = fixture(false); + const value = ["AbCdEfGhIjKlMnOpQrStU", "vWxYz0123456789AbCd"].join(""); + const payload = `${"x".repeat(768 * 1024 - 43)}\naws_secret_access_key=\n${"\u200b".repeat(35_000)}\n${value}\n`; + expect(scan(payload).findings.map((finding) => finding.id)).toContain("aws.secret_key"); + fs.writeFileSync(path.join(repo, "payload.txt"), payload); + git(repo, "add", "payload.txt"); + git(repo, "commit", "-qm", "invisible seam"); + const push = spawnSync("git", ["push", "origin", "main"], { cwd: repo, encoding: "utf8", timeout: 30_000 }); + expect(push.status).toBe(1); + expect(push.stderr).toContain("aws.secret_key"); + }); + + test("context needing more than the engine byte cap blocks instead of scanning a truncated window", () => { + const { repo, origin, head } = fixture(false); + const value = ["AbCdEfGhIjKlMnOpQrStU", "vWxYz0123456789AbCd"].join(""); + const payload = `${"x".repeat(768 * 1024 - 43)}\naws_secret_access_key=\n${"\u200b".repeat(450_000)}\n${value}\n`; + fs.writeFileSync(path.join(repo, "payload.txt"), payload); + git(repo, "add", "payload.txt"); + git(repo, "commit", "-qm", "unscannable context"); + const push = spawnSync("git", ["push", "origin", "main"], { cwd: repo, encoding: "utf8", timeout: 30_000 }); + expect(push.status).toBe(1); + expect(push.stderr).toContain("engine.input_too_large"); + expect(git(origin, "rev-parse", "refs/heads/main")).toBe(head); + }); + + for (const [name, label, expected] of [ + ["HTML entity", "aws&secret_access_key=", true], + ["fullwidth Unicode", "aws_secret_access_key=", true], + ["broken entity separated by zero-width", "aws&\u200bamp;secret_access_key=", false], + ] as const) { + test(`${name} near a seam keeps the detector's exact normalization semantics`, () => { + const { repo } = fixture(false); + const value = ["AbCdEfGhIjKlMnOpQrStU", "vWxYz0123456789AbCd"].join(""); + const payload = `${"x".repeat(768 * 1024 - 43)}\n${label}\n${value}\n`; + expect(scan(payload).findings.some((finding) => finding.id === "aws.secret_key")).toBe(expected); + fs.writeFileSync(path.join(repo, "payload.txt"), payload); + git(repo, "add", "payload.txt"); + git(repo, "commit", "-qm", "normalization seam"); + const push = spawnSync("git", ["push", "origin", "main"], { cwd: repo, encoding: "utf8", timeout: 30_000 }); + expect(push.status).toBe(expected ? 1 : 0); + expect(push.stderr.includes("aws.secret_key")).toBe(expected); + }); + } + + test("a large clean diff and a sub-cap long line pass, while an over-cap line fails closed", () => { + const { repo, origin, head } = fixture(false); + fs.writeFileSync(path.join(repo, "large.txt"), "ordinary content\n".repeat(160_000)); + git(repo, "add", "large.txt"); + git(repo, "commit", "-qm", "large clean diff"); + let push = spawnSync("git", ["push", "origin", "main"], { cwd: repo, encoding: "utf8", timeout: 60_000 }); + expect(push.status).toBe(0); + fs.writeFileSync(path.join(repo, "long.txt"), "x".repeat(900_000) + "\n"); + git(repo, "add", "long.txt"); + git(repo, "commit", "-qm", "long clean line"); + push = spawnSync("git", ["push", "origin", "main"], { cwd: repo, encoding: "utf8", timeout: 60_000 }); + expect(push.status).toBe(0); + const priorTip = git(origin, "rev-parse", "refs/heads/main"); + expect(priorTip).not.toBe(head); + fs.writeFileSync(path.join(repo, "too-long.txt"), "x".repeat(1_100_000) + "\n"); + git(repo, "add", "too-long.txt"); + git(repo, "commit", "-qm", "over cap line"); + push = spawnSync("git", ["push", "origin", "main"], { cwd: repo, encoding: "utf8", timeout: 60_000 }); + expect(push.status).toBe(1); + expect(push.stderr).toContain("engine.input_too_large"); + expect(git(origin, "rev-parse", "refs/heads/main")).toBe(priorTip); + }); + + test("a detectable credential at the end of a long single line is still blocked", () => { + const { repo, origin, head } = fixture(false); + fs.writeFileSync(path.join(repo, "long.txt"), `${"x".repeat(900_000)} key ${key}\n`); + git(repo, "add", "long.txt"); + git(repo, "commit", "-qm", "long line with key"); + const push = spawnSync("git", ["push", "origin", "main"], { cwd: repo, encoding: "utf8", timeout: 30_000 }); + expect(push.status).toBe(1); + expect(push.stderr).toContain("aws.access_key"); + expect(git(origin, "rev-parse", "refs/heads/main")).toBe(head); + }); + + test("neighboring individually scannable long clean lines do not force an oversize failure", () => { + const { repo, origin } = fixture(false); + fs.writeFileSync(path.join(repo, "long.txt"), `${"x".repeat(900_000)}\n${"y".repeat(900_000)}\n`); + git(repo, "add", "long.txt"); + git(repo, "commit", "-qm", "neighboring long lines"); + const tip = git(repo, "rev-parse", "HEAD"); + const push = spawnSync("git", ["push", "origin", "main"], { cwd: repo, encoding: "utf8", timeout: 30_000 }); + expect(push.status).toBe(0); + expect(push.stderr).not.toContain("engine.input_too_large"); + expect(git(origin, "rev-parse", "refs/heads/main")).toBe(tip); + }); +}); diff --git a/test/redact-span-binding.test.ts b/test/redact-span-binding.test.ts new file mode 100644 index 000000000..23eeff51e --- /dev/null +++ b/test/redact-span-binding.test.ts @@ -0,0 +1,72 @@ +import { describe, expect, test } from "bun:test"; +import { redactFindingSpans, scan } from "../lib/redact-engine"; +import { redact, sanitizeForJson } from "../lib/cso/process"; + +const secret = ["8Fk2pQ9vXz4wL7mN", "3rT6yB1cD5eG0hJq"].join(""); +const marker = ""; + +describe("redaction binds each finding to its original span", () => { + test("never masks a later unflagged assignment instead of the flagged value", () => { + const input = `DB_PASSWORD=${secret}\nOTHER_API_KEY="your-api-key-here"`; + expect(scan(input).findings.map((f) => f.id)).toEqual(["env.kv"]); + expect(redactFindingSpans(input)).toBe(`DB_PASSWORD=${marker}\nOTHER_API_KEY="your-api-key-here"`); + }); + + test("literal example values are suppressed before redaction", () => { + const input = `DB_PASSWORD=synthetic-example-secret\nOTHER_API_KEY="your-api-key-here"`; + expect(scan(input).findings).toEqual([]); + expect(redactFindingSpans(input)).toBe(input); + }); + + for (const [name, prefix, suffix] of [ + ["first line", "", ""], + ["later line", "unrelated line\n", "\nend"], + ["indented", "header\n ", ""], + ["CRLF", "header\r\n", "\r\nend"], + ]) { + test(`${name}: masks the captured value and keeps surrounding text`, () => { + const input = `${prefix}DB_PASSWORD=${secret}${suffix}`; + expect(scan(input).findings.map((f) => f.id)).toEqual(["env.kv"]); + expect(redactFindingSpans(input)).toBe(`${prefix}DB_PASSWORD=${marker}${suffix}`); + }); + } + + test("repeated identical values map independently, preserving unflagged neighbors", () => { + const input = `DB_PASSWORD=${secret}\nOTHER_API_KEY="your-api-key-here"\nAPI_KEY=${secret}`; + expect(scan(input).findings.map((f) => f.id)).toEqual(["env.kv", "env.kv"]); + expect(redactFindingSpans(input)).toBe(`DB_PASSWORD=${marker}\nOTHER_API_KEY="your-api-key-here"\nAPI_KEY=${marker}`); + }); + + test("many findings retain exact order without per-finding raw rescans", () => { + const input = Array.from({ length: 300 }, (_, i) => `API_KEY=${secret}${i.toString(36)}`).join("\n"); + const output = redactFindingSpans(input); + expect(scan(input).findings).toHaveLength(300); + expect(output).toBe(Array.from({ length: 300 }, () => `API_KEY=${marker}`).join("\n")); + }); + + test("normalization maps fullwidth Unicode, entity text, and zero-width bytes back to original span", () => { + const encoded = `${secret.slice(0, 9)}&${secret.slice(9)}`; + const entity = `${secret.slice(0, 9)}&${secret.slice(9)}`; + const invisible = `${secret.slice(0, 9)}\u200b${secret.slice(9)}`; + for (const value of [encoded, entity, invisible]) { + const input = `DB_PASSWORD=${value}\nOTHER_API_KEY="your-api-key-here"`; + expect(scan(input).findings.map((f) => f.id)).toEqual(["env.kv"]); + expect(redactFindingSpans(input)).toBe(`DB_PASSWORD=${marker}\nOTHER_API_KEY="your-api-key-here"`); + } + }); + + test("overlapping JWT and Bearer findings coalesce; marker-only and unlocated oversize finding still withhold", () => { + const part = "Ab3dE6fGh8Ij9Kl0Mn1O"; + const jwt = `eyJ${part}.eyJ${part}.${part}`; + expect(scan(`Authorization: Bearer ${jwt}`).findings.map((f) => f.id)).toEqual(["auth.bearer", "jwt"]); + expect(redactFindingSpans(`Authorization: Bearer ${jwt}`)).toMatch(/^Authorization: Bearer $/); + expect(redactFindingSpans("-----BEGIN " + "PRIVATE KEY-----\nbody")).toBeNull(); + expect(redactFindingSpans(`DB_PASSWORD=${secret}`, { maxBytes: 10 })).toBeNull(); + }); + + test("CSO process and JSON output use the exact span, without dropping safe context", () => { + const input = `DB_PASSWORD=${secret}\nOTHER_API_KEY="your-api-key-here"`; + expect(redact(input)).toBe(`DB_PASSWORD=${marker}\nOTHER_API_KEY="your-api-key-here"`); + expect(sanitizeForJson({ output: input })).toEqual({ output: `DB_PASSWORD=${marker}\nOTHER_API_KEY="your-api-key-here"` }); + }); +}); diff --git a/test/redact-unicode-offsets.test.ts b/test/redact-unicode-offsets.test.ts new file mode 100644 index 000000000..a13be13ac --- /dev/null +++ b/test/redact-unicode-offsets.test.ts @@ -0,0 +1,67 @@ +import { describe, expect, test } from "bun:test"; +import * as path from "node:path"; +import { applyRedactions, normalizeWithMap, redactFindingSpans, scan } from "../lib/redact-engine"; +import { redact, sanitizeForJson } from "../lib/cso/process"; + +const secret = ["8Fk2pQ9vXz4wL7mN", "3rT6yB1cD5eG0hJq"].join(""); + +describe("normalized redaction offsets use UTF-16 units", () => { + test("every BMP input maps every emitted UTF-16 unit and the end sentinel", () => { + const mismatches: number[] = []; + for (let code = 0; code <= 0xffff; code++) { + const { normalized, map } = normalizeWithMap(String.fromCharCode(code)); + if (map.length !== normalized.length + 1 + || map[normalized.length] !== 1 + || map.slice(0, -1).some((offset) => offset !== 0)) { + mismatches.push(code); + } + } + expect(mismatches).toEqual([]); + }); + + for (const count of [0, 1, 64]) { + test(`${count} supplementary NFKC expansions preserve the exact credential span`, () => { + const prefix = "\uFA6C".repeat(count) + "\n"; + const suffix = "\n" + "z".repeat(256); + const input = `${prefix}DB_PASSWORD=${secret}${suffix}`; + const expected = `${prefix}DB_PASSWORD=${suffix}`; + expect(redactFindingSpans(input)).toBe(expected); + expect(scan(input).findings.map(({ id, line, col }) => ({ id, line, col }))) + .toEqual([{ id: "env.kv", line: 2, col: 13 }]); + expect(redact(input)).toBe(expected); + expect(sanitizeForJson({ output: input })).toEqual({ output: expected }); + }); + } + + test("supplementary source text, entities, and zero-width input retain their original bytes", () => { + const prefix = "\u{242EE} & \u200b\n"; + const value = `${secret.slice(0, 9)}\u200b${secret.slice(9)}`; + const input = `${prefix}DB_PASSWORD=${value}\r\nend`; + const { normalized, map } = normalizeWithMap(input); + expect(map).toHaveLength(normalized.length + 1); + expect(map[normalized.length]).toBe(input.length); + expect(redactFindingSpans(input)).toBe(`${prefix}DB_PASSWORD=\r\nend`); + }); + + for (const suffix of ["", " after"]) { + test(`both masking APIs preserve exact email boundaries with suffix ${JSON.stringify(suffix)}`, () => { + const prefix = "\uFA6C contact: "; + const input = prefix + "reviewer@audit.invalid" + suffix; + expect(redactFindingSpans(input)).toBe(prefix + "" + suffix); + const result = applyRedactions(input, ["pii.email"]); + expect(result.body).toBe(prefix + "" + suffix); + expect(result.skipped).toEqual([]); + }); + } + + test("the actual auto-redact CLI masks a detected email at EOF after supplementary expansion", () => { + const prefix = "\uFA6C contact: "; + const input = prefix + "reviewer@audit.invalid"; + const result = Bun.spawnSync([ + process.execPath, path.resolve(import.meta.dir, "../bin/gstack-redact"), "--auto-redact", "pii.email", + ], { stdin: Buffer.from(input), timeout: 30_000 }); + expect(result.exitCode).toBe(0); + expect(result.stdout.toString()).toBe(prefix + ""); + expect(result.stderr.toString()).not.toContain("could not be auto-redacted"); + }); +}); diff --git a/test/telemetry.test.ts b/test/telemetry.test.ts index fa7c6974d..00ee01595 100644 --- a/test/telemetry.test.ts +++ b/test/telemetry.test.ts @@ -321,13 +321,9 @@ describe('gstack-telemetry-log', () => { expect(events[0].error_message.length).toBeLessThanOrEqual(200); }); - test('fails closed: error_message becomes null when the engine cannot relocate a span (#1947)', () => { + test('redacts an anchored error_message value at its original offset (#1947, #2930)', () => { setConfig('telemetry', 'anonymous'); const secret = '8Fk2pQ9vXz4wL7mN3rT6yB1cD5eG0hJq'; - // env.kv-shaped finding (line-anchored, so the assignment leads the - // message): the span (value) starts past the regex match start, - // locateSpan misses it, redactFindingSpans returns null — the bin must - // drop the whole message, never pass it through raw. run( `${BIN}/gstack-telemetry-log --skill qa --duration 10 --outcome error --error-message 'API_KEY=${secret} rejected by daemon' --session-id red-4`, ); @@ -335,7 +331,7 @@ describe('gstack-telemetry-log', () => { const lines = readJsonl(); expect(lines).toHaveLength(1); const event = JSON.parse(lines[0]); - expect(event.error_message).toBeNull(); + expect(event.error_message).toBe('API_KEY= rejected by daemon'); expect(lines[0]).not.toContain(secret); });