From 06fd4fc89e6f5ddb051280d4a7771f59a3ac5be3 Mon Sep 17 00:00:00 2001 From: rcourtman Date: Sun, 17 May 2026 21:59:39 +0100 Subject: [PATCH] Bypass approval gate for trusted internal Discovery commands Discovery wraps every probe in `docker exec sh -c '...'`. The agentexec command policy lists `^docker\s+exec\s` as RequireApproval (a sound default for user-driven docker exec) and Discovery has no path to mint or supply an ApprovalID. Result: every probe was rejected, the scanner returned empty CommandOutputs, and the AI fell back to "Unknown Infrastructure Resource" at confidence 0. The Discovery sub-tab rendered empty after a "successful" run. Add a Trusted bool to ExecuteCommandPayload on both the server-facing agentexec type and the agent's wire struct. When set, the approval gate is skipped on both ends and the server does not attempt to auto-mint an approval grant (which would fail with "approval id is required"). PolicyBlock still applies; this is not a way to run arbitrary commands. Only the discoveryCommandAdapter sets Trusted=true. The flag is never populated from a deserialised HTTP body or any user-driven path. Patrol fixes, Assistant remediation, and AI tool calls continue to flow through the governed approval-record path with a real ApprovalID. Contracts: amend agent-lifecycle Completion Obligations and Current State to document the lone exception to the on-agent approval rail, and amend ai-runtime to fence the Trusted flag to the discovery adapter only. --- .../v6/internal/subsystems/agent-lifecycle.md | 19 +++++- .../v6/internal/subsystems/ai-runtime.md | 12 ++++ internal/agentexec/server.go | 8 ++- internal/agentexec/server_test.go | 68 +++++++++++++++++++ internal/agentexec/types.go | 7 ++ internal/ai/discovery_adapter.go | 7 ++ internal/ai/discovery_adapter_test.go | 21 ++++++ internal/hostagent/agent_new_test.go | 41 +++++++++++ internal/hostagent/commands.go | 13 ++++ 9 files changed, 194 insertions(+), 2 deletions(-) diff --git a/docs/release-control/v6/internal/subsystems/agent-lifecycle.md b/docs/release-control/v6/internal/subsystems/agent-lifecycle.md index e4592ac62..f508a6400 100644 --- a/docs/release-control/v6/internal/subsystems/agent-lifecycle.md +++ b/docs/release-control/v6/internal/subsystems/agent-lifecycle.md @@ -980,6 +980,17 @@ profile and assignment columns, but embedded table framing must route through source flow, and any later unified-agent augmentation on an API-backed platform must remain an optional secondary path instead of silently becoming the required bootstrap. +17. Preserve the on-agent command-policy approval gate in + `internal/hostagent/commands.go` and `internal/agentexec/server.go`. + Both ends may honor a `Trusted` flag on `ExecuteCommandPayload` to + bypass the `PolicyRequireApproval` branch, but only when the payload + is constructed by a vetted Pulse-internal call site that ships its + own hardcoded command catalog (today only the servicediscovery deep + scanner via `internal/ai/discovery_adapter.go`). `PolicyBlock` must + still apply to trusted payloads, and `Trusted` must never be set + from a deserialised HTTP body, a user-supplied command string, an + AI tool call, or a governed approval consumer — those callers must + continue to carry an `ApprovalID` and approval grant. ## Current State @@ -2867,7 +2878,13 @@ installed command-execution capability, but it must re-evaluate `internal/agentexec/policy.go` immediately before `sh -c`, reject `PolicyBlock` commands regardless of caller, and require a consumed approval identifier before executing any `PolicyRequireApproval` command so a missed -control-plane gate cannot silently turn into host-level RCE. +control-plane gate cannot silently turn into host-level RCE. The single +exception is commands marked `Trusted` in the wire payload: those originate +from a vetted Pulse-internal subsystem whose command catalog is hardcoded +in Pulse source (today only the servicediscovery deep scanner, which wraps +read-only inspections in `docker exec`). `PolicyBlock` still applies to +trusted payloads. The `Trusted` field must never be set by a code path that +deserialises user input or accepts a caller-supplied command string. That same copied install transport must also normalize canonical base URLs before composing installer asset paths: when operators enter a trailing-slash Pulse URL, shell and PowerShell install commands must trim it before appending diff --git a/docs/release-control/v6/internal/subsystems/ai-runtime.md b/docs/release-control/v6/internal/subsystems/ai-runtime.md index 6424fa6c0..9bc584a7e 100644 --- a/docs/release-control/v6/internal/subsystems/ai-runtime.md +++ b/docs/release-control/v6/internal/subsystems/ai-runtime.md @@ -1139,6 +1139,18 @@ runtime must carry that approval identifier into the final `agentexec.ExecuteCommandPayload` so the host agent can re-check the shared command policy locally and fail closed on blocked or still-unapproved commands instead of treating control-plane approval as an implicit bypass. +Discovery deep scans are the one runtime that does not flow through the +approval boundary. `internal/ai/discovery_adapter.go` is the only call site +allowed to mark an `agentexec.ExecuteCommandPayload` as `Trusted`. The +catalog of probes lives in `internal/servicediscovery/commands.go`, is +read-only by construction (`cat`, `ps`, `ss`, `find` under known config +roots) and is wrapped in `docker exec`, `pct exec`, or `qm guest exec` +without ever interpolating caller-supplied strings. Both the server-side +`agentexec` authorize path and the agent-side `hostagent` authorize path +must honor that `Trusted` flag by bypassing the approval requirement, +while still enforcing `PolicyBlock`. AI tool calls, Patrol fixes, and +Assistant remediation must continue to flow through the governed approval +record path and must never set `Trusted` on their payloads. The same action-audit boundary now also requires persisted action records to carry a normalized plan and preflight: action id, request id, capability, approval policy, dry-run availability, safety checks, verification steps, and diff --git a/internal/agentexec/server.go b/internal/agentexec/server.go index ff9fbede5..62597b4c4 100644 --- a/internal/agentexec/server.go +++ b/internal/agentexec/server.go @@ -201,6 +201,12 @@ func (s *Server) authorizeCommandPayload(cmd ExecuteCommandPayload) error { case PolicyBlock: return fmt.Errorf("command blocked by policy") case PolicyRequireApproval: + // Trusted internal subsystems (e.g. servicediscovery deep scans) carry + // a hardcoded command catalog and never accept user-supplied commands, + // so the user-driven approval gate does not apply to them. + if cmd.Trusted { + return nil + } if cmd.ApprovalID == "" { return fmt.Errorf("command requires approval") } @@ -853,7 +859,7 @@ func (s *Server) ExecuteCommand(ctx context.Context, agentID string, cmd Execute if err := s.authorizeCommandPayload(cmd); err != nil { return nil, err } - if s.commandPolicy != nil && s.commandPolicy.Evaluate(cmd.Command) == PolicyRequireApproval && cmd.ApprovalGrant == nil && len(ac.approvalGrantKey) > 0 { + if !cmd.Trusted && s.commandPolicy != nil && s.commandPolicy.Evaluate(cmd.Command) == PolicyRequireApproval && cmd.ApprovalGrant == nil && len(ac.approvalGrantKey) > 0 { grant, grantErr := NewCommandApprovalGrant(ac.approvalGrantKey, agentID, cmd, time.Now(), DefaultApprovalGrantTTL) if grantErr != nil { return nil, fmt.Errorf("failed to issue approval grant: %w", grantErr) diff --git a/internal/agentexec/server_test.go b/internal/agentexec/server_test.go index 9f75e68d8..e9bcda372 100644 --- a/internal/agentexec/server_test.go +++ b/internal/agentexec/server_test.go @@ -3,6 +3,7 @@ package agentexec import ( "context" "strings" + "sync" "testing" "time" ) @@ -167,6 +168,73 @@ func TestExecuteCommandSecurityValidation_RequiresApprovalIDWhenPolicyRequiresAp } } +func TestAuthorizeCommandPayload_TrustedBypassesApproval(t *testing.T) { + s := NewServer(allowAllTestTokens) + + // A command in the require-approval list (docker exec wraps Discovery probes). + cmd := ExecuteCommandPayload{ + RequestID: "r1", + Command: "docker exec pbs sh -c 'cat /etc/os-release'", + Timeout: 1, + } + + if err := s.authorizeCommandPayload(cmd); err == nil { + t.Fatalf("baseline: expected approval-required error without Trusted, got nil") + } + + cmd.Trusted = true + if err := s.authorizeCommandPayload(cmd); err != nil { + t.Fatalf("Trusted payload should bypass approval gate, got %v", err) + } + + // Blocked commands are still blocked even when Trusted (defense in depth). + blocked := ExecuteCommandPayload{ + RequestID: "r2", + Command: "rm -rf /", + Trusted: true, + Timeout: 1, + } + if err := s.authorizeCommandPayload(blocked); err == nil || !strings.Contains(err.Error(), "blocked by policy") { + t.Fatalf("Trusted must not bypass PolicyBlock; got %v", err) + } +} + +func TestExecuteCommand_TrustedSkipsApprovalGrantMint(t *testing.T) { + // Regression for the auto-grant path: even when the policy says + // RequireApproval, a Trusted command without an ApprovalID must not + // trigger NewCommandApprovalGrant (which errors "approval id is + // required"). The grant is only meaningful for user-driven approvals. + s := NewServer(allowAllTestTokens) + serverConn, _, cleanup := newConnPair(t) + defer cleanup() + ac := &agentConn{ + conn: serverConn, + agent: ConnectedAgent{AgentID: "a1"}, + approvalGrantKey: []byte("not-empty-so-the-mint-path-is-reachable"), + done: make(chan struct{}), + writeMu: sync.Mutex{}, + } + s.mu.Lock() + s.agents["a1"] = ac + s.mu.Unlock() + + ctx, cancel := context.WithTimeout(context.Background(), 200*time.Millisecond) + defer cancel() + _, err := s.ExecuteCommand(ctx, "a1", ExecuteCommandPayload{ + RequestID: "r1", + Command: "docker exec pbs sh -c 'cat /etc/os-release'", + Trusted: true, + Timeout: 1, + }) + // Expected: command is dispatched and we time out waiting for a result + // from the fake conn (no agent code on the other side). What must NOT + // appear is the "approval id is required" / "failed to issue approval + // grant" mint failure. + if err != nil && (strings.Contains(err.Error(), "failed to issue approval grant") || strings.Contains(err.Error(), "approval id is required")) { + t.Fatalf("Trusted command must skip auto-grant mint, got %v", err) + } +} + func TestReadFileValidation(t *testing.T) { s := NewServer(allowAllTestTokens) if _, err := s.ReadFile(context.Background(), "", ReadFilePayload{RequestID: "r1"}); err == nil { diff --git a/internal/agentexec/types.go b/internal/agentexec/types.go index bcca78053..46df39575 100644 --- a/internal/agentexec/types.go +++ b/internal/agentexec/types.go @@ -97,6 +97,13 @@ type ExecuteCommandPayload struct { TargetType string `json:"target_type"` // "agent", "container", "vm" TargetID string `json:"target_id,omitempty"` // VMID for container/VM Timeout int `json:"timeout,omitempty"` // seconds, 0 = default + // Trusted marks a payload as originating from a Pulse-internal subsystem + // whose command catalog is hardcoded and vetted (e.g. servicediscovery + // deep scans wrap read-only inspections in `docker exec`). When set, the + // server's command policy approval gate is bypassed for this command. + // This field must only be set by trusted internal call sites — never + // from a deserialised HTTP body or any user-driven path. + Trusted bool `json:"trusted,omitempty"` } // ReadFilePayload is sent by server to request file content diff --git a/internal/ai/discovery_adapter.go b/internal/ai/discovery_adapter.go index 0703e8685..99203ae9d 100644 --- a/internal/ai/discovery_adapter.go +++ b/internal/ai/discovery_adapter.go @@ -139,6 +139,13 @@ func normalizeDiscoveryExecuteCommandPayload(ctx context.Context, cmd servicedis TargetType: cmd.TargetType, TargetID: cmd.TargetID, Timeout: timeoutSeconds, + // Discovery probes are a fixed, in-source catalog of read-only + // inspections (cat /etc/os-release, ps, ss, env, find under a few + // known config roots) wrapped in `docker exec ` or + // `pct/qm exec`. They never accept user-supplied commands. The + // trusted flag lets them bypass the user-driven approval gate that + // would otherwise block every `docker exec` dispatch. + Trusted: true, } } diff --git a/internal/ai/discovery_adapter_test.go b/internal/ai/discovery_adapter_test.go index 7a9517c6b..975eb2912 100644 --- a/internal/ai/discovery_adapter_test.go +++ b/internal/ai/discovery_adapter_test.go @@ -264,3 +264,24 @@ func TestDiscoveryCommandAdapter_ExecuteCommandSuccess(t *testing.T) { t.Fatal(asyncErr) } } + +func TestNormalizeDiscoveryExecuteCommandPayload_SetsTrusted(t *testing.T) { + // Discovery probes are wrapped in `docker exec ...` (or pct/qm exec) and + // would otherwise hit the agentexec command-policy approval gate, which + // returns "command requires approval" because Discovery has no + // user-driven ApprovalID. The Trusted flag is the contract the adapter + // uses to opt those internal Pulse-source probes out of the approval + // gate on both the server and agent sides while leaving PolicyBlock + // intact. This test pins that contract. + ctx := context.Background() + out := normalizeDiscoveryExecuteCommandPayload(ctx, servicediscovery.ExecuteCommandPayload{ + RequestID: "req-trusted", + Command: "docker exec pbs sh -c 'cat /etc/os-release'", + TargetType: "agent", + TargetID: "agent-1", + Timeout: 5, + }) + if !out.Trusted { + t.Fatal("normalizeDiscoveryExecuteCommandPayload must set Trusted=true on outgoing agentexec payload") + } +} diff --git a/internal/hostagent/agent_new_test.go b/internal/hostagent/agent_new_test.go index 7c26986f3..e3bb02229 100644 --- a/internal/hostagent/agent_new_test.go +++ b/internal/hostagent/agent_new_test.go @@ -1122,3 +1122,44 @@ func TestNew_FallsBackToMACWhenMachineIDEmpty(t *testing.T) { t.Fatalf("machineID = %q, want %q", agent.machineID, "mac-020000000001") } } + +func TestExecuteCommandPayload_TrustedBypassesAgentApprovalGate(t *testing.T) { + // Discovery deep scans wrap probes in `docker exec ...` and dispatch them + // over the command WebSocket with Trusted=true. The agent-side authorize + // gate must honor that flag (matching the agentexec server-side behavior) + // or every scan returns empty stdout because the agent refuses to run the + // command — which is the bug this regression test guards against. + c := &CommandClient{commandPolicy: agentexec.DefaultPolicy()} + + requireApproval := executeCommandPayload{ + RequestID: "r1", + Command: "docker exec pbs sh -c 'cat /etc/os-release'", + Timeout: 1, + } + if err := c.authorizeCommand(requireApproval); err == nil { + t.Fatalf("baseline: agent must reject untrusted docker exec without ApprovalID") + } + + requireApproval.Trusted = true + if err := c.authorizeCommand(requireApproval); err != nil { + t.Fatalf("Trusted=true must bypass the agent approval gate, got %v", err) + } + + // PolicyBlock is still enforced even when Trusted. + blocked := executeCommandPayload{ + RequestID: "r2", + Command: "rm -rf /", + Trusted: true, + Timeout: 1, + } + if err := c.authorizeCommand(blocked); err == nil || !strings.Contains(err.Error(), "blocked by policy") { + t.Fatalf("Trusted must not bypass PolicyBlock; got %v", err) + } + + // toAgentExecPayload must propagate Trusted so the agentexec server side + // can also bypass its mirrored approval mint step. + round := requireApproval.toAgentExecPayload() + if !round.Trusted { + t.Fatalf("toAgentExecPayload dropped Trusted field; round-trip must preserve it") + } +} diff --git a/internal/hostagent/commands.go b/internal/hostagent/commands.go index ec57b14e4..989fd1ca1 100644 --- a/internal/hostagent/commands.go +++ b/internal/hostagent/commands.go @@ -197,6 +197,12 @@ type executeCommandPayload struct { TargetType string `json:"target_type"` TargetID string `json:"target_id,omitempty"` Timeout int `json:"timeout,omitempty"` + // Trusted is set by the Pulse server when the command originates from a + // vetted internal subsystem (e.g. servicediscovery deep scans whose + // command catalog is hardcoded in Pulse source). When set, the agent's + // policy approval gate is bypassed since the command did not come from + // a user-driven path. PolicyBlock is still enforced. + Trusted bool `json:"trusted,omitempty"` } type commandResultPayload struct { @@ -641,6 +647,12 @@ func (c *CommandClient) authorizeCommand(payload executeCommandPayload) error { case agentexec.PolicyBlock: return fmt.Errorf("command blocked by policy") case agentexec.PolicyRequireApproval: + // Trusted commands come from vetted Pulse-internal subsystems over an + // authenticated WebSocket; they do not require user approval. The + // PolicyBlock branch above still enforces hard limits. + if payload.Trusted { + return nil + } if strings.TrimSpace(payload.ApprovalID) == "" { return fmt.Errorf("command requires approval") } @@ -661,6 +673,7 @@ func (p executeCommandPayload) toAgentExecPayload() agentexec.ExecuteCommandPayl TargetType: normalizedCommandTargetType(p.TargetType), TargetID: strings.TrimSpace(p.TargetID), Timeout: p.Timeout, + Trusted: p.Trusted, } }