fix: make audit log an explicit operator convenience buffer - #233
Open
martinshub-tech wants to merge 1 commit into
Open
fix: make audit log an explicit operator convenience buffer#233martinshub-tech wants to merge 1 commit into
martinshub-tech wants to merge 1 commit into
Conversation
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 #224
🎯 Design Decision: Operator Convenience Buffer
This PR explicitly designates the in-memory log as an operator convenience buffer, rather than a genuine audit trail.
Reasoning: The application currently lacks a database or external persistence layer. Introducing durable storage (like a database table or structured external log shipping) solely for this feature would significantly increase architectural complexity and external dependencies. Treating this as a convenience buffer preserves its utility as a lightweight debugging aid without promising false guarantees.
👁️ Eviction Visibility
Silent eviction under load undermines the buffer's utility.
Change: We have added an evictedCount to BoundedHistory which is now exposed in the GET /api/v1/audit response.
Result: Operators can now observe exactly how many entries have been dropped since startup due to the rolling limit, removing the silent failure mode. We have also updated the OpenAPI documentation to unambiguously reflect these guarantees.
🛡️ redactSensitiveData Coverage & Gaps
Current Coverage: The denylist correctly captures exact, case-insensitive matches for common credential keys like x-api-key, authorization, password, secret, token, bearer, and private_key. It traverses nested objects well.
Identified Gaps:
The denylist requires an exact match. For example, api-key and apikey are redacted, but api_key (with an underscore) is not.
Domain-specific sensitive fields like credentials, cert, signature, mac, stripe_key or PII are not covered.
Query parameters containing secrets (though stripped from the req.path by Express routing logic) wouldn't be redacted if they inadvertently ended up in the body or headers under non-standard keys.
Since this remains a volatile in-memory buffer, these redaction gaps are less critical than they would be if the logs were durably persisted.
🧪 Testing
Added a test in auditLog.test.ts to prove the previous silent eviction behavior now successfully exposes the evictedCount.
All tests, linters, and builds pass.