diff --git a/plan/credential-store.md b/plan/credential-store.md index 5262b9e..6dd4626 100644 --- a/plan/credential-store.md +++ b/plan/credential-store.md @@ -118,6 +118,57 @@ re-proposed: the OS keyring, and encrypting the DPoP key at rest. fatally, since the command somebody runs because their accounts are wrong must not be the command a wrong thing stops +- [x] **Three things only worked while one atgc ran at a time**, found + together because they share a cause: many agent processes spawn + dynamically against one shared, often ephemeral `$HOME`, so several + atgc invocations overlapping is the ordinary case and anything that + assumes otherwise is a defect rather than an edge. + + `registry::update` took the lock with the blocking `take`, on a doc + comment saying "these callers are sync". None of them were: every one + is reached from an `async fn`, so a contended registry write parked a + tokio worker for up to the thirty-second `WAIT` — and the peer it was + waiting on is usually doing the token refresh that would end the wait, + which on a one- or two-core runner may have had no other thread to run + on. `take` had no other caller and is gone; `take_async` is the one way + in, split over a `take_async_at` that names its lock file so the wait + loop is testable at all. + + Resolving `--account @handle` then called `remember` with a `?`. + `remember` always re-stamps `handle_checked_at`, so unlike the session + store's `put` there was no "already correct" short-circuit and a real + locked write always happened: every handle-named command took the + process-global lock, and a wave of twenty agents serialized twenty + acquisitions on the hot path of nearly every command — to keep a label + current that nothing reads for correctness. Worse, the `?` meant a lock + lost to an unrelated slow process failed the command the user asked + for. `refresh_cached_handle` skips the write while the registry already + agrees and the stamp is under a day old, and uses `lock::or_skip` so a + busy lock skips a cache repair rather than ending a command. `auth + status` did the same thing once per account and now uses it too — the + entry above had already stopped it being fatal, and this stops it + being unconditional and stops it warning per account about contention + nobody can act on. Both callers reach the file through the same `edit` + the entry above put the refusal in, so a best-effort cache repair + cannot overwrite a registry it could not read either. The TTL decision + is a pure function of the entry + and the current time, tested either side of the day and over every + shape that has nothing to compare against, including a stamp from the + future + +- [x] `push-key-.pub` was the one writer under `~/.config/atgc` ignoring + the atomic-write discipline the entries below established: a bare + `fs::write`, which truncates and then writes, to a path keyed by DID + alone. Two atgc processes pushing as the same account are two writers + of one file, so `ssh -i` could open it in the window between and get an + empty or half-written key, failing the push with an ssh error naming + nothing that would explain it. It goes through `config::dir::write_atomic` + now, at 0600 like everything else here — nothing in a public key is + secret and ssh asks no particular mode of the public half, but the + directory is 0700 already so a looser file buys nobody access, and + being the one exception is how a mode gets copied somewhere it matters. + Pinned by a test that reads the path while three threads rewrite it and + fails against the old `fs::write` - [x] File session store at ~/.config/atgc/sessions.json, mode 0600 - [x] `sessions.json` and `accounts.json` are now written atomically: stage the bytes in a same-directory temp file created at the destination's diff --git a/src/clients/git/ssh.rs b/src/clients/git/ssh.rs index 2b00cde..e71430e 100644 --- a/src/clients/git/ssh.rs +++ b/src/clients/git/ssh.rs @@ -408,9 +408,9 @@ mod tests { PushKey, Registered, agent_key_path_in, base64_decode, base64_encode, fingerprint, identity_in, keys_from_records, keys_in, normalize, write_agent_key, }; - use std::path::Path; use crate::testutil::TempKeys; use serde_json::json; + use std::path::Path; /// A real `sh.tangled.publicKey` listRecords response. const KEYS: &str = include_str!(concat!( diff --git a/src/config/account/registry.rs b/src/config/account/registry.rs index a3f0886..e8a48b0 100644 --- a/src/config/account/registry.rs +++ b/src/config/account/registry.rs @@ -511,7 +511,11 @@ mod tests { ..Account::default() }; assert!( - !cached_handle_is_current(Some(&no_handle), "alice.example", at("2026-08-26T09:00:00Z")), + !cached_handle_is_current( + Some(&no_handle), + "alice.example", + at("2026-08-26T09:00:00Z") + ), "an entry that carries only a client_id" ); @@ -520,7 +524,11 @@ mod tests { ..Account::default() }; assert!( - !cached_handle_is_current(Some(&unstamped), "alice.example", at("2026-08-26T09:00:00Z")), + !cached_handle_is_current( + Some(&unstamped), + "alice.example", + at("2026-08-26T09:00:00Z") + ), "a cached handle with no stamp beside it" );