Skip to content

Commit 547ff18

Browse files
Merge branch 'master' into fix/api-deploy-ttl-claim-required-2026-05-30
2 parents afbcf28 + 243e10b commit 547ff18

4 files changed

Lines changed: 191 additions & 9 deletions

File tree

internal/handlers/storage_presign_middleware_test.go

Lines changed: 14 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -99,8 +99,15 @@ func TestPresign_RegistryHasMiddleware(t *testing.T) {
9999
why string
100100
}{
101101
{
102-
needle: "middleware.OptionalAuth(cfg)",
103-
why: "session JWT cross-check requires OptionalAuth to populate team_id when present",
102+
// H46 F1 (2026-05-21) intent + the 2026-05-30 follow-up fix:
103+
// the chain MUST use OptionalAuthStrict so a malformed/expired
104+
// bearer 401s instead of silently downgrading to the
105+
// anonymous-via-token path. The earlier needle here was
106+
// OptionalAuth(cfg) (matched both variants by prefix) —
107+
// post-2026-05-30 we pin the strict variant explicitly so a
108+
// future drop-back to bare OptionalAuth fails this test.
109+
needle: "middleware.OptionalAuthStrict(cfg)",
110+
why: "session JWT cross-check (strict): present-but-bad bearer must 401, missing bearer still anonymous",
104111
},
105112
{
106113
needle: "middleware.PresignTokenRateLimit(rdb)",
@@ -151,7 +158,11 @@ func TestPresign_TestHelpersMirrorMiddleware(t *testing.T) {
151158
block := srcStr[idx:end]
152159

153160
mustHave := []string{
154-
"middleware.OptionalAuth(cfg)",
161+
// Mirror the production strict-auth wiring (see
162+
// TestPresign_RegistryHasMiddleware). A testhelpers mirror that
163+
// stays on bare OptionalAuth means handler tests would falsely
164+
// pass while production rejects.
165+
"middleware.OptionalAuthStrict(cfg)",
155166
"middleware.PresignTokenRateLimit(rdb)",
156167
`middleware.Idempotency(rdb, "storage.presign")`,
157168
}
Lines changed: 148 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,148 @@
1+
// optional_auth_strict_coverage_test.go — registry-iterating regression test
2+
// for the "anonymous-capable mutating endpoint MUST 401 on a malformed bearer"
3+
// contract (T19 P1-7, MR-P1-38; rule 18: registry-iterating tests, not
4+
// hand-typed lists).
5+
//
6+
// History:
7+
//
8+
// - 2026-05-20: T19 P1-7 migrated /db/new, /vector/new, /cache/new,
9+
// /nosql/new, /queue/new, /storage/new, /webhook/new from bare
10+
// OptionalAuth to OptionalAuthStrict so an agent presenting an
11+
// expired/typo'd bearer header sees a 401 instead of silently
12+
// getting anonymous-tier provisioning.
13+
// - 2026-05-21: H46 F1 followed up on /storage/:token/presign for the
14+
// same reason.
15+
// - 2026-05-30: this file. /stacks/new + DELETE /stacks/:slug were the
16+
// two remaining single-site-fallacy misses (rule 17 surface). This
17+
// test iterates the live route list so the next time a new
18+
// anonymous-capable mutating endpoint ships, it is verified to be on
19+
// the strict variant — not by reading the router source, but by
20+
// replaying a malformed bearer at the route itself.
21+
//
22+
// Design:
23+
//
24+
// A malformed bearer ("Bearer not-a-jwt") on a strict route MUST
25+
// produce 401 BEFORE the handler runs. A missing bearer header on the
26+
// same route must still pass through to the handler (the routes are
27+
// anonymous-capable). The test asserts both wire shapes per route, so
28+
// a future drop-back to bare OptionalAuth fails this test loudly
29+
// regardless of how the route is registered.
30+
package router_test
31+
32+
import (
33+
"net/http"
34+
"net/http/httptest"
35+
"testing"
36+
37+
"github.com/stretchr/testify/assert"
38+
"github.com/stretchr/testify/require"
39+
40+
"instant.dev/internal/email"
41+
"instant.dev/internal/plans"
42+
"instant.dev/internal/router"
43+
"instant.dev/internal/testhelpers"
44+
)
45+
46+
// anonymousCapableMutatingRoutes is the registry of routes that:
47+
//
48+
// 1. accept anonymous callers (no Authorization header is OK), AND
49+
// 2. mutate state (POST/DELETE/PATCH/PUT — never GET).
50+
//
51+
// Every entry MUST be wired with middleware.OptionalAuthStrict (not bare
52+
// OptionalAuth) per T19 P1-7 / MR-P1-38: a present-but-malformed bearer
53+
// header is an agent typo / stale token, and silently downgrading to the
54+
// anonymous tier gives no signal to the caller.
55+
//
56+
// Adding a new anonymous-capable mutating endpoint? Add it here AND wire
57+
// OptionalAuthStrict in router.go. The test below will fail loudly if
58+
// the chain is wrong.
59+
var anonymousCapableMutatingRoutes = []struct {
60+
method string
61+
path string
62+
}{
63+
{"POST", "/db/new"},
64+
{"POST", "/vector/new"},
65+
{"POST", "/cache/new"},
66+
{"POST", "/nosql/new"},
67+
{"POST", "/queue/new"},
68+
{"POST", "/storage/new"},
69+
{"POST", "/webhook/new"},
70+
{"POST", "/stacks/new"},
71+
// DELETE /stacks/:slug — anonymous stacks own their slug as a secret;
72+
// a bad bearer here used to silently downgrade and (after the slug
73+
// lookup) delete the anonymous stack if the slug happened to match.
74+
{"DELETE", "/stacks/anonymous-slug-does-not-exist"},
75+
// POST /storage/:token/presign — H46 F1 (2026-05-21). Same contract:
76+
// strict mode keeps a stale session from signing for an unowned
77+
// tenant prefix.
78+
{"POST", "/storage/some-token/presign"},
79+
}
80+
81+
// TestRouter_AnonymousMutatingRoutes_StrictBearer iterates the registry
82+
// above and asserts that every entry rejects a malformed bearer with 401
83+
// (the OptionalAuthStrict contract). This is a rule-18 registry-driven
84+
// test: a future drop-back to bare OptionalAuth on any one of these
85+
// routes fails here regardless of how the router source happens to be
86+
// arranged.
87+
func TestRouter_AnonymousMutatingRoutes_StrictBearer(t *testing.T) {
88+
db, dbClean := testhelpers.SetupTestDB(t)
89+
defer dbClean()
90+
rdb, rdbClean := testhelpers.SetupTestRedis(t)
91+
defer rdbClean()
92+
93+
cfg := newRouterTestConfig()
94+
cfg.Environment = "production"
95+
// Storage provider must boot so /storage/new and /storage/:token/presign
96+
// are registered. shared-key + AllowSharedKey=true reuses the T3
97+
// success-branch setup from router_coverage_test.go.
98+
cfg.ObjectStoreEndpoint = "do-spaces.example.com"
99+
cfg.ObjectStoreMode = "shared-key"
100+
cfg.ObjectStoreAllowSharedKey = true
101+
cfg.ObjectStoreAccessKey = "AKIATEST"
102+
cfg.ObjectStoreSecretKey = "secret-32-bytes-long-padded-here-okay!"
103+
cfg.ObjectStoreBucket = "instant-shared-test"
104+
cfg.ObjectStoreSecure = true
105+
106+
mailer := email.NewNoop()
107+
planReg := plans.Default()
108+
109+
app, _ := router.NewWithHooks(cfg, db, rdb, nil, mailer, planReg, nil, nil)
110+
require.NotNil(t, app)
111+
112+
for _, r := range anonymousCapableMutatingRoutes {
113+
t.Run(r.method+" "+r.path, func(t *testing.T) {
114+
// Probe 1: malformed bearer → 401. This is the strict-mode
115+
// contract. The exact 401 reason (malformed/expired/etc.)
116+
// is asserted in middleware/auth_test.go; here we only care
117+
// that the route does NOT silently downgrade to anonymous.
118+
req := httptest.NewRequest(r.method, r.path, nil)
119+
req.Header.Set("Authorization", "Bearer this-is-not-a-jwt")
120+
resp, err := app.Test(req, 5_000)
121+
require.NoError(t, err)
122+
defer resp.Body.Close()
123+
assert.Equalf(t, http.StatusUnauthorized, resp.StatusCode,
124+
"%s %s must 401 on a malformed bearer (OptionalAuthStrict); "+
125+
"got %d. If you added this route with bare OptionalAuth, "+
126+
"swap to OptionalAuthStrict — see router.go comment "+
127+
"above the /db/new line for the rationale.",
128+
r.method, r.path, resp.StatusCode)
129+
130+
// Probe 2: no Authorization header at all → must NOT 401.
131+
// The routes are explicitly anonymous-capable; the strict
132+
// variant only triggers when a header is PRESENT but bad.
133+
// We accept any non-401 status — the handler downstream
134+
// may 4xx for a missing body / unknown slug / etc., but
135+
// that proves the middleware chain let the request through.
136+
req2 := httptest.NewRequest(r.method, r.path, nil)
137+
resp2, err := app.Test(req2, 5_000)
138+
require.NoError(t, err)
139+
defer resp2.Body.Close()
140+
assert.NotEqualf(t, http.StatusUnauthorized, resp2.StatusCode,
141+
"%s %s must NOT 401 when no Authorization header is sent "+
142+
"(routes are anonymous-capable); got %d. If you tightened "+
143+
"this route to require auth, remove it from the "+
144+
"anonymousCapableMutatingRoutes registry above.",
145+
r.method, r.path, resp2.StatusCode)
146+
})
147+
}
148+
}

internal/router/router.go

Lines changed: 23 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -659,8 +659,13 @@ func NewWithHooks(cfg *config.Config, db *sql.DB, rdb *redis.Client, geoDbs *mid
659659
// boundary for the anonymous case; strict mode ensures a caller who
660660
// *thinks* they're authenticated but presents a stale session
661661
// doesn't sign for an unowned tenant prefix.
662+
//
663+
// 2026-05-30: the H46 F1 fix landed in the comment but not in the
664+
// chain (the route was still using bare OptionalAuth) — caught by
665+
// the registry-iterating regression test in
666+
// optional_auth_strict_coverage_test.go. Now matches the comment.
662667
app.Post("/storage/:token/presign",
663-
middleware.OptionalAuth(cfg),
668+
middleware.OptionalAuthStrict(cfg),
664669
middleware.PresignTokenRateLimit(rdb),
665670
middleware.Idempotency(rdb, "storage.presign"),
666671
storageH.PresignStorage,
@@ -728,18 +733,32 @@ func NewWithHooks(cfg *config.Config, db *sql.DB, rdb *redis.Client, geoDbs *mid
728733
deployGroup.Post("/:id/redeploy", deployH.Redeploy)
729734

730735
// Stacks — Phase 6 multi-service.
731-
// New/Get/Logs/Delete use OptionalAuth (anonymous stacks supported, same as /db/new etc.).
736+
// New/Get/Logs/Delete are anonymous-capable (same model as /db/new etc.).
732737
// UpdateEnv/Redeploy require auth (mutations on owned stacks).
733738
// RequireWritable rejects impersonated sessions on all mutating
734739
// stack endpoints (POST/PATCH/DELETE) so an admin viewing the
735740
// customer's stack page can't accidentally redeploy / nuke it.
736741
// Idempotency middleware on /stacks/new + /stacks/:slug/redeploy
737742
// covers accidental double-clicks / agent retries the same way it
738743
// does for /deploy/new (multipart-aware fingerprint) and /db/new etc.
739-
app.Post("/stacks/new", middleware.OptionalAuth(cfg), middleware.RequireWritable(), middleware.Idempotency(rdb, "stacks.new"), stackH.New)
744+
//
745+
// MUTATING routes (POST/DELETE) use OptionalAuthStrict for the same
746+
// reason as /db/new etc. (T19 P1-7, 2026-05-20): a present-but-bad
747+
// bearer header returns 401 instead of silently downgrading the
748+
// caller to anonymous-tier provisioning. /stacks/new + DELETE
749+
// /stacks/:slug were missed in the original strict-mode wave — this
750+
// closes the surface (rule 17 / MR-P1-38 follow-up). A missing
751+
// Authorization header still passes through as anonymous, since the
752+
// routes are explicitly anonymous-capable.
753+
//
754+
// READ routes (GET) intentionally stay non-strict: a logged-out tab
755+
// reading /stacks/:slug (e.g. follow-up after revocation) should not
756+
// 401 the page — it should serve the anonymous read view if the slug
757+
// belongs to an anonymous stack, or 404 otherwise.
758+
app.Post("/stacks/new", middleware.OptionalAuthStrict(cfg), middleware.RequireWritable(), middleware.Idempotency(rdb, "stacks.new"), stackH.New)
740759
app.Get("/stacks/:slug", middleware.OptionalAuth(cfg), stackH.Get)
741760
app.Get("/stacks/:slug/logs/:svc", middleware.OptionalAuth(cfg), stackH.Logs)
742-
app.Delete("/stacks/:slug", middleware.OptionalAuth(cfg), middleware.RequireWritable(), stackH.Delete)
761+
app.Delete("/stacks/:slug", middleware.OptionalAuthStrict(cfg), middleware.RequireWritable(), stackH.Delete)
743762
app.Patch("/stacks/:slug/env", middleware.RequireAuth(cfg), middleware.RequireWritable(), stackH.UpdateEnv)
744763
app.Post("/stacks/:slug/redeploy", middleware.RequireAuth(cfg), middleware.RequireWritable(), middleware.Idempotency(rdb, "stacks.redeploy"), stackH.Redeploy)
745764

internal/testhelpers/testhelpers.go

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1066,9 +1066,13 @@ func NewTestAppWithServices(t *testing.T, db *sql.DB, rdb *redis.Client, service
10661066
// B17-P0 (BugBash 2026-05-20): broker-mode presign with the production
10671067
// middleware chain so handler-level tests see the same guarantees as
10681068
// production callers. See internal/router/router.go for the wiring
1069-
// rationale (OptionalAuth → PresignTokenRateLimit → Idempotency).
1069+
// rationale (OptionalAuthStrict → PresignTokenRateLimit → Idempotency).
1070+
// 2026-05-30: switched to OptionalAuthStrict to mirror production after
1071+
// the H46 F1 fix landed in router.go (the comment had said strict but
1072+
// the chain was bare). Handler-level tests need the strict variant or
1073+
// they would falsely pass while prod rejects a bad bearer.
10701074
app.Post("/storage/:token/presign",
1071-
middleware.OptionalAuth(cfg),
1075+
middleware.OptionalAuthStrict(cfg),
10721076
middleware.PresignTokenRateLimit(rdb),
10731077
middleware.Idempotency(rdb, "storage.presign"),
10741078
storageH.PresignStorage,

0 commit comments

Comments
 (0)