diff --git a/docs/api/openapi.yaml b/docs/api/openapi.yaml index 6274c3408..1736800d0 100644 --- a/docs/api/openapi.yaml +++ b/docs/api/openapi.yaml @@ -15716,15 +15716,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 @@ -15746,8 +15749,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 @@ -15833,7 +15837,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`)." @@ -15860,8 +15870,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 @@ -30816,21 +30826,22 @@ components: documents.inbound.invalidUpdate, 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.restoreDuplicate, 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/scripts/__tests__/import-documents.test.ts b/scripts/__tests__/import-documents.test.ts index c547945be..4117e1048 100644 --- a/scripts/__tests__/import-documents.test.ts +++ b/scripts/__tests__/import-documents.test.ts @@ -90,6 +90,8 @@ async function healthlog( noLookup?: boolean; /** The address redirects elsewhere (http → https, a moved host). */ redirect?: string; + /** Lookups are spent: every lookup answers 429 with a long Retry-After. */ + lookup429?: boolean; } = {}, ) { const stored = new Set(); @@ -110,6 +112,13 @@ async function healthlog( !opts.noLookup ) { lookups.push(queryKey); + if (opts.lookup429) { + return { + status: 429, + headers: { "retry-after": "3600" }, + json: { data: null, error: "Too many lookups." }, + }; + } const deleted = opts.deleted?.includes(queryKey) ?? false; const known = deleted || stored.has(queryKey); return { @@ -158,9 +167,17 @@ async function healthlog( size = v.size; } } - // The key rides the query string, where HealthLog reads it first. - fields.sourceSystem = url.searchParams.get("sourceSystem") ?? ""; - fields.sourceId = url.searchParams.get("sourceId") ?? ""; + // The key rides the query string, where HealthLog reads it first, and + // the form; HealthLog refuses the two disagreeing. + if ( + fields.sourceSystem !== url.searchParams.get("sourceSystem") || + fields.sourceId !== url.searchParams.get("sourceId") + ) { + return { + status: 422, + json: { data: null, error: "source key mismatch" }, + }; + } received.push({ auth: req.headers.authorization, fields, filename, size }); const key = queryKey; if (opts.tooLarge?.includes(key)) { @@ -546,6 +563,28 @@ describe("import-documents.mjs — edges", () => { expect(res.stdout).toMatch(/Imported: 2/); }); + it("does not wait on spent lookups: the keyed upload answers instead", async () => { + const hl = await healthlog({ lookup429: true }); + const pl = await paperless(); + const started = Date.now(); + const res = await runScript( + [ + "paperless", + "--paperless-url", + pl.url, + "--healthlog-url", + hl.url, + "--tag", + "HealthLog", + ], + env, + ); + expect(res.code).toBe(0); + expect(res.stdout).toMatch(/Imported: 2/); + // A Retry-After of an hour was not honoured for a lookup. + expect(Date.now() - started).toBeLessThan(20_000); + }); + it("stops on a redirecting HealthLog address and names where it points", async () => { const hl = await healthlog({ redirect: "https://health.example" }); const pl = await paperless(); diff --git a/scripts/import-documents.mjs b/scripts/import-documents.mjs index 06f679248..307247b35 100644 --- a/scripts/import-documents.mjs +++ b/scripts/import-documents.mjs @@ -201,7 +201,7 @@ function retryAfterSeconds(response, fallback) { * service pacing us), a 5xx or a dropped connection is retried a few times * with a growing pause, and anything else is returned to the caller. */ -async function request(label, url, init, makeBody) { +async function request(label, url, init, makeBody, { waitOn429 = true } = {}) { let transient = 0; for (;;) { let response; @@ -218,7 +218,7 @@ async function request(label, url, init, makeBody) { await wait(TRANSIENT_BACKOFF_MS[transient++], `${label} did not answer`); continue; } - if (response.status === 429) { + if (response.status === 429 && waitOn429) { await response.arrayBuffer().catch(() => {}); const seconds = retryAfterSeconds(response, 60); await wait(seconds * 1000, `${label} asked to slow down`); @@ -569,12 +569,20 @@ function stopOnRefusal(response, body, url) { */ async function lookup(options, source, doc) { const url = `${options.healthlogUrl}/api/documents/inbound/source?${keyQuery(source, doc)}`; - const response = await request("HealthLog", url, { - headers: { - Authorization: `Bearer ${options.healthlogToken}`, - Accept: "application/json", + // Not worth waiting for: if the lookups are spent, the upload with the key + // answers the same question. + const response = await request( + "HealthLog", + url, + { + headers: { + Authorization: `Bearer ${options.healthlogToken}`, + Accept: "application/json", + }, }, - }); + undefined, + { waitOn429: false }, + ); const body = await readBody(response); stopOnRefusal(response, body, url); if (!response.ok || !body?.data?.known) return null; @@ -603,6 +611,9 @@ async function upload(options, source, doc, file) { if (doc.date) form.append("documentDate", doc.date); if (kind) form.append("kind", kind); if (!options.aiRead) form.append("aiRead", "defer"); + // In the form as well as the address; HealthLog checks they agree. + form.append("sourceSystem", source.system); + form.append("sourceId", doc.sourceId.slice(0, SOURCE_ID_MAX)); return form; }, ); @@ -789,8 +800,7 @@ function printSummary(options, tally, listed, totalBytes, unknownSizes) { if (!options.dryRun && tally.imported > 0 && !options.aiRead) { out( `${tally.imported} document(s) imported without automatic AI reading. ` + - "To have one read, open it in HealthLog and choose Read with AI or Generate summary; " + - "Documents > Index all for search reads the ones that had no text to search.", + "To have one read, open it in HealthLog and choose Read with AI or Generate summary.", ); } } 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 f899f226c..92602141d 100644 --- a/src/lib/openapi/error-codes.ts +++ b/src/lib/openapi/error-codes.ts @@ -132,6 +132,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); }); });