From ecd77e046bf83ed1da4fe464434f43c8e5b367af Mon Sep 17 00:00:00 2001 From: Bryce Adelstein Lelbach Date: Sat, 25 Jul 2026 10:26:12 +0000 Subject: [PATCH] Make signature CI pull-request aware --- .../workflows/pr-comment-git-signatures.yml | 31 +++- .github/workflows/test-git-signatures.yml | 135 +++++++++++++++--- 2 files changed, 138 insertions(+), 28 deletions(-) diff --git a/.github/workflows/pr-comment-git-signatures.yml b/.github/workflows/pr-comment-git-signatures.yml index 4dacce5f..723d3ef8 100644 --- a/.github/workflows/pr-comment-git-signatures.yml +++ b/.github/workflows/pr-comment-git-signatures.yml @@ -33,14 +33,26 @@ jobs: const pullRequests = context.payload.workflow_run.pull_requests; let prNumber; if (pullRequests && pullRequests.length > 0) { + const pullRequest = pullRequests[0]; + const currentRepo = process.env.GITHUB_REPOSITORY; + const currentPullPrefix = + `${process.env.GITHUB_API_URL}/repos/${currentRepo}/pulls/`; + const baseRepo = pullRequest.base?.repo?.full_name; + if ((pullRequest.url && !pullRequest.url.startsWith(currentPullPrefix)) || + (baseRepo && baseRepo !== currentRepo)) { + console.log('Associated pull request belongs to another repository; skipping.'); + return; + } prNumber = pullRequests[0].number; } else { - try { - prNumber = parseInt(fs.readFileSync('pr_number', 'utf8').trim()); - } catch (e) { - console.log('No PR number found, skipping comment.'); + const mirror = context.payload.workflow_run.head_branch.match( + /^pull-request\/([1-9]\d*)$/ + ); + if (!mirror) { + console.log('Workflow run is not associated with a pull request; skipping.'); return; } + prNumber = Number(mirror[1]); } if (!prNumber || isNaN(prNumber)) { console.log('No valid PR number found, skipping comment.'); @@ -55,6 +67,14 @@ jobs: return; } + const { data: pullRequest } = await github.rest.pulls.get({ + owner: context.repo.owner, + repo: context.repo.repo, + pull_number: prNumber + }); + const baseCloneUrl = pullRequest.base.repo.clone_url; + const baseRef = pullRequest.base.ref; + const runUrl = `${process.env.GITHUB_SERVER_URL}/${process.env.GITHUB_REPOSITORY}/actions/runs/${{ github.event.workflow_run.id }}`; const commentBody = ` @@ -85,7 +105,8 @@ jobs: 2. **Re-sign your commits:** \`\`\`bash - git rebase -i origin/main --exec "git commit --amend --no-edit -S" + git fetch ${baseCloneUrl} ${baseRef} + git rebase -i FETCH_HEAD --exec "git commit --amend --no-edit -S" git push --force-with-lease \`\`\` diff --git a/.github/workflows/test-git-signatures.yml b/.github/workflows/test-git-signatures.yml index 6ca6a8eb..8e1e0982 100644 --- a/.github/workflows/test-git-signatures.yml +++ b/.github/workflows/test-git-signatures.yml @@ -14,6 +14,7 @@ concurrency: # Minimal permissions - only read access needed for checks permissions: contents: read + pull-requests: read jobs: test-git-signatures: @@ -23,40 +24,133 @@ jobs: - name: Checkout repository uses: actions/checkout@v4 with: - fetch-depth: 0 # Need full history to compare with main + fetch-depth: 0 # Needed for merge-group commit ranges - name: Check commit signatures id: signature-check uses: actions/github-script@v7 with: script: | - const { execSync } = require('child_process'); - const fs = require('fs'); + const { execFileSync } = require('child_process'); - // Get current branch name const currentBranch = process.env.GITHUB_REF_NAME; console.log(`Current branch: ${currentBranch}`); - // Skip signature check on main branch to avoid failing on historical unsigned commits + // Main contains historical unsigned commits. Feature and event + // branches are checked when their commits are introduced. if (currentBranch === 'main') { console.log('On main branch - skipping signature check for historical commits.'); console.log('Signature verification only runs on PR branches.'); return; } - // Get commits that are on this branch but not on origin/main - let commitShas; - try { - const output = execSync('git rev-list origin/main..HEAD', { encoding: 'utf-8' }); - commitShas = output.trim().split('\n').filter(sha => sha.length > 0); - } catch (error) { - console.log('Could not compare with origin/main, checking all commits on this branch.'); - // Fallback: just check the current commit + const unique = values => [...new Set(values.filter(Boolean))]; + + const commitsForOpenPullRequests = async () => { + try { + const mirror = currentBranch.match(/^pull-request\/(\d+)$/); + let pulls; + if (mirror) { + const pull = await github.rest.pulls.get({ + owner: context.repo.owner, + repo: context.repo.repo, + pull_number: Number(mirror[1]) + }); + pulls = [pull.data]; + } else { + pulls = await github.paginate( + github.rest.repos.listPullRequestsAssociatedWithCommit, + { + owner: context.repo.owner, + repo: context.repo.repo, + commit_sha: context.payload.after, + per_page: 100 + } + ); + pulls = pulls.filter(pull => + pull.state === 'open' && pull.head.sha === context.payload.after + ); + } + + if (pulls.length === 0) { + return null; + } + + const shas = []; + for (const pull of pulls) { + const targetOwner = pull.base.repo.owner.login; + const targetRepo = pull.base.repo.name; + console.log( + `Checking commits from ${targetOwner}/${targetRepo}#${pull.number} ` + + `(base=${pull.base.ref}).` + ); + const commits = await github.paginate( + github.rest.repos.compareCommitsWithBasehead, + { + owner: targetOwner, + repo: targetRepo, + basehead: `${pull.base.sha}...${context.payload.after}`, + per_page: 100 + }, + response => response.data.commits + ); + shas.push(...commits.map(commit => commit.sha)); + } + return unique(shas); + } catch (error) { + throw new Error(`Could not determine pull-request commits: ${error.message}`); + } + }; + + const commitsFromPush = () => { + const pushed = context.payload.commits || []; + if (pushed.length >= 2048) { + throw new Error( + 'Push exposes 2048 commits, the event payload limit; ' + + 'refusing to skip signature checks.' + ); + } + return pushed.map(commit => commit.id); + }; + + const commitsInRange = (base, head) => { + const shaPattern = /^[0-9a-f]{40}$/; + if (!shaPattern.test(base) || !shaPattern.test(head)) { + throw new Error(`Invalid commit range: ${base}..${head}`); + } + const output = execFileSync( + 'git', ['rev-list', `${base}..${head}`], { encoding: 'utf-8' } + ); + return output.trim().split('\n').filter(Boolean); + }; + + let commitShas = null; + if (context.eventName === 'push') { + if (context.payload.deleted) { + console.log('Branch deleted - no commits to check.'); + return; + } + + commitShas = await commitsForOpenPullRequests(); + if (commitShas === null) { + if (context.payload.created) { + console.log('New branch without an open PR - checking pushed commits.'); + } else { + console.log('No open PR found - checking commits introduced by this push.'); + } + commitShas = commitsFromPush(); + } + } else if (context.eventName === 'merge_group') { + const mergeGroup = context.payload.merge_group; + commitShas = commitsInRange(mergeGroup.base_sha, mergeGroup.head_sha); + } else { commitShas = [process.env.GITHUB_SHA]; } + commitShas = unique(commitShas); + if (commitShas.length === 0) { - console.log('No new commits to check (branch is up to date with main).'); + console.log('No new commits to check.'); return; } @@ -90,8 +184,10 @@ jobs: }); } } catch (error) { - console.log(`⚠ ${sha.substring(0, 7)} - Could not verify (${error.message})`); - // Don't fail on API errors for individual commits + core.setFailed( + `Could not verify ${sha.substring(0, 7)} via GitHub API: ${error.message}` + ); + return; } } @@ -111,11 +207,6 @@ jobs: console.log(`\n✅ All ${commitShas.length} commit(s) are properly signed.`); } - - name: Get PR info - id: get-pr-info - if: always() && startsWith(github.ref_name, 'pull-request/') - uses: nv-gha-runners/get-pr-info@main - - name: Save PR comment data if: always() run: | @@ -124,8 +215,6 @@ jobs: cat << 'EOF' > ./pr-comment-data/unsigned_commits ${{ steps.signature-check.outputs.unsigned_commits }} EOF - PR_NUMBER='${{ startsWith(github.ref_name, 'pull-request/') && fromJSON(steps.get-pr-info.outputs.pr-info).number || '' }}' - echo "${PR_NUMBER}" > ./pr-comment-data/pr_number - name: Upload PR comment data if: always()