From 096a8cf5a7f1cab9b0416eef4ca859b62e40f7f4 Mon Sep 17 00:00:00 2001 From: sneak Date: Tue, 22 Sep 2026 23:41:27 +0000 Subject: [PATCH] Validate session snapshots and wipe keys on logout (closes #10) Client.fromJSON checks every snapshot field and each key's decoded length and throws an error naming the bad field. toJSON reads the token through a new ApiClient.getAuthToken and throws when there is none. logout zeroes the key buffers in place; collectionsSince re-checks for logout after its request so it never decrypts with zeroed keys. The CLI now reports a corrupt session file separately from a missing one. Model: opus-5-5 --- README.md | 9 +- TODO.md | 7 ++ bin/quak.ts | 36 +++---- src/api/client.ts | 4 + src/cli-session.ts | 26 +++++ src/client.ts | 73 +++++++++++-- test/client/session.test.ts | 197 ++++++++++++++++++++++++++++++++++++ 7 files changed, 319 insertions(+), 33 deletions(-) create mode 100644 src/cli-session.ts create mode 100644 test/client/session.test.ts diff --git a/README.md b/README.md index 38b8039..1b11afc 100644 --- a/README.md +++ b/README.md @@ -422,13 +422,18 @@ decides how to persist sessions. `client.toJSON()` returns a `ClientSnapshot` (a plain serializable object with base64-encoded keys) that the consumer can write to disk, a database, or whatever else fits their use case. `Client.fromJSON(snapshot)` restores a -working client from that snapshot without re-authenticating. +working client from that snapshot without re-authenticating; it checks every +field and each key's length first, and throws an error naming the bad field. +`client.logout()` clears the token and zeroes the key buffers in place; every +later call on that client throws. The CLI stores the snapshot at the platform-appropriate data directory via `env-paths`: `~/Library/Application Support/quak/session.json` on macOS, `$XDG_DATA_HOME/quak/session.json` on Linux. The file is written with mode `0600`. The key material is stored in cleartext in the JSON; treat this file as -you would treat the password itself. +you would treat the password itself. A missing file is reported as "not logged +in"; a file that exists but is corrupt is reported as such, naming the bad +field. Both exit with status 1. ### CLI surface diff --git a/TODO.md b/TODO.md index 2300972..43efe80 100644 --- a/TODO.md +++ b/TODO.md @@ -18,6 +18,13 @@ Tag v1.0.0. # Completed Steps +- 2026-09-22: Hardened the client session lifecycle (issue 10). + `Client.fromJSON` checks every snapshot field and each key's decoded length + and names the bad field; `toJSON` reads the token through + `ApiClient.getAuthToken` and throws when there is none; `logout` zeroes the + key buffers, and `collectionsSince` re-checks for logout after its request so + it never decrypts with zeroed keys. The CLI reports a corrupt session file + separately from a missing one (`src/cli-session.ts`). - 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/bin/quak.ts b/bin/quak.ts index 068620c..6760490 100644 --- a/bin/quak.ts +++ b/bin/quak.ts @@ -2,13 +2,7 @@ import { input, password as passwordPrompt } from "@inquirer/prompts"; import { stdout, stderr } from "node:process"; -import { - copyFileSync, - existsSync, - mkdirSync, - readFileSync, - writeFileSync, -} from "node:fs"; +import { copyFileSync, existsSync, mkdirSync, writeFileSync } from "node:fs"; import { join } from "node:path"; import { Command } from "commander"; import envPaths from "env-paths"; @@ -22,6 +16,7 @@ import { thumbnailName, } from "../src/cli-output.js"; import { freshCollections, freshFiles, freshFile } from "../src/cli-read.js"; +import { loadSession } from "../src/cli-session.js"; import { runMetadataBackup } from "../src/metadata-backup.js"; import { listMissingThumbnails, @@ -31,15 +26,6 @@ import { const paths = envPaths("quak", { suffix: "" }); const sessionPath = join(paths.data, "session.json"); -const loadSession = (): ClientSnapshot | null => { - if (!existsSync(sessionPath)) return null; - try { - return JSON.parse(readFileSync(sessionPath, "utf-8")) as ClientSnapshot; - } catch { - return null; - } -}; - const saveSession = (snapshot: ClientSnapshot): void => { mkdirSync(paths.data, { recursive: true, mode: 0o700 }); writeFileSync(sessionPath, JSON.stringify(snapshot, null, 2), { @@ -48,14 +34,23 @@ const saveSession = (snapshot: ClientSnapshot): void => { }; const requireSession = (): Client => { - const snapshot = loadSession(); - if (!snapshot) { + let client: Client | null; + try { + client = loadSession(sessionPath); + } catch (err) { + stderr.write( + `${err instanceof Error ? err.message : err}\n` + + `Run "quak logout" and then "quak login" to replace it.\n`, + ); + process.exit(1); + } + if (!client) { stderr.write( `Not logged in. Run "quak login" first.\nSession file: ${sessionPath}\n`, ); process.exit(1); } - return Client.fromJSON(snapshot); + return client; }; const prompt = async (message: string): Promise => input({ message }); @@ -157,7 +152,8 @@ program program .command("whoami") .description("Print the logged-in account") - .action(() => { + .action(async () => { + await init(); const client = requireSession(); const info = client.whoami(); stdout.write(JSON.stringify(info) + "\n"); diff --git a/src/api/client.ts b/src/api/client.ts index f3b9996..61f874f 100644 --- a/src/api/client.ts +++ b/src/api/client.ts @@ -139,6 +139,10 @@ export class ApiClient { this.token = undefined; } + getAuthToken(): string | undefined { + return this.token; + } + // The policy this client was configured with, so that a caller wrapping a // whole operation in its own `withRetry` — the download layer — runs under // the same settings rather than under the library defaults. diff --git a/src/cli-session.ts b/src/cli-session.ts new file mode 100644 index 0000000..72228d2 --- /dev/null +++ b/src/cli-session.ts @@ -0,0 +1,26 @@ +// How the CLI reads its saved session file back into a `Client`. +// +// A missing file means "not logged in" and returns null. A file that exists but +// cannot be read back into a client (bad JSON, a missing field, a key of the +// wrong length) throws an error saying the session file is corrupt, so the CLI +// can tell the user which of the two it is. Needs `init()` first. + +import { existsSync, readFileSync } from "node:fs"; +import type { ApiClientOptions } from "./api/client.js"; +import { Client } from "./client.js"; + +export const loadSession = ( + path: string, + apiOptions?: ApiClientOptions, +): Client | null => { + if (!existsSync(path)) return null; + try { + return Client.fromJSON( + JSON.parse(readFileSync(path, "utf-8")), + apiOptions, + ); + } catch (err) { + const reason = err instanceof Error ? err.message : String(err); + throw new Error(`Session file ${path} is corrupt: ${reason}`); + } +}; diff --git a/src/client.ts b/src/client.ts index 3acfd48..8859877 100644 --- a/src/client.ts +++ b/src/client.ts @@ -125,18 +125,57 @@ export class Client { ); } - static fromJSON( - snapshot: ClientSnapshot, - apiOptions?: ApiClientOptions, - ): Client { - const api = new ApiClient({ ...apiOptions, authToken: snapshot.token }); + // Restore a client from a `toJSON()` snapshot. The snapshot usually comes + // straight from `JSON.parse` of a file on disk, so every field is checked + // before use; a bad one throws an error naming it. Needs `init()` first. + static fromJSON(snapshot: unknown, apiOptions?: ApiClientOptions): Client { + const invalid = (field: string, problem: string): Error => + new Error(`Invalid session data: ${field} ${problem}`); + + if (typeof snapshot !== "object" || snapshot === null) { + throw new Error("Invalid session data: not a JSON object"); + } + const s = snapshot as Record; + for (const field of ["email", "token"]) { + if (typeof s[field] !== "string" || s[field] === "") { + throw invalid(field, "must be a non-empty string"); + } + } + if (!Number.isInteger(s.userID)) { + throw invalid("userID", "must be an integer"); + } + const key = (field: string): Uint8Array => { + const value = s[field]; + if (typeof value !== "string") { + throw invalid(field, "must be a base64 string"); + } + let bytes: Uint8Array; + try { + bytes = fromBase64(value); + } catch { + throw invalid(field, "is not valid base64"); + } + // The master key (secretbox) and the key pair (box) are all 32 bytes. + if (bytes.length !== 32) { + throw invalid( + field, + `must decode to 32 bytes, got ${bytes.length}`, + ); + } + return bytes; + }; + + const api = new ApiClient({ + ...apiOptions, + authToken: s.token as string, + }); return new Client( api, - snapshot.email, - snapshot.userID, - fromBase64(snapshot.masterKey), - fromBase64(snapshot.secretKey), - fromBase64(snapshot.publicKey), + s.email as string, + s.userID as number, + key("masterKey"), + key("secretKey"), + key("publicKey"), ); } @@ -164,19 +203,29 @@ export class Client { toJSON(): ClientSnapshot { this.assertLoggedIn(); + const token = this.api.getAuthToken(); + if (!token) { + throw new Error("Cannot serialize client: it has no auth token"); + } return { email: this.email, userID: this.userID, - token: this.api["token"]!, + token, masterKey: toBase64(this.masterKey), secretKey: toBase64(this.secretKey), publicKey: toBase64(this.publicKey), }; } + // Zeroes the key buffers in place, so any copy of the reference held + // elsewhere is wiped too. Every method checks `assertLoggedIn` before + // touching the keys, so nothing decrypts with the zeroed keys. logout(): void { this.loggedOut = true; this.api.clearAuthToken(); + this.masterKey.fill(0); + this.secretKey.fill(0); + this.publicKey.fill(0); } // Enumerate collections changed since `sinceTime`. Live collections are @@ -192,6 +241,8 @@ export class Client { const { collections: raws } = await this.api.getJSON<{ collections: RawCollection[]; }>("/collections/v2", { sinceTime: args.sinceTime }); + // logout() may have zeroed the keys while the request was in flight. + this.assertLoggedIn(); const collections: Collection[] = []; const deleted: number[] = []; diff --git a/test/client/session.test.ts b/test/client/session.test.ts new file mode 100644 index 0000000..7aa2359 --- /dev/null +++ b/test/client/session.test.ts @@ -0,0 +1,197 @@ +/** + * Tests for the client session lifecycle: `toJSON`, `fromJSON`, `logout`, and + * the CLI's `loadSession`, which reads the saved session file back into a + * client. + */ + +import { mkdtempSync, rmSync, writeFileSync } from "node:fs"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; +import sodium from "libsodium-wrappers-sumo"; +import { afterAll, beforeAll, describe, expect, it } from "vitest"; +import { init, toBase64 } from "../../src/crypto/index.js"; +import { Client, type ClientSnapshot } from "../../src/client.js"; +import { loadSession } from "../../src/cli-session.js"; + +const validSnapshot = (): ClientSnapshot => { + const kp = sodium.crypto_box_keypair(); + return { + email: "user@example.com", + userID: 42, + token: "test-token", + masterKey: toBase64(sodium.crypto_secretbox_keygen()), + secretKey: toBase64(kp.privateKey), + publicKey: toBase64(kp.publicKey), + }; +}; + +// The client's key buffers are private; the tests read them to prove that +// logout wipes them. +const keyBuffers = (client: Client): Uint8Array[] => [ + client["masterKey"], + client["secretKey"], + client["publicKey"], +]; + +beforeAll(async () => { + await init(); +}); + +describe("Client.toJSON", () => { + it("round-trips through fromJSON unchanged", () => { + const snapshot = validSnapshot(); + expect(Client.fromJSON(snapshot).toJSON()).toEqual(snapshot); + }); + + it("throws instead of emitting a snapshot without a token", () => { + const client = Client.fromJSON(validSnapshot()); + client.getApiClient().clearAuthToken(); + expect(() => client.toJSON()).toThrow(/no auth token/); + }); +}); + +describe("Client.fromJSON", () => { + const shortKey = toBase64(new Uint8Array(16)); + + it.each([ + ["email", undefined], + ["email", 7], + ["email", ""], + ["token", undefined], + ["token", null], + ["token", ""], + ["userID", undefined], + ["userID", "42"], + ["userID", 4.2], + ["masterKey", undefined], + ["masterKey", 7], + ["masterKey", "not base64!"], + ["masterKey", shortKey], + ["secretKey", undefined], + ["secretKey", "not base64!"], + ["secretKey", shortKey], + ["publicKey", undefined], + ["publicKey", "not base64!"], + ["publicKey", shortKey], + ])("rejects %s = %j, naming the field", (field, value) => { + const snapshot: Record = { ...validSnapshot() }; + snapshot[field] = value; + expect(() => Client.fromJSON(snapshot)).toThrow( + new RegExp(`^Invalid session data: ${field} `), + ); + }); + + it.each([null, "a string", 42])("rejects a non-object %j", (value) => { + expect(() => Client.fromJSON(value)).toThrow( + /^Invalid session data: not a JSON object/, + ); + }); +}); + +describe("Client.logout", () => { + it("zeroes the key buffers and clears the token", () => { + const client = Client.fromJSON(validSnapshot()); + const api = client.getApiClient(); + const keys = keyBuffers(client); + + client.logout(); + + for (const key of keys) { + expect(key.length).toBe(32); + expect(key.every((b) => b === 0)).toBe(true); + } + expect(api.getAuthToken()).toBeUndefined(); + }); + + it("makes every later operation throw", async () => { + const client = Client.fromJSON(validSnapshot()); + client.logout(); + + expect(() => client.whoami()).toThrow(/logged out/); + expect(() => client.toJSON()).toThrow(/logged out/); + expect(() => client.getApiClient()).toThrow(/logged out/); + expect(() => client.contentSource()).toThrow(/logged out/); + await expect(client.listCollections()).rejects.toThrow(/logged out/); + await expect(client.collectionsSince({ sinceTime: 0 })).rejects.toThrow( + /logged out/, + ); + await expect( + client.filesSince({ + collectionID: 1, + collectionKey: new Uint8Array(32), + sinceTime: 0, + }), + ).rejects.toThrow(/logged out/); + await expect( + client.fetchMLData({ fileIDs: [1], fileKeys: new Map() }), + ).rejects.toThrow(/logged out/); + }); + + it("stops a listing in flight from decrypting with the zeroed keys", async () => { + // The server answers only after the client has logged out. If the + // listing went on to decrypt this row with all-zero keys it would fail + // with a decryption error, not the logged-out one. + const row = { + id: 1, + owner: { id: 42 }, + encryptedKey: toBase64(new Uint8Array(48)), + keyDecryptionNonce: toBase64(new Uint8Array(24)), + updationTime: 1, + }; + const client: Client = Client.fromJSON(validSnapshot(), { + fetch: async () => { + client.logout(); + return new Response(JSON.stringify({ collections: [row] }), { + status: 200, + headers: { "content-type": "application/json" }, + }); + }, + }); + + await expect(client.listCollections()).rejects.toThrow(/logged out/); + }); +}); + +describe("loadSession", () => { + let dir: string; + + beforeAll(() => { + dir = mkdtempSync(join(tmpdir(), "quak-session-test-")); + }); + + afterAll(() => { + rmSync(dir, { recursive: true, force: true }); + }); + + it("returns null when there is no session file", () => { + expect(loadSession(join(dir, "missing.json"))).toBeNull(); + }); + + it("restores a client from a valid session file", () => { + const path = join(dir, "valid.json"); + writeFileSync(path, JSON.stringify(validSnapshot())); + expect(loadSession(path)!.whoami()).toEqual({ + email: "user@example.com", + userID: 42, + }); + }); + + it("says the file is corrupt when it is not JSON", () => { + const path = join(dir, "truncated.json"); + writeFileSync(path, '{"email": "user@exa'); + expect(() => loadSession(path)).toThrow( + `Session file ${path} is corrupt`, + ); + }); + + it("says the file is corrupt and names the bad field", () => { + const path = join(dir, "bad-key.json"); + writeFileSync( + path, + JSON.stringify({ ...validSnapshot(), secretKey: "AAAA" }), + ); + expect(() => loadSession(path)).toThrow( + new RegExp(`^Session file ${path} is corrupt: .*secretKey`), + ); + }); +});