[CI] Add issue/PR pytest command workflow - #843
Merged
Conversation
Contributor
|
Note Gemini is unable to generate a review for this pull request due to the file types involved not being currently supported. |
Contributor
WalkthroughAdds a GitHub Actions workflow that runs pytest when an issue comment begins with Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant User as "User (Issue Comment)"
participant GH as "GitHub Actions"
participant Runner as "Self-hosted GPU Runner"
participant Conda as "Local Conda"
participant Pytest as "pytest process"
participant Artifact as "Artifact Storage"
participant Issue as "Issue Comments (API)"
rect rgba(200,200,255,0.5)
User->>GH: create `issue_comment` starting with `pytest <args>`
GH->>GH: validate author association & extract args
GH->>User: add 👀 reaction
end
rect rgba(200,255,200,0.5)
GH->>Runner: dispatch job (checkout PR/head or default)
Runner->>Conda: probe common dirs for `conda` executable
Conda-->>Runner: export paths/env vars
Runner->>Runner: uninstall / install packages, pip install project
end
rect rgba(255,200,200,0.5)
Runner->>Pytest: run `pytest <args>`, capture stdout/stderr -> `pytest_output.txt`
Pytest-->>Runner: exit code & status
Runner->>Artifact: upload `pytest_output.txt`
end
rect rgba(255,255,200,0.5)
GH->>Artifact: download artifact (post job)
GH->>Issue: post comment with command, exit code, logs link, and truncated output
Issue-->>User: visible result comment
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (1)
.github/workflows/issue-pytest-command.yml (1)
54-54: actionlint: unknown runner label.
actionlintflagsnvidia-h100-pt2-7as unknown. If it's a custom self-hosted label, suppress the warning by adding anactionlint.yamlwith the allowed labels so CI stays clean and future workflows don't get spurious errors.# .github/actionlint.yaml self-hosted-runner: labels: - nvidia-h100-pt2-7 - nvidia-h100-1 - nvidia-h100-2🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/issue-pytest-command.yml at line 54, actionlint flags the custom runner label "nvidia-h100-pt2-7" as unknown; add an actionlint config to whitelist your self-hosted labels by creating .github/actionlint.yaml and under the "self-hosted-runner" -> "labels" list include "nvidia-h100-pt2-7" (and other valid labels like "nvidia-h100-1", "nvidia-h100-2") so actionlint stops reporting the unknown runner for the runs-on: nvidia-h100-pt2-7 entry in the workflow.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.github/workflows/issue-pytest-command.yml:
- Around line 52-54: The run-pytest job (job id "run-pytest") lacks a timeout
and can hang the self-hosted H100 runner; add a timeout by including a
timeout-minutes value on the job (e.g., timeout-minutes: 60) or set per-step
timeouts for the pytest step so the job will be forcibly canceled after the
configured number of minutes to prevent indefinite blocking.
- Around line 28-36: The ARGS assignment currently uses `${COMMENT_BODY#pytest
}` which preserves everything after the prefix including newlineed text; change
it to extract only the first line of COMMENT_BODY before removing the `pytest `
prefix so multi-line comments don't leak into pytest args (e.g. compute
FIRST_LINE from COMMENT_BODY using shell first-line extraction like parameter
expansion `${COMMENT_BODY%%$'\n'*}` or `printf '%s\n' "$COMMENT_BODY" | sed -n
'1p'`, then set ARGS="${FIRST_LINE#pytest }" and continue writing ARGS to
GITHUB_OUTPUT as before).
- Around line 54-87: The conditional branches in the find_conda step never match
because runs-on uses "nvidia-h100-pt2-7" while the script compares runner.name
to "nvidia-h100-1" / "nvidia-h100-2", leaving TARGET_CONDA_ENV at its default;
update the runner name comparisons (used in the find_conda shell step and
referencing runner.name) to match the actual runner labels (or change runs-on to
the labels you intended), or alternatively make TARGET_CONDA_ENV selection
driven by an input/label so the logic in find_conda correctly sets
TARGET_CONDA_ENV for your real runners.
- Around line 136-173: The post-result job and the upload step must run
regardless of earlier failures and handle a missing artifact: add if: always()
to the post-result job, add if: always() to the "Upload pytest output" step (and
set the upload action input if-no-files-found: warn or ignore so it won’t fail
if pytest_output.txt is absent), and add if: always() to the "Download pytest
output" step; then make the GitHub Script step (the script in the "Post comment"
step) check for existence of pytest_output.txt (fs.existsSync) and use a short
fallback message when the file is missing before reading/attaching it to the
comment.
- Around line 152-172: The script is vulnerable to injection by interpolating
Actions outputs directly into the inline JS; instead, pass values like
needs.parse.outputs.pytest_args, needs.run-pytest.outputs.status,
needs.run-pytest.outputs.exit_code, and needs.parse.outputs.issue_number into
the step via env: and read them inside the github-script using process.env
(e.g., const args = process.env.PYTEST_ARGS), then build and sanitize output
(respecting MAX_LEN) and call github.rest.issues.createComment with the safe
body; update references in the script for status, exitCode, args, and
issue_number to use the env vars and ensure any backticks/newlines are handled
or escaped before interpolation into body.
---
Nitpick comments:
In @.github/workflows/issue-pytest-command.yml:
- Line 54: actionlint flags the custom runner label "nvidia-h100-pt2-7" as
unknown; add an actionlint config to whitelist your self-hosted labels by
creating .github/actionlint.yaml and under the "self-hosted-runner" -> "labels"
list include "nvidia-h100-pt2-7" (and other valid labels like "nvidia-h100-1",
"nvidia-h100-2") so actionlint stops reporting the unknown runner for the
runs-on: nvidia-h100-pt2-7 entry in the workflow.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 1876a7ff-60e0-4865-aae6-a678dcb014e2
📒 Files selected for processing (1)
.github/workflows/issue-pytest-command.yml
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
.github/workflows/issue-pytest-command.yml (2)
43-51: Consistency nit: passcomment_idviaenv:likepost-resultdoes.
post-resultwas hardened to read outputs fromprocess.envto avoid expression-context injection into the inline JS. This step still interpolates${{ needs.parse.outputs.comment_id }}straight into the script body. It's a number today so it's safe, but for consistency with the security-hardening pattern elsewhere in this file, consider the same treatment here. Also considerawait-ing the call so failures surface as step failures instead of unhandled-rejection warnings.♻️ Proposed refactor
- uses: actions/github-script@v7 + env: + COMMENT_ID: ${{ needs.parse.outputs.comment_id }} with: script: | - github.rest.reactions.createForIssueComment({ + await github.rest.reactions.createForIssueComment({ owner: context.repo.owner, repo: context.repo.repo, - comment_id: ${{ needs.parse.outputs.comment_id }}, + comment_id: Number(process.env.COMMENT_ID), content: 'eyes', });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/issue-pytest-command.yml around lines 43 - 51, Change the inline github-script step so it reads the comment id from an environment variable (e.g. process.env.COMMENT_ID) instead of interpolating ${{ needs.parse.outputs.comment_id }} directly into the script, and await the call to github.rest.reactions.createForIssueComment so errors become step failures; update the step to set env: COMMENT_ID: ${{ needs.parse.outputs.comment_id }} and use await github.rest.reactions.createForIssueComment({ owner: context.repo.owner, repo: context.repo.repo, comment_id: Number(process.env.COMMENT_ID), content: 'eyes' }) inside the script.
182-182: Output formatting: the template literal will render unwanted backslashes in the markdown comment.The
\\`` sequences will parse successfully in the template literal (no SyntaxError), but they produce the literal two-character string`in the output instead of just a backtick. For code fences and other markdown backticks, this results in escaped backticks like`rather than clean ```` in the rendered comment.Use a single ``` to escape backticks, which will produce only the backtick character in the output. Alternatively, extract the fence pattern into a variable to avoid scattered escape sequences and improve readability:
🔧 Proposed fix
- const body = `## ${status} — Pytest Results (H100 PyTorch 2.7)\n\n**Command:** \\`pytest ${args}\\`\n**Exit code:** ${exitCode}\n\n[View workflow run logs](${runUrl})\n\n<details><summary>Click to expand pytest output</summary>\n\n\\`\\`\\`\n${output}\n\\`\\`\\`\n\n</details>`; + const fence = '```'; + const body = [ + `## ${status} — Pytest Results (H100 PyTorch 2.7)`, + ``, + `**Command:** \`pytest ${args}\``, + `**Exit code:** ${exitCode}`, + ``, + `[View workflow run logs](${runUrl})`, + ``, + `<details><summary>Click to expand pytest output</summary>`, + ``, + fence, + output, + fence, + ``, + `</details>`, + ].join('\n');🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/issue-pytest-command.yml at line 182, The template literal assigned to body currently uses double-escaped backticks (e.g., \\`) which produce literal backslashes in the rendered markdown; update the construction of body (variable body) so markdown fences and inline backticks are inserted as real backticks — either replace the double-escaped sequences with single-escaped backticks (e.g., `\`` for inline code) or refactor to build body as an array of lines and join('\n') using a fence variable (e.g., fence) and the existing symbols status, args, exitCode, runUrl, output to ensure code fences and inline code render correctly without extra backslashes.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.github/workflows/issue-pytest-command.yml:
- Around line 63-118: The workflow currently checks out refs/pull/.../head and
runs $CONDA_BIN_PATH/pip install . and pytest in the "Install/Update
Dependencies" step after the "Check out repo" step (steps/ref and find_conda),
which allows arbitrary code execution from untrusted PR authors; update the
gating logic in the parse step so it also validates the PR author's trust
(author_association) or requires a maintainer-applied label before allowing
is_pr=true, and then change the run-pytest job to use a safe ref (merge/ref or
the default branch) or run inside an ephemeral sandbox/container and skip
$CONDA_BIN_PATH/pip install . for untrusted PRs (or only install from a pinned
build artifact), and ensure the job's conditional uses the new trusted PR gate
instead of the current commenter-only gate so "Install/Update Dependencies",
pytest collection, and any build backend execution are never performed on
attacker-controlled heads.
---
Nitpick comments:
In @.github/workflows/issue-pytest-command.yml:
- Around line 43-51: Change the inline github-script step so it reads the
comment id from an environment variable (e.g. process.env.COMMENT_ID) instead of
interpolating ${{ needs.parse.outputs.comment_id }} directly into the script,
and await the call to github.rest.reactions.createForIssueComment so errors
become step failures; update the step to set env: COMMENT_ID: ${{
needs.parse.outputs.comment_id }} and use await
github.rest.reactions.createForIssueComment({ owner: context.repo.owner, repo:
context.repo.repo, comment_id: Number(process.env.COMMENT_ID), content: 'eyes'
}) inside the script.
- Line 182: The template literal assigned to body currently uses double-escaped
backticks (e.g., \\`) which produce literal backslashes in the rendered
markdown; update the construction of body (variable body) so markdown fences and
inline backticks are inserted as real backticks — either replace the
double-escaped sequences with single-escaped backticks (e.g., `\`` for inline
code) or refactor to build body as an array of lines and join('\n') using a
fence variable (e.g., fence) and the existing symbols status, args, exitCode,
runUrl, output to ensure code fences and inline code render correctly without
extra backslashes.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: fe51b7b9-e52d-4cbd-ba94-63c32580965b
📒 Files selected for processing (2)
.github/actionlint.yaml.github/workflows/issue-pytest-command.yml
✅ Files skipped from review due to trivial changes (1)
- .github/actionlint.yaml
Comment on lines
+63
to
+118
| - name: Determine checkout ref | ||
| id: ref | ||
| run: | | ||
| if [ "${{ needs.parse.outputs.is_pr }}" = "true" ]; then | ||
| echo "ref=refs/pull/${{ needs.parse.outputs.issue_number }}/head" >> $GITHUB_OUTPUT | ||
| else | ||
| echo "ref=${{ github.event.repository.default_branch }}" >> $GITHUB_OUTPUT | ||
| fi | ||
|
|
||
| - name: Check out repo | ||
| uses: actions/checkout@v4 | ||
| with: | ||
| ref: ${{ steps.ref.outputs.ref }} | ||
|
|
||
| - name: Discover Conda Path and Set Env Vars | ||
| id: find_conda | ||
| shell: bash | ||
| run: | | ||
| set -e | ||
| TARGET_CONDA_ENV="pytorch_2_7" | ||
| echo "Determining conda environment based on runner: ${{ runner.name }}" | ||
| case "${{ runner.name }}" in | ||
| nvidia-h100-pt2-7|nvidia-h100-1) TARGET_CONDA_ENV="pytorch_2_7" ;; | ||
| nvidia-h100-2) TARGET_CONDA_ENV="pytorch_2_7_1" ;; | ||
| esac | ||
| echo "--> Runner is '${{ runner.name }}', selected environment is '${TARGET_CONDA_ENV}'" | ||
|
|
||
| echo "Searching for Conda installation in home directory ($HOME)..." | ||
| POSSIBLE_NAMES=("miniforge3" "miniconda3" "anaconda3") | ||
| FOUND_PATH="" | ||
| for name in "${POSSIBLE_NAMES[@]}"; do | ||
| CANDIDATE_PATH="$HOME/$name" | ||
| echo "--> Checking for path: ${CANDIDATE_PATH}" | ||
| if [ -d "${CANDIDATE_PATH}" ] && [ -x "${CANDIDATE_PATH}/bin/conda" ]; then | ||
| echo " Found valid Conda installation: ${CANDIDATE_PATH}" | ||
| FOUND_PATH="${CANDIDATE_PATH}" | ||
| break | ||
| fi | ||
| done | ||
|
|
||
| if [ -n "${FOUND_PATH}" ]; then | ||
| echo "Setting CONDA environment variable to: ${FOUND_PATH}" | ||
| echo "CONDA=${FOUND_PATH}" >> $GITHUB_ENV | ||
| echo "CONDA_BIN_PATH=${FOUND_PATH}/envs/${TARGET_CONDA_ENV}/bin" >> $GITHUB_ENV | ||
| echo "CONDA_ENV_NAME=${TARGET_CONDA_ENV}" >> $GITHUB_ENV | ||
| else | ||
| echo "::error::Could not automatically find a Conda installation." | ||
| exit 1 | ||
| fi | ||
|
|
||
| - name: Install/Update Dependencies | ||
| shell: bash | ||
| run: | | ||
| $CONDA_BIN_PATH/pip uninstall -y flash-linear-attention | ||
| $CONDA_BIN_PATH/pip install -U pytest setuptools wheel ninja | ||
| $CONDA_BIN_PATH/pip install . |
Contributor
There was a problem hiding this comment.
Critical: arbitrary code execution on self-hosted GPU runner from untrusted PR heads.
When is_pr is true, this job checks out refs/pull/N/head (attacker-controlled code from the PR author) and then runs pip install . on a persistent self-hosted H100 runner. pip install . executes setup.py / pyproject.toml build backends, and pytest collection runs conftest.py — both are arbitrary code execution vectors. The trust gate on line 19 only validates the commenter's author_association, not the PR author's. Any external contributor can open a malicious PR, wait for any OWNER/MEMBER/COLLABORATOR to type pytest ..., and then compromise the runner (secrets exfiltration, persistent backdoor, crypto-mining on the H100, etc.). This is the well-known GitHub "pwn-request" pattern, and it's especially dangerous on self-hosted runners where filesystem/caches persist across jobs.
At minimum, also require the PR author to be trusted, or require a maintainer-applied label as an additional gate. Ideally the GPU job should also run in an ephemeral container.
🔒 Suggested hardening (PR-author trust gate in parse)
parse:
if: |
contains(fromJSON('["OWNER", "MEMBER", "COLLABORATOR"]'), github.event.comment.author_association) &&
startsWith(github.event.comment.body, 'pytest ')
runs-on: ubuntu-latest
outputs:
pytest_args: ${{ steps.parse.outputs.pytest_args }}
issue_number: ${{ github.event.issue.number }}
is_pr: ${{ github.event.issue.pull_request != null }}
comment_id: ${{ github.event.comment.id }}
+ pr_trusted: ${{ steps.trust.outputs.trusted }}
steps:
- id: parse
...
+ - id: trust
+ if: ${{ github.event.issue.pull_request != null }}
+ env:
+ GH_TOKEN: ${{ secrets.GITHUB_TOKEN }}
+ run: |
+ ASSOC=$(gh api "repos/${{ github.repository }}/pulls/${{ github.event.issue.number }}" --jq '.author_association')
+ case "$ASSOC" in
+ OWNER|MEMBER|COLLABORATOR) echo "trusted=true" >> "$GITHUB_OUTPUT" ;;
+ *) echo "trusted=false" >> "$GITHUB_OUTPUT" ;;
+ esacAnd gate run-pytest on it:
run-pytest:
needs: parse
+ if: needs.parse.outputs.is_pr != 'true' || needs.parse.outputs.pr_trusted == 'true'
runs-on: nvidia-h100-pt2-7🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.github/workflows/issue-pytest-command.yml around lines 63 - 118, The
workflow currently checks out refs/pull/.../head and runs $CONDA_BIN_PATH/pip
install . and pytest in the "Install/Update Dependencies" step after the "Check
out repo" step (steps/ref and find_conda), which allows arbitrary code execution
from untrusted PR authors; update the gating logic in the parse step so it also
validates the PR author's trust (author_association) or requires a
maintainer-applied label before allowing is_pr=true, and then change the
run-pytest job to use a safe ref (merge/ref or the default branch) or run inside
an ephemeral sandbox/container and skip $CONDA_BIN_PATH/pip install . for
untrusted PRs (or only install from a pinned build artifact), and ensure the
job's conditional uses the new trusted PR gate instead of the current
commenter-only gate so "Install/Update Dependencies", pytest collection, and any
build backend execution are never performed on attacker-controlled heads.
This was referenced Aug 7, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary by CodeRabbit