diff --git a/.github/workflows/pr-review-by-openhands.yml b/.github/workflows/pr-review-by-openhands.yml index 9cf0523..7080764 100644 --- a/.github/workflows/pr-review-by-openhands.yml +++ b/.github/workflows/pr-review-by-openhands.yml @@ -2,16 +2,11 @@ name: PR Review by OpenHands on: - # Use pull_request_target to allow fork PRs to access secrets when triggered by maintainers - # Security: This workflow runs when: - # 1. A new PR is opened (non-draft), OR - # 2. A draft PR is marked as ready for review, OR - # 3. A maintainer adds the 'review-this' label, OR - # 4. A maintainer requests openhands-agent or all-hands-bot as a reviewer - # Adding labels and requesting reviewers requires write access. - # The PR code is explicitly checked out for review, but secrets are only accessible - # because the workflow runs in the base repository context. - pull_request_target: + # TEMPORARY MITIGATION (Clinejection hardening) + # + # We temporarily avoid `pull_request_target` here. We'll restore it after the PR review + # workflow is fully hardened for untrusted execution. + pull_request: types: [opened, ready_for_review, labeled, review_requested] permissions: @@ -21,18 +16,22 @@ permissions: jobs: pr-review: - # Run when one of the following conditions is met: - # 1. A new non-draft PR is opened by a non-first-time contributor, OR - # 2. A draft PR is converted to ready for review by a non-first-time contributor, OR - # 3. 'review-this' label is added, OR - # 4. openhands-agent or all-hands-bot is requested as a reviewer - # Note: FIRST_TIME_CONTRIBUTOR and NONE PRs require manual trigger via label/reviewer request. + # Note: fork PRs will not have access to repository secrets under `pull_request`. + # Skip forks to avoid noisy failures until we restore a hardened `pull_request_target` flow. if: | - (github.event.action == 'opened' && github.event.pull_request.draft == false && github.event.pull_request.author_association != 'FIRST_TIME_CONTRIBUTOR' && github.event.pull_request.author_association != 'NONE') || - (github.event.action == 'ready_for_review' && github.event.pull_request.author_association != 'FIRST_TIME_CONTRIBUTOR' && github.event.pull_request.author_association != 'NONE') || - github.event.label.name == 'review-this' || - github.event.requested_reviewer.login == 'openhands-agent' || - github.event.requested_reviewer.login == 'all-hands-bot' + github.event.pull_request.head.repo.full_name == github.repository && + ( + (github.event.action == 'opened' && github.event.pull_request.draft == false) || + github.event.action == 'ready_for_review' || + (github.event.action == 'labeled' && github.event.label.name == 'review-this') || + ( + github.event.action == 'review_requested' && + ( + github.event.requested_reviewer.login == 'openhands-agent' || + github.event.requested_reviewer.login == 'all-hands-bot' + ) + ) + ) concurrency: group: pr-review-${{ github.event.pull_request.number }} cancel-in-progress: true diff --git a/plugins/pr-review/README.md b/plugins/pr-review/README.md index 4c457fb..ce78dfe 100644 --- a/plugins/pr-review/README.md +++ b/plugins/pr-review/README.md @@ -115,14 +115,18 @@ Create a `review-this` label for manual review triggers: PR reviews are automatically triggered when: -1. A new non-draft PR is opened (by non-first-time contributors) -2. A draft PR is marked as ready for review -3. The `review-this` label is added -4. `openhands-agent` or `all-hands-bot` is requested as a reviewer +1. A new non-draft PR is opened by a collaborator (author association: `COLLABORATOR`, `MEMBER`, or `OWNER`) +2. A draft PR is marked as ready for review by a collaborator +3. The `review-this` label is added by a user with **write (or higher)** permission +4. `openhands-agent` is requested as a reviewer by a user with **write** permission ### Requesting a Review -**Option 1: Request as Reviewer (Recommended)** +Only users with **write (or higher)** permission can trigger a review. + +In this workflow, the **triggering user** is `github.actor`: the account that performed the action that started the run (e.g. the person who added the `review-this` label or requested the reviewer) — not necessarily the PR author. + +**Option 1: Request as Reviewer** 1. Open the PR 2. Click **Reviewers** in the sidebar 3. Select `openhands-agent` as a reviewer @@ -251,7 +255,7 @@ Also update any `sdk-repo` and `sdk-version` inputs to `extensions-repo` and `ex ### Review Not Triggered 1. Check that the workflow file is in `.github/workflows/` -2. Verify the PR author association (first-time contributors need manual trigger) +2. Verify the PR author association (automatic runs only for collaborators; otherwise a maintainer must trigger via label/reviewer request) 3. Ensure secrets are configured correctly ### Review Comments Not Appearing @@ -269,7 +273,7 @@ If you see rate limit errors: ## Security - Uses `pull_request_target` to safely access secrets for fork PRs -- Only triggers for trusted contributors or when maintainers add labels/reviewers +- Automatic reviews only run for collaborators; manual triggers require **write (or higher)** permission - PR code is checked out explicitly; secrets are not exposed to PR code - Credentials are not persisted during checkout diff --git a/plugins/pr-review/workflows/pr-review-by-openhands.yml b/plugins/pr-review/workflows/pr-review-by-openhands.yml index 9cf0523..7080764 100644 --- a/plugins/pr-review/workflows/pr-review-by-openhands.yml +++ b/plugins/pr-review/workflows/pr-review-by-openhands.yml @@ -2,16 +2,11 @@ name: PR Review by OpenHands on: - # Use pull_request_target to allow fork PRs to access secrets when triggered by maintainers - # Security: This workflow runs when: - # 1. A new PR is opened (non-draft), OR - # 2. A draft PR is marked as ready for review, OR - # 3. A maintainer adds the 'review-this' label, OR - # 4. A maintainer requests openhands-agent or all-hands-bot as a reviewer - # Adding labels and requesting reviewers requires write access. - # The PR code is explicitly checked out for review, but secrets are only accessible - # because the workflow runs in the base repository context. - pull_request_target: + # TEMPORARY MITIGATION (Clinejection hardening) + # + # We temporarily avoid `pull_request_target` here. We'll restore it after the PR review + # workflow is fully hardened for untrusted execution. + pull_request: types: [opened, ready_for_review, labeled, review_requested] permissions: @@ -21,18 +16,22 @@ permissions: jobs: pr-review: - # Run when one of the following conditions is met: - # 1. A new non-draft PR is opened by a non-first-time contributor, OR - # 2. A draft PR is converted to ready for review by a non-first-time contributor, OR - # 3. 'review-this' label is added, OR - # 4. openhands-agent or all-hands-bot is requested as a reviewer - # Note: FIRST_TIME_CONTRIBUTOR and NONE PRs require manual trigger via label/reviewer request. + # Note: fork PRs will not have access to repository secrets under `pull_request`. + # Skip forks to avoid noisy failures until we restore a hardened `pull_request_target` flow. if: | - (github.event.action == 'opened' && github.event.pull_request.draft == false && github.event.pull_request.author_association != 'FIRST_TIME_CONTRIBUTOR' && github.event.pull_request.author_association != 'NONE') || - (github.event.action == 'ready_for_review' && github.event.pull_request.author_association != 'FIRST_TIME_CONTRIBUTOR' && github.event.pull_request.author_association != 'NONE') || - github.event.label.name == 'review-this' || - github.event.requested_reviewer.login == 'openhands-agent' || - github.event.requested_reviewer.login == 'all-hands-bot' + github.event.pull_request.head.repo.full_name == github.repository && + ( + (github.event.action == 'opened' && github.event.pull_request.draft == false) || + github.event.action == 'ready_for_review' || + (github.event.action == 'labeled' && github.event.label.name == 'review-this') || + ( + github.event.action == 'review_requested' && + ( + github.event.requested_reviewer.login == 'openhands-agent' || + github.event.requested_reviewer.login == 'all-hands-bot' + ) + ) + ) concurrency: group: pr-review-${{ github.event.pull_request.number }} cancel-in-progress: true