diff --git a/docs/release-control/v6/internal/subsystems/agent-lifecycle.md b/docs/release-control/v6/internal/subsystems/agent-lifecycle.md index 268855e88..8b12c92a9 100644 --- a/docs/release-control/v6/internal/subsystems/agent-lifecycle.md +++ b/docs/release-control/v6/internal/subsystems/agent-lifecycle.md @@ -100,6 +100,13 @@ that binary, not separate customer-facing agent products. ## Shared Boundaries +Assistant transport scopes do not grant agent command authority. `ai:chat` +and `relay:mobile:access` remain conversation/read/session scopes; an +interactive infrastructure invocation must carry server-bound `ai:execute` +authority and still satisfy the separate agent execution/token-binding and +control-level gates. Unknown or absent request scope never falls back to agent +execution permission. + Commercial v5-to-v6 migration retry state may pass through `internal/api/` handlers that agent-lifecycle also references, but the ownership remains API/cloud-paid. Agent lifecycle surfaces may observe paid-migration posture for diff --git a/docs/release-control/v6/internal/subsystems/ai-runtime.md b/docs/release-control/v6/internal/subsystems/ai-runtime.md index f97f27255..5909e377e 100644 --- a/docs/release-control/v6/internal/subsystems/ai-runtime.md +++ b/docs/release-control/v6/internal/subsystems/ai-runtime.md @@ -30,6 +30,25 @@ contracts. The subsystem also owns AI orchestration, runtime cost control, shared AI transport surfaces, and browser-visible Assistant transcript actions that define what visible operator/model text can leave the transcript without exposing hidden provider/tool metadata. + +Interactive Assistant invocation authority is fail-closed and request-local. +The `read_only` presentation means no model-invokable durable Pulse-state or +infrastructure mutation is projected or executable. An `ai:chat` token grants +conversation, read, and session access only; it cannot write knowledge, change +finding state, approve, or execute. Infrastructure mutation additionally +requires explicit `ai:execute` authority and a control level that permits it. +Provider projection and registry execution consume the same invocation-aware +policy. Detection may write only through its explicit finding report/resolve +allowlist; investigation is domain-read-only and may emit at most one +side-effect-free typed proposal whose correlation identity is server-authored. +Unknown profiles, actions, aliases, origins, scopes, and mutation +classifications fail closed. + +Discovery `run` currently persists read-derived evidence in the discovery +cache. That cache write is not an H1 finding/knowledge/infrastructure mutation +exception and remains a separately governed residual under +`lane-followup:agent-native-continuous-discovery-findings` until discovery's +canonical persistence boundary is resolved. App-shell Patrol chrome may expose only content-free current-work pressure, such as an open-work count on the stable `Patrol` tab, from Patrol-owned findings and approval read models. It must not rename the destination, create a second diff --git a/docs/release-control/v6/internal/subsystems/api-contracts.md b/docs/release-control/v6/internal/subsystems/api-contracts.md index 18d44371c..e96223ecb 100644 --- a/docs/release-control/v6/internal/subsystems/api-contracts.md +++ b/docs/release-control/v6/internal/subsystems/api-contracts.md @@ -1657,7 +1657,7 @@ payload shape change when the portal presents compact client rows. persisted host continuity, so a server restart or v6 upgrade does not change the explanatory grouping model before the live inventory rebuild catches up. Genuinely new host identities must still return - the canonical monitored-system blocked payload. + the canonical monitored-system blocked payload. Independently, Assistant scope is server-bound: `ai:chat` authorizes conversation, reads, sessions, and knowledge reads only; durable knowledge changes and explicit governed approval/execution require `ai:execute`, relay-mobile chat does not inherit it, trusted proposal correlation identity is server-authored, and unknown/internal model fields fail closed before capture or store access. ## Extension Points diff --git a/docs/release-control/v6/internal/subsystems/security-privacy.md b/docs/release-control/v6/internal/subsystems/security-privacy.md index e5f19730e..537b88e65 100644 --- a/docs/release-control/v6/internal/subsystems/security-privacy.md +++ b/docs/release-control/v6/internal/subsystems/security-privacy.md @@ -72,6 +72,13 @@ controls as normal product settings. ## Shared Boundaries +API token scope copy must match runtime authority. `ai:chat` covers Assistant +conversation, model selection, sessions, and knowledge reads only. Knowledge +save/delete/import/clear and explicit governed action approval/execution +require `ai:execute`; relay-mobile chat access does not inherit it. Browser +sessions retain their authenticated product permissions, while token requests +with missing, unknown, or unrelated scopes fail closed. + 1. `frontend-modern/src/api/security.ts` shared with `api-contracts`: the security frontend client is both a security/privacy control surface and a canonical API payload contract boundary. 2. `frontend-modern/src/components/Settings/APIAccessPanel.tsx` shared with `frontend-primitives`: the API Access settings intro is both a security/privacy token-management trust surface and a canonical settings-shell presentation boundary. Its Docker / Podman token wording must come from diff --git a/docs/release-control/v6/internal/subsystems/storage-recovery.md b/docs/release-control/v6/internal/subsystems/storage-recovery.md index 53ace0775..7188d380d 100644 --- a/docs/release-control/v6/internal/subsystems/storage-recovery.md +++ b/docs/release-control/v6/internal/subsystems/storage-recovery.md @@ -56,6 +56,13 @@ state. ## Shared Boundaries +Assistant access to recovery points and storage evidence is read-side context. +`ai:chat`, relay-mobile chat, and the `read_only` control level cannot project +or execute storage/infrastructure mutation, finding lifecycle changes, or +knowledge persistence. Any future storage action must cross the explicit +`ai:execute` plus governed action-plan boundary; it must not be introduced as +a recovery-provider read helper or compatibility alias. + 1. `frontend-modern/src/features/proxmox/ProxmoxBackupServersTable.tsx` shared with `unified-resources`: Proxmox backup server table rows are both a storage/recovery backup-health surface and a unified-resource platform-table consumer boundary. 2. `frontend-modern/src/features/proxmox/ProxmoxRecoverableTable.tsx` shared with `unified-resources`: Proxmox recoverable workload table rows are both a storage/recovery coverage surface and a unified-resource platform-table consumer boundary. 3. `internal/api/setup_script_render.go` shared with `agent-lifecycle`, `api-contracts`: the generated Proxmox setup-script is a shared boundary across agent lifecycle (forced-command keys, install/uninstall edits), API contracts (rendered token shape and encoded rerun URL), and storage/recovery (backup visibility grants, Pulse-managed temperature SSH keys, and SMART disk-temperature collection). diff --git a/frontend-modern/src/components/Settings/__tests__/APITokenManager.test.tsx b/frontend-modern/src/components/Settings/__tests__/APITokenManager.test.tsx index 117282607..dab06325a 100644 --- a/frontend-modern/src/components/Settings/__tests__/APITokenManager.test.tsx +++ b/frontend-modern/src/components/Settings/__tests__/APITokenManager.test.tsx @@ -424,11 +424,11 @@ describe('APITokenManager', () => { expect(screen.getByText('AI')).toBeInTheDocument(); expect(screen.getByRole('button', { name: 'Pulse Assistant chat' })).toHaveAttribute( 'title', - 'Use interactive Pulse Assistant sessions, models, and knowledge endpoints.', + 'Use interactive Pulse Assistant sessions, models, and read knowledge.', ); expect(screen.getByRole('button', { name: 'Pulse Intelligence actions' })).toHaveAttribute( 'title', - 'Use governed Patrol actions for plans, approvals, policy-allowed fixes, verification, and history.', + 'Use governed Patrol actions for plans, approvals, policy-allowed fixes, verification, history, and knowledge changes.', ); expect(screen.getByText('Security')).toBeInTheDocument(); expect(screen.getByRole('button', { name: 'Audit logs (read)' })).toBeInTheDocument(); diff --git a/frontend-modern/src/constants/apiScopes.ts b/frontend-modern/src/constants/apiScopes.ts index 7ef5ad1b9..8dd1ae2f5 100644 --- a/frontend-modern/src/constants/apiScopes.ts +++ b/frontend-modern/src/constants/apiScopes.ts @@ -40,14 +40,14 @@ export const API_SCOPE_OPTIONS: APIScopeOption[] = [ { value: AI_CHAT_SCOPE, label: 'Pulse Assistant chat', - description: 'Use interactive Pulse Assistant sessions, models, and knowledge endpoints.', + description: 'Use interactive Pulse Assistant sessions, models, and read knowledge.', group: 'AI', }, { value: AI_EXECUTE_SCOPE, label: 'Pulse Intelligence actions', description: - 'Use governed Patrol actions for plans, approvals, policy-allowed fixes, verification, and history.', + 'Use governed Patrol actions for plans, approvals, policy-allowed fixes, verification, history, and knowledge changes.', group: 'AI', }, { diff --git a/internal/agentcapabilities/invocation.go b/internal/agentcapabilities/invocation.go index 8e213450a..de5494bec 100644 --- a/internal/agentcapabilities/invocation.go +++ b/internal/agentcapabilities/invocation.go @@ -274,6 +274,23 @@ func ClassifyRegisteredInvocation(toolName string, args map[string]interface{}) return descriptor.Classify(args) } +// ClassifyLegacyAssistantInvocation classifies the compatibility aliases used +// by /api/ai/execute. Keeping this mapping here lets its provider projection +// and runtime boundary consume the same closed mutation vocabulary as the +// registry-backed Assistant. Unknown aliases fail closed. +func ClassifyLegacyAssistantInvocation(toolName string) InvocationClass { + switch strings.TrimSpace(toolName) { + case LegacyAssistantFetchURLToolName: + return InvocationClass{Kind: ToolCallKindRead, Mutation: MutationNone} + case LegacyAssistantRunCommandToolName: + return InvocationClass{Kind: ToolCallKindWrite, Mutation: MutationInfrastructure} + case LegacyAssistantSetResourceURLToolName, ResolveFindingCapabilityName, DismissFindingCapabilityName: + return InvocationClass{Kind: ToolCallKindWrite, Mutation: MutationPulseState} + default: + return FailClosedInvocationClass() + } +} + // RedactedProposalParamsMarker replaces proposal parameter values in every // durable or user-visible exposure of a patrol_propose_action call. const RedactedProposalParamsMarker = "[redacted-proposal-params]" diff --git a/internal/agentcapabilities/invocation_test.go b/internal/agentcapabilities/invocation_test.go index a61b59067..7cc24a614 100644 --- a/internal/agentcapabilities/invocation_test.go +++ b/internal/agentcapabilities/invocation_test.go @@ -169,3 +169,19 @@ func TestRestrictedExposureVocabulary(t *testing.T) { t.Fatalf("projector must redact params, got %#v", projected["params"]) } } + +func TestLegacyAssistantAliasesUseClosedInvocationClassification(t *testing.T) { + tests := map[string]MutationTarget{ + LegacyAssistantFetchURLToolName: MutationNone, + LegacyAssistantRunCommandToolName: MutationInfrastructure, + LegacyAssistantSetResourceURLToolName: MutationPulseState, + ResolveFindingCapabilityName: MutationPulseState, + DismissFindingCapabilityName: MutationPulseState, + "unknown_alias": MutationInfrastructure, + } + for name, want := range tests { + if got := ClassifyLegacyAssistantInvocation(name); got.Mutation != want { + t.Fatalf("%s mutation = %q, want %q", name, got.Mutation, want) + } + } +} diff --git a/internal/ai/chat/agentic_additional_test.go b/internal/ai/chat/agentic_additional_test.go index 7b022286c..5beab15d4 100644 --- a/internal/ai/chat/agentic_additional_test.go +++ b/internal/ai/chat/agentic_additional_test.go @@ -373,7 +373,7 @@ func TestBuildAutomaticFallbackSummary_UsesToolNamesNotCallIDsOrRawOutput(t *tes func TestExecuteToolSafely_RecoversPanic(t *testing.T) { exec := tools.NewPulseToolExecutor(tools.ExecutorConfig{}) exec.RegisterTool(tools.RegisteredTool{ - Invocation: tools.StaticInvocation(agentcapabilities.ToolCallKindWrite, agentcapabilities.MutationPulseState), + Invocation: tools.StaticInvocation(agentcapabilities.ToolCallKindRead, agentcapabilities.MutationNone), Definition: tools.Tool{ Name: "panic_tool", InputSchema: tools.InputSchema{ @@ -666,7 +666,7 @@ func TestAgenticLoop_DoesNotAutoRecoverStructuredToolCall(t *testing.T) { recoveryCalls := 0 executor.RegisterTool(tools.RegisteredTool{ - Invocation: tools.StaticInvocation(agentcapabilities.ToolCallKindWrite, agentcapabilities.MutationPulseState), + Invocation: tools.StaticInvocation(agentcapabilities.ToolCallKindRead, agentcapabilities.MutationNone), Definition: tools.Tool{ Name: "fail_tool", InputSchema: tools.InputSchema{ @@ -686,7 +686,7 @@ func TestAgenticLoop_DoesNotAutoRecoverStructuredToolCall(t *testing.T) { }, }) executor.RegisterTool(tools.RegisteredTool{ - Invocation: tools.StaticInvocation(agentcapabilities.ToolCallKindWrite, agentcapabilities.MutationPulseState), + Invocation: tools.StaticInvocation(agentcapabilities.ToolCallKindRead, agentcapabilities.MutationNone), Definition: tools.Tool{ Name: "recovery_tool", InputSchema: tools.InputSchema{ @@ -770,7 +770,7 @@ func TestAgenticLoop_NormalizesProviderToolCallsThroughSharedProjection(t *testi executor := tools.NewPulseToolExecutor(tools.ExecutorConfig{}) var capturedArgs map[string]interface{} executor.RegisterTool(tools.RegisteredTool{ - Invocation: tools.StaticInvocation(agentcapabilities.ToolCallKindWrite, agentcapabilities.MutationPulseState), + Invocation: tools.StaticInvocation(agentcapabilities.ToolCallKindRead, agentcapabilities.MutationNone), Definition: tools.Tool{ Name: "test_tool", InputSchema: tools.InputSchema{ diff --git a/internal/ai/chat/execute_authority.go b/internal/ai/chat/execute_authority.go new file mode 100644 index 000000000..13429eaf4 --- /dev/null +++ b/internal/ai/chat/execute_authority.go @@ -0,0 +1,17 @@ +package chat + +import "context" + +type executeAuthorityContextKey struct{} + +// WithExecuteAuthority carries trusted transport authority into read-only +// projection endpoints. The value is server-authored and cannot originate in +// a model/provider payload. +func WithExecuteAuthority(ctx context.Context, allowed bool) context.Context { + return context.WithValue(ctx, executeAuthorityContextKey{}, allowed) +} + +func executeAuthorityFromContext(ctx context.Context) bool { + allowed, _ := ctx.Value(executeAuthorityContextKey{}).(bool) + return allowed +} diff --git a/internal/ai/chat/service.go b/internal/ai/chat/service.go index 82056b96b..d2b0baadc 100644 --- a/internal/ai/chat/service.go +++ b/internal/ai/chat/service.go @@ -844,6 +844,7 @@ func (s *Service) ExecuteStream(ctx context.Context, req ExecuteRequest, callbac effectiveExecutor.SetControlLevel(effectiveControlLevel) effectiveExecutor.SetAutonomousMode(autonomousMode) effectiveExecutor.ApplyExecutionProfile(tools.ProfileInteractiveAssistant) + effectiveExecutor.SetExecuteAuthority(req.HasExecuteAuthority) effectiveExecutor.SetResolvedContext(resolvedCtx) } filteredTools := s.toolsForExecutor(effectiveExecutor, autonomousMode) @@ -3008,6 +3009,7 @@ func (s *Service) ListAvailableTools(ctx context.Context, prompt string) []strin effectiveExecutor.SetControlLevel(effectiveControlLevel) effectiveExecutor.SetAutonomousMode(autonomousMode) effectiveExecutor.ApplyExecutionProfile(tools.ProfileInteractiveAssistant) + effectiveExecutor.SetExecuteAuthority(executeAuthorityFromContext(ctx)) availableTools := s.toolsForExecutor(effectiveExecutor, autonomousMode) names := make([]string, 0, len(availableTools)) for _, tool := range availableTools { @@ -3024,10 +3026,18 @@ func (s *Service) ListAvailableTools(ctx context.Context, prompt string) []strin // AssistantSurfaceToolContract returns the live native Assistant surface tool // summary for the current runtime mode. It keeps API/UI consumers on the // executor-owned projection instead of rebuilding tool buckets from names. -func (s *Service) AssistantSurfaceToolContract(_ context.Context) agentcapabilities.SurfaceToolContract { +func (s *Service) AssistantSurfaceToolContract(ctx context.Context) agentcapabilities.SurfaceToolContract { s.mu.RLock() executor := s.executor + effectiveControlLevel := s.effectiveControlLevelLocked() s.mu.RUnlock() + if executor == nil { + return agentcapabilities.SurfaceToolContract{} + } + executor = executor.Clone() + executor.SetControlLevel(effectiveControlLevel) + executor.ApplyExecutionProfile(tools.ProfileInteractiveAssistant) + executor.SetExecuteAuthority(executeAuthorityFromContext(ctx)) return executor.AssistantSurfaceToolContract(agentcapabilities.AssistantProviderToolOptions{ IncludeQuestionTool: !s.isAutonomousModeEnabled(), diff --git a/internal/ai/chat/service_execute_additional_test.go b/internal/ai/chat/service_execute_additional_test.go index 4e63bfcaf..90ea7d836 100644 --- a/internal/ai/chat/service_execute_additional_test.go +++ b/internal/ai/chat/service_execute_additional_test.go @@ -1755,10 +1755,11 @@ func TestService_ExecuteStream_RequestAutonomousOverrideClampsToolExecutor(t *te approvalRequiredMode := false approvalSeen := false err = svc.ExecuteStream(ctx, ExecuteRequest{ - SessionID: "sess-request-override", - Prompt: "check disk space", - AutonomousMode: &approvalRequiredMode, - MaxTurns: 2, + SessionID: "sess-request-override", + Prompt: "check disk space", + AutonomousMode: &approvalRequiredMode, + HasExecuteAuthority: true, + MaxTurns: 2, }, func(event StreamEvent) { if event.Type == "approval_needed" { approvalSeen = true diff --git a/internal/ai/chat/service_tooling_test.go b/internal/ai/chat/service_tooling_test.go index f7b5dae50..869c64126 100644 --- a/internal/ai/chat/service_tooling_test.go +++ b/internal/ai/chat/service_tooling_test.go @@ -263,6 +263,14 @@ func TestAssistantSurfaceToolContractUsesRuntimeAssistantProjection(t *testing.T if len(interactive.CapabilityNames) != 0 { t.Fatalf("Assistant surface must not expose MCP capability names: %#v", interactive.CapabilityNames) } + if stringSliceContains(interactive.ToolNames, agentcapabilities.PulseControlToolName) { + t.Fatalf("chat-only surface exposed infrastructure control: %#v", interactive.ToolNames) + } + + executeAuthorized := svc.AssistantSurfaceToolContract(WithExecuteAuthority(context.Background(), true)) + if !stringSliceContains(executeAuthorized.ToolNames, agentcapabilities.PulseControlToolName) { + t.Fatalf("execute-authorized controlled surface missing governed infrastructure control: %#v", executeAuthorized.ToolNames) + } svc.SetAutonomousMode(true) autonomous := svc.AssistantSurfaceToolContract(context.Background()) @@ -577,7 +585,7 @@ func testRunCommandToolDefinition() tools.Tool { func TestExecuteCommand_SuccessAndExitCode(t *testing.T) { exec := tools.NewPulseToolExecutor(tools.ExecutorConfig{}) exec.RegisterTool(tools.RegisteredTool{ - Invocation: tools.StaticInvocation(agentcapabilities.ToolCallKindWrite, agentcapabilities.MutationPulseState), + Invocation: tools.StaticInvocation(agentcapabilities.ToolCallKindRead, agentcapabilities.MutationNone), Definition: testRunCommandToolDefinition(), Handler: func(ctx context.Context, exec *tools.PulseToolExecutor, args map[string]interface{}) (tools.CallToolResult, error) { return tools.NewTextResult("Command failed (exit code 7): boom"), nil @@ -601,7 +609,7 @@ func TestExecuteCommand_SuccessAndExitCode(t *testing.T) { func TestExecuteCommand_ErrorAndApprovalPaths(t *testing.T) { exec := tools.NewPulseToolExecutor(tools.ExecutorConfig{}) exec.RegisterTool(tools.RegisteredTool{ - Invocation: tools.StaticInvocation(agentcapabilities.ToolCallKindWrite, agentcapabilities.MutationPulseState), + Invocation: tools.StaticInvocation(agentcapabilities.ToolCallKindRead, agentcapabilities.MutationNone), Definition: testRunCommandToolDefinition(), Handler: func(ctx context.Context, exec *tools.PulseToolExecutor, args map[string]interface{}) (tools.CallToolResult, error) { return tools.NewErrorResult(context.Canceled), nil @@ -619,7 +627,7 @@ func TestExecuteCommand_ErrorAndApprovalPaths(t *testing.T) { // fresh executor instead of swapping the handler in place. approvalExec := tools.NewPulseToolExecutor(tools.ExecutorConfig{}) approvalExec.RegisterTool(tools.RegisteredTool{ - Invocation: tools.StaticInvocation(agentcapabilities.ToolCallKindWrite, agentcapabilities.MutationPulseState), + Invocation: tools.StaticInvocation(agentcapabilities.ToolCallKindRead, agentcapabilities.MutationNone), Definition: testRunCommandToolDefinition(), Handler: func(ctx context.Context, exec *tools.PulseToolExecutor, args map[string]interface{}) (tools.CallToolResult, error) { return tools.NewTextResult("APPROVAL_REQUIRED: requires approval"), nil @@ -636,7 +644,7 @@ func TestExecuteCommand_ErrorAndApprovalPaths(t *testing.T) { func TestExecuteCommandUsesSharedResultTextProjection(t *testing.T) { exec := tools.NewPulseToolExecutor(tools.ExecutorConfig{}) exec.RegisterTool(tools.RegisteredTool{ - Invocation: tools.StaticInvocation(agentcapabilities.ToolCallKindWrite, agentcapabilities.MutationPulseState), + Invocation: tools.StaticInvocation(agentcapabilities.ToolCallKindRead, agentcapabilities.MutationNone), Definition: testRunCommandToolDefinition(), Handler: func(ctx context.Context, exec *tools.PulseToolExecutor, args map[string]interface{}) (tools.CallToolResult, error) { return tools.CallToolResult{ @@ -669,7 +677,7 @@ func TestExecuteAssistantTool_ErrorsAndSuccess(t *testing.T) { newService := func(handler func(ctx context.Context, exec *tools.PulseToolExecutor, args map[string]interface{}) (tools.CallToolResult, error)) *Service { exec := tools.NewPulseToolExecutor(tools.ExecutorConfig{}) exec.RegisterTool(tools.RegisteredTool{ - Invocation: tools.StaticInvocation(agentcapabilities.ToolCallKindWrite, agentcapabilities.MutationPulseState), + Invocation: tools.StaticInvocation(agentcapabilities.ToolCallKindRead, agentcapabilities.MutationNone), Definition: tools.Tool{Name: "test_tool"}, Handler: handler, }) @@ -702,7 +710,7 @@ func TestExecuteAssistantTool_ErrorsAndSuccess(t *testing.T) { func TestExecuteAssistantToolUsesSharedResultTextProjection(t *testing.T) { exec := tools.NewPulseToolExecutor(tools.ExecutorConfig{}) exec.RegisterTool(tools.RegisteredTool{ - Invocation: tools.StaticInvocation(agentcapabilities.ToolCallKindWrite, agentcapabilities.MutationPulseState), + Invocation: tools.StaticInvocation(agentcapabilities.ToolCallKindRead, agentcapabilities.MutationNone), Definition: tools.Tool{Name: "test_tool"}, Handler: func(ctx context.Context, exec *tools.PulseToolExecutor, args map[string]interface{}) (tools.CallToolResult, error) { return tools.CallToolResult{ diff --git a/internal/ai/chat/types.go b/internal/ai/chat/types.go index c02e88c2a..6df795fa4 100644 --- a/internal/ai/chat/types.go +++ b/internal/ai/chat/types.go @@ -304,6 +304,9 @@ type ExecuteRequest struct { HandoffMetadata HandoffMetadata `json:"handoff_metadata,omitempty"` // Browser-safe identity for restoring saved product handoffs. MaxTurns int `json:"max_turns,omitempty"` // Override max agentic turns (0 = use default) AutonomousMode *bool `json:"autonomous_mode,omitempty"` // Per-request autonomous override (nil = use service default) + // HasExecuteAuthority is injected by the authenticated server boundary. + // It is never accepted from provider/model JSON. + HasExecuteAuthority bool `json:"-"` // SuppressSessionEvent is set by HTTP streaming callers that have already // emitted the browser-visible session event before expensive handoff or diff --git a/internal/ai/patrol_findings_additional_test.go b/internal/ai/patrol_findings_additional_test.go index 04ec984fa..1827835da 100644 --- a/internal/ai/patrol_findings_additional_test.go +++ b/internal/ai/patrol_findings_additional_test.go @@ -400,7 +400,7 @@ func TestExecuteToolCallTreatsSharedApprovalAndPolicyMarkersAsInconclusive(t *te Name: "patrol_marker_probe", Description: "test-only marker probe", }, - Invocation: tools.StaticInvocation(agentcapabilities.ToolCallKindWrite, agentcapabilities.MutationPulseState), + Invocation: tools.StaticInvocation(agentcapabilities.ToolCallKindRead, agentcapabilities.MutationNone), Handler: func(context.Context, *tools.PulseToolExecutor, map[string]interface{}) (tools.CallToolResult, error) { return tools.NewTextResult(tt.resultText), nil }, diff --git a/internal/ai/service.go b/internal/ai/service.go index 88ed5ac22..575201286 100644 --- a/internal/ai/service.go +++ b/internal/ai/service.go @@ -30,6 +30,7 @@ import ( "github.com/rcourtman/pulse-go-rewrite/internal/ai/memory" "github.com/rcourtman/pulse-go-rewrite/internal/ai/modelboundary" "github.com/rcourtman/pulse-go-rewrite/internal/ai/providers" + aitools "github.com/rcourtman/pulse-go-rewrite/internal/ai/tools" "github.com/rcourtman/pulse-go-rewrite/internal/config" "github.com/rcourtman/pulse-go-rewrite/internal/models" "github.com/rcourtman/pulse-go-rewrite/internal/servicediscovery" @@ -2514,6 +2515,17 @@ Always execute the commands rather than telling the user how to do it.` anyNeedsApproval := false for _, tc := range resp.ToolCalls { toolInput := s.getToolInputDisplay(tc) + class := agentcapabilities.ClassifyLegacyAssistantInvocation(tc.Name) + if !s.legacyAssistantInvocationPolicy().Allows(tc.Name, class) { + result := agentcapabilities.ToolResultText(agentcapabilities.NewInvocationBlockedToolResult(tc.Name, class)) + execution := ToolExecution{Name: tc.Name, Input: toolInput, Output: result, Success: false} + toolExecutions = append(toolExecutions, execution) + callback(StreamEvent{Type: "tool_start", Data: ToolStartData{Name: tc.Name, Input: toolInput}}) + callback(StreamEvent{Type: "tool_end", Data: ToolEndData{Name: tc.Name, Input: toolInput, Output: result, Success: false}}) + projection := newLegacyServiceProviderToolResultContextProjection(tc.ID, result, true) + messages = append(messages, providers.Message{Role: "user", ToolResult: &projection.Model}) + continue + } // Check if this command needs approval needsApproval := false @@ -3075,9 +3087,26 @@ func (s *Service) hasAgentForTarget(req ExecuteRequest) bool { } // getTools returns the available tools for AI +func (s *Service) legacyAssistantInvocationPolicy() aitools.InvocationPolicy { + s.mu.RLock() + cfg := s.cfg + s.mu.RUnlock() + level := aitools.ControlLevelReadOnly + if cfg != nil { + level = aitools.ControlLevel(cfg.GetControlLevel()) + } + return aitools.InvocationPolicy{ + ControlLevel: level, + HasExecuteAuthority: true, + ExecuteAuthorityBound: true, + PulseStateAllowlist: map[string]bool{}, + Profile: aitools.ProfileInteractiveAssistant, + } +} + func (s *Service) getTools() []providers.Tool { - tools := append([]providers.Tool{}, agentcapabilities.LegacyAssistantUtilityProviderTools()...) - tools = append(tools, + candidates := append([]providers.Tool{}, agentcapabilities.LegacyAssistantUtilityProviderTools()...) + candidates = append(candidates, providers.Tool{ Name: agentcapabilities.ResolveFindingCapabilityName, Description: "Mark an AI patrol finding as resolved after successfully fixing the issue. Use the finding ID shown in your Patrol Finding Context section. Call this after verifying the fix worked - do NOT ask the user for the finding ID.", @@ -3092,14 +3121,21 @@ func (s *Service) getTools() []providers.Tool { // Add web search tool for Anthropic provider if s.provider != nil && s.provider.Name() == "anthropic" { - tools = append(tools, providers.Tool{ + candidates = append(candidates, providers.Tool{ Type: "web_search_20250305", Name: "web_search", MaxUses: 3, // Limit searches per request to control costs }) } - return tools + policy := s.legacyAssistantInvocationPolicy() + available := make([]providers.Tool, 0, len(candidates)) + for _, candidate := range candidates { + if candidate.Name == "web_search" || policy.Allows(candidate.Name, agentcapabilities.ClassifyLegacyAssistantInvocation(candidate.Name)) { + available = append(available, candidate) + } + } + return available } // executeTool executes a tool call and returns the result @@ -3108,6 +3144,11 @@ func (s *Service) executeTool(ctx context.Context, req ExecuteRequest, tc provid Name: tc.Name, Success: false, } + class := agentcapabilities.ClassifyLegacyAssistantInvocation(tc.Name) + if !s.legacyAssistantInvocationPolicy().Allows(tc.Name, class) { + execution.Output = agentcapabilities.ToolResultText(agentcapabilities.NewInvocationBlockedToolResult(tc.Name, class)) + return execution.Output, execution + } switch tc.Name { case agentcapabilities.LegacyAssistantRunCommandToolName: diff --git a/internal/ai/service_extended_test.go b/internal/ai/service_extended_test.go index e05702ab1..e16146a84 100644 --- a/internal/ai/service_extended_test.go +++ b/internal/ai/service_extended_test.go @@ -36,6 +36,31 @@ func TestServiceCommercialLogsUseSafeRemediationCopy(t *testing.T) { } } +func TestLegacyAssistantReadOnlyProjectionAndRuntimeAgree(t *testing.T) { + svc := &Service{cfg: &config.AIConfig{ControlLevel: config.ControlLevelReadOnly}} + projected := map[string]bool{} + for _, tool := range svc.getTools() { + projected[tool.Name] = true + } + if !projected[agentcapabilities.LegacyAssistantFetchURLToolName] { + t.Fatal("read-only legacy projection must retain fetch_url") + } + for _, name := range []string{ + agentcapabilities.LegacyAssistantRunCommandToolName, + agentcapabilities.LegacyAssistantSetResourceURLToolName, + agentcapabilities.ResolveFindingCapabilityName, + agentcapabilities.DismissFindingCapabilityName, + } { + if projected[name] { + t.Fatalf("read-only legacy projection exposed mutating alias %q", name) + } + result, execution := svc.executeTool(context.Background(), ExecuteRequest{}, providers.ToolCall{Name: name}) + if execution.Success || !strings.Contains(result, "Invocation blocked") { + t.Fatalf("read-only legacy runtime did not block %q: success=%v result=%q", name, execution.Success, result) + } + } +} + func TestService_GetToolInputDisplay(t *testing.T) { svc := NewService(nil, nil) @@ -110,7 +135,7 @@ func TestService_GetToolInputDisplay(t *testing.T) { func TestService_GetTools(t *testing.T) { svc := NewService(nil, nil) - svc.cfg = &config.AIConfig{Enabled: true} + svc.cfg = &config.AIConfig{Enabled: true, ControlLevel: config.ControlLevelControlled} tools := svc.getTools() if len(tools) == 0 { @@ -132,6 +157,7 @@ func TestService_GetTools(t *testing.T) { func TestService_GetToolsUsesSharedLegacyUtilitySchemas(t *testing.T) { svc := NewService(nil, nil) + svc.cfg = &config.AIConfig{ControlLevel: config.ControlLevelControlled} tools := svc.getTools() byName := map[string]providers.Tool{} for _, tool := range tools { @@ -139,6 +165,12 @@ func TestService_GetToolsUsesSharedLegacyUtilitySchemas(t *testing.T) { } for _, shared := range agentcapabilities.LegacyAssistantUtilityProviderTools() { + if shared.Name == agentcapabilities.LegacyAssistantSetResourceURLToolName { + if _, ok := byName[shared.Name]; ok { + t.Fatalf("legacy Assistant projection exposed Pulse-state mutation %s", shared.Name) + } + continue + } assistantTool, ok := byName[shared.Name] if !ok { t.Fatalf("legacy Assistant tools missing %s", shared.Name) @@ -163,8 +195,9 @@ func TestService_GetToolsUsesSharedLegacyUtilitySchemas(t *testing.T) { } } -func TestService_GetToolsUsesManifestFindingLifecycleSchemas(t *testing.T) { +func TestService_GetToolsExcludesFindingLifecycleMutations(t *testing.T) { svc := NewService(nil, nil) + svc.cfg = &config.AIConfig{ControlLevel: config.ControlLevelControlled} tools := svc.getTools() byName := map[string]providers.Tool{} for _, tool := range tools { @@ -175,25 +208,9 @@ func TestService_GetToolsUsesManifestFindingLifecycleSchemas(t *testing.T) { agentcapabilities.ResolveFindingCapabilityName, agentcapabilities.DismissFindingCapabilityName, } { - assistantTool, ok := byName[name] - if !ok { - t.Fatalf("legacy Assistant tools missing %s", name) + if _, ok := byName[name]; ok { + t.Fatalf("legacy Assistant projection exposed finding lifecycle mutation %s", name) } - capability, ok := agentcapabilities.FindCapability(agentcapabilities.CanonicalManifest().Capabilities, name) - if !ok { - t.Fatalf("canonical manifest missing %s", name) - } - manifestSchema := agentcapabilities.ProviderInputSchemaFromRaw(agentcapabilities.ToolInputSchema(capability)) - if !providerSchemaRequiredEqual(assistantTool.InputSchema, manifestSchema) { - t.Fatalf("%s Assistant provider schema required = %#v, want manifest required %#v", - name, assistantTool.InputSchema["required"], manifestSchema["required"]) - } - } - if providerSchemaRequiredContains(byName[agentcapabilities.ResolveFindingCapabilityName].InputSchema, agentcapabilities.ResolutionNoteArgumentName) { - t.Fatalf("%s must not require optional %s", agentcapabilities.ResolveFindingCapabilityName, agentcapabilities.ResolutionNoteArgumentName) - } - if providerSchemaRequiredContains(byName[agentcapabilities.DismissFindingCapabilityName].InputSchema, agentcapabilities.NoteArgumentName) { - t.Fatalf("%s must not require optional note", agentcapabilities.DismissFindingCapabilityName) } } @@ -804,6 +821,7 @@ func TestService_ExecuteTool_RunCommand(t *testing.T) { }, } svc := NewService(nil, mockAgentServer) + svc.cfg = &config.AIConfig{ControlLevel: config.ControlLevelControlled} // Mock policy mockPolicy := &mockPolicy{ @@ -841,7 +859,7 @@ func TestService_ExecuteTool_RunCommand(t *testing.T) { // 3. Approval required (non-autonomous) mockPolicy.decision = agentexec.PolicyRequireApproval - svc.cfg = &config.AIConfig{ControlLevel: config.ControlLevelReadOnly} + svc.cfg = &config.AIConfig{ControlLevel: config.ControlLevelControlled} result, exec = svc.executeTool(ctx, req, tc) if !exec.Success { t.Errorf("Expected success (not an error to need approval), got: %s", result) @@ -877,7 +895,7 @@ func TestService_ExecuteTool_RunCommand(t *testing.T) { } } -func TestService_ExecuteTool_SetResourceURL(t *testing.T) { +func TestService_ExecuteTool_SetResourceURLBlockedBeforeStore(t *testing.T) { svc := NewService(nil, nil) // Mock metadata provider @@ -897,11 +915,11 @@ func TestService_ExecuteTool_SetResourceURL(t *testing.T) { req := ExecuteRequest{} result, exec := svc.executeTool(ctx, req, tc) - if !exec.Success { - t.Errorf("Expected success, got failure: %s", result) + if exec.Success || !strings.Contains(result, "Invocation blocked") { + t.Fatalf("expected direct metadata mutation to be blocked, got success=%v result=%q", exec.Success, result) } - if mockMeta.lastGuestID != "instance-101" || mockMeta.lastGuestURL != "http://example.com:8080" { - t.Errorf("Metadata provider not called correctly: %+v", mockMeta) + if mockMeta.lastGuestID != "" || mockMeta.lastGuestURL != "" { + t.Fatalf("metadata provider was called despite policy block: %+v", mockMeta) } } @@ -1120,7 +1138,7 @@ func TestService_TestConnection_Extended(t *testing.T) { } -func TestService_ExecuteTool_ResolveFinding(t *testing.T) { +func TestService_ExecuteTool_ResolveFindingBlockedBeforeStore(t *testing.T) { tmpDir := t.TempDir() persistence := config.NewConfigPersistence(tmpDir) svc := NewService(persistence, nil) @@ -1157,65 +1175,15 @@ func TestService_ExecuteTool_ResolveFinding(t *testing.T) { } result, exec := svc.executeTool(ctx, req, tc) - if !exec.Success { - t.Errorf("Expected success, got failure: %s", result) + if exec.Success || !strings.Contains(result, "Invocation blocked") { + t.Fatalf("expected direct finding resolution to be blocked, got success=%v result=%q", exec.Success, result) } - if !strings.Contains(result, "Finding resolved!") { - t.Errorf("Expected 'Finding resolved!' in result, got: %s", result) - } - - // 2. Missing finding_id (should use from request context) - req2 := ExecuteRequest{FindingID: "test-finding-1"} - tc2 := providers.ToolCall{ - Name: agentcapabilities.ResolveFindingCapabilityName, - Input: map[string]interface{}{ - agentcapabilities.ResolutionNoteArgumentName: "Fixed it", - }, - } - _, exec2 := svc.executeTool(ctx, req2, tc2) - // Finding already resolved, so this will fail - if exec2.Success { - t.Error("Expected failure for already resolved finding") - } - - // 3. Missing resolution_note is allowed by the shared Pulse Intelligence manifest - findingWithoutNote := &Finding{ - ID: "new-finding", - Severity: FindingSeverityWarning, - ResourceID: "vm-101", - ResourceName: "test-vm-2", - Title: "Recovered CPU", - } - patrol.GetFindings().Add(findingWithoutNote) - tc3 := providers.ToolCall{ - Name: agentcapabilities.ResolveFindingCapabilityName, - Input: map[string]interface{}{ - agentcapabilities.FindingIDArgumentName: "new-finding", - }, - } - result3, exec3 := svc.executeTool(ctx, ExecuteRequest{}, tc3) - if !exec3.Success { - t.Errorf("Expected success without resolution_note, got failure: %s", result3) - } - if strings.Contains(result3, "resolution_note is required") { - t.Errorf("Unexpected resolution_note error: %s", result3) - } - - // 4. Missing both - tc4 := providers.ToolCall{ - Name: agentcapabilities.ResolveFindingCapabilityName, - Input: map[string]interface{}{}, - } - result4, exec4 := svc.executeTool(ctx, ExecuteRequest{}, tc4) - if exec4.Success { - t.Error("Expected failure for missing finding_id") - } - if !strings.Contains(result4, "finding_id is required") { - t.Errorf("Expected finding_id error, got: %s", result4) + if got := patrol.GetFindings().Get("test-finding-1"); got == nil || got.ResolvedAt != nil { + t.Fatalf("finding store changed despite policy block: %+v", got) } } -func TestService_ExecuteTool_SetResourceURL_EdgeCases(t *testing.T) { +func TestService_ExecuteTool_SetResourceURLFailsClosedBeforeValidation(t *testing.T) { svc := NewService(nil, nil) mockMeta := &mockMetadataProvider{} svc.metadataProvider = mockMeta @@ -1234,41 +1202,11 @@ func TestService_ExecuteTool_SetResourceURL_EdgeCases(t *testing.T) { if exec.Success { t.Error("Expected failure for missing resource_type") } - if !strings.Contains(result, "resource_type is required") { - t.Errorf("Expected resource_type error, got: %s", result) + if !strings.Contains(result, "Invocation blocked") { + t.Errorf("expected policy block before argument validation, got: %s", result) } - - // 2. Missing resource_id but present in request context - tc2 := providers.ToolCall{ - Name: "set_resource_url", - Input: map[string]interface{}{ - "resource_type": "vm", - "url": "http://example.com:8080", - }, - } - req := ExecuteRequest{TargetID: "vm-from-context"} - result2, exec2 := svc.executeTool(ctx, req, tc2) - if !exec2.Success { - t.Errorf("Expected success when resource_id from context, got: %s", result2) - } - if mockMeta.lastGuestID != "vm-from-context" { - t.Errorf("Expected resource_id from context, got: %s", mockMeta.lastGuestID) - } - - // 3. Missing resource_id completely - tc3 := providers.ToolCall{ - Name: "set_resource_url", - Input: map[string]interface{}{ - "resource_type": "vm", - "url": "http://example.com", - }, - } - result3, exec3 := svc.executeTool(ctx, ExecuteRequest{}, tc3) - if exec3.Success { - t.Error("Expected failure for missing resource_id") - } - if !strings.Contains(result3, "resource_id is required") { - t.Errorf("Expected resource_id error, got: %s", result3) + if mockMeta.lastGuestID != "" || mockMeta.lastGuestURL != "" { + t.Fatalf("metadata provider was called despite policy block: %+v", mockMeta) } } @@ -1286,8 +1224,8 @@ func TestService_ExecuteTool_UnknownTool(t *testing.T) { if exec.Success { t.Error("Expected failure for unknown tool") } - if !strings.Contains(result, "Unknown tool") { - t.Errorf("Expected 'Unknown tool' error, got: %s", result) + if !strings.Contains(result, "Invocation blocked") { + t.Errorf("expected unknown alias to fail closed, got: %s", result) } } @@ -1319,6 +1257,7 @@ func TestService_ExecuteTool_RunCommand_WithTargetHost(t *testing.T) { agents: []agentexec.ConnectedAgent{mockAgent}, } svc := NewService(nil, mockServer) + svc.cfg = &config.AIConfig{ControlLevel: config.ControlLevelControlled} svc.policy = &mockPolicy{decision: agentexec.PolicyAllow} canonicalStore := unifiedresources.NewMemoryStore() svc.resourceExportStore = canonicalStore @@ -3243,7 +3182,7 @@ func TestService_FindingContextCommandRouting(t *testing.T) { tmpDir := t.TempDir() persistence := config.NewConfigPersistence(tmpDir) svc := NewService(persistence, mockAgentServer) - svc.cfg = &config.AIConfig{Enabled: true} + svc.cfg = &config.AIConfig{Enabled: true, ControlLevel: config.ControlLevelControlled} svc.limits = executionLimits{ chatSlots: make(chan struct{}, 10), patrolSlots: make(chan struct{}, 10), diff --git a/internal/ai/service_test.go b/internal/ai/service_test.go index 22d6c8de6..36af7d3df 100644 --- a/internal/ai/service_test.go +++ b/internal/ai/service_test.go @@ -1174,6 +1174,7 @@ func TestService_RunCommand(t *testing.T) { func TestService_ExecuteTool(t *testing.T) { svc := NewService(nil, nil) + svc.cfg = &config.AIConfig{ControlLevel: config.ControlLevelControlled} ctx := context.Background() req := ExecuteRequest{Prompt: "test"} tc := providers.ToolCall{ diff --git a/internal/ai/tools/execution_profile.go b/internal/ai/tools/execution_profile.go index 2a77aea71..04de0e1ec 100644 --- a/internal/ai/tools/execution_profile.go +++ b/internal/ai/tools/execution_profile.go @@ -88,7 +88,10 @@ func (e *PulseToolExecutor) ApplyExecutionProfile(profile ExecutionProfile) { e.pulseStateAllowlist = map[string]bool{} default: e.denyInfrastructureMutations = false - e.pulseStateAllowlist = nil + // Interactive Assistant is conversation/read/session authority. + // Finding and knowledge lifecycle writes remain explicit product/API + // operations and are never model-invokable. + e.pulseStateAllowlist = map[string]bool{} } } diff --git a/internal/ai/tools/executor.go b/internal/ai/tools/executor.go index 8e721130f..dc9628849 100644 --- a/internal/ai/tools/executor.go +++ b/internal/ai/tools/executor.go @@ -559,6 +559,11 @@ type PulseToolExecutor struct { targetID string isAutonomous bool orgID string + // hasExecuteAuthority is request-local transport authority. Control + // settings describe operator policy; they never grant a chat-only token + // permission to execute infrastructure mutations. + hasExecuteAuthority bool + executeAuthorityBound bool // denyInfrastructureMutations is the request-local execution // restriction for non-interactive read-only workloads (e.g. Patrol // investigations): every infrastructure-mutating invocation is @@ -568,8 +573,7 @@ type PulseToolExecutor struct { // interactive questions and grants no mutation authority. denyInfrastructureMutations bool // pulseStateAllowlist restricts pulse-state mutations to the named - // tools when non-nil (empty map = deny all). Nil means the default - // posture: pulse-state mutations are not restricted by profile. + // tools. Nil and an empty map both deny all. pulseStateAllowlist map[string]bool // executionProfile is the core-owned request posture; see // execution_profile.go. @@ -734,6 +738,8 @@ func (e *PulseToolExecutor) Clone() *PulseToolExecutor { targetID: e.targetID, isAutonomous: e.isAutonomous, orgID: e.orgID, + hasExecuteAuthority: e.hasExecuteAuthority, + executeAuthorityBound: e.executeAuthorityBound, denyInfrastructureMutations: e.denyInfrastructureMutations, pulseStateAllowlist: clonePulseStateAllowlist(e.pulseStateAllowlist), executionProfile: e.executionProfile, @@ -816,6 +822,14 @@ func (e *PulseToolExecutor) SetControlLevel(level ControlLevel) { e.controlLevel = level } +// SetExecuteAuthority binds the current request's explicit ai:execute +// authority to this executor clone. It is deliberately not serialized and +// defaults false. +func (e *PulseToolExecutor) SetExecuteAuthority(allowed bool) { + e.hasExecuteAuthority = allowed + e.executeAuthorityBound = true +} + // SetProtectedGuests updates the protected guests list func (e *PulseToolExecutor) SetProtectedGuests(vmids []string) { e.protectedGuests = vmids @@ -1000,6 +1014,8 @@ func (e *PulseToolExecutor) GetResolvedContext() ResolvedContextProvider { func (e *PulseToolExecutor) invocationPolicy() InvocationPolicy { return InvocationPolicy{ ControlLevel: e.controlLevel, + HasExecuteAuthority: e.hasExecuteAuthority, + ExecuteAuthorityBound: e.executeAuthorityBound, DenyInfrastructureMutations: e.denyInfrastructureMutations, PulseStateAllowlist: clonePulseStateAllowlist(e.pulseStateAllowlist), Profile: e.executionProfile, diff --git a/internal/ai/tools/executor_setters_test.go b/internal/ai/tools/executor_setters_test.go index fe841063e..441dcbea1 100644 --- a/internal/ai/tools/executor_setters_test.go +++ b/internal/ai/tools/executor_setters_test.go @@ -233,7 +233,7 @@ func TestToolRegistry_ListToolsReturnsIndependentDefinitions(t *testing.T) { "mode": {Type: "string", Enum: enum}, } registry.RegisterExtension(RegisteredTool{ - Invocation: StaticInvocation(agentcapabilities.ToolCallKindWrite, agentcapabilities.MutationPulseState), + Invocation: StaticInvocation(agentcapabilities.ToolCallKindRead, agentcapabilities.MutationNone), Definition: Tool{ Name: "read", InputSchema: InputSchema{ @@ -309,7 +309,7 @@ func TestToolGovernanceUsesSharedAgentCapabilityShape(t *testing.T) { func TestToolRegistry_ExecuteControlToolReadOnlyUsesAssistantAndPatrolGuidance(t *testing.T) { registry := NewToolRegistry() registry.RegisterExtension(RegisteredTool{ - Invocation: StaticInvocation(agentcapabilities.ToolCallKindWrite, agentcapabilities.MutationPulseState), + Invocation: StaticInvocation(agentcapabilities.ToolCallKindWrite, agentcapabilities.MutationInfrastructure), Definition: Tool{Name: "control"}, RequireControl: true, Handler: func(ctx context.Context, e *PulseToolExecutor, args map[string]interface{}) (CallToolResult, error) { @@ -336,7 +336,7 @@ func TestToolRegistryExecuteNormalizesSharedToolCallParams(t *testing.T) { var gotArgs map[string]interface{} var gotArgsLenBeforeMutation int registry.RegisterExtension(RegisteredTool{ - Invocation: StaticInvocation(agentcapabilities.ToolCallKindWrite, agentcapabilities.MutationPulseState), + Invocation: StaticInvocation(agentcapabilities.ToolCallKindRead, agentcapabilities.MutationNone), Definition: Tool{Name: "test_tool", InputSchema: InputSchema{Properties: map[string]PropertySchema{ "resource_id": {Type: "string"}, "body": {Type: "object"}, @@ -372,7 +372,7 @@ func TestToolRegistryExecuteNormalizesSharedToolCallParams(t *testing.T) { assert.Contains(t, interpreted.Text, "invalid tools/call params: tool name is required") registry.RegisterExtension(RegisteredTool{ - Invocation: StaticInvocation(agentcapabilities.ToolCallKindWrite, agentcapabilities.MutationPulseState), + Invocation: StaticInvocation(agentcapabilities.ToolCallKindRead, agentcapabilities.MutationNone), Definition: Tool{Name: "empty_args"}, Handler: func(ctx context.Context, e *PulseToolExecutor, args map[string]interface{}) (CallToolResult, error) { gotArgs = args @@ -391,14 +391,14 @@ func TestToolRegistryExecuteNormalizesSharedToolResult(t *testing.T) { registry := NewToolRegistry() handlerContent := []Content{{Type: "text", Text: "ok"}} registry.RegisterExtension(RegisteredTool{ - Invocation: StaticInvocation(agentcapabilities.ToolCallKindWrite, agentcapabilities.MutationPulseState), + Invocation: StaticInvocation(agentcapabilities.ToolCallKindRead, agentcapabilities.MutationNone), Definition: Tool{Name: "read"}, Handler: func(ctx context.Context, e *PulseToolExecutor, args map[string]interface{}) (CallToolResult, error) { return CallToolResult{Content: handlerContent}, nil }, }) registry.RegisterExtension(RegisteredTool{ - Invocation: StaticInvocation(agentcapabilities.ToolCallKindWrite, agentcapabilities.MutationPulseState), + Invocation: StaticInvocation(agentcapabilities.ToolCallKindRead, agentcapabilities.MutationNone), Definition: Tool{Name: "empty"}, Handler: func(ctx context.Context, e *PulseToolExecutor, args map[string]interface{}) (CallToolResult, error) { return CallToolResult{}, nil diff --git a/internal/ai/tools/invocation_policy_test.go b/internal/ai/tools/invocation_policy_test.go index 64288dc3f..8a24c7502 100644 --- a/internal/ai/tools/invocation_policy_test.go +++ b/internal/ai/tools/invocation_policy_test.go @@ -3,6 +3,7 @@ package tools import ( "context" "testing" + "time" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" @@ -10,6 +11,34 @@ import ( "github.com/rcourtman/pulse-go-rewrite/internal/agentcapabilities" ) +type mutationCountingFindingsManager struct { + resolveCalls int + dismissCalls int +} + +func (m *mutationCountingFindingsManager) ResolveFinding(string, string) error { + m.resolveCalls++ + return nil +} + +func (m *mutationCountingFindingsManager) DismissFinding(string, string, string) error { + m.dismissCalls++ + return nil +} + +type mutationCountingKnowledgeStore struct { + saveCalls int +} + +func (m *mutationCountingKnowledgeStore) SaveNote(string, string, string) error { + m.saveCalls++ + return nil +} + +func (m *mutationCountingKnowledgeStore) GetKnowledge(string, string) []KnowledgeEntry { + return nil +} + // The invocation-policy regression proofs: the registry blocks // infrastructure-mutating invocations before any handler runs, the offered // provider schema and the runtime boundary consume the same descriptor, and @@ -131,6 +160,103 @@ func TestProjectionAndRuntimeEnforcementAgree(t *testing.T) { assert.NotContains(t, byName["pulse_kubernetes"], "scale") assert.NotContains(t, byName["pulse_kubernetes"], "exec") assert.Contains(t, byName["pulse_kubernetes"], "pods") + assert.NotContains(t, byName[agentcapabilities.PulseAlertsToolName], "resolve") + assert.NotContains(t, byName[agentcapabilities.PulseAlertsToolName], "dismiss") + assert.Contains(t, byName[agentcapabilities.PulseAlertsToolName], "list") + assert.NotContains(t, byName[agentcapabilities.PulseKnowledgeToolName], "remember") + assert.Contains(t, byName[agentcapabilities.PulseKnowledgeToolName], "recall") +} + +func TestReadOnlyProjectionExcludesPulseStateMutations(t *testing.T) { + exec := newInvocationPolicyExecutor(t) + exec.SetControlLevel(ControlLevelReadOnly) + exec.ApplyExecutionProfile(ProfileInteractiveAssistant) + + projected := map[string][]string{} + for _, tool := range exec.registry.ListTools(exec.invocationPolicy()) { + descriptor, ok := agentcapabilities.InvocationDescriptorFor(tool.Name) + if !ok || descriptor.Discriminator == "" { + continue + } + projected[tool.Name] = tool.InputSchema.Properties[descriptor.Discriminator].Enum + } + + assert.Equal(t, []string{"list", "findings", "resolved"}, projected[agentcapabilities.PulseAlertsToolName]) + assert.Equal(t, []string{"recall", "incidents", "correlate"}, projected[agentcapabilities.PulseKnowledgeToolName]) +} + +func TestReadOnlyRuntimeBlocksPulseStateBeforeHandlers(t *testing.T) { + findings := &mutationCountingFindingsManager{} + knowledge := &mutationCountingKnowledgeStore{} + exec := NewPulseToolExecutor(ExecutorConfig{ + FindingsManager: findings, + KnowledgeStoreProvider: knowledge, + }) + exec.SetControlLevel(ControlLevelReadOnly) + exec.ApplyExecutionProfile(ProfileInteractiveAssistant) + + for _, call := range []struct { + name string + args map[string]interface{} + }{ + {name: agentcapabilities.PulseAlertsToolName, args: map[string]interface{}{"action": "resolve", "finding_id": "f-1"}}, + {name: agentcapabilities.PulseAlertsToolName, args: map[string]interface{}{"action": "dismiss", "finding_id": "f-1", "reason": "not_an_issue"}}, + {name: agentcapabilities.PulseKnowledgeToolName, args: map[string]interface{}{"action": "remember", "resource_id": "vm:42", "note": "persist me"}}, + } { + text := executeBlockedText(t, exec, call.name, call.args) + assert.Contains(t, text, "Invocation blocked") + } + + assert.Zero(t, findings.resolveCalls) + assert.Zero(t, findings.dismissCalls) + assert.Zero(t, knowledge.saveCalls) + + // A read remains executable under the same policy. + result, err := exec.registry.Execute(context.Background(), exec, agentcapabilities.PulseKnowledgeToolName, map[string]interface{}{ + "action": "incidents", "resource_id": "vm:42", "timestamp": time.Now().UTC().Format(time.RFC3339), + }) + require.NoError(t, err) + require.NotEmpty(t, result.Content) + assert.NotContains(t, result.Content[0].Text, "Invocation blocked") +} + +func TestChatOnlyAuthorityBlocksInfrastructureBeforeHandler(t *testing.T) { + exec := newInvocationPolicyExecutor(t) + exec.SetControlLevel(ControlLevelControlled) + exec.ApplyExecutionProfile(ProfileInteractiveAssistant) + exec.SetExecuteAuthority(false) + + for _, tool := range exec.registry.ListTools(exec.invocationPolicy()) { + if tool.Name == agentcapabilities.PulseControlToolName { + t.Fatal("chat-only authority projected pulse_control") + } + } + result, err := exec.registry.Execute(context.Background(), exec, agentcapabilities.PulseControlToolName, map[string]interface{}{ + "action": "restart", + }) + require.NoError(t, err) + assert.Contains(t, agentcapabilities.ToolResultText(result), "Invocation blocked") + + // Execute authority may expose governed infrastructure operations, but it + // never re-enables direct Assistant finding/knowledge writes. + exec.SetExecuteAuthority(true) + projected := exec.registry.ListTools(exec.invocationPolicy()) + assert.True(t, hasToolNamed(projected, agentcapabilities.PulseControlToolName)) + for _, tool := range projected { + if tool.Name == agentcapabilities.PulseAlertsToolName { + assert.NotContains(t, tool.InputSchema.Properties["action"].Enum, "resolve") + assert.NotContains(t, tool.InputSchema.Properties["action"].Enum, "dismiss") + } + } +} + +func hasToolNamed(tools []Tool, name string) bool { + for _, tool := range tools { + if tool.Name == name { + return true + } + } + return false } func TestExecutorClonesKeepRequestPoliciesIsolated(t *testing.T) { diff --git a/internal/ai/tools/proposal_capture_test.go b/internal/ai/tools/proposal_capture_test.go index 0317bff8c..408a58610 100644 --- a/internal/ai/tools/proposal_capture_test.go +++ b/internal/ai/tools/proposal_capture_test.go @@ -292,3 +292,46 @@ func TestProposalValidationUsesCanonicalPlannerRules(t *testing.T) { t.Fatalf("outcome error = %v, want ErrProposalAttemptsFailed", err) } } + +func TestProposeActionSchemaExposesOnlyModelAuthoredFields(t *testing.T) { + exec := NewPulseToolExecutor(ExecutorConfig{}) + exec.ApplyExecutionProfile(ProfilePatrolInvestigation) + exec.SetProposalCapture(NewProposalCapture(ProposalIdentity{}, testProposalCatalog())) + + for _, tool := range exec.registry.ListTools(exec.invocationPolicy()) { + if tool.Name != agentcapabilities.PatrolProposeActionToolName { + continue + } + assert.ElementsMatch(t, + []string{"resource_id", "capability_name", "params", "reason"}, + mapKeys(tool.InputSchema.Properties), + ) + return + } + t.Fatal("patrol_propose_action missing from investigation projection") +} + +func TestProposeActionRejectsUnknownAndInternalFieldsBeforeCapture(t *testing.T) { + capture := NewProposalCapture(ProposalIdentity{FindingID: "trusted-f", InvestigationID: "trusted-i"}, testProposalCatalog()) + exec := newInvestigationExecutor(t, capture) + + for _, field := range []string{"finding_id", "investigation_id", agentcapabilities.ApprovalArgumentKey} { + args := proposeArgs() + args[field] = "model-authored" + result := executePropose(t, exec, "call-"+field, args) + assert.Contains(t, result.Content[0].Text, "invalid tools/call params") + } + + proposal, failed, err := capture.Outcome() + require.NoError(t, err) + assert.Nil(t, proposal) + assert.Zero(t, failed, "schema rejection must occur before the proposal handler records an attempt") +} + +func mapKeys[V any](values map[string]V) []string { + keys := make([]string, 0, len(values)) + for key := range values { + keys = append(keys, key) + } + return keys +} diff --git a/internal/ai/tools/registry.go b/internal/ai/tools/registry.go index f8c45e935..975256e4b 100644 --- a/internal/ai/tools/registry.go +++ b/internal/ai/tools/registry.go @@ -95,11 +95,15 @@ func NewToolRegistry() *ToolRegistry { // projection and runtime execution so the offered schema and the // enforcement boundary can never disagree. type InvocationPolicy struct { - ControlLevel ControlLevel + ControlLevel ControlLevel + // Execute authority is bound at authenticated transport boundaries. + // Unbound policies are reserved for trusted internal/test callers; + // external request paths must always bind an explicit true/false value. + HasExecuteAuthority bool + ExecuteAuthorityBound bool DenyInfrastructureMutations bool // PulseStateAllowlist restricts pulse-state mutations to the named - // tools when non-nil (an empty map denies all pulse-state - // mutations). Nil leaves pulse-state mutations unrestricted. An + // tools. Nil and an empty map both deny all pulse-state mutations. An // allowlist rather than a boolean: Patrol detection must record and // resolve findings without also being able to dismiss alerts or // write knowledge. @@ -129,6 +133,9 @@ func clonePulseStateAllowlist(allowlist map[string]bool) map[string]bool { // independent of registration validation, so a class that somehow // bypasses Validate still cannot execute. func (p InvocationPolicy) Allows(toolName string, class agentcapabilities.InvocationClass) bool { + if !p.Profile.Valid() || !class.Valid() { + return false + } // patrol_propose_action is investigation-profile-only: a fabricated // call under any other posture is rejected here, before the handler, // and the same check keeps it out of every other profile's projected @@ -140,12 +147,9 @@ func (p InvocationPolicy) Allows(toolName string, class agentcapabilities.Invoca case agentcapabilities.MutationNone: return true case agentcapabilities.MutationPulseState: - if p.PulseStateAllowlist == nil { - return true - } return p.PulseStateAllowlist[toolName] case agentcapabilities.MutationInfrastructure: - if p.DenyInfrastructureMutations { + if p.Profile != ProfileInteractiveAssistant || (p.ExecuteAuthorityBound && !p.HasExecuteAuthority) || p.DenyInfrastructureMutations { return false } return agentcapabilities.ControlLevelAllowsControlTools(p.ControlLevel) @@ -410,7 +414,9 @@ func (r *ToolRegistry) Execute(ctx context.Context, e *PulseToolExecutor, name s } policy := e.invocationPolicy() if !policy.Allows(name, class) { - if class.Mutation == agentcapabilities.MutationInfrastructure && !policy.DenyInfrastructureMutations { + if class.Mutation == agentcapabilities.MutationInfrastructure && + !policy.DenyInfrastructureMutations && + !agentcapabilities.ControlLevelAllowsControlTools(policy.ControlLevel) { // Refused purely by control level: keep the actionable // operator guidance for enabling control tools. return agentcapabilities.NewControlToolsDisabledToolResult(), nil @@ -427,6 +433,13 @@ func (r *ToolRegistry) Execute(ctx context.Context, e *PulseToolExecutor, name s if err := agentcapabilities.ValidateDeclaredToolArguments(tool.Definition.InputSchema, args); err != nil { return agentcapabilities.NewInvalidToolCallParamsResult(err), nil } + if name == agentcapabilities.PatrolProposeActionToolName { + for key := range args { + if agentcapabilities.IsInternalToolArgument(key) { + return agentcapabilities.NewInvalidToolCallParamsResult(fmt.Errorf("argument %q is server-internal and cannot be model-authored", key)), nil + } + } + } result, err := tool.Handler(ctx, e, args) return result.NormalizeCollections(), err diff --git a/internal/ai/tools/tools_patrol_test.go b/internal/ai/tools/tools_patrol_test.go index a762cf300..646f3ca5a 100644 --- a/internal/ai/tools/tools_patrol_test.go +++ b/internal/ai/tools/tools_patrol_test.go @@ -684,7 +684,11 @@ func TestPatrolToolsRegistered(t *testing.T) { exec := NewPulseToolExecutor(ExecutorConfig{}) // Patrol tools should be registered but availability depends on patrolFindingCreator - tools := exec.registry.ListTools(InvocationPolicy{ControlLevel: ControlLevelReadOnly}) + tools := exec.registry.ListTools(InvocationPolicy{ + ControlLevel: ControlLevelReadOnly, + PulseStateAllowlist: patrolDetectionPulseStateAllowlist(), + Profile: ProfilePatrolDetection, + }) found := map[string]bool{} var resolveTool Tool for _, tool := range tools { diff --git a/internal/api/ai_handler.go b/internal/api/ai_handler.go index 2c762dfc6..40793547c 100644 --- a/internal/api/ai_handler.go +++ b/internal/api/ai_handler.go @@ -29,6 +29,7 @@ import ( recoverymanager "github.com/rcourtman/pulse-go-rewrite/internal/recovery/manager" "github.com/rcourtman/pulse-go-rewrite/internal/unifiedresources" "github.com/rcourtman/pulse-go-rewrite/pkg/aicontracts" + pulsauth "github.com/rcourtman/pulse-go-rewrite/pkg/auth" "github.com/rcourtman/pulse-go-rewrite/pkg/reporting" "github.com/rs/zerolog/log" ) @@ -86,6 +87,11 @@ type AIService interface { GetBaseURL() string } +func assistantExecuteAuthority(ctx context.Context) bool { + token := pulsauth.GetAPIToken(ctx) + return token == nil || token.HasScope(config.ScopeAIExecute) +} + type aiServiceRuntimeConfigReader interface { GetConfig() *config.AIConfig } @@ -2417,7 +2423,7 @@ func (h *AIHandler) HandleChat(w http.ResponseWriter, r *http.Request) { // Auth already handled by RequireAuth wrapper - no need to check again - ctx := r.Context() + ctx := chat.WithExecuteAuthority(r.Context(), assistantExecuteAuthority(r.Context())) if !h.IsRunning(ctx) { http.Error(w, "Pulse Assistant is not running", http.StatusServiceUnavailable) return @@ -2696,6 +2702,7 @@ func (h *AIHandler) HandleChat(w http.ResponseWriter, r *http.Request) { HandoffActions: handoffActions, HandoffMetadata: handoffMetadata, AutonomousMode: chatAutonomousModeForFindingHandoff(nil, findingID, handoffContext, handoffResources, handoffActions, handoffMetadata), + HasExecuteAuthority: assistantExecuteAuthority(ctx), SuppressSessionEvent: true, }, func(event chat.StreamEvent) { if event.Type == "done" { @@ -2985,7 +2992,7 @@ func (h *AIHandler) HandleAssistantSurfaceTools(w http.ResponseWriter, r *http.R return } - ctx := r.Context() + ctx := chat.WithExecuteAuthority(r.Context(), assistantExecuteAuthority(r.Context())) if !h.IsRunning(ctx) { http.Error(w, "Pulse Assistant is not running", http.StatusServiceUnavailable) return diff --git a/internal/api/ai_handler_test.go b/internal/api/ai_handler_test.go index f4deff6e1..992d35759 100644 --- a/internal/api/ai_handler_test.go +++ b/internal/api/ai_handler_test.go @@ -23,6 +23,7 @@ import ( "github.com/rcourtman/pulse-go-rewrite/internal/monitoring" "github.com/rcourtman/pulse-go-rewrite/internal/unifiedresources" "github.com/rcourtman/pulse-go-rewrite/pkg/aicontracts" + pulsauth "github.com/rcourtman/pulse-go-rewrite/pkg/auth" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/mock" "github.com/stretchr/testify/require" @@ -32,6 +33,29 @@ type MockAIService struct { mock.Mock } +type assistantScopeToken map[string]bool + +func (t assistantScopeToken) HasScope(scope string) bool { return t[scope] } + +func TestAssistantExecuteAuthorityTokenMatrix(t *testing.T) { + tests := []struct { + name string + ctx context.Context + want bool + }{ + {name: "browser session", ctx: context.Background(), want: true}, + {name: "chat token", ctx: pulsauth.WithAPIToken(context.Background(), assistantScopeToken{config.ScopeAIChat: true}), want: false}, + {name: "relay token", ctx: pulsauth.WithAPIToken(context.Background(), assistantScopeToken{config.ScopeRelayMobileAccess: true}), want: false}, + {name: "execute token", ctx: pulsauth.WithAPIToken(context.Background(), assistantScopeToken{config.ScopeAIChat: true, config.ScopeAIExecute: true}), want: true}, + {name: "unknown scope", ctx: pulsauth.WithAPIToken(context.Background(), assistantScopeToken{"ai:unknown": true}), want: false}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + assert.Equal(t, tt.want, assistantExecuteAuthority(tt.ctx)) + }) + } +} + type syncingMockAIService struct { MockAIService cfg *config.AIConfig diff --git a/internal/api/ai_handlers_knowledge_test.go b/internal/api/ai_handlers_knowledge_test.go index 3deeb30ac..33590e102 100644 --- a/internal/api/ai_handlers_knowledge_test.go +++ b/internal/api/ai_handlers_knowledge_test.go @@ -1329,14 +1329,14 @@ func TestKnowledgeEndpoints_RequireAuth(t *testing.T) { } } -func TestKnowledgeEndpoints_RequireAIChatScope(t *testing.T) { +func TestKnowledgeEndpoints_RequireReadOrExecuteScope(t *testing.T) { t.Setenv("ALLOW_ADMIN_BYPASS", "") t.Setenv("PULSE_DEV", "") t.Setenv("NODE_ENV", "") resetAdminBypassState() t.Cleanup(resetAdminBypassState) - // Create a token with monitoring:read scope (NOT ai:chat) + // Create a token with monitoring:read scope (neither ai:chat nor ai:execute). rawToken := "knowledge-scope-test-123.12345678" record := newTokenRecord(t, rawToken, []string{config.ScopeMonitoringRead}, nil) cfg := newTestConfigWithTokens(t, record) @@ -1347,13 +1347,14 @@ func TestKnowledgeEndpoints_RequireAIChatScope(t *testing.T) { method string path string body string + scope string }{ - {http.MethodGet, "/api/ai/knowledge?guest_id=vm-1", ""}, - {http.MethodPost, "/api/ai/knowledge/save", `{}`}, - {http.MethodPost, "/api/ai/knowledge/delete", `{}`}, - {http.MethodGet, "/api/ai/knowledge/export?guest_id=vm-1", ""}, - {http.MethodPost, "/api/ai/knowledge/import", `{}`}, - {http.MethodPost, "/api/ai/knowledge/clear", `{}`}, + {http.MethodGet, "/api/ai/knowledge?guest_id=vm-1", "", config.ScopeAIChat}, + {http.MethodPost, "/api/ai/knowledge/save", `{}`, config.ScopeAIExecute}, + {http.MethodPost, "/api/ai/knowledge/delete", `{}`, config.ScopeAIExecute}, + {http.MethodGet, "/api/ai/knowledge/export?guest_id=vm-1", "", config.ScopeAIChat}, + {http.MethodPost, "/api/ai/knowledge/import", `{}`, config.ScopeAIExecute}, + {http.MethodPost, "/api/ai/knowledge/clear", `{}`, config.ScopeAIExecute}, } for _, ep := range endpoints { @@ -1371,9 +1372,35 @@ func TestKnowledgeEndpoints_RequireAIChatScope(t *testing.T) { if rec.Code != http.StatusForbidden { t.Fatalf("expected %d for wrong scope, got %d: %s", http.StatusForbidden, rec.Code, rec.Body.String()) } - if !strings.Contains(rec.Body.String(), config.ScopeAIChat) { - t.Fatalf("expected error to mention %q scope, got %q", config.ScopeAIChat, rec.Body.String()) + if !strings.Contains(rec.Body.String(), ep.scope) { + t.Fatalf("expected error to mention %q scope, got %q", ep.scope, rec.Body.String()) } }) } } + +func TestAIChatScopeCannotWriteKnowledge(t *testing.T) { + rawToken := "knowledge-chat-only-123.12345678" + record := newTokenRecord(t, rawToken, []string{config.ScopeAIChat}, nil) + cfg := newTestConfigWithTokens(t, record) + router := NewRouter(cfg, nil, nil, nil, nil, "1.0.0") + t.Cleanup(router.shutdownBackgroundWorkers) + + for _, endpoint := range []string{ + "/api/ai/knowledge/save", + "/api/ai/knowledge/delete", + "/api/ai/knowledge/import", + "/api/ai/knowledge/clear", + } { + req := httptest.NewRequest(http.MethodPost, endpoint, strings.NewReader(`{}`)) + req.Header.Set("X-API-Token", rawToken) + rec := httptest.NewRecorder() + router.Handler().ServeHTTP(rec, req) + if rec.Code != http.StatusForbidden { + t.Fatalf("expected ai:chat-only token to be forbidden on %s, got %d: %s", endpoint, rec.Code, rec.Body.String()) + } + if !strings.Contains(rec.Body.String(), config.ScopeAIExecute) { + t.Fatalf("expected %s to require %q, got %q", endpoint, config.ScopeAIExecute, rec.Body.String()) + } + } +} diff --git a/internal/api/contract_test.go b/internal/api/contract_test.go index 3e84e3df3..26b9e2ef3 100644 --- a/internal/api/contract_test.go +++ b/internal/api/contract_test.go @@ -15985,7 +15985,8 @@ func TestContract_PulseIntelligenceSurfaceToolProjectionKeepsAssistantAndMCPDist } chatService := string(chatServiceSource) for _, required := range []string{ - `func (s *Service) AssistantSurfaceToolContract(_ context.Context) agentcapabilities.SurfaceToolContract`, + `func (s *Service) AssistantSurfaceToolContract(ctx context.Context) agentcapabilities.SurfaceToolContract`, + `executor.SetExecuteAuthority(executeAuthorityFromContext(ctx))`, `executor.AssistantSurfaceToolContract(agentcapabilities.AssistantProviderToolOptions{`, `IncludeQuestionTool: !s.isAutonomousModeEnabled(),`, } { @@ -18585,7 +18586,7 @@ func TestContract_PulseMCPAdapterProjectsAgentCapabilitiesManifest(t *testing.T) // consumes the same descriptor the projection filters with. `class = tool.Invocation.Classify(args)`, `if !policy.Allows(name, class) {`, - `if class.Mutation == agentcapabilities.MutationInfrastructure && !policy.DenyInfrastructureMutations {`, + `!agentcapabilities.ControlLevelAllowsControlTools(policy.ControlLevel)`, `agentcapabilities.NewInvocationBlockedToolResult(name, class)`, `descriptor.Validate(name, discriminatorEnum(tool.Definition, descriptor.Discriminator))`, `func projectToolForPolicy(tool RegisteredTool, policy InvocationPolicy) (RegisteredTool, bool)`, diff --git a/internal/api/router_routes_ai_relay.go b/internal/api/router_routes_ai_relay.go index ee2934239..2371a1cc6 100644 --- a/internal/api/router_routes_ai_relay.go +++ b/internal/api/router_routes_ai_relay.go @@ -51,13 +51,13 @@ func (r *Router) registerAIRelayRoutesGroup() { r.mux.HandleFunc("/api/ai/investigate-alert", RequireAdmin(r.config, RequireScope(config.ScopeAIExecute, r.aiAlertAnalysisEndpoints.HandleInvestigateAlert))) r.mux.HandleFunc("/api/ai/run-command", RequireAdmin(r.config, RequireScope(config.ScopeAIExecute, r.aiSettingsHandler.HandleRunCommand))) - // SECURITY: AI Knowledge endpoints require ai:chat scope to prevent arbitrary guest data access + // Knowledge reads belong to ai:chat; durable mutations require ai:execute. r.mux.HandleFunc("/api/ai/knowledge", RequireAuth(r.config, RequireScope(config.ScopeAIChat, r.aiSettingsHandler.HandleGetGuestKnowledge))) - r.mux.HandleFunc("/api/ai/knowledge/save", RequireAuth(r.config, RequireScope(config.ScopeAIChat, r.aiSettingsHandler.HandleSaveGuestNote))) - r.mux.HandleFunc("/api/ai/knowledge/delete", RequireAuth(r.config, RequireScope(config.ScopeAIChat, r.aiSettingsHandler.HandleDeleteGuestNote))) + r.mux.HandleFunc("/api/ai/knowledge/save", RequireAuth(r.config, RequireScope(config.ScopeAIExecute, r.aiSettingsHandler.HandleSaveGuestNote))) + r.mux.HandleFunc("/api/ai/knowledge/delete", RequireAuth(r.config, RequireScope(config.ScopeAIExecute, r.aiSettingsHandler.HandleDeleteGuestNote))) r.mux.HandleFunc("/api/ai/knowledge/export", RequireAuth(r.config, RequireScope(config.ScopeAIChat, r.aiSettingsHandler.HandleExportGuestKnowledge))) - r.mux.HandleFunc("/api/ai/knowledge/import", RequireAuth(r.config, RequireScope(config.ScopeAIChat, r.aiSettingsHandler.HandleImportGuestKnowledge))) - r.mux.HandleFunc("/api/ai/knowledge/clear", RequireAuth(r.config, RequireScope(config.ScopeAIChat, r.aiSettingsHandler.HandleClearGuestKnowledge))) + r.mux.HandleFunc("/api/ai/knowledge/import", RequireAuth(r.config, RequireScope(config.ScopeAIExecute, r.aiSettingsHandler.HandleImportGuestKnowledge))) + r.mux.HandleFunc("/api/ai/knowledge/clear", RequireAuth(r.config, RequireScope(config.ScopeAIExecute, r.aiSettingsHandler.HandleClearGuestKnowledge))) // SECURITY: Debug context leaks system prompt and infra details - require settings:read scope r.mux.HandleFunc("/api/ai/debug/context", RequireAdmin(r.config, RequireScope(config.ScopeSettingsRead, r.aiSettingsHandler.HandleDebugContext))) // SECURITY: Connected agents list could reveal fleet topology - require ai:execute scope diff --git a/internal/api/security_regression_test.go b/internal/api/security_regression_test.go index 1aee9b161..5e16e865a 100644 --- a/internal/api/security_regression_test.go +++ b/internal/api/security_regression_test.go @@ -3280,11 +3280,7 @@ func TestAIChatEndpointsRequireAIChatScope(t *testing.T) { {method: http.MethodGet, path: "/api/ai/sessions/session-1", body: ""}, {method: http.MethodGet, path: "/api/ai/question/q-1", body: ""}, {method: http.MethodGet, path: "/api/ai/knowledge", body: ""}, - {method: http.MethodPost, path: "/api/ai/knowledge/save", body: `{}`}, - {method: http.MethodPost, path: "/api/ai/knowledge/delete", body: `{}`}, {method: http.MethodGet, path: "/api/ai/knowledge/export", body: ""}, - {method: http.MethodPost, path: "/api/ai/knowledge/import", body: `{}`}, - {method: http.MethodPost, path: "/api/ai/knowledge/clear", body: `{}`}, } for _, tc := range paths { @@ -3301,6 +3297,28 @@ func TestAIChatEndpointsRequireAIChatScope(t *testing.T) { } } +func TestAIKnowledgeMutationEndpointsRequireAIExecuteScope(t *testing.T) { + rawToken := "ai-chat-only-token-123.12345678" + record := newTokenRecord(t, rawToken, []string{config.ScopeAIChat}, nil) + cfg := newTestConfigWithTokens(t, record) + router := NewRouter(cfg, nil, nil, nil, nil, "1.0.0") + + for _, path := range []string{ + "/api/ai/knowledge/save", + "/api/ai/knowledge/delete", + "/api/ai/knowledge/import", + "/api/ai/knowledge/clear", + } { + req := httptest.NewRequest(http.MethodPost, path, strings.NewReader(`{}`)) + req.Header.Set("X-API-Token", rawToken) + rec := httptest.NewRecorder() + router.Handler().ServeHTTP(rec, req) + if rec.Code != http.StatusForbidden || !strings.Contains(rec.Body.String(), config.ScopeAIExecute) { + t.Fatalf("expected %s to require %q, got %d: %s", path, config.ScopeAIExecute, rec.Code, rec.Body.String()) + } + } +} + func TestAuditEndpointsRequireLicenseFeature(t *testing.T) { rawToken := "audit-license-token-123.12345678" record := newTokenRecord(t, rawToken, []string{config.ScopeSettingsRead, config.ScopeAuditRead}, nil) diff --git a/tests/integration/tests/80-assistant-read-only-scope-integrity.spec.ts b/tests/integration/tests/80-assistant-read-only-scope-integrity.spec.ts new file mode 100644 index 000000000..82cf55b38 --- /dev/null +++ b/tests/integration/tests/80-assistant-read-only-scope-integrity.spec.ts @@ -0,0 +1,102 @@ +import fs from "node:fs"; +import path from "node:path"; +import { fileURLToPath } from "node:url"; +import { test as base, expect, type Route } from "@playwright/test"; +import { createAuthenticatedStorageState } from "./helpers"; + +const __dirname = path.dirname(fileURLToPath(import.meta.url)); +type WorkerFixtures = { authStorageStatePath: string }; + +const test = base.extend<{}, WorkerFixtures>({ + storageState: async ({ authStorageStatePath }, use) => + use(authStorageStatePath), + authStorageStatePath: [ + async ({ browser }, use, workerInfo) => { + const storageStatePath = path.resolve( + __dirname, + "..", + "..", + "tmp", + "playwright-auth", + `assistant-read-only-${workerInfo.project.name}.json`, + ); + fs.mkdirSync(path.dirname(storageStatePath), { recursive: true }); + await createAuthenticatedStorageState(browser, storageStatePath); + try { + await use(storageStatePath); + } finally { + fs.rmSync(storageStatePath, { force: true }); + } + }, + { scope: "worker" }, + ], +}); + +test("Assistant makes the read-only promise literal in the current browser build", async ({ + page, +}) => { + const fulfillSettings = async (route: Route) => { + await route.fulfill({ + status: 200, + contentType: "application/json", + body: JSON.stringify({ + model: "openrouter:deepseek/deepseek-chat", + control_level: "read_only", + }), + }); + }; + await page.route("**/api/ai/settings", fulfillSettings); + await page.route("**/api/settings/ai", fulfillSettings); + await page.route("**/api/ai/status", (route) => + route.fulfill({ + status: 200, + contentType: "application/json", + body: '{"running":true}', + }), + ); + await page.route("**/api/ai/models", (route) => + route.fulfill({ + status: 200, + contentType: "application/json", + body: JSON.stringify({ + models: [ + { id: "openrouter:deepseek/deepseek-chat", name: "DeepSeek Chat" }, + ], + }), + }), + ); + await page.route("**/api/ai/sessions", (route) => + route.fulfill({ status: 200, contentType: "application/json", body: "[]" }), + ); + + await page.goto("/truenas/overview", { + waitUntil: "domcontentloaded", + }); + await page + .getByRole("button", { name: "Ask Pulse Assistant about TrueNAS" }) + .click(); + + const mode = page.getByRole("button", { + name: "Assistant chat action mode: Read-only", + }); + await expect(mode).toBeVisible(); + await mode.click(); + await expect( + page.getByRole("menuitemradio", { name: /Read-only/ }), + ).toHaveAttribute("aria-checked", "true"); + await expect(page.getByText("Observes only")).toBeVisible(); + + await page.getByRole("button", { name: "Collapse Pulse Assistant" }).click(); + await page.goto("/settings/security/api", { waitUntil: "domcontentloaded" }); + await page.getByRole("button", { name: "New token" }).click(); + await page.getByText("Custom scopes", { exact: true }).click(); + await expect( + page.getByRole("button", { name: "Pulse Assistant chat" }), + ).toHaveAttribute( + "title", + "Use interactive Pulse Assistant sessions, models, and read knowledge.", + ); + await expect( + page.getByRole("button", { name: "Pulse Intelligence actions" }), + ).toHaveAttribute("title", /verification, history, and knowledge changes/); +});