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

@@ -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()
+			}
 		}
 	}
 

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

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
 }

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},