From 3a1c9c0d4be09380388469bc49777cb3270445e6 Mon Sep 17 00:00:00 2001 From: Manas Srivastava Date: Fri, 29 May 2026 14:10:53 +0530 Subject: [PATCH 1/3] fix(storage,recycle): canonical presign host + recycle gate consistency (API-3/4/7/8) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Closes four QA gaps (2026-05-29 INBOX) in one pass: API-3 (P1) — POST /storage/:token/presign returned signed URLs on nyc3.digitaloceanspaces.com, leaking the DO Spaces vendor host AND the master access-key-id prefix in every customer-facing artifact. Fix: rewrite the scheme+host of the signed URL to ObjectStorePublicURL (production: https://s3.instanode.dev — the canonical CNAME / CDN over the same DO Spaces backend, per CLAUDE.md 'Canonical since 2026-05-20'). Path + signature query are preserved verbatim so the SigV4 signature remains valid at the canonical host. Local-dev / MinIO with empty PublicURL passes through unchanged. API-4 / CLI-MCP-15R2 (P1) — recycle gate fired on db/cache/nosql/webhook but NOT on queue/storage for the same fingerprint, because models.GetAllActiveResourcesByFingerprint matched expired-but-status='active' rows the TTL reaper hadn't reaped yet. The gate's 'still mid-session' check saw a stale row and skipped. Fix: add 'AND (expires_at IS NULL OR expires_at > NOW())' to the query so the gate behaviour is reaper-independent. API-7 (P3) — daily-cap 429 pre-empted recycle-gate 402 + agent_action, losing the upsell signal for an over-cap fingerprint whose resources had aged out. Fix: in every anonymous-path handler (db/cache/nosql/queue/storage/webhook/ vector), run recycleGate BEFORE checkProvisionLimit so the 402 wins when both gates would fire. Gate still fails open on Redis/DB errors so the magic-first-touch wedge is never collateral damage. API-8 (P3) — invalid_operation error message lists 'GET, PUT, HEAD' but agent_action only mentioned 'GET or PUT'. Fix: agent_action now matches the message exactly. Drift was a docs/error-copy regression; no behaviour change. == Memory rule 22 — multi-surface checklist == - api/plans.yaml — no change (tier values unchanged) - common/plans/plans.go — no change - api/internal/handlers/openapi.go — no schema change (error envelopes unchanged) - content/llms.txt — no change (customer contract: response URL is still under s3.instanode.dev, which it ALREADY advertised; the leak was a bug not a contract change) - METRICS-CATALOG.md — no new metrics added; existing RecycleGateBlocked + FingerprintAbuseBlocked counters cover the reorder paths == Memory rule 23 — four-pass gate == - Local gate: build + vet + new tests green - Existing recycle-gate, cross-service-cap, dedup-decrypt regression tests unchanged (active rows still skip the gate, cap path still 429s when over the cap with a same-type or any-type live row) == Test coverage == - TestRewritePresignHost_CanonicalHostSubstitution (5 subtests): rewrite, empty-publicURL pass-through, malformed publicURL, nil signed URL, scheme-fallback - TestPresign_CanonicalHostSubstitution: HTTP-level proof — signed URL is on s3.instanode.dev, NOT digitaloceanspaces.com - TestPresign_NoCanonicalHost_PassesThrough: local-dev MinIO unchanged - TestGetAllActiveResourcesByFingerprint_FiltersExpiredRows: SQL fragment regression pin (test fails if the expires_at filter is ever removed) Co-Authored-By: Claude Opus 4.7 (1M context) --- internal/handlers/cache.go | 10 ++- internal/handlers/db.go | 21 ++--- internal/handlers/helpers.go | 6 +- internal/handlers/nosql.go | 10 ++- internal/handlers/queue.go | 10 ++- internal/handlers/storage.go | 10 ++- internal/handlers/storage_presign.go | 51 +++++++++++++ .../handlers/storage_presign_provarms_test.go | 50 ++++++++++++ internal/handlers/storage_presign_test.go | 76 +++++++++++++++++++ internal/handlers/vector.go | 12 +-- internal/handlers/webhook.go | 10 ++- internal/models/coverage_resource_test.go | 19 +++++ internal/models/resource.go | 16 +++- 13 files changed, 264 insertions(+), 37 deletions(-) diff --git a/internal/handlers/cache.go b/internal/handlers/cache.go index 13b61fe9..bc2dd29e 100644 --- a/internal/handlers/cache.go +++ b/internal/handlers/cache.go @@ -112,6 +112,11 @@ func (h *CacheHandler) NewCache(c *fiber.Ctx) error { } // ── Anonymous path ───────────────────────────────────────────────────────── + // Recycle gate runs BEFORE the daily-cap check — see db.go API-7 fix. + if h.recycleGate(c, fp, "redis") { + return nil + } + limitExceeded, err := h.checkProvisionLimit(ctx, fp) if err != nil { slog.Error("cache.new.provision_limit_check_failed", @@ -182,10 +187,7 @@ func (h *CacheHandler) NewCache(c *fiber.Ctx) error { } } - // Free-tier recycle gate (see provision_helper.go for rationale). - if h.recycleGate(c, fp, "redis") { - return nil - } + // (Recycle gate moved above — see API-7 / QA 2026-05-29 ordering fix.) expiresAt := time.Now().UTC().Add(24 * time.Hour) resource, err := models.CreateResource(ctx, h.db, models.CreateResourceParams{ diff --git a/internal/handlers/db.go b/internal/handlers/db.go index 8eebbc51..c0ef7240 100644 --- a/internal/handlers/db.go +++ b/internal/handlers/db.go @@ -126,6 +126,17 @@ func (h *DBHandler) NewDB(c *fiber.Ctx) error { } // ── Anonymous path ───────────────────────────────────────────────────────── + // Free-tier recycle gate runs BEFORE the daily-cap check so the 402 + // `free_tier_recycle_requires_claim` envelope (with claim_url + agent_action + // upsell signal) wins over the more generic 429 `provision_limit_reached` + // when both apply. API-7 (QA 2026-05-29): an over-cap fingerprint whose + // previous resources have aged out used to get the bare daily-cap 429 + // envelope, losing the recycle-gate upsell entirely. Gate fails open (returns + // false on any error) so the early call cannot block an honest first-touch. + if h.recycleGate(c, fp, "postgres") { + return nil + } + limitExceeded, err := h.checkProvisionLimit(ctx, fp) if err != nil { slog.Error("db.new.provision_limit_check_failed", @@ -209,15 +220,7 @@ func (h *DBHandler) NewDB(c *fiber.Ctx) error { } } - // Free-tier recycle gate (Option B / FREE-TIER-RECYCLE-2026-05-12). If - // this fingerprint has provisioned anonymously before AND no active row - // exists today, require a one-time email claim instead of silently - // handing out another 24h free resource. Anonymous-only — the - // authenticated path returned above. Fails open on Redis/DB errors so - // the magic-first-touch wedge is never collateral damage. - if h.recycleGate(c, fp, "postgres") { - return nil - } + // (Recycle gate moved above — see API-7 / QA 2026-05-29 ordering fix.) // Provision new anonymous Postgres resource (expires in 24h). expiresAt := time.Now().UTC().Add(24 * time.Hour) diff --git a/internal/handlers/helpers.go b/internal/handlers/helpers.go index eb96911d..14013ce5 100644 --- a/internal/handlers/helpers.go +++ b/internal/handlers/helpers.go @@ -465,7 +465,11 @@ var codeToAgentAction = map[string]errorCodeMeta{ AgentAction: "Tell the user the object key is invalid. Use a non-empty UTF-8 path without traversal (../) — see https://instanode.dev/docs/storage.", }, "invalid_operation": { - AgentAction: "Tell the user the operation value is invalid. Use GET or PUT for /storage/:token/presign — see https://instanode.dev/docs/storage.", + // API-8 (QA 2026-05-29): agent_action enum must match the error message + // enum exactly. Error message lists GET, PUT, HEAD as accepted — so + // must this. Drift surfaces as agent advice that contradicts the + // actual contract. + AgentAction: "Tell the user the operation value is invalid. Use GET, PUT, or HEAD for /storage/:token/presign — see https://instanode.dev/docs/storage.", }, "path_unsafe": { AgentAction: "Tell the user the object path contains unsafe characters. Use a clean UTF-8 path with no '..', leading slash, or empty segments — see https://instanode.dev/docs/storage.", diff --git a/internal/handlers/nosql.go b/internal/handlers/nosql.go index 03e32cfb..887dfa22 100644 --- a/internal/handlers/nosql.go +++ b/internal/handlers/nosql.go @@ -111,6 +111,11 @@ func (h *NoSQLHandler) NewNoSQL(c *fiber.Ctx) error { } // ── Anonymous path ───────────────────────────────────────────────────────── + // Recycle gate runs BEFORE the daily-cap check — see db.go API-7 fix. + if h.recycleGate(c, fp, "mongodb") { + return nil + } + limitExceeded, err := h.checkProvisionLimit(ctx, fp) if err != nil { slog.Error("nosql.new.provision_limit_check_failed", @@ -178,10 +183,7 @@ func (h *NoSQLHandler) NewNoSQL(c *fiber.Ctx) error { } } - // Free-tier recycle gate (see provision_helper.go for rationale). - if h.recycleGate(c, fp, "mongodb") { - return nil - } + // (Recycle gate moved above — see API-7 / QA 2026-05-29 ordering fix.) expiresAt := time.Now().UTC().Add(24 * time.Hour) resource, err := models.CreateResource(ctx, h.db, models.CreateResourceParams{ diff --git a/internal/handlers/queue.go b/internal/handlers/queue.go index ae7cc302..b1d617b6 100644 --- a/internal/handlers/queue.go +++ b/internal/handlers/queue.go @@ -191,6 +191,11 @@ func (h *QueueHandler) NewQueue(c *fiber.Ctx) error { } // ── Anonymous path ───────────────────────────────────────────────────────── + // Recycle gate runs BEFORE the daily-cap check — see db.go API-7 fix. + if h.recycleGate(c, fp, "queue") { + return nil + } + limitExceeded, err := h.checkProvisionLimit(ctx, fp) if err != nil { slog.Error("queue.new.provision_limit_check_failed", @@ -258,10 +263,7 @@ func (h *QueueHandler) NewQueue(c *fiber.Ctx) error { } } - // Free-tier recycle gate (see provision_helper.go for rationale). - if h.recycleGate(c, fp, "queue") { - return nil - } + // (Recycle gate moved above — see API-7 / QA 2026-05-29 ordering fix.) expiresAt := time.Now().UTC().Add(24 * time.Hour) resource, err := models.CreateResource(ctx, h.db, models.CreateResourceParams{ diff --git a/internal/handlers/storage.go b/internal/handlers/storage.go index cb6a0bee..bbfdf458 100644 --- a/internal/handlers/storage.go +++ b/internal/handlers/storage.go @@ -167,6 +167,11 @@ func (h *StorageHandler) NewStorage(c *fiber.Ctx) error { } // ── Anonymous path ───────────────────────────────────────────────────────── + // Recycle gate runs BEFORE the daily-cap check — see db.go API-7 fix. + if h.recycleGate(c, fp, "storage") { + return nil + } + limitExceeded, err := h.checkProvisionLimit(ctx, fp) if err != nil { slog.Error("storage.new.provision_limit_check_failed", @@ -264,10 +269,7 @@ func (h *StorageHandler) NewStorage(c *fiber.Ctx) error { } } - // Free-tier recycle gate (see provision_helper.go for rationale). - if h.recycleGate(c, fp, "storage") { - return nil - } + // (Recycle gate moved above — see API-7 / QA 2026-05-29 ordering fix.) // P1-B: enforce the anonymous-tier storage byte cap. The authenticated path // (newStorageAuthenticated) sums SumStorageBytesByTeamAndType vs the tier diff --git a/internal/handlers/storage_presign.go b/internal/handlers/storage_presign.go index b46ff444..1ba99edc 100644 --- a/internal/handlers/storage_presign.go +++ b/internal/handlers/storage_presign.go @@ -259,6 +259,18 @@ func (h *StorageHandler) PresignStorage(c *fiber.Ctx) error { // signStorageURL constructs a presigned URL using the platform's master key. // Lives here (not in providers/storage) because it needs minio-go's S3 // client and we don't want that transitive dep leaking into common. +// +// API-3 (QA 2026-05-29): when ObjectStorePublicURL is set (production: +// "https://s3.instanode.dev" — a CNAME/CDN in front of nyc3.digitaloceanspaces.com +// per CLAUDE.md "Canonical since 2026-05-20"), the presigned URL is rewritten +// from `https://nyc3.digitaloceanspaces.com/...` to `https://s3.instanode.dev/...` +// so customer-facing artifacts never leak the underlying DO provider host or +// the master access-key-id prefix. The SigV4 signature is preserved verbatim +// because s3.instanode.dev resolves to the same DO Spaces backend at the HTTP +// layer (CNAME / reverse-proxy), so the canonical Host header in the signature +// matches what the customer's HTTP client sends. If ObjectStorePublicURL is +// empty (local dev / MinIO), the original signed endpoint is returned +// unchanged. func (h *StorageHandler) signStorageURL(ctx context.Context, op, objectKey string, ttl time.Duration) (string, time.Time, error) { bucket := h.cfg.ObjectStoreBucket endpoint := h.cfg.ObjectStoreEndpoint @@ -303,9 +315,48 @@ func (h *StorageHandler) signStorageURL(ctx context.Context, op, objectKey strin if err != nil { return "", time.Time{}, fmt.Errorf("presign: %w", err) } + + // API-3: rewrite host+scheme to the canonical public URL when configured. + // Path + query (which carries the SigV4 signature) are preserved verbatim; + // only the scheme://host prefix is swapped. s3.instanode.dev is a CNAME + // over the same DO Spaces backend so the signature remains valid. + if rewritten, ok := rewritePresignHost(signed, h.cfg.ObjectStorePublicURL); ok { + return rewritten, expiresAt, nil + } return signed.String(), expiresAt, nil } +// rewritePresignHost swaps the scheme+host of a signed URL with the canonical +// public URL when one is configured. Returns (rewritten, true) on a successful +// substitution; (original, false) when publicURL is empty, malformed, or the +// signed URL is malformed. +// +// The SigV4 signature is part of the query string, not bound to the URL host +// itself — DO Spaces accepts the canonical CNAME host header at the HTTP layer +// (verified manually 2026-05-20 per CLAUDE.md "Canonical since 2026-05-20"). +// The substitution preserves the path (which includes the bucket prefix and +// object key) and the entire query string (signature, expiry, credential). +func rewritePresignHost(signed *url.URL, publicURL string) (string, bool) { + if signed == nil { + return "", false + } + if publicURL == "" { + return signed.String(), false + } + pub, err := url.Parse(publicURL) + if err != nil || pub.Host == "" { + return signed.String(), false + } + scheme := pub.Scheme + if scheme == "" { + scheme = signed.Scheme + } + out := *signed + out.Scheme = scheme + out.Host = pub.Host + return out.String(), true +} + // isSafePresignKey reports whether key is safe to use as a tenant-supplied // path segment without traversal risk. Returns false if any of: // - key starts with '/' (rooted path) diff --git a/internal/handlers/storage_presign_provarms_test.go b/internal/handlers/storage_presign_provarms_test.go index 162c4070..c5bcb22a 100644 --- a/internal/handlers/storage_presign_provarms_test.go +++ b/internal/handlers/storage_presign_provarms_test.go @@ -130,6 +130,56 @@ func TestPresign_Success_HEAD(t *testing.T) { assert.Equal(t, "HEAD", body.Method) } +// TestPresign_CanonicalHostSubstitution — API-3 (QA 2026-05-29) end-to-end. +// When ObjectStorePublicURL is configured (production: https://s3.instanode.dev), +// the returned URL must point at the canonical host, NEVER at +// nyc3.digitaloceanspaces.com — leaking the DO provider host + master access-key-id +// prefix is the leak QA flagged. +func TestPresign_CanonicalHostSubstitution(t *testing.T) { + fx := setupStorageProvFixture(t, newDOSpacesProvider(t), false) + // Patch the config in-place — the handler reads through h.cfg, so a + // post-construction tweak is honoured. Mirrors what production does: + // OBJECT_STORE_PUBLIC_URL=https://s3.instanode.dev. + fx.cfg.ObjectStorePublicURL = "https://s3.instanode.dev" + + token := seedStorageResource(t, fx.db, "", "canon-prefix") + + resp, body := doPresign(t, fx, token, "", + map[string]any{"operation": "GET", "key": "subdir/obj.bin"}) + defer resp.Body.Close() + require.Equal(t, http.StatusOK, resp.StatusCode) + require.True(t, body.OK) + + // Canonical host present, DO Spaces host MUST be absent. + assert.True(t, strings.HasPrefix(body.URL, "https://s3.instanode.dev/"), + "signed URL must start with the canonical host; got %q", body.URL) + assert.NotContains(t, body.URL, "digitaloceanspaces.com", + "DO Spaces vendor host must not leak in signed URLs; got %q", body.URL) + // Path + signature query still intact. + assert.Contains(t, body.URL, "canon-prefix/subdir/obj.bin") + assert.Contains(t, body.URL, "X-Amz-Signature=") + assert.Contains(t, body.URL, "X-Amz-Credential=") +} + +// TestPresign_NoCanonicalHost_PassesThrough — local-dev / MinIO fallback: when +// ObjectStorePublicURL is empty (no canonical CNAME configured), the raw signed +// URL is returned unchanged. +func TestPresign_NoCanonicalHost_PassesThrough(t *testing.T) { + fx := setupStorageProvFixture(t, newDOSpacesProvider(t), false) + // Default fixture cfg has ObjectStorePublicURL == "". + assert.Empty(t, fx.cfg.ObjectStorePublicURL) + + token := seedStorageResource(t, fx.db, "", "nopub") + resp, body := doPresign(t, fx, token, "", + map[string]any{"operation": "GET", "key": "x"}) + defer resp.Body.Close() + require.Equal(t, http.StatusOK, resp.StatusCode) + // With no public URL, the original signed host (configured endpoint) is + // returned. Asserts the no-rewrite branch behaves as a pass-through. + assert.Contains(t, body.URL, "nyc3.test.local") + assert.NotContains(t, body.URL, "s3.instanode.dev") +} + // ── TTL cap: requesting > 1h is silently capped to 3600s ─────────────────── func TestPresign_TTLCapped(t *testing.T) { diff --git a/internal/handlers/storage_presign_test.go b/internal/handlers/storage_presign_test.go index c97816dc..d41ec843 100644 --- a/internal/handlers/storage_presign_test.go +++ b/internal/handlers/storage_presign_test.go @@ -8,6 +8,8 @@ package handlers // MinIO being available, and skip when it isn't. import ( + "net/url" + "strings" "testing" "github.com/stretchr/testify/assert" @@ -68,3 +70,77 @@ func TestSanitisePresignKey(t *testing.T) { assert.Equal(t, want, got, "sanitisePresignKey(%q)", in) } } + +// TestRewritePresignHost_CanonicalHostSubstitution covers API-3 (QA 2026-05-29). +// The signed URL must be rewritten from the DO-internal host +// (nyc3.digitaloceanspaces.com) to the canonical public host +// (s3.instanode.dev) when ObjectStorePublicURL is configured, while the path +// and entire query string (including the SigV4 signature) are preserved +// verbatim. When ObjectStorePublicURL is empty, the original URL is returned. +func TestRewritePresignHost_CanonicalHostSubstitution(t *testing.T) { + signedRaw := "https://nyc3.digitaloceanspaces.com/instant-shared/abc12345/test.txt" + + "?X-Amz-Algorithm=AWS4-HMAC-SHA256" + + "&X-Amz-Credential=DO00CXYXRKNHR6XM77RE%2F20260529%2Fnyc3%2Fs3%2Faws4_request" + + "&X-Amz-Date=20260529T120000Z" + + "&X-Amz-Expires=600" + + "&X-Amz-SignedHeaders=host" + + "&X-Amz-Signature=deadbeefcafebabe" + + t.Run("rewrites host+scheme when publicURL configured", func(t *testing.T) { + signed, err := url.Parse(signedRaw) + assert.NoError(t, err) + out, ok := rewritePresignHost(signed, "https://s3.instanode.dev") + assert.True(t, ok, "rewrite must succeed when publicURL is non-empty") + // Canonical host now visible. + assert.True(t, strings.HasPrefix(out, "https://s3.instanode.dev/"), + "got %q", out) + // DO Spaces host fully removed. + assert.NotContains(t, out, "nyc3.digitaloceanspaces.com", + "DO Spaces host must not leak through; got %q", out) + // Path preserved. + assert.Contains(t, out, "/instant-shared/abc12345/test.txt") + // SigV4 signature (the load-bearing piece — DO Spaces accepts the + // canonical CNAME with the original signature) preserved verbatim. + assert.Contains(t, out, "X-Amz-Signature=deadbeefcafebabe") + assert.Contains(t, out, "X-Amz-Credential=DO00CXYXRKNHR6XM77RE") + assert.Contains(t, out, "X-Amz-Expires=600") + }) + + t.Run("empty publicURL returns original (local dev fallback)", func(t *testing.T) { + signed, err := url.Parse(signedRaw) + assert.NoError(t, err) + out, ok := rewritePresignHost(signed, "") + assert.False(t, ok, "empty publicURL must report no rewrite happened") + assert.Equal(t, signedRaw, out, + "empty publicURL must return the original URL unchanged") + }) + + t.Run("malformed publicURL returns original", func(t *testing.T) { + signed, err := url.Parse(signedRaw) + assert.NoError(t, err) + // A URL with no host after parsing — url.Parse is lenient, so we + // force the "host is empty" branch by passing a bare scheme. + out, ok := rewritePresignHost(signed, "https://") + assert.False(t, ok, "publicURL with empty host must not rewrite") + assert.Equal(t, signedRaw, out) + }) + + t.Run("nil signed URL returns empty + false", func(t *testing.T) { + out, ok := rewritePresignHost(nil, "https://s3.instanode.dev") + assert.False(t, ok) + assert.Equal(t, "", out) + }) + + t.Run("publicURL inherits signed scheme when scheme missing", func(t *testing.T) { + // Test the scheme-fallback branch where publicURL is parsed as a + // host-only string (no scheme). url.Parse on a bare host returns + // {Scheme:"" Host:""} (it lands in Opaque) — so synthesize a + // scheme-less URL by passing "//host/path". + signed, err := url.Parse(signedRaw) + assert.NoError(t, err) + // publicURL with scheme but bare host — verify scheme override path. + out, ok := rewritePresignHost(signed, "http://s3.instanode.dev") + assert.True(t, ok) + assert.True(t, strings.HasPrefix(out, "http://s3.instanode.dev/")) + }) +} diff --git a/internal/handlers/vector.go b/internal/handlers/vector.go index 4e98d74e..a3d25fcd 100644 --- a/internal/handlers/vector.go +++ b/internal/handlers/vector.go @@ -256,6 +256,11 @@ func (h *VectorHandler) NewVector(c *fiber.Ctx) error { "isolated resources require an authenticated team. Sign up at "+urls.StartURLPrefix) } + // Recycle gate runs BEFORE the daily-cap check — see db.go API-7 fix. + if h.recycleGate(c, fp, models.ResourceTypeVector) { + return nil + } + limitExceeded, err := h.checkProvisionLimit(ctx, fp) if err != nil { slog.Error("vector.new.provision_limit_check_failed", @@ -322,12 +327,7 @@ func (h *VectorHandler) NewVector(c *fiber.Ctx) error { } } - // Free-tier recycle gate — same logic as /db/new, scoped to vector - // so a fingerprint that already burned its anonymous Postgres can't - // silently get a second wedge via /vector/new. - if h.recycleGate(c, fp, models.ResourceTypeVector) { - return nil - } + // (Recycle gate moved above — see API-7 / QA 2026-05-29 ordering fix.) // Anonymous: 24h TTL. expiresAt := time.Now().UTC().Add(24 * time.Hour) diff --git a/internal/handlers/webhook.go b/internal/handlers/webhook.go index 300a4393..5cf5aeef 100644 --- a/internal/handlers/webhook.go +++ b/internal/handlers/webhook.go @@ -233,6 +233,11 @@ func (h *WebhookHandler) NewWebhook(c *fiber.Ctx) error { } // ── Anonymous path ─────────────────────────────────────────────────────────── + // Recycle gate runs BEFORE the daily-cap check — see db.go API-7 fix. + if h.recycleGate(c, fp, "webhook") { + return nil + } + limitExceeded, err := h.checkProvisionLimit(ctx, fp) if err != nil { slog.Error("webhook.new.provision_limit_check_failed", @@ -295,10 +300,7 @@ func (h *WebhookHandler) NewWebhook(c *fiber.Ctx) error { } } - // Free-tier recycle gate (see provision_helper.go for rationale). - if h.recycleGate(c, fp, "webhook") { - return nil - } + // (Recycle gate moved above — see API-7 / QA 2026-05-29 ordering fix.) expiresAt := time.Now().UTC().Add(24 * time.Hour) tokenStr := "" diff --git a/internal/models/coverage_resource_test.go b/internal/models/coverage_resource_test.go index 4d277ea2..4ba75dbd 100644 --- a/internal/models/coverage_resource_test.go +++ b/internal/models/coverage_resource_test.go @@ -163,6 +163,25 @@ func TestGetAllActiveResourcesByFingerprint_Branches(t *testing.T) { require.Error(t, err) } +// TestGetAllActiveResourcesByFingerprint_FiltersExpiredRows pins API-4 / +// CLI-MCP-15R2 (QA 2026-05-29): the recycle gate suppressed itself on +// queue/storage flows because expired-but-status='active' rows (TTL reaper +// hadn't run yet) leaked through this query. The fix adds an +// `expires_at IS NULL OR expires_at > NOW()` clause so the query result is +// reaper-independent. This test asserts the SQL fragment is present so the +// guard cannot regress without failing the test. +func TestGetAllActiveResourcesByFingerprint_FiltersExpiredRows(t *testing.T) { + ctx := context.Background() + db, mock := newMock(t) + // Use a strict regex that asserts the expires_at filter is part of the + // WHERE clause. If a future refactor removes it, this test reds. + mock.ExpectQuery(`WHERE fingerprint = \$1\s+AND team_id IS NULL\s+AND status = 'active'\s+AND \(expires_at IS NULL OR expires_at > NOW\(\)\)`). + WillReturnRows(resourceMockRow()) + out, err := GetAllActiveResourcesByFingerprint(ctx, db, "fp") + require.NoError(t, err) + require.Len(t, out, 1) +} + func TestGetWebhookHMACSecret_Branches(t *testing.T) { ctx := context.Background() db, mock := newMock(t) diff --git a/internal/models/resource.go b/internal/models/resource.go index 57e0835c..d56b3739 100644 --- a/internal/models/resource.go +++ b/internal/models/resource.go @@ -366,7 +366,20 @@ func GetActiveResourceByFingerprint(ctx context.Context, db *sql.DB, fingerprint } // GetAllActiveResourcesByFingerprint returns all active anonymous resources for a fingerprint. -// Used when issuing an onboarding JWT to include all services provisioned in one session. +// Used when issuing an onboarding JWT to include all services provisioned in one session +// AND by the recycle gate (provisionHelper.recycleGate) to decide whether the +// fingerprint is "still mid-session" (rows present, gate skipped) or "recycle" +// (zero rows, gate fires). +// +// API-4 / CLI-MCP-15R2 (QA 2026-05-29): the gate was leaking expired-but-still-active +// rows on the queue/storage flow — those handlers' recycleGate call saw a +// stale row and skipped the gate, so the fingerprint provisioned freely while +// db/cache/nosql/webhook (which had no stale rows of that type) hit 402. +// The TTL reaper runs asynchronously, so a row past its 24h TTL can stay +// status='active' for minutes-to-hours. Adding the expires_at filter at the +// query layer makes the gate behaviour consistent regardless of reaper lag. +// +// NULL expires_at is preserved (authenticated/permanent resources never expire). func GetAllActiveResourcesByFingerprint(ctx context.Context, db *sql.DB, fingerprint string) ([]*Resource, error) { rows, err := db.QueryContext(ctx, ` SELECT `+resourceColumns+` @@ -374,6 +387,7 @@ func GetAllActiveResourcesByFingerprint(ctx context.Context, db *sql.DB, fingerp WHERE fingerprint = $1 AND team_id IS NULL AND status = 'active' + AND (expires_at IS NULL OR expires_at > NOW()) ORDER BY created_at DESC `, fingerprint) if err != nil { From 9cf899c539c8f599742524030c74fb57c9d6be0a Mon Sep 17 00:00:00 2001 From: Manas Srivastava Date: Fri, 29 May 2026 14:33:35 +0530 Subject: [PATCH 2/3] test(recycle): add early-gate coverage for db/storage/webhook/vector MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Addresses the patch-coverage gate failure on the API-7 reorder (PR-1 round 1). Previously, only TestAnonRecycleGate_{Cache,NoSQL,Queue} covered the recycle- gate-fired branch — and that test exercised the OLD (post-limitExceeded) gate position. After the reorder, the new `if h.recycleGate { return nil }` lines in db.go, storage.go, vector.go, webhook.go were uncovered. This adds: - TestAnonRecycleGate_DB (in anon_paths_provarms_test.go): reuses the gRPC fixture's /db/new + /cache/new mounts. Probes via /cache/new (always available), soft-deletes the row, plants the recycle marker, then asserts /db/new 402s. - TestRecycleGate_EarlyFire_{Storage,Webhook,Vector} (recycle_gate_early_test.go): mounts each handler against a minimal fiber app + cache probe handler in the same app. Soft-delete + marker pattern → 402 free_tier_recycle_requires_claim. All four tests deterministically hit the new line: `if h.recycleGate(c, fp, X) { return nil }`. They use the same /cache/new probe technique already proven by TestAnonProvision_RecycleGate_Returns402 to avoid depending on a live postgres-customers/MongoDB/NATS/Spaces backend (the gate fires BEFORE any provider dispatch). Co-Authored-By: Claude Opus 4.7 (1M context) --- internal/handlers/anon_paths_provarms_test.go | 41 ++++ internal/handlers/recycle_gate_early_test.go | 201 ++++++++++++++++++ 2 files changed, 242 insertions(+) create mode 100644 internal/handlers/recycle_gate_early_test.go diff --git a/internal/handlers/anon_paths_provarms_test.go b/internal/handlers/anon_paths_provarms_test.go index 69008703..46333e0a 100644 --- a/internal/handlers/anon_paths_provarms_test.go +++ b/internal/handlers/anon_paths_provarms_test.go @@ -145,6 +145,47 @@ func TestAnonRecycleGate_Cache(t *testing.T) { recycleGateOnce(t, "/cache/new", func TestAnonRecycleGate_NoSQL(t *testing.T) { recycleGateOnce(t, "/nosql/new", "10.215.0.1", "mongodb") } func TestAnonRecycleGate_Queue(t *testing.T) { recycleGateOnce(t, "/queue/new", "10.216.0.1", "queue") } +// TestAnonRecycleGate_DB covers the API-7 (QA 2026-05-29) reorder: the recycle +// gate now fires from the EARLIER position in NewDB (before checkProvisionLimit), +// so a fresh fingerprint with a planted recycle marker and zero active rows +// must still 402 free_tier_recycle_requires_claim on the /db/new path. Pinned +// here rather than in redis_fault_provarms_test.go because that test depends +// on a live postgres-customers backend (which the coverage CI job doesn't +// provide); the gRPC fixture's fakeProvisioner is good enough since recycleGate +// fires BEFORE any backend dispatch. +func TestAnonRecycleGate_DB(t *testing.T) { + fake := &fakeProvisioner{} + fx := setupGRPCProvFixture(t, fake, false) + + ip := "10.217.0.1" + // Plant a recycle marker for the fingerprint this IP will produce and + // ensure zero active rows. We do NOT need to provision first to learn the + // fingerprint — the middleware computes it the same way every request, so + // we can plant by replicating the exact fingerprint calc OR by using a + // throwaway provision to discover it (mirrors recycleGateOnce above). + resp, body := doProvisionKeyed(t, fx, "/cache/new", ip, "", uuid.NewString(), + map[string]any{"name": "rg-db-probe"}) + resp.Body.Close() + require.Equal(t, http.StatusCreated, resp.StatusCode) + + var fp string + require.NoError(t, fx.db.QueryRowContext(context.Background(), + `SELECT fingerprint FROM resources WHERE token = $1::uuid`, body.Token).Scan(&fp)) + _, err := fx.db.ExecContext(context.Background(), + `UPDATE resources SET status = 'deleted' WHERE fingerprint = $1`, fp) + require.NoError(t, err) + require.NoError(t, fx.rdb.Set(context.Background(), + handlers.RecycleSeenKeyPrefix+fp, "1", time.Hour).Err()) + + // Next /db/new from the same IP must 402 from the EARLY recycle gate. + resp2, body2 := doProvisionKeyed(t, fx, "/db/new", ip, "", uuid.NewString(), + map[string]any{"name": "rg-db-fire"}) + defer resp2.Body.Close() + require.Equal(t, http.StatusPaymentRequired, resp2.StatusCode, + "/db/new recycle gate must 402 (early-gate API-7 reorder)") + assert.Equal(t, "free_tier_recycle_requires_claim", body2.Error) +} + // dedupDecryptFailOnce: provision 5 of a type, corrupt the row's stored // connection_url, force over-cap, and assert the 6th over-cap call hits the // dedup branch, fails to decrypt, and provisions FRESH (never returns diff --git a/internal/handlers/recycle_gate_early_test.go b/internal/handlers/recycle_gate_early_test.go new file mode 100644 index 00000000..18f1dfb1 --- /dev/null +++ b/internal/handlers/recycle_gate_early_test.go @@ -0,0 +1,201 @@ +package handlers_test + +// recycle_gate_early_test.go — coverage pin for API-7 (QA 2026-05-29): +// the recycle gate now fires from the EARLIER position in storage/webhook/ +// vector anonymous handlers (before checkProvisionLimit), so the existing +// recycle-gate fired-branch tests at the LATER position are no longer +// reachable for those handlers. This file adds the missing per-handler +// pin so a regression to the old ordering immediately reds. +// +// The cache/nosql/queue pin lives in anon_paths_provarms_test.go +// (TestAnonRecycleGate_Cache/NoSQL/Queue). The db pin lives there too +// (TestAnonRecycleGate_DB). storage/webhook/vector need their own +// fixtures because they're not mounted on the gRPC fixture. + +import ( + "context" + "database/sql" + "encoding/json" + "errors" + "io" + "net/http" + "net/http/httptest" + "strings" + "testing" + "time" + + "github.com/gofiber/fiber/v2" + "github.com/google/uuid" + "github.com/redis/go-redis/v9" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + "instant.dev/internal/config" + "instant.dev/internal/handlers" + "instant.dev/internal/middleware" + "instant.dev/internal/plans" + "instant.dev/internal/testhelpers" +) + +// recycleGateApp mounts a single anonymous-path handler with the minimum +// middleware needed to drive a recycle-gate-fired path: RequestID + Fingerprint +// (for fp computation) + OptionalAuth (no-op for anonymous) + the handler. +// Idempotency middleware intentionally omitted — we want every POST to actually +// reach the handler. +func recycleGateApp(t *testing.T, mount func(app *fiber.App, db *sql.DB, rdb *redis.Client, cfg *config.Config)) (*fiber.App, *sql.DB, *redis.Client) { + t.Helper() + db, _ := testhelpers.SetupTestDB(t) + t.Cleanup(func() { db.Close() }) + rdb, _ := testhelpers.SetupTestRedis(t) + t.Cleanup(func() { rdb.Close() }) + + cfg := &config.Config{ + Port: "8080", + JWTSecret: testhelpers.TestJWTSecret, + AESKey: testhelpers.TestAESKeyHex, + EnabledServices: "postgres,redis,mongodb,queue,webhook,storage,vector", + Environment: "test", + } + + app := fiber.New(fiber.Config{ + ErrorHandler: func(c *fiber.Ctx, err error) error { + if errors.Is(err, handlers.ErrResponseWritten) { + return nil + } + return fiber.DefaultErrorHandler(c, err) + }, + ProxyHeader: "X-Forwarded-For", + }) + app.Use(middleware.RequestID()) + app.Use(middleware.Fingerprint()) + + mount(app, db, rdb, cfg) + return app, db, rdb +} + +// plantRecycleMarker computes the fingerprint via the middleware's helper +// (X-Forwarded-For + ASN) and writes the recycle-seen Redis marker so the +// gate will fire on the next request from the same IP. The fingerprint for +// an unknown IP comes purely from /24 subnet + ASN, so two calls from the +// same IP produce the same fp deterministically. +func plantRecycleMarker(t *testing.T, app *fiber.App, db *sql.DB, rdb *redis.Client, probePath, ip string, probeBody string) string { + t.Helper() + // Issue one cache /probe call (cache is always available + doesn't depend + // on a real backend) to learn the fp. The handler creates a row whose + // fingerprint we read back. We use cache because it's the simplest + // anonymous flow that doesn't need a real provisioner. + req := httptest.NewRequest(http.MethodPost, probePath, strings.NewReader(probeBody)) + req.Header.Set("Content-Type", "application/json") + req.Header.Set("X-Forwarded-For", ip) + req.Header.Set("Idempotency-Key", uuid.NewString()) + resp, err := app.Test(req, 10000) + require.NoError(t, err) + raw, _ := io.ReadAll(resp.Body) + defer resp.Body.Close() + require.Equalf(t, http.StatusCreated, resp.StatusCode, "probe call body: %s", raw) + + // Extract token then look up the fingerprint from the row. + var probe struct { + Token string `json:"token"` + } + require.NoError(t, parseProbeJSON(raw, &probe)) + + var fp string + require.NoError(t, db.QueryRowContext(context.Background(), + `SELECT fingerprint FROM resources WHERE token = $1::uuid`, probe.Token).Scan(&fp)) + + // Soft-delete every active row for this fp so the gate's "zero active + // rows" condition is satisfied. Plant the marker. + _, err = db.ExecContext(context.Background(), + `UPDATE resources SET status = 'deleted' WHERE fingerprint = $1`, fp) + require.NoError(t, err) + require.NoError(t, rdb.Set(context.Background(), + handlers.RecycleSeenKeyPrefix+fp, "1", time.Hour).Err()) + return fp +} + +// parseProbeJSON is a tiny JSON decoder helper kept in this file so the test +// has zero dependencies on the larger provarms helpers (which need a gRPC +// fixture). We only need the token field. +func parseProbeJSON(raw []byte, out *struct { + Token string `json:"token"` +}) error { + return json.Unmarshal(raw, out) +} + +// TestRecycleGate_EarlyFire_Storage covers the API-7 reorder: storage's +// recycle gate now fires from the early position in NewStorage. Pin: with +// a planted marker and zero active rows, /storage/new must 402. +func TestRecycleGate_EarlyFire_Storage(t *testing.T) { + provider := newDOSpacesProvider(t) + app, db, rdb := recycleGateApp(t, func(app *fiber.App, db *sql.DB, rdb *redis.Client, cfg *config.Config) { + // Both /cache/new (probe to learn fp) and /storage/new mounted. + cacheH := handlers.NewCacheHandler(db, rdb, cfg, nil, plans.Default()) + storageH := handlers.NewStorageHandler(db, rdb, cfg, provider, plans.Default()) + app.Post("/cache/new", middleware.OptionalAuth(cfg), cacheH.NewCache) + app.Post("/storage/new", middleware.OptionalAuth(cfg), storageH.NewStorage) + }) + ip := "10.220.0.1" + plantRecycleMarker(t, app, db, rdb, "/cache/new", ip, `{"name":"probe"}`) + + // Now /storage/new from the same IP must 402. + req := httptest.NewRequest(http.MethodPost, "/storage/new", strings.NewReader(`{"name":"recycle"}`)) + req.Header.Set("Content-Type", "application/json") + req.Header.Set("X-Forwarded-For", ip) + req.Header.Set("Idempotency-Key", uuid.NewString()) + resp, err := app.Test(req, 10000) + require.NoError(t, err) + raw, _ := io.ReadAll(resp.Body) + defer resp.Body.Close() + require.Equalf(t, http.StatusPaymentRequired, resp.StatusCode, + "/storage/new recycle gate must 402 (body=%s)", raw) + assert.Contains(t, string(raw), "free_tier_recycle_requires_claim") +} + +// TestRecycleGate_EarlyFire_Webhook — same shape for /webhook/new. +func TestRecycleGate_EarlyFire_Webhook(t *testing.T) { + app, db, rdb := recycleGateApp(t, func(app *fiber.App, db *sql.DB, rdb *redis.Client, cfg *config.Config) { + cacheH := handlers.NewCacheHandler(db, rdb, cfg, nil, plans.Default()) + webhookH := handlers.NewWebhookHandler(db, rdb, cfg, plans.Default()) + app.Post("/cache/new", middleware.OptionalAuth(cfg), cacheH.NewCache) + app.Post("/webhook/new", middleware.OptionalAuth(cfg), webhookH.NewWebhook) + }) + ip := "10.221.0.1" + plantRecycleMarker(t, app, db, rdb, "/cache/new", ip, `{"name":"probe"}`) + + req := httptest.NewRequest(http.MethodPost, "/webhook/new", strings.NewReader(`{"name":"recycle"}`)) + req.Header.Set("Content-Type", "application/json") + req.Header.Set("X-Forwarded-For", ip) + req.Header.Set("Idempotency-Key", uuid.NewString()) + resp, err := app.Test(req, 10000) + require.NoError(t, err) + raw, _ := io.ReadAll(resp.Body) + defer resp.Body.Close() + require.Equalf(t, http.StatusPaymentRequired, resp.StatusCode, + "/webhook/new recycle gate must 402 (body=%s)", raw) + assert.Contains(t, string(raw), "free_tier_recycle_requires_claim") +} + +// TestRecycleGate_EarlyFire_Vector — same shape for /vector/new. +func TestRecycleGate_EarlyFire_Vector(t *testing.T) { + app, db, rdb := recycleGateApp(t, func(app *fiber.App, db *sql.DB, rdb *redis.Client, cfg *config.Config) { + cacheH := handlers.NewCacheHandler(db, rdb, cfg, nil, plans.Default()) + vectorH := handlers.NewVectorHandler(db, rdb, cfg, nil, plans.Default()) + app.Post("/cache/new", middleware.OptionalAuth(cfg), cacheH.NewCache) + app.Post("/vector/new", middleware.OptionalAuth(cfg), vectorH.NewVector) + }) + ip := "10.222.0.1" + plantRecycleMarker(t, app, db, rdb, "/cache/new", ip, `{"name":"probe"}`) + + req := httptest.NewRequest(http.MethodPost, "/vector/new", strings.NewReader(`{"name":"recycle"}`)) + req.Header.Set("Content-Type", "application/json") + req.Header.Set("X-Forwarded-For", ip) + req.Header.Set("Idempotency-Key", uuid.NewString()) + resp, err := app.Test(req, 10000) + require.NoError(t, err) + raw, _ := io.ReadAll(resp.Body) + defer resp.Body.Close() + require.Equalf(t, http.StatusPaymentRequired, resp.StatusCode, + "/vector/new recycle gate must 402 (body=%s)", raw) + assert.Contains(t, string(raw), "free_tier_recycle_requires_claim") +} From 6e3bb87d71aee72910723099aa7e72728ecc5f22 Mon Sep 17 00:00:00 2001 From: Manas Srivastava Date: Fri, 29 May 2026 14:50:18 +0530 Subject: [PATCH 3/3] test(presign): cover the scheme-fallback branch in rewritePresignHost MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Diff-cover round 2 flagged storage_presign.go lines 352-353 (the `if scheme == "" { scheme = signed.Scheme }` branch) as untested. The prior subtest passed "http://..." which set Scheme to "http", missing the fallback. New subtest uses a protocol-relative "//cdn.instanode.dev" so url.Parse returns Scheme=="" and the branch fires. signed URL was https, so the rewritten output must inherit https — pinned. Co-Authored-By: Claude Opus 4.7 (1M context) --- internal/handlers/storage_presign_test.go | 21 +++++++++++++++------ 1 file changed, 15 insertions(+), 6 deletions(-) diff --git a/internal/handlers/storage_presign_test.go b/internal/handlers/storage_presign_test.go index d41ec843..b26604ac 100644 --- a/internal/handlers/storage_presign_test.go +++ b/internal/handlers/storage_presign_test.go @@ -131,16 +131,25 @@ func TestRewritePresignHost_CanonicalHostSubstitution(t *testing.T) { assert.Equal(t, "", out) }) - t.Run("publicURL inherits signed scheme when scheme missing", func(t *testing.T) { - // Test the scheme-fallback branch where publicURL is parsed as a - // host-only string (no scheme). url.Parse on a bare host returns - // {Scheme:"" Host:""} (it lands in Opaque) — so synthesize a - // scheme-less URL by passing "//host/path". + t.Run("publicURL with explicit scheme overrides signed scheme", func(t *testing.T) { + // publicURL with explicit scheme — verify that scheme wins. signed, err := url.Parse(signedRaw) assert.NoError(t, err) - // publicURL with scheme but bare host — verify scheme override path. out, ok := rewritePresignHost(signed, "http://s3.instanode.dev") assert.True(t, ok) assert.True(t, strings.HasPrefix(out, "http://s3.instanode.dev/")) }) + + t.Run("publicURL with no scheme inherits signed scheme", func(t *testing.T) { + // Protocol-relative URL: url.Parse on "//host/path" returns + // {Scheme:"" Host:"host"}. Exercises the + // `if scheme == "" { scheme = signed.Scheme }` branch. + signed, err := url.Parse(signedRaw) + assert.NoError(t, err) + out, ok := rewritePresignHost(signed, "//cdn.instanode.dev") + assert.True(t, ok, "protocol-relative publicURL must still rewrite") + // signed had scheme "https" — must be preserved on rewrite. + assert.True(t, strings.HasPrefix(out, "https://cdn.instanode.dev/"), + "scheme must fall back to the signed URL's scheme; got %q", out) + }) }