Repository navigation
Report storage failures instead of hiding them - #116
Merged
Merged
Conversation
list_profiles() turned any index read error or corrupt JSON into an empty list. After a denied macOS keychain prompt, the next login rewrote the index to hold only the new profile, orphaning the rest, and `auth logout --all` reported "Logged out from 0 profile(s)" as a success. resolve_token likewise mapped every get_token error to "Not authenticated" (exit 3), sending a user with a locked keychain to log in again. get_token now returns Ok(None) for an absent token (and when the keyring is disabled); only that is "Not authenticated". Any other store error propagates as KEYRING_ERROR (exit 10) with its cause, and keychain read errors add a hint to unlock it or set TEAMS_CLI_TOKEN_STORE=file. A Windows token with a missing chunk is now exit 10 too. list_profiles returns Result: an absent index is empty, an unreadable or corrupt one is an error, and add/remove never write after a failed read. `auth list` and `logout --all` surface it. No on-disk or keychain format change. Tested with unit tests on in-memory and always-failing stores (corrupt or unreadable index is never rewritten) and file-store CLI tests: corrupt or unreadable index makes `auth list` / `logout` exit 10 with the index unchanged; absent token exits 3; unreadable or unparseable token exits 10 with the cause. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Hd5XXx9UkpLZpAAkvPPyUg
`auth switch` saved with `save_config(.., None)`, so with `--config customer.toml` it wrote customer.toml's contents plus the new default profile over the default config file and left customer.toml unchanged. A failed save was only a tracing warning beside a success. `config path` always reported the default path, ignoring `--config`. `save_config` truncated the file and wrote it in place, so a failed write (full disk, crash) could leave a partial config. `auth switch` now saves to the `--config` path, and a failed save is a config error (exit 10). `config path` reports the `--config` file when given and adds `source`: `flag` or `default`. `save_config` goes through `write_config_text`, the temp-file-and-rename writer login already uses (0600, writes through symlinks), so `config set`, `config init` and `auth switch` keep the old file on failure. No cross-process locking. Tests: switch with `--config` updates that file and creates no default config under a temp HOME; a switch whose save fails exits 10; `config path` with and without `--config`; `save_config` into a read-only directory fails and leaves the original file intact. All four fail without the fix. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Hd5XXx9UkpLZpAAkvPPyUg
With TEAMS_CLI_TOKEN_STORE=file, token file names map ':', '/' and '\' to '.', so profiles `customer/a` and `customer.a` shared one token file and a command could run as the wrong account. A new profile's name must now be 1-64 of A-Z a-z 0-9 . _ - and not `.` or `..`. `auth login` (via --profile, TEAMS_CLI_PROFILE or default.profile), `config set profiles.<name>...` and `config set default.profile` refuse other names with exit code 2. A name already in the config or the token index is accepted, so existing profiles keep working. The only name the CLI derives itself, `default`, conforms. The file store's name encoding is unchanged. Tests: unit tests for the name check, the `default` fallback and the existing-profile exemption; CLI tests for each refusal path, valid names, and legacy names in the config and in the file store index. An invalid name is checked against the token index; if that index cannot be read, the storage error (exit 10) is reported rather than read as no profiles. Valid names never read the index. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Hd5XXx9UkpLZpAAkvPPyUg
The file token store maps ':', '/' and '\' to '.', and existing profiles keep names outside the new set. So a legacy 'customer/a' and a new, valid 'customer.a' still mapped to one token file: logging in to the new one overwrote the old one's credentials. A new name is now also refused (exit 2) when its token file name matches any existing profile's, in the config or the token index. The mapping is shared with the file store, so the two cannot drift. New valid names now read the index; an unreadable index is still reported (exit 10), not taken as no profiles. Tests: unit cases for an indexed and a configured legacy name, and the CLI login case with 'customer/a' in the file store index; the CLI case fails against the previous check. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Hd5XXx9UkpLZpAAkvPPyUg
macOS and Windows file systems ignore case by default, so 'Work' and 'work' address one token file. The collision check compared names exactly, letting a new 'work' replace an existing 'Work' profile's credentials. Compare them ignoring ASCII case. Test: an existing 'Work' refuses a new 'work'; fails with an exact comparison. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Hd5XXx9UkpLZpAAkvPPyUg
auth status put the signed-in user only inside token_diagnostics, so .data.user read null although auth list shows it; and any failure to load a token, a locked or denied keychain included, came out as authenticated: false with exit 1, the "signed out" answer. Report user and tenant_id at the top level as well. A token store that cannot be read is now KEYRING_ERROR (exit 10), as for every other command; any other failure stays authenticated: false (exit 1) and says why in reason. Tests: top-level user and tenant from a token's claims; a missing token is exit 1 with a reason, an unreadable store is exit 10. Both fail against the previous handler. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Hd5XXx9UkpLZpAAkvPPyUg
Merged
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.
A denied or locked keychain was reported as "Not authenticated, run
teams auth login", and logging in again could rewrite the profile index down to one entry. Several config paths had similar silent failures. These are now reported as what they are.Behaviour changes:
KEYRING_ERROR(exit 10) with the cause, not exit 3 or an empty list. The index is never written after a failed read. (auth liststill shows a single profile's unreadable token as empty fields.)auth switch --config Fsaves to F, not the default config, and a failed save exits 10.config pathhonours--configand addssource.config set,config initandauth switchwrite atomically.A-Z a-z 0-9 . _ -. A new name that would share an existing profile's token file (the file store maps/and:to., and macOS ignores case) exits 2. Existing names keep working.auth statusadds top-leveluserandtenant_id, and areasonwhen signed out.Review guide:
3847108store and index errors. Check the never-write-after-failed-read path insrc/auth/keyring.rs.ba2454e--configand atomic saves.6626b11,ca0b0eband0c15e50profile names and the collision check insrc/config.rs.f0a8375auth status.Testing: unit tests with a store that fails every read, and CLI tests on the file store. Checked live with a delegated login.
https://claude.ai/code/session_01Hd5XXx9UkpLZpAAkvPPyUg