From 7101f11133c2aefb267b85782eaf3bd3129232ad Mon Sep 17 00:00:00 2001 From: gunar Date: Sun, 1 Feb 2026 11:45:31 -0300 Subject: [PATCH] Fail fast for invalid RESTIC_PACK_SIZE env values (#5592) Co-authored-by: Michael Eischer --- changelog/unreleased/pull-5592 | 7 ++++++ cmd/restic/main.go | 6 ++++- internal/global/global.go | 19 ++++++++++++---- internal/global/global_test.go | 41 ++++++++++++++++++++++++++++++++++ 4 files changed, 68 insertions(+), 5 deletions(-) create mode 100644 changelog/unreleased/pull-5592 diff --git a/changelog/unreleased/pull-5592 b/changelog/unreleased/pull-5592 new file mode 100644 index 000000000..87a1bfd8b --- /dev/null +++ b/changelog/unreleased/pull-5592 @@ -0,0 +1,7 @@ +Bugfix: Return error if `RESTIC_PACK_SIZE` contains invalid value + +If the environment variable `RESTIC_PACK_SIZE` could not be parsed, then +restic ignored its value. Now, the restic commands fail with an error, unless +the command-line option `--pack-size` was specified. + +https://github.com/restic/restic/pull/5592 diff --git a/cmd/restic/main.go b/cmd/restic/main.go index 3307d2787..6ed2811f3 100644 --- a/cmd/restic/main.go +++ b/cmd/restic/main.go @@ -49,6 +49,10 @@ The full documentation can be found at https://restic.readthedocs.io/ . DisableAutoGenTag: true, PersistentPreRunE: func(c *cobra.Command, _ []string) error { + switch c.Name() { + case "__complete", "__completeNoDesc": + return nil + } return globalOptions.PreRun(needsPassword(c.Name())) }, } @@ -114,7 +118,7 @@ The full documentation can be found at https://restic.readthedocs.io/ . // user for authentication). func needsPassword(cmd string) bool { switch cmd { - case "cache", "generate", "help", "options", "self-update", "version", "__complete": + case "cache", "generate", "help", "options", "self-update", "version", "__complete", "__completeNoDesc": return false default: return true diff --git a/internal/global/global.go b/internal/global/global.go index c65338b92..88439462a 100644 --- a/internal/global/global.go +++ b/internal/global/global.go @@ -81,6 +81,9 @@ type Options struct { Options []string Extended options.Options + + // packSizeFlag is used to detect if --pack-size was set (CLI overrides env). + packSizeFlag *pflag.Flag } func (opts *Options) AddFlags(f *pflag.FlagSet) { @@ -106,7 +109,8 @@ func (opts *Options) AddFlags(f *pflag.FlagSet) { f.BoolVar(&opts.NoExtraVerify, "no-extra-verify", false, "skip additional verification of data before upload (see documentation)") f.IntVar(&opts.Limits.UploadKb, "limit-upload", 0, "limits uploads to a maximum `rate` in KiB/s. (default: unlimited)") f.IntVar(&opts.Limits.DownloadKb, "limit-download", 0, "limits downloads to a maximum `rate` in KiB/s. (default: unlimited)") - f.UintVar(&opts.PackSize, "pack-size", 0, "set target pack `size` in MiB, created pack files may be larger (default: $RESTIC_PACK_SIZE)") + const packSizeFlag = "pack-size" + f.UintVar(&opts.PackSize, packSizeFlag, 0, "set target pack `size` in MiB, created pack files may be larger (default: $RESTIC_PACK_SIZE)") f.StringSliceVarP(&opts.Options, "option", "o", []string{}, "set extended option (`key=value`, can be specified multiple times)") f.StringVar(&opts.HTTPUserAgent, "http-user-agent", "", "set a http user agent for outgoing http requests") f.DurationVar(&opts.StuckRequestTimeout, "stuck-request-timeout", 5*time.Minute, "`duration` after which to retry stuck requests") @@ -125,9 +129,7 @@ func (opts *Options) AddFlags(f *pflag.FlagSet) { // ignore error as there's no good way to handle it _ = opts.Compression.Set(comp) } - // parse target pack size from env, on error the default value will be used - targetPackSize, _ := strconv.ParseUint(os.Getenv("RESTIC_PACK_SIZE"), 10, 32) - opts.PackSize = uint(targetPackSize) + opts.packSizeFlag = f.Lookup(packSizeFlag) if os.Getenv("RESTIC_HTTP_USER_AGENT") != "" { opts.HTTPUserAgent = os.Getenv("RESTIC_HTTP_USER_AGENT") @@ -135,6 +137,15 @@ func (opts *Options) AddFlags(f *pflag.FlagSet) { } func (opts *Options) PreRun(needsPassword bool) error { + if envVal := os.Getenv("RESTIC_PACK_SIZE"); envVal != "" && !opts.packSizeFlag.Changed { + targetPackSize, err := strconv.ParseUint(envVal, 10, 32) + if err != nil { + // Failing fast here keeps backups from running for a long time with the wrong pack size. + return errors.Fatalf("invalid value for RESTIC_PACK_SIZE %q: %v", envVal, err) + } + opts.PackSize = uint(targetPackSize) + } + // set verbosity, default is one opts.Verbosity = 1 if opts.Quiet && opts.Verbose > 0 { diff --git a/internal/global/global_test.go b/internal/global/global_test.go index 7d5ead722..474d9d0e1 100644 --- a/internal/global/global_test.go +++ b/internal/global/global_test.go @@ -7,6 +7,9 @@ import ( "strings" "testing" + "github.com/spf13/pflag" + + "github.com/restic/restic/internal/errors" rtest "github.com/restic/restic/internal/test" ) @@ -49,3 +52,41 @@ func TestReadEmptyPassword(t *testing.T) { _, err = readPassword(context.TODO(), opts, "test") rtest.Assert(t, strings.Contains(err.Error(), "must not be specified together with providing a password via a cli option or environment variable"), "unexpected error message, got %v", err) } + +func TestPackSizeEnvParseError(t *testing.T) { + t.Setenv("RESTIC_PACK_SIZE", "64MiB") + + var gopts Options + gopts.AddFlags(pflag.NewFlagSet("test", pflag.ContinueOnError)) + + err := gopts.PreRun(false) + rtest.Assert(t, err != nil, "expected error for invalid pack size env") + rtest.Assert(t, errors.IsFatal(err), "expected fatal error for invalid pack size env, got %T", err) + rtest.Assert(t, strings.Contains(err.Error(), "RESTIC_PACK_SIZE"), "error should mention RESTIC_PACK_SIZE, got %v", err) +} + +func TestPackSizeEnvApplied(t *testing.T) { + t.Setenv("RESTIC_PACK_SIZE", "64") + + var gopts Options + gopts.AddFlags(pflag.NewFlagSet("test", pflag.ContinueOnError)) + + err := gopts.PreRun(false) + rtest.OK(t, err) + rtest.Equals(t, uint(64), gopts.PackSize) +} + +func TestPackSizeEnvIgnoredWhenFlagSet(t *testing.T) { + t.Setenv("RESTIC_PACK_SIZE", "64MiB") + + var gopts Options + fs := pflag.NewFlagSet("test", pflag.ContinueOnError) + gopts.AddFlags(fs) + + err := fs.Set("pack-size", "64") + rtest.OK(t, err) + + err = gopts.PreRun(false) + rtest.OK(t, err) + rtest.Equals(t, uint(64), gopts.PackSize) +}