diff --git a/internal/handlers/auth.go b/internal/handlers/auth.go index 01513052..a22675cd 100644 --- a/internal/handlers/auth.go +++ b/internal/handlers/auth.go @@ -7,6 +7,7 @@ import ( "encoding/json" "errors" "fmt" + "html" "io" "log/slog" "net/http" @@ -946,6 +947,15 @@ func (h *AuthHandler) consumeOAuthState(ctx context.Context, state string) bool // renderAuthError sends a 400 with a small HTML page so a browser landing on // a broken callback URL gets a readable message instead of raw JSON. +// +// SEC-API FINDING-23 (2026-05-29): headline + detail are interpolated into HTML +// via fmt.Sprintf. Every existing caller passes static literals, but the function +// itself is the only HTML emitter in the api and was unsafe-by-default — any +// future caller passing a user-influenced value (OAuth profile name, JWT claim, +// upstream error string) would have introduced reflected XSS on api.instanode.dev +// (cookies for that host stealable). Both arguments are now html.EscapeString'd +// at the sink so the helper is safe for every caller without forcing them to +// remember to escape — defense in depth. func renderAuthError(c *fiber.Ctx, status int, headline, detail string) error { c.Set("Content-Type", "text/html; charset=utf-8") body := fmt.Sprintf(` @@ -956,7 +966,7 @@ func renderAuthError(c *fiber.Ctx, status int, headline, detail string) error {

%s

Try signing in again →

-`, headline, detail) +`, html.EscapeString(headline), html.EscapeString(detail)) return c.Status(status).SendString(body) } diff --git a/internal/handlers/auth_helpers_coverage_test.go b/internal/handlers/auth_helpers_coverage_test.go index e9eb66d2..b901495a 100644 --- a/internal/handlers/auth_helpers_coverage_test.go +++ b/internal/handlers/auth_helpers_coverage_test.go @@ -271,6 +271,46 @@ func TestAuth_RenderAuthError_StatusAndContentType(t *testing.T) { assert.Contains(t, body, "Detail") } +// SEC-API FINDING-23 regression: renderAuthError must HTML-escape both +// the headline and detail args so a future caller passing user-influenced +// input (OAuth profile name, JWT email claim, upstream error) cannot +// inject script into the api.instanode.dev origin. Closed-form negative +// — the literal `` payloads MUST NOT appear in +// the response body; their escaped forms MUST appear. +func TestAuth_RenderAuthError_HTMLEscapesPayload(t *testing.T) { + app := fiber.New() + const xssHeadline = `` + const xssDetail = `

` + app.Get("/e", func(c *fiber.Ctx) error { + return renderAuthError(c, fiber.StatusBadRequest, xssHeadline, xssDetail) + }) + + req := httptest.NewRequest(http.MethodGet, "/e", nil) + resp, err := app.Test(req, 5000) + require.NoError(t, err) + defer resp.Body.Close() + + buf := make([]byte, 4096) + n, _ := resp.Body.Read(buf) + body := string(buf[:n]) + + // Negative — raw opening tags must not survive (they would let the + // browser parse the payload as live HTML). With `<` and `>` HTML-escaped, + // the entire payload becomes inert text inside the surrounding

/

+ // containers — attribute-like sequences (`onerror=...`) inside an inert + // run never run. + assert.NotContains(t, body, "", "raw must be escaped") + assert.NotContains(t, body, "