Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion apps/cli/package.json
Original file line number Diff line number Diff line change
@@ -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"
Expand Down
9 changes: 7 additions & 2 deletions apps/cli/src/scan/__tests__/sarif.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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');
});
});

Expand Down
37 changes: 27 additions & 10 deletions apps/cli/src/scan/sarif.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 <ours>
*
* 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
Expand Down Expand Up @@ -224,7 +241,7 @@ export function buildSarif(findings: readonly ScanFinding[], options: SarifOptio
},
],
partialFingerprints: {
primaryLocationLineHash: fingerprintOf(finding),
[FINGERPRINT_KEY]: fingerprintOf(finding),
},
properties: {
severity: finding.severity,
Expand Down
Loading