fix(audit): warn when storage parser falls back to Custom for framework namespace#24
Merged
Merged
Conversation
…rk namespace
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
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
All four audit storage backends ended
AuditEventKindreconstruction with aCustomcatch-all that silently wrapped any unknown event-kind string. For genuinely user-defined kinds this is correct, but for framework-owned namespaces (auth.*,http.*,account.*,config.*) it swallows version-skew bugs without signal: a downstream emitter on a newer acton-service writes a new kind, an older reader returnsCustom("auth.token.missing"), and any Rust consumer pattern-matching on the typed variant misses the event.This PR factors the catch-all logic into two
pub(crate)helpers inaudit/storage/mod.rs:looks_like_framework_kind(s)— true if the stored string begins with one of the four framework-owned prefixes.parse_custom_kind(s)— strips thecustom.prefix (preserving prior round-trip behavior for user-defined kinds) and emits atracing::warn!naming the exact stored string whenlooks_like_framework_kindis true.All four parser catch-alls route through
parse_custom_kind. The warn fires once per parsed event, names the offending string, and is structured (stored_kind = %s), so operators can pipe it into their existing log-aggregation alerts.Closes #20.
Behavior
custom.user.exported_data,billing.invoice.paid): unchanged. Stripcustom.prefix when present, no warn.auth.login.success, etc.): unchanged. Match the typed arm before the catch-all.auth.token.missingfrom an older reader, or any future addition): now emittracing::warn!and returnCustom(name)as before — the value-shape is unchanged, only observability improves.Test plan
Five new unit tests in
audit::storage::helper_testscover:framework_prefixes_detected— all four prefixes detected.non_framework_prefixes_ignored—custom., bare user strings, empty string.parse_custom_strips_custom_prefix— preserves user-defined custom round-trip.parse_custom_preserves_unprefixed_user_strings— unrecognized non-framework strings pass through.parse_custom_passes_through_framework_strings_for_visibility— framework-prefixed unknowns preserve their string (warn is verified via manual tracing-subscriber inspection; a tracing-test dep is overkill here).All five pass under
--features "full,audit". The full--features fullCI run also passes clean (audit module isn't compiled there, but the helpers compile fine and clippy is green under both feature sets).cargo clippy -p acton-service --all-targets --features full -- -D warnings— cleancargo clippy -p acton-service --all-targets --no-default-features --features "full,crypto-ring" -- -D warnings— cleancargo nextest run -p acton-service --features full— 520/520 passcargo nextest run -p acton-service --features "full,audit"— 5 new helper tests pass (one pre-existing extensions test fails on origin/main under this feature combo; unrelated to this PR)Note for follow-up
The
--features fullCI matrix does NOT includeaudit, so any test that needs the audit feature (mine, and the existing ClickHouse roundtrip test) doesn't run in CI. Worth a separate CI matrix entry to cover audit code paths — would have caught the missing parser arms in #15 earlier.