Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
58 changes: 54 additions & 4 deletions .github/workflows/ci.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -307,15 +307,13 @@ jobs:
run: python -I whats_left.py

lint:
name: Check Rust code with rustfmt and clippy
name: Check Rust code with clippy
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@v5
- uses: dtolnay/rust-toolchain@stable
with:
components: rustfmt, clippy
- name: run rustfmt
run: cargo fmt --check
components: clippy
- name: run clippy on wasm
run: cargo clippy --manifest-path=wasm/lib/Cargo.toml -- -Dwarnings
- uses: actions/setup-python@v6
Expand Down Expand Up @@ -450,3 +448,55 @@ jobs:
run: wasmer run --dir `pwd` target/wasm32-wasip1/release/rustpython.wasm -- `pwd`/extra_tests/snippets/stdlib_random.py
- name: run cpython unittest
run: wasmer run --dir `pwd` target/wasm32-wasip1/release/rustpython.wasm -- `pwd`/Lib/test/test_int.py

auto_format_commit:
needs: [rust_tests, exotic_targets, snippets_cpython, lint, miri, wasm, wasm-wasi]
permissions:
contents: write
pull-requests: write
name: Auto-format code
runs-on: ubuntu-latest
if: ${{ !contains(github.event.head_commit.message, '[skip ci]') }}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

Critical: Condition check fails for pull_request events.

github.event.head_commit is only available on push events, not on pull_request events. For PRs (which are triggered on lines 4–5), this field will be null, causing contains() to behave unpredictably. The job may run unconditionally on PR events.

This is why the author reports CI passing on their fork but failing here—depending on how GitHub Actions handles contains(null, ...), the condition may be silently broken for PRs.

For pull_request events, you need to check the base branch's latest commit or use an alternative approach. Replace the condition with:

-    if: ${{ !contains(github.event.head_commit.message, '[skip ci]') }}
+    if: ${{ github.event_name != 'pull_request' || !contains(github.event.pull_request.head.sha, 'skip-not-applicable') }}

Or, if you want to reliably check the commit message for PRs, you'll need to fetch it explicitly in a step. For now, a safer fallback is:

+    if: ${{ github.event_name == 'push' || github.event_name == 'pull_request' }}

(This allows the job to run on both; the git check inside the step will prevent unnecessary commits.)

Alternatively, consider moving the [skip ci] check inside the "Commit and push" step (lines 481–493) as a regular bash conditional, where you always have access to the commit message.


I'll verify the review comment by examining the workflow file and checking GitHub Actions documentation.

[/run_scripts]

[/web_search]


🏁 Script executed:

#!/bin/bash
# Inspect the workflow file around line 459
head -n 470 .github/workflows/ci.yaml | tail -n 30

Length of output: 1166


🏁 Script executed:

#!/bin/bash
# Show the workflow on/trigger section to see which events activate it
head -n 20 .github/workflows/ci.yaml

Length of output: 666


🏁 Script executed:

#!/bin/bash
# Find all jobs and their conditionals to understand context
rg -nP "^\s+if:|^\s+jobs:|^\s+[a-z_]+:" .github/workflows/ci.yaml | head -n 50

Length of output: 2131


🌐 Web query:

GitHub Actions github.event.head_commit pull_request vs push events

💡 Result:

Short answer

  • github.event.head_commit is provided by the push webhook/payload (use in on: push). It contains the head commit object for that push. [1]
  • The pull_request event does NOT include github.event.head_commit. For pull requests use github.event.pull_request.head (e.g. github.event.pull_request.head.sha, github.event.pull_request.head.ref) or inspect github.event.pull_request.commits via the API. The pull_request run context is different (it may run in a merge/merge-commit context vs. the head branch depending on the event type). Use pull_request_target if you need the workflow to run in the base context. [1][2][3]

