fix(public-shares): size a streamed share from storage, audit every delivered response, cover both read paths

Author: waleedlatif1

apps/sim/lib/projects/files/__integration__/content-sharing.integration.ts

@@ -800,71 +800,83 @@ describe('Public file shares against PostgreSQL and private storage', () => {
         '@/lib/public-shares/application'
       )
       const { encryptSecret } = await import('@/lib/core/security/encryption')
-      for (const change of ['disabled', 'password', 'archived', 'provenance'] as const) {
-        const f = await fixture()
-        const { file } = await createProjectFile.execute({
-          principal: f.principal,
-          input: createInput(f.projectId),
-        })
-        const { share } = await updateProjectFileShare.execute({
-          principal: f.principal,
-          input: {
-            projectId: f.projectId,
-            fileId: file.id,
-            isActive: true,
-            authType: 'password',
-            password: 'initial-password',
-          },
-        })
-        const access = await authorizePublicFileShare({
-          token: share.token,
-          credential: { method: 'POST', password: 'initial-password' },
-        })
-        if (!access.authorized) throw new Error('Initial credential rejected')
-        const applyChange = async () => {
-          if (change === 'disabled')
-            await db
-              .update(publicShare)
-              .set({ isActive: false })
-              .where(eq(publicShare.id, share.id))
-          if (change === 'password')
-            await db
-              .update(publicShare)
-              .set({ password: (await encryptSecret('replacement-password')).encrypted })
-              .where(eq(publicShare.id, share.id))
-          if (change === 'provenance')
-            await db
-              .update(workspaceFileSecretProvenance)
-              .set({ status: 'unknown' })
-              .where(eq(workspaceFileSecretProvenance.fileId, file.id))
-          if (change === 'archived')
-            await db
-              .update(workspaceFiles)
-              .set({ deletedAt: new Date() })
-              .where(eq(workspaceFiles.id, file.id))
-        }
-        // The change lands while the object is being read, buffered or streamed alike.
-        const download = storage.downloadFile
-        const downloadStream = storage.downloadFileStream
-        const spies = [
-          vi.spyOn(storage, 'downloadFile').mockImplementation(async (options) => {
-            const result = await download(options)
-            if (options.key === file.key) await applyChange()
-            return result
-          }),
-          vi.spyOn(storage, 'downloadFileStream').mockImplementation(async (options) => {
-            const result = await downloadStream(options)
-            if (options.key === file.key) await applyChange()
-            return result
-          }),
-        ]
-        await expect(
-          sharedBytes(readPublicFileShareContent({ grant: access.grant }))
-        ).rejects.toMatchObject({
-          code: change === 'provenance' ? 'forbidden' : 'not_found',
-        })
-        for (const spy of spies) spy.mockRestore()
+      // A Markdown file streams from storage; a page is rendered whole. Both reads must refuse.
+      const shared = {
+        streamed: (projectId: string) => createInput(projectId),
+        rendered: (projectId: string) => ({
+          ...createInput(projectId, '---\ntitle: Architecture\n---\n\nShared page'),
+          name: 'Architecture',
+          contentType: 'text/x-sim-page',
+        }),
       }
+      for (const [read, input] of Object.entries(shared))
+        for (const change of ['disabled', 'password', 'archived', 'provenance'] as const) {
+          const f = await fixture()
+          const { file } = await createProjectFile.execute({
+            principal: f.principal,
+            input: input(f.projectId),
+          })
+          const { share } = await updateProjectFileShare.execute({
+            principal: f.principal,
+            input: {
+              projectId: f.projectId,
+              fileId: file.id,
+              isActive: true,
+              authType: 'password',
+              password: 'initial-password',
+            },
+          })
+          const access = await authorizePublicFileShare({
+            token: share.token,
+            credential: { method: 'POST', password: 'initial-password' },
+          })
+          if (!access.authorized) throw new Error('Initial credential rejected')
+          const applyChange = async () => {
+            if (change === 'disabled')
+              await db
+                .update(publicShare)
+                .set({ isActive: false })
+                .where(eq(publicShare.id, share.id))
+            if (change === 'password')
+              await db
+                .update(publicShare)
+                .set({ password: (await encryptSecret('replacement-password')).encrypted })
+                .where(eq(publicShare.id, share.id))
+            if (change === 'provenance')
+              await db
+                .update(workspaceFileSecretProvenance)
+                .set({ status: 'unknown' })
+                .where(eq(workspaceFileSecretProvenance.fileId, file.id))
+            if (change === 'archived')
+              await db
+                .update(workspaceFiles)
+                .set({ deletedAt: new Date() })
+                .where(eq(workspaceFiles.id, file.id))
+          }
+          // The change lands while the object is being read, buffered or streamed alike.
+          const download = storage.downloadFile
+          const downloadStream = storage.downloadFileStream
+          const spies = [
+            vi.spyOn(storage, 'downloadFile').mockImplementation(async (options) => {
+              const result = await download(options)
+              if (options.key === file.key) await applyChange()
+              return result
+            }),
+            vi.spyOn(storage, 'downloadFileStream').mockImplementation(async (options) => {
+              const result = await downloadStream(options)
+              if (options.key === file.key) await applyChange()
+              return result
+            }),
+          ]
+          const delivered = readPublicFileShareContent({ grant: access.grant })
+          await expect(sharedBytes(delivered)).rejects.toMatchObject({
+            code: change === 'provenance' ? 'forbidden' : 'not_found',
+          })
+          // Each fixture took the path it names: the stream refuses on open, the page as it renders.
+          if (read === 'streamed') expect(await delivered).toMatchObject({ kind: 'stream' })
+          else await expect(delivered).rejects.toBeDefined()
+          for (const spy of spies) spy.mockRestore()
+        }
     }
   )
 

