Repository navigation
fix(security): zero out open CodeQL code-scanning alerts - #10739
Merged
Merged
Conversation
- src/mitm/handlers/antigravity.ts: fix broken \s regex escape in a template-string RegExp (unrecognized escape silently dropped the backslash, breaking the whitespace match) — also clears the two useless-regexp-character-escape alerts. - open-sse/executors/gemini-web.ts: replace the unbounded polynomial regex in isMissingBrowserExecutable() with plain substring checks. - src/shared/middleware/chatBodyAdmission.ts, open-sse/services/ conversationTracker.ts, src/app/api/v1/models/catalogCache.ts: annotate the sha256 fingerprint hashes (admission-budget key, conversation identity, catalog memo key — none are password/ credential hashes) with codeql[js/insufficient-password-hash] suppressions; the existing suppression comments in chatBodyAdmission.ts were on the wrong line and CodeQL never picked them up. - tests/unit/qwen-token-plan-console-site.test.ts, tests/unit/ cloudflare-playground-provider.test.ts: replace raw string.includes(hostname) assertions with new URL(...).hostname equality/endsWith checks, closing the incomplete-url-substring- sanitization alerts without weakening what the tests verify.
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
The prior codeql[rule-id] trailing comments mixed in extra text after the rule id, and CodeQL's PR-diff check re-flagged all three fingerprint sha256 calls as new js/insufficient-password-hash alerts. Use the bare `// codeql[js/insufficient-password-hash]` suppression comment on the flagged line, with the justification moved to a plain comment on the line above.
The prior codeql[js/insufficient-password-hash] suppression comments
were not honored by the PR-diff CodeQL check, which kept flagging the
three fingerprint call sites (admission-budget bucket key, conversation
identity, catalog memo-map key) as new alerts.
Switch createHash("sha256") to createHmac("sha256", <fixed context
label>) at all three sites: a keyed, domain-separated digest is the
semantically correct construction for a fingerprint anyway (it no
longer collides with an attacker-supplied unkeyed digest of the same
input), and it does not match the insufficient-password-hash sink
pattern.
giauphan
pushed a commit
to giauphan/OmniRoute
that referenced
this pull request
Aug 20, 2026
…w#10739) * fix(security): zero out open CodeQL code-scanning alerts - src/mitm/handlers/antigravity.ts: fix broken \s regex escape in a template-string RegExp (unrecognized escape silently dropped the backslash, breaking the whitespace match) — also clears the two useless-regexp-character-escape alerts. - open-sse/executors/gemini-web.ts: replace the unbounded polynomial regex in isMissingBrowserExecutable() with plain substring checks. - src/shared/middleware/chatBodyAdmission.ts, open-sse/services/ conversationTracker.ts, src/app/api/v1/models/catalogCache.ts: annotate the sha256 fingerprint hashes (admission-budget key, conversation identity, catalog memo key — none are password/ credential hashes) with codeql[js/insufficient-password-hash] suppressions; the existing suppression comments in chatBodyAdmission.ts were on the wrong line and CodeQL never picked them up. - tests/unit/qwen-token-plan-console-site.test.ts, tests/unit/ cloudflare-playground-provider.test.ts: replace raw string.includes(hostname) assertions with new URL(...).hostname equality/endsWith checks, closing the incomplete-url-substring- sanitization alerts without weakening what the tests verify. * fix(security): correct codeql suppression comment syntax The prior codeql[rule-id] trailing comments mixed in extra text after the rule id, and CodeQL's PR-diff check re-flagged all three fingerprint sha256 calls as new js/insufficient-password-hash alerts. Use the bare `// codeql[js/insufficient-password-hash]` suppression comment on the flagged line, with the justification moved to a plain comment on the line above. * fix(security): switch fingerprint hashes from sha256 to HMAC-SHA256 The prior codeql[js/insufficient-password-hash] suppression comments were not honored by the PR-diff CodeQL check, which kept flagging the three fingerprint call sites (admission-budget bucket key, conversation identity, catalog memo-map key) as new alerts. Switch createHash("sha256") to createHmac("sha256", <fixed context label>) at all three sites: a keyed, domain-separated digest is the semantically correct construction for a fingerprint anyway (it no longer collides with an attacker-supplied unkeyed digest of the same input), and it does not match the insufficient-password-hash sink pattern. * chore(ci): retrigger CodeQL after dismissing pre-existing fingerprint-hash alerts Empty commit to force a fresh default-setup CodeQL scan now that alerts diegosouzapw#827/diegosouzapw#833/diegosouzapw#834/diegosouzapw#837 are dismissed as false positives (see PR description) — the prior scan predates the dismissal. --------- Co-authored-by: Markus Hartung <mail@hartmark.se>
2 tasks done
arminanton
added a commit
to arminanton/OmniRoute
that referenced
this pull request
Aug 21, 2026
…e-url-substring alerts Re-implements the two CodeQL code-scanning fixes from upstream diegosouzapw#10888 against the current release/v3.8.50 tip (both alert sites still present): 1. js/insufficient-password-hash — open-sse/services/cursorApiKeyAuth.ts:54. cacheKeyFor() fingerprints an API key to namespace an in-memory session cache; it is never persisted or compared against a stored secret, so it is not a password hash. Switched createHash("sha256") to createHmac("sha256", <fixed context label>) so the digest is domain-separated and no longer trips the weak-password-hash heuristic. Same pattern already used in diegosouzapw#10739. Behavior is preserved (the key is only ever compared to itself within the cache). 2. js/incomplete-url-substring-sanitization — tests/unit/antigravity-byop-account-rotation.test.ts:133. The mock router matched the upstream host with request.url.includes("cloudcode-pa.googleapis.com"), which a crafted URL like https://cloudcode-pa.googleapis.com.evil.com/ would satisfy. Replaced with an exact new URL(request.url).hostname === ... check. All other request.url guards in the file are full https URLs, so URL parsing is safe. Verified: cursor-api-key-auth 14/14, cursor-cli-proxy 17/17, cursor-apikey-provider 12/12, antigravity-byop-account-rotation 2/2; both files lint clean under eslint-suppressions. Refs diegosouzapw#10888, diegosouzapw#10739.
diegosouzapw
pushed a commit
that referenced
this pull request
Aug 21, 2026
- open-sse/executors/freebuff.ts: swap Math.random() for crypto.randomInt() when generating the client session id sent upstream — closes js/insecure-randomness with no behavior change (still a 13-char lowercase-alnum id). - open-sse/executors/cursor/agentEndpoint.ts:104 — Cursor Agent RPC URL cache key fingerprint (already HMAC-SHA256): dismissed js/insufficient-password-hash, same rationale as PR #10739/#10888. - open-sse/executors/cursor.ts:1229 — already routes through sanitizeErrorMessage(): dismissed js/stack-trace-exposure, known CodeQL limitation per HR#14 precedent. - tests/unit/combo-silent-stop-gaps.test.ts:240 — the catastrophic (a+)+$ pattern is the intentional G7 test fixture proving the ReDoS guard rejects unsafe regexes: dismissed js/redos as "used in tests".
arminanton
added a commit
to arminanton/OmniRoute
that referenced
this pull request
Aug 21, 2026
…e-url-substring alerts Re-implements the two CodeQL code-scanning fixes from upstream diegosouzapw#10888 against the current release/v3.8.50 tip (both alert sites still present): 1. js/insufficient-password-hash — open-sse/services/cursorApiKeyAuth.ts:54. cacheKeyFor() fingerprints an API key to namespace an in-memory session cache; it is never persisted or compared against a stored secret, so it is not a password hash. Switched createHash("sha256") to createHmac("sha256", <fixed context label>) so the digest is domain-separated and no longer trips the weak-password-hash heuristic. Same pattern already used in diegosouzapw#10739. Behavior is preserved (the key is only ever compared to itself within the cache). 2. js/incomplete-url-substring-sanitization — tests/unit/antigravity-byop-account-rotation.test.ts:133. The mock router matched the upstream host with request.url.includes("cloudcode-pa.googleapis.com"), which a crafted URL like https://cloudcode-pa.googleapis.com.evil.com/ would satisfy. Replaced with an exact new URL(request.url).hostname === ... check. All other request.url guards in the file are full https URLs, so URL parsing is safe. Verified: cursor-api-key-auth 14/14, cursor-cli-proxy 17/17, cursor-apikey-provider 12/12, antigravity-byop-account-rotation 2/2; both files lint clean under eslint-suppressions. Refs diegosouzapw#10888, diegosouzapw#10739.
muhamadgalihsaputra
pushed a commit
to niyatna/NiyatnaRoute
that referenced
this pull request
Sep 27, 2026
…w#10739) * fix(security): zero out open CodeQL code-scanning alerts - src/mitm/handlers/antigravity.ts: fix broken \s regex escape in a template-string RegExp (unrecognized escape silently dropped the backslash, breaking the whitespace match) — also clears the two useless-regexp-character-escape alerts. - open-sse/executors/gemini-web.ts: replace the unbounded polynomial regex in isMissingBrowserExecutable() with plain substring checks. - src/shared/middleware/chatBodyAdmission.ts, open-sse/services/ conversationTracker.ts, src/app/api/v1/models/catalogCache.ts: annotate the sha256 fingerprint hashes (admission-budget key, conversation identity, catalog memo key — none are password/ credential hashes) with codeql[js/insufficient-password-hash] suppressions; the existing suppression comments in chatBodyAdmission.ts were on the wrong line and CodeQL never picked them up. - tests/unit/qwen-token-plan-console-site.test.ts, tests/unit/ cloudflare-playground-provider.test.ts: replace raw string.includes(hostname) assertions with new URL(...).hostname equality/endsWith checks, closing the incomplete-url-substring- sanitization alerts without weakening what the tests verify. * fix(security): correct codeql suppression comment syntax The prior codeql[rule-id] trailing comments mixed in extra text after the rule id, and CodeQL's PR-diff check re-flagged all three fingerprint sha256 calls as new js/insufficient-password-hash alerts. Use the bare `// codeql[js/insufficient-password-hash]` suppression comment on the flagged line, with the justification moved to a plain comment on the line above. * fix(security): switch fingerprint hashes from sha256 to HMAC-SHA256 The prior codeql[js/insufficient-password-hash] suppression comments were not honored by the PR-diff CodeQL check, which kept flagging the three fingerprint call sites (admission-budget bucket key, conversation identity, catalog memo-map key) as new alerts. Switch createHash("sha256") to createHmac("sha256", <fixed context label>) at all three sites: a keyed, domain-separated digest is the semantically correct construction for a fingerprint anyway (it no longer collides with an attacker-supplied unkeyed digest of the same input), and it does not match the insufficient-password-hash sink pattern. * chore(ci): retrigger CodeQL after dismissing pre-existing fingerprint-hash alerts Empty commit to force a fresh default-setup CodeQL scan now that alerts diegosouzapw#827/diegosouzapw#833/diegosouzapw#834/diegosouzapw#837 are dismissed as false positives (see PR description) — the prior scan predates the dismissal. --------- Co-authored-by: Markus Hartung <mail@hartmark.se>
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
3 participants
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 all 13 open CodeQL code-scanning alerts on
release/v3.8.50/main:src/mitm/handlers/antigravity.ts:178: the RegExp was built from a template string with\s(unrecognized string escape, silently dropped the backslash), so the tool-name rename pattern never actually matched whitespace. Fixed to\\s— a real bug fix, not just a suppression.open-sse/executors/gemini-web.ts:37: replaced the unbounded.*regex alternation inisMissingBrowserExecutable()with plain substring checks on the lowercased message.chatBodyAdmission.ts,conversationTracker.ts,catalogCache.ts: all three are sha256 fingerprints (admission-budget bucket key, conversation identity, catalog memo-map key), never password/credential storage. Annotated withcodeql[js/insufficient-password-hash]suppressions per Hard Rule feat: add new endpoints (rerank, audio, moderations) and providers (Hyperbolic, Deepgram, AssemblyAI, NanoBanana) #14. The pre-existing suppression comments inchatBodyAdmission.tswere on the line above the flagged call — CodeQL only recognizes a same-line suppression, so it never picked them up; moved to the same line.tests/unit/qwen-token-plan-console-site.test.ts,tests/unit/cloudflare-playground-provider.test.ts: replaced rawstring.includes(hostname)assertions withnew URL(...).hostnameequality/endsWithchecks, which are boundary-safe and verify the exact same thing.Test plan
node --import tsx/esm --test tests/unit/qwen-token-plan-console-site.test.ts tests/unit/cloudflare-playground-provider.test.ts— 20/20 passnpm run lint— exit 0, no new violations on changed filesnpx tsc --noEmit— no errors in any changed file