Skip to content

test(security): exact SAN-entry match in mitm leaf-cert test (CodeQL #746) - #7824

Merged
diegosouzapw merged 1 commit into
release/v3.8.49from
test/codeql-746-san-exact-match
Jul 20, 2026
Merged

diegosouzapw merged 1 commit into
release/v3.8.49from
test/codeql-746-san-exact-match

Conversation

@diegosouzapw

Copy link
Copy Markdown
Collaborator

Why this matters right now

#746 is the ONLY open CodeQL alert in the repo, and check:codeql-ratchet counts alerts repo-wide — so it single-handedly keeps Quality Ratchet red on every open PR. Right now that is blocking ~10 contributor PRs (#7816, #7813, #7810, #7804, #7807, #7358, #7296, #6958, #7027, #7299) whose only failing checks are this base-red — they have no defect of their own.

The alert is live on the exact tip (5e96d5254), has no open PR covering it, and came in with #7731 (already merged). This clears it at the source.

Root cause

tests/unit/mitm-root-ca-leaf-issuance-6684.test.ts:37 asserted the leaf cert's SAN with a host substring check:

cert.subjectAltName && cert.subjectAltName.includes(host)

CodeQL flags this as js/incomplete-url-substring-sanitization (HIGH) — and it is genuinely loose: a SAN of notexample.com satisfies host = "example.com".

Fix

Exact SAN-entry membership instead of substring:

const sanEntries = (cert.subjectAltName ?? "").split(",").map((entry) => entry.trim());
assert.ok(
  sanEntries.includes(`DNS:${host}`),
  `expected SAN to include DNS:${host}, got ${cert.subjectAltName}`
);

Stronger, not weaker — it now verifies the full DNS:<host> entry rather than any coincidental substring, and it is no longer a substring-of-host sink. allHosts() comes from MITM_TOOL_HOSTS (DNS names, no IPs), so the DNS: prefix is correct.

Validation (Hard Rule #18)

node --import tsx/esm --test tests/unit/mitm-root-ca-leaf-issuance-6684.test.ts
→ 5 pass, 0 fail

Test-only; no production code touched. File exists only on release/v3.8.49 — no main companion needed (merge-gates §8). Once merged + rescanned, repo CodeQL count hits 0 and Quality Ratchet goes green across the whole open-PR queue.

…746)

CodeQL js/incomplete-url-substring-sanitization (HIGH) flags
cert.subjectAltName.includes(host) in the #6684 leaf-issuance test as a
host-substring check. It is the ONLY open CodeQL alert repo-wide, and
check:codeql-ratchet counts alerts repo-wide, so it keeps Quality Ratchet
red on every open PR — currently blocking ~10 contributor PRs that have no
defect of their own.

Assert exact SAN-entry membership (split on ',' + Array.includes of the
full 'DNS:<host>' entry) instead of a substring. Stronger: a SAN of
'notexample.com' no longer satisfies host 'example.com'. All 5 tests pass.
@gemini-code-assist

Copy link
Copy Markdown
Contributor

Warning

You have reached your daily quota limit. Please wait up to 24 hours and I will start processing your requests again!

@diegosouzapw

Copy link
Copy Markdown
Collaborator Author

Verified independently: gh api .../code-scanning/alerts?state=open confirms alert #746 (js/incomplete-url-substring-sanitization) at this exact file/line is the ONLY open CodeQL alert in the repo, and config/quality/quality-baseline.json confirms metrics.codeqlAlerts.value=0 — so this one alert is indeed failing the repo-wide Quality Ratchet gate on every open PR, exactly as described.

Ran node --import tsx/esm --test tests/unit/mitm-root-ca-leaf-issuance-6684.test.ts on the PR head in an isolated worktree: 5/5 pass. The new assertion is strictly stronger than the one it replaces (exact DNS:<host> SAN-entry membership vs. a host substring), matching the established pattern already used for the same CodeQL rule elsewhere in this repo (#4386, #5130, #4356).

All 6 red checks on this PR (Docs Gates, Fast Quality Gates, all 4 Unit Tests fast-path shards, dast-smoke) were individually traced and are pre-existing base-red, unrelated to the touched file: Fast Quality Gates fails on the known TICK_MS is not defined regression in tokenHealthCheck.ts (fixed by #7830 in this same batch), Docs Gates and part of the Unit Tests shards fail on a pre-existing check:env-doc-sync drift, and the remaining shard failures (mitm-tool-hosts sync, a rate-limit timing flake, etc.) don't touch this file either.

merge-ready — no mandatory items.

@diegosouzapw

Copy link
Copy Markdown
Collaborator Author

Validated in local merge-train on tomni-proxmox-113 @ 19bcf534352e24cdd2448cdfdfd64d5bd9695c74 (FAST gates green: static + changed tests + vitest)

@diegosouzapw
diegosouzapw merged commit a7a6b5d into release/v3.8.49 Jul 20, 2026
9 of 10 checks passed
@diegosouzapw
diegosouzapw deleted the test/codeql-746-san-exact-match branch July 23, 2026 00:24
HouMinXi pushed a commit to HouMinXi/OmniRoute that referenced this pull request Aug 2, 2026
…heaper-inference#746) (cheaper-inference#7824)

CodeQL js/incomplete-url-substring-sanitization (HIGH) flags
cert.subjectAltName.includes(host) in the cheaper-inference#6684 leaf-issuance test as a
host-substring check. It is the ONLY open CodeQL alert repo-wide, and
check:codeql-ratchet counts alerts repo-wide, so it keeps Quality Ratchet
red on every open PR — currently blocking ~10 contributor PRs that have no
defect of their own.

Assert exact SAN-entry membership (split on ',' + Array.includes of the
full 'DNS:<host>' entry) instead of a substring. Stronger: a SAN of
'notexample.com' no longer satisfies host 'example.com'. All 5 tests pass.
muhamadgalihsaputra pushed a commit to niyatna/NiyatnaRoute that referenced this pull request Sep 27, 2026
…heaper-inference#746) (cheaper-inference#7824)

CodeQL js/incomplete-url-substring-sanitization (HIGH) flags
cert.subjectAltName.includes(host) in the cheaper-inference#6684 leaf-issuance test as a
host-substring check. It is the ONLY open CodeQL alert repo-wide, and
check:codeql-ratchet counts alerts repo-wide, so it keeps Quality Ratchet
red on every open PR — currently blocking ~10 contributor PRs that have no
defect of their own.

Assert exact SAN-entry membership (split on ',' + Array.includes of the
full 'DNS:<host>' entry) instead of a substring. Stronger: a SAN of
'notexample.com' no longer satisfies host 'example.com'. All 5 tests pass.
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.

1 participant