Skip to content

Commit 58e9eeb

Browse files
ralyodioclaude
andauthored
fix(sarif): publish the fingerprint under a namespaced key (#90)
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>
1 parent 60ac23d commit 58e9eeb

3 files changed

Lines changed: 35 additions & 13 deletions

File tree

apps/cli/package.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
{
22
"name": "@profullstack/threatcrush",
3-
"version": "0.6.1",
3+
"version": "0.6.2",
44
"description": "All-in-one security agent daemon — monitor, detect, scan, and protect servers in real-time",
55
"bin": {
66
"threatcrush": "./dist/index.js"

apps/cli/src/scan/__tests__/sarif.test.ts

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -52,10 +52,15 @@ describe('fingerprints', () => {
5252
expect(fingerprintOf(finding({ excerpt: 'something else' }))).not.toBe(base);
5353
});
5454

55-
it('is the value that reaches the SARIF document', () => {
55+
it('is published under a namespaced key, not the reserved one', () => {
56+
// `primaryLocationLineHash` is computed by GitHub's upload action, which
57+
// logs an inconsistent-fingerprint warning for every finding when we also
58+
// supply it — whatever value we put there.
5659
const f = finding();
5760
const log = buildSarif([f], { toolVersion: '1.0.0', base: '/repo', root: '/repo' });
58-
expect(firstResult(log).partialFingerprints.primaryLocationLineHash).toBe(fingerprintOf(f));
61+
const prints = firstResult(log).partialFingerprints;
62+
expect(prints['threatcrush/contentHash/v1']).toBe(fingerprintOf(f));
63+
expect(prints).not.toHaveProperty('primaryLocationLineHash');
5964
});
6065
});
6166

apps/cli/src/scan/sarif.ts

Lines changed: 27 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -29,19 +29,36 @@ import { isAbsolute, relative, resolve, sep } from 'node:path';
2929
import type { ScanFinding, Severity } from './types.js';
3030

3131
/**
32-
* A stable identity for a finding, for `partialFingerprints`.
32+
* The key our fingerprint is published under.
33+
*
34+
* Deliberately *not* `primaryLocationLineHash`. That name is reserved: the
35+
* CodeQL upload action computes its own value for it and logs
36+
*
37+
* Calculated fingerprint of 13bfd14c5cc763c:1 for file debtap line 104,
38+
* but found existing inconsistent fingerprint value <ours>
39+
*
40+
* for every finding whose value differs from what it derived — which is any
41+
* value we supply, whatever it contains. The first attempt at this replaced
42+
* the old `ruleId:file:line` with a content hash and still logged the warning,
43+
* because the collision is over the *key*, not the format.
3344
*
34-
* `primaryLocationLineHash` is a key GitHub reserves and recomputes: it
35-
* expects a hash of the offending *content*, and anything else is reported as
36-
* an inconsistent fingerprint on every upload.
45+
* Namespacing it leaves GitHub to compute the fingerprint it wants while other
46+
* SARIF consumers keep a stable identity from us. The version suffix is there
47+
* so the hash input can change later without silently redefining what an
48+
* existing value meant.
49+
*/
50+
const FINGERPRINT_KEY = 'threatcrush/contentHash/v1';
51+
52+
/**
53+
* A stable identity for a finding, for `partialFingerprints`.
3754
*
3855
* The line number is deliberately not part of it. It used to be — the value
3956
* was `ruleId:file:line` — which meant adding an import at the top of a file
40-
* re-fingerprinted every finding below it. GitHub then treats them as new
41-
* alerts: previously dismissed ones come back, and review comments detach from
42-
* the code they were written about. Hashing the rule, the file and the matched
43-
* text instead keeps one finding identified as one finding while it moves
44-
* around the file.
57+
* re-fingerprinted every finding below it. A consumer that tracks findings by
58+
* fingerprint then treats them as new: previously dismissed ones come back,
59+
* and review comments detach from the code they were written about. Hashing
60+
* the rule, the file and the matched text instead keeps one finding identified
61+
* as one finding while it moves around the file.
4562
*
4663
* Whitespace is normalised so reindentation does not count as a new finding.
4764
* Two identical lines in one file collide onto one fingerprint, which is the
@@ -224,7 +241,7 @@ export function buildSarif(findings: readonly ScanFinding[], options: SarifOptio
224241
},
225242
],
226243
partialFingerprints: {
227-
primaryLocationLineHash: fingerprintOf(finding),
244+
[FINGERPRINT_KEY]: fingerprintOf(finding),
228245
},
229246
properties: {
230247
severity: finding.severity,

0 commit comments

Comments
 (0)