docs: add Security Considerations section to openhands-automation skill - #429
Merged
Conversation
The skill documented how to fetch secrets and register webhooks, but had no guidance on trust boundaries. In practice this shows up as a real pattern: every custom automation that spawns a conversation lists every configured org secret and forwards all of them, because that's the only example available to copy from, rather than passing an explicit allowlist of the names actually needed. Adds references/security.md covering: - untrusted event content vs. verified sender (signature verification proves who sent an event, not that its content is safe) - least-privilege secret scoping for spawned conversations, with a concrete allowlist pattern contrasted against the forward-everything anti-pattern - scoping triggers narrowly (GitHub is org-wide by default with no registration step) - sender-level authorization as distinct from content-based filters - verifying a packaged script actually runs before deploying it, since py_compile only catches syntax errors, not config values that render as valid-but-wrong Python SKILL.md gets a short pointer section summarizing the two points that apply to every automation, plus a references/ entry matching the existing custom-automation.md and ab-testing.md pattern.
… source Tightened prose and code samples in security.md - same five points, less than two-thirds the length. Also fixes the py_compile gap directly in custom-automation.md's existing "Validate Before Packaging" section rather than only documenting it in the new file.
Extends the secrets bullet in Security Considerations - when it's not obvious which secrets a spawned conversation needs, ask rather than guess or default to forwarding everything.
VascoSch92
marked this pull request as ready for review
July 30, 2026 10:32
skills/index.js embeds each skill's SKILL.md content verbatim for the catalog. The openhands-automation edits changed that content, so the generated index was stale - CI's test job catches this drift.
neubig
approved these changes
Jul 30, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
The
openhands-automationskill covers fetching secrets and verifying webhook signatures, but has no guidance on trust boundaries. In practice: checking several already-deployed custom automations (PR reviewer, issue triage, Slack monitor), every one forwards every configured org secret to its spawned conversation — not a deliberate choice, just the only pattern the skill gives to copy from.What's added
references/security.md(new, ~580 words) — untrusted content vs. verified sender, least-privilege secret allowlisting (anti-pattern + fix), narrowing triggers, sender-level authorization, verifying a script runs before deploying it.SKILL.md— two-bullet pointer section + areferences/entry, matching the existingcustom-automation.md/ab-testing.mdpattern.custom-automation.md— the existing "Validate Before Packaging" section only mentionspy_compile, which doesn't catch config values that are valid-but-wrong Python (e.g.json.dumpsemittingtrue/false). Added a note + link to the new file's verification section, fixed at the source rather than only in the new doc.Test plan
mainbefore editing, so this isn't against a stale copyclaude plugin validate plugins/openhands/passes (same check CI runs)