From aa8e4db070f5b7adeceaab358995f6395b9e0a35 Mon Sep 17 00:00:00 2001 From: Lucas Winther Date: Fri, 30 Oct 2026 21:29:38 +0100 Subject: [PATCH] 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 () => {