Repository navigation
Avoid shell interpolation of delayed comment input #465
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.
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
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -9,6 +9,7 @@ on: | |
| delay_minutes: | ||
| description: 'Minutes to wait before commenting' | ||
| required: true | ||
| type: number | ||
| message: | ||
| description: 'Comment body' | ||
| required: true | ||
|
|
@@ -23,9 +24,19 @@ jobs: | |
| steps: | ||
| - name: Convert minutes to seconds | ||
| id: calc | ||
| run: echo "secs=$(( ${{ github.event.inputs.delay_minutes }} * 60 ))" >> "$GITHUB_OUTPUT" | ||
| env: | ||
| DELAY_MINUTES: ${{ inputs.delay_minutes }} | ||
| shell: bash | ||
| run: | | ||
| if [[ ! "$DELAY_MINUTES" =~ ^[0-9]+$ ]]; then | ||
| echo "delay_minutes must be a non-negative integer" >&2 | ||
| exit 1 | ||
| fi | ||
| echo "secs=$((10#$DELAY_MINUTES * 60))" >> "$GITHUB_OUTPUT" | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win 🔎 Supported by static analysis🏁 Script executed: sed -n '1,90p' .github/workflows/delayed-pr-comment.yml
printf '%s\n' '--- bash arithmetic probes ---'
bash -c 'for n in 153722867280912930 153722867280912931; do printf "%s -> " "$n"; x=$((10#$n * 60)); printf "%s\n" "$x"; done'Repository: leynos/cuprum Length of output: 1691 🌐 Web query:
💡 Result: <source_evidence> Citations:
🌐 Web query:
💡 Result: <source_evidence> Citations:
Reject decimal delay values that overflow Bash arithmetic. When Reject values above 🧰 Tools🪛 zizmor (1.30.0)[warning] 2-46: overly broad permissions (excessive-permissions): default permissions used due to no permissions: block (excessive-permissions) 🤖 Prompt for AI Agents |
||
| - name: Wait requested time | ||
| run: sleep ${{ steps.calc.outputs.secs }} | ||
| env: | ||
| SECONDS_TO_WAIT: ${{ steps.calc.outputs.secs }} | ||
| run: sleep "$SECONDS_TO_WAIT" | ||
| shell: bash | ||
| - name: Comment PR | ||
| uses: thollander/actions-comment-pull-request@24bffb9b452ba05a4f3f77933840a6a841d1b32b | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This security fix adds an input boundary and changes the delay calculation without any test that proves valid integers still produce the expected output or that crafted values are rejected before shell evaluation. The existing test reference to this workflow only checks runner placement, so a future edit could restore direct interpolation while all current checks remain green; add a failing-before/passing-after regression test around this step.
AGENTS.md reference: AGENTS.md:L81-L86
Useful? React with 👍 / 👎.