Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 11 additions & 1 deletion internal/handlers/auth.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import (
"encoding/json"
"errors"
"fmt"
"html"
"io"
"log/slog"
"net/http"
Expand Down Expand Up @@ -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(`<!DOCTYPE html>
Expand All @@ -956,7 +966,7 @@ func renderAuthError(c *fiber.Ctx, status int, headline, detail string) error {
<p style="color:#444;">%s</p>
<p><a href="https://instanode.dev/login">Try signing in again &rarr;</a></p>
</body>
</html>`, headline, detail)
</html>`, html.EscapeString(headline), html.EscapeString(detail))
return c.Status(status).SendString(body)
}

Expand Down
40 changes: 40 additions & 0 deletions internal/handlers/auth_helpers_coverage_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 `<script>` and `</script>` 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 = `<script>alert("xss-headline")</script>`
const xssDetail = `</p><img src=x onerror="alert('xss-detail')">`
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 <h2>/<p>
// containers — attribute-like sequences (`onerror=...`) inside an inert
// run never run.
assert.NotContains(t, body, "<script>", "raw <script> must be escaped")
assert.NotContains(t, body, "</script>", "raw </script> must be escaped")
assert.NotContains(t, body, "<img src=x", "raw <img must be escaped")

// Positive — escaped forms must be present so the message still
// renders as visible text in the browser.
assert.Contains(t, body, "&lt;script&gt;", "headline must be HTML-escaped")
assert.Contains(t, body, "&lt;/script&gt;", "headline closer must be HTML-escaped")
assert.Contains(t, body, "&lt;/p&gt;", "detail prefix must be HTML-escaped")
assert.Contains(t, body, "&lt;img src=x", "detail <img must be HTML-escaped")
}

// TestSignSessionJWT_RoundTrip mints a JWT via signSessionJWT and
// asserts the resulting token decodes with the expected uid/tid/email.
func TestAuth_SignSessionJWT_RoundTrip(t *testing.T) {
Expand Down
12 changes: 10 additions & 2 deletions internal/middleware/fingerprint.go
Original file line number Diff line number Diff line change
Expand Up @@ -124,9 +124,17 @@ func e2eTokenAccepted(c *fiber.Ctx) bool {
if subtle.ConstantTimeCompare([]byte(got), []byte(expected)) == 1 {
return true
}
// SEC-API FINDING-26 (2026-05-29): previously logged `expected_len` +
// `got_prefix` on every mismatch. Both are info-disclosure: expected_len
// hands an attacker who can read the api logs the byte-length of
// E2E_TEST_TOKEN (narrows brute-force search), and got_prefix is the
// attacker's own guess being echoed back into long-term log storage
// (correlation-grep risk, plus attests the env var is configured in
// prod). Keep only `got_len` — the attacker already knows the length of
// their own input, so this leaks nothing new while still letting an SRE
// distinguish "malformed/missing" from "wrong-content" failures.
slog.Warn("e2e_bypass.token_mismatch",
"got_len", len(got), "expected_len", len(expected),
"got_prefix", got[:min(8, len(got))])
"got_len", len(got))
return false
}

Loading