mirror of
https://github.com/rcourtman/Pulse.git
synced 2026-05-22 03:02:35 +00:00
Persist a refused audit record when plan drift is caught
Slice 13 added the drift refusal path but only logged at WARN level — the audit history showed nothing, so an operator reviewing the action trail could not see "Pulse caught this drift attempt." Now the drift branch writes a Failed audit record with Result.ErrorMessage prefixed "plan_drift:" and dispatches a Failed lifecycle event before returning ErrActionPlanDrift. The record carries the same Request, Plan, and Approvals snapshots that a normal audit record would, so the operator-facing audit row shows exactly what was attempted and what was approved. The "plan_drift:" prefix is a stable token for downstream surfaces (audit UI filters, alert rules) to distinguish drift refusals from generic execution failures. Extends TestExecuteCommandWithAuditRefusesPayloadDriftAgainstApprovedPlan to assert the audit record exists with State=Failed and the plan_drift error message after refusal. Updates the code-standards snippet check for ErrActionPlanDrift to match gofmt's actual alignment in actions.go. ai-runtime contract pinned with the audit-record-on-drift rule.
This commit is contained in:
parent
a9e652e05b
commit
22ce58cb9b
4 changed files with 69 additions and 27 deletions
|
|
@ -303,6 +303,14 @@ runtime cost control, and shared AI transport surfaces.
|
|||
action path uses a different hash function (`actionPlanHashForParams`)
|
||||
so a coherent canonical-hash refactor must precede adding the same
|
||||
check there.
|
||||
Drift refusal must also persist a Failed audit record with the
|
||||
Request, Plan, and Approvals snapshots intact and a Result whose
|
||||
ErrorMessage is prefixed `plan_drift:` so the audit trail shows
|
||||
every drift attempt that was caught. Operators reviewing the action
|
||||
audit history must be able to see drift refusals as first-class
|
||||
audit rows, not only in WARN-level logs; the `plan_drift:` prefix
|
||||
is a stable token for audit-UI filters and alert rules to
|
||||
distinguish drift from generic execution failures.
|
||||
`FindingsStore.GetTrustSummary` returns a snapshot of how currently
|
||||
tracked findings have resolved (tracked, currently-active, resolved,
|
||||
auto-resolved, fix-verified, fix-failed, dismissed-as-noise,
|
||||
|
|
|
|||
|
|
@ -74,32 +74,6 @@ func (e *PulseToolExecutor) executeCommandWithAudit(
|
|||
}
|
||||
approvalRecords := approvalRecordsForID(approvalID)
|
||||
|
||||
// Plan-drift check: the operator approved a specific command + target +
|
||||
// reason combination, and the broker must refuse to run a different one
|
||||
// even when the approval ID resolves. Compares the approved hash against
|
||||
// a freshly-recomputed approval-equivalent hash from the actual payload.
|
||||
if planFromApproval {
|
||||
if driftErr := validateApprovedCommandPlanHash(
|
||||
plan.PlanHash,
|
||||
plan.ActionID,
|
||||
plan.RequestID,
|
||||
capabilityName,
|
||||
resourceID,
|
||||
payload.Command,
|
||||
payload.TargetType,
|
||||
payload.TargetID,
|
||||
reason,
|
||||
); driftErr != nil {
|
||||
log.Warn().
|
||||
Str("action_id", plan.ActionID).
|
||||
Str("approval_id", approvalID).
|
||||
Str("capability", capabilityName).
|
||||
Err(driftErr).
|
||||
Msg("Refusing action execution: payload does not match approved plan hash")
|
||||
return nil, driftErr
|
||||
}
|
||||
}
|
||||
|
||||
record := unifiedresources.ActionAuditRecord{
|
||||
ID: actionID,
|
||||
CreatedAt: now,
|
||||
|
|
@ -124,6 +98,43 @@ func (e *PulseToolExecutor) executeCommandWithAudit(
|
|||
}
|
||||
record.Approvals = approvalRecords
|
||||
|
||||
// Plan-drift check: the operator approved a specific command + target +
|
||||
// reason combination, and the broker must refuse to run a different one
|
||||
// even when the approval ID resolves. Compares the approved hash against
|
||||
// a freshly-recomputed approval-equivalent hash from the actual payload.
|
||||
// Persists a refused audit record so operators can see drift caught in
|
||||
// the audit history rather than only in WARN logs.
|
||||
if planFromApproval {
|
||||
if driftErr := validateApprovedCommandPlanHash(
|
||||
plan.PlanHash,
|
||||
plan.ActionID,
|
||||
plan.RequestID,
|
||||
capabilityName,
|
||||
resourceID,
|
||||
payload.Command,
|
||||
payload.TargetType,
|
||||
payload.TargetID,
|
||||
reason,
|
||||
); driftErr != nil {
|
||||
log.Warn().
|
||||
Str("action_id", plan.ActionID).
|
||||
Str("approval_id", approvalID).
|
||||
Str("capability", capabilityName).
|
||||
Err(driftErr).
|
||||
Msg("Refusing action execution: payload does not match approved plan hash")
|
||||
now := time.Now().UTC()
|
||||
record.State = unifiedresources.ActionStateFailed
|
||||
record.UpdatedAt = now
|
||||
record.Result = &unifiedresources.ExecutionResult{
|
||||
Success: false,
|
||||
ErrorMessage: fmt.Sprintf("plan_drift: %s", driftErr.Error()),
|
||||
}
|
||||
e.recordActionAudit(record)
|
||||
e.recordActionLifecycle(record.ID, unifiedresources.ActionStateFailed, requestedBy, "plan drift refused")
|
||||
return nil, driftErr
|
||||
}
|
||||
}
|
||||
|
||||
record, err := e.recordActionExecutionStart(record, approvalID, requestedBy, fmt.Sprintf("dispatching command to agent %s", agentID), planFromApproval)
|
||||
if err != nil {
|
||||
return nil, err
|
||||
|
|
|
|||
|
|
@ -3,6 +3,7 @@ package tools
|
|||
import (
|
||||
"context"
|
||||
"errors"
|
||||
"strings"
|
||||
"testing"
|
||||
"time"
|
||||
|
||||
|
|
@ -556,6 +557,28 @@ func TestExecuteCommandWithAuditRefusesPayloadDriftAgainstApprovedPlan(t *testin
|
|||
t.Fatalf("expected nil result on drift refusal, got %#v", result)
|
||||
}
|
||||
agentServer.AssertNotCalled(t, "ExecuteCommand", mock.Anything, mock.Anything, mock.Anything)
|
||||
|
||||
// Drift refusal must be observable in the audit history, not just in
|
||||
// WARN logs. Operators reviewing the action audit trail need to see
|
||||
// "Pulse caught this drift attempt" recorded as a Failed action with
|
||||
// a plan_drift error message.
|
||||
audits, err := actionStore.GetActionAudits("agent-1", time.Time{}, 10)
|
||||
if err != nil {
|
||||
t.Fatalf("GetActionAudits: %v", err)
|
||||
}
|
||||
if len(audits) != 1 {
|
||||
t.Fatalf("expected 1 drift-refused audit record, got %d", len(audits))
|
||||
}
|
||||
driftAudit := audits[0]
|
||||
if driftAudit.State != unifiedresources.ActionStateFailed {
|
||||
t.Fatalf("drift audit state = %q, want %q", driftAudit.State, unifiedresources.ActionStateFailed)
|
||||
}
|
||||
if driftAudit.Result == nil || driftAudit.Result.Success {
|
||||
t.Fatalf("expected drift audit Result.Success=false, got %#v", driftAudit.Result)
|
||||
}
|
||||
if driftAudit.Result == nil || !strings.Contains(driftAudit.Result.ErrorMessage, "plan_drift") {
|
||||
t.Fatalf("expected drift audit ErrorMessage to include plan_drift, got %q", driftAudit.Result.ErrorMessage)
|
||||
}
|
||||
}
|
||||
|
||||
// TestExecuteCommandWithAuditAllowsMatchingPlanHash covers the positive case:
|
||||
|
|
|
|||
|
|
@ -358,7 +358,7 @@ func TestActionExecutionContractStaysAPIOwned(t *testing.T) {
|
|||
// at approval time. Pinning it here keeps the broker contract
|
||||
// honest: drift refusal cannot silently turn into another error
|
||||
// kind that callers fail to detect.
|
||||
"ErrActionPlanDrift = errors.New(",
|
||||
"ErrActionPlanDrift = errors.New(",
|
||||
},
|
||||
filepath.Join(".", "store.go"): {
|
||||
"RecordActionExecutionStart(record ActionAuditRecord, event ActionLifecycleEvent) error",
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue