From f57bd6328b428b9581417ecd10407b8a1adfcaad Mon Sep 17 00:00:00 2001 From: Dave Jong Date: Thu, 27 Aug 2026 11:10:31 +0200 Subject: [PATCH] Prove the engine blocks what the platform generated MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Link 3, and the half of the canary's claim this repository owns: the platform proves "this reviewed recipe plus this map produces these exact rule bytes", and this proves those exact bytes block the exploit and allow a benign control. Neither is worth anything alone — a rule generated correctly and never fired is the defining defect of this product, and it is invisible from the side that generated it. The artifact is VENDORED, not rebuilt. Nothing here parses a recipe, evaluates a detector or binds a coordinate: `rule_v2` and `when` go into the bundle verbatim, exactly as the guard receives them from the rules endpoint. A second implementation of the binding would recreate the duplicate-implementation seam the canary exists to catch, and both sides could then be wrong together. CVE-2017-5941, with the published exploit shape rather than a string lifted from the signature — `_$$ND_FUNC$$_function(){ require("child_process")... }()` is what this vulnerability is actually exploited with, and a payload copied out of the regex would only prove the regex matches itself. Four engine assertions, each load-bearing, confirmed by mutating the handover: exploit blocked wrong parameter → 3 fail; signature widened → still blocks benign request allowed signature widened to /./ → 1 fails another endpoint untouched scope removed → 2 fail; scope on another route → 3 fail dry-run detects without blocking the mode a site starts in; if it blocked, adopting protection would be a traffic change rather than an observation Three more check the handover itself, so a hand-edited artifact cannot pass: the checksum must match its own rule bytes, the recipe hash must be present, and the rule must be scoped and bound. ## Against the built engine, and against what shipped `dist/protect.js` is what an application loads. Testing `src/` would prove the source blocks the exploit and say nothing about the bundle, and the bundler is a real failure surface — an export dropped, a branch shaken out, an edge build diverging. So the PR check runs after the build, and the publish workflow runs the same suite twice: once on the build about to ship, and once on the engine unpacked from the PUBLISHED tarball, via `PS_CANARY_ENGINE`. The second is the only proof that covers packaging: a file left out of `files`, a wrong `exports` map or a bad entry point produces an artifact that passes every local check and cannot protect an application. After the publish rather than before, because the tarball does not exist until then — a failure there cannot unpublish the version, but not knowing is worse. Verified against the real registry: the published 0.3.31 engine blocks the exploit and allows the control. `PS_REQUIRE_CANARY` turns a skip into a failure, following the packaged-bin check. A plain checkout has no `dist/`, so skipping is honest there — but in CI a skipped canary reads exactly like a passing one, and this is the check that proves a generated rule blocks a real exploit. Verified both ways: it refuses to skip with the flag set, and without it the three artifact-integrity tests still run. The fixture ships nowhere: `npm pack --dry-run` finds no match for it. Co-Authored-By: Claude Opus 5 (1M context) --- .github/workflows/ci.yml | 11 ++ .github/workflows/publish.yml | 44 +++++ tests/protect/canary-engine-proof.test.ts | 156 ++++++++++++++++++ .../fixtures/canary/cve-2017-5941.rule.json | 28 ++++ 4 files changed, 239 insertions(+) create mode 100644 tests/protect/canary-engine-proof.test.ts create mode 100644 tests/protect/fixtures/canary/cve-2017-5941.rule.json diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index af32096a..f9e70d70 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -85,6 +85,17 @@ jobs: env: PS_REQUIRE_BIN_CHECK: '1' + # After the build, because it loads `dist/protect.js` — what an application actually loads. Testing + # the source would prove the source blocks the exploit and say nothing about the bundle, and the + # bundler is a real failure surface: an export dropped, a branch shaken out, an edge build diverging. + # + # `PS_REQUIRE_CANARY` turns a skipped run into a failure. A canary that skips reads exactly like one + # that passed, and this is the check that proves a generated rule blocks a real exploit. + - name: Canary — the built engine blocks the exploit and allows the control + run: npx vitest run tests/protect/canary-engine-proof.test.ts + env: + PS_REQUIRE_CANARY: '1' + - name: Verify package contents run: npm pack --dry-run diff --git a/.github/workflows/publish.yml b/.github/workflows/publish.yml index 4413c571..00b316b8 100644 --- a/.github/workflows/publish.yml +++ b/.github/workflows/publish.yml @@ -52,6 +52,13 @@ jobs: - name: Build run: npm run build + # Before publishing anything: does the build about to ship block the exploit the canary is built + # around? `PS_REQUIRE_CANARY` refuses to skip, because a skipped canary reads as a passing one. + - name: Canary — the build about to ship blocks the exploit + run: npx vitest run tests/protect/canary-engine-proof.test.ts + env: + PS_REQUIRE_CANARY: '1' + - name: Verify package contents run: npm pack --dry-run @@ -104,3 +111,40 @@ jobs: - name: Publish to npm run: npm publish --access public --provenance + + # The proof that matters most, and the only one that covers PACKAGING. Everything above tests a build + # in this checkout; this installs the published tarball from the registry and runs the same canary + # against the engine inside it. A file left out of `files`, a wrong `exports` map, or a bad entry + # point produces an artifact that passes every local check and cannot protect an application — which + # is indistinguishable from protection until an attack is not blocked. + # + # After the publish rather than before, because the tarball does not exist until then. A failure here + # cannot unpublish the version; it is a loud signal to deprecate it, which is the honest position: + # the alternative is not knowing. + - name: Canary — the PUBLISHED tarball blocks the exploit + run: | + set -euo pipefail + version='${{ steps.version.outputs.version }}' + + # npm needs a moment before a fresh version resolves; a failure to install here is not a failure + # of the engine and must not be reported as one. + for attempt in 1 2 3 4 5; do + if npm pack "@patchstack/connect@${version}" --pack-destination /tmp >/dev/null 2>&1; then + break + fi + if [ "$attempt" = "5" ]; then + echo "::error::Could not fetch @patchstack/connect@${version} from the registry to verify it. The published engine is UNVERIFIED — this is not evidence that it works." + exit 1 + fi + sleep 15 + done + + mkdir -p /tmp/published && tar -xzf /tmp/patchstack-connect-"${version}".tgz -C /tmp/published + engine=/tmp/published/package/dist/protect.js + + if [ ! -f "$engine" ]; then + echo "::error::The published tarball has no dist/protect.js. Nothing that installs this package can protect anything." + exit 1 + fi + + PS_CANARY_ENGINE="$engine" PS_REQUIRE_CANARY=1 npx vitest run tests/protect/canary-engine-proof.test.ts diff --git a/tests/protect/canary-engine-proof.test.ts b/tests/protect/canary-engine-proof.test.ts new file mode 100644 index 00000000..e1a1fa03 --- /dev/null +++ b/tests/protect/canary-engine-proof.test.ts @@ -0,0 +1,156 @@ +import { describe, expect, it } from 'vitest'; +import { createHash } from 'node:crypto'; +import { existsSync, readFileSync } from 'node:fs'; +import { fileURLToPath } from 'node:url'; +import path from 'node:path'; + +/** + * Link 3 of the cross-repo canary: the engine's half of the claim. + * + * The platform proves "this reviewed recipe plus this map produces these exact rule bytes". This proves + * the other half — those exact bytes block the exploit and allow a benign control. Neither is worth + * anything alone: a rule that is generated correctly and never fires is the defining defect of this + * product, and it is invisible from the side that generated it. + * + * The artifact is VENDORED, not rebuilt. Nothing here parses the recipe, evaluates a detector or binds a + * coordinate — a second implementation of any of that would recreate the duplicate-implementation seam the + * canary exists to catch, and both sides could then be wrong together. `rule_v2` and `when` go into the + * bundle verbatim, exactly as the guard receives them from the rules endpoint. + * + * CVE-2017-5941: `node-serialize`'s `unserialize` executes an embedded `$$ND_FUNC$$` function expression, + * so a request body reaching it is remote code execution. Public, which is why the rule and the requests + * can live here while the recipe stays private in hub. + * + * ## Why this runs against the BUILT engine + * + * `dist/protect.js` is what an application loads. Testing `src/` would prove the source blocks the + * exploit and say nothing about whether the bundle does — and the bundler is a real failure surface: an + * export dropped, a branch shaken out, an edge build diverging. The published tarball gets the same proof + * in the release workflow, so a PR proves the branch and a release proves what shipped. + */ +const root = path.resolve(path.dirname(fileURLToPath(import.meta.url)), '..', '..'); +const artifactPath = path.join(root, 'tests', 'protect', 'fixtures', 'canary', 'cve-2017-5941.rule.json'); + +/** + * The engine under test. `PS_CANARY_ENGINE` lets the release workflow point this at the engine unpacked + * from the published tarball, so one suite covers both "the branch blocks it" and "what shipped blocks + * it" without a second copy of the proof to drift. + */ +const enginePath = process.env.PS_CANARY_ENGINE ?? path.join(root, 'dist', 'protect.js'); + +// `dist/` is gitignored and built on publish, so a plain checkout has nothing to load and skipping is the +// honest answer. In CI it is the opposite: the run that is SUPPOSED to prove the shipped engine blocks a +// real exploit must not quietly prove nothing. `PS_REQUIRE_CANARY` is set by the post-build step and turns +// the skip into a failure — a skipped canary reads exactly like a passing one. +const built = existsSync(enginePath); +const required = process.env.PS_REQUIRE_CANARY === '1'; + +if (required && !built) { + throw new Error( + `PS_REQUIRE_CANARY=1 but ${enginePath} does not exist — this check runs after the build. ` + + 'Refusing to skip, because a skipped canary reads exactly like a passing one.', + ); +} + +const artifact = JSON.parse(readFileSync(artifactPath, 'utf8')); + +/** The bundle shape the rules endpoint serves, with the vendored rule dropped in unchanged. */ +const bundleOf = (rule: Record) => ({ + firewall: [rule], + whitelists: [], + whitelist_keys: {}, +}); + +const URL_BASE = 'https://app.test'; + +/** + * The published exploit shape for CVE-2017-5941. `_$$ND_FUNC$$_` makes `unserialize` evaluate the string + * as a function and invoke it immediately; the payload reaches for `child_process` to run a command. + * + * Not a synthetic string chosen to match the signature — the point is that the rule the platform generated + * catches the shape this vulnerability is actually exploited with. A test that fed it a token lifted from + * the regex would prove the regex matches itself. + */ +const EXPLOIT = JSON.stringify({ + state: '_$$ND_FUNC$$_function (){ require("child_process").exec("id > /tmp/pwned", function(){}); }()', +}); + +/** A legitimate serialized payload for the same endpoint: no require, no process, nothing to execute. */ +const BENIGN = JSON.stringify({ state: '{"cart":[{"sku":"AB-1","qty":2}],"currency":"EUR"}' }); + +const post = (body: string, url = `${URL_BASE}/api/restore`) => + new Request(url, { method: 'POST', headers: { 'content-type': 'application/json' }, body }); + +describe('the vendored canary rule is the one the platform generated', () => { + // Integrity of the handover, checkable without regenerating anything. If the artifact were hand-edited + // here — a signature widened, a parameter changed — the tests below would still pass while the engine + // proved something the platform never produced. + it('carries a checksum matching its own rule bytes', () => { + const digest = `sha256:${createHash('sha256').update(JSON.stringify(artifact.rule)).digest('hex').slice(0, 32)}`; + + expect(digest).toBe(artifact.rule_checksum); + }); + + it('names the recipe it was derived from', () => { + // Not decoration: it is what lets a failure here be traced to a specific reviewed recipe rather than + // to "some rule stopped working". + expect(artifact.cve).toBe('CVE-2017-5941'); + expect(artifact.package).toBe('node-serialize'); + expect(artifact.recipe_hash).toMatch(/^sha256:[0-9a-f]{32}$/); + }); + + it('is scoped and bound, as a generated rule must be', () => { + // Read from the artifact rather than asserted against hard-coded values, so this states the SHAPE the + // engine needs and not a copy of the platform's output. An unscoped generated rule applies everywhere. + expect(artifact.rule.when).toEqual({ method: 'POST', path: '/api/restore' }); + expect(artifact.rule.rule_v2.map((c: any) => c.parameter)).toEqual(['post.state']); + }); +}); + +describe.skipIf(!built)('the built engine, given that rule verbatim', () => { + async function guard(mode: 'block' | 'dry-run' = 'block') { + const { createProtection } = await import(/* @vite-ignore */ enginePath); + const protection: any = await createProtection({ rules: bundleOf(artifact.rule), mode }); + + return protection.fetchGuard(); + } + + it('blocks the published exploit', async () => { + const response = await (await guard())(post(EXPLOIT)); + + expect(response?.status).toBe(403); + }); + + it('allows a benign request to the same endpoint', async () => { + // The false-positive half. A rule that blocks everything is not protection, and every canary that + // omits this proves only that something refuses traffic. + const response = await (await guard())(post(BENIGN)); + + expect(response ?? null).toBeNull(); + }); + + it('leaves another endpoint alone, because the rule is scoped', async () => { + // `when` is the difference between a rule aimed at one generated coordinate and a site-wide filter. + // Without this, a scope silently ignored by the engine would look identical to one honoured. + const response = await (await guard())(post(EXPLOIT, `${URL_BASE}/api/profile`)); + + expect(response ?? null).toBeNull(); + }); + + it('detects but does not block in dry-run, so enrolment stays reversible', async () => { + // The mode a site starts in. If dry-run blocked, adopting protection would be a traffic change rather + // than an observation, and nobody would run it. + const detections: unknown[] = []; + const { createProtection } = await import(/* @vite-ignore */ enginePath); + const protection: any = await createProtection({ + rules: bundleOf(artifact.rule), + mode: 'dry-run', + onDetect: (d: unknown) => detections.push(d), + }); + + const response = await protection.fetchGuard()(post(EXPLOIT)); + + expect(response ?? null).toBeNull(); + expect(detections).toHaveLength(1); + }); +}); diff --git a/tests/protect/fixtures/canary/cve-2017-5941.rule.json b/tests/protect/fixtures/canary/cve-2017-5941.rule.json new file mode 100644 index 00000000..29a4ca24 --- /dev/null +++ b/tests/protect/fixtures/canary/cve-2017-5941.rule.json @@ -0,0 +1,28 @@ +{ + "note": "Generated by this platform from the reviewed recipe and the canary map. Do not edit; regenerate.", + "cve": "CVE-2017-5941", + "package": "node-serialize", + "recipe_hash": "sha256:92cc3e30d5188e20beac841a6df8bd49", + "rule": { + "id": "pulse-canary", + "rule_v2": [ + { + "match": { + "type": "regex", + "value": "/(require\\s*\\(|process\\.|child_process|global\\.|constructor\\s*\\[)/i" + }, + "mutations": [ + "urldecode" + ], + "parameter": "post.state" + } + ], + "source_revision": "1", + "title": "Block code-injection payloads reaching the vulnerable evaluation", + "when": { + "method": "POST", + "path": "/api/restore" + } + }, + "rule_checksum": "sha256:f39b7dca67370f694977a1059b4d0bf9" +}