Skip to content

Commit 1833d92

Browse files
committed
fix(favicon): resolve metadata at request time, fully honor whitelabel favicon
Addresses the second round of review findings on PR #5588: - Cursor (High): app/layout.tsx exported a static `export const metadata = generateBrandedMetadata()`. Next.js treats a static metadata object as truly constant (eligible for build-time resolution on any statically-rendered route sharing the layout), so it doesn't know this one secretly depends on process.env via getDeploymentEnv() - same bake-in risk already fixed for next.config.ts. Converted to generateMetadata(), the documented way to force per-request resolution. - Greptile (P1): the earlier whitelabel fallback fixed the *tint* but not the underlying precedence gap - Sim's exact-size PNG/apple entries were still emitted alongside the tenant's generic sizes:'any' entry, and browsers can prefer the more specific size match. Now the tenant favicon fully replaces every icon slot instead of being appended alongside Sim's. - Cursor (Medium): app/api/favicon/route.ts never checked brand.faviconUrl, so a whitelabeled dev/staging deployment still served Sim's tinted icon at the legacy /favicon.ico path. Now checks brand.faviconUrl first, matching generateBrandedMetadata()'s precedence.
1 parent 944c5e4 commit 1833d92

3 files changed

Lines changed: 36 additions & 19 deletions

File tree

apps/sim/app/api/favicon/route.ts

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
import { type NextRequest, NextResponse } from 'next/server'
22
import { getDeploymentEnv } from '@/lib/core/config/env-flags'
33
import { withRouteHandler } from '@/lib/core/utils/with-route-handler'
4+
import { getBrandConfig } from '@/ee/whitelabeling/branding'
45

56
export const dynamic = 'force-dynamic'
67

@@ -21,8 +22,13 @@ const FAVICON_DESTINATIONS: Record<ReturnType<typeof getDeploymentEnv>, string>
2122
* here, at request time, not in `next.config.ts`, which only runs once during
2223
* that shared build and would freeze whichever tier happened to be active
2324
* then (never the tier the container actually ends up running as).
25+
*
26+
* A whitelabeled deployment's own favicon always wins here too, matching
27+
* `generateBrandedMetadata()` (`ee/whitelabeling/metadata.ts`) — a tenant on
28+
* a dev/staging environment should never see Sim's internal tinted mark.
2429
*/
2530
export const GET = withRouteHandler(async (request: NextRequest) => {
26-
const destination = FAVICON_DESTINATIONS[getDeploymentEnv()]
31+
const brand = getBrandConfig()
32+
const destination = brand.faviconUrl || FAVICON_DESTINATIONS[getDeploymentEnv()]
2733
return NextResponse.redirect(new URL(destination, request.url))
2834
})

apps/sim/app/layout.tsx

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,19 @@ export const viewport: Viewport = {
2424
],
2525
}
2626

27-
export const metadata: Metadata = generateBrandedMetadata()
27+
/**
28+
* Not a static `export const metadata` object: {@link generateBrandedMetadata}
29+
* reads `getDeploymentEnv()` for the favicon set, which depends on
30+
* `process.env` being populated by `bootstrap.ts`'s runtime secrets load. A
31+
* static object is resolved once at module-evaluation time and Next.js treats
32+
* it as truly constant (eligible for build-time resolution on any
33+
* statically-rendered route sharing this layout) — `generateMetadata()`
34+
* forces the same per-request resolution `app/api/favicon/route.ts` already
35+
* uses.
36+
*/
37+
export function generateMetadata(): Metadata {
38+
return generateBrandedMetadata()
39+
}
2840

2941
const GTM_ID = 'GTM-T7PHSRX5' as const
3042
const GA_ID = 'G-DR7YBE70VS' as const

apps/sim/ee/whitelabeling/metadata.ts

Lines changed: 16 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -139,23 +139,22 @@ export function generateBrandedMetadata(override: Partial<Metadata> = {}): Metad
139139
},
140140
manifest: '/manifest.webmanifest',
141141
icons: {
142-
icon: [
143-
...(brand.faviconUrl ? [] : [{ url: icons.svg, type: 'image/svg+xml', sizes: 'any' }]),
144-
{ url: icons.favicon16, sizes: '16x16', type: 'image/png' },
145-
{ url: icons.favicon32, sizes: '32x32', type: 'image/png' },
146-
{
147-
url: icons.android192,
148-
sizes: '192x192',
149-
type: 'image/png',
150-
},
151-
{
152-
url: icons.android512,
153-
sizes: '512x512',
154-
type: 'image/png',
155-
},
156-
...(brand.faviconUrl ? [{ url: brand.faviconUrl, sizes: 'any', type: 'image/png' }] : []),
157-
],
158-
apple: icons.apple,
142+
// A whitelabeled deployment only ever supplies one favicon URL (no
143+
// multi-size set of its own) - emitting Sim's exact-size PNG/apple
144+
// entries alongside it would let browsers prefer those more specific
145+
// sizes over the tenant's generic `sizes: 'any'` entry, showing Sim
146+
// branding instead. So the tenant favicon fully replaces every slot
147+
// rather than being appended to Sim's.
148+
icon: brand.faviconUrl
149+
? [{ url: brand.faviconUrl, sizes: 'any', type: 'image/png' }]
150+
: [
151+
{ url: icons.svg, type: 'image/svg+xml', sizes: 'any' },
152+
{ url: icons.favicon16, sizes: '16x16', type: 'image/png' },
153+
{ url: icons.favicon32, sizes: '32x32', type: 'image/png' },
154+
{ url: icons.android192, sizes: '192x192', type: 'image/png' },
155+
{ url: icons.android512, sizes: '512x512', type: 'image/png' },
156+
],
157+
apple: brand.faviconUrl || icons.apple,
159158
shortcut: brand.faviconUrl || icons.svg,
160159
},
161160
appleWebApp: {

0 commit comments

Comments
 (0)