Merge pull request #3581 from ThomasWaldmann/borg-config-validation

borg config: add some validation, fixes #3566

src/borg/archiver.py

@@ -1523,6 +1523,46 @@ class Archiver:
     @with_repository(exclusive=True, cache=True, compatibility=(Manifest.Operation.WRITE,))
     def do_config(self, args, repository, manifest, key, cache):
         """get, set, and delete values in a repository or cache config file"""
+
+        def repo_validate(section, name, value=None, check_value=True):
+            if section not in ['repository', ]:
+                raise ValueError('Invalid section')
+            if name in ['segments_per_dir', 'max_segment_size', 'storage_quota', ]:
+                if check_value:
+                    try:
+                        int(value)
+                    except ValueError:
+                        raise ValueError('Invalid value') from None
+                    if name == 'max_segment_size':
+                        if int(value) >= MAX_SEGMENT_SIZE_LIMIT:
+                            raise ValueError('Invalid value: max_segment_size >= %d' % MAX_SEGMENT_SIZE_LIMIT)
+            elif name in ['additional_free_space', ]:
+                if check_value:
+                    try:
+                        parse_file_size(value)
+                    except ValueError:
+                        raise ValueError('Invalid value') from None
+            elif name in ['append_only', ]:
+                if check_value and value not in ['0', '1']:
+                    raise ValueError('Invalid value')
+            elif name in ['id', ]:
+                if check_value:
+                    try:
+                        bin_id = unhexlify(value)
+                    except:
+                        raise ValueError('Invalid value, must be 64 hex digits') from None
+                    if len(bin_id) != 32:
+                        raise ValueError('Invalid value, must be 64 hex digits')
+            else:
+                raise ValueError('Invalid name')
+
+        def cache_validate(section, name, value=None, check_value=True):
+            if section not in ['cache', ]:
+                raise ValueError('Invalid section')
+            # I looked at the cache config and did not see anything a user would want to edit,
+            # so, for now, raise for any key name
+            raise ValueError('Invalid name')
+
         try:
             section, name = args.name.split('.')
         except ValueError:
@@ -1533,16 +1573,20 @@ class Archiver:
             cache.cache_config.load()
             config = cache.cache_config._config
             save = cache.cache_config.save
+            validate = cache_validate
         else:
             config = repository.config
             save = lambda: repository.save_config(repository.path, repository.config)
+            validate = repo_validate
 
         if args.delete:
+            validate(section, name, check_value=False)
             config.remove_option(section, name)
             if len(config.options(section)) == 0:
                 config.remove_section(section)
             save()
         elif args.value:
+            validate(section, name, args.value)
             if section not in config.sections():
                 config.add_section(section)
             config.set(section, name, args.value)

src/borg/constants.py

@@ -36,6 +36,9 @@ MAX_DATA_SIZE = 20971479
 MAX_OBJECT_SIZE = MAX_DATA_SIZE + 41  # see LoggedIO.put_header_fmt.size assertion in repository module
 assert MAX_OBJECT_SIZE == 20 * 1024 * 1024
 
+# repo config max_segment_size value must be below this limit to stay within uint32 offsets:
+MAX_SEGMENT_SIZE_LIMIT = 2 ** 32 - MAX_OBJECT_SIZE
+
 # borg.remote read() buffer size
 BUFSIZE = 10 * 1024 * 1024
 

src/borg/repository.py

@@ -122,6 +122,9 @@ class Repository:
     class InvalidRepository(Error):
         """{} is not a valid repository. Check repo config."""
 
+    class InvalidRepositoryConfig(Error):
+        """{} does not have a valid configuration. Check repo config [{}]."""
+
     class AtticRepository(Error):
         """Attic repository detected. Please run "borg upgrade {}"."""
 
@@ -384,6 +387,9 @@ class Repository:
             self.close()
             raise self.InvalidRepository(path)
         self.max_segment_size = self.config.getint('repository', 'max_segment_size')
+        if self.max_segment_size >= MAX_SEGMENT_SIZE_LIMIT:
+            self.close()
+            raise self.InvalidRepositoryConfig(path, 'max_segment_size >= %d' % MAX_SEGMENT_SIZE_LIMIT)  # issue 3592
         self.segments_per_dir = self.config.getint('repository', 'segments_per_dir')
         self.additional_free_space = parse_file_size(self.config.get('repository', 'additional_free_space', fallback=0))
         # append_only can be set in the constructor

src/borg/testsuite/archiver.py

@@ -2782,14 +2782,17 @@ id: 2 / e29442 3506da 4e1ea7 / 25f62a 5a3d41 - 02
         self.create_test_files()
         os.unlink('input/flagfile')
         self.cmd('init', '--encryption=repokey', self.repository_location)
-        for flags in [[], ['--cache']]:
-            for cfg_key in {'testkey', 'testsection.testkey'}:
-                self.cmd('config', self.repository_location, *flags, cfg_key, exit_code=1)
-                self.cmd('config', self.repository_location, *flags, cfg_key, 'testcontents')
-                output = self.cmd('config', self.repository_location, *flags, cfg_key)
-                assert output == 'testcontents\n'
-                self.cmd('config', self.repository_location, *flags, '--delete', cfg_key)
-                self.cmd('config', self.repository_location, *flags, cfg_key, exit_code=1)
+        for cfg_key, cfg_value in [
+            ('additional_free_space', '2G'),
+            ('repository.append_only', '1'),
+        ]:
+            output = self.cmd('config', self.repository_location, cfg_key)
+            assert output == '0' + '\n'
+            self.cmd('config', self.repository_location, cfg_key, cfg_value)
+            output = self.cmd('config', self.repository_location, cfg_key)
+            assert output == cfg_value + '\n'
+            self.cmd('config', self.repository_location, '--delete', cfg_key)
+            self.cmd('config', self.repository_location, cfg_key, exit_code=1)
 
     requires_gnutar = pytest.mark.skipif(not have_gnutar(), reason='GNU tar must be installed for this test.')
     requires_gzip = pytest.mark.skipif(not shutil.which('gzip'), reason='gzip must be installed for this test.')