From 6d96ac7b1f77ece59385dcf38c70cc4f228361ea Mon Sep 17 00:00:00 2001 From: Zach Dunn Date: Mon, 13 Jul 2026 17:11:04 -0400 Subject: [PATCH 01/23] feat(api): file_metadata table + core module Adds the D1 table and file-metadata.ts core module (validation, get/set/ delete, AND-filter search) that later tasks build the metadata CRUD endpoints and search filter on top of. --- .../20260713210559_file_metadata.sql | 10 + apps/api/src/file-metadata.ts | 213 ++++++++++++++ apps/api/test/file-metadata-sqlite.test.ts | 265 ++++++++++++++++++ apps/api/test/file-metadata.test.ts | 93 ++++++ packages/errors/src/codes.ts | 5 + 5 files changed, 586 insertions(+) create mode 100644 apps/api/migrations/20260713210559_file_metadata.sql create mode 100644 apps/api/src/file-metadata.ts create mode 100644 apps/api/test/file-metadata-sqlite.test.ts create mode 100644 apps/api/test/file-metadata.test.ts diff --git a/apps/api/migrations/20260713210559_file_metadata.sql b/apps/api/migrations/20260713210559_file_metadata.sql new file mode 100644 index 0000000..4ee6845 --- /dev/null +++ b/apps/api/migrations/20260713210559_file_metadata.sql @@ -0,0 +1,10 @@ +CREATE TABLE file_metadata ( + workspace TEXT NOT NULL, + object_key TEXT NOT NULL, + meta_key TEXT NOT NULL, + meta_value TEXT NOT NULL, + updated_at TEXT NOT NULL, + PRIMARY KEY (workspace, object_key, meta_key) +); +CREATE INDEX file_metadata_lookup_idx + ON file_metadata (workspace, meta_key, meta_value); diff --git a/apps/api/src/file-metadata.ts b/apps/api/src/file-metadata.ts new file mode 100644 index 0000000..a04fb87 --- /dev/null +++ b/apps/api/src/file-metadata.ts @@ -0,0 +1,213 @@ +/** + * Per-file queryable metadata (`file_metadata` D1 table). + * + * The queryable-tag tier for uploads.sh objects (see + * `.context/2026-07-13-file-metadata-design.md`): capped, mutable key-value + * pairs stored one row per pair, scoped by `(workspace, object_key)`. Distinct + * from R2 custom metadata (`provenance.ts`), which stays unqueryable and + * server/allowlist-controlled. + */ + +import { ValidationError } from "@uploads/errors"; + +/** Lowercase key, optionally namespaced with dots (e.g. `gh.repo`). */ +export const META_KEY_RE = /^[a-z][a-z0-9._-]{0,63}$/; + +/** Cap applied both to a single write request and to a file's total keys post-merge. */ +export const META_MAX_KEYS = 24; + +/** Sum of key+value UTF-8 bytes, enforced per file (and, defensively, per request). */ +export const META_MAX_TOTAL_BYTES = 8192; + +/** Max value length in characters. */ +export const META_VALUE_MAX = 512; + +// Printable ASCII only — same rule as provenance.ts's VALUE_SAFE_RE. +const VALUE_SAFE_RE = /^[\x20-\x7E]+$/; + +const encoder = new TextEncoder(); + +/** + * Throws a `ValidationError` (AppError, type "validation") if `meta` violates + * key format, value format/length, the per-map key-count cap, or the total + * key+value byte cap. Callers use this both on the raw request payload and + * on the post-merge map so a write can never silently exceed the caps. + */ +export function validateMetadataEntries(meta: Record): void { + const keys = Object.keys(meta); + if (keys.length > META_MAX_KEYS) { + throw new ValidationError(`metadata must have at most ${META_MAX_KEYS} keys.`, { + code: "file_metadata_limit_exceeded", + details: { limit: META_MAX_KEYS, count: keys.length }, + }); + } + + let totalBytes = 0; + for (const key of keys) { + if (!META_KEY_RE.test(key)) { + throw new ValidationError(`invalid metadata key: ${key}`, { + code: "file_metadata_invalid_key", + details: { key }, + }); + } + const value = meta[key]; + if ( + typeof value !== "string" || + value.length < 1 || + value.length > META_VALUE_MAX || + !VALUE_SAFE_RE.test(value) + ) { + throw new ValidationError(`invalid metadata value for key: ${key}`, { + code: "file_metadata_invalid_value", + details: { key }, + }); + } + totalBytes += encoder.encode(key).byteLength + encoder.encode(value).byteLength; + } + + if (totalBytes > META_MAX_TOTAL_BYTES) { + throw new ValidationError(`metadata exceeds ${META_MAX_TOTAL_BYTES} total bytes.`, { + code: "file_metadata_limit_exceeded", + details: { limit: META_MAX_TOTAL_BYTES, bytes: totalBytes }, + }); + } +} + +interface MetaRow { + meta_key: string; + meta_value: string; +} + +/** All metadata for one object, keyed by `(workspace, object_key)`. */ +export async function getFileMetadata( + db: D1Database, + workspace: string, + objectKey: string, +): Promise> { + const result = await db + .prepare( + `SELECT meta_key, meta_value FROM file_metadata WHERE workspace = ? AND object_key = ?`, + ) + .bind(workspace, objectKey) + .all(); + const metadata: Record = {}; + for (const row of result.results) metadata[row.meta_key] = row.meta_value; + return metadata; +} + +/** + * Merge `set` into the object's metadata and drop `remove` keys, enforcing + * caps against the post-merge state. Rejects (no write) if the caps would be + * violated; otherwise upserts/deletes atomically and returns the final map. + * `remove` is applied before `set`, so a key present in both ends up set. + */ +export async function setFileMetadata( + db: D1Database, + workspace: string, + objectKey: string, + set: Record, + remove: string[] = [], +): Promise> { + validateMetadataEntries(set); + + const current = await getFileMetadata(db, workspace, objectKey); + const next: Record = { ...current }; + for (const key of remove) delete next[key]; + Object.assign(next, set); + + validateMetadataEntries(next); + + const now = new Date().toISOString(); + const statements = []; + for (const [key, value] of Object.entries(set)) { + statements.push( + db + .prepare( + `INSERT INTO file_metadata (workspace, object_key, meta_key, meta_value, updated_at) + VALUES (?, ?, ?, ?, ?) + ON CONFLICT(workspace, object_key, meta_key) + DO UPDATE SET meta_value = excluded.meta_value, updated_at = excluded.updated_at`, + ) + .bind(workspace, objectKey, key, value, now), + ); + } + for (const key of remove) { + if (key in set) continue; // set wins when a key is both removed and set + statements.push( + db + .prepare( + `DELETE FROM file_metadata WHERE workspace = ? AND object_key = ? AND meta_key = ?`, + ) + .bind(workspace, objectKey, key), + ); + } + if (statements.length > 0) await db.batch(statements); + + return next; +} + +/** Deletes all metadata rows for an object (e.g. on object delete). */ +export async function deleteFileMetadata( + db: D1Database, + workspace: string, + objectKey: string, +): Promise { + await db + .prepare(`DELETE FROM file_metadata WHERE workspace = ? AND object_key = ?`) + .bind(workspace, objectKey) + .run(); +} + +const FIND_DEFAULT_LIMIT = 50; +const FIND_MAX_LIMIT = 500; + +/** + * Finds objects whose metadata matches ALL `filters` (ANDed equality), with + * an optional key-prefix and result limit. Returns each match's key plus its + * full metadata map (not just the matched pairs). + */ +export async function findObjectsByMetadata( + db: D1Database, + workspace: string, + filters: Record, + opts: { prefix?: string; limit?: number } = {}, +): Promise }>> { + const entries = Object.entries(filters); + if (entries.length === 0) return []; + + const limit = Math.max(1, Math.min(opts.limit ?? FIND_DEFAULT_LIMIT, FIND_MAX_LIMIT)); + + const conditions = entries.map(() => `(meta_key = ? AND meta_value = ?)`).join(" OR "); + const params: unknown[] = [workspace]; + for (const [key, value] of entries) params.push(key, value); + + let sql = `SELECT object_key FROM file_metadata WHERE workspace = ? AND (${conditions})`; + if (opts.prefix) { + sql += ` AND object_key LIKE ? || '%'`; + params.push(opts.prefix); + } + sql += ` GROUP BY object_key HAVING COUNT(DISTINCT meta_key) = ? ORDER BY object_key LIMIT ?`; + params.push(entries.length, limit); + + const matched = await db + .prepare(sql) + .bind(...params) + .all<{ object_key: string }>(); + const keys = matched.results.map((row) => row.object_key); + if (keys.length === 0) return []; + + const placeholders = keys.map(() => "?").join(", "); + const hydrated = await db + .prepare( + `SELECT object_key, meta_key, meta_value FROM file_metadata + WHERE workspace = ? AND object_key IN (${placeholders})`, + ) + .bind(workspace, ...keys) + .all<{ object_key: string; meta_key: string; meta_value: string }>(); + + const byKey = new Map>(keys.map((key) => [key, {}])); + for (const row of hydrated.results) { + byKey.get(row.object_key)![row.meta_key] = row.meta_value; + } + return keys.map((key) => ({ key, metadata: byKey.get(key)! })); +} diff --git a/apps/api/test/file-metadata-sqlite.test.ts b/apps/api/test/file-metadata-sqlite.test.ts new file mode 100644 index 0000000..8e6a6fb --- /dev/null +++ b/apps/api/test/file-metadata-sqlite.test.ts @@ -0,0 +1,265 @@ +/// + +import { readFileSync } from "node:fs"; +import { DatabaseSync } from "node:sqlite"; +import { AppError } from "@uploads/errors"; +import { describe, expect, it } from "vitest"; +import { + META_MAX_KEYS, + deleteFileMetadata, + findObjectsByMetadata, + getFileMetadata, + setFileMetadata, +} from "../src/file-metadata"; + +type SqliteValue = string | number | bigint | null | Uint8Array; + +class SqliteStatement { + private values: SqliteValue[] = []; + + constructor( + readonly owner: SqliteD1, + readonly sql: string, + ) {} + + bind(...values: unknown[]) { + this.values = values as SqliteValue[]; + return this; + } + + async first(): Promise { + return (this.owner.db.prepare(this.sql).get(...this.values) as T | undefined) ?? null; + } + + async all(): Promise> { + return { + success: true, + results: this.owner.db.prepare(this.sql).all(...this.values) as T[], + meta: {}, + } as D1Result; + } + + async run(): Promise { + return this.runSync() as unknown as D1Result; + } + + runSync() { + const result = this.owner.db.prepare(this.sql).run(...this.values); + return { + success: true, + results: [], + meta: { changes: Number(result.changes) }, + }; + } +} + +class SqliteD1 { + readonly db = new DatabaseSync(":memory:"); + + constructor() { + this.db.exec(readFileSync("migrations/20260713210559_file_metadata.sql", "utf8")); + } + + prepare(sql: string) { + return new SqliteStatement(this, sql); + } + + async batch(statements: SqliteStatement[]): Promise { + this.db.exec("BEGIN IMMEDIATE"); + try { + const results = statements.map((statement) => statement.runSync() as unknown as D1Result); + this.db.exec("COMMIT"); + return results; + } catch (error) { + this.db.exec("ROLLBACK"); + throw error; + } + } + + close() { + this.db.close(); + } +} + +function database(sqlite: SqliteD1): D1Database { + return sqlite as unknown as D1Database; +} + +describe("file metadata persistence against SQLite", () => { + it("returns an empty map for an object with no metadata", async () => { + const sqlite = new SqliteD1(); + try { + await expect(getFileMetadata(database(sqlite), "alpha", "f/one.png")).resolves.toEqual({}); + } finally { + sqlite.close(); + } + }); + + it("sets, reads, and scopes metadata by workspace and object key", async () => { + const sqlite = new SqliteD1(); + try { + const result = await setFileMetadata(database(sqlite), "alpha", "f/one.png", { + app: "screenshots", + "gh.repo": "buildinternet/uploads", + }); + expect(result).toEqual({ app: "screenshots", "gh.repo": "buildinternet/uploads" }); + + await expect(getFileMetadata(database(sqlite), "alpha", "f/one.png")).resolves.toEqual({ + app: "screenshots", + "gh.repo": "buildinternet/uploads", + }); + // Different workspace / different key must not see it. + await expect(getFileMetadata(database(sqlite), "beta", "f/one.png")).resolves.toEqual({}); + await expect(getFileMetadata(database(sqlite), "alpha", "f/two.png")).resolves.toEqual({}); + } finally { + sqlite.close(); + } + }); + + it("merges set and remove in the same call, upserting and deleting keys", async () => { + const sqlite = new SqliteD1(); + try { + await setFileMetadata(database(sqlite), "alpha", "f/one.png", { + app: "screenshots", + page: "/checkout", + device: "iphone", + }); + + const merged = await setFileMetadata( + database(sqlite), + "alpha", + "f/one.png", + { page: "/cart", resolution: "1170x2532" }, + ["device"], + ); + + expect(merged).toEqual({ + app: "screenshots", + page: "/cart", + resolution: "1170x2532", + }); + await expect(getFileMetadata(database(sqlite), "alpha", "f/one.png")).resolves.toEqual( + merged, + ); + } finally { + sqlite.close(); + } + }); + + it("rejects a merge that would push the post-merge file over the key cap", async () => { + const sqlite = new SqliteD1(); + try { + const initial: Record = {}; + for (let i = 0; i < META_MAX_KEYS; i++) initial[`k${i}`] = "v"; + await setFileMetadata(database(sqlite), "alpha", "f/one.png", initial); + + await expect( + setFileMetadata(database(sqlite), "alpha", "f/one.png", { extra: "v" }), + ).rejects.toBeInstanceOf(AppError); + + // Rejected merge must not have partially applied. + await expect(getFileMetadata(database(sqlite), "alpha", "f/one.png")).resolves.toEqual( + initial, + ); + } finally { + sqlite.close(); + } + }); + + it("allows a merge at the cap when it simultaneously removes a key", async () => { + const sqlite = new SqliteD1(); + try { + const initial: Record = {}; + for (let i = 0; i < META_MAX_KEYS; i++) initial[`k${i}`] = "v"; + await setFileMetadata(database(sqlite), "alpha", "f/one.png", initial); + + const result = await setFileMetadata(database(sqlite), "alpha", "f/one.png", { extra: "v" }, [ + "k0", + ]); + expect(Object.keys(result)).toHaveLength(META_MAX_KEYS); + expect(result.extra).toBe("v"); + expect(result.k0).toBeUndefined(); + } finally { + sqlite.close(); + } + }); + + it("deletes all metadata for an object", async () => { + const sqlite = new SqliteD1(); + try { + await setFileMetadata(database(sqlite), "alpha", "f/one.png", { app: "screenshots" }); + await deleteFileMetadata(database(sqlite), "alpha", "f/one.png"); + await expect(getFileMetadata(database(sqlite), "alpha", "f/one.png")).resolves.toEqual({}); + } finally { + sqlite.close(); + } + }); + + it("finds only objects matching ALL AND-ed filters, scoped by workspace", async () => { + const sqlite = new SqliteD1(); + try { + await setFileMetadata(database(sqlite), "alpha", "f/one.png", { + app: "screenshots", + page: "/checkout", + }); + await setFileMetadata(database(sqlite), "alpha", "f/two.png", { + app: "screenshots", + page: "/cart", + }); + await setFileMetadata(database(sqlite), "alpha", "f/three.png", { + app: "other", + page: "/checkout", + }); + await setFileMetadata(database(sqlite), "beta", "f/one.png", { + app: "screenshots", + page: "/checkout", + }); + + const matches = await findObjectsByMetadata(database(sqlite), "alpha", { + app: "screenshots", + page: "/checkout", + }); + expect(matches).toEqual([ + { key: "f/one.png", metadata: { app: "screenshots", page: "/checkout" } }, + ]); + } finally { + sqlite.close(); + } + }); + + it("combines a key prefix filter with metadata AND-filters", async () => { + const sqlite = new SqliteD1(); + try { + await setFileMetadata(database(sqlite), "alpha", "gh/one.png", { app: "gh" }); + await setFileMetadata(database(sqlite), "alpha", "screenshots/one.png", { app: "gh" }); + + const matches = await findObjectsByMetadata( + database(sqlite), + "alpha", + { app: "gh" }, + { prefix: "gh/" }, + ); + expect(matches).toEqual([{ key: "gh/one.png", metadata: { app: "gh" } }]); + } finally { + sqlite.close(); + } + }); + + it("respects a limit and returns an empty array for no filters or no matches", async () => { + const sqlite = new SqliteD1(); + try { + await setFileMetadata(database(sqlite), "alpha", "f/one.png", { app: "a" }); + await setFileMetadata(database(sqlite), "alpha", "f/two.png", { app: "a" }); + + await expect(findObjectsByMetadata(database(sqlite), "alpha", {})).resolves.toEqual([]); + await expect( + findObjectsByMetadata(database(sqlite), "alpha", { app: "nope" }), + ).resolves.toEqual([]); + await expect( + findObjectsByMetadata(database(sqlite), "alpha", { app: "a" }, { limit: 1 }), + ).resolves.toHaveLength(1); + } finally { + sqlite.close(); + } + }); +}); diff --git a/apps/api/test/file-metadata.test.ts b/apps/api/test/file-metadata.test.ts new file mode 100644 index 0000000..4ad4648 --- /dev/null +++ b/apps/api/test/file-metadata.test.ts @@ -0,0 +1,93 @@ +import { AppError } from "@uploads/errors"; +import { describe, expect, it } from "vitest"; +import { + META_KEY_RE, + META_MAX_KEYS, + META_MAX_TOTAL_BYTES, + META_VALUE_MAX, + validateMetadataEntries, +} from "../src/file-metadata"; + +describe("META_KEY_RE", () => { + it("accepts lowercase, digit, dot, underscore, and dash keys starting with a letter", () => { + for (const key of ["app", "gh.repo", "device_type", "resolution-2x", "a", "a".repeat(64)]) { + expect(META_KEY_RE.test(key)).toBe(true); + } + }); + + it("rejects keys that are empty, too long, uppercase, or start wrong", () => { + for (const key of [ + "", + "a".repeat(65), + "Gh.repo", + "1abc", + "_abc", + "-abc", + "has space", + "emoji😀", + ]) { + expect(META_KEY_RE.test(key)).toBe(false); + } + }); +}); + +describe("validateMetadataEntries", () => { + it("accepts a well-formed map", () => { + expect(() => validateMetadataEntries({ app: "screenshots", "gh.repo": "a/b" })).not.toThrow(); + }); + + it("throws AppError for an invalid key", () => { + expect(() => validateMetadataEntries({ "Bad Key": "x" })).toThrow(AppError); + }); + + it("throws AppError for an empty value", () => { + expect(() => validateMetadataEntries({ app: "" })).toThrow(AppError); + }); + + it("throws AppError for a value over META_VALUE_MAX", () => { + expect(() => validateMetadataEntries({ app: "x".repeat(META_VALUE_MAX + 1) })).toThrow( + AppError, + ); + expect(() => validateMetadataEntries({ app: "x".repeat(META_VALUE_MAX) })).not.toThrow(); + }); + + it("throws AppError for a non-printable value", () => { + expect(() => validateMetadataEntries({ app: "café" })).toThrow(AppError); + expect(() => validateMetadataEntries({ app: "line\nbreak" })).toThrow(AppError); + }); + + it("throws AppError when the map has more than META_MAX_KEYS entries", () => { + const tooMany: Record = {}; + for (let i = 0; i < META_MAX_KEYS + 1; i++) tooMany[`k${i}`] = "v"; + expect(() => validateMetadataEntries(tooMany)).toThrow(AppError); + + const atCap: Record = {}; + for (let i = 0; i < META_MAX_KEYS; i++) atCap[`k${i}`] = "v"; + expect(() => validateMetadataEntries(atCap)).not.toThrow(); + }); + + it("throws AppError when total key+value UTF-8 bytes exceed META_MAX_TOTAL_BYTES", () => { + // One giant value alone should trip the total-bytes cap even though it's a + // single key under META_MAX_KEYS and under META_VALUE_MAX. + const many: Record = {}; + let bytes = 0; + for (let i = 0; i < META_MAX_KEYS && bytes <= META_MAX_TOTAL_BYTES; i++) { + const key = `k${i}`; + const value = "x".repeat(META_VALUE_MAX); + many[key] = value; + bytes += key.length + value.length; + } + expect(() => validateMetadataEntries(many)).toThrow(AppError); + }); + + it("is a validation-family AppError with the invalid-request status", () => { + try { + validateMetadataEntries({ "Bad Key": "x" }); + throw new Error("expected validateMetadataEntries to throw"); + } catch (err) { + expect(err).toBeInstanceOf(AppError); + expect((err as AppError).type).toBe("validation"); + expect((err as AppError).status).toBe(400); + } + }); +}); diff --git a/packages/errors/src/codes.ts b/packages/errors/src/codes.ts index ffc737e..b95de81 100644 --- a/packages/errors/src/codes.ts +++ b/packages/errors/src/codes.ts @@ -56,6 +56,11 @@ export const ERROR_CODES = [ "gallery_reference_not_found", "gallery_invalid_reference", + // File metadata + "file_metadata_invalid_key", + "file_metadata_invalid_value", + "file_metadata_limit_exceeded", + // Admin "invalid_workspace", "workspace_not_found", From bd12c2ed5303f8ee83dabdea722c7a43375291cd Mon Sep 17 00:00:00 2001 From: Zach Dunn Date: Mon, 13 Jul 2026 17:15:47 -0400 Subject: [PATCH 02/23] docs(api): note accepted concurrency tradeoff in setFileMetadata --- apps/api/src/file-metadata.ts | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/apps/api/src/file-metadata.ts b/apps/api/src/file-metadata.ts index a04fb87..ab7c423 100644 --- a/apps/api/src/file-metadata.ts +++ b/apps/api/src/file-metadata.ts @@ -100,6 +100,12 @@ export async function getFileMetadata( * caps against the post-merge state. Rejects (no write) if the caps would be * violated; otherwise upserts/deletes atomically and returns the final map. * `remove` is applied before `set`, so a key present in both ends up set. + * + * Concurrency: the read → validate → batch write is not guarded, so two + * concurrent merges on the same object can land a combined state slightly + * over the caps (same accepted last-write-wins tradeoff as visibility + * rewrites in files-core.ts; single-tenant writes sit behind the write + * rate limiter). Caps are re-enforced on the next merge. */ export async function setFileMetadata( db: D1Database, From db68398d93c29d0fe10a7a210681d3cab3ef16b8 Mon Sep 17 00:00:00 2001 From: Zach Dunn Date: Mon, 13 Jul 2026 17:23:56 -0400 Subject: [PATCH 03/23] feat(api): capture custom metadata at upload; cascade on delete PUT splits X-Uploads-Meta-* headers: allowlisted keys still go to R2 provenance unchanged, everything else is validated (file-metadata.ts) and stored in D1 via setFileMetadata inside putObject. Invalid keys/values or a >24-key cap breach now reject the upload instead of being silently dropped. Overwriting a key does a full delete-then-set replace of its metadata; deleteObject cascades via deleteFileMetadata; dryRun writes nothing. --- apps/api/src/files-core.ts | 31 ++++++++++++++-- apps/api/src/provenance.ts | 28 +++++++++++++++ apps/api/src/routes/files.ts | 5 +-- apps/api/test/routes-files.test.ts | Bin 8619 -> 14942 bytes apps/api/test/routes-galleries.test.ts | 8 +++++ apps/api/test/routes-key-policy.test.ts | 3 ++ apps/api/test/routes-public-files.test.ts | 6 ++++ apps/api/test/usage-fake-d1.ts | 41 ++++++++++++++++++++++ 8 files changed, 118 insertions(+), 4 deletions(-) diff --git a/apps/api/src/files-core.ts b/apps/api/src/files-core.ts index 0da06e6..5d3fe77 100644 --- a/apps/api/src/files-core.ts +++ b/apps/api/src/files-core.ts @@ -13,6 +13,7 @@ import { } from "@uploads/errors"; import type { Files } from "@uploads/storage"; import { checkPutBudget } from "./budget"; +import { deleteFileMetadata, setFileMetadata, validateMetadataEntries } from "./file-metadata"; import { DEFAULT_MAX_UPLOAD_BYTES, inspectUpload, resolveUploadPolicy } from "./guards"; import { checkKeyPolicy, resolveKeyPolicy } from "./key-policy"; import { @@ -113,7 +114,19 @@ export async function putObject( key: string, bytes: Uint8Array, workspaceName: string, - opts?: { provenance?: Record; visibility?: Visibility }, + opts?: { + provenance?: Record; + visibility?: Visibility; + /** + * Custom queryable metadata (D1 `file_metadata`), distinct from the R2 + * `provenance` bag above. When present (even `{}`), this call fully + * replaces any metadata already stored for the key — delete-then-set — + * so an overwrite never leaves stale rows from a prior put. Omit + * entirely (undefined) to leave existing metadata untouched, which is + * what non-route callers (e.g. the MCP worker) do today. + */ + metadata?: Record; + }, ): Promise<{ key: string; url: string | null; @@ -125,6 +138,10 @@ export async function putObject( const finalKey = finalizeUploadKey(key, ws); if (bytes.byteLength === 0) throw new ValidationError("empty body", { code: "empty_body" }); + // Validate custom metadata before any write so a bad key/value or a + // cap breach rejects the whole upload instead of landing bytes first. + if (opts?.metadata) validateMetadataEntries(opts.metadata); + const inspection = inspectUpload(bytes, resolveUploadPolicy(ws)); if (!inspection.ok) throw inspection.error; @@ -169,6 +186,15 @@ export async function putObject( metadata: storageMetadata, }); + if (opts?.metadata) { + // Full replace: an overwrite must not inherit a prior put's custom + // metadata, so clear the row set before (re-)writing this request's. + await deleteFileMetadata(env.DB, workspaceName, finalKey); + if (Object.keys(opts.metadata).length > 0) { + await setFileMetadata(env.DB, workspaceName, finalKey, opts.metadata); + } + } + await recordUsageSafe(env.DB, workspaceName, { bytes: deltaBytes, objects: prev === null ? 1 : 0, @@ -311,7 +337,7 @@ export async function listObjects( }; } -/** Delete an object and decrement the workspace ledger when size was known. */ +/** Delete an object (and its D1 custom metadata) and decrement the workspace ledger when size was known. */ export async function deleteObject( env: Env, ws: WorkspaceRecord, @@ -324,6 +350,7 @@ export async function deleteObject( const prev = await existingSize(store, key); await store.delete(key); + await deleteFileMetadata(env.DB, workspaceName, key); if (prev !== null) { await recordUsageSafe(env.DB, workspaceName, { diff --git a/apps/api/src/provenance.ts b/apps/api/src/provenance.ts index ce3dccc..f995c9c 100644 --- a/apps/api/src/provenance.ts +++ b/apps/api/src/provenance.ts @@ -98,6 +98,34 @@ export function provenanceFromHeaders( return raw; } +const META_HEADER_PREFIX = "x-uploads-meta-"; + +/** + * Split every `X-Uploads-Meta-` request header into the allowlisted + * provenance bag (stored as R2 custom metadata, unchanged) and everything + * else (candidate custom file metadata, stored in D1 — see `file-metadata.ts`). + * Unlike `sanitizeProvenance`, non-allowlisted keys are surfaced here rather + * than dropped: the caller is responsible for validating and persisting them + * (`validateMetadataEntries`/`setFileMetadata`), so bad input rejects the + * upload instead of silently vanishing. + */ +export function splitUploadMetaHeaders(headers: Headers): { + provenance: Record; + custom: Record; +} { + const provenance: Record = {}; + const custom: Record = {}; + for (const [rawName, rawValue] of headers.entries()) { + const name = rawName.toLowerCase(); + if (!name.startsWith(META_HEADER_PREFIX)) continue; + const key = name.slice(META_HEADER_PREFIX.length); + if (!key || rawValue === "") continue; + if (CLIENT_ALLOWED.has(key)) provenance[key] = rawValue; + else custom[key] = rawValue; + } + return { provenance, custom }; +} + /** Lowercase hex SHA-256 of the stored object body. */ export async function contentSha256Hex(bytes: Uint8Array): Promise { const digest = await crypto.subtle.digest("SHA-256", new Uint8Array(bytes)); diff --git a/apps/api/src/routes/files.ts b/apps/api/src/routes/files.ts index 7fbfdff..224a3bd 100644 --- a/apps/api/src/routes/files.ts +++ b/apps/api/src/routes/files.ts @@ -8,7 +8,7 @@ import { listObjects, putObject, } from "../files-core"; -import { provenanceFromHeaders } from "../provenance"; +import { splitUploadMetaHeaders } from "../provenance"; import { publicUrl, storage, storageConfig } from "../storage"; import { requireScope, type WorkspaceVars } from "../workspace"; import { checkDeclaredLength, resolveUploadPolicy, writeRateLimit } from "../guards"; @@ -105,13 +105,14 @@ export const files = new Hono() const body = await c.req.arrayBuffer(); const visibility = sanitizeVisibility(c.req.header("x-uploads-visibility")); + const { provenance, custom } = splitUploadMetaHeaders(c.req.raw.headers); const result = await putObject( c.env, c.get("workspace"), key, new Uint8Array(body), c.get("workspaceName"), - { provenance: provenanceFromHeaders((n) => c.req.header(n)), visibility }, + { provenance, visibility, metadata: custom }, ); return c.json({ workspace: c.get("workspaceName"), ...result }, 201); }) diff --git a/apps/api/test/routes-files.test.ts b/apps/api/test/routes-files.test.ts index 0d6f26ed84e2567f764a33102602b1a7ea4d8c59..05ec4e0fbb53eb2c6a3b9ee227b492ae28ff2f4e 100644 GIT binary patch literal 14942 zcmds8ZFAek5$WsPOYiEGR9=p}AORV|P_l88V60YFKN%9;L% z{=)u}KD&1Zf-j(GIf*mXjA;rlcYC|f?moMB@&|F0WOPd{)r}JMA_!{KQE5BzTdGFt zCRXjNMt(+P>L$^EDp!7{(ya1y&9`s9@-EfU#_q6vsV0xPjV>uj6Ij?DJjRs}Ic0;(KY|+j#uV_v)rb*(g@@AxbXO*lVjJ z)sB+R>^RrE#*ceG8FRJ9Y8)wAeQg?u|#jpI~l`#IgN5&2zmzH-{h zD9$1`9k#MSk(bi3e0wRE>CaGW<`4X^ckFf5{?9`%*x90Cco|05A&sl<%+0EdOmx`# z30ikMs_Td9eGQZg{q1J zw&n*EaqGvno{BR*MM^PRg1&?Yn!|?dF!11NiFkrGP04SI5A|+rykctNn=sg zc#Z!zhhimkZ*zIX=owB}{Q5e;u zIK_8G!_@1kdZ0Qzl~6j=Q#tSr?M78<4Kr$a?aP!$xnPI>XdU{3uKlb}9?a(j&^0`L z5G5nXgxFN0G?E$Y)AhEOw!MzxaayYHUHLGfAJRoU%(M?(G%h*{HdW?VYI`9bMO;%! z+fP-8AUg}4M8izR3DlJeL<(GiAl+dQQ2KL#s8?s9@e0=Lq)O3LD5~pcTn>(ICQD;L zv2v;|1Z=EzhhaPOqmTwr94l{c7vba#KhqYhQSrOJ_WZdc*we43Z3G-lh*zCl6bp+Q z|com@7)&G)@@6t)4HB+ZQ7^?vzZdM#x3ns1+AXd|BW$nq@;mKzfON;-{b*vX? z5b4Dj!Q#?Ins+SHCNL|^4LNsqB>5}=?8MA^ih&oq$Un|RTmiII@C0)5)be?2aL{<_ z^)V@A5lQ(4b^BpE7=kJ}6>&gz#%gLA1M4 z7ZiI)o$Sn$AFaf4E3IIj8MAzfDRw|cIE=D_S5Qq6iF^BR_fPie)zRU>T}XtcKJ+6f zO2?RquJq(n>RMerOEgoa5wqCkIGNT%!B{r3Ao;c1Dn%&Ck|DAeD|`!-u-*5~1R%(-2xwQ-dh{(&YL|8z>}VtqUAY_&!D7RBbX7&JmdnRdJxgLlgbFGD~YRWR3!642JlFe5R0^E zKfGc~AqXakkJ!3J?~&7dFL?IB)@!<)dk#zA%2-isg!VG^7R6R}c!?sb6(Np*>y6EG zT4swCE(o{gDnyJZ>xz@;3X>;WR6+Tw4c(IJ7FULG5P6-nj^fON-`d;Y4v6u%fBOFI z;fuZFk9+&CUYx!?G55i7y8aDro7Wol`mh!F?OiWbrwQ0krJrSSy47ef-vx_y&1`Pk zH!ny{`n{L-x5&HpP?qOquOsDaRf`KB$aTn9#OQkUdL>W`*LB6xy~qJOT&ERzDhDS0 zz6!5cr=qE&{qJ5MpB(+jpg}^1Ou}(AbP}n%YXmYrePf7{pDSC`X>Ur%E$*LtQx(v>S;8oWg{ID)LL?R3Y$5v9oOs5+JTG*{|opApGOh!o! zqDl8;rO3ikYv|;=x4T81mKJixuz;9ALf`yw^!jA~WmtFXhkJy15>zDRSL{@+>bJLN8A%f^hCAkGw!cMT|zepMEvI= zW_*6Oan71R8wrB~A6&U29PQ$_*P)NbfSHG(Gv+(mjq{?!6e2VJ%D}+rFcPl_)t@)a zqPlh_f&?vX##Dt6Eu~I`k1)z$YB95DhPf15(K zj2jDEF|3YQ>}>mp^Cltahmpnr4gG*c9$q)k=r*K-hq%behAE1@XjeHK&Go5Keo9eZ zqiogV@Z)^NK`8;mfp}@bS>d+Qd`d$Oyc&K_8D{nun~KfcJdj%$z(`QA+O(|lJhNpj zX=Nd%7Q6y+>?`HTAjj(pqv|jPy`h8R_C1s<7VL{(HnVdy67i%RrQwmM6d)uRaYT7p zz_5Y;JYG0l{Khyz@3ZKACBBtj^&Z^}f(pQ>n{5bf(%@c(Ec3*a8o7AALybEtfTf_? zL~+ZPT`!Enx^}$S8!^|=3f)|Pe3uUI>0_)rjxAfWonAB=O^dN5ZAEkt(PG?`&n8$P z#OK!4FMf>hQLaizJ`1FLwV_bGR60A7Ii&r!x&HM%)(Y0{-JA+N+eP~r+;KBE^XCK^ zNK+<1q767EyMO($?IZ%O<*$lQ1`yQU6!9T+lq6{6q@NtF{flcz`E;aqFb>Cs!U6Suc{u`A&6 zK1qCzXA!N)w05wDqhYwcp7ScH#|w0;80zmLTHIn;7~L2%nobG9)n5$?%d>(8k*$U9 z;2Q-tF~c+T5({PO(;|3zSWd&TW9V4QnbSG=u1Vp!hGeG29K)_rc+Qdnb2+E5qgcVE zqa`xWWYVcTxjNktrzi)rO^%FYMs%(ViX(s;ZB;)-_aVAwPP>$lE=ZEMB(JK@8#Q__ zg*qpXvr4aDK}*g+Vm>haUm|>SK1|x8fYlrLQjKwk1T`0Lp`sAd`}C+kO^ucEaRH=8 z@OXqBu4C>p%6$S%p-MoA6(|-fFmnM>=kIk*3ex%kWUappd=xL708MkH-VXfSk;uHcGFN!dOnI5}&8F!Qp(~_kv=A?kdY@EfSwx@nM|TVx}&TeL-{4AN!w&uD)}7gEJp*>>Es3c`RC?Yiw& zInU*F8vi_N)}MIw?)mLEo8t$K=~S}9KxkZ;TCrI)*7P`2;dQk#WC#ZyF!XuLb&^)5 zB*G*DJ{{%QW%FwM{$VZ$=u~ZXcfz)~EX~U--)tjbIx+JTf17Fs$UD4d+YKr*g>jL+SpR5TEKb$arp{k0h-hD*^omN}v9g!j8}6^I zUTFNpiGRh+_sUQGmX9+JBYo(LQ?7PATjX=JH)lI+xJPfUkM0nQeOJ6+kc7=TGyqGw zL9;c@B(ndI3oPMOFJqO`<`X+e6U899>88=5KGEbcYcQkPidMPhkF2Xdq@zvZz#U#x zfJ&K0avU$!$`l;^9`@N$yr8=DL*0vzqb702SsHqRy;TAbf02 zaG~Ew)6?OMJGh_I@U>C!UR2~B>7ErVN?xHG-U^Mms`*F`H9u(C|!mK1M|5 z4UVub1&bl$lPJhZzW&ho2f}ig$*$UUAG;bzQYz+_ZShsGPBSvFzRR*WAKdy4 z2>TpSe-XzV%&%1o5|y)fX783UufmF7ze+?7mO$n4Ki+0dV z-BCp)qMK9TLZ-#Rdx^%@rs6KdszSHekPzt-<6tOn5+g@2J(+bU?i9eKYPQG4`-1(I zdR?od+l0b0dWd*_Ynb5VY%wl2af+Cp(Z*x^&nJn^FlQGto_zV>)*oN+VVlZTWs0ER z&5!cFMOjg23;o65x=-Tz~Irh-7FON-O=V1Tj#m6`MKYl#guhM`2{U3A<=c=K3dBLB&f{80mn*@XX)We(ps$b@VmjAx*d z-il=At_UfAkdPbsj;{I~4=-STo}|CWfhMN#jdowPFR|fRM$|=yS-aV$^B1*S!8dS{ z5$l;*4hVh0UGRs45gKFGW2Ti^MEYzwGtm_2!GavE2mWJfDA$S>T}m8=JOBG17W8R< delta 128 zcmcatvf6pV#EmC4nKxJS&1RfzC@|AjAvd)owJ0+$U5`rv2wa@36so~21%-m5)PlsK zR4WAyO$A#!1r3 { "utf8", ), ); + // Task 2: putObject/deleteObject (exercised via fileRequest below) now + // read/write `file_metadata` on every put/delete. + db.exec( + readFileSync( + fileURLToPath(new NodeURL("../migrations/20260713210559_file_metadata.sql", import.meta.url)), + "utf8", + ), + ); bucket = new FakeR2Bucket(); await bucket.put("alpha/screenshots/one.png", PNG); records = { diff --git a/apps/api/test/routes-key-policy.test.ts b/apps/api/test/routes-key-policy.test.ts index 9df4a97..f8d28de 100644 --- a/apps/api/test/routes-key-policy.test.ts +++ b/apps/api/test/routes-key-policy.test.ts @@ -45,6 +45,9 @@ async function makeEnv(overrides: Partial = {}) { async run() { return { success: true, meta: { changes: 0 }, results: [] }; }, + async all() { + return { success: true, results: [], meta: {} }; + }, }), async batch(stmts: { run: () => Promise }[]) { return Promise.all(stmts.map((s) => s.run())); diff --git a/apps/api/test/routes-public-files.test.ts b/apps/api/test/routes-public-files.test.ts index c8081f4..ec15605 100644 --- a/apps/api/test/routes-public-files.test.ts +++ b/apps/api/test/routes-public-files.test.ts @@ -41,6 +41,9 @@ async function makeEnv(overrides: Partial = {}) { const bucket = new FakeR2Bucket(); const env = { REGISTRY: { get: async () => record, put: async () => undefined }, + // No-op D1: this suite doesn't assert on file_metadata rows, just that + // putObject's D1 metadata write (`.all()` read-before-write inside + // setFileMetadata) doesn't blow up. DB: { prepare: () => ({ bind() { @@ -52,6 +55,9 @@ async function makeEnv(overrides: Partial = {}) { async run() { return { success: true, meta: { changes: 0 }, results: [] }; }, + async all() { + return { success: true, results: [], meta: {} }; + }, }), async batch(stmts: { run: () => Promise }[]) { return Promise.all(stmts.map((s) => s.run())); diff --git a/apps/api/test/usage-fake-d1.ts b/apps/api/test/usage-fake-d1.ts index 9f4701c..fcf50c5 100644 --- a/apps/api/test/usage-fake-d1.ts +++ b/apps/api/test/usage-fake-d1.ts @@ -14,10 +14,15 @@ export type UsageRow = { export class UsageFakeD1 { usage = new Map(); + // Backs `file_metadata` so putObject/deleteObject's D1 metadata + // cascade (Task 2) doesn't blow up in suites that only care about the + // usage ledger. Keyed by `${workspace} ${objectKey}`. + fileMetadata = new Map>(); prepare = (sql: string) => { const normalized = sql.replace(/\s+/g, " ").trim(); let values: unknown[] = []; + const metaScopeKey = (workspace: string, objectKey: string) => `${workspace} ${objectKey}`; const stmt = { bind: (...v: unknown[]) => { @@ -31,7 +36,43 @@ export class UsageFakeD1 { } throw new Error(`unsupported first: ${normalized}`); }, + all: async () => { + if (normalized.startsWith("SELECT meta_key, meta_value FROM file_metadata")) { + const [workspace, objectKey] = values as [string, string]; + const map = this.fileMetadata.get(metaScopeKey(workspace, objectKey)) ?? new Map(); + return { + success: true as const, + results: [...map.entries()].map(([meta_key, meta_value]) => ({ + meta_key, + meta_value, + })) as T[], + meta: {}, + }; + } + throw new Error(`unsupported all: ${normalized}`); + }, run: async () => { + if (normalized.startsWith("INSERT INTO file_metadata")) { + const [workspace, objectKey, key, value] = values as [string, string, string, string]; + const scope = metaScopeKey(workspace, objectKey); + const map = this.fileMetadata.get(scope) ?? new Map(); + map.set(key, value); + this.fileMetadata.set(scope, map); + return { success: true as const, meta: { changes: 1 }, results: [] }; + } + if ( + normalized.startsWith("DELETE FROM file_metadata") && + normalized.includes("meta_key = ?") + ) { + const [workspace, objectKey, key] = values as [string, string, string]; + this.fileMetadata.get(metaScopeKey(workspace, objectKey))?.delete(key); + return { success: true as const, meta: { changes: 1 }, results: [] }; + } + if (normalized.startsWith("DELETE FROM file_metadata")) { + const [workspace, objectKey] = values as [string, string]; + this.fileMetadata.delete(metaScopeKey(workspace, objectKey)); + return { success: true as const, meta: { changes: 1 }, results: [] }; + } if (normalized.startsWith("INSERT OR IGNORE INTO workspace_usage")) { // applyUsageDelta: (ws, period, updatedAt) with zeros // setUsageTotals: (ws, bytes, objects, period, updatedAt) From 1ab10780885dc157b7059b543e8bf2250040607e Mon Sep 17 00:00:00 2001 From: Zach Dunn Date: Mon, 13 Jul 2026 17:30:29 -0400 Subject: [PATCH 04/23] fix(api): reject empty custom metadata values instead of dropping them splitUploadMetaHeaders pre-filtered empty header values before validation, reproducing the old silent drop for X-Uploads-Meta- with an empty value. Empty (and empty-key) custom entries now flow to validateMetadataEntries so the upload 400s with the typed error; the allowlisted provenance branch keeps its historical lenience. --- apps/api/src/provenance.ts | 14 +++++++++++--- apps/api/test/routes-files.test.ts | Bin 14942 -> 15947 bytes 2 files changed, 11 insertions(+), 3 deletions(-) diff --git a/apps/api/src/provenance.ts b/apps/api/src/provenance.ts index f995c9c..74fe7f9 100644 --- a/apps/api/src/provenance.ts +++ b/apps/api/src/provenance.ts @@ -119,9 +119,17 @@ export function splitUploadMetaHeaders(headers: Headers): { const name = rawName.toLowerCase(); if (!name.startsWith(META_HEADER_PREFIX)) continue; const key = name.slice(META_HEADER_PREFIX.length); - if (!key || rawValue === "") continue; - if (CLIENT_ALLOWED.has(key)) provenance[key] = rawValue; - else custom[key] = rawValue; + if (CLIENT_ALLOWED.has(key)) { + // Provenance keeps its historical lenience: an empty value is ignored, + // matching provenanceFromHeaders (sanitizeProvenance drops it anyway). + if (rawValue !== "") provenance[key] = rawValue; + } else { + // Custom keys must not pre-filter: empty values (and even an empty key + // from a bare `X-Uploads-Meta-` header name) flow to + // validateMetadataEntries so the upload rejects with a typed error + // instead of reproducing the old silent drop. + custom[key] = rawValue; + } } return { provenance, custom }; } diff --git a/apps/api/test/routes-files.test.ts b/apps/api/test/routes-files.test.ts index 05ec4e0fbb53eb2c6a3b9ee227b492ae28ff2f4e..ff948bc09a048854e453b6c044ffbd351b0600e6 100644 GIT binary patch delta 380 zcmX|-(Mm!=5QZ-WE9#;kj9@sX9tF+3DHNi-?H#h^oKZK{-DP*Q#C(Rpd4iw^2!dXs zH|U}#2yeTrRnfhhoqzuM=KrS8r#IW+uliOC2bRW=3_Y0v43I=BNWe&_A%fJNP^4hG z;H5%MY7D6})=J$2NepLgelDJK^J06JEASp<_@h!PpmWAGVFEO^*aS~5=v$DObW_2U zuS@&4OJ!{#E7!=hF^ Date: Mon, 13 Jul 2026 17:42:06 -0400 Subject: [PATCH 05/23] feat(api): file metadata read/patch endpoints Adds GET/PATCH /v1/:workspace/files/:key/metadata on top of Task 1's file-metadata.ts (caps, merge semantics) and Task 2's put/delete cascade, so callers can read and update an object's queryable metadata without re-uploading the file. --- apps/api/src/routes/files.ts | 72 +++++++++++++++++++++++++++++ apps/api/test/routes-files.test.ts | Bin 15947 -> 24100 bytes 2 files changed, 72 insertions(+) diff --git a/apps/api/src/routes/files.ts b/apps/api/src/routes/files.ts index 224a3bd..af77a2e 100644 --- a/apps/api/src/routes/files.ts +++ b/apps/api/src/routes/files.ts @@ -8,6 +8,7 @@ import { listObjects, putObject, } from "../files-core"; +import { getFileMetadata, setFileMetadata } from "../file-metadata"; import { splitUploadMetaHeaders } from "../provenance"; import { publicUrl, storage, storageConfig } from "../storage"; import { requireScope, type WorkspaceVars } from "../workspace"; @@ -123,6 +124,77 @@ export const files = new Hono() return c.json(await listObjects(c.env, c.get("workspace"), { prefix, limit, cursor })); }) + // Queryable metadata (file_metadata D1 table) — registered before the raw + // `/:key{.+}` GET route below. Verified by test (see + // "an object whose key literally ends in '/metadata'" in + // test/routes-files.test.ts): with this registration order, Hono's router + // resolves GET `...//metadata` to *this* route even when `` + // happens to be a real object whose own key ends in the literal + // `/metadata` segment (e.g. `screenshots/shot.png/metadata`) — the raw + // `/:key{.+}` GET route never sees that request, so that object's own + // metadata/contentType/url are not fetchable via GET, only its sibling + // `.../metadata` reads the *other* object's (`screenshots/shot.png`) + // metadata map instead. PUT/DELETE are unaffected (no PUT/DELETE metadata + // route exists to shadow them), so such an object can still be uploaded + // and deleted via the raw route — only its GET is shadowed. Accepted + // tradeoff: metadata is a first-class sibling resource, and keys ending in + // the literal `/metadata` suffix are not a realistic upload pattern. + .get("/:key{.+}/metadata", requireScope("files:read"), async (c) => { + const key = c.req.param("key"); + if (badKey(key)) throw new ValidationError("invalid key", { code: "invalid_key" }); + const ws = c.get("workspace"); + const store = await storage(c.env, ws); + if (!(await store.exists(key))) throw new NotFoundError(); + const metadata = await getFileMetadata(c.env.DB, c.get("workspaceName"), key); + return c.json({ metadata }); + }) + + .patch("/:key{.+}/metadata", writeRateLimit, requireScope("files:write"), async (c) => { + const key = c.req.param("key"); + if (badKey(key)) throw new ValidationError("invalid key", { code: "invalid_key" }); + const ws = c.get("workspace"); + const store = await storage(c.env, ws); + if (!(await store.exists(key))) throw new NotFoundError(); + + const body = await c.req.json().catch(() => null); + if (body === null || typeof body !== "object" || Array.isArray(body)) { + throw new ValidationError("invalid request body", { code: "invalid_body" }); + } + const { set, delete: remove } = body as { + set?: unknown; + delete?: unknown; + }; + if (set !== undefined && (typeof set !== "object" || set === null || Array.isArray(set))) { + throw new ValidationError("`set` must be an object of string values", { + code: "invalid_body", + }); + } + if (set !== undefined) { + for (const value of Object.values(set as Record)) { + if (typeof value !== "string") { + throw new ValidationError("`set` values must be strings", { code: "invalid_body" }); + } + } + } + if ( + remove !== undefined && + (!Array.isArray(remove) || remove.some((item) => typeof item !== "string")) + ) { + throw new ValidationError("`delete` must be an array of strings", { + code: "invalid_body", + }); + } + + const metadata = await setFileMetadata( + c.env.DB, + c.get("workspaceName"), + key, + (set as Record | undefined) ?? {}, + (remove as string[] | undefined) ?? [], + ); + return c.json({ metadata }); + }) + .get("/:key{.+}", requireScope("files:read"), async (c) => { const key = c.req.param("key"); if (badKey(key)) throw new ValidationError("invalid key", { code: "invalid_key" }); diff --git a/apps/api/test/routes-files.test.ts b/apps/api/test/routes-files.test.ts index ff948bc09a048854e453b6c044ffbd351b0600e6..19331dd34aba89fe8f8859d201a4803e5e5347fe 100644 GIT binary patch delta 5519 zcmcgwTW=f36^4((ziANSZfYY{irvSGMF!<5+U6z(5^G9C3%@TGTGH zyA-V|3iDbNXi!X_i$3(BK!L`Afj$-a1AS=U3j}R{KvDM>^gA=V6eUu&@=`$%sO8L= zGvE2nch1?reEz#XA9()sU^ACW(R?j*JXcgHD0ww$lcZ-g55e}6t)8B8-Nl)nOKY&fc4luR*hg*ZdSRz&^~J%V1EhX) z@J|Qm#m7I$9Z=sMu`W7xni`(J8r0xRS|9B$RdBSGq>&MQV{I)-o9e+^eTN>WiN96J z!xz;2TgRGCnG*bfu3fvfy*kTR&^N$7_}>2G%dqad9gm3|yFixSm~m_?y=xn7yNPAC zEQ@+IwA8of4mY>0>)qP@LN{kRZC1sytQN?vSF4?0#)!8d&6+2>R`U5JUy4xLOCp4G zj!HK`GlQm!*%?NWnW8!^wt&6jp?bKa#3)MU*yZ zasGq3`y`gVb~ zzNqYCyeIVaF7m%r@KI z!gD&4c6dtEh(v**P) zwPLBmeJdWT*}}}?%{xeYb$u!|AKf}l)=dxU?}nMhjheKu3{X~`l3*#%JqppD>^eWYdP`%=VSaQq2cr#wN!>k_ZyQ zJvEeis~S2tFuMa={rJ`Bb~C(J0GIa}v2Hd=yBx*OKxa}E1XzQ#gNUp@QOFpnfxbOp zS=a*>u(MVJL=CwdLxl5;@Vyr66dN@j73i_0;kFt$%&Q|dW7-+3qrcKxvG2JKvUEx8&5@8*+G4yW8+qymS7g5vaeUsdi`Fs~p zckH?+gMharF6p&<@v*s2KE~9&{Or@;E&?8 z{#4_PMrM4Hc^q)uI9!g=r#PC+brWD#2E?vbM+2?TB|bMGWP(rZO}1;?hCa+slXLN6 zlB?gY_@O4((2GWx?VI?`|8FciSbzEsVU7_LDREqTScm^QILS3;_7yVu3OR363FMx- zdga{(ZYy>WErDaY>$=G~R;4vj#T>VW^yBJF3DwhVK2aRdUstsdH~~ z51*wfeQ$eMKU$?YlcE-&P9uKGd`b?UKz{qXjy1P=BSqoVdV^P_q;L zj*bXnj627Qhw+O=uj=4+2S%n1lCBL^bZNAeB%?gC(Xyn4j~9t!?qIy)J__0yRD|s{ zIO#TYMeJ>n!}qx5OkYqxe|to?oFYAV-V6l`_ivlAVt3ibj+Y|?Jha}&2+GG{1Pw4c zxOS@3=h#y{3>yE*YI$t}4O4+!;deV(Z7&EN%!O6KmxP}2Rvu}KIqt+avllV+_)T~& zSwQrT_(E~v;Co5axT1UXFt4rEFv2AD(98BBY#E%;^8pRdc)kXWhgoxT_!U9U@Mq}; zU=j{-AiWWyj? zfCr{a!3rMqF(hbfv9Z`z(C;i)`K1{jTgRI*^2%hX=c8{ob++qmxShG@C_XfJpx1tI z_GM|-E7cit+&%P#ExmG?{`KXrO~FOf*mzWMV-UKe8el&*wERbu7h!GXsf8sPCY_xc=uGS zCVbfzlhyO7#9@qA#vJIYet)4~{dn@t;bJ^%4{vT8)8o?(nF#&5gs!NWv#Hmf|H<%! z8z9VL1*nSvFOl>+MLR+n&q^W2F=S{(tUGwG$yhT5Ku!HtQ@@nSs?U=t_4TDAy>)lh fMa-r-9@o{0Q}3vMkLZbSJnEKt&fCY7$JzTohKu6| delta 65 zcmZ3ohw*gHhAdW*w9>rflFa-(h1|sKR5u`YanhJ<#U{_lFgc2C8zaMJL3Vw{&8xVg U*f(1V88J>S5fR($Dypgn0Pa#0NB{r; From ee1c5f33891166ebc9c511e74a7690027c2e4db0 Mon Sep 17 00:00:00 2001 From: Zach Dunn Date: Mon, 13 Jul 2026 17:53:51 -0400 Subject: [PATCH 06/23] feat(api): filter file listings by metadata GET /v1/:workspace/files accepts repeatable meta.= params, ANDed, and switches the listing to the D1 file_metadata index instead of the R2 prefix-list when at least one is present. Combines with prefix and limit. Invalid keys or a repeated same-key param are rejected with a ValidationError. No meta.* params leaves the existing R2 path unchanged. --- apps/api/src/routes/files.ts | 54 ++++++++- apps/api/test/routes-files.test.ts | 179 +++++++++++++++++++++++++++++ 2 files changed, 231 insertions(+), 2 deletions(-) diff --git a/apps/api/src/routes/files.ts b/apps/api/src/routes/files.ts index af77a2e..fc5e1d7 100644 --- a/apps/api/src/routes/files.ts +++ b/apps/api/src/routes/files.ts @@ -8,7 +8,12 @@ import { listObjects, putObject, } from "../files-core"; -import { getFileMetadata, setFileMetadata } from "../file-metadata"; +import { + META_KEY_RE, + findObjectsByMetadata, + getFileMetadata, + setFileMetadata, +} from "../file-metadata"; import { splitUploadMetaHeaders } from "../provenance"; import { publicUrl, storage, storageConfig } from "../storage"; import { requireScope, type WorkspaceVars } from "../workspace"; @@ -118,8 +123,53 @@ export const files = new Hono() return c.json({ workspace: c.get("workspaceName"), ...result }, 201); }) + // Repeatable `meta.=` params switch the listing to the D1 + // metadata index (ANDed equality across all given pairs) instead of the + // R2 prefix-list below; see file-metadata.ts's `findObjectsByMetadata`. + // No `meta.*` params at all leaves the existing R2 path untouched. .get("/", requireScope("files:read"), async (c) => { - const { prefix, cursor } = c.req.query(); + const query = c.req.query(); + const metaParamKeys = Object.keys(query).filter((k) => k.startsWith("meta.")); + + if (metaParamKeys.length > 0) { + const filters: Record = {}; + for (const param of metaParamKeys) { + const key = param.slice("meta.".length); + if (!META_KEY_RE.test(key)) { + throw new ValidationError(`invalid metadata key: ${key}`, { + code: "file_metadata_invalid_key", + details: { key }, + }); + } + const values = c.req.queries(param) ?? []; + if (values.length > 1) { + throw new ValidationError(`repeated metadata filter for key: ${key}`, { + code: "file_metadata_duplicate_filter", + details: { key }, + }); + } + filters[key] = values[0] ?? query[param]; + } + + const ws = c.get("workspace"); + const limitParam = c.req.query("limit"); + const limit = limitParam ? Number(limitParam) || undefined : undefined; + const matches = await findObjectsByMetadata(c.env.DB, c.get("workspaceName"), filters, { + prefix: query.prefix, + limit, + }); + const cfg = await storageConfig(c.env, ws); + return c.json({ + items: matches.map((match) => ({ + key: match.key, + url: publicUrl(cfg, match.key), + metadata: match.metadata, + })), + cursor: null, + }); + } + + const { prefix, cursor } = query; const limit = Number(c.req.query("limit") ?? 100) || 100; return c.json(await listObjects(c.env, c.get("workspace"), { prefix, limit, cursor })); }) diff --git a/apps/api/test/routes-files.test.ts b/apps/api/test/routes-files.test.ts index 19331dd..86c617d 100644 --- a/apps/api/test/routes-files.test.ts +++ b/apps/api/test/routes-files.test.ts @@ -101,6 +101,55 @@ function makeFakeDB(authToken?: FakeAuthToken) { ); return { success: true, results: results as T[], meta: {} }; } + // findObjectsByMetadata's match query: ANDed equality filters + // (parsed by counting the repeated `(meta_key = ? AND meta_value = ?)` + // clause), an optional prefix LIKE, and a trailing HAVING-count + + // LIMIT. Mirrors the real SQL semantics against the in-memory store + // so route-level filter tests exercise real wiring, not a stub. + if (normalized.startsWith("SELECT object_key FROM file_metadata WHERE workspace")) { + const filterCount = (normalized.match(/meta_key = \? AND meta_value = \?/g) ?? []) + .length; + const hasPrefix = normalized.includes("object_key LIKE ? || '%'"); + let idx = 0; + const workspace = args[idx++] as string; + const filters: Array<[string, string]> = []; + for (let i = 0; i < filterCount; i++) { + filters.push([args[idx] as string, args[idx + 1] as string]); + idx += 2; + } + const prefix = hasPrefix ? (args[idx++] as string) : undefined; + const requiredCount = args[idx++] as number; + const limit = args[idx++] as number; + + const results: { object_key: string }[] = []; + const prefixMatch = `${workspace} `; + for (const [scopedKey, map] of metadata.entries()) { + if (!scopedKey.startsWith(prefixMatch)) continue; + const objectKey = scopedKey.slice(prefixMatch.length); + if (prefix && !objectKey.startsWith(prefix)) continue; + let matchCount = 0; + for (const [key, value] of filters) { + if (map.get(key) === value) matchCount++; + } + if (matchCount === requiredCount) results.push({ object_key: objectKey }); + } + results.sort((a, b) => + a.object_key < b.object_key ? -1 : a.object_key > b.object_key ? 1 : 0, + ); + return { success: true, results: results.slice(0, limit) as T[], meta: {} }; + } + if (normalized.startsWith("SELECT object_key, meta_key, meta_value FROM file_metadata")) { + const [workspace, ...keys] = args as [string, ...string[]]; + const results: { object_key: string; meta_key: string; meta_value: string }[] = []; + for (const key of keys) { + const map = metadata.get(scopeKey(workspace, key)); + if (!map) continue; + for (const [meta_key, meta_value] of map.entries()) { + results.push({ object_key: key, meta_key, meta_value }); + } + } + return { success: true, results: results as T[], meta: {} }; + } return { success: true, results: [] as T[], meta: {} }; }, }; @@ -612,3 +661,133 @@ describe("GET/PATCH /v1/:workspace/files/:key/metadata", () => { expect(json).toEqual({ metadata: {} }); }); }); + +function listFiles(env: Parameters[2], qs: string) { + return app.request( + `/v1/default/files${qs}`, + { headers: { Authorization: `Bearer ${TOKEN}` } }, + env, + ); +} + +interface ListedFile { + key: string; + url: string; + metadata: Record; +} + +describe("GET /v1/:workspace/files list + meta.* filter", () => { + it("no meta.* params: existing R2 prefix-list path is unchanged", async () => { + const { env } = await makeEnv(); + await putShot(env); + + const res = await listFiles(env, ""); + expect(res.status).toBe(200); + const json = (await res.json()) as { + items: Array<{ key: string; url: string; size: number; contentType: string }>; + cursor: string | null; + }; + expect(json.cursor).toBeNull(); + expect(json.items).toHaveLength(1); + expect(json.items[0]).toMatchObject({ + key: "screenshots/shot.png", + url: "https://storage.uploads.sh/default/screenshots/shot.png", + size: PNG.byteLength, + contentType: "image/png", + }); + }); + + it("filters by a single meta.* param via D1", async () => { + const { env } = await makeEnv(); + await putShot(env, { headers: { "X-Uploads-Meta-Device": "mobile" } }); + + const res = await listFiles(env, "?meta.device=mobile"); + expect(res.status).toBe(200); + const json = (await res.json()) as { items: ListedFile[]; cursor: string | null }; + expect(json.cursor).toBeNull(); + expect(json.items).toEqual([ + { + key: "screenshots/shot.png", + url: "https://storage.uploads.sh/default/screenshots/shot.png", + metadata: { device: "mobile" }, + }, + ]); + }); + + it("ANDs two meta.* filters, excluding an object that matches only one", async () => { + const { env } = await makeEnv(); + await putShot(env, { + headers: { "X-Uploads-Meta-Device": "mobile", "X-Uploads-Meta-App": "screenshots" }, + }); + // Second object matches `device` but not `app`. + await app.request( + "/v1/default/files/other/shot2.png", + { + method: "PUT", + headers: { + Authorization: `Bearer ${TOKEN}`, + "Content-Type": "image/png", + "X-Uploads-Meta-Device": "mobile", + "X-Uploads-Meta-App": "web", + }, + body: PNG, + }, + env, + ); + + const res = await listFiles(env, "?meta.device=mobile&meta.app=screenshots"); + expect(res.status).toBe(200); + const json = (await res.json()) as { items: ListedFile[] }; + expect(json.items).toHaveLength(1); + expect(json.items[0].key).toBe("screenshots/shot.png"); + }); + + it("combines a meta.* filter with a prefix", async () => { + const { env } = await makeEnv(); + await putShot(env, { headers: { "X-Uploads-Meta-App": "screenshots" } }); + await app.request( + "/v1/default/files/other/shot2.png", + { + method: "PUT", + headers: { + Authorization: `Bearer ${TOKEN}`, + "Content-Type": "image/png", + "X-Uploads-Meta-App": "screenshots", + }, + body: PNG, + }, + env, + ); + + const res = await listFiles(env, "?meta.app=screenshots&prefix=screenshots/"); + expect(res.status).toBe(200); + const json = (await res.json()) as { items: ListedFile[] }; + expect(json.items).toEqual([ + { + key: "screenshots/shot.png", + url: "https://storage.uploads.sh/default/screenshots/shot.png", + metadata: { app: "screenshots" }, + }, + ]); + }); + + it("rejects an invalid meta.* key with a validation error", async () => { + const { env } = await makeEnv(); + await putShot(env); + + const res = await listFiles(env, "?meta.1bad=x"); + expect(res.status).toBe(400); + const json = (await res.json()) as { error: { type: string } }; + expect(json.error.type).toBe("validation"); + }); + + it("rejects a repeated same-key meta.* param", async () => { + const { env } = await makeEnv(); + await putShot(env); + + const res = await listFiles(env, "?meta.device=mobile&meta.device=desktop"); + expect(res.status).toBe(400); + const json = (await res.json()) as { error: { type: string } }; + expect(json.error.type).toBe("validation"); + }); +}); From f23e58c071e0810bd3462a285af1c97f7aa4a370 Mon Sep 17 00:00:00 2001 From: Zach Dunn Date: Mon, 13 Jul 2026 17:57:16 -0400 Subject: [PATCH 07/23] docs(api): note visibility caveat on metadata-filtered listings --- apps/api/src/routes/files.ts | 3 +++ 1 file changed, 3 insertions(+) diff --git a/apps/api/src/routes/files.ts b/apps/api/src/routes/files.ts index fc5e1d7..d545286 100644 --- a/apps/api/src/routes/files.ts +++ b/apps/api/src/routes/files.ts @@ -127,6 +127,9 @@ export const files = new Hono() // metadata index (ANDed equality across all given pairs) instead of the // R2 prefix-list below; see file-metadata.ts's `findObjectsByMetadata`. // No `meta.*` params at all leaves the existing R2 path untouched. + // Contract caveat: D1-path items carry no `visibility` annotation (that + // lives in R2 custom metadata and would cost a HEAD per result to + // hydrate); callers needing the private marker must HEAD the object. .get("/", requireScope("files:read"), async (c) => { const query = c.req.query(); const metaParamKeys = Object.keys(query).filter((k) => k.startsWith("meta.")); From dc1236675e8bfef2f3c28dbc4920c14ff17bc264 Mon Sep 17 00:00:00 2001 From: Zach Dunn Date: Mon, 13 Jul 2026 18:02:50 -0400 Subject: [PATCH 08/23] feat(api): expose file metadata on public file endpoint GET /public/files/:workspace/:key now returns the file's D1 file_metadata map (omitted when empty) and a derived github object built from gh.repo/ gh.kind/gh.number when all three parse validly. Malformed or incomplete gh.* pairs just fall back to raw metadata with no github object. The private-file 401 gate still runs before the metadata fetch, so nothing leaks on private objects. --- apps/api/src/routes/public-files.ts | 49 +++++- apps/api/test/routes-public-files.test.ts | 190 +++++++++++++++++++++- 2 files changed, 233 insertions(+), 6 deletions(-) diff --git a/apps/api/src/routes/public-files.ts b/apps/api/src/routes/public-files.ts index e7fbfb3..0fffa4f 100644 --- a/apps/api/src/routes/public-files.ts +++ b/apps/api/src/routes/public-files.ts @@ -1,10 +1,46 @@ import { NotFoundError, UnauthorizedError } from "@uploads/errors"; import { Hono } from "hono"; import { badKey } from "../files-core"; +import { getFileMetadata } from "../file-metadata"; import { publicUrl, storage, storageConfig } from "../storage"; import { objectVisibility } from "../visibility"; import { loadWorkspaceRecord, type WorkspaceVars } from "../workspace"; +type GithubKind = "pull" | "issue"; + +interface GithubContext { + repo: string; + kind: GithubKind; + number: number; + url: string; +} + +// Deliberately permissive (not the full GitHub owner/repo charset) — this only +// gates the derived convenience object; malformed input just falls back to +// leaving the raw `gh.*` pairs in `metadata` (see task-5 brief). +const REPO_RE = /^[A-Za-z0-9._-]+\/[A-Za-z0-9._-]+$/; +const POSITIVE_INT_RE = /^[1-9][0-9]*$/; + +/** + * Derives the `github` convenience object from `gh.repo`/`gh.kind`/`gh.number` + * when all three are present and valid. Any missing or malformed piece omits + * the object entirely — the raw pairs still flow through in `metadata`. + */ +function deriveGithubContext(metadata: Record): GithubContext | undefined { + const repo = metadata["gh.repo"]; + const kind = metadata["gh.kind"]; + const numberRaw = metadata["gh.number"]; + if (!repo || !kind || !numberRaw) return undefined; + if (kind !== "pull" && kind !== "issue") return undefined; + if (!REPO_RE.test(repo)) return undefined; + if (!POSITIVE_INT_RE.test(numberRaw)) return undefined; + const number = Number(numberRaw); + if (!Number.isSafeInteger(number)) return undefined; + + const path = kind === "pull" ? "pull" : "issues"; + return { repo, kind, number, url: `https://github.com/${repo}/${path}/${number}` }; +} + /** * Public, unauthenticated metadata for a single object, so `apps/web` (which has * no storage bindings) can render a chrome-wrapped file page at @@ -13,7 +49,11 @@ import { loadWorkspaceRecord, type WorkspaceVars } from "../workspace"; * Security posture mirrors public galleries: exact-key lookup only, no listing, * no bearer token in the browser. It adds no new *access* — the bytes are already * served unsigned off the R2 public domain — only a curated metadata view. Raw - * provenance is deliberately omitted here (unlike the authenticated HEAD). + * provenance (R2 custom metadata: client, content-sha256, …) is deliberately + * omitted here (unlike the authenticated HEAD). Queryable `file_metadata` (D1) + * is a separate, intentionally-public tier — see `file-metadata.ts` — and is + * included below (with a `github` object derived from any `gh.*` pairs), since + * it follows the object's own visibility rather than authenticated-only access. * * `visibility: "private"` (#139) gates this JSON endpoint with 401 `auth_required`. * NOTE this is metadata-only enforcement: on a workspace with `publicBaseUrl`, the @@ -46,6 +86,11 @@ export const publicFiles = new Hono().get("/:workspace/:key{.+}", throw new UnauthorizedError("sign in to view this file", { code: "auth_required" }); } + // Fetched only after the visibility gate above — a private object 401s + // before this D1 read ever happens, so metadata never leaks. + const metadata = await getFileMetadata(c.env.DB, workspace, key); + const github = deriveGithubContext(metadata); + return c.json({ workspace, key, @@ -53,5 +98,7 @@ export const publicFiles = new Hono().get("/:workspace/:key{.+}", size: meta.size ?? 0, contentType: meta.type ?? "application/octet-stream", ...(meta.lastModified != null ? { uploaded: new Date(meta.lastModified).toISOString() } : {}), + ...(Object.keys(metadata).length > 0 ? { metadata } : {}), + ...(github ? { github } : {}), }); }); diff --git a/apps/api/test/routes-public-files.test.ts b/apps/api/test/routes-public-files.test.ts index ec15605..7306e55 100644 --- a/apps/api/test/routes-public-files.test.ts +++ b/apps/api/test/routes-public-files.test.ts @@ -11,6 +11,67 @@ import { sha256Hex, type WorkspaceRecord } from "../src/workspace"; const TOKEN = "secret-token"; +interface MetaRow { + meta_key: string; + meta_value: string; +} + +/** + * Fake D1 backing `file_metadata` with a real in-memory store (mirrors + * routes-files.test.ts's makeFakeDB), so this suite can assert on the + * `metadata`/`github` DTO fields the public endpoint derives from real rows + * rather than a stubbed-empty read. + */ +function makeFakeDB() { + const metadata = new Map>(); + const scopeKey = (workspace: string, objectKey: string) => `${workspace} ${objectKey}`; + + return { + prepare(sql: string) { + const normalized = sql.replace(/\s+/g, " ").trim(); + let args: unknown[] = []; + return { + bind(...values: unknown[]) { + args = values; + return this; + }, + async first() { + return null; + }, + async run() { + if (normalized.startsWith("INSERT INTO file_metadata")) { + const [workspace, objectKey, key, value] = args as [string, string, string, string]; + const map = metadata.get(scopeKey(workspace, objectKey)) ?? new Map(); + map.set(key, value); + metadata.set(scopeKey(workspace, objectKey), map); + } else if (normalized.includes("meta_key = ?")) { + const [workspace, objectKey, key] = args as [string, string, string]; + metadata.get(scopeKey(workspace, objectKey))?.delete(key); + } else if (normalized.startsWith("DELETE FROM file_metadata")) { + const [workspace, objectKey] = args as [string, string]; + metadata.delete(scopeKey(workspace, objectKey)); + } + return { success: true, meta: { changes: 0 }, results: [] }; + }, + async all() { + if (normalized.startsWith("SELECT meta_key, meta_value FROM file_metadata")) { + const [workspace, objectKey] = args as [string, string]; + const map = metadata.get(scopeKey(workspace, objectKey)) ?? new Map(); + const results = [...map.entries()].map( + ([meta_key, meta_value]) => ({ meta_key, meta_value }) as MetaRow, + ); + return { success: true, results: results as T[], meta: {} }; + } + return { success: true, results: [] as T[], meta: {} }; + }, + }; + }, + async batch(stmts: { run: () => Promise }[]) { + return Promise.all(stmts.map((s) => s.run())); + }, + }; +} + beforeAll(() => { if (!(crypto.subtle as SubtleCrypto & { timingSafeEqual?: unknown }).timingSafeEqual) { Object.defineProperty(crypto.subtle, "timingSafeEqual", { @@ -28,7 +89,10 @@ beforeAll(() => { const PNG = new Uint8Array([0x89, 0x50, 0x4e, 0x47, 0x0d, 0x0a, 0x1a, 0x0a, 1, 2, 3, 4]); -async function makeEnv(overrides: Partial = {}) { +async function makeEnv( + overrides: Partial = {}, + opts: { db?: ReturnType } = {}, +) { const record: WorkspaceRecord = { provider: "r2", bucket: "uploads-default", @@ -41,10 +105,10 @@ async function makeEnv(overrides: Partial = {}) { const bucket = new FakeR2Bucket(); const env = { REGISTRY: { get: async () => record, put: async () => undefined }, - // No-op D1: this suite doesn't assert on file_metadata rows, just that - // putObject's D1 metadata write (`.all()` read-before-write inside - // setFileMetadata) doesn't blow up. - DB: { + // Defaults to a no-op D1 (existing tests don't assert on file_metadata rows, + // just that putObject's D1 write doesn't blow up); pass `opts.db` to back + // `file_metadata` with a real in-memory store for the metadata/github tests. + DB: opts.db ?? { prepare: () => ({ bind() { return this; @@ -173,4 +237,120 @@ describe("GET /public/files/:workspace/:key", () => { const res = await app.request("/public/files/default/screenshots/shot.png", {}, env); expect(res.status).toBe(200); }); + + it("includes file_metadata and a derived github object when gh.* is valid", async () => { + const { env } = await makeEnv({}, { db: makeFakeDB() }); + await seedShot(env, { + "X-Uploads-Meta-gh.repo": "buildinternet/uploads", + "X-Uploads-Meta-gh.kind": "pull", + "X-Uploads-Meta-gh.number": "142", + "X-Uploads-Meta-app": "uploads-cli", + }); + + const res = await app.request("/public/files/default/screenshots/shot.png", {}, env); + expect(res.status).toBe(200); + const json = (await res.json()) as { + metadata?: Record; + github?: { repo: string; kind: string; number: number; url: string }; + }; + expect(json.metadata).toEqual({ + "gh.repo": "buildinternet/uploads", + "gh.kind": "pull", + "gh.number": "142", + app: "uploads-cli", + }); + expect(json.github).toEqual({ + repo: "buildinternet/uploads", + kind: "pull", + number: 142, + url: "https://github.com/buildinternet/uploads/pull/142", + }); + }); + + it("derives an issues URL for gh.kind = issue", async () => { + const { env } = await makeEnv({}, { db: makeFakeDB() }); + await seedShot(env, { + "X-Uploads-Meta-gh.repo": "buildinternet/uploads", + "X-Uploads-Meta-gh.kind": "issue", + "X-Uploads-Meta-gh.number": "7", + }); + + const res = await app.request("/public/files/default/screenshots/shot.png", {}, env); + const json = (await res.json()) as { github?: { url: string } }; + expect(json.github?.url).toBe("https://github.com/buildinternet/uploads/issues/7"); + }); + + it("omits both fields when the file has no metadata", async () => { + const { env } = await makeEnv({}, { db: makeFakeDB() }); + await seedShot(env); + + const res = await app.request("/public/files/default/screenshots/shot.png", {}, env); + const json = (await res.json()) as Record; + expect(json).not.toHaveProperty("metadata"); + expect(json).not.toHaveProperty("github"); + }); + + it("omits github but keeps raw pairs when gh.* is malformed (non-numeric number)", async () => { + const { env } = await makeEnv({}, { db: makeFakeDB() }); + await seedShot(env, { + "X-Uploads-Meta-gh.repo": "buildinternet/uploads", + "X-Uploads-Meta-gh.kind": "pull", + "X-Uploads-Meta-gh.number": "not-a-number", + }); + + const res = await app.request("/public/files/default/screenshots/shot.png", {}, env); + const json = (await res.json()) as { + metadata?: Record; + github?: unknown; + }; + expect(json.metadata).toEqual({ + "gh.repo": "buildinternet/uploads", + "gh.kind": "pull", + "gh.number": "not-a-number", + }); + expect(json.github).toBeUndefined(); + }); + + it("omits github when gh.kind is neither pull nor issue", async () => { + const { env } = await makeEnv({}, { db: makeFakeDB() }); + await seedShot(env, { + "X-Uploads-Meta-gh.repo": "buildinternet/uploads", + "X-Uploads-Meta-gh.kind": "discussion", + "X-Uploads-Meta-gh.number": "5", + }); + + const res = await app.request("/public/files/default/screenshots/shot.png", {}, env); + const json = (await res.json()) as { metadata?: Record; github?: unknown }; + expect(json.metadata).toBeDefined(); + expect(json.github).toBeUndefined(); + }); + + it("omits github when a gh.* key is missing entirely", async () => { + const { env } = await makeEnv({}, { db: makeFakeDB() }); + await seedShot(env, { + "X-Uploads-Meta-gh.repo": "buildinternet/uploads", + "X-Uploads-Meta-gh.number": "5", + }); + + const res = await app.request("/public/files/default/screenshots/shot.png", {}, env); + const json = (await res.json()) as { metadata?: Record; github?: unknown }; + expect(json.metadata).toEqual({ "gh.repo": "buildinternet/uploads", "gh.number": "5" }); + expect(json.github).toBeUndefined(); + }); + + it("401s with auth_required for a private file and never fetches/leaks metadata", async () => { + const { env } = await makeEnv({}, { db: makeFakeDB() }); + await seedShot(env, { + "X-Uploads-Visibility": "private", + "X-Uploads-Meta-gh.repo": "buildinternet/uploads", + "X-Uploads-Meta-gh.kind": "pull", + "X-Uploads-Meta-gh.number": "1", + }); + + const res = await app.request("/public/files/default/screenshots/shot.png", {}, env); + expect(res.status).toBe(401); + const json = (await res.json()) as Record; + expect(json).not.toHaveProperty("metadata"); + expect(json).not.toHaveProperty("github"); + }); }); From d098c81baa88b3f1ff4e59364cb3ec14f2e2c2f9 Mon Sep 17 00:00:00 2001 From: Zach Dunn Date: Mon, 13 Jul 2026 18:11:02 -0400 Subject: [PATCH 09/23] fix(api): reserve server provenance keys from custom metadata MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A client could store a D1 file_metadata row named content-sha256 (via upload headers or the metadata PATCH), shadowing the server-computed integrity hash in R2 provenance. Server-set provenance keys are now a named constant (PROVENANCE_SERVER_KEYS) and validateMetadataEntries — the shared choke point for upload capture, PATCH, and future callers — rejects them with file_metadata_reserved_key. gh.* keys stay writable (system-managed by convention per the design doc). --- apps/api/src/file-metadata.ts | 16 +++++++++++ apps/api/src/provenance.ts | 10 ++++++- apps/api/test/file-metadata.test.ts | 12 ++++++++ apps/api/test/routes-files.test.ts | 35 +++++++++++++++++++++-- apps/api/test/routes-public-files.test.ts | 7 ++--- packages/errors/src/codes.ts | 1 + 6 files changed, 74 insertions(+), 7 deletions(-) diff --git a/apps/api/src/file-metadata.ts b/apps/api/src/file-metadata.ts index ab7c423..d87ae12 100644 --- a/apps/api/src/file-metadata.ts +++ b/apps/api/src/file-metadata.ts @@ -9,10 +9,20 @@ */ import { ValidationError } from "@uploads/errors"; +import { PROVENANCE_SERVER_KEYS } from "./provenance"; /** Lowercase key, optionally namespaced with dots (e.g. `gh.repo`). */ export const META_KEY_RE = /^[a-z][a-z0-9._-]{0,63}$/; +/** + * Server-set provenance keys (e.g. `content-sha256`) are reserved: a custom + * metadata row with the same name would be a spoofable shadow of a value the + * server computes and vouches for. Enforced here — the single choke point for + * upload capture, the PATCH endpoint, and any future setFileMetadata caller. + * `gh.*` keys are NOT reserved: system-managed by convention only (design doc). + */ +const RESERVED_META_KEYS = new Set(PROVENANCE_SERVER_KEYS); + /** Cap applied both to a single write request and to a file's total keys post-merge. */ export const META_MAX_KEYS = 24; @@ -50,6 +60,12 @@ export function validateMetadataEntries(meta: Record): void { details: { key }, }); } + if (RESERVED_META_KEYS.has(key)) { + throw new ValidationError(`reserved metadata key: ${key}`, { + code: "file_metadata_reserved_key", + details: { key }, + }); + } const value = meta[key]; if ( typeof value !== "string" || diff --git a/apps/api/src/provenance.ts b/apps/api/src/provenance.ts index 74fe7f9..de48a76 100644 --- a/apps/api/src/provenance.ts +++ b/apps/api/src/provenance.ts @@ -37,7 +37,15 @@ export const PROVENANCE_CLIENT_KEYS = [ "keep-exif", ] as const; -export const PROVENANCE_KEYS = [...PROVENANCE_CLIENT_KEYS, "content-sha256"] as const; +/** + * Server-computed provenance keys — never accepted from headers, and reserved + * from the D1 custom-metadata namespace too (see file-metadata.ts's + * `validateMetadataEntries`) so a client can't store a spoofable shadow of + * the real integrity hash under the same name. + */ +export const PROVENANCE_SERVER_KEYS = ["content-sha256"] as const; + +export const PROVENANCE_KEYS = [...PROVENANCE_CLIENT_KEYS, ...PROVENANCE_SERVER_KEYS] as const; export type ProvenanceKey = (typeof PROVENANCE_KEYS)[number]; export type ProvenanceClientKey = (typeof PROVENANCE_CLIENT_KEYS)[number]; diff --git a/apps/api/test/file-metadata.test.ts b/apps/api/test/file-metadata.test.ts index 4ad4648..de92205 100644 --- a/apps/api/test/file-metadata.test.ts +++ b/apps/api/test/file-metadata.test.ts @@ -40,6 +40,18 @@ describe("validateMetadataEntries", () => { expect(() => validateMetadataEntries({ "Bad Key": "x" })).toThrow(AppError); }); + it("throws a reserved-key AppError for server-set provenance keys like content-sha256", () => { + try { + validateMetadataEntries({ "content-sha256": "0".repeat(64) }); + throw new Error("expected validateMetadataEntries to throw"); + } catch (err) { + expect(err).toBeInstanceOf(AppError); + expect((err as AppError & { code?: string }).code).toBe("file_metadata_reserved_key"); + } + // gh.* keys stay writable — system-managed by convention, not reserved. + expect(() => validateMetadataEntries({ "gh.repo": "a/b" })).not.toThrow(); + }); + it("throws AppError for an empty value", () => { expect(() => validateMetadataEntries({ app: "" })).toThrow(AppError); }); diff --git a/apps/api/test/routes-files.test.ts b/apps/api/test/routes-files.test.ts index 86c617d..a839329 100644 --- a/apps/api/test/routes-files.test.ts +++ b/apps/api/test/routes-files.test.ts @@ -299,8 +299,8 @@ describe("PUT /v1/:workspace/files upload guardrails", () => { "X-Uploads-Meta-Client-Version": "0.3.0", "X-Uploads-Meta-Optimized": "1", "X-Uploads-Meta-Frame": "phone", - "X-Uploads-Meta-Secret": "should-drop", - "X-Uploads-Meta-Content-Sha256": "0".repeat(64), + // Non-allowlisted: lands in D1 custom metadata, never in R2 provenance. + "X-Uploads-Meta-Secret": "custom-not-provenance", }, }); expect(res.status).toBe(201); @@ -403,6 +403,21 @@ describe("PUT /v1/:workspace/files custom metadata capture + cascade", () => { expect(bucket.store.has("default/screenshots/shot.png")).toBe(false); }); + it("rejects an upload spoofing the server-set content-sha256 as custom metadata", async () => { + const { env, db, bucket } = await makeEnv(); + const res = await putShot(env, { + headers: { "X-Uploads-Meta-Content-Sha256": "0".repeat(64) }, + }); + expect(res.status).toBe(400); + const json = (await res.json()) as { error: { type: string; code: string } }; + expect(json.error.type).toBe("validation"); + expect(json.error.code).toBe("file_metadata_reserved_key"); + expect(bucket.store.has("default/screenshots/shot.png")).toBe(false); + await expect( + getFileMetadata(db as unknown as D1Database, "default", "screenshots/shot.png"), + ).resolves.toEqual({}); + }); + it("rejects an empty custom metadata value instead of silently dropping it", async () => { const { env, bucket } = await makeEnv(); const res = await putShot(env, { headers: { "X-Uploads-Meta-App": "" } }); @@ -532,6 +547,22 @@ describe("GET/PATCH /v1/:workspace/files/:key/metadata", () => { expect(await res.json()).toEqual({ metadata: { gh_pr: "142" } }); }); + it("PATCH rejects setting the reserved content-sha256 key with 400", async () => { + const { env } = await makeEnv(); + await putShot(env); + + const patch = await patchMeta(env, "screenshots/shot.png", { + set: { "content-sha256": "0".repeat(64) }, + }); + expect(patch.status).toBe(400); + const json = (await patch.json()) as { error: { type: string; code: string } }; + expect(json.error.type).toBe("validation"); + expect(json.error.code).toBe("file_metadata_reserved_key"); + + const res = await getMeta(env, "screenshots/shot.png"); + expect(await res.json()).toEqual({ metadata: {} }); + }); + it("PATCH delete removes a key", async () => { const { env } = await makeEnv(); await putShot(env); diff --git a/apps/api/test/routes-public-files.test.ts b/apps/api/test/routes-public-files.test.ts index 7306e55..31a2d13 100644 --- a/apps/api/test/routes-public-files.test.ts +++ b/apps/api/test/routes-public-files.test.ts @@ -174,10 +174,9 @@ describe("GET /public/files/:workspace/:key", () => { it("never surfaces provenance metadata on the public surface", async () => { const { env } = await makeEnv(); - await seedShot(env, { - "X-Uploads-Meta-Client": "uploads-cli", - "X-Uploads-Meta-Content-Sha256": "0".repeat(64), - }); + // The server always writes content-sha256 provenance itself; a client + // spoof attempt now rejects the upload outright (file_metadata_reserved_key). + await seedShot(env, { "X-Uploads-Meta-Client": "uploads-cli" }); const res = await app.request("/public/files/default/screenshots/shot.png", {}, env); expect(res.status).toBe(200); diff --git a/packages/errors/src/codes.ts b/packages/errors/src/codes.ts index b95de81..5a75184 100644 --- a/packages/errors/src/codes.ts +++ b/packages/errors/src/codes.ts @@ -60,6 +60,7 @@ export const ERROR_CODES = [ "file_metadata_invalid_key", "file_metadata_invalid_value", "file_metadata_limit_exceeded", + "file_metadata_reserved_key", // Admin "invalid_workspace", From 7e00a84841b2d6389d0b2a13ee395d594bbf7e5c Mon Sep 17 00:00:00 2001 From: Zach Dunn Date: Mon, 13 Jul 2026 18:16:04 -0400 Subject: [PATCH 10/23] feat(web): attachment context + metadata on file pages Task 6: render the "Attached to" GitHub PR/issue row and a generic Metadata section on the /f/ file page, consuming Task 5's github/metadata fields from GET /public/files/:workspace/:key. --- apps/web/src/lib/public-file.test.ts | 33 ++++++++++++++ apps/web/src/lib/public-file.ts | 45 ++++++++++++++++++- .../src/pages/f/[workspace]/[...key].astro | 32 +++++++++++++ 3 files changed, 109 insertions(+), 1 deletion(-) diff --git a/apps/web/src/lib/public-file.test.ts b/apps/web/src/lib/public-file.test.ts index 7ebb2f9..22ba916 100644 --- a/apps/web/src/lib/public-file.test.ts +++ b/apps/web/src/lib/public-file.test.ts @@ -66,6 +66,39 @@ describe("isPublicFile", () => { expect(isPublicFile({ ...file, contentType: "x".repeat(129) })).toBe(false); expect(isPublicFile(null)).toBe(false); }); + + it("accepts metadata + github when both are well-formed", () => { + const github = { + repo: "buildinternet/uploads", + kind: "pull", + number: 142, + url: "https://github.com/buildinternet/uploads/pull/142", + } as const; + const metadata = { "gh.repo": "buildinternet/uploads", "gh.kind": "pull", "gh.number": "142" }; + expect(isPublicFile({ ...file, metadata, github })).toBe(true); + }); + + it("rejects malformed metadata maps", () => { + expect(isPublicFile({ ...file, metadata: {} })).toBe(false); + expect(isPublicFile({ ...file, metadata: { "Bad Key": "x" } })).toBe(false); + expect(isPublicFile({ ...file, metadata: { ok: "x".repeat(513) } })).toBe(false); + const tooManyKeys = Object.fromEntries(Array.from({ length: 25 }, (_, i) => [`k${i}`, "v"])); + expect(isPublicFile({ ...file, metadata: tooManyKeys })).toBe(false); + }); + + it("rejects malformed github contexts", () => { + const base = { + repo: "buildinternet/uploads", + kind: "pull", + number: 142, + url: "https://github.com/buildinternet/uploads/pull/142", + }; + expect(isPublicFile({ ...file, github: { ...base, kind: "commit" } })).toBe(false); + expect(isPublicFile({ ...file, github: { ...base, number: 0 } })).toBe(false); + expect( + isPublicFile({ ...file, github: { ...base, url: "http://github.com/x/y/pull/1" } }), + ).toBe(false); + }); }); describe("key safety + path building", () => { diff --git a/apps/web/src/lib/public-file.ts b/apps/web/src/lib/public-file.ts index c5cad3a..01b6549 100644 --- a/apps/web/src/lib/public-file.ts +++ b/apps/web/src/lib/public-file.ts @@ -5,6 +5,14 @@ import { CF_RUM_CONNECT_SRC, CF_RUM_SCRIPT_SRC, STYLE_SRC_SELF_AND_INLINE } from // no storage bindings, so the API endpoint is the single-object security // surface. This mirrors public-gallery.ts, minus collection/ordering concerns. +/** GitHub PR/issue an object is attached to, derived server-side from `gh.*` metadata. */ +export interface GithubContext { + repo: string; + kind: "pull" | "issue"; + number: number; + url: string; +} + /** Allowlisted metadata for one public object, as returned by the API. */ export interface PublicFile { workspace: string; @@ -13,6 +21,10 @@ export interface PublicFile { size: number; contentType: string; uploaded: string | null; + /** Queryable `gh.*`-and-other custom metadata pairs; omitted when there are none. */ + metadata?: Record; + /** Convenience view of `gh.repo`/`gh.kind`/`gh.number`, when all three are present and valid. */ + github?: GithubContext; } /** Result of resolving a public file: the DTO, a hard 404, a private gate, or a soft outage. */ @@ -149,6 +161,33 @@ function isAuthRequiredError(value: unknown): boolean { return error.code === "auth_required" && text(error.message, 512); } +/** Mirrors apps/api's `META_KEY_RE` (file-metadata.ts) — lowercase, optionally dotted. */ +const META_KEY_RE = /^[a-z][a-z0-9._-]{0,63}$/; +/** Mirrors apps/api's `META_MAX_KEYS`/`META_VALUE_MAX` caps for one file's metadata map. */ +const META_MAX_KEYS = 24; +const META_VALUE_MAX = 512; + +/** Bounded, non-empty `Record` of metadata pairs — never sent empty by the API. */ +function isMetadataMap(value: unknown): value is Record { + if (typeof value !== "object" || value === null) return false; + const entries = Object.entries(value as Record); + if (entries.length === 0 || entries.length > META_MAX_KEYS) return false; + return entries.every(([key, v]) => META_KEY_RE.test(key) && text(v, META_VALUE_MAX)); +} + +/** Bounded `github` convenience object — mirrors apps/api's `deriveGithubContext` shape. */ +function isGithubContext(value: unknown): value is GithubContext { + if (typeof value !== "object" || value === null) return false; + const github = value as Record; + return ( + text(github.repo, 200) && + (github.kind === "pull" || github.kind === "issue") && + Number.isSafeInteger(github.number) && + (github.number as number) > 0 && + httpsUrl(github.url) + ); +} + /** Validate an untrusted API response against the bounded {@link PublicFile} shape. */ export function isPublicFile(value: unknown): value is PublicFile { if (typeof value !== "object" || value === null) return false; @@ -157,6 +196,8 @@ export function isPublicFile(value: unknown): value is PublicFile { file.uploaded === undefined || file.uploaded === null || (text(file.uploaded, 64) && Number.isFinite(Date.parse(file.uploaded as string))); + const metadataOk = file.metadata === undefined || isMetadataMap(file.metadata); + const githubOk = file.github === undefined || isGithubContext(file.github); return ( text(file.workspace, 64) && text(file.key, 1024) && @@ -164,7 +205,9 @@ export function isPublicFile(value: unknown): value is PublicFile { Number.isSafeInteger(file.size) && (file.size as number) >= 0 && text(file.contentType, 128) && - uploadedOk + uploadedOk && + metadataOk && + githubOk ); } diff --git a/apps/web/src/pages/f/[workspace]/[...key].astro b/apps/web/src/pages/f/[workspace]/[...key].astro index b396954..4c0f5c9 100644 --- a/apps/web/src/pages/f/[workspace]/[...key].astro +++ b/apps/web/src/pages/f/[workspace]/[...key].astro @@ -40,6 +40,13 @@ const uploadedLabel = : null; const canonical = new URL(file ? filePath(workspace, key) : Astro.url.pathname, Astro.url.origin) .href; +// Generic metadata list excludes `gh.*` pairs — those render as the "Attached to" +// row above via `file.github` instead of duplicating them here. +const metadataEntries = file?.metadata + ? Object.entries(file.metadata) + .filter(([metaKey]) => !metaKey.startsWith("gh.")) + .sort(([a], [b]) => a.localeCompare(b)) + : []; const title = file ? `${filename} · uploads.sh` : result.status === "auth_required" @@ -77,6 +84,9 @@ const title = file dl.meta { display:grid; grid-template-columns:auto 1fr; gap:6px 16px; margin:14px 0 0; font:12px var(--mono); } dl.meta dt { color:var(--muted); text-transform:uppercase; letter-spacing:.08em; font-size:11px; } dl.meta dd { margin:0; color:var(--body); overflow-wrap:anywhere; } + .gh-kind { margin-left:8px; padding:1px 5px; border:1px solid var(--line); border-radius:4px; color:var(--muted); font-size:10px; text-transform:uppercase; letter-spacing:.06em; } + .section-label { margin:16px 0 0; color:var(--muted); text-transform:uppercase; letter-spacing:.08em; font:11px var(--mono); } + dl.meta.metadata { margin-top:6px; } .actions { display:flex; flex-wrap:wrap; gap:10px; align-items:center; margin-top:16px; } .actions a.original { display:inline-block; padding:9px 15px; border:1px solid var(--line); border-radius:var(--radius-md); background:var(--panel); color:var(--fg); font:12px var(--mono); text-decoration:none; } .actions a.original:hover, .actions a.original:focus-visible { border-color:var(--accent); color:var(--accent); outline:none; } @@ -112,7 +122,29 @@ const title = file
Type
{file.contentType}
Size
{formatBytes(file.size)}
{uploadedLabel && <>
Uploaded
{uploadedLabel}
} + {file.github && ( + <> +
Attached to
+
+ {file.github.repo}#{file.github.number} + {file.github.kind === "pull" ? "PR" : "Issue"} +
+ + )} + {metadataEntries.length > 0 && ( + <> +
Metadata
+
+ {metadataEntries.map(([metaKey, metaValue]) => ( + <> +
{metaKey}
+
{metaValue}
+ + ))} +
+ + )}
{kind(file.contentType) !== "unsupported" && Open original ↗} From f0edda97dc097ef216d018e7dd0f222df7b43717 Mon Sep 17 00:00:00 2001 From: Zach Dunn Date: Mon, 13 Jul 2026 18:37:04 -0400 Subject: [PATCH 11/23] feat(uploads): metadata flags, meta/find commands, attach gh context - put --meta k=v (repeatable, first-= split) validated client-side and sent as more X-Uploads-Meta- headers alongside provenance - attach writes gh.repo/gh.kind/gh.number/gh.ref automatically from its resolved target; --meta extras merge in, target pairs win on collision - new meta get/set commands and list --meta / find k=v filter alias - client gains getMetadata/patchMetadata/findFiles; cli-args flags now support repeatable string flags via flagValues --- .changeset/file-metadata.md | 8 ++ packages/uploads/README.md | 8 +- packages/uploads/src/cli-args.ts | 46 +++++- packages/uploads/src/cli.ts | 14 +- packages/uploads/src/client.ts | 74 ++++++++++ packages/uploads/src/commands.ts | 135 +++++++++++++++++- packages/uploads/src/github.ts | 17 +++ packages/uploads/src/index.ts | 14 ++ packages/uploads/src/metadata.ts | 76 ++++++++++ packages/uploads/test/cli-args.test.ts | 35 +++++ packages/uploads/test/client-metadata.test.ts | 134 +++++++++++++++++ packages/uploads/test/commands-attach.test.ts | 49 ++++++- packages/uploads/test/commands-find.test.ts | 71 +++++++++ packages/uploads/test/commands-list.test.ts | 55 +++++++ packages/uploads/test/commands-meta.test.ts | 129 +++++++++++++++++ packages/uploads/test/commands-put.test.ts | 72 +++++++++- packages/uploads/test/metadata.test.ts | 81 +++++++++++ skills/uploads-cli/SKILL.md | 30 ++++ 18 files changed, 1035 insertions(+), 13 deletions(-) create mode 100644 .changeset/file-metadata.md create mode 100644 packages/uploads/src/metadata.ts create mode 100644 packages/uploads/test/cli-args.test.ts create mode 100644 packages/uploads/test/client-metadata.test.ts create mode 100644 packages/uploads/test/commands-find.test.ts create mode 100644 packages/uploads/test/commands-meta.test.ts create mode 100644 packages/uploads/test/metadata.test.ts diff --git a/.changeset/file-metadata.md b/.changeset/file-metadata.md new file mode 100644 index 0000000..d19f823 --- /dev/null +++ b/.changeset/file-metadata.md @@ -0,0 +1,8 @@ +--- +"@buildinternet/uploads": minor +--- + +Add queryable custom metadata to the CLI: `put --meta k=v` (repeatable), `attach` +now writes `gh.repo`/`gh.kind`/`gh.number`/`gh.ref` automatically (plus its own +`--meta` extras), new `meta get`/`meta set` commands, `list --meta k=v` and the +`find k=v...` alias for filtering objects by metadata. diff --git a/packages/uploads/README.md b/packages/uploads/README.md index 7978f69..e01ad01 100644 --- a/packages/uploads/README.md +++ b/packages/uploads/README.md @@ -25,6 +25,10 @@ uploads put ./capture-2026-…Z.png --pr 123 --name hero.png # clean leaf, sta uploads put ./shot.png --pr 123 --name hero.png --dry-run --format url # preview URL, no upload uploads gallery create --title "Release screenshots" uploads put ./after.png --gallery gal_example +uploads put ./shot.png --meta app=myapp --meta page=settings # queryable custom metadata +uploads meta get screenshots/myapp/42/shot.png +uploads meta set screenshots/myapp/42/shot.png page=onboarding --delete device +uploads find app=myapp page=settings # or: list --meta app=myapp uploads doctor ``` @@ -32,8 +36,8 @@ Inside this monorepo only, `pnpm uploads …` builds the package first so you pi up local source; product docs and PR “how to try it” examples should use the global `uploads` form above. -Commands: `attach`, `put`, `gallery`, `comment`, `list`, `delete`, `usage`, `reconcile`, -`purge-expired`, `setup`, `install`, `config`, `doctor`, `health`, `mcp`. +Commands: `attach`, `put`, `gallery`, `comment`, `list`, `find`, `meta`, `delete`, `usage`, +`reconcile`, `purge-expired`, `setup`, `install`, `config`, `doctor`, `health`, `mcp`. **Globals (before the command):** `--api-url`, `--token`, `--workspace` / `-w`, `--env-file`, `--json`, `--quiet`, `--version` / `-V`, `-h` / `--help`. diff --git a/packages/uploads/src/cli-args.ts b/packages/uploads/src/cli-args.ts index 87b80cf..6f5733c 100644 --- a/packages/uploads/src/cli-args.ts +++ b/packages/uploads/src/cli-args.ts @@ -96,17 +96,38 @@ export class UsageError extends Error { export interface CommandFlags { positionals: string[]; - flags: Map; + /** Repeated string flags (e.g. `--meta k=v --meta k2=v2`) collapse into a string[]. */ + flags: Map; help: boolean; } +/** Records a flag occurrence, turning a repeated string flag into an array. */ +function setFlag(flags: CommandFlags["flags"], name: string, value: string | boolean): void { + const existing = flags.get(name); + if (existing === undefined) { + flags.set(name, value); + return; + } + if (Array.isArray(existing)) { + if (typeof value === "string") existing.push(value); + return; + } + if (typeof existing === "string" && typeof value === "string") { + flags.set(name, [existing, value]); + return; + } + flags.set(name, value); +} + /** * Parse command-specific args. Supports `--flag value`, `--flag=value`, and - * boolean `--flag` flags. + * boolean `--flag` flags. A flag repeated multiple times with string values + * (e.g. `--meta app=x --meta page=y`) collapses into a `string[]` — read it + * with `flagValues`, not `flagString`. */ export function parseCommandArgs(args: string[]): CommandFlags { const positionals: string[] = []; - const flags = new Map(); + const flags: CommandFlags["flags"] = new Map(); let help = false; let i = 0; @@ -122,7 +143,7 @@ export function parseCommandArgs(args: string[]): CommandFlags { if (arg.startsWith("--")) { const eq = arg.indexOf("="); if (eq !== -1) { - flags.set(arg.slice(0, eq), arg.slice(eq + 1)); + setFlag(flags, arg.slice(0, eq), arg.slice(eq + 1)); i++; continue; } @@ -130,12 +151,12 @@ export function parseCommandArgs(args: string[]): CommandFlags { const name = arg; const next = args[i + 1]; if (next && !next.startsWith("-")) { - flags.set(name, next); + setFlag(flags, name, next); i += 2; continue; } - flags.set(name, true); + setFlag(flags, name, true); i++; continue; } @@ -156,6 +177,19 @@ export function flagBool(flags: CommandFlags["flags"], name: string): boolean { return flags.get(name) === true; } +/** + * Every string value passed for a repeatable flag (e.g. `--meta k=v`), in + * argument order. Empty when the flag is absent; a single occurrence yields + * a one-element array. + */ +export function flagValues(flags: CommandFlags["flags"], name: string): string[] { + const value = flags.get(name); + if (value === undefined) return []; + if (Array.isArray(value)) return value; + if (typeof value === "string") return [value]; + return []; +} + /** Command-level workspace override (`--workspace` / `-w`). */ export function commandWorkspace(flags: CommandFlags["flags"]): string | undefined { return flagString(flags, "--workspace") ?? flagString(flags, "-w"); diff --git a/packages/uploads/src/cli.ts b/packages/uploads/src/cli.ts index 940b9ad..a55fa22 100644 --- a/packages/uploads/src/cli.ts +++ b/packages/uploads/src/cli.ts @@ -13,6 +13,8 @@ import { runPut, runAttach, runList, + runFind, + runMeta, runDelete, runHealth, runDoctor, @@ -62,7 +64,9 @@ Commands: put Upload (+ URL + markdown for GitHub) gallery Create and organize public media galleries comment Create/update a PR/issue attachments comment (via gh) - list List objects + list List objects (--meta k=v filters by queryable metadata) + find k=v... List objects matching metadata (alias for list --meta) + meta Get/set an object's queryable metadata delete Delete object usage Workspace storage / upload counters reconcile Rebuild usage ledger from storage @@ -264,6 +268,8 @@ export async function runCli(argv: string[]): Promise { case "put": case "gallery": case "list": + case "find": + case "meta": case "delete": case "usage": case "reconcile": @@ -287,6 +293,12 @@ export async function runCli(argv: string[]): Promise { case "list": code = await runList(ctx, cmdArgs, showHelp); break; + case "find": + code = await runFind(ctx, cmdArgs, showHelp); + break; + case "meta": + code = await runMeta(ctx, cmdArgs, showHelp); + break; case "delete": code = await runDelete(ctx, cmdArgs, showHelp); break; diff --git a/packages/uploads/src/client.ts b/packages/uploads/src/client.ts index f37fd4c..bd898f3 100644 --- a/packages/uploads/src/client.ts +++ b/packages/uploads/src/client.ts @@ -22,6 +22,13 @@ export interface PutOptions { deriveRepoFromGit?: boolean; /** Stored as R2 custom metadata; echoed on put/head. */ provenance?: ProvenanceInput; + /** + * Queryable custom metadata (D1 `file_metadata`), sent alongside provenance + * as more `X-Uploads-Meta-` headers — the server routes each key to R2 + * (provenance) or D1 (everything else) by name. See `metadata.ts` for the + * client-side validation callers should run before this. + */ + metadata?: Record; /** Validate key + resolve public URL without writing. `size` is local bytes only. */ dryRun?: boolean; } @@ -32,6 +39,31 @@ export interface ListOptions { cursor?: string; } +export interface FindFilesOptions { + prefix?: string; + limit?: number; +} + +export interface FindFilesItem { + key: string; + url: string | null; + metadata: Record; +} + +export interface FindFilesResult { + items: FindFilesItem[]; + cursor: string | null; +} + +export interface GetMetadataResult { + metadata: Record; +} + +export interface PatchMetadataOptions { + set?: Record; + delete?: string[]; +} + export interface PutResult { workspace: string; key: string; @@ -561,6 +593,13 @@ export function createUploadsClient(config: UploadsClientConfig) { if (v !== undefined && v !== "") headers[`X-Uploads-Meta-${k}`] = v; } } + // Same header prefix as provenance above; the server splits allowlisted + // provenance keys (R2) from everything else (D1 file_metadata) by name. + if (opts.metadata) { + for (const [k, v] of Object.entries(opts.metadata)) { + if (v !== undefined && v !== "") headers[`X-Uploads-Meta-${k}`] = v; + } + } const result = await request<{ workspace: string; @@ -605,6 +644,41 @@ export function createUploadsClient(config: UploadsClientConfig) { return request("DELETE", `${filesBase(config)}/${encodeKeyPath(key)}`); }, + /** `GET /v1/:workspace/files/:key/metadata` — the object's queryable metadata. */ + async getMetadata(key: string): Promise { + return request( + "GET", + `${filesBase(config)}/${encodeKeyPath(key)}/metadata`, + ); + }, + + /** `PATCH /v1/:workspace/files/:key/metadata` — merge `set`/`delete`; returns the merged map. */ + async patchMetadata(key: string, opts: PatchMetadataOptions): Promise { + return request( + "PATCH", + `${filesBase(config)}/${encodeKeyPath(key)}/metadata`, + { + body: new TextEncoder().encode(JSON.stringify(opts)), + headers: { "Content-Type": "application/json" }, + }, + ); + }, + + /** + * `GET /v1/:workspace/files?meta.=&…` — ANDed equality filter over + * queryable metadata. `filters` must be pre-validated (see `metadata.ts`). + */ + async findFiles( + filters: Record, + opts: FindFilesOptions = {}, + ): Promise { + const params = new URLSearchParams(); + for (const [k, v] of Object.entries(filters)) params.append(`meta.${k}`, v); + if (opts.prefix) params.set("prefix", opts.prefix); + if (opts.limit != null) params.set("limit", String(opts.limit)); + return request("GET", `${filesBase(config)}?${params.toString()}`); + }, + async head(key: string): Promise { return request("GET", `${filesBase(config)}/${encodeKeyPath(key)}`); }, diff --git a/packages/uploads/src/commands.ts b/packages/uploads/src/commands.ts index f8fff22..2c46b88 100644 --- a/packages/uploads/src/commands.ts +++ b/packages/uploads/src/commands.ts @@ -6,6 +6,7 @@ import { flagString, flagBool, flagInt, + flagValues, UsageError, type CommandFlags, } from "./cli-args.js"; @@ -18,9 +19,11 @@ import { import { buildMarkdown } from "./embed.js"; import { UploadsError } from "./errors.js"; import { writeJson, writeStdout } from "./io.js"; +import { parseMetaFlags } from "./metadata.js"; import { ghAttachmentKey, ghKeyPrefix, + ghMetadataFromTarget, attachmentsCommentBody, type GhTarget, type AttachmentItem, @@ -108,6 +111,7 @@ Options: --issue Attach to an issue: key gh///issues// --comment With --pr/--issue: update one managed comment with attachments and linked galleries via local gh auth --gallery Add the uploaded object to this public gallery + --meta Queryable custom metadata (repeatable; value may contain "="): key ^[a-z][a-z0-9._-]{0,63}$, value 1-512 printable ASCII, max 24 pairs --dry-run Print key + public URL without uploading. Not with --comment/--gallery Exit codes: 0 ok · 2 usage/token/file · 3 auth/policy · 4 network · 1 other. @@ -121,6 +125,7 @@ Examples: uploads put ./capture-….webp --pr 128 --name hero.webp uploads put ./shot.png --pr 128 --name hero.webp --dry-run --format url uploads put ./after.png --gallery gal_example + uploads put ./shot.png --meta app=myapp --meta page=settings `; /** @@ -337,12 +342,16 @@ Options: --optimize-quality <1-100> WebP quality (default: 85) --keep-exif Keep EXIF/XMP/ICC when optimizing (default: strip for privacy) --workspace, -w Override workspace + --meta Extra queryable metadata (repeatable; value may contain "="). + gh.repo/gh.kind/gh.number/gh.ref are always set from the resolved + target — a --meta pair with the same key is overridden by it. Examples: uploads attach ./before.png ./after.png uploads attach ./mobile.png --frame phone uploads attach ./shot.png --pr 123 --repo myorg/myapp uploads attach ./artifact.zip --issue 45 --no-comment + uploads attach ./shot.png --meta app=myapp --meta page=settings `; export async function runAttach( @@ -372,6 +381,10 @@ export async function runAttach( const optimizeOpts = optimizeOptionsFromFlags(parsed.flags, defaults); const frameOpts = frameOptionsFromFlags(parsed.flags); const contentTypeOverride = flagString(parsed.flags, "--content-type"); + // User-supplied extras first, then the resolved target's gh.* — explicit + // target pairs always win over a same-named --meta extra (documented above). + const metaExtras = parseMetaFlags(flagValues(parsed.flags, "--meta")); + const metadata = { ...metaExtras, ...ghMetadataFromTarget(target) }; const results = []; for (const file of parsed.positionals) { if (file === "-") @@ -397,6 +410,7 @@ export async function runAttach( frameId: prepared.frame?.framed ? prepared.frame.frameId : undefined, keepExif: optimizeOpts.keepExif === true, }), + metadata, }); results.push({ ...result, @@ -466,6 +480,11 @@ export async function runPut( const galleryId = flagString(parsed.flags, "--gallery"); const nameFlag = flagString(parsed.flags, "--name"); const dryRun = flagBool(parsed.flags, "--dry-run"); + // Validate --meta up front (fail fast, before reading/optimizing the file). + const metadata = ((): Record | undefined => { + const pairs = flagValues(parsed.flags, "--meta"); + return pairs.length > 0 ? parseMetaFlags(pairs) : undefined; + })(); if (wantComment && typeof parsed.flags.get("--comment") === "string") { throw new UsageError("--comment takes no value — place it after the file argument"); } @@ -563,6 +582,7 @@ export async function runPut( frameId: prepared.frame?.framed ? prepared.frame.frameId : undefined, keepExif: optimizeOpts.keepExif === true, }), + metadata, }); const markdown = buildMarkdown(result.url, { alt, width }); @@ -862,16 +882,37 @@ export async function runGallery(ctx: CliContext, args: string[], help = false): // --- list --- -const LIST_HELP = `uploads list [--prefix

] [--pr | --issue ] [--repo ] [--limit ] [--cursor ] [--all] [--workspace ] +const LIST_HELP = `uploads list [--prefix

] [--pr | --issue ] [--repo ] [--limit ] [--cursor ] [--all] [--meta ]... [--workspace ] Default prefix: UPLOADS_DEFAULT_PREFIX (screenshots if unset). +--meta (repeatable, ANDed) switches to the metadata filter endpoint — +returned items include their matched metadata. Combines with --prefix, not +with --pr/--issue/--all. See also: uploads find (positional-pair alias). + Examples: uploads list --prefix screenshots/ uploads list --pr 123 uploads list --all --json + uploads list --meta gh.repo=buildinternet/uploads --meta gh.number=123 `; +/** `--meta k=v` (repeatable) filter path, shared by `runList` and `runFind`. */ +async function runFindFiles( + ctx: CliContext, + filters: Record, + flags: CommandFlags["flags"], +): Promise { + const prefix = flagString(flags, "--prefix"); + const limit = flagInt(flags, "--limit", "--limit"); + const result = await ctx.client.findFiles(filters, { prefix, limit }); + if (ctx.json) await writeJson(result); + else + for (const item of result.items) + await writeStdout(`${item.key}${item.url ? ` ${item.url}` : ""}\n`); + return 0; +} + export async function runList( ctx: CliContext, args: string[], @@ -883,6 +924,16 @@ export async function runList( process.stderr.write(LIST_HELP); return 0; } + const metaPairs = flagValues(parsed.flags, "--meta"); + if (metaPairs.length > 0) { + if (ghTargetFromFlags(parsed.flags, run)) { + throw new UsageError("--meta cannot be combined with --pr/--issue"); + } + if (flagBool(parsed.flags, "--all")) { + throw new UsageError("--meta cannot be combined with --all"); + } + return runFindFiles(ctx, parseMetaFlags(metaPairs), parsed.flags); + } const defaults = resolvePutDefaults({ envFile: ctx.envFile }); const prefixFlag = flagString(parsed.flags, "--prefix"); let prefix = prefixFlag ?? (defaults.prefix ? `${defaults.prefix}/` : undefined); @@ -914,6 +965,88 @@ export async function runList( return 0; } +// --- find --- + +const FIND_HELP = `uploads find k=v [k=v...] [--prefix

] [--limit ] [--workspace ] + +Human-friendly alias for \`uploads list --meta k=v...\` — same metadata filter +(ANDed equality), same output; pairs are positional instead of repeated flags. + +Examples: + uploads find gh.repo=buildinternet/uploads gh.number=123 + uploads find app=myapp page=settings --prefix screenshots/ +`; + +export async function runFind(ctx: CliContext, args: string[], help = false): Promise { + const parsed = parseCommandArgs(args); + if (help || parsed.help) { + process.stderr.write(FIND_HELP); + return 0; + } + if (parsed.positionals.length === 0) { + process.stderr.write(FIND_HELP); + return 2; + } + const filters = parseMetaFlags(parsed.positionals); + return runFindFiles(ctx, filters, parsed.flags); +} + +// --- meta --- + +const META_HELP = `uploads meta [args] + +Read/write an object's queryable custom metadata (D1-backed key-value pairs; +distinct from the R2 provenance headers put on upload). + +Commands: + get Show metadata for an object + set k=v [k=v...] [--delete k]... Merge-set and/or delete pairs + +Examples: + uploads meta get screenshots/myapp/42/shot.png + uploads meta set screenshots/myapp/42/shot.png app=myapp page=settings + uploads meta set screenshots/myapp/42/shot.png --delete app --delete page +`; + +export async function runMeta(ctx: CliContext, args: string[], help = false): Promise { + const parsed = parseCommandArgs(args); + const action = parsed.positionals[0]; + if (help || parsed.help || !action) { + process.stderr.write(META_HELP); + return help || parsed.help ? 0 : 2; + } + + switch (action) { + case "get": { + const key = parsed.positionals[1]; + if (!key) throw new UsageError("meta get requires an object key"); + const result = await ctx.client.getMetadata(key); + if (ctx.json) await writeJson(result); + else for (const [k, v] of Object.entries(result.metadata)) await writeStdout(`${k}=${v}\n`); + return 0; + } + case "set": { + const key = parsed.positionals[1]; + if (!key) throw new UsageError("meta set requires an object key"); + const pairs = parsed.positionals.slice(2); + const del = flagValues(parsed.flags, "--delete"); + if (pairs.length === 0 && del.length === 0) { + throw new UsageError("meta set requires k=v pairs and/or --delete "); + } + const set = pairs.length > 0 ? parseMetaFlags(pairs) : undefined; + const result = await ctx.client.patchMetadata(key, { + set, + delete: del.length > 0 ? del : undefined, + }); + if (ctx.json) await writeJson(result); + else for (const [k, v] of Object.entries(result.metadata)) await writeStdout(`${k}=${v}\n`); + return 0; + } + default: + throw new UsageError(`unknown meta command: ${action}`); + } +} + // --- delete --- const DELETE_HELP = `uploads delete [--dry-run] [--workspace ] diff --git a/packages/uploads/src/github.ts b/packages/uploads/src/github.ts index d5b0620..696d3db 100644 --- a/packages/uploads/src/github.ts +++ b/packages/uploads/src/github.ts @@ -85,6 +85,23 @@ export function ghAttachmentKey(target: GhTarget, filename: string): string { return `${ghKeyPrefix(target)}${sanitizeKeySegment(filename)}`; } +/** + * The four `gh.*` queryable-metadata pairs `uploads attach` writes + * automatically (`.context/2026-07-13-file-metadata-design.md`). `gh.kind` + * uses the API's singular vocabulary (`pull`/`issue`), distinct from + * `GhTarget.kind`'s URL-segment spelling (`pull`/`issues`). `gh.ref` reuses + * the same lowercased `owner/repo#number` coordinate as gallery GitHub + * references, so both surfaces resolve the same lookup key. + */ +export function ghMetadataFromTarget(target: GhTarget): Record { + return { + "gh.repo": target.repo, + "gh.kind": target.kind === "issues" ? "issue" : "pull", + "gh.number": String(target.num), + "gh.ref": `${target.repo.toLowerCase()}#${target.num}`, + }; +} + /** Hidden marker identifying the one comment this CLI manages. Never change it — existing comments are found by exact match. */ export const ATTACHMENTS_MARKER = ""; diff --git a/packages/uploads/src/index.ts b/packages/uploads/src/index.ts index 15903aa..15c5e14 100644 --- a/packages/uploads/src/index.ts +++ b/packages/uploads/src/index.ts @@ -61,13 +61,27 @@ export { type ReconcileResult, type PurgeExpiredResult, type PurgeExpiredResponse, + type FindFilesOptions, + type FindFilesItem, + type FindFilesResult, + type GetMetadataResult, + type PatchMetadataOptions, } from "./client.js"; export { buildCliProvenance } from "./provenance.js"; +export { + META_KEY_RE, + META_VALUE_MAX, + META_MAX_KEYS, + validateMetaEntry, + parseMetaPair, + parseMetaFlags, +} from "./metadata.js"; export { ATTACHMENTS_MARKER, attachmentsCommentBody, ghAttachmentKey, ghKeyPrefix, + ghMetadataFromTarget, isValidRepo, parseRepoFromRemoteUrl, type AttachmentItem, diff --git a/packages/uploads/src/metadata.ts b/packages/uploads/src/metadata.ts new file mode 100644 index 0000000..eb3e52a --- /dev/null +++ b/packages/uploads/src/metadata.ts @@ -0,0 +1,76 @@ +/** + * Client-side validation for `--meta k=v` / `meta set` pairs, mirroring the + * server rules in `apps/api/src/file-metadata.ts` (`.context/2026-07-13-file-metadata-design.md`). + * Validating here lets the CLI fail fast with a readable message instead of a + * round-trip 400 — the server remains the source of truth and re-validates on + * every write. + */ +import { UsageError } from "./cli-args.js"; + +/** Lowercase key, optionally dot-namespaced (e.g. `gh.repo`). Mirrors META_KEY_RE server-side. */ +export const META_KEY_RE = /^[a-z][a-z0-9._-]{0,63}$/; + +/** Max value length in characters (mirrors META_VALUE_MAX server-side). */ +export const META_VALUE_MAX = 512; + +/** Cap on keys per request (mirrors META_MAX_KEYS server-side). */ +export const META_MAX_KEYS = 24; + +// Printable ASCII only — same rule as the server's file-metadata.ts. +const VALUE_SAFE_RE = /^[\x20-\x7E]+$/; + +/** + * Server-computed keys the API rejects as custom metadata (currently just + * `content-sha256`). `gh.*` is NOT reserved here: it's system-managed by + * convention (attach flow), not blocked — the server happily accepts + * user-supplied `gh.*` extras via `--meta`. + */ +const RESERVED_META_KEYS = new Set(["content-sha256"]); + +/** Throws `UsageError` with a readable message if `key`/`value` violate the metadata rules. */ +export function validateMetaEntry(key: string, value: string): void { + if (!META_KEY_RE.test(key)) { + throw new UsageError(`invalid metadata key: "${key}" (must match ^[a-z][a-z0-9._-]{0,63}$)`); + } + if (RESERVED_META_KEYS.has(key)) { + throw new UsageError(`invalid metadata key: "${key}" is reserved (server-computed)`); + } + if (value.length < 1 || value.length > META_VALUE_MAX || !VALUE_SAFE_RE.test(value)) { + throw new UsageError( + `invalid metadata value for key "${key}": must be 1-${META_VALUE_MAX} printable ASCII characters`, + ); + } +} + +/** + * Split `k=v` on the FIRST "=" (so values may themselves contain "="), then + * validate the pair. Throws `UsageError` on malformed input. + */ +export function parseMetaPair(raw: string): [string, string] { + const eq = raw.indexOf("="); + if (eq === -1) { + throw new UsageError(`invalid --meta value: "${raw}" (expected key=value)`); + } + const key = raw.slice(0, eq); + const value = raw.slice(eq + 1); + validateMetaEntry(key, value); + return [key, value]; +} + +/** + * Parse and validate a batch of `k=v` pairs (e.g. every `--meta` occurrence, + * or `meta set`'s positional pairs) into a map. Fails fast on the first + * invalid pair or when the batch exceeds `META_MAX_KEYS`. Later duplicate + * keys in the same batch win (last write). + */ +export function parseMetaFlags(pairs: string[]): Record { + if (pairs.length > META_MAX_KEYS) { + throw new UsageError(`too many --meta pairs: at most ${META_MAX_KEYS} per request`); + } + const result: Record = {}; + for (const pair of pairs) { + const [key, value] = parseMetaPair(pair); + result[key] = value; + } + return result; +} diff --git a/packages/uploads/test/cli-args.test.ts b/packages/uploads/test/cli-args.test.ts new file mode 100644 index 0000000..fbdc72b --- /dev/null +++ b/packages/uploads/test/cli-args.test.ts @@ -0,0 +1,35 @@ +import { describe, expect, it } from "vitest"; +import { flagString, flagValues, parseCommandArgs } from "../src/cli-args.js"; + +describe("parseCommandArgs repeatable flags", () => { + it("collapses a single occurrence into a one-element array via flagValues", () => { + const { flags } = parseCommandArgs(["--meta", "app=x"]); + expect(flagValues(flags, "--meta")).toEqual(["app=x"]); + expect(flagString(flags, "--meta")).toBe("app=x"); + }); + + it("collects repeated occurrences into an array in argument order", () => { + const { flags } = parseCommandArgs(["--meta", "app=x", "--meta", "page=y"]); + expect(flagValues(flags, "--meta")).toEqual(["app=x", "page=y"]); + }); + + it("flagString returns undefined once a flag has repeated (avoids silently using only the last value)", () => { + const { flags } = parseCommandArgs(["--meta", "app=x", "--meta", "page=y"]); + expect(flagString(flags, "--meta")).toBeUndefined(); + }); + + it("returns an empty array when the flag is absent", () => { + const { flags } = parseCommandArgs(["--other", "1"]); + expect(flagValues(flags, "--meta")).toEqual([]); + }); + + it("supports repeated --flag=value form", () => { + const { flags } = parseCommandArgs(["--meta=app=x", "--meta=page=y"]); + expect(flagValues(flags, "--meta")).toEqual(["app=x", "page=y"]); + }); + + it("does not disturb unrelated single-occurrence flags", () => { + const { flags } = parseCommandArgs(["--meta", "app=x", "--prefix", "screenshots/"]); + expect(flagString(flags, "--prefix")).toBe("screenshots/"); + }); +}); diff --git a/packages/uploads/test/client-metadata.test.ts b/packages/uploads/test/client-metadata.test.ts new file mode 100644 index 0000000..b7ac1ed --- /dev/null +++ b/packages/uploads/test/client-metadata.test.ts @@ -0,0 +1,134 @@ +import { afterEach, describe, expect, it, vi } from "vitest"; +import { createUploadsClient } from "../src/client.js"; + +afterEach(() => vi.unstubAllGlobals()); + +describe("put metadata headers", () => { + it("emits X-Uploads-Meta- headers for metadata alongside provenance", async () => { + let seenHeaders: Headers | undefined; + const fetch = vi.fn(async (_input: string | URL | Request, init?: RequestInit) => { + seenHeaders = new Headers(init?.headers); + return new Response( + JSON.stringify({ + workspace: "test", + key: "screenshots/a.png", + url: "https://storage.test/a.png", + size: 1, + contentType: "image/png", + }), + { status: 201 }, + ); + }); + vi.stubGlobal("fetch", fetch); + const client = createUploadsClient({ + apiUrl: "https://api.test", + workspace: "test", + token: "up_test_x", + }); + + await client.put(new Uint8Array([1]), { + filename: "a.png", + key: "screenshots/a.png", + provenance: { client: "uploads-cli" }, + metadata: { app: "myapp", "gh.repo": "buildinternet/uploads" }, + }); + + expect(seenHeaders?.get("X-Uploads-Meta-client")).toBe("uploads-cli"); + expect(seenHeaders?.get("X-Uploads-Meta-app")).toBe("myapp"); + expect(seenHeaders?.get("X-Uploads-Meta-gh.repo")).toBe("buildinternet/uploads"); + }); + + it("omits metadata headers entirely when metadata is not provided", async () => { + let seenHeaders: Headers | undefined; + const fetch = vi.fn(async (_input: string | URL | Request, init?: RequestInit) => { + seenHeaders = new Headers(init?.headers); + return new Response( + JSON.stringify({ + workspace: "test", + key: "screenshots/a.png", + url: "https://storage.test/a.png", + size: 1, + contentType: "image/png", + }), + { status: 201 }, + ); + }); + vi.stubGlobal("fetch", fetch); + const client = createUploadsClient({ + apiUrl: "https://api.test", + workspace: "test", + token: "up_test_x", + }); + + await client.put(new Uint8Array([1]), { filename: "a.png", key: "screenshots/a.png" }); + + expect( + [...seenHeaders!.keys()].some((k) => k.toLowerCase().startsWith("x-uploads-meta-")), + ).toBe(false); + }); +}); + +describe("metadata CRUD client methods", () => { + it("getMetadata GETs the /metadata sibling route", async () => { + const fetch = vi.fn(async (input: string | URL | Request, init?: RequestInit) => { + expect(String(input)).toBe("https://api.test/v1/test/files/screenshots/a.png/metadata"); + expect(init?.method).toBe("GET"); + return new Response(JSON.stringify({ metadata: { app: "myapp" } })); + }); + vi.stubGlobal("fetch", fetch); + const client = createUploadsClient({ + apiUrl: "https://api.test", + workspace: "test", + token: "up_test_x", + }); + expect(await client.getMetadata("screenshots/a.png")).toEqual({ metadata: { app: "myapp" } }); + }); + + it("patchMetadata PATCHes { set, delete } and returns the merged map", async () => { + const fetch = vi.fn(async (input: string | URL | Request, init?: RequestInit) => { + expect(String(input)).toBe("https://api.test/v1/test/files/screenshots/a.png/metadata"); + expect(init?.method).toBe("PATCH"); + const body = JSON.parse(new TextDecoder().decode(init?.body as Uint8Array)); + expect(body).toEqual({ set: { app: "myapp" }, delete: ["page"] }); + return new Response(JSON.stringify({ metadata: { app: "myapp" } })); + }); + vi.stubGlobal("fetch", fetch); + const client = createUploadsClient({ + apiUrl: "https://api.test", + workspace: "test", + token: "up_test_x", + }); + expect( + await client.patchMetadata("screenshots/a.png", { set: { app: "myapp" }, delete: ["page"] }), + ).toEqual({ metadata: { app: "myapp" } }); + }); + + it("findFiles sends repeatable ANDed meta. params plus prefix/limit", async () => { + const fetch = vi.fn(async (input: string | URL | Request) => { + const url = new URL(String(input)); + expect(url.pathname).toBe("/v1/test/files"); + expect(url.searchParams.getAll("meta.gh.repo")).toEqual(["buildinternet/uploads"]); + expect(url.searchParams.getAll("meta.gh.number")).toEqual(["123"]); + expect(url.searchParams.get("prefix")).toBe("gh/"); + expect(url.searchParams.get("limit")).toBe("10"); + return new Response( + JSON.stringify({ + items: [{ key: "gh/o/r/pull/123/a.png", url: "https://x.test/a.png", metadata: {} }], + cursor: null, + }), + ); + }); + vi.stubGlobal("fetch", fetch); + const client = createUploadsClient({ + apiUrl: "https://api.test", + workspace: "test", + token: "up_test_x", + }); + const result = await client.findFiles( + { "gh.repo": "buildinternet/uploads", "gh.number": "123" }, + { prefix: "gh/", limit: 10 }, + ); + expect(result.items[0].key).toBe("gh/o/r/pull/123/a.png"); + expect(result.cursor).toBeNull(); + }); +}); diff --git a/packages/uploads/test/commands-attach.test.ts b/packages/uploads/test/commands-attach.test.ts index 01bf9b1..898dafa 100644 --- a/packages/uploads/test/commands-attach.test.ts +++ b/packages/uploads/test/commands-attach.test.ts @@ -18,6 +18,7 @@ function files(...names: string[]): string[] { function fakeClient() { const puts: string[] = []; + const metadataByKey: Record | undefined> = {}; const list = async ({ prefix }: { prefix?: string } = {}) => ({ items: puts .filter((key) => key.startsWith(prefix ?? "")) @@ -25,8 +26,9 @@ function fakeClient() { cursor: null, }); const client = { - put: async (_body: Uint8Array, opts: { key: string }) => { + put: async (_body: Uint8Array, opts: { key: string; metadata?: Record }) => { puts.push(opts.key); + metadataByKey[opts.key] = opts.metadata; return { workspace: "test", key: opts.key, @@ -40,7 +42,7 @@ function fakeClient() { findGalleriesByReference: async () => ({ galleries: [], nextCursor: null }), getGallery: async () => ({ items: [] }), } as unknown as UploadsClient; - return { client, puts }; + return { client, puts, metadataByKey }; } function ctxWith(client: UploadsClient): CliContext { @@ -111,3 +113,46 @@ describe("runAttach", () => { ).rejects.toThrow(UsageError); }); }); + +describe("runAttach gh.* metadata", () => { + it("writes gh.repo/gh.kind/gh.number/gh.ref for a pull request target", async () => { + const { client, metadataByKey } = fakeClient(); + const { run } = ghRunner(); + await runAttach(ctxWith(client), files("shot.png"), false, run); + expect(metadataByKey["gh/buildinternet/uploads/pull/123/shot.png"]).toEqual({ + "gh.repo": "buildinternet/uploads", + "gh.kind": "pull", + "gh.number": "123", + "gh.ref": "buildinternet/uploads#123", + }); + }); + + it("uses gh.kind=issue for an --issue target", async () => { + const { client, metadataByKey } = fakeClient(); + const { run } = ghRunner(); + await runAttach( + ctxWith(client), + [...files("artifact.zip"), "--issue", "45", "--repo", "o/r", "--no-comment"], + false, + run, + ); + expect(metadataByKey["gh/o/r/issues/45/artifact.zip"]).toMatchObject({ + "gh.kind": "issue", + "gh.ref": "o/r#45", + }); + }); + + it("merges --meta extras and lets the resolved target's gh.* win over a same-named extra", async () => { + const { client, metadataByKey } = fakeClient(); + const { run } = ghRunner(); + await runAttach( + ctxWith(client), + [...files("shot.png"), "--meta", "app=myapp", "--meta", "gh.repo=should-be-overridden/nope"], + false, + run, + ); + const metadata = metadataByKey["gh/buildinternet/uploads/pull/123/shot.png"]; + expect(metadata?.app).toBe("myapp"); + expect(metadata?.["gh.repo"]).toBe("buildinternet/uploads"); + }); +}); diff --git a/packages/uploads/test/commands-find.test.ts b/packages/uploads/test/commands-find.test.ts new file mode 100644 index 0000000..7506d4b --- /dev/null +++ b/packages/uploads/test/commands-find.test.ts @@ -0,0 +1,71 @@ +import { describe, expect, it } from "vitest"; +import { UsageError } from "../src/cli-args.js"; +import type { UploadsClient } from "../src/client.js"; +import { runFind, type CliContext } from "../src/commands.js"; + +function fakeClient() { + const calls: { filters: Record; prefix?: string; limit?: number }[] = []; + const client = { + findFiles: async ( + filters: Record, + opts: { prefix?: string; limit?: number } = {}, + ) => { + calls.push({ filters, prefix: opts.prefix, limit: opts.limit }); + return { + items: [{ key: "gh/o/r/pull/123/a.png", url: "https://x.test/a.png", metadata: filters }], + cursor: null, + }; + }, + } as unknown as UploadsClient; + return { client, calls }; +} + +function ctxWith(client: UploadsClient): CliContext { + return { + config: { + apiUrl: "https://x.test", + workspace: "test", + token: "up_test_x", + workspaceSource: "override", + configPath: "/tmp/uploads-test-config", + configExists: false, + }, + client, + json: false, + quiet: true, + }; +} + +describe("runFind", () => { + it("parses positional k=v pairs and hits the filter endpoint", async () => { + const { client, calls } = fakeClient(); + const code = await runFind( + ctxWith(client), + ["gh.repo=buildinternet/uploads", "gh.number=123"], + false, + ); + expect(code).toBe(0); + expect(calls[0].filters).toEqual({ "gh.repo": "buildinternet/uploads", "gh.number": "123" }); + }); + + it("requires at least one pair", async () => { + const { client } = fakeClient(); + expect(await runFind(ctxWith(client), [], false)).toBe(2); + }); + + it("rejects a malformed pair", async () => { + const { client } = fakeClient(); + await expect(runFind(ctxWith(client), ["nokeyvalue"], false)).rejects.toThrow(UsageError); + }); + + it("combines with --prefix and --limit", async () => { + const { client, calls } = fakeClient(); + await runFind( + ctxWith(client), + ["app=myapp", "--prefix", "screenshots/", "--limit", "5"], + false, + ); + expect(calls[0].prefix).toBe("screenshots/"); + expect(calls[0].limit).toBe(5); + }); +}); diff --git a/packages/uploads/test/commands-list.test.ts b/packages/uploads/test/commands-list.test.ts index cc935a8..0a4d02e 100644 --- a/packages/uploads/test/commands-list.test.ts +++ b/packages/uploads/test/commands-list.test.ts @@ -35,6 +35,25 @@ const noRun: CommandRunner = () => { throw new Error("runner should not be called"); }; +const ghRun: CommandRunner = () => "o/r"; + +function fakeFindClient() { + const calls: { filters: Record; prefix?: string; limit?: number }[] = []; + const client = { + findFiles: async ( + filters: Record, + opts: { prefix?: string; limit?: number } = {}, + ) => { + calls.push({ filters, prefix: opts.prefix, limit: opts.limit }); + return { + items: [{ key: "gh/o/r/pull/1/a.png", url: "https://x.test/a.png", metadata: filters }], + cursor: null, + }; + }, + } as unknown as UploadsClient; + return { client, calls }; +} + describe("runList --pr/--issue", () => { it("translates --pr to the gh prefix", async () => { const { client, prefixes } = fakeListClient(); @@ -61,3 +80,39 @@ describe("runList --pr/--issue", () => { expect(prefixes[0]).toBe("screenshots/"); }); }); + +describe("runList --meta", () => { + it("switches to the metadata filter endpoint with a single --meta pair", async () => { + const { client, calls } = fakeFindClient(); + const code = await runList(ctxWith(client), ["--meta", "app=myapp"], false, noRun); + expect(code).toBe(0); + expect(calls[0].filters).toEqual({ app: "myapp" }); + }); + + it("ANDs repeated --meta pairs and combines with --prefix/--limit", async () => { + const { client, calls } = fakeFindClient(); + await runList( + ctxWith(client), + ["--meta", "gh.repo=o/r", "--meta", "gh.number=1", "--prefix", "gh/", "--limit", "10"], + false, + noRun, + ); + expect(calls[0].filters).toEqual({ "gh.repo": "o/r", "gh.number": "1" }); + expect(calls[0].prefix).toBe("gh/"); + expect(calls[0].limit).toBe(10); + }); + + it("rejects --meta combined with --pr", async () => { + const { client } = fakeFindClient(); + await expect( + runList(ctxWith(client), ["--meta", "app=x", "--pr", "1", "--repo", "o/r"], false, ghRun), + ).rejects.toThrow(UsageError); + }); + + it("rejects --meta combined with --all", async () => { + const { client } = fakeFindClient(); + await expect( + runList(ctxWith(client), ["--meta", "app=x", "--all"], false, noRun), + ).rejects.toThrow(UsageError); + }); +}); diff --git a/packages/uploads/test/commands-meta.test.ts b/packages/uploads/test/commands-meta.test.ts new file mode 100644 index 0000000..48f6b4f --- /dev/null +++ b/packages/uploads/test/commands-meta.test.ts @@ -0,0 +1,129 @@ +import { describe, expect, it } from "vitest"; +import { UsageError } from "../src/cli-args.js"; +import type { UploadsClient } from "../src/client.js"; +import { runMeta, type CliContext } from "../src/commands.js"; + +function fakeClient() { + const getCalls: string[] = []; + const patchCalls: { key: string; set?: Record; delete?: string[] }[] = []; + const client = { + getMetadata: async (key: string) => { + getCalls.push(key); + return { metadata: { app: "myapp" } }; + }, + patchMetadata: async ( + key: string, + opts: { set?: Record; delete?: string[] }, + ) => { + patchCalls.push({ key, ...opts }); + return { metadata: { ...opts.set } }; + }, + } as unknown as UploadsClient; + return { client, getCalls, patchCalls }; +} + +function ctxWith(client: UploadsClient): CliContext { + return { + config: { + apiUrl: "https://x.test", + workspace: "test", + token: "up_test_x", + workspaceSource: "override", + configPath: "/tmp/uploads-test-config", + configExists: false, + }, + client, + json: false, + quiet: true, + }; +} + +describe("runMeta get", () => { + it("fetches metadata for a key", async () => { + const { client, getCalls } = fakeClient(); + const code = await runMeta(ctxWith(client), ["get", "screenshots/a.png"], false); + expect(code).toBe(0); + expect(getCalls).toEqual(["screenshots/a.png"]); + }); + + it("requires a key", async () => { + const { client } = fakeClient(); + await expect(runMeta(ctxWith(client), ["get"], false)).rejects.toThrow(UsageError); + }); +}); + +describe("runMeta set", () => { + it("sends k=v pairs as `set`", async () => { + const { client, patchCalls } = fakeClient(); + const code = await runMeta( + ctxWith(client), + ["set", "screenshots/a.png", "app=myapp", "page=settings"], + false, + ); + expect(code).toBe(0); + expect(patchCalls[0]).toEqual({ + key: "screenshots/a.png", + set: { app: "myapp", page: "settings" }, + delete: undefined, + }); + }); + + it("sends repeated --delete flags as a `delete` array", async () => { + const { client, patchCalls } = fakeClient(); + await runMeta( + ctxWith(client), + ["set", "screenshots/a.png", "--delete", "app", "--delete", "page"], + false, + ); + expect(patchCalls[0]).toEqual({ + key: "screenshots/a.png", + set: undefined, + delete: ["app", "page"], + }); + }); + + it("combines set pairs and --delete in one call", async () => { + const { client, patchCalls } = fakeClient(); + await runMeta( + ctxWith(client), + ["set", "screenshots/a.png", "app=myapp", "--delete", "page"], + false, + ); + expect(patchCalls[0]).toEqual({ + key: "screenshots/a.png", + set: { app: "myapp" }, + delete: ["page"], + }); + }); + + it("requires a key", async () => { + const { client } = fakeClient(); + await expect(runMeta(ctxWith(client), ["set"], false)).rejects.toThrow(UsageError); + }); + + it("requires at least one k=v pair or --delete", async () => { + const { client } = fakeClient(); + await expect(runMeta(ctxWith(client), ["set", "screenshots/a.png"], false)).rejects.toThrow( + UsageError, + ); + }); + + it("rejects a malformed k=v pair", async () => { + const { client } = fakeClient(); + await expect( + runMeta(ctxWith(client), ["set", "screenshots/a.png", "nokeyvalue"], false), + ).rejects.toThrow(UsageError); + }); +}); + +describe("runMeta unknown command", () => { + it("rejects an unrecognized subcommand", async () => { + const { client } = fakeClient(); + await expect(runMeta(ctxWith(client), ["bogus"], false)).rejects.toThrow(UsageError); + }); + + it("prints help and returns 2 with no subcommand", async () => { + const { client } = fakeClient(); + expect(await runMeta(ctxWith(client), [], false)).toBe(2); + }); +}); diff --git a/packages/uploads/test/commands-put.test.ts b/packages/uploads/test/commands-put.test.ts index 68e6ca2..225706e 100644 --- a/packages/uploads/test/commands-put.test.ts +++ b/packages/uploads/test/commands-put.test.ts @@ -16,11 +16,18 @@ function fakeClient() { prefix?: string; dryRun?: boolean; body: Uint8Array; + metadata?: Record; }[] = []; const client = { put: async ( body: Uint8Array, - opts: { filename: string; key?: string; prefix?: string; dryRun?: boolean }, + opts: { + filename: string; + key?: string; + prefix?: string; + dryRun?: boolean; + metadata?: Record; + }, ) => { puts.push({ key: opts.key, @@ -28,6 +35,7 @@ function fakeClient() { prefix: opts.prefix, dryRun: opts.dryRun, body, + metadata: opts.metadata, }); return { workspace: "test", @@ -310,3 +318,65 @@ describe("runPut --destination", () => { ).rejects.toThrow(/must start with destination root/); }); }); + +describe("runPut --meta", () => { + it("passes a single --meta pair through to the client", async () => { + const { client, puts } = fakeClient(); + const code = await runPut( + ctxWith(client), + [tmpFile(), "--repo", "myapp", "--no-git", "--meta", "app=myapp"], + false, + noRun, + ); + expect(code).toBe(0); + expect(puts[0].metadata).toEqual({ app: "myapp" }); + }); + + it("collects repeated --meta pairs into one map", async () => { + const { client, puts } = fakeClient(); + await runPut( + ctxWith(client), + [tmpFile(), "--repo", "myapp", "--no-git", "--meta", "app=myapp", "--meta", "page=settings"], + false, + noRun, + ); + expect(puts[0].metadata).toEqual({ app: "myapp", page: "settings" }); + }); + + it("splits a --meta value containing '=' on the first '=' only", async () => { + const { client, puts } = fakeClient(); + await runPut( + ctxWith(client), + [tmpFile(), "--repo", "myapp", "--no-git", "--meta", "url=https://x.test/a?b=c"], + false, + noRun, + ); + expect(puts[0].metadata).toEqual({ url: "https://x.test/a?b=c" }); + }); + + it("omits metadata entirely when --meta is not passed", async () => { + const { client, puts } = fakeClient(); + await runPut(ctxWith(client), [tmpFile(), "--repo", "myapp", "--no-git"], false, noRun); + expect(puts[0].metadata).toBeUndefined(); + }); + + it("rejects an invalid --meta key before uploading", async () => { + const { client, puts } = fakeClient(); + await expect( + runPut( + ctxWith(client), + [tmpFile(), "--repo", "myapp", "--no-git", "--meta", "Bad-Key=x"], + false, + noRun, + ), + ).rejects.toThrow(UsageError); + expect(puts).toEqual([]); + }); + + it("rejects a malformed --meta pair with no '='", async () => { + const { client } = fakeClient(); + await expect( + runPut(ctxWith(client), [tmpFile(), "--meta", "noequals"], false, noRun), + ).rejects.toThrow(UsageError); + }); +}); diff --git a/packages/uploads/test/metadata.test.ts b/packages/uploads/test/metadata.test.ts new file mode 100644 index 0000000..ef5d2c0 --- /dev/null +++ b/packages/uploads/test/metadata.test.ts @@ -0,0 +1,81 @@ +import { describe, expect, it } from "vitest"; +import { UsageError } from "../src/cli-args.js"; +import { parseMetaFlags, parseMetaPair, validateMetaEntry } from "../src/metadata.js"; + +describe("parseMetaPair", () => { + it("splits on the first '=' only", () => { + expect(parseMetaPair("url=https://example.com/a?b=c")).toEqual([ + "url", + "https://example.com/a?b=c", + ]); + }); + + it("rejects a pair with no '='", () => { + expect(() => parseMetaPair("nokeyvalue")).toThrow(UsageError); + }); + + it("rejects an invalid key", () => { + expect(() => parseMetaPair("Bad-Key=x")).toThrow(UsageError); + expect(() => parseMetaPair("1abc=x")).toThrow(UsageError); + }); + + it("accepts a dot-namespaced key", () => { + expect(parseMetaPair("gh.repo=buildinternet/uploads")).toEqual([ + "gh.repo", + "buildinternet/uploads", + ]); + }); + + it("rejects the reserved content-sha256 key", () => { + expect(() => parseMetaPair("content-sha256=abc")).toThrow(UsageError); + }); + + it("rejects an empty value", () => { + expect(() => parseMetaPair("app=")).toThrow(UsageError); + }); + + it("rejects a value over 512 chars", () => { + expect(() => parseMetaPair(`app=${"x".repeat(513)}`)).toThrow(UsageError); + }); + + it("accepts a value at exactly 512 chars", () => { + const value = "x".repeat(512); + expect(parseMetaPair(`app=${value}`)).toEqual(["app", value]); + }); + + it("rejects a non-printable-ASCII value", () => { + expect(() => parseMetaPair("app=café")).toThrow(UsageError); + }); +}); + +describe("validateMetaEntry", () => { + it("does not throw for a valid pair", () => { + expect(() => validateMetaEntry("page", "settings")).not.toThrow(); + }); +}); + +describe("parseMetaFlags", () => { + it("parses multiple pairs into a map", () => { + expect(parseMetaFlags(["app=myapp", "page=settings"])).toEqual({ + app: "myapp", + page: "settings", + }); + }); + + it("returns an empty map for no pairs", () => { + expect(parseMetaFlags([])).toEqual({}); + }); + + it("last write wins for a duplicate key in the same batch", () => { + expect(parseMetaFlags(["app=one", "app=two"])).toEqual({ app: "two" }); + }); + + it("rejects more than 24 pairs", () => { + const pairs = Array.from({ length: 25 }, (_, i) => `k${i}=v`); + expect(() => parseMetaFlags(pairs)).toThrow(UsageError); + }); + + it("fails fast on the first invalid pair", () => { + expect(() => parseMetaFlags(["ok=1", "Bad=2"])).toThrow(UsageError); + }); +}); diff --git a/skills/uploads-cli/SKILL.md b/skills/uploads-cli/SKILL.md index c8e5c6f..c9c6ef4 100644 --- a/skills/uploads-cli/SKILL.md +++ b/skills/uploads-cli/SKILL.md @@ -188,6 +188,26 @@ uploads put ./shot.png --format markdown # just the ![]()/ snippet uploads put ./shot.png --json # {workspace,key,url,size,markdown} ``` +## Custom metadata & search + +Every object can carry queryable key-value metadata (distinct from optimize/frame +provenance) — tag uploads at put time, then find them later: + +```bash +uploads put ./shot.png --meta app=myapp --meta page=settings --meta device=iphone-16 +uploads meta get screenshots/myapp/42/shot.png +uploads meta set screenshots/myapp/42/shot.png page=onboarding --delete device +uploads list --meta app=myapp --meta page=settings # ANDed, repeatable +uploads find app=myapp page=settings # same filter, positional pairs +``` + +Rules (validated client-side, fail-fast, before uploading): key +`^[a-z][a-z0-9._-]{0,63}$` (lowercase, dot-namespacing allowed, e.g. `gh.repo`); +value 1–512 printable ASCII characters; `--meta k=v` may repeat up to 24 times per +request; a value may itself contain `=` (only the first `=` splits key from value). +`content-sha256` is reserved (server-computed). `uploads attach` writes its own +`gh.*` reserved-by-convention keys automatically — see below. + ## Public media galleries Use galleries when several existing public uploads should be shared as one ordered collection. @@ -240,6 +260,12 @@ Then reference the URL in the PR/issue markdown you write with `gh`: Dashboard after ``` +`uploads attach` (below) additionally writes `gh.repo`/`gh.kind`/`gh.number`/`gh.ref` +as queryable metadata automatically, so `uploads find gh.ref=myorg/myapp#123` or +`uploads list --meta gh.repo=myorg/myapp` finds everything attached to that PR/issue +without needing the `gh/...` prefix. Add `--meta k=v` extras to `attach` for your own +pairs on top — a `--meta gh.*` override loses to the target's own `gh.*` values. + ### Option B — managed attachments comment (`--comment` / `comment`) Add `--comment` to upload **and** create/update a single marker-owned comment on the PR/issue. It keeps loose `gh/...` attachments and every public gallery linked to that PR/issue in clearly separate sections, with up to three available gallery images inline. It finds its own prior comment via a hidden marker and edits it in place — it never touches the description or other comments: @@ -277,7 +303,11 @@ comment --body-file` over inline HEREDOCs. ```bash uploads list --prefix screenshots/ # list objects (key + url) uploads list --pr 123 # everything attached to a PR +uploads list --meta app=myapp # filter by metadata (repeatable, ANDed) +uploads find app=myapp page=settings # same filter, human-friendly positional pairs uploads list --all --json # paginate fully, machine-readable +uploads meta get # show an object's metadata +uploads meta set k=v [k=v…] [--delete k]… # merge-set / delete metadata pairs uploads delete # remove an object uploads delete --dry-run # show what would be deleted uploads usage # storage / monthly upload counters (+ limits) From 181b79af18b0be0f0fedb8cf5493b7409c0fc603 Mon Sep 17 00:00:00 2001 From: Zach Dunn Date: Mon, 13 Jul 2026 18:46:41 -0400 Subject: [PATCH 12/23] fix(uploads): normalize gh.repo casing; restore flagString last-value-wins; mirror byte cap --- packages/uploads/src/cli-args.ts | 10 ++++++++- packages/uploads/src/github.ts | 14 ++++++++----- packages/uploads/src/index.ts | 1 + packages/uploads/src/metadata.ts | 21 +++++++++++++++++-- packages/uploads/test/cli-args.test.ts | 9 ++++++-- packages/uploads/test/commands-attach.test.ts | 17 +++++++++++++++ packages/uploads/test/metadata.test.ts | 12 +++++++++++ 7 files changed, 74 insertions(+), 10 deletions(-) diff --git a/packages/uploads/src/cli-args.ts b/packages/uploads/src/cli-args.ts index 6f5733c..4c3149a 100644 --- a/packages/uploads/src/cli-args.ts +++ b/packages/uploads/src/cli-args.ts @@ -168,9 +168,17 @@ export function parseCommandArgs(args: string[]): CommandFlags { return { positionals, flags, help }; } +/** + * Single string value for a flag. A repeated single-value flag keeps the + * pre-repeatable-flags behavior: the last occurrence wins (e.g. + * `--repo a --repo b` → `"b"`). Genuinely repeatable flags should use + * `flagValues` instead. + */ export function flagString(flags: CommandFlags["flags"], name: string): string | undefined { const value = flags.get(name); - return typeof value === "string" ? value : undefined; + if (typeof value === "string") return value; + if (Array.isArray(value) && value.length > 0) return value[value.length - 1]; + return undefined; } export function flagBool(flags: CommandFlags["flags"], name: string): boolean { diff --git a/packages/uploads/src/github.ts b/packages/uploads/src/github.ts index 696d3db..c695f36 100644 --- a/packages/uploads/src/github.ts +++ b/packages/uploads/src/github.ts @@ -89,16 +89,20 @@ export function ghAttachmentKey(target: GhTarget, filename: string): string { * The four `gh.*` queryable-metadata pairs `uploads attach` writes * automatically (`.context/2026-07-13-file-metadata-design.md`). `gh.kind` * uses the API's singular vocabulary (`pull`/`issue`), distinct from - * `GhTarget.kind`'s URL-segment spelling (`pull`/`issues`). `gh.ref` reuses - * the same lowercased `owner/repo#number` coordinate as gallery GitHub - * references, so both surfaces resolve the same lookup key. + * `GhTarget.kind`'s URL-segment spelling (`pull`/`issues`). `gh.repo` and + * `gh.ref` are both lowercased so exact-match metadata search has one + * canonical spelling regardless of source casing (`--repo`, git remote, and + * `gh` output vary); `gh.ref` uses the same lowercased `owner/repo#number` + * coordinate as gallery GitHub references, so both surfaces resolve the same + * lookup key. */ export function ghMetadataFromTarget(target: GhTarget): Record { + const repo = target.repo.toLowerCase(); return { - "gh.repo": target.repo, + "gh.repo": repo, "gh.kind": target.kind === "issues" ? "issue" : "pull", "gh.number": String(target.num), - "gh.ref": `${target.repo.toLowerCase()}#${target.num}`, + "gh.ref": `${repo}#${target.num}`, }; } diff --git a/packages/uploads/src/index.ts b/packages/uploads/src/index.ts index 15c5e14..3b03af8 100644 --- a/packages/uploads/src/index.ts +++ b/packages/uploads/src/index.ts @@ -72,6 +72,7 @@ export { META_KEY_RE, META_VALUE_MAX, META_MAX_KEYS, + META_MAX_TOTAL_BYTES, validateMetaEntry, parseMetaPair, parseMetaFlags, diff --git a/packages/uploads/src/metadata.ts b/packages/uploads/src/metadata.ts index eb3e52a..3bb9335 100644 --- a/packages/uploads/src/metadata.ts +++ b/packages/uploads/src/metadata.ts @@ -16,9 +16,14 @@ export const META_VALUE_MAX = 512; /** Cap on keys per request (mirrors META_MAX_KEYS server-side). */ export const META_MAX_KEYS = 24; +/** Cap on total UTF-8 key+value bytes per request (mirrors META_MAX_TOTAL_BYTES server-side). */ +export const META_MAX_TOTAL_BYTES = 8192; + // Printable ASCII only — same rule as the server's file-metadata.ts. const VALUE_SAFE_RE = /^[\x20-\x7E]+$/; +const encoder = new TextEncoder(); + /** * Server-computed keys the API rejects as custom metadata (currently just * `content-sha256`). `gh.*` is NOT reserved here: it's system-managed by @@ -60,8 +65,9 @@ export function parseMetaPair(raw: string): [string, string] { /** * Parse and validate a batch of `k=v` pairs (e.g. every `--meta` occurrence, * or `meta set`'s positional pairs) into a map. Fails fast on the first - * invalid pair or when the batch exceeds `META_MAX_KEYS`. Later duplicate - * keys in the same batch win (last write). + * invalid pair or when the batch exceeds `META_MAX_KEYS` or + * `META_MAX_TOTAL_BYTES`. Later duplicate keys in the same batch win + * (last write). */ export function parseMetaFlags(pairs: string[]): Record { if (pairs.length > META_MAX_KEYS) { @@ -72,5 +78,16 @@ export function parseMetaFlags(pairs: string[]): Record { const [key, value] = parseMetaPair(pair); result[key] = value; } + // Aggregate byte cap over the deduplicated map — same accounting as the + // server (sum of UTF-8 key+value bytes, META_MAX_TOTAL_BYTES). + let totalBytes = 0; + for (const [key, value] of Object.entries(result)) { + totalBytes += encoder.encode(key).byteLength + encoder.encode(value).byteLength; + } + if (totalBytes > META_MAX_TOTAL_BYTES) { + throw new UsageError( + `metadata too large: ${totalBytes} bytes of keys+values exceeds the ${META_MAX_TOTAL_BYTES}-byte limit per request`, + ); + } return result; } diff --git a/packages/uploads/test/cli-args.test.ts b/packages/uploads/test/cli-args.test.ts index fbdc72b..6aab4ec 100644 --- a/packages/uploads/test/cli-args.test.ts +++ b/packages/uploads/test/cli-args.test.ts @@ -13,9 +13,14 @@ describe("parseCommandArgs repeatable flags", () => { expect(flagValues(flags, "--meta")).toEqual(["app=x", "page=y"]); }); - it("flagString returns undefined once a flag has repeated (avoids silently using only the last value)", () => { + it("flagString keeps last-value-wins when a single-value flag is repeated", () => { + const { flags } = parseCommandArgs(["--repo", "a/b", "--repo", "c/d"]); + expect(flagString(flags, "--repo")).toBe("c/d"); + }); + + it("flagString returns the last of repeated occurrences for any flag", () => { const { flags } = parseCommandArgs(["--meta", "app=x", "--meta", "page=y"]); - expect(flagString(flags, "--meta")).toBeUndefined(); + expect(flagString(flags, "--meta")).toBe("page=y"); }); it("returns an empty array when the flag is absent", () => { diff --git a/packages/uploads/test/commands-attach.test.ts b/packages/uploads/test/commands-attach.test.ts index 898dafa..88bc6c7 100644 --- a/packages/uploads/test/commands-attach.test.ts +++ b/packages/uploads/test/commands-attach.test.ts @@ -142,6 +142,23 @@ describe("runAttach gh.* metadata", () => { }); }); + it("lowercases gh.repo and gh.ref for a mixed-case repo so metadata search stays exact-match", async () => { + const { client, metadataByKey } = fakeClient(); + // Mixed-case repo, as gh/--repo can return; key path keeps original case + // (ghAttachmentKey), but gh.* metadata is normalized to lowercase. + const run: CommandRunner = (_cmd, args) => { + if (args[0] === "repo") return "BuildInternet/Uploads\n"; + if (args[0] === "pr" && args[1] === "view") return "123\n"; + if (args[1]?.includes("per_page=100")) return "[]"; + return JSON.stringify({ id: 9 }); + }; + await runAttach(ctxWith(client), files("shot.png"), false, run); + expect(metadataByKey["gh/BuildInternet/Uploads/pull/123/shot.png"]).toMatchObject({ + "gh.repo": "buildinternet/uploads", + "gh.ref": "buildinternet/uploads#123", + }); + }); + it("merges --meta extras and lets the resolved target's gh.* win over a same-named extra", async () => { const { client, metadataByKey } = fakeClient(); const { run } = ghRunner(); diff --git a/packages/uploads/test/metadata.test.ts b/packages/uploads/test/metadata.test.ts index ef5d2c0..7b9e936 100644 --- a/packages/uploads/test/metadata.test.ts +++ b/packages/uploads/test/metadata.test.ts @@ -78,4 +78,16 @@ describe("parseMetaFlags", () => { it("fails fast on the first invalid pair", () => { expect(() => parseMetaFlags(["ok=1", "Bad=2"])).toThrow(UsageError); }); + + it("rejects a batch whose total key+value bytes exceed 8192", () => { + // 17 pairs x (k + 512-char value) ≈ 8.7 KB > 8192, each pair individually valid. + const pairs = Array.from({ length: 17 }, (_, i) => `k${i}=${"x".repeat(512)}`); + expect(() => parseMetaFlags(pairs)).toThrow(/8192-byte limit/); + }); + + it("accepts a batch just under the byte cap", () => { + // 15 pairs x ~514 bytes ≈ 7.7 KB < 8192. + const pairs = Array.from({ length: 15 }, (_, i) => `k${i}=${"x".repeat(510)}`); + expect(() => parseMetaFlags(pairs)).not.toThrow(); + }); }); From 9156a4659a5357d0857a717a2c638a43ad9b57ec Mon Sep 17 00:00:00 2001 From: Zach Dunn Date: Mon, 13 Jul 2026 19:02:24 -0400 Subject: [PATCH 13/23] feat(mcp): metadata on put; set_metadata/find_files tools MCP parity for per-file metadata (Task 8). Remote worker's put tool and the local stdio MCP's put/attach tools gain a metadata param; attach keeps auto-injecting gh.* via the shared command-layer helper. New set_metadata and find_files tools mirror the CLI's meta set / find commands. --- .changeset/file-metadata.md | 5 + apps/api/package.json | 1 + apps/mcp/src/tools.ts | 35 ++++- apps/mcp/test/mcp.test.ts | 91 ++++++++++- packages/uploads/src/mcp/args.ts | 15 ++ packages/uploads/src/mcp/server.ts | 2 +- packages/uploads/src/mcp/tools.ts | 100 +++++++++++- packages/uploads/src/metadata.ts | 11 ++ packages/uploads/test/mcp.test.ts | 207 ++++++++++++++++++++++++- packages/uploads/test/metadata.test.ts | 41 ++++- 10 files changed, 492 insertions(+), 16 deletions(-) diff --git a/.changeset/file-metadata.md b/.changeset/file-metadata.md index d19f823..d7007f7 100644 --- a/.changeset/file-metadata.md +++ b/.changeset/file-metadata.md @@ -6,3 +6,8 @@ Add queryable custom metadata to the CLI: `put --meta k=v` (repeatable), `attach now writes `gh.repo`/`gh.kind`/`gh.number`/`gh.ref` automatically (plus its own `--meta` extras), new `meta get`/`meta set` commands, `list --meta k=v` and the `find k=v...` alias for filtering objects by metadata. + +MCP parity: the local stdio MCP's `put`/`attach` tools gain a `metadata` param +(same gh.\* auto-injection as `attach`), and two new tools — `set_metadata` +(merge-set/delete) and `find_files` (metadata filter) — mirror the CLI's +`meta set`/`find`. The hosted MCP's `put` tool also gains a `metadata` param. diff --git a/apps/api/package.json b/apps/api/package.json index 5399c91..a025813 100644 --- a/apps/api/package.json +++ b/apps/api/package.json @@ -8,6 +8,7 @@ "./workspace": "./src/workspace.ts", "./storage": "./src/storage.ts", "./files": "./src/files-core.ts", + "./file-metadata": "./src/file-metadata.ts", "./guards": "./src/guards.ts", "./budget": "./src/budget.ts", "./usage": "./src/usage.ts", diff --git a/apps/mcp/src/tools.ts b/apps/mcp/src/tools.ts index 0ccd896..d09644e 100644 --- a/apps/mcp/src/tools.ts +++ b/apps/mcp/src/tools.ts @@ -6,8 +6,15 @@ * a filesystem or the gh CLI (attach, comment, doctor) stay stdio-only. */ import { buildMarkdown, buildScreenshotKey } from "@buildinternet/uploads"; -import { optPosInt, optString, usage, type McpTool } from "@buildinternet/uploads/mcp"; +import { + optPosInt, + optString, + optStringRecord, + usage, + type McpTool, +} from "@buildinternet/uploads/mcp"; import { badKey } from "@uploads/api/files"; +import { validateMetadataEntries } from "@uploads/api/file-metadata"; import { addExternalReference, addGalleryItem, @@ -296,6 +303,12 @@ export function createRemoteTools(ctx: RemoteToolContext): McpTool[] { type: "number", description: "Emit markdown instead of a plain image embed.", }, + metadata: { + type: "object", + additionalProperties: { type: "string" }, + description: + "Queryable custom metadata (key→value), separate from provenance. Omit to leave any metadata already stored for this key untouched; pass an object (even {}) to fully replace it. Keys: lowercase, ^[a-z][a-z0-9._-]{0,63}$. Values: 1-512 printable ASCII characters. Caps: at most 24 keys, at most 8192 total key+value bytes. Suggested keys: app, url, page, device, resolution, commit, branch. `gh.*` is reserved by convention for GitHub PR/issue attachment context (repo/kind/number/ref), normally system-managed by the attach flow.", + }, }, required: ["contentBase64", "filename"], additionalProperties: false, @@ -308,6 +321,15 @@ export function createRemoteTools(ctx: RemoteToolContext): McpTool[] { if (!contentBase64) usage("contentBase64 is required"); if (!filename) usage("filename is required"); + const metadata = optStringRecord(args, "metadata"); + if (metadata) { + try { + validateMetadataEntries(metadata); + } catch (err) { + usage(err instanceof Error ? err.message : String(err)); + } + } + const explicitKey = optString(args, "key"); const prefix = optString(args, "prefix"); const repo = optString(args, "repo"); @@ -332,7 +354,16 @@ export function createRemoteTools(ctx: RemoteToolContext): McpTool[] { // Key/body validation and the size/type guardrails live in putObject, // shared with the REST API — the stored content type is sniffed there. - const result = await putObject(env, workspace, key, bytes, workspaceName); + // metadata is undefined when omitted (leave existing D1 rows + // untouched); passing opts only when defined preserves that. + const result = await putObject( + env, + workspace, + key, + bytes, + workspaceName, + metadata !== undefined ? { metadata } : undefined, + ); const markdown = result.url === null ? undefined diff --git a/apps/mcp/test/mcp.test.ts b/apps/mcp/test/mcp.test.ts index eea8163..ae61d83 100644 --- a/apps/mcp/test/mcp.test.ts +++ b/apps/mcp/test/mcp.test.ts @@ -40,17 +40,24 @@ beforeAll(() => { */ async function makeEnv( options: { d1?: { tokenHash: string; scopes: string }; rateLimitOk?: boolean } = {}, -): Promise<{ env: Env; bucket: FakeR2Bucket }> { +): Promise<{ env: Env; bucket: FakeR2Bucket; metadata: Map> }> { const record: WorkspaceRecord = { ...workspace, tokenHash: await sha256Hex(TOKEN) }; const bucket = new FakeR2Bucket(); + // Keyed by `${workspace} ${objectKey}` -> ordered meta_key -> meta_value, + // real enough to exercise putObject's file_metadata read/write/delete path + // (see apps/api/test/routes-files.test.ts's makeFakeDB for the fuller version). + const metadata = new Map>(); + const scopeKey = (ws: string, objectKey: string) => `${ws} ${objectKey}`; const env = { REGISTRY: { get: async (key: string) => (key === "ws:test-ws" ? record : null), put: async () => undefined, }, DB: { - // run() no-op for workspace_usage metering after put/delete. - prepare: () => { + // run() no-ops for workspace_usage metering; file_metadata reads/writes + // are backed by the `metadata` map above. + prepare: (sql: string) => { + const normalized = sql.replace(/\s+/g, " ").trim(); let values: unknown[] = []; return { bind(...next: unknown[]) { @@ -75,8 +82,35 @@ async function makeEnv( return null; }, async run() { + if (normalized.startsWith("INSERT INTO file_metadata")) { + const [ws, objectKey, key, value] = values as [string, string, string, string]; + const map = metadata.get(scopeKey(ws, objectKey)) ?? new Map(); + map.set(key, value); + metadata.set(scopeKey(ws, objectKey), map); + } else if (normalized.includes("meta_key = ?") && normalized.startsWith("DELETE")) { + const [ws, objectKey, key] = values as [string, string, string]; + metadata.get(scopeKey(ws, objectKey))?.delete(key); + } else if (normalized.startsWith("DELETE FROM file_metadata")) { + const [ws, objectKey] = values as [string, string]; + metadata.delete(scopeKey(ws, objectKey)); + } return { success: true, meta: { changes: 0 }, results: [] }; }, + async all() { + if (normalized.startsWith("SELECT meta_key, meta_value FROM file_metadata")) { + const [ws, objectKey] = values as [string, string]; + const map = metadata.get(scopeKey(ws, objectKey)) ?? new Map(); + return { + success: true, + results: [...map.entries()].map(([meta_key, meta_value]) => ({ + meta_key, + meta_value, + })) as T[], + meta: {}, + }; + } + return { success: true, results: [] as T[], meta: {} }; + }, }; }, async batch(stmts: { run: () => Promise }[]) { @@ -88,7 +122,7 @@ async function makeEnv( ? {} : { WRITE_LIMITER: { limit: async () => ({ success: options.rateLimitOk }) } }), } as unknown as Env; - return { env, bucket }; + return { env, bucket, metadata }; } async function makeGalleryEnv(): Promise<{ env: Env; bucket: FakeR2Bucket }> { @@ -330,6 +364,55 @@ describe("mcp worker", () => { expect(bucket.store.get("shots/shot.png")?.contentType).toBe("image/png"); }); + it("writes custom metadata alongside the upload", async () => { + const { env, metadata } = await makeEnv(); + const result = await callTool(env, "put", { + contentBase64: PNG_B64, + filename: "shot.png", + key: "shots/tagged.png", + metadata: { app: "myapp", page: "settings" }, + }); + expect(result.isError).toBe(false); + expect(Object.fromEntries(metadata.get("test-ws shots/tagged.png") ?? [])).toEqual({ + app: "myapp", + page: "settings", + }); + }); + + it("leaves existing metadata untouched when the metadata argument is omitted", async () => { + const { env, metadata } = await makeEnv(); + await callTool(env, "put", { + contentBase64: PNG_B64, + filename: "shot.png", + key: "shots/tagged.png", + metadata: { app: "myapp" }, + }); + const result = await callTool(env, "put", { + contentBase64: PNG_B64, + filename: "shot.png", + key: "shots/tagged.png", + }); + expect(result.isError).toBe(false); + expect(Object.fromEntries(metadata.get("test-ws shots/tagged.png") ?? [])).toEqual({ + app: "myapp", + }); + }); + + it("rejects invalid metadata as a tool error before uploading", async () => { + const { env, bucket } = await makeEnv(); + const result = await callTool(env, "put", { + contentBase64: PNG_B64, + filename: "shot.png", + key: "shots/bad.png", + metadata: { "Bad-Key": "x" }, + }); + expect(result.isError).toBe(true); + expect(result.content).toEqual([ + { type: "text", text: "invalid metadata key: Bad-Key (USAGE)" }, + ]); + expect(bucket.store.size).toBe(0); + }); + it("computes the default screenshot key without git derivation", async () => { const { env, bucket } = await makeEnv(); const result = await callTool(env, "put", { diff --git a/packages/uploads/src/mcp/args.ts b/packages/uploads/src/mcp/args.ts index 8094a0e..225e292 100644 --- a/packages/uploads/src/mcp/args.ts +++ b/packages/uploads/src/mcp/args.ts @@ -26,3 +26,18 @@ export function optPosInt(args: ToolArgs, name: string): number | undefined { } return v; } + +/** A JSON-object argument of string→string pairs (e.g. a `metadata` or `filters` param). */ +export function optStringRecord(args: ToolArgs, name: string): Record | undefined { + const v = args[name]; + if (v === undefined || v === null) return undefined; + if (typeof v !== "object" || Array.isArray(v)) { + usage(`${name} must be an object of string values`); + } + const result: Record = {}; + for (const [key, value] of Object.entries(v as Record)) { + if (typeof value !== "string") usage(`${name}.${key} must be a string`); + result[key] = value; + } + return result; +} diff --git a/packages/uploads/src/mcp/server.ts b/packages/uploads/src/mcp/server.ts index 3189de0..74ee502 100644 --- a/packages/uploads/src/mcp/server.ts +++ b/packages/uploads/src/mcp/server.ts @@ -9,7 +9,7 @@ */ import { UploadsError } from "../errors.js"; -export { optPosInt, optString, usage, type ToolArgs } from "./args.js"; +export { optPosInt, optString, optStringRecord, usage, type ToolArgs } from "./args.js"; export interface McpTool { name: string; diff --git a/packages/uploads/src/mcp/tools.ts b/packages/uploads/src/mcp/tools.ts index 2451a95..a84197a 100644 --- a/packages/uploads/src/mcp/tools.ts +++ b/packages/uploads/src/mcp/tools.ts @@ -24,7 +24,8 @@ import { } from "../config.js"; import { buildMarkdown } from "../embed.js"; import { resolvePutPrefix } from "../destinations.js"; -import { ghAttachmentKey, ghKeyPrefix, type GhTarget } from "../github.js"; +import { ghAttachmentKey, ghKeyPrefix, ghMetadataFromTarget, type GhTarget } from "../github.js"; +import { validateMetaMap } from "../metadata.js"; import { rewriteKeyExtension, type OptimizeImageOptions } from "../optimize.js"; import { buildCliProvenance } from "../provenance.js"; import { @@ -33,9 +34,19 @@ import { resolveRepo, type CommandRunner, } from "../github-gh.js"; -import { optPosInt, optString, usage, type ToolArgs } from "./args.js"; +import { optPosInt, optString, optStringRecord, usage, type ToolArgs } from "./args.js"; import type { McpTool } from "./server.js"; +/** Shared tool-description text for the `metadata` object param on `put`/`attach`. */ +const METADATA_DESCRIPTION = + "Queryable custom metadata (key→value), separate from provenance. Omit to leave any metadata already stored for this key untouched; pass an object (even {}) to fully replace it. Keys: lowercase, ^[a-z][a-z0-9._-]{0,63}$. Values: 1-512 printable ASCII characters. Caps: at most 24 keys, at most 8192 total key+value bytes. Suggested keys: app, url, page, device, resolution, commit, branch. `gh.*` is reserved by convention for GitHub PR/issue attachment context (repo/kind/number/ref)."; + +const metadataProp = { + type: "object", + additionalProperties: { type: "string" }, + description: METADATA_DESCRIPTION, +}; + function optBool(args: ToolArgs, name: string): boolean { const v = args[name]; if (v === undefined || v === null) return false; @@ -400,6 +411,7 @@ export function createUploadsMcpTools(opts: { type: "boolean", description: "Resolve key + public URL without uploading. Not with comment.", }, + metadata: metadataProp, workspace: workspaceProp, }, additionalProperties: false, @@ -429,6 +441,11 @@ export function createUploadsMcpTools(opts: { if (refArg) usage("ref cannot be combined with pr/issue"); if (prefixArg) usage("prefix cannot be combined with pr/issue"); } + // Validate up front (fail fast, before reading/optimizing the file). + // undefined leaves existing metadata untouched; an object (even {}) + // fully replaces it — see metadataProp's description. + const metadata = optStringRecord(args, "metadata"); + if (metadata) validateMetaMap(metadata); let resolvedPrefix: string | undefined; try { resolvedPrefix = resolvePutPrefix({ @@ -475,6 +492,7 @@ export function createUploadsMcpTools(opts: { frameId: prepared.frame?.framed ? prepared.frame.frameId : undefined, keepExif: optimizeOpts.keepExif === true, }), + metadata, }); const markdown = buildMarkdown(result.url, { alt: optString(args, "alt") ?? sourceName, @@ -542,6 +560,12 @@ export function createUploadsMcpTools(opts: { "Keep EXIF/XMP/ICC when optimizing (default: strip for privacy on public embeds).", }, ...frameProps, + metadata: { + ...metadataProp, + description: + "Extra queryable metadata (key→value), merged with the automatic gh.repo/gh.kind/gh.number/gh.ref pairs — a gh.* pair here loses to the resolved target's own gh.* value. " + + METADATA_DESCRIPTION, + }, workspace: workspaceProp, }, required: ["files"], @@ -560,6 +584,12 @@ export function createUploadsMcpTools(opts: { const defaults = resolvePutDefaults({ envFile: globals.envFile }); const frameOpts = mcpFrameOptions(args); const optimizeOpts = mcpOptimizeOptions(args, defaults); + // User-supplied extras first, then the resolved target's gh.* — + // explicit target pairs always win over a same-named metadata extra + // (mirrors runAttach in ../commands.js). + const metaExtras = optStringRecord(args, "metadata") ?? {}; + if (Object.keys(metaExtras).length > 0) validateMetaMap(metaExtras); + const metadata = { ...metaExtras, ...ghMetadataFromTarget(target) }; const uploads = []; for (const file of files) { @@ -579,6 +609,7 @@ export function createUploadsMcpTools(opts: { frameId: prepared.frame?.framed ? prepared.frame.frameId : undefined, keepExif: optimizeOpts.keepExif === true, }), + metadata, }); uploads.push({ ...result, @@ -663,6 +694,71 @@ export function createUploadsMcpTools(opts: { return client.delete(key); }, }, + { + name: "set_metadata", + description: + "Merge-set and/or delete an object's queryable custom metadata (D1-backed key-value pairs; distinct from the R2 provenance headers put on upload). `set` pairs win over `delete` when a key appears in both. " + + METADATA_DESCRIPTION + + " Requires at least one of `set` or `delete`. Same as `uploads meta set`.", + inputSchema: { + type: "object", + properties: { + key: { type: "string", description: "Object key to update." }, + set: { ...metadataProp, description: "Keys to set/overwrite. " + METADATA_DESCRIPTION }, + delete: { + type: "array", + items: { type: "string" }, + description: "Keys to remove.", + }, + workspace: workspaceProp, + }, + required: ["key"], + additionalProperties: false, + }, + async handler(args) { + const key = optString(args, "key"); + if (!key) usage("key is required"); + const set = optStringRecord(args, "set"); + const del = optStringArray(args, "delete"); + if ((!set || Object.keys(set).length === 0) && (!del || del.length === 0)) { + usage("set_metadata requires set and/or delete"); + } + if (set) validateMetaMap(set); + const { client } = clientFor(args); + return client.patchMetadata(key, { set, delete: del }); + }, + }, + { + name: "find_files", + description: + "Find objects in the workspace whose queryable custom metadata matches ALL of `filters` (ANDed equality). Returns each match's key, public URL, and full metadata map. Same as `uploads find k=v...` / `uploads list --meta k=v`.", + inputSchema: { + type: "object", + properties: { + filters: { + ...metadataProp, + description: "Metadata equality filters (at least one pair). " + METADATA_DESCRIPTION, + }, + prefix: { type: "string", description: "Key prefix filter, combinable with filters." }, + limit: { type: "number", description: "Page size (default 50, max 500)." }, + workspace: workspaceProp, + }, + required: ["filters"], + additionalProperties: false, + }, + async handler(args) { + const filters = optStringRecord(args, "filters"); + if (!filters || Object.keys(filters).length === 0) { + usage("filters must have at least one key"); + } + validateMetaMap(filters); + const { client } = clientFor(args); + return client.findFiles(filters, { + prefix: optString(args, "prefix"), + limit: optPosInt(args, "limit"), + }); + }, + }, { name: "usage", description: diff --git a/packages/uploads/src/metadata.ts b/packages/uploads/src/metadata.ts index 3bb9335..ef54e59 100644 --- a/packages/uploads/src/metadata.ts +++ b/packages/uploads/src/metadata.ts @@ -91,3 +91,14 @@ export function parseMetaFlags(pairs: string[]): Record { } return result; } + +/** + * Validates a pre-parsed key→value metadata map (e.g. an MCP tool's object + * argument) against the same rules as {@link parseMetaFlags}, without + * requiring "k=v" string parsing first. Reuses `parseMetaFlags` by + * reconstructing "k=v" pairs — safe even when a value contains "=", since + * `parseMetaPair` only splits on the first occurrence. Throws `UsageError`. + */ +export function validateMetaMap(meta: Record): void { + parseMetaFlags(Object.entries(meta).map(([key, value]) => `${key}=${value}`)); +} diff --git a/packages/uploads/test/mcp.test.ts b/packages/uploads/test/mcp.test.ts index c2d2fed..1898873 100644 --- a/packages/uploads/test/mcp.test.ts +++ b/packages/uploads/test/mcp.test.ts @@ -17,9 +17,17 @@ import { createUploadsMcpTools } from "../src/mcp/tools.js"; /** Fake client factory capturing every resolved config and put()/delete() call. */ function fakeFactory() { - const puts: Array<{ key?: string; filename: string; contentType?: string }> = []; + const puts: Array<{ + key?: string; + filename: string; + contentType?: string; + metadata?: Record; + }> = []; const deletes: string[] = []; const configs: UploadsClientConfig[] = []; + // Keyed by object key, mirroring the server's per-key metadata rows well + // enough to exercise set_metadata/find_files wiring without a real API. + const metadataStore = new Map>(); const list = async ({ prefix }: { prefix?: string } = {}) => ({ items: puts .filter(({ key }) => (key ?? "").startsWith(prefix ?? "")) @@ -31,12 +39,23 @@ function fakeFactory() { return { put: async ( body: Uint8Array, - opts: { filename: string; key?: string; contentType?: string }, + opts: { + filename: string; + key?: string; + contentType?: string; + metadata?: Record; + }, ) => { // Record the effective key, so list()'s prefix filter (and key // assertions) see what a real client would have stored. const key = opts.key ?? "generated/key.png"; - puts.push({ key, filename: opts.filename, contentType: opts.contentType }); + puts.push({ + key, + filename: opts.filename, + contentType: opts.contentType, + metadata: opts.metadata, + }); + if (opts.metadata !== undefined) metadataStore.set(key, opts.metadata); return { workspace: config.workspace, key, @@ -57,9 +76,33 @@ function fakeFactory() { throw new Error("unexpected head"); }, health: async () => ({ ok: true }), + getMetadata: async (key: string) => ({ metadata: metadataStore.get(key) ?? {} }), + patchMetadata: async ( + key: string, + opts: { set?: Record; delete?: string[] }, + ) => { + const current = { ...metadataStore.get(key) }; + for (const k of opts.delete ?? []) delete current[k]; + Object.assign(current, opts.set ?? {}); + metadataStore.set(key, current); + return { metadata: current }; + }, + findFiles: async ( + filters: Record, + opts: { prefix?: string; limit?: number } = {}, + ) => { + const items = [...metadataStore.entries()] + .filter(([key, meta]) => { + if (opts.prefix && !key.startsWith(opts.prefix)) return false; + return Object.entries(filters).every(([k, v]) => meta[k] === v); + }) + .slice(0, opts.limit ?? 50) + .map(([key, meta]) => ({ key, url: `https://x.test/${key}`, metadata: meta })); + return { items, cursor: null }; + }, } as unknown as UploadsClient; }; - return { factory, puts, deletes, configs }; + return { factory, puts, deletes, configs, metadataStore }; } /** In-memory gallery API contract used to exercise the MCP mutation workflow. */ @@ -151,7 +194,7 @@ function serverWith(overrides?: { runner?: CommandRunner; factory?: (config: UploadsClientConfig) => UploadsClient; }) { - const { factory, puts, deletes, configs } = fakeFactory(); + const { factory, puts, deletes, configs, metadataStore } = fakeFactory(); const server = createMcpServer({ serverInfo: { name: "uploads", version: "0.0.0-test" }, tools: createUploadsMcpTools({ @@ -160,7 +203,7 @@ function serverWith(overrides?: { clientFactory: overrides?.factory ?? factory, }), }); - return { server, puts, deletes, configs }; + return { server, puts, deletes, configs, metadataStore }; } async function rpc( @@ -275,6 +318,8 @@ describe("tools/list", () => { "attach", "list", "delete", + "set_metadata", + "find_files", "usage", "reconcile", "purge_expired", @@ -416,6 +461,46 @@ describe("tools/call put", () => { expect(res.result.isError).toBe(true); expect(res.result.content[0].text).toContain("file or contentBase64"); }); + + it("passes custom metadata through to the client", async () => { + const { server, puts } = serverWith(); + const res = await rpc(server, "tools/call", { + name: "put", + arguments: { + contentBase64: PNG_B64, + filename: "shot.png", + key: "tagged/shot.png", + metadata: { app: "myapp", page: "settings" }, + }, + }); + expect(res.result.isError).toBe(false); + expect(puts[0].metadata).toEqual({ app: "myapp", page: "settings" }); + }); + + it("leaves metadata undefined (untouched) when the argument is omitted", async () => { + const { server, puts } = serverWith(); + const res = await rpc(server, "tools/call", { + name: "put", + arguments: { contentBase64: PNG_B64, filename: "shot.png", key: "plain/shot.png" }, + }); + expect(res.result.isError).toBe(false); + expect(puts[0].metadata).toBeUndefined(); + }); + + it("rejects an invalid metadata key as a tool error", async () => { + const { server } = serverWith(); + const res = await rpc(server, "tools/call", { + name: "put", + arguments: { + contentBase64: PNG_B64, + filename: "shot.png", + key: "bad/shot.png", + metadata: { "Bad-Key": "x" }, + }, + }); + expect(res.result.isError).toBe(true); + expect(res.result.content[0].text).toContain("invalid metadata key"); + }); }); describe("tools/call attach", () => { @@ -446,6 +531,40 @@ describe("tools/call attach", () => { expect(res.result.isError).toBe(true); expect(res.result.content[0].text).toContain("files"); }); + + it("auto-injects gh.* metadata, merged with user extras", async () => { + const { run } = ghRunner(); + const { server, puts } = serverWith({ runner: run }); + const dir = mkdtempSync(join(tmpdir(), "uploads-mcp-test-")); + const file = join(dir, "before.png"); + writeFileSync(file, "png"); + + await rpc(server, "tools/call", { + name: "attach", + arguments: { files: [file], metadata: { app: "myapp" } }, + }); + expect(puts[0].metadata).toEqual({ + app: "myapp", + "gh.repo": "buildinternet/uploads", + "gh.kind": "pull", + "gh.number": "123", + "gh.ref": "buildinternet/uploads#123", + }); + }); + + it("a gh.* metadata extra loses to the resolved target's own value", async () => { + const { run } = ghRunner(); + const { server, puts } = serverWith({ runner: run }); + const dir = mkdtempSync(join(tmpdir(), "uploads-mcp-test-")); + const file = join(dir, "before.png"); + writeFileSync(file, "png"); + + await rpc(server, "tools/call", { + name: "attach", + arguments: { files: [file], metadata: { "gh.repo": "someone/else" } }, + }); + expect(puts[0].metadata?.["gh.repo"]).toBe("buildinternet/uploads"); + }); }); describe("tools/call list, delete, comment", () => { @@ -517,6 +636,82 @@ describe("tools/call list, delete, comment", () => { }); }); +describe("tools/call set_metadata, find_files", () => { + it("sets and deletes metadata, returning the merged map", async () => { + const { server, metadataStore } = serverWith(); + metadataStore.set("shots/a.png", { app: "myapp", page: "old" }); + + const res = await rpc(server, "tools/call", { + name: "set_metadata", + arguments: { + key: "shots/a.png", + set: { page: "settings" }, + delete: ["app"], + }, + }); + expect(res.result.isError).toBe(false); + expect(res.result.structuredContent).toEqual({ metadata: { page: "settings" } }); + }); + + it("set wins when a key is both set and deleted", async () => { + const { server } = serverWith(); + const res = await rpc(server, "tools/call", { + name: "set_metadata", + arguments: { key: "shots/a.png", set: { app: "myapp" }, delete: ["app"] }, + }); + expect(res.result.structuredContent).toEqual({ metadata: { app: "myapp" } }); + }); + + it("requires at least one of set or delete", async () => { + const { server } = serverWith(); + const res = await rpc(server, "tools/call", { + name: "set_metadata", + arguments: { key: "shots/a.png" }, + }); + expect(res.result.isError).toBe(true); + expect(res.result.content[0].text).toContain("set and/or delete"); + }); + + it("rejects an invalid set key as a tool error", async () => { + const { server } = serverWith(); + const res = await rpc(server, "tools/call", { + name: "set_metadata", + arguments: { key: "shots/a.png", set: { "Bad-Key": "x" } }, + }); + expect(res.result.isError).toBe(true); + expect(res.result.content[0].text).toContain("invalid metadata key"); + }); + + it("finds objects matching ANDed metadata filters", async () => { + const { server, metadataStore } = serverWith(); + metadataStore.set("shots/a.png", { app: "myapp", page: "settings" }); + metadataStore.set("shots/b.png", { app: "myapp", page: "home" }); + + const res = await rpc(server, "tools/call", { + name: "find_files", + arguments: { filters: { app: "myapp", page: "settings" } }, + }); + expect(res.result.isError).toBe(false); + expect(res.result.structuredContent).toEqual({ + items: [ + { + key: "shots/a.png", + url: "https://x.test/shots/a.png", + metadata: { app: "myapp", page: "settings" }, + }, + ], + cursor: null, + }); + }); + + it("requires at least one filter", async () => { + const { server } = serverWith(); + const res = await rpc(server, "tools/call", { name: "find_files", arguments: { filters: {} } }); + expect(res.result.isError).toBe(true); + expect(res.result.content[0].text).toContain("filters"); + }); +}); + describe("config resolution", () => { it("surfaces a missing token as a tool error, not a server failure", async () => { vi.stubEnv("UPLOADS_TOKEN", ""); diff --git a/packages/uploads/test/metadata.test.ts b/packages/uploads/test/metadata.test.ts index 7b9e936..9f045e7 100644 --- a/packages/uploads/test/metadata.test.ts +++ b/packages/uploads/test/metadata.test.ts @@ -1,6 +1,11 @@ import { describe, expect, it } from "vitest"; import { UsageError } from "../src/cli-args.js"; -import { parseMetaFlags, parseMetaPair, validateMetaEntry } from "../src/metadata.js"; +import { + parseMetaFlags, + parseMetaPair, + validateMetaEntry, + validateMetaMap, +} from "../src/metadata.js"; describe("parseMetaPair", () => { it("splits on the first '=' only", () => { @@ -91,3 +96,37 @@ describe("parseMetaFlags", () => { expect(() => parseMetaFlags(pairs)).not.toThrow(); }); }); + +describe("validateMetaMap", () => { + it("does not throw for a valid map", () => { + expect(() => validateMetaMap({ app: "myapp", page: "settings" })).not.toThrow(); + }); + + it("accepts an empty map", () => { + expect(() => validateMetaMap({})).not.toThrow(); + }); + + it("rejects an invalid key", () => { + expect(() => validateMetaMap({ "Bad-Key": "x" })).toThrow(UsageError); + }); + + it("rejects the reserved content-sha256 key", () => { + expect(() => validateMetaMap({ "content-sha256": "abc" })).toThrow(UsageError); + }); + + it("preserves a value containing '=' (no re-split corruption)", () => { + expect(() => validateMetaMap({ url: "https://example.com/a?b=c" })).not.toThrow(); + }); + + it("rejects more than 24 keys", () => { + const meta = Object.fromEntries(Array.from({ length: 25 }, (_, i) => [`k${i}`, "v"])); + expect(() => validateMetaMap(meta)).toThrow(UsageError); + }); + + it("rejects a map whose total key+value bytes exceed 8192", () => { + const meta = Object.fromEntries( + Array.from({ length: 17 }, (_, i) => [`k${i}`, "x".repeat(512)]), + ); + expect(() => validateMetaMap(meta)).toThrow(/8192-byte limit/); + }); +}); From be2914f3f4ae5626f1280f8c326c9dd88f0341c3 Mon Sep 17 00:00:00 2001 From: Zach Dunn Date: Mon, 13 Jul 2026 19:13:31 -0400 Subject: [PATCH 14/23] feat(api): backfill gh attachment metadata script --- apps/api/scripts/backfill-gh-metadata.d.mts | 43 +++++ apps/api/scripts/backfill-gh-metadata.mjs | 187 ++++++++++++++++++++ apps/api/test/backfill-gh-metadata.test.ts | 155 ++++++++++++++++ docs/ops.md | 17 ++ 4 files changed, 402 insertions(+) create mode 100644 apps/api/scripts/backfill-gh-metadata.d.mts create mode 100644 apps/api/scripts/backfill-gh-metadata.mjs create mode 100644 apps/api/test/backfill-gh-metadata.test.ts diff --git a/apps/api/scripts/backfill-gh-metadata.d.mts b/apps/api/scripts/backfill-gh-metadata.d.mts new file mode 100644 index 0000000..0bfbb94 --- /dev/null +++ b/apps/api/scripts/backfill-gh-metadata.d.mts @@ -0,0 +1,43 @@ +/** Type declarations for backfill-gh-metadata.mjs, consumed by test/backfill-gh-metadata.test.ts. */ + +export const GH_KEY_RE: RegExp; + +export interface GhMetadataPlan { + key: string; + metadata: { + "gh.repo": string; + "gh.kind": "pull" | "issue"; + "gh.number": string; + "gh.ref": string; + }; +} + +export function planForKey(key: string): GhMetadataPlan | null; + +export interface RunBackfillSummary { + matched: number; + patched: number; + skipped: number; + errors: number; +} + +/** Minimal fetch-Response shape the loop consumes — real `fetch` and test stubs both satisfy it. */ +export interface FetchLikeResponse { + ok: boolean; + status: number; + json(): Promise; +} + +export type FetchLike = (url: string, init?: Record) => Promise; + +export interface RunBackfillOptions { + apiUrl: string; + workspace: string; + token: string; + dryRun?: boolean; + prefix?: string; + fetchImpl?: FetchLike; + log?: (message: string) => void; +} + +export function runBackfill(options: RunBackfillOptions): Promise; diff --git a/apps/api/scripts/backfill-gh-metadata.mjs b/apps/api/scripts/backfill-gh-metadata.mjs new file mode 100644 index 0000000..b93435e --- /dev/null +++ b/apps/api/scripts/backfill-gh-metadata.mjs @@ -0,0 +1,187 @@ +#!/usr/bin/env node +/** + * One-time backfill: derive gh.* metadata for objects uploaded under gh/... + * before per-file metadata existed, so they become searchable the same way + * `uploads attach` writes them going forward (see ghMetadataFromTarget in + * packages/uploads/src/github.ts — this script mirrors that mapping). + * + * Pages GET /v1/:ws/files?prefix=gh/ (cursor loop), parses each key against + * GH_KEY_RE, and PATCHes /v1/:ws/files//metadata with `{ set: {...} }` + * for every match. Non-matching keys under gh/ are skipped and counted, not + * treated as errors. Merge-semantics PATCH means re-running is harmless + * (idempotent) — safe to re-run after interruption. + * + * BINDING: gh.* metadata values are canonical lowercase (gh.repo, gh.ref); + * the object key itself keeps its original casing untouched. gh.kind is + * "issue" for keys under gh/.../issues/... and "pull" for gh/.../pull/.... + * + * Usage (from apps/api, with UPLOADS_API_URL / UPLOADS_WORKSPACE / + * UPLOADS_TOKEN in the monorepo-root .env, same names as .env.example): + * node --env-file=../../.env scripts/backfill-gh-metadata.mjs --dry-run + * node --env-file=../../.env scripts/backfill-gh-metadata.mjs + * node --env-file=../../.env scripts/backfill-gh-metadata.mjs --workspace other-ws + * + * Never point this at a production workspace during testing — use a local + * `wrangler dev` stack (UPLOADS_API_URL=http://localhost:8787) first. + */ +import { pathToFileURL } from "node:url"; + +/** Anchored gh///// prefix — see task brief. */ +export const GH_KEY_RE = /^gh\/([^/]+)\/([^/]+)\/(pull|issues)\/(\d+)\//; + +/** + * Pure key-parsing + plan-building logic, no I/O. Returns the PATCH plan for + * a matching key, or null when the key doesn't match the gh/ layout (caller + * counts those as skipped, not errors). + */ +export function planForKey(key) { + const m = GH_KEY_RE.exec(key); + if (!m) return null; + const [, ownerRaw, repoRaw, kindSeg, number] = m; + const repo = `${ownerRaw}/${repoRaw}`.toLowerCase(); + const kind = kindSeg === "issues" ? "issue" : "pull"; + return { + key, + metadata: { + "gh.repo": repo, + "gh.kind": kind, + "gh.number": number, + "gh.ref": `${repo}#${number}`, + }, + }; +} + +/** + * Orchestrates the list→plan→PATCH loop. `fetchImpl` is injectable for + * tests; production callers pass the global `fetch`. + * + * @returns {Promise<{matched: number, patched: number, skipped: number, errors: number}>} + */ +export async function runBackfill({ + apiUrl, + workspace, + token, + dryRun = false, + prefix = "gh/", + fetchImpl = fetch, + log = console.log, +}) { + const base = apiUrl.replace(/\/$/, ""); + const counts = { matched: 0, patched: 0, skipped: 0, errors: 0 }; + let cursor; + + for (;;) { + const listUrl = new URL(`${base}/v1/${workspace}/files`); + listUrl.searchParams.set("prefix", prefix); + if (cursor) listUrl.searchParams.set("cursor", cursor); + + const listRes = await fetchImpl(listUrl.toString(), { + headers: { Authorization: `Bearer ${token}` }, + }); + if (!listRes.ok) { + counts.errors += 1; + log(`error: list failed (HTTP ${listRes.status})`); + break; + } + const page = await listRes.json(); + const items = Array.isArray(page.items) ? page.items : []; + + for (const item of items) { + const plan = planForKey(item.key); + if (!plan) { + counts.skipped += 1; + log(`skip ${item.key} (does not match gh/ layout)`); + continue; + } + counts.matched += 1; + + if (dryRun) { + log(`[dry-run] would PATCH ${plan.key} set=${JSON.stringify(plan.metadata)}`); + continue; + } + + const patchUrl = `${base}/v1/${workspace}/files/${plan.key}/metadata`; + const patchRes = await fetchImpl(patchUrl, { + method: "PATCH", + headers: { + Authorization: `Bearer ${token}`, + "Content-Type": "application/json", + }, + body: JSON.stringify({ set: plan.metadata }), + }); + if (!patchRes.ok) { + counts.errors += 1; + log(`error: PATCH ${plan.key} failed (HTTP ${patchRes.status})`); + continue; + } + counts.patched += 1; + log(`patch ${plan.key} set=${JSON.stringify(plan.metadata)}`); + } + + cursor = page.cursor; + if (!cursor) break; + } + + return counts; +} + +function parseArgs(argv) { + const opts = { dryRun: false, workspace: undefined }; + for (let i = 0; i < argv.length; i++) { + const arg = argv[i]; + if (arg === "--dry-run") { + opts.dryRun = true; + } else if (arg === "--workspace") { + opts.workspace = argv[++i]; + } else { + throw new Error(`unexpected argument: ${arg}`); + } + } + return opts; +} + +async function main() { + let opts; + try { + opts = parseArgs(process.argv.slice(2)); + } catch (err) { + console.error(`error: ${err instanceof Error ? err.message : String(err)}`); + process.exit(1); + } + const apiUrl = process.env.UPLOADS_API_URL ?? "https://api.uploads.sh"; + const workspace = opts.workspace ?? process.env.UPLOADS_WORKSPACE; + const token = process.env.UPLOADS_TOKEN; + + if (!workspace) { + console.error("error: UPLOADS_WORKSPACE (or --workspace) is required"); + process.exit(1); + } + if (!token) { + console.error("error: UPLOADS_TOKEN is required"); + process.exit(1); + } + + console.log( + `backfilling gh.* metadata for ${apiUrl} workspace=${workspace}${opts.dryRun ? " (dry-run)" : ""}`, + ); + + const summary = await runBackfill({ apiUrl, workspace, token, dryRun: opts.dryRun }); + + console.log( + `\ndone: matched=${summary.matched} patched=${summary.patched} skipped=${summary.skipped} errors=${summary.errors}`, + ); + process.exit(summary.errors > 0 ? 1 : 0); +} + +// Only run when executed directly (`node backfill-gh-metadata.mjs`), not when +// imported for its pure functions (test/backfill-gh-metadata.test.ts). +const isMain = (() => { + try { + return Boolean(process.argv[1]) && import.meta.url === pathToFileURL(process.argv[1]).href; + } catch { + return false; + } +})(); +if (isMain) { + main(); +} diff --git a/apps/api/test/backfill-gh-metadata.test.ts b/apps/api/test/backfill-gh-metadata.test.ts new file mode 100644 index 0000000..67c3607 --- /dev/null +++ b/apps/api/test/backfill-gh-metadata.test.ts @@ -0,0 +1,155 @@ +import { describe, expect, it, vi } from "vitest"; +import { GH_KEY_RE, planForKey, runBackfill } from "../scripts/backfill-gh-metadata.mjs"; + +describe("planForKey", () => { + it("matches a pull-request key and lowercases metadata values only", () => { + const plan = planForKey("gh/OWNER/REPO/pull/12/screenshot.png"); + expect(plan).toEqual({ + key: "gh/OWNER/REPO/pull/12/screenshot.png", + metadata: { + "gh.repo": "owner/repo", + "gh.kind": "pull", + "gh.number": "12", + "gh.ref": "owner/repo#12", + }, + }); + }); + + it("maps the 'issues' path segment to gh.kind 'issue'", () => { + const plan = planForKey("gh/foo/bar/issues/7/img.png"); + expect(plan).toEqual({ + key: "gh/foo/bar/issues/7/img.png", + metadata: { + "gh.repo": "foo/bar", + "gh.kind": "issue", + "gh.number": "7", + "gh.ref": "foo/bar#7", + }, + }); + }); + + it("returns null for keys that don't match the gh/ layout", () => { + for (const key of [ + "gh/onlytwo/segments", + "gh/foo/bar/issues/abc/img.png", + "gh/foo/bar/merge/12/img.png", + "screenshots/shot.png", + "gh/foo/bar/pull//img.png", + ]) { + expect(planForKey(key)).toBeNull(); + } + }); + + it("keeps the object key's original casing", () => { + const plan = planForKey("gh/MixedCase/Repo/pull/3/x.png"); + expect(plan?.key).toBe("gh/MixedCase/Repo/pull/3/x.png"); + expect(plan?.metadata["gh.repo"]).toBe("mixedcase/repo"); + }); +}); + +describe("GH_KEY_RE", () => { + it("matches the anchored gh///// prefix", () => { + expect(GH_KEY_RE.test("gh/o/r/pull/1/x")).toBe(true); + expect(GH_KEY_RE.test("gh/o/r/pull/1/")).toBe(true); + expect(GH_KEY_RE.test("other/gh/o/r/pull/1/x")).toBe(false); + }); +}); + +function jsonResponse(body: unknown, status = 200) { + return { + ok: status >= 200 && status < 300, + status, + json: async () => body, + text: async () => JSON.stringify(body), + }; +} + +describe("runBackfill", () => { + const baseOpts = { + apiUrl: "https://api.example.test", + workspace: "default", + token: "tok_123", + }; + + it("paginates the list endpoint and PATCHes each matching key", async () => { + const calls: { url: string; init?: Record }[] = []; + const fetchImpl = vi.fn(async (url: string, init?: Record) => { + calls.push({ url, init }); + if (url.includes("/files?")) { + if (url.includes("cursor=page2")) { + return jsonResponse({ + items: [{ key: "gh/foo/bar/issues/7/img.png" }], + cursor: null, + }); + } + return jsonResponse({ + items: [{ key: "gh/owner/repo/pull/12/a.png" }, { key: "not-gh/whatever" }], + cursor: "page2", + }); + } + if (url.includes("/metadata")) { + return jsonResponse({ metadata: {} }); + } + throw new Error(`unexpected fetch: ${url}`); + }); + + const summary = await runBackfill({ ...baseOpts, fetchImpl }); + + expect(summary).toEqual({ matched: 2, patched: 2, skipped: 1, errors: 0 }); + + const listCalls = calls.filter((c) => c.url.includes("/files?")); + expect(listCalls).toHaveLength(2); + expect(listCalls[1].url).toContain("cursor=page2"); + + const patchCalls = calls.filter((c) => c.url.includes("/metadata")); + expect(patchCalls).toHaveLength(2); + expect(patchCalls[0].url).toContain("gh/owner/repo/pull/12/a.png/metadata"); + expect(patchCalls[0].init?.method).toBe("PATCH"); + expect(JSON.parse(patchCalls[0].init?.body as string)).toEqual({ + set: { + "gh.repo": "owner/repo", + "gh.kind": "pull", + "gh.number": "12", + "gh.ref": "owner/repo#12", + }, + }); + const patchHeaders = patchCalls[0].init?.headers as Record | undefined; + expect(patchHeaders?.Authorization).toBe("Bearer tok_123"); + }); + + it("dry-run performs no PATCH requests", async () => { + const fetchImpl = vi.fn(async (url: string) => { + if (url.includes("/files?")) { + return jsonResponse({ items: [{ key: "gh/owner/repo/pull/12/a.png" }], cursor: null }); + } + throw new Error(`unexpected fetch in dry-run: ${url}`); + }); + + const summary = await runBackfill({ ...baseOpts, dryRun: true, fetchImpl }); + + expect(summary).toEqual({ matched: 1, patched: 0, skipped: 0, errors: 0 }); + expect(fetchImpl).toHaveBeenCalledTimes(1); + }); + + it("counts a non-2xx PATCH as an error and continues to the next item", async () => { + const fetchImpl = vi.fn(async (url: string) => { + if (url.includes("/files?")) { + return jsonResponse({ + items: [{ key: "gh/owner/repo/pull/12/a.png" }, { key: "gh/owner/repo/pull/13/b.png" }], + cursor: null, + }); + } + if (url.includes("/pull/12/a.png/metadata")) { + return jsonResponse({ error: "boom" }, 500); + } + if (url.includes("/pull/13/b.png/metadata")) { + return jsonResponse({ metadata: {} }); + } + throw new Error(`unexpected fetch: ${url}`); + }); + + const summary = await runBackfill({ ...baseOpts, fetchImpl }); + + expect(summary).toEqual({ matched: 2, patched: 1, skipped: 0, errors: 1 }); + }); +}); diff --git a/docs/ops.md b/docs/ops.md index 6f6f7c3..53480d9 100644 --- a/docs/ops.md +++ b/docs/ops.md @@ -39,6 +39,23 @@ uploads purge-expired # needs retentionDays The API worker also runs a **daily cron** (`0 6 * * *` UTC) that purges every workspace with `retentionDays` set. Logs: `retention_sweep` JSON. +## Backfill gh metadata + +One-time script for objects uploaded under `gh/...` before per-file metadata +existed — derives `gh.repo` / `gh.kind` / `gh.number` / `gh.ref` from each key +and PATCHes it in, matching what `uploads attach` now writes going forward. +Idempotent (safe to re-run) and paginates the whole `gh/` prefix itself. + +```bash +node --env-file=.env apps/api/scripts/backfill-gh-metadata.mjs --dry-run +node --env-file=.env apps/api/scripts/backfill-gh-metadata.mjs +``` + +`UPLOADS_API_URL` / `UPLOADS_WORKSPACE` / `UPLOADS_TOKEN` come from `.env` +(same names as `.env.example`); `--workspace ` overrides the workspace +for one run. Test against a local `wrangler dev` stack first — never point +this at production while testing. + ## Invitations Invite people through the session-authenticated **admin UI at `/admin`**, signed in From cd413f4a1146ec9cabfdab4dadedd674225985a3 Mon Sep 17 00:00:00 2001 From: Zach Dunn Date: Mon, 13 Jul 2026 19:20:03 -0400 Subject: [PATCH 15/23] fix(api): handle transport errors in gh metadata backfill --- apps/api/scripts/backfill-gh-metadata.mjs | 51 +++++++++++++++++----- apps/api/test/backfill-gh-metadata.test.ts | 35 +++++++++++++++ 2 files changed, 74 insertions(+), 12 deletions(-) diff --git a/apps/api/scripts/backfill-gh-metadata.mjs b/apps/api/scripts/backfill-gh-metadata.mjs index b93435e..769cf08 100644 --- a/apps/api/scripts/backfill-gh-metadata.mjs +++ b/apps/api/scripts/backfill-gh-metadata.mjs @@ -75,9 +75,19 @@ export async function runBackfill({ listUrl.searchParams.set("prefix", prefix); if (cursor) listUrl.searchParams.set("cursor", cursor); - const listRes = await fetchImpl(listUrl.toString(), { - headers: { Authorization: `Bearer ${token}` }, - }); + // A failed list call (non-2xx or transport error) aborts the loop — + // without the page we can't discover further keys. A failed PATCH below + // only skips that one key. Both count as errors (non-zero exit). + let listRes; + try { + listRes = await fetchImpl(listUrl.toString(), { + headers: { Authorization: `Bearer ${token}` }, + }); + } catch (err) { + counts.errors += 1; + log(`error: list failed (${err instanceof Error ? err.message : String(err)})`); + break; + } if (!listRes.ok) { counts.errors += 1; log(`error: list failed (HTTP ${listRes.status})`); @@ -101,14 +111,23 @@ export async function runBackfill({ } const patchUrl = `${base}/v1/${workspace}/files/${plan.key}/metadata`; - const patchRes = await fetchImpl(patchUrl, { - method: "PATCH", - headers: { - Authorization: `Bearer ${token}`, - "Content-Type": "application/json", - }, - body: JSON.stringify({ set: plan.metadata }), - }); + let patchRes; + try { + patchRes = await fetchImpl(patchUrl, { + method: "PATCH", + headers: { + Authorization: `Bearer ${token}`, + "Content-Type": "application/json", + }, + body: JSON.stringify({ set: plan.metadata }), + }); + } catch (err) { + counts.errors += 1; + log( + `error: PATCH ${plan.key} failed (${err instanceof Error ? err.message : String(err)})`, + ); + continue; + } if (!patchRes.ok) { counts.errors += 1; log(`error: PATCH ${plan.key} failed (HTTP ${patchRes.status})`); @@ -165,7 +184,15 @@ async function main() { `backfilling gh.* metadata for ${apiUrl} workspace=${workspace}${opts.dryRun ? " (dry-run)" : ""}`, ); - const summary = await runBackfill({ apiUrl, workspace, token, dryRun: opts.dryRun }); + // runBackfill handles fetch failures itself; this catch is the last line of + // defense (e.g. a malformed list body) — exit non-zero, no raw stack dump. + let summary; + try { + summary = await runBackfill({ apiUrl, workspace, token, dryRun: opts.dryRun }); + } catch (err) { + console.error(`error: backfill failed (${err instanceof Error ? err.message : String(err)})`); + process.exit(1); + } console.log( `\ndone: matched=${summary.matched} patched=${summary.patched} skipped=${summary.skipped} errors=${summary.errors}`, diff --git a/apps/api/test/backfill-gh-metadata.test.ts b/apps/api/test/backfill-gh-metadata.test.ts index 67c3607..c72b849 100644 --- a/apps/api/test/backfill-gh-metadata.test.ts +++ b/apps/api/test/backfill-gh-metadata.test.ts @@ -152,4 +152,39 @@ describe("runBackfill", () => { expect(summary).toEqual({ matched: 2, patched: 1, skipped: 0, errors: 1 }); }); + + it("counts a thrown PATCH fetch (transport error) as an error and continues", async () => { + const fetchImpl = vi.fn(async (url: string) => { + if (url.includes("/files?")) { + return jsonResponse({ + items: [{ key: "gh/owner/repo/pull/12/a.png" }, { key: "gh/owner/repo/pull/13/b.png" }], + cursor: null, + }); + } + if (url.includes("/pull/12/a.png/metadata")) { + throw new TypeError("fetch failed: connection reset"); + } + if (url.includes("/pull/13/b.png/metadata")) { + return jsonResponse({ metadata: {} }); + } + throw new Error(`unexpected fetch: ${url}`); + }); + + const summary = await runBackfill({ ...baseOpts, fetchImpl }); + + expect(summary).toEqual({ matched: 2, patched: 1, skipped: 0, errors: 1 }); + // Both PATCHes were attempted despite the first one throwing. + expect(fetchImpl.mock.calls.filter(([url]) => url.includes("/metadata"))).toHaveLength(2); + }); + + it("aborts cleanly with an error counted when the list fetch throws (transport error)", async () => { + const fetchImpl = vi.fn(async () => { + throw new TypeError("fetch failed: getaddrinfo ENOTFOUND"); + }); + + const summary = await runBackfill({ ...baseOpts, fetchImpl }); + + expect(summary).toEqual({ matched: 0, patched: 0, skipped: 0, errors: 1 }); + expect(fetchImpl).toHaveBeenCalledTimes(1); + }); }); From 1084aa23036d54d185c11ef6598a203d5e9faa03 Mon Sep 17 00:00:00 2001 From: Zach Dunn Date: Mon, 13 Jul 2026 19:33:28 -0400 Subject: [PATCH 16/23] fix(api,errors): metadata final-review fixes (escape LIKE, PUT semantics, filter cap, reserved keys) Consolidated fix wave ahead of the per-file metadata PR: escape SQL LIKE wildcards in findObjectsByMetadata's prefix so `_`/`%` in a prefix match literally; make the REST PUT route pass metadata: undefined (preserve) instead of always full-replacing when a re-PUT carries no custom X-Uploads-Meta-* headers; cap GET list's meta.* filter params at META_MAX_KEYS with a typed error; register the file_metadata_duplicate_filter and file_metadata_too_many_filters error codes; reserve `visibility` as a custom metadata key so it can't shadow the real R2 visibility gate. --- apps/api/src/file-metadata.ts | 21 +++++- apps/api/src/files-core.ts | 7 +- apps/api/src/routes/files.ts | 17 ++++- apps/api/test/file-metadata-sqlite.test.ts | 18 +++++ apps/api/test/file-metadata.test.ts | 10 +++ apps/api/test/routes-files.test.ts | 77 +++++++++++++++++++++- packages/errors/src/codes.ts | 2 + 7 files changed, 145 insertions(+), 7 deletions(-) diff --git a/apps/api/src/file-metadata.ts b/apps/api/src/file-metadata.ts index d87ae12..65e1adc 100644 --- a/apps/api/src/file-metadata.ts +++ b/apps/api/src/file-metadata.ts @@ -21,7 +21,11 @@ export const META_KEY_RE = /^[a-z][a-z0-9._-]{0,63}$/; * upload capture, the PATCH endpoint, and any future setFileMetadata caller. * `gh.*` keys are NOT reserved: system-managed by convention only (design doc). */ -const RESERVED_META_KEYS = new Set(PROVENANCE_SERVER_KEYS); +// `visibility` is reserved too: it names the R2-backed public/private gate +// (visibility.ts's VISIBILITY_META_KEY), not a piece of D1 custom metadata. A +// custom row with this name would render on the public /f/ panel looking +// like an access-control setting when it's just an unrelated user tag. +const RESERVED_META_KEYS = new Set([...PROVENANCE_SERVER_KEYS, "visibility"]); /** Cap applied both to a single write request and to a file's total keys post-merge. */ export const META_MAX_KEYS = 24; @@ -183,6 +187,17 @@ export async function deleteFileMetadata( const FIND_DEFAULT_LIMIT = 50; const FIND_MAX_LIMIT = 500; +/** + * Escapes SQL LIKE metacharacters (`%`, `_`, and the escape character itself) + * so a prefix like `my_app/` matches only literal underscores — paired with + * `ESCAPE '\'` in the query. Without this, `_` and `%` in `opts.prefix` are + * interpreted as single-char/any-run wildcards and over-match (e.g. `my_app/` + * would also match `myXapp/`). + */ +function escapeLikePattern(raw: string): string { + return raw.replace(/[\\%_]/g, (ch) => `\\${ch}`); +} + /** * Finds objects whose metadata matches ALL `filters` (ANDed equality), with * an optional key-prefix and result limit. Returns each match's key plus its @@ -205,8 +220,8 @@ export async function findObjectsByMetadata( let sql = `SELECT object_key FROM file_metadata WHERE workspace = ? AND (${conditions})`; if (opts.prefix) { - sql += ` AND object_key LIKE ? || '%'`; - params.push(opts.prefix); + sql += ` AND object_key LIKE ? || '%' ESCAPE '\\'`; + params.push(escapeLikePattern(opts.prefix)); } sql += ` GROUP BY object_key HAVING COUNT(DISTINCT meta_key) = ? ORDER BY object_key LIMIT ?`; params.push(entries.length, limit); diff --git a/apps/api/src/files-core.ts b/apps/api/src/files-core.ts index 5d3fe77..f35f6d7 100644 --- a/apps/api/src/files-core.ts +++ b/apps/api/src/files-core.ts @@ -122,8 +122,11 @@ export async function putObject( * `provenance` bag above. When present (even `{}`), this call fully * replaces any metadata already stored for the key — delete-then-set — * so an overwrite never leaves stale rows from a prior put. Omit - * entirely (undefined) to leave existing metadata untouched, which is - * what non-route callers (e.g. the MCP worker) do today. + * entirely (undefined) to leave existing metadata untouched. Every + * caller follows this contract: the REST PUT route passes `undefined` + * when the request had no custom (non-provenance) `X-Uploads-Meta-*` + * headers, and the MCP `put`/`attach` tools pass `undefined` when their + * `metadata` argument was omitted. */ metadata?: Record; }, diff --git a/apps/api/src/routes/files.ts b/apps/api/src/routes/files.ts index d545286..85cc0e6 100644 --- a/apps/api/src/routes/files.ts +++ b/apps/api/src/routes/files.ts @@ -10,6 +10,7 @@ import { } from "../files-core"; import { META_KEY_RE, + META_MAX_KEYS, findObjectsByMetadata, getFileMetadata, setFileMetadata, @@ -112,13 +113,20 @@ export const files = new Hono() const body = await c.req.arrayBuffer(); const visibility = sanitizeVisibility(c.req.header("x-uploads-visibility")); const { provenance, custom } = splitUploadMetaHeaders(c.req.raw.headers); + // No custom (non-provenance) X-Uploads-Meta-* headers at all: pass + // `metadata: undefined` so putObject leaves any existing D1 metadata + // untouched (matches the MCP `put` tool's omit-preserves semantics). + // At least one custom header: keep the existing full-replace behavior, + // even when that header's value alone ends up empty/invalid (putObject + // still validates and rejects before any write). + const hasCustomMeta = Object.keys(custom).length > 0; const result = await putObject( c.env, c.get("workspace"), key, new Uint8Array(body), c.get("workspaceName"), - { provenance, visibility, metadata: custom }, + { provenance, visibility, metadata: hasCustomMeta ? custom : undefined }, ); return c.json({ workspace: c.get("workspaceName"), ...result }, 201); }) @@ -135,6 +143,13 @@ export const files = new Hono() const metaParamKeys = Object.keys(query).filter((k) => k.startsWith("meta.")); if (metaParamKeys.length > 0) { + if (metaParamKeys.length > META_MAX_KEYS) { + throw new ValidationError(`too many meta.* filters (max ${META_MAX_KEYS})`, { + code: "file_metadata_too_many_filters", + details: { limit: META_MAX_KEYS, count: metaParamKeys.length }, + }); + } + const filters: Record = {}; for (const param of metaParamKeys) { const key = param.slice("meta.".length); diff --git a/apps/api/test/file-metadata-sqlite.test.ts b/apps/api/test/file-metadata-sqlite.test.ts index 8e6a6fb..53e10d9 100644 --- a/apps/api/test/file-metadata-sqlite.test.ts +++ b/apps/api/test/file-metadata-sqlite.test.ts @@ -245,6 +245,24 @@ describe("file metadata persistence against SQLite", () => { } }); + it("treats an underscore in the prefix literally, not as a SQL LIKE wildcard", async () => { + const sqlite = new SqliteD1(); + try { + await setFileMetadata(database(sqlite), "alpha", "my_app/x.png", { app: "a" }); + await setFileMetadata(database(sqlite), "alpha", "myxapp/x.png", { app: "a" }); + + const matches = await findObjectsByMetadata( + database(sqlite), + "alpha", + { app: "a" }, + { prefix: "my_app/" }, + ); + expect(matches).toEqual([{ key: "my_app/x.png", metadata: { app: "a" } }]); + } finally { + sqlite.close(); + } + }); + it("respects a limit and returns an empty array for no filters or no matches", async () => { const sqlite = new SqliteD1(); try { diff --git a/apps/api/test/file-metadata.test.ts b/apps/api/test/file-metadata.test.ts index de92205..c2f1865 100644 --- a/apps/api/test/file-metadata.test.ts +++ b/apps/api/test/file-metadata.test.ts @@ -52,6 +52,16 @@ describe("validateMetadataEntries", () => { expect(() => validateMetadataEntries({ "gh.repo": "a/b" })).not.toThrow(); }); + it("throws a reserved-key AppError for `visibility` (would shadow the R2 visibility gate)", () => { + try { + validateMetadataEntries({ visibility: "private" }); + throw new Error("expected validateMetadataEntries to throw"); + } catch (err) { + expect(err).toBeInstanceOf(AppError); + expect((err as AppError & { code?: string }).code).toBe("file_metadata_reserved_key"); + } + }); + it("throws AppError for an empty value", () => { expect(() => validateMetadataEntries({ app: "" })).toThrow(AppError); }); diff --git a/apps/api/test/routes-files.test.ts b/apps/api/test/routes-files.test.ts index a839329..6fe7e05 100644 --- a/apps/api/test/routes-files.test.ts +++ b/apps/api/test/routes-files.test.ts @@ -418,6 +418,21 @@ describe("PUT /v1/:workspace/files custom metadata capture + cascade", () => { ).resolves.toEqual({}); }); + it("rejects an upload trying to shadow the R2 visibility gate as custom metadata", async () => { + const { env, db, bucket } = await makeEnv(); + const res = await putShot(env, { + headers: { "X-Uploads-Meta-Visibility": "private" }, + }); + expect(res.status).toBe(400); + const json = (await res.json()) as { error: { type: string; code: string } }; + expect(json.error.type).toBe("validation"); + expect(json.error.code).toBe("file_metadata_reserved_key"); + expect(bucket.store.has("default/screenshots/shot.png")).toBe(false); + await expect( + getFileMetadata(db as unknown as D1Database, "default", "screenshots/shot.png"), + ).resolves.toEqual({}); + }); + it("rejects an empty custom metadata value instead of silently dropping it", async () => { const { env, bucket } = await makeEnv(); const res = await putShot(env, { headers: { "X-Uploads-Meta-App": "" } }); @@ -473,7 +488,7 @@ describe("PUT /v1/:workspace/files custom metadata capture + cascade", () => { ).resolves.toEqual({}); }); - it("re-PUT (overwrite) replaces prior custom metadata rather than merging", async () => { + it("re-PUT with at least one custom header still fully replaces prior custom metadata", async () => { const { env, db } = await makeEnv(); const first = await putShot(env, { headers: { "X-Uploads-Meta-App": "web", "X-Uploads-Meta-Page": "/checkout" }, @@ -489,6 +504,37 @@ describe("PUT /v1/:workspace/files custom metadata capture + cascade", () => { getFileMetadata(db as unknown as D1Database, "default", "screenshots/shot.png"), ).resolves.toEqual({ page: "/cart" }); }); + + it("re-PUT with no custom headers preserves prior custom metadata", async () => { + const { env, db } = await makeEnv(); + const first = await putShot(env, { + headers: { "X-Uploads-Meta-App": "web", "X-Uploads-Meta-Page": "/checkout" }, + }); + expect(first.status).toBe(201); + await expect( + getFileMetadata(db as unknown as D1Database, "default", "screenshots/shot.png"), + ).resolves.toEqual({ app: "web", page: "/checkout" }); + + // No X-Uploads-Meta-* headers at all — not even a provenance-only one. + const second = await putShot(env); + expect(second.status).toBe(201); + await expect( + getFileMetadata(db as unknown as D1Database, "default", "screenshots/shot.png"), + ).resolves.toEqual({ app: "web", page: "/checkout" }); + }); + + it("re-PUT with only allowlisted provenance headers (no custom keys) preserves prior custom metadata", async () => { + const { env, db } = await makeEnv(); + const first = await putShot(env, { headers: { "X-Uploads-Meta-App": "web" } }); + expect(first.status).toBe(201); + + // "client" is an allowlisted provenance key, not custom metadata. + const second = await putShot(env, { headers: { "X-Uploads-Meta-Client": "cli" } }); + expect(second.status).toBe(201); + await expect( + getFileMetadata(db as unknown as D1Database, "default", "screenshots/shot.png"), + ).resolves.toEqual({ app: "web" }); + }); }); function getMeta(env: Parameters[2], key: string, token = TOKEN) { @@ -563,6 +609,22 @@ describe("GET/PATCH /v1/:workspace/files/:key/metadata", () => { expect(await res.json()).toEqual({ metadata: {} }); }); + it("PATCH rejects setting the reserved visibility key with 400", async () => { + const { env } = await makeEnv(); + await putShot(env); + + const patch = await patchMeta(env, "screenshots/shot.png", { + set: { visibility: "private" }, + }); + expect(patch.status).toBe(400); + const json = (await patch.json()) as { error: { type: string; code: string } }; + expect(json.error.type).toBe("validation"); + expect(json.error.code).toBe("file_metadata_reserved_key"); + + const res = await getMeta(env, "screenshots/shot.png"); + expect(await res.json()).toEqual({ metadata: {} }); + }); + it("PATCH delete removes a key", async () => { const { env } = await makeEnv(); await putShot(env); @@ -821,4 +883,17 @@ describe("GET /v1/:workspace/files list + meta.* filter", () => { const json = (await res.json()) as { error: { type: string } }; expect(json.error.type).toBe("validation"); }); + + it("rejects more than 24 meta.* filter params with a typed error", async () => { + const { env } = await makeEnv(); + await putShot(env); + + const qs = "?" + Array.from({ length: 25 }, (_, i) => `meta.k${i}=v`).join("&"); + const res = await listFiles(env, qs); + expect(res.status).toBe(400); + const json = (await res.json()) as { error: { type: string; code: string; details: unknown } }; + expect(json.error.type).toBe("validation"); + expect(json.error.code).toBe("file_metadata_too_many_filters"); + expect(json.error.details).toEqual({ limit: 24, count: 25 }); + }); }); diff --git a/packages/errors/src/codes.ts b/packages/errors/src/codes.ts index 5a75184..c46c28b 100644 --- a/packages/errors/src/codes.ts +++ b/packages/errors/src/codes.ts @@ -61,6 +61,8 @@ export const ERROR_CODES = [ "file_metadata_invalid_value", "file_metadata_limit_exceeded", "file_metadata_reserved_key", + "file_metadata_duplicate_filter", + "file_metadata_too_many_filters", // Admin "invalid_workspace", From 17d988bf35e6cfff739c39f665ecc01c73cb1837 Mon Sep 17 00:00:00 2001 From: Zach Dunn Date: Mon, 13 Jul 2026 19:37:05 -0400 Subject: [PATCH 17/23] feat(mcp): remote MCP parity for set_metadata and find_files tools Mirrors the local stdio MCP's set_metadata/find_files tools on the remote worker (apps/mcp/src/tools.ts), implemented against the shared api modules (setFileMetadata/findObjectsByMetadata) so both MCP surfaces expose the same metadata CRUD/search behavior called out in the design doc. --- apps/mcp/src/tools.ts | 100 ++++++++++++++++++++++- apps/mcp/test/mcp.test.ts | 164 ++++++++++++++++++++++++++++++++++++++ 2 files changed, 263 insertions(+), 1 deletion(-) diff --git a/apps/mcp/src/tools.ts b/apps/mcp/src/tools.ts index d09644e..a9b4d8b 100644 --- a/apps/mcp/src/tools.ts +++ b/apps/mcp/src/tools.ts @@ -14,7 +14,11 @@ import { type McpTool, } from "@buildinternet/uploads/mcp"; import { badKey } from "@uploads/api/files"; -import { validateMetadataEntries } from "@uploads/api/file-metadata"; +import { + findObjectsByMetadata, + setFileMetadata, + validateMetadataEntries, +} from "@uploads/api/file-metadata"; import { addExternalReference, addGalleryItem, @@ -47,6 +51,25 @@ export interface RemoteToolContext { authScopes: readonly FileScope[]; } +/** Shared tool-description text for the metadata-shaped params (mirrors packages/uploads/src/mcp/tools.ts). */ +const METADATA_DESCRIPTION = + "Queryable custom metadata (key→value), separate from provenance. Keys: lowercase, ^[a-z][a-z0-9._-]{0,63}$. Values: 1-512 printable ASCII characters. Caps: at most 24 keys, at most 8192 total key+value bytes. Suggested keys: app, url, page, device, resolution, commit, branch. `gh.*` is reserved by convention for GitHub PR/issue attachment context (repo/kind/number/ref)."; + +const metadataProp = { + type: "object", + additionalProperties: { type: "string" }, + description: METADATA_DESCRIPTION, +}; + +function optStringArray(args: Record, name: string): string[] | undefined { + const v = args[name]; + if (v === undefined || v === null) return undefined; + if (!Array.isArray(v) || v.some((item) => typeof item !== "string")) { + usage(`${name} must be an array of strings`); + } + return v as string[]; +} + function decodeBase64(value: string, maxBytes: number): Uint8Array { // Pre-decode size gate: base64 encodes 3 bytes per 4 chars, so a string // longer than this cannot decode to a within-limit payload. Rejecting here @@ -415,6 +438,81 @@ export function createRemoteTools(ctx: RemoteToolContext): McpTool[] { return deleteObject(env, workspace, key, workspaceName); }, }, + { + name: "set_metadata", + description: + "Merge-set and/or delete an object's queryable custom metadata (D1-backed key-value pairs; distinct from the R2 provenance headers put on upload). `set` pairs win over `delete` when a key appears in both. " + + METADATA_DESCRIPTION + + " Requires at least one of `set` or `delete`. Same as the CLI/local MCP's `set_metadata` tool.", + inputSchema: { + type: "object", + properties: { + key: { type: "string", description: "Object key to update." }, + set: { ...metadataProp, description: "Keys to set/overwrite. " + METADATA_DESCRIPTION }, + delete: { + type: "array", + items: { type: "string" }, + description: "Keys to remove.", + }, + }, + required: ["key"], + additionalProperties: false, + }, + async handler(args) { + requireScope("files:write"); + await requireWriteBudget(); + const key = optString(args, "key"); + if (!key) usage("key is required"); + if (badKey(key)) usage("invalid key"); + const set = optStringRecord(args, "set"); + const del = optStringArray(args, "delete"); + if ((!set || Object.keys(set).length === 0) && (!del || del.length === 0)) { + usage("set_metadata requires set and/or delete"); + } + const store = await storage(env, workspace); + if (!(await store.exists(key))) throw new Error("object not found"); + const metadata = await setFileMetadata(env.DB, workspaceName, key, set ?? {}, del ?? []); + return { metadata }; + }, + }, + { + name: "find_files", + description: + "Find objects in the workspace whose queryable custom metadata matches ALL of `filters` (ANDed equality). Returns each match's key, public URL, and full metadata map. Same as the CLI/local MCP's `find_files` tool.", + inputSchema: { + type: "object", + properties: { + filters: { + ...metadataProp, + description: "Metadata equality filters (at least one pair). " + METADATA_DESCRIPTION, + }, + prefix: { type: "string", description: "Key prefix filter, combinable with filters." }, + limit: { type: "number", description: "Page size (default 50, max 500)." }, + }, + required: ["filters"], + additionalProperties: false, + }, + async handler(args) { + requireScope("files:read"); + const filters = optStringRecord(args, "filters"); + if (!filters || Object.keys(filters).length === 0) { + usage("filters must have at least one key"); + } + const cfg = await storageConfig(env, workspace); + const matches = await findObjectsByMetadata(env.DB, workspaceName, filters, { + prefix: optString(args, "prefix"), + limit: optPosInt(args, "limit"), + }); + return { + items: matches.map((match) => ({ + key: match.key, + url: publicUrl(cfg, match.key), + metadata: match.metadata, + })), + cursor: null, + }; + }, + }, { name: "usage", description: diff --git a/apps/mcp/test/mcp.test.ts b/apps/mcp/test/mcp.test.ts index ae61d83..427fb54 100644 --- a/apps/mcp/test/mcp.test.ts +++ b/apps/mcp/test/mcp.test.ts @@ -109,6 +109,53 @@ async function makeEnv( meta: {}, }; } + // findObjectsByMetadata's first query: matches by ANDed key/value + // pairs (+ optional escaped-LIKE prefix), grouped/having-counted. + // Good enough for tests — not a general SQL engine. + if (normalized.startsWith("SELECT object_key FROM file_metadata WHERE workspace")) { + const vals = values as unknown[]; + const ws = vals[0] as string; + const hasPrefix = normalized.includes("LIKE"); + const pairsEnd = hasPrefix ? vals.length - 3 : vals.length - 2; + const pairs: Array<[string, string]> = []; + for (let i = 1; i < pairsEnd; i += 2) { + pairs.push([vals[i] as string, vals[i + 1] as string]); + } + const prefix = hasPrefix + ? (vals[pairsEnd] as string).replace(/\\([%_\\])/g, "$1") + : undefined; + const limit = vals[vals.length - 1] as number; + + const matches: string[] = []; + for (const [scoped, map] of metadata.entries()) { + const [scopedWs, objectKey] = scoped.split(" "); + if (scopedWs !== ws) continue; + if (prefix !== undefined && !objectKey.startsWith(prefix)) continue; + if (pairs.every(([k, v]) => map.get(k) === v)) matches.push(objectKey); + } + matches.sort(); + return { + success: true, + results: matches.slice(0, limit).map((object_key) => ({ object_key })) as T[], + meta: {}, + }; + } + // findObjectsByMetadata's hydrate query: full metadata for a set of keys. + if ( + normalized.startsWith("SELECT object_key, meta_key, meta_value FROM file_metadata") + ) { + const [ws, ...keys] = values as string[]; + const results: Array<{ object_key: string; meta_key: string; meta_value: string }> = + []; + for (const objectKey of keys) { + const map = metadata.get(scopeKey(ws, objectKey)); + if (!map) continue; + for (const [meta_key, meta_value] of map.entries()) { + results.push({ object_key: objectKey, meta_key, meta_value }); + } + } + return { success: true, results: results as T[], meta: {} }; + } return { success: true, results: [] as T[], meta: {} }; }, }; @@ -329,6 +376,7 @@ describe("mcp worker", () => { const body = (await response.json()) as { result: { tools: { name: string }[] } }; expect(body.result.tools.map((tool) => tool.name).sort()).toEqual([ "delete", + "find_files", "gallery_add", "gallery_create", "gallery_find_by_reference", @@ -339,6 +387,7 @@ describe("mcp worker", () => { "purge_expired", "put", "reconcile", + "set_metadata", "usage", ]); }); @@ -413,6 +462,121 @@ describe("mcp worker", () => { expect(bucket.store.size).toBe(0); }); + it("set_metadata merges set + delete and returns the resulting map", async () => { + const { env, metadata } = await makeEnv(); + await callTool(env, "put", { + contentBase64: PNG_B64, + filename: "shot.png", + key: "shots/tagged.png", + metadata: { app: "myapp", page: "/checkout" }, + }); + + const result = await callTool(env, "set_metadata", { + key: "shots/tagged.png", + set: { page: "/cart" }, + delete: ["app"], + }); + expect(result.isError).toBe(false); + expect(result.structuredContent).toEqual({ metadata: { page: "/cart" } }); + expect(Object.fromEntries(metadata.get("test-ws shots/tagged.png") ?? [])).toEqual({ + page: "/cart", + }); + }); + + it("set_metadata requires at least one of set/delete", async () => { + const { env } = await makeEnv(); + await callTool(env, "put", { + contentBase64: PNG_B64, + filename: "shot.png", + key: "shots/tagged.png", + }); + const result = await callTool(env, "set_metadata", { key: "shots/tagged.png" }); + expect(result.isError).toBe(true); + expect(result.content).toEqual([ + { type: "text", text: "set_metadata requires set and/or delete (USAGE)" }, + ]); + }); + + it("set_metadata 404s for an object that doesn't exist", async () => { + const { env } = await makeEnv(); + const result = await callTool(env, "set_metadata", { + key: "shots/missing.png", + set: { app: "x" }, + }); + expect(result.isError).toBe(true); + }); + + it("set_metadata rejects a reserved key as a tool error", async () => { + const { env } = await makeEnv(); + await callTool(env, "put", { + contentBase64: PNG_B64, + filename: "shot.png", + key: "shots/tagged.png", + }); + const result = await callTool(env, "set_metadata", { + key: "shots/tagged.png", + set: { "content-sha256": "0".repeat(64) }, + }); + expect(result.isError).toBe(true); + }); + + it("set_metadata enforces files:write scope", async () => { + const token = "up_test-ws_read-only-token"; + const { env } = await makeEnv({ + d1: { tokenHash: await sha256Hex(token), scopes: JSON.stringify(["files:read"]) }, + }); + const result = await callTool( + env, + "set_metadata", + { key: "shots/x.png", set: { app: "x" } }, + token, + ); + expect(result.isError).toBe(true); + expect(result.content).toEqual([ + { type: "text", text: "forbidden: requires files:write scope" }, + ]); + }); + + it("find_files finds objects matching ALL ANDed filters, with public URLs", async () => { + const { env } = await makeEnv(); + await callTool(env, "put", { + contentBase64: PNG_B64, + filename: "shot.png", + key: "shots/one.png", + metadata: { app: "myapp", page: "/checkout" }, + }); + await callTool(env, "put", { + contentBase64: PNG_B64, + filename: "shot.png", + key: "shots/two.png", + metadata: { app: "myapp", page: "/cart" }, + }); + + const result = await callTool(env, "find_files", { + filters: { app: "myapp", page: "/checkout" }, + }); + expect(result.isError).toBe(false); + expect(result.structuredContent).toEqual({ + items: [ + { + key: "shots/one.png", + url: "https://storage.example.com/shots/one.png", + metadata: { app: "myapp", page: "/checkout" }, + }, + ], + cursor: null, + }); + }); + + it("find_files requires at least one filter", async () => { + const { env } = await makeEnv(); + const result = await callTool(env, "find_files", { filters: {} }); + expect(result.isError).toBe(true); + expect(result.content).toEqual([ + { type: "text", text: "filters must have at least one key (USAGE)" }, + ]); + }); + it("computes the default screenshot key without git derivation", async () => { const { env, bucket } = await makeEnv(); const result = await callTool(env, "put", { From 8234d1f45ade99713ed2f39f8269bd13c00e2b25 Mon Sep 17 00:00:00 2001 From: Zach Dunn Date: Mon, 13 Jul 2026 19:39:09 -0400 Subject: [PATCH 18/23] docs(uploads-cli): document put --meta replace-vs-preserve semantics Adds the re-upload metadata contract (--meta replaces, omitted preserves; attach always replaces since it always sends gh.*) to the CLI's put/attach --help text and the uploads-cli skill doc, per fix 7 of the metadata final-review pass. --- packages/uploads/src/commands.ts | 6 ++++++ skills/uploads-cli/SKILL.md | 10 ++++++++-- 2 files changed, 14 insertions(+), 2 deletions(-) diff --git a/packages/uploads/src/commands.ts b/packages/uploads/src/commands.ts index 2c46b88..746297d 100644 --- a/packages/uploads/src/commands.ts +++ b/packages/uploads/src/commands.ts @@ -112,6 +112,9 @@ Options: --comment With --pr/--issue: update one managed comment with attachments and linked galleries via local gh auth --gallery Add the uploaded object to this public gallery --meta Queryable custom metadata (repeatable; value may contain "="): key ^[a-z][a-z0-9._-]{0,63}$, value 1-512 printable ASCII, max 24 pairs + Re-uploading to an existing key WITH --meta replaces that file's + entire metadata set; without --meta the existing metadata is + preserved. Use "uploads meta set" to edit individual keys. --dry-run Print key + public URL without uploading. Not with --comment/--gallery Exit codes: 0 ok · 2 usage/token/file · 3 auth/policy · 4 network · 1 other. @@ -345,6 +348,9 @@ Options: --meta Extra queryable metadata (repeatable; value may contain "="). gh.repo/gh.kind/gh.number/gh.ref are always set from the resolved target — a --meta pair with the same key is overridden by it. + Because attach always sends its own gh.* pairs, re-attaching to + the same key always replaces that file's entire metadata set + (never preserves) — use "uploads meta set" to add to it instead. Examples: uploads attach ./before.png ./after.png diff --git a/skills/uploads-cli/SKILL.md b/skills/uploads-cli/SKILL.md index c9c6ef4..f0b1bad 100644 --- a/skills/uploads-cli/SKILL.md +++ b/skills/uploads-cli/SKILL.md @@ -205,8 +205,14 @@ Rules (validated client-side, fail-fast, before uploading): key `^[a-z][a-z0-9._-]{0,63}$` (lowercase, dot-namespacing allowed, e.g. `gh.repo`); value 1–512 printable ASCII characters; `--meta k=v` may repeat up to 24 times per request; a value may itself contain `=` (only the first `=` splits key from value). -`content-sha256` is reserved (server-computed). `uploads attach` writes its own -`gh.*` reserved-by-convention keys automatically — see below. +`content-sha256` and `visibility` are reserved (server-computed / the real R2 +visibility gate, respectively). `uploads attach` writes its own `gh.*` +reserved-by-convention keys automatically — see below. + +**Re-upload semantics:** re-uploading to an existing key **with** `--meta` replaces +that file's entire metadata set (delete-then-set, not a merge); re-uploading +**without** `--meta` at all preserves the existing metadata untouched. Use +`uploads meta set` to edit individual keys without disturbing the rest. ## Public media galleries From 530fc2b1c80d919de7f7bf15e665b374a2af2c70 Mon Sep 17 00:00:00 2001 From: Zach Dunn Date: Mon, 13 Jul 2026 19:44:09 -0400 Subject: [PATCH 19/23] fix(mcp): cap find_files filters to match REST endpoint --- apps/mcp/src/tools.ts | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/apps/mcp/src/tools.ts b/apps/mcp/src/tools.ts index a9b4d8b..7fac60a 100644 --- a/apps/mcp/src/tools.ts +++ b/apps/mcp/src/tools.ts @@ -15,6 +15,7 @@ import { } from "@buildinternet/uploads/mcp"; import { badKey } from "@uploads/api/files"; import { + META_MAX_KEYS, findObjectsByMetadata, setFileMetadata, validateMetadataEntries, @@ -498,6 +499,10 @@ export function createRemoteTools(ctx: RemoteToolContext): McpTool[] { if (!filters || Object.keys(filters).length === 0) { usage("filters must have at least one key"); } + // Mirror the REST list endpoint's meta.* filter cap. + if (Object.keys(filters).length > META_MAX_KEYS) { + usage(`too many filters (max ${META_MAX_KEYS})`); + } const cfg = await storageConfig(env, workspace); const matches = await findObjectsByMetadata(env.DB, workspaceName, filters, { prefix: optString(args, "prefix"), From c74793297c4a1f7df2d209a3eb5500f9c58b7e1d Mon Sep 17 00:00:00 2001 From: Zach Dunn Date: Mon, 13 Jul 2026 20:15:20 -0400 Subject: [PATCH 20/23] test(api): extract shared fake-D1 test harnesses for file metadata Consolidate three copy-pasted node:sqlite fake-D1 harnesses (file-metadata-sqlite/galleries-sqlite) into apps/api/test/helpers/sqlite-d1.ts parameterized by migration path(s) and optional pragmas, and consolidate the three near-identical hand-rolled file_metadata D1 mocks in routes-files.test.ts, routes-public-files.test.ts, and usage-fake-d1.ts into a shared FileMetadataTable helper that each suite's fake D1 tries first, falling back to its own auth_tokens/workspace_usage logic. No behavior change. --- apps/api/test/file-metadata-sqlite.test.ts | 96 ++------------ apps/api/test/galleries-sqlite.test.ts | 90 ++----------- .../test/helpers/fake-file-metadata-table.ts | 125 ++++++++++++++++++ apps/api/test/helpers/sqlite-d1.ts | 92 +++++++++++++ apps/api/test/routes-files.test.ts | 99 ++------------ apps/api/test/routes-public-files.test.ts | 45 ++----- apps/api/test/usage-fake-d1.ts | 47 ++----- 7 files changed, 276 insertions(+), 318 deletions(-) create mode 100644 apps/api/test/helpers/fake-file-metadata-table.ts create mode 100644 apps/api/test/helpers/sqlite-d1.ts diff --git a/apps/api/test/file-metadata-sqlite.test.ts b/apps/api/test/file-metadata-sqlite.test.ts index 53e10d9..18812b6 100644 --- a/apps/api/test/file-metadata-sqlite.test.ts +++ b/apps/api/test/file-metadata-sqlite.test.ts @@ -1,7 +1,5 @@ /// -import { readFileSync } from "node:fs"; -import { DatabaseSync } from "node:sqlite"; import { AppError } from "@uploads/errors"; import { describe, expect, it } from "vitest"; import { @@ -11,83 +9,13 @@ import { getFileMetadata, setFileMetadata, } from "../src/file-metadata"; +import { SqliteD1, database } from "./helpers/sqlite-d1"; -type SqliteValue = string | number | bigint | null | Uint8Array; - -class SqliteStatement { - private values: SqliteValue[] = []; - - constructor( - readonly owner: SqliteD1, - readonly sql: string, - ) {} - - bind(...values: unknown[]) { - this.values = values as SqliteValue[]; - return this; - } - - async first(): Promise { - return (this.owner.db.prepare(this.sql).get(...this.values) as T | undefined) ?? null; - } - - async all(): Promise> { - return { - success: true, - results: this.owner.db.prepare(this.sql).all(...this.values) as T[], - meta: {}, - } as D1Result; - } - - async run(): Promise { - return this.runSync() as unknown as D1Result; - } - - runSync() { - const result = this.owner.db.prepare(this.sql).run(...this.values); - return { - success: true, - results: [], - meta: { changes: Number(result.changes) }, - }; - } -} - -class SqliteD1 { - readonly db = new DatabaseSync(":memory:"); - - constructor() { - this.db.exec(readFileSync("migrations/20260713210559_file_metadata.sql", "utf8")); - } - - prepare(sql: string) { - return new SqliteStatement(this, sql); - } - - async batch(statements: SqliteStatement[]): Promise { - this.db.exec("BEGIN IMMEDIATE"); - try { - const results = statements.map((statement) => statement.runSync() as unknown as D1Result); - this.db.exec("COMMIT"); - return results; - } catch (error) { - this.db.exec("ROLLBACK"); - throw error; - } - } - - close() { - this.db.close(); - } -} - -function database(sqlite: SqliteD1): D1Database { - return sqlite as unknown as D1Database; -} +const MIGRATION = "migrations/20260713210559_file_metadata.sql"; describe("file metadata persistence against SQLite", () => { it("returns an empty map for an object with no metadata", async () => { - const sqlite = new SqliteD1(); + const sqlite = new SqliteD1(MIGRATION); try { await expect(getFileMetadata(database(sqlite), "alpha", "f/one.png")).resolves.toEqual({}); } finally { @@ -96,7 +24,7 @@ describe("file metadata persistence against SQLite", () => { }); it("sets, reads, and scopes metadata by workspace and object key", async () => { - const sqlite = new SqliteD1(); + const sqlite = new SqliteD1(MIGRATION); try { const result = await setFileMetadata(database(sqlite), "alpha", "f/one.png", { app: "screenshots", @@ -117,7 +45,7 @@ describe("file metadata persistence against SQLite", () => { }); it("merges set and remove in the same call, upserting and deleting keys", async () => { - const sqlite = new SqliteD1(); + const sqlite = new SqliteD1(MIGRATION); try { await setFileMetadata(database(sqlite), "alpha", "f/one.png", { app: "screenshots", @@ -147,7 +75,7 @@ describe("file metadata persistence against SQLite", () => { }); it("rejects a merge that would push the post-merge file over the key cap", async () => { - const sqlite = new SqliteD1(); + const sqlite = new SqliteD1(MIGRATION); try { const initial: Record = {}; for (let i = 0; i < META_MAX_KEYS; i++) initial[`k${i}`] = "v"; @@ -167,7 +95,7 @@ describe("file metadata persistence against SQLite", () => { }); it("allows a merge at the cap when it simultaneously removes a key", async () => { - const sqlite = new SqliteD1(); + const sqlite = new SqliteD1(MIGRATION); try { const initial: Record = {}; for (let i = 0; i < META_MAX_KEYS; i++) initial[`k${i}`] = "v"; @@ -185,7 +113,7 @@ describe("file metadata persistence against SQLite", () => { }); it("deletes all metadata for an object", async () => { - const sqlite = new SqliteD1(); + const sqlite = new SqliteD1(MIGRATION); try { await setFileMetadata(database(sqlite), "alpha", "f/one.png", { app: "screenshots" }); await deleteFileMetadata(database(sqlite), "alpha", "f/one.png"); @@ -196,7 +124,7 @@ describe("file metadata persistence against SQLite", () => { }); it("finds only objects matching ALL AND-ed filters, scoped by workspace", async () => { - const sqlite = new SqliteD1(); + const sqlite = new SqliteD1(MIGRATION); try { await setFileMetadata(database(sqlite), "alpha", "f/one.png", { app: "screenshots", @@ -228,7 +156,7 @@ describe("file metadata persistence against SQLite", () => { }); it("combines a key prefix filter with metadata AND-filters", async () => { - const sqlite = new SqliteD1(); + const sqlite = new SqliteD1(MIGRATION); try { await setFileMetadata(database(sqlite), "alpha", "gh/one.png", { app: "gh" }); await setFileMetadata(database(sqlite), "alpha", "screenshots/one.png", { app: "gh" }); @@ -246,7 +174,7 @@ describe("file metadata persistence against SQLite", () => { }); it("treats an underscore in the prefix literally, not as a SQL LIKE wildcard", async () => { - const sqlite = new SqliteD1(); + const sqlite = new SqliteD1(MIGRATION); try { await setFileMetadata(database(sqlite), "alpha", "my_app/x.png", { app: "a" }); await setFileMetadata(database(sqlite), "alpha", "myxapp/x.png", { app: "a" }); @@ -264,7 +192,7 @@ describe("file metadata persistence against SQLite", () => { }); it("respects a limit and returns an empty array for no filters or no matches", async () => { - const sqlite = new SqliteD1(); + const sqlite = new SqliteD1(MIGRATION); try { await setFileMetadata(database(sqlite), "alpha", "f/one.png", { app: "a" }); await setFileMetadata(database(sqlite), "alpha", "f/two.png", { app: "a" }); diff --git a/apps/api/test/galleries-sqlite.test.ts b/apps/api/test/galleries-sqlite.test.ts index ce7c874..7f3241f 100644 --- a/apps/api/test/galleries-sqlite.test.ts +++ b/apps/api/test/galleries-sqlite.test.ts @@ -1,7 +1,5 @@ /// -import { readFileSync } from "node:fs"; -import { DatabaseSync } from "node:sqlite"; import { describe, expect, it } from "vitest"; import { MAX_GALLERIES_PER_WORKSPACE, @@ -16,79 +14,13 @@ import { removeGalleryItem, reorderGalleryItems, } from "../src/galleries"; +import { SqliteD1, database } from "./helpers/sqlite-d1"; -type SqliteValue = string | number | bigint | null | Uint8Array; +const MIGRATION = "migrations/20260711180000_galleries.sql"; +const PRAGMAS = ["PRAGMA foreign_keys = ON"]; -class SqliteStatement { - private values: SqliteValue[] = []; - - constructor( - readonly owner: SqliteD1, - readonly sql: string, - ) {} - - bind(...values: unknown[]) { - this.values = values as SqliteValue[]; - return this; - } - - async first(): Promise { - return (this.owner.db.prepare(this.sql).get(...this.values) as T | undefined) ?? null; - } - - async all(): Promise> { - return { - success: true, - results: this.owner.db.prepare(this.sql).all(...this.values) as T[], - meta: {}, - } as D1Result; - } - - async run(): Promise { - return this.runSync() as unknown as D1Result; - } - - runSync() { - const result = this.owner.db.prepare(this.sql).run(...this.values); - return { - success: true, - results: [], - meta: { changes: Number(result.changes) }, - }; - } -} - -class SqliteD1 { - readonly db = new DatabaseSync(":memory:"); - - constructor() { - this.db.exec("PRAGMA foreign_keys = ON"); - this.db.exec(readFileSync("migrations/20260711180000_galleries.sql", "utf8")); - } - - prepare(sql: string) { - return new SqliteStatement(this, sql); - } - - async batch(statements: SqliteStatement[]): Promise { - this.db.exec("BEGIN IMMEDIATE"); - try { - const results = statements.map((statement) => statement.runSync() as unknown as D1Result); - this.db.exec("COMMIT"); - return results; - } catch (error) { - this.db.exec("ROLLBACK"); - throw error; - } - } - - close() { - this.db.close(); - } -} - -function database(sqlite: SqliteD1): D1Database { - return sqlite as unknown as D1Database; +function newSqliteD1(): SqliteD1 { + return new SqliteD1(MIGRATION, PRAGMAS); } async function gallery(sqlite: SqliteD1, workspace = "alpha") { @@ -103,7 +35,7 @@ async function gallery(sqlite: SqliteD1, workspace = "alpha") { describe("gallery persistence against SQLite", () => { it("applies the migration with foreign keys and cascades hard deletes", async () => { - const sqlite = new SqliteD1(); + const sqlite = newSqliteD1(); try { const created = await gallery(sqlite); const item = await addGalleryItem(database(sqlite), "alpha", created.id, { @@ -135,7 +67,7 @@ describe("gallery persistence against SQLite", () => { }); it("keeps item mutations versioned, tenant-scoped, ordered, and idempotent", async () => { - const sqlite = new SqliteD1(); + const sqlite = newSqliteD1(); try { const created = await gallery(sqlite); const first = await addGalleryItem(database(sqlite), "alpha", created.id, { @@ -194,7 +126,7 @@ describe("gallery persistence against SQLite", () => { }); it("enforces the active-gallery cap atomically per workspace", async () => { - const sqlite = new SqliteD1(); + const sqlite = newSqliteD1(); try { for (let index = 0; index < MAX_GALLERIES_PER_WORKSPACE; index++) { await expect(gallery(sqlite)).resolves.toMatchObject({ workspace: "alpha" }); @@ -211,7 +143,7 @@ describe("gallery persistence against SQLite", () => { }); it("enforces the item cap inside the conditional insert", async () => { - const sqlite = new SqliteD1(); + const sqlite = newSqliteD1(); try { const created = await gallery(sqlite); const insert = sqlite.db.prepare( @@ -242,7 +174,7 @@ describe("gallery persistence against SQLite", () => { }); it("keeps external-reference retries idempotent and removals precise", async () => { - const sqlite = new SqliteD1(); + const sqlite = newSqliteD1(); try { const created = await gallery(sqlite); const input = { @@ -287,7 +219,7 @@ describe("gallery persistence against SQLite", () => { }); it("allows only one of two concurrent same-version mutations to commit", async () => { - const sqlite = new SqliteD1(); + const sqlite = newSqliteD1(); try { const created = await gallery(sqlite); const results = await Promise.all([ diff --git a/apps/api/test/helpers/fake-file-metadata-table.ts b/apps/api/test/helpers/fake-file-metadata-table.ts new file mode 100644 index 0000000..30cc55a --- /dev/null +++ b/apps/api/test/helpers/fake-file-metadata-table.ts @@ -0,0 +1,125 @@ +/** + * Shared in-memory `file_metadata` table backing for apps/api route tests + * (routes-files.test.ts, routes-public-files.test.ts, usage-fake-d1.ts), so + * putObject/deleteObject/setFileMetadata/findObjectsByMetadata's D1 reads and + * writes behave for real without a full sqlite-backed D1 (see + * file-metadata-sqlite.test.ts for that). Each caller's fake D1 `prepare()` + * tries `tryRun`/`tryAll` first and falls back to its own auth_tokens / + * workspace_usage logic for everything else. + */ + +export interface MetaRow { + meta_key: string; + meta_value: string; +} + +export interface FakeRunResult { + success: true; + meta: { changes: number }; + results: []; +} + +export interface FakeAllResult { + success: true; + results: T[]; + meta: Record; +} + +export class FileMetadataTable { + /** Keyed by `${workspace} ${objectKey}` -> ordered meta_key -> meta_value. */ + readonly metadata = new Map>(); + + private scopeKey(workspace: string, objectKey: string): string { + return `${workspace} ${objectKey}`; + } + + /** Handles the write-side (INSERT/DELETE) statements targeting file_metadata, else undefined. */ + tryRun(normalizedSql: string, args: unknown[]): FakeRunResult | undefined { + if (normalizedSql.startsWith("INSERT INTO file_metadata")) { + const [workspace, objectKey, key, value] = args as [string, string, string, string]; + const scope = this.scopeKey(workspace, objectKey); + const map = this.metadata.get(scope) ?? new Map(); + map.set(key, value); + this.metadata.set(scope, map); + return { success: true, meta: { changes: 1 }, results: [] }; + } + if ( + normalizedSql.startsWith("DELETE FROM file_metadata") && + normalizedSql.includes("meta_key = ?") + ) { + // Single-key delete (setFileMetadata's `remove` path). + const [workspace, objectKey, key] = args as [string, string, string]; + this.metadata.get(this.scopeKey(workspace, objectKey))?.delete(key); + return { success: true, meta: { changes: 1 }, results: [] }; + } + if (normalizedSql.startsWith("DELETE FROM file_metadata")) { + // Whole-object delete (deleteFileMetadata). + const [workspace, objectKey] = args as [string, string]; + this.metadata.delete(this.scopeKey(workspace, objectKey)); + return { success: true, meta: { changes: 1 }, results: [] }; + } + return undefined; + } + + /** Handles the read-side (SELECT) statements targeting file_metadata, else undefined. */ + tryAll(normalizedSql: string, args: unknown[]): FakeAllResult | undefined { + if (normalizedSql.startsWith("SELECT meta_key, meta_value FROM file_metadata")) { + const [workspace, objectKey] = args as [string, string]; + const map = + this.metadata.get(this.scopeKey(workspace, objectKey)) ?? new Map(); + const results = [...map.entries()].map( + ([meta_key, meta_value]) => ({ meta_key, meta_value }) as MetaRow, + ); + return { success: true, results: results as T[], meta: {} }; + } + // findObjectsByMetadata's match query: ANDed equality filters (parsed by + // counting the repeated `(meta_key = ? AND meta_value = ?)` clause), an + // optional prefix LIKE, and a trailing HAVING-count + LIMIT. Mirrors the + // real SQL semantics against the in-memory store so route-level filter + // tests exercise real wiring, not a stub. + if (normalizedSql.startsWith("SELECT object_key FROM file_metadata WHERE workspace")) { + const filterCount = (normalizedSql.match(/meta_key = \? AND meta_value = \?/g) ?? []).length; + const hasPrefix = normalizedSql.includes("object_key LIKE ? || '%'"); + let idx = 0; + const workspace = args[idx++] as string; + const filters: Array<[string, string]> = []; + for (let i = 0; i < filterCount; i++) { + filters.push([args[idx] as string, args[idx + 1] as string]); + idx += 2; + } + const prefix = hasPrefix ? (args[idx++] as string) : undefined; + const requiredCount = args[idx++] as number; + const limit = args[idx++] as number; + + const results: { object_key: string }[] = []; + const prefixMatch = `${workspace} `; + for (const [scopedKey, map] of this.metadata.entries()) { + if (!scopedKey.startsWith(prefixMatch)) continue; + const objectKey = scopedKey.slice(prefixMatch.length); + if (prefix && !objectKey.startsWith(prefix)) continue; + let matchCount = 0; + for (const [key, value] of filters) { + if (map.get(key) === value) matchCount++; + } + if (matchCount === requiredCount) results.push({ object_key: objectKey }); + } + results.sort((a, b) => + a.object_key < b.object_key ? -1 : a.object_key > b.object_key ? 1 : 0, + ); + return { success: true, results: results.slice(0, limit) as T[], meta: {} }; + } + if (normalizedSql.startsWith("SELECT object_key, meta_key, meta_value FROM file_metadata")) { + const [workspace, ...keys] = args as [string, ...string[]]; + const results: { object_key: string; meta_key: string; meta_value: string }[] = []; + for (const key of keys) { + const map = this.metadata.get(this.scopeKey(workspace, key)); + if (!map) continue; + for (const [meta_key, meta_value] of map.entries()) { + results.push({ object_key: key, meta_key, meta_value }); + } + } + return { success: true, results: results as T[], meta: {} }; + } + return undefined; + } +} diff --git a/apps/api/test/helpers/sqlite-d1.ts b/apps/api/test/helpers/sqlite-d1.ts new file mode 100644 index 0000000..db62068 --- /dev/null +++ b/apps/api/test/helpers/sqlite-d1.ts @@ -0,0 +1,92 @@ +/// + +/** + * Shared node:sqlite-backed fake D1, parameterized by the migration file(s) + * to apply on construction. Used by suites (file-metadata-sqlite.test.ts, + * galleries-sqlite.test.ts) that need real SQL semantics — foreign keys, + * uniqueness, GROUP BY/HAVING, transactions — rather than a hand-rolled map. + */ + +import { readFileSync } from "node:fs"; +import { DatabaseSync } from "node:sqlite"; + +type SqliteValue = string | number | bigint | null | Uint8Array; + +export class SqliteStatement { + private values: SqliteValue[] = []; + + constructor( + readonly owner: SqliteD1, + readonly sql: string, + ) {} + + bind(...values: unknown[]) { + this.values = values as SqliteValue[]; + return this; + } + + async first(): Promise { + return (this.owner.db.prepare(this.sql).get(...this.values) as T | undefined) ?? null; + } + + async all(): Promise> { + return { + success: true, + results: this.owner.db.prepare(this.sql).all(...this.values) as T[], + meta: {}, + } as D1Result; + } + + async run(): Promise { + return this.runSync() as unknown as D1Result; + } + + runSync() { + const result = this.owner.db.prepare(this.sql).run(...this.values); + return { + success: true, + results: [], + meta: { changes: Number(result.changes) }, + }; + } +} + +export class SqliteD1 { + readonly db = new DatabaseSync(":memory:"); + + /** + * @param migrationPaths One or more migration file paths (relative to + * apps/api), applied in order. + * @param pragmas Optional PRAGMA statements to run before the migrations + * (e.g. `["PRAGMA foreign_keys = ON"]`). + */ + constructor(migrationPaths: string | string[], pragmas: string[] = []) { + for (const pragma of pragmas) this.db.exec(pragma); + const paths = Array.isArray(migrationPaths) ? migrationPaths : [migrationPaths]; + for (const path of paths) this.db.exec(readFileSync(path, "utf8")); + } + + prepare(sql: string) { + return new SqliteStatement(this, sql); + } + + async batch(statements: SqliteStatement[]): Promise { + this.db.exec("BEGIN IMMEDIATE"); + try { + const results = statements.map((statement) => statement.runSync() as unknown as D1Result); + this.db.exec("COMMIT"); + return results; + } catch (error) { + this.db.exec("ROLLBACK"); + throw error; + } + } + + close() { + this.db.close(); + } +} + +export function database(sqlite: SqliteD1): D1Database { + return sqlite as unknown as D1Database; +} diff --git a/apps/api/test/routes-files.test.ts b/apps/api/test/routes-files.test.ts index be473bd..2f3fbe7 100644 --- a/apps/api/test/routes-files.test.ts +++ b/apps/api/test/routes-files.test.ts @@ -1,5 +1,6 @@ import { beforeAll, describe, expect, it } from "vitest"; import { FakeR2Bucket } from "./fake-r2"; +import { FileMetadataTable } from "./helpers/fake-file-metadata-table"; import { app } from "../src/index"; import { getFileMetadata } from "../src/file-metadata"; import { sha256Hex, type WorkspaceRecord } from "../src/workspace"; @@ -23,17 +24,12 @@ beforeAll(() => { const PNG = new Uint8Array([0x89, 0x50, 0x4e, 0x47, 0x0d, 0x0a, 0x1a, 0x0a, 1, 2, 3, 4]); -interface MetaRow { - meta_key: string; - meta_value: string; -} - /** * Fake D1 that no-ops the usage-ledger surface (as before) but backs - * `file_metadata` with a real in-memory store, so the metadata-cascade - * behavior in `putObject`/`deleteObject` can be exercised at the route - * level without a full sqlite-backed D1 (see file-metadata-sqlite.test.ts - * for that). + * `file_metadata` with a real in-memory store (via the shared + * `FileMetadataTable`), so the metadata-cascade behavior in + * `putObject`/`deleteObject` can be exercised at the route level without a + * full sqlite-backed D1 (see file-metadata-sqlite.test.ts for that). */ /** Optional scoped auth_tokens row, backing `findActiveToken` for scope-enforcement tests. */ interface FakeAuthToken { @@ -42,12 +38,10 @@ interface FakeAuthToken { } function makeFakeDB(authToken?: FakeAuthToken) { - // Keyed by `${workspace} ${objectKey}` -> ordered meta_key -> meta_value. - const metadata = new Map>(); - const scopeKey = (workspace: string, objectKey: string) => `${workspace} ${objectKey}`; + const table = new FileMetadataTable(); return { - metadata, + metadata: table.metadata, prepare(sql: string) { const normalized = sql.replace(/\s+/g, " ").trim(); let args: unknown[] = []; @@ -76,81 +70,14 @@ function makeFakeDB(authToken?: FakeAuthToken) { return null; }, async run() { - if (normalized.startsWith("INSERT INTO file_metadata")) { - const [workspace, objectKey, key, value] = args as [string, string, string, string]; - const map = metadata.get(scopeKey(workspace, objectKey)) ?? new Map(); - map.set(key, value); - metadata.set(scopeKey(workspace, objectKey), map); - } else if (normalized.includes("meta_key = ?")) { - // Single-key delete (`setFileMetadata`'s `remove` path). - const [workspace, objectKey, key] = args as [string, string, string]; - metadata.get(scopeKey(workspace, objectKey))?.delete(key); - } else if (normalized.startsWith("DELETE FROM file_metadata")) { - // Whole-object delete (`deleteFileMetadata`). - const [workspace, objectKey] = args as [string, string]; - metadata.delete(scopeKey(workspace, objectKey)); - } - return { success: true, meta: { changes: 0 }, results: [] }; + return ( + table.tryRun(normalized, args) ?? { success: true, meta: { changes: 0 }, results: [] } + ); }, async all() { - if (normalized.startsWith("SELECT meta_key, meta_value FROM file_metadata")) { - const [workspace, objectKey] = args as [string, string]; - const map = metadata.get(scopeKey(workspace, objectKey)) ?? new Map(); - const results = [...map.entries()].map( - ([meta_key, meta_value]) => ({ meta_key, meta_value }) as MetaRow, - ); - return { success: true, results: results as T[], meta: {} }; - } - // findObjectsByMetadata's match query: ANDed equality filters - // (parsed by counting the repeated `(meta_key = ? AND meta_value = ?)` - // clause), an optional prefix LIKE, and a trailing HAVING-count + - // LIMIT. Mirrors the real SQL semantics against the in-memory store - // so route-level filter tests exercise real wiring, not a stub. - if (normalized.startsWith("SELECT object_key FROM file_metadata WHERE workspace")) { - const filterCount = (normalized.match(/meta_key = \? AND meta_value = \?/g) ?? []) - .length; - const hasPrefix = normalized.includes("object_key LIKE ? || '%'"); - let idx = 0; - const workspace = args[idx++] as string; - const filters: Array<[string, string]> = []; - for (let i = 0; i < filterCount; i++) { - filters.push([args[idx] as string, args[idx + 1] as string]); - idx += 2; - } - const prefix = hasPrefix ? (args[idx++] as string) : undefined; - const requiredCount = args[idx++] as number; - const limit = args[idx++] as number; - - const results: { object_key: string }[] = []; - const prefixMatch = `${workspace} `; - for (const [scopedKey, map] of metadata.entries()) { - if (!scopedKey.startsWith(prefixMatch)) continue; - const objectKey = scopedKey.slice(prefixMatch.length); - if (prefix && !objectKey.startsWith(prefix)) continue; - let matchCount = 0; - for (const [key, value] of filters) { - if (map.get(key) === value) matchCount++; - } - if (matchCount === requiredCount) results.push({ object_key: objectKey }); - } - results.sort((a, b) => - a.object_key < b.object_key ? -1 : a.object_key > b.object_key ? 1 : 0, - ); - return { success: true, results: results.slice(0, limit) as T[], meta: {} }; - } - if (normalized.startsWith("SELECT object_key, meta_key, meta_value FROM file_metadata")) { - const [workspace, ...keys] = args as [string, ...string[]]; - const results: { object_key: string; meta_key: string; meta_value: string }[] = []; - for (const key of keys) { - const map = metadata.get(scopeKey(workspace, key)); - if (!map) continue; - for (const [meta_key, meta_value] of map.entries()) { - results.push({ object_key: key, meta_key, meta_value }); - } - } - return { success: true, results: results as T[], meta: {} }; - } - return { success: true, results: [] as T[], meta: {} }; + return ( + table.tryAll(normalized, args) ?? { success: true, results: [] as T[], meta: {} } + ); }, }; }, diff --git a/apps/api/test/routes-public-files.test.ts b/apps/api/test/routes-public-files.test.ts index 31a2d13..ea28edf 100644 --- a/apps/api/test/routes-public-files.test.ts +++ b/apps/api/test/routes-public-files.test.ts @@ -1,5 +1,6 @@ import { beforeAll, describe, expect, it } from "vitest"; import { FakeR2Bucket } from "./fake-r2"; +import { FileMetadataTable } from "./helpers/fake-file-metadata-table"; import { app } from "../src/index"; import { sha256Hex, type WorkspaceRecord } from "../src/workspace"; @@ -11,20 +12,14 @@ import { sha256Hex, type WorkspaceRecord } from "../src/workspace"; const TOKEN = "secret-token"; -interface MetaRow { - meta_key: string; - meta_value: string; -} - /** - * Fake D1 backing `file_metadata` with a real in-memory store (mirrors - * routes-files.test.ts's makeFakeDB), so this suite can assert on the - * `metadata`/`github` DTO fields the public endpoint derives from real rows - * rather than a stubbed-empty read. + * Fake D1 backing `file_metadata` with a real in-memory store (shared + * `FileMetadataTable`, also used by routes-files.test.ts), so this suite can + * assert on the `metadata`/`github` DTO fields the public endpoint derives + * from real rows rather than a stubbed-empty read. */ function makeFakeDB() { - const metadata = new Map>(); - const scopeKey = (workspace: string, objectKey: string) => `${workspace} ${objectKey}`; + const table = new FileMetadataTable(); return { prepare(sql: string) { @@ -39,30 +34,14 @@ function makeFakeDB() { return null; }, async run() { - if (normalized.startsWith("INSERT INTO file_metadata")) { - const [workspace, objectKey, key, value] = args as [string, string, string, string]; - const map = metadata.get(scopeKey(workspace, objectKey)) ?? new Map(); - map.set(key, value); - metadata.set(scopeKey(workspace, objectKey), map); - } else if (normalized.includes("meta_key = ?")) { - const [workspace, objectKey, key] = args as [string, string, string]; - metadata.get(scopeKey(workspace, objectKey))?.delete(key); - } else if (normalized.startsWith("DELETE FROM file_metadata")) { - const [workspace, objectKey] = args as [string, string]; - metadata.delete(scopeKey(workspace, objectKey)); - } - return { success: true, meta: { changes: 0 }, results: [] }; + return ( + table.tryRun(normalized, args) ?? { success: true, meta: { changes: 0 }, results: [] } + ); }, async all() { - if (normalized.startsWith("SELECT meta_key, meta_value FROM file_metadata")) { - const [workspace, objectKey] = args as [string, string]; - const map = metadata.get(scopeKey(workspace, objectKey)) ?? new Map(); - const results = [...map.entries()].map( - ([meta_key, meta_value]) => ({ meta_key, meta_value }) as MetaRow, - ); - return { success: true, results: results as T[], meta: {} }; - } - return { success: true, results: [] as T[], meta: {} }; + return ( + table.tryAll(normalized, args) ?? { success: true, results: [] as T[], meta: {} } + ); }, }; }, diff --git a/apps/api/test/usage-fake-d1.ts b/apps/api/test/usage-fake-d1.ts index fcf50c5..ff792b8 100644 --- a/apps/api/test/usage-fake-d1.ts +++ b/apps/api/test/usage-fake-d1.ts @@ -3,6 +3,8 @@ * and optional no-op auth_tokens lookups for route tests. */ +import { FileMetadataTable } from "./helpers/fake-file-metadata-table"; + export type UsageRow = { workspace: string; bytes: number; @@ -16,13 +18,15 @@ export class UsageFakeD1 { usage = new Map(); // Backs `file_metadata` so putObject/deleteObject's D1 metadata // cascade (Task 2) doesn't blow up in suites that only care about the - // usage ledger. Keyed by `${workspace} ${objectKey}`. - fileMetadata = new Map>(); + // usage ledger. + private fileMetadataTable = new FileMetadataTable(); + get fileMetadata() { + return this.fileMetadataTable.metadata; + } prepare = (sql: string) => { const normalized = sql.replace(/\s+/g, " ").trim(); let values: unknown[] = []; - const metaScopeKey = (workspace: string, objectKey: string) => `${workspace} ${objectKey}`; const stmt = { bind: (...v: unknown[]) => { @@ -37,42 +41,13 @@ export class UsageFakeD1 { throw new Error(`unsupported first: ${normalized}`); }, all: async () => { - if (normalized.startsWith("SELECT meta_key, meta_value FROM file_metadata")) { - const [workspace, objectKey] = values as [string, string]; - const map = this.fileMetadata.get(metaScopeKey(workspace, objectKey)) ?? new Map(); - return { - success: true as const, - results: [...map.entries()].map(([meta_key, meta_value]) => ({ - meta_key, - meta_value, - })) as T[], - meta: {}, - }; - } + const result = this.fileMetadataTable.tryAll(normalized, values); + if (result) return result; throw new Error(`unsupported all: ${normalized}`); }, run: async () => { - if (normalized.startsWith("INSERT INTO file_metadata")) { - const [workspace, objectKey, key, value] = values as [string, string, string, string]; - const scope = metaScopeKey(workspace, objectKey); - const map = this.fileMetadata.get(scope) ?? new Map(); - map.set(key, value); - this.fileMetadata.set(scope, map); - return { success: true as const, meta: { changes: 1 }, results: [] }; - } - if ( - normalized.startsWith("DELETE FROM file_metadata") && - normalized.includes("meta_key = ?") - ) { - const [workspace, objectKey, key] = values as [string, string, string]; - this.fileMetadata.get(metaScopeKey(workspace, objectKey))?.delete(key); - return { success: true as const, meta: { changes: 1 }, results: [] }; - } - if (normalized.startsWith("DELETE FROM file_metadata")) { - const [workspace, objectKey] = values as [string, string]; - this.fileMetadata.delete(metaScopeKey(workspace, objectKey)); - return { success: true as const, meta: { changes: 1 }, results: [] }; - } + const metaResult = this.fileMetadataTable.tryRun(normalized, values); + if (metaResult) return metaResult; if (normalized.startsWith("INSERT OR IGNORE INTO workspace_usage")) { // applyUsageDelta: (ws, period, updatedAt) with zeros // setUsageTotals: (ws, bytes, objects, period, updatedAt) From c3d4d834cbcaf35c349772c3cb64a1bc81e7b5b9 Mon Sep 17 00:00:00 2001 From: Zach Dunn Date: Mon, 13 Jul 2026 20:21:24 -0400 Subject: [PATCH 21/23] refactor(api,mcp,uploads): dedupe MCP metadata helpers, parallelize independent awaits Promote optStringArray/METADATA_DESCRIPTION/metadataProp from duplicated copies in apps/mcp/src/tools.ts and packages/uploads/src/mcp/tools.ts into packages/uploads/src/mcp/args.ts (re-exported from server.ts), extract a shared validateMetadataFilters (count cap + key format) in apps/api/src/file-metadata.ts used by both the REST list endpoint and the MCP find_files tool, run the independent storageConfig/findObjectsByMetadata awaits in Promise.all on both surfaces, and make the CLI's validateMetaMap validate entries directly instead of round-tripping through parseMetaFlags's "k=v" string reconstruction. Also adds "visibility" to the CLI's reserved metadata keys to match the server, with a test. --- apps/api/src/file-metadata.ts | 28 ++++++++++++++ apps/api/src/routes/files.ts | 32 ++++++---------- apps/mcp/src/tools.ts | 42 +++++++-------------- packages/uploads/src/mcp/args.ts | 24 ++++++++++++ packages/uploads/src/mcp/server.ts | 11 +++++- packages/uploads/src/mcp/tools.ts | 30 +++++---------- packages/uploads/src/metadata.ts | 51 ++++++++++++++++---------- packages/uploads/test/metadata.test.ts | 8 ++++ 8 files changed, 137 insertions(+), 89 deletions(-) diff --git a/apps/api/src/file-metadata.ts b/apps/api/src/file-metadata.ts index 65e1adc..623b3f9 100644 --- a/apps/api/src/file-metadata.ts +++ b/apps/api/src/file-metadata.ts @@ -93,6 +93,34 @@ export function validateMetadataEntries(meta: Record): void { } } +/** + * Validates a `meta.*`-style equality-filter map (REST list endpoint's + * `meta.=` query params, the MCP `find_files` tool's `filters` + * argument): enforces the same count cap and key format as metadata writes, + * using the same typed error codes so existing callers' error handling is + * unaffected. Does not validate filter values (unlike write-side metadata, + * an empty or arbitrary-length filter value is fine — it just won't match + * anything) and does not check for duplicate/repeated params, which is + * query-string-specific and stays in the REST route. + */ +export function validateMetadataFilters(filters: Record): void { + const keys = Object.keys(filters); + if (keys.length > META_MAX_KEYS) { + throw new ValidationError(`too many meta.* filters (max ${META_MAX_KEYS})`, { + code: "file_metadata_too_many_filters", + details: { limit: META_MAX_KEYS, count: keys.length }, + }); + } + for (const key of keys) { + if (!META_KEY_RE.test(key)) { + throw new ValidationError(`invalid metadata key: ${key}`, { + code: "file_metadata_invalid_key", + details: { key }, + }); + } + } +} + interface MetaRow { meta_key: string; meta_value: string; diff --git a/apps/api/src/routes/files.ts b/apps/api/src/routes/files.ts index f441ac5..9d8bdee 100644 --- a/apps/api/src/routes/files.ts +++ b/apps/api/src/routes/files.ts @@ -9,11 +9,10 @@ import { putObject, } from "../files-core"; import { - META_KEY_RE, - META_MAX_KEYS, findObjectsByMetadata, getFileMetadata, setFileMetadata, + validateMetadataFilters, } from "../file-metadata"; import { splitUploadMetaHeaders } from "../provenance"; import { objectPublicUrls, storage, storageConfig } from "../storage"; @@ -147,22 +146,12 @@ export const files = new Hono() const metaParamKeys = Object.keys(query).filter((k) => k.startsWith("meta.")); if (metaParamKeys.length > 0) { - if (metaParamKeys.length > META_MAX_KEYS) { - throw new ValidationError(`too many meta.* filters (max ${META_MAX_KEYS})`, { - code: "file_metadata_too_many_filters", - details: { limit: META_MAX_KEYS, count: metaParamKeys.length }, - }); - } - + // Duplicate-param detection is query-string-specific (repeated same + // key), so it stays here; count cap + key format are shared with the + // MCP find_files tool via validateMetadataFilters. const filters: Record = {}; for (const param of metaParamKeys) { const key = param.slice("meta.".length); - if (!META_KEY_RE.test(key)) { - throw new ValidationError(`invalid metadata key: ${key}`, { - code: "file_metadata_invalid_key", - details: { key }, - }); - } const values = c.req.queries(param) ?? []; if (values.length > 1) { throw new ValidationError(`repeated metadata filter for key: ${key}`, { @@ -172,15 +161,18 @@ export const files = new Hono() } filters[key] = values[0] ?? query[param]; } + validateMetadataFilters(filters); const ws = c.get("workspace"); const limitParam = c.req.query("limit"); const limit = limitParam ? Number(limitParam) || undefined : undefined; - const matches = await findObjectsByMetadata(c.env.DB, c.get("workspaceName"), filters, { - prefix: query.prefix, - limit, - }); - const cfg = await storageConfig(c.env, ws); + const [cfg, matches] = await Promise.all([ + storageConfig(c.env, ws), + findObjectsByMetadata(c.env.DB, c.get("workspaceName"), filters, { + prefix: query.prefix, + limit, + }), + ]); return c.json({ items: matches.map((match) => { const urls = objectPublicUrls(c.env, cfg, match.key); diff --git a/apps/mcp/src/tools.ts b/apps/mcp/src/tools.ts index 7fac60a..9e96b4c 100644 --- a/apps/mcp/src/tools.ts +++ b/apps/mcp/src/tools.ts @@ -7,18 +7,21 @@ */ import { buildMarkdown, buildScreenshotKey } from "@buildinternet/uploads"; import { + METADATA_DESCRIPTION, + metadataProp, optPosInt, optString, + optStringArray, optStringRecord, usage, type McpTool, } from "@buildinternet/uploads/mcp"; import { badKey } from "@uploads/api/files"; import { - META_MAX_KEYS, findObjectsByMetadata, setFileMetadata, validateMetadataEntries, + validateMetadataFilters, } from "@uploads/api/file-metadata"; import { addExternalReference, @@ -52,25 +55,6 @@ export interface RemoteToolContext { authScopes: readonly FileScope[]; } -/** Shared tool-description text for the metadata-shaped params (mirrors packages/uploads/src/mcp/tools.ts). */ -const METADATA_DESCRIPTION = - "Queryable custom metadata (key→value), separate from provenance. Keys: lowercase, ^[a-z][a-z0-9._-]{0,63}$. Values: 1-512 printable ASCII characters. Caps: at most 24 keys, at most 8192 total key+value bytes. Suggested keys: app, url, page, device, resolution, commit, branch. `gh.*` is reserved by convention for GitHub PR/issue attachment context (repo/kind/number/ref)."; - -const metadataProp = { - type: "object", - additionalProperties: { type: "string" }, - description: METADATA_DESCRIPTION, -}; - -function optStringArray(args: Record, name: string): string[] | undefined { - const v = args[name]; - if (v === undefined || v === null) return undefined; - if (!Array.isArray(v) || v.some((item) => typeof item !== "string")) { - usage(`${name} must be an array of strings`); - } - return v as string[]; -} - function decodeBase64(value: string, maxBytes: number): Uint8Array { // Pre-decode size gate: base64 encodes 3 bytes per 4 chars, so a string // longer than this cannot decode to a within-limit payload. Rejecting here @@ -499,15 +483,15 @@ export function createRemoteTools(ctx: RemoteToolContext): McpTool[] { if (!filters || Object.keys(filters).length === 0) { usage("filters must have at least one key"); } - // Mirror the REST list endpoint's meta.* filter cap. - if (Object.keys(filters).length > META_MAX_KEYS) { - usage(`too many filters (max ${META_MAX_KEYS})`); - } - const cfg = await storageConfig(env, workspace); - const matches = await findObjectsByMetadata(env.DB, workspaceName, filters, { - prefix: optString(args, "prefix"), - limit: optPosInt(args, "limit"), - }); + // Shares the count cap + key-format checks with the REST list endpoint's meta.* filters. + validateMetadataFilters(filters); + const [cfg, matches] = await Promise.all([ + storageConfig(env, workspace), + findObjectsByMetadata(env.DB, workspaceName, filters, { + prefix: optString(args, "prefix"), + limit: optPosInt(args, "limit"), + }), + ]); return { items: matches.map((match) => ({ key: match.key, diff --git a/packages/uploads/src/mcp/args.ts b/packages/uploads/src/mcp/args.ts index 225e292..a0bfd3d 100644 --- a/packages/uploads/src/mcp/args.ts +++ b/packages/uploads/src/mcp/args.ts @@ -41,3 +41,27 @@ export function optStringRecord(args: ToolArgs, name: string): Record typeof item !== "string")) { + usage(`${name} must be an array of strings`); + } + return v as string[]; +} + +/** + * Shared tool-description text for the metadata-shaped `metadata`/`set`/ + * `filters` params across the CLI/local MCP (put/attach/set_metadata/ + * find_files) and the remote MCP worker (set_metadata/find_files). + */ +export const METADATA_DESCRIPTION = + "Queryable custom metadata (key→value), separate from provenance. Omit to leave any metadata already stored for this key untouched; pass an object (even {}) to fully replace it. Keys: lowercase, ^[a-z][a-z0-9._-]{0,63}$. Values: 1-512 printable ASCII characters. Caps: at most 24 keys, at most 8192 total key+value bytes. Suggested keys: app, url, page, device, resolution, commit, branch. `gh.*` is reserved by convention for GitHub PR/issue attachment context (repo/kind/number/ref)."; + +export const metadataProp = { + type: "object", + additionalProperties: { type: "string" }, + description: METADATA_DESCRIPTION, +}; diff --git a/packages/uploads/src/mcp/server.ts b/packages/uploads/src/mcp/server.ts index 74ee502..08043a6 100644 --- a/packages/uploads/src/mcp/server.ts +++ b/packages/uploads/src/mcp/server.ts @@ -9,7 +9,16 @@ */ import { UploadsError } from "../errors.js"; -export { optPosInt, optString, optStringRecord, usage, type ToolArgs } from "./args.js"; +export { + METADATA_DESCRIPTION, + metadataProp, + optPosInt, + optString, + optStringArray, + optStringRecord, + usage, + type ToolArgs, +} from "./args.js"; export interface McpTool { name: string; diff --git a/packages/uploads/src/mcp/tools.ts b/packages/uploads/src/mcp/tools.ts index db7bc4c..c190714 100644 --- a/packages/uploads/src/mcp/tools.ts +++ b/packages/uploads/src/mcp/tools.ts @@ -35,19 +35,18 @@ import { resolveRepo, type CommandRunner, } from "../github-gh.js"; -import { optPosInt, optString, optStringRecord, usage, type ToolArgs } from "./args.js"; +import { + METADATA_DESCRIPTION, + metadataProp, + optPosInt, + optString, + optStringArray, + optStringRecord, + usage, + type ToolArgs, +} from "./args.js"; import type { McpTool } from "./server.js"; -/** Shared tool-description text for the `metadata` object param on `put`/`attach`. */ -const METADATA_DESCRIPTION = - "Queryable custom metadata (key→value), separate from provenance. Omit to leave any metadata already stored for this key untouched; pass an object (even {}) to fully replace it. Keys: lowercase, ^[a-z][a-z0-9._-]{0,63}$. Values: 1-512 printable ASCII characters. Caps: at most 24 keys, at most 8192 total key+value bytes. Suggested keys: app, url, page, device, resolution, commit, branch. `gh.*` is reserved by convention for GitHub PR/issue attachment context (repo/kind/number/ref)."; - -const metadataProp = { - type: "object", - additionalProperties: { type: "string" }, - description: METADATA_DESCRIPTION, -}; - function optBool(args: ToolArgs, name: string): boolean { const v = args[name]; if (v === undefined || v === null) return false; @@ -55,15 +54,6 @@ function optBool(args: ToolArgs, name: string): boolean { return v; } -function optStringArray(args: ToolArgs, name: string): string[] | undefined { - const v = args[name]; - if (v === undefined || v === null) return undefined; - if (!Array.isArray(v) || v.some((item) => typeof item !== "string")) { - usage(`${name} must be an array of strings`); - } - return v as string[]; -} - function mcpOptimizeOptions( args: ToolArgs, defaults: { noOptimize?: boolean; keepExif?: boolean }, diff --git a/packages/uploads/src/metadata.ts b/packages/uploads/src/metadata.ts index ef54e59..d5f5761 100644 --- a/packages/uploads/src/metadata.ts +++ b/packages/uploads/src/metadata.ts @@ -25,12 +25,14 @@ const VALUE_SAFE_RE = /^[\x20-\x7E]+$/; const encoder = new TextEncoder(); /** - * Server-computed keys the API rejects as custom metadata (currently just - * `content-sha256`). `gh.*` is NOT reserved here: it's system-managed by - * convention (attach flow), not blocked — the server happily accepts - * user-supplied `gh.*` extras via `--meta`. + * Server-computed/server-reserved keys the API rejects as custom metadata: + * `content-sha256` (server-computed provenance) and `visibility` (names the + * R2-backed public/private gate, not a piece of D1 custom metadata — mirrors + * apps/api/src/file-metadata.ts's RESERVED_META_KEYS). `gh.*` is NOT reserved + * here: it's system-managed by convention (attach flow), not blocked — the + * server happily accepts user-supplied `gh.*` extras via `--meta`. */ -const RESERVED_META_KEYS = new Set(["content-sha256"]); +const RESERVED_META_KEYS = new Set(["content-sha256", "visibility"]); /** Throws `UsageError` with a readable message if `key`/`value` violate the metadata rules. */ export function validateMetaEntry(key: string, value: string): void { @@ -62,6 +64,19 @@ export function parseMetaPair(raw: string): [string, string] { return [key, value]; } +/** Throws `UsageError` if the aggregate UTF-8 key+value bytes of `entries` exceed `META_MAX_TOTAL_BYTES`. */ +function enforceMetaByteCap(entries: Array<[string, string]>): void { + let totalBytes = 0; + for (const [key, value] of entries) { + totalBytes += encoder.encode(key).byteLength + encoder.encode(value).byteLength; + } + if (totalBytes > META_MAX_TOTAL_BYTES) { + throw new UsageError( + `metadata too large: ${totalBytes} bytes of keys+values exceeds the ${META_MAX_TOTAL_BYTES}-byte limit per request`, + ); + } +} + /** * Parse and validate a batch of `k=v` pairs (e.g. every `--meta` occurrence, * or `meta set`'s positional pairs) into a map. Fails fast on the first @@ -80,25 +95,23 @@ export function parseMetaFlags(pairs: string[]): Record { } // Aggregate byte cap over the deduplicated map — same accounting as the // server (sum of UTF-8 key+value bytes, META_MAX_TOTAL_BYTES). - let totalBytes = 0; - for (const [key, value] of Object.entries(result)) { - totalBytes += encoder.encode(key).byteLength + encoder.encode(value).byteLength; - } - if (totalBytes > META_MAX_TOTAL_BYTES) { - throw new UsageError( - `metadata too large: ${totalBytes} bytes of keys+values exceeds the ${META_MAX_TOTAL_BYTES}-byte limit per request`, - ); - } + enforceMetaByteCap(Object.entries(result)); return result; } /** * Validates a pre-parsed key→value metadata map (e.g. an MCP tool's object - * argument) against the same rules as {@link parseMetaFlags}, without - * requiring "k=v" string parsing first. Reuses `parseMetaFlags` by - * reconstructing "k=v" pairs — safe even when a value contains "=", since - * `parseMetaPair` only splits on the first occurrence. Throws `UsageError`. + * argument) against the same rules as {@link parseMetaFlags} — the same + * per-entry validation (`validateMetaEntry`, which `parseMetaPair` also + * uses), the same key-count cap, and the same aggregate byte cap — without + * round-tripping through "k=v" string reconstruction and re-parsing first. + * Throws `UsageError`. */ export function validateMetaMap(meta: Record): void { - parseMetaFlags(Object.entries(meta).map(([key, value]) => `${key}=${value}`)); + const entries = Object.entries(meta); + if (entries.length > META_MAX_KEYS) { + throw new UsageError(`too many --meta pairs: at most ${META_MAX_KEYS} per request`); + } + for (const [key, value] of entries) validateMetaEntry(key, value); + enforceMetaByteCap(entries); } diff --git a/packages/uploads/test/metadata.test.ts b/packages/uploads/test/metadata.test.ts index 9f045e7..374b980 100644 --- a/packages/uploads/test/metadata.test.ts +++ b/packages/uploads/test/metadata.test.ts @@ -35,6 +35,10 @@ describe("parseMetaPair", () => { expect(() => parseMetaPair("content-sha256=abc")).toThrow(UsageError); }); + it("rejects the reserved visibility key", () => { + expect(() => parseMetaPair("visibility=private")).toThrow(UsageError); + }); + it("rejects an empty value", () => { expect(() => parseMetaPair("app=")).toThrow(UsageError); }); @@ -114,6 +118,10 @@ describe("validateMetaMap", () => { expect(() => validateMetaMap({ "content-sha256": "abc" })).toThrow(UsageError); }); + it("rejects the reserved visibility key", () => { + expect(() => validateMetaMap({ visibility: "private" })).toThrow(UsageError); + }); + it("preserves a value containing '=' (no re-split corruption)", () => { expect(() => validateMetaMap({ url: "https://example.com/a?b=c" })).not.toThrow(); }); From 1ee73ff950fa63e3f492b500c0d74e0e2a7876be Mon Sep 17 00:00:00 2001 From: Zach Dunn Date: Mon, 13 Jul 2026 20:24:20 -0400 Subject: [PATCH 22/23] refactor(api): atomic replaceFileMetadata for the full-replace-on-upload path putObject's full-replace path called deleteFileMetadata then setFileMetadata, which re-reads the now-guaranteed-empty map and re-validates before writing. Add replaceFileMetadata (one validateMetadataEntries call, then a single db.batch delete-all + upserts) and use it from putObject, making delete+set atomic in one batch as a side effect. setFileMetadata's merge/PATCH path is unchanged. Adds sqlite-backed tests for replace/clear/cap-rejection/atomicity. --- apps/api/src/file-metadata.ts | 35 +++++++ apps/api/src/files-core.ts | 11 +-- apps/api/test/file-metadata-sqlite.test.ts | 107 +++++++++++++++++++++ 3 files changed, 147 insertions(+), 6 deletions(-) diff --git a/apps/api/src/file-metadata.ts b/apps/api/src/file-metadata.ts index 623b3f9..85b1c0d 100644 --- a/apps/api/src/file-metadata.ts +++ b/apps/api/src/file-metadata.ts @@ -212,6 +212,41 @@ export async function deleteFileMetadata( .run(); } +/** + * Fully replaces an object's metadata: validates `metadata` once (there's no + * prior state to merge against, so unlike `setFileMetadata` there's nothing + * to re-read first), then deletes any existing rows and inserts the new set + * in a single `db.batch` — atomic, and without the wasted + * guaranteed-empty-map SELECT that `deleteFileMetadata` + `setFileMetadata` + * would otherwise incur. Used by `putObject`'s full-replace-on-upload path. + */ +export async function replaceFileMetadata( + db: D1Database, + workspace: string, + objectKey: string, + metadata: Record, +): Promise { + validateMetadataEntries(metadata); + + const now = new Date().toISOString(); + const statements = [ + db + .prepare(`DELETE FROM file_metadata WHERE workspace = ? AND object_key = ?`) + .bind(workspace, objectKey), + ...Object.entries(metadata).map(([key, value]) => + db + .prepare( + `INSERT INTO file_metadata (workspace, object_key, meta_key, meta_value, updated_at) + VALUES (?, ?, ?, ?, ?) + ON CONFLICT(workspace, object_key, meta_key) + DO UPDATE SET meta_value = excluded.meta_value, updated_at = excluded.updated_at`, + ) + .bind(workspace, objectKey, key, value, now), + ), + ]; + await db.batch(statements); +} + const FIND_DEFAULT_LIMIT = 50; const FIND_MAX_LIMIT = 500; diff --git a/apps/api/src/files-core.ts b/apps/api/src/files-core.ts index 7a6bcd0..7f6ad2b 100644 --- a/apps/api/src/files-core.ts +++ b/apps/api/src/files-core.ts @@ -13,7 +13,7 @@ import { } from "@uploads/errors"; import type { Files } from "@uploads/storage"; import { checkPutBudget } from "./budget"; -import { deleteFileMetadata, setFileMetadata, validateMetadataEntries } from "./file-metadata"; +import { deleteFileMetadata, replaceFileMetadata, validateMetadataEntries } from "./file-metadata"; import { DEFAULT_MAX_UPLOAD_BYTES, inspectUpload, resolveUploadPolicy } from "./guards"; import { checkKeyPolicy, resolveKeyPolicy } from "./key-policy"; import { @@ -193,11 +193,10 @@ export async function putObject( if (opts?.metadata) { // Full replace: an overwrite must not inherit a prior put's custom - // metadata, so clear the row set before (re-)writing this request's. - await deleteFileMetadata(env.DB, workspaceName, finalKey); - if (Object.keys(opts.metadata).length > 0) { - await setFileMetadata(env.DB, workspaceName, finalKey, opts.metadata); - } + // metadata, so clear the row set before (re-)writing this request's, in + // one atomic batch (replaceFileMetadata) rather than a delete followed + // by a separate re-read-then-write. + await replaceFileMetadata(env.DB, workspaceName, finalKey, opts.metadata); } await recordUsageSafe(env.DB, workspaceName, { diff --git a/apps/api/test/file-metadata-sqlite.test.ts b/apps/api/test/file-metadata-sqlite.test.ts index 18812b6..b615144 100644 --- a/apps/api/test/file-metadata-sqlite.test.ts +++ b/apps/api/test/file-metadata-sqlite.test.ts @@ -7,6 +7,7 @@ import { deleteFileMetadata, findObjectsByMetadata, getFileMetadata, + replaceFileMetadata, setFileMetadata, } from "../src/file-metadata"; import { SqliteD1, database } from "./helpers/sqlite-d1"; @@ -208,4 +209,110 @@ describe("file metadata persistence against SQLite", () => { sqlite.close(); } }); + + describe("replaceFileMetadata", () => { + it("fully replaces prior metadata rather than merging", async () => { + const sqlite = new SqliteD1(MIGRATION); + try { + await setFileMetadata(database(sqlite), "alpha", "f/one.png", { + app: "screenshots", + page: "/checkout", + }); + + await replaceFileMetadata(database(sqlite), "alpha", "f/one.png", { app: "other" }); + + await expect(getFileMetadata(database(sqlite), "alpha", "f/one.png")).resolves.toEqual({ + app: "other", + }); + } finally { + sqlite.close(); + } + }); + + it("clears all metadata when replacing with an empty map", async () => { + const sqlite = new SqliteD1(MIGRATION); + try { + await setFileMetadata(database(sqlite), "alpha", "f/one.png", { app: "screenshots" }); + + await replaceFileMetadata(database(sqlite), "alpha", "f/one.png", {}); + + await expect(getFileMetadata(database(sqlite), "alpha", "f/one.png")).resolves.toEqual({}); + } finally { + sqlite.close(); + } + }); + + it("does not touch other objects or workspaces", async () => { + const sqlite = new SqliteD1(MIGRATION); + try { + await setFileMetadata(database(sqlite), "alpha", "f/two.png", { app: "keep" }); + await setFileMetadata(database(sqlite), "beta", "f/one.png", { app: "keep" }); + + await replaceFileMetadata(database(sqlite), "alpha", "f/one.png", { app: "new" }); + + await expect(getFileMetadata(database(sqlite), "alpha", "f/two.png")).resolves.toEqual({ + app: "keep", + }); + await expect(getFileMetadata(database(sqlite), "beta", "f/one.png")).resolves.toEqual({ + app: "keep", + }); + } finally { + sqlite.close(); + } + }); + + it("rejects metadata over the caps, writing nothing", async () => { + const sqlite = new SqliteD1(MIGRATION); + try { + await setFileMetadata(database(sqlite), "alpha", "f/one.png", { app: "screenshots" }); + + const tooMany: Record = {}; + for (let i = 0; i < META_MAX_KEYS + 1; i++) tooMany[`k${i}`] = "v"; + + await expect( + replaceFileMetadata(database(sqlite), "alpha", "f/one.png", tooMany), + ).rejects.toBeInstanceOf(AppError); + + // Rejected replace must not have touched the prior state. + await expect(getFileMetadata(database(sqlite), "alpha", "f/one.png")).resolves.toEqual({ + app: "screenshots", + }); + } finally { + sqlite.close(); + } + }); + + it("is atomic: a batch failure leaves the prior metadata untouched", async () => { + const sqlite = new SqliteD1(MIGRATION); + try { + await setFileMetadata(database(sqlite), "alpha", "f/one.png", { app: "screenshots" }); + + // replaceFileMetadata's own validation can't fail mid-batch (it + // runs before the batch is built), so exercise atomicity directly + // against the transaction wrapper: a delete + insert followed by a + // statement that violates a real constraint should roll back the + // whole batch, including the delete and insert that preceded it. + await expect( + sqlite.batch([ + sqlite + .prepare(`DELETE FROM file_metadata WHERE workspace = ? AND object_key = ?`) + .bind("alpha", "f/one.png"), + sqlite + .prepare( + `INSERT INTO file_metadata (workspace, object_key, meta_key, meta_value, updated_at) VALUES (?, ?, ?, ?, ?)`, + ) + .bind("alpha", "f/one.png", "app", "replaced", "2026-07-13T00:00:00.000Z"), + sqlite.prepare(`INSERT INTO no_such_table (x) VALUES (1)`), + ]), + ).rejects.toThrow(); + + // Rolled back: the DELETE + INSERT from the failed batch never committed. + await expect(getFileMetadata(database(sqlite), "alpha", "f/one.png")).resolves.toEqual({ + app: "screenshots", + }); + } finally { + sqlite.close(); + } + }); + }); }); From a328f3ca4bc97478c23e7ba348b2e725052099c6 Mon Sep 17 00:00:00 2001 From: Zach Dunn Date: Mon, 13 Jul 2026 20:49:24 -0400 Subject: [PATCH 23/23] fix(uploads,api,web): address CodeRabbit review findings - SKILL.md: warn that non-gh.* --meta/MCP metadata values render publicly on the /f/ file page. - CLI: reject --cursor with metadata search (list --meta / find), which previously accepted and silently ignored it. - mcp/args.ts optStringRecord: use a null-prototype accumulator so a __proto__ metadata key can't be silently dropped by the inherited setter. - mcp/tools.ts attach + commands.ts runAttach: validate the merged metadata (extras + gh.* pairs) against the key/byte caps, not just the extras alone, so the merge can't silently exceed the cap server-side. - files-core.ts putObject: record usage before writing custom metadata, so a failing metadata batch no longer leaves the ledger under-counted. --- apps/api/src/files-core.ts | 16 ++- .../routes-files-usage-resilience.test.ts | 108 ++++++++++++++++++ packages/uploads/src/commands.ts | 9 +- packages/uploads/src/mcp/args.ts | 7 +- packages/uploads/src/mcp/tools.ts | 7 +- packages/uploads/test/commands-attach.test.ts | 10 ++ packages/uploads/test/commands-find.test.ts | 7 ++ packages/uploads/test/commands-list.test.ts | 7 ++ packages/uploads/test/mcp-args.test.ts | 37 ++++++ packages/uploads/test/mcp.test.ts | 19 +++ skills/uploads-cli/SKILL.md | 4 + 11 files changed, 221 insertions(+), 10 deletions(-) create mode 100644 apps/api/test/routes-files-usage-resilience.test.ts create mode 100644 packages/uploads/test/mcp-args.test.ts diff --git a/apps/api/src/files-core.ts b/apps/api/src/files-core.ts index 7f6ad2b..e141a6c 100644 --- a/apps/api/src/files-core.ts +++ b/apps/api/src/files-core.ts @@ -191,6 +191,16 @@ export async function putObject( metadata: storageMetadata, }); + // Usage accounting first: the object is already durably stored above, so + // the ledger must be updated regardless of whether the metadata batch + // below succeeds — otherwise a metadata failure leaves bytes/objects + // stored but under-counted (recordUsageSafe never throws). + await recordUsageSafe(env.DB, workspaceName, { + bytes: deltaBytes, + objects: prev === null ? 1 : 0, + uploads: 1, + }); + if (opts?.metadata) { // Full replace: an overwrite must not inherit a prior put's custom // metadata, so clear the row set before (re-)writing this request's, in @@ -199,12 +209,6 @@ export async function putObject( await replaceFileMetadata(env.DB, workspaceName, finalKey, opts.metadata); } - await recordUsageSafe(env.DB, workspaceName, { - bytes: deltaBytes, - objects: prev === null ? 1 : 0, - uploads: 1, - }); - const cfg = await storageConfig(env, ws); const urls = objectPublicUrls(env, cfg, finalKey); return { diff --git a/apps/api/test/routes-files-usage-resilience.test.ts b/apps/api/test/routes-files-usage-resilience.test.ts new file mode 100644 index 0000000..be077c0 --- /dev/null +++ b/apps/api/test/routes-files-usage-resilience.test.ts @@ -0,0 +1,108 @@ +import { beforeAll, describe, expect, it } from "vitest"; +import { FakeR2Bucket } from "./fake-r2"; +import { UsageFakeD1 } from "./usage-fake-d1"; +import { app } from "../src/index"; +import { sha256Hex, type WorkspaceRecord } from "../src/workspace"; + +const TOKEN = "secret-token"; +const PNG = new Uint8Array([0x89, 0x50, 0x4e, 0x47, 0x0d, 0x0a, 0x1a, 0x0a, 1, 2, 3, 4]); + +beforeAll(() => { + if (!(crypto.subtle as SubtleCrypto & { timingSafeEqual?: unknown }).timingSafeEqual) { + Object.defineProperty(crypto.subtle, "timingSafeEqual", { + value: (left: ArrayBufferView, right: ArrayBufferView) => { + const a = new Uint8Array(left.buffer, left.byteOffset, left.byteLength); + const b = new Uint8Array(right.buffer, right.byteOffset, right.byteLength); + if (a.length !== b.length) return false; + let difference = 0; + for (let index = 0; index < a.length; index++) difference |= a[index] ^ b[index]; + return difference === 0; + }, + }); + } +}); + +/** + * Wraps `UsageFakeD1` (which keeps a real workspace_usage ledger) so + * `batch()` throws for the `file_metadata` write batch specifically — + * simulating replaceFileMetadata's db.batch() failing (e.g. a transient D1 + * error) on an otherwise-valid request. `prepare` and `batch` on + * `UsageFakeD1` are instance fields (arrow functions), not prototype + * methods, so this composes over an instance rather than subclassing (a + * `super.prepare()` call would not resolve). Tags each prepared statement + * with its source SQL at `prepare()` time so `batch()` can tell which table + * a given call's statements target. + */ +function failingMetadataBatchD1() { + const inner = new UsageFakeD1(); + const prepare = (sql: string) => { + const stmt = inner.prepare(sql) as unknown as Record; + stmt.__sql = sql; + return stmt; + }; + const batch = async (statements: { run: () => Promise; __sql?: string }[]) => { + if (statements.some((s) => s.__sql?.includes("file_metadata"))) { + throw new Error("simulated D1 batch failure"); + } + return inner.batch(statements); + }; + return { inner, prepare, batch }; +} + +async function makeEnv(db: { prepare: unknown; batch: unknown }) { + const record: WorkspaceRecord = { + provider: "r2", + bucket: "uploads-default", + binding: "UPLOADS_DEFAULT", + prefix: "default/", + publicBaseUrl: "https://storage.uploads.sh", + tokenHash: await sha256Hex(TOKEN), + }; + const bucket = new FakeR2Bucket(); + return { + env: { + REGISTRY: { get: async () => record, put: async () => undefined }, + DB: db, + UPLOADS_DEFAULT: bucket, + WRITE_LIMITER: { limit: async () => ({ success: true }) }, + }, + bucket, + }; +} + +const auth = { Authorization: `Bearer ${TOKEN}` }; + +describe("PUT /v1/:workspace/files usage accounting survives a metadata failure", () => { + it("still records usage when the custom-metadata D1 batch throws", async () => { + const { inner: db, prepare, batch } = failingMetadataBatchD1(); + const { env, bucket } = await makeEnv({ prepare, batch }); + + const res = await app.request( + "/v1/default/files/screenshots/shot.png", + { + method: "PUT", + headers: { + ...auth, + "Content-Type": "image/png", + "X-Uploads-Meta-App": "myapp", + }, + body: PNG, + }, + env, + ); + + // The metadata batch failure propagates as a 5xx — the object is stored + // (files-core stores to R2 before touching D1) but the response reports + // the failure rather than a false 201. + expect(res.status).toBeGreaterThanOrEqual(500); + expect(bucket.store.has("default/screenshots/shot.png")).toBe(true); + + // The whole point of the fix: usage accounting isn't skipped just + // because the metadata write failed afterward. + const usageRow = db.usage.get("default"); + expect(usageRow).toBeDefined(); + expect(usageRow?.bytes).toBe(PNG.byteLength); + expect(usageRow?.objects).toBe(1); + expect(usageRow?.uploads_in_period).toBe(1); + }); +}); diff --git a/packages/uploads/src/commands.ts b/packages/uploads/src/commands.ts index 326a511..c32c8c2 100644 --- a/packages/uploads/src/commands.ts +++ b/packages/uploads/src/commands.ts @@ -20,7 +20,7 @@ import { buildMarkdown } from "./embed.js"; import { urlForGithubEmbed } from "./public-urls.js"; import { UploadsError } from "./errors.js"; import { writeJson, writeStdout } from "./io.js"; -import { parseMetaFlags } from "./metadata.js"; +import { parseMetaFlags, validateMetaMap } from "./metadata.js"; import { ghAttachmentKey, ghKeyPrefix, @@ -394,8 +394,12 @@ export async function runAttach( const contentTypeOverride = flagString(parsed.flags, "--content-type"); // User-supplied extras first, then the resolved target's gh.* — explicit // target pairs always win over a same-named --meta extra (documented above). + // Validate the merged map (not just the extras) so the 24-key/8KB caps are + // enforced client-side even when extras alone are under the cap but extras + // + the 4 gh.* pairs push the merged map over it. const metaExtras = parseMetaFlags(flagValues(parsed.flags, "--meta")); const metadata = { ...metaExtras, ...ghMetadataFromTarget(target) }; + if (Object.keys(metadata).length > 0) validateMetaMap(metadata); const results = []; for (const file of parsed.positionals) { if (file === "-") @@ -919,6 +923,9 @@ async function runFindFiles( filters: Record, flags: CommandFlags["flags"], ): Promise { + if (flagString(flags, "--cursor") !== undefined) { + throw new UsageError("--cursor is not supported with metadata filters"); + } const prefix = flagString(flags, "--prefix"); const limit = flagInt(flags, "--limit", "--limit"); const result = await ctx.client.findFiles(filters, { prefix, limit }); diff --git a/packages/uploads/src/mcp/args.ts b/packages/uploads/src/mcp/args.ts index a0bfd3d..d25d8f1 100644 --- a/packages/uploads/src/mcp/args.ts +++ b/packages/uploads/src/mcp/args.ts @@ -34,7 +34,12 @@ export function optStringRecord(args: ToolArgs, name: string): Record = {}; + // Object.create(null): a plain `{}` would silently drop a `__proto__` key + // (it hits the inherited setter instead of becoming an own property), + // turning a malicious/malformed key into a no-op rather than a rejected + // input. A null-prototype object makes every key a real own property so + // downstream validation (e.g. META_KEY_RE) sees and rejects it. + const result = Object.create(null) as Record; for (const [key, value] of Object.entries(v as Record)) { if (typeof value !== "string") usage(`${name}.${key} must be a string`); result[key] = value; diff --git a/packages/uploads/src/mcp/tools.ts b/packages/uploads/src/mcp/tools.ts index c190714..ee2afcc 100644 --- a/packages/uploads/src/mcp/tools.ts +++ b/packages/uploads/src/mcp/tools.ts @@ -577,10 +577,13 @@ export function createUploadsMcpTools(opts: { const optimizeOpts = mcpOptimizeOptions(args, defaults); // User-supplied extras first, then the resolved target's gh.* — // explicit target pairs always win over a same-named metadata extra - // (mirrors runAttach in ../commands.js). + // (mirrors runAttach in ../commands.js). Validate the merged map (not + // just the extras) so the 24-key/8KB caps are enforced client-side — + // extras alone might pass while extras + the 4 gh.* pairs exceed the + // cap, which would otherwise only be caught server-side after upload. const metaExtras = optStringRecord(args, "metadata") ?? {}; - if (Object.keys(metaExtras).length > 0) validateMetaMap(metaExtras); const metadata = { ...metaExtras, ...ghMetadataFromTarget(target) }; + if (Object.keys(metadata).length > 0) validateMetaMap(metadata); const uploads = []; for (const file of files) { diff --git a/packages/uploads/test/commands-attach.test.ts b/packages/uploads/test/commands-attach.test.ts index 10921b7..970a90c 100644 --- a/packages/uploads/test/commands-attach.test.ts +++ b/packages/uploads/test/commands-attach.test.ts @@ -173,4 +173,14 @@ describe("runAttach gh.* metadata", () => { expect(metadata?.app).toBe("myapp"); expect(metadata?.["gh.repo"]).toBe("buildinternet/uploads"); }); + + it("rejects when 22 extras + the 4 automatic gh.* pairs exceed the 24-key cap", async () => { + const { client } = fakeClient(); + const { run } = ghRunner(); + const metaFlags: string[] = []; + for (let i = 0; i < 22; i++) metaFlags.push("--meta", `k${i}=v`); + await expect( + runAttach(ctxWith(client), [...files("shot.png"), ...metaFlags], false, run), + ).rejects.toThrow(UsageError); + }); }); diff --git a/packages/uploads/test/commands-find.test.ts b/packages/uploads/test/commands-find.test.ts index 7506d4b..b5abd26 100644 --- a/packages/uploads/test/commands-find.test.ts +++ b/packages/uploads/test/commands-find.test.ts @@ -68,4 +68,11 @@ describe("runFind", () => { expect(calls[0].prefix).toBe("screenshots/"); expect(calls[0].limit).toBe(5); }); + + it("rejects --cursor with metadata filters", async () => { + const { client } = fakeClient(); + await expect(runFind(ctxWith(client), ["app=myapp", "--cursor", "abc"], false)).rejects.toThrow( + UsageError, + ); + }); }); diff --git a/packages/uploads/test/commands-list.test.ts b/packages/uploads/test/commands-list.test.ts index 0a4d02e..91c8eb1 100644 --- a/packages/uploads/test/commands-list.test.ts +++ b/packages/uploads/test/commands-list.test.ts @@ -115,4 +115,11 @@ describe("runList --meta", () => { runList(ctxWith(client), ["--meta", "app=x", "--all"], false, noRun), ).rejects.toThrow(UsageError); }); + + it("rejects --meta combined with --cursor", async () => { + const { client } = fakeFindClient(); + await expect( + runList(ctxWith(client), ["--meta", "app=x", "--cursor", "abc"], false, noRun), + ).rejects.toThrow(UsageError); + }); }); diff --git a/packages/uploads/test/mcp-args.test.ts b/packages/uploads/test/mcp-args.test.ts new file mode 100644 index 0000000..305e636 --- /dev/null +++ b/packages/uploads/test/mcp-args.test.ts @@ -0,0 +1,37 @@ +import { describe, expect, it } from "vitest"; +import { UsageError } from "../src/cli-args.js"; +import { optStringRecord } from "../src/mcp/args.js"; +import { validateMetaMap } from "../src/metadata.js"; + +describe("optStringRecord", () => { + it("parses a plain string-value object", () => { + expect(optStringRecord({ metadata: { app: "myapp", page: "settings" } }, "metadata")).toEqual({ + app: "myapp", + page: "settings", + }); + }); + + it("returns undefined when the arg is absent", () => { + expect(optStringRecord({}, "metadata")).toBeUndefined(); + }); + + it("does not silently drop a __proto__ key (prototype-pollution guard)", () => { + // Use JSON.parse, not an object literal: `{ __proto__: "x" }` in source + // is special-cased by the language to set the prototype rather than + // create an own property, which would falsely pass this test. Real MCP + // input arrives JSON-parsed (JSON.parse creates a genuine own property + // named "__proto__" via CreateDataProperty), which is the case that + // actually reaches optStringRecord. + const args = JSON.parse('{"metadata":{"__proto__":"x"}}') as { metadata: unknown }; + const result = optStringRecord(args, "metadata"); + // A plain `{}` accumulator would have result["__proto__"] = v hit the + // inherited setter and vanish, leaving `{}` — which downstream code + // treats as "clear all metadata" instead of an invalid key. With a + // null-prototype accumulator, __proto__ becomes a real own key that + // reaches (and is rejected by) metadata key validation. + expect(result).toBeDefined(); + expect(Object.prototype.hasOwnProperty.call(result, "__proto__")).toBe(true); + expect(Object.keys(result!)).toContain("__proto__"); + expect(() => validateMetaMap(result!)).toThrow(UsageError); + }); +}); diff --git a/packages/uploads/test/mcp.test.ts b/packages/uploads/test/mcp.test.ts index ef3825f..afe2fca 100644 --- a/packages/uploads/test/mcp.test.ts +++ b/packages/uploads/test/mcp.test.ts @@ -566,6 +566,25 @@ describe("tools/call attach", () => { }); expect(puts[0].metadata?.["gh.repo"]).toBe("buildinternet/uploads"); }); + + it("rejects when 22 extras + the 4 automatic gh.* pairs exceed the 24-key cap", async () => { + const { run } = ghRunner(); + const { server, puts } = serverWith({ runner: run }); + const dir = mkdtempSync(join(tmpdir(), "uploads-mcp-test-")); + const file = join(dir, "before.png"); + writeFileSync(file, "png"); + + const metadata: Record = {}; + for (let i = 0; i < 22; i++) metadata[`k${i}`] = "v"; + + const res = await rpc(server, "tools/call", { + name: "attach", + arguments: { files: [file], metadata }, + }); + expect(res.result.isError).toBe(true); + expect(res.result.content[0].text).toContain("too many"); + expect(puts.length).toBe(0); + }); }); describe("tools/call list, delete, comment", () => { diff --git a/skills/uploads-cli/SKILL.md b/skills/uploads-cli/SKILL.md index fe30319..f5deb4f 100644 --- a/skills/uploads-cli/SKILL.md +++ b/skills/uploads-cli/SKILL.md @@ -233,6 +233,10 @@ that file's entire metadata set (delete-then-set, not a merge); re-uploading **without** `--meta` at all preserves the existing metadata untouched. Use `uploads meta set` to edit individual keys without disturbing the rest. +Non-`gh.*` metadata values supplied via `--meta` (CLI) or `metadata` (MCP) render +on the object's public `/f/` file page. Treat them like the URL itself: +don't put internal notes, secrets, tokens, IDs, or private paths in them. + ## Public media galleries Use galleries when several existing public uploads should be shared as one ordered collection.