-
Notifications
You must be signed in to change notification settings - Fork 2k
Improve evaluate-pr-tests workflow: slash_command + workflow_dispatch, security hardening #34678
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from 1 commit
0cc2384
2dfcf71
d79ff20
8234b33
e1dee57
f4c4791
c089534
e6004b7
00af259
17c0f27
f4f02df
27e14d6
69c0a16
f3dc6a9
7bcc980
04bf387
082ed7a
e7fdf22
b40ccb5
d142b43
cfee2bc
e76545e
bf19b8c
0faafa9
f8ab3c5
6154bfd
3b49c7a
72356a7
338d7c1
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,9 +1,8 @@ | ||
| --- | ||
| description: Evaluates test quality, coverage, and appropriateness on PRs that add or modify tests | ||
| on: | ||
| pull_request: | ||
| types: [opened, synchronize, reopened, ready_for_review] | ||
| forks: ["*"] | ||
| pull_request_target: | ||
| types: [opened, synchronize, reopened] | ||
| paths: | ||
| - 'src/**/tests/**' | ||
| - 'src/**/test/**' | ||
|
|
@@ -15,9 +14,10 @@ on: | |
| description: 'PR number to evaluate' | ||
| required: true | ||
| type: number | ||
| roles: all | ||
|
|
||
|
PureWeen marked this conversation as resolved.
Outdated
|
||
| if: >- | ||
| (github.event_name == 'pull_request' && github.event.pull_request.draft == false) || | ||
| (github.event_name == 'pull_request_target' && github.event.pull_request.draft == false) || | ||
| github.event_name == 'workflow_dispatch' || | ||
| (github.event_name == 'issue_comment' && | ||
| github.event.issue.pull_request && | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I always guard against forks as well, preventing the workflow from running on forks except for the Simple case that needs adapting to your scenario: |
||
|
|
@@ -57,7 +57,7 @@ timeout-minutes: 15 | |
|
|
||
| steps: | ||
| - name: Gate — skip if no test source files in diff | ||
| if: github.event_name == 'pull_request' | ||
| if: github.event_name == 'pull_request_target' | ||
| env: | ||
| GH_TOKEN: ${{ github.token }} | ||
| PR_NUMBER: ${{ github.event.pull_request.number }} | ||
|
|
@@ -73,7 +73,7 @@ steps: | |
| echo "✅ Found test files to evaluate:" | ||
| echo "$TEST_FILES" | head -20 | ||
|
|
||
| # Only needed for workflow_dispatch — for pull_request and issue_comment, | ||
| # Only needed for workflow_dispatch — for pull_request_target 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 | ||
|
|
||
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.
I do think this will still lead to the 'Approve and run workflows' button showing up for PRs from untrusted forks. We need to solidify the guidance we give for when to hit that button. I really wish that button navigated into a list of workflows needing approval for the PR with boxes to select which to approve.