diff --git a/internal/coordinator/handlers_agent.go b/internal/coordinator/handlers_agent.go index 8b37298..682b513 100644 --- a/internal/coordinator/handlers_agent.go +++ b/internal/coordinator/handlers_agent.go @@ -1543,6 +1543,27 @@ func (s *Server) handleCreateAgents(w http.ResponseWriter, r *http.Request, spac return } + // Space enforcement (SEC-007): When a parent is specified, enforce that the + // new agent is created in the same space as the parent. This prevents agents + // from being spawned in arbitrary spaces via the HTTP API. + if req.Parent != "" { + s.mu.RLock() + parentSpace, parentFound := s.findAgentSpace(req.Parent) + s.mu.RUnlock() + + if !parentFound { + writeJSONError(w, fmt.Sprintf("parent agent %q not found in any space", req.Parent), http.StatusBadRequest) + return + } + if parentSpace != spaceName { + writeJSONError(w, fmt.Sprintf( + "space enforcement: agent %q in space %q cannot spawn agents in space %q", + req.Parent, parentSpace, spaceName, + ), http.StatusForbidden) + return + } + } + var createOpts SessionCreateOpts if backend.Name() == "ambient" { sessionName := tmuxDefaultSession(spaceName, req.Name) diff --git a/internal/coordinator/helpers.go b/internal/coordinator/helpers.go index b51f9c5..b4f5a92 100644 --- a/internal/coordinator/helpers.go +++ b/internal/coordinator/helpers.go @@ -187,3 +187,16 @@ func pruneNotifications(ag *AgentUpdate) { } ag.Notifications = append(unread, read...) } + +// findAgentSpace searches all spaces to find which space contains the specified agent. +// Returns the space name and true if found, empty string and false otherwise. +// Must be called with s.mu.RLock held. +func (s *Server) findAgentSpace(agentName string) (string, bool) { + for spaceName, ks := range s.spaces { + canonical := resolveAgentName(ks, agentName) + if _, exists := ks.Agents[canonical]; exists { + return spaceName, true + } + } + return "", false +} diff --git a/internal/coordinator/mcp_tools.go b/internal/coordinator/mcp_tools.go index 476cd8e..ca84241 100644 --- a/internal/coordinator/mcp_tools.go +++ b/internal/coordinator/mcp_tools.go @@ -1088,6 +1088,25 @@ func (s *Server) addToolSpawnAgent(srv *mcp.Server) { workDir := strArg(args, "work_dir") spawnerName := strArg(args, "parent") + // Space enforcement (SEC-007): When a parent/spawner is specified, enforce + // that the new agent is created in the same space as the parent. This prevents + // agents from spawning other agents in arbitrary spaces. + if spawnerName != "" { + s.mu.RLock() + spawnerSpace, spawnerFound := s.findAgentSpace(spawnerName) + s.mu.RUnlock() + + if !spawnerFound { + return toolError(fmt.Sprintf("parent agent %q not found in any space", spawnerName)), nil + } + if spawnerSpace != spaceName { + return toolError(fmt.Sprintf( + "space enforcement: agent %q in space %q cannot spawn agents in space %q", + spawnerName, spawnerSpace, spaceName, + )), nil + } + } + if workDir != "" { // Ensure an AgentConfig entry exists so spawnAgentService picks up the workDir. s.mu.Lock() diff --git a/internal/coordinator/server_test.go b/internal/coordinator/server_test.go index 0246cc3..24e4093 100644 --- a/internal/coordinator/server_test.go +++ b/internal/coordinator/server_test.go @@ -4850,3 +4850,132 @@ func TestOperatorExcludedFromHierarchy(t *testing.T) { t.Errorf("operator should not appear in hierarchy nodes") } } +// TestSpawnAgentSpaceEnforcement verifies SEC-007: agents can only spawn other +// agents in their own space, not in arbitrary spaces. This prevents cross-space +// spawning attacks where an agent in space-a creates agents in space-b. +func TestSpawnAgentSpaceEnforcement(t *testing.T) { + srv, cleanup := mustStartServer(t) + defer cleanup() + base := serverBaseURL(srv) + + // Setup: Create a parent agent in space-a + spaceA := "space-a" + parentAgent := "parent-agent" + + // Register parent agent in space-a via direct API call + payload := map[string]any{ + "status": "active", + "summary": parentAgent + ": parent in space-a", + } + resp := postJSON(t, base+"/spaces/"+spaceA+"/agent/"+parentAgent, payload) + resp.Body.Close() + if resp.StatusCode != http.StatusAccepted { + t.Fatalf("setup: failed to create parent agent: status %d", resp.StatusCode) + } + + // Test 1: MCP tool - Attempt to spawn child in space-b with parent from space-a (should fail) + t.Run("MCP_CrossSpaceSpawnRejected", func(t *testing.T) { + result := callMCPTool(t, base, "spawn_agent", map[string]any{ + "space": "space-b", // different space + "name": "child-agent", + "parent": parentAgent, // parent is in space-a + }) + + // Should contain error about space enforcement + if !strings.Contains(result, "space enforcement") { + t.Errorf("Expected space enforcement error, got: %s", result) + } + if !strings.Contains(result, "space-a") || !strings.Contains(result, "space-b") { + t.Errorf("Error should mention both spaces, got: %s", result) + } + }) + + // Test 2: MCP tool - Spawn child in same space as parent (should succeed) + t.Run("MCP_SameSpaceSpawnAllowed", func(t *testing.T) { + result := callMCPTool(t, base, "spawn_agent", map[string]any{ + "space": spaceA, // same space as parent + "name": "child-agent-ok", + "parent": parentAgent, + }) + + // Should succeed (no error) + if strings.Contains(strings.ToLower(result), "error") { + t.Errorf("Expected success, got error: %s", result) + } + // Verify response contains success indicators + var response map[string]any + if err := json.Unmarshal([]byte(result), &response); err == nil { + if ok, _ := response["ok"].(bool); !ok { + t.Errorf("Expected ok:true in response, got: %s", result) + } + } + }) + + // Test 3: MCP tool - Non-existent parent should be rejected + t.Run("MCP_NonExistentParentRejected", func(t *testing.T) { + result := callMCPTool(t, base, "spawn_agent", map[string]any{ + "space": spaceA, + "name": "orphan-agent", + "parent": "non-existent-parent", + }) + + if !strings.Contains(result, "not found") { + t.Errorf("Expected 'not found' error for non-existent parent, got: %s", result) + } + }) + + // Test 4: HTTP endpoint - Attempt cross-space spawn via POST /spaces/{space}/agents + t.Run("HTTP_CrossSpaceSpawnRejected", func(t *testing.T) { + createReq := map[string]any{ + "name": "http-child", + "parent": parentAgent, // parent is in space-a + "backend": "tmux", + } + resp := postJSON(t, base+"/spaces/space-b/agents", createReq) + defer resp.Body.Close() + + if resp.StatusCode != http.StatusForbidden { + t.Errorf("Expected 403 Forbidden for cross-space spawn, got %d", resp.StatusCode) + } + + // Read error message + bodyBytes, _ := io.ReadAll(resp.Body) + bodyStr := string(bodyBytes) + if !strings.Contains(bodyStr, "space enforcement") { + t.Errorf("Expected space enforcement error message, got: %s", bodyStr) + } + }) + + // Test 5: HTTP endpoint - Same space spawn should succeed (or at least not fail on space check) + t.Run("HTTP_SameSpaceSpawnAllowed", func(t *testing.T) { + createReq := map[string]any{ + "name": "http-child-ok", + "parent": parentAgent, // parent is in space-a + "backend": "tmux", + } + resp := postJSON(t, base+"/spaces/"+spaceA+"/agents", createReq) + defer resp.Body.Close() + + // Should not be rejected for space enforcement (may fail for other reasons like no tmux) + if resp.StatusCode == http.StatusForbidden { + bodyBytes, _ := io.ReadAll(resp.Body) + if strings.Contains(string(bodyBytes), "space enforcement") { + t.Errorf("Same-space spawn should not be rejected by space enforcement") + } + } + }) + + // Test 6: No parent specified - should be allowed (backward compatibility) + t.Run("MCP_NoParentAllowed", func(t *testing.T) { + result := callMCPTool(t, base, "spawn_agent", map[string]any{ + "space": "space-c", // any space + "name": "independent-agent", + // no parent specified + }) + + // Should not fail on space enforcement (may fail for other reasons) + if strings.Contains(result, "space enforcement") { + t.Errorf("Spawn without parent should not trigger space enforcement, got: %s", result) + } + }) +}