diff --git a/.github/instructions/gh-aw-workflows.instructions.md b/.github/instructions/gh-aw-workflows.instructions.md index 30d02675e639..c8cd70ecc3be 100644 --- a/.github/instructions/gh-aw-workflows.instructions.md +++ b/.github/instructions/gh-aw-workflows.instructions.md @@ -44,7 +44,9 @@ The prompt is built in the **activation job** via `{{#runtime-import .github/wor ### Fork PR Activation Gate -`gh aw compile` automatically injects a fork guard into the activation job's `if:` condition: `head.repo.id == repository_id`. This blocks fork PRs on `pull_request` events. This is **platform behavior** — do not add it manually. +By default, `gh aw compile` automatically injects a fork guard into the activation job's `if:` condition: `head.repo.id == repository_id`. This blocks fork PRs on `pull_request` events. + +To **allow fork PRs**, add `forks: ["*"]` to the `pull_request` trigger in the `.md` frontmatter. The compiler removes the auto-injected guard from the compiled `if:` conditions. This is safe when the workflow uses the `Checkout-GhAwPr.ps1` pattern (checkout + trusted-infra restore) and the agent is sandboxed. ## Fork PR Handling @@ -58,16 +60,17 @@ Reference: https://securitylab.github.com/resources/github-actions-preventing-pw | Trigger | `checkout_pr_branch.cjs` runs? | Fork handling | |---------|-------------------------------|---------------| -| `pull_request` | ✅ Yes | Blocked by auto-generated activation gate | +| `pull_request` (default) | ✅ Yes | Blocked by auto-generated activation gate unless `forks: ["*"]` is set | +| `pull_request` + `forks: ["*"]` | ✅ Yes | ✅ Works — user steps restore trusted infra before agent runs | | `workflow_dispatch` | ❌ Skipped | ✅ Works — user steps handle checkout and restore is final | | `issue_comment` (same-repo) | ✅ Yes | ✅ Works — files already on PR branch | -| `issue_comment` (fork) | N/A | ❌ Blocked by fail-closed fork guard in `Checkout-GhAwPr.ps1` | +| `issue_comment` (fork) | ✅ Yes | ⚠️ Works — `checkout_pr_branch.cjs` re-checks out fork branch after user steps, potentially overwriting restored infra. Acceptable because agent is sandboxed (no credentials, max 1 comment via safe-outputs). Pre-flight check catches missing `SKILL.md` if fork isn't rebased. | ### The `issue_comment` + Fork Problem -For `/slash-command` triggers on fork PRs, `checkout_pr_branch.cjs` runs AFTER all user steps and re-checks out the fork branch. This overwrites any files restored by user steps (e.g., `.github/skills/`). There is no way to run user steps after platform steps. A fork could include a crafted `SKILL.md` that alters the agent's evaluation behavior. +For `/slash-command` triggers on fork PRs, `checkout_pr_branch.cjs` runs AFTER all user steps and re-checks out the fork branch. This overwrites any files restored by user steps (e.g., `.github/skills/`). A fork could include a crafted `SKILL.md` that alters the agent's evaluation behavior. -**Current approach (fail-closed fork guard):** `Checkout-GhAwPr.ps1` checks `isCrossRepository` via `gh pr view` for `issue_comment` triggers. If the PR is from a fork or the API call fails, the script exits with code 1. Fork PRs should use `workflow_dispatch` instead, where `checkout_pr_branch.cjs` is skipped and the user step restore is the final workspace state. +**Accepted residual risk:** The agent runs in a sandboxed container with all credentials scrubbed. The worst outcome is a manipulated evaluation comment (`safe-outputs: add-comment: max: 1`). The agent has no ability to push code, access secrets, or exfiltrate data. The pre-flight check in the agent prompt catches the case where `SKILL.md` is missing entirely (fork not rebased on `main`). **Upstream issue:** [github/gh-aw#18481](https://github.com/github/gh-aw/issues/18481) — "Using gh-aw in forks of repositories" @@ -86,16 +89,16 @@ steps: The script: 1. Captures the base branch SHA before checkout -2. **Fork guard** (`issue_comment` only): checks `isCrossRepository` — exits 1 if fork or API failure -3. Checks out the PR branch via `gh pr checkout` -4. Deletes `.github/skills/` and `.github/instructions/` (prevents fork-added files) -5. Restores them from the base branch SHA (best-effort, non-fatal) +2. Checks out the PR branch via `gh pr checkout` +3. Deletes `.github/skills/` and `.github/instructions/` (prevents fork-added files) +4. Restores them from the base branch SHA (best-effort, non-fatal) **Behavior by trigger:** -- **`workflow_dispatch`**: Fork guard skipped. Platform checkout is skipped, so the restore IS the final workspace state (trusted files from base branch) -- **`issue_comment`** (same-repo): Fork guard passes. Platform re-checks out PR branch — files already match, effectively a no-op -- **`issue_comment`** (fork): Fork guard rejects — exits 1 with actionable notice to use `workflow_dispatch` -- **`pull_request`** (same-repo): Fork guard skipped. Files already exist, restore is a no-op +- **`workflow_dispatch`**: Platform checkout is skipped, so the restore IS the final workspace state (trusted files from base branch) +- **`pull_request`** (same-repo): User step restores trusted infra. `checkout_pr_branch.cjs` runs after and re-checks out PR branch — for same-repo PRs, skill files typically match main unless the PR modified them. +- **`pull_request`** (fork with `forks: ["*"]`): Same as above, but fork's skill files may differ. Same residual risk as `issue_comment` fork case — agent is sandboxed, pre-flight catches missing `SKILL.md`. +- **`issue_comment`** (same-repo): Platform re-checks out PR branch — files already match, effectively a no-op +- **`issue_comment`** (fork): Platform re-checks out fork branch after us, overwriting restored files. Agent is sandboxed; pre-flight in the prompt catches missing `SKILL.md` ### Anti-Patterns @@ -197,7 +200,7 @@ Manual triggers (`workflow_dispatch`, `issue_comment`) should bypass the gate. N | What | Behavior | Workaround | |------|----------|------------| -| User steps always before platform steps | Cannot run user code after `checkout_pr_branch.cjs` | Use `workflow_dispatch` for fork PRs; see [gh-aw#18481](https://github.com/github/gh-aw/issues/18481) | +| User steps always before platform steps | Cannot run user code after `checkout_pr_branch.cjs` | For `issue_comment` fork PRs, accept sandboxed residual risk; see [gh-aw#18481](https://github.com/github/gh-aw/issues/18481) | | `--allow-all-tools` in lock.yml | Emitted by `gh aw compile` | Cannot override from `.md` source | | MCP integrity filtering | Fork PRs blocked as "unapproved" | Use `steps:` checkout instead of MCP | | `gh` CLI inside agent | Credentials scrubbed | Use `steps:` for API calls, or MCP tools | @@ -215,8 +218,8 @@ Manual triggers (`workflow_dispatch`, `issue_comment`) should bypass the gate. N | Symptom | Cause | Fix | |---------|-------|-----| | Agent evaluates wrong PR | `workflow_dispatch` checks out workflow branch | Add `gh pr checkout` in `steps:` | -| Agent can't find SKILL.md | Fork PR branch doesn't have `.github/skills/` | Agent posts "rebase or use `workflow_dispatch`" message; or rebase fork on `main` | -| Fork PR rejected on `/evaluate-tests` | Fail-closed fork guard in `Checkout-GhAwPr.ps1` | Use `workflow_dispatch` with `pr_number` input instead | +| Agent can't find SKILL.md | Fork PR branch doesn't include `.github/skills/` | Rebase fork on `main`, or use `workflow_dispatch` with `pr_number` input | +| Fork PR skipped on `pull_request` | `forks: ["*"]` not in workflow frontmatter | Add `forks: ["*"]` under `pull_request:` in the `.md` source and recompile | | `gh` commands fail in agent | Credentials scrubbed inside container | Move to `steps:` section | | Lock file out of date | Forgot to recompile | Run `gh aw compile` | | Integrity filtering warning | MCP reading fork PR data | Expected, non-blocking | diff --git a/.github/scripts/Checkout-GhAwPr.ps1 b/.github/scripts/Checkout-GhAwPr.ps1 index 4a6380a50421..a2f9533bb7d2 100644 --- a/.github/scripts/Checkout-GhAwPr.ps1 +++ b/.github/scripts/Checkout-GhAwPr.ps1 @@ -4,10 +4,13 @@ .DESCRIPTION Checks out a PR branch and restores trusted agent infrastructure (skills, - instructions) from the base branch. For issue_comment triggers, fork PRs - are rejected (fail-closed) because the platform's checkout_pr_branch.cjs - overwrites restored files after user steps. Fork PRs should use - workflow_dispatch instead. + instructions) from the base branch. Works for both same-repo and fork PRs. + + This script is only invoked for workflow_dispatch triggers. For pull_request + and issue_comment, the gh-aw platform's checkout_pr_branch.cjs handles PR + checkout automatically (it runs as a platform step after all user steps). + workflow_dispatch skips the platform checkout entirely, so this script is + the only thing that gets the PR code onto disk. SECURITY NOTE: This script checks out PR code onto disk. This is safe because NO subsequent user steps execute workspace code — the gh-aw @@ -26,7 +29,6 @@ PR_NUMBER - PR number to check out GITHUB_REPOSITORY - owner/repo (set by GitHub Actions) GITHUB_ENV - path to env file (set by GitHub Actions) - GITHUB_EVENT_NAME - trigger type (set by GitHub Actions) #> $ErrorActionPreference = 'Stop' @@ -40,25 +42,6 @@ if (-not $env:PR_NUMBER -or $env:PR_NUMBER -eq '0') { $PrNumber = $env:PR_NUMBER -# ── Fork guard (issue_comment only) ───────────────────────────────────────── -# For issue_comment triggers, platform's checkout_pr_branch.cjs runs AFTER user -# steps and re-checks out the fork branch, overwriting any restored skill/instruction -# files. A fork could include a crafted SKILL.md that alters agent behavior. -# Fail closed: if we can't verify origin, exit 1 (not 0). -# Fork PRs can still be evaluated via workflow_dispatch (where platform checkout is skipped). - -if ($env:GITHUB_EVENT_NAME -eq 'issue_comment') { - $isFork = gh pr view $PrNumber --repo $env:GITHUB_REPOSITORY --json isCrossRepository --jq '.isCrossRepository' 2>&1 - if ($LASTEXITCODE -ne 0) { - Write-Host "❌ Could not verify PR origin — failing closed" - exit 1 - } - if ($isFork -eq 'true') { - Write-Host "::notice::Fork PR detected — /evaluate-tests via issue_comment is not supported for fork PRs. Use workflow_dispatch with pr_number=$PrNumber instead." - exit 1 - } -} - # ── Save base branch SHA ───────────────────────────────────────────────────── # Must be captured BEFORE checkout replaces HEAD. # Exported for potential use by downstream platform steps (e.g., checkout_pr_branch.cjs) @@ -82,11 +65,9 @@ Write-Host "✅ Checked out PR #$PrNumber" git log --oneline -1 # ── Restore agent infrastructure from base branch ──────────────────────────── -# Best-effort restore of skill/instruction files from the base branch. -# - workflow_dispatch: platform checkout is skipped, so this IS the final state -# - issue_comment (same-repo): platform's checkout_pr_branch.cjs runs after and -# overwrites, but files already match (same repo). Fork PRs are blocked above. -# - pull_request (same-repo): files already exist, this is a no-op +# This script only runs for workflow_dispatch (other triggers use the platform's +# checkout_pr_branch.cjs instead). For workflow_dispatch the platform checkout is +# skipped, so this restore IS the final workspace state. # rm -rf first to prevent fork-added files from surviving the restore. if (Test-Path '.github/skills/') { Remove-Item -Recurse -Force '.github/skills/' } diff --git a/.github/workflows/copilot-evaluate-tests.lock.yml b/.github/workflows/copilot-evaluate-tests.lock.yml index c1e75132b153..5b5dc7f9b37d 100644 --- a/.github/workflows/copilot-evaluate-tests.lock.yml +++ b/.github/workflows/copilot-evaluate-tests.lock.yml @@ -22,7 +22,7 @@ # # Evaluates test quality, coverage, and appropriateness on PRs that add or modify tests # -# gh-aw-metadata: {"schema_version":"v2","frontmatter_hash":"9a0e480927d61956be62f85669d2e599525442694d280126849fe87e727c31df","compiler_version":"v0.62.2","strict":true} +# gh-aw-metadata: {"schema_version":"v2","frontmatter_hash":"d671028235c1b911c7a816a257b07b02793a6b57747b4358f792af183e26ca07","compiler_version":"v0.62.2","strict":true} name: "Evaluate PR Tests" "on": @@ -30,6 +30,8 @@ name: "Evaluate PR Tests" types: - created pull_request: + # forks: # Fork filtering applied via job conditions + # - "*" # Fork filtering applied via job conditions paths: - src/**/tests/** - src/**/test/** @@ -57,9 +59,9 @@ jobs: activation: needs: pre_activation if: > - (needs.pre_activation.outputs.activated == 'true') && (((github.event_name == 'pull_request' && github.event.pull_request.draft == false) || github.event_name == 'workflow_dispatch' || (github.event_name == 'issue_comment' && + (needs.pre_activation.outputs.activated == 'true') && ((github.event_name == 'pull_request' && github.event.pull_request.draft == false) || github.event_name == 'workflow_dispatch' || (github.event_name == 'issue_comment' && github.event.issue.pull_request && - startsWith(github.event.comment.body, '/evaluate-tests'))) && ((github.event_name != 'pull_request') || (github.event.pull_request.head.repo.id == github.repository_id))) + startsWith(github.event.comment.body, '/evaluate-tests'))) runs-on: ubuntu-slim permissions: contents: read @@ -324,7 +326,8 @@ jobs: run: "TEST_FILES=$(gh pr diff \"$PR_NUMBER\" --repo \"$GITHUB_REPOSITORY\" --name-only \\\n | grep -E '\\.(cs|xaml)$' \\\n | grep -iE '(tests?/|TestCases|UnitTests|DeviceTests)' \\\n || true)\nif [ -z \"$TEST_FILES\" ]; then\n echo \"⏭️ No test source files (.cs/.xaml) found in PR diff. Skipping evaluation.\"\n exit 1\nfi\necho \"✅ Found test files to evaluate:\"\necho \"$TEST_FILES\" | head -20\n" - env: GH_TOKEN: ${{ github.token }} - PR_NUMBER: ${{ github.event.pull_request.number || github.event.issue.number || inputs.pr_number }} + PR_NUMBER: ${{ inputs.pr_number }} + if: github.event_name == 'workflow_dispatch' name: Checkout PR and restore agent infrastructure run: pwsh .github/scripts/Checkout-GhAwPr.ps1 @@ -986,9 +989,9 @@ jobs: pre_activation: if: > - ((github.event_name == 'pull_request' && github.event.pull_request.draft == false) || github.event_name == 'workflow_dispatch' || (github.event_name == 'issue_comment' && + (github.event_name == 'pull_request' && github.event.pull_request.draft == false) || github.event_name == 'workflow_dispatch' || (github.event_name == 'issue_comment' && github.event.issue.pull_request && - startsWith(github.event.comment.body, '/evaluate-tests'))) && ((github.event_name != 'pull_request') || (github.event.pull_request.head.repo.id == github.repository_id)) + startsWith(github.event.comment.body, '/evaluate-tests')) runs-on: ubuntu-slim outputs: activated: ${{ steps.check_membership.outputs.is_team_member == 'true' }} diff --git a/.github/workflows/copilot-evaluate-tests.md b/.github/workflows/copilot-evaluate-tests.md index c84f83f8855e..854c2ec407d8 100644 --- a/.github/workflows/copilot-evaluate-tests.md +++ b/.github/workflows/copilot-evaluate-tests.md @@ -3,6 +3,7 @@ description: Evaluates test quality, coverage, and appropriateness on PRs that a on: pull_request: types: [opened, synchronize, reopened, ready_for_review] + forks: ["*"] paths: - 'src/**/tests/**' - 'src/**/test/**' @@ -72,10 +73,14 @@ steps: echo "✅ Found test files to evaluate:" echo "$TEST_FILES" | head -20 + # Only needed for workflow_dispatch — for pull_request and issue_comment, + # the gh-aw platform's checkout_pr_branch.cjs handles PR checkout automatically. + # workflow_dispatch skips the platform checkout entirely, so we must do it here. - name: Checkout PR and restore agent infrastructure + if: github.event_name == 'workflow_dispatch' env: GH_TOKEN: ${{ github.token }} - PR_NUMBER: ${{ github.event.pull_request.number || github.event.issue.number || inputs.pr_number }} + PR_NUMBER: ${{ inputs.pr_number }} run: pwsh .github/scripts/Checkout-GhAwPr.ps1 ---