Pre-existing bug exposed by slice 33: the contract test had two
filepath.Join(".", "actions.go") entries in its requiredSnippets map.
Go silently drops duplicate map keys (last entry wins), so the first
list — including the canonical ErrActionPlanDrift pin and the new
ErrResourceRemediationLocked pin from slice 33 — was never actually
running. The test reported PASS while pinning nothing it claimed to.
Merges both lists into a single actions.go entry so every snippet
runs. Also corrects ErrActionPlanDrift's expected alignment to the
single-space form gofmt produced when the var block grew the new
sentinel.
Third compounding slice on per-resource operator state, completing the
suppression triangle: maintenance window (slice 31), intentionally
offline (slice 32), and now NeverAutoRemediate at the action-broker
boundary. When the operator has set NeverAutoRemediate=true on a
resource, executeCommandWithAudit refuses the dispatch even with a
valid approval and matching plan hash — per-resource operator intent
outranks per-action approval.
The check sits next to the drift refusal in the same audit hot path:
fail-open on store-lookup error (logged but doesn't block), persist a
Failed audit record with `resource_remediation_locked:` prefix on the
ErrorMessage so audit-UI filters and alert rules can branch on the
stable token. Returns ErrResourceRemediationLocked sentinel for
caller-side branching.
Reuses the existing actionAuditStore handle (which is the unified
ResourceStore from slice 29), so no new provider plumbing needed —
the broker already has the data path it needs.
Closes the read-after-write piece of the user's act-with-control loop:
"Act with control: ask for approval, execute through Pulse tools, then
verify." Until now Pulse authored verification narrative copy at
approval time but never actually executed the check.
Adds ActionVerificationResult to internal/unifiedresources/actions.go
and embeds it as ExecutionResult.Verification so the existing
result_json column persists the outcome without a schema migration.
The struct records: did the check run, what command did Pulse send,
what came back, did it succeed, when, and any failure note.
Adds VerificationCommandForCommand in tools_control.go that derives
the per-class read-after-write check used by the broker:
- service-restart / service-start / service-reload / service-stop
→ systemctl is-active <unit>
Container classes (container-restart, container-stop) are
intentionally deferred to pulse_docker's existing tool-level
docker inspect verification — adding a broker-level dispatch would
double-run the same check.
executeCommandWithAudit now runs the derived verification command via
the same agent path immediately after a successful dispatch, captures
output and exit code, and writes the result onto
ExecutionResult.Verification before recordActionExecutionResult
persists. If the check returns non-zero, Verification.Success=false
with a Note explaining the exit code so the audit history honestly
shows that Pulse ran the action but couldn't confirm it took.
Verification is best-effort and class-scoped: unknown command shapes
leave Verification nil rather than fabricating a verified=true entry,
matching the no-fabrication boundary from Impact / preflight authoring.
Tests cover:
- VerificationCommandForCommand derives the right check per class,
shell-escapes single quotes in unit names, and returns "", false
for docker (deferred) and unknown commands.
- TestExecuteCommandWithAuditRunsClassDerivedVerificationAfterDispatch
- TestExecuteCommandWithAuditMarksVerificationFailedWhenReadbackDoesNotConfirm
- TestExecuteCommandWithAuditSkipsVerificationForUnclassifiedCommands
Updates ai-runtime contract with the read-after-write rule and
container-class deferral. Adds Completion Obligation #21 to
unified-resources pinning ExecutionResult.Verification shape and the
no-fabrication boundary. Extends TestActionExecutionContractStaysAPIOwned
to pin ActionVerificationResult struct and the Verification field on
ExecutionResult.
The action audit log is plaintext SQL. Operators sometimes paste
secrets into a natural-language `reason` field ("rotate the key
sk-abc123 because it leaked"), and command output sometimes echoes
tokens. Both flows previously persisted unredacted, leaving the
audit history as a leaked-credential surface.
Adds RedactAuditText / RedactAuditRecord in a new
internal/unifiedresources/audit_redaction.go with a curated regex set
that targets the credential shapes most likely to be real secrets:
- URL with embedded basic-auth credentials (https://user:pass@host)
- Authorization: Bearer <token> and x-api-key: <token> headers
- Query-string secret params (?api_key=, &token=, &access_token=)
- JSON-style secret fields ("api_key": "...", "password": "...")
- Env-style or CLI-style secret assignments (PASSWORD=, api_key=)
- OpenAI/Anthropic-style API keys (sk-...)
The set is intentionally narrower than the patrol-failure redactor in
internal/ai/patrol_runtime_failure.go: it does NOT strip arbitrary
URLs, because operators legitimately reference runbooks, ticket
links, and GitHub issues in audit reasons. Only patterns very likely
to be real secrets are touched.
Wired at the persistence boundary (top of each store method) so all
record paths get redaction uniformly:
- SQLiteResourceStore.RecordActionAudit
- SQLiteResourceStore.RecordActionExecutionStart
- SQLiteResourceStore.RecordActionExecutionResult
- MemoryStore.RecordActionAudit
- MemoryStore.RecordActionExecutionStart
- MemoryStore.RecordActionExecutionResult
Plan, Approvals, and identity fields are left alone — they are
produced by Pulse, not operators or external command output, so they
do not need redaction (and changing them would break PlanHash drift
detection).
Verification artifacts:
- audit_redaction_test.go: pattern-by-pattern coverage plus public-URL
passthrough and empty-string passthrough; full RedactAuditRecord
shape test (Reason + Params string values + Result output redacted;
non-string Params and Plan untouched).
- code_standards_test.go: pins RedactAuditRecord call sites in
store.go and helper signatures in audit_redaction.go so future
refactors cannot silently bypass redaction.
- registry_test.go: cross-cutting integration test that exercises the
registry-store boundary (MemoryStore.RecordActionAudit) and asserts
redaction is applied while Plan fields stay untouched.
Adds new Completion Obligation #20 to unified-resources contract
pinning the redaction-at-persistence-boundary rule and the
no-touch-Plan-fields invariant.
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.
PlanHash existed on ActionPlan as the contract for "the operator
approved exactly this (command, target, reason) combination" but the
broker never validated it: at execute time the freshly-recomputed hash
was overwritten by the approved plan's hash via mergeApprovedActionPlan
without comparison. A drifted payload (LLM re-emits with different
command, agent ID changes between approval and execute, malicious
injection) would run under a stale approval.
Fix: at the dispatch boundary in executeCommandWithAudit, recompute the
approval-equivalent hash from the actual payload using approvalPlanHash
(same function used at approval-creation time, so direct comparison is
meaningful), compare to plan.PlanHash, and refuse with a new
ErrActionPlanDrift error when they differ. The drift refusal also logs
at WARN level with action_id, approval_id, and both hashes so audit
review can see when drift was caught.
When approvedHash is empty (older approval records or contract paths
that did not author one), validation is skipped and existing behavior
is preserved.
Adds two tests:
- TestExecuteCommandWithAuditRefusesPayloadDriftAgainstApprovedPlan:
approval is for "systemctl restart workload"; payload at execute
time is "rm -rf /var/log/pulse"; expects ErrActionPlanDrift and no
agent dispatch.
- TestExecuteCommandWithAuditAllowsMatchingPlanHash: same payload
matches the approved hash; expects normal dispatch.
Updates two pre-existing tests that used stub PlanHash strings:
- TestExecuteCommandWithDeniedApprovalDoesNotDispatch: was testing the
denial path with PlanHash:"sha256:test". Now uses the real approval-
equivalent hash so denial fires (not drift) and the test still
isolates the denial behavior.
- ControlledConsumesApprovedCommandWithResolvedRoutingTarget: same fix
pattern.
Extends TestActionExecutionContractStaysAPIOwned in code_standards
test to pin ErrActionPlanDrift's existence in actions.go so future
refactors cannot silently downgrade drift into a generic error kind.
Native action path (executeNativeActionWithAudit) is left for a
follow-up: it has the same drift gap but uses actionPlanHashForParams
which is shaped differently from approvalPlanHash, so a sound fix
needs a coherent canonical hash function rather than just adding the
check. Contracts pinned in ai-runtime and unified-resources (new
Completion Obligation #20).
rcourtman