From d460fb0a7d651a5e0703d2e473770de15b7f2652 Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 6 Jul 2026 21:07:12 +0000 Subject: [PATCH] fix(EXPOSED_SECRETS): redact detected secrets in finding evidence MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The secret-scanning rule printed each matched credential verbatim in the finding's `evidence` field (truncated only past 60 chars). That evidence flows into the scan report's stdout/JSON output and any artifact or CI log derived from it, so the scanner re-exposed the very secrets it exists to flag — an ironic and real leak for a tool sold as a CI/CD security gate. Report matches by type and length only (e.g. "OpenAI API key (29 chars, redacted)"), never the value. Secret patterns are now named to drive the type label. Adds EXPOSED_SECRETS tests (previously uncovered) asserting the rule fires and that no raw credential appears anywhere in the finding. Co-Authored-By: Claude Opus 4.8 Claude-Session: https://claude.ai/code/session_01HVdyf7Ehsa8z1kBq7LX5NA --- CHANGELOG.md | 8 ++++++ src/__tests__/scanner.test.ts | 53 +++++++++++++++++++++++++++++++++++ src/rules/runtime-rules.ts | 27 ++++++++++++------ 3 files changed, 80 insertions(+), 8 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 34c673f..0f1f1d6 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,14 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +### Security +- `EXPOSED_SECRETS` no longer echoes detected secrets in its own output. The + rule's `evidence` previously included the matched credential verbatim (only + truncated past 60 chars), so a scan report — stdout/JSON, and any artifact or + CI log derived from it — re-exposed the very secrets it was flagging. Findings + now report each match by type and length only (e.g. `OpenAI API key (29 chars, + redacted)`), never the value. + ### Fixed - URL/endpoint mode no longer produces false findings (#27). Scanning a URL previously ran every config rule against an effectively empty config, so any diff --git a/src/__tests__/scanner.test.ts b/src/__tests__/scanner.test.ts index dd8e44a..12f6334 100644 --- a/src/__tests__/scanner.test.ts +++ b/src/__tests__/scanner.test.ts @@ -535,6 +535,59 @@ describe('DEBUG_MODE_ENABLED rule', () => { }); }); +// ─── EXPOSED_SECRETS rule ───────────────────────────────────────────────────── + +describe('EXPOSED_SECRETS rule', () => { + const SECRETS = { + openai: 'sk-abcdefghijklmnop1234567890', + aws: 'AKIAIOSFODNN7EXAMPLE', + github: 'ghp_abcdefghijklmnopqrstuvwxyz0123456789', + }; + + it('fires when a config string contains a recognizable secret', () => { + const config: ParsedMcpConfig = { rawStrings: [SECRETS.openai] }; + const findings = runRuntimeRule(RuleId.EXPOSED_SECRETS, config); + expect(findings).toHaveLength(1); + expect(findings[0].ruleId).toBe(RuleId.EXPOSED_SECRETS); + expect(findings[0].severity).toBe(Severity.CRITICAL); + }); + + it('never echoes the detected secret value in the finding (redacted evidence)', () => { + const config: ParsedMcpConfig = { + rawStrings: [SECRETS.openai, SECRETS.aws, SECRETS.github], + }; + const findings = runRuntimeRule(RuleId.EXPOSED_SECRETS, config); + expect(findings).toHaveLength(1); + + // The whole finding — every field — must be free of the raw credentials, + // since it flows into stdout/JSON/SARIF/CI logs where it would be re-exposed. + const serialized = JSON.stringify(findings[0]); + for (const secret of Object.values(SECRETS)) { + expect(serialized).not.toContain(secret); + } + + // Evidence should still identify what was found: type, count, and length. + expect(findings[0].evidence).toContain('OpenAI API key'); + expect(findings[0].evidence).toContain('AWS access key ID'); + expect(findings[0].evidence).toContain('GitHub token'); + expect(findings[0].evidence).toMatch(/redacted/i); + }); + + it('does not fire when no strings match a secret pattern', () => { + const config: ParsedMcpConfig = { + rawStrings: ['https://example.com', 'bearer', 'a normal description'], + }; + const findings = runRuntimeRule(RuleId.EXPOSED_SECRETS, config); + expect(findings).toHaveLength(0); + }); + + it('does not fire when rawStrings is absent', () => { + const config: ParsedMcpConfig = {}; + const findings = runRuntimeRule(RuleId.EXPOSED_SECRETS, config); + expect(findings).toHaveLength(0); + }); +}); + // ─── INSECURE_TRANSPORT rule ────────────────────────────────────────────────── describe('INSECURE_TRANSPORT rule', () => { diff --git a/src/rules/runtime-rules.ts b/src/rules/runtime-rules.ts index 0d0c3fd..cbd6ec2 100644 --- a/src/rules/runtime-rules.ts +++ b/src/rules/runtime-rules.ts @@ -2,11 +2,15 @@ import { Finding, RuleId, Severity } from '../types.js'; import { ParsedMcpConfig } from '../parser.js'; import { Rule } from './index.js'; -const SECRET_PATTERNS: RegExp[] = [ - /sk-[a-zA-Z0-9]{20,}/i, // OpenAI API key - /ghp_[a-zA-Z0-9]{36}/i, // GitHub PAT - /AKIA[0-9A-Z]{16}/i, // AWS access key - /[Pp]assword\s*[=:]\s*\S{8,}/, // Password assignment +/** + * Named secret detectors. Each carries a human-readable `name` so a match can + * be reported by *type* rather than by echoing the credential itself. + */ +const SECRET_PATTERNS: Array<{ name: string; pattern: RegExp }> = [ + { name: 'OpenAI API key', pattern: /sk-[a-zA-Z0-9]{20,}/i }, + { name: 'GitHub token', pattern: /ghp_[a-zA-Z0-9]{36}/i }, + { name: 'AWS access key ID', pattern: /AKIA[0-9A-Z]{16}/i }, + { name: 'hardcoded password', pattern: /[Pp]assword\s*[=:]\s*\S{8,}/ }, ]; export const runtimeRules: Rule[] = [ @@ -96,12 +100,16 @@ export const runtimeRules: Rule[] = [ title: 'Potential Secret Exposed in Configuration', check(config: ParsedMcpConfig): Finding[] { const rawStrings = config.rawStrings ?? []; + // Report matches by *type and length only*. This scanner exists to keep + // secrets out of plaintext config, so it must never echo a detected + // credential back into its own report (stdout, JSON, SARIF, CI logs), + // where it would be re-exposed and potentially stored or indexed. const matched: string[] = []; for (const s of rawStrings) { - for (const pattern of SECRET_PATTERNS) { + for (const { name, pattern } of SECRET_PATTERNS) { if (pattern.test(s)) { - matched.push(s.length > 60 ? s.slice(0, 57) + '...' : s); + matched.push(`${name} (${s.length} chars, redacted)`); break; } } @@ -117,7 +125,10 @@ export const runtimeRules: Rule[] = [ 'The configuration file appears to contain one or more hardcoded secrets ' + '(API keys, tokens, or passwords). Secrets committed to config files can be ' + 'extracted by anyone with access to the file.', - evidence: `Matched value(s): ${matched.slice(0, 3).join('; ')}`, + evidence: + `Detected ${matched.length} secret(s) — values redacted: ` + + matched.slice(0, 3).join('; ') + + (matched.length > 3 ? `; +${matched.length - 3} more` : ''), remediation: 'Remove secrets from configuration files. Use environment variables, a secrets manager ' + '(e.g., AWS Secrets Manager, HashiCorp Vault), or a .env file excluded from version control.',