Repository: borgbackup/borg

Issue:
Specifying the same path twice breaks hardlink during extraction
If you accidentally specify a path to be backed up twice and this path contain hardlinks, you cannot succesfully extract these files

Eg:
```
mkdir /tmp/borgtest /tmp/borgrepo /tmp/borgtest/a /tmp/borgtest/b /tmp/extractlocation
borg init -e none /tmp/borgrepo
echo "Hi Everybody" > /tmp/borgtest/a/hardlink
ln /tmp/borgtest/a/hardlink /tmp/borgtest/b/hardlink

borg create /tmp/borgrepo::testarchive /tmp/borgtest/ /tmp/borgtest/
borg list /tmp/borgrepo::testarchive

cd /tmp/extractlocation
borg extract /tmp/borgrepo::testarchive
```

returns

```
tmp/borgtest/a/hardlink: link: [Errno 2] No such file or directory: '/private/tmp/extractlocation/tmp/borgtest/a/hardlink' -> '/private/tmp/extractlocation/tmp/borgtest/a/hardlink'
tmp/borgtest/b/hardlink: link: [Errno 2] No such file or directory: '/private/tmp/extractlocation/tmp/borgtest/a/hardlink' -> '/private/tmp/extractlocation/tmp/borgtest/b/hardlink'
```

and both the hardlink and the file it was pointing to will be gone

When I list the archive I can see the file and the hardlink are both there

```
borg list /tmp/borgrepo::testarchive

drwxr-xr-x arnold wheel         0 Wed, 2020-12-30 10:27:21 tmp/borgtest
drwxr-xr-x arnold wheel         0 Wed, 2020-12-30 10:27:24 tmp/borgtest/a
-rw-r--r-- arnold wheel        13 Wed, 2020-12-30 10:27:45 tmp/borgtest/a/hardlink
drwxr-xr-x arnold wheel         0 Wed, 2020-12-30 10:27:28 tmp/borgtest/b
hrw-r--r-- arnold wheel         0 Wed, 2020-12-30 10:27:45 tmp/borgtest/b/hardlink link to tmp/borgtest/a/hardlink
drwxr-xr-x arnold wheel         0 Wed, 2020-12-30 10:27:21 tmp/borgtest
drwxr-xr-x arnold wheel         0 Wed, 2020-12-30 10:27:24 tmp/borgtest/a
hrw-r--r-- arnold wheel         0 Wed, 2020-12-30 10:27:45 tmp/borgtest/a/hardlink link to tmp/borgtest/a/hardlink
drwxr-xr-x arnold wheel         0 Wed, 2020-12-30 10:27:28 tmp/borgtest/b
hrw-r--r-- arnold wheel         0 Wed, 2020-12-30 10:27:45 tmp/borgtest/b/hardlink link to tmp/borgtest/a/hardlink
```

So I assume I could probably still rescue the file somehow if i wanted, and that the file is actually written to disk during restore and then removed.

Now of course, one shouldn't do this in the first place - but shouldn't borg have realised it's creating a hardlink to itself (.. a/hardlink to ../a/hardlink) and ignore/bail out on that point ? Or ignore/fail duplicate pathnames on the command line?


Discussion:

Patch:
diff --git a/src/borg/archiver.py b/src/borg/archiver.py
index 651dcf3b..5227852f 100644
--- a/src/borg/archiver.py
+++ b/src/borg/archiver.py
@@ -603,6 +603,9 @@ def create_inner(archive, cache, fso):
                                        exclude_caches=args.exclude_caches, exclude_if_present=args.exclude_if_present,
                                        keep_exclude_tags=args.keep_exclude_tags, skip_inodes=skip_inodes,
                                        restrict_dev=restrict_dev, read_special=args.read_special, dry_run=dry_run)
+                        # if we get back here, we've finished recursing into <path>,
+                        # we do not ever want to get back in there (even if path is given twice as recursion root)
+                        skip_inodes.add((st.st_ino, st.st_dev))
             if not dry_run:
                 if args.progress:
                     archive.stats.show_progress(final=True)


