diff --git a/README.md b/README.md index d53d79f..cf248f9 100644 --- a/README.md +++ b/README.md @@ -235,6 +235,9 @@ engineer = "claude-sonnet-5" # model used for Engineer roles # skip_acceptance_criteria_check = false # if true, don't require an "Acceptance Criteria" / "受け入れ条件" section # label = "needs-clarification" # label applied to Issues judged not ready; also excludes them from list_issues +# [security] +# trusted_author_associations = ["OWNER", "MEMBER", "COLLABORATOR"] # default shown; other possible values: "CONTRIBUTOR", "FIRST_TIME_CONTRIBUTOR", "NONE" + # [notification] # webhook_url = "https://hooks.slack.com/services/..." # Slack, Discord, or generic webhook # type = "slack" # "slack" | "discord" | "generic" (auto-detected from URL if omitted) @@ -268,6 +271,21 @@ When an Issue is judged not ready, HERMIT: Once the requirements are filled in and the label is removed, the Issue returns to the normal queue on the next cycle. +### Trusted Issue Authors (`[security]`) + +HERMIT can run against public repositories, where anyone can open an Issue. Since the Superintendent hands Issue bodies to an Engineer that runs locally with broad tool access (including `Bash(*)`), an Issue from an untrusted stranger must never be treated as an instruction. + +To prevent this, `list_issues` (`ListOpenIssues` / `ListAllIssues`) filters every Issue by the GitHub-computed `author_association` between its author and the repository, keeping only Issues from authors in the `trusted_author_associations` allowlist: + +```toml +[security] +trusted_author_associations = ["OWNER", "MEMBER", "COLLABORATOR"] # default shown +``` + +- **Default and fallback**: `OWNER`, `MEMBER`, and `COLLABORATOR` only. This is applied both when `[security]` is omitted from `harness.toml` entirely and when `trusted_author_associations` is present but left empty — there is no configuration that resolves to "allow everyone." `CONTRIBUTOR`, `FIRST_TIME_CONTRIBUTOR`, and `NONE` (the associations any GitHub account, including a first-time stranger, can have on a public repo) are excluded by default. +- **Custom allowlist**: set `trusted_author_associations` to any subset (or superset) of GitHub's `author_association` values to widen or narrow the allowlist for your project. +- **Visibility**: Issues excluded by this filter are never silently dropped — each exclusion is logged (`security: excluding issue #N in owner/repo ...: author_association="..." is not in the trusted allowlist`) so an operator watching HERMIT's logs can notice. Excluded Issues also never reach `assign_issue` / `create_worktree`, since the Superintendent only acts on what `list_issues` returns. + ### Model Selection The `[model]` section lets you specify which Claude model each role uses. You can also apply a preset with `hermit use`: diff --git a/REQUIREMENTS.md b/REQUIREMENTS.md index c7d963c..a2c6baa 100644 --- a/REQUIREMENTS.md +++ b/REQUIREMENTS.md @@ -204,3 +204,11 @@ reconcile sweep の review-test は、要件の「仕様」が変わったとき - 受け入れ条件: `Requirement.Hash` が `受け入れ条件` と `verify` のみから計算され、要件ブロック全体からは計算されないこと。`実装状況` 行のみを変更しても次の sweep で review-test が発火しないこと。`受け入れ条件` の変更、および `verify` の `test` ↔ `manual` の切り替えは従来どおり発火すること。見出しや説明文のみの変更では発火しないこと。ハッシュストアに計算方式のバージョンが記録され、方式変更後の初回 sweep は全件を再計算・保存するのみで Issue を起票しないこと - verify: test - 実装状況: 実装済み — `internal/requirements/requirements.go` の `specHash` が `AcceptanceCriteria` と `Verify` のみからハッシュを計算するように変更 (旧 `hashText(block)` を置き換え)。`internal/requirements/hashstore.go` の `HashStore` インターフェースを `Load() (version int, hashes map[string]string, err error)` / `Save(version int, hashes map[string]string) error` に拡張し、`HashSchemeVersion` 定数 (現在値 2) を導入。旧形式 (バージョン無しの素の map) のファイルは version 0 として扱われ後方互換。`internal/requirements/sweep.go` の `Sweep` は読み込んだバージョンが `HashSchemeVersion` と異なる場合 `schemeChanged` として HashChanged 判定を強制的に false にし (review-test を発火させず)、sweep 終了時に現行バージョンでハッシュを保存し直すことで移行を1回のsweepで完了させる。自己増殖ループの回帰テストは `internal/requirements/sweep_test.go` の `TestSweep_ImplementationStatusOnlyChange_DoesNotFireReviewTest`、スキーマ移行の回帰テストは同ファイルの `TestSweep_HashSchemeMigration_DoesNotFireReviewTest_JustRecomputesAndSaves`、ハッシュ計算自体の単体テストは `internal/requirements/requirements_test.go` の `TestParse_HashUnaffectedByImplementationStatusField` / `TestParse_HashUnaffectedByTitleOrDescriptionOnly` / `TestParse_HashChangesWithVerifyMode` で検証。REQ-ID 命名規約に沿った `TestREQ016_ReviewTestHashIgnoresImplementationStatus` を追加 + +## REQ-017: list_issues は信頼できる author_association の Issue のみを返す + +HERMIT は public リポジトリで運用され得るため、第三者が作成した Issue の本文がそのまま Engineer への指示としてローカルで実行されることを防ぐ。`ListOpenIssues` / `ListAllIssues` は GitHub API の `author_association` を参照し、信頼できる association を持つ Issue のみを返す。信頼する association は `harness.toml` の `[security] trusted_author_associations` で設定可能で、既定値は `OWNER` / `MEMBER` / `COLLABORATOR` の 3 種のみ (`CONTRIBUTOR` / `FIRST_TIME_CONTRIBUTOR` / `NONE` は含めない)。 + +- 受け入れ条件: 信頼できない author_association を持つ Issue が `ListOpenIssues` / `ListAllIssues` の結果から除外されること。`harness.toml` に設定が無い、または空の場合も安全な既定値 (OWNER/MEMBER/COLLABORATOR) が適用され「全員許可」にフォールバックしないこと。除外時にログが出力されること +- verify: test +- 実装状況: 実装済み — `internal/github/client.go` の `listOpenIssuesFromRepo` が各 Issue の `author_association` を `(*Client).isTrustedAuthor` で判定し、非信頼の Issue を除外したうえで `log.Printf` により除外理由 (Issue番号・owner/repo・association) を記録する。信頼リストは `(*Client).SetTrustedAuthorAssociations` で設定でき、未設定または空スライスを渡した場合は `gh.DefaultTrustedAuthorAssociations` (`OWNER`/`MEMBER`/`COLLABORATOR`) にフォールバックする — 「空 = 全員許可」にはならない。`cmd/hermit/main.go` の `Config.Security.TrustedAuthorAssociations` (`harness.toml` の `[security] trusted_author_associations`) が `cmdServe` / `cmdDryRun` で `SetTrustedAuthorAssociations` に渡される。除外された Issue が `assign_issue` / `create_worktree` の対象にならないのは、`list_issues` (`internal/mcp/tools.go`) がこの層より上位で `ListOpenIssues`/`ListAllIssues` の返り値のみをキューとして扱うため自動的に保証される。テストは `internal/github/req_test.go` の `TestREQ017_*` diff --git a/cmd/hermit/dryrun.go b/cmd/hermit/dryrun.go index fcf2b10..408b62c 100644 --- a/cmd/hermit/dryrun.go +++ b/cmd/hermit/dryrun.go @@ -17,6 +17,7 @@ func cmdDryRun() { cfg := loadConfig() token := githubToken() client := gh.NewClient(token, cfg.GitHub.Owner, cfg.GitHub.Repo) + client.SetTrustedAuthorAssociations(cfg.Security.TrustedAuthorAssociations) prefix := resolveBranchPrefix(cfg) fmt.Println("Dry-run mode: showing what would happen") diff --git a/cmd/hermit/main.go b/cmd/hermit/main.go index 7900873..4873bc2 100644 --- a/cmd/hermit/main.go +++ b/cmd/hermit/main.go @@ -158,6 +158,21 @@ type Config struct { // needing to opt in. Paths []string `toml:"paths"` } `toml:"requirements"` + Security struct { + // TrustedAuthorAssociations is the allowlist of GitHub + // "author_association" values (e.g. "OWNER", "MEMBER", + // "COLLABORATOR", "CONTRIBUTOR", "FIRST_TIME_CONTRIBUTOR", "NONE") + // whose Issues are surfaced by list_issues. HERMIT runs unattended + // with broad local tool access, and public repos accept Issues from + // anyone, so an Issue from an untrusted author must never reach the + // loop as an implicit instruction (Issue #178). + // + // Default when empty/omitted: falls back to + // gh.DefaultTrustedAuthorAssociations ("OWNER", "MEMBER", + // "COLLABORATOR") — a missing [security] section is NOT a no-op / + // "allow everyone" fallback; it resolves to this same safe default. + TrustedAuthorAssociations []string `toml:"trusted_author_associations"` + } `toml:"security"` } // resolveRiskConfig builds the effective default risk.Config (harness.toml's @@ -471,6 +486,7 @@ func cmdServe() { cfg := loadConfig() token := githubToken() client := gh.NewClient(token, cfg.GitHub.Owner, cfg.GitHub.Repo) + client.SetTrustedAuthorAssociations(cfg.Security.TrustedAuthorAssociations) prefix := resolveBranchPrefix(cfg) runRequirementsHearingCheck(rootDir, cfg, client) diff --git a/cmd/hermit/templates/harness.toml.tmpl b/cmd/hermit/templates/harness.toml.tmpl index 38987eb..f5694dd 100644 --- a/cmd/hermit/templates/harness.toml.tmpl +++ b/cmd/hermit/templates/harness.toml.tmpl @@ -41,6 +41,16 @@ analyst_effort = "{{ .AnalystEffort }}" # reasoning effort: low/medium/ # skip_acceptance_criteria_check = false # if true, don't require an "Acceptance Criteria" / "受け入れ条件" section # label = "needs-clarification" # label applied to Issues judged not ready; also excludes them from list_issues +# [security] +# # Allowlist of GitHub "author_association" values whose Issues are surfaced +# # by list_issues. Since HERMIT runs unattended with broad local tool access +# # and public repos accept Issues from anyone, an Issue from an untrusted +# # author must never reach the loop as an implicit instruction. Omitting this +# # section (or leaving it empty) is NOT "allow everyone" — it falls back to +# # the same safe default shown below. Excluded Issues are logged (not +# # silently dropped) so a human operator can notice. +# trusted_author_associations = ["OWNER", "MEMBER", "COLLABORATOR"] # default shown; other possible values: "CONTRIBUTOR", "FIRST_TIME_CONTRIBUTOR", "NONE" + # [notification] # webhook_url = "https://hooks.slack.com/services/..." # Slack, Discord, or generic webhook # type = "slack" # "slack" | "discord" | "generic" (auto-detected from URL if omitted) diff --git a/internal/github/client.go b/internal/github/client.go index 6b45db1..573c76e 100644 --- a/internal/github/client.go +++ b/internal/github/client.go @@ -3,6 +3,7 @@ package github import ( "context" "fmt" + "log" "os/exec" "regexp" "strconv" @@ -13,6 +14,21 @@ import ( "golang.org/x/oauth2" ) +// DefaultTrustedAuthorAssociations is the safe-by-default allowlist of +// GitHub "author_association" values whose Issues are surfaced to the +// HERMIT loop, applied whenever harness.toml does not configure +// [security].trusted_author_associations (or leaves it empty). +// +// HERMIT runs unattended with broad local tool access (Bash(*) among it — +// see Issue #180), and public repositories accept Issues from anyone. Since +// Issue #178, list_issues only returns Issues whose author has one of these +// associations with the repository — CONTRIBUTOR, FIRST_TIME_CONTRIBUTOR, +// and NONE (the associations any GitHub account can have with a public +// repo) are deliberately excluded from the default so a stranger's Issue +// body can never reach an Engineer as an instruction without an explicit +// opt-in. +var DefaultTrustedAuthorAssociations = []string{"OWNER", "MEMBER", "COLLABORATOR"} + // issueRefRe matches common issue reference patterns in PR bodies/titles // e.g. "closes #31", "fixes #31", "#31" var issueRefRe = regexp.MustCompile(`(?i)(?:close[sd]?|fix(?:e[sd])?|resolve[sd]?)?\s*#(\d+)`) @@ -48,6 +64,14 @@ type Issue struct { // Owner and Repo are populated in multi-repo mode to identify the source repo. Owner string `json:"Owner,omitempty"` Repo string `json:"Repo,omitempty"` + // AuthorAssociation is the GitHub-computed relationship between the + // Issue's author and the repository (e.g. "OWNER", "MEMBER", + // "COLLABORATOR", "CONTRIBUTOR", "FIRST_TIME_CONTRIBUTOR", "NONE"). + // Issues whose association is not in the trusted allowlist are already + // filtered out before this struct is returned by ListOpenIssues / + // ListAllIssues (see (*Client).isTrustedAuthor); the field is retained + // on the returned value for observability/testing. + AuthorAssociation string `json:"AuthorAssociation,omitempty"` } // PRInfo holds a summary of an open pull request returned by ListOpenPRs. @@ -94,6 +118,14 @@ type Client struct { gh *gogithub.Client owner string repo string + // trustedAssociations is the allowlist of author_association values + // (upper-cased) whose Issues are surfaced by ListOpenIssues/ListAllIssues. + // nil/empty means "not configured" and DefaultTrustedAuthorAssociations + // is used — see isTrustedAuthor. This keeps the safe default in effect + // both for callers that never call SetTrustedAuthorAssociations (e.g. + // existing tests constructing Client via struct literal) and for + // harness.toml deployments that omit the [security] section entirely. + trustedAssociations []string } func NewClient(token, owner, repo string) *Client { @@ -106,9 +138,46 @@ func NewClient(token, owner, repo string) *Client { } } +// SetTrustedAuthorAssociations configures the allowlist of GitHub +// "author_association" values (case-insensitive) whose Issues are surfaced +// by ListOpenIssues/ListAllIssues. Passing nil or an empty slice restores +// the safe default (DefaultTrustedAuthorAssociations) rather than allowing +// every author — there is intentionally no way to configure "allow +// everyone" through this method (see Issue #178). +func (c *Client) SetTrustedAuthorAssociations(assocs []string) { + c.trustedAssociations = assocs +} + +// isTrustedAuthor reports whether the given author_association value (as +// returned by the GitHub API, e.g. "OWNER", "COLLABORATOR", "NONE") is +// allowed to have its Issues surfaced to the HERMIT loop. Comparison is +// case-insensitive. When no allowlist has been configured on the client, +// DefaultTrustedAuthorAssociations is used — an unset/misconfigured +// allowlist never falls back to "allow everyone". +func (c *Client) isTrustedAuthor(association string) bool { + allowed := c.trustedAssociations + if len(allowed) == 0 { + allowed = DefaultTrustedAuthorAssociations + } + for _, a := range allowed { + if strings.EqualFold(a, association) { + return true + } + } + return false +} + // listOpenIssuesFromRepo fetches open issues from a specific owner/repo pair, // optionally filtering by label. Each returned Issue has its Owner and Repo // fields set to the provided values. +// +// Issues whose author does not have a trusted author_association (see +// isTrustedAuthor/DefaultTrustedAuthorAssociations) are excluded from the +// result — HERMIT runs unattended with broad local tool access, and public +// repos accept Issues from anyone, so an untrusted Issue body must never +// reach the loop as an implicit instruction (Issue #178). Excluded issues +// are not silently dropped: each one is logged so a human operator watching +// HERMIT's logs can notice. func (c *Client) listOpenIssuesFromRepo(owner, repo, label string) ([]Issue, error) { opts := &gogithub.IssueListByRepoOptions{ State: "open", @@ -125,17 +194,24 @@ func (c *Client) listOpenIssuesFromRepo(owner, repo, label string) ([]Issue, err if i.PullRequestLinks != nil { continue // skip PRs } + association := i.GetAuthorAssociation() + if !c.isTrustedAuthor(association) { + log.Printf("security: excluding issue #%d in %s/%s from the queue: author_association=%q is not in the trusted allowlist", + i.GetNumber(), owner, repo, association) + continue + } var labels []string for _, l := range i.Labels { labels = append(labels, l.GetName()) } result = append(result, Issue{ - Number: i.GetNumber(), - Title: i.GetTitle(), - Body: i.GetBody(), - Labels: labels, - Owner: owner, - Repo: repo, + Number: i.GetNumber(), + Title: i.GetTitle(), + Body: i.GetBody(), + Labels: labels, + Owner: owner, + Repo: repo, + AuthorAssociation: association, }) } return result, nil diff --git a/internal/github/multi_repo_test.go b/internal/github/multi_repo_test.go index 6b84a42..7ec3739 100644 --- a/internal/github/multi_repo_test.go +++ b/internal/github/multi_repo_test.go @@ -63,7 +63,7 @@ func TestListOpenIssues_SingleRepo(t *testing.T) { mux.HandleFunc("/repos/owner/repo/issues", func(w http.ResponseWriter, r *http.Request) { w.Header().Set("Content-Type", "application/json") json.NewEncoder(w).Encode(issueListResponse([]map[string]any{ - {"number": 1, "title": "Issue one", "body": "body", "labels": []map[string]any{}}, + {"number": 1, "title": "Issue one", "body": "body", "labels": []map[string]any{}, "author_association": "OWNER"}, {"number": 2, "title": "Issue two", "body": "body2", "labels": []map[string]any{}, "pull_request": map[string]any{"url": "http://example.com"}}, // should be skipped })) @@ -100,7 +100,7 @@ func TestListOpenIssues_LabelFilter(t *testing.T) { json.NewEncoder(w).Encode([]map[string]any{ {"number": 5, "title": "Labeled issue", "body": "", "labels": []map[string]any{ {"name": "hermit"}, - }}, + }, "author_association": "MEMBER"}, }) }) @@ -138,7 +138,7 @@ func TestListOpenIssuesFromRepo_SetsOwnerAndRepo(t *testing.T) { mux.HandleFunc("/repos/owner/repo/issues", func(w http.ResponseWriter, r *http.Request) { w.Header().Set("Content-Type", "application/json") json.NewEncoder(w).Encode([]map[string]any{ - {"number": 10, "title": "Test", "body": "", "labels": []map[string]any{}}, + {"number": 10, "title": "Test", "body": "", "labels": []map[string]any{}, "author_association": "OWNER"}, }) }) @@ -166,13 +166,13 @@ func TestListAllIssues_MultiRepo(t *testing.T) { mux.HandleFunc("/repos/org/frontend/issues", func(w http.ResponseWriter, r *http.Request) { w.Header().Set("Content-Type", "application/json") json.NewEncoder(w).Encode([]map[string]any{ - {"number": 1, "title": "Frontend issue", "body": "", "labels": []map[string]any{}}, + {"number": 1, "title": "Frontend issue", "body": "", "labels": []map[string]any{}, "author_association": "OWNER"}, }) }) mux.HandleFunc("/repos/org/backend/issues", func(w http.ResponseWriter, r *http.Request) { w.Header().Set("Content-Type", "application/json") json.NewEncoder(w).Encode([]map[string]any{ - {"number": 2, "title": "Backend issue", "body": "", "labels": []map[string]any{}}, + {"number": 2, "title": "Backend issue", "body": "", "labels": []map[string]any{}, "author_association": "COLLABORATOR"}, }) }) @@ -211,7 +211,7 @@ func TestListAllIssues_EmptyRepos_FallbackToPrimary(t *testing.T) { mux.HandleFunc("/repos/owner/repo/issues", func(w http.ResponseWriter, r *http.Request) { w.Header().Set("Content-Type", "application/json") json.NewEncoder(w).Encode([]map[string]any{ - {"number": 99, "title": "Primary repo issue", "body": "", "labels": []map[string]any{}}, + {"number": 99, "title": "Primary repo issue", "body": "", "labels": []map[string]any{}, "author_association": "OWNER"}, }) }) @@ -241,7 +241,7 @@ func TestListAllIssues_MultiRepo_WithLabelFilter(t *testing.T) { } w.Header().Set("Content-Type", "application/json") json.NewEncoder(w).Encode([]map[string]any{ - {"number": 3, "title": "Tagged issue", "body": "", "labels": []map[string]any{{"name": "hermit"}}}, + {"number": 3, "title": "Tagged issue", "body": "", "labels": []map[string]any{{"name": "hermit"}}, "author_association": "OWNER"}, }) }) diff --git a/internal/github/req_test.go b/internal/github/req_test.go index 8a30cf6..da5f49d 100644 --- a/internal/github/req_test.go +++ b/internal/github/req_test.go @@ -29,7 +29,7 @@ func TestREQ003_ListIssues_LabelFilterAndPRsSkipped(t *testing.T) { json.NewEncoder(w).Encode([]map[string]any{ {"number": 5, "title": "Labeled issue", "body": "", "labels": []map[string]any{ {"name": "hermit"}, - }}, + }, "author_association": "OWNER"}, {"number": 6, "title": "A PR, not an issue", "body": "", "labels": []map[string]any{}, "pull_request": map[string]any{"url": "http://example.com"}}, }) @@ -49,3 +49,192 @@ func TestREQ003_ListIssues_LabelFilterAndPRsSkipped(t *testing.T) { t.Errorf("issues[0].Number = %d, want 5", issues[0].Number) } } + +// REQ-named tests for REQ-017 (Issue #178): list_issues only surfaces Issues +// from authors with a trusted author_association. + +// TestREQ017_ListOpenIssues_ExcludesUntrustedAuthors verifies that Issues +// whose author_association is not in the trusted allowlist (default: +// OWNER/MEMBER/COLLABORATOR) are excluded from ListOpenIssues, while trusted +// ones are kept. +func TestREQ017_ListOpenIssues_ExcludesUntrustedAuthors(t *testing.T) { + mux := http.NewServeMux() + mux.HandleFunc("/repos/owner/repo/issues", func(w http.ResponseWriter, r *http.Request) { + w.Header().Set("Content-Type", "application/json") + json.NewEncoder(w).Encode([]map[string]any{ + {"number": 1, "title": "From owner", "body": "b", "labels": []map[string]any{}, "author_association": "OWNER"}, + {"number": 2, "title": "From member", "body": "b", "labels": []map[string]any{}, "author_association": "MEMBER"}, + {"number": 3, "title": "From collaborator", "body": "b", "labels": []map[string]any{}, "author_association": "COLLABORATOR"}, + {"number": 4, "title": "From contributor", "body": "b", "labels": []map[string]any{}, "author_association": "CONTRIBUTOR"}, + {"number": 5, "title": "From first-timer", "body": "b", "labels": []map[string]any{}, "author_association": "FIRST_TIME_CONTRIBUTOR"}, + {"number": 6, "title": "From nobody", "body": "b", "labels": []map[string]any{}, "author_association": "NONE"}, + }) + }) + + client, teardown := newTestClient(t, mux) + defer teardown() + + issues, err := client.ListOpenIssues("") + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + + var got []int + for _, iss := range issues { + got = append(got, iss.Number) + } + want := []int{1, 2, 3} + if len(got) != len(want) { + t.Fatalf("expected issues %v, got %v", want, got) + } + for i, n := range want { + if got[i] != n { + t.Errorf("issue index %d: got #%d, want #%d (full result: %v)", i, got[i], n, got) + } + } +} + +// TestREQ017_ListOpenIssues_DefaultsToSafeAllowlistWhenUnconfigured verifies +// that a Client which never had SetTrustedAuthorAssociations called (i.e. +// harness.toml has no [security] section) still applies the safe default +// allowlist rather than allowing every author through. +func TestREQ017_ListOpenIssues_DefaultsToSafeAllowlistWhenUnconfigured(t *testing.T) { + mux := http.NewServeMux() + mux.HandleFunc("/repos/owner/repo/issues", func(w http.ResponseWriter, r *http.Request) { + w.Header().Set("Content-Type", "application/json") + json.NewEncoder(w).Encode([]map[string]any{ + {"number": 1, "title": "Untrusted", "body": "b", "labels": []map[string]any{}, "author_association": "NONE"}, + }) + }) + + // newTestClient never calls SetTrustedAuthorAssociations, mirroring a + // harness.toml with no [security] section. + client, teardown := newTestClient(t, mux) + defer teardown() + + issues, err := client.ListOpenIssues("") + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if len(issues) != 0 { + t.Errorf("expected untrusted-author issue to be excluded by default, got %+v", issues) + } +} + +// TestREQ017_SetTrustedAuthorAssociations_EmptySliceFallsBackToDefault +// verifies that explicitly configuring an empty allowlist (e.g. +// harness.toml's trusted_author_associations left as `[]` or omitted, which +// TOML decoding leaves as a nil/empty slice) does not degrade into "allow +// everyone" — it must resolve to the same safe default as never calling +// SetTrustedAuthorAssociations at all. +func TestREQ017_SetTrustedAuthorAssociations_EmptySliceFallsBackToDefault(t *testing.T) { + mux := http.NewServeMux() + mux.HandleFunc("/repos/owner/repo/issues", func(w http.ResponseWriter, r *http.Request) { + w.Header().Set("Content-Type", "application/json") + json.NewEncoder(w).Encode([]map[string]any{ + {"number": 1, "title": "Untrusted", "body": "b", "labels": []map[string]any{}, "author_association": "NONE"}, + {"number": 2, "title": "Trusted", "body": "b", "labels": []map[string]any{}, "author_association": "OWNER"}, + }) + }) + + client, teardown := newTestClient(t, mux) + defer teardown() + client.SetTrustedAuthorAssociations(nil) + + issues, err := client.ListOpenIssues("") + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if len(issues) != 1 || issues[0].Number != 2 { + t.Errorf("expected only the OWNER-authored issue #2, got %+v", issues) + } +} + +// TestREQ017_SetTrustedAuthorAssociations_CustomAllowlistIsHonored verifies +// that an explicit, non-empty allowlist configured via +// SetTrustedAuthorAssociations (i.e. harness.toml's +// [security].trusted_author_associations) is applied instead of the +// built-in default, including case-insensitive matching. +func TestREQ017_SetTrustedAuthorAssociations_CustomAllowlistIsHonored(t *testing.T) { + mux := http.NewServeMux() + mux.HandleFunc("/repos/owner/repo/issues", func(w http.ResponseWriter, r *http.Request) { + w.Header().Set("Content-Type", "application/json") + json.NewEncoder(w).Encode([]map[string]any{ + {"number": 1, "title": "Contributor", "body": "b", "labels": []map[string]any{}, "author_association": "CONTRIBUTOR"}, + {"number": 2, "title": "None", "body": "b", "labels": []map[string]any{}, "author_association": "NONE"}, + }) + }) + + client, teardown := newTestClient(t, mux) + defer teardown() + // Lower-case on purpose to verify case-insensitive comparison. + client.SetTrustedAuthorAssociations([]string{"contributor"}) + + issues, err := client.ListOpenIssues("") + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if len(issues) != 1 || issues[0].Number != 1 { + t.Errorf("expected only the CONTRIBUTOR-authored issue #1 under custom allowlist, got %+v", issues) + } +} + +// TestREQ017_ListAllIssues_ExcludesUntrustedAuthorsAcrossRepos verifies the +// multi-repo path (ListAllIssues) applies the same author_association +// filtering as the single-repo path. +func TestREQ017_ListAllIssues_ExcludesUntrustedAuthorsAcrossRepos(t *testing.T) { + mux := http.NewServeMux() + mux.HandleFunc("/repos/org/frontend/issues", func(w http.ResponseWriter, r *http.Request) { + w.Header().Set("Content-Type", "application/json") + json.NewEncoder(w).Encode([]map[string]any{ + {"number": 1, "title": "Trusted", "body": "", "labels": []map[string]any{}, "author_association": "MEMBER"}, + {"number": 2, "title": "Untrusted", "body": "", "labels": []map[string]any{}, "author_association": "NONE"}, + }) + }) + mux.HandleFunc("/repos/org/backend/issues", func(w http.ResponseWriter, r *http.Request) { + w.Header().Set("Content-Type", "application/json") + json.NewEncoder(w).Encode([]map[string]any{ + {"number": 3, "title": "Untrusted", "body": "", "labels": []map[string]any{}, "author_association": "FIRST_TIME_CONTRIBUTOR"}, + }) + }) + + client, teardown := newTestClientFor(t, mux, "org", "frontend") + defer teardown() + + repos := []RepoConfig{ + {Owner: "org", Repo: "frontend"}, + {Owner: "org", Repo: "backend"}, + } + issues, err := client.ListAllIssues(repos) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if len(issues) != 1 || issues[0].Number != 1 { + t.Errorf("expected only issue #1 (MEMBER) to survive across both repos, got %+v", issues) + } +} + +// TestREQ017_Issue_AuthorAssociationFieldIsPopulated verifies that surviving +// Issues carry their author_association value through to the returned +// struct, so callers/operators can observe why an Issue was (or would be) +// trusted. +func TestREQ017_Issue_AuthorAssociationFieldIsPopulated(t *testing.T) { + mux := http.NewServeMux() + mux.HandleFunc("/repos/owner/repo/issues", func(w http.ResponseWriter, r *http.Request) { + w.Header().Set("Content-Type", "application/json") + json.NewEncoder(w).Encode([]map[string]any{ + {"number": 1, "title": "From owner", "body": "b", "labels": []map[string]any{}, "author_association": "OWNER"}, + }) + }) + + client, teardown := newTestClient(t, mux) + defer teardown() + + issues, err := client.listOpenIssuesFromRepo("owner", "repo", "") + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if len(issues) != 1 || issues[0].AuthorAssociation != "OWNER" { + t.Errorf("expected AuthorAssociation=OWNER, got %+v", issues) + } +}