Resolve template injection on comment command#7546
Resolve template injection on comment command#7546youknowone merged 1 commit intoRustPython:mainfrom
Conversation
📝 WalkthroughWalkthroughA GitHub Actions workflow step that assigns issues now retrieves the assignee username via a step-level environment variable instead of direct template interpolation in the JSON payload. The Changes
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~3 minutes Possibly related PRs
Suggested reviewers
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 |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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/comment-commands.yml:
- Around line 21-23: Replace the compile-time interpolation of ${{ env.USER }}
in the curl payload with a runtime-built, JSON-encoded payload using the shell
USER variable; specifically, stop using `${{ env.USER }}` inside the curl -d
string in the curl invocation and instead construct the JSON at runtime (e.g.,
use jq or another JSON encoder to create '{"assignees":["<user>"]}' from the
shell $USER) and pass that encoded payload to curl; change the step that sets
env: USER and the curl invocation in .github/workflows/comment-commands.yml so
the JSON is built and escaped by the runner (refer to the USER env variable and
the curl -d invocation in the file).
🪄 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: Path: .coderabbit.yml
Review profile: CHILL
Plan: Pro
Run ID: c688521f-dae5-4a95-ab77-25e077a517b1
📒 Files selected for processing (1)
.github/workflows/comment-commands.yml
| curl -H "Authorization: token ${{ github.token }}" -d '{"assignees": ["${{ env.USER }}"]}' https://api.github.com/repos/${{ github.repository }}/issues/${{ github.event.issue.number }}/assignees | ||
| env: | ||
| USER: ${{ github.event.comment.user.login }} |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
`For GitHub Actions security hardening:
- Are expressions like
${{ env.USER }}insiderun:interpolated before shell execution? - What is GitHub’s documented guidance for handling untrusted input in workflows (especially script injections)?
- Is
jqavailable by default onubuntu-slimGitHub-hosted runners?`
💡 Result:
-
Yes. In a
run:step, expressions inside${{ ... }}(including${{ env.USER }}) are evaluated and substituted before the temporary shell script is executed, which is why this pattern can enable shell injection if the value is attacker-controlled. [1] -
GitHub’s guidance for untrusted input / script injection (documented):
- Treat GitHub context fields that can be influenced by users (for example issue/PR
title,body, branch names, etc.) as potentially untrusted and prevent them from flowing into places where they may be interpreted as code. [1] - Avoid embedding untrusted values directly via
${{ ... }}insiderun:scripts. Prefer assigning the expression to an intermediate environment variable and then referencing it using the shell’s native variable syntax (for example"$TITLE"in bash). [2][3]
- Treat GitHub context fields that can be influenced by users (for example issue/PR
-
Yes.
jqis included by default on the GitHub-hosted runner labelubuntu-slim(the image’s installed software list includesjq, and it’s also present as an installed apt package). [4]
Sources
[1] GitHub Docs — “Script injections”
[2] GitHub Security Lab — “Keeping your GitHub Actions and workflows secure Part 2: Untrusted input”
[3] CodeQL query help — “Code injection” (GitHub Actions)
[4] actions/runner-images — ubuntu-slim-Readme.md (installed software list)
Use runtime JSON encoding instead of expression interpolation in payload.
At line 21, ${{ env.USER }} is still evaluated as a GitHub Actions expression before the shell script runs, which means untrusted input (github.event.comment.user.login) flows through expression interpolation into the JSON. GitHub's security guidance explicitly recommends against this pattern. Build the JSON payload at runtime with proper escaping instead.
Suggested hardening
- - run: |
- curl -H "Authorization: token ${{ github.token }}" -d '{"assignees": ["${{ env.USER }}"]}' https://api.github.com/repos/${{ github.repository }}/issues/${{ github.event.issue.number }}/assignees
+ - run: |
+ payload="$(jq -cn --arg user "$USER" '{assignees: [$user]}')"
+ curl -fsS \
+ -H "Authorization: token ${{ github.token }}" \
+ -H "Content-Type: application/json" \
+ -d "$payload" \
+ "https://api.github.com/repos/${{ github.repository }}/issues/${{ github.event.issue.number }}/assignees"
env:
USER: ${{ github.event.comment.user.login }}This uses shell variable substitution ($USER) instead of GitHub expression interpolation, and jq encodes the JSON safely. jq is available by default on ubuntu-slim runners.
📝 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.
| curl -H "Authorization: token ${{ github.token }}" -d '{"assignees": ["${{ env.USER }}"]}' https://api.github.com/repos/${{ github.repository }}/issues/${{ github.event.issue.number }}/assignees | |
| env: | |
| USER: ${{ github.event.comment.user.login }} | |
| payload="$(jq -cn --arg user "$USER" '{assignees: [$user]}')" | |
| curl -fsS \ | |
| -H "Authorization: token ${{ github.token }}" \ | |
| -H "Content-Type: application/json" \ | |
| -d "$payload" \ | |
| "https://api.github.com/repos/${{ github.repository }}/issues/${{ github.event.issue.number }}/assignees" | |
| env: | |
| USER: ${{ github.event.comment.user.login }} |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.github/workflows/comment-commands.yml around lines 21 - 23, Replace the
compile-time interpolation of ${{ env.USER }} in the curl payload with a
runtime-built, JSON-encoded payload using the shell USER variable; specifically,
stop using `${{ env.USER }}` inside the curl -d string in the curl invocation
and instead construct the JSON at runtime (e.g., use jq or another JSON encoder
to create '{"assignees":["<user>"]}' from the shell $USER) and pass that encoded
payload to curl; change the step that sets env: USER and the curl invocation in
.github/workflows/comment-commands.yml so the JSON is built and escaped by the
runner (refer to the USER env variable and the curl -d invocation in the file).
| # Using REST API and not `gh issue edit`. https://github.com/cli/cli/issues/6235#issuecomment-1243487651 | ||
| - run: | | ||
| curl -H "Authorization: token ${{ github.token }}" -d '{"assignees": ["${{ github.event.comment.user.login }}"]}' https://api.github.com/repos/${{ github.repository }}/issues/${{ github.event.issue.number }}/assignees | ||
| curl -H "Authorization: token ${{ github.token }}" -d '{"assignees": ["${{ env.USER }}"]}' https://api.github.com/repos/${{ github.repository }}/issues/${{ github.event.issue.number }}/assignees |
Check notice
Code scanning / zizmor
code injection via template expansion Note
Validated that this version still works by pushing this branch to my main branch and triggering it at ShaharNaveh#53 (comment)
Summary by CodeRabbit