Skip to content

Commit 19d0db2

Browse files
fix(auth): close CSRF on /auth/email/start + per-IP magic-link rate-limit (P0)
Auth P0 chain found by QA 2026-05-29. Two findings on the magic-link abuse surface, ship together. Findings closed in this PR: AUTH-163 (P0): /auth/email/start accepted Content-Type: application/ x-www-form-urlencoded and inserted a magic_links row. Combined with no Origin/Referer enforcement that was a textbook CSRF primitive: a malicious site could <form action="https://api.instanode.dev/ auth/email/start" method="POST"> to spam any arbitrary email with magic-links from the victim's IP. Fix: require Content-Type: application/json (charset suffix permitted). Form-encoded bodies are rejected with 400 invalid_content_type BEFORE the body is parsed or the DB is touched. The legitimate dashboard / CLI / agent flows all set application/json — the only callers that send urlencoded bodies are third-party-origin browser forms, which is exactly the attack surface we want to close. AUTH-107 / AUTH-097 (P0/P1): no CAPTCHA on /auth/email/start AND no per-IP magic-link rate-limit. Without CAPTCHA (deferred — the Brevo sender gap (project memory project_brevo_sender_not_validated) is the real blocker, and CAPTCHA on a non-deliverable email path just frustrates legitimate users), per-IP limit is the load-bearing abuse-defence. Existing per-email limit (5/hr/email) is trivially bypassed by rotating the email — an attacker can spam any number of 3rd-party addresses from a single IP. Fix: per-IP rate limit of magicLinkPerIPRateLimit (5) per magicLinkPerIPRateLimitWindow (1h), keyed by SHA-256 of c.IP() in Redis under magicLinkPerIPRLKeyPrefix ("ml:ip:rl"). Runs AFTER email validation (so a typo doesn't burn budget) but BEFORE the per-email limit (so it fires even when the per-email budget is fresh). Fail- open on Redis error per CLAUDE.md convention 1. Limited path returns 202 — identical to the success path — so an attacker gains no enumeration signal. Operator visibility via metrics.MagicLinkEmailRateLimited counter + magic_link.start. ip_rate_limited structured log. Regression tests (each reproduces the original exploit and asserts the fix blocks it): TestAuthEmailStart_RejectsFormUrlencoded TestAuthEmailStart_AcceptsJSONWithCharset (guardrail — legit flow) TestAuthEmailStart_PerIPRateLimit (counter-level + handler-level) Rule-22 surface checklist: - api/internal/handlers/magic_link.go (handler) - OpenAPI: handlers/openapi.go not updated (no new public envelope — invalid_content_type follows the existing error-code envelope shape; per-IP-limited still returns the existing 202 success shape). Follow-up: surface invalid_content_type in codeToAgentAction so agents get the right "set Content-Type: application/json" prose without consulting the spec. Same polish PR as the PR-1 reauth_required entry. - content/llms.txt not impacted - instanode-web pricing not impacted - NR alert: bump ml:ip:rl rate-limit counter into the existing magic_link.email_rate_limited alert (counter reused). Follow-up to split into per-IP and per-email tiles for operator clarity. Live-verify (rule 14) and curl evidence (per finding) attached in PR body. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent f2fb140 commit 19d0db2

2 files changed

Lines changed: 287 additions & 0 deletions

File tree

