From 48e49949b340ec1b1d46a276833e055a8c18cf01 Mon Sep 17 00:00:00 2001 From: Hayden Date: Mon, 7 Sep 2026 18:05:51 +0000 Subject: [PATCH 1/2] fix: isolate monthly node review publication --- .github/codex/prompts/node-version-review.md | 7 +- .github/workflows/node-version-review.yml | 148 +++++++-- NODE_VERSION_POLICY.md | 9 +- docs/architecture/security-model.md | 20 +- scripts/node-version-review.py | 194 ++++++++++++ tests/node-version-review.test.ts | 301 +++++++++++++++++++ 6 files changed, 642 insertions(+), 37 deletions(-) create mode 100644 scripts/node-version-review.py create mode 100644 tests/node-version-review.test.ts diff --git a/.github/codex/prompts/node-version-review.md b/.github/codex/prompts/node-version-review.md index a64b90504..9213e51fa 100644 --- a/.github/codex/prompts/node-version-review.md +++ b/.github/codex/prompts/node-version-review.md @@ -34,8 +34,11 @@ If it has drifted, prepare one focused change: Do not commit, push, open a pull request, call GitHub, or modify repository secrets. You may run the fast policy checker while editing, but do not spend -this run repeating the full lint and test suites; the workflow owns validation -and opens the draft pull request after you finish. +this run repeating the full lint and test suites. This job has read-only +repository access. The workflow exports the allowed edits as untrusted JSON; +separate jobs verify the proposal against the original workflow commit, run +repository validation, and publish the draft pull request. Only the publishing +job has repository write access, and it does not execute proposed code. Your final response becomes the draft pull request body. Write concise Markdown with these sections: diff --git a/.github/workflows/node-version-review.yml b/.github/workflows/node-version-review.yml index dd306e938..5922d4e69 100644 --- a/.github/workflows/node-version-review.yml +++ b/.github/workflows/node-version-review.yml @@ -1,8 +1,7 @@ # Asks Codex once a month to compare this repository with the official Node.js # lifecycle, make any needed support-policy edits, validate them, and open a # draft pull request. If nothing has drifted, the run makes no pull request. -# Because GitHub suppresses CI events for its own token, checks start when a -# maintainer marks the draft ready; human review is always required before merge. +# Human review is always required before merge. name: Monthly Node.js version review on: @@ -14,36 +13,30 @@ concurrency: group: monthly-node-version-review cancel-in-progress: false -permissions: - contents: write - pull-requests: write +# Proposal generation and validation never receive repository write authority. +permissions: {} jobs: - review: + propose: if: github.repository == 'openai/openai-node' && github.ref == 'refs/heads/main' runs-on: ubuntu-latest timeout-minutes: 30 + permissions: + contents: read environment: ci + outputs: + artifact-id: ${{ steps.upload.outputs.artifact-id }} steps: - - name: Check out main + - name: Check out the workflow commit uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6 with: - ref: main + ref: ${{ github.sha }} persist-credentials: false - - name: Set up pnpm - uses: pnpm/action-setup@f40ffcd9367d9f12939873eb1018b921a783ffaa # v4 - - name: Set up Node uses: actions/setup-node@49933ea5288caeca8642d1e84afbd3f7d6820020 # v4 with: node-version-file: '.nvmrc' - cache: pnpm - - - name: Install dependencies - run: | - pnpm install --frozen-lockfile - ./scripts/steady/install - name: Ask Codex to review Node.js support id: codex @@ -56,6 +49,73 @@ jobs: permission-profile: ':workspace' safety-strategy: drop-sudo + - name: Export the untrusted proposal as JSON + env: + EXPECTED_BASE_SHA: ${{ github.sha }} + PROPOSAL_PATH: ${{ runner.temp }}/node-review-export/proposal.json + REVIEW_BODY: ${{ steps.codex.outputs.final-message }} + run: | + python3 scripts/node-version-review.py export \ + --base-sha "$EXPECTED_BASE_SHA" \ + --proposal "$PROPOSAL_PATH" + + - name: Upload the original proposal + id: upload + uses: actions/upload-artifact@043fb46d1a93c77aae656e7c1c64a875d1fc6a0a # v7.0.1 + with: + name: node-review-proposal-${{ github.run_id }}-${{ github.run_attempt }} + path: ${{ runner.temp }}/node-review-export/proposal.json + if-no-files-found: error + retention-days: 1 + + validate: + needs: propose + if: github.repository == 'openai/openai-node' && github.ref == 'refs/heads/main' + runs-on: ubuntu-latest + timeout-minutes: 30 + permissions: + contents: read + steps: + - name: Check out the workflow commit + uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6 + with: + ref: ${{ github.sha }} + persist-credentials: false + + - name: Download the original proposal + uses: actions/download-artifact@3e5f45b2cfb9172054b4087a40e8e0b5a5461e7c # v8.0.1 + with: + artifact-ids: ${{ needs.propose.outputs.artifact-id }} + path: ${{ runner.temp }}/node-review-proposal + merge-multiple: true + digest-mismatch: error + + - name: Verify and apply the proposal + id: proposal + env: + EXPECTED_BASE_SHA: ${{ github.sha }} + PROPOSAL_PATH: ${{ runner.temp }}/node-review-proposal/proposal.json + BODY_PATH: ${{ runner.temp }}/node-review-body.md + run: | + python3 scripts/node-version-review.py apply \ + --base-sha "$EXPECTED_BASE_SHA" \ + --proposal "$PROPOSAL_PATH" \ + --body-output "$BODY_PATH" + + - name: Set up pnpm + uses: pnpm/action-setup@f40ffcd9367d9f12939873eb1018b921a783ffaa # v4 + + - name: Set up Node + uses: actions/setup-node@49933ea5288caeca8642d1e84afbd3f7d6820020 # v4 + with: + node-version-file: '.nvmrc' + cache: pnpm + + - name: Install dependencies + run: | + pnpm install --frozen-lockfile + ./scripts/steady/install + - name: Check Node.js version policy run: node --experimental-strip-types scripts/check-node-version-policy.ts @@ -89,22 +149,62 @@ jobs: - name: Run tests run: ./scripts/test + publish: + needs: [propose, validate] + if: github.repository == 'openai/openai-node' && github.ref == 'refs/heads/main' + runs-on: ubuntu-latest + timeout-minutes: 10 + permissions: + contents: write + pull-requests: write + steps: + - name: Check out the workflow commit + uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6 + with: + ref: ${{ github.sha }} + persist-credentials: false + + # create-pull-request resets the working branch to origin/main. Keep both + # refs at the validated commit so a concurrent main update cannot rebase it. + - name: Preserve the validated base + env: + EXPECTED_BASE_SHA: ${{ github.sha }} + run: | + test "$(git rev-parse HEAD)" = "$EXPECTED_BASE_SHA" + git checkout -B main "$EXPECTED_BASE_SHA" + git update-ref refs/remotes/origin/main "$EXPECTED_BASE_SHA" + + - name: Download the original proposal + uses: actions/download-artifact@3e5f45b2cfb9172054b4087a40e8e0b5a5461e7c # v8.0.1 + with: + artifact-ids: ${{ needs.propose.outputs.artifact-id }} + path: ${{ runner.temp }}/node-review-proposal + merge-multiple: true + digest-mismatch: error + + - name: Verify and apply the proposal + id: proposal + env: + EXPECTED_BASE_SHA: ${{ github.sha }} + PROPOSAL_PATH: ${{ runner.temp }}/node-review-proposal/proposal.json + BODY_PATH: ${{ runner.temp }}/node-review-body.md + run: | + python3 scripts/node-version-review.py apply \ + --base-sha "$EXPECTED_BASE_SHA" \ + --proposal "$PROPOSAL_PATH" \ + --body-output "$BODY_PATH" + - name: Open or update the draft pull request uses: peter-evans/create-pull-request@22a9089034f40e5a961c8808d113e2c98fb63676 # v7 with: token: ${{ github.token }} + base: main branch: codex/monthly-node-version-update delete-branch: true draft: always-true commit-message: 'chore(node): review supported Node.js versions' title: 'chore(node): review supported Node.js versions' - body: | - Automated monthly review of the Node.js support policy. - - ${{ steps.codex.outputs.final-message }} - - Before marking this draft ready, choose the release classification - required by `NODE_VERSION_POLICY.md` and update the title. + body-path: ${{ runner.temp }}/node-review-body.md add-paths: | .nvmrc package.json diff --git a/NODE_VERSION_POLICY.md b/NODE_VERSION_POLICY.md index 333831c58..40030d114 100644 --- a/NODE_VERSION_POLICY.md +++ b/NODE_VERSION_POLICY.md @@ -60,9 +60,12 @@ type-checked `scripts/check-node-version-policy.ts` fails when those projections drift and emits the matrix consumed by CI. Each month, `.github/workflows/node-version-review.yml` asks Codex to research -the official Node.js schedule, reconcile the policy artifacts, run repository -validation, and open or update one draft pull request. Generated changes never -merge automatically. +the official Node.js schedule and propose policy edits in a job with read-only +repository access. Separate jobs verify the same JSON proposal against the +workflow commit, run repository validation, and open or update one draft pull +request. Only the publishing job has repository write access; it applies +verified policy files without installing dependencies or executing proposed +code. Generated changes never merge automatically. For upstream dates and lifecycle definitions, see the [Node.js release schedule](https://github.com/nodejs/Release/blob/main/schedule.json) diff --git a/docs/architecture/security-model.md b/docs/architecture/security-model.md index d6e3ab68c..cf0a5600e 100644 --- a/docs/architecture/security-model.md +++ b/docs/architecture/security-model.md @@ -61,7 +61,7 @@ SDK verifier before parsing (`src/client.ts:505-612`, | Bedrock providers | Dependency-free bearer provider or AWS access key, secret key, session token, profile, or credential-provider chain | Explicit provider options select one bearer or AWS credential mode and a Bedrock endpoint/region | Normalized configured or region-derived Bedrock endpoint | Bedrock authority receives bearer or SigV4-signed request | Provider-owned Authorization, endpoint/region checks, replayable-body requirement, and manual redirects | `src/providers/bedrock.ts:14-50`, `src/providers/bedrock/aws.ts:26-64`, `src/providers/bedrock/aws.ts:161-249`, `src/providers/bedrock/aws.ts:252-289` | | Stable and beta Realtime | API key, ephemeral key, or Azure credential in a WebSocket handshake; untrusted server frames | Stable defaults derive from the client but trusted `buildRealtimeURL` may supply any `wss:` URL; beta requires the configured origin | Stable trusted `wss:` URL or beta matching configured `wss:` origin | Configured or explicitly trusted Realtime authority; parsed events to application listeners | Stable requires `wss:`; legacy Bedrock clients additionally require the configured origin; beta requires `wss:` plus configured-origin equality; both `ws` transports protect credential redirects and both surfaces validate own string `type` | `src/realtime/internal-base.ts:155-166`, `src/realtime/internal-base.ts:276-281`, `src/realtime/websocket.ts:204-230`, `src/internal/bedrock.ts:247-263`, `src/realtime/ws.ts:63-105`, `src/beta/realtime/websocket.ts:132-143`, `src/beta/realtime/websocket.ts:210-258`, `src/beta/realtime/ws.ts:18-28`, `src/beta/realtime/ws.ts:68-117` | | Trusted Castiron reporting | Lower-trust `workflow_run` metadata, trusted main reporter, status and PR-comment writes | Completed upstream custom-code workflow is checked against event, path, repository, head SHA, and current PR identity | Main checkout plus recomputed Git-object report | Commit statuses and one PR comment | Trusted main code recomputes reports; freshness and identity checks gate writes; candidate artifacts are not trusted inputs | `.github/workflows/castiron-custom-code-comment.yml:4-24`, `.github/workflows/castiron-custom-code-comment.yml:34-98`, `.github/workflows/castiron-custom-code-comment.yml:116-200` | -| Monthly Node version review | OpenAI API key plus repository and pull-request write authority for an automated Codex run | Scheduled or manual main-only workflow, `environment: ci`, fixed prompt file, workspace permission profile | Main checkout and `codex/monthly-node-version-update` draft PR | Codex action and GitHub draft-PR publisher | Main-only condition, pinned actions, `drop-sudo`, constrained add-paths, and draft PR; required human review and branch protection are external assumptions | `.github/workflows/node-version-review.yml:17-55`, `.github/workflows/node-version-review.yml:90-111` | +| Monthly Node version review | OpenAI API key in the proposal job; repository and pull-request write authority only in the publisher | Scheduled or manual main-only workflow; isolated proposal, validation, and publication jobs | Same immutable JSON artifact and workflow commit in fresh validation and publisher checkouts | Codex action, read-only validation, and GitHub draft-PR publisher | Pinned actions, strict policy-file allowlist, engines.node-only package reconstruction, independent verification before dependency execution and publication, fixed validated base, and draft PR; human review and branch protection are external assumptions | `.github/workflows/node-version-review.yml:16-117`, `.github/workflows/node-version-review.yml:152-213`, `scripts/node-version-review.py` | ## 2. Threat Model, Trust Boundaries, and Assumptions @@ -308,15 +308,19 @@ artifact crosses into protected CI, release, or publication credentials. `.github/workflows/create-releases.yml:288-346`). - **Workflow metadata or automation prompts to privileged writes:** the trusted Castiron comment workflow consumes lower-trust `workflow_run` state - before writing statuses and PR comments, while the monthly Node review gives - an automated Codex run an API key and repository/PR write authority. Their - main-only code, freshness/identity checks, constrained paths, and draft - status are repository-visible controls; required human review and branch - protection are external assumptions + before writing statuses and PR comments. The monthly Node review isolates + Codex with read-only repository access and an API key. Fresh validation and + publisher jobs independently verify the same JSON proposal against the + workflow commit, allowing only policy files and `package.json#engines.node`. + Validation has read-only access; the publisher has repository/PR write access + and applies verified data without installing dependencies or executing + proposed code. Its local base remains at the validated commit. Required human + review and branch protection are external assumptions (`.github/workflows/castiron-custom-code-comment.yml:4-24`, `.github/workflows/castiron-custom-code-comment.yml:116-200`, - `.github/workflows/node-version-review.yml:17-55`, - `.github/workflows/node-version-review.yml:90-111`). + `.github/workflows/node-version-review.yml:16-117`, + `.github/workflows/node-version-review.yml:152-213`, + `scripts/node-version-review.py`). - **Dependencies to build execution:** frozen lockfiles and workspace dependency policy reduce supply-chain risk, but dependency lifecycle behavior remains a real boundary when independently mutable artifacts can reach a trusted build diff --git a/scripts/node-version-review.py b/scripts/node-version-review.py new file mode 100644 index 000000000..1dc50b79f --- /dev/null +++ b/scripts/node-version-review.py @@ -0,0 +1,194 @@ +"""Transfer monthly Node policy proposals as data between isolated workflow jobs.""" + +import argparse +import json +import os +import re +import stat +import subprocess +from pathlib import Path + +ALLOWED_FILES = frozenset( + { + ".nvmrc", + "package.json", + "README.md", + ".github/CONTRIBUTING.md", + "NODE_VERSION_POLICY.md", + } +) +# This is a workflow artifact limit; it does not constrain SDK request payloads. +MAX_PROPOSAL_BYTES = 1024 * 1024 + + +def reject(message): + raise ValueError(message) + + +def unique_object(pairs): + result = {} + for key, value in pairs: + if key in result: + reject("Duplicate JSON key") + result[key] = value + return result + + +def parse_json(content): + return json.loads( + content, + object_pairs_hook=unique_object, + parse_constant=lambda _: reject("Non-finite JSON number"), + ) + + +def canonical(value): + return json.dumps(value, sort_keys=True, ensure_ascii=False, allow_nan=False) + + +def git(*args): + return subprocess.check_output(["git", *args], text=True, encoding="utf-8").rstrip( + "\n" + ) + + +def regular_file(path): + """Do not follow links in either the file or its containing directories.""" + for parent in path.parents: + if parent.is_symlink(): + reject("Symlinked parent directory") + mode = path.lstat().st_mode + if not stat.S_ISREG(mode) or mode & 0o111: + reject("Proposal files must be regular, non-executable files") + if path.stat().st_size > MAX_PROPOSAL_BYTES: + reject("Proposal exceeds the workflow artifact size limit") + + +def text_bytes(content): + if not isinstance(content, str) or "\0" in content: + reject("Expected UTF-8 text without NUL bytes") + encoded = content.encode("utf-8") + if len(encoded) > MAX_PROPOSAL_BYTES: + reject("Proposal exceeds the workflow artifact size limit") + return encoded + + +def baseline_file(base_sha, filename): + entry = git("ls-tree", base_sha, "--", filename) + if not entry.startswith("100644 blob ") or entry.split("\t")[-1] != filename: + reject("Baseline file must be a tracked regular file") + return subprocess.check_output(["git", "show", f"{base_sha}:{filename}"]).decode( + "utf-8" + ) + + +def verify(proposal, base_sha, root): + if not isinstance(proposal, dict) or set(proposal) != {"base_sha", "files", "body"}: + reject("Invalid proposal fields") + if proposal["base_sha"] != base_sha: + reject("Proposal does not match the workflow commit") + files = proposal["files"] + if not isinstance(files, dict) or not set(files) <= ALLOWED_FILES: + reject("Proposal contains files outside the Node policy allowlist") + body = text_bytes(proposal["body"]) + updates = {} + for filename, content in files.items(): + text_bytes(content) + regular_file(root / filename) + baseline = baseline_file(base_sha, filename) + if filename == ".nvmrc" and not re.fullmatch(r"[1-9][0-9]*\n?", content): + reject(".nvmrc must contain one ASCII Node.js major version") + if filename == "package.json": + candidate = parse_json(content) + expected = parse_json(baseline) + if not isinstance(candidate, dict) or not isinstance( + candidate.get("engines"), dict + ): + reject("Invalid package.json engines") + node = candidate["engines"].get("node") + if not isinstance(node, str) or not re.fullmatch( + r">=[1-9][0-9]*\.0\.0", node + ): + reject("Invalid package.json engines.node") + expected["engines"]["node"] = node + # Canonical JSON distinguishes booleans and numbers, unlike Python equality. + if canonical(candidate) != canonical(expected): + reject("Only package.json engines.node may change") + content = ( + json.dumps(expected, ensure_ascii=False, indent=2, allow_nan=False) + + "\n" + ) + if content != baseline: + updates[filename] = text_bytes(content) + return updates, body + + +def main(): + parser = argparse.ArgumentParser(description=__doc__) + parser.add_argument("command", choices=["export", "apply"]) + parser.add_argument("--base-sha", required=True) + parser.add_argument("--proposal", required=True, type=Path) + parser.add_argument("--body-output", type=Path) + args = parser.parse_args() + root = Path.cwd() + if ( + not re.fullmatch(r"[0-9a-f]{40}", args.base_sha) + or git("rev-parse", "HEAD") != args.base_sha + ): + reject("Checkout does not match the workflow commit") + if Path(git("rev-parse", "--show-toplevel")) != root: + reject("Run from the repository root") + + if args.command == "export": + changed = git("diff", "--name-only", "-z", args.base_sha, "--").split("\0") + untracked = git("ls-files", "--others", "--exclude-standard", "-z").split("\0") + filenames = set(changed + untracked) - {""} + if not filenames <= ALLOWED_FILES: + reject("Working tree contains changes outside the Node policy allowlist") + files = {} + for filename in sorted(filenames): + regular_file(root / filename) + files[filename] = (root / filename).read_text(encoding="utf-8") + proposal = { + "base_sha": args.base_sha, + "files": files, + "body": os.environ.get("REVIEW_BODY", ""), + } + verify(proposal, args.base_sha, root) + encoded = text_bytes(json.dumps(proposal, ensure_ascii=False, allow_nan=False)) + args.proposal.parent.mkdir(parents=True, exist_ok=True) + args.proposal.write_bytes(encoded) + return + + if git("status", "--porcelain", "--untracked-files=all"): + reject("Apply requires a fresh, clean checkout") + regular_file(args.proposal.absolute()) + proposal = parse_json(args.proposal.read_bytes().decode("utf-8")) + updates, body = verify(proposal, args.base_sha, root) + if args.body_output is None or args.body_output.resolve().is_relative_to(root): + reject("The pull request body must be outside the checkout") + for parent in args.body_output.absolute().parents: + if parent.is_symlink() or not parent.is_dir(): + reject("Invalid pull request body directory") + if args.body_output.exists() or args.body_output.is_symlink(): + regular_file(args.body_output.absolute()) + + # Everything is validated before any approved file or PR body is materialized. + for filename, content in updates.items(): + (root / filename).write_bytes(content) + args.body_output.write_bytes( + b"Automated monthly review of the Node.js support policy.\n\n" + + body + + b"\n\nBefore marking this draft ready, choose the release classification " + b"required by `NODE_VERSION_POLICY.md` and update the title.\n" + ) + if output := os.environ.get("GITHUB_OUTPUT"): + with open(output, "a", encoding="utf-8") as stream: + stream.write(f"changed={'true' if updates else 'false'}\n") + + +if __name__ == "__main__": + try: + main() + except (ValueError, OSError, subprocess.CalledProcessError) as error: + raise SystemExit(f"Node review proposal rejected: {error}") from None diff --git a/tests/node-version-review.test.ts b/tests/node-version-review.test.ts new file mode 100644 index 000000000..20835d989 --- /dev/null +++ b/tests/node-version-review.test.ts @@ -0,0 +1,301 @@ +import { spawnSync } from 'node:child_process'; +import { + chmodSync, + existsSync, + mkdirSync, + mkdtempSync, + readFileSync, + rmSync, + symlinkSync, + writeFileSync, +} from 'node:fs'; +import { tmpdir } from 'node:os'; +import path from 'node:path'; + +const root = process.cwd(); +const verifier = path.join(root, 'scripts/node-version-review.py'); +const workflow = readFileSync(path.join(root, '.github/workflows/node-version-review.yml'), 'utf-8'); +const files = ['.nvmrc', 'package.json', 'README.md', '.github/CONTRIBUTING.md', 'NODE_VERSION_POLICY.md']; +const environment = { + PATH: process.env['PATH'], + GIT_CONFIG_NOSYSTEM: '1', + GIT_CONFIG_GLOBAL: '/dev/null', +}; + +// These jobs run on Ubuntu and require its Python and Git installations. +const describeOnUnix = process.platform === 'win32' ? describe.skip : describe; +describeOnUnix('monthly Node review proposal boundary', () => { + let fixture: string; + let checkout: string; + let proposalPath: string; + let bodyPath: string; + let outputPath: string; + let base: string; + let metadata: Record; + + function git(...args: string[]) { + const result = spawnSync('git', args, { cwd: checkout, env: environment, encoding: 'utf-8' }); + expect(result.error).toBeUndefined(); + expect(result.status).toBe(0); + return result.stdout.trim(); + } + + function commit() { + git('add', '.'); + git('-c', 'commit.gpgsign=false', 'commit', '-qm', 'fixture'); + return git('rev-parse', 'HEAD'); + } + + function run(command = 'apply', extraEnv: Record = {}) { + const result = spawnSync( + 'python3', + [verifier, command, '--base-sha', base, '--proposal', proposalPath, '--body-output', bodyPath], + { + cwd: checkout, + env: { ...environment, GITHUB_OUTPUT: outputPath, ...extraEnv }, + encoding: 'utf-8', + timeout: 10_000, + }, + ); + expect(result.error).toBeUndefined(); + expect(result.signal).toBeNull(); + return result; + } + + function proposal(changes: Record = {}, body: unknown = 'Review summary') { + return { base_sha: base, files: changes, body }; + } + + function rejects(raw: string) { + const before = files.map((file) => readFileSync(path.join(checkout, file), 'utf-8')); + writeFileSync(proposalPath, raw); + expect(run().status).toBe(1); + expect(files.map((file) => readFileSync(path.join(checkout, file), 'utf-8'))).toEqual(before); + expect(existsSync(bodyPath)).toBe(false); + expect(existsSync(outputPath)).toBe(false); + expect(git('status', '--porcelain')).toBe(''); + } + + beforeEach(() => { + fixture = mkdtempSync(path.join(tmpdir(), 'openai-node-review-')); + checkout = path.join(fixture, 'checkout'); + proposalPath = path.join(fixture, 'proposal.json'); + bodyPath = path.join(fixture, 'body.md'); + outputPath = path.join(fixture, 'output'); + mkdirSync(path.join(checkout, '.github'), { recursive: true }); + metadata = { + name: 'policy-fixture', + engines: { node: '>=22.0.0' }, + scripts: { test: 'echo trusted' }, + dependencies: { example: '1.0.0' }, + exports: { '.': './index.js' }, + packageManager: 'pnpm@10.10.0', + flags: { enabled: true, count: 1, empty: null }, + }; + for (const file of files) { + writeFileSync(path.join(checkout, file), 'Baseline documentation\n'); + } + writeFileSync(path.join(checkout, '.nvmrc'), '24\n'); + writeFileSync(path.join(checkout, 'package.json'), `${JSON.stringify(metadata, null, 2)}\n`); + git('init', '-q', '-b', 'main'); + git('config', 'user.name', 'Workflow Test'); + git('config', 'user.email', 'workflow-test@example.com'); + base = commit(); + }); + + afterEach(() => rmSync(fixture, { recursive: true, force: true })); + + test('exports and applies policy edits with an inert Markdown body', () => { + const body = `## Summary\n$(touch '${fixture}/executed')\n\`touch '${fixture}/executed'\`\n`; + metadata['engines'].node = '>=24.0.0'; + writeFileSync(path.join(checkout, 'package.json'), JSON.stringify(metadata)); + writeFileSync(path.join(checkout, '.nvmrc'), '26\n'); + writeFileSync(path.join(checkout, 'README.md'), 'Updated policy\n'); + expect(run('export', { REVIEW_BODY: body }).status).toBe(0); + git('reset', '--hard', base); + expect(run().status).toBe(0); + expect(JSON.parse(readFileSync(path.join(checkout, 'package.json'), 'utf-8'))).toEqual(metadata); + expect(readFileSync(path.join(checkout, '.nvmrc'), 'utf-8')).toBe('26\n'); + expect(readFileSync(path.join(checkout, 'README.md'), 'utf-8')).toBe('Updated policy\n'); + expect(readFileSync(bodyPath, 'utf-8')).toContain(body); + expect(readFileSync(outputPath, 'utf-8')).toBe('changed=true\n'); + expect(existsSync(path.join(fixture, 'executed'))).toBe(false); + }); + + test('preserves actual repository package formatting when there is no policy change', () => { + const actualPackage = readFileSync(path.join(root, 'package.json'), 'utf-8'); + writeFileSync(path.join(checkout, 'package.json'), actualPackage); + base = commit(); + writeFileSync(proposalPath, JSON.stringify(proposal({ 'package.json': actualPackage }))); + expect(run().status).toBe(0); + expect(readFileSync(path.join(checkout, 'package.json'), 'utf-8')).toBe(actualPackage); + expect(readFileSync(outputPath, 'utf-8')).toBe('changed=false\n'); + expect(git('status', '--porcelain')).toBe(''); + }); + + test('accepts an empty proposal as a no-op', () => { + expect(run('export').status).toBe(0); + expect(JSON.parse(readFileSync(proposalPath, 'utf-8')).files).toEqual({}); + expect(run().status).toBe(0); + expect(readFileSync(outputPath, 'utf-8')).toBe('changed=false\n'); + }); + + test.each([ + ['wrong commit', () => ({ ...proposal(), base_sha: '0'.repeat(40) })], + ['unknown path', () => proposal({ 'scripts/lint': 'touch /tmp/executed' })], + ['traversal', () => proposal({ '../README.md': 'text' })], + ['extra field', () => ({ ...proposal(), mode: '100755' })], + ['non-object envelope', () => []], + ['non-object files', () => ({ ...proposal(), files: [] })], + ['non-text body', () => proposal({}, [])], + ['non-text file', () => proposal({ 'README.md': { content: 'text', mode: '100755' } })], + ['NUL text', () => proposal({ 'README.md': '\0' })], + ['invalid later file', () => proposal({ 'README.md': 'Valid first edit', 'package.json': '{}' })], + ])('rejects %s without partially applying files', (_name, makeProposal) => { + rejects(JSON.stringify(makeProposal())); + }); + + test.each(['lts/*', 'v26', '026', '26\n28', '２６', '26\n\n'])('rejects .nvmrc value %j', (value) => { + rejects(JSON.stringify(proposal({ '.nvmrc': value }))); + }); + + test.each(['scripts', 'dependencies', 'exports', 'packageManager'])( + 'rejects package.json %s changes', + (key) => { + metadata[key] = key === 'packageManager' ? 'pnpm@99.0.0' : { changed: 'untrusted' }; + rejects(JSON.stringify(proposal({ 'package.json': JSON.stringify(metadata) }))); + }, + ); + + test.each([ + ['boolean to number', '"enabled":true', '"enabled":1'], + ['number to boolean', '"count":1', '"count":true'], + ['integer to float', '"count":1', '"count":1.0'], + ['nonfinite number', '"count":1', '"count":NaN'], + ['overflowing number', '"count":1', '"count":1e999'], + ['duplicate escaped key', '"count":1', '"count":1,"\\u0063ount":1'], + ])('rejects package.json %s', (_name, before, after) => { + rejects(JSON.stringify(proposal({ 'package.json': JSON.stringify(metadata).replace(before, after) }))); + }); + + test.each(['"body":"first",', '"\\u0062ody":"first",'])('rejects duplicate envelope keys %s', (key) => { + rejects(JSON.stringify(proposal()).replace('{', `{${key}`)); + }); + + test.each(['NaN', 'Infinity', '-Infinity'])('rejects nonstandard JSON %s', (constant) => { + rejects(JSON.stringify(proposal()).replace('"Review summary"', constant)); + }); + + test.each(['symlink', 'executable', 'source change'])('does not export %s', (kind) => { + const target = path.join(checkout, 'README.md'); + if (kind === 'symlink') { + rmSync(target); + symlinkSync(bodyPath, target); + } else if (kind === 'executable') { + chmodSync(target, 0o755); + } else { + writeFileSync(path.join(checkout, 'unapproved.js'), 'throw new Error("untrusted")'); + } + expect(run('export').status).toBe(1); + expect(existsSync(proposalPath)).toBe(false); + }); + + test('rejects a symlinked artifact', () => { + const target = path.join(fixture, 'other.json'); + writeFileSync(target, JSON.stringify(proposal({ 'README.md': 'new' }))); + symlinkSync(target, proposalPath); + expect(run().status).toBe(1); + expect(git('status', '--porcelain')).toBe(''); + expect(existsSync(bodyPath)).toBe(false); + }); + + test('keeps the validated base when remote main advances before publication', () => { + const remote = path.join(fixture, 'remote.git'); + git('init', '--bare', '-q', remote); + git('remote', 'add', 'origin', remote); + git('push', '-q', 'origin', 'main'); + writeFileSync(path.join(checkout, 'README.md'), 'Concurrent main update\n'); + const advanced = commit(); + git('push', '-q', 'origin', 'main'); + git('checkout', '--detach', base); + expect(git('rev-parse', 'origin/main')).toBe(advanced); + + const step = workflow + .split(' - name: Preserve the validated base\n')[1] + ?.split('\n - name:')[0]; + const script = step + ?.split(' run: |\n')[1] + ?.split('\n') + .map((line) => line.slice(10)) + .join('\n'); + if (!script) { + throw new Error('Missing trusted publisher base setup'); + } + const result = spawnSync('bash', ['-e', '-c', script], { + cwd: checkout, + env: { ...environment, EXPECTED_BASE_SHA: base }, + encoding: 'utf-8', + }); + expect(result.status).toBe(0); + expect(git('symbolic-ref', '--short', 'HEAD')).toBe('main'); + expect(git('rev-parse', 'main')).toBe(base); + expect(git('rev-parse', 'origin/main')).toBe(base); + expect(git('ls-remote', 'origin', 'refs/heads/main')).toContain(advanced); + writeFileSync(proposalPath, JSON.stringify(proposal({ 'README.md': 'Reviewed policy\n' }))); + expect(run().status).toBe(0); + commit(); + expect(git('rev-parse', 'HEAD^')).toBe(base); + }); +}); + +describe('monthly Node review workflow authority', () => { + const propose = workflow.split('\n propose:\n')[1]?.split('\n validate:\n')[0] ?? ''; + const validate = workflow.split('\n validate:\n')[1]?.split('\n publish:\n')[0] ?? ''; + const publish = workflow.split('\n publish:\n')[1] ?? ''; + + test('separates agent, validation, and publication capabilities', () => { + expect(workflow.split('\njobs:')[0]).toContain('permissions: {}'); + for (const job of [propose, validate, publish]) { + expect(job).toContain(`ref: \${{ github.sha }}`); + expect(job).toContain('persist-credentials: false'); + } + for (const job of [propose, validate]) { + expect(job).toContain('contents: read'); + expect(job).not.toContain(': write'); + } + expect(propose).toContain('environment: ci'); + expect(propose).toContain('secrets.OPENAI_API_KEY'); + expect(validate).not.toMatch(/environment:|secrets\.|codex-action/u); + expect(publish).not.toMatch( + /environment:|secrets\.|codex-action|setup-node|pnpm|scripts\/(?:test|build|lint)/u, + ); + expect(publish).toContain('needs: [propose, validate]'); + expect(publish).toContain('contents: write'); + expect(publish).toContain('pull-requests: write'); + expect(publish).toContain('base: main'); + expect(publish).toContain(`body-path: \${{ runner.temp }}/node-review-body.md`); + expect(publish).toContain('draft: always-true'); + expect(publish).toContain('delete-branch: true'); + expect(publish).not.toContain('final-message'); + // Let the pinned action clean up an obsolete draft branch on a no-op, too. + expect(publish.split(' - name: Open or update the draft pull request\n')[1]).not.toContain('if:'); + }); + + test('independently verifies the same original artifact before installation or publication', () => { + expect(propose).toContain(`node-review-proposal-\${{ github.run_id }}-\${{ github.run_attempt }}`); + expect(workflow.match(/uses: actions\/upload-artifact@/gu)).toHaveLength(1); + for (const job of [validate, publish]) { + expect(job).toContain(`artifact-ids: \${{ needs.propose.outputs.artifact-id }}`); + expect(job).toContain('digest-mismatch: error'); + expect(job).toContain(`EXPECTED_BASE_SHA: \${{ github.sha }}`); + expect(job).toContain('python3 scripts/node-version-review.py apply'); + } + expect(validate.indexOf('Verify and apply')).toBeLessThan(validate.indexOf('Set up pnpm')); + expect(validate.indexOf('Verify and apply')).toBeLessThan(validate.indexOf('Set up Node')); + expect(publish.indexOf('Preserve the validated base')).toBeLessThan(publish.indexOf('Verify and apply')); + expect(publish.indexOf('Verify and apply')).toBeLessThan(publish.indexOf('create-pull-request@')); + for (const action of workflow.matchAll(/uses: (?[^\n ]+)/gu)) { + expect(action[1]).toMatch(/@[a-f0-9]{40}$/u); + } + }); +}); From 75d3b66d1b02d7251dcbb5e52b426ce399dc1828 Mon Sep 17 00:00:00 2001 From: Hayden Date: Mon, 7 Sep 2026 18:09:37 +0000 Subject: [PATCH 2/2] test: construct duplicate JSON fixture directly --- tests/node-version-review.test.ts | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/tests/node-version-review.test.ts b/tests/node-version-review.test.ts index 20835d989..1f56bffe8 100644 --- a/tests/node-version-review.test.ts +++ b/tests/node-version-review.test.ts @@ -179,7 +179,8 @@ describeOnUnix('monthly Node review proposal boundary', () => { }); test.each(['"body":"first",', '"\\u0062ody":"first",'])('rejects duplicate envelope keys %s', (key) => { - rejects(JSON.stringify(proposal()).replace('{', `{${key}`)); + const raw = JSON.stringify(proposal()); + rejects(`{${key}${raw.slice(1)}`); }); test.each(['NaN', 'Infinity', '-Infinity'])('rejects nonstandard JSON %s', (constant) => {