diff --git a/CHANGELOG.md b/CHANGELOG.md index b80505d3..d384fe2a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,26 @@ 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.4] - 2026-06-25 + +The 0.5.4 patch fixes project-scope and ownership stamping on video summaries and claims ([#181](https://github.com/parafovea/fovea/pull/181)). Project collaborators could not see a teammate's summary or add claims under it because summaries were persisted without their persona's project, and model-generated summaries and extracted claims were left unowned. Nothing is breaking; the API additively gains a `projectId` field on summary and claim responses. + +### Fixed + +#### Summaries and Claims Are Stamped With Their Persona's Project + +- Every video-summary and claim write now stamps `projectId` from the persona (or the parent summary). Previously the interactive summary route (`server/src/routes/summaries.ts`), the summarization and claim-extraction workers (`server/src/queues/setup.ts`), and the auto-created empty summary (`server/src/repositories/ClaimRepository.ts`) all omitted it, so rows were born `projectId = NULL` and were invisible to every project collaborator except the creator — which `403`'d them at the parent-summary read gate when they tried to add claims, and hid the content from project-scoped queries. The summary update path also re-stamps the scope so a previously NULL-scoped row heals on its next save, and a migration backfills existing summaries and claims. + +#### Model-Generated Summaries and Extracted Claims Are Owned + +- The summarization and claim-extraction queue workers created rows without `createdBy`, leaving model-generated summaries and extracted claims owned by no one (readable only by an admin). The requesting user is now threaded through the queue payload and stamped as the owner on create. + +### Added + +#### projectId on Summary and Claim API Responses + +- The `VideoSummary` and `Claim` API responses now include `projectId`, so clients can reflect a resource's project scope; its prior absence had helped the stamping defect go unnoticed. + ## [0.5.3] - 2026-06-24 The 0.5.3 patch fixes a claims-workspace interaction bug ([#177](https://github.com/parafovea/fovea/pull/177)). No API shapes change and nothing is breaking. diff --git a/annotation-tool/package.json b/annotation-tool/package.json index a1f349d8..a9f5c9cc 100644 --- a/annotation-tool/package.json +++ b/annotation-tool/package.json @@ -1,7 +1,7 @@ { "name": "@fovea/annotation-tool", "private": true, - "version": "0.5.3", + "version": "0.5.4", "type": "module", "scripts": { "dev": "vite", diff --git a/annotation-tool/src/api/generated/openapi.ts b/annotation-tool/src/api/generated/openapi.ts index 918265a7..13281501 100644 --- a/annotation-tool/src/api/generated/openapi.ts +++ b/annotation-tool/src/api/generated/openapi.ts @@ -4104,6 +4104,7 @@ export interface paths { processingTimeFusion?: number; comment?: string | null; createdBy?: string; + projectId?: string | null; /** Format: date-time */ createdAt: string; /** Format: date-time */ @@ -4188,6 +4189,7 @@ export interface paths { processingTimeFusion?: number; comment?: string | null; createdBy?: string; + projectId?: string | null; /** Format: date-time */ createdAt: string; /** Format: date-time */ @@ -4324,6 +4326,7 @@ export interface paths { processingTimeFusion?: number; comment?: string | null; createdBy?: string; + projectId?: string | null; /** Format: date-time */ createdAt: string; /** Format: date-time */ @@ -4496,6 +4499,7 @@ export interface paths { processingTimeFusion?: number; comment?: string | null; createdBy?: string; + projectId?: string | null; /** Format: date-time */ createdAt: string; /** Format: date-time */ @@ -4638,6 +4642,7 @@ export interface paths { processingTimeFusion?: number; comment?: string | null; createdBy?: string; + projectId?: string | null; /** Format: date-time */ createdAt: string; /** Format: date-time */ @@ -4744,6 +4749,7 @@ export interface paths { processingTimeFusion?: number; comment?: string | null; createdBy?: string; + projectId?: string | null; /** Format: date-time */ createdAt: string; /** Format: date-time */ @@ -4854,6 +4860,7 @@ export interface paths { metadata?: ("text" | "non-text")[] | null; comment?: null | string; createdBy?: null | string; + projectId?: null | string; /** Format: date-time */ createdAt: string; /** Format: date-time */ @@ -5010,6 +5017,7 @@ export interface paths { metadata?: ("text" | "non-text")[] | null; comment?: null | string; createdBy?: null | string; + projectId?: null | string; /** Format: date-time */ createdAt: string; /** Format: date-time */ @@ -5139,6 +5147,7 @@ export interface paths { metadata?: ("text" | "non-text")[] | null; comment?: null | string; createdBy?: null | string; + projectId?: null | string; /** Format: date-time */ createdAt: string; /** Format: date-time */ @@ -5292,6 +5301,7 @@ export interface paths { metadata?: ("text" | "non-text")[] | null; comment?: null | string; createdBy?: null | string; + projectId?: null | string; /** Format: date-time */ createdAt: string; /** Format: date-time */ @@ -6032,6 +6042,7 @@ export interface paths { metadata?: ("text" | "non-text")[] | null; comment?: null | string; createdBy?: null | string; + projectId?: null | string; /** Format: date-time */ createdAt: string; /** Format: date-time */ @@ -11469,6 +11480,7 @@ export interface components { metadata?: ("text" | "non-text")[] | null; comment?: null | string; createdBy?: null | string; + projectId?: null | string; /** Format: date-time */ createdAt: string; /** Format: date-time */ diff --git a/annotation-tool/test/e2e/regression/claims/summary-project-scope.spec.ts b/annotation-tool/test/e2e/regression/claims/summary-project-scope.spec.ts new file mode 100644 index 00000000..602f3a7e --- /dev/null +++ b/annotation-tool/test/e2e/regression/claims/summary-project-scope.spec.ts @@ -0,0 +1,72 @@ +/** + * @file summary-project-scope.spec.ts + * @description End-to-end guard, through the running stack, that a video + * summary and the claims under it are stamped with their persona's project. + * A summary born with projectId = NULL is invisible to project collaborators + * and 403s their claim creation; this spec drives the real create routes and + * asserts the persisted project scope is returned on the API. + * + * The cross-user 403 itself is reproduced in the server integration test + * (test/integration/summary-claim-project-scope.test.ts); E2E worker users are + * system admins that bypass CASL, so here we verify the stamping that the + * authorization relies on. + */ + +import { test, expect } from '../../fixtures/test-context.js' + +test.describe('Summary project scope', () => { + test('stamps the persona project on summaries and claims created through the stack', async ({ + page, + testUser, + testVideo, + }) => { + const api = page.request + + // Create a project and a persona scoped to it. + const slug = `scope-${Date.now()}` + const projectRes = await api.post('/api/projects', { + data: { name: `Scope ${slug}`, slug }, + }) + expect(projectRes.status()).toBe(201) + const projectId = (await projectRes.json()).id + + const personaRes = await api.post('/api/personas', { + data: { + name: `Project persona ${slug}`, + role: 'Analyst', + informationNeed: 'Project-scoped authoring', + projectId, + }, + }) + expect(personaRes.status()).toBe(201) + const personaId = (await personaRes.json()).id + + // Create a summary under the project persona. + const summaryRes = await api.post('/api/summaries', { + data: { + videoId: testVideo.id, + personaId, + summary: [{ type: 'text', content: 'A red car drives through the intersection.' }], + }, + }) + expect(summaryRes.status()).toBe(201) + const summaryId = (await summaryRes.json()).id + + // The persisted summary carries the persona's project, not NULL. + const readRes = await api.get(`/api/videos/${testVideo.id}/summaries/${personaId}`) + expect(readRes.status()).toBe(200) + expect((await readRes.json()).projectId).toBe(projectId) + + // A claim created under it inherits the same project scope. + const claimRes = await api.post(`/api/summaries/${summaryId}/claims`, { + data: { summaryType: 'video', text: 'The car is red.', audio: ['speech'] }, + }) + expect(claimRes.status()).toBe(201) + + const claimsRes = await api.get(`/api/summaries/${summaryId}/claims`) + expect(claimsRes.status()).toBe(200) + const claims = await claimsRes.json() + expect(claims.length).toBeGreaterThan(0) + expect(claims[0].projectId).toBe(projectId) + }) +}) diff --git a/docs/docs/project/changelog.md b/docs/docs/project/changelog.md index 5ffc06a2..c6dfe0cf 100644 --- a/docs/docs/project/changelog.md +++ b/docs/docs/project/changelog.md @@ -11,6 +11,26 @@ 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.4] - 2026-06-25 + +The 0.5.4 patch fixes project-scope and ownership stamping on video summaries and claims ([#181](https://github.com/parafovea/fovea/pull/181)). Project collaborators could not see a teammate's summary or add claims under it because summaries were persisted without their persona's project, and model-generated summaries and extracted claims were left unowned. Nothing is breaking; the API additively gains a `projectId` field on summary and claim responses. + +### Fixed + +#### Summaries and Claims Are Stamped With Their Persona's Project + +- Every video-summary and claim write now stamps `projectId` from the persona (or the parent summary). Previously the interactive summary route (`server/src/routes/summaries.ts`), the summarization and claim-extraction workers (`server/src/queues/setup.ts`), and the auto-created empty summary (`server/src/repositories/ClaimRepository.ts`) all omitted it, so rows were born `projectId = NULL` and were invisible to every project collaborator except the creator — which `403`'d them at the parent-summary read gate when they tried to add claims, and hid the content from project-scoped queries. The summary update path also re-stamps the scope so a previously NULL-scoped row heals on its next save, and a migration backfills existing summaries and claims. + +#### Model-Generated Summaries and Extracted Claims Are Owned + +- The summarization and claim-extraction queue workers created rows without `createdBy`, leaving model-generated summaries and extracted claims owned by no one (readable only by an admin). The requesting user is now threaded through the queue payload and stamped as the owner on create. + +### Added + +#### projectId on Summary and Claim API Responses + +- The `VideoSummary` and `Claim` API responses now include `projectId`, so clients can reflect a resource's project scope; its prior absence had helped the stamping defect go unnoticed. + ## [0.5.3] - 2026-06-24 The 0.5.3 patch fixes a claims-workspace interaction bug ([#177](https://github.com/parafovea/fovea/pull/177)). No API shapes change and nothing is breaking. diff --git a/docs/package.json b/docs/package.json index 39e6f064..3c899092 100644 --- a/docs/package.json +++ b/docs/package.json @@ -1,6 +1,6 @@ { "name": "docs", - "version": "0.5.3", + "version": "0.5.4", "private": true, "scripts": { "docusaurus": "docusaurus", diff --git a/model-service/package.json b/model-service/package.json index 973c9526..a61d358e 100644 --- a/model-service/package.json +++ b/model-service/package.json @@ -1,6 +1,6 @@ { "name": "fovea-model-service", - "version": "0.5.3", + "version": "0.5.4", "description": "AI model inference service for video annotation", "scripts": { "dev": "source venv/bin/activate && uvicorn src.main:app --reload --host 0.0.0.0 --port 8000", diff --git a/model-service/pyproject.toml b/model-service/pyproject.toml index 814425db..da2057c4 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.3" +version = "0.5.4" description = "Model service for fovea video annotation tool" requires-python = ">=3.12" dependencies = [ diff --git a/package.json b/package.json index bde34b09..1da26ab5 100644 --- a/package.json +++ b/package.json @@ -1,7 +1,7 @@ { "name": "fovea", "private": true, - "version": "0.5.3", + "version": "0.5.4", "description": "FOVEA - Flexible Ontology Visual Event Analyzer", "packageManager": "pnpm@10.15.0", "engines": { diff --git a/server/openapi.json b/server/openapi.json index ff73e08d..a8de1fd1 100644 --- a/server/openapi.json +++ b/server/openapi.json @@ -432,6 +432,12 @@ "string" ] }, + "projectId": { + "type": [ + "null", + "string" + ] + }, "createdAt": { "format": "date-time", "type": "string" @@ -8149,6 +8155,16 @@ "createdBy": { "type": "string" }, + "projectId": { + "anyOf": [ + { + "type": "string" + }, + { + "type": "null" + } + ] + }, "createdAt": { "format": "date-time", "type": "string" @@ -8385,6 +8401,16 @@ "createdBy": { "type": "string" }, + "projectId": { + "anyOf": [ + { + "type": "string" + }, + { + "type": "null" + } + ] + }, "createdAt": { "format": "date-time", "type": "string" @@ -8706,6 +8732,16 @@ "createdBy": { "type": "string" }, + "projectId": { + "anyOf": [ + { + "type": "string" + }, + { + "type": "null" + } + ] + }, "createdAt": { "format": "date-time", "type": "string" @@ -9170,6 +9206,16 @@ "createdBy": { "type": "string" }, + "projectId": { + "anyOf": [ + { + "type": "string" + }, + { + "type": "null" + } + ] + }, "createdAt": { "format": "date-time", "type": "string" @@ -9636,6 +9682,16 @@ "createdBy": { "type": "string" }, + "projectId": { + "anyOf": [ + { + "type": "string" + }, + { + "type": "null" + } + ] + }, "createdAt": { "format": "date-time", "type": "string" @@ -9963,6 +10019,16 @@ "createdBy": { "type": "string" }, + "projectId": { + "anyOf": [ + { + "type": "string" + }, + { + "type": "null" + } + ] + }, "createdAt": { "format": "date-time", "type": "string" @@ -10485,6 +10551,12 @@ "string" ] }, + "projectId": { + "type": [ + "null", + "string" + ] + }, "createdAt": { "format": "date-time", "type": "string" @@ -11398,6 +11470,12 @@ "string" ] }, + "projectId": { + "type": [ + "null", + "string" + ] + }, "createdAt": { "format": "date-time", "type": "string" @@ -11933,6 +12011,12 @@ "string" ] }, + "projectId": { + "type": [ + "null", + "string" + ] + }, "createdAt": { "format": "date-time", "type": "string" @@ -12833,6 +12917,12 @@ "string" ] }, + "projectId": { + "type": [ + "null", + "string" + ] + }, "createdAt": { "format": "date-time", "type": "string" @@ -14859,6 +14949,12 @@ "string" ] }, + "projectId": { + "type": [ + "null", + "string" + ] + }, "createdAt": { "format": "date-time", "type": "string" diff --git a/server/package.json b/server/package.json index 1bb815aa..df975bf9 100644 --- a/server/package.json +++ b/server/package.json @@ -1,6 +1,6 @@ { "name": "@fovea/server", - "version": "0.5.3", + "version": "0.5.4", "private": true, "type": "module", "scripts": { diff --git a/server/prisma/migrations/20260625000000_backfill_summary_claim_project_scope/migration.sql b/server/prisma/migrations/20260625000000_backfill_summary_claim_project_scope/migration.sql new file mode 100644 index 00000000..8786d6ab --- /dev/null +++ b/server/prisma/migrations/20260625000000_backfill_summary_claim_project_scope/migration.sql @@ -0,0 +1,27 @@ +-- Backfill project scope onto summaries and claims that were created before +-- projectId was stamped on write. Such rows were born projectId = NULL even +-- when they belonged to a project persona, so project collaborators/managers +-- (whose read rule is { projectId IN (...) }) could not see them and were +-- 403'd when adding claims under them. +-- +-- A video summary inherits its persona's project; a video claim inherits its +-- (now-healed) parent summary's project. Rows whose persona/summary is personal +-- (projectId NULL) are intentionally left NULL. Only NULL rows are touched, so +-- this is safe to re-run and never overwrites an existing scope. + +-- 1) Summaries inherit their persona's project. +UPDATE "video_summaries" AS s +SET "projectId" = p."projectId" +FROM "personas" AS p +WHERE s."personaId" = p."id" + AND s."projectId" IS NULL + AND p."projectId" IS NOT NULL; + +-- 2) Video claims inherit their parent summary's (now-backfilled) project. +UPDATE "claims" AS c +SET "projectId" = s."projectId" +FROM "video_summaries" AS s +WHERE c."summaryType" = 'video' + AND c."summaryId" = s."id" + AND c."projectId" IS NULL + AND s."projectId" IS NOT NULL; diff --git a/server/src/queues/setup.ts b/server/src/queues/setup.ts index 2ec66fa3..3481fe50 100644 --- a/server/src/queues/setup.ts +++ b/server/src/queues/setup.ts @@ -172,6 +172,8 @@ export interface AudioOverridesJobData { export interface VideoSummarizationJobData { videoId: string; personaId: string; + /** Id of the user who requested the summary; stamped as the row's owner. */ + createdBy?: string; frameSampleRate?: number; maxFrames?: number; enableAudio?: boolean; @@ -213,6 +215,7 @@ export const videoWorker = new Worker< const { videoId, personaId, + createdBy, frameSampleRate = 1, maxFrames = 30, enableAudio, @@ -341,11 +344,22 @@ export const videoWorker = new Worker< processingTimeAudio: modelResponse.processing_time_audio || undefined, processingTimeVisual: modelResponse.processing_time_visual || undefined, processingTimeFusion: modelResponse.processing_time_fusion || undefined, + // Re-stamp project scope so a row created before this fix is healed. + projectId: persona.projectId, updatedAt: new Date(), }, create: { videoId, personaId, + // Stamp the requesting user as owner so the generated summary is + // readable by its creator (a personal persona has no project scope to + // fall back on). Only set on create; the update branch preserves the + // original owner. + createdBy: createdBy ?? undefined, + // Stamp the persona's project so a model-generated summary is born in + // the right project scope; without it the row is NULL-scoped and + // project members cannot read it. + projectId: persona.projectId, // Convert text summary to GlossItem[] format summary: [{ type: 'text', content: modelResponse.summary }], visualAnalysis: modelResponse.visual_analysis, @@ -453,6 +467,8 @@ claimQueueEvents.on("failed", async ({ jobId, failedReason }) => { export interface ClaimExtractionJobData { summaryId: string; summaryType: "video" | "collection"; + /** Id of the user who requested extraction; stamped as each claim's owner. */ + createdBy?: string; config: { inputSources: { includeSummaryText: boolean; @@ -515,7 +531,7 @@ export const claimWorker = new Worker< >( "claim-extraction", async (job): Promise => { - const { summaryId, summaryType, config } = job.data; + const { summaryId, summaryType, config, createdBy } = job.data; await job.updateProgress(10); @@ -641,6 +657,12 @@ export const claimWorker = new Worker< 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, diff --git a/server/src/repositories/ClaimRepository.ts b/server/src/repositories/ClaimRepository.ts index 6dc6631e..d96f756d 100644 --- a/server/src/repositories/ClaimRepository.ts +++ b/server/src/repositories/ClaimRepository.ts @@ -172,11 +172,24 @@ export class ClaimRepository { * Upserts a video summary for a (videoId, personaId) pair, creating an empty * one if absent and leaving an existing one untouched. * + * A freshly created summary is stamped with the caller's id and the persona's + * project scope so it is owned and project-visible from birth. Without this + * the auto-created parent summary would be NULL-scoped and unreadable, which + * 403s project collaborators adding claims under it (and orphans the summary + * from its own creator). + * * @param videoId - the video ID * @param personaId - the persona UUID + * @param projectId - the persona's project scope (null for personal personas) + * @param createdBy - the id of the user the summary is created for * @returns the existing or newly created summary */ - async upsertEmptyVideoSummary(videoId: string, personaId: string): Promise { + async upsertEmptyVideoSummary( + videoId: string, + personaId: string, + projectId: string | null, + createdBy: string, + ): Promise { return this.prisma.videoSummary.upsert({ where: { videoId_personaId: { @@ -188,6 +201,8 @@ export class ClaimRepository { videoId, personaId, summary: [], + projectId: projectId ?? undefined, + createdBy, }, update: {}, }) diff --git a/server/src/routes/claims.ts b/server/src/routes/claims.ts index 2dad13db..647d9b11 100644 --- a/server/src/routes/claims.ts +++ b/server/src/routes/claims.ts @@ -106,6 +106,9 @@ const ClaimSchema: any = Type.Recursive(This => Type.Object({ ])), comment: Type.Optional(NullableString), createdBy: Type.Optional(NullableString), + // The project the claim is scoped to (inherited from its summary; null for + // personal personas). Exposed so project scope is observable on the API. + projectId: Type.Optional(NullableString), createdAt: Type.String({ format: 'date-time' }), updatedAt: Type.String({ format: 'date-time' }), subclaims: Type.Optional(Type.Array(This)) diff --git a/server/src/routes/summaries.ts b/server/src/routes/summaries.ts index eb8285dd..585006a3 100644 --- a/server/src/routes/summaries.ts +++ b/server/src/routes/summaries.ts @@ -46,6 +46,7 @@ interface AudioOverridesJob { interface SummarizeJobData { videoId: string; personaId: string; + createdBy: string; frameSampleRate: number; maxFrames: number; enableAudio?: boolean; @@ -113,6 +114,10 @@ const VideoSummarySchema = Type.Object({ processingTimeFusion: Type.Optional(Type.Number()), comment: Type.Optional(Type.Union([Type.String(), Type.Null()])), createdBy: Type.Optional(Type.String()), + // The project the summary is scoped to (null for personal personas). + // Exposed so clients can reflect project scope and so the scope is + // observable (its prior absence helped hide a stamping defect). + projectId: Type.Optional(Type.Union([Type.String(), Type.Null()])), createdAt: Type.String({ format: 'date-time' }), updatedAt: Type.String({ format: 'date-time' }), }) @@ -390,6 +395,7 @@ const summariesRoute: FastifyPluginAsync = async (fastify) => { const jobData: SummarizeJobData = { videoId, personaId, + createdBy: userId, frameSampleRate, maxFrames, } @@ -658,6 +664,10 @@ const summariesRoute: FastifyPluginAsync = async (fastify) => { processingTimeAudio: processingTimeAudio || undefined, processingTimeVisual: processingTimeVisual || undefined, processingTimeFusion: processingTimeFusion || undefined, + // Re-stamp the project scope from the persona on every save. The + // summary's project is always its persona's project; restamping + // also heals any row created before projectId was stamped. + projectId: persona.projectId, updatedAt: new Date(), }, create: { @@ -678,6 +688,11 @@ const summariesRoute: FastifyPluginAsync = async (fastify) => { processingTimeVisual: processingTimeVisual || undefined, processingTimeFusion: processingTimeFusion || undefined, createdBy: userId, + // Stamp the persona's project so the summary is born in the right + // project scope. Without this the row is NULL-scoped and project + // collaborators (read rule { projectId: { in: [...] } }) cannot see + // it, which 403s their attempts to add claims under it. + projectId: persona.projectId, }, }) diff --git a/server/src/services/claim-service.ts b/server/src/services/claim-service.ts index 78956b20..762a09fa 100644 --- a/server/src/services/claim-service.ts +++ b/server/src/services/claim-service.ts @@ -568,6 +568,7 @@ export class ClaimService { const jobData: ClaimExtractionJobData = { summaryId, summaryType, + createdBy: this.userId ?? undefined, config: { inputSources: config.inputSources, extractionStrategy: config.extractionStrategy, @@ -938,8 +939,15 @@ export class ClaimService { throw new ForbiddenError('Cannot update this VideoSummary') } - // Find or create VideoSummary - const summary = await this.repository.upsertEmptyVideoSummary(videoId, personaId) + // Find or create VideoSummary, stamping the persona's project scope and + // the caller as owner so the auto-created parent is project-visible and + // owned (mirrors the child claim's projectId/createdBy stamping below). + const summary = await this.repository.upsertEmptyVideoSummary( + videoId, + personaId, + persona.projectId, + userId, + ) // If parentClaimId provided, verify it exists and belongs to same // summary, and that the caller can update it. diff --git a/server/test/integration/summary-claim-project-scope.test.ts b/server/test/integration/summary-claim-project-scope.test.ts new file mode 100644 index 00000000..d6329a48 --- /dev/null +++ b/server/test/integration/summary-claim-project-scope.test.ts @@ -0,0 +1,176 @@ +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 { hashPassword } from '../../src/lib/password.js' + +/** + * Reproduces the project-scope collaboration defect: a VideoSummary (and the + * claims under it) must be born stamped with the persona's project, otherwise + * project collaborators — who read project content through the project-scoped + * CASL rule, not ownership — cannot see the summary and are 403'd when adding + * claims under it. + * + * The members here are NON-admin project members granted ONLY project-scoped + * permissions (mirroring production), so the broad system-scope grants used by + * other route tests do not mask the bug. Against the unfixed code the + * collaborator assertions fail (the summary is born projectId = NULL). + */ +describe('Summary/Claim project scope for collaborators', () => { + let app: FastifyInstance + let prisma: PrismaClient + const PROJECT_ID = 'fixed-project-id' + + beforeAll(async () => { + app = await buildApp() + prisma = app.prisma + }) + + afterAll(async () => { + await app.close() + }) + + // creator (owns the persona) and collaborator (a second project member) + let creator: { id: string; token: string } + let collaborator: { id: string; token: string } + let personaId: string + let videoId: string + let video2Id: string + + async function makeMember(username: string, role: string): Promise<{ id: string; token: string }> { + const user = await prisma.user.create({ + data: { + username, + email: `${username}@example.com`, + passwordHash: await hashPassword(`pw-${username}`), + displayName: username, + isAdmin: false, + systemRole: 'user', + }, + }) + const session = await prisma.session.create({ + data: { + userId: user.id, + token: `test-session-${user.id}`, + expiresAt: new Date(Date.now() + 24 * 60 * 60 * 1000), + }, + }) + await prisma.projectMembership.create({ + data: { userId: user.id, projectId: PROJECT_ID, role }, + }) + return { id: user.id, token: session.token } + } + + beforeEach(async () => { + await prisma.loginAttempt.deleteMany() + await prisma.claimRelation.deleteMany() + await prisma.claim.deleteMany() + await prisma.annotation.deleteMany() + await prisma.videoSummary.deleteMany() + await prisma.ontology.deleteMany() + await prisma.worldState.deleteMany() + await prisma.persona.deleteMany() + await prisma.video.deleteMany() + await prisma.session.deleteMany() + await prisma.projectMembership.deleteMany() + await prisma.project.deleteMany() + await prisma.rolePermission.deleteMany() + await prisma.user.deleteMany() + + // Project-scope-only permissions for the annotator role. Deliberately NOT + // the broad system-scope grants used elsewhere, so reads resolve through + // the project condition and the NULL-projectId defect is observable. + await prisma.rolePermission.createMany({ + data: ['persona', 'summary', 'claim'].flatMap((resourceType) => + ['read', 'create'].map((action) => ({ + scope: 'project' as const, + role: 'annotator', + resourceType, + action, + ownOnly: false, + })), + ), + }) + + await prisma.project.create({ + data: { id: PROJECT_ID, name: 'Scope Project', slug: 'scope-project', createdBy: 'seed', ownerUserId: null }, + }) + + creator = await makeMember('creator', 'annotator') + collaborator = await makeMember('collaborator', 'annotator') + + // A project persona owned by the creator. + const persona = await prisma.persona.create({ + data: { + userId: creator.id, + projectId: PROJECT_ID, + name: 'Project persona', + role: 'Analyst', + informationNeed: 'Project-scoped authoring', + }, + }) + personaId = persona.id + + videoId = (await prisma.video.create({ data: { filename: 'a.mp4', path: '/v/a.mp4', duration: 60 } })).id + video2Id = (await prisma.video.create({ data: { filename: 'b.mp4', path: '/v/b.mp4', duration: 60 } })).id + }) + + it('stamps the persona project on POST /api/summaries so a collaborator can read it and add a claim', async () => { + // Creator authors the summary. + const createRes = await app.inject({ + method: 'POST', + url: '/api/summaries', + cookies: { session_token: creator.token }, + payload: { + videoId, + personaId, + summary: [{ type: 'text', content: 'A red car drives through the intersection.' }], + }, + }) + expect(createRes.statusCode).toBe(201) + const summaryId = createRes.json().id + + // The row is born in the persona's project, not NULL. + const persisted = await prisma.videoSummary.findUnique({ where: { id: summaryId } }) + expect(persisted?.projectId).toBe(PROJECT_ID) + + // The collaborator (not the creator) can now see it through project scope. + const listRes = await app.inject({ + method: 'GET', + url: `/api/videos/${videoId}/summaries`, + cookies: { session_token: collaborator.token }, + }) + expect(listRes.statusCode).toBe(200) + expect(listRes.json().map((s: { id: string }) => s.id)).toContain(summaryId) + + // And the collaborator can add a claim under it (the reported 403 is gone). + const claimRes = await app.inject({ + method: 'POST', + url: `/api/summaries/${summaryId}/claims`, + cookies: { session_token: collaborator.token }, + payload: { summaryType: 'video', text: 'The car is red.', audio: ['speech'] }, + }) + expect(claimRes.statusCode).toBe(201) + + const claim = await prisma.claim.findFirst({ where: { summaryId } }) + expect(claim?.projectId).toBe(PROJECT_ID) + }) + + it('auto-creates a project-scoped, owned summary when a collaborator adds the first claim', async () => { + // No summary exists for (video2, persona); the claim create must mint one + // stamped with the persona project and owned by the claim author. + const claimRes = await app.inject({ + method: 'POST', + url: `/api/videos/${video2Id}/personas/${personaId}/claims`, + cookies: { session_token: collaborator.token }, + payload: { text: 'An auto-created summary should be project scoped.' }, + }) + expect(claimRes.statusCode).toBe(201) + + const summary = await prisma.videoSummary.findUnique({ + where: { videoId_personaId: { videoId: video2Id, personaId } }, + }) + expect(summary?.projectId).toBe(PROJECT_ID) + expect(summary?.createdBy).toBe(collaborator.id) + }) +}) diff --git a/server/test/lib/summary-project-scope-read.test.ts b/server/test/lib/summary-project-scope-read.test.ts new file mode 100644 index 00000000..2c08c8bd --- /dev/null +++ b/server/test/lib/summary-project-scope-read.test.ts @@ -0,0 +1,49 @@ +import { describe, it, expect } from 'vitest' +import { subject } from '@casl/ability' +import { + defineAbilitiesFor, + type UserRoles, + type RolePermissionRow, +} from '../../src/lib/abilities.js' + +/** + * A project member reads project content through the project-scoped CASL rule + * (`{ projectId: { in: [...] } }`), not through ownership. A VideoSummary born + * with `projectId = NULL` is therefore invisible to every project collaborator + * except its own creator — which is exactly the failure caused by omitting + * `projectId` when persisting a summary. These tests pin that invariant at the + * authorization layer so a regression in projectId stamping is caught even + * without a database. + */ +describe('project-scoped summary read depends on a stamped projectId', () => { + const PROJECT = 'project-1' + const permissions: RolePermissionRow[] = [ + { scope: 'project', role: 'annotator', resourceType: 'summary', action: 'read', ownOnly: false }, + ] + const memberRoles: UserRoles = { + systemRole: 'user', + groupRoles: [], + projectRoles: [{ projectId: PROJECT, role: 'annotator' }], + } + + it('lets a collaborator read a summary stamped with their project', () => { + const collaborator = defineAbilitiesFor('user-b', memberRoles, permissions) + expect( + collaborator.can('read', subject('VideoSummary', { projectId: PROJECT, createdBy: 'user-a' })), + ).toBe(true) + }) + + it('hides a NULL-projectId summary from a project collaborator (the bug)', () => { + const collaborator = defineAbilitiesFor('user-b', memberRoles, permissions) + expect( + collaborator.can('read', subject('VideoSummary', { projectId: null, createdBy: 'user-a' })), + ).toBe(false) + }) + + it('still lets the creator read their own NULL-projectId summary (why the creator is not blocked)', () => { + const creator = defineAbilitiesFor('user-a', memberRoles, permissions) + expect( + creator.can('read', subject('VideoSummary', { projectId: null, createdBy: 'user-a' })), + ).toBe(true) + }) +}) diff --git a/server/test/repositories/ClaimRepository.test.ts b/server/test/repositories/ClaimRepository.test.ts new file mode 100644 index 00000000..597ebd35 --- /dev/null +++ b/server/test/repositories/ClaimRepository.test.ts @@ -0,0 +1,45 @@ +import { describe, it, expect, vi } from 'vitest' +import { PrismaClient } from '@prisma/client' +import { ClaimRepository } from '../../src/repositories/ClaimRepository.js' + +/** + * Unit coverage for the auto-created parent summary. When a claim is added to a + * (video, persona) that has no summary yet, the repository creates an empty one + * — and it must stamp the persona's project scope and the caller as owner, or + * the summary is born NULL-scoped/orphaned and becomes invisible to project + * collaborators (and even to its own creator). + */ +describe('ClaimRepository.upsertEmptyVideoSummary', () => { + const makeRepo = () => { + const upsert = vi.fn().mockResolvedValue({ id: 'summary-1' }) + const prisma = { videoSummary: { upsert } } as unknown as PrismaClient + return { repo: new ClaimRepository(prisma), upsert } + } + + it('stamps projectId and createdBy on the created summary', async () => { + const { repo, upsert } = makeRepo() + + await repo.upsertEmptyVideoSummary('video-1', 'persona-1', 'project-1', 'user-1') + + expect(upsert).toHaveBeenCalledWith( + expect.objectContaining({ + create: expect.objectContaining({ + videoId: 'video-1', + personaId: 'persona-1', + projectId: 'project-1', + createdBy: 'user-1', + }), + }), + ) + }) + + it('passes projectId undefined for a personal persona while still stamping the owner', async () => { + const { repo, upsert } = makeRepo() + + await repo.upsertEmptyVideoSummary('video-1', 'persona-1', null, 'user-1') + + const arg = upsert.mock.calls[0][0] + expect(arg.create.projectId).toBeUndefined() + expect(arg.create.createdBy).toBe('user-1') + }) +}) diff --git a/server/test/services/claim-service-generate.test.ts b/server/test/services/claim-service-generate.test.ts new file mode 100644 index 00000000..c96d4865 --- /dev/null +++ b/server/test/services/claim-service-generate.test.ts @@ -0,0 +1,53 @@ +import { describe, it, expect, vi, beforeEach } from 'vitest' + +// Mock the queue module before importing the service. This both lets us assert +// the enqueued job payload and avoids the real BullMQ/Redis connections the +// module opens on import. `vi.hoisted` makes the spy available inside the +// hoisted mock factory. +const { extractionAdd } = vi.hoisted(() => ({ extractionAdd: vi.fn() })) +vi.mock('../../src/queues/setup.js', () => ({ + claimExtractionQueue: { add: extractionAdd, getJob: vi.fn() }, + claimSynthesisQueue: { add: vi.fn(), getJob: vi.fn() }, +})) + +import { ClaimService } from '../../src/services/claim-service.js' +import { ClaimRepository } from '../../src/repositories/ClaimRepository.js' +import { defineAbilitiesFor, type UserRoles } from '../../src/lib/abilities.js' + +/** + * Claim extraction runs in a background worker that has no request user, so the + * initiating user must be threaded through the job payload to be stamped as the + * owner of each extracted claim. Without it, model-extracted claims are born + * createdBy = NULL and are unreadable to the person who requested them. + */ +describe('ClaimService.generateClaims threads the requesting user into the job', () => { + const adminRoles: UserRoles = { systemRole: 'system_admin', groupRoles: [], projectRoles: [] } + + beforeEach(() => { + extractionAdd.mockReset() + extractionAdd.mockResolvedValue({ id: 'job-1' }) + }) + + it('enqueues the extraction job with createdBy set to the caller', async () => { + const repository = { + findVideoSummaryById: vi.fn().mockResolvedValue({ id: 'summary-1', projectId: null, createdBy: 'user-1' }), + } as unknown as ClaimRepository + const ability = defineAbilitiesFor('user-1', adminRoles, []) + const service = new ClaimService(repository, ability, 'user-1', 'system_admin') + + await service.generateClaims('summary-1', { + inputSources: { + includeSummaryText: true, + includeAnnotations: false, + includeOntology: false, + ontologyDepth: 'names-only', + }, + extractionStrategy: 'sentence-based', + }) + + expect(extractionAdd).toHaveBeenCalledTimes(1) + const jobData = extractionAdd.mock.calls[0][1] + expect(jobData.createdBy).toBe('user-1') + expect(jobData.summaryId).toBe('summary-1') + }) +})