mirror of
https://github.com/rcourtman/Pulse.git
synced 2026-09-10 02:25:56 +00:00
Bypass approval gate for trusted internal Discovery commands
Discovery wraps every probe in `docker exec <container> 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.
This commit is contained in:
@@ -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
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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)
|
||||
|
||||
@@ -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 {
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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 <container>` 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,
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -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")
|
||||
}
|
||||
}
|
||||
|
||||
@@ -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")
|
||||
}
|
||||
}
|
||||
|
||||
@@ -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,
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
Reference in New Issue
Block a user