Skip to content

Commit 875dc07

Browse files
committed
fix(api): harden token handling after code review
Addresses seven findings from reviewing the client-verification feature: - Percent-decode the ?token= query carrier. The web client escapes the token in the SSE URL, so a CCTRACE_API_TOKEN containing URL-reserved characters used to fail only on the EventSource path. - Give the unpersisted fallback token its own source, "ephemeral", instead of reporting it as env-provided. Settings now explains that the token file could not be written rather than blaming CCTRACE_API_TOKEN, and Regenerate is disabled with an accurate message. - Heal multi-process divergence: on a token mismatch the middleware re-reads the token file once and adopts a rotated value before rejecting, so a second cctrace process sharing the file no longer returns 401 until restarted. Happy-path requests never touch the disk. - Rotate atomically via a 0600 temp file and rename, so concurrent readers (the TUI re-reads per request, Vite watches the file) never observe an empty file. The first-run reader also retries briefly while the O_EXCL creator is still writing. - The Node helper no longer fabricates a token when the file exists but is empty; it retries briefly, then throws so Vite fails loudly instead of baking a token no backend accepts. - MessageDetail drops the saved main-body scroll offset when a panel is replaced at the same depth, instead of restoring a stale position on the next unrelated push or pop. - The static-fallback cookie middleware resolves the origin allowlist only for HTML responses, not for every asset request. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01ARMV5uP7TQ9NyXsqAjpo2E
1 parent 7f90c2a commit 875dc07

11 files changed

Lines changed: 558 additions & 101 deletions

File tree

bin/api-token.mjs

Lines changed: 30 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,26 @@ function readToken(path) {
4343
}
4444
}
4545

