fix(sarif): publish the fingerprint under a namespaced key (v0.6.2) - #90
Merged
Conversation
Release 0.6.2. 0.6.1 made partialFingerprints a content hash and the inconsistent- fingerprint warning kept appearing, now comparing against the new hash: Calculated fingerprint of 13bfd14c5cc763c:1 for file debtap line 104, but found existing inconsistent fingerprint value a279c9a6c714c186... The collision is over the key, not the format. `primaryLocationLineHash` is computed by the CodeQL upload action itself, and it warns whenever the value it derived differs from one already present — which is any value we supply, whatever it contains. Publish under `threatcrush/contentHash/v1` instead. GitHub computes the fingerprint it wants, other SARIF consumers keep a stable identity from us, and the warning goes away. The version suffix leaves room to change the hash input later without silently redefining an existing value. The hash itself is unchanged from 0.6.1, so identities carry over. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
ThreatCrush Security Scan158 finding(s) HIGH/CRITICAL: 14 | MEDIUM: 108 | LOW: 36
…and 108 more. Full results in the Security tab. Snippets are redacted; ThreatCrush never prints matched credential material. |
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.
Release 0.6.2. Completes #89, which fixed half the problem.
What #89 missed
#89 replaced
ruleId:file:linewith a content hash. The stability half of that was right and stands. The warning was not fixed — the next real upload logged it again, now comparing against the new hash:The collision is over the key, not the format.
primaryLocationLineHashis computed by the CodeQL upload action itself, and it warns whenever the value it derived differs from one already present in the document. That is any value we supply, whatever it contains — a correct content hash included.The fix
Publish under
threatcrush/contentHash/v1and leaveprimaryLocationLineHashunset.GitHub then computes the fingerprint it wants and stops complaining; SARIF consumers that are not GitHub keep a stable identity from us. The version suffix means the hash input can change later without silently redefining what an existing value meant.
The hash function is unchanged from 0.6.1, so identities carry over — this is a key rename, not a re-fingerprint.
Verification
The test that asserted the value reached
primaryLocationLineHashnow asserts the opposite: the fingerprint appears under the namespaced key, andprimaryLocationLineHashis absent. The other four fingerprint tests from #89 are unchanged and still pass — it is a hash, it survives a move from line 23 to line 891, it survives reindentation, and it still separates different rules, files and content.105 tests pass.
tsc --noEmitclean.Confirmed against ralyodio/debtap after merge, which is where both warnings were observed.