mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-11-09 16:02:36 +01:00
fix(ci): let the lockfile bot own the sandbox provider lockfiles (#19522)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work. > - The repository has a supply-chain rule: contributors do not commit lockfiles. A bot and verified Dependabot commits write them. > - The rule checks only the root `pnpm-lock.yaml`. > - The sandbox providers are outside the root workspace, so they have two more pnpm lockfiles. Pull requests edited them by hand, for example #12791, #18839 and #19489. > - This pull request makes the same rule and the same bot cover those two lockfiles. > - The benefit is one lockfile rule for every managed pnpm lockfile, and no lockfile edits in contributor pull requests. ## Linked Issues or Issue Description Refs #19489 **What existing behavior does this improve?** The lockfile policy and the lockfile refresh bot. **Subsystem affected** CI: `.github/scripts/lockfile-trust.mjs`, `.github/workflows/refresh-lockfile.yml`, and the sandbox provider installs. **Current behavior** `lockfile-trust.mjs` blocks contributor edits to `pnpm-lock.yaml` only. A contributor can edit `scripts/typecheck-sandbox-providers/pnpm-lock.yaml` or `packages/plugins/sandbox-providers/daytona/pnpm-lock.yaml`, and CI passes. The refresh bot does not update these files. `typecheck:test-programs` fails on a stale provider lockfile and tells the contributor to commit a new one. **Proposed behavior** The policy blocks contributor edits to all three lockfiles. The refresh bot regenerates all three after merge. CI resolves a stale provider lockfile in place for the run, as it does for the root lockfile. **Reason and benefit** Lockfile content comes only from the bot or from verified Dependabot commits. Contributors do not need to run pnpm in special folders. **Breaking changes** None. A pull request that edits a provider lockfile now fails the policy check. Remove the lockfile change from the pull request. The bot commits it after merge. ## What Changed - `lockfile-trust.mjs` now exports `LOCKFILES`: the root lockfile, the shared sandbox provider lockfile, and the Daytona lockfile. The policy blocks contributor edits to each of them. A Dependabot pull request must match a verified Dependabot commit for each lockfile that it changes. The `merged` command and the reporting-lock proof still use only the root lockfile. - `check-pr-lockfile.mjs` (advisory check) uses the same list. - `refresh-lockfile.yml`: - A job-level `LOCKFILES` env lists the same three files. A test checks that it equals the policy list. - A new step regenerates the two provider lockfiles with `--resolution-only --ignore-scripts --no-frozen-lockfile`. - The unexpected-changes check, the change check, and `git add` cover all three lockfiles. - The release-age gate checks the added versions in every lockfile. - A held refresh saves all three lockfiles at their repository paths, with a `source-locks.sha256` file. The next run reuses them only if every master lockfile is unchanged. An artifact in the old format is not reused. - The PR title is now `chore(lockfile): refresh pnpm lockfiles`. - `check-lockfile-release-age.mjs` accepts more than one `--base`/`--head` pair. - `scripts/typecheck-test-programs.mjs`: if the frozen install of the provider workspace fails, the script resolves the lockfile in place and installs frozen again. - `scripts/build-standalone-public-packages.mjs`: the same fallback applies to a package with a committed lockfile (Daytona). `release.sh` already restores tracked files when it exits. - `doc/DEVELOPING.md` and `doc/RELEASE-AUTOMATION-SETUP.md` describe the managed lockfiles. They also say that the Pi and Kimi distribution lockfiles stay hand-managed, because they hold attested provider pins. ## Verification - `node --test .github/scripts/tests/*.test.mjs scripts/__tests__/build-standalone-concurrency.test.mjs`: 731 tests pass. New tests cover: - the policy for provider lockfiles, including a real git history with a Dependabot commit and a later hand edit - the workflow and policy lists match - the unexpected-changes check, run with bash against a fixture - a held refresh round trip, and its invalidation when a provider lockfile changes on master - the release-age gate across lockfile pairs - the inline fallback in both install scripts - `node --test scripts/typecheck-test-programs.test.mjs`: the install tests pass. Two tests fail locally only because this worktree has no root `node_modules` (no `tsc`). They also fail without this change. - I ran the new bot commands on master with pnpm 9.15.4. No package version changed. pnpm 9.15.4 removes `libc:` fields and adds `deprecated:` notes, so the first refresh is a metadata-only change. The release-age gate passes it because it adds no versions. - I ran the release-age and held-lock steps locally with bash against this commit. The gate reported `eligible=true`, and `sha256sum --check` accepted the saved hashes. - The workflow YAML parses, and `bash -n` accepts every `run` block. - Not testable before merge: the policy job loads `lockfile-trust.mjs` from the base branch, so the new rule applies only to pull requests opened after this merges. The refresh workflow runs only on master. ## Risks - The release build now resolves a stale Daytona lockfile in place, from the registry. The root release install already resolves without a frozen lockfile, so this is the same level of trust. - The paid runner e2e job still installs Daytona with `--frozen-lockfile`. After a Daytona manifest change merges, its Daytona cases fail until the bot's refresh merges. This matches the rule that paid jobs refuse a new root lockfile until its refresh merges. - Dependabot pull requests for the provider entries in #19489 change a provider lockfile. They pass only if each lockfile matches a verified Dependabot commit, the same as for the root lockfile. - Low risk otherwise: the bot keeps its existing token, environment, and merge rules. ## Model Used - Claude Opus 5.5 (`claude-opus-5-5`) in Claude Code, with tool use (shell, file edits, and git). ## Checklist - [x] I have included a thinking path that traces from project context to this change - [x] I have specified the model used (with version and capability details) - [x] I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work - [x] I have searched GitHub for duplicate or related PRs and linked them above - [x] I have either (a) linked existing issues with `Fixes: #` / `Closes #` / `Refs #` OR (b) described the issue in-PR following the relevant issue template - [x] I have not referenced internal/instance-local Paperclip issues or links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip` URLs) - [x] My branch name describes the change (e.g. `docs/...`, `fix/...`) and contains no internal Paperclip ticket id or instance-derived details - [x] I have run tests locally and they pass - [x] I have added or updated tests where applicable - [x] I have updated relevant documentation to reflect my changes - [x] I have considered and documented any risks above - [x] All Paperclip CI gates are green - [x] Greptile is 5/5 with no open P2s, recommendations, or follow-ups - [x] I will address all Greptile and reviewer comments before requesting merge 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
1 parent
1485d3de1b
commit
e3041f5481
14 files changed
+443
-91
No files matched your search
@@ -1,16 +1,19 @@
|
||||
#!/usr/bin/env node
|
||||
/**
|
||||
* check-lockfile-release-age.mjs
|
||||
* Decides whether the lockfile bot may merge a refreshed pnpm-lock.yaml without review.
|
||||
* Decides whether the lockfile bot may merge refreshed pnpm lockfiles without review.
|
||||
* Every package version that the refresh adds must be published on npm for at least
|
||||
* the minimum age. A version that is too new, or that cannot be verified, holds the
|
||||
* PR for a human.
|
||||
*
|
||||
* Exports: parseLockfilePackages(text), addedPackageVersions(baseText, headText),
|
||||
* addedPackageVersionsAcross(pairs),
|
||||
* checkReleaseAge(entries, { fetchPublishTime, now, minAgeMs, concurrency })
|
||||
*
|
||||
* CLI: node check-lockfile-release-age.mjs --base <file> --head <file> --min-age-days <n>
|
||||
* Writes `eligible=true|false` to GITHUB_OUTPUT and a Markdown report to --report.
|
||||
* CLI: node check-lockfile-release-age.mjs --base <file> --head <file> [--base <file> --head <file> ...]
|
||||
* --min-age-days <n>
|
||||
* Pass one --base/--head pair per lockfile, in the same order. Writes `eligible=true|false`
|
||||
* to GITHUB_OUTPUT and a Markdown report to --report.
|
||||
*/
|
||||
import { appendFileSync, readFileSync, writeFileSync } from 'node:fs';
|
||||
import { fileURLToPath } from 'node:url';
|
||||
@@ -65,6 +68,15 @@ export function addedPackageVersions(baseText, headText) {
|
||||
.map(([key, resolution]) => ({ key, ...splitPackageKey(key), resolution }));
|
||||
}
|
||||
|
||||
/** Added versions across several base/head lockfile pairs, each listed once. */
|
||||
export function addedPackageVersionsAcross(pairs) {
|
||||
const entries = new Map();
|
||||
for (const [baseText, headText] of pairs) {
|
||||
for (const entry of addedPackageVersions(baseText, headText)) entries.set(`${entry.key}\0${entry.resolution}`, entry);
|
||||
}
|
||||
return [...entries.values()].sort((a, b) => a.key.localeCompare(b.key));
|
||||
}
|
||||
|
||||
export async function fetchNpmPublishTime(name, version, fetchImpl = fetch) {
|
||||
const res = await fetchImpl(`https://registry.npmjs.org/${name.replace('/', '%2f')}`, {
|
||||
headers: { Accept: 'application/json' },
|
||||
@@ -135,19 +147,21 @@ export function formatReport(result, minAgeDays) {
|
||||
async function main() {
|
||||
const { values } = parseArgs({
|
||||
options: {
|
||||
base: { type: 'string' },
|
||||
head: { type: 'string' },
|
||||
base: { type: 'string', multiple: true },
|
||||
head: { type: 'string', multiple: true },
|
||||
'min-age-days': { type: 'string' },
|
||||
report: { type: 'string' },
|
||||
},
|
||||
});
|
||||
const minAgeDays = Number(values['min-age-days']);
|
||||
if (!values.base || !values.head || !Number.isFinite(minAgeDays) || minAgeDays < 0) {
|
||||
console.error('Usage: check-lockfile-release-age.mjs --base <file> --head <file> --min-age-days <n> [--report <file>]');
|
||||
const bases = values.base ?? [];
|
||||
const heads = values.head ?? [];
|
||||
if (bases.length === 0 || bases.length !== heads.length || !Number.isFinite(minAgeDays) || minAgeDays < 0) {
|
||||
console.error('Usage: check-lockfile-release-age.mjs --base <file> --head <file> [--base <file> --head <file> ...] --min-age-days <n> [--report <file>]');
|
||||
process.exit(2);
|
||||
}
|
||||
|
||||
const entries = addedPackageVersions(readFileSync(values.base, 'utf8'), readFileSync(values.head, 'utf8'));
|
||||
const entries = addedPackageVersionsAcross(bases.map((base, index) => [readFileSync(base, 'utf8'), readFileSync(heads[index], 'utf8')]));
|
||||
const result = await checkReleaseAge(entries, { minAgeMs: minAgeDays * DAY_MS });
|
||||
const report = formatReport(result, minAgeDays);
|
||||
console.log(report);
|
||||
|
||||
@@ -6,24 +6,24 @@
|
||||
* Export: checkLockfile(files, prAuthor, prBranch) → { passed, failures }
|
||||
*/
|
||||
import { fileURLToPath } from 'node:url';
|
||||
import { DEPENDABOT_LOGIN, LOCKFILE, REFRESH_BRANCH, REFRESH_PR_AUTHOR, REPORT_LOCK_FILES } from './lockfile-trust.mjs';
|
||||
import { DEPENDABOT_LOGIN, LOCKFILES, REFRESH_BRANCH, REFRESH_PR_AUTHOR, REPORT_LOCK_FILES } from './lockfile-trust.mjs';
|
||||
|
||||
export function checkLockfile(files, prAuthor, prBranch) {
|
||||
const changed = files.map(f => f.filename);
|
||||
const lockfileChanged = changed.includes(LOCKFILE);
|
||||
const lockfilesChanged = changed.filter(f => LOCKFILES.includes(f));
|
||||
const proofChanged = changed.filter(f => REPORT_LOCK_FILES.includes(f));
|
||||
if (!lockfileChanged && proofChanged.length === 0) return { passed: true, failures: [] };
|
||||
if (lockfilesChanged.length === 0 && proofChanged.length === 0) return { passed: true, failures: [] };
|
||||
|
||||
if (prAuthor === REFRESH_PR_AUTHOR && prBranch === REFRESH_BRANCH) return { passed: true, failures: [] };
|
||||
if (prAuthor === DEPENDABOT_LOGIN && proofChanged.length === 0) return { passed: true, failures: [] };
|
||||
|
||||
const edited = [lockfileChanged && LOCKFILE, ...proofChanged].filter(Boolean).map(f => `\`${f}\``).join(', ');
|
||||
const edited = [...lockfilesChanged, ...proofChanged].map(f => `\`${f}\``).join(', ');
|
||||
return {
|
||||
passed: false,
|
||||
failures: [
|
||||
`You have changes to ${edited} — \`pr.yml\` will hard-fail this PR because only the lockfile refresh workflow and Dependabot may change lock content. ` +
|
||||
'To fix: run `pnpm install` locally, exclude the lockfile from your commit, push again. ' +
|
||||
'The lockfile is regenerated automatically by the refresh bot on a schedule.',
|
||||
'The refresh bot regenerates every managed lockfile after your PR merges.',
|
||||
],
|
||||
};
|
||||
}
|
||||
|
||||
@@ -2,9 +2,9 @@
|
||||
/**
|
||||
* lockfile-trust.mjs
|
||||
*
|
||||
* Decides who may change the dependency lock. Only trusted environments write it:
|
||||
* Decides who may change the dependency locks (LOCKFILES). Only trusted environments write them:
|
||||
* - the lockfile refresh workflow (`chore/refresh-lockfile`, opened by github-actions[bot]),
|
||||
* - Dependabot, when the exact lockfile bytes come from a verified Dependabot commit.
|
||||
* - Dependabot, when the exact bytes of each lockfile it changes come from a verified Dependabot commit.
|
||||
*
|
||||
* The reporting-lock proof (`tests/runner-e2e/report-lock.json` and `.patch`) can rewrite the
|
||||
* lock that paid evaluation jobs install, so it counts as lock content too. Only the refresh
|
||||
@@ -24,7 +24,16 @@ import { appendFileSync, realpathSync } from 'node:fs';
|
||||
import path from 'node:path';
|
||||
import { fileURLToPath } from 'node:url';
|
||||
|
||||
// The root lockfile. The reporting-lock proof and the `merged` command follow this one only.
|
||||
export const LOCKFILE = 'pnpm-lock.yaml';
|
||||
// Sandbox providers stay out of the root workspace (see doc/DEVELOPING.md), so they have their own
|
||||
// pnpm lockfiles. The refresh workflow regenerates these too, and its LOCKFILES env lists the same
|
||||
// paths. Hand-managed pins, such as the Pi and Kimi distributions, are not listed.
|
||||
export const PROVIDER_LOCKFILES = [
|
||||
'scripts/typecheck-sandbox-providers/pnpm-lock.yaml',
|
||||
'packages/plugins/sandbox-providers/daytona/pnpm-lock.yaml',
|
||||
];
|
||||
export const LOCKFILES = [LOCKFILE, ...PROVIDER_LOCKFILES];
|
||||
export const REPORT_LOCK_FILES = ['tests/runner-e2e/report-lock.json', 'tests/runner-e2e/report-lock.patch'];
|
||||
export const DEPENDABOT_LOGIN = 'dependabot[bot]';
|
||||
export const REFRESH_BRANCH = 'chore/refresh-lockfile';
|
||||
@@ -61,12 +70,14 @@ export function findDependabotLockCommit({ lockBlob, commits, lockBlobAt }) {
|
||||
|
||||
/**
|
||||
* Classify the lock-content change in a pull request.
|
||||
* `dependabotLockCommits` maps each changed lockfile to the verified Dependabot commit that holds
|
||||
* its exact bytes, or to null.
|
||||
* @returns {{ allowed: boolean, writer: 'none' | 'dependabot' | 'refresh-bot', reason: string }}
|
||||
*/
|
||||
export function classifyPullRequest({ changedPaths, author, headRepo, baseRepo, headRef, dependabotLockCommit }) {
|
||||
const lockfileChanged = changedPaths.includes(LOCKFILE);
|
||||
export function classifyPullRequest({ changedPaths, author, headRepo, baseRepo, headRef, dependabotLockCommits = {} }) {
|
||||
const changedLockfiles = LOCKFILES.filter((file) => changedPaths.includes(file));
|
||||
const proofChanged = changedPaths.some((file) => REPORT_LOCK_FILES.includes(file));
|
||||
if (!lockfileChanged && !proofChanged) {
|
||||
if (changedLockfiles.length === 0 && !proofChanged) {
|
||||
return { allowed: true, writer: 'none', reason: 'The pull request does not change lock content.' };
|
||||
}
|
||||
|
||||
@@ -80,14 +91,16 @@ export function classifyPullRequest({ changedPaths, author, headRepo, baseRepo,
|
||||
return { allowed: false, writer: 'dependabot',
|
||||
reason: `Dependabot pull requests must not change ${REPORT_LOCK_FILES.join(' or ')}. The lockfile refresh renews the reporting-lock proof after merge.` };
|
||||
}
|
||||
if (!dependabotLockCommit) {
|
||||
const unverified = changedLockfiles.filter((file) => !dependabotLockCommits[file]);
|
||||
if (unverified.length > 0) {
|
||||
return { allowed: false, writer: 'dependabot',
|
||||
reason: `${LOCKFILE} does not match any verified Dependabot commit in this pull request. ${RECREATE_HINT}` };
|
||||
reason: `${unverified.join(', ')} does not match any verified Dependabot commit in this pull request. ${RECREATE_HINT}` };
|
||||
}
|
||||
return { allowed: true, writer: 'dependabot', reason: `${LOCKFILE} matches verified Dependabot commit ${dependabotLockCommit}.` };
|
||||
return { allowed: true, writer: 'dependabot',
|
||||
reason: changedLockfiles.map((file) => `${file} matches verified Dependabot commit ${dependabotLockCommits[file]}.`).join(' ') };
|
||||
}
|
||||
|
||||
const files = [lockfileChanged && LOCKFILE, ...REPORT_LOCK_FILES.filter((file) => changedPaths.includes(file))].filter(Boolean);
|
||||
const files = [...changedLockfiles, ...REPORT_LOCK_FILES.filter((file) => changedPaths.includes(file))];
|
||||
return { allowed: false, writer: 'none',
|
||||
reason: `Do not commit ${files.join(', ')} in pull requests. CI owns lockfile updates.` };
|
||||
}
|
||||
@@ -146,18 +159,22 @@ function checkPullRequest() {
|
||||
// Diff against the merge base so recent base-branch commits are not counted as PR changes.
|
||||
const changedPaths = run('git', ['diff', '--name-only', `${baseSha}...${headSha}`]).split('\n').filter(Boolean);
|
||||
|
||||
let dependabotLockCommit = null;
|
||||
if (author === DEPENDABOT_LOGIN && changedPaths.includes(LOCKFILE)) {
|
||||
dependabotLockCommit = findDependabotLockCommit({
|
||||
lockBlob: tryRun('git', ['rev-parse', `${headSha}:${LOCKFILE}`]),
|
||||
commits: pullCommits(repository, required('PR_NUMBER')),
|
||||
lockBlobAt: (sha) => tryRun('git', ['rev-parse', `${sha}:${LOCKFILE}`]),
|
||||
});
|
||||
const dependabotLockCommits = {};
|
||||
const changedLockfiles = LOCKFILES.filter((file) => changedPaths.includes(file));
|
||||
if (author === DEPENDABOT_LOGIN && changedLockfiles.length > 0) {
|
||||
const commits = pullCommits(repository, required('PR_NUMBER'));
|
||||
for (const file of changedLockfiles) {
|
||||
dependabotLockCommits[file] = findDependabotLockCommit({
|
||||
lockBlob: tryRun('git', ['rev-parse', `${headSha}:${file}`]),
|
||||
commits,
|
||||
lockBlobAt: (sha) => tryRun('git', ['rev-parse', `${sha}:${file}`]),
|
||||
});
|
||||
}
|
||||
}
|
||||
|
||||
const result = classifyPullRequest({
|
||||
changedPaths, author, headRepo: process.env.HEAD_REPO ?? '', baseRepo: repository,
|
||||
headRef: process.env.HEAD_REF ?? '', dependabotLockCommit,
|
||||
headRef: process.env.HEAD_REF ?? '', dependabotLockCommits,
|
||||
});
|
||||
console.log(result.reason);
|
||||
writeOutputs({ writer: result.writer });
|
||||
|
||||
@@ -1,7 +1,12 @@
|
||||
import { test } from 'node:test';
|
||||
import assert from 'node:assert/strict';
|
||||
import { spawnSync } from 'node:child_process';
|
||||
import { mkdtempSync, rmSync, writeFileSync } from 'node:fs';
|
||||
import { tmpdir } from 'node:os';
|
||||
import path from 'node:path';
|
||||
import {
|
||||
addedPackageVersions,
|
||||
addedPackageVersionsAcross,
|
||||
checkReleaseAge,
|
||||
formatReport,
|
||||
parseLockfilePackages,
|
||||
@@ -66,6 +71,30 @@ test('lists only package versions the refresh adds', () => {
|
||||
]);
|
||||
});
|
||||
|
||||
test('lists the versions each lockfile adds, once', () => {
|
||||
const pairs = [
|
||||
[lockfile(['left-pad@1.3.0']), lockfile(['left-pad@1.3.0', 'shared@2.0.0', 'root-only@1.0.0'])],
|
||||
[lockfile([]), lockfile(['shared@2.0.0', 'provider-only@3.0.0'])],
|
||||
];
|
||||
assert.deepEqual(addedPackageVersionsAcross(pairs).map(entry => entry.key), ['provider-only@3.0.0', 'root-only@1.0.0', 'shared@2.0.0']);
|
||||
});
|
||||
|
||||
test('the CLI rejects unpaired lockfiles before any registry lookup', () => {
|
||||
const root = mkdtempSync(path.join(tmpdir(), 'release-age-cli-'));
|
||||
try {
|
||||
const file = path.join(root, 'pnpm-lock.yaml');
|
||||
writeFileSync(file, lockfile([]));
|
||||
const result = spawnSync(process.execPath, [
|
||||
path.resolve('.github/scripts/check-lockfile-release-age.mjs'),
|
||||
'--base', file, '--head', file, '--base', file, '--min-age-days', '3',
|
||||
], { encoding: 'utf8' });
|
||||
assert.equal(result.status, 2, result.stderr);
|
||||
assert.match(result.stderr, /Usage/);
|
||||
} finally {
|
||||
rmSync(root, { recursive: true, force: true });
|
||||
}
|
||||
});
|
||||
|
||||
test('lists an existing version whose resolution changed', () => {
|
||||
const swapped = ['left-pad@1.3.0', '{tarball: https://example.com/left-pad-1.3.0.tgz}'];
|
||||
assert.deepEqual(addedPackageVersions(lockfile(['left-pad@1.3.0']), lockfile([swapped])), [
|
||||
|
||||
@@ -47,3 +47,16 @@ test('fails when a regular user changes the reporting-lock proof', () => {
|
||||
assert.equal(result.passed, false);
|
||||
assert.ok(result.failures[0].includes('report-lock.patch'));
|
||||
});
|
||||
|
||||
test('fails when a regular user changes a sandbox provider lockfile', () => {
|
||||
for (const file of ['scripts/typecheck-sandbox-providers/pnpm-lock.yaml', 'packages/plugins/sandbox-providers/daytona/pnpm-lock.yaml']) {
|
||||
const result = checkLockfile(makeFiles([file]), 'someuser', 'fix/bug');
|
||||
assert.equal(result.passed, false, file);
|
||||
assert.ok(result.failures[0].includes(file));
|
||||
}
|
||||
});
|
||||
|
||||
test('passes when Dependabot changes a sandbox provider lockfile', () => {
|
||||
const files = makeFiles(['packages/plugins/sandbox-providers/daytona/package.json', 'packages/plugins/sandbox-providers/daytona/pnpm-lock.yaml']);
|
||||
assert.equal(checkLockfile(files, 'dependabot[bot]', 'dependabot/npm_and_yarn/packages/plugins/sandbox-providers/daytona/zod-4.6.6').passed, true);
|
||||
});
|
||||
@@ -1,9 +1,10 @@
|
||||
import test from "node:test";
|
||||
import assert from "node:assert/strict";
|
||||
import { execFileSync, spawnSync } from "node:child_process";
|
||||
import { mkdtempSync, readFileSync, rmSync, writeFileSync } from "node:fs";
|
||||
import { mkdirSync, mkdtempSync, readFileSync, rmSync, writeFileSync } from "node:fs";
|
||||
import { tmpdir } from "node:os";
|
||||
import path from "node:path";
|
||||
import { LOCKFILES } from "../lockfile-trust.mjs";
|
||||
|
||||
const workflow = readFileSync(new URL("../../workflows/refresh-lockfile.yml", import.meta.url), "utf8");
|
||||
const stepNames = [...workflow.matchAll(/^ - name: (.+)$/gm)].map((match) => match[1]);
|
||||
@@ -46,17 +47,18 @@ test("a held refresh keeps its versions through the next master run's artifact",
|
||||
assert.match(reuse, /gh run download "\$run_id" --name held-pnpm-lock/);
|
||||
assert.match(
|
||||
reuse,
|
||||
/if \[ "\$\(sha256sum < pnpm-lock\.yaml\)" != "\$\(cat "\$held\/source-lock\.sha256"\)" \]; then[\s\S]*?exit 0\n fi\n cp "\$held\/pnpm-lock\.yaml" pnpm-lock\.yaml/,
|
||||
"a held lock must only be reused while master still has the lock it was resolved from",
|
||||
/if ! sha256sum --check --status "\$held\/source-locks\.sha256"; then[\s\S]*?exit 0\n fi\n for lock in \$LOCKFILES; do\n cp "\$held\/\$lock" "\$lock"/,
|
||||
"held locks must only be reused while master still has every lock they were resolved from",
|
||||
);
|
||||
assert.match(reuse, /if \[ ! -f "\$held\/source-locks\.sha256" \]; then[\s\S]*?exit 0/, "an older held artifact must not be reused");
|
||||
assert.doesNotMatch(reuse, /chore\/refresh-lockfile/, "the PR branch is writable by others and must not seed resolution");
|
||||
|
||||
for (const name of ["Stage held lockfile", "Save held lockfile for the next refresh"]) {
|
||||
assert.equal(condition(name), "steps.release-age.outputs.eligible == 'false'");
|
||||
}
|
||||
const stage = step("Stage held lockfile");
|
||||
assert.match(stage, /cp pnpm-lock\.yaml "\$RUNNER_TEMP\/held-upload\/pnpm-lock\.yaml"/);
|
||||
assert.match(stage, /sha256sum < "\$RUNNER_TEMP\/base-pnpm-lock\.yaml" > "\$RUNNER_TEMP\/held-upload\/source-lock\.sha256"/);
|
||||
assert.match(stage, /for lock in \$LOCKFILES; do[\s\S]*?cp "\$lock" "\$held\/\$lock"/);
|
||||
assert.match(stage, /\(cd "\$RUNNER_TEMP\/base-locks" && sha256sum \$LOCKFILES\) > "\$held\/source-locks\.sha256"/);
|
||||
const save = step("Save held lockfile for the next refresh");
|
||||
assert.match(save, /name: held-pnpm-lock/);
|
||||
assert.match(save, /path: \$\{\{ runner\.temp \}\}\/held-upload\//);
|
||||
@@ -95,3 +97,74 @@ test("a refresh that renews only the reporting-lock proof still opens a pull req
|
||||
rmSync(root, { recursive: true, force: true });
|
||||
}
|
||||
});
|
||||
|
||||
test("the workflow owns the same lockfiles that the trust policy guards", () => {
|
||||
const env = workflow.match(/^ LOCKFILES: (.+)$/m)?.[1];
|
||||
assert.ok(env, "the refresh job must define LOCKFILES");
|
||||
assert.deepEqual(env.trim().split(/\s+/), LOCKFILES);
|
||||
assert.match(step("Refresh sandbox provider lockfiles"),
|
||||
/pnpm --dir scripts\/typecheck-sandbox-providers install --resolution-only --ignore-scripts --no-frozen-lockfile/);
|
||||
assert.match(step("Refresh sandbox provider lockfiles"),
|
||||
/pnpm --dir packages\/plugins\/sandbox-providers\/daytona install --ignore-workspace --resolution-only --ignore-scripts --no-frozen-lockfile/);
|
||||
assert.ok(stepNames.indexOf("Refresh sandbox provider lockfiles") < stepNames.indexOf("Fail on unexpected file changes"));
|
||||
assert.match(step("Check release age of added package versions"), /for lock in \$LOCKFILES; do[\s\S]*?pairs\+=\(--base "\$base\/\$lock" --head "\$lock"\)/);
|
||||
});
|
||||
|
||||
// Run a workflow step's script with bash, as GitHub would.
|
||||
const runStep = (name, cwd, env) => {
|
||||
const lines = step(name).split("\n");
|
||||
const start = lines.findIndex((line) => line === " run: |") + 1;
|
||||
const script = lines.slice(start).map((line) => line.slice(10)).join("\n");
|
||||
return spawnSync("bash", ["-e", "-c", script], { cwd, encoding: "utf8", env: { ...process.env, LOCKFILES: LOCKFILES.join(" "), ...env } });
|
||||
};
|
||||
|
||||
test("unexpected-change check admits every managed lockfile and nothing else", () => {
|
||||
const root = mkdtempSync(path.join(tmpdir(), "refresh-unexpected-"));
|
||||
try {
|
||||
const git = (...args) => execFileSync("git", args, { cwd: root });
|
||||
for (const file of [...LOCKFILES, "notes.md"]) {
|
||||
mkdirSync(path.dirname(path.join(root, file)), { recursive: true });
|
||||
writeFileSync(path.join(root, file), "old\n");
|
||||
}
|
||||
git("init", "-q");
|
||||
git("add", ".");
|
||||
git("-c", "user.name=Fixture", "-c", "user.email=fixture@example.invalid", "commit", "-qm", "fixture");
|
||||
for (const file of LOCKFILES) writeFileSync(path.join(root, file), "new\n");
|
||||
const allowed = runStep("Fail on unexpected file changes", root, {});
|
||||
assert.equal(allowed.status, 0, allowed.stdout + allowed.stderr);
|
||||
writeFileSync(path.join(root, "notes.md"), "new\n");
|
||||
const blocked = runStep("Fail on unexpected file changes", root, {});
|
||||
assert.equal(blocked.status, 1, blocked.stdout + blocked.stderr);
|
||||
assert.match(blocked.stdout, /notes\.md/);
|
||||
} finally {
|
||||
rmSync(root, { recursive: true, force: true });
|
||||
}
|
||||
});
|
||||
|
||||
test("a held refresh round-trips every managed lockfile", () => {
|
||||
const root = mkdtempSync(path.join(tmpdir(), "refresh-held-"));
|
||||
const temp = mkdtempSync(path.join(tmpdir(), "refresh-held-temp-"));
|
||||
try {
|
||||
for (const file of LOCKFILES) {
|
||||
for (const dir of [root, path.join(temp, "base-locks")]) {
|
||||
mkdirSync(path.dirname(path.join(dir, file)), { recursive: true });
|
||||
writeFileSync(path.join(dir, file), `master ${file}\n`);
|
||||
}
|
||||
writeFileSync(path.join(root, file), `held ${file}\n`);
|
||||
}
|
||||
const staged = runStep("Stage held lockfile", root, { RUNNER_TEMP: temp });
|
||||
assert.equal(staged.status, 0, staged.stderr);
|
||||
const held = path.join(temp, "held-upload");
|
||||
for (const file of LOCKFILES) assert.equal(readFileSync(path.join(held, file), "utf8"), `held ${file}\n`);
|
||||
|
||||
// The next run checks the hashes against master's lockfiles, then copies the held ones in.
|
||||
for (const file of LOCKFILES) writeFileSync(path.join(root, file), `master ${file}\n`);
|
||||
const check = `sha256sum --check --status "${held}/source-locks.sha256"`;
|
||||
assert.equal(spawnSync("bash", ["-c", check], { cwd: root }).status, 0, "unchanged master locks must match the held source hashes");
|
||||
writeFileSync(path.join(root, LOCKFILES[1]), "changed on master\n");
|
||||
assert.notEqual(spawnSync("bash", ["-c", check], { cwd: root }).status, 0, "a changed provider lock must invalidate the held refresh");
|
||||
} finally {
|
||||
rmSync(root, { recursive: true, force: true });
|
||||
rmSync(temp, { recursive: true, force: true });
|
||||
}
|
||||
});
|
||||
@@ -1,7 +1,7 @@
|
||||
import { test } from 'node:test';
|
||||
import assert from 'node:assert/strict';
|
||||
import { execFileSync, spawnSync } from 'node:child_process';
|
||||
import { chmodSync, mkdtempSync, readFileSync, rmSync, symlinkSync, writeFileSync } from 'node:fs';
|
||||
import { chmodSync, mkdirSync, mkdtempSync, readFileSync, rmSync, symlinkSync, writeFileSync } from 'node:fs';
|
||||
import { tmpdir } from 'node:os';
|
||||
import path from 'node:path';
|
||||
import {
|
||||
@@ -10,10 +10,12 @@ import {
|
||||
findMergedDependabotPull,
|
||||
isDependabotCommit,
|
||||
isGitHubSignedCommit,
|
||||
LOCKFILES,
|
||||
PROVIDER_LOCKFILES,
|
||||
} from '../lockfile-trust.mjs';
|
||||
|
||||
const repo = 'paperclipai/paperclip';
|
||||
const base = { author: 'someuser', headRepo: repo, baseRepo: repo, headRef: 'fix/bug', dependabotLockCommit: null };
|
||||
const base = { author: 'someuser', headRepo: repo, baseRepo: repo, headRef: 'fix/bug', dependabotLockCommits: {} };
|
||||
|
||||
test('allows a pull request that does not change lock content', () => {
|
||||
assert.deepEqual(
|
||||
@@ -56,7 +58,7 @@ test('allows the refresh workflow to change the lockfile and its proof', () => {
|
||||
|
||||
test('allows a Dependabot lockfile that matches a verified Dependabot commit', () => {
|
||||
const result = classifyPullRequest({ ...base, author: 'dependabot[bot]', headRef: 'dependabot/npm_and_yarn/zod-4.6.6',
|
||||
changedPaths: ['package.json', 'pnpm-lock.yaml'], dependabotLockCommit: 'abc123' });
|
||||
changedPaths: ['package.json', 'pnpm-lock.yaml'], dependabotLockCommits: { 'pnpm-lock.yaml': 'abc123' } });
|
||||
assert.equal(result.allowed, true);
|
||||
assert.equal(result.writer, 'dependabot');
|
||||
});
|
||||
@@ -68,17 +70,49 @@ test('blocks a Dependabot lockfile that no verified Dependabot commit produced',
|
||||
});
|
||||
|
||||
test('blocks a Dependabot pull request that changes the reporting-lock proof', () => {
|
||||
const result = classifyPullRequest({ ...base, author: 'dependabot[bot]', dependabotLockCommit: 'abc123',
|
||||
const result = classifyPullRequest({ ...base, author: 'dependabot[bot]', dependabotLockCommits: { 'pnpm-lock.yaml': 'abc123' },
|
||||
changedPaths: ['pnpm-lock.yaml', 'tests/runner-e2e/report-lock.json'] });
|
||||
assert.equal(result.allowed, false);
|
||||
});
|
||||
|
||||
test('blocks a Dependabot author from a fork', () => {
|
||||
const result = classifyPullRequest({ ...base, author: 'dependabot[bot]', headRepo: 'someone/paperclip',
|
||||
dependabotLockCommit: 'abc123', changedPaths: ['pnpm-lock.yaml'] });
|
||||
dependabotLockCommits: { 'pnpm-lock.yaml': 'abc123' }, changedPaths: ['pnpm-lock.yaml'] });
|
||||
assert.equal(result.allowed, false);
|
||||
});
|
||||
|
||||
test('guards the sandbox provider lockfiles as well as the root lockfile', () => {
|
||||
assert.deepEqual(LOCKFILES, ['pnpm-lock.yaml', ...PROVIDER_LOCKFILES]);
|
||||
for (const file of PROVIDER_LOCKFILES) {
|
||||
const result = classifyPullRequest({ ...base, changedPaths: [file] });
|
||||
assert.equal(result.allowed, false, file);
|
||||
assert.ok(result.reason.includes(file), result.reason);
|
||||
}
|
||||
});
|
||||
|
||||
test('allows the refresh workflow to change the provider lockfiles', () => {
|
||||
const result = classifyPullRequest({ ...base, author: 'github-actions[bot]', headRef: 'chore/refresh-lockfile',
|
||||
changedPaths: LOCKFILES });
|
||||
assert.equal(result.allowed, true);
|
||||
assert.equal(result.writer, 'refresh-bot');
|
||||
});
|
||||
|
||||
test('allows a Dependabot provider lockfile that matches a verified Dependabot commit', () => {
|
||||
const [file] = PROVIDER_LOCKFILES;
|
||||
const result = classifyPullRequest({ ...base, author: 'dependabot[bot]',
|
||||
changedPaths: ['packages/plugins/sandbox-providers/e2b/package.json', file], dependabotLockCommits: { [file]: 'abc123' } });
|
||||
assert.equal(result.allowed, true);
|
||||
assert.equal(result.writer, 'dependabot');
|
||||
});
|
||||
|
||||
test('blocks a Dependabot pull request when any changed lockfile is unverified', () => {
|
||||
const [file] = PROVIDER_LOCKFILES;
|
||||
const result = classifyPullRequest({ ...base, author: 'dependabot[bot]',
|
||||
changedPaths: ['pnpm-lock.yaml', file], dependabotLockCommits: { 'pnpm-lock.yaml': 'abc123', [file]: null } });
|
||||
assert.equal(result.allowed, false);
|
||||
assert.ok(result.reason.startsWith(`${file} does not match`), result.reason);
|
||||
});
|
||||
|
||||
test('finds only verified Dependabot commits that hold the exact lockfile', () => {
|
||||
const blobs = { a: 'lock-1', b: 'lock-2', c: 'lock-2', d: 'lock-2' };
|
||||
const lockBlobAt = (sha) => blobs[sha] ?? null;
|
||||
@@ -132,7 +166,10 @@ function runPrCheck(build) {
|
||||
try {
|
||||
const git = (...args) => execFileSync('git', args, { cwd: root, encoding: 'utf8' }).trim();
|
||||
const commit = (author, files) => {
|
||||
for (const [file, contents] of Object.entries(files)) writeFileSync(path.join(root, file), contents);
|
||||
for (const [file, contents] of Object.entries(files)) {
|
||||
mkdirSync(path.dirname(path.join(root, file)), { recursive: true });
|
||||
writeFileSync(path.join(root, file), contents);
|
||||
}
|
||||
git('add', '.');
|
||||
git('-c', `user.name=${author}`, '-c', `user.email=${author}@example.invalid`, 'commit', '-qm', author);
|
||||
return git('rev-parse', 'HEAD');
|
||||
@@ -190,6 +227,36 @@ test('pr command blocks a lockfile commit whose Dependabot author another commit
|
||||
assert.match(result.stdout, /Untrusted lockfile change/);
|
||||
});
|
||||
|
||||
test('pr command checks each lockfile a Dependabot pull request changes', () => {
|
||||
const [file] = PROVIDER_LOCKFILES;
|
||||
const admitted = runPrCheck((commit) => {
|
||||
const sha = commit('dependabot', { [file]: 'lock: dependabot\n', 'package.json': '{"zod":"4.6.6"}\n' });
|
||||
return { author: 'dependabot[bot]', commits: [{ sha, login: 'dependabot[bot]', committer: 'web-flow', verified: true }] };
|
||||
});
|
||||
assert.equal(admitted.status, 0, admitted.stdout);
|
||||
assert.match(admitted.output, /^writer=dependabot$/m);
|
||||
|
||||
const edited = runPrCheck((commit) => {
|
||||
const sha = commit('dependabot', { 'pnpm-lock.yaml': 'lock: dependabot\n', [file]: 'lock: dependabot\n' });
|
||||
const edit = commit('maintainer', { [file]: 'lock: edited\n' });
|
||||
return { author: 'dependabot[bot]', commits: [
|
||||
{ sha, login: 'dependabot[bot]', committer: 'web-flow', verified: true },
|
||||
{ sha: edit, login: 'maintainer', committer: 'web-flow', verified: true },
|
||||
] };
|
||||
});
|
||||
assert.equal(edited.status, 1, edited.stdout);
|
||||
assert.ok(edited.stdout.includes(`${file} does not match`), edited.stdout);
|
||||
});
|
||||
|
||||
test('pr command blocks a contributor change to a provider lockfile', () => {
|
||||
const result = runPrCheck((commit) => {
|
||||
commit('someuser', { [PROVIDER_LOCKFILES[1]]: 'lock: contributor\n' });
|
||||
return { author: 'someuser', commits: [] };
|
||||
});
|
||||
assert.equal(result.status, 1);
|
||||
assert.match(result.output, /^writer=none$/m);
|
||||
});
|
||||
|
||||
test('pr command blocks a contributor lockfile change', () => {
|
||||
const result = runPrCheck((commit) => {
|
||||
commit('someuser', { 'pnpm-lock.yaml': 'lock: contributor\n' });
|
||||
|
||||
Reference in new issue
Block a user