diff --git a/fs/configmap.go b/fs/configmap.go index 7711f76b2..ae48189c3 100644 --- a/fs/configmap.go +++ b/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) { - envKey := ConfigToEnv(string(configName), key) +func (c configEnvVars) Get(key string) (value string, ok bool) { + 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 diff --git a/fs/configmap_test.go b/fs/configmap_test.go new file mode 100644 index 000000000..89b717e41 --- /dev/null +++ b/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") +} diff --git a/fs/registry_test.go b/fs/registry_test.go index 1f8ec58b0..6f62485c2 100644 --- a/fs/registry_test.go +++ b/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}