chore: update PR review workflows to latest version #13
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
922398d to
7e90c05
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: 922398dfbf
ℹ️ 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.
In this pull_request_target workflow, broadening AXRUN_ALLOW to bash:* gives the agent arbitrary command execution while secrets are present (AXVAULT, PERPLEXITY_API_KEY, GH_TOKEN). If a PR description or diff contains prompt‑injection content, the agent can now run exfiltration commands (e.g., curl) that were previously blocked by the narrower allowlist, which is a concrete secret‑leak risk. Consider restoring a minimal allowlist or moving unrestricted execution to a trusted, non‑PR context.
Useful? React with 👍 / 👎.
| type: number | ||
|
|
||
| env: | ||
| AXRUN_ALLOW: "read,write,glob,grep,bash:*" |
There was a problem hiding this comment.
This workflow also runs on pull_request_target with secrets available and now sets AXRUN_ALLOW to bash:*. That means an injected prompt from untrusted PR content can trigger arbitrary shell commands and leak secrets over the network; the previous constrained allowlist would have mitigated this. Keeping a restricted command set (or isolating the job to trusted triggers only) avoids a new secret‑exfiltration path.
Useful? React with 👍 / 👎.
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
|
|
||
| on: | ||
| pull_request_target: | ||
| types: [opened] |
There was a problem hiding this comment.
ℹ️ Info: The pr-review-approach.yml workflow only triggers on opened events, while pr-review-code.yml triggers on opened, synchronize, reopened. This is an intentional design difference—approach reviews only run once when the PR is created since the high-level approach typically doesn't change with subsequent commits. Worth noting that this means pushing new commits won't re-trigger approach reviews, which seems like the intended behavior.
| 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 change from a restrictive allow-list (bash:gh api*,bash:gh pr comment*,...) to bash:* broadens permissions significantly. The comment explains this is safe in GitHub Actions' sandboxed environment, which is a reasonable justification—the workflow already has pull-requests: write permissions and runs on ubuntu-latest with ephemeral VMs. This simplifies maintenance by not requiring updates when new commands are needed.
There was a problem hiding this comment.
PR Review Details
The introduction of AXRUN_ALLOW: "read,write,glob,grep,bash:*" in a pull_request_target workflow presents a significant security risk. Since the workflow checks out untrusted code (PR head) and has access to secrets, an LLM agent subject to prompt injection could be coerced into executing arbitrary commands to exfiltrate these secrets.
Summary: Found 2 high security issues and 2 low logic issues. The relaxation of permissions in the review workflows is the primary concern.
Review by Gemini (gemini-3-pro-preview)
| 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: Allowing bash:* in a pull_request_target workflow that checks out untrusted code is dangerous.
The workflow has access to secrets (AXVAULT, PERPLEXITY_API_KEY). If the LLM is prompt-injected by the content of a file in the PR (e.g., via a comment), it could be tricked into executing arbitrary commands (like exfiltrating environment variables via curl).
Recommendation: Revert to a strict allowlist of commands (e.g., git, gh, ls, cat) or remove bash:* and only allow read-only tools.
| type: number | ||
|
|
||
| env: | ||
| AXRUN_ALLOW: "read,write,glob,grep,bash:*" |
There was a problem hiding this comment.
🟠 High: Security Risk: Same issue as pr-review-code.yml. pull_request_target with bash:* permission and secrets exposes the repo to exfiltration via prompt injection from untrusted PR code.
| npm_config_https_proxy= \ | ||
| npx -y axrun@latest --agent ${{ matrix.agent }} --model ${{ matrix.model }} \ | ||
| npx -y axrun@latest --agent ${{ matrix.agent }} \ | ||
| ${{ matrix.provider && format('--provider "{0}"', matrix.provider) || '' }} \ |
There was a problem hiding this comment.
🟢 Low: matrix.provider is used here but is not defined in the matrix strategy above. This expression will always evaluate to an empty string. If you intended to support providers (e.g., for copilot), add them to the matrix definition; otherwise, this line can be removed.
| npm_config_proxy= \ | ||
| npm_config_https_proxy= \ | ||
| npx -y axrun@latest --agent ${{ matrix.agent }} \ | ||
| ${{ matrix.provider && format('--provider "{0}"', matrix.provider) || '' }} \ |
There was a problem hiding this comment.
🟢 Low: Same as pr-review-code.yml: matrix.provider is not defined in the matrix, so this effectively does nothing.
| type: number | ||
|
|
||
| env: | ||
| AXRUN_ALLOW: "read,write,glob,grep,bash:*" |
There was a problem hiding this comment.
ℹ️ Info: The permission model AXRUN_ALLOW: "read,write,glob,grep,bash:*" grants full bash access within the sandboxed GitHub Actions environment. This is appropriate for trusted code running from pull_request_target (base branch), since the prompt is embedded and cannot be modified by PRs. The fork check at line 18 also prevents untrusted code from accessing secrets.
|
|
||
| jobs: | ||
| review: | ||
| if: github.event_name == 'workflow_dispatch' || github.event.pull_request.head.repo.fork == false |
There was a problem hiding this comment.
ℹ️ Info: The condition github.event_name == 'workflow_dispatch' || github.event.pull_request.head.repo.fork == false correctly handles both manual triggers and automated PR events while blocking forked PRs. This is consistent with the security model used in pr-review-code.yml.
| 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 why bash:* is safe here—GitHub Actions provides sandboxing, and pull_request_target ensures the workflow code comes from the trusted base branch rather than the PR.
There was a problem hiding this comment.
Summary: Found 0 issues. The new approach review workflow is well-structured and properly secured for internal PRs. The updates to the code review prompt successfully address previous limitations regarding output formatting and context gathering.
Review by Gemini (gemini-3-pro-preview)
|
|
||
| jobs: | ||
| review: | ||
| if: github.event_name == 'workflow_dispatch' || github.event.pull_request.head.repo.fork == false |
There was a problem hiding this comment.
ℹ️ Info: Correctly restricts pull_request_target to internal PRs (fork == false). This is crucial for security as it prevents malicious code from forks from accessing secrets or writing to the repo via the trusted workflow context.
| pull_request_target: | ||
| types: [opened, synchronize, reopened] | ||
|
|
||
| env: |
There was a problem hiding this comment.
ℹ️ Info: Switching to permissive bash:* simplifies workflow maintenance. Given the fork == false restriction and the isolated nature of the axrun environment, this trade-off between strict whitelisting and flexibility is acceptable for internal use.
There was a problem hiding this comment.
Pull request overview
This PR updates the PR review workflows by renaming pr-review-axrun.yml to pr-review-code.yml and adding a new pr-review-approach.yml workflow for high-level approach reviews. The changes synchronize workflow configurations with the review-tests repository and update review instructions to be more comprehensive.
Changes:
- Renamed workflow file and expanded code review instructions with additional guidance on exploring codebases and posting reviews
- Added new approach review workflow for high-level architectural feedback
- Removed the "codex" agent from the matrix configuration and updated environment variable permissions
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
.github/workflows/pr-review-code.yml |
Renamed from pr-review-axrun.yml, updated review instructions, expanded AXRUN_ALLOW permissions, removed codex agent, added provider parameter support |
.github/workflows/pr-review-approach.yml |
New workflow for high-level approach reviews with simplified review criteria focused on alternative solutions |
Comments suppressed due to low confidence (5)
.github/workflows/pr-review-code.yml:12
- The AXRUN_ALLOW environment variable now permits "write" operations and uses a wildcard for bash commands ("bash:*"). This significantly expands the allowed operations compared to the previous restricted list. While the comment states this is "safe in GitHub Actions sandboxed environment," this increases the attack surface. Consider whether write permissions are necessary for a PR review workflow, as reviews typically only need to read code and post comments.
.github/workflows/pr-review-code.yml:33 - The model name "gemini-3-pro-preview" may not exist. As of January 2025, Google's Gemini models include versions like "gemini-1.5-pro" and "gemini-2.0-flash-exp", but there is no "gemini-3-pro-preview" version documented. Verify this model name is correct and supported by the axrun tool, or update to a valid Gemini model version.
.github/workflows/pr-review-code.yml:361 - The npx command now includes a conditional provider parameter using GitHub Actions expression syntax. However, the matrix configuration doesn't define a "provider" field for any of the agents (claude, gemini, copilot). This means the condition will always evaluate to empty string, making this parameter effectively unused. If provider configuration is intended for future use, consider documenting it or adding it to the matrix where needed.
.github/workflows/pr-review-code.yml:37 - The model name "gpt-5.2" does not exist. As of January 2025, OpenAI's GPT models include GPT-4, GPT-4 Turbo, and various GPT-3.5 versions, but there is no GPT-5.2 release. Verify this model name is correct and supported, or update to a valid OpenAI model identifier like "gpt-4" or "gpt-4-turbo".
.github/workflows/pr-review-code.yml:34 - The removed "codex" agent references model "gpt-5.2-codex" which also does not exist. This removal is appropriate since the model name is invalid.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| npm_config_proxy= \ | ||
| npm_config_https_proxy= \ | ||
| npx -y axrun@latest --agent ${{ matrix.agent }} \ | ||
| ${{ matrix.provider && format('--provider "{0}"', matrix.provider) || '' }} \ |
There was a problem hiding this comment.
The npx command includes a conditional provider parameter that will always evaluate to empty since no matrix entry defines a "provider" field. This is the same pattern as in pr-review-code.yml. If provider configuration is not currently needed, consider removing this conditional to simplify the command.
| ${{ matrix.provider && format('--provider "{0}"', matrix.provider) || '' }} \ |
Jercik
left a comment
Jercik
left a comment
There was a problem hiding this comment.
PR Review Details
Main concern: These workflows run with repository secrets available (workflow_dispatch; pull_request_target for non-fork PRs) and now grant the AI runner unrestricted command execution (AXRUN_ALLOW: bash:*). That combination materially increases the blast radius of prompt-injection / tool misuse and makes secret exfiltration much easier.
Good: Keeping the prompt embedded in the workflow (not in PR-controlled code) and blocking forked PRs on pull_request_target reduces risk for the PR-triggered path.
Summary: Found 2 critical issues and 1 info-level observation.
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 expanded to bash:* (line 12) while the job exports secrets to the agent process (AXVAULT/PERPLEXITY_API_KEY, lines 65–66) and runs on pull_request_target (line 7). This makes secret exfiltration via prompt-injection / agent misbehavior dramatically easier because any command (e.g. curl, node -e, etc.) becomes permitted.
Recommendation: revert to a tight allowlist (only the specific gh, git, cat, rg/grep, jq, etc. commands you expect) and/or ensure the agent runs with no repo secrets.
Ref: GitHub docs on pull_request_target security considerations: https://docs.github.com/actions/using-workflows/events-that-trigger-workflows#pull_request_target
|
|
||
| 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 guard allows workflow_dispatch unconditionally (if: github.event_name == 'workflow_dispatch' || ..., line 18). Since repository secrets are available to workflow_dispatch runs, a maintainer could accidentally dispatch this workflow for a fork PR and then it will checkout attacker-controlled PR head SHA (lines 53–60) while exposing secrets to the review runner (lines 79–81).
Recommendation: apply the same fork restriction for the dispatch path (e.g. after resolving PR number, check head.repo.fork via gh pr view ... and fail fast if true), so manual runs can’t be pointed at untrusted forks.
| 1. **Inline comments** on specific lines (via GitHub Reviews API) | ||
| 2. **A summary comment** with the overall assessment | ||
| **Understand the codebase first, then review the diff.** The diff shows _what_ changed, but you need codebase context to understand _why_ it matters. Explore related files, check how changed code is used elsewhere, and verify the changes fit the existing architecture. |
There was a problem hiding this comment.
ℹ️ Info: Good addition calling out ‘Understand the codebase first, then review the diff’ (line 82). That guidance should reduce context-free, low-signal review comments.
|
🎉 This PR is included in version 1.1.0 🎉 The release is available on: Your semantic-release bot 📦🚀 |
Summary
pr-review-axrun.ymltopr-review-code.yml(detailed code review)pr-review-approach.yml(high-level approach review)Synced from review-tests repository.