fix(security): tighten ?return=... allowlist on login + logout

The post-login redirect honored an arbitrary ?return= query value as
long as it wasn't literally /login or /logout. An attacker phishing a
victim to https://gearbox.example.com/login?return=https://attacker
would land them on the attacker's harvest page immediately after a
successful login, with the URL bar showing only the moment of redirect
from a legitimate origin. The Logout handler had the same shape and
could relay the value back to the login page.

Replace the two-string blocklist with a same-origin allowlist:
isSafeReturnURL accepts only relative paths starting with `/`, rejects
protocol-relative `//host`, rejects `/\…` (some browsers normalize the
backslash to a forward slash, opening another protocol-relative path),
rejects any embedded backslash, and rejects schemed URIs entirely.
Applies to both LoginPost and Logout.

Test pins down the accept/reject matrix including the textbook bypasses
(`//evil.example.com`, `/\evil.example.com`, `javascript:`, `data:`).

P0-4 from the 2026-05 security audit.

Author: sarg3nt

gearbox/internal/framework/handler/login.go

@@ -10,6 +10,37 @@ import (
 	"github.com/sarg3nt/gearbox/internal/framework/templates/pages"
 )
 
+// isSafeReturnURL reports whether a `?return=` (or `?next=`) query value is
+// safe to redirect the browser to after login/logout. Only same-origin
+// relative paths are allowed — anything that could escape to an external host
+// is rejected so the login page can't be used as a phishing redirector.
+//
+// The accepted shape is exactly: starts with `/`, does NOT start with `//`
+// (protocol-relative), does NOT start with `/\` (some browsers normalize this
+// to `//` and follow it as protocol-relative), and does not contain a
+// scheme-like prefix elsewhere. Backslashes anywhere in the value are
+// rejected because some browsers normalize them to forward slashes during
+// URL parsing, opening sneak paths like `/\/evil.example.com`.
+//
+// 2026-05 audit P0-4.
+func isSafeReturnURL(s string) bool {
+	if s == "" {
+		return false
+	}
+	if len(s) < 1 || s[0] != '/' {
+		return false
+	}
+	if len(s) >= 2 && (s[1] == '/' || s[1] == '\\') {
+		return false
+	}
+	for i := 0; i < len(s); i++ {
+		if s[i] == '\\' {
+			return false
+		}
+	}
+	return true
+}
+
 // resolvePostLoginPath returns the path the user should land on after login
 // or after visiting "/" without a destination. Cascade:
 //
@@ -124,9 +155,13 @@ func (h *Handler) LoginPost(w http.ResponseWriter, r *http.Request) {
 	}
 
 	// Redirect to return URL if provided, otherwise honor the per-user
-	// default-landing-path with system fallback.
+	// default-landing-path with system fallback. The return URL must be a
+	// same-origin relative path — anything else (an absolute URL, a
+	// protocol-relative URL, a `javascript:` URI) is rejected to prevent
+	// the login page from being used as a phishing redirector
+	// (2026-05 audit P0-4).
 	redirectTarget := h.resolvePostLoginPath(user.ID)
-	if returnURL != "" && returnURL != "/login" && returnURL != "/logout" {
+	if isSafeReturnURL(returnURL) && returnURL != "/login" && returnURL != "/logout" {
 		redirectTarget = returnURL
 	}
 	http.Redirect(w, r, redirectTarget, http.StatusSeeOther)
@@ -178,9 +213,12 @@ func (h *Handler) Logout(w http.ResponseWriter, r *http.Request) {
 		}
 	}
 
-	// Build redirect URL with logout message
+	// Build redirect URL with logout message. The return URL must pass the
+	// same safety check as the post-login redirect (2026-05 audit P0-4) so
+	// the logout page can't be used to inject an attacker-controlled
+	// `?return=...` into the login page's URL.
 	redirectURL := "/login?message=" + url.QueryEscape("You have been logged out.")
-	if returnURL != "" && returnURL != "/" && returnURL != "/login" && returnURL != "/logout" {
+	if isSafeReturnURL(returnURL) && returnURL != "/" && returnURL != "/login" && returnURL != "/logout" {
 		redirectURL += "&return=" + url.QueryEscape(returnURL)
 	}
 

gearbox/internal/framework/handler/login_test.go

@@ -0,0 +1,41 @@
+package handler
+
+import "testing"
+
+// 2026-05 audit P0-4. The login and logout handlers both consult
+// isSafeReturnURL to decide whether a ?return=... query value can be
+// used as a redirect target. These tests pin down the matrix so future
+// edits don't accidentally widen what the helper accepts.
+func TestIsSafeReturnURL(t *testing.T) {
+	tests := []struct {
+		name  string
+		input string
+		want  bool
+	}{
+		// Safe: same-origin relative paths.
+		{"plain-root", "/", true},
+		{"plain-path", "/dashboard", true},
+		{"nested-path", "/settings/profile", true},
+		{"with-query", "/search?q=hello", true},
+		{"with-fragment", "/page#section", true},
+
+		// Unsafe: anything that could escape origin.
+		{"empty", "", false},
+		{"absolute-https", "https://evil.example.com/x", false},
+		{"absolute-http", "http://evil.example.com/x", false},
+		{"protocol-relative", "//evil.example.com/x", false},
+		{"backslash-relative", "/\\evil.example.com", false},
+		{"backslash-prefix", "\\evil.example.com", false},
+		{"javascript-uri", "javascript:alert(1)", false},
+		{"data-uri", "data:text/html,evil", false},
+		{"no-leading-slash", "dashboard", false},
+		{"backslash-anywhere", "/legit\\path", false},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			if got := isSafeReturnURL(tt.input); got != tt.want {
+				t.Errorf("isSafeReturnURL(%q) = %v, want %v", tt.input, got, tt.want)
+			}
+		})
+	}
+}