Add gh-aw rerun review scanner #35685
There are no files selected for viewing
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
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
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,52 @@ | ||
| --- | ||
| description: "Security rules for the Copilot PR-review pipeline. Read before editing." | ||
| applyTo: "eng/pipelines/ci-copilot.yml,eng/scripts/detect-ui-test-categories.ps1,.github/scripts/**,.github/pr-review/**,.github/skills/pr-review/**,.github/skills/verify-tests-fail-without-fix/**,.github/skills/try-fix/**,.github/skills/run-device-tests/**,.github/workflows/review-trigger.yml,.github/workflows/pr-review-queue.yml,.github/workflows/copilot-evaluate-tests.*" | ||
| --- | ||
|
|
||
| # CI Copilot pipeline — security rules | ||
|
|
||
| This pipeline runs **untrusted PR code** on AzDO agents with these tokens in scope: | ||
|
|
||
| - `GH_COMMENT_TOKEN` / `GH_TOKEN` — `maui-bot` PAT (post comments, labels, reviews on any PR) | ||
| - `COPILOT_GITHUB_TOKEN` — Copilot CLI install token | ||
| - AzDO GitHub service-connection PAT — repo contents, PRs, checks, workflows | ||
|
|
||
| Once the PR is merged into the worktree, the author controls every `.csproj`, `Directory.Build.targets`, source generator, analyzer, test, `.ps1`, and `.yml` the pipeline subsequently runs. | ||
|
|
||
| ## Rules | ||
|
|
||
| 1. **Per-task `env:` scoping.** Only put tokens a task needs. The Copilot-agent task gets `COPILOT_GITHUB_TOKEN` only — never `GH_TOKEN`. Pass `--secret-env-vars=GH_TOKEN,GITHUB_TOKEN,COPILOT_GITHUB_TOKEN` to the Copilot CLI. | ||
|
|
||
| 2. **`persistCredentials: false` on every `checkout: self`** unless the task pushes. Default checkout writes the service-connection PAT into `.git/config` as `extraheader`, readable by any subprocess. | ||
|
|
||
| 3. **Trusted-copy scripts before merging the PR.** Setup task (still on `main`) copies `.github/scripts`, `.github/skills`, `eng/scripts` to `$(Build.ArtifactStagingDirectory)/trusted-github/`, then `chmod -R a-w`. Later tasks invoke scripts from `$TRUSTED/...`, never from the merged worktree. In PowerShell use `$ScriptsDir` / `$SkillsDir` / `$EngScriptsDir` (canonical impl in `Review-PR.ps1`). New post-merge scripts must be added to the Setup copy block. | ||
|
|
||
| 4. **Strip tokens before invoking PR-controlled code.** Wrap every `dotnet build|test|run|pack`, `msbuild`, `dotnet cake`, `BuildAndRun*.ps1`, `Run-DeviceTests.ps1`, `Invoke-UITestWithRetry.ps1` in `Invoke-WithoutGhTokens { ... }` (defined in `Review-PR.ps1` and `verify-tests-fail.ps1` — saves/clears/restores `GH_TOKEN`, `GITHUB_TOKEN`, `COPILOT_GITHUB_TOKEN`). **Wrap as close to the subprocess as possible, not at the outer trusted-script boundary** — a trusted script may itself need `gh` for metadata (e.g., `verify-tests-fail.ps1` calls `Detect-TestsInDiff.ps1` which uses `gh api`), so wrapping the whole script breaks its detection path. Wrap only the line that launches the PR-controlled process. Exception: scripts that ONLY call `gh` for PR metadata (`Detect-TestsInDiff.ps1`, `Find-RegressionRisks.ps1`, `detect-ui-test-categories.ps1`) don't need wrapping at all — they keep the token. | ||
|
|
||
| 5. **Cross-phase signal files in `$(Agent.TempDirectory)`** (or `$TRUSTED`), never `$RepoRoot/...`. PR code can overwrite anything in the worktree, including a gate verdict. Readers must not silently fall back to a worktree path if the trusted one is missing. | ||
|
|
||
| 6. **Strip `##vso[...]` from PR-controlled stdout.** Pipe through `tr -d '\r' | sed -E 's/##vso\[[^]]*\]//g'` — bare `sed` misses CRLF lines and the agent will execute the directive. | ||
|
|
||
| 7. **`gh-aw` workflows.** Pin compiler version (≥ v0.68.4 strips `pull-requests: write` per `gh-aw#28767`). Regenerate `.lock.yml` with `gh aw compile` in the **same commit** as any `.md` frontmatter edit (stale lock ⇒ all dispatches fail). `workflow_dispatch` triggers must restore trusted `.github/` from main (see `Checkout-GhAwPr.ps1`). | ||
|
|
||
| 8. **No token republish.** Don't `setvariable` a token (visible to every later task, even with `issecret=true`). Don't write tokens to worktree files. Don't echo token names. | ||
|
|
||
| ## Review checklist | ||
|
|
||
| - [ ] New `checkout: self` has `persistCredentials: false`. | ||
| - [ ] New `env:` block lists only the tokens that task needs; Copilot task has no `GH_TOKEN`. | ||
| - [ ] New post-merge script invoked via `$ScriptsDir` / `$SkillsDir` / `$EngScriptsDir`, not `$RepoRoot/...`, AND added to Setup copy block. | ||
| - [ ] New invocation of PR-controlled code (`dotnet test|build|run`, `BuildAndRun*`, `Run-DeviceTests`, `Invoke-UITestWithRetry`) is wrapped in `Invoke-WithoutGhTokens` AT THE CALL SITE (not at an outer boundary). | ||
| - [ ] New cross-phase state file lives under `$(Agent.TempDirectory)` / `$TRUSTED`. | ||
| - [ ] New PR-stdout pipe uses `tr -d '\r' | sed -E 's/##vso\[[^]]*\]//g'`. | ||
| - [ ] Edited `.github/workflows/*.md` has matching `.lock.yml` regenerated in same commit. | ||
|
|
||
| ## Grep these during review | ||
|
|
||
| ```bash | ||
| git grep -nE 'dotnet (test|build|run|pack)' eng/pipelines/ci-copilot.yml .github/scripts .github/skills | grep -v Invoke-WithoutGhTokens | ||
| git grep -nE 'Join-Path \$RepoRoot ".*\.(ps1|sh)"' .github/scripts .github/skills | ||
| git grep -nA1 'checkout: self' eng/pipelines/ci-copilot.yml | grep -v persistCredentials | ||
| git grep -nE 'Set-Content.*\$RepoRoot.*(gate-result|sentinel|verdict)' .github/scripts .github/skills | ||
| git grep -nE 'sed.*##vso' eng/pipelines/ci-copilot.yml | grep -v 'tr -d' | ||
| ``` |
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
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,191 @@ | ||
| #!/usr/bin/env pwsh | ||
| <# | ||
| .SYNOPSIS | ||
| Applies AI rerun scanner decisions: react, remove queue label, and trigger AzDO. | ||
| #> | ||
|
|
||
| param( | ||
| [string]$Owner = 'dotnet', | ||
| [string]$Repo = 'maui', | ||
| [string]$DefaultPipelineRef = 'main', | ||
| [switch]$DryRun | ||
| ) | ||
|
|
||
| $ErrorActionPreference = 'Stop' | ||
| $ReadyForRerunLabel = 's/agent-ready-for-rerun' | ||
|
|
||
| . "$PSScriptRoot/shared/Update-AgentLabels.ps1" | ||
|
|
||
| function Get-AgentItems { | ||
| if (-not $env:GH_AW_AGENT_OUTPUT -or -not (Test-Path $env:GH_AW_AGENT_OUTPUT)) { | ||
| throw "GH_AW_AGENT_OUTPUT is missing or does not exist." | ||
| } | ||
|
|
||
| $payload = Get-Content -Raw -LiteralPath $env:GH_AW_AGENT_OUTPUT | ConvertFrom-Json | ||
| return @($payload.items | Where-Object { $_.type -eq 'trigger_rerun_review' }) | ||
| } | ||
|
|
||
| function Add-CommentReaction { | ||
| param( | ||
| [Parameter(Mandatory = $true)][Int64]$CommentId, | ||
| [Parameter(Mandatory = $true)][ValidateSet('+1', '-1')][string]$Content | ||
| ) | ||
|
|
||
| if ($DryRun) { | ||
| Write-Host "[dry-run] Would react '$Content' to comment $CommentId" | ||
| return | ||
| } | ||
|
|
||
| $tmp = New-TemporaryFile | ||
| try { | ||
| @{ content = $Content } | ConvertTo-Json -Compress | Set-Content -LiteralPath $tmp -Encoding utf8 -NoNewline | ||
| & gh api "repos/$Owner/$Repo/issues/comments/$CommentId/reactions" ` | ||
| --method POST ` | ||
| -H "Accept: application/vnd.github+json" ` | ||
| --input $tmp 1>$null 2>$null | ||
| if ($LASTEXITCODE -ne 0) { | ||
| Write-Host " ⚠️ Reaction '$Content' may already exist on comment $CommentId" -ForegroundColor Yellow | ||
| } else { | ||
| Write-Host " ✅ Reacted '$Content' to comment $CommentId" -ForegroundColor Green | ||
| } | ||
| } finally { | ||
| Remove-Item -LiteralPath $tmp -Force -ErrorAction SilentlyContinue | ||
| } | ||
| } | ||
|
|
||
| function Get-PlatformFromLabels { | ||
| param([string[]]$Labels, [string]$Fallback) | ||
|
|
||
| if ($Fallback) { | ||
| return $Fallback | ||
| } | ||
|
|
||
| $lower = @($Labels | ForEach-Object { $_.ToLowerInvariant() }) | ||
| if ($lower -contains 'platform/ios') { return 'ios' } | ||
| if ($lower -contains 'platform/macos' -or $lower -contains 'platform/maccatalyst') { return 'catalyst' } | ||
| if ($lower -contains 'platform/android') { return 'android' } | ||
| if ($lower -contains 'platform/windows') { return 'windows' } | ||
| return 'android' | ||
| } | ||
|
|
||
| function Invoke-AzDOReviewPipeline { | ||
| param( | ||
| [Parameter(Mandatory = $true)][int]$PRNumber, | ||
| [Parameter(Mandatory = $true)][string]$Platform, | ||
| [string]$PipelineRef = 'main' | ||
| ) | ||
|
|
||
| if ($DryRun) { | ||
| Write-Host "[dry-run] Would trigger maui-copilot for PR #$PRNumber (platform: $Platform, ref: $PipelineRef)" | ||
| return | ||
| } | ||
|
|
||
| if (-not $env:AZDO_TRIGGER_TENANT_ID -or -not $env:AZDO_TRIGGER_CLIENT_ID) { | ||
| throw "AZDO_TRIGGER_TENANT_ID and AZDO_TRIGGER_CLIENT_ID secrets are required to trigger AzDO." | ||
| } | ||
|
|
||
| $oidcResponse = Invoke-RestMethod ` | ||
| -Headers @{ Authorization = "bearer $env:ACTIONS_ID_TOKEN_REQUEST_TOKEN" } ` | ||
| -Uri "$($env:ACTIONS_ID_TOKEN_REQUEST_URL)&audience=api://AzureADTokenExchange" | ||
| $oidcToken = $oidcResponse.value | ||
| if (-not $oidcToken) { | ||
| throw "Failed to get GitHub OIDC token." | ||
| } | ||
| "::add-mask::$oidcToken" | Write-Host | ||
|
|
||
| $aadResponse = Invoke-RestMethod ` | ||
| -Method Post ` | ||
| -Uri "https://login.microsoftonline.com/$($env:AZDO_TRIGGER_TENANT_ID)/oauth2/v2.0/token" ` | ||
| -Body @{ | ||
| grant_type = 'client_credentials' | ||
| client_id = $env:AZDO_TRIGGER_CLIENT_ID | ||
| client_assertion_type = 'urn:ietf:params:oauth:client-assertion-type:jwt-bearer' | ||
| client_assertion = $oidcToken | ||
| scope = '499b84ac-1321-427f-aa17-267ca6975798/.default' | ||
| } | ||
|
|
||
| $azdoToken = $aadResponse.access_token | ||
| if (-not $azdoToken) { | ||
| throw "Failed to get Azure DevOps token." | ||
| } | ||
| "::add-mask::$azdoToken" | Write-Host | ||
|
|
||
| $payload = @{ | ||
| templateParameters = @{ | ||
| PRNumber = [string]$PRNumber | ||
| Platform = $Platform | ||
| } | ||
| resources = @{ | ||
| repositories = @{ | ||
| self = @{ | ||
| refName = "refs/heads/$PipelineRef" | ||
| } | ||
| } | ||
| } | ||
| } | ConvertTo-Json -Depth 10 | ||
|
|
||
| $response = Invoke-RestMethod ` | ||
| -Method Post ` | ||
| -Uri "https://dev.azure.com/DevDiv/DevDiv/_apis/pipelines/27723/runs?api-version=7.1" ` | ||
| -Headers @{ Authorization = "Bearer $azdoToken"; 'Content-Type' = 'application/json' } ` | ||
| -Body $payload | ||
|
|
||
| Write-Host " ✅ Triggered maui-copilot run $($response.id) for PR #$PRNumber" | ||
| } | ||
|
|
||
| $items = Get-AgentItems | ||
| if ($items.Count -eq 0) { | ||
| Write-Host "No trigger_rerun_review decisions found." | ||
| exit 0 | ||
| } | ||
|
|
||
| foreach ($item in $items) { | ||
| $prNumber = [int]$item.pr_number | ||
| $decision = ([string]$item.decision).Trim().ToLowerInvariant() | ||
| $rerunCommentId = [Int64]$item.rerun_comment_id | ||
| $reason = [string]$item.reason | ||
| $expectedHeadSha = [string]$item.expected_head_sha | ||
|
|
||
| if ($decision -notin @('trigger', 'skip')) { | ||
| throw "Invalid decision '$decision' for PR #$prNumber." | ||
| } | ||
| if ($prNumber -le 0) { | ||
| throw "Invalid PR number '$($item.pr_number)'." | ||
| } | ||
| if ($rerunCommentId -le 0) { | ||
|
Member
There was a problem hiding this comment. ❌ Logic — This validation runs before branching on So a valid Fix: Only require |
||
| throw "Invalid rerun comment id '$($item.rerun_comment_id)' for PR #$prNumber." | ||
| } | ||
|
|
||
| Write-Host "Processing PR #$prNumber decision=$decision reason=$reason" | ||
| $pr = gh api "repos/$Owner/$Repo/pulls/$prNumber" | ConvertFrom-Json | ||
| if ($pr.state -ne 'open') { | ||
| Write-Host " ⏭️ PR #$prNumber is not open ($($pr.state)); skipping" | ||
| continue | ||
| } | ||
| if ($expectedHeadSha -and $pr.head.sha -ne $expectedHeadSha) { | ||
| Write-Host " ⏭️ PR #$prNumber head changed from $expectedHeadSha to $($pr.head.sha); skipping stale decision" | ||
| continue | ||
| } | ||
|
|
||
| $labels = @(gh api "repos/$Owner/$Repo/issues/$prNumber/labels" --jq '.[].name' 2>$null) | ||
| if ($labels -notcontains $ReadyForRerunLabel) { | ||
| Write-Host " ⏭️ PR #$prNumber no longer has $ReadyForRerunLabel; skipping" | ||
| continue | ||
| } | ||
|
|
||
| if ($decision -eq 'trigger') { | ||
| Add-CommentReaction -CommentId $rerunCommentId -Content '+1' | ||
| $platform = Get-PlatformFromLabels -Labels $labels -Fallback ([string]$item.platform) | ||
| Invoke-AzDOReviewPipeline -PRNumber $prNumber -Platform $platform -PipelineRef $DefaultPipelineRef | ||
| } else { | ||
| Add-CommentReaction -CommentId $rerunCommentId -Content '-1' | ||
| Write-Host " ⏭️ AI scanner decided not to trigger PR #$prNumber" | ||
| } | ||
|
|
||
| if ($DryRun) { | ||
| Write-Host "[dry-run] Would remove $ReadyForRerunLabel from PR #$prNumber" | ||
| } else { | ||
| Remove-Label -PRNumber $prNumber -LabelName $ReadyForRerunLabel -Owner $Owner -Repo $Repo | Out-Null | ||
| Write-Host " ✅ Removed $ReadyForRerunLabel from PR #$prNumber" | ||
| } | ||
| } | ||
Oops, something went wrong.
Oops, something went wrong.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
refName = "refs/heads/$PipelineRef"blindly prependsrefs/heads/even when$PipelineRefalready starts with it.Normalize-PipelineRefstrips most metacharacters but allows/, sorefs/heads/mainsurvives unchanged → final refName isrefs/heads/refs/heads/main, which AzDO rejects.User scenario: someone types
/review --branch refs/heads/main(a perfectly reasonable expectation given the rest of.github/usesrefs/heads/...). The pipeline trigger fails, the catch block re-throws, the entire scanner batch aborts (see comment on line 171). Same hazard exists in the bash version of this prepend atreview-trigger.yml:369.Flagged by: 1/3 reviewer; verified directly against source.
Fix: In
Normalize-PipelineRef(and the equivalent block inResolve-RerunEligibility.ps1andreview-trigger.yml), strip a leadingrefs/heads/before sanitization.