Lines changed: 191 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,191 @@
1+
package handlers
2+
3+
// auth_csrf_rl_authp0_test.go — regression tests for AUTH-163, AUTH-107,
4+
// AUTH-097 shipped 2026-05-29.
5+
//
6+
// Lives in package handlers so it can use the same in-package
7+
// recordingMailer / setupCoverageRedis fixtures established by
8+
// magic_link_coverage_test.go and cli_auth_coverage_test.go.
9+
10+
import (
11+
"context"
12+
"encoding/json"
13+
"errors"
14+
"net/http"
15+
"net/http/httptest"
16+
"strings"
17+
"testing"
18+
19+
"github.com/gofiber/fiber/v2"
20+
"github.com/redis/go-redis/v9"
21+
"github.com/stretchr/testify/assert"
22+
"github.com/stretchr/testify/require"
23+
24+
"instant.dev/internal/config"
25+
"instant.dev/internal/middleware"
26+
)
27+
28+
// csrfRLApp wires the magic-link Start route with an isolated Redis
29+
// connection so the per-IP counter can be observed independently from
30+
// other tests. setupCoverageRedis flushes-on-cleanup via DB-14 isolation.
31+
func csrfRLApp(t *testing.T, rdb *redis.Client) (*fiber.App, *recordingMailer) {
32+
t.Helper()
33+
cfg := &config.Config{JWTSecret: logoutTestSecret}
34+
authH := NewAuthHandler(nil, cfg)
35+
mailer := &recordingMailer{}
36+
h := NewMagicLinkHandlerWithMailerAndRedis(nil, cfg, mailer, authH, rdb)
37+
app := fiber.New(fiber.Config{
38+
ErrorHandler: func(c *fiber.Ctx, err error) error {
39+
if errors.Is(err, ErrResponseWritten) {
40+
return nil
41+
}
42+
code := fiber.StatusInternalServerError
43+
if e, ok := err.(*fiber.Error); ok {
44+
code = e.Code
45+
}
46+
return c.Status(code).JSON(fiber.Map{"ok": false, "error": err.Error()})
47+
},
48+
})
49+
app.Use(middleware.RequestID())
50+
app.Post("/auth/email/start", h.Start)
51+
return app, mailer
52+
}
53+
54+
// flushPerIPRL clears the per-IP Redis budget so tests in the same
55+
// package don't poison each other. Mirrors the cleanup the per-email
56+
// limit relies on (setupCoverageRedis uses DB 14 which is otherwise
57+
// untouched, but the magicLinkPerIPRLKeyPrefix is new and not auto-
58+
// flushed by any existing helper — so we clear it explicitly).
59+
func flushPerIPRL(t *testing.T, rdb *redis.Client) {
60+
t.Helper()
61+
ctx := context.Background()
62+
iter := rdb.Scan(ctx, 0, magicLinkPerIPRLKeyPrefix+":*", 1000).Iterator()
63+
for iter.Next(ctx) {
64+
_ = rdb.Del(ctx, iter.Val()).Err()
65+
}
66+
}
67+
68+
// TestAuthEmailStart_RejectsFormUrlencoded — AUTH-163.
69+
//
70+
// Original exploit (QA confirmed):
71+
//
72+
// POST /auth/email/start
73+
// Content-Type: application/x-www-form-urlencoded
74+
// email=qa@x.com
75+
// → HTTP 202 + magic-link inserted in platform_db.
76+
//
77+
// Combined with no Origin enforcement this was a textbook CSRF: any
78+
// malicious site could <form action="..."> to spam an arbitrary email
79+
// with magic-links from the victim's IP.
80+
//
81+
// Fix: Content-Type must be application/json (charset suffix permitted).
82+
// Form-encoded bodies are rejected with 400 invalid_content_type BEFORE
83+
// the body is parsed or the DB is touched.
84+
func TestAuthEmailStart_RejectsFormUrlencoded(t *testing.T) {
85+
rdb, clean := setupCoverageRedis(t)
86+
defer clean()
87+
defer flushPerIPRL(t, rdb)
88+
app, mailer := csrfRLApp(t, rdb)
89+
90+
req := httptest.NewRequest(http.MethodPost, "/auth/email/start",
91+
strings.NewReader("email=qa%40example.com&return_to=https%3A%2F%2Finstanode.dev"))
92+
req.Header.Set("Content-Type", "application/x-www-form-urlencoded")
93+
resp, err := app.Test(req, 5000)
94+
require.NoError(t, err)
95+
defer resp.Body.Close()
96+
97+
require.Equal(t, http.StatusBadRequest, resp.StatusCode,
98+
"AUTH-163: urlencoded bodies must be 400, not 202")
99+
var envelope struct {
100+
Error string `json:"error"`
101+
}
102+
require.NoError(t, json.NewDecoder(resp.Body).Decode(&envelope))
103+
assert.Equal(t, "invalid_content_type", envelope.Error)
104+
assert.Empty(t, mailer.calls, "no mail must be sent on the CSRF path")
105+
}
106+
107+
// TestAuthEmailStart_AcceptsJSONWithCharset — guardrail.
108+
// The legitimate dashboard flow sometimes sets
109+
// Content-Type: application/json; charset=utf-8. The new gate must
110+
// accept that — only the bare media type is checked.
111+
func TestAuthEmailStart_AcceptsJSONWithCharset(t *testing.T) {
112+
rdb, clean := setupCoverageRedis(t)
113+
defer clean()
114+
defer flushPerIPRL(t, rdb)
115+
app, _ := csrfRLApp(t, rdb)
116+
117+
req := httptest.NewRequest(http.MethodPost, "/auth/email/start",
118+
strings.NewReader(`{"email":"not-an-email"}`))
119+
req.Header.Set("Content-Type", "application/json; charset=utf-8")
120+
resp, err := app.Test(req, 5000)
121+
require.NoError(t, err)
122+
defer resp.Body.Close()
123+
124+
// We send an invalid email so we don't need a DB; the load-bearing
125+
// assertion is that the response is NOT invalid_content_type.
126+
require.Equal(t, http.StatusBadRequest, resp.StatusCode)
127+
var envelope struct {
128+
Error string `json:"error"`
129+
}
130+
require.NoError(t, json.NewDecoder(resp.Body).Decode(&envelope))
131+
assert.Equal(t, "invalid_email", envelope.Error,
132+
"application/json; charset=utf-8 must NOT trip the new content-type gate")
133+
}
134+
135+
// TestAuthEmailStart_PerIPRateLimit — AUTH-097 / AUTH-107.
136+
//
137+
// The per-email-hash limit is bypassed by rotating the email. The new
138+
// per-IP limit caps to magicLinkPerIPRateLimit calls per IP per window.
139+
// The (limit+1)th request from the same IP must be silently absorbed.
140+
//
141+
// Two-layer assertion:
142+
//
143+
// 1. Function-level (checkPerIPRateLimit): exercise the counter
144+
// boundary directly so the cap value and Redis expiry semantics are
145+
// pinned regardless of handler wiring.
146+
//
147+
// 2. Handler-level (POST /auth/email/start): pre-populate the Redis
148+
// counter for the test's source IP (0.0.0.0 — fiber app.Test uses
149+
// this for in-process requests) to one over the cap, then hit the
150+
// handler with a valid Content-Type + valid-syntax email and assert
151+
// 202 + no mailer call. This proves the gate is wired into Start.
152+
func TestAuthEmailStart_PerIPRateLimit(t *testing.T) {
153+
rdb, clean := setupCoverageRedis(t)
154+
defer clean()
155+
defer flushPerIPRL(t, rdb)
156+
157+
const probeIP = "203.0.113.99" // TEST-NET-3, RFC 5737.
158+
ctx := context.Background()
159+
160+
// Layer 1: function-level boundary check.
161+
for i := 1; i <= magicLinkPerIPRateLimit; i++ {
162+
limited, err := checkPerIPRateLimit(ctx, rdb, probeIP)
163+
require.NoError(t, err, "call %d: Redis must be healthy", i)
164+
assert.False(t, limited, "call %d under the per-IP cap must NOT be limited", i)
165+
}
166+
limited, err := checkPerIPRateLimit(ctx, rdb, probeIP)
167+
require.NoError(t, err)
168+
assert.True(t, limited,
169+
"AUTH-097: the (magicLinkPerIPRateLimit+1)th call from the same IP must be limited")
170+
171+
// Layer 2: handler integration. Fiber's app.Test connects from
172+
// 0.0.0.0 — pre-populate that key directly so the next handler
173+
// call hits the over-cap path WITHOUT us having to drive a full
174+
// magic-link flow with a wired DB.
175+
const fiberTestIP = "0.0.0.0"
176+
key := perIPRateLimitKey(fiberTestIP)
177+
require.NoError(t, rdb.Set(ctx, key, magicLinkPerIPRateLimit+1, magicLinkPerIPRateLimitWindow).Err(),
178+
"pre-populate the per-IP counter for the fiber-test source IP")
179+
180+
app, mailer := csrfRLApp(t, rdb)
181+
req := httptest.NewRequest(http.MethodPost, "/auth/email/start",
182+
strings.NewReader(`{"email":"survivor@example.com"}`))
183+
req.Header.Set("Content-Type", "application/json")
184+
resp, err := app.Test(req, 5000)
185+
require.NoError(t, err)
186+
defer resp.Body.Close()
187+
assert.Equal(t, http.StatusAccepted, resp.StatusCode,
188+
"AUTH-097: silent-absorb path returns 202 with no enumeration signal")
189+
assert.Empty(t, mailer.calls,
190+
"the limited call must NOT invoke the mailer")
191+
}

