feat: show path and state metadata in the managed attachments comment

.changeset/attachment-comment-metadata.md

@@ -0,0 +1,5 @@
+---
+"@buildinternet/uploads": minor
+---
+
+Managed GitHub attachment comments now show an upload's canonical `path` and `state` metadata — a screenshot tagged `--state before` on `/settings` renders as `/settings · before` beneath the image instead of just its filename. Attachments without that metadata render exactly as before.

apps/api/src/file-metadata.ts

@@ -364,24 +364,35 @@ const METADATA_LOOKUP_CHUNK = 100;
  * metadata each key already has. Keys with no rows are simply absent from
  * the returned map (not present with an empty object). Chunks the `keys`
  * list to stay under D1/SQLite's bound-parameter limit per statement.
+ *
+ * `opts.metaKeys` narrows the SELECT to the named meta keys (issue #365).
+ * Prefer it on hot paths: the filter rides the `(workspace, object_key,
+ * meta_key)` primary-key index, so the read stays flat at ~3-5 D1 rows per
+ * object key however many keys that object carries. An empty array is treated
+ * as "no filter", not "select nothing" — a silently empty result map is the
+ * worse failure mode for a caller that built the list dynamically.
  */
 export async function getMetadataForKeys(
   db: D1Database,
   workspace: string,
   keys: string[],
+  opts: { metaKeys?: string[] } = {},
 ): Promise<Map<string, Record<string, string>>> {
   const out = new Map<string, Record<string, string>>();
   if (keys.length === 0) return out;
 
+  const metaKeys = opts.metaKeys?.length ? opts.metaKeys : undefined;
+  const metaFilter = metaKeys ? ` AND meta_key IN (${metaKeys.map(() => "?").join(", ")})` : "";
+
   for (let i = 0; i < keys.length; i += METADATA_LOOKUP_CHUNK) {
     const chunk = keys.slice(i, i + METADATA_LOOKUP_CHUNK);
     const placeholders = chunk.map(() => "?").join(", ");
     const result = await db
       .prepare(
         `SELECT object_key, meta_key, meta_value FROM file_metadata
-         WHERE workspace = ? AND object_key IN (${placeholders})`,
+         WHERE workspace = ? AND object_key IN (${placeholders})${metaFilter}`,
       )
-      .bind(workspace, ...chunk)
+      .bind(workspace, ...chunk, ...(metaKeys ?? []))
       .all<{ object_key: string; meta_key: string; meta_value: string }>();
     for (const row of result.results) {
       let map = out.get(row.object_key);

apps/api/src/github-comment-render.test.ts

@@ -14,6 +14,7 @@ function loadFixture(name: string) {
 
 const golden = loadFixture("github-comment-golden.json");
 const goldenCap = loadFixture("github-comment-golden-cap.json");
+const goldenMeta = loadFixture("github-comment-golden-meta.json");
 
 describe("attachmentsCommentBody (api copy)", () => {
   it("renders the golden body byte-for-byte", () => {
@@ -25,4 +26,33 @@ describe("attachmentsCommentBody (api copy)", () => {
       goldenCap.expected,
     );
   });
+
+  it("renders path/state captions, and nothing when they are absent", () => {
+    expect(attachmentsCommentBody(goldenMeta.items, goldenMeta.galleries)).toBe(
+      goldenMeta.expected,
+    );
+  });
+
+  it("captions overflow rows and escapes markdown metacharacters", () => {
+    // 18 images exceeds MAX_INLINE_ATTACHMENT_IMAGES (16), so the last two
+    // collapse into the <details> list — those rows must caption too.
+    const items = Array.from({ length: 18 }, (_, i) => ({
+      key: `gh/acme/web/pull/12/shot-${String(i).padStart(2, "0")}.png`,
+      url: `https://uploads.sh/f/shot-${i}.png`,
+      embedUrl: `https://embed.uploads.sh/f/shot-${i}.png`,
+      pageUrl: `https://uploads.sh/f/acme/shot-${i}.png`,
+      // Tildes included: GitHub strikes through text wrapped in a matching pair
+      // of one or two tildes, so `~e~` would render struck through unescaped.
+      meta: { path: "/a_b[c]*d~e~", state: "after" },
+    }));
+
+    const body = attachmentsCommentBody(items);
+
+    // Markdown context: the metacharacters are backslash-escaped.
+    expect(body).toContain(
+      "- [shot-17.png](https://uploads.sh/f/acme/shot-17.png) · /a\\_b\\[c\\]\\*d\\~e\\~ · after",
+    );
+    // HTML context: <sub> needs no markdown escaping, so the value is verbatim.
+    expect(body).toContain("<sub>/a_b[c]*d~e~ · after</sub>");
+  });
 });

