fix(thumbnails): duplicate a copied thumbnail with GetObject + PutObject instead of CopyObject (#4072)

* fix(thumbnails): duplicate a copied thumbnail with GetObject + PutObject instead of CopyObject

CopyObject names its source as `<bucket>/<key>` in a header, which skips
any path in the client's endpoint, so it can miss an object that
Bucket/Key requests reach. The copy handler now reads the source object
(still behind the HeadObject size bound) and writes it under the copy's
key with its content type.

* fix(thumbnails): check the GetObject length before buffering a copied thumbnail

The size check runs on a HEAD and the read on a separate GET, so an object
replaced in between was read whole into memory. Reject a GET response whose
length exceeds the cap and destroy its body before reading.

extensions/thumbnails.test.ts

@@ -2,7 +2,7 @@ import {
     GetObjectCommand,
     HeadObjectCommand,
     PutObjectCommand,
-    type S3Client,
+    S3Client,
 } from '@aws-sdk/client-s3';
 import type { Request, Response } from 'express';
 import crypto from 'node:crypto';
@@ -563,15 +563,44 @@ describe('thumbnails extension — handleFsCopyNodeThumbnail', () => {
         await server?.shutdown();
     });
 
-    const copyNode = async (thumbnail: string | null, copyUuid: string) => {
+    const copyNode = async (
+        thumbnail: string | null,
+        copyUuid: string,
+        client: S3Client = s3,
+    ) => {
         const db = { write: vi.fn().mockResolvedValue(undefined) };
         await handleFsCopyNodeThumbnail(
             { copy: { thumbnail, uuid: copyUuid } },
-            { s3, bucketName: BUCKET, bucketEndpoint: BUCKET_ENDPOINT, db },
+            {
+                s3: client,
+                bucketName: BUCKET,
+                bucketEndpoint: BUCKET_ENDPOINT,
+                db,
+            },
         );
         return db;
     };
 
+    // The key the copied row was repointed at, asserting it is bound to it.
+    const repointedKey = (
+        db: Awaited<ReturnType<typeof copyNode>>,
+        copyUuid: string,
+    ): string => {
+        expect(db.write).toHaveBeenCalledTimes(1);
+        const [sql, [pointer, uuid]] = db.write.mock.calls[0] as [
+            string,
+            [string, string],
+        ];
+        expect(sql).toBe(
+            'UPDATE `fsentries` SET `thumbnail` = ? WHERE `uuid` = ?',
+        );
+        expect(uuid).toBe(copyUuid);
+        expect(pointer).toMatch(
+            new RegExp(`^s3://${BUCKET}/thumbnails/${copyUuid}/`),
+        );
+        return pointer.slice(`s3://${BUCKET}/`.length);
+    };
+
     it.each([
         ['an entry-bound key', () => mintedKey()],
         ['a key that predates entry binding', legacyKey],
@@ -603,6 +632,7 @@ describe('thumbnails extension — handleFsCopyNodeThumbnail', () => {
             const duplicated = await s3.send(
                 new GetObjectCommand({ Bucket: BUCKET, Key: newKey }),
             );
+            expect(duplicated.ContentType).toBe('image/png');
             expect(
                 (await streamToBuffer(duplicated.Body as never)).equals(body),
             ).toBe(true);
@@ -610,6 +640,72 @@ describe('thumbnails extension — handleFsCopyNodeThumbnail', () => {
         },
     );
 
+    // The SDK puts an endpoint's path in front of every Bucket/Key request but
+    // not in front of a CopyObject source, so on such a store HeadObject finds
+    // the source and CopyObject reports NoSuchKey.
+    it('duplicates through a client whose endpoint carries a path', async () => {
+        const endpoint = await s3.config.endpoint!();
+        const client = new S3Client({
+            region: await s3.config.region(),
+            endpoint: `${endpoint.protocol}//${endpoint.hostname}:${endpoint.port}/${BUCKET}`,
+            credentials: await s3.config.credentials(),
+            forcePathStyle: true,
+        });
+        const sourceKey = mintedKey();
+        const body = Buffer.from(TINY_PNG_BASE64, 'base64');
+        await putObject(client, sourceKey, body);
+
+        const copyUuid = crypto.randomUUID();
+        const db = await copyNode(
+            `s3://${BUCKET}/${sourceKey}`,
+            copyUuid,
+            client,
+        );
+
+        const duplicated = await client.send(
+            new GetObjectCommand({
+                Bucket: BUCKET,
+                Key: repointedKey(db, copyUuid),
+            }),
+        );
+        expect(duplicated.ContentType).toBe('image/png');
+        expect(
+            (await streamToBuffer(duplicated.Body as never)).equals(body),
+        ).toBe(true);
+    });
+
+    it("lets either entry's removal leave the other's thumbnail in place", async () => {
+        const sourceUuid = crypto.randomUUID();
+        const sourceKey = mintedKey(sourceUuid);
+        const sourcePointer = `s3://${BUCKET}/${sourceKey}`;
+        await putObject(s3, sourceKey, Buffer.from(TINY_PNG_BASE64, 'base64'));
+        const remove = (thumbnail: string, uuid: string) =>
+            handleFsRemoveNodeThumbnail(
+                { target: { thumbnail, uuid } },
+                { s3, bucketName: BUCKET, bucketEndpoint: BUCKET_ENDPOINT },
+            );
+
+        // Removing a copy leaves the source's object...
+        const firstUuid = crypto.randomUUID();
+        const firstKey = repointedKey(
+            await copyNode(sourcePointer, firstUuid),
+            firstUuid,
+        );
+        await remove(`s3://${BUCKET}/${firstKey}`, firstUuid);
+        expect(await objectExists(s3, firstKey)).toBe(false);
+        expect(await objectExists(s3, sourceKey)).toBe(true);
+
+        // ...and removing the source leaves a copy's.
+        const secondUuid = crypto.randomUUID();
+        const secondKey = repointedKey(
+            await copyNode(sourcePointer, secondUuid),
+            secondUuid,
+        );
+        await remove(sourcePointer, sourceUuid);
+        expect(await objectExists(s3, sourceKey)).toBe(false);
+        expect(await objectExists(s3, secondKey)).toBe(true);
+    });
+
     it('drops the pointer when the shared object is already gone', async () => {
         const copyUuid = crypto.randomUUID();
         const db = await copyNode(
@@ -638,6 +734,40 @@ describe('thumbnails extension — handleFsCopyNodeThumbnail', () => {
         expect(params).toEqual([copyUuid]);
     });
 
+    it('drops the pointer when the object outgrows the bound after its size was checked', async () => {
+        const sourceKey = mintedKey();
+        await putObject(s3, sourceKey, Buffer.from(TINY_PNG_BASE64, 'base64'));
+        const sent: unknown[] = [];
+        // Replaces the object between the HEAD and the GET.
+        const racing = {
+            send: async (command: unknown) => {
+                sent.push(command);
+                const result = await s3.send(command as never);
+                if (command instanceof HeadObjectCommand) {
+                    await putObject(
+                        s3,
+                        sourceKey,
+                        Buffer.alloc(2 * 1024 * 1024 + 1),
+                    );
+                }
+                return result;
+            },
+        } as unknown as S3Client;
+
+        const copyUuid = crypto.randomUUID();
+        const db = await copyNode(
+            `s3://${BUCKET}/${sourceKey}`,
+            copyUuid,
+            racing,
+        );
+
+        expect(db.write).toHaveBeenCalledTimes(1);
+        const [sql, params] = db.write.mock.calls[0] as [string, [string]];
+        expect(sql).toContain('NULL');
+        expect(params).toEqual([copyUuid]);
+        expect(sent.some((c) => c instanceof PutObjectCommand)).toBe(false);
+    });
+
     it('does not duplicate an object the pointer names but we did not mint', async () => {
         const foreignKey = crypto.randomUUID(); // shaped like an fs object key
         const db = await copyNode(
@@ -828,4 +958,62 @@ describe('thumbnails extension — signed batch upload through /fs', () => {
         );
         expect(entry.thumbnail).toBe('https://example.com/thumb.png');
     });
+
+    it('gives a copy its own thumbnail that survives moves and the source being removed', async () => {
+        const { actor, username } = await makeActor();
+        const s3 = server.clients.s3.get();
+        const fs = server.services.fs;
+        const { entry: source } = await upload(
+            actor,
+            `/${username}/Documents/photo.png`,
+            (s) => s.thumbnailUrl,
+        );
+        const userId = source.userId;
+        const sourceKey = source.thumbnail!.slice(`s3://${BUCKET}/`.length);
+        const desktop = (await server.stores.fsEntry.getEntryByPath(
+            `/${username}/Desktop`,
+        ))!;
+        // The listener writes the row directly, so read it the same way.
+        const storedThumbnail = async (uuid: string) => {
+            const [row] = (await server.clients.db.read(
+                'SELECT `thumbnail` FROM `fsentries` WHERE `uuid` = ?',
+                [uuid],
+            )) as Array<{ thumbnail: string | null }>;
+            return row?.thumbnail ?? null;
+        };
+
+        const copy = await runWithContext({ actor }, () =>
+            fs.copy(userId, { source, destinationParent: desktop }),
+        );
+        // `fs.copy.node` is fire-and-forget.
+        const copyPointer = await vi.waitFor(async () => {
+            const pointer = await storedThumbnail(copy.uuid);
+            expect(pointer).toMatch(
+                new RegExp(`^s3://${BUCKET}/thumbnails/${copy.uuid}/`),
+            );
+            return pointer!;
+        });
+        const copyKey = copyPointer.slice(`s3://${BUCKET}/`.length);
+
+        const documents = (await server.stores.fsEntry.getEntryByPath(
+            `/${username}/Documents`,
+        ))!;
+        const moved = await runWithContext({ actor }, () =>
+            fs.move(userId, {
+                source: copy,
+                destinationParent: documents,
+                newName: 'moved.png',
+            }),
+        );
+        expect(moved.uuid).toBe(copy.uuid);
+        expect(await storedThumbnail(moved.uuid)).toBe(copyPointer);
+
+        await runWithContext({ actor }, () =>
+            fs.remove(userId, { entry: source }),
+        );
+        await vi.waitFor(async () => {
+            expect(await objectExists(s3, sourceKey)).toBe(false);
+        });
+        expect(await objectExists(s3, copyKey)).toBe(true);
+    });
 });

extensions/thumbnails.ts

@@ -1,5 +1,4 @@
 import {
-    CopyObjectCommand,
     DeleteObjectCommand,
     GetObjectCommand,
     HeadObjectCommand,
@@ -10,6 +9,7 @@ import { getSignedUrl } from '@aws-sdk/s3-request-presigner';
 import { extension } from '@heyputer/backend/src/extensions';
 import { isMissingObjectError } from '@heyputer/backend/src/stores/fs/S3ObjectStore';
 import crypto from 'node:crypto';
+import type { Readable } from 'node:stream';
 import sharp from 'sharp';
 const clients = extension.import('client');
 
@@ -396,11 +396,24 @@ export const handleFsCopyNodeThumbnail = async (
         ) {
             throw new Error('thumbnail exceeds the size bound');
         }
+        // Read and rewrite rather than CopyObject: a copy source skips any path
+        // in the client's endpoint, so it can miss an object Bucket/Key reach.
+        const source = await deps.s3.send(
+            new GetObjectCommand({ Bucket: deps.bucketName, Key: sourceKey }),
+        );
+        // The object can be replaced after the HEAD; bound what gets buffered.
+        if ((source.ContentLength ?? 0) > MAX_THUMBNAIL_BYTES) {
+            (source.Body as Readable | undefined)?.destroy();
+            throw new Error('thumbnail exceeds the size bound');
+        }
+        const body = await source.Body?.transformToByteArray();
+        if (!body) throw new Error('thumbnail has no body');
         await deps.s3.send(
-            new CopyObjectCommand({
+            new PutObjectCommand({
                 Bucket: deps.bucketName,
-                CopySource: `${deps.bucketName}/${sourceKey}`,
                 Key: newKey,
+                Body: body,
+                ContentType: source.ContentType,
             }),
         );
     } catch (err) {