Harden workflow GitHub context handling#3372
Conversation
Pass attacker-controllable GitHub context and workflow values through environment variables before shell use. Co-authored-by: openhands <openhands@all-hands.dev>
Python API breakage checks — ✅ PASSEDResult: ✅ PASSED |
REST API breakage checks (OpenAPI) — ✅ PASSEDResult: ✅ PASSED |
all-hands-bot
left a comment
There was a problem hiding this comment.
Good security hardening work. The approach — moving ${{ ... }} GitHub context expressions into env: blocks and referencing them as $VAR_NAME in shell — is the correct fix for script injection vulnerabilities (CWE-78). Coverage is comprehensive across all 12 workflow files.
Two minor points worth addressing before merge (both low-risk, one stylistic and one functional).
This review was generated by an AI agent (OpenHands) on behalf of the user through OpenHands Automation.
Co-authored-by: openhands <openhands@all-hands.dev>
Co-authored-by: openhands <openhands@all-hands.dev>
Co-authored-by: openhands <openhands@all-hands.dev>
Co-authored-by: openhands <openhands@all-hands.dev>
all-hands-bot
left a comment
There was a problem hiding this comment.
🟡 Taste Rating: Acceptable — the hardening direction is good, but two workflow-input paths still need tightening before approval.
Issues found:
.github/workflows/integration-runner.ymlstill expandsworkflow_dispatchmodel_idsinto Python source in theResolve model configurationsheredoc (model_ids = "$MODEL_IDS".split(",")). A quoted heredoc plusos.environwould avoid pre-validation code injection.- See inline note on
version-bump-prs.ymlfor$GITHUB_OUTPUTinjection before validation.
Risk: 🟡 Medium, because these workflows handle attacker-influenced inputs and some downstream jobs use elevated tokens/secrets.
This review was generated by an AI agent (OpenHands) on behalf of the user.
Was this automated review useful? React with 👍 or 👎 to this review to help us measure review quality.
Workflow run: https://github.com/OpenHands/software-agent-sdk/actions/runs/26371139976
Co-authored-by: openhands <openhands@all-hands.dev>
all-hands-bot
left a comment
There was a problem hiding this comment.
🟡 Taste Rating: Acceptable — the hardening direction is right, but a few injection/regression issues still need follow-up.
I left inline comments for the model-resolution regression, unsafe output handling, and JSON payload construction. One additional sweep item: .github/workflows/server.yml still builds build-info/*.json with an unquoted heredoc containing ${{ env.IMAGE }} / step outputs, so workflow-dispatch image or malformed outputs can still be expanded by the shell; please generate that JSON via env + jq -n as well.
[RISK ASSESSMENT]
- [Overall PR]
⚠️ Risk Assessment: 🟡 MEDIUM. These are privileged CI/workflow paths, and the remaining shell/output/JSON handling gaps can break runs or preserve injection surfaces.
VERDICT: ❌ Needs follow-up before approval.
This review was generated by an AI agent (OpenHands) on behalf of the user.
Was this automated review useful? React with 👍 or 👎 to this review to help us measure review quality.
Workflow run: https://github.com/OpenHands/software-agent-sdk/actions/runs/26371627904
Co-authored-by: openhands <openhands@all-hands.dev>
all-hands-bot
left a comment
There was a problem hiding this comment.
🟡 Taste Rating: Acceptable — the direction is right, but a few workflow input/output paths still need tightening.
Risk: 🟡 Medium (privileged CI/release workflows; issues are scoped to workflow hardening). This review was generated by an AI agent (OpenHands) on behalf of the user.
Was this automated review useful? React with 👍 or 👎 to this review to help us measure review quality.
Workflow run: https://github.com/OpenHands/software-agent-sdk/actions/runs/26372133139
Co-authored-by: openhands <openhands@all-hands.dev>
Why
Fixes #3371. GitHub Actions run blocks should not interpolate attacker-influenced GitHub context, workflow inputs, or derived outputs directly into shell scripts.
Summary
run:interpolation into stepenv:blocks.${{ ... }}expansion.Issue Number
Fixes #3371
How to Test
python+PyYAMLvalidation over all changed workflow/action YAML files across the audited repositories:validated changed yaml files: 33remaining suspicious run blocks: 0git diff --checkacross all audited repositoriesuv run pre-commit run --files $(git diff --name-only)inOpenHands/software-agent-sdk(yamlfmt passed; Python hooks skipped because only workflow files changed)Video/Screenshots
Not applicable; workflow hardening only.
Type
Notes
This PR was created by an AI agent (OpenHands) on behalf of the user.
@enyst can click here to continue refining the PR
Agent 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.13-nodejs22-slimgolang: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:511f918-pythonRun
All tags pushed for this build
About Multi-Architecture Support
511f918-python) is a multi-arch manifest supporting both amd64 and arm64511f918-python-amd64) are also available if needed