Refuse a relative file:// storage_url and decode its escapes (closes #299)
check / check (push) Waiting to run

The path after file:// was used as written. file://dest resolved
against the working directory, so a backup run from another directory
wrote elsewhere, exited 0 and left a snapshot that could not be
restored; my%20stick created a directory with that literal name.

Config load now refuses a file:// storage_url whose path is not
absolute. ParseStorageURL refuses it too and decodes percent-escapes;
a literal space still works. The rule is written in both packages
because config cannot import storage, which imports config.

A % not followed by two hex digits now fails when the storage is
opened, not at config load.

Model: opus-5-5
This commit was merged in pull request #330.
This commit is contained in:
2026-10-13 11:27:59 +02:00
parent 0288735aaf
commit 5f6455ba17
5 changed files with 87 additions and 17 deletions
+12 -2
View File
@@ -61,6 +61,9 @@ var (
errBadS3PartSize = errors.New("s3.part_size must be between 5MiB and 5GiB")
errBadStorageScheme = errors.New(
"storage_url must start with s3://, file://, or rclone://")
errFileURLNotAbsolute = errors.New(
"a file:// storage_url needs an absolute path, " +
"as in file:///path/to/backup")
errStorageNotConfigured = errors.New(
"storage not configured; set storage_url or provide s3.endpoint + " +
"s3.bucket + credentials")
@@ -461,11 +464,18 @@ func (c *Config) validateStorage() error {
// validateStorageURL validates URL-based storage configuration. File and
// rclone URLs need no credentials; S3 URLs require the legacy s3.*
// credential fields.
// credential fields. A file URL must name an absolute path: a relative one
// would resolve against the working directory, so the destination would
// move with it.
func (c *Config) validateStorageURL() error {
switch {
case strings.HasPrefix(c.StorageURL, "file://"):
// File storage doesn't need S3 credentials
// storage.ParseStorageURL applies the same rule, but config
// cannot import storage, which imports config.
if !strings.HasPrefix(c.StorageURL, "file:///") {
return errFileURLNotAbsolute
}
return nil
case strings.HasPrefix(c.StorageURL, "rclone://"):
// Rclone storage uses rclone's own config
+29
View File
@@ -337,6 +337,35 @@ func TestLoadS3PartSize(t *testing.T) {
})
}
// TestLoadRejectsRelativeFileURL checks that a file:// storage_url with a
// relative path fails at load. Accepted, it would resolve against the
// working directory, so a backup run from another directory would write
// to a different destination and still succeed.
func TestLoadRejectsRelativeFileURL(t *testing.T) {
t.Parallel()
const relativeConfig = "snapshots:\n" +
" test:\n" +
" paths: [/tmp/vaultik-test-source]\n" +
"storage_url: file://dest\n"
path := filepath.Join(t.TempDir(), "config.yml")
err := os.WriteFile(path, []byte(relativeConfig), 0o600)
if err != nil {
t.Fatalf("write config: %v", err)
}
_, err = Load(path)
if !errors.Is(err, errFileURLNotAbsolute) {
t.Fatalf("Load() error = %v, want errFileURLNotAbsolute", err)
}
if !strings.Contains(err.Error(), "absolute path") {
t.Errorf("error %q does not say to use an absolute path", err.Error())
}
}
// TestValidateAgeRecipients checks that recipients are parsed at config load
// (a bad entry fails immediately, not mid-backup) and that no invalid entry —
// least of all a pasted secret key — is echoed in the error. An empty list