fix(registry): honor credential store when fetching backend images - #1033
Merged
Conversation
Contributor
There was a problem hiding this comment.
Hey - I've left some high level feedback:
- The fake docker-credential helper in TestResolveDigest_AuthenticatedMirror_CredentialStore is implemented as a shell script and skipped on Windows; consider rewriting it in Go so the regression test runs cross-platform and doesn’t depend on /bin/sh being present.
- defaultCredentials currently hard-codes Docker Hub hostnames in isHubHost; if other Hub endpoints or alternate schemes are introduced, it may be safer to centralize or reuse the host normalization logic already present in the authn package to avoid divergence.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The fake docker-credential helper in TestResolveDigest_AuthenticatedMirror_CredentialStore is implemented as a shell script and skipped on Windows; consider rewriting it in Go so the regression test runs cross-platform and doesn’t depend on /bin/sh being present.
- defaultCredentials currently hard-codes Docker Hub hostnames in isHubHost; if other Hub endpoints or alternate schemes are introduced, it may be safer to centralize or reuse the host normalization logic already present in the authn package to avoid divergence.Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Signed-off-by: Dorin Geman <dorin.geman@docker.com>
doringeman
force-pushed
the
mirror-fetcher-e2e
branch
from
August 6, 2026 13:25
d85bef6 to
13b0dc0
Compare
ericcurtin
approved these changes
Aug 6, 2026
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.
The backend image fetcher only read inline auths entries from ~/.docker/config.json, so credentials that docker login stores in the OS keychain were invisible and pulls through an authenticated registry mirror failed with a misleading Docker Hub 403. It now resolves credentials through credential helpers and the credential store, scopes DOCKER_HUB_* environment credentials to Hub's own hosts, and adds an inference.RegistryCredentials hook so Docker Desktop can inject in-process credentials instead of shelling out to a helper.