From adc7f2ebfa3fe412430944a36ef5e61910d21145 Mon Sep 17 00:00:00 2001 From: Pastalikek65 Date: Wed, 12 Aug 2026 10:20:10 +0300 Subject: [PATCH] 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 | 19 +++++---- fs/configmap_test.go | 93 ++++++++++++++++++++++++++++++++++++++++++++ fs/registry_test.go | 2 +- 3 files changed, 106 insertions(+), 8 deletions(-) create mode 100644 fs/configmap_test.go 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}