Duplicate-upload guard: same file, same item returns the existing row (#7)

docs/api.md

@@ -27,7 +27,7 @@ Consult `openapi.json` for every field and response.
 | Signals | `GET /signals` | `PUT` or `DELETE /signals/{sub}/{remoteId}` |
 | Mentions | `GET /mentions?direction=inbound` or `outbound` | `PATCH /mentions/{id}` with `hidden` |
 | Settings | `GET /settings` | `PATCH /settings` |
-| Media | Included in item detail | `POST /media` with a multipart file (`inline=true` when the client places it in the text), `DELETE /media/{id}` (detaches; deletes the file only when no published version shows it) |
+| Media | Included in item detail | `POST /media` with a multipart file (`inline=true` when the client places it in the text; the same file sent again to the same item in the same placement returns the existing row with `200` and `duplicate: true`), `DELETE /media/{id}` (detaches; deletes the file only when no published version shows it) |
 | Imported items | `GET /imports/{sub}/{id}`, `GET /imports/{sub}/{id}/history`, `GET /imports/{sub}/{id}/versions/{v}` (history and public versions, read from the origin) | The subscription importer manages these items |
 | Reading | `GET /reading`, `GET /reading/imported` (imported items only, cursor-paged; see below) | Read only |
 | Read state | `readVersion` on each imported Reading entry | `PUT`/`DELETE /reading/{sub}/{remoteId}/read`, `POST /reading/read`, `POST /reading/unread` |

migrations/0028_media_sha256.sql

@@ -0,0 +1,21 @@
+-- The upload guard (blygger-studio#7): a SHA-256 of an attachment's bytes, so
+-- an identical file uploaded again to the same item returns the existing row
+-- instead of storing a second copy. NULL for rows uploaded before this
+-- migration (not backfilled) and for uploads not attached to an item.
+ALTER TABLE media ADD COLUMN sha256 TEXT;
+CREATE INDEX media_item_sha256 ON media(item_id, sha256);
+
+-- The polling cache's change trigger (0024) lists every column it watches, and
+-- an oracle test holds it to the table's full column set. Recreated with
+-- `sha256` added; otherwise identical to 0024's.
+DROP TRIGGER IF EXISTS change_media_update;
+CREATE TRIGGER change_media_update
+AFTER UPDATE ON media
+WHEN OLD."alt" IS NOT NEW."alt" OR OLD."created" IS NOT NEW."created" OR OLD."id" IS NOT NEW."id" OR OLD."inline" IS NOT NEW."inline" OR OLD."item_id" IS NOT NEW."item_id" OR OLD."mime" IS NOT NEW."mime" OR OLD."r2_key" IS NOT NEW."r2_key" OR OLD."sha256" IS NOT NEW."sha256"
+BEGIN
+  SELECT RAISE(ABORT, 'missing change state') WHERE NOT EXISTS(SELECT 1 FROM change_state WHERE id=1);
+  UPDATE change_state SET
+    items=items+(1),
+    feed=feed+(1)
+  WHERE id=1;
+END;

openapi.json

@@ -5417,6 +5417,39 @@
           }
         },
         "responses": {
+          "200": {
+            "description": "Already attached",
+            "content": {
+              "application/json": {
+                "schema": {
+                  "type": "object",
+                  "properties": {
+                    "id": {
+                      "type": "string"
+                    },
+                    "url": {
+                      "type": "string"
+                    },
+                    "mime": {
+                      "type": "string"
+                    },
+                    "duplicate": {
+                      "type": "boolean",
+                      "enum": [
+                        true
+                      ]
+                    }
+                  },
+                  "required": [
+                    "id",
+                    "url",
+                    "mime",
+                    "duplicate"
+                  ]
+                }
+              }
+            }
+          },
           "201": {
             "description": "Success",
             "content": {

sdk/generated/types.gen.ts

@@ -1800,6 +1800,15 @@ export type UploadMediaErrors = {
 export type UploadMediaError = UploadMediaErrors[keyof UploadMediaErrors];
 
 export type UploadMediaResponses = {
+    /**
+     * Already attached
+     */
+    200: {
+        id: string;
+        url: string;
+        mime: string;
+        duplicate: true;
+    };
     /**
      * Success
      */

sdk/generated/zod.gen.ts

@@ -858,14 +858,19 @@ export const zUploadMediaBody = z.object({
     inline: z.enum(['true', 'false']).optional()
 });
 
-/**
- * Success
- */
-export const zUploadMediaResponse = z.object({
-    id: z.string(),
-    url: z.string(),
-    mime: z.string()
-});
+export const zUploadMediaResponse = z.union([
+    z.object({
+        id: z.string(),
+        url: z.string(),
+        mime: z.string(),
+        duplicate: z.literal(true)
+    }),
+    z.object({
+        id: z.string(),
+        url: z.string(),
+        mime: z.string()
+    })
+]);
 
 export const zDeleteMediaPath = z.object({
     id: z.string().min(1)

src/api.ts

@@ -23,6 +23,7 @@ import {
   getMedia,
   getSettings,
   getVersion,
+  findDuplicateMedia,
   insertMedia,
   pinVersion,
   publish,
@@ -47,7 +48,7 @@ import { BLOCKING, staleThreads, threadFreshness } from "./freshness.ts";
 import { draftChangeNote } from "./change-note.ts";
 import { platformFetchFor } from "./importer/http.ts";
 import { reconcileIndex } from "./importer/poll.ts";
-import { isFollowableUrl, isValidTimeZone, newMediaId, normalizeMount, nowIso } from "./util.ts";
+import { hex, isFollowableUrl, isValidTimeZone, newMediaId, normalizeMount, nowIso } from "./util.ts";
 
 const MEDIA_TYPES: Record<string, string> = {
   "image/png": "png",
@@ -408,20 +409,28 @@ api.openapi(routes.uploadMedia, async (c) => {
   if (typeof itemId === "string" && itemId && !(await getItem(c.env.DB, itemId))) {
     return c.json({ error: "item_id not found" }, 404);
   }
+  const bytes = await file.arrayBuffer();
+  // The studio sends inline=true for an upload it places in the text
+  // (studio#24): shown only where its line is, never appended.
+  const inline = form?.get("inline") === "true" ? 1 : 0;
+  // The same file (bytes and type) uploaded to the same item in the same
+  // placement is the same attachment (studio#7): return the row already
+  // there, store nothing.
+  const sha256 = typeof itemId === "string" && itemId ? hex(new Uint8Array(await crypto.subtle.digest("SHA-256", bytes))) : null;
+  const existing = sha256 ? await findDuplicateMedia(c.env.DB, itemId as string, sha256, file.type, inline) : null;
+  if (existing) return c.json({ id: existing.id, url: existing.r2_key, mime: existing.mime, duplicate: true }, 200);
   const id = newMediaId();
   const r2Key = `media/${id}.${ext}`;
-  await c.env.MEDIA.put(r2Key, await file.arrayBuffer(), { httpMetadata: { contentType: file.type } });
+  await c.env.MEDIA.put(r2Key, bytes, { httpMetadata: { contentType: file.type } });
   const alt = form?.get("alt");
   const row = await insertMedia(c.env.DB, {
     id,
     item_id: typeof itemId === "string" && itemId ? itemId : null,
     r2_key: r2Key,
     mime: file.type,
     alt: typeof alt === "string" ? alt : null,
-    // The studio sends inline=true for an upload it places in the text
-    // (studio#24): shown only where its line is, never appended.
-    inline: form?.get("inline") === "true" ? 1 : 0,
-  }, c.get('draftOnlyMedia') === true);
+    inline,
+  }, c.get('draftOnlyMedia') === true, sha256);
   if (!row) {
     await c.env.MEDIA.delete(r2Key);
     c.header('WWW-Authenticate', 'Bearer error="insufficient_scope", scope="owner:draft owner:publish"');

src/contract/routes.ts

@@ -136,5 +136,7 @@ export const routes = {
   // Read routes contributed by the extensions in this repository (extensions/catalog.ts).
   ...extensionRoutes,
 };
+// An identical file uploaded again to the same item returns the existing row (studio#7).
+routes.uploadMedia.responses[200] = { description: "Already attached", content: json(z.object({ id: z.string(), url: z.string(), mime: z.string(), duplicate: z.literal(true) })) };
 // The resolve/confirm operation has two successful response shapes and statuses.
 routes.createSubscription.responses[201] = { description: "Subscribed", content: json(subscribed) };

src/model.ts

@@ -648,17 +648,26 @@ export async function insertMedia(
   db: D1Database,
   row: Omit<MediaRow, "created">,
   unpublishedOnly = false,
+  sha256: string | null = null,
 ): Promise<MediaRow | null> {
   const created = nowIso();
   // Evaluate publication and insert in one statement. A publisher can run while
   // the R2 upload awaits; a separate earlier read is not an authorization gate.
   const result = await db
-    .prepare("INSERT INTO media (id, item_id, r2_key, mime, alt, created, inline) SELECT ?, ?, ?, ?, ?, ?, ? WHERE ? = 0 OR ? IS NULL OR EXISTS (SELECT 1 FROM items WHERE id = ? AND version = 0)")
-    .bind(row.id, row.item_id, row.r2_key, row.mime, row.alt, created, row.inline, unpublishedOnly ? 1 : 0, row.item_id, row.item_id)
+    .prepare("INSERT INTO media (id, item_id, r2_key, mime, alt, created, inline, sha256) SELECT ?, ?, ?, ?, ?, ?, ?, ? WHERE ? = 0 OR ? IS NULL OR EXISTS (SELECT 1 FROM items WHERE id = ? AND version = 0)")
+    .bind(row.id, row.item_id, row.r2_key, row.mime, row.alt, created, row.inline, sha256, unpublishedOnly ? 1 : 0, row.item_id, row.item_id)
     .run();
   return result.meta.changes ? { ...row, created } : null;
 }
 
+/** The item's earlier upload of the same file (bytes and type) in the same placement, if any (migration 0028, studio#7). */
+export async function findDuplicateMedia(db: D1Database, itemId: string, sha256: string, mime: string, inline: number): Promise<MediaRow | null> {
+  return db
+    .prepare("SELECT id, item_id, r2_key, mime, alt, created, inline FROM media WHERE item_id = ? AND sha256 = ? AND mime = ? AND inline = ? ORDER BY created ASC LIMIT 1")
+    .bind(itemId, sha256, mime, inline)
+    .first<MediaRow>();
+}
+
 export async function getMedia(db: D1Database, id: string): Promise<MediaRow | null> {
   return db.prepare("SELECT * FROM media WHERE id = ?").bind(id).first<MediaRow>();
 }

test/mcp-media.oracle.test.ts

@@ -73,7 +73,14 @@ describe('MCP multipart media transport parity', () => {
     expect(await (await d.fetch('/blyg/items/' + item.id + '.json')).json()).toEqual(before);
     expect(await storedCount()).toEqual({ r2: 0, rows: 0 });
     const permitted = await d.credential(['owner:draft', 'owner:publish']);
-    expect((await result(await d.upload(permitted, { file: { filename: 'boundary.png', contentType: 'image/png', dataBase64: 'aW1hZ2U=' }, item_id: item.id }))).error).toBe(false);
+    const attached = await result(await d.upload(permitted, { file: { filename: 'boundary.png', contentType: 'image/png', dataBase64: 'aW1hZ2U=' }, item_id: item.id }));
+    expect(attached.error).toBe(false);
+    // The same file again under draft scope is the attached row (studio#7): returned, nothing written.
+    const stored = await storedCount(), after = await (await d.fetch('/blyg/items/' + item.id + '.json')).json();
+    const again = await result(await d.upload(token, { file: { filename: 'boundary.png', contentType: 'image/png', dataBase64: 'aW1hZ2U=' }, item_id: item.id }));
+    expect(again.error).toBe(false); expect(again.value).toEqual({ ...attached.value, duplicate: true });
+    expect(await storedCount()).toEqual(stored);
+    expect(await (await d.fetch('/blyg/items/' + item.id + '.json')).json()).toEqual(after);
   });
 
   it('discovers uploadMedia for draft scopes', async () => {
@@ -84,16 +91,19 @@ describe('MCP multipart media transport parity', () => {
   });
   it('uses the same bytes, MIME types and attachment metadata as multipart REST', async () => {
     const d = await driver(), token = await d.credential(['owner:draft']);
-    const item = await (await d.owner('/api/items', { content_md: 'media parity' })).json() as any;
+    // One item per transport: the same file sent twice to one item is a
+    // duplicate (studio#7) and would not exercise MCP's own write.
+    const restItem = await (await d.owner('/api/items', { content_md: 'media parity (REST)' })).json() as any;
+    const mcpItem = await (await d.owner('/api/items', { content_md: 'media parity (MCP)' })).json() as any;
     const bytes = new Uint8Array([0x89, 0x50, 0x4e, 0x47, 1, 2, 3]);
     for (const contentType of ['image/png', 'image/jpeg', 'image/gif', 'image/webp', 'image/svg+xml']) {
-      const extra = { item_id: item.id, alt: 'a diagram', inline: 'true' };
-      const rest = await d.restUpload(bytes, contentType, extra); expect(rest.status).toBe(201);
+      const extra = { alt: 'a diagram', inline: 'true' };
+      const rest = await d.restUpload(bytes, contentType, { item_id: restItem.id, ...extra }); expect(rest.status).toBe(201);
       const expected = await rest.json() as any;
-      const observed = await result(await d.upload(token, { file: { filename: 'same-file.dat', contentType, dataBase64: base64(bytes) }, ...extra }));
-      expect(observed.error).toBe(false); expect(observed.value.mime).toBe(expected.mime);
+      const observed = await result(await d.upload(token, { file: { filename: 'same-file.dat', contentType, dataBase64: base64(bytes) }, item_id: mcpItem.id, ...extra }));
+      expect(observed.error).toBe(false); expect(observed.value.mime).toBe(expected.mime); expect(observed.value.duplicate).toBeUndefined();
       expect(observed.value.url.split('.').pop()).toBe(expected.url.split('.').pop());
-      for (const value of [expected, observed.value]) {
+      for (const [value, item] of [[expected, restItem], [observed.value, mcpItem]]) {
         expect((await d.fetch('/blyg/' + value.url)).status).toBe(404);
         const publicBytes = await d.fetch('/blyg/' + value.url, { headers: { cookie: d.cookie } }); expect(publicBytes.status).toBe(200);
         expect(publicBytes.headers.get('content-type')).toBe(contentType);

test/media-visibility-security.oracle.test.ts

@@ -28,8 +28,8 @@ async function fixture() {
   const login = await receive('/blyg/studio/login', { method: 'POST', headers: { 'CF-Connecting-IP': 'fd00:' + crypto.randomUUID().replaceAll('-', '').match(/.{4}/g)!.slice(0,7).join(':') }, body: new URLSearchParams({ password: env.OWNER_PASSWORD }) });
   expect(login.status).toBe(302); const cookie = login.headers.get('set-cookie')!.split(';')[0];
   const owner = (path: string, method = 'GET', body?: unknown) => receive(path, { method, headers: { cookie, 'Content-Type': 'application/json' }, ...(body === undefined ? {} : { body: JSON.stringify(body) }) });
-  const upload = async (item?: string, inline = false) => {
-    const form = new FormData(); form.set('file', new File(['private fixture bytes'], 'private.png', { type: 'image/png' }));
+  const upload = async (item?: string, inline = false, bytes = 'private fixture bytes') => {
+    const form = new FormData(); form.set('file', new File([bytes], 'private.png', { type: 'image/png' }));
     if (item) form.set('item_id', item); if (inline) form.set('inline', 'true');
     const response = await receive('/api/media', { method: 'POST', headers: { cookie }, body: form });
     expect(response.status).toBe(201); return response.json() as Promise<{id:string;url:string}>;
@@ -75,7 +75,8 @@ it.each([false, true])('withdrawal keeps published media and leaves later upload
   // Attaching to an ever-published item is itself a publish act (publish scope);
   // an inline upload is public only once published text shows it.
   if (inline) {
-    const later = await f.upload(id, true);
+    // Other bytes: the same file again would be the published row (studio#7).
+    const later = await f.upload(id, true, 'later fixture bytes');
     expect((await receive('/blyg/' + later.url)).status, 'an inline upload no published text showed stays private on a withdrawn item').toBe(404);
   }
 });

test/media.test.ts

@@ -84,3 +84,41 @@ describe("avatar resolution (session 19)", () => {
   });
 });
 
+
+describe("duplicate uploads (studio#7, migration 0028)", () => {
+  const png = (...b: number[]) => new File([new Uint8Array([0x89, 0x50, 0x4e, 0x47, ...b])], "p.png", { type: "image/png" });
+
+  it("returns the existing row for the same bytes on the same item, and stores nothing", async () => {
+    const cookie = await login();
+    const id = await createAndPublish(cookie, "fragment with a repeated image");
+    const first = await upload(cookie, png(1, 2), { item_id: id, alt: "first" });
+    expect(first.status).toBe(201);
+    const again = await upload(cookie, png(1, 2), { item_id: id, alt: "second" });
+    expect(again.status).toBe(200);
+    expect(again.json).toEqual({ id: first.json.id, url: first.json.url, mime: "image/png", duplicate: true });
+
+    const item = await (await getPublic(`/blyg/items/${id}.json`)).json<any>();
+    expect(item.media).toEqual([{ url: first.json.url, mime: "image/png", alt: "first" }]);
+  });
+
+  it("stores a new row for different bytes, another type, another item, another placement, or no item", async () => {
+    const cookie = await login();
+    const a = await createAndPublish(cookie, "item a");
+    const b = await createAndPublish(cookie, "item b");
+    const base = await upload(cookie, png(3), { item_id: a });
+    expect(base.status).toBe(201);
+    const others = [
+      await upload(cookie, png(4), { item_id: a }),
+      await upload(cookie, new File([new Uint8Array([0x89, 0x50, 0x4e, 0x47, 3])], "p.webp", { type: "image/webp" }), { item_id: a }),
+      await upload(cookie, png(3), { item_id: b }),
+      await upload(cookie, png(3), { item_id: a, inline: "true" }),
+      await upload(cookie, png(3)),
+      await upload(cookie, png(3)),
+    ];
+    for (const r of others) {
+      expect(r.status).toBe(201);
+      expect(r.json.duplicate).toBeUndefined();
+    }
+    expect(new Set([base, ...others].map((r) => r.json.id)).size).toBe(7);
+  });
+});