fix: prevent internal origins from leaking into public URLs - #32
Merged
Conversation
📝 WalkthroughWalkthroughThe change centralizes public-origin resolution through ChangesPublic origin and URL handling
Estimated code review effort: 3 (Moderate) | ~25 minutes 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 2📝 Generate docstrings 💡
🛠️ Fix failing CI checks 💡
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 |
Molret
marked this pull request as ready for review
August 12, 2026 07:14
|
📚 No documentation changes were needed for this PR. |
Strix Security ReviewNo security issues found. Updated for Reviewed by Strix |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/web/src/lib/public-origin.ts (1)
26-30: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winUse the documented fallback chain for every public URL.
loadHarlyConfig()supportsNEXT_PUBLIC_APP_URLandBETTER_AUTH_URLwhenHARLY_URLis absent.getHarlyPublicOrigin()omitsBETTER_AUTH_URL.publicRedirectUrl()andpublicBaseUrl()omit both fallbacks. A supported legacy deployment can therefore generate proxy-origin redirects or fail public URL generation.
apps/web/src/lib/public-origin.ts#L26-L30: addBETTER_AUTH_URLbeforeDEFAULT_PUBLIC_ORIGIN.apps/web/src/lib/public-origin.test.ts#L23-L44: add coverage for aBETTER_AUTH_URL-only configuration.apps/web/src/proxy.ts#L68-L70: use the same configured-origin fallback chain before falling back torequest.nextUrl.origin.apps/web/src/server/scim/http.ts#L25-L26: use the same configured-origin fallback chain before usingrequest.nextUrl.origin.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/web/src/lib/public-origin.ts` around lines 26 - 30, Use the documented configured-origin fallback chain consistently: update getHarlyPublicOrigin() in apps/web/src/lib/public-origin.ts#L26-L30 to check BETTER_AUTH_URL after NEXT_PUBLIC_APP_URL, add BETTER_AUTH_URL-only coverage in apps/web/src/lib/public-origin.test.ts#L23-L44, and update apps/web/src/proxy.ts#L68-L70 and apps/web/src/server/scim/http.ts#L25-L26 to use the same chain before request.nextUrl.origin.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@apps/web/src/app/layout.tsx`:
- Line 39: Ensure the public origin is resolved from the final HARLY_URL rather
than the build-time placeholder: update metadataBase in
apps/web/src/app/layout.tsx, resolve the origin inside robots() and export
dynamic = "force-dynamic" in apps/web/src/app/robots.ts, and make no change to
apps/web/src/app/sitemap.ts because its existing dynamic configuration already
handles this.
In `@apps/web/src/features/workspaces/actions.ts`:
- Around line 428-436: Move each getHarlyPublicOrigin() call in the affected
workspace actions before its corresponding membership insert, invitation insert,
or invitation-expiry update, so origin resolution completes before any database
mutation. Reuse the resolved appUrl when constructing the related WelcomeEmail
or invitation email URLs, including the flows around sendWorkspaceEmail at the
shown locations.
In `@packages/config/src/index.ts`:
- Around line 75-82: Update the production host validation in
packages/config/src/index.ts (75-82) to reject all 127.0.0.0/8 addresses and
hostnames ending in .localhost; update apps/web/src/lib/public-origin.ts (6-17)
to reject the full 127.0.0.0/8 range. Add production rejection cases for
https://127.0.0.2 and https://tenant.localhost in
packages/config/src/index.test.ts (28-38), and for https://127.0.0.2 in
apps/web/src/lib/public-origin.test.ts (23-44).
---
Outside diff comments:
In `@apps/web/src/lib/public-origin.ts`:
- Around line 26-30: Use the documented configured-origin fallback chain
consistently: update getHarlyPublicOrigin() in
apps/web/src/lib/public-origin.ts#L26-L30 to check BETTER_AUTH_URL after
NEXT_PUBLIC_APP_URL, add BETTER_AUTH_URL-only coverage in
apps/web/src/lib/public-origin.test.ts#L23-L44, and update
apps/web/src/proxy.ts#L68-L70 and apps/web/src/server/scim/http.ts#L25-L26 to
use the same chain before request.nextUrl.origin.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: e4d26f3d-b751-4af9-92e1-04ef7f090871
📒 Files selected for processing (44)
Dockerfileapps/web/src/app/(dashboard)/settings/developers/page.tsxapps/web/src/app/(dashboard)/settings/email/replies/page.tsxapps/web/src/app/(dashboard)/settings/portal/page.tsxapps/web/src/app/(fullscreen)/dashboard/jobs/[jobId]/page.tsxapps/web/src/app/api/integrations/google/callback/route.tsapps/web/src/app/api/integrations/outlook/callback/route.tsapps/web/src/app/api/integrations/outlook/install/route.tsapps/web/src/app/api/integrations/slack/callback/route.tsapps/web/src/app/api/integrations/slack/install/route.tsapps/web/src/app/api/integrations/zoom/callback/route.tsapps/web/src/app/api/integrations/zoom/install/route.tsapps/web/src/app/api/portal/auth/callback/github/route.tsapps/web/src/app/api/portal/auth/callback/google/route.tsapps/web/src/app/api/portal/auth/callback/linkedin/route.tsapps/web/src/app/api/portal/auth/route.tsapps/web/src/app/api/public/v1/image/presign/route.tsapps/web/src/app/api/public/v1/resume/presign/route.test.tsapps/web/src/app/api/public/v1/resume/presign/route.tsapps/web/src/app/api/setup/claim/route.tsapps/web/src/app/api/setup/complete/route.tsapps/web/src/app/api/v1/openapi.json/route.tsapps/web/src/app/layout.tsxapps/web/src/app/robots.tsapps/web/src/app/sitemap.tsapps/web/src/features/auth/actions.tsapps/web/src/features/jobs/service.tsapps/web/src/features/portal/actions.tsapps/web/src/features/workspaces/ReplyHandlingSettingsCard.tsxapps/web/src/features/workspaces/actions.tsapps/web/src/features/workspaces/cal-settings-actions.tsapps/web/src/lib/email/branding.tsapps/web/src/lib/email/outbox-processor.tsapps/web/src/lib/gcal/config.tsapps/web/src/lib/passkey.tsapps/web/src/lib/public-origin.test.tsapps/web/src/lib/public-origin.tsapps/web/src/proxy.tsapps/web/src/server/notify/dispatch.tsapps/web/src/server/notify/slack.tsapps/web/src/server/scim/http.tspackages/config/README.mdpackages/config/src/index.test.tspackages/config/src/index.ts
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.
What changed
HARLY_URLfor upload URLs, file URLs, OAuth callbacks, redirects, metadata, webhooks, notifications, and integrations.localhost, loopback, or unspecified bind addresses.https://build.invalidplaceholder for the production build only.https://0.0.0.0:3000presign failure.Root cause
Public presign routes used
request.urlto make relative local-storage URLs absolute. Behind the reverse proxy, that request origin could be0.0.0.0:3000, so the browser attempted the upload against an internal address and reported a CORS failure with status null.Validation
pnpm --filter web test— 189 files passed, 871 tests passed; 13 files skipped by integration configuration.pnpm --filter web typecheckpnpm --filter @harly/config typecheckpnpm --filter web exec eslint ...Summary by CodeRabbit
Greptile Summary
The PR centralizes externally visible URL construction around the configured Harly public origin and strengthens production validation against local and bind addresses.
HARLY_URLfor presigned files, OAuth callbacks, redirects, metadata, notifications, webhooks, and integrations.Confidence Score: 5/5
The PR appears safe to merge.
No blocking failure remains.
Important Files Changed
Flowchart
%%{init: {'theme': 'neutral'}}%% flowchart LR E[HARLY_URL] --> V[Production origin validation] V --> O[getHarlyPublicOrigin] O --> U[Upload and file URLs] O --> A[OAuth callbacks and redirects] O --> M[Metadata and public pages] O --> N[Notifications and webhooks] B[Production build] --> P[https://build.invalid placeholder] R[Runtime requests] --> OReviews (2): Last reviewed commit: "fix: address PR origin review feedback" | Re-trigger Greptile