fix(security): close CodeQL alert #11 (incomplete URL substring sanitization in sample mock-fetch)#100
Merged
Merged
Conversation
…le mock-fetch CodeQL alert #11 (HIGH severity, js/incomplete-url-substring-sanitization) on samples/portal-host/src/mock-fetch.ts:117. The previous shape was: if (!url.startsWith(BASE)) return originalFetch(input, init); with BASE = 'https://hooks.example.com'. The startsWith check correctly catches paths like https://hooks.example.com/api/v1/portal/endpoints, but it ALSO matches https://hooks.example.com.attacker.com/api/v1/portal/endpoints — the BASE prefix can be followed by an arbitrary host suffix. This is fine for the sample's mock semantics in isolation (the worst case is the mock answers a request that should have gone to the real network), but the kind of pattern that absolutely should not get copy-pasted into production code. Fixed by parsing the URL and comparing protocol + host explicitly via WHATWG URL. Same intent, no substring trap. Also flipped path = url.slice(BASE.length).split('?')[0] to path = parsed.pathname which is the same value via a safer route. The route handlers downstream are unchanged. Closes the CodeQL alert. No test changes required — the mock continues to serve the same routes for the same inputs the sample emits.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Closes CodeQL alert #11 (HIGH severity,
js/incomplete-url-substring-sanitization) onsamples/portal-host/src/mock-fetch.ts:117.The previous shape:
with
BASE = "https://hooks.example.com". ThestartsWithcheck correctly catches paths likehttps://hooks.example.com/api/v1/portal/endpoints, but it ALSO matcheshttps://hooks.example.com.attacker.com/api/v1/portal/endpoints— the BASE prefix can be followed by an arbitrary host suffix. Fine for the sample's mock semantics in isolation (the worst case is the mock answers a request that should have gone to the real network), but the kind of pattern that absolutely should not get copy-pasted into production code.Fix
Parse the URL and compare protocol + host explicitly via WHATWG URL:
Same intent, no substring trap. Also flipped
path = url.slice(BASE.length).split("?")[0]→path = parsed.pathnamewhich is the same value via a safer route. Route handlers downstream are unchanged.Test plan