Skip to content

Commit dbc9d33

Browse files
committed
admin: run login preflight before body peek (Codex P2)
Reorder HandleLogin so preflightLogin (method, content-type, rate limit) runs before peekClaimedActor. Previously a wrong-method, wrong-content-type, or already-throttled request still forced a 16 KiB body read before being rejected — that lets a hostile client hold handler goroutines open with slow uploads and undermines the rate limiter's protection. Trade-off: 405 / 415 / 429 audit lines no longer record claimed_actor (only the remote address). The IP signal is sufficient for follow-up since those rejections are by definition either non-conforming requests or already-throttled IPs; the gain is that those rejections are now genuinely cheap to serve. readLoginRequest still overwrites claimed_actor with the canonical trimmed value on the happy path. Tests stay green.
1 parent 2c8297e commit dbc9d33

1 file changed

Lines changed: 16 additions & 7 deletions

File tree

internal/admin/auth_handler.go

Lines changed: 16 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -143,21 +143,30 @@ type loginResponse struct {
143143
// Login events (success and failure) emit admin_audit slog entries
144144
// directly. The generic Audit middleware cannot do this because it runs
145145
// before the handler knows who the caller is claiming to be.
146+
//
147+
// Preflight runs before any body inspection so a rejected request
148+
// (wrong method, wrong content type, or rate-limited) returns
149+
// without forcing a body read. That trade-off costs the
150+
// claimed_actor signal on those audit lines — but the IP recorded
151+
// by remote_addr is already enough to follow up on, and reading
152+
// the body before throttling would let a hostile client hold
153+
// handler goroutines open with slow bodies.
146154
func (s *AuthService) HandleLogin(w http.ResponseWriter, r *http.Request) {
147155
rec := newStatusRecorder(w)
148156
defer s.auditLogin(r, rec)
149157

150-
// Best-effort peek at the claimed access key so the audit line
151-
// includes it even when preflight (415/429) or readLoginRequest
152-
// rejects the call. On the happy path readLoginRequest will
153-
// overwrite the same field with the canonical trimmed value.
154-
rec.claimedActor = peekClaimedActor(r)
155-
156158
if !s.preflightLogin(rec, r) {
157159
return
158160
}
161+
// Best-effort peek at the claimed access key so the audit line
162+
// captures it even when readLoginRequest rejects the body. The
163+
// happy path overwrites the same field with the canonical
164+
// trimmed value below.
165+
rec.claimedActor = peekClaimedActor(r)
159166
req, ok := readLoginRequest(rec, r)
160-
rec.claimedActor = req.AccessKey
167+
if ok {
168+
rec.claimedActor = req.AccessKey
169+
}
161170
if !ok {
162171
return
163172
}

0 commit comments

Comments
 (0)