feat: automatically trust project's hosted handler domain (built-with domain)#1470
feat: automatically trust project's hosted handler domain (built-with domain)#1470N2D4 wants to merge 2 commits into
Conversation
… domain) The <project-id>.built-with-stack-auth.com domain (or whatever the configured hosted handler domain suffix is) is now automatically accepted as a trusted domain for each project. Changes: - redirect-urls.tsx: Include hosted handler domain in trusted domains for redirect URL validation (covers OAuth callbacks, passkeys, etc.) - turnstile.tsx: Accept hosted handler hostname for Turnstile validation - oauth/model.tsx: Include hosted handler in OAuth client redirect URIs Co-Authored-By: Konstantin Wohlwend <n2d4xc@gmail.com>
🤖 Devin AI EngineerI'll be helping with this pull request! Here's what you should know: ✅ I will automatically:
Note: I can only respond to comments from users who have write access to this repository. ⚙️ Control Options:
|
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Caution Review failedPull request was closed or merged during review 📝 WalkthroughWalkthroughBackend validation endpoints now trust a project-specific hosted handler domain derived from an environment-configurable suffix. A new shared helper ChangesHosted Handler Domain Trust Configuration
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Co-Authored-By: Konstantin Wohlwend <n2d4xc@gmail.com>
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 2 potential issues.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit cfb9af8. Configure here.
| */ | ||
| export function getHostedHandlerDomainSuffix(): string { | ||
| return getEnvVariable("NEXT_PUBLIC_STACK_HOSTED_HANDLER_DOMAIN_SUFFIX", defaultHostedHandlerDomainSuffix); | ||
| } |
There was a problem hiding this comment.
Dev suffix breaks hosted trust
High Severity
The getHostedHandlerDomainSuffix function returns the raw NEXT_PUBLIC_STACK_HOSTED_HANDLER_DOMAIN_SUFFIX env var. In local dev, this includes an unresolved ${NEXT_PUBLIC_STACK_PORT_PREFIX:-81} placeholder. This causes the backend to trust a malformed hosted domain, leading to automatic redirect URL trust failures due to a mismatch with client-used domains.
Reviewed by Cursor Bugbot for commit cfb9af8. Configure here.
| trustedDomains: [ | ||
| ...Object.values(tenancy.config.domains.trustedDomains).map(domain => domain.baseUrl), | ||
| `https://${hostedDomain}`, | ||
| ], |
There was a problem hiding this comment.
Ignores hosted handler URL template
Medium Severity
Redirect, OAuth, and Turnstile trust only https://{projectId}{domainSuffix}, but hosted handler URLs come from NEXT_PUBLIC_STACK_HOSTED_HANDLER_URL_TEMPLATE when set (e.g. path-based http://localhost:PORT/{projectId}/handler/...). Those real URLs can stay untrusted despite this change.
Additional Locations (2)
Reviewed by Cursor Bugbot for commit cfb9af8. Configure here.
There was a problem hiding this comment.
Pull request overview
This PR updates the backend’s domain/redirect validation logic to automatically treat each project’s hosted handler domain (<project-id><suffix>, e.g. <project-id>.built-with-stack-auth.com) as trusted without requiring explicit tenancy configuration, aligning redirect URL validation, OAuth redirect URI registration, and Turnstile hostname checks.
Changes:
- Add
getHostedHandlerDomainSuffix()and include the hosted handler HTTPS origin invalidateRedirectUrltrusted domains. - Always append the hosted handler
/handlerURL to OAuth clientredirectUris. - Allow the hosted handler hostname in Turnstile hostname validation.
- Add unit tests for hosted handler redirect URL trust behavior.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 5 comments.
| File | Description |
|---|---|
| apps/backend/src/lib/redirect-urls.tsx | Adds hosted handler suffix helper and auto-trusts hosted handler domain for redirect validation. |
| apps/backend/src/oauth/model.tsx | Adds hosted handler /handler URL to OAuth client redirect URIs. |
| apps/backend/src/lib/turnstile.tsx | Allows hosted handler hostname during Turnstile verification. |
| apps/backend/src/lib/redirect-urls.test.tsx | Adds tests to validate hosted handler domain redirect behavior. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
|
||
| /** | ||
| * Returns the domain suffix for the hosted handler (e.g. ".built-with-stack-auth.com" in | ||
| * production, ".localhost:8109" in local dev). | ||
| */ | ||
| export function getHostedHandlerDomainSuffix(): string { | ||
| return getEnvVariable("NEXT_PUBLIC_STACK_HOSTED_HANDLER_DOMAIN_SUFFIX", defaultHostedHandlerDomainSuffix); |
| const hostedDomain = `${tenancy.project.id}${getHostedHandlerDomainSuffix()}`; | ||
| return validateRedirectUrlAgainstTrustedDomains(urlOrString, { | ||
| allowLocalhost: tenancy.config.domains.allowLocalhost, | ||
| trustedDomains: Object.values(tenancy.config.domains.trustedDomains).map(domain => domain.baseUrl), | ||
| trustedDomains: [ | ||
| ...Object.values(tenancy.config.domains.trustedDomains).map(domain => domain.baseUrl), | ||
| `https://${hostedDomain}`, | ||
| ], |
| describe('hosted handler domain (built-with domain)', () => { | ||
| it('should trust the project hosted handler domain with default suffix', () => { | ||
| vi.stubEnv('NEXT_PUBLIC_STACK_HOSTED_HANDLER_DOMAIN_SUFFIX', '.built-with-stack-auth.com'); | ||
| const projectId = '12345678-1234-1234-1234-123456789012'; | ||
| const tenancy = createMockTenancy({ | ||
| domains: { | ||
| allowLocalhost: false, | ||
| trustedDomains: {}, | ||
| }, | ||
| }, projectId); | ||
|
|
||
| // HTTPS on the built-with domain should be trusted | ||
| expect(validateRedirectUrl(`https://${projectId}.built-with-stack-auth.com/callback`, tenancy)).toBe(true); | ||
| expect(validateRedirectUrl(`https://${projectId}.built-with-stack-auth.com/`, tenancy)).toBe(true); | ||
| expect(validateRedirectUrl(`https://${projectId}.built-with-stack-auth.com/handler/oauth-callback`, tenancy)).toBe(true); | ||
|
|
||
| // HTTP on the built-with domain should NOT be trusted | ||
| expect(validateRedirectUrl(`http://${projectId}.built-with-stack-auth.com/callback`, tenancy)).toBe(false); | ||
|
|
||
| // Different project IDs should NOT be trusted | ||
| expect(validateRedirectUrl('https://other-project.built-with-stack-auth.com/callback', tenancy)).toBe(false); | ||
|
|
||
| // Unrelated domains should NOT be trusted | ||
| expect(validateRedirectUrl('https://example.com/callback', tenancy)).toBe(false); | ||
|
|
||
| vi.unstubAllEnvs(); | ||
| }); |
| // The project's hosted handler domain (e.g. <project-id>.built-with-stack-auth.com) is always trusted | ||
| const hostedHandlerUrl = createUrlIfValid(`https://${tenancy.project.id}${getHostedHandlerDomainSuffix()}`); | ||
| if (hostedHandlerUrl != null && hostedHandlerUrl.hostname === hostname) { | ||
| return true; | ||
| } | ||
|
|
| // The project's hosted handler domain is always trusted | ||
| const hostedDomain = `${tenancy.project.id}${getHostedHandlerDomainSuffix()}`; | ||
| redirectUris.push(new URL("/handler", `https://${hostedDomain}`).toString()); | ||
|
|
Greptile SummaryThis PR automatically trusts each project's hosted handler domain (
Confidence Score: 3/5Safe to merge for HTTPS-only hosted-handler flows, but the localhost OAuth fallback in the OAuth model is now unreachable, which may silently break allowLocalhost-only projects depending on how the oauth2-server library behaves. The redirect-URL and Turnstile changes are clean and well-tested. The concern in oauth/model.tsx is that the unconditional hosted-domain push makes the allowLocalhost guard permanently dead code, so the localhost URI is never added to client.redirectUris for projects with no configured trusted domains. apps/backend/src/oauth/model.tsx — the allowLocalhost empty-list fallback block needs verification or removal. Important Files Changed
Prompt To Fix All With AIFix the following 2 code review issues. Work through them one at a time, proposing concise fixes.
---
### Issue 1 of 2
apps/backend/src/lib/redirect-urls.test.tsx:519-520
**"Wrong port" test case uses HTTP, not HTTPS — it tests the wrong thing**
The assertion passes because the protocol is HTTP, not because the port is wrong. Since only `https://…` is added to trusted domains, any HTTP URL returns `false` regardless of the port. The test comment says "Wrong port should NOT be trusted" but it never reaches port-matching logic. To actually verify the port-mismatch rejection, the URL should use `https://`.
### Issue 2 of 2
apps/backend/src/oauth/model.tsx:83-85
**Dead code — localhost fallback is never reached**
The hosted domain URI is pushed unconditionally on line 81, so `redirectUris.length` is always `>= 1` when this guard is evaluated — the `length === 0` condition can never be true and the localhost fallback is unreachable. For projects that rely on `allowLocalhost: true` with no configured trusted domains, the localhost URI no longer appears in `client.redirectUris`. If the oauth2-server library uses that list for auto-selection when no `redirect_uri` is included in the request, those OAuth flows would change behavior silently. The block should either be removed (if `validateRedirectUri` is fully authoritative) or repositioned so the localhost fallback is still added when appropriate.
Reviews (1): Last reviewed commit: "fix: only trust https:// for built-with ..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
|
|
||
| // Wrong port should NOT be trusted |
There was a problem hiding this comment.
The assertion passes because the protocol is HTTP, not because the port is wrong. Since only https://… is added to trusted domains, any HTTP URL returns false regardless of the port. The test comment says "Wrong port should NOT be trusted" but it never reaches port-matching logic. To actually verify the port-mismatch rejection, the URL should use https://.
Prompt To Fix With AI
This is a comment left during a code review.
Path: apps/backend/src/lib/redirect-urls.test.tsx
Line: 519-520
Comment:
**"Wrong port" test case uses HTTP, not HTTPS — it tests the wrong thing**
The assertion passes because the protocol is HTTP, not because the port is wrong. Since only `https://…` is added to trusted domains, any HTTP URL returns `false` regardless of the port. The test comment says "Wrong port should NOT be trusted" but it never reaches port-matching logic. To actually verify the port-mismatch rejection, the URL should use `https://`.
How can I resolve this? If you propose a fix, please make it concise.| if (redirectUris.length === 0 && tenancy.config.domains.allowLocalhost) { | ||
| redirectUris.push("http://localhost"); | ||
| } |
There was a problem hiding this comment.
The hosted domain URI is pushed unconditionally on line 81, so redirectUris.length is always >= 1 when this guard is evaluated — the length === 0 condition can never be true and the localhost fallback is unreachable. For projects that rely on allowLocalhost: true with no configured trusted domains, the localhost URI no longer appears in client.redirectUris. If the oauth2-server library uses that list for auto-selection when no redirect_uri is included in the request, those OAuth flows would change behavior silently. The block should either be removed (if validateRedirectUri is fully authoritative) or repositioned so the localhost fallback is still added when appropriate.
Prompt To Fix With AI
This is a comment left during a code review.
Path: apps/backend/src/oauth/model.tsx
Line: 83-85
Comment:
**Dead code — localhost fallback is never reached**
The hosted domain URI is pushed unconditionally on line 81, so `redirectUris.length` is always `>= 1` when this guard is evaluated — the `length === 0` condition can never be true and the localhost fallback is unreachable. For projects that rely on `allowLocalhost: true` with no configured trusted domains, the localhost URI no longer appears in `client.redirectUris`. If the oauth2-server library uses that list for auto-selection when no `redirect_uri` is included in the request, those OAuth flows would change behavior silently. The block should either be removed (if `validateRedirectUri` is fully authoritative) or repositioned so the localhost fallback is still added when appropriate.
How can I resolve this? If you propose a fix, please make it concise.
Automatically accept
<project-id>.built-with-stack-auth.com(or whatever the configured hosted handler domain suffix is) as a trusted domain for each project, without requiring it to be explicitly added in the project config.Changes:
redirect-urls.tsx: Include hosted handler domain (both HTTP and HTTPS) in trusted domains for redirect URL validation — this covers OAuth callbacks, passkeys, and all other redirect checksturnstile.tsx: Accept the hosted handler hostname for Turnstile verificationoauth/model.tsx: Include hosted handler URL in OAuth client redirect URIsLink to Devin session: https://app.devin.ai/sessions/11f027a384674ca9b703b14db1a80d46
Requested by: @N2D4
Note
Medium Risk
Expands the set of automatically trusted redirect/hostname origins based on an env-configured suffix and project ID, affecting OAuth redirect validation and bot-challenge hostname checks. Risk is moderate because mistakes could unintentionally widen redirect URI acceptance.
Overview
Automatically trusts the project hosted handler domain (
<project-id>${NEXT_PUBLIC_STACK_HOSTED_HANDLER_DOMAIN_SUFFIX}) without requiring it in tenancy config.validateRedirectUrlnow always includeshttps://<project-id>.<suffix>in its trusted domain list (with a newgetHostedHandlerDomainSuffix()helper),OAuthModel.getClientalways adds the hosted handler/handlerURL toredirectUris, and Turnstile hostname validation now treats the hosted handler hostname as allowed.Adds unit tests covering default/custom suffix behavior and ensuring only the correct project/HTTPS hosted domain is accepted.
Reviewed by Cursor Bugbot for commit cfb9af8. Bugbot is set up for automated code reviews on this repo. Configure here.
Summary by CodeRabbit
New Features
Tests