chore: update PR review workflows to latest version #26
Conversation
- Rename pr-review-axrun.yml to pr-review-code.yml - Add pr-review-approach.yml for high-level approach reviews - Remove legacy workflow files
Jercik
force-pushed
the
feat/update-pr-review-workflow
branch
from
3288539 to
d6c793a
Compare
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 32885392ef
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| - name: Checkout PR head | ||
| uses: actions/checkout@v6 | ||
| with: | ||
| ref: ${{ steps.pr.outputs.head_sha }} | ||
| fetch-depth: 10 |
There was a problem hiding this comment.
Because the job if allows workflow_dispatch regardless of fork status, maintainers can manually run this workflow for forked PRs. In that case gh pr view (lines 47–54) will return a head_sha that exists only in the fork, but actions/checkout here uses the base repository by default with just ref, so the checkout will fail with an unknown ref when the commit is not in the base repo. If manual runs are intended to support forked PRs, also capture headRepository from gh pr view and pass it as repository: (or restrict workflow_dispatch to non‑forks).
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 32885392ef
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| env: | ||
| AXRUN_ALLOW: "read,glob,grep,bash:gh api*,bash:gh pr comment*,bash:gh pr diff*,bash:gh pr view*,bash:npx -y askpplx*,bash:ls*,bash:cat*,bash:grep*,bash:tree*,bash:pwd*,bash:head*,bash:tail*,bash:jq*,bash:wc*,bash:sort*,bash:uniq*,bash:cut*,bash:git diff*,bash:git log*,bash:git show*,bash:git status*,bash:git rev-parse*" | ||
| # Allow all CLI commands - safe in GitHub Actions sandboxed environment | ||
| AXRUN_ALLOW: "read,write,glob,grep,bash:*" |
There was a problem hiding this comment.
Because this workflow runs on pull_request_target and exports secrets (AXVAULT, PERPLEXITY_API_KEY, GITHUB_TOKEN), setting AXRUN_ALLOW to bash:* gives the LLM unrestricted shell access; a malicious PR description or file can prompt-inject the agent into running env, curl, or similar to exfiltrate those secrets. This is a new risk compared to the previous allow list of specific commands; consider scoping the allow list to the minimal commands needed, removing secrets from this job, or running without pull_request_target to avoid secret exposure.
Useful? React with 👍 / 👎.
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
| env: | ||
| AXRUN_ALLOW: "read,glob,grep,bash:gh api*,bash:gh pr comment*,bash:gh pr diff*,bash:gh pr view*,bash:npx -y askpplx*,bash:ls*,bash:cat*,bash:grep*,bash:tree*,bash:pwd*,bash:head*,bash:tail*,bash:jq*,bash:wc*,bash:sort*,bash:uniq*,bash:cut*,bash:git diff*,bash:git log*,bash:git show*,bash:git status*,bash:git rev-parse*" | ||
| # Allow all CLI commands - safe in GitHub Actions sandboxed environment | ||
| AXRUN_ALLOW: "read,write,glob,grep,bash:*" |
There was a problem hiding this comment.
ℹ️ Info: The permission expansion from a restrictive whitelist (bash:gh api*, bash:ls*, etc.) to bash:* is a notable security posture change. The comment explains the rationale (GitHub Actions sandbox), which is reasonable for this use case. The workflow already has good security practices:
- Uses
pull_request_targetwith trusted code from base branch - Skips forked PRs (
github.event.pull_request.head.repo.fork == false) - Hardened npm settings prevent registry attacks
No action needed - just noting this for visibility.
|
|
||
| on: | ||
| pull_request_target: | ||
| types: [opened] |
There was a problem hiding this comment.
🟡 Medium: The PR description states "Remove legacy workflow files" but no files were actually deleted in this PR. The pr-review-axrun.yml was renamed to pr-review-code.yml, not removed. Consider updating the PR description to accurately reflect the changes:
- Rename
pr-review-axrun.ymltopr-review-code.yml - Add
pr-review-approach.yml Remove legacy workflow files(this didn't happen)
| env: | ||
| AXRUN_ALLOW: "read,glob,grep,bash:gh api*,bash:gh pr comment*,bash:gh pr diff*,bash:gh pr view*,bash:npx -y askpplx*,bash:ls*,bash:cat*,bash:grep*,bash:tree*,bash:pwd*,bash:head*,bash:tail*,bash:jq*,bash:wc*,bash:sort*,bash:uniq*,bash:cut*,bash:git diff*,bash:git log*,bash:git show*,bash:git status*,bash:git rev-parse*" | ||
| # Allow all CLI commands - safe in GitHub Actions sandboxed environment | ||
| AXRUN_ALLOW: "read,write,glob,grep,bash:*" |
There was a problem hiding this comment.
🔴 Critical: Broad bash:* and write permissions on pull_request_target create a significant security vulnerability. Since this workflow has access to secrets (AXVAULT, PERPLEXITY_API_KEY) and write permissions, an attacker could manipulate the PR code (which the agent reads) to trick the LLM into executing malicious commands (e.g., curl to exfiltrate secrets).
Recommendation: Revert to a strict allowlist (e.g., bash:gh*,bash:git*) and remove write unless absolutely necessary.
| type: number | ||
|
|
||
| env: | ||
| AXRUN_ALLOW: "read,write,glob,grep,bash:*" |
There was a problem hiding this comment.
🔴 Critical: Same security vulnerability as noted in pr-review-code.yml. bash:* allows execution of any command, and pull_request_target provides the secrets. This combination allows malicious PRs to exfiltrate secrets via prompt injection.
Jercik
left a comment
Jercik
left a comment
There was a problem hiding this comment.
PR Review Details
Main concern is security posture of the PR review workflows: the updated AXRUN allowlist is now fully permissive (), which increases blast radius if an LLM is prompt-injected by PR content while secrets are available in .
Summary: Found 2 critical, 1 high, and 1 medium issue.
Review by Copilot (gpt-5.2)
| env: | ||
| AXRUN_ALLOW: "read,glob,grep,bash:gh api*,bash:gh pr comment*,bash:gh pr diff*,bash:gh pr view*,bash:npx -y askpplx*,bash:ls*,bash:cat*,bash:grep*,bash:tree*,bash:pwd*,bash:head*,bash:tail*,bash:jq*,bash:wc*,bash:sort*,bash:uniq*,bash:cut*,bash:git diff*,bash:git log*,bash:git show*,bash:git status*,bash:git rev-parse*" | ||
| # Allow all CLI commands - safe in GitHub Actions sandboxed environment | ||
| AXRUN_ALLOW: "read,write,glob,grep,bash:*" |
There was a problem hiding this comment.
🔴 Critical: gives the LLM effectively arbitrary command execution in a workflow (secrets are available for non-fork PRs). If PR content prompt-injects the agent, it can trivially exfiltrate secrets by running commands and posting them back via /PR comments. Recommend reverting to a least-privilege allowlist (like the previous , , , , etc.) and avoid broad unless you can guarantee no secrets + no untrusted inputs are ever read.
| type: number | ||
|
|
||
| env: | ||
| AXRUN_ALLOW: "read,write,glob,grep,bash:*" |
There was a problem hiding this comment.
🔴 Critical: Same issue here: is fully permissive (). In combination with + checking out PR head and reading untrusted PR files, this materially increases the risk of prompt-injection leading to secret exfiltration. The comment 'safe in GitHub Actions sandboxed environment' is misleading when secrets are present.
|
|
||
| jobs: | ||
| review: | ||
| if: github.event_name == 'workflow_dispatch' || github.event.pull_request.head.repo.fork == false |
There was a problem hiding this comment.
🟠 High: bypasses the fork safety check for manual runs. A maintainer can accidentally this workflow against a fork PR number, which would then run with secrets and check out untrusted fork code/content. Suggest adding an explicit guard after resolving PR info (e.g., or ) and when it’s a fork/cross-repo PR.
|
|
||
| on: | ||
| pull_request_target: | ||
| types: [opened] |
There was a problem hiding this comment.
🟡 Medium: This approach review only triggers on . If you expect it to re-run when the PR changes, include / (like the code review workflow). If the intent is 'one-time approach review', this is fine.
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| - name: Checkout PR head | ||
| uses: actions/checkout@v6 | ||
| with: | ||
| ref: ${{ steps.pr.outputs.head_sha }} | ||
| fetch-depth: 10 | ||
|
|
||
| - name: Setup Node.js |
Check failure
Code scanning / CodeQL
Checkout of untrusted code in trusted context High
There was a problem hiding this comment.
Pull request overview
This PR updates the PR review workflows to the latest version synced from the review-tests repository. The changes rename the existing code review workflow and add a new high-level approach review workflow.
Changes:
- Renamed
pr-review-axrun.ymltopr-review-code.ymlwith updated prompts and guidance - Added
pr-review-approach.ymlfor high-level architectural review - Removed the "codex" agent from the review matrix
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
.github/workflows/pr-review-code.yml |
Renamed workflow with enhanced review guidance, simplified AXRUN_ALLOW permissions, removed codex agent, and added provider parameter support |
.github/workflows/pr-review-approach.yml |
New workflow for high-level approach reviews that runs only on PR open events |
Comments suppressed due to low confidence (1)
.github/workflows/pr-review-code.yml:246
- The example signature should read "Code review by" to match the updated format specified in line 300. The current example shows "Review by Claude (opus)" but should be "Code review by Claude (opus)" for consistency.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
PR Review Details
The introduction of pr-review-approach.yml and the update to pr-review-code.yml significantly broaden the capabilities of the review agents. While this enables more thorough reviews (reproduction, deep exploration), the combination of pull_request_target, checkout of the head SHA, and bash:* permissions creates a potential security risk, even with the fork restriction.
Summary: Found 2 high, 1 medium, and 1 info issue. Major concerns regarding the expanded bash:* permissions in pull_request_target context.
Review by Gemini (gemini-3-pro-preview)
| type: number | ||
|
|
||
| env: | ||
| AXRUN_ALLOW: "read,write,glob,grep,bash:*" |
There was a problem hiding this comment.
🔴 High: Security Risk: Granting bash:* permission in a pull_request_target workflow that checks out the PR head (ref: ${{ steps.pr.outputs.head_sha }}) is dangerous.
If the agent decides to execute a script from the PR (e.g., for "exploration" or "reproduction"), it runs in the context of the base repo with access to secrets (AXVAULT, etc.). A malicious PR could include a script that exfiltrates these secrets.
Recommendation:
- Remove
bash:*if not strictly needed for an "approach" review. - If execution is needed, do not use
pull_request_targetwith an unsafe checkout. See GitHub Security Lab: Preventing pwn requests.
|
|
||
| jobs: | ||
| review: | ||
| if: github.event_name == 'workflow_dispatch' || github.event.pull_request.head.repo.fork == false |
There was a problem hiding this comment.
🟡 Medium: Logic: This condition restricts the workflow to internal PRs only (fork == false).
If the goal of using pull_request_target was to support external contributors (forks) with access to secrets, this disables it.
If the goal is strictly internal reviews, pull_request (non-target) might be safer and sufficient, as internal branches typically have access to secrets anyway.
Recommendation: Verify if external contributors are intended to be supported. If so, this check prevents it (but fixing the security issue above is a prerequisite for enabling forks).
| env: | ||
| AXRUN_ALLOW: "read,glob,grep,bash:gh api*,bash:gh pr comment*,bash:gh pr diff*,bash:gh pr view*,bash:npx -y askpplx*,bash:ls*,bash:cat*,bash:grep*,bash:tree*,bash:pwd*,bash:head*,bash:tail*,bash:jq*,bash:wc*,bash:sort*,bash:uniq*,bash:cut*,bash:git diff*,bash:git log*,bash:git show*,bash:git status*,bash:git rev-parse*" | ||
| # Allow all CLI commands - safe in GitHub Actions sandboxed environment | ||
| AXRUN_ALLOW: "read,write,glob,grep,bash:*" |
There was a problem hiding this comment.
🟠 High: Security Risk: Expanding permissions to bash:* significantly increases the attack surface.
The prompt explicitly encourages the agent to "Run a minimal reproduction" and "Explore Beyond the Diff". If the agent executes code from the PR (e.g., npm test, node reproduce.js), it risks Remote Code Execution (RCE) with access to secrets, as this workflow runs on pull_request_target with the head SHA checked out.
| | General architectural observation | N/A | `comments` array, attach to first changed line of any modified file | | ||
| **Use inline comments only for line-specific issues.** File-level observations go in the summary body—the GitHub API requires a `line` number for every entry in the `comments` array. | ||
| **All issues go in inline comments.** The GitHub API requires a `line` number for every entry in the `comments` array, and that line must appear in a diff hunk. If an issue doesn't have a specific line, use the first changed line of the most relevant modified file (or any modified file if none is more relevant). |
There was a problem hiding this comment.
ℹ️ Info: Excellent update to the instructions. Explicitly telling the agent where to place file-level and architectural issues handles the GitHub API limitations gracefully.
| name: PR Approach Review | ||
|
|
||
| on: | ||
| pull_request_target: |
There was a problem hiding this comment.
ℹ️ Info: Running approach reviews only on opened (not synchronize or reopened) makes sense—the overall approach typically doesn't change between commits. The code review workflow handles subsequent pushes.
| env: | ||
| AXRUN_ALLOW: "read,glob,grep,bash:gh api*,bash:gh pr comment*,bash:gh pr diff*,bash:gh pr view*,bash:npx -y askpplx*,bash:ls*,bash:cat*,bash:grep*,bash:tree*,bash:pwd*,bash:head*,bash:tail*,bash:jq*,bash:wc*,bash:sort*,bash:uniq*,bash:cut*,bash:git diff*,bash:git log*,bash:git show*,bash:git status*,bash:git rev-parse*" | ||
| # Allow all CLI commands - safe in GitHub Actions sandboxed environment | ||
| AXRUN_ALLOW: "read,write,glob,grep,bash:*" |
There was a problem hiding this comment.
ℹ️ Info: The comment accurately explains the rationale for bash:*. In GitHub Actions' sandboxed environment with ephemeral runners, the previous restrictive allowlist added maintenance burden without meaningful security benefit—attackers who could inject commands into the workflow YAML already have full access.
Jercik
left a comment
Jercik
left a comment
There was a problem hiding this comment.
Summary: Found 2 critical issues.
Review by Copilot (gpt-5.2)
| env: | ||
| AXRUN_ALLOW: "read,glob,grep,bash:gh api*,bash:gh pr comment*,bash:gh pr diff*,bash:gh pr view*,bash:npx -y askpplx*,bash:ls*,bash:cat*,bash:grep*,bash:tree*,bash:pwd*,bash:head*,bash:tail*,bash:jq*,bash:wc*,bash:sort*,bash:uniq*,bash:cut*,bash:git diff*,bash:git log*,bash:git show*,bash:git status*,bash:git rev-parse*" | ||
| # Allow all CLI commands - safe in GitHub Actions sandboxed environment | ||
| AXRUN_ALLOW: "read,write,glob,grep,bash:*" |
There was a problem hiding this comment.
🔴 Critical: AXRUN_ALLOW is set to bash:* (line 12) while the job also exposes secrets via env (AXVAULT, PERPLEXITY_API_KEY) and checks out PR code under pull_request_target (lines 7-8, 41-45). This creates a prompt-injection / secret-exfiltration risk because untrusted PR content can influence the agent to run arbitrary commands and leak secrets. GitHub Security Lab explicitly warns that combining pull_request_target with checkout of PR head increases impact and can lead to secret theft/repo compromise: https://securitylab.github.com/resources/github-actions-preventing-pwn-requests/ and https://securitylab.github.com/resources/github-actions-new-patterns-and-mitigations/.
Fix: tighten the allowlist to the minimal needed commands (e.g., gh pr view, gh pr diff, gh api .../reviews, cat, grep), and avoid bash:* (especially network-capable tools). Also the comment “safe in GitHub Actions sandboxed environment” is misleading—runners can make outbound network calls, so secrets can be exfiltrated.
|
|
||
| jobs: | ||
| review: | ||
| if: github.event_name == 'workflow_dispatch' || github.event.pull_request.head.repo.fork == false |
There was a problem hiding this comment.
🔴 Critical: The job-level if allows workflow_dispatch runs unconditionally (github.event_name == 'workflow_dispatch' || ...fork == false, line 18). In the dispatch path, the workflow resolves an arbitrary PR number and then checks out that PR’s head SHA (lines 47-60). This means a maintainer can accidentally run the workflow (with secrets enabled) against a fork PR, defeating the fork guard and executing untrusted code with secret access. This is the exact risk class called out for pull_request_target workflows: https://securitylab.github.com/resources/github-actions-preventing-pwn-requests/.
Fix: after resolving PR_NUMBER, query PR metadata (e.g., gh pr view $PR_NUMBER --json headRepository --jq '.headRepository.isFork') and fail/exit for forks (or run a “no-secrets” mode for fork PRs).
Summary
pr-review-axrun.ymltopr-review-code.yml(detailed code review)pr-review-approach.yml(high-level approach review)Synced from review-tests repository.