Skip to content

fix: use in-memory token store in tests to avoid OS keychain prompt#2836

Merged
dgageot merged 1 commit into
docker:mainfrom
dgageot:no-keychain-in-tests
May 20, 2026
Merged

fix: use in-memory token store in tests to avoid OS keychain prompt#2836
dgageot merged 1 commit into
docker:mainfrom
dgageot:no-keychain-in-tests

Conversation

@dgageot
Copy link
Copy Markdown
Member

@dgageot dgageot commented May 20, 2026

No description provided.

@dgageot dgageot requested a review from a team as a code owner May 20, 2026 14:13
Copy link
Copy Markdown

@docker-agent docker-agent left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Assessment: 🟡 NEEDS ATTENTION

// outbound HTTP request, popping a macOS password prompt for the
// `docker-agent-oauth` keychain item on developer machines that have a
// token from a prior login.
var defaultStore = sync.OnceValue(func() OAuthTokenStore {
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[NOTABLE] Shared singleton InMemoryTokenStore may leak token state between tests

The sync.OnceValue factory runs exactly once per process lifetime. Under go test, all tests in the package share the same InMemoryTokenStore instance returned by defaultStore(). Any test that stores a token via NewKeyringTokenStore() (which delegates to defaultStore()) will leave that token visible to all subsequent tests in the same binary run.

This is an accepted trade-off for the production singleton design (multiple toolsets sharing one store), and most existing tests already use NewInMemoryTokenStore() directly for isolation. However, integration tests or future tests that call NewKeyringTokenStore() and expect an empty store at start-up will see stale tokens from earlier tests — a subtle source of test flakiness.

Consider documenting this shared-state behaviour at the defaultStore declaration, or providing a test-helper that resets the singleton between tests (e.g., via t.Cleanup and a package-internal reset function).

@dgageot dgageot merged commit 5ccf598 into docker:main May 20, 2026
9 checks passed
@aheritier aheritier added area/security Authentication, authorization, secrets, vulnerabilities kind/fix PR fixes a bug (maps to fix: commit prefix) labels May 23, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/security Authentication, authorization, secrets, vulnerabilities kind/fix PR fixes a bug (maps to fix: commit prefix)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants