chore: Add PR validation workflow#1602
Conversation
Automatically validates non-maintainer PRs by checking: - Issue reference exists in PR body - Referenced issue has discussion between author and maintainer - Referenced issue is not assigned to someone else Also enforces that all PRs start as drafts. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
![sentry[bot]](https://avatars.githubusercontent.com/in/12637?s=60&v=4){kind=small}
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
| env: | ||
| GH_TOKEN: ${{github.token}} | ||
| PR_URL: ${{ github.event.pull_request.html_url }} | ||
| run: | | ||
| gh pr ready "$PR_URL" --undo |
There was a problem hiding this comment.
Bug: The workflow uses the default github.token to convert a PR to a draft, which lacks the required permissions, causing the step to fail.
Severity: HIGH
Suggested Fix
In the "Convert PR to draft" step, change the GH_TOKEN environment variable to use the generated app token: GH_TOKEN: ${{ steps.app-token.outputs.token }}.
Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent.
Verify if this is a real issue. If it is, propose a fix; if not, explain why it's not
valid.
Location: .github/workflows/validate-pr.yml#L279-L283
Potential issue: The "Convert PR to draft" step in the `enforce-draft` job uses the
default `GH_TOKEN: ${{github.token}}`. This token lacks the necessary permissions to
change a pull request's draft status via the GraphQL API, which the `gh` CLI uses
internally. This will cause the step to fail with a "Resource not accessible by
integration" error whenever a non-draft PR is submitted. The workflow correctly
generates a more privileged GitHub App token in the preceding step
(`steps.app-token.outputs.token`) but fails to use it for this operation.
|
|
||
| for (const user of usersToCheck) { | ||
| if (user === prAuthor) continue; | ||
| if (await isMaintainer(repo.owner, repo.repo, user)) { |
There was a problem hiding this comment.
Bug: The maintainer check incorrectly validates against the PR's repository instead of the referenced issue's repository, breaking cross-repo logic.
Severity: CRITICAL
Suggested Fix
Change the call to isMaintainer to use the properties of the referenced issue's repository. Replace isMaintainer(repo.owner, repo.repo, user) with isMaintainer(ref.owner, ref.repo, user).
Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent.
Verify if this is a real issue. If it is, propose a fix; if not, explain why it's not
valid.
Location: .github/workflows/validate-pr.yml#L204
Potential issue: When checking for maintainer participation on a linked issue, the logic
incorrectly checks for maintainer status on the pull request's repository instead of the
issue's repository. The code at line 204 calls `isMaintainer(repo.owner, repo.repo,
user)`, but for cross-repository issue references, it should check against the issue's
repository using `ref.owner` and `ref.repo`. This bug will cause the workflow to
incorrectly close valid pull requests that reference issues in other repositories where
a maintainer of that other repository has participated.
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 2 potential issues.
Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
| runs-on: ubuntu-24.04 | ||
| permissions: | ||
| pull-requests: write | ||
| contents: write |
There was a problem hiding this comment.
Unnecessary contents: write permission grants excessive access
Medium Severity
Both jobs declare contents: write permission, but no step in either job modifies repository file contents. The workflow only interacts with PRs (comments, labels, state changes, draft conversion) which are covered by pull-requests: write. Granting contents: write on a pull_request_target trigger — which runs in the context of the base branch — unnecessarily broadens the attack surface.
Additional Locations (1)
|
|
||
| for (const user of usersToCheck) { | ||
| if (user === prAuthor) continue; | ||
| if (await isMaintainer(repo.owner, repo.repo, user)) { |
There was a problem hiding this comment.
Maintainer check uses PR repo instead of issue repo
Low Severity
The isMaintainer call passes repo.owner, repo.repo (the PR's repository) but the comment on line 193 says it checks "admin/maintain access on the issue's repo." For cross-repo references (e.g., an issue in getsentry/sentry referenced from a PR on getsentry/sentry-native), this checks maintainer status on the wrong repo. If intentional, the comment is misleading; if the comment reflects intent, ref.owner and ref.repo are needed instead.
|
Closing in favor of rolling this out via the shared composite action in getsentry/github-workflows#153 |
3 participants
Summary
validate-pr.ymlworkflow to automatically validate non-maintainer PRsRollout of getsentry/sentry-python#4233 across all SDK repos.
Test plan
SDK_MAINTAINER_BOT_APP_IDvar andSDK_MAINTAINER_BOT_PRIVATE_KEYsecret are available to this repoCo-Authored-By: Claude Opus 4.6 (1M context) noreply@anthropic.com