From ccc2f95715b70ea6c705e80dd42aaa2aae565be5 Mon Sep 17 00:00:00 2001 From: Artem Lytkin Date: Thu, 20 Aug 2026 00:55:40 +0300 Subject: [PATCH] cli/config: error when the config directory is not a directory Setting DOCKER_CONFIG (or --config) to the config file itself instead of the directory containing it is a common mistake, and one the CLI handled inconsistently: - on Windows, opening "/config.json" fails with a "not exist" error, which load() treats as "no config file present", so the misconfiguration was silently ignored, and every command ran with default configuration. "docker logout" then reported success without writing anything, because there are no credentials to erase in a defaulted config, and it returns before saving. "docker login" did fail, but only once it tried to write, with "mkdir \config.json: The system cannot find the path specified". - on unix, the same open fails with ENOTDIR, so the CLI did report an error, but it named a path that cannot exist ("open /tmp/x/config.json/config.json: not a directory"). Stat the config directory when opening the config file fails, and report the actual problem. The check has to precede the os.IsNotExist branch, because on Windows this case *is* an "is not exist" error. The write path is unchanged, and still fails with the mkdir error above. This adds one stat to the paths where opening the config file already failed, which includes the common case of a fresh install that has no ~/.docker yet. Signed-off-by: Artem Lytkin --- cli/config/config.go | 18 ++++++++++++++++++ cli/config/config_test.go | 28 ++++++++++++++++++++++++++++ 2 files changed, 46 insertions(+) diff --git a/cli/config/config.go b/cli/config/config.go index 5a637805091c..ad5ca1cecf9d 100644 --- a/cli/config/config.go +++ b/cli/config/config.go @@ -136,6 +136,24 @@ func load(configDir string) (*configfile.ConfigFile, error) { file, err := os.Open(filename) if err != nil { + // The config-directory must be a directory. If it is a regular file + // (for example, when DOCKER_CONFIG or --config points at the config + // file itself instead of the directory holding it), opening the file + // fails with a platform-specific error; ENOTDIR on unix, and a + // "not exist" error on Windows. Detect that situation here, so that + // we consistently report the actual problem instead of either + // silently ignoring it, or producing an error for a path that + // does not exist. + // + // The check must come before the [os.IsNotExist] branch below, as + // the Windows error satisfies it. The underlying error is not + // wrapped for the same reason: it would still satisfy + // [errors.Is](err, [fs.ErrNotExist]) on Windows, making this + // misconfiguration indistinguishable from "no config file present" + // for callers that check for it. + if fi, statErr := os.Stat(configDir); statErr == nil && !fi.IsDir() { + return configFile, fmt.Errorf("loading config file: config directory (%s) is not a directory", configDir) + } if os.IsNotExist(err) { // It is OK for no configuration file to be present, in which // case we return a default struct. diff --git a/cli/config/config_test.go b/cli/config/config_test.go index 922641a726ad..dc1db291255c 100644 --- a/cli/config/config_test.go +++ b/cli/config/config_test.go @@ -50,6 +50,34 @@ func TestMissingFile(t *testing.T) { saveConfigAndValidateNewFormat(t, config, tmpHome) } +// TestLoadConfigDirIsFile verifies that we produce an error if the +// config-directory is not a directory, which happens when DOCKER_CONFIG +// or --config is set to the config file itself instead of the directory +// containing it. +func TestLoadConfigDirIsFile(t *testing.T) { + cfgDir := filepath.Join(t.TempDir(), ConfigFileName) + err := os.WriteFile(cfgDir, []byte(`{}`), 0o644) + assert.NilError(t, err) + + expected := fmt.Sprintf("loading config file: config directory (%s) is not a directory", cfgDir) + + t.Run("Load", func(t *testing.T) { + _, err := Load(cfgDir) + assert.Check(t, is.Error(err, expected)) + }) + + t.Run("LoadDefaultConfigFile", func(t *testing.T) { + oldDir := Dir() + SetDir(cfgDir) + t.Cleanup(func() { SetDir(oldDir) }) + + buffer := new(bytes.Buffer) + configFile := LoadDefaultConfigFile(buffer) + assert.Check(t, configFile != nil) + assert.Check(t, is.Contains(buffer.String(), "WARNING: Error "+expected)) + }) +} + // TestLoadDanglingSymlink verifies that we gracefully handle dangling symlinks. // // TODO(thaJeztah): consider whether we want dangling symlinks to be an error condition instead.