From 29b4bf34b5c268f03db9aff2749d6ad2b43ed33a Mon Sep 17 00:00:00 2001 From: Kyleasmth <45497561+Kyleasmth@users.noreply.github.com> Date: Mon, 5 Oct 2026 12:55:24 -0700 Subject: [PATCH 1/5] feat(ci): pin the breaking-change signoff to the release, not the commit hash Port of the RN Expo change (YPE-6067). The signoff comment named the PR head SHA, so every push voided it: on Expo's first real major that cost four rounds, three of them lost to a lint fix and review feedback that changed nothing about what was approved. It is also stricter than this repo's own review policy, where `dismiss_stale_reviews_on_push` is false, so an ordinary code review survives a push while a breaking-change acknowledgment did not. React has not hit this only because no major has been through its gate yet. The signoff now names a token derived from the release: every changeset at head by git blob id, plus the resulting version. Ordinary pushes leave it standing; adding or editing a changeset, or moving the version, requires a fresh one. `isChangeset` is hoisted to module scope so the token and the major detector read the same rule rather than drifting apart. --- .github/scripts/major-release-signoff.test.sh | 33 +++++++++++++ .github/workflows/major-release-signoff.yml | 28 ++++++++--- scripts/preview-release.mjs | 48 ++++++++++++++++++- 3 files changed, 102 insertions(+), 7 deletions(-) diff --git a/.github/scripts/major-release-signoff.test.sh b/.github/scripts/major-release-signoff.test.sh index 90cf61f7..b3958dd4 100644 --- a/.github/scripts/major-release-signoff.test.sh +++ b/.github/scripts/major-release-signoff.test.sh @@ -454,5 +454,38 @@ else pass "an unresolved context fails the status on every event" fi +# --- signoff is pinned to the release, not the commit (YPE-6068) --------------- + +if grep -q 'TOKEN_RE="(\^|\[\^0-9A-Za-z\])\${SIGNOFF_TOKEN}' "$WORKFLOW" \ + && ! grep -q 'SHA_RE=' "$WORKFLOW"; then + pass "the signoff matches the release token, not the head SHA" +else + fail "the signoff matches the release token, not the head SHA" \ + "matching on HEAD_SHA voids a valid signoff on every push" +fi + +if grep -q 'SIGNOFF_TOKEN" =~ \^\[0-9a-f\]{16}\$' "$WORKFLOW"; then + pass "a malformed release token fails closed" +else + fail "a malformed release token fails closed" \ + "an empty token would degenerate the matcher into one that matches almost any comment" +fi + +if grep -q 'echo "v\$NEXT_VERSION \$SIGNOFF_TOKEN' "$WORKFLOW" \ + && ! grep -q 'echo "v\$NEXT_VERSION \$HEAD_SHA' "$WORKFLOW"; then + pass "the copy-paste block names the token the matcher looks for" +else + fail "the copy-paste block names the token the matcher looks for" \ + "instructions that name a different value than the matcher are unfollowable" +fi + +if grep -q 'signoff_token: \${{ steps.preview.outputs.signoff_token }}' "$WORKFLOW" \ + && grep -q 'SIGNOFF_TOKEN: \${{ needs.preview.outputs.signoff_token }}' "$WORKFLOW"; then + pass "the token reaches the signoff and comment steps from the preview job" +else + fail "the token reaches the signoff and comment steps from the preview job" \ + "an unset SIGNOFF_TOKEN is caught by the guard, but only after the job has already run" +fi + printf '\n%d passed, %d failed\n' "$passes" "$failures" [[ "$failures" -eq 0 ]] diff --git a/.github/workflows/major-release-signoff.yml b/.github/workflows/major-release-signoff.yml index bf3e70d3..eb814471 100644 --- a/.github/workflows/major-release-signoff.yml +++ b/.github/workflows/major-release-signoff.yml @@ -259,6 +259,7 @@ jobs: outputs: is_major: ${{ steps.preview.outputs.is_major }} next: ${{ steps.preview.outputs.next }} + signoff_token: ${{ steps.preview.outputs.signoff_token }} release_type: ${{ steps.preview.outputs.release_type }} steps: - name: Checkout PR head @@ -344,6 +345,7 @@ jobs: # "null", which is not empty, so the `${VAR:-default}` fallbacks downstream never # fire and the status reads "No breaking changeset added (null)". echo "next=$(jq -r '.next // empty' preview.json)" >> "$GITHUB_OUTPUT" + echo "signoff_token=$(jq -r '.signoff_token // empty' preview.json)" >> "$GITHUB_OUTPUT" echo "release_type=$(jq -r '.release_type // empty' preview.json)" >> "$GITHUB_OUTPUT" @@ -503,6 +505,7 @@ jobs: PR_AUTHOR: ${{ needs.context.outputs.pr_author }} HEAD_SHA: ${{ needs.context.outputs.head_sha }} NEXT_VERSION: ${{ steps.decision.outputs.next }} + SIGNOFF_TOKEN: ${{ needs.preview.outputs.signoff_token }} run: | set -euo pipefail # Guard: a major changeset must also have produced a real `next` @@ -539,8 +542,20 @@ jobs: # The full 40 characters, not an abbreviation: a 7-character prefix is # 28 bits, which is cheap to brute-force, so an author could craft a new # commit sharing the approved prefix and inherit its signoff. - SHA_RE="(^|[^0-9A-Za-z])${HEAD_SHA}([^0-9A-Za-z]|$)" - echo "Signoff must name head $HEAD_SHA." + # A signoff approves a release, so it names that release: the `.changeset/` state plus + # the resulting version, digested by `signoffToken` in scripts/preview-release.mjs. + # Naming the head SHA instead voided the signoff on every push, so a lint fix cost a + # fresh one while changing nothing about what was approved. Comparing timestamps is not + # an option either: `committer.date` is attacker-controlled, so a backdated commit could + # inherit an older approval. The digest is content-derived and cannot be backdated. + # 64 bits, not a short prefix, so a second preimage against an approved token is out of + # reach for someone wanting a different release to inherit its signoff. + if ! [[ "$SIGNOFF_TOKEN" =~ ^[0-9a-f]{16}$ ]]; then + echo "::error::Refusing to evaluate a signoff without a well-formed release token." + exit 1 + fi + TOKEN_RE="(^|[^0-9A-Za-z])${SIGNOFF_TOKEN}([^0-9A-Za-z]|$)" + echo "Signoff must name release token $SIGNOFF_TOKEN." COMMENTS=$(gh api --paginate \ "repos/${{ github.repository }}/issues/$PR_NUMBER/comments" \ @@ -563,7 +578,7 @@ jobs: NORM_BODY=$(normalize "$BODY") if [[ "$NORM_BODY" != *"$NORM_TEMPLATE"* ]]; then continue; fi if ! [[ "$BODY" =~ $VERSION_RE ]] || ! [[ "$BODY" =~ $ROCKET_RE ]]; then continue; fi - if ! [[ "$BODY" =~ $SHA_RE ]]; then continue; fi + if ! [[ "$BODY" =~ $TOKEN_RE ]]; then continue; fi # Then verify write+ permission. Cached so a chatty approver doesn't # cost one API call per comment. PERM="${PERM_CACHE[$LOGIN]:-}" @@ -609,6 +624,7 @@ jobs: PR_NUMBER: ${{ needs.context.outputs.pr_number }} HEAD_SHA: ${{ needs.context.outputs.head_sha }} NEXT_VERSION: ${{ steps.decision.outputs.next }} + SIGNOFF_TOKEN: ${{ needs.preview.outputs.signoff_token }} run: | set -euo pipefail MARKER='' @@ -622,7 +638,7 @@ jobs: echo echo "1. The verbatim acknowledgment phrase below," echo "2. The precise next version (\`v$NEXT_VERSION\` or \`$NEXT_VERSION\`)," - echo "3. The full commit hash being approved (\`$HEAD_SHA\`), and" + echo "3. The release token being approved (\`$SIGNOFF_TOKEN\`), and" echo "4. A 🚀 (\`:rocket:\`) emoji." echo echo "I confirm that this is an intentional breaking change, and I have read the release procedures. I understand and have documented its impact upon release." @@ -632,10 +648,10 @@ jobs: echo '```' echo "I confirm that this is an intentional breaking change, and I have read the release procedures. I understand and have documented its impact upon release." echo - echo "v$NEXT_VERSION $HEAD_SHA 🚀" + echo "v$NEXT_VERSION $SIGNOFF_TOKEN 🚀" echo '```' echo - echo "The check re-runs automatically when a qualifying comment is posted or edited. Pushing new commits changes the hash, so a fresh signoff is needed." + echo "The check re-runs automatically when a qualifying comment is posted or edited. The token covers this release's changesets and version, so ordinary pushes (review fixes, lint, docs) leave this signoff standing; adding or editing a changeset, or a change to the version, needs a fresh one." } > comment.md EXISTING=$(gh api --paginate \ diff --git a/scripts/preview-release.mjs b/scripts/preview-release.mjs index a54b7c20..12f91e11 100755 --- a/scripts/preview-release.mjs +++ b/scripts/preview-release.mjs @@ -22,6 +22,7 @@ // Usage: // node scripts/preview-release.mjs --base [--head ] import { execFileSync } from 'node:child_process'; +import { createHash } from 'node:crypto'; import parseChangeset from '@changesets/parse'; import { mkdtempSync, readFileSync, rmSync } from 'node:fs'; import { tmpdir } from 'node:os'; @@ -135,7 +136,7 @@ function addedChangesetLevels(base) { // `[^]` rather than `.`: a filename containing a line terminator would otherwise fail // this test and drop out of the scan entirely. - const isChangeset = (f) => /^\.changeset\/[^]+\.md$/.test(f) && !/README\.md$/.test(f); + const isChangeset = isChangesetPath; const levels = []; const touched = []; @@ -177,6 +178,51 @@ if (versions.length > 1) { process.exit(1); } +/** Mirrors @changesets/read: a `.md` under `.changeset/` that is not README. */ +const isChangesetPath = (f) => /^\.changeset\/[^]+\.md$/.test(f) && !/README\.md$/.test(f); + +/** + * A stable identifier for *what a signoff approves*: the release's `.changeset/` state. + * + * Pinning the signoff to the head commit meant every push voided it, so a lint fix or a review + * tweak cost a fresh signoff while changing nothing about the release. That trains people to + * rubber-stamp the gate, and it is stricter than the repo's own review policy, which keeps + * approvals across pushes (`dismiss_stale_reviews_on_push: false`). + * + * Keyed on every changeset at head plus the resulting version, deliberately wider than "only the + * changesets declaring major": any changeset edit can move the release, and erring toward asking + * again is the safe direction. Blob ids come from git, so the hash changes if and only if the + * content does. Renaming a changeset re-triggers, which is correct: Changesets reads it by path. + * + * 16 hex characters (64 bits). The signoff names this value, so a collaborator who wanted a + * different release to inherit an existing signoff would have to grind a second preimage against + * it; 64 bits puts that out of reach, where a short prefix would not. + */ +function signoffToken(nextVersion) { + const raw = execFileSync('git', ['ls-tree', '-r', '-z', head, '--', '.changeset'], { + cwd: REPO_ROOT, + encoding: 'utf8', + maxBuffer: 64 * 1024 * 1024, + }); + const entries = raw + .split('\0') + .filter((line) => line !== '') + .map((line) => { + // ` \t` + const [meta, path] = line.split('\t'); + const [, , object] = meta.split(/\s+/); + return { path, object }; + }) + .filter(({ path }) => isChangesetPath(path)) + .sort((a, b) => (a.path < b.path ? -1 : a.path > b.path ? 1 : 0)) + .map(({ path, object }) => `${object} ${path}`); + + return createHash('sha256') + .update(`${nextVersion ?? ''}\n${entries.join('\n')}`) + .digest('hex') + .slice(0, 16); +} + const { added, levels } = addedChangesetLevels(args.base); console.log( From fba8c9e2e3f0f1729eab42f73de9ee22f57565e2 Mon Sep 17 00:00:00 2001 From: Kyleasmth <45497561+Kyleasmth@users.noreply.github.com> Date: Mon, 5 Oct 2026 13:56:24 -0700 Subject: [PATCH 2/5] chore: add an empty changeset for the CI-only signoff change --- .changeset/signoff-release-token.md | 4 ++++ 1 file changed, 4 insertions(+) create mode 100644 .changeset/signoff-release-token.md diff --git a/.changeset/signoff-release-token.md b/.changeset/signoff-release-token.md new file mode 100644 index 00000000..067d4efe --- /dev/null +++ b/.changeset/signoff-release-token.md @@ -0,0 +1,4 @@ +--- +--- + +Pin the breaking-change signoff to the release's changesets and version rather than the PR head commit, so an ordinary push no longer voids a valid signoff. CI-only, no package changes. From d7c7e9b2bd246d1c160c2de0593a9b912ebf9a12 Mon Sep 17 00:00:00 2001 From: Kyleasmth <45497561+Kyleasmth@users.noreply.github.com> Date: Tue, 6 Oct 2026 11:11:49 -0700 Subject: [PATCH 3/5] fix(ci): emit the signoff token, and cover it with behavioural tests Greptile found that `signoffToken` was defined but never called here, so the preview emitted no `signoff_token`, the gate's guard rejected the empty value, and every major PR would have been blocked behind a signoff that could never pass. The earlier edit missed because it matched on a six-space indent where this file uses four, and a no-op `String.replace` reports nothing. The four assertions that were supposed to catch this only read the workflow's text, so they all passed while the key was never written. Replaced with checks that bind the two sides together. - emit `signoff_token`, and assert the preview writes the key the workflow's jq reads - move `signoffToken` into its own module so the suite can drive it against real git trees - split the ls-tree line at the first tab only; a tab in a changeset path was truncating it out of the digest, and a file that is absent cannot be seen to change - restore the module from main with the rest of the tooling, or a PR could ship its own token logic and mint one matching an older signoff Eight behavioural cases plus the contract and restore-list assertions, each mutation-tested: removing the emit, dropping the module from the restore list, and reverting the tab fix each turn a test red. --- .github/scripts/major-release-signoff.test.sh | 19 ++++++ .github/scripts/signoff-token.test.mjs | 64 +++++++++++++++++++ .github/workflows/major-release-signoff.yml | 2 +- package.json | 2 +- scripts/preview-release.mjs | 50 +-------------- scripts/signoff-token.mjs | 54 ++++++++++++++++ 6 files changed, 142 insertions(+), 49 deletions(-) create mode 100644 .github/scripts/signoff-token.test.mjs create mode 100644 scripts/signoff-token.mjs diff --git a/.github/scripts/major-release-signoff.test.sh b/.github/scripts/major-release-signoff.test.sh index b3958dd4..7da8a4a4 100644 --- a/.github/scripts/major-release-signoff.test.sh +++ b/.github/scripts/major-release-signoff.test.sh @@ -487,5 +487,24 @@ else "an unset SIGNOFF_TOKEN is caught by the guard, but only after the job has already run" fi +if grep -qF 'scripts/preview-release.mjs scripts/signoff-token.mjs' "$WORKFLOW"; then + pass "the token module is restored from main with the rest of the tooling" +else + fail "the token module is restored from main with the rest of the tooling" \ + "a PR-supplied token module could mint a token matching an older signoff" +fi + +# The gate reads `.signoff_token` out of preview.json. If the preview stops emitting it the +# guard rejects an empty token and every major PR blocks, which is how this shipped the first +# time: four assertions about the workflow's text all passed while the key was never written. +PREVIEW_SRC="$ROOT/scripts/preview-release.mjs" +if grep -q "signoff_token: signoffToken(" "$PREVIEW_SRC" \ + && grep -q "jq -r '.signoff_token // empty' preview.json" "$WORKFLOW"; then + pass "the preview emits the token under the key the gate reads" +else + fail "the preview emits the token under the key the gate reads" \ + "an unemitted or renamed key blocks every major PR behind an empty-token guard" +fi + printf '\n%d passed, %d failed\n' "$passes" "$failures" [[ "$failures" -eq 0 ]] diff --git a/.github/scripts/signoff-token.test.mjs b/.github/scripts/signoff-token.test.mjs new file mode 100644 index 00000000..08eba756 --- /dev/null +++ b/.github/scripts/signoff-token.test.mjs @@ -0,0 +1,64 @@ +// Drives signoffToken against real git trees. Each case mutates a scratch repo and asserts +// whether the token moved, which is the property the gate relies on. +import { execFileSync } from 'node:child_process' +import { mkdtempSync, mkdirSync, writeFileSync, rmSync, renameSync } from 'node:fs' +import { tmpdir } from 'node:os' +import { join } from 'node:path' +import { signoffToken } from '../../scripts/signoff-token.mjs' + +const repo = mkdtempSync(join(tmpdir(), 'signoff-token-')) +const git = (...a) => execFileSync('git', a, { cwd: repo, encoding: 'utf8' }).trim() +const commit = (msg) => { git('add', '-A'); git('-c', 'commit.gpgsign=false', 'commit', '-q', '-m', msg); return git('rev-parse', 'HEAD') } +const tok = (head, v = '2.0.0') => signoffToken({ repoRoot: repo, head, nextVersion: v }) + +git('init', '-q', '-b', 'main'); git('config', 'user.email', 't@t'); git('config', 'user.name', 't') +mkdirSync(join(repo, '.changeset')) +writeFileSync(join(repo, '.changeset', 'a.md'), "---\n'p': major\n---\n\nbreak\n") +writeFileSync(join(repo, '.changeset', 'README.md'), 'docs\n') +writeFileSync(join(repo, 'src.txt'), 'code\n') +const base = commit('base'); const baseTok = tok(base) + +const results = [] +const check = (name, actual, expectSame) => + results.push({ name, ok: (actual === baseTok) === expectSame, actual, expectSame }) + +writeFileSync(join(repo, 'src.txt'), 'code changed\n') +check('ordinary code push keeps the token', tok(commit('code')), true) + +writeFileSync(join(repo, '.changeset', 'README.md'), 'docs edited\n') +check('editing .changeset/README.md keeps the token', tok(commit('readme')), true) + +check('a version change replaces the token', tok(git('rev-parse', 'HEAD'), '2.0.1'), false) + +const afterDocs = git('rev-parse', 'HEAD') +writeFileSync(join(repo, '.changeset', 'b.md'), "---\n'p': patch\n---\n\nfix\n") +check('adding a changeset replaces the token', tok(commit('add changeset')), false) + +git('reset', '-q', '--hard', afterDocs) +writeFileSync(join(repo, '.changeset', 'a.md'), "---\n'p': major\n---\n\nbreak differently\n") +check('editing a changeset replaces the token', tok(commit('edit changeset')), false) + +git('reset', '-q', '--hard', afterDocs) +rmSync(join(repo, '.changeset', 'a.md')) +check('deleting a changeset replaces the token', tok(commit('delete changeset')), false) + +git('reset', '-q', '--hard', afterDocs) +renameSync(join(repo, '.changeset', 'a.md'), join(repo, '.changeset', 'renamed.md')) +check('renaming a changeset replaces the token', tok(commit('rename changeset')), false) + +// A tab in the path is why the blob line is split at the first tab only. Splitting on every +// tab drops the file from the digest, and a dropped file cannot be seen to change, so the +// discriminating assertion is that EDITING it moves the token. +git('reset', '-q', '--hard', afterDocs) +renameSync(join(repo, '.changeset', 'a.md'), join(repo, '.changeset', 'with\ttab.md')) +const tabBefore = tok(commit('tab-named changeset')) +writeFileSync(join(repo, '.changeset', 'with\ttab.md'), "---\n'p': major\n---\n\nbreak, reworded\n") +const tabAfter = tok(commit('edit the tab-named changeset')) +results.push({ + name: 'editing a tab-named changeset replaces the token', + ok: tabBefore !== tabAfter, +}) + +rmSync(repo, { recursive: true, force: true }) +for (const r of results) console.log(`${r.ok ? 'ok' : 'FAIL'}\t${r.name}`) +process.exit(results.every((r) => r.ok) ? 0 : 1) diff --git a/.github/workflows/major-release-signoff.yml b/.github/workflows/major-release-signoff.yml index eb814471..d5b9406a 100644 --- a/.github/workflows/major-release-signoff.yml +++ b/.github/workflows/major-release-signoff.yml @@ -280,7 +280,7 @@ jobs: # under, and the changesets config have to agree, so restoring some from main and # taking the rest from the PR gives a detector without its own dependencies. if git cat-file -e main:scripts/preview-release.mjs 2>/dev/null; then - for f in scripts/preview-release.mjs package.json pnpm-lock.yaml \ + for f in scripts/preview-release.mjs scripts/signoff-token.mjs package.json pnpm-lock.yaml \ pnpm-workspace.yaml .changeset/config.json; do git cat-file -e "main:$f" 2>/dev/null && git checkout main -- "$f" done diff --git a/package.json b/package.json index 1fb7d39b..e83d5a54 100644 --- a/package.json +++ b/package.json @@ -42,7 +42,7 @@ "analyze:select": "node scripts/analyze-select.mjs", "generate:i18n": "pnpm --filter @youversion/platform-react-ui generate:i18n", "check:i18n": "node scripts/check-i18n-parity.mjs", - "test:ci-scripts": "bash .github/scripts/check-locale-ownership.test.sh && bash .github/scripts/major-release-signoff.test.sh", + "test:ci-scripts": "bash .github/scripts/check-locale-ownership.test.sh && bash .github/scripts/major-release-signoff.test.sh && node .github/scripts/signoff-token.test.mjs", "storybook": "pnpm --filter @youversion/platform-react-ui storybook" }, "devDependencies": { diff --git a/scripts/preview-release.mjs b/scripts/preview-release.mjs index 12f91e11..6a5a5593 100755 --- a/scripts/preview-release.mjs +++ b/scripts/preview-release.mjs @@ -17,12 +17,12 @@ // unrelated PR until the release went out. // // Output (stdout): one line of JSON: -// { current, next, release_type, is_major, introduced_major, packages, added_changesets } +// { current, next, release_type, is_major, introduced_major, packages, added_changesets, signoff_token } // // Usage: // node scripts/preview-release.mjs --base [--head ] import { execFileSync } from 'node:child_process'; -import { createHash } from 'node:crypto'; +import { isChangesetPath, signoffToken } from './signoff-token.mjs'; import parseChangeset from '@changesets/parse'; import { mkdtempSync, readFileSync, rmSync } from 'node:fs'; import { tmpdir } from 'node:os'; @@ -178,51 +178,6 @@ if (versions.length > 1) { process.exit(1); } -/** Mirrors @changesets/read: a `.md` under `.changeset/` that is not README. */ -const isChangesetPath = (f) => /^\.changeset\/[^]+\.md$/.test(f) && !/README\.md$/.test(f); - -/** - * A stable identifier for *what a signoff approves*: the release's `.changeset/` state. - * - * Pinning the signoff to the head commit meant every push voided it, so a lint fix or a review - * tweak cost a fresh signoff while changing nothing about the release. That trains people to - * rubber-stamp the gate, and it is stricter than the repo's own review policy, which keeps - * approvals across pushes (`dismiss_stale_reviews_on_push: false`). - * - * Keyed on every changeset at head plus the resulting version, deliberately wider than "only the - * changesets declaring major": any changeset edit can move the release, and erring toward asking - * again is the safe direction. Blob ids come from git, so the hash changes if and only if the - * content does. Renaming a changeset re-triggers, which is correct: Changesets reads it by path. - * - * 16 hex characters (64 bits). The signoff names this value, so a collaborator who wanted a - * different release to inherit an existing signoff would have to grind a second preimage against - * it; 64 bits puts that out of reach, where a short prefix would not. - */ -function signoffToken(nextVersion) { - const raw = execFileSync('git', ['ls-tree', '-r', '-z', head, '--', '.changeset'], { - cwd: REPO_ROOT, - encoding: 'utf8', - maxBuffer: 64 * 1024 * 1024, - }); - const entries = raw - .split('\0') - .filter((line) => line !== '') - .map((line) => { - // ` \t` - const [meta, path] = line.split('\t'); - const [, , object] = meta.split(/\s+/); - return { path, object }; - }) - .filter(({ path }) => isChangesetPath(path)) - .sort((a, b) => (a.path < b.path ? -1 : a.path > b.path ? 1 : 0)) - .map(({ path, object }) => `${object} ${path}`); - - return createHash('sha256') - .update(`${nextVersion ?? ''}\n${entries.join('\n')}`) - .digest('hex') - .slice(0, 16); -} - const { added, levels } = addedChangesetLevels(args.base); console.log( @@ -234,5 +189,6 @@ console.log( introduced_major: levels.some((l) => l.level === 'major'), packages: releases.map((r) => r.name), added_changesets: added, + signoff_token: signoffToken({ repoRoot: REPO_ROOT, head, nextVersion: versions[0] ?? null }), }), ); diff --git a/scripts/signoff-token.mjs b/scripts/signoff-token.mjs new file mode 100644 index 00000000..20007463 --- /dev/null +++ b/scripts/signoff-token.mjs @@ -0,0 +1,54 @@ +#!/usr/bin/env node +/** + * The identifier a breaking-change signoff names. + * + * Pinning a signoff to the head commit meant every push voided it, so a lint fix or a review + * tweak cost a fresh signoff while changing nothing about the release. That trains people to + * rubber-stamp the gate, and it is stricter than this repo's own review policy, which keeps + * approvals across pushes (`dismiss_stale_reviews_on_push: false`). + * + * Keyed on every changeset at head plus the resulting version, deliberately wider than "only the + * changesets declaring major": any changeset edit can move the release, and erring toward asking + * again is the safe direction. Blob ids come from git, so the hash changes if and only if the + * content does. Renaming a changeset re-triggers, which is correct: Changesets reads it by path. + * + * 16 hex characters (64 bits). The signoff names this value, so a collaborator who wanted a + * different release to inherit an existing signoff would have to grind a second preimage against + * it; 64 bits puts that out of reach, where a short prefix would not. + * + * Lives in its own module so the gate's suite can exercise it against real git trees. Testing it + * through `preview-release.mjs` would mean standing up Changesets and a workspace first, which is + * how it previously shipped covered only by assertions about the workflow's text. + */ +import { execFileSync } from 'node:child_process'; +import { createHash } from 'node:crypto'; + +/** Mirrors @changesets/read: a `.md` under `.changeset/` that is not README. */ +export const isChangesetPath = (f) => + /^\.changeset\/[^]+\.md$/.test(f) && !/README\.md$/.test(f); + +export function signoffToken({ repoRoot, head, nextVersion }) { + const raw = execFileSync('git', ['ls-tree', '-r', '-z', head, '--', '.changeset'], { + cwd: repoRoot, + encoding: 'utf8', + maxBuffer: 64 * 1024 * 1024, + }); + const entries = raw + .split('\0') + .filter((line) => line !== '') + .map((line) => { + // ` \t`. Split at the first tab only: a path may contain one, + // and losing its tail would drop the file from the digest, so an edit to it would not + // re-trigger a signoff. + const tab = line.indexOf('\t'); + return { meta: line.slice(0, tab), path: line.slice(tab + 1) }; + }) + .filter(({ path }) => isChangesetPath(path)) + .sort((a, b) => (a.path < b.path ? -1 : a.path > b.path ? 1 : 0)) + .map(({ meta, path }) => `${meta.split(/\s+/)[2]} ${path}`); + + return createHash('sha256') + .update(`${nextVersion ?? ''}\n${entries.join('\n')}`) + .digest('hex') + .slice(0, 16); +} From f14b1d21412026eac1d2c5260e01f5e04a16e58c Mon Sep 17 00:00:00 2001 From: Kyleasmth <45497561+Kyleasmth@users.noreply.github.com> Date: Tue, 6 Oct 2026 11:45:34 -0700 Subject: [PATCH 4/5] refactor(ci): keep the one-line destructure and still split at the first tab --- scripts/signoff-token.mjs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/scripts/signoff-token.mjs b/scripts/signoff-token.mjs index 20007463..c9cff638 100644 --- a/scripts/signoff-token.mjs +++ b/scripts/signoff-token.mjs @@ -40,8 +40,8 @@ export function signoffToken({ repoRoot, head, nextVersion }) { // ` \t`. Split at the first tab only: a path may contain one, // and losing its tail would drop the file from the digest, so an edit to it would not // re-trigger a signoff. - const tab = line.indexOf('\t'); - return { meta: line.slice(0, tab), path: line.slice(tab + 1) }; + const [meta, path] = line.split(/\t(.*)/s); + return { meta, path }; }) .filter(({ path }) => isChangesetPath(path)) .sort((a, b) => (a.path < b.path ? -1 : a.path > b.path ? 1 : 0)) From 62e73f492904b98ad8f537e99e749dab2b7b7313 Mon Sep 17 00:00:00 2001 From: Kyleasmth <45497561+Kyleasmth@users.noreply.github.com> Date: Thu, 8 Oct 2026 13:55:57 -0700 Subject: [PATCH 5/5] ci: key the signoff token on this PR's changesets and config, and run the script tests in CI --- .github/scripts/signoff-token.test.mjs | 30 ++++++++++- .github/workflows/ci.yml | 3 ++ .github/workflows/major-release-signoff.yml | 12 ++--- scripts/preview-release.mjs | 7 ++- scripts/signoff-token.mjs | 58 +++++++++++++-------- 5 files changed, 75 insertions(+), 35 deletions(-) diff --git a/.github/scripts/signoff-token.test.mjs b/.github/scripts/signoff-token.test.mjs index 08eba756..a4a53dd9 100644 --- a/.github/scripts/signoff-token.test.mjs +++ b/.github/scripts/signoff-token.test.mjs @@ -9,14 +9,16 @@ import { signoffToken } from '../../scripts/signoff-token.mjs' const repo = mkdtempSync(join(tmpdir(), 'signoff-token-')) const git = (...a) => execFileSync('git', a, { cwd: repo, encoding: 'utf8' }).trim() const commit = (msg) => { git('add', '-A'); git('-c', 'commit.gpgsign=false', 'commit', '-q', '-m', msg); return git('rev-parse', 'HEAD') } -const tok = (head, v = '2.0.0') => signoffToken({ repoRoot: repo, head, nextVersion: v }) +let base +const tok = (head, v = '2.0.0') => signoffToken({ repoRoot: repo, base, head, nextVersion: v }) git('init', '-q', '-b', 'main'); git('config', 'user.email', 't@t'); git('config', 'user.name', 't') mkdirSync(join(repo, '.changeset')) writeFileSync(join(repo, '.changeset', 'a.md'), "---\n'p': major\n---\n\nbreak\n") writeFileSync(join(repo, '.changeset', 'README.md'), 'docs\n') writeFileSync(join(repo, 'src.txt'), 'code\n') -const base = commit('base'); const baseTok = tok(base) +writeFileSync(join(repo, '.changeset', 'config.json'), '{}\n') +base = commit('base'); const baseTok = tok(base) const results = [] const check = (name, actual, expectSame) => @@ -59,6 +61,30 @@ results.push({ ok: tabBefore !== tabAfter, }) +git('reset', '-q', '--hard', afterDocs) +writeFileSync(join(repo, '.changeset', 'config.json'), '{"fixed": []}\n') +check('editing .changeset/config.json replaces the token', tok(commit('edit config')), false) + +// The branch has to be current with main before it merges, and every other PR brings its own +// changeset. Merging main in must leave the signoff standing, so only this PR's changesets count. +git('reset', '-q', '--hard', afterDocs) +git('checkout', '-q', '-b', 'pr') +writeFileSync(join(repo, '.changeset', 'mine.md'), "---\n'p': major\n---\n\nmine\n") +const prTok = tok(commit('pr changeset')) +git('checkout', '-q', 'main') +git('reset', '-q', '--hard', afterDocs) +writeFileSync(join(repo, '.changeset', 'theirs.md'), "---\n'p': patch\n---\n\ntheirs\n") +commit('another PR lands on main') +git('checkout', '-q', 'pr') +git('-c', 'commit.gpgsign=false', 'merge', '-q', '--no-edit', 'main') +const savedBase = base +base = 'main' +results.push({ + name: 'merging a changeset in from main keeps the token', + ok: tok(git('rev-parse', 'HEAD')) === prTok, +}) +base = savedBase + rmSync(repo, { recursive: true, force: true }) for (const r of results) console.log(`${r.ok ? 'ok' : 'FAIL'}\t${r.name}`) process.exit(results.every((r) => r.ok) ? 0 : 1) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 76560c8a..1ef77d03 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -83,6 +83,9 @@ jobs: - name: Run lint run: pnpm lint + - name: Run repo script regression tests + run: pnpm test:ci-scripts + typecheck: name: Type Check runs-on: ubuntu-latest diff --git a/.github/workflows/major-release-signoff.yml b/.github/workflows/major-release-signoff.yml index d5b9406a..52581857 100644 --- a/.github/workflows/major-release-signoff.yml +++ b/.github/workflows/major-release-signoff.yml @@ -535,15 +535,9 @@ jobs: } NORM_TEMPLATE=$(normalize "$TEMPLATE_PHRASE") - # A signoff approves the code it was given on, so it names that code. Comparing - # timestamps instead would trust `committer.date`, which the committer sets, so a - # backdated commit could inherit an older approval. A SHA cannot be backdated, and - # a new push changes it, which retires the previous signoff on its own. - # The full 40 characters, not an abbreviation: a 7-character prefix is - # 28 bits, which is cheap to brute-force, so an author could craft a new - # commit sharing the approved prefix and inherit its signoff. - # A signoff approves a release, so it names that release: the `.changeset/` state plus - # the resulting version, digested by `signoffToken` in scripts/preview-release.mjs. + # A signoff approves a release, so it names that release: the changesets this PR + # touches, `.changeset/config.json` and the resulting version, digested by + # `signoffToken` in scripts/signoff-token.mjs. # Naming the head SHA instead voided the signoff on every push, so a lint fix cost a # fresh one while changing nothing about what was approved. Comparing timestamps is not # an option either: `committer.date` is attacker-controlled, so a backdated commit could diff --git a/scripts/preview-release.mjs b/scripts/preview-release.mjs index 6a5a5593..c7704821 100755 --- a/scripts/preview-release.mjs +++ b/scripts/preview-release.mjs @@ -189,6 +189,11 @@ console.log( introduced_major: levels.some((l) => l.level === 'major'), packages: releases.map((r) => r.name), added_changesets: added, - signoff_token: signoffToken({ repoRoot: REPO_ROOT, head, nextVersion: versions[0] ?? null }), + signoff_token: signoffToken({ + repoRoot: REPO_ROOT, + base: args.base, + head, + nextVersion: versions[0] ?? null, + }), }), ); diff --git a/scripts/signoff-token.mjs b/scripts/signoff-token.mjs index c9cff638..bc28f0a3 100644 --- a/scripts/signoff-token.mjs +++ b/scripts/signoff-token.mjs @@ -7,10 +7,16 @@ * rubber-stamp the gate, and it is stricter than this repo's own review policy, which keeps * approvals across pushes (`dismiss_stale_reviews_on_push: false`). * - * Keyed on every changeset at head plus the resulting version, deliberately wider than "only the - * changesets declaring major": any changeset edit can move the release, and erring toward asking - * again is the safe direction. Blob ids come from git, so the hash changes if and only if the - * content does. Renaming a changeset re-triggers, which is correct: Changesets reads it by path. + * Keyed on the changesets this PR adds, edits, renames or removes since `base` (the merge-base), plus + * `.changeset/config.json` at head and the resulting version. Keying on every changeset at head + * voided the signoff whenever main was merged in, because main brings other PRs' changesets. + * Every changeset this PR touches counts, not only the ones declaring major: any changeset edit + * can move the release, and erring toward asking again is the safe direction. Blob ids come from + * git, so the hash changes if and only if the content does. Renaming a changeset re-triggers, + * which is correct: Changesets reads it by path. + * + * `config.json` is read from the head commit, not the working tree: the preview job restores + * main's copy before running, but after the merge this PR's copy is the one that governs. * * 16 hex characters (64 bits). The signoff names this value, so a collaborator who wanted a * different release to inherit an existing signoff would have to grind a second preimage against @@ -27,28 +33,34 @@ import { createHash } from 'node:crypto'; export const isChangesetPath = (f) => /^\.changeset\/[^]+\.md$/.test(f) && !/README\.md$/.test(f); -export function signoffToken({ repoRoot, head, nextVersion }) { - const raw = execFileSync('git', ['ls-tree', '-r', '-z', head, '--', '.changeset'], { - cwd: repoRoot, - encoding: 'utf8', - maxBuffer: 64 * 1024 * 1024, - }); - const entries = raw +export function signoffToken({ repoRoot, base, head, nextVersion }) { + const git = (...args) => + execFileSync('git', args, { cwd: repoRoot, encoding: 'utf8', maxBuffer: 64 * 1024 * 1024 }); + // `--raw -z`: `: `, then one path, or two + // for a rename. `-z` keeps unusual paths unquoted, so they still pass the eligibility test. + const fields = git('diff', '--raw', '-z', '-M', '--no-abbrev', base, head, '--', '.changeset') .split('\0') - .filter((line) => line !== '') - .map((line) => { - // ` \t`. Split at the first tab only: a path may contain one, - // and losing its tail would drop the file from the digest, so an edit to it would not - // re-trigger a signoff. - const [meta, path] = line.split(/\t(.*)/s); - return { meta, path }; - }) - .filter(({ path }) => isChangesetPath(path)) - .sort((a, b) => (a.path < b.path ? -1 : a.path > b.path ? 1 : 0)) - .map(({ meta, path }) => `${meta.split(/\s+/)[2]} ${path}`); + .filter((f) => f !== ''); + const changes = []; + for (let i = 0; i < fields.length; ) { + const [, newMode, , newBlob, status] = fields[i].split(' '); + const pathCount = /^[RC]/.test(status) ? 2 : 1; + const paths = fields.slice(i + 1, i + 1 + pathCount); + i += 1 + pathCount; + if (!paths.some(isChangesetPath)) continue; + changes.push([status[0], newMode, newBlob, ...paths]); + } + changes.sort((a, b) => (a.join('\0') < b.join('\0') ? -1 : 1)); + + let config = null; + try { + config = git('rev-parse', '--verify', '--quiet', `${head}:.changeset/config.json`).trim(); + } catch { + // Absent at head. Null still hashes distinctly from any blob id. + } return createHash('sha256') - .update(`${nextVersion ?? ''}\n${entries.join('\n')}`) + .update(JSON.stringify({ nextVersion: nextVersion ?? null, config, changes })) .digest('hex') .slice(0, 16); }