diff --git a/TODO.md b/TODO.md index 2300972..705df9a 100644 --- a/TODO.md +++ b/TODO.md @@ -18,6 +18,14 @@ Tag v1.0.0. # Completed Steps +- 2026-09-22: Sanitized file names taken from server metadata (issue 9). A new + `src/filename.ts` holds the one sanitizer, used by `quak get`/`get-thumb` + without `--out`, `downloadFile`/`downloadThumbnail` without `outPath`, and the + backup and metadata backup trees; it removes separators, control characters, + leading dots and Windows device names, and falls back to a name built from the + ID for an empty title. Originals-cache extensions are letters and digits only, + else `.bin`. A user-supplied path is used as is. `decryptFile` reads a missing + or non-string title as "" and rejects metadata that is not a JSON object. - 2026-09-22: Rewrote the README API reference (and the Getting Started / usage snippets) to match the shipped cache/API library on `next` (issue 53, issue 13). Documented `Library.open` and its options, the default-read vs `fresh()` diff --git a/src/backup.ts b/src/backup.ts index 7d26ef8..655a41c 100644 --- a/src/backup.ts +++ b/src/backup.ts @@ -40,8 +40,9 @@ import { symlinkSync, writeFileSync, } from "node:fs"; -import { basename, dirname, extname, join, relative } from "node:path"; +import { basename, dirname, join, relative } from "node:path"; +import { safeExtension, sanitizeFileName } from "./filename.js"; import type { Collection, EnteFile } from "./model/types.js"; export type ProgressCallback = (message: string) => void; @@ -108,16 +109,11 @@ interface FailureEntry { const LEDGER_VERSION = 1; -const sanitizePath = (name: string): string => - name.replace(/[/\\:*?"<>|]/g, "_").replace(/^\.+/, "_"); - // The originals/ filename for a file: ``, the extension taken from the // title (or `.bin`). Matches the content cache's own naming so a present check // lines up with what a fetch would write. -const originalName = (file: EnteFile): string => { - const ext = extname(file.metadata.title || "") || ".bin"; - return `${file.id}${ext}`; -}; +const originalName = (file: EnteFile): string => + `${file.id}${safeExtension(file.metadata.title)}`; // A regular file with content is treated as complete. A zero-byte file is not: // it is the shape an aborted write leaves and must be re-fetched. @@ -370,7 +366,7 @@ export const runBackup = async ( // Then the per-collection symlink trees and JSON. for (const c of collections) { - const colDirName = sanitizePath(c.name || `collection-${c.id}`); + const colDirName = sanitizeFileName(c.name, `collection-${c.id}`); const colDir = join(collectionsDir, colDirName); mkdirSync(colDir, { recursive: true }); @@ -381,8 +377,9 @@ export const runBackup = async ( if (!includeOriginals) continue; const orig = join(originalsDir, originalName(file)); if (!isPresent(orig)) continue; - const linkName = sanitizePath( - file.metadata.title || `file-${file.id}`, + const linkName = sanitizeFileName( + file.metadata.title, + `file-${file.id}`, ); const linkPath = join(colDir, linkName); try { diff --git a/src/cli-output.ts b/src/cli-output.ts index c774221..2bf9b21 100644 --- a/src/cli-output.ts +++ b/src/cli-output.ts @@ -9,6 +9,7 @@ // `metadata.title`, and issue #52 requires that output stay byte-identical, so // the commands shape their output from the raw `EnteFile` through here. +import { sanitizeFileName } from "./filename.js"; import type { EnteFile, FileType, Microseconds } from "./model/types.js"; // One row of `quak files --json`. @@ -32,9 +33,11 @@ export const fileListRow = (file: EnteFile): FileListRow => ({ export const fileListLine = (file: EnteFile): string => `${file.id}\t${file.metadata.fileType}\t${file.metadata.title}`; -// Default output path for `quak get` when `--out` is not given. -export const originalName = (file: EnteFile): string => file.metadata.title; +// Default output path for `quak get` when `--out` is not given. The title comes +// from the server, so it is sanitized; `--out` is the user's and is used as is. +export const originalName = (file: EnteFile): string => + sanitizeFileName(file.metadata.title, `file-${file.id}`); // Default output path for `quak get-thumb` when `--out` is not given. export const thumbnailName = (file: EnteFile): string => - `thumb_${file.metadata.title}`; + `thumb_${originalName(file)}`; diff --git a/src/download/index.ts b/src/download/index.ts index 19e17f7..bb3f16e 100644 --- a/src/download/index.ts +++ b/src/download/index.ts @@ -11,6 +11,7 @@ import { streamTagFinal, } from "../crypto/index.js"; import { TruncatedStreamError } from "../errors.js"; +import { sanitizeFileName } from "../filename.js"; import { withRetry } from "../retry.js"; import type { ApiClient } from "../api/client.js"; import type { EnteFile } from "../model/types.js"; @@ -286,7 +287,10 @@ export const downloadFile = async ( outPath?: string, onProgress?: ProgressCallback, ): Promise => { - const resolvedPath = outPath ?? file.metadata.title; + // `outPath` is the caller's and is used as is; the title is the server's + // and is sanitized so it can only name a file in the current directory. + const resolvedPath = + outPath ?? sanitizeFileName(file.metadata.title, `file-${file.id}`); const header = fromBase64(file.file.decryptionHeader); const bytesWritten = await fetchAndDecrypt( api, @@ -305,7 +309,9 @@ export const downloadThumbnail = async ( outPath?: string, onProgress?: ProgressCallback, ): Promise => { - const resolvedPath = outPath ?? `thumb_${file.metadata.title}`; + const resolvedPath = + outPath ?? + `thumb_${sanitizeFileName(file.metadata.title, `file-${file.id}`)}`; const header = fromBase64(file.thumbnail.decryptionHeader); const bytesWritten = await fetchAndDecrypt( api, diff --git a/src/filename.ts b/src/filename.ts new file mode 100644 index 0000000..963257f --- /dev/null +++ b/src/filename.ts @@ -0,0 +1,37 @@ +// File names built from server-supplied metadata. +// +// A file's title and a collection's name are decrypted from data the server +// hands us, and quak does not trust the server. Any name taken from them and +// used on disk goes through here, so it can only ever name one file inside the +// directory the caller chose: never a path, never `..`, never hidden, never a +// Windows device name. +// +// A path the user typed (`--out`, `outPath`) is not passed through here: the +// caller is trusted, the server is not. + +import { extname } from "node:path"; + +// Path separators, characters Windows forbids in file names, and control +// characters (NUL included). +// eslint-disable-next-line no-control-regex +const UNSAFE_CHARACTERS = /[/\\:*?"<>|\x00-\x1f\x7f]/g; + +// Names Windows reserves for devices, with or without an extension. +const RESERVED_DEVICE_NAME = /^(con|prn|aux|nul|com[1-9]|lpt[1-9])(\.|$)/i; + +// `name` made safe to use as a single file name. Each unsafe character becomes +// `_`, a leading run of dots becomes one `_`, and a device name gets a leading +// `_`. A name with none of these comes back unchanged. An empty name becomes +// `fallback`, which the caller derives from the record's ID. +export const sanitizeFileName = (name: string, fallback: string): string => { + if (name === "") return fallback; + const cleaned = name.replace(UNSAFE_CHARACTERS, "_").replace(/^\.+/, "_"); + return RESERVED_DEVICE_NAME.test(cleaned) ? `_${cleaned}` : cleaned; +}; + +// The extension of `title` (".jpg"), or ".bin" when it has none or it holds +// anything but letters and digits. +export const safeExtension = (title: string): string => { + const ext = extname(title); + return /^\.[A-Za-z0-9]+$/.test(ext) ? ext : ".bin"; +}; diff --git a/src/library/content.ts b/src/library/content.ts index 2cb19d4..c5c4c6e 100644 --- a/src/library/content.ts +++ b/src/library/content.ts @@ -40,6 +40,7 @@ import { downloadThumbnail, type ProgressCallback, } from "../download/index.js"; +import { safeExtension } from "../filename.js"; import type { EnteFile } from "../model/types.js"; import type { Priority, RequestPools } from "./pools.js"; @@ -209,10 +210,8 @@ class AbortDrop extends Error { } } -const originalName = (file: EnteFile): string => { - const ext = extname(file.metadata.title || "") || ".bin"; - return `${file.id}${ext}`; -}; +const originalName = (file: EnteFile): string => + `${file.id}${safeExtension(file.metadata.title)}`; // The fileID a cache filename encodes, or undefined when the name is not one // the cache writes (``). diff --git a/src/metadata-backup.ts b/src/metadata-backup.ts index d5877bd..92c068e 100644 --- a/src/metadata-backup.ts +++ b/src/metadata-backup.ts @@ -4,6 +4,7 @@ import * as jpeg from "jpeg-js"; import exifReader from "exif-reader"; import type { Client } from "./client.js"; import type { Library, Photo } from "./library/index.js"; +import { sanitizeFileName } from "./filename.js"; import { fetchMLData } from "./mldata-fetch.js"; import type { EnteFile } from "./model/types.js"; @@ -14,9 +15,6 @@ export interface MetadataBackupOptions { onProgress?: ProgressCallback; } -const sanitizePath = (name: string): string => - name.replace(/[/\\:*?"<>|]/g, "_").replace(/^\.+/, "_"); - // Extract the raw EXIF APP1 segment from JPEG bytes. Returns the EXIF // data buffer (starting after the APP1 length field, at the "Exif\0\0" // header) or undefined if no APP1 marker is found. @@ -151,7 +149,7 @@ export const runMetadataBackup = async ( const col = lib.getCollection(album.collectionID); if (!col) continue; - const dirName = `${col.id}-${sanitizePath(col.name || "unnamed")}`; + const dirName = `${col.id}-${sanitizeFileName(col.name, "unnamed")}`; const colDir = join(outDir, "collections", dirName); mkdirSync(colDir, { recursive: true }); diff --git a/src/model/decrypt.ts b/src/model/decrypt.ts index 253e3f0..4458b4c 100644 --- a/src/model/decrypt.ts +++ b/src/model/decrypt.ts @@ -98,9 +98,18 @@ export const decryptFile = ( key, ); const metadataJSON = JSON.parse(new TextDecoder().decode(metadataBytes)); + if ( + typeof metadataJSON !== "object" || + metadataJSON === null || + Array.isArray(metadataJSON) + ) { + throw new Error(`file ${raw.id}: metadata is not a JSON object`); + } const metadata: FileMetadata = { - title: metadataJSON.title ?? "", + // The server controls this JSON: a title that is missing or not a + // string becomes "", never an arbitrary value. + title: typeof metadataJSON.title === "string" ? metadataJSON.title : "", fileType: parseFileType(metadataJSON.fileType ?? -1), creationTime: metadataJSON.creationTime ?? 0, modificationTime: metadataJSON.modificationTime ?? 0, diff --git a/test/cli/backup.test.ts b/test/cli/backup.test.ts index 7823730..7ae5e97 100644 --- a/test/cli/backup.test.ts +++ b/test/cli/backup.test.ts @@ -36,6 +36,7 @@ import { lstatSync, mkdirSync, mkdtempSync, + readdirSync, readFileSync, readlinkSync, rmSync, @@ -107,6 +108,29 @@ class MockClient { } } +// A server that names an album and a file so as to climb out of the backup +// directory. +class HostileClient extends MockClient { + override async collectionsSince(): Promise { + const page = await super.collectionsSince(); + return { + ...page, + collections: page.collections.length + ? [collection(3, "../escape")] + : [], + }; + } + override async filesSince(args: { + collectionID: number; + }): Promise { + const files = + args.collectionID === 3 + ? [file(300, 3, "../../.ssh/authorized_keys")] + : []; + return { files, deleted: [], cursor: 1 }; + } +} + // A content source that writes byte buffers of the expected length and can be // told to fail one fileID's original, to exercise per-file resilience. interface StubSource extends ContentSource { @@ -136,9 +160,12 @@ const stubSource = (): StubSource => { let root: string; -const openLibrary = (source: ContentSource): Promise => +const openLibrary = ( + source: ContentSource, + client: MockClient = new MockClient(), +): Promise => Library.open({ - client: new MockClient(), + client, cacheDirectory: join(root, "cache"), contentSource: source, refreshIntervalSeconds: 3600, @@ -243,6 +270,30 @@ describe("lib.backup", () => { lib.close(); }); + it("keeps server-supplied album and file names inside the backup", async () => { + const lib = await openLibrary(stubSource(), new HostileClient()); + const outDir = join(root, "backup"); + + const result = await lib.backup({ downloadDirectory: outDir }); + + expect(result.failed).toBe(0); + // The title has no usable extension, so the original is `.bin`. + expect(existsSync(join(outDir, "originals", "300.bin"))).toBe(true); + const link = join( + outDir, + "collections", + "__escape", + "__.._.ssh_authorized_keys", + ); + expect(lstatSync(link).isSymbolicLink()).toBe(true); + expect(existsSync(join(outDir, "collections", "__escape.json"))).toBe( + true, + ); + // Nothing landed beside or above the backup directory. + expect(readdirSync(root).sort()).toEqual(["backup", "cache"]); + lib.close(); + }); + it("is an idempotent no-op when every original is already present", async () => { const source = stubSource(); const lib = await openLibrary(source); diff --git a/test/cli/metadata-backup.test.ts b/test/cli/metadata-backup.test.ts index c13c612..560ca76 100644 --- a/test/cli/metadata-backup.test.ts +++ b/test/cli/metadata-backup.test.ts @@ -165,14 +165,15 @@ const buildMetaMock = async (): Promise => { }, }; - // Collection 2: "Work" with no magic metadata + // Collection 2: "../Work" with no magic metadata. The server chose a name + // that tries to climb out of the backup directory. const ck2 = sodium.crypto_secretbox_keygen(); const { ciphertext: encCK2, nonce: ck2N } = encryptSecretbox( ck2, masterKey, ); const { ciphertext: encCN2, nonce: cn2N } = encryptSecretbox( - new TextEncoder().encode("Work"), + new TextEncoder().encode("../Work"), ck2, ); const rawColl2 = { @@ -496,7 +497,8 @@ describe("quak backup-metadata", () => { await runBackup(outDir); const collDirs = readdirSync(join(outDir, "collections")); - expect(collDirs.length).toBe(2); + // "../Work" is sanitized into one directory name. + expect(collDirs.sort()).toEqual(["10-Vacation", "20-__Work"]); // Find the Vacation collection dir (prefixed with ID) const vacDir = collDirs.find((d) => d.includes("Vacation"))!; diff --git a/test/cli/output.test.ts b/test/cli/output.test.ts index 3f5a718..ce03e30 100644 --- a/test/cli/output.test.ts +++ b/test/cli/output.test.ts @@ -64,6 +64,24 @@ describe("CLI file output (issue #52)", () => { expect(thumbnailName(renamedFile)).toBe(`thumb_${RAW_TITLE}`); }); + it("sanitizes the title when naming `quak get` downloads", () => { + // Without `--out`, the server-supplied title names the file, so it must + // not be able to point outside the working directory. + const hostile = { + ...renamedFile, + metadata: { ...renamedFile.metadata, title: "../../.bashrc" }, + }; + expect(originalName(hostile)).toBe("__.._.bashrc"); + expect(thumbnailName(hostile)).toBe("thumb___.._.bashrc"); + + const untitled = { + ...renamedFile, + metadata: { ...renamedFile.metadata, title: "" }, + }; + expect(originalName(untitled)).toBe("file-100"); + expect(thumbnailName(untitled)).toBe("thumb_file-100"); + }); + it("does not use the editedName/editedTime projection", () => { const record = deriveRecords([], [renamedFile]).photos.get(100); // The projection prefers the edits and reports milliseconds; the CLI diff --git a/test/download/download.test.ts b/test/download/download.test.ts index eb27c65..52ca6f0 100644 --- a/test/download/download.test.ts +++ b/test/download/download.test.ts @@ -49,6 +49,7 @@ import { existsSync, + mkdirSync, readdirSync, readFileSync, rmSync, @@ -501,6 +502,22 @@ const entryPoints = [ { name: "downloadThumbnail", download: downloadThumbnail }, ]; +// With no `outPath`, the destination is named after `metadata.title`, relative +// to the working directory. Such tests run inside a temporary directory: +// `make check` must not create files in the repo root. +const inDirectory = async ( + dir: string, + run: () => Promise, +): Promise => { + const previous = process.cwd(); + process.chdir(dir); + try { + return await run(); + } finally { + process.chdir(previous); + } +}; + // --------------------------------------------------------------------------- // Tests // --------------------------------------------------------------------------- @@ -536,26 +553,59 @@ describe("downloadFile", () => { }); 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); - const titlePath = join(testDir, "fallback-name.png"); - file.metadata.title = titlePath; + file.metadata.title = "fallback-name.png"; + const dir = mkdtempSync(join(testDir, "title-")); const api = new ApiClient({ fetch: mockFetchForBody(ciphertext) }); - const result = await downloadFile(api, file); + const result = await inDirectory(dir, () => downloadFile(api, file)); - expect(result.path).toBe(titlePath); - expect(readFileSync(result.path)).toEqual(Buffer.from(plaintext)); + expect(result.path).toBe("fallback-name.png"); + expect(readFileSync(join(dir, "fallback-name.png"))).toEqual( + Buffer.from(plaintext), + ); + }); + + it("keeps a hostile title inside the working directory", async () => { + // The server controls the title. `../escaped.png` must not write to + // the parent directory; it becomes one file name in the current one. + const { api, file } = fixtureFor( + multiChunkKey, + multiChunk.header, + multiChunk.body, + ); + file.metadata.title = "../escaped.png"; + const parent = mkdtempSync(join(testDir, "hostile-")); + const dir = join(parent, "cwd"); + mkdirSync(dir); + + const result = await inDirectory(dir, () => downloadFile(api, file)); + + expect(result.path).toBe("__escaped.png"); + expect(readdirSync(dir)).toEqual(["__escaped.png"]); + expect(readdirSync(parent)).toEqual(["cwd"]); + }); + + it("uses an explicit outPath verbatim, even one with ..", async () => { + // The caller is trusted: its path is not sanitized. + const { api, file } = fixtureFor( + multiChunkKey, + multiChunk.header, + multiChunk.body, + ); + const dir = mkdtempSync(join(testDir, "explicit-")); + mkdirSync(join(dir, "sub")); + const outPath = join(dir, "sub", "..", "explicit.bin"); + + const result = await downloadFile(api, file, outPath); + + expect(result.path).toBe(outPath); + expect(existsSync(join(dir, "explicit.bin"))).toBe(true); }); it("handles a larger single-chunk file (random binary payload)", async () => { @@ -617,6 +667,23 @@ describe("downloadThumbnail", () => { expect(result).toEqual({ path: outPath, bytesWritten: 4 }); expect(readFileSync(outPath)).toEqual(Buffer.from(plaintext)); }); + + it("names the thumbnail thumb_ plus the sanitized title", async () => { + const { api, file } = fixtureFor( + multiChunkKey, + multiChunk.header, + multiChunk.body, + ); + file.metadata.title = "/etc/passwd"; + const dir = mkdtempSync(join(testDir, "thumb-title-")); + + const result = await inDirectory(dir, () => + downloadThumbnail(api, file), + ); + + expect(result.path).toBe("thumb__etc_passwd"); + expect(readdirSync(dir)).toEqual(["thumb__etc_passwd"]); + }); }); // --------------------------------------------------------------------------- diff --git a/test/filename/filename.test.ts b/test/filename/filename.test.ts new file mode 100644 index 0000000..81e7c56 --- /dev/null +++ b/test/filename/filename.test.ts @@ -0,0 +1,93 @@ +// File names built from server-supplied metadata. +// +// quak does not trust the server. A file's title and a collection's name are +// decrypted from data the server hands us, and a hostile server (or a +// compromised account) can set them to anything. quak uses them to name files +// on disk: `quak get` without `--out`, `downloadFile` without `outPath`, the +// backup's symlink and collection directories, and the extension of every file +// in the originals cache. Each of those goes through `sanitizeFileName` or +// `safeExtension`, so a title can only ever name one file inside the directory +// the caller chose. +// +// A path the user supplies (`--out`, `outPath`) is never sanitized: the caller +// is trusted, the server is not. + +import { describe, expect, it } from "vitest"; + +import { safeExtension, sanitizeFileName } from "../../src/filename.js"; + +const FALLBACK = "file-42"; + +describe("sanitizeFileName", () => { + it("passes a normal title through unchanged", () => { + expect(sanitizeFileName("IMG_0001.HEIC", FALLBACK)).toBe( + "IMG_0001.HEIC", + ); + expect(sanitizeFileName("Holiday 2024 (1).jpg", FALLBACK)).toBe( + "Holiday 2024 (1).jpg", + ); + expect(sanitizeFileName("café.jpg", FALLBACK)).toBe("café.jpg"); + }); + + it("cannot climb out of the directory with ../", () => { + // Without sanitizing, this would overwrite the user's SSH keys. + expect(sanitizeFileName("../../.ssh/authorized_keys", FALLBACK)).toBe( + "__.._.ssh_authorized_keys", + ); + expect(sanitizeFileName("..", FALLBACK)).toBe("_"); + expect(sanitizeFileName("..\\..\\x", FALLBACK)).toBe("__.._x"); + }); + + it("cannot name an absolute path", () => { + expect(sanitizeFileName("/etc/passwd", FALLBACK)).toBe("_etc_passwd"); + expect(sanitizeFileName("C:\\Windows\\x.dll", FALLBACK)).toBe( + "C__Windows_x.dll", + ); + }); + + it("replaces embedded separators, so the name stays one file", () => { + expect(sanitizeFileName("a/b\\c.jpg", FALLBACK)).toBe("a_b_c.jpg"); + }); + + it("replaces NUL and other control characters", () => { + // A NUL truncates the path in C code and makes Node's fs throw. + expect(sanitizeFileName("evil\0.jpg", FALLBACK)).toBe("evil_.jpg"); + expect(sanitizeFileName("line\nbreak\x7f.jpg", FALLBACK)).toBe( + "line_break_.jpg", + ); + }); + + it("does not produce a hidden file", () => { + expect(sanitizeFileName(".bashrc", FALLBACK)).toBe("_bashrc"); + }); + + it("does not produce a Windows device name", () => { + expect(sanitizeFileName("CON", FALLBACK)).toBe("_CON"); + expect(sanitizeFileName("nul.txt", FALLBACK)).toBe("_nul.txt"); + expect(sanitizeFileName("LPT1", FALLBACK)).toBe("_LPT1"); + // Only the exact names are reserved. + expect(sanitizeFileName("console.jpg", FALLBACK)).toBe("console.jpg"); + }); + + it("falls back to the given name for an empty title", () => { + expect(sanitizeFileName("", FALLBACK)).toBe(FALLBACK); + }); +}); + +describe("safeExtension", () => { + it("keeps a normal extension", () => { + expect(safeExtension("IMG_0001.HEIC")).toBe(".HEIC"); + expect(safeExtension("clip.mp4")).toBe(".mp4"); + }); + + it("uses .bin when there is no extension", () => { + expect(safeExtension("")).toBe(".bin"); + expect(safeExtension("README")).toBe(".bin"); + }); + + it("uses .bin when the extension holds anything but letters and digits", () => { + expect(safeExtension("x.j\\..\\pg")).toBe(".bin"); + expect(safeExtension("x.jp g")).toBe(".bin"); + expect(safeExtension("x.jpg\0")).toBe(".bin"); + }); +}); diff --git a/test/library/content.test.ts b/test/library/content.test.ts index 6fe91ab..188e8a7 100644 --- a/test/library/content.test.ts +++ b/test/library/content.test.ts @@ -226,6 +226,25 @@ describe("ContentCache.original / thumbnail", () => { expect(skips).toEqual(["skipped"]); }); + it("takes only a letters-and-digits extension from the title", async () => { + // The title comes from the server; an extension such as `.\..\x` + // must not reach the cache file name, so it becomes `.bin`. + const { cache } = buildCache({ + files: [file(1, "a.jpg"), file(2, "b.\\..\\x"), file(3, "")], + }); + await cache.open(); + + expect((await cache.original(1)).path).toBe( + join(cacheDir, "originals", "1.jpg"), + ); + expect((await cache.original(2)).path).toBe( + join(cacheDir, "originals", "2.bin"), + ); + expect((await cache.original(3)).path).toBe( + join(cacheDir, "originals", "3.bin"), + ); + }); + it("serves a file already present in the download directory without fetching", async () => { const downloadDirectory = join(root, "backup"); mkdirSync(join(downloadDirectory, "originals"), { recursive: true }); diff --git a/test/model/decrypt.test.ts b/test/model/decrypt.test.ts index 049f92c..76dac9e 100644 --- a/test/model/decrypt.test.ts +++ b/test/model/decrypt.test.ts @@ -146,10 +146,13 @@ const buildSharedRawCollection = ( const buildRawFile = ( collectionKey: Uint8Array, opts?: { - title?: string; + // Any JSON value; `undefined` leaves the title out of the metadata. + title?: unknown; fileType?: number; creationTime?: number; info?: { fileSize?: number; thumbSize?: number }; + // Replaces the whole metadata JSON value. + metadata?: unknown; }, ): RawEnteFile => { const fileKey = sodium.crypto_secretbox_keygen(); @@ -158,8 +161,8 @@ const buildRawFile = ( collectionKey, ); - const metadata = { - title: opts?.title ?? "IMG_0001.jpg", + const defaultMetadata = { + title: opts && "title" in opts ? opts.title : "IMG_0001.jpg", fileType: opts?.fileType ?? 0, creationTime: opts?.creationTime ?? 1700000000000000, modificationTime: 1700000000000000, @@ -167,6 +170,8 @@ const buildRawFile = ( longitude: 2.3522, hash: "abcdef1234567890", }; + const metadata = + opts && "metadata" in opts ? opts.metadata : defaultMetadata; // File metadata is encrypted as a single-chunk secretstream blob // (not secretbox). The decryptionHeader is the secretstream init header. const metadataBytes = new TextEncoder().encode(JSON.stringify(metadata)); @@ -321,6 +326,31 @@ describe("model.decryptFile", () => { expect(file.metadata.longitude).toBeCloseTo(2.3522); }); + it("reads a missing or non-string title as an empty string", () => { + // The server controls the metadata JSON. A title that is not a + // string must not reach code that builds file names from it. + const masterKey = sodium.crypto_secretbox_keygen(); + const { collectionKey } = buildRawCollection(masterKey); + for (const title of [undefined, null, 42, ["a"], { x: "../y" }]) { + const file = decryptFile( + buildRawFile(collectionKey, { title }), + collectionKey, + ); + expect(file.metadata.title).toBe(""); + } + }); + + it("rejects metadata that is not a JSON object", () => { + const masterKey = sodium.crypto_secretbox_keygen(); + const { collectionKey } = buildRawCollection(masterKey); + for (const metadata of [null, "IMG_0001.jpg", 7, []]) { + const raw = buildRawFile(collectionKey, { metadata }); + expect(() => decryptFile(raw, collectionKey)).toThrow( + "file 200: metadata is not a JSON object", + ); + } + }); + it("maps fileType numbers to FileType strings", () => { // Ente uses: 0=image, 1=video, 2=livePhoto const masterKey = sodium.crypto_secretbox_keygen();