chore(audit): weekly cloud audit — completion-gate false-greens, security-KB race, projection & retention gaps

[tzone85]

Weekly automated security/quality audit of the default branch. Fanned out parallel
sub-agents across concurrency, silent-failure, event-sourcing-wiring, and security
categories, adversarially verified every candidate, and fixed the ones that survived.
Seven real code defects fixed (each with a regression test), plus one CALLED
standard-library vulnerability surfaced by CI govulncheck.

Findings fixed

1. Completion gate reports green on code whose tests do not compile

internal/engine/verification_loop.go · parseGoTestJSON / checkTests

parseGoTestJSON skipped every go test -json event with an empty Test field,
and checkTests discarded the command's exit code. A test-package compile
failure is reported by go test -json only as package-scoped events
("build-fail", and a package "fail" carrying FailedBuild) that have no
Test field — so they were dropped and TestsFailing came back 0. Because
go build ./... skips _test.go files, BuildPasses was true. Net effect:
ShouldRunFixCycle returned false and the completion gate emitted
REQ_COMPLETED for a composed mainline whose test suite does not even compile —
exactly the cross-story drift (story A changes a signature, story B's test still
calls the old one) the gate exists to catch. Verified empirically with a real
go test -json run.

Fix: parseGoTestJSON now counts package-level build-fail / FailedBuild
events as failures (without double-counting ordinary assertion failures), and
checkTests treats a non-zero go test exit with zero parsed failures as a
structural failure. Regression tests: TestParseGoTestJSON_CountsTestCompileFailure,
TestParseGoTestJSON_NormalResultsUnaffected,
TestRunVerificationLoop_NonCompilingTests_NotGreen.

2. Completion-gate verification runs unbounded subprocesses and drops its context

internal/engine/verification_loop.go · RunVerificationLoop / checkTests / ensureDependencies

RunVerificationLoop(ctx, …) received a context but never threaded it into its
subprocess calls, and checkTests / ensureDependencies ran with no timeout.
The gate runs these over an LLM-generated composed mainline; a single hung test
(infinite loop, blocked on stdin/network) or a stalled go mod download /
npm install would block before REQ_COMPLETED / REQ_BLOCKED could ever be
emitted, and the dropped ctx meant even Ctrl-C on nxd resume couldn't cancel
it. The sibling paths (validateBuild, the Gemma runtime) already bound their
subprocesses; this one was left open.

Fix: thread ctx through and run every verification subprocess via
exec.CommandContext, with a boundedContext helper that applies a backstop
timeout when the caller's ctx has no deadline (deps 5m, tests 15m) while
preserving caller cancellation.

3. Security knowledge base: non-atomic write + missing lock can permanently disable the gate

internal/security/knowledge.go · KnowledgeBase.Save; internal/engine/security_gate.go · SecurityGate.upskill

