Merge pull request #1536 from restic/fix-restorer-permissions

restore: Fix directory permissions

changelog/0.8.2/issue-1512

@@ -0,0 +1,9 @@
+Bugfix: Restore directory permissions as the last step
+
+This change allows restoring into directories that were not writable during
+backup. Before, restic created the directory, set the read-only mode and then
+failed to create files in the directory. This change now restores the directory
+(with its permissions) as the very last step.
+
+https://github.com/restic/restic/issues/1512
+https://github.com/restic/restic/pull/1536

internal/restic/restorer.go

@@ -79,13 +79,6 @@ func (res *Restorer) restoreTo(ctx context.Context, target, location string, tre
 		selectedForRestore, childMayBeSelected := res.SelectFilter(nodeLocation, nodeTarget, node)
 		debug.Log("SelectFilter returned %v %v", selectedForRestore, childMayBeSelected)
 
-		if selectedForRestore {
-			err = res.restoreNodeTo(ctx, node, nodeTarget, nodeLocation, idx)
-			if err != nil {
-				return err
-			}
-		}
-
 		if node.Type == "dir" && childMayBeSelected {
 			if node.Subtree == nil {
 				return errors.Errorf("Dir without subtree in tree %v", treeID.Str())
@@ -98,14 +91,19 @@ func (res *Restorer) restoreTo(ctx context.Context, target, location string, tre
 					return err
 				}
 			}
+		}
 
-			if selectedForRestore {
+		if selectedForRestore {
-				// Restore directory timestamp at the end. If we would do it earlier, restoring files within
+			err = res.restoreNodeTo(ctx, node, nodeTarget, nodeLocation, idx)
-				// the directory would overwrite the timestamp of the directory they are in.
+			if err != nil {
-				err = node.RestoreTimestamps(nodeTarget)
+				return err
-				if err != nil {
+			}
-					return err
+
-				}
+			// Restore directory timestamp at the end. If we would do it earlier, restoring files within
+			// the directory would overwrite the timestamp of the directory they are in.
+			err = node.RestoreTimestamps(nodeTarget)
+			if err != nil {
+				return err
 			}
 		}
 	}

internal/restic/restorer_test.go

@@ -29,6 +29,7 @@ type File struct {
 
 type Dir struct {
 	Nodes map[string]Node
+	Mode  os.FileMode
 }
 
 func saveFile(t testing.TB, repo restic.Repository, node File) restic.ID {
@@ -63,9 +64,15 @@ func saveDir(t testing.TB, repo restic.Repository, nodes map[string]Node) restic
 			})
 		case Dir:
 			id = saveDir(t, repo, node.Nodes)
+
+			mode := node.Mode
+			if mode == 0 {
+				mode = 0755
+			}
+
 			tree.Insert(&restic.Node{
 				Type:    "dir",
-				Mode:    0755,
+				Mode:    mode,
 				Name:    name,
 				UID:     uint32(os.Getuid()),
 				GID:     uint32(os.Getgid()),
@@ -166,6 +173,34 @@ func TestRestorer(t *testing.T) {
 				"dir/subdir/file": "file in subdir",
 			},
 		},
+		{
+			Snapshot: Snapshot{
+				Nodes: map[string]Node{
+					"dir": Dir{
+						Mode: 0444,
+					},
+					"file": File{"top-level file"},
+				},
+			},
+			Files: map[string]string{
+				"file": "top-level file",
+			},
+		},
+		{
+			Snapshot: Snapshot{
+				Nodes: map[string]Node{
+					"dir": Dir{
+						Mode: 0555,
+						Nodes: map[string]Node{
+							"file": File{"file in dir"},
+						},
+					},
+				},
+			},
+			Files: map[string]string{
+				"dir/file": "file in dir",
+			},
+		},
 
 		// test cases with invalid/constructed names
 		{