Skip to content

Commit 2ec22bf

Browse files
committed
auth: clarify timeout is indeterminate in probe contract
The wrapper produces TimeoutError for any operation that misses the deadline, not only locked-but-reachable keyrings. The login path's optimistic stay-on-keyring is still right (cost of guessing wrong is one wasted OAuth ceremony, not a silently-plaintext token), but the docstring and inline comment overclaimed by labeling timeout as 'reachable but locked'.
1 parent 6cc96a5 commit 2ec22bf

2 files changed

Lines changed: 22 additions & 16 deletions

File tree

libs/auth/storage/cache.go

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -139,16 +139,16 @@ func applyLoginFallback(ctx context.Context, mode StorageMode, explicit bool, f
139139
return c, mode, nil
140140
case StorageModeSecure:
141141
if probeErr := f.probeKeyring(); probeErr != nil {
142-
// A timeout means the keyring is reachable but locked: the OS
143-
// unlock prompt is on screen and the user is mid-typing. Stay on
144-
// keyring regardless of explicit; by the time OAuth finishes the
145-
// prompt has been answered and the final Store will succeed
146-
// against an unlocked keyring. Mirrors gh CLI's accidentally-
147-
// similar flow where AuthTokenWriteable's early keyring.Get
148-
// triggers the same dialog without committing to plaintext.
142+
// Timeout is indeterminate: usually a locked keyring with the
143+
// GUI unlock prompt up and the user typing, but a hung or slow
144+
// daemon produces the same error. Stay on keyring optimistically.
145+
// If the user is unlocking, the final Store at end of OAuth lands
146+
// in the now-unlocked keyring. If the keyring is genuinely stuck,
147+
// the final Store also times out and login fails late, same
148+
// outcome as failing here, but with no silently-plaintext token.
149149
var timeoutErr *TimeoutError
150150
if errors.As(probeErr, &timeoutErr) {
151-
log.Debugf(ctx, "keyring probe timed out (%v); user is likely unlocking, staying on keyring", probeErr)
151+
log.Debugf(ctx, "keyring probe timed out (%v); staying on keyring optimistically", probeErr)
152152
return f.newKeyring(), mode, nil
153153
}
154154
if explicit {

libs/auth/storage/keyring.go

Lines changed: 14 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -91,14 +91,20 @@ func NewKeyringCache() cache.TokenCache {
9191
// ProbeKeyring returns nil if a write+delete cycle completed within the
9292
// standard timeout. Callers distinguish two non-nil shapes via errors.As:
9393
//
94-
// - *TimeoutError: the keyring is reachable but locked. On Linux this
95-
// usually means the GUI unlock prompt is up and the user has not
96-
// finished typing yet. The login path treats this as "stay on keyring"
97-
// because the prompt continues in parallel with OAuth and the final
98-
// Store will succeed against the by-then-unlocked keyring.
99-
// - any other error: the keyring is genuinely unavailable (no daemon,
100-
// headless session with no secret service, ...). Login falls back to
101-
// plaintext now rather than failing after OAuth.
94+
// - *TimeoutError: indeterminate. The keyring did not respond within
95+
// the timeout. The common cause on Linux is a locked collection
96+
// waiting on a GUI unlock prompt with the user mid-typing, but a
97+
// hung or slow daemon produces the same shape. The login path
98+
// optimistically stays on keyring: if the user is unlocking, the
99+
// prompt continues in parallel with OAuth and the final Store
100+
// succeeds against an unlocked keyring; if the keyring is genuinely
101+
// stuck, the final Store also times out and login fails late
102+
// instead of early. The cost of guessing wrong is one wasted OAuth
103+
// ceremony, not a silently-plaintext token.
104+
// - any other error: the keyring returned a definitive failure (no
105+
// daemon, headless session with no secret service, dismissed prompt,
106+
// ...). Login falls back to plaintext now rather than failing after
107+
// OAuth.
102108
//
103109
// Probing also has a useful side effect: triggering the unlock prompt up
104110
// front, before the browser step. The user can answer it while OAuth is in

0 commit comments

Comments
 (0)