Skip to content

fix: MISSING_TLS false positive on secure wss:// transports#18

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

fix: MISSING_TLS false positive on secure wss:// transports#18
dmchaledev wants to merge 1 commit into
mainfrom
claude/amazing-franklin-i23vxf

Conversation

@dmchaledev

Copy link
Copy Markdown
Contributor

Summary

A HIGH-severity false positive: a fully secure wss:// MCP server is flagged as MISSING_TLS"Missing TLS — Server Uses Plain HTTP" — claiming it "uses plain HTTP (not HTTPS)", which is factually wrong. wss:// is a TLS-encrypted WebSocket. For a tool sold as a CI/CD security gate, this fails the gate on a properly-secured server and erodes trust in the scanner's output.

Repro (before this PR)

// wss-config.json — TLS WebSocket, authenticated, rate-limited
{
  "transport": {
    "url": "wss://secure.example.com/mcp",
    "auth": { "type": "bearer", "token": "abcdefghijklmnopqrstuvwxyz0123456789abcd" }
  },
  "rateLimit": { "enabled": true, "requestsPerMinute": 100 }
}
$ node dist/cli.js wss-config.json --format=table
HIGH | MISSING_TLS | Missing TLS — Server Uses Plain HTTP
Found 1 finding(s): 0 critical, 1 high, 0 medium, 0 low      # false positive (score 15)

Root cause

src/rules/auth-rules.ts (MISSING_TLS) fires for any URL that neither starts with https:// nor sets transport.tls:

if (url && (url.startsWith('http://') || (!isTls && !url.startsWith('https://')))) { ... }

parseConfig sets transport.tls = serverUrl.startsWith('https://'), so for a wss:// URL tls is false. The (!isTls && !startsWith('https://')) clause then matches wss:// (and ws://), even though this rule is specifically about the HTTP family ("Server Uses Plain HTTP"). WebSocket security is already owned by INSECURE_TRANSPORT (which correctly flags only ws://), so ws:// was also being double-counted.

Fix

Scope MISSING_TLS to the HTTP family by excluding ws:// and wss://. ws:// remains covered by INSECURE_TRANSPORT; wss:// is already encrypted. Plain http:// and scheme-less-without-tls cases still fire, so no real finding is lost.

Verification

$ node dist/cli.js wss-config.json --format=table   →  0 findings, PASSED        ✓ (was HIGH false positive)
$ node dist/cli.js http-config.json --format=table  →  MISSING_TLS               ✓ still flagged
$ node dist/cli.js ws-config.json --format=table    →  INSECURE_TRANSPORT only   ✓ no MISSING_TLS double-count
  • npm run typecheck ✓ · npm run lint (0 warnings) ✓
  • Added two regression tests to the MISSING_TLS block (scanner.test.ts) for the wss:// and ws:// cases. Verified passing in isolation (5/5, including a check that scheme-less-without-tls still fires).

Note on CI

CI on main is currently red for a pre-existing, unrelated reason — the src/sarif.ts import.meta/createRequire compile error under ts-jest's CommonJS transform (introduced in #9, fixed by open PR #12). That breaks the scanner and sarif suites' compilation regardless of this change. This PR adds no new failures; the new tests pass once #12 lands. Typecheck and lint are green here.

https://claude.ai/code/session_01AS5hiMB3scrZaMFfRZhiHp


Generated by Claude Code

The MISSING_TLS rule flagged any URL that neither started with https://
nor set transport.tls. parseConfig leaves transport.tls=false for
non-https schemes, so a fully secure wss:// WebSocket endpoint was
reported as HIGH 'Missing TLS - Server Uses Plain HTTP' - a factually
wrong false positive that fails the CI gate on a properly-secured server
(and double-counts ws:// alongside INSECURE_TRANSPORT).

Scope the rule to the HTTP family by excluding ws:// and wss:// URLs.
ws:// remains covered by INSECURE_TRANSPORT; wss:// is already encrypted.
Plain http:// and scheme-less-without-tls cases still fire.

Adds regression tests for the wss:// and ws:// cases.
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.

2 participants