From d881d7d7a8717d23f8df290f79b351763224d85a Mon Sep 17 00:00:00 2001 From: Anthony Ettinger Date: Mon, 10 Aug 2026 17:36:24 +0000 Subject: [PATCH] fix(sarif): make partialFingerprints a content hash MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Release 0.6.1. `primaryLocationLineHash` is a key GitHub reserves and recomputes: it expects a hash of the offending content. We were sending `ruleId:file:line`, so every upload logged an inconsistent-fingerprint warning for every finding — 8 of them on the first real run against ralyodio/debtap. The line number in the value was the worse half. Adding an import at the top of a file re-fingerprinted every finding below it, and GitHub then treats those as new alerts: previously dismissed ones come back and review comments detach from the code they were written about. On a scanner that runs per pull request, that is most commits. Hash the rule, the file and the matched text instead, with whitespace normalised so reindentation is not a new finding. One finding stays one finding while it moves around the file. 105 tests, up from 100. Co-Authored-By: Claude Opus 5 --- apps/cli/package.json | 2 +- apps/cli/src/scan/__tests__/sarif.test.ts | 40 ++++++++++++++++++++++- apps/cli/src/scan/sarif.ts | 31 +++++++++++++++++- 3 files changed, 70 insertions(+), 3 deletions(-) diff --git a/apps/cli/package.json b/apps/cli/package.json index 541e2e9..a98df20 100644 --- a/apps/cli/package.json +++ b/apps/cli/package.json @@ -1,6 +1,6 @@ { "name": "@profullstack/threatcrush", - "version": "0.6.0", + "version": "0.6.1", "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 e60c123..08107c2 100644 --- a/apps/cli/src/scan/__tests__/sarif.test.ts +++ b/apps/cli/src/scan/__tests__/sarif.test.ts @@ -1,5 +1,5 @@ import { describe, expect, it } from 'vitest'; -import { buildSarif, sarifLevel, securitySeverity, toArtifactUri } from '../sarif.js'; +import { buildSarif, fingerprintOf, sarifLevel, securitySeverity, toArtifactUri } from '../sarif.js'; import type { ScanFinding } from '../types.js'; const finding = (overrides: Partial = {}): ScanFinding => ({ @@ -21,6 +21,44 @@ const firstResult = (log: unknown): any => (log as any).runs[0].results[0]; const uriOf = (log: unknown): string => firstResult(log).locations[0].physicalLocation.artifactLocation.uri; +/** + * `primaryLocationLineHash` is a key GitHub reserves and recomputes. It used to + * carry `ruleId:file:line`, which GitHub rejected as inconsistent on every + * upload and which changed whenever code above the finding moved — so dismissed + * alerts came back and review comments detached. + */ +describe('fingerprints', () => { + it('is a hash, not a readable triple', () => { + const value = fingerprintOf(finding()); + expect(value).toMatch(/^[0-9a-f]{32}$/); + expect(value).not.toContain('secret-aws-access-key'); + expect(value).not.toContain('23'); + }); + + it('survives the finding moving to another line', () => { + expect(fingerprintOf(finding({ line: 23 }))).toBe(fingerprintOf(finding({ line: 891 }))); + }); + + it('survives reindentation', () => { + const a = finding({ excerpt: 'foo(bar)' }); + const b = finding({ excerpt: ' foo(bar) ' }); + expect(fingerprintOf(a)).toBe(fingerprintOf(b)); + }); + + it('separates different rules, files and content', () => { + const base = fingerprintOf(finding()); + expect(fingerprintOf(finding({ ruleId: 'secret-github-token' }))).not.toBe(base); + expect(fingerprintOf(finding({ file: 'other/creds.env' }))).not.toBe(base); + expect(fingerprintOf(finding({ excerpt: 'something else' }))).not.toBe(base); + }); + + it('is the value that reaches the SARIF document', () => { + const f = finding(); + const log = buildSarif([f], { toolVersion: '1.0.0', base: '/repo', root: '/repo' }); + expect(firstResult(log).partialFingerprints.primaryLocationLineHash).toBe(fingerprintOf(f)); + }); +}); + describe('artifact URIs', () => { it('resolves finding paths against the scan root, not the working directory', () => { // The bug this guards: findings carry paths relative to the scan root, so diff --git a/apps/cli/src/scan/sarif.ts b/apps/cli/src/scan/sarif.ts index 8c99b22..d76e4e4 100644 --- a/apps/cli/src/scan/sarif.ts +++ b/apps/cli/src/scan/sarif.ts @@ -24,9 +24,38 @@ * the consumer was scoping to. */ +import { createHash } from 'node:crypto'; import { isAbsolute, relative, resolve, sep } from 'node:path'; import type { ScanFinding, Severity } from './types.js'; +/** + * A stable identity for a finding, for `partialFingerprints`. + * + * `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. + * + * 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. + * + * 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 + * right trade: they are the same defect, and SARIF locations still tell them + * apart. + */ +export function fingerprintOf(finding: ScanFinding): string { + const content = finding.excerpt.replace(/\s+/g, ' ').trim(); + return createHash('sha256') + .update(`${finding.ruleId}\n${finding.file}\n${content}`) + .digest('hex') + .slice(0, 32); +} + export const SARIF_VERSION = '2.1.0'; export const SARIF_SCHEMA = 'https://raw.githubusercontent.com/oasis-tcs/sarif-spec/master/Schemata/sarif-schema-2.1.0.json'; @@ -195,7 +224,7 @@ export function buildSarif(findings: readonly ScanFinding[], options: SarifOptio }, ], partialFingerprints: { - primaryLocationLineHash: `${finding.ruleId}:${finding.file}:${Math.max(1, finding.line)}`, + primaryLocationLineHash: fingerprintOf(finding), }, properties: { severity: finding.severity,