Add a static check for detectors that don't set SecretParts

.github/workflows/lint.yml

@@ -50,3 +50,18 @@ jobs:
     steps:
       - uses: actions/checkout@v4
       - run: semgrep --config=hack/semgrep-rules/detectors.yaml pkg/detectors/
+  checksecretparts:
+    # Warning-only: reports detector packages that construct detectors.Result
+    # without populating SecretParts. Flip to hard-fail (drop continue-on-error
+    # and run with -fail) after every detector has been migrated. See
+    # hack/checksecretparts/README.md.
+    name: checksecretparts (warning)
+    runs-on: ubuntu-latest
+    continue-on-error: true
+    steps:
+      - uses: actions/checkout@v4
+      - uses: actions/setup-go@v5
+        with:
+          go-version: "1.25"
+      - name: Run checksecretparts
+        run: go run ./hack/checksecretparts ./pkg/detectors

hack/checksecretparts/README.md

@@ -0,0 +1,59 @@
+# checksecretparts
+
+Static analysis check that finds detector packages which construct
+`detectors.Result` values without populating the `SecretParts` field.
+
+## What it checks
+
+For each directory under `pkg/detectors/` (recursing into subpackages):
+
+1. Find every composite literal of the form `detectors.Result{...}` or
+   `&detectors.Result{...}` in non-test `.go` files.
+2. If the package does not mention `SecretParts` anywhere (neither in the
+   literal nor in a later `x.SecretParts = ...` assignment), emit a warning
+   for each construction site.
+
+Test files (`_test.go`) are ignored on both sides — construction sites in
+tests are not flagged, and `SecretParts` references in tests do not suppress
+findings, because some tests zero the field for comparison.
+
+## Why warning-only
+
+This check ships as **warning-only** because ~907 existing detectors do not
+yet populate `SecretParts` (see the SecretParts design doc, step C).
+Hard-failing today would block every unrelated PR. The check is wired into
+CI with `continue-on-error: true` so the findings are visible without
+gating merges.
+
+## Running locally
+
+```sh
+# Warning mode (default): prints findings, always exits 0 unless scanning fails.
+go run ./hack/checksecretparts
+
+# Scan specific directories instead of ./pkg/detectors.
+go run ./hack/checksecretparts ./pkg/detectors/aws ./pkg/detectors/github
+
+# Fail mode: exit 1 if any findings are reported. Use this once every detector
+# has been migrated to populate SecretParts.
+go run ./hack/checksecretparts -fail
+```
+
+## Flipping warning → fail
+
+Once step C is complete and every detector populates `SecretParts`, make
+this check gating:
+
+1. In `.github/workflows/lint.yml`, drop `continue-on-error: true` from the
+   `checksecretparts` job and change the run step to pass `-fail`.
+2. Land any remaining migrations in the same PR as the flip.
+
+## Scope limits
+
+- It is a syntactic check. It matches `detectors.Result` by selector-expr
+  name; packages that rename the import (`d "...detectors"`) would not be
+  caught. No such rename exists in the current codebase.
+- It does not verify that `SecretParts` is populated on every code path —
+  only that the package touches the field at all. A finer-grained
+  dataflow check is deliberately out of scope; the rough check is enough
+  to surface unmigrated detectors.

hack/checksecretparts/check.go

