From dd936dc3f352c160d9824630eb6b32ae675d3887 Mon Sep 17 00:00:00 2001 From: Omry Yadan Date: Fri, 23 Oct 2026 19:58:41 +0800 Subject: [PATCH] [node-maintenance-repair/2] Show the selected batch in the native Check Reads the selected exact patch updates from a managed maintenance PR and shows them in the shared App's **Dependency repair** Check. The receiver fetches the canonical repository and PR, requires an open bot-authored PR from `maintenance/node-security` in the same repository targeting its current default branch, and writes only its own Check. The C1 parser validates checked and unchecked rows, rejects duplicates and malformed or non-increasing same-minor patches, and preserves version strings. Shared fixtures bind sorted selections, PR number, head and base to the exact SHA-256 fingerprint. PR edits refresh an existing idle Check, including when overlapping initial deliveries created duplicates; a changed head receives a Check bound to that head. Bounded canonical rechecks reconcile edits that arrive during a Check write. Empty or invalid selections cannot expose an action. The token stays restricted to one repository with Contents read, Pull requests read and Checks write. This slice adds no Run action, database, dispatch or dependency repair. It depends on the receiver in #42. Validation: 96 Worker unit tests with 100% line and function coverage; native Workers receiver regression and pinned Wrangler bundle; 1,294 repository tests, Ruff, formatting and Pyrefly. Live selection and head-refresh acceptance is tracked separately for the approved deployment. --- docs/node-maintenance-operator.md | 32 ++- repair-app/selection-fixtures.json | 332 ++++++++++++++++++++++++++ repair-app/worker.mjs | 97 +++++++- repair-app/worker.test.mjs | 370 ++++++++++++++++++++++++++++- 4 files changed, 823 insertions(+), 8 deletions(-) create mode 100644 repair-app/selection-fixtures.json diff --git a/docs/node-maintenance-operator.md b/docs/node-maintenance-operator.md index 05597d9..bcb8479 100644 --- a/docs/node-maintenance-operator.md +++ b/docs/node-maintenance-operator.md @@ -4,8 +4,32 @@ This reference is for ongoing maintenance of the shared GitHub App and Cloudflar service. Repository maintainers install the shared App; they do not host it. The receiver verifies signed webhook deliveries and reads canonical repository -metadata with a Contents-read token restricted to one repository. Check actions, -dispatch, storage and dependency repairs belong to later slices. +metadata with a token restricted to one repository. For managed PRs it also +reads the canonical PR and displays the selected package versions in the App's +**Dependency repair** Check. Check actions, dispatch, storage and dependency +repairs belong to later slices. + +## Check the selected batch + +The PR must be open, bot-authored, from `maintenance/node-security` in the same +repository, and target its current default branch. Its description contains one +selection block: + +```markdown + +- [x] Allow `launch-editor` `2.14.1 → 2.14.2` by updating its explicit pin. + +``` + +Select at most 20 distinct packages. Each change must increase the patch version +within the same major and minor. Unchecked rows must also be valid and unique. +Malformed or missing selections produce a Check that asks for attention. + +Editing selections or changing the PR head refreshes the Check from canonical +GitHub data. The Check displays the exact selected versions, bound to the PR +number, head and base. It has no Run button in this slice. Checkbox edits do not +start a run. The token grants only Contents read, Pull requests read and Checks +write; the service never updates another App's Check. ## Check the receiver locally @@ -18,7 +42,9 @@ npm run test:runtime npm run check ``` -The unit suite reports coverage for the receiver. The runtime test executes it +The unit suite reports coverage for the receiver, selection parser and Check +updates. `selection-fixtures.json` holds the shared C1 cases and expected +fingerprints. The runtime test executes the receiver in local Cloudflare Workers with synthetic credentials and intercepted GitHub requests. The final command bundles the preview with `--dry-run`; it does not deploy. The CI job runs the same checks without App or Cloudflare credentials. diff --git a/repair-app/selection-fixtures.json b/repair-app/selection-fixtures.json new file mode 100644 index 0000000..8460421 --- /dev/null +++ b/repair-app/selection-fixtures.json @@ -0,0 +1,332 @@ +{ + "schema_version": 1, + "pr": { + "number": 12, + "head_sha": "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa", + "base_sha": "bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb" + }, + "cases": [ + { + "name": "one selected patch", + "body": "\n- [x] Allow `launch-editor` `2.14.1 → 2.14.2` by updating its explicit pin.\n", + "items": [ + { + "name": "launch-editor", + "from": "2.14.1", + "to": "2.14.2" + } + ], + "fingerprint": "33b17f82caff79dd2065972e92b5e5b83aa761be9c5799dcbbced028f0f60849" + }, + { + "name": "unchecked item", + "body": "\n- [ ] Allow `launch-editor` `2.14.1 → 2.14.2` by updating its explicit pin.\n", + "items": [], + "fingerprint": "75001f5e4c1d47514f4acf0d5760893f104f10f673e2da7d5be350e2672589f7" + }, + { + "name": "empty block", + "body": "\n", + "items": [], + "fingerprint": "75001f5e4c1d47514f4acf0d5760893f104f10f673e2da7d5be350e2672589f7" + }, + { + "name": "sorted scoped and uppercase check", + "body": "\n- [x] Allow `z-package` `2.14.1 → 2.14.2` by updating its explicit pin.\n- [X] Allow `@scope/pkg` `2.14.1 → 2.14.2` by updating its explicit pin.\n- [ ] Allow `a-package` `2.14.1 → 2.14.2` by updating its explicit pin.\n", + "items": [ + { + "name": "@scope/pkg", + "from": "2.14.1", + "to": "2.14.2" + }, + { + "name": "z-package", + "from": "2.14.1", + "to": "2.14.2" + } + ], + "fingerprint": "26e58144229911e19f502c5bb5e30d795b07fda0646c0092c37710758cd055b7" + }, + { + "name": "CRLF and surrounding prose", + "body": "Before\r\n\r\n- [x] Allow `launch-editor` `2.14.1 → 2.14.2` by updating its explicit pin.\r\n\r\nAfter", + "items": [ + { + "name": "launch-editor", + "from": "2.14.1", + "to": "2.14.2" + } + ], + "fingerprint": "33b17f82caff79dd2065972e92b5e5b83aa761be9c5799dcbbced028f0f60849" + }, + { + "name": "large exact patch strings", + "body": "\n- [x] Allow `launch-editor` `2.14.9007199254740992 → 2.14.9007199254740993` by updating its explicit pin.\n", + "items": [ + { + "name": "launch-editor", + "from": "2.14.9007199254740992", + "to": "2.14.9007199254740993" + } + ], + "fingerprint": "924a2b588ada48ce305ff44de023f469ef6da46a5effdafe8022ff6b36a667d4" + }, + { + "name": "twenty selected", + "body": "\n- [x] Allow `package-00` `2.14.1 → 2.14.2` by updating its explicit pin.\n- [x] Allow `package-01` `2.14.1 → 2.14.2` by updating its explicit pin.\n- [x] Allow `package-02` `2.14.1 → 2.14.2` by updating its explicit pin.\n- [x] Allow `package-03` `2.14.1 → 2.14.2` by updating its explicit pin.\n- [x] Allow `package-04` `2.14.1 → 2.14.2` by updating its explicit pin.\n- [x] Allow `package-05` `2.14.1 → 2.14.2` by updating its explicit pin.\n- [x] Allow `package-06` `2.14.1 → 2.14.2` by updating its explicit pin.\n- [x] Allow `package-07` `2.14.1 → 2.14.2` by updating its explicit pin.\n- [x] Allow `package-08` `2.14.1 → 2.14.2` by updating its explicit pin.\n- [x] Allow `package-09` `2.14.1 → 2.14.2` by updating its explicit pin.\n- [x] Allow `package-10` `2.14.1 → 2.14.2` by updating its explicit pin.\n- [x] Allow `package-11` `2.14.1 → 2.14.2` by updating its explicit pin.\n- [x] Allow `package-12` `2.14.1 → 2.14.2` by updating its explicit pin.\n- [x] Allow `package-13` `2.14.1 → 2.14.2` by updating its explicit pin.\n- [x] Allow `package-14` `2.14.1 → 2.14.2` by updating its explicit pin.\n- [x] Allow `package-15` `2.14.1 → 2.14.2` by updating its explicit pin.\n- [x] Allow `package-16` `2.14.1 → 2.14.2` by updating its explicit pin.\n- [x] Allow `package-17` `2.14.1 → 2.14.2` by updating its explicit pin.\n- [x] Allow `package-18` `2.14.1 → 2.14.2` by updating its explicit pin.\n- [x] Allow `package-19` `2.14.1 → 2.14.2` by updating its explicit pin.\n", + "items": [ + { + "name": "package-00", + "from": "2.14.1", + "to": "2.14.2" + }, + { + "name": "package-01", + "from": "2.14.1", + "to": "2.14.2" + }, + { + "name": "package-02", + "from": "2.14.1", + "to": "2.14.2" + }, + { + "name": "package-03", + "from": "2.14.1", + "to": "2.14.2" + }, + { + "name": "package-04", + "from": "2.14.1", + "to": "2.14.2" + }, + { + "name": "package-05", + "from": "2.14.1", + "to": "2.14.2" + }, + { + "name": "package-06", + "from": "2.14.1", + "to": "2.14.2" + }, + { + "name": "package-07", + "from": "2.14.1", + "to": "2.14.2" + }, + { + "name": "package-08", + "from": "2.14.1", + "to": "2.14.2" + }, + { + "name": "package-09", + "from": "2.14.1", + "to": "2.14.2" + }, + { + "name": "package-10", + "from": "2.14.1", + "to": "2.14.2" + }, + { + "name": "package-11", + "from": "2.14.1", + "to": "2.14.2" + }, + { + "name": "package-12", + "from": "2.14.1", + "to": "2.14.2" + }, + { + "name": "package-13", + "from": "2.14.1", + "to": "2.14.2" + }, + { + "name": "package-14", + "from": "2.14.1", + "to": "2.14.2" + }, + { + "name": "package-15", + "from": "2.14.1", + "to": "2.14.2" + }, + { + "name": "package-16", + "from": "2.14.1", + "to": "2.14.2" + }, + { + "name": "package-17", + "from": "2.14.1", + "to": "2.14.2" + }, + { + "name": "package-18", + "from": "2.14.1", + "to": "2.14.2" + }, + { + "name": "package-19", + "from": "2.14.1", + "to": "2.14.2" + } + ], + "fingerprint": "7833cd0924acca54c2fea66a9bcd2a11a24d63e7932aaa3e4442d7810f849de9" + }, + { + "name": "missing", + "body": null, + "invalid": true + }, + { + "name": "no markers", + "body": "hello", + "invalid": true + }, + { + "name": "reversed", + "body": "\n", + "invalid": true + }, + { + "name": "inline start", + "body": "prefix \n- [x] Allow `launch-editor` `2.14.1 → 2.14.2` by updating its explicit pin.\n", + "invalid": true + }, + { + "name": "inline end", + "body": "\n- [x] Allow `launch-editor` `2.14.1 → 2.14.2` by updating its explicit pin.\n suffix", + "invalid": true + }, + { + "name": "repeated start", + "body": "\n\n- [x] Allow `launch-editor` `2.14.1 → 2.14.2` by updating its explicit pin.\n", + "invalid": true + }, + { + "name": "repeated end", + "body": "\n- [x] Allow `launch-editor` `2.14.1 → 2.14.2` by updating its explicit pin.\n\n", + "invalid": true + }, + { + "name": "duplicate checked", + "body": "\n- [x] Allow `launch-editor` `2.14.1 → 2.14.2` by updating its explicit pin.\n- [x] Allow `launch-editor` `2.14.1 → 2.14.2` by updating its explicit pin.\n", + "invalid": true + }, + { + "name": "duplicate unchecked", + "body": "\n- [ ] Allow `launch-editor` `2.14.1 → 2.14.2` by updating its explicit pin.\n- [ ] Allow `launch-editor` `2.14.1 → 2.14.2` by updating its explicit pin.\n", + "invalid": true + }, + { + "name": "duplicate mixed", + "body": "\n- [x] Allow `launch-editor` `2.14.1 → 2.14.2` by updating its explicit pin.\n- [ ] Allow `launch-editor` `2.14.1 → 2.14.2` by updating its explicit pin.\n", + "invalid": true + }, + { + "name": "blank line", + "body": "\n\n- [x] Allow `launch-editor` `2.14.1 → 2.14.2` by updating its explicit pin.\n", + "invalid": true + }, + { + "name": "wrong grammar", + "body": "\n- [x] Upgrade `launch-editor` `2.14.1 → 2.14.2` by updating its explicit pin.\n", + "invalid": true + }, + { + "name": "wrong arrow", + "body": "\n- [x] Allow `launch-editor` `2.14.1 -> 2.14.2` by updating its explicit pin.\n", + "invalid": true + }, + { + "name": "trailing space", + "body": "\n- [x] Allow `launch-editor` `2.14.1 → 2.14.2` by updating its explicit pin. \n", + "invalid": true + }, + { + "name": "uppercase package", + "body": "\n- [x] Allow `Package` `2.14.1 → 2.14.2` by updating its explicit pin.\n", + "invalid": true + }, + { + "name": "bad scope", + "body": "\n- [x] Allow `@scope/Pkg` `2.14.1 → 2.14.2` by updating its explicit pin.\n", + "invalid": true + }, + { + "name": "leading dot", + "body": "\n- [x] Allow `.pkg` `2.14.1 → 2.14.2` by updating its explicit pin.\n", + "invalid": true + }, + { + "name": "leading underscore", + "body": "\n- [x] Allow `_pkg` `2.14.1 → 2.14.2` by updating its explicit pin.\n", + "invalid": true + }, + { + "name": "npm name too long", + "body": "\n- [x] Allow `ppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppppp` `2.14.1 → 2.14.2` by updating its explicit pin.\n", + "invalid": true + }, + { + "name": "major jump", + "body": "\n- [x] Allow `launch-editor` `2.14.1 → 3.14.2` by updating its explicit pin.\n", + "invalid": true + }, + { + "name": "minor jump", + "body": "\n- [x] Allow `launch-editor` `2.14.1 → 2.15.2` by updating its explicit pin.\n", + "invalid": true + }, + { + "name": "equal version", + "body": "\n- [x] Allow `launch-editor` `2.14.1 → 2.14.1` by updating its explicit pin.\n", + "invalid": true + }, + { + "name": "downgrade", + "body": "\n- [x] Allow `launch-editor` `2.14.3 → 2.14.2` by updating its explicit pin.\n", + "invalid": true + }, + { + "name": "leading zero", + "body": "\n- [x] Allow `launch-editor` `2.14.01 → 2.14.2` by updating its explicit pin.\n", + "invalid": true + }, + { + "name": "prerelease", + "body": "\n- [x] Allow `launch-editor` `2.14.1 → 2.14.2-beta.1` by updating its explicit pin.\n", + "invalid": true + }, + { + "name": "build metadata", + "body": "\n- [x] Allow `launch-editor` `2.14.1 → 2.14.2+meta` by updating its explicit pin.\n", + "invalid": true + }, + { + "name": "invalid unchecked patch", + "body": "\n- [ ] Allow `launch-editor` `2.14.1 → 2.15.2` by updating its explicit pin.\n", + "invalid": true + }, + { + "name": "twenty one selected", + "body": "\n- [x] Allow `package-00` `2.14.1 → 2.14.2` by updating its explicit pin.\n- [x] Allow `package-01` `2.14.1 → 2.14.2` by updating its explicit pin.\n- [x] Allow `package-02` `2.14.1 → 2.14.2` by updating its explicit pin.\n- [x] Allow `package-03` `2.14.1 → 2.14.2` by updating its explicit pin.\n- [x] Allow `package-04` `2.14.1 → 2.14.2` by updating its explicit pin.\n- [x] Allow `package-05` `2.14.1 → 2.14.2` by updating its explicit pin.\n- [x] Allow `package-06` `2.14.1 → 2.14.2` by updating its explicit pin.\n- [x] Allow `package-07` `2.14.1 → 2.14.2` by updating its explicit pin.\n- [x] Allow `package-08` `2.14.1 → 2.14.2` by updating its explicit pin.\n- [x] Allow `package-09` `2.14.1 → 2.14.2` by updating its explicit pin.\n- [x] Allow `package-10` `2.14.1 → 2.14.2` by updating its explicit pin.\n- [x] Allow `package-11` `2.14.1 → 2.14.2` by updating its explicit pin.\n- [x] Allow `package-12` `2.14.1 → 2.14.2` by updating its explicit pin.\n- [x] Allow `package-13` `2.14.1 → 2.14.2` by updating its explicit pin.\n- [x] Allow `package-14` `2.14.1 → 2.14.2` by updating its explicit pin.\n- [x] Allow `package-15` `2.14.1 → 2.14.2` by updating its explicit pin.\n- [x] Allow `package-16` `2.14.1 → 2.14.2` by updating its explicit pin.\n- [x] Allow `package-17` `2.14.1 → 2.14.2` by updating its explicit pin.\n- [x] Allow `package-18` `2.14.1 → 2.14.2` by updating its explicit pin.\n- [x] Allow `package-19` `2.14.1 → 2.14.2` by updating its explicit pin.\n- [x] Allow `package-20` `2.14.1 → 2.14.2` by updating its explicit pin.\n", + "invalid": true + }, + { + "name": "reserved npm name node_modules", + "body": "\n- [x] Allow `node_modules` `2.14.1 → 2.14.2` by updating its explicit pin.\n", + "invalid": true + }, + { + "name": "reserved npm name favicon.ico", + "body": "\n- [x] Allow `favicon.ico` `2.14.1 → 2.14.2` by updating its explicit pin.\n", + "invalid": true + } + ] +} diff --git a/repair-app/worker.mjs b/repair-app/worker.mjs index 935d847..00d88ee 100644 --- a/repair-app/worker.mjs +++ b/repair-app/worker.mjs @@ -2,6 +2,41 @@ const encoder = new TextEncoder(); const MAX_BODY_BYTES = 2_000_000; const DELIVERY_ID = /^[a-f0-9]{8}(?:-[a-f0-9]{4}){3}-[a-f0-9]{12}$/; const PR_ACTIONS = ['opened', 'edited', 'synchronize', 'reopened']; +const START = ''; +const END = ''; +const VERSION = '(?:0|[1-9][0-9]*)\\.(?:0|[1-9][0-9]*)\\.(?:0|[1-9][0-9]*)'; +const PACKAGE = '(?:@[a-z0-9][a-z0-9._-]*/)?[a-z0-9][a-z0-9._-]*'; +const LINE = new RegExp('^- \\[([ xX])\\] Allow `(' + PACKAGE + ')` `(' + VERSION + ') → (' + VERSION + ')` by updating its explicit pin\\.$'); +const CHECK = 'Dependency repair'; +const MAX_RECONCILIATIONS = 3; + +export function selections(body) { + if (typeof body !== 'string' || body.split(START).length !== 2 || body.split(END).length !== 2) { + throw new Error('Expected one selection block'); + } + const lines = body.split(/\r?\n/), start = lines.indexOf(START), end = lines.indexOf(END); + if (start < 0 || end <= start) throw new Error('Invalid selection markers'); + const items = [], names = new Set(); + for (const line of lines.slice(start + 1, end)) { + const match = LINE.exec(line); + if (!match || match[2].length > 214 || ['node_modules', 'favicon.ico'].includes(match[2]) || + names.has(match[2])) throw new Error('Invalid selection line'); + names.add(match[2]); + const from = match[3].split('.'), to = match[4].split('.'); + if (from[0] !== to[0] || from[1] !== to[1] || BigInt(from[2]) >= BigInt(to[2])) { + throw new Error('Only increasing same-minor patches are allowed'); + } + if (match[1].toLowerCase() === 'x') items.push({name: match[2], from: match[3], to: match[4]}); + } + if (items.length > 20) throw new Error('Too many selected fixes'); + return items.sort((a, b) => a.name < b.name ? -1 : a.name > b.name ? 1 : 0); +} + +export async function selectionFingerprint(pr, items) { + const bytes = encoder.encode(JSON.stringify([pr.number, pr.head.sha, pr.base.sha, items])); + return [...new Uint8Array(await crypto.subtle.digest('SHA-256', bytes))] + .map(value => value.toString(16).padStart(2, '0')).join(''); +} async function readBody(request, signal) { signal.throwIfAborted(); @@ -107,12 +142,12 @@ export async function api(token, path, method = 'GET', body, signal = AbortSigna try { return await response.json(); } catch { throw new Error('Invalid GitHub response'); } } -export async function installationToken(env, installationID, repositoryID, signal) { +export async function installationToken(env, installationID, repositoryID, signal, permissions = {contents: 'read'}) { if (![installationID, repositoryID].every(id => Number.isSafeInteger(id) && id > 0)) { throw new Error('Invalid installation or repository ID'); } const result = await api(await appJWT(env), `/app/installations/${installationID}/access_tokens`, 'POST', { - repository_ids: [repositoryID], permissions: {contents: 'read'}, + repository_ids: [repositoryID], permissions, }, signal); if (typeof result?.token !== 'string' || !result.token.trim()) throw new Error('Invalid installation token response'); return result.token; @@ -123,11 +158,65 @@ async function readRepository(payload, env, signal) { if (typeof repository?.full_name !== 'string' || !/^[A-Za-z0-9_.-]+\/[A-Za-z0-9_.-]+$/.test(repository.full_name)) { throw new Error('Invalid repository name'); } - const token = await installationToken(env, payload.installation?.id, repository.id, signal); + const number = payload.number; + const hasPR = Number.isSafeInteger(number) && number > 0; + const permissions = hasPR ? {contents: 'read', pull_requests: 'read', checks: 'write'} : {contents: 'read'}; + const token = await installationToken(env, payload.installation?.id, repository.id, signal, permissions); const current = await api(token, `/repos/${repository.full_name}`, 'GET', undefined, signal); if (current?.id !== repository.id || current.full_name?.toLowerCase() !== repository.full_name.toLowerCase()) { throw new Error('Repository identity changed'); } + if (hasPR) await refreshSelectionCheck(token, current, number, env.APP_ID, signal); +} + +async function refreshSelectionCheck(token, repository, number, appID, signal) { + const prefix = `/repos/${repository.full_name}`; + for (let attempt = 0; attempt < MAX_RECONCILIATIONS; attempt++) { + const pr = await api(token, `${prefix}/pulls/${number}`, 'GET', undefined, signal); + if (typeof repository.default_branch !== 'string' || !repository.default_branch || + pr?.number !== number || pr.state !== 'open' || pr.user?.type !== 'Bot' || + pr.head?.repo?.id !== repository.id || pr.base?.repo?.id !== repository.id || + pr.head.ref !== 'maintenance/node-security' || pr.base.ref !== repository.default_branch || + !/^[a-f0-9]{40}$/.test(pr.head.sha || '') || !/^[a-f0-9]{40}$/.test(pr.base.sha || '')) return; + let items = [], valid = true; + try { items = selections(pr.body); } catch { valid = false; } + const external = `node-repair:${number}:${await selectionFingerprint(pr, items)}`; + const state = `${valid ? 'valid' : 'invalid'}:${external}`; + const list = await api(token, `${prefix}/commits/${pr.head.sha}/check-runs?check_name=Dependency%20repair&app_id=${appID}&filter=all&per_page=100`, 'GET', undefined, signal); + if (!Array.isArray(list?.check_runs) || !Number.isSafeInteger(list.total_count) || + list.total_count < 0 || list.total_count > 100 || list.total_count !== list.check_runs.length) { + throw new Error('Incomplete Check list'); + } + const owned = list.check_runs.filter(check => String(check.app?.id) === String(appID) && + check.name === CHECK && check.head_sha === pr.head.sha && + new RegExp(`^node-repair:${number}:[a-f0-9]{64}$`).test(check.external_id || '')); + if (owned.some(check => !Number.isSafeInteger(check.id) || check.id <= 0 || check.status !== 'completed')) { + throw new Error('Repair Check is not idle'); + } + const check = owned.reduce((newest, candidate) => !newest || candidate.id > newest.id ? candidate : newest, undefined); + const output = { + title: !valid ? 'Repair selections need attention' : items.length ? `${items.length} selected fix${items.length === 1 ? '' : 'es'}` : 'No fixes selected', + summary: !valid ? 'Use one valid selection block in the PR description. No run was started.' : items.length ? + items.map(item => `- \`${item.name}\` \`${item.from} → ${item.to}\``).join('\n') + + '\n\nSelections are displayed for review. Running fixes is not enabled yet.' : + 'Select the allowed fixes in the PR description. Editing checkboxes does not start a run.', + }; + await api(token, `${prefix}/check-runs${check ? `/${check.id}` : ''}`, check ? 'PATCH' : 'POST', { + ...(check ? {} : {name: CHECK, head_sha: pr.head.sha}), external_id: external, + status: 'completed', conclusion: 'neutral', output, actions: [], + }, signal); + const current = await api(token, `${prefix}/pulls/${number}`, 'GET', undefined, signal); + if (typeof repository.default_branch !== 'string' || !repository.default_branch || + current?.number !== number || current.state !== 'open' || current.user?.type !== 'Bot' || + current.head?.repo?.id !== repository.id || current.base?.repo?.id !== repository.id || + current.head.ref !== 'maintenance/node-security' || current.base.ref !== repository.default_branch || + !/^[a-f0-9]{40}$/.test(current.head.sha || '') || !/^[a-f0-9]{40}$/.test(current.base.sha || '')) return; + let currentItems = [], currentValid = true; + try { currentItems = selections(current.body); } catch { currentValid = false; } + const currentExternal = `node-repair:${number}:${await selectionFingerprint(current, currentItems)}`; + if (`${currentValid ? 'valid' : 'invalid'}:${currentExternal}` === state) return; + } + throw new Error('Repair Check freshness did not settle'); } export default { @@ -146,7 +235,7 @@ export default { if (!DELIVERY_ID.test(request.headers.get('x-github-delivery') || '')) throw new Error('Invalid delivery ID'); const payload = JSON.parse(new TextDecoder('utf-8', {fatal: true}).decode(body)); if (!payload || typeof payload !== 'object' || Array.isArray(payload)) throw new Error('Invalid payload'); - // Slice 1 authenticates and reads metadata only. Later slices own Checks and dispatch. + // PR edits refresh the selected batch only. Later slices own actions and dispatch. if (request.headers.get('x-github-event') === 'pull_request' && PR_ACTIONS.includes(payload.action)) { await readRepository(payload, env, controller.signal); } diff --git a/repair-app/worker.test.mjs b/repair-app/worker.test.mjs index 671745b..80175db 100644 --- a/repair-app/worker.test.mjs +++ b/repair-app/worker.test.mjs @@ -1,7 +1,8 @@ import assert from 'node:assert/strict'; import {createHmac, createPrivateKey, generateKeyPairSync, verify} from 'node:crypto'; import {beforeEach, test} from 'node:test'; -import worker, {api, installationToken, verifySignature} from './worker.mjs'; +import {readFileSync} from 'node:fs'; +import worker, {api, installationToken, selections, selectionFingerprint, verifySignature} from './worker.mjs'; const keys = generateKeyPairSync('rsa', {modulusLength: 2048, privateKeyEncoding: {type: 'pkcs8', format: 'pem'}}); @@ -260,3 +261,370 @@ test('network and invalid upstream JSON errors stay sanitized in the client and fetch.mock.mockImplementation(async () => new Response('private-invalid-json')); await assert.rejects(api(token, '/repos/fixture/repo'), /^Error: Invalid GitHub response$/); }); + +const fixtures = JSON.parse(readFileSync(new URL('./selection-fixtures.json', import.meta.url), 'utf8')); +const selectionPR = {number: fixtures.pr.number, head: {sha: fixtures.pr.head_sha}, base: {sha: fixtures.pr.base_sha}}; +for (const fixture of fixtures.cases) { + test(`C1 fixture: ${fixture.name}`, async () => { + if (fixture.invalid) assert.throws(() => selections(fixture.body)); + else { + const items = selections(fixture.body); + assert.deepEqual(items, fixture.items); + assert.equal(await selectionFingerprint(selectionPR, items), fixture.fingerprint); + } + }); +} + +test('C1 fingerprint binds the PR, head, base and sorted selected versions', async () => { + const items = selections(fixtures.cases[0].body); + const original = await selectionFingerprint(selectionPR, items); + for (const changed of [{...selectionPR, number: 13}, {...selectionPR, head: {sha: 'c'.repeat(40)}}, + {...selectionPR, base: {sha: 'c'.repeat(40)}}]) { + assert.notEqual(await selectionFingerprint(changed, items), original); + } + assert.notEqual(await selectionFingerprint(selectionPR, [{...items[0], to: '2.14.3'}]), original); +}); + +function checkMock({changePR, list = {total_count: 0, check_runs: []}, failPath} = {}) { + const repository = {id: 7, full_name: 'fixture/repo', default_branch: 'main'}; + const pr = {number: 12, state: 'open', user: {type: 'Bot'}, body: fixtures.cases[0].body, + head: {sha: 'a'.repeat(40), ref: 'maintenance/node-security', repo: {id: 7}}, + base: {sha: 'b'.repeat(40), ref: 'main', repo: {id: 7}}}; + changePR?.(pr, repository); + const calls = []; + fetch.mock.mockImplementation(async (url, options) => { + const path = new URL(url).pathname; + const body = options.body === undefined ? undefined : JSON.parse(options.body); + calls.push({path, url, method: options.method, body, signal: options.signal}); + assert.equal(options.redirect, 'manual'); + if (path === failPath) return new Response('private upstream failure', {status: 403}); + if (path === '/app/installations/3/access_tokens') return json({token}); + assert.equal(options.headers.Authorization, `Bearer ${token}`); + if (path === '/repos/fixture/repo') return json(repository); + if (path === '/repos/fixture/repo/pulls/12') return json(pr); + if (path === `/repos/fixture/repo/commits/${pr.head.sha}/check-runs`) { + const query = new URL(url).searchParams; + assert.equal(query.get('app_id'), env.APP_ID); + assert.equal(query.get('filter'), 'all'); + assert.equal(query.get('check_name'), 'Dependency repair'); + assert.equal(query.get('per_page'), '100'); + return json(list); + } + if (path === '/repos/fixture/repo/check-runs' || path === '/repos/fixture/repo/check-runs/77' || + path === '/repos/fixture/repo/check-runs/78') return json({id: 77}); + throw new Error(`Unexpected API path: ${path}`); + }); + return {calls, pr, repository}; +} +const ownCheck = (overrides = {}) => ({id: 77, app: {id: 42}, name: 'Dependency repair', + head_sha: 'a'.repeat(40), external_id: 'node-repair:12:' + '0'.repeat(64), status: 'completed', ...overrides}); +const listOf = (...checks) => ({total_count: checks.length, check_runs: checks}); +const checkWrites = calls => calls.filter(call => call.path.includes('/check-runs') && call.method !== 'GET'); +const managedPR = (body, headSha = 'a'.repeat(40), state = 'open') => ({number: 12, state, user: {type: 'Bot'}, body, + head: {sha: headSha, ref: 'maintenance/node-security', repo: {id: 7}}, + base: {sha: 'b'.repeat(40), ref: 'main', repo: {id: 7}}}); + +test('canonical bot PR creates an App Check with exact selections and no action or dispatch', async () => { + const {calls, pr} = checkMock(); + // Event-local PR identity and body are not used as approval evidence. + const event = {...payload, number: 12, pull_request: {user: {type: 'User'}, head: {ref: 'other'}, body: 'forged'}}; + assert.equal((await worker.fetch(signed(event, 'pull_request'), env)).status, 200); + assert.deepEqual(calls[0].body, {repository_ids: [7], permissions: {contents: 'read', pull_requests: 'read', checks: 'write'}}); + assert.ok(calls.every(call => call.signal === calls[0].signal)); + const writes = checkWrites(calls); assert.equal(writes.length, 1); + assert.equal(writes[0].method, 'POST'); + assert.deepEqual(writes[0].body, {name: 'Dependency repair', head_sha: pr.head.sha, + external_id: `node-repair:12:${await selectionFingerprint(pr, selections(pr.body))}`, + status: 'completed', conclusion: 'neutral', output: {title: '1 selected fix', + summary: '- `launch-editor` `2.14.1 → 2.14.2`\n\nSelections are displayed for review. Running fixes is not enabled yet.'}, actions: []}); + assert.equal(calls.length, 6); +}); + +for (const action of ['opened', 'edited', 'synchronize', 'reopened']) { + test(`managed PR ${action} refreshes its own idle Check`, async () => { + const {calls} = checkMock({list: listOf(ownCheck())}); + assert.equal((await worker.fetch(signed({...payload, action, number: 12}, 'pull_request'), env)).status, 200); + const writes = checkWrites(calls); assert.equal(writes.length, 1); + assert.equal(writes[0].path, '/repos/fixture/repo/check-runs/77'); + assert.equal(writes[0].method, 'PATCH'); + assert.deepEqual(writes[0].body.actions, []); + assert.equal(writes[0].body.head_sha, undefined); + }); +} + +test('selection edits and clearing the block replace the idle fingerprint and summary', async () => { + const versions = [], identities = []; + for (const body of [fixtures.cases[0].body, fixtures.cases[0].body.replace('2.14.2', '2.14.3'), + fixtures.cases[1].body, 'malformed selection']) { + const {calls} = checkMock({changePR: pr => { pr.body = body; }, list: listOf(ownCheck())}); + assert.equal((await worker.fetch(signed({...payload, number: 12}, 'pull_request'), env)).status, 200); + const output = checkWrites(calls)[0].body; + identities.push(output.external_id);versions.push(output.output.summary); + assert.deepEqual(output.actions, []); + } + assert.notEqual(identities[0], identities[1]);assert.notEqual(identities[1], identities[2]); + assert.match(versions[1], /2\.14\.3/);assert.doesNotMatch(versions[2], /launch-editor/); + assert.match(versions[3], /valid selection block/); +}); + +test('head synchronization creates a Check on the canonical new head', async () => { + const {calls, pr} = checkMock({changePR: pr => { pr.head.sha = 'c'.repeat(40); }}); + assert.equal((await worker.fetch(signed({...payload, number: 12, action: 'synchronize', + pull_request: {head: {sha: 'a'.repeat(40)}}}, 'pull_request'), env)).status, 200); + const output = checkWrites(calls)[0].body; + assert.equal(output.head_sha, 'c'.repeat(40)); + assert.equal(output.external_id, `node-repair:12:${await selectionFingerprint(pr, selections(pr.body))}`); +}); + +for (const [name, changePR] of [ + ['closed', pr => { pr.state = 'closed'; }], ['human author', pr => { pr.user.type = 'User'; }], + ['missing author', pr => { pr.user = null; }], ['fork', pr => { pr.head.repo.id = 99; }], + ['wrong base repository', pr => { pr.base.repo.id = 99; }], ['deleted head repository', pr => { pr.head.repo = null; }], + ['wrong branch', pr => { pr.head.ref = 'feature'; }], ['stale default branch', pr => { pr.base.ref = 'master'; }], + ['wrong PR number', pr => { pr.number = 13; }], ['bad head SHA', pr => { pr.head.sha = 'bad'; }], + ['bad base SHA', pr => { pr.base.sha = 'bad'; }], + ['missing default branch', (pr, repo) => { delete repo.default_branch; delete pr.base.ref; }], +]) { + test(`canonical ${name} PR cannot write a Check`, async () => { + const {calls} = checkMock({changePR}); + assert.equal((await worker.fetch(signed({...payload, number: 12}, 'pull_request'), env)).status, 200); + assert.equal(checkWrites(calls).length, 0);assert.equal(calls.length, 3); + }); +} + +test('canonical current default branch is used after a repository rename', async () => { + const {calls} = checkMock({changePR: (pr, repo) => { repo.default_branch = 'trunk'; pr.base.ref = 'trunk'; }}); + assert.equal((await worker.fetch(signed({...payload, number: 12}, 'pull_request'), env)).status, 200); + assert.equal(checkWrites(calls).length, 1); +}); + +for (const foreign of [ownCheck({app: {id: 999}}), ownCheck({name: 'Other Check'}), + ownCheck({head_sha: 'c'.repeat(40)}), ownCheck({external_id: 'node-repair:13:' + '0'.repeat(64)}), + ownCheck({external_id: 'unrelated'})]) { + test(`unrelated Check is never reused: ${JSON.stringify(foreign)}`, async () => { + const {calls} = checkMock({list: listOf(foreign)}); + assert.equal((await worker.fetch(signed({...payload, number: 12}, 'pull_request'), env)).status, 200); + assert.equal(checkWrites(calls)[0].method, 'POST'); + assert.equal(checkWrites(calls)[0].path, '/repos/fixture/repo/check-runs'); + }); +} + +for (const list of [{}, {total_count: 101, check_runs: []}, {total_count: -1, check_runs: []}, + {total_count: 1, check_runs: []}, listOf(ownCheck({id: 0})), listOf(ownCheck({id: '77'})), + listOf(ownCheck({status: 'in_progress'}))]) { + test(`incomplete or non-idle Check state fails without a write: ${JSON.stringify(list)}`, async () => { + const {calls} = checkMock({list}); + assert.equal((await worker.fetch(signed({...payload, number: 12}, 'pull_request'), env)).status, 500); + assert.equal(checkWrites(calls).length, 0); + }); +} + +test('duplicate owned idle Checks refresh the newest Check', async () => { + const {calls} = checkMock({list: listOf(ownCheck({id: 78}), ownCheck({id: 77}))}); + assert.equal((await worker.fetch(signed({...payload, number: 12}, 'pull_request'), env)).status, 200); + const writes = checkWrites(calls); + assert.equal(writes.length, 1); + assert.equal(writes[0].path, '/repos/fixture/repo/check-runs/78'); + assert.equal(writes[0].method, 'PATCH'); + assert.deepEqual(writes[0].body.actions, []); +}); + +test('overlapping initial deliveries recover from duplicate idle Checks on a later edit', async () => { + const repository = {id: 7, full_name: 'fixture/repo', default_branch: 'main'}; + const pr = {number: 12, state: 'open', user: {type: 'Bot'}, body: fixtures.cases[0].body, + head: {sha: 'a'.repeat(40), ref: 'maintenance/node-security', repo: {id: 7}}, + base: {sha: 'b'.repeat(40), ref: 'main', repo: {id: 7}}}; + const calls = [], created = []; + let phase = 'initial', listCalls = 0, resolveLists; + const listsReady = new Promise(resolve => { resolveLists = resolve; }); + fetch.mock.mockImplementation(async (url, options) => { + const path = new URL(url).pathname; + const body = options.body === undefined ? undefined : JSON.parse(options.body); + calls.push({path, method: options.method, body}); + assert.equal(options.redirect, 'manual'); + if (path === '/app/installations/3/access_tokens') return json({token}); + assert.equal(options.headers.Authorization, `Bearer ${token}`); + if (path === '/repos/fixture/repo') return json(repository); + if (path === '/repos/fixture/repo/pulls/12') return json(pr); + if (path === `/repos/fixture/repo/commits/${pr.head.sha}/check-runs`) { + if (phase === 'initial') { + listCalls++; + if (listCalls === 2) resolveLists(); + await listsReady; + return json({total_count: 0, check_runs: []}); + } + return json(listOf(ownCheck({id: 77}), ownCheck({id: 78}))); + } + if (path === '/repos/fixture/repo/check-runs') { + created.push(body); + return json({id: created.length === 1 ? 77 : 78}); + } + if (path === '/repos/fixture/repo/check-runs/78') return json({id: 78}); + throw new Error(`Unexpected API path: ${path}`); + }); + + const first = worker.fetch(signed({...payload, number: 12, action: 'opened'}, 'pull_request'), env); + const second = worker.fetch(signed({...payload, number: 12, action: 'opened'}, 'pull_request'), env); + const initialResponses = await Promise.all([first, second]); + assert.deepEqual(initialResponses.map(response => response.status), [200, 200]); + assert.equal(listCalls, 2); + assert.equal(created.length, 2); + + phase = 'edit'; + pr.body = fixtures.cases[0].body.replace('2.14.2', '2.14.3'); + const edit = await worker.fetch(signed({...payload, number: 12, action: 'edited'}, 'pull_request'), env); + assert.equal(edit.status, 200); + const writes = checkWrites(calls); + assert.deepEqual(writes.slice(0, 2).map(write => write.method), ['POST', 'POST']); + assert.equal(writes.length, 3); + assert.equal(writes[2].path, '/repos/fixture/repo/check-runs/78'); + assert.equal(writes[2].method, 'PATCH'); + assert.match(writes[2].body.output.summary, /2\.14\.3/); + assert.deepEqual(writes[2].body.actions, []); +}); + +test('overlapping edits reconcile an existing Check to the current canonical selection', async () => { + const repository = {id: 7, full_name: 'fixture/repo', default_branch: 'main'}; + let body = fixtures.cases[0].body; + const calls = [], writes = []; + let listCalls = 0, releaseFirstList, markFirstList; + const firstListReached = new Promise(resolve => { markFirstList = resolve; }); + const firstListHold = new Promise(resolve => { releaseFirstList = resolve; }); + fetch.mock.mockImplementation(async (url, options) => { + const path = new URL(url).pathname; + const requestBody = options.body === undefined ? undefined : JSON.parse(options.body); + calls.push({path, method: options.method, body: requestBody}); + assert.equal(options.redirect, 'manual'); + if (path === '/app/installations/3/access_tokens') return json({token}); + assert.equal(options.headers.Authorization, `Bearer ${token}`); + if (path === '/repos/fixture/repo') return json(repository); + if (path === '/repos/fixture/repo/pulls/12') return json(managedPR(body)); + if (path === `/repos/fixture/repo/commits/${'a'.repeat(40)}/check-runs`) { + if (++listCalls === 1) { markFirstList(); await firstListHold; } + return json(listOf(ownCheck())); + } + if (path === '/repos/fixture/repo/check-runs/77' && options.method === 'PATCH') { + writes.push(requestBody); + return json({id: 77}); + } + throw new Error(`Unexpected API path: ${options.method} ${path}`); + }); + + const older = worker.fetch(signed({...payload, number: 12, action: 'edited'}, 'pull_request'), env); + await firstListReached; + body = fixtures.cases[0].body.replace('2.14.2', '2.14.3'); + const newer = worker.fetch(signed({...payload, number: 12, action: 'edited'}, 'pull_request', + {'x-github-delivery': '94e535a5-2a7d-491d-858a-3df51ce211de'}), env); + const newerResponse = await newer; + releaseFirstList(); + const olderResponse = await older; + assert.equal(newerResponse.status, 200); + assert.equal(olderResponse.status, 200); + assert.equal(writes.length, 3); + assert.match(writes[0].output.summary, /2\.14\.3/); + assert.match(writes[1].output.summary, /2\.14\.2/); + assert.match(writes[2].output.summary, /2\.14\.3/); + assert.equal(writes[0].external_id, writes[2].external_id); + assert.notEqual(writes[1].external_id, writes[2].external_id); + assert.ok(writes.every(write => write.actions.length === 0)); +}); + +test('repeated canonical changes exhaust bounded Check reconciliation', async () => { + const repository = {id: 7, full_name: 'fixture/repo', default_branch: 'main'}; + const writes = []; + let prReads = 0; + fetch.mock.mockImplementation(async (url, options) => { + const path = new URL(url).pathname; + const requestBody = options.body === undefined ? undefined : JSON.parse(options.body); + assert.equal(options.redirect, 'manual'); + if (path === '/app/installations/3/access_tokens') return json({token}); + assert.equal(options.headers.Authorization, `Bearer ${token}`); + if (path === '/repos/fixture/repo') return json(repository); + if (path === '/repos/fixture/repo/pulls/12') { + prReads++; + return json(managedPR(fixtures.cases[0].body.replace('2.14.2', `2.14.${prReads + 1}`))); + } + if (path === `/repos/fixture/repo/commits/${'a'.repeat(40)}/check-runs`) return json(listOf(ownCheck())); + if (path === '/repos/fixture/repo/check-runs/77' && options.method === 'PATCH') { + writes.push(requestBody); + return json({id: 77}); + } + throw new Error(`Unexpected API path: ${options.method} ${path}`); + }); + const response = await worker.fetch(signed({...payload, number: 12, action: 'edited'}, 'pull_request'), env); + assert.equal(response.status, 500); + assert.equal(prReads, 6); + assert.equal(writes.length, 3); + assert.deepEqual(writes.map(write => write.actions), [[], [], []]); +}); + +test('eligibility loss after a Check write stops reconciliation safely', async () => { + const repository = {id: 7, full_name: 'fixture/repo', default_branch: 'main'}; + let prReads = 0; + const writes = []; + fetch.mock.mockImplementation(async (url, options) => { + const path = new URL(url).pathname; + const requestBody = options.body === undefined ? undefined : JSON.parse(options.body); + assert.equal(options.redirect, 'manual'); + if (path === '/app/installations/3/access_tokens') return json({token}); + assert.equal(options.headers.Authorization, `Bearer ${token}`); + if (path === '/repos/fixture/repo') return json(repository); + if (path === '/repos/fixture/repo/pulls/12') { + prReads++; + return json(managedPR(fixtures.cases[0].body, 'a'.repeat(40), prReads === 2 ? 'closed' : 'open')); + } + if (path === `/repos/fixture/repo/commits/${'a'.repeat(40)}/check-runs`) return json(listOf(ownCheck())); + if (path === '/repos/fixture/repo/check-runs/77' && options.method === 'PATCH') { + writes.push(requestBody); + return json({id: 77}); + } + throw new Error(`Unexpected API path: ${options.method} ${path}`); + }); + const response = await worker.fetch(signed({...payload, number: 12, action: 'edited'}, 'pull_request'), env); + assert.equal(response.status, 200); + assert.equal(prReads, 2); + assert.equal(writes.length, 1); + assert.deepEqual(writes[0].actions, []); +}); + +test('head changes during reconciliation refresh the new canonical head', async () => { + const repository = {id: 7, full_name: 'fixture/repo', default_branch: 'main'}; + const oldHead = 'a'.repeat(40), newHead = 'c'.repeat(40); + let prReads = 0; + const calls = []; + fetch.mock.mockImplementation(async (url, options) => { + const path = new URL(url).pathname; + const requestBody = options.body === undefined ? undefined : JSON.parse(options.body); + calls.push({path, method: options.method, body: requestBody}); + assert.equal(options.redirect, 'manual'); + if (path === '/app/installations/3/access_tokens') return json({token}); + assert.equal(options.headers.Authorization, `Bearer ${token}`); + if (path === '/repos/fixture/repo') return json(repository); + if (path === '/repos/fixture/repo/pulls/12') { + prReads++; + return json(managedPR(fixtures.cases[0].body, prReads === 1 ? oldHead : newHead)); + } + if (path === `/repos/fixture/repo/commits/${oldHead}/check-runs`) return json(listOf(ownCheck())); + if (path === `/repos/fixture/repo/commits/${newHead}/check-runs`) return json(listOf()); + if (path === '/repos/fixture/repo/check-runs/77' && options.method === 'PATCH') return json({id: 77}); + if (path === '/repos/fixture/repo/check-runs' && options.method === 'POST') return json({id: 88}); + throw new Error(`Unexpected API path: ${options.method} ${path}`); + }); + const response = await worker.fetch(signed({...payload, number: 12, action: 'edited'}, 'pull_request'), env); + assert.equal(response.status, 200); + const writes = checkWrites(calls); + assert.equal(writes.length, 2); + assert.equal(writes[0].method, 'PATCH'); + assert.equal(writes[1].method, 'POST'); + assert.equal(writes[1].body.head_sha, newHead); + assert.deepEqual(writes.map(write => write.body.actions), [[], []]); +}); + +test('canonical PR or Check-list API failures are sanitized and cannot cause writes', async () => { + for (const failPath of ['/repos/fixture/repo/pulls/12', `/repos/fixture/repo/commits/${'a'.repeat(40)}/check-runs`]) { + const {calls} = checkMock({failPath}); + const response = await worker.fetch(signed({...payload, number: 12}, 'pull_request'), env); + assert.equal(response.status, 500);assert.equal(await response.text(), 'Webhook processing failed'); + assert.equal(checkWrites(calls).length, 0); + } +});