Repository navigation
fix(api): suppress false-positive CodeQL password-hash alert on JWT signing - #58
Merged
Merged
Conversation
…igning CodeQL alert 1 (js/insufficient-password-hash) flags the RS256 JWT-grant signature in createJwtAssertion because the aud claim is derived from config.oauthBaseUrl, whose name matches CodeQL's password heuristic. The value is the public DocuSign OAuth host, not a secret, and the operation is a signature, not a password hash. An inline suppression is used instead of an API dismissal so it survives edits and file moves (a dismissal is keyed to the alert fingerprint).
grimen
added a commit
that referenced
this pull request
Sep 10, 2026
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.
blink-admin
pushed a commit
that referenced
this pull request
Sep 10, 2026
…he marker count (#81) * ci: exclude the JWT-grant false positive from CodeQL by query, for good 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. * ci: suppress the JWT-grant false positive in place, not the whole query 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.
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
1 participant
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 #1 (
js/insufficient-password-hash, the last open alert onmain) with an inline suppression on the flagged statement.createJwtAssertionbuilds the RS256 JWT-grant assertion DocuSign requires. Theaudclaim is derived fromconfig.oauthBaseUrl, and CodeQL's name heuristic treats that as password-like, so it reports the signature as an insecure password hash. The value is the public OAuth host, not a secret, andcreateSign('RSA-SHA256')is a signature, not a hash of a credential.// codeql[js/insufficient-password-hash]marker travels with the code, is visible in review, and scopes to exactly this rule on this statement. No repo-widequery-filtersexclusion, nopaths-ignore, no rename ofoauthBaseUrl.Verification
js/insufficient-password-hashresult forapps/api/src/providers/docusign/client.ts.mainscan closes alert Bump the actions group with 3 updates #1 as fixed.