fix(favicons): 204 for no icon, only 404 is a provider miss, reject empty labels and non-DNS hostnames

A miss is an ordinary answer, so it returns 204 (which an img still reports as
failed) instead of an error status; binary internal routes may now declare
several success statuses and present a no-content response, sharing the JSON
builder's status validation. Providers are asked one at a time so a second only
learns a host the first had no icon for, a non-200 body is never read, and
anything but 404/410 counts as unavailable rather than a cached miss.

Author: waleedlatif1

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

@@ -19,13 +19,22 @@ export const GET = defineInternalBinaryRoute({
   errorPolicy: internalOrchestrationErrorPolicy,
   mapInput: ({ query }) => ({ host: query.host }),
   useCase: getFavicon,
-  present: ({ bytes, contentType }) => ({
-    body: new Uint8Array(bytes),
-    contentType,
-    contentLength: bytes.length,
-    headers: {
-      'Cache-Control': 'private, max-age=86400, stale-while-revalidate=604800',
-      'X-Content-Type-Options': 'nosniff',
-    },
-  }),
+  /** No icon is an ordinary answer, not an error: 204, which an `<img>` still reports as failed. */
+  present: (favicon) =>
+    favicon.kind === 'hit'
+      ? {
+          body: new Uint8Array(favicon.bytes),
+          contentType: favicon.contentType,
+          contentLength: favicon.bytes.length,
+          headers: {
+            'Cache-Control': 'private, max-age=86400, stale-while-revalidate=604800',
+            'X-Content-Type-Options': 'nosniff',
+          },
+        }
+      : {
+          status: 204,
+          headers: {
+            'Cache-Control': favicon.kind === 'miss' ? 'private, max-age=3600' : 'no-store',
+          },
+        },
 })

apps/sim/app/workspace/[workspaceId]/home/components/message-content/components/source-chip/source-icon.test.tsx

@@ -29,6 +29,6 @@ describe('source hostname branding', () => {
     container.innerHTML = renderToStaticMarkup(<SourceIcon source={{ url }} />)
     const image = container.querySelector('img')
     expect(image).not.toBeNull()
-    expect(new URL(image!.src).searchParams.get('domain')).toBe(new URL(url).hostname)
+    expect(new URL(image!.src).searchParams.get('host')).toBe(new URL(url).hostname)
   })
 })

apps/sim/lib/api/contracts/favicon.ts

@@ -2,7 +2,7 @@ import { z } from 'zod'
 import { defineRouteContract } from '@/lib/api/contracts/types'
 
 /**
- * A site's icon by hostname: image bytes, or 404 when none can be served — neither the host nor
+ * A site's icon by hostname: image bytes, or 204 when none can be served — neither the host nor
  * a parent has one, the host is internal, or the providers are unavailable.
  */
 export const getFaviconContract = defineRouteContract({
@@ -15,5 +15,5 @@ export const getFaviconContract = defineRouteContract({
       .min(1, 'host is required')
       .max(253, 'host must be 253 characters or less'),
   }),
-  response: { mode: 'binary' },
+  response: { mode: 'binary', status: [200, 204] },
 })

apps/sim/lib/api/server/routes/definition.ts

@@ -20,6 +20,27 @@ export function requireJsonRouteDefinition(
     )
   }
 
+  return declaredSuccessStatuses(contract)
+}
+
+export function requireBinaryRouteDefinition(
+  contract: AnyApiRouteContract,
+  declaredOperation: ApplicationOperation,
+  useCaseOperation: ApplicationOperation
+): JsonRouteDefinitionMetadata {
+  if (contract.response.mode !== 'binary') {
+    throw new Error(`${contract.method} ${contract.path} requires a binary response contract`)
+  }
+  if (declaredOperation.id !== useCaseOperation.id) {
+    throw new Error(
+      `Route operation ${declaredOperation.id} does not match use case ${useCaseOperation.id}`
+    )
+  }
+  return declaredSuccessStatuses(contract)
+}
+
+/** The contract's success statuses, 200 by default; the first is a route's default status. */
+function declaredSuccessStatuses(contract: AnyApiRouteContract): JsonRouteDefinitionMetadata {
   const configuredStatus = contract.response.status
   const successStatuses =
     configuredStatus === undefined
@@ -41,30 +62,6 @@ export function requireJsonRouteDefinition(
   return { successStatus: successStatuses[0], successStatuses }
 }
 
-export function requireBinaryRouteDefinition(
-  contract: AnyApiRouteContract,
-  declaredOperation: ApplicationOperation,
-  useCaseOperation: ApplicationOperation
-): JsonRouteDefinitionMetadata {
-  if (contract.response.mode !== 'binary') {
-    throw new Error(`${contract.method} ${contract.path} requires a binary response contract`)
-  }
-  if (declaredOperation.id !== useCaseOperation.id) {
-    throw new Error(
-      `Route operation ${declaredOperation.id} does not match use case ${useCaseOperation.id}`
-    )
-  }
-  const configuredStatus = contract.response.status
-  if (configuredStatus !== undefined && typeof configuredStatus !== 'number') {
-    throw new Error(`${contract.method} ${contract.path} must declare one success status`)
-  }
-  const successStatus = configuredStatus ?? 200
-  if (successStatus < 200 || successStatus >= 300) {
-    throw new Error(`${contract.method} ${contract.path} has a non-success response status`)
-  }
-  return { successStatus, successStatuses: [successStatus] }
-}
-
 /**
  * Whether an incoming request's method is the one its contract declares.
  *

apps/sim/lib/api/server/routes/internal-binary-route.ts

@@ -19,6 +19,7 @@ import type {
   JsonErrorResponseDescriptor,
   JsonNextRouteHandler,
   JsonRouteContext,
+  NoContentResponseDescriptor,
 } from '@/lib/api/server/routes/types'
 import type { ParsedRequest, ParseRequestOptions } from '@/lib/api/server/validation'
 import { parseRequest } from '@/lib/api/server/validation'
@@ -35,7 +36,12 @@ interface InternalBinaryRouteDefinition<
   operation: O
   mapInput(input: ParsedRequest<C>): I
   useCase: OperationUseCase<NoInfer<O>, I, R>
-  present(result: R): BinaryResponseDescriptor | Promise<BinaryResponseDescriptor>
+  present(
+    result: R
+  ):
+    | BinaryResponseDescriptor
+    | NoContentResponseDescriptor
+    | Promise<BinaryResponseDescriptor | NoContentResponseDescriptor>
 }
 
 interface InternalBinaryRouteOptions<
@@ -64,7 +70,7 @@ export function defineInternalBinaryRoute<
   I,
   R,
 >(options: InternalBinaryRouteOptions<C, O, I, R>): JsonNextRouteHandler {
-  const { successStatus } = requireBinaryRouteDefinition(
+  const { successStatus, successStatuses } = requireBinaryRouteDefinition(
     options.contract,
     options.operation,
     options.useCase.operation
@@ -116,6 +122,14 @@ export function defineInternalBinaryRoute<
         const result = await options.useCase.execute({ principal, input, request })
         const descriptor = await options.present(result)
         await options.onSuccess?.({ principal, input, result })
+        if ('status' in descriptor) {
+          if (!successStatuses.includes(descriptor.status)) {
+            throw new Error(
+              `Internal binary route produced undeclared success status ${descriptor.status}; expected ${successStatuses.join(', ')}`
+            )
+          }
+          return new NextResponse(null, { status: descriptor.status, headers: descriptor.headers })
+        }
         const headers = new Headers(descriptor.headers)
         headers.set('Content-Type', descriptor.contentType)
         if (descriptor.contentDisposition) {

apps/sim/lib/api/server/routes/types.ts

@@ -30,6 +30,12 @@ export interface BinaryResponseDescriptor {
   headers?: HeadersInit
 }
 
+/** An empty success from a binary route whose contract declares 204. */
+export interface NoContentResponseDescriptor {
+  status: 204
+  headers?: HeadersInit
+}
+
 export interface JsonErrorResponseDescriptor {
   body: unknown
   status: number

apps/sim/lib/favicon/application.ts

@@ -1,19 +1,14 @@
 import { defineOperation } from '@/lib/core/application'
-import { OrchestrationError } from '@/lib/core/orchestration/types'
 import { defineAuthorizedCredentialUserUseCase } from '@/lib/credentials/application/authorized-user-use-case'
 import { resolveFavicon } from '@/lib/favicon/resolve.server'
 
-/** A site's icon for the signed-in user; a host with none anywhere is `not_found`. */
+/** A site's icon for the signed-in user, or why there is none to serve. */
 export const getFavicon = defineAuthorizedCredentialUserUseCase({
   // permission-group-exempt: A public site's icon reveals nothing the caller's permissions govern.
   operation: defineOperation({
     id: 'favicons.get',
     principalKinds: ['session'],
     capability: 'none',
   }),
-  async execute({ input }: { input: { host: string } }) {
-    const favicon = await resolveFavicon(input.host)
-    if (favicon.kind !== 'hit') throw new OrchestrationError('not_found', 'No icon for this host')
-    return favicon
-  },
+  execute: ({ input }: { input: { host: string } }) => resolveFavicon(input.host),
 })

apps/sim/lib/favicon/hosts.test.ts

@@ -60,6 +60,27 @@ describe('faviconHostCandidates', () => {
     expect(faviconHostCandidates('bücher.de')).toEqual(['xn--bcher-kva.de'])
   })
 
+  it.each(['x.com&size=1.com', 'a&b=c.com', 'a%26b.com', "it's.com", 'a(b).com', 'a+b.com'])(
+    'offers nothing for %j, whose punctuation would reshape a provider URL',
+    (host) => {
+      expect(faviconHostCandidates(host)).toEqual([])
+    }
+  )
+
+  it.each(['example.com..', '.example.com', 'docs..example.com'])(
+    'offers nothing for %j, which has an empty label',
+    (host) => {
+      expect(faviconHostCandidates(host)).toEqual([])
+    }
+  )
+
+  it('keeps an underscore, which some real hosts use', () => {
+    expect(faviconHostCandidates('my_app.example.com')).toEqual([
+      'my_app.example.com',
+      'example.com',
+    ])
+  })
+
   it.each(['', 'https://example.com/path', 'exa mple.com', 'user@example.com', 'com'])(
     'offers nothing for %j',
     (host) => {

apps/sim/lib/favicon/hosts.ts

@@ -8,13 +8,18 @@ const HOST_SHAPE = /^[^\s/?#@\\]+$/
  * `acme.atlassian.net` is invisible to providers, but its platform is not, so each parent
  * domain follows, 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
- * the public DNS — yield nothing, so they never leave the server.
+ * 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.
  */
 export function faviconHostCandidates(input: string): string[] {
   const url = HOST_SHAPE.test(input) ? URL.parse(`https://${input}`) : null
   if (!url) return []
   const hostname = url.hostname.replace(/\.$/, '')
-  const { isIp, isIcann, publicSuffix } = parse(hostname, { allowPrivateDomains: false })
+  if (hostname.split('.').some((label) => !label)) return []
+  const { isIp, isIcann, publicSuffix } = parse(hostname, {
+    allowPrivateDomains: false,
+    validateHostname: true,
+  })
   if (isIp || !isIcann || !publicSuffix || /(^|\.)arpa$/.test(publicSuffix)) return []
   const labels = hostname.split('.')
   const candidateCount = labels.length - publicSuffix.split('.').length

apps/sim/lib/favicon/probe.test.ts

@@ -38,7 +38,11 @@ describe('classifyProviderResponse', () => {
     expect(classifyProviderResponse(200, oversized)).toEqual({ kind: 'miss' })
   })
 
-  it.each([429, 500, 502, 503])('treats %i as unavailable, not as a miss', (status) => {
+  it('counts a 410 as a miss, like a 404', () => {
+    expect(classifyProviderResponse(410, PNG)).toEqual({ kind: 'miss' })
+  })
+
+  it.each([403, 408, 429, 500, 502, 503])('treats %i as unavailable, not as a miss', (status) => {
     expect(classifyProviderResponse(status, PNG)).toEqual({ kind: 'unavailable' })
   })
 })