Skip to content

security: harden PR review workflow triggers and checkout#158

Draft
enyst wants to merge 3 commits intomainfrom
openhands/harden-pr-review-workflow-security
Draft

security: harden PR review workflow triggers and checkout#158
enyst wants to merge 3 commits intomainfrom
openhands/harden-pr-review-workflow-security

Conversation

@enyst
Copy link
Copy Markdown
Collaborator

@enyst enyst commented Apr 8, 2026

Summary

  • require explicit maintainer action before the PR review workflow runs
  • review fork PRs from a trusted base checkout while sourcing the diff from the GitHub API
  • default the workflow template to the ephemeral github.token
  • update PR review docs/tests to match the hardened behavior

Why

This is the remaining worthwhile follow-up from the closed OpenHands/software-agent-sdk#2119 investigation. The software-agent-sdk side was largely superseded, but the migrated PR-review workflow/plugin now lives in OpenHands/extensions, where the privileged pull_request_target flow still needed hardening.

The setup-uv cache hardening was split into #159 so it can be reviewed and merged independently.

Testing

  • uv run --with pytest pytest tests/test_pr_review_review_context.py tests/test_workflow_sync.py
  • git diff --check

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

@enyst can click here to continue refining the PR

Co-authored-by: openhands <openhands@all-hands.dev>
@enyst
Copy link
Copy Markdown
Collaborator Author

enyst commented Apr 8, 2026

@OpenHands Separate the cache fix in another PR from main. Mark it ready for review and babysit until an AI reviewer agent comments; address its inline feedback if any. If it approves it, merge it. Then post a comment here tagging me, so that I know.

@openhands-ai

This comment was marked as outdated.

Co-authored-by: openhands <openhands@all-hands.dev>
Copy link
Copy Markdown
Collaborator Author

enyst commented Apr 8, 2026

@enyst Done — I split the cache change into #159, marked it ready for review, waited for the AI reviewer, and it approved with no inline feedback to address. I then merged #159.

PR #158 is now updated to exclude the cache change and references #159 in the description.

This comment was created by an AI assistant (OpenHands) on behalf of the user.

@openhands-ai

This comment was marked as duplicate.

Co-authored-by: openhands <openhands@all-hands.dev>
Copy link
Copy Markdown
Collaborator Author

enyst commented Apr 8, 2026

Merged main into this branch and fixed the conflict. The overlap came from #159 landing on main while this PR was still editing the same PR-review security docs/files, so I kept both pieces: main's cache-hardening note and this PR's trigger/checkout/github.token changes.

Re-ran: uv run --with pytest pytest tests/test_pr_review_review_context.py tests/test_workflow_sync.py and git diff --check.

This comment was created by an AI assistant (OpenHands) on behalf of the user.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants