fix(scan): stop generic-bearer flagging test fixtures, and honour .forge/waivers

CHANGELOG.md

@@ -4,6 +4,18 @@ All notable changes to forge will be documented in this file. Format follows [Ke
 
 ## [Unreleased]
 
+### Fixed
+
+- **`forge scan security` was permanently red on real projects because of `generic-bearer` false positives.** The built-in `generic-bearer` heuristic flagged quoted 16+ character literals on test fixtures and documentation placeholders (`test_access_token`, `whsec_placeholder`, `mock-refresh-token`, `sbp_your_token_here`). On a real Next.js/Supabase repo that was 56 findings, every one a placeholder, drowning out real hits. The rule now recognises a placeholder *structurally* — a phrase of plain words that either contains a marker word (`test`, `mock`, `fake`, `dummy`, `example`, `placeholder`, `invalid`, `your`, …) or sits in test code — and stops reporting it. Opaque values (`sk_live_…`, `sk_test_…`, hex, UUIDs, base62, JWT headers, mixed-case or letter/digit blends) are still reported in production code **and** in test files, and phrase-shaped values with no marker are still reported outside tests. Only `generic-bearer` changed; the AWS, `sk-`, GitHub-token and private-key rules are untouched. Measured on the repo that produced the findings: 56 → 3, and the 3 that remain (a documented public verify token and two camelCase fixtures) are exactly the cases a heuristic should not guess.
+- **The waiver registry (`.forge/waivers/`, DEV-M1-17) existed but was never consulted by the scanner**, so there was no in-tool way to accept a specific finding. `forge scan <family>` and the `forge ship` security checkpoint now apply waivers: matching findings are removed from the result and counted in the new `waived` field (JSON) / `waived:` line (text) and do not affect the exit code. A waiver missing `rationale`, `approved_by` or `expires_at` fails the scan instead of silently exempting findings; an expired waiver is never honoured and is named in the result `note`.
+- **`waiver.Registry.IsWaived` returned "expired" as soon as it met an expired waiver, even when a valid waiver for the same rule and file followed it.** A lapsed waiver could therefore not be renewed by adding a new entry. A valid match now wins; "expired" is returned only when every matching waiver has lapsed.
+
+### Added
+
+- `ScanResult.Waived` (`"waived"` in `--json`) — number of findings suppressed by waivers.
+- `docs/verbs/scan.md`: how `generic-bearer` treats placeholders, and the waiver file format.
+
+
 ## [1.10.8] — 2026-09-21 — Agent-mode arch debate no longer discards answers, `ship` stops claiming false progress, and the pre-push hook stops inheriting git's `GIT_DIR`
 
 All fixes below were found dogfooding `forge ship --agent-mode` through the spec and arch checkpoints of a real feature on a Next.js/Supabase + Python two-repo system, plus a hook bug found while pushing this very change.

docs/verbs/scan.md

@@ -24,3 +24,47 @@ forge scan
 forge scan --only security
 forge scan --json | jq '.findings'
 ```
+
+## Placeholders in `generic-bearer`
+
+The built-in `generic-bearer` rule flags a quoted literal of 16+ characters assigned to a name
+containing `token`, `secret`, `password` or `api-key`. That is a heuristic, so it also matches test
+fixtures (`test_access_token`, `whsec_placeholder`) and documentation (`sbp_your_token_here`).
+
+Since the rule learned to recognise these, a value is **not** reported when it is a *phrase* — two or
+more `-`/`_` separated segments, each a plain word (all lower case, all UPPER case or Capitalised,
+with at most six trailing digits) — **and** either
+
+- it contains a marker word (`test`, `mock`, `fake`, `dummy`, `example`, `sample`, `placeholder`,
+  `changeme`, `invalid`, `your`, `redacted`, `xxx`, `demo`, `fixture`, `stub`), anywhere in the tree, or
+- the file is test code (a `test`/`tests`/`__tests__`/`__mocks__`/`mocks`/`e2e` directory, or a
+  `*.test.*`, `*.spec.*`, `*_test.*`, `test_*` or `*.mock.*` file).
+
+Opaque values are never excused: anything that mixes upper and lower case or letters and digits inside
+one segment, or is a single long segment (live and test-mode Stripe keys, hex, UUIDs, base62, JWT
+headers), is still reported in production code **and** in test files. A phrase-shaped value with no
+marker (`correct-horse-battery-staple`) is still reported outside test code. Only `generic-bearer` is
+affected: the AWS, `sk-`, GitHub-token and private-key-block rules fire everywhere, including tests.
+
+## Waivers
+
+Accept a specific finding with a waiver instead of leaving the gate red. Put YAML files in
+`.forge/waivers/` (commit them):
+
+```yaml
+- id: W-001
+  rule_id: generic-bearer                       # the rule name printed next to the finding
+  file_path: src/lib/webhookConfig.ts           # optional; omit to cover the rule in every file
+  rationale: >-
+    Public webhook verify token that customers type into their Meta app; documented as not a secret.
+  approved_by: alice
+  expires_at: "2027-03-31"                      # YYYY-MM-DD
+```
+
+- `rationale`, `approved_by` and `expires_at` are required. A waiver missing any of them makes the scan
+  **fail** rather than silently exempt findings.
+- An expired waiver is never honoured. The finding comes back and the `note` says which waiver lapsed.
+  To renew, add a new entry; a valid waiver wins over an expired one that also matches.
+- `file_path` is matched against the scan-relative, slash-separated path (as printed in the finding).
+- Waived findings are removed from `findings`, do not affect `count`, `status` or the exit code, and are
+  counted in `waived` (JSON) / `waived:` (text). `forge ship`'s security checkpoint honours the same files.

internal/cli/cmdscan/placeholder.go

@@ -0,0 +1,104 @@
+// Copyright 2024 The Forge Authors
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package cmdscan
+
+import (
+	"regexp"
+	"strings"
+)
+
+// The built-in `generic-bearer` rule is a heuristic: any quoted literal of 16+
+// [A-Za-z0-9_-] characters assigned to a name containing token/secret/password/
+// api-key. On a real project that produces a wall of findings on test fixtures
+// ("test_access_token", "whsec_placeholder", "mock-refresh-token") and on
+// documentation placeholders ("sbp_your_token_here"), which drowns out real hits
+// and makes `forge scan security` permanently red.
+//
+// isPlaceholderCredential recognises those literals structurally rather than by
+// silencing whole directories, so a real secret that happens to sit in a test
+// file is still reported:
+//
+//   - A value is a candidate only if it is a *phrase*: two or more segments split
+//     on '-' or '_', and every segment is a plain word (all lower case, all upper
+//     case, or Capitalised) optionally followed by up to six digits — or a short
+//     run of digits. Random credentials are opaque: they mix upper and lower
+//     case, letters and digits inside one segment, or are a single long segment,
+//     so they never qualify. That is why live and test-mode Stripe keys, real
+//     webhook secrets, hex,
+//     UUIDs, base62 and JWT-ish strings are still flagged.
+//   - A phrase is then treated as a placeholder only when something says it is
+//     not real: it contains an explicit marker word (test, mock, fake, dummy,
+//     example, sample, placeholder, invalid, your, …) OR it sits in a test path.
+//     A phrase-shaped value in production code with no marker
+//     ("correct-horse-battery-staple") is still reported.
+//
+// Provider-specific rules (AWS keys, sk- keys, GitHub tokens, private-key
+// blocks) are not affected: they are checked independently of this heuristic.
+
+// placeholderMarkers are segment words that state, in the value itself, that it
+// is not a real credential.
+var placeholderMarkers = map[string]struct{}{
+	"test": {}, "mock": {}, "fake": {}, "dummy": {}, "example": {}, "sample": {},
+	"placeholder": {}, "changeme": {}, "invalid": {}, "your": {}, "redacted": {},
+	"xxx": {}, "demo": {}, "fixture": {}, "stub": {},
+}
+
+// plainSegment matches one word of a phrase: lower, UPPER or Capitalised
+// letters with at most six trailing digits, or a bare run of up to six digits.
+// Mixed-case blends such as "qWeRtY" and letter/digit blends such as "a1b2c3"
+// do not match, which is what keeps opaque credentials out.
+var plainSegment = regexp.MustCompile(`^(?:[a-z]+|[A-Z]+|[A-Z][a-z]+)[0-9]{0,6}$|^[0-9]{1,6}$`)
+
+// testPathDirs are directory names that mark a path as test code.
+var testPathDirs = map[string]struct{}{
+	"test": {}, "tests": {}, "__tests__": {}, "__mocks__": {}, "mocks": {},
+	"e2e": {}, "__fixtures__": {},
+}
+
+// isTestPath reports whether rel (slash-separated, relative to the scan root)
+// is test code, by directory name or by conventional file-name pattern.
+func isTestPath(rel string) bool {
+	parts := strings.Split(strings.ToLower(rel), "/")
+	for _, dir := range parts[:len(parts)-1] {
+		if _, ok := testPathDirs[dir]; ok {
+			return true
+		}
+	}
+	base := parts[len(parts)-1]
+	return strings.Contains(base, ".test.") || strings.Contains(base, ".spec.") ||
+		strings.Contains(base, "_test.") || strings.HasPrefix(base, "test_") ||
+		strings.Contains(base, ".mock.")
+}
+
+// isPlaceholderCredential reports whether a quoted literal matched by the
+// generic-bearer heuristic is recognisably not a real credential. See the
+// package comment above for the exact rule and its limits.
+func isPlaceholderCredential(rel, value string) bool {
+	segments := strings.FieldsFunc(value, func(r rune) bool { return r == '-' || r == '_' })
+	if len(segments) < 2 {
+		return false
+	}
+	marked := false
+	for _, seg := range segments {
+		if !plainSegment.MatchString(seg) {
+			return false
+		}
+		word := strings.ToLower(strings.TrimRight(seg, "0123456789"))
+		if _, ok := placeholderMarkers[word]; ok {
+			marked = true
+		}
+	}
+	return marked || isTestPath(rel)
+}

internal/cli/cmdscan/placeholder_test.go

@@ -0,0 +1,149 @@
+// Copyright 2024 The Forge Authors
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package cmdscan
+
+import "testing"
+
+// Values below are taken from a real project's scan output (56 generic-bearer
+// findings, all fixtures/docs) so the rule is pinned to real data, not to
+// examples invented to fit it.
+func TestIsPlaceholderCredential_RealWorldPlaceholders(t *testing.T) {
+	t.Parallel()
+	cases := []struct {
+		name, rel, value string
+	}{
+		// explicit marker word, anywhere in the tree
+		{"mock in test", "tests/unit/a.test.ts", "mock-refresh-token"},
+		{"mock in setup file", "jest.setup.js", "mock-refresh-token"},
+		{"test marker in prod code", "src/lib/oauth/google.ts", "promotiai-test-connection-probe-invalid-token"},
+		{"your in docs", "docs/QUICK_APPLY_GUIDE.md", "sbp_your_token_here"},
+		{"YOUR upper case in script", "scripts/restart.ps1", "YOUR_ACCESS_TOKEN"},
+		{"placeholder word", "src/x.ts", "whsec_placeholder_value"},
+		{"invalid marker", "src/x.ts", "invalid_refresh_token"},
+		{"do-not-use", "src/webhook.test.ts", "test-secret-do-not-use-in-prod"},
+		// no marker, but plain words in test code
+		{"phrase in tests dir", "tests/integration/oauth.test.js", "duplicate_secret"},
+		{"trailing digits are plain", "tests/unit/oauth.test.ts", "invalid_token_12345"},
+		{"digit suffix on a word", "tests/unit/tw.test.ts", "tw-oauth1-access-token"},
+		{"new_access_token", "src/test/refresh.test.ts", "new_access_token"},
+		{"file-name pattern only", "src/lib/refresh.spec.ts", "brand_new_token_abc123"},
+		{"__tests__ dir", "src/__tests__/a.ts", "my_super_secret_value_here"},
+	}
+	for _, c := range cases {
+		c := c
+		t.Run(c.name, func(t *testing.T) {
+			t.Parallel()
+			if !isPlaceholderCredential(c.rel, c.value) {
+				t.Fatalf("isPlaceholderCredential(%q, %q) = false; want true", c.rel, c.value)
+			}
+		})
+	}
+}
+
+// The important half: anything that looks like a real credential must keep
+// being reported, in production code AND in test files.
+func TestIsPlaceholderCredential_RealSecretsStillFlagged(t *testing.T) {
+	t.Parallel()
+	// Secret-shaped values are assembled from fragments so no source line contains a
+	// contiguous provider key: GitHub push protection (rightly) cannot tell a fixture
+	// from a leak, and blocked the first push of this file.
+	secrets := map[string]string{
+		"stripe live key":        "sk_" + "live_51HabcDEFghiJKLmnoPQRstu",
+		"stripe test key":        "sk_" + "test_51HabcDEFghiJKLmnoPQRstu", // test-MODE keys are still secrets
+		"random base62":          "q8Zr3KfL0wXv9TbNc2YpHs7D",
+		"hex":                    "3f2a9c1e7b4d40a8b6c5d2e1f0a9b8c7",
+		"uuid":                   "3f2a9c1e-7b4d-40a8-b6c5-d2e1f0a9b8c7",
+		"jwt header":             "eyJhbGciOiJIUzI1NiIsInR5cCI",
+		"real whsec":             "whsec_" + "Xk3Lm9Qw2Ert7Yu1Io5PaSd4Fg",
+		"mixed case blend":       "qWeRtY_asDfGh_zXcVbN",
+		"letter-digit blend":     "live-tok-a1b2c3d4e5f6g7h8",
+		"marker but opaque part": "test_Xk3Lm9Qw2Ert7Yu1Io5PaSd4Fg",
+		"single long lower word": "abcdefghijklmnopqrstuvwxyz",
+		"camelCase single token": "xClientSecret123abc456def789ghi",
+	}
+	for name, v := range secrets {
+		name, v := name, v
+		for _, rel := range []string{"src/lib/auth.ts", "tests/unit/auth.test.ts"} {
+			rel := rel
+			t.Run(name+" @ "+rel, func(t *testing.T) {
+				t.Parallel()
+				if isPlaceholderCredential(rel, v) {
+					t.Fatalf("isPlaceholderCredential(%q, %q) = true; a real-looking secret must still be reported", rel, v)
+				}
+			})
+		}
+	}
+}
+
+// A phrase-shaped value with no marker is only excused inside test code. The same
+// literal in production code is still a plausible hard-coded password.
+func TestIsPlaceholderCredential_PhraseInProductionStillFlagged(t *testing.T) {
+	t.Parallel()
+	for _, v := range []string{"correct-horse-battery-staple", "my-secret-password-2024", "promotiai-social-inbox-webhook"} {
+		if isPlaceholderCredential("src/lib/auth.ts", v) {
+			t.Errorf("%q in production code must still be reported", v)
+		}
+		if !isPlaceholderCredential("tests/unit/auth.test.ts", v) {
+			t.Errorf("%q in a test file should be treated as a fixture", v)
+		}
+	}
+}
+
+func TestIsPlaceholderCredential_Boundaries(t *testing.T) {
+	t.Parallel()
+	cases := []struct {
+		value string
+		want  bool
+	}{
+		{"", false},
+		{"testtesttesttesttest", false}, // marker, but one segment: not phrase-shaped
+		{"test_", false},                // one segment after splitting
+		{"_test_mock_", true},           // leading/trailing separators are ignored
+		{"test__mock--fake", true},      // repeated separators collapse
+		{"test_1234567", false},         // 7 digits is not a short number
+		{"test_123456", true},           // 6 digits is
+		{"Test_Mock_Token", true},       // Capitalised words
+		{"TEST_MOCK_TOKEN", true},       // UPPER words
+		{"tEsT_mock_token", false},      // mixed-case blend
+	}
+	for _, c := range cases {
+		if got := isPlaceholderCredential("src/x.ts", c.value); got != c.want {
+			t.Errorf("isPlaceholderCredential(src/x.ts, %q) = %v; want %v", c.value, got, c.want)
+		}
+	}
+}
+
+func TestIsTestPath(t *testing.T) {
+	t.Parallel()
+	yes := []string{
+		"tests/a.js", "test/a.js", "src/__tests__/a.ts", "src/__mocks__/x.ts", "e2e/login.ts",
+		"src/lib/a.test.ts", "src/lib/a.spec.js", "pkg/a_test.go", "scripts/test_helpers.py", "src/api.mock.ts",
+		"TESTS/A.JS", // case-insensitive
+	}
+	no := []string{
+		"src/lib/a.ts", "src/contest/a.ts", "src/latest/a.ts", "docs/spec/a.md", "src/attestation.ts",
+		"src/lib/testify.ts", "a.ts",
+	}
+	for _, p := range yes {
+		if !isTestPath(p) {
+			t.Errorf("isTestPath(%q) = false; want true", p)
+		}
+	}
+	for _, p := range no {
+		if isTestPath(p) {
+			t.Errorf("isTestPath(%q) = true; want false", p)
+		}
+	}
+}

internal/cli/cmdscan/scan.go

@@ -54,6 +54,9 @@ type ScanResult struct {
 	Count    int       `json:"count"`
 	Status   string    `json:"status"` // "clean", "suspicious", "found"
 	Note     string    `json:"note,omitempty"`
+	// Waived counts findings suppressed by a valid .forge/waivers entry. They are
+	// removed from Findings and do not affect Count, Status or the exit code.
+	Waived int `json:"waived,omitempty"`
 }
 
 func init() {
@@ -176,6 +179,11 @@ func New() *cobra.Command {
 			// G-022: assign confidence scores.
 			res.Findings = AssignConfidence(res.Findings)
 
+			// DEV-M1-17: drop findings covered by a valid, unexpired .forge/waivers entry.
+			if err := ApplyWaivers(root, res); err != nil {
+				return err
+			}
+
 			// G-023: --since diff against baseline.
 			if since != "" {
 				baseline := loadScanBaseline(root, scanner)
@@ -769,12 +777,20 @@ func scanWithBuiltinPatterns(root string) []Finding {
 		{"private-key-block", regexp.MustCompile(`-----BEGIN [A-Z ]*PRIVATE KEY-----`)},
 		// generic-bearer: require the value to be a quoted string literal so that
 		// variable-name references (e.g. token = csrfTokenVar) are not flagged.
-		{"generic-bearer", regexp.MustCompile(`(?i)(bearer|api[_-]?key|token|secret|password)\s*[:=]\s*["'][A-Za-z0-9_\-]{16,}["']`)},
+		// The value is captured (group 2) so recognisable placeholders can be
+		// dropped — see isPlaceholderCredential.
+		{"generic-bearer", regexp.MustCompile(`(?i)(bearer|api[_-]?key|token|secret|password)\s*[:=]\s*["']([A-Za-z0-9_\-]{16,})["']`)},
 	}
 	return scanFiles(root, func(rel string, line int, text string) []Finding {
 		var out []Finding
 		for _, r := range rules {
-			if loc := r.Pattern.FindStringIndex(text); loc != nil {
+			if loc := r.Pattern.FindStringSubmatchIndex(text); loc != nil {
+				// A phrase-shaped literal that says it is not real (marker word) or that
+				// lives in test code is a fixture/doc placeholder, not a leaked secret.
+				if r.Name == "generic-bearer" && len(loc) >= 6 &&
+					isPlaceholderCredential(rel, text[loc[4]:loc[5]]) {
+					continue
+				}
 				out = append(out, Finding{
 					File: rel, Line: line, Rule: r.Name,
 					Match: truncate(text, 80), Secret: text[loc[0]:loc[1]],
@@ -1088,6 +1104,9 @@ func renderText(cmd *cobra.Command, r *ScanResult) {
 	fmt.Fprintf(w, "forge scan\n")
 	fmt.Fprintf(w, "findings: %d\n", r.Count)
 	fmt.Fprintf(w, "status:   %s\n", r.Status)
+	if r.Waived > 0 {
+		fmt.Fprintf(w, "waived:   %d\n", r.Waived)
+	}
 	if r.Note != "" {
 		fmt.Fprintf(w, "note:     %s\n", r.Note)
 	}