Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 18 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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`:
Expand Down
8 changes: 8 additions & 0 deletions REQUIREMENTS.md
Original file line number Diff line number Diff line change
Expand Up @@ -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_*`
1 change: 1 addition & 0 deletions cmd/hermit/dryrun.go
Original file line number Diff line number Diff line change
Expand Up @@ -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")
Expand Down
16 changes: 16 additions & 0 deletions cmd/hermit/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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)
Expand Down
10 changes: 10 additions & 0 deletions cmd/hermit/templates/harness.toml.tmpl
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
88 changes: 82 additions & 6 deletions internal/github/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package github
import (
"context"
"fmt"
"log"
"os/exec"
"regexp"
"strconv"
Expand All @@ -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+)`)
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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 {
Expand All @@ -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",
Expand All @@ -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
Expand Down
14 changes: 7 additions & 7 deletions internal/github/multi_repo_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}))
Expand Down Expand Up @@ -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"},
})
})

Expand Down Expand Up @@ -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"},
})
})

Expand Down Expand Up @@ -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"},
})
})

Expand Down Expand Up @@ -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"},
})
})

Expand Down Expand Up @@ -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"},
})
})

Expand Down
Loading
Loading