From 105feca22bae274f341f97b4880120b1b204a567 Mon Sep 17 00:00:00 2001 From: Nick Craig-Wood Date: Tue, 23 Jun 2026 10:47:04 +0100 Subject: [PATCH] fs: fix command line flag being ignored when set to its default value A backend flag set on the command line to a value that happened to equal its default was silently ignored, letting the config file win instead. For example --sftp-user defaults to the current user, so connecting as that same user with --sftp-user=USER had no effect and caused rclone to use the value from the config file. See: https://forum.rclone.org/t/sftp-user-parsing-as-cli-argument-broken/53955 --- fs/config/flags/flags.go | 6 ++++++ fs/configmap.go | 6 +++++- fs/registry.go | 20 ++++++++++++++++++++ fs/registry_test.go | 27 ++++++++++++++++++++++++++- 4 files changed, 57 insertions(+), 2 deletions(-) diff --git a/fs/config/flags/flags.go b/fs/config/flags/flags.go index 912c0b2d6..5e96483e8 100644 --- a/fs/config/flags/flags.go +++ b/fs/config/flags/flags.go @@ -168,6 +168,12 @@ func installFlag(flags *pflag.FlagSet, name string, groupsString string) { } fs.Debugf(nil, "Setting --%s %q from environment variable %s=%q", name, flag.Value, envKey, envValue) flag.DefValue = envValue + // This is a default from the environment, not an explicit + // flag on the command line, so don't let it take precedence + // over more specific configuration sources. + if isOption { + opt.MarkUnset() + } } } diff --git a/fs/configmap.go b/fs/configmap.go index 6f5ba5d52..7711f76b2 100644 --- a/fs/configmap.go +++ b/fs/configmap.go @@ -64,7 +64,11 @@ type regInfoValues struct { // the default values func (r *regInfoValues) Get(key string) (value string, ok bool) { opt := r.options.Get(key) - if opt != nil && (r.useDefault || !opt.IsDefault()) { + // Return the value if it was explicitly set (e.g. a flag on the + // command line, even when equal to the default) or if it differs + // from the default (e.g. loaded from the environment as a flag + // default by installFlag). + if opt != nil && (r.useDefault || opt.IsSet() || !opt.IsDefault()) { return opt.String(), true } return "", false diff --git a/fs/registry.go b/fs/registry.go index 59e82936c..a11c57d22 100644 --- a/fs/registry.go +++ b/fs/registry.go @@ -229,6 +229,7 @@ type Option struct { Provider string `json:",omitempty"` // set to filter on provider Default any // default value, nil => "", if set (and not to nil or "") then Required does nothing Value any // value to be set by flags + set bool // true once Set has been called, so an explicit value equal to Default is still honoured over the config file Examples OptionExamples `json:",omitempty"` // predefined values that can be selected from list (multiple-choice option) ShortOpt string `json:",omitempty"` // the short option for this if required Hide OptionVisibility // set this to hide the config from the configurator or the command line @@ -327,6 +328,7 @@ func (o *Option) Set(s string) (err error) { stringArray = []string{} } o.Value = append(stringArray, s) + o.set = true return nil } newValue, err := configstruct.StringToInterface(v, s) @@ -334,9 +336,27 @@ func (o *Option) Set(s string) (err error) { return err } o.Value = newValue + o.set = true return nil } +// IsSet returns true if Set has been called on the option, for example +// when its flag was provided on the command line. Unlike IsDefault this +// is true even when the value supplied is equal to the default. +func (o *Option) IsSet() bool { + return o.set +} + +// MarkUnset marks the option as not having been set explicitly. +// +// It is used after a value has been loaded from the environment as a +// flag default, so that the value is treated like a default rather than +// an explicit command line flag and more specific configuration sources +// still take precedence over it. +func (o *Option) MarkUnset() { + o.set = false +} + type typer interface { Type() string } diff --git a/fs/registry_test.go b/fs/registry_test.go index 593049c38..1f8ec58b0 100644 --- a/fs/registry_test.go +++ b/fs/registry_test.go @@ -45,9 +45,31 @@ var ( Name: "case_insensitive", Default: false, Value: true, + set: true, // set as if from a flag Advanced: true, } - testOptions = Options{nouncOption, copyLinksOption, caseInsensitiveOption} + // setToDefaultOption has been set explicitly to a value which + // happens to equal its default - it must still override the config + setToDefaultOption = Option{ + Name: "set_to_default", + Default: "deflt", + Value: "deflt", + set: true, + } + // envDefaultOption has had a value loaded as a default from the + // environment (so set is false). It is only honoured over more + // specific sources when it differs from the default. + envDefaultOption = Option{ + Name: "env_default", + Default: "deflt", + Value: "deflt", // equal to default, so must not be surfaced + } + envDefaultDiffOption = Option{ + Name: "env_default_diff", + Default: "deflt", + Value: "fromenv", // differs from default, so must be surfaced + } + testOptions = Options{nouncOption, copyLinksOption, caseInsensitiveOption, setToDefaultOption, envDefaultOption, envDefaultDiffOption} ) func TestOptionsSetValues(t *testing.T) { @@ -266,6 +288,9 @@ func TestOptionGetters(t *testing.T) { {regInfoValuesGetterFalse, "not_found", "", false}, {regInfoValuesGetterFalse, "case_insensitive", "true", true}, {regInfoValuesGetterFalse, "copy_links", "", false}, + {regInfoValuesGetterFalse, "set_to_default", "deflt", true}, + {regInfoValuesGetterFalse, "env_default", "", false}, + {regInfoValuesGetterFalse, "env_default_diff", "fromenv", true}, {regInfoValuesGetterTrue, "not_found", "", false}, {regInfoValuesGetterTrue, "case_insensitive", "true", true}, {regInfoValuesGetterTrue, "copy_links", "false", true},