diff --git a/cmd/gateway.go b/cmd/gateway.go index 2eb6e84a..88904675 100644 --- a/cmd/gateway.go +++ b/cmd/gateway.go @@ -242,8 +242,8 @@ func runGateway() { slog.Info("bootstrap: capabilities backfill complete", "agents", count) } - // Subagent system - subagentMgr := setupSubagents(providerRegistry, cfg, msgBus, toolsReg, workspace, sandboxMgr) + // Subagent system (secureCLI store wired so subagent ExecTools enforce the gate) + subagentMgr := setupSubagents(providerRegistry, cfg, msgBus, toolsReg, workspace, sandboxMgr, pgStores.SecureCLI) if subagentMgr != nil { // Wire announce queue for batched subagent result delivery (matching TS debounce pattern). announceQueue := tools.NewAnnounceQueue(1000, 20, makeDelegateAnnounceCallback(subagentMgr, msgBus)) diff --git a/cmd/gateway_agents.go b/cmd/gateway_agents.go index e2cb91bf..35b8ca79 100644 --- a/cmd/gateway_agents.go +++ b/cmd/gateway_agents.go @@ -158,7 +158,7 @@ func buildEmbeddingProvider( return nil } -func setupSubagents(providerReg *providers.Registry, cfg *config.Config, msgBus *bus.MessageBus, toolsReg *tools.Registry, workspace string, sandboxMgr sandbox.Manager) *tools.SubagentManager { +func setupSubagents(providerReg *providers.Registry, cfg *config.Config, msgBus *bus.MessageBus, toolsReg *tools.Registry, workspace string, sandboxMgr sandbox.Manager, secureCLIStore store.SecureCLIStore) *tools.SubagentManager { names := providerReg.List(context.Background()) if len(names) == 0 { return nil @@ -202,24 +202,48 @@ func setupSubagents(providerReg *providers.Registry, cfg *config.Config, msgBus // NOTE: SubagentManager.applyDenyList() handles deny lists after createTools(), // so we don't apply deny lists here. toolsFactory := func() *tools.Registry { - reg := toolsReg.Clone() - if sandboxMgr != nil { - reg.Register(tools.NewSandboxedReadFileTool(workspace, agentCfg.RestrictToWorkspace, sandboxMgr)) - reg.Register(tools.NewSandboxedWriteFileTool(workspace, agentCfg.RestrictToWorkspace, sandboxMgr)) - reg.Register(tools.NewSandboxedListFilesTool(workspace, agentCfg.RestrictToWorkspace, sandboxMgr)) - reg.Register(tools.NewSandboxedExecTool(workspace, agentCfg.RestrictToWorkspace, sandboxMgr)) - } else { - reg.Register(tools.NewReadFileTool(workspace, agentCfg.RestrictToWorkspace)) - reg.Register(tools.NewWriteFileTool(workspace, agentCfg.RestrictToWorkspace)) - reg.Register(tools.NewListFilesTool(workspace, agentCfg.RestrictToWorkspace)) - reg.Register(tools.NewExecTool(workspace, agentCfg.RestrictToWorkspace)) - } + reg, _ := buildSubagentToolsRegistry(toolsReg, workspace, agentCfg.RestrictToWorkspace, sandboxMgr, secureCLIStore) return reg } return tools.NewSubagentManager(provider, providerReg, agentCfg.Model, msgBus, toolsFactory, subCfg) } +// buildSubagentToolsRegistry produces a cloned tool registry for a subagent +// with workspace-scoped file/exec tools registered. The returned ExecTool is +// also returned for test assertion (Red Team F3): callers verify that the +// secureCLIStore is wired so the subagent's exec path enforces the gate. +func buildSubagentToolsRegistry( + parentReg *tools.Registry, + workspace string, + restrict bool, + sandboxMgr sandbox.Manager, + secureCLIStore store.SecureCLIStore, +) (*tools.Registry, *tools.ExecTool) { + reg := parentReg.Clone() + var execTool *tools.ExecTool + if sandboxMgr != nil { + reg.Register(tools.NewSandboxedReadFileTool(workspace, restrict, sandboxMgr)) + reg.Register(tools.NewSandboxedWriteFileTool(workspace, restrict, sandboxMgr)) + reg.Register(tools.NewSandboxedListFilesTool(workspace, restrict, sandboxMgr)) + execTool = tools.NewSandboxedExecTool(workspace, restrict, sandboxMgr) + reg.Register(execTool) + } else { + reg.Register(tools.NewReadFileTool(workspace, restrict)) + reg.Register(tools.NewWriteFileTool(workspace, restrict)) + reg.Register(tools.NewListFilesTool(workspace, restrict)) + execTool = tools.NewExecTool(workspace, restrict) + reg.Register(execTool) + } + // Red Team F3: subagent ExecTool must enforce the secure-CLI gate + // (and env scrub on fall-through) — without this, a parent agent + // can spawn a subagent to bypass the gate via host-inherited env. + if secureCLIStore != nil { + execTool.SetSecureCLIStore(secureCLIStore) + } + return reg, execTool +} + // setupTTS creates the TTS manager from config and registers providers. // Edge TTS is always registered (free, no API key required). // Always returns a non-nil manager with at least one provider. diff --git a/cmd/gateway_agents_test.go b/cmd/gateway_agents_test.go index 1d84313f..a0634a09 100644 --- a/cmd/gateway_agents_test.go +++ b/cmd/gateway_agents_test.go @@ -7,9 +7,85 @@ import ( "net/http/httptest" "testing" + "github.com/google/uuid" + "github.com/nextlevelbuilder/goclaw/internal/store" + "github.com/nextlevelbuilder/goclaw/internal/tools" ) +// --- Red Team F3: subagent ExecTool must inherit secureCLIStore wiring --- + +type stubSecureCLIStoreCmd struct{} + +func (s *stubSecureCLIStoreCmd) Create(ctx context.Context, b *store.SecureCLIBinary) error { + return nil +} +func (s *stubSecureCLIStoreCmd) Get(ctx context.Context, id uuid.UUID) (*store.SecureCLIBinary, error) { + return nil, nil +} +func (s *stubSecureCLIStoreCmd) Update(ctx context.Context, id uuid.UUID, updates map[string]any) error { + return nil +} +func (s *stubSecureCLIStoreCmd) Delete(ctx context.Context, id uuid.UUID) error { return nil } +func (s *stubSecureCLIStoreCmd) List(ctx context.Context) ([]store.SecureCLIBinary, error) { + return nil, nil +} +func (s *stubSecureCLIStoreCmd) ListEnabled(ctx context.Context) ([]store.SecureCLIBinary, error) { + return nil, nil +} +func (s *stubSecureCLIStoreCmd) ListForAgent(ctx context.Context, agentID uuid.UUID) ([]store.SecureCLIBinary, error) { + return nil, nil +} +func (s *stubSecureCLIStoreCmd) IsRegisteredBinary(ctx context.Context, binaryName string) (bool, error) { + return false, nil +} +func (s *stubSecureCLIStoreCmd) LookupByBinary(ctx context.Context, binaryName string, agentID *uuid.UUID, userID string) (*store.SecureCLIBinary, error) { + return nil, nil +} +func (s *stubSecureCLIStoreCmd) GetUserCredentials(ctx context.Context, binaryID uuid.UUID, userID string) (*store.SecureCLIUserCredential, error) { + return nil, nil +} +func (s *stubSecureCLIStoreCmd) SetUserCredentials(ctx context.Context, binaryID uuid.UUID, userID string, encryptedEnv []byte) error { + return nil +} +func (s *stubSecureCLIStoreCmd) DeleteUserCredentials(ctx context.Context, binaryID uuid.UUID, userID string) error { + return nil +} +func (s *stubSecureCLIStoreCmd) ListUserCredentials(ctx context.Context, binaryID uuid.UUID) ([]store.SecureCLIUserCredential, error) { + return nil, nil +} + +// TestSubagentExecTool_StoreWired ensures the subagent tool factory wires the +// SecureCLIStore into the subagent's ExecTool, so the gate enforces on +// spawned-subagent exec (Red Team F3). A missing wiring would let a parent +// agent bypass the gate by delegating the exec to a subagent. +func TestSubagentExecTool_StoreWired(t *testing.T) { + parent := tools.NewRegistry() + stub := &stubSecureCLIStoreCmd{} + + _, execTool := buildSubagentToolsRegistry(parent, t.TempDir(), false, nil, stub) + if execTool == nil { + t.Fatal("expected non-nil exec tool from factory") + } + if !execTool.HasSecureCLIStore() { + t.Fatalf("expected subagent ExecTool to have SecureCLIStore wired (Red Team F3)") + } +} + +// TestSubagentExecTool_NilStoreIsSafe ensures the factory does not panic when +// the store is unavailable (Lite edition / no encryption key). The exec tool +// simply lacks the gate — same as today's Lite behavior. +func TestSubagentExecTool_NilStoreIsSafe(t *testing.T) { + parent := tools.NewRegistry() + _, execTool := buildSubagentToolsRegistry(parent, t.TempDir(), false, nil, nil) + if execTool == nil { + t.Fatal("expected non-nil exec tool from factory") + } + if execTool.HasSecureCLIStore() { + t.Fatalf("expected no SecureCLIStore when passed nil (Lite path)") + } +} + func captureEmbeddingRequest(t *testing.T, es *store.EmbeddingSettings) map[string]any { t.Helper() diff --git a/docs/03-tools-system.md b/docs/03-tools-system.md index 3e25da95..727f68e8 100644 --- a/docs/03-tools-system.md +++ b/docs/03-tools-system.md @@ -293,6 +293,39 @@ The `exec` tool allows the LLM to run shell commands, with multiple defense laye } ``` +#### Grant enforcement (hard deny) + +Registering a binary is not the same as letting every agent exec it. A registered +binary with `is_global=false` **requires a row in `secure_cli_agent_grants`** +for the calling agent; otherwise shell exec is denied before any process runs. +Closes the class of bypass where an ungranted agent could still invoke the +binary via host fallthrough and pick up inherited env (`$GH_TOKEN`, etc.) or +on-disk OAuth state (`~/.config/gh`). + +- **Enforcement site:** `internal/tools/shell.go` — gate runs after normalization + and before exec approval, keyed on `store.SecureCLIStore.IsRegisteredBinary`. +- **Name matching:** `filepath.Base(strings.TrimSpace(strings.ToLower(name)))` — + case-insensitive (macOS APFS), directory-prefix stripped, trims whitespace. +- **Shell wrapper unwrap:** detects and peels `sh -c / bash -c / zsh -c / dash -c`, + leading `env K=V ...`, `nohup`, `stdbuf`, `timeout ...` up to depth 3. Commands + nested deeper than 3 wrappers are rejected as adversarial. +- **Global binaries:** `is_global=true` skips the gate (no grant needed). Shared + utilities without per-agent scoping. +- **Fail-CLOSED:** if the grant lookup errors (DB down, timeout) the exec is + denied with a retry message. 2s per-lookup context timeout. +- **Env scrubbing on fall-through:** when a command bypasses the credentialed + path and runs on the host, the child process env is scrubbed of credential + keys (static deny list + dynamic keys from every registered binary of the + same tenant) before spawn. Prevents `$GH_TOKEN` leaking into a non-gh command. +- **Log events:** + - `security.credentialed_binary_denied` — agent attempted an ungranted + registered binary. Fields: `binary, wrapper, agent_id, tenant_id, command_prefix`. + - `security.credentialed_binary_gate_error` — lookup failed; exec denied. + - `security.credentialed_binary_wrapper_too_deep` — >3 shell wrappers. +- **Subagents:** subagent `ExecTool`s are wired with the same `SecureCLIStore` + (`cmd/gateway_agents.go` → `buildSubagentToolsRegistry`) so a parent cannot + bypass the gate by delegating the exec to a spawned child. + --- ### Deny Patterns diff --git a/docs/09-security.md b/docs/09-security.md index 7a268889..b58b8a4b 100644 --- a/docs/09-security.md +++ b/docs/09-security.md @@ -111,6 +111,17 @@ All four filesystem tools (`read_file`, `write_file`, `list_files`, `edit`) impl - Sandbox escape → Docker container isolation if sandbox enabled - Verbose flag leakage → Separate deny_verbose list blocks verbose/debug output +**Agent-level grant enforcement** -- The gate runs **before** any process spawn, blocking ungranted agents from executing registered binaries: + +| Control | Implementation | +|---------|-----------------| +| **Grant lookup** | `store.SecureCLIStore.IsRegisteredBinary(ctx, binaryName)` checks `secure_cli_agent_grants` table. Non-global binaries require a row for the calling agent. | +| **Fail-CLOSED** | If the grant lookup errors (DB down, timeout), exec is denied with retry message. Per-lookup timeout: 2 seconds. | +| **Env scrubbing** | When a command escapes the credentialed path (e.g., via adversarial `exec` tool), child process env is scrubbed of all credential keys (static deny list + dynamic keys from every registered binary in the tenant) before spawn. Prevents credential leakage into non-credentialed commands. | +| **Wrapper unwrap** | Blocks shell wrappers (`sh -c`, `bash -c`, etc.) that attempt to evade binary path matching. Checks up to 3 levels of nesting; deeper chains are rejected as adversarial. | +| **Logging** | Three security events: `security.credentialed_binary_denied` (ungranted agent), `security.credentialed_binary_gate_error` (lookup failure), `security.credentialed_binary_wrapper_too_deep` (nested wrapper attack). All include: binary, wrapper, agent_id, tenant_id, command prefix. | +| **Subagent wiring** | Subagent `ExecTool`s use the same `SecureCLIStore` via `cmd/gateway_agents.go` → `buildSubagentToolsRegistry`. Parent agents cannot bypass the gate by delegating exec to spawned subagents. | + ### Layer 4: Output Security | Mechanism | Detail | diff --git a/docs/17-changelog.md b/docs/17-changelog.md index 34aa4160..16db25f4 100644 --- a/docs/17-changelog.md +++ b/docs/17-changelog.md @@ -4,6 +4,30 @@ All notable changes to GoClaw Gateway are documented here. Format follows [Keep --- +### Secure CLI grant enforcement — registered binaries now hard-deny ungranted exec (2026-04-17) + +Closes a credential-scope bypass: an agent with no `secure_cli_agent_grants` row for a registered binary could still invoke it via shell fallthrough (`gh ...`, `sh -c 'gh ...'`) and pick up inherited env (`$GH_TOKEN`) or on-disk OAuth state (`~/.config/gh`). Registration is now an authorization boundary, not only a credential-injection hint. + +#### Security + +- **Shell gate (`internal/tools/shell.go`):** new enforcement branch runs after command normalization and before exec approval. Queries `SecureCLIStore.IsRegisteredBinary` per candidate; registered-but-ungranted binaries return `Binary %q requires a secure CLI grant. Ask admin to grant access to this agent.` Case-insensitive match (`filepath.Base(strings.ToLower(name))`) — macOS APFS safe. +- **Wrapper unwrap (depth 3):** strips leading `sh -c / bash -c / zsh -c / dash -c`, `env K=V ...`, `nohup`, `stdbuf`, `timeout ...` before the check, so `sh -c 'gh api ...'` still denies. Nesting beyond 3 is rejected as adversarial. +- **Fail-CLOSED:** lookup errors (DB down, 2s timeout) deny exec with a retry message, never fall through to host exec. New log events: `security.credentialed_binary_denied`, `security.credentialed_binary_gate_error`, `security.credentialed_binary_wrapper_too_deep`. +- **Env scrubbing on fall-through (`internal/tools/env_scrub.go`):** child-process env is stripped of credential keys before spawn — static deny list (`GH_TOKEN`, `AWS_SECRET_ACCESS_KEY`, `OPENAI_API_KEY`, …) plus dynamic keys collected from every registered binary of the tenant. `HOME`, `PATH`, `TERM`, `LANG`, `USER`, `TZ` preserved. Prevents a non-`gh` command from inheriting `$GH_TOKEN`. +- **Subagent wiring (`cmd/gateway_agents.go`):** subagent `ExecTool` registrations now receive the same `SecureCLIStore` via `buildSubagentToolsRegistry`. Parent agent can no longer bypass the gate by delegating the exec to a spawned child. +- **`store.SecureCLIStore.IsRegisteredBinary`:** new method on both PG (`internal/store/pg/secure_cli.go`) and SQLite (`internal/store/sqlitestore/secure-cli.go`) backends. Scoped to tenant, returns case-insensitive match. + +#### Affected editions + +- **Standard (PG):** full gate. +- **Lite (desktop/SQLite):** same gate via SQLite-backed `SecureCLIStore` — build-tag `sqliteonly` verified. + +#### Migration + +None. Existing binaries default `is_global=true` (no grant required) so current deployments are unaffected. To restrict a binary, set `is_global=false` and insert `secure_cli_agent_grants` rows per agent. + +--- + ### ACTOR vs SCOPE — #915 group permission fix + propagation (2026-04-16) Resolves Issue #915 (Telegram group `write_file` permission denied after `/addwriter`) and closes an adjacent silent-privilege-bypass discovered during the audit. diff --git a/internal/store/pg/secure_cli.go b/internal/store/pg/secure_cli.go index 86fd1572..ec4a481c 100644 --- a/internal/store/pg/secure_cli.go +++ b/internal/store/pg/secure_cli.go @@ -7,6 +7,7 @@ import ( "errors" "fmt" "log/slog" + "strings" "time" "github.com/google/uuid" @@ -41,6 +42,10 @@ func (s *PGSecureCLIStore) Create(ctx context.Context, b *store.SecureCLIBinary) b.ID = store.GenNewID() } + // Normalize binary_name to lowercase so IsRegisteredBinary (which lowercases + // the candidate) can match. Admin entering "Gh" becomes "gh". + b.BinaryName = strings.ToLower(strings.TrimSpace(b.BinaryName)) + // Encrypt env if provided var envBytes []byte if len(b.EncryptedEnv) > 0 && s.encKey != "" { @@ -183,6 +188,13 @@ func (s *PGSecureCLIStore) Update(ctx context.Context, id uuid.UUID, updates map } } + // Normalize binary_name to lowercase if updated (parity with Create). + if nameVal, ok := updates["binary_name"]; ok { + if nameStr, isStr := nameVal.(string); isStr { + updates["binary_name"] = strings.ToLower(strings.TrimSpace(nameStr)) + } + } + // Encrypt env if present in updates if envVal, ok := updates["encrypted_env"]; ok { if envStr, isStr := envVal.(string); isStr && envStr != "" && s.encKey != "" { @@ -408,6 +420,35 @@ func (s *PGSecureCLIStore) ListEnabled(ctx context.Context) ([]store.SecureCLIBi return s.scanRows(rows) } +// IsRegisteredBinary reports whether a binary requires a grant (is_global=false) +// and is enabled for the current tenant. See interface godoc for rationale. +func (s *PGSecureCLIStore) IsRegisteredBinary(ctx context.Context, binaryName string) (bool, error) { + name := strings.ToLower(strings.TrimSpace(binaryName)) + if name == "" { + return false, nil + } + query := `SELECT EXISTS( + SELECT 1 FROM secure_cli_binaries + WHERE LOWER(binary_name) = $1 + AND enabled = true + AND is_global = false` + args := []any{name} + if !store.IsCrossTenant(ctx) { + tid := store.TenantIDFromContext(ctx) + if tid == uuid.Nil { + return false, nil + } + query += ` AND tenant_id = $2` + args = append(args, tid) + } + query += `)` + var exists bool + if err := s.db.QueryRowContext(ctx, query, args...).Scan(&exists); err != nil { + return false, err + } + return exists, nil +} + // ListForAgent returns all CLIs accessible by an agent (global + granted), // with grant overrides merged into the returned configs. func (s *PGSecureCLIStore) ListForAgent(ctx context.Context, agentID uuid.UUID) ([]store.SecureCLIBinary, error) { diff --git a/internal/store/secure_cli_store.go b/internal/store/secure_cli_store.go index ec9a073f..aa846f2f 100644 --- a/internal/store/secure_cli_store.go +++ b/internal/store/secure_cli_store.go @@ -94,6 +94,15 @@ type SecureCLIStore interface { // with grant overrides merged into the returned configs. ListForAgent(ctx context.Context, agentID uuid.UUID) ([]SecureCLIBinary, error) + // IsRegisteredBinary reports whether a binary with the given name is + // registered and enabled for the tenant in ctx AND requires a grant + // (is_global = false). Used by the shell exec gate to hard-deny + // execution of credentialed binaries when the calling agent has no + // grant. is_global = true binaries are open to all agents and MUST + // NOT be reported as gate-needing. Tenant-scoped unless IsCrossTenant(ctx). + // Returns (false, nil) when name is empty or tenant context is missing. + IsRegisteredBinary(ctx context.Context, binaryName string) (bool, error) + // --- Per-user credential management --- GetUserCredentials(ctx context.Context, binaryID uuid.UUID, userID string) (*SecureCLIUserCredential, error) diff --git a/internal/store/sqlitestore/secure-cli.go b/internal/store/sqlitestore/secure-cli.go index da1de441..ac2ce199 100644 --- a/internal/store/sqlitestore/secure-cli.go +++ b/internal/store/sqlitestore/secure-cli.go @@ -9,6 +9,7 @@ import ( "errors" "fmt" "log/slog" + "strings" "time" "github.com/google/uuid" @@ -43,6 +44,10 @@ func (s *SQLiteSecureCLIStore) Create(ctx context.Context, b *store.SecureCLIBin b.ID = store.GenNewID() } + // Normalize binary_name to lowercase so IsRegisteredBinary (which lowercases + // the candidate) can match. Admin entering "Gh" becomes "gh". + b.BinaryName = strings.ToLower(strings.TrimSpace(b.BinaryName)) + var envBytes []byte if len(b.EncryptedEnv) > 0 && s.encKey != "" { encrypted, err := crypto.Encrypt(string(b.EncryptedEnv), s.encKey) @@ -191,6 +196,13 @@ func (s *SQLiteSecureCLIStore) Update(ctx context.Context, id uuid.UUID, updates } } + // Normalize binary_name to lowercase if updated (parity with Create). + if nameVal, ok := updates["binary_name"]; ok { + if nameStr, isStr := nameVal.(string); isStr { + updates["binary_name"] = strings.ToLower(strings.TrimSpace(nameStr)) + } + } + // Encrypt env if present in updates if envVal, ok := updates["encrypted_env"]; ok { if envStr, isStr := envVal.(string); isStr && envStr != "" && s.encKey != "" { @@ -410,6 +422,35 @@ func (s *SQLiteSecureCLIStore) ListEnabled(ctx context.Context) ([]store.SecureC return s.scanRows(rows) } +// IsRegisteredBinary reports whether a binary requires a grant (is_global=0) +// and is enabled for the current tenant. See interface godoc for rationale. +func (s *SQLiteSecureCLIStore) IsRegisteredBinary(ctx context.Context, binaryName string) (bool, error) { + name := strings.ToLower(strings.TrimSpace(binaryName)) + if name == "" { + return false, nil + } + query := `SELECT EXISTS( + SELECT 1 FROM secure_cli_binaries + WHERE LOWER(binary_name) = ? + AND enabled = 1 + AND is_global = 0` + args := []any{name} + if !store.IsCrossTenant(ctx) { + tid := store.TenantIDFromContext(ctx) + if tid == uuid.Nil { + return false, nil + } + query += ` AND tenant_id = ?` + args = append(args, tid) + } + query += `)` + var exists bool + if err := s.db.QueryRowContext(ctx, query, args...).Scan(&exists); err != nil { + return false, err + } + return exists, nil +} + // ListForAgent returns all CLIs accessible by an agent (global + granted), // with grant overrides merged into the returned configs. func (s *SQLiteSecureCLIStore) ListForAgent(ctx context.Context, agentID uuid.UUID) ([]store.SecureCLIBinary, error) { diff --git a/internal/store/sqlitestore/secure_cli_is_registered_test.go b/internal/store/sqlitestore/secure_cli_is_registered_test.go new file mode 100644 index 00000000..16d30823 --- /dev/null +++ b/internal/store/sqlitestore/secure_cli_is_registered_test.go @@ -0,0 +1,191 @@ +//go:build sqlite || sqliteonly + +package sqlitestore + +import ( + "context" + "database/sql" + "path/filepath" + "testing" + + "github.com/google/uuid" + + "github.com/nextlevelbuilder/goclaw/internal/store" +) + +// testEncKey is a 32-byte AES key used for SQLite secure-cli tests. +// Its actual value is irrelevant for IsRegisteredBinary (metadata-only query). +const testEncKey = "test-key-32-bytes-aaaaaaaaaaaaaaa" + +func newTestSQLiteSecureCLI(t *testing.T) (*SQLiteSecureCLIStore, *sql.DB) { + t.Helper() + db, err := OpenDB(filepath.Join(t.TempDir(), "secure_cli.db")) + if err != nil { + t.Fatalf("OpenDB: %v", err) + } + t.Cleanup(func() { _ = db.Close() }) + if err := EnsureSchema(db); err != nil { + t.Fatalf("EnsureSchema: %v", err) + } + return NewSQLiteSecureCLIStore(db, testEncKey), db +} + +// seedTenant inserts a tenant row and returns its ID. +func seedTenant(t *testing.T, db *sql.DB, slug string) uuid.UUID { + t.Helper() + id := uuid.New() + _, err := db.Exec( + `INSERT INTO tenants (id, name, slug, status) VALUES (?, ?, ?, 'active')`, + id, "Tenant-"+slug, slug, + ) + if err != nil { + t.Fatalf("seed tenant %s: %v", slug, err) + } + return id +} + +// seedBinary inserts a secure_cli_binaries row with the given fields. +func seedBinary(t *testing.T, db *sql.DB, tenantID uuid.UUID, name string, enabled, isGlobal bool) { + t.Helper() + _, err := db.Exec( + `INSERT INTO secure_cli_binaries + (id, binary_name, encrypted_env, is_global, enabled, tenant_id) + VALUES (?, ?, ?, ?, ?, ?)`, + uuid.New(), name, []byte("{}"), isGlobal, enabled, tenantID, + ) + if err != nil { + t.Fatalf("seed binary %s: %v", name, err) + } +} + +func TestSQLite_IsRegisteredBinary_ReturnsTrueForEnabledNonGlobal(t *testing.T) { + s, db := newTestSQLiteSecureCLI(t) + tid := seedTenant(t, db, "t-true") + seedBinary(t, db, tid, "gh", true, false) + + ctx := store.WithTenantID(context.Background(), tid) + got, err := s.IsRegisteredBinary(ctx, "gh") + if err != nil { + t.Fatalf("unexpected err: %v", err) + } + if !got { + t.Fatalf("expected true for enabled non-global binary") + } +} + +// Red Team F2 regression guard: is_global=true must NOT be reported as +// gate-needing — those binaries are open to all agents without a grant. +func TestSQLite_IsRegisteredBinary_FalseForGlobalBinary(t *testing.T) { + s, db := newTestSQLiteSecureCLI(t) + tid := seedTenant(t, db, "t-global") + seedBinary(t, db, tid, "ls", true, true) + + ctx := store.WithTenantID(context.Background(), tid) + got, err := s.IsRegisteredBinary(ctx, "ls") + if err != nil { + t.Fatalf("unexpected err: %v", err) + } + if got { + t.Fatalf("expected false for is_global=true binary (would deny access otherwise)") + } +} + +func TestSQLite_IsRegisteredBinary_FalseForDisabled(t *testing.T) { + s, db := newTestSQLiteSecureCLI(t) + tid := seedTenant(t, db, "t-disabled") + seedBinary(t, db, tid, "gh", false, false) + + ctx := store.WithTenantID(context.Background(), tid) + got, err := s.IsRegisteredBinary(ctx, "gh") + if err != nil { + t.Fatalf("unexpected err: %v", err) + } + if got { + t.Fatalf("expected false for disabled binary") + } +} + +func TestSQLite_IsRegisteredBinary_FalseForWrongTenant(t *testing.T) { + s, db := newTestSQLiteSecureCLI(t) + tidA := seedTenant(t, db, "t-a") + tidB := seedTenant(t, db, "t-b") + seedBinary(t, db, tidA, "gh", true, false) + + ctx := store.WithTenantID(context.Background(), tidB) + got, err := s.IsRegisteredBinary(ctx, "gh") + if err != nil { + t.Fatalf("unexpected err: %v", err) + } + if got { + t.Fatalf("expected false across tenants") + } +} + +func TestSQLite_IsRegisteredBinary_FalseForUnknownName(t *testing.T) { + s, db := newTestSQLiteSecureCLI(t) + tid := seedTenant(t, db, "t-unk") + ctx := store.WithTenantID(context.Background(), tid) + got, err := s.IsRegisteredBinary(ctx, "nonexistent") + if err != nil { + t.Fatalf("unexpected err: %v", err) + } + if got { + t.Fatalf("expected false for unknown name") + } +} + +func TestSQLite_IsRegisteredBinary_RespectsCrossTenant(t *testing.T) { + s, db := newTestSQLiteSecureCLI(t) + tid := seedTenant(t, db, "t-xtenant") + seedBinary(t, db, tid, "gh", true, false) + + ctx := store.WithCrossTenant(context.Background()) + got, err := s.IsRegisteredBinary(ctx, "gh") + if err != nil { + t.Fatalf("unexpected err: %v", err) + } + if !got { + t.Fatalf("expected true under cross-tenant ctx") + } +} + +func TestSQLite_IsRegisteredBinary_EmptyNameReturnsFalse(t *testing.T) { + s, _ := newTestSQLiteSecureCLI(t) + ctx := store.WithCrossTenant(context.Background()) + got, err := s.IsRegisteredBinary(ctx, "") + if err != nil { + t.Fatalf("unexpected err: %v", err) + } + if got { + t.Fatalf("expected false for empty name") + } +} + +func TestSQLite_IsRegisteredBinary_NilTenantNotCrossTenant(t *testing.T) { + s, _ := newTestSQLiteSecureCLI(t) + got, err := s.IsRegisteredBinary(context.Background(), "gh") + if err != nil { + t.Fatalf("unexpected err: %v", err) + } + if got { + t.Fatalf("expected false when tenant unset and not cross-tenant") + } +} + +// Red Team F8: case-insensitive match — macOS/APFS resolves GH → gh. +func TestSQLite_IsRegisteredBinary_CaseInsensitive(t *testing.T) { + s, db := newTestSQLiteSecureCLI(t) + tid := seedTenant(t, db, "t-case") + seedBinary(t, db, tid, "gh", true, false) + + ctx := store.WithTenantID(context.Background(), tid) + for _, q := range []string{"gh", "GH", "Gh", " gh "} { + got, err := s.IsRegisteredBinary(ctx, q) + if err != nil { + t.Fatalf("unexpected err for %q: %v", q, err) + } + if !got { + t.Fatalf("expected true for %q (case-insensitive)", q) + } + } +} diff --git a/internal/tools/credentialed_exec.go b/internal/tools/credentialed_exec.go index 695e74c4..291e84fe 100644 --- a/internal/tools/credentialed_exec.go +++ b/internal/tools/credentialed_exec.go @@ -10,6 +10,7 @@ import ( "maps" "os" "os/exec" + "path/filepath" "regexp" "slices" "strings" @@ -22,6 +23,141 @@ import ( "github.com/nextlevelbuilder/goclaw/internal/store" ) +// maxWrapperDepth is the hard cap on shell-wrapper unwrapping. Commands nested +// deeper than this are denied unconditionally as adversarial — real commands +// never wrap beyond depth 3. +const maxWrapperDepth = 3 + +// wrapperBinaries identifies shell/exec wrappers whose first arg after -c is +// the real command to gate. Key is the normalized base name. +var wrapperBinaries = map[string]bool{ + "sh": true, "bash": true, "zsh": true, "dash": true, + "env": true, "nohup": true, "stdbuf": true, "timeout": true, +} + +// normalizeBinaryName returns the lowercased file base of a binary reference. +// Examples: "/usr/bin/gh" → "gh", "./GH" → "gh", " Gh " → "gh". +// Applied at BOTH the gate lookup and lookupCredentialedBinary so the two +// layers agree on identity. (Red Team F5) +func normalizeBinaryName(s string) string { + return filepath.Base(strings.TrimSpace(strings.ToLower(s))) +} + +// detectWrapper recognises shell-wrapper invocations and returns the inner +// command string. Supported shapes: +// +// sh -c "" (also bash / zsh / dash / /bin/sh / /usr/bin/env sh ...) +// env [K=V ...] ... (no -c; the real binary is ) +// nohup ... +// stdbuf -oL ... +// timeout 10 ... +// +// Returns wrapper=normalized wrapper name, innerCmd=remaining command string. +// ok=false when cmd is not a recognised wrapper or parsing fails. +func detectWrapper(cmd string) (wrapper string, innerCmd string, ok bool) { + parser := shellwords.NewParser() + parser.ParseBacktick = false + parser.ParseEnv = false + words, err := parser.Parse(cmd) + if err != nil || len(words) == 0 { + return "", "", false + } + head := normalizeBinaryName(words[0]) + if !wrapperBinaries[head] { + return "", "", false + } + + switch head { + case "sh", "bash", "zsh", "dash": + // sh -c "" → inner is word[2]. + for i := 1; i < len(words); i++ { + if words[i] == "-c" && i+1 < len(words) { + return head, words[i+1], true + } + } + return "", "", false + case "env": + // env [K=V ...] [args...] OR env -S "" + for i := 1; i < len(words); i++ { + w := words[i] + if strings.Contains(w, "=") && !strings.HasPrefix(w, "-") { + continue // env var assignment, skip + } + if w == "-i" || w == "-u" || w == "-" { + continue + } + if strings.HasPrefix(w, "-") { + // Unknown env flag — bail out of wrapper detection. + return "", "", false + } + // First non-assignment, non-flag token is the real command. + return "env", strings.Join(append([]string{w}, words[i+1:]...), " "), true + } + return "", "", false + case "nohup": + if len(words) < 2 { + return "", "", false + } + return "nohup", strings.Join(words[1:], " "), true + case "stdbuf": + // stdbuf [-oL -eL -iL ...] ... + for i := 1; i < len(words); i++ { + if strings.HasPrefix(words[i], "-") { + continue + } + return "stdbuf", strings.Join(words[i:], " "), true + } + return "", "", false + case "timeout": + // timeout [--foreground] [-k DUR] ... + for i := 1; i < len(words); i++ { + w := words[i] + if strings.HasPrefix(w, "-") { + continue + } + // First non-flag token is the DURATION — skip it. + if i+1 < len(words) { + return "timeout", strings.Join(words[i+1:], " "), true + } + return "", "", false + } + return "", "", false + } + return "", "", false +} + +// gateCandidate is one step of wrapper unwrapping; it captures the binary +// name to gate and (optionally) the wrapper token that introduced it. +type gateCandidate struct { + binary string // normalized (lowercase, file base) + wrapper string // "" for outermost direct invocation; else wrapper name +} + +// collectGateCandidates returns the ordered list of binaries extracted from +// a command via recursive wrapper unwrapping (outermost → innermost). +// tooDeep=true when unwrapping exceeds maxWrapperDepth — callers must deny. +func collectGateCandidates(cmd string) (candidates []gateCandidate, tooDeep bool) { + current := cmd + wrapper := "" + for depth := 0; ; depth++ { + bin, _, err := parseCommandBinary(current) + if err != nil || bin == "" { + return candidates, false + } + norm := normalizeBinaryName(bin) + candidates = append(candidates, gateCandidate{binary: norm, wrapper: wrapper}) + w, inner, ok := detectWrapper(current) + if !ok || strings.TrimSpace(inner) == "" { + return candidates, false + } + if depth+1 > maxWrapperDepth { + return candidates, true + } + wrapper = w + current = inner + } +} + // shellOperatorPattern detects shell metacharacters that indicate command chaining. // These are unsafe in credentialed mode because they allow reading injected env vars. var shellOperatorPattern = regexp.MustCompile(`[;|&<>\n\r` + "`" + `]|\$\(|\$\{`) @@ -417,6 +553,10 @@ func (t *ExecTool) lookupCredentialedBinary(ctx context.Context, command string) if err != nil { return nil, "", nil } + // Normalize lookup key so path/case variants (/usr/bin/gh, ./gh, GH) all + // resolve to the same registry row. Same helper is used by the gate + // branch in Execute — identity must agree at both layers. (Red Team F5) + normBinary := normalizeBinaryName(binary) // Get agent ID from context for scoped lookup agentID := store.AgentIDFromContext(ctx) var agentIDPtr *uuid.UUID @@ -427,7 +567,7 @@ func (t *ExecTool) lookupCredentialedBinary(ctx context.Context, command string) // Uses CredentialUserIDFromContext to pick up merged tenant user identity // (falls back to UserIDFromContext when not set). userID := store.CredentialUserIDFromContext(ctx) - cred, err := t.secureCLIStore.LookupByBinary(ctx, binary, agentIDPtr, userID) + cred, err := t.secureCLIStore.LookupByBinary(ctx, normBinary, agentIDPtr, userID) if err != nil { slog.Warn("secure_cli.lookup: query failed", "binary", binary, "agent_id", agentID, "error", err) return nil, "", nil diff --git a/internal/tools/env_scrub.go b/internal/tools/env_scrub.go new file mode 100644 index 00000000..bd26f969 --- /dev/null +++ b/internal/tools/env_scrub.go @@ -0,0 +1,177 @@ +package tools + +import ( + "context" + "log/slog" + "strings" +) + +// staticCredentialEnvKeys are always stripped from fall-through exec env. +// These cover the most common provider / CLI credentials so an agent that +// runs a non-credentialed command cannot exfiltrate host secrets via the +// child process environment. +var staticCredentialEnvKeys = []string{ + "GH_TOKEN", + "GITHUB_TOKEN", + "GH_ENTERPRISE_TOKEN", + "GH_CONFIG_DIR", + "ANTHROPIC_API_KEY", + "OPENAI_API_KEY", + "NPM_TOKEN", + "DOCKER_PASSWORD", + "DOCKER_AUTH", + "AWS_ACCESS_KEY_ID", + "AWS_SECRET_ACCESS_KEY", + "AWS_SESSION_TOKEN", + "GOOGLE_APPLICATION_CREDENTIALS", + "HUGGINGFACE_TOKEN", + "HF_TOKEN", + "GCP_SA_KEY", + "AZURE_CLIENT_SECRET", + "CLOUDFLARE_API_TOKEN", +} + +// scrubCredentialEnv returns env with any KEY=VALUE pair removed whose key +// matches staticCredentialEnvKeys or any key in dynamicKeys. Comparison is +// case-sensitive per POSIX — env names are case-sensitive. +func scrubCredentialEnv(env []string, dynamicKeys []string) []string { + if len(env) == 0 { + return env + } + deny := make(map[string]struct{}, len(staticCredentialEnvKeys)+len(dynamicKeys)) + for _, k := range staticCredentialEnvKeys { + deny[k] = struct{}{} + } + for _, k := range dynamicKeys { + if k == "" { + continue + } + deny[k] = struct{}{} + } + out := make([]string, 0, len(env)) + for _, kv := range env { + i := strings.IndexByte(kv, '=') + if i <= 0 { + out = append(out, kv) + continue + } + if _, drop := deny[kv[:i]]; drop { + continue + } + out = append(out, kv) + } + return out +} + +// credentialEnvKeys returns the union of static deny-list keys and any +// dynamic keys discovered via the secure-cli registry (encrypted_env for +// each enabled binary scoped to the caller's tenant). +// +// No caching: tenant scope comes from ctx, and a shared ExecTool instance +// serves all tenants. Caching would require a per-tenant map with +// invalidation on binary Create/Update/Delete — out of scope here (YAGNI). +// This path runs only on host fall-through exec, which is already the +// slow path. One ListEnabled query is acceptable. +// +// Fails soft: store errors fall back to the static list — env scrubbing +// never blocks exec on a DB hiccup. +func (t *ExecTool) credentialEnvKeys(ctx context.Context) []string { + if t.secureCLIStore == nil { + return staticCredentialEnvKeys + } + + bins, err := t.secureCLIStore.ListEnabled(ctx) + if err != nil { + slog.Warn("security.credentialed_env_scrub_list_error", "error", err) + return staticCredentialEnvKeys + } + + seen := make(map[string]struct{}, len(staticCredentialEnvKeys)) + merged := make([]string, 0, len(staticCredentialEnvKeys)) + for _, k := range staticCredentialEnvKeys { + if _, ok := seen[k]; ok { + continue + } + seen[k] = struct{}{} + merged = append(merged, k) + } + for _, b := range bins { + for _, k := range extractJSONTopKeys(b.EncryptedEnv) { + if _, ok := seen[k]; ok { + continue + } + seen[k] = struct{}{} + merged = append(merged, k) + } + } + return merged +} + +// extractJSONTopKeys returns the top-level string keys of a JSON object +// without unmarshalling values. Tolerant: returns nil on any parse error. +// Kept local to avoid pulling json into env_scrub for a tiny helper. +func extractJSONTopKeys(data []byte) []string { + s := strings.TrimSpace(string(data)) + if !strings.HasPrefix(s, "{") || !strings.HasSuffix(s, "}") { + return nil + } + var keys []string + inner := s[1 : len(s)-1] + i := 0 + for i < len(inner) { + // skip whitespace + commas + for i < len(inner) && (inner[i] == ' ' || inner[i] == '\n' || inner[i] == '\t' || inner[i] == '\r' || inner[i] == ',') { + i++ + } + if i >= len(inner) || inner[i] != '"' { + return keys + } + i++ // consume opening quote + start := i + for i < len(inner) && inner[i] != '"' { + if inner[i] == '\\' && i+1 < len(inner) { + i += 2 + continue + } + i++ + } + if i > len(inner) { + return keys + } + keys = append(keys, inner[start:i]) + // advance past closing quote + optional spaces + colon + value + if i < len(inner) { + i++ + } + depth := 0 + inStr := false + for i < len(inner) { + c := inner[i] + if inStr { + if c == '\\' && i+1 < len(inner) { + i += 2 + continue + } + if c == '"' { + inStr = false + } + i++ + continue + } + switch c { + case '"': + inStr = true + case '{', '[': + depth++ + case '}', ']': + depth-- + } + if c == ',' && depth == 0 { + i++ + break + } + i++ + } + } + return keys +} diff --git a/internal/tools/env_scrub_test.go b/internal/tools/env_scrub_test.go new file mode 100644 index 00000000..c431239d --- /dev/null +++ b/internal/tools/env_scrub_test.go @@ -0,0 +1,114 @@ +package tools + +import ( + "strings" + "testing" +) + +func envContains(env []string, key string) bool { + prefix := key + "=" + for _, kv := range env { + if strings.HasPrefix(kv, prefix) { + return true + } + } + return false +} + +func TestScrubCredentialEnv_StripsStatic(t *testing.T) { + in := []string{ + "HOME=/root", + "GH_TOKEN=secret-abc", + "PATH=/usr/bin", + "AWS_SECRET_ACCESS_KEY=topsecret", + } + out := scrubCredentialEnv(in, nil) + + if envContains(out, "GH_TOKEN") { + t.Fatalf("GH_TOKEN must be scrubbed, got: %v", out) + } + if envContains(out, "AWS_SECRET_ACCESS_KEY") { + t.Fatalf("AWS_SECRET_ACCESS_KEY must be scrubbed, got: %v", out) + } + if !envContains(out, "HOME") || !envContains(out, "PATH") { + t.Fatalf("essential vars must be preserved, got: %v", out) + } +} + +func TestScrubCredentialEnv_StripsDynamic(t *testing.T) { + in := []string{ + "HOME=/root", + "MY_CUSTOM_SECRET=hello", + "KEEP_ME=yes", + } + out := scrubCredentialEnv(in, []string{"MY_CUSTOM_SECRET"}) + + if envContains(out, "MY_CUSTOM_SECRET") { + t.Fatalf("MY_CUSTOM_SECRET must be scrubbed, got: %v", out) + } + if !envContains(out, "KEEP_ME") { + t.Fatalf("KEEP_ME must be preserved, got: %v", out) + } +} + +func TestScrubCredentialEnv_PreservesEssentials(t *testing.T) { + in := []string{ + "HOME=/root", + "PATH=/usr/bin", + "TERM=xterm", + "LANG=en_US.UTF-8", + "USER=alice", + "TZ=UTC", + } + out := scrubCredentialEnv(in, nil) + for _, k := range []string{"HOME", "PATH", "TERM", "LANG", "USER", "TZ"} { + if !envContains(out, k) { + t.Fatalf("expected %s preserved, got: %v", k, out) + } + } +} + +func TestScrubCredentialEnv_PreservesUnrelated(t *testing.T) { + in := []string{ + "FOO=bar", + "RANDOM_APP_FLAG=1", + "NPM_TOKEN=leakme", // static deny-list → should be scrubbed + } + out := scrubCredentialEnv(in, nil) + if !envContains(out, "FOO") { + t.Fatalf("FOO must be preserved, got: %v", out) + } + if !envContains(out, "RANDOM_APP_FLAG") { + t.Fatalf("RANDOM_APP_FLAG must be preserved, got: %v", out) + } + if envContains(out, "NPM_TOKEN") { + t.Fatalf("NPM_TOKEN must be scrubbed (static), got: %v", out) + } +} + +// Basic smoke for the JSON key extractor used to feed dynamic keys. +func TestExtractJSONTopKeys_Simple(t *testing.T) { + keys := extractJSONTopKeys([]byte(`{"GH_TOKEN":"x","GH_CONFIG_DIR":"/tmp"}`)) + if len(keys) != 2 { + t.Fatalf("expected 2 keys, got %d: %v", len(keys), keys) + } + want := map[string]bool{"GH_TOKEN": true, "GH_CONFIG_DIR": true} + for _, k := range keys { + if !want[k] { + t.Fatalf("unexpected key %q", k) + } + } +} + +func TestExtractJSONTopKeys_NestedIgnored(t *testing.T) { + keys := extractJSONTopKeys([]byte(`{"A":{"nested":"v"},"B":"x"}`)) + if len(keys) != 2 { + t.Fatalf("expected 2 top keys, got %d: %v", len(keys), keys) + } +} + +func TestExtractJSONTopKeys_Malformed(t *testing.T) { + if keys := extractJSONTopKeys([]byte(`not json`)); keys != nil { + t.Fatalf("expected nil on malformed input, got %v", keys) + } +} diff --git a/internal/tools/shell.go b/internal/tools/shell.go index 17a09b91..bced1086 100644 --- a/internal/tools/shell.go +++ b/internal/tools/shell.go @@ -6,6 +6,7 @@ import ( "errors" "fmt" "log/slog" + "os" "os/exec" "regexp" "strings" @@ -109,6 +110,13 @@ func (t *ExecTool) SetSecureCLIStore(s store.SecureCLIStore) { t.secureCLIStore = s } +// HasSecureCLIStore reports whether a credential store is wired. +// Intended for wiring-check tests that verify subagent ExecTools also enforce +// the secure-CLI gate (Red Team F3). +func (t *ExecTool) HasSecureCLIStore() bool { + return t.secureCLIStore != nil +} + func (t *ExecTool) Name() string { return "exec" } func (t *ExecTool) Description() string { return "Execute a shell command and return its output" } func (t *ExecTool) Parameters() map[string]any { @@ -244,6 +252,45 @@ func (t *ExecTool) Execute(ctx context.Context, args map[string]any) *Result { return t.executeCredentialed(ctx, cred, binary, cmdArgs, cwd, sandboxKey, command) } + // Secure CLI gate: registered-but-not-granted binaries MUST NOT fall through + // to host exec with parent env. Works on the already-normalized command + // (Red Team F6) and unwraps shell wrappers up to depth 3 (Red Team F1). + // Fails CLOSED on DB error (Red Team F7). + if t.secureCLIStore != nil { + candidates, tooDeep := collectGateCandidates(normalizedCommand) + if tooDeep { + slog.Warn("security.credentialed_binary_wrapper_too_deep", + "command", truncateCmd(normalizedCommand, 80), + "agent_id", store.AgentIDFromContext(ctx)) + return ErrorResult("Command nesting too deep (>3 shell wrappers). This looks adversarial; if legitimate, flatten the command.") + } + for _, c := range candidates { + if c.binary == "" { + continue + } + gctx, cancel := context.WithTimeout(ctx, 2*time.Second) + registered, rerr := t.secureCLIStore.IsRegisteredBinary(gctx, c.binary) + cancel() + if rerr != nil { + slog.Warn("security.credentialed_binary_gate_error", + "binary", c.binary, "error", rerr, + "agent_id", store.AgentIDFromContext(ctx)) + return ErrorResult("Secure CLI gate temporarily unavailable. Retry in a moment.") + } + if registered { + slog.Warn("security.credentialed_binary_denied", + "binary", c.binary, + "wrapper", c.wrapper, + "agent_id", store.AgentIDFromContext(ctx), + "tenant_id", store.TenantIDFromContext(ctx), + "command_prefix", truncateCmd(normalizedCommand, 80)) + return ErrorResult(fmt.Sprintf( + "Binary %q requires a secure CLI grant. Ask admin to grant access to this agent.", + c.binary)) + } + } + } + // Exec approval check (matching TS exec-approval.ts pipeline) if t.approvalMgr != nil { switch t.approvalMgr.CheckCommand(command) { @@ -318,6 +365,17 @@ func (t *ExecTool) executeOnHost(ctx context.Context, command, cwd string) *Resu cmd := exec.Command("sh", "-c", command) cmd.Dir = cwd + // Scrub credential env vars so fall-through exec cannot exfiltrate + // host secrets (Red Team F4). Uses static deny list + dynamic keys + // discovered from any registered secure-cli binary for this tenant. + var dynKeys []string + if t.secureCLIStore != nil { + dynKeys = t.credentialEnvKeys(ctx) + } else { + dynKeys = staticCredentialEnvKeys + } + cmd.Env = scrubCredentialEnv(os.Environ(), dynKeys) + // Place the child in its own process group so killProcessGroup(-pgid, sig) // reaches the shell and all of its forked children. setProcessGroup(cmd) diff --git a/internal/tools/shell_credentialed_gate_test.go b/internal/tools/shell_credentialed_gate_test.go new file mode 100644 index 00000000..f815f8b4 --- /dev/null +++ b/internal/tools/shell_credentialed_gate_test.go @@ -0,0 +1,404 @@ +package tools + +import ( + "context" + "encoding/json" + "errors" + "strings" + "sync" + "testing" + "time" + + "github.com/google/uuid" + + "github.com/nextlevelbuilder/goclaw/internal/store" +) + +// stubSecureCLIStore is a minimal in-memory SecureCLIStore used by shell gate +// unit tests. Only LookupByBinary (Phase 1) and IsRegisteredBinary (Phase 2) +// have meaningful logic; the rest return zero values to satisfy the interface. +type stubSecureCLIStore struct { + mu sync.Mutex + byName map[string]*store.SecureCLIBinary + registered map[string]bool + lookupCalls int + isRegisteredErr error + // isRegisteredSleep makes IsRegisteredBinary block for this duration to + // exercise the 2s fail-closed timeout in Phase 3. + isRegisteredSleep time.Duration + // lastLookupName captures the exact name passed to LookupByBinary so + // tests can assert normalization happened before the call. + lastLookupName string +} + +func newStubSecureCLIStore() *stubSecureCLIStore { + return &stubSecureCLIStore{ + byName: map[string]*store.SecureCLIBinary{}, + registered: map[string]bool{}, + } +} + +// --- Meaningful methods --- + +func (s *stubSecureCLIStore) LookupByBinary(ctx context.Context, binaryName string, agentID *uuid.UUID, userID string) (*store.SecureCLIBinary, error) { + s.mu.Lock() + defer s.mu.Unlock() + s.lookupCalls++ + s.lastLookupName = binaryName + return s.byName[binaryName], nil +} + +// IsRegisteredBinary is exercised by Phase 3 tests. Keeping the stub +// implementation here keeps the interface satisfied from Phase 2 onwards. +func (s *stubSecureCLIStore) IsRegisteredBinary(ctx context.Context, binaryName string) (bool, error) { + if s.isRegisteredSleep > 0 { + select { + case <-time.After(s.isRegisteredSleep): + case <-ctx.Done(): + return false, ctx.Err() + } + } + s.mu.Lock() + defer s.mu.Unlock() + if s.isRegisteredErr != nil { + return false, s.isRegisteredErr + } + return s.registered[binaryName], nil +} + +// --- Zero-value stubs for interface satisfaction --- + +func (s *stubSecureCLIStore) Create(ctx context.Context, b *store.SecureCLIBinary) error { + return nil +} +func (s *stubSecureCLIStore) Get(ctx context.Context, id uuid.UUID) (*store.SecureCLIBinary, error) { + return nil, nil +} +func (s *stubSecureCLIStore) Update(ctx context.Context, id uuid.UUID, updates map[string]any) error { + return nil +} +func (s *stubSecureCLIStore) Delete(ctx context.Context, id uuid.UUID) error { return nil } +func (s *stubSecureCLIStore) List(ctx context.Context) ([]store.SecureCLIBinary, error) { + return nil, nil +} +func (s *stubSecureCLIStore) ListEnabled(ctx context.Context) ([]store.SecureCLIBinary, error) { + s.mu.Lock() + defer s.mu.Unlock() + out := make([]store.SecureCLIBinary, 0, len(s.byName)) + for _, b := range s.byName { + if b != nil { + out = append(out, *b) + } + } + return out, nil +} +func (s *stubSecureCLIStore) ListForAgent(ctx context.Context, agentID uuid.UUID) ([]store.SecureCLIBinary, error) { + return nil, nil +} +func (s *stubSecureCLIStore) GetUserCredentials(ctx context.Context, binaryID uuid.UUID, userID string) (*store.SecureCLIUserCredential, error) { + return nil, nil +} +func (s *stubSecureCLIStore) SetUserCredentials(ctx context.Context, binaryID uuid.UUID, userID string, encryptedEnv []byte) error { + return nil +} +func (s *stubSecureCLIStore) DeleteUserCredentials(ctx context.Context, binaryID uuid.UUID, userID string) error { + return nil +} +func (s *stubSecureCLIStore) ListUserCredentials(ctx context.Context, binaryID uuid.UUID) ([]store.SecureCLIUserCredential, error) { + return nil, nil +} + +// --- Phase 1 characterization tests --- + +// TestExec_NoStoreWired_FallsThrough confirms that when secureCLIStore is nil +// (Lite edition / missing encryption key), Execute falls through to host exec. +func TestExec_NoStoreWired_FallsThrough(t *testing.T) { + tool := NewExecTool(t.TempDir(), false) + result := tool.Execute(context.Background(), map[string]any{"command": "echo hello"}) + if result.IsError { + t.Fatalf("expected success, got error: %s", result.ForLLM) + } + if !strings.Contains(result.ForLLM, "hello") { + t.Fatalf("expected output to contain %q, got %q", "hello", result.ForLLM) + } +} + +// TestExec_UnregisteredBinary_FallsThrough: stub wired but zero entries — +// unregistered binary should fall through to host exec unchanged. +func TestExec_UnregisteredBinary_FallsThrough(t *testing.T) { + stub := newStubSecureCLIStore() + tool := NewExecTool(t.TempDir(), false) + tool.SetSecureCLIStore(stub) + + ctx := store.WithTenantID(store.WithAgentID(context.Background(), uuid.New()), uuid.New()) + result := tool.Execute(ctx, map[string]any{"command": "echo hello"}) + + if result.IsError { + t.Fatalf("expected success, got error: %s", result.ForLLM) + } + if !strings.Contains(result.ForLLM, "hello") { + t.Fatalf("expected output to contain %q, got %q", "hello", result.ForLLM) + } + stub.mu.Lock() + calls := stub.lookupCalls + stub.mu.Unlock() + if calls != 1 { + t.Fatalf("expected lookupCalls == 1 (gate consulted), got %d", calls) + } +} + +// TestExec_GrantedBinary_UsesCredentialedPath — Red Team F9 applied: +// sentinel binary name (not echo) proves the credentialed branch was taken. +func TestExec_GrantedBinary_UsesCredentialedPath(t *testing.T) { + const sentinel = "xyz_goclaw_sentinel" + stub := newStubSecureCLIStore() + stub.byName[sentinel] = &store.SecureCLIBinary{ + BinaryName: sentinel, + EncryptedEnv: []byte("{}"), + TimeoutSeconds: 10, + DenyArgs: json.RawMessage("[]"), + DenyVerbose: json.RawMessage("[]"), + } + + tool := NewExecTool(t.TempDir(), false) + tool.SetSecureCLIStore(stub) + + ctx := store.WithTenantID(store.WithAgentID(context.Background(), uuid.New()), uuid.New()) + result := tool.Execute(ctx, map[string]any{"command": sentinel + " --help"}) + + stub.mu.Lock() + calls := stub.lookupCalls + stub.mu.Unlock() + if calls != 1 { + t.Fatalf("expected lookupCalls == 1 (gate consulted store), got %d", calls) + } + if !result.IsError { + t.Fatalf("expected IsError=true (credentialed branch entered but binary not on PATH), got %+v", result) + } + // Soft sanity: the error must NOT come from sh fall-through. A fall-through + // error would typically contain "sh: 1:" or the shell path — proving we + // took a different code path than intended. + if strings.Contains(result.ForLLM, "sh: 1:") || strings.Contains(result.ForLLM, "/bin/sh") { + t.Fatalf("expected credentialed-path error, got shell fall-through error: %s", result.ForLLM) + } +} + +// --- Phase 3 gate-enforcement tests --- + +// helper: build a gate-wired ExecTool with a fresh stub + ctx. +func newGateTestTool(t *testing.T) (*ExecTool, *stubSecureCLIStore, context.Context) { + t.Helper() + stub := newStubSecureCLIStore() + tool := NewExecTool(t.TempDir(), false) + tool.SetSecureCLIStore(stub) + ctx := store.WithTenantID(store.WithAgentID(context.Background(), uuid.New()), uuid.New()) + return tool, stub, ctx +} + +// TestExec_BlocksRegisteredBinaryWhenNoGrant: registered without grant → deny. +func TestExec_BlocksRegisteredBinaryWhenNoGrant(t *testing.T) { + tool, stub, ctx := newGateTestTool(t) + stub.registered["gh"] = true + + result := tool.Execute(ctx, map[string]any{"command": "gh auth status"}) + if !result.IsError { + t.Fatalf("expected deny, got success: %+v", result) + } + if !strings.Contains(result.ForLLM, "requires a secure CLI grant") { + t.Fatalf("expected grant-required message, got: %s", result.ForLLM) + } + if !strings.Contains(result.ForLLM, "gh") { + t.Fatalf("expected binary name in message, got: %s", result.ForLLM) + } +} + +// TestExec_AllowsUnregisteredBinary: gate passes through when no match. +func TestExec_AllowsUnregisteredBinary(t *testing.T) { + tool, _, ctx := newGateTestTool(t) + result := tool.Execute(ctx, map[string]any{"command": "echo hello"}) + if result.IsError { + t.Fatalf("expected pass-through, got error: %s", result.ForLLM) + } + if !strings.Contains(result.ForLLM, "hello") { + t.Fatalf("expected echo output, got: %s", result.ForLLM) + } +} + +// TestExec_BinaryNameNormalization (Red Team F5/F8): case + path variants all match. +func TestExec_BinaryNameNormalization(t *testing.T) { + cases := []string{ + "gh version", + "/usr/bin/gh version", + "./gh version", + "GH version", + "Gh version", + " gh version ", + } + for _, cmd := range cases { + t.Run(cmd, func(t *testing.T) { + tool, stub, ctx := newGateTestTool(t) + stub.registered["gh"] = true + result := tool.Execute(ctx, map[string]any{"command": cmd}) + if !result.IsError { + t.Fatalf("expected deny for %q, got success: %+v", cmd, result) + } + if !strings.Contains(result.ForLLM, "requires a secure CLI grant") { + t.Fatalf("expected grant-required message for %q, got: %s", cmd, result.ForLLM) + } + }) + } +} + +// Shell-wrapper denials (Red Team F1). +func TestExec_BlocksShellWrapper_ShDashC(t *testing.T) { + tool, stub, ctx := newGateTestTool(t) + stub.registered["gh"] = true + result := tool.Execute(ctx, map[string]any{"command": "sh -c 'gh auth status'"}) + if !result.IsError || !strings.Contains(result.ForLLM, "requires a secure CLI grant") { + t.Fatalf("expected deny through sh -c wrapper, got: %+v", result) + } +} + +func TestExec_BlocksShellWrapper_BashDashC(t *testing.T) { + tool, stub, ctx := newGateTestTool(t) + stub.registered["gh"] = true + result := tool.Execute(ctx, map[string]any{"command": `bash -c "gh api repos/x/y"`}) + if !result.IsError || !strings.Contains(result.ForLLM, "requires a secure CLI grant") { + t.Fatalf("expected deny through bash -c wrapper, got: %+v", result) + } +} + +func TestExec_BlocksShellWrapper_SlashBinSh(t *testing.T) { + tool, stub, ctx := newGateTestTool(t) + stub.registered["gh"] = true + result := tool.Execute(ctx, map[string]any{"command": "/bin/sh -c 'gh auth status'"}) + if !result.IsError || !strings.Contains(result.ForLLM, "requires a secure CLI grant") { + t.Fatalf("expected deny through /bin/sh -c, got: %+v", result) + } +} + +func TestExec_BlocksEnvWrapper(t *testing.T) { + tool, stub, ctx := newGateTestTool(t) + stub.registered["gh"] = true + result := tool.Execute(ctx, map[string]any{"command": "env GH_TOKEN=x gh auth status"}) + if !result.IsError || !strings.Contains(result.ForLLM, "requires a secure CLI grant") { + t.Fatalf("expected deny through env wrapper, got: %+v", result) + } +} + +func TestExec_BlocksEnvUsrBinEnvSh(t *testing.T) { + tool, stub, ctx := newGateTestTool(t) + stub.registered["gh"] = true + result := tool.Execute(ctx, map[string]any{"command": "/usr/bin/env sh -c 'gh api'"}) + if !result.IsError || !strings.Contains(result.ForLLM, "requires a secure CLI grant") { + t.Fatalf("expected deny through env→sh wrapper, got: %+v", result) + } +} + +func TestExec_BlocksNestedWrapper_Depth3(t *testing.T) { + tool, stub, ctx := newGateTestTool(t) + stub.registered["gh"] = true + // bash -c "sh -c 'env GH_TOKEN=x gh auth'" + cmd := `bash -c "sh -c 'env GH_TOKEN=x gh auth'"` + result := tool.Execute(ctx, map[string]any{"command": cmd}) + if !result.IsError || !strings.Contains(result.ForLLM, "requires a secure CLI grant") { + t.Fatalf("expected deny through depth-3 wrapper, got: %+v", result) + } +} + +// TestExec_RejectsWrapperDepthCap (Validation Session 1): depth 4+ → unconditional deny. +func TestExec_RejectsWrapperDepthCap(t *testing.T) { + tool, _, ctx := newGateTestTool(t) + // 4-level nesting: bash -c 'sh -c "bash -c \"sh -c gh\"..."' + cmd := `bash -c "sh -c 'bash -c \"sh -c gh\"'"` + result := tool.Execute(ctx, map[string]any{"command": cmd}) + if !result.IsError { + t.Fatalf("expected deny for depth-4+ nesting, got: %+v", result) + } + if !strings.Contains(result.ForLLM, "Command nesting too deep") { + t.Fatalf("expected nesting-too-deep message, got: %s", result.ForLLM) + } +} + +func TestExec_AllowsShellWrapperWithUnregisteredInner(t *testing.T) { + tool, _, ctx := newGateTestTool(t) + // registered empty, inner is echo (not registered) → fall-through. + result := tool.Execute(ctx, map[string]any{"command": "sh -c 'echo hi'"}) + if result.IsError { + t.Fatalf("expected pass-through when inner unregistered, got error: %s", result.ForLLM) + } + if !strings.Contains(result.ForLLM, "hi") { + t.Fatalf("expected echo output, got: %s", result.ForLLM) + } +} + +// Normalization consistency at LookupByBinary (Red Team F5). +func TestExec_LookupPathNormalizes(t *testing.T) { + const sentinel = "xyz_goclaw_sentinel" + tool, stub, ctx := newGateTestTool(t) + stub.byName[sentinel] = &store.SecureCLIBinary{ + BinaryName: sentinel, + EncryptedEnv: []byte("{}"), + TimeoutSeconds: 10, + DenyArgs: json.RawMessage("[]"), + DenyVerbose: json.RawMessage("[]"), + } + // Provide path-prefixed, mixed-case, padded form. + _ = tool.Execute(ctx, map[string]any{"command": " /usr/local/bin/XYZ_GOCLAW_SENTINEL --help "}) + stub.mu.Lock() + got := stub.lastLookupName + stub.mu.Unlock() + if got != sentinel { + t.Fatalf("expected LookupByBinary called with normalized %q, got %q", sentinel, got) + } +} + +// Fail-CLOSED on gate DB error (Red Team F7). +func TestExec_GateDbError_FailsClosed(t *testing.T) { + tool, stub, ctx := newGateTestTool(t) + stub.isRegisteredErr = errors.New("simulated db outage") + result := tool.Execute(ctx, map[string]any{"command": "echo hello"}) + if !result.IsError { + t.Fatalf("expected fail-closed, got success: %+v", result) + } + if !strings.Contains(result.ForLLM, "Secure CLI gate temporarily unavailable") { + t.Fatalf("expected gate-unavailable message, got: %s", result.ForLLM) + } + if strings.Contains(result.ForLLM, "hello") { + t.Fatalf("command must NOT execute on gate error, got: %s", result.ForLLM) + } +} + +// Fail-CLOSED on gate DB timeout (Red Team F7). +func TestExec_GateDbTimeout_FailsClosed(t *testing.T) { + tool, stub, ctx := newGateTestTool(t) + stub.isRegisteredSleep = 3 * time.Second + start := time.Now() + result := tool.Execute(ctx, map[string]any{"command": "echo hello"}) + elapsed := time.Since(start) + if !result.IsError { + t.Fatalf("expected fail-closed on timeout, got success: %+v", result) + } + if !strings.Contains(result.ForLLM, "Secure CLI gate temporarily unavailable") { + t.Fatalf("expected gate-unavailable message, got: %s", result.ForLLM) + } + // 2s context timeout + some slack. Must not wait full 3s sleep. + if elapsed > 2500*time.Millisecond { + t.Fatalf("expected gate timeout ≤2.5s, took %s", elapsed) + } +} + +// Env scrub in fall-through exec (Red Team F4): GH_TOKEN must not leak into child. +func TestExec_FallThrough_ScrubsGHToken(t *testing.T) { + tool, _, ctx := newGateTestTool(t) + t.Setenv("GH_TOKEN", "supersecretvalue") + // Use single-quote printf so shell sees the literal; our gate lets "sh" + // fall through (sh is not registered, echo is not registered). + result := tool.Execute(ctx, map[string]any{"command": `sh -c 'echo "token=$GH_TOKEN"'`}) + if result.IsError { + t.Fatalf("expected pass-through, got: %s", result.ForLLM) + } + if strings.Contains(result.ForLLM, "supersecretvalue") { + t.Fatalf("GH_TOKEN leaked to child process: %s", result.ForLLM) + } +} diff --git a/tests/integration/secure_cli_gate_enforcement_test.go b/tests/integration/secure_cli_gate_enforcement_test.go new file mode 100644 index 00000000..01f309ec --- /dev/null +++ b/tests/integration/secure_cli_gate_enforcement_test.go @@ -0,0 +1,229 @@ +//go:build integration + +package integration + +import ( + "context" + "database/sql" + "strings" + "testing" + + "github.com/google/uuid" + + "github.com/nextlevelbuilder/goclaw/internal/store" + "github.com/nextlevelbuilder/goclaw/internal/store/pg" + "github.com/nextlevelbuilder/goclaw/internal/tools" +) + +// gateTestBinaryName is deliberately NOT a real binary on PATH so the +// "allowed" path can never accidentally exec something real. +const gateTestBinaryName = "goclaw_test_cli" + +// gateFixture holds the common seeds for gate enforcement tests. +type gateFixture struct { + db *sql.DB + tool *tools.ExecTool + tenantID uuid.UUID + agentA uuid.UUID + agentB uuid.UUID + binaryID uuid.UUID +} + +// setupGateTest seeds: tenant, two agents under that SAME tenant (Red Team F10), +// a non-global registered binary, and a grant only for agentA. Returns a wired +// ExecTool + the IDs. All rows are cleaned up via t.Cleanup. +func setupGateTest(t *testing.T) *gateFixture { + t.Helper() + + db := testDB(t) + tenantID, agentA := seedTenantAgent(t, db) + agentB := seedSecondAgent(t, db, tenantID) + binaryID := seedGateBinary(t, db, tenantID, gateTestBinaryName, false) + seedGrant(t, db, tenantID, binaryID, agentA) + + secStore := pg.NewPGSecureCLIStore(db, testEncryptionKey) + tool := tools.NewExecTool(t.TempDir(), false) + tool.SetSecureCLIStore(secStore) + + return &gateFixture{ + db: db, + tool: tool, + tenantID: tenantID, + agentA: agentA, + agentB: agentB, + binaryID: binaryID, + } +} + +// seedSecondAgent inserts an additional agent under an existing tenant. Used +// to satisfy Red Team F10: both test agents must share the same tenant to +// prove the gate (not tenant isolation) is what denies ungranted exec. +func seedSecondAgent(t *testing.T, db *sql.DB, tenantID uuid.UUID) uuid.UUID { + t.Helper() + + agentID := uuid.New() + agentKey := "test-" + agentID.String()[:8] + + _, err := db.Exec( + `INSERT INTO agents (id, tenant_id, agent_key, agent_type, status, provider, model, owner_id) + VALUES ($1, $2, $3, 'predefined', 'active', 'test', 'test-model', 'test-owner')`, + agentID, tenantID, agentKey) + if err != nil { + t.Fatalf("seed second agent: %v", err) + } + + t.Cleanup(func() { + db.Exec("DELETE FROM agents WHERE id = $1", agentID) + }) + + return agentID +} + +// seedGateBinary inserts a secure_cli_binaries row with a caller-specified +// binary_name and is_global flag. Matches shape used by seedSecureCLI. +func seedGateBinary(t *testing.T, db *sql.DB, tenantID uuid.UUID, name string, isGlobal bool) uuid.UUID { + t.Helper() + + binaryID := uuid.New() + _, err := db.Exec( + `INSERT INTO secure_cli_binaries (id, tenant_id, binary_name, encrypted_env, description, enabled, is_global) + VALUES ($1, $2, $3, $4, 'gate test CLI', true, $5)`, + binaryID, tenantID, name, []byte(`{}`), isGlobal) + if err != nil { + t.Fatalf("seed gate binary: %v", err) + } + + t.Cleanup(func() { + db.Exec("DELETE FROM secure_cli_agent_grants WHERE binary_id = $1", binaryID) + db.Exec("DELETE FROM secure_cli_user_credentials WHERE binary_id = $1", binaryID) + db.Exec("DELETE FROM secure_cli_binaries WHERE id = $1", binaryID) + }) + + return binaryID +} + +// seedGrant inserts a secure_cli_agent_grants row tying an agent to a binary. +func seedGrant(t *testing.T, db *sql.DB, tenantID, binaryID, agentID uuid.UUID) { + t.Helper() + + _, err := db.Exec( + `INSERT INTO secure_cli_agent_grants (binary_id, agent_id, tenant_id, enabled) + VALUES ($1, $2, $3, true)`, + binaryID, agentID, tenantID) + if err != nil { + t.Fatalf("seed grant: %v", err) + } +} + +// gateCtx builds a ctx with tenant + agent set for gate enforcement. +func gateCtx(tenantID, agentID uuid.UUID) context.Context { + ctx := store.WithTenantID(context.Background(), tenantID) + return store.WithAgentID(ctx, agentID) +} + +// TestSecureCLIGate_DeniesUngranted proves the gate denies ungranted agents +// for a registered, non-global binary (FR2 — the primary fix). +func TestSecureCLIGate_DeniesUngranted(t *testing.T) { + t.Parallel() + + f := setupGateTest(t) + ctx := gateCtx(f.tenantID, f.agentB) + + result := f.tool.Execute(ctx, map[string]any{ + "command": gateTestBinaryName + " --help", + }) + + if !result.IsError { + t.Fatalf("expected IsError=true for ungranted exec, got: %+v", result) + } + if !strings.Contains(result.ForLLM, "requires a secure CLI grant") { + t.Fatalf("expected deny message, got: %s", result.ForLLM) + } +} + +// TestSecureCLIGate_AllowsGrantedAgent proves the gate permits a granted +// agent past the deny branch (FR3). The downstream credentialed exec may +// still fail because the binary is not on PATH — we assert only that the +// deny message is NOT returned. +func TestSecureCLIGate_AllowsGrantedAgent(t *testing.T) { + t.Parallel() + + f := setupGateTest(t) + ctx := gateCtx(f.tenantID, f.agentA) + + result := f.tool.Execute(ctx, map[string]any{ + "command": gateTestBinaryName + " --help", + }) + + if strings.Contains(result.ForLLM, "requires a secure CLI grant") { + t.Fatalf("granted agent unexpectedly denied: %s", result.ForLLM) + } +} + +// TestSecureCLIGate_UnregisteredBinaryUnchanged proves the gate is a no-op +// for binaries not in the registry (FR4). `echo` must run normally. +func TestSecureCLIGate_UnregisteredBinaryUnchanged(t *testing.T) { + t.Parallel() + + db := testDB(t) + tenantID, agentID := seedTenantAgent(t, db) + + secStore := pg.NewPGSecureCLIStore(db, testEncryptionKey) + tool := tools.NewExecTool(t.TempDir(), false) + tool.SetSecureCLIStore(secStore) + + ctx := gateCtx(tenantID, agentID) + result := tool.Execute(ctx, map[string]any{ + "command": "echo hello", + }) + + if result.IsError { + t.Fatalf("expected no error for unregistered binary, got: %s", result.ForLLM) + } + if !strings.Contains(result.ForLLM, "hello") { + t.Fatalf("expected output to contain 'hello', got: %s", result.ForLLM) + } +} + +// TestSecureCLIGate_IsGlobalBinaryNotDenied is Red Team F2 regression guard. +// A global binary (is_global=true) needs no grant; the gate must NOT deny. +func TestSecureCLIGate_IsGlobalBinaryNotDenied(t *testing.T) { + t.Parallel() + + db := testDB(t) + tenantID, agentID := seedTenantAgent(t, db) + seedGateBinary(t, db, tenantID, "goclaw_global_test", true) + + secStore := pg.NewPGSecureCLIStore(db, testEncryptionKey) + tool := tools.NewExecTool(t.TempDir(), false) + tool.SetSecureCLIStore(secStore) + + ctx := gateCtx(tenantID, agentID) + result := tool.Execute(ctx, map[string]any{ + "command": "goclaw_global_test --help", + }) + + if strings.Contains(result.ForLLM, "requires a secure CLI grant") { + t.Fatalf("global binary unexpectedly denied by gate: %s", result.ForLLM) + } +} + +// TestSecureCLIGate_ShellWrapperBypassDenied is Red Team F1 integration guard. +// Wrapping the registered binary in `sh -c '...'` must still hit the deny path. +func TestSecureCLIGate_ShellWrapperBypassDenied(t *testing.T) { + t.Parallel() + + f := setupGateTest(t) + ctx := gateCtx(f.tenantID, f.agentB) + + result := f.tool.Execute(ctx, map[string]any{ + "command": "sh -c '" + gateTestBinaryName + " --help'", + }) + + if !result.IsError { + t.Fatalf("expected IsError=true for wrapped exec, got: %+v", result) + } + if !strings.Contains(result.ForLLM, "requires a secure CLI grant") { + t.Fatalf("expected deny message for sh -c wrap, got: %s", result.ForLLM) + } +}