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.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"
Expand Down
40 changes: 39 additions & 1 deletion apps/cli/src/scan/__tests__/sarif.test.ts
Original file line number Diff line number Diff line change
@@ -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> = {}): ScanFinding => ({
Expand All @@ -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
Expand Down
31 changes: 30 additions & 1 deletion apps/cli/src/scan/sarif.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -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,
Expand Down
Loading