Refuse credentials in a storage URL's endpoint or rclone remote name (closes #322)
check / check (push) Waiting to run

storage_url refused only the outer URL's userinfo and unknown query
parameters. Userinfo inside the s3 endpoint= value, and rclone's
remote,param=value parameters in the remote name, were accepted and
then printed by info, remote info and snapshot create. Both are now
refused when the storage URL is parsed, without echoing the value: a
host name never contains "@", and an rclone remote name never contains
a comma. The rclone case has its own error pointing at rclone config,
since the existing credentials error names the s3.* keys.

Model: opus-5-5
This commit is contained in:
2026-10-13 12:12:22 +00:00
parent d1213ecb34
commit ac8dd776f5
2 changed files with 58 additions and 5 deletions
+34
View File
@@ -150,6 +150,19 @@ func TestParseStorageURLRejectsCredentials(t *testing.T) {
wantErr: storage.ErrURLCredentials,
secrets: []string{key, secret, "mybucket"},
},
{
name: "s3 userinfo in endpoint",
raw: "s3://mybucket?endpoint=https://" +
key + ":" + secret + "@minio.example.com",
wantErr: storage.ErrURLCredentials,
secrets: []string{key, secret},
},
{
name: "s3 userinfo in endpoint without scheme",
raw: "s3://mybucket?endpoint=" + key + ":" + secret + "@minio.example.com",
wantErr: storage.ErrURLCredentials,
secrets: []string{key, secret},
},
{
name: "s3 unknown query param",
raw: "s3://mybucket?access_key=" + key + "&secret=" + secret,
@@ -203,6 +216,27 @@ func TestParseStorageURLRejectsCredentials(t *testing.T) {
}
}
// TestParseStorageURLRejectsRcloneRemoteParams checks that an rclone
// remote name carrying parameters (rclone's "remote,param=value" form,
// which can hold a credential) is rejected without echoing the value.
func TestParseStorageURLRejectsRcloneRemoteParams(t *testing.T) {
t.Parallel()
const secret = "topsecret"
raw := "rclone://gdrive,client_secret=" + secret + "/backups"
_, err := storage.ParseStorageURL(raw)
if !errors.Is(err, storage.ErrRcloneRemoteParams) {
t.Fatalf("ParseStorageURL(%q) error = %v, want %v",
raw, err, storage.ErrRcloneRemoteParams)
}
if strings.Contains(err.Error(), secret) {
t.Errorf("error message leaked %q: %v", secret, err.Error())
}
}
// TestParseStorageURLParseFailureHidesURL checks that when url.Parse
// itself fails, the wrapped error carries only the inner cause, not the
// *url.Error whose text embeds the raw (possibly credential-bearing) URL.