chore(audit): weekly cloud audit — fix 4 fail-open / data-loss defects

[tzone85]

Weekly automated security/quality audit (cloud). Fan-out sub-agents by category (concurrency, silent-failures, event-sourcing wiring, security, correctness), each finding adversarially verified before fixing. Three rounds run; the third was dry (two consecutive dry categories). Every fix ships a Go regression test.

Findings fixed

1. internal/security/scanners.go · parseGovulncheck → runGovulncheck/parseGovulncheckJSON

Real bug — fail-open dependency scan. The four JSON scanners (gosec, gitleaks, semgrep, npm-audit) get failure detection for free: a crash emits non-JSON, json.Unmarshal fails, and RunScanners routes them to failed. govulncheck was the exception — its text mode emits no Vulnerability # line on a network/build/timeout error, so a failed run was indistinguishable from a clean one and landed in ran (clean). This defeated even security.require_scanners: true, which only inspects skipped/failed. It is the exact false-open the surrounding code documents itself as preventing.
Fix: switch to govulncheck -json. In JSON mode govulncheck exits non-zero only on a tool error (findings do not set a non-zero code) and always emits a leading config handshake, so a non-zero exit, a garbled/truncated stream, or a missing handshake are all surfaced as errors → routed to failed. The parser also now reports only symbol-level (called) vulnerabilities, matching the prior intent. (This failure mode is live in the audit environment itself — vuln.go.dev is network-blocked, which the old code would have reported as scanned-clean.)

2. internal/web/auth.go · RequireToken

Real bug — fail-open auth. RequireToken(token, next) set AllowUnauthenticated: token == "", so RequireToken("") silently served a fully unauthenticated dashboard — directly contradicting its own doc comment, NewAuthMiddleware's documented panic contract, and CLAUDE.md's stated guarantee that RequireToken("") PANICS. Currently uncalled, but a loaded footgun for any future caller / port whose token is empty by mistake.
Fix: pass only Token: token so an empty token panics (fail-closed), matching the contract. The fail-open path remains reachable only via an explicit AllowUnauthenticated: true.

3. internal/engine/conflict_resolver.go · handleBinaryConflict

Real bug — silent data loss. Small (kept) binary conflicts resolved with git checkout --ours. But RebaseWithResolution runs inside a git rebase, where the sides are inverted — --ours is the base/upstream, --theirs is the replayed story commit (pinned by git.TestConflictSides_OursIsBaseTheirsIsStory, and matching the text/JSON deterministic paths which all keep --theirs). So when the base also touched a small binary asset, the story's change was silently reverted to the base version and the requirement merged with the story's work lost. Verified empirically with a real rebase (--ours→base, --theirs→story).
Fix: use git checkout --theirs so the story's binary change wins, as documented. Event message + CLAUDE.md corrected.

4. internal/engine/verification_loop.go · checkTests / parseGoTestJSON

Real bug — completion gate fail-open. A Go test binary that fails to compile (e.g. a merged story's _test.go references a symbol another story renamed — exactly the cross-story drift this gate exists to catch) emits only package-level go test -json events with an empty Test field (build-fail, fail). All were skipped, so a non-compiling suite parsed as (0,0,0) = clean; go build ./... doesn't compile test files, so checkBuild also passed; the discarded exit code hid the rest → ShouldRunFixCycle returned false → REQ_COMPLETED on a tree whose tests don't even compile. Verified empirically.
Fix: count package-level build-fail events as failures, and treat a non-zero test-runner exit with zero attributable per-test failures (compile error, panic, vet failure, or a jest/vitest runner that failed to launch) as red. Fails closed.

Verification

• go build ./... — clean
• go vet ./... — clean
• go test ./... -count=1 — all packages pass except four pre-existing, environment-only failures unrelated to this diff, each confirmed failing on clean main via git stash:
  • internal/cli TestApproveStory_* / TestApproveAll_* / TestRunApprove_* — gh CLI not installed in the sandbox.
  • internal/engine TestWriteLockFile_InvalidPath / TestWriteMetadata_InvalidDir — run as root, which can write to the "invalid" paths the tests expect to be rejected.
  • internal/figma TestResolveToken_UnreadableFileIsDistinguished — root bypasses the 0600 unreadable-file check.
  • internal/repolearn TestCountFilesInDir_NonexistentDir — /nonexistent/path actually exists in this container image.
  • (The internal/engine conflict-resolver rebase tests also fail on a bare checkout because git init defaults to master; they pass with git config init.defaultBranch main, as CI sets.)
• make doc-coverage (TestDocCoverage_*) — pass. No new CLI commands, config fields, or event types were added.
• New/updated regression tests, all passing: TestParseGovulncheckJSON, TestParseGovulncheckJSON_FailedRunIsNotClean, TestParseGovulncheckJSON_ImportOnlyFindingsNotCalled, TestRunScanners_GovulncheckFailureNotReportedClean, TestRequireToken_EmptyTokenPanics, TestBinaryConflict_SmallBinaryKeepsStoryVersion, TestParseGoTestJSON_BuildFailIsCounted, TestCheckTests_BrokenTestFileIsNotClean.

Tooling limits (fallback per audit policy): make lint (golangci-lint) could not run — the latest golangci-lint release still embeds Go 1.25 analysis packages and refuses this module's go 1.26.4 directive. make vuln (govulncheck) could not complete — the environment's network policy blocks vuln.go.dev (403 CONNECT). Fell back to build + vet + test + doc-coverage as specified. Notably, finding #1 hardens VXD's own govulncheck integration against exactly the blocked-DB failure this environment exhibits.

🤖 Generated with Claude Code

https://claude.ai/code/session_01LFb5kxYoibXzSz3YGnmJrU

---

Generated by Claude Code