diff --git a/.github/scripts/tests/lockfile-refresh-merge.test.mjs b/.github/scripts/tests/lockfile-refresh-merge.test.mjs index 3001529c5df..79655b0f1d0 100644 --- a/.github/scripts/tests/lockfile-refresh-merge.test.mjs +++ b/.github/scripts/tests/lockfile-refresh-merge.test.mjs @@ -168,3 +168,20 @@ test("a held refresh round-trips every managed lockfile", () => { rmSync(temp, { recursive: true, force: true }); } }); + +// The SDK public source proof pins the lock too. The bot merges without CI, so +// a refresh that left those pins stale turned master red (#20615). +test("the refresh commit resyncs and checks the SDK public source pins before pushing", () => { + const upsert = step("Create or update pull request"); + const order = [ + 'git commit -m "chore(lockfile): refresh pnpm lockfiles"', + 'node tests/runner-e2e/source-sdk-public-inputs.mjs "$(git rev-parse HEAD)" --write', + "git add tests/runner-e2e/source-sdk-public-inputs.json", + "git commit --amend --no-edit", + 'node tests/runner-e2e/source-sdk-public-inputs.mjs "$(git rev-parse HEAD)" --check', + 'git push --force origin "$BRANCH"', + 'echo "head_sha=$(git rev-parse HEAD)" >> "$GITHUB_OUTPUT"', + ].map((line) => upsert.indexOf(line)); + assert.ok(order.every((index) => index !== -1), `missing resync command: ${order}`); + assert.deepEqual([...order].sort((a, b) => a - b), order, "resync must follow the lock commit and precede the push and head_sha"); +}); diff --git a/.github/workflows/refresh-lockfile.yml b/.github/workflows/refresh-lockfile.yml index 87665452ab8..c0c38edb5fd 100644 --- a/.github/workflows/refresh-lockfile.yml +++ b/.github/workflows/refresh-lockfile.yml @@ -151,6 +151,13 @@ jobs: git checkout -B "$BRANCH" git add $LOCKFILES tests/runner-e2e/report-lock.json tests/runner-e2e/report-lock.patch git commit -m "chore(lockfile): refresh pnpm lockfiles" + # The SDK public source proof pins the committed lock blob and the + # report-lock output hash. This PR merges without CI, so resync those + # pins from the commit above and check them before pushing (#20615). + node tests/runner-e2e/source-sdk-public-inputs.mjs "$(git rev-parse HEAD)" --write + git add tests/runner-e2e/source-sdk-public-inputs.json + git commit --amend --no-edit + node tests/runner-e2e/source-sdk-public-inputs.mjs "$(git rev-parse HEAD)" --check git push --force origin "$BRANCH" # Only reuse an open PR from this repository owner, not a fork with the same branch name. @@ -189,6 +196,7 @@ jobs: - Refresh the dependency lock from the manifests on master. - Refresh the sandbox provider lockfiles from their manifests on master. - Include the matching offline report correction and hashes. + - Resync the SDK public source proof's lock pins from the refreshed lock. ## Verification diff --git a/doc/RELEASE-AUTOMATION-SETUP.md b/doc/RELEASE-AUTOMATION-SETUP.md index 1906df8f9ce..9ea3f95fe4a 100644 --- a/doc/RELEASE-AUTOMATION-SETUP.md +++ b/doc/RELEASE-AUTOMATION-SETUP.md @@ -226,7 +226,7 @@ If `@cryppadotta` is not the right reviewer identity in the public repo, change - this workflow, on its `chore/refresh-lockfile` branch - Dependabot, but only when the lockfile in its pull request matches a verified Dependabot commit -The reporting-lock proof (`tests/runner-e2e/report-lock.json` and `report-lock.patch`) controls the lock that paid evaluation jobs install, so it counts as lock content too. Only this workflow writes it. +The reporting-lock proof (`tests/runner-e2e/report-lock.json` and `report-lock.patch`) controls the lock that paid evaluation jobs install, so it counts as lock content too. Only this workflow writes it. The same refresh commit also resyncs `tests/runner-e2e/source-sdk-public-inputs.json` with `node tests/runner-e2e/source-sdk-public-inputs.mjs --write`, because that registry pins the committed lock blob and copies `lockSha256` from the reviewed `report-lock.json` output. The refresh merges without CI, so a missing resync would turn the Runner support suite red on `master`. `.github/scripts/lockfile-trust.mjs` enforces these rules in the `pr-trusted.yml` policy job. The job runs the copy of the script from the current tip of the base branch, which is the first parent of GitHub's merge commit. A pull request therefore cannot change the check that judges it. The job does not use `pull_request.base.sha`, because GitHub does not update it on every push. To update a Dependabot pull request that is behind `master`, comment `@dependabot recreate`. Do not merge `master` into its branch, because a merged lockfile no longer matches Dependabot's commit. diff --git a/tests/runner-e2e/OPERATIONS.md b/tests/runner-e2e/OPERATIONS.md index 6174437ee69..1ed5f0ec39c 100644 --- a/tests/runner-e2e/OPERATIONS.md +++ b/tests/runner-e2e/OPERATIONS.md @@ -978,7 +978,8 @@ without registry resolution. The worker target and its lock artifact never control report dependencies. An already-reviewed output lock is used unchanged. An unknown committed lock fails before install; review the correction with dependency changes. The existing lockfile-refresh bot runs `prepare-report-lock.mjs --refresh` and includes the correction and hashes in its -lockfile PR. The lockfile bot merges that PR without checks or code-owner review +lockfile PR. The same commit resyncs `source-sdk-public-inputs.json`, whose +`lockSha256` is copied from the committed `report-lock.json` output hash. The lockfile bot merges that PR without checks or code-owner review when every added package version is at least 3 days old on npm; otherwise the PR waits for a maintainer (see `doc/RELEASE-AUTOMATION-SETUP.md` ยง8.1). The bot merges only the commit the workflow generated. It refuses to bless an diff --git a/tests/runner-e2e/source-sdk-public-inputs.mjs b/tests/runner-e2e/source-sdk-public-inputs.mjs index 71acb8169d3..d20306f504a 100644 --- a/tests/runner-e2e/source-sdk-public-inputs.mjs +++ b/tests/runner-e2e/source-sdk-public-inputs.mjs @@ -1,4 +1,6 @@ -// Offline source-pin maintenance only. Runtime/compiler/parser/zod pins are never regenerated here. +// Offline source-pin maintenance only. Compiler/parser/zod pins are never regenerated here. +// lockSha256 is not chosen here either: it is copied from the committed reviewed report-lock +// policy (tests/runner-e2e/report-lock.json), which only the trusted lockfile refresh writes. import { execFileSync } from "node:child_process"; import { createHash } from "node:crypto"; import { lstat, readFile, readdir, realpath, writeFile } from "node:fs/promises"; @@ -6,6 +8,7 @@ import path from "node:path"; import { fileURLToPath } from "node:url"; const registryPath = "tests/runner-e2e/source-sdk-public-inputs.json"; +const reportLockPath = "tests/runner-e2e/report-lock.json"; const sourceRoots = ["packages/shared/src", "packages/plugins/sdk/src"]; const buildInputs = ["package.json", "packages/plugins/sdk/package.json", "packages/plugins/sdk/tsconfig.json", "packages/shared/package.json", "packages/shared/tsconfig.json", "pnpm-lock.yaml", @@ -15,6 +18,23 @@ const blob = bytes => createHash("sha1").update(`blob ${bytes.length}\0`).update const sha256 = bytes => createHash("sha256").update(bytes).digest("hex"); const fail = () => { throw new Error("SDK public input synchronization refused"); }; const git = (root, args) => execFileSync("git", args, { cwd: root, encoding: "utf8", maxBuffer: 4 * 1024 * 1024, stdio: ["ignore", "pipe", "pipe"] }); +const committedBytes = (root, revision, file) => execFileSync("git", ["show", `${revision}:${file}`], { cwd: root, maxBuffer: 64 * 1024 * 1024, stdio: ["ignore", "pipe", "pipe"] }); +const hex64 = value => typeof value === "string" && /^[a-f0-9]{64}$/.test(value); + +/** + * The reviewed lock identity the SDK proof must use at this revision: the committed + * report-lock policy's output hash, admitted only while the committed lock is that + * policy's reviewed input or output (the same rule the reviewed SDK reference applies). + */ +export function reviewedSdkLockSha256(root, revision) { + const policy = JSON.parse(committedBytes(root, revision, reportLockPath).toString("utf8")); + if (!hex64(policy.inputSha256) || !hex64(policy.outputSha256)) fail(); + const committedLock = sha256(committedBytes(root, revision, "pnpm-lock.yaml")); + if (![policy.inputSha256, policy.outputSha256].includes(committedLock)) { + throw new Error(`SDK public input synchronization refused: the committed pnpm-lock.yaml is not reviewed by ${reportLockPath}`); + } + return policy.outputSha256; +} export async function synchronizeSdkPublicInputs(root, revision, { write = false } = {}) { if (!/^[a-f0-9]{40}$/.test(revision) || await realpath(root) !== root || git(root, ["rev-parse", "HEAD"]).trim() !== revision) fail(); @@ -29,6 +49,7 @@ export async function synchronizeSdkPublicInputs(root, revision, { write = false if (!match || match[3].split("/").includes("..") || Object.hasOwn(inputs, match[3])) fail(); inputs[match[3]] = match[2]; } + const lockSha256 = reviewedSdkLockSha256(root, revision); if (buildInputs.some(file => !inputs[file]) || sourceRoots.some(directory => !Object.keys(inputs).some(file => file.startsWith(`${directory}/`)))) fail(); const actualSources = []; const collect = async directory => { @@ -47,9 +68,9 @@ export async function synchronizeSdkPublicInputs(root, revision, { write = false if (!stat.isFile() || stat.isSymbolicLink() || await realpath(filename) !== filename || stat.size > 8 * 1024 * 1024) fail(); const bytes = await readFile(filename); // Preserve the existing, separately authenticated restored-lock exception. - if (blob(bytes) !== expected && !(file === "pnpm-lock.yaml" && sha256(bytes) === baseline.lockSha256)) fail(); + if (blob(bytes) !== expected && !(file === "pnpm-lock.yaml" && sha256(bytes) === lockSha256)) fail(); } - const updated = { ...baseline, inputs: Object.fromEntries(Object.entries(inputs).sort(([a], [b]) => a < b ? -1 : a > b ? 1 : 0)) }; + const updated = { ...baseline, lockSha256, inputs: Object.fromEntries(Object.entries(inputs).sort(([a], [b]) => a < b ? -1 : a > b ? 1 : 0)) }; const output = `${JSON.stringify(updated, null, 2)}\n`; const filename = path.join(root, registryPath), stat = await lstat(filename); if (!stat.isFile() || stat.isSymbolicLink() || await realpath(filename) !== filename) fail(); @@ -58,27 +79,34 @@ export async function synchronizeSdkPublicInputs(root, revision, { write = false if (current !== baselineBytes && current !== output) fail(); if (git(root, ["rev-parse", "HEAD"]).trim() !== revision) fail(); const result = { revision, inputs: Object.keys(inputs).length, changed: current !== output, - registrySha256: sha256(output), runtimePinsChanged: false }; + registrySha256: sha256(output), lockPinChanged: baseline.lockSha256 !== lockSha256, runtimePinsChanged: false }; if (write && result.changed) await writeFile(filename, output); return result; } /** * Cheap drift guard: the source pins in the registry file must equal the blobs - * this checkout committed. Compares Git trees only (no working-tree reads), so - * a stale registry fails with the inputs that changed instead of the opaque - * "SDK public source proof unavailable" raised later by the source proof. + * this checkout committed, and its lockSha256 must equal the committed reviewed + * report-lock output. Compares Git trees only (no working-tree reads), so a + * stale registry fails with the inputs that changed instead of the opaque + * "SDK public source proof unavailable" or "Reviewed SDK reference policy does + * not match public proof" raised later by the source proof. */ export async function staleSdkPublicInputs(root, revision = "HEAD") { - const pinned = JSON.parse(await readFile(path.join(root, registryPath), "utf8")).inputs ?? {}; + const registry = JSON.parse(await readFile(path.join(root, registryPath), "utf8")); + const pinned = registry.inputs ?? {}; const committed = {}; for (const entry of git(root, ["ls-tree", "-rz", revision, "--", ...sourceRoots, ...buildInputs]).split("\0").filter(Boolean)) { const match = /^\d+ blob ([a-f0-9]{40})\t(.+)$/.exec(entry); if (match) committed[match[2]] = match[1]; } - return [...new Set([...Object.keys(pinned), ...Object.keys(committed)])].sort() + const stale = [...new Set([...Object.keys(pinned), ...Object.keys(committed)])].sort() .filter(file => pinned[file] !== committed[file]) .map(file => ({ file, change: !pinned[file] ? "unpinned" : !committed[file] ? "removed" : "changed" })); + // A lockfile refresh rewrites report-lock.json; lockSha256 must follow its output hash. + const policy = JSON.parse(git(root, ["show", `${revision}:${reportLockPath}`])); + if (registry.lockSha256 !== policy.outputSha256) stale.push({ file: `${reportLockPath} outputSha256`, change: "lockSha256 changed" }); + return stale; } if (process.argv[1] && path.resolve(process.argv[1]) === fileURLToPath(import.meta.url)) { diff --git a/tests/runner-e2e/source-sdk-public-inputs.test.mjs b/tests/runner-e2e/source-sdk-public-inputs.test.mjs index 7ed7e009807..98c3cfc68ad 100644 --- a/tests/runner-e2e/source-sdk-public-inputs.test.mjs +++ b/tests/runner-e2e/source-sdk-public-inputs.test.mjs @@ -9,6 +9,9 @@ import { fileURLToPath } from "node:url"; import { staleSdkPublicInputs, synchronizeSdkPublicInputs } from "./source-sdk-public-inputs.mjs"; const registry = "tests/runner-e2e/source-sdk-public-inputs.json"; +const reportLock = "tests/runner-e2e/report-lock.json"; +const reportPolicy = (inputSha256, outputSha256) => `${JSON.stringify({ inputSha256, outputSha256, patchSha256: "0".repeat(64), basis: "synthetic" }, null, 2)}\n`; +const commit = (root, message) => { git(root, "add", "-A"); git(root, "-c", "user.name=Synthetic unit", "-c", "user.email=synthetic@example.invalid", "commit", "-qm", message); return git(root, "rev-parse", "HEAD").trim(); }; const roots = []; const digest = bytes => createHash("sha256").update(bytes).digest("hex"); const blob = bytes => createHash("sha1").update(`blob ${Buffer.byteLength(bytes)}\0`).update(bytes).digest("hex"); @@ -25,6 +28,8 @@ async function fixture() { parserPackageSha256: "b".repeat(64), parserEntrySha256: "c".repeat(64), zodPackageSha256: "d".repeat(64), zodEntrySha256: "e".repeat(64), zodTypeFiles: { "index.d.cts": "f".repeat(64) } }; await put(root, registry, `${JSON.stringify(baseline, null, 2)}\n`); + // The reviewed report-lock policy: committed lock in, authenticated restored lock out. + await put(root, reportLock, reportPolicy(digest("synthetic original pnpm-lock.yaml\n"), baseline.lockSha256)); git(root, "init", "-q"); git(root, "add", "."); git(root, "-c", "user.name=Synthetic unit", "-c", "user.email=synthetic@example.invalid", "commit", "-qm", "synthetic source"); return { root, baseline, revision: git(root, "rev-parse", "HEAD").trim() }; @@ -72,6 +77,38 @@ test("admits only the separately pinned restored lock overlay without changing i await assert.rejects(synchronizeSdkPublicInputs(root, revision)); }); +// A lockfile refresh rewrites the lock and report-lock.json together (#20615). +// The sync carries the new reviewed output hash into lockSha256; nothing else. +test("a lockfile refresh moves lockSha256 to the committed reviewed report-lock output", async () => { + const { root, baseline, revision: first } = await fixture(); + await synchronizeSdkPublicInputs(root, first, { write: true }); + commit(root, "synced"); + const refreshed = "synthetic refreshed pnpm-lock.yaml\n"; + await put(root, "pnpm-lock.yaml", refreshed); + await put(root, reportLock, reportPolicy(baseline.lockSha256, digest(refreshed))); + const revision = commit(root, "lockfile refresh"); + assert.deepEqual(await staleSdkPublicInputs(root), [ + { file: "pnpm-lock.yaml", change: "changed" }, + { file: `${reportLock} outputSha256`, change: "lockSha256 changed" }, + ]); + const result = await synchronizeSdkPublicInputs(root, revision, { write: true }); + assert.equal(result.lockPinChanged, true); assert.equal(result.runtimePinsChanged, false); + const updated = JSON.parse(await readFile(path.join(root, registry), "utf8")); + assert.equal(updated.lockSha256, digest(refreshed)); + assert.equal(updated.inputs["pnpm-lock.yaml"], blob(refreshed)); + const { inputs: _old, lockSha256: _oldLock, ...runtime } = baseline, { inputs: _new, lockSha256: _newLock, ...afterRuntime } = updated; + assert.deepEqual(afterRuntime, runtime); + assert.deepEqual(await staleSdkPublicInputs(root), []); +}); +test("refuses a committed lock that the report-lock policy does not review", async () => { + const { root } = await fixture(); + await put(root, "pnpm-lock.yaml", "synthetic unreviewed lock\n"); + const revision = commit(root, "unreviewed lock"); + const before = await readFile(path.join(root, registry)); + await assert.rejects(synchronizeSdkPublicInputs(root, revision, { write: true }), /not reviewed by tests\/runner-e2e\/report-lock\.json/); + assert.deepEqual(await readFile(path.join(root, registry)), before); +}); + // The pins only prove the SDK public types while they match the committed // source. A PR that edits packages/shared/src (or another pinned input) without // refreshing them breaks every SDK-dependent Runner cell before its server diff --git a/tests/runner-e2e/source-sdk-reviewed-fixture.ts b/tests/runner-e2e/source-sdk-reviewed-fixture.ts index cfdbaafdccf..2eabe2a4571 100644 --- a/tests/runner-e2e/source-sdk-reviewed-fixture.ts +++ b/tests/runner-e2e/source-sdk-reviewed-fixture.ts @@ -38,7 +38,8 @@ export async function prepareReviewedSdkReference(original:string,directory:stri const policyFile="tests/runner-e2e/report-lock.json",patchFile="tests/runner-e2e/report-lock.patch"; const policyBytes=gitFile(original,policyFile),policy=JSON.parse(policyBytes.toString("utf8")); if(!(await readFile(path.join(original,policyFile))).equals(policyBytes)||policy.outputSha256!==pins.lockSha256) - throw new Error("Reviewed SDK reference policy does not match public proof"); + throw new Error("Reviewed SDK reference policy does not match public proof: source-sdk-public-inputs.json lockSha256 must equal the committed report-lock.json outputSha256. " + +"Commit the lock change, then run `node tests/runner-e2e/source-sdk-public-inputs.mjs $(git rev-parse HEAD) --write` and commit the updated registry."); const committed=gitFile(original,"pnpm-lock.yaml"); if(![policy.inputSha256,policy.outputSha256].includes(sha(committed)))throw new Error("Committed SDK reference lock is not reviewed"); for(const [file,expected]of Object.entries(pins.inputs)){