fix(web): unoptimize external next/image srcs instead of a host wildcard - #8501
Merged
pandeymangg merged 8 commits intoJul 23, 2026
Conversation
next.config.mjs built a curated images.remotePatterns allowlist and
then unconditionally appended { protocol: https, hostname: '**' },
turning the image optimizer into an open proxy (bandwidth/CPU abuse,
SSRF-adjacent surface, optimizer cache poisoning).
- remove the unconditional wildcard
- derive instance-specific hosts (WEBAPP_URL, PUBLIC_URL,
S3_ENDPOINT_URL) from the environment, deduped against the allowlist
- uploaded files are unaffected: they are served from same-origin
/storage/... paths which never consult remotePatterns
- deployments that render user-provided image URLs from arbitrary
hosts can opt back in with IMAGES_ALLOW_ALL_HTTPS_HOSTS=1
(documented in .env.example, hashed via turbo build.env)
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
Contributor
WalkthroughAdded a shared image-host allowlist and 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 Checkov (3.3.8)turbo.jsonTraceback (most recent call last): 🔧 ESLint
ESLint install timed out. The project may have too many dependencies for the sandbox. 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 |
mattinannt
marked this pull request as draft
July 13, 2026 12:31
The previous approach rebuilt images.remotePatterns from WEBAPP_URL / PUBLIC_URL / S3_ENDPOINT_URL at config-eval time plus an IMAGES_ALLOW_ALL_HTTPS_HOSTS=1 opt-in wildcard. That can't work for Formbricks' deployment model: next.config (incl. remotePatterns) is frozen into the build (required-server-files.json), the same image serves multiple domains (app./ksa.formbricks.com + every self-hoster), and the wildcard's real purpose is arbitrary user-provided external image URLs — unknowable at build time. New model (ENG-1678): - lib/image-hosts.ts is the single source of truth for optimizable hosts (universal provider/CDN hosts only; NOT the deployment domain). next.config builds remotePatterns from it via jiti. - isExternalImageSrc() decides per <Image>: relative /storage paths and allowlisted hosts optimize; any other absolute URL renders , so the optimizer never proxies arbitrary hosts. - Applied unoptimized to the ~14 dynamic <Image> sites (backgrounds, logos, picture choices, file previews, integration icons). - Removed the env-derivation + wildcard, the app.formbricks.com entry, and IMAGES_ALLOW_ALL_HTTPS_HOSTS (turbo.json build.env + .env.example). Works identically on both clouds and every self-host with zero config; first-party /storage uploads (relative) still optimize via localPatterns and never need the running domain in remotePatterns. Verified: build bakes only the curated hosts, no '**'; pnpm lint/test/ build green; new lib/image-hosts.test.ts covers the predicate. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Entire-Checkpoint: 33605bfa81a8
mattinannt
changed the title
No conflicts. Keeps this PR's isExternalImageSrc/unoptimized approach on top of current main. Entire-Checkpoint: 2bab7c2c0e4a
mattinannt
marked this pull request as ready for review
July 21, 2026 12:34
formbricks-cdn.s3.eu-central-1.amazonaws.com is Formbricks-Cloud infrastructure and is referenced only by test/mock fixtures (a Playwright file-upload helper that fetch()es sample images, and i18n.mock.ts) — no product feature serves images from it. Uploaded files (cloud and self-hosted alike) are served same-origin via relative /storage/... paths, so no deployment's S3 host ever needs to be in remotePatterns. The allowlist now contains only universal provider hosts real features use: OAuth avatars and the Unsplash picker. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Entire-Checkpoint: 8e2ce98eae3f
Member
Author
|
@coderabbitai please review |
Contributor
|
✅ Action performedReview finished.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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/modules/ui/components/media-background/index.tsx`:
- Line 119: Update both Image usages in
apps/web/modules/ui/components/media-background/index.tsx at lines 119-119 and
155-155: replace layout="fill" with fill, move objectFit into style while
preserving the existing cover and filter styling, and replace onLoadingComplete
with onLoad so both background image cases use modern Next.js props.
🪄 Autofix (Beta)
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: Repository UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: f0a537c7-9a78-4379-920a-3acabf120297
📒 Files selected for processing (17)
apps/web/app/(app)/workspaces/[workspaceId]/surveys/[surveyId]/(analysis)/summary/components/PictureChoiceSummary.tsxapps/web/lib/image-hosts.test.tsapps/web/lib/image-hosts.tsapps/web/modules/ee/whitelabel/email-customization/components/email-customization-settings.tsxapps/web/modules/ee/whitelabel/favicon-customization/components/favicon-customization-settings.tsxapps/web/modules/survey/editor/components/logo-settings-card.tsxapps/web/modules/survey/editor/components/option-ids.tsxapps/web/modules/survey/editor/components/unsplash-images.tsxapps/web/modules/ui/components/client-logo/index.tsxapps/web/modules/ui/components/connect-integration/index.tsxapps/web/modules/ui/components/file-input/index.tsxapps/web/modules/ui/components/input-combo-box/index.tsxapps/web/modules/ui/components/media-background/index.tsxapps/web/modules/ui/components/picture-selection-response/index.tsxapps/web/modules/workspaces/settings/look/components/edit-logo.tsxapps/web/next.config.mjsturbo.json
SonarQube (S1874): the callable jiti(id) signature is deprecated. The image-host allowlist is just a string array, so it doesn't need TS transpilation at config load — moved it to a plain lib/optimizable-image- hosts.mjs that next.config.mjs static-imports and lib/image-hosts.ts re-exports. Single source of truth preserved; no jiti in this path. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Entire-Checkpoint: a0faf2956b57
Both background <Image> usages used legacy next/image props that Next 16 deprecates (and layout/objectFit are ignored by the modern component): - layout="fill" -> fill - objectFit="cover" -> moved into style (merged with the existing filter) - onLoadingComplete -> onLoad Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Entire-Checkpoint: 3d04bff84ec5
|
pandeymangg
approved these changes
Jul 23, 2026
pandeymangg
deleted the
matti/eng-1678-stop-nextimage-from-allowing-arbitrary-remote-hosts
branch
July 23, 2026 07:48
9 tasks
10 tasks
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
2 participants
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
Removes the
images.remotePatternshostname: "**"wildcard from the Next.js image optimizer and replaces it with a model that is safe by default and needs no per-deployment configuration.Why
With the wildcard,
next/imagewould fetch and optimize an image from any HTTPS host — effectively an open proxy (SSRF-adjacent surface, bandwidth/CPU abuse, optimizer-cache poisoning). It can't simply be replaced with a static allowlist, because:next.config(includingremotePatterns) is frozen into the build — the same image servesapp.formbricks.com,ksa.formbricks.com, and every self-hoster's own domain, so the running domain can't be baked in.How
The optimizer is only ever pointed at hosts we control; everything else is served directly (unoptimized), so the optimizer is never a proxy for hosts we don't control.
apps/web/lib/image-hosts.tsis the single source of truth:OPTIMIZABLE_IMAGE_HOSTS— the universal provider/CDN hosts that are identical on every deployment (GitHub/Slack/Google avatars, Unsplash, the cloud CDN bucket). The deployment's own domain is intentionally not listed.isExternalImageSrc(src)—false(optimize) for relative paths,data:URIs, static imports, and allowlisted hosts;true(bypass the optimizer) for any other absolute URL.next.config.mjsbuildsimages.remotePatternsfromOPTIMIZABLE_IMAGE_HOSTS.<Image>sites (backgrounds, logos, picture choices, file previews, integration icons) setunoptimized={isExternalImageSrc(src)}.First-party uploads need no allowlist entry: they're served from same-origin
/storage/...(relative) paths, which Next treats as local images (localPatterns, optimized by default) and never checks againstremotePatterns. That's why the running domain doesn't need to be listed and the same build works on any domain.Result: first-party
/storageuploads and known providers are optimized; arbitrary external images render directly (no optimization, but no proxy risk); zero per-deployment config — identical behavior on both clouds and every self-host.Validation
**(verified in.next/required-server-files.json).pnpm build(12/12),pnpm lint, andpnpm test(22/22) pass.apps/web/lib/image-hosts.test.tscoversisExternalImageSrc.Fixes ENG-1678
🤖 Generated with Claude Code