46+
/** Synchronous pause — Vite's `config` hook is synchronous, and the window
47+
* between the backend's O_EXCL create and its write is a few milliseconds. */
48+
function pauseMs(ms) {
49+
Atomics.wait(new Int32Array(new SharedArrayBuffer(4)), 0, 0, ms);
50+
}
51+
52+
const EMPTY_FILE_RETRIES = 10;
53+
const EMPTY_FILE_RETRY_MS = 20;
54+
55+
/** Re-read a file another creator has just made, tolerating the moment
56+
* between its create and its write. */
57+
function readTokenWithRetry(path) {
58+
for (let attempt = 0; attempt <= EMPTY_FILE_RETRIES; attempt++) {
59+
const t = readToken(path);
60+
if (t) return t;
61+
if (attempt < EMPTY_FILE_RETRIES) pauseMs(EMPTY_FILE_RETRY_MS);
62+
}
63+
return null;
64+
}
65+
4666
/**
4767
* Resolve the token the web UI should send, or `null` when verification is
4868
* off. Creates the token file (mode 0600) when it does not exist yet.
@@ -63,7 +83,16 @@ export function resolveApiToken(opts = {}) {
6383
writeFileSync(path, `${fresh}\n`, { flag: "wx", mode: 0o600 });
6484
return fresh;
6585
} catch (err) {
66-
if (err && err.code === "EEXIST") return readToken(path) ?? fresh;
86+
if (err && err.code === "EEXIST") {
87+
const winner = readTokenWithRetry(path);
88+
if (winner) return winner;
89+
// Never hand the UI a token no backend accepts: fail loudly instead, the
90+
// same way the Rust side treats an empty token file.
91+
throw new Error(
92+
`api-token file exists but is empty: ${path} — delete it and restart, or set CCTRACE_API_TOKEN`,
93+
{ cause: err },
94+
);
95+
}
6796
throw err;
6897
}
6998
}

bin/api-token.test.mjs

Lines changed: 22 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,11 @@ const enoent = () => {
1919
e.code = "ENOENT";
2020
throw e;
2121
};
22+
const eexist = () => {
23+
const e = new Error("EEXIST");
24+
e.code = "EEXIST";
25+
throw e;
26+
};
2227

2328
beforeEach(() => {
2429
vi.clearAllMocks();
@@ -96,14 +101,26 @@ describe("resolveApiToken", () => {
96101

97102
it("re-reads the winner's token when the backend created the file first (EEXIST)", () => {
98103
readFileSync.mockImplementationOnce(enoent).mockImplementation(() => "winner\n");
99-
writeFileSync.mockImplementation(() => {
100-
const e = new Error("EEXIST");
101-
e.code = "EEXIST";
102-
throw e;
103-
});
104+
writeFileSync.mockImplementation(eexist);
104105
expect(resolveApiToken({ ...opts, env: {} })).toBe("winner");
105106
});
106107

108+
it("waits for the winner to finish writing when the file is momentarily empty", () => {
109+
readFileSync
110+
.mockImplementationOnce(enoent) // initial probe: not there yet
111+
.mockImplementationOnce(() => "") // EEXIST: created but not yet written
112+
.mockImplementationOnce(() => "\n")
113+
.mockImplementation(() => "winner\n");
114+
writeFileSync.mockImplementation(eexist);
115+
expect(resolveApiToken({ ...opts, env: {} })).toBe("winner");
116+
});
117+
118+
it("throws instead of fabricating a token when the file stays empty", () => {
119+
readFileSync.mockImplementationOnce(enoent).mockImplementation(() => "");
120+
writeFileSync.mockImplementation(eexist);
121+
expect(() => resolveApiToken({ ...opts, env: {} })).toThrow(/exists but is empty/);
122+
});
123+
107124
it("rethrows unexpected write errors", () => {
108125
writeFileSync.mockImplementation(() => {
109126
const e = new Error("EACCES");

specs/04-http-api.md

Lines changed: 17 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -77,8 +77,9 @@ state (see [Authentication](#authentication)).
7777
}
7878
```
7979

80-
`api_token_source` is `"file"` (rotatable from Settings), `"env"` (`CCTRACE_API_TOKEN`, read-only)
81-
or `"disabled"` (`CCTRACE_API_AUTH=off`, `api_token` is `null`).
80+
`api_token_source` is `"file"` (rotatable from Settings), `"env"` (`CCTRACE_API_TOKEN`, read-only),
81+
`"ephemeral"` (the token file was unusable at startup; one-off, read-only) or `"disabled"`
82+
(`CCTRACE_API_AUTH=off`, `api_token` is `null`).
8283

8384
---
8485

@@ -401,11 +402,18 @@ over IPC, never HTTP.
401402

402403
Creation uses `O_EXCL`: in `cctrace --web`, Tauri starts Vite _before_ the Rust binary, and the
403404
Vite plugin (`bin/api-token.mjs`) may create the file first. Whichever side loses the race re-reads
404-
the winner's token, so both converge. If the file cannot be read or created the server fails
405-
**closed** — it runs with an unpersisted random token, never unauthenticated.
405+
the winner's token (retrying briefly while the winner is still writing), so both converge. If the
406+
file cannot be read or created the server fails **closed**: it runs with an unpersisted one-off token
407+
(`api_token_source: "ephemeral"`), never unauthenticated, and Settings says so rather than blaming
408+
`CCTRACE_API_TOKEN`. The Vite side throws instead, so the dev server fails loudly rather than baking
409+
a token no backend accepts.
406410

407411
`AppState.api_auth: RwLock<ApiAuth>` holds the live value; the middleware reads it on every request
408-
so a rotation takes effect immediately.
412+
so a rotation takes effect immediately. Rotation writes a sibling temp file and renames it over the
413+
token file, so concurrent readers never see it empty. When a request's token does **not** match and
414+
the live token came from the file, the middleware re-reads the file once before rejecting: if another
415+
cctrace process (a background `--web` service next to the desktop app, say) rotated it, the live
416+
token heals on the spot instead of every client getting 401 until a restart.
409417

410418
### Accepted carriers (`auth::require_api_token`)
411419

@@ -446,9 +454,10 @@ still 404 rather than 401.
446454

447455
### Settings UI
448456

449-
`GET /api/settings` reports `api_auth_enabled`, `api_token_source` (`"file" | "env" | "disabled"`)
450-
and `api_token`. The Settings modal's **API access** section shows the token masked with Show /
451-
Copy / Regenerate; Regenerate is a two-click confirm and is disabled for `env` tokens.
457+
`GET /api/settings` reports `api_auth_enabled`, `api_token_source`
458+
(`"file" | "env" | "ephemeral" | "disabled"`) and `api_token`. The Settings modal's **API access**
459+
section shows the token masked with Show / Copy / Regenerate; Regenerate is a two-click confirm and
460+
is disabled for `env` and `ephemeral` tokens.
452461
`POST /api/settings/token/regenerate` (or the `regenerate_api_token` Tauri command) performs the
453462
rotation.
454463

0 commit comments

Comments
 (0)