Author SHA1 Message Date
sneak 437ec591ad 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
2026-10-13 04:02:59 +00:00
11 changed files with 117 additions and 142 deletions
+4 -1
View File
@@ -429,7 +429,10 @@ object store. Credentials are read from `s3.access_key_id` and
`s3.secret_access_key` in the config file.
**Local filesystem** (`file:///path/to/backup`): Stores blobs and metadata on
a local or mounted filesystem. Useful for testing or backing up to a NAS.
a local or mounted filesystem. Useful for testing or backing up to a NAS. The
path must be absolute; a relative one is refused when the config loads.
Percent-escapes in the path are decoded, so `my%20stick` and `my stick` name
the same directory.
**Rclone** (`rclone://remote/path`): Uses rclone's 70+ supported cloud
providers. Requires rclone to be configured separately (`rclone config`).
+10 -9
View File
@@ -22,15 +22,16 @@ the tag exists and is exercised; what is left is merging `next` to
# Completed Steps
- 2026-10-13: Recorded the size of the stored object for a blob a backup
found already stored
([issue #300](https://git.eeqj.de/sneak/vaultik/issues/300)). When the
upload was skipped, the blob row and the manifest kept the size of the
local copy, which differs from the stored object's when
`compression_level` or the recipients changed since it was written.
`snapshot verify` then failed on a good snapshot, shallow and deep,
and the listed sizes were wrong. Marking the blob uploaded now records
the size of the object in storage.
- 2026-10-13: Refused a `file://` `storage_url` whose path is relative
([issue #299](https://git.eeqj.de/sneak/vaultik/issues/299)). The text
after `file://` was used as written, so `file://dest` resolved against
the working directory: a backup run from another directory wrote to a
different destination, still exited 0, and left a snapshot that could
not be restored. Such a URL now fails when the config loads, with an
error asking for an absolute path. Percent-escapes in the path are now
decoded, so `my%20stick` names the directory `my stick` instead of
creating one named `my%20stick`; a path written with a literal space
works as before.
- 2026-10-13: Made `snapshot remove` fail for a snapshot that does not
exist ([issue #293](https://git.eeqj.de/sneak/vaultik/issues/293)).
+2 -2
View File
@@ -90,7 +90,7 @@ Stores information about packed, compressed, and encrypted blob files.
- `created_ts` (INTEGER NOT NULL) - Creation timestamp
- `finished_ts` (INTEGER) - Finalization timestamp (NULL if in progress)
- `uncompressed_size` (INTEGER NOT NULL DEFAULT 0) - Total size of chunks before compression
- `compressed_size` (INTEGER NOT NULL DEFAULT 0) - Size after compression and encryption. Once uploaded, the size of the object in storage: when the blob was already stored, under another compression level or other recipients, this differs from the size of the local copy
- `compressed_size` (INTEGER NOT NULL DEFAULT 0) - Size after compression and encryption
- `uploaded_ts` (INTEGER) - Upload completion timestamp (NULL if not uploaded)
**Purpose:** Tracks blob lifecycle from creation through upload. The UUID primary key allows immediate association of chunks with blobs.
@@ -183,7 +183,7 @@ Tracks blob upload metrics.
- `UPDATE blobs SET blob_hash = ?, finished_ts = ?` - Finalize blob after packing
4. **Upload**
- `UPDATE blobs SET uploaded_ts = ?, compressed_size = ?` - Mark blob as uploaded, with the size of the object in storage (the upload is skipped when the blob is already stored)
- `UPDATE blobs SET uploaded_ts = ?` - Mark blob as uploaded
- `INSERT INTO uploads` - Record upload metrics with snapshot_id
- `INSERT INTO snapshot_blobs` - Associate blob with snapshot
+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
+6 -9
View File
@@ -159,16 +159,13 @@ func (r *BlobRepository) UpdateFinished(
return nil
}
// UpdateUploaded marks a blob as uploaded and records the size of the
// stored object as its compressed size. That size can differ from the
// local copy's when an identical blob was already stored with another
// compression level or other recipients.
// UpdateUploaded marks a blob as uploaded
func (r *BlobRepository) UpdateUploaded(
ctx context.Context, tx *sql.Tx, id string, storedSize int64,
ctx context.Context, tx *sql.Tx, id string,
) error {
query := `
UPDATE blobs
SET uploaded_ts = ?, compressed_size = ?
UPDATE blobs
SET uploaded_ts = ?
WHERE id = ?
`
@@ -176,9 +173,9 @@ func (r *BlobRepository) UpdateUploaded(
var err error
if tx != nil {
_, err = tx.ExecContext(ctx, query, now, storedSize, id)
_, err = tx.ExecContext(ctx, query, now, id)
} else {
_, err = r.db.ExecWithLog(ctx, query, now, storedSize, id)
_, err = r.db.ExecWithLog(ctx, query, now, id)
}
if err != nil {
+2 -6
View File
@@ -122,8 +122,8 @@ func TestBlobRepositoryUpdates(t *testing.T) {
t.Errorf("expected compressed size 500, got %d", updated.CompressedSize)
}
// Test UpdateUploaded, with a stored object larger than the local copy
err = repo.UpdateUploaded(ctx, nil, blob.ID.String(), 600)
// Test UpdateUploaded
err = repo.UpdateUploaded(ctx, nil, blob.ID.String())
if err != nil {
t.Fatalf("failed to update blob as uploaded: %v", err)
}
@@ -141,10 +141,6 @@ func TestBlobRepositoryUpdates(t *testing.T) {
if uploaded.UploadedTS.Before(now.Add(-1 * time.Second)) {
t.Error("uploaded timestamp should be around test time")
}
if uploaded.CompressedSize != 600 {
t.Errorf("expected the stored size 600, got %d", uploaded.CompressedSize)
}
}
func TestBlobRepositoryDuplicate(t *testing.T) {
+10 -16
View File
@@ -1509,8 +1509,7 @@ func (s *Scanner) finalizeProcessPhase(ctx context.Context, result *ScanResult)
// snapshot references has uploaded_ts set, so deduplication and
// interrupted-run repair treat these blobs as trustworthy.
err = s.repos.WithTx(ctx, func(ctx context.Context, tx *sql.Tx) error {
err := s.repos.Blobs.UpdateUploaded(
ctx, tx, b.ID, b.Compressed)
err := s.repos.Blobs.UpdateUploaded(ctx, tx, b.ID)
if err != nil {
return fmt.Errorf("marking blob uploaded: %w", err)
}
@@ -1546,7 +1545,7 @@ func (s *Scanner) handleBlobReady(
blobPath := fmt.Sprintf("blobs/%s/%s/%s",
finishedBlob.Hash[:2], finishedBlob.Hash[2:4], finishedBlob.Hash)
storedSize, blobExists, err := s.uploadBlobIfNeeded(
blobExists, err := s.uploadBlobIfNeeded(
ctx, blobPath, blobWithReader, startTime, result)
if err != nil {
s.cleanupBlobTempFile(blobWithReader)
@@ -1554,8 +1553,7 @@ func (s *Scanner) handleBlobReady(
return fmt.Errorf("uploading blob %s: %w", finishedBlob.Hash, err)
}
err = s.recordBlobMetadata(
ctx, finishedBlob, storedSize, blobExists, startTime)
err = s.recordBlobMetadata(ctx, finishedBlob, blobExists, startTime)
if err != nil {
s.cleanupBlobTempFile(blobWithReader)
@@ -1577,22 +1575,20 @@ func (s *Scanner) handleBlobReady(
}
// uploadBlobIfNeeded uploads the blob to storage if it doesn't already
// exist. It returns the size of the object in storage, which for an
// existing object can differ from the local copy's, and whether it
// already existed.
// exist, returns whether it existed
func (s *Scanner) uploadBlobIfNeeded(
ctx context.Context,
blobPath string,
blobWithReader *blob.WithReader,
startTime time.Time,
result *ScanResult,
) (int64, bool, error) {
) (bool, error) {
finishedBlob := blobWithReader.FinishedBlob
// Check if blob already exists (deduplication after restart)
destination := s.storage.Info().Location
stored, err := s.storage.Stat(ctx, blobPath)
_, err := s.storage.Stat(ctx, blobPath)
if err == nil {
log.Info("Blob already exists in storage, skipping upload",
"hash", finishedBlob.Hash,
@@ -1601,7 +1597,7 @@ func (s *Scanner) uploadBlobIfNeeded(
s.ui.Hex(finishedBlob.Hash), s.ui.Size(finishedBlob.Compressed),
s.ui.Path(destination))
return stored.Size, true, nil
return true, nil
}
s.ui.Beginf("Uploading blob %s (%s) to %s.",
@@ -1615,7 +1611,7 @@ func (s *Scanner) uploadBlobIfNeeded(
if err != nil {
log.Error("Failed to upload blob", "hash", finishedBlob.Hash, "error", err)
return 0, false, fmt.Errorf("uploading blob to storage: %w", err)
return false, fmt.Errorf("uploading blob to storage: %w", err)
}
uploadDuration := time.Since(startTime)
@@ -1646,7 +1642,7 @@ func (s *Scanner) uploadBlobIfNeeded(
stats.BytesUploaded.Add(finishedBlob.Compressed)
}
return finishedBlob.Compressed, false, nil
return false, nil
}
// makeUploadProgressCallback creates a progress callback for blob uploads.
@@ -1716,7 +1712,6 @@ func (s *Scanner) makeUploadProgressCallback(
func (s *Scanner) recordBlobMetadata(
ctx context.Context,
finishedBlob *blob.FinishedBlob,
storedSize int64,
blobExists bool,
startTime time.Time,
) error {
@@ -1728,8 +1723,7 @@ func (s *Scanner) recordBlobMetadata(
uploadDuration := time.Since(startTime)
return s.repos.WithTx(ctx, func(txCtx context.Context, tx *sql.Tx) error {
err := s.repos.Blobs.UpdateUploaded(
txCtx, tx, finishedBlob.ID, storedSize)
err := s.repos.Blobs.UpdateUploaded(txCtx, tx, finishedBlob.ID)
if err != nil {
return fmt.Errorf("updating blob upload timestamp: %w", err)
}
+26 -12
View File
@@ -17,8 +17,10 @@ const (
// Sentinel errors for storage URL parsing.
var (
ErrEmptyStorageURL = errors.New("storage URL is empty")
ErrEmptyFilePath = errors.New("file URL path is empty")
ErrEmptyStorageURL = errors.New("storage URL is empty")
ErrEmptyFilePath = errors.New("file URL path is empty")
ErrRelativeFilePath = errors.New(
"file URL needs an absolute path, as in file:///path/to/backup")
ErrMissingBucket = errors.New("s3 URL missing bucket name")
ErrMissingRemote = errors.New("rclone URL missing remote name")
ErrUnsupportedScheme = errors.New(
@@ -51,17 +53,8 @@ func ParseStorageURL(rawURL string) (*URL, error) {
return nil, ErrEmptyStorageURL
}
// Handle file:// URLs
if after, ok := strings.CutPrefix(rawURL, "file://"); ok {
path := after
if path == "" {
return nil, ErrEmptyFilePath
}
return &URL{
Scheme: schemeFile,
Prefix: path,
}, nil
return parseFileURL(after)
}
if strings.HasPrefix(rawURL, "s3://") {
@@ -75,6 +68,27 @@ func ParseStorageURL(rawURL string) (*URL, error) {
return nil, ErrUnsupportedScheme
}
// parseFileURL parses the path that follows file://. The path must be
// absolute: a relative one would resolve against the working directory,
// so the destination would move with it. Percent-escapes are decoded, so
// my%20stick names the directory "my stick"; a literal space is kept.
func parseFileURL(rawPath string) (*URL, error) {
if rawPath == "" {
return nil, ErrEmptyFilePath
}
if !strings.HasPrefix(rawPath, "/") {
return nil, ErrRelativeFilePath
}
path, err := url.PathUnescape(rawPath)
if err != nil {
return nil, fmt.Errorf("invalid file URL path: %w", err)
}
return &URL{Scheme: schemeFile, Prefix: path}, nil
}
// parseS3URL parses an s3://bucket/prefix URL. It rejects credentials in
// the userinfo and any query parameter other than endpoint, region and
// ssl, so a credential-bearing URL is never stored or echoed.
+16 -2
View File
@@ -14,7 +14,10 @@ import (
func TestParseStorageURLValid(t *testing.T) {
t.Parallel()
const bucket = "mybucket"
const (
bucket = "mybucket"
fileScheme = "file"
)
cases := []struct {
name string
@@ -24,7 +27,17 @@ func TestParseStorageURLValid(t *testing.T) {
{
name: "file absolute path",
raw: "file:///var/backups/vaultik",
want: &storage.URL{Scheme: "file", Prefix: "/var/backups/vaultik"},
want: &storage.URL{Scheme: fileScheme, Prefix: "/var/backups/vaultik"},
},
{
name: "file percent-escaped path is decoded",
raw: "file:///Volumes/my%20stick/backup",
want: &storage.URL{Scheme: fileScheme, Prefix: "/Volumes/my stick/backup"},
},
{
name: "file path with a literal space",
raw: "file:///Volumes/my stick/backup",
want: &storage.URL{Scheme: fileScheme, Prefix: "/Volumes/my stick/backup"},
},
{
name: "s3 bucket and prefix, ssl defaults on",
@@ -90,6 +103,7 @@ func TestParseStorageURLErrors(t *testing.T) {
}{
{"empty url", "", storage.ErrEmptyStorageURL},
{"file empty path", "file://", storage.ErrEmptyFilePath},
{"file relative path", "file://dest", storage.ErrRelativeFilePath},
{"s3 missing bucket", "s3://", storage.ErrMissingBucket},
{"s3 missing bucket with path", "s3:///justprefix", storage.ErrMissingBucket},
{"rclone missing remote", "rclone://", storage.ErrMissingRemote},
@@ -1,83 +0,0 @@
package vaultik_test
import (
"context"
"io"
"path/filepath"
"testing"
"filippo.io/age"
"github.com/spf13/afero"
"github.com/stretchr/testify/require"
"sneak.berlin/go/vaultik/internal/log"
"sneak.berlin/go/vaultik/internal/ui"
"sneak.berlin/go/vaultik/internal/vaultik"
)
// TestVerifyPassesWhenBackupReusesStoredBlob backs up a file, then backs it
// up again from an empty local index, as after `database delete`, with an
// extra age recipient. The second backup builds the same blob, finds it
// already stored and skips the upload, but its own copy is larger, because
// age writes a header entry for each recipient. The second snapshot must
// record the size of the stored object, so shallow and deep verify pass.
func TestVerifyPassesWhenBackupReusesStoredBlob(t *testing.T) {
log.Initialize(log.Config{})
t.Parallel()
fs := afero.NewOsFs()
tempDir := t.TempDir()
dataDir := filepath.Join(tempDir, "source")
storeDir := filepath.Join(tempDir, "remote")
secondDBPath := filepath.Join(tempDir, "second.sqlite")
chunkSize := int64(64 * 1024)
maxBlobSize := int64(512 * 1024)
require.NoError(t, fs.MkdirAll(dataDir, 0o755))
require.NoError(t, afero.WriteFile(fs,
filepath.Join(dataDir, "data.bin"),
bytesPattern("reuse-", int(chunkSize*3)), 0o644))
ctx := context.Background()
runFileStorageBackup(ctx, t, fs, dataDir, storeDir,
filepath.Join(tempDir, "first.sqlite"), chunkSize, maxBlobSize)
env := setupDedupBackupEnv(
ctx, t, fs, storeDir, secondDBPath, chunkSize, maxBlobSize)
defer func() { _ = env.db.Close() }()
extra, err := age.GenerateX25519Identity()
require.NoError(t, err)
env.cfg.AgeRecipients = append(env.cfg.AgeRecipients,
extra.Recipient().String())
snapshotID, result := runDedupSnapshot(ctx, t, env.sm, env.makeScanner(),
env.cfg.Hostname, "second", dataDir, secondDBPath)
require.Positive(t, result.BlobsCreated)
require.Zero(t, result.BlobsUploaded,
"the second backup should find every blob already stored")
require.NoError(t, env.db.Close())
verifier := &vaultik.Vaultik{
Config: env.cfg,
Storage: env.storer,
Fs: fs,
Stdout: io.Discard,
Stderr: io.Discard,
UI: ui.NewWithColor(io.Discard, false),
}
verifier.SetContext(ctx)
require.NoError(t,
verifier.VerifySnapshotWithOptions(snapshotID, &vaultik.VerifyOptions{}),
"shallow verify should pass")
require.NoError(t,
verifier.VerifySnapshotWithOptions(snapshotID,
&vaultik.VerifyOptions{Deep: true}),
"deep verify should pass")
}