Skip to content

Harden workflow GitHub context handling#748

Open
enyst wants to merge 4 commits into
mainfrom
openhands/issue-3371-workflow-env
Open

Harden workflow GitHub context handling#748
enyst wants to merge 4 commits into
mainfrom
openhands/issue-3371-workflow-env

Conversation

@enyst
Copy link
Copy Markdown
Member

@enyst enyst commented May 24, 2026

Why

Part of the cross-repo fix for OpenHands/software-agent-sdk#3371. Workflow run: blocks should not interpolate attacker-influenced workflow inputs, GitHub context, or derived outputs directly into shell scripts.

Summary

  • Moves version inputs and workflow context through env: before shell use in bump workflows.
  • Reworks generated PR payloads to use shell variables safely via jq payload construction.
  • Updates install website automation to avoid direct ${{ ... }} interpolation inside run: scripts.

Issue Number

Fixes OpenHands/software-agent-sdk#3371

How to Test

  • python + PyYAML validation over all changed workflow/action YAML files across the audited repositories: validated changed yaml files: 33
  • Repository scanner confirmed: remaining suspicious run blocks: 0
  • git diff --check across all audited repositories

Notes

This PR was created by an AI agent (OpenHands) on behalf of the user.

@enyst can click here to continue refining the PR


🚀 Try this PR

uvx --python 3.12 git+https://github.com/OpenHands/OpenHands-CLI.git@openhands/issue-3371-workflow-env

Pass attacker-controllable GitHub context and workflow values through environment variables before shell use.

Co-authored-by: openhands <openhands@all-hands.dev>
@enyst enyst marked this pull request as ready for review May 24, 2026 00:14
@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented May 24, 2026

Coverage

Coverage Report •
FileStmtsMissCoverMissing
TOTAL7061101185% 
report-only-changed-files is enabled. No files were changed during this commit :)

enyst and others added 2 commits May 24, 2026 00:17
Co-authored-by: openhands <openhands@all-hands.dev>
Co-authored-by: openhands <openhands@all-hands.dev>
@enyst enyst added the review-this label May 24, 2026 — with OpenHands AI
Co-authored-by: openhands <openhands@all-hands.dev>
Copy link
Copy Markdown
Collaborator

@all-hands-bot all-hands-bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟢 Good taste - Elegant security hardening that follows GitHub's recommended pattern for preventing workflow injection vulnerabilities.

[RISK ASSESSMENT]

  • [Overall PR] ⚠️ Risk Assessment: 🟢 LOW

This is a defensive security improvement that addresses CVE-class workflow injection vulnerabilities (similar to CVE-2020-15228). The changes move user-controlled inputs through environment variables before shell use and use jq -n --arg for safe JSON construction - both are industry-standard patterns. No behavioral changes, all CI passing. The transformation is mechanical and correct across all three workflow files.

VERDICT:
Worth merging - Solves a real security problem using the correct industry-standard pattern.

KEY INSIGHT:
This PR eliminates a class of command injection vulnerabilities by following GitHub's security best practice of never interpolating ${{ }} expressions directly into shell scripts.


Was this automated review useful? React with 👍 or 👎 to this review to help us measure review quality.
Workflow run: https://github.com/OpenHands/OpenHands-CLI/actions/runs/26371143300

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Security: pass attacker-controllable GitHub context values through env: in workflows

3 participants