FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

Clarify commit message from pr-auto-commit by moreal · Pull Request #6706 · RustPython/RustPython · GitHub

/ RustPython Public

Clarify commit message from pr-auto-commit #6706

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.

Sign up for GitHub

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

Merged
youknowone merged 2 commits into from
Jan 11, 2026
Conversation 12 Commits 2 Checks 12 Files changed
Merged

Clarify commit message from pr-auto-commit #6706

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
a5a6ca1
Clarify commit message from pr-auto-commit
moreal Jan 11, 2026
0ec2add
Auto-format: cargo fmt --all
github-actions[bot] Jan 11, 2026
File filter

Filter by extension

Filter by extension .py  (1) .yaml  (1) All 2 file types selected
Only manifest files
Viewed files
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Unified
Split
Hide whitespace
Diff view
Unified
Split
Hide whitespace
58 changes: 47 additions & 11 deletions .github/workflows/pr-auto-commit.yaml
Show comments
View file
Open in desktop

ShaharNaveh Jan 11, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Can you please adjust the message at line 103?

moreal Jan 11, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Oh, I missed it. I fixed it! Thanks for letting me know it 🙏🏻
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
Show hidden characters
Original file line number Diff line number Diff line change
Expand Up @@ -34,42 +34,77 @@ jobs:
with:
components: rustfmt

- name: Configure git
run: |
git config user.name "github-actions[bot]"
git config user.email "github-actions[bot]@users.noreply.github.com"
echo "" > /tmp/committed_commands.txt

Comment on lines +37 to +42

coderabbitai Bot Jan 11, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

⚠️ Potential issue | 🟡 Minor

Avoid writing a leading blank line to committed_commands.txt.
echo "" > ... creates an initial empty line, which will show up in the PR comment output.

Proposed fix 
       - name: Configure git
         run: |
           git config user.name "github-actions[bot]"
           git config user.email "github-actions[bot]@users.noreply.github.com"
-          echo "" > /tmp/committed_commands.txt
+          : > /tmp/committed_commands.txt
🤖 Prompt for AI Agents 
In @.github/workflows/pr-auto-commit.yaml around lines 37 - 42, The workflow
currently uses echo "" to initialize /tmp/committed_commands.txt which writes a
leading blank line; replace that echo call with a true truncation/no-op
redirection so the file is empty without a newline (e.g., use a plain shell
redirection or printf with an empty string) and keep the file name
/tmp/committed_commands.txt unchanged.

- name: Run cargo fmt
run: |
echo "Running cargo fmt --all on PR #${{ github.event.pull_request.number }}"
cargo fmt --all
if [ -n "$(git status --porcelain)" ]; then
git add -u
git commit -m "Auto-format: cargo fmt --all"
echo "- \`cargo fmt --all\`" >> /tmp/committed_commands.txt
fi

- name: Install ruff
uses: astral-sh/ruff-action@57714a7c8a2e59f32539362ba31877a1957dded1 # v3.5.1
with:
version: "0.14.9"
args: "--version"

- run: ruff format
- run: ruff check --select I --fix
- run: python scripts/generate_opcode_metadata.py
- name: Run ruff format
run: |
ruff format
if [ -n "$(git status --porcelain)" ]; then
git add -u
git commit -m "Auto-format: ruff format"
echo "- \`ruff format\`" >> /tmp/committed_commands.txt
fi

- name: Configure git
- name: Run ruff check import sorting
run: |
git config user.name "github-actions[bot]"
git config user.email "github-actions[bot]@users.noreply.github.com"
ruff check --select I --fix
if [ -n "$(git status --porcelain)" ]; then
git add -u
git commit -m "Auto-format: ruff check --select I --fix"
echo "- \`ruff check --select I --fix\`" >> /tmp/committed_commands.txt
fi

- name: Run generate_opcode_metadata.py
run: |
python scripts/generate_opcode_metadata.py
if [ -n "$(git status --porcelain)" ]; then
git add -u
git commit -m "Auto-generate: generate_opcode_metadata.py"
echo "- \`python scripts/generate_opcode_metadata.py\`" >> /tmp/committed_commands.txt
fi

