Harden untrusted-input handling, refactor check engine, and enforce golangci-lint

.claude/knowledge/architecture-boundaries.md

@@ -21,3 +21,6 @@ Key architectural decisions, service boundaries, data flow, integration points,
 - **MCP `config_path` confinement**: caller-supplied `config_path` is confined (`confinePath` in `mcp_tools.go`) to the server config dir + cwd + client roots; the server's own `--config` default is never confined. This closed a remote arbitrary-file-read finding. Streaming: `core.ScanOptions.OnSectionComplete` (tagged `json:"-"` — marshaling a non-nil func errors) fires in `runner/support.FinalizeSection`.
 - **`codeguard serve --mcp --http`** serves Streamable HTTP for remote hosts (Devin): `POST {mcp-path}` returns `application/json` for sync methods and `text/event-stream` for `tools/call`; optional static-bearer auth (`--auth-token`/`$CODEGUARD_MCP_AUTH_TOKEN`, constant-time compared), `GET /healthz`, body/concurrency caps, SIGINT/SIGTERM graceful shutdown. No OAuth. MCP resources (`codeguard://rules`, `codeguard://config`, `codeguard://rules/{rule_id}`) and prompts (`review-diff`, `triage-findings`, `explain-rule`) wrap the same SDK accessors the tools use. Devin onboarding pack: `examples/hooks/devin/`.
 - The taint engine emits a single rule ID per language, chosen by sink: `goTaintRuleID`/`pyTaintRuleID` return `security.ssrf.{go,python}` when the sink is an outbound-HTTP callee (`goHTTPSinkArgIndex`/`pyHTTPSinkArgIndex`), else the generic `security.taint.{go,python}`. To add an SSRF sink, add it to the arg-index map; the rule routing and OWASP A10 mapping follow automatically.
+- **Section dispatch is data-driven, not a hardcoded ladder.** `runner/checks/Build` iterates `sectionRegistry` (`runner/checks/registry.go`) — a `[]sectionDef{id, name, enabled(sc), run(ctx, sc, checkEnv)}`. To add a check section, append a `sectionDef`; do NOT reintroduce an `if cfg.X {...}` ladder. Every section runs through `safeRun` (in `checks.go`), which recovers panics into a `StatusWarn` SectionResult with a `checks.section.panic` finding — so a panic in one check degrades that section to a warning instead of aborting the whole scan. Preserve registry ordering; it determines output section order.
+- **Git subprocess hardening contract (threat model: untrusted PR).** `base_ref` from MCP/CLI is validated by `runnersupport.ValidateBaseRef` (rejects leading `-` to block git option injection; restricts charset; preserves the `stdin` sentinel) BEFORE reaching git, and `--end-of-options` is passed to git diff/show/worktree. All git/govulncheck subprocesses use `exec.CommandContext` with a contained timeout and read output through `io.LimitReader` (`maxGitOutputBytes`) to prevent hangs/OOM. The `//nolint:contextcheck` markers in the git layer (`runner/support`, `ai/fix`, `ai/semantic`, `checks/quality/quality.go`, `runner/runner.go`) are DELIBERATE — the contained timeout is the chosen design; threading caller `ctx` end-to-end is a tracked follow-up. Don't "fix" them by removing the nolint without actually threading ctx. `runner/support/MatchPattern` (glob→regex) uses `regexp.Compile` + a `sync.Map` cache and returns false (never panics) on a malformed config glob.
+- **Lint is enforced in CI.** `make lint` is `go vet`; `make lint-strict` / the CI `golangci-lint-action` step (now blocking — no `continue-on-error`) runs the 16-linter `.golangci.yml`. `golangci-lint run` must report 0 before pushing. The config excludes revive's documentation/naming style rules (`exported`, `package-comments`, `unused-parameter`) and excludes `tests/` from gosec/errcheck/prealloc/bodyclose (intentional fixtures). Cache/identity keys use sha256 (not sha1). Documenting the public `pkg/codeguard` API with doc comments is a known follow-up (revive `exported` is currently excluded rather than satisfied).

.github/workflows/ci.yml

@@ -49,6 +49,11 @@ jobs:
       - name: Run go vet
         run: make lint
 
+      - name: Run golangci-lint
+        uses: golangci/golangci-lint-action@v8
+        with:
+          version: v2.12.2
+
   test:
     name: test
     needs: lint

.golangci.yml

@@ -1,7 +1,48 @@
 version: "2"
-
 run:
-  timeout: 3m
-
+  timeout: 5m
 linters:
