Repository navigation
refactor: extract all hardcoded URL/domain literals into internal/urls package - #12
Merged
Merged
Conversation
…s package
User-facing pain that drove this: every domain rename or proxy hostname
tweak required scraping the codebase with grep + sed. The instant.dev →
instanode.dev rename earlier today touched 28 files and still missed
some (we're still finding stragglers). The "Use named constants, not
inline strings" memo applies — extracting was overdue.
New package internal/urls/urls.go centralises:
Public hostnames:
PublicAPIBase = "https://api.instanode.dev"
PublicMarketingBase = "https://instanode.dev"
StartURLPrefix = PublicMarketingBase + "/start"
DeploymentWildcard = "deployment.instanode.dev"
StoragePublicHost = "s3.instanode.dev"
Cluster-internal proxy FQDNs (for in-cluster workloads — friction PR #2):
InternalPGProxy = "instant-pg-proxy.instant.svc.cluster.local:5432"
InternalRedisProxy = "instant-redis-proxy.instant.svc.cluster.local:6379"
InternalMongoProxy = "instant-mongo-proxy.instant.svc.cluster.local:27017"
InternalNATSProxy = "instant-nats-proxy.instant.svc.cluster.local:4222"
InternalMinIO = "minio.instant-data.svc.cluster.local:9000"
Helper:
UpgradeStartURL(jwt) — canonical builder for /start?t=<jwt>, replaces
12 sites that had identical fmt.Sprintf calls.
Call sites refactored:
- 12 fmt.Sprintf upgrade-URL calls across 6 provisioning handlers
→ urls.UpgradeStartURL(jwtToken)
- proxiedInternalURL in handlers/internal_url.go → uses 4 urls.Internal*
- upgradeNote / limitExceededNote bare links → urls.StartURLPrefix
- canonicalAPIBase (handlers/auth.go) → alias of urls.PublicAPIBase
- defaultCanonicalResourceURL (middleware/auth.go) → alias of
urls.PublicAPIBase (eliminates the long-standing duplication of the
same string in two different files)
- 15 user-facing error strings ("Sign up at https://instanode.dev/start")
→ string concat with urls.StartURLPrefix
Audit results:
Before: 28+ occurrences of "instanode.dev/start" in non-test handler code
After: 1 occurrence (an inline example URL inside the OpenAPI JSON
description string — appropriate to leave as a literal since it's
sample documentation, not a runtime-constructed URL)
Before: 4 proxy FQDN string literals in handlers/internal_url.go
After: 0 (all 4 use urls.* constants)
Tests:
internal/urls/urls_test.go (12 sub-tests):
TestPublicHostnames_MatchExpectedShape — guards 5 public hosts
TestInternalProxyHostnames_CorrectPortsAndService — guards 5 internal
TestUpgradeStartURL_Composition — builder semantics
Existing tests still pass:
TestProxiedInternalURL — 7 sub-cases unchanged after refactor
TestUpgradeNote_DoesNotMentionTrial / TestLimitExceededNote_… — 6 cases
TestWhoami_*, TestDeployNew_EnvVars_*, all TestOpenAPI_* — pass
Live verification on v2.1.0-urls-package in prod:
POST /db/new (fresh) → upgrade host = "instanode.dev"
internal_url host = "instant-pg-proxy.instant.svc.cluster.local:5432"
note starts with "Works for 24h free"
GET /api/v1/whoami → 200 with team_id + plan_tier + team_name
(proves the middleware audience lookup still works
via the urls.PublicAPIBase alias)
Out of scope (separate follow-ups):
- Email templates in internal/email/email.go (45 occurrences) — these
are marketing copy strings with their own care; need different
extraction approach (probably a templated config).
- SDK references (sdk-go, sdk-node, mcp) — each is its own repo;
consolidating across modules needs a shared common/urls package and
cross-repo coordination.
- Worker / provisioner repos — they have far fewer URL string
instances and can do their own thing.
- The OpenAPI bearerAuth description's inline example URL — sample
documentation, not a runtime URL.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
4 of 5 tasks
mastermanas805
added a commit
that referenced
this pull request
Jun 2, 2026
CI (full real-DB suite) caught three existing tests that encoded the pre-fix behavior batch-1 deliberately changed: - TestPlansRegistry_IsDedicatedTier: team is now dedicated (#12). - TestDeployCancelDelete_CrossTeam: cross-tenant now 404 not 403 (#22). - TestBrevo_Receive_UnknownMessageID (billing_coverage): delivered UPDATE now carries the terminal-class guard (8 args) + an existence-probe SELECT (#6). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
mastermanas805
added a commit
that referenced
this pull request
Jun 2, 2026
- auth_final2 startEmptyNameGoogleOAuth: emit email_verified:"true" so the Google verified-email requirement (#7) is satisfied by the bespoke server. - tier_enforcement IsDedicatedTier: team is now dedicated (#12, via merged common defaultYAML). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
mastermanas805
added a commit
that referenced
this pull request
Jun 2, 2026
…nt takeover) (#219) * fix(auth): OAuth must require a provider-verified email (#7/#9, account takeover) GitHub and Google OAuth linked accounts by email WITHOUT confirming the provider had verified that email — an attacker controlling an unverified address equal to a victim's could link into / impersonate the victim. - GitHub (#9): fetchGitHubUser now ALWAYS resolves the address from /user/emails and accepts ONLY a primary+verified entry, ignoring the attacker-settable public /user profile email entirely. findOrCreateUserGitHub refuses to link-by-email or create a new identity when no verified email resolved. - Google (#7): decode email_verified (tokeninfo, string) / verified_email (userinfo v2, bool) onto googleUser.EmailVerified; findOrCreateUserGoogle refuses link-by-email / create unless verified. Existing google_id matches are unaffected. Per product decision 2026-06-02 (require verified email, reject if unverified). Hermetic regression tests added (httptest, no DB): verified-primary wins over public email; unverified → empty/ rejected; Google verified flag flows through. Test harness updated to emit the verified flags. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * test(auth): cover OAuth unverified-email refusal (#7/#9 reject branches) DB-backed tests: GitHub login with no primary+verified email and Google login with email_verified=false are both REFUSED (non-200) — exercises the errOAuthEmailUnverified return branches. Closes the #219 patch-coverage gap on auth.go. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * test(api): fix OAuth + tier tests broken by batch-2 on this branch - auth_final2 startEmptyNameGoogleOAuth: emit email_verified:"true" so the Google verified-email requirement (#7) is satisfied by the bespoke server. - tier_enforcement IsDedicatedTier: team is now dedicated (#12, via merged common defaultYAML). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * test(auth): cover the unverified-email guard in findOrCreateUserGoogle (#219 coverage) diff-cover flagged auth.go:1420-1422 (the bug-bash #7 account-takeover guard) uncovered: the existing handler-level test drives the id_token/body flow, but the browser-callback path (GoogleCallbackBrowser → userinfo v2 → findOrCreateUserGoogle) reaches the guard via a code path that test doesn't exercise deterministically. Adds hermetic white-box tests (package handlers, sqlmock — no DB container) that call findOrCreateUserGoogle / findOrCreateUserGitHub directly with an unverified/empty-email new identity and a missing google_id/github_id lookup, asserting errOAuthEmailUnverified. Locks the security guard against regression. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
mastermanas805
added a commit
that referenced
this pull request
Jun 2, 2026
… 404 leak, Team dedicated (#218) * fix(api): bug-bash batch — free TTL, brevo non-clobber, dedup expiry, 404 leak, Team dedicated Five confirmed bugs from the 2026-06-02 platform bug bash: - #4 (P1) free-tier resources never expired: authenticated provisions hardcoded ExpiresAt=nil even for free/anonymous tiers. Add resourceExpiryForTier (24h for ephemeral tiers, nil for paid) and apply it at all 10 authenticated CreateResource sites. Per product decision: enforce plans.yaml's documented 24h TTL for claimed-unpaid resources. - #6 (P1) Brevo 'delivered' webhook clobbered a terminal bounce/complaint on out-of-order delivery, corrupting the email truth surface (rule 12). Guard the UPDATE against terminal classes; distinguish terminal-kept from unknown. - #17/#20 (P2) fingerprint dedup-return handed back credentials for active-but-expired anonymous resources: add the expires_at filter to both GetActiveResourceByFingerprint[Type], matching GetAllActiveResourcesByFingerprint. - #22 (P3) deploy CancelDelete returned 403 cross-tenant (leaking existence); now 404 like the other deploy endpoints. - #12 (P2) Team tier gets dedicated infra: add dedicated:true to team + team_yearly in plans.yaml (pairs with common defaultYAML). Per product decision 2026-06-02. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * test(brevo): cover #6 terminal-kept non-clobber path + fix delivered mocks The delivered UPDATE now carries the terminal-class guard (8 args) and a 0-row result triggers an existence probe. Update expectDeliveredUpdate to the new arg list, add the SELECT mock to the unknown-message test, and add a terminal-kept regression (delivered-after-bounce → matched:true, class preserved). Closes the batch-1 patch-coverage gap on brevo_webhook.go. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * test(api): update existing tests for batch-1 behavior changes CI (full real-DB suite) caught three existing tests that encoded the pre-fix behavior batch-1 deliberately changed: - TestPlansRegistry_IsDedicatedTier: team is now dedicated (#12). - TestDeployCancelDelete_CrossTeam: cross-tenant now 404 not 403 (#22). - TestBrevo_Receive_UnknownMessageID (billing_coverage): delivered UPDATE now carries the terminal-class guard (8 args) + an existence-probe SELECT (#6). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * test(brevo): cover delivered-handler SELECT-probe error branch (#218 coverage) diff-cover flagged brevo_webhook.go:530-531 — the `if qErr != nil` arm of the delivered handler's existence probe (bug bash #6). When the terminal-class- guarded UPDATE affects 0 rows, a follow-up SELECT distinguishes terminal-kept from genuinely-unknown; a non-ErrNoRows fault on that probe must surface as an error (→ 500) so Brevo retries rather than the message being mislabeled. Adds TestBrevo_Receive_Delivered_ProbeError (sqlmock, hermetic): UPDATE → 0 rows, SELECT probe → generic error, asserts 500. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
mastermanas805
added a commit
that referenced
this pull request
Jun 3, 2026
…12) A repeated DELETE on a resource (retry, double-click, or concurrent request) re-ran SoftDeleteResource and re-issued the backend deprovision a second time. The second teardown races the first and can error against an already-gone backend, surfacing a spurious 5xx for what is logically a success. Add an idempotent early-return after the team-ownership check: when the resource is already status='deleted', report success ({"ok":true,"already_deleted":true,"id":...}) and do nothing. The first DELETE already tore the backend down. Test: TestResourceDelete_Idempotent_DoubleDelete — first DELETE → 200, second DELETE → 200 with already_deleted=true. Verified against real Postgres+Redis locally; runs in CI build-and-test. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
mastermanas805
added a commit
that referenced
this pull request
Jun 3, 2026
…12) A repeated DELETE on a resource (retry, double-click, or concurrent request) re-ran SoftDeleteResource and re-issued the backend deprovision a second time. The second teardown races the first and can error against an already-gone backend, surfacing a spurious 5xx for what is logically a success. Add an idempotent early-return after the team-ownership check: when the resource is already status='deleted', report success ({"ok":true,"already_deleted":true,"id":...}) and do nothing. The first DELETE already tore the backend down. Test: TestResourceDelete_Idempotent_DoubleDelete — first DELETE → 200, second DELETE → 200 with already_deleted=true. Verified against real Postgres+Redis locally; runs in CI build-and-test. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
mastermanas805
added a commit
that referenced
this pull request
Jun 3, 2026
…12) (#229) A repeated DELETE on a resource (retry, double-click, or concurrent request) re-ran SoftDeleteResource and re-issued the backend deprovision a second time. The second teardown races the first and can error against an already-gone backend, surfacing a spurious 5xx for what is logically a success. Add an idempotent early-return after the team-ownership check: when the resource is already status='deleted', report success ({"ok":true,"already_deleted":true,"id":...}) and do nothing. The first DELETE already tore the backend down. Test: TestResourceDelete_Idempotent_DoubleDelete — first DELETE → 200, second DELETE → 200 with already_deleted=true. Verified against real Postgres+Redis locally; runs in CI build-and-test. Co-authored-by: Claude Opus 4.8 (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
1 participant
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
Every domain rename so far has been a multi-file sed sweep that missed corners. The `instant.dev → instanode.dev` rename touched 28 files and still left stragglers we're finding hours later. The "Use named constants, not inline strings" memo finally applies here.
New `internal/urls/urls.go` package centralises:
Call sites refactored
Audit before/after
Test plan
Out of scope (follow-ups noted in commit)
🤖 Generated with Claude Code