diff --git a/apps/sim/components/favicon.tsx b/apps/sim/components/favicon.tsx index 53a192c8d59..89cf0ab72b1 100644 --- a/apps/sim/components/favicon.tsx +++ b/apps/sim/components/favicon.tsx @@ -2,8 +2,16 @@ import { type ReactNode, useState } from 'react' import { cn } from '@sim/emcn' +import { LRUCache } from 'lru-cache' import { faviconSrc } from '@/lib/favicon/url' +/** + * Hosts whose icon just failed, so an icon that mounts again renders its fallback at once + * instead of an empty slot first. An `` can't tell no icon from a provider outage, so this + * lasts only as long as the route retries an outage; a confirmed miss stays in the HTTP cache. + */ +const knownMisses = new LRUCache({ max: 500, ttl: 5 * 60 * 1000 }) + interface FaviconProps { /** The site's hostname; without one the fallback renders. */ host: string | null | undefined @@ -20,7 +28,7 @@ interface FaviconProps { */ export function Favicon({ host, fallback, className }: FaviconProps) { const [failedHost, setFailedHost] = useState(null) - if (!host || failedHost === host) return fallback + if (!host || failedHost === host || knownMisses.has(host)) return fallback return ( setFailedHost(host)} + onError={() => { + knownMisses.set(host, true) + setFailedHost(host) + }} /> ) } diff --git a/apps/sim/lib/favicon/hosts.test.ts b/apps/sim/lib/favicon/hosts.test.ts index adf37453d92..c47ced23dd2 100644 --- a/apps/sim/lib/favicon/hosts.test.ts +++ b/apps/sim/lib/favicon/hosts.test.ts @@ -40,6 +40,79 @@ describe('faviconHostCandidates', () => { ]) }) + it.each([ + ['portal.awsapps.com', ['portal.awsapps.com', 'awsapps.com', 'aws.amazon.com']], + ['acme.coupahost.com', ['acme.coupahost.com', 'coupahost.com', 'coupa.com']], + [ + 'docs.acme.gitlab-dedicated.site', + [ + 'docs.acme.gitlab-dedicated.site', + 'acme.gitlab-dedicated.site', + 'gitlab-dedicated.site', + 'gitlab.com', + ], + ], + [ + 'acme.gitlab-dedicated.com', + ['acme.gitlab-dedicated.com', 'gitlab-dedicated.com', 'gitlab.com'], + ], + ['acme.testrail.io', ['acme.testrail.io', 'testrail.io', 'testrail.com']], + [ + 'acme.databricksapps.mil', + ['acme.databricksapps.mil', 'databricksapps.mil', 'databricks.com'], + ], + [ + 'dbc-1.cloud.databricks.us', + ['dbc-1.cloud.databricks.us', 'cloud.databricks.us', 'databricks.us', 'databricks.com'], + ], + ])( + 'ends a platform tenant %s at the platform, after its own host and parents', + (host, expected) => { + expect(faviconHostCandidates(host)).toEqual(expected) + } + ) + + it.each([ + ['gitlab.acme-internal.io', 'gitlab.com'], + ['jira.acme-internal.io', 'atlassian.com'], + ['testrail.qa.acme-internal.io', 'testrail.com'], + ])('ends a self-hosted %s at its product', (host, product) => { + expect(faviconHostCandidates(host).at(-1)).toBe(product) + }) + + it.each([ + 'mygitlab.acme-internal.io', + 'docs.gitlab.acme-internal.io', + 'notawsapps.com', + 'acme.herokuapp.com', + 'gitlab.azurewebsites.net', + 'gitlab.acme.vercel.app', + ])('adds no platform for %s, which only resembles one', (host) => { + const candidates = faviconHostCandidates(host) + expect(candidates).not.toContain('gitlab.com') + expect(candidates).not.toContain('aws.amazon.com') + expect(candidates.length).toBeGreaterThan(0) + }) + + it.each(['gitlab.corp', 'gitlab', 'jira.local'])( + 'still offers nothing for internal %s', + (host) => { + expect(faviconHostCandidates(host)).toEqual([]) + } + ) + + it('adds nothing for a platform whose parent the walk already reaches', () => { + expect(faviconHostCandidates('acme.my.salesforce.com')).toEqual([ + 'acme.my.salesforce.com', + 'my.salesforce.com', + 'salesforce.com', + ]) + }) + + it('never repeats a host the walk already reaches', () => { + expect(faviconHostCandidates('gitlab.gitlab.com')).toEqual(['gitlab.gitlab.com', 'gitlab.com']) + }) + it('tries at most three parents', () => { expect(faviconHostCandidates('a.b.c.d.e.example.com')).toEqual([ 'a.b.c.d.e.example.com', diff --git a/apps/sim/lib/favicon/hosts.ts b/apps/sim/lib/favicon/hosts.ts index 5615f58a256..c7b85840f29 100644 --- a/apps/sim/lib/favicon/hosts.ts +++ b/apps/sim/lib/favicon/hosts.ts @@ -1,4 +1,5 @@ import { parse } from 'tldts' +import { platformSite } from '@/lib/favicon/platforms' const MAX_PARENT_HOSTS = 3 const HOST_SHAPE = /^[^\s/?#@\\]+$/ @@ -6,8 +7,10 @@ const HOST_SHAPE = /^[^\s/?#@\\]+$/ /** * Hosts to ask a favicon provider about, most specific first. A private tenant such as * `acme.atlassian.net` is invisible to providers, but its platform is not, so each parent - * domain follows — at most three, stopping before the ICANN public suffix (`gitlab.io` is a valid last step; - * `io` and `co.uk` are not). Internal hosts — IP literals, single labels, and names outside + * domain follows — at most three, stopping before the ICANN public suffix (`gitlab.io` is a valid + * last step; `io` and `co.uk` are not). A host on a platform no provider can see even at the + * parent (`x.awsapps.com`, a self-hosted `gitlab.`) ends at the platform's own site, after + * its own host and parents so a tenant's custom icon still wins. Internal hosts — IP literals, single labels, and names outside * the public DNS — yield nothing, so they never leave the server, and neither does a name with * characters DNS can't hold, which could otherwise reshape a provider's URL. */ @@ -23,7 +26,9 @@ export function faviconHostCandidates(input: string): string[] { if (isIp || !isIcann || !publicSuffix || /(^|\.)arpa$/.test(publicSuffix)) return [] const labels = hostname.split('.') const candidateCount = labels.length - publicSuffix.split('.').length - return labels + const candidates = labels .slice(0, Math.min(candidateCount, MAX_PARENT_HOSTS + 1)) .map((_, index) => labels.slice(index).join('.')) + const platform = platformSite(hostname) + return platform && !candidates.includes(platform) ? [...candidates, platform] : candidates } diff --git a/apps/sim/lib/favicon/platforms.ts b/apps/sim/lib/favicon/platforms.ts new file mode 100644 index 00000000000..93e54075803 --- /dev/null +++ b/apps/sim/lib/favicon/platforms.ts @@ -0,0 +1,52 @@ +import { parse } from 'tldts' + +/** + * Platforms whose tenant hosts no icon provider can see — not even at any parent the walk + * reaches — mapped to the platform's own site, so a tenant still shows the product it is. A + * platform whose parent already resolves (`salesforce.com`, `databricks.com`, `atlassian.net`) + * needs no entry. Generic hosting domains (`herokuapp.com`, `azurewebsites.net`, + * `cloudfront.net`) are deliberately absent: they serve other companies' apps, and the host's + * logo would misrepresent them. GitLab Pages on Dedicated (`gitlab-dedicated.site`) maps like + * `gitlab.io`, whose parent already shows GitLab's icon. + */ +const PLATFORM_SITES: ReadonlyMap = new Map([ + ['awsapps.com', 'aws.amazon.com'], + ['azuredatabricks.net', 'databricks.com'], + ['coupacloud.com', 'coupa.com'], + ['coupahost.com', 'coupa.com'], + ['databricks.us', 'databricks.com'], + ['databricksapps.com', 'databricks.com'], + ['databricksapps.mil', 'databricks.com'], + ['databricksapps.us', 'databricks.com'], + ['gitlab-dedicated.com', 'gitlab.com'], + ['gitlab-dedicated.site', 'gitlab.com'], + ['gitlab-dedicated.systems', 'gitlab.com'], + ['jamacloud.com', 'jamasoftware.com'], + ['oktapreview.com', 'okta.com'], + ['testrail.io', 'testrail.com'], + ['ultipro.com', 'ukg.com'], +]) + +/** + * Self-hosted tools conventionally served at `.` — only on a company's + * own domain, never on a hosting platform's suffix (`gitlab.azurewebsites.net` is someone's app). + */ +const SELF_HOSTED_PRODUCTS: ReadonlyMap = new Map([ + ['confluence', 'atlassian.com'], + ['gitlab', 'gitlab.com'], + ['grafana', 'grafana.com'], + ['jenkins', 'jenkins.io'], + ['jira', 'atlassian.com'], + ['testrail', 'testrail.com'], +]) + +/** The public site of the platform a host belongs to, matched on whole labels, or `null`. */ +export function platformSite(hostname: string): string | null { + const labels = hostname.split('.') + for (let index = 0; index < labels.length; index++) { + const site = PLATFORM_SITES.get(labels.slice(index).join('.')) + if (site) return site + } + if (parse(hostname, { allowPrivateDomains: true }).isPrivate) return null + return SELF_HOSTED_PRODUCTS.get(labels[0]) ?? null +} diff --git a/apps/sim/lib/favicon/resolve.server.test.ts b/apps/sim/lib/favicon/resolve.server.test.ts new file mode 100644 index 00000000000..cc89109539d --- /dev/null +++ b/apps/sim/lib/favicon/resolve.server.test.ts @@ -0,0 +1,74 @@ +import { + inputValidationMock, + inputValidationMockFns, +} from '@sim/testing/mocks/input-validation.mock' +import { createMockRedis } from '@sim/testing/mocks/redis.mock' +import { redisConfigMockFns } from '@sim/testing/mocks/redis-config.mock' +import { beforeEach, describe, expect, it, vi } from 'vitest' +import { resolveFavicon } from '@/lib/favicon/resolve.server' + +vi.mock('@/lib/core/security/input-validation.server', () => inputValidationMock) + +const PNG = Buffer.from([0x89, 0x50, 0x4e, 0x47, 0x0d, 0x0a, 0x1a, 0x0a, 0, 0, 0, 0]) +const DAY = 24 * 60 * 60 + +type Answer = 'icon' | 'none' | 'timeout' + +/** Each host's answer at both providers; any host not listed has no icon. */ +let answers: Record = {} +let stored: Map + +function hostOf(url: string): string { + const match = url.match(/url=https:\/\/([^&]+)&/) ?? url.match(/\/ip3\/(.+)\.ico$/) + if (!match) throw new Error(`Unexpected provider URL ${url}`) + return match[1] +} + +beforeEach(() => { + answers = {} + stored = new Map() + const redis = createMockRedis() + redis.getBuffer.mockImplementation(async (key: string) => stored.get(key)?.value ?? null) + redis.set.mockImplementation( + async (key: string, value: Buffer | string, _ex: string, ttl: number) => { + stored.set(key, { value: Buffer.from(value), ttl }) + return 'OK' + } + ) + redisConfigMockFns.mockGetRedisClient.mockReturnValue(redis) + inputValidationMockFns.mockSecureFetchWithValidation.mockImplementation(async (url: string) => { + const answer = answers[hostOf(url)] ?? 'none' + if (answer === 'timeout') throw new Error('timed out') + return { + status: answer === 'icon' ? 200 : 404, + arrayBuffer: async () => PNG.buffer.slice(PNG.byteOffset, PNG.byteOffset + PNG.byteLength), + body: { cancel: async () => {} }, + } + }) +}) + +const ttlOf = (host: string) => stored.get(`favicon:v3:${host}`)?.ttl + +describe('resolveFavicon', () => { + it('keeps a parent icon chosen after a timeout provisional when a child host reuses it', async () => { + answers = { 'gitlab.acme-internal.io': 'timeout', 'gitlab.com': 'icon' } + expect((await resolveFavicon('gitlab.acme-internal.io')).kind).toBe('hit') + expect(ttlOf('gitlab.acme-internal.io')).toBe(DAY) + + expect((await resolveFavicon('docs.gitlab.acme-internal.io')).kind).toBe('hit') + expect(ttlOf('docs.gitlab.acme-internal.io')).toBe(DAY) + }) + + it('still reaches the product fallback after its parent was cached as a miss', async () => { + answers = { 'atlassian.com': 'icon' } + expect((await resolveFavicon('acme-internal.io')).kind).toBe('miss') + + expect((await resolveFavicon('jira.acme-internal.io')).kind).toBe('hit') + }) + + it('remembers a definite icon for a month', async () => { + answers = { 'acme.example.com': 'icon' } + expect((await resolveFavicon('acme.example.com')).kind).toBe('hit') + expect(ttlOf('acme.example.com')).toBe(30 * DAY) + }) +}) diff --git a/apps/sim/lib/favicon/resolve.server.ts b/apps/sim/lib/favicon/resolve.server.ts index 26c7c08f638..53caf74f274 100644 --- a/apps/sim/lib/favicon/resolve.server.ts +++ b/apps/sim/lib/favicon/resolve.server.ts @@ -12,8 +12,15 @@ const PROVIDER_TIMEOUT_MS = 1500 const RESOLVE_DEADLINE_MS = 4000 const HIT_TTL_SECONDS = 30 * 24 * 60 * 60 const MISS_TTL_SECONDS = 24 * 60 * 60 +/** A parent's icon chosen because the host's own lookup timed out: retried a day later, not a month. */ +const PROVISIONAL_HIT_TTL_SECONDS = 24 * 60 * 60 const UNAVAILABLE_TTL_SECONDS = 5 * 60 -/** Cached values: an icon's bytes, or one of these markers. */ +/** + * Cached values: a hit is one prefix byte — definite or provisional, which a host reusing it + * must inherit — followed by the icon; the markers stand for no icon and for an outage. + */ +const DEFINITE_HIT_PREFIX = 'h' +const PROVISIONAL_HIT_PREFIX = 'p' const MISS_MARKER = '' const UNAVAILABLE_MARKER = '-' @@ -28,13 +35,13 @@ const PROVIDERS: readonly ((host: string) => string)[] = [ ] type FaviconResolution = - | { kind: 'hit'; contentType: string; bytes: Buffer } + | { kind: 'hit'; contentType: string; bytes: Buffer; provisional?: true } | { kind: 'miss' } | { kind: 'unavailable' } const inFlight = new Map>() -const cacheKey = (host: string) => `favicon:v2:${host}` +const cacheKey = (host: string) => `favicon:v3:${host}` /** A host's key holds the result of the walk that starts at that host. */ async function readCache(host: string): Promise { @@ -44,8 +51,13 @@ async function readCache(host: string): Promise { const cached = await redis.getBuffer(cacheKey(host)) if (cached === null) return null if (cached.toString() === UNAVAILABLE_MARKER) return { kind: 'unavailable' } - const result = classifyProviderResponse(200, cached) - return result.kind === 'hit' ? { ...result, bytes: cached } : { kind: 'miss' } + const prefix = cached.subarray(0, 1).toString() + const bytes = cached.subarray(1) + const result = classifyProviderResponse(200, bytes) + if (result.kind !== 'hit') return { kind: 'miss' } + return prefix === PROVISIONAL_HIT_PREFIX + ? { ...result, bytes, provisional: true } + : { ...result, bytes } } catch (error) { logger.warn('Favicon cache read failed', { error }) return null @@ -57,7 +69,15 @@ async function writeCache(host: string, resolution: FaviconResolution): Promise< if (!redis) return const [value, ttl] = resolution.kind === 'hit' - ? ([resolution.bytes, HIT_TTL_SECONDS] as const) + ? resolution.provisional + ? ([ + Buffer.concat([Buffer.from(PROVISIONAL_HIT_PREFIX), resolution.bytes]), + PROVISIONAL_HIT_TTL_SECONDS, + ] as const) + : ([ + Buffer.concat([Buffer.from(DEFINITE_HIT_PREFIX), resolution.bytes]), + HIT_TTL_SECONDS, + ] as const) : resolution.kind === 'miss' ? ([MISS_MARKER, MISS_TTL_SECONDS] as const) : ([UNAVAILABLE_MARKER, UNAVAILABLE_TTL_SECONDS] as const) @@ -101,18 +121,29 @@ async function askProviders(host: string, signal: AbortSignal): Promise { let unavailable = false + const covered = new Set() for (const [index, host] of candidates.entries()) { + if (covered.has(host)) continue const parent = index > 0 const cached = parent ? await readCache(host) : null - if (cached) return cached.kind === 'miss' && unavailable ? { kind: 'unavailable' } : cached + if (cached?.kind === 'hit') { + return unavailable || cached.provisional ? { ...cached, provisional: true } : cached + } + if (cached) { + if (cached.kind === 'unavailable') unavailable = true + for (const reached of faviconHostCandidates(host)) covered.add(reached) + continue + } const result = await askProviders(host, signal) if (result.kind === 'hit') { if (parent) await writeCache(host, result) - return result + return unavailable ? { ...result, provisional: true } : result } if (result.kind === 'unavailable') unavailable = true } @@ -120,10 +151,11 @@ async function walk(candidates: string[], signal: AbortSignal): Promise { const candidates = faviconHostCandidates(host) diff --git a/packages/testing/src/mocks/redis.mock.ts b/packages/testing/src/mocks/redis.mock.ts index f78b5f2024e..947165d11f0 100644 --- a/packages/testing/src/mocks/redis.mock.ts +++ b/packages/testing/src/mocks/redis.mock.ts @@ -30,6 +30,7 @@ export function createMockRedis() { // Key operations get: vi.fn().mockResolvedValue(null), + getBuffer: vi.fn().mockResolvedValue(null), set: vi.fn().mockResolvedValue('OK'), del: vi.fn().mockResolvedValue(1), exists: vi.fn().mockResolvedValue(0),