diff --git a/pkg/distribution/oci/authn/authn.go b/pkg/distribution/oci/authn/authn.go index 1883e7e1c..19a67c831 100644 --- a/pkg/distribution/oci/authn/authn.go +++ b/pkg/distribution/oci/authn/authn.go @@ -200,6 +200,28 @@ func getAuthFromConfig(registry string) (Authenticator, error) { return nil, nil } +// FromDockerConfig resolves credentials for registry from ~/.docker/config.json +// alone — credential helpers (credHelpers), then the credential store +// (credsStore), then inline auths entries. +// +// Unlike DefaultKeychain.Resolve it deliberately does not consult the +// DOCKER_USERNAME/DOCKER_HUB_USER environment variables, so a caller that must +// not offer Hub credentials to an unrelated host (for example a corporate +// registry mirror) can scope that decision itself. +// +// A nil Authenticator with a nil error means no credentials were found. That is +// not an error: the registry may allow anonymous access. +func FromDockerConfig(registry string) (Authenticator, error) { + auth, err := getAuthFromConfig(registry) + if err != nil { + if errors.Is(err, os.ErrNotExist) { + return nil, nil + } + return nil, err + } + return auth, nil +} + // getServerAddressForRegistry returns the server address used for credential lookup. // Docker Hub credentials are stored under "https://index.docker.io/v1/". func getServerAddressForRegistry(registry string) string { diff --git a/pkg/inference/backends/diffusers/diffusers.go b/pkg/inference/backends/diffusers/diffusers.go index 4b6517bce..c75ff1bee 100644 --- a/pkg/inference/backends/diffusers/diffusers.go +++ b/pkg/inference/backends/diffusers/diffusers.go @@ -56,13 +56,16 @@ type diffusers struct { installDir string // registryMirrors is the list of registry mirrors to try before registry-1.docker.io. registryMirrors []string + // registryCredentials, if non-nil, resolves credentials for the registry (or + // mirror) the backend image is fetched from. + registryCredentials inference.RegistryCredentials // commandModifier, if non-nil, is applied to the server process before it starts. commandModifier func(*exec.Cmd) } // New creates a new diffusers-based backend for image generation. // customPythonPath is an optional path to a custom python3 binary; if empty, the default installation is used. -func New(log logging.Logger, modelManager *models.Manager, serverLog logging.Logger, conf *Config, customPythonPath string, registryMirrors []string, commandModifier func(*exec.Cmd)) (inference.Backend, error) { +func New(log logging.Logger, modelManager *models.Manager, serverLog logging.Logger, conf *Config, customPythonPath string, registryMirrors []string, registryCredentials inference.RegistryCredentials, commandModifier func(*exec.Cmd)) (inference.Backend, error) { // If no config is provided, use the default configuration if conf == nil { conf = NewDefaultConfig() @@ -75,15 +78,16 @@ func New(log logging.Logger, modelManager *models.Manager, serverLog logging.Log installDir := filepath.Join(homeDir, defaultInstallDir) return &diffusers{ - log: log, - modelManager: modelManager, - serverLog: serverLog, - config: conf, - status: inference.FormatNotInstalled(""), - customPythonPath: customPythonPath, - installDir: installDir, - registryMirrors: registryMirrors, - commandModifier: commandModifier, + log: log, + modelManager: modelManager, + serverLog: serverLog, + config: conf, + status: inference.FormatNotInstalled(""), + customPythonPath: customPythonPath, + installDir: installDir, + registryMirrors: registryMirrors, + registryCredentials: registryCredentials, + commandModifier: commandModifier, }, nil } @@ -158,7 +162,7 @@ func (d *diffusers) downloadAndExtract(ctx context.Context) error { // Pull the image image := fmt.Sprintf("registry-1.docker.io/docker/model-runner:diffusers-%s", diffusersVersion) - if err := dockerhub.PullPlatform(ctx, image, filepath.Join(downloadDir, "image.tar"), runtime.GOOS, runtime.GOARCH, d.registryMirrors); err != nil { + if err := dockerhub.PullPlatform(ctx, image, filepath.Join(downloadDir, "image.tar"), runtime.GOOS, runtime.GOARCH, d.registryMirrors, d.registryCredentials); err != nil { return fmt.Errorf("failed to pull image: %w", err) } diff --git a/pkg/inference/backends/llamacpp/download.go b/pkg/inference/backends/llamacpp/download.go index bafb8e729..c14c84a59 100644 --- a/pkg/inference/backends/llamacpp/download.go +++ b/pkg/inference/backends/llamacpp/download.go @@ -95,12 +95,12 @@ func (l *llamaCpp) downloadLatestLlamaCpp(ctx context.Context, log logging.Logge // Resolve the desired tag to a digest via the Registry HTTP API v2. This // honors l.registryMirrors (typically a corporate Artifactory / Nexus / - // Harbor mirror configured for docker.io) and credentials populated by - // `docker login`, so customers behind a private mirror with no direct - // egress to registry-1.docker.io can still resolve and pull the backend - // image. See docker/model-runner#TBD. + // Harbor mirror configured for docker.io) and l.registryCredentials — or, + // when those are nil, credentials populated by `docker login` — so customers + // behind a private mirror with no direct egress to registry-1.docker.io can + // still resolve and pull the backend image. tagRef := fmt.Sprintf("registry-1.docker.io/%s/%s:%s", hubNamespace, hubRepo, desiredTag) - latest, err := dockerhub.ResolveDigest(ctx, tagRef, l.registryMirrors) + latest, err := dockerhub.ResolveDigest(ctx, tagRef, l.registryMirrors, l.registryCredentials) if err != nil { log.Warn("could not resolve llama.cpp tag", "tag", desiredTag, "mirrors", l.registryMirrors, "error", err) return fmt.Errorf("could not resolve the %s tag: %w", desiredTag, err) @@ -126,7 +126,7 @@ func (l *llamaCpp) downloadLatestLlamaCpp(ctx context.Context, log logging.Logge defer os.RemoveAll(downloadDir) l.status = inference.FormatInstalling(fmt.Sprintf("%s llama.cpp %s", inference.DetailDownloading, desiredTag)) - if extractErr := extractFromImage(ctx, log, image, runtime.GOOS, runtime.GOARCH, downloadDir, l.registryMirrors); extractErr != nil { + if extractErr := extractFromImage(ctx, log, image, runtime.GOOS, runtime.GOARCH, downloadDir, l.registryMirrors, l.registryCredentials); extractErr != nil { return fmt.Errorf("could not extract image: %w", extractErr) } @@ -215,14 +215,14 @@ func (l *llamaCpp) writeInstalledVersion(log logging.Logger, rec installedVersio } //nolint:unused // Used in platform-specific files (download_darwin.go, download_windows.go) -func extractFromImage(ctx context.Context, log logging.Logger, image, requiredOs, requiredArch, destination string, mirrors []string) error { +func extractFromImage(ctx context.Context, log logging.Logger, image, requiredOs, requiredArch, destination string, mirrors []string, creds inference.RegistryCredentials) error { log.Info("Extracting image", "image", image, "destination", destination) tmpDir, err := os.MkdirTemp("", "docker-tar-extract") if err != nil { return err } imageTar := filepath.Join(tmpDir, "save.tar") - if err := dockerhub.PullPlatform(ctx, image, imageTar, requiredOs, requiredArch, mirrors); err != nil { + if err := dockerhub.PullPlatform(ctx, image, imageTar, requiredOs, requiredArch, mirrors, creds); err != nil { return err } return dockerhub.Extract(imageTar, requiredArch, requiredOs, destination) diff --git a/pkg/inference/backends/llamacpp/llamacpp.go b/pkg/inference/backends/llamacpp/llamacpp.go index 2b25d79db..e006dc2e4 100644 --- a/pkg/inference/backends/llamacpp/llamacpp.go +++ b/pkg/inference/backends/llamacpp/llamacpp.go @@ -53,6 +53,10 @@ type llamaCpp struct { gpuSupported bool // registryMirrors is the list of registry mirrors to try before registry-1.docker.io. registryMirrors []string + // registryCredentials, if non-nil, resolves credentials for the registry (or + // mirror) the backend image is fetched from. When nil, credentials come from + // the environment and ~/.docker/config.json. + registryCredentials inference.RegistryCredentials // commandModifier, if non-nil, is applied to the server process before it starts. commandModifier func(*exec.Cmd) } @@ -67,6 +71,7 @@ func New( installDir string, conf config.BackendConfig, registryMirrors []string, + registryCredentials inference.RegistryCredentials, commandModifier func(*exec.Cmd), ) (inference.Backend, error) { // If no config is provided, use the default configuration @@ -83,14 +88,15 @@ func New( } return &llamaCpp{ - log: log, - modelManager: modelManager, - serverLog: serverLog, - installDir: installDir, - status: inference.FormatNotInstalled(""), - config: conf, - registryMirrors: registryMirrors, - commandModifier: commandModifier, + log: log, + modelManager: modelManager, + serverLog: serverLog, + installDir: installDir, + status: inference.FormatNotInstalled(""), + config: conf, + registryMirrors: registryMirrors, + registryCredentials: registryCredentials, + commandModifier: commandModifier, }, nil } diff --git a/pkg/inference/backends/vllm/vllm.go b/pkg/inference/backends/vllm/vllm.go index cdfb7a469..9d0025244 100644 --- a/pkg/inference/backends/vllm/vllm.go +++ b/pkg/inference/backends/vllm/vllm.go @@ -46,17 +46,21 @@ type vLLM struct { customBinaryPath string // registryMirrors is the list of registry mirrors to try before registry-1.docker.io. registryMirrors []string + // registryCredentials, if non-nil, resolves credentials for the registry (or + // mirror) the backend image is fetched from. + registryCredentials inference.RegistryCredentials // commandModifier, if non-nil, is applied to the server process before it starts. commandModifier func(*exec.Cmd) } // Options holds the configuration for the unified vLLM backend constructor. type Options struct { - Config *Config // Linux-only: extra vllm args (nil = defaults) - LinuxBinaryPath string // Linux: custom vllm binary path - MetalPythonPath string // macOS ARM64: custom python path - RegistryMirrors []string // registry mirrors tried before registry-1.docker.io - CommandModifier func(*exec.Cmd) // applied to the server process before it starts + Config *Config // Linux-only: extra vllm args (nil = defaults) + LinuxBinaryPath string // Linux: custom vllm binary path + MetalPythonPath string // macOS ARM64: custom python path + RegistryMirrors []string // registry mirrors tried before registry-1.docker.io + RegistryCredentials inference.RegistryCredentials // resolves credentials for the registry or mirror; nil falls back to the environment and ~/.docker/config.json + CommandModifier func(*exec.Cmd) // applied to the server process before it starts } // New creates the appropriate vLLM backend for the current platform. @@ -65,9 +69,9 @@ type Options struct { // methods return errors. func New(log logging.Logger, modelManager *models.Manager, serverLog logging.Logger, opts Options) (inference.Backend, error) { if platform.SupportsVLLMMetal() { - return newMetal(log, modelManager, serverLog, opts.MetalPythonPath, opts.RegistryMirrors, opts.CommandModifier) + return newMetal(log, modelManager, serverLog, opts.MetalPythonPath, opts.RegistryMirrors, opts.RegistryCredentials, opts.CommandModifier) } - return newLinux(log, modelManager, serverLog, opts.Config, opts.LinuxBinaryPath, opts.RegistryMirrors, opts.CommandModifier) + return newLinux(log, modelManager, serverLog, opts.Config, opts.LinuxBinaryPath, opts.RegistryMirrors, opts.RegistryCredentials, opts.CommandModifier) } // NeedsDeferredInstall reports whether vllm on the current platform @@ -78,21 +82,22 @@ func NeedsDeferredInstall() bool { // newLinux creates a new Linux vLLM-based backend. // customBinaryPath is an optional path to a custom vllm binary; if empty, the default path is used. -func newLinux(log logging.Logger, modelManager *models.Manager, serverLog logging.Logger, conf *Config, customBinaryPath string, registryMirrors []string, commandModifier func(*exec.Cmd)) (inference.Backend, error) { +func newLinux(log logging.Logger, modelManager *models.Manager, serverLog logging.Logger, conf *Config, customBinaryPath string, registryMirrors []string, registryCredentials inference.RegistryCredentials, commandModifier func(*exec.Cmd)) (inference.Backend, error) { // If no config is provided, use the default configuration if conf == nil { conf = NewDefaultVLLMConfig() } return &vLLM{ - log: log, - modelManager: modelManager, - serverLog: serverLog, - config: conf, - status: inference.FormatNotInstalled(""), - customBinaryPath: customBinaryPath, - registryMirrors: registryMirrors, - commandModifier: commandModifier, + log: log, + modelManager: modelManager, + serverLog: serverLog, + config: conf, + status: inference.FormatNotInstalled(""), + customBinaryPath: customBinaryPath, + registryMirrors: registryMirrors, + registryCredentials: registryCredentials, + commandModifier: commandModifier, }, nil } diff --git a/pkg/inference/backends/vllm/vllm_metal.go b/pkg/inference/backends/vllm/vllm_metal.go index 4caa8ec27..fcb373ce4 100644 --- a/pkg/inference/backends/vllm/vllm_metal.go +++ b/pkg/inference/backends/vllm/vllm_metal.go @@ -53,13 +53,16 @@ type vllmMetal struct { status string // registryMirrors is the list of registry mirrors to try before registry-1.docker.io. registryMirrors []string + // registryCredentials, if non-nil, resolves credentials for the registry (or + // mirror) the backend image is fetched from. + registryCredentials inference.RegistryCredentials // commandModifier, if non-nil, is applied to the server process before it starts. commandModifier func(*exec.Cmd) } // newMetal creates a new vllm-metal backend. // customPythonPath is an optional path to a custom python3 binary; if empty, the default installation is used. -func newMetal(log logging.Logger, modelManager *models.Manager, serverLog logging.Logger, customPythonPath string, registryMirrors []string, commandModifier func(*exec.Cmd)) (inference.Backend, error) { +func newMetal(log logging.Logger, modelManager *models.Manager, serverLog logging.Logger, customPythonPath string, registryMirrors []string, registryCredentials inference.RegistryCredentials, commandModifier func(*exec.Cmd)) (inference.Backend, error) { homeDir, err := os.UserHomeDir() if err != nil { return nil, fmt.Errorf("failed to get user home directory: %w", err) @@ -67,14 +70,15 @@ func newMetal(log logging.Logger, modelManager *models.Manager, serverLog loggin installDir := filepath.Join(homeDir, defaultInstallDir) return &vllmMetal{ - log: log, - modelManager: modelManager, - serverLog: serverLog, - customPythonPath: customPythonPath, - installDir: installDir, - status: inference.FormatNotInstalled(""), - registryMirrors: registryMirrors, - commandModifier: commandModifier, + log: log, + modelManager: modelManager, + serverLog: serverLog, + customPythonPath: customPythonPath, + installDir: installDir, + status: inference.FormatNotInstalled(""), + registryMirrors: registryMirrors, + registryCredentials: registryCredentials, + commandModifier: commandModifier, }, nil } @@ -149,7 +153,7 @@ func (v *vllmMetal) downloadAndExtract(ctx context.Context, _ *http.Client) erro // Pull the image image := fmt.Sprintf("registry-1.docker.io/docker/model-runner:vllm-metal-%s", vllmMetalVersion) - if err := dockerhub.PullPlatform(ctx, image, filepath.Join(downloadDir, "image.tar"), runtime.GOOS, runtime.GOARCH, v.registryMirrors); err != nil { + if err := dockerhub.PullPlatform(ctx, image, filepath.Join(downloadDir, "image.tar"), runtime.GOOS, runtime.GOARCH, v.registryMirrors, v.registryCredentials); err != nil { return fmt.Errorf("failed to pull image: %w", err) } diff --git a/pkg/inference/credentials.go b/pkg/inference/credentials.go new file mode 100644 index 000000000..c4995a689 --- /dev/null +++ b/pkg/inference/credentials.go @@ -0,0 +1,20 @@ +package inference + +// RegistryCredentials resolves registry credentials for a registry host, +// returning the username and secret to authenticate with. Returning an empty +// username and secret with a nil error means no credentials are available for +// that host and anonymous access should be attempted. +// +// An embedder that already holds registry credentials in process supplies one of +// these so backend image pulls authenticate without shelling out to a +// docker-credential-* helper — which also removes any dependency on the helper +// being on PATH. Docker Desktop does this: it is itself the credential backend. +// +// When nil, backends fall back to resolving credentials from the environment and +// ~/.docker/config.json (including credHelpers and credsStore). +// +// The host is the registry the request is being made to, so for a pull routed +// through a registry mirror it is the mirror's host, not registry-1.docker.io. +// Credentials must therefore be resolved per host: Docker Hub credentials do not +// apply to a third-party mirror. +type RegistryCredentials func(host string) (username, secret string, err error) diff --git a/pkg/internal/dockerhub/credentials.go b/pkg/internal/dockerhub/credentials.go new file mode 100644 index 000000000..ffd53fac8 --- /dev/null +++ b/pkg/internal/dockerhub/credentials.go @@ -0,0 +1,90 @@ +package dockerhub + +import ( + "log/slog" + "os" + + "github.com/docker/model-runner/pkg/distribution/oci/authn" +) + +// Credentials resolves registry credentials for a registry host, returning the +// username and secret to authenticate with. An empty username and secret with a +// nil error means no credentials are available and the caller should attempt +// anonymous access. +// +// It is an alias rather than a defined type so that callers can pass their own +// named function type (e.g. inference.RegistryCredentials) without a conversion. +type Credentials = func(host string) (username, secret string, err error) + +// isHubHost reports whether host is one of the names Docker Hub is reached by, +// and therefore whether Docker Hub credentials apply to it. A registry mirror +// standing in for Hub is not a Hub host: it needs its own credentials, and +// offering Hub credentials to an unrelated third-party host would leak them. +func isHubHost(host string) bool { + switch host { + case "docker.io", "registry-1.docker.io", "index.docker.io": + return true + } + return false +} + +// defaultCredentials resolves registry credentials the way the Docker CLI does: +// DOCKER_HUB_USER/DOCKER_HUB_PASSWORD for Docker Hub itself, then +// ~/.docker/config.json — credential helpers (credHelpers), the credential store +// (credsStore), and finally inline auths entries. +// +// Consulting the credential store is what makes authenticated registry mirrors +// work. `docker login` on Docker Desktop stores the secret in the OS keychain and +// leaves the auths entry's "auth" field empty, so a config.json-only lookup finds +// nothing and the request is sent unauthenticated. Against a mirror that requires +// authentication that yields a 401, the resolver then falls through to +// registry-1.docker.io and the surfaced error names Hub rather than the mirror — +// which makes the real cause (missing credentials for the mirror) invisible. +// +// Docker Desktop supplies its own resolver instead of this one, because it holds +// the credentials in process and does not need to shell out to a helper. See +// inference.RegistryCredentials. +func defaultCredentials(host string) (string, string, error) { + if isHubHost(host) { + if user, password := os.Getenv("DOCKER_HUB_USER"), os.Getenv("DOCKER_HUB_PASSWORD"); user != "" && password != "" { + slog.Debug("using Docker Hub credentials from the environment", "host", host, "user", user) + return user, password, nil + } + } + authenticator, err := authn.FromDockerConfig(host) + if err != nil { + return "", "", err + } + if authenticator == nil { + slog.Debug("no registry credentials found", "host", host) + return "", "", nil + } + config, err := authenticator.Authorization() + if err != nil { + return "", "", err + } + return credentialsFromAuthConfig(host, config) +} + +// credentialsFromAuthConfig converts an authn.AuthConfig into the +// (username, secret) pair containerd's authorizer expects. containerd treats an +// empty username as "the secret is a token", which is how identity and registry +// tokens are conveyed. +func credentialsFromAuthConfig(host string, config *authn.AuthConfig) (string, string, error) { + if config == nil { + return "", "", nil + } + switch { + case config.Username != "" && config.Password != "": + slog.Debug("using registry credentials", "host", host, "user", config.Username) + return config.Username, config.Password, nil + case config.IdentityToken != "": + slog.Debug("using identity token for registry", "host", host) + return "", config.IdentityToken, nil + case config.RegistryToken != "": + slog.Debug("using registry token for registry", "host", host) + return "", config.RegistryToken, nil + } + slog.Debug("no usable registry credentials", "host", host) + return "", "", nil +} diff --git a/pkg/internal/dockerhub/credentials_test.go b/pkg/internal/dockerhub/credentials_test.go new file mode 100644 index 000000000..8eaa39379 --- /dev/null +++ b/pkg/internal/dockerhub/credentials_test.go @@ -0,0 +1,300 @@ +package dockerhub + +import ( + "context" + "encoding/base64" + "fmt" + "net/http" + "net/http/httptest" + "os" + "path/filepath" + "runtime" + "strings" + "sync/atomic" + "testing" + "time" +) + +// authenticatedRegistry is a Docker Registry v2 handler that requires HTTP Basic +// authentication, mirroring how a JFrog Artifactory / Nexus / Harbor pull-through +// mirror behaves: unauthenticated requests get a 401 plus a WWW-Authenticate +// challenge, and only requests carrying the expected credentials are served. +type authenticatedRegistry struct { + tag string + digest string + user string + password string + + // unauthorized counts the requests rejected for missing/incorrect credentials. + unauthorized atomic.Int64 + // authorized counts the requests that presented the expected credentials. + authorized atomic.Int64 +} + +func (h *authenticatedRegistry) ServeHTTP(w http.ResponseWriter, r *http.Request) { + user, password, ok := r.BasicAuth() + if !ok || user != h.user || password != h.password { + h.unauthorized.Add(1) + // A Basic challenge keeps the test focused on credential plumbing rather + // than on the token-fetch dance a Bearer challenge would trigger. + w.Header().Set("WWW-Authenticate", `Basic realm="registry"`) + w.Header().Set("Docker-Distribution-API-Version", "registry/2.0") + http.Error(w, "authentication required", http.StatusUnauthorized) + return + } + h.authorized.Add(1) + + switch { + case r.URL.Path == "/v2/" || r.URL.Path == "/v2": + w.Header().Set("Docker-Distribution-API-Version", "registry/2.0") + w.WriteHeader(http.StatusOK) + case strings.HasSuffix(r.URL.Path, "/manifests/"+h.tag): + body := []byte(`{"schemaVersion":2,"mediaType":"application/vnd.oci.image.index.v1+json","manifests":[]}`) + w.Header().Set("Docker-Content-Digest", h.digest) + w.Header().Set("Content-Type", "application/vnd.oci.image.index.v1+json") + w.Header().Set("Content-Length", fmt.Sprintf("%d", len(body))) + if r.Method == http.MethodHead { + w.WriteHeader(http.StatusOK) + return + } + w.WriteHeader(http.StatusOK) + _, _ = w.Write(body) + default: + http.Error(w, "not found", http.StatusNotFound) + } +} + +const ( + testMirrorUser = "mirroruser" + testMirrorPassword = "mirrorsecret" + testMirrorTag = "latest-cuda" + testMirrorDigest = "sha256:aa3e239c00000000000000000000000000000000000000000000000000c0ffee" + testHubRef = "registry-1.docker.io/docker/docker-model-backend-llamacpp:" + testMirrorTag +) + +// isolateDockerConfig points credential lookups at an empty temporary home so a +// developer's real ~/.docker/config.json cannot influence the result. +func isolateDockerConfig(t *testing.T) string { + t.Helper() + home := t.TempDir() + t.Setenv("HOME", home) + t.Setenv("USERPROFILE", home) // os.UserHomeDir on Windows + t.Setenv("DOCKER_HUB_USER", "") + t.Setenv("DOCKER_HUB_PASSWORD", "") + t.Setenv("DOCKER_USERNAME", "") + t.Setenv("DOCKER_PASSWORD", "") + if err := os.MkdirAll(filepath.Join(home, ".docker"), 0o755); err != nil { + t.Fatalf("creating .docker dir: %v", err) + } + return home +} + +// TestResolveDigest_AuthenticatedMirror_InjectedCredentials covers the path +// Docker Desktop uses: it holds registry credentials in process and injects a +// resolver, so backend image pulls authenticate against a private mirror without +// shelling out to a docker-credential-* helper. +func TestResolveDigest_AuthenticatedMirror_InjectedCredentials(t *testing.T) { + isolateDockerConfig(t) + + registry := &authenticatedRegistry{ + tag: testMirrorTag, digest: testMirrorDigest, + user: testMirrorUser, password: testMirrorPassword, + } + srv := httptest.NewServer(registry) + defer srv.Close() + + ctx, cancel := context.WithTimeout(t.Context(), 20*time.Second) + defer cancel() + + var askedFor []string + creds := func(host string) (string, string, error) { + askedFor = append(askedFor, host) + return testMirrorUser, testMirrorPassword, nil + } + + got, err := ResolveDigest(ctx, testHubRef, []string{srv.URL}, creds) + if err != nil { + t.Fatalf("ResolveDigest with injected credentials failed: %v", err) + } + if got != testMirrorDigest { + t.Fatalf("digest mismatch: got %q want %q", got, testMirrorDigest) + } + if registry.authorized.Load() == 0 { + t.Fatal("expected at least one authenticated request to the mirror, got none") + } + + // The credentials callback must be asked for the mirror's host, not + // registry-1.docker.io: a mirror needs its own credentials. + mirrorHost := strings.TrimPrefix(srv.URL, "http://") + if len(askedFor) == 0 { + t.Fatal("credentials callback was never invoked") + } + for _, host := range askedFor { + if host != mirrorHost { + t.Fatalf("credentials requested for %q, want the mirror host %q", host, mirrorHost) + } + } +} + +// TestAuthenticatedRegistry_EnforcesAuthentication is the negative control for the +// tests that resolve through an authenticated mirror: it proves the fixture really +// does require credentials, so those tests cannot pass against a registry that +// serves everyone. +// +// It probes the fixture directly rather than going through ResolveDigest. Driving +// the resolver without credentials is not a usable assertion here: containerd +// treats the mirror as one host in an ordered list and falls through to +// registry-1.docker.io when it is rejected, so the call reaches the real Docker Hub +// — which makes the test depend on the network and leaves idle connections behind. +// +// That fall-through is worth naming, because it is why this class of bug is hard to +// see: when a mirror turns the fetcher away, the error that surfaces names Hub, not +// the mirror that actually refused it. +func TestAuthenticatedRegistry_EnforcesAuthentication(t *testing.T) { + registry := &authenticatedRegistry{ + tag: testMirrorTag, digest: testMirrorDigest, + user: testMirrorUser, password: testMirrorPassword, + } + srv := httptest.NewServer(registry) + defer srv.Close() + + client := srv.Client() + + request, err := http.NewRequestWithContext(t.Context(), http.MethodGet, srv.URL+"/v2/", http.NoBody) + if err != nil { + t.Fatalf("building request: %v", err) + } + response, err := client.Do(request) + if err != nil { + t.Fatalf("unauthenticated probe failed: %v", err) + } + defer response.Body.Close() + if response.StatusCode != http.StatusUnauthorized { + t.Fatalf("unauthenticated probe returned %d, want 401", response.StatusCode) + } + if got := response.Header.Get("WWW-Authenticate"); got == "" { + t.Fatal("401 response carried no WWW-Authenticate challenge") + } + + request, err = http.NewRequestWithContext(t.Context(), http.MethodGet, srv.URL+"/v2/", http.NoBody) + if err != nil { + t.Fatalf("building request: %v", err) + } + request.SetBasicAuth(testMirrorUser, testMirrorPassword) + response, err = client.Do(request) + if err != nil { + t.Fatalf("authenticated probe failed: %v", err) + } + defer response.Body.Close() + if response.StatusCode != http.StatusOK { + t.Fatalf("authenticated probe returned %d, want 200", response.StatusCode) + } +} + +// TestResolveDigest_AuthenticatedMirror_CredentialStore is the regression test for +// the bug this fix addresses. `docker login` on Docker Desktop stores the secret in +// the OS keychain via credsStore and leaves the auths entry's "auth" field empty, so +// a config.json-only lookup finds nothing, the mirror answers 401, and the resolver +// falls through to registry-1.docker.io — surfacing a Hub error that hides the real +// cause. Credentials must be read through the configured helper. +func TestResolveDigest_AuthenticatedMirror_CredentialStore(t *testing.T) { + if runtime.GOOS == "windows" { + t.Skip("the fake credential helper is a shell script") + } + home := isolateDockerConfig(t) + + registry := &authenticatedRegistry{ + tag: testMirrorTag, digest: testMirrorDigest, + user: testMirrorUser, password: testMirrorPassword, + } + srv := httptest.NewServer(registry) + defer srv.Close() + mirrorHost := strings.TrimPrefix(srv.URL, "http://") + + // A credsStore entry with an auths entry whose "auth" field is empty is + // exactly the state `docker login` leaves behind on Docker Desktop. + config := fmt.Sprintf(`{"auths":{%q:{}},"credsStore":%q}`, mirrorHost, "modelrunnertest") + if err := os.WriteFile(filepath.Join(home, ".docker", "config.json"), []byte(config), 0o600); err != nil { + t.Fatalf("writing config.json: %v", err) + } + + // Stand in for docker-credential-desktop: read the server address on stdin + // and print the credentials the keychain would hold. + helperDir := t.TempDir() + helper := fmt.Sprintf(`#!/bin/sh +[ "$1" = "get" ] || exit 1 +cat >/dev/null +printf '{"ServerURL":"%s","Username":"%s","Secret":"%s"}\n' +`, mirrorHost, testMirrorUser, testMirrorPassword) + helperPath := filepath.Join(helperDir, "docker-credential-modelrunnertest") + if err := os.WriteFile(helperPath, []byte(helper), 0o700); err != nil { + t.Fatalf("writing credential helper: %v", err) + } + t.Setenv("PATH", helperDir+string(os.PathListSeparator)+os.Getenv("PATH")) + + ctx, cancel := context.WithTimeout(t.Context(), 20*time.Second) + defer cancel() + + got, err := ResolveDigest(ctx, testHubRef, []string{srv.URL}, nil) + if err != nil { + t.Fatalf("ResolveDigest did not use the credential store: %v", err) + } + if got != testMirrorDigest { + t.Fatalf("digest mismatch: got %q want %q", got, testMirrorDigest) + } + if registry.authorized.Load() == 0 { + t.Fatal("expected at least one authenticated request to the mirror, got none") + } +} + +// TestDefaultCredentials_HubEnvIsNotOfferedToMirrors verifies that Docker Hub +// credentials taken from the environment are scoped to Docker Hub's own hosts. +// Returning them for any host handed them to whatever third-party registry mirror +// happened to be configured. +func TestDefaultCredentials_HubEnvIsNotOfferedToMirrors(t *testing.T) { + isolateDockerConfig(t) + t.Setenv("DOCKER_HUB_USER", "hubuser") + t.Setenv("DOCKER_HUB_PASSWORD", "hubsecret") + + for _, host := range []string{"registry-1.docker.io", "docker.io", "index.docker.io"} { + user, password, err := defaultCredentials(host) + if err != nil { + t.Fatalf("defaultCredentials(%q) returned error: %v", host, err) + } + if user != "hubuser" || password != "hubsecret" { + t.Fatalf("defaultCredentials(%q) = (%q, %q), want the Hub credentials", host, user, password) + } + } + + for _, host := range []string{"devopsartifactory.corp.example.com", "nexus.internal:8082", "ghcr.io"} { + user, password, err := defaultCredentials(host) + if err != nil { + t.Fatalf("defaultCredentials(%q) returned error: %v", host, err) + } + if user != "" || password != "" { + t.Fatalf("defaultCredentials(%q) leaked Hub credentials: (%q, %q)", host, user, password) + } + } +} + +// TestDefaultCredentials_InlineAuths covers the plain config.json case, including +// the Docker Hub key normalization: credentials for registry-1.docker.io are +// stored under "https://index.docker.io/v1/", which an exact host comparison +// never matched. +func TestDefaultCredentials_InlineAuths(t *testing.T) { + home := isolateDockerConfig(t) + + encoded := base64.StdEncoding.EncodeToString([]byte("hubuser:hubsecret")) + config := fmt.Sprintf(`{"auths":{"https://index.docker.io/v1/":{"auth":%q}}}`, encoded) + if err := os.WriteFile(filepath.Join(home, ".docker", "config.json"), []byte(config), 0o600); err != nil { + t.Fatalf("writing config.json: %v", err) + } + + user, password, err := defaultCredentials("registry-1.docker.io") + if err != nil { + t.Fatalf("defaultCredentials returned error: %v", err) + } + if user != "hubuser" || password != "hubsecret" { + t.Fatalf("defaultCredentials = (%q, %q), want (hubuser, hubsecret)", user, password) + } +} diff --git a/pkg/internal/dockerhub/download.go b/pkg/internal/dockerhub/download.go index e6b6acaa9..a972b533f 100644 --- a/pkg/internal/dockerhub/download.go +++ b/pkg/internal/dockerhub/download.go @@ -2,14 +2,12 @@ package dockerhub import ( "context" - "encoding/base64" "errors" "fmt" "log/slog" "net/http" "os" "path/filepath" - "strings" "time" "github.com/containerd/containerd/v2/core/content" @@ -21,12 +19,15 @@ import ( "github.com/containerd/containerd/v2/plugins/content/local" "github.com/containerd/errdefs" "github.com/containerd/platforms" - "github.com/docker/model-runner/pkg/internal/jsonutil" "github.com/docker/model-runner/pkg/internal/registryutil" v1 "github.com/opencontainers/image-spec/specs-go/v1" ) -func PullPlatform(ctx context.Context, image, destination, requiredOs, requiredArch string, mirrors []string) error { +// PullPlatform downloads image for the given OS/architecture and writes it to +// destination as a tarball. Mirrors are tried before registry-1.docker.io for +// Docker Hub references. When creds is nil, credentials are resolved from the +// environment and ~/.docker/config.json. +func PullPlatform(ctx context.Context, image, destination, requiredOs, requiredArch string, mirrors []string, creds Credentials) error { if err := os.MkdirAll(filepath.Dir(destination), 0o755); err != nil { return fmt.Errorf("creating destination directory %s: %w", filepath.Dir(destination), err) } @@ -43,7 +44,7 @@ func PullPlatform(ctx context.Context, image, destination, requiredOs, requiredA if err != nil { return fmt.Errorf("creating new content store: %w", err) } - resolver := newResolver(mirrors) + resolver := newResolver(mirrors, creds) desc, err := retry(ctx, 10, 1*time.Second, func() (*v1.Descriptor, error) { return fetch(ctx, resolver, store, image, requiredOs, requiredArch) }) @@ -58,11 +59,12 @@ func PullPlatform(ctx context.Context, image, destination, requiredOs, requiredA // returns the resolved digest. It does not download any blobs; it issues only the manifest // HEAD/GET that the registry resolver needs. // -// Authentication uses the same credentials lookup as PullPlatform (env vars -// DOCKER_HUB_USER/DOCKER_HUB_PASSWORD or ~/.docker/config.json), so a prior -// `docker login ` is honored. -func ResolveDigest(ctx context.Context, ref string, mirrors []string) (string, error) { - resolver := newResolver(mirrors) +// Authentication uses the same credentials lookup as PullPlatform: creds when +// non-nil, otherwise the environment and ~/.docker/config.json (including +// credHelpers and credsStore), so a prior `docker login ` is +// honored. +func ResolveDigest(ctx context.Context, ref string, mirrors []string, creds Credentials) (string, error) { + resolver := newResolver(mirrors, creds) desc, err := retry(ctx, 10, 1*time.Second, func() (*v1.Descriptor, error) { name, d, err := resolver.Resolve(ctx, ref) if err != nil { @@ -77,10 +79,14 @@ func ResolveDigest(ctx context.Context, ref string, mirrors []string) (string, e return desc.Digest.String(), nil } -// newResolver builds a containerd docker resolver that authenticates via -// dockerCredentials and tries the given mirrors before the upstream registry. -func newResolver(mirrors []string) remotes.Resolver { - authorizer := docker.NewDockerAuthorizer(docker.WithAuthCreds(dockerCredentials)) +// newResolver builds a containerd docker resolver that tries the given mirrors +// before the upstream registry, authenticating with creds — or, when creds is +// nil, with defaultCredentials. +func newResolver(mirrors []string, creds Credentials) remotes.Resolver { + if creds == nil { + creds = defaultCredentials + } + authorizer := docker.NewDockerAuthorizer(docker.WithAuthCreds(creds)) return docker.NewResolver(docker.ResolverOptions{ Hosts: registryutil.RegistryHosts(mirrors, authorizer, nil), }) @@ -156,43 +162,3 @@ func fetch(ctx context.Context, resolver remotes.Resolver, store content.Store, } return &desc, nil } - -func dockerCredentials(host string) (string, string, error) { - hubUsername, hubPassword := os.Getenv("DOCKER_HUB_USER"), os.Getenv("DOCKER_HUB_PASSWORD") - if hubUsername != "" && hubPassword != "" { - return hubUsername, hubPassword, nil - } - slog.Debug("checking for registry auth config", "host", host) - home, err := os.UserHomeDir() - if err != nil { - return "", "", err - } - credentialConfig := filepath.Join(home, ".docker", "config.json") - cfg := struct { - Auths map[string]struct { - Auth string - } - }{} - if err := jsonutil.ReadFile(credentialConfig, &cfg); err != nil { - if errors.Is(err, os.ErrNotExist) { - return "", "", nil - } - return "", "", err - } - for h, r := range cfg.Auths { - if h == host { - creds, err := base64.StdEncoding.DecodeString(r.Auth) - if err != nil { - return "", "", err - } - parts := strings.SplitN(string(creds), ":", 2) - if len(parts) != 2 { - slog.Debug("skipping non-user/password auth for registry", "host", host, "auth_type", parts[0]) - return "", "", nil - } - slog.Debug("using auth for registry", "host", host, "user", parts[0]) - return parts[0], parts[1], nil - } - } - return "", "", nil -} diff --git a/pkg/internal/dockerhub/download_test.go b/pkg/internal/dockerhub/download_test.go index d5cb66e42..bcf9bb1d3 100644 --- a/pkg/internal/dockerhub/download_test.go +++ b/pkg/internal/dockerhub/download_test.go @@ -67,7 +67,7 @@ func TestResolveDigest_UsesMirror(t *testing.T) { // Reference points at registry-1.docker.io; the mirror should intercept it. ref := "registry-1.docker.io/docker/docker-model-backend-llamacpp:latest-cuda" - got, err := ResolveDigest(ctx, ref, []string{srv.URL}) + got, err := ResolveDigest(ctx, ref, []string{srv.URL}, nil) if err != nil { t.Fatalf("ResolveDigest returned error: %v", err) } @@ -124,7 +124,7 @@ func TestResolveDigest_UsesMirrorPathPrefix(t *testing.T) { defer cancel() ref := "registry-1.docker.io/docker/docker-model-backend-llamacpp:" + tag - got, err := ResolveDigest(ctx, ref, []string{srv.URL + prefix}) + got, err := ResolveDigest(ctx, ref, []string{srv.URL + prefix}, nil) if err != nil { t.Fatalf("ResolveDigest returned error: %v", err) } @@ -150,7 +150,7 @@ func TestResolveDigest_CanceledContext(t *testing.T) { done := make(chan struct{}) var resolveErr error go func() { - _, resolveErr = ResolveDigest(ctx, "registry-1.docker.io/docker/docker-model-backend-llamacpp:latest-cuda", nil) + _, resolveErr = ResolveDigest(ctx, "registry-1.docker.io/docker/docker-model-backend-llamacpp:latest-cuda", nil, nil) close(done) }() select { diff --git a/pkg/routing/backends.go b/pkg/routing/backends.go index 451e7484f..bcc83e474 100644 --- a/pkg/routing/backends.go +++ b/pkg/routing/backends.go @@ -42,6 +42,16 @@ type BackendsConfig struct { // injected by Docker Desktop from daemon.json registry-mirrors. RegistryMirrors []string + // RegistryCredentials, if non-nil, resolves credentials for the registry (or + // mirror) that backend images are pulled from. Embedders that already hold + // registry credentials in process — Docker Desktop, which is itself the + // credential backend — supply one so pulls authenticate without shelling out + // to a docker-credential-* helper. + // + // When nil, credentials are resolved from the environment and + // ~/.docker/config.json, including credHelpers and credsStore. + RegistryCredentials inference.RegistryCredentials + // CommandModifier, if non-nil, is applied to every backend runner process // immediately before it starts (see backends.RunnerConfig.CommandModifier). // Embedders use it to customize process attributes such as credentials or @@ -62,7 +72,7 @@ func DefaultBackendDefs(cfg BackendsConfig) []BackendDef { defs := []BackendDef{ {Name: llamacpp.Name, Deferred: llamacpp.NeedsDeferredInstall(), Init: func(mm *models.Manager) (inference.Backend, error) { - return llamacpp.New(cfg.Log, mm, sl(llamacpp.Name), cfg.LlamaCppPath, cfg.LlamaCppConfig, cfg.RegistryMirrors, cfg.CommandModifier) + return llamacpp.New(cfg.Log, mm, sl(llamacpp.Name), cfg.LlamaCppPath, cfg.LlamaCppConfig, cfg.RegistryMirrors, cfg.RegistryCredentials, cfg.CommandModifier) }}, } @@ -92,7 +102,7 @@ func DefaultBackendDefs(cfg BackendsConfig) []BackendDef { Name: diffusers.Name, Deferred: true, Init: func(mm *models.Manager) (inference.Backend, error) { - return diffusers.New(cfg.Log, mm, sl(diffusers.Name), nil, cfg.DiffusersPath, cfg.RegistryMirrors, cfg.CommandModifier) + return diffusers.New(cfg.Log, mm, sl(diffusers.Name), nil, cfg.DiffusersPath, cfg.RegistryMirrors, cfg.RegistryCredentials, cfg.CommandModifier) }, }) }