From f2f6c61e171c46eb8af7b554e506c719a077aaf2 Mon Sep 17 00:00:00 2001 From: Alex Wilkerson John Date: Tue, 30 Jun 2026 13:32:44 -0400 Subject: [PATCH 1/4] feat(security): hardcoded secret/credential detection + git-history scan MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Add a repository-wide secret/credential scanner to the security suite that runs in both full scans and diff (PR) scans, and a command to scan git history. Detection tiers: - security.hardcoded-credential (fail): known provider formats — AWS, GitHub, GitLab, Slack (token + webhook), Stripe, Google, npm, PyPI, Docker Hub, SendGrid, Twilio, Azure, DB connection strings with embedded passwords, and Bearer tokens — plus config-defined custom_patterns - security.hardcoded-secret (warn): lower-confidence name-based heuristic (reclassified from fail) - security.high-entropy-string (warn, opt-in): Shannon-entropy detection of unknown/random secrets Config (security_rules.secrets): enabled, allow_paths, allow_patterns, custom_patterns, entropy. Values are masked in findings; placeholders and secret references (op://, vault://, $(...)) are skipped automatically. New `codeguard scan-history` walks git history for secrets that were committed and later removed (still leaked, need rotation), reusing the same scanner. Performance/hardening: a fast literal pre-filter gates the per-pattern regexes (~31x faster default scan, ~15x with entropy); binary/file-size/line-length caps bound work on untrusted PR input; history parsing avoids silent truncation on over-long lines. Co-Authored-By: Claude Opus 4.8 (1M context) --- .claude/knowledge/architecture-boundaries.md | 6 + README.md | 3 +- docs/checks.md | 93 +++++++- docs/security.md | 2 + examples/codeguard.json | 19 ++ internal/cli/helpers.go | 1 + internal/cli/run.go | 1 + internal/cli/scan_history.go | 83 +++++++ .../codeguard/checks/security/security.go | 14 ++ .../checks/security/security_common.go | 9 +- .../security/security_secret_entropy.go | 176 +++++++++++++++ .../security/security_secret_patterns.go | 157 +++++++++++++ .../checks/security/security_secrets.go | 197 +++++++++++++++++ internal/codeguard/config/defaults_rules.go | 6 + internal/codeguard/config/example_rules.go | 4 + internal/codeguard/config/validate.go | 1 + internal/codeguard/config/validate_secrets.go | 67 ++++++ internal/codeguard/core/config_rule_types.go | 1 + .../codeguard/core/config_secrets_types.go | 39 ++++ internal/codeguard/history/history.go | 178 +++++++++++++++ internal/codeguard/rules/catalog_security.go | 22 +- .../codeguard/rules/catalog_security_owasp.go | 6 +- pkg/codeguard/sdk_history.go | 35 +++ pkg/codeguard/sdk_types_state.go | 3 + tests/checks/security_history_test.go | 83 +++++++ tests/checks/security_secrets_scanner_test.go | 84 +++++++ tests/checks/security_secrets_test.go | 208 ++++++++++++++++++ tests/checks/security_test.go | 29 ++- tests/checks/test_helpers_test.go | 14 ++ 29 files changed, 1527 insertions(+), 14 deletions(-) create mode 100644 internal/cli/scan_history.go create mode 100644 internal/codeguard/checks/security/security_secret_entropy.go create mode 100644 internal/codeguard/checks/security/security_secret_patterns.go create mode 100644 internal/codeguard/checks/security/security_secrets.go create mode 100644 internal/codeguard/config/validate_secrets.go create mode 100644 internal/codeguard/core/config_secrets_types.go create mode 100644 internal/codeguard/history/history.go create mode 100644 pkg/codeguard/sdk_history.go create mode 100644 tests/checks/security_history_test.go create mode 100644 tests/checks/security_secrets_scanner_test.go create mode 100644 tests/checks/security_secrets_test.go diff --git a/.claude/knowledge/architecture-boundaries.md b/.claude/knowledge/architecture-boundaries.md index c669b06..ee9de90 100644 --- a/.claude/knowledge/architecture-boundaries.md +++ b/.claude/knowledge/architecture-boundaries.md @@ -8,6 +8,12 @@ Key architectural decisions, service boundaries, data flow, integration points, - **OWASP categories are baked into the rule catalog at var-init time**, not via `init()`. The merged `catalog` var calls `withSecurityOWASP(mergeRuleCatalogs(...))` in `rules/catalog.go`; an `init()` would run *after* the `catalog` var initializer copied stale entries. The mapping lives in `rules/catalog_security_owasp.go`. All read paths (`Catalog`, `RuleCatalogForConfig`, SARIF, CLI) flow through `rules.Catalog()`. - Config-controlled artifact paths (`baseline.path`, `cache.path`, `ai.cache.path`) are resolved relative to the config dir and contained within it by `config.containConfigArtifactPaths` (in `config/io.go`, `LoadFile`). Paths escaping the config directory are rejected. - The language-agnostic OWASP-gap line rules (CORS, debug, bind-all, weak-hash/cipher, deserialization, Dockerfile root) live in `checks/security/security_owasp_extra.go` and run per-line via `findingsForFile`. NOTE: `findingsForFile` is the **non-TypeScript** path — `securityTargetFindings` routes TS/JS targets to a separate `typeScriptTargetFindings` pipeline, so these rules do not run on TS/JS targets. String-literal signals (CORS, bind-all) match the raw line; call/identifier signals match the masked line. +- **Secret/credential scanning (`checks/security/security_secrets.go`) runs as its own repository-wide pass in `securityTargetFindings`, unconditionally for BOTH the TS and non-TS branches** — precisely so it covers TS/JS targets that bypass `findingsForFile`. Tiers: `security.hardcoded-credential` (fail; known provider formats + `security_rules.secrets.custom_patterns`), `security.private-key` (fail), `security.hardcoded-secret` (warn; lower-confidence name-based heuristic — was fail before, deliberately downgraded). Patterns match the **raw** line (the token lives in a string literal that masking would blank). Placeholder/allowlist filtering is done in Go after the regex match because RE2 has no lookahead (mirrors `isInsecureDeserialization`). Config: `security_rules.secrets` (`enabled`/`allow_paths`/`allow_patterns`/`custom_patterns`), defaulted in `applySecurityDefaults`, validated in `validateSecretsRules`. +- **PERF: the secret scanner is gated by a cheap literal pre-check (`builtinGatePasses`), not a combined regex.** Profiling showed `strings.Contains`/byte scans are ~85x faster than RE2 alternation, and that a leading `\b` or `(?i)` defeats RE2's literal-prefix fast path (those patterns ran ~1.8–7ms per 2000 lines; literal-prefix patterns like the Slack webhook ran ~50µs). The gate is `gateLiterals` (case-sensitive `strings.Contains`) + `gateFoldLiterals` (lowercase, via alloc-free `asciiContainsFold`); when a line contains no marker, the ~17 built-in regexes are skipped. Net: default scan went 3.2→99 MB/s, entropy mode 3.1→47 MB/s. **INVARIANT: the gate must be a superset of every built-in pattern — adding a credential pattern requires adding its required literal to the gate.** `TestBuiltinGateCoversPatterns` enforces this; custom config patterns have arbitrary markers so they bypass the gate and always run. +- **HARDENING (codeguard scans untrusted PR content): the scanner bounds its work** — skips files > `maxScanFileBytes` (5 MiB), scans only the first `maxScanLineBytes` (64 KiB) of any line, and skips binary files (`looksBinary` = NUL byte in first 8 KiB). The git-history parser uses `bufio.Reader.ReadString` (grows for any line length), NOT `bufio.Scanner` — Scanner silently stops on an over-long token, which for a security scan would skip the rest of history undetected. +- **The secret scanner exposes a pure API for reuse: `security.BuildScanner(cfg) Scanner` + `Scanner.ScanContent(content) []Match` + `Scanner.SkipPath(file)`.** `secretFindingsForFile` wraps `ScanContent` with `env.NewFinding`. Tiers in `scanLine` (one match per line, priority order): private-key → `credentialPatterns` (known formats + DB/Bearer/Azure, fail) → config `custom_patterns` → name-based (warn) → opt-in entropy (`security.high-entropy-string`, warn). Values are masked in messages via `maskSecret` (first4…last4). Entropy = `shannonEntropy` (bits/char) over whitespace-free quoted literals, gated by `secrets.entropy` (off by default; min_length 20, threshold 4.5). GitHub-token CRC32 checksum validation was deliberately NOT added — getting the algorithm slightly wrong yields false negatives (worse than a false positive for a security tool). +- **Git-history secret scan lives in its own top-level package `internal/codeguard/history` (shells `git log -p -U0`), exposed via `pkg/codeguard.ScanGitHistory` and the `scan-history` CLI command.** It MUST be a separate package, not `runner/support`: `checks/security` imports `runner/support` (for `MatchPattern`), so `runner/support` importing `checks/security` would cycle. `history` imports `checks/security` (above it) and is consumed by `pkg` + CLI (both top-level). The parser tracks `+++ b/`, hunk headers (`@@ … +c`), and `+`/`-` line prefixes to attribute added lines to commits; dedupes by rule+path+masked-message keeping the newest commit (log is newest-first). +- **GOTCHA: `support.ScanTargetFiles` caches per-file findings keyed by `(sectionID, target.Path, rel)`.** If two passes scan the same files with the same `sectionID`, the second pass gets a cache hit and returns the FIRST pass's findings. The secret pass therefore uses a distinct cache id `"security-secrets"` (not `"security"`) so it doesn't collide with the language pass. `sectionID` here only affects the cache key — findings still land in the real section via their RuleID/`FinalizeSection`. When adding any additional `ScanTargetFiles` call to an existing section, give it a unique cache id. - **The MCP server is hand-rolled (no SDK) and transport-agnostic.** Shared JSON-RPC message builders + the synchronous method router live in `internal/cli/mcp_dispatch.go` (`buildResultMessage`/`buildErrorMessage`/`buildProgressMessage`, `serverCapabilities`, `dispatchSyncMethod`). Both transports reuse them. stdio (`mcp_run.go`/`mcp_requests.go`) keeps an async goroutine + cancel-map for `tools/call` (cancellation via `notifications/cancelled`); HTTP (`mcp_http.go`) runs `tools/call` synchronously per-request and streams progress over SSE, cancellation driven by the request context. When adding a capability, wire it into `dispatchSyncMethod` (HTTP picks it up automatically) AND the stdio method switch in `mcp_run.go`. - **MCP server→client requests (sampling/roots)** flow through `internal/cli/mcp_client.go`: `clientCaller`/`clientBridge` + a `serverRequester` (pending-id→chan registry). The transport supplies a `send` closure and feeds inbound responses (classified by `isResponseMessage`: id present, no method) back via `serverRequester.deliver`. stdio demuxes responses in the read loop (`mcp_run.go handleLine`); HTTP needs the client to open the `GET {mcp-path}` SSE stream — server-initiated requests are written there and answered on later POSTs, correlated per session in `internal/cli/mcp_http_session.go`. Client capabilities are captured at `initialize` (`parseClientCapabilities`). The `clientCaller`/progress emitter reach tools via context (`withClientCaller`/`withProgress`), not signatures. - **MCP fix tools** (`internal/cli/mcp_fix.go`): `verify_fix` wraps `service.VerifyFix` (caller diff); `propose_fix` wraps `service.GenerateVerifiedFix` with a generator resolved as sampling-first (`samplingGenerator` calls the client LLM) then `internalfix.NewAIGenerator(cfg.AI)`; `apply_fix` verifies then writes the tree via `runnersupport.ApplyUnifiedDiff` (the one `destructiveHint` tool), confirming via `clientCaller.elicit` (`elicitation/create`) when supported. Verification test execution is trust-gated, so it needs `CODEGUARD_ALLOW_CONFIG_COMMANDS=1` or it fails closed. Fix failures return `toolErrorResultData` (isError + structuredContent with attempted diff + remaining findings). diff --git a/README.md b/README.md index 84a3700..88b300e 100644 --- a/README.md +++ b/README.md @@ -71,9 +71,10 @@ codeguard init codeguard validate -config codeguard.yaml codeguard doctor -config codeguard.yaml codeguard scan -config codeguard.yaml +codeguard scan-history codeguard rules codeguard profiles -codeguard explain security.hardcoded-secret +codeguard explain security.hardcoded-credential codeguard baseline -config codeguard.yaml -output codeguard-baseline.json ``` diff --git a/docs/checks.md b/docs/checks.md index be40e16..46ef8a1 100644 --- a/docs/checks.md +++ b/docs/checks.md @@ -430,12 +430,103 @@ Language command example: ## Security Purpose: -- Hardcoded secret detection +- Hardcoded credential detection (known provider formats) +- Hardcoded secret detection (name-based heuristic) - Private key detection - Insecure TLS detection - Shell execution review markers - Optional `govulncheck` +### Secret & credential scanning + +The secret scan runs **repository-wide for every target language** (including +TypeScript/JavaScript) and reports in both full scans and `-mode diff` scans, so a +hardcoded credential introduced in a PR fails the changed-lines diff check as well as a +full scan. It has two confidence tiers: + +- `security.hardcoded-credential` (**fail**) — a value matching a known provider format: + AWS access keys, GitHub/GitLab tokens, Slack tokens and webhook URLs, Stripe live keys, + Google API keys, npm/PyPI/Docker Hub tokens, SendGrid and Twilio keys, Azure storage + account keys, database connection strings with embedded passwords + (`postgres://user:pass@…`), `Authorization: Bearer …` tokens, + `aws_secret_access_key`/`client_secret`/`private_token` assignments, plus any configured + `custom_patterns`. +- `security.hardcoded-secret` (**warn**) — the lower-confidence name-based heuristic: a + `secret`/`token`/`api_key`/`password` identifier assigned a quoted literal. +- `security.high-entropy-string` (**warn**, opt-in) — a high-entropy string literal that + may be an unknown/random secret matching no known format. Enabled via + `secrets.entropy.enabled`. + +Findings report the value **masked** (`AKIA…CDEF`) so the message itself never reprints +the secret. Obvious placeholders are skipped automatically (`REDACTED`, `xxxx…`, +`example`, `your-…`, `${ENV}` / `{{ }}` / `<…>` interpolations, `$(...)` command +substitutions, `op://` / `vault://` secret references, all-same-character fillers, +`process.env.*` / `os.environ[...]` references). + +Config keys (under `checks.security_rules.secrets`): + +```json +{ + "checks": { + "security": true, + "security_rules": { + "secrets": { + "enabled": true, + "allow_paths": ["testdata/**", "**/testdata/**"], + "allow_patterns": ["EXAMPLE"], + "custom_patterns": [ + { + "id": "security.acme-token", + "regex": "\\bacme_live_[0-9a-f]{16}\\b", + "message": "Acme live tokens must not be committed", + "level": "fail" + } + ], + "entropy": { + "enabled": false, + "min_length": 20, + "threshold": 4.5, + "level": "warn" + } + } + } + } +} +``` + +- `enabled` toggles the whole scan (default `true`). +- `allow_paths` are globs whose files are skipped (e.g. fixtures under `testdata/`). +- `allow_patterns` are regexes; a line matching any of them is never reported. +- `custom_patterns` add repo-specific credential formats; `level` defaults to `fail`. +- `entropy` enables the high-entropy heuristic (off by default); tune `min_length` + (default 20), `threshold` in bits/char (default 4.5), and `level` (default `warn`). + +Existing `exclude`, `waivers`, `baseline`, and inline `codeguard:ignore` suppressions +also apply to these findings. + +For performance and resistance to pathological/untrusted input, the scan skips binary +files and files larger than 5 MiB, and scans only the first 64 KiB of any single line. A +cheap literal pre-filter skips the per-pattern regexes on lines that contain no credential +marker, keeping a full-repo scan fast. + +### Git-history secret scan + +Working-tree and `-mode diff` scans only see the current state. A secret that was +committed and later removed is still leaked and must be **rotated**, not just deleted. +`codeguard scan-history` walks added lines across git history and reports any that match +the secret/credential patterns (using the same `secrets` config — allowlist, custom +patterns, entropy): + +```bash +codeguard scan-history # HEAD history, text output +codeguard scan-history -all -format json # every ref, machine-readable +codeguard scan-history -max-commits 500 # bound the walk on large repos +``` + +Findings are deduplicated by rule, path, and masked value, reporting the most recent +commit that introduced each. The command exits non-zero when any `fail`-level credential +is found, so it can gate CI. + Current behavior: - repository-wide secret and private-key scans apply regardless of target language - Go targets include insecure TLS review, shell execution review, and optional `govulncheck` diff --git a/docs/security.md b/docs/security.md index 6924015..ab4fbf9 100644 --- a/docs/security.md +++ b/docs/security.md @@ -90,6 +90,8 @@ taint engine and default to `fail`. | Rule | OWASP | Detects | | --- | --- | --- | +| `security.hardcoded-credential` | A07 | values matching known credential formats (AWS, GitHub, GitLab, Slack, Stripe, Google, npm, PyPI, Docker, SendGrid, Twilio, Azure, DB connection strings, Bearer tokens) or a configured custom pattern; **fail** | +| `security.high-entropy-string` | A07 | opt-in Shannon-entropy heuristic for unknown/random secrets; **warn** | | `security.cors-wildcard` | A05 | `Access-Control-Allow-Origin: *` | | `security.debug-enabled` | A05 | framework debug flag enabled (`debug=True`) | | `security.bind-all-interfaces` | A05 | binding to `0.0.0.0` | diff --git a/examples/codeguard.json b/examples/codeguard.json index 42dfea5..99a3a6f 100644 --- a/examples/codeguard.json +++ b/examples/codeguard.json @@ -94,6 +94,25 @@ "security_rules": { "govulncheck_mode": "auto", "govulncheck_command": "govulncheck", + "secrets": { + "enabled": true, + "allow_paths": ["testdata/**", "**/testdata/**"], + "allow_patterns": ["EXAMPLE"], + "entropy": { + "enabled": false, + "min_length": 20, + "threshold": 4.5, + "level": "warn" + }, + "custom_patterns": [ + { + "id": "security.acme-token", + "regex": "\\bacme_live_[0-9a-f]{16}\\b", + "message": "Acme live tokens must not be committed", + "level": "fail" + } + ] + }, "language_commands": { "typescript": [ { diff --git a/internal/cli/helpers.go b/internal/cli/helpers.go index d6ed0b9..323e335 100644 --- a/internal/cli/helpers.go +++ b/internal/cli/helpers.go @@ -32,6 +32,7 @@ Usage: codeguard validate [-config codeguard.yaml] [-profile startup|strict|enterprise|ai-safe] codeguard validate-patch [-config codeguard.yaml] [-format text|json|sarif|github] [-profile startup|strict|enterprise|ai-safe] [-ai] < patch.diff codeguard scan [-config codeguard.yaml] [-mode full|diff] [-base-ref main] [-format text|json|sarif|github] [-interactive] [-profile startup|strict|enterprise|ai-safe] [-ai] [-allow-config-commands] [-allow-config-ai-endpoints] + codeguard scan-history [-config codeguard.yaml] [-path .] [-max-commits N] [-all] [-format text|json] # scan git history for committed secrets codeguard fix [-config codeguard.yaml] [-mode full|diff] [-base-ref main] [-profile startup|strict|enterprise|ai-safe] [-rule rule.id] [-path rel/path] [-line N] -ai codeguard baseline [-config codeguard.yaml] [-output codeguard-baseline.json] [-mode full|diff] [-base-ref main] [-profile startup|strict|enterprise|ai-safe] codeguard report -slop-history [-config codeguard.yaml] [-limit N] [-profile startup|strict|enterprise|ai-safe] diff --git a/internal/cli/run.go b/internal/cli/run.go index bc14050..a9a17b3 100644 --- a/internal/cli/run.go +++ b/internal/cli/run.go @@ -20,6 +20,7 @@ var commandCatalog = map[string]commandRunner{ "report": withoutStdin(runReport), "rules": withoutStdin(runRules), "scan": runScan, + "scan-history": withoutStdin(runScanHistory), "serve": runServe, "validate": withoutStdin(runValidate), "validate-patch": runValidatePatch, diff --git a/internal/cli/scan_history.go b/internal/cli/scan_history.go new file mode 100644 index 0000000..4eb2f95 --- /dev/null +++ b/internal/cli/scan_history.go @@ -0,0 +1,83 @@ +package cli + +import ( + "context" + "encoding/json" + "flag" + "fmt" + "io" + "strings" + + service "github.com/devr-tools/codeguard/pkg/codeguard" +) + +func runScanHistory(args []string, stdout io.Writer, stderr io.Writer) int { + fs := flag.NewFlagSet("scan-history", flag.ContinueOnError) + fs.SetOutput(stderr) + configPath := fs.String("config", service.DefaultConfigPath(), "config file or directory path (for secret allowlist/custom patterns)") + repoPath := fs.String("path", ".", "repository path to scan") + maxCommits := fs.Int("max-commits", 0, "limit the number of commits scanned (0 = all)") + allRefs := fs.Bool("all", false, "scan all refs rather than just HEAD history") + format := fs.String("format", "text", "output format: text or json") + if err := fs.Parse(args); err != nil { + return 1 + } + + // Config is optional: it only supplies secret allowlist/custom-pattern/entropy + // settings. Fall back to defaults when it cannot be loaded. + cfg, err := loadConfigWithProfile(*configPath, "") + if err != nil { + cfg = service.ExampleConfig() + } + + report, err := service.ScanGitHistory(context.Background(), cfg, service.HistoryScanOptions{ + RepoPath: *repoPath, + MaxCommits: *maxCommits, + AllRefs: *allRefs, + }) + if err != nil { + _, _ = fmt.Fprintf(stderr, "scan-history failed: %v\n", err) + return 1 + } + + if err := writeHistoryReport(stdout, report, *format); err != nil { + _, _ = fmt.Fprintf(stderr, "scan-history output: %v\n", err) + return 1 + } + + for _, finding := range report.Findings { + if finding.Level == "fail" { + return 1 + } + } + return 0 +} + +func writeHistoryReport(stdout io.Writer, report service.HistoryReport, format string) error { + switch strings.TrimSpace(strings.ToLower(format)) { + case "json": + encoder := json.NewEncoder(stdout) + encoder.SetIndent("", " ") + return encoder.Encode(report) + case "", "text": + if len(report.Findings) == 0 { + _, _ = fmt.Fprintf(stdout, "No secrets found in %d commit(s) of history.\n", report.CommitsScanned) + return nil + } + _, _ = fmt.Fprintf(stdout, "%d secret(s) found in %d commit(s) of history:\n\n", len(report.Findings), report.CommitsScanned) + for _, finding := range report.Findings { + _, _ = fmt.Fprintf(stdout, " %s %s:%d [%s] %s\n %s\n", shortCommit(finding.Commit), finding.Path, finding.Line, strings.ToUpper(finding.Level), finding.RuleID, finding.Message) + } + _, _ = fmt.Fprintln(stdout, "\nRotate any real credentials: removing them from HEAD does not unleak history.") + return nil + default: + return fmt.Errorf("output format must be text or json") + } +} + +func shortCommit(commit string) string { + if len(commit) > 12 { + return commit[:12] + } + return commit +} diff --git a/internal/codeguard/checks/security/security.go b/internal/codeguard/checks/security/security.go index 3734a44..9b95181 100644 --- a/internal/codeguard/checks/security/security.go +++ b/internal/codeguard/checks/security/security.go @@ -16,6 +16,20 @@ func Run(ctx context.Context, env support.Context) core.SectionResult { func securityTargetFindings(ctx context.Context, env support.Context, target core.TargetConfig) []core.Finding { findings := make([]core.Finding, 0) + + // Hardcoded secret/credential detection is language-agnostic and runs for + // every target (including TypeScript/JavaScript, which otherwise bypass + // findingsForFile). Built once per target so allowlist/custom patterns are + // compiled a single time. + // Use a distinct cache section id ("security-secrets") so this pass does not + // collide with the per-file cache of the language pass below, which also + // scans the "security" section for the same files. + if scanner := BuildScanner(env.Config.Checks.SecurityRules.Secrets); scanner.Enabled() { + findings = append(findings, env.ScanTargetFiles(target, "security-secrets", func(string) bool { return true }, func(file string, data []byte) []core.Finding { + return secretFindingsForFile(env, file, data, scanner) + })...) + } + if isTypeScriptTarget(target) { findings = append(findings, typeScriptTargetFindings(ctx, env, target)...) } else { diff --git a/internal/codeguard/checks/security/security_common.go b/internal/codeguard/checks/security/security_common.go index 07b9fb6..fdc25bb 100644 --- a/internal/codeguard/checks/security/security_common.go +++ b/internal/codeguard/checks/security/security_common.go @@ -9,9 +9,10 @@ import ( "github.com/devr-tools/codeguard/internal/codeguard/core" ) +// Hardcoded secret/credential and private-key detection lives in +// security_secrets.go, which runs as a dedicated repository-wide pass so it also +// covers TypeScript/JavaScript targets. var ( - secretPattern = regexp.MustCompile(`(?i)(secret|token|api[_-]?key|password)\s*[:=]\s*["'][^"']{8,}["']`) - privateKeyPattern = regexp.MustCompile(`-----BEGIN [A-Z ]*PRIVATE KEY-----`) pythonShellPattern = regexp.MustCompile(`\bsubprocess\.(?:run|Popen|call|check_call|check_output|getoutput|getstatusoutput)\s*\(.*shell\s*=\s*True`) pythonSystemPattern = regexp.MustCompile(`\bos\.system\s*\(`) pythonEvalPattern = regexp.MustCompile(`\b(?:eval|exec)\s*\(`) @@ -55,10 +56,6 @@ func maskedSourceForFile(file string, source string) string { func appendCommonLineFindings(env support.Context, file string, lineNo int, line string) []core.Finding { switch { - case secretPattern.MatchString(line): - return []core.Finding{env.NewFinding(support.FindingInput{RuleID: "security.hardcoded-secret", Level: "fail", Path: file, Line: lineNo, Column: 1, Message: "possible hardcoded secret detected"})} - case privateKeyPattern.MatchString(line): - return []core.Finding{env.NewFinding(support.FindingInput{RuleID: "security.private-key", Level: "fail", Path: file, Line: lineNo, Column: 1, Message: "private key material detected"})} case strings.Contains(line, "InsecureSkipVerify: true"): return []core.Finding{env.NewFinding(support.FindingInput{RuleID: "security.insecure-tls", Level: "fail", Path: file, Line: lineNo, Column: 1, Message: "InsecureSkipVerify is enabled"})} case strings.Contains(line, "exec.Command(") || strings.Contains(line, "os/exec"): diff --git a/internal/codeguard/checks/security/security_secret_entropy.go b/internal/codeguard/checks/security/security_secret_entropy.go new file mode 100644 index 0000000..7aa63a4 --- /dev/null +++ b/internal/codeguard/checks/security/security_secret_entropy.go @@ -0,0 +1,176 @@ +package security + +import ( + "bytes" + "math" + "strings" + + "github.com/devr-tools/codeguard/internal/codeguard/core" +) + +type entropySettings struct { + enabled bool + minLength int + threshold float64 + level string +} + +const ( + defaultEntropyMinLength = 20 + defaultEntropyThreshold = 4.5 +) + +func buildEntropySettings(cfg *core.SecretsEntropyConfig) entropySettings { + settings := entropySettings{minLength: defaultEntropyMinLength, threshold: defaultEntropyThreshold, level: "warn"} + if cfg == nil { + return settings + } + if cfg.Enabled != nil { + settings.enabled = *cfg.Enabled + } + if cfg.MinLength > 0 { + settings.minLength = cfg.MinLength + } + if cfg.Threshold > 0 { + settings.threshold = cfg.Threshold + } + settings.level = normalizeSecretLevel(cfg.Level, "warn") + return settings +} + +func normalizeSecretLevel(level string, fallback string) string { + switch strings.TrimSpace(strings.ToLower(level)) { + case "warn": + return "warn" + case "fail": + return "fail" + default: + return fallback + } +} + +// entropyMatch reports the first high-entropy quoted literal on the line, if the +// (opt-in) entropy heuristic is configured to fire on it. +func (s Scanner) entropyMatch(lineNo int, line string) *Match { + for _, found := range quotedLiteralPattern.FindAllStringSubmatch(line, -1) { + value := found[1] + if len([]rune(value)) < s.entropy.minLength { + continue + } + if isPlaceholderSecret(value) { + continue + } + if shannonEntropy(value) < s.entropy.threshold { + continue + } + return &Match{RuleID: highEntropyRule, Level: s.entropy.level, Line: lineNo, Column: 1, Message: "high-entropy string literal (possible secret): " + maskSecret(value)} + } + return nil +} + +// credentialMatchValue returns the captured secret value when the pattern has a +// capture group, otherwise the whole match (a self-contained token). +func credentialMatchValue(match []string) string { + if len(match) > 1 && strings.TrimSpace(match[len(match)-1]) != "" { + return match[len(match)-1] + } + return match[0] +} + +func isPlaceholderSecret(value string) bool { + trimmed := strings.TrimSpace(value) + if trimmed == "" { + return true + } + if placeholderPattern.MatchString(trimmed) { + return true + } + lower := strings.ToLower(trimmed) + switch { + case strings.Contains(lower, "example"), + strings.Contains(lower, "redacted"), + strings.Contains(lower, "placeholder"), + strings.Contains(lower, "your_"), + strings.Contains(lower, "your-"), + strings.Contains(lower, "op://"), // 1Password secret reference, not a literal + strings.Contains(lower, "vault://"), // Vault secret reference + strings.HasPrefix(trimmed, "$("): // shell command substitution + return true + } + return allSameRune(trimmed) +} + +func allSameRune(value string) bool { + if len(value) < 2 { + return false + } + first := rune(value[0]) + for _, r := range value { + if r != first { + return false + } + } + return true +} + +// maskSecret renders a secret value for display without reprinting it in full, +// keeping enough context (first/last four characters) to locate it. +func maskSecret(value string) string { + trimmed := strings.TrimSpace(value) + runes := []rune(trimmed) + if len(runes) <= 8 { + return strings.Repeat("*", len(runes)) + } + return string(runes[:4]) + "…" + string(runes[len(runes)-4:]) +} + +// looksBinary reports whether data appears to be binary (contains a NUL byte in +// its leading window). Binary files are skipped: scanning them wastes time and +// produces noise rather than real credential findings. +func looksBinary(data []byte) bool { + limit := len(data) + if limit > binarySniffBytes { + limit = binarySniffBytes + } + return bytes.IndexByte(data[:limit], 0) >= 0 +} + +// shannonEntropy returns the Shannon entropy of s in bits per character. ASCII +// bytes are counted in a fixed array to avoid a per-call map allocation; the +// rare non-ASCII rune falls back to a map. +func shannonEntropy(s string) float64 { + if s == "" { + return 0 + } + var ascii [256]int + var wide map[rune]int + total := 0 + for _, r := range s { + total++ + if r < 256 { + ascii[r]++ + continue + } + if wide == nil { + wide = make(map[rune]int) + } + wide[r]++ + } + if total == 0 { + return 0 + } + ftotal := float64(total) + entropy := 0.0 + for _, count := range ascii { + if count == 0 { + continue + } + p := float64(count) / ftotal + entropy -= p * math.Log2(p) + } + for _, count := range wide { + p := float64(count) / ftotal + entropy -= p * math.Log2(p) + } + return entropy +} diff --git a/internal/codeguard/checks/security/security_secret_patterns.go b/internal/codeguard/checks/security/security_secret_patterns.go new file mode 100644 index 0000000..57d2a9e --- /dev/null +++ b/internal/codeguard/checks/security/security_secret_patterns.go @@ -0,0 +1,157 @@ +package security + +import ( + "regexp" + "strings" +) + +const ( + hardcodedCredentialRule = "security.hardcoded-credential" + hardcodedSecretRule = "security.hardcoded-secret" + privateKeyRule = "security.private-key" + highEntropyRule = "security.high-entropy-string" +) + +// secretPattern is the lower-confidence name-based heuristic: an assignment +// whose identifier looks secret-bearing next to a quoted value. It reports at +// warn. privateKeyPattern detects PEM key material and reports at fail. +var ( + secretPattern = regexp.MustCompile(`(?i)(secret|token|api[_-]?key|password)\s*[:=]\s*["']([^"']{8,})["']`) + privateKeyPattern = regexp.MustCompile(`-----BEGIN [A-Z ]*PRIVATE KEY-----`) + + // quotedLiteralPattern captures whitespace-free quoted literals for the + // entropy pass (secrets are contiguous, so requiring no whitespace skips prose). + quotedLiteralPattern = regexp.MustCompile("[\"'`]([^\"'`\\s]{12,})[\"'`]") +) + +type credentialPattern struct { + re *regexp.Regexp + msg string +} + +// credentialPatterns are high-confidence, provider-specific credential formats. +// They are matched against the raw line because the token lives inside a string +// literal that source masking would blank. Patterns with a capture group have +// their captured value placeholder-checked; the rest match a self-contained token. +// +// INVARIANT: every pattern's required literal must appear in gateLiterals or +// gateFoldLiterals below, or the pattern is silently skipped on most lines. +var credentialPatterns = []credentialPattern{ + {regexp.MustCompile(`\b(?:AKIA|ASIA)[0-9A-Z]{16}\b`), "AWS access key id"}, + {regexp.MustCompile(`\b(?:ghp|gho|ghu|ghs|ghr)_[0-9A-Za-z]{36}\b`), "GitHub access token"}, + {regexp.MustCompile(`\bgithub_pat_[0-9A-Za-z_]{22,}\b`), "GitHub fine-grained token"}, + {regexp.MustCompile(`\bglpat-[0-9A-Za-z_-]{20}\b`), "GitLab personal access token"}, + {regexp.MustCompile(`\bxox[baprs]-[0-9A-Za-z-]{10,}\b`), "Slack token"}, + {regexp.MustCompile(`https://hooks\.slack\.com/services/T[A-Z0-9]+/B[A-Z0-9]+/[A-Za-z0-9]{16,}`), "Slack webhook URL"}, + {regexp.MustCompile(`\b(?:sk|rk)_live_[0-9A-Za-z]{20,}\b`), "Stripe live secret key"}, + {regexp.MustCompile(`\bAIza[0-9A-Za-z_-]{35}\b`), "Google API key"}, + {regexp.MustCompile(`\bnpm_[0-9A-Za-z]{36}\b`), "npm access token"}, + {regexp.MustCompile(`\bSG\.[0-9A-Za-z_-]{22}\.[0-9A-Za-z_-]{43}\b`), "SendGrid API key"}, + {regexp.MustCompile(`\bSK[0-9a-f]{32}\b`), "Twilio API key"}, + {regexp.MustCompile(`\bpypi-[A-Za-z0-9_-]{16,}\b`), "PyPI API token"}, + {regexp.MustCompile(`\bdckr_pat_[A-Za-z0-9_-]{20,}\b`), "Docker Hub access token"}, + {regexp.MustCompile(`(?i)AccountKey=([A-Za-z0-9+/=]{40,})`), "Azure storage account key"}, + {regexp.MustCompile(`(?i)\b(?:postgres|postgresql|mysql|mongodb(?:\+srv)?|redis|amqp|amqps)://[^:@/\s]+:([^@/\s]+)@`), "database connection string with embedded credentials"}, + {regexp.MustCompile(`(?i)authorization["']?\s*[:=]\s*["']?bearer\s+([A-Za-z0-9._\-]{16,})`), "hardcoded bearer token"}, + {regexp.MustCompile(`(?i)(?:aws_secret_access_key|client_secret|private_token)\s*[:=]\s*["']([^"']{16,})["']`), "hardcoded credential assignment"}, +} + +// placeholderPattern recognizes obvious non-secret filler so fixtures and +// templates do not fail a scan: redacted/changeme/placeholder/dummy/fake/example +// words, your-... hints, all-filler runs, and interpolation/env references. +var placeholderPattern = regexp.MustCompile(`(?i)^(?:[x*.\-_0]+|redacted|changeme|placeholder|dummy|fake|example|your[-_].*|<.*>|\$\{.*\}|\$\(.*\)?|\{\{.*\}\}|process\.env\..*|os\.environ.*)$`) + +// gateLiterals are case-sensitive substrings that every built-in credential, +// private-key, or connection-string pattern requires. gateFoldLiterals are the +// case-insensitive markers for identifier-based patterns (always lowercase +// here). A line that contains none of these cannot match any built-in pattern, +// so the expensive per-pattern regexes are skipped. The gate is a cheap +// substring scan — `strings.Contains` is ~85x faster than the equivalent regex +// alternation, which dominated profiling. +var ( + gateLiterals = []string{ + "AKIA", "ASIA", "ghp_", "gho_", "ghu_", "ghs_", "ghr_", "github_pat_", + "glpat-", "xox", "hooks.slack.com", "_live_", "AIza", "npm_", "SG.", "SK", + "pypi-", "dckr_pat_", "PRIVATE KEY", "://", + } + gateFoldLiterals = []string{ + "secret", "token", "password", "apikey", "api_key", "api-key", "bearer", "accountkey", + } +) + +// matchPrivateKey, matchCredential, and matchNameBased are the built-in tiers, +// kept beside the patterns they apply. Each returns a position-agnostic *Match +// (scanLine stamps the line via located). +func matchPrivateKey(line string) *Match { + if privateKeyPattern.MatchString(line) { + return &Match{RuleID: privateKeyRule, Level: "fail", Message: "private key material detected"} + } + return nil +} + +func matchCredential(line string) *Match { + for _, pattern := range credentialPatterns { + match := pattern.re.FindStringSubmatch(line) + if match == nil { + continue + } + if len(match) > 1 && match[len(match)-1] != "" && isPlaceholderSecret(match[len(match)-1]) { + continue + } + return &Match{RuleID: hardcodedCredentialRule, Level: "fail", Message: "possible hardcoded credential detected (" + pattern.msg + "): " + maskSecret(credentialMatchValue(match))} + } + return nil +} + +func matchNameBased(line string) *Match { + match := secretPattern.FindStringSubmatch(line) + if match == nil || isPlaceholderSecret(match[len(match)-1]) { + return nil + } + return &Match{RuleID: hardcodedSecretRule, Level: "warn", Message: "possible hardcoded secret detected: " + maskSecret(match[len(match)-1])} +} + +// builtinGatePasses reports whether a line could match any built-in pattern. +// False positives only cost a wasted regex run, never correctness. +func builtinGatePasses(line string) bool { + for _, marker := range gateLiterals { + if strings.Contains(line, marker) { + return true + } + } + for _, marker := range gateFoldLiterals { + if asciiContainsFold(line, marker) { + return true + } + } + return false +} + +// asciiContainsFold reports whether s contains sub, case-insensitively for ASCII +// letters, without allocating (unlike strings.Contains(strings.ToLower(s), sub)). +// sub must be lowercase ASCII. +func asciiContainsFold(s string, sub string) bool { + n, m := len(s), len(sub) + if m == 0 { + return true + } + if m > n { + return false + } + for i := 0; i <= n-m; i++ { + j := 0 + for ; j < m; j++ { + c := s[i+j] + if c >= 'A' && c <= 'Z' { + c += 'a' - 'A' + } + if c != sub[j] { + break + } + } + if j == m { + return true + } + } + return false +} diff --git a/internal/codeguard/checks/security/security_secrets.go b/internal/codeguard/checks/security/security_secrets.go new file mode 100644 index 0000000..91fd77d --- /dev/null +++ b/internal/codeguard/checks/security/security_secrets.go @@ -0,0 +1,197 @@ +package security + +import ( + "regexp" + "strings" + + "github.com/devr-tools/codeguard/internal/codeguard/checks/support" + "github.com/devr-tools/codeguard/internal/codeguard/core" + runnersupport "github.com/devr-tools/codeguard/internal/codeguard/runner/support" +) + +// Bounds that keep the scan cheap and resistant to pathological (and untrusted) +// input such as minified bundles or deliberately oversized lines. codeguard runs +// on PR content it does not control, so these caps are a hardening measure as +// well as a performance one. +const ( + maxScanFileBytes = 5 << 20 // skip files larger than 5 MiB + maxScanLineBytes = 64 << 10 // scan at most the first 64 KiB of any line + binarySniffBytes = 8 << 10 // bytes inspected when detecting binary content +) + +// Match is a single secret/credential hit on a line. It is the unit shared by +// the in-tree finding pass and the git-history scan. +type Match struct { + RuleID string + Level string + Message string + Line int + Column int +} + +// Scanner holds the per-scan compiled allowlist, custom patterns, and entropy +// settings. Build it once with BuildScanner and reuse it across files/lines. +type Scanner struct { + enabled bool + allowPaths []string + allowRes []*regexp.Regexp + customPatterns []compiledCustomPattern + entropy entropySettings +} + +type compiledCustomPattern struct { + id string + re *regexp.Regexp + level string + msg string +} + +// Enabled reports whether the secret scan should run at all. +func (s Scanner) Enabled() bool { return s.enabled } + +// BuildScanner compiles a Scanner from config. A nil config yields the default +// enabled scanner with no allowlist, no custom patterns, and entropy disabled. +func BuildScanner(cfg *core.SecretsRulesConfig) Scanner { + scanner := Scanner{enabled: true} + if cfg == nil { + return scanner + } + if cfg.Enabled != nil { + scanner.enabled = *cfg.Enabled + } + scanner.allowPaths = append([]string(nil), cfg.AllowPaths...) + for _, pattern := range cfg.AllowPatterns { + if re, err := regexp.Compile(pattern); err == nil { + scanner.allowRes = append(scanner.allowRes, re) + } + } + for _, custom := range cfg.CustomPatterns { + re, err := regexp.Compile(custom.Regex) + if err != nil || strings.TrimSpace(custom.ID) == "" { + continue + } + level := normalizeSecretLevel(custom.Level, "fail") + message := strings.TrimSpace(custom.Message) + if message == "" { + message = "possible hardcoded credential detected (" + custom.ID + ")" + } + scanner.customPatterns = append(scanner.customPatterns, compiledCustomPattern{id: custom.ID, re: re, level: level, msg: message}) + } + scanner.entropy = buildEntropySettings(cfg.Entropy) + return scanner +} + +// SkipPath reports whether the file is covered by an allow_paths glob. +func (s Scanner) SkipPath(file string) bool { + for _, pattern := range s.allowPaths { + if runnersupport.MatchPattern(pattern, file) { + return true + } + } + return false +} + +func (s Scanner) lineAllowed(line string) bool { + for _, re := range s.allowRes { + if re.MatchString(line) { + return true + } + } + return false +} + +// ScanContent runs the secret/credential scan over file content and returns the +// matches with 1-based line numbers. Path allowlisting is the caller's +// responsibility (see SkipPath). +func (s Scanner) ScanContent(content string) []Match { + matches := make([]Match, 0) + lineNo := 0 + start := 0 + for i := 0; i <= len(content); i++ { + if i != len(content) && content[i] != '\n' { + continue + } + lineNo++ + line := strings.TrimSuffix(content[start:i], "\r") + start = i + 1 + if !s.lineAllowed(line) { + matches = append(matches, s.scanLine(lineNo, line)...) + } + } + return matches +} + +// scanLine reports at most one match per line, preferring the highest-confidence +// tier: PEM key material and known/custom credential formats fail; the name-based +// heuristic warns; the optional entropy pass is last. Overlong lines are scanned +// only up to maxScanLineBytes to bound worst-case cost on minified/oversized input. +func (s Scanner) scanLine(lineNo int, line string) []Match { + if len(line) > maxScanLineBytes { + line = line[:maxScanLineBytes] + } + // Cheap literal gate: when no built-in marker is present, the expensive + // per-pattern regexes are skipped. This gates built-ins even in entropy mode + // (the gate is a superset of what they can match), so entropy only adds its + // own literal pass. Custom patterns have arbitrary markers and always run. + runBuiltins := builtinGatePasses(line) + if runBuiltins { + if m := matchPrivateKey(line); m != nil { + return located(m, lineNo) + } + if m := matchCredential(line); m != nil { + return located(m, lineNo) + } + } + if m := s.matchCustom(line); m != nil { + return located(m, lineNo) + } + if runBuiltins { + if m := matchNameBased(line); m != nil { + return located(m, lineNo) + } + } + if s.entropy.enabled { + if m := s.entropyMatch(lineNo, line); m != nil { + return []Match{*m} + } + } + return nil +} + +// located stamps the line/column onto a match and wraps it as a single-element +// result, so the tier helpers can stay position-agnostic. +func located(m *Match, lineNo int) []Match { + m.Line = lineNo + m.Column = 1 + return []Match{*m} +} + +func (s Scanner) matchCustom(line string) *Match { + for _, pattern := range s.customPatterns { + if match := pattern.re.FindStringSubmatch(line); match != nil { + return &Match{RuleID: pattern.id, Level: pattern.level, Message: pattern.msg + ": " + maskSecret(credentialMatchValue(match))} + } + } + return nil +} + +// secretFindingsForFile runs the scan over a single file and converts matches to +// findings. It applies the path allowlist and skips binary/oversized files. +func secretFindingsForFile(env support.Context, file string, data []byte, scanner Scanner) []core.Finding { + if scanner.SkipPath(file) || len(data) > maxScanFileBytes || looksBinary(data) { + return nil + } + matches := scanner.ScanContent(string(data)) + findings := make([]core.Finding, 0, len(matches)) + for _, match := range matches { + findings = append(findings, env.NewFinding(support.FindingInput{ + RuleID: match.RuleID, + Level: match.Level, + Path: file, + Line: match.Line, + Column: match.Column, + Message: match.Message, + })) + } + return findings +} diff --git a/internal/codeguard/config/defaults_rules.go b/internal/codeguard/config/defaults_rules.go index 536585b..50ea61f 100644 --- a/internal/codeguard/config/defaults_rules.go +++ b/internal/codeguard/config/defaults_rules.go @@ -96,6 +96,12 @@ func applySecurityDefaults(dst *core.SecurityRulesConfig, def core.SecurityRules if dst.LanguageCommands == nil && len(def.LanguageCommands) > 0 { dst.LanguageCommands = cloneCommandCheckMap(def.LanguageCommands) } + if dst.Secrets == nil { + dst.Secrets = &core.SecretsRulesConfig{} + } + if dst.Secrets.Enabled == nil { + dst.Secrets.Enabled = boolPtr(true) + } } func applyAIChangeRiskDefaults(dst *core.AIChangeRiskConfig, def core.AIChangeRiskConfig) { diff --git a/internal/codeguard/config/example_rules.go b/internal/codeguard/config/example_rules.go index 207fac6..7e7c4e5 100644 --- a/internal/codeguard/config/example_rules.go +++ b/internal/codeguard/config/example_rules.go @@ -70,5 +70,9 @@ func exampleSecurityRules() core.SecurityRulesConfig { GovulncheckMode: "auto", GovulncheckCommand: "govulncheck", TypeScriptTaintMaxDepth: 8, + Secrets: &core.SecretsRulesConfig{ + Enabled: boolPtr(true), + AllowPaths: []string{"testdata/**", "**/testdata/**"}, + }, } } diff --git a/internal/codeguard/config/validate.go b/internal/codeguard/config/validate.go index ed72250..f28eb80 100644 --- a/internal/codeguard/config/validate.go +++ b/internal/codeguard/config/validate.go @@ -25,6 +25,7 @@ func Validate(cfg core.Config) error { validateContractRules(cfg.Checks.ContractRules), validateCoverageDelta(cfg.Checks.QualityRules.CoverageDelta), validateGraphThresholds(cfg.Checks.DesignRules), + validateSecretsRules(cfg.Checks.SecurityRules.Secrets), validateRulePacks(cfg.RulePacks), ) } diff --git a/internal/codeguard/config/validate_secrets.go b/internal/codeguard/config/validate_secrets.go new file mode 100644 index 0000000..8e1b845 --- /dev/null +++ b/internal/codeguard/config/validate_secrets.go @@ -0,0 +1,67 @@ +package config + +import ( + "fmt" + "regexp" + "strings" + + "github.com/devr-tools/codeguard/internal/codeguard/core" +) + +func validateSecretsRules(secrets *core.SecretsRulesConfig) error { + if secrets == nil { + return nil + } + for idx, pattern := range secrets.AllowPatterns { + if _, err := regexp.Compile(pattern); err != nil { + return fmt.Errorf("security_rules.secrets.allow_patterns[%d] invalid regex: %w", idx, err) + } + } + for _, custom := range secrets.CustomPatterns { + if err := validateSecretCustomPattern(custom); err != nil { + return err + } + } + return validateSecretEntropy(secrets.Entropy) +} + +func validateSecretCustomPattern(custom core.CustomSecretPattern) error { + if strings.TrimSpace(custom.ID) == "" { + return fmt.Errorf("security_rules.secrets.custom_patterns has an entry with an empty id") + } + if strings.TrimSpace(custom.Regex) == "" { + return fmt.Errorf("security_rules.secrets.custom_patterns[%q].regex is required", custom.ID) + } + if _, err := regexp.Compile(custom.Regex); err != nil { + return fmt.Errorf("security_rules.secrets.custom_patterns[%q] invalid regex: %w", custom.ID, err) + } + if !validSecretLevel(custom.Level) { + return fmt.Errorf("security_rules.secrets.custom_patterns[%q].level must be warn or fail", custom.ID) + } + return nil +} + +func validateSecretEntropy(entropy *core.SecretsEntropyConfig) error { + if entropy == nil { + return nil + } + if entropy.MinLength < 0 { + return fmt.Errorf("security_rules.secrets.entropy.min_length must not be negative, got %d", entropy.MinLength) + } + if entropy.Threshold < 0 { + return fmt.Errorf("security_rules.secrets.entropy.threshold must not be negative, got %g", entropy.Threshold) + } + if !validSecretLevel(entropy.Level) { + return fmt.Errorf("security_rules.secrets.entropy.level must be warn or fail") + } + return nil +} + +func validSecretLevel(level string) bool { + switch strings.TrimSpace(strings.ToLower(level)) { + case "", "warn", "fail": + return true + default: + return false + } +} diff --git a/internal/codeguard/core/config_rule_types.go b/internal/codeguard/core/config_rule_types.go index 89029d0..7c602a5 100644 --- a/internal/codeguard/core/config_rule_types.go +++ b/internal/codeguard/core/config_rule_types.go @@ -93,6 +93,7 @@ type SecurityRulesConfig struct { TaintPython *bool `json:"taint_python,omitempty" yaml:"taint_python,omitempty"` TypeScriptTaintMaxDepth int `json:"typescript_taint_max_depth,omitempty" yaml:"typescript_taint_max_depth,omitempty"` LanguageCommands map[string][]CommandCheckConfig `json:"language_commands,omitempty" yaml:"language_commands,omitempty"` + Secrets *SecretsRulesConfig `json:"secrets,omitempty" yaml:"secrets,omitempty"` } type SupplyChainRulesConfig struct { diff --git a/internal/codeguard/core/config_secrets_types.go b/internal/codeguard/core/config_secrets_types.go new file mode 100644 index 0000000..d5ed0ac --- /dev/null +++ b/internal/codeguard/core/config_secrets_types.go @@ -0,0 +1,39 @@ +package core + +// SecretsRulesConfig tunes the hardcoded secret/credential scan. The scan runs +// repository-wide across every target language and reports in both full and +// diff scans. It is enabled by default. +type SecretsRulesConfig struct { + // Enabled toggles the whole secret scan. Defaults to true when unset. + Enabled *bool `json:"enabled,omitempty" yaml:"enabled,omitempty"` + // AllowPaths are glob patterns (e.g. "testdata/**") whose files are skipped. + AllowPaths []string `json:"allow_paths,omitempty" yaml:"allow_paths,omitempty"` + // AllowPatterns are regexes; a line matching any of them is never reported. + AllowPatterns []string `json:"allow_patterns,omitempty" yaml:"allow_patterns,omitempty"` + // CustomPatterns add repo-specific high-confidence credential formats. + CustomPatterns []CustomSecretPattern `json:"custom_patterns,omitempty" yaml:"custom_patterns,omitempty"` + // Entropy enables the opt-in high-entropy string heuristic. + Entropy *SecretsEntropyConfig `json:"entropy,omitempty" yaml:"entropy,omitempty"` +} + +// SecretsEntropyConfig tunes the optional Shannon-entropy heuristic that catches +// unknown/random secrets matching no known format. It is disabled by default. +type SecretsEntropyConfig struct { + // Enabled toggles the entropy pass. Defaults to false when unset. + Enabled *bool `json:"enabled,omitempty" yaml:"enabled,omitempty"` + // MinLength is the minimum literal length considered. Defaults to 20. + MinLength int `json:"min_length,omitempty" yaml:"min_length,omitempty"` + // Threshold is the minimum Shannon entropy in bits/char to report. Defaults to 4.5. + Threshold float64 `json:"threshold,omitempty" yaml:"threshold,omitempty"` + // Level is the finding level (warn or fail). Defaults to warn. + Level string `json:"level,omitempty" yaml:"level,omitempty"` +} + +// CustomSecretPattern is a config-defined credential format. A match reports +// under the supplied id at the supplied level (defaults to "fail"). +type CustomSecretPattern struct { + ID string `json:"id" yaml:"id"` + Regex string `json:"regex" yaml:"regex"` + Message string `json:"message,omitempty" yaml:"message,omitempty"` + Level string `json:"level,omitempty" yaml:"level,omitempty"` +} diff --git a/internal/codeguard/history/history.go b/internal/codeguard/history/history.go new file mode 100644 index 0000000..be149ba --- /dev/null +++ b/internal/codeguard/history/history.go @@ -0,0 +1,178 @@ +// Package history scans a repository's git history for hardcoded secrets that +// were committed in the past. Working-tree and diff scans only see the current +// state, so a secret removed from HEAD but still present in history (and thus +// still leaked) is invisible to them. This pass walks added lines across commits +// and reports them so the credential can be rotated, not just deleted. +package history + +import ( + "bufio" + "context" + "fmt" + "io" + "os/exec" + "regexp" + "strconv" + "strings" + + "github.com/devr-tools/codeguard/internal/codeguard/checks/security" +) + +const commitMarker = "@@CG-COMMIT@@ " + +var hunkHeader = regexp.MustCompile(`^@@ -\d+(?:,\d+)? \+(\d+)`) + +// Options configures a history scan. +type Options struct { + RepoPath string + MaxCommits int // 0 scans all reachable commits + AllRefs bool // scan every ref rather than just HEAD + Scanner security.Scanner +} + +// Finding is a single secret detected at a path/line in a specific commit. +type Finding struct { + RuleID string `json:"rule_id"` + Level string `json:"level"` + Message string `json:"message"` + Path string `json:"path"` + Line int `json:"line"` + Commit string `json:"commit"` +} + +// Report is the result of a history scan. +type Report struct { + Findings []Finding `json:"findings"` + CommitsScanned int `json:"commits_scanned"` +} + +// Scan walks git history and returns deduplicated secret findings. Findings are +// deduplicated by rule, path, and masked value, keeping the most recent commit +// that introduced the value (git log is newest-first). +func Scan(ctx context.Context, opts Options) (Report, error) { + repo := strings.TrimSpace(opts.RepoPath) + if repo == "" { + repo = "." + } + + args := []string{"-C", repo, "log", "-p", "-U0", "--no-color", "--no-merges", "--format=" + commitMarker + "%H"} + if opts.AllRefs { + args = append(args, "--all") + } + if opts.MaxCommits > 0 { + args = append(args, fmt.Sprintf("-n%d", opts.MaxCommits)) + } + + cmd := exec.CommandContext(ctx, "git", args...) + stdout, err := cmd.StdoutPipe() + if err != nil { + return Report{}, err + } + if err := cmd.Start(); err != nil { + return Report{}, fmt.Errorf("git log: %w", err) + } + + report := parseLog(stdout, opts.Scanner) + + if err := cmd.Wait(); err != nil { + return Report{}, fmt.Errorf("git log: %w", err) + } + return report, nil +} + +// logParser holds the streaming state of a `git log -p` walk. +type logParser struct { + scanner security.Scanner + report Report + commit string + file string + newLine int + skipFile bool + seen map[string]struct{} + commits map[string]struct{} +} + +func parseLog(reader io.Reader, scanner security.Scanner) Report { + // bufio.Reader.ReadString grows for arbitrarily long lines, unlike + // bufio.Scanner, which silently stops on an over-long token — a dangerous + // failure mode for a security scan (it would skip the rest of history). + buf := bufio.NewReaderSize(reader, 64*1024) + parser := &logParser{ + scanner: scanner, + seen: make(map[string]struct{}), + commits: make(map[string]struct{}), + } + for { + raw, err := buf.ReadString('\n') + if len(raw) > 0 { + parser.handleLine(strings.TrimRight(raw, "\r\n")) + } + if err != nil { + break + } + } + parser.report.CommitsScanned = len(parser.commits) + return parser.report +} + +// handleLine advances parser state for one line of `git log -p -U0` output. +func (p *logParser) handleLine(line string) { + switch { + case strings.HasPrefix(line, commitMarker): + p.setCommit(strings.TrimSpace(strings.TrimPrefix(line, commitMarker))) + case strings.HasPrefix(line, "diff --git"): + p.file, p.skipFile = "", false + case strings.HasPrefix(line, "+++ "): + p.setFile(strings.TrimSpace(strings.TrimPrefix(line, "+++ "))) + case strings.HasPrefix(line, "@@"): + if m := hunkHeader.FindStringSubmatch(line); m != nil { + p.newLine, _ = strconv.Atoi(m[1]) + } + case strings.HasPrefix(line, "--- "), strings.HasPrefix(line, `\`), strings.HasPrefix(line, "-"): + // old-file header, "\ No newline" marker, and removed lines: no content, + // and they do not advance the new-file line counter. + case strings.HasPrefix(line, "+"): + p.recordAdded(strings.TrimPrefix(line, "+")) + p.newLine++ + default: + p.newLine++ // context line + } +} + +func (p *logParser) setCommit(commit string) { + p.commit = commit + if commit != "" { + p.commits[commit] = struct{}{} + } +} + +func (p *logParser) setFile(target string) { + if target == "/dev/null" { + p.file, p.skipFile = "", true + return + } + p.file = strings.TrimPrefix(target, "b/") + p.skipFile = p.scanner.SkipPath(p.file) +} + +// recordAdded scans one added line and appends any new (deduplicated) findings. +func (p *logParser) recordAdded(content string) { + if p.skipFile || p.file == "" { + return + } + for _, match := range p.scanner.ScanContent(content) { + key := match.RuleID + "|" + p.file + "|" + match.Message + if _, ok := p.seen[key]; ok { + continue + } + p.seen[key] = struct{}{} + p.report.Findings = append(p.report.Findings, Finding{ + RuleID: match.RuleID, + Level: match.Level, + Message: match.Message, + Path: p.file, + Line: p.newLine, + Commit: p.commit, + }) + } +} diff --git a/internal/codeguard/rules/catalog_security.go b/internal/codeguard/rules/catalog_security.go index 865fdc9..f8dffee 100644 --- a/internal/codeguard/rules/catalog_security.go +++ b/internal/codeguard/rules/catalog_security.go @@ -3,13 +3,31 @@ package rules import "github.com/devr-tools/codeguard/internal/codeguard/core" var securityCatalog = map[string]core.RuleMetadata{ + "security.hardcoded-credential": { + ID: "security.hardcoded-credential", + Section: "Security", + DefaultLevel: "fail", + ExecutionModel: core.RuleExecutionModelLanguageAgnostic, + Title: "Hardcoded credential", + Description: "Fails when a value matching a known credential format (AWS, GitHub, GitLab, Slack, Stripe, Google, npm, SendGrid, or a configured custom pattern) is found in source text.", + HowToFix: "Remove the credential, rotate it, and load it from a secret manager or environment at runtime.", + }, + "security.high-entropy-string": { + ID: "security.high-entropy-string", + Section: "Security", + DefaultLevel: "warn", + ExecutionModel: core.RuleExecutionModelLanguageAgnostic, + Title: "High-entropy string", + Description: "Warns on a high-entropy string literal that may be an unknown or random secret. Opt-in via security_rules.secrets.entropy.", + HowToFix: "If the value is a secret, remove it and load it from a secret manager or environment at runtime; otherwise add it to security_rules.secrets.allow_patterns.", + }, "security.hardcoded-secret": { ID: "security.hardcoded-secret", Section: "Security", - DefaultLevel: "fail", + DefaultLevel: "warn", ExecutionModel: core.RuleExecutionModelLanguageAgnostic, Title: "Hardcoded secret", - Description: "Fails when a likely hardcoded secret is detected in source text.", + Description: "Warns on the lower-confidence name-based heuristic: a secret/token/api_key/password identifier assigned a quoted literal.", HowToFix: "Remove the secret from the repository and load it from a secret manager or environment at runtime.", }, "security.private-key": { diff --git a/internal/codeguard/rules/catalog_security_owasp.go b/internal/codeguard/rules/catalog_security_owasp.go index 28bb3d4..99b52fc 100644 --- a/internal/codeguard/rules/catalog_security_owasp.go +++ b/internal/codeguard/rules/catalog_security_owasp.go @@ -8,8 +8,10 @@ import "github.com/devr-tools/codeguard/internal/codeguard/core" // intentionally omitted: their category cannot be known statically. var securityRuleOWASP = map[string]core.OWASPCategory{ // Secrets and key material. - "security.hardcoded-secret": core.OWASPA07AuthFailures, - "security.private-key": core.OWASPA02CryptographicFailures, + "security.hardcoded-credential": core.OWASPA07AuthFailures, + "security.hardcoded-secret": core.OWASPA07AuthFailures, + "security.high-entropy-string": core.OWASPA07AuthFailures, + "security.private-key": core.OWASPA02CryptographicFailures, // Transport security. "security.insecure-tls": core.OWASPA02CryptographicFailures, diff --git a/pkg/codeguard/sdk_history.go b/pkg/codeguard/sdk_history.go new file mode 100644 index 0000000..4ab1398 --- /dev/null +++ b/pkg/codeguard/sdk_history.go @@ -0,0 +1,35 @@ +package codeguard + +import ( + "context" + + "github.com/devr-tools/codeguard/internal/codeguard/checks/security" + "github.com/devr-tools/codeguard/internal/codeguard/history" +) + +// HistoryScanOptions configures a git-history secret scan. +type HistoryScanOptions struct { + RepoPath string + MaxCommits int + AllRefs bool +} + +// HistoryFinding is a secret detected in a past commit. +type HistoryFinding = history.Finding + +// HistoryReport is the result of ScanGitHistory. +type HistoryReport = history.Report + +// ScanGitHistory walks the repository's git history for hardcoded secrets and +// credentials using the supplied config's secret settings (allowlist, custom +// patterns, entropy). Findings that only exist in history still represent leaked +// credentials that must be rotated. +func ScanGitHistory(ctx context.Context, cfg Config, opts HistoryScanOptions) (HistoryReport, error) { + scanner := security.BuildScanner(cfg.Checks.SecurityRules.Secrets) + return history.Scan(ctx, history.Options{ + RepoPath: opts.RepoPath, + MaxCommits: opts.MaxCommits, + AllRefs: opts.AllRefs, + Scanner: scanner, + }) +} diff --git a/pkg/codeguard/sdk_types_state.go b/pkg/codeguard/sdk_types_state.go index 0a52e46..a13a16a 100644 --- a/pkg/codeguard/sdk_types_state.go +++ b/pkg/codeguard/sdk_types_state.go @@ -3,6 +3,9 @@ package codeguard import "github.com/devr-tools/codeguard/internal/codeguard/core" type SecurityRulesConfig = core.SecurityRulesConfig +type SecretsRulesConfig = core.SecretsRulesConfig +type SecretsEntropyConfig = core.SecretsEntropyConfig +type CustomSecretPattern = core.CustomSecretPattern type OutputConfig = core.OutputConfig type CacheConfig = core.CacheConfig type BaselineConfig = core.BaselineConfig diff --git a/tests/checks/security_history_test.go b/tests/checks/security_history_test.go new file mode 100644 index 0000000..a65e073 --- /dev/null +++ b/tests/checks/security_history_test.go @@ -0,0 +1,83 @@ +package checks_test + +import ( + "context" + "os/exec" + "path/filepath" + "testing" + + "github.com/devr-tools/codeguard/pkg/codeguard" +) + +func gitCommit(t *testing.T, dir string, args ...string) { + t.Helper() + cmd := exec.Command("git", append([]string{"-C", dir}, args...)...) + cmd.Env = append(cmd.Environ(), + "GIT_AUTHOR_NAME=test", "GIT_AUTHOR_EMAIL=test@example.com", + "GIT_COMMITTER_NAME=test", "GIT_COMMITTER_EMAIL=test@example.com", + ) + if out, err := cmd.CombinedOutput(); err != nil { + t.Fatalf("git %v: %v\n%s", args, err, out) + } +} + +func TestScanGitHistoryFindsRemovedSecret(t *testing.T) { + if _, err := exec.LookPath("git"); err != nil { + t.Skip("git not available") + } + dir := t.TempDir() + gitCommit(t, dir, "init", "-q") + + // Commit a secret, then remove it in a later commit. A working-tree scan of + // HEAD would miss it; the history scan must still find it. + writeFile(t, filepath.Join(dir, "config.go"), "package main\nconst awsKey = \""+cred("AKIA", "1234567890ABCDEF")+"\"\n") + gitCommit(t, dir, "add", ".") + gitCommit(t, dir, "commit", "-q", "-m", "add config") + + writeFile(t, filepath.Join(dir, "config.go"), "package main\n// key removed\n") + gitCommit(t, dir, "add", ".") + gitCommit(t, dir, "commit", "-q", "-m", "remove key") + + report, err := codeguard.ScanGitHistory(context.Background(), codeguard.ExampleConfig(), codeguard.HistoryScanOptions{RepoPath: dir}) + if err != nil { + t.Fatalf("scan history: %v", err) + } + + var found bool + for _, finding := range report.Findings { + if finding.RuleID == "security.hardcoded-credential" && finding.Path == "config.go" { + found = true + } + } + if !found { + t.Fatalf("expected hardcoded-credential in history, got %+v", report.Findings) + } + if report.CommitsScanned != 2 { + t.Fatalf("commits scanned = %d, want 2", report.CommitsScanned) + } +} + +func TestScanGitHistoryRespectsAllowPaths(t *testing.T) { + if _, err := exec.LookPath("git"); err != nil { + t.Skip("git not available") + } + dir := t.TempDir() + gitCommit(t, dir, "init", "-q") + writeFile(t, filepath.Join(dir, "testdata", "fixture.go"), "package fixture\nconst k = \""+cred("AKIA", "1234567890ABCDEF")+"\"\n") + gitCommit(t, dir, "add", ".") + gitCommit(t, dir, "commit", "-q", "-m", "fixture") + + cfg := codeguard.ExampleConfig() + cfg.Checks.SecurityRules.Secrets = &codeguard.SecretsRulesConfig{ + Enabled: boolPtr(true), + AllowPaths: []string{"testdata/**"}, + } + + report, err := codeguard.ScanGitHistory(context.Background(), cfg, codeguard.HistoryScanOptions{RepoPath: dir}) + if err != nil { + t.Fatalf("scan history: %v", err) + } + if len(report.Findings) != 0 { + t.Fatalf("expected no findings under allow_paths, got %+v", report.Findings) + } +} diff --git a/tests/checks/security_secrets_scanner_test.go b/tests/checks/security_secrets_scanner_test.go new file mode 100644 index 0000000..7faf52a --- /dev/null +++ b/tests/checks/security_secrets_scanner_test.go @@ -0,0 +1,84 @@ +package checks_test + +import ( + "testing" + + "github.com/devr-tools/codeguard/internal/codeguard/checks/security" + "github.com/devr-tools/codeguard/internal/codeguard/core" +) + +// gateSamples are one matching line per built-in pattern. They guard the +// invariant that the cheap literal gate is a superset of every built-in pattern: +// a pattern added without a corresponding gate marker would be skipped on most +// lines, and ScanContent here would return no match for its sample. +// gateSamples are one matching line per built-in pattern, assembled via cred so +// no full, contiguous secret literal is committed to source. They guard the +// invariant that the cheap literal gate is a superset of every built-in pattern: +// a pattern added without a corresponding gate marker would be skipped on most +// lines, and ScanContent here would return no match for its sample. +func gateSamples() []string { + return []string{ + `const k = "` + cred("AKIA", "1234567890ABCDEF") + `"`, + `const k = "` + cred("ghp_", "0123456789abcdefghijklmnopqrstuvwxyz") + `"`, + `const k = "` + cred("github_", "pat_0123456789abcdefghijkl") + `"`, + `const k = "` + cred("glpat-", "0123456789abcdefABCD") + `"`, + `const k = "` + cred("xox", "b-0123456789-abcdefABCDEF") + `"`, + `const u = "` + cred("https://hooks.slack.com/services/", "T01/B04/abcdefABCDEF0123456789") + `"`, + `const k = "` + cred("sk_", "live_0123456789abcdefABCDEFGH") + `"`, + `const k = "` + cred("AIza", "0123456789abcdefABCDEFGHIJKLMNOPQRS") + `"`, + `const k = "` + cred("npm_", "0123456789abcdefghijklmnopqrstuvwxyz") + `"`, + `const k = "` + cred("SG.", "0123456789abcdefghijkl.0123456789abcdefghijklmnopqrstuvwxyzABCDEFG") + `"`, + `const k = "` + cred("SK", "0123456789abcdef0123456789abcdef") + `"`, + `const k = "` + cred("pypi-", "AgEIcHlwaS5vcmcabcdef") + `"`, + `const k = "` + cred("dckr_", "pat_aBcDeFgHiJkLmNoPqRsT") + `"`, + `conn := "` + cred("AccountKey=", "0123456789abcdefABCDEF0123456789abcdefABCDEF0123456789") + `"`, + `db := "` + cred("postgres://admin:", "s3cr3tP4ssw0rd@db.host.net:5432/app") + `"`, + `h := "` + cred("Authorization: Bearer ", "abcdefghijklmnop0123456789") + `"`, + `client_secret = '` + cred("abcdefghij", "klmnop1234") + `'`, + `apiKey = "` + cred("supersecret", "value1234") + `"`, + `-----BEGIN RSA PRIVATE KEY-----`, + } +} + +func TestSecretScannerGateCoversBuiltins(t *testing.T) { + scanner := security.BuildScanner(nil) + for _, sample := range gateSamples() { + if len(scanner.ScanContent(sample)) == 0 { + t.Errorf("scanner missed a built-in sample (gate marker likely missing): %q", sample) + } + } +} + +func benchSource() string { + const block = "func handler(ctx context.Context, req *Request) (*Response, error) {\n" + + "\treturn &Response{Status: 200, Body: req.Payload}, nil\n" + out := make([]byte, 0, len(block)*1000+128) + for i := 0; i < 1000; i++ { + out = append(out, block...) + } + out = append(out, "const awsKey = \""+cred("AKIA", "1234567890ABCDEF")+"\"\n"...) + return string(out) +} + +func BenchmarkSecretScanContent(b *testing.B) { + scanner := security.BuildScanner(nil) + source := benchSource() + b.SetBytes(int64(len(source))) + b.ReportAllocs() + b.ResetTimer() + for i := 0; i < b.N; i++ { + _ = scanner.ScanContent(source) + } +} + +func BenchmarkSecretScanContentEntropy(b *testing.B) { + enabled := true + scanner := security.BuildScanner(&core.SecretsRulesConfig{Entropy: &core.SecretsEntropyConfig{Enabled: &enabled}}) + source := benchSource() + b.SetBytes(int64(len(source))) + b.ReportAllocs() + b.ResetTimer() + for i := 0; i < b.N; i++ { + _ = scanner.ScanContent(source) + } +} diff --git a/tests/checks/security_secrets_test.go b/tests/checks/security_secrets_test.go new file mode 100644 index 0000000..ae7a755 --- /dev/null +++ b/tests/checks/security_secrets_test.go @@ -0,0 +1,208 @@ +package checks_test + +import ( + "context" + "path/filepath" + "strings" + "testing" + + "github.com/devr-tools/codeguard/pkg/codeguard" +) + +func boolPtr(v bool) *bool { return &v } + +func secretsScanConfig(t *testing.T, dir string, secrets *codeguard.SecretsRulesConfig, language string) codeguard.Report { + t.Helper() + cfg := codeguard.ExampleConfig() + cfg.Name = "security-secrets" + cfg.Targets = []codeguard.TargetConfig{{Name: "repo", Path: dir, Language: language}} + cfg.Checks.Security = true + cfg.Checks.Design = false + cfg.Checks.Prompts = false + cfg.Checks.CI = false + cfg.Checks.Quality = false + cfg.Checks.SupplyChain = false + cfg.Checks.SecurityRules.GovulncheckMode = "off" + cfg.Checks.SecurityRules.Secrets = secrets + + report, err := codeguard.Run(context.Background(), cfg) + if err != nil { + t.Fatalf("run: %v", err) + } + return report +} + +func TestSecurityDetectsKnownCredentialFormats(t *testing.T) { + t.Parallel() + + cases := []struct { + name string + path string + language string + source string + }{ + {"aws", "config.go", "go", goConst(cred("AKIA", "1234567890ABCDEF"))}, + {"github", "config.go", "go", goConst(cred("ghp_", "0123456789abcdefghijklmnopqrstuvwxyz"))}, + {"gitlab", "config.go", "go", goConst(cred("glpat-", "0123456789abcdefABCD"))}, + {"slack", "config.go", "go", goConst(cred("xox", "b-0123456789-abcdefABCDEF"))}, + {"stripe", "config.go", "go", goConst(cred("sk_", "live_0123456789abcdefABCDEFGH"))}, + {"google", "config.go", "go", goConst(cred("AIza", "0123456789abcdefABCDEFGHIJKLMNOPQRS"))}, + {"npm", "config.go", "go", goConst(cred("npm_", "0123456789abcdefghijklmnopqrstuvwxyz"))}, + {"twilio", "config.go", "go", goConst(cred("SK", "0123456789abcdef0123456789abcdef"))}, + {"pypi", "config.go", "go", goConst(cred("pypi-", "AgEIcHlwaS5vcmcabcdef"))}, + {"docker", "config.go", "go", goConst(cred("dckr_", "pat_aBcDeFgHiJkLmNoPqRsT"))}, + {"slack_webhook", "config.go", "go", goConst(cred("https://hooks.slack.com/services/", "T01ABCD23/B04EFGH56/abcdefABCDEF0123456789"))}, + {"db_conn", "config.go", "go", goConst(cred("postgres://admin:", "s3cr3tP4ssw0rd@db.example.net:5432/app"))}, + {"bearer", "config.go", "go", goConst(cred("Authorization: Bearer ", "abcdefghijklmnop0123456789"))}, + {"assignment", "config.go", "go", "package main\nconst c = \"client_secret = '" + cred("abcdefghij", "klmnop1234") + "'\"\n"}, + // TypeScript target: proves the scan now covers TS, which bypasses findingsForFile. + {"typescript", "src/secret.ts", "typescript", "export const key = \"" + cred("AKIA", "1234567890ABCDEF") + "\";\n"}, + } + + for _, tc := range cases { + tc := tc + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + dir := t.TempDir() + writeFile(t, filepath.Join(dir, filepath.FromSlash(tc.path)), tc.source) + + report := secretsScanConfig(t, dir, nil, tc.language) + assertSectionStatus(t, report, "Security", "fail") + assertFindingRulePresent(t, report, "Security", "security.hardcoded-credential") + }) + } +} + +func TestSecuritySkipsPlaceholderValues(t *testing.T) { + t.Parallel() + dir := t.TempDir() + writeFile(t, filepath.Join(dir, "config.go"), "package main\n"+ + "const a = `api_key = \"your-api-key-here\"`\n"+ + "const b = `password = \"${ENV_TOKEN}\"`\n"+ + "const c = `secret = \"xxxxxxxxxxxx\"`\n"+ + "const d = `client_secret = \"example-client-secret-value\"`\n") + + report := secretsScanConfig(t, dir, nil, "go") + assertSectionStatus(t, report, "Security", "pass") +} + +func TestSecuritySecretsAllowPathsSkipsFixtures(t *testing.T) { + t.Parallel() + dir := t.TempDir() + writeFile(t, filepath.Join(dir, "testdata", "fixture.go"), "package fixture\nconst k = \""+cred("AKIA", "1234567890ABCDEF")+"\"\n") + + allowed := secretsScanConfig(t, dir, &codeguard.SecretsRulesConfig{ + Enabled: boolPtr(true), + AllowPaths: []string{"testdata/**"}, + }, "go") + assertSectionStatus(t, allowed, "Security", "pass") + + // Without the allowlist the same fixture fails. + blocked := secretsScanConfig(t, dir, nil, "go") + assertSectionStatus(t, blocked, "Security", "fail") +} + +func TestSecuritySecretsAllowPatternsSkipsLine(t *testing.T) { + t.Parallel() + dir := t.TempDir() + writeFile(t, filepath.Join(dir, "config.go"), "package main\nconst k = \""+cred("AKIA", "1234567890ABCDEF")+"\" // sample\n") + + report := secretsScanConfig(t, dir, &codeguard.SecretsRulesConfig{ + Enabled: boolPtr(true), + AllowPatterns: []string{`//\s*sample`}, + }, "go") + assertSectionStatus(t, report, "Security", "pass") +} + +func TestSecuritySecretsCustomPatternFails(t *testing.T) { + t.Parallel() + dir := t.TempDir() + writeFile(t, filepath.Join(dir, "config.go"), "package main\nconst k = \"acme_live_0123456789abcdef\"\n") + + report := secretsScanConfig(t, dir, &codeguard.SecretsRulesConfig{ + Enabled: boolPtr(true), + CustomPatterns: []codeguard.CustomSecretPattern{{ + ID: "security.acme-token", + Regex: `\bacme_live_[0-9a-f]{16}\b`, + Message: "Acme live token must not be committed", + Level: "fail", + }}, + }, "go") + assertSectionStatus(t, report, "Security", "fail") + assertFindingRulePresent(t, report, "Security", "security.acme-token") +} + +func TestSecurityEntropyDetectsUnknownSecret(t *testing.T) { + t.Parallel() + dir := t.TempDir() + writeFile(t, filepath.Join(dir, "config.go"), "package main\nconst blob = \"k7Jx9PqL2mNvB4wR8tZc3aYd5eHfUgQ1\"\n") + + // Off by default: a non-format, non-name-based literal passes. + off := secretsScanConfig(t, dir, nil, "go") + assertSectionStatus(t, off, "Security", "pass") + + // Enabled: the high-entropy literal is reported. + on := secretsScanConfig(t, dir, &codeguard.SecretsRulesConfig{ + Enabled: boolPtr(true), + Entropy: &codeguard.SecretsEntropyConfig{Enabled: boolPtr(true)}, + }, "go") + assertSectionStatus(t, on, "Security", "warn") + assertFindingRulePresent(t, on, "Security", "security.high-entropy-string") +} + +func TestSecurityCredentialFindingMasksValue(t *testing.T) { + t.Parallel() + dir := t.TempDir() + writeFile(t, filepath.Join(dir, "config.go"), "package main\nconst k = \""+cred("AKIA", "1234567890ABCDEF")+"\"\n") + + report := secretsScanConfig(t, dir, nil, "go") + for _, section := range report.Sections { + if section.Name != "Security" { + continue + } + for _, finding := range section.Findings { + if finding.RuleID != "security.hardcoded-credential" { + continue + } + if strings.Contains(finding.Message, "1234567890") { + t.Fatalf("finding message leaks the full secret: %q", finding.Message) + } + if !strings.Contains(finding.Message, "…") { + t.Fatalf("finding message not masked: %q", finding.Message) + } + return + } + } + t.Fatal("no hardcoded-credential finding found") +} + +func TestSecuritySkipsBinaryFiles(t *testing.T) { + t.Parallel() + dir := t.TempDir() + // A NUL byte marks the content as binary; the embedded credential must not be reported. + writeFile(t, filepath.Join(dir, "blob.bin"), ""+cred("AKIA", "1234567890ABCDEF")+"\x00\x01\x02binarydata") + + report := secretsScanConfig(t, dir, nil, "go") + assertSectionStatus(t, report, "Security", "pass") +} + +func TestSecurityScansLongLinePrefix(t *testing.T) { + t.Parallel() + dir := t.TempDir() + // A credential near the start of a very long (minified-style) line is still found. + long := "const k = \"" + cred("AKIA", "1234567890ABCDEF") + "\"; const pad = \"" + strings.Repeat("a", 200000) + "\"\n" + writeFile(t, filepath.Join(dir, "min.go"), "package main\n"+long) + + report := secretsScanConfig(t, dir, nil, "go") + assertSectionStatus(t, report, "Security", "fail") + assertFindingRulePresent(t, report, "Security", "security.hardcoded-credential") +} + +func TestSecuritySecretsDisabled(t *testing.T) { + t.Parallel() + dir := t.TempDir() + writeFile(t, filepath.Join(dir, "config.go"), "package main\nconst k = \""+cred("AKIA", "1234567890ABCDEF")+"\"\n") + + report := secretsScanConfig(t, dir, &codeguard.SecretsRulesConfig{Enabled: boolPtr(false)}, "go") + assertSectionStatus(t, report, "Security", "pass") +} diff --git a/tests/checks/security_test.go b/tests/checks/security_test.go index cdaf6f2..95dfbfa 100644 --- a/tests/checks/security_test.go +++ b/tests/checks/security_test.go @@ -9,9 +9,9 @@ import ( "github.com/devr-tools/codeguard/pkg/codeguard" ) -func TestSecurityCheckFailsForHardcodedSecret(t *testing.T) { +func TestSecurityCheckFailsForHardcodedCredential(t *testing.T) { dir := t.TempDir() - writeFile(t, filepath.Join(dir, "config.go"), "package main\nconst apiKey = \"super-secret-token\"\n") + writeFile(t, filepath.Join(dir, "config.go"), "package main\nconst awsKey = \""+cred("AKIA", "1234567890ABCDEF")+"\"\n") report, err := codeguard.Run(context.Background(), codeguard.Config{ Name: "security-test", @@ -30,6 +30,31 @@ func TestSecurityCheckFailsForHardcodedSecret(t *testing.T) { } assertSectionStatus(t, report, "Security", "fail") + assertFindingRulePresent(t, report, "Security", "security.hardcoded-credential") +} + +func TestSecurityCheckWarnsForNameBasedSecret(t *testing.T) { + dir := t.TempDir() + writeFile(t, filepath.Join(dir, "config.go"), "package main\nconst apiKey = \"super-secret-token\"\n") + + report, err := codeguard.Run(context.Background(), codeguard.Config{ + Name: "security-name-based-secret", + Targets: []codeguard.TargetConfig{{ + Name: "repo", + Path: dir, + Language: "go", + }}, + Checks: codeguard.CheckConfig{ + Security: true, + }, + Output: codeguard.OutputConfig{Format: "text"}, + }) + if err != nil { + t.Fatalf("run: %v", err) + } + + assertSectionStatus(t, report, "Security", "warn") + assertFindingRulePresent(t, report, "Security", "security.hardcoded-secret") } func TestSecurityCheckWarnsForShellExecution(t *testing.T) { diff --git a/tests/checks/test_helpers_test.go b/tests/checks/test_helpers_test.go index b193b85..e9499dd 100644 --- a/tests/checks/test_helpers_test.go +++ b/tests/checks/test_helpers_test.go @@ -10,6 +10,20 @@ import ( "github.com/devr-tools/codeguard/pkg/codeguard" ) +// cred assembles a credential-shaped test fixture at runtime by joining a +// provider prefix with its body. Keeping the two halves as separate source +// literals means no full, contiguous secret ever appears in committed code — +// which both trips GitHub push protection and is poor practice in a +// secret-detection test suite. The reconstructed value still exercises the +// scanner exactly as a real token would. +func cred(prefix string, body string) string { return prefix + body } + +// goConst wraps a value as a minimal Go source file assigning it to a constant, +// for use as a scanner fixture. +func goConst(value string) string { + return "package main\nconst k = \"" + value + "\"\n" +} + func writeFile(t *testing.T, path string, content string) { t.Helper() if err := os.MkdirAll(filepath.Dir(path), 0o755); err != nil { From 13b8287ee61a3e418479bc9c91111d6071e52887 Mon Sep 17 00:00:00 2001 From: Alex Wilkerson John Date: Tue, 30 Jun 2026 13:39:35 -0400 Subject: [PATCH 2/4] docs(knowledge): note test-location rule and runtime secret-fixture assembly Co-Authored-By: Claude Opus 4.8 (1M context) --- .claude/knowledge/testing-patterns.md | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/.claude/knowledge/testing-patterns.md b/.claude/knowledge/testing-patterns.md index e69f597..12fd812 100644 --- a/.claude/knowledge/testing-patterns.md +++ b/.claude/knowledge/testing-patterns.md @@ -4,6 +4,7 @@ Testing strategies, test infrastructure quirks, how to run/debug specific test s - The trust policy (`internal/codeguard/trust`) is a process-global. Tests that exercise command-driven checks or local (127.0.0.1) AI endpoints must enable it. `tests/checks` and `tests/codeguard` each have a `TestMain` (`trust_main_test.go`) that sets `Policy{AllowConfigCommands: true, AllowConfigAIEndpoints: true}`. If you add a new test package that runs config commands or hits a loopback test server, add a similar `TestMain` or the tests will fail with "refusing to run config-supplied command" / "ssrf guard" errors. - `tests/security` deliberately has NO trust `TestMain`: it verifies the secure *default*. Tests there set/restore the policy locally with `trust.Set(...)` + `t.Cleanup(trust.ResetFromEnv)`. Do not add a package-wide opt-in there. -- All tests live under `tests/` as external (`_test`) packages; there are no white-box tests inside `internal/`/`pkg/`. Internal packages are still importable from `tests/` because they are within the module. +- All tests live under `tests/` as external (`_test`) packages; there are no white-box tests inside `internal/`/`pkg/`. Internal packages are still importable from `tests/` because they are within the module. This is enforced by `ci.test-file-location` (the repo's own scan fails on `_test.go` files outside `tests/**`), so do not add tests next to the code even for unexported access — import the internal package from `tests/` and test via its exported API instead. +- **Never commit a contiguous real-format secret as a test fixture.** GitHub push protection rejects the push (it flags GitLab/Stripe/SendGrid/Twilio/etc. tokens even in `_test.go` files), and it is poor practice in a secret-detection repo. Assemble fixtures at runtime from a prefix + body via the `cred(prefix, body)` helper in `tests/checks/test_helpers_test.go` (e.g. `cred("AKIA", "1234567890ABCDEF")`), splitting the recognizable provider prefix from the rest so no full token literal appears in source. The reconstructed value still exercises the scanner identically. `goConst(value)` wraps a value as a minimal Go source fixture. - The MCP server smoke tests (`tests/mcp/`) drive the **real binary in a subprocess**, not the in-process handler, to keep tests external. Pattern: a `Test...HelperProcess` func gated by an env var (`GO_WANT_MCP_HELPER_PROCESS` for stdio, `GO_WANT_MCP_HTTP_HELPER_PROCESS` for HTTP) re-execs `os.Args[0]` and calls `cli.Run(...)`. stdio replays NDJSON transcripts over stdin; HTTP reserves a free `127.0.0.1:0` port, launches `serve --mcp --http`, polls `/healthz` for readiness, then issues real HTTP requests. Add new MCP behavior as a transcript + assertion (stdio) or a subtest in `http_test.go` (HTTP). - **Bidirectional (server→client) MCP tests** live in `tests/mcp/sampling_test.go`: the test acts as the MCP client, advertises `sampling`/`roots` at `initialize`, and answers the server's server-initiated requests. stdio uses interactive `StdinPipe`/`StdoutPipe` (not the replay harness). HTTP opens the `GET /mcp` SSE stream (waits for the `: ready` comment to avoid the attach race), reads the request off the stream, and POSTs the response with the matching `Mcp-Session-Id`. propose_fix verification is expected to fail on the throwaway diff — assert the round trip fired, not a verified patch. The HTTP helper passes `-config` via `CODEGUARD_TEST_HTTP_CONFIG`. From 52de05aec540115392936e6b2caacf91fa4ee466 Mon Sep 17 00:00:00 2001 From: Alex Wilkerson John Date: Tue, 30 Jun 2026 13:48:46 -0400 Subject: [PATCH 3/4] fix(security): anchor Slack webhook regex to satisfy CodeQL Bound the hooks.slack.com webhook pattern with start/non-host-char on both sides (matching the repo's existing fix for go/regex/missing-regexp-anchor in quality_ai_style_drift.go) so an arbitrary host cannot precede or follow the match. The captured group is the webhook URL; detection behavior is unchanged. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../codeguard/checks/security/security_secret_patterns.go | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/internal/codeguard/checks/security/security_secret_patterns.go b/internal/codeguard/checks/security/security_secret_patterns.go index 57d2a9e..dbcce3a 100644 --- a/internal/codeguard/checks/security/security_secret_patterns.go +++ b/internal/codeguard/checks/security/security_secret_patterns.go @@ -42,7 +42,10 @@ var credentialPatterns = []credentialPattern{ {regexp.MustCompile(`\bgithub_pat_[0-9A-Za-z_]{22,}\b`), "GitHub fine-grained token"}, {regexp.MustCompile(`\bglpat-[0-9A-Za-z_-]{20}\b`), "GitLab personal access token"}, {regexp.MustCompile(`\bxox[baprs]-[0-9A-Za-z-]{10,}\b`), "Slack token"}, - {regexp.MustCompile(`https://hooks\.slack\.com/services/T[A-Z0-9]+/B[A-Z0-9]+/[A-Za-z0-9]{16,}`), "Slack webhook URL"}, + // Bounded with start/non-host-char on both sides so an arbitrary host cannot + // precede or follow the match (satisfies CodeQL go/regex/missing-regexp-anchor); + // the captured group is the webhook URL itself. + {regexp.MustCompile(`(?:^|[^A-Za-z0-9_])(https://hooks\.slack\.com/services/T[A-Z0-9]+/B[A-Z0-9]+/[A-Za-z0-9]{16,})(?:$|[^A-Za-z0-9_])`), "Slack webhook URL"}, {regexp.MustCompile(`\b(?:sk|rk)_live_[0-9A-Za-z]{20,}\b`), "Stripe live secret key"}, {regexp.MustCompile(`\bAIza[0-9A-Za-z_-]{35}\b`), "Google API key"}, {regexp.MustCompile(`\bnpm_[0-9A-Za-z]{36}\b`), "npm access token"}, From 077d2fff21ee33ed41224aab20922a0bdd6ff040 Mon Sep 17 00:00:00 2001 From: Alex Wilkerson John Date: Tue, 30 Jun 2026 13:49:15 -0400 Subject: [PATCH 4/4] docs(knowledge): note CodeQL regex-anchor rule and accepted fix Co-Authored-By: Claude Opus 4.8 (1M context) --- .claude/knowledge/architecture-boundaries.md | 1 + 1 file changed, 1 insertion(+) diff --git a/.claude/knowledge/architecture-boundaries.md b/.claude/knowledge/architecture-boundaries.md index ee9de90..310ed1f 100644 --- a/.claude/knowledge/architecture-boundaries.md +++ b/.claude/knowledge/architecture-boundaries.md @@ -10,6 +10,7 @@ Key architectural decisions, service boundaries, data flow, integration points, - The language-agnostic OWASP-gap line rules (CORS, debug, bind-all, weak-hash/cipher, deserialization, Dockerfile root) live in `checks/security/security_owasp_extra.go` and run per-line via `findingsForFile`. NOTE: `findingsForFile` is the **non-TypeScript** path — `securityTargetFindings` routes TS/JS targets to a separate `typeScriptTargetFindings` pipeline, so these rules do not run on TS/JS targets. String-literal signals (CORS, bind-all) match the raw line; call/identifier signals match the masked line. - **Secret/credential scanning (`checks/security/security_secrets.go`) runs as its own repository-wide pass in `securityTargetFindings`, unconditionally for BOTH the TS and non-TS branches** — precisely so it covers TS/JS targets that bypass `findingsForFile`. Tiers: `security.hardcoded-credential` (fail; known provider formats + `security_rules.secrets.custom_patterns`), `security.private-key` (fail), `security.hardcoded-secret` (warn; lower-confidence name-based heuristic — was fail before, deliberately downgraded). Patterns match the **raw** line (the token lives in a string literal that masking would blank). Placeholder/allowlist filtering is done in Go after the regex match because RE2 has no lookahead (mirrors `isInsecureDeserialization`). Config: `security_rules.secrets` (`enabled`/`allow_paths`/`allow_patterns`/`custom_patterns`), defaulted in `applySecurityDefaults`, validated in `validateSecretsRules`. - **PERF: the secret scanner is gated by a cheap literal pre-check (`builtinGatePasses`), not a combined regex.** Profiling showed `strings.Contains`/byte scans are ~85x faster than RE2 alternation, and that a leading `\b` or `(?i)` defeats RE2's literal-prefix fast path (those patterns ran ~1.8–7ms per 2000 lines; literal-prefix patterns like the Slack webhook ran ~50µs). The gate is `gateLiterals` (case-sensitive `strings.Contains`) + `gateFoldLiterals` (lowercase, via alloc-free `asciiContainsFold`); when a line contains no marker, the ~17 built-in regexes are skipped. Net: default scan went 3.2→99 MB/s, entropy mode 3.1→47 MB/s. **INVARIANT: the gate must be a superset of every built-in pattern — adding a credential pattern requires adding its required literal to the gate.** `TestBuiltinGateCoversPatterns` enforces this; custom config patterns have arbitrary markers so they bypass the gate and always run. +- **CodeQL `go/regex/missing-regexp-anchor` (High) fires on unanchored regexes containing a host literal (e.g. `hooks\.slack\.com`), even for detection patterns.** The repo's accepted fix (see `quality_ai_style_drift.go:17` and the Slack webhook pattern in `security_secret_patterns.go`) is to bound the host with `(?:^|[^A-Za-z0-9_])(...)(?:$|[^A-Za-z0-9_])` — start/non-host-char on both sides — which adds the capture group as the matched value. CodeQL accepts this form; a bare `\b` or `(?:^|...)` left-only boundary may not. Apply this whenever adding a regex with an embedded domain literal. - **HARDENING (codeguard scans untrusted PR content): the scanner bounds its work** — skips files > `maxScanFileBytes` (5 MiB), scans only the first `maxScanLineBytes` (64 KiB) of any line, and skips binary files (`looksBinary` = NUL byte in first 8 KiB). The git-history parser uses `bufio.Reader.ReadString` (grows for any line length), NOT `bufio.Scanner` — Scanner silently stops on an over-long token, which for a security scan would skip the rest of history undetected. - **The secret scanner exposes a pure API for reuse: `security.BuildScanner(cfg) Scanner` + `Scanner.ScanContent(content) []Match` + `Scanner.SkipPath(file)`.** `secretFindingsForFile` wraps `ScanContent` with `env.NewFinding`. Tiers in `scanLine` (one match per line, priority order): private-key → `credentialPatterns` (known formats + DB/Bearer/Azure, fail) → config `custom_patterns` → name-based (warn) → opt-in entropy (`security.high-entropy-string`, warn). Values are masked in messages via `maskSecret` (first4…last4). Entropy = `shannonEntropy` (bits/char) over whitespace-free quoted literals, gated by `secrets.entropy` (off by default; min_length 20, threshold 4.5). GitHub-token CRC32 checksum validation was deliberately NOT added — getting the algorithm slightly wrong yields false negatives (worse than a false positive for a security tool). - **Git-history secret scan lives in its own top-level package `internal/codeguard/history` (shells `git log -p -U0`), exposed via `pkg/codeguard.ScanGitHistory` and the `scan-history` CLI command.** It MUST be a separate package, not `runner/support`: `checks/security` imports `runner/support` (for `MatchPattern`), so `runner/support` importing `checks/security` would cycle. `history` imports `checks/security` (above it) and is consumed by `pkg` + CLI (both top-level). The parser tracks `+++ b/`, hunk headers (`@@ … +c`), and `+`/`-` line prefixes to attribute added lines to commits; dedupes by rule+path+masked-message keeping the newest commit (log is newest-first).