fix(shares, resources): close what the review pass turned up

Nine findings, four of them mine from earlier in this branch.

**The R6 guarantee was not enforced.** `check-resource-views.ts` reported
`R6 … 0 (at baseline)` while a real violation sat in the tree:
`useParams<{ workspaceId: string }>()` in `add-connector-modal.tsx`. The pattern
demanded `(` immediately after the hook name, so the generic form — the dominant
idiom in this repo, quoted in two of the rule's own TSDoc blocks — was invisible.
Widened the pattern, watched it go 0 → 1, threaded `workspaceId` in as a prop
exactly as its sibling modal already documents, confirmed 0.

**An inline-image fan-out charged against a whole-file budget.** Every embedded
image spent a `content` token — 60/min, sized for downloading a file. A
thirty-image shared document spent half a reader's minute on one page view and
the second view inside that minute returned 429, which renders as broken images
with no stated reason. Added an `inline` scope denominated in what actually
happens (`INLINE_IMAGES_PER_VIEW * INLINE_VIEWS_PER_MINUTE`).

**The per-share ceiling equalled the per-IP budget it backstops.** Both 60/min
for `content`, so one visitor at full rate saturated the link and the aggregate
bound before the bucket it exists to protect. Now derived from the per-IP config
rather than written out, so `aggregate > per-IP` holds by construction. Dropped
the `execute` tier — no caller, no consumer of its type.

**My `.webm` fix reasoned from a false premise.** Routing `getContentType`
through `resolveEffectiveMimeType` was a no-op: `contentTypeMap` was consulted
first, so `DUAL_CONTAINER_MIME` never ran. The real defect was that
`contentTypeMap` duplicated 38 of `EXTENSION_TO_MIME`'s entries — drift between
two hand-maintained tables is what left `.mkv`/`.flac`/`.aac`/`.opus`/`.avi` at
`application/octet-stream`, unseekable. Collapsed to the three Google
pseudo-extensions no MIME table knows. Verified empirically before collapsing:
exactly five resolutions change, three are those pseudo-types, and `js`/`ts`
move to the WHATWG spellings while staying attachments either way. The TSDoc's
`.mp4` example was also false — staging already had that entry.

Also: one home for `REVALIDATE_CACHE_CONTROL` (four copies, three files);
`getContentType` no longer computed twice per serve branch; a dead default
parameter, a dead `binaryExtensions` export, and two TSDoc blocks that had
drifted above the wrong function; `toError` in place of the banned
`instanceof Error ? … : new Error(…)` in the three routes this branch touches.

New tests for `lib/public-shares`, which had none — 12 pinning the
aggregate-exceeds-per-IP invariant, the inline fan-out budget, bucket
separation and the 429 shape. Verified they fail against the original bug:
setting the multiple back to 1 turns two of them red.

Suite: 21113 passed. R6/R3c 0, 23 audits, lint and type-check clean.

Author: waleedlatif1

apps/sim/app/api/files/public/[token]/content/route.ts

@@ -1,5 +1,6 @@
 import { AuditAction, AuditResourceType, recordAudit } from '@sim/audit'
 import { createLogger } from '@sim/logger'
+import { toError } from '@sim/utils/errors'
 import type { NextRequest } from 'next/server'
 import { NextResponse } from 'next/server'
 import { getPublicFileContentContract } from '@/lib/api/contracts/public-shares'
@@ -19,6 +20,7 @@ import {
   createFileResponse,
   FileNotFoundError,
   getContentType,
+  REVALIDATE_CACHE_CONTROL,
 } from '@/app/api/files/utils'
 
 export const dynamic = 'force-dynamic'
@@ -163,7 +165,7 @@ export const GET = withRouteHandler(
           size: head.size,
           contentType: mediaContentType,
           filename: file.originalName,
-          cacheControl: 'private, no-cache, must-revalidate',
+          cacheControl: REVALIDATE_CACHE_CONTROL,
           rangeHeader,
         })
       }
@@ -231,14 +233,14 @@ export const GET = withRouteHandler(
         buffer,
         contentType,
         filename: file.originalName,
-        cacheControl: 'private, no-cache, must-revalidate',
+        cacheControl: REVALIDATE_CACHE_CONTROL,
       })
     } catch (error) {
       logger.error('Error serving public shared file:', error)
       if (error instanceof FileNotFoundError) {
         return createErrorResponse(error)
       }
-      return createErrorResponse(error instanceof Error ? error : new Error('Failed to serve file'))
+      return createErrorResponse(toError(error))
     }
   }
 )

apps/sim/app/api/files/public/[token]/inline/route.test.ts

@@ -125,21 +125,26 @@ describe('GET /api/files/public/[token]/inline', () => {
     expect(mockDownloadFile).not.toHaveBeenCalled()
   })
 
-  it('charges the per-IP content bucket exactly once', async () => {
+  /**
+   * The `inline` scope, not `content`. One page view fans out to one request per
+   * embedded image, so charging these against the whole-file download budget
+   * made the second view of an image-heavy document 429.
+   */
+  it('charges the per-IP inline bucket exactly once', async () => {
     await GET(req(`fileId=${FILE_ID}`), params)
     expect(mockEnforcePerIp).toHaveBeenCalledTimes(1)
-    expect(mockEnforcePerIp).toHaveBeenCalledWith(expect.anything(), 'content')
+    expect(mockEnforcePerIp).toHaveBeenCalledWith(expect.anything(), 'inline')
   })
 
   /**
    * One page of a shared document fans out to many inline requests, so the
    * aggregate per-share ceiling matters most here — the per-IP bucket alone does
    * not bound a link that is passed around.
    */
-  it('enforces the per-share content bucket with the resolved share id', async () => {
+  it('enforces the per-share inline bucket with the resolved share id', async () => {
     await GET(req(`fileId=${FILE_ID}`), params)
     expect(mockEnforcePerShare).toHaveBeenCalledTimes(1)
-    expect(mockEnforcePerShare).toHaveBeenCalledWith('content', 'sh_1')
+    expect(mockEnforcePerShare).toHaveBeenCalledWith('inline', 'sh_1')
   })
 
   it('never charges the per-share bucket for a request that fails the auth gate', async () => {

apps/sim/app/api/files/public/[token]/inline/route.ts

@@ -1,5 +1,6 @@
 import { AuditAction, AuditResourceType, recordAudit } from '@sim/audit'
 import { createLogger } from '@sim/logger'
+import { toError } from '@sim/utils/errors'
 import type { NextRequest } from 'next/server'
 import { NextResponse } from 'next/server'
 import { getPublicInlineFileContract } from '@/lib/api/contracts/public-shares'
@@ -43,7 +44,7 @@ export const GET = withRouteHandler(
     const requestId = generateRequestId()
 
     try {
-      const limited = await enforcePerIpRateLimit(request, 'content')
+      const limited = await enforcePerIpRateLimit(request, 'inline')
       if (limited) return limited
 
       const parsed = await parseRequest(getPublicInlineFileContract, request, context)
@@ -76,7 +77,7 @@ export const GET = withRouteHandler(
        * storage — one page of a shared document fans out to many inline
        * requests.
        */
-      const shareLimited = await enforcePerShareRateLimit('content', resolved.share.id)
+      const shareLimited = await enforcePerShareRateLimit('inline', resolved.share.id)
       if (shareLimited) return shareLimited
 
       const { file: doc } = resolved
@@ -125,7 +126,7 @@ export const GET = withRouteHandler(
         return createErrorResponse(error)
       }
       logger.error('Error serving public inline image:', error)
-      return createErrorResponse(error instanceof Error ? error : new Error('Failed to serve file'))
+      return createErrorResponse(toError(error))
     }
   }
 )

apps/sim/app/api/files/serve-inline-image.ts

@@ -3,7 +3,11 @@ import type { NextResponse } from 'next/server'
 import { downloadFile } from '@/lib/uploads/core/storage-service'
 import type { ResolvedInlineImage } from '@/lib/uploads/server/inline-image'
 import { sniffImageContentType } from '@/lib/uploads/utils/validation'
-import { createFileResponse, FileNotFoundError } from '@/app/api/files/utils'
+import {
+  createFileResponse,
+  FileNotFoundError,
+  REVALIDATE_CACHE_CONTROL,
+} from '@/app/api/files/utils'
 
 const logger = createLogger('InlineImageServe')
 
@@ -13,7 +17,6 @@ const logger = createLogger('InlineImageServe')
  * server-side deletion/authorization check rather than serving a stale (possibly no-longer-authorized)
  * image from cache. Private so no shared cache/CDN ever stores it.
  */
-const INLINE_CACHE_CONTROL = 'private, no-cache, must-revalidate'
 
 /**
  * Download and respond with an already-workspace-scoped inline image — the single serving tail for both
@@ -41,6 +44,6 @@ export async function serveInlineImage(
     buffer,
     contentType,
     filename: image.filename,
-    cacheControl: INLINE_CACHE_CONTROL,
+    cacheControl: REVALIDATE_CACHE_CONTROL,
   })
 }

apps/sim/app/api/files/serve/[...path]/route.test.ts

@@ -69,6 +69,25 @@ vi.mock('@/lib/uploads/core/storage-service', () => storageServiceMock)
 
 vi.mock('@/lib/uploads/utils/file-utils', () => ({
   inferContextFromKey: mockInferContextFromKey,
+  sanitizeFileKey: (key: string) => key,
+  /**
+   * `getContentType` resolves every extension through here now that the local
+   * 41-entry copy of the MIME table is gone, so the mock has to answer for the
+   * extensions these tests serve.
+   */
+  resolveEffectiveMimeType: (_declared: string | undefined, filename: string) => {
+    const ext = filename.split('.').pop()?.toLowerCase() ?? ''
+    const types: Record<string, string> = {
+      pdf: 'application/pdf',
+      jpg: 'image/jpeg',
+      jpeg: 'image/jpeg',
+      png: 'image/png',
+      txt: 'text/plain',
+      mp4: 'video/mp4',
+      webm: 'video/webm',
+    }
+    return types[ext] ?? 'application/octet-stream'
+  },
 }))
 
 vi.mock('@/lib/uploads/setup.server', () => ({}))
@@ -93,6 +112,7 @@ vi.mock('@/app/api/files/utils', () => ({
   extractStorageKey: vi.fn().mockImplementation((path: string) => path.split('/').pop()),
   extractFilename: vi.fn().mockImplementation((path: string) => path.split('/').pop()),
   findLocalFile: mockFindLocalFile,
+  REVALIDATE_CACHE_CONTROL: 'private, no-cache, must-revalidate',
 }))
 
 import { GET } from '@/app/api/files/serve/[...path]/route'

apps/sim/app/api/files/serve/[...path]/route.ts

@@ -1,6 +1,7 @@
 import { createReadStream } from 'fs'
 import { readFile, stat } from 'fs/promises'
 import { createLogger } from '@sim/logger'
+import { toError } from '@sim/utils/errors'
 import type { NextRequest } from 'next/server'
 import { NextResponse } from 'next/server'
 import { fileServeParamsSchema, fileServeQuerySchema } from '@/lib/api/contracts/storage-transfer'
@@ -25,6 +26,7 @@ import {
   FileNotFoundError,
   findLocalFile,
   getContentType,
+  REVALIDATE_CACHE_CONTROL,
 } from '@/app/api/files/utils'
 
 const logger = createLogger('FilesServeAPI')
@@ -103,7 +105,6 @@ function getWorkspaceIdForCompile(key: string): string | undefined {
 }
 
 const IMMUTABLE_CACHE_CONTROL = 'private, max-age=31536000, immutable'
-const WORKSPACE_REVALIDATE_CACHE_CONTROL = 'private, no-cache, must-revalidate'
 /** For the genuinely-public, pre-auth asset routes (avatars, OG images, workspace logos) — these are
  *  intentionally shared-cacheable. Passed EXPLICITLY so the default response cache stays `private`. */
 const PUBLIC_ASSET_CACHE_CONTROL = 'public, max-age=31536000'
@@ -120,7 +121,7 @@ function resolveServeCacheControl(
   context: string | undefined
 ): string | undefined {
   if (versioned) return IMMUTABLE_CACHE_CONTROL
-  return context === 'workspace' ? WORKSPACE_REVALIDATE_CACHE_CONTROL : undefined
+  return context === 'workspace' ? REVALIDATE_CACHE_CONTROL : undefined
 }
 
 export const GET = withRouteHandler(
@@ -217,7 +218,7 @@ export const GET = withRouteHandler(
         return createErrorResponse(error)
       }
 
-      return createErrorResponse(error instanceof Error ? error : new Error('Failed to serve file'))
+      return createErrorResponse(toError(error))
     }
   }
 )
@@ -227,7 +228,7 @@ async function handleLocalFile(
   userId: string,
   options: ServeOptions,
   signal: AbortSignal | undefined,
-  rangeHeader: string | null = null
+  rangeHeader: string | null
 ): Promise<NextResponse> {
   const ownerKey = `user:${userId}`
   try {
@@ -262,14 +263,15 @@ async function handleLocalFile(
      * than a storage key, so it opens its own stream — without this branch the
      * scrubber works against cloud storage and silently dies on a dev machine.
      */
-    if (isMediaContentType(getContentType(displayName))) {
+    const mediaType = getContentType(displayName)
+    if (isMediaContentType(mediaType)) {
       const { size } = await stat(filePath)
       logger.info('Local media served', { userId, filename, size })
 
       return await createByteRangeResponse({
         openStream: (range) => createReadStream(filePath, range),
         size,
-        contentType: getContentType(displayName),
+        contentType: mediaType,
         filename: displayName,
         cacheControl: resolveServeCacheControl(options.versioned, contextParam),
         rangeHeader,
@@ -307,7 +309,7 @@ async function handleCloudProxy(
   userId: string,
   options: ServeOptions,
   signal: AbortSignal | undefined,
-  rangeHeader: string | null = null
+  rangeHeader: string | null
 ): Promise<NextResponse> {
   const ownerKey = `user:${userId}`
   try {
@@ -337,7 +339,8 @@ async function handleCloudProxy(
      * generated-document compile, which no media file is subject to. `copilot`
      * keeps the buffered path — its bytes come from a different reader.
      */
-    if (context !== 'copilot' && isMediaContentType(getContentType(displayName))) {
+    const mediaType = getContentType(displayName)
+    if (context !== 'copilot' && isMediaContentType(mediaType)) {
       const head = await headObject(cloudKey, context)
       if (!head) throw new FileNotFoundError(`File not found: ${cloudKey}`)
 
@@ -346,7 +349,7 @@ async function handleCloudProxy(
       return await createByteRangeResponse({
         openStream: (range) => downloadFileStream({ key: cloudKey, context, range }),
         size: head.size,
-        contentType: getContentType(displayName),
+        contentType: mediaType,
         filename: displayName,
         cacheControl: resolveServeCacheControl(options.versioned, context),
         rangeHeader,

apps/sim/app/api/files/utils.ts

@@ -9,7 +9,7 @@ import {
   parseByteRange,
   unsatisfiableContentRangeHeader,
 } from '@/lib/uploads/utils/byte-range'
-import { getMimeTypeFromExtension, sanitizeFileKey } from '@/lib/uploads/utils/file-utils'
+import { resolveEffectiveMimeType, sanitizeFileKey } from '@/lib/uploads/utils/file-utils'
 
 const logger = createLogger('FilesUtils')
 
@@ -44,79 +44,51 @@ export class InvalidRequestError extends Error {
   }
 }
 
-export const contentTypeMap: Record<string, string> = {
-  txt: 'text/plain',
-  csv: 'text/csv',
-  json: 'application/json',
-  xml: 'application/xml',
-  md: 'text/markdown',
-  html: 'text/html',
-  css: 'text/css',
-  js: 'application/javascript',
-  ts: 'application/typescript',
-  pdf: 'application/pdf',
+/**
+ * The pseudo-extensions no real filename ends in, so no MIME table knows them.
+ *
+ * Everything else resolves through {@link resolveEffectiveMimeType}. This map
+ * used to carry 41 entries, 38 of which duplicated `EXTENSION_TO_MIME`
+ * byte-for-byte — and that duplication was the bug: a container present in one
+ * table and missing from the other (`.mkv`, `.flac`, `.aac`, `.opus`, `.avi`)
+ * resolved to `application/octet-stream`, which silently disabled byte-range
+ * serving and left it unseekable. Two tables cannot be kept in agreement by
+ * hand, so there is now one.
+ */
+/**
+ * Cache policy for bytes a viewer must be re-authorized for on every request:
+ * private, and revalidated rather than served from the browser's back-forward
+ * cache after access is revoked.
+ *
+ * One export because it is the caching policy for every authenticated and every
+ * shared byte, and it was previously written out in four places across three
+ * files — kept in agreement only by eye.
+ */
+export const REVALIDATE_CACHE_CONTROL = 'private, no-cache, must-revalidate'
+
+const GOOGLE_PSEUDO_MIME: Record<string, string> = {
   googleDoc: 'application/vnd.google-apps.document',
-  doc: 'application/msword',
-  docx: 'application/vnd.openxmlformats-officedocument.wordprocessingml.document',
-  xls: 'application/vnd.ms-excel',
-  xlsx: 'application/vnd.openxmlformats-officedocument.spreadsheetml.sheet',
   googleSheet: 'application/vnd.google-apps.spreadsheet',
-  ppt: 'application/vnd.ms-powerpoint',
-  pptx: 'application/vnd.openxmlformats-officedocument.presentationml.presentation',
-  png: 'image/png',
-  jpg: 'image/jpeg',
-  jpeg: 'image/jpeg',
-  gif: 'image/gif',
-  svg: 'image/svg+xml',
-  webp: 'image/webp',
-  avif: 'image/avif',
-  bmp: 'image/bmp',
-  ico: 'image/x-icon',
-  mp3: 'audio/mpeg',
-  m4a: 'audio/mp4',
-  wav: 'audio/wav',
-  ogg: 'audio/ogg',
-  flac: 'audio/flac',
-  aac: 'audio/aac',
-  opus: 'audio/opus',
-  mp4: 'video/mp4',
-  mov: 'video/quicktime',
-  avi: 'video/x-msvideo',
-  mkv: 'video/x-matroska',
-  webm: 'video/webm',
-  zip: 'application/zip',
   googleFolder: 'application/vnd.google-apps.folder',
 }
 
-export const binaryExtensions = [
-  'doc',
-  'docx',
-  'xls',
-  'xlsx',
-  'ppt',
-  'pptx',
-  'zip',
-  'png',
-  'jpg',
-  'jpeg',
-  'gif',
-  'webp',
-  'pdf',
-]
-
 /**
  * Content type for a stored file, by extension.
  *
- * {@link contentTypeMap} is consulted first because it carries entries the
- * extension map has no notion of (the Google Workspace pseudo-types), then the
- * canonical {@link getMimeTypeFromExtension} covers everything else — audio and
- * video included. Keeping only the local map meant `.mp4` resolved to
- * `application/octet-stream`, which silently disabled byte-range serving and
- * left media unseekable.
+ * {@link GOOGLE_PSEUDO_MIME} is consulted first for the three Workspace
+ * pseudo-extensions no MIME table knows; everything else goes to
+ * {@link resolveEffectiveMimeType}.
+ *
+ * That resolver rather than the raw extension table because what a response
+ * declares IS presentation, and it is the only one that knows a dual container.
+ * `.webm` holds either audio or video; the extension table answers `audio/webm`
+ * so the speech-to-text route does not send it down an ffmpeg path it does not
+ * need, while the viewer routes it to a `<video>`. Declaring `audio/webm` on the
+ * wire would leave one user-facing surface disagreeing with the rest.
  */
 export function getContentType(filename: string): string {
   const extension = filename.split('.').pop()?.toLowerCase() || ''
-  return contentTypeMap[extension] || getMimeTypeFromExtension(extension)
+  return GOOGLE_PSEUDO_MIME[extension] || resolveEffectiveMimeType(undefined, filename)
 }
 
 export function extractFilename(path: string): string {