Skip to content

Fix pull request head validation for renamed forks and blocked pushes #720

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
coopernetes merged 1 commit into from
Oct 7, 2026
Merged

Fix pull request head validation for renamed forks and blocked pushes #720

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
31 changes: 20 additions & 11 deletions docs/configuration/scm-api.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -61,10 +61,19 @@ The caller's `User-Agent` and the CLI version it advertises are recorded on ever
a CLI upgrade has changed its wire format, which otherwise surfaces only as an unexplained denial. It is not used to
gate: `User-Agent` is caller-controlled, so nothing branches on it.

`require-validated-head` refuses a pull/merge request create whose head commit fogwall has no push record for — closing
the gap where a contributor pushes straight to their fork, never touching fogwall, then opens the pull request through
it. The lookup is keyed on the commit SHA alone, not the repository, since a fork push and the upstream pull/merge
request are two different repositories.
`require-validated-head` refuses a pull/merge request create whose head commit fogwall has not approved or forwarded —
closing the gap where a contributor pushes straight to their fork, never touching fogwall or past a push fogwall
blocked, then opens the pull request through it. A push that was blocked, rejected, canceled or is still awaiting review
does not count. The lookup is keyed on the commit SHA alone, not the repository, since a fork push and the upstream
pull/merge request are two different repositories.

A fork pull request's head is read from the head owner's fork, whatever that fork is named. On GitHub, the head is the
commit GitHub itself resolves `owner:branch` to against the base branch; when the owner holds several forks of the
upstream, that is their oldest one, and GitHub refuses a branch that exists only on a newer fork. On Gitea/Forgejo, it
is the head owner's direct fork of the upstream. For an upstream with at most 50 forks it is found exactly in the
upstream's fork list; for a larger upstream, the first lookup tries the fork of the upstream's name, then searches the
owner's 150 most recently updated forks. A fork once found is cached for 6 hours and re-verified on each use. A head
fogwall cannot resolve is refused.

It defaults on, so a pull/merge request's head must trace to a push fogwall saw. Relax it (set `false`) where these
workflows are common: a rebase, amend, or force-push after pushing through fogwall changes the SHA a validated push
Expand Down Expand Up @@ -100,13 +109,13 @@ resolution, no extra round-trip.

## SCM API properties

| Property | Type | Default | Description |
| ------------------------------------------------- | ------- | ------- | ---------------------------------------------------------------------------------------------------------------------------------------- |
| `scm-api.node-id-cache-ttl` | string | `PT5M` | ISO-8601 duration. See the security note above. |
| `providers.<name>.scm-api.enabled` | boolean | `false` | Whether the SCM API proxy is mounted for this provider. |
| `providers.<name>.scm-api.port` | int | — | Dedicated listener port. **Required** when `enabled`; startup fails without it. |
| `providers.<name>.scm-api.require-validated-head` | boolean | `true` | Refuse a pull/merge request whose head commit has no fogwall push record. Relax where rebase/amend/force-push/web-UI commits are common. |
| `providers.<name>.scm-api.merge-enabled` | boolean | `false` | Allow merging PR/MRs through this provider. Independent of the `MERGE` grant; both are required. |
| Property | Type | Default | Description |
| ------------------------------------------------- | ------- | ------- | --------------------------------------------------------------------------------------------------------------------------------------------------- |
| `scm-api.node-id-cache-ttl` | string | `PT5M` | ISO-8601 duration. See the security note above. |
| `providers.<name>.scm-api.enabled` | boolean | `false` | Whether the SCM API proxy is mounted for this provider. |
| `providers.<name>.scm-api.port` | int | — | Dedicated listener port. **Required** when `enabled`; startup fails without it. |
| `providers.<name>.scm-api.require-validated-head` | boolean | `true` | Refuse a pull/merge request whose head commit fogwall has not approved or forwarded. Relax where rebase/amend/force-push/web-UI commits are common. |
| `providers.<name>.scm-api.merge-enabled` | boolean | `false` | Allow merging PR/MRs through this provider. Independent of the `MERGE` grant; both are required. |

## Token model

