From 5a509408ce72f1a0d87816ddf424d99704b34c6d Mon Sep 17 00:00:00 2001 From: Anthony Ettinger Date: Sun, 16 Aug 2026 07:05:23 +0000 Subject: [PATCH] fix(threatcrush-scan): pin the CLI to 0.11.2, which has the false-positive fix MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The pin protects consumers from a bad publish. It equally withholds a good one, and that side of it just bit: pack 1.6.0 pinned 0.11.0, which predates the false-positive work released in 0.11.2. A consumer on the newer pack got the noisier scanner while an older install tracking @latest got the fixed one. On qryptchat-web that is 91 findings versus 13, and five spurious HIGHs versus none. All five were false — three flagged TOKEN="${TOKEN:-}", a shell reading a token from the environment, which is the remediation reported as the defect. Verified the hash against the published tarball with the same pipeline the workflow uses, rather than copying it from `npm view` alone: npm pack @profullstack/threatcrush@0.11.2 openssl dgst -sha512 -binary *.tgz | openssl base64 -A Both match sha512-8N3jqCQixK0Onc+/bvuJaNCSvGZlJYZcSAGsd1nEfRZ4kOu1Ifom 7Bd1t2muYJAmAxBTPmz1iseWSay/0gg3Gw==. Adds a test that the spec and the hash cannot drift apart, and that the README quotes the same version the manifest pins — a stale table is how someone bumps the spec while reading the previous version's hash, and a mismatched hash fails closed with a job log that does not say why. Confirmed the test fails when the spec is moved on its own. Co-Authored-By: Claude Opus 5 (1M context) --- packages/actions/src/index.test.ts | 26 +++++++++++++++++++ packages/actions/threatcrush-scan/README.md | 25 ++++++++++++++++-- .../threatcrush-scan/sh1pt.actionpack.yaml | 6 ++--- 3 files changed, 52 insertions(+), 5 deletions(-) diff --git a/packages/actions/src/index.test.ts b/packages/actions/src/index.test.ts index 4d1493d1..1fb7cfc8 100644 --- a/packages/actions/src/index.test.ts +++ b/packages/actions/src/index.test.ts @@ -1,3 +1,5 @@ +import { readFile } from 'node:fs/promises'; +import { join } from 'node:path'; import { describe, it, expect } from 'vitest'; import { parse as parseYaml } from 'yaml'; import { renderPack } from '@profullstack/sh1pt-actions-fleet-core'; @@ -236,6 +238,30 @@ describe('built-in packs', () => { } }); + it('pins the CLI and its integrity hash to the same version', async () => { + // These two must move together. A hash left behind from the previous + // version fails *closed* — the workflow refuses to install and every + // consumer's scan stops — which is the right direction to fail and a + // thoroughly confusing one to debug from the job log. + const catalog = await loadBuiltinPacks(); + const entry = catalog.get('threatcrush-scan'); + if (!entry) throw new Error('threatcrush-scan not in catalog'); + + const inputs = entry.manifest.inputs as Record; + const spec = inputs.threatcrushPackageSpec?.default ?? ''; + const integrity = inputs.threatcrushIntegrity?.default ?? ''; + + expect(spec).toMatch(/^@profullstack\/threatcrush@\d+\.\d+\.\d+$/); + expect(integrity).toMatch(/^sha512-[A-Za-z0-9+/]+={0,2}$/); + + // The README's inputs table quotes both, and a stale table is how someone + // ends up bumping the spec while reading the old version's hash. + const readme = await readFile(join(entry.packDir, 'README.md'), 'utf-8'); + const version = spec.split('@').pop(); + expect(readme).toContain(spec); + expect(readme).toContain(`sha512 of ${version}`); + }); + it('orders the report by severity rather than by file', async () => { // The 50-row cap used to be applied in SARIF order, which is file order, // so which findings survived truncation was decided by where they sat in diff --git a/packages/actions/threatcrush-scan/README.md b/packages/actions/threatcrush-scan/README.md index 554c966f..e172b9b4 100644 --- a/packages/actions/threatcrush-scan/README.md +++ b/packages/actions/threatcrush-scan/README.md @@ -14,11 +14,32 @@ sh1pt actions install threatcrush-scan --repo owner/name --pr | --- | --- | --- | | `scanPath` | `.` | Path to scan, relative to the repository root. | | `nodeVersion` | `20` | See *Node 20, deliberately*, below. | -| `threatcrushPackageSpec` | `@profullstack/threatcrush@0.11.0` | npm spec used to install the CLI. Pinned rather than `@latest` so one bad publish cannot break every consumer at once; bump it in a pack release. | -| `threatcrushIntegrity` | *(sha512 of 0.11.0)* | SRI hash of that tarball. The workflow downloads, hashes and compares before installing, and refuses to install on a mismatch. Bump it with the spec — read it from `npm view dist.integrity`. Empty skips the check. | +| `threatcrushPackageSpec` | `@profullstack/threatcrush@0.11.2` | npm spec used to install the CLI. Pinned rather than `@latest` so one bad publish cannot break every consumer at once; bump it in a pack release. | +| `threatcrushIntegrity` | *(sha512 of 0.11.2)* | SRI hash of that tarball. The workflow downloads, hashes and compares before installing, and refuses to install on a mismatch. Bump it with the spec — read it from `npm view dist.integrity`. Empty skips the check. | | `failOn` | *(empty)* | Comma-separated severities that fail the job, e.g. `critical,high`. Empty is report-only. | | `uploadSarif` | `true` | Upload to the Security tab. | +## Pinned means pinned — including for fixes + +The pin protects consumers from a bad publish. It equally withholds a good one: +a repository on this pack does **not** pick up a scanner release until +`threatcrushPackageSpec` and `threatcrushIntegrity` are bumped here and the +fleet re-syncs. + +That has already produced the counter-intuitive case. Pack `1.6.0` pinned +`0.11.0`, which predates the false-positive work in `0.11.2` — so a consumer on +the *newer* pack got the *noisier* scanner, while an older install tracking +`@latest` got the fixed one. On qryptchat-web that was the difference between 91 +findings and 13, with five spurious HIGHs. + +So a scanner release is not finished until this pin moves. Bump both inputs in +the same edit — a hash from a different version fails closed, which is the right +direction to fail but a confusing one to debug: + +```bash +npm view @profullstack/threatcrush@ dist.integrity +``` + ## Report-only by default `failOn` is empty on purpose. A repository with pre-existing findings should diff --git a/packages/actions/threatcrush-scan/sh1pt.actionpack.yaml b/packages/actions/threatcrush-scan/sh1pt.actionpack.yaml index 015a5cfc..744aa500 100644 --- a/packages/actions/threatcrush-scan/sh1pt.actionpack.yaml +++ b/packages/actions/threatcrush-scan/sh1pt.actionpack.yaml @@ -5,7 +5,7 @@ description: >- Scans pull requests for hardcoded credentials, injection, SSRF, unsafe deserialisation and dependency tampering, and uploads SARIF to the Security tab. -version: 1.6.0 +version: 1.6.1 publisher: profullstack visibility: public license: MIT @@ -32,7 +32,7 @@ inputs: that fails without a full toolchain. threatcrushPackageSpec: type: string - default: '@profullstack/threatcrush@0.11.0' + default: '@profullstack/threatcrush@0.11.2' description: >- npm spec used to install the CLI. Pinned, not `@latest`: a scanner that runs on every pull request is a dependency, and `@latest` means one bad @@ -42,7 +42,7 @@ inputs: version that was checked first. threatcrushIntegrity: type: string - default: 'sha512-EKcaxsgiydi7qCH0FhvNviKUpyVi/CImwNS6Kx3IbWMuUjUPCXISAUIzFnYo8BiC+jG9dxfFDMBlwZdhqhwWfQ==' + default: 'sha512-8N3jqCQixK0Onc+/bvuJaNCSvGZlJYZcSAGsd1nEfRZ4kOu1Ifom7Bd1t2muYJAmAxBTPmz1iseWSay/0gg3Gw==' description: >- Subresource-integrity hash of the tarball named by threatcrushPackageSpec, in npm's own `sha512-` form. The workflow downloads, hashes and