Repository navigation
ci: suppress the JWT-grant CodeQL false positive in place, and make the marker count - #81
Merged
Merged
Conversation
js/insufficient-password-hash has flagged the same RS256 JWT-grant signature three times (alerts 1, 10, 15): the JWT's aud claim is read from config.oauthBaseUrl, whose name trips the query's password heuristic. The inline codeql[] marker from #58 is not honoured by this setup, and an API dismissal is keyed on the alert's fingerprint, so every file move re-opened it. The query is now excluded in .github/codeql/codeql-config.yml (query-filters), which the workflow reads through config-file; the suite selection moves there too. The finding cannot come back on any branch, and the rule for the next false positive is written down in AGENTS.md / CLAUDE.md.
Excluding js/insufficient-password-hash repo-wide would also hide a real weak password hash added later. The inline marker is the right tool - it silences one line and travels with the code - and the reason it never worked is that the default suite does not run the pack's AlertSuppression query, so the codeql[] comment from #58 was never read. The config now runs that query next to security-and-quality, the marker sits alone on the line above the flagged sign.update(), and the rule in AGENTS.md / CLAUDE.md says so.
grimen
changed the title
blink-admin
approved these changes
Sep 10, 2026
This was referenced Sep 10, 2026
blink-admin
pushed a commit
that referenced
this pull request
Sep 15, 2026
…m it misread (#108) * fix(node): send the OAuth host alone as the JWT aud claim DOCUSIGN_OAUTH_URL with a trailing slash put 'account-d.docusign.com/' in the assertion's aud claim, which DocuSign has tolerated and is not what the claim means; an http:// value put the entire URL there, which it would not have tolerated. .replace('https://', '') only ever removed that one prefix, so neither shape was handled. It also stops CodeQL's js/insufficient-password-hash firing: the rule read the 'auth' in oauthBaseUrl as a credential and RSA-SHA256 as a password hash. That false positive has been suppressed, dismissed and excluded four times since #58 - alerts 1, 10, 15 and 18 - and came back every time. Not tripping it is the only fix that stays, and `make codeql` now reports no finding for it at all rather than a suppressed one. * test(service): drop an unused vitest import js/unused-local-variable, and correct on its own terms: nothing in the file uses vi. * ci: stop claiming an inline marker suppresses a CodeQL alert It does not, on GitHub. The CodeQL CLI honours // codeql[<rule-id>] and writes suppressions: [{state: accepted}] onto the result - and GitHub code scanning ignores that property. Verified on analysis 1779039789: the uploaded SARIF carried the suppression and alert 18 was open with dismissed_reason: null. make codeql said otherwise, because codeql-findings.mjs decided "suppressed" by reading the same property GitHub discards. So the local gate exited 0 on a finding that was red on GitHub, and nobody had a reason to look. That is how this survived #58 and #81 and came back as alerts 1, 10, 15 and 18. Every finding now counts as open here too, which is what GitHub will do with it. The AlertSuppression pack goes with it: it produces a suppression nothing consumes and a local signal that misleads. Excluding a query repo-wide is still refused - #81 tried it and reverted it inside the same PR, for the right reason. The config, CLAUDE.md, AGENTS.md, the workflow and the development guide all told the next person to reach for the marker. They now say what actually works: make the query not fire.
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
2 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.
js/insufficient-password-hashhas flagged the same RS256 JWT-grant signature three times (alerts 1, 10, 15): the JWT'saudclaim is read fromconfig.oauthBaseUrl, whose name trips the query's password heuristic. The inlinecodeql[]marker from #58 never worked because the default suite does not run the pack'sAlertSuppressionquery, so the comment was never read; and an API dismissal is keyed on the alert's fingerprint, so every file move re-opened it.What changes
.github/codeql/codeql-config.yml: thesecurity-and-qualitysuite pluscodeql/javascript-queries:AlertSuppression.ql. The workflow reads it throughconfig-file.// codeql[js/insufficient-password-hash]alone on the line above the flaggedsign.update(signingInput). It suppresses that one line and travels with the code. The query stays on for everything else - a real weak password hash elsewhere still fails.Verified
make check-ci(actionlint + shellcheck) and the docs-tables check pass. This PR's own CodeQL run is the proof: the analysis must still run the query and report the finding as suppressed, not open.