Expand Down
43 changes: 43 additions & 0 deletions docs/internals/scm-api-proxy.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -153,6 +153,31 @@ The schema has a separate `input.headRepositoryId` for the head repository, but
namespaced `headRefName` instead. It is worth knowing it exists: reading it as the target would authorize the repository
the contributor already owns.

The owner prefix names an account, not a repository: the fork above is `RBC/coopernetes-test-repo`, not `RBC/test-repo`.
For `require-validated-head`, `GitHubHeadShaResolver` lets GitHub pair the owner with a repository: one
`repository { ref(qualifiedName: "refs/heads/<baseRefName>") { compare(headRef: <headRefName>) { headTarget { oid } } } }`
query on the base repository, with `headRefName` exactly as sent. The provenance check is keyed on the head SHA alone,
so the fork's name is never needed. A `null` compare (GitHub answers `NOT_FOUND`, "Could not resolve head ref"), any
GraphQL error, or a missing `baseRefName` is refused.

An owner can hold several forks of the same base. GitHub then pairs `owner:branch` with one of them, the oldest, and
`compare` and `createPullRequest` agree on it. Observed against `coopernetes/test-repo` with three `RBC` forks, each
branch carrying a distinct commit per fork:

| Branch on | `compare` | `createPullRequest` |
| --------------------------- | -------------------- | ---------------------------------------- |
| oldest fork and newer forks | oldest fork's commit | opened from the oldest fork, same commit |
| newer forks only | `NOT_FOUND` | refused ("Head ref must be a branch") |

Neither searches the owner's other forks, so a branch on a newer fork cannot be proposed through `owner:branch` at all.
This pairing is observed behaviour, not documented API. The resolver depends on the two agreeing, and every mismatch
observed fails closed.

`headRepositoryId` names a fork exactly, so the branch is read from that repository's node instead of through `compare`;
an `owner:` prefix that disagrees with its owner is refused. A `headRefName` with a repository segment
(`owner:repo:branch`) is refused: `compare` ignores the segment and would read the oldest fork's branch instead. `gh`
sends neither: its `--head` takes only `branch` or `owner:branch`.

Subject IDs are safer to cache than repository IDs. A GitHub issue transfer mints a new node ID in the destination and
leaves the old one as a redirect, so `issueId → repo` has no rename staleness. `repositoryId → owner/name` is the
mapping that needs a conservative TTL.
Expand Down Expand Up @@ -427,6 +452,24 @@ POST https://gitea.com/api/v1/repos/coopernetes/test-repo/pulls
The path segment is the upstream — whatever `--repo` names — and the fork appears only in the body as
`head: "<user>:<branch>"`, the same shape GitHub uses. No `target_project_id` handling is needed.

Gitea/Forgejo pair `<user>` with the upstream by fork relationship, not by name. Their `parseCompareInfo` accepts the
user's direct fork of the upstream (`GetForkedRepo(headUser, baseRepo)`), else the upstream's own `parent` when the user
owns it, else the upstream itself; a sibling fork or a fork of a fork is not a valid head. `ForgejoHeadShaResolver`
mirrors that, since the API has no direct-fork lookup:

1. A fork resolved earlier for the same provider, upstream and user, cached by repository ID for 6 hours:
`GET /repositories/{id}`, used only if its `parent` is still the upstream and the user still owns it. Otherwise the
entry is dropped and resolution continues.
2. `GET /repos/{upstream}`; its `parent` when the user owns it.
3. When its `forks_count` is at most 50, `GET /repos/{upstream}/forks?limit=50&page=1`: the user's entry. This is exact.
A page shorter than `forks_count` (a server's lower page cap, forks the caller cannot see) falls through.
4. Otherwise the user's repository of the upstream's name, if its `parent` is the upstream.
5. Otherwise `GET /repos/search?uid=…&exclusive=true&mode=fork`, most recently updated first, at most three pages of 50,
for the one whose `parent` is the upstream.

A fork found by steps 3 to 5 is cached. A cached branch lookup costs two calls; an uncached one on a small upstream
three.

`fj` behaves the same way by construction: `--head` is forwarded verbatim (`prs.rs`, `Some(head) => Some(head)`) and the
repo comes from `-r/--repo` into `repo_create_pull_request(owner, repo, …)`.

Expand Down
Loading
Loading