Skip to content

Commit 3611381

Browse files
committed
admin: nosniff header + drop dead nil-check (Claude review)
Two minor items from Claude's review on PR #633: - Add X-Content-Type-Options: nosniff to both writeJSONError and writeAdminJSON. The admin surface is JSON-only, so MIME sniffing is never useful and the header guards against XSS-via-sniffing on any payload that ever reaches the response writer through an unexpected path. Cookie-gated admin endpoints already constrain attack surface, but this is cheap defence in depth. - Remove the dead `if resp.Tables == nil { resp.Tables = []string{} }` guard from handleList. paginateDynamoTableNames is total over its input — the "cursor past end" branch returns []string{} and every other branch returns a real sub-slice, both non-nil. Replace the guard with a comment that documents the producer-side invariant so future readers don't assume the function can return nil.
1 parent 455ad39 commit 3611381

2 files changed

Lines changed: 18 additions & 5 deletions

File tree

internal/admin/dynamo_handler.go

Lines changed: 13 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -163,11 +163,12 @@ func (h *DynamoHandler) handleList(w http.ResponseWriter, r *http.Request) {
163163
if next != "" {
164164
resp.NextToken = encodeDynamoNextToken(next)
165165
}
166-
// Tables is an array contract; mint an empty slice rather than
167-
// emitting JSON `null` when the cluster has no tables yet.
168-
if resp.Tables == nil {
169-
resp.Tables = []string{}
170-
}
166+
// paginateDynamoTableNames is total over its input — it always
167+
// returns a non-nil slice (an empty []string{} on the
168+
// "cursor past end" branch, a real sub-slice otherwise) so the
169+
// JSON shape is always `"tables": []` rather than `null` even
170+
// without an explicit nil-check here. The Tables array
171+
// contract is enforced at the producer.
171172
writeAdminJSON(w, r.Context(), h.logger, resp)
172173
}
173174

@@ -288,6 +289,13 @@ func writeAdminJSON(w http.ResponseWriter, ctx context.Context, logger *slog.Log
288289
return
289290
}
290291
w.Header().Set("Content-Type", "application/json; charset=utf-8")
292+
// Defence-in-depth: tell the browser not to MIME-sniff the
293+
// response body. The admin surface is JSON-only, so a sniffed
294+
// "this might be HTML" guess is never useful and could enable
295+
// XSS-via-sniffing on a hostile payload that somehow reached
296+
// here. Cookie-gated admin endpoints + a single static
297+
// Content-Type make this cheap and standard.
298+
w.Header().Set("X-Content-Type-Options", "nosniff")
291299
w.Header().Set("Cache-Control", "no-store")
292300
w.WriteHeader(http.StatusOK)
293301
if _, werr := w.Write(payload); werr != nil {

internal/admin/router.go

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -277,6 +277,11 @@ func writeJSONNotFound(w http.ResponseWriter, _ *http.Request) {
277277

278278
func writeJSONError(w http.ResponseWriter, status int, code, msg string) {
279279
w.Header().Set("Content-Type", "application/json; charset=utf-8")
280+
// Defence-in-depth header: the admin surface is JSON-only, so
281+
// declare nosniff to prevent a misbehaving browser from
282+
// content-sniffing an error body into something executable.
283+
// Cheap and standard for cookie-gated admin endpoints.
284+
w.Header().Set("X-Content-Type-Options", "nosniff")
280285
w.Header().Set("Cache-Control", "no-store")
281286
w.WriteHeader(status)
282287
_ = json.NewEncoder(w).Encode(errorResponse{Error: code, Message: msg})

0 commit comments

Comments
 (0)