Skip to content

feat: cache role credentials in files, keep only the session in the keyring - #39

Draft
alukach wants to merge 7 commits into
mainfrom
feat/sts-creds-in-file
Draft

alukach wants to merge 7 commits into
mainfrom
feat/sts-creds-in-file

Conversation

@alukach

@alukach alukach commented Oct 13, 2026

Copy link
Copy Markdown
Contributor

Stacked on #22 (per-environment caches). Retarget to main once #22 merges.

Problem

macOS keychain entries trust only the binary that created them. Release binaries aren't signed with a Developer ID, so after every CLI upgrade the Keychain asks for the user's password once for each entry: the login session plus one per role, per environment. A user with a couple of roles across prod and staging gets a burst of password prompts after each upgrade.

Change

  • Role credentials (STS keys) go in the 0600 cache files, not the keyring. They're short-lived and re-created from the session, much like the AWS CLI's own ~/.aws/cli/cache. A creds call that finds fresh credentials never touches the keyring.
  • The login session, which holds the long-lived refresh token, stays in the keyring (with the existing file fallback on headless machines).
  • No logout on upgrade: role reads try the file first, then the keyring, so credentials cached by earlier versions are still found. The next login writes a file that takes precedence.
  • Entries from v0.3.0 or earlier that carry their own refresh token keep being written to the keyring, so a refresh token never lands in a plain file.
  • README: the storage section now describes the split.

Result: after an upgrade, one keychain prompt per environment (for the session), and only when credentials actually need re-creating.

Testing

  • cargo test and cargo test --features staging: 30 passed, including the new role_entries_round_trip_through_their_file
  • cargo clippy --all-targets: clean
  • Manual: creds with HOME pointed at a temp dir holding only a file-cached entry prints it, with no keychain access
  • Not exercised against a real keychain, to avoid password prompts: the fallback read of entries written by earlier versions. It's the same keyring read as before, now running only when the file is missing.

🤖 Generated with Claude Code

alukach and others added 7 commits September 30, 2026 11:28
The cache key was the role ARN alone, and both environments default to
`_default`, so a staging login overwrote the prod entry (and `creds`
would silently serve/refresh staging credentials for a prod profile).
Key entries by `<proxy host>/<role>` and let `creds` take
`--proxy-url` / `SOURCE_PROXY_URL` to select the environment.

BREAKING CHANGE: existing cached logins are no longer found; run
`source-coop login` once after upgrading.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Existing prod caches (keyed by role ARN alone) stay readable, so upgrading
needs no re-login. Only non-production proxies get the host prefix.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Rebuild per-proxy cache keys on main's slot-based cache.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Key the login session by environment as well as role credentials, so a
staging login no longer replaces production's session. Normalize proxy
URLs with url::Url, keep non-default environments in their own cache
directory, ignore cached entries minted at another environment's proxy,
and add --proxy-url to 'auth token'.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…onment

- The bare (pre-environment) slots now always mean data.source.coop, so
  staging and production builds sharing the keyring can't read each
  other's caches.
- Sessions record their proxy and are ignored under another environment.
- An environment with nothing cached adopts its entry from the production
  slot, where versions before environments stored every proxy's.
- Ports get their own cache-directory segment, since '_' is legal in hosts.
- login writes credentials under the role lock.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Bare entries record no proxy, so the ones in the production slot are
production's; copying them into an empty staging slot served prod
credentials as staging until they expired.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…eyring

STS credentials are short-lived and re-minted from the login session, so
they don't need the keyring. Keeping them out of it means a creds call
with fresh credentials never touches the keyring, and macOS asks for the
keychain password after each upgrade for one entry (the session) instead
of one per role.

Reads try the file first and fall back to the keyring, so credentials
cached by earlier versions keep working. Entries that still carry their
own refresh token (v0.3.0 and earlier) stay in the keyring.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@claude

claude Bot commented Oct 13, 2026 •

Copy link
Copy Markdown

Claude finished @alukach's task in 14s —— View job


✅ No blocking issues — safe to merge.

I read src/cache.rs and the README change. Nothing in the diff is incorrect or a security problem. I didn't run cargo test or clippy here, so the results in your PR description are the ones I'm relying on.

  • Refresh tokens: write_role keeps any entry with refresh.is_some() in the keyring, so a refresh token never lands in a plain file.
  • Read order: the file is read first and the keyring second, so a fresh creds call never touches the keyring. A file written by a later login takes precedence over an old keyring entry.
  • Legacy migration: the copy into the per-environment slot in read_credentials goes through write_role, so a migrated entry follows the same rules.
  • Keyring errors: read_keyring treats NoEntry and an unavailable keyring as None, as before. A keyring entry that fails to parse still returns an error, which is the old behaviour too.

Non-blocking note: an old keyring entry is never deleted once the file takes over. If the user never runs logout, a stale STS entry stays in the keychain. It's short-lived and harmless, but a one-time delete_credential on a successful file write would tidy it up. The catch is that the delete itself may trigger the keychain prompt this PR is avoiding, so leaving it is probably right.

Simplify (ponytail)

  • src/cache.rs:278 read_slot: this is now only used for the session. If read_session is the sole caller, fold it into read_session, or keep it as the single keyring-then-file helper. Not worth a change.

💰 Estimated review cost: $0.11 · 0m13s · 6 turns

Base automatically changed from feat/per-environment-cache to main October 13, 2026 22:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant