feat: Phase 1 hardening — Keychain token store + HTTP retry

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<WhoAmI> {
     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));

src/api/http.ts

@@ -0,0 +1,102 @@
+/**
+ * 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<void>;
+  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<void> => 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 clampDelay(exp + Math.floor(Math.random() * base));
+}
+
+function retryAfterMs(res: Response): number | null {
+  const header = res.headers.get("retry-after")?.trim();
+  if (!header) {
+    return 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 {
+  if (status === 429) {
+    return true;
+  }
+  return status >= 500 && isIdempotent;
+}
+
+export async function fetchWithRetry(
+  input: string | URL,
+  init: RequestInit,
+  opts: RetryOptions = {},
+): Promise<Response> {
+  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)) {
+      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;
+  }
+}

src/api/session.ts

@@ -4,18 +4,20 @@
  */
 import { CtClient, type WhoAmI } from "./ctClient.js";
 import { readToken } from "../auth/tokenStore.js";
+import { resolveConfig } from "../config.js";
 
 export interface AuthedSession {
   client: CtClient;
   me: WhoAmI;
 }
 
 export async function authedSession(): Promise<AuthedSession> {
+  const config = resolveConfig();
   const token = await readToken();
   if (!token) {
     throw new Error("Not logged in. Run `ct auth login --token <token>` first.");
   }
-  const client = new CtClient();
+  const client = new CtClient(config);
   const me = await client.authenticate(token);
   return { client, me };
 }

src/auth/tokenStore.ts

@@ -1,56 +1,90 @@
 /**
  * Persistence for the personal ChurchTools login token.
  *
- * Precedence when reading:
+ * 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. credentials file at `~/.config/ct-cli/credentials.json`
+ *   2. macOS Keychain
  *
- * 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.
+ * Note: `security ... -w <token>` 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 { 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";
+const KEYCHAIN_ACCOUNT = "login-token";
+
+function isMac(): boolean {
+  return platform() === "darwin";
+}
 
-interface Credentials {
-  host: string;
-  token: string;
+async function keychainSet(token: string): Promise<void> {
+  await run("security", [
+    "add-generic-password",
+    "-U",
+    "-s",
+    KEYCHAIN_SERVICE,
+    "-a",
+    KEYCHAIN_ACCOUNT,
+    "-w",
+    token,
+  ]);
 }
 
-function configDir(): string {
-  const base = process.env.XDG_CONFIG_HOME?.trim() || join(homedir(), ".config");
-  return join(base, "ct-cli");
+async function keychainGet(): Promise<string | null> {
+  try {
+    const { stdout } = await run("security", [
+      "find-generic-password",
+      "-s",
+      KEYCHAIN_SERVICE,
+      "-a",
+      KEYCHAIN_ACCOUNT,
+      "-w",
+    ]);
+    return stdout.trim() || null;
+  } catch {
+    return null;
+  }
 }
 
-function credentialsPath(): string {
-  return join(configDir(), "credentials.json");
+async function keychainDelete(): Promise<void> {
+  try {
+    await run("security", ["delete-generic-password", "-s", KEYCHAIN_SERVICE, "-a", KEYCHAIN_ACCOUNT]);
+  } catch {
+    /* not present — nothing to delete */
+  }
 }
 
-export async function storeToken(host: string, token: string): Promise<string> {
-  const dir = configDir();
-  await mkdir(dir, { 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;
+/** Persist the token in the macOS Keychain; returns a human-readable location. */
+export async function storeToken(token: string): Promise<string> {
+  if (!isMac()) {
+    throw new Error(
+      "Token storage requires the macOS Keychain. On other platforms, set CT_LOGINTOKEN instead.",
+    );
+  }
+  await keychainSet(token);
+  return `macOS Keychain (service "${KEYCHAIN_SERVICE}", account "${KEYCHAIN_ACCOUNT}")`;
 }
 
 export async function readToken(): Promise<string | null> {
   const fromEnv = process.env.CT_LOGINTOKEN?.trim();
   if (fromEnv) {
     return fromEnv;
   }
-  try {
-    const raw = await readFile(credentialsPath(), "utf8");
-    const parsed = JSON.parse(raw) as Partial<Credentials>;
-    return parsed.token?.trim() || null;
-  } catch {
-    return null;
+  if (isMac()) {
+    return keychainGet();
   }
+  return null;
 }
 
 export async function clearToken(): Promise<void> {
-  await rm(credentialsPath(), { force: true });
+  if (isMac()) {
+    await keychainDelete();
+  }
 }

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(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<CtInfo>("/info");
       if (ctInfo.version) {