apps/sim/lib/public-shares/application/read.ts

@@ -34,11 +34,7 @@ import {
 import { storedObjectEtag } from '@/lib/uploads/server/delivery'
 import { hasEmbeddedFileRef } from '@/lib/uploads/server/embedded-image-refs'
 import { resolveServableImageBytes } from '@/lib/uploads/server/image-derivative'
-import {
-  type ByteRange,
-  getWorkspaceFileSize,
-  MAX_BUFFERED_TRANSFER_BYTES,
-} from '@/lib/uploads/shared/types'
+import { type ByteRange, MAX_BUFFERED_TRANSFER_BYTES } from '@/lib/uploads/shared/types'
 import { resolveEffectiveMimeType } from '@/lib/uploads/utils/file'
 import { sniffImageContentType } from '@/lib/uploads/utils/validation'
 import { reportWorkspaceFileDelivery } from '@/lib/workspace-files/application/file-delivery-observer'
@@ -143,6 +139,8 @@ function recordPublicShareDownload(input: {
   servedFile: WorkspaceFileRow
   operation: PublicFileReadOperation
   bytes: number
+  /** The inclusive span a ranged response carried; absent for the whole file. */
+  range?: ByteRange
   request?: OrchestrationRequestContext
 }) {
   const { servedFile } = input
@@ -164,6 +162,7 @@ function recordPublicShareDownload(input: {
       anonymous: true,
       owner: input.owner,
       bytes: input.bytes,
+      ...(input.range ? { range: `${input.range.start}-${input.range.end}` } : {}),
     },
   })
 }
