From 74e1581a01583940377da32bcec7c3ed1a1e24db Mon Sep 17 00:00:00 2001 From: enyst Date: Thu, 12 Mar 2026 14:42:32 +0000 Subject: [PATCH 1/2] Temporarily harden PR review workflow Co-authored-by: openhands --- .github/workflows/pr-review-by-openhands.yml | 43 ++++++++++--------- plugins/pr-review/README.md | 10 ++--- .../workflows/pr-review-by-openhands.yml | 43 ++++++++++--------- 3 files changed, 49 insertions(+), 47 deletions(-) diff --git a/.github/workflows/pr-review-by-openhands.yml b/.github/workflows/pr-review-by-openhands.yml index 9cf0523..1a53722 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 @@ -45,6 +44,8 @@ jobs: llm-base-url: https://llm-proxy.app.all-hands.dev # Review style: roasted (other option: standard) review-style: roasted + # Use the PR's head commit SHA to test plugin changes on the extensions repo itself + extensions-version: ${{ github.event.pull_request.head.sha }} llm-api-key: ${{ secrets.LLM_API_KEY }} github-token: ${{ secrets.ALLHANDS_BOT_GITHUB_PAT }} lmnr-api-key: ${{ secrets.LMNR_SKILLS_API_KEY }} diff --git a/plugins/pr-review/README.md b/plugins/pr-review/README.md index ac84112..4614373 100644 --- a/plugins/pr-review/README.md +++ b/plugins/pr-review/README.md @@ -256,7 +256,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. Fork PRs are currently skipped because the workflow uses `pull_request` while the hardened `pull_request_target` flow is being restored 3. Ensure secrets are configured correctly ### Review Comments Not Appearing @@ -273,10 +273,10 @@ 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 -- PR code is checked out explicitly; secrets are not exposed to PR code -- Credentials are not persisted during checkout +- Uses `pull_request` as a temporary hardening measure while the `pull_request_target` flow is being secured +- Only triggers for same-repository PRs when opened, marked ready for review, labeled `review-this`, or when `openhands-agent` / `all-hands-bot` is requested +- Fork PRs are skipped until the hardened `pull_request_target` flow is restored +- PR code is checked out explicitly and credentials are not persisted during checkout ## Contributing diff --git a/plugins/pr-review/workflows/pr-review-by-openhands.yml b/plugins/pr-review/workflows/pr-review-by-openhands.yml index 9cf0523..1a53722 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 @@ -45,6 +44,8 @@ jobs: llm-base-url: https://llm-proxy.app.all-hands.dev # Review style: roasted (other option: standard) review-style: roasted + # Use the PR's head commit SHA to test plugin changes on the extensions repo itself + extensions-version: ${{ github.event.pull_request.head.sha }} llm-api-key: ${{ secrets.LLM_API_KEY }} github-token: ${{ secrets.ALLHANDS_BOT_GITHUB_PAT }} lmnr-api-key: ${{ secrets.LMNR_SKILLS_API_KEY }} From 6089960a451990d8b218b8088afcc86b6815251f Mon Sep 17 00:00:00 2001 From: openhands Date: Thu, 12 Mar 2026 17:31:50 +0000 Subject: [PATCH 2/2] Tighten PR review workflow wording Co-authored-by: openhands --- .github/workflows/pr-review-by-openhands.yml | 9 +++------ plugins/pr-review/README.md | 6 +++--- plugins/pr-review/workflows/pr-review-by-openhands.yml | 9 +++------ 3 files changed, 9 insertions(+), 15 deletions(-) diff --git a/.github/workflows/pr-review-by-openhands.yml b/.github/workflows/pr-review-by-openhands.yml index 1a53722..16d38b9 100644 --- a/.github/workflows/pr-review-by-openhands.yml +++ b/.github/workflows/pr-review-by-openhands.yml @@ -2,10 +2,7 @@ name: PR Review by OpenHands on: - # 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. + # Run on pull requests and review same-repository PRs only. pull_request: types: [opened, ready_for_review, labeled, review_requested] @@ -16,8 +13,8 @@ permissions: jobs: pr-review: - # 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. + # Fork PRs do not have access to repository secrets under `pull_request`. + # Review same-repository PRs only. if: | github.event.pull_request.head.repo.full_name == github.repository && ( diff --git a/plugins/pr-review/README.md b/plugins/pr-review/README.md index 4614373..4e7d1dc 100644 --- a/plugins/pr-review/README.md +++ b/plugins/pr-review/README.md @@ -256,7 +256,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. Fork PRs are currently skipped because the workflow uses `pull_request` while the hardened `pull_request_target` flow is being restored +2. This workflow reviews same-repository PRs only; fork PRs are skipped 3. Ensure secrets are configured correctly ### Review Comments Not Appearing @@ -273,9 +273,9 @@ If you see rate limit errors: ## Security -- Uses `pull_request` as a temporary hardening measure while the `pull_request_target` flow is being secured +- Uses `pull_request` - Only triggers for same-repository PRs when opened, marked ready for review, labeled `review-this`, or when `openhands-agent` / `all-hands-bot` is requested -- Fork PRs are skipped until the hardened `pull_request_target` flow is restored +- Fork PRs are skipped - PR code is checked out explicitly and credentials are not persisted during checkout ## Contributing diff --git a/plugins/pr-review/workflows/pr-review-by-openhands.yml b/plugins/pr-review/workflows/pr-review-by-openhands.yml index 1a53722..16d38b9 100644 --- a/plugins/pr-review/workflows/pr-review-by-openhands.yml +++ b/plugins/pr-review/workflows/pr-review-by-openhands.yml @@ -2,10 +2,7 @@ name: PR Review by OpenHands on: - # 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. + # Run on pull requests and review same-repository PRs only. pull_request: types: [opened, ready_for_review, labeled, review_requested] @@ -16,8 +13,8 @@ permissions: jobs: pr-review: - # 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. + # Fork PRs do not have access to repository secrets under `pull_request`. + # Review same-repository PRs only. if: | github.event.pull_request.head.repo.full_name == github.repository && (