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); + } +});