Skip to content

fix(sarif): make partialFingerprints a content hash (v0.6.1) - #89

Merged
ralyodio merged 1 commit into
masterfrom
fix/sarif-fingerprints
Aug 10, 2026
Merged

fix(sarif): make partialFingerprints a content hash (v0.6.1)#89
ralyodio merged 1 commit into
masterfrom
fix/sarif-fingerprints

Conversation

@ralyodio

Copy link
Copy Markdown
Contributor

Release 0.6.1.

The bug

primaryLocationLineHash is a key GitHub reserves and recomputes — it expects a hash of the offending content. We were sending a readable triple:

primaryLocationLineHash: `${finding.ruleId}:${finding.file}:${Math.max(1, finding.line)}`

The first real upload said so, once per finding:

! Calculated fingerprint of e36b923340bc3f20:1 for file debtap line 128,
  but found existing inconsistent fingerprint value sh-insecure-transport-flag:debtap:128

Eight warnings, eight findings, on ralyodio/debtap#1.

Why it matters more than the warning

The line number was baked into the identity. Adding an import at the top of a file re-fingerprints every finding below it, and GitHub treats a changed fingerprint as a new alert — so:

  • alerts a maintainer already dismissed come back;
  • review comments detach from the code they were written about;
  • "3 new alerts" on a PR that introduced none.

On a scanner that runs per pull request, that is most commits. It is the kind of noise that gets a security check muted rather than fixed.

The fix

Hash the rule, the file and the matched text — not the position:

createHash('sha256').update(`${ruleId}\n${file}\n${content}`).digest('hex').slice(0, 32)

Whitespace is normalised first, so reindenting is not a new finding either. Two identical lines in one file collide onto one fingerprint, which is the right trade: they are the same defect, and the SARIF locations still tell them apart.

Verification

Five tests added: the value is a hash and leaks neither rule id nor line number; it survives the finding moving from line 23 to line 891; it survives reindentation; it still separates different rules, files and content; and it is the value that actually reaches the document.

105 tests pass, up from 100. tsc --noEmit clean.

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 <noreply@anthropic.com>
@github-actions

Copy link
Copy Markdown

ThreatCrush Security Scan

158 finding(s)

HIGH/CRITICAL: 14 | MEDIUM: 108 | LOW: 36

Severity Rule Location
HIGH sql-template-interpolation apps/cli/src/scan/__tests__/code-rules.test.ts:31
HIGH secret-aws-access-key apps/cli/src/scan/secret-rules.ts:192
HIGH js-shell-exec-interpolation modules/code-scanner/src/__tests__/sast.test.ts:31
HIGH js-shell-exec-interpolation modules/code-scanner/src/__tests__/sast.test.ts:102
HIGH js-shell-exec-interpolation modules/code-scanner/src/__tests__/sast.test.ts:108
HIGH secret-aws-access-key modules/code-scanner/src/secrets/rules.ts:74
HIGH secret-aws-access-key prd/0003-detect-hardcoded-secrets-before-they-are-committed-or-served.md:126
HIGH sh-eval-expansion .githooks/pre-commit:26
HIGH js-unsafe-yaml-load apps/cli/src/scan/__tests__/code-rules.test.ts:216
HIGH secret-generic-credential modules/spend-guard/config/example.conf.toml:13
HIGH secret-generic-credential modules/spend-guard/README.md:84
HIGH secret-generic-credential PRD.md:268
HIGH sh-remote-script-execution scripts/smoke-test.sh:46
HIGH sh-remote-script-execution scripts/smoke-test.sh:47
MEDIUM insecure-temp-file .githooks/commit-msg:16
MEDIUM insecure-temp-file .githooks/post-commit:20
MEDIUM js-shell-exec-interpolation apps/cli/src/commands/init.ts:70
MEDIUM js-shell-exec-interpolation apps/cli/src/commands/init.ts:79
MEDIUM sql-template-interpolation apps/cli/src/commands/properties.ts:226
MEDIUM js-shell-exec-interpolation apps/cli/src/commands/service.ts:88
MEDIUM js-shell-exec-interpolation apps/cli/src/commands/service.ts:111
MEDIUM sql-template-interpolation apps/cli/src/core/state.ts:121
MEDIUM sql-template-interpolation apps/cli/src/core/state.ts:125
MEDIUM js-shell-exec-interpolation apps/cli/src/daemon/firewall/adapters.ts:31
MEDIUM js-shell-exec-interpolation apps/cli/src/daemon/firewall/adapters.ts:33
MEDIUM js-shell-exec-interpolation apps/cli/src/daemon/firewall/adapters.ts:34
MEDIUM js-shell-exec-interpolation apps/cli/src/daemon/firewall/adapters.ts:35
MEDIUM js-shell-exec-interpolation apps/cli/src/daemon/firewall/adapters.ts:36
MEDIUM js-shell-exec-interpolation apps/cli/src/daemon/firewall/adapters.ts:43
MEDIUM sql-template-interpolation apps/cli/src/daemon/firewall/adapters.ts:49
MEDIUM js-shell-exec-interpolation apps/cli/src/daemon/firewall/adapters.ts:49
MEDIUM js-shell-exec-interpolation apps/cli/src/daemon/firewall/adapters.ts:56
MEDIUM js-shell-exec-interpolation apps/cli/src/daemon/firewall/adapters.ts:63
MEDIUM js-shell-exec-interpolation apps/cli/src/daemon/firewall/adapters.ts:82
MEDIUM js-shell-exec-interpolation apps/cli/src/daemon/firewall/adapters.ts:84
MEDIUM js-shell-exec-interpolation apps/cli/src/daemon/firewall/adapters.ts:85
MEDIUM js-shell-exec-interpolation apps/cli/src/daemon/firewall/adapters.ts:93
MEDIUM js-shell-exec-interpolation apps/cli/src/daemon/firewall/adapters.ts:98
MEDIUM js-shell-exec-interpolation apps/cli/src/daemon/firewall/adapters.ts:105
MEDIUM js-shell-exec-interpolation apps/cli/src/daemon/firewall/adapters.ts:112
MEDIUM sql-template-interpolation apps/cli/src/index.ts:105
MEDIUM sql-template-interpolation apps/cli/src/index.ts:110
MEDIUM sql-template-interpolation apps/cli/src/index.ts:120
MEDIUM js-shell-exec-interpolation apps/cli/src/index.ts:411
MEDIUM sql-string-concatenation apps/cli/src/scan/__tests__/code-rules.test.ts:21
MEDIUM sql-template-interpolation apps/cli/src/scan/__tests__/code-rules.test.ts:35
MEDIUM sql-string-concatenation apps/cli/src/scan/__tests__/code-rules.test.ts:40
MEDIUM sql-string-concatenation apps/cli/src/scan/__tests__/code-rules.test.ts:49
MEDIUM sql-string-concatenation apps/cli/src/scan/__tests__/code-rules.test.ts:53
MEDIUM js-shell-exec-interpolation apps/cli/src/scan/__tests__/code-rules.test.ts:65

…and 108 more. Full results in the Security tab.

Snippets are redacted; ThreatCrush never prints matched credential material.

@ralyodio
ralyodio merged commit 60ac23d into master Aug 10, 2026
9 checks passed
@ralyodio
ralyodio deleted the fix/sarif-fingerprints branch August 10, 2026 17:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant