From 499b7dd9a79730bec77651cb2519b8b4c12c0ff9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marc-Andr=C3=A9=20Bombeck?= Date: Sat, 26 Sep 2026 00:38:26 +0200 Subject: [PATCH] fix(documents): keep deferred imports local and meter every re-send A document uploaded with aiRead=defer is now indexed locally by any automatic path, because the indexer reads the marker from the row rather than from the job. Index all for search leaves such documents out and no longer counts them. Only an explicit Read with AI or Generate summary ends the hold. Uploads that send a body keep their slot of the hourly limit, duplicates included; handing it back let a token send the same bytes under id after id for free. One document takes at most 20 further source ids, beyond which the upload answers 409 documents.inbound.sourceAliasLimit. The query-string key check is metered on the lookup allowance it shares with GET /api/documents/inbound/source. Refs #1038 --- docs/api/openapi.yaml | 68 +++++++++-------- .../documents/inbound/__tests__/route.test.ts | 58 ++++++++++++--- .../inbound/reindex/__tests__/route.test.ts | 9 ++- .../api/documents/inbound/reindex/route.ts | 9 ++- src/app/api/documents/inbound/route.ts | 73 +++++++++++++------ src/app/api/documents/inbound/source/route.ts | 25 +++---- .../__tests__/index-document.test.ts | 15 ++++ src/lib/documents/ai-route-support.ts | 6 ++ src/lib/documents/index-document.ts | 27 ++++--- src/lib/documents/source-key.ts | 40 +++++++++- .../document-content-index-backfill.test.ts | 4 + .../jobs/document-content-index-backfill.ts | 3 + src/lib/openapi/error-codes.ts | 1 + src/lib/openapi/routes/documents.ts | 11 ++- tests/integration/documents-import.test.ts | 23 ++++-- 15 files changed, 265 insertions(+), 107 deletions(-) diff --git a/docs/api/openapi.yaml b/docs/api/openapi.yaml index 588dc7eb2..4b47958d5 100644 --- a/docs/api/openapi.yaml +++ b/docs/api/openapi.yaml @@ -15679,15 +15679,18 @@ paths: `sourceId` (its id there, printable, up to 128 characters; needs `sourceSystem`) key the upload. A re-send with a known key is a duplicate: 200 with the live row (`meta.duplicate: true`), or, when the owner DELETED that document, 200 with `data.deleted: true` and `meta.deleted: true` and nothing stored; this holds after the 30-day - purge too. The key may instead ride the query string (`?sourceSystem=…&sourceId=…`): it is then answered before - the body is read and before the hourly allowance is charged, so a re-send of a document already held costs - neither; a key in both places must agree (else 422). A key that is answered with an existing document by its - bytes is remembered for that document, so deleting it keeps that key deleted too. `OTHER` is one shared - namespace per account: two different systems both sent as `OTHER` must not reuse each other's ids. An upload - answered as a duplicate or as deleted hands its slot of the hourly allowance back. `aiRead=defer` holds back - automatic AI reading for this upload: the thumbnail and a local text index still run, the summary and lab - staging do not, and the document stays out of the summary catch-up until the person reads it with AI or - generates its summary. Absent, behaviour is unchanged. + purge too. The key may also ride the query string (`?sourceSystem=…&sourceId=…`): a key the vault already holds + is then answered at once, the file is not stored or processed, and the request counts only toward the lookup + allowance shared with `GET /api/documents/inbound/source` (5000 an hour per token, per person for a session; 429 + past it), not toward the hourly upload limit. A key in both places must agree (else 422). Every other upload + counts toward the hourly upload limit, duplicates included. A key that is answered with an existing document by + its bytes is remembered for that document, so deleting it keeps that key deleted too; one document takes at most + 20 such keys, beyond which the upload is refused 409 (`documents.inbound.sourceAliasLimit`). `OTHER` is one + shared namespace per account: two different systems both sent as `OTHER` must not reuse each other's ids. + `aiRead=defer` holds back automatic AI reading for this upload: the thumbnail and a local text index still run, + the summary and lab staging do not, and the document stays out of every automatic AI path (the summary catch-up, + the automatic index, Index all for search) until the person reads it with AI or generates its summary. Absent, + behaviour is unchanged. **Narrow token.** Also reachable with a `documents:write` Bearer (minted at `POST /api/tokens/documents`), which @@ -15709,8 +15712,9 @@ paths: - PAPERLESS - PAPRA - OTHER - description: Import source key, first half. Sent here instead of (or agreeing with) the form field, it is checked before - the body is read or the hourly allowance is charged. + description: "Import source key, first half. Sent here (and, if also in the form, agreeing with it), a key the vault + already holds is answered at once: the file is not stored or processed and the request does not count toward + the hourly upload limit, only toward the lookup allowance shared with `GET /api/documents/inbound/source`." - name: sourceId in: query required: false @@ -15796,7 +15800,13 @@ paths: schema: $ref: "#/components/schemas/DocumentUploadEnvelope" "401": *a1 - "409": *a7 + "409": + description: The same bytes are already stored under 20 other import source ids (`documents.inbound.sourceAliasLimit`). + Nothing was stored or remembered. + content: + application/json: + schema: + $ref: "#/components/schemas/ErrorEnvelope" "413": description: "Too large: `meta.reason` is `fileTooLarge` (with `maxFileBytes`) or `quotaExceeded` (with `quotaBytes` + `usedBytes`)." @@ -15823,8 +15833,8 @@ paths: `known`, the document `id` (null once a deleted document was purged), and whether the owner `deleted` it. Checks the document's own key, keys remembered for it when an import sent the same bytes under another key, and the keys of purged documents. Reachable with a `documents:write` Bearer (the scope's second and last route) or a - cookie session; stores nothing. 5000 lookups an hour per token (per person for a session); gated on the - `inboundDocuments` module." + cookie session; stores nothing. 5000 lookups an hour per token (per person for a session), shared with the + upload's query-string key check; gated on the `inboundDocuments` module." parameters: - name: sourceSystem in: query @@ -30738,21 +30748,21 @@ components: documents.inbound.localOcrDisabled, documents.inbound.notFound, documents.inbound.notIndexed, documents.inbound.pdfNeedsAnthropic, documents.inbound.providerUnsupported, documents.inbound.quotaExceeded, documents.inbound.rateLimited, documents.inbound.restoreDuplicate, - documents.inbound.restoreGone, documents.inbound.uploadBusy, documents.inbound.uploadTimeout, - documents.inbound.vaccinationNotFound; encounter — encounter.invalid, encounter.link.invalid, - encounter.practitioner-not-found, encounter.reminder-conflict, encounter.reminder-not-found; environment - — environment.invalid, environment.no_home, environment.range_too_large, environment.travel.not_found; - export — export.selection.empty, export.selection.unknown_leaf; familyHistory — familyHistory.invalid; - feedback — feedback.body.invalid; health_score_config — health_score_config.invalid, - health_score_config.too_narrow; illness — illness.day-log.invalid, illness.disabled, - illness.episode.chronic-no-resolve, illness.episode.invalid, illness.episode.invalid-window, - illness.episode.parent-not-found, illness.insights.invalid; import — import.write_failed; ingest — - ingest.rate_limited; insights — insights.generate.budgetExceeded, insights.generate.outboundScreened, - insights.generate.profileScopeChanged, insights.generate.scopeChanged; labs — - labs.biomarker.create.invalid, labs.biomarker.duplicate, labs.biomarker.notFound, - labs.biomarker.update.invalid, labs.biomarker.update.referenceRangeInvalid, labs.create.invalid, - labs.ocr.budgetExceeded, labs.ocr.extractFailed, labs.ocr.fileTooLarge, labs.ocr.fileType, - labs.ocr.localOcrDisabled, labs.ocr.pdfNeedsAnthropic, labs.ocr.providerUnsupported, + documents.inbound.restoreGone, documents.inbound.sourceAliasLimit, documents.inbound.uploadBusy, + documents.inbound.uploadTimeout, documents.inbound.vaccinationNotFound; encounter — encounter.invalid, + encounter.link.invalid, encounter.practitioner-not-found, encounter.reminder-conflict, + encounter.reminder-not-found; environment — environment.invalid, environment.no_home, + environment.range_too_large, environment.travel.not_found; export — export.selection.empty, + export.selection.unknown_leaf; familyHistory — familyHistory.invalid; feedback — feedback.body.invalid; + health_score_config — health_score_config.invalid, health_score_config.too_narrow; illness — + illness.day-log.invalid, illness.disabled, illness.episode.chronic-no-resolve, illness.episode.invalid, + illness.episode.invalid-window, illness.episode.parent-not-found, illness.insights.invalid; import — + import.write_failed; ingest — ingest.rate_limited; insights — insights.generate.budgetExceeded, + insights.generate.outboundScreened, insights.generate.profileScopeChanged, + insights.generate.scopeChanged; labs — labs.biomarker.create.invalid, labs.biomarker.duplicate, + labs.biomarker.notFound, labs.biomarker.update.invalid, labs.biomarker.update.referenceRangeInvalid, + labs.create.invalid, labs.ocr.budgetExceeded, labs.ocr.extractFailed, labs.ocr.fileTooLarge, + labs.ocr.fileType, labs.ocr.localOcrDisabled, labs.ocr.pdfNeedsAnthropic, labs.ocr.providerUnsupported, labs.ocr.rateLimited, labs.restore.invalid, labs.result.notFound, labs.update.invalid, labs.update.linkedFieldsImmutable, labs.update.numericExpected, labs.update.qualitativeExpected, labs.update.referenceRangeInvalid, labs.update.sourceReferenceRangeInvalid; managed_profile — diff --git a/src/app/api/documents/inbound/__tests__/route.test.ts b/src/app/api/documents/inbound/__tests__/route.test.ts index 8bb21a712..bcb479701 100644 --- a/src/app/api/documents/inbound/__tests__/route.test.ts +++ b/src/app/api/documents/inbound/__tests__/route.test.ts @@ -56,6 +56,7 @@ vi.mock("@/lib/db", () => { documentSourceAlias: { findUnique: vi.fn(), createMany: vi.fn(), + count: vi.fn(), }, extractedFact: { groupBy: vi.fn(), @@ -247,6 +248,7 @@ beforeEach(() => { vi.mocked(prisma.documentSourceAlias.createMany).mockResolvedValue({ count: 1, } as never); + vi.mocked(prisma.documentSourceAlias.count).mockResolvedValue(0 as never); // No settings row / no user override → policy defaults (25 MiB / 1 GiB). vi.mocked(prisma.appSettings.findUnique).mockResolvedValue(null as never); vi.mocked(prisma.user.findUnique).mockResolvedValue(null as never); @@ -844,7 +846,28 @@ describe("POST /api/documents/inbound — document import", () => { ); expect(res.status).toBe(200); expect((await res.json()).data).toEqual({ id: "doc-src", duplicate: true }); - expect(checkRateLimit).not.toHaveBeenCalled(); + // Metered on the lookup bucket only; no upload slot is spent. + expect( + vi.mocked(checkRateLimit).mock.calls.map((c) => [c[0], c[1]]), + ).toEqual([["documents-source:token:tok-1", 5000]]); + }); + + it("429s the query-string check once the lookup bucket is spent", async () => { + armToken([DOCUMENTS_WRITE_SCOPE]); + vi.mocked(checkRateLimit).mockResolvedValueOnce({ + allowed: false, + remaining: 0, + limit: 5000, + resetAt: Date.now() + 60_000, + } as never); + const res = await post( + new Request( + "http://localhost/api/documents/inbound?sourceSystem=PAPRA&sourceId=doc_1", + { method: "POST", body: "x" }, + ), + ); + expect(res.status).toBe(429); + expect(prisma.inboundDocument.findFirst).not.toHaveBeenCalled(); }); it("422s half a query-string key", async () => { @@ -871,7 +894,7 @@ describe("POST /api/documents/inbound — document import", () => { expect(txCreate).not.toHaveBeenCalled(); }); - it("remembers the key when keyed bytes are already stored, and refunds the slot", async () => { + it("remembers the key when keyed bytes are already stored, and keeps the slot charged", async () => { // Own-key lookup misses; the content lookup finds a live row. vi.mocked(prisma.inboundDocument.findFirst) .mockResolvedValueOnce(null as never) @@ -891,10 +914,27 @@ describe("POST /api/documents/inbound — document import", () => { ], skipDuplicates: true, }); - expect(refundRateLimit).toHaveBeenCalledWith("documents-upload:user-1"); + // The body was read: the slot stays spent, so the same bytes under id + // after id are not free. + expect(refundRateLimit).not.toHaveBeenCalled(); expect(txCreate).not.toHaveBeenCalled(); }); + it("refuses a further id for bytes already held under the maximum", async () => { + vi.mocked(prisma.inboundDocument.findFirst) + .mockResolvedValueOnce(null as never) + .mockResolvedValueOnce(docRow({ id: "doc-manual" }) as never); + vi.mocked(prisma.documentSourceAlias.count).mockResolvedValue(20 as never); + const res = await post( + mkUpload({ sourceSystem: "PAPRA", sourceId: "doc_21" }), + ); + expect(res.status).toBe(409); + expect((await res.json()).meta.errorCode).toBe( + "documents.inbound.sourceAliasLimit", + ); + expect(prisma.documentSourceAlias.createMany).not.toHaveBeenCalled(); + }); + it("resolves a key held as an alias of a deleted document to deleted", async () => { vi.mocked(prisma.documentSourceAlias.findUnique).mockResolvedValue({ document: docRow({ id: "doc-gone", deletedAt: new Date() }), @@ -908,22 +948,18 @@ describe("POST /api/documents/inbound — document import", () => { duplicate: true, deleted: true, }); - expect(refundRateLimit).toHaveBeenCalled(); + expect(refundRateLimit).not.toHaveBeenCalled(); expect(txCreate).not.toHaveBeenCalled(); }); - it("refunds the slot on a plain content duplicate", async () => { + it("keeps a plain content duplicate charged and writes no alias", async () => { vi.mocked(prisma.inboundDocument.findFirst).mockResolvedValue( docRow({ id: "doc-existing" }) as never, ); await post(mkUpload()); - expect(refundRateLimit).toHaveBeenCalledWith("documents-upload:user-1"); - expect(prisma.documentSourceAlias.createMany).not.toHaveBeenCalled(); - }); - - it("does not refund a stored upload", async () => { - await post(mkUpload()); + expect(checkRateLimit).toHaveBeenCalledTimes(1); expect(refundRateLimit).not.toHaveBeenCalled(); + expect(prisma.documentSourceAlias.createMany).not.toHaveBeenCalled(); }); it("persists the deferral on the row", async () => { diff --git a/src/app/api/documents/inbound/reindex/__tests__/route.test.ts b/src/app/api/documents/inbound/reindex/__tests__/route.test.ts index d975d002c..bcb3dc018 100644 --- a/src/app/api/documents/inbound/reindex/__tests__/route.test.ts +++ b/src/app/api/documents/inbound/reindex/__tests__/route.test.ts @@ -114,11 +114,16 @@ describe("POST /api/documents/inbound/reindex", () => { it("scopes both counts to the caller's LIVE documents (the gauge's own figures)", async () => { await POST(req() as never); + // Imports held back from AI reading (#1038) are not part of the run and + // stay out of both figures. expect(prisma.inboundDocument.count).toHaveBeenCalledWith({ - where: { userId: "user-1", deletedAt: null }, + where: { userId: "user-1", deletedAt: null, aiReadDeferred: false }, }); expect(prisma.documentContentIndex.count).toHaveBeenCalledWith({ - where: { userId: "user-1", document: { deletedAt: null } }, + where: { + userId: "user-1", + document: { deletedAt: null, aiReadDeferred: false }, + }, }); }); }); diff --git a/src/app/api/documents/inbound/reindex/route.ts b/src/app/api/documents/inbound/reindex/route.ts index b5a60d403..5002aa2af 100644 --- a/src/app/api/documents/inbound/reindex/route.ts +++ b/src/app/api/documents/inbound/reindex/route.ts @@ -67,11 +67,16 @@ export const POST = apiHandler(async (request) => { let enqueued = 0; if (jobCreated) { const [indexedCount, totalCount] = await Promise.all([ + // Imports held back from AI reading (#1038) are not part of this run + // and are left out of both figures. prisma.documentContentIndex.count({ - where: { userId: user.id, document: { deletedAt: null } }, + where: { + userId: user.id, + document: { deletedAt: null, aiReadDeferred: false }, + }, }), prisma.inboundDocument.count({ - where: { userId: user.id, deletedAt: null }, + where: { userId: user.id, deletedAt: null, aiReadDeferred: false }, }), ]); enqueued = Math.max(0, totalCount - indexedCount); diff --git a/src/app/api/documents/inbound/route.ts b/src/app/api/documents/inbound/route.ts index e07b9c2df..70097db7b 100644 --- a/src/app/api/documents/inbound/route.ts +++ b/src/app/api/documents/inbound/route.ts @@ -70,6 +70,7 @@ import { enqueueDocumentSummary } from "@/lib/jobs/document-summary"; import { enqueueDocumentThumbnail } from "@/lib/jobs/document-thumbnail"; import { DOCUMENTS_WRITE_SCOPE } from "@/lib/documents/scopes"; import { + checkSourceLookupRateLimit, findSourceKey, rememberSourceAlias, type SourceKeyMatch, @@ -86,11 +87,7 @@ import { linkTargets } from "@/lib/links"; import { annotate } from "@/lib/logging/context"; import { requireModuleEnabled } from "@/lib/modules/gate"; import { isP2002 } from "@/lib/prisma-errors"; -import { - checkRateLimit, - rateLimitHeaders, - refundRateLimit, -} from "@/lib/rate-limit"; +import { checkRateLimit, rateLimitHeaders } from "@/lib/rate-limit"; import { documentCreateSchema, documentListQuerySchema, @@ -203,6 +200,19 @@ function deletedResponse(ctx: UploadContext, id: string | null): NextResponse { return receiptResponse(id, 200, { duplicate: true, deleted: true }); } +/** + * The same bytes have already been sent under too many source keys + * (`MAX_SOURCE_ALIASES_PER_DOCUMENT`). A conflict with what is stored, not a + * rate: nothing is remembered and nothing stored. + */ +function aliasLimitResponse(): NextResponse { + return apiError( + "This file is already stored under too many source ids.", + 409, + { errorCode: "documents.inbound.sourceAliasLimit" }, + ); +} + /** Answer a source key that is already held: duplicate, or deleted. */ function answerSourceKey( ctx: UploadContext, @@ -307,6 +317,22 @@ async function processUpload( ); } queryKey = parsedKey.data; + // Metered like the lookup route, on the same bucket: the check stores + // nothing and costs no upload slot, but it is not a free oracle either. + const lookupRl = await checkSourceLookupRateLimit( + scoped, + auth.session.id, + user.id, + ); + if (!lookupRl.allowed) { + const response = apiError("Too many lookups. Try again later.", 429, { + errorCode: "documents.inbound.rateLimited", + }); + for (const [k, v] of Object.entries(rateLimitHeaders(lookupRl))) { + response.headers.set(k, v); + } + return response; + } const match = await findSourceKey( user.id, queryKey.sourceSystem, @@ -337,11 +363,10 @@ async function processUpload( scoped ? resolveDocumentUploadLimitPerHour() : UPLOAD_LIMIT_PER_HOUR, UPLOAD_WINDOW_MS, ); - // A slot pays for a document stored. An upload that turns out to be one - // already held (same key, same bytes) hands its slot back, so re-sends — - // a Paperless workflow firing on every edit, a re-run over an archive — - // never eat into the allowance. - const refundSlot = () => refundRateLimit(bucketKey).catch(() => {}); + // Every upload that reaches the body pays its slot, stored or not. The cheap + // way to re-send is the query-string key above, answered before this line; + // handing slots back after a full body read would let a leaked token send + // the same bytes under id after id for free. if (!rl.allowed) { const response = apiError("Too many uploads. Try again later.", 429, { errorCode: "documents.inbound.rateLimited", @@ -452,10 +477,7 @@ async function processUpload( // the ledger remembers. if (sourceSystem && sourceId && !queryKey) { const match = await findSourceKey(user.id, sourceSystem, sourceId); - if (match) { - await refundSlot(); - return answerSourceKey(ctx, match); - } + if (match) return answerSourceKey(ctx, match); } let buffer: Buffer; @@ -522,9 +544,14 @@ async function processUpload( // none) is answered with that document — and the key is remembered for // it, so once the person deletes the document this key stays deleted too. if (sourceSystem && sourceId) { - await rememberSourceAlias(user.id, existing.id, sourceSystem, sourceId); + const remembered = await rememberSourceAlias( + user.id, + existing.id, + sourceSystem, + sourceId, + ); + if (remembered === "limit") return aliasLimitResponse(); } - await refundSlot(); return duplicateResponse(ctx, existing); } @@ -626,10 +653,7 @@ async function processUpload( // paths above would have. if (sourceSystem && sourceId) { const match = await findSourceKey(user.id, sourceSystem, sourceId); - if (match) { - await refundSlot(); - return answerSourceKey(ctx, match); - } + if (match) return answerSourceKey(ctx, match); } const winner = await prisma.inboundDocument.findFirst({ where: { userId: user.id, contentSha256, deletedAt: null }, @@ -637,9 +661,14 @@ async function processUpload( }); if (winner) { if (sourceSystem && sourceId) { - await rememberSourceAlias(user.id, winner.id, sourceSystem, sourceId); + const remembered = await rememberSourceAlias( + user.id, + winner.id, + sourceSystem, + sourceId, + ); + if (remembered === "limit") return aliasLimitResponse(); } - await refundSlot(); return duplicateResponse(ctx, winner); } } diff --git a/src/app/api/documents/inbound/source/route.ts b/src/app/api/documents/inbound/source/route.ts index bdc22cc83..caada8b66 100644 --- a/src/app/api/documents/inbound/source/route.ts +++ b/src/app/api/documents/inbound/source/route.ts @@ -20,22 +20,17 @@ import { sanitiseZodIssues, } from "@/lib/api-response"; import { DOCUMENTS_WRITE_SCOPE } from "@/lib/documents/scopes"; -import { findSourceKey } from "@/lib/documents/source-key"; +import { + checkSourceLookupRateLimit, + findSourceKey, +} from "@/lib/documents/source-key"; import { annotate } from "@/lib/logging/context"; import { requireModuleEnabled } from "@/lib/modules/gate"; -import { checkRateLimit, rateLimitHeaders } from "@/lib/rate-limit"; +import { rateLimitHeaders } from "@/lib/rate-limit"; import { documentSourceKeySchema } from "@/lib/validations/inbound-documents"; export const dynamic = "force-dynamic"; -/** - * Lookups per caller per hour. Generous on purpose: an importer asks once per - * document in the source, so a re-sync of a few thousand documents has to fit - * in an hour. A lookup reads three indexed rows and stores nothing. - */ -const LOOKUP_LIMIT_PER_HOUR = 5000; -const LOOKUP_WINDOW_MS = 60 * 60 * 1000; - export const GET = apiHandler(async (request: Request) => { const auth = await requireAuth(DOCUMENTS_WRITE_SCOPE); const { user } = auth; @@ -43,12 +38,10 @@ export const GET = apiHandler(async (request: Request) => { const gate = await requireModuleEnabled(user.id, "inboundDocuments"); if (!gate.enabled) return gate.response; - const rl = await checkRateLimit( - isScopedCredential(auth) - ? `documents-source:token:${auth.session.id}` - : `documents-source:${user.id}`, - LOOKUP_LIMIT_PER_HOUR, - LOOKUP_WINDOW_MS, + const rl = await checkSourceLookupRateLimit( + isScopedCredential(auth), + auth.session.id, + user.id, ); if (!rl.allowed) { const response = apiError("Too many lookups. Try again later.", 429, { diff --git a/src/lib/documents/__tests__/index-document.test.ts b/src/lib/documents/__tests__/index-document.test.ts index 40fb7409d..1a06e6d38 100644 --- a/src/lib/documents/__tests__/index-document.test.ts +++ b/src/lib/documents/__tests__/index-document.test.ts @@ -169,6 +169,21 @@ describe("indexDocumentContent — provider-first path", () => { }); describe("indexDocumentContent — local fallback path", () => { + it("a deferred row stays local even when the job did not ask for it", async () => { + // #1038 — the row decides: an automatic index of a deferred document + // never reaches a provider, whichever path enqueued it. + vi.mocked(loadOwnedDocument).mockResolvedValue({ + ...DOC, + aiReadDeferred: true, + } as never); + vi.mocked(resolveDocumentVisionProvider).mockResolvedValue(PICK as never); + visionOk(); + const outcome = await indexDocumentContent("user-1", "doc-1"); + expect(outcome).toMatchObject({ indexed: true, source: "local-pdf" }); + expect(resolveDocumentVisionProvider).not.toHaveBeenCalled(); + expect(transcribeDocument).not.toHaveBeenCalled(); + }); + it("a deferred import stays local even with an external pick and the toggle ON", async () => { // #1038 — `aiRead=defer`: the person asked for AI reading to wait. vi.mocked(resolveDocumentVisionProvider).mockResolvedValue(PICK as never); diff --git a/src/lib/documents/ai-route-support.ts b/src/lib/documents/ai-route-support.ts index 49cf6a6fb..5a903e484 100644 --- a/src/lib/documents/ai-route-support.ts +++ b/src/lib/documents/ai-route-support.ts @@ -113,6 +113,11 @@ export interface LoadedDocument { contentCodec: string; mimeType: string; status: string; + /** + * The import asked for `aiRead=defer` (#1038). Automatic paths read the + * document locally only while it is set. + */ + aiReadDeferred: boolean; } /** Load one live, owner-scoped document with the columns the AI actions need. */ @@ -129,6 +134,7 @@ export async function loadOwnedDocument( contentCodec: true, mimeType: true, status: true, + aiReadDeferred: true, }, }); } diff --git a/src/lib/documents/index-document.ts b/src/lib/documents/index-document.ts index faf8e984e..9674b2e1d 100644 --- a/src/lib/documents/index-document.ts +++ b/src/lib/documents/index-document.ts @@ -388,17 +388,20 @@ export async function indexDocumentContent( const document = await loadOwnedDocument(userId, documentId); if (!document) return { indexed: false, reason: "not-found" }; // v1.39.2 (#1038) — an import that asked for `aiRead=defer` gets the - // provider-free text layer only. An empty context is the same shape "no - // provider configured" already produces, so the tree falls straight to the - // local path without resolving a chain it would not use. - const provider: ResolvedIndexProvider = options.localOnly - ? { - chain: [], - pick: null, - consentOk: false, - dailyCap: 0, - costOwner: "operator", - } - : await resolveIndexProvider(userId); + // provider-free text layer only. The row decides, not only the job that + // asked: any automatic index of a deferred document stays local until the + // person reads it with AI themselves. An empty context is the same shape + // "no provider configured" already produces, so the tree falls straight to + // the local path without resolving a chain it would not use. + const provider: ResolvedIndexProvider = + options.localOnly || document.aiReadDeferred + ? { + chain: [], + pick: null, + consentOk: false, + dailyCap: 0, + costOwner: "operator", + } + : await resolveIndexProvider(userId); return indexLoadedDocument(userId, document, provider); } diff --git a/src/lib/documents/source-key.ts b/src/lib/documents/source-key.ts index 0ce97c450..0c5656f5b 100644 --- a/src/lib/documents/source-key.ts +++ b/src/lib/documents/source-key.ts @@ -15,6 +15,7 @@ * route both ask here, so they cannot disagree about what a key means. */ import { prisma } from "@/lib/db"; +import { checkRateLimit, type RateLimitResult } from "@/lib/rate-limit"; import type { DocumentSourceSystemValue } from "@/lib/validations/inbound-documents"; import type { SerialisableDocument } from "@/lib/documents/store"; @@ -54,6 +55,14 @@ export async function findSourceKey( return purged ? { state: "deleted", id: null } : null; } +/** + * How many further keys one document may collect. Real archives hold a file + * under one or two ids; the cap stops a leaked token from growing the table + * by sending the same bytes under id after id. Past it the upload is refused + * (409) rather than answered without remembering the key. + */ +export const MAX_SOURCE_ALIASES_PER_DOCUMENT = 20; + /** * Remember that `sourceId` in `sourceSystem` was answered with `documentId` * (same bytes, stored earlier). Without this the key would be forgotten, and @@ -65,9 +74,36 @@ export async function rememberSourceAlias( documentId: string, sourceSystem: DocumentSourceSystemValue, sourceId: string, -): Promise { - await prisma.documentSourceAlias.createMany({ +): Promise<"remembered" | "known" | "limit"> { + const held = await prisma.documentSourceAlias.count({ + where: { userId, documentId }, + }); + if (held >= MAX_SOURCE_ALIASES_PER_DOCUMENT) return "limit"; + const { count } = await prisma.documentSourceAlias.createMany({ data: [{ userId, documentId, sourceSystem, sourceId }], skipDuplicates: true, }); + return count > 0 ? "remembered" : "known"; +} + +/** + * Lookups of a source key per caller per hour, shared by the lookup route and + * the upload's query-string check so neither is an unmetered way to ask. + * Generous: an importer asks once per document, and a re-sync of a few + * thousand documents has to fit in an hour. Keyed on the token for a + * `documents:write` caller, on the person otherwise. + */ +const SOURCE_LOOKUP_LIMIT_PER_HOUR = 5000; +const SOURCE_LOOKUP_WINDOW_MS = 60 * 60 * 1000; + +export function checkSourceLookupRateLimit( + scoped: boolean, + tokenId: string, + userId: string, +): Promise { + return checkRateLimit( + scoped ? `documents-source:token:${tokenId}` : `documents-source:${userId}`, + SOURCE_LOOKUP_LIMIT_PER_HOUR, + SOURCE_LOOKUP_WINDOW_MS, + ); } diff --git a/src/lib/jobs/__tests__/document-content-index-backfill.test.ts b/src/lib/jobs/__tests__/document-content-index-backfill.test.ts index ca33cee73..189bcc9b9 100644 --- a/src/lib/jobs/__tests__/document-content-index-backfill.test.ts +++ b/src/lib/jobs/__tests__/document-content-index-backfill.test.ts @@ -270,6 +270,10 @@ describe("runContentIndexBackfillForUser", () => { reason: "ok", }); expect(upsertContentIndex).toHaveBeenCalledTimes(2); + // Imports held back from AI reading (#1038) are never candidates. + expect( + vi.mocked(prisma.inboundDocument.findMany).mock.calls[0]?.[0]?.where, + ).toMatchObject({ aiReadDeferred: false }); }); it("includes PDFs as candidates for an image-input provider and rasterises them", async () => { diff --git a/src/lib/jobs/document-content-index-backfill.ts b/src/lib/jobs/document-content-index-backfill.ts index 17b616348..2dc1bceb9 100644 --- a/src/lib/jobs/document-content-index-backfill.ts +++ b/src/lib/jobs/document-content-index-backfill.ts @@ -231,6 +231,9 @@ export async function runContentIndexBackfillForUser( deletedAt: null, mimeType: { in: candidateMimes }, contentIndex: { is: null }, + // An import held back from AI reading (#1038) is read only when the + // person opens it and asks; "index all" leaves it local. + aiReadDeferred: false, }, select: { id: true }, orderBy: { id: "asc" }, diff --git a/src/lib/openapi/error-codes.ts b/src/lib/openapi/error-codes.ts index 64548daa8..e25602b75 100644 --- a/src/lib/openapi/error-codes.ts +++ b/src/lib/openapi/error-codes.ts @@ -131,6 +131,7 @@ export const ERROR_CODE_CATALOGUE: Readonly> = "documents.inbound.rateLimited", "documents.inbound.restoreDuplicate", "documents.inbound.restoreGone", + "documents.inbound.sourceAliasLimit", "documents.inbound.uploadBusy", "documents.inbound.uploadTimeout", "documents.inbound.vaccinationNotFound", diff --git a/src/lib/openapi/routes/documents.ts b/src/lib/openapi/routes/documents.ts index 002253923..d31ad375d 100644 --- a/src/lib/openapi/routes/documents.ts +++ b/src/lib/openapi/routes/documents.ts @@ -407,7 +407,7 @@ export const inboundDocumentPaths: NonNullable = { required: false, schema: { type: "string", enum: [...DOCUMENT_SOURCE_SYSTEMS] }, description: - "Import source key, first half. Sent here instead of (or agreeing with) the form field, it is checked before the body is read or the hourly allowance is charged.", + "Import source key, first half. Sent here (and, if also in the form, agreeing with it), a key the vault already holds is answered at once: the file is not stored or processed and the request does not count toward the hourly upload limit, only toward the lookup allowance shared with `GET /api/documents/inbound/source`.", }, { name: "sourceId", @@ -422,7 +422,7 @@ export const inboundDocumentPaths: NonNullable = { summary: "Store a document (no extraction)", description: 'STORE-ONLY upload. Stores the raw document ENCRYPTED at rest with `status: STORED` and runs NO extraction — provider-free, no AI consent / budget / egress. A file is always filable, even with no document-scan provider configured. `multipart/form-data`: a `file` plus optional `title`, `kind`, `documentDate` (YYYY-MM-DD), and repeated `episodeIds` form fields (pre-link to the caller\'s illness/condition episodes). Accepted types (magic-byte sniffed, never the wire Content-Type): PDF/JPEG/PNG/WebP/GIF render inline; Office (docx/xlsx/pptx/doc/xls/ppt), text/CSV/Markdown/RTF, TIFF, HEIC/HEIF, XML/JSON are stored verbatim and served download-only. HEIC is stored as-is but attachment-only — prefer transcoding to JPEG client-side for inline preview parity. Error contract: `413` with `meta.reason = "fileTooLarge"` (+ `maxFileBytes`) or `"quotaExceeded"` (+ `quotaBytes`, `usedBytes`); `415` with `meta.reason = "unsupportedType"`. A same-user duplicate (same bytes) returns 200 + `meta.duplicate: true` with the existing row — not an error. `Idempotency-Key` honoured. Read `GET /api/documents/inbound/usage` for the effective limits before offering an upload. AI extraction is a separate opt-in action — see `POST /api/documents/inbound/{id}/extract`.\n\n' + - "**Importing from another system (v1.39.2).** Optional `sourceSystem` (`PAPERLESS`, `PAPRA`, `OTHER`) and `sourceId` (its id there, printable, up to 128 characters; needs `sourceSystem`) key the upload. A re-send with a known key is a duplicate: 200 with the live row (`meta.duplicate: true`), or, when the owner DELETED that document, 200 with `data.deleted: true` and `meta.deleted: true` and nothing stored; this holds after the 30-day purge too. The key may instead ride the query string (`?sourceSystem=…&sourceId=…`): it is then answered before the body is read and before the hourly allowance is charged, so a re-send of a document already held costs neither; a key in both places must agree (else 422). A key that is answered with an existing document by its bytes is remembered for that document, so deleting it keeps that key deleted too. `OTHER` is one shared namespace per account: two different systems both sent as `OTHER` must not reuse each other\'s ids. An upload answered as a duplicate or as deleted hands its slot of the hourly allowance back. `aiRead=defer` holds back automatic AI reading for this upload: the thumbnail and a local text index still run, the summary and lab staging do not, and the document stays out of the summary catch-up until the person reads it with AI or generates its summary. Absent, behaviour is unchanged.\n\n" + + "**Importing from another system (v1.39.2).** Optional `sourceSystem` (`PAPERLESS`, `PAPRA`, `OTHER`) and `sourceId` (its id there, printable, up to 128 characters; needs `sourceSystem`) key the upload. A re-send with a known key is a duplicate: 200 with the live row (`meta.duplicate: true`), or, when the owner DELETED that document, 200 with `data.deleted: true` and `meta.deleted: true` and nothing stored; this holds after the 30-day purge too. The key may also ride the query string (`?sourceSystem=…&sourceId=…`): a key the vault already holds is then answered at once, the file is not stored or processed, and the request counts only toward the lookup allowance shared with `GET /api/documents/inbound/source` (5000 an hour per token, per person for a session; 429 past it), not toward the hourly upload limit. A key in both places must agree (else 422). Every other upload counts toward the hourly upload limit, duplicates included. A key that is answered with an existing document by its bytes is remembered for that document, so deleting it keeps that key deleted too; one document takes at most 20 such keys, beyond which the upload is refused 409 (`documents.inbound.sourceAliasLimit`). `OTHER` is one shared namespace per account: two different systems both sent as `OTHER` must not reuse each other\'s ids. `aiRead=defer` holds back automatic AI reading for this upload: the thumbnail and a local text index still run, the summary and lab staging do not, and the document stays out of every automatic AI path (the summary catch-up, the automatic index, Index all for search) until the person reads it with AI or generates its summary. Absent, behaviour is unchanged.\n\n" + "**Narrow token.** Also reachable with a `documents:write` Bearer (minted at `POST /api/tokens/documents`), which reaches only this route and the source-key lookup `GET /api/documents/inbound/source`. Such a caller draws on a bucket of its own, keyed on the token (default 120 an hour, `DOCUMENT_UPLOAD_LIMIT_PER_HOUR`, clamped 1-1000; the 429 carries `Retry-After` and `X-RateLimit-*`), and gets a `DocumentUploadReceipt` instead of the stored row; its 413 `quotaExceeded` carries no `quotaBytes` / `usedBytes`. The response is an untagged union (row or receipt) on purpose: a discriminator would need a new field on the row every existing client decodes. A cookie or wildcard caller keeps the 60-an-hour per-user bucket and the full row. With the vault module off the answer is 403 `module.disabled` for every caller.", requestBody: { required: true, @@ -494,6 +494,11 @@ export const inboundDocumentPaths: NonNullable = { "Too large: `meta.reason` is `fileTooLarge` (with `maxFileBytes`) or `quotaExceeded` (with `quotaBytes` + `usedBytes`).", content: { "application/json": { schema: errorEnvelope } }, }, + "409": { + description: + "The same bytes are already stored under 20 other import source ids (`documents.inbound.sourceAliasLimit`). Nothing was stored or remembered.", + content: { "application/json": { schema: errorEnvelope } }, + }, "415": { description: 'Unsupported or unidentifiable type (`meta.reason = "unsupportedType"`). Executables, HTML/SVG, and generic archives are always refused.', @@ -508,7 +513,7 @@ export const inboundDocumentPaths: NonNullable = { tags: ["Documents"], summary: "Look up an import source key", description: - "Whether the vault already holds a document under this import source key, so an importer can skip a document before downloading it from the source system. Answers exactly what an upload with the same key would: `known`, the document `id` (null once a deleted document was purged), and whether the owner `deleted` it. Checks the document's own key, keys remembered for it when an import sent the same bytes under another key, and the keys of purged documents. Reachable with a `documents:write` Bearer (the scope's second and last route) or a cookie session; stores nothing. 5000 lookups an hour per token (per person for a session); gated on the `inboundDocuments` module.", + "Whether the vault already holds a document under this import source key, so an importer can skip a document before downloading it from the source system. Answers exactly what an upload with the same key would: `known`, the document `id` (null once a deleted document was purged), and whether the owner `deleted` it. Checks the document's own key, keys remembered for it when an import sent the same bytes under another key, and the keys of purged documents. Reachable with a `documents:write` Bearer (the scope's second and last route) or a cookie session; stores nothing. 5000 lookups an hour per token (per person for a session), shared with the upload's query-string key check; gated on the `inboundDocuments` module.", parameters: [ { name: "sourceSystem", diff --git a/tests/integration/documents-import.test.ts b/tests/integration/documents-import.test.ts index cc67863a1..d6b82bcc7 100644 --- a/tests/integration/documents-import.test.ts +++ b/tests/integration/documents-import.test.ts @@ -400,7 +400,7 @@ describe("source keys that reach a document by its bytes (#1038)", () => { }); }); -describe("re-sends cost no allowance and no body read (#1038)", () => { +describe("re-sends with the key in the address cost no upload slot (#1038)", () => { it("answers a query-string key before the body and the bucket, and the lookup agrees", async () => { await seedUser({ inboundDocuments: true }); token = await mintDocumentToken(); @@ -459,18 +459,25 @@ describe("re-sends cost no allowance and no body read (#1038)", () => { expect(blocked.status).toBe(429); }); - it("a duplicate by bytes hands its slot back", async () => { + it("an upload that sends its body pays its slot, duplicate or not", async () => { await seedUser({ inboundDocuments: true }); token = await mintDocumentToken(); process.env.DOCUMENT_UPLOAD_LIMIT_PER_HOUR = "2"; asToken(); expect((await post(upload(pdf("one")))).status).toBe(201); - // Charged and handed back each time; without the refund the second of - // these would already meet a spent bucket. - expect((await post(upload(pdf("one")))).status).toBe(200); - expect((await post(upload(pdf("one")))).status).toBe(200); - expect((await post(upload(pdf("two")))).status).toBe(201); - expect((await post(upload(pdf("three")))).status).toBe(429); + // The same bytes under a new id: answered, remembered, and charged, so + // id after id is not a free way to grow the alias table. + const again = await post( + upload(pdf("one"), { sourceSystem: "OTHER", sourceId: "a1" }), + ); + expect(again.status).toBe(200); + expect( + ( + await post( + upload(pdf("one"), { sourceSystem: "OTHER", sourceId: "a2" }), + ) + ).status, + ).toBe(429); }); });