fix: redact secrets from audit log descriptions#103
Open
christianromeni wants to merge 1 commit into
Open
Conversation
The audit middleware persisted admin API request bodies into audit_logs.description, leaking passwords, upstream API keys, OAuth secrets, auth tokens and license keys in cleartext. Any org_admin with audit-log read access could see them. buildDescription now replaces the values of sensitive fields with [REDACTED] recursively (nested objects and arrays included), keyed by a case-insensitive field-name match so variants like "Password" cannot slip through. Non-sensitive fields keep the existing drop behaviour. Migration 0013 clears description on existing rows that may already contain secrets. Affected secrets should be rotated.
✅ Snyk checks have passed. No issues have been found so far.
💻 Catch issues earlier using the plugins for VS Code, JetBrains IDEs, Visual Studio, and Eclipse. |
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
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.
Problem
The audit middleware wrote admin API request bodies into
audit_logs.description, which leaked secrets in cleartext: user passwords, upstream provider API keys, MCPauth_token/oauth_client_secret, SSOclient_secret, and license keys. Anyorg_adminwith audit-log read access could read them.Fix
buildDescriptionnow replaces the values of sensitive fields with[REDACTED]instead of persisting them, recursively through nested objects and arrays of objects.strings.ToLower), because Go unmarshals JSON into the request structs case-insensitively - so{"Password": "..."}would otherwise be processed by the handler but slip past redaction.password,api_key,auth_token,oauth_client_secret,client_secret,key,token.Existing data
Migration
0013_redact_audit_log_secretsclearsdescriptionon existing rows whose value contains a known sensitive key (conservative LIKE match, SQLite + PostgreSQL portable, idempotent). The down migration is an intentional no-op - the original values are gone and must not be restored. Operators should rotate any secrets that were entered via the admin API before this fix.Tests
buildDescription: top-level + nested + array redaction, case-insensitive variants (Password,API_KEY,Client_Secret), sensitive-field-with-object-value, exact-match negatives (max_tokensnot touched), unchanged drop behaviour.description.go test -race ./internal/audit/ ./internal/db/ ./internal/api/...clean.Note: the login endpoint never went through this path (it builds its own description from the email only) - verified, no password leak there.