Prevent unsafe uses of forget --keep-tag

changelog/unreleased/issue-4568

@@ -0,0 +1,17 @@
+Bugfix: Prevent `forget --keep-tags invalid` from deleting all snapshots
+
+Running `forget --keep-tags invalid`, where the tag `invalid` does not
+exist in the repository, would remove all snapshots. This is especially
+problematic if the tag name contains a typo.
+
+The `forget` command now fails with an error if all snapshots in a snapshot
+group would be deleted. This prevents the above example from deleting all
+snapshots.
+
+It is possible to temporarily disable the new check by setting the environment variable
+`RESTIC_FEATURES=safe-forget-keep-tags=false`. Note that this feature flag
+will be removed in the next minor restic version.
+
+https://github.com/restic/restic/issues/4568
+https://github.com/restic/restic/pull/4764
+https://forum.restic.net/t/delete-all-snapshots-in-one-command-is-this-feature-intentional/6923/3

changelog/unreleased/pull-4764

@@ -0,0 +1,8 @@
+Enhancement: Remove all snapshots using `forget --unsafe-allow-remove-all`
+
+The forget command now supports the `--unsafe-allow-remove-all` option. It must
+always be combined with a snapshot filter (by host, path or tag).
+For example the command `forget --tag example --unsafe-allow-remove-all`,
+removes all snapshots with tag `example`.
+
+https://github.com/restic/restic/pull/4764

cmd/restic/cmd_forget.go

