Require approval for backend alert investigation commands

This commit is contained in:
rcourtman
2026-05-07 01:24:57 +01:00
parent f177d16545
commit ad61b6c19f
11 changed files with 152 additions and 28 deletions
@@ -1094,6 +1094,11 @@ AI handlers add split scoped-trigger fields, recency labels, or trigger-state
transport for Patrol, lifecycle-adjacent setup and fleet surfaces must treat
those payloads as Patrol-only runtime context and must not reinterpret them as
agent install readiness, enrollment health, or fleet-control state.
That same shared AI handler dependency also assumes direct alert-investigation
execution mode is AI/API-owned. Request-scoped `AutonomousMode:false` and
`RequireCommandApproval:true` on `/api/ai/investigate-alert` are Assistant
action-governance facts, not agent install readiness, command reachability, or
fleet-control capability signals.
That same shared `internal/api/` dependency also now assumes SSO test and
metadata-preview routes fail closed on validated outbound URL handling.
Lifecycle-adjacent setup and hosted bootstrap surfaces may depend on those
@@ -1055,6 +1055,13 @@ only governed prompt/context data, but the submitted chat request must set
`autonomous_mode:false`, preserve the operator's persistent Assistant
control-level setting, and disclose the temporary approval-required mode in
the drawer instead of showing the generic Autonomous warning.
Direct alert-investigation runtime handoffs follow the same rule even when
they bypass the chat drawer. `/api/ai/investigate-alert` must set
`ai.ExecuteRequest.AutonomousMode` to false plus
`ai.ExecuteRequest.RequireCommandApproval` to true, and
`internal/ai/alert_provider.go` must frame diagnostics as approval-bound
operator actions rather than instructing the model to execute commands because
they appear safe.
Those backend AI and Patrol change summaries should derive their canonical
labels and provenance fragments from
`internal/unifiedresources/change_presentation.go`, so the resource-model
@@ -327,7 +327,12 @@ the canonical monitored-system blocked payload.
surfaces. Frontend finding-discussion handoffs that carry any live approval,
proposed-fix, fix outcome, or remediation-plan reference must force a
request-local approval-required Assistant mode instead of inheriting the
user's persistent autonomous control setting. The operator
user's persistent autonomous control setting. Direct alert-investigation API
handoffs through `internal/api/ai_handlers.go` must enforce that same
request-scoped boundary by setting `ai.ExecuteRequest.AutonomousMode` to
false and `ai.ExecuteRequest.RequireCommandApproval` to true; API proof must
keep this guarded in both `internal/api/ai_handlers_test.go` and
`internal/api/contract_test.go`. The operator
decision and action-posture lines in the briefing must derive from those
same structured action references after recovery so the briefing cannot
contradict the handoff action payload. Related finding
@@ -393,6 +393,11 @@ bypass the API fail-closed execution gate.
19. Preserve shipped local security-doc guidance in shared `internal/api/` config/setup helpers so storage- and recovery-adjacent transport surfaces do not reintroduce GitHub `main` security links when the running build already serves its own local security documentation route.
20. Keep shared `internal/api/` Patrol transport and alert-trigger edits feature-isolated: Patrol-specific recency fields, callback fan-out, or alert-bridge wiring changes must not leak into recovery queries, storage links, or recovery-adjacent install/setup flows unless this contract changes in the same slice.
The same adjacency rule applies to AI settings transport in `internal/api/ai_handlers.go`: provider auth state, masked-secret payload fields, and provider-test model selection remain AI/runtime plus API-contract concerns and must not be absorbed into storage/recovery transport ownership just because those handlers live under the shared backend API tree.
Direct alert-investigation execution controls in `internal/api/ai_handlers.go`
follow that same split: request-scoped `AutonomousMode:false` and
`RequireCommandApproval:true` are AI action-governance constraints, not
storage/recovery restore approval, recovery freshness, or storage diagnostic
payload semantics.
That same adjacent boundary also keeps the retired Patrol quickstart
contract out of storage/recovery ownership: shared AI handlers no longer
expose active quickstart credit, token, or hosted-model provider state, and
+1 -1
View File
@@ -300,7 +300,7 @@ func GenerateAlertInvestigationPrompt(req AlertInvestigationRequest) string {
prompt.WriteString("1. Identify the root cause of this alert\n")
prompt.WriteString("2. Check related metrics and system state\n")
prompt.WriteString("3. Suggest specific remediation steps\n")
prompt.WriteString("4. If safe, execute diagnostic commands to gather more info\n")
prompt.WriteString("4. Ask for operator approval before running any diagnostic command or change\n")
return prompt.String()
}
+6
View File
@@ -229,6 +229,12 @@ func TestGenerateAlertInvestigationPrompt(t *testing.T) {
!strings.Contains(out, "**Node:** pve1") {
t.Fatalf("unexpected prompt: %s", out)
}
if !strings.Contains(out, "Ask for operator approval before running any diagnostic command") {
t.Fatalf("expected approval-bound diagnostic guidance, got: %s", out)
}
if strings.Contains(out, "execute diagnostic commands") {
t.Fatalf("prompt must not instruct autonomous diagnostic command execution: %s", out)
}
}
func TestAlertInvestigationRequestJSONCanonicalOutput(t *testing.T) {
+49 -23
View File
@@ -1838,6 +1838,13 @@ func (s *Service) IsAutonomous() bool {
return true
}
func (s *Service) isAutonomousForRequest(req ExecuteRequest) bool {
if req.AutonomousMode != nil && !*req.AutonomousMode {
return false
}
return s.IsAutonomous()
}
// ConversationMessage represents a message in conversation history
type ConversationMessage struct {
Role string `json:"role"` // "user" or "assistant"
@@ -1846,15 +1853,17 @@ type ConversationMessage struct {
// ExecuteRequest represents a request to execute an AI prompt
type ExecuteRequest struct {
Prompt string `json:"prompt"`
TargetType string `json:"target_type,omitempty"` // "agent", "system-container", "vm"
TargetID string `json:"target_id,omitempty"`
Context map[string]interface{} `json:"context,omitempty"` // Current metrics, state, etc.
SystemPrompt string `json:"system_prompt,omitempty"` // Override system prompt
History []ConversationMessage `json:"history,omitempty"` // Previous conversation messages
FindingID string `json:"finding_id,omitempty"` // If fixing a patrol finding, the ID to resolve
Model string `json:"model,omitempty"` // Override model for this request (for user selection in chat)
UseCase string `json:"use_case,omitempty"` // "chat" or "patrol" - determines which default model to use
Prompt string `json:"prompt"`
TargetType string `json:"target_type,omitempty"` // "agent", "system-container", "vm"
TargetID string `json:"target_id,omitempty"`
Context map[string]interface{} `json:"context,omitempty"` // Current metrics, state, etc.
SystemPrompt string `json:"system_prompt,omitempty"` // Override system prompt
History []ConversationMessage `json:"history,omitempty"` // Previous conversation messages
FindingID string `json:"finding_id,omitempty"` // If fixing a patrol finding, the ID to resolve
Model string `json:"model,omitempty"` // Override model for this request (for user selection in chat)
UseCase string `json:"use_case,omitempty"` // "chat" or "patrol" - determines which default model to use
AutonomousMode *bool `json:"autonomous_mode,omitempty"` // Per-request execution override; false keeps scoped handoffs approval-required
RequireCommandApproval bool `json:"require_command_approval,omitempty"` // Force every run_command tool call through operator approval
}
// ExecuteResponse represents the AI's response
@@ -2386,6 +2395,7 @@ Always execute the commands rather than telling the user how to do it.`
// Check if this command needs approval
needsApproval := false
approvalID := ""
approvalReason := "Command requires user approval"
if tc.Name == "run_command" {
cmd, _ := tc.Input["command"].(string)
runOnHost, _ := tc.Input["run_on_host"].(bool)
@@ -2406,7 +2416,7 @@ Always execute the commands rather than telling the user how to do it.`
}
}
isAuto := s.IsAutonomous()
isAuto := s.isAutonomousForRequest(req)
policyDecision := s.policy.Evaluate(cmd)
log.Debug().
Bool("autonomous", isAuto).
@@ -2446,10 +2456,13 @@ Always execute the commands rather than telling the user how to do it.`
continue
}
// If policy requires approval and we're not in autonomous mode, request approval.
if !isAuto && policyDecision == agentexec.PolicyRequireApproval {
if req.RequireCommandApproval || (!isAuto && policyDecision == agentexec.PolicyRequireApproval) {
needsApproval = true
anyNeedsApproval = true
approvalReason = "Security policy requires approval"
if req.RequireCommandApproval {
approvalReason = "This handoff requires operator approval before command execution"
}
approvalID = createRunCommandApprovalRecord(
approvalOrgID,
cmd,
@@ -2457,7 +2470,7 @@ Always execute the commands rather than telling the user how to do it.`
req,
runOnHost,
targetHost,
"Security policy requires approval",
approvalReason,
)
callback(StreamEvent{
Type: "approval_needed",
@@ -2482,7 +2495,7 @@ Always execute the commands rather than telling the user how to do it.`
// Note: We don't add to toolExecutions here because the approval_needed event
// already tells the frontend to show the approval UI
cmd, _ := tc.Input["command"].(string)
result = formatApprovalNeededToolResult(cmd, tc.ID, "Command requires user approval", approvalID)
result = formatApprovalNeededToolResult(cmd, tc.ID, approvalReason, approvalID)
execution = ToolExecution{
Name: tc.Name,
Input: toolInput,
@@ -3106,10 +3119,14 @@ func (s *Service) executeTool(ctx context.Context, req ExecuteRequest, tc provid
execution.Output = formatPolicyBlockedToolResult(command, "This command is blocked by security policy")
return execution.Output, execution
}
if decision == agentexec.PolicyRequireApproval && !s.IsAutonomous() {
if req.RequireCommandApproval || (decision == agentexec.PolicyRequireApproval && !s.isAutonomousForRequest(req)) {
s.mu.RLock()
approvalOrgID := s.orgID
s.mu.RUnlock()
approvalReason := "Security policy requires approval"
if req.RequireCommandApproval {
approvalReason = "This handoff requires operator approval before command execution"
}
approvalID := createRunCommandApprovalRecord(
approvalOrgID,
command,
@@ -3117,9 +3134,9 @@ func (s *Service) executeTool(ctx context.Context, req ExecuteRequest, tc provid
req,
runOnHost,
targetHost,
"Security policy requires approval",
approvalReason,
)
execution.Output = formatApprovalNeededToolResult(command, tc.ID, "Security policy requires approval", approvalID)
execution.Output = formatApprovalNeededToolResult(command, tc.ID, approvalReason, approvalID)
execution.Success = true // Not an error, just needs approval
return execution.Output, execution
}
@@ -3992,14 +4009,23 @@ func (s *Service) RunCommand(ctx context.Context, req RunCommandRequest) (*RunCo
// buildSystemPrompt creates the system prompt based on the request context
func (s *Service) buildSystemPrompt(req ExecuteRequest, destinationModel string) string {
prompt := `You are Pulse's diagnostic assistant for Proxmox, Docker, and Kubernetes homelabs.
## Command Approval
Pulse has a built-in approval system:
commandApproval := `Pulse has a built-in approval system:
- Safe commands (read-only: ls, df, cat, ps) execute immediately
- Destructive commands (rm, service restart, apt install) require user approval
- Blocked commands are never executed
Execute commands normally - Pulse handles the approval flow automatically.
Execute commands normally - Pulse handles the approval flow automatically.`
if req.RequireCommandApproval {
commandApproval = `This handoff is approval-bound:
- Do not run commands just because they look safe.
- Any run_command tool call will pause for explicit operator approval before execution.
- Prefer existing metrics, alert context, and known system state before requesting command approval.
- Blocked commands are never executed.`
}
prompt := fmt.Sprintf(`You are Pulse's diagnostic assistant for Proxmox, Docker, and Kubernetes homelabs.
## Command Approval
%s
## Command Execution
- run_on_host=true: Run on PVE/Docker host (pct, qm, vzdump, docker commands)
@@ -4030,7 +4056,7 @@ Example: "homelab:pve-node:201" for VMID 201 on node pve-node in instance homela
## Installing/Updating Pulse
curl -sSL https://raw.githubusercontent.com/rcourtman/Pulse/main/install.sh | bash
After: systemctl enable pulse && systemctl start pulse
Latest version: https://api.github.com/repos/rcourtman/Pulse/releases/latest`
Latest version: https://api.github.com/repos/rcourtman/Pulse/releases/latest`, commandApproval)
// Add custom context from AI settings (user's infrastructure description)
s.mu.RLock()
+41
View File
@@ -735,6 +735,32 @@ func TestService_ExecuteTool_RunCommand(t *testing.T) {
if !strings.Contains(result, "APPROVAL_REQUIRED") {
t.Errorf("Expected APPROVAL_REQUIRED message, got %s", result)
}
// 4. Request-scoped approval mode must clamp an autonomous default.
autonomousMode := false
svc.cfg = &config.AIConfig{ControlLevel: config.ControlLevelAutonomous}
req.AutonomousMode = &autonomousMode
result, exec = svc.executeTool(ctx, req, tc)
if !exec.Success {
t.Errorf("Expected success (not an error to need approval), got: %s", result)
}
if !strings.Contains(result, "APPROVAL_REQUIRED") {
t.Errorf("Expected request-scoped approval mode to require approval, got %s", result)
}
// 5. Approval-bound handoffs require approval even for policy-allowed commands.
mockPolicy.decision = agentexec.PolicyAllow
req.RequireCommandApproval = true
result, exec = svc.executeTool(ctx, req, tc)
if !exec.Success {
t.Errorf("Expected success (not an error to need approval), got: %s", result)
}
if !strings.Contains(result, "APPROVAL_REQUIRED") {
t.Errorf("Expected approval-bound handoff to require approval, got %s", result)
}
if !strings.Contains(result, "handoff requires operator approval") {
t.Errorf("Expected handoff approval reason, got %s", result)
}
}
func TestService_ExecuteTool_SetResourceURL(t *testing.T) {
@@ -2929,6 +2955,21 @@ func TestService_FindingContextInSystemPrompt(t *testing.T) {
}
}
func TestService_BuildSystemPrompt_ApprovalBoundCommandHandoff(t *testing.T) {
svc := NewService(nil, nil)
prompt := svc.buildSystemPrompt(ExecuteRequest{RequireCommandApproval: true}, "")
if !strings.Contains(prompt, "This handoff is approval-bound") {
t.Fatalf("expected approval-bound command guidance, got: %s", prompt)
}
if !strings.Contains(prompt, "Any run_command tool call will pause for explicit operator approval") {
t.Fatalf("expected all command tool calls to require approval, got: %s", prompt)
}
if strings.Contains(prompt, "Safe commands (read-only: ls, df, cat, ps) execute immediately") {
t.Fatalf("approval-bound prompt must not say safe commands execute immediately: %s", prompt)
}
}
// TestService_FindingContextCommandRouting verifies that commands route correctly
// when ExecuteStream is called with a FindingID. This is an end-to-end test.
func TestService_FindingContextCommandRouting(t *testing.T) {
+6 -3
View File
@@ -4300,10 +4300,13 @@ func (h *AISettingsHandler) HandleInvestigateAlert(w http.ResponseWriter, r *htt
}
}()
autonomousMode := false
resp, err := h.GetAIService(r.Context()).ExecuteStream(ctx, ai.ExecuteRequest{
Prompt: investigationPrompt,
TargetType: targetType,
TargetID: targetID,
Prompt: investigationPrompt,
TargetType: targetType,
TargetID: targetID,
AutonomousMode: &autonomousMode,
RequireCommandApproval: true,
Context: map[string]interface{}{
"alertIdentifier": alertIdentifier,
"alertType": req.AlertType,
+12
View File
@@ -2048,6 +2048,18 @@ func TestHandleInvestigateAlert_AcceptsCanonicalAlertIdentifier(t *testing.T) {
require.NotContains(t, rec.Header().Get("Content-Type"), "text/event-stream")
}
func TestHandleInvestigateAlert_ForcesApprovalBoundExecuteRequest(t *testing.T) {
t.Parallel()
source, err := os.ReadFile("ai_handlers.go")
require.NoError(t, err)
text := string(source)
require.Contains(t, text, "autonomousMode := false")
require.Contains(t, text, "AutonomousMode: &autonomousMode")
require.Contains(t, text, "RequireCommandApproval: true")
}
// ========================================
// AISettingsHandler setter method tests
// ========================================
+14
View File
@@ -295,6 +295,20 @@ func TestContract_AssistantFindingContextUsesModelOnlyHandoff(t *testing.T) {
}
}
func TestContract_InvestigateAlertIsApprovalBound(t *testing.T) {
source, err := os.ReadFile(filepath.Clean("ai_handlers.go"))
if err != nil {
t.Fatalf("read ai_handlers.go: %v", err)
}
text := string(source)
if !strings.Contains(text, "autonomousMode := false") ||
!strings.Contains(text, "AutonomousMode: &autonomousMode") ||
!strings.Contains(text, "RequireCommandApproval: true") {
t.Fatal("investigate-alert API handoff must force request-scoped approval and command approval")
}
}
func TestContract_ProxmoxSetupScriptUsesPrivilegeSeparatedTokenACLs(t *testing.T) {
storagePerms := `
pveum aclmod /storage -user pulse-monitor@pve -role PVEDatastoreAdmin