Skip to content

fix(cli): validate --rule and reject unknown rule IDs (closes #11)#17

Open
dmchaledev wants to merge 1 commit into
mainfrom
claude/amazing-franklin-5025so
Open

fix(cli): validate --rule and reject unknown rule IDs (closes #11)#17
dmchaledev wants to merge 1 commit into
mainfrom
claude/amazing-franklin-5025so

Conversation

@dmchaledev

Copy link
Copy Markdown
Contributor

Summary

Fixes issue #11 — a false-green security gate. The CLI validated --format and --fail-on but not --rule: an unknown or mistyped rule ID was cast straight to RuleId and passed to scan(). Because the scanner filters the rule set down to the requested IDs (src/scanner.ts:33-35), a value that matches no real rule filtered it to nothing → zero findings, perfect score, passed: true, exit 0.

For a tool advertised as a CI/CD security gate, a single typo silently turned the gate green:

# Intended: gate on the no-auth rule
mcp-security-scanner --rule=NOAUTH --exit-code ./mcp-config.json   # typo: should be NO_AUTH
# Before: 0 findings, PASSED, exit 0  -> security check silently bypassed

Fix

  • Validate --rule against the RuleId enum; exit 2 with a clear error on an unknown value, mirroring the existing --format / --fail-on handling.
  • List the valid rule IDs in --help so they're discoverable.
  • Extract argument parsing into a side-effect-free src/args.ts (no console output, no process.exit, no top-level execution) returning a discriminated ParseResult. cli.ts now consumes it and owns all I/O and exit codes. This makes the whole arg layer unit-testable — previously impossible because cli.ts calls main() at import time.

Behavior is otherwise unchanged: same flags, same error messages, same exit codes.

Testing

$ node dist/cli.js examples/vulnerable-config.json --rule=NOAUTH --exit-code
Error: Unknown rule "NOAUTH". Use --help to see valid rule IDs.   # exit 2

$ node dist/cli.js examples/vulnerable-config.json --rule=NO_AUTH --format=table
CRITICAL | NO_AUTH | No Authentication Configured ... FAILED       # exit 1
  • npm run build ✓ · npm run lint (0 warnings) ✓
  • New src/__tests__/args.test.ts16 tests covering the unknown-rule regression plus format / fail-on / target / help parsing. The test imports only ../args (not ../sarif), so it compiles and passes under the current Jest config.

Note on CI

CI on main is currently red for a pre-existing, unrelated reason — the src/sarif.ts import.meta compile error under ts-jest's CommonJS transform (introduced in #9, fixed by open PR #12). That breaks the scanner and sarif suites regardless of this change. This PR adds no new failures, and the new args suite passes in isolation. Merging #12 first turns the full suite green.

Closes #11

https://claude.ai/code/session_016JMDkNCXgutHpTsKiCnD6v


Generated by Claude Code

An unknown or mistyped --rule value (e.g. NOAUTH instead of NO_AUTH) was
cast straight to RuleId and passed to scan(). Because the scanner filters
the rule set to the requested IDs, a value matching no real rule filtered
it down to nothing: the scan produced zero findings, a perfect score,
passed:true, and exit code 0 — silently turning a CI security gate green.

Validate each --rule value against the RuleId enum and exit 2 on an
unknown value, mirroring how --format and --fail-on already behave. Also
list the valid rule IDs in --help so they are discoverable.

Argument parsing is extracted into a side-effect-free src/args.ts module
(no console output, no process.exit, no top-level execution) so it can be
unit-tested directly; cli.ts now consumes a discriminated ParseResult and
owns all I/O and exit codes. Adds src/__tests__/args.test.ts covering the
unknown-rule regression plus format/fail-on/target/help parsing.

Closes #11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

CLI silently passes when --rule is given an unknown/typo'd rule ID (false "PASSED" security gate)

2 participants