From e3a759d768f624a3e28c09ca9f8a427b5216eb87 Mon Sep 17 00:00:00 2001 From: Lucas Winther Date: Fri, 30 Oct 2026 21:25:59 +0100 Subject: [PATCH 1/4] fix(core): soft-delete source connections with deleted_at Migration 0003 adds source_connections.deleted_at. deleteConnection sets it; listing, lookups, updates, the scheduler and the sync handler ignore deleted connections, so a paused credential-less connection stays visible and can be resumed. Co-Authored-By: Claude Opus 5.5 --- apps/server/src/routes/sources.ts | 3 +- packages/core/src/connections.ts | 34 ++++++++++------ packages/core/src/jobs.ts | 1 + packages/core/test/api-support.test.ts | 39 ++++++++++++++++++- packages/core/test/connections.test.ts | 3 +- .../0003_source_connections_deleted_at.ts | 13 +++++++ packages/db/src/migrations/index.ts | 2 + packages/db/src/schema.ts | 2 + 8 files changed, 81 insertions(+), 16 deletions(-) create mode 100644 packages/db/src/migrations/0003_source_connections_deleted_at.ts diff --git a/apps/server/src/routes/sources.ts b/apps/server/src/routes/sources.ts index b8a27fb..8cb6061 100644 --- a/apps/server/src/routes/sources.ts +++ b/apps/server/src/routes/sources.ts @@ -4,7 +4,6 @@ import { devicePoll, deviceStart, getConnection, - isDeletedConnection, listConnections, listRuns, updateConnection, @@ -48,7 +47,7 @@ export const sourceRoutes = [ path: "sources", summary: "Your source connections (never includes secrets).", auth: "any:read", - handler: async ({ deps, user }) => (await listConnections(deps.db, user.id)).filter((c) => !isDeletedConnection(c)), + handler: async ({ deps, user }) => listConnections(deps.db, user.id), }), route({ method: "POST", diff --git a/packages/core/src/connections.ts b/packages/core/src/connections.ts index bb12cc8..bab549c 100644 --- a/packages/core/src/connections.ts +++ b/packages/core/src/connections.ts @@ -77,6 +77,7 @@ export async function createConnection(db: Db, secret: string, userId: string, i status: "active", last_run_at: null, created_at: nowIso(), + deleted_at: null, }) .execute(); return id; @@ -94,7 +95,7 @@ export interface ConnectionPatch { export async function updateConnection(db: Db, secret: string, userId: string, id: string, patch: ConnectionPatch, manifest: PluginManifest): Promise { checkSchedule(patch.scheduleMinutes); await db.k.transaction().execute(async (tx) => { - let q = tx.selectFrom("source_connections").selectAll().where("id", "=", id).where("user_id", "=", userId); + let q = tx.selectFrom("source_connections").selectAll().where("id", "=", id).where("user_id", "=", userId).where("deleted_at", "is", null); if (db.dialect === "postgres") q = q.forUpdate(); const row = await q.executeTakeFirst(); if (!row) throw new NotFoundError("connection"); @@ -116,20 +117,27 @@ export async function updateConnection(db: Db, secret: string, userId: string, i }); } -/** Soft delete: the connection is paused and its credentials are wiped. Records and runs stay. */ +/** Soft delete: `deleted_at` is set, the connection is paused and its credentials are wiped. Records and runs stay. */ export async function deleteConnection(db: Db, userId: string, id: string): Promise { const r = await db.k .updateTable("source_connections") - .set({ status: "paused", credentials_enc: null }) + .set({ status: "paused", credentials_enc: null, deleted_at: nowIso() }) .where("id", "=", id) .where("user_id", "=", userId) + .where("deleted_at", "is", null) .executeTakeFirst(); if (r.numUpdatedRows !== 1n) throw new NotFoundError("connection"); } -/** Never returns secrets: plain config plus `hasCredentials`. */ +/** The user's live (not deleted) connections. Never returns secrets: plain config plus `hasCredentials`. */ export async function listConnections(db: Db, userId: string): Promise { - const rows = await db.k.selectFrom("source_connections").selectAll().where("user_id", "=", userId).orderBy("created_at").execute(); + const rows = await db.k + .selectFrom("source_connections") + .selectAll() + .where("user_id", "=", userId) + .where("deleted_at", "is", null) + .orderBy("created_at") + .execute(); return rows.map((r) => ({ id: r.id, pluginId: r.plugin_id, @@ -143,14 +151,15 @@ export async function listConnections(db: Db, userId: string): Promise): boolean { - return c.status === "paused" && !c.hasCredentials; -} - async function ownedRow(db: Db, userId: string, id: string) { - const r = await db.k.selectFrom("source_connections").selectAll().where("id", "=", id).where("user_id", "=", userId).executeTakeFirst(); - if (!r || (r.status === "paused" && r.credentials_enc === null)) throw new NotFoundError("connection"); + const r = await db.k + .selectFrom("source_connections") + .selectAll() + .where("id", "=", id) + .where("user_id", "=", userId) + .where("deleted_at", "is", null) + .executeTakeFirst(); + if (!r) throw new NotFoundError("connection"); return r; } @@ -342,6 +351,7 @@ export function syncHandler(deps: SyncHandlerDeps): (payload: SyncPayload, jc?: const { db } = deps; const full = await db.k.selectFrom("source_connections").selectAll().where("id", "=", payload.connectionId).executeTakeFirst(); if (!full) throw new DropJobError("connection not found"); + if (full.deleted_at !== null) throw new DropJobError("connection is deleted"); if (full.status === "paused") throw new DropJobError("connection is paused"); const plugin = deps.registry.get(full.plugin_id); if (!plugin) return failBeforeRun(db, full.id, payload.file ? "file" : (payload.mode ?? "full"), `Plugin '${full.plugin_id}' is not installed or failed to load.`); diff --git a/packages/core/src/jobs.ts b/packages/core/src/jobs.ts index 4620857..155ce2c 100644 --- a/packages/core/src/jobs.ts +++ b/packages/core/src/jobs.ts @@ -198,6 +198,7 @@ export function startScheduler(db: Db, opts: { tickMs?: number } = {}): { stop() .selectFrom("source_connections") .select(["id", "schedule_minutes", "last_run_at"]) .where("status", "!=", "paused") + .where("deleted_at", "is", null) .where("schedule_minutes", "is not", null) .execute(); const now = Date.now(); diff --git a/packages/core/test/api-support.test.ts b/packages/core/test/api-support.test.ts index 552134c..b5a26c0 100644 --- a/packages/core/test/api-support.test.ts +++ b/packages/core/test/api-support.test.ts @@ -7,7 +7,9 @@ import { forEachDialect } from "@sttracker/db/testing"; import { definePlugin, type SourcePlugin } from "@sttracker/plugin-sdk/types"; import { listAttachmentLinks, readAttachment, requireLinkTarget, storeAttachment } from "../src/attachments.ts"; import { createAppToken, listAppTokens, revokeAppToken } from "../src/auth.ts"; -import { createConnection, deleteConnection, devicePoll, deviceStart, getConnection, listRuns, syncHandler } from "../src/connections.ts"; +import { createConnection, deleteConnection, devicePoll, deviceStart, getConnection, listConnections, listRuns, syncHandler, updateConnection } from "../src/connections.ts"; +import { startScheduler } from "../src/jobs.ts"; +import { DropJobError } from "../src/jobs.ts"; import { listSuggestions, quickAdd } from "../src/entries.ts"; import { addEngagement } from "../src/engagements.ts"; import { NotFoundError, ValidationError } from "../src/errors.ts"; @@ -114,6 +116,41 @@ forEachDialect("api support", (getDb) => { await expect(getConnection(db, u, id)).rejects.toBeInstanceOf(NotFoundError); }); + test("a paused connection without credentials stays listed and can be unpaused; a deleted one is gone", async () => { + const db = getDb(); + const u = await makeUser(db); + const id = await createConnection(db, SECRET, u, { pluginId: "echo", name: "E", config: {} }, echo.manifest); + await updateConnection(db, SECRET, u, id, { status: "paused" }, echo.manifest); + expect((await listConnections(db, u)).map((c) => c.id)).toEqual([id]); + expect((await getConnection(db, u, id)).status).toBe("paused"); + await updateConnection(db, SECRET, u, id, { status: "active" }, echo.manifest); + expect((await getConnection(db, u, id)).status).toBe("active"); + + await deleteConnection(db, u, id); + const row = await db.k.selectFrom("source_connections").selectAll().where("id", "=", id).executeTakeFirstOrThrow(); + expect(row).toMatchObject({ status: "paused", credentials_enc: null }); + expect(row.deleted_at).not.toBeNull(); + expect(await listConnections(db, u)).toEqual([]); + await expect(getConnection(db, u, id)).rejects.toBeInstanceOf(NotFoundError); + await expect(updateConnection(db, SECRET, u, id, { status: "active" }, echo.manifest)).rejects.toBeInstanceOf(NotFoundError); + await expect(deleteConnection(db, u, id)).rejects.toBeInstanceOf(NotFoundError); + }); + + test("deleted connections are never scheduled and their queued syncs are dropped", async () => { + const db = getDb(); + const u = await makeUser(db); + const id = await createConnection(db, SECRET, u, { pluginId: "echo", name: "E", config: {}, scheduleMinutes: 1 }, echo.manifest); + // Simulate a delete that left status "error" (e.g. a row edited by hand): deleted_at alone must exclude it. + await deleteConnection(db, u, id); + await db.k.updateTable("source_connections").set({ status: "error" }).where("id", "=", id).execute(); + const sched = startScheduler(db, { tickMs: 3_600_000 }); + await sched.tickNow(); + sched.stop(); + expect(await db.k.selectFrom("jobs").select("id").execute()).toEqual([]); + const sync = syncHandler({ db, secret: SECRET, registry: reg }); + await expect(sync({ connectionId: id })).rejects.toBeInstanceOf(DropJobError); + }); + test("syncHandler runs file mode from a base64 payload", async () => { const db = getDb(); const u = await makeUser(db); diff --git a/packages/core/test/connections.test.ts b/packages/core/test/connections.test.ts index 1859c9d..cd8dfaf 100644 --- a/packages/core/test/connections.test.ts +++ b/packages/core/test/connections.test.ts @@ -55,7 +55,8 @@ forEachDialect("connections", (getDb) => { r = await raw(id); expect(r.status).toBe("paused"); expect(r.credentials_enc).toBeNull(); - expect((await listConnections(db, u))[0]!.hasCredentials).toBe(false); + expect(r.deleted_at).not.toBeNull(); + expect(await listConnections(db, u)).toEqual([]); }); test("cursor persists on set; http uses manifest rate limit", async () => { diff --git a/packages/db/src/migrations/0003_source_connections_deleted_at.ts b/packages/db/src/migrations/0003_source_connections_deleted_at.ts new file mode 100644 index 0000000..9b4763c --- /dev/null +++ b/packages/db/src/migrations/0003_source_connections_deleted_at.ts @@ -0,0 +1,13 @@ +import type { Kysely } from "kysely"; +import type { Migration } from "kysely/migration"; + +/** Soft delete for source connections: a deleted connection is distinct from one the user merely paused. */ +export const sourceConnectionsDeletedAt: Migration = { + async up(db: Kysely) { + await db.schema.alterTable("source_connections").addColumn("deleted_at", "text").execute(); + }, + + async down(db: Kysely) { + await db.schema.alterTable("source_connections").dropColumn("deleted_at").execute(); + }, +}; diff --git a/packages/db/src/migrations/index.ts b/packages/db/src/migrations/index.ts index 4dc1ca0..fd6adc3 100644 --- a/packages/db/src/migrations/index.ts +++ b/packages/db/src/migrations/index.ts @@ -1,9 +1,11 @@ import type { Migration } from "kysely/migration"; import { initial } from "./0001_initial.ts"; import { worksPositionUnique } from "./0002_works_position_unique.ts"; +import { sourceConnectionsDeletedAt } from "./0003_source_connections_deleted_at.ts"; /** All migrations, keyed by sortable name. Imported statically so bundles need no filesystem. */ export const migrations: Record = { "0001_initial": initial, "0002_works_position_unique": worksPositionUnique, + "0003_source_connections_deleted_at": sourceConnectionsDeletedAt, }; diff --git a/packages/db/src/schema.ts b/packages/db/src/schema.ts index 81c27bf..0d85c9d 100644 --- a/packages/db/src/schema.ts +++ b/packages/db/src/schema.ts @@ -269,6 +269,8 @@ export interface SourceConnectionsTable { status: ConnectionStatus; last_run_at: string | null; created_at: string; + /** Set by `deleteConnection`; deleted connections are hidden, never scheduled and never synced. */ + deleted_at: string | null; } export interface SourceRunsTable { From aa8e4db070f5b7adeceaab358995f6395b9e0a35 Mon Sep 17 00:00:00 2001 From: Lucas Winther Date: Fri, 30 Oct 2026 21:29:38 +0100 Subject: [PATCH 2/4] fix(core): token writes keep their actor in history but are user-owned in field_sources fieldOwner(actor) maps every non-plugin actor to "user" wherever field_sources is written (entries, engagements, list ownership from history), so syncs never overwrite a field an API token edited. authenticateAppToken also returns the token name; revokeAppToken reports whether it revoked; listSuggestions queries the canonical work id. Co-Authored-By: Claude Opus 5.5 --- packages/core/src/auth.ts | 16 ++++-- packages/core/src/engagements.ts | 6 +- packages/core/src/entries.ts | 13 +++-- packages/core/src/history.ts | 9 +++ packages/core/src/sync/apply.ts | 6 +- packages/core/test/api-support.test.ts | 78 +++++++++++++++++++++++++- 6 files changed, 110 insertions(+), 18 deletions(-) diff --git a/packages/core/src/auth.ts b/packages/core/src/auth.ts index 8bf546d..6cefcdc 100644 --- a/packages/core/src/auth.ts +++ b/packages/core/src/auth.ts @@ -170,11 +170,17 @@ export async function createAppToken( } export async function verifyAppToken(db: Db, token: string, scope: TokenScope): Promise { + return (await authenticateAppToken(db, token, scope))?.user ?? null; +} + +/** Like `verifyAppToken`, also returning the token's name (API writes record actor `token:`). */ +export async function authenticateAppToken(db: Db, token: string, scope: TokenScope): Promise<{ user: User; tokenName: string } | null> { const row = await db.k .selectFrom("app_tokens") .innerJoin("users", "users.id", "app_tokens.user_id") .select([ "app_tokens.id as token_id", + "app_tokens.name as token_name", "app_tokens.scopes", "users.id", "users.username", @@ -186,17 +192,19 @@ export async function verifyAppToken(db: Db, token: string, scope: TokenScope): .executeTakeFirst(); if (!row || !fromJson(row.scopes, []).includes(scope)) return null; await db.k.updateTable("app_tokens").set({ last_used_at: nowIso() }).where("id", "=", row.token_id).execute(); - return toUser(row); + return { user: toUser(row), tokenName: row.token_name }; } -export async function revokeAppToken(db: Db, userId: string, id: string): Promise { - await db.k +/** Revoke one of the user's live tokens. Returns false when no such live token exists (unknown, foreign or revoked). */ +export async function revokeAppToken(db: Db, userId: string, id: string): Promise { + const r = await db.k .updateTable("app_tokens") .set({ revoked_at: nowIso() }) .where("id", "=", id) .where("user_id", "=", userId) .where("revoked_at", "is", null) - .execute(); + .executeTakeFirst(); + return Number(r.numUpdatedRows) === 1; } export interface AppTokenInfo { diff --git a/packages/core/src/engagements.ts b/packages/core/src/engagements.ts index e758308..eea8494 100644 --- a/packages/core/src/engagements.ts +++ b/packages/core/src/engagements.ts @@ -15,7 +15,7 @@ import { toStoredDate, } from "./entries.ts"; import { NotFoundError, ValidationError } from "./errors.ts"; -import { type Actor, type Executor, inTx, recordHistory } from "./history.ts"; +import { type Actor, type Executor, fieldOwner, inTx, recordHistory } from "./history.ts"; /** One time through a work (spec §6.3). Dates are partial; `null` means unknown. */ export interface EngagementPatch { @@ -144,7 +144,7 @@ async function write( ): Promise { const cols = await toColumns(k, userId, mediaType, patch); const fieldSources = { ...current.fieldSources }; - for (const key of Object.keys(patch)) fieldSources[key] = actor; + for (const key of Object.keys(patch)) fieldSources[key] = fieldOwner(actor) as Actor; await k .updateTable("engagements") .set({ ...cols, ...extra, field_sources: toJson(fieldSources), updated_at: nowIso() }) @@ -175,7 +175,7 @@ export async function addEngagement( const { mediaType } = await requireEntry(k, userId, entryId); const cols = await toColumns(k, userId, mediaType, patch); const fieldSources: FieldSources = {}; - for (const key of Object.keys(patch)) fieldSources[key] = actor; + for (const key of Object.keys(patch)) fieldSources[key] = fieldOwner(actor) as Actor; const now = nowIso(); const row: Row = { id: newId(), diff --git a/packages/core/src/entries.ts b/packages/core/src/entries.ts index 142f87c..ac5fb3f 100644 --- a/packages/core/src/entries.ts +++ b/packages/core/src/entries.ts @@ -7,7 +7,7 @@ import { parsePartialDate } from "@sttracker/plugin-sdk/partial-date"; import type { MediaType, PartialDate } from "@sttracker/plugin-sdk/types"; import { customFieldsFor, validateCustomValues } from "./custom-fields.ts"; import { NotFoundError, ValidationError } from "./errors.ts"; -import { type Actor, type Executor, inTx, recordHistory } from "./history.ts"; +import { type Actor, type Executor, fieldOwner, inTx, recordHistory } from "./history.ts"; // --------------------------------------------------------------------------------------------- // Shared user-layer helpers (also used by engagements, lists and copies). @@ -320,7 +320,7 @@ async function writeEntry( ): Promise { const fieldSources: Record = { ...current.fieldSources }; for (const key of Object.keys(patch)) { - fieldSources[key] = opts.sources && key in opts.sources ? opts.sources[key] : actor; + fieldSources[key] = opts.sources && key in opts.sources ? opts.sources[key] : (fieldOwner(actor) as Actor); if (fieldSources[key] === undefined) delete fieldSources[key]; } const cols = { ...patchToColumns(patch), ...opts.extra, field_sources: toJson(fieldSources), updated_at: nowIso() }; @@ -985,14 +985,17 @@ export interface MatchSuggestion { createdAt: string; } -/** The user's pending match suggestions for a work, best first. Throws `NotFoundError` if the work is not visible. */ +/** + * The user's pending match suggestions for a work (after following aliases and merges to the canonical work), best first. + * Throws `NotFoundError` if the work is not visible. + */ export async function listSuggestions(db: Db, userId: string, workId: string): Promise { - await resolveVisibleWork(db.k, userId, workId); + const work = await resolveVisibleWork(db.k, userId, workId); const rows = await db.k .selectFrom("match_suggestions") .selectAll() .where("user_id", "=", userId) - .where("work_id", "=", workId) + .where("work_id", "=", work.id) .where("status", "=", "pending") .orderBy("score", "desc") .orderBy("id") diff --git a/packages/core/src/history.ts b/packages/core/src/history.ts index 34b8da6..b0b30cc 100644 --- a/packages/core/src/history.ts +++ b/packages/core/src/history.ts @@ -12,6 +12,15 @@ import { restoreListFromHistory } from "./lists.ts"; /** Who made a change. Plugin actors are `plugin::`; API tokens are `token:`. */ export type Actor = "user" | `plugin:${string}:${string}` | `token:${string}`; +/** + * Who owns a field in `field_sources` after `actor` writes it. Only plugin actors keep their name; the user, + * through the UI or an API token, is always `"user"`, so syncs never overwrite a field a script edited. + * History rows keep the full actor. + */ +export function fieldOwner(actor: string): string { + return actor.startsWith("plugin:") ? actor : "user"; +} + /** User-layer entities that write `user_history`. */ export type HistoryEntity = "entry" | "engagement" | "copy" | "list"; diff --git a/packages/core/src/sync/apply.ts b/packages/core/src/sync/apply.ts index c9c8e8c..88bcf5a 100644 --- a/packages/core/src/sync/apply.ts +++ b/packages/core/src/sync/apply.ts @@ -6,7 +6,7 @@ import type { ChangeStatus, ChangeType, Database } from "@sttracker/db/schema"; import type { EngagementFields, EntryFields, PartialDate, SourceRecord, WorkHint } from "@sttracker/plugin-sdk/types"; import { addEngagement, type EngagementPatch, updateEngagement } from "../engagements.ts"; import { type EntryPatch, fromStoredDate, resolveVisibleWork, upsertEntry } from "../entries.ts"; -import type { Actor, Executor } from "../history.ts"; +import { type Actor, type Executor, fieldOwner } from "../history.ts"; import { createList, replaceListItems, updateList } from "../lists.ts"; import { matchWork } from "../matching.ts"; import { resolveAliasIn, updateWorkFromHintIn } from "../works.ts"; @@ -146,8 +146,8 @@ export async function listFieldSources(k: Executor, listId: string, sourceActor: const fs: Record = { items: sourceActor }; for (const r of rows) { const after = fromJson(r.after, {}); - for (const f of ["name", "description"]) if (f in after) fs[f] = r.actor; - if (r.actor === "user") fs.items = "user"; + for (const f of ["name", "description"]) if (f in after) fs[f] = fieldOwner(r.actor); + if (fieldOwner(r.actor) === "user") fs.items = "user"; } return fs; } diff --git a/packages/core/test/api-support.test.ts b/packages/core/test/api-support.test.ts index b5a26c0..5bc2c13 100644 --- a/packages/core/test/api-support.test.ts +++ b/packages/core/test/api-support.test.ts @@ -6,7 +6,11 @@ import { newId, nowIso } from "@sttracker/db/ids"; import { forEachDialect } from "@sttracker/db/testing"; import { definePlugin, type SourcePlugin } from "@sttracker/plugin-sdk/types"; import { listAttachmentLinks, readAttachment, requireLinkTarget, storeAttachment } from "../src/attachments.ts"; -import { createAppToken, listAppTokens, revokeAppToken } from "../src/auth.ts"; +import { authenticateAppToken, createAppToken, listAppTokens, revokeAppToken } from "../src/auth.ts"; +import { getHistory } from "../src/history.ts"; +import { fieldOwner } from "../src/history.ts"; +import { getEntry, upsertEntry } from "../src/entries.ts"; +import { listEngagements } from "../src/engagements.ts"; import { createConnection, deleteConnection, devicePoll, deviceStart, getConnection, listConnections, listRuns, syncHandler, updateConnection } from "../src/connections.ts"; import { startScheduler } from "../src/jobs.ts"; import { DropJobError } from "../src/jobs.ts"; @@ -39,7 +43,21 @@ const fileish: SourcePlugin = definePlugin({ poll: async (_ctx, deviceCode) => (deviceCode === "dc" ? { access_token: "tok" } : "pending"), }, }); -const reg2: PluginRegistry = { get: (id) => (id === "fileish" ? fileish : reg.get(id)), list: () => [...reg.list(), fileish.manifest], reload: async () => {} }; +let flipStatus: "planned" | "completed" = "planned"; +const flip: SourcePlugin = definePlugin({ + manifest: { id: "flip", name: "Flip", version: "1", sdkVersion: 1, mediaTypes: ["book"], capabilities: ["pull"], config: [] }, + async *pull() { + yield { + kind: "entry", + key: "flip-1", + raw: flipStatus, + work: { title: "Flip", mediaType: "book", externalIds: { isbn13: "9780000000010" } }, + entry: { status: flipStatus, rating: flipStatus === "planned" ? 40 : 90 }, + }; + }, +}); +const extra: Record = { fileish, flip }; +const reg2: PluginRegistry = { get: (id) => extra[id] ?? reg.get(id), list: () => [...reg.list(), fileish.manifest, flip.manifest], reload: async () => {} }; forEachDialect("api support", (getDb) => { test("listAppTokens returns metadata only, revoked flagged, scoped to user", async () => { @@ -48,7 +66,9 @@ forEachDialect("api support", (getDb) => { const other = await makeUser(db, "bob"); const t = await createAppToken(db, u, "script", ["read"]); await createAppToken(db, other, "x", ["write"]); - await revokeAppToken(db, u, t.id); + expect(await revokeAppToken(db, other, t.id)).toBe(false); + expect(await revokeAppToken(db, u, t.id)).toBe(true); + expect(await revokeAppToken(db, u, t.id)).toBe(false); const list = await listAppTokens(db, u); expect(list).toHaveLength(1); expect(list[0]).toMatchObject({ id: t.id, name: "script", scopes: ["read"], lastUsedAt: null }); @@ -56,6 +76,53 @@ forEachDialect("api support", (getDb) => { expect(JSON.stringify(list)).not.toContain("hash"); }); + test("fieldOwner: only plugin actors keep their name in field_sources", () => { + expect(fieldOwner("user")).toBe("user"); + expect(fieldOwner("token:script")).toBe("user"); + expect(fieldOwner("plugin:echo:c1")).toBe("plugin:echo:c1"); + }); + + test("authenticateAppToken returns the user and the token name", async () => { + const db = getDb(); + const u = await makeUser(db); + const t = await createAppToken(db, u, "script", ["write"]); + expect(await authenticateAppToken(db, t.token, "write")).toMatchObject({ user: { id: u }, tokenName: "script" }); + expect(await authenticateAppToken(db, t.token, "read")).toBeNull(); + }); + + test("a token write keeps its actor in history, is owned by the user, and a later sync files a pending change", async () => { + const db = getDb(); + const u = await makeUser(db); + flipStatus = "planned"; + const id = await createConnection(db, SECRET, u, { pluginId: "flip", name: "F", config: {} }, flip.manifest); + const sync = syncHandler({ db, secret: SECRET, registry: reg2 }); + await sync({ connectionId: id, mode: "full" }); + const entry = await db.k.selectFrom("entries").selectAll().where("user_id", "=", u).executeTakeFirstOrThrow(); + await upsertEntry(db, u, entry.work_id, { status: "dropped" }, "token:script"); + const view = await getEntry(db, u, entry.id); + expect(view.fieldSources.status).toBe("user"); + expect(view.fieldSources.rating).toBe(`plugin:flip:${id}`); + const hist = await getHistory(db, u, "entry", entry.id); + expect(hist[0]!.actor).toBe("token:script"); + + flipStatus = "completed"; + await sync({ connectionId: id, mode: "full" }); + const after = await getEntry(db, u, entry.id); + expect(after.status).toBe("dropped"); + expect(after.rating).toBe(90); + const pending = await db.k.selectFrom("change_events").selectAll().where("user_id", "=", u).where("status", "=", "pending").execute(); + expect(pending.map((p) => [p.type, JSON.parse(p.payload).field])).toEqual([["changed", "status"]]); + }); + + test("token-authored engagement fields are owned by the user", async () => { + const db = getDb(); + const u = await makeUser(db); + const e = await quickAdd(db, u, { title: "Dune" }); + await addEngagement(db, u, e.id, { platform: "kindle" }, "token:script"); + const [g] = await listEngagements(db, u, e.id); + expect(g!.fieldSources).toEqual({ platform: "user" }); + }); + test("listSuggestions lists pending suggestions for a visible work", async () => { const db = getDb(); const u = await makeUser(db); @@ -70,6 +137,11 @@ forEachDialect("api support", (getDb) => { expect(s[0]).toMatchObject({ workId: draft.workId, candidateWorkId: good, score: 0.75, status: "pending" }); const other = await makeUser(db, "bob"); await expect(listSuggestions(db, other, draft.workId)).rejects.toBeInstanceOf(NotFoundError); + + // Asking by an id the user aliased away lists the canonical work's suggestions. + const old = await makeWork(db, { title: "Old Dune", ownerUserId: u }); + await db.k.insertInto("work_aliases").values({ id: newId(), user_id: u, from_work_id: old, to_work_id: draft.workId }).execute(); + expect((await listSuggestions(db, u, old)).map((x) => x.workId)).toEqual([draft.workId]); }); test("requireLinkTarget checks ownership per entity; links and filename are readable", async () => { From 5f7d54b805c65035cc132fbd7b4e90a62f14ce20 Mon Sep 17 00:00:00 2001 From: Lucas Winther Date: Fri, 30 Oct 2026 21:30:43 +0100 Subject: [PATCH 3/4] fix(server): token actor, login CSRF, 415, cookie decoding, admin invites, production default - API writes pass token: for Bearer requests and "user" for sessions. - X-Requested-With: sttracker on every non-Bearer write, login and register included. - JSON-body routes reject non-JSON content types with 415. - A malformed stt_session cookie counts as no cookie (401), not 500. - DELETE me/tokens/:id is 404 for unknown or foreign ids; admin/invites is admin-only. - Bun.serve development mode only with NODE_ENV=development or --hot. - OpenAPI documents multipart fields. Co-Authored-By: Claude Opus 5.5 --- apps/server/src/http.ts | 44 ++++++++--- apps/server/src/main.ts | 4 +- apps/server/src/openapi.ts | 10 ++- apps/server/src/routes/attachments.ts | 1 + apps/server/src/routes/auth.ts | 4 +- apps/server/src/routes/copies.ts | 8 +- apps/server/src/routes/engagements.ts | 12 +-- apps/server/src/routes/entries.ts | 12 +-- apps/server/src/routes/lists.ts | 18 ++--- apps/server/test/api.test.ts | 107 ++++++++++++++++++++++++-- 10 files changed, 174 insertions(+), 46 deletions(-) diff --git a/apps/server/src/http.ts b/apps/server/src/http.ts index 77ec8ba..27b20e4 100644 --- a/apps/server/src/http.ts +++ b/apps/server/src/http.ts @@ -1,5 +1,6 @@ -import { getSessionUser, verifyAppToken, type User } from "@sttracker/core/auth"; +import { authenticateAppToken, getSessionUser, verifyAppToken, type User } from "@sttracker/core/auth"; import { AuthError, ImportError, NotFoundError, ValidationError } from "@sttracker/core/errors"; +import type { Actor } from "@sttracker/core/history"; import { isUniqueViolation } from "@sttracker/core/matching"; import type { PluginRegistry } from "@sttracker/core/registry"; import type { Db } from "@sttracker/db/db"; @@ -33,6 +34,8 @@ export interface Ctx { query: Q; /** The authenticated user. Throws on `auth: "none"` routes, which have none. */ user: User; + /** Who writes: `token:` for Bearer requests, `"user"` for sessions. Pass it to every core write. */ + actor: Actor; /** The session token, when the request was authenticated by cookie. */ sessionToken: string | null; /** Parsed multipart body, for `multipart: true` routes. */ @@ -51,6 +54,8 @@ export interface RouteDef { query?: z.ZodType; /** Parse the body as multipart form data (upload limit) instead of JSON. */ multipart?: boolean; + /** Non-file multipart fields, for the OpenAPI document (handlers validate them). Every multipart route also takes `file`. */ + formFields?: z.ZodObject; /** Rate limit per client IP (login, register). */ rateLimited?: boolean; /** Only admins may call it (403 otherwise). */ @@ -132,7 +137,13 @@ function readCookie(req: Request, name: string): string | null { if (!header) return null; for (const part of header.split(";")) { const i = part.indexOf("="); - if (i > 0 && part.slice(0, i).trim() === name) return decodeURIComponent(part.slice(i + 1).trim()) || null; + if (i > 0 && part.slice(0, i).trim() === name) { + try { + return decodeURIComponent(part.slice(i + 1).trim()) || null; + } catch { + return null; // malformed percent-encoding: treat as no cookie + } + } } return null; } @@ -185,16 +196,20 @@ export function rateLimiter(limit: number, windowMs: number): (key: string) => b const SCOPES: TokenScope[] = ["read", "write", "ingest"]; /** Resolve the caller for `mode`. Returns the user and the session token used, if any. */ -async function authenticate(req: Request, mode: AuthMode, db: Db): Promise<{ user: User | null; sessionToken: string | null }> { - if (mode === "none") return { user: null, sessionToken: null }; +async function authenticate( + req: Request, + mode: AuthMode, + db: Db, +): Promise<{ user: User | null; sessionToken: string | null; actor: Actor }> { + if (mode === "none") return { user: null, sessionToken: null, actor: "user" }; const authz = req.headers.get("authorization"); const bearer = authz?.startsWith("Bearer ") ? authz.slice(7).trim() : null; const [kind, scopeName] = mode.split(":") as ["session" | "token" | "any", TokenScope | undefined]; if (kind !== "session" && bearer) { const scope = scopeName!; - const user = await verifyAppToken(db, bearer, scope); - if (user) return { user, sessionToken: null }; + const auth = await authenticateAppToken(db, bearer, scope); + if (auth) return { user: auth.user, sessionToken: null, actor: `token:${auth.tokenName}` }; for (const other of SCOPES) { if (other !== scope && (await verifyAppToken(db, bearer, other))) { throw new HttpError(403, "forbidden", `This app token lacks the '${scope}' scope.`); @@ -207,11 +222,7 @@ async function authenticate(req: Request, mode: AuthMode, db: Db): Promise<{ use const token = readCookie(req, SESSION_COOKIE); const user = token ? await getSessionUser(db, token) : null; if (!user) throw new HttpError(401, "unauthenticated", "Sign in first."); - // CSRF: browsers cannot set this header cross-site without a CORS preflight, which we never grant. - if (req.method !== "GET" && req.method !== "HEAD" && req.headers.get(CSRF_HEADER) !== CSRF_VALUE) { - throw new HttpError(403, "forbidden", `Cookie-authenticated requests must send the header X-Requested-With: ${CSRF_VALUE}.`); - } - return { user, sessionToken: token }; + return { user, sessionToken: token, actor: "user" }; } interface Compiled { @@ -261,7 +272,13 @@ export function apiHandler(routes: RouteDef[], deps: AppDeps, prefix = "/api/v1/ if (def.rateLimited && !limit(`${def.path}|${ip}`)) { return errorResponse(429, "rate_limited", "Too many attempts. Wait a minute and try again.", { "retry-after": "60" }); } - const { user, sessionToken } = await authenticate(req, def.auth, deps.db); + // CSRF: every non-Bearer write (login and register included) must carry this header. Browsers cannot add it + // cross-site without a CORS preflight, which the server never grants. Bearer requests are not ambient, so exempt. + const bearer = req.headers.get("authorization")?.startsWith("Bearer ") ?? false; + if (req.method !== "GET" && req.method !== "HEAD" && !bearer && req.headers.get(CSRF_HEADER) !== CSRF_VALUE) { + throw new HttpError(403, "forbidden", `Browser requests that change data must send the header X-Requested-With: ${CSRF_VALUE}.`); + } + const { user, sessionToken, actor } = await authenticate(req, def.auth, deps.db); if (def.admin && user?.role !== "admin") throw new HttpError(403, "forbidden", "Only an admin can do that."); let body: unknown; @@ -279,6 +296,8 @@ export function apiHandler(routes: RouteDef[], deps: AppDeps, prefix = "/api/v1/ const bytes = await readLimited(req, MAX_JSON_BYTES); let raw: unknown; if (bytes.byteLength > 0) { + const type = (req.headers.get("content-type") ?? "").split(";")[0]!.trim().toLowerCase(); + if (type !== "application/json") throw new HttpError(415, "unsupported_media_type", "Send the body as Content-Type: application/json."); try { raw = JSON.parse(new TextDecoder().decode(bytes)); } catch { @@ -299,6 +318,7 @@ export function apiHandler(routes: RouteDef[], deps: AppDeps, prefix = "/api/v1/ if (!user) throw new Error(`route ${def.method} ${def.path} has no authenticated user`); return user; }, + actor, sessionToken, get form(): FormData { if (!form) throw new Error(`route ${def.method} ${def.path} is not multipart`); diff --git a/apps/server/src/main.ts b/apps/server/src/main.ts index c4ebf9a..b4c0420 100644 --- a/apps/server/src/main.ts +++ b/apps/server/src/main.ts @@ -40,7 +40,9 @@ const backupTimer = setInterval(() => void ensureBackup(), BACKUP_CHECK_MS); const app = createApp({ db, registry, secret: config.secret, dataDir: config.dataDir, publicUrl: config.publicUrl }); const server = Bun.serve({ port: config.port, - development: process.env.NODE_ENV !== "production", + // Production unless asked: dev mode adds HMR and verbose error pages. + // `bun --hot` puts the flag in execArgv, not Bun.argv. + development: process.env.NODE_ENV === "development" || process.execArgv.includes("--hot"), routes: { "/api/*": (req, srv) => app.fetch(req, srv), "/ingest/*": (req, srv) => app.fetch(req, srv), diff --git a/apps/server/src/openapi.ts b/apps/server/src/openapi.ts index 3012d2f..291b71a 100644 --- a/apps/server/src/openapi.ts +++ b/apps/server/src/openapi.ts @@ -46,7 +46,15 @@ export function buildOpenApi(routes: RouteDef[], prefix = "/api/v1"): JsonSchema }; if (parameters.length > 0) op.parameters = parameters; if (r.body) op.requestBody = { required: !r.body.safeParse(undefined).success, content: { "application/json": { schema: toSchema(r.body) } } }; - if (r.multipart) op.requestBody = { required: true, content: { "multipart/form-data": { schema: { type: "object" } } } }; + if (r.multipart) { + const fields = r.formFields ? (toSchema(r.formFields) as { properties?: JsonSchema; required?: string[] }) : {}; + const schema = { + type: "object", + properties: { file: { type: "string", format: "binary" }, ...fields.properties }, + required: ["file", ...(fields.required ?? [])], + }; + op.requestBody = { required: true, content: { "multipart/form-data": { schema } } }; + } (paths[path] ??= {})[r.method.toLowerCase()] = op; } return { diff --git a/apps/server/src/routes/attachments.ts b/apps/server/src/routes/attachments.ts index f759980..cf6ad43 100644 --- a/apps/server/src/routes/attachments.ts +++ b/apps/server/src/routes/attachments.ts @@ -24,6 +24,7 @@ export const attachmentRoutes = [ summary: "Upload a file (multipart: file, entity, entityId, caption) and link it to one of your records.", auth: "any:write", multipart: true, + formFields: fields, handler: async ({ deps, user, form }) => { const file = formFile(form); const f = fields.parse({ diff --git a/apps/server/src/routes/auth.ts b/apps/server/src/routes/auth.ts index f691def..3032b22 100644 --- a/apps/server/src/routes/auth.ts +++ b/apps/server/src/routes/auth.ts @@ -1,4 +1,5 @@ import { createAppToken, createInvite, listAppTokens, login, logout, registerUser, revokeAppToken } from "@sttracker/core/auth"; +import { NotFoundError } from "@sttracker/core/errors"; import { z } from "zod"; import { clearSessionCookie, json, noContent, route, sessionCookie } from "../http.ts"; @@ -61,7 +62,7 @@ export const authRoutes = [ summary: "Revoke an app token.", auth: "session", handler: async ({ deps, user, params }) => { - await revokeAppToken(deps.db, user.id, params.id!); + if (!(await revokeAppToken(deps.db, user.id, params.id!))) throw new NotFoundError("app token"); return noContent(); }, }), @@ -70,6 +71,7 @@ export const authRoutes = [ path: "admin/invites", summary: "Create an invite code (admin). The code is returned once.", auth: "session", + admin: true, body: z.object({ ttlDays: z.number().int().min(1).max(365).optional() }).strict().optional(), handler: async ({ deps, user, body }) => json({ code: await createInvite(deps.db, user.id, body?.ttlDays) }, 201), }), diff --git a/apps/server/src/routes/copies.ts b/apps/server/src/routes/copies.ts index 6de9f3b..ee62ddf 100644 --- a/apps/server/src/routes/copies.ts +++ b/apps/server/src/routes/copies.ts @@ -35,7 +35,7 @@ export const copyRoutes = [ summary: "Record an owned copy.", auth: "any:write", body: copyPatch, - handler: async ({ deps, user, body }) => json(await createCopy(deps.db, user.id, body as CopyArg), 201), + handler: async ({ deps, user, body, actor }) => json(await createCopy(deps.db, user.id, body as CopyArg, actor), 201), }), route({ method: "PATCH", @@ -43,15 +43,15 @@ export const copyRoutes = [ summary: "Edit an owned copy.", auth: "any:write", body: copyPatch, - handler: async ({ deps, user, params, body }) => updateCopy(deps.db, user.id, params.id!, body as CopyArg), + handler: async ({ deps, user, params, body, actor }) => updateCopy(deps.db, user.id, params.id!, body as CopyArg, actor), }), route({ method: "DELETE", path: "copies/:id", summary: "Soft-delete an owned copy.", auth: "any:write", - handler: async ({ deps, user, params }) => { - await softDeleteCopy(deps.db, user.id, params.id!); + handler: async ({ deps, user, params, actor }) => { + await softDeleteCopy(deps.db, user.id, params.id!, actor); return noContent(); }, }), diff --git a/apps/server/src/routes/engagements.ts b/apps/server/src/routes/engagements.ts index a239947..5ff5ca3 100644 --- a/apps/server/src/routes/engagements.ts +++ b/apps/server/src/routes/engagements.ts @@ -30,8 +30,8 @@ export const engagementRoutes = [ summary: "Log a read, watch or play.", auth: "any:write", body: engagementPatch, - handler: async ({ deps, user, params, body }) => - json(await addEngagement(deps.db, user.id, params.id!, body as Parameters[3], "user"), 201), + handler: async ({ deps, user, params, body, actor }) => + json(await addEngagement(deps.db, user.id, params.id!, body as Parameters[3], actor), 201), }), route({ method: "PATCH", @@ -39,16 +39,16 @@ export const engagementRoutes = [ summary: "Edit an engagement.", auth: "any:write", body: engagementPatch, - handler: async ({ deps, user, params, body }) => - updateEngagement(deps.db, user.id, params.id!, body as Parameters[3], "user"), + handler: async ({ deps, user, params, body, actor }) => + updateEngagement(deps.db, user.id, params.id!, body as Parameters[3], actor), }), route({ method: "DELETE", path: "engagements/:id", summary: "Soft-delete an engagement.", auth: "any:write", - handler: async ({ deps, user, params }) => { - await softDeleteEngagement(deps.db, user.id, params.id!, "user"); + handler: async ({ deps, user, params, actor }) => { + await softDeleteEngagement(deps.db, user.id, params.id!, actor); return noContent(); }, }), diff --git a/apps/server/src/routes/entries.ts b/apps/server/src/routes/entries.ts index c8c5657..6b81c21 100644 --- a/apps/server/src/routes/entries.ts +++ b/apps/server/src/routes/entries.ts @@ -60,9 +60,9 @@ export const entryRoutes = [ summary: "Edit an entry. Edited fields become user-owned (syncs stop overwriting them).", auth: "any:write", body: entryPatch, - handler: async ({ deps, user, params, body }) => { + handler: async ({ deps, user, params, body, actor }) => { const cur = await getEntry(deps.db, user.id, params.id!); - await upsertEntry(deps.db, user.id, cur.workId, body as Parameters[3], "user"); + await upsertEntry(deps.db, user.id, cur.workId, body as Parameters[3], actor); return getEntry(deps.db, user.id, params.id!); }, }), @@ -71,8 +71,8 @@ export const entryRoutes = [ path: "entries/:id", summary: "Soft-delete an entry.", auth: "any:write", - handler: async ({ deps, user, params }) => { - await softDeleteEntry(deps.db, user.id, params.id!, "user"); + handler: async ({ deps, user, params, actor }) => { + await softDeleteEntry(deps.db, user.id, params.id!, actor); return noContent(); }, }), @@ -99,8 +99,8 @@ export const entryRoutes = [ summary: "Move an entry to another work (for you only).", auth: "any:write", body: z.object({ workId: z.string().min(1) }).strict(), - handler: async ({ deps, user, params, body }) => { - const moved = await moveEntryToWork(deps.db, user.id, params.id!, body.workId, "user"); + handler: async ({ deps, user, params, body, actor }) => { + const moved = await moveEntryToWork(deps.db, user.id, params.id!, body.workId, actor); return getEntry(deps.db, user.id, moved.id); }, }), diff --git a/apps/server/src/routes/lists.ts b/apps/server/src/routes/lists.ts index b260d38..98d6f0d 100644 --- a/apps/server/src/routes/lists.ts +++ b/apps/server/src/routes/lists.ts @@ -25,7 +25,7 @@ export const listRoutes = [ summary: "Create a list.", auth: "any:write", body: listPatch.required({ name: true }), - handler: async ({ deps, user, body }) => json(await createList(deps.db, user.id, body as ListPatchArg & { name: string }), 201), + handler: async ({ deps, user, body, actor }) => json(await createList(deps.db, user.id, body as ListPatchArg & { name: string }, actor), 201), }), route({ method: "GET", @@ -40,15 +40,15 @@ export const listRoutes = [ summary: "Edit a list.", auth: "any:write", body: listPatch, - handler: async ({ deps, user, params, body }) => updateList(deps.db, user.id, params.id!, body as ListPatchArg), + handler: async ({ deps, user, params, body, actor }) => updateList(deps.db, user.id, params.id!, body as ListPatchArg, actor), }), route({ method: "DELETE", path: "lists/:id", summary: "Soft-delete a list.", auth: "any:write", - handler: async ({ deps, user, params }) => { - await softDeleteList(deps.db, user.id, params.id!); + handler: async ({ deps, user, params, actor }) => { + await softDeleteList(deps.db, user.id, params.id!, actor); return noContent(); }, }), @@ -58,7 +58,7 @@ export const listRoutes = [ summary: "Add a work to a list.", auth: "any:write", body: z.object({ workId: z.string().min(1), note: text(10_000).nullable().optional(), position: z.number().int().min(0).optional() }).strict(), - handler: async ({ deps, user, params, body }) => json(await addListItem(deps.db, user.id, params.id!, body), 201), + handler: async ({ deps, user, params, body, actor }) => json(await addListItem(deps.db, user.id, params.id!, body, actor), 201), }), route({ method: "PATCH", @@ -66,8 +66,8 @@ export const listRoutes = [ summary: "Move a list item to a new 0-based position.", auth: "any:write", body: z.object({ position: z.number().int().min(0) }).strict(), - handler: async ({ deps, user, params, body }) => { - await moveListItem(deps.db, user.id, params.id!, body.position); + handler: async ({ deps, user, params, body, actor }) => { + await moveListItem(deps.db, user.id, params.id!, body.position, actor); return noContent(); }, }), @@ -76,8 +76,8 @@ export const listRoutes = [ path: "list-items/:id", summary: "Remove an item from a list.", auth: "any:write", - handler: async ({ deps, user, params }) => { - await removeListItem(deps.db, user.id, params.id!); + handler: async ({ deps, user, params, actor }) => { + await removeListItem(deps.db, user.id, params.id!, actor); return noContent(); }, }), diff --git a/apps/server/test/api.test.ts b/apps/server/test/api.test.ts index e18fa97..e32299d 100644 --- a/apps/server/test/api.test.ts +++ b/apps/server/test/api.test.ts @@ -23,15 +23,15 @@ interface CallOpts { token?: string; headers?: Record; form?: FormData; + /** Omit the X-Requested-With header that browsers (the web app) send on writes. */ + noCsrf?: boolean; } async function call(app: App, method: string, path: string, o: CallOpts = {}): Promise { const headers: Record = { ...o.headers }; - if (o.cookie) { - headers.cookie = o.cookie; - headers["x-requested-with"] ??= "sttracker"; - } + if (o.cookie) headers.cookie = o.cookie; if (o.token) headers.authorization = `Bearer ${o.token}`; + else if (!o.noCsrf) headers["x-requested-with"] ??= "sttracker"; let body: BodyInit | undefined; if (o.form) body = o.form; else if (o.body !== undefined) { @@ -184,8 +184,16 @@ forEachDialect("api", (getDb) => { test("cookie-authenticated writes need the CSRF header", async () => { const cookie = await signUp(app, "ana"); - const res = await call(app, "POST", "entries/quick", { cookie, body: { title: "Dune" }, headers: { "x-requested-with": "" } }); + const res = await call(app, "POST", "entries/quick", { cookie, body: { title: "Dune" }, noCsrf: true }); expect(res.status).toBe(403); + // Login and register too (login CSRF). + const login = await call(app, "POST", "auth/login", { body: { username: "ana", password: "correct horse" }, noCsrf: true }); + expect(login.status).toBe(403); + expect(login.headers.get("set-cookie")).toBeNull(); + expect((await call(app, "POST", "auth/register", { body: { username: "zed", password: "correct horse" }, noCsrf: true })).status).toBe(403); + // Bearer requests are exempt. + const { token } = (await (await call(app, "POST", "me/tokens", { cookie, body: { name: "w", scopes: ["write"] } })).json()) as { token: string }; + expect((await call(app, "POST", "entries/quick", { token, body: { title: "Dune" } })).status).toBe(201); }); test("login is rate limited to 10 per minute", async () => { @@ -264,16 +272,97 @@ forEachDialect("api", (getDb) => { expect(got.headers.get("x-content-type-options")).toBe("nosniff"); expect(got.headers.get("content-disposition")).toBe('inline; filename="we_ird_na_me_.png"'); + const { code } = (await (await call(app, "POST", "admin/invites", { cookie: ana })).json()) as { code: string }; + const bob = await signUp(app, "bob", code); + const bobs = (await (await call(app, "POST", "entries/quick", { cookie: bob, body: { title: "Bob's" } })).json()) as { id: string }; const form2 = new FormData(); form2.set("file", new File([new Uint8Array([1])], "a.png", { type: "image/png" })); form2.set("entity", "entry"); - form2.set("entityId", "0190f000-0000-7000-8000-000000000000"); + form2.set("entityId", bobs.id); expect((await call(app, "POST", "attachments", { cookie: ana, form: form2 })).status).toBe(404); + expect((await call(app, "GET", `attachments/${att.id}`, { cookie: bob })).status).toBe(404); expect((await call(app, "DELETE", `attachments/${att.id}/links/${att.links[0]!.id}`, { cookie: ana })).status).toBe(204); expect((await call(app, "GET", `attachments/${att.id}`, { cookie: ana })).status).toBe(404); }); + test("JSON bodies need Content-Type application/json (415)", async () => { + const res = await app.fetch( + new Request(`${BASE}/auth/register`, { + method: "POST", + headers: { "content-type": "text/plain", "x-requested-with": "sttracker" }, + body: JSON.stringify({ username: "ana", password: "correct horse" }), + }), + ); + expect(res.status).toBe(415); + expect(((await res.json()) as { error: { code: string } }).error.code).toBe("unsupported_media_type"); + }); + + test("a malformed session cookie is 401, not 500", async () => { + expect((await call(app, "GET", "me", { headers: { cookie: "stt_session=%" } })).status).toBe(401); + }); + + test("cookie plus a bogus Bearer on a write is 401", async () => { + const cookie = await signUp(app, "ana"); + const res = await call(app, "POST", "entries/quick", { cookie, token: "stt_bogus", body: { title: "Dune" } }); + expect(res.status).toBe(401); + }); + + test("a write-scope token can write; history records the token, field_sources the user", async () => { + const cookie = await signUp(app, "ana"); + const add = (await (await call(app, "POST", "entries/quick", { cookie, body: { title: "Dune" } })).json()) as { id: string }; + const { token } = (await (await call(app, "POST", "me/tokens", { cookie, body: { name: "script", scopes: ["write", "read"] } })).json()) as { token: string }; + const res = await call(app, "PATCH", `entries/${add.id}`, { token, body: { rating: 70 } }); + expect(res.status).toBe(200); + expect(((await res.json()) as { fieldSources: Record }).fieldSources.rating).toBe("user"); + const hist = (await (await call(app, "GET", `entries/${add.id}/history`, { token })).json()) as { actor: string }[]; + expect(hist[0]!.actor).toBe("token:script"); + }); + + test("DELETE me/tokens/:id is 404 for unknown or foreign tokens", async () => { + const ana = await signUp(app, "ana"); + const { code } = (await (await call(app, "POST", "admin/invites", { cookie: ana })).json()) as { code: string }; + const bob = await signUp(app, "bob", code); + const { id } = (await (await call(app, "POST", "me/tokens", { cookie: ana, body: { name: "t", scopes: ["read"] } })).json()) as { id: string }; + expect((await call(app, "DELETE", `me/tokens/${id}`, { cookie: bob })).status).toBe(404); + expect((await call(app, "DELETE", "me/tokens/nope", { cookie: ana })).status).toBe(404); + expect((await call(app, "DELETE", `me/tokens/${id}`, { cookie: ana })).status).toBe(204); + }); + + test("admin-only routes are 403 for members", async () => { + const ana = await signUp(app, "ana"); + const { code } = (await (await call(app, "POST", "admin/invites", { cookie: ana })).json()) as { code: string }; + const bob = await signUp(app, "bob", code); + expect((await call(app, "POST", "admin/plugins/reload", { cookie: bob })).status).toBe(403); + expect((await call(app, "POST", "admin/invites", { cookie: bob })).status).toBe(403); + const warn = spyOn(console, "warn").mockImplementation(() => {}); + try { + expect((await call(app, "POST", "admin/plugins/reload", { cookie: ana })).status).toBe(200); + } finally { + warn.mockRestore(); + } + }); + + test("a paused source without credentials stays listed and can be resumed", async () => { + const cookie = await signUp(app, "ana"); + const { id } = (await (await call(app, "POST", "sources", { cookie, body: { pluginId: "echo", name: "E" } })).json()) as { id: string }; + expect((await call(app, "PATCH", `sources/${id}`, { cookie, body: { status: "paused" } })).status).toBe(200); + const listed = (await (await call(app, "GET", "sources", { cookie })).json()) as { id: string; status: string }[]; + expect(listed).toMatchObject([{ id, status: "paused" }]); + const resumed = await call(app, "PATCH", `sources/${id}`, { cookie, body: { status: "active" } }); + expect(((await resumed.json()) as { status: string }).status).toBe("active"); + }); + + test("a multipart upload over 50 MB is 413", async () => { + const cookie = await signUp(app, "ana"); + const { id } = (await (await call(app, "POST", "sources", { cookie, body: { pluginId: "echo", name: "E" } })).json()) as { id: string }; + const form = new FormData(); + form.set("file", new File([new Uint8Array(50 * 1024 * 1024 + 1)], "big.csv", { type: "text/csv" })); + const res = await call(app, "POST", `sources/${id}/import`, { cookie, form }); + expect(res.status).toBe(413); + expect(((await res.json()) as { error: { code: string } }).error.code).toBe("validation"); + }); + test("openapi.json lists /api/v1/entries", async () => { const res = await call(app, "GET", "openapi.json"); expect(res.status).toBe(200); @@ -281,6 +370,12 @@ forEachDialect("api", (getDb) => { expect(doc.openapi).toStartWith("3."); expect(doc.paths["/api/v1/entries"]?.get?.summary).toBeTruthy(); expect(doc.paths["/api/v1/entries/{id}"]?.patch).toBeTruthy(); + const up = doc.paths["/api/v1/attachments"]?.post as unknown as { + requestBody: { content: Record; required: string[] } }> }; + }; + const mp = up.requestBody.content["multipart/form-data"]!.schema; + expect(Object.keys(mp.properties).sort()).toEqual(["caption", "entity", "entityId", "file"]); + expect(mp.required).toEqual(["file", "entity", "entityId"]); }); test("about lists attributions; ingest and unknown routes are 404 JSON", async () => { From 0b34ede62e35b6ae65d2414d49998f19f7622689 Mon Sep 17 00:00:00 2001 From: Lucas Winther Date: Fri, 30 Oct 2026 21:30:59 +0100 Subject: [PATCH 4/4] docs: token actor, connection deleted_at and CSRF decisions; follow-ups Co-Authored-By: Claude Opus 5.5 --- docs/ARCHITECTURE.md | 8 +++++--- docs/DECISIONS.md | 18 ++++++++++++++++++ docs/TODO.md | 3 ++- 3 files changed, 25 insertions(+), 4 deletions(-) diff --git a/docs/ARCHITECTURE.md b/docs/ARCHITECTURE.md index 80bab88..fd20afa 100644 --- a/docs/ARCHITECTURE.md +++ b/docs/ARCHITECTURE.md @@ -26,9 +26,11 @@ user layer or files a Changes-inbox event. HTTP request (`apps/server`): 1. `Bun.serve` routes `/api/*` and `/ingest/*` to `app.fetch`; everything else gets the web app (HTML import). 2. `apiHandler` matches method + path in the route table, applies the per-IP rate limit (login/register), authenticates - (`session` cookie `stt_session`, Bearer app token with a scope, or either), checks `X-Requested-With: sttracker` on - cookie-authenticated writes, reads the body (JSON ≤ 1 MB, multipart ≤ 50 MB) and validates it with zod. -3. The handler calls one core function with the caller's `userId` and actor `"user"`. Core enforces ownership + (`session` cookie `stt_session`, Bearer app token with a scope, or either), requires `X-Requested-With: sttracker` on + every non-Bearer write (login included), reads the body (JSON ≤ 1 MB with `Content-Type: application/json`, + multipart ≤ 50 MB) and validates it with zod. +3. The handler calls one core function with the caller's `userId` and actor (`token:` for Bearer, else `"user"`; + `field_sources` stores both as `"user"`). Core enforces ownership (`NotFoundError` → 404). Errors become `{ error: { code, message } }`; unknown errors are logged and return 500 `internal`. 4. `POST sources/:id/run|import` only enqueue a `sync` job (lockKey `sync:`, 409 when held); the worker runs it. diff --git a/docs/DECISIONS.md b/docs/DECISIONS.md index 2fb29c6..869d0dd 100644 --- a/docs/DECISIONS.md +++ b/docs/DECISIONS.md @@ -4,6 +4,24 @@ Append-only. Never edit or delete an entry. Supersede it with a new one that lin One entry per decision a future maintainer would otherwise have to reverse-engineer. No index: `grep -n '^## ' docs/DECISIONS.md` lists every entry (date, title, line number). +## 2026-10-30: Token actor in history, connection deleted_at, CSRF on every browser write +**Status:** accepted. Supersedes the **Actor**, **CSRF** and **Deleted connections** bullets of +"HTTP API: auth modes, CSRF header, error mapping, actor" below. +**Context:** Task 16 fix round 1 (controller rulings). +**Decision:** +- **Actor vs. owner.** API writes pass actor `token:` for Bearer requests and `"user"` for sessions, so history + shows which script changed what (spec §6.9). `field_sources` stores `fieldOwner(actor)`: every actor that does not start + with `plugin:` is stored as `"user"`. A token edit is the user's edit, and syncs file a pending change instead of + overwriting it. List ownership, which is read from history, applies the same mapping. +- **CSRF.** `X-Requested-With: sttracker` is required on every non-GET request that has no Bearer token, including the + `auth: "none"` routes login and register (login CSRF). JSON-body routes also require `Content-Type: application/json` + (415 `unsupported_media_type` otherwise), so a cross-site simple form post cannot pass as JSON. +- **Deleted connections** carry `source_connections.deleted_at` (migration 0003). `deleteConnection` sets it and still pauses + the connection and wipes its credentials. Listing, lookups, updates, the scheduler and the sync handler ignore deleted rows. + A user-paused connection without credentials is a normal, visible, resumable connection. +- **Dev mode** (`Bun.serve({ development })`) is on only with `NODE_ENV=development` or `bun --hot` (detected through + `process.execArgv`; `Bun.argv` does not carry runtime flags). Everything else runs in production mode. + ## 2026-10-30: HTTP API: auth modes, CSRF header, error mapping, actor **Status:** accepted **Context:** Task 16 (`apps/server`). diff --git a/docs/TODO.md b/docs/TODO.md index 0f8ce4f..0ef72bc 100644 --- a/docs/TODO.md +++ b/docs/TODO.md @@ -11,10 +11,11 @@ Small, specific, independently actionable. Remove only when actually done. ## Next - [ ] API: response schemas in the OpenAPI document (requests only today). +- [ ] Deployment docs (Task 19): the server runs `Bun.serve` in production mode unless `NODE_ENV=development` or `bun --hot`, so Docker needs no `NODE_ENV`. Document `NODE_ENV=development` for local debugging. +- [ ] API: `restoreVersion`, `acceptSuggestion` and `resolveChange` take no actor, so history records `"user"` for them even when an app token calls them. Thread the actor through if script provenance matters there. - [ ] API: login/register rate limit is per process and keyed on the socket IP; behind a reverse proxy every client shares one key. Add a trusted-proxy setting for `X-Forwarded-For`. - [ ] API: file imports are base64 in the `jobs.payload` row (up to ~67 MB of text). Move uploads to `DATA_DIR/uploads/` and pass a path. - [ ] API: `PATCH list-items/:id` only moves items; editing an item's note needs a core function. -- [ ] Connections: a user-paused connection that never had credentials looks deleted (`paused` + no credentials) and disappears from `GET sources`. Add an explicit `deleted_at`. - [ ] Phase 2 and Phase 3 plans (spec §12). - [ ] Generate image thumbnails for attachments (Phase 1 sizes images with CSS).