@@ -0,0 +1,184 @@
+package main
+
+import (
+	"fmt"
+	"go/ast"
+	"go/parser"
+	"go/token"
+	"os"
+	"path/filepath"
+	"sort"
+	"strings"
+)
+
+// Finding describes a single detectors.Result{} construction in a package that
+// never references the SecretParts field.
+type Finding struct {
+	// Position is the source location of the offending composite literal.
+	Position token.Position
+	// Package is the directory containing the finding.
+	Package string
+}
+
+// CheckPackageDir runs the SecretParts check on a single directory. It returns
+// one Finding per detectors.Result{} construction site in the directory,
+// filtered so that packages which mention SecretParts anywhere produce no
+// findings. Test files (_test.go) are ignored on both sides: construction
+// sites in them are not reported, and references in them do not suppress
+// findings (test files commonly zero the field for comparison — see
+// pkg/detectors/gitlab/v1/gitlab_integration_test.go).
+func CheckPackageDir(dir string) ([]Finding, error) {
+	fset := token.NewFileSet()
+	entries, err := os.ReadDir(dir)
+	if err != nil {
+		return nil, err
+	}
+
+	var files []*ast.File
+	for _, e := range entries {
+		if e.IsDir() {
+			continue
+		}
+		name := e.Name()
+		if !strings.HasSuffix(name, ".go") || strings.HasSuffix(name, "_test.go") {
+			continue
+		}
+		path := filepath.Join(dir, name)
+		f, err := parser.ParseFile(fset, path, nil, parser.SkipObjectResolution)
+		if err != nil {
+			return nil, fmt.Errorf("parse %s: %w", path, err)
+		}
+		files = append(files, f)
+	}
+	if len(files) == 0 {
+		return nil, nil
+	}
+
+	return checkFiles(fset, dir, files), nil
+}
+
+// checkFiles inspects a set of parsed, non-test files from a single directory
+// and returns findings. It is separated from CheckPackageDir so that tests can
+// drive it with synthetic ASTs.
+func checkFiles(fset *token.FileSet, dir string, files []*ast.File) []Finding {
+	var (
+		constructions  []token.Position
+		hasSecretParts bool
+	)
+
+	for _, f := range files {
+		if fileReferencesSecretParts(f) {
+			hasSecretParts = true
+		}
+		constructions = append(constructions, findResultConstructions(fset, f)...)
+	}
+
+	if hasSecretParts || len(constructions) == 0 {
+		return nil
+	}
+
+	sort.Slice(constructions, func(i, j int) bool {
+		if constructions[i].Filename != constructions[j].Filename {
+			return constructions[i].Filename < constructions[j].Filename
+		}
+		return constructions[i].Offset < constructions[j].Offset
+	})
+
+	out := make([]Finding, len(constructions))
+	for i, pos := range constructions {
+		out[i] = Finding{Position: pos, Package: dir}
+	}
+	return out
+}
+
+// findResultConstructions returns positions of composite literals with the
+// type detectors.Result (selector expression with identifier "detectors" and
+// selector "Result"). It covers both bare literals and pointer literals
+// (&detectors.Result{}).
+func findResultConstructions(fset *token.FileSet, f *ast.File) []token.Position {
+	var positions []token.Position
+	ast.Inspect(f, func(n ast.Node) bool {
+		lit, ok := n.(*ast.CompositeLit)
+		if !ok {
+			return true
+		}
+		if !isDetectorsResultType(lit.Type) {
+			return true
+		}
+		if hasSecretPartsKey(lit) {
+			return true
+		}
+		positions = append(positions, fset.Position(lit.Lbrace))
+		return true
+	})
+	return positions
+}
+
+func isDetectorsResultType(expr ast.Expr) bool {
+	// Unwrap *detectors.Result which won't appear for composite literal Type,
+	// but handle parenthesized forms defensively.
+	for {
+		switch e := expr.(type) {
+		case *ast.ParenExpr:
+			expr = e.X
+			continue
+		case *ast.StarExpr:
+			expr = e.X
+			continue
+		}
+		break
+	}
+	sel, ok := expr.(*ast.SelectorExpr)
+	if !ok {
+		return false
+	}
+	pkg, ok := sel.X.(*ast.Ident)
+	if !ok {
+		return false
+	}
+	return pkg.Name == "detectors" && sel.Sel.Name == "Result"
+}
+
+func hasSecretPartsKey(lit *ast.CompositeLit) bool {
+	for _, elt := range lit.Elts {
+		kv, ok := elt.(*ast.KeyValueExpr)
+		if !ok {
+			continue
+		}
+		key, ok := kv.Key.(*ast.Ident)
+		if !ok {
+			continue
+		}
+		if key.Name == "SecretParts" {
+			return true
+		}
+	}
+	return false
+}
+
+// fileReferencesSecretParts returns true if the file mentions the identifier
+// "SecretParts" in any form: a composite-literal key, a selector expression
+// (x.SecretParts), or a bare identifier. The rationale is that if a detector
+// package touches SecretParts at all — whether on the construction site or in
+// a later assignment — it has been migrated; the check's job is to find
+// packages that never touch it.
+func fileReferencesSecretParts(f *ast.File) bool {
+	found := false
+	ast.Inspect(f, func(n ast.Node) bool {
+		if found {
+			return false
+		}
+		switch x := n.(type) {
+		case *ast.Ident:
+			if x.Name == "SecretParts" {
+				found = true
+			}
+		case *ast.SelectorExpr:
+			if x.Sel != nil && x.Sel.Name == "SecretParts" {
+				found = true
+			}
+		}
+		return !found
+	})
+	return found
+}