Test patch:
diff --git a/src/borg/testsuite/archiver.py b/src/borg/testsuite/archiver.py
index fbe5931a..5f64b39f 100644
--- a/src/borg/testsuite/archiver.py
+++ b/src/borg/testsuite/archiver.py
@@ -381,6 +381,8 @@ def create_test_files(self):
 
 
 class ArchiverTestCase(ArchiverTestCaseBase):
+    requires_hardlinks = pytest.mark.skipif(not are_hardlinks_supported(), reason='hardlinks not supported')
+
     def test_basic_functionality(self):
         have_root = self.create_test_files()
         # fork required to test show-rc output
@@ -444,6 +446,25 @@ def filter(output):
         # the interesting parts of info_output2 and info_output should be same
         self.assert_equal(filter(info_output), filter(info_output2))
 
+    @requires_hardlinks
+    def test_create_duplicate_root(self):
+        # setup for #5603
+        path_a = os.path.join(self.input_path, 'a')
+        path_b = os.path.join(self.input_path, 'b')
+        os.mkdir(path_a)
+        os.mkdir(path_b)
+        hl_a = os.path.join(path_a, 'hardlink')
+        hl_b = os.path.join(path_b, 'hardlink')
+        self.create_regular_file(hl_a, contents=b'123456')
+        os.link(hl_a, hl_b)
+        self.cmd('init', '--encryption=none', self.repository_location)
+        self.cmd('create', self.repository_location + '::test', 'input', 'input')  # give input twice!
+        # test if created archive has 'input' contents twice:
+        archive_list = self.cmd('list', '--json-lines', self.repository_location + '::test')
+        paths = [json.loads(line)['path'] for line in archive_list.split('\n') if line]
+        # we have all fs items exactly once!
+        assert paths == ['input', 'input/a', 'input/a/hardlink', 'input/b', 'input/b/hardlink']
+
     def test_init_parent_dirs(self):
         parent_path = os.path.join(self.tmpdir, 'parent1', 'parent2')
         repository_path = os.path.join(parent_path, 'repository')
@@ -792,8 +813,6 @@ def _extract_hardlinks_setup(self):
         self.cmd('init', '--encryption=repokey', self.repository_location)
         self.cmd('create', self.repository_location + '::test', 'input')
 
-    requires_hardlinks = pytest.mark.skipif(not are_hardlinks_supported(), reason='hardlinks not supported')
-
     @requires_hardlinks
     @unittest.skipUnless(llfuse, 'llfuse not installed')
     def test_fuse_mount_hardlinks(self):
@@ -857,6 +876,29 @@ def test_extract_hardlinks2(self):
             assert os.stat('input/dir1/aaaa').st_nlink == 2
             assert os.stat('input/dir1/source2').st_nlink == 2
 
+    @requires_hardlinks
+    def test_extract_hardlinks_twice(self):
+        # setup for #5603
+        path_a = os.path.join(self.input_path, 'a')
+        path_b = os.path.join(self.input_path, 'b')
+        os.mkdir(path_a)
+        os.mkdir(path_b)
+        hl_a = os.path.join(path_a, 'hardlink')
+        hl_b = os.path.join(path_b, 'hardlink')
+        self.create_regular_file(hl_a, contents=b'123456')
+        os.link(hl_a, hl_b)
+        self.cmd('init', '--encryption=none', self.repository_location)
+        self.cmd('create', self.repository_location + '::test', 'input', 'input')  # give input twice!
+        # now test extraction
+        with changedir('output'):
+            self.cmd('extract', self.repository_location + '::test')
+            # if issue #5603 happens, extraction gives rc == 1 (triggering AssertionError) and warnings like:
+            # input/a/hardlink: link: [Errno 2] No such file or directory: 'input/a/hardlink' -> 'input/a/hardlink'
+            # input/b/hardlink: link: [Errno 2] No such file or directory: 'input/a/hardlink' -> 'input/b/hardlink'
+            # otherwise, when fixed, the hardlinks should be there and have a link count of 2
+            assert os.stat('input/a/hardlink').st_nlink == 2
+            assert os.stat('input/b/hardlink').st_nlink == 2
+
     def test_extract_include_exclude(self):
         self.cmd('init', '--encryption=repokey', self.repository_location)
         self.create_regular_file('file1', size=1024 * 80)