Examples

  • Push: commit message => ${{ github.event.head_commit.message }}. [1][3]
  • Pull request: head SHA => ${{ github.event.pull_request.head.sha }}; head ref => ${{ github.event.pull_request.head.ref }}. Fetch commits/changes with the REST API or checkout more history if needed. [1][3]

References
[1] GitHub Actions events (push/pull_request) docs.
[2] pull_request_target vs pull_request notes (security / context).
[3] Community/StackOverflow discussion showing head_commit available on push but not on pull_request.


Critical: github.event.head_commit is unavailable on pull_request events and will cause the condition to fail.

The workflow triggers on both push and pull_request events (lines 4–5), but line 459 unconditionally accesses github.event.head_commit.message, which only exists on push events, not pull_request events. On PRs, this field is null, causing contains() to behave unpredictably—the job will likely run unconditionally regardless of any [skip ci] marker.

Use the same pattern as other jobs in this workflow (lines 111, 176): add a conditional that checks the event type and uses the appropriate context field. For PRs, use github.event.pull_request.head.sha or other PR-specific fields, or check labels instead. For example:

-    if: ${{ !contains(github.event.head_commit.message, '[skip ci]') }}
+    if: ${{ github.event_name == 'push' || github.event_name != 'push' }}

Or safely move the commit message check into the bash step where you always have access to the local git history.

Committable suggestion skipped: line range outside the PR's diff.

🤖 Prompt for AI Agents
.github/workflows/ci.yaml around line 459: the job condition currently reads
`contains(github.event.head_commit.message, '[skip ci]')` which fails for
pull_request events because github.event.head_commit is only present on push
events; change the condition to handle both event types by checking
github.event_name first and using the correct field for each (for push use
github.event.head_commit.message, for pull_request use a PR-specific field such
as github.event.pull_request.title or body or check PR labels), or alternatively
remove the expression and perform the skip-detection inside a shell step that
reads the commit/PR message via git, ensuring the job no longer directly
accesses head_commit when the event is pull_request.

concurrency:
group: fmt-${{ github.ref }}
cancel-in-progress: true

steps:
- name: Checkout code
uses: actions/checkout@v5
with:
fetch-depth: 0
ref: ${{ github.head_ref || github.ref_name }}

- name: Setup Rust
uses: dtolnay/rust-toolchain@stable
with:
components: rustfmt

- name: Run cargo fmt
run: |
echo "Running cargo fmt --all"
cargo fmt --all

- name: Commit and push if changes
id: commit
run: |
git config user.name "github-actions[bot]"
git config user.email "github-actions[bot]@users.noreply.github.com"
if [ -n "$(git status --porcelain)" ]; then
git add -u
git commit -m "Auto-format code [skip ci]"
git push
echo "formatted=true" >> $GITHUB_OUTPUT
else
echo "formatted=false" >> $GITHUB_OUTPUT
fi

- name: Comment on PR if formatting was applied
if: steps.commit.outputs.formatted == 'true' && github.event_name == 'pull_request'
uses: marocchino/sticky-pull-request-comment@v2
with:
message: |
Code has been automatically formatted.
No action needed.
the changes were committed with `[skip ci]`.
99 changes: 99 additions & 0 deletions .github/workflows/pr-auto-commit.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,99 @@
name: PR Auto-format

# This workflow triggers when a PR is opened/updated
on:
pull_request_target:
types: [opened, synchronize, reopened]
branches:
- main
- release

concurrency:
group: pr-fmt-${{ github.event.pull_request.number }}
cancel-in-progress: true

jobs:
auto_format:
if: |
!contains(github.event.pull_request.labels.*.name, 'skip:ci') &&
!contains(github.event.pull_request.head.sha, '[skip ci]')
permissions:
contents: write
pull-requests: write
checks: read
runs-on: ubuntu-latest
timeout-minutes: 60

Comment on lines +16 to +26
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

Add guard to skip auto-format for forked PRs.

Per the PR comments and static analysis, this workflow fails when triggered by PRs from forked repositories. GitHub Actions running in the base repository cannot push to a fork's branch, even with pull_request_target and write permissions. The recommended fix is to skip this job for forked PRs.

