diff --git a/CHANGELOG.md b/CHANGELOG.md index ede1f645..79f8ecd9 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,46 @@ All notable changes to the Fovea project will be documented in this file. The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/), and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). +## [0.5.9] - 2026-06-30 + +The 0.5.9 patch is the first of three releases remediating a second swarm audit of the shipped 0.5.x line — the backend slice — fixing authorization, lost-update concurrency, idempotency, and data-fidelity defects in the Fastify server ([#194](https://github.com/parafovea/fovea/pull/194)). Roughly a third of the findings were gaps in the first audit's own fixes: a fix addressed specific instances of a defect class without closing every instance. Nothing is breaking. + +### Fixed + +#### Authorization + +- The `/api/models/*` proxy routes were unauthenticated: any caller could read the model configuration and, worse, drive the shared inference service — selecting, loading, or unloading a model affects every user — with no credentials. The routes now require authentication, and the state-changing select/load/unload operations additionally require admin (`server/src/app.ts`, `server/src/routes/models.ts`). +- A persona could be attached to a project by a non-member, since the create ability passed for any self-owned persona regardless of the target project. Project-scoped persona creation now verifies project membership and returns 403 otherwise (`server/src/services/persona-service.ts`). +- The import "replace" paths overwrote or deleted an existing annotation, persona, summary, claim, or claim-relation found by id with no instance-level check, so a crafted import could clobber another user's row. Each replace now requires update (or delete) on the existing row, mirroring the ontology-import guard (`server/src/services/import/entity-importers.ts`). + +#### Concurrency — lost-update hardening + +- The optimistic-concurrency guard on world state and ontologies compared on `updatedAt`, which two writes landing in the same millisecond may share — letting the second silently overwrite the first. Both tables now carry a monotonic `version` column, and every guarded write compares-and-swaps on it; guard exhaustion now returns 409 and a missing row 404 rather than a generic 500 (`server/prisma/migrations/20260630120000_add_optimistic_version_and_admin_apikey_uniqueness`, `server/src/repositories/{WorldStateRepository,ProjectRepository,PersonaRepository}.ts`, `server/src/services/world-state-service.ts`). +- Persona type-deletion and persona deletion wrote the annotation delete, the ontology gloss cleanup, and the world-state cleanup as separate unguarded statements, so a partial failure left them disagreeing and a concurrent edit could be clobbered. Each now runs in a single transaction with the ontology and world-state writes routed through the version guard, recomputing from a fresh read (`server/src/services/persona-service.ts`). +- The admin "clear world state" path wrote through a bare id update that could half-survive a concurrent write; it now clears through the version guard (`server/src/services/world-state-service.ts`). +- First-access creation of a project world state raced into the compound unique constraint and surfaced a 500; it now upserts the empty row and merges through the guard (`server/src/services/project-service.ts`). +- The claim-extraction worker re-saved claims on every run with no dedup, so a job retry or double-submit duplicated every claim for a summary. Re-extraction is now idempotent: the prior extracted claims are replaced inside one transaction, preserving manually authored claims (`server/src/queues/setup.ts`). + +#### Idempotency and conflict status + +- Several create and update paths surfaced a duplicate as a 500 rather than a 409: the self-service profile email/username update, project and group membership adds, and admin API keys. Each now maps the unique-constraint violation to a 409 (`server/src/routes/{users,groups}.ts`, `server/src/services/{project-service,api-key-service}.ts`). +- Admin API keys were unconstrained, since the compound `@@unique([userId, provider])` does not bind rows with a null userId in Postgres, so the advertised 409 never fired. A partial unique index now enforces one admin key per provider, deduping any existing duplicates first (`server/prisma/migrations/20260630120000_add_optimistic_version_and_admin_apikey_uniqueness`). +- A claim create that lost the same-id race re-threw the raw unique violation as a 500 when the existing row was not updatable; it now returns 403, mirroring the pre-create authorization check (`server/src/services/claim-service.ts`). +- A service-layer Zod validation failure — one that bypasses Fastify's schema validation — fell through to a generic 500; it now returns 400 with the field-level issues (`server/src/app.ts`). +- The ontology-save handler returned a bespoke 500 that leaked the raw error message to the client; it now re-throws to the global handler for a safe generic 500, keeping the detail in the logs (`server/src/routes/ontology.ts`). + +#### Data fidelity + +- Forking a shared video summary copied the summary body but dropped every extracted claim, leaving the fork's claim view empty. The fork now deep-copies the claim tree under fresh ids and carries the denormalized `claimsJson`, re-pointing every claim reference — row ids, parent links, and the references embedded in glosses — at the new ids (`server/src/routes/sharing.ts`). +- Cross-summary import conflict detection compared relation ids against the collection id set and read the world-state row with a narrower filter than the writer used, so it could miss or misreport conflicts. Both now key on the correct row and the relation id set (`server/src/services/import/conflict-resolver.ts`, `server/src/services/import-handler.ts`). +- A merge-keyframes import resolution on an existing annotation fell through to an unconditional create and collided on the primary key; the create is now guarded by an existence check (`server/src/services/import/entity-importers.ts`). + +#### Demo and cache hygiene + +- The idle-demo-user sweeper deleted users but never evicted their cached abilities, leaking memory as anonymous demo sessions churned; it now invalidates each swept user's ability cache (`server/src/demo/idle-reset.ts`). +- The anonymous-session mint endpoint had neither a per-route rate limit nor a population ceiling, so it could be driven to create users faster than the sweeper reclaims them; it now rate-limits per IP and refuses once a live-anonymous-user ceiling is reached (`server/src/demo/anonymous-session.ts`). +- Admin user creation accepted an independent `systemRole` that could diverge from `isAdmin` (CASL keys on the former, the admin middleware on the latter); the two are now coupled. Admin user deletion now invalidates the deleted user's cached abilities (`server/src/routes/users.ts`). + ## [0.5.8] - 2026-06-30 The 0.5.8 patch is the third and final audit-driven hardening release — the model-service slice — fixing resource and concurrency defects in the Python inference service ([#192](https://github.com/parafovea/fovea/pull/192)). Nothing is breaking. diff --git a/annotation-tool/package.json b/annotation-tool/package.json index 654e4cb2..cb4868bf 100644 --- a/annotation-tool/package.json +++ b/annotation-tool/package.json @@ -1,7 +1,7 @@ { "name": "@fovea/annotation-tool", "private": true, - "version": "0.5.8", + "version": "0.5.9", "type": "module", "scripts": { "dev": "vite", diff --git a/annotation-tool/src/api/generated/openapi.ts b/annotation-tool/src/api/generated/openapi.ts index cf9fa039..4205804b 100644 --- a/annotation-tool/src/api/generated/openapi.ts +++ b/annotation-tool/src/api/generated/openapi.ts @@ -565,6 +565,22 @@ export interface paths { }; }; }; + /** @description Default Response */ + 409: { + headers: { + [name: string]: unknown; + }; + content: { + "application/json": { + /** @description Machine-readable error code */ + error: string; + /** @description Human-readable error message */ + message: string; + /** @description Additional error context */ + details?: unknown; + }; + }; + }; }; }; post?: never; diff --git a/docs/docs/project/changelog.md b/docs/docs/project/changelog.md index e681e002..681583a7 100644 --- a/docs/docs/project/changelog.md +++ b/docs/docs/project/changelog.md @@ -11,6 +11,46 @@ All notable changes to the Fovea project will be documented in this file. The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/), and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). +## [0.5.9] - 2026-06-30 + +The 0.5.9 patch is the first of three releases remediating a second swarm audit of the shipped 0.5.x line — the backend slice — fixing authorization, lost-update concurrency, idempotency, and data-fidelity defects in the Fastify server ([#194](https://github.com/parafovea/fovea/pull/194)). Roughly a third of the findings were gaps in the first audit's own fixes: a fix addressed specific instances of a defect class without closing every instance. Nothing is breaking. + +### Fixed + +#### Authorization + +- The `/api/models/*` proxy routes were unauthenticated: any caller could read the model configuration and, worse, drive the shared inference service — selecting, loading, or unloading a model affects every user — with no credentials. The routes now require authentication, and the state-changing select/load/unload operations additionally require admin (`server/src/app.ts`, `server/src/routes/models.ts`). +- A persona could be attached to a project by a non-member, since the create ability passed for any self-owned persona regardless of the target project. Project-scoped persona creation now verifies project membership and returns 403 otherwise (`server/src/services/persona-service.ts`). +- The import "replace" paths overwrote or deleted an existing annotation, persona, summary, claim, or claim-relation found by id with no instance-level check, so a crafted import could clobber another user's row. Each replace now requires update (or delete) on the existing row, mirroring the ontology-import guard (`server/src/services/import/entity-importers.ts`). + +#### Concurrency — lost-update hardening + +- The optimistic-concurrency guard on world state and ontologies compared on `updatedAt`, which two writes landing in the same millisecond may share — letting the second silently overwrite the first. Both tables now carry a monotonic `version` column, and every guarded write compares-and-swaps on it; guard exhaustion now returns 409 and a missing row 404 rather than a generic 500 (`server/prisma/migrations/20260630120000_add_optimistic_version_and_admin_apikey_uniqueness`, `server/src/repositories/{WorldStateRepository,ProjectRepository,PersonaRepository}.ts`, `server/src/services/world-state-service.ts`). +- Persona type-deletion and persona deletion wrote the annotation delete, the ontology gloss cleanup, and the world-state cleanup as separate unguarded statements, so a partial failure left them disagreeing and a concurrent edit could be clobbered. Each now runs in a single transaction with the ontology and world-state writes routed through the version guard, recomputing from a fresh read (`server/src/services/persona-service.ts`). +- The admin "clear world state" path wrote through a bare id update that could half-survive a concurrent write; it now clears through the version guard (`server/src/services/world-state-service.ts`). +- First-access creation of a project world state raced into the compound unique constraint and surfaced a 500; it now upserts the empty row and merges through the guard (`server/src/services/project-service.ts`). +- The claim-extraction worker re-saved claims on every run with no dedup, so a job retry or double-submit duplicated every claim for a summary. Re-extraction is now idempotent: the prior extracted claims are replaced inside one transaction, preserving manually authored claims (`server/src/queues/setup.ts`). + +#### Idempotency and conflict status + +- Several create and update paths surfaced a duplicate as a 500 rather than a 409: the self-service profile email/username update, project and group membership adds, and admin API keys. Each now maps the unique-constraint violation to a 409 (`server/src/routes/{users,groups}.ts`, `server/src/services/{project-service,api-key-service}.ts`). +- Admin API keys were unconstrained, since the compound `@@unique([userId, provider])` does not bind rows with a null userId in Postgres, so the advertised 409 never fired. A partial unique index now enforces one admin key per provider, deduping any existing duplicates first (`server/prisma/migrations/20260630120000_add_optimistic_version_and_admin_apikey_uniqueness`). +- A claim create that lost the same-id race re-threw the raw unique violation as a 500 when the existing row was not updatable; it now returns 403, mirroring the pre-create authorization check (`server/src/services/claim-service.ts`). +- A service-layer Zod validation failure — one that bypasses Fastify's schema validation — fell through to a generic 500; it now returns 400 with the field-level issues (`server/src/app.ts`). +- The ontology-save handler returned a bespoke 500 that leaked the raw error message to the client; it now re-throws to the global handler for a safe generic 500, keeping the detail in the logs (`server/src/routes/ontology.ts`). + +#### Data fidelity + +- Forking a shared video summary copied the summary body but dropped every extracted claim, leaving the fork's claim view empty. The fork now deep-copies the claim tree under fresh ids and carries the denormalized `claimsJson`, re-pointing every claim reference — row ids, parent links, and the references embedded in glosses — at the new ids (`server/src/routes/sharing.ts`). +- Cross-summary import conflict detection compared relation ids against the collection id set and read the world-state row with a narrower filter than the writer used, so it could miss or misreport conflicts. Both now key on the correct row and the relation id set (`server/src/services/import/conflict-resolver.ts`, `server/src/services/import-handler.ts`). +- A merge-keyframes import resolution on an existing annotation fell through to an unconditional create and collided on the primary key; the create is now guarded by an existence check (`server/src/services/import/entity-importers.ts`). + +#### Demo and cache hygiene + +- The idle-demo-user sweeper deleted users but never evicted their cached abilities, leaking memory as anonymous demo sessions churned; it now invalidates each swept user's ability cache (`server/src/demo/idle-reset.ts`). +- The anonymous-session mint endpoint had neither a per-route rate limit nor a population ceiling, so it could be driven to create users faster than the sweeper reclaims them; it now rate-limits per IP and refuses once a live-anonymous-user ceiling is reached (`server/src/demo/anonymous-session.ts`). +- Admin user creation accepted an independent `systemRole` that could diverge from `isAdmin` (CASL keys on the former, the admin middleware on the latter); the two are now coupled. Admin user deletion now invalidates the deleted user's cached abilities (`server/src/routes/users.ts`). + ## [0.5.8] - 2026-06-30 The 0.5.8 patch is the third and final audit-driven hardening release — the model-service slice — fixing resource and concurrency defects in the Python inference service ([#192](https://github.com/parafovea/fovea/pull/192)). Nothing is breaking. diff --git a/model-service/pyproject.toml b/model-service/pyproject.toml index 971975ef..7de09350 100644 --- a/model-service/pyproject.toml +++ b/model-service/pyproject.toml @@ -4,7 +4,7 @@ build-backend = "setuptools.build_meta" [project] name = "fovea-model-service" -version = "0.5.8" +version = "0.5.9" description = "Model service for fovea video annotation tool" requires-python = ">=3.12" dependencies = [ diff --git a/package.json b/package.json index 5cf5b1d7..f03657e4 100644 --- a/package.json +++ b/package.json @@ -1,7 +1,7 @@ { "name": "fovea", "private": true, - "version": "0.5.8", + "version": "0.5.9", "description": "FOVEA - Flexible Ontology Visual Event Analyzer", "packageManager": "pnpm@10.15.0", "engines": { diff --git a/server/openapi.json b/server/openapi.json index 7169d83e..a42e8321 100644 --- a/server/openapi.json +++ b/server/openapi.json @@ -1229,6 +1229,33 @@ } } } + }, + "409": { + "description": "Default Response", + "content": { + "application/json": { + "schema": { + "type": "object", + "required": [ + "error", + "message" + ], + "properties": { + "error": { + "description": "Machine-readable error code", + "type": "string" + }, + "message": { + "description": "Human-readable error message", + "type": "string" + }, + "details": { + "description": "Additional error context" + } + } + } + } + } } } } diff --git a/server/package.json b/server/package.json index 63355ba1..a2903043 100644 --- a/server/package.json +++ b/server/package.json @@ -1,6 +1,6 @@ { "name": "@fovea/server", - "version": "0.5.8", + "version": "0.5.9", "private": true, "type": "module", "scripts": { diff --git a/server/prisma/migrations/20260630120000_add_optimistic_version_and_admin_apikey_uniqueness/migration.sql b/server/prisma/migrations/20260630120000_add_optimistic_version_and_admin_apikey_uniqueness/migration.sql new file mode 100644 index 00000000..17d87826 --- /dev/null +++ b/server/prisma/migrations/20260630120000_add_optimistic_version_and_admin_apikey_uniqueness/migration.sql @@ -0,0 +1,33 @@ +-- 0.5.9 backend correctness hardening. +-- +-- (1) Adds a monotonic `version` column to world_state and ontologies so the +-- optimistic-concurrency guard can compare-and-swap on a value that always +-- advances. The prior guard matched on `updatedAt`, which two writes landing in +-- the same millisecond can share, letting the second silently clobber the first. +-- Existing rows start at version 0; the guard increments on every write. +-- +-- (2) Adds a partial unique index enforcing one admin API key per provider +-- (userId IS NULL). The compound @@unique([userId, provider]) does not constrain +-- admin keys because Postgres treats NULL userIds as distinct, so the advertised +-- 409 never fired and duplicate admin keys could accumulate. Dedupe first. +-- +-- The partial unique index cannot be expressed in schema.prisma and is created +-- here as raw SQL; the project applies migrations with `prisma migrate deploy`, +-- which leaves it in place. + +-- AlterTable: optimistic-concurrency version tokens. +ALTER TABLE "world_state" ADD COLUMN "version" INTEGER NOT NULL DEFAULT 0; +ALTER TABLE "ontologies" ADD COLUMN "version" INTEGER NOT NULL DEFAULT 0; + +-- ApiKey: an admin key (userId NULL) must be unique per provider (BUG-15). +-- Dedupe first, treating NULL userIds as equal (IS NOT DISTINCT FROM via the +-- explicit NULL guard), keeping the most-recently-updated admin key per provider +-- (older duplicates, a rare artifact of the unconstrained path, are removed). +DELETE FROM "api_keys" a +USING "api_keys" b +WHERE a."userId" IS NULL AND b."userId" IS NULL + AND a."provider" = b."provider" + AND (a."updatedAt" < b."updatedAt" OR (a."updatedAt" = b."updatedAt" AND a.ctid > b.ctid)); + +CREATE UNIQUE INDEX "api_keys_admin_provider_key" + ON "api_keys"("provider") WHERE "userId" IS NULL; diff --git a/server/prisma/schema.prisma b/server/prisma/schema.prisma index ae521c9b..8f5f6431 100644 --- a/server/prisma/schema.prisma +++ b/server/prisma/schema.prisma @@ -220,6 +220,11 @@ model Ontology { /// JSON structure: RelationType[] relationTypes Json @default("[]") + /// Monotonic optimistic-concurrency token. Incremented on every guarded + /// write; the compare-and-swap guard matches on it (not on updatedAt, which + /// can fail to advance for two writes landing in the same millisecond). + version Int @default(0) + createdAt DateTime @default(now()) updatedAt DateTime @updatedAt @@ -249,6 +254,11 @@ model WorldState { /// JSON structure: OntologyRelation[] relations Json @default("[]") + /// Monotonic optimistic-concurrency token. Incremented on every guarded + /// write; the compare-and-swap guard matches on it (not on updatedAt, which + /// can fail to advance for two writes landing in the same millisecond). + version Int @default(0) + projectId String? project Project? @relation(fields: [projectId], references: [id], onDelete: SetNull) diff --git a/server/src/app.ts b/server/src/app.ts index a7e720c8..5a9a246c 100644 --- a/server/src/app.ts +++ b/server/src/app.ts @@ -13,8 +13,9 @@ import { videoSummarizationQueue, claimExtractionQueue, closeQueues } from './qu import { apiRequestCounter, apiRequestDuration } from './metrics.js' import { config } from './config.js' import { prisma } from './lib/prisma.js' +import { ZodError } from 'zod' import { AppError, TooManyRequestsError } from './lib/errors.js' -import { requireAdmin } from './middleware/auth.js' +import { requireAdmin, requireAuth } from './middleware/auth.js' import { recordApiError } from './lib/errorMetrics.js' /** @@ -214,6 +215,19 @@ export async function buildApp() { }) } + // Handle Zod validation errors raised by service-layer `.parse()` calls + // that bypass Fastify's TypeBox schema validation (so they carry no + // `error.validation`). These are client input faults, not server failures — + // surface 400 with the field-level issues rather than a generic 500. + if (error instanceof ZodError) { + recordApiError(method, route, 400, 'VALIDATION_ERROR', 'warning') + return reply.code(400).send({ + error: 'VALIDATION_ERROR', + message: 'Request validation failed', + details: error.issues + }) + } + // Handle rate limit errors from @fastify/rate-limit plugin // These are not AppError instances but have statusCode 429 if (error.statusCode === 429) { @@ -316,8 +330,16 @@ export async function buildApp() { const videosRoute = await import('./routes/videos/index.js') await app.register(videosRoute.default) + // The model routes proxy to the shared Python model-service and include + // state-changing operations (select/load/unload affect every user's + // inference). Register them in an encapsulated child context that requires + // authentication so they are never world-readable/-writable; the mutating + // routes additionally require admin (declared per-route in models.ts). const modelsRoute = await import('./routes/models.js') - await app.register(modelsRoute.default) + await app.register(async (modelsScope) => { + modelsScope.addHook('onRequest', requireAuth) + await modelsScope.register(modelsRoute.default) + }) const annotationsRoute = await import('./routes/annotations.js') await app.register(annotationsRoute.default) diff --git a/server/src/demo/anonymous-session.ts b/server/src/demo/anonymous-session.ts index b01baa87..84ddbec8 100644 --- a/server/src/demo/anonymous-session.ts +++ b/server/src/demo/anonymous-session.ts @@ -26,6 +26,7 @@ import type { FastifyInstance, FastifyPluginAsync } from 'fastify' import { prisma } from '../lib/prisma.js' import { authService } from '../services/auth-service.js' import { config } from '../config.js' +import { TooManyRequestsError } from '../lib/errors.js' import { isAnonymousAuthAllowed } from './config.js' interface AnonymousSessionResponse { @@ -36,6 +37,17 @@ interface AnonymousSessionResponse { const ANON_USERNAME_PREFIX = 'demo-anonymous-' const ANON_SESSION_TTL_DAYS = 1 +/** + * Ceiling on the number of live anonymous demo users. The idle-reset sweeper + * reclaims idle users every minute, but a flood of create requests can mint + * rows faster than the sweeper deletes them. Refusing new sessions once this + * many anonymous users already exist bounds the population regardless of + * request rate, so the create path can never outrun the sweeper. Sized for + * booth-scale concurrency (well above the few hundred visitors a CVPR demo + * sees at once) while still capping a runaway create loop. + */ +const MAX_ANONYMOUS_USERS = 500 + const anonymousSessionPlugin: FastifyPluginAsync = async (app: FastifyInstance) => { if (!isAnonymousAuthAllowed()) { app.log.info('[demo] anonymous-session endpoint NOT registered (flag off)') @@ -61,6 +73,19 @@ const anonymousSessionPlugin: FastifyPluginAsync = async (app: FastifyInstance) }, }, }, + // Tight per-route cap on top of the global limit, keyed by IP via the + // global @fastify/rate-limit keyGenerator. Minting a User + Session is + // far more expensive than a typical request, so a single visitor has no + // legitimate reason to hit this more than a few times a minute. The + // plugin is not registered in test mode (config.server.isTest), so this + // config is inert there and the count-ceiling guard below is the real + // protection. + config: { + rateLimit: { + max: 5, + timeWindow: '1 minute', + }, + }, }, async (request, reply) => { // Reuse the existing anonymous user if the visitor's session @@ -88,6 +113,26 @@ const anonymousSessionPlugin: FastifyPluginAsync = async (app: FastifyInstance) } } + // Cap the live anonymous-user population before minting a new one. This + // is the primary defense: it bounds the number of rows regardless of how + // fast the route is hit, so the create path cannot outrun the idle-reset + // sweeper even if the rate limit is disabled (as in test mode). Anonymous + // demo users are identified by the username prefix, the same predicate + // the sweeper uses. + const anonymousCount = await prisma.user.count({ + where: { username: { startsWith: ANON_USERNAME_PREFIX } }, + }) + if (anonymousCount >= MAX_ANONYMOUS_USERS) { + request.log.warn( + { anonymousCount, ceiling: MAX_ANONYMOUS_USERS }, + '[demo] anonymous-session refused: live anonymous-user ceiling reached', + ) + throw new TooManyRequestsError( + 'The demo is at capacity. Please try again in a few minutes.', + 60, + ) + } + const suffix = crypto.randomBytes(6).toString('hex') const username = `${ANON_USERNAME_PREFIX}${suffix}` diff --git a/server/src/demo/idle-reset.ts b/server/src/demo/idle-reset.ts index a100bfcc..a9baa09e 100644 --- a/server/src/demo/idle-reset.ts +++ b/server/src/demo/idle-reset.ts @@ -19,6 +19,7 @@ import type { FastifyInstance } from 'fastify' import { prisma } from '../lib/prisma.js' +import { invalidateUserAbilities } from '../middleware/abilities.js' const IDLE_WINDOW_MS = 10 * 60 * 1000 // 10 minutes (plan §5.3) const SWEEP_INTERVAL_MS = 60 * 1000 // 1 minute @@ -63,5 +64,14 @@ async function sweepOnce(app: FastifyInstance): Promise { const ids = stale.map((u: { id: string }) => u.id) const deleted = await prisma.user.deleteMany({ where: { id: { in: ids } } }) + + // Evict the deleted users from the per-user ability cache. buildAbilities + // populates this map on first request and never expires entries, so without + // explicit eviction the cache grows by one entry per anonymous visitor and + // never shrinks as demo sessions churn. + for (const id of ids) { + invalidateUserAbilities(id) + } + app.log.info({ swept: deleted.count }, '[demo] idle-reset swept stale anonymous users') } diff --git a/server/src/queues/setup.ts b/server/src/queues/setup.ts index 3481fe50..18305352 100644 --- a/server/src/queues/setup.ts +++ b/server/src/queues/setup.ts @@ -648,111 +648,141 @@ export const claimWorker = new Worker< await job.updateProgress(60); - // Save claims to database - async function saveClaim( - claimData: (typeof modelResponse.claims)[0], - parentClaimId?: string, - ): Promise { - const claim = await prisma.claim.create({ - data: { - summaryId, - summaryType, - // Inherit the parent summary's project scope so extracted claims are - // project-visible (mirrors the interactive claim-create paths). - projectId: summary?.projectId ?? undefined, - // Owned by the user who requested extraction, so it is readable by - // its creator (mirrors the interactive claim-create paths). - createdBy: createdBy ?? undefined, - text: claimData.text, - gloss: [], - parentClaimId, - textSpans: claimData.char_start - ? [ - { - sentenceIndex: claimData.sentence_index, - charStart: claimData.char_start, - charEnd: claimData.char_end, - }, - ] - : undefined, - confidence: claimData.confidence, - modelUsed: modelResponse.model_used, - extractionStrategy: config.extractionStrategy, - }, - }); - - // Recursively save subclaims - if (claimData.subclaims && claimData.subclaims.length > 0) { - for (const subclaimData of claimData.subclaims as (typeof modelResponse.claims)[0][]) { - await saveClaim(subclaimData, claim.id); + const countClaims = (claims: unknown[]): number => { + let count = claims.length; + for (const claim of claims as { subclaims?: unknown[] }[]) { + if (claim.subclaims && claim.subclaims.length > 0) { + count += countClaims(claim.subclaims); } } + return count; + }; - return claim.id; - } + // Persist the extracted claims idempotently. A job retry or double-submit + // would otherwise duplicate every claim, since each create always inserts a + // new row. To keep extraction idempotent per summary, delete the summary's + // previously extracted claims, re-insert the fresh set, and rewrite the + // denormalized claimsJson in a single transaction so a partial failure + // never leaves the summary with a mix of old and new claims. + // + // The delete is scoped to model-extracted claims only (extractionStrategy + // other than "manual"); manually authored claims are stamped with + // extractionStrategy "manual" by the interactive create paths and must be + // preserved across re-extraction. Deleting the extracted root claims also + // removes their subclaims via the schema's onDelete: Cascade, but the + // extracted subclaims carry the same non-"manual" extractionStrategy and so + // fall within the delete scope regardless. + const { allClaims, totalClaims } = await prisma.$transaction( + async (tx) => { + // Remove the previously extracted claims for this summary, leaving any + // manually authored claims in place. + await tx.claim.deleteMany({ + where: { + summaryId, + summaryType, + extractionStrategy: { not: "manual" }, + }, + }); + + // Re-insert the freshly extracted claim tree. + const saveClaim = async ( + claimData: (typeof modelResponse.claims)[0], + parentClaimId?: string, + ): Promise => { + const claim = await tx.claim.create({ + data: { + summaryId, + summaryType, + // Inherit the parent summary's project scope so extracted claims + // are project-visible (mirrors the interactive claim-create + // paths). + projectId: summary?.projectId ?? undefined, + // Owned by the user who requested extraction, so it is readable by + // its creator (mirrors the interactive claim-create paths). + createdBy: createdBy ?? undefined, + text: claimData.text, + gloss: [], + parentClaimId, + textSpans: claimData.char_start + ? [ + { + sentenceIndex: claimData.sentence_index, + charStart: claimData.char_start, + charEnd: claimData.char_end, + }, + ] + : undefined, + confidence: claimData.confidence, + modelUsed: modelResponse.model_used, + extractionStrategy: config.extractionStrategy, + }, + }); - // Save all root claims - for (const claimData of modelResponse.claims) { - await saveClaim(claimData); - } + // Recursively save subclaims + if (claimData.subclaims && claimData.subclaims.length > 0) { + for (const subclaimData of claimData.subclaims as (typeof modelResponse.claims)[0][]) { + await saveClaim(subclaimData, claim.id); + } + } - await job.updateProgress(80); + return claim.id; + }; - // Update denormalized JSON - const allClaims = await prisma.claim.findMany({ - where: { - summaryId, - summaryType, - parentClaimId: null, - }, - include: { - subclaims: { + // Save all root claims + for (const claimData of modelResponse.claims) { + await saveClaim(claimData); + } + + // Rebuild the denormalized JSON from the current claim tree. + const rootClaims = await tx.claim.findMany({ + where: { + summaryId, + summaryType, + parentClaimId: null, + }, include: { subclaims: { include: { - subclaims: true, + subclaims: { + include: { + subclaims: true, + }, + }, }, }, }, - }, - }, - orderBy: [{ createdAt: "asc" }], - }); + orderBy: [{ createdAt: "asc" }], + }); - const countClaims = (claims: unknown[]): number => { - let count = claims.length; - for (const claim of claims as { subclaims?: unknown[] }[]) { - if (claim.subclaims && claim.subclaims.length > 0) { - count += countClaims(claim.subclaims); - } - } - return count; - }; + const treeTotalClaims = countClaims(rootClaims); - const totalClaims = countClaims(allClaims); + const claimsJson = { + version: "1.0", + claims: rootClaims, + metadata: { + extractedAt: new Date().toISOString(), + totalClaims: treeTotalClaims, + totalSubclaims: treeTotalClaims - rootClaims.length, + maxDepth: config.maxSubclaimDepth || 3, + }, + }; - const claimsJson = { - version: "1.0", - claims: allClaims, - metadata: { - extractedAt: new Date().toISOString(), - totalClaims, - totalSubclaims: totalClaims - allClaims.length, - maxDepth: config.maxSubclaimDepth || 3, - }, - }; + if (summaryType === "video") { + await tx.videoSummary.update({ + where: { id: summaryId }, + data: { + // eslint-disable-next-line @typescript-eslint/no-explicit-any -- Prisma JSON type requires any + claimsJson: claimsJson as any, + claimsExtractedAt: new Date(), + }, + }); + } - if (summaryType === "video") { - await prisma.videoSummary.update({ - where: { id: summaryId }, - data: { - // eslint-disable-next-line @typescript-eslint/no-explicit-any -- Prisma JSON type requires any - claimsJson: claimsJson as any, - claimsExtractedAt: new Date(), - }, - }); - } + return { allClaims: rootClaims, totalClaims: treeTotalClaims }; + }, + ); + await job.updateProgress(80); await job.updateProgress(100); return { diff --git a/server/src/repositories/PersonaRepository.ts b/server/src/repositories/PersonaRepository.ts index ff979b1b..82e65ed7 100644 --- a/server/src/repositories/PersonaRepository.ts +++ b/server/src/repositories/PersonaRepository.ts @@ -1,4 +1,5 @@ import { PrismaClient, Persona, Ontology, Prisma } from '@prisma/client' +import { ConflictError, NotFoundError } from '../lib/errors.js' /** * Persona row joined with its ontology. @@ -123,35 +124,25 @@ export class PersonaRepository { return this.prisma.persona.delete({ where: { id } }) } - /** - * Updates a persona's ontology, keyed by personaId. - * - * @param personaId - Persona UUID owning the ontology - * @param data - Prisma ontology update input - * @returns the updated ontology - */ - async updateOntology(personaId: string, data: Prisma.OntologyUpdateInput): Promise { - return this.prisma.ontology.update({ - where: { personaId }, - data - }) - } - /** * Applies an optimistic-concurrency update to a persona's ontology row. * * Reads the current ontology row (by personaId), lets `transform` compute the - * new column values from it, then writes them guarded by the row's - * `updatedAt`. If a concurrent writer committed first the guard misses (count - * 0) and we retry against the freshly-read row, so both writes land instead of - * one silently clobbering the other. This is what makes the whole-blob - * ontology PUT a safe per-id merge under concurrent writers (the transform - * re-runs on fresh state each attempt). + * new column values from it, then writes them guarded by the row's `version`. + * If a concurrent writer committed first the version has advanced, the guard + * misses (count 0), and we retry against the freshly-read row, so both writes + * land instead of one silently clobbering the other. The guard keys on the + * monotonic `version` rather than `updatedAt` because two writes landing in + * the same millisecond can share an `updatedAt`, which would let the second + * silently overwrite the first. This is what makes the whole-blob ontology PUT + * a safe per-id merge under concurrent writers (the transform re-runs on fresh + * state each attempt). * * @param personaId - Persona UUID owning the ontology * @param transform - computes the Prisma update input from the current row * @returns the updated ontology - * @throws when no ontology row exists or the write keeps conflicting + * @throws {NotFoundError} when no ontology row exists + * @throws {ConflictError} when the write keeps conflicting after retries */ async updateOntologyOptimistic( personaId: string, @@ -162,17 +153,17 @@ export class PersonaRepository { where: { personaId }, }) if (!current) { - throw new Error('No ontology to update') + throw new NotFoundError('Ontology', personaId) } const result = await this.prisma.ontology.updateMany({ - where: { id: current.id, updatedAt: current.updatedAt }, - data: { ...transform(current), updatedAt: new Date() }, + where: { id: current.id, version: current.version }, + data: { ...transform(current), version: { increment: 1 } }, }) if (result.count === 1) { return this.prisma.ontology.findUniqueOrThrow({ where: { id: current.id } }) } } - throw new Error('Ontology update conflicted after retries') + throw new ConflictError('Ontology update conflicted after retries') } /** @@ -218,14 +209,4 @@ export class PersonaRepository { where: { userId, projectId: null } }) } - - /** - * Updates a world state row by ID. - * - * @param id - World state ID - * @param data - Prisma world state update input - */ - async updateWorldState(id: string, data: Prisma.WorldStateUpdateInput): Promise { - await this.prisma.worldState.update({ where: { id }, data }) - } } diff --git a/server/src/repositories/ProjectRepository.ts b/server/src/repositories/ProjectRepository.ts index f46f0f67..ee738808 100644 --- a/server/src/repositories/ProjectRepository.ts +++ b/server/src/repositories/ProjectRepository.ts @@ -1,4 +1,5 @@ import { PrismaClient, Project, ProjectMembership, Persona, WorldState, Prisma } from '@prisma/client' +import { ConflictError, NotFoundError } from '../lib/errors.js' /** * Project row joined with its members (each carrying the public user @@ -445,10 +446,13 @@ export class ProjectRepository { * state. * * Reads the current (userId, projectId) row, lets `transform` compute the new - * column values from it, then writes them guarded by the row's `updatedAt`. - * If a concurrent writer committed first the guard misses (count 0) and we - * retry against the freshly-read row, so both writes land instead of one - * silently clobbering the other. Project world state is multi-user (every + * column values from it, then writes them guarded by the row's `version`. If + * a concurrent writer committed first the version has advanced, the guard + * misses (count 0), and we retry against the freshly-read row, so both writes + * land instead of one silently clobbering the other. The guard keys on the + * monotonic `version` rather than `updatedAt` because two writes landing in + * the same millisecond can share an `updatedAt`, which would let the second + * silently overwrite the first. Project world state is multi-user (every * member shares the row), which makes this guard essential for the whole-blob * PUT to be a safe per-id merge under concurrent writers (the transform * re-runs on fresh state each attempt). @@ -457,7 +461,8 @@ export class ProjectRepository { * @param projectId - Project UUID * @param transform - computes the Prisma update input from the current row * @returns the updated world state - * @throws when no project row exists or the write keeps conflicting + * @throws {NotFoundError} when no project row exists + * @throws {ConflictError} when the write keeps conflicting after retries */ async updateProjectWorldStateOptimistic( userId: string, @@ -469,16 +474,16 @@ export class ProjectRepository { where: { userId_projectId: { userId, projectId } }, }) if (!current) { - throw new Error('No project world state to update') + throw new NotFoundError('Project world state', `${userId}:${projectId}`) } const result = await this.prisma.worldState.updateMany({ - where: { userId, projectId, updatedAt: current.updatedAt }, - data: { ...transform(current), updatedAt: new Date() }, + where: { userId, projectId, version: current.version }, + data: { ...transform(current), version: { increment: 1 } }, }) if (result.count === 1) { return this.prisma.worldState.findUniqueOrThrow({ where: { id: current.id } }) } } - throw new Error('Project world state update conflicted after retries') + throw new ConflictError('Project world state update conflicted after retries') } } diff --git a/server/src/repositories/WorldStateRepository.ts b/server/src/repositories/WorldStateRepository.ts index fe928d7f..867164af 100644 --- a/server/src/repositories/WorldStateRepository.ts +++ b/server/src/repositories/WorldStateRepository.ts @@ -1,4 +1,5 @@ import { PrismaClient, User, WorldState, Prisma } from '@prisma/client' +import { ConflictError, NotFoundError } from '../lib/errors.js' /** * Persona row joined with its ontology. @@ -114,34 +115,24 @@ export class WorldStateRepository { return this.prisma.worldState.create({ data }) } - /** - * Updates a world state row by ID. - * - * @param id - World state ID - * @param data - Prisma world state update input - * @returns the updated world state - */ - async updateWorldState(id: string, data: Prisma.WorldStateUpdateInput): Promise { - return this.prisma.worldState.update({ - where: { id }, - data - }) - } - /** * Applies an optimistic-concurrency update to a user's personal world state. * * Reads the current row, lets `transform` compute the new column values from - * it, then writes them guarded by the row's `updatedAt`. If a concurrent - * writer committed first the guard misses (count 0) and we retry against the - * freshly-read row, so both writes land instead of one silently clobbering - * the other. This is what makes the whole-blob PUT a safe per-id merge under + * it, then writes them guarded by the row's `version`. If a concurrent writer + * committed first the version has advanced, the guard misses (count 0), and we + * retry against the freshly-read row, so both writes land instead of one + * silently clobbering the other. The guard keys on the monotonic `version` + * rather than `updatedAt` because two writes landing in the same millisecond + * can share an `updatedAt`, which would let the second silently overwrite the + * first. This is what makes the whole-blob PUT a safe per-id merge under * concurrent writers (the transform re-runs on fresh state each attempt). * * @param userId - owning user ID * @param transform - computes the Prisma update input from the current row * @returns the updated world state - * @throws when no personal row exists or the write keeps conflicting + * @throws {NotFoundError} when no personal row exists + * @throws {ConflictError} when the write keeps conflicting after retries */ async updatePersonalWorldStateOptimistic( userId: string, @@ -152,17 +143,17 @@ export class WorldStateRepository { where: { userId, projectId: null }, }) if (!current) { - throw new Error('No personal world state to update') + throw new NotFoundError('World state', userId) } const result = await this.prisma.worldState.updateMany({ - where: { id: current.id, updatedAt: current.updatedAt }, - data: { ...transform(current), updatedAt: new Date() }, + where: { id: current.id, version: current.version }, + data: { ...transform(current), version: { increment: 1 } }, }) if (result.count === 1) { return this.prisma.worldState.findUniqueOrThrow({ where: { id: current.id } }) } } - throw new Error('Personal world state update conflicted after retries') + throw new ConflictError('Personal world state update conflicted after retries') } /** diff --git a/server/src/routes/groups.ts b/server/src/routes/groups.ts index 9569c049..17c74a65 100644 --- a/server/src/routes/groups.ts +++ b/server/src/routes/groups.ts @@ -556,18 +556,28 @@ const groupsRoute: FastifyPluginAsync = async (fastify) => { throw new ConflictError('User is already a member of this group') } - const membership = await fastify.prisma.groupMembership.create({ - data: { - userId: targetUserId, - groupId, - role: targetRole, - }, - include: { - user: { - select: { id: true, username: true, displayName: true, email: true }, + let membership + try { + membership = await fastify.prisma.groupMembership.create({ + data: { + userId: targetUserId, + groupId, + role: targetRole, }, - }, - }) + include: { + user: { + select: { id: true, username: true, displayName: true, email: true }, + }, + }, + }) + } catch (error: unknown) { + // The existence pre-check narrows the common case, but a concurrent add + // can race past it and hit the unique constraint — return 409, not 500. + if (error instanceof Prisma.PrismaClientKnownRequestError && error.code === 'P2002') { + throw new ConflictError('User is already a member of this group') + } + throw error + } // Newly added member's abilities now include group-scope roles invalidateUserAbilities(targetUserId) @@ -1053,18 +1063,28 @@ const groupsRoute: FastifyPluginAsync = async (fastify) => { throw new ConflictError('User is already a member of this group') } - const membership = await fastify.prisma.groupMembership.create({ - data: { - userId: targetUserId, - groupId, - role: targetRole, - }, - include: { - user: { - select: { id: true, username: true, displayName: true, email: true }, + let membership + try { + membership = await fastify.prisma.groupMembership.create({ + data: { + userId: targetUserId, + groupId, + role: targetRole, }, - }, - }) + include: { + user: { + select: { id: true, username: true, displayName: true, email: true }, + }, + }, + }) + } catch (error: unknown) { + // A concurrent add can race past the existence pre-check and hit the + // unique constraint — return 409, not 500. + if (error instanceof Prisma.PrismaClientKnownRequestError && error.code === 'P2002') { + throw new ConflictError('User is already a member of this group') + } + throw error + } // Admin-added member's abilities must pick up group-scope roles invalidateUserAbilities(targetUserId) diff --git a/server/src/routes/models.ts b/server/src/routes/models.ts index 84f534af..3ff69246 100644 --- a/server/src/routes/models.ts +++ b/server/src/routes/models.ts @@ -3,6 +3,7 @@ import axios, { AxiosError } from 'axios' import camelcaseKeys from 'camelcase-keys' import { config } from '../config.js' import { InternalError, ValidationError } from '../lib/errors.js' +import { requireAdmin } from '../middleware/auth.js' /** * Map a user-supplied taskType to one of the nine canonical @@ -224,6 +225,7 @@ const modelsRoute: FastifyPluginAsync = async (fastify) => { modelName: string } }>('/api/models/select', { + onRequest: [requireAdmin], schema: { description: 'Select model for task type', tags: ['models'], @@ -384,6 +386,7 @@ const modelsRoute: FastifyPluginAsync = async (fastify) => { fastify.post<{ Params: { taskType: string } }>('/api/models/load/:taskType', { + onRequest: [requireAdmin], schema: { description: 'Load model for task type', tags: ['models'], @@ -430,6 +433,7 @@ const modelsRoute: FastifyPluginAsync = async (fastify) => { fastify.post<{ Params: { taskType: string } }>('/api/models/unload/:taskType', { + onRequest: [requireAdmin], schema: { description: 'Unload model for task type', tags: ['models'], diff --git a/server/src/routes/ontology.ts b/server/src/routes/ontology.ts index 6acdc8e3..a4ecaed6 100644 --- a/server/src/routes/ontology.ts +++ b/server/src/routes/ontology.ts @@ -419,10 +419,10 @@ const ontologyRoute: FastifyPluginAsync = async (fastify) => { fastify.log.error(`Prisma error meta: ${JSON.stringify((error as Record).meta)}`) } } - return reply.code(500).send({ - error: 'Failed to save ontology data', - details: error instanceof Error ? error.message : 'Unknown error' - }) + // Re-throw to the global handler, which returns a safe generic 500. The + // raw error message (which can carry DB/internal detail) is logged above + // but never sent to the client. + throw new InternalError('Failed to save ontology data') } }) diff --git a/server/src/routes/sharing.ts b/server/src/routes/sharing.ts index 1f680e85..b1660d2e 100644 --- a/server/src/routes/sharing.ts +++ b/server/src/routes/sharing.ts @@ -8,6 +8,7 @@ * @module */ +import { randomUUID } from 'node:crypto' import { Type, Static } from '@sinclair/typebox' import { FastifyPluginAsync } from 'fastify' import { Prisma, PrismaClient } from '@prisma/client' @@ -17,6 +18,112 @@ function toJson(value: unknown): Prisma.InputJsonValue { return JSON.parse(JSON.stringify(value)) } +/** + * Deep-copies a JSON value, replacing any string leaf that is a key in `idMap` + * with its mapped value. The fork re-points every claim-id reference at the + * new claim ids in one pass: row ids, `parentClaimId`, and the `$` claim + * references embedded in gloss arrays. Claim ids are UUIDs, so a non-id string + * cannot collide with a map key. Returns a fresh structure (the input is not + * mutated). + */ +function remapClaimIds(value: unknown, idMap: Map): unknown { + if (typeof value === 'string') { + return idMap.get(value) ?? value + } + if (Array.isArray(value)) { + return value.map((item) => remapClaimIds(item, idMap)) + } + if (value && typeof value === 'object') { + const out: Record = {} + for (const [key, item] of Object.entries(value)) { + out[key] = remapClaimIds(item, idMap) + } + return out + } + return value +} + +/** A claim's nullable JSON field copied through the id remap, or JSON null. */ +function remappedJsonOrNull( + value: Prisma.JsonValue | null, + idMap: Map +): Prisma.InputJsonValue | typeof Prisma.JsonNull { + return value === null || value === undefined + ? Prisma.JsonNull + : toJson(remapClaimIds(value, idMap)) +} + +/** + * Deep-copies a summary's claim rows under a new summary, re-pointing every + * claim id (and the references between claims) at fresh ids via `idMap`. + * + * Rows are inserted parents-before-children so each `parentClaimId` foreign key + * resolves at insert time. Every copied claim is stamped with the forker as + * `createdBy` and left in personal scope (no projectId), matching the forked + * summary and persona. World-object references (claimEventId/claimTimeId/ + * claimLocationId) are not remapped: world state is not forked, so they keep + * pointing at the same object ids the source named. + * + * @param tx - the Prisma transaction client running the fork + * @param sourceClaims - the source summary's claim rows (flat list) + * @param idMap - source claim id to forked claim id + * @param newSummaryId - id of the forked summary the copies attach to + * @param ownerUserId - UUID of the user who will own the forked claims + */ +async function forkClaimRows( + tx: Prisma.TransactionClient, + sourceClaims: Array>, + idMap: Map, + newSummaryId: string, + ownerUserId: string +): Promise { + const byOldId = new Map(sourceClaims.map((c) => [c.id, c])) + const inserted = new Set() + + const insert = async (claim: Prisma.ClaimGetPayload): Promise => { + if (inserted.has(claim.id)) { + return + } + // Ensure the parent row exists first so the FK resolves. Only intra-summary + // parents are remapped; an out-of-summary parent (should not occur) falls + // through to null rather than dangling. + if (claim.parentClaimId && byOldId.has(claim.parentClaimId)) { + await insert(byOldId.get(claim.parentClaimId)!) + } + await tx.claim.create({ + data: { + id: idMap.get(claim.id)!, + summaryId: newSummaryId, + summaryType: claim.summaryType, + text: claim.text, + gloss: toJson(remapClaimIds(claim.gloss, idMap)), + parentClaimId: claim.parentClaimId ? idMap.get(claim.parentClaimId) ?? null : null, + textSpans: remappedJsonOrNull(claim.textSpans, idMap), + timeSpans: remappedJsonOrNull(claim.timeSpans, idMap), + claimerType: claim.claimerType, + claimerGloss: remappedJsonOrNull(claim.claimerGloss, idMap), + claimRelation: remappedJsonOrNull(claim.claimRelation, idMap), + claimEventId: claim.claimEventId, + claimTimeId: claim.claimTimeId, + claimLocationId: claim.claimLocationId, + confidence: claim.confidence, + modelUsed: claim.modelUsed, + extractionStrategy: claim.extractionStrategy, + audio: claim.audio === null ? Prisma.JsonNull : toJson(claim.audio), + video: claim.video === null ? Prisma.JsonNull : toJson(claim.video), + metadata: claim.metadata === null ? Prisma.JsonNull : toJson(claim.metadata), + comment: claim.comment, + createdBy: ownerUserId, + }, + }) + inserted.add(claim.id) + } + + for (const claim of sourceClaims) { + await insert(claim) + } +} + /** * Deep-forks a persona (and its ontology) into a new persona owned by * `ownerUserId`. @@ -110,7 +217,19 @@ async function forkSummary( // lands in the forker's scope under a NEW personaId, keeping the same videoId. const forkedPersona = await forkPersona(tx, source.personaId, ownerUserId) - return tx.videoSummary.create({ + // Build the claim id remap up front, from the source summary's claim rows, so + // the copied rows AND the denormalized claimsJson re-point at the same new + // ids. Without this the fork drops every extracted claim, leaving the forked + // summary's claim view empty. + const sourceClaims = await tx.claim.findMany({ + where: { summaryId: sourceSummaryId, summaryType: 'video' }, + }) + const idMap = new Map() + for (const claim of sourceClaims) { + idMap.set(claim.id, randomUUID()) + } + + const forked = await tx.videoSummary.create({ data: { videoId: source.videoId, personaId: forkedPersona.id, @@ -125,10 +244,23 @@ async function forkSummary( audioLanguage: source.audioLanguage, speakerCount: source.speakerCount, comment: source.comment, + // Carry the denormalized claim view (re-pointed at the forked claim ids) + // and its extraction metadata so the fork reads identically to the source. + claimsJson: + source.claimsJson === null + ? Prisma.JsonNull + : toJson(remapClaimIds(source.claimsJson, idMap)), + claimsVersion: source.claimsVersion, + claimsExtractedAt: source.claimsExtractedAt, createdBy: ownerUserId, }, select: { id: true }, }) + + // Deep-copy the claim rows themselves under the forked summary, parents first. + await forkClaimRows(tx, sourceClaims, idMap, forked.id, ownerUserId) + + return forked } import { trace } from '@opentelemetry/api' import { requireAuth } from '@middleware/auth.js' diff --git a/server/src/routes/users.ts b/server/src/routes/users.ts index fa80e6c3..1e576ec2 100644 --- a/server/src/routes/users.ts +++ b/server/src/routes/users.ts @@ -51,8 +51,7 @@ const createUserSchema = z.object({ email: z.string().email().optional().nullable(), password: z.string().min(6, 'Password must be at least 6 characters'), displayName: z.string().min(1, 'Display name is required'), - isAdmin: z.boolean().optional().default(false), - systemRole: z.enum(['user', 'system_admin']).optional() + isAdmin: z.boolean().optional().default(false) }) /** @@ -147,7 +146,8 @@ const usersRoute: FastifyPluginAsync = async (fastify) => { 200: UserSchema, 401: Type.Object({ error: Type.String() - }) + }), + 409: ErrorResponseSchema } } }, async (request, reply) => { @@ -174,19 +174,32 @@ const usersRoute: FastifyPluginAsync = async (fastify) => { updateData.passwordHash = await bcrypt.hash(validatedData.password, 12) } - const user = await fastify.prisma.user.update({ - where: { id: request.user.id }, - data: updateData, - select: { - id: true, - username: true, - email: true, - displayName: true, - isAdmin: true, - createdAt: true, - updatedAt: true + let user + try { + user = await fastify.prisma.user.update({ + where: { id: request.user.id }, + data: updateData, + select: { + id: true, + username: true, + email: true, + displayName: true, + isAdmin: true, + createdAt: true, + updatedAt: true + } + }) + } catch (error: unknown) { + // A duplicate email/username is a client conflict, not a server fault — + // surface 409 with the offending field rather than letting it 500. + if (error instanceof Prisma.PrismaClientKnownRequestError && error.code === 'P2002') { + throw new ConflictError(conflictMessageFromP2002(error, { + email: 'A user with this email already exists', + username: 'A user with this username already exists', + })) } - }) + throw error + } // A password change must invalidate every existing session so a stolen or // shared token cannot survive the reset. Revoke all of this user's sessions, @@ -301,7 +314,10 @@ const usersRoute: FastifyPluginAsync = async (fastify) => { passwordHash, displayName: validatedData.displayName, isAdmin: validatedData.isAdmin, - ...(validatedData.systemRole ? { systemRole: validatedData.systemRole } : {}) + // Derive systemRole from isAdmin so the two admin signals cannot + // diverge: requireAdmin gates on isAdmin while CASL `manage all` + // gates on systemRole. Mirrors the coupling in the update handler. + systemRole: validatedData.isAdmin ? 'system_admin' : 'user' }, select: { id: true, @@ -531,6 +547,9 @@ const usersRoute: FastifyPluginAsync = async (fastify) => { await fastify.prisma.user.delete({ where: { id: userId } }) + // Clear the deleted user's cached abilities so a lingering in-memory entry + // cannot outlive the account if its id is ever observed again. + invalidateUserAbilities(userId) return reply.send({ success: true }) } catch (error: unknown) { if (error && typeof error === 'object' && 'code' in error && error.code === 'P2025') { diff --git a/server/src/services/api-key-service.ts b/server/src/services/api-key-service.ts index 7cf26e17..faaf9c4a 100644 --- a/server/src/services/api-key-service.ts +++ b/server/src/services/api-key-service.ts @@ -1,5 +1,6 @@ -import { PrismaClient, ApiKey } from '@prisma/client' +import { PrismaClient, ApiKey, Prisma } from '@prisma/client' import { config } from '../config.js' +import { ConflictError } from '../lib/errors.js' import { encryptApiKey, decryptApiKey } from '../lib/encryption.js' /** @@ -67,6 +68,7 @@ export async function getApiKey( * @param data.keyName - Human-readable key name * @param data.apiKey - Plaintext API key to encrypt * @returns Created API key without encrypted data + * @throws {ConflictError} when an admin key already exists for the provider */ export async function createApiKey( prisma: PrismaClient, @@ -77,19 +79,50 @@ export async function createApiKey( apiKey: string } ): Promise { + const isAdminKey = data.userId === null + const conflictMessage = isAdminKey + ? 'Admin API key for this provider already exists' + : 'API key for this provider already exists' + + // The @@unique([userId, provider]) constraint does not prevent duplicate admin + // keys: Postgres treats NULL userIds as distinct, so two admin keys for the + // same provider both satisfy the index. A partial unique index on + // api_keys(provider) WHERE userId IS NULL (added by a separate migration) + // closes that gap; pre-check here so the conflict surfaces deterministically + // even before the index is in place. + if (isAdminKey) { + const existingAdminKey = await prisma.apiKey.findFirst({ + where: { userId: null, provider: data.provider } + }) + if (existingAdminKey) { + throw new ConflictError(conflictMessage) + } + } + const { encrypted, mask } = encryptApiKey(data.apiKey) - const key = await prisma.apiKey.create({ - data: { - userId: data.userId, - provider: data.provider, - keyName: data.keyName, - encryptedKey: encrypted, - keyMask: mask, - isActive: true, - usageCount: 0 + let key + try { + key = await prisma.apiKey.create({ + data: { + userId: data.userId, + provider: data.provider, + keyName: data.keyName, + encryptedKey: encrypted, + keyMask: mask, + isActive: true, + usageCount: 0 + } + }) + } catch (error) { + // A concurrent create can race past the pre-check into the unique + // constraint (the @@unique for user keys, the partial index for admin + // keys) — surface 409, not 500. + if (error instanceof Prisma.PrismaClientKnownRequestError && error.code === 'P2002') { + throw new ConflictError(conflictMessage) } - }) + throw error + } // eslint-disable-next-line @typescript-eslint/no-unused-vars const { encryptedKey, ...rest } = key diff --git a/server/src/services/claim-service.ts b/server/src/services/claim-service.ts index 13ea4fe7..f507a9a7 100644 --- a/server/src/services/claim-service.ts +++ b/server/src/services/claim-service.ts @@ -478,7 +478,15 @@ export class ClaimService { } catch (err) { if (id && err instanceof Prisma.PrismaClientKnownRequestError && err.code === 'P2002') { const existing = await this.repository.findClaimById(id) - if (existing && ability.can('update', subject('Claim', existing))) { + if (existing) { + // A concurrent insert with the same id won the race. Re-authorize + // against the existing row (so a caller cannot hijack another user's + // claim by supplying its id), then return the current tree + // idempotently — mirroring the pre-create idempotent path above. A + // denied update is a 403, never a raw P2002 surfacing as a 500. + if (!ability.can('update', subject('Claim', existing))) { + throw new ForbiddenError('Cannot create this Claim') + } return this.repository.findClaimTree(summaryId, summaryType) } } diff --git a/server/src/services/import-handler.ts b/server/src/services/import-handler.ts index fb31077a..8b5be42d 100644 --- a/server/src/services/import-handler.ts +++ b/server/src/services/import-handler.ts @@ -22,8 +22,7 @@ import { Resolution, ImportOptions, ImportResult, - DependencyGraph, - ExistingData + DependencyGraph } from './import-types.js' import { SequenceValidator } from './import-validator.js' import { parseLine, validateLine } from './import/line-parser.js' @@ -33,7 +32,8 @@ import { generateCrossUserResolutions, isCrossUserImport, remapIds, - resolveConflicts + resolveConflicts, + ExistingDataWithRelations } from './import/conflict-resolver.js' import { EntityImporter, ImportedIds } from './import/entity-importers.js' @@ -93,7 +93,7 @@ export class ImportHandler { * @param existingData - existing data in database * @returns array of conflicts */ - async detectConflicts(lines: ImportLine[], existingData: ExistingData): Promise { + async detectConflicts(lines: ImportLine[], existingData: ExistingDataWithRelations): Promise { return detectConflicts(lines, existingData, this.userId) } @@ -147,12 +147,19 @@ export class ImportHandler { * * @returns existing data */ - async loadExistingData(): Promise { + async loadExistingData(): Promise { const [personas, videos, allWorldStates, userWorldState, annotations, summaries, claims, claimRelations, ontologies] = await Promise.all([ this.prisma.persona.findMany({ select: { id: true, userId: true } }), this.prisma.video.findMany({ select: { id: true } }), this.prisma.worldState.findMany(), - this.prisma.worldState.findFirst({ where: { userId: this.userId } }), + // Scope the read to the same (userId, projectId) row the importer WRITES + // in importLines, so ownership-based conflict detection reads the row + // that will actually be mutated rather than an unrelated personal one. + // orderBy gives a deterministic pick if duplicates ever exist. + this.prisma.worldState.findFirst({ + where: { userId: this.userId, projectId: this.projectId }, + orderBy: { createdAt: 'asc' } + }), this.prisma.annotation.findMany({ select: { id: true, personaId: true } }), this.prisma.videoSummary.findMany({ select: { id: true, personaId: true } }), this.prisma.claim.findMany({ select: { id: true, summaryId: true } }), @@ -203,12 +210,13 @@ export class ImportHandler { } } - const existingData: ExistingData = { + const existingData: ExistingDataWithRelations = { personaIds: new Set(personas.map(p => p.id)), entityIds: new Set(), eventIds: new Set(), timeIds: new Set(), collectionIds: new Set(), + relationIds: new Set(), annotationIds: new Set(annotations.map(a => a.id)), videoIds: new Set(videos.map(v => v.id)), summaryIds: new Set(summaries.map(s => s.id)), @@ -259,6 +267,13 @@ export class ImportHandler { } } } + if (Array.isArray(worldState.relations)) { + for (const relation of worldState.relations) { + if (relation && typeof relation === 'object' && 'id' in relation) { + existingData.relationIds?.add(relation.id as string) + } + } + } } return existingData diff --git a/server/src/services/import/conflict-resolver.ts b/server/src/services/import/conflict-resolver.ts index 20b0e553..7f552a0e 100644 --- a/server/src/services/import/conflict-resolver.ts +++ b/server/src/services/import/conflict-resolver.ts @@ -66,6 +66,18 @@ function remapInlineIds(text: string, pattern: RegExp, idMap: Map idMap.get(m.toLowerCase()) ?? m) } +/** + * ExistingData augmented with the set of world-state relation ids. Relations + * live in each world state's `relations` array rather than a dedicated table, + * so their ids are collected separately and supplied alongside ExistingData. + * The field is optional so callers that build a bare ExistingData (unit-test + * stubs) remain compatible; the relation case treats a missing set as "no + * existing relations". + */ +export interface ExistingDataWithRelations extends ExistingData { + relationIds?: Set +} + /** * Detect conflicts between import data and existing database data. * @@ -74,7 +86,7 @@ function remapInlineIds(text: string, pattern: RegExp, idMap: Map Prisma.OntologyUpdateInput, + ): Promise { + for (let attempt = 0; attempt < 5; attempt++) { + const current = await tx.ontology.findUnique({ where: { personaId } }) + if (!current) { + throw new NotFoundError('Ontology', personaId) + } + const result = await tx.ontology.updateMany({ + where: { id: current.id, version: current.version }, + data: { ...transform(current), version: { increment: 1 } }, + }) + if (result.count === 1) { + return + } + } + throw new ConflictError('Ontology update conflicted after retries') + } + + /** + * Applies a version-guarded optimistic update to a user's personal world + * state inside an existing transaction. + * + * Mirrors {@link updateOntologyOptimisticTx} for the world-state row the + * type- and persona-deletion cleanup paths mutate, so the world-state write + * commits or rolls back atomically with the ontology cleanup and never + * clobbers a concurrent world edit. + * + * @param tx - the transaction client to run the read/write inside + * @param userId - owning user ID + * @param transform - computes the Prisma update input from the current row + * @throws {NotFoundError} when no personal world state row exists + * @throws {ConflictError} when the write keeps conflicting after retries + */ + private async updatePersonalWorldStateOptimisticTx( + tx: Prisma.TransactionClient, + userId: string, + transform: (current: WorldState) => Prisma.WorldStateUpdateInput, + ): Promise { + for (let attempt = 0; attempt < 5; attempt++) { + const current = await tx.worldState.findFirst({ where: { userId, projectId: null } }) + if (!current) { + throw new NotFoundError('World state', userId) + } + const result = await tx.worldState.updateMany({ + where: { id: current.id, version: current.version }, + data: { ...transform(current), version: { increment: 1 } }, + }) + if (result.count === 1) { + return + } + } + throw new ConflictError('Personal world state update conflicted after retries') + } + /** * Lists the personas visible to the caller. * @@ -185,6 +260,19 @@ export class PersonaService { throw new ForbiddenError('Cannot create Persona in this scope') } + // A project-scoped persona may only be created by a member of that project. + // The generic create ability passes for any self-owned persona regardless + // of projectId, so a non-member could otherwise attach a persona to a + // project they cannot access; verify membership explicitly. + if (projectId) { + const membership = await prisma.projectMembership.findUnique({ + where: { userId_projectId: { userId, projectId } }, + }) + if (!membership) { + throw new ForbiddenError('Cannot create a persona in this project') + } + } + // Only system_admin may flag a persona as system-generated, since system // personas are visible to unauthenticated visitors via the unauthenticated // list branch. A non-admin attempt is silently coerced to false rather @@ -362,61 +450,73 @@ export class PersonaService { throw new ForbiddenError('Cannot delete this Persona') } - const worldState = await this.repository.findPersonalWorldState(existing.userId) - if (worldState) { - interface EntityWithAssignments { - typeAssignments?: Array<{ personaId: string; [key: string]: unknown }> - [key: string]: unknown - } - interface EventWithInterpretations { - personaInterpretations?: Array<{ personaId: string; [key: string]: unknown }> - [key: string]: unknown - } - interface CollectionWithAssignments { - typeAssignments?: Array<{ personaId: string; [key: string]: unknown }> - [key: string]: unknown - } + interface EntityWithAssignments { + typeAssignments?: Array<{ personaId: string; [key: string]: unknown }> + [key: string]: unknown + } + interface EventWithInterpretations { + personaInterpretations?: Array<{ personaId: string; [key: string]: unknown }> + [key: string]: unknown + } + interface CollectionWithAssignments { + typeAssignments?: Array<{ personaId: string; [key: string]: unknown }> + [key: string]: unknown + } - const entities = (worldState.entities as EntityWithAssignments[]) || [] - const cleanedEntities = entities.map(entity => ({ - ...entity, - typeAssignments: (entity.typeAssignments || []).filter(a => a.personaId !== id) - })) - - const events = (worldState.events as EventWithInterpretations[]) || [] - const cleanedEvents = events.map(event => ({ - ...event, - personaInterpretations: (event.personaInterpretations || []).filter(i => i.personaId !== id) - })) - - const entityCollections = (worldState.entityCollections as CollectionWithAssignments[]) || [] - const cleanedEntityCollections = entityCollections.map(collection => ({ - ...collection, - typeAssignments: (collection.typeAssignments || []).filter(a => a.personaId !== id) - })) - - const eventCollections = (worldState.eventCollections as CollectionWithAssignments[]) || [] - const cleanedEventCollections = eventCollections.map(collection => ({ - ...collection, - typeAssignments: (collection.typeAssignments || []).filter(a => a.personaId !== id) - })) - - await this.repository.updateWorldState(worldState.id, { - entities: toJson(cleanedEntities), - events: toJson(cleanedEvents), - entityCollections: toJson(cleanedEntityCollections), - eventCollections: toJson(cleanedEventCollections) + // Clean the owner's personal world state of this persona's type assignments + // and interpretations, then delete the persona (cascading its ontology, + // summaries, and annotations) — both inside one transaction so a partial + // failure cannot leave the world state referencing a deleted persona. The + // world cleanup runs through the version guard so it does not clobber a + // concurrent world edit. + await prisma.$transaction(async (tx) => { + const worldState = await tx.worldState.findFirst({ + where: { userId: existing.userId, projectId: null }, }) - } + if (worldState) { + await this.updatePersonalWorldStateOptimisticTx(tx, existing.userId, (current) => { + const entities = (current.entities as EntityWithAssignments[]) || [] + const cleanedEntities = entities.map(entity => ({ + ...entity, + typeAssignments: (entity.typeAssignments || []).filter(a => a.personaId !== id) + })) + + const events = (current.events as EventWithInterpretations[]) || [] + const cleanedEvents = events.map(event => ({ + ...event, + personaInterpretations: (event.personaInterpretations || []).filter(i => i.personaId !== id) + })) + + const entityCollections = (current.entityCollections as CollectionWithAssignments[]) || [] + const cleanedEntityCollections = entityCollections.map(collection => ({ + ...collection, + typeAssignments: (collection.typeAssignments || []).filter(a => a.personaId !== id) + })) + + const eventCollections = (current.eventCollections as CollectionWithAssignments[]) || [] + const cleanedEventCollections = eventCollections.map(collection => ({ + ...collection, + typeAssignments: (collection.typeAssignments || []).filter(a => a.personaId !== id) + })) + + return { + entities: toJson(cleanedEntities), + events: toJson(cleanedEvents), + entityCollections: toJson(cleanedEntityCollections), + eventCollections: toJson(cleanedEventCollections) + } + }) + } - try { - await this.repository.delete(id) - } catch (error: unknown) { - if (error && typeof error === 'object' && 'code' in error && error.code === 'P2025') { - throw new NotFoundError('Persona', id) + try { + await tx.persona.delete({ where: { id } }) + } catch (error: unknown) { + if (error && typeof error === 'object' && 'code' in error && error.code === 'P2025') { + throw new NotFoundError('Persona', id) + } + throw error } - throw error - } + }) } /** @@ -647,54 +747,62 @@ export class PersonaService { if (!targetType) { throw new NotFoundError('Entity type', typeId) } - const typeName = targetType.name - const updatedEntityTypes = entityTypes.filter(t => t.id !== typeId) - - const roleTypes = asTypesWithGloss(ontology.roleTypes) - const eventTypes = asTypesWithGloss(ontology.eventTypes) - const relationTypes = asTypesWithGloss(ontology.relationTypes) + // Delete the matching annotations, rewrite every ontology gloss that + // referenced the type to plain text, and strip the type's world-state + // assignments — all inside one transaction so a partial failure cannot + // leave annotations, glosses, and world state disagreeing. The ontology and + // world writes recompute from a fresh, version-guarded read so the cleanup + // does not clobber a concurrent edit. Counts are taken from the row each + // guarded write actually lands on. let glossReferences = 0 - glossReferences += countTypeRefsInGlosses(updatedEntityTypes, typeId, personaId, 'entity') - glossReferences += countTypeRefsInGlosses(roleTypes, typeId, personaId, 'entity') - glossReferences += countTypeRefsInGlosses(eventTypes, typeId, personaId, 'entity') - glossReferences += countTypeRefsInGlosses(relationTypes, typeId, personaId, 'entity') - - const cleanedEntityTypes = updateGlossesInTypes(updatedEntityTypes, typeId, personaId, 'entity', typeName) - const cleanedRoleTypes = updateGlossesInTypes(roleTypes, typeId, personaId, 'entity', typeName) - const cleanedEventTypes = updateGlossesInTypes(eventTypes, typeId, personaId, 'entity', typeName) - const cleanedRelationTypes = updateGlossesInTypes(relationTypes, typeId, personaId, 'entity', typeName) - - const deleteResult = await this.repository.deleteAnnotations({ - personaId, - type: 'entity', - label: typeId - }) - let worldAssignments = 0 - const worldState = await this.repository.findPersonalWorldState(persona.userId) - if (worldState) { - const entities = asEntities(worldState.entities) - worldAssignments = countTypeAssignments(entities, typeId, personaId) - const cleanedEntities = removeTypeAssignmentsFromEntities(entities, typeId, personaId) - await this.repository.updateWorldState(worldState.id, { - entities: toJson(cleanedEntities) + let annotations = 0 + await prisma.$transaction(async (tx) => { + const deleteResult = await tx.annotation.deleteMany({ + where: { personaId, type: 'entity', label: typeId } + }) + annotations = deleteResult.count + + await this.updateOntologyOptimisticTx(tx, personaId, (current) => { + const currentEntityTypes = asTypesWithGloss(current.entityTypes) + const currentRoleTypes = asTypesWithGloss(current.roleTypes) + const currentEventTypes = asTypesWithGloss(current.eventTypes) + const currentRelationTypes = asTypesWithGloss(current.relationTypes) + const updatedEntityTypes = currentEntityTypes.filter(t => t.id !== typeId) + + glossReferences = 0 + glossReferences += countTypeRefsInGlosses(updatedEntityTypes, typeId, personaId, 'entity') + glossReferences += countTypeRefsInGlosses(currentRoleTypes, typeId, personaId, 'entity') + glossReferences += countTypeRefsInGlosses(currentEventTypes, typeId, personaId, 'entity') + glossReferences += countTypeRefsInGlosses(currentRelationTypes, typeId, personaId, 'entity') + + return { + entityTypes: toJson(updateGlossesInTypes(updatedEntityTypes, typeId, personaId, 'entity', typeName)), + roleTypes: toJson(updateGlossesInTypes(currentRoleTypes, typeId, personaId, 'entity', typeName)), + eventTypes: toJson(updateGlossesInTypes(currentEventTypes, typeId, personaId, 'entity', typeName)), + relationTypes: toJson(updateGlossesInTypes(currentRelationTypes, typeId, personaId, 'entity', typeName)), + } }) - } - await this.repository.updateOntology(personaId, { - entityTypes: toJson(cleanedEntityTypes), - roleTypes: toJson(cleanedRoleTypes), - eventTypes: toJson(cleanedEventTypes), - relationTypes: toJson(cleanedRelationTypes) + const worldState = await tx.worldState.findFirst({ + where: { userId: persona.userId, projectId: null }, + }) + if (worldState) { + await this.updatePersonalWorldStateOptimisticTx(tx, persona.userId, (current) => { + const entities = asEntities(current.entities) + worldAssignments = countTypeAssignments(entities, typeId, personaId) + return { entities: toJson(removeTypeAssignmentsFromEntities(entities, typeId, personaId)) } + }) + } }) return { message: `Entity type "${typeName}" deleted successfully`, cleanedUp: { glossReferences, - annotations: deleteResult.count, + annotations, worldAssignments } } @@ -752,64 +860,76 @@ export class PersonaService { if (!targetType) { throw new NotFoundError('Role type', typeId) } - const typeName = targetType.name - const updatedRoleTypes = roleTypes.filter(t => t.id !== typeId) - - const entityTypes = asTypesWithGloss(ontology.entityTypes) - const eventTypesForGloss = asTypesWithGloss(ontology.eventTypes) - const relationTypes = asTypesWithGloss(ontology.relationTypes) + // Delete the matching annotations and rewrite the ontology — dropping the + // role, converting its glosses to text, and removing it from every event + // type's role slots — inside one transaction so a partial failure cannot + // leave annotations and ontology disagreeing. The ontology write recomputes + // from a fresh, version-guarded read so the cleanup does not clobber a + // concurrent edit. let glossReferences = 0 - glossReferences += countTypeRefsInGlosses(entityTypes, typeId, personaId, 'role') - glossReferences += countTypeRefsInGlosses(updatedRoleTypes, typeId, personaId, 'role') - glossReferences += countTypeRefsInGlosses(eventTypesForGloss, typeId, personaId, 'role') - glossReferences += countTypeRefsInGlosses(relationTypes, typeId, personaId, 'role') - - const cleanedEntityTypes = updateGlossesInTypes(entityTypes, typeId, personaId, 'role', typeName) - const cleanedRoleTypes = updateGlossesInTypes(updatedRoleTypes, typeId, personaId, 'role', typeName) - const cleanedEventTypesGloss = updateGlossesInTypes(eventTypesForGloss, typeId, personaId, 'role', typeName) - const cleanedRelationTypes = updateGlossesInTypes(relationTypes, typeId, personaId, 'role', typeName) - - // Remove the role from event-type role slots, preserving every other field - // on each event type. The raw event types carry the `roles` array, which - // the gloss-only mapping above does not. - const eventTypesRaw = ontology.eventTypes - const eventRoleReferences = this.countEventRoleReferences(eventTypesRaw, typeId) - let cleanedEventTypes = cleanedEventTypesGloss - if (Array.isArray(eventTypesRaw)) { - cleanedEventTypes = cleanedEventTypesGloss.map(et => { - const rawEvent = eventTypesRaw.find(raw => - raw && typeof raw === 'object' && 'id' in raw && (raw as { id: string }).id === et.id - ) - if (rawEvent && typeof rawEvent === 'object' && 'roles' in rawEvent) { - const roles = (rawEvent as { roles?: Array<{ roleTypeId: string }> }).roles - if (roles) { - return { ...et, roles: roles.filter(r => r.roleTypeId !== typeId) } - } - } - return et + let eventRoleReferences = 0 + let annotations = 0 + await prisma.$transaction(async (tx) => { + const deleteResult = await tx.annotation.deleteMany({ + where: { personaId, type: 'role', label: typeId } }) - } - - const deleteResult = await this.repository.deleteAnnotations({ - personaId, - type: 'role', - label: typeId - }) + annotations = deleteResult.count + + await this.updateOntologyOptimisticTx(tx, personaId, (current) => { + const currentEntityTypes = asTypesWithGloss(current.entityTypes) + const currentRoleTypes = asTypesWithGloss(current.roleTypes) + const currentEventTypesForGloss = asTypesWithGloss(current.eventTypes) + const currentRelationTypes = asTypesWithGloss(current.relationTypes) + const updatedRoleTypes = currentRoleTypes.filter(t => t.id !== typeId) + + glossReferences = 0 + glossReferences += countTypeRefsInGlosses(currentEntityTypes, typeId, personaId, 'role') + glossReferences += countTypeRefsInGlosses(updatedRoleTypes, typeId, personaId, 'role') + glossReferences += countTypeRefsInGlosses(currentEventTypesForGloss, typeId, personaId, 'role') + glossReferences += countTypeRefsInGlosses(currentRelationTypes, typeId, personaId, 'role') + + const cleanedEntityTypes = updateGlossesInTypes(currentEntityTypes, typeId, personaId, 'role', typeName) + const cleanedRoleTypes = updateGlossesInTypes(updatedRoleTypes, typeId, personaId, 'role', typeName) + const cleanedEventTypesGloss = updateGlossesInTypes(currentEventTypesForGloss, typeId, personaId, 'role', typeName) + const cleanedRelationTypes = updateGlossesInTypes(currentRelationTypes, typeId, personaId, 'role', typeName) + + // Remove the role from event-type role slots, preserving every other + // field on each event type. The raw event types carry the `roles` + // array, which the gloss-only mapping above does not. + const eventTypesRaw = current.eventTypes + eventRoleReferences = this.countEventRoleReferences(eventTypesRaw, typeId) + let cleanedEventTypes = cleanedEventTypesGloss + if (Array.isArray(eventTypesRaw)) { + cleanedEventTypes = cleanedEventTypesGloss.map(et => { + const rawEvent = eventTypesRaw.find(raw => + raw && typeof raw === 'object' && 'id' in raw && (raw as { id: string }).id === et.id + ) + if (rawEvent && typeof rawEvent === 'object' && 'roles' in rawEvent) { + const roles = (rawEvent as { roles?: Array<{ roleTypeId: string }> }).roles + if (roles) { + return { ...et, roles: roles.filter(r => r.roleTypeId !== typeId) } + } + } + return et + }) + } - await this.repository.updateOntology(personaId, { - entityTypes: toJson(cleanedEntityTypes), - roleTypes: toJson(cleanedRoleTypes), - eventTypes: toJson(cleanedEventTypes), - relationTypes: toJson(cleanedRelationTypes) + return { + entityTypes: toJson(cleanedEntityTypes), + roleTypes: toJson(cleanedRoleTypes), + eventTypes: toJson(cleanedEventTypes), + relationTypes: toJson(cleanedRelationTypes), + } + }) }) return { message: `Role type "${typeName}" deleted successfully`, cleanedUp: { glossReferences, - annotations: deleteResult.count, + annotations, eventRoleReferences } } @@ -892,54 +1012,61 @@ export class PersonaService { if (!targetType) { throw new NotFoundError('Event type', typeId) } - const typeName = targetType.name - const updatedEventTypes = eventTypes.filter(t => t.id !== typeId) - - const entityTypes = asTypesWithGloss(ontology.entityTypes) - const roleTypes = asTypesWithGloss(ontology.roleTypes) - const relationTypes = asTypesWithGloss(ontology.relationTypes) + // Delete the matching annotations, rewrite every ontology gloss that + // referenced the type to plain text, and strip the type's world-state + // interpretations — all inside one transaction so a partial failure cannot + // leave annotations, glosses, and world state disagreeing. The ontology and + // world writes recompute from a fresh, version-guarded read so the cleanup + // does not clobber a concurrent edit. let glossReferences = 0 - glossReferences += countTypeRefsInGlosses(entityTypes, typeId, personaId, 'event') - glossReferences += countTypeRefsInGlosses(roleTypes, typeId, personaId, 'event') - glossReferences += countTypeRefsInGlosses(updatedEventTypes, typeId, personaId, 'event') - glossReferences += countTypeRefsInGlosses(relationTypes, typeId, personaId, 'event') - - const cleanedEntityTypes = updateGlossesInTypes(entityTypes, typeId, personaId, 'event', typeName) - const cleanedRoleTypes = updateGlossesInTypes(roleTypes, typeId, personaId, 'event', typeName) - const cleanedEventTypes = updateGlossesInTypes(updatedEventTypes, typeId, personaId, 'event', typeName) - const cleanedRelationTypes = updateGlossesInTypes(relationTypes, typeId, personaId, 'event', typeName) - - const deleteResult = await this.repository.deleteAnnotations({ - personaId, - type: 'event', - label: typeId - }) - let worldInterpretations = 0 - const worldState = await this.repository.findPersonalWorldState(persona.userId) - if (worldState) { - const events = asEvents(worldState.events) - worldInterpretations = countEventInterpretations(events, typeId, personaId) - const cleanedEvents = removeEventInterpretationsFromEvents(events, typeId, personaId) - await this.repository.updateWorldState(worldState.id, { - events: toJson(cleanedEvents) + let annotations = 0 + await prisma.$transaction(async (tx) => { + const deleteResult = await tx.annotation.deleteMany({ + where: { personaId, type: 'event', label: typeId } + }) + annotations = deleteResult.count + + await this.updateOntologyOptimisticTx(tx, personaId, (current) => { + const currentEntityTypes = asTypesWithGloss(current.entityTypes) + const currentRoleTypes = asTypesWithGloss(current.roleTypes) + const currentEventTypes = asTypesWithGloss(current.eventTypes) + const currentRelationTypes = asTypesWithGloss(current.relationTypes) + const updatedEventTypes = currentEventTypes.filter(t => t.id !== typeId) + + glossReferences = 0 + glossReferences += countTypeRefsInGlosses(currentEntityTypes, typeId, personaId, 'event') + glossReferences += countTypeRefsInGlosses(currentRoleTypes, typeId, personaId, 'event') + glossReferences += countTypeRefsInGlosses(updatedEventTypes, typeId, personaId, 'event') + glossReferences += countTypeRefsInGlosses(currentRelationTypes, typeId, personaId, 'event') + + return { + entityTypes: toJson(updateGlossesInTypes(currentEntityTypes, typeId, personaId, 'event', typeName)), + roleTypes: toJson(updateGlossesInTypes(currentRoleTypes, typeId, personaId, 'event', typeName)), + eventTypes: toJson(updateGlossesInTypes(updatedEventTypes, typeId, personaId, 'event', typeName)), + relationTypes: toJson(updateGlossesInTypes(currentRelationTypes, typeId, personaId, 'event', typeName)), + } }) - } - await this.repository.updateOntology(personaId, { - entityTypes: toJson(cleanedEntityTypes), - roleTypes: toJson(cleanedRoleTypes), - eventTypes: toJson(cleanedEventTypes), - relationTypes: toJson(cleanedRelationTypes) + const worldState = await tx.worldState.findFirst({ + where: { userId: persona.userId, projectId: null }, + }) + if (worldState) { + await this.updatePersonalWorldStateOptimisticTx(tx, persona.userId, (current) => { + const events = asEvents(current.events) + worldInterpretations = countEventInterpretations(events, typeId, personaId) + return { events: toJson(removeEventInterpretationsFromEvents(events, typeId, personaId)) } + }) + } }) return { message: `Event type "${typeName}" deleted successfully`, cleanedUp: { glossReferences, - annotations: deleteResult.count, + annotations, worldInterpretations } } @@ -991,30 +1118,31 @@ export class PersonaService { if (!targetType) { throw new NotFoundError('Relation type', typeId) } - const typeName = targetType.name - const updatedRelationTypes = relationTypes.filter(t => t.id !== typeId) - - const entityTypes = asTypesWithGloss(ontology.entityTypes) - const roleTypes = asTypesWithGloss(ontology.roleTypes) - const eventTypes = asTypesWithGloss(ontology.eventTypes) + // Route the gloss cleanup through the version-guarded optimistic update so a + // concurrent ontology edit is not clobbered; recompute from the fresh row. + // A single write needs no enclosing transaction. let glossReferences = 0 - glossReferences += countTypeRefsInGlosses(entityTypes, typeId, personaId, 'relation') - glossReferences += countTypeRefsInGlosses(roleTypes, typeId, personaId, 'relation') - glossReferences += countTypeRefsInGlosses(eventTypes, typeId, personaId, 'relation') - glossReferences += countTypeRefsInGlosses(updatedRelationTypes, typeId, personaId, 'relation') - - const cleanedEntityTypes = updateGlossesInTypes(entityTypes, typeId, personaId, 'relation', typeName) - const cleanedRoleTypes = updateGlossesInTypes(roleTypes, typeId, personaId, 'relation', typeName) - const cleanedEventTypes = updateGlossesInTypes(eventTypes, typeId, personaId, 'relation', typeName) - const cleanedRelationTypes = updateGlossesInTypes(updatedRelationTypes, typeId, personaId, 'relation', typeName) - - await this.repository.updateOntology(personaId, { - entityTypes: toJson(cleanedEntityTypes), - roleTypes: toJson(cleanedRoleTypes), - eventTypes: toJson(cleanedEventTypes), - relationTypes: toJson(cleanedRelationTypes) + await this.repository.updateOntologyOptimistic(personaId, (current) => { + const currentEntityTypes = asTypesWithGloss(current.entityTypes) + const currentRoleTypes = asTypesWithGloss(current.roleTypes) + const currentEventTypes = asTypesWithGloss(current.eventTypes) + const currentRelationTypes = asTypesWithGloss(current.relationTypes) + const updatedRelationTypes = currentRelationTypes.filter(t => t.id !== typeId) + + glossReferences = 0 + glossReferences += countTypeRefsInGlosses(currentEntityTypes, typeId, personaId, 'relation') + glossReferences += countTypeRefsInGlosses(currentRoleTypes, typeId, personaId, 'relation') + glossReferences += countTypeRefsInGlosses(currentEventTypes, typeId, personaId, 'relation') + glossReferences += countTypeRefsInGlosses(updatedRelationTypes, typeId, personaId, 'relation') + + return { + entityTypes: toJson(updateGlossesInTypes(currentEntityTypes, typeId, personaId, 'relation', typeName)), + roleTypes: toJson(updateGlossesInTypes(currentRoleTypes, typeId, personaId, 'relation', typeName)), + eventTypes: toJson(updateGlossesInTypes(currentEventTypes, typeId, personaId, 'relation', typeName)), + relationTypes: toJson(updateGlossesInTypes(updatedRelationTypes, typeId, personaId, 'relation', typeName)), + } }) return { diff --git a/server/src/services/project-service.ts b/server/src/services/project-service.ts index d13b9311..6a9fb2fa 100644 --- a/server/src/services/project-service.ts +++ b/server/src/services/project-service.ts @@ -423,7 +423,18 @@ export class ProjectService { throw new ConflictError('User is already a member of this project') } - const membership = await this.repository.createMembership(targetUserId, projectId, role) + let membership + try { + membership = await this.repository.createMembership(targetUserId, projectId, role) + } catch (error) { + // The pre-check narrows the common case, but a concurrent add of the same + // member can still race past it into the @@unique([userId, projectId]) + // constraint — surface 409, not 500. + if (error instanceof Prisma.PrismaClientKnownRequestError && error.code === 'P2002') { + throw new ConflictError('User is already a member of this project') + } + throw error + } // Newly added member picks up project-scope role permissions. invalidateUserAbilities(targetUserId) @@ -672,13 +683,20 @@ export class ProjectService { throw new ForbiddenError('You must be a project member to update world state') } - const existing = await this.repository.findWorldState(userId, projectId) + // Guarantee the (userId, projectId) row exists before merging. Upserting the + // empty row collapses the find-then-create race on concurrent first access: + // a plain create would have both callers miss the find, both create, and the + // second hit the @@unique([userId, projectId]) constraint (P2002 -> 500). + // With the row guaranteed, both the existing-row and first-access paths run + // through the same guarded optimistic merge below. + await this.repository.upsertEmptyWorldState(userId, projectId) // Merge each provided array by id (upsert) instead of replacing it, under // optimistic concurrency. Project world state is multi-user (every project // member shares the (userId, projectId) row), so whole-blob replacement // would let concurrent writers clobber each other's additions; the merge - // re-runs against the freshly-read row on each retry. + // re-runs against the freshly-read row on each retry. On a first-access row + // each `current` array is empty, so the merge yields exactly the input. const mergeTransform = (current: PrismaWorldState): Prisma.WorldStateUpdateInput => ({ entities: input.entities !== undefined ? mergeById(current.entities, input.entities) : undefined, events: input.events !== undefined ? mergeById(current.events, input.events) : undefined, @@ -689,22 +707,11 @@ export class ProjectService { relations: input.relations !== undefined ? mergeById(current.relations, input.relations) : undefined, }) - let worldState - if (existing) { - worldState = await this.repository.updateProjectWorldStateOptimistic(userId, projectId, mergeTransform) - } else { - worldState = await this.repository.createWorldState({ - userId, - projectId, - entities: toJson(input.entities || []), - events: toJson(input.events || []), - times: toJson(input.times || []), - entityCollections: toJson(input.entityCollections || []), - eventCollections: toJson(input.eventCollections || []), - timeCollections: toJson(input.timeCollections || []), - relations: toJson(input.relations || []), - }) - } + const worldState = await this.repository.updateProjectWorldStateOptimistic( + userId, + projectId, + mergeTransform + ) return this.mapWorldState(worldState) } diff --git a/server/src/services/world-state-service.ts b/server/src/services/world-state-service.ts index 14910c67..aa18a2c0 100644 --- a/server/src/services/world-state-service.ts +++ b/server/src/services/world-state-service.ts @@ -2,7 +2,7 @@ import { Prisma, type WorldState as PrismaWorldState } from '@prisma/client' import { subject } from '@casl/ability' import { accessibleBy } from '@casl/prisma' import type { AppAbility } from '../lib/abilities.js' -import { NotFoundError, UnauthorizedError, InternalError, ForbiddenError } from '../lib/errors.js' +import { NotFoundError, UnauthorizedError, InternalError, ForbiddenError, ConflictError } from '../lib/errors.js' import { demoWidensWorldState } from '../lib/demo-rbac.js' import { isSingleUserMode } from './user-service.js' import { config } from '../config.js' @@ -385,7 +385,10 @@ export class WorldStateService { if (this.ability && !this.ability.can('delete', subject('WorldState', existingWorldState))) { throw new ForbiddenError('Cannot delete this WorldState') } - await this.repository.updateWorldState(existingWorldState.id, emptyData) + // Clear through the optimistic guard rather than a bare id update so the + // version advances and a concurrent writer cannot have half its blob + // survive the clear; the empty-arrays transform ignores the current row. + await this.repository.updatePersonalWorldStateOptimistic(userId, () => ({ ...emptyData })) } else { if (this.ability) { const candidate = subject('WorldState', { userId, projectId: null }) @@ -560,13 +563,17 @@ export class WorldStateService { * and writes through the supplied transaction client so the guarded delete * write and the gloss cleanup commit atomically. Reads the current row, * lets `transform` compute the new column values from it, then writes them - * guarded by the row's `updatedAt`; a missed guard (count 0) retries against - * the freshly read row so a concurrent addition is not clobbered. + * guarded by the row's `version`; a missed guard (count 0) retries against + * the freshly read row so a concurrent addition is not clobbered. The guard + * keys on the monotonic `version` rather than `updatedAt` because two writes + * landing in the same millisecond can share an `updatedAt`, which would let + * the second silently overwrite the first. * * @param tx - the transaction client to run the read/write inside * @param userId - owning user ID * @param transform - computes the Prisma update input from the current row - * @throws when no personal row exists or the write keeps conflicting + * @throws {NotFoundError} when no personal row exists + * @throws {ConflictError} when the write keeps conflicting after retries */ private async updatePersonalWorldStateOptimisticTx( tx: Prisma.TransactionClient, @@ -578,17 +585,17 @@ export class WorldStateService { where: { userId, projectId: null }, }) if (!current) { - throw new Error('No personal world state to update') + throw new NotFoundError('World state', userId) } const result = await tx.worldState.updateMany({ - where: { id: current.id, updatedAt: current.updatedAt }, - data: { ...transform(current), updatedAt: new Date() }, + where: { id: current.id, version: current.version }, + data: { ...transform(current), version: { increment: 1 } }, }) if (result.count === 1) { return } } - throw new Error('Personal world state update conflicted after retries') + throw new ConflictError('Personal world state update conflicted after retries') } /** diff --git a/server/test/integration/persona-type-deletion.test.ts b/server/test/integration/persona-type-deletion.test.ts new file mode 100644 index 00000000..8b564819 --- /dev/null +++ b/server/test/integration/persona-type-deletion.test.ts @@ -0,0 +1,121 @@ +import { describe, it, expect, beforeAll, afterAll, beforeEach } from 'vitest' +import { buildApp } from '../../src/app.js' +import { FastifyInstance } from 'fastify' +import { PrismaClient } from '@prisma/client' +import { seedBaselinePermissions, createRegularTestUser } from '../helpers/rbac-test-setup.js' + +/** + * Ontology type deletion runs the annotation delete, the ontology gloss + * cleanup, and the personal world-state cleanup in one transaction, each write + * routed through the monotonic version guard. This asserts the whole cleanup + * lands (type gone, annotations deleted, world assignments stripped) and that + * the guard advanced, on the DELETE endpoint the frontend uses for graceful + * type removal. + */ +describe('Persona ontology type deletion', () => { + let app: FastifyInstance + let prisma: PrismaClient + + beforeAll(async () => { + app = await buildApp() + prisma = app.prisma + }) + afterAll(async () => { + await app.close() + }) + + beforeEach(async () => { + await prisma.claim.deleteMany() + await prisma.videoSummary.deleteMany() + await prisma.annotation.deleteMany() + await prisma.worldState.deleteMany() + await prisma.ontology.deleteMany() + await prisma.persona.deleteMany() + await prisma.video.deleteMany() + await prisma.session.deleteMany() + await prisma.rolePermission.deleteMany() + await prisma.user.deleteMany() + await seedBaselinePermissions(prisma) + }) + + it('deletes an entity type, its annotations, and its world assignments atomically', async () => { + const user = await createRegularTestUser(prisma, { username: 'td', email: 'td@example.com' }) + + const video = await prisma.video.create({ data: { filename: 'td.mp4', path: '/td.mp4' } }) + const persona = await prisma.persona.create({ + data: { + userId: user.id, + name: 'P', + role: 'r', + informationNeed: 'n', + ontology: { + create: { + entityTypes: [ + { id: 'et1', name: 'Person', gloss: [] }, + { id: 'et2', name: 'Place', gloss: [] }, + ], + eventTypes: [], + roleTypes: [], + relationTypes: [], + }, + }, + }, + }) + + // An annotation labelled with the doomed type. + await prisma.annotation.create({ + data: { + videoId: video.id, + personaId: persona.id, + type: 'entity', + label: 'et1', + frames: {}, + createdByUserId: user.id, + }, + }) + + // A personal world entity assigned the doomed type for this persona. + await prisma.worldState.create({ + data: { + userId: user.id, + entities: [ + { id: 'e1', name: 'Alice', typeAssignments: [{ personaId: persona.id, entityTypeId: 'et1' }] }, + ], + events: [], + times: [], + entityCollections: [], + eventCollections: [], + timeCollections: [], + relations: [], + }, + }) + + const res = await app.inject({ + method: 'DELETE', + url: `/api/personas/${persona.id}/ontology/entities/et1`, + cookies: { session_token: user.sessionToken }, + }) + expect(res.statusCode).toBe(200) + + // The type is gone and stays gone; the sibling type is untouched. + const ontology = await prisma.ontology.findUnique({ where: { personaId: persona.id } }) + const entityTypeIds = (ontology!.entityTypes as Array<{ id: string }>).map((t) => t.id) + expect(entityTypeIds).toContain('et2') + expect(entityTypeIds).not.toContain('et1') + + // The matching annotation was deleted in the same transaction. + const remaining = await prisma.annotation.count({ + where: { personaId: persona.id, type: 'entity', label: 'et1' }, + }) + expect(remaining).toBe(0) + + // The world assignment for the type was stripped. + const worldState = await prisma.worldState.findFirst({ where: { userId: user.id, projectId: null } }) + const entities = worldState!.entities as Array<{ typeAssignments: Array<{ entityTypeId: string }> }> + expect(entities[0].typeAssignments.some((a) => a.entityTypeId === 'et1')).toBe(false) + + // Each guarded write advanced its row's version. + expect(ontology!.version).toBeGreaterThanOrEqual(1) + expect(worldState!.version).toBeGreaterThanOrEqual(1) + }) +}) diff --git a/server/test/integration/v059-hardening.test.ts b/server/test/integration/v059-hardening.test.ts new file mode 100644 index 00000000..05e700a9 --- /dev/null +++ b/server/test/integration/v059-hardening.test.ts @@ -0,0 +1,230 @@ +import { describe, it, expect, beforeAll, afterAll, beforeEach } from 'vitest' +import { buildApp } from '../../src/app.js' +import { FastifyInstance } from 'fastify' +import { PrismaClient } from '@prisma/client' +import { seedBaselinePermissions, createAdminTestUser, createRegularTestUser } from '../helpers/rbac-test-setup.js' + +/** + * 0.5.9 backend correctness/security hardening: model-route authentication, + * project-membership enforcement on persona create, the monotonic optimistic + * version guard, admin API-key per-provider uniqueness, profile duplicate-email + * conflict handling, claim-id authorization, and deep-fork of a summary's claims. + */ +describe('0.5.9 backend hardening', () => { + let app: FastifyInstance + let prisma: PrismaClient + + beforeAll(async () => { + app = await buildApp() + prisma = app.prisma + }) + afterAll(async () => { + await app.close() + }) + + beforeEach(async () => { + await prisma.loginAttempt.deleteMany() + await prisma.claimRelation.deleteMany() + await prisma.claim.deleteMany() + await prisma.resourceShare.deleteMany() + await prisma.videoSummary.deleteMany() + await prisma.apiKey.deleteMany() + await prisma.worldState.deleteMany() + await prisma.projectMembership.deleteMany() + await prisma.project.deleteMany() + await prisma.ontology.deleteMany() + await prisma.persona.deleteMany() + await prisma.annotation.deleteMany() + await prisma.video.deleteMany() + await prisma.session.deleteMany() + await prisma.rolePermission.deleteMany() + await prisma.user.deleteMany() + await seedBaselinePermissions(prisma) + }) + + describe('model routes require authentication', () => { + it('rejects an unauthenticated read of the model config with 401', async () => { + const res = await app.inject({ method: 'GET', url: '/api/models/config' }) + expect(res.statusCode).toBe(401) + }) + + it('rejects a non-admin model selection with 403', async () => { + const user = await createRegularTestUser(prisma, { username: 'mdl', email: 'mdl@example.com' }) + const res = await app.inject({ + method: 'POST', + url: '/api/models/select', + cookies: { session_token: user.sessionToken }, + payload: { task_type: 'detection', model_name: 'yolov8n' }, + }) + expect(res.statusCode).toBe(403) + }) + }) + + describe('persona create is scoped to project membership', () => { + it('forbids a non-member from creating a persona in a project', async () => { + const owner = await createRegularTestUser(prisma, { username: 'owner', email: 'owner@example.com' }) + const outsider = await createRegularTestUser(prisma, { username: 'outsider', email: 'outsider@example.com' }) + + const project = await prisma.project.create({ + data: { name: 'Closed', slug: 'closed-project', createdBy: owner.id }, + }) + await prisma.projectMembership.create({ + data: { userId: owner.id, projectId: project.id, role: 'project_owner' }, + }) + + const res = await app.inject({ + method: 'POST', + url: '/api/personas', + cookies: { session_token: outsider.sessionToken }, + payload: { name: 'Sneaky', role: 'analyst', informationNeed: 'x', projectId: project.id }, + }) + expect(res.statusCode).toBe(403) + }) + + it('allows a member to create a persona in their project', async () => { + const owner = await createRegularTestUser(prisma, { username: 'owner2', email: 'owner2@example.com' }) + const project = await prisma.project.create({ + data: { name: 'Open', slug: 'open-project', createdBy: owner.id }, + }) + await prisma.projectMembership.create({ + data: { userId: owner.id, projectId: project.id, role: 'project_owner' }, + }) + + const res = await app.inject({ + method: 'POST', + url: '/api/personas', + cookies: { session_token: owner.sessionToken }, + payload: { name: 'Mine', role: 'analyst', informationNeed: 'x', projectId: project.id }, + }) + expect(res.statusCode).toBe(201) + }) + }) + + describe('personal world state carries a monotonic version guard', () => { + it('increments the version on each guarded write and merges concurrent additions', async () => { + const user = await createRegularTestUser(prisma, { username: 'wsv', email: 'wsv@example.com' }) + const headers = { cookies: { session_token: user.sessionToken } } + + // Materialize the personal row (version 0). + await app.inject({ method: 'GET', url: '/api/world', ...headers }) + + const put = (entities: Array<{ id: string; name: string }>) => + app.inject({ method: 'PUT', url: '/api/world', ...headers, payload: { entities } }) + + expect((await put([{ id: 'e1', name: 'E1' }])).statusCode).toBe(200) + expect((await put([{ id: 'e2', name: 'E2' }])).statusCode).toBe(200) + + const row = await prisma.worldState.findFirst({ where: { userId: user.id, projectId: null } }) + expect(row?.version).toBe(2) + // Merge-by-id keeps both additions; neither write clobbered the other. + const ids = (row?.entities as Array<{ id: string }>).map((e) => e.id).sort() + expect(ids).toEqual(['e1', 'e2']) + }) + }) + + describe('admin API keys are unique per provider', () => { + it('returns 409 on a second admin key for the same provider', async () => { + const admin = await createAdminTestUser(prisma, { username: 'akadmin', email: 'akadmin@example.com' }) + const create = () => + app.inject({ + method: 'POST', + url: '/api/admin/api-keys', + cookies: { session_token: admin.sessionToken }, + payload: { provider: 'ANTHROPIC', keyName: 'primary', apiKey: 'sk-test-abcdefghijklmnop' }, + }) + expect((await create()).statusCode).toBe(201) + expect((await create()).statusCode).toBe(409) + }) + }) + + describe('self-service profile update', () => { + it('returns 409 (not 500) when changing to an already-used email', async () => { + await createRegularTestUser(prisma, { username: 'taken', email: 'taken@example.com' }) + const mover = await createRegularTestUser(prisma, { username: 'mover', email: 'mover@example.com' }) + + const res = await app.inject({ + method: 'PUT', + url: '/api/user/profile', + cookies: { session_token: mover.sessionToken }, + payload: { email: 'taken@example.com' }, + }) + expect(res.statusCode).toBe(409) + }) + }) + + describe('forking a shared summary deep-copies its claims', () => { + it('copies the claim tree and denormalized claimsJson under fresh ids', async () => { + const sharer = await createRegularTestUser(prisma, { username: 'sharer', email: 'sharer@example.com' }) + const forker = await createRegularTestUser(prisma, { username: 'forker', email: 'forker@example.com' }) + + const video = await prisma.video.create({ data: { filename: 'fork.mp4', path: '/fork.mp4' } }) + const persona = await prisma.persona.create({ + data: { userId: sharer.id, name: 'P', role: 'r', informationNeed: 'n' }, + }) + const summary = await prisma.videoSummary.create({ + data: { videoId: video.id, personaId: persona.id, createdBy: sharer.id }, + }) + const parent = await prisma.claim.create({ + data: { summaryId: summary.id, summaryType: 'video', text: 'Parent claim', createdBy: sharer.id }, + }) + const child = await prisma.claim.create({ + data: { + summaryId: summary.id, + summaryType: 'video', + text: 'Child claim', + parentClaimId: parent.id, + createdBy: sharer.id, + }, + }) + await prisma.videoSummary.update({ + where: { id: summary.id }, + data: { + claimsVersion: '1.0', + claimsExtractedAt: new Date(), + claimsJson: { + version: '1.0', + claims: [{ id: parent.id, text: 'Parent claim', subclaims: [{ id: child.id, text: 'Child claim' }] }], + metadata: { totalClaims: 2 }, + }, + }, + }) + + const share = await prisma.resourceShare.create({ + data: { + resourceType: 'summary', + resourceId: summary.id, + sharedByUserId: sharer.id, + sharedWithUserId: forker.id, + permissionLevel: 'forkable', + }, + }) + + const res = await app.inject({ + method: 'POST', + url: `/api/sharing/${share.id}/fork`, + cookies: { session_token: forker.sessionToken }, + }) + expect(res.statusCode).toBe(201) + const forkedSummaryId = res.json().resourceId as string + + const forkedClaims = await prisma.claim.findMany({ where: { summaryId: forkedSummaryId } }) + expect(forkedClaims).toHaveLength(2) + // Fresh ids, not the source ids. + const forkedIds = forkedClaims.map((c) => c.id) + expect(forkedIds).not.toContain(parent.id) + expect(forkedIds).not.toContain(child.id) + // Parent/child hierarchy preserved, re-pointed at the new parent id. + const forkedParent = forkedClaims.find((c) => c.parentClaimId === null) + const forkedChild = forkedClaims.find((c) => c.parentClaimId !== null) + expect(forkedParent?.text).toBe('Parent claim') + expect(forkedChild?.text).toBe('Child claim') + expect(forkedChild?.parentClaimId).toBe(forkedParent?.id) + + // Denormalized claimsJson carried over and re-pointed at the new ids. + const forkedSummary = await prisma.videoSummary.findUnique({ where: { id: forkedSummaryId } }) + const json = forkedSummary?.claimsJson as { claims: Array<{ id: string; subclaims: Array<{ id: string }> }> } + expect(json.claims[0].id).toBe(forkedParent?.id) + expect(json.claims[0].subclaims[0].id).toBe(forkedChild?.id) + }) + }) +}) diff --git a/server/test/models/models.test.ts b/server/test/models/models.test.ts index f138076c..032e846b 100644 --- a/server/test/models/models.test.ts +++ b/server/test/models/models.test.ts @@ -17,6 +17,11 @@ describe('Model Routes', () => { let app: FastifyInstance beforeEach(async () => { + // The state-changing select/load/unload routes require admin. This suite + // exercises proxy behavior and validation, not auth, so enable the + // isolated-test admin bypass to reach the handlers under test. + vi.stubEnv('ALLOW_TEST_ADMIN_BYPASS', 'true') + // Create fresh Fastify instance for each test app = Fastify({ logger: false }) @@ -65,6 +70,7 @@ describe('Model Routes', () => { afterEach(async () => { await app.close() + vi.unstubAllEnvs() }) describe('GET /api/models/config', () => { diff --git a/server/test/routes/api-keys.test.ts b/server/test/routes/api-keys.test.ts index 67ecd44c..d276a1a1 100644 --- a/server/test/routes/api-keys.test.ts +++ b/server/test/routes/api-keys.test.ts @@ -577,10 +577,10 @@ describe('API Key Routes', () => { expect(dbKey!.userId).toBeNull() }) - it('allows multiple admin keys for same provider', async () => { - // Note: PostgreSQL treats NULL as not equal to NULL in unique constraints, - // so multiple admin keys (userId: null) can exist for the same provider. - // This is a known limitation of the current schema design. + it('rejects a second admin key for the same provider with 409', async () => { + // A partial unique index (provider WHERE userId IS NULL) enforces one + // admin key per provider; the compound @@unique([userId, provider]) does + // not, because PostgreSQL treats NULL userIds as distinct. // Create first admin key const firstResponse = await app.inject({ @@ -595,7 +595,7 @@ describe('API Key Routes', () => { }) expect(firstResponse.statusCode).toBe(201) - // Create second admin key for same provider (currently allowed) + // A second admin key for the same provider conflicts. const secondResponse = await app.inject({ method: 'POST', url: '/api/admin/api-keys', @@ -606,15 +606,13 @@ describe('API Key Routes', () => { apiKey: 'sk-openai-key-2' } }) + expect(secondResponse.statusCode).toBe(409) - // Currently succeeds due to NULL != NULL in PostgreSQL - expect(secondResponse.statusCode).toBe(201) - - // Verify both keys exist + // Only the first key persists. const keys = await prisma.apiKey.findMany({ where: { userId: null, provider: 'OPENAI' } }) - expect(keys).toHaveLength(2) + expect(keys).toHaveLength(1) }) it('returns 403 for non-admin', async () => { diff --git a/server/test/routes/models-503.test.ts b/server/test/routes/models-503.test.ts index 3c22e1cd..310b7cbd 100644 --- a/server/test/routes/models-503.test.ts +++ b/server/test/routes/models-503.test.ts @@ -18,6 +18,11 @@ describe('Model Routes - 503 Service Unavailable', () => { let app: FastifyInstance beforeEach(async () => { + // The state-changing select/load/unload routes require admin. This suite + // exercises the proxy's unavailable-service handling, not auth, so enable + // the isolated-test admin bypass to reach the handler under test. + vi.stubEnv('ALLOW_TEST_ADMIN_BYPASS', 'true') + app = Fastify({ logger: false }) app.setErrorHandler((error, request, reply) => { @@ -42,6 +47,7 @@ describe('Model Routes - 503 Service Unavailable', () => { afterEach(async () => { await app.close() + vi.unstubAllEnvs() }) describe('connection refused (no response)', () => {