Repository navigation
fix: reuse the ChurchTools session across invocations (#145) - #149
Merged
Merged
Conversation
Every `ct` invocation re-ran the login handshake (`whoami?login_token=` +
`csrftoken`), because the session lived in `CtApiClient` instance fields and the
CLI is one-shot. Three `ct get raw` calls in a row were enough to trip CT's
login rate limit, and every command then failed with
`✗ Login failed (whoami) (HTTP 429)` before reaching its endpoint.
The session (cookie + CSRF token) is now cached per host and reused:
- `src/auth/sessionStore.ts` keeps `{host, cookie, csrfToken, obtainedAt,
tokenHash}` in the macOS Keychain under a `session:<host>` account — not a
file. A live session cookie is as powerful as the login token, so it gets the
same protection, and on Linux/Windows (no keychain backend) there is simply no
cache rather than a plaintext cookie on disk.
- Host binding (#30) is enforced twice: the entry is host-keyed AND the blob
re-states its host, checked on read. A session is also bound to a SHA-256
fingerprint of the token that bought it (the hash, never the token).
- `CtClient.authenticate` resumes a cached session and confirms it with a plain
`GET /whoami` (no `login_token`, so no login rate-limit cost) instead of the
handshake; `me` still comes from the server.
- A 401 on a real request drops the cache, re-logs in once and replays the
request, so an expired or server-invalidated session never surfaces as an
error. Capped at one automatic re-login per process.
- `ct auth logout` removes the cached session with the credentials.
- A 429 from the handshake now says the login rate limit was hit and how long to
wait (from `Retry-After`) instead of reading like a rejected credential.
- `parseRetryAfterMs` is split out of `http.ts`'s clamped retry wait so the
message can quote the server's real ask.
Closes #145.
Claude-Session: https://claude.ai/code/session_018JShVZYNLaRb4hF5KbHCXG
…hain read per account Two review findings on the session cache: - `reauthAttempts` was a lifetime cap, so one 401 that was never about the session (CT answers 401 for a CSRF failure too) spent the run's only self-heal — a genuine expiry later in a long apply then surfaced as a bare 401. Reset the budget once a re-login has produced a successful replay; a replay that 401s again still leaves it spent, so an always-401 server cannot become a login storm. - `storeSession` cleared the memoized keychain reads for *every* account, so a fresh login evicted the credential blob's cached read and could re-prompt for a locked Keychain. `keychainSet`/`keychainDelete` now invalidate only the account they touched. Claude-Session: https://claude.ai/code/session_018JShVZYNLaRb4hF5KbHCXG
The page documents `ct get`'s schema reads, which the session self-heal does not touch — re-read against both sources, no content change needed. Claude-Session: https://claude.ai/code/session_018JShVZYNLaRb4hF5KbHCXG
Resolves the #138 overlap: `isSecureStorageAvailable` now sits on the imported `isMac` this branch moved into keychain.ts, and `ct auth login` keeps both the guided token bootstrap and the fresh-but-cached session handshake. Claude-Session: https://claude.ai/code/session_018JShVZYNLaRb4hF5KbHCXG
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The problem
ctis one-shot, but the session it buys lived inCtClientinstance fields — so everyinvocation re-ran
GET /api/whoami?login_token=…+GET /api/csrftoken. ChurchToolsrate-limits logins, so three
ct get rawcalls in a row were enough to be throttled, andevery command then died with
✗ Login failed (whoami) (HTTP 429)before it reached itsendpoint. The login token was persisted; the session it buys was not.
What changed
A per-host session cache (
src/auth/sessionStore.ts) holding{host, cookie, csrfToken, obtainedAt, tokenHash}.Resume instead of handshake.
CtClient.authenticate()loads the cached session, adoptsthe cookie + CSRF token, and confirms them with a plain
GET /whoami— nologin_token, soit is an ordinary authenticated read and costs nothing against the login rate limit. Two
requests (one of them a login) become one non-login request. The identity still comes from
the server rather than from a stale local copy, so
ct auth statusstays honest.Self-healing. A 401 on a real request drops the cached session, re-logs in once and
replays the request, so an expired or server-invalidated session never surfaces to the user.
A 401 means the request was rejected before it was processed, so the replay is safe for
writes too. Capped at one automatic re-login per process, so a server that always 401s
cannot become a login storm — the very failure mode this PR exists to fix.
ct auth loginis exempt (authenticate(token, { fresh: true })): a command whose wholejob is to prove a credential must not be answered from a cache. It still stores the
session it buys.
ct auth logoutdeletes the session with the credentials — inclearCredentials, soboth the plain and the
--envlogout paths get it. A cookie left behind is a live handle onthe instance; leaving it would mean logout had not logged out.
Clearer 429 +
Retry-After.fetchWithRetryalready honouredRetry-After(429 isretried for writes too, capped at 60s per sleep) — that behaviour is now pinned by tests. On
top of it, a 429 from the handshake no longer reads like a rejected credential:
parseRetryAfterMsis split out of the internal clamped wait so the message can quote theserver's real ask instead of the 60s sleep cap.
Storage decision: the Keychain, not a 0600 file
The issue suggested a
0600file next to the credential. I put it in the macOS Keychaininstead, under a separate
session:<host>account in the existingct-cliservice.as it lives it is exactly as powerful as the login token. Same power ⇒ same protection.
The Keychain is encrypted at rest and ACL'd by the OS; a
0600file under$HOMEisneither — it lands in backups, in
tarred home directories, and is readable by everyprocess running as the user.
ct auth logouthas one place toclean and
SECURITY.mdhas one story to tell.keychain.tshas no filefallback, so on Linux/Windows there is no session cache at all — behaviour there is
exactly today's, a handshake per invocation. This repo deliberately refuses to write the
login token to a plaintext file on those platforms; the cookie must not become the
loophole that writes a credential-equivalent there anyway. (Those platforms use
CT_LOGINTOKEN/CT_HOSTand typically run one command per CI job, where the cache buyslittle.)
The keychain plumbing (
securityCLI + the per-process read memoization) moved tosrc/auth/keychain.ts, shared bytokenStoreandsessionStore— no behaviour change,resetKeychainCacheis re-exported fromtokenStoreso existing callers and tests areunaffected.
Host binding (#30) is preserved
The rule from #30 — a stored credential is only ever sent to the host it was captured
against — now covers the session as well, enforced in three places:
session:<host>), andreadSession(host, …)only everlooks under that host's account.
cannot smuggle host A's cookie into a request to host B.
authedSessionbuilds the client after the existing token↔host refusal, with the sameresolved host — so a
CT_HOSToverride that refuses to send the token cannot send acached session either.
Additionally the session is bound to the token that bought it via a SHA-256 fingerprint
(the hash is stored, never the token), so logging in as somebody else does not silently
reuse the previous user's session. Sessions age out after 12h; a clock moved backwards
invalidates rather than extends.
Neither the cookie nor the CSRF token is ever logged, printed or included in an error — a
test pins that the token does not leak into the 429 message either.
Tests
tests/sessionStore.test.ts(store round-trip, own keychain account, token stored only as ahash, host-A-never-for-host-B, mismatched-host blob refused, different-token refused, TTL +
backwards clock, corrupt blob, nothing stored or read on a non-macOS platform, per-host
clear, and logout clearing the session for both logout paths) and
tests/session-cache.test.ts(session written and reused across two client constructionswith the handshake skipped, cached cookie/CSRF carried into later requests, host A's session
never sent to host B, rejected cached session replaced by a real handshake, 401 → drop →
re-auth → retry succeeds, no login storm, no-cache client unchanged,
fresh: true, 500during the resume probe propagates, and the 429 message/
Retry-After/no-token-leak). PlusRetry-Aftercoverage intests/http.test.ts.All mocked — no live API calls were made while building this (the stored token points at a
production instance, and probing it is what surfaced the issue).
npm test(890 passed),npm run typecheck,npm run lint,npm run buildall pass;docs/handbuch/field-definitions.mdwas re-read against its sources and re-signed sincesrc/api/ctClient.tschanged.Deliberately left out
ct auth statusstill handshakes per env. Its--allpreflight exists to prove eachtoken works; answering it from a session cache would make a green line mean less than it
does today. It gets no cache.
Set-Cookie, to avoid a Keychain write per request. A rotated cookie is used for the restof the process and re-bought next time.
ajax()/legacyPostForm()keep their existing behaviour — the 401 self-heal coversthe REST request path only. The legacy endpoints answer an expired session with a redirect
rather than a clean 401, which is a separate problem from this one.
ct auth refresh/ cache-inspection command. Nothing yet asks for one.Closes #145.
https://claude.ai/code/session_018JShVZYNLaRb4hF5KbHCXG