Skip to content

fix: add missing Checkout-GhAwPr.ps1 script#788

Merged
PureWeen merged 1 commit intomainfrom
fix/checkout-script
Apr 28, 2026
Merged

fix: add missing Checkout-GhAwPr.ps1 script#788
PureWeen merged 1 commit intomainfrom
fix/checkout-script

Conversation

@PureWeen
Copy link
Copy Markdown
Owner

review-shared.md references .github/scripts/Checkout-GhAwPr.ps1 for workflow_dispatch PR checkout, but the script was never created. Expert-review fails with exit code 64 when dispatched via workflow_dispatch.

review-shared.md references this script in the workflow_dispatch
checkout step, but it was never created. Without it, expert-review
fails with exit code 64 when dispatched via workflow_dispatch.

The script: checks PR author has write access, checks out the PR
branch, restores .github/ from the base branch to prevent prompt
injection via modified workflow files.

Co-authored-by: Copilot <[email protected]>
@PureWeen PureWeen merged commit 2b5cc77 into main Apr 28, 2026
@PureWeen PureWeen deleted the fix/checkout-script branch April 28, 2026 17:28
Copy link
Copy Markdown
Contributor

@github-actions github-actions Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Expert Code Review: 4 findings (2 critical, 2 moderate) — 3 posted inline, 1 design-level concern in summary comment below.

Generated by Expert Code Review (auto) for issue #788 · ● 7M

Comment on lines +48 to +57
git fetch origin "pull/$prNumber/head:pr-$prNumber" 2>&1 | Write-Host
git checkout "pr-$prNumber" 2>&1 | Write-Host

# Save the PR HEAD SHA
$prSha = git rev-parse HEAD
Write-Host "PR HEAD: $prSha"

# Restore .github/ from the base branch to prevent workflow tampering
Write-Host "Restoring .github/ from $baseBranch..."
git checkout "origin/$baseBranch" -- .github/ 2>&1 | Write-Host
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔴 CRITICAL · 3/3 reviewers · git exit codes never checked — security contract voided on failure

$ErrorActionPreference = 'Stop' only governs PowerShell cmdlets, not native executables. None of the three git commands check $LASTEXITCODE. If git fetch fails (deleted PR, network error), the script continues — git checkout fails silently, and the .github/ restore (line 57) runs against whatever tree is already in the workspace. The script exits 0 regardless, voiding its core security purpose.

Fix: Check $LASTEXITCODE after each git command:

git fetch origin "pull/$prNumber/head:pr-$prNumber" 2>&1 | Write-Host
if ($LASTEXITCODE -ne 0) { Write-Error "git fetch failed"; exit 1 }

git checkout "pr-$prNumber" 2>&1 | Write-Host
if ($LASTEXITCODE -ne 0) { Write-Error "git checkout failed"; exit 1 }

# ... same for line 57's .github/ restore
git checkout "origin/$baseBranch" -- .github/ 2>&1 | Write-Host
if ($LASTEXITCODE -ne 0) { Write-Error ".github/ restore failed"; exit 1 }

Alternatively, use $PSNativeCommandErrorActionPreference = $true (PowerShell 7.3+).


# Restore .github/ from the base branch to prevent workflow tampering
Write-Host "Restoring .github/ from $baseBranch..."
git checkout "origin/$baseBranch" -- .github/ 2>&1 | Write-Host
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 MODERATE · 3/3 reviewers · origin/$baseBranch may not exist — .github/ restore can silently fail

The script only fetches pull/$prNumber/head. It never fetches the base branch. In shallow clones or when the PR targets a non-default branch (e.g., release/v2), origin/$baseBranch won't exist locally. Combined with the missing exit-code checks above, this failure is silent and leaves the PR's untrusted .github/ on disk.

Fix: Fetch the base ref before restoring (prefer immutable SHA via baseRefOid):

# Add baseRefOid to the gh pr view --json fields
$baseSha = $pr.baseRefOid
git fetch origin $baseSha 2>&1 | Write-Host
if ($LASTEXITCODE -ne 0) { Write-Error "Failed to fetch base ref"; exit 1 }
git checkout $baseSha -- .github/ 2>&1 | Write-Host
if ($LASTEXITCODE -ne 0) { Write-Error "Failed to restore .github/"; exit 1 }

Comment on lines +35 to +36
if ($LASTEXITCODE -ne 0) {
Write-Warning "Could not verify author permissions: $permJson"
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔴 CRITICAL · 3/3 reviewers · Permission check fail-open — API failure silently bypasses access gate

The collaborators API returns HTTP 404 for non-collaborators (the exact users this gate should block). When $LASTEXITCODE -ne 0, the script logs a Write-Warning and continues — the checkout proceeds without any authorization check. This also triggers on rate limits, insufficient token scope, or network errors.

Scenario: A fork PR author with read-only access triggers workflow_dispatch. The API returns 404, the warning is logged and ignored, and the untrusted PR is checked out and reviewed.

Fix: Fail closed — treat API failure as denied:

if ($LASTEXITCODE -ne 0) {
    Write-Error "Could not verify author permissions (non-collaborator or API error): $permJson"
    exit 1
}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant