security: Harden GitHub workflows against prompt injection cache pivots#2119
security: Harden GitHub workflows against prompt injection cache pivots#2119
Conversation
Co-authored-by: openhands <openhands@all-hands.dev>
all-hands-bot
left a comment
There was a problem hiding this comment.
Taste Rating: 🟢 Good taste - Pragmatic security hardening
This PR addresses a real attack vector (LLM prompt injection → untrusted code execution in CI → cache poisoning) with a straightforward mitigation: remove automatic execution of untrusted PR code in privileged workflows.
Key Security Wins:
- Gating PR review workflow behind explicit maintainer label removes the untrusted execution path
- Checking out base repo instead of PR head prevents malicious code from running
- Disabling uv cache prevents cross-workflow poisoning attacks
- Switching from PAT to ephemeral
github.tokenreduces credential exposure - Removing unnecessary
id-token: writeshrinks attack surface
Trade-offs (acceptable):
- Reduced automation - maintainers must manually apply
review-thislabel - PR review agent uses
mainSDK instead of PR changes (correct security posture)
Verdict: ✅ Worth merging - Solves real problem without over-engineering.
Key Insight: You correctly identified that convenience (auto-review on PR open) was creating a security hole, and fixed it with the principle of least privilege. This is exactly the right engineering judgment.
|
@OpenHands I think we may have already got part of this in main branch from other PRs, please merge main into this PR and fix the conflicts. |
|
I'm on it! enyst can track my progress at all-hands.dev |
Co-authored-by: openhands <openhands@all-hands.dev>
Work summary (new since the last update)
Completion checklist
Conciseness check
|
API breakage checks (Griffe)Result: Passed |
|
[Automatic Post]: It has been a while since there was any activity on this PR. @enyst, are you still working on it? If so, please go ahead, if not then please request review, close it, or request that someone else follow up. |
2 similar comments
|
[Automatic Post]: It has been a while since there was any activity on this PR. @enyst, are you still working on it? If so, please go ahead, if not then please request review, close it, or request that someone else follow up. |
|
[Automatic Post]: It has been a while since there was any activity on this PR. @enyst, are you still working on it? If so, please go ahead, if not then please request review, close it, or request that someone else follow up. |
|
@OpenHands I think this was solved a while ago. If there is something necessary in it that wasn’t solved already, decide if you’d recommend it, and if yes open a new PR with it, and if not, just tell me so. Clone yourself the extensions repo too so you have the full picture. Note that I close this PR. Don’t push to it. |
|
I'm on it! enyst can track my progress at all-hands.dev |
|
I dug through both the current branch here and the migrated PR-review implementation in . My recommendation: keep this PR closed. I would not revive it in . What I found:
I opened the follow-up there instead:
That PR hardens the current home of the feature by:
So my call is: don’t reopen/reuse this PR branch; the meaningful remaining fix belongs in the extensions repo, and I moved it there. I did not push to this closed PR branch. This comment was created by an AI assistant (OpenHands) on behalf of the user. |
|
Here’s the clean summary after checking both repositories: My recommendation: keep this PR closed. I would not revive it in What I found:
I opened the follow-up there instead:
That PR hardens the current home of the feature by:
So my call is: don’t reopen or reuse this PR branch; the meaningful remaining fix belongs in the extensions repo, and I moved it there. I did not push to this closed PR branch. This comment was created by an AI assistant (OpenHands) on behalf of the user. |
|
Since my last summary, there were no further code or workflow changes. The work remains as previously reported. Completion status
Conciseness of changes
Final answerThe request has been fully addressed: the old PR should remain closed, and the meaningful remaining hardening was moved into |
Summary
Hardens GitHub Actions workflows against the attack pattern where LLM prompt injection leads to code execution in CI, which then enables cache poisoning pivots into more privileged workflows.
Attack Pattern Reference: https://adnanthekhan.com/posts/clinejection/
Security Changes
1. PR Review by OpenHands Workflow
opened/ready_for_review/review_requestedreview-thislabelpull_request_targetworkflow with secret access2. PR Review Composite Action
setup-uvenable-cache: false) to prevent cache poisoning3. Auto-label Issues Workflow
GITHUB_TOKEN4. Integration + Condenser Label Workflows
id-token: writepermissionsetup-matrixjob to only run for relevant labels/triggersSecurity Impact
✅ Eliminates untrusted-code-execution path in
pull_request_targetworkflow✅ Prevents cross-workflow cache poisoning attacks
✅ Reduces secret exposure by using ephemeral tokens
✅ Applies least privilege to workflow permissions
Testing
review-thislabel application by maintainersAgent Server images for this PR
• GHCR package: https://github.com/OpenHands/agent-sdk/pkgs/container/agent-server
Variants & Base Images
eclipse-temurin:17-jdknikolaik/python-nodejs:python3.12-nodejs22golang:1.21-bookwormPull (multi-arch manifest)
# Each variant is a multi-arch manifest supporting both amd64 and arm64 docker pull ghcr.io/openhands/agent-server:14a89ae-pythonRun
All tags pushed for this build
About Multi-Architecture Support
14a89ae-python) is a multi-arch manifest supporting both amd64 and arm6414a89ae-python-amd64) are also available if needed