From 1aaa8cad00c83ab84bec0317dcd3ebb4af9b988a Mon Sep 17 00:00:00 2001 From: Roland Rodriguez Date: Wed, 27 May 2026 20:13:03 -0500 Subject: [PATCH] fix(audit): warn when storage parser falls back to Custom for framework namespace MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit All four storage backends' AuditEventKind parsers ended with a Custom catch-all that quietly wrapped any unknown event-kind string. For genuinely user-defined custom events this is correct, but for the framework-owned namespaces (auth.*, http.*, account.*, config.*) it silently swallows version-skew bugs — a downstream emitter on a newer acton-service writes a new kind, an older reader returns Custom("auth.token.missing"), and any consumer matching on the typed variant misses the event with no signal. Factor the catch-all logic into pub(crate) helpers in audit/storage/mod.rs: - looks_like_framework_kind(s) — true for strings beginning with one of the four framework-owned prefixes. - parse_custom_kind(s) — strips the "custom." prefix (preserving the prior round-trip behavior for user-defined kinds) and emits a tracing::warn! when looks_like_framework_kind is true, naming the exact stored string so operators can grep for it. All four parser catch-alls route through parse_custom_kind. Add unit tests covering both helpers across the framework-namespace, custom- namespace, and bare-user-string cases. Closes #20 --- .../src/audit/storage/clickhouse_impl.rs | 5 +- acton-service/src/audit/storage/mod.rs | 66 +++++++++++++++++++ acton-service/src/audit/storage/pg.rs | 5 +- .../src/audit/storage/surrealdb_impl.rs | 5 +- acton-service/src/audit/storage/turso.rs | 5 +- 5 files changed, 70 insertions(+), 16 deletions(-) diff --git a/acton-service/src/audit/storage/clickhouse_impl.rs b/acton-service/src/audit/storage/clickhouse_impl.rs index 800af5a..826f3dd 100644 --- a/acton-service/src/audit/storage/clickhouse_impl.rs +++ b/acton-service/src/audit/storage/clickhouse_impl.rs @@ -176,10 +176,7 @@ impl From for AuditEvent { "config.drift_detected" => AuditEventKind::ConfigDriftDetected, "http.request" => AuditEventKind::HttpRequest, "http.request.denied" => AuditEventKind::HttpRequestDenied, - other => { - let name = other.strip_prefix("custom.").unwrap_or(other); - AuditEventKind::Custom(name.to_string()) - } + other => AuditEventKind::Custom(super::parse_custom_kind(other)), }; let severity = match row.severity { diff --git a/acton-service/src/audit/storage/mod.rs b/acton-service/src/audit/storage/mod.rs index a31e812..b22490d 100644 --- a/acton-service/src/audit/storage/mod.rs +++ b/acton-service/src/audit/storage/mod.rs @@ -28,6 +28,72 @@ pub mod surrealdb_impl; #[cfg(feature = "clickhouse")] pub mod clickhouse_impl; +/// Returns true if the stored event-kind string looks like a framework-owned +/// kind (`auth.*`, `http.*`, `account.*`, `config.*`) that should have been +/// recognized by the parser. Used by parser catch-alls to detect likely +/// version skew between an emitter and a reader. +pub(crate) fn looks_like_framework_kind(s: &str) -> bool { + s.starts_with("auth.") + || s.starts_with("http.") + || s.starts_with("account.") + || s.starts_with("config.") +} + +/// Helper for storage-backend parser catch-alls. +/// +/// Strips the `custom.` prefix when present (so user-defined custom events +/// round-trip cleanly) and emits a `tracing::warn!` when the input looks +/// like a framework-owned kind that no parser arm matched — i.e. the +/// emitter is on a newer version than this reader. +pub(crate) fn parse_custom_kind(s: &str) -> String { + if looks_like_framework_kind(s) { + tracing::warn!( + stored_kind = %s, + "unrecognized framework audit event kind — falling back to Custom; likely version skew between emitter and reader" + ); + } + s.strip_prefix("custom.").unwrap_or(s).to_string() +} + +#[cfg(test)] +mod helper_tests { + use super::{looks_like_framework_kind, parse_custom_kind}; + + #[test] + fn framework_prefixes_detected() { + assert!(looks_like_framework_kind("auth.token.invalid")); + assert!(looks_like_framework_kind("http.request.denied")); + assert!(looks_like_framework_kind("account.created")); + assert!(looks_like_framework_kind("config.drift_detected")); + } + + #[test] + fn non_framework_prefixes_ignored() { + assert!(!looks_like_framework_kind("custom.user.exported")); + assert!(!looks_like_framework_kind("user.signed_up")); + assert!(!looks_like_framework_kind("billing.invoice.paid")); + assert!(!looks_like_framework_kind("")); + } + + #[test] + fn parse_custom_strips_custom_prefix() { + assert_eq!(parse_custom_kind("custom.user.exported"), "user.exported"); + } + + #[test] + fn parse_custom_preserves_unprefixed_user_strings() { + assert_eq!(parse_custom_kind("billing.invoice.paid"), "billing.invoice.paid"); + } + + #[test] + fn parse_custom_passes_through_framework_strings_for_visibility() { + // The warn fires (verified manually / via tracing subscribers in + // integration tests); we assert the returned string preserves the + // original so operators can grep for it in their event store. + assert_eq!(parse_custom_kind("auth.token.invalid"), "auth.token.invalid"); + } +} + /// Trait for audit event persistence backends /// /// Implementations MUST enforce append-only semantics at the database level diff --git a/acton-service/src/audit/storage/pg.rs b/acton-service/src/audit/storage/pg.rs index 5e25cae..cf6180c 100644 --- a/acton-service/src/audit/storage/pg.rs +++ b/acton-service/src/audit/storage/pg.rs @@ -305,10 +305,7 @@ impl From for AuditEvent { "config.drift_detected" => AuditEventKind::ConfigDriftDetected, "http.request" => AuditEventKind::HttpRequest, "http.request.denied" => AuditEventKind::HttpRequestDenied, - other => { - let name = other.strip_prefix("custom.").unwrap_or(other); - AuditEventKind::Custom(name.to_string()) - } + other => AuditEventKind::Custom(super::parse_custom_kind(other)), }; let severity = match row.severity { diff --git a/acton-service/src/audit/storage/surrealdb_impl.rs b/acton-service/src/audit/storage/surrealdb_impl.rs index d6f5e06..981aed5 100644 --- a/acton-service/src/audit/storage/surrealdb_impl.rs +++ b/acton-service/src/audit/storage/surrealdb_impl.rs @@ -406,10 +406,7 @@ fn parse_event_kind(s: &str) -> AuditEventKind { "config.drift_detected" => AuditEventKind::ConfigDriftDetected, "http.request" => AuditEventKind::HttpRequest, "http.request.denied" => AuditEventKind::HttpRequestDenied, - other => { - let name = other.strip_prefix("custom.").unwrap_or(other); - AuditEventKind::Custom(name.to_string()) - } + other => AuditEventKind::Custom(super::parse_custom_kind(other)), } } diff --git a/acton-service/src/audit/storage/turso.rs b/acton-service/src/audit/storage/turso.rs index 896d01a..352e18c 100644 --- a/acton-service/src/audit/storage/turso.rs +++ b/acton-service/src/audit/storage/turso.rs @@ -398,10 +398,7 @@ fn parse_event_kind(s: &str) -> AuditEventKind { "config.drift_detected" => AuditEventKind::ConfigDriftDetected, "http.request" => AuditEventKind::HttpRequest, "http.request.denied" => AuditEventKind::HttpRequestDenied, - other => { - let name = other.strip_prefix("custom.").unwrap_or(other); - AuditEventKind::Custom(name.to_string()) - } + other => AuditEventKind::Custom(super::parse_custom_kind(other)), } }