Skip to content

admin: Redact sensitive request headers in API logs#7578

Open
steadytao wants to merge 1 commit intocaddyserver:masterfrom
steadytao:fix/admin-api-header-redaction
Open

admin: Redact sensitive request headers in API logs#7578
steadytao wants to merge 1 commit intocaddyserver:masterfrom
steadytao:fix/admin-api-header-redaction

Conversation

@steadytao
Copy link
Copy Markdown
Contributor

Fixes the admin API request logging path so sensitive request headers are redacted instead of being logged verbatim.

Before this change, adminHandler.ServeHTTP logged headers with zap.Reflect("headers", r.Header), which bypassed the normal header redaction behavior used elsewhere in the codebase. This could expose values from headers such as Authorization, Proxy-Authorization, Cookie, and Set-Cookie in admin API logs.

This change replaces that raw header logging with a structured marshaler that redacts those sensitive headers while leaving ordinary headers visible.

Why adminLoggableHTTPHeader instead of reusing loggableHTTPHeader?

  • The existing reusable header marshaler lives in modules/caddyhttp, but admin.go is in the root caddy package. Importing modules/caddyhttp from admin.go creates an import cycle (caddy -> modules/caddyhttp -> caddy) so the admin handler cannot directly reuse that type. For this fix, the smallest clean option is to keep an admin-specific private marshaler in admin.go with the same redaction behavior.

This change:

  • redacts Authorization, Proxy-Authorization, Cookie, and Set-Cookie in admin API request logs
  • adds a regression test covering sensitive-header redaction in the admin handler

Should fix #7566.

Assistance Disclosure

No AI was used.

@francislavoie
Copy link
Copy Markdown
Member

We could possibly move loggableHTTPHeader into internal/ to allow it to be shared 🤔

But I'm okay with this too.

@steadytao steadytao force-pushed the fix/admin-api-header-redaction branch from da6fd61 to 0aff561 Compare March 18, 2026 14:04
@steadytao
Copy link
Copy Markdown
Contributor Author

  • Updated this to move the shared header/string-array log marshalers into internal/ and point both the admin API logging path and existing HTTP/logging helpers at the shared implementation. This keeps the admin fix, avoids the import cycle and makes the redaction behaviour reusable across the codebase which was one purpose of this fix in the first place.

@francislavoie Much better. Much appreciated.

@steadytao
Copy link
Copy Markdown
Contributor Author

steadytao commented Mar 23, 2026

Reusing internal.LoggableHTTPHeader for admin logs lines up redaction with caddyhttp.LoggableHTTPRequest. nit: MarshalLogObject still iterates http.Header map keys in random order—same nondeterminism as zap.Reflect, so avoid snapshot tests that assume stable header ordering.

@themavik Thanks. http.Header iteration is nondeterministic in general but that wasn’t introduced by this PR. The change here was specifically to align redaction behaviour and the test coverage does not depend on header ordering. Please remember the context of the PR rather then simply referencing issues that are not currently covered.

@francislavoie
Copy link
Copy Markdown
Member

Ignore that. It was a shitty AI bot spamming hundreds of repos. We blocked it and reported it.

@steadytao
Copy link
Copy Markdown
Contributor Author

Ignore that. It was a shitty AI bot spamming hundreds of repos. We blocked it and reported it.

Ah, much appreciated

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

admin: redact sensitive request headers in API logs

2 participants