-  default: standard
+  enable:
+    - errcheck
+    - govet
+    - staticcheck
+    - errorlint
+    - gocritic
+    - revive
+    - ineffassign
+    - unconvert
+    - prealloc
+    - copyloopvar
+    - unparam
+    - gosec
+    - bodyclose
+    - contextcheck
+    - misspell
+    - nakedret
+  settings:
+    govet:
+      enable:
+        - shadow
+    errcheck:
+      exclude-functions:
+        - fmt.Fprintf
+        - fmt.Fprintln
+        - fmt.Fprint
+  exclusions:
+    rules:
+      # Test fixtures intentionally use fake "credentials", 0o755 helper
+      # scripts, unclosed test HTTP bodies, and unchecked cleanup errors.
+      - path: tests/
+        linters:
+          - errcheck
+          - gosec
+          - prealloc
+          - bodyclose
+      # revive's documentation/naming style rules are not enforced: exported-doc
+      # (a doc comment on every exported symbol), package-comments, and
+      # unused-parameter (e.g. the unused *http.Request in http.HandlerFunc
+      # callbacks). The correctness/safety linters above are what gate CI.
+      # Documenting the public pkg/codeguard API is tracked as a follow-up.
+      - linters:
+          - revive
+        text: "should have comment|should have a package comment|seems to be unused"

Makefile

@@ -24,13 +24,14 @@ export GOMODCACHE
 
 .DEFAULT_GOAL := help
 
-.PHONY: help fmt fmt-check lint test codeguard-ci check ci build release release-snapshot release-check deploy commit table table-diff table-interactive clean
+.PHONY: help fmt fmt-check lint lint-strict test codeguard-ci check ci build release release-snapshot release-check deploy commit table table-diff table-interactive clean
 
 help:
 	@printf "\ncodeguard make targets\n\n"
 	@printf "  make fmt        Format Go files\n"
 	@printf "  make fmt-check  Verify Go files are formatted\n"
 	@printf "  make lint       Run go vet\n"
+	@printf "  make lint-strict  Run golangci-lint (enforced in CI)\n"
 	@printf "  make test       Run the Go test suite\n"
 	@printf "  make codeguard-ci  Validate and scan this repository with codeguard\n"
 	@printf "  make check      Run fmt-check, lint, test, and codeguard-ci\n"
@@ -61,6 +62,9 @@ fmt-check:
 lint:
 	$(GO) vet ./...
 
+lint-strict:
+	golangci-lint run
+
 test:
 	@set -o pipefail; $(GO) test ./... 2>&1 | grep -v '\[no test files\]'
 

internal/cli/commands.go

@@ -17,51 +17,51 @@ func runInit(args []string, stdin io.Reader, stdout io.Writer, stderr io.Writer)
 	output := fs.String("output", service.DefaultConfigPath(), "output config path")
 	interactive := fs.Bool("interactive", false, "prompt for config values in the terminal")
 	profile := fs.String("profile", "", "optional policy profile: startup, strict, enterprise, ai-safe")
