From ccb9b3d11d6aafbf22947821f7a04e82a6ef1d52 Mon Sep 17 00:00:00 2001 From: sneak Date: Wed, 14 Oct 2026 00:17:35 +0000 Subject: [PATCH 1/2] Tests: an ID on the command line is decimal digits only files --collection, get, get-thumb and helper fix-missing-thumbnails --file must refuse 0x10, 1e3, 1.5, -1, an empty string, abc and an integer past Number.MAX_SAFE_INTEGER with one line naming the argument, and exit 1 before any request. A good ID goes on to the server. The commands run on a real client over a fake Ente server holding an empty account. The two older tests of a non-numeric ID are folded into these. Model: opus-5-5 --- test/cli/commands.test.ts | 138 +++++++++++++++++++++++++++++++++++--- 1 file changed, 127 insertions(+), 11 deletions(-) diff --git a/test/cli/commands.test.ts b/test/cli/commands.test.ts index 8e068c8..09189a5 100644 --- a/test/cli/commands.test.ts +++ b/test/cli/commands.test.ts @@ -511,11 +511,6 @@ describe("files", () => { expect(await filesCommand(context(), { collection: "9" })).toBe(1); expect(stderr.text).toBe("Collection 9 not found\n"); }); - - it("exits 1 for a collection ID that is not a number", async () => { - expect(await filesCommand(context(), { collection: "abc" })).toBe(1); - expect(stderr.text).toBe("Invalid collection ID\n"); - }); }); describe("get and get-thumb", () => { @@ -546,12 +541,6 @@ describe("get and get-thumb", () => { expect(stderr.text).toBe("File 999 not found\n"); expect(existsSync(out)).toBe(false); }); - - it("both exit 1 for a file ID that is not a number", async () => { - expect(await getCommand(context(), "abc", {})).toBe(1); - expect(await getThumbCommand(context(), "abc", {})).toBe(1); - expect(stderr.text).toBe("Invalid file ID\nInvalid file ID\n"); - }); }); // A live photo, which Ente stores as one ZIP, is written as its image and its @@ -1035,3 +1024,130 @@ describe("backup-metadata and the thumbnail helpers refresh first", () => { expect(stdout.text).toBe(""); }); }); + +// `files --collection`, `get`, `get-thumb` and `helper fix-missing-thumbnails +// --file` take an ID only when it is written in decimal digits and is a safe +// integer. `Number()` alone reads `0x10` as 16, `1e3` as 1000 and an empty +// string as 0, so the command would act on a file the user never named. +// Anything else is refused with one line naming it, and exit 1, before any +// request. +// +// These run on a real client, read back from a saved session, over a fake Ente +// server holding an empty account, which records each request it receives. With +// a good ID the command goes on to ask the server, and finds nothing there. +describe("an ID given on the command line", () => { + const NOT_IDS = [ + "0x10", + "1e3", + "1.5", + "-1", + "", + "abc", + // One past Number.MAX_SAFE_INTEGER: from here on a number cannot + // hold every integer, so two IDs could read as one. + "9007199254740992", + ]; + + const requests: Request[] = []; + + const emptyAccount = (): CliContext => { + const ctx = context(); + const key = toBase64(new Uint8Array(32)); + saveSession(ctx.sessionDir, { + email: "cli@example.com", + userID: USER_ID, + token: "token", + masterKey: key, + secretKey: key, + publicKey: key, + }); + return { + ...ctx, + loadSession: (path) => + loadSession(path, { + fetch: async (url, init) => { + requests.push(new Request(url, init)); + return new Response( + JSON.stringify({ collections: [] }), + { headers: { "content-type": "application/json" } }, + ); + }, + }), + }; + }; + + beforeEach(() => { + requests.length = 0; + }); + + it("files looks up a decimal collection ID", async () => { + const ctx = emptyAccount(); + expect(await filesCommand(ctx, { collection: "1" })).toBe(1); + expect(stderr.text).toBe("Collection 1 not found\n"); + expect(requests).not.toHaveLength(0); + }); + + it.each(NOT_IDS)( + "files refuses --collection %j before any request", + async (arg) => { + const ctx = emptyAccount(); + expect(await filesCommand(ctx, { collection: arg })).toBe(1); + expect(stderr.text).toBe(`Invalid collection ID: ${arg}\n`); + expect(stdout.text).toBe(""); + expect(requests).toHaveLength(0); + }, + ); + + it("get looks up a decimal file ID", async () => { + expect(await getCommand(emptyAccount(), "100", {})).toBe(1); + expect(stderr.text).toBe("File 100 not found\n"); + expect(requests).not.toHaveLength(0); + }); + + it.each(NOT_IDS)("get refuses %j before any request", async (arg) => { + expect(await getCommand(emptyAccount(), arg, {})).toBe(1); + expect(stderr.text).toBe(`Invalid file ID: ${arg}\n`); + expect(stdout.text).toBe(""); + expect(requests).toHaveLength(0); + }); + + it("get-thumb looks up a decimal file ID", async () => { + expect(await getThumbCommand(emptyAccount(), "100", {})).toBe(1); + expect(stderr.text).toBe("File 100 not found\n"); + expect(requests).not.toHaveLength(0); + }); + + it.each(NOT_IDS)("get-thumb refuses %j before any request", async (arg) => { + expect(await getThumbCommand(emptyAccount(), arg, {})).toBe(1); + expect(stderr.text).toBe(`Invalid file ID: ${arg}\n`); + expect(stdout.text).toBe(""); + expect(requests).toHaveLength(0); + }); + + // The fixer reports each file it was given; on this account neither is + // found, which is a failure, so it exits 1. + it("fix-missing-thumbnails looks up each decimal --file ID", async () => { + const ctx = emptyAccount(); + const opts = { file: ["100", "200"], json: true }; + expect(await fixMissingThumbnailsCommand(ctx, opts)).toBe(1); + expect(JSON.parse(stdout.text)).toMatchObject([ + { fileID: 100, reason: "file not found in any collection" }, + { fileID: 200, reason: "file not found in any collection" }, + ]); + expect(requests).not.toHaveLength(0); + }); + + // One bad ID fails the whole command, so the good one before it is not + // fixed either. + it.each(NOT_IDS)( + "fix-missing-thumbnails refuses --file 100 %j before any request", + async (arg) => { + const ctx = emptyAccount(); + const opts = { file: ["100", arg] }; + expect(await fixMissingThumbnailsCommand(ctx, opts)).toBe(1); + expect(stderr.text).toBe(`Invalid file ID: ${arg}\n`); + expect(stdout.text).toBe(""); + expect(requests).toHaveLength(0); + }, + ); +}); -- 2.54.0 From 6a6b3ef478916a555355551959e200952370b4a5 Mon Sep 17 00:00:00 2001 From: sneak Date: Wed, 14 Oct 2026 00:24:54 +0000 Subject: [PATCH 2/2] An ID on the command line is decimal digits only (closes #185) files --collection, get, get-thumb and helper fix-missing-thumbnails --file read an ID with Number(), so 0x10 meant 16, 1e3 meant 1000 and an empty string meant 0, and --file dropped an ID it could not read. parseID in src/cli-commands.ts now takes an ID only when it is decimal digits and a safe integer; anything else prints "Invalid file ID: ARG" or "Invalid collection ID: ARG" and the command exits 1. The fixer checks every --file ID before it opens the library, so one bad ID fails the whole command before any request. Model: opus-5-5 --- README.md | 7 +++++++ TODO.md | 9 +++++++++ src/cli-commands.ts | 47 +++++++++++++++++++++++++++------------------ 3 files changed, 44 insertions(+), 19 deletions(-) diff --git a/README.md b/README.md index 62d3697..32a0b9d 100644 --- a/README.md +++ b/README.md @@ -626,6 +626,13 @@ exits 1, or 3 when the server no longer accepts the saved session (see "Session handling"). `--cache-dir` overrides where the cache lives; without it each account gets its own directory under the per-user cache path. +An ID given to `files --collection`, `get`, `get-thumb` or +`helper fix-missing-thumbnails --file` is written in decimal digits, no larger +than 9007199254740991. Anything else, such as `0x10`, `1e3` or `-1`, is refused +before any request with one line, `Invalid file ID: ` or +`Invalid collection ID: `, and exit status 1. One bad `--file` ID refuses +the whole command, so nothing is fixed. + `get` and `get-thumb` resolve the file by ID directly, so `--collection` is accepted for backward compatibility but ignored. For a live photo, `get` writes its image and its video, each named after the title with its own extension, as diff --git a/TODO.md b/TODO.md index 2ab03da..bacb24a 100644 --- a/TODO.md +++ b/TODO.md @@ -25,6 +25,15 @@ declares one. # Completed Steps +- 2026-10-14: `files --collection`, `get`, `get-thumb` and + `helper fix-missing-thumbnails --file` take an ID only when it is written in + decimal digits and is a safe integer (issue 185). Before, `Number()` read + `0x10` as 16, `1e3` as 1000 and an empty string as 0, and `--file` dropped an + ID it could not read without a word. Anything else now prints + `Invalid file ID: ` or `Invalid collection ID: ` and exits 1 before + any request; one bad `--file` ID fails the whole command. `parseID` in + `src/cli-commands.ts` does the check for all four. + - 2026-10-06: The README says how to run `quak backup` from cron (issue 170): log in once as the job's user, a crontab with a backup every night and `--verify` on Sundays appending to a log file, and the `HOME` and diff --git a/src/cli-commands.ts b/src/cli-commands.ts index f519414..5f8b0b6 100644 --- a/src/cli-commands.ts +++ b/src/cli-commands.ts @@ -97,6 +97,20 @@ const requireSession = (ctx: CliContext): Client | undefined => { return client; }; +// `text` as an ID, or undefined after telling the user it is not one. An ID is +// written in decimal digits and is a safe integer; `Number()` alone would read +// `0x10` as 16, `1e3` as 1000 and "" as 0. +const parseID = ( + ctx: CliContext, + kind: "file" | "collection", + text: string, +): number | undefined => { + const id = Number(text); + if (/^[0-9]+$/.test(text) && Number.isSafeInteger(id)) return id; + ctx.stderr.write(`Invalid ${kind} ID: ${text}\n`); + return undefined; +}; + // A library client that omits `fetchMLData`, so the point commands below do not // kick the library's background ML backfill: they read metadata, or fetch one // file's content, and exit. `backup` and `backup-metadata` handle ML on their @@ -247,11 +261,8 @@ export const filesCommand = async ( await init(); const client = requireSession(ctx); if (!client) return 3; - const collectionID = Number(opts.collection); - if (!Number.isFinite(collectionID)) { - ctx.stderr.write("Invalid collection ID\n"); - return 1; - } + const collectionID = parseID(ctx, "collection", opts.collection); + if (collectionID === undefined) return 1; const lib = await openReadLibrary(ctx, client); try { @@ -289,11 +300,8 @@ export const getCommand = async ( await init(); const client = requireSession(ctx); if (!client) return 3; - const fileID = Number(fileIDStr); - if (!Number.isFinite(fileID)) { - ctx.stderr.write("Invalid file ID\n"); - return 1; - } + const fileID = parseID(ctx, "file", fileIDStr); + if (fileID === undefined) return 1; const lib = await openReadLibrary(ctx, client); try { @@ -347,11 +355,8 @@ export const getThumbCommand = async ( await init(); const client = requireSession(ctx); if (!client) return 3; - const fileID = Number(fileIDStr); - if (!Number.isFinite(fileID)) { - ctx.stderr.write("Invalid file ID\n"); - return 1; - } + const fileID = parseID(ctx, "file", fileIDStr); + if (fileID === undefined) return 1; const lib = await openReadLibrary(ctx, client); try { @@ -516,15 +521,19 @@ export const fixMissingThumbnailsCommand = async ( await init(); const client = requireSession(ctx); if (!client) return 3; + // One bad --file ID fails the whole command, before anything is fetched. + let fileIDs: number[] = []; + for (const text of opts.file ?? []) { + const fileID = parseID(ctx, "file", text); + if (fileID === undefined) return 1; + fileIDs.push(fileID); + } const lib = await openReadLibrary(ctx, client); try { // Refresh first so files added since the cache was written are found; // a failed refresh throws. await lib.fresh(); - let fileIDs: number[]; - if (opts.file && opts.file.length > 0) { - fileIDs = opts.file.map(Number).filter(Number.isFinite); - } else { + if (fileIDs.length === 0) { ctx.stderr.write("Scanning for missing thumbnails...\n"); const missing = await listMissingThumbnails(lib, client, (msg) => { if (!opts.json) ctx.stderr.write(msg + "\n"); -- 2.54.0