Compare commits

..
Author SHA1 Message Date
sneak 061ecc69dc 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 without echoing the value: a host name never contains "@", and
an rclone remote name never contains a comma. The comma is checked
before url.Parse, because url.Parse refuses a name such as
":s3,secret_access_key=..." or "x,secret=a:b", and its error prints
the name from the colon on. The rclone case has its own error pointing
at rclone config, since the existing credentials error names the s3.*
keys.

Model: opus-5-5
2026-10-13 13:41:26 +00:00
clawbot 8c54e0ff75 Make a mistyped subcommand under a group command an error (closes #319)
check / check (push) Waiting to run
snapshot, config, remote, database and completion had no Run, and
cobra checks a command's arguments only when it has one, so `vaultik
snapshot craete` printed the group's help and exited 0. Each group now
prints its help only when given no argument; any other word fails as
an unknown command with exit 1, as at the top level.

cobra adds the completion command only when Execute runs, so the root
command now adds it itself to give it a Run. Its scripts then go to
os.Stdout as it was at that moment; a later SetOut does not redirect
them.

Judgement call: the error has no "Did you mean" suggestion, which the
top level gives.

Model: opus-5-5
2026-10-13 14:14:39 +02:00
clawbot d1213ecb34 Make a leading **/ in an exclude pattern match at the top level (closes #303)
check / check (push) Waiting to run
An unanchored exclude pattern such as **/node_modules or **/*.pyc
needed a / before the name, so it excluded sub/node_modules and
sub/y.pyc but backed up node_modules/ and x.pyc directly under the
snapshot path. An unanchored pattern already matches at every depth,
so the leading **/ is now dropped when the pattern is compiled and,
as in gitignore, the pattern matches the top level too.

Judgement call: an anchored /**/x is left as it was; the issue covers
patterns that start with **/.

Model: opus-5-5
2026-10-13 13:50:11 +02:00
clawbot 5f6455ba17 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 11:27:59 +02:00
clawbot 0288735aaf Remove the blob temp file when a backup stops part-way (closes #302)
check / check (push) Waiting to run
When `snapshot create` stopped on a file error, a full temporary
directory or Ctrl-C, the blob being packed was never finalized, and its
`vaultik-blob-*.tmp` file (up to `blob_size_limit` of encrypted data)
stayed in `$TMPDIR`. Phase 2 now calls the new
`Packer.DiscardCurrentBlob` on the way out, which closes and removes
that file. After a successful flush no blob is in progress, so the call
then does nothing.

The blob's row in the local index is left, as before, for the next
run's repair of interrupted blobs to remove.

Model: opus-5-5
2026-10-13 11:14:55 +02:00
18 changed files with 406 additions and 213 deletions
+5 -4
View File
@@ -372,9 +372,7 @@ on the destination in one go, use `vaultik remote nuke --force`.
store — its remote key (or an unambiguous leading part of it) as shown store — its remote key (or an unambiguous leading part of it) as shown
by `snapshot list`. See by `snapshot list`. See
[restoring on another machine](#restoring-on-another-machine). [restoring on another machine](#restoring-on-another-machine).
* Optional path arguments to restore specific files/directories (default: all). * Optional path arguments to restore specific files/directories (default: all)
A path that matches nothing in the snapshot is named in a warning, and the
command exits non-zero after restoring the paths that match
* Preserves file permissions, timestamps, ownership (ownership requires root), * Preserves file permissions, timestamps, ownership (ownership requires root),
symlinks, and empty directories symlinks, and empty directories
* `--verify`: After restoring, verify every file's chunk hashes match * `--verify`: After restoring, verify every file's chunk hashes match
@@ -435,7 +433,10 @@ object store. Credentials are read from `s3.access_key_id` and
`s3.secret_access_key` in the config file. `s3.secret_access_key` in the config file.
**Local filesystem** (`file:///path/to/backup`): Stores blobs and metadata on **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 **Rclone** (`rclone://remote/path`): Uses rclone's 70+ supported cloud
providers. Requires rclone to be configured separately (`rclone config`). providers. Requires rclone to be configured separately (`rclone config`).
+11
View File
@@ -245,6 +245,17 @@ func (p *Packer) Flush(ctx context.Context) error {
return nil return nil
} }
// DiscardCurrentBlob closes and removes the temp file of the blob being
// packed, if any, and drops that blob. A caller that stops before Flush
// must call it, or the temp file is left behind. Thread-safe.
func (p *Packer) DiscardCurrentBlob() {
p.mu.Lock()
defer p.mu.Unlock()
p.cleanupTempFile()
p.currentBlob = nil
}
// FinalizeBlob finalizes the current blob being assembled. // FinalizeBlob finalizes the current blob being assembled.
// This compresses the accumulated chunks, encrypts the result, and computes // This compresses the accumulated chunks, encrypts the result, and computes
// the content-addressed hash. The finalized blob is either passed to the // the content-addressed hash. The finalized blob is either passed to the
+6
View File
@@ -251,6 +251,12 @@ func NewConfigCommand() *cobra.Command {
Use: "config", Use: "config",
Short: "Manage the configuration file", Short: "Manage the configuration file",
Long: "Commands for creating, editing, and querying the vaultik config file.", Long: "Commands for creating, editing, and querying the vaultik config file.",
// cobra checks Args only on a command that has a Run: without
// one, a mistyped subcommand printed help and exited 0.
Args: cobra.NoArgs,
Run: func(cmd *cobra.Command, _ []string) {
_ = cmd.Help()
},
} }
cmd.AddCommand(newConfigInitCommand()) cmd.AddCommand(newConfigInitCommand())
+6
View File
@@ -15,6 +15,12 @@ func NewDatabaseCommand() *cobra.Command {
Use: "database", Use: "database",
Short: "Manage the local state database", Short: "Manage the local state database",
Long: `Commands for managing the local SQLite state database.`, Long: `Commands for managing the local SQLite state database.`,
// cobra checks Args only on a command that has a Run: without
// one, a mistyped subcommand printed help and exited 0.
Args: cobra.NoArgs,
Run: func(cmd *cobra.Command, _ []string) {
_ = cmd.Help()
},
} }
cmd.AddCommand( cmd.AddCommand(
@@ -0,0 +1,43 @@
package cli //nolint:testpackage // shares runEntry and the command-word constants
import (
"testing"
"github.com/stretchr/testify/assert"
)
// TestEntryGroupRejectsUnknownSubcommand runs each group command with a
// mistyped subcommand, as a crontab line might carry one. It must fail
// with exit 1 and name the word, as `vaultik bogus` does at the top
// level, instead of printing the group's help and exiting 0 having done
// nothing. The group name alone still prints its help.
//
//nolint:paralleltest // replaces os.Args, os.Stdout and os.Stderr
func TestEntryGroupRejectsUnknownSubcommand(t *testing.T) {
for _, testCase := range []struct {
group string
mistyped string
}{
{group: cmdSnapshot, mistyped: "craete"},
{group: "config", mistyped: "inti"},
{group: cmdRemote, mistyped: "ifno"},
{group: "database", mistyped: "delte"},
{group: "completion", mistyped: "nosuchshell"},
} {
t.Run(testCase.group, func(t *testing.T) {
code, stdout, stderr := runEntry(t,
testCase.group, testCase.mistyped)
assert.Equal(t, 1, code)
assert.Empty(t, stdout)
assert.Contains(t, stderr,
`unknown command "`+testCase.mistyped+`"`)
code, stdout, _ = runEntry(t, testCase.group)
assert.Equal(t, 0, code)
assert.Contains(t, stdout,
programName+" "+testCase.group+" [command]")
})
}
}
+6
View File
@@ -18,6 +18,12 @@ func NewRemoteCommand() *cobra.Command {
Use: "remote", Use: "remote",
Short: "Remote storage management commands", Short: "Remote storage management commands",
Long: "Commands for inspecting and managing remote storage", Long: "Commands for inspecting and managing remote storage",
// cobra checks Args only on a command that has a Run: without
// one, a mistyped subcommand printed help and exited 0.
Args: cobra.NoArgs,
Run: func(cmd *cobra.Command, _ []string) {
_ = cmd.Help()
},
} }
// Add subcommands // Add subcommands
+15
View File
@@ -74,6 +74,21 @@ on the source system.`,
NewDatabaseCommand(), NewDatabaseCommand(),
) )
// cobra adds the completion command only when Execute runs, and with
// no Run, so it skipped its own cobra.NoArgs and a mistyped shell name
// printed help and exited 0. Adding it here lets it have a Run. Its
// scripts go to os.Stdout as it is now: a later SetOut does not
// redirect them.
cmd.InitDefaultCompletionCmd()
for _, sub := range cmd.Commands() {
if sub.Name() == "completion" {
sub.Run = func(completion *cobra.Command, _ []string) {
_ = completion.Help()
}
}
}
return cmd return cmd
} }
+6
View File
@@ -41,6 +41,12 @@ func NewSnapshotCommand() *cobra.Command {
Use: "snapshot", Use: "snapshot",
Short: "Snapshot management commands", Short: "Snapshot management commands",
Long: "Commands for creating, listing, and managing snapshots", Long: "Commands for creating, listing, and managing snapshots",
// cobra checks Args only on a command that has a Run: without
// one, a mistyped subcommand printed help and exited 0.
Args: cobra.NoArgs,
Run: func(cmd *cobra.Command, _ []string) {
_ = cmd.Help()
},
} }
// Add subcommands // Add subcommands
-2
View File
@@ -30,8 +30,6 @@ This command will restore files from the specified snapshot to the
target directory. target directory.
If no paths are specified, all files are restored. If no paths are specified, all files are restored.
If paths are specified, only matching files/directories are restored. If paths are specified, only matching files/directories are restored.
A path that matches nothing in the snapshot is named in a warning, and
the command exits non-zero after restoring the paths that match.
The snapshot may be named by its ID or, when restoring on a host with no The snapshot may be named by its ID or, when restoring on a host with no
local index, by the remote key that 'snapshot list' prints for a local index, by the remote key that 'snapshot list' prints for a
+12 -2
View File
@@ -61,6 +61,9 @@ var (
errBadS3PartSize = errors.New("s3.part_size must be between 5MiB and 5GiB") errBadS3PartSize = errors.New("s3.part_size must be between 5MiB and 5GiB")
errBadStorageScheme = errors.New( errBadStorageScheme = errors.New(
"storage_url must start with s3://, file://, or rclone://") "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( errStorageNotConfigured = errors.New(
"storage not configured; set storage_url or provide s3.endpoint + " + "storage not configured; set storage_url or provide s3.endpoint + " +
"s3.bucket + credentials") "s3.bucket + credentials")
@@ -461,11 +464,18 @@ func (c *Config) validateStorage() error {
// validateStorageURL validates URL-based storage configuration. File and // validateStorageURL validates URL-based storage configuration. File and
// rclone URLs need no credentials; S3 URLs require the legacy s3.* // 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 { func (c *Config) validateStorageURL() error {
switch { switch {
case strings.HasPrefix(c.StorageURL, "file://"): 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 return nil
case strings.HasPrefix(c.StorageURL, "rclone://"): case strings.HasPrefix(c.StorageURL, "rclone://"):
// Rclone storage uses rclone's own config // 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 // TestValidateAgeRecipients checks that recipients are parsed at config load
// (a bad entry fails immediately, not mid-backup) and that no invalid entry — // (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 // least of all a pasted secret key — is echoed in the error. An empty list
+47
View File
@@ -531,3 +531,50 @@ func TestExcludePatterns_UnanchoredPatternFile(t *testing.T) {
require.Equal(t, 1, result.FilesScanned, require.Equal(t, 1, result.FilesScanned,
"Unanchored pattern for file should match anywhere") "Unanchored pattern for file should match anywhere")
} }
func TestExcludePatterns_DoubleStarDirectoryAtEveryDepth(t *testing.T) {
t.Parallel()
// As in gitignore, a leading **/ also matches directly under the source dir
fs := setupAnchoredTestFS(t)
scanner, repos, cleanup := createTestScanner(t, fs, []string{"**/projectname"})
defer cleanup()
ctx := context.Background()
createSnapshotRecord(ctx, t, repos, "test-snapshot")
result, err := scanner.Scan(ctx, "/backup", "test-snapshot")
require.NoError(t, err)
// **/projectname should exclude BOTH:
// - /backup/projectname/file.txt
// - /backup/otherproject/projectname/file.txt
// Total files: 4, excluded: 2
require.Equal(t, 2, result.FilesScanned,
"**/ directory pattern should match at the top level and nested")
}
func TestExcludePatterns_DoubleStarFileAtEveryDepth(t *testing.T) {
t.Parallel()
// As in gitignore, a leading **/ also matches directly under the source dir
fs := setupAnchoredTestFS(t)
scanner, repos, cleanup := createTestScanner(t, fs, []string{"**/*.txt"})
defer cleanup()
ctx := context.Background()
createSnapshotRecord(ctx, t, repos, "test-snapshot")
result, err := scanner.Scan(ctx, "/backup", "test-snapshot")
require.NoError(t, err)
// **/*.txt should exclude ALL .txt files:
// - /backup/file.txt
// - /backup/projectname/file.txt
// - /backup/otherproject/projectname/file.txt
// Total files: 4, excluded: 3
require.Equal(t, 1, result.FilesScanned,
"**/ file pattern should match at the top level and nested")
}
+13
View File
@@ -1295,6 +1295,11 @@ func (s *Scanner) batchAddFilesToSnapshot(
func (s *Scanner) processPhase( func (s *Scanner) processPhase(
ctx context.Context, filesToProcess []*FileToProcess, result *ScanResult, ctx context.Context, filesToProcess []*FileToProcess, result *ScanResult,
) error { ) error {
// On an error or cancellation the blob being packed is never
// finalized, so its temp file must be removed here. After a
// successful flush there is no such blob and this does nothing.
defer s.packer.DiscardCurrentBlob()
// Calculate total bytes to process // Calculate total bytes to process
var totalBytes int64 var totalBytes int64
for _, f := range filesToProcess { for _, f := range filesToProcess {
@@ -2013,6 +2018,14 @@ func compileExcludePatterns(patterns []string) []compiledPattern {
// Remove trailing slash if present (directory indicator) // Remove trailing slash if present (directory indicator)
pattern = strings.TrimSuffix(pattern, "/") pattern = strings.TrimSuffix(pattern, "/")
// A leading "**/" demands a "/" before the name and so misses an
// entry directly under the source path. An unanchored pattern
// already matches at every depth, so drop the prefix: as in
// gitignore, "**/x" then also matches a top-level "x".
if !anchored {
pattern = strings.TrimPrefix(pattern, "**/")
}
// Compile the glob pattern // Compile the glob pattern
// For patterns without path separators, we need to match them as components // For patterns without path separators, we need to match them as components
// e.g., ".git" should match ".git" anywhere in the path // e.g., ".git" should match ".git" anywhere in the path
+57
View File
@@ -381,3 +381,60 @@ func TestScannerCancelStopsSkipErrorsRun(t *testing.T) {
}) })
} }
} }
// TestScannerRemovesBlobTempFileWhenRunStops checks that a run stopped
// part-way, by a file it cannot read or by a cancellation, removes the
// temp file of the blob it was packing.
func TestScannerRemovesBlobTempFileWhenRunStops(t *testing.T) {
t.Parallel()
// a.txt is backed up first and starts a blob; the run stops at b.txt.
const target = "/source/b.txt"
tests := []struct {
name string
newFs func(cancel context.CancelFunc) afero.Fs
}{
{
name: "file cannot be read",
newFs: func(context.CancelFunc) afero.Fs {
return &readFailFs{Fs: afero.NewMemMapFs(), target: target}
},
},
{
name: "cancelled",
newFs: func(cancel context.CancelFunc) afero.Fs {
return &cancelOnOpenFs{
Fs: afero.NewMemMapFs(), target: target, cancel: cancel,
}
},
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
t.Parallel()
ctx, cancel := context.WithCancel(context.Background())
defer cancel()
fs := tt.newFs(cancel)
writeSkipErrorTestFile(t, fs, "/source/a.txt", "first file content")
writeSkipErrorTestFile(t, fs, target, "second file content")
_, err := runSkipErrorScan(ctx, t, fs, "/source", false, nil)
if err == nil {
t.Fatal("expected the scan to stop with an error, got nil")
}
left, err := afero.Glob(fs, filepath.Join(os.TempDir(), "vaultik-blob-*"))
if err != nil {
t.Fatalf("listing blob temp files: %v", err)
}
if len(left) != 0 {
t.Fatalf("expected no blob temp file left, got %v", left)
}
})
}
}
+58 -18
View File
@@ -17,8 +17,10 @@ const (
// Sentinel errors for storage URL parsing. // Sentinel errors for storage URL parsing.
var ( var (
ErrEmptyStorageURL = errors.New("storage URL is empty") ErrEmptyStorageURL = errors.New("storage URL is empty")
ErrEmptyFilePath = errors.New("file URL path 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") ErrMissingBucket = errors.New("s3 URL missing bucket name")
ErrMissingRemote = errors.New("rclone URL missing remote name") ErrMissingRemote = errors.New("rclone URL missing remote name")
ErrUnsupportedScheme = errors.New( ErrUnsupportedScheme = errors.New(
@@ -27,7 +29,10 @@ var (
ErrURLCredentials = errors.New( ErrURLCredentials = errors.New(
"storage URL must not carry credentials; " + "storage URL must not carry credentials; " +
"set s3.access_key_id and s3.secret_access_key in the config instead") "set s3.access_key_id and s3.secret_access_key in the config instead")
ErrURLUnknownParam = errors.New("unknown query parameter in storage URL") ErrURLUnknownParam = errors.New("unknown query parameter in storage URL")
ErrRcloneRemoteParams = errors.New(
"rclone URL must not carry parameters in the remote name; " +
"set them on the remote with rclone config instead")
) )
// URL represents a parsed storage URL. // URL represents a parsed storage URL.
@@ -51,17 +56,8 @@ func ParseStorageURL(rawURL string) (*URL, error) {
return nil, ErrEmptyStorageURL return nil, ErrEmptyStorageURL
} }
// Handle file:// URLs
if after, ok := strings.CutPrefix(rawURL, "file://"); ok { if after, ok := strings.CutPrefix(rawURL, "file://"); ok {
path := after return parseFileURL(after)
if path == "" {
return nil, ErrEmptyFilePath
}
return &URL{
Scheme: schemeFile,
Prefix: path,
}, nil
} }
if strings.HasPrefix(rawURL, "s3://") { if strings.HasPrefix(rawURL, "s3://") {
@@ -75,9 +71,31 @@ func ParseStorageURL(rawURL string) (*URL, error) {
return nil, ErrUnsupportedScheme 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 // parseS3URL parses an s3://bucket/prefix URL. It rejects credentials in
// the userinfo and any query parameter other than endpoint, region and // the userinfo or in the endpoint parameter's userinfo, and any query
// ssl, so a credential-bearing URL is never stored or echoed. // parameter other than endpoint, region and ssl, so a credential-bearing
// URL is never stored or echoed.
func parseS3URL(rawURL string) (*URL, error) { func parseS3URL(rawURL string) (*URL, error) {
u, err := url.Parse(rawURL) u, err := url.Parse(rawURL)
if err != nil { if err != nil {
@@ -100,6 +118,14 @@ func parseS3URL(rawURL string) (*URL, error) {
return nil, err return nil, err
} }
// A host name never contains "@", so one in an endpoint, with or
// without a scheme, can only introduce userinfo.
for _, endpoint := range query["endpoint"] {
if strings.Contains(endpoint, "@") {
return nil, ErrURLCredentials
}
}
return &URL{ return &URL{
Scheme: schemeS3, Scheme: schemeS3,
Bucket: bucket, Bucket: bucket,
@@ -111,9 +137,23 @@ func parseS3URL(rawURL string) (*URL, error) {
} }
// parseRcloneURL parses an rclone://remote/path URL. rclone:// takes no // parseRcloneURL parses an rclone://remote/path URL. rclone:// takes no
// query parameters, so credentials in the userinfo and any parameter at // query parameters, so credentials in the userinfo, parameters in the
// all are rejected rather than silently ignored. // remote name and any query parameter at all are rejected rather than
// silently ignored.
func parseRcloneURL(rawURL string) (*URL, error) { func parseRcloneURL(rawURL string) (*URL, error) {
// The remote name runs up to the first "/", "?" or "#". It never
// contains a comma; one starts rclone's remote,param=value form,
// whose values can be credentials. This is checked before url.Parse,
// whose error for such a name can print part of it.
remote := strings.TrimPrefix(rawURL, "rclone://")
if end := strings.IndexAny(remote, "/?#"); end >= 0 {
remote = remote[:end]
}
if strings.Contains(remote, ",") {
return nil, ErrRcloneRemoteParams
}
u, err := url.Parse(rawURL) u, err := url.Parse(rawURL)
if err != nil { if err != nil {
return nil, wrapParseError(err) return nil, wrapParseError(err)
@@ -123,7 +163,7 @@ func parseRcloneURL(rawURL string) (*URL, error) {
return nil, ErrURLCredentials return nil, ErrURLCredentials
} }
remote := u.Host remote = u.Host
if remote == "" { if remote == "" {
return nil, ErrMissingRemote return nil, ErrMissingRemote
} }
+77 -2
View File
@@ -14,7 +14,10 @@ import (
func TestParseStorageURLValid(t *testing.T) { func TestParseStorageURLValid(t *testing.T) {
t.Parallel() t.Parallel()
const bucket = "mybucket" const (
bucket = "mybucket"
fileScheme = "file"
)
cases := []struct { cases := []struct {
name string name string
@@ -24,7 +27,17 @@ func TestParseStorageURLValid(t *testing.T) {
{ {
name: "file absolute path", name: "file absolute path",
raw: "file:///var/backups/vaultik", 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", name: "s3 bucket and prefix, ssl defaults on",
@@ -90,6 +103,7 @@ func TestParseStorageURLErrors(t *testing.T) {
}{ }{
{"empty url", "", storage.ErrEmptyStorageURL}, {"empty url", "", storage.ErrEmptyStorageURL},
{"file empty path", "file://", storage.ErrEmptyFilePath}, {"file empty path", "file://", storage.ErrEmptyFilePath},
{"file relative path", "file://dest", storage.ErrRelativeFilePath},
{"s3 missing bucket", "s3://", storage.ErrMissingBucket}, {"s3 missing bucket", "s3://", storage.ErrMissingBucket},
{"s3 missing bucket with path", "s3:///justprefix", storage.ErrMissingBucket}, {"s3 missing bucket with path", "s3:///justprefix", storage.ErrMissingBucket},
{"rclone missing remote", "rclone://", storage.ErrMissingRemote}, {"rclone missing remote", "rclone://", storage.ErrMissingRemote},
@@ -136,6 +150,19 @@ func TestParseStorageURLRejectsCredentials(t *testing.T) {
wantErr: storage.ErrURLCredentials, wantErr: storage.ErrURLCredentials,
secrets: []string{key, secret, "mybucket"}, 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", name: "s3 unknown query param",
raw: "s3://mybucket?access_key=" + key + "&secret=" + secret, raw: "s3://mybucket?access_key=" + key + "&secret=" + secret,
@@ -189,6 +216,54 @@ 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,
// including the URLs that url.Parse itself refuses.
func TestParseStorageURLRejectsRcloneRemoteParams(t *testing.T) {
t.Parallel()
cases := []struct {
name string
raw string
secrets []string // must not appear in the error message
}{
{
name: "configured remote",
raw: "rclone://gdrive,client_secret=topsecret/backups",
secrets: []string{"topsecret"},
},
{
name: "remote defined inline",
raw: "rclone://:s3,access_key_id=KEY,secret_access_key=SECRET/bucket",
secrets: []string{"KEY", "SECRET"},
},
{
name: "colon in a parameter value",
raw: "rclone://gdrive,client_secret=ab:cd/backups",
secrets: []string{"cd"},
},
}
for _, tc := range cases {
t.Run(tc.name, func(t *testing.T) {
t.Parallel()
_, err := storage.ParseStorageURL(tc.raw)
if !errors.Is(err, storage.ErrRcloneRemoteParams) {
t.Fatalf("ParseStorageURL(%q) error = %v, want %v",
tc.raw, err, storage.ErrRcloneRemoteParams)
}
for _, secret := range tc.secrets {
if strings.Contains(err.Error(), secret) {
t.Errorf("error message leaked %q: %v", secret, err.Error())
}
}
})
}
}
// TestParseStorageURLParseFailureHidesURL checks that when url.Parse // TestParseStorageURLParseFailureHidesURL checks that when url.Parse
// itself fails, the wrapped error carries only the inner cause, not the // itself fails, the wrapped error carries only the inner cause, not the
// *url.Error whose text embeds the raw (possibly credential-bearing) URL. // *url.Error whose text embeds the raw (possibly credential-bearing) URL.
+15 -43
View File
@@ -27,7 +27,6 @@ import (
// Sentinel errors for restore failures. // Sentinel errors for restore failures.
var ( var (
errFilesFailedRestore = errors.New("file(s) failed to restore") errFilesFailedRestore = errors.New("file(s) failed to restore")
errPathsNotInSnapshot = errors.New("requested path(s) not in the snapshot")
errFilesFailedVerify = errors.New("files failed verification") errFilesFailedVerify = errors.New("files failed verification")
errDecryptionKeyRequired = errors.New( errDecryptionKeyRequired = errors.New(
"decryption key required for restore\n\n" + "decryption key required for restore\n\n" +
@@ -150,7 +149,7 @@ func (v *Vaultik) Restore(opts *RestoreOptions) error {
repos := database.NewRepositories(tempDB) repos := database.NewRepositories(tempDB)
// Step 2: Get list of files to restore // Step 2: Get list of files to restore
files, unmatched, err := v.getFilesToRestore(v.ctx, repos, opts.Paths) files, err := v.getFilesToRestore(v.ctx, repos, opts.Paths)
if err != nil { if err != nil {
return fmt.Errorf("getting files to restore: %w", err) return fmt.Errorf("getting files to restore: %w", err)
} }
@@ -159,7 +158,7 @@ func (v *Vaultik) Restore(opts *RestoreOptions) error {
log.Warn("No files found to restore") log.Warn("No files found to restore")
v.UI.Warningf("No files found to restore.") v.UI.Warningf("No files found to restore.")
return v.reportUnmatchedPaths(unmatched) return nil
} }
log.Info("Found files to restore", "count", len(files)) log.Info("Found files to restore", "count", len(files))
@@ -199,16 +198,14 @@ func (v *Vaultik) Restore(opts *RestoreOptions) error {
v.UI.Duration(result.Duration), v.UI.Duration(result.Duration),
) )
return v.finishRestore(repos, files, unmatched, opts, result) return v.finishRestore(repos, files, opts, result)
} }
// finishRestore emits the post-restore warnings, runs optional // finishRestore emits the post-restore warnings, runs optional
// verification, and converts any failed-file count or requested path // verification, and converts any failed-file count into an error.
// that matched nothing into an error.
func (v *Vaultik) finishRestore( func (v *Vaultik) finishRestore(
repos *database.Repositories, repos *database.Repositories,
files []*database.File, files []*database.File,
unmatched []string,
opts *RestoreOptions, opts *RestoreOptions,
result *RestoreResult, result *RestoreResult,
) error { ) error {
@@ -226,8 +223,6 @@ func (v *Vaultik) finishRestore(
} }
} }
unmatchedErr := v.reportUnmatchedPaths(unmatched)
// Run verification if requested // Run verification if requested
if opts.Verify { if opts.Verify {
err := v.handleRestoreVerification(repos, files, opts, result) err := v.handleRestoreVerification(repos, files, opts, result)
@@ -240,23 +235,7 @@ func (v *Vaultik) finishRestore(
return fmt.Errorf("%d %w", result.FilesFailed, errFilesFailedRestore) return fmt.Errorf("%d %w", result.FilesFailed, errFilesFailedRestore)
} }
return unmatchedErr return nil
}
// reportUnmatchedPaths warns, by name, about each requested path that
// matched nothing in the snapshot, and returns an error counting them,
// or nil when there are none.
func (v *Vaultik) reportUnmatchedPaths(unmatched []string) error {
if len(unmatched) == 0 {
return nil
}
for _, path := range unmatched {
log.Warn("Requested path matches nothing in the snapshot", "path", path)
v.UI.Warningf("%s matches nothing in the snapshot.", v.UI.Path(path))
}
return fmt.Errorf("%d %w", len(unmatched), errPathsNotInSnapshot)
} }
// restoreIdentities parses the configured age secret key once into every // restoreIdentities parses the configured age secret key once into every
@@ -833,34 +812,27 @@ func (v *Vaultik) materializeSnapshotDB(
return db, tempDir, nil return db, tempDir, nil
} }
// getFilesToRestore returns the list of files to restore based on path // getFilesToRestore returns the list of files to restore based on path filters
// filters, and the filters, as given, that matched no file.
func (v *Vaultik) getFilesToRestore( func (v *Vaultik) getFilesToRestore(
ctx context.Context, repos *database.Repositories, pathFilters []string, ctx context.Context, repos *database.Repositories, pathFilters []string,
) ([]*database.File, []string, error) { ) ([]*database.File, error) {
// If no filters, get all files // If no filters, get all files
if len(pathFilters) == 0 { if len(pathFilters) == 0 {
files, err := repos.Files.ListAll(ctx) return repos.Files.ListAll(ctx)
return files, nil, err
} }
// Get files matching the path filters // Get files matching the path filters
var ( var result []*database.File
result []*database.File
unmatched []string
)
seen := make(map[string]bool) seen := make(map[string]bool)
for _, filter := range pathFilters { for _, filter := range pathFilters {
files, err := repos.Files.ListUnderPath(ctx, filepath.Clean(filter)) // Normalize the filter path
if err != nil { filter = filepath.Clean(filter)
return nil, nil, fmt.Errorf("listing files under %s: %w", filter, err)
}
if len(files) == 0 { files, err := repos.Files.ListUnderPath(ctx, filter)
unmatched = append(unmatched, filter) if err != nil {
return nil, fmt.Errorf("listing files under %s: %w", filter, err)
} }
for _, file := range files { for _, file := range files {
@@ -871,7 +843,7 @@ func (v *Vaultik) getFilesToRestore(
} }
} }
return result, unmatched, nil return result, nil
} }
// buildChunkToBlobMap creates a mapping from chunk hash to blob information // buildChunkToBlobMap creates a mapping from chunk hash to blob information
@@ -1,142 +0,0 @@
package vaultik_test
import (
"bytes"
"context"
"os"
"path/filepath"
"strings"
"testing"
"github.com/spf13/afero"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
"sneak.berlin/go/vaultik/internal/config"
"sneak.berlin/go/vaultik/internal/database"
"sneak.berlin/go/vaultik/internal/log"
"sneak.berlin/go/vaultik/internal/storage"
"sneak.berlin/go/vaultik/internal/ui"
"sneak.berlin/go/vaultik/internal/vaultik"
)
// These tests cover https://git.eeqj.de/sneak/vaultik/issues/309: a path
// given to restore that matches nothing in the snapshot is named in a
// warning and makes the restore fail, after the paths that do match are
// restored.
func TestRestoreNamesUnmatchedPathAndRestoresTheMatchingOne(t *testing.T) {
log.Initialize(log.Config{})
t.Parallel()
fs := afero.NewOsFs()
tempDir := t.TempDir()
dataDir := filepath.Join(tempDir, "src")
storeDir := filepath.Join(tempDir, "remote")
restoreDir := filepath.Join(tempDir, "restored")
dbPath := filepath.Join(tempDir, "index.sqlite")
ctx := context.Background()
cfg := faultTestConfig()
files := writeFaultSourceTree(t, fs, dataDir)
store, err := storage.NewFileStorer(storeDir)
require.NoError(t, err)
db, err := database.New(ctx, dbPath)
require.NoError(t, err)
id := fullFaultBackup(ctx, t, fs, store, cfg,
database.NewRepositories(db), dataDir, dbPath, "unmatched")
require.NoError(t, db.Close())
matched := filepath.Join(dataDir, "small.txt")
unmatched := filepath.Join(dataDir, "typo.txt")
var out bytes.Buffer
v := newReaderVaultik(ctx, cfg, store, nil, fs)
v.UI = ui.NewWithColor(&out, false)
err = v.Restore(&vaultik.RestoreOptions{
SnapshotID: id,
TargetDir: restoreDir,
Paths: []string{matched, unmatched},
})
require.Error(t, err, "a path that matches nothing must fail the restore")
assert.Contains(t, out.String(), unmatched,
"the path that matches nothing must be named")
assert.NotContains(t, out.String(), matched,
"the path that matches must not be reported")
got, err := afero.ReadFile(fs, filepath.Join(restoreDir, matched))
require.NoError(t, err, "the path that matches must still be restored")
assert.Equal(t, files[matched], got)
}
// When no requested path matches, nothing is restored and each path is
// named. One path is relative. The other goes through the symlink that
// is the snapshot's configured path, while the entries are stored under
// the directory the symlink points to.
func TestRestoreNamesEveryPathWhenNothingMatches(t *testing.T) {
log.Initialize(log.Config{})
t.Parallel()
const snapshotName = "linked"
fs := afero.NewOsFs()
tempDir := t.TempDir()
dataDir := filepath.Join(tempDir, "src")
linkDir := filepath.Join(tempDir, "srclink")
storeDir := filepath.Join(tempDir, "remote")
restoreDir := filepath.Join(tempDir, "restored")
dbPath := filepath.Join(tempDir, "index.sqlite")
ctx := context.Background()
writeFaultSourceTree(t, fs, dataDir)
require.NoError(t, os.Symlink(dataDir, linkDir))
cfg := faultTestConfig()
cfg.IndexPath = dbPath
cfg.ChunkSize = config.Size(faultChunkSize)
cfg.Snapshots = map[string]config.SnapshotConfig{
snapshotName: {Paths: []string{linkDir}},
}
store, err := storage.NewFileStorer(storeDir)
require.NoError(t, err)
db, err := database.New(ctx, dbPath)
require.NoError(t, err)
repos := database.NewRepositories(db)
v := newBackupVaultik(ctx, cfg, store, repos, db, fs)
require.NoError(t, backUp(v, snapshotName))
id := localSnapshotID(ctx, t, repos, snapshotName)
require.NoError(t, db.Close())
throughLink := filepath.Join(linkDir, "sub")
relative := strings.TrimPrefix(filepath.Join(dataDir, "small.txt"), "/")
var out bytes.Buffer
reader := newReaderVaultik(ctx, cfg, store, nil, fs)
reader.UI = ui.NewWithColor(&out, false)
err = reader.Restore(&vaultik.RestoreOptions{
SnapshotID: id,
TargetDir: restoreDir,
Paths: []string{throughLink, relative},
})
require.Error(t, err, "a restore that matches nothing must fail")
assert.Contains(t, out.String(), throughLink,
"the path through the symlink must be named")
assert.Contains(t, out.String(), relative,
"the relative path must be named")
_, err = fs.Stat(restoreDir)
assert.True(t, os.IsNotExist(err), "nothing must be restored")
}