Skip to content

Commit 944c5e4

Browse files
committed
fix(favicon): resolve deployment tier at request time, not build time
Addresses review findings on PR #5588: - Cursor (Medium): next.config.ts's rewrites() runs once during the shared, environment-agnostic Docker build (docker/app.Dockerfile builds with dummy env values; bootstrap.ts hydrates process.env from AWS Secrets Manager only at container boot, after the build already ran) - so resolving getDeploymentEnv() inside next.config.ts baked in whichever tier happened to be active during that shared build, never the tier the container actually runs as. Fixed by rewriting /favicon.ico to a stable path (/api/favicon) and resolving the destination inside that route handler instead, which runs per-request at runtime like generateBrandedMetadata() already correctly does. Marked force-dynamic per Next's own docs: generated icon routes are statically cached by default unless they use a request-time API or dynamic config. - Greptile (P1): a whitelabeled deployment's favicon slots (16/32/192/512 PNGs, apple-touch-icon) fell back to the environment-tinted set instead of neutral branding when brand.faviconUrl was set. Now falls back to the production (neutral) asset set specifically for whitelabeled deployments, matching pre-PR behavior for those slots. - Greptile (P2, ico-serves-svg): verified via `git show origin/staging` that /favicon.ico -> /icon.svg is pre-existing, unchanged production behavior from before this PR - not addressed here, out of scope. Also: added app/api/favicon/route.ts to the check:api-validation INDIRECT_ZOD_ROUTES allowlist (no client-supplied input - only reads the server's own deployment-tier env var) and bumped the audit baseline.
1 parent 1682470 commit 944c5e4

4 files changed

Lines changed: 43 additions & 18 deletions

File tree

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

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,28 @@
1+
import { type NextRequest, NextResponse } from 'next/server'
2+
import { getDeploymentEnv } from '@/lib/core/config/env-flags'
3+
import { withRouteHandler } from '@/lib/core/utils/with-route-handler'
4+
5+
export const dynamic = 'force-dynamic'
6+
7+
const FAVICON_DESTINATIONS: Record<ReturnType<typeof getDeploymentEnv>, string> = {
8+
development: '/icon-dev.svg',
9+
staging: '/icon-staging.svg',
10+
production: '/icon.svg',
11+
}
12+
13+
/**
14+
* Redirect target for the legacy `/favicon.ico` path (rewritten here in
15+
* `next.config.ts`), resolved per-request rather than baked into
16+
* `next.config.ts`'s `rewrites()` directly. This app's Docker image is built
17+
* once with dummy env values and promoted through dev/staging/production —
18+
* `bootstrap.ts` hydrates `process.env` from AWS Secrets Manager at container
19+
* boot, after the build already ran (`docker/app.Dockerfile`) — so a
20+
* same-process-lifetime decision like `getDeploymentEnv()` must be evaluated
21+
* here, at request time, not in `next.config.ts`, which only runs once during
22+
* that shared build and would freeze whichever tier happened to be active
23+
* then (never the tier the container actually ends up running as).
24+
*/
25+
export const GET = withRouteHandler(async (request: NextRequest) => {
26+
const destination = FAVICON_DESTINATIONS[getDeploymentEnv()]
27+
return NextResponse.redirect(new URL(destination, request.url))
28+
})

apps/sim/ee/whitelabeling/metadata.ts

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -52,7 +52,11 @@ const ICON_SETS: Record<ReturnType<typeof getDeploymentEnv>, FaviconSet> = {
5252
*/
5353
export function generateBrandedMetadata(override: Partial<Metadata> = {}): Metadata {
5454
const brand = getBrandConfig()
55-
const icons = ICON_SETS[getDeploymentEnv()]
55+
// A whitelabeled deployment's favicon slots should read as the tenant's own
56+
// brand, never an internal Sim dev/staging color — fall back to production's
57+
// neutral assets (matching pre-environment-favicon behavior) for the sizes
58+
// `brand.faviconUrl` doesn't cover, instead of the environment-tinted set.
59+
const icons = ICON_SETS[brand.faviconUrl ? 'production' : getDeploymentEnv()]
5660

5761
const defaultTitle = brand.name
5862
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.`

apps/sim/next.config.ts

Lines changed: 5 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
import path from 'node:path'
22
import type { NextConfig } from 'next'
33
import { env, isTruthy } from './lib/core/config/env'
4-
import { getDeploymentEnv, isDev } from './lib/core/config/env-flags'
4+
import { isDev } from './lib/core/config/env-flags'
55
import {
66
getChatEmbedCSPPolicy,
77
getMainCSPPolicy,
@@ -26,19 +26,6 @@ const minimalRegistryAlias: Record<string, string> = useMinimalRegistry
2626
}
2727
: {}
2828

29-
/**
30-
* Per-environment favicon for `/favicon.ico` requests (the legacy path browsers
31-
* probe automatically) — same wordmark mark as production, different background
32-
* color, so a dev/staging tab is never mistaken for prod at a glance. The
33-
* `<link rel="icon">` tags Next renders from `generateBrandedMetadata()`
34-
* (`ee/whitelabeling/metadata.ts`) use the same per-environment set.
35-
*/
36-
const FAVICON_ICO_DESTINATIONS: Record<ReturnType<typeof getDeploymentEnv>, string> = {
37-
development: '/icon-dev.svg',
38-
staging: '/icon-staging.svg',
39-
production: '/icon.svg',
40-
}
41-
4229
const nextConfig: NextConfig = {
4330
devIndicators: false,
4431
poweredByHeader: false,
@@ -450,8 +437,11 @@ const nextConfig: NextConfig = {
450437
async rewrites() {
451438
return [
452439
{
440+
// Resolved per-request in app/api/favicon/route.ts, not here — see
441+
// that file's TSDoc for why: this config only runs once during the
442+
// shared, environment-agnostic Docker build.
453443
source: '/favicon.ico',
454-
destination: FAVICON_ICO_DESTINATIONS[getDeploymentEnv()],
444+
destination: '/api/favicon',
455445
},
456446
{
457447
source: '/r/:shortCode',

scripts/check-api-validation-contracts.ts

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -9,8 +9,8 @@ const QUERY_HOOKS_DIR = path.join(ROOT, 'apps/sim/hooks/queries')
99
const SELECTOR_HOOKS_DIR = path.join(ROOT, 'apps/sim/hooks/selectors')
1010

1111
const BASELINE = {
12-
totalRoutes: 927,
13-
zodRoutes: 927,
12+
totalRoutes: 928,
13+
zodRoutes: 928,
1414
nonZodRoutes: 0,
1515
} as const
1616

@@ -55,6 +55,9 @@ const INDIRECT_ZOD_ROUTES = new Set([
5555
// tokens and there are no params, query, or body to validate. Previously had
5656
// no-op `validateSchema(noInputSchema, {})` guards.
5757
'apps/sim/app/api/health/route.ts',
58+
// Redirect target for the legacy /favicon.ico rewrite in next.config.ts —
59+
// reads only the server's own deployment-tier env var, no params/query/body.
60+
'apps/sim/app/api/favicon/route.ts',
5861
'apps/sim/app/api/settings/allowed-providers/route.ts',
5962
'apps/sim/app/api/settings/allowed-integrations/route.ts',
6063
'apps/sim/app/api/settings/allowed-mcp-domains/route.ts',

0 commit comments

Comments
 (0)