OpenHands / OpenHands/extensions

Track remaining qa-changes security related follow-ups

Open
#212 1 comment 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

Dominant language
Python
Stars
148
Forks
90
Avg merge
1d 17h
Merged PRs (30d)
36

Description

Context

Follow-up from PR #135 (feat: add qa-changes plugin for automated PR QA validation) and the blast-radius review comment here:

This is intended as a follow-up tracking issue, not a blocker for merging the current PR.

Already addressed in PR #135

These items from the original list are already handled and should not be tracked here as remaining work:

  • ✅ Disable shared uv cache by default in qa-changes (8b89af1)
  • ✅ Tell the QA prompt to treat all PR-derived content as untrusted, not just the PR body (81a9527)
  • ✅ Gracefully skip fork PR runs when secrets are unavailable instead of failing (9774ad8)

Remaining follow-ups worth tracking

These are the items from the original blast-radius list that still seem relevant after the fixes above, especially for secret-bearing runs on same-repo PRs or maintainer-triggered runs:

  • Do not load repo-controlled guidance from the PR head for secret-bearing QA runs

    • Load AGENTS.md / project skills from the base branch or another maintainer-controlled source instead of the checked-out PR head.
  • Decide whether secret-bearing QA should be manual / maintainer-triggered only

    • The original recommendation was to prefer qa-this / reviewer-request style triggering over automatic secret-bearing execution.
  • Document the runner trust model more explicitly

    • The example workflow currently uses GitHub-hosted runners, but docs should clearly say this workflow should not be used on self-hosted/shared runners without stronger sandboxing.
  • Make telemetry opt-in in the example workflow

    • lmnr-api-key is optional in the action, but the example workflow still wires in the Laminar secret by default.
  • Review and minimize credential scope

    • Prefer the default workflow GITHUB_TOKEN over broader PAT/App tokens.
    • Re-check whether the current permissions are the minimum required (for example, whether issues: write is still needed in addition to review posting).
  • Separate execution from privileged reporting

    • Longer-term, the runner that executes PR code should ideally not share the same trust domain / credentials as the component that posts the final QA result.
  • Reduce raw secret exposure to subprocesses

    • Today the QA agent can use shell + gh api directly. A narrower harness-owned reporting primitive would reduce the blast radius.
  • Add network / egress restrictions guidance

    • During setup/exercise, ideally allow only the minimum outbound destinations needed (for example package registries, localhost, GitHub API).
  • Add security rails / analyzers for obvious exfiltration paths

    • Examples: env, printenv, reading credential files, arbitrary curl/wget, browsing attacker-controlled non-localhost URLs surfaced by the PR, etc.
  • Consider a stronger executor isolation model

    • Examples from the original list: two-sandbox split, fresh isolated executor, no persistent trust/caches/host mounts, tighter containment for long-term hardening.

Notes

The fork-specific concern from the original comment is now less urgent because the action skips cleanly when secrets are unavailable on fork PRs. The items above are the ones that still appear to matter for broader rollout or for any run that executes untrusted PR code while secrets are present.

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

Contributor guide

No contributing guide indexed for this repository

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start by reviewing PR #135 and the linked blast-radius comment, then inspect the qa-changes example workflow and its handling of AGENTS.md, project skills, secrets, permissions, and reporting. This issue tracks several independent security and architecture decisions rather than one bounded change. Done requires narrowing the scope and addressing or explicitly deciding the remaining follow-ups.

Written by the indexing model from the issue text.

Assessment

Tech stack
github-actions
Domain
ci-cd, devops, security
Issue type
Feature
Difficulty
5/5
Estimated time
Over a week
Activity status
Active
Clarity
Needs clarification
Newbie friendliness
25/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.