@@ -299,30 +298,34 @@ export async function checkPublicFileShareContent(
 /**
  * A shared file served as its stored bytes, read from storage only as far as a response needs:
  * the whole object, or one inclusive span. The bytes are never held, so a file of any size is
- * served and a byte range is read on its own.
+ * served and a byte range is read on its own. Storage is asked for the object first, so a missing
+ * object is refused before any header is sent, and every length is measured against the object.
  *
  * `open` rechecks the grant, the revision, and the file's provenance once storage has opened and
- * before the stream is handed out, as a buffered read does after rendering. One view is audited
- * once: a download, or the first span of a player's ranged reads.
+ * before the stream is handed out, as a buffered read does after rendering. Each delivered
+ * response is reported and audited, with the span it carried.
  */
-function streamSharedFile(
+async function streamSharedFile(
   grant: VerifiedPublicFileShareGrant,
   initial: PublicFileShareSnapshot,
   operation: PublicFileReadOperation,
   request?: OrchestrationRequestContext
 ) {
   const { file } = initial
-  const size = getWorkspaceFileSize(file)
+  const context = initial.adapter.storageContext
+  const stored = await headObject(file.key, context)
+  if (!stored) throw new OrchestrationError('not_found', 'File not found')
+  const { size } = stored
   return {
     kind: 'stream' as const,
     filename: file.originalName,
     contentType: resolveEffectiveMimeType(undefined, file.originalName),
     size,
     /** A content write uploads under a fresh key and stamps `contentUpdatedAt`. */
-    etag: storedObjectEtag([file.key, size, file.contentUpdatedAt.getTime()]),
+    etag: storedObjectEtag([file.key, size, file.contentUpdatedAt.getTime(), stored.version ?? '']),
     async open(range: ByteRange | undefined): Promise<ReadableStream<Uint8Array>> {
       const stream = nodeReadableToWebStream(
-        await downloadFileStream({ key: file.key, context: initial.adapter.storageContext, range })
+        await downloadFileStream({ key: file.key, context, range })
       )
       try {
         const provenance = await withPublicFileShareGrant(
@@ -338,18 +341,17 @@ function streamSharedFile(
         await stream.cancel(error).catch(() => undefined)
         throw asOrchestrationError(error) ?? error
       }
-      if (!range || range.start === 0) {
-        recordPublicShareDownload({
-          workspaceId: initial.workspaceId,
-          organizationId: initial.organizationId,
-          owner: initial.owner,
-          sharedFile: file,
-          servedFile: file,
-          operation,
-          bytes: range ? range.end - range.start + 1 : size,
-          request,
-        })
-      }
+      recordPublicShareDownload({
+        workspaceId: initial.workspaceId,
+        organizationId: initial.organizationId,
+        owner: initial.owner,
+        sharedFile: file,
+        servedFile: file,
+        operation,
+        bytes: range ? range.end - range.start + 1 : size,
+        range,
+        request,
+      })
       return stream
     },
   }
@@ -374,7 +376,7 @@ export async function readPublicFileShareContent({
       return snapshot
     })
     if (!preview && servesStoredBytes(initial.file.contentType, initial.file.originalName)) {
-      return streamSharedFile(grant, initial, operation, request)
+      return await streamSharedFile(grant, initial, operation, request)
     }
     const raw = await downloadFile({
       key: initial.file.key,

apps/sim/scripts/test-project-file-public-e2e.ts

@@ -60,12 +60,18 @@ const image = await sharp({
 
 async function request(
   path: string,
-  options: { method?: string; body?: Record<string, unknown>; cookie?: string } = {}
+  options: {
+    method?: string
+    body?: Record<string, unknown>
+    cookie?: string
+    headers?: Record<string, string>
+  } = {}
 ) {
   const headers = new Headers({
     Origin: base.origin,
     'Content-Type': 'application/json',
     'X-Forwarded-For': proofIp,
+    ...options.headers,
   })
   if (options.cookie) headers.set('Cookie', options.cookie)
   // boundary-raw-fetch: this proof exercises actual public binary, RSC, and credential HTTP surfaces.
@@ -281,10 +287,13 @@ try {
       )
     })
     await check(
-      `${owner}: source and inline scan enforce independent real byte ceilings`,
+      `${owner}: stored bytes stream past the buffered ceiling while the inline scan keeps its own`,
       async () => {
-        await withSparseFile(required(doc.id), 100 * 1024 * 1024 + 1, async () => {
-          assertBinaryError(await request(`${path}/content`), 413)
+        const size = 100 * 1024 * 1024 + 1
+        await withSparseFile(required(doc.id), size, async () => {
+          const head = await request(`${path}/content`, { headers: { range: 'bytes=0-0' } })
+          assert.equal(head.status, 206)
+          assert.equal(head.headers.get('content-range'), `bytes 0-0/${size}`)
           assertBinaryError(await request(`${path}/inline?fileId=${required(photo.id)}`), 404)
         })
         await withSparseFile(required(photo.id), 100 * 1024 * 1024 + 1, async () => {