@@ -3,10 +3,12 @@ package main
 import (
 	"context"
 	"encoding/json"
+	"fmt"
 	"io"
 	"strconv"
 
 	"github.com/restic/restic/internal/errors"
+	"github.com/restic/restic/internal/feature"
 	"github.com/restic/restic/internal/restic"
 	"github.com/restic/restic/internal/ui/termstatus"
 	"github.com/spf13/cobra"
@@ -91,6 +93,8 @@ type ForgetOptions struct {
 	WithinYearly  restic.Duration
 	KeepTags      restic.TagLists
 
+	UnsafeAllowRemoveAll bool
+
 	restic.SnapshotFilter
 	Compact bool
 
@@ -120,6 +124,7 @@ func init() {
 	f.VarP(&forgetOptions.WithinMonthly, "keep-within-monthly", "", "keep monthly snapshots that are newer than `duration` (eg. 1y5m7d2h) relative to the latest snapshot")
 	f.VarP(&forgetOptions.WithinYearly, "keep-within-yearly", "", "keep yearly snapshots that are newer than `duration` (eg. 1y5m7d2h) relative to the latest snapshot")
 	f.Var(&forgetOptions.KeepTags, "keep-tag", "keep snapshots with this `taglist` (can be specified multiple times)")
+	f.BoolVar(&forgetOptions.UnsafeAllowRemoveAll, "unsafe-allow-remove-all", false, "allow deleting all snapshots of a snapshot group")
 
 	initMultiSnapshotFilter(f, &forgetOptions.SnapshotFilter, false)
 	f.StringArrayVar(&forgetOptions.Hosts, "hostname", nil, "only consider snapshots with the given `hostname` (can be specified multiple times)")
@@ -221,54 +226,62 @@ func runForget(ctx context.Context, opts ForgetOptions, pruneOptions PruneOption
 			Tags:          opts.KeepTags,
 		}
 
-		if policy.Empty() && len(args) == 0 {
-			printer.P("no policy was specified, no snapshots will be removed\n")
+		if policy.Empty() {
+			if opts.UnsafeAllowRemoveAll {
+				if opts.SnapshotFilter.Empty() {
+					return errors.Fatal("--unsafe-allow-remove-all is not allowed unless a snapshot filter option is specified")
+				}
+				// UnsafeAllowRemoveAll together with snapshot filter is fine
+			} else {
+				return errors.Fatal("no policy was specified, no snapshots will be removed")
+			}
 		}
 
-		if !policy.Empty() {
-			printer.P("Applying Policy: %v\n", policy)
-
-			for k, snapshotGroup := range snapshotGroups {
-				if gopts.Verbose >= 1 && !gopts.JSON {
-					err = PrintSnapshotGroupHeader(globalOptions.stdout, k)
-					if err != nil {
-						return err
-					}
-				}
+		printer.P("Applying Policy: %v\n", policy)
 
-				var key restic.SnapshotGroupKey
-				if json.Unmarshal([]byte(k), &key) != nil {
+		for k, snapshotGroup := range snapshotGroups {
+			if gopts.Verbose >= 1 && !gopts.JSON {
+				err = PrintSnapshotGroupHeader(globalOptions.stdout, k)
+				if err != nil {
 					return err
 				}
+			}
+
+			var key restic.SnapshotGroupKey
+			if json.Unmarshal([]byte(k), &key) != nil {
+				return err
+			}
 
-				var fg ForgetGroup
-				fg.Tags = key.Tags
-				fg.Host = key.Hostname
-				fg.Paths = key.Paths
+			var fg ForgetGroup
+			fg.Tags = key.Tags
+			fg.Host = key.Hostname
+			fg.Paths = key.Paths
 
-				keep, remove, reasons := restic.ApplyPolicy(snapshotGroup, policy)
+			keep, remove, reasons := restic.ApplyPolicy(snapshotGroup, policy)
 
-				if len(keep) != 0 && !gopts.Quiet && !gopts.JSON {
-					printer.P("keep %d snapshots:\n", len(keep))
-					PrintSnapshots(globalOptions.stdout, keep, reasons, opts.Compact)
-					printer.P("\n")
-				}
-				fg.Keep = asJSONSnapshots(keep)
+			if feature.Flag.Enabled(feature.SafeForgetKeepTags) && !policy.Empty() && len(keep) == 0 {
+				return fmt.Errorf("refusing to delete last snapshot of snapshot group \"%v\"", key.String())
+			}
+			if len(keep) != 0 && !gopts.Quiet && !gopts.JSON {
+				printer.P("keep %d snapshots:\n", len(keep))
+				PrintSnapshots(globalOptions.stdout, keep, reasons, opts.Compact)
+				printer.P("\n")
+			}
+			fg.Keep = asJSONSnapshots(keep)
 
-				if len(remove) != 0 && !gopts.Quiet && !gopts.JSON {
-					printer.P("remove %d snapshots:\n", len(remove))
-					PrintSnapshots(globalOptions.stdout, remove, nil, opts.Compact)
-					printer.P("\n")
-				}
-				fg.Remove = asJSONSnapshots(remove)
+			if len(remove) != 0 && !gopts.Quiet && !gopts.JSON {
+				printer.P("remove %d snapshots:\n", len(remove))
+				PrintSnapshots(globalOptions.stdout, remove, nil, opts.Compact)
+				printer.P("\n")
+			}
+			fg.Remove = asJSONSnapshots(remove)
 
-				fg.Reasons = asJSONKeeps(reasons)
+			fg.Reasons = asJSONKeeps(reasons)
 
-				jsonGroups = append(jsonGroups, &fg)
+			jsonGroups = append(jsonGroups, &fg)
 
-				for _, sn := range remove {
-					removeSnIDs.Insert(*sn.ID())
-				}
+			for _, sn := range remove {
+				removeSnIDs.Insert(*sn.ID())
 			}
 		}
 	}

cmd/restic/cmd_forget_integration_test.go

@@ -2,18 +2,65 @@ package main
 
 import (
 	"context"
+	"path/filepath"
+	"strings"
 	"testing"
 
+	"github.com/restic/restic/internal/restic"
 	rtest "github.com/restic/restic/internal/test"
 	"github.com/restic/restic/internal/ui/termstatus"
 )
 
-func testRunForget(t testing.TB, gopts GlobalOptions, args ...string) {
-	opts := ForgetOptions{}
+func testRunForgetMayFail(gopts GlobalOptions, opts ForgetOptions, args ...string) error {
 	pruneOpts := PruneOptions{
 		MaxUnused: "5%",
 	}
-	rtest.OK(t, withTermStatus(gopts, func(ctx context.Context, term *termstatus.Terminal) error {
+	return withTermStatus(gopts, func(ctx context.Context, term *termstatus.Terminal) error {
 		return runForget(context.TODO(), opts, pruneOpts, gopts, term, args)
-	}))
+	})
+}
+
+func testRunForget(t testing.TB, gopts GlobalOptions, opts ForgetOptions, args ...string) {
+	rtest.OK(t, testRunForgetMayFail(gopts, opts, args...))
+}
+
+func TestRunForgetSafetyNet(t *testing.T) {
+	env, cleanup := withTestEnvironment(t)
+	defer cleanup()
+
+	testSetupBackupData(t, env)
+
+	opts := BackupOptions{
+		Host: "example",
+	}
+	testRunBackup(t, "", []string{filepath.Join(env.testdata, "0", "0", "9")}, opts, env.gopts)
+	testRunBackup(t, "", []string{filepath.Join(env.testdata, "0", "0", "9")}, opts, env.gopts)
+	testListSnapshots(t, env.gopts, 2)
+
+	// --keep-tags invalid
+	err := testRunForgetMayFail(env.gopts, ForgetOptions{
+		KeepTags: restic.TagLists{restic.TagList{"invalid"}},
+		GroupBy:  restic.SnapshotGroupByOptions{Host: true, Path: true},
+	})
+	rtest.Assert(t, strings.Contains(err.Error(), `refusing to delete last snapshot of snapshot group "host example, path`), "wrong error message got %v", err)
+
+	// disallow `forget --unsafe-allow-remove-all`
+	err = testRunForgetMayFail(env.gopts, ForgetOptions{
+		UnsafeAllowRemoveAll: true,
+	})
+	rtest.Assert(t, strings.Contains(err.Error(), `--unsafe-allow-remove-all is not allowed unless a snapshot filter option is specified`), "wrong error message got %v", err)
+
+	// disallow `forget` without options
+	err = testRunForgetMayFail(env.gopts, ForgetOptions{})
+	rtest.Assert(t, strings.Contains(err.Error(), `no policy was specified, no snapshots will be removed`), "wrong error message got %v", err)
+
+	// `forget --host example --unsafe-allow-remmove-all` should work
+	testRunForget(t, env.gopts, ForgetOptions{
+		UnsafeAllowRemoveAll: true,
+		GroupBy:              restic.SnapshotGroupByOptions{Host: true, Path: true},
+		SnapshotFilter: restic.SnapshotFilter{
+			Hosts: []string{opts.Host},
+		},
+	})
+	testListSnapshots(t, env.gopts, 0)
 }

cmd/restic/cmd_prune_integration_test.go

@@ -75,7 +75,7 @@ func createPrunableRepo(t *testing.T, env *testEnvironment) {
 	testListSnapshots(t, env.gopts, 3)
 
 	testRunForgetJSON(t, env.gopts)
-	testRunForget(t, env.gopts, firstSnapshot.String())
+	testRunForget(t, env.gopts, ForgetOptions{}, firstSnapshot.String())
 }
 
 func testRunForgetJSON(t testing.TB, gopts GlobalOptions, args ...string) {
@@ -129,7 +129,7 @@ func TestPruneWithDamagedRepository(t *testing.T) {
 	// create and delete snapshot to create unused blobs
 	testRunBackup(t, "", []string{filepath.Join(env.testdata, "0", "0", "9", "2")}, opts, env.gopts)
 	firstSnapshot := testListSnapshots(t, env.gopts, 1)[0]
-	testRunForget(t, env.gopts, firstSnapshot.String())
+	testRunForget(t, env.gopts, ForgetOptions{}, firstSnapshot.String())
 
 	oldPacks := listPacks(env.gopts, t)
 

cmd/restic/cmd_repair_snapshots_integration_test.go

@@ -62,7 +62,7 @@ func TestRepairSnapshotsWithLostData(t *testing.T) {
 	testRunCheckMustFail(t, env.gopts)
 
 	// repository must be ok after removing the broken snapshots
-	testRunForget(t, env.gopts, snapshotIDs[0].String(), snapshotIDs[1].String())
+	testRunForget(t, env.gopts, ForgetOptions{}, snapshotIDs[0].String(), snapshotIDs[1].String())
 	testListSnapshots(t, env.gopts, 2)
 	_, err := testRunCheckOutput(env.gopts, false)
 	rtest.OK(t, err)
@@ -86,7 +86,7 @@ func TestRepairSnapshotsWithLostTree(t *testing.T) {
 
 	// remove tree for foo/bar and the now completely broken first snapshot
 	removePacks(env.gopts, t, restic.NewIDSet(oldPacks...))
-	testRunForget(t, env.gopts, oldSnapshot[0].String())
+	testRunForget(t, env.gopts, ForgetOptions{}, oldSnapshot[0].String())
 	testRunCheckMustFail(t, env.gopts)
 
 	// repair

doc/060_forget.rst

@@ -182,7 +182,9 @@ The ``forget`` command accepts the following policy options:
 -  ``--keep-yearly n`` for the last ``n`` years which have one or more
    snapshots, keep only the most recent one for each year.
 -  ``--keep-tag`` keep all snapshots which have all tags specified by
-   this option (can be specified multiple times).
+   this option (can be specified multiple times). The ``forget`` command will
+   exit with an error if all snapshots in a snapshot group would be removed
+   as none of them have the specified tags.
 -  ``--keep-within duration`` keep all snapshots having a timestamp within
    the specified duration of the latest snapshot, where ``duration`` is a
    number of years, months, days, and hours. E.g. ``2y5m7d3h`` will keep all
@@ -336,12 +338,23 @@ year and yearly for the last 75 years, you can instead specify ``forget
 --keep-within-yearly 75y`` (note that `1w` is not a recognized duration, so
 you will have to specify `7d` instead).
 
+
+Removing all snapshots
+======================
+
 For safety reasons, restic refuses to act on an "empty" policy. For example,
 if one were to specify ``--keep-last 0`` to forget *all* snapshots in the
 repository, restic will respond that no snapshots will be removed. To delete
 all snapshots, use ``--keep-last 1`` and then finally remove the last snapshot
 manually (by passing the ID to ``forget``).
 
+Since restic 0.17.0, it is possible to delete all snapshots for a specific
+host, tag or path using the ``--unsafe-allow-remove-all`` option. The option
+must always be combined with a snapshot filter (by host, path or tag).
+For example the command ``forget --tag example --unsafe-allow-remove-all``
+removes all snapshots with tag ``example``.
+
+
 Security considerations in append-only mode
 ===========================================
 

internal/feature/registry.go

@@ -9,6 +9,7 @@ const (
 	DeprecateLegacyIndex    FlagName = "deprecate-legacy-index"
 	DeprecateS3LegacyLayout FlagName = "deprecate-s3-legacy-layout"
 	DeviceIDForHardlinks    FlagName = "device-id-for-hardlinks"
+	SafeForgetKeepTags      FlagName = "safe-forget-keep-tags"
 )
 
 func init() {
@@ -17,5 +18,6 @@ func init() {
 		DeprecateLegacyIndex:    {Type: Beta, Description: "disable support for index format used by restic 0.1.0. Use `restic repair index` to update the index if necessary."},
 		DeprecateS3LegacyLayout: {Type: Beta, Description: "disable support for S3 legacy layout used up to restic 0.7.0. Use `RESTIC_FEATURES=deprecate-s3-legacy-layout=false restic migrate s3_layout` to migrate your S3 repository if necessary."},
 		DeviceIDForHardlinks:    {Type: Alpha, Description: "store deviceID only for hardlinks to reduce metadata changes for example when using btrfs subvolumes. Will be removed in a future restic version after repository format 3 is available"},
+		SafeForgetKeepTags:      {Type: Beta, Description: "prevent deleting all snapshots if the tag passed to `forget --keep-tags tagname` does not exist"},
 	})
 }

internal/restic/snapshot_find.go

@@ -24,7 +24,7 @@ type SnapshotFilter struct {
 	TimestampLimit time.Time
 }
 
-func (f *SnapshotFilter) empty() bool {
+func (f *SnapshotFilter) Empty() bool {
 	return len(f.Hosts)+len(f.Tags)+len(f.Paths) == 0
 }
 
@@ -173,7 +173,7 @@ func (f *SnapshotFilter) FindAll(ctx context.Context, be Lister, loader LoaderUn
 		}
 
 		// Give the user some indication their filters are not used.
-		if !usedFilter && !f.empty() {
+		if !usedFilter && !f.Empty() {
 			return fn("filters", nil, errors.Errorf("explicit snapshot ids are given"))
 		}
 		return nil

internal/restic/snapshot_group.go

@@ -66,6 +66,20 @@ type SnapshotGroupKey struct {
 	Tags     []string `json:"tags"`
 }
 
+func (s *SnapshotGroupKey) String() string {
+	var parts []string
+	if s.Hostname != "" {
+		parts = append(parts, fmt.Sprintf("host %v", s.Hostname))
+	}
+	if len(s.Paths) != 0 {
+		parts = append(parts, fmt.Sprintf("path %v", s.Paths))
+	}
+	if len(s.Tags) != 0 {
+		parts = append(parts, fmt.Sprintf("tags %v", s.Tags))
+	}
+	return strings.Join(parts, ", ")
+}
+
 // GroupSnapshots takes a list of snapshots and a grouping criteria and creates
 // a grouped list of snapshots.
 func GroupSnapshots(snapshots Snapshots, groupBy SnapshotGroupByOptions) (map[string]Snapshots, bool, error) {

internal/restic/snapshot_policy.go

@@ -94,7 +94,11 @@ func (e ExpirePolicy) String() (s string) {
 		s += fmt.Sprintf("all snapshots within %s of the newest", e.Within)
 	}
 
-	s = "keep " + s
+	if s == "" {
+		s = "remove"
+	} else {
+		s = "keep " + s
+	}
 
 	return s
 }
@@ -186,16 +190,6 @@ func ApplyPolicy(list Snapshots, p ExpirePolicy) (keep, remove Snapshots, reason
 	// sort newest snapshots first
 	sort.Stable(list)
 
-	if p.Empty() {
-		for _, sn := range list {
-			reasons = append(reasons, KeepReason{
-				Snapshot: sn,
-				Matches:  []string{"policy is empty"},
-			})
-		}
-		return list, remove, reasons
-	}
-
 	if len(list) == 0 {
 		return list, nil, nil
 	}