ci: enable CI for fork PRs via /run-ci+[full-head-sha] comment

[rodrigosf672]

[This should NOT be merged without security review]

Summary

Closes #6628

External contributors opening PRs from forks currently see a wall of CI failures because workflows can't access repo secrets. This PR fixes that with a comment-triggered, per-commit approval gate:

1. A maintainer reviews the fork PR diff and comments /run-ci + full SHA for the commit that matches head.
2. A dispatcher workflow verifies the commenter is a member of @posit-dev/positron-dev and adds the run-ci label
3. The label addition triggers the existing CI pipeline for that specific SHA
4. When the fork author pushes new commits, a revocation workflow automatically strips the label and the CI stop until a maintainer re-approves.

Internal PRs continue to run CI immediately with no change in behavior.

Happy path

Internal PRs run CI immediately, as before. Fork PRs are blocked until a @posit-dev/positron-dev member reviews the diff and approves a specific commit with /run-ci <sha> — the approval is pinned to that exact commit and is revoked automatically on any new push. (Any team member counts as a maintainer for this gate; the two are treated as one and the same.)

flowchart TD
    A[Fork PR opened / new push] --> B{Is it a fork?}
    B -- No --> C[CI runs immediately]
    B -- Yes --> D[CI blocked — no run-ci label]

    D --> E[Maintainer reviews diff]
    E --> F["Commenter posts /run-ci + SHA"]
    F --> G{"Commenter ∈ positron-dev (= maintainer)?"}
    G -- No --> H[Rejected; no action]
    G -- Yes --> I["Bot adds run-ci label + 🚀 reaction"]
    I --> J["labeled event → CI runs for HEAD SHA"]

    J --> K[Fork author pushes new commits]
    K --> L["synchronize event → label auto-removed"]
    L --> M["Bot comments: approval revoked, /run-ci to re-run"]
    M --> D

Loading

Failure scenarios

1. Revoke workflow fails (label persists after push)

This is extremely unlikely, but if the revoke workflow fails and the run-ci label is not auto-removed after a push, CI still won't run. The authorize gate checks the event type, not the label state. Hence, a synchronize event from a fork is rejected regardless of what labels are present. Only a fresh labeled event (triggered by /run-ci by a maintainer) can pass the gate. To recover, a maintainer removes the stale label, reviews the new diff, and comments /run-ci.

flowchart TD
    A[Fork author pushes new commits] --> B["synchronize event fires"]
    B --> C["ci-revoke-fork.yml fails ❌"]
    C --> D["run-ci label remains on PR"]
    D --> E["test-pull-request.yml fires on synchronize"]
    E --> F{"authorize gate checks event action"}
    F -->|"action == 'synchronize' ≠ 'labeled'"| G["Gate BLOCKED; CI does not run ✅"]
    G --> H["Maintainer notices stale label"]
    H --> I["Removes label, reviews diff, comments /run-ci"]

Loading

2. Approve workflow fails (label never added)

If the approve workflow fails (e.g. expired PAT, GitHub API outage), the run-ci label is never added, so no labeled event fires. Without that event, the authorize gate never evaluates for the fork PR, hence CI simply doesn't start. The maintainer sees the failed workflow run in the Actions tab, investigates/resolves the issue, and comments /run-ci again.

flowchart TD
    A["Commenter posts /run-ci"] --> B["ci-approve-fork.yml fires"]
    B --> C{"Team membership check (∈ positron-dev = maintainer)"}
    C -->|"API error / PAT expired"| D["Workflow fails ❌"]
    D --> E["No label added, no labeled event"]
    E --> F["CI never triggers; safe default ✅"]
    F --> G["Maintainer sees failed workflow run"]
    G --> H["Fixes PAT / retries /run-ci"]

Loading

3. Manual label removal (cancel in-flight CI)

