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