Skip to content

Commit 37a3b05

Browse files
committed
fix(favicon): close static/ISR bake-in gap, fix manifest whitelabel leak
Addresses the third round of review findings on PR #5588: - Cursor (Medium): generateMetadata() alone does not force per-request evaluation - only the overall page being dynamic does, and a plain process.env read is not a Next.js "dynamic signal" Next can detect. Verified this doesn't matter for any route TODAY (0 of 1007 real page routes are static/ISR-eligible - only non-HTML RSS/sitemap/robots/OG files are), but it was a latent gap: any future static or ISR page added under the root layout would silently get build-time (production-tier) favicon metadata baked in, with no error. Added `export const dynamic = 'force-dynamic'` to app/layout.tsx, matching the same declaration already on app/manifest.ts and app/api/favicon/route.ts. Verified via full build that the 9 legitimately-static routes (RSS/sitemap/robots/OG-image files, which don't render through the root layout's page tree) are unaffected - same count, same routes, before and after. - Greptile (P1, reported as two separate threads): app/manifest.ts (PWA install/home-screen icons) never checked brand.faviconUrl at all, so a whitelabeled deployment would still show Sim's icon when installed even after the HTML favicon was fixed. Now uses brand.faviconUrl when set, same precedence as generateBrandedMetadata(). - Cursor (Low): the `brand.faviconUrl ? 'production' : getDeploymentEnv()` ternary computed a value never actually read when brand.faviconUrl is set (every consumer already short-circuits on brand.faviconUrl first). Simplified to always resolve the environment tier.
1 parent 5412f70 commit 37a3b05

3 files changed

Lines changed: 45 additions & 23 deletions

File tree

apps/sim/app/layout.tsx

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

27+
/**
28+
* Forces every route under this layout to render dynamically. Every real page
29+
* in this app already renders dynamically today (session/workspace state), so
30+
* this changes nothing currently — but it's the difference between "happens
31+
* to be true" and "guaranteed": {@link generateBrandedMetadata} reads
32+
* `getDeploymentEnv()` for the favicon set, which depends on `process.env`
33+
* being populated by `bootstrap.ts`'s runtime secrets load, and
34+
* `generateMetadata()` alone does not force per-request evaluation — Next
35+
* still prerenders it at build time for any route that's otherwise static or
36+
* ISR-eligible. Without this, a future static/ISR page added under this
37+
* layout would silently get build-time (production-tier) favicon metadata
38+
* baked in, with no error. Matches the `force-dynamic` already declared on
39+
* `app/manifest.ts` and `app/api/favicon/route.ts`.
40+
*/
41+
export const dynamic = 'force-dynamic'
42+
2743
/**
2844
* Not a static `export const metadata` object: {@link generateBrandedMetadata}
2945
* reads `getDeploymentEnv()` for the favicon set, which depends on
@@ -32,7 +48,9 @@ export const viewport: Viewport = {
3248
* it as truly constant (eligible for build-time resolution on any
3349
* statically-rendered route sharing this layout) — `generateMetadata()`
3450
* forces the same per-request resolution `app/api/favicon/route.ts` already
35-
* uses.
51+
* uses. Paired with the `dynamic = 'force-dynamic'` export above, which
52+
* guarantees that resolution actually happens per-request rather than only
53+
* "happening to" because every current page is otherwise dynamic.
3654
*/
3755
export function generateMetadata(): Metadata {
3856
return generateBrandedMetadata()

apps/sim/app/manifest.ts

Lines changed: 22 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -20,23 +20,28 @@ export default function manifest(): MetadataRoute.Manifest {
2020
background_color: '#ffffff',
2121
theme_color: brand.theme?.primaryColor || '#33C482',
2222
orientation: 'portrait-primary',
23-
icons: [
24-
{
25-
src: '/favicon/android-chrome-192x192.png',
26-
sizes: '192x192',
27-
type: 'image/png',
28-
},
29-
{
30-
src: '/favicon/android-chrome-512x512.png',
31-
sizes: '512x512',
32-
type: 'image/png',
33-
},
34-
{
35-
src: '/favicon/apple-touch-icon.png',
36-
sizes: '180x180',
37-
type: 'image/png',
38-
},
39-
],
23+
// A whitelabeled deployment's install/home-screen icon should be the
24+
// tenant's own brand, not Sim's — same precedence as the HTML favicon
25+
// slots in generateBrandedMetadata() (ee/whitelabeling/metadata.ts).
26+
icons: brand.faviconUrl
27+
? [{ src: brand.faviconUrl, sizes: 'any', type: 'image/png' }]
28+
: [
29+
{
30+
src: '/favicon/android-chrome-192x192.png',
31+
sizes: '192x192',
32+
type: 'image/png',
33+
},
34+
{
35+
src: '/favicon/android-chrome-512x512.png',
36+
sizes: '512x512',
37+
type: 'image/png',
38+
},
39+
{
40+
src: '/favicon/apple-touch-icon.png',
41+
sizes: '180x180',
42+
type: 'image/png',
43+
},
44+
],
4045
categories: ['productivity', 'developer', 'business'],
4146
shortcuts: [
4247
{

apps/sim/ee/whitelabeling/metadata.ts

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -53,11 +53,10 @@ export const ICON_SETS: Record<ReturnType<typeof getDeploymentEnv>, FaviconSet>
5353
*/
5454
export function generateBrandedMetadata(override: Partial<Metadata> = {}): Metadata {
5555
const brand = getBrandConfig()
56-
// A whitelabeled deployment's favicon slots should read as the tenant's own
57-
// brand, never an internal Sim dev/staging color — fall back to production's
58-
// neutral assets (matching pre-environment-favicon behavior) for the sizes
59-
// `brand.faviconUrl` doesn't cover, instead of the environment-tinted set.
60-
const icons = ICON_SETS[brand.faviconUrl ? 'production' : getDeploymentEnv()]
56+
// Only read when brand.faviconUrl is unset — every icon/apple/shortcut slot
57+
// below short-circuits on brand.faviconUrl first when it's set, so this
58+
// never needs to reflect the whitelabel case.
59+
const icons = ICON_SETS[getDeploymentEnv()]
6160

6261
const defaultTitle = brand.name
6362
const summaryFull = `Sim is the open-source AI workspace where teams build, deploy, and manage AI agents. Connect 1,000+ integrations and every major LLM to create agents that automate real work — visually, conversationally, or with code. Trusted by over 100,000 builders — from startups to Fortune 500 companies. SOC2 compliant.`

0 commit comments

Comments
 (0)