From f05a290194be791df95727ed01f6cd9974314d28 Mon Sep 17 00:00:00 2001 From: Cursor Agent Date: Wed, 28 Oct 2026 03:38:29 +0000 Subject: [PATCH 1/5] feat: record classifier results and hold releases in an append-only log Safety flags stay on ai.* rows. Each classification and each hold release appends a file_events row, including the content hash, so the history survives an overwrite. Co-authored-by: Zach Dunn --- .../migrations/20261028120000_file_events.sql | 46 +++ apps/api/src/classifier.ts | 90 +++- apps/api/src/file-events.ts | 389 ++++++++++++++++++ apps/api/src/file-metadata.ts | 3 +- apps/api/src/files-core.ts | 2 + apps/api/src/retention.ts | 2 + apps/api/src/routes/workspace-files.ts | 30 +- apps/api/src/upload-hold-read.ts | 56 ++- apps/api/src/upload-hold-service.ts | 26 +- apps/api/src/upload-hold.ts | 10 +- apps/api/src/workspace-teardown.ts | 2 + apps/api/test/classifier-upload.test.ts | 109 +++++ apps/api/test/file-events.test.ts | 160 +++++++ apps/api/test/usage-fake-d1.ts | 81 ++++ apps/web/src/content/docs/ai-labels.mdx | 4 +- docs/ops.md | 31 +- 16 files changed, 999 insertions(+), 42 deletions(-) create mode 100644 apps/api/migrations/20261028120000_file_events.sql create mode 100644 apps/api/src/file-events.ts create mode 100644 apps/api/test/file-events.test.ts diff --git a/apps/api/migrations/20261028120000_file_events.sql b/apps/api/migrations/20261028120000_file_events.sql new file mode 100644 index 000000000..d840c20cf --- /dev/null +++ b/apps/api/migrations/20261028120000_file_events.sql @@ -0,0 +1,46 @@ +-- Append-only log of classifier attempts and member hold overrides +-- (issue #1136, storage redesign step 1). +-- +-- Rows are inserted and never updated. An overwrite clears the current +-- `ai.*` rows and then appends another event; it does not delete the +-- earlier ones. Object delete, retention, and workspace teardown remove +-- the rows for keys that no longer exist. +-- +-- `content_sha256` is the bytes this event describes. It is the future +-- blob id for content-addressed storage and is not a foreign key. An +-- override applies to a key only while that key's current +-- `file_content_hash` equals `content_sha256`. A null hash applies only +-- while the key has no hash. A different hash is a different set of bytes +-- and the override is ignored. +-- +-- `kind` is classified, reused_exact, reused_perceptual, skipped, failed, +-- or override. `action` is release or hold, and only on an override. +-- Safety columns are the answers in force for that attempt. They are a +-- copy. Nothing here writes `file_metadata`. +CREATE TABLE file_events ( + id INTEGER PRIMARY KEY AUTOINCREMENT, + workspace TEXT NOT NULL, + object_key TEXT NOT NULL, + content_sha256 TEXT, + kind TEXT NOT NULL, + classifier_version TEXT, + model TEXT, + reason TEXT, + action TEXT, + actor TEXT, + rating TEXT, + rating_confidence TEXT, + content_flag TEXT, + content_confidence TEXT, + secret TEXT, + secret_confidence TEXT, + pii TEXT, + pii_confidence TEXT, + created_at TEXT NOT NULL +); + +-- History of one key, oldest first. Also serves the latest-override read. +CREATE INDEX file_events_key_idx ON file_events (workspace, object_key, id); + +-- Future blob lookup: every event that described these bytes. +CREATE INDEX file_events_hash_idx ON file_events (workspace, content_sha256, id); diff --git a/apps/api/src/classifier.ts b/apps/api/src/classifier.ts index 9211ba3c9..41d2f7654 100644 --- a/apps/api/src/classifier.ts +++ b/apps/api/src/classifier.ts @@ -21,7 +21,9 @@ * A call that times out, errors, or answers below the confidence floor * writes `ai.classifier=failed` and `ai.error`, but only when these bytes * have no earlier result. New bytes clear every classifier row first - * (`CLASSIFIER_ROW_KEYS`). + * (`CLASSIFIER_ROW_KEYS`). Each attempt also appends a `file_events` row + * (`classified`, `reused_exact`, `reused_perceptual`, `skipped`, `failed`). + * The log keeps the earlier attempt after those rows are cleared. * * Before calling, a finished classification in the same workspace can stand * in: identical bytes reuse all rows (including safety flags and holds); an @@ -40,6 +42,7 @@ */ import { contentHashDonorKeys, contentHashForKey } from "./content-hash"; +import { appendClassificationEvent, type ClassificationEventKind } from "./file-events"; import { PHASH_META_KEY, SAFETY_FLAG_KEYS, @@ -1159,6 +1162,32 @@ function classifierVersionMarker(provider: ClassifierProvider, resized: boolean) return resized ? CLASSIFIER_RESIZED_VERSION : CLASSIFIER_VERSION; } +function classifierModel(provider: ClassifierProvider): string { + return provider === "luna" ? CLASSIFIER_LUNA_MODEL : CLASSIFIER_JEV_MODEL; +} + +/** History of this attempt. Never throws, and never writes `ai.*`. */ +async function recordClassificationEvent( + env: Env, + workspaceName: string, + key: string, + bytes: Uint8Array, + kind: ClassificationEventKind, + meta: Record | undefined, + extra: { model?: string | null; reason?: string | null } = {}, +): Promise { + await appendClassificationEvent(env, { + workspace: workspaceName, + objectKey: key, + bytes, + kind, + classifierVersion: meta?.["ai.classifier"] ?? null, + model: extra.model ?? null, + reason: extra.reason ?? null, + safety: meta, + }); +} + /** Display labels a skip must not leave behind from an earlier classification. */ const CLASSIFIER_DISPLAY_KEYS = ["ai.kind", "ai.surface", "ai.screen", "ai.ready"] as const; @@ -1178,6 +1207,7 @@ async function storeSkip( key: string, reason: string, provider: ClassifierProvider, + bytes: Uint8Array, ): Promise> { const db = dbFor(env); await deleteServerFileMetadataKeys(db, workspaceName, key, [ @@ -1186,6 +1216,7 @@ async function storeSkip( ]); const meta = skipMeta(reason, provider); await setServerFileMetadata(db, workspaceName, key, meta); + await recordClassificationEvent(env, workspaceName, key, bytes, "skipped", meta, { reason }); return meta; } @@ -1212,14 +1243,33 @@ async function storeFailure( key: string, reason: ClassifierErrorReason, provider: ClassifierProvider, + bytes: Uint8Array, ): Promise | undefined> { console.log({ event: "classifier_call_failed", reason, provider }); + const model = classifierModel(provider); const db = dbFor(env); const existing = (await getFileMetadata(db, workspaceName, key))["ai.classifier"]; - if (existing !== undefined && existing !== CLASSIFIER_FAILED) return undefined; + // A retry that fails on bytes we already labeled must not erase them. + // The attempt is still an event, with no safety snapshot of its own. + if (existing !== undefined && existing !== CLASSIFIER_FAILED) { + await recordClassificationEvent( + env, + workspaceName, + key, + bytes, + "failed", + { "ai.classifier": CLASSIFIER_FAILED }, + { model, reason }, + ); + return undefined; + } await deleteServerFileMetadataKeys(db, workspaceName, key, ["ai.skip"]); const meta = { "ai.classifier": CLASSIFIER_FAILED, "ai.error": reason }; await setServerFileMetadata(db, workspaceName, key, meta); + await recordClassificationEvent(env, workspaceName, key, bytes, "failed", meta, { + model, + reason, + }); return meta; } @@ -1421,6 +1471,7 @@ async function matchPerceptualReuse( workspaceName: string, key: string, phash: string, + bytes: Uint8Array, ): Promise | undefined> { const hits = await findObjectsByMetadata( dbFor(env), @@ -1433,7 +1484,9 @@ async function matchPerceptualReuse( const meta = perceptualReuseMeta(hit.metadata, phash); if (!meta) continue; console.log({ event: "classifier_reused", match: "phash", workspace: workspaceName }); - return storeDecision(env, workspaceName, key, meta); + const stored = await storeDecision(env, workspaceName, key, meta); + await recordClassificationEvent(env, workspaceName, key, bytes, "reused_perceptual", stored); + return stored; } return undefined; } @@ -1478,7 +1531,9 @@ async function tryReuseClassification( ]); if (recorded === sha && hasReuseBody(own)) { console.log({ event: "classifier_reused", match: "sha256", workspace: workspaceName }); - return { meta: classifierRowsOf(own), hold: true }; + const meta = classifierRowsOf(own); + await recordClassificationEvent(env, workspaceName, key, bytes, "reused_exact", meta); + return { meta, hold: true }; } const phash = imagePhash(bytes, contentType); @@ -1486,11 +1541,13 @@ async function tryReuseClassification( const meta = exactReuseMeta(await getFileMetadata(db, workspaceName, donorKey), phash); if (!meta) continue; console.log({ event: "classifier_reused", match: "sha256", workspace: workspaceName }); - return { meta: await storeDecision(env, workspaceName, key, meta), hold: true }; + const stored = await storeDecision(env, workspaceName, key, meta); + await recordClassificationEvent(env, workspaceName, key, bytes, "reused_exact", stored); + return { meta: stored, hold: true }; } if (phash) { - const meta = await matchPerceptualReuse(env, workspaceName, key, phash); + const meta = await matchPerceptualReuse(env, workspaceName, key, phash, bytes); if (meta) return { meta, hold: false }; } return { hold: false, phash }; @@ -1521,7 +1578,7 @@ export async function classifyAndStore( if (!(await classificationAllowed(env, ws, workspaceName))) return undefined; const provider = await resolveClassifierProvider(env, ws, workspaceName); const trivial = trivialImageSkip(contentType, bytes); - if (trivial) return await storeSkip(env, workspaceName, key, trivial, provider); + if (trivial) return await storeSkip(env, workspaceName, key, trivial, provider, bytes); const reused = await tryReuseClassification(env, workspaceName, key, bytes, contentType); if (reused.meta) { @@ -1555,7 +1612,7 @@ export async function classifyAndStore( if (!resizeController.signal.aborted) { console.error({ event: "classifier_image_transform_failed", err }); } - return await storeSkip(env, workspaceName, key, reason, provider); + return await storeSkip(env, workspaceName, key, reason, provider, bytes); } if (!prepared.image) { @@ -1565,6 +1622,7 @@ export async function classifyAndStore( key, prepared.skip ?? CLASSIFIER_SKIP_UNSUPPORTED, provider, + bytes, ); } @@ -1573,7 +1631,7 @@ export async function classifyAndStore( phash = imagePhash(prepared.hashable.bytes, prepared.hashable.contentType); if (phash) { try { - const meta = await matchPerceptualReuse(env, workspaceName, key, phash); + const meta = await matchPerceptualReuse(env, workspaceName, key, phash, bytes); if (meta) { return finishReused( env, @@ -1611,13 +1669,23 @@ export async function classifyAndStore( ? CLASSIFIER_ERROR_TIMEOUT : CLASSIFIER_ERROR_PROVIDER; console.error({ event: "classifier_failed", workspace: workspaceName, key, reason, err }); - return await storeFailure(env, workspaceName, key, reason, provider); + return await storeFailure(env, workspaceName, key, reason, provider, bytes); } if (!meta) { - return await storeFailure(env, workspaceName, key, CLASSIFIER_ERROR_NO_ANSWER, provider); + return await storeFailure( + env, + workspaceName, + key, + CLASSIFIER_ERROR_NO_ANSWER, + provider, + bytes, + ); } if (phash) meta[PHASH_META_KEY] = phash; const stored = await storeDecision(env, workspaceName, key, meta); + await recordClassificationEvent(env, workspaceName, key, bytes, "classified", stored, { + model: classifierModel(provider), + }); // Loaded lazily: the hold re-syncs the PR comment, and the comment // service imports files-core, which imports this module. const { applyUploadHold, resyncCommentAfterClassification } = diff --git a/apps/api/src/file-events.ts b/apps/api/src/file-events.ts new file mode 100644 index 000000000..7401877c3 --- /dev/null +++ b/apps/api/src/file-events.ts @@ -0,0 +1,389 @@ +/** + * Append-only classifier attempts and hold overrides (`file_events`). + * + * Current safety flags stay on `ai.*` rows. This log is the history that + * an overwrite does not erase, and the member release that must not edit + * those rows. `content_sha256` is the bytes the event describes, so a + * later blob id can use the same column without a rewrite. + * + * Writes are best-effort. A missing table (a database that has not migrated) + * or a test double that does not model it must not fail a classify or an + * approve. The `ai.*` rows remain the current result. + */ + +import { contentHashForKey } from "./content-hash"; +import { dbFor, type D1Queryable } from "./db-session"; +import { contentSha256Hex } from "./provenance"; + +export const CLASSIFICATION_EVENT_KINDS = [ + "classified", + "reused_exact", + "reused_perceptual", + "skipped", + "failed", +] as const; + +export type ClassificationEventKind = (typeof CLASSIFICATION_EVENT_KINDS)[number]; + +export type OverrideAction = "release" | "hold"; + +export interface FileEvent { + id: number; + contentSha256: string | null; + kind: string; + classifierVersion: string | null; + model: string | null; + reason: string | null; + action: string | null; + actor: string | null; + rating: string | null; + ratingConfidence: string | null; + contentFlag: string | null; + contentConfidence: string | null; + secret: string | null; + secretConfidence: string | null; + pii: string | null; + piiConfidence: string | null; + createdAt: string; +} + +export interface OverrideEvent { + action: string; + actor: string | null; + reason: string | null; + contentSha256: string | null; + createdAt: string; +} + +const INSERT_SQL = `INSERT INTO file_events ( + workspace, object_key, content_sha256, kind, classifier_version, model, + reason, action, actor, + rating, rating_confidence, content_flag, content_confidence, + secret, secret_confidence, pii, pii_confidence, created_at +) VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?)`; + +interface SafetyColumns { + rating: string | null; + ratingConfidence: string | null; + contentFlag: string | null; + contentConfidence: string | null; + secret: string | null; + secretConfidence: string | null; + pii: string | null; + piiConfidence: string | null; +} + +function text(value: string | null | undefined): string | null { + return value ?? null; +} + +function safetyOf(meta: Record | undefined): SafetyColumns { + const read = (key: string) => text(meta?.[key]); + return { + rating: read("ai.rating"), + ratingConfidence: read("ai.rating.confidence"), + contentFlag: read("ai.content"), + contentConfidence: read("ai.content.confidence"), + secret: read("ai.secret"), + secretConfidence: read("ai.secret.confidence"), + pii: read("ai.pii"), + piiConfidence: read("ai.pii.confidence"), + }; +} + +function safetyBinds(safety: SafetyColumns): Array { + return [ + safety.rating, + safety.ratingConfidence, + safety.contentFlag, + safety.contentConfidence, + safety.secret, + safety.secretConfidence, + safety.pii, + safety.piiConfidence, + ]; +} + +/** A missing table or an unmodeled test double. Real SQL errors still log. */ +function ignorableEventError(err: unknown): boolean { + const message = err instanceof Error ? err.message : String(err); + return message.includes("no such table") || message.includes("unsupported "); +} + +function logEventError(event: string, workspace: string, err: unknown): void { + if (ignorableEventError(err)) return; + console.error({ event, workspace, err }); +} + +async function insertEvent( + env: Env, + workspace: string, + objectKey: string, + contentSha256: string | null, + kind: string, + fields: { + classifierVersion?: string | null; + model?: string | null; + reason?: string | null; + action?: string | null; + actor?: string | null; + safety?: Record; + at: string; + }, +): Promise { + const safety = safetyOf(fields.safety); + try { + await dbFor(env) + .prepare(INSERT_SQL) + .bind( + workspace, + objectKey, + contentSha256, + kind, + text(fields.classifierVersion), + text(fields.model), + text(fields.reason), + text(fields.action), + text(fields.actor), + ...safetyBinds(safety), + fields.at, + ) + .run(); + } catch (err) { + logEventError("file_event_append_failed", workspace, err); + } +} + +/** + * One classifier attempt. `bytes` are the object body, so the stored hash + * matches `file_content_hash` for this put. Never throws. + */ +export async function appendClassificationEvent( + env: Env, + input: { + workspace: string; + objectKey: string; + bytes: Uint8Array; + kind: ClassificationEventKind; + classifierVersion?: string | null; + model?: string | null; + reason?: string | null; + safety?: Record; + }, +): Promise { + let contentSha256: string | null = null; + try { + contentSha256 = await contentSha256Hex(input.bytes); + } catch { + contentSha256 = null; + } + await insertEvent(env, input.workspace, input.objectKey, contentSha256, input.kind, { + classifierVersion: input.classifierVersion, + model: input.model, + reason: input.reason, + safety: input.safety, + at: new Date().toISOString(), + }); +} + +/** + * A member release or a hold-again. Copies the safety flags in force and + * does not write them. The hash is the key's current `file_content_hash`, + * so the override stops applying when those bytes change. Never throws. + */ +export async function appendOverrideEvent( + env: Env, + input: { + workspace: string; + objectKey: string; + action: OverrideAction; + actor?: string | null; + reason?: string | null; + safety?: Record; + at: string; + }, +): Promise { + const contentSha256 = await contentHashForKey(dbFor(env), input.workspace, input.objectKey); + await insertEvent(env, input.workspace, input.objectKey, contentSha256 ?? null, "override", { + action: input.action, + actor: input.actor, + reason: input.reason, + safety: input.safety, + at: input.at, + }); +} + +interface EventRow { + id: number; + content_sha256: string | null; + kind: string; + classifier_version: string | null; + model: string | null; + reason: string | null; + action: string | null; + actor: string | null; + rating: string | null; + rating_confidence: string | null; + content_flag: string | null; + content_confidence: string | null; + secret: string | null; + secret_confidence: string | null; + pii: string | null; + pii_confidence: string | null; + created_at: string; +} + +function eventFromRow(row: EventRow): FileEvent { + return { + id: Number(row.id), + contentSha256: row.content_sha256, + kind: row.kind, + classifierVersion: row.classifier_version, + model: row.model, + reason: row.reason, + action: row.action, + actor: row.actor, + rating: row.rating, + ratingConfidence: row.rating_confidence, + contentFlag: row.content_flag, + contentConfidence: row.content_confidence, + secret: row.secret, + secretConfidence: row.secret_confidence, + pii: row.pii, + piiConfidence: row.pii_confidence, + createdAt: row.created_at, + }; +} + +/** Oldest first. Empty when the table is missing. */ +export async function listFileEvents( + db: D1Queryable, + workspace: string, + objectKey: string, +): Promise { + try { + const result = await db + .prepare( + `SELECT id, content_sha256, kind, classifier_version, model, reason, action, actor, + rating, rating_confidence, content_flag, content_confidence, + secret, secret_confidence, pii, pii_confidence, created_at + FROM file_events + WHERE workspace = ? AND object_key = ? + ORDER BY id ASC + LIMIT 500`, + ) + .bind(workspace, objectKey) + .all(); + return (result.results ?? []).map(eventFromRow); + } catch (err) { + logEventError("file_event_read_failed", workspace, err); + return []; + } +} + +/** + * The newest override for this key, or null. Callers compare + * `contentSha256` to the key's current hash before treating it as current. + */ +export async function latestOverrideEvent( + db: D1Queryable, + workspace: string, + objectKey: string, +): Promise { + try { + const row = await db + .prepare( + `SELECT action, actor, reason, content_sha256, created_at + FROM file_events + WHERE workspace = ? AND object_key = ? AND kind = 'override' + ORDER BY id DESC + LIMIT 1`, + ) + .bind(workspace, objectKey) + .first<{ + action: string; + actor: string | null; + reason: string | null; + content_sha256: string | null; + created_at: string; + }>(); + if (!row) return null; + return { + action: row.action, + actor: row.actor, + reason: row.reason, + contentSha256: row.content_sha256, + createdAt: row.created_at, + }; + } catch (err) { + logEventError("file_event_read_failed", workspace, err); + return null; + } +} + +/** + * An override describes the bytes in `eventSha`. It applies while the key + * still points at those bytes. Null matches only a key with no hash. + */ +export function overrideApplies(eventSha: string | null, currentSha: string | null): boolean { + if (eventSha === null) return currentSha === null; + return eventSha === currentSha; +} + +async function deleteWhere(db: D1Queryable, sql: string, binds: unknown[]): Promise { + await db + .prepare(sql) + .bind(...binds) + .run(); +} + +/** Best-effort. Object delete drops the log for that key. */ +export async function deleteFileEvents( + db: D1Queryable, + workspace: string, + objectKey: string, +): Promise { + try { + await deleteWhere(db, `DELETE FROM file_events WHERE workspace = ? AND object_key = ?`, [ + workspace, + objectKey, + ]); + } catch (err) { + logEventError("file_event_delete_failed", workspace, err); + } +} + +/** Best-effort. Retention passes the keys it just removed from the bucket. */ +export async function deleteFileEventsForKeys( + db: D1Queryable, + workspace: string, + keys: string[], +): Promise { + if (keys.length === 0) return; + try { + const chunkSize = 90; + for (let i = 0; i < keys.length; i += chunkSize) { + const chunk = keys.slice(i, i + chunkSize); + const placeholders = chunk.map(() => "?").join(", "); + await deleteWhere( + db, + `DELETE FROM file_events WHERE workspace = ? AND object_key IN (${placeholders})`, + [workspace, ...chunk], + ); + } + } catch (err) { + logEventError("file_event_delete_failed", workspace, err); + } +} + +/** Best-effort. Workspace teardown must not leave another tenant's judgments. */ +export async function deleteFileEventsForWorkspace( + db: D1Queryable, + workspace: string, +): Promise { + try { + await deleteWhere(db, `DELETE FROM file_events WHERE workspace = ?`, [workspace]); + } catch (err) { + logEventError("file_event_delete_failed", workspace, err); + } +} diff --git a/apps/api/src/file-metadata.ts b/apps/api/src/file-metadata.ts index 1230e3fea..e0ae1b37b 100644 --- a/apps/api/src/file-metadata.ts +++ b/apps/api/src/file-metadata.ts @@ -56,7 +56,8 @@ export const SAFETY_FLAG_FIELDS = CLASSIFIER_SAFETY_FIELDS; * classification is scheduled. A classification that writes a value * (including a confident `none`) replaces that key; one that omits a key * leaves the old value. Object delete removes them. They don't bump - * `ai.classifier`. + * `ai.classifier`. The append-only history is `file_events`; an overwrite + * does not delete it. */ export const SAFETY_FLAG_KEYS: ReadonlySet = new Set( SAFETY_FLAG_FIELDS.flatMap((field) => [`ai.${field}`, `ai.${field}.confidence`]), diff --git a/apps/api/src/files-core.ts b/apps/api/src/files-core.ts index e2036128f..952dd5d5a 100644 --- a/apps/api/src/files-core.ts +++ b/apps/api/src/files-core.ts @@ -21,6 +21,7 @@ import { storageBudgetDenial, uploadBudgetDenial, } from "./budget"; +import { deleteFileEvents } from "./file-events"; import { deleteFileMetadata, deleteServerFileMetadataKeys, @@ -1321,6 +1322,7 @@ export async function deleteObject( }); if (heldSize !== null) hits.push({ size: heldSize, sharedLane: true }); await deleteFileMetadata(dbFor(env), workspaceName, key); + await deleteFileEvents(dbFor(env), workspaceName, key); // A held file going away ends its hold: rewrite the Slack DMs about it. if (heldSize !== null) await notifySlackHoldResolved(env, workspaceName, key, "deleted"); diff --git a/apps/api/src/retention.ts b/apps/api/src/retention.ts index 542785964..490b13832 100644 --- a/apps/api/src/retention.ts +++ b/apps/api/src/retention.ts @@ -16,6 +16,7 @@ */ import { positiveLimit } from "./budget"; import { deleteFileMetadataForKeys } from "./file-metadata"; +import { deleteFileEventsForKeys } from "./file-events"; import { deleteAttachmentsForKeysSafe, parseAttachmentKey } from "./github-attachment-index"; import { reconcileWorkspaceUsage, type ReconcileResult } from "./reconcile"; import { storage } from "./storage"; @@ -89,6 +90,7 @@ export async function purgeExpiredObjects( // Bulk form: native multi-delete on R2/S3 when available. await store.delete(batch); await deleteFileMetadataForKeys(dbFor(env), workspaceName, batch); + await deleteFileEventsForKeys(dbFor(env), workspaceName, batch); // Attachment index rows follow their objects (issue #934); chunked the // same way and best-effort, so a D1 hiccup never strands the sweep. // Filtered to keys that can actually hold a row — a retention sweep is diff --git a/apps/api/src/routes/workspace-files.ts b/apps/api/src/routes/workspace-files.ts index 822465449..6b306af4d 100644 --- a/apps/api/src/routes/workspace-files.ts +++ b/apps/api/src/routes/workspace-files.ts @@ -48,18 +48,8 @@ import { setObjectVisibility, } from "../files-core"; import { approveUpload, approverLabel, holdAgain } from "../upload-hold-service"; -import { listHeldKeys } from "../upload-hold-read"; -import { - HOLD_APPROVED, - HOLD_AREA_KEY, - HOLD_AT_KEY, - HOLD_BY_KEY, - HOLD_META_KEY, - HOLD_MOVED_KEY, - HOLD_PENDING, - parseHoldArea, - storedHoldReason, -} from "../upload-hold"; +import { effectiveHold, listHeldKeys } from "../upload-hold-read"; +import { HOLD_AREA_KEY, HOLD_MOVED_KEY, parseHoldArea, storedHoldReason } from "../upload-hold"; import { readHeld } from "../held-storage"; import { getFileMetadata, @@ -524,7 +514,8 @@ export const workspaceFiles = new Hono() async (c) => { const key = c.req.query("key") ?? ""; if (badKey(key)) throw new NotFoundError(); - const result = await holdAgain(c.env, c.get("workspace"), c.req.param("workspace"), key); + const by = await approverLabel(c.env, c.get("uploaderIdentity")); + const result = await holdAgain(c.env, c.get("workspace"), c.req.param("workspace"), key, by); return c.json({ key, ...result }); }, ) @@ -535,14 +526,15 @@ export const workspaceFiles = new Hono() .get("/:workspace/files/review", dualWorkspaceAuth(), scoped("files:read"), async (c) => { const key = c.req.query("key") ?? ""; if (badKey(key)) throw new NotFoundError(); - const meta = await getFileMetadata(dbFor(c.env), c.get("workspaceName"), key); - const hold = meta[HOLD_META_KEY]; + const workspaceName = c.get("workspaceName"); + const meta = await getFileMetadata(dbFor(c.env), workspaceName, key); + const view = await effectiveHold(c.env, workspaceName, key, meta); return c.json({ key, - hold: hold === HOLD_PENDING || hold === HOLD_APPROVED ? hold : null, - reason: hold ? storedHoldReason(meta) : null, - by: hold === HOLD_APPROVED ? (meta[HOLD_BY_KEY] ?? null) : null, - at: hold === HOLD_APPROVED ? (meta[HOLD_AT_KEY] ?? null) : null, + hold: view.hold, + reason: view.hold ? storedHoldReason(meta) : null, + by: view.by, + at: view.at, }); }) diff --git a/apps/api/src/upload-hold-read.ts b/apps/api/src/upload-hold-read.ts index ba5ecb55a..691a1e04b 100644 --- a/apps/api/src/upload-hold-read.ts +++ b/apps/api/src/upload-hold-read.ts @@ -6,9 +6,18 @@ */ import { dbFor } from "./db-session"; +import { contentHashForKey } from "./content-hash"; import { getMetadataForKeys } from "./file-metadata"; +import { latestOverrideEvent, overrideApplies } from "./file-events"; import { aiRows } from "./classifier-admin"; -import { HOLD_META_KEY, HOLD_PENDING, isHeld } from "./upload-hold"; +import { + HOLD_APPROVED, + HOLD_AT_KEY, + HOLD_BY_KEY, + HOLD_META_KEY, + HOLD_PENDING, + isHeld, +} from "./upload-hold"; /** The subset of `keys` held for approval. One batched D1 read. */ export async function heldKeysAmong( @@ -30,6 +39,51 @@ export async function isKeyHeld(env: Env, workspaceName: string, key: string): P return (await heldKeysAmong(env, workspaceName, [key])).has(key); } +export interface EffectiveHold { + hold: "pending" | "approved" | null; + by: string | null; + at: string | null; +} + +/** + * Hold state for a member view. `ai.hold` is the effective state for the + * current bytes (pending, approved, or absent). A release override for + * those same bytes supplies who released it and when. An override for a + * different hash is an earlier set of bytes and is ignored. A pending row + * stays pending: that is what comments and the public gate already read. + */ +export async function effectiveHold( + env: Env, + workspace: string, + objectKey: string, + meta: Record, +): Promise { + const row = meta[HOLD_META_KEY]; + const fromRow = (): EffectiveHold => { + if (row !== HOLD_PENDING && row !== HOLD_APPROVED) return { hold: null, by: null, at: null }; + const released = row === HOLD_APPROVED; + return { + hold: row, + by: released ? (meta[HOLD_BY_KEY] ?? null) : null, + at: released ? (meta[HOLD_AT_KEY] ?? null) : null, + }; + }; + if (row !== HOLD_APPROVED) return fromRow(); + const db = dbFor(env); + const [override, currentSha] = await Promise.all([ + latestOverrideEvent(db, workspace, objectKey), + contentHashForKey(db, workspace, objectKey), + ]); + if ( + !override || + override.action !== "release" || + !overrideApplies(override.contentSha256, currentSha ?? null) + ) { + return fromRow(); + } + return { hold: "approved", by: override.actor, at: override.createdAt }; +} + /** * Every key in the workspace held for approval. A moved hold has no public * object to list, so the comment and `uploads approve` find held files diff --git a/apps/api/src/upload-hold-service.ts b/apps/api/src/upload-hold-service.ts index 11451508f..73103e80a 100644 --- a/apps/api/src/upload-hold-service.ts +++ b/apps/api/src/upload-hold-service.ts @@ -12,6 +12,7 @@ import { ServiceUnavailableError } from "@uploads/errors"; import { classifierEvaluationContext } from "./classifier"; import { dbFor } from "./db-session"; +import { appendOverrideEvent } from "./file-events"; import { deleteServerFileMetadataKeys, getFileMetadata, @@ -125,6 +126,11 @@ export type ApproveUploadResult = { approved: true } | { approved: false; reason * Release a held file. A file that is not held (never flagged, already * approved, or no row for this key) is `not_held`, which the route reports * as a 200 so a retried approve is harmless. + * + * Safety flags (`ai.secret`, `ai.pii`, `ai.rating`, `ai.content`) stay as + * the classifier wrote them. The release is an override event (who, when, + * the flags in force, the content hash) plus `ai.hold=approved`, which is + * the effective hold every existing reader already checks. */ export async function approveUpload( env: Env, @@ -146,11 +152,20 @@ export async function approveUpload( } await deleteServerFileMetadataKeys(db, workspaceName, key, [HOLD_MOVED_KEY]); } + const at = new Date().toISOString(); await setServerFileMetadata(db, workspaceName, key, { [HOLD_META_KEY]: HOLD_APPROVED, - [HOLD_AT_KEY]: new Date().toISOString(), + [HOLD_AT_KEY]: at, ...(by ? { [HOLD_BY_KEY]: by } : {}), }); + await appendOverrideEvent(env, { + workspace: workspaceName, + objectKey: key, + action: "release", + actor: by ?? null, + safety: existing, + at, + }); console.log({ event: "upload_approved", workspace: workspaceName }); await Promise.all([ resyncAttachmentComment(env, ws, workspaceName, key, existing), @@ -174,6 +189,7 @@ export async function holdAgain( ws: WorkspaceRecord, workspaceName: string, key: string, + by?: string | null, ): Promise { const db = dbFor(env); const existing = await getFileMetadata(db, workspaceName, key); @@ -181,6 +197,14 @@ export async function holdAgain( // The row lands first, as in `applyUploadHold`. await setServerFileMetadata(db, workspaceName, key, { [HOLD_META_KEY]: HOLD_PENDING }); await deleteServerFileMetadataKeys(db, workspaceName, key, [HOLD_BY_KEY, HOLD_AT_KEY]); + await appendOverrideEvent(env, { + workspace: workspaceName, + objectKey: key, + action: "hold", + actor: by ?? null, + safety: existing, + at: new Date().toISOString(), + }); const moved = await moveToHeld(env, ws, workspaceName, key); console.log({ event: "upload_held_again", workspace: workspaceName, moved }); await resyncAttachmentComment(env, ws, workspaceName, key, existing); diff --git a/apps/api/src/upload-hold.ts b/apps/api/src/upload-hold.ts index 2edd047d7..82aeabecf 100644 --- a/apps/api/src/upload-hold.ts +++ b/apps/api/src/upload-hold.ts @@ -8,9 +8,13 @@ * with `POST /v1/workspaces/:workspace/files/approve`, which moves the * bytes back to the same key. * - * State is one server-owned row, `ai.hold`: `pending` or `approved`. It is - * in `CLASSIFIER_ROW_KEYS`, so it shares the classifier's lifetime: - * identical bytes keep it (an approval sticks), and new bytes clear it. + * State for the current bytes is one server-owned row, `ai.hold`: + * `pending` or `approved`. It is in `CLASSIFIER_ROW_KEYS`, so it shares + * the classifier's lifetime: identical bytes keep it (an approval sticks), + * and new bytes clear it. A release also appends an override event + * (`file-events.ts`) with who released it and the safety flags in force. + * That event does not change `ai.secret`, `ai.pii`, `ai.rating`, or + * `ai.content`, and it stays after new bytes clear the row. * * Env-free, so the CLI's local-gh comment path can share the predicate. */ diff --git a/apps/api/src/workspace-teardown.ts b/apps/api/src/workspace-teardown.ts index 037be96be..dfb6c8768 100644 --- a/apps/api/src/workspace-teardown.ts +++ b/apps/api/src/workspace-teardown.ts @@ -17,6 +17,7 @@ */ import { dbFor } from "./db-session"; import { deleteFileMetadataForWorkspace } from "./file-metadata"; +import { deleteFileEventsForWorkspace } from "./file-events"; import { deleteGalleriesForWorkspace } from "./galleries"; import { deleteFeedsForWorkspace } from "./feeds"; import { deleteAttachmentsForWorkspaceSafe } from "./github-attachment-index"; @@ -105,6 +106,7 @@ export async function teardownWorkspace( const { galleries } = await deleteGalleriesForWorkspace(dbFor(env), name); await deleteFeedsForWorkspace(dbFor(env), name); await deleteFileMetadataForWorkspace(dbFor(env), name); + await deleteFileEventsForWorkspace(dbFor(env), name); // The attachment index is workspace-scoped like file_metadata (issue // #934) — a torn-down workspace must not leave rows behind. await deleteAttachmentsForWorkspaceSafe(dbFor(env), name); diff --git a/apps/api/test/classifier-upload.test.ts b/apps/api/test/classifier-upload.test.ts index 4fc2556ae..27cec567d 100644 --- a/apps/api/test/classifier-upload.test.ts +++ b/apps/api/test/classifier-upload.test.ts @@ -5,6 +5,7 @@ import { deleteObject, putObject } from "../src/files-core"; import { contentSha256Hex } from "../src/provenance"; import { FakeR2Bucket } from "./fake-r2"; import { findObjectsByMetadata, getMetadataForKeys } from "../src/file-metadata"; +import { listFileEvents } from "../src/file-events"; import { imagePhash } from "../src/phash"; import { CLASSIFIER_JEV_MODEL, @@ -1248,6 +1249,10 @@ describe("safety flags", () => { expect(copied?.["ai.surface"]).toBe("desktop"); expect(copied?.["ai.screen"]).toBe("settings"); expect(copied?.["ai.classifier"]).toBe(CLASSIFIER_VERSION); + const reuseEvents = await listFileEvents(harness.env.DB, WORKSPACE, second.key); + expect(reuseEvents.map((event) => event.kind)).toEqual(["reused_perceptual"]); + expect(reuseEvents[0]?.secret).toBeNull(); + expect(reuseEvents[0]?.classifierVersion).toBe(CLASSIFIER_VERSION); for (const key of [ "ai.rating", "ai.rating.confidence", @@ -1768,3 +1773,107 @@ describe("held uploads move out of the public bucket", () => { expect(await rows(harness.env)).toBeUndefined(); }); }); + +describe("classifier event log", () => { + const key = "images/flagged.png"; + + it("appends a classification for each set of bytes and keeps the earlier event", async () => { + let calls = 0; + const harness = makeClassifierEnv(() => { + calls += 1; + if (calls === 1) return FLAGGED; + return { + answers: { + secret: jevAnswer("none", 0.93), + pii: jevAnswer("none", 0.91), + rating: jevAnswer("safe", 0.96), + content: jevAnswer("none", 0.94), + }, + }; + }); + const firstBytes = atLeast(pngOf(400, 280)); + const secondBytes = atLeast(pngOf(500, 280)); + await putFlagged(harness, firstBytes); + await putFlagged(harness, secondBytes, { replace: true }); + + const events = await listFileEvents(harness.env.DB, WORKSPACE, key); + expect(events.map((event) => event.kind)).toEqual(["classified", "classified"]); + expect(events[0]).toMatchObject({ + secret: "api_key", + rating: "explicit", + model: CLASSIFIER_JEV_MODEL, + classifierVersion: CLASSIFIER_VERSION, + contentSha256: await contentSha256Hex(firstBytes), + }); + expect(events[1]).toMatchObject({ + secret: "none", + rating: "safe", + contentFlag: "none", + contentSha256: await contentSha256Hex(secondBytes), + }); + const stored = (await getMetadataForKeys(harness.env.DB, WORKSPACE, [key])).get(key); + expect(stored?.["ai.secret"]).toBe("none"); + expect(stored?.["ai.rating"]).toBe("safe"); + }); + + it("appends reused_exact when the same bytes are classified again", async () => { + const harness = makeClassifierEnv(() => FLAGGED); + await putFlagged(harness, FITTING_PNG); + await putFlagged(harness, FITTING_PNG, { replace: true }); + const events = await listFileEvents(harness.env.DB, WORKSPACE, key); + expect(events.map((event) => event.kind)).toEqual(["classified", "reused_exact"]); + expect(events[1]).toMatchObject({ secret: "api_key", model: null }); + }); + + it("appends skipped when the image never reaches the model", async () => { + const { env, ws } = makeClassifierEnv(); + await classifyAndStore(env, ws, WORKSPACE, "images/tiny.png", pngOf(8, 8), "image/png"); + const events = await listFileEvents(env.DB, WORKSPACE, "images/tiny.png"); + expect(events).toMatchObject([{ kind: "skipped", reason: "small", model: null }]); + }); + + it("appends failed and keeps the classification of the bytes it replaced", async () => { + let calls = 0; + const harness = makeClassifierEnv(() => { + calls += 1; + if (calls === 1) return FLAGGED; + throw new Error("provider down"); + }); + await putFlagged(harness, atLeast(pngOf(400, 280))); + await putFlagged(harness, atLeast(pngOf(512, 300)), { replace: true }); + const events = await listFileEvents(harness.env.DB, WORKSPACE, key); + expect(events.map((event) => event.kind)).toEqual(["classified", "failed"]); + expect(events[0]?.secret).toBe("api_key"); + expect(events[1]).toMatchObject({ + reason: "provider_error", + model: CLASSIFIER_JEV_MODEL, + secret: null, + }); + const stored = (await getMetadataForKeys(harness.env.DB, WORKSPACE, [key])).get(key); + expect(stored?.["ai.secret"]).toBeUndefined(); + expect(stored?.["ai.classifier"]).toBe("failed"); + }); + + it("records a release override and leaves the safety flag in place", async () => { + const harness = makeClassifierEnv(() => FLAGGED); + await putFlagged(harness, FITTING_PNG, { enforceHold: true }); + expect(await approveUpload(harness.env, harness.ws, WORKSPACE, key, "Member")).toEqual({ + approved: true, + }); + const stored = (await getMetadataForKeys(harness.env.DB, WORKSPACE, [key])).get(key); + expect(stored?.["ai.secret"]).toBe("api_key"); + expect(stored?.["ai.pii"]).toBe("financial"); + expect(stored?.["ai.hold"]).toBe("approved"); + const events = await listFileEvents(harness.env.DB, WORKSPACE, key); + const override = events.find((event) => event.kind === "override"); + expect(override).toMatchObject({ + action: "release", + actor: "Member", + secret: "api_key", + pii: "financial", + rating: "explicit", + }); + expect(override?.createdAt).toBe(stored?.["ai.hold.at"]); + expect(override?.contentSha256).toMatch(/^[a-f0-9]{64}$/); + }); +}); diff --git a/apps/api/test/file-events.test.ts b/apps/api/test/file-events.test.ts new file mode 100644 index 000000000..effe0adab --- /dev/null +++ b/apps/api/test/file-events.test.ts @@ -0,0 +1,160 @@ +import { describe, expect, it } from "vitest"; +import { recordContentHash } from "../src/content-hash"; +import { + appendClassificationEvent, + appendOverrideEvent, + deleteFileEvents, + listFileEvents, +} from "../src/file-events"; +import { getFileMetadata, setFileMetadata, setServerFileMetadata } from "../src/file-metadata"; +import { contentSha256Hex } from "../src/provenance"; +import { approveUpload, holdAgain } from "../src/upload-hold-service"; +import { effectiveHold } from "../src/upload-hold-read"; +import type { WorkspaceRecord } from "../src/workspace"; +import { SqliteD1, database } from "./helpers/sqlite-d1"; + +const MIGRATIONS = [ + "migrations/20260713210559_file_metadata.sql", + "migrations/20260724140000_file_content_hash.sql", + "migrations/20261028120000_file_events.sql", +]; + +const ws: WorkspaceRecord = { + provider: "r2", + bucket: "uploads-default", + binding: "UPLOADS_DEFAULT", + prefix: "acme/", + publicBaseUrl: "https://storage.uploads.sh", +}; + +function makeDb() { + const sqlite = new SqliteD1(MIGRATIONS); + const db = database(sqlite); + const env = { DB: db } as Env; + return { sqlite, db, env }; +} + +describe("file_events", () => { + it("refuses a client set or delete of a safety flag", async () => { + const { sqlite, db } = makeDb(); + await setServerFileMetadata(db, "acme", "a.png", { "ai.secret": "api_key" }); + await expect(setFileMetadata(db, "acme", "a.png", { "ai.secret": "none" })).rejects.toThrow( + /reserved metadata key: ai\.secret/, + ); + await expect(setFileMetadata(db, "acme", "a.png", {}, ["ai.secret"])).rejects.toThrow( + /reserved metadata key: ai\.secret/, + ); + expect((await getFileMetadata(db, "acme", "a.png"))["ai.secret"]).toBe("api_key"); + sqlite.close(); + }); + + it("records a release override and leaves the safety flags", async () => { + const { sqlite, db, env } = makeDb(); + await setServerFileMetadata(db, "acme", "shots/a.png", { + "ai.hold": "pending", + "ai.secret": "api_key", + "ai.secret.confidence": "high", + "ai.pii": "financial", + "ai.rating": "explicit", + "ai.content": "sexual", + }); + await recordContentHash(db, "acme", "shots/a.png", "ab".repeat(32)); + expect(await approveUpload(env, ws, "acme", "shots/a.png", "Member")).toEqual({ + approved: true, + }); + + const stored = await getFileMetadata(db, "acme", "shots/a.png"); + expect(stored["ai.secret"]).toBe("api_key"); + expect(stored["ai.pii"]).toBe("financial"); + expect(stored["ai.rating"]).toBe("explicit"); + expect(stored["ai.content"]).toBe("sexual"); + expect(stored["ai.hold"]).toBe("approved"); + + const events = await listFileEvents(db, "acme", "shots/a.png"); + expect(events).toMatchObject([ + { + kind: "override", + action: "release", + actor: "Member", + secret: "api_key", + secretConfidence: "high", + pii: "financial", + rating: "explicit", + contentFlag: "sexual", + contentSha256: "ab".repeat(32), + }, + ]); + const view = await effectiveHold(env, "acme", "shots/a.png", stored); + expect(view).toEqual({ hold: "approved", by: "Member", at: stored["ai.hold.at"] }); + + expect(await holdAgain(env, ws, "acme", "shots/a.png", "Member")).toEqual({ held: true }); + const after = await getFileMetadata(db, "acme", "shots/a.png"); + expect(after["ai.hold"]).toBe("pending"); + expect(after["ai.secret"]).toBe("api_key"); + expect((await listFileEvents(db, "acme", "shots/a.png")).map((event) => event.action)).toEqual([ + "release", + "hold", + ]); + sqlite.close(); + }); + + it("keeps earlier events, and ignores an override once the hash changes", async () => { + const { sqlite, db, env } = makeDb(); + const first = new Uint8Array([1, 2, 3]); + const second = new Uint8Array([4, 5, 6]); + await appendClassificationEvent(env, { + workspace: "acme", + objectKey: "a.png", + bytes: first, + kind: "classified", + classifierVersion: "v5", + model: "jev", + safety: { "ai.secret": "api_key", "ai.secret.confidence": "high" }, + }); + await appendClassificationEvent(env, { + workspace: "acme", + objectKey: "a.png", + bytes: second, + kind: "failed", + classifierVersion: "failed", + model: "jev", + reason: "provider_error", + }); + const events = await listFileEvents(db, "acme", "a.png"); + expect(events.map((event) => event.kind)).toEqual(["classified", "failed"]); + expect(events[0]).toMatchObject({ + secret: "api_key", + contentSha256: await contentSha256Hex(first), + }); + expect(events[1]?.contentSha256).toBe(await contentSha256Hex(second)); + expect(events[0]?.contentSha256).not.toBe(events[1]?.contentSha256); + + await setServerFileMetadata(db, "acme", "a.png", { + "ai.hold": "approved", + "ai.hold.by": "Row", + "ai.hold.at": "2026-01-01T00:00:00.000Z", + }); + await recordContentHash(db, "acme", "a.png", "aa".repeat(32)); + await appendOverrideEvent(env, { + workspace: "acme", + objectKey: "a.png", + action: "release", + actor: "Event", + at: "2026-02-01T00:00:00.000Z", + }); + const matched = await getFileMetadata(db, "acme", "a.png"); + expect(await effectiveHold(env, "acme", "a.png", matched)).toMatchObject({ + hold: "approved", + by: "Event", + }); + await recordContentHash(db, "acme", "a.png", "bb".repeat(32)); + expect(await effectiveHold(env, "acme", "a.png", matched)).toMatchObject({ + hold: "approved", + by: "Row", + }); + + await deleteFileEvents(db, "acme", "a.png"); + expect(await listFileEvents(db, "acme", "a.png")).toEqual([]); + sqlite.close(); + }); +}); diff --git a/apps/api/test/usage-fake-d1.ts b/apps/api/test/usage-fake-d1.ts index de93fb71b..75ba45832 100644 --- a/apps/api/test/usage-fake-d1.ts +++ b/apps/api/test/usage-fake-d1.ts @@ -86,6 +86,9 @@ export class UsageFakeD1 { } /** `${workspace} ${objectKey}` → stored-body hash. Backs `file_content_hash`. */ private contentHashes = new Map(); + /** Append-only `file_events`. */ + private fileEvents: Array> = []; + private fileEventSeq = 1; prepare = (sql: string) => { const normalized = sql.replace(/\s+/g, " ").trim(); @@ -120,6 +123,16 @@ export class UsageFakeD1 { const row = this.contentHashes.get(`${workspace} ${objectKey}`); return row === undefined ? null : { content_sha256: row.hash }; } + if (normalized.includes("FROM file_events") && normalized.includes("kind = 'override'")) { + const [workspace, objectKey] = values as [string, string]; + const hits = this.fileEvents.filter( + (row) => + row.workspace === workspace && + row.object_key === objectKey && + row.kind === "override", + ); + return hits.length === 0 ? null : hits[hits.length - 1]; + } throw new Error(`unsupported first: ${normalized}`); }, all: async () => { @@ -144,6 +157,13 @@ export class UsageFakeD1 { if (normalized.includes("FROM galleries") || normalized.includes("gallery_")) { return { success: true as const, results: [] as T[], meta: {} }; } + if (normalized.includes("FROM file_events")) { + const [workspace, objectKey] = values as [string, string]; + const hits = this.fileEvents.filter( + (row) => row.workspace === workspace && row.object_key === objectKey, + ); + return { success: true as const, results: hits as T[], meta: {} }; + } if (normalized.includes("FROM file_content_hash h")) { const [workspace, hash, selfKey] = values as [string, string, string]; const prefix = `${workspace} `; @@ -184,6 +204,67 @@ export class UsageFakeD1 { if (claimResult) return claimResult; const attachmentRunResult = this.attachmentIndexTable.tryRun(normalized, values); if (attachmentRunResult) return attachmentRunResult; + if (normalized.startsWith("INSERT INTO file_events")) { + const [ + workspace, + objectKey, + contentSha256, + kind, + classifierVersion, + model, + reason, + action, + actor, + rating, + ratingConfidence, + contentFlag, + contentConfidence, + secret, + secretConfidence, + pii, + piiConfidence, + createdAt, + ] = values as Array; + this.fileEvents.push({ + id: this.fileEventSeq++, + workspace, + object_key: objectKey, + content_sha256: contentSha256, + kind, + classifier_version: classifierVersion, + model, + reason, + action, + actor, + rating, + rating_confidence: ratingConfidence, + content_flag: contentFlag, + content_confidence: contentConfidence, + secret, + secret_confidence: secretConfidence, + pii, + pii_confidence: piiConfidence, + created_at: createdAt, + }); + return { success: true as const, meta: { changes: 1 }, results: [] }; + } + if (normalized.startsWith("DELETE FROM file_events")) { + const workspace = values[0] as string; + if (normalized.includes("object_key IN")) { + const keys = new Set(values.slice(1) as string[]); + this.fileEvents = this.fileEvents.filter( + (row) => row.workspace !== workspace || !keys.has(row.object_key as string), + ); + } else if (normalized.includes("object_key = ?")) { + const objectKey = values[1] as string; + this.fileEvents = this.fileEvents.filter( + (row) => row.workspace !== workspace || row.object_key !== objectKey, + ); + } else { + this.fileEvents = this.fileEvents.filter((row) => row.workspace !== workspace); + } + return { success: true as const, meta: { changes: 1 }, results: [] }; + } if (normalized.startsWith("INSERT INTO file_content_hash")) { const [workspace, objectKey, hash, updatedAt] = values as [ string, diff --git a/apps/web/src/content/docs/ai-labels.mdx b/apps/web/src/content/docs/ai-labels.mdx index 4d5636a95..6f6f674cd 100644 --- a/apps/web/src/content/docs/ai-labels.mdx +++ b/apps/web/src/content/docs/ai-labels.mdx @@ -65,6 +65,8 @@ A stored flag stays until a later classification writes that same key. A confide The classifier version does not change for these flags. `v5`, `v6`, `v4`, and `skipped` still name how the image was prepared. A file labeled before the flags existed simply has none of these keys. +You cannot set, change, or delete these flags from the CLI, the API, or MCP. A member release does not change them either. It records a separate override: who released the hold, and when. +
@@ -94,7 +96,7 @@ curl -X POST -H "Authorization: Bearer $UPLOADS_TOKEN" \ With the [Slack app](/docs/slack#held-uploads) connected, the uploader (or, without one, the workspace admins) gets a Slack message for each new hold, with an **Approve** button. To get an email for each new hold instead, or as well, turn on **An upload is held for review** under Email notifications in your [account settings](/account/profile). It's off by default. -Approval writes `ai.hold=approved` and re-syncs the comment. Re-uploading identical bytes keeps the approval. New bytes are classified again and can be held again. `ai.pii=contact` and `ai.pii=other` never hold a file, and the `ai.rating` and `ai.content` flags never hold one. +Approval records that override, writes `ai.hold=approved`, and re-syncs the comment. The safety flags stay as the classifier wrote them. Re-uploading identical bytes keeps the approval. New bytes are classified again and can be held again. `ai.pii=contact` and `ai.pii=other` never hold a file, and the `ai.rating` and `ai.content` flags never hold one.
diff --git a/docs/ops.md b/docs/ops.md index 7ce565f0f..9929e3112 100644 --- a/docs/ops.md +++ b/docs/ops.md @@ -1422,11 +1422,32 @@ logs `classifier_hold_area` or `classifier_hold_area_failed` and leaves the hold as it is. `POST /v1/workspaces/:workspace/files/approve?key=` writes -`ai.hold=approved` and re-syncs the comment (`upload_approved`). `ai.hold` -is in `CLASSIFIER_ROW_KEYS`, so identical bytes keep an approval and new -bytes start over. Promote and attach copy every classifier row onto the -new key, since the copy is not classified again. `ai.rating`, -`ai.content`, and `ai.pii=contact` never hold a file. +`ai.hold=approved` and re-syncs the comment (`upload_approved`). It does +not change `ai.secret`, `ai.pii`, `ai.rating`, or `ai.content`. The same +call appends an override row to `file_events` (`apps/api/src/file-events.ts`): +who released the hold, when, the content hash, and the safety flags in +force. CLI, MCP, Slack, the file page, and `/admin/classifier` release all +go through `approveUpload`, so they all append that row. A client cannot +write the log, and cannot set or delete `ai.*`. + +`ai.hold` is in `CLASSIFIER_ROW_KEYS`, so identical bytes keep an approval +and new bytes start over. The event log keeps the earlier attempt either +way. Each classification appends +another row: `classified`, `reused_exact`, `reused_perceptual`, `skipped` +(with the reason), or `failed`, plus the model, the classifier version, +and the safety answers. New bytes still clear the current `ai.*` rows +before the next classification. The earlier events stay. `content_sha256` +on each row is the bytes that event describes, so a later content-addressed +blob can use the same column. An override applies only while the key's +current hash equals that column. Promote and attach copy every classifier +row onto the new key, since the copy is not classified again. They do not +copy events. The bytes' earlier classification is still findable by hash. +`ai.rating`, `ai.content`, and `ai.pii=contact` never hold a file. + +The table is on the existing D1 database. No new binding. A database that +has not applied the migration still classifies and still approves; the +append is skipped. Object delete, retention, and workspace teardown remove +the rows for keys that are gone. An overwrite does not. ### Allowlist (Flagship is the control plane) From 58ae6b44a4f0e173a0b950821e98c135dadd71a4 Mon Sep 17 00:00:00 2001 From: Cursor Agent Date: Wed, 28 Oct 2026 03:43:32 +0000 Subject: [PATCH 2/5] refactor: trim the classifier event log Drop the extra readers, the fake D1 table, and the repeated cases. The log, the release override, and the missing-migration no-op stay. Co-authored-by: Zach Dunn --- apps/api/src/classifier.ts | 3 - apps/api/src/file-events.ts | 330 ++++++------------------ apps/api/src/upload-hold-service.ts | 8 +- apps/api/test/classifier-upload.test.ts | 138 +++------- apps/api/test/file-events.test.ts | 159 +++--------- apps/api/test/usage-fake-d1.ts | 81 ------ docs/ops.md | 34 +-- 7 files changed, 175 insertions(+), 578 deletions(-) diff --git a/apps/api/src/classifier.ts b/apps/api/src/classifier.ts index 41d2f7654..d14893e55 100644 --- a/apps/api/src/classifier.ts +++ b/apps/api/src/classifier.ts @@ -1166,7 +1166,6 @@ function classifierModel(provider: ClassifierProvider): string { return provider === "luna" ? CLASSIFIER_LUNA_MODEL : CLASSIFIER_JEV_MODEL; } -/** History of this attempt. Never throws, and never writes `ai.*`. */ async function recordClassificationEvent( env: Env, workspaceName: string, @@ -1249,8 +1248,6 @@ async function storeFailure( const model = classifierModel(provider); const db = dbFor(env); const existing = (await getFileMetadata(db, workspaceName, key))["ai.classifier"]; - // A retry that fails on bytes we already labeled must not erase them. - // The attempt is still an event, with no safety snapshot of its own. if (existing !== undefined && existing !== CLASSIFIER_FAILED) { await recordClassificationEvent( env, diff --git a/apps/api/src/file-events.ts b/apps/api/src/file-events.ts index 7401877c3..36529258d 100644 --- a/apps/api/src/file-events.ts +++ b/apps/api/src/file-events.ts @@ -1,60 +1,7 @@ -/** - * Append-only classifier attempts and hold overrides (`file_events`). - * - * Current safety flags stay on `ai.*` rows. This log is the history that - * an overwrite does not erase, and the member release that must not edit - * those rows. `content_sha256` is the bytes the event describes, so a - * later blob id can use the same column without a rewrite. - * - * Writes are best-effort. A missing table (a database that has not migrated) - * or a test double that does not model it must not fail a classify or an - * approve. The `ai.*` rows remain the current result. - */ - import { contentHashForKey } from "./content-hash"; import { dbFor, type D1Queryable } from "./db-session"; import { contentSha256Hex } from "./provenance"; -export const CLASSIFICATION_EVENT_KINDS = [ - "classified", - "reused_exact", - "reused_perceptual", - "skipped", - "failed", -] as const; - -export type ClassificationEventKind = (typeof CLASSIFICATION_EVENT_KINDS)[number]; - -export type OverrideAction = "release" | "hold"; - -export interface FileEvent { - id: number; - contentSha256: string | null; - kind: string; - classifierVersion: string | null; - model: string | null; - reason: string | null; - action: string | null; - actor: string | null; - rating: string | null; - ratingConfidence: string | null; - contentFlag: string | null; - contentConfidence: string | null; - secret: string | null; - secretConfidence: string | null; - pii: string | null; - piiConfidence: string | null; - createdAt: string; -} - -export interface OverrideEvent { - action: string; - actor: string | null; - reason: string | null; - contentSha256: string | null; - createdAt: string; -} - const INSERT_SQL = `INSERT INTO file_events ( workspace, object_key, content_sha256, kind, classifier_version, model, reason, action, actor, @@ -62,66 +9,25 @@ const INSERT_SQL = `INSERT INTO file_events ( secret, secret_confidence, pii, pii_confidence, created_at ) VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?)`; -interface SafetyColumns { - rating: string | null; - ratingConfidence: string | null; - contentFlag: string | null; - contentConfidence: string | null; - secret: string | null; - secretConfidence: string | null; - pii: string | null; - piiConfidence: string | null; -} +export type ClassificationEventKind = + | "classified" + | "reused_exact" + | "reused_perceptual" + | "skipped" + | "failed"; -function text(value: string | null | undefined): string | null { - return value ?? null; -} - -function safetyOf(meta: Record | undefined): SafetyColumns { - const read = (key: string) => text(meta?.[key]); - return { - rating: read("ai.rating"), - ratingConfidence: read("ai.rating.confidence"), - contentFlag: read("ai.content"), - contentConfidence: read("ai.content.confidence"), - secret: read("ai.secret"), - secretConfidence: read("ai.secret.confidence"), - pii: read("ai.pii"), - piiConfidence: read("ai.pii.confidence"), - }; -} - -function safetyBinds(safety: SafetyColumns): Array { - return [ - safety.rating, - safety.ratingConfidence, - safety.contentFlag, - safety.contentConfidence, - safety.secret, - safety.secretConfidence, - safety.pii, - safety.piiConfidence, - ]; -} - -/** A missing table or an unmodeled test double. Real SQL errors still log. */ -function ignorableEventError(err: unknown): boolean { +function ignorable(err: unknown): boolean { const message = err instanceof Error ? err.message : String(err); return message.includes("no such table") || message.includes("unsupported "); } -function logEventError(event: string, workspace: string, err: unknown): void { - if (ignorableEventError(err)) return; - console.error({ event, workspace, err }); -} - async function insertEvent( env: Env, - workspace: string, - objectKey: string, - contentSha256: string | null, - kind: string, - fields: { + row: { + workspace: string; + objectKey: string; + contentSha256: string | null; + kind: string; classifierVersion?: string | null; model?: string | null; reason?: string | null; @@ -131,33 +37,38 @@ async function insertEvent( at: string; }, ): Promise { - const safety = safetyOf(fields.safety); + const meta = row.safety; + const read = (key: string) => meta?.[key] ?? null; try { await dbFor(env) .prepare(INSERT_SQL) .bind( - workspace, - objectKey, - contentSha256, - kind, - text(fields.classifierVersion), - text(fields.model), - text(fields.reason), - text(fields.action), - text(fields.actor), - ...safetyBinds(safety), - fields.at, + row.workspace, + row.objectKey, + row.contentSha256, + row.kind, + row.classifierVersion ?? null, + row.model ?? null, + row.reason ?? null, + row.action ?? null, + row.actor ?? null, + read("ai.rating"), + read("ai.rating.confidence"), + read("ai.content"), + read("ai.content.confidence"), + read("ai.secret"), + read("ai.secret.confidence"), + read("ai.pii"), + read("ai.pii.confidence"), + row.at, ) .run(); } catch (err) { - logEventError("file_event_append_failed", workspace, err); + if (ignorable(err)) return; + console.error({ event: "file_event_append_failed", workspace: row.workspace, err }); } } -/** - * One classifier attempt. `bytes` are the object body, so the stored hash - * matches `file_content_hash` for this put. Never throws. - */ export async function appendClassificationEvent( env: Env, input: { @@ -177,7 +88,11 @@ export async function appendClassificationEvent( } catch { contentSha256 = null; } - await insertEvent(env, input.workspace, input.objectKey, contentSha256, input.kind, { + await insertEvent(env, { + workspace: input.workspace, + objectKey: input.objectKey, + contentSha256, + kind: input.kind, classifierVersion: input.classifierVersion, model: input.model, reason: input.reason, @@ -186,17 +101,12 @@ export async function appendClassificationEvent( }); } -/** - * A member release or a hold-again. Copies the safety flags in force and - * does not write them. The hash is the key's current `file_content_hash`, - * so the override stops applying when those bytes change. Never throws. - */ export async function appendOverrideEvent( env: Env, input: { workspace: string; objectKey: string; - action: OverrideAction; + action: "release" | "hold"; actor?: string | null; reason?: string | null; safety?: Record; @@ -204,7 +114,11 @@ export async function appendOverrideEvent( }, ): Promise { const contentSha256 = await contentHashForKey(dbFor(env), input.workspace, input.objectKey); - await insertEvent(env, input.workspace, input.objectKey, contentSha256 ?? null, "override", { + await insertEvent(env, { + workspace: input.workspace, + objectKey: input.objectKey, + contentSha256: contentSha256 ?? null, + kind: "override", action: input.action, actor: input.actor, reason: input.reason, @@ -213,87 +127,20 @@ export async function appendOverrideEvent( }); } -interface EventRow { - id: number; - content_sha256: string | null; - kind: string; - classifier_version: string | null; - model: string | null; - reason: string | null; - action: string | null; - actor: string | null; - rating: string | null; - rating_confidence: string | null; - content_flag: string | null; - content_confidence: string | null; - secret: string | null; - secret_confidence: string | null; - pii: string | null; - pii_confidence: string | null; - created_at: string; -} - -function eventFromRow(row: EventRow): FileEvent { - return { - id: Number(row.id), - contentSha256: row.content_sha256, - kind: row.kind, - classifierVersion: row.classifier_version, - model: row.model, - reason: row.reason, - action: row.action, - actor: row.actor, - rating: row.rating, - ratingConfidence: row.rating_confidence, - contentFlag: row.content_flag, - contentConfidence: row.content_confidence, - secret: row.secret, - secretConfidence: row.secret_confidence, - pii: row.pii, - piiConfidence: row.pii_confidence, - createdAt: row.created_at, - }; -} - -/** Oldest first. Empty when the table is missing. */ -export async function listFileEvents( - db: D1Queryable, - workspace: string, - objectKey: string, -): Promise { - try { - const result = await db - .prepare( - `SELECT id, content_sha256, kind, classifier_version, model, reason, action, actor, - rating, rating_confidence, content_flag, content_confidence, - secret, secret_confidence, pii, pii_confidence, created_at - FROM file_events - WHERE workspace = ? AND object_key = ? - ORDER BY id ASC - LIMIT 500`, - ) - .bind(workspace, objectKey) - .all(); - return (result.results ?? []).map(eventFromRow); - } catch (err) { - logEventError("file_event_read_failed", workspace, err); - return []; - } -} - -/** - * The newest override for this key, or null. Callers compare - * `contentSha256` to the key's current hash before treating it as current. - */ export async function latestOverrideEvent( db: D1Queryable, workspace: string, objectKey: string, -): Promise { +): Promise<{ + action: string; + actor: string | null; + contentSha256: string | null; + createdAt: string; +} | null> { try { const row = await db .prepare( - `SELECT action, actor, reason, content_sha256, created_at + `SELECT action, actor, content_sha256, created_at FROM file_events WHERE workspace = ? AND object_key = ? AND kind = 'override' ORDER BY id DESC @@ -303,7 +150,6 @@ export async function latestOverrideEvent( .first<{ action: string; actor: string | null; - reason: string | null; content_sha256: string | null; created_at: string; }>(); @@ -311,79 +157,71 @@ export async function latestOverrideEvent( return { action: row.action, actor: row.actor, - reason: row.reason, contentSha256: row.content_sha256, createdAt: row.created_at, }; } catch (err) { - logEventError("file_event_read_failed", workspace, err); + if (ignorable(err)) return null; + console.error({ event: "file_event_read_failed", workspace, err }); return null; } } -/** - * An override describes the bytes in `eventSha`. It applies while the key - * still points at those bytes. Null matches only a key with no hash. - */ export function overrideApplies(eventSha: string | null, currentSha: string | null): boolean { if (eventSha === null) return currentSha === null; return eventSha === currentSha; } -async function deleteWhere(db: D1Queryable, sql: string, binds: unknown[]): Promise { - await db - .prepare(sql) - .bind(...binds) - .run(); +async function runDelete( + db: D1Queryable, + workspace: string, + sql: string, + binds: unknown[], +): Promise { + try { + await db + .prepare(sql) + .bind(...binds) + .run(); + } catch (err) { + if (ignorable(err)) return; + console.error({ event: "file_event_delete_failed", workspace, err }); + } } -/** Best-effort. Object delete drops the log for that key. */ export async function deleteFileEvents( db: D1Queryable, workspace: string, objectKey: string, ): Promise { - try { - await deleteWhere(db, `DELETE FROM file_events WHERE workspace = ? AND object_key = ?`, [ - workspace, - objectKey, - ]); - } catch (err) { - logEventError("file_event_delete_failed", workspace, err); - } + await runDelete(db, workspace, `DELETE FROM file_events WHERE workspace = ? AND object_key = ?`, [ + workspace, + objectKey, + ]); } -/** Best-effort. Retention passes the keys it just removed from the bucket. */ export async function deleteFileEventsForKeys( db: D1Queryable, workspace: string, keys: string[], ): Promise { if (keys.length === 0) return; - try { - const chunkSize = 90; - for (let i = 0; i < keys.length; i += chunkSize) { - const chunk = keys.slice(i, i + chunkSize); - const placeholders = chunk.map(() => "?").join(", "); - await deleteWhere( - db, - `DELETE FROM file_events WHERE workspace = ? AND object_key IN (${placeholders})`, - [workspace, ...chunk], - ); - } - } catch (err) { - logEventError("file_event_delete_failed", workspace, err); + const chunkSize = 90; + for (let i = 0; i < keys.length; i += chunkSize) { + const chunk = keys.slice(i, i + chunkSize); + const placeholders = chunk.map(() => "?").join(", "); + await runDelete( + db, + workspace, + `DELETE FROM file_events WHERE workspace = ? AND object_key IN (${placeholders})`, + [workspace, ...chunk], + ); } } -/** Best-effort. Workspace teardown must not leave another tenant's judgments. */ export async function deleteFileEventsForWorkspace( db: D1Queryable, workspace: string, ): Promise { - try { - await deleteWhere(db, `DELETE FROM file_events WHERE workspace = ?`, [workspace]); - } catch (err) { - logEventError("file_event_delete_failed", workspace, err); - } + await runDelete(db, workspace, `DELETE FROM file_events WHERE workspace = ?`, [workspace]); } diff --git a/apps/api/src/upload-hold-service.ts b/apps/api/src/upload-hold-service.ts index 73103e80a..079d1141d 100644 --- a/apps/api/src/upload-hold-service.ts +++ b/apps/api/src/upload-hold-service.ts @@ -125,12 +125,8 @@ export type ApproveUploadResult = { approved: true } | { approved: false; reason /** * Release a held file. A file that is not held (never flagged, already * approved, or no row for this key) is `not_held`, which the route reports - * as a 200 so a retried approve is harmless. - * - * Safety flags (`ai.secret`, `ai.pii`, `ai.rating`, `ai.content`) stay as - * the classifier wrote them. The release is an override event (who, when, - * the flags in force, the content hash) plus `ai.hold=approved`, which is - * the effective hold every existing reader already checks. + * as a 200 so a retried approve is harmless. Safety flags stay as + * classified. The release also appends an override event. */ export async function approveUpload( env: Env, diff --git a/apps/api/test/classifier-upload.test.ts b/apps/api/test/classifier-upload.test.ts index 27cec567d..cab097d63 100644 --- a/apps/api/test/classifier-upload.test.ts +++ b/apps/api/test/classifier-upload.test.ts @@ -5,7 +5,6 @@ import { deleteObject, putObject } from "../src/files-core"; import { contentSha256Hex } from "../src/provenance"; import { FakeR2Bucket } from "./fake-r2"; import { findObjectsByMetadata, getMetadataForKeys } from "../src/file-metadata"; -import { listFileEvents } from "../src/file-events"; import { imagePhash } from "../src/phash"; import { CLASSIFIER_JEV_MODEL, @@ -1249,10 +1248,6 @@ describe("safety flags", () => { expect(copied?.["ai.surface"]).toBe("desktop"); expect(copied?.["ai.screen"]).toBe("settings"); expect(copied?.["ai.classifier"]).toBe(CLASSIFIER_VERSION); - const reuseEvents = await listFileEvents(harness.env.DB, WORKSPACE, second.key); - expect(reuseEvents.map((event) => event.kind)).toEqual(["reused_perceptual"]); - expect(reuseEvents[0]?.secret).toBeNull(); - expect(reuseEvents[0]?.classifierVersion).toBe(CLASSIFIER_VERSION); for (const key of [ "ai.rating", "ai.rating.confidence", @@ -1775,105 +1770,52 @@ describe("held uploads move out of the public bucket", () => { }); describe("classifier event log", () => { - const key = "images/flagged.png"; - - it("appends a classification for each set of bytes and keeps the earlier event", async () => { - let calls = 0; - const harness = makeClassifierEnv(() => { - calls += 1; - if (calls === 1) return FLAGGED; + it("appends skip, classify, and reuse events across an overwrite", async () => { + const harness = makeClassifierEnv(() => FLAGGED); + const events: Array<{ kind: string; reason: string | null; secret: string | null }> = []; + const db = harness.env.DB as unknown as { prepare: (sql: string) => unknown }; + const prepare = db.prepare.bind(db); + db.prepare = (sql: string) => { + const normalized = sql.replace(/\s+/g, " ").trim(); + if (!normalized.includes("file_events")) return prepare(sql); + let values: unknown[] = []; return { - answers: { - secret: jevAnswer("none", 0.93), - pii: jevAnswer("none", 0.91), - rating: jevAnswer("safe", 0.96), - content: jevAnswer("none", 0.94), + bind(...next: unknown[]) { + values = next; + return this; + }, + async run() { + if (normalized.startsWith("INSERT INTO file_events")) { + events.push({ + kind: String(values[3]), + reason: (values[6] as string | null) ?? null, + secret: (values[13] as string | null) ?? null, + }); + } + return { success: true, meta: { changes: 0 }, results: [] }; + }, + async all() { + return { success: true, results: [], meta: {} }; + }, + async first() { + return null; }, }; - }); - const firstBytes = atLeast(pngOf(400, 280)); - const secondBytes = atLeast(pngOf(500, 280)); - await putFlagged(harness, firstBytes); - await putFlagged(harness, secondBytes, { replace: true }); - - const events = await listFileEvents(harness.env.DB, WORKSPACE, key); - expect(events.map((event) => event.kind)).toEqual(["classified", "classified"]); - expect(events[0]).toMatchObject({ - secret: "api_key", - rating: "explicit", - model: CLASSIFIER_JEV_MODEL, - classifierVersion: CLASSIFIER_VERSION, - contentSha256: await contentSha256Hex(firstBytes), - }); - expect(events[1]).toMatchObject({ - secret: "none", - rating: "safe", - contentFlag: "none", - contentSha256: await contentSha256Hex(secondBytes), - }); - const stored = (await getMetadataForKeys(harness.env.DB, WORKSPACE, [key])).get(key); - expect(stored?.["ai.secret"]).toBe("none"); - expect(stored?.["ai.rating"]).toBe("safe"); - }); + }; - it("appends reused_exact when the same bytes are classified again", async () => { - const harness = makeClassifierEnv(() => FLAGGED); - await putFlagged(harness, FITTING_PNG); + await putFlagged(harness, pngOf(8, 8)); + await putFlagged(harness, FITTING_PNG, { replace: true }); await putFlagged(harness, FITTING_PNG, { replace: true }); - const events = await listFileEvents(harness.env.DB, WORKSPACE, key); - expect(events.map((event) => event.kind)).toEqual(["classified", "reused_exact"]); - expect(events[1]).toMatchObject({ secret: "api_key", model: null }); - }); - - it("appends skipped when the image never reaches the model", async () => { - const { env, ws } = makeClassifierEnv(); - await classifyAndStore(env, ws, WORKSPACE, "images/tiny.png", pngOf(8, 8), "image/png"); - const events = await listFileEvents(env.DB, WORKSPACE, "images/tiny.png"); - expect(events).toMatchObject([{ kind: "skipped", reason: "small", model: null }]); - }); - - it("appends failed and keeps the classification of the bytes it replaced", async () => { - let calls = 0; - const harness = makeClassifierEnv(() => { - calls += 1; - if (calls === 1) return FLAGGED; - throw new Error("provider down"); - }); - await putFlagged(harness, atLeast(pngOf(400, 280))); - await putFlagged(harness, atLeast(pngOf(512, 300)), { replace: true }); - const events = await listFileEvents(harness.env.DB, WORKSPACE, key); - expect(events.map((event) => event.kind)).toEqual(["classified", "failed"]); - expect(events[0]?.secret).toBe("api_key"); - expect(events[1]).toMatchObject({ - reason: "provider_error", - model: CLASSIFIER_JEV_MODEL, - secret: null, - }); - const stored = (await getMetadataForKeys(harness.env.DB, WORKSPACE, [key])).get(key); - expect(stored?.["ai.secret"]).toBeUndefined(); - expect(stored?.["ai.classifier"]).toBe("failed"); - }); - it("records a release override and leaves the safety flag in place", async () => { - const harness = makeClassifierEnv(() => FLAGGED); - await putFlagged(harness, FITTING_PNG, { enforceHold: true }); - expect(await approveUpload(harness.env, harness.ws, WORKSPACE, key, "Member")).toEqual({ - approved: true, - }); - const stored = (await getMetadataForKeys(harness.env.DB, WORKSPACE, [key])).get(key); + expect(events).toEqual([ + { kind: "skipped", reason: "small", secret: null }, + { kind: "classified", reason: null, secret: "api_key" }, + { kind: "reused_exact", reason: null, secret: "api_key" }, + ]); + const stored = ( + await getMetadataForKeys(harness.env.DB, WORKSPACE, ["images/flagged.png"]) + ).get("images/flagged.png"); expect(stored?.["ai.secret"]).toBe("api_key"); - expect(stored?.["ai.pii"]).toBe("financial"); - expect(stored?.["ai.hold"]).toBe("approved"); - const events = await listFileEvents(harness.env.DB, WORKSPACE, key); - const override = events.find((event) => event.kind === "override"); - expect(override).toMatchObject({ - action: "release", - actor: "Member", - secret: "api_key", - pii: "financial", - rating: "explicit", - }); - expect(override?.createdAt).toBe(stored?.["ai.hold.at"]); - expect(override?.contentSha256).toMatch(/^[a-f0-9]{64}$/); + expect(stored?.["ai.skip"]).toBeUndefined(); }); }); diff --git a/apps/api/test/file-events.test.ts b/apps/api/test/file-events.test.ts index effe0adab..667526dd6 100644 --- a/apps/api/test/file-events.test.ts +++ b/apps/api/test/file-events.test.ts @@ -1,24 +1,10 @@ import { describe, expect, it } from "vitest"; import { recordContentHash } from "../src/content-hash"; -import { - appendClassificationEvent, - appendOverrideEvent, - deleteFileEvents, - listFileEvents, -} from "../src/file-events"; -import { getFileMetadata, setFileMetadata, setServerFileMetadata } from "../src/file-metadata"; -import { contentSha256Hex } from "../src/provenance"; -import { approveUpload, holdAgain } from "../src/upload-hold-service"; -import { effectiveHold } from "../src/upload-hold-read"; +import { getFileMetadata, setServerFileMetadata } from "../src/file-metadata"; +import { approveUpload } from "../src/upload-hold-service"; import type { WorkspaceRecord } from "../src/workspace"; import { SqliteD1, database } from "./helpers/sqlite-d1"; -const MIGRATIONS = [ - "migrations/20260713210559_file_metadata.sql", - "migrations/20260724140000_file_content_hash.sql", - "migrations/20261028120000_file_events.sql", -]; - const ws: WorkspaceRecord = { provider: "r2", bucket: "uploads-default", @@ -27,33 +13,18 @@ const ws: WorkspaceRecord = { publicBaseUrl: "https://storage.uploads.sh", }; -function makeDb() { - const sqlite = new SqliteD1(MIGRATIONS); - const db = database(sqlite); - const env = { DB: db } as Env; - return { sqlite, db, env }; -} - describe("file_events", () => { - it("refuses a client set or delete of a safety flag", async () => { - const { sqlite, db } = makeDb(); - await setServerFileMetadata(db, "acme", "a.png", { "ai.secret": "api_key" }); - await expect(setFileMetadata(db, "acme", "a.png", { "ai.secret": "none" })).rejects.toThrow( - /reserved metadata key: ai\.secret/, - ); - await expect(setFileMetadata(db, "acme", "a.png", {}, ["ai.secret"])).rejects.toThrow( - /reserved metadata key: ai\.secret/, - ); - expect((await getFileMetadata(db, "acme", "a.png"))["ai.secret"]).toBe("api_key"); - sqlite.close(); - }); - it("records a release override and leaves the safety flags", async () => { - const { sqlite, db, env } = makeDb(); + const sqlite = new SqliteD1([ + "migrations/20260713210559_file_metadata.sql", + "migrations/20260724140000_file_content_hash.sql", + "migrations/20261028120000_file_events.sql", + ]); + const db = database(sqlite); + const env = { DB: db } as Env; await setServerFileMetadata(db, "acme", "shots/a.png", { "ai.hold": "pending", "ai.secret": "api_key", - "ai.secret.confidence": "high", "ai.pii": "financial", "ai.rating": "explicit", "ai.content": "sexual", @@ -68,93 +39,41 @@ describe("file_events", () => { expect(stored["ai.pii"]).toBe("financial"); expect(stored["ai.rating"]).toBe("explicit"); expect(stored["ai.content"]).toBe("sexual"); - expect(stored["ai.hold"]).toBe("approved"); - - const events = await listFileEvents(db, "acme", "shots/a.png"); - expect(events).toMatchObject([ - { - kind: "override", - action: "release", - actor: "Member", - secret: "api_key", - secretConfidence: "high", - pii: "financial", - rating: "explicit", - contentFlag: "sexual", - contentSha256: "ab".repeat(32), - }, - ]); - const view = await effectiveHold(env, "acme", "shots/a.png", stored); - expect(view).toEqual({ hold: "approved", by: "Member", at: stored["ai.hold.at"] }); - - expect(await holdAgain(env, ws, "acme", "shots/a.png", "Member")).toEqual({ held: true }); - const after = await getFileMetadata(db, "acme", "shots/a.png"); - expect(after["ai.hold"]).toBe("pending"); - expect(after["ai.secret"]).toBe("api_key"); - expect((await listFileEvents(db, "acme", "shots/a.png")).map((event) => event.action)).toEqual([ - "release", - "hold", - ]); - sqlite.close(); - }); - - it("keeps earlier events, and ignores an override once the hash changes", async () => { - const { sqlite, db, env } = makeDb(); - const first = new Uint8Array([1, 2, 3]); - const second = new Uint8Array([4, 5, 6]); - await appendClassificationEvent(env, { - workspace: "acme", - objectKey: "a.png", - bytes: first, - kind: "classified", - classifierVersion: "v5", - model: "jev", - safety: { "ai.secret": "api_key", "ai.secret.confidence": "high" }, - }); - await appendClassificationEvent(env, { - workspace: "acme", - objectKey: "a.png", - bytes: second, - kind: "failed", - classifierVersion: "failed", - model: "jev", - reason: "provider_error", - }); - const events = await listFileEvents(db, "acme", "a.png"); - expect(events.map((event) => event.kind)).toEqual(["classified", "failed"]); - expect(events[0]).toMatchObject({ + const event = await db + .prepare( + `SELECT kind, action, actor, secret, content_sha256 FROM file_events + WHERE workspace = ? AND object_key = ?`, + ) + .bind("acme", "shots/a.png") + .first<{ + kind: string; + action: string; + actor: string; + secret: string; + content_sha256: string; + }>(); + expect(event).toEqual({ + kind: "override", + action: "release", + actor: "Member", secret: "api_key", - contentSha256: await contentSha256Hex(first), + content_sha256: "ab".repeat(32), }); - expect(events[1]?.contentSha256).toBe(await contentSha256Hex(second)); - expect(events[0]?.contentSha256).not.toBe(events[1]?.contentSha256); + sqlite.close(); + }); + it("still approves when the event table is missing", async () => { + const sqlite = new SqliteD1(["migrations/20260713210559_file_metadata.sql"]); + const db = database(sqlite); + const env = { DB: db } as Env; await setServerFileMetadata(db, "acme", "a.png", { - "ai.hold": "approved", - "ai.hold.by": "Row", - "ai.hold.at": "2026-01-01T00:00:00.000Z", - }); - await recordContentHash(db, "acme", "a.png", "aa".repeat(32)); - await appendOverrideEvent(env, { - workspace: "acme", - objectKey: "a.png", - action: "release", - actor: "Event", - at: "2026-02-01T00:00:00.000Z", - }); - const matched = await getFileMetadata(db, "acme", "a.png"); - expect(await effectiveHold(env, "acme", "a.png", matched)).toMatchObject({ - hold: "approved", - by: "Event", - }); - await recordContentHash(db, "acme", "a.png", "bb".repeat(32)); - expect(await effectiveHold(env, "acme", "a.png", matched)).toMatchObject({ - hold: "approved", - by: "Row", + "ai.hold": "pending", + "ai.secret": "api_key", }); - - await deleteFileEvents(db, "acme", "a.png"); - expect(await listFileEvents(db, "acme", "a.png")).toEqual([]); + expect(await approveUpload(env, ws, "acme", "a.png", "Member")).toEqual({ approved: true }); + const stored = await getFileMetadata(db, "acme", "a.png"); + expect(stored["ai.secret"]).toBe("api_key"); + expect(stored["ai.hold"]).toBe("approved"); sqlite.close(); }); }); diff --git a/apps/api/test/usage-fake-d1.ts b/apps/api/test/usage-fake-d1.ts index 75ba45832..de93fb71b 100644 --- a/apps/api/test/usage-fake-d1.ts +++ b/apps/api/test/usage-fake-d1.ts @@ -86,9 +86,6 @@ export class UsageFakeD1 { } /** `${workspace} ${objectKey}` → stored-body hash. Backs `file_content_hash`. */ private contentHashes = new Map(); - /** Append-only `file_events`. */ - private fileEvents: Array> = []; - private fileEventSeq = 1; prepare = (sql: string) => { const normalized = sql.replace(/\s+/g, " ").trim(); @@ -123,16 +120,6 @@ export class UsageFakeD1 { const row = this.contentHashes.get(`${workspace} ${objectKey}`); return row === undefined ? null : { content_sha256: row.hash }; } - if (normalized.includes("FROM file_events") && normalized.includes("kind = 'override'")) { - const [workspace, objectKey] = values as [string, string]; - const hits = this.fileEvents.filter( - (row) => - row.workspace === workspace && - row.object_key === objectKey && - row.kind === "override", - ); - return hits.length === 0 ? null : hits[hits.length - 1]; - } throw new Error(`unsupported first: ${normalized}`); }, all: async () => { @@ -157,13 +144,6 @@ export class UsageFakeD1 { if (normalized.includes("FROM galleries") || normalized.includes("gallery_")) { return { success: true as const, results: [] as T[], meta: {} }; } - if (normalized.includes("FROM file_events")) { - const [workspace, objectKey] = values as [string, string]; - const hits = this.fileEvents.filter( - (row) => row.workspace === workspace && row.object_key === objectKey, - ); - return { success: true as const, results: hits as T[], meta: {} }; - } if (normalized.includes("FROM file_content_hash h")) { const [workspace, hash, selfKey] = values as [string, string, string]; const prefix = `${workspace} `; @@ -204,67 +184,6 @@ export class UsageFakeD1 { if (claimResult) return claimResult; const attachmentRunResult = this.attachmentIndexTable.tryRun(normalized, values); if (attachmentRunResult) return attachmentRunResult; - if (normalized.startsWith("INSERT INTO file_events")) { - const [ - workspace, - objectKey, - contentSha256, - kind, - classifierVersion, - model, - reason, - action, - actor, - rating, - ratingConfidence, - contentFlag, - contentConfidence, - secret, - secretConfidence, - pii, - piiConfidence, - createdAt, - ] = values as Array; - this.fileEvents.push({ - id: this.fileEventSeq++, - workspace, - object_key: objectKey, - content_sha256: contentSha256, - kind, - classifier_version: classifierVersion, - model, - reason, - action, - actor, - rating, - rating_confidence: ratingConfidence, - content_flag: contentFlag, - content_confidence: contentConfidence, - secret, - secret_confidence: secretConfidence, - pii, - pii_confidence: piiConfidence, - created_at: createdAt, - }); - return { success: true as const, meta: { changes: 1 }, results: [] }; - } - if (normalized.startsWith("DELETE FROM file_events")) { - const workspace = values[0] as string; - if (normalized.includes("object_key IN")) { - const keys = new Set(values.slice(1) as string[]); - this.fileEvents = this.fileEvents.filter( - (row) => row.workspace !== workspace || !keys.has(row.object_key as string), - ); - } else if (normalized.includes("object_key = ?")) { - const objectKey = values[1] as string; - this.fileEvents = this.fileEvents.filter( - (row) => row.workspace !== workspace || row.object_key !== objectKey, - ); - } else { - this.fileEvents = this.fileEvents.filter((row) => row.workspace !== workspace); - } - return { success: true as const, meta: { changes: 1 }, results: [] }; - } if (normalized.startsWith("INSERT INTO file_content_hash")) { const [workspace, objectKey, hash, updatedAt] = values as [ string, diff --git a/docs/ops.md b/docs/ops.md index 9929e3112..093d624d3 100644 --- a/docs/ops.md +++ b/docs/ops.md @@ -1424,30 +1424,16 @@ the hold as it is. `POST /v1/workspaces/:workspace/files/approve?key=` writes `ai.hold=approved` and re-syncs the comment (`upload_approved`). It does not change `ai.secret`, `ai.pii`, `ai.rating`, or `ai.content`. The same -call appends an override row to `file_events` (`apps/api/src/file-events.ts`): -who released the hold, when, the content hash, and the safety flags in -force. CLI, MCP, Slack, the file page, and `/admin/classifier` release all -go through `approveUpload`, so they all append that row. A client cannot -write the log, and cannot set or delete `ai.*`. - -`ai.hold` is in `CLASSIFIER_ROW_KEYS`, so identical bytes keep an approval -and new bytes start over. The event log keeps the earlier attempt either -way. Each classification appends -another row: `classified`, `reused_exact`, `reused_perceptual`, `skipped` -(with the reason), or `failed`, plus the model, the classifier version, -and the safety answers. New bytes still clear the current `ai.*` rows -before the next classification. The earlier events stay. `content_sha256` -on each row is the bytes that event describes, so a later content-addressed -blob can use the same column. An override applies only while the key's -current hash equals that column. Promote and attach copy every classifier -row onto the new key, since the copy is not classified again. They do not -copy events. The bytes' earlier classification is still findable by hash. -`ai.rating`, `ai.content`, and `ai.pii=contact` never hold a file. - -The table is on the existing D1 database. No new binding. A database that -has not applied the migration still classifies and still approves; the -append is skipped. Object delete, retention, and workspace teardown remove -the rows for keys that are gone. An overwrite does not. +call appends a `file_events` row: who released the hold, when, the content +hash, and the flags in force. Classification appends `classified`, +`reused_exact`, `reused_perceptual`, `skipped`, or `failed` the same way. +New bytes still clear the current `ai.*` rows. The log keeps the earlier +row. `content_sha256` is those bytes, for a later blob id. The table is on +the existing D1 database. A missing migration skips the append. `ai.hold` +is in `CLASSIFIER_ROW_KEYS`, so identical bytes keep an approval and new +bytes start over. Promote and attach copy every classifier row onto the +new key, since the copy is not classified again. `ai.rating`, +`ai.content`, and `ai.pii=contact` never hold a file. ### Allowlist (Flagship is the control plane) From 6761d5ca751d3e59b62b70f1867bf649281fa699 Mon Sep 17 00:00:00 2001 From: Cursor Agent Date: Wed, 28 Oct 2026 11:39:45 +0000 Subject: [PATCH 3/5] fix: keep classifier events after delete and require an override hash Object delete and retention no longer erase file_events. An override records the bytes' hash, or the approve fails. Co-authored-by: Zach Dunn --- .../migrations/20261028120000_file_events.sql | 13 +- apps/api/src/file-events.ts | 121 +++++++++--------- apps/api/src/file-metadata.ts | 4 +- apps/api/src/files-core.ts | 2 - apps/api/src/held-storage.ts | 42 ++++-- apps/api/src/retention.ts | 2 - apps/api/src/upload-hold-read.ts | 7 +- apps/api/src/upload-hold-service.ts | 29 ++++- apps/api/test/file-events.test.ts | 19 ++- apps/api/test/routes-workspace-files.test.ts | 1 + apps/api/test/upload-hold-release.test.ts | 7 +- docs/ops.md | 10 +- 12 files changed, 153 insertions(+), 104 deletions(-) diff --git a/apps/api/migrations/20261028120000_file_events.sql b/apps/api/migrations/20261028120000_file_events.sql index d840c20cf..fdc225bf5 100644 --- a/apps/api/migrations/20261028120000_file_events.sql +++ b/apps/api/migrations/20261028120000_file_events.sql @@ -3,15 +3,16 @@ -- -- Rows are inserted and never updated. An overwrite clears the current -- `ai.*` rows and then appends another event; it does not delete the --- earlier ones. Object delete, retention, and workspace teardown remove --- the rows for keys that no longer exist. +-- earlier ones. Object delete and retention leave the log in place, so a +-- release cannot be erased by deleting the file. Workspace teardown +-- removes the workspace's rows. -- -- `content_sha256` is the bytes this event describes. It is the future -- blob id for content-addressed storage and is not a foreign key. An --- override applies to a key only while that key's current --- `file_content_hash` equals `content_sha256`. A null hash applies only --- while the key has no hash. A different hash is a different set of bytes --- and the override is ignored. +-- override's hash is never null: it is the object's `content-sha256`, or +-- the hash of the body restored from the held bucket. An override applies +-- to a key only while that key's current bytes hash to `content_sha256`. +-- A different hash is a different set of bytes and the override is ignored. -- -- `kind` is classified, reused_exact, reused_perceptual, skipped, failed, -- or override. `action` is release or hold, and only on an override. diff --git a/apps/api/src/file-events.ts b/apps/api/src/file-events.ts index 36529258d..26543732a 100644 --- a/apps/api/src/file-events.ts +++ b/apps/api/src/file-events.ts @@ -1,4 +1,4 @@ -import { contentHashForKey } from "./content-hash"; +import { ValidationError } from "@uploads/errors"; import { dbFor, type D1Queryable } from "./db-session"; import { contentSha256Hex } from "./provenance"; @@ -18,9 +18,11 @@ export type ClassificationEventKind = function ignorable(err: unknown): boolean { const message = err instanceof Error ? err.message : String(err); - return message.includes("no such table") || message.includes("unsupported "); + return message.includes("no such table"); } +const SHA256_RE = /^[a-f0-9]{64}$/; + async function insertEvent( env: Env, row: { @@ -106,6 +108,7 @@ export async function appendOverrideEvent( input: { workspace: string; objectKey: string; + contentSha256: string; action: "release" | "hold"; actor?: string | null; reason?: string | null; @@ -113,11 +116,18 @@ export async function appendOverrideEvent( at: string; }, ): Promise { - const contentSha256 = await contentHashForKey(dbFor(env), input.workspace, input.objectKey); + if (!SHA256_RE.test(input.contentSha256)) { + throw new ValidationError( + "this file has no content hash, so the hold change cannot be recorded", + { + code: "content_hash_unknown", + }, + ); + } await insertEvent(env, { workspace: input.workspace, objectKey: input.objectKey, - contentSha256: contentSha256 ?? null, + contentSha256: input.contentSha256, kind: "override", action: input.action, actor: input.actor, @@ -127,44 +137,57 @@ export async function appendOverrideEvent( }); } -export async function latestOverrideEvent( - db: D1Queryable, - workspace: string, - objectKey: string, -): Promise<{ +export interface OverrideEvent { action: string; actor: string | null; contentSha256: string | null; createdAt: string; -} | null> { +} + +/** Latest override per key, newest id first. One key is `latestOverridesForKeys(db, ws, [key])`. */ +export async function latestOverridesForKeys( + db: D1Queryable, + workspace: string, + keys: string[], +): Promise> { + const found = new Map(); + if (keys.length === 0) return found; try { - const row = await db - .prepare( - `SELECT action, actor, content_sha256, created_at - FROM file_events - WHERE workspace = ? AND object_key = ? AND kind = 'override' - ORDER BY id DESC - LIMIT 1`, - ) - .bind(workspace, objectKey) - .first<{ - action: string; - actor: string | null; - content_sha256: string | null; - created_at: string; - }>(); - if (!row) return null; - return { - action: row.action, - actor: row.actor, - contentSha256: row.content_sha256, - createdAt: row.created_at, - }; + const chunkSize = 80; + for (let i = 0; i < keys.length; i += chunkSize) { + const chunk = keys.slice(i, i + chunkSize); + const placeholders = chunk.map(() => "?").join(", "); + const result = await db + .prepare( + `SELECT object_key, action, actor, content_sha256, created_at + FROM file_events + WHERE workspace = ? AND kind = 'override' AND object_key IN (${placeholders}) + ORDER BY id DESC`, + ) + .bind(workspace, ...chunk) + .all<{ + object_key: string; + action: string; + actor: string | null; + content_sha256: string | null; + created_at: string; + }>(); + for (const row of result.results ?? []) { + if (found.has(row.object_key)) continue; + found.set(row.object_key, { + action: row.action, + actor: row.actor, + contentSha256: row.content_sha256, + createdAt: row.created_at, + }); + } + } } catch (err) { - if (ignorable(err)) return null; + if (ignorable(err)) return found; console.error({ event: "file_event_read_failed", workspace, err }); - return null; + return found; } + return found; } export function overrideApplies(eventSha: string | null, currentSha: string | null): boolean { @@ -189,36 +212,6 @@ async function runDelete( } } -export async function deleteFileEvents( - db: D1Queryable, - workspace: string, - objectKey: string, -): Promise { - await runDelete(db, workspace, `DELETE FROM file_events WHERE workspace = ? AND object_key = ?`, [ - workspace, - objectKey, - ]); -} - -export async function deleteFileEventsForKeys( - db: D1Queryable, - workspace: string, - keys: string[], -): Promise { - if (keys.length === 0) return; - const chunkSize = 90; - for (let i = 0; i < keys.length; i += chunkSize) { - const chunk = keys.slice(i, i + chunkSize); - const placeholders = chunk.map(() => "?").join(", "); - await runDelete( - db, - workspace, - `DELETE FROM file_events WHERE workspace = ? AND object_key IN (${placeholders})`, - [workspace, ...chunk], - ); - } -} - export async function deleteFileEventsForWorkspace( db: D1Queryable, workspace: string, diff --git a/apps/api/src/file-metadata.ts b/apps/api/src/file-metadata.ts index e0ae1b37b..1a9fc9caf 100644 --- a/apps/api/src/file-metadata.ts +++ b/apps/api/src/file-metadata.ts @@ -56,8 +56,8 @@ export const SAFETY_FLAG_FIELDS = CLASSIFIER_SAFETY_FIELDS; * classification is scheduled. A classification that writes a value * (including a confident `none`) replaces that key; one that omits a key * leaves the old value. Object delete removes them. They don't bump - * `ai.classifier`. The append-only history is `file_events`; an overwrite - * does not delete it. + * `ai.classifier`. The append-only history is `file_events`. An overwrite, + * a delete, and retention leave it in place. Workspace teardown removes it. */ export const SAFETY_FLAG_KEYS: ReadonlySet = new Set( SAFETY_FLAG_FIELDS.flatMap((field) => [`ai.${field}`, `ai.${field}.confidence`]), diff --git a/apps/api/src/files-core.ts b/apps/api/src/files-core.ts index 952dd5d5a..e2036128f 100644 --- a/apps/api/src/files-core.ts +++ b/apps/api/src/files-core.ts @@ -21,7 +21,6 @@ import { storageBudgetDenial, uploadBudgetDenial, } from "./budget"; -import { deleteFileEvents } from "./file-events"; import { deleteFileMetadata, deleteServerFileMetadataKeys, @@ -1322,7 +1321,6 @@ export async function deleteObject( }); if (heldSize !== null) hits.push({ size: heldSize, sharedLane: true }); await deleteFileMetadata(dbFor(env), workspaceName, key); - await deleteFileEvents(dbFor(env), workspaceName, key); // A held file going away ends its hold: rewrite the Slack DMs about it. if (heldSize !== null) await notifySlackHoldResolved(env, workspaceName, key, "deleted"); diff --git a/apps/api/src/held-storage.ts b/apps/api/src/held-storage.ts index 1ed622021..f7e56660c 100644 --- a/apps/api/src/held-storage.ts +++ b/apps/api/src/held-storage.ts @@ -17,10 +17,16 @@ import { createStorage, type Files } from "@uploads/storage"; import { dbFor } from "./db-session"; import { setServerFileMetadata } from "./file-metadata"; import { UPLOAD_CACHE_CONTROL } from "./files-core"; +import { contentSha256Hex } from "./provenance"; import { HOLD_MOVED_KEY } from "./upload-hold"; import { isKeyHeld } from "./upload-hold-read"; import { posterKeyFor } from "./poster"; -import { isSharedLane, resolveObjectLane, storageConfigs } from "./storage"; +import { + isSharedLane, + resolveObjectLane, + resolveObjectLaneWithHead, + storageConfigs, +} from "./storage"; import type { WorkspaceRecord } from "./workspace"; /** The held bucket, or `undefined` where it is not bound (tests, older deploys). */ @@ -67,24 +73,38 @@ export async function moveToHeld( /** * Copy a held object (and its poster) back to its key on the hosted lane, - * then drop the held copy. `false` when the held bucket has no copy. + * then drop the held copy. The return is the SHA-256 of the restored body, + * or `null` when the held bucket has no copy. */ export async function restoreFromHeld( env: Env, ws: WorkspaceRecord, workspaceName: string, key: string, -): Promise { +): Promise { const held = heldBucket(env); - if (!held) return false; + if (!held) return null; const store = await sharedLaneStore(env, ws); - if (!store) return false; - const restored = await copyIn(held, store, heldObjectKey(workspaceName, key), key); - if (!restored) return false; + if (!store) return null; + const sha = await copyIn(held, store, heldObjectKey(workspaceName, key), key); + if (!sha) return null; const posterKey = posterKeyFor(key); await copyIn(held, store, heldObjectKey(workspaceName, posterKey), posterKey); await held.delete(heldKeysFor(workspaceName, key)); - return true; + return sha; +} + +const SHA256_RE = /^[a-f0-9]{64}$/; + +/** `content-sha256` on the object, for a hold that never left the public bucket. */ +export async function objectContentSha256( + env: Env, + ws: WorkspaceRecord, + key: string, +): Promise { + const lane = await resolveObjectLaneWithHead(env, ws, key); + const value = lane?.head.metadata?.["content-sha256"]; + return typeof value === "string" && SHA256_RE.test(value) ? value : undefined; } /** @@ -154,14 +174,14 @@ async function copyIn( store: Files, heldKey: string, key: string, -): Promise { +): Promise { const source = await held.get(heldKey); - if (!source) return false; + if (!source) return null; const bytes = new Uint8Array(await source.arrayBuffer()); await store.upload(key, bytes, { contentType: source.httpMetadata?.contentType ?? "application/octet-stream", cacheControl: UPLOAD_CACHE_CONTROL, metadata: { ...source.customMetadata }, }); - return true; + return contentSha256Hex(bytes); } diff --git a/apps/api/src/retention.ts b/apps/api/src/retention.ts index 490b13832..542785964 100644 --- a/apps/api/src/retention.ts +++ b/apps/api/src/retention.ts @@ -16,7 +16,6 @@ */ import { positiveLimit } from "./budget"; import { deleteFileMetadataForKeys } from "./file-metadata"; -import { deleteFileEventsForKeys } from "./file-events"; import { deleteAttachmentsForKeysSafe, parseAttachmentKey } from "./github-attachment-index"; import { reconcileWorkspaceUsage, type ReconcileResult } from "./reconcile"; import { storage } from "./storage"; @@ -90,7 +89,6 @@ export async function purgeExpiredObjects( // Bulk form: native multi-delete on R2/S3 when available. await store.delete(batch); await deleteFileMetadataForKeys(dbFor(env), workspaceName, batch); - await deleteFileEventsForKeys(dbFor(env), workspaceName, batch); // Attachment index rows follow their objects (issue #934); chunked the // same way and best-effort, so a D1 hiccup never strands the sweep. // Filtered to keys that can actually hold a row — a retention sweep is diff --git a/apps/api/src/upload-hold-read.ts b/apps/api/src/upload-hold-read.ts index 691a1e04b..989cc6652 100644 --- a/apps/api/src/upload-hold-read.ts +++ b/apps/api/src/upload-hold-read.ts @@ -8,7 +8,7 @@ import { dbFor } from "./db-session"; import { contentHashForKey } from "./content-hash"; import { getMetadataForKeys } from "./file-metadata"; -import { latestOverrideEvent, overrideApplies } from "./file-events"; +import { latestOverridesForKeys, overrideApplies } from "./file-events"; import { aiRows } from "./classifier-admin"; import { HOLD_APPROVED, @@ -70,10 +70,11 @@ export async function effectiveHold( }; if (row !== HOLD_APPROVED) return fromRow(); const db = dbFor(env); - const [override, currentSha] = await Promise.all([ - latestOverrideEvent(db, workspace, objectKey), + const [overrides, currentSha] = await Promise.all([ + latestOverridesForKeys(db, workspace, [objectKey]), contentHashForKey(db, workspace, objectKey), ]); + const override = overrides.get(objectKey) ?? null; if ( !override || override.action !== "release" || diff --git a/apps/api/src/upload-hold-service.ts b/apps/api/src/upload-hold-service.ts index 079d1141d..91bf467dd 100644 --- a/apps/api/src/upload-hold-service.ts +++ b/apps/api/src/upload-hold-service.ts @@ -9,7 +9,7 @@ * (`hold-email-notify.ts`). */ -import { ServiceUnavailableError } from "@uploads/errors"; +import { ServiceUnavailableError, ValidationError } from "@uploads/errors"; import { classifierEvaluationContext } from "./classifier"; import { dbFor } from "./db-session"; import { appendOverrideEvent } from "./file-events"; @@ -19,7 +19,7 @@ import { setServerFileMetadata, } from "./file-metadata"; import { parseAttachmentKey } from "./github-attachment-index"; -import { moveToHeld, restoreFromHeld } from "./held-storage"; +import { moveToHeld, objectContentSha256, restoreFromHeld } from "./held-storage"; import { emailHold } from "./hold-email-notify"; import { notifySlackHoldResolved, notifySlackOfHold } from "./slack-hold-notify"; import { postManagedComment } from "./github-comment-service"; @@ -138,15 +138,27 @@ export async function approveUpload( const db = dbFor(env); const existing = await getFileMetadata(db, workspaceName, key); if (existing[HOLD_META_KEY] !== HOLD_PENDING) return { approved: false, reason: "not_held" }; + let contentSha256: string | undefined; if (existing[HOLD_MOVED_KEY] === "1") { // Bytes first: approving a file whose bytes cannot come back would - // leave its links broken with no hold left to explain why. - if (!(await restoreFromHeld(env, ws, workspaceName, key))) { + // leave its links broken with no hold left to explain why. The hash is + // of that body, not the best-effort D1 index. + const restored = await restoreFromHeld(env, ws, workspaceName, key); + if (!restored) { throw new ServiceUnavailableError("the held copy of this file could not be restored", { code: "held_restore_failed", }); } + contentSha256 = restored; await deleteServerFileMetadataKeys(db, workspaceName, key, [HOLD_MOVED_KEY]); + } else { + contentSha256 = await objectContentSha256(env, ws, key); + } + if (!contentSha256) { + throw new ValidationError( + "this file has no content hash, so the hold change cannot be recorded", + { code: "content_hash_unknown" }, + ); } const at = new Date().toISOString(); await setServerFileMetadata(db, workspaceName, key, { @@ -157,6 +169,7 @@ export async function approveUpload( await appendOverrideEvent(env, { workspace: workspaceName, objectKey: key, + contentSha256, action: "release", actor: by ?? null, safety: existing, @@ -190,12 +203,20 @@ export async function holdAgain( const db = dbFor(env); const existing = await getFileMetadata(db, workspaceName, key); if (existing[HOLD_META_KEY] !== HOLD_APPROVED) return { held: false, reason: "not_approved" }; + const contentSha256 = await objectContentSha256(env, ws, key); + if (!contentSha256) { + throw new ValidationError( + "this file has no content hash, so the hold change cannot be recorded", + { code: "content_hash_unknown" }, + ); + } // The row lands first, as in `applyUploadHold`. await setServerFileMetadata(db, workspaceName, key, { [HOLD_META_KEY]: HOLD_PENDING }); await deleteServerFileMetadataKeys(db, workspaceName, key, [HOLD_BY_KEY, HOLD_AT_KEY]); await appendOverrideEvent(env, { workspace: workspaceName, objectKey: key, + contentSha256, action: "hold", actor: by ?? null, safety: existing, diff --git a/apps/api/test/file-events.test.ts b/apps/api/test/file-events.test.ts index 667526dd6..ba95a6a9e 100644 --- a/apps/api/test/file-events.test.ts +++ b/apps/api/test/file-events.test.ts @@ -1,10 +1,12 @@ import { describe, expect, it } from "vitest"; -import { recordContentHash } from "../src/content-hash"; import { getFileMetadata, setServerFileMetadata } from "../src/file-metadata"; import { approveUpload } from "../src/upload-hold-service"; import type { WorkspaceRecord } from "../src/workspace"; +import { FakeR2Bucket } from "./fake-r2"; import { SqliteD1, database } from "./helpers/sqlite-d1"; +const SHA = "ab".repeat(32); + const ws: WorkspaceRecord = { provider: "r2", bucket: "uploads-default", @@ -21,7 +23,11 @@ describe("file_events", () => { "migrations/20261028120000_file_events.sql", ]); const db = database(sqlite); - const env = { DB: db } as Env; + const bucket = new FakeR2Bucket(); + await bucket.put("acme/shots/a.png", new Uint8Array([1, 2, 3]), { + customMetadata: { "content-sha256": SHA }, + }); + const env = { DB: db, UPLOADS_DEFAULT: bucket } as unknown as Env; await setServerFileMetadata(db, "acme", "shots/a.png", { "ai.hold": "pending", "ai.secret": "api_key", @@ -29,7 +35,6 @@ describe("file_events", () => { "ai.rating": "explicit", "ai.content": "sexual", }); - await recordContentHash(db, "acme", "shots/a.png", "ab".repeat(32)); expect(await approveUpload(env, ws, "acme", "shots/a.png", "Member")).toEqual({ approved: true, }); @@ -57,7 +62,7 @@ describe("file_events", () => { action: "release", actor: "Member", secret: "api_key", - content_sha256: "ab".repeat(32), + content_sha256: SHA, }); sqlite.close(); }); @@ -65,7 +70,11 @@ describe("file_events", () => { it("still approves when the event table is missing", async () => { const sqlite = new SqliteD1(["migrations/20260713210559_file_metadata.sql"]); const db = database(sqlite); - const env = { DB: db } as Env; + const bucket = new FakeR2Bucket(); + await bucket.put("acme/a.png", new Uint8Array([1]), { + customMetadata: { "content-sha256": SHA }, + }); + const env = { DB: db, UPLOADS_DEFAULT: bucket } as unknown as Env; await setServerFileMetadata(db, "acme", "a.png", { "ai.hold": "pending", "ai.secret": "api_key", diff --git a/apps/api/test/routes-workspace-files.test.ts b/apps/api/test/routes-workspace-files.test.ts index 815d9d274..763f68fca 100644 --- a/apps/api/test/routes-workspace-files.test.ts +++ b/apps/api/test/routes-workspace-files.test.ts @@ -151,6 +151,7 @@ async function makeEnv( async function seed(bucket: FakeR2Bucket) { await bucket.put("acme/shots/a.png", new Uint8Array([1, 2, 3]).buffer, { httpMetadata: { contentType: "image/png" }, + customMetadata: { "content-sha256": "ab".repeat(32) }, }); } diff --git a/apps/api/test/upload-hold-release.test.ts b/apps/api/test/upload-hold-release.test.ts index bae4e388d..3b09f9a6e 100644 --- a/apps/api/test/upload-hold-release.test.ts +++ b/apps/api/test/upload-hold-release.test.ts @@ -28,7 +28,7 @@ function makeEnv() { REGISTRY: registry, UPLOADS_DEFAULT: bucket, } as unknown as Env; - return { env, registry, record }; + return { env, registry, record, bucket }; } async function hold(env: Env, workspace: string, key: string) { @@ -53,9 +53,12 @@ describe("operator hold release", () => { }); it("approves every pending hold, or one workspace's", async () => { - const { env, registry, record } = makeEnv(); + const { env, registry, record, bucket } = makeEnv(); await registry.put("ws:acme", JSON.stringify(record("acme"))); await registry.put("ws:beta", JSON.stringify(record("beta"))); + const sha = { "content-sha256": "ab".repeat(32) }; + await bucket.put("acme/a.png", new Uint8Array([1]), { customMetadata: sha }); + await bucket.put("beta/c.png", new Uint8Array([2]), { customMetadata: sha }); await hold(env, "acme", "a.png"); await hold(env, "beta", "c.png"); diff --git a/docs/ops.md b/docs/ops.md index 093d624d3..579b5fb7c 100644 --- a/docs/ops.md +++ b/docs/ops.md @@ -1425,11 +1425,15 @@ the hold as it is. `ai.hold=approved` and re-syncs the comment (`upload_approved`). It does not change `ai.secret`, `ai.pii`, `ai.rating`, or `ai.content`. The same call appends a `file_events` row: who released the hold, when, the content -hash, and the flags in force. Classification appends `classified`, +hash, and the flags in force. The hash is the object's `content-sha256`, +or the hash of the body copied back from the held bucket. A file with +neither fails the approve. Classification appends `classified`, `reused_exact`, `reused_perceptual`, `skipped`, or `failed` the same way. New bytes still clear the current `ai.*` rows. The log keeps the earlier -row. `content_sha256` is those bytes, for a later blob id. The table is on -the existing D1 database. A missing migration skips the append. `ai.hold` +row. Deleting the file, or the retention sweep, does not delete the log. +Workspace teardown does. `content_sha256` is those bytes, for a later blob +id. The table is on the existing D1 database. A missing migration skips +the append. `ai.hold` is in `CLASSIFIER_ROW_KEYS`, so identical bytes keep an approval and new bytes start over. Promote and attach copy every classifier row onto the new key, since the copy is not classified again. `ai.rating`, From 104bd51a5043a3ef11fe4cad2b11f875206aca4d Mon Sep 17 00:00:00 2001 From: Cursor Agent Date: Wed, 28 Oct 2026 12:06:43 +0000 Subject: [PATCH 4/5] test: store a content hash on the MCP approve fixture Approve refuses a file whose bytes have no hash. The held-file test puts that hash on the object. Co-authored-by: Zach Dunn --- apps/mcp/test/mcp.test.ts | 7 +++++-- packages/storage/test/fake-r2.ts | 14 +++++++++++--- 2 files changed, 16 insertions(+), 5 deletions(-) diff --git a/apps/mcp/test/mcp.test.ts b/apps/mcp/test/mcp.test.ts index 995aa7392..a95fa4128 100644 --- a/apps/mcp/test/mcp.test.ts +++ b/apps/mcp/test/mcp.test.ts @@ -1492,9 +1492,12 @@ describe("mcp worker", () => { }); it("list_held, get_metadata, and approve cover a held file", async () => { - const { env, metadata } = await makeEnv(); + const { env, bucket, metadata } = await makeEnv(); const key = "gh/acme/web/pull/12/env.png"; - // A moved hold: rows only, no object in the bucket. + // Bytes stay in the bucket and carry the hash the release records. + await bucket.put(key, new Uint8Array([1]), { + customMetadata: { "content-sha256": "ab".repeat(32) }, + }); metadata.set( `test-ws ${key}`, new Map([ diff --git a/packages/storage/test/fake-r2.ts b/packages/storage/test/fake-r2.ts index 9b1f3ba08..501b9e41f 100644 --- a/packages/storage/test/fake-r2.ts +++ b/packages/storage/test/fake-r2.ts @@ -6,6 +6,7 @@ interface StoredObject { data: Uint8Array; contentType?: string; + customMetadata?: Record; } export class FakeR2Bucket { @@ -22,7 +23,7 @@ export class FakeR2Bucket { storageClass: "Standard", checksums: {}, httpMetadata: { contentType: obj.contentType }, - customMetadata: {}, + customMetadata: obj.customMetadata ? { ...obj.customMetadata } : {}, writeHttpMetadata() {}, }; } @@ -30,7 +31,10 @@ export class FakeR2Bucket { async put( key: string, value: ArrayBuffer | ArrayBufferView | string | ReadableStream | null, - opts?: { httpMetadata?: { contentType?: string } | Headers }, + opts?: { + httpMetadata?: { contentType?: string } | Headers; + customMetadata?: Record; + }, ) { let data: Uint8Array; if (typeof value === "string") data = new TextEncoder().encode(value); @@ -58,7 +62,11 @@ export class FakeR2Bucket { httpMetadata instanceof Headers ? (httpMetadata.get("content-type") ?? undefined) : httpMetadata?.contentType; - const obj = { data, contentType }; + const obj = { + data, + contentType, + customMetadata: opts?.customMetadata ? { ...opts.customMetadata } : undefined, + }; this.store.set(key, obj); return this.meta(key, obj); } From 8bd2f3ef1575b3ef11d82b699a222c4f8115c4c5 Mon Sep 17 00:00:00 2001 From: Cursor Agent Date: Wed, 28 Oct 2026 12:11:17 +0000 Subject: [PATCH 5/5] test: type the preview bucket metadata as string values The in-memory bucket now returns custom metadata. The preview double's empty object has to match that type. Co-authored-by: Zach Dunn --- apps/mcp/test/staged-media-tools.test.ts | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/apps/mcp/test/staged-media-tools.test.ts b/apps/mcp/test/staged-media-tools.test.ts index 340a72823..feae772b3 100644 --- a/apps/mcp/test/staged-media-tools.test.ts +++ b/apps/mcp/test/staged-media-tools.test.ts @@ -14,12 +14,10 @@ class PreviewBucket extends FakeR2Bucket { privateKeys = new Set(); override async head(key: string) { const head = await super.head(key); - return ( - head && { - ...head, - customMetadata: this.privateKeys.has(key) ? { visibility: "private" } : {}, - } - ); + const customMetadata: Record = this.privateKeys.has(key) + ? { visibility: "private" } + : {}; + return head && { ...head, customMetadata }; } }