diff --git a/.github/workflows/CLAUDE.md b/.github/workflows/CLAUDE.md index a7a27a9..04bdfa5 100644 --- a/.github/workflows/CLAUDE.md +++ b/.github/workflows/CLAUDE.md @@ -8,6 +8,32 @@ for all test steps (PostgreSQL matrix, checkouts, git config, pgtap install, etc.). Called by both `ci.yml` here and by `pgxntool/ci.yml` for the commit-with-no-tests path. +## Branch pairing and account matching + +A change to either repo should normally ship with a **matching branch (same +name) on the same account** in the other repo. CI resolves the sibling using +both the branch name **and** the PR head's account: + +- **pgxntool PRs** must have a paired pgxntool-test PR (matched by branch name + + head account) — enforced by `ci.yml`'s `check-test-pr`. The only escape is the + `commit-with-no-tests` label, which a maintainer applies (guarded by + `protect-label.yml`); then pgxntool is tested against pgxntool-test master. +- **pgxntool-test PRs** may stand alone (test-only changes are a normal pattern). + `resolve` looks for a paired pgxntool branch on the PR head's account; if none + exists, it falls back to pgxntool master. + +### Never cross-account (security) + +The sibling branch is matched **only on the PR head's own account**. We never +use a same-named branch from any other account — including Postgres-Extensions — +to pair with a fork's branch. The **sole** cross-account exception is `master`: +when there's no paired branch, the other repo is taken from +**`Postgres-Extensions/master` only**, never the fork's master (which may be +stale or deliberately modified). This is why `run-tests.yml` takes explicit +`pgxntool-owner` / `pgxntool-test-owner` inputs: the caller resolves the correct +account for each repo (head account for the paired side, `Postgres-Extensions` +for a master fallback) rather than assuming everything lives under one owner. + ## Cross-repo reusable workflow — tradeoffs and constraints `run-tests.yml` is referenced from **pgxntool** as: diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 5cbacf7..28ba601 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -21,15 +21,22 @@ jobs: runs-on: ubuntu-latest outputs: pgxntool-ref: ${{ steps.pgxntool-ref.outputs.ref }} + pgxntool-owner: ${{ steps.pgxntool-ref.outputs.owner }} steps: - name: Resolve pgxntool branch id: pgxntool-ref - # PR-controlled values (head_ref) are passed via env, never interpolated - # directly into the script, to avoid shell injection. + # PR-controlled values (head_ref, head owner) are passed via env, never + # interpolated directly into the script, to avoid shell injection. env: BRANCH: ${{ github.head_ref }} - REPO_OWNER: ${{ github.repository_owner }} + # The account this PR's branch lives on: the contributor's fork for + # fork PRs, or Postgres-Extensions for a maintainer branch. + HEAD_OWNER: ${{ github.event.pull_request.head.repo.owner.login }} + # This PR's own pgxntool-test is checked out from the base repo at the + # merge SHA — used only to print an accurate BRANCHES line below. + TEST_OWNER: ${{ github.repository_owner }} + TEST_SHA: ${{ github.sha }} run: | # Reject anything that isn't a valid git branch name before using it. git check-ref-format --branch "$BRANCH" >/dev/null 2>&1 || { @@ -37,20 +44,30 @@ jobs: exit 1 } - # Check whether pgxntool has a branch with the same name as this PR. - # If it does, use it — changes in this test PR may depend on changes - # in a paired pgxntool branch. If not, fall back to master. + # Look for a paired pgxntool branch of the SAME name on the SAME + # account (HEAD_OWNER). A pgxntool change should ship with a matching + # pgxntool-test branch and vice versa; if this test PR has one, use it. + # + # SECURITY — never cross-account: we only ever match a branch on + # HEAD_OWNER. If there's no paired branch (e.g. a pgxntool-test-only + # PR), we fall back to pgxntool master from Postgres-Extensions ONLY, + # never the fork's master (which may be stale or modified) and never a + # same-named branch on any other account. master is the sole ref we + # take cross-account. # # --exit-code makes git ls-remote return non-zero when the ref is # absent, so we can branch on the result cleanly without parsing output. if git ls-remote --exit-code --heads \ - "https://github.com/${REPO_OWNER}/pgxntool.git" \ + "https://github.com/${HEAD_OWNER}/pgxntool.git" \ "refs/heads/${BRANCH}" > /dev/null 2>&1; then echo "ref=${BRANCH}" >> "$GITHUB_OUTPUT" - echo "=== BRANCHES: pgxntool-test=${BRANCH} pgxntool=${BRANCH} ===" + echo "owner=${HEAD_OWNER}" >> "$GITHUB_OUTPUT" + # Same shape/order as run-tests.yml: pgxntool first, owner/ref for each. + echo "=== BRANCHES: pgxntool=${HEAD_OWNER}/${BRANCH} pgxntool-test=${TEST_OWNER}/${TEST_SHA} ===" else echo "ref=master" >> "$GITHUB_OUTPUT" - echo "=== BRANCHES: pgxntool-test=${BRANCH} pgxntool=master (branch not found in pgxntool) ===" + echo "owner=Postgres-Extensions" >> "$GITHUB_OUTPUT" + echo "=== BRANCHES: pgxntool=Postgres-Extensions/master pgxntool-test=${TEST_OWNER}/${TEST_SHA} (no paired pgxntool branch on ${HEAD_OWNER}) ===" fi test: @@ -60,5 +77,11 @@ jobs: # the cross-repo reusable workflow tradeoffs and merge order constraints. uses: ./.github/workflows/run-tests.yml with: + # pgxntool: the paired branch on this PR's account, or Postgres-Extensions + # master when there's no pairing (resolved above). + pgxntool-owner: ${{ needs.resolve.outputs.pgxntool-owner }} pgxntool-branch: ${{ needs.resolve.outputs.pgxntool-ref }} + # pgxntool-test: this PR's own merged code, which lives on the base repo + # (github.repository_owner) at the merge SHA. + pgxntool-test-owner: ${{ github.repository_owner }} pgxntool-test-ref: ${{ github.sha }} diff --git a/.github/workflows/run-tests.yml b/.github/workflows/run-tests.yml index 19c3e6c..04c101f 100644 --- a/.github/workflows/run-tests.yml +++ b/.github/workflows/run-tests.yml @@ -14,6 +14,16 @@ on: description: 'The pgxntool branch to check out and use as the local branch name' type: string required: true + pgxntool-owner: + description: 'GitHub account that owns the pgxntool repo to check out. Defaults to this repo owner. Set to the PR head account so a fork PR pulls its paired branch from the same fork.' + type: string + required: false + default: '' + pgxntool-test-owner: + description: 'GitHub account that owns the pgxntool-test repo to check out. Defaults to this repo owner.' + type: string + required: false + default: '' jobs: test: @@ -23,7 +33,9 @@ jobs: # Caller-controlled inputs are exposed as env vars and referenced as $VARs # in run: blocks below, never interpolated directly into the shell. env: + PGXNTOOL_OWNER: ${{ inputs.pgxntool-owner || github.repository_owner }} PGXNTOOL_BRANCH: ${{ inputs.pgxntool-branch }} + PGXNTOOL_TEST_OWNER: ${{ inputs.pgxntool-test-owner || github.repository_owner }} PGXNTOOL_TEST_REF: ${{ inputs.pgxntool-test-ref }} strategy: matrix: @@ -39,13 +51,13 @@ jobs: # are easy to correlate to the right code. This is especially useful # when debugging cross-repo issues where the wrong branch was picked. # The === BRANCHES: === line is parsed by the CI monitor script. - echo "=== BRANCHES: pgxntool=${PGXNTOOL_BRANCH} pgxntool-test=${PGXNTOOL_TEST_REF} ===" + echo "=== BRANCHES: pgxntool=${PGXNTOOL_OWNER}/${PGXNTOOL_BRANCH} pgxntool-test=${PGXNTOOL_TEST_OWNER}/${PGXNTOOL_TEST_REF} ===" - name: Check out pgxntool-test uses: actions/checkout@v4 with: persist-credentials: false - repository: ${{ github.repository_owner }}/pgxntool-test + repository: ${{ inputs.pgxntool-test-owner || github.repository_owner }}/pgxntool-test ref: ${{ inputs.pgxntool-test-ref }} path: pgxntool-test # REQUIRED: pgxntool-test includes BATS as a git submodule at @@ -59,7 +71,7 @@ jobs: uses: actions/checkout@v4 with: persist-credentials: false - repository: ${{ github.repository_owner }}/pgxntool + repository: ${{ inputs.pgxntool-owner || github.repository_owner }}/pgxntool ref: ${{ inputs.pgxntool-branch }} path: pgxntool submodules: false # pgxntool has no submodules diff --git a/README.md b/README.md index 94e6857..309274d 100644 --- a/README.md +++ b/README.md @@ -101,31 +101,33 @@ Each test file automatically runs its prerequisites if needed, so they can be ru `pgxntool` is the framework itself; `pgxntool-test` is the test harness for it. They are kept separate because `pgxntool` is embedded into extension projects via `git subtree` — you don't want test infrastructure polluting those projects. The CI is designed to coordinate changes across both repos. -A pgxntool change **should almost always have a corresponding test change** in pgxntool-test. Commits touching only pgxntool (with no test changes) should be rare. It is also normal and common to have commits that only touch pgxntool-test (e.g., improving test coverage) with no corresponding pgxntool change — as long as tests pass, that's fine. +**The norm is that a change is paired across both repos** — it comes with a matching branch (same name, same account) in the other repo. This is effectively required for `pgxntool` changes: a pgxntool change should almost always have a corresponding pgxntool-test change, and CI blocks an unpaired pgxntool PR unless a maintainer applies the `commit-with-no-tests` label (see below). A pgxntool-test-only change (with no pgxntool counterpart) is the occasional exception — for example, improving test coverage or infrastructure — and CI runs it against `pgxntool` master. Neither repo should be changed in isolation as a matter of routine. ### PR conventions -When your change requires modifications to both repos, open PRs in **both repos using the same branch name**. For example, if your feature branch is named `feature/add-pgtle-support`, create that branch in both `pgxntool` and `pgxntool-test`. **Branch names must match exactly** — the CI uses the branch name to find the paired PR. +A change normally touches both repos, so open a PR in **each repo from a branch with the same name, on the same GitHub account** (your fork). For example, if your feature branch is named `feature/add-pgtle-support`, push it to both your `pgxntool` and `pgxntool-test` forks and open a PR from each. **The branch name *and* the account must match** — CI pairs the two PRs using both. (The exception is a pgxntool-test-only change, which needs no pgxntool PR; see above.) ### How CI works **When you open a PR in `pgxntool-test`:** -CI checks whether `pgxntool` has a branch with the same name. If it does, tests run against that pgxntool branch. If not, tests run against `pgxntool/master`. Results appear directly on your pgxntool-test PR. +CI looks for a `pgxntool` branch with the same name **on your account** (the PR's head fork). If it exists, tests run against that branch; if not, they run against `pgxntool` master **from Postgres-Extensions**. Results appear directly on your pgxntool-test PR. **When you open a PR in `pgxntool`:** -CI waits for the paired pgxntool-test PR's CI to complete (polling for up to 20 minutes), then checks whether it passed. pgxntool CI does not run tests itself — it relies entirely on the pgxntool-test CI results. +CI waits for the paired pgxntool-test PR (matched by branch name **and** account) to complete (polling for up to 20 minutes), then checks whether it passed. pgxntool CI does not run tests itself — it relies entirely on the pgxntool-test CI results. - **If a paired test PR is found and its CI passes**: pgxntool CI passes. There is no test duplication. - **If no paired test PR is found**: pgxntool CI fails (see below). +> **Fork contributors — security note:** CI only ever pairs branches within the **same account**. It will never match your fork's branch to a same-named branch on a different account (including Postgres-Extensions). When there is no paired branch, the *other* repo is always taken from **`Postgres-Extensions/master`** — never a fork's `master` — so a stale or modified fork `master` can't influence the run. `master` is the only ref ever taken cross-account. + ### What to do when pgxntool CI fails with "No paired test PR found" -This failure means CI couldn't find an open PR in pgxntool-test with a matching branch name. The fix is almost always: +This failure means CI couldn't find an open pgxntool-test PR **from your account** with a matching branch name. The fix is almost always: -1. **Open a PR in pgxntool-test** from a branch with the **same name** as your pgxntool branch. +1. **Open a PR in pgxntool-test** from a branch with the **same name, on the same account (your fork)** as your pgxntool branch. 2. Re-run the failing CI check on your pgxntool PR (or push a new commit to trigger it). -**Branch names must match exactly.** If your pgxntool branch is `fix/parse-bug`, your pgxntool-test branch must also be `fix/parse-bug`. +**The branch name and account must match exactly.** If your pgxntool branch is `fix/parse-bug` on `your-account`, your pgxntool-test branch must also be `fix/parse-bug` on `your-account`. ### The `commit-with-no-tests` label @@ -143,7 +145,7 @@ To request the label: ### Branch protection The `check-test-pr` status check on pgxntool is a required check for merging to `master`. It only passes when either: -- A corresponding pgxntool-test PR exists (with a matching branch name) and its tests are **passing**, or +- A corresponding pgxntool-test PR exists (matching branch name **and** account) and its tests are **passing**, or - A maintainer has applied the `commit-with-no-tests` label. This ensures pgxntool changes cannot be merged without passing test coverage.