OpenHands / OpenHands/extensions
Track remaining qa-changes security related follow-ups
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.
- Load
-
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.
- The original recommendation was to prefer
-
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-keyis 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_TOKENover broader PAT/App tokens. - Re-check whether the current permissions are the minimum required (for example, whether
issues: writeis still needed in addition to review posting).
- Prefer the default workflow
-
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 apidirectly. A narrower harness-owned reporting primitive would reduce the blast radius.
- Today the QA agent can use shell +
-
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, arbitrarycurl/wget, browsing attacker-controlled non-localhost URLs surfaced by the PR, etc.
- Examples:
-
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
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- 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