If a maintainer removes the run-ci label while CI is running (e.g. they spot something suspicious in the diff mid-run), the unlabeled event fires and starts a new workflow run in the same concurrency group. Because cancel-in-progress: true is set, this new run cancels the in-flight CI immediately. The new run then hits the authorize gate, which rejects unlabeled events unconditionally, so it exits without doing anything. The PR returns to a blocked state.

flowchart TD
    A["CI running for fork PR"] --> B["Maintainer removes run-ci label"]
    B --> C["unlabeled event → new workflow run starts"]
    C --> D["Same concurrency group → cancel-in-progress: true"]
    D --> E["In-flight CI cancelled ✅"]
    E --> F["New run hits authorize gate"]
    F --> G["action == 'unlabeled' → gate fails immediately"]
    G --> H["PR returns to blocked state"]

Loading

4. New commit pushed after approval (per-commit revocation + SHA pin)

After a maintainer approves commit A (label added, CI triggered for A), the fork author pushes commit B. The synchronize event fires ci-revoke-fork.yml, which strips the run-ci label and comments that approval is revoked. Independently, the CI that was triggered by the labeled event is pinned to commit A — the labeled event payload carries the head SHA at label time — so it never tests the unreviewed B even if it is still running. Running B requires a fresh /run-ci <B-sha> after re-review.

flowchart TD
    A["Maintainer approves commit A → run-ci label added"] --> B["CI triggered for commit A (pinned by labeled event payload)"]
    B --> C["Fork author pushes commit B"]
    C --> D["synchronize event → ci-revoke-fork.yml"]
    D --> E["run-ci label removed + revoke comment posted"]
    E --> F["Unreviewed commit B never runs CI ✅"]
    F --> G["Maintainer re-reviews, comments /run-ci B-sha"]

Loading

5. Maintainer approves a stale commit (push before approval — TOCTOU)

A maintainer reviews commit A, but the fork author pushes commit B before the maintainer comments. Still looking at the old diff, the maintainer comments /run-ci <A-sha>. ci-approve-fork.yml resolves the PR's current head (B) and compares it to the requested SHA (A); because they differ, it rejects the approval, posts a comment showing the new head, and adds no label. CI never runs the unreviewed B. The maintainer must re-review and comment /run-ci <B-sha>. (A bare /run-ci, or a short/partial SHA, is rejected the same way — only an exact full 40-char head SHA approves.)

flowchart TD
    A["Maintainer reviews commit A"] --> B["Fork author pushes commit B (new head)"]
    B --> C["Maintainer comments /run-ci A-sha"]
    C --> D["ci-approve-fork resolves current head = B"]
    D --> E{"requested A == head B?"}
    E -->|"No — stale / mismatched SHA"| F["Approval rejected; no label added ✅"]
    F --> G["Bot comments new head; maintainer re-reviews"]
    G --> H["Maintainer comments /run-ci B-sha"]

Loading

Security model

Threat	Mitigation
Fork injects malicious workflow steps	pull_request_target resolves YAML from main, not the fork
Fork author self-approves	/run-ci verified against posit-dev/positron-dev team membership
Approved code mutates after push	Label auto-revoked on every synchronize — re-approval required per commit
Author races a push between review and approval (TOCTOU)	/run-ci requires the full 40-char SHA the maintainer reviewed; approval is rejected unless it exactly matches the current head
Short-SHA prefix collision on the pin	Full 40-char SHA required; short SHAs (a brute-forceable 28-bit prefix at 7 chars) are rejected
Accidental approval from prose / comment injection	Command must be on its own line, so a mention like "don't /run-ci yet" is ignored; the comment body is read via an env var, never interpolated into a shell script
Secrets leak to unapproved code	authorize gate blocks all downstream jobs; secrets never flow without label
Write-scoped pull_request_target token reused by fork code	persist-credentials: false on every checkout that pulls fork code, so the token is not left in .git/config
Race between revoke and CI trigger	authorize only accepts fork PRs via the labeled event path, never synchronize
Revoke workflow fails	Gate is event-type based, not label-state based; stale label is inert

Prior discussion (from the issue)

• Julia proposed the stopgap and linked the DVC blog post on pull_request_target + environment gates
• Austin suggested pull_request_target with a protected environment
• Sharon noted GitHub's approval setting doesn't apply to pull_request_target events
• Comment-based gate (with full sha needed) chosen over label-based or environment-based for: team restriction, per-commit revocation, and simpler UX.

Test plan

This test plan requires PR to be merged, which can only happen after security approval. Do NOT merge without security approval.

• Open a PR from a fork → verify CI does NOT run
• Non-team-member comments /run-ci → verify label is NOT added
• Team member comments /run-ci → verify label added, 🚀 reaction, CI triggers
• Push a new commit to the fork PR → verify label removed, comment posted, CI does NOT run
• Team member comments /run-ci again → verify CI re-triggers for new SHA
• Manually remove run-ci label while CI is running → verify CI cancelled
• Open an internal PR → verify CI runs immediately on open and on push (no label needed)

Observation: upon merged, these tests above will be conducted in three different GitHub accounts (part of positron-dev, part of posit-dev but NOT part of positron-dev, and external, NOT part of posit-dev).

Potential security questions

Please find below some security questions one could ask and the respective answers based on the approach planned to be adopted here.

1. Why pull_request_target instead of GitHub's native "require approval for outside collaborators"?
The native setting runs fork PRs as pull_request, which has no access to repo secrets. Our e2e/build jobs need them (interpreters, licenses, OIDC), so we need pull_request_target plus a manual gate.