internal/handlers/magic_link.go

Lines changed: 96 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,19 @@ const magicLinkEmailRateLimitWindow = time.Hour
3838
// coupling to a string literal buried in a format string.
3939
const magicLinkEmailRLKeyPrefix = "ml:email:rl"
4040

41+
// magicLinkPerIPRateLimit caps magic-link Start calls per source IP per
42+
// magicLinkPerIPRateLimitWindow (AUTH-097 / AUTH-107). The existing
43+
// per-email limit is bypassed by rotating the address; this is the
44+
// per-IP backstop. 5/hr matches the per-email cap so the two limits
45+
// behave symmetrically from an operator-monitoring perspective.
46+
const magicLinkPerIPRateLimit = 5
47+
const magicLinkPerIPRateLimitWindow = time.Hour
48+
49+
// magicLinkPerIPRLKeyPrefix is the Redis key prefix for per-IP rate
50+
// limits. Distinct from magicLinkEmailRLKeyPrefix so the two budgets
51+
// can be observed and tuned independently in NR / Prom.
52+
const magicLinkPerIPRLKeyPrefix = "ml:ip:rl"
53+
4154
// magicLinkStartMaxBodyBytes caps the inbound POST /auth/email/start JSON
4255
// body. Real bodies are ~80 bytes (email + return_to); 1 KiB is comfortable
4356
// for a future field without inviting megabyte-sized abuse payloads. The
@@ -129,6 +142,37 @@ func checkEmailRateLimit(ctx context.Context, rdb *redis.Client, emailAddr strin
129142
return count > int64(magicLinkEmailRateLimit), nil
130143
}
131144

