diff --git a/internal/api/agent_events_test.go b/internal/api/agent_events_test.go index 2a923ba50..e08cd21d7 100644 --- a/internal/api/agent_events_test.go +++ b/internal/api/agent_events_test.go @@ -646,6 +646,64 @@ func TestAgentEventBroadcaster_PublishActionCompletedRoundTripsVerification(t *t } } +func TestProjectAgentActionCompletedPayloadUsesCanonicalVerification(t *testing.T) { + ranAt := time.Now().UTC() + payload, ok := projectAgentActionCompletedPayload(unifiedresources.ActionAuditRecord{ + ID: "action-router-canonical-verification", + UpdatedAt: ranAt, + State: unifiedresources.ActionStateCompleted, + Request: unifiedresources.ActionRequest{ + ResourceID: "vm:router-canonical", + CapabilityName: "restart_service", + RequestedBy: "agent:ops", + }, + Result: &unifiedresources.ExecutionResult{Success: true}, + Verification: &unifiedresources.ActionVerificationResult{ + Ran: true, + Success: true, + Command: "systemctl is-active nginx", + RanAt: ranAt, + }, + }) + if !ok { + t.Fatal("expected terminal action to project") + } + if payload.Verification == nil || payload.Verification.Command != "systemctl is-active nginx" { + t.Fatalf("top-level canonical verification did not project onto router payload: %+v", payload.Verification) + } + + payload, ok = projectAgentActionCompletedPayload(unifiedresources.ActionAuditRecord{ + ID: "action-router-unrun-verification", + UpdatedAt: ranAt, + State: unifiedresources.ActionStateCompleted, + Request: unifiedresources.ActionRequest{ + ResourceID: "vm:router-unrun", + CapabilityName: "restart_service", + RequestedBy: "agent:ops", + }, + Result: &unifiedresources.ExecutionResult{ + Success: true, + Verification: &unifiedresources.ActionVerificationResult{ + Ran: false, + Success: true, + Command: "should not leak", + Output: "sensitive output", + Note: "sensitive note", + RanAt: ranAt, + }, + }, + }) + if !ok { + t.Fatal("expected terminal action to project") + } + if payload.Verification == nil || payload.Verification.Ran { + t.Fatalf("expected sanitized ran=false verification, got %+v", payload.Verification) + } + if payload.Verification.Command != "" || payload.Verification.Note != "" || !payload.Verification.RanAt.IsZero() || payload.Verification.Success { + t.Fatalf("router payload leaked ran=false verification details: %+v", payload.Verification) + } +} + func TestAgentEventBroadcaster_PublishActionCompletedAbsentVerificationOmitsField(t *testing.T) { // Refused-before-dispatch failures have no verification result // (verification only runs after a successful execute). The diff --git a/internal/api/agent_resource_context.go b/internal/api/agent_resource_context.go index 75d64e9eb..3028b0986 100644 --- a/internal/api/agent_resource_context.go +++ b/internal/api/agent_resource_context.go @@ -515,9 +515,9 @@ func projectAgentResourceActions( if audit.Result != nil { summary.Success = audit.Result.Success summary.ErrorMessage = audit.Result.ErrorMessage - if v := projectAgentResourceVerification(audit.Result.Verification); v != nil { - summary.Verification = v - } + } + if v := projectAgentResourceVerification(unified.CanonicalActionVerification(audit)); v != nil { + summary.Verification = v } out = append(out, summary) } diff --git a/internal/api/agent_resource_context_test.go b/internal/api/agent_resource_context_test.go index 24027dc04..3998a1d8a 100644 --- a/internal/api/agent_resource_context_test.go +++ b/internal/api/agent_resource_context_test.go @@ -469,6 +469,64 @@ func TestHandleAgentResourceContext_RedactsCommandsForMonitoringReadTokens(t *te } } +func TestProjectAgentResourceActionsUsesCanonicalVerification(t *testing.T) { + ranAt := time.Now().UTC() + summaries := projectAgentResourceActions([]unified.ActionAuditRecord{ + { + ID: "action-resource-canonical-verification", + CreatedAt: ranAt.Add(-time.Minute), + UpdatedAt: ranAt, + State: unified.ActionStateCompleted, + Request: unified.ActionRequest{ + ResourceID: "vm:resource-canonical", + CapabilityName: "restart_service", + RequestedBy: "agent:ops", + }, + Result: &unified.ExecutionResult{Success: true}, + Verification: &unified.ActionVerificationResult{ + Ran: true, + Success: true, + Command: "systemctl is-active nginx", + RanAt: ranAt, + }, + }, + { + ID: "action-resource-unrun-verification", + CreatedAt: ranAt.Add(-2 * time.Minute), + UpdatedAt: ranAt.Add(-time.Minute), + State: unified.ActionStateCompleted, + Request: unified.ActionRequest{ + ResourceID: "vm:resource-unrun", + CapabilityName: "restart_service", + RequestedBy: "agent:ops", + }, + Result: &unified.ExecutionResult{ + Success: true, + Verification: &unified.ActionVerificationResult{ + Ran: false, + Success: true, + Command: "should not leak", + Output: "sensitive output", + Note: "sensitive note", + RanAt: ranAt, + }, + }, + }, + }) + if len(summaries) != 2 { + t.Fatalf("summaries len = %d, want 2", len(summaries)) + } + if summaries[0].Verification == nil || summaries[0].Verification.Command != "systemctl is-active nginx" { + t.Fatalf("top-level canonical verification did not project onto resource summary: %+v", summaries[0].Verification) + } + if summaries[1].Verification == nil || summaries[1].Verification.Ran { + t.Fatalf("expected sanitized ran=false verification, got %+v", summaries[1].Verification) + } + if summaries[1].Verification.Command != "" || summaries[1].Verification.Note != "" || !summaries[1].Verification.RanAt.IsZero() || summaries[1].Verification.Success { + t.Fatalf("resource summary leaked ran=false verification details: %+v", summaries[1].Verification) + } +} + func TestHandleAgentResourceContext_PendingApprovalsEmptyArrayWhenNone(t *testing.T) { // Absent or empty must surface as an empty array, not as a // missing field — agents iterate without nil-checking. This diff --git a/internal/api/contract_test.go b/internal/api/contract_test.go index eb3297f35..5700a7bef 100644 --- a/internal/api/contract_test.go +++ b/internal/api/contract_test.go @@ -13874,7 +13874,8 @@ func TestContract_ActionCompletedPayloadCarriesVerification(t *testing.T) { // TestContract_RouterBridgesVerificationOntoActionCompleted pins // that the router-side bridge actually carries the verification -// projection from record.Result.Verification onto the SSE payload. +// projection from the canonical top-level verification helper onto +// the SSE payload. // The payload field exists is one half of the contract; this is the // other half — that the bridge populates it when the underlying // audit has a verification result. @@ -13884,8 +13885,8 @@ func TestContract_RouterBridgesVerificationOntoActionCompleted(t *testing.T) { t.Fatalf("read router.go: %v", err) } src := string(source) - if !strings.Contains(src, "if v := projectAgentResourceVerification(record.Result.Verification); v != nil {") { - t.Error("router.go must project record.Result.Verification onto the action.completed payload via projectAgentResourceVerification — drift here means verification reaches the audit store but never the SSE stream") + if !strings.Contains(src, "if v := projectAgentResourceVerification(unifiedresources.CanonicalActionVerification(record)); v != nil {") { + t.Error("router.go must project canonical action verification onto the action.completed payload via projectAgentResourceVerification — drift here means verification reaches the audit store but never the SSE stream") } if !strings.Contains(src, "payload.Verification = v") { t.Error("router.go must assign the projected verification onto payload.Verification") @@ -13911,6 +13912,9 @@ func TestContract_AgentResourceActionSummaryCarriesVerification(t *testing.T) { if !strings.Contains(src, "func projectAgentResourceVerification(v *unified.ActionVerificationResult)") { t.Error("projectAgentResourceVerification must exist as the shared helper — both the SSE bridge and the bundle's projector route through it so the wire shape cannot drift between surfaces") } + if !strings.Contains(src, "projectAgentResourceVerification(unified.CanonicalActionVerification(audit))") { + t.Error("AgentResourceActionSummary must project canonical action verification, not only legacy result.verification") + } } // TestContract_OperatorStateWriteServerPopulatesAttribution pins the diff --git a/internal/api/router.go b/internal/api/router.go index 9b6aee422..98d3b4338 100644 --- a/internal/api/router.go +++ b/internal/api/router.go @@ -2999,9 +2999,9 @@ func projectAgentActionCompletedPayload(record unifiedresources.ActionAuditRecor if record.Result != nil { payload.Success = record.Result.Success payload.ErrorMessage = record.Result.ErrorMessage - if v := projectAgentResourceVerification(record.Result.Verification); v != nil { - payload.Verification = v - } + } + if v := projectAgentResourceVerification(unifiedresources.CanonicalActionVerification(record)); v != nil { + payload.Verification = v } return payload, true } diff --git a/internal/unifiedresources/store.go b/internal/unifiedresources/store.go index 3c7f56f59..1e7b47f1f 100644 --- a/internal/unifiedresources/store.go +++ b/internal/unifiedresources/store.go @@ -1107,7 +1107,33 @@ func scanActionAuditRecord(scanner actionAuditScanner) (ActionAuditRecord, error record.Request.RequestID = requestID record.Request.ResourceID = CanonicalResourceID(record.Request.ResourceID) _ = actionID - return record, nil + return normalizeActionAuditRecordFromStore(record), nil +} + +func normalizeActionAuditRecordFromStore(record ActionAuditRecord) ActionAuditRecord { + normalized, err := NormalizeActionAuditRecord(record) + if err == nil { + return normalized + } + + if record.Result != nil { + result := *record.Result + result.Output = strings.TrimSpace(result.Output) + result.ErrorMessage = strings.TrimSpace(result.ErrorMessage) + result.Verification = NormalizeActionVerificationResult(result.Verification) + record.Result = &result + } + record.Verification = NormalizeActionVerificationResult(record.Verification) + if record.Verification == nil && record.Result != nil { + record.Verification = cloneActionVerificationResult(record.Result.Verification) + } + if record.Verification != nil && record.Result != nil { + result := *record.Result + result.Verification = cloneActionVerificationResult(record.Verification) + record.Result = &result + } + record.VerificationOutcome = NormalizeVerificationOutcome(record.VerificationOutcome) + return record } func (s *SQLiteResourceStore) GetActionAudit(actionID string) (ActionAuditRecord, bool, error) { diff --git a/internal/unifiedresources/store_test.go b/internal/unifiedresources/store_test.go index 50daf2ad1..c80632bdd 100644 --- a/internal/unifiedresources/store_test.go +++ b/internal/unifiedresources/store_test.go @@ -2,6 +2,7 @@ package unifiedresources import ( "database/sql" + "encoding/json" "errors" "fmt" "os" @@ -426,6 +427,37 @@ func newTestStore(t *testing.T) *SQLiteResourceStore { return store } +func insertRawActionAuditForTest(t *testing.T, store *SQLiteResourceStore, record ActionAuditRecord) { + t.Helper() + requestJSON, err := json.Marshal(record.Request) + if err != nil { + t.Fatalf("marshal request: %v", err) + } + planJSON, err := json.Marshal(record.Plan) + if err != nil { + t.Fatalf("marshal plan: %v", err) + } + approvalsJSON, err := json.Marshal(record.Approvals) + if err != nil { + t.Fatalf("marshal approvals: %v", err) + } + resultJSON, err := json.Marshal(record.Result) + if err != nil { + t.Fatalf("marshal result: %v", err) + } + verificationOutcomeJSON, err := json.Marshal(record.VerificationOutcome) + if err != nil { + t.Fatalf("marshal verification outcome: %v", err) + } + _, err = store.db.Exec(` + INSERT INTO action_audits (id, action_id, canonical_id, request_id, created_at, updated_at, state, request_json, plan_json, approvals_json, result_json, verification_outcome_json) + VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?) + `, record.ID, record.ID, CanonicalResourceID(record.Request.ResourceID), record.Request.RequestID, record.CreatedAt, record.UpdatedAt, string(record.State), string(requestJSON), string(planJSON), string(approvalsJSON), string(resultJSON), string(verificationOutcomeJSON)) + if err != nil { + t.Fatalf("insert raw action audit: %v", err) + } +} + func TestRecordChange_RoundTrip(t *testing.T) { store := newTestStore(t) now := time.Now().UTC().Truncate(time.Second) @@ -1288,11 +1320,115 @@ func TestActionAuditRecord_RoundTrip(t *testing.T) { if verification == nil || !verification.Ran || verification.Command != result.Verification.Command { t.Fatalf("canonical verification round-trip failed: %+v", verification) } + if got.Verification == nil || got.Verification.Command != verification.Command { + t.Fatalf("top-level verification was not restored from sqlite row: top-level=%+v canonical=%+v", got.Verification, verification) + } if got.Result.Verification == nil || got.Result.Verification.Command != verification.Command { t.Fatalf("result verification did not stay aligned with canonical verification: result=%+v canonical=%+v", got.Result.Verification, verification) } } +func TestActionAuditRecord_RoundTripLegacyResultVerificationCanonicalizesSQLiteRead(t *testing.T) { + store := newTestStore(t) + now := time.Date(2026, 3, 18, 14, 0, 0, 0, time.UTC) + + insertRawActionAuditForTest(t, store, ActionAuditRecord{ + ID: "action-legacy-verification", + CreatedAt: now, + UpdatedAt: now.Add(time.Minute), + State: ActionStateCompleted, + Request: ActionRequest{ + RequestID: "req-legacy-verification", + ResourceID: "vm:legacy-verification", + CapabilityName: "restart", + RequestedBy: "agent:test", + }, + Plan: ActionPlan{ + ActionID: "action-legacy-verification", + RequestID: "req-legacy-verification", + Allowed: true, + }, + Result: &ExecutionResult{ + Success: true, + Verification: &ActionVerificationResult{ + Ran: true, + Success: true, + Command: "systemctl is-active nginx", + Output: "active", + RanAt: now.Add(30 * time.Second), + }, + }, + }) + + got, ok, err := store.GetActionAudit("action-legacy-verification") + if err != nil { + t.Fatalf("GetActionAudit: %v", err) + } + if !ok { + t.Fatal("expected raw legacy action audit row") + } + if got.Verification == nil || got.Verification.Command != "systemctl is-active nginx" { + t.Fatalf("legacy result.verification was not restored onto top-level verification: %+v", got.Verification) + } + if got.Result == nil || got.Result.Verification == nil || got.Result.Verification.Command != got.Verification.Command { + t.Fatalf("legacy result verification was not kept aligned: result=%+v canonical=%+v", got.Result, got.Verification) + } +} + +func TestActionAuditRecord_RoundTripMalformedUnrunVerificationScrubsSQLiteRead(t *testing.T) { + store := newTestStore(t) + now := time.Date(2026, 3, 18, 14, 30, 0, 0, time.UTC) + + insertRawActionAuditForTest(t, store, ActionAuditRecord{ + ID: "action-malformed-unrun-verification", + CreatedAt: now, + UpdatedAt: now.Add(time.Minute), + State: ActionStateCompleted, + Request: ActionRequest{ + RequestID: "req-malformed-unrun-verification", + ResourceID: "vm:malformed-unrun-verification", + CapabilityName: "restart", + }, + Plan: ActionPlan{ + ActionID: "action-malformed-unrun-verification", + RequestID: "req-malformed-unrun-verification", + Allowed: true, + }, + Result: &ExecutionResult{ + Success: true, + Verification: &ActionVerificationResult{ + Ran: false, + Success: true, + Command: "should not leak", + Output: "sensitive output", + Note: "sensitive note", + RanAt: now.Add(30 * time.Second), + }, + }, + }) + + results, err := store.GetActionAudits("vm:malformed-unrun-verification", now.Add(-time.Hour), 10) + if err != nil { + t.Fatalf("GetActionAudits: %v", err) + } + if len(results) != 1 { + t.Fatalf("expected 1 raw malformed action audit row, got %d", len(results)) + } + got := results[0] + if got.Verification == nil || got.Verification.Ran { + t.Fatalf("expected sanitized ran=false verification, got %+v", got.Verification) + } + if got.Verification.Command != "" || got.Verification.Output != "" || got.Verification.Note != "" || !got.Verification.RanAt.IsZero() || got.Verification.Success { + t.Fatalf("top-level ran=false verification leaked details: %+v", got.Verification) + } + if got.Result == nil || got.Result.Verification == nil { + t.Fatalf("expected result verification to remain present and aligned: %+v", got.Result) + } + if got.Result.Verification.Command != "" || got.Result.Verification.Output != "" || got.Result.Verification.Note != "" || !got.Result.Verification.RanAt.IsZero() || got.Result.Verification.Success { + t.Fatalf("result ran=false verification leaked details: %+v", got.Result.Verification) + } +} + func TestMemoryStore_RecordActionAudit_UpsertsByID(t *testing.T) { store := NewMemoryStore() now := time.Date(2026, 3, 18, 13, 30, 0, 0, time.UTC)