Add a script for adopting fork pull requests - #97252
Merged
Merged
Conversation
Contributor
Stats from current PR🔴 1 regression, 2 improvements
📊 All Metrics📖 Metrics GlossaryDev Server Metrics:
Build Metrics:
Change Thresholds:
⚡ Dev Server
📦 Dev Server (Webpack) (Legacy)📦 Dev Server (Webpack)
⚡ Production Builds
📦 Production Builds (Webpack) (Legacy)📦 Production Builds (Webpack)
📦 Bundle SizesBundle Sizes⚡ TurbopackClient Main Bundles
Server Middleware
Build DetailsBuild Manifests
Build Cache
📦 WebpackClient Main Bundles
Polyfills
Pages
Server Edge SSR
Middleware
Build DetailsBuild Manifests
Build Cache
🔄 Shared (bundler-independent)Runtimes
📎 Tarball URLCommit: a48007c |
Contributor
Tests PassedCommit: a48007c |
![vercel[bot]](https://avatars.githubusercontent.com/in/8329?s=60&v=4){kind=small}
eps1lon
marked this pull request as ready for review
August 12, 2026 18:12
ztanner
reviewed
Aug 13, 2026
| const remote = await detectUpstreamRemote() | ||
| const pr = await fetchPullRequest(prNumber) | ||
|
|
||
| await preflight(pr, branch, dryRun) |
Member
There was a problem hiding this comment.
I think it'd be worth rejecting if headRefOid changes from the time the commands run to when the PR information is fetched, just in case the underlying ref changes and now the contributor is adopting a PR that changed underneath them which could have security implications. Probably should even confirm with the adopter the sha of what they are fetching before proceeding
Member
Author
There was a problem hiding this comment.
we push the original commit that was approved on now.
| console.log(bold(`Checking out #${pr.number} as ${branch}`)) | ||
| // Fetches refs/pull/<n>/head, so the fork does not need to be a remote. The | ||
| // commits are not rewritten: authorship has to reach the merge intact. | ||
| await runInherit('gh', [ |
Member
There was a problem hiding this comment.
gh pr checkout can execute contributor-added hooks locally (for example, .husky/post-checkout). I think adoption should run with hooks disabled for all Git/rebase subprocesses. It may also be worth using a throwaway repo so the untrusted checkout and any failed rebase state never touch the maintainer’s working copy.
Member
Author
There was a problem hiding this comment.
This is out of scope here. You approve before any of that runs. If we're concerned about this, we should reject the script outright.
Member
Author
There was a problem hiding this comment.
I'll see if we can disable the hooks but if that's your threat model, adopting a PR cannot work.
Member
There was a problem hiding this comment.
I don’t think this makes adoption impossible. The intended trust grant is to CI after review, not to contributor-controlled hooks on the maintainer's machine. Disabling hooks during checkout/rebase/push seems like cheap defense in depth in case review misses something.
Member
Author
There was a problem hiding this comment.
all the commands now run with git hooks disabled.
| const headerEnd = lines.indexOf('') | ||
| const headers = headerEnd === -1 ? lines : lines.slice(0, headerEnd) | ||
|
|
||
| if (!headers.some((line) => line.startsWith('gpgsig'))) { |
Member
There was a problem hiding this comment.
This may be an edge case, but a commit can contain a gpgsig header without GitHub considering the signature verified. (ie an unknown key or malformed signature). Should we re-sign unless GitHub reports it as verified?
Member
Author
There was a problem hiding this comment.
We can follow-up if that's an issue.
eps1lon
force-pushed
the
sebbie/fork-pr-adoption-command-9c6eb4
branch
2 times, most recently
from
August 13, 2026 15:11
ad8e723 to
3c47d53
Compare
Pull requests from forks run without repository secrets, so deploy tests never run on external contributions. The workaround is to re-push the contributor's commits to a branch in `vercel/next.js` and open a replacement pull request from there, which is both fiddly to do by hand and genuinely dangerous, so this change adds `scripts/adopt-pr.js` to encode the procedure. The script takes a PR number, resolves which remote points at `vercel/next.js` rather than assuming a name (a maintainer clone has it as `origin`, a fork-based clone as `upstream`), checks out the contributor's commits without rewriting them so authorship survives to the merge, pushes `adopt/<pr-number>`, and opens a draft PR whose body is the contributor's description verbatim behind an `Adopts #N. Closes #N.` line. That line closes the original on merge and cross-links it. The description is copied rather than rewritten because it is the contributor's own text, and because any `Fixes #123` inside it has to survive: the original PR closes unmerged and so never fires its own. The central piece is a confirmation gate. Adoption runs the contributor's code on a branch where CI has repository secrets, which is exactly what a fork PR is denied, so anything executing during install, build, or test can read and exfiltrate them. Before touching git, the script prints the PR link, its status, the author's handle and association, the diff size, and every file the PR touches. That list is deliberately unranked: a payload can sit in any fixture or source file, so marking some paths as risky would only imply the rest are safe. It is gathered from the paginated REST endpoint because `gh pr view --json files` silently caps at 100 entries, and a truncated list would read as the complete surface. If the result still falls short of `changedFiles`, the shortfall is reported rather than hidden. Confirming requires retyping the author's handle so the adopter has to look at who they are trusting, and a non-interactive stdin is refused outright rather than silently proceeding. Draft and closed pull requests are both adoptable, since a contributor may still be iterating or may have abandoned a change that went unreviewed, so their status is surfaced in the header rather than enforced. Merged pull requests are refused because their commits are already in `canary`. The remaining preflight checks also run before anything mutates: the PR must come from a fork to be worth adopting, the working tree must have no uncommitted changes to tracked files, and the target branch must not already exist. Expected problems of that kind print as plain messages, while unexpected failures keep their stack and `cause` chain.
The adopted pull request now inherits the original's base branch. It was being opened against a hard-coded `canary` while the fetched `baseRefName` went unused, so adopting a fork PR aimed at a release branch silently retargeted it. That is not hypothetical: #96739 targets `next-15-5`, and adopting it would have redirected a release-branch fix into `canary`. A base other than the default is highlighted in the summary, since it changes what the change means. Contributor commits usually arrive unsigned, and protected branches require verified signatures, so the branch is re-signed before it is pushed. Signing rewrites commits, so each `Author` is preserved and the tree is compared before and after: re-signing must never alter content, and `--rebase-merges` is not trusted to be content-preserving on faith. If the rebase fails the script aborts it, and if the tree moves it resets and refuses to push. Signature detection deliberately reads the raw commit headers rather than `%G?`. That placeholder reports whether a signature *verifies*, not whether one exists, so with SSH signing and no `gpg.ssh.allowedSignersFile` it reads `N` for every commit, including freshly signed ones and GitHub's own signed merge commits. On #97233 it reports `N` for both commits while the headers correctly distinguish GitHub's signed merge from the contributor's unsigned commit. The script now shells out through `execa` like the other scripts here, and the `UsageError` class is gone in favour of plain errors. `pnpm pr-adopt` is wired up as a shortcut.
Adoption fetched the pull request only after the adopter had reviewed and confirmed it, and took whatever `refs/pull/<n>/head` pointed at by then. A contributor could push in that window, so the code that reached a branch with secrets access was not necessarily the code that was vouched for. The head SHA is now captured with the rest of the metadata, shown in the confirmation summary, and compared against what was actually fetched; a mismatch aborts before anything is pushed. This is not a theoretical window: #97233 moved from `c24c79db` to `5294c014`, and from 7 files to 9, over the course of writing this. Checking out a fork branch also ran contributor code locally. `.husky/*` hook scripts are tracked, so a pull request can add `.husky/post-checkout` or edit `.husky/pre-commit`, and husky's shim executes `.husky/<hook>` from the working tree. Checking the branch out, re-signing it and pushing it each fire hooks, and `rebase --exec` was the worst of them because `git commit --amend` runs `pre-commit` once per replayed commit. Every subprocess now runs with `core.hooksPath` pointed at an empty directory. That is injected through `GIT_CONFIG_COUNT` and friends rather than `-c`, because the environment is inherited by the git processes that `gh` and `git rebase --exec` spawn, which is where the exposure was. An existing `GIT_CONFIG_COUNT` is appended to rather than overwritten. So that no call site can opt out of this by accident, every subprocess goes through one of three helpers, each of which passes that environment. Reaching for `execa` directly to get `reject: false` would have quietly skipped hook suppression, so `runAllowingFailure` covers that case and there is no remaining reason to call `execa` anywhere else in the file. The checkout now happens in a throwaway worktree under the system temp directory, created from the pull request's base so it stays small, and removed in a `finally` on success and failure alike. Contributor files and any half-finished rebase never touch the maintainer's checkout, which is no longer switched at all. The dirty-tree precondition is gone as a result: the working copy is not involved, so it may be as dirty as the maintainer likes.
Working directly in the user's checkout switches their branch out from under them and can leave a failed rebase behind, and it is the wrong place to put a contributor's files when the branch is untrusted. This adds a section to `AGENTS.md` telling agents to prefer a throwaway worktree and to remove it in a cleanup path that also runs on failure. The section carries the two things that make the practice work in this repo. A fresh worktree has no `node_modules`, so `pnpm` and `npx` do not run in it until the root one is symlinked in. That symlink still leaves per-package `node_modules` and a built `packages/next/dist` missing, so `tsc --noEmit` reports `TS2307` for dependencies like `fast-glob` and `dotenv`; those are artifacts of the worktree rather than regressions, and the note says to confirm against the main checkout instead of trying to fix them. The "Linting and Types" section now also says to type-check with `pnpm typescript` rather than a hand-rolled `tsconfig` aimed at one file. The root config includes `scripts/**/*.js` and loads this repo's type augmentations, and a config that misses them reports clean while CI fails. `NodeJS.ProcessEnv` is the example, since `packages/next/types/global.d.ts` declares `NODE_ENV` as required on it.
eps1lon
force-pushed
the
sebbie/fork-pr-adoption-command-9c6eb4
branch
from
August 13, 2026 15:15
3c47d53 to
a48007c
Compare
ztanner
approved these changes
Aug 13, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fork PRs run without repository secrets, so deploy tests never run on external contributions. The workaround is to re-push the contributor's commits to a branch in
vercel/next.jsand open a replacement PR from there, which is fiddly by hand and carries a real security risk. This adds apr-adoptPNPM script to encode the procedure.We copy over the title and body as-is and prepend a
Adopts #N. Closes #N.line so that the underlying PR automatically closes and we create the proper backlinks.Commits are signed
The description is copied rather than rewritten for two reasons: it is the contributor's own text, and any
Fixes #123inside it has to survive, since the original PR closes unmerged and so never fires its own.The confirmation gate
Adoption runs the contributor's code on a branch where CI has repository secrets, which is exactly what a fork PR is denied. Anything executing during install, build, or test can read and exfiltrate them, so before touching git the script prints the PR link, status, author handle and association, diff size, and every file touched. Confirming requires retyping the author's handle, and a non-interactive stdin is refused rather than silently proceeding.
Two details worth a reviewer's attention:
.github/,package.json, lockfiles). That was removed: a payload can sit in any fixture or source file, and printing "no high-risk paths touched" manufactures false assurance.gh pr view --json filessilently caps at 100 entries. Upgrade React fromcbb046ab-20260731to7dfc7ccd-20260803#96550 has 106 changed files and returns 100, which would present a truncated list as the complete surface. If the count still falls short ofchangedFiles(GitHub stops at 3000), the shortfall is reported rather than hidden.Draft and closed PRs are both adoptable, since a contributor may still be iterating or may have abandoned an unreviewed change; the status is surfaced rather than enforced. Merged PRs are refused because their commits are already in
canary.Test plan