Client session handling: stop reaching into private state, validate snapshots, clear keys on logout #10

New Issue

clawbot · 2026-08-09T03:45:01+02:00

clawbot commented

2026-08-09 03:45:01 +02:00

Problem

Three defects in src/client.ts, all in the session lifecycle:

toJSON reads a private field by string index. src/client.ts:141 does
token: this.api["token"]! to bypass TypeScript's private-field check, with a non-null
assertion on top. If toJSON() is called on a client with no token, this silently emits
undefined into a snapshot that is then written to disk as a valid session file.
fromJSON does not validate the snapshot. src/client.ts:107-120 runs fromBase64
over four fields with no shape check. A truncated, hand-edited, or corrupt
session.json throws a raw libsodium error out of the CLI with no indication that the
session file is the problem. bin/quak.ts compounds this by swallowing session-load errors
entirely, so a corrupt session is indistinguishable from "not logged in".
logout() does not clear key material. src/client.ts:148-151 flips a flag and clears
the API token, but masterKey, secretKey and publicKey stay live in memory. For a
client whose whole job is holding the keys to someone's photo library, logout should mean
the keys are gone.

Definition of done

ApiClient exposes a proper accessor for the current auth token; Client.toJSON uses it
and no longer indexes into a private field or uses a non-null assertion.
toJSON() on a client with no auth token fails with a clear error rather than emitting a
snapshot containing undefined.
fromJSON validates the snapshot: required fields present, correct types, base64 decoding
to the expected byte lengths for each key. An invalid snapshot throws an error that names
the offending field and says the session data is invalid.
bin/quak.ts distinguishes "no session file" from "session file is corrupt" and tells the
user which one it is, with a non-zero exit code for the corrupt case.
logout() zeroes the key material buffers in addition to clearing the token, and any
subsequent operation on the logged-out client throws the same way it does today.
Tests cover each of the above, including toJSON round-tripping through fromJSON
unchanged, and every rejection case in item 3.
make check green.
TODO.md updated in the same commit.

## Problem Three defects in `src/client.ts`, all in the session lifecycle: 1. **`toJSON` reads a private field by string index.** `src/client.ts:141` does `token: this.api["token"]!` to bypass TypeScript's private-field check, with a non-null assertion on top. If `toJSON()` is called on a client with no token, this silently emits `undefined` into a snapshot that is then written to disk as a valid session file. 2. **`fromJSON` does not validate the snapshot.** `src/client.ts:107-120` runs `fromBase64` over four fields with no shape check. A truncated, hand-edited, or corrupt `session.json` throws a raw libsodium error out of the CLI with no indication that the session file is the problem. `bin/quak.ts` compounds this by swallowing session-load errors entirely, so a corrupt session is indistinguishable from "not logged in". 3. **`logout()` does not clear key material.** `src/client.ts:148-151` flips a flag and clears the API token, but `masterKey`, `secretKey` and `publicKey` stay live in memory. For a client whose whole job is holding the keys to someone's photo library, logout should mean the keys are gone. ## Definition of done 1. `ApiClient` exposes a proper accessor for the current auth token; `Client.toJSON` uses it and no longer indexes into a private field or uses a non-null assertion. 2. `toJSON()` on a client with no auth token fails with a clear error rather than emitting a snapshot containing `undefined`. 3. `fromJSON` validates the snapshot: required fields present, correct types, base64 decoding to the expected byte lengths for each key. An invalid snapshot throws an error that names the offending field and says the session data is invalid. 4. `bin/quak.ts` distinguishes "no session file" from "session file is corrupt" and tells the user which one it is, with a non-zero exit code for the corrupt case. 5. `logout()` zeroes the key material buffers in addition to clearing the token, and any subsequent operation on the logged-out client throws the same way it does today. 6. Tests cover each of the above, including `toJSON` round-tripping through `fromJSON` unchanged, and every rejection case in item 3. 7. `make check` green. 8. `TODO.md` updated in the same commit.

clawbot added this to the 1.0.0 milestone 2026-08-09 03:45:01 +02:00

clawbot self-assigned this 2026-08-09 03:45:01 +02:00

clawbot referenced this issue

2026-08-09 03:47:27 +02:00

Tag v1.0.0 #15

clawbot referenced this issue

2026-08-09 08:05:06 +02:00

Retry transient network failures with exponential backoff (closes #2) #23

Sign in to join this conversation.