From 00ba606131f0d0d39b1ad51ee426f77e564de8de Mon Sep 17 00:00:00 2001 From: Anthony Ettinger Date: Mon, 10 Aug 2026 17:49:00 +0000 Subject: [PATCH] fix(sarif): publish the fingerprint under a namespaced key MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 --- apps/cli/package.json | 2 +- apps/cli/src/scan/__tests__/sarif.test.ts | 9 ++++-- apps/cli/src/scan/sarif.ts | 37 +++++++++++++++++------ 3 files changed, 35 insertions(+), 13 deletions(-) diff --git a/apps/cli/package.json b/apps/cli/package.json index a98df20..2986aab 100644 --- a/apps/cli/package.json +++ b/apps/cli/package.json @@ -1,6 +1,6 @@ { "name": "@profullstack/threatcrush", - "version": "0.6.1", + "version": "0.6.2", "description": "All-in-one security agent daemon — monitor, detect, scan, and protect servers in real-time", "bin": { "threatcrush": "./dist/index.js" diff --git a/apps/cli/src/scan/__tests__/sarif.test.ts b/apps/cli/src/scan/__tests__/sarif.test.ts index 08107c2..5dca05a 100644 --- a/apps/cli/src/scan/__tests__/sarif.test.ts +++ b/apps/cli/src/scan/__tests__/sarif.test.ts @@ -52,10 +52,15 @@ describe('fingerprints', () => { expect(fingerprintOf(finding({ excerpt: 'something else' }))).not.toBe(base); }); - it('is the value that reaches the SARIF document', () => { + it('is published under a namespaced key, not the reserved one', () => { + // `primaryLocationLineHash` is computed by GitHub's upload action, which + // logs an inconsistent-fingerprint warning for every finding when we also + // supply it — whatever value we put there. const f = finding(); const log = buildSarif([f], { toolVersion: '1.0.0', base: '/repo', root: '/repo' }); - expect(firstResult(log).partialFingerprints.primaryLocationLineHash).toBe(fingerprintOf(f)); + const prints = firstResult(log).partialFingerprints; + expect(prints['threatcrush/contentHash/v1']).toBe(fingerprintOf(f)); + expect(prints).not.toHaveProperty('primaryLocationLineHash'); }); }); diff --git a/apps/cli/src/scan/sarif.ts b/apps/cli/src/scan/sarif.ts index d76e4e4..3b653ec 100644 --- a/apps/cli/src/scan/sarif.ts +++ b/apps/cli/src/scan/sarif.ts @@ -29,19 +29,36 @@ import { isAbsolute, relative, resolve, sep } from 'node:path'; import type { ScanFinding, Severity } from './types.js'; /** - * A stable identity for a finding, for `partialFingerprints`. + * The key our fingerprint is published under. + * + * Deliberately *not* `primaryLocationLineHash`. That name is reserved: the + * CodeQL upload action computes its own value for it and logs + * + * Calculated fingerprint of 13bfd14c5cc763c:1 for file debtap line 104, + * but found existing inconsistent fingerprint value + * + * for every finding whose value differs from what it derived — which is any + * value we supply, whatever it contains. The first attempt at this replaced + * the old `ruleId:file:line` with a content hash and still logged the warning, + * because the collision is over the *key*, not the format. * - * `primaryLocationLineHash` is a key GitHub reserves and recomputes: it - * expects a hash of the offending *content*, and anything else is reported as - * an inconsistent fingerprint on every upload. + * Namespacing it leaves GitHub to compute the fingerprint it wants while other + * SARIF consumers keep a stable identity from us. The version suffix is there + * so the hash input can change later without silently redefining what an + * existing value meant. + */ +const FINGERPRINT_KEY = 'threatcrush/contentHash/v1'; + +/** + * A stable identity for a finding, for `partialFingerprints`. * * The line number is deliberately not part of it. It used to be — the value * was `ruleId:file:line` — which meant adding an import at the top of a file - * re-fingerprinted every finding below it. GitHub then treats them as new - * alerts: previously dismissed ones come back, and review comments detach from - * the code they were written about. Hashing the rule, the file and the matched - * text instead keeps one finding identified as one finding while it moves - * around the file. + * re-fingerprinted every finding below it. A consumer that tracks findings by + * fingerprint then treats them as new: previously dismissed ones come back, + * and review comments detach from the code they were written about. Hashing + * the rule, the file and the matched text instead keeps one finding identified + * as one finding while it moves around the file. * * Whitespace is normalised so reindentation does not count as a new finding. * 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 }, ], partialFingerprints: { - primaryLocationLineHash: fingerprintOf(finding), + [FINGERPRINT_KEY]: fingerprintOf(finding), }, properties: { severity: finding.severity,