From 01a7224f29e49b63c6a63538cf0a3265f6fe6008 Mon Sep 17 00:00:00 2001 From: Maurice Date: Sat, 19 Sep 2026 12:05:37 +0200 Subject: [PATCH] fix(docsync): key the Synology device token per connection and share one transport The DSM trusted-device token was cached per NAS and account. A second trip whose owner knew the account password but never had the authenticator could bind the same account and log in on the first trip's token, a two-factor bypass across trip owners. Nothing ever cleared the entry either, so a token DSM stopped trusting went out ahead of any fresh code on every attempt until the process restarted. The token is now filed per connection, with instance and account still in the key. An edit keeps the connection id, so the token survives a password change and a stale code in the form. An unsaved form (connection 0) keeps what its probe earns on the session only, and the saved connection adopts it from there. A token DSM refuses with 403 or 404 is dropped for that connection alone. The four clients each carried their own copy of the transport, and the copies had drifted. provider-http.ts now holds the SSRF-guarded fetch with its timeout and certificate switch, the reading of a request that got no answer, the base status table and the guard on a streamed download. Timeouts, caps, auth headers and each client's error class and wording are unchanged. Where the copies differed by accident they now agree: - Paperless recognises UNABLE_TO_GET_ISSUER_CERT_LOCALLY (a private CA) as tls_untrusted instead of unreachable. - ERR_SSL_* handshake failures read as unreachable on WebDAV too, since the self-signed switch cannot fix them. - Synology maps HTTP 409, 415 and 507 like the others, Paperless maps 415. - Every download is refused on an announced length over its ceiling and counted as it streams. WebDAV and Paperless had no ceiling, Synology only read the header, and Papra's limiter left the source pinned when it tripped. - Papra reported a download without Content-Length as zero bytes, so the size from the listing was never used. PapraProvider and SynologyDriveProvider are renamed PapraDocumentProvider and SynologyDriveDocumentProvider, matching the other adapters. --- server/src/nest/doc-sync/doc-sync.module.ts | 16 +- .../doc-sync/providers/paperless.client.ts | 108 ++----- .../doc-sync/providers/paperless.provider.ts | 32 +- .../nest/doc-sync/providers/papra.client.ts | 173 +++------- .../nest/doc-sync/providers/papra.provider.ts | 2 +- .../nest/doc-sync/providers/provider-http.ts | 212 ++++++++++++ .../providers/synology-drive.client.ts | 195 +++++------ .../providers/synology-drive.provider.ts | 3 +- .../nest/doc-sync/providers/webdav.client.ts | 138 +++----- server/tests/e2e/doc-sync.e2e.test.ts | 8 +- .../nest/doc-sync/paperless.provider.test.ts | 19 ++ .../unit/nest/doc-sync/papra.provider.test.ts | 15 +- .../unit/nest/doc-sync/provider-http.test.ts | 304 ++++++++++++++++++ .../doc-sync/synology-drive.provider.test.ts | 187 +++++++++-- .../nest/doc-sync/webdav.provider.test.ts | 21 +- 15 files changed, 973 insertions(+), 460 deletions(-) create mode 100644 server/src/nest/doc-sync/providers/provider-http.ts create mode 100644 server/tests/unit/nest/doc-sync/provider-http.test.ts diff --git a/server/src/nest/doc-sync/doc-sync.module.ts b/server/src/nest/doc-sync/doc-sync.module.ts index fa8c35db6..dfeefcb00 100644 --- a/server/src/nest/doc-sync/doc-sync.module.ts +++ b/server/src/nest/doc-sync/doc-sync.module.ts @@ -18,8 +18,8 @@ import { DocSyncService } from './doc-sync.service'; import { DOCUMENT_PROVIDERS, type DocumentProvider } from './document-provider'; import { DocumentProviderRegistry } from './document-provider.registry'; import { PaperlessDocumentProvider } from './providers/paperless.provider'; -import { PapraProvider } from './providers/papra.provider'; -import { SynologyDriveProvider } from './providers/synology-drive.provider'; +import { PapraDocumentProvider } from './providers/papra.provider'; +import { SynologyDriveDocumentProvider } from './providers/synology-drive.provider'; import { NextcloudDocumentProvider, OpencloudDocumentProvider } from './providers/webdav.provider'; import { PaperlessClient } from './providers/paperless.client'; import { PapraClient } from './providers/papra.client'; @@ -68,25 +68,25 @@ import { WebdavClient } from './providers/webdav.client'; SynologyDriveClient, WebdavClient, PaperlessDocumentProvider, - PapraProvider, + PapraDocumentProvider, NextcloudDocumentProvider, OpencloudDocumentProvider, - SynologyDriveProvider, + SynologyDriveDocumentProvider, { provide: DOCUMENT_PROVIDERS, useFactory: ( paperless: PaperlessDocumentProvider, - papra: PapraProvider, + papra: PapraDocumentProvider, nextcloud: NextcloudDocumentProvider, opencloud: OpencloudDocumentProvider, - synology: SynologyDriveProvider, + synology: SynologyDriveDocumentProvider, ): readonly DocumentProvider[] => [paperless, papra, nextcloud, opencloud, synology], inject: [ PaperlessDocumentProvider, - PapraProvider, + PapraDocumentProvider, NextcloudDocumentProvider, OpencloudDocumentProvider, - SynologyDriveProvider, + SynologyDriveDocumentProvider, ], }, ], diff --git a/server/src/nest/doc-sync/providers/paperless.client.ts b/server/src/nest/doc-sync/providers/paperless.client.ts index 5bdd391dd..d73a55bd7 100644 --- a/server/src/nest/doc-sync/providers/paperless.client.ts +++ b/server/src/nest/doc-sync/providers/paperless.client.ts @@ -1,7 +1,12 @@ import { randomUUID } from 'node:crypto'; import type { DocsyncErrorCode } from '@trek/shared'; -import { safeFetch, SsrfBlockedError } from '../../../utils/ssrfGuard'; import { readCappedJson, readCappedText, discardBody } from '../../../utils/cappedFetch'; +import { + providerFetch, + statusErrorCode, + type TransportFailure, + type TransportFailureCode, +} from './provider-http'; import { PROVIDER_JSON_MAX_BYTES, PROVIDER_MAX_PAGES, @@ -325,67 +330,15 @@ export function sha256OrNull(value: string | null): string | null { return value !== null && /^[0-9a-f]{64}$/i.test(value) ? value.toLowerCase() : null; } -/** - * Whether a thrown error is a timeout rather than a dead host. - * - * `AbortSignal.timeout` rejects with a DOMException named `TimeoutError`; an - * aborted request surfaces as `AbortError`. Both mean the instance did not - * answer in time, which is a different thing for the user to fix than a host - * that refused the connection. - */ -function isTimeout(err: unknown): boolean { - if (!(err instanceof Error)) return false; - return err.name === 'TimeoutError' || err.name === 'AbortError'; -} +const TRANSPORT_MESSAGES: Record = { + ssrf_blocked: 'The address is blocked', + timeout: 'Paperless did not answer in time', + tls_untrusted: 'The certificate of this Paperless instance is not trusted', + unreachable: 'Could not reach Paperless', +}; -const TLS_ERROR_CODES = new Set([ - 'DEPTH_ZERO_SELF_SIGNED_CERT', - 'SELF_SIGNED_CERT_IN_CHAIN', - 'UNABLE_TO_VERIFY_LEAF_SIGNATURE', - 'CERT_HAS_EXPIRED', - 'ERR_TLS_CERT_ALTNAME_INVALID', - 'CERT_SIGNATURE_FAILURE', -]); - -/** - * A self-signed or otherwise untrusted certificate, anywhere in the cause chain. - * - * undici wraps the TLS failure in a `TypeError: fetch failed`, so the code that - * says what actually went wrong is one or two `cause` levels down. Telling this - * apart from "unreachable" is what lets the settings UI offer the "this - * instance uses a self-signed certificate" switch instead of a dead end. - */ -function isTlsFailure(err: unknown): boolean { - let current: unknown = err; - for (let depth = 0; depth < 5 && current instanceof Error; depth++) { - const code = 'code' in current ? String((current as { code?: unknown }).code) : ''; - if (TLS_ERROR_CODES.has(code)) return true; - current = (current as { cause?: unknown }).cause; - } - return false; -} - -function transportError(err: unknown): PaperlessError { - if (err instanceof SsrfBlockedError) { - return new PaperlessError('ssrf_blocked', 'The address is blocked', undefined, err.message); - } - if (isTimeout(err)) { - return new PaperlessError('timeout', 'Paperless did not answer in time'); - } - if (isTlsFailure(err)) { - return new PaperlessError( - 'tls_untrusted', - 'The certificate of this Paperless instance is not trusted', - undefined, - err instanceof Error ? err.message : undefined, - ); - } - return new PaperlessError( - 'unreachable', - 'Could not reach Paperless', - undefined, - err instanceof Error ? err.message : undefined, - ); +function transportError(failure: TransportFailure): PaperlessError { + return new PaperlessError(failure.code, TRANSPORT_MESSAGES[failure.code], undefined, failure.detail); } /** @@ -400,14 +353,7 @@ function classify(status: number, body: string): DocsyncErrorCode { if (/unique constraint|already exists/i.test(body)) return 'conflict'; return 'provider_error'; } - if (status === 401) return 'unauthorized'; - if (status === 403) return 'forbidden'; - if (status === 404) return 'not_found'; - if (status === 409) return 'conflict'; - if (status === 413) return 'too_large'; - if (status === 429) return 'rate_limited'; - if (status === 507) return 'quota_exceeded'; - return 'provider_error'; + return statusErrorCode(status); } /** Trim an upstream message to something a log line can hold. */ @@ -513,21 +459,15 @@ export class PaperlessClient { // as a bare `TypeError: fetch failed` two layers up. } - let response: Response; - try { - response = await safeFetch( - url.toString(), - { - method: options.method ?? 'GET', - headers, - body, - signal: AbortSignal.timeout(options.timeoutMs ?? PROVIDER_TIMEOUT_MS) as AbortSignal, - }, - { rejectUnauthorized: !creds.allowInsecureTls }, - ); - } catch (err: unknown) { - throw transportError(err); - } + const response = await providerFetch( + url.toString(), + { method: options.method ?? 'GET', headers, body }, + { + timeoutMs: options.timeoutMs ?? PROVIDER_TIMEOUT_MS, + allowInsecureTls: creds.allowInsecureTls, + onTransportFailure: transportError, + }, + ); if (!response.ok) { // The body is what separates "wrong file type" from "wrong request", so it diff --git a/server/src/nest/doc-sync/providers/paperless.provider.ts b/server/src/nest/doc-sync/providers/paperless.provider.ts index dca72ccfc..cc9a017ae 100644 --- a/server/src/nest/doc-sync/providers/paperless.provider.ts +++ b/server/src/nest/doc-sync/providers/paperless.provider.ts @@ -23,6 +23,7 @@ import { type PaperlessDocument, type PaperlessTag, } from './paperless.client'; +import { DOWNLOAD_MAX_BYTES, guardDownload } from './provider-http'; /** * Paperless-ngx as a document scope. @@ -412,12 +413,20 @@ export class PaperlessDocumentProvider implements DocumentProvider { // alongside the bytes, and it fails cheaply when the document is gone. const doc = await this.client.getDocument(creds, documentId); const response = await this.client.downloadOriginal(creds, documentId); - if (response.body === null) { - return docFail('provider_error', 'Paperless sent no body for the document'); - } + const download = guardDownload(response, { + maxBytes: DOWNLOAD_MAX_BYTES, + tooLarge: (declared) => + new PaperlessError( + 'too_large', + 'The document is larger than TREK will transfer', + undefined, + `content_length=${declared}`, + ), + noBody: () => new PaperlessError('provider_error', 'Paperless sent no body for the document'), + }); return docOk({ - body: Readable.fromWeb(response.body as Parameters[0]), - size: declaredLength(response.headers.get('content-length')), + body: download.body, + size: download.size, mimeType: response.headers.get('content-type') ?? doc.mimeType, remoteVersion: doc.modified ?? doc.added ?? String(doc.id), }); @@ -688,19 +697,6 @@ function credsOf(conn: DocumentConnectionRef): PaperlessCreds | null { }; } -/** - * The byte count a download announced, or null. - * - * An absent header must not become 0: `Number('')` is 0, and a size of zero - * reads as "an empty file" to everything downstream. Paperless omits the header - * whenever the response is compressed on the way out. - */ -function declaredLength(header: string | null): number | null { - if (header === null || header.trim().length === 0) return null; - const value = Number(header); - return Number.isFinite(value) && value >= 0 ? value : null; -} - function parseRemoteId(remoteId: string): number | null { if (!/^\d+$/.test(remoteId.trim())) return null; const id = Number.parseInt(remoteId.trim(), 10); diff --git a/server/src/nest/doc-sync/providers/papra.client.ts b/server/src/nest/doc-sync/providers/papra.client.ts index cc4e90d55..d06d376e6 100644 --- a/server/src/nest/doc-sync/providers/papra.client.ts +++ b/server/src/nest/doc-sync/providers/papra.client.ts @@ -1,9 +1,15 @@ -import { exceedsDeclaredLength, discardBody, readCappedJson } from '../../../utils/cappedFetch'; -import { safeFetch, SsrfBlockedError } from '../../../utils/ssrfGuard'; +import { readCappedJson } from '../../../utils/cappedFetch'; import type { DocsyncErrorCode } from '@trek/shared'; +import { + guardDownload, + providerFetch, + statusErrorCode, + type TransportFailure, + type TransportFailureCode, +} from './provider-http'; import crypto from 'node:crypto'; -import { Readable, Transform } from 'node:stream'; +import { Readable } from 'node:stream'; /** * Thin HTTP client for the Papra REST API (github.com/papra-hq/papra). @@ -262,66 +268,19 @@ export function tagSearchQuery(tagName: string): string | null { function classify(status: number, papraCode: string | null): DocsyncErrorCode { if (papraCode === 'tags.not_found') return 'scope_missing'; if (status === 400) return papraCode === 'server.invalid_request.params' ? 'not_found' : 'provider_error'; - if (status === 401) return 'unauthorized'; - if (status === 403) return 'forbidden'; - if (status === 404) return 'not_found'; - if (status === 409) return 'conflict'; - if (status === 413) return 'too_large'; - if (status === 415) return 'unsupported_type'; - if (status === 429) return 'rate_limited'; - if (status === 507) return 'quota_exceeded'; - return 'provider_error'; + return statusErrorCode(status); } +const TRANSPORT_MESSAGES: Record = { + ssrf_blocked: 'The Papra URL is not allowed', + timeout: 'Papra did not answer in time', + tls_untrusted: 'Papra presented a certificate that is not trusted', + unreachable: 'Could not reach Papra', +}; + /** Why a request never produced a response at all. */ -function classifyTransport(err: unknown): PapraError { - if (err instanceof SsrfBlockedError) { - return new PapraError('ssrf_blocked', 'The Papra URL is not allowed', undefined, err.message); - } - const detail = err instanceof Error ? err.message : String(err); - const name = err instanceof Error ? err.name : ''; - if (name === 'TimeoutError' || name === 'AbortError') { - return new PapraError('timeout', 'Papra did not answer in time', undefined, detail); - } - // undici buries the certificate verdict in a cause chain, so the whole chain - // is searched rather than the top-level message, which is a bare "fetch failed". - if (/self-signed|self signed|CERT_|ERR_TLS|unable to verify|certificate/i.test(causeChain(err))) { - return new PapraError('tls_untrusted', 'Papra presented a certificate that is not trusted', undefined, detail); - } - return new PapraError('unreachable', 'Could not reach Papra', undefined, detail); -} - -function causeChain(err: unknown): string { - const parts: string[] = []; - let current: unknown = err; - for (let depth = 0; depth < 5 && current instanceof Error; depth++) { - parts.push(current.message, String((current as { code?: unknown }).code ?? '')); - current = current.cause; - } - return parts.join(' '); -} - -/** - * A stream that fails instead of growing past `maxBytes`. - * - * The download is handed to the caller as a stream rather than buffered, so the - * declared-length check is only half a cap: a chunked answer declares nothing. - * This is the other half. - */ -function capStream(source: Readable, maxBytes: number): Readable { - let seen = 0; - const limiter = new Transform({ - transform(chunk: Buffer, _encoding, done) { - seen += chunk.length; - if (seen > maxBytes) { - done(new Error(`Papra document exceeded ${maxBytes} bytes`)); - return; - } - done(null, chunk); - }, - }); - source.on('error', (err) => limiter.destroy(err)); - return source.pipe(limiter); +function transportError(failure: TransportFailure): PapraError { + return new PapraError(failure.code, TRANSPORT_MESSAGES[failure.code], undefined, failure.detail); } /** RFC 7578 as undici's own FormData writes it: UTF-8 bytes, three characters escaped. */ @@ -380,21 +339,11 @@ export class PapraClient { }; if (options.json !== undefined) headers['Content-Type'] = 'application/json'; - let response: Response; - try { - response = await safeFetch( - url.toString(), - { - method, - headers, - body: options.json === undefined ? undefined : JSON.stringify(options.json), - signal: AbortSignal.timeout(TIMEOUT_MS) as AbortSignal, - }, - { rejectUnauthorized: !creds.allowInsecureTls }, - ); - } catch (err: unknown) { - throw classifyTransport(err); - } + const response = await providerFetch( + url.toString(), + { method, headers, body: options.json === undefined ? undefined : JSON.stringify(options.json) }, + { timeoutMs: TIMEOUT_MS, allowInsecureTls: creds.allowInsecureTls, onTransportFailure: transportError }, + ); return this.readBody(response, `${method} ${path}`, options.expectEmpty === true); } @@ -564,20 +513,11 @@ export class PapraClient { async downloadDocument(creds: PapraCreds, documentId: string): Promise { const url = `${papraApiBase(creds.baseUrl) + this.orgPath(creds)}/documents/${encodeURIComponent(documentId)}/file`; - let response: Response; - try { - response = await safeFetch( - url, - { - method: 'GET', - headers: { Authorization: `Bearer ${creds.apiKey}` }, - signal: AbortSignal.timeout(TRANSFER_TIMEOUT_MS) as AbortSignal, - }, - { rejectUnauthorized: !creds.allowInsecureTls }, - ); - } catch (err: unknown) { - throw classifyTransport(err); - } + const response = await providerFetch( + url, + { method: 'GET', headers: { Authorization: `Bearer ${creds.apiKey}` } }, + { timeoutMs: TRANSFER_TIMEOUT_MS, allowInsecureTls: creds.allowInsecureTls, onTransportFailure: transportError }, + ); if (!response.ok) { const body = await readCappedJson(response, MAX_JSON_BYTES); @@ -590,19 +530,12 @@ export class PapraClient { code ?? undefined, ); } - if (exceedsDeclaredLength(response, MAX_DOWNLOAD_BYTES)) { - discardBody(response); - throw new PapraError('too_large', 'The Papra document is larger than TREK will transfer', response.status); - } - if (!response.body) { - throw new PapraError('provider_error', 'Papra answered the document file with no body', response.status); - } - - const declared = Number(response.headers.get('content-length') ?? ''); - return { - body: capStream(Readable.fromWeb(response.body), MAX_DOWNLOAD_BYTES), - size: Number.isFinite(declared) && declared >= 0 ? declared : null, - }; + return guardDownload(response, { + maxBytes: MAX_DOWNLOAD_BYTES, + tooLarge: () => + new PapraError('too_large', 'The Papra document is larger than TREK will transfer', response.status), + noBody: () => new PapraError('provider_error', 'Papra answered the document file with no body', response.status), + }); } /** @@ -635,28 +568,22 @@ export class PapraClient { } const sentAt = Date.now(); - let response: Response; - try { - response = await safeFetch( - `${papraApiBase(creds.baseUrl) + this.orgPath(creds)}/documents`, - { - method: 'POST', - headers: { - Authorization: `Bearer ${creds.apiKey}`, - Accept: 'application/json', - 'Content-Type': `multipart/form-data; boundary=${boundary}`, - }, - body: Readable.toWeb(Readable.from(frame())), - // Required by undici for a streamed request body; the platform types - // carry the field, the DOM lib this project does not load does not. - duplex: 'half', - signal: AbortSignal.timeout(TRANSFER_TIMEOUT_MS) as AbortSignal, + const response = await providerFetch( + `${papraApiBase(creds.baseUrl) + this.orgPath(creds)}/documents`, + { + method: 'POST', + headers: { + Authorization: `Bearer ${creds.apiKey}`, + Accept: 'application/json', + 'Content-Type': `multipart/form-data; boundary=${boundary}`, }, - { rejectUnauthorized: !creds.allowInsecureTls }, - ); - } catch (err: unknown) { - throw classifyTransport(err); - } + body: Readable.toWeb(Readable.from(frame())), + // Required by undici for a streamed request body; the platform types + // carry the field, the DOM lib this project does not load does not. + duplex: 'half', + }, + { timeoutMs: TRANSFER_TIMEOUT_MS, allowInsecureTls: creds.allowInsecureTls, onTransportFailure: transportError }, + ); const elapsedMs = Date.now() - sentAt; const body = await this.readBody(response, 'POST /documents', false); diff --git a/server/src/nest/doc-sync/providers/papra.provider.ts b/server/src/nest/doc-sync/providers/papra.provider.ts index f4ccfa862..93f0b7dc4 100644 --- a/server/src/nest/doc-sync/providers/papra.provider.ts +++ b/server/src/nest/doc-sync/providers/papra.provider.ts @@ -42,7 +42,7 @@ import crypto from 'node:crypto'; * an existing document at all. */ @Injectable() -export class PapraProvider implements DocumentProvider { +export class PapraDocumentProvider implements DocumentProvider { readonly id = 'papra'; private readonly client = new PapraClient(); diff --git a/server/src/nest/doc-sync/providers/provider-http.ts b/server/src/nest/doc-sync/providers/provider-http.ts new file mode 100644 index 000000000..8eb3bb7f3 --- /dev/null +++ b/server/src/nest/doc-sync/providers/provider-http.ts @@ -0,0 +1,212 @@ +import { pipeline, Readable, Transform } from 'node:stream'; +import type { DocsyncErrorCode } from '@trek/shared'; +import { safeFetch, SsrfBlockedError } from '../../../utils/ssrfGuard'; +import { discardBody } from '../../../utils/cappedFetch'; + +/** + * The transport the four document clients share: the SSRF-guarded fetch with + * its timeout and certificate switch, the reading of a request that never got + * an answer, the base status table, and the guard on a streamed download. + * + * What stays in the clients is what differs on purpose: how each one + * authenticates, the shape of its requests (Papra's JSON, WebDAV's methods and + * Depth header, DSM's JSON-encoded form parameters), how it reads an error + * body, and which statuses mean something narrower on that product. Each + * client also keeps its own error class and wording, so the `instanceof` + * checks in the adapters see exactly the errors they always saw. + */ + +/** The four ways a request can fail without an HTTP answer. */ +export type TransportFailureCode = Extract< + DocsyncErrorCode, + 'ssrf_blocked' | 'timeout' | 'tls_untrusted' | 'unreachable' +>; + +export interface TransportFailure { + code: TransportFailureCode; + /** Every message in the cause chain, for the self-hoster's log. */ + detail: string; +} + +export interface ProviderFetchOptions { + timeoutMs: number; + /** The connection's self-signed switch. It relaxes the certificate check only; the SSRF guard still applies. */ + allowInsecureTls: boolean; + /** The client's own error for a request that never got an answer. */ + onTransportFailure: (failure: TransportFailure) => Error; +} + +export interface DownloadGuard { + maxBytes: number; + /** The client's error for a download that announces more than `maxBytes`. */ + tooLarge: (declared: number) => Error; + /** And for one that arrives without a body to stream. */ + noBody: () => Error; +} + +export interface GuardedDownload { + body: Readable; + /** The announced length, or null when the answer did not state one. */ + size: number | null; +} + +/** + * The ceiling for one download where an adapter has no number of its own. + * + * A runaway guard, not the install's file-size policy: the sync core applies + * that to every byte it pulls, and this sits far above it so that the core's + * verdict is the one a user sees. + */ +export const DOWNLOAD_MAX_BYTES = 2 * 1024 * 1024 * 1024; + +/** undici's own timers, plus the abort `AbortSignal.timeout` fires. */ +const TIMEOUT_NAME = /^(TimeoutError|AbortError)$/; +const TIMEOUT_CODE = /ABORT_ERR|HEADERS_TIMEOUT|BODY_TIMEOUT/i; + +/** + * Codes that are a verdict on the certificate rather than on the connection, + * which is what lets the settings screen offer the self-signed switch instead + * of a dead end. Most carry CERT or TLS in the name; these two do not. + * + * ERR_SSL_* is deliberately not among them: a handshake that fails on the + * protocol (https against a plain http port, say) is not something the switch + * can fix, and `tls_untrusted` tells the user it is. + */ +const CERTIFICATE_CODES = new Set(['UNABLE_TO_VERIFY_LEAF_SIGNATURE', 'HOSTNAME_MISMATCH']); +const CERTIFICATE_CODE = /CERT|SELF_SIGNED|ERR_TLS/i; + +/** + * The codes every product answers alike. A client layers its own meaning on + * top through `overrides`, and `fallback` answers whatever neither table knows. + */ +const STATUS_CODES: Readonly> = { + 401: 'unauthorized', + 403: 'forbidden', + 404: 'not_found', + 409: 'conflict', + 413: 'too_large', + 415: 'unsupported_type', + 429: 'rate_limited', + 507: 'quota_exceeded', +}; + +/** The error and each `cause` below it. undici hides the real reason one or two levels down. */ +function causeChain(err: unknown): unknown[] { + const chain: unknown[] = []; + let current: unknown = err; + for (let depth = 0; depth < 5 && current !== undefined && current !== null; depth++) { + chain.push(current); + current = current instanceof Error ? current.cause : undefined; + } + return chain; +} + +function codeOf(entry: unknown): string { + return typeof entry === 'object' && entry !== null && 'code' in entry + ? String((entry as { code: unknown }).code) + : ''; +} + +/** Why a request never produced a response. */ +export function classifyTransportFailure(err: unknown): TransportFailure { + const chain = causeChain(err); + const detail = chain + .map((entry) => (entry instanceof Error ? entry.message : String(entry))) + .filter((message) => message.length > 0) + .join(': '); + + if (err instanceof SsrfBlockedError) return { code: 'ssrf_blocked', detail }; + + const codes = chain.map(codeOf); + const names = chain.map((entry) => (entry instanceof Error ? entry.name : '')); + if (names.some((name) => TIMEOUT_NAME.test(name)) || codes.some((code) => TIMEOUT_CODE.test(code))) { + return { code: 'timeout', detail }; + } + if (codes.some((code) => CERTIFICATE_CODES.has(code) || CERTIFICATE_CODE.test(code))) { + return { code: 'tls_untrusted', detail }; + } + return { code: 'unreachable', detail }; +} + +/** + * One request to a user's instance: through the SSRF guard, bounded by + * `timeoutMs`, with the certificate check relaxed only when the connection + * asks for it. The timeout covers the body as well, so a streamed transfer is + * bounded end to end rather than only until the headers arrive. + */ +export async function providerFetch(url: string, init: RequestInit, options: ProviderFetchOptions): Promise { + try { + return await safeFetch( + url, + { ...init, signal: AbortSignal.timeout(options.timeoutMs) as AbortSignal }, + { rejectUnauthorized: !options.allowInsecureTls }, + ); + } catch (err: unknown) { + throw options.onTransportFailure(classifyTransportFailure(err)); + } +} + +/** The docsync code for an HTTP status that is not a product-specific answer. */ +export function statusErrorCode( + status: number, + overrides: Readonly> = {}, + fallback: DocsyncErrorCode = 'provider_error', +): DocsyncErrorCode { + return overrides[status] ?? STATUS_CODES[status] ?? fallback; +} + +/** + * The byte count a response announced, or null. + * + * An absent header must not become 0: `Number('')` is 0, and a size of zero + * reads as "an empty file" to everything downstream. Compressed and chunked + * answers routinely leave it out. + */ +export function declaredLength(header: string | null): number | null { + if (header === null || header.trim().length === 0) return null; + const value = Number(header); + return Number.isFinite(value) && value >= 0 ? value : null; +} + +/** + * Count the bytes as they pass. A declared length is only half a cap: a + * chunked answer declares nothing, and nothing stops a server from sending + * more than it announced. + */ +function limitStream(source: Readable, maxBytes: number): Readable { + let seen = 0; + const limiter = new Transform({ + transform(chunk: Buffer, _encoding, done) { + seen += chunk.length; + if (seen > maxBytes) { + done(new Error(`The download exceeded ${maxBytes} bytes`)); + return; + } + done(null, chunk); + }, + }); + // pipeline rather than pipe: a limiter that gives up, or a consumer that + // stops reading, has to take the source down with it. undici keeps the + // socket reserved until the body is released. + pipeline(source, limiter, () => {}); + return limiter; +} + +/** + * A download handed on as a stream, never buffered: the caller pipes it into + * storage. An announced length over the limit is refused before a byte is + * read, and the bytes are counted on the way through for the answers that + * announce nothing. + */ +export function guardDownload(response: Response, guard: DownloadGuard): GuardedDownload { + const size = declaredLength(response.headers.get('content-length')); + if (size !== null && size > guard.maxBytes) { + discardBody(response); + throw guard.tooLarge(size); + } + if (!response.body) throw guard.noBody(); + return { + body: limitStream(Readable.fromWeb(response.body as Parameters[0]), guard.maxBytes), + size, + }; +} diff --git a/server/src/nest/doc-sync/providers/synology-drive.client.ts b/server/src/nest/doc-sync/providers/synology-drive.client.ts index 90fe7fb7c..b5d3b2cb4 100644 --- a/server/src/nest/doc-sync/providers/synology-drive.client.ts +++ b/server/src/nest/doc-sync/providers/synology-drive.client.ts @@ -1,14 +1,20 @@ import { Readable } from 'node:stream'; import { createHash } from 'node:crypto'; import type { DocsyncErrorCode } from '@trek/shared'; -import { safeFetch, SsrfBlockedError } from '../../../utils/ssrfGuard'; -import { discardBody, exceedsDeclaredLength, readCappedJson } from '../../../utils/cappedFetch'; +import { discardBody, readCappedJson } from '../../../utils/cappedFetch'; import { PROVIDER_JSON_MAX_BYTES, PROVIDER_MAX_PAGES, PROVIDER_TIMEOUT_MS, PROVIDER_TRANSFER_TIMEOUT_MS, } from '../doc-sync.constants'; +import { + guardDownload, + providerFetch, + statusErrorCode, + type TransportFailure, + type TransportFailureCode, +} from './provider-http'; /** * Thin HTTP client for the Synology DSM Web API (SYNO.API.Auth + FileStation). @@ -90,6 +96,14 @@ export const MAX_FILE_PATH_LENGTH = 1024; /** Session-level codes: the SID is gone, and exactly one fresh login may fix it. */ const SESSION_CODES = new Set([105, 106, 107, 119]); +/** + * What SYNO.API.Auth answers when a login carried a device token and DSM still + * wants the second factor (403) or rejects it (404): the token is no longer + * trusted, because someone removed TREK from the account's trusted devices or + * it expired. + */ +const DEVICE_TOKEN_REFUSED_CODES = new Set([403, 404]); + /** * Generic Web API codes, identical on every endpoint. * @@ -182,20 +196,19 @@ const FILE_CODES: Record = { 1805: 'conflict', }; -/** OpenSSL / Node verdicts that mean "the certificate, not the connection". */ -const TLS_ERROR_CODES = new Set([ - 'SELF_SIGNED_CERT_IN_CHAIN', - 'DEPTH_ZERO_SELF_SIGNED_CERT', - 'UNABLE_TO_VERIFY_LEAF_SIGNATURE', - 'UNABLE_TO_GET_ISSUER_CERT', - 'UNABLE_TO_GET_ISSUER_CERT_LOCALLY', - 'CERT_HAS_EXPIRED', - 'CERT_NOT_YET_VALID', - 'ERR_TLS_CERT_ALTNAME_INVALID', - 'HOSTNAME_MISMATCH', -]); +const TRANSPORT_MESSAGES: Record = { + ssrf_blocked: 'Blocked by the SSRF guard', + timeout: 'The NAS did not answer in time', + tls_untrusted: 'The certificate the NAS presented is not trusted', + unreachable: 'Could not reach the NAS', +}; export interface SynologyDriveCreds { + /** + * The TREK connection these credentials belong to, or 0 for a form that has + * not been saved yet. It keys the device token; see `deviceTokens`. + */ + connectionId: number; /** Instance origin including the DSM port, e.g. `https://nas.example.com:5001`. */ baseUrl: string; username: string; @@ -203,7 +216,7 @@ export interface SynologyDriveCreds { /** * A TOTP code from the connection form. Single-use by construction, so it can * only ever serve the first login; the device token that login returns is what - * keeps later logins working, and it lives in this process's session cache. + * keeps later logins working, and it lives in this process's memory only. */ otpCode?: string; allowInsecureTls: boolean; @@ -400,37 +413,12 @@ function parseEntryList(value: unknown, key: string): SynoEntry[] { return entries; } -function transportError(error: unknown): SynologyDriveError { - if (error instanceof SsrfBlockedError) { - return new SynologyDriveError('ssrf_blocked', 'Blocked by the SSRF guard', { detail: error.message }); - } - const name = error instanceof Error ? error.name : ''; - if (name === 'TimeoutError' || name === 'AbortError') { - return new SynologyDriveError('timeout', 'The NAS did not answer in time'); - } - const cause = error instanceof Error && 'cause' in error ? (error as { cause?: unknown }).cause : undefined; - const codes = [error, cause] - .map((candidate) => (isRecord(candidate) && typeof candidate.code === 'string' ? candidate.code : null)) - .filter((code): code is string => code !== null); - if (codes.some((code) => TLS_ERROR_CODES.has(code))) { - return new SynologyDriveError('tls_untrusted', 'The certificate the NAS presented is not trusted', { - detail: codes.join(','), - }); - } - return new SynologyDriveError('unreachable', 'Could not reach the NAS', { - detail: error instanceof Error ? error.message : String(error), - }); +function transportError(failure: TransportFailure): SynologyDriveError { + return new SynologyDriveError(failure.code, TRANSPORT_MESSAGES[failure.code], { detail: failure.detail }); } function httpError(status: number): SynologyDriveError { - const code: DocsyncErrorCode = - status === 401 ? 'unauthorized' - : status === 403 ? 'forbidden' - : status === 404 ? 'not_found' - : status === 413 ? 'too_large' - : status === 429 ? 'rate_limited' - : 'provider_error'; - return new SynologyDriveError(code, `The NAS answered HTTP ${status}`, { status }); + return new SynologyDriveError(statusErrorCode(status), `The NAS answered HTTP ${status}`, { status }); } function appError(synoCode: number, isAuth: boolean): SynologyDriveError { @@ -464,12 +452,23 @@ export class SynologyDriveClient { private readonly lockouts = new Map(); /** - * DSM's trusted-device tokens, keyed by instance and account only. + * DSM's trusted-device tokens, one per TREK connection. * - * Deliberately not by credential: the token belongs to the account and the - * device, not to the password, so it has to survive a password edit and the - * moment the single-use OTP is cleared out of the form. Losing it would mean - * the next session expiry needs a human with an authenticator app. + * The token is DSM's record that somebody passed the second factor, and that + * somebody is whoever typed the code into this connection's form. Keyed by + * instance and account alone, a second trip's owner who knew the password but + * never had the authenticator could point a connection at the same account + * and log in on the first trip's token. + * + * Not keyed by the password or the code: the token has to survive a password + * edit and the single-use OTP going stale in the form, or the next session + * expiry needs a human with an authenticator app. An edit keeps the + * connection id, so it keeps the token. Instance and account stay in the key + * so a connection repointed at another NAS or account starts without one. + * + * A form that has not been saved has no id to file a token under (every + * unsaved form is connection 0), so what its probe earns rides on the session + * only, and the saved connection adopts it from there; see `session()`. */ private readonly deviceTokens = new Map(); @@ -484,16 +483,22 @@ export class SynologyDriveClient { return `${normalizeBaseUrl(creds.baseUrl)}\u0000${creds.username}`; } + /** Where this connection's device token lives, or null for a form not saved yet. */ + private deviceKey(creds: SynologyDriveCreds): string | null { + return creds.connectionId > 0 ? `${creds.connectionId}\u0000${this.accountKey(creds)}` : null; + } + private async request( url: string, init: RequestInit, creds: SynologyDriveCreds, + timeoutMs: number, ): Promise { - try { - return await safeFetch(url, init, { rejectUnauthorized: !creds.allowInsecureTls }); - } catch (error: unknown) { - throw transportError(error); - } + return providerFetch(url, init, { + timeoutMs, + allowInsecureTls: creds.allowInsecureTls, + onTransportFailure: transportError, + }); } /** @@ -528,13 +533,9 @@ export class SynologyDriveClient { const response = await this.request( normalizeBaseUrl(creds.baseUrl) + cgi, - { - method: 'POST', - headers, - body, - signal: AbortSignal.timeout(opts.timeoutMs ?? PROVIDER_TIMEOUT_MS) as AbortSignal, - }, + { method: 'POST', headers, body }, creds, + opts.timeoutMs ?? PROVIDER_TIMEOUT_MS, ); if (!response.ok) { @@ -583,22 +584,39 @@ export class SynologyDriveClient { } } + /** + * The cached session for this credential, or a fresh login. + * + * A cached session hands its device token to a saved connection that has + * none yet. That is how the code typed into a form that was tested before it + * was saved reaches the saved connection: the session is keyed by the whole + * credential, code included, so only a connection presenting that same code + * can find it. + */ private async session(creds: SynologyDriveCreds): Promise { const cached = this.sessions.get(this.sessionKey(creds)); - if (cached && Date.now() - cached.createdAt < SESSION_MAX_AGE_MS) return cached; - return await this.login(creds, this.deviceTokens.get(this.accountKey(creds)) ?? null); + if (cached && Date.now() - cached.createdAt < SESSION_MAX_AGE_MS) { + const deviceKey = this.deviceKey(creds); + if (deviceKey && cached.deviceId && !this.deviceTokens.has(deviceKey)) { + this.deviceTokens.set(deviceKey, cached.deviceId); + } + return cached; + } + return await this.login(creds); } /** * Log in, honouring the lockout. * - * The device token from a previous login is reused when there is one: the - * connection form's OTP is a single TOTP code that expires in thirty seconds, - * so it can only ever serve the first login of a process. Without the device - * token, every session expiry would need a human to type a new code. + * The connection's device token is reused when there is one: the form's OTP + * is a single TOTP code that expires in thirty seconds, so it can only ever + * serve the first login of a process. Without the device token, every session + * expiry would need a human to type a new code. */ - private async login(creds: SynologyDriveCreds, deviceId: string | null): Promise { + private async login(creds: SynologyDriveCreds): Promise { const key = this.sessionKey(creds); + const deviceKey = this.deviceKey(creds); + const deviceId = deviceKey ? this.deviceTokens.get(deviceKey) ?? null : null; const lockout = this.lockouts.get(key); if (lockout && lockout.until > Date.now()) { throw new SynologyDriveError(lockout.code, 'The NAS rejected these credentials; not retrying yet', { @@ -633,6 +651,12 @@ export class SynologyDriveClient { data = await this.call(creds, params, { cgi: AUTH_CGI }); } catch (error: unknown) { if (error instanceof SynologyDriveError && error.synoCode && error.synoCode >= 400) { + // A token DSM no longer trusts would otherwise go out ahead of any fresh + // code typed into the form, on every attempt, until the process restarts. + if (deviceKey && deviceId && DEVICE_TOKEN_REFUSED_CODES.has(error.synoCode) + && this.deviceTokens.get(deviceKey) === deviceId) { + this.deviceTokens.delete(deviceKey); + } this.lockouts.set(key, { until: Date.now() + (error.synoCode === 407 ? AUTOBLOCK_LOCKOUT_MS : CREDENTIAL_LOCKOUT_MS), code: error.code, @@ -647,7 +671,7 @@ export class SynologyDriveClient { throw new SynologyDriveError('provider_error', 'The NAS accepted the login but returned no session id'); } const issuedDeviceId = isRecord(data) && typeof data.did === 'string' ? data.did : deviceId; - if (issuedDeviceId) this.deviceTokens.set(this.accountKey(creds), issuedDeviceId); + if (deviceKey && issuedDeviceId) this.deviceTokens.set(deviceKey, issuedDeviceId); const session: SynoSession = { sid, synoToken: isRecord(data) && typeof data.synotoken === 'string' ? data.synotoken : null, @@ -847,9 +871,9 @@ export class SynologyDriveClient { // undici refuses a streaming body without it, and a stream is the point: // nothing here ever holds the whole file. duplex: 'half', - signal: AbortSignal.timeout(PROVIDER_TRANSFER_TIMEOUT_MS) as AbortSignal, }, creds, + PROVIDER_TRANSFER_TIMEOUT_MS, ); if (!response.ok) { @@ -871,8 +895,7 @@ export class SynologyDriveClient { * * A GET, because Download is the one FileStation method documented as one and * the session travels in the cookie, so nothing secret reaches the query - * string. The declared length is checked before the body is handed on: past - * this point nobody counts the bytes any more. + * string. */ async download(creds: SynologyDriveCreds, path: string, maxBytes: number): Promise { const session = await this.session(creds); @@ -888,12 +911,9 @@ export class SynologyDriveClient { const response = await this.request( url.toString(), - { - method: 'GET', - headers, - signal: AbortSignal.timeout(PROVIDER_TRANSFER_TIMEOUT_MS) as AbortSignal, - }, + { method: 'GET', headers }, creds, + PROVIDER_TRANSFER_TIMEOUT_MS, ); if (!response.ok) { @@ -908,22 +928,15 @@ export class SynologyDriveClient { const error = isRecord(parsed) && isRecord(parsed.error) ? parsed.error : undefined; throw appError(typeof error?.code === 'number' ? error.code : 0, false); } - if (exceedsDeclaredLength(response, maxBytes)) { - discardBody(response); - throw new SynologyDriveError('too_large', 'The file is larger than this install allows', { - detail: `content_length=${response.headers.get('content-length')}`, - }); - } - if (!response.body) { - throw new SynologyDriveError('provider_error', 'The NAS answered the download with an empty body'); - } - - const declared = Number(response.headers.get('content-length') ?? ''); - return { - body: Readable.fromWeb(response.body as Parameters[0]), - size: Number.isFinite(declared) && declared > 0 ? declared : null, - mimeType: contentType, - }; + const { body, size } = guardDownload(response, { + maxBytes, + tooLarge: (declared) => + new SynologyDriveError('too_large', 'The file is larger than this install allows', { + detail: `content_length=${declared}`, + }), + noBody: () => new SynologyDriveError('provider_error', 'The NAS answered the download with an empty body'), + }); + return { body, size, mimeType: contentType }; } /** Rename in place. The path is the identity here, so this also moves it. */ diff --git a/server/src/nest/doc-sync/providers/synology-drive.provider.ts b/server/src/nest/doc-sync/providers/synology-drive.provider.ts index ee430ae5b..64ba71603 100644 --- a/server/src/nest/doc-sync/providers/synology-drive.provider.ts +++ b/server/src/nest/doc-sync/providers/synology-drive.provider.ts @@ -153,7 +153,7 @@ function scopeOption(path: string, label?: string): DocumentScopeOption { } @Injectable() -export class SynologyDriveProvider implements DocumentProvider { +export class SynologyDriveDocumentProvider implements DocumentProvider { readonly id = PROVIDER_ID; /** @@ -540,6 +540,7 @@ export class SynologyDriveProvider implements DocumentProvider { if (!username || !password || !conn.baseUrl.trim()) return null; const otpCode = (conn.secrets.otp_code ?? '').trim(); return { + connectionId: conn.connectionId, baseUrl: conn.baseUrl, username, password, diff --git a/server/src/nest/doc-sync/providers/webdav.client.ts b/server/src/nest/doc-sync/providers/webdav.client.ts index 4d59ee168..19f905345 100644 --- a/server/src/nest/doc-sync/providers/webdav.client.ts +++ b/server/src/nest/doc-sync/providers/webdav.client.ts @@ -2,13 +2,13 @@ import { Injectable } from '@nestjs/common'; import { Readable } from 'node:stream'; import { XMLParser } from 'fast-xml-parser'; import type { DocsyncErrorCode } from '@trek/shared'; -import { safeFetch, SsrfBlockedError } from '../../../utils/ssrfGuard'; import { readCappedJson, readCappedText } from '../../../utils/cappedFetch'; import { PROVIDER_JSON_MAX_BYTES, PROVIDER_TIMEOUT_MS, PROVIDER_TRANSFER_TIMEOUT_MS, } from '../doc-sync.constants'; +import { DOWNLOAD_MAX_BYTES, guardDownload, providerFetch, statusErrorCode } from './provider-http'; /** * HTTP client for Nextcloud and OpenCloud. This is the ONLY place that talks to @@ -260,56 +260,19 @@ function decodeHrefPath(href: string): string { } } +/** + * 405 is MKCOL on an existing collection, 412 a failed If-Match, 423 a locked + * node: with 409, four ways of saying the caller's picture of the remote is + * stale. + */ +const STALE_STATUSES: Readonly> = { + 405: 'conflict', + 412: 'conflict', + 423: 'conflict', +}; + function classifyStatus(status: number): DocsyncErrorCode { - if (status === 401) return 'unauthorized'; - if (status === 403) return 'forbidden'; - if (status === 404) return 'not_found'; - // 405 is MKCOL on an existing collection, 412 a failed If-Match, 423 a locked - // node: three ways of saying the caller's picture of the remote is stale. - if (status === 405 || status === 409 || status === 412 || status === 423) return 'conflict'; - if (status === 413) return 'too_large'; - if (status === 415) return 'unsupported_type'; - if (status === 429) return 'rate_limited'; - if (status === 507) return 'quota_exceeded'; - return status >= 500 ? 'provider_error' : 'unknown'; -} - -/** Every `cause` in the chain, because undici hides the real reason two deep. */ -function causeChain(err: unknown): unknown[] { - const chain: unknown[] = []; - let current: unknown = err; - for (let depth = 0; depth < 5 && current !== undefined && current !== null; depth++) { - chain.push(current); - current = current instanceof Error ? (current as Error & { cause?: unknown }).cause : undefined; - } - return chain; -} - -function classifyTransport(err: unknown): { code: DocsyncErrorCode; detail: string } { - if (err instanceof SsrfBlockedError) { - return { code: 'ssrf_blocked', detail: err.message }; - } - const chain = causeChain(err); - const markers = chain.map((entry) => { - const named = entry instanceof Error ? entry.name : ''; - const coded = - typeof entry === 'object' && entry !== null && 'code' in entry - ? String((entry as { code: unknown }).code) - : ''; - return `${named}|${coded}`; - }); - const detail = chain - .map((entry) => (entry instanceof Error ? entry.message : String(entry))) - .filter((message) => message.length > 0) - .join(': '); - - if (markers.some((marker) => /TimeoutError|ABORT_ERR|HEADERS_TIMEOUT|BODY_TIMEOUT/i.test(marker))) { - return { code: 'timeout', detail }; - } - if (markers.some((marker) => /CERT|SELF_SIGNED|ERR_TLS|ERR_SSL/i.test(marker))) { - return { code: 'tls_untrusted', detail }; - } - return { code: 'unreachable', detail }; + return statusErrorCode(status, STALE_STATUSES, status >= 500 ? 'provider_error' : 'unknown'); } /** @@ -434,33 +397,6 @@ function escapeXmlText(value: string): string { .replace(/>/g, '>'); } -/** - * A web stream as a Node stream, by hand rather than through `Readable.fromWeb`, - * because the two `ReadableStream` declarations in scope — the platform's and - * `node:stream/web`'s — are different types and bridging them needs a cast this - * avoids. Cancelling the reader on destroy matters: undici keeps the connection - * reserved until the body ends. - */ -function toNodeStream(stream: ReadableStream): Readable { - const reader = stream.getReader(); - return new Readable({ - read() { - reader.read().then( - ({ done, value }) => { - this.push(done || !value ? null : Buffer.from(value)); - }, - (err: unknown) => { - this.destroy(err instanceof Error ? err : new Error(String(err))); - }, - ); - }, - destroy(err, callback) { - void reader.cancel().catch(() => {}); - callback(err); - }, - }); -} - @Injectable() export class WebdavClient { /** @@ -478,20 +414,12 @@ export class WebdavClient { const headers = new Headers(init.headers); headers.set('Authorization', `Basic ${auth}`); - try { - return await safeFetch( - `${creds.origin}${path}`, - { - ...init, - headers, - signal: AbortSignal.timeout(timeoutMs) as AbortSignal, - }, - { rejectUnauthorized: !creds.allowInsecureTls }, - ); - } catch (err: unknown) { - const { code, detail } = classifyTransport(err); - throw new WebdavError(code, 'Could not reach the WebDAV instance', undefined, detail); - } + return providerFetch(`${creds.origin}${path}`, { ...init, headers }, { + timeoutMs, + allowInsecureTls: creds.allowInsecureTls, + onTransportFailure: ({ code, detail }) => + new WebdavError(code, 'Could not reach the WebDAV instance', undefined, detail), + }); } /** The error for a response nobody wanted, with a bounded slice of its body as detail. */ @@ -654,25 +582,31 @@ export class WebdavClient { /** * GET, handed back as a stream. * - * Deliberately not read through the size cap: the caller pipes it into - * storage, and buffering a trip's scanned passport in memory to count its - * bytes is the opposite of what the cap is for. The declared length is - * reported so the core can refuse an oversized document before writing it. + * Never buffered: the caller pipes it into storage, and holding a trip's + * scanned passport in memory to count its bytes is the opposite of what a + * cap is for. The guard counts them on the way through instead. */ async get(creds: WebdavCreds, path: string): Promise { const response = await this.send(creds, path, { method: 'GET' }, PROVIDER_TRANSFER_TIMEOUT_MS); if (!response.ok) { throw await this.failed(response, 'GET'); } - const stream = response.body; - if (!stream) { - throw new WebdavError('provider_error', 'The instance answered a download without a body', response.status); - } - const length = response.headers.get('content-length'); + const { body, size } = guardDownload(response, { + maxBytes: DOWNLOAD_MAX_BYTES, + tooLarge: (declared) => + new WebdavError( + 'too_large', + 'The document is larger than TREK will transfer', + response.status, + `content_length=${declared}`, + ), + noBody: () => + new WebdavError('provider_error', 'The instance answered a download without a body', response.status), + }); const mimeType = response.headers.get('content-type'); return { - body: toNodeStream(stream), - size: length !== null && length !== '' && Number.isFinite(Number(length)) ? Number(length) : null, + body, + size, mimeType: mimeType ? mimeType.split(';')[0].trim() : null, etag: response.headers.get('oc-etag') ?? response.headers.get('etag') ?? '', }; diff --git a/server/tests/e2e/doc-sync.e2e.test.ts b/server/tests/e2e/doc-sync.e2e.test.ts index 0bd7dd469..454d8f125 100644 --- a/server/tests/e2e/doc-sync.e2e.test.ts +++ b/server/tests/e2e/doc-sync.e2e.test.ts @@ -76,8 +76,8 @@ import { DatabaseModule } from '../../src/nest/database/database.module'; import { AddonsService } from '../../src/nest/addons/addons.service'; import { DOCUMENT_PROVIDERS } from '../../src/nest/doc-sync/document-provider'; import { PaperlessDocumentProvider } from '../../src/nest/doc-sync/providers/paperless.provider'; -import { PapraProvider } from '../../src/nest/doc-sync/providers/papra.provider'; -import { SynologyDriveProvider } from '../../src/nest/doc-sync/providers/synology-drive.provider'; +import { PapraDocumentProvider } from '../../src/nest/doc-sync/providers/papra.provider'; +import { SynologyDriveDocumentProvider } from '../../src/nest/doc-sync/providers/synology-drive.provider'; import { NextcloudDocumentProvider, OpencloudDocumentProvider } from '../../src/nest/doc-sync/providers/webdav.provider'; import { TrekExceptionFilter } from '../../src/nest/common/trek-exception.filter'; import { ZodValidationPipe } from '../../src/nest/common/zod-validation.pipe'; @@ -129,10 +129,10 @@ describe('Document sync e2e (real guards + real services + temp SQLite)', () => async function build() { const providers = [ [PaperlessDocumentProvider, 'paperless'], - [PapraProvider, 'papra'], + [PapraDocumentProvider, 'papra'], [NextcloudDocumentProvider, 'nextcloud'], [OpencloudDocumentProvider, 'opencloud'], - [SynologyDriveProvider, 'synologydrive'], + [SynologyDriveDocumentProvider, 'synologydrive'], ] as const; let builder = Test.createTestingModule({ imports: [DatabaseModule, DocSyncModule] }) .overrideProvider(AddonsService) diff --git a/server/tests/unit/nest/doc-sync/paperless.provider.test.ts b/server/tests/unit/nest/doc-sync/paperless.provider.test.ts index 23c0d2023..83c14e06c 100644 --- a/server/tests/unit/nest/doc-sync/paperless.provider.test.ts +++ b/server/tests/unit/nest/doc-sync/paperless.provider.test.ts @@ -614,6 +614,25 @@ describe('PaperlessProvider — fetching bytes', () => { expect(expectFail(await provider.fetch(CONN, SCOPE, '../../etc/passwd')).code).toBe('not_found'); expect(calls).toHaveLength(before); }); + + it('PAPERLESS-043: a download without a body is a provider error, not an empty file', async () => { + on('GET /api/documents/11/', reply(docRow())); + on('GET /api/documents/11/download/', reply('')); + expect(expectFail(await provider.fetch(CONN, SCOPE, '11'))).toMatchObject({ + code: 'provider_error', + detail: 'Paperless sent no body for the document', + }); + }); + + it('PAPERLESS-044: a download announcing more than TREK transfers is refused before it is read', async () => { + on('GET /api/documents/11/', reply(docRow())); + on('GET /api/documents/11/download/', streamReply('%PDF', { 'content-length': String(3 * 1024 ** 3) })); + expect(expectFail(await provider.fetch(CONN, SCOPE, '11'))).toMatchObject({ + code: 'too_large', + detail: `content_length=${3 * 1024 ** 3}`, + }); + expect(discardBodyMock).toHaveBeenCalledTimes(1); + }); }); describe('PaperlessProvider — pushing bytes', () => { diff --git a/server/tests/unit/nest/doc-sync/papra.provider.test.ts b/server/tests/unit/nest/doc-sync/papra.provider.test.ts index 90c48249f..48e10b9ac 100644 --- a/server/tests/unit/nest/doc-sync/papra.provider.test.ts +++ b/server/tests/unit/nest/doc-sync/papra.provider.test.ts @@ -1,6 +1,6 @@ /** - * PapraProvider — what it puts on the wire, and what it makes of what comes - * back. + * PapraDocumentProvider: what it puts on the wire, and what it makes of what + * comes back. * * Papra is the provider whose answers lie the most: an upload that stores * nothing and restores a trashed twin instead still answers 200, a missing key @@ -27,7 +27,7 @@ import { snapshotVersion, tagSearchQuery, } from '../../../../src/nest/doc-sync/providers/papra.client'; -import { PapraProvider, buildScopeKey, parseScopeKey } from '../../../../src/nest/doc-sync/providers/papra.provider'; +import { PapraDocumentProvider, buildScopeKey, parseScopeKey } from '../../../../src/nest/doc-sync/providers/papra.provider'; import { multipartHeader } from '../../../../src/nest/doc-sync/providers/papra.client'; import crypto from 'node:crypto'; @@ -189,12 +189,12 @@ function err(result: { success: boolean; error?: { code: string; detail?: string return result.error!; } -let provider: PapraProvider; +let provider: PapraDocumentProvider; beforeEach(() => { calls = []; safeFetchMock.mockReset(); - provider = new PapraProvider(); + provider = new PapraDocumentProvider(); }); describe('capabilities', () => { @@ -565,6 +565,11 @@ describe('fetch', () => { expect(requested(1).pathname.endsWith('/file')).toBe(true); }); + it('reads a download without a length as the size the record lists, not as an empty file', async () => { + answerWith(reply({ body: { document: doc() } }), reply({ stream: Buffer.from('bordkarte bytes') })); + expect(ok(await provider.fetch(CONN, SCOPE, 'doc_mm5jovmyfbe4podpho983esv')).size).toBe(4096); + }); + it('refuses a body the instance declares as larger than TREK will transfer', async () => { answerWith( reply({ body: { document: doc() } }), diff --git a/server/tests/unit/nest/doc-sync/provider-http.test.ts b/server/tests/unit/nest/doc-sync/provider-http.test.ts new file mode 100644 index 000000000..3103b8c16 --- /dev/null +++ b/server/tests/unit/nest/doc-sync/provider-http.test.ts @@ -0,0 +1,304 @@ +/** + * The transport the four document clients share. + * + * The clients' own suites pin what each product puts on the wire; this one + * pins the pieces they all stand on, because a regression here breaks five + * providers at once: which failures read as a timeout, a certificate or a dead + * host, that the timeout and the self-signed switch reach the fetch, and that + * a download can neither announce nor stream its way past its ceiling. + * + * Only `safeFetch` is mocked. `cappedFetch` runs for real, so releasing an + * oversized body is the release that ships. + */ +import { describe, it, expect, vi, beforeEach } from 'vitest'; + +const { safeFetchMock, SsrfBlockedErrorMock } = vi.hoisted(() => { + class SsrfBlockedError extends Error { + constructor(message: string) { + super(message); + this.name = 'SsrfBlockedError'; + } + } + return { safeFetchMock: vi.fn(), SsrfBlockedErrorMock: SsrfBlockedError }; +}); +vi.mock('../../../../src/utils/ssrfGuard', () => ({ + safeFetch: safeFetchMock, + SsrfBlockedError: SsrfBlockedErrorMock, +})); + +import { + classifyTransportFailure, + declaredLength, + guardDownload, + providerFetch, + statusErrorCode, + type TransportFailure, +} from '../../../../src/nest/doc-sync/providers/provider-http'; + +/** `TypeError: fetch failed` with the real reason underneath, the way undici reports it. */ +function wrapped(cause: Error): Error { + return new TypeError('fetch failed', { cause }); +} + +function coded(message: string, code: string): Error { + return Object.assign(new Error(message), { code }); +} + +function named(message: string, name: string): Error { + return Object.assign(new Error(message), { name }); +} + +class TransportError extends Error { + constructor(readonly failure: TransportFailure) { + super(`transport: ${failure.code}`); + } +} + +const OPTIONS = { + timeoutMs: 5000, + allowInsecureTls: false, + onTransportFailure: (failure: TransportFailure) => new TransportError(failure), +}; + +/** A body that hands out `chunk`-sized pieces until `total` bytes, recording a cancel. */ +function source(chunk: number, total: number): { stream: ReadableStream; cancelled: () => boolean } { + let sent = 0; + let cancelled = false; + const stream = new ReadableStream({ + pull(controller) { + if (sent >= total) { + controller.close(); + return; + } + sent += chunk; + controller.enqueue(new Uint8Array(chunk).fill(97)); + }, + cancel() { + cancelled = true; + }, + }); + return { stream, cancelled: () => cancelled }; +} + +function download(body: ReadableStream | null, headers: Record = {}): Response { + return { ok: true, status: 200, headers: new Headers(headers), body } as unknown as Response; +} + +class TooLarge extends Error { + constructor(readonly declared: number) { + super('too large'); + } +} + +const GUARD = { + maxBytes: 4096, + tooLarge: (declared: number) => new TooLarge(declared), + noBody: () => new Error('no body'), +}; + +async function drain(stream: NodeJS.ReadableStream): Promise { + const chunks: Buffer[] = []; + for await (const chunk of stream) chunks.push(Buffer.from(chunk as Uint8Array)); + return Buffer.concat(chunks); +} + +beforeEach(() => { + safeFetchMock.mockReset(); +}); + +describe('classifyTransportFailure', () => { + it('PROVIDER-HTTP-001: the SSRF guard is its own verdict and keeps its reason', () => { + expect(classifyTransportFailure(new SsrfBlockedErrorMock('Requests to loopback are not allowed'))).toEqual({ + code: 'ssrf_blocked', + detail: 'Requests to loopback are not allowed', + }); + }); + + it.each([ + ['TimeoutError, as AbortSignal.timeout rejects', named('The operation was aborted due to timeout', 'TimeoutError')], + ['AbortError', named('This operation was aborted', 'AbortError')], + ['TimeoutError under undici\'s wrapper', wrapped(named('aborted', 'TimeoutError'))], + ['UND_ERR_HEADERS_TIMEOUT', wrapped(coded('Headers Timeout Error', 'UND_ERR_HEADERS_TIMEOUT'))], + ['UND_ERR_BODY_TIMEOUT', wrapped(coded('Body Timeout Error', 'UND_ERR_BODY_TIMEOUT'))], + ['ABORT_ERR', wrapped(coded('The operation was aborted', 'ABORT_ERR'))], + ])('PROVIDER-HTTP-002: a timeout, spelled as %s', (_label, error) => { + expect(classifyTransportFailure(error).code).toBe('timeout'); + }); + + it.each([ + 'DEPTH_ZERO_SELF_SIGNED_CERT', + 'SELF_SIGNED_CERT_IN_CHAIN', + // The private-CA case: the chain is fine, the issuer is simply unknown here. + 'UNABLE_TO_GET_ISSUER_CERT_LOCALLY', + 'UNABLE_TO_VERIFY_LEAF_SIGNATURE', + 'CERT_HAS_EXPIRED', + 'CERT_SIGNATURE_FAILURE', + 'ERR_TLS_CERT_ALTNAME_INVALID', + 'HOSTNAME_MISMATCH', + ])('PROVIDER-HTTP-003: %s is an untrusted certificate', (code) => { + expect(classifyTransportFailure(wrapped(coded('certificate verdict', code))).code).toBe('tls_untrusted'); + }); + + it('PROVIDER-HTTP-004: a protocol failure in the handshake is not offered as a certificate problem', () => { + // https against a plain http port. The self-signed switch cannot fix it. + const error = wrapped(coded('wrong version number', 'ERR_SSL_WRONG_VERSION_NUMBER')); + expect(classifyTransportFailure(error).code).toBe('unreachable'); + }); + + it('PROVIDER-HTTP-005: everything else is a host that cannot be reached, with the whole chain as detail', () => { + const error = wrapped(coded('connect ECONNREFUSED 192.168.1.9:5001', 'ECONNREFUSED')); + expect(classifyTransportFailure(error)).toEqual({ + code: 'unreachable', + detail: 'fetch failed: connect ECONNREFUSED 192.168.1.9:5001', + }); + expect(classifyTransportFailure('socket hang up')).toEqual({ code: 'unreachable', detail: 'socket hang up' }); + }); + + it('PROVIDER-HTTP-006: the cause chain is followed five levels and no further', () => { + let error: Error = coded('self-signed certificate', 'DEPTH_ZERO_SELF_SIGNED_CERT'); + for (let level = 0; level < 4; level++) error = wrapped(error); + expect(classifyTransportFailure(error).code).toBe('tls_untrusted'); + expect(classifyTransportFailure(wrapped(error)).code).toBe('unreachable'); + }); +}); + +describe('providerFetch', () => { + it('PROVIDER-HTTP-010: hands the request to the SSRF guard with a timeout and the certificate check on', async () => { + const answer = { ok: true, status: 200 }; + safeFetchMock.mockResolvedValue(answer); + const body = new URLSearchParams({ a: '1' }); + + const response = await providerFetch( + 'https://nas.example.org:5001/webapi/entry.cgi', + { method: 'POST', headers: { Accept: 'application/json' }, body, duplex: 'half' } as RequestInit, + OPTIONS, + ); + + expect(response).toBe(answer); + const [url, init, options] = safeFetchMock.mock.calls[0] as [string, RequestInit & { duplex?: string }, unknown]; + expect(url).toBe('https://nas.example.org:5001/webapi/entry.cgi'); + expect(init).toMatchObject({ method: 'POST', headers: { Accept: 'application/json' }, body, duplex: 'half' }); + expect(init.signal).toBeInstanceOf(AbortSignal); + expect(options).toEqual({ rejectUnauthorized: true }); + }); + + it('PROVIDER-HTTP-011: relaxes the certificate check only when the connection says so', async () => { + safeFetchMock.mockResolvedValue({ ok: true, status: 200 }); + await providerFetch('https://nas.example.org', {}, { ...OPTIONS, allowInsecureTls: true }); + expect(safeFetchMock.mock.calls[0][2]).toEqual({ rejectUnauthorized: false }); + }); + + it('PROVIDER-HTTP-012: an instance that never answers is cut off at the timeout', async () => { + safeFetchMock.mockImplementation( + (_url: string, init: RequestInit) => + new Promise((_resolve, reject) => { + init.signal?.addEventListener('abort', () => reject(init.signal?.reason)); + }), + ); + + const pending = providerFetch('https://nas.example.org', {}, { ...OPTIONS, timeoutMs: 20 }); + + await expect(pending).rejects.toBeInstanceOf(TransportError); + await expect(pending).rejects.toMatchObject({ failure: { code: 'timeout' } }); + }); + + it('PROVIDER-HTTP-013: a refusal becomes the client\'s own error, built from the classified failure', async () => { + safeFetchMock.mockRejectedValue(new SsrfBlockedErrorMock('Requests to link-local addresses are not allowed')); + const onTransportFailure = vi.fn((failure: TransportFailure) => new TransportError(failure)); + + const pending = providerFetch('http://169.254.169.254/', {}, { ...OPTIONS, onTransportFailure }); + + await expect(pending).rejects.toMatchObject({ + failure: { code: 'ssrf_blocked', detail: 'Requests to link-local addresses are not allowed' }, + }); + expect(onTransportFailure).toHaveBeenCalledTimes(1); + }); +}); + +describe('statusErrorCode', () => { + it('PROVIDER-HTTP-020: maps the statuses every product answers alike', () => { + const expected: Array<[number, string]> = [ + [401, 'unauthorized'], + [403, 'forbidden'], + [404, 'not_found'], + [409, 'conflict'], + [413, 'too_large'], + [415, 'unsupported_type'], + [429, 'rate_limited'], + [507, 'quota_exceeded'], + [500, 'provider_error'], + [418, 'provider_error'], + ]; + for (const [status, code] of expected) expect(statusErrorCode(status), `http ${status}`).toBe(code); + }); + + it('PROVIDER-HTTP-021: a client\'s own reading of a status wins, and its fallback answers the rest', () => { + expect(statusErrorCode(412, { 412: 'conflict' })).toBe('conflict'); + expect(statusErrorCode(404, { 404: 'scope_missing' })).toBe('scope_missing'); + expect(statusErrorCode(418, {}, 'unknown')).toBe('unknown'); + expect(statusErrorCode(401, {}, 'unknown')).toBe('unauthorized'); + }); +}); + +describe('declaredLength', () => { + it('PROVIDER-HTTP-030: an absent or unreadable header is an unknown size, never zero', () => { + expect(declaredLength(null)).toBeNull(); + expect(declaredLength('')).toBeNull(); + expect(declaredLength(' ')).toBeNull(); + expect(declaredLength('many')).toBeNull(); + expect(declaredLength('-1')).toBeNull(); + expect(declaredLength('0')).toBe(0); + expect(declaredLength('4096')).toBe(4096); + }); +}); + +describe('guardDownload', () => { + it('PROVIDER-HTTP-040: streams a body within the limit and reports what it announced', async () => { + const { stream } = source(1024, 3072); + const guarded = guardDownload(download(stream, { 'content-length': '3072' }), GUARD); + + expect(guarded.size).toBe(3072); + expect((await drain(guarded.body)).length).toBe(3072); + }); + + it('PROVIDER-HTTP-041: refuses an announced length over the limit before reading, and lets the socket go', () => { + const { stream, cancelled } = source(1024, 8192); + let thrown: unknown; + try { + guardDownload(download(stream, { 'content-length': '8192' }), GUARD); + } catch (error: unknown) { + thrown = error; + } + + expect(thrown).toBeInstanceOf(TooLarge); + expect((thrown as TooLarge).declared).toBe(8192); + expect(cancelled()).toBe(true); + }); + + it('PROVIDER-HTTP-042: a body that announces nothing and runs past the limit fails mid-stream', async () => { + const { stream, cancelled } = source(1024, 64 * 1024); + const guarded = guardDownload(download(stream), GUARD); + + expect(guarded.size).toBeNull(); + await expect(drain(guarded.body)).rejects.toThrow(/exceeded 4096 bytes/); + await vi.waitFor(() => expect(cancelled()).toBe(true)); + }); + + it('PROVIDER-HTTP-043: a body that lies about its length is held to the limit all the same', async () => { + const { stream } = source(1024, 64 * 1024); + const guarded = guardDownload(download(stream, { 'content-length': '100' }), GUARD); + await expect(drain(guarded.body)).rejects.toThrow(/exceeded/); + }); + + it('PROVIDER-HTTP-044: a consumer that stops reading releases the source', async () => { + const { stream, cancelled } = source(1024, 64 * 1024); + const guarded = guardDownload(download(stream), { ...GUARD, maxBytes: 1024 * 1024 }); + + guarded.body.destroy(); + await vi.waitFor(() => expect(cancelled()).toBe(true)); + }); + + it('PROVIDER-HTTP-045: an answer without a body is the client\'s own error', () => { + expect(() => guardDownload(download(null, { 'content-length': '10' }), GUARD)).toThrow('no body'); + }); +}); diff --git a/server/tests/unit/nest/doc-sync/synology-drive.provider.test.ts b/server/tests/unit/nest/doc-sync/synology-drive.provider.test.ts index 245596436..c31db0adc 100644 --- a/server/tests/unit/nest/doc-sync/synology-drive.provider.test.ts +++ b/server/tests/unit/nest/doc-sync/synology-drive.provider.test.ts @@ -1,6 +1,6 @@ /** - * SynologyDriveProvider — what it puts on the wire, and what it makes of what - * comes back. + * SynologyDriveDocumentProvider: what it puts on the wire, and what it makes + * of what comes back. * * FileStation is the provider the sync core's weakest-common-denominator model * was shaped around: no change feed, no stable file id, app-level errors inside @@ -13,7 +13,7 @@ * failure rather than a mock returning undefined on command, and the size cap * is the one that ships. */ -import { describe, it, expect, vi, beforeEach } from 'vitest'; +import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest'; import { createHash } from 'node:crypto'; import { Readable } from 'node:stream'; @@ -31,7 +31,7 @@ vi.mock('../../../../src/utils/ssrfGuard', () => ({ SsrfBlockedError: SsrfBlockedErrorMock, })); -import { SynologyDriveProvider } from '../../../../src/nest/doc-sync/providers/synology-drive.provider'; +import { SynologyDriveDocumentProvider } from '../../../../src/nest/doc-sync/providers/synology-drive.provider'; import { isValidSynoName, isWithinScope, @@ -240,7 +240,7 @@ function pushRoutes(path: string, size = BYTES.length, md5 = BYTES_MD5): Record< }; } -let provider: SynologyDriveProvider; +let provider: SynologyDriveDocumentProvider; beforeEach(() => { calls = []; @@ -250,12 +250,12 @@ beforeEach(() => { throw new Error('unstubbed safeFetch'); }); install(); - provider = new SynologyDriveProvider(); + provider = new SynologyDriveDocumentProvider(); }); // ── capabilities ───────────────────────────────────────────────────────────── -describe('SynologyDriveProvider — capabilities', () => { +describe('SynologyDriveDocumentProvider: capabilities', () => { it('SYNO-PROVIDER-001: promises nothing FileStation cannot do', () => { expect(provider.id).toBe('synologydrive'); expect(provider.capabilities()).toMatchObject({ @@ -280,7 +280,7 @@ describe('SynologyDriveProvider — capabilities', () => { // ── login and session ──────────────────────────────────────────────────────── -describe('SynologyDriveProvider — login', () => { +describe('SynologyDriveDocumentProvider: login', () => { it('SYNO-PROVIDER-010: logs in on auth.cgi with JSON-quoted values and asks for a CSRF token', async () => { route({ 'SYNO.FileStation.Info:get': ok({ hostname: 'nas' }) }); const result = await provider.probe(connection()); @@ -427,9 +427,133 @@ describe('SynologyDriveProvider — login', () => { }); }); +// ── device tokens ──────────────────────────────────────────────────────────── + +describe('SynologyDriveDocumentProvider: device tokens', () => { + /** DSM with two-factor on: a login needs a trusted device or a code, and a code earns a device. */ + function twoFactorNas(refused: string[] = []): Handler { + return (call) => { + const { device_id: deviceId, otp_code: otpCode } = call.params; + if (deviceId && refused.includes(deviceId)) return fail(403); + if (deviceId) return ok({ sid: `SID-${deviceId}` }); + if (otpCode) return ok({ sid: `SID-${otpCode}`, did: `DEVICE-${otpCode}` }); + return fail(403); + }; + } + + const withCode = (code: string) => ({ secrets: { password: 'nas-secret', otp_code: code } }); + const logins = () => callsTo('SYNO.API.Auth', 'login'); + + /** Push every cached session past its age, so the next call logs in again. */ + function expireSessions(): void { + vi.setSystemTime(Date.now() + 31 * 60 * 1000); + calls = []; + } + + beforeEach(() => { + vi.useFakeTimers(); + }); + + afterEach(() => { + vi.useRealTimers(); + }); + + it('SYNO-PROVIDER-025: another trip on the same account cannot log in on this trip\'s token', async () => { + route({ 'SYNO.API.Auth:login': twoFactorNas(), 'SYNO.FileStation.Info:get': ok({ hostname: 'nas' }) }); + expect(await provider.probe(connection(withCode('111111')))).toMatchObject({ success: true }); + + // Same NAS, same account, same password, a different trip and owner, and + // never a code from the authenticator. + const stranger = await provider.probe(connection({ connectionId: 4, ownerId: 12 })); + + expect(stranger).toMatchObject({ success: false, error: { code: 'unauthorized' } }); + expect(logins()[1].params.device_id).toBeUndefined(); + }); + + it('SYNO-PROVIDER-026: two connections to one account each log in again on their own token', async () => { + route({ 'SYNO.API.Auth:login': twoFactorNas(), 'SYNO.FileStation.Info:get': ok({ hostname: 'nas' }) }); + const first = connection(withCode('111111')); + const second = connection({ connectionId: 4, ownerId: 12, ...withCode('222222') }); + await provider.probe(first); + await provider.probe(second); + + expireSessions(); + await provider.probe(first); + await provider.probe(second); + + expect(logins().map((call) => call.params.device_id)).toEqual(['DEVICE-111111', 'DEVICE-222222']); + expect(logins().every((call) => call.params.otp_code === undefined)).toBe(true); + }); + + it('SYNO-PROVIDER-027: a token DSM stops trusting is dropped on that connection and no other', async () => { + route({ 'SYNO.API.Auth:login': twoFactorNas(), 'SYNO.FileStation.Info:get': ok({ hostname: 'nas' }) }); + const first = connection(withCode('111111')); + const second = connection({ connectionId: 4, ownerId: 12, ...withCode('222222') }); + await provider.probe(first); + await provider.probe(second); + + // Someone removed the first connection's device from DSM's trusted list. + route({ + 'SYNO.API.Auth:login': twoFactorNas(['DEVICE-111111']), + 'SYNO.FileStation.Info:get': ok({ hostname: 'nas' }), + }); + expireSessions(); + expect(await provider.probe(first)).toMatchObject({ success: false, error: { code: 'unauthorized' } }); + expect(await provider.probe(second)).toMatchObject({ success: true }); + + // Once the lockout has passed, the first connection offers the code in its + // form again instead of the token DSM refused. + vi.setSystemTime(Date.now() + 61 * 1000); + calls = []; + await provider.probe(first); + expect(logins()[0].params).toMatchObject({ otp_code: '111111', enable_device_token: 'yes' }); + expect(logins()[0].params.device_id).toBeUndefined(); + }); + + it('SYNO-PROVIDER-028: the token outlives a password edit and a stale code, but not a new account', async () => { + route({ 'SYNO.API.Auth:login': twoFactorNas(), 'SYNO.FileStation.Info:get': ok({ hostname: 'nas' }) }); + await provider.probe(connection(withCode('111111'))); + + await provider.probe(connection({ secrets: { password: 'rotated', otp_code: '111111' } })); + await provider.probe(connection({ secrets: { password: 'rotated' } })); + expect(logins().slice(1).map((call) => call.params.device_id)).toEqual(['DEVICE-111111', 'DEVICE-111111']); + + // The same connection repointed at another NAS, or at another account on + // this one, has passed nobody's second factor there. + await provider.probe(connection({ baseUrl: 'https://other-nas.example.org:5001' })); + await provider.probe(connection({ settings: { username: 'someone-else', base_path: '/trek' } })); + expect(logins().slice(3).map((call) => call.params.device_id)).toEqual([undefined, undefined]); + }); + + it('SYNO-PROVIDER-029: a code tested before the form was saved carries over to the saved connection', async () => { + route({ + 'SYNO.API.Auth:login': twoFactorNas(), + 'SYNO.FileStation.Info:get': ok({ hostname: 'nas' }), + 'SYNO.FileStation.List:list_share': ok({ shares: [] }), + 'SYNO.FileStation.List:list': ok({ total: 0, files: [] }), + }); + // Every unsaved form is connection 0, so what one of them earns must not + // be something the next unsaved form can use. + await provider.probe(connection({ connectionId: 0, ...withCode('111111') })); + expect(await provider.probe(connection({ connectionId: 0, ownerId: 12 }))).toMatchObject({ success: false }); + expect(logins()[1].params.device_id).toBeUndefined(); + + // Saved as connection 9: the scope picker runs on the session the test + // opened, and the next login after it expired needs no second code. + const saved = connection({ connectionId: 9, ...withCode('111111') }); + await provider.listScopes(saved); + expireSessions(); + await provider.listScopes(saved); + + expect(logins()).toHaveLength(1); + expect(logins()[0].params.device_id).toBe('DEVICE-111111'); + expect(logins()[0].params.otp_code).toBeUndefined(); + }); +}); + // ── scopes ─────────────────────────────────────────────────────────────────── -describe('SynologyDriveProvider — scopes', () => { +describe('SynologyDriveDocumentProvider: scopes', () => { it('SYNO-PROVIDER-030: offers the shares and the folders under the base path', async () => { route({ 'SYNO.FileStation.List:list_share': ok({ shares: [entry('/trek', { dir: true }), entry('/photo', { dir: true })] }), @@ -522,7 +646,7 @@ describe('SynologyDriveProvider — scopes', () => { // ── listing ────────────────────────────────────────────────────────────────── -describe('SynologyDriveProvider — list', () => { +describe('SynologyDriveDocumentProvider: list', () => { it('SYNO-PROVIDER-040: walks sub-folders, skips the bin and maps each file', async () => { const byFolder: Record = { [SCOPE]: [entry(`${SCOPE}/boarding.pdf`, { size: 12, mtime: 1_700_000_500 }), entry(`${SCOPE}/leg-2`, { dir: true }), entry(`${SCOPE}/.trek-trash`, { dir: true })], @@ -646,7 +770,7 @@ describe('SynologyDriveProvider — list', () => { // ── fetch ──────────────────────────────────────────────────────────────────── -describe('SynologyDriveProvider — fetch', () => { +describe('SynologyDriveDocumentProvider: fetch', () => { it('SYNO-PROVIDER-050: reads the version before the bytes and streams them', async () => { route({ 'SYNO.FileStation.List:getinfo': ok({ files: [entry(`${SCOPE}/a.pdf`, { size: BYTES.length, mtime: 1_700_000_300 })] }), @@ -700,11 +824,20 @@ describe('SynologyDriveProvider — fetch', () => { const result = await provider.fetch(connection(), scope(), `${SCOPE}/leg-2`); expect(result).toMatchObject({ success: false, error: { code: 'not_found' } }); }); + + it('SYNO-PROVIDER-055: a download that arrives without a body is a provider error, not an empty file', async () => { + route({ + 'SYNO.FileStation.List:getinfo': ok({ files: [entry(`${SCOPE}/a.pdf`)] }), + 'SYNO.FileStation.Download:download': reply(null, { raw: '', contentType: 'application/pdf' }), + }); + const result = await provider.fetch(connection(), scope(), `${SCOPE}/a.pdf`); + expect(result).toMatchObject({ success: false, error: { code: 'provider_error' } }); + }); }); // ── push ───────────────────────────────────────────────────────────────────── -describe('SynologyDriveProvider — push', () => { +describe('SynologyDriveDocumentProvider: push', () => { it('SYNO-PROVIDER-060: sends the fields first, the file last, and the mtime in milliseconds', async () => { route(pushRoutes(`${SCOPE}/itinerary.pdf`)); const result = await provider.push(connection(), scope(), push()); @@ -817,7 +950,7 @@ describe('SynologyDriveProvider — push', () => { // ── rename and trash ───────────────────────────────────────────────────────── -describe('SynologyDriveProvider — rename and trash', () => { +describe('SynologyDriveDocumentProvider: rename and trash', () => { it('SYNO-PROVIDER-070: rename sends the v2 array form and answers with the new version', async () => { route({ 'SYNO.FileStation.Rename:rename': ok({ files: [entry(`${SCOPE}/gate-b12.pdf`, { size: 7, mtime: 1_700_000_777 })] }), @@ -890,7 +1023,7 @@ describe('SynologyDriveProvider — rename and trash', () => { // ── error classification ───────────────────────────────────────────────────── -describe('SynologyDriveProvider — how a NAS disappoints', () => { +describe('SynologyDriveDocumentProvider: how a NAS disappoints', () => { it('SYNO-PROVIDER-080: 408 means "password expired" on auth.cgi and "no such file" on entry.cgi', async () => { route({ 'SYNO.API.Auth:login': fail(408) }); expect(await provider.resolveScope(connection(), scope())).toMatchObject({ @@ -898,7 +1031,7 @@ describe('SynologyDriveProvider — how a NAS disappoints', () => { error: { code: 'unauthorized' }, }); - provider = new SynologyDriveProvider(); + provider = new SynologyDriveDocumentProvider(); calls = []; route({ 'SYNO.FileStation.List:getinfo': fail(408) }); expect(await provider.resolveScope(connection(), scope())).toMatchObject({ @@ -919,7 +1052,7 @@ describe('SynologyDriveProvider — how a NAS disappoints', () => { [117, 'forbidden'], ]; for (const [synoCode, code] of expected) { - provider = new SynologyDriveProvider(); + provider = new SynologyDriveDocumentProvider(); calls = []; route({ 'SYNO.FileStation.List:list': sequence(fail(synoCode), fail(synoCode)) }); const result = await provider.list(connection(), scope()); @@ -974,7 +1107,7 @@ describe('SynologyDriveProvider — how a NAS disappoints', () => { it('SYNO-PROVIDER-088: the remaining HTTP statuses a NAS or its proxy answers', async () => { for (const [status, code] of [[403, 'forbidden'], [404, 'not_found'], [413, 'too_large'], [429, 'rate_limited']] as const) { - provider = new SynologyDriveProvider(); + provider = new SynologyDriveDocumentProvider(); calls = []; route({ 'SYNO.API.Auth:login': reply({}, { status }) }); expect(await provider.probe(connection()), `http ${status}`).toMatchObject({ @@ -993,9 +1126,19 @@ describe('SynologyDriveProvider — how a NAS disappoints', () => { }); it('SYNO-PROVIDER-090: an upload the NAS answers with a bare HTTP error keeps the status', async () => { + // 507 reads as a full volume, the same verdict DSM's own 415/416/1101 give. route({ 'SYNO.FileStation.Upload:upload': reply({}, { status: 507 }) }); - const result = await provider.push(connection(), scope(), push()); - expect(result).toMatchObject({ success: false, error: { code: 'provider_error', status: 507 } }); + expect(await provider.push(connection(), scope(), push())).toMatchObject({ + success: false, + error: { code: 'quota_exceeded', status: 507 }, + }); + + provider = new SynologyDriveDocumentProvider(); + route({ 'SYNO.FileStation.Upload:upload': reply({}, { status: 500 }) }); + expect(await provider.push(connection(), scope(), push())).toMatchObject({ + success: false, + error: { code: 'provider_error', status: 500 }, + }); }); it('SYNO-PROVIDER-091: an upload answered with HTML is a provider_error, not a parse crash', async () => { @@ -1011,21 +1154,21 @@ describe('SynologyDriveProvider — how a NAS disappoints', () => { error: { code: 'scope_missing' }, }); - provider = new SynologyDriveProvider(); + provider = new SynologyDriveDocumentProvider(); route({ 'SYNO.FileStation.CreateFolder:create': ok({ folders: [] }) }); expect(await provider.createScope(connection(), 'kyoto')).toMatchObject({ success: false, error: { code: 'provider_error' }, }); - provider = new SynologyDriveProvider(); + provider = new SynologyDriveDocumentProvider(); route({ 'SYNO.FileStation.Rename:rename': ok({ files: [] }) }); expect(await provider.rename(connection(), scope(), `${SCOPE}/a.pdf`, 'b.pdf')).toMatchObject({ success: false, error: { code: 'provider_error' }, }); - provider = new SynologyDriveProvider(); + provider = new SynologyDriveDocumentProvider(); route({ 'SYNO.FileStation.Upload:upload': ok({}), 'SYNO.FileStation.List:getinfo': ok({ files: [entry(`${SCOPE}/itinerary.pdf`)] }), diff --git a/server/tests/unit/nest/doc-sync/webdav.provider.test.ts b/server/tests/unit/nest/doc-sync/webdav.provider.test.ts index d59a33f54..4f286d8f8 100644 --- a/server/tests/unit/nest/doc-sync/webdav.provider.test.ts +++ b/server/tests/unit/nest/doc-sync/webdav.provider.test.ts @@ -29,7 +29,8 @@ vi.mock('../../../../src/utils/ssrfGuard', () => ({ }, })); -vi.mock('../../../../src/utils/cappedFetch', () => ({ +vi.mock('../../../../src/utils/cappedFetch', async (importOriginal) => ({ + ...(await importOriginal()), readCappedText: async (res: Response) => { const text = await res.text(); return { text: text.slice(0, capBytes), truncated: text.length > capBytes }; @@ -730,6 +731,24 @@ describe('fetch, rename and trash', () => { expect(requests('PROPFIND')).toHaveLength(1); }); + it('refuses a download announcing more than TREK transfers, and one that arrives without a body', async () => { + route(`PROPFIND ${NC_ROOT}/TREK/trip%2042`, folder()); + route( + `GET ${NC_ROOT}/TREK/trip%2042/pass%20scan.pdf`, + reply('x', { headers: { 'content-length': String(3 * 1024 ** 3) } }), + reply(null, { status: 200 }), + ); + + expect(await nextcloud.fetch(ncConn, ncScope, '61')).toMatchObject({ + success: false, + error: { code: 'too_large', status: 200 }, + }); + expect(await nextcloud.fetch(ncConn, ncScope, '61')).toMatchObject({ + success: false, + error: { code: 'provider_error', status: 200 }, + }); + }); + it('says not_found for an id the folder no longer holds', async () => { route(`PROPFIND ${NC_ROOT}/TREK/trip%2042`, folder());