From 5d111e68c0355760ee932317da3ca9b0e284c74b Mon Sep 17 00:00:00 2001 From: Thando Mini Date: Thu, 2 Jul 2026 02:29:37 +0200 Subject: [PATCH 1/4] =?UTF-8?q?chore(security):=20dogfood=20scan=20hardeni?= =?UTF-8?q?ng=20=E2=80=94=20pin=20GH=20Actions=20to=20SHAs=20+=20annotate?= =?UTF-8?q?=20accepted=20findings?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Ran vxd security scan on vxd itself (346 findings) and closed the high-severity set: - Pin all 14 GitHub Actions references in ci.yml to full commit SHAs (mutable-tag supply-chain class, CWE-1357) with version comments. - Annotate the 29 accepted-by-design findings with #nosec / nosemgrep and a one-line rationale each: sampler seed conversion + math/rand (statistical sampling, crypto seed), server shutdown contexts (fresh ctx after parent cancel is the graceful-shutdown idiom), G703 path-taint sites (paths derive from $HOME/worktrees inside the operator trust boundary), and the 15 dangerous-exec-command sites that ARE the orchestrator's core function (each with its upstream validation named). vxd security scan . now reports 0 high+ findings on its own tree, so the scan is usable as a self-gate (--min high) once CI billing is restored. --- .github/workflows/ci.yml | 28 +++++++++++++------------- internal/agent/render.go | 2 +- internal/autoresearch/driver.go | 2 +- internal/autoresearch/sampler.go | 7 ++++--- internal/cli/autoresearch.go | 1 + internal/cli/req.go | 2 +- internal/codegraph/runner.go | 2 +- internal/dashstart/spawn.go | 2 +- internal/engine/integration_build.go | 2 +- internal/engine/monitor_git_hygiene.go | 2 ++ internal/engine/qa.go | 2 +- internal/engine/sanitize_output.go | 3 ++- internal/engine/verification_loop.go | 2 +- internal/engine/wave_context.go | 1 + internal/improve/implementer.go | 4 ++-- internal/improve/proposal.go | 2 +- internal/improve/repolearn.go | 2 +- internal/llm/claude_cli.go | 2 +- internal/llm/codex_cli.go | 2 +- internal/memory/server.go | 2 +- internal/preflight/checks.go | 4 +++- internal/shellexec/shellexec.go | 4 ++-- internal/web/auth.go | 1 + internal/web/server.go | 2 +- 24 files changed, 46 insertions(+), 37 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index d514845..693358d 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -16,10 +16,10 @@ jobs: runs-on: ubuntu-latest steps: - name: Checkout - uses: actions/checkout@v4 + uses: actions/checkout@34e114876b0b11c390a56381ad16ebd13914f8d5 # v4.3.1 - name: Setup Go - uses: actions/setup-go@v5 + uses: actions/setup-go@40f1582b2485089dde7abd97c1529aa768e1baff # v5.6.0 with: go-version: '1.26' @@ -52,7 +52,7 @@ jobs: CLAUDECODE: "" - name: Upload coverage - uses: actions/upload-artifact@v4 + uses: actions/upload-artifact@ea165f8d65b6e75b540449e92b4886f43607fa02 # v4.6.2 with: name: coverage path: coverage.out @@ -62,15 +62,15 @@ jobs: runs-on: ubuntu-latest steps: - name: Checkout - uses: actions/checkout@v4 + uses: actions/checkout@34e114876b0b11c390a56381ad16ebd13914f8d5 # v4.3.1 - name: Setup Go - uses: actions/setup-go@v5 + uses: actions/setup-go@40f1582b2485089dde7abd97c1529aa768e1baff # v5.6.0 with: go-version: '1.26' - name: Run golangci-lint - uses: golangci/golangci-lint-action@v8 + uses: golangci/golangci-lint-action@4afd733a84b1f43292c63897423277bb7f4313a9 # v8.0.0 with: version: v2.12.2 @@ -84,10 +84,10 @@ jobs: runs-on: ubuntu-latest steps: - name: Checkout - uses: actions/checkout@v4 + uses: actions/checkout@34e114876b0b11c390a56381ad16ebd13914f8d5 # v4.3.1 - name: Run govulncheck - uses: golang/govulncheck-action@v1 + uses: golang/govulncheck-action@b625fbe08f3bccbe446d94fbf87fcc875a4f50ee # v1.0.4 with: go-version-input: '1.26' go-package: ./... @@ -97,10 +97,10 @@ jobs: runs-on: ubuntu-latest steps: - name: Checkout - uses: actions/checkout@v4 + uses: actions/checkout@34e114876b0b11c390a56381ad16ebd13914f8d5 # v4.3.1 - name: Setup Go - uses: actions/setup-go@v5 + uses: actions/setup-go@40f1582b2485089dde7abd97c1529aa768e1baff # v5.6.0 with: go-version: '1.26' @@ -110,7 +110,7 @@ jobs: go build -o vxd-improve ./cmd/vxd-improve - name: Upload binary - uses: actions/upload-artifact@v4 + uses: actions/upload-artifact@ea165f8d65b6e75b540449e92b4886f43607fa02 # v4.6.2 with: name: vxd-linux path: vxd @@ -122,17 +122,17 @@ jobs: if: startsWith(github.ref, 'refs/tags/v') steps: - name: Checkout - uses: actions/checkout@v4 + uses: actions/checkout@34e114876b0b11c390a56381ad16ebd13914f8d5 # v4.3.1 with: fetch-depth: 0 - name: Setup Go - uses: actions/setup-go@v5 + uses: actions/setup-go@40f1582b2485089dde7abd97c1529aa768e1baff # v5.6.0 with: go-version: '1.26' - name: Run GoReleaser - uses: goreleaser/goreleaser-action@v6 + uses: goreleaser/goreleaser-action@e435ccd777264be153ace6237001ef4d979d3a7a # v6.4.0 with: version: '~> v2' args: release --clean diff --git a/internal/agent/render.go b/internal/agent/render.go index af405f0..f3fb3c2 100644 --- a/internal/agent/render.go +++ b/internal/agent/render.go @@ -2,7 +2,7 @@ package agent import ( "bytes" - "text/template" + "text/template" // nosemgrep: go.lang.security.audit.xss.import-text-template.import-text-template -- renders agent prompts (plain text), never HTML ) // TemplateContext holds all data available to prompt templates. diff --git a/internal/autoresearch/driver.go b/internal/autoresearch/driver.go index 2a33708..6cbba7f 100644 --- a/internal/autoresearch/driver.go +++ b/internal/autoresearch/driver.go @@ -161,7 +161,7 @@ func autoCommit(worktree, branch string) { } func runIn(dir string, args ...string) (string, error) { - cmd := exec.Command(args[0], args[1:]...) + cmd := exec.Command(args[0], args[1:]...) // nosemgrep: go.lang.security.audit.dangerous-exec-command.dangerous-exec-command -- args are fixed git/tooling argv built by this package, not user input cmd.Dir = dir out, err := cmd.CombinedOutput() return string(out), err diff --git a/internal/autoresearch/sampler.go b/internal/autoresearch/sampler.go index 05cd046..cbfc8a8 100644 --- a/internal/autoresearch/sampler.go +++ b/internal/autoresearch/sampler.go @@ -5,7 +5,7 @@ import ( "encoding/binary" "log" "math" - "math/rand" + "math/rand" // nosemgrep: go.lang.security.audit.crypto.math_random.math-random-used -- statistical sampling only; seeded from crypto/rand "sync" ) @@ -52,7 +52,7 @@ func NewBayesSampler(classes []ExperimentClass, priorAlpha, priorBeta float64) * classes: append([]ExperimentClass(nil), classes...), priorAlpha: priorAlpha, priorBeta: priorBeta, - rng: rand.New(rand.NewSource(secureSeed())), + rng: rand.New(rand.NewSource(secureSeed())), // #nosec G404 -- Thompson sampling needs statistical, not cryptographic, randomness; the seed itself comes from crypto/rand (secureSeed) } } @@ -73,6 +73,7 @@ func secureSeed() int64 { log.Printf("[autoresearch] CRITICAL: crypto/rand.Read failed: %v — falling back to deterministic seed; Thompson sampling will be predictable for this process", err) return 1 } + // #nosec G115 -- b is 8 random bytes; the uint64→int64 wraparound is harmless because only the bit pattern matters for a seed. return int64(binary.LittleEndian.Uint64(b[:])) } @@ -80,7 +81,7 @@ func secureSeed() int64 { func (s *BayesSampler) SetSeed(seed int64) { s.mu.Lock() defer s.mu.Unlock() - s.rng = rand.New(rand.NewSource(seed)) + s.rng = rand.New(rand.NewSource(seed)) // #nosec G404 -- deterministic test seeding by design } // Classes returns the class set this sampler covers. diff --git a/internal/cli/autoresearch.go b/internal/cli/autoresearch.go index c3360cc..c66b039 100644 --- a/internal/cli/autoresearch.go +++ b/internal/cli/autoresearch.go @@ -337,6 +337,7 @@ func lookPath(name string) (string, error) { continue } full := filepath.Join(dir, name) + // #nosec G703 -- PATH lookup mirrors exec.LookPath; dirs come from the operator's own environment if fi, err := os.Stat(full); err == nil && !fi.IsDir() { return full, nil } diff --git a/internal/cli/req.go b/internal/cli/req.go index 6a05124..fa76b3d 100644 --- a/internal/cli/req.go +++ b/internal/cli/req.go @@ -59,7 +59,7 @@ The requirement text can be provided as: func forkReqDaemon(self, reqID, logPath string, extraArgs []string) *exec.Cmd { // Build the child argv: vxd resume [extraArgs...] argv := append([]string{"resume", reqID}, extraArgs...) - cmd := exec.Command(self, argv...) + cmd := exec.Command(self, argv...) // nosemgrep: go.lang.security.audit.dangerous-exec-command.dangerous-exec-command -- re-execs vxd's own binary (os.Executable) to self-daemonize // Detach from the current process group (platform-specific: Setsid on // Unix, CREATE_NEW_PROCESS_GROUP on Windows). diff --git a/internal/codegraph/runner.go b/internal/codegraph/runner.go index 34492c0..05d511c 100644 --- a/internal/codegraph/runner.go +++ b/internal/codegraph/runner.go @@ -96,7 +96,7 @@ func GraphDBPath(repoPath string) string { // run executes code-review-graph with the given args in the repo directory. func (r *Runner) run(ctx context.Context, repoPath string, args ...string) (string, error) { - cmd := exec.CommandContext(ctx, r.BinPath, args...) + cmd := exec.CommandContext(ctx, r.BinPath, args...) // nosemgrep: go.lang.security.audit.dangerous-exec-command.dangerous-exec-command -- BinPath is the code-review-graph binary resolved from PATH; args are fixed subcommands cmd.Dir = repoPath var stdout, stderr bytes.Buffer diff --git a/internal/dashstart/spawn.go b/internal/dashstart/spawn.go index 084f709..b6c15e7 100644 --- a/internal/dashstart/spawn.go +++ b/internal/dashstart/spawn.go @@ -50,7 +50,7 @@ func BuildCmd(args SpawnArgs) (*exec.Cmd, error) { cmdArgs = append(cmdArgs, "--no-open") } - cmd := exec.Command(args.Self, cmdArgs...) + cmd := exec.Command(args.Self, cmdArgs...) // nosemgrep: go.lang.security.audit.dangerous-exec-command.dangerous-exec-command -- spawns vxd's own binary (os.Executable) as the dashboard daemon cmd.Env = FilteredEnv() return cmd, nil } diff --git a/internal/engine/integration_build.go b/internal/engine/integration_build.go index b85255c..3b9a36d 100644 --- a/internal/engine/integration_build.go +++ b/internal/engine/integration_build.go @@ -102,7 +102,7 @@ func runIntegrationBuild(repoDir string) error { ctx, cancel := context.WithTimeout(context.Background(), integrationBuildTimeout) defer cancel() - cmd := exec.CommandContext(ctx, argv[0], argv[1:]...) + cmd := exec.CommandContext(ctx, argv[0], argv[1:]...) // nosemgrep: go.lang.security.audit.dangerous-exec-command.dangerous-exec-command -- build command comes from operator vxd.yaml, gated by ValidateConfigShellCommand cmd.Dir = repoDir out, err := cmd.CombinedOutput() if err != nil { diff --git a/internal/engine/monitor_git_hygiene.go b/internal/engine/monitor_git_hygiene.go index 7a72fc1..1472491 100644 --- a/internal/engine/monitor_git_hygiene.go +++ b/internal/engine/monitor_git_hygiene.go @@ -213,6 +213,7 @@ func stripBinariesFromBranch(worktreePath, storyID string) { giPath := filepath.Join(worktreePath, ".gitignore") giData, _ := os.ReadFile(giPath) appendix := "\n# auto-detected binaries (stripped by vxd)\n" + strings.Join(binaries, "\n") + "\n" + // #nosec G703 -- giPath is /.gitignore; the worktree root comes from vxd's own executor if err := os.WriteFile(giPath, append(giData, []byte(appendix)...), 0o644); err != nil { log.Printf("[hygiene] failed to append binary patterns to %s: %v (stripped binaries may reappear)", giPath, err) } @@ -368,6 +369,7 @@ func ensureGitignorePatterns(worktreePath string) { } appendix := "\n# VXD agent artifacts (auto-added)\n" + strings.Join(toAdd, "\n") + "\n" + // #nosec G703 -- giPath is /.gitignore in the operator's own checkout if err := os.WriteFile(giPath, append(existing, []byte(appendix)...), 0o644); err != nil { log.Printf("[gitignore] failed to update %s: %v", giPath, err) } diff --git a/internal/engine/qa.go b/internal/engine/qa.go index 8d47c55..26d37b2 100644 --- a/internal/engine/qa.go +++ b/internal/engine/qa.go @@ -61,7 +61,7 @@ type ExecRunner struct{} // Run executes the given command in the specified working directory and // returns the combined stdout/stderr output. func (e *ExecRunner) Run(ctx context.Context, workDir, name string, args ...string) (string, error) { - cmd := exec.CommandContext(ctx, name, args...) + cmd := exec.CommandContext(ctx, name, args...) // nosemgrep: go.lang.security.audit.dangerous-exec-command.dangerous-exec-command -- QA commands come from operator vxd.yaml, gated by ValidateConfigShellCommand cmd.Dir = workDir out, err := cmd.CombinedOutput() return string(out), err diff --git a/internal/engine/sanitize_output.go b/internal/engine/sanitize_output.go index a96d6eb..de62503 100644 --- a/internal/engine/sanitize_output.go +++ b/internal/engine/sanitize_output.go @@ -137,6 +137,7 @@ func scrubFile(path string) bool { // Write back without the preamble cleaned := strings.Join(remaining, "\n") + // #nosec G703 -- path was produced by walking the local worktree; sanitizer rewrites the same file in place if err := os.WriteFile(path, []byte(cleaned), 0644); err != nil { log.Printf("[sanitize] failed to write cleaned file %s: %v", path, err) return false @@ -192,7 +193,7 @@ func validateNodeProject(dir string) error { // Try tsc --noEmit first (TypeScript), then build tscPath := filepath.Join(dir, "node_modules", ".bin", "tsc") if fileExists(tscPath) { - cmd := exec.Command(tscPath, "--noEmit") + cmd := exec.Command(tscPath, "--noEmit") // nosemgrep: go.lang.security.audit.dangerous-exec-command.dangerous-exec-command -- fixed argv; tscPath is /node_modules/.bin/tsc cmd.Dir = dir if out, err := cmd.CombinedOutput(); err != nil { return fmt.Errorf("TypeScript check failed:\n%s", truncateOutput(string(out), 500)) diff --git a/internal/engine/verification_loop.go b/internal/engine/verification_loop.go index 835765a..d2ec68f 100644 --- a/internal/engine/verification_loop.go +++ b/internal/engine/verification_loop.go @@ -230,7 +230,7 @@ func scanForHallucinations(repoDir string) []string { if !isSourceExt(ext) { return nil } - data, err := os.ReadFile(path) + data, err := os.ReadFile(path) // #nosec G122 G304 -- best-effort read-only scan of the local repo tree; a TOCTOU race only skips a file if err != nil { return nil } diff --git a/internal/engine/wave_context.go b/internal/engine/wave_context.go index 1652f01..084ac56 100644 --- a/internal/engine/wave_context.go +++ b/internal/engine/wave_context.go @@ -84,6 +84,7 @@ func appendToWaveContext(path, storyID, entry string) { content += entry + // #nosec G703 -- path is /WAVE_CONTEXT.md; the worktree root comes from vxd's own executor, not user input if err := os.WriteFile(path, []byte(content), 0o644); err != nil { log.Printf("[wave-context] failed to write %s: %v", path, err) } diff --git a/internal/improve/implementer.go b/internal/improve/implementer.go index 47e9c35..2bdf508 100644 --- a/internal/improve/implementer.go +++ b/internal/improve/implementer.go @@ -119,7 +119,7 @@ RULES: Work in the current directory.`, finding.Title, finding.SourceURL, finding.ImplementationPlan, finding.TestStrategy) - cmd := exec.CommandContext(ctx, impl.claudePath, "-p", prompt, "--output-format", "json", "--max-turns", "25") + cmd := exec.CommandContext(ctx, impl.claudePath, "-p", prompt, "--output-format", "json", "--max-turns", "25") // nosemgrep: go.lang.security.audit.dangerous-exec-command.dangerous-exec-command -- claudePath resolved from PATH; prompt is a single argv element, injection-scanned by findingHasInjection cmd.Dir = impl.repoPath // Unset ANTHROPIC_API_KEY so Claude uses subscription (free) instead of // exhausted API credits. Unset CLAUDECODE to prevent nested-session errors. @@ -237,7 +237,7 @@ func (impl *Implementer) gitOutput(args ...string) (string, error) { } func (impl *Implementer) run(name string, args ...string) error { - cmd := exec.Command(name, args...) + cmd := exec.Command(name, args...) // nosemgrep: go.lang.security.audit.dangerous-exec-command.dangerous-exec-command -- fixed git argv built by the caller within this package cmd.Dir = impl.repoPath out, err := cmd.CombinedOutput() if err != nil { diff --git a/internal/improve/proposal.go b/internal/improve/proposal.go index 0114ab3..04a9a63 100644 --- a/internal/improve/proposal.go +++ b/internal/improve/proposal.go @@ -100,7 +100,7 @@ func (d *ProposalDrafter) DraftProposal(ctx context.Context, opp Opportunity) (s // Call Claude CLI. Strip ANTHROPIC_API_KEY so Claude uses Max subscription // instead of API credits, and strip CLAUDECODE to prevent nested-session // errors when VXD is invoked inside Claude Code (ENV-2). - cmd := exec.CommandContext(ctx, d.claudePath, "-p", prompt, "--output-format", "text") + cmd := exec.CommandContext(ctx, d.claudePath, "-p", prompt, "--output-format", "text") // nosemgrep: go.lang.security.audit.dangerous-exec-command.dangerous-exec-command -- claudePath resolved from PATH; prompt is a single argv element, never shell-interpolated cmd.Dir = d.workDir cmd.Env = llm.FilterClaudeEnv(os.Environ()) diff --git a/internal/improve/repolearn.go b/internal/improve/repolearn.go index 0307ae3..3979209 100644 --- a/internal/improve/repolearn.go +++ b/internal/improve/repolearn.go @@ -183,7 +183,7 @@ For each relevant pattern, output a JSON array with objects containing: Only include patterns with relevance >= 5. Output ONLY the JSON array.`, repo.Name, focusAreas, diffContent) // Call LLM via Claude CLI - cmd := exec.CommandContext(ctx, rl.claudeCLI, "-p", prompt, "--output-format", "json") + cmd := exec.CommandContext(ctx, rl.claudeCLI, "-p", prompt, "--output-format", "json") // nosemgrep: go.lang.security.audit.dangerous-exec-command.dangerous-exec-command -- claudeCLI resolved from PATH; prompt is a single argv element, never shell-interpolated out, err := cmd.Output() if err != nil { return nil, fmt.Errorf("claude CLI: %w", err) diff --git a/internal/llm/claude_cli.go b/internal/llm/claude_cli.go index 370d023..d64b9b1 100644 --- a/internal/llm/claude_cli.go +++ b/internal/llm/claude_cli.go @@ -58,7 +58,7 @@ func (c *ClaudeCLIClient) Complete(ctx context.Context, req CompletionRequest) ( // turns of file reads before producing a plan. args = append(args, "--max-turns", "50") - cmd := exec.CommandContext(ctx, c.cliPath, args...) + cmd := exec.CommandContext(ctx, c.cliPath, args...) // nosemgrep: go.lang.security.audit.dangerous-exec-command.dangerous-exec-command -- cliPath is the claude binary from PATH; model name gated by ValidateModelName, prompt via stdin cmd.Stdin = strings.NewReader(prompt) // Strip ANTHROPIC_API_KEY so Claude Code uses the user's Max subscription // instead of a potentially expired/empty API key. Also strip CLAUDECODE to diff --git a/internal/llm/codex_cli.go b/internal/llm/codex_cli.go index 26d7e4c..d22a65e 100644 --- a/internal/llm/codex_cli.go +++ b/internal/llm/codex_cli.go @@ -69,7 +69,7 @@ func (c *CodexCLIClient) Complete(ctx context.Context, req CompletionRequest) (C "-", // read the prompt from stdin } - cmd := exec.CommandContext(ctx, c.cliPath, args...) + cmd := exec.CommandContext(ctx, c.cliPath, args...) // nosemgrep: go.lang.security.audit.dangerous-exec-command.dangerous-exec-command -- cliPath is the codex binary from PATH; model name gated by ValidateModelName, prompt via stdin cmd.Stdin = strings.NewReader(prompt) // Strip OPENAI_API_KEY so Codex uses the subscription, not API credits. cmd.Env = FilterCodexEnv(os.Environ()) diff --git a/internal/memory/server.go b/internal/memory/server.go index 73cdf1f..ddd2866 100644 --- a/internal/memory/server.go +++ b/internal/memory/server.go @@ -107,7 +107,7 @@ func (s *Server) Start(ctx context.Context) error { openBrowser(browserURL) } - go func() { + go func() { // #nosec G118 -- the request-scoped ctx is already cancelled when this runs; graceful shutdown requires a fresh context <-ctx.Done() shutdownCtx, cancel := context.WithTimeout(context.Background(), 3*time.Second) defer cancel() diff --git a/internal/preflight/checks.go b/internal/preflight/checks.go index db3b914..3b10202 100644 --- a/internal/preflight/checks.go +++ b/internal/preflight/checks.go @@ -234,16 +234,18 @@ func CheckProject() Result { func CheckStateDir() Result { home := os.Getenv("HOME") stateDir := filepath.Join(home, ".vxd", "projects") + // #nosec G703 -- stateDir derives from $HOME on the operator's own host; no untrusted input reaches this path if _, err := os.Stat(stateDir); os.IsNotExist(err) { return Result{Name: "state_dir", Severity: SeverityInfo, Passed: true, Message: fmt.Sprintf("State dir: %s (will be created on first run)", stateDir)} } tmp := filepath.Join(stateDir, ".preflight-test") + // #nosec G703 G306 -- writability probe with throwaway content under $HOME if err := os.WriteFile(tmp, []byte("test"), 0644); err != nil { return Result{Name: "state_dir", Severity: SeverityInfo, Passed: false, Message: fmt.Sprintf("State dir not writable: %s", stateDir)} } - _ = os.Remove(tmp) // best-effort cleanup of the probe file + _ = os.Remove(tmp) // #nosec G703 -- best-effort cleanup of the probe file under $HOME return Result{Name: "state_dir", Severity: SeverityInfo, Passed: true, Message: fmt.Sprintf("State dir: %s", stateDir)} } diff --git a/internal/shellexec/shellexec.go b/internal/shellexec/shellexec.go index 4874b63..1792ea3 100644 --- a/internal/shellexec/shellexec.go +++ b/internal/shellexec/shellexec.go @@ -58,11 +58,11 @@ func filenameBase(p string) string { // Command returns an exec.Cmd that runs command via the host shell. func Command(command string) *exec.Cmd { exe, flag := Shell() - return exec.Command(exe, flag, command) + return exec.Command(exe, flag, command) // nosemgrep: go.lang.security.audit.dangerous-exec-command.dangerous-exec-command -- documented trust boundary: shell commands come from operator vxd.yaml (see CLAUDE.md item 52) } // CommandContext is the context-aware variant of Command. func CommandContext(ctx context.Context, command string) *exec.Cmd { exe, flag := Shell() - return exec.CommandContext(ctx, exe, flag, command) + return exec.CommandContext(ctx, exe, flag, command) // nosemgrep: go.lang.security.audit.dangerous-exec-command.dangerous-exec-command -- documented trust boundary: shell commands come from operator vxd.yaml (see CLAUDE.md item 52) } diff --git a/internal/web/auth.go b/internal/web/auth.go index 3117c88..bbc9137 100644 --- a/internal/web/auth.go +++ b/internal/web/auth.go @@ -231,6 +231,7 @@ func (a *authenticator) wrap(next http.Handler) http.Handler { // reverse proxy — and only then assert Secure. Reverse- // proxied deployments still get the full protection. secure := r.TLS != nil || strings.EqualFold(r.Header.Get("X-Forwarded-Proto"), "https") + // nosemgrep: go.lang.security.audit.net.cookie-missing-secure.cookie-missing-secure -- Secure is asserted conditionally above; unconditional Secure breaks plain-HTTP localhost auth http.SetCookie(w, &http.Cookie{ Name: TokenCookieName, Value: a.token, diff --git a/internal/web/server.go b/internal/web/server.go index 1a65bc9..faf121a 100644 --- a/internal/web/server.go +++ b/internal/web/server.go @@ -157,7 +157,7 @@ func (s *Server) Start(ctx context.Context) error { go s.hub.Run(ctx) // Graceful shutdown - go func() { + go func() { // #nosec G118 -- the request-scoped ctx is already cancelled when this runs; graceful shutdown requires a fresh context <-ctx.Done() shutdownCtx, cancel := context.WithTimeout(context.Background(), 3*time.Second) defer cancel() From 5190da02ec86650875f8287788a2cf74b90201d8 Mon Sep 17 00:00:00 2001 From: Thando Mini Date: Thu, 2 Jul 2026 02:32:48 +0200 Subject: [PATCH 2/4] =?UTF-8?q?feat(preflight):=20security=5Fscanners=20ch?= =?UTF-8?q?eck=20=E2=80=94=20surface=20missing=20SAST/secret=20tools?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The per-story security gate degrades gracefully when a scanner binary is absent (skipped, never fatal), which left operators with no signal that scan coverage was reduced. New CheckSecurityScanners (WARNING tier) lists missing binaries from the security.KnownScanners registry with install hints (security.InstallHint) and joins AllChecks — vxd preflight now runs 16 checks. lookPath is injected for testability, matching CheckBinaryPath's pattern. 4 new tests including a dangling-wire guard (AllChecks must include the check). Docs updated (CLAUDE.md + README check counts, security-agent section). --- CLAUDE.md | 4 +- README.md | 2 +- internal/preflight/checks.go | 39 +++++++++- internal/preflight/checks_security_test.go | 84 ++++++++++++++++++++++ internal/preflight/checks_test.go | 6 +- internal/security/scanners.go | 27 +++++++ 6 files changed, 154 insertions(+), 8 deletions(-) create mode 100644 internal/preflight/checks_security_test.go diff --git a/CLAUDE.md b/CLAUDE.md index 008def0..b309a0e 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -172,7 +172,7 @@ dashboard: | `vxd metrics` | Success rates, timing, escalations, SLA breaches per requirement | | `vxd estimate "req"` | Cost estimation with `--quick`, `--json`, `--rate` | | `vxd report ` | Client delivery report (`--html`, `--internal`) | -| `vxd preflight` | Run 15 pre-flight checks before dispatch | +| `vxd preflight` | Run 16 pre-flight checks before dispatch | | `vxd approve ` | Approve a story PR for merge (`--all ` for batch) | | `vxd approve-plan` | Approve story plan before dispatch | | `vxd reject-plan` | Reject a plan with feedback | @@ -451,7 +451,7 @@ A self-upskilling security agent embedded in vxd's core so every build is review - **Self-upskilling:** confirmed high+ findings whose vuln CLASS (CWE → OWASP category → tool rule) isn't already `Covers`ed are added as `learned` rules, persisted, and announced via `SECURITY_RULE_LEARNED`. The grown KB is what the gate applies on the next build — and what `vxd security kb` shows. - **Forward-embedded in core:** the planner's ENGINEERING STANDARDS block now spells out the OWASP Top 10 so every planned story is *designed* secure; the per-story gate enforces the *live* KB at merge; `resume.go` wires both (`TestResume_WiresSecurityGate`). Skipped in dry-run and when `security.disable_gate`. - **Config:** `security.disable_gate` (default false=ON), `security.gate_severity`, `security.auto_learn` (default true), `security.kb_path`. **Events:** `STORY_SECURITY_PASSED/FAILED`, `SECURITY_SCAN_COMPLETED`, `SECURITY_RULE_LEARNED` (all in the projection switch; `TestProject_AllDeclaredEventsHandled` guards exhaustiveness). -- **Tests:** `internal/security/*_test.go` (16: KB roundtrip/immutability/lang-filter/checklist/Covers, scanner applicability, all 5 parsers, report) + `engine/security_gate_test.go` (7: scan aggregation+event, block-on-critical, pass-below-threshold, self-upskill on new class, no-relearn known class, LLM-findings parse). **Host scanner install + NXD port pending.** +- **Tests:** `internal/security/*_test.go` (16: KB roundtrip/immutability/lang-filter/checklist/Covers, scanner applicability, all 5 parsers, report) + `engine/security_gate_test.go` (7: scan aggregation+event, block-on-critical, pass-below-threshold, self-upskill on new class, no-relearn known class, LLM-findings parse). Host scanners installed (gosec, govulncheck, gitleaks, semgrep); the `security_scanners` preflight check (`CheckSecurityScanners`, WARNING tier) reports any that go missing, with install hints from `security.InstallHint`. **NXD port pending.** ### Model ID Compatibility - **Use undated aliases, not dated snapshots.** Current defaults: `claude-opus-4-8` (tech_lead), `claude-sonnet-4-6` (senior/qa/manager), `claude-haiku-4-5` (cheapest). All three are verified working on the Claude CLI subscription tier. diff --git a/README.md b/README.md index 638a540..89ec247 100644 --- a/README.md +++ b/README.md @@ -232,7 +232,7 @@ vhs docs/demo.tape | `vxd dashboard status` | Show whether the always-on dashboard daemon is running (PID, port, URL). | | `vxd dashboard stop` | SIGTERM the always-on dashboard daemon and remove its pidfile (idempotent). | | `vxd watch [req-id]` | Terminal-friendly always-on status: tails events for one requirement (defaults to the newest in the current repo) until terminal status or Ctrl+C. | -| `vxd preflight` | Run pre-flight environment checks (15 checks, 3 severity tiers) | +| `vxd preflight` | Run pre-flight environment checks (16 checks, 3 severity tiers) | | `vxd estimate ` | Estimate cost (`--quick`, `--json`, `--rate`, `--save`) | | `vxd report ` | Generate client delivery report (`--html`, `--internal`, `--output`) | | `vxd metrics [--req ID]` | Show pipeline performance metrics with agent activity stats | diff --git a/internal/preflight/checks.go b/internal/preflight/checks.go index 3b10202..8274f20 100644 --- a/internal/preflight/checks.go +++ b/internal/preflight/checks.go @@ -17,6 +17,7 @@ import ( "github.com/tzone85/vortex-dispatch/internal/devdb/docker" "github.com/tzone85/vortex-dispatch/internal/devdb/ghost" "github.com/tzone85/vortex-dispatch/internal/engine" + "github.com/tzone85/vortex-dispatch/internal/security" ) // --- CRITICAL checks --- @@ -326,6 +327,39 @@ func CheckBinaryPath(executablePath string) Result { )} } +// CheckSecurityScanners warns when security scanner binaries the per-story +// security gate relies on are missing from PATH. The gate degrades gracefully +// (a missing tool is skipped, never fatal), so this check is the only surface +// that tells the operator scan coverage is reduced. lookPath is injected so +// the check can be unit-tested; pass exec.LookPath for the real host. +func CheckSecurityScanners(lookPath func(string) (string, error)) Result { + var installed, missing []string + var hints []string + seen := map[string]bool{} + for _, s := range security.KnownScanners() { + if seen[s.Bin] { + continue + } + seen[s.Bin] = true + if _, err := lookPath(s.Bin); err == nil { + installed = append(installed, s.Bin) + continue + } + missing = append(missing, s.Bin) + if hint := security.InstallHint(s.Bin); hint != "" { + hints = append(hints, hint) + } + } + if len(missing) == 0 { + return Result{Name: "security_scanners", Severity: SeverityWarning, Passed: true, + Message: fmt.Sprintf("Security scanners installed: %s", strings.Join(installed, ", "))} + } + return Result{Name: "security_scanners", Severity: SeverityWarning, Passed: false, + Message: fmt.Sprintf( + "Security scanners missing: %s — the security gate will skip them (reduced coverage). install: %s", + strings.Join(missing, ", "), strings.Join(hints, " && "))} +} + // --- Check sets --- // DispatchChecks returns the 9 checks run before every dispatch operation. @@ -338,12 +372,13 @@ func DispatchChecks() []Check { } } -// AllChecks returns all 14 checks including informational ones shown by +// AllChecks returns all 16 checks including informational ones shown by // `vxd preflight`. func AllChecks() []Check { binaryCheck := func() Result { return CheckBinaryPath("") } + scannerCheck := func() Result { return CheckSecurityScanners(exec.LookPath) } return append(DispatchChecks(), - binaryCheck, + binaryCheck, scannerCheck, CheckConfig, CheckProject, CheckStateDir, CheckBillingConfig, CheckOllama, ) } diff --git a/internal/preflight/checks_security_test.go b/internal/preflight/checks_security_test.go new file mode 100644 index 0000000..f624d66 --- /dev/null +++ b/internal/preflight/checks_security_test.go @@ -0,0 +1,84 @@ +package preflight + +import ( + "fmt" + "strings" + "testing" +) + +// lookPathAll simulates a host where every scanner binary is installed. +func lookPathAll(name string) (string, error) { + return "/usr/local/bin/" + name, nil +} + +// lookPathNone simulates a host with no scanner binaries at all. +func lookPathNone(name string) (string, error) { + return "", fmt.Errorf("%s not found", name) +} + +func TestCheckSecurityScanners_AllInstalled(t *testing.T) { + res := CheckSecurityScanners(lookPathAll) + if !res.Passed { + t.Fatalf("expected pass when all scanners installed, got: %s", res.Message) + } + if res.Severity != SeverityWarning { + t.Fatalf("expected WARNING severity, got %v", res.Severity) + } + if res.Name != "security_scanners" { + t.Fatalf("unexpected check name %q", res.Name) + } + if !strings.Contains(res.Message, "gosec") { + t.Fatalf("pass message should list installed scanners, got: %s", res.Message) + } +} + +func TestCheckSecurityScanners_AllMissing(t *testing.T) { + res := CheckSecurityScanners(lookPathNone) + if res.Passed { + t.Fatalf("expected fail when all scanners missing, got: %s", res.Message) + } + // Every registry binary must be named so the operator knows what to install. + for _, bin := range []string{"gosec", "govulncheck", "gitleaks", "semgrep"} { + if !strings.Contains(res.Message, bin) { + t.Errorf("message missing scanner %q: %s", bin, res.Message) + } + } + // Message must carry an actionable install hint, not just a list of names. + if !strings.Contains(res.Message, "install") { + t.Errorf("message should include install guidance: %s", res.Message) + } +} + +func TestCheckSecurityScanners_PartiallyMissing(t *testing.T) { + lookPath := func(name string) (string, error) { + if name == "gosec" || name == "npm" { + return "", fmt.Errorf("not found") + } + return "/usr/local/bin/" + name, nil + } + res := CheckSecurityScanners(lookPath) + if res.Passed { + t.Fatalf("expected fail when some scanners missing, got: %s", res.Message) + } + if !strings.Contains(res.Message, "gosec") { + t.Errorf("missing scanner gosec not named: %s", res.Message) + } + if strings.Contains(res.Message, "missing: gitleaks") || strings.Contains(res.Message, "missing: semgrep") { + t.Errorf("installed scanners must not be listed as missing: %s", res.Message) + } +} + +func TestAllChecks_IncludesSecurityScanners(t *testing.T) { + // The check is diagnostic (vxd preflight), not a dispatch gate — so it must + // appear in AllChecks. Identify it by running each check and matching Name. + found := false + for _, check := range AllChecks() { + if check().Name == "security_scanners" { + found = true + break + } + } + if !found { + t.Fatal("AllChecks() does not include the security_scanners check — the wire is dangling") + } +} diff --git a/internal/preflight/checks_test.go b/internal/preflight/checks_test.go index 7c0f9b9..241dbf6 100644 --- a/internal/preflight/checks_test.go +++ b/internal/preflight/checks_test.go @@ -194,10 +194,10 @@ func TestDispatchChecks_Returns9(t *testing.T) { } } -func TestAllChecks_Returns15(t *testing.T) { +func TestAllChecks_Returns16(t *testing.T) { checks := preflight.AllChecks() - if len(checks) != 15 { - t.Fatalf("expected 15 total checks, got %d", len(checks)) + if len(checks) != 16 { + t.Fatalf("expected 16 total checks, got %d", len(checks)) } } diff --git a/internal/security/scanners.go b/internal/security/scanners.go index c30f63d..b8586b5 100644 --- a/internal/security/scanners.go +++ b/internal/security/scanners.go @@ -105,6 +105,33 @@ func RunScanners(ctx context.Context, repoDir string) (findings []Finding, ran, return DedupeFindings(findings), ran, skipped } +// KnownScanners returns the full scanner registry regardless of PATH +// availability or repo languages, so other packages — e.g. preflight — can +// report on missing tools without duplicating the list. +func KnownScanners() []Scanner { + return allScanners() +} + +// InstallHint returns the install command for a scanner binary, or "" when no +// hint is known. Hints target macOS/Homebrew and the Go toolchain — the two +// supported operator setups. +func InstallHint(bin string) string { + switch bin { + case "gosec": + return "go install github.com/securego/gosec/v2/cmd/gosec@latest" + case "govulncheck": + return "go install golang.org/x/vuln/cmd/govulncheck@latest" + case "gitleaks": + return "brew install gitleaks" + case "semgrep": + return "brew install semgrep" + case "npm": + return "brew install node" + default: + return "" + } +} + // DetectScanners returns the scanners applicable to repoDir and available on the // host. Detection combines language inspection with exec.LookPath. func DetectScanners(repoDir string) []Scanner { From a7bb198f2b6e549dcfd2cfdafa810492f5a7499b Mon Sep 17 00:00:00 2001 From: Thando Mini Date: Thu, 2 Jul 2026 02:41:22 +0200 Subject: [PATCH 3/4] fix(watch): vxd watch silently dropped every event for real requirements + test coverage restore MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two matcher bugs made `vxd watch` a silent no-op tail in production: 1. Story events: eventMatchesReq compared evt.StoryID[:8] against reqID[:8], but story IDs are namespaced with storyIDPrefix(reqID) — sha256(reqID)[:8] for any reqID longer than 8 chars, which every real ULID reqID is. The prefixes never matched, so no story event ever printed. The matcher now uses the exported engine.StoryIDPrefix (single source of truth). 2. Requirement events: the code commented 'REQ_* events get routed via payload below' but no payload routing existed — REQ_SUBMITTED/PLANNED/COMPLETED/ BLOCKED never printed. Now matched via the payload req_id/id keys (the two keys real emitters use: planner uses 'id', the planning heartbeat 'req_id'). The old TestEventMatchesReq_PrefixMatch pinned the broken raw-prefix behavior and was replaced (test was wrong, not the spec). New tests: hashed-prefix match, short-reqID verbatim match, payload routing for both key spellings, cross-requirement rejection, and an end-to-end tailRequirementEvents run against real file+sqlite stores that pins print-and-exit-on-terminal. Also restores the internal/cli coverage regression from PR #109 (68.0% → 72.9%): the security scan/kb commands, dashboard status/stop daemon commands, and watch were all untested. New: security_test.go (9 tests — pure helpers, kb text/json, scan with empty PATH pinning graceful degradation + skipped- scanner reporting), dashboard_daemon_test.go (7 tests — not-running status, idempotent stop, malformed/stale pidfiles, watch unknown-req error). --- internal/cli/dashboard_daemon_test.go | 98 +++++++++++++++++ internal/cli/security_test.go | 148 ++++++++++++++++++++++++++ internal/cli/watch.go | 29 +++-- internal/cli/watch_test.go | 130 ++++++++++++++++++++-- internal/engine/planner.go | 7 ++ 5 files changed, 397 insertions(+), 15 deletions(-) create mode 100644 internal/cli/dashboard_daemon_test.go create mode 100644 internal/cli/security_test.go diff --git a/internal/cli/dashboard_daemon_test.go b/internal/cli/dashboard_daemon_test.go new file mode 100644 index 0000000..8ee9c2c --- /dev/null +++ b/internal/cli/dashboard_daemon_test.go @@ -0,0 +1,98 @@ +package cli + +import ( + "bytes" + "os" + "path/filepath" + "strings" + "testing" +) + +func TestDashboardStatus_NotRunning(t *testing.T) { + pidfile := filepath.Join(t.TempDir(), "dashboard.pid") // never created + cmd := newDashboardStatusCmd() + var out bytes.Buffer + cmd.SetOut(&out) + cmd.SetArgs([]string{"--pidfile", pidfile}) + if err := cmd.Execute(); err != nil { + t.Fatalf("status must not error when daemon absent: %v", err) + } + if !strings.Contains(out.String(), "not running") { + t.Errorf("expected 'not running', got:\n%s", out.String()) + } + if !strings.Contains(out.String(), pidfile) { + t.Errorf("status should print the pidfile path it checked:\n%s", out.String()) + } +} + +func TestDashboardStop_NoPidfile(t *testing.T) { + pidfile := filepath.Join(t.TempDir(), "dashboard.pid") + cmd := newDashboardStopCmd() + var out bytes.Buffer + cmd.SetOut(&out) + cmd.SetArgs([]string{"--pidfile", pidfile}) + if err := cmd.Execute(); err != nil { + t.Fatalf("stop must be idempotent with no pidfile: %v", err) + } + if !strings.Contains(out.String(), "not running") { + t.Errorf("expected 'not running' note, got:\n%s", out.String()) + } +} + +func TestDashboardStop_MalformedPidfile(t *testing.T) { + pidfile := filepath.Join(t.TempDir(), "dashboard.pid") + if err := os.WriteFile(pidfile, []byte("not-a-pid"), 0o600); err != nil { + t.Fatal(err) + } + cmd := newDashboardStopCmd() + var out bytes.Buffer + cmd.SetOut(&out) + cmd.SetArgs([]string{"--pidfile", pidfile}) + if err := cmd.Execute(); err == nil || !strings.Contains(err.Error(), "malformed pidfile") { + t.Fatalf("expected malformed-pidfile error, got %v", err) + } +} + +func TestDashboardStop_StalePid(t *testing.T) { + pidfile := filepath.Join(t.TempDir(), "dashboard.pid") + // PID far above pid_max on macOS/Linux defaults — signal gets ESRCH. + if err := os.WriteFile(pidfile, []byte("99999999"), 0o600); err != nil { + t.Fatal(err) + } + cmd := newDashboardStopCmd() + var out bytes.Buffer + cmd.SetOut(&out) + cmd.SetArgs([]string{"--pidfile", pidfile}) + if err := cmd.Execute(); err != nil { + t.Fatalf("stop on stale pid must be idempotent: %v", err) + } + if _, err := os.Stat(pidfile); !os.IsNotExist(err) { + t.Error("stale pidfile must be removed") + } +} + +func TestDefaultDashboardPidfile_UnderHome(t *testing.T) { + got := defaultDashboardPidfile() + if !strings.HasSuffix(got, filepath.Join(".vxd", "dashboard.pid")) && + !strings.HasSuffix(got, "vxd-dashboard.pid") { + t.Errorf("unexpected pidfile location %q", got) + } +} + +func TestPidStarted_BestEffort(t *testing.T) { + // On hosts without /proc (macOS) this returns zero time and nil error; on + // Linux it returns a real time. Either way it must never return an error. + ts, err := pidStarted(os.Getpid()) + if err != nil { + t.Fatalf("pidStarted must be best-effort, got error: %v", err) + } + _ = ts +} + +func TestRunWatch_UnknownRequirement(t *testing.T) { + cmd := newWatchCmd() + driveWithVxdYaml(t, cmd, "does-not-exist") + if err := cmd.Execute(); err == nil || !strings.Contains(err.Error(), "get requirement") { + t.Fatalf("expected get-requirement error for unknown req, got %v", err) + } +} diff --git a/internal/cli/security_test.go b/internal/cli/security_test.go new file mode 100644 index 0000000..d4f5f72 --- /dev/null +++ b/internal/cli/security_test.go @@ -0,0 +1,148 @@ +package cli + +import ( + "encoding/json" + "os" + "path/filepath" + "strings" + "testing" + + "github.com/tzone85/vortex-dispatch/internal/config" +) + +func TestResolveScanPath_ExplicitArg(t *testing.T) { + dir := t.TempDir() + got, err := resolveScanPath([]string{dir}) + if err != nil { + t.Fatalf("resolveScanPath: %v", err) + } + if got != dir { + t.Errorf("want %q, got %q", dir, got) + } +} + +func TestResolveScanPath_RelativeBecomesAbsolute(t *testing.T) { + got, err := resolveScanPath([]string{"."}) + if err != nil { + t.Fatalf("resolveScanPath: %v", err) + } + if !filepath.IsAbs(got) { + t.Errorf("expected absolute path, got %q", got) + } +} + +func TestResolveScanPath_DefaultsToCwd(t *testing.T) { + got, err := resolveScanPath(nil) + if err != nil { + t.Fatalf("resolveScanPath: %v", err) + } + cwd, _ := os.Getwd() + if got != cwd { + t.Errorf("want cwd %q, got %q", cwd, got) + } +} + +func TestSecurityKBPath_Configured(t *testing.T) { + cfg := config.DefaultConfig() + cfg.Security.KBPath = "/explicit/kb.json" + if got := securityKBPath(cfg); got != "/explicit/kb.json" { + t.Errorf("configured kb_path must win, got %q", got) + } +} + +func TestSecurityKBPath_DefaultUnderStateDir(t *testing.T) { + cfg := config.DefaultConfig() + cfg.Security.KBPath = "" + cfg.Workspace.StateDir = "/tmp/vxd-state" + want := filepath.Join("/tmp/vxd-state", "security", "knowledge.json") + if got := securityKBPath(cfg); got != want { + t.Errorf("want %q, got %q", want, got) + } +} + +func TestSecurityKBCmd_TextOutput(t *testing.T) { + cmd := newSecurityKBCmd() + out := driveWithVxdYaml(t, cmd) + if err := cmd.Execute(); err != nil { + t.Fatalf("kb command: %v", err) + } + got := out.String() + if !strings.Contains(got, "Security knowledge base v") { + t.Errorf("missing header:\n%s", got) + } + // The baseline seeds the OWASP Top 10 — at least one A0x rule must render. + if !strings.Contains(got, "A01") { + t.Errorf("baseline OWASP rules missing:\n%s", got) + } +} + +func TestSecurityKBCmd_JSONOutput(t *testing.T) { + cmd := newSecurityKBCmd() + out := driveWithVxdYaml(t, cmd, "--json") + if err := cmd.Execute(); err != nil { + t.Fatalf("kb --json: %v", err) + } + var kb struct { + Version int `json:"version"` + Rules []struct { + ID string `json:"id"` + } `json:"rules"` + } + if err := json.Unmarshal(out.Bytes(), &kb); err != nil { + t.Fatalf("output is not valid JSON: %v\n%s", err, out.String()) + } + if len(kb.Rules) == 0 { + t.Error("baseline knowledge base must not be empty") + } +} + +// TestSecurityScanCmd_NoScannersInstalled drives the full scan command with an +// empty PATH so every scanner is skipped (the graceful-degradation path): the +// scan must succeed, report zero findings, and list what it skipped rather +// than pretending it covered anything. +func TestSecurityScanCmd_NoScannersInstalled(t *testing.T) { + t.Setenv("PATH", t.TempDir()) // empty dir: no scanner binaries resolvable + + repo := t.TempDir() + if err := os.WriteFile(filepath.Join(repo, "main.go"), []byte("package main\n"), 0o600); err != nil { + t.Fatal(err) + } + + cmd := newSecurityScanCmd() + out := driveWithVxdYaml(t, cmd, "--json", repo) + if err := cmd.Execute(); err != nil { + t.Fatalf("scan with no scanners must pass (0 findings < --min high): %v", err) + } + + var report struct { + Findings []any `json:"findings"` + Skipped []string `json:"skipped"` + } + if err := json.Unmarshal(out.Bytes(), &report); err != nil { + t.Fatalf("output is not valid JSON: %v\n%s", err, out.String()) + } + if len(report.Findings) != 0 { + t.Errorf("no scanners ran — findings must be empty, got %d", len(report.Findings)) + } + if len(report.Skipped) == 0 { + t.Error("skipped scanners must be reported, not silently dropped") + } +} + +func TestSecurityScanCmd_MarkdownOutput(t *testing.T) { + t.Setenv("PATH", t.TempDir()) + + repo := t.TempDir() + if err := os.WriteFile(filepath.Join(repo, "main.go"), []byte("package main\n"), 0o600); err != nil { + t.Fatal(err) + } + + cmd := newSecurityScanCmd() + out := driveWithVxdYaml(t, cmd, repo) + if err := cmd.Execute(); err != nil { + t.Fatalf("scan: %v", err) + } + if out.Len() == 0 { + t.Error("markdown report must not be empty") + } +} diff --git a/internal/cli/watch.go b/internal/cli/watch.go index 3b6702a..9fa283e 100644 --- a/internal/cli/watch.go +++ b/internal/cli/watch.go @@ -6,10 +6,12 @@ import ( "os" "os/signal" "sort" + "strings" "syscall" "time" "github.com/spf13/cobra" + "github.com/tzone85/vortex-dispatch/internal/engine" "github.com/tzone85/vortex-dispatch/internal/state" ) @@ -156,16 +158,29 @@ func tailRequirementEvents(ctx context.Context, out interface { } // eventMatchesReq returns true iff evt belongs to the watched requirement. -// Story events carry ReqID via payload; cheaper to scan storyID against the -// projection store than to JSON-decode every payload, so we accept both -// direct ReqID matches and StoryID-belonging-to-this-req matches. +// Story events are matched by their ID namespace: story IDs start with +// engine.StoryIDPrefix(reqID) — sha256(reqID)[:8] for real (>8-char) reqIDs, +// the reqID verbatim for short test fixtures. Comparing raw reqID prefixes +// here would silently drop every story event in production. Requirement-level +// events (REQ_*) carry no StoryID; they are matched by the req_id payload field. func eventMatchesReq(evt state.Event, reqID string) bool { - if evt.StoryID != "" && len(evt.StoryID) >= 8 && evt.StoryID[:8] == reqID[:min(8, len(reqID))] { + prefix := engine.StoryIDPrefix(reqID) + if evt.StoryID != "" && (evt.StoryID == prefix || strings.HasPrefix(evt.StoryID, prefix+"-")) { return true } - // Many events carry ReqID in the payload — but rather than decode here, - // the StoryID-prefix check above already covers the majority. REQ_* - // events with no StoryID get routed via payload below. + if len(evt.Payload) > 0 { + // Requirement-level payload keys are not uniform across emitters: + // REQ_SUBMITTED/REQ_COMPLETED/REQ_BLOCKED carry "id", the planning + // heartbeat and story-adjacent events carry "req_id". Accept both; + // exact equality with the full reqID cannot collide with story IDs + // (those are always <8-char-prefix>-). + payload := state.DecodePayload(evt.Payload) + for _, key := range []string{"req_id", "id"} { + if id, ok := payload[key].(string); ok && id == reqID { + return true + } + } + } return false } diff --git a/internal/cli/watch_test.go b/internal/cli/watch_test.go index bf089b5..492db06 100644 --- a/internal/cli/watch_test.go +++ b/internal/cli/watch_test.go @@ -1,10 +1,14 @@ package cli import ( + "context" + "encoding/json" + "path/filepath" "strings" "testing" "time" + "github.com/tzone85/vortex-dispatch/internal/engine" "github.com/tzone85/vortex-dispatch/internal/state" ) @@ -42,16 +46,46 @@ func TestFormatWatchLine_OmitsEmptyFields(t *testing.T) { } } -func TestEventMatchesReq_PrefixMatch(t *testing.T) { - reqID := "01HABCDEF1234567" - evt := state.Event{StoryID: "01HABCDE-S2"} - if !eventMatchesReq(evt, reqID) { - t.Errorf("expected event with matching 8-char prefix to match") +// ulidLikeReqID is 26 chars — the length `vxd req` really generates. Story IDs +// for reqIDs >8 chars are namespaced with sha256(reqID)[:8] (StoryIDPrefix), +// NOT reqID[:8], so a matcher comparing raw reqID prefixes silently drops +// every story event in production. +const ulidLikeReqID = "01JYZX8Q2M3N4P5R6S7T8V9W0X" + +func TestEventMatchesReq_HashedStoryPrefix(t *testing.T) { + storyID := engine.StoryIDPrefix(ulidLikeReqID) + "-s-001" + evt := state.Event{Type: "STORY_STARTED", StoryID: storyID} + if !eventMatchesReq(evt, ulidLikeReqID) { + t.Fatalf("story %s must match req %s (hashed prefix)", storyID, ulidLikeReqID) + } +} + +func TestEventMatchesReq_ShortReqIDVerbatimPrefix(t *testing.T) { + evt := state.Event{Type: "STORY_STARTED", StoryID: "r-001-s-002"} + if !eventMatchesReq(evt, "r-001") { + t.Fatal("short reqIDs are used verbatim as story prefixes and must match") } +} + +func TestEventMatchesReq_ReqEventViaPayload(t *testing.T) { + payload, _ := json.Marshal(map[string]any{"req_id": ulidLikeReqID}) + evt := state.Event{Type: "REQ_PLANNED", Payload: payload} + if !eventMatchesReq(evt, ulidLikeReqID) { + t.Fatal("REQ_* events carry req_id in the payload and must match") + } +} - other := state.Event{StoryID: "ZZZZZZZZ-S2"} - if eventMatchesReq(other, reqID) { - t.Errorf("unrelated story should not match") +func TestEventMatchesReq_OtherRequirement(t *testing.T) { + otherStory := engine.StoryIDPrefix("01JOTHERREQIDENTIFIER00000") + "-s-001" + payload, _ := json.Marshal(map[string]any{"req_id": "someone-else"}) + for _, evt := range []state.Event{ + {Type: "STORY_STARTED", StoryID: otherStory}, + {Type: "REQ_PLANNED", Payload: payload}, + {Type: "AGENT_STUCK"}, + } { + if eventMatchesReq(evt, ulidLikeReqID) { + t.Errorf("event %+v must NOT match req %s", evt, ulidLikeReqID) + } } } @@ -65,3 +99,83 @@ func TestTerminalRequirementStatuses_CoversExpected(t *testing.T) { t.Errorf("'in_progress' must NOT be terminal") } } + +// newTestStores builds real (file + in-memory-sqlite) stores in a temp dir. +func newTestStores(t *testing.T) stores { + t.Helper() + es, err := state.NewFileStore(filepath.Join(t.TempDir(), "events.jsonl")) + if err != nil { + t.Fatalf("event store: %v", err) + } + ps, err := state.NewSQLiteStore(":memory:") + if err != nil { + t.Fatalf("projection store: %v", err) + } + t.Cleanup(func() { _ = es.Close(); _ = ps.Close() }) + return stores{Events: es, Proj: ps} +} + +func appendAndProject(t *testing.T, s stores, evt state.Event) { + t.Helper() + if err := s.Events.Append(evt); err != nil { + t.Fatalf("append: %v", err) + } + if err := s.Proj.Project(evt); err != nil { + t.Fatalf("project: %v", err) + } +} + +func TestTailRequirementEvents_PrintsMatchesAndStopsOnTerminal(t *testing.T) { + s := newTestStores(t) + reqID := ulidLikeReqID + storyID := engine.StoryIDPrefix(reqID) + "-s-001" + + // Payload keys mirror the real emitters: REQ_SUBMITTED (planner) and + // REQ_COMPLETED (emitRequirementOutcome) both use "id", not "req_id". + submitPayload, _ := json.Marshal(map[string]any{"id": reqID, "title": "Build the thing", "description": "x"}) + appendAndProject(t, s, state.Event{ + ID: "e1", Type: state.EventReqSubmitted, Timestamp: time.Now().Add(-3 * time.Second), + AgentID: "system", Payload: submitPayload, + }) + appendAndProject(t, s, state.Event{ + ID: "e2", Type: "STORY_STARTED", Timestamp: time.Now().Add(-2 * time.Second), + AgentID: "agent-1", StoryID: storyID, + }) + donePayload, _ := json.Marshal(map[string]any{"id": reqID}) + appendAndProject(t, s, state.Event{ + ID: "e3", Type: state.EventReqCompleted, Timestamp: time.Now().Add(-1 * time.Second), + AgentID: "system", Payload: donePayload, + }) + + ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second) + defer cancel() + var out strings.Builder + if err := tailRequirementEvents(ctx, &out, s, reqID); err != nil { + t.Fatalf("tail: %v", err) + } + got := out.String() + if !strings.Contains(got, "STORY_STARTED") { + t.Errorf("output missing story event:\n%s", got) + } + if !strings.Contains(got, string(state.EventReqCompleted)) { + t.Errorf("output missing REQ_COMPLETED event:\n%s", got) + } + if !strings.Contains(got, "terminal status") { + t.Errorf("output should announce terminal exit:\n%s", got) + } +} + +func TestResolveWatchReqID_ExplicitArgWins(t *testing.T) { + got, err := resolveWatchReqID(newWatchCmd(), stores{}, []string{"explicit-id"}) + if err != nil || got != "explicit-id" { + t.Fatalf("want explicit-id, got %q err=%v", got, err) + } +} + +func TestResolveWatchReqID_NoRequirements(t *testing.T) { + s := newTestStores(t) + cmd := newWatchCmd() + if _, err := resolveWatchReqID(cmd, s, nil); err == nil || !strings.Contains(err.Error(), "no requirement to watch") { + t.Fatalf("expected 'no requirement to watch' error, got %v", err) + } +} diff --git a/internal/engine/planner.go b/internal/engine/planner.go index 4e4a4e1..c9d5f51 100644 --- a/internal/engine/planner.go +++ b/internal/engine/planner.go @@ -488,6 +488,13 @@ func storyIDPrefix(reqID string) string { return hex.EncodeToString(sum[:])[:8] } +// StoryIDPrefix exposes the story-ID namespace derivation to other packages +// (e.g. `vxd watch` matching events to a requirement). There is exactly one +// way to map reqID → story prefix; callers must never truncate reqID themselves. +func StoryIDPrefix(reqID string) string { + return storyIDPrefix(reqID) +} + // scribeStorySuffix is the stable, un-prefixed id of the README-scribe story. const scribeStorySuffix = "scribe-readme" From b1761f6942f59d8f4779beb69a71a47d7be705ba Mon Sep 17 00:00:00 2001 From: Thando Mini Date: Thu, 2 Jul 2026 02:52:13 +0200 Subject: [PATCH 4/4] =?UTF-8?q?fix(review):=20apply=20go-reviewer=20findin?= =?UTF-8?q?gs=20=E2=80=94=20complete=20G124=20suppression,=20kill=20dead?= =?UTF-8?q?=20branch=20+=20inert=20assertions?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - auth.go: the cookie site had the nosemgrep half of the annotation but gosec G124 still fired; add the #nosec with the same rationale. - watch.go: drop the unreachable evt.StoryID == prefix branch — the planner always emits - IDs, so HasPrefix covers every real case. - checks_security_test.go: the installed-scanner negative assertions matched a substring ('missing: gitleaks') that could never occur in the real message format; assert on the bare scanner names so a regression can actually fire. --- internal/cli/watch.go | 2 +- internal/preflight/checks_security_test.go | 5 ++++- internal/web/auth.go | 1 + 3 files changed, 6 insertions(+), 2 deletions(-) diff --git a/internal/cli/watch.go b/internal/cli/watch.go index 9fa283e..358c0e6 100644 --- a/internal/cli/watch.go +++ b/internal/cli/watch.go @@ -165,7 +165,7 @@ func tailRequirementEvents(ctx context.Context, out interface { // events (REQ_*) carry no StoryID; they are matched by the req_id payload field. func eventMatchesReq(evt state.Event, reqID string) bool { prefix := engine.StoryIDPrefix(reqID) - if evt.StoryID != "" && (evt.StoryID == prefix || strings.HasPrefix(evt.StoryID, prefix+"-")) { + if strings.HasPrefix(evt.StoryID, prefix+"-") { return true } if len(evt.Payload) > 0 { diff --git a/internal/preflight/checks_security_test.go b/internal/preflight/checks_security_test.go index f624d66..cff08cb 100644 --- a/internal/preflight/checks_security_test.go +++ b/internal/preflight/checks_security_test.go @@ -63,7 +63,10 @@ func TestCheckSecurityScanners_PartiallyMissing(t *testing.T) { if !strings.Contains(res.Message, "gosec") { t.Errorf("missing scanner gosec not named: %s", res.Message) } - if strings.Contains(res.Message, "missing: gitleaks") || strings.Contains(res.Message, "missing: semgrep") { + // The message names only what is missing: "Security scanners missing: + // — ... install: ...". Installed scanners must not appear in it + // (their names never occur in the install hints either). + if strings.Contains(res.Message, "gitleaks") || strings.Contains(res.Message, "semgrep") { t.Errorf("installed scanners must not be listed as missing: %s", res.Message) } } diff --git a/internal/web/auth.go b/internal/web/auth.go index bbc9137..c98d17e 100644 --- a/internal/web/auth.go +++ b/internal/web/auth.go @@ -232,6 +232,7 @@ func (a *authenticator) wrap(next http.Handler) http.Handler { // proxied deployments still get the full protection. secure := r.TLS != nil || strings.EqualFold(r.Header.Get("X-Forwarded-Proto"), "https") // nosemgrep: go.lang.security.audit.net.cookie-missing-secure.cookie-missing-secure -- Secure is asserted conditionally above; unconditional Secure breaks plain-HTTP localhost auth + // #nosec G124 -- same rationale: Secure comes from the `secure` variable computed above (true under TLS or X-Forwarded-Proto:https) http.SetCookie(w, &http.Cookie{ Name: TokenCookieName, Value: a.token,