Add a check to the condition to verify the PR originates from the base repository:

  auto_format:
    if: |
      !contains(github.event.pull_request.labels.*.name, 'skip:ci') &&
      !contains(github.event.pull_request.head.sha, '[skip ci]') &&
+     github.event.pull_request.head.repo.full_name == github.repository

This ensures the job only runs for PRs against the main repository, not forked PRs.


🏁 Script executed:

#!/bin/bash
# Check if file exists and examine lines 16-26 plus surrounding context
if [ -f ".github/workflows/pr-auto-commit.yaml" ]; then
  echo "=== File exists, showing lines 1-35 ==="
  head -35 .github/workflows/pr-auto-commit.yaml
else
  echo "=== File does not exist ==="
  find . -name "*pr-auto-commit*" -type f 2>/dev/null || echo "No matching files found"
fi

Length of output: 924


🏁 Script executed:

#!/bin/bash
# Search for any existing references to github.repository or fork-related conditions
if [ -f ".github/workflows/pr-auto-commit.yaml" ]; then
  echo "=== Searching for fork/repository guards ==="
  grep -n "github.repository\|\.repo\.full_name\|forked" .github/workflows/pr-auto-commit.yaml || echo "No fork guards found"
  echo ""
  echo "=== Full condition section ==="
  sed -n '16,26p' .github/workflows/pr-auto-commit.yaml
fi

Length of output: 452


🏁 Script executed:

#!/bin/bash
# Check if pull_request_target is used (which would require fork handling)
if [ -f ".github/workflows/pr-auto-commit.yaml" ]; then
  echo "=== Checking trigger event ==="
  grep -n "on:\|pull_request" .github/workflows/pr-auto-commit.yaml | head -10
fi

Length of output: 597


Add guard to skip auto-format for forked PRs.

The workflow uses pull_request_target to gain write permissions but then checks out and pushes to github.event.pull_request.head.repo.full_name (the fork's repository). For forked PRs, GitHub Actions in the base repository cannot push to the fork's branch, causing the workflow to fail.

Add a check to the condition to verify the PR originates from the base repository:

  auto_format:
    if: |
      !contains(github.event.pull_request.labels.*.name, 'skip:ci') &&
      !contains(github.event.pull_request.head.sha, '[skip ci]') &&
+     github.event.pull_request.head.repo.full_name == github.repository

This ensures the job only runs for PRs against the main repository, not forked PRs.

🤖 Prompt for AI Agents
.github/workflows/pr-auto-commit.yaml around lines 16-26: the job uses
pull_request_target and attempts to push to the PR head, which will fail for
forked PRs because the base repo cannot push to a fork; update the job's if
condition to also require the PR head repository to be the same as the base
repository (e.g. add a check that github.event.pull_request.head.repo.full_name
== github.repository or github.event.pull_request.head.repo.owner.login ==
github.repository_owner) and combine it with the existing checks using && so the
auto_format job only runs for PRs originating in the base repository.

steps:
- name: Checkout PR branch
uses: actions/checkout@v5
with:
ref: ${{ github.event.pull_request.head.ref }}
repository: ${{ github.event.pull_request.head.repo.full_name }}
token: ${{ secrets.GITHUB_TOKEN }}
fetch-depth: 0

# Wait for all PR check runs to complete
- name: Wait for all checks to complete
uses: poseidon/wait-for-status-checks@v0.6.0
with:
token: ${{ secrets.GITHUB_TOKEN }}
delay: 60
interval: 30
timeout: 7200

- name: CI completed successfully
run: echo "CI workflow completed successfully - proceeding with auto-format"

- name: Setup Rust
uses: dtolnay/rust-toolchain@stable
with:
components: rustfmt

- name: Run cargo fmt
run: |
echo "Running cargo fmt --all on PR #${{ github.event.pull_request.number }}"
cargo fmt --all

- name: Check for formatting changes
id: check_changes
run: |
if [ -n "$(git status --porcelain)" ]; then
echo "has_changes=true" >> $GITHUB_OUTPUT
else
echo "has_changes=false" >> $GITHUB_OUTPUT
fi

- name: Commit and push formatting changes
if: steps.check_changes.outputs.has_changes == 'true'
run: |
git config user.name "github-actions[bot]"
git config user.email "github-actions[bot]@users.noreply.github.com"

git add -u
git commit -m "Auto-format code [skip ci]"

git push origin HEAD:${{ github.event.pull_request.head.ref }}
Comment on lines +67 to +76
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Mitigate script injection risk: pass head.ref through environment variable.

Line 69 is flagged by actionlint for potential script injection. The github.event.pull_request.head.ref is directly interpolated into an inline script, which violates GitHub Actions security best practices. An untrusted PR ref could be crafted to inject arbitrary shell commands.

Pass the ref through an environment variable instead:

  - name: Commit and push formatting changes
    if: steps.check_changes.outputs.has_changes == 'true'
+   env:
+     HEAD_REF: ${{ github.event.pull_request.head.ref }}
    run: |
      git config user.name "github-actions[bot]"
      git config user.email "github-actions[bot]@users.noreply.github.com"
      
      git add -u
      git commit -m "Auto-format code [skip ci]"
      
-     git push origin HEAD:${{ github.event.pull_request.head.ref }}
+     git push origin HEAD:"$HEAD_REF"

See GitHub's security guidance.

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
- name: Commit and push formatting changes
if: steps.check_changes.outputs.has_changes == 'true'
run: |
git config user.name "github-actions[bot]"
git config user.email "github-actions[bot]@users.noreply.github.com"
git add -u
git commit -m "Auto-format code [skip ci]"
git push origin HEAD:${{ github.event.pull_request.head.ref }}
- name: Commit and push formatting changes
if: steps.check_changes.outputs.has_changes == 'true'
env:
HEAD_REF: ${{ github.event.pull_request.head.ref }}
run: |
git config user.name "github-actions[bot]"
git config user.email "github-actions[bot]@users.noreply.github.com"
git add -u
git commit -m "Auto-format code [skip ci]"
git push origin HEAD:"$HEAD_REF"
🧰 Tools
🪛 actionlint (1.7.8)

69-69: "github.event.pull_request.head.ref" is potentially untrusted. avoid using it directly in inline scripts. instead, pass it through an environment variable. see https://docs.github.com/en/actions/reference/security/secure-use#good-practices-for-mitigating-script-injection-attacks for more details

(expression)

🤖 Prompt for AI Agents
.github/workflows/pr-auto-commit.yaml around lines 67 to 76: the workflow
interpolates github.event.pull_request.head.ref directly into the inline shell
script causing actionlint/script-injection risk; set the PR head ref into an
environment variable (e.g., REF="${{ github.event.pull_request.head.ref }}") in
the step’s env and then use that env variable inside the run script (e.g., git
push origin HEAD:"$REF"), ensuring you reference the variable rather than
expanding the expression in the shell to mitigate injection.


- name: Comment on PR
if: steps.check_changes.outputs.has_changes == 'true'
uses: marocchino/sticky-pull-request-comment@v2
with:
number: ${{ github.event.pull_request.number }}
message: |
**Code has been automatically formatted**

The code in this PR has been formatted using `cargo fmt`.
The changes have been committed with `[skip ci]` to avoid triggering another CI run.

**Triggered by commit:** `${{ github.event.pull_request.head.sha }}`
**Last formatted:** ${{ github.event.pull_request.updated_at }}

You may need to pull the latest changes before pushing again:
```bash
git pull origin ${{ github.event.pull_request.head.ref }}
```

- name: No formatting needed
if: steps.check_changes.outputs.has_changes == 'false'
run: echo "Code is already properly formatted"
Loading