fix(api): W11 hardening — scrub internal_url for anon, wire X-Idempotent-Replay (P1 regressions)#93
Merged
Conversation
Anonymous /db/new, /cache/new, /nosql/new, /queue/new, /vector/new responses leaked the cluster-internal proxy FQDN (instant-pg-proxy.instant.svc.cluster.local, redis-proxy, mongo-proxy, nats-proxy) to any unauthenticated curl. P1 (anon AI agent persona) flagged this as infra topology disclosure with zero utility for the caller — anon resources can't run /deploy/new workloads against the proxy because POST /deploy/new requires a claimed team. This change introduces setInternalURL(resp, tier, connectionURL, kind) in internal_url.go as the single chokepoint for the omit-on-anon rule. ~12 handler callsites switch from inline `"internal_url":` map literals to setInternalURL calls (or get the field stripped entirely on known-anonymous response paths). Twin response paths (db.go, cache.go, nosql.go) keep an inline defensive guard against tier=="anonymous" even though twin requires an authenticated team in practice. Paid tiers (hobby, hobby_plus, pro, growth, team) still receive internal_url unchanged — Pro users running compute alongside their DB legitimately need it (DOKS doesn't hairpin public LB traffic). Tests: - internal/handlers/internal_url_test.go::TestSetInternalURL — anonymous omits, hobby/pro/team/growth emit, empty URL omits on all tiers. Plus TestSetInternalURL_ReturnsSameMap for the chaining contract. - e2e/w11_anon_internal_url_e2e_test.go — POST /cache/new from anonymous IP returns body with NO internal_url field, while connection_url stays populated. Pre-existing failures NOT caused by this change (confirmed against origin/master): TestTeamSelf_Get_ReturnsTeamRow, TestTeamSelf_Patch_RenamesTeam (Scan column-count mismatch), TestAll_ReturnsAllPlans / TestHobbyPlus_TierMatrix / TestYearlyVariants_MirrorMonthly (plans.yaml drift from hobby_plus addition in #92). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
P1 reported `Idempotency-Key` appeared to be silently shadowed by
fingerprint dedup: same-key + same-body returned the fingerprint's
existing token AND no `X-Idempotent-Replay: true` header surfaced.
Investigation showed the middleware wiring in internal/router/router.go
already places Idempotency BEFORE the handler in the per-route chain
(OptionalAuth → RequireWritable → Idempotency → handler), so a cache
hit DOES short-circuit before the handler's fingerprint-dedup branch
runs, and idempotency.go:164 DOES set the replay header on the cached
path. The contract was already structurally correct — what was missing
was explicit black-box coverage that pins the precedence at the HTTP
boundary so a future per-route misconfig (e.g. middleware accidentally
moved AFTER the handler) fails loudly in CI rather than re-introducing
the silent regression.
This change:
1. Expands the idempotency.go header doc-block with an explicit
precedence section covering the three branches:
- key + cached ⇒ replay cached token, X-Idempotent-Replay: true
- key + no cache ⇒ handler runs; response cached for next call
- no key ⇒ handler's fingerprint dedup; header NEVER set
2. Adds e2e/w11_idempotency_e2e_test.go with three black-box tests:
- TestE2E_W11_Idempotency_ReplaysWithHeader: two calls same key +
body return same token, second response carries the header.
- TestE2E_W11_Idempotency_DifferentBody_Returns409: same key +
different body → 409 idempotency_key_conflict.
- TestE2E_W11_FingerprintDedup_NoIdempotencyKey_StillWorks:
fingerprint dedup still functions when no key is sent, and the
replay header is correctly absent on that path.
No middleware code-path changes — the existing unit tests in
internal/middleware/idempotency_test.go (TestIdempotency_ReplaySameBody_
CachedResponse, TestIdempotency_ReplayDifferentBody_Returns409, etc.)
continue to pin the same contracts at the unit layer.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Two P1-flagged API hardening regressions, one PR (tests ship in same PR per Manas's iron rule).
Fix 1 — Scrub
internal_urlfor anonymous-tier provision responsesP1 (anon AI agent persona) found that anonymous
POST /db/new,/cache/new,/nosql/new,/queue/new,/vector/newresponses includedinternal_url: instant-pg-proxy.instant.svc.cluster.local:5432(and the redis/mongo/nats equivalents). This leaks cluster infra topology to any unauthenticated curl, and serves zero purpose for anon callers — they can't run/deploy/newworkloads (which is what the internal proxy is for) without a claimed team.setInternalURL(resp, tier, connectionURL, kind)ininternal/handlers/internal_url.gois the single chokepoint for the omit-on-anon rule."internal_url":map literals tosetInternalURLcalls (or get the field stripped entirely on known-anonymous response paths).hobby,hobby_plus,pro,growth,team) still receiveinternal_urlunchanged — Pro users running compute alongside their DB legitimately need it (DOKS doesn't hairpin public LB traffic).Fix 2 — Lock
X-Idempotent-Replayprecedence vs fingerprint dedupP1 reported
Idempotency-Keyappeared silently shadowed by fingerprint dedup. Investigation showed the middleware wiring ininternal/router/router.goalready placesIdempotencyBEFORE the handler (OptionalAuth → RequireWritable → Idempotency → handler), so a cache hit DOES short-circuit before the handler's fingerprint-dedup branch runs, andidempotency.go:164DOES set the replay header. The contract was already structurally correct — what was missing was explicit black-box coverage that pins the precedence at the HTTP boundary so a future per-route misconfig fails loudly in CI.idempotency.goheader doc-block with an explicit precedence section.Files changed
Fix 1
internal/handlers/internal_url.go— newsetInternalURLhelper +internalURLResponseKey/tierAnonymousnamed constantsinternal/handlers/internal_url_test.go—TestSetInternalURL(7 cases: anonymous omits, hobby/pro/team/growth emit, empty URL omits) +TestSetInternalURL_ReturnsSameMapinternal/handlers/db.go,cache.go,nosql.go,queue.go,vector.go— scrub internal_url on anonymous response paths; route paid paths through helpere2e/w11_anon_internal_url_e2e_test.go— black-box assertion thatPOST /cache/newfrom anon caller returns nointernal_urlFix 2
internal/middleware/idempotency.go— doc-block: explicit precedence section covering all three branchese2e/w11_idempotency_e2e_test.go— three tests: same-key replay sets header, different-body returns 409, no-key path still works without headerTest plan
go build ./...— cleango vet ./...— cleango test ./internal/handlers/ -run "TestSetInternalURL|TestProxiedInternalURL" -count=1— PASS (10 cases)go test ./internal/middleware/ -count=1— PASS (no regressions)go build -tags e2e ./e2e/...— cleanv6.2.3.Pre-existing failures (not regressed by this PR, confirmed against
origin/master)TestTeamSelf_Get_ReturnsTeamRow/TestTeamSelf_Patch_RenamesTeam—models.GetTeamByIDScan column-count mismatch (schema drift).TestAll_ReturnsAllPlans/TestHobbyPlus_TierMatrix/TestYearlyVariants_MirrorMonthly—plans.yamldrift fromhobby_plusaddition in W11: add hobby_plus tier ($19/mo) to api #92.🤖 Generated with Claude Code