Skip to content

Commit 52750d4

Browse files
fix: redact secrets from audit log descriptions
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.
1 parent 57900ae commit 52750d4

5 files changed

Lines changed: 727 additions & 10 deletions

File tree

Lines changed: 331 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,331 @@
1+
package audit
2+
3+
// White-box unit tests for buildDescription and its helpers. These live in
4+
// package audit (not audit_test) so that the unexported buildDescription,
5+
// redactMap, and redactSlice functions are directly accessible.
6+
7+
import (
8+
"strings"
9+
"testing"
10+
)
11+
12+
// TestBuildDescription covers the full contract of buildDescription:
13+
// sensitive-field redaction, zero-value dropping, recursive descent, and
14+
// fallback behaviour for invalid input.
15+
func TestBuildDescription(t *testing.T) {
16+
t.Parallel()
17+
18+
tests := []struct {
19+
name string
20+
body string
21+
// checks is a list of assertions expressed as functions so each test case
22+
// can assert the combination of contains/not-contains that matters to it.
23+
checks []func(t *testing.T, got string)
24+
}{
25+
// ------------------------------------------------------------------
26+
// Top-level sensitive field: password
27+
// ------------------------------------------------------------------
28+
{
29+
name: "top-level password is redacted",
30+
body: `{"email":"a@b.c","password":"secret123"}`,
31+
checks: []func(t *testing.T, got string){
32+
containsStr(`"email":"a@b.c"`),
33+
containsStr(`"[REDACTED]"`),
34+
containsStr(`"password"`),
35+
notContainsStr("secret123"),
36+
},
37+
},
38+
// ------------------------------------------------------------------
39+
// All sensitive field names — one subtest per field
40+
// ------------------------------------------------------------------
41+
{
42+
name: "api_key is redacted",
43+
body: `{"api_key":"sk-live-abc123","name":"my-model"}`,
44+
checks: []func(t *testing.T, got string){
45+
containsStr(`"api_key"`),
46+
containsStr(`"[REDACTED]"`),
47+
notContainsStr("sk-live-abc123"),
48+
containsStr(`"name":"my-model"`),
49+
},
50+
},
51+
{
52+
name: "auth_token is redacted",
53+
body: `{"auth_token":"tok-xyz","url":"https://example.com"}`,
54+
checks: []func(t *testing.T, got string){
55+
containsStr(`"auth_token"`),
56+
containsStr(`"[REDACTED]"`),
57+
notContainsStr("tok-xyz"),
58+
},
59+
},
60+
{
61+
name: "oauth_client_secret is redacted",
62+
body: `{"oauth_client_secret":"oauth-secret-99","client_id":"client-1"}`,
63+
checks: []func(t *testing.T, got string){
64+
containsStr(`"oauth_client_secret"`),
65+
containsStr(`"[REDACTED]"`),
66+
notContainsStr("oauth-secret-99"),
67+
containsStr(`"client_id":"client-1"`),
68+
},
69+
},
70+
{
71+
name: "client_secret is redacted",
72+
body: `{"client_secret":"cs-top-secret","issuer":"https://idp.example"}`,
73+
checks: []func(t *testing.T, got string){
74+
containsStr(`"client_secret"`),
75+
containsStr(`"[REDACTED]"`),
76+
notContainsStr("cs-top-secret"),
77+
},
78+
},
79+
{
80+
name: "key is redacted",
81+
body: `{"key":"vl-license-jwt-xxx","seat_count":5}`,
82+
checks: []func(t *testing.T, got string){
83+
containsStr(`"key"`),
84+
containsStr(`"[REDACTED]"`),
85+
notContainsStr("vl-license-jwt-xxx"),
86+
containsStr(`"seat_count"`),
87+
},
88+
},
89+
// ------------------------------------------------------------------
90+
// Nested object: sensitive field inside nested config object
91+
// ------------------------------------------------------------------
92+
{
93+
name: "api_key nested inside config object is redacted",
94+
body: `{"config":{"api_key":"sk-live-123","name":"x"}}`,
95+
checks: []func(t *testing.T, got string){
96+
containsStr(`"api_key"`),
97+
containsStr(`"[REDACTED]"`),
98+
notContainsStr("sk-live-123"),
99+
containsStr(`"name":"x"`),
100+
},
101+
},
102+
// ------------------------------------------------------------------
103+
// Array of objects: sensitive field inside each array element
104+
// ------------------------------------------------------------------
105+
{
106+
name: "auth_token inside array of objects is redacted for all elements",
107+
body: `{"servers":[{"auth_token":"tok-1","host":"h1"},{"auth_token":"tok-2","host":"h2"}]}`,
108+
checks: []func(t *testing.T, got string){
109+
containsStr(`"[REDACTED]"`),
110+
notContainsStr("tok-1"),
111+
notContainsStr("tok-2"),
112+
containsStr(`"host":"h1"`),
113+
containsStr(`"host":"h2"`),
114+
},
115+
},
116+
// ------------------------------------------------------------------
117+
// Negative test: "token"-containing field names that are NOT sensitive
118+
// ------------------------------------------------------------------
119+
{
120+
name: "max_tokens and token_count are not redacted (no substring match)",
121+
body: `{"max_tokens":100,"token_count":5,"prompt":"hello"}`,
122+
checks: []func(t *testing.T, got string){
123+
notContainsStr(`"[REDACTED]"`),
124+
containsStr(`"max_tokens"`),
125+
containsStr(`"token_count"`),
126+
// "prompt" is a non-zero string so it is included too
127+
containsStr(`"prompt":"hello"`),
128+
},
129+
},
130+
// ------------------------------------------------------------------
131+
// Drop behaviour: zero/empty/null/false values for non-sensitive fields
132+
// ------------------------------------------------------------------
133+
{
134+
name: "zero-value non-sensitive fields are dropped",
135+
body: `{"name":"","count":0,"flag":false,"x":null,"kept":"value"}`,
136+
checks: []func(t *testing.T, got string){
137+
notContainsStr(`"name"`),
138+
notContainsStr(`"count"`),
139+
notContainsStr(`"flag"`),
140+
notContainsStr(`"x"`),
141+
containsStr(`"kept":"value"`),
142+
},
143+
},
144+
// ------------------------------------------------------------------
145+
// Sensitive field with empty value: document the actual behaviour.
146+
// The implementation always emits sensitive fields as "[REDACTED]"
147+
// regardless of their value, including the empty string.
148+
// ------------------------------------------------------------------
149+
{
150+
name: "password with empty string value is emitted as [REDACTED]",
151+
body: `{"password":""}`,
152+
checks: []func(t *testing.T, got string){
153+
containsStr(`"password"`),
154+
containsStr(`"[REDACTED]"`),
155+
},
156+
},
157+
// ------------------------------------------------------------------
158+
// Invalid JSON: returns empty string (existing fallback behaviour)
159+
// ------------------------------------------------------------------
160+
{
161+
name: "invalid JSON returns empty string",
162+
body: `{not valid json`,
163+
checks: []func(t *testing.T, got string){
164+
func(t *testing.T, got string) {
165+
t.Helper()
166+
if got != "" {
167+
t.Errorf("buildDescription(%q) = %q, want empty string for invalid JSON", `{not valid json`, got)
168+
}
169+
},
170+
},
171+
},
172+
// ------------------------------------------------------------------
173+
// Empty body: returns empty string
174+
// ------------------------------------------------------------------
175+
{
176+
name: "empty body returns empty string",
177+
body: ``,
178+
checks: []func(t *testing.T, got string){
179+
func(t *testing.T, got string) {
180+
t.Helper()
181+
if got != "" {
182+
t.Errorf("buildDescription(empty) = %q, want empty string", got)
183+
}
184+
},
185+
},
186+
},
187+
// ------------------------------------------------------------------
188+
// Body that is all-zero after dropping: returns empty string
189+
// ------------------------------------------------------------------
190+
{
191+
name: "body with only zero-value fields returns empty string",
192+
body: `{"name":"","count":0,"active":false,"data":null}`,
193+
checks: []func(t *testing.T, got string){
194+
func(t *testing.T, got string) {
195+
t.Helper()
196+
if got != "" {
197+
t.Errorf("buildDescription(all-zero) = %q, want empty string", got)
198+
}
199+
},
200+
},
201+
},
202+
// ------------------------------------------------------------------
203+
// Multiple sensitive fields at once: every one is redacted
204+
// ------------------------------------------------------------------
205+
{
206+
name: "multiple sensitive fields in same body are all redacted",
207+
body: `{"password":"p","api_key":"k","name":"bob"}`,
208+
checks: []func(t *testing.T, got string){
209+
notContainsStr(`"p"`),
210+
notContainsStr(`"k"`),
211+
containsStr(`"password"`),
212+
containsStr(`"api_key"`),
213+
containsStr(`"name":"bob"`),
214+
},
215+
},
216+
// ------------------------------------------------------------------
217+
// Deeply nested: sensitive field two levels deep
218+
// ------------------------------------------------------------------
219+
{
220+
name: "client_secret two levels deep is redacted",
221+
body: `{"sso":{"provider":"google","credentials":{"client_secret":"deep-secret","client_id":"cid-1"}}}`,
222+
checks: []func(t *testing.T, got string){
223+
containsStr(`"client_secret"`),
224+
containsStr(`"[REDACTED]"`),
225+
notContainsStr("deep-secret"),
226+
containsStr(`"client_id":"cid-1"`),
227+
},
228+
},
229+
// ------------------------------------------------------------------
230+
// Array of scalars: scalars pass through unchanged
231+
// ------------------------------------------------------------------
232+
{
233+
name: "array of scalars passes through without redaction",
234+
body: `{"tags":["alpha","beta"],"active":true}`,
235+
checks: []func(t *testing.T, got string){
236+
containsStr(`"tags"`),
237+
containsStr("alpha"),
238+
containsStr("beta"),
239+
notContainsStr(`"[REDACTED]"`),
240+
},
241+
},
242+
// ------------------------------------------------------------------
243+
// Fix 1 – Case-insensitive redaction
244+
// ------------------------------------------------------------------
245+
{
246+
name: "mixed-case Password field is redacted (case-insensitive)",
247+
body: `{"email":"a@b.c","Password":"secret"}`,
248+
checks: []func(t *testing.T, got string){
249+
containsStr(`"email":"a@b.c"`),
250+
containsStr(`"[REDACTED]"`),
251+
notContainsStr("secret"),
252+
},
253+
},
254+
{
255+
name: "all-caps API_KEY field is redacted (case-insensitive)",
256+
body: `{"API_KEY":"sk-upper-case","name":"m"}`,
257+
checks: []func(t *testing.T, got string){
258+
containsStr(`"[REDACTED]"`),
259+
notContainsStr("sk-upper-case"),
260+
containsStr(`"name":"m"`),
261+
},
262+
},
263+
{
264+
name: "mixed-case Client_Secret field is redacted (case-insensitive)",
265+
body: `{"Client_Secret":"cs-mixed","issuer":"https://idp.example"}`,
266+
checks: []func(t *testing.T, got string){
267+
containsStr(`"[REDACTED]"`),
268+
notContainsStr("cs-mixed"),
269+
},
270+
},
271+
// ------------------------------------------------------------------
272+
// Fix 2 – "token" field added to sensitiveFields
273+
// ------------------------------------------------------------------
274+
{
275+
name: "token field is redacted",
276+
body: `{"token":"invite-tok-abc","email":"a@b.c"}`,
277+
checks: []func(t *testing.T, got string){
278+
containsStr(`"token"`),
279+
containsStr(`"[REDACTED]"`),
280+
notContainsStr("invite-tok-abc"),
281+
containsStr(`"email":"a@b.c"`),
282+
},
283+
},
284+
// ------------------------------------------------------------------
285+
// Edge-case: sensitive field whose value is itself an object.
286+
// The sensitiveFields check must fire BEFORE recursion so the nested
287+
// content is never included in the output.
288+
// ------------------------------------------------------------------
289+
{
290+
name: "sensitive field with object value is redacted without recursing",
291+
body: `{"password":{"hash":"abc123","algo":"bcrypt"}}`,
292+
checks: []func(t *testing.T, got string){
293+
containsStr(`"password"`),
294+
containsStr(`"[REDACTED]"`),
295+
notContainsStr("abc123"),
296+
notContainsStr("bcrypt"),
297+
},
298+
},
299+
}
300+
301+
for _, tc := range tests {
302+
t.Run(tc.name, func(t *testing.T) {
303+
t.Parallel()
304+
305+
got := buildDescription([]byte(tc.body))
306+
for _, check := range tc.checks {
307+
check(t, got)
308+
}
309+
})
310+
}
311+
}
312+
313+
// containsStr returns a check function asserting that got contains sub.
314+
func containsStr(sub string) func(t *testing.T, got string) {
315+
return func(t *testing.T, got string) {
316+
t.Helper()
317+
if !strings.Contains(got, sub) {
318+
t.Errorf("buildDescription output %q does not contain %q", got, sub)
319+
}
320+
}
321+
}
322+
323+
// notContainsStr returns a check function asserting that got does NOT contain sub.
324+
func notContainsStr(sub string) func(t *testing.T, got string) {
325+
return func(t *testing.T, got string) {
326+
t.Helper()
327+
if strings.Contains(got, sub) {
328+
t.Errorf("buildDescription output %q must not contain %q", got, sub)
329+
}
330+
}
331+
}

0 commit comments

Comments
 (0)