From 22ce58cb9b9ae648aa6dbbfd583ed40173b9029b Mon Sep 17 00:00:00 2001 From: rcourtman Date: Fri, 8 May 2026 22:20:01 +0100 Subject: [PATCH] Persist a refused audit record when plan drift is caught MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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. --- .../v6/internal/subsystems/ai-runtime.md | 8 +++ internal/ai/tools/action_audit.go | 63 +++++++++++-------- .../ai/tools/action_audit_execution_test.go | 23 +++++++ .../unifiedresources/code_standards_test.go | 2 +- 4 files changed, 69 insertions(+), 27 deletions(-) diff --git a/docs/release-control/v6/internal/subsystems/ai-runtime.md b/docs/release-control/v6/internal/subsystems/ai-runtime.md index 1b03badd4..67a54399a 100644 --- a/docs/release-control/v6/internal/subsystems/ai-runtime.md +++ b/docs/release-control/v6/internal/subsystems/ai-runtime.md @@ -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, diff --git a/internal/ai/tools/action_audit.go b/internal/ai/tools/action_audit.go index 40040a16a..eebc376ed 100644 --- a/internal/ai/tools/action_audit.go +++ b/internal/ai/tools/action_audit.go @@ -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 diff --git a/internal/ai/tools/action_audit_execution_test.go b/internal/ai/tools/action_audit_execution_test.go index 63b38cfb4..f643603aa 100644 --- a/internal/ai/tools/action_audit_execution_test.go +++ b/internal/ai/tools/action_audit_execution_test.go @@ -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: diff --git a/internal/unifiedresources/code_standards_test.go b/internal/unifiedresources/code_standards_test.go index e7ba7adc5..adcd70a77 100644 --- a/internal/unifiedresources/code_standards_test.go +++ b/internal/unifiedresources/code_standards_test.go @@ -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",