2. Can a fork change the gate itself (logic, triggers, permissions)?
No. pull_request_target resolves all workflow YAML from the base branch (main), so edits to .github/workflows/** in the fork are ignored for the gating run.

3. Can a fork author approve their own PR?
No. /run-ci is honored only if the commenter is an active member of posit-dev/positron-dev, verified server-side. The check uses comment.user.login (GitHub-controlled), so it can't be spoofed via the comment body.

4. Is the /run-ci trigger injectable?
No. The command must be on its own line (prose mentions are ignored), and the attacker-controlled comment body is read via an env var, never interpolated into the shell script — so there's no command-injection surface.

5. Can the code that runs differ from the code that was reviewed (TOCTOU)?
No. /run-ci requires the exact full 40-char head SHA; a mismatch (push-after-review) is rejected. The labeled event payload pins head.sha at label time, so the triggered run tests exactly the approved commit, and any later push auto-revokes the label.

6. Why require the full SHA instead of a short one?
A 7-char short SHA is only a 28-bit prefix and is brute-forceable via commit grinding; the full 40-char SHA (160 bits) is not. Short/partial SHAs are rejected.

[github-actions]

E2E Tests 🚀
This PR will run tests tagged with: @:critical

^{readme  valid tags}

[Copilot]

Pull request overview

This PR introduces a maintainer-mediated approval gate that lets CI run on fork-based PRs only after a trusted reviewer comments /run-ci, using pull_request_target + a run-ci label + automatic revocation on new commits.

Changes:

• Switches PR CI workflow to pull_request_target and adds an authorize gate that only permits fork PR CI runs via a fresh labeled: run-ci event.
• Adds a comment-triggered approval workflow that verifies team membership and applies the run-ci label (plus 🚀 reaction).
• Adds a revocation workflow that removes run-ci on new pushes to fork PRs and posts a notification comment.

Review Findings (blocking)

• .github/workflows/test-pull-request.yml: authorize uses - run: true, but run: must be a string. This will fail workflow validation. Replace with a no-op command like run: echo "authorized" (or quote it as run: "true").
• Reusable workflows commit checkout may fail under pull_request_target: In the reusable workflows you added git checkout ${{ inputs.commit }} after actions/checkout, but under pull_request_target the default ref is the base branch, so that commit may not be present locally. Prefer checking out the requested SHA directly in actions/checkout (e.g. with: ref: ${{ inputs.commit }}) or explicitly fetching the SHA before git checkout.
• .github/workflows/ci-revoke-fork.yml: The workflow posts a PR comment via gh pr comment, but only requests pull-requests: write. Creating PR comments generally requires issues: write (PR comments are issue comments), so the “Post comment” step is likely to fail without adding permissions: issues: write.

Reviewed changes

Copilot reviewed 9 out of 9 changed files in this pull request and generated no comments.

Show a summary per file

File	Description
.github/workflows/test-pull-request.yml	Moves CI to pull_request_target, adds authorize gate, and propagates head SHA to downstream reusable workflows.
.github/workflows/test-e2e-workbench-ubuntu.yml	Adds optional commit input and attempts to checkout a specific SHA for workbench E2E.
.github/workflows/test-e2e-ubuntu.yml	Adds optional commit input and attempts to checkout a specific SHA for Ubuntu E2E.
.github/workflows/test-e2e-pyrefly.yml	Adds optional commit input and attempts to checkout a specific SHA for Pyrefly E2E.
.github/workflows/test-e2e-jupyter-ubuntu.yml	Adds optional commit input and attempts to checkout a specific SHA for Jupyter E2E.
.github/workflows/ci-revoke-fork.yml	New workflow to revoke run-ci label on fork PR pushes and notify maintainers.
.github/workflows/ci-approve-fork.yml	New workflow to authorize /run-ci comments by team members and apply the run-ci label.
.github/workflows/build-workbench-linux.yml	Adds optional commit input and attempts to checkout a specific SHA for workbench builds.
.github/workflows/build-jupyter-linux.yml	Adds optional commit input and attempts to checkout a specific SHA for server builds.

[rodrigosf672]

Pull request overview

This PR introduces a maintainer-mediated approval gate that lets CI run on fork-based PRs only after a trusted reviewer comments /run-ci, using pull_request_target + a run-ci label + automatic revocation on new commits.

Changes:

• Switches PR CI workflow to pull_request_target and adds an authorize gate that only permits fork PR CI runs via a fresh labeled: run-ci event.
• Adds a comment-triggered approval workflow that verifies team membership and applies the run-ci label (plus 🚀 reaction).
• Adds a revocation workflow that removes run-ci on new pushes to fork PRs and posts a notification comment.

Review Findings (blocking)

• .github/workflows/test-pull-request.yml: authorize uses - run: true, but run: must be a string. This will fail workflow validation. Replace with a no-op command like run: echo "authorized" (or quote it as run: "true").
• Reusable workflows commit checkout may fail under pull_request_target: In the reusable workflows you added git checkout ${{ inputs.commit }} after actions/checkout, but under pull_request_target the default ref is the base branch, so that commit may not be present locally. Prefer checking out the requested SHA directly in actions/checkout (e.g. with: ref: ${{ inputs.commit }}) or explicitly fetching the SHA before git checkout.
• .github/workflows/ci-revoke-fork.yml: The workflow posts a PR comment via gh pr comment, but only requests pull-requests: write. Creating PR comments generally requires issues: write (PR comments are issue comments), so the “Post comment” step is likely to fail without adding permissions: issues: write.

Reviewed changes

Copilot reviewed 9 out of 9 changed files in this pull request and generated no comments.

Show a summary per file

Finding 1 (run: true fails validation)

This is not a real concern. Although YAML parses bare true as a boolean, GitHub Actions coerces it to the string "true", which is a valid shell command that exits 0. It should work, and I'll double check it after merge.

Finding 3 (missing issues: write for gh pr comment)

This is not a real concern either. PR comments are covered by pull-requests: write when the target is a pull request. issues: write is only needed for issue-specific endpoints like the reactions API. Again, I'll also double check this doesn't fail once merged.

Finding 2 (fork commit unreachable after checkout).

This is actually a concern/edge case that I missed, and it has been addressed in the latest commit.

[rodrigosf672]

Note: This PR is still active and under review/work; it is not stale.

[juliasilge]

We're going to close this out now after the security review; see #6628 (comment) for more details.

We'll keep an eye on options we might have for improving this in the future.