Repository navigation
fix(api): guard the audit action/entityType vocabulary against drift - #439
Merged
Merged
Conversation
…258) Root-cause follow-up to #247, which fixed the symptom (the SPA's AUDIT_ACTION_VALUES/ENTITY_TYPE_VALUES had fallen behind the server) without adding anything to stop it recurring. Hoists every audit.WriteAsync(...) action string literal into a new AuditActions constants class (mirrors #231's validator-errorCode registry), and adds AuditVocabularyCoverageTests: - Every audit.WriteAsync( call site under src/ is walked (found by scanning every .cs file, not a maintained project list) and its action argument is asserted to be a direct AuditActions.X reference or a two-branch ternary of them -- never an inline literal, with one explicit, line-pinned exception for IdentityProvider's ResetPasswordAndRevokeAsync (its action is a parameter because it has two callers, each of which now passes an AuditActions member). - AuditActions' reflected fields are asserted to be a 1:1 match with web/src/i18n/enums.ts's AUDIT_ACTION_VALUES. - The observed entityType arguments (nameof(Type) or a bare literal at every call site -- no indirection exists for this dimension today) are asserted to be a 1:1 match with ENTITY_TYPE_VALUES. Mutation-checked: a reverted call site, an extra registry entry, a bogus entityType literal, and a removed exception-list entry each independently make the guard fail with a clear message; all four restored to green. A false-positive hit during development is worth recording -- the scanner's own regex initially matched a mention of "audit.WriteAsync(" inside this file's doc comments, so call sites are now found against a comment-stripped view rather than trusting that phrase never appears in prose again. No current drift: the 32 actions and 14 entity types found in source already matched the client lists exactly.
Owner
Author
|
@codex review Generated by Claude Code |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 0d3999d20d
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
- Match every IAuditWriter-typed receiver, not just the literal `audit`
variable name, when scanning for .WriteAsync(...) call sites — a
differently-named injection would previously slip past the guard
undetected. Mutation-tested by renaming a handler's field and reverting
its call to a raw literal; confirmed caught, then reverted.
- Validate every actual caller of IdentityProvider's
ResetPasswordAndRevokeAsync (whose action reaches WriteAsync through a
parameter) instead of trusting the exemption's original manual review of
its two callers. Distinguishes the method's own declaration from a real
call site by checking whether '{' follows the argument list.
Mutation-tested with a fake third caller passing a raw string literal;
confirmed caught, then reverted.
- Strip comments before parsing the SPA's AUDIT_ACTION_VALUES /
ENTITY_TYPE_VALUES arrays, so commenting out a removed action doesn't
still read as present. Mutation-tested by commenting out an entry;
confirmed the guard reported the drift, then reverted.
Owner
Author
|
@codex address that feedback Generated by Claude Code |
|
Summary
Testing
|
This was referenced Aug 7, 2026
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.
Closes #258.
Problem
The SPA's audit-log action filter and entity-type labels are driven by two hand-maintained lists in
web/src/i18n/enums.ts(AUDIT_ACTION_VALUES,ENTITY_TYPE_VALUES). Nothing kept them in sync with what the server actually emits — #247 was the symptom (the client list had fallen 3 codes behind), caught by manual review, not a gate.Fix
Actions — hoisted every inline action string literal at every
audit.WriteAsync(...)call site into a newAuditActionsconstants class (same shape as #231's validator-errorCode registry), so the vocabulary is reflectable rather than re-derived by grepping call sites — which can't work in general, sinceIdentityProvider.ResetPasswordAndRevokeAsynctakes its action as a parameter (two callers, two different actions), so the literal doesn't live at theWriteAsynccall itself.Entity types — no such indirection exists today (every call site is
nameof(Type)or a bare literal), soAuditVocabularyCoverageTestswalks everyaudit.WriteAsync(...)call site directly (found by scanning every.csfile undersrc/, not a maintained project list).Three guard facts:
AuditActions.Xreference or a two-branch ternary of them — never an inline literal — with one explicit, line-pinned exception for theResetPasswordAndRevokeAsyncindirection (moving that call even one line, or a new dynamic call site anywhere, fails the test).AuditActions's reflected fields are a 1:1 match withAUDIT_ACTION_VALUES.entityTypevalues are a 1:1 match withENTITY_TYPE_VALUES.No current drift — the 32 actions and 14 entity types found in source already matched the client lists exactly; this is prevention, not a fix to an existing gap.
Verification
Cluckwork.Application.Testsgreen (114 existing + 3 new), 303Cluckwork.Domain.Testsgreen, 37 non-DockerCluckwork.Api.IntegrationTestsgreen (this environment has no Docker for the Testcontainers-backed suite — relies on CI).AuditActionsfield, a bogusentityTypeliteral, and a removed exception-list entry each independently made the relevant guard fail with a clear message; all four restored to green.audit.WriteAsync(inside this file's own doc comments (a real false positive, caught immediately). Fixed by matching against a comment-stripped view rather than relying on that exact phrase never appearing in prose again.Docs
No GLOSSARY/Help-page change — internal audit-vocabulary tooling, zero product-facing behavior change (no
web/files touched, no audit action/entityType renamed).Generated by Claude Code