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.',