postExecutionPipeline runs one goroutine per completed story, all sharing one
SecurityGate, which had no mutex. When auto_learn is on (default), two stories
in the same wave that both learn a rule race two truncating os.WriteFile calls
against the same kbPath: best case a lost update, worst case interleaved bytes
producing invalid JSON. And LoadKnowledgeBase only falls back to the baseline
when the file is missing — a corrupt file errors forever, and ReviewStory
errors are swallowed ("continuing to merge"), so one torn write silently and
permanently disables the security gate for all future stories. (The sibling
routing.Save already solved this with temp-file + rename; the KB hadn't.)

Fix: Save now writes to a temp file and os.Renames it into place (atomic on
POSIX). upskill takes an upskillMu and, under the lock, reloads the persisted
KB before merging so concurrent stories build on each other's learned rules
instead of clobbering them; a cheap pre-check skips the lock entirely when there
is nothing learnable. Regression test:
TestKnowledgeBase_Save_AtomicUnderConcurrency (runs under -race).

4. Event-sourcing wiring gap: AGENT_TERMINATED silently no-ops in the projection

internal/state/sqlite.go · SQLiteStore.Project

The dashboard "kill agent" handler (web/handlers.go) and the controller's
auto-cancel (engine/controller.go) both emit AGENT_TERMINATED and call
Project(evt) expecting a mutation — but the switch had no case for it, so it
fell through to default: return nil. The agent row stayed status='idle' with
its session_name / current_story_id still populated. Consequences: nxd agents and the dashboard show a killed agent as a live idle one, and crash
recovery's buildSessionStoryMap keeps mapping the dead session to its story,
so recovery can try to reconcile an already-killed tmux session. The dashboard's
agentStatusStyle already renders a "terminated" status — this is the
projection wiring it was waiting on.

Fix: add a case EventAgentTerminated → projectAgentTerminated, which sets
status='terminated' and clears current_story_id (dropping the dead session
from the recovery map). Regression test: TestProject_AgentTerminated_TransitionsRow.

5. Investigator shell-command filter weaker than the runtime's — allows redirection / expansion

internal/engine/investigator.go · Investigator.isCommandAllowed

The investigator's run_command filter blocked only chaining
(; && || | $( ` \n) before routing the command through shellexec →
sh -c. It left redirection (>, <), background (&), bare variable
expansion ($HOME, ${IFS}), and \r \t \x00 \ open — so an allowlisted
prefix (cat, grep, ls) combined with redirection could write or exfiltrate
outside the repo without any chaining operator, e.g.
cat internal/secret > /home/user/.bashrc. The native runtime's run_command
guard already rejects the full metacharacter set; the two enforcement points had
drifted.

Fix: reject the same canonical set the runtime uses
(;&|$ ` <>\n\r\t\x00\\). Regression test:
TestInvestigator_CommandAllowlist_RejectsRedirectionAndExpansion (and the
existing chaining tests still pass).

6. Orphan-DB retention window is silently defeated on the Docker path — can delete in-use databases

internal/devdb/recovery.go · ReleaseOrphans

ReleaseOrphans protects freshly-created orphan DBs from reaping with
if db.CreatedAt.After(cutoff). But the Docker provider (the production path)
never populates CreatedAt — Postgres exposes no reliable per-database creation
time, and dbFromName leaves it zero. A zero time.Time is always
Before(cutoff), so every Docker orphan is deleted regardless of the
configured RetainHours / minAge: the retention window is dead. NXD shares one
host Postgres across requirements, so nxd resume for requirement A classifies
requirement B's live DBs (different story IDs) as orphans — and the window meant
to spare them never fires, so A's resume can drop B's in-use databases. The unit
test and the null provider both set CreatedAt, which is why the first round
missed it; only the real Docker List path leaves it zero. (lifecycle.go
already guards CreatedAt.IsZero(); ReleaseOrphans didn't.)

Fix: treat an unknown (zero) CreatedAt as "too young to reap; keep for human
review" — if db.CreatedAt.IsZero() || db.CreatedAt.After(cutoff). Fail safe,
since the timestamp can't be backfilled from Postgres. Regression test:
TestReleaseOrphans_ZeroCreatedAtKeptNotDeleted.

7. coverage_above gate reads only the first package's coverage — false green on multi-package targets

internal/criteria/evaluator.go · evalCoverageAbove / parseCoverage

evalCoverageAbove defaults its target to ./... when no target is set, then
ran go test -cover ./... and passed the output to parseCoverage, whose
FindStringSubmatch returns only the first coverage: X% line. go test -cover ./... prints one such line per package, so the gate read only the
alphabetically-first package's number: a repo where package aaa is 100% and
package zzz is 5% passes a coverage_above: 80 criterion. A gating quality
check silently reporting green is exactly the false-negative this audit targets.

Fix: write a merged -coverprofile and compute the statement-weighted aggregate
via go tool cover -func (total: line); fall back to the old per-run parse
only when the profile can't be reduced. Regression test:
TestCoverageAbove_MultiPackage_AggregatesNotFirstPackage (aaa=100%, zzz≈0% →
aggregate far below 80% → correctly fails).

8. CALLED stdlib vulnerability GO-2026-5856 (crypto/tls) — surfaced by CI govulncheck

go.mod · toolchain; .github/workflows/ci.yml · go-version

CI govulncheck flagged GO-2026-5856 (Encrypted Client Hello privacy leak in
crypto/tls), present in the go1.26.4 standard library and CALLED via
web.Server.Start, docker.Client.Ping, metrics.Recorder.Record, and
update.Checker. Fixed in go1.26.5.

Fix: bump the go.mod toolchain directive and the CI setup-go pins from
1.26.4 to 1.26.5 — the same remediation pattern the repo already uses for
the earlier 1.26.x stdlib CVEs. Build/vet/tests re-verified against go1.26.5.
(govulncheck could not be run in the audit sandbox — vuln.go.dev is network
-blocked there — so this one came from CI, which can reach the vuln DB.)

Flagged but NOT actioned (needs a maintainer decision)

A first-round agent flagged vxd/vortex references (and the private repo URL
…/vortex-dispatch) in docs/history/vxd.yaml, docs/history/*.md, and
CHANGELOG.md. I deliberately did not scrub these: the repo's own codified
leak policy, scripts/check-leaks.sh, guards specific client names, a corporate
email domain, and the private tzone85/project-x repo, but does not list
vxd/vortex and explicitly allowlists docs/history/ (the CI Leak check
passes on this branch). Rewriting archived history against a checked-in,
considered policy is a maintainer call, not an automated one — flagging it here
instead. If the sibling name should be guarded, add vxd|vortex to
check-leaks.sh FORBIDDEN and scrub accordingly.

Verification

• go build ./... — clean (go1.26.5)
• go vet ./... — clean
• go test ./... -count=1 — pass
• go test ./internal/engine/ ./internal/security/ ./internal/state/ -race — pass (covers the new concurrency fix)
• govulncheck ./... — the CALLED vuln GO-2026-5856 (finding docs: clarify API key usage in configuration guide #8) was found by CI govulncheck and fixed by the toolchain bump; the audit sandbox itself cannot reach vuln.go.dev (403 on CONNECT), so CI is the authoritative govulncheck run here.
• golangci-lint — could not run in the audit sandbox (v2.5.0 refuses a go 1.26 module because its own build targets go 1.25, and no 1.26-ready release resolves); the CI Lint job is authoritative. go vet is clean locally.

🤖 Generated with Claude Code