apps/api/src/github-comment-render.ts

@@ -83,6 +83,14 @@ export interface AttachmentItem {
   embedUrl?: string | null;
   /** Canonical `/f/` file-page URL (server-computed). Preferred click-through target; falls back to `url`. */
   pageUrl?: string | null;
+  /**
+   * The only canonical metadata the managed comment renders (issue #365).
+   * Deliberately two named fields rather than `Record<string, string>`: the
+   * comment is posted publicly, and keeping the set narrow at the type level
+   * mirrors the server-side query filter that never fetches EXIF-derived
+   * keys like `device`/`software` for this path.
+   */
+  meta?: { path?: string; state?: string };
 }
 
 /** A public gallery linked to the PR or issue whose managed comment is syncing. */
@@ -127,6 +135,53 @@ function escapeHtmlText(s: string): string {
   return escapeHtmlAttr(s).replace(/'/g, "&#39;").replace(/>/g, "&gt;");
 }
 
+/**
+ * Backslash-escape the markdown metacharacters that can appear in a metadata
+ * value. `~` is in the set because GitHub's strikethrough extension treats a
+ * matching pair of ONE or two tildes as markup, so an unescaped `/a~b~c` would
+ * render with `b` struck through.
+ */
+function escapeMarkdownText(s: string): string {
+  return s.replace(/([\\`*_[\]~])/g, "\\$1");
+}
+
+/**
+ * An attachment's caption parts — `path`, then `state` (issue #365). Empty
+ * when neither is usable, so callers emit nothing at all and a body with no
+ * metadata stays byte-identical to the pre-#365 render.
+ *
+ * Neither value is pre-sanitized: metadata values are printable ASCII up to
+ * 512 chars, and while the CLI validates `--state` against a closed enum,
+ * `PATCH /v1/:workspace/files/:key` can set any valid metadata value. A
+ * whitespace-only value passes that validation (length-1 printable ASCII), so
+ * treat it as absent rather than rendering a dangling separator.
+ */
+function metaCaptionParts(meta: AttachmentItem["meta"]): string[] {
+  const parts: string[] = [];
+  for (const value of [meta?.path, meta?.state]) {
+    const trimmed = value?.trim();
+    if (trimmed) parts.push(trimmed);
+  }
+  return parts;
+}
+
+/** `<sub>` caption body for an inline image, or null when there is nothing to say. */
+function metaCaptionHtml(meta: AttachmentItem["meta"]): string | null {
+  const parts = metaCaptionParts(meta);
+  return parts.length > 0 ? parts.map(escapeHtmlText).join(" · ") : null;
+}
+
+/**
+ * ` · …` suffix for a markdown list row, or `""` when there is nothing to add.
+ * HTML-escapes first, then markdown-escapes: HTML escaping introduces no
+ * backslashes or brackets, so the markdown pass cannot corrupt its entities.
+ */
+function metaCaptionMarkdown(meta: AttachmentItem["meta"]): string {
+  const parts = metaCaptionParts(meta);
+  if (parts.length === 0) return "";
+  return ` · ${parts.map((p) => escapeMarkdownText(escapeHtmlText(p))).join(" · ")}`;
+}
+
 /**
  * Render the one marker-owned GitHub comment. When there are no galleries this
  * intentionally preserves the legacy attachment-only body byte-for-byte.
@@ -184,11 +239,13 @@ export function attachmentsCommentBody(
       const href = escapeHtmlAttr(link ?? (src as string));
       const imgSrc = escapeHtmlAttr(src as string);
       lines.push(`<a href="${href}"><img width="${w}" alt="${alt}" src="${imgSrc}"></a>`);
+      const caption = metaCaptionHtml(item.meta);
+      if (caption) lines.push(`<sub>${caption}</sub>`);
       lines.push("");
     } else if (link) {
-      lines.push(`- [${name}](${link})`);
+      lines.push(`- [${name}](${link})${metaCaptionMarkdown(item.meta)}`);
     } else {
-      lines.push(`- ${name}`);
+      lines.push(`- ${name}${metaCaptionMarkdown(item.meta)}`);
     }
   }
   if (overflowImages.length > 0) {
@@ -197,7 +254,8 @@ export function attachmentsCommentBody(
     for (const item of overflowImages) {
       const name = item.key.slice(item.key.lastIndexOf("/") + 1);
       const link = item.pageUrl ?? item.url;
-      lines.push(link ? `- [${name}](${link})` : `- ${name}`);
+      const suffix = metaCaptionMarkdown(item.meta);
+      lines.push(link ? `- [${name}](${link})${suffix}` : `- ${name}${suffix}`);
     }
     lines.push("", "</details>", "");
   }

apps/api/src/github-comment.test.ts

@@ -4,12 +4,16 @@ import { describe, expect, it } from "vitest";
 import { gatherCommentBody, upsertBotComment } from "./github-comment";
 import { ATTACHMENTS_MARKER, attachmentsMarker } from "./github-comment-render";
 import { addExternalReference, addGalleryItem, createGallery } from "./galleries";
+import { replaceFileMetadata } from "./file-metadata";
 import type { WorkspaceRecord } from "./workspace";
 import { FakeR2Bucket } from "../test/fake-r2";
 import { FakeKv } from "../test/fake-kv";
 import { SqliteD1, database } from "../test/helpers/sqlite-d1";
 
-const MIGRATION = "migrations/20260711180000_galleries.sql";
+const MIGRATION = [
+  "migrations/20260711180000_galleries.sql",
+  "migrations/20260713210559_file_metadata.sql",
+];
 const PRAGMAS = ["PRAGMA foreign_keys = ON"];
 const PNG = new Uint8Array([0x89, 0x50, 0x4e, 0x47, 0x0d, 0x0a, 0x1a, 0x0a]);
 
@@ -189,6 +193,111 @@ describe("gatherCommentBody", () => {
   });
 });
 
+describe("gatherCommentBody attachment metadata (issue #365)", () => {
+  async function seed(env: Env, bucket: FakeR2Bucket, meta: Record<string, string>) {
+    await bucket.put("acme/gh/acme/web/pull/12/before.webp", PNG);
+    await replaceFileMetadata(env.DB, "acme", "gh/acme/web/pull/12/before.webp", meta);
+  }
+
+  it("renders path and state from D1 on the attachment", async () => {
+    const { env, ws, workspaceName, bucket } = makeTestEnv();
+    await seed(env, bucket, { path: "/settings", state: "before" });
+
+    const result = await gatherCommentBody(env, ws, workspaceName, {
+      repo: "acme/web",
+      num: 12,
+      kind: "pull",
+    });
+
+    expect(result.skip).toBe(false);
+    expect((result as { body: string }).body).toContain("<sub>/settings · before</sub>");
+  });
+
+  it("never fetches or renders EXIF-derived keys", async () => {
+    const { env, ws, workspaceName, bucket } = makeTestEnv();
+    await seed(env, bucket, {
+      path: "/settings",
+      device: "iPhone 15 Pro",
+      software: "Adobe Photoshop 26.0",
+    });
+
+    const result = await gatherCommentBody(env, ws, workspaceName, {
+      repo: "acme/web",
+      num: 12,
+      kind: "pull",
+    });
+
+    const body = (result as { body: string }).body;
+    expect(body).toContain("<sub>/settings</sub>");
+    expect(body).not.toContain("iPhone");
+    expect(body).not.toContain("Photoshop");
+  });
+
+  it("renders exactly as before when the workspace has no metadata", async () => {
+    const { env, ws, workspaceName, bucket } = makeTestEnv();
+    await bucket.put("acme/gh/acme/web/pull/12/before.webp", PNG);
+
+    const result = await gatherCommentBody(env, ws, workspaceName, {
+      repo: "acme/web",
+      num: 12,
+      kind: "pull",
+    });
+
+    expect((result as { body: string }).body).not.toContain("<sub>/");
+  });
+
+  it("skips the D1 read entirely when githubCommentShowMetadata is false", async () => {
+    const { env, ws, workspaceName, bucket } = makeTestEnv();
+    await seed(env, bucket, { path: "/settings", state: "before" });
+
+    let metadataQueries = 0;
+    // Delegate explicitly rather than spreading `env.DB` — it is a class
+    // instance, so a spread would drop its prototype methods.
+    const spied = {
+      prepare: (sql: string) => {
+        if (sql.includes("FROM file_metadata")) metadataQueries++;
+        return env.DB.prepare(sql);
+      },
+      batch: (statements: D1PreparedStatement[]) => env.DB.batch(statements),
+    } as unknown as D1Database;
+
+    const result = await gatherCommentBody(
+      { ...env, DB: spied } as Env,
+      { ...ws, githubCommentShowMetadata: false },
+      workspaceName,
+      { repo: "acme/web", num: 12, kind: "pull" },
+    );
+
+    expect(metadataQueries).toBe(0);
+    expect((result as { body: string }).body).not.toContain("<sub>/settings");
+  });
+
+  it("does not leak another workspace's metadata for the same object key", async () => {
+    const { env, ws, workspaceName, bucket } = makeTestEnv();
+    await seed(env, bucket, { path: "/settings", state: "before" });
+
+    // A different workspace's metadata on the SAME object key — must not leak
+    // into acme's rendered comment. The D1 row is scoped by the `workspace`
+    // column, not by anything derived from the object key.
+    await replaceFileMetadata(env.DB, "other-ws", "gh/acme/web/pull/12/before.webp", {
+      path: "/intruder-path",
+      state: "compromised",
+    });
+
+    const result = await gatherCommentBody(env, ws, workspaceName, {
+      repo: "acme/web",
+      num: 12,
+      kind: "pull",
+    });
+
+    expect(result.skip).toBe(false);
+    const body = (result as { body: string }).body;
+    expect(body).toContain("<sub>/settings · before</sub>");
+    expect(body).not.toContain("/intruder-path");
+    expect(body).not.toContain("compromised");
+  });
+});
+
 describe("upsertBotComment", () => {
   it("creates the comment when no marker comment exists", async () => {
     const { env } = makeTestEnv();

apps/api/src/github-comment.ts

@@ -8,6 +8,7 @@
  */
 
 import { listObjects } from "./files-core";
+import { getMetadataForKeys } from "./file-metadata";
 import { findGalleriesByReference, listGalleryItems, type GalleryCursor } from "./galleries";
 import { galleryUrl, hydrateOwnerGallery } from "./gallery-service";
 import { parseExternalReference } from "./external-references";
@@ -42,7 +43,7 @@ export async function gatherCommentBody(
   // Attachments (R2 list) and galleries (D1) are independent reads — overlap
   // them so the request only waits the longer of the two, not their sum.
   const [items, galleries] = await Promise.all([
-    gatherAttachments(env, ws, target),
+    gatherAttachments(env, ws, workspaceName, target),
     gatherGalleries(env, ws, workspaceName, target),
   ]);
 
@@ -52,17 +53,27 @@ export async function gatherCommentBody(
   return { skip: false, body: attachmentsCommentBody(items, galleries, marker), count };
 }
 
+/**
+ * The only metadata keys the managed comment reads or renders (issue #365).
+ * Filtering at the query rather than the renderer keeps the D1 cost flat at
+ * ~3-5 rows read per attachment however many keys a file carries, and means
+ * EXIF-derived keys are never fetched for a surface that posts publicly.
+ */
+const COMMENT_META_KEYS = ["path", "state"];
+
 /** The workspace's own objects under the stable gh key prefix. */
 async function gatherAttachments(
   env: Env,
   ws: WorkspaceRecord,
+  workspaceName: string,
   target: GhTarget,
 ): Promise<AttachmentItem[]> {
   // Per-workspace choice (issue #304): default (undefined/true) links the
   // managed comment's attachments to their `/f/` file page (issue #301's
   // behavior); `false` links to raw object bytes instead. Attachments only —
   // does not affect gallery `itemUrl` below, which is a separate feature.
   const linkToFilePage = ws.githubCommentLinkToFilePage !== false;
+  const showMetadata = ws.githubCommentShowMetadata !== false; // issue #365
   const items: AttachmentItem[] = [];
   let cursor: string | undefined;
   do {
@@ -80,6 +91,25 @@ async function gatherAttachments(
       });
     cursor = page.cursor ?? undefined;
   } while (cursor);
+
+  if (!showMetadata || items.length === 0) return items;
+
+  // D1 rows are tenant-scoped by `workspaceName` (the caller's own slug), not
+  // by anything derived from `ws` — same trust boundary as gatherGalleries.
+  const metaByKey = await getMetadataForKeys(
+    env.DB,
+    workspaceName,
+    items.map((item) => item.key),
+    { metaKeys: COMMENT_META_KEYS },
+  );
+  for (const item of items) {
+    const meta = metaByKey.get(item.key);
+    if (!meta) continue;
+    const { path, state } = meta;
+    if (path || state) {
+      item.meta = { ...(path ? { path } : {}), ...(state ? { state } : {}) };
+    }
+  }
   return items;
 }