diff --git a/docs/ADRs/0090-runtime-neutral-sandbox-hooks-contract.md b/docs/ADRs/0090-runtime-neutral-sandbox-hooks-contract.md index 77214f7269..b24e2383e0 100644 --- a/docs/ADRs/0090-runtime-neutral-sandbox-hooks-contract.md +++ b/docs/ADRs/0090-runtime-neutral-sandbox-hooks-contract.md @@ -77,7 +77,8 @@ feature: - The contract is versioned (v1 = the scripts' current stdin/stdout fields). Verifying it against Claude Code surfaced two pre-existing gaps the runtimes.md matrix now records instead of a blanket ✓: the runner's - `settings.json` is not loaded from where it is written (#6358), and the + hook wiring was not loaded from where it was written (#6358 — since fixed + via `--settings`), and the PostToolUse payload differs (`tool_response`, `hookSpecificOutput.updatedToolOutput`, parallel execution — #6357). - Alongside this decision the runner's remaining Claude-specific branches diff --git a/docs/architecture.md b/docs/architecture.md index 6a8fe8c81d..ffca23e4f1 100644 --- a/docs/architecture.md +++ b/docs/architecture.md @@ -169,7 +169,7 @@ repo baseline and overrides) **Open questions:** -- Does the harness live inside the sandbox (configuring the agent from within its isolation boundary) or outside it (preparing the environment before the agent starts)? (Tool permissions are injected as a host-managed `.claude/settings.json` — configured outside, enforced inside; see [ADR 0027](ADRs/0027-allowed-and-disallowed-tools-for-agents.md). General harness placement remains open.) +- Does the harness live inside the sandbox (configuring the agent from within its isolation boundary) or outside it (preparing the environment before the agent starts)? (Security hooks are injected as a runner-owned `hooks.json` loaded via `--settings`; see [ADR 0027](ADRs/0027-allowed-and-disallowed-tools-for-agents.md). General harness placement remains open.) - How is codebase context assembled? (See [codebase-context.md](problems/codebase-context.md).) - How do we version and test harness configurations? (See [testing-agents.md](problems/testing-agents.md).) (Functional tests now test the full pipeline including harness-assembled configuration — [ADR 0052](ADRs/0052-functional-tests-for-agent-pipelines.md). Harness versioning remains open.) diff --git a/docs/guides/dev/cli-internals.md b/docs/guides/dev/cli-internals.md index dc6a2d792e..8539a24d14 100644 --- a/docs/guides/dev/cli-internals.md +++ b/docs/guides/dev/cli-internals.md @@ -429,11 +429,12 @@ Vendoring commit messages use title + body (upload and stale delete). `github st │ ┌──────────────────────────────────────────┐ │ │ │ Exec() — Run agent in sandbox │ │ │ │ │ │ -│ │ Command built by buildClaudeCommand(): │ │ +│ │ Command built by buildRunCommand(): │ │ │ │ cd {repoDir} && │ │ │ │ . {envFile} && │ │ │ │ claude --print --verbose │ │ │ │ --output-format stream-json │ │ +│ │ [--settings {hooksSettingsPath}] │ │ │ │ --model {model} │ │ │ │ --effort {effort} │ │ │ │ --agent {agent} │ │ diff --git a/docs/runtimes.md b/docs/runtimes.md index 4164e4ffe4..b3c58b1fb8 100644 --- a/docs/runtimes.md +++ b/docs/runtimes.md @@ -18,15 +18,15 @@ When adding a runtime, fill in the security matrix below and register it in `run |---------|---------------|-------------|-----------------|---------------------------| | **Host-side context injection scan** (DeBERTa / LLM Guard, unicode, SSRF patterns on repo context files) | Host + sandbox `scan context` | ✓ | N/A — stub | Requires sandbox image with ML models; harness `security.host_scanners` | | **Host-side runtime content scan** (agent def, SKILL.md, plugin JSON before upload) | Host (`scanRuntimeContent`) | ✓ | N/A — stub | Uses `security.InputPipeline()`; not part of `Runtime` interface — runner responsibility | -| **Tirith** (Bash command scanning) | Sandbox PreToolUse hook | wired; **not loaded by Claude Code today** (#6358) | N/A — stub | `tirith_check.py`; harness `security.sandbox_hooks.tirith`; fails open on missing binary/timeout unless `TIRITH_REQUIRED=1` | -| **SSRF pre-tool** | Sandbox PreToolUse hook | wired; not loaded (#6358) | N/A — stub | `ssrf_pretool.py`; default on | -| **Canary token detection** | Sandbox Pre/PostToolUse hooks | wired; not loaded (#6358); post-tool field mismatch (#6357) | N/A — stub | `canary_pretool.py` / `canary_posttool.py`; both inert unless `FULLSEND_CANARY_TOKEN` is set | -| **Secret redaction** | Sandbox PostToolUse hook | wired; not loaded (#6358); not effective even when loaded (#6357) | N/A — stub | `secret_redact_posttool.py` | -| **Unicode normalization** | Sandbox PostToolUse hook | wired; not loaded (#6358); not effective even when loaded (#6357) | N/A — stub | `unicode_posttool.py` | -| **Context suppression** | Sandbox PostToolUse hook | wired; not loaded (#6358); not effective even when loaded (#6357) | N/A — stub | `context_suppress_posttool.py` | -| **Tool allowlist** | Sandbox PreToolUse hook | opt-in; wired; not loaded (#6358) | N/A — stub | `tool_allowlist_pretool.py`; requires `FULLSEND_TOOL_ALLOWLIST` (fail-closed when unset) | +| **Tirith** (Bash command scanning) | Sandbox PreToolUse hook | ✓ (loaded via `--settings`, #6358) | N/A — stub | `tirith_check.py`; harness `security.sandbox_hooks.tirith`; fails open on missing binary/timeout unless `TIRITH_REQUIRED=1` | +| **SSRF pre-tool** | Sandbox PreToolUse hook | ✓ (e2e-guarded by `hooks-loaded.feature`) | N/A — stub | `ssrf_pretool.py`; default on | +| **Canary token detection** | Sandbox Pre/PostToolUse hooks | pre ✓; post-tool field mismatch (#6357) | N/A — stub | `canary_pretool.py` / `canary_posttool.py`; both inert unless `FULLSEND_CANARY_TOKEN` is set | +| **Secret redaction** | Sandbox PostToolUse hook | wired; not effective under Claude Code (#6357) | N/A — stub | `secret_redact_posttool.py` | +| **Unicode normalization** | Sandbox PostToolUse hook | wired; not effective under Claude Code (#6357) | N/A — stub | `unicode_posttool.py` | +| **Context suppression** | Sandbox PostToolUse hook | wired; not effective under Claude Code (#6357) | N/A — stub | `context_suppress_posttool.py` | +| **Tool allowlist** | Sandbox PreToolUse hook | opt-in; ✓ when enabled | N/A — stub | `tool_allowlist_pretool.py`; requires `FULLSEND_TOOL_ALLOWLIST` (fail-closed when unset) | | **Prompt injection (DeBERTa)** | Host Path A + sandbox Path B | ✓ | N/A — stub | Same scanner stack as context files when enabled in harness | -| **Sandbox tool hooks wiring** | `SandboxHooksBootstrap` type assert in `Bootstrap` | ✓ scripts + settings.json installed — but written to `/sandbox/workspace/.claude/`, which the CLI (cwd `/sandbox/workspace/`) does not read (#6358) | ✗ — `Bootstrap` is a stub; must wire `security.HookPlan` via OpenCode plugin hooks | Hook scripts and wiring plan are runtime-neutral (see [Sandbox hook contract](#sandbox-hook-contract)); a runtime that ignores `SandboxHooksBootstrap` installs **no** sandbox tool hooks — say so explicitly here | +| **Sandbox tool hooks wiring** | `SandboxHooksBootstrap` type assert in `Bootstrap` | ✓ scripts at `claude-config/hooks/`, wiring at `claude-config/hooks.json` via `--settings` (#6358) | ✗ — `Bootstrap` is a stub; must wire `security.HookPlan` via OpenCode plugin hooks | Hook scripts and wiring plan are runtime-neutral (see [Sandbox hook contract](#sandbox-hook-contract)); a runtime that ignores `SandboxHooksBootstrap` installs **no** sandbox tool hooks — say so explicitly here | | **Transcript / debug artifacts** | `TranscriptHandler` (+ optional `DebugLogNamer`) | ✓ (stream-json, `claude-debug.log`) | No-op — see #1935 | Format-specific; not shared across runtimes. Debug-log filename defaults to `agent-debug.log` unless the runtime implements `DebugLogNamer` | ### Fail modes @@ -52,12 +52,12 @@ A runtime whose `Bootstrap` does not type-assert `SandboxHooksBootstrap` will ** The hook scripts in `internal/security/hooks/*.py` are plain programs with no Claude Code dependency; Claude Code invokes them through `settings.json`. Any runtime can call them from its own tool-call interception point (OpenCode `tool.execute.before/after`, pi `tool_call`/`tool_result`, Cursor hooks, …). -- **Files:** `security.HookFiles(cfg)` returns `filename → script bytes` for the enabled hooks; `runtime.installHookScripts(sandbox, dir, cfg)` creates `dir` in the sandbox and uploads them there (executable) — any directory works. Claude uses `/sandbox/workspace/.claude/hooks/` (`security.SandboxHooksDir`). -- **Wiring:** `security.HookPlan(cfg)` returns ordered `HookGroup{Phase, Tools, Scripts}` entries. `Phase` is `PreToolUse` or `PostToolUse`; `Tools` are Claude Code tool names (`Bash`, `Read`, `WebFetch`, `*` = all) — runtimes with other names translate before matching (see #608). **Adapters must run the `Scripts` of one group sequentially in the listed order, feeding each script's modified result to the next** (the PostToolUse order suppress → unicode → redact is a security invariant). `GenerateClaudeSettings` is rendered from `HookPlan`, so the two cannot diverge. +- **Files:** `security.HookFiles(cfg)` returns `filename → script bytes` for the enabled hooks; `runtime.installHookScripts(sandbox, dir, cfg)` creates `dir` in the sandbox and uploads them there (executable) — any directory works. Claude uses `/sandbox/claude-config/hooks/` (`security.SandboxHooksDir`), with the wiring at `/sandbox/claude-config/hooks.json` (`security.SandboxHooksSettings`) loaded via `--settings`. +- **Wiring:** `security.HookPlan(cfg)` returns ordered `HookGroup{Phase, Tools, Scripts}` entries. `Phase` is `PreToolUse` or `PostToolUse`; `Tools` are Claude Code tool names (`Bash`, `Read`, `WebFetch`, `*` = all) — runtimes with other names translate before matching (see #608). **Adapters must run the `Scripts` of one group sequentially in the listed order, feeding each script's modified result to the next** (the PostToolUse order suppress → unicode → redact is a security invariant). `GenerateHooksConfig` is rendered from `HookPlan`, so the two cannot diverge. - **Wire protocol (per script):** JSON on stdin — `{"tool_name": ..., "tool_input": {...}}` for PreToolUse, plus `"tool_result"` for PostToolUse. Exit `0` = allow. *Blocking* scripts (all PreToolUse scripts, and `canary_posttool.py`) exit `1` and print `{"decision":"block","reason":"..."}` on stdout; the adapter must stop the tool call (or, post-tool, drop the result) and surface the reason. *Sanitizing* PostToolUse scripts (`context_suppress`, `unicode`, `secret_redact`) always exit `0` and print `{"tool_result": }` when they changed something; empty stdout = unchanged. - **Fail modes:** blocking scripts fail **closed** on malformed JSON or oversized input (> 10 × 1024 × 1024 characters, read from text-mode stdin) — they block. Empty/whitespace-only stdin is treated as "no tool call" and allowed by every script; a payload without `tool_name` blocks only in the allowlist hook. `tirith_check.py` fails **open** when the `tirith` binary is missing, times out or errors, unless `TIRITH_REQUIRED=1` (which `appendHookEnv` writes when Tirith is enabled — adapters must make sure it reaches the script). Sanitizing scripts fail **open** — malformed or oversized input is passed through unchanged (exit 0, empty stdout; the unicode hook logs an `input_truncated` finding). Adapters must not treat a sanitizer's empty stdout as an error. - **Environment:** `runtime.appendHookEnv` writes `TIRITH_FAIL_ON` / `TIRITH_REQUIRED` into `/sandbox/workspace/.env`; the runtime must launch the scripts with that file sourced (Claude's run command does). Scripts also read `FULLSEND_TRACE_ID`, `FULLSEND_TOOL_ALLOWLIST` (allowlist hook, fail-closed when unset) and `FULLSEND_CANARY_TOKEN` (both canary hooks are no-ops when it is empty; supply it via harness `env.sandbox`/`host_files`), and write findings to `/sandbox/workspace/.security/findings.jsonl`. -- **Claude Code caveats (#6358, #6357):** (1) *Loading* — the runner writes `settings.json` to `/sandbox/workspace/.claude/` but starts the CLI from `/sandbox/workspace/`; Claude Code reads project settings only from `/.claude/` (plus `$CLAUDE_CONFIG_DIR/settings.json`, managed settings and `--settings`), so **today no sandbox tool hook is loaded at all** — verified from a `--debug` run (`Hooks: Found 0 total hooks in registry`); tracked in #6358. (2) *Payload* — Claude Code's PostToolUse input carries the output as `tool_response` (the scripts read `tool_result`), replacing output requires `hookSpecificOutput.updatedToolOutput` (the scripts print a bare `tool_result`), and all matching hooks run in parallel with no output chaining; tracked in #6357. (3) *Blocking* — Claude Code keys on the stdout JSON on any exit code (`decision:"block"` is deprecated for PreToolUse but still maps to `deny`) and treats a bare exit `1` as non-blocking (exit `2` is its own blocking code); a local control run confirmed the scripts' "exit 1 + `{"decision":"block"}`" convention does block once the settings are loaded. Net: after #6358, the PreToolUse half of the contract is effective under Claude Code; the PostToolUse scripts additionally need #6357. +- **Claude Code caveats (#6357):** (1) *Loading* — fixed by #6358: the hook wiring is written to the runner-owned `/sandbox/claude-config/hooks.json` and passed explicitly via `--settings`, so it loads regardless of the CLI's working directory (previously it sat unread in `/sandbox/workspace/.claude/`); the `hooks-loaded.feature` behaviour scenario guards the "silently not loaded" regression class. Note Claude Code still auto-loads a target repo's own `/.claude/settings.json` hooks from `` — a separate exposure to assess. (2) *Payload* — Claude Code's PostToolUse input carries the output as `tool_response` (the scripts read `tool_result`), replacing output requires `hookSpecificOutput.updatedToolOutput` (the scripts print a bare `tool_result`), and all matching hooks run in parallel with no output chaining; tracked in #6357. (3) *Blocking* — Claude Code keys on the stdout JSON on any exit code (`decision:"block"` is deprecated for PreToolUse but still maps to `deny`) and treats a bare exit `1` as non-blocking (exit `2` is its own blocking code); a local control run confirmed the scripts' "exit 1 + `{"decision":"block"}`" convention does block once the settings are loaded. Net: the PreToolUse half of the contract is effective under Claude Code; the PostToolUse scripts additionally need #6357. ### Runtime-specific config key support @@ -70,7 +70,7 @@ Harness keys are runtime-neutral in the YAML but each runtime owns their transla | `plugins` | Claude plugin marketplace layout (`bootstrapPlugins`) | — | ignored | Claude-specific format; warn and skip if unsupported | | Agent frontmatter `tools:` (`Bash(gh,jq)` syntax, ADR 0027) | Native Claude permission syntax | — | ignored | Enforce via `--tools`/allowlist plus a hook adapter; Claude tool names differ in case from most runtimes (#608) | | `skills` | `CLAUDE_CONFIG_DIR/skills/` | — | ignored | Agent Skills spec (`SKILL.md`) is portable; destination is `rt.ConfigDir() + "/skills"` (also used by the runtime fetch service) | -| `security.sandbox_hooks` | `SandboxHooksBootstrap` → settings.json | ✗ (stub) | ignored | See [Sandbox hook contract](#sandbox-hook-contract) | +| `security.sandbox_hooks` | `SandboxHooksBootstrap` → hooks.json via `--settings` | ✗ (stub) | ignored | See [Sandbox hook contract](#sandbox-hook-contract) | | `--debug` (CLI flag) | `--debug-file`, artifact `claude-debug.log` | — | no-op | Implement `DebugLogNamer` to name the artifact | ## Sandbox workspace layout @@ -86,15 +86,14 @@ The sandbox has two key directories that map to Claude Code's config levels: │ │ ├── code-review/SKILL.md Built-in skills (personal level — wins on collision) │ │ ├── pr-review/SKILL.md │ │ └── ... -│ └── plugins/ -│ └── ... Plugin state (simplified; see bootstrapPlugins()) +│ ├── plugins/ +│ │ └── ... Plugin state (simplified; see bootstrapPlugins()) +│ ├── hooks/ Security hook scripts (PreToolUse, PostToolUse) +│ └── hooks.json Hook wiring (loaded via --settings in buildRunCommand) │ └── workspace/ ← SandboxWorkspace ├── .env Environment variables (sourced before claude) ├── .env.d/ Additional env files (host_files expand) - ├── .claude/ - │ ├── hooks/ Security hooks (PreToolUse, PostToolUse) - │ └── settings.json Hook wiring (separate from plugin config) │ └── / ← Claude Code's working directory (cd target) ├── CLAUDE.md Project instructions (repo's own or injected bridge) diff --git a/e2e/behaviour/features/dispatch/hooks-loaded.feature b/e2e/behaviour/features/dispatch/hooks-loaded.feature new file mode 100644 index 0000000000..2a5fb9e19b --- /dev/null +++ b/e2e/behaviour/features/dispatch/hooks-loaded.feature @@ -0,0 +1,32 @@ +Feature: Sandbox security hooks are loaded via --settings + + Security hooks (SSRF, canary, secret redaction, etc.) are installed under + the runner-owned claude-config/ directory and wired via the --settings flag + so Claude Code loads them regardless of its working directory. This scenario + verifies that at least one blocking PreToolUse hook fires end-to-end — + catching the "silently not loaded" class of regression where hook wiring + exists but the CLI never reads it. + + Scenario: SSRF PreToolUse hook blocks a disallowed URL + Given the enrolled test repository + And a custom harness "hooks-smoke" with: + """ + agent: agents/triage.md + role: triage + slug: fullsend-ai-hooks-smoke + model: opus + image: ghcr.io/fullsend-ai/fullsend-sandbox:latest + trigger: | + event.entity.kind == "work_item" + && event.transition.kind == "label_changed" + && event.transition.label.name == "ready-for-hooks-smoke" + """ + And a dummy agent that would: + | description | op | args | + | Fetch metadata endpoint | url_get | http://169.254.169.254/latest/meta-data/ | + | Emit triage JSON | write_fixture | output/agent-result.json, fixtures/triage/sufficient.json | + And an issue + When the issue is labeled "ready-for-hooks-smoke" + Then the harness "hooks-smoke" workflow completes successfully + And the agent will fail to Fetch metadata endpoint + And the agent will succeed to Emit triage JSON diff --git a/internal/cli/run.go b/internal/cli/run.go index e9ae42779c..5b65444c9c 100644 --- a/internal/cli/run.go +++ b/internal/cli/run.go @@ -1518,17 +1518,22 @@ func runAgent(ctx context.Context, agentName, fullsendDir, outputBase, targetRep agentCtx, agentSpan := tracer.Start(ctx, "agent", trace.WithAttributes(agentSpanStartAttrs(iteration, agentName)...)) var metrics agentruntime.RunMetrics + hooksSettings := "" + if h.SecurityEnabled() { + hooksSettings = security.SandboxHooksSettings + } exitCode, runErr := rt.Run(agentCtx, agentruntime.RunParams{ - SandboxName: sandboxName, - AgentBaseName: agentBaseName, - Model: h.Model, - Effort: h.Effort, - RepoDir: remoteRepositoryDir, - FullsendDir: absFullsendDir, - PluginDirs: pluginDirs, - Debug: debug, - Timeout: timeout, - OutputPath: filepath.Join(iterDir, "output.jsonl"), + SandboxName: sandboxName, + AgentBaseName: agentBaseName, + Model: h.Model, + Effort: h.Effort, + RepoDir: remoteRepositoryDir, + FullsendDir: absFullsendDir, + PluginDirs: pluginDirs, + Debug: debug, + HooksSettingsPath: hooksSettings, + Timeout: timeout, + OutputPath: filepath.Join(iterDir, "output.jsonl"), }, printer, agentStart, &metrics) close(heartbeatDone) @@ -1793,7 +1798,7 @@ func runAgent(ctx context.Context, agentName, fullsendDir, outputBase, targetRep func bootstrapCommon(sandboxName, fullsendBinary string, h *harness.Harness) error { // Runner-level dirs only; sandbox hook scripts are installed by the runtime - // (Claude: workspace/.claude/ via installClaudeHooks) when the bootstrap + // (Claude: claude-config/hooks/ via installClaudeHooks) when the bootstrap // input implements SandboxHooksBootstrap. mkdirCmd := fmt.Sprintf("mkdir -p %s/bin %s/.env.d %s/.security", sandbox.SandboxWorkspace, sandbox.SandboxWorkspace, sandbox.SandboxWorkspace) diff --git a/internal/runtime/claude.go b/internal/runtime/claude.go index a524cfa614..f422a8263c 100644 --- a/internal/runtime/claude.go +++ b/internal/runtime/claude.go @@ -310,6 +310,10 @@ func buildRunCommand(params RunParams) string { "--output-format stream-json", } + if params.HooksSettingsPath != "" { + parts = append(parts, fmt.Sprintf("--settings '%s'", strings.ReplaceAll(params.HooksSettingsPath, "'", "'\\''"))) + } + if params.Debug != "" { parts = append(parts, fmt.Sprintf("--debug-file '%s/%s'", sandbox.SandboxWorkspace, claudeDebugLog)) if params.Debug != "*" { @@ -338,40 +342,39 @@ func buildRunCommand(params RunParams) string { return strings.Join(parts, " ") } -// Claude Code reads two settings.json files in the sandbox: +// Claude Code reads settings from two separate files in the sandbox: // - {CLAUDE_CONFIG_DIR}/settings.json — plugin marketplace state (bootstrapPlugins) -// - {SandboxWorkspace}/.claude/settings.json — security Pre/PostToolUse hooks (here) +// - {CLAUDE_CONFIG_DIR}/hooks.json — security Pre/PostToolUse hooks (here) // -// Keep these paths separate; merging them would mix plugin config with hook wiring. +// The hooks file is loaded via --settings in buildRunCommand, which takes +// precedence over project/local settings. Hook scripts and wiring are +// co-located under the runner-owned config directory, outside the +// agent-writable workspace tree (#6358). func installClaudeHooks(sandboxName string, hooks security.SandboxHookConfig) error { - // security.SandboxHooksDir is the directory the generated settings.json - // commands point at; installHookScripts creates it (and its .claude parent). + // security.SandboxHooksDir is the directory the generated hooks.json + // commands point at; installHookScripts creates it. if err := installHookScripts(sandboxName, security.SandboxHooksDir, hooks); err != nil { return err } - settingsJSON, err := security.GenerateClaudeSettings(hooks) + hooksJSON, err := security.GenerateHooksConfig(hooks) if err != nil { - return fmt.Errorf("generating claude settings: %w", err) + return fmt.Errorf("generating hooks config: %w", err) } - tmpSettings, err := os.CreateTemp("", "fullsend-settings-*.json") + tmpDir, err := os.MkdirTemp("", "fullsend-hooks-") if err != nil { - return fmt.Errorf("creating temp settings file: %w", err) + return fmt.Errorf("creating temp hooks file: %w", err) } - if _, err := tmpSettings.Write(settingsJSON); err != nil { - tmpSettings.Close() - os.Remove(tmpSettings.Name()) - return fmt.Errorf("writing settings: %w", err) + defer os.RemoveAll(tmpDir) + tmpPath := filepath.Join(tmpDir, "hooks.json") + if err := os.WriteFile(tmpPath, hooksJSON, 0o600); err != nil { + return fmt.Errorf("writing hooks config: %w", err) } - tmpSettings.Close() - remoteSettings := fmt.Sprintf("%s/.claude/settings.json", sandbox.SandboxWorkspace) - if err := sandbox.Upload(sandboxName, tmpSettings.Name(), remoteSettings); err != nil { - os.Remove(tmpSettings.Name()) - return fmt.Errorf("copying settings.json to sandbox: %w", err) + if err := sandbox.Upload(sandboxName, tmpPath, security.SandboxHooksSettings); err != nil { + return fmt.Errorf("copying hooks.json to sandbox: %w", err) } - os.Remove(tmpSettings.Name()) return appendHookEnv(sandboxName, hooks) } diff --git a/internal/runtime/claude_test.go b/internal/runtime/claude_test.go index f466c98984..ed69d1e445 100644 --- a/internal/runtime/claude_test.go +++ b/internal/runtime/claude_test.go @@ -15,7 +15,9 @@ import ( "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + "github.com/fullsend-ai/fullsend/internal/harness" "github.com/fullsend-ai/fullsend/internal/sandbox" + "github.com/fullsend-ai/fullsend/internal/security" "github.com/fullsend-ai/fullsend/internal/ui" ) @@ -177,6 +179,29 @@ func TestBuildRunCommand_NoPlugins(t *testing.T) { assert.NotContains(t, cmd, "--plugin-dir") } +func TestBuildRunCommand_WithHooksSettings(t *testing.T) { + cmd := buildRunCommand(RunParams{ + AgentBaseName: "agent", + RepoDir: "/sandbox/workspace/repo", + HooksSettingsPath: "/sandbox/claude-config/hooks.json", + }) + assert.Contains(t, cmd, "--settings '/sandbox/claude-config/hooks.json'") +} + +func TestBuildRunCommand_WithoutHooksSettings(t *testing.T) { + cmd := testRunCommand("agent", "", "/sandbox/workspace/repo", nil, "") + assert.NotContains(t, cmd, "--settings") +} + +func TestBuildRunCommand_HooksSettingsEscapesQuotes(t *testing.T) { + cmd := buildRunCommand(RunParams{ + AgentBaseName: "agent", + RepoDir: "/sandbox/workspace/repo", + HooksSettingsPath: "/sandbox/path'with'quotes/hooks.json", + }) + assert.Contains(t, cmd, `--settings '/sandbox/path'\''with'\''quotes/hooks.json'`) +} + func TestBuildRunCommand_DebugDisabled(t *testing.T) { cmd := testRunCommand("agent", "", "/sandbox/workspace/repo", nil, "") assert.NotContains(t, cmd, "--debug-file") @@ -211,6 +236,18 @@ func TestBuildRunCommand_NoDoubleSpaces(t *testing.T) { cmd := testRunCommandWithEffort(tc.agentName, tc.model, tc.effort, "/sandbox/workspace/repo", tc.pluginDirs, tc.debug) assert.NotContains(t, cmd, " ", "command should not contain double spaces") }) + t.Run(tc.name+" with hooks settings", func(t *testing.T) { + cmd := buildRunCommand(RunParams{ + AgentBaseName: tc.agentName, + Model: tc.model, + Effort: tc.effort, + RepoDir: "/sandbox/workspace/repo", + PluginDirs: tc.pluginDirs, + Debug: tc.debug, + HooksSettingsPath: "/sandbox/claude-config/hooks.json", + }) + assert.NotContains(t, cmd, " ", "command should not contain double spaces") + }) } } @@ -796,3 +833,76 @@ func TestClaudeRuntimeSystem(t *testing.T) { // in the CLI) keeps telemetry runtime-agnostic per ADR 0050. assert.Equal(t, "anthropic", ClaudeRuntime{}.System()) } + +// TestInstallClaudeHooks_HappyPath verifies that installClaudeHooks writes +// hook scripts and the hooks.json wiring to the runner-owned config +// directory. Uses a stub openshell so sandbox.Exec/Upload succeed without a +// real sandbox. +func TestInstallClaudeHooks_HappyPath(t *testing.T) { + logPath := filepath.Join(t.TempDir(), "openshell.log") + stubDir := t.TempDir() + script := "#!/bin/sh\necho \"$@\" >> '" + logPath + "'\nexit 0\n" + require.NoError(t, os.WriteFile(filepath.Join(stubDir, "openshell"), []byte(script), 0o755)) + t.Setenv("PATH", stubDir) + + hooks := security.SandboxHookConfig{} // default hooks (all enabled) + require.NoError(t, installClaudeHooks("test-sandbox", hooks)) + + logBytes, err := os.ReadFile(logPath) + require.NoError(t, err) + log := string(logBytes) + assert.Contains(t, log, security.SandboxHooksDir+"/tirith_check.py") + assert.Contains(t, log, security.SandboxHooksSettings) +} + +// TestInstallClaudeHooks_SettingsUploadError verifies that installClaudeHooks +// returns a descriptive error when the hooks.json wiring upload fails. +func TestInstallClaudeHooks_SettingsUploadError(t *testing.T) { + stubDir := t.TempDir() + // Stub that succeeds for all operations except the hooks.json upload. + script := "#!/bin/sh\ncase \"$*\" in *hooks.json*) exit 1 ;; esac\nexit 0\n" + require.NoError(t, os.WriteFile(filepath.Join(stubDir, "openshell"), []byte(script), 0o755)) + t.Setenv("PATH", stubDir) + + hooks := security.SandboxHookConfig{} + err := installClaudeHooks("test-sandbox", hooks) + require.Error(t, err) + assert.Contains(t, err.Error(), "copying hooks.json to sandbox") +} + +// TestInstallClaudeHooks_OpenshellNotInPath verifies that installClaudeHooks +// fails fast when openshell is not available. +func TestInstallClaudeHooks_OpenshellNotInPath(t *testing.T) { + t.Setenv("PATH", "") + + hooks := security.SandboxHookConfig{} + err := installClaudeHooks("test-sandbox", hooks) + require.Error(t, err) +} + +// TestInstallClaudeHooks_TempFileError covers the temp-file creation failure +// branch: with every hook disabled, installHookScripts performs no uploads, +// so the first CreateTemp call is the one for hooks.json — pointing TMPDIR at +// a regular file makes it fail. +func TestInstallClaudeHooks_TempFileError(t *testing.T) { + stubDir := t.TempDir() + require.NoError(t, os.WriteFile(filepath.Join(stubDir, "openshell"), []byte("#!/bin/sh\nexit 0\n"), 0o755)) + t.Setenv("PATH", stubDir) + notADir := filepath.Join(t.TempDir(), "file") + require.NoError(t, os.WriteFile(notADir, []byte("x"), 0o644)) + t.Setenv("TMPDIR", notADir) + + off := false + h := &harness.Harness{Security: &harness.SecurityConfig{SandboxHooks: &harness.SandboxHooks{ + Tirith: &harness.TirithConfig{Enabled: &off}, + SSRFPreTool: &off, + CanaryPreTool: &off, + CanaryPostTool: &off, + SecretRedactPostTool: &off, + UnicodePostTool: &off, + ContextSuppressPostTool: &off, + }}} + err := installClaudeHooks("test-sandbox", security.SandboxHookConfigFromHarness(h)) + require.Error(t, err) + assert.Contains(t, err.Error(), "creating temp hooks file") +} diff --git a/internal/runtime/runtime.go b/internal/runtime/runtime.go index fe80b0f9af..d84b2d9de3 100644 --- a/internal/runtime/runtime.go +++ b/internal/runtime/runtime.go @@ -32,9 +32,12 @@ type RunParams struct { FullsendDir string PluginDirs []string Debug string - Timeout time.Duration - OutputPath string // if set, tee stream-json stdout to this file - OnEvent func(AgentEvent) // if non-nil, called with normalized events during Run + // HooksSettingsPath, if set, is passed as --settings so Claude Code + // loads the runner's hook wiring regardless of its working directory. + HooksSettingsPath string + Timeout time.Duration + OutputPath string // if set, tee stream-json stdout to this file + OnEvent func(AgentEvent) // if non-nil, called with normalized events during Run } // TranscriptError holds extracted error information from a runtime transcript. diff --git a/internal/runtime/sandbox_hooks_bootstrap_test.go b/internal/runtime/sandbox_hooks_bootstrap_test.go index 3d5df658cc..b97058a8c4 100644 --- a/internal/runtime/sandbox_hooks_bootstrap_test.go +++ b/internal/runtime/sandbox_hooks_bootstrap_test.go @@ -46,10 +46,10 @@ func TestClaudeRuntime_Bootstrap_InstallsSandboxHooks(t *testing.T) { log := string(logBytes) // Hook scripts are uploaded to the Claude hooks dir and chmod'ed. - assert.Contains(t, log, "/sandbox/workspace/.claude/hooks/tirith_check.py") - assert.Contains(t, log, "chmod +x '/sandbox/workspace/.claude/hooks/tirith_check.py'") - // Claude-specific wiring (settings.json) is still installed. - assert.Contains(t, log, "/sandbox/workspace/.claude/settings.json") + assert.Contains(t, log, "/sandbox/claude-config/hooks/tirith_check.py") + assert.Contains(t, log, "chmod +x '/sandbox/claude-config/hooks/tirith_check.py'") + // Claude-specific wiring (hooks.json, loaded via --settings) is installed. + assert.Contains(t, log, "/sandbox/claude-config/hooks.json") // Hook env is appended to the workspace .env. assert.Contains(t, log, "export TIRITH_FAIL_ON=high") assert.Contains(t, log, "export TIRITH_REQUIRED=1") @@ -68,7 +68,7 @@ func TestClaudeRuntime_Bootstrap_NoHooksWithoutExtension(t *testing.T) { logBytes, err := os.ReadFile(logPath) require.NoError(t, err) - assert.False(t, strings.Contains(string(logBytes), ".claude/hooks/"), + assert.False(t, strings.Contains(string(logBytes), "claude-config/hooks"), "no hook scripts must be installed when the input lacks SandboxHooksBootstrap") } diff --git a/internal/security/hooks.go b/internal/security/hooks.go index c43ef1e547..520e58b0cb 100644 --- a/internal/security/hooks.go +++ b/internal/security/hooks.go @@ -44,14 +44,20 @@ type hookMatcher struct { Hooks []hookEntry `json:"hooks"` } -// claudeSettings represents the .claude/settings.json structure. -type claudeSettings struct { +// hooksConfig represents the hooks.json structure for Claude Code hook wiring. +type hooksConfig struct { Hooks map[string][]hookMatcher `json:"hooks"` } -// SandboxHooksDir is the path where hook scripts are installed inside the -// sandbox. Must match sandbox.SandboxWorkspace + "/.claude/hooks". -const SandboxHooksDir = sandbox.SandboxWorkspace + "/.claude/hooks" +// SandboxHooksDir is the directory where hook scripts are installed inside +// the sandbox. Co-located with SandboxHooksSettings under the runner-owned +// config directory so they are outside the agent-writable workspace tree. +const SandboxHooksDir = sandbox.SandboxClaudeConfig + "/hooks" + +// SandboxHooksSettings is the path where the hook wiring hooks.json is +// written inside the sandbox. buildRunCommand passes this via --settings so +// Claude Code loads the hooks regardless of its working directory. +const SandboxHooksSettings = sandbox.SandboxClaudeConfig + "/hooks.json" // HookPhase identifies when a sandbox hook group runs relative to a tool call. // The names match Claude Code's settings.json event names; other runtimes map @@ -82,7 +88,7 @@ const AllTools = "*" // HookPlan returns the runtime-neutral wiring for the enabled sandbox hooks: // which scripts run in which phase, for which tools, in what order. It is the -// single source of truth consumed by GenerateClaudeSettings (Claude Code) and +// single source of truth consumed by GenerateHooksConfig (Claude Code) and // by any other runtime's hook adapter, so the two cannot diverge. func HookPlan(hooks SandboxHookConfig) []HookGroup { var plan []HookGroup @@ -132,10 +138,8 @@ func HookPlan(hooks SandboxHookConfig) []HookGroup { // NOTE: Claude Code runs all matching hooks in parallel and does not // pipe one hook's output into the next, and its PostToolUse payload uses // `tool_response` rather than the `tool_result` these scripts read — so - // under Claude Code this chain is not effective today (fullsend#6357); - // the settings.json it is rendered into is also not loaded from where - // the runner writes it (fullsend#6358). The ordering here is the - // contract adapters must meet. + // under Claude Code this chain is not effective today (fullsend#6357). + // The ordering here is the contract adapters must meet. var postScripts []string if contextSuppressPostToolEnabled(hooks) { postScripts = append(postScripts, "context_suppress_posttool.py") @@ -166,11 +170,12 @@ func HookPlan(hooks SandboxHookConfig) []HookGroup { return plan } -// GenerateClaudeSettings produces a .claude/settings.json with security hooks -// configured according to hooks. Returns the JSON bytes. The wiring comes from -// HookPlan; this function only renders it in Claude Code's settings format. -func GenerateClaudeSettings(hooks SandboxHookConfig) ([]byte, error) { - settings := claudeSettings{ +// GenerateHooksConfig produces the hooks.json Claude Code hook wiring, +// loaded via --settings in buildRunCommand. Returns the JSON bytes. The +// wiring comes from HookPlan; this function only renders it in Claude Code's +// settings format. +func GenerateHooksConfig(hooks SandboxHookConfig) ([]byte, error) { + cfg := hooksConfig{ Hooks: make(map[string][]hookMatcher), } @@ -181,13 +186,13 @@ func GenerateClaudeSettings(hooks SandboxHookConfig) ([]byte, error) { Type: "command", Command: "python3 " + SandboxHooksDir + "/" + script, }) } - settings.Hooks[string(g.Phase)] = append(settings.Hooks[string(g.Phase)], hookMatcher{ + cfg.Hooks[string(g.Phase)] = append(cfg.Hooks[string(g.Phase)], hookMatcher{ Matcher: strings.Join(g.Tools, "|"), Hooks: entries, }) } - return json.MarshalIndent(settings, "", " ") + return json.MarshalIndent(cfg, "", " ") } // HookFiles returns a map of filename -> content for all enabled hook scripts. diff --git a/internal/security/hooks_test.go b/internal/security/hooks_test.go index 4a2fccb32d..81664756f1 100644 --- a/internal/security/hooks_test.go +++ b/internal/security/hooks_test.go @@ -11,9 +11,9 @@ import ( "github.com/fullsend-ai/fullsend/internal/harness" ) -func TestGenerateClaudeSettings_AllDefaults(t *testing.T) { +func TestGenerateHooksConfig_AllDefaults(t *testing.T) { h := &harness.Harness{Agent: "test.md"} - data, err := GenerateClaudeSettings(SandboxHookConfigFromHarness(h)) + data, err := GenerateHooksConfig(SandboxHookConfigFromHarness(h)) require.NoError(t, err) var settings map[string]any @@ -42,7 +42,7 @@ func TestGenerateClaudeSettings_AllDefaults(t *testing.T) { assert.Len(t, canaryHooks, 1) } -func TestGenerateClaudeSettings_TirithDisabled(t *testing.T) { +func TestGenerateHooksConfig_TirithDisabled(t *testing.T) { disabled := false h := &harness.Harness{ Agent: "test.md", @@ -52,7 +52,7 @@ func TestGenerateClaudeSettings_TirithDisabled(t *testing.T) { }, }, } - data, err := GenerateClaudeSettings(SandboxHookConfigFromHarness(h)) + data, err := GenerateHooksConfig(SandboxHookConfigFromHarness(h)) require.NoError(t, err) var settings map[string]any @@ -63,7 +63,7 @@ func TestGenerateClaudeSettings_TirithDisabled(t *testing.T) { assert.Len(t, preTools, 2) // ssrf + canary_pretool } -func TestGenerateClaudeSettings_AllHooksDisabled(t *testing.T) { +func TestGenerateHooksConfig_AllHooksDisabled(t *testing.T) { disabled := false h := &harness.Harness{ Agent: "test.md", @@ -80,7 +80,7 @@ func TestGenerateClaudeSettings_AllHooksDisabled(t *testing.T) { }, }, } - data, err := GenerateClaudeSettings(SandboxHookConfigFromHarness(h)) + data, err := GenerateHooksConfig(SandboxHookConfigFromHarness(h)) require.NoError(t, err) var settings map[string]any @@ -151,7 +151,7 @@ func TestEmbeddedHooksNotEmpty(t *testing.T) { assert.NotEmpty(t, ToolAllowlistPreToolHook) } -func TestGenerateClaudeSettings_UnicodeDisabled(t *testing.T) { +func TestGenerateHooksConfig_UnicodeDisabled(t *testing.T) { disabled := false h := &harness.Harness{ Agent: "test.md", @@ -161,7 +161,7 @@ func TestGenerateClaudeSettings_UnicodeDisabled(t *testing.T) { }, }, } - data, err := GenerateClaudeSettings(SandboxHookConfigFromHarness(h)) + data, err := GenerateHooksConfig(SandboxHookConfigFromHarness(h)) require.NoError(t, err) var settings map[string]any @@ -177,7 +177,7 @@ func TestGenerateClaudeSettings_UnicodeDisabled(t *testing.T) { assert.Len(t, chainedHooks, 2) // context_suppress + secret_redact } -func TestGenerateClaudeSettings_SecretRedactDisabled(t *testing.T) { +func TestGenerateHooksConfig_SecretRedactDisabled(t *testing.T) { disabled := false h := &harness.Harness{ Agent: "test.md", @@ -187,7 +187,7 @@ func TestGenerateClaudeSettings_SecretRedactDisabled(t *testing.T) { }, }, } - data, err := GenerateClaudeSettings(SandboxHookConfigFromHarness(h)) + data, err := GenerateHooksConfig(SandboxHookConfigFromHarness(h)) require.NoError(t, err) var settings map[string]any @@ -203,7 +203,7 @@ func TestGenerateClaudeSettings_SecretRedactDisabled(t *testing.T) { assert.Len(t, chainedHooks, 2) // context_suppress + unicode } -func TestGenerateClaudeSettings_ContextSuppressDisabled(t *testing.T) { +func TestGenerateHooksConfig_ContextSuppressDisabled(t *testing.T) { disabled := false h := &harness.Harness{ Agent: "test.md", @@ -213,7 +213,7 @@ func TestGenerateClaudeSettings_ContextSuppressDisabled(t *testing.T) { }, }, } - data, err := GenerateClaudeSettings(SandboxHookConfigFromHarness(h)) + data, err := GenerateHooksConfig(SandboxHookConfigFromHarness(h)) require.NoError(t, err) var settings map[string]any @@ -229,9 +229,9 @@ func TestGenerateClaudeSettings_ContextSuppressDisabled(t *testing.T) { assert.Len(t, chainedHooks, 2) // unicode + secret_redact } -func TestGenerateClaudeSettings_PostToolSanitizeHookOrder(t *testing.T) { +func TestGenerateHooksConfig_PostToolSanitizeHookOrder(t *testing.T) { h := &harness.Harness{Agent: "test.md"} - data, err := GenerateClaudeSettings(SandboxHookConfigFromHarness(h)) + data, err := GenerateHooksConfig(SandboxHookConfigFromHarness(h)) require.NoError(t, err) var settings map[string]any @@ -265,7 +265,7 @@ func TestGenerateClaudeSettings_PostToolSanitizeHookOrder(t *testing.T) { assert.Less(t, unicodeIdx, redactIdx, "unicode must run before secret_redact") } -func TestGenerateClaudeSettings_CanaryPostToolDisabled(t *testing.T) { +func TestGenerateHooksConfig_CanaryPostToolDisabled(t *testing.T) { disabled := false h := &harness.Harness{ Agent: "test.md", @@ -275,7 +275,7 @@ func TestGenerateClaudeSettings_CanaryPostToolDisabled(t *testing.T) { }, }, } - data, err := GenerateClaudeSettings(SandboxHookConfigFromHarness(h)) + data, err := GenerateHooksConfig(SandboxHookConfigFromHarness(h)) require.NoError(t, err) var settings map[string]any @@ -293,7 +293,7 @@ func TestGenerateClaudeSettings_CanaryPostToolDisabled(t *testing.T) { assert.Len(t, preTools, 3) // tirith + ssrf + canary_pretool } -func TestGenerateClaudeSettings_CanaryPreToolDisabled(t *testing.T) { +func TestGenerateHooksConfig_CanaryPreToolDisabled(t *testing.T) { disabled := false h := &harness.Harness{ Agent: "test.md", @@ -303,7 +303,7 @@ func TestGenerateClaudeSettings_CanaryPreToolDisabled(t *testing.T) { }, }, } - data, err := GenerateClaudeSettings(SandboxHookConfigFromHarness(h)) + data, err := GenerateHooksConfig(SandboxHookConfigFromHarness(h)) require.NoError(t, err) var settings map[string]any @@ -318,7 +318,7 @@ func TestGenerateClaudeSettings_CanaryPreToolDisabled(t *testing.T) { assert.Len(t, postTools, 2) // chain + canary_posttool } -func TestGenerateClaudeSettings_ToolAllowlistEnabled(t *testing.T) { +func TestGenerateHooksConfig_ToolAllowlistEnabled(t *testing.T) { enabled := true h := &harness.Harness{ Agent: "test.md", @@ -328,7 +328,7 @@ func TestGenerateClaudeSettings_ToolAllowlistEnabled(t *testing.T) { }, }, } - data, err := GenerateClaudeSettings(SandboxHookConfigFromHarness(h)) + data, err := GenerateHooksConfig(SandboxHookConfigFromHarness(h)) require.NoError(t, err) var settings map[string]any @@ -433,7 +433,7 @@ func TestHookPlan_CoversHookFiles_AllEnabled(t *testing.T) { // With the opt-in allowlist enabled, every shipped script must be scheduled // exactly once and every scheduled script must be shipped — the "cannot - // diverge" invariant between HookFiles, HookPlan and GenerateClaudeSettings. + // diverge" invariant between HookFiles, HookPlan and GenerateHooksConfig. seen := map[string]int{} for _, g := range plan { for _, s := range g.Scripts { @@ -446,7 +446,7 @@ func TestHookPlan_CoversHookFiles_AllEnabled(t *testing.T) { } assert.Contains(t, seen, "tool_allowlist_pretool.py") - settings, err := GenerateClaudeSettings(cfg) + settings, err := GenerateHooksConfig(cfg) require.NoError(t, err) for name := range files { assert.Contains(t, string(settings), SandboxHooksDir+"/"+name)