chore(audit): weekly cloud audit — two silent false-clean gates (completion + govulncheck)

CHANGELOG.md

@@ -37,7 +37,7 @@ _(no entries yet — open a PR to add a line under the relevant subsection.)_
 - Auto-commit before rebase to prevent unstaged changes failure
 - Graceful failure handling — artifact filter, re-planner guardrails, sub-story validation
 - Command injection, path traversal, and input validation hardening
-- Port 3 VXD fixes — split suffix, gitDiff master, duplicate validation
+- Port 3 upstream fixes — split suffix, gitDiff master, duplicate validation
 
 ### Improved
 - Engine test coverage: 66% → 72%

internal/agent/frontend_test.go

@@ -6,7 +6,7 @@ import (
 )
 
 // The frontend design brief is injected into the goal prompt only for
-// UI-facing stories (ctx.IsFrontend). It is the vxd factory's design skill:
+// UI-facing stories (ctx.IsFrontend). It is the factory's design skill:
 // token-first planning, one signature element, the named anti-pattern looks,
 // and a non-negotiable accessibility floor.
 func TestGoalPrompt_FrontendBriefInjectedWhenFlagSet(t *testing.T) {

internal/engine/verification_loop.go

@@ -197,13 +197,27 @@ func parseGoTestJSON(output string) (passing, failing, total int) {
 			Action string `json:"Action"`
 			Test   string `json:"Test"`
 		}
-		if err := json.Unmarshal([]byte(line), &evt); err != nil || evt.Test == "" {
+		if err := json.Unmarshal([]byte(line), &evt); err != nil {
 			continue
 		}
 		switch evt.Action {
 		case "pass":
-			passing++
+			if evt.Test != "" {
+				passing++
+			}
 		case "fail":
+			if evt.Test != "" {
+				failing++
+			}
+		case "build-fail":
+			// A package whose test binary fails to COMPILE emits a build-fail
+			// event with no Test (and no Package) field. `go build ./...` does
+			// not compile _test.go files, so checkBuild stays green and this is
+			// the only signal that the composed mainline's tests are red — e.g.
+			// when one story removes a symbol another story's test references.
+			// Without counting it, parseGoTestJSON returns 0/0/0, ShouldRunFixCycle
+			// sees no failures, and the completion gate emits REQ_COMPLETED on a
+			// mainline whose tests do not even build.
 			failing++
 		}
 	}

internal/engine/verification_loop_test.go

@@ -0,0 +1,65 @@
+package engine
+
+import "testing"
+
+// TestParseGoTestJSON_BuildFailCountsAsFailing pins the regression where a test
+// suite that does not COMPILE was reported as 0 failing. `go build ./...` does
+// not compile _test.go files, so a cross-story break in a test's dependency
+// keeps checkBuild green while `go test -json` emits a build-fail event with no
+// Test field. If that event is not counted, the completion gate emits
+// REQ_COMPLETED on a mainline whose tests do not build.
+func TestParseGoTestJSON_BuildFailCountsAsFailing(t *testing.T) {
+	// Verbatim `go test -count=1 -json ./...` output for a package whose test
+	// file references an undefined symbol (captured from the real toolchain).
+	out := `{"ImportPath":"demo [demo.test]","Action":"build-output","Output":"# demo [demo.test]\n"}
+{"ImportPath":"demo [demo.test]","Action":"build-output","Output":"./foo_test.go:3:32: undefined: Bar\n"}
+{"ImportPath":"demo [demo.test]","Action":"build-fail"}
+{"Time":"2026-07-06T05:10:31.594769294Z","Action":"start","Package":"demo"}
+{"Time":"2026-07-06T05:10:31.594812561Z","Action":"output","Package":"demo","Output":"FAIL\tdemo [build failed]\n"}
+{"Time":"2026-07-06T05:10:31.594817791Z","Action":"fail","Package":"demo","Elapsed":0,"FailedBuild":"demo [demo.test]"}
+`
+	passing, failing, total := parseGoTestJSON(out)
+	if failing < 1 {
+		t.Fatalf("build-fail must count as a test failure: got passing=%d failing=%d total=%d", passing, failing, total)
+	}
+	if passing != 0 {
+		t.Errorf("no tests ran, expected 0 passing, got %d", passing)
+	}
+
+	// The gate must not permit completion on this result.
+	if !ShouldRunFixCycle(VerificationResult{BuildPasses: true, TestsPassing: passing, TestsFailing: failing, TestsTotal: total}) {
+		t.Error("ShouldRunFixCycle must return true when the test suite fails to build")
+	}
+}
+
+// TestParseGoTestJSON_CountsTestLevelOnly guards the existing behavior: only
+// per-test pass/fail events are counted, so a package-level pass/fail (Test=="")
+// that always accompanies test events does not double-count.
+func TestParseGoTestJSON_CountsTestLevelOnly(t *testing.T) {
+	out := `{"Action":"run","Test":"TestA"}
+{"Action":"pass","Test":"TestA"}
+{"Action":"run","Test":"TestB"}
+{"Action":"fail","Test":"TestB"}
+{"Action":"fail","Package":"demo"}
+{"Action":"output","Package":"demo","Output":"FAIL\n"}
+`
+	passing, failing, total := parseGoTestJSON(out)
+	if passing != 1 || failing != 1 || total != 2 {
+		t.Fatalf("expected 1 passing / 1 failing / 2 total, got %d/%d/%d", passing, failing, total)
+	}
+}
+
+// TestParseGoTestJSON_AllGreen confirms a fully passing suite still reads clean.
+func TestParseGoTestJSON_AllGreen(t *testing.T) {
+	out := `{"Action":"pass","Test":"TestA"}
+{"Action":"pass","Test":"TestB"}
+{"Action":"pass","Package":"demo"}
+`
+	passing, failing, _ := parseGoTestJSON(out)
+	if passing != 2 || failing != 0 {
+		t.Fatalf("expected 2 passing / 0 failing, got %d/%d", passing, failing)
+	}
+	if ShouldRunFixCycle(VerificationResult{BuildPasses: true, TestsPassing: passing, TestsFailing: failing}) {
+		t.Error("ShouldRunFixCycle must return false for a green suite")
+	}
+}

internal/security/scanners.go

@@ -5,6 +5,8 @@ import (
 	"bytes"
 	"context"
 	"encoding/json"
+	"errors"
+	"fmt"
 	"log"
 	"os/exec"
 	"path/filepath"
@@ -365,12 +367,28 @@ func (s Scanner) Run(ctx context.Context, repoDir string) ([]Finding, error) {
 		return nil, nil
 	}
 	cmd.Dir = repoDir
-	out, _ := cmd.CombinedOutput() // exit code intentionally ignored; parse output
+	// Exit code is intentionally ignored for the JSON scanners: they exit
+	// non-zero when they find issues, and a genuine run failure corrupts the
+	// JSON so the parser returns an error (→ recorded as failed). govulncheck
+	// is the exception — its text output is empty both when it finds nothing
+	// AND when it never ran (offline, no go.mod, load error), so a clean parse
+	// cannot tell the two apart; we must inspect its exit code (see below).
+	out, runErr := cmd.CombinedOutput()
 
 	switch s.Kind {
 	case ScannerGosec:
 		return parseGosec(out, repoDir)
 	case ScannerGovulncheck:
+		// Exit 0 (no vulnerabilities) and exit 3 (vulnerabilities found) are
+		// both successful analyses. Any other outcome — exit 1 (load/network/
+		// config error, e.g. NXD's offline-first host cannot reach vuln.go.dev),
+		// exit 2 (usage), or a non-exit failure such as a timeout — means the
+		// scan never inspected the code. Returning an error here routes it to
+		// RunScanners' `failed` list instead of masquerading as a clean run,
+		// which is the whole point of that list.
+		if !govulncheckCompleted(runErr) {
+			return nil, fmt.Errorf("govulncheck did not complete (dependency-CVE coverage lost): %s", scannerFailureDetail(out, runErr))
+		}
 		return parseGovulncheck(out)
 	case ScannerGitleaks:
 		return parseGitleaks(out, repoDir)
@@ -382,3 +400,36 @@ func (s Scanner) Run(ctx context.Context, repoDir string) ([]Finding, error) {
 		return nil, nil
 	}
 }
+
+// govulncheckCompleted reports whether a govulncheck invocation actually ran a
+// full analysis. govulncheck exits 0 when it finds no vulnerabilities and 3 when
+// it does (both successful runs, per golang.org/x/vuln internal/scan/errors.go).
+// Every other outcome — exit 1 (package load / network / config error), exit 2
+// (usage), or a non-*exec.ExitError failure such as a context timeout — means it
+// never inspected the code and its empty text output must not be read as clean.
+func govulncheckCompleted(runErr error) bool {
+	if runErr == nil {
+		return true // exit 0: analysis completed, no vulnerabilities
+	}
+	var ee *exec.ExitError
+	if errors.As(runErr, &ee) && ee.ExitCode() == 3 {
+		return true // exit 3: analysis completed, vulnerabilities found
+	}
+	return false
+}
+
+// scannerFailureDetail summarizes why a scanner run failed for logging: the
+// first non-empty line of tool output (usually the diagnostic), falling back to
+// the exec error string.
+func scannerFailureDetail(out []byte, runErr error) string {
+	sc := bufio.NewScanner(bytes.NewReader(out))
+	for sc.Scan() {
+		if line := strings.TrimSpace(sc.Text()); line != "" {
+			return line
+		}
+	}
+	if runErr != nil {
+		return runErr.Error()
+	}
+	return "no output"
+}

internal/security/scanners_test.go

@@ -1,12 +1,59 @@
 package security
 
 import (
+	"fmt"
 	"os"
+	"os/exec"
 	"path/filepath"
 	"sort"
 	"testing"
 )
 
+// exitErr runs `sh -c "exit N"` to obtain a real *exec.ExitError carrying the
+// given code — the same error type os/exec returns for a scanner subprocess.
+func exitErr(t *testing.T, code int) error {
+	t.Helper()
+	err := exec.Command("sh", "-c", fmt.Sprintf("exit %d", code)).Run()
+	if err == nil {
+		t.Fatalf("expected non-nil error for exit %d", code)
+	}
+	return err
+}
+
+// TestGovulncheckCompleted pins the exit-code contract that keeps a govulncheck
+// run which never inspected the code (offline host, no go.mod, load error) from
+// being recorded as a clean scan. Exit 0 (no vulns) and 3 (vulns found) are
+// successful analyses; exit 1/2 and non-exit failures are coverage loss.
+func TestGovulncheckCompleted(t *testing.T) {
+	if !govulncheckCompleted(nil) {
+		t.Error("nil error (exit 0, no vulnerabilities) must count as a completed scan")
+	}
+	if !govulncheckCompleted(exitErr(t, 3)) {
+		t.Error("exit 3 (vulnerabilities found) must count as a completed scan")
+	}
+	if govulncheckCompleted(exitErr(t, 1)) {
+		t.Error("exit 1 (load/network/config error) must NOT count as completed — coverage was lost")
+	}
+	if govulncheckCompleted(exitErr(t, 2)) {
+		t.Error("exit 2 (usage error) must NOT count as completed")
+	}
+	if govulncheckCompleted(fmt.Errorf("context deadline exceeded")) {
+		t.Error("a non-exit failure (e.g. timeout) must NOT count as completed")
+	}
+}
+
+// TestScannerFailureDetail confirms the diagnostic prefers the first non-empty
+// output line (the tool's own error) over the bare exec status.
+func TestScannerFailureDetail(t *testing.T) {
+	out := []byte("\n\ngovulncheck: fetching vulnerabilities: Forbidden\nmore\n")
+	if got := scannerFailureDetail(out, exitErr(t, 1)); got != "govulncheck: fetching vulnerabilities: Forbidden" {
+		t.Errorf("unexpected detail: %q", got)
+	}
+	if got := scannerFailureDetail(nil, fmt.Errorf("boom")); got != "boom" {
+		t.Errorf("empty output should fall back to the error string, got %q", got)
+	}
+}
+
 func TestDetectLanguages(t *testing.T) {
 	dir := t.TempDir()
 	write := func(name, body string) {