actions followups#2220
Conversation
|
👋 @ofek1weiss |
📝 WalkthroughWalkthroughGitHub Actions workflows are refactored to use environment variables for better parameter passing and consistency. Additionally, workflow permissions are scoped using the least privilege principle. These changes improve maintainability and security across CI/CD pipelines by centralizing variable definitions and restricting access. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
.github/workflows/close_pylon_issue.yml (1)
31-37:⚠️ Potential issue | 🟠 MajorAdd timeout and error handling to the
curlPATCH request.The curl command silently succeeds on HTTP 4xx/5xx responses and has no timeout, allowing the workflow to report success when ticket closure fails or hang indefinitely on network issues.
✅ Suggested reliability flags
curl --request PATCH \ + --fail-with-body \ + --silent --show-error \ + --connect-timeout 10 \ + --max-time 30 \ + --retry 3 --retry-all-errors --retry-delay 2 \ --url "https://api.usepylon.com/issues/$PYLON_TICKET_ID" \ --header "Authorization: $PYLON_API_KEY" \ --header 'Content-Type: application/json' \ --data '{ "state": "closed" }'🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/close_pylon_issue.yml around lines 31 - 37, The curl PATCH should use reliability and error flags so failures/timeouts surface: update the command that calls curl with PYLON_TICKET_ID and PYLON_API_KEY to include --fail (or --fail-with-body), --show-error (or -S), a connection timeout like --connect-timeout 5 and a global timeout like --max-time 30, and ensure the step fails on non-zero exit (or explicitly check curl's exit status) so HTTP 4xx/5xx or network timeouts don't silently succeed.
🤖 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/close_pylon_issue.yml:
- Around line 24-29: The "Close Pylon Ticket" step currently trusts an
unvalidated PYLON_TICKET_ID and makes an unreliable curl call; update the step
condition to gate use of PYLON_API_KEY by verifying the event is trusted (for
pull_request_target ensure github.event.pull_request.head.repo.full_name ==
github.repository or require event_name not equal to "pull_request_target") so
secrets are never used for forked PRs, and harden the HTTP call that closes the
ticket (the curl invocation) by adding --fail, --silent/--show-error and a
--max-time timeout and explicitly check curl's exit status and HTTP response to
fail the step on non-2xx responses; reference the "Close Pylon Ticket" step, the
PYLON_TICKET_ID and PYLON_API_KEY env vars, and the curl command in the run
block when making these changes.
---
Outside diff comments:
In @.github/workflows/close_pylon_issue.yml:
- Around line 31-37: The curl PATCH should use reliability and error flags so
failures/timeouts surface: update the command that calls curl with
PYLON_TICKET_ID and PYLON_API_KEY to include --fail (or --fail-with-body),
--show-error (or -S), a connection timeout like --connect-timeout 5 and a global
timeout like --max-time 30, and ensure the step fails on non-zero exit (or
explicitly check curl's exit status) so HTTP 4xx/5xx or network timeouts don't
silently succeed.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: a91dfda9-bbda-45d1-846a-34f91f610d72
📒 Files selected for processing (5)
.github/workflows/close_pylon_issue.yml.github/workflows/release.yml.github/workflows/test-release.yml.github/workflows/test-warehouse.yml.github/workflows/triage-labels.yml
| - name: Close Pylon Ticket | ||
| if: steps.extract_ticket_id.outputs.pylon_ticket_id != '' | ||
| env: | ||
| PYLON_TICKET_ID: ${{ steps.extract_ticket_id.outputs.pylon_ticket_id }} | ||
| PYLON_API_KEY: ${{ secrets.PYLON_API_KEY }} | ||
| run: | |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
Does GitHub Actions pull_request_target expose repository secrets to workflows triggered by forked pull requests, and what mitigations are recommended when processing untrusted PR body content?
💡 Result:
Yes, GitHub Actions workflows triggered by the pull_request_target event have access to repository secrets, even when triggered by pull requests from forked repositories. This is because pull_request_target runs in the context of the base (target) repository's default branch, granting it read access to repository secrets and a GITHUB_TOKEN with write permissions by default. However, this introduces significant security risks, known as "pwn requests," where a malicious PR author can exploit the workflow to execute untrusted code from the fork while having access to secrets. For example, if the workflow checks out the PR head SHA (e.g., github.event.pull_request.head.sha), runs builds, or processes untrusted inputs like the PR body, an attacker can inject code to exfiltrate secrets. Recommended mitigations when using pull_request_target, especially for processing untrusted PR body content: - Never check out, build, or run untrusted code from the PR (e.g., avoid actions/checkout with ref: ${{ github.event.pull_request.head.sha }}). Use only trusted base branch code. - Do not pass secrets as environment variables or inputs to steps that could execute untrusted code. The GITHUB_TOKEN is still accessible in memory even without explicit use. - Restrict to metadata-only operations (e.g., labeling, commenting based on PR title/body without executing code). - Add conditions to skip forks: if: github.event.pull_request.head.repo.full_name == github.repository. - Use label-based gating: Trigger only on specific labels (e.g., types: [labeled]) applied manually after review. - Split workflows: Use pull_request (unprivileged, no secrets) for building untrusted code, then workflow_run (privileged) for post-processing. - Set minimal permissions: permissions: { contents: read } or less. - For PR body processing: Sanitize inputs strictly; avoid dynamic code execution (e.g., no eval on body content). - Enable required reviewers for workflows from forks in repo settings. - Use GitHub Environments with required approvers for secret access. - Scan workflows with CodeQL for vulnerabilities. Prefer pull_request trigger for CI/testing on forks, as it has no secret access.
Citations:
- 1: https://securitylab.github.com/resources/github-actions-preventing-pwn-requests
- 2: https://docs.github.com/actions/using-workflows/events-that-trigger-workflows
- 3: https://www.paulserban.eu/blog/post/pullrequest-vs-pullrequesttarget-the-github-actions-trigger-hiding-a-security-nightmare/
- 4: https://stackoverflow.com/questions/76952023/how-to-make-github-actions-safely-access-secrets-for-prs-created-from-forks
- 5: https://github.blog/changelog/2025-11-07-actions-pull_request_target-and-environment-branch-protections-changes/
- 6: https://securitylab.github.com/resources/github-actions-new-patterns-and-mitigations/
🏁 Script executed:
cat .github/workflows/close_pylon_issue.ymlRepository: elementary-data/elementary
Length of output: 1364
Harden pull_request_target trust gating before using secrets.PYLON_API_KEY and add error handling to the curl call.
This workflow has two issues:
-
Security: Line 25 only checks that an ID exists. On
pull_request_target, PR body content is untrusted input from forked PRs; this lets external contributors drive a privileged API call (closing arbitrary Pylon tickets) by embedding a valid ticket ID. The condition must verify the request comes from a trusted source. -
Reliability: The curl command (lines 31-35) lacks
--failand timeout flags, so HTTP errors won't fail the step and requests could hang indefinitely. The step will report success even if the API call fails.
🔒 Suggested fixes
- name: Close Pylon Ticket
- if: steps.extract_ticket_id.outputs.pylon_ticket_id != ''
+ if: |
+ steps.extract_ticket_id.outputs.pylon_ticket_id != '' &&
+ (
+ github.event_name == 'issues' ||
+ github.event.pull_request.head.repo.full_name == github.repository
+ )
env:
PYLON_TICKET_ID: ${{ steps.extract_ticket_id.outputs.pylon_ticket_id }}
PYLON_API_KEY: ${{ secrets.PYLON_API_KEY }}
run: |
echo "Closing Pylon Ticket ID: $PYLON_TICKET_ID"
curl --fail --connect-timeout 10 --max-time 30 \
--request PATCH \
--url "https://api.usepylon.com/issues/$PYLON_TICKET_ID" \
--header "Authorization: $PYLON_API_KEY" \
--header 'Content-Type: application/json' \
--data '{
"state": "closed"
}'📝 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.
| - name: Close Pylon Ticket | |
| if: steps.extract_ticket_id.outputs.pylon_ticket_id != '' | |
| env: | |
| PYLON_TICKET_ID: ${{ steps.extract_ticket_id.outputs.pylon_ticket_id }} | |
| PYLON_API_KEY: ${{ secrets.PYLON_API_KEY }} | |
| run: | | |
| - name: Close Pylon Ticket | |
| if: | | |
| steps.extract_ticket_id.outputs.pylon_ticket_id != '' && | |
| ( | |
| github.event_name == 'issues' || | |
| github.event.pull_request.head.repo.full_name == github.repository | |
| ) | |
| env: | |
| PYLON_TICKET_ID: ${{ steps.extract_ticket_id.outputs.pylon_ticket_id }} | |
| PYLON_API_KEY: ${{ secrets.PYLON_API_KEY }} | |
| run: | | |
| echo "Closing Pylon Ticket ID: $PYLON_TICKET_ID" | |
| curl --fail --connect-timeout 10 --max-time 30 \ | |
| --request PATCH \ | |
| --url "https://api.usepylon.com/issues/$PYLON_TICKET_ID" \ | |
| --header "Authorization: $PYLON_API_KEY" \ | |
| --header 'Content-Type: application/json' \ | |
| --data '{ | |
| "state": "closed" | |
| }' |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.github/workflows/close_pylon_issue.yml around lines 24 - 29, The "Close
Pylon Ticket" step currently trusts an unvalidated PYLON_TICKET_ID and makes an
unreliable curl call; update the step condition to gate use of PYLON_API_KEY by
verifying the event is trusted (for pull_request_target ensure
github.event.pull_request.head.repo.full_name == github.repository or require
event_name not equal to "pull_request_target") so secrets are never used for
forked PRs, and harden the HTTP call that closes the ticket (the curl
invocation) by adding --fail, --silent/--show-error and a --max-time timeout and
explicitly check curl's exit status and HTTP response to fail the step on
non-2xx responses; reference the "Close Pylon Ticket" step, the PYLON_TICKET_ID
and PYLON_API_KEY env vars, and the curl command in the run block when making
these changes.
2 participants
Summary by CodeRabbit