Skip to content

Unknown rule ids in --only / --disable / severityOverrides are silently ignored #6

Description

@royalpinto007

What is wanted

Fail loudly on unknown rule ids passed to --only, --disable, severityOverrides, and disabledRules.

The problem

Rule id filters are matched by plain string comparison and never validated against the catalog.

In src/engine/engine.ts, activeRules (lines 149-157):

if (enabledRules && enabledRules.length > 0) {
  return enabledRules.includes(rule.id);
}
return !disabledRules.includes(rule.id);

A typo in --only therefore produces an empty rule set, a clean report, and exit code 0. A typo in --disable produces a full run and the user believes a rule was suppressed when it was not. Same for severityOverrides in a .mcpauditrc, which normalizeConfig (src/config.ts lines 85-93) validates the severity value of but never the rule id key.

The one thing already validated is --fail-on, in overlayFlags (src/cli.ts lines 89-97), which throws a helpful message listing the valid severities. This issue is asking for the same treatment for rule ids.

Concretely:

mcp-audit static fixtures/manifests/http-no-auth.json --only MCP41

MCP041 is the real id. Today this exits 0 with zero findings and no complaint, on a manifest that the CI self-scan job in .github/workflows/ci.yml uses precisely because it is not clean.

Why it matters

A scanner that silently reports "nothing found" because of a typo is worse than one that errors, and this failure mode is invisible in CI: the job goes green.

Suggested approach

  1. src/rules/index.ts already exports getRule(id) and ALL_RULES. Use them.
  2. Add a validation step, ideally in runAudit (src/audit.ts lines 35-48) so it covers config-file and CLI paths alike, or in overlayFlags plus loadConfig if you prefer to keep runAudit pure. State which you chose in the PR.
  3. The error message should follow the --fail-on precedent: name the unknown id and point at mcp-audit rules. A "did you mean" suggestion is a nice extra but not required.
  4. Consider whether an unknown id in a config file should be a hard error or a warning to stderr. A hard error is more consistent with assertSeverity in src/config.ts, which throws.
  5. Add cases to test/engine.test.ts or test/config.test.ts covering an unknown id in --only, in --disable, and in severityOverrides.

Comment below if you want to pick this up. I usually reply within a day.

Metadata

Metadata

Assignees

No one assigned

    Labels

    enhancementNew feature or requesthelp wantedExtra attention is needed

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions