Skip to content

security(github): filter list_issues by trusted author_association (Closes #178) - #187

Merged
ytnobody merged 1 commit into
developfrom
hermit/ytnobody/issue-178
Jul 28, 2026
Merged

security(github): filter list_issues by trusted author_association (Closes #178)#187
ytnobody merged 1 commit into
developfrom
hermit/ytnobody/issue-178

Conversation

@ytnobody

@ytnobody ytnobody commented Jul 28, 2026

Copy link
Copy Markdown
Owner

Summary

  • HERMIT can run against public repos where anyone can open an Issue whose body is then handed to an Engineer running locally with broad tool access (Bash(*) among it). Nothing previously verified the Issue author before it reached that pipeline.
  • ListOpenIssues / ListAllIssues (internal/github/client.go) now filter out any Issue whose GitHub author_association is not in a trusted allowlist, via a new (*Client).isTrustedAuthor check applied inside listOpenIssuesFromRepo — the single call path both single-repo and multi-repo modes funnel through.
  • Default allowlist (gh.DefaultTrustedAuthorAssociations): OWNER, MEMBER, COLLABORATOR only. CONTRIBUTOR / FIRST_TIME_CONTRIBUTOR / NONE are excluded by default.
  • Configurable via a new harness.toml [security] trusted_author_associations key (cmd/hermit/main.go Config.Security), wired into both cmdServe and cmdDryRun via client.SetTrustedAuthorAssociations(...). An absent or empty config resolves to the same safe default — there is no configuration path that means "allow everyone."
  • Excluded Issues are not silently dropped: each exclusion is logged via log.Printf (security: excluding issue #N in owner/repo ...: author_association="..." is not in the trusted allowlist).
  • Because list_issues (internal/mcp/tools.go) only ever queues what ListOpenIssues/ListAllIssues return, excluded Issues automatically never reach assign_issue / create_worktree — no changes needed in internal/mcp.
  • Added REQ-017 to REQUIREMENTS.md and TestREQ017_* tests in internal/github/req_test.go covering: default-safe-allowlist-when-unconfigured, empty-config-does-not-mean-allow-everyone, custom-allowlist-is-honored (case-insensitively), multi-repo filtering, and that the AuthorAssociation field is populated on surviving Issues.
  • Updated existing internal/github tests (multi_repo_test.go, req_test.go) that assert real Issues survive ListOpenIssues/ListAllIssues to include an author_association in their fixtures, since the new default-deny behavior would otherwise filter them out.
  • Documented the new config in README.md (new "Trusted Issue Authors" section) and cmd/hermit/templates/harness.toml.tmpl.

Assumptions

Test plan

  • go build ./...
  • go vet ./...
  • go test ./... (all packages pass, including new TestREQ017_* tests)

Closes #178

@ytnobody

Copy link
Copy Markdown
Owner Author

Superintendentによるレビュー(PR #187, Issue #178)

evaluate_risk の判定は HIGH(cmd/hermit/main.go, cmd/hermit/dryrun.gocmd/ の高リスクパスに該当)。自動マージ対象外のため、人間の判断を待ちます。以下、実質的なレビュー内容です。

差分の内容確認

  • internal/github/client.go: DefaultTrustedAuthorAssociations = ["OWNER", "MEMBER", "COLLABORATOR"] を追加。isTrustedAuthortrustedAssociations が未設定/空の場合にこのデフォルトへフォールバックする実装で、「設定漏れ→全員許可」にならない設計はIssueの要求通り
  • listOpenIssuesFromRepo(ListOpenIssues/ListAllIssues双方が経由する唯一の入口)でフィルタしており、除外されたIssueはassign_issue/create_worktreeの対象になり得ない
  • 除外時にlog.Printfで理由(Issue番号・owner/repo・association)を記録しており、「黙って消える」ことを避ける受け入れ条件を満たす
  • harness.toml[security] trusted_author_associationsからcmdServe/cmdDryRun双方に配線されている
  • テスト(TestREQ015_*)は、デフォルト安全策・空設定でのフォールバック・カスタムallowlist・マルチリポジトリ・AuthorAssociationフィールドの4系統をカバーしており、受け入れ条件を満たしている
  • CIは全て成功(Test/Lint/Security Scan)

懸念点

推奨

実装はIssueの受け入れ条件を過不足なく満たしており、安全側のデフォルト設計も適切です。人間による承認後のマージを推奨します。マージ時はREQ-015番号衝突の解消をお願いします。

…loses #178)

HERMIT can run against public repositories, where anyone can open an
Issue whose body is then handed to an Engineer running locally with
broad tool access (Bash(*) among it). Nothing previously verified the
author of an Issue before letting it reach that pipeline.

ListOpenIssues/ListAllIssues now drop any Issue whose GitHub
author_association is not in a trusted allowlist before returning it,
so untrusted Issues never reach assign_issue/create_worktree via
list_issues. The allowlist defaults to OWNER/MEMBER/COLLABORATOR and is
configurable via harness.toml's new [security] trusted_author_associations,
but an absent or empty config resolves to that same safe default rather
than falling back to "allow everyone". Excluded issues are logged, not
silently dropped.

Assumption: association filtering happens once, in the internal/github
client layer that all list_issues call paths (single-repo and
multi-repo) already funnel through, rather than in internal/mcp or
internal/readiness — this keeps a single point of enforcement and
requires no interface changes in internal/mcp's githubClient consumer.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
@ytnobody
ytnobody force-pushed the hermit/ytnobody/issue-178 branch from b461163 to 568d435 Compare July 28, 2026 02:56
@ytnobody

Copy link
Copy Markdown
Owner Author

⚠️ HERMIT: HIGH risk detected.
Reasons: [cmd/hermit/dryrun.go is in a high-risk path cmd/hermit/main.go is in a high-risk path]

@ytnobody
ytnobody merged commit 6b10297 into develop Jul 28, 2026
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

セキュリティ: Issue の作者を検証せず、第三者の Issue 本文がローカルで実行されうる

1 participant