diff --git a/TODO.md b/TODO.md index 9efc22a..f1e961d 100644 --- a/TODO.md +++ b/TODO.md @@ -20,6 +20,10 @@ downloads, cover it with mock-server tests, and update the README TODO checkbox. # Completed Steps +- 2026-08-09: Downloads verify the secretstream terminated on `TAG_FINAL` and + write output atomically: a truncated body is rejected instead of landing on + disk as a short file, and plaintext is staged in a sibling temp file and + renamed into place, so a failed download leaves the destination untouched. - 2026-07-07 Adopted scripts-to-rule-them-all: `script/` entrypoints, Makefile shims, README Entrypoints section - 2026-06-10: Decrypted collections shared by other users (sealed-box keys); diff --git a/src/crypto/index.ts b/src/crypto/index.ts index ef129f7..4e3e53b 100644 --- a/src/crypto/index.ts +++ b/src/crypto/index.ts @@ -14,5 +14,6 @@ export { pullStreamChunk, STREAM_CHUNK_OVERHEAD, STREAM_CHUNK_SIZE, + streamTagFinal, type StreamPullState, } from "./stream.js"; diff --git a/src/crypto/stream.ts b/src/crypto/stream.ts index f255c59..d2bde06 100644 --- a/src/crypto/stream.ts +++ b/src/crypto/stream.ts @@ -8,6 +8,27 @@ export const STREAM_CHUNK_SIZE = 4 * 1024 * 1024; // 16 bytes of Poly1305 tag plus 1 byte of secretstream tag. export const STREAM_CHUNK_OVERHEAD = 17; +// libsodium's crypto_secretstream_xchacha20poly1305_TAG_FINAL: the tag that +// marks the last chunk of a stream. Exported so callers (the download layer) +// can detect truncation without importing sodium themselves. +// +// This is a function rather than a constant, and that is load-bearing: +// libsodium attaches its constants to the module object inside +// `ready.then(...)`, which resolves long after this module is evaluated. A +// module-level read would bind `undefined`, every downstream comparison +// against it would then be false, and every valid download would be rejected +// as truncated. Reading at call time returns the library's own value, so +// there is also no second copy of a protocol constant to keep in sync. +// +// Under vitest sodium is already initialised in the worker before this module +// is evaluated, so an eager read would pick up a real value there and the +// ordinary tests could not tell the difference. The regression guard is +// "streamTagFinal() reads the constant at call time, not at import time" in +// test/crypto/stream.test.ts, which reproduces the plain-Node ESM ordering +// against a stand-in sodium module; it goes red if this becomes eager. +export const streamTagFinal = (): number => + sodium.crypto_secretstream_xchacha20poly1305_TAG_FINAL; + // Encrypt a small blob as a single secretstream chunk with TAG_FINAL. // Returns the header and ciphertext. Used for encrypting thumbnails // and metadata before upload. @@ -37,9 +58,6 @@ export const initStreamPull = ( ): StreamPullState => sodium.crypto_secretstream_xchacha20poly1305_init_pull(header, key); -// Decrypt one ciphertext chunk. Returns the plaintext and the secretstream -// tag (0=MESSAGE, 1=PUSH, 2=REKEY, 3=FINAL). The caller should verify the -// stream ended on TAG_FINAL to detect truncation. // Decrypt a small blob that was encrypted as a single secretstream chunk // with TAG_FINAL. Ente uses this form ("blob") for file metadata and // magic metadata — anything under ~1 MiB that isn't chunked. @@ -50,12 +68,18 @@ export const decryptBlob = ( ): Uint8Array => { const state = initStreamPull(header, key); const { plaintext, tag } = pullStreamChunk(state, ciphertext); - if (tag !== sodium.crypto_secretstream_xchacha20poly1305_TAG_FINAL) { - throw new Error(`decryptBlob: expected TAG_FINAL (3), got tag ${tag}`); + const tagFinal = streamTagFinal(); + if (tag !== tagFinal) { + throw new Error( + `decryptBlob: expected TAG_FINAL (${tagFinal}), got tag ${tag}`, + ); } return plaintext; }; +// Decrypt one ciphertext chunk. Returns the plaintext and the secretstream +// tag (0=MESSAGE, 1=PUSH, 2=REKEY, 3=FINAL). The caller must verify the +// stream ended on TAG_FINAL to detect truncation. export const pullStreamChunk = ( state: StreamPullState, ciphertext: Uint8Array, diff --git a/src/download/index.ts b/src/download/index.ts index 10610d7..6e7a6b1 100644 --- a/src/download/index.ts +++ b/src/download/index.ts @@ -1,10 +1,13 @@ -import { writeFile } from "node:fs/promises"; +import { randomUUID } from "node:crypto"; +import { rename, rm, writeFile } from "node:fs/promises"; +import { dirname, join } from "node:path"; import { fromBase64, initStreamPull, pullStreamChunk, STREAM_CHUNK_OVERHEAD, STREAM_CHUNK_SIZE, + streamTagFinal, } from "../crypto/index.js"; import type { ApiClient } from "../api/client.js"; import type { EnteFile } from "../model/types.js"; @@ -26,6 +29,8 @@ const streamDecrypt = async ( let buffer = new Uint8Array(0); const plainChunks: Uint8Array[] = []; let totalPlain = 0; + let chunksPulled = 0; + let lastTag = -1; for (;;) { const { done, value } = await reader.read(); @@ -39,21 +44,58 @@ const streamDecrypt = async ( while (buffer.length >= ENC_CHUNK_SIZE) { const encChunk = buffer.slice(0, ENC_CHUNK_SIZE); buffer = buffer.slice(ENC_CHUNK_SIZE); - const { plaintext } = pullStreamChunk(state, encChunk); + const { plaintext, tag } = pullStreamChunk(state, encChunk); plainChunks.push(plaintext); totalPlain += plaintext.length; + chunksPulled++; + lastTag = tag; } if (done) { if (buffer.length > 0) { - const { plaintext } = pullStreamChunk(state, buffer); - plainChunks.push(plaintext); - totalPlain += plaintext.length; + // Whatever is left over once every whole chunk has been + // consumed must be the stream's final chunk, and a final + // chunk that actually arrived in full authenticates. If it + // does not, the body stopped part-way through a chunk — the + // ordinary shape of a dropped connection. Poly1305 cannot + // tell a partial chunk from a corrupt one, so this is + // reported as the truncation it almost always is, with the + // authentication failure kept as the error's cause. + let pulled; + try { + pulled = pullStreamChunk(state, buffer); + } catch (err) { + throw new Error( + `download: stream truncated: response body ended with ${buffer.length} trailing bytes that did not authenticate as a final chunk (transfer stopped mid-chunk, or the data is corrupt)`, + { cause: err }, + ); + } + plainChunks.push(pulled.plaintext); + totalPlain += pulled.plaintext.length; + chunksPulled++; + lastTag = pulled.tag; } break; } } + // Only the last chunk of a secretstream carries TAG_FINAL. Everything a + // dropped connection did deliver still decrypts and authenticates, so the + // absence of TAG_FINAL is the only evidence that the body was cut short. + // Returning a short plaintext here would put a corrupt file on disk that + // later backup runs would treat as complete. + if (chunksPulled === 0) { + throw new Error( + "download: stream truncated: response body contained no secretstream chunks", + ); + } + const tagFinal = streamTagFinal(); + if (lastTag !== tagFinal) { + throw new Error( + `download: stream truncated: last chunk tag ${lastTag}, expected TAG_FINAL (${tagFinal})`, + ); + } + const result = new Uint8Array(totalPlain); let offset = 0; for (const chunk of plainChunks) { @@ -63,6 +105,29 @@ const streamDecrypt = async ( return result; }; +// Write `plaintext` to `destination` atomically: stage it in a temporary +// sibling file (same directory, so the rename cannot cross a filesystem +// boundary) and rename it into place. Callers therefore never observe a +// partially written destination, and a pre-existing file at that path is +// replaced only once the new contents are complete on disk. +const writeAtomic = async ( + destination: string, + plaintext: Uint8Array, +): Promise => { + // The random suffix keeps concurrent downloads of the same destination + // from stepping on each other's temporary file. + const tmpPath = join(dirname(destination), `.quak-${randomUUID()}.tmp`); + try { + await writeFile(tmpPath, plaintext); + await rename(tmpPath, destination); + } catch (err) { + // Best-effort cleanup. A failure to remove the temporary file must + // never replace the error that actually explains what went wrong. + await rm(tmpPath, { force: true }).catch(() => undefined); + throw err; + } +}; + export const downloadFile = async ( api: ApiClient, file: EnteFile, @@ -72,7 +137,7 @@ export const downloadFile = async ( const stream = await api.getFileStream(file.id); const header = fromBase64(file.file.decryptionHeader); const plaintext = await streamDecrypt(stream, header, file.key); - await writeFile(resolvedPath, plaintext); + await writeAtomic(resolvedPath, plaintext); return { path: resolvedPath, bytesWritten: plaintext.length }; }; @@ -85,6 +150,6 @@ export const downloadThumbnail = async ( const stream = await api.getThumbnailStream(file.id); const header = fromBase64(file.thumbnail.decryptionHeader); const plaintext = await streamDecrypt(stream, header, file.key); - await writeFile(resolvedPath, plaintext); + await writeAtomic(resolvedPath, plaintext); return { path: resolvedPath, bytesWritten: plaintext.length }; }; diff --git a/test/crypto/stream.test.ts b/test/crypto/stream.test.ts index db67bc6..331317f 100644 --- a/test/crypto/stream.test.ts +++ b/test/crypto/stream.test.ts @@ -15,7 +15,12 @@ * stream ended on a `TAG_FINAL` chunk and was therefore not truncated. * * These tests pin: - * - The chunk-size constants match Ente's expectations. + * - The chunk-size constants match Ente's expectations, and the + * re-exported `streamTagFinal()` matches the tag a real final chunk + * carries. + * - `streamTagFinal()` reads libsodium's constant at call time rather than + * at import time, which it must, because the constant does not exist yet + * when this library's modules are evaluated. * - The pull state can decrypt a multi-chunk stream produced by * sodium.crypto_secretstream_xchacha20poly1305_push, in order. * - The tag byte is propagated to the caller. @@ -23,13 +28,14 @@ */ import sodium from "libsodium-wrappers-sumo"; -import { beforeAll, describe, expect, it } from "vitest"; +import { beforeAll, describe, expect, it, vi } from "vitest"; import { init, initStreamPull, pullStreamChunk, STREAM_CHUNK_OVERHEAD, STREAM_CHUNK_SIZE, + streamTagFinal, } from "../../src/crypto/index.js"; describe("crypto stream constants", () => { @@ -45,6 +51,85 @@ describe("crypto stream constants", () => { it("STREAM_CHUNK_OVERHEAD is 17 bytes", () => { expect(STREAM_CHUNK_OVERHEAD).toBe(17); }); + + /** + * `streamTagFinal()` is re-exported so callers can detect a truncated + * stream (a body that ended on a non-final chunk) without importing + * libsodium themselves. + * + * It is a function, not a constant, and that is load-bearing: libsodium + * attaches its own constants to the module object only after + * `sodium.ready` resolves, which is long after this library's modules are + * evaluated. Reading the value at call time yields libsodium's number; + * reading it at module scope would yield `undefined`, and every + * truncation check downstream would then compare against `undefined` and + * reject good downloads. The eagerness itself is guarded by the next test; + * this one pins the value, comparing it against the tag observed on a real + * final chunk pulled back off the wire format. + */ + it("streamTagFinal() is the tag carried by a real final chunk", async () => { + await init(); + await sodium.ready; + + const key = sodium.crypto_secretstream_xchacha20poly1305_keygen(); + const push = + sodium.crypto_secretstream_xchacha20poly1305_init_push(key); + const ciphertext = sodium.crypto_secretstream_xchacha20poly1305_push( + push.state, + new TextEncoder().encode("last chunk"), + null, + sodium.crypto_secretstream_xchacha20poly1305_TAG_FINAL, + ); + + const state = initStreamPull(push.header, key); + expect(pullStreamChunk(state, ciphertext).tag).toBe(streamTagFinal()); + }); + + /** + * The regression guard for the eagerness property described above. + * + * Under vitest, sodium is already initialised in the worker process by + * the time any source module is evaluated, so an eager module-level read + * would happen to pick up a real value and no ordinary test could tell + * the difference. This test recreates the ordering that a plain Node ESM + * consumer sees: a stand-in sodium module whose `TAG_FINAL` property does + * not exist yet when `src/crypto/stream.ts` is evaluated and only appears + * afterwards, exactly as libsodium attaches its constants inside + * `ready.then(...)`. + * + * A call-time read observes the value that appeared after evaluation; a + * module-level read binds `undefined` and this test fails. The stand-in + * uses a sentinel rather than the real tag number so that a read which + * somehow reached the real libsodium would fail too. + */ + it("streamTagFinal() reads the constant at call time, not at import time", async () => { + const SENTINEL = 42; + const late: { tagFinal: number | undefined } = { tagFinal: undefined }; + + vi.resetModules(); + vi.doMock("libsodium-wrappers-sumo", () => ({ + default: { + ready: Promise.resolve(), + get crypto_secretstream_xchacha20poly1305_TAG_FINAL() { + return late.tagFinal; + }, + }, + })); + + try { + // Evaluated while the constant is still absent, as it is before + // `sodium.ready` resolves. + const fresh = await import("../../src/crypto/stream.js"); + expect(late.tagFinal).toBeUndefined(); + + // libsodium attaches its constants; a lazy accessor sees them. + late.tagFinal = SENTINEL; + expect(fresh.streamTagFinal()).toBe(SENTINEL); + } finally { + vi.doUnmock("libsodium-wrappers-sumo"); + vi.resetModules(); + } + }); }); describe("crypto.initStreamPull / pullStreamChunk", () => { diff --git a/test/download/download.test.ts b/test/download/download.test.ts index 1bcd625..db50a9d 100644 --- a/test/download/download.test.ts +++ b/test/download/download.test.ts @@ -9,20 +9,57 @@ * secretstream ciphertext chunks. Each chunk is at most * `STREAM_CHUNK_SIZE + STREAM_CHUNK_OVERHEAD` bytes (4 MiB + 17 bytes). * The download function buffers incoming network data, splits it on the - * chunk boundary, and feeds each piece to `pullStreamChunk`. The last - * chunk carries `TAG_FINAL`; any truncation is detected because the tag - * will be missing. + * chunk boundary, and feeds each piece to `pullStreamChunk`. + * + * Two contracts are load-bearing for anyone using this library as a backup + * tool, and both are documented by the tests below: + * + * 1. **Truncation is an error, never a short file.** Only the final chunk of + * a secretstream carries `TAG_FINAL`. A download cut short by a dropped + * connection still decrypts cleanly up to the last whole chunk, so without + * an explicit `TAG_FINAL` check a truncated body is indistinguishable from + * a complete one. `streamDecrypt` therefore refuses to return unless the + * stream ended on `TAG_FINAL`, and the error says the stream was truncated. + * A transfer that stopped part-way through a chunk is reported the same + * way, since a final chunk that arrived in full always authenticates. + * + * 2. **The destination path is written atomically.** Plaintext goes to a + * temporary sibling file first and is `rename`d into place only after the + * whole stream has decrypted and verified. A caller that sees no exception + * can rely on the destination containing the complete, authenticated file; + * a caller that sees an exception can rely on the destination being + * untouched — whatever was there before is still there, byte for byte, and + * no partial file has appeared. This matters because `runBackup` skips any + * existing non-empty file, so a partial write would be treated as complete + * forever after. The staging file and the rename are observed directly (see + * the `rename` hook below), not inferred from an empty directory. * * These tests build synthetic encrypted files using sodium's push API, * serve them from a mock fetch, and verify the decrypted output on disk. */ -import { existsSync, readFileSync, rmSync, mkdtempSync } from "node:fs"; -import { join } from "node:path"; +import { + existsSync, + readdirSync, + readFileSync, + rmSync, + mkdtempSync, + writeFileSync, +} from "node:fs"; +import { dirname, join } from "node:path"; import { tmpdir } from "node:os"; +import { createHash } from "node:crypto"; import sodium from "libsodium-wrappers-sumo"; -import { beforeAll, afterAll, describe, expect, it } from "vitest"; -import { init, toBase64 } from "../../src/crypto/index.js"; +import { + beforeAll, + beforeEach, + afterAll, + describe, + expect, + it, + vi, +} from "vitest"; +import { init, toBase64, STREAM_CHUNK_SIZE } from "../../src/crypto/index.js"; import { ApiClient } from "../../src/api/client.js"; import { downloadFile, downloadThumbnail } from "../../src/download/index.js"; import type { EnteFile, FileMetadata } from "../../src/model/types.js"; @@ -31,6 +68,52 @@ import type { EnteFile, FileMetadata } from "../../src/model/types.js"; // Test helpers // --------------------------------------------------------------------------- +/** + * `rename` is intercepted so that the tests can observe — and fail — the + * final step of the atomic write. + * + * Every other failure in this file is injected inside `streamDecrypt`, which + * runs before anything is written to disk. Those tests therefore cannot tell + * an atomic write from a plain `writeFile`: under both, nothing was ever + * created, so an empty directory proves nothing about cleanup. This hook is + * what closes that gap. It records each rename the downloader performs, + * including whether the source existed at that moment (i.e. that the staged + * temp file really was written), and can be told to fail the rename so the + * cleanup path runs with a temp file genuinely on disk. + * + * `vi.hoisted` is required: `vi.mock` factories are hoisted above the imports, + * so a plain module-level `const` would still be in its temporal dead zone by + * the time the factory runs. + */ +const renameHook = vi.hoisted(() => ({ + calls: [] as { from: string; to: string; sourceExisted: boolean }[], + failWith: null as Error | null, +})); + +vi.mock("node:fs/promises", async (importOriginal) => { + const actual = await importOriginal(); + const { existsSync: sourceExists } = await import("node:fs"); + return { + ...actual, + rename: async (from: string, to: string): Promise => { + renameHook.calls.push({ + from, + to, + sourceExisted: sourceExists(from), + }); + if (renameHook.failWith !== null) { + throw renameHook.failWith; + } + await actual.rename(from, to); + }, + }; +}); + +beforeEach(() => { + renameHook.calls.length = 0; + renameHook.failWith = null; +}); + let testDir: string; beforeAll(async () => { @@ -45,6 +128,35 @@ afterAll(() => { } }); +/** + * Deterministic stand-in for random test payloads. + * + * Fixture *content* is never load-bearing here — the assertions turn on + * length, framing, and the secretstream tag — but it must not be a constant + * fill either, or a downloader that reordered or repeated chunks would still + * produce the expected bytes. A seeded linear congruential generator gives + * both: byte patterns that differ across every offset and seed, reproducible + * on any machine, which is what the README asks of fixtures. + * + * It is also the difference between a fast suite and a broken one. + * `sodium.randombytes_buf` goes through the wasm wrapper a byte at a time and + * costs roughly 20 seconds for the 4 MiB chunk below — about two hundred + * times what it costs to encrypt the same buffer, and on its own enough to + * push `make test` past the 30-second cap in `script/test`. This loop fills + * 4 MiB in a few milliseconds. + */ +const patternBytes = (length: number, seed: number): Uint8Array => { + const out = new Uint8Array(length); + let x = seed >>> 0; + for (let i = 0; i < length; i++) { + // Numerical Recipes' LCG constants; the high byte is used because + // the low bits of an LCG have short periods. + x = (Math.imul(x, 1664525) + 1013904223) >>> 0; + out[i] = (x >>> 24) & 0xff; + } + return out; +}; + /** * Encrypt `plaintext` as a secretstream file body (single chunk with * TAG_FINAL). Returns the key, header, and ciphertext that the mock CDN @@ -64,6 +176,79 @@ const encryptFileBody = ( return { header: push.header, ciphertext }; }; +/** + * Encrypt a body that spans more than one secretstream chunk, the way the + * server does for files larger than the 4 MiB plaintext chunk size. + * + * Framing matters here: the downloader splits the byte stream on fixed + * `STREAM_CHUNK_SIZE + STREAM_CHUNK_OVERHEAD` boundaries, so every chunk + * except the last must carry exactly `STREAM_CHUNK_SIZE` plaintext bytes. + * Only the last chunk is tagged `TAG_FINAL`; the leading ones are + * `TAG_MESSAGE`. + * + * Returns the header, the concatenated body, the plaintext it decrypts to, + * and `finalChunkOffset` — the byte offset at which the `TAG_FINAL` chunk + * begins, so a test can slice it off to simulate a connection that dropped + * before the end of the file. + */ +const encryptMultiChunkBody = ( + key: Uint8Array, + leadingChunks: number, + finalChunkPlainSize: number, +): { + header: Uint8Array; + body: Uint8Array; + plaintext: Uint8Array; + finalChunkOffset: number; +} => { + const push = sodium.crypto_secretstream_xchacha20poly1305_init_push(key); + const cipherParts: Uint8Array[] = []; + const plainParts: Uint8Array[] = []; + + for (let i = 0; i < leadingChunks; i++) { + const plain = patternBytes(STREAM_CHUNK_SIZE, i + 1); + plainParts.push(plain); + cipherParts.push( + sodium.crypto_secretstream_xchacha20poly1305_push( + push.state, + plain, + null, + sodium.crypto_secretstream_xchacha20poly1305_TAG_MESSAGE, + ), + ); + } + + const finalPlain = patternBytes(finalChunkPlainSize, leadingChunks + 1); + plainParts.push(finalPlain); + const finalCipher = sodium.crypto_secretstream_xchacha20poly1305_push( + push.state, + finalPlain, + null, + sodium.crypto_secretstream_xchacha20poly1305_TAG_FINAL, + ); + + const finalChunkOffset = cipherParts.reduce((n, c) => n + c.length, 0); + cipherParts.push(finalCipher); + + return { + header: push.header, + body: concat(cipherParts), + plaintext: concat(plainParts), + finalChunkOffset, + }; +}; + +const concat = (parts: Uint8Array[]): Uint8Array => { + const total = parts.reduce((n, p) => n + p.length, 0); + const out = new Uint8Array(total); + let offset = 0; + for (const p of parts) { + out.set(p, offset); + offset += p.length; + } + return out; +}; + const buildMockEnteFile = ( key: Uint8Array, fileHeader: Uint8Array, @@ -90,6 +275,79 @@ const mockFetchForBody = (body: Uint8Array) => { return fake as typeof globalThis.fetch; }; +/** + * Encrypt a body consisting of one chunk that is *not* tagged TAG_FINAL. + * + * This is the cheap way to present a stream that ended without its final + * chunk: the downloader pulls it, authenticates it, and finds the stream + * over on a TAG_MESSAGE chunk — the same terminal condition as a large file + * whose last chunk was lost, without paying for a 4 MiB fixture. The + * multi-chunk fixture above covers the realistic wire shape; this one is + * used where the test is really about what happens on disk afterwards. + */ +const encryptNonFinalBody = ( + plaintext: Uint8Array, + key: Uint8Array, +): { header: Uint8Array; ciphertext: Uint8Array } => { + const push = sodium.crypto_secretstream_xchacha20poly1305_init_push(key); + const ciphertext = sodium.crypto_secretstream_xchacha20poly1305_push( + push.state, + plaintext, + null, + sodium.crypto_secretstream_xchacha20poly1305_TAG_MESSAGE, + ); + return { header: push.header, ciphertext }; +}; + +/** + * Compare file contents by digest rather than with `toEqual`. Vitest's deep + * equality walks multi-megabyte buffers byte by byte, which costs seconds on + * the 4 MiB fixtures; a digest comparison is exact and effectively free. + */ +const expectSameBytes = (actual: Uint8Array, expected: Uint8Array): void => { + expect(actual.length).toBe(expected.length); + expect(createHash("sha256").update(actual).digest("hex")).toBe( + createHash("sha256").update(expected).digest("hex"), + ); +}; + +/** + * A multi-chunk fixture shared by the truncation tests: one full 4 MiB + * `TAG_MESSAGE` chunk followed by a small `TAG_FINAL` chunk. It is built once + * and shared because encrypting 4 MiB costs about 100ms. Its plaintext is + * generated rather than drawn from the CSPRNG, which is what keeps that + * encryption the whole cost of the fixture. + */ +let multiChunk: ReturnType; +let multiChunkKey: Uint8Array; + +beforeAll(() => { + multiChunkKey = sodium.crypto_secretstream_xchacha20poly1305_keygen(); + multiChunk = encryptMultiChunkBody(multiChunkKey, 1, 1024); +}); + +/** + * Build an EnteFile plus ApiClient whose file *and* thumbnail streams both + * serve `body` under `header`. The download path under test is otherwise + * identical for the two, so every truncation/atomicity case below runs + * against both entry points from a single fixture. + */ +const fixtureFor = ( + key: Uint8Array, + header: Uint8Array, + body: Uint8Array, +): { api: ApiClient; file: EnteFile } => ({ + api: new ApiClient({ fetch: mockFetchForBody(body) }), + file: buildMockEnteFile(key, header, header), +}); + +// The two entry points share `streamDecrypt` and the atomic-write wrapper, +// so the contract tests are written once and run against both. +const entryPoints = [ + { name: "downloadFile", download: downloadFile }, + { name: "downloadThumbnail", download: downloadThumbnail }, +]; + // --------------------------------------------------------------------------- // Tests // --------------------------------------------------------------------------- @@ -113,35 +371,45 @@ describe("downloadFile", () => { const result = await downloadFile(api, file, outPath); - expect(result.path).toBe(outPath); - expect(result.bytesWritten).toBe(plaintext.length); + // The whole DownloadResult shape is asserted, not just its fields: + // callers depend on `path` being the destination they asked for + // (never the temporary file used along the way) and on + // `bytesWritten` being the plaintext length. + expect(result).toEqual({ + path: outPath, + bytesWritten: plaintext.length, + }); expect(readFileSync(outPath)).toEqual(Buffer.from(plaintext)); }); it("uses metadata.title as filename when outPath is omitted", async () => { + // With no `outPath`, the destination is `metadata.title`, used + // verbatim as a path. The title here is therefore given inside the + // test's temporary directory: a bare relative name would resolve + // against the process working directory, i.e. the repo root, and + // `make check` must not create files in the repo — a failure between + // the write and any cleanup would leave one behind. const plaintext = new Uint8Array([1, 2, 3]); const key = sodium.crypto_secretstream_xchacha20poly1305_keygen(); const { header, ciphertext } = encryptFileBody(plaintext, key); const thumbPush = sodium.crypto_secretstream_xchacha20poly1305_init_push(key); const file = buildMockEnteFile(key, header, thumbPush.header); - file.metadata.title = "fallback-name.png"; + const titlePath = join(testDir, "fallback-name.png"); + file.metadata.title = titlePath; const api = new ApiClient({ fetch: mockFetchForBody(ciphertext) }); const result = await downloadFile(api, file); - expect(result.path).toBe("fallback-name.png"); - // Clean up since it writes to cwd - if (existsSync(result.path)) rmSync(result.path); + expect(result.path).toBe(titlePath); + expect(readFileSync(result.path)).toEqual(Buffer.from(plaintext)); }); it("handles a larger single-chunk file (random binary payload)", async () => { // Most photos are under 4 MiB and therefore a single secretstream // chunk. This test exercises a non-trivial payload size with - // random binary data (not just ASCII) to verify no encoding bugs. - // Multi-chunk (>4 MiB) decryption is verified by the live - // integration test against real photos from the dev account. - const plaintext = sodium.randombytes_buf(100_000); + // arbitrary binary data (not just ASCII) to verify no encoding bugs. + const plaintext = patternBytes(100_000, 11); const key = sodium.crypto_secretstream_xchacha20poly1305_keygen(); const { header, ciphertext } = encryptFileBody(plaintext, key); const thumbPush = @@ -153,7 +421,27 @@ describe("downloadFile", () => { const result = await downloadFile(api, file, outPath); expect(result.bytesWritten).toBe(100_000); - expect(readFileSync(outPath)).toEqual(Buffer.from(plaintext)); + expectSameBytes(readFileSync(outPath), plaintext); + }); + + it("decrypts a body that spans several secretstream chunks", async () => { + // Files over 4 MiB arrive as several ciphertext chunks concatenated + // into one HTTP body. The downloader has to re-split them on the + // exact chunk boundary; getting that wrong corrupts every large + // photo in an account. This is also the positive control for the + // truncation tests below: it proves the multi-chunk fixture itself + // decrypts cleanly when nothing has been removed from it. + const { api, file } = fixtureFor( + multiChunkKey, + multiChunk.header, + multiChunk.body, + ); + const outPath = join(testDir, "multi-chunk.bin"); + + const result = await downloadFile(api, file, outPath); + + expect(result.bytesWritten).toBe(multiChunk.plaintext.length); + expectSameBytes(readFileSync(outPath), multiChunk.plaintext); }); }); @@ -173,7 +461,297 @@ describe("downloadThumbnail", () => { const result = await downloadThumbnail(api, file, outPath); - expect(result.bytesWritten).toBe(4); + expect(result).toEqual({ path: outPath, bytesWritten: 4 }); expect(readFileSync(outPath)).toEqual(Buffer.from(plaintext)); }); }); + +// --------------------------------------------------------------------------- +// Truncation detection and atomic writes +// +// Everything below is the failure contract. It is deliberately written once +// per entry point via `entryPoints`, because `downloadFile` and +// `downloadThumbnail` must behave identically here: a corrupt thumbnail is +// just as unacceptable as a corrupt original, and `runBackup` trusts both. +// --------------------------------------------------------------------------- + +describe.each(entryPoints)( + "$name truncation handling", + ({ name, download }) => { + /** A fresh, empty directory so leftover-file assertions are meaningful. */ + const freshDir = (): string => { + const dir = mkdtempSync(join(testDir, `${name}-`)); + return dir; + }; + + it("rejects a body whose final TAG_FINAL chunk never arrived", async () => { + // Simulate a connection that dropped after the first 4 MiB chunk. + // Every byte that did arrive decrypts and authenticates perfectly — + // that is precisely the danger. The only signal that the file is + // incomplete is the absence of a chunk tagged TAG_FINAL, so the + // downloader must treat "stream ended on TAG_MESSAGE" as a hard + // error rather than returning a short file. + const truncatedBody = multiChunk.body.slice( + 0, + multiChunk.finalChunkOffset, + ); + const { api, file } = fixtureFor( + multiChunkKey, + multiChunk.header, + truncatedBody, + ); + const outPath = join(freshDir(), "truncated.bin"); + + await expect(download(api, file, outPath)).rejects.toThrow( + /truncated/i, + ); + }); + + it("rejects a body whose final chunk arrived only in part", async () => { + // The likelier shape of a dropped connection: the transfer stops + // in the middle of a chunk rather than neatly between two. The + // bytes that arrived are a prefix of a complete chunk, so Poly1305 + // rejects them — which is, cryptographically, indistinguishable + // from corruption of a whole chunk. + // + // It is still reported as truncation, because that is what it + // almost always is and because this library's entire reason for + // checking TAG_FINAL is to make a short transfer visible. Calling + // a short transfer "authentication failed" would send a user + // hunting for a corrupt file when their network is at fault. The + // underlying authentication failure is kept as the error's + // `cause`, so the real diagnosis is never lost. + const shortBody = multiChunk.body.slice( + 0, + multiChunk.body.length - 8, + ); + const { api, file } = fixtureFor( + multiChunkKey, + multiChunk.header, + shortBody, + ); + const dir = freshDir(); + const outPath = join(dir, "partial-final.bin"); + + const err = await download(api, file, outPath).catch( + (e: unknown) => e, + ); + + expect(err).toBeInstanceOf(Error); + expect((err as Error).message).toMatch(/truncated/i); + expect((err as Error).cause).toBeInstanceOf(Error); + expect(((err as Error).cause as Error).message).toMatch( + /authentication failed/i, + ); + + // And, as with every other failure, the destination is untouched + // and no staged temp file survives. + expect(existsSync(outPath)).toBe(false); + expect(readdirSync(dir)).toEqual([]); + }); + + it("rejects an empty body instead of writing a zero-byte file", async () => { + // Ente always emits at least one chunk, even for empty content: + // `encryptBlob` shows that a zero-length plaintext still produces a + // TAG_FINAL chunk. A body with no chunks at all therefore means the + // transfer failed, not that the file is empty. Writing a zero-byte + // file here would be the worst outcome, because `runBackup` would + // then see a file it considers present and never retry it. + const { api, file } = fixtureFor( + multiChunkKey, + multiChunk.header, + new Uint8Array(0), + ); + const outPath = join(freshDir(), "empty.bin"); + + await expect(download(api, file, outPath)).rejects.toThrow( + /truncated/i, + ); + }); + + it("leaves no file at the destination after a truncated download", async () => { + // The caller's contract: if the promise rejects, the destination + // path does not exist. Nothing downstream should ever have to guess + // whether a leftover file is complete. + // + // The body here is a single chunk that was never tagged TAG_FINAL, + // which puts the downloader in exactly the terminal state a lost + // last chunk produces, without the cost of a 4 MiB fixture. What + // this test is really about is the state of the filesystem after + // the rejection. + const key = sodium.crypto_secretstream_xchacha20poly1305_keygen(); + const { header, ciphertext } = encryptNonFinalBody( + patternBytes(256, 21), + key, + ); + const { api, file } = fixtureFor(key, header, ciphertext); + const dir = freshDir(); + const outPath = join(dir, "absent.bin"); + + await expect(download(api, file, outPath)).rejects.toThrow( + /truncated/i, + ); + + expect(existsSync(outPath)).toBe(false); + // And no temporary scratch file is left behind either: the download + // stages plaintext in a sibling temp file, which must be removed on + // the failure path so repeated failures cannot fill the disk. + expect(readdirSync(dir)).toEqual([]); + }); + + it("reports a corrupt whole chunk as an authentication failure, not truncation", async () => { + // The counterpart to the partial-final-chunk case above, and the + // reason the two are distinguishable at all. A byte is flipped + // inside the first chunk of a multi-chunk body: that chunk arrives + // complete — the stream goes on past it — so its failure cannot be + // a short transfer. It is corruption, and the caller is told so, + // with `pullStreamChunk`'s error propagated unchanged because it is + // the real diagnosis. + // + // The same on-disk guarantee holds for this failure mode as for + // every other: nothing at the destination, nothing left over. + const corrupted = Uint8Array.from(multiChunk.body); + corrupted[10] ^= 0xff; + + const { api, file } = fixtureFor( + multiChunkKey, + multiChunk.header, + corrupted, + ); + const dir = freshDir(); + const outPath = join(dir, "corrupt.bin"); + + await expect(download(api, file, outPath)).rejects.toThrow( + /authentication failed/i, + ); + + expect(existsSync(outPath)).toBe(false); + expect(readdirSync(dir)).toEqual([]); + }); + + it("does not clobber an existing file when the download fails", async () => { + // The repair case. A user re-running a backup over a directory that + // already holds good originals must never end up worse off: a failed + // download leaves the previous contents exactly as they were, so the + // old good copy survives until a complete new one is available to + // replace it in a single rename. + const existing = new TextEncoder().encode( + "previously downloaded, known-good contents", + ); + const key = sodium.crypto_secretstream_xchacha20poly1305_keygen(); + const { header, ciphertext } = encryptNonFinalBody( + patternBytes(256, 22), + key, + ); + const { api, file } = fixtureFor(key, header, ciphertext); + const dir = freshDir(); + const outPath = join(dir, "existing.bin"); + writeFileSync(outPath, existing); + + await expect(download(api, file, outPath)).rejects.toThrow( + /truncated/i, + ); + + expect(readFileSync(outPath)).toEqual(Buffer.from(existing)); + expect(readdirSync(dir)).toEqual(["existing.bin"]); + }); + + it("replaces an existing file when the download succeeds", async () => { + // The mirror image of the previous test: a complete download does + // overwrite whatever was at the destination, atomically, via rename. + const existing = new TextEncoder().encode("stale contents"); + const plaintext = patternBytes(512, 23); + const key = sodium.crypto_secretstream_xchacha20poly1305_keygen(); + const { header, ciphertext } = encryptFileBody(plaintext, key); + + const { api, file } = fixtureFor(key, header, ciphertext); + const dir = freshDir(); + const outPath = join(dir, "replaced.bin"); + writeFileSync(outPath, existing); + + const result = await download(api, file, outPath); + + expect(result).toEqual({ path: outPath, bytesWritten: 512 }); + expect(readFileSync(outPath)).toEqual(Buffer.from(plaintext)); + // The temp file is gone once the rename has happened, so a + // successful download leaves exactly one file behind. + expect(readdirSync(dir)).toEqual(["replaced.bin"]); + }); + + it("stages the plaintext in a sibling temp file and renames it into place", async () => { + // The atomic write, observed directly rather than inferred from an + // empty directory. Every other failure in this file is injected + // inside `streamDecrypt`, which runs before anything is written — + // so under those tests a plain `writeFile` to the destination would + // look identical. This one watches the rename itself. + // + // Two properties are load-bearing. The staging file must exist on + // disk when the rename happens: that is what makes the destination + // appear complete or not at all, instead of filling up as bytes + // land. And it must be a sibling of the destination, because + // `rename` is only atomic within one filesystem — staging in + // `/tmp` and renaming across a mount point would silently become a + // copy, reintroducing the partial-file window this exists to close. + const plaintext = patternBytes(512, 31); + const key = sodium.crypto_secretstream_xchacha20poly1305_keygen(); + const { header, ciphertext } = encryptFileBody(plaintext, key); + const { api, file } = fixtureFor(key, header, ciphertext); + const dir = freshDir(); + const outPath = join(dir, "staged.bin"); + + await download(api, file, outPath); + + expect(renameHook.calls).toHaveLength(1); + const staged = renameHook.calls[0]!; + expect(staged.to).toBe(outPath); + expect(staged.from).not.toBe(outPath); + expect(dirname(staged.from)).toBe(dir); + expect(staged.sourceExisted).toBe(true); + + // Afterwards the temp file is gone and only the destination is + // left, holding the complete plaintext. + expect(existsSync(staged.from)).toBe(false); + expect(readFileSync(outPath)).toEqual(Buffer.from(plaintext)); + expect(readdirSync(dir)).toEqual(["staged.bin"]); + }); + + it("removes the staged temp file when the rename itself fails", async () => { + // The cleanup path. It can only run when something fails at or + // after the write, which no amount of bad network data can + // produce: by the time anything is written the whole stream has + // already decrypted and verified. Failing the rename is what + // reaches it — a real possibility on a full disk, a read-only + // mount, or a destination that has become a directory. + // + // This is the only case in which the temp file is on disk at the + // moment of failure, so it is the only one that can show it is + // actually removed rather than merely never created. It also pins + // that the caller sees the original failure: a cleanup that threw + // over the top of it would hide why the download failed. + const existing = new TextEncoder().encode("known-good contents"); + const plaintext = patternBytes(512, 32); + const key = sodium.crypto_secretstream_xchacha20poly1305_keygen(); + const { header, ciphertext } = encryptFileBody(plaintext, key); + const { api, file } = fixtureFor(key, header, ciphertext); + const dir = freshDir(); + const outPath = join(dir, "rename-fails.bin"); + writeFileSync(outPath, existing); + + renameHook.failWith = new Error("simulated rename failure"); + + await expect(download(api, file, outPath)).rejects.toThrow( + "simulated rename failure", + ); + + expect(renameHook.calls).toHaveLength(1); + const staged = renameHook.calls[0]!; + expect(staged.sourceExisted).toBe(true); + expect(existsSync(staged.from)).toBe(false); + + // The previous contents are still there, untouched, and the + // directory holds nothing else. + expect(readFileSync(outPath)).toEqual(Buffer.from(existing)); + expect(readdirSync(dir)).toEqual(["rename-fails.bin"]); + }); + }, +);