diff --git a/CHANGELOG.md b/CHANGELOG.md index 34c673f..7173a21 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,12 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] ### Fixed +- `--exit-code` no longer fails on the informational `URL_SCAN_LIMITED` note. + Scanning a secure `https://`/`wss://` endpoint with `--exit-code` printed + `PASSED` but still exited `1`, because the INFO note counted as a "finding" — + failing CI on every secure endpoint and contradicting the note's documented + "never fails a gate" contract. The exit-code gate now ignores INFO-severity + findings and trips only on actionable (LOW+) ones. - URL/endpoint mode no longer produces false findings (#27). Scanning a URL previously ran every config rule against an effectively empty config, so any endpoint always emitted a CRITICAL `NO_AUTH` and a MEDIUM `MISSING_RATE_LIMIT` diff --git a/src/__tests__/exit.test.ts b/src/__tests__/exit.test.ts new file mode 100644 index 0000000..87505b1 --- /dev/null +++ b/src/__tests__/exit.test.ts @@ -0,0 +1,69 @@ +import { shouldExitNonZero } from '../exit'; +import { Finding, RuleId, SecurityReport, Severity } from '../types'; + +function makeFinding(severity: Severity, ruleId: RuleId = RuleId.NO_AUTH): Finding { + return { + ruleId, + severity, + title: 'Test', + description: 'Test finding', + remediation: 'Fix it', + }; +} + +function makeReport(findings: Finding[], passed: boolean): SecurityReport { + return { + findings, + score: 0, + passed, + summary: { critical: 0, high: 0, medium: 0, low: 0, info: 0 }, + scannedAt: '2026-01-01T00:00:00.000Z', + durationMs: 1, + }; +} + +describe('shouldExitNonZero', () => { + it('exits non-zero when the scan did not pass, regardless of --exit-code', () => { + const report = makeReport([makeFinding(Severity.CRITICAL)], false); + expect(shouldExitNonZero(report, false)).toBe(true); + expect(shouldExitNonZero(report, true)).toBe(true); + }); + + it('exits zero on a passing scan without --exit-code', () => { + const report = makeReport([makeFinding(Severity.LOW)], true); + expect(shouldExitNonZero(report, false)).toBe(false); + }); + + it('exits non-zero with --exit-code when an actionable finding is present', () => { + const report = makeReport([makeFinding(Severity.LOW)], true); + expect(shouldExitNonZero(report, true)).toBe(true); + }); + + // Regression: URL mode emits a URL_SCAN_LIMITED INFO note for every scan. + // It is documented as "never fails a gate", so --exit-code on a secure + // https:// / wss:// endpoint (whose only finding is that note) must exit 0. + it('does not exit non-zero with --exit-code when the only finding is an INFO note', () => { + const report = makeReport( + [makeFinding(Severity.INFO, RuleId.URL_SCAN_LIMITED)], + true + ); + expect(shouldExitNonZero(report, true)).toBe(false); + }); + + it('exits non-zero with --exit-code when an actionable finding accompanies an INFO note', () => { + const report = makeReport( + [ + makeFinding(Severity.INFO, RuleId.URL_SCAN_LIMITED), + makeFinding(Severity.MEDIUM, RuleId.MISSING_RATE_LIMIT), + ], + true + ); + expect(shouldExitNonZero(report, true)).toBe(true); + }); + + it('exits zero with no findings and no --exit-code', () => { + const report = makeReport([], true); + expect(shouldExitNonZero(report, false)).toBe(false); + expect(shouldExitNonZero(report, true)).toBe(false); + }); +}); diff --git a/src/cli.ts b/src/cli.ts index 86283dd..3119c41 100644 --- a/src/cli.ts +++ b/src/cli.ts @@ -18,6 +18,7 @@ import { scan } from './scanner.js'; import { toSarif } from './sarif.js'; import { Severity, Finding, RuleId } from './types.js'; import { parseArgs } from './args.js'; +import { shouldExitNonZero } from './exit.js'; /** Selectable rule IDs (excludes the URL_SCAN_LIMITED informational note). */ const VALID_RULE_IDS = Object.values(RuleId).filter( @@ -136,11 +137,7 @@ async function main(): Promise { printTable(report.findings, report.passed); } - const shouldFail = - !report.passed || - (exitCode && report.findings.length > 0); - - if (shouldFail) { + if (shouldExitNonZero(report, exitCode)) { process.exit(1); } } catch (err) { diff --git a/src/exit.ts b/src/exit.ts new file mode 100644 index 0000000..08d57ea --- /dev/null +++ b/src/exit.ts @@ -0,0 +1,30 @@ +/** + * Pure CLI exit-status logic for mcp-security-scanner. + * + * Kept free of side effects (no process.exit / console calls) so it can be + * unit-tested directly, separate from the executable entry point in cli.ts. + */ + +import { SecurityReport, Severity } from './types.js'; + +/** + * Decide whether the CLI should exit with a non-zero status. + * + * The gate fails when either: + * - the scan did not pass (`report.passed` already accounts for the score, + * critical findings, and any `--fail-on` threshold), or + * - `--exit-code` was given and there is at least one *actionable* finding. + * + * Informational notes (INFO severity — e.g. the `URL_SCAN_LIMITED` note emitted + * for every URL scan) are not vulnerabilities and must never trip the gate. + * Counting them as "findings" would fail CI on every secure `https://`/`wss://` + * endpoint, directly contradicting the report's own `passed: true`. + */ +export function shouldExitNonZero( + report: SecurityReport, + exitCodeFlag: boolean +): boolean { + if (!report.passed) return true; + if (!exitCodeFlag) return false; + return report.findings.some((f) => f.severity !== Severity.INFO); +}