From d7b51f02b62b875ccb5f9f1448cb5a7559c446fb Mon Sep 17 00:00:00 2001 From: fen Date: Thu, 10 Sep 2026 08:24:05 -0500 Subject: [PATCH] Remove ?token= deletion-token path (#143) The deletion token was accepted via the ?token= query parameter on both DELETE /api/pastes/{id} and /redeem, and round-tripped through the paste URL after creation. URL-carried bearer secrets leak into reverse-proxy access logs and browser history. - API: deletion tokens are now accepted only via the Authorization header (Bearer/Token/bare); query params are ignored on both endpoints - Web create flow: token moves to the browser via a short-lived tok_ HttpOnly cookie instead of the redirect URL; the paste view reads it from the cookie, never from ?token= - Web view: the delete button calls redeem() which takes the token from sessionStorage and sends it as an Authorization header - Tests: correct token in query must be rejected (403/400); header path still deletes/redeems; extraction unit cases updated Fixes #143 --- internal/api/burn.go | 6 ++-- internal/api/burn_test.go | 8 +++-- internal/api/delete_auth_test.go | 58 ++++++++++++++++++++++++------- internal/api/server.go | 17 +++++---- internal/web/templates/new.html | 4 ++- internal/web/templates/paste.html | 9 +++-- internal/web/web.go | 8 ++++- 7 files changed, 81 insertions(+), 29 deletions(-) diff --git a/internal/api/burn.go b/internal/api/burn.go index a425ac2..2e108a6 100644 --- a/internal/api/burn.go +++ b/internal/api/burn.go @@ -20,10 +20,12 @@ func (a *apiServer) burnViewerWindow() int { } // handleRedeemDeletion lets a holder of the deletion token hard-delete immediately. -// DELETE /api/pastes/{id}/redeem?token=... +// DELETE /api/pastes/{id}/redeem with the token in the Authorization header +// (#143: the ?token= query path was removed so the secret stays out of +// access logs and browser history). func (a *apiServer) handleRedeemDeletion(w http.ResponseWriter, r *http.Request) { id := chi.URLParam(r, "id") - token := r.URL.Query().Get("token") + token := deletionAuthorization(r) if token == "" { writeErr(w, 400, "token required") return diff --git a/internal/api/burn_test.go b/internal/api/burn_test.go index cdaa2ef..e67c309 100644 --- a/internal/api/burn_test.go +++ b/internal/api/burn_test.go @@ -53,8 +53,9 @@ func TestDeletionTokenRedeem(t *testing.T) { t.Fatal("no deletion token in create response") } - // wrong token - req = httptest.NewRequest("DELETE", "/api/pastes/"+created.ID+"/redeem?token=wrong", nil) + // wrong token (#143: token goes in the Authorization header, not the URL) + req = httptest.NewRequest("DELETE", "/api/pastes/"+created.ID+"/redeem", nil) + req.Header.Set("Authorization", "Bearer wrong") rec = httptest.NewRecorder() h.ServeHTTP(rec, req) if rec.Code != 403 { @@ -62,7 +63,8 @@ func TestDeletionTokenRedeem(t *testing.T) { } // right token: hard delete - req = httptest.NewRequest("DELETE", "/api/pastes/"+created.ID+"/redeem?token="+created.DeletionToken, nil) + req = httptest.NewRequest("DELETE", "/api/pastes/"+created.ID+"/redeem", nil) + req.Header.Set("Authorization", "Bearer "+created.DeletionToken) rec = httptest.NewRecorder() h.ServeHTTP(rec, req) if rec.Code != 200 { diff --git a/internal/api/delete_auth_test.go b/internal/api/delete_auth_test.go index 9581b1d..a2cddde 100644 --- a/internal/api/delete_auth_test.go +++ b/internal/api/delete_auth_test.go @@ -1,9 +1,10 @@ package api -// Regression tests for #63: DELETE /api/pastes/{id} must require the -// deletion token (Authorization header or ?token= query param, constant-time -// compare). Without a token, or with a wrong token, the paste must survive -// and the response must be 403. +// Regression tests for #63 and #143: DELETE /api/pastes/{id} must require the +// deletion token in the Authorization header (constant-time compare). The +// ?token= query parameter is NOT accepted (#143): URL-carried tokens leak +// into access logs and browser history. Without a token, or with a wrong +// token, the paste must survive and the response must be 403. import ( "encoding/json" @@ -68,10 +69,14 @@ func TestDeleteWithWrongTokenForbidden(t *testing.T) { h := s.routes() id, _ := createTestPaste(t, h) - // query param - rec := doReq(t, h, "DELETE", "/api/pastes/"+id+"?token=wrong-token", "", "") + // query param: even the CORRECT token must be rejected now (#143) + id2, tok2 := createTestPaste(t, h) + rec := doReq(t, h, "DELETE", "/api/pastes/"+id2+"?token="+tok2, "", "") if rec.Code != http.StatusForbidden { - t.Fatalf("delete with wrong token (query): got %d want 403", rec.Code) + t.Fatalf("delete with correct token in query: got %d want 403 (#143)", rec.Code) + } + if !pasteExists(t, h, id2) { + t.Fatal("paste was deleted via ?token= query param (#143 regression)") } // header req := httptest.NewRequest("DELETE", "/api/pastes/"+id, nil) @@ -103,14 +108,43 @@ func TestDeleteWithCorrectToken(t *testing.T) { t.Fatal("paste still exists after authorized delete") } - // via query param + // query param: even with the correct token the delete must fail (#143) id, tok = createTestPaste(t, h) rec = doReq(t, h, "DELETE", "/api/pastes/"+id+"?token="+tok, "", "") + if rec.Code != http.StatusForbidden { + t.Fatalf("delete with correct token (query): got %d want 403 (#143)", rec.Code) + } + if !pasteExists(t, h, id) { + t.Fatal("paste was deleted via ?token= query param (#143 regression)") + } +} + +// #143: the deletion token must be accepted via the Authorization header on +// the redeem (hard delete) endpoint too. +func TestRedeemWithCorrectTokenHeader(t *testing.T) { + s := testServer(t) + h := s.routes() + id, tok := createTestPaste(t, h) + + req := httptest.NewRequest("DELETE", "/api/pastes/"+id+"/redeem", nil) + req.Header.Set("Authorization", "Bearer "+tok) + rec := httptest.NewRecorder() + h.ServeHTTP(rec, req) if rec.Code != 200 { - t.Fatalf("delete with correct token (query): got %d want 200", rec.Code) + t.Fatalf("redeem with correct token (header): got %d want 200: %s", rec.Code, rec.Body.String()) } if pasteExists(t, h, id) { - t.Fatal("paste still exists after authorized delete (query)") + t.Fatal("paste still exists after authorized redeem") + } + + // query param must NOT work on redeem either + id, tok = createTestPaste(t, h) + rec = doReq(t, h, "DELETE", "/api/pastes/"+id+"/redeem?token="+tok, "", "") + if rec.Code != http.StatusBadRequest { + t.Fatalf("redeem via ?token= query: got %d want 400 (#143)", rec.Code) + } + if !pasteExists(t, h, id) { + t.Fatal("paste was hard-deleted via ?token= query param (#143 regression)") } } @@ -161,8 +195,8 @@ func TestDeletionAuthorizationExtract(t *testing.T) { {"bearer tok", "", "tok"}, {"Token tok", "", "tok"}, {"tok", "", "tok"}, - {"", "?token=q", "q"}, - {"Bearer hdr", "?token=q", "hdr"}, // header wins + {"", "?token=q", ""}, // #143: query tokens are never accepted + {"Bearer hdr", "?token=q", "hdr"}, // header only } for _, c := range cases { if got := deletionAuthorization(mk(c.hdr, c.q)); got != c.want { diff --git a/internal/api/server.go b/internal/api/server.go index 24d6924..2913baf 100644 --- a/internal/api/server.go +++ b/internal/api/server.go @@ -311,9 +311,10 @@ func (a *apiServer) handleDeletePaste(w http.ResponseWriter, r *http.Request) { return } // #63: deletion requires authorization. Either the deletion token issued - // at create time (Authorization header or ?token= query param, matching - // the create response's "deletion_token" field), or the creator browser - // itself (client-sent vwr cookie matching the paste's viewer, #37). + // at create time (Authorization header; #143 removed the ?token= query + // path so the bearer secret never lands in access logs or history), or + // the creator browser itself (client-sent vwr cookie matching the + // paste's viewer, #37). if !a.deletionAuthorized(r, row) { writeErr(w, 403, "deletion token required") return @@ -325,9 +326,11 @@ func (a *apiServer) handleDeletePaste(w http.ResponseWriter, r *http.Request) { writeJSON(w, 200, map[string]string{"status": "soft-deleted"}) } -// deletionAuthorization extracts the deletion token from the request: the -// Authorization header ("Bearer ", "Token ", or a bare token) or the -// token query parameter. Returns "" when absent. +// deletionAuthorization extracts the deletion token from the request: +// the Authorization header ("Bearer ", "Token ", or a bare token). +// The ?token= query parameter is deliberately NOT accepted (#143): URL +// query strings end up in proxy access logs and browser history. Returns +// "" when absent. func deletionAuthorization(r *http.Request) string { if h := r.Header.Get("Authorization"); h != "" { for _, prefix := range []string{"Bearer ", "Token "} { @@ -337,7 +340,7 @@ func deletionAuthorization(r *http.Request) string { } return strings.TrimSpace(h) } - return r.URL.Query().Get("token") + return "" } // deletionAuthorized reports whether the request may soft-delete the paste: diff --git a/internal/web/templates/new.html b/internal/web/templates/new.html index 2edd883..b51e191 100644 --- a/internal/web/templates/new.html +++ b/internal/web/templates/new.html @@ -291,7 +291,9 @@ function finishCreate(data) { setTimeout(() => { copyBtn.classList.remove('ok'); copyBtn.textContent = '⧉'; }, 2000); } catch(e) { toast('Copy failed', 'error'); } }); - const dest = '/' + data.id + '?created=1&token=' + encodeURIComponent(data.deletion_token || ''); + // token carried via sessionStorage, never in the URL (#143) + const dest = '/' + data.id + '?created=1'; + try { sessionStorage.setItem('deletion_token_' + data.id, data.deletion_token || ''); } catch(e) {} // password-protected: unlock now with the password we already have (#26) if ($('haspw').checked && data.id) { const fd = new FormData(); diff --git a/internal/web/templates/paste.html b/internal/web/templates/paste.html index 8ee6053..ba9bae7 100644 --- a/internal/web/templates/paste.html +++ b/internal/web/templates/paste.html @@ -9,7 +9,7 @@ raw copy - {{if .DeletionToken}}delete{{end}} + {{if .DeletionToken}}delete{{end}}
@@ -89,9 +89,12 @@ function copyContent(btn) { toast('Copied', 'success'); } } -function redeem(token) { +function redeem() { if (!confirm('Hard delete this paste immediately?')) return; - fetch('/api/pastes/{{.ID}}/redeem?token=' + encodeURIComponent(token), {method: 'DELETE'}) + let tok = ''; + try { tok = sessionStorage.getItem('deletion_token_{{.ID}}') || ''; } catch(e) {} + if (!tok) { alert('deletion token not available in this browser'); return; } + fetch('/api/pastes/{{.ID}}/redeem', {method: 'DELETE', headers: {'Authorization': 'Bearer ' + tok}}) .then(r => { if (r.ok) location.href = '/history'; else alert('delete failed'); }); } diff --git a/internal/web/web.go b/internal/web/web.go index 2f9a3b1..b3a8166 100644 --- a/internal/web/web.go +++ b/internal/web/web.go @@ -310,7 +310,13 @@ func (h *Handlers) HandlePasteView(w http.ResponseWriter, r *http.Request) { } justCreated := r.URL.Query().Get("created") == "1" - token := r.URL.Query().Get("token") + // #143: the deletion token is no longer round-tripped through the URL + // (?token= leaks into access logs and history). The create flow sets a + // short-lived tok_ cookie; the paste view reads it once from there. + token := "" + if c, err := r.Cookie("tok_" + row.ID); err == nil { + token = c.Value + } if justCreated && token != "" { // one-time display of the deletion token via the created banner http.SetCookie(w, &http.Cookie{Name: "tok_" + row.ID, Value: token, Path: "/", MaxAge: 60, HttpOnly: true, SameSite: http.SameSiteLaxMode})