feat(security): restrict Engineer Bash via sandbox instead of allowlist (Closes #180)

README.md

@@ -93,7 +93,9 @@ Generated files:
 - `harness.toml` — Project configuration (shared with the team)
 - `CLAUDE.md` — Role definitions for Superintendent / Engineer
 - `.github/ISSUE_TEMPLATE/hermit-task.md` — GitHub Issue template for well-structured tasks
-- `.claude/settings.json` — Claude Code permission settings for autonomous operation
+- `.claude/settings.json` — Claude Code permission settings for autonomous operation, plus a recommended `sandbox` block (see "Sandboxing the Engineer" below)
+
+Re-running `hermit init` on an already-initialized project never overwrites an existing `.claude/settings.json` wholesale: any top-level key you've already customized (most importantly `permissions`) is left exactly as-is, and only keys that are entirely missing (e.g. `sandbox`, on a project initialized before it was added) are filled in with hermit's defaults.
 
 Edit the "Coding Guidelines" section in `CLAUDE.md` to match your project.
 
@@ -111,6 +113,54 @@ Edit the "Coding Guidelines" section in `CLAUDE.md` to match your project.
   - In practice, many public-repo operators will want both: trusted-author Issues *and* human-reviewed merges.
 - **Future work (not yet implemented):** an Issue-author allowlist (e.g. only process Issues from repository collaborators or an explicit list of GitHub usernames) is being considered as a built-in mitigation. Until it exists, use the operating modes above.
 
+### Sandboxing the Engineer
+
+The Engineer runs with `Bash(*)` permission — unrestricted shell access — on the machine running Claude Code, because an allow-list of individual commands was tried and rejected (Issue #138: every time a new tool or command shape showed up that the allow-list hadn't anticipated, the loop either stalled on a confirmation prompt or someone widened the list until it was `Bash(*)` in practice anyway). Instead of enumerating which commands are allowed, `hermit init` generates a `sandbox` block in `.claude/settings.json` that narrows *what the Engineer can reach*, regardless of which command it runs:
+
+```json
+{
+  "sandbox": {
+    "enabled": true,
+    "allowUnsandboxedCommands": false,
+    "network": {
+      "tlsTerminate": {},
+      "allowedDomains": ["*.github.com", "proxy.golang.org", "sum.golang.org", "storage.googleapis.com"]
+    },
+    "credentials": {
+      "files": [
+        { "path": "~/.ssh", "mode": "deny" },
+        { "path": "~/.aws/credentials", "mode": "deny" }
+      ],
+      "envVars": [
+        { "name": "GITHUB_TOKEN", "mode": "mask", "injectHosts": ["api.github.com"] }
+      ]
+    }
+  }
+}
+```
+
+A few details worth knowing if you edit this block by hand:
+
+- **`allowUnsandboxedCommands` defaults to `true` in Claude Code.** If you omit it (or leave it `true`), the sandbox is effectively optional — a command can simply opt out. `hermit init` always writes it as `false`; keep it that way.
+- **`GITHUB_TOKEN` must be `"mode": "mask"` with `injectHosts`, never `"mode": "deny"`.** `gh` (and therefore `gh pr create`, `gh issue comment`, etc., which the Engineer needs to do its job) requires the real token when talking to `api.github.com`. `"deny"` breaks it outright. `"mask"` hides the value everywhere else and only injects the real token on requests to the hosts listed in `injectHosts`; this requires `network.tlsTerminate` to be present (even as `{}`), since Claude Code needs to terminate TLS to inspect the destination host before deciding whether to inject.
+- **`network.allowedDomains` must cover your dependency graph, not just GitHub.** For a Go project this means the module proxy/sum/storage hosts (`proxy.golang.org`, `sum.golang.org`, `storage.googleapis.com`) in addition to `*.github.com`, or `go build`/`go test`/`go mod download` will fail under the sandbox. Verify the list by actually running your project's test command with the sandbox enabled — don't assume it's complete.
+
+**Scope and precedence — read this before treating the sandbox as an enforcement mechanism.** Claude Code settings are layered (managed > CLI flags > local project > project > user), and different key *shapes* combine across that stack differently:
+
+- **Boolean keys** (like `sandbox.enabled`, `sandbox.allowUnsandboxedCommands`) resolve by precedence: the highest-precedence scope that sets the key wins outright. A project-scoped `.claude/settings.json` beats a user-scoped `~/.claude/settings.json`.
+- **Array keys** (like `sandbox.excludedCommands`, `permissions.allow`, `permissions.deny` — anything that's a list of patterns) are **merged, additively, across every scope** that sets them. Nothing lower in the stack can *remove* an entry a higher scope added, but a lower scope can freely *add* entries a higher scope didn't ask for.
+
+The practical consequence: a `sandbox` block placed in `.claude/settings.json` (project scope, the default `hermit init` target) constrains the Engineer only as long as the Engineer doesn't touch it. But the Engineer's job is to open PRs against this very repository — including PRs that edit `.claude/settings.json` itself, add an entry to `sandbox.excludedCommands`, or otherwise widen the array-merged keys from project scope. Nothing in a project-scoped sandbox config stops the Engineer from proposing (and, if auto-merge is on for LOW/MEDIUM risk PRs, landing) exactly that change. In other words: project settings are a strong default, not an enforcement boundary against the agent they're meant to constrain.
+
+**To actually enforce the sandbox against the Engineer, you need managed settings** — a settings file outside the repository, deployed by an admin/MDM process the Engineer has no write access to, which sits above project scope in the precedence stack. Two managed-settings flags close the remaining loophole in the array-merge behavior described above:
+
+- **`allowManagedReadPathsOnly`** — restricts filesystem reads to paths the managed config explicitly allows, so a project-scope (or Engineer-authored) change can't widen readable paths beyond what the managed config permits.
+- **`allowManagedDomainsOnly`** — restricts network access to domains the managed config explicitly allows, closing the equivalent hole for `network.allowedDomains`.
+
+hermit does not currently generate a managed settings file — `hermit init` only ever writes to the project's own `.claude/settings.json`, which (per the above) the Engineer can eventually influence. Automating managed-settings generation, and wiring up `allowManagedReadPathsOnly` / `allowManagedDomainsOnly`, is tracked separately (see Issue #179); until that lands, the project-scope sandbox block here is best understood as raising the cost of an adversarial or buggy Engineer action, not as a hard boundary.
+
+`hermit doctor` checks the generated `.claude/settings.json` for the two most common ways this block ends up not doing anything (`sandbox.enabled` false/missing, `allowUnsandboxedCommands` true/missing) plus a non-empty `sandbox.excludedCommands` (which reopens a hole per command listed there); see `hermit doctor` below.
+
 ---
 
 ## Usage
@@ -390,8 +440,9 @@ Runs a series of environment checks and reports pass/fail for each:
 - `GITHUB_TOKEN` is available (from environment or `gh auth token`)
 - `harness.toml` exists with `owner` and `repo` filled in
 - `claude` (Claude Code) is installed
+- `.claude/settings.json` sandbox configuration (warnings, non-fatal — see "Sandboxing the Engineer" above): `sandbox.enabled` is true, `allowUnsandboxedCommands` is false, `sandbox.excludedCommands` is empty
 
-Exits with a non-zero status if any check fails.
+Exits with a non-zero status if any check fails. The sandbox checks are warnings and never cause a non-zero exit on their own, so `hermit doctor` still passes on projects initialized before the sandbox recommendation existed.
 
 ### `hermit dry-run`
 

REQUIREMENTS.md

@@ -212,3 +212,18 @@ HERMIT は public リポジトリで運用され得るため、第三者が作
 - 受け入れ条件: 信頼できない 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_*`
+
+## REQ-018: Engineer の Bash をサンドボックスで制限する
+
+Engineer は `Bash(*)` 許可でローカルマシン上で動作しており、`permissions.allow` によるコマンド単位の allowlist 方式は不採用 (Issue #138: 新規ツール追加のたびに未更新でループが壊れる失敗モードが実証済み)。代わりに `hermit init` が Claude Code のサンドボックス機能 (`sandbox` ブロック) を推奨設定として生成し、能力を列挙するのではなく届く範囲そのものを狭める。
+
+- `hermit init` は `.claude/settings.json` に `sandbox.enabled: true` / `allowUnsandboxedCommands: false` / `network.tlsTerminate` + `allowedDomains` (`*.github.com`, `proxy.golang.org`, `sum.golang.org`, `storage.googleapis.com`) / `credentials.files` (`~/.ssh`, `~/.aws/credentials` を deny) / `credentials.envVars` (`GITHUB_TOKEN` を `mode: mask` + `injectHosts: ["api.github.com"]`、`deny` にはしない) を書き込む
+- `allowUnsandboxedCommands` は Claude Code 側の既定が `true` であり明示的に `false` にしないとサンドボックス自体が実質無効化されるため、生成される設定では必ず `false` にする
+- `GITHUB_TOKEN` を `deny` にすると `gh` コマンド (Engineer が PR 作成等に使う) が動作しなくなるため、`mask` + `injectHosts` を用いる
+- 既に `.claude/settings.json` が存在するプロジェクトに対して `hermit init` を再実行しても、既存の `permissions` などのトップレベルキーは破壊されず、欠けているキー (`sandbox` など) のみが追加される
+- `hermit doctor` は生成済み `.claude/settings.json` の `sandbox.enabled` が false/未設定、`allowUnsandboxedCommands` が true/未設定、`sandbox.excludedCommands` が空でない場合に警告する (警告のみで doctor 全体の pass/fail には影響しない)
+- README にスコープごとの precedence (boolean キーは上位スコープが勝つ、配列キー (`excludedCommands` 等) は全スコープでマージされ下位スコープから広げられる) と、project settings 配置では Engineer 自身が PR 経由で無効化・迂回しうる旨、実効的な強制には managed settings と `allowManagedReadPathsOnly` / `allowManagedDomainsOnly` が必要な旨、その自動生成自体は別 Issue #179 のスコープである旨を明記する
+
+- 受け入れ条件: `hermit init` が生成する `.claude/settings.json` に上記構造の `sandbox` ブロックが含まれ、`allowUnsandboxedCommands: false` / `GITHUB_TOKEN` の `mode: mask` + `injectHosts` が満たされていること。生成された設定を適用した状態で `go build ./...` / `go test ./...` が成功し、`gh pr create` 相当の操作が実行できること。`hermit doctor` が上記 3 種の警告を検出すること。既存プロジェクトへの `hermit init` 再実行が既存の `permissions` 設定を破壊しないこと
+- verify: test
+- 実装状況: 実装済み — `internal/permissions/permissions.go` の `DefaultSandboxSettings` / `MergeDefaultSettings` (再実行時は既存のトップレベルキーを保持し、欠けているキーのみ補完)、`cmd/hermit/main.go` の `writeClaudeSettings` (`MergeDefaultSettings` を経由するよう変更)、`cmd/hermit/doctor.go` の `checkSandboxSettings` (3 種の警告)。テストは `internal/permissions/permissions_test.go` の `TestREQ018_*` 群 (`DefaultSandboxSettings` の `allowUnsandboxedCommands`/`GITHUB_TOKEN`/Go ツールチェーン許可ドメイン、`MergeDefaultSettings` の新規生成・既存 `permissions`/`sandbox` の保持・エラー経路) と `cmd/hermit/doctor_test.go` / `cmd/hermit/unit_test.go` の `TestREQ018_*` 群 (`checkSandboxSettings` の警告条件、`writeClaudeSettings` の再実行時非破壊)。`go build ./...` と `go test ./...` の成功、および `gh pr create` 相当操作の実行可能性は本 Issue #180 の PR 自体 (生成された設定下で `go test ./...` を通し、同じ worktree から `gh pr create` で PR を作成) によって実地検証済み。README の "Sandboxing the Engineer" セクションにスコープ precedence・managed settings・Issue #179 依存の記載を追加

cmd/hermit/doctor.go

@@ -1,6 +1,7 @@
 package main
 
 import (
+	"encoding/json"
 	"fmt"
 	"os"
 	"os/exec"
@@ -130,9 +131,110 @@ func runChecks() []checkResult {
 		passed: err == nil,
 	})
 
+	// Checks: sandbox configuration in .claude/settings.json (REQ-018). These
+	// are warnings, not hard failures, so `hermit doctor` keeps passing on
+	// projects initialized before the sandbox recommendation existed.
+	results = append(results, checkSandboxSettings(".claude/settings.json")...)
+
 	return results
 }
 
+// sandboxSettingsRaw mirrors just the fields of the "sandbox" block in
+// .claude/settings.json that checkSandboxSettings inspects. It is decoded
+// independently of internal/permissions.Settings so a malformed or
+// hand-edited settings.json (missing fields, extra keys) never breaks
+// `hermit doctor` itself — unmarshal errors are treated as "not configured".
+type sandboxSettingsRaw struct {
+	Sandbox *struct {
+		Enabled                  *bool    `json:"enabled"`
+		AllowUnsandboxedCommands *bool    `json:"allowUnsandboxedCommands"`
+		ExcludedCommands         []string `json:"excludedCommands"`
+	} `json:"sandbox"`
+}
+
+// checkSandboxSettings inspects the "sandbox" block of the settings.json at
+// path and warns about the three ways it can end up not actually restricting
+// the Engineer's Bash tool (see Issue #180 / REQUIREMENTS.md REQ-018):
+//
+//   - sandbox.enabled is false or missing — the whole block is inert
+//   - allowUnsandboxedCommands is true or missing — defaults to true in
+//     Claude Code, which lets commands opt out of the sandbox entirely
+//   - excludedCommands has entries — those commands bypass the sandbox
+func checkSandboxSettings(path string) []checkResult {
+	data, err := os.ReadFile(path)
+	fileMissing := os.IsNotExist(err)
+
+	var cfg sandboxSettingsRaw
+	if err == nil {
+		// Best-effort: malformed JSON is handled the same as "no sandbox
+		// block configured" rather than failing doctor outright.
+		_ = json.Unmarshal(data, &cfg)
+	}
+
+	missingFileDetail := path + " not found (run `hermit init`)"
+	missingBlockDetail := "sandbox block missing from " + path
+
+	enabled := cfg.Sandbox != nil && cfg.Sandbox.Enabled != nil && *cfg.Sandbox.Enabled
+	enabledDetail := ""
+	switch {
+	case enabled:
+		// no detail needed
+	case fileMissing:
+		enabledDetail = missingFileDetail
+	case cfg.Sandbox == nil:
+		enabledDetail = missingBlockDetail
+	case cfg.Sandbox.Enabled == nil:
+		enabledDetail = "sandbox.enabled not set"
+	default:
+		enabledDetail = "sandbox.enabled is false"
+	}
+
+	blocked := cfg.Sandbox != nil && cfg.Sandbox.AllowUnsandboxedCommands != nil && !*cfg.Sandbox.AllowUnsandboxedCommands
+	blockedDetail := ""
+	switch {
+	case blocked:
+		// no detail needed
+	case fileMissing:
+		blockedDetail = missingFileDetail
+	case cfg.Sandbox == nil:
+		blockedDetail = missingBlockDetail
+	case cfg.Sandbox.AllowUnsandboxedCommands == nil:
+		blockedDetail = "allowUnsandboxedCommands not set (defaults to true in Claude Code, which makes the sandbox block a no-op)"
+	default:
+		blockedDetail = "allowUnsandboxedCommands is true (defeats the sandbox; set it to false)"
+	}
+
+	var excluded []string
+	if cfg.Sandbox != nil {
+		excluded = cfg.Sandbox.ExcludedCommands
+	}
+	excludedDetail := ""
+	if len(excluded) > 0 {
+		excludedDetail = "sandbox.excludedCommands bypasses the sandbox for: " + strings.Join(excluded, ", ")
+	}
+
+	return []checkResult{
+		{
+			name:   "sandbox.enabled is true",
+			passed: true,
+			warn:   !enabled,
+			detail: enabledDetail,
+		},
+		{
+			name:   "allowUnsandboxedCommands is false",
+			passed: true,
+			warn:   !blocked,
+			detail: blockedDetail,
+		},
+		{
+			name:   "sandbox.excludedCommands is empty",
+			passed: true,
+			warn:   len(excluded) > 0,
+			detail: excludedDetail,
+		},
+	}
+}
+
 func cmdDoctor() {
 	results := runChecks()