145+
// perIPRateLimitKey returns the Redis key for a given source IP. IP is
146+
// hashed (SHA-256) so raw IPs never appear in Redis MONITOR output / key
147+
// dumps / logs — same PII-hygiene pattern emailRateLimitKey uses.
148+
func perIPRateLimitKey(ip string) string {
149+
h := sha256.Sum256([]byte(ip))
150+
return fmt.Sprintf("%s:%x", magicLinkPerIPRLKeyPrefix, h[:])
151+
}
152+
153+
// checkPerIPRateLimit increments the per-IP Redis counter and returns
154+
// (limited, err). Same fail-open semantics as checkEmailRateLimit — a
155+
// Redis outage must never block legitimate sign-in. The 202 response on
156+
// the limited path is identical to the success path (no enumeration
157+
// signal). AUTH-097 / AUTH-107.
158+
func checkPerIPRateLimit(ctx context.Context, rdb *redis.Client, ip string) (limited bool, err error) {
159+
if rdb == nil || ip == "" {
160+
return false, nil
161+
}
162+
key := perIPRateLimitKey(ip)
163+
pipe := rdb.Pipeline()
164+
incrCmd := pipe.Incr(ctx, key)
165+
pipe.Expire(ctx, key, magicLinkPerIPRateLimitWindow)
166+
if _, execErr := pipe.Exec(ctx); execErr != nil {
167+
return false, fmt.Errorf("magic_link.ip_rl: %w", execErr)
168+
}
169+
count, resultErr := incrCmd.Result()
170+
if resultErr != nil {
171+
return false, fmt.Errorf("magic_link.ip_rl.result: %w", resultErr)
172+
}
173+
return count > int64(magicLinkPerIPRateLimit), nil
174+
}
175+
132176
// magicLinkStartRequest is the body for POST /auth/email/start.
133177
type magicLinkStartRequest struct {
134178
Email string `json:"email"`
@@ -155,6 +199,30 @@ type magicLinkStartRequest struct {
155199
func (h *MagicLinkHandler) Start(c *fiber.Ctx) error {
156200
requestID := middleware.GetRequestID(c)
157201

202+
// AUTH-163 (P0): the server previously accepted
203+
// Content-Type: application/x-www-form-urlencoded on this endpoint
204+
// (Fiber's BodyParser autodetects the content-type and merrily
205+
// decoded `email=qa@x.com&return_to=...` form bodies). Combined with
206+
// no Origin/Referer enforcement that was a textbook CSRF primitive:
207+
// any malicious site could <form action="https://api.instanode.dev/
208+
// auth/email/start" method="POST"> to spam any arbitrary email with
209+
// magic-links from the victim's IP.
210+
//
211+
// Fail-closed: require Content-Type to start with application/json.
212+
// The legitimate dashboard / CLI / agent flows all set
213+
// application/json; the only callers that send urlencoded bodies are
214+
// browser <form> POSTs from third-party origins — which is exactly
215+
// the attack surface we want to close.
216+
ct := strings.ToLower(strings.TrimSpace(c.Get("Content-Type")))
217+
// Strip any "; charset=..." suffix before comparing.
218+
if semi := strings.IndexByte(ct, ';'); semi >= 0 {
219+
ct = strings.TrimSpace(ct[:semi])
220+
}
221+
if ct != "application/json" {
222+
return respondError(c, fiber.StatusBadRequest, "invalid_content_type",
223+
"POST /auth/email/start requires Content-Type: application/json. Form-encoded bodies are rejected to close the CSRF primitive.")
224+
}
225+
158226
// B4-F5 (BugBash 2026-05-20): the global Fiber BodyLimit is 50MiB to
159227
// accommodate /deploy/new tarballs — that's far too generous for a
160228
// 2-field JSON envelope. A 10MB JSON body on /auth/email/start passed
@@ -178,6 +246,34 @@ func (h *MagicLinkHandler) Start(c *fiber.Ctx) error {
178246
return respondError(c, fiber.StatusBadRequest, "invalid_email", "A valid email address is required")
179247
}
180248

249+
// AUTH-107 / AUTH-097 (P0/P1): per-IP rate limit. The existing per-
250+
// email-hash limit is bypassed by rotating the address — an attacker
251+
// can spam any number of arbitrary 3rd-party addresses from one IP.
252+
// With Brevo currently configured to return 201 then internally
253+
// reject (project memory project_brevo_sender_not_validated), every
254+
// spam send still chews through DB rows + Brevo quota.
255+
//
256+
// Runs AFTER email validation so a typo doesn't burn budget; runs
257+
// BEFORE the per-email limit because the per-IP limit is the wider
258+
// abuse-defence layer and we want it to fire even when the per-email
259+
// budget is fresh. Fail-open on Redis error per CLAUDE.md convention
260+
// 1. Limited path returns 202 — identical to the success path —
261+
// so an attacker gains no enumeration signal.
262+
if ipLimited, ipErr := checkPerIPRateLimit(c.Context(), h.rdb, c.IP()); ipErr != nil {
263+
slog.Warn("magic_link.start.ip_rl_error",
264+
"error", ipErr,
265+
"request_id", requestID,
266+
)
267+
// fail-open
268+
} else if ipLimited {
269+
metrics.MagicLinkEmailRateLimited.Inc()
270+
slog.Warn("magic_link.start.ip_rate_limited",
271+
"request_id", requestID,
272+
"ip", c.IP(),
273+
)
274+
return c.Status(fiber.StatusAccepted).JSON(fiber.Map{"ok": true})
275+
}
276+
181277
// Per-email rate limit (A04). Fail-open on Redis error so a cache
182278
// outage never blocks sign-in. The 202 response on the limited path is
183279
// identical to the success path — the attacker gains no signal.

0 commit comments

Comments
 (0)