From 7056b1f7f1a6b435cf8023d8a5618d4c21990640 Mon Sep 17 00:00:00 2001 From: Felix Kotschenreuther Date: Tue, 7 Jul 2026 12:48:44 +0200 Subject: [PATCH 1/2] =?UTF-8?q?feat:=20Phase=201=20hardening=20=E2=80=94?= =?UTF-8?q?=20Keychain=20token=20store=20+=20HTTP=20retry?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Token store is now host-keyed and macOS Keychain-backed (via the `security` CLI), falling back to the 0600 credentials file on non-macOS or when Keychain is unavailable. readToken precedence: CT_LOGINTOKEN env -> Keychain -> file. - New `src/api/http.ts`: bounded retry with exponential backoff + jitter, honouring `Retry-After`. Only idempotent GET/HEAD retry on 5xx/network; writes retry solely on 429 (server rejected before processing) — never blindly repeat a possibly-applied write. Wired into every CtClient call. - Defer the typed client: hand-written client stays (generated schema.d.ts is 8.2 MB; kept gitignored, generate on demand). - Tests: +7 (retry matrix, Retry-After, write-safety, env precedence) = 15. Refs #3 --- src/api/ctClient.ts | 29 ++++++++---- src/api/http.ts | 77 +++++++++++++++++++++++++++++++ src/api/session.ts | 6 ++- src/auth/tokenStore.ts | 99 +++++++++++++++++++++++++++++++++------- src/commands/auth.ts | 9 ++-- tests/http.test.ts | 68 +++++++++++++++++++++++++++ tests/tokenStore.test.ts | 19 ++++++++ 7 files changed, 276 insertions(+), 31 deletions(-) create mode 100644 src/api/http.ts create mode 100644 tests/http.test.ts create mode 100644 tests/tokenStore.test.ts diff --git a/src/api/ctClient.ts b/src/api/ctClient.ts index 3a06f21..d6a135f 100644 --- a/src/api/ctClient.ts +++ b/src/api/ctClient.ts @@ -14,6 +14,7 @@ * here can be swapped for `openapi-fetch` while keeping this class's surface. */ import { resolveConfig, type CtConfig } from "../config.js"; +import { fetchWithRetry } from "./http.js"; export interface WhoAmI { id: number; @@ -48,7 +49,11 @@ export class CtClient { /** Run the login-token handshake and cache the session cookie + CSRF token. */ async authenticate(loginToken: string): Promise { const url = `${this.config.host}/api/whoami?login_token=${encodeURIComponent(loginToken)}`; - const res = await fetch(url, { headers: { Accept: "application/json" } }); + const res = await fetchWithRetry( + url, + { headers: { Accept: "application/json" } }, + { isIdempotent: true }, + ); this.captureCookie(res); if (!res.ok) { throw new CtApiError(`Login failed (whoami)`, res.status, await safeBody(res)); @@ -82,11 +87,15 @@ export class CtClient { if (body !== undefined) { headers["Content-Type"] = "application/json"; } - const res = await fetch(`${this.config.host}/api${path}`, { - method, - headers, - body: body !== undefined ? JSON.stringify(body) : undefined, - }); + const res = await fetchWithRetry( + `${this.config.host}/api${path}`, + { + method, + headers, + body: body !== undefined ? JSON.stringify(body) : undefined, + }, + { isIdempotent: method === "GET" || method === "HEAD" }, + ); this.captureCookie(res); if (!res.ok) { throw new CtApiError(`${method} ${path} failed`, res.status, await safeBody(res)); @@ -102,9 +111,11 @@ export class CtClient { if (!this.cookie) { return; } - const res = await fetch(`${this.config.host}/api/csrftoken`, { - headers: { Accept: "application/json", Cookie: this.cookie }, - }); + const res = await fetchWithRetry( + `${this.config.host}/api/csrftoken`, + { headers: { Accept: "application/json", Cookie: this.cookie } }, + { isIdempotent: true }, + ); this.captureCookie(res); if (!res.ok) { throw new CtApiError("Failed to fetch CSRF token", res.status, await safeBody(res)); diff --git a/src/api/http.ts b/src/api/http.ts new file mode 100644 index 0000000..e18caa8 --- /dev/null +++ b/src/api/http.ts @@ -0,0 +1,77 @@ +/** + * HTTP with rate-limit awareness and bounded retry. + * + * ChurchTools rate-limits bursts (HTTP 429) and can return transient 5xx. This + * wrapper retries with exponential backoff + jitter, honouring a `Retry-After` + * header when present. + * + * Safety: only **idempotent** requests (GET/HEAD) are retried on 5xx or network + * errors — a write that may have already been applied is never blindly repeated. + * A 429 is always safe to retry: the server rejected the request before + * processing it. + */ + +export interface RetryOptions { + /** Max additional attempts after the first (total attempts = retries + 1). */ + retries?: number; + baseDelayMs?: number; + /** GET/HEAD are idempotent; writes are not. Controls 5xx/network retry. */ + isIdempotent?: boolean; + sleep?: (ms: number) => Promise; + fetchImpl?: typeof fetch; +} + +const defaultSleep = (ms: number): Promise => new Promise((resolve) => setTimeout(resolve, ms)); + +function backoffMs(base: number, attempt: number): number { + const exp = base * 2 ** (attempt - 1); + return exp + Math.floor(Math.random() * base); +} + +function retryAfterMs(res: Response): number | null { + const header = res.headers.get("retry-after"); + if (!header) { + return null; + } + const seconds = Number.parseInt(header, 10); + return Number.isFinite(seconds) ? seconds * 1000 : null; +} + +function shouldRetryStatus(status: number, isIdempotent: boolean): boolean { + if (status === 429) { + return true; + } + return status >= 500 && isIdempotent; +} + +export async function fetchWithRetry( + input: string | URL, + init: RequestInit, + opts: RetryOptions = {}, +): Promise { + const retries = opts.retries ?? 3; + const base = opts.baseDelayMs ?? 500; + const isIdempotent = opts.isIdempotent ?? true; + const sleep = opts.sleep ?? defaultSleep; + const doFetch = opts.fetchImpl ?? fetch; + + let attempt = 0; + for (;;) { + attempt++; + let res: Response; + try { + res = await doFetch(input, init); + } catch (err) { + if (attempt <= retries && isIdempotent) { + await sleep(backoffMs(base, attempt)); + continue; + } + throw err; + } + if (attempt <= retries && shouldRetryStatus(res.status, isIdempotent)) { + await sleep(retryAfterMs(res) ?? backoffMs(base, attempt)); + continue; + } + return res; + } +} diff --git a/src/api/session.ts b/src/api/session.ts index 2c93c5a..79a6b9e 100644 --- a/src/api/session.ts +++ b/src/api/session.ts @@ -4,6 +4,7 @@ */ import { CtClient, type WhoAmI } from "./ctClient.js"; import { readToken } from "../auth/tokenStore.js"; +import { resolveConfig } from "../config.js"; export interface AuthedSession { client: CtClient; @@ -11,11 +12,12 @@ export interface AuthedSession { } export async function authedSession(): Promise { - const token = await readToken(); + const config = resolveConfig(); + const token = await readToken(config.host); if (!token) { throw new Error("Not logged in. Run `ct auth login --token ` first."); } - const client = new CtClient(); + const client = new CtClient(config); const me = await client.authenticate(token); return { client, me }; } diff --git a/src/auth/tokenStore.ts b/src/auth/tokenStore.ts index 3473d44..fbfb26f 100644 --- a/src/auth/tokenStore.ts +++ b/src/auth/tokenStore.ts @@ -1,23 +1,36 @@ /** - * Persistence for the personal ChurchTools login token. + * Persistence for the personal ChurchTools login token, keyed by host so one + * machine can hold tokens for several instances. * - * Precedence when reading: + * Read precedence: * 1. `CT_LOGINTOKEN` environment variable (CI / one-off use) - * 2. credentials file at `~/.config/ct-cli/credentials.json` + * 2. macOS Keychain (via the `security` CLI), when on darwin + * 3. credentials file at `~/.config/ct-cli/credentials.json` (0600 fallback) * - * TODO(Phase 1, #3): move the file store behind the macOS Keychain (e.g. - * `security add-generic-password`) and keep the file only as a non-macOS - * fallback. The interface below stays the same so callers don't change. + * The Keychain is preferred on macOS; the file store is the cross-platform + * fallback and also catches tokens written before Keychain support existed. + * + * Note: `security ... -w ` passes the token as an argv, briefly visible + * to `ps`. Acceptable for a local developer CLI; the value never touches git. */ -import { homedir } from "node:os"; +import { homedir, platform } from "node:os"; import { join } from "node:path"; import { mkdir, readFile, writeFile, rm, chmod } from "node:fs/promises"; +import { execFile } from "node:child_process"; +import { promisify } from "node:util"; + +const run = promisify(execFile); +const KEYCHAIN_SERVICE = "ct-cli"; interface Credentials { host: string; token: string; } +function isMac(): boolean { + return platform() === "darwin"; +} + function configDir(): string { const base = process.env.XDG_CONFIG_HOME?.trim() || join(homedir(), ".config"); return join(base, "ct-cli"); @@ -27,9 +40,36 @@ function credentialsPath(): string { return join(configDir(), "credentials.json"); } -export async function storeToken(host: string, token: string): Promise { - const dir = configDir(); - await mkdir(dir, { recursive: true }); +async function keychainSet(host: string, token: string): Promise { + await run("security", ["add-generic-password", "-U", "-s", KEYCHAIN_SERVICE, "-a", host, "-w", token]); +} + +async function keychainGet(host: string): Promise { + try { + const { stdout } = await run("security", [ + "find-generic-password", + "-s", + KEYCHAIN_SERVICE, + "-a", + host, + "-w", + ]); + return stdout.trim() || null; + } catch { + return null; + } +} + +async function keychainDelete(host: string): Promise { + try { + await run("security", ["delete-generic-password", "-s", KEYCHAIN_SERVICE, "-a", host]); + } catch { + /* not present — nothing to delete */ + } +} + +async function fileStore(host: string, token: string): Promise { + await mkdir(configDir(), { recursive: true }); const path = credentialsPath(); const payload: Credentials = { host, token }; await writeFile(path, `${JSON.stringify(payload, null, 2)}\n`, { mode: 0o600 }); @@ -37,11 +77,7 @@ export async function storeToken(host: string, token: string): Promise { return path; } -export async function readToken(): Promise { - const fromEnv = process.env.CT_LOGINTOKEN?.trim(); - if (fromEnv) { - return fromEnv; - } +async function fileRead(): Promise { try { const raw = await readFile(credentialsPath(), "utf8"); const parsed = JSON.parse(raw) as Partial; @@ -51,6 +87,37 @@ export async function readToken(): Promise { } } -export async function clearToken(): Promise { +/** Persist the token; returns a human-readable description of where it landed. */ +export async function storeToken(host: string, token: string): Promise { + if (isMac()) { + try { + await keychainSet(host, token); + return `macOS Keychain (service "${KEYCHAIN_SERVICE}", account "${host}")`; + } catch { + /* fall back to file */ + } + } + const path = await fileStore(host, token); + return `${path} (mode 0600)`; +} + +export async function readToken(host: string): Promise { + const fromEnv = process.env.CT_LOGINTOKEN?.trim(); + if (fromEnv) { + return fromEnv; + } + if (isMac()) { + const fromKeychain = await keychainGet(host); + if (fromKeychain) { + return fromKeychain; + } + } + return fileRead(); +} + +export async function clearToken(host: string): Promise { + if (isMac()) { + await keychainDelete(host); + } await rm(credentialsPath(), { force: true }); } diff --git a/src/commands/auth.ts b/src/commands/auth.ts index 66a7520..a2aa890 100644 --- a/src/commands/auth.ts +++ b/src/commands/auth.ts @@ -23,9 +23,9 @@ export function authCommand(): Command { } const client = new CtClient(config); const me = await client.authenticate(token); - const path = await storeToken(config.host, token); + const location = await storeToken(config.host, token); success(`Logged in to ${config.host} as ${me.firstName ?? ""} ${me.lastName ?? ""} (#${me.id})`.trim()); - info(`Token stored at ${path} (mode 0600).`); + info(`Token stored in ${location}.`); const ctInfo = await client.get("/info"); if (ctInfo.version) { @@ -43,7 +43,8 @@ export function authCommand(): Command { .command("status") .description("Show the currently authenticated user") .action(async () => { - if (!(await readToken())) { + const config = resolveConfig(); + if (!(await readToken(config.host))) { error("Not logged in. Run `ct auth login --token `."); process.exitCode = 1; return; @@ -56,7 +57,7 @@ export function authCommand(): Command { .command("logout") .description("Remove the stored login token") .action(async () => { - await clearToken(); + await clearToken(resolveConfig().host); success("Logged out — stored token removed."); }); diff --git a/tests/http.test.ts b/tests/http.test.ts new file mode 100644 index 0000000..f79ca53 --- /dev/null +++ b/tests/http.test.ts @@ -0,0 +1,68 @@ +import { describe, it, expect, vi } from "vitest"; +import { fetchWithRetry } from "../src/api/http.js"; + +const noSleep = () => Promise.resolve(); + +function res(status: number, headers: Record = {}): Response { + return new Response(status === 204 ? null : "{}", { status, headers }); +} + +describe("fetchWithRetry", () => { + it("retries a 429 then succeeds", async () => { + const fetchImpl = vi.fn().mockResolvedValueOnce(res(429)).mockResolvedValueOnce(res(200)); + const out = await fetchWithRetry("https://x/api/campuses", {}, { sleep: noSleep, fetchImpl }); + expect(out.status).toBe(200); + expect(fetchImpl).toHaveBeenCalledTimes(2); + }); + + it("honours Retry-After for the wait duration", async () => { + const fetchImpl = vi + .fn() + .mockResolvedValueOnce(res(429, { "retry-after": "2" })) + .mockResolvedValueOnce(res(200)); + const sleep = vi.fn(() => Promise.resolve()); + await fetchWithRetry("https://x/api/campuses", {}, { sleep, fetchImpl }); + expect(sleep).toHaveBeenCalledWith(2000); + }); + + it("retries idempotent GET on 500", async () => { + const fetchImpl = vi.fn().mockResolvedValueOnce(res(500)).mockResolvedValueOnce(res(200)); + const out = await fetchWithRetry( + "https://x/api/campuses", + {}, + { sleep: noSleep, fetchImpl, isIdempotent: true }, + ); + expect(out.status).toBe(200); + expect(fetchImpl).toHaveBeenCalledTimes(2); + }); + + it("does NOT retry a write on 500", async () => { + const fetchImpl = vi.fn().mockResolvedValueOnce(res(500)); + const out = await fetchWithRetry( + "https://x/api/campuses", + { method: "POST" }, + { sleep: noSleep, fetchImpl, isIdempotent: false }, + ); + expect(out.status).toBe(500); + expect(fetchImpl).toHaveBeenCalledTimes(1); + }); + + it("gives up after the retry budget and returns the last response", async () => { + const fetchImpl = vi.fn().mockResolvedValue(res(429)); + const out = await fetchWithRetry("https://x/api/campuses", {}, { retries: 2, sleep: noSleep, fetchImpl }); + expect(out.status).toBe(429); + expect(fetchImpl).toHaveBeenCalledTimes(3); + }); + + it("does not retry a write on a network error", async () => { + const fetchImpl = vi.fn().mockRejectedValue(new Error("ECONNRESET")); + await expect( + fetchWithRetry( + "https://x/api/campuses", + { method: "POST" }, + { sleep: noSleep, fetchImpl, isIdempotent: false }, + ), + ).rejects.toThrow("ECONNRESET"); + expect(fetchImpl).toHaveBeenCalledTimes(1); + }); +}); diff --git a/tests/tokenStore.test.ts b/tests/tokenStore.test.ts new file mode 100644 index 0000000..22a7404 --- /dev/null +++ b/tests/tokenStore.test.ts @@ -0,0 +1,19 @@ +import { describe, it, expect, afterEach } from "vitest"; +import { readToken } from "../src/auth/tokenStore.js"; + +const original = process.env.CT_LOGINTOKEN; + +afterEach(() => { + if (original === undefined) { + delete process.env.CT_LOGINTOKEN; + } else { + process.env.CT_LOGINTOKEN = original; + } +}); + +describe("readToken", () => { + it("prefers the CT_LOGINTOKEN env var over any store", async () => { + process.env.CT_LOGINTOKEN = "env-token"; + await expect(readToken("https://eqrm.church.tools")).resolves.toBe("env-token"); + }); +}); From 193cd83e80a4ae1ec53063e87c3d2b4be250e54a Mon Sep 17 00:00:00 2001 From: Felix Kotschenreuther Date: Tue, 7 Jul 2026 13:09:18 +0200 Subject: [PATCH 2/2] fix: Keychain-only single-token store + harden Retry-After Address PR #8 review findings: - Token store: drop the file fallback and host-keying entirely. A single token now lives in the macOS Keychain (fixed account); CI/non-mac hosts use CT_LOGINTOKEN. Removes the bug where the non-host-keyed file fallback could return or overwrite another host's token. - http: validate and clamp Retry-After. Cap any single wait at 60s (an outsized value no longer hangs the CLI), reject negative/malformed values (they fell through to a 0ms immediate retry), and honour the HTTP-date form. - http: drain the body of discarded intermediate responses before retrying. - Tests: cover Retry-After cap + malformed fallback; update token-store test. --- src/api/http.ts | 35 +++++++++++-- src/api/session.ts | 2 +- src/auth/tokenStore.ts | 105 ++++++++++++++------------------------- src/commands/auth.ts | 7 ++- tests/http.test.ts | 22 ++++++++ tests/tokenStore.test.ts | 2 +- 6 files changed, 93 insertions(+), 80 deletions(-) diff --git a/src/api/http.ts b/src/api/http.ts index e18caa8..4ccf99e 100644 --- a/src/api/http.ts +++ b/src/api/http.ts @@ -21,20 +21,42 @@ export interface RetryOptions { fetchImpl?: typeof fetch; } +/** Cap on any single wait, so an outsized `Retry-After` can't hang the CLI. */ +const MAX_DELAY_MS = 60_000; + const defaultSleep = (ms: number): Promise => new Promise((resolve) => setTimeout(resolve, ms)); +function clampDelay(ms: number): number { + if (!Number.isFinite(ms)) { + return 0; + } + return Math.min(Math.max(ms, 0), MAX_DELAY_MS); +} + function backoffMs(base: number, attempt: number): number { const exp = base * 2 ** (attempt - 1); - return exp + Math.floor(Math.random() * base); + return clampDelay(exp + Math.floor(Math.random() * base)); } function retryAfterMs(res: Response): number | null { - const header = res.headers.get("retry-after"); + const header = res.headers.get("retry-after")?.trim(); if (!header) { return null; } - const seconds = Number.parseInt(header, 10); - return Number.isFinite(seconds) ? seconds * 1000 : null; + // `Retry-After` is either a non-negative delta-seconds count or an HTTP-date. + if (/^\d+$/.test(header)) { + return clampDelay(Number.parseInt(header, 10) * 1000); + } + // Only treat it as a date when it actually looks like one — `Date.parse` is + // lax enough to accept e.g. "-5" as a year, which must not become a 0ms wait. + if (/[a-zA-Z]/.test(header)) { + const dateMs = Date.parse(header); + if (Number.isFinite(dateMs)) { + return clampDelay(dateMs - Date.now()); + } + } + // Unparseable (e.g. a negative or malformed value): fall back to backoff. + return null; } function shouldRetryStatus(status: number, isIdempotent: boolean): boolean { @@ -69,7 +91,10 @@ export async function fetchWithRetry( throw err; } if (attempt <= retries && shouldRetryStatus(res.status, isIdempotent)) { - await sleep(retryAfterMs(res) ?? backoffMs(base, attempt)); + const delay = retryAfterMs(res) ?? backoffMs(base, attempt); + // Drain the response we're discarding so its socket isn't left buffered. + await res.body?.cancel().catch(() => {}); + await sleep(delay); continue; } return res; diff --git a/src/api/session.ts b/src/api/session.ts index 79a6b9e..ebd2c8f 100644 --- a/src/api/session.ts +++ b/src/api/session.ts @@ -13,7 +13,7 @@ export interface AuthedSession { export async function authedSession(): Promise { const config = resolveConfig(); - const token = await readToken(config.host); + const token = await readToken(); if (!token) { throw new Error("Not logged in. Run `ct auth login --token ` first."); } diff --git a/src/auth/tokenStore.ts b/src/auth/tokenStore.ts index fbfb26f..812d500 100644 --- a/src/auth/tokenStore.ts +++ b/src/auth/tokenStore.ts @@ -1,57 +1,50 @@ /** - * Persistence for the personal ChurchTools login token, keyed by host so one - * machine can hold tokens for several instances. + * Persistence for the personal ChurchTools login token. + * + * A single token is stored in the macOS Keychain (via the `security` CLI). + * There is no file fallback: on CI or non-macOS hosts, supply the token through + * the `CT_LOGINTOKEN` environment variable instead. * * Read precedence: * 1. `CT_LOGINTOKEN` environment variable (CI / one-off use) - * 2. macOS Keychain (via the `security` CLI), when on darwin - * 3. credentials file at `~/.config/ct-cli/credentials.json` (0600 fallback) - * - * The Keychain is preferred on macOS; the file store is the cross-platform - * fallback and also catches tokens written before Keychain support existed. + * 2. macOS Keychain * * Note: `security ... -w ` passes the token as an argv, briefly visible * to `ps`. Acceptable for a local developer CLI; the value never touches git. */ -import { homedir, platform } from "node:os"; -import { join } from "node:path"; -import { mkdir, readFile, writeFile, rm, chmod } from "node:fs/promises"; +import { platform } from "node:os"; import { execFile } from "node:child_process"; import { promisify } from "node:util"; const run = promisify(execFile); const KEYCHAIN_SERVICE = "ct-cli"; - -interface Credentials { - host: string; - token: string; -} +const KEYCHAIN_ACCOUNT = "login-token"; function isMac(): boolean { return platform() === "darwin"; } -function configDir(): string { - const base = process.env.XDG_CONFIG_HOME?.trim() || join(homedir(), ".config"); - return join(base, "ct-cli"); +async function keychainSet(token: string): Promise { + await run("security", [ + "add-generic-password", + "-U", + "-s", + KEYCHAIN_SERVICE, + "-a", + KEYCHAIN_ACCOUNT, + "-w", + token, + ]); } -function credentialsPath(): string { - return join(configDir(), "credentials.json"); -} - -async function keychainSet(host: string, token: string): Promise { - await run("security", ["add-generic-password", "-U", "-s", KEYCHAIN_SERVICE, "-a", host, "-w", token]); -} - -async function keychainGet(host: string): Promise { +async function keychainGet(): Promise { try { const { stdout } = await run("security", [ "find-generic-password", "-s", KEYCHAIN_SERVICE, "-a", - host, + KEYCHAIN_ACCOUNT, "-w", ]); return stdout.trim() || null; @@ -60,64 +53,38 @@ async function keychainGet(host: string): Promise { } } -async function keychainDelete(host: string): Promise { +async function keychainDelete(): Promise { try { - await run("security", ["delete-generic-password", "-s", KEYCHAIN_SERVICE, "-a", host]); + await run("security", ["delete-generic-password", "-s", KEYCHAIN_SERVICE, "-a", KEYCHAIN_ACCOUNT]); } catch { /* not present — nothing to delete */ } } -async function fileStore(host: string, token: string): Promise { - await mkdir(configDir(), { recursive: true }); - const path = credentialsPath(); - const payload: Credentials = { host, token }; - await writeFile(path, `${JSON.stringify(payload, null, 2)}\n`, { mode: 0o600 }); - await chmod(path, 0o600); - return path; -} - -async function fileRead(): Promise { - try { - const raw = await readFile(credentialsPath(), "utf8"); - const parsed = JSON.parse(raw) as Partial; - return parsed.token?.trim() || null; - } catch { - return null; - } -} - -/** Persist the token; returns a human-readable description of where it landed. */ -export async function storeToken(host: string, token: string): Promise { - if (isMac()) { - try { - await keychainSet(host, token); - return `macOS Keychain (service "${KEYCHAIN_SERVICE}", account "${host}")`; - } catch { - /* fall back to file */ - } +/** Persist the token in the macOS Keychain; returns a human-readable location. */ +export async function storeToken(token: string): Promise { + if (!isMac()) { + throw new Error( + "Token storage requires the macOS Keychain. On other platforms, set CT_LOGINTOKEN instead.", + ); } - const path = await fileStore(host, token); - return `${path} (mode 0600)`; + await keychainSet(token); + return `macOS Keychain (service "${KEYCHAIN_SERVICE}", account "${KEYCHAIN_ACCOUNT}")`; } -export async function readToken(host: string): Promise { +export async function readToken(): Promise { const fromEnv = process.env.CT_LOGINTOKEN?.trim(); if (fromEnv) { return fromEnv; } if (isMac()) { - const fromKeychain = await keychainGet(host); - if (fromKeychain) { - return fromKeychain; - } + return keychainGet(); } - return fileRead(); + return null; } -export async function clearToken(host: string): Promise { +export async function clearToken(): Promise { if (isMac()) { - await keychainDelete(host); + await keychainDelete(); } - await rm(credentialsPath(), { force: true }); } diff --git a/src/commands/auth.ts b/src/commands/auth.ts index a2aa890..6e1aac1 100644 --- a/src/commands/auth.ts +++ b/src/commands/auth.ts @@ -23,7 +23,7 @@ export function authCommand(): Command { } const client = new CtClient(config); const me = await client.authenticate(token); - const location = await storeToken(config.host, token); + const location = await storeToken(token); success(`Logged in to ${config.host} as ${me.firstName ?? ""} ${me.lastName ?? ""} (#${me.id})`.trim()); info(`Token stored in ${location}.`); @@ -43,8 +43,7 @@ export function authCommand(): Command { .command("status") .description("Show the currently authenticated user") .action(async () => { - const config = resolveConfig(); - if (!(await readToken(config.host))) { + if (!(await readToken())) { error("Not logged in. Run `ct auth login --token `."); process.exitCode = 1; return; @@ -57,7 +56,7 @@ export function authCommand(): Command { .command("logout") .description("Remove the stored login token") .action(async () => { - await clearToken(resolveConfig().host); + await clearToken(); success("Logged out — stored token removed."); }); diff --git a/tests/http.test.ts b/tests/http.test.ts index f79ca53..fa0565a 100644 --- a/tests/http.test.ts +++ b/tests/http.test.ts @@ -25,6 +25,28 @@ describe("fetchWithRetry", () => { expect(sleep).toHaveBeenCalledWith(2000); }); + it("caps an outsized Retry-After so the CLI can't hang for hours", async () => { + const fetchImpl = vi + .fn() + .mockResolvedValueOnce(res(429, { "retry-after": "86400" })) + .mockResolvedValueOnce(res(200)); + const sleep = vi.fn(() => Promise.resolve()); + await fetchWithRetry("https://x/api/campuses", {}, { sleep, fetchImpl }); + expect(sleep).toHaveBeenCalledWith(60_000); + }); + + it("ignores a malformed Retry-After and falls back to backoff", async () => { + const fetchImpl = vi + .fn() + .mockResolvedValueOnce(res(429, { "retry-after": "-5" })) + .mockResolvedValueOnce(res(200)); + const sleep = vi.fn<(ms: number) => Promise>(() => Promise.resolve()); + await fetchWithRetry("https://x/api/campuses", {}, { baseDelayMs: 500, sleep, fetchImpl }); + // Backoff, not an immediate (negative → 0) retry. + expect(sleep).toHaveBeenCalledTimes(1); + expect(sleep.mock.calls[0]?.[0] ?? 0).toBeGreaterThanOrEqual(500); + }); + it("retries idempotent GET on 500", async () => { const fetchImpl = vi.fn().mockResolvedValueOnce(res(500)).mockResolvedValueOnce(res(200)); const out = await fetchWithRetry( diff --git a/tests/tokenStore.test.ts b/tests/tokenStore.test.ts index 22a7404..61711c0 100644 --- a/tests/tokenStore.test.ts +++ b/tests/tokenStore.test.ts @@ -14,6 +14,6 @@ afterEach(() => { describe("readToken", () => { it("prefers the CT_LOGINTOKEN env var over any store", async () => { process.env.CT_LOGINTOKEN = "env-token"; - await expect(readToken("https://eqrm.church.tools")).resolves.toBe("env-token"); + await expect(readToken()).resolves.toBe("env-token"); }); });