-	if err := fs.Parse(args); err != nil {
-		return 1
+	if ok, code := parseFlags(fs, args, stderr); !ok {
+		return code
 	}
 
 	cfg, err := exampleConfigForProfile(strings.TrimSpace(*profile))
 	if err != nil {
 		_, _ = fmt.Fprintf(stderr, "init profile: %v\n", err)
-		return 1
+		return exitError
 	}
 	if *interactive {
 		if err := promptInitValues(bufio.NewReader(stdin), stdout, output, &cfg.Name); err != nil {
 			_, _ = fmt.Fprintf(stderr, "interactive init: %v\n", err)
-			return 1
+			return exitError
 		}
 	}
 
 	if err := service.WriteConfigFile(*output, cfg); err != nil {
 		_, _ = fmt.Fprintf(stderr, "write config: %v\n", err)
-		return 1
+		return exitError
 	}
 	_, _ = fmt.Fprintf(stdout, "wrote %s\n", *output)
-	return 0
+	return exitOK
 }
 
 func runValidate(args []string, stdout io.Writer, stderr io.Writer) int {
 	fs := flag.NewFlagSet("validate", flag.ContinueOnError)
 	fs.SetOutput(stderr)
 	configPath := fs.String("config", service.DefaultConfigPath(), "config file or directory path")
 	profile := fs.String("profile", "", "optional policy profile override")
-	if err := fs.Parse(args); err != nil {
-		return 1
+	if ok, code := parseFlags(fs, args, stderr); !ok {
+		return code
 	}
 
 	cfg, err := loadConfigWithProfile(*configPath, *profile)
 	if err != nil {
 		_, _ = fmt.Fprintf(stderr, "load config: %v\n", err)
-		return 1
+		return exitError
 	}
 	if err := service.ValidateConfig(cfg); err != nil {
 		_, _ = fmt.Fprintf(stderr, "invalid config: %v\n", err)
-		return 1
+		return exitError
 	}
 
 	_, _ = fmt.Fprintln(stdout, "config valid")
-	return 0
+	return exitOK
 }
 
 func runScan(args []string, stdin io.Reader, stdout io.Writer, stderr io.Writer) int {
@@ -72,36 +72,36 @@ func runScan(args []string, stdin io.Reader, stdout io.Writer, stderr io.Writer)
 	format := fs.String("format", "", "optional output format override: text, json, sarif, github")
 	enableAI := fs.Bool("ai", false, "enable optional AI-assisted analysis")
 	interactive := fs.Bool("interactive", false, "prompt for scan inputs in the terminal")
-	if err := fs.Parse(args); err != nil {
-		return 1
+	if ok, code := parseFlags(fs, args, stderr); !ok {
+		return code
 	}
 	flags.applyTrustPolicy()
 
 	if err := promptScanInputs(*interactive, stdin, stdout, &inputs); err != nil {
 		_, _ = fmt.Fprintf(stderr, "interactive scan: %v\n", err)
-		return 1
+		return exitError
 	}
 
 	scanMode, err := parseScanMode(*inputs.mode)
 	if err != nil {
 		_, _ = fmt.Fprintln(stderr, err)
-		return 1
+		return exitError
 	}
 
 	cfg, err := loadConfigWithProfile(*inputs.configPath, *flags.profile)
 	if err != nil {
 		_, _ = fmt.Fprintf(stderr, "load config: %v\n", err)
-		return 1
+		return exitError
 	}
 	if trimmedFormat := strings.TrimSpace(*format); trimmedFormat != "" {
 		cfg.Output.Format = trimmedFormat
 	}
 
 	if err := executeScan(stdout, cfg, scanMode, strings.TrimSpace(*inputs.baseRef), *enableAI); err != nil {
 		_, _ = fmt.Fprintf(stderr, "scan failed: %v\n", err)
-		return 1
+		return exitError
 	}
-	return 0
+	return exitOK
 }
 
 func runValidatePatch(args []string, stdin io.Reader, stdout io.Writer, stderr io.Writer) int {
@@ -111,24 +111,24 @@ func runValidatePatch(args []string, stdin io.Reader, stdout io.Writer, stderr i
 	format := fs.String("format", "", "optional output format override: text, json, sarif, github")
 	enableAI := fs.Bool("ai", false, "enable optional AI-assisted analysis")
 	profile := fs.String("profile", "", "optional policy profile override")
-	if err := fs.Parse(args); err != nil {
-		return 1
+	if ok, code := parseFlags(fs, args, stderr); !ok {
+		return code
 	}
 
 	diffText, err := io.ReadAll(stdin)
 	if err != nil {
 		_, _ = fmt.Fprintf(stderr, "read patch stdin: %v\n", err)
-		return 1
+		return exitError
 	}
 	if strings.TrimSpace(string(diffText)) == "" {
 		_, _ = fmt.Fprintln(stderr, "validate-patch requires a unified diff on stdin")
-		return 1
+		return exitError
 	}
 
 	cfg, err := loadConfigWithProfile(*configPath, *profile)
 	if err != nil {
 		_, _ = fmt.Fprintf(stderr, "load config: %v\n", err)
-		return 1
+		return exitError
 	}
 	if trimmedFormat := strings.TrimSpace(*format); trimmedFormat != "" {
 		cfg.Output.Format = trimmedFormat
@@ -142,32 +142,32 @@ func runValidatePatch(args []string, stdin io.Reader, stdout io.Writer, stderr i
 	})
 	if err != nil {
 		_, _ = fmt.Fprintf(stderr, "patch validation failed: %v\n", err)
-		return 1
+		return exitError
 	}
 	if err := service.WriteReport(stdout, report, cfg.Output.Format); err != nil {
 		_, _ = fmt.Fprintf(stderr, "write report: %v\n", err)
-		return 1
+		return exitError
 	}
 	if report.Summary.FailedSections > 0 {
-		return 1
+		return exitError
 	}
-	return 0
+	return exitOK
 }
 
 func runBaseline(args []string, stdout io.Writer, stderr io.Writer) int {
 	fs := flag.NewFlagSet("baseline", flag.ContinueOnError)
 	fs.SetOutput(stderr)
 	flags := registerScanRunFlags(fs)
 	outputPath := fs.String("output", "codeguard-baseline.json", "baseline output path")
-	if err := fs.Parse(args); err != nil {
-		return 1
+	if ok, code := parseFlags(fs, args, stderr); !ok {
+		return code
 	}
 	flags.applyTrustPolicy()
 
 	cfg, err := loadConfigWithProfile(*flags.configPath, *flags.profile)
 	if err != nil {
 		_, _ = fmt.Fprintf(stderr, "load config: %v\n", err)
-		return 1
+		return exitError
 	}
 	cfg.Baseline.Path = ""
 
@@ -177,14 +177,14 @@ func runBaseline(args []string, stdout io.Writer, stderr io.Writer) int {
 	})
 	if err != nil {
 		_, _ = fmt.Fprintf(stderr, "baseline scan failed: %v\n", err)
-		return 1
+		return exitError
 	}
 	if err := service.WriteBaselineFile(*outputPath, service.BaselineEntriesFromReport(report)); err != nil {
 		_, _ = fmt.Fprintf(stderr, "write baseline: %v\n", err)
-		return 1
+		return exitError
 	}
 	_, _ = fmt.Fprintf(stdout, "wrote %s\n", *outputPath)
-	return 0
+	return exitOK
 }
 
 func promptInitValues(reader *bufio.Reader, stdout io.Writer, output *string, configName *string) error {

internal/cli/doctor_helpers.go

@@ -37,7 +37,7 @@ func targetDoctorChecks(targets []service.TargetConfig) []doctorCheck {
 }
 
 func repoDoctorCheck(target service.TargetConfig) doctorCheck {
-	if err := exec.Command("git", "-C", target.Path, "rev-parse", "--show-toplevel").Run(); err != nil {
+	if err := exec.Command("git", "-C", target.Path, "rev-parse", "--show-toplevel").Run(); err != nil { //nolint:gosec // fixed git subcommand; target.Path is a config scan target
 		return warnDoctorCheck("repo:"+target.Name, fmt.Sprintf("%s is not a git worktree; diff scans will not work", target.Path))
 	}
 	return passDoctorCheck("repo:"+target.Name, "git worktree detected")

internal/cli/exit.go

@@ -0,0 +1,26 @@
+package cli
+
+import (
+	"flag"
+	"io"
+)
+
+// Process exit codes returned by command handlers.
+const (
+	// exitOK indicates the command completed successfully.
+	exitOK = 0
+	// exitError indicates the command failed, or a scan/patch reported one or
+	// more failed sections.
+	exitError = 1
+)
+
+// parseFlags parses args into fs, returning ok=false together with the exit
+// code a handler should return when parsing fails. Flag errors are reported by
+// the FlagSet's own output (set to stderr by callers), so no additional message
+// is written here.
+func parseFlags(fs *flag.FlagSet, args []string, _ io.Writer) (ok bool, code int) {
+	if err := fs.Parse(args); err != nil {
+		return false, exitError
+	}
+	return true, exitOK
+}

internal/cli/info.go

@@ -32,7 +32,7 @@ func runRules(args []string, stdout io.Writer, stderr io.Writer) int {
 		var line strings.Builder
 		line.WriteString(rule.ID)
 		line.WriteByte('\t')
-		line.WriteString(string(rule.DefaultLevel))
+		line.WriteString(rule.DefaultLevel)
 		line.WriteByte('\t')
 		line.WriteString(string(rule.ExecutionModel))
 		line.WriteByte('\t')

internal/cli/mcp_client_requester.go

@@ -56,21 +56,19 @@ func (r *serverRequester) call(ctx context.Context, send func(id string) error)
 	}
 }
 
-// deliver routes an inbound response to a waiting call. It returns false when no
-// server-initiated request is awaiting that id (so the caller can dispatch the
-// message normally).
-func (r *serverRequester) deliver(id string, raw json.RawMessage) bool {
+// deliver routes an inbound response to the call awaiting that id. It is a no-op
+// when no server-initiated request is pending for the id.
+func (r *serverRequester) deliver(id string, raw json.RawMessage) {
 	r.mu.Lock()
 	ch, ok := r.pending[id]
 	r.mu.Unlock()
 	if !ok {
-		return false
+		return
 	}
 	select {
 	case ch <- raw:
 	default:
 	}
-	return true
 }
 
 func parseServerResponse(raw json.RawMessage) (json.RawMessage, error) {

internal/cli/mcp_dispatch.go

@@ -18,7 +18,7 @@ const mcpServerInstructions = "Use validate_patch before writing files to disk w
 func buildResultMessage(id json.RawMessage, result any) map[string]any {
 	return map[string]any{
 		"jsonrpc": "2.0",
-		"id":      json.RawMessage(id),
+		"id":      id,
 		"result":  result,
 	}
 }
@@ -31,7 +31,7 @@ func buildErrorMessage(id *json.RawMessage, code int, message string) map[string
 		"error":   mcpError{Code: code, Message: message},
 	}
 	if id != nil {
-		payload["id"] = json.RawMessage(*id)
+		payload["id"] = *id
 	} else {
 		payload["id"] = nil
 	}
@@ -44,7 +44,7 @@ func buildProgressMessage(token json.RawMessage, progress float64, total float64
 		"jsonrpc": "2.0",
 		"method":  "notifications/progress",
 		"params": map[string]any{
-			"progressToken": json.RawMessage(token),
+			"progressToken": token,
 			"progress":      progress,
 			"total":         total,
 			"message":       message,