fix(panel-workflow): use pull_request_target so labelled fork PRs run#837
Merged
danielmeppiel merged 1 commit intomainfrom Apr 22, 2026
Merged
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
Follow-up to #836 to prevent label-triggered PR Review Panel runs on fork PRs from failing due to missing secrets, by making fork PRs skip cleanly and pushing maintainers to the workflow_dispatch path for forks.
Changes:
- Removed
forks: ["*"]from thepull_requesttrigger in the gh-aw source workflow and updated the header rationale. - Updated the compiled lock workflow to gate
pull_requestruns to same-repo PRs viagithub.event.pull_request.head.repo.id == github.repository_id.
Show a summary per file
| File | Description |
|---|---|
| .github/workflows/pr-review-panel.md | Drops the forks trigger attribute and updates the frontmatter commentary/rationale for fork behavior. |
| .github/workflows/pr-review-panel.lock.yml | Regenerates the compiled workflow, adding same-repo gating to avoid secret validation on fork PR label events. |
Copilot's findings
- Files reviewed: 2/2 changed files
- Comments generated: 1
Comment on lines
14
to
19
| # 2. workflow_dispatch: manual trigger taking a PR number. This is the | ||
| # only path that works for fork PRs on GitHub.com and GHES, because | ||
| # REQUIRED path for fork PRs on GitHub.com and GHES, because | ||
| # `pull_request` from forks does NOT pass repository secrets | ||
| # (COPILOT_GITHUB_TOKEN etc.), and gh-aw blocks `pull_request_target` | ||
| # on public repos. workflow_dispatch always runs in the base/trusted | ||
| # context with full secrets, regardless of where the PR head lives. |
There was a problem hiding this comment.
The header comment says workflow_dispatch is the REQUIRED path for fork PRs on GitHub.com and GHES, and that pull_request from forks does not pass repository secrets. On GitHub.com public repos this is true, but on GHES/private repos it can be configurable. Consider scoping the statement to this repo/public GitHub.com (or rewording to avoid implying it's universally true on GHES).
See below for a potential fix:
# 2. workflow_dispatch: manual trigger taking a PR number. In this repo,
# this is the maintainer-approved path for fork PRs because this
# workflow expects repository secrets (COPILOT_GITHUB_TOKEN etc.),
# we do not run that secret-bearing path from fork-originated
# `pull_request` events, and gh-aw blocks `pull_request_target`
# on public repos. Maintainers use workflow_dispatch to run the
# review from this repo's trusted base context for the current setup.
PR #836 only dropped synchronize and added workflow_dispatch; the pull_request trigger still skips fork PRs because GitHub does not pass secrets to fork-PR workflows. Result: labelling a fork PR (e.g. #815) silently failed at the secret validation step. Switch to pull_request_target, which runs in the base repo context with full secrets regardless of where the PR head lives. Combined with roles: [admin, maintainer, write] (auto-injected by gh-aw), the trust gate becomes 'only repo maintainers can trigger' -- which matches the existing model that applying the panel-review label already requires write access. Why pull_request_target is safe for this workflow despite the well-known pwn-request pattern: - permissions are read-only (no write to contents / actions) - no actions/checkout of PR head; only gh pr view / gh pr diff (returns inert text) - imports pinned to microsoft/apm#main (panel skill + persona definitions are trusted, not from the PR) - single write surface is safe-outputs.add-comment (max 1) gh-aw does not expose names: on pull_request_target, so label-name filtering moves into the prompt body (Step 0) -- a one-line bash guard that exits cleanly when any non-panel-review label fires. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
danielmeppiel
force-pushed
the
fix/panel-drop-forks-attr-836-followup
branch
from
a3d4c85 to
9fec13c
Compare
danielmeppiel
changed the title
danielmeppiel
added a commit
that referenced
this pull request
Apr 22, 2026
- Bump version to 0.9.1 in pyproject.toml and uv.lock - Move CHANGELOG [Unreleased] entries into [0.9.1] - 2026-04-22 - Audit pass: 1 PR = 1 entry, no bloat - Consolidate the seven scattered entries from #832 into a single Added line that names the closed issues (#827/#829/#831/#834) and keeps the migration warning inline - Combine the three fork-PR fixes (#826, #836, #837) into one Fixed entry per the multi-PR convention - Drop the doc-only consolidation commits from the log Highlights of 0.9.1: - Install-time enforcement of org apm-policy.yml (#832) - pr-review-panel automation now usable from fork PRs (#824, #826, #836, #837) - Docs site publish gated on stable releases only (#822) - Repository dogfooding via .apm/ as primitive source of truth (#823) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
danielmeppiel
added a commit
that referenced
this pull request
Apr 22, 2026
- Bump version to 0.9.1 in pyproject.toml and uv.lock - Move CHANGELOG [Unreleased] entries into [0.9.1] - 2026-04-22 - Audit pass: 1 PR = 1 entry, no bloat - Consolidate the seven scattered entries from #832 into a single Added line that names the closed issues (#827/#829/#831/#834) and keeps the migration warning inline - Combine the three fork-PR fixes (#826, #836, #837) into one Fixed entry per the multi-PR convention - Drop the doc-only consolidation commits from the log Highlights of 0.9.1: - Install-time enforcement of org apm-policy.yml (#832) - pr-review-panel automation now usable from fork PRs (#824, #826, #836, #837) - Docs site publish gated on stable releases only (#822) - Repository dogfooding via .apm/ as primitive source of truth (#823) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
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
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.
What
Switch the panel-review workflow trigger from
pull_requesttopull_request_targetso labelled fork PRs (like #815) actually run instead of silently failing at secret validation.Why PR #836 was incomplete
#836 dropped
synchronizeand addedworkflow_dispatch, but keptpull_request:as the label-driven trigger. Forks PRs do not receive secrets underpull_request, so labelling them produced a failed run with emptyCOPILOT_GITHUB_TOKEN. See run https://github.com/microsoft/apm/actions/runs/24769454293.Trust model
Maintainer applies
panel-reviewlabel -> workflow runs in base repo context with full secrets, regardless of fork origin.pull_request_target: types: [labeled]roles: [admin, maintainer, write]-- only repo maintainers can fire itnames:onpull_request_target)actions/checkoutof PR head; onlygh pr view/gh pr diff(inert text)microsoft/apm#main(panel skill + persona files always come from main, not the PR)contents/pull-requests/issues: readonlysafe-outputs.add-comment: max 1(single comment via gh-aw's permission-isolated downstream job)This is why the standard "pwn-request" exploit does NOT apply: there is no path from PR-controlled code to a write operation. The only attacker-controlled input is text we feed to the agent as inert prompt context.
Why label-as-trust-gate is sufficient
The
panel-reviewlabel can only be applied by users with repo write access. Combined with gh-aw'srolescheck, the workflow only ever runs when a maintainer explicitly opts in. This is the same model GitHub recommends for ChatOps viaissue_comment(see actions/runner#163 discussion). Gated environments with manual approval would be a strictly worse UX for the same threat model.How to verify
After merge, label PR #815 (or any other fork PR) with
panel-review. Workflow should fire and post a verdict comment.Files
.github/workflows/pr-review-panel.md-- trigger + Step 0 guard + updated rationale.github/workflows/pr-review-panel.lock.yml-- regenerated bygh aw compileFollow-up to #836.