Comment on lines 43 to 85

coderabbitai Bot Jan 11, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

⚠️ Potential issue | 🟠 Major

Fix conditional-commit staging: git add -u can make git commit fail.
You gate on git status --porcelain (which includes untracked files), but then stage with git add -u (which excludes untracked). If the only changes are new files, git commit fails and the job aborts.

Proposed fix (apply to all 4 commit blocks) 
       - name: Run cargo fmt
         run: |
           echo "Running cargo fmt --all on PR #${{ github.event.pull_request.number }}"
           cargo fmt --all
           if [ -n "$(git status --porcelain)" ]; then
-            git add -u
-            git commit -m "Auto-format: cargo fmt --all"
+            git add -A
+            if ! git diff --cached --quiet; then
+              git commit -m "Auto-format: cargo fmt --all"
+            fi
             echo "- \`cargo fmt --all\`" >> /tmp/committed_commands.txt
           fi
       - name: Run ruff format
         run: |
           ruff format
           if [ -n "$(git status --porcelain)" ]; then
-            git add -u
-            git commit -m "Auto-format: ruff format"
+            git add -A
+            if ! git diff --cached --quiet; then
+              git commit -m "Auto-format: ruff format"
+            fi
             echo "- \`ruff format\`" >> /tmp/committed_commands.txt
           fi
       - name: Run ruff check import sorting
         run: |
           ruff check --select I --fix
           if [ -n "$(git status --porcelain)" ]; then
-            git add -u
-            git commit -m "Auto-format: ruff check --select I --fix"
+            git add -A
+            if ! git diff --cached --quiet; then
+              git commit -m "Auto-format: ruff check --select I --fix"
+            fi
             echo "- \`ruff check --select I --fix\`" >> /tmp/committed_commands.txt
           fi
       - name: Run generate_opcode_metadata.py
         run: |
           python scripts/generate_opcode_metadata.py
           if [ -n "$(git status --porcelain)" ]; then
-            git add -u
-            git commit -m "Auto-generate: generate_opcode_metadata.py"
+            git add -A
+            if ! git diff --cached --quiet; then
+              git commit -m "Auto-generate: generate_opcode_metadata.py"
+            fi
             echo "- \`python scripts/generate_opcode_metadata.py\`" >> /tmp/committed_commands.txt
           fi
🤖 Prompt for AI Agents 
In @.github/workflows/pr-auto-commit.yaml around lines 43 - 85, The commit steps
currently stage with git add -u (which ignores untracked files) but gate on git
status --porcelain (which sees untracked), causing git commit to fail when only
new files exist; for each of the four commit blocks (the cargo fmt, ruff format,
ruff check --select I --fix, and generate_opcode_metadata.py blocks) replace git
add -u with git add -A so untracked files are included before committing,
ensuring git commit succeeds when new files were created.

Comment on lines +77 to 85

coderabbitai Bot Jan 11, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

⚠️ Potential issue | 🔴 Critical

Critical: pull_request_target + running a PR-controlled Python script can exfiltrate AUTO_COMMIT_PAT.
Because you check out the PR’s head SHA and then run python scripts/generate_opcode_metadata.py, a fork PR can modify that script and execute arbitrary code with access to secrets.

Minimal mitigation (keeps the workflow simple): skip this step for fork PRs.

Proposed fix 
       - name: Run generate_opcode_metadata.py
+        if: ${{ github.event.pull_request.head.repo.full_name == github.repository }}
         run: |
           python scripts/generate_opcode_metadata.py
           if [ -n "$(git status --porcelain)" ]; then
             git add -u
             git commit -m "Auto-generate: generate_opcode_metadata.py"
             echo "- \`python scripts/generate_opcode_metadata.py\`" >> /tmp/committed_commands.txt
           fi
🤖 Prompt for AI Agents 
In @.github/workflows/pr-auto-commit.yaml around lines 77 - 85, The "Run
generate_opcode_metadata.py" step runs a PR-controlled script and can exfiltrate
AUTO_COMMIT_PAT for forked PRs; guard that step so it does not run for forked
repositories by adding a condition that checks the PR head repo is not a fork
(e.g., use github.event.pull_request.head.repo.fork == false) before executing
python scripts/generate_opcode_metadata.py and the git commit logic, leaving the
step intact for non-fork PRs only.

- name: Check for changes
id: check-changes
run: |
if [ -n "$(git status --porcelain)" ]; then
if [ "$(git rev-parse HEAD)" != "${{ github.event.pull_request.head.sha }}" ]; then
echo "has_changes=true" >> $GITHUB_OUTPUT
else
echo "has_changes=false" >> $GITHUB_OUTPUT
fi

- name: Commit and push formatting changes
- name: Push formatting changes
if: steps.check-changes.outputs.has_changes == 'true'
run: |
git add -u
git commit -m "Auto-format: cargo fmt --all"
git push origin HEAD:${{ github.event.pull_request.head.ref }}

Comment on lines +95 to 99

coderabbitai Bot Jan 11, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

⚠️ Potential issue | 🟠 Major

Fix potential script-injection via ${{ github.event.pull_request.head.ref }} (actionlint finding).
Pass via env and quote the ref in the git push.

Proposed fix 
       - name: Push formatting changes
         if: steps.check-changes.outputs.has_changes == 'true'
+        env:
+          PR_HEAD_REF: ${{ github.event.pull_request.head.ref }}
         run: |
-          git push origin HEAD:${{ github.event.pull_request.head.ref }}
+          git push origin "HEAD:refs/heads/${PR_HEAD_REF}"
📝 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: Push formatting changes
if: steps.check-changes.outputs.has_changes == 'true'
run: |
git add -u
git commit -m "Auto-format: cargo fmt --all"
git push origin HEAD:${{ github.event.pull_request.head.ref }}
- name: Push formatting changes
if: steps.check-changes.outputs.has_changes == 'true'
env:
PR_HEAD_REF: ${{ github.event.pull_request.head.ref }}
run: |
git push origin "HEAD:refs/heads/${PR_HEAD_REF}"
🧰 Tools 🪛 actionlint (1.7.10)

97-97: "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 
In @.github/workflows/pr-auto-commit.yaml around lines 95 - 99, The git push
step uses an unquoted expression `${{ github.event.pull_request.head.ref }}`
which actionlint flags for possible script injection; fix by passing the PR ref
via an environment variable and quoting it in the push command: set an env key
(e.g., PR_REF) to `${{ github.event.pull_request.head.ref }}` on the "Push
formatting changes" step and replace the direct expression in the run command
with a quoted reference to that env var (e.g., git push origin
HEAD:"${PR_REF}"), ensuring the ref is properly quoted to prevent shell
injection.

- name: Read committed commands
id: committed-commands
if: steps.check-changes.outputs.has_changes == 'true'
run: |
echo "list<<EOF" >> $GITHUB_OUTPUT
cat /tmp/committed_commands.txt >> $GITHUB_OUTPUT
echo "EOF" >> $GITHUB_OUTPUT

- name: Comment on PR
if: steps.check-changes.outputs.has_changes == 'true'
uses: marocchino/sticky-pull-request-comment@v2
Expand All @@ -78,7 +113,8 @@ jobs:
message: |
**Code has been automatically formatted**

The code in this PR has been formatted using `cargo fmt --all`.
The code in this PR has been formatted using:
${{ steps.committed-commands.outputs.list }}
Please pull the latest changes before pushing again:
```bash
git pull origin ${{ github.event.pull_request.head.ref }}
Expand Down
2 changes: 1 addition & 1 deletion scripts/fix_test.py
Show comments
View file
Open in desktop
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
Show hidden characters
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@
import sys
from pathlib import Path

from lib_updater import apply_patches, PatchSpec, UtMethod
from lib_updater import PatchSpec, UtMethod, apply_patches


def parse_args():
Expand Down
Loading
Back | FazBrowse Home | New Git URL