config: redact env var config values in logs

Before this change the environment variable getters in fs/configmap.go
logged the option value with %q, so a password set via
RCLONE_CONFIG_remote_pass (or RCLONE_remote_pass) was printed in full
to the debug log. Values from the config file were already redacted,
which made the leak easy to miss.

This change routes both getters through fs.RedactOptionValue, which
looks up the option in the backend's option list: options marked
IsPassword or Sensitive log as XXX, unknown options are conservatively
redacted, and --dump auth still shows the value for debugging.

Fixes #5794

fs/configmap.go

@@ -3,20 +3,25 @@
 package fs
 
 import (
+	"context"
 	"os"
 
 	"github.com/rclone/rclone/fs/config/configmap"
 )
 
 // A configmap.Getter to read from the environment RCLONE_CONFIG_backend_option_name
-type configEnvVars string
+type configEnvVars struct {
+	configName string
+	options    Options
+}
 
 // Get a config item from the environment variables if possible
-func (configName configEnvVars) Get(key string) (value string, ok bool) {
+func (c configEnvVars) Get(key string) (value string, ok bool) {
-	envKey := ConfigToEnv(string(configName), key)
+	envKey := ConfigToEnv(c.configName, key)
 	value, ok = os.LookupEnv(envKey)
 	if ok {
-		Debugf(nil, "Setting %s=%q for %q from environment variable %s", key, value, configName, envKey)
+		opt := c.options.Get(key)
+		Debugf(nil, "Setting %s=%s for %q from environment variable %s", key, RedactOptionValue(GetConfig(context.Background()), opt, value), c.configName, envKey)
 	}
 	return value, ok
 }
@@ -41,13 +46,13 @@ func (oev optionEnvVars) Get(key string) (value string, ok bool) {
 	}
 	value, ok = os.LookupEnv(envKey)
 	if ok {
-		Debugf(nil, "Setting %s %s=%q from environment variable %s", oev.prefix, key, value, envKey)
+		Debugf(nil, "Setting %s %s=%s from environment variable %s", oev.prefix, key, RedactOptionValue(GetConfig(context.Background()), opt, value), envKey)
 	} else if opt.NoPrefix {
 		// For options with NoPrefix set, check without prefix too
 		envKey := OptionToEnv(key)
 		value, ok = os.LookupEnv(envKey)
 		if ok {
-			Debugf(nil, "Setting %s=%q for %s from environment variable %s", key, value, oev.prefix, envKey)
+			Debugf(nil, "Setting %s=%s for %s from environment variable %s", key, RedactOptionValue(GetConfig(context.Background()), opt, value), oev.prefix, envKey)
 		}
 	}
 	return value, ok
@@ -125,7 +130,7 @@ func ConfigMap(prefix string, options Options, configName string, connectionStri
 
 	// remote specific environment vars
 	if configName != "" {
-		config.AddGetter(configEnvVars(configName), configmap.PriorityNormal)
+		config.AddGetter(configEnvVars{configName: configName, options: options}, configmap.PriorityNormal)
 	}
 
 	// backend specific environment vars

fs/configmap_test.go

@@ -0,0 +1,93 @@
+package fs
+
+import (
+	"bytes"
+	"context"
+	"log/slog"
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+)
+
+// captureLog redirects the fs logger into a buffer and enables debug
+// logging for the duration of the test.
+func captureLog(t *testing.T) *bytes.Buffer {
+	var buf bytes.Buffer
+	oldLogger := logger
+	t.Cleanup(func() { logger = oldLogger })
+	SetLogger(slog.NewTextHandler(&buf, &slog.HandlerOptions{Level: slog.LevelDebug}))
+	ci := GetConfig(context.Background())
+	oldLevel := ci.LogLevel
+	t.Cleanup(func() { ci.LogLevel = oldLevel })
+	ci.LogLevel = LogLevelDebug
+	return &buf
+}
+
+// TestConfigEnvVarsRedactsPassword checks that the value of a
+// password config option set via the environment
+// (RCLONE_CONFIG_backend_option) is not logged (see #5794).
+func TestConfigEnvVarsRedactsPassword(t *testing.T) {
+	t.Setenv("RCLONE_CONFIG_SRC_PASS", "obscured_pass")
+	t.Setenv("RCLONE_CONFIG_SRC_HOST", "example.com")
+	buf := captureLog(t)
+
+	config := configEnvVars{
+		configName: "src",
+		options: Options{
+			{Name: "pass", IsPassword: true},
+			{Name: "host"},
+		},
+	}
+
+	// The value is still returned to the caller...
+	value, ok := config.Get("pass")
+	assert.True(t, ok)
+	assert.Equal(t, "obscured_pass", value)
+	// ...but not logged
+	assert.NotContains(t, buf.String(), "obscured_pass")
+
+	// Non sensitive values are still logged
+	buf.Reset()
+	value, ok = config.Get("host")
+	assert.True(t, ok)
+	assert.Equal(t, "example.com", value)
+	assert.Contains(t, buf.String(), "example.com")
+}
+
+// TestConfigEnvVarsDumpAuthShowsPassword checks that with --dump auth
+// set the password value is still shown in the log for debugging.
+func TestConfigEnvVarsDumpAuthShowsPassword(t *testing.T) {
+	t.Setenv("RCLONE_CONFIG_SRC_PASS", "obscured_pass")
+	buf := captureLog(t)
+	ci := GetConfig(context.Background())
+	oldDump := ci.Dump
+	t.Cleanup(func() { ci.Dump = oldDump })
+	ci.Dump = DumpAuth
+
+	config := configEnvVars{
+		configName: "src",
+		options: Options{
+			{Name: "pass", IsPassword: true},
+		},
+	}
+	_, ok := config.Get("pass")
+	assert.True(t, ok)
+	assert.Contains(t, buf.String(), "obscured_pass")
+}
+
+// TestOptionEnvVarsRedactsPassword checks that the value of a
+// password option set via the environment (RCLONE_backend_option) is
+// not logged (see #5794).
+func TestOptionEnvVarsRedactsPassword(t *testing.T) {
+	t.Setenv("RCLONE_SFTP_PASS", "obscured_pass")
+	buf := captureLog(t)
+
+	oev := optionEnvVars{
+		prefix:  "sftp",
+		options: Options{{Name: "pass", IsPassword: true}},
+	}
+	value, ok := oev.Get("pass")
+	assert.True(t, ok)
+	assert.Equal(t, "obscured_pass", value)
+	assert.NotContains(t, buf.String(), "obscured_pass")
+}

fs/registry_test.go

@@ -254,7 +254,7 @@ func TestOptionGetters(t *testing.T) {
 	// set up getters
 
 	// A configmap.Getter to read from the environment RCLONE_CONFIG_backend_option_name
-	configEnvVarsGetter := configEnvVars("local")
+	configEnvVarsGetter := configEnvVars{configName: "local"}
 
 	// A configmap.Getter to read from the environment RCLONE_option_name
 	optionEnvVarsGetter := optionEnvVars{"local", testOptions}