Repository: borgbackup/borg

Issue:
borg init repo -> unencrypted repo
Only applies to master branch (borg 1.1beta).

When blake2b was introduced, we removed the default of aes256-hmac-sha256.

Then intention was that the user does an informed decision between hmac-sha256 and blake2b MACs.

But: if a user does not specify --encryption, we now fall back to no encryption at all (and this was not intended).

Discussion:
codecov-io: # [Codecov](https://codecov.io/gh/borgbackup/borg/pull/2166?src=pr&el=h1) Report
> Merging [#2166](https://codecov.io/gh/borgbackup/borg/pull/2166?src=pr&el=desc) into [master](https://codecov.io/gh/borgbackup/borg/commit/833d0ab76cbf5af2a253d2c245722e16bdcf9887?src=pr&el=desc) will **decrease** coverage by `-1%`.
> The diff coverage is `33.33%`.


```diff
@@           Coverage Diff           @@
##           master    #2166   +/-   ##
=======================================
- Coverage   83.55%   82.56%   -1%     
=======================================
  Files          20       20           
  Lines        7177     7179    +2     
  Branches     1227     1228    +1     
=======================================
- Hits         5997     5927   -70     
- Misses        849      919   +70     
- Partials      331      333    +2
```


| [Impacted Files](https://codecov.io/gh/borgbackup/borg/pull/2166?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [src/borg/key.py](https://codecov.io/gh/borgbackup/borg/compare/833d0ab76cbf5af2a253d2c245722e16bdcf9887...cb64746ad34ded088caf42d90fadfc00d278f9cd?src=pr&el=tree#diff-c3JjL2Jvcmcva2V5LnB5) | `87.22% <ø> (-0.36%)` | :x: |
| [src/borg/archiver.py](https://codecov.io/gh/borgbackup/borg/compare/833d0ab76cbf5af2a253d2c245722e16bdcf9887...cb64746ad34ded088caf42d90fadfc00d278f9cd?src=pr&el=tree#diff-c3JjL2JvcmcvYXJjaGl2ZXIucHk=) | `84.95% <100%> (+0.14%)` | :white_check_mark: |
| [src/borg/xattr.py](https://codecov.io/gh/borgbackup/borg/compare/833d0ab76cbf5af2a253d2c245722e16bdcf9887...cb64746ad34ded088caf42d90fadfc00d278f9cd?src=pr&el=tree#diff-c3JjL2JvcmcveGF0dHIucHk=) | `56% <ø> (-23.5%)` | :x: |
| [src/borg/platform/__init__.py](https://codecov.io/gh/borgbackup/borg/compare/833d0ab76cbf5af2a253d2c245722e16bdcf9887...cb64746ad34ded088caf42d90fadfc00d278f9cd?src=pr&el=tree#diff-c3JjL2JvcmcvcGxhdGZvcm0vX19pbml0X18ucHk=) | `73.68% <ø> (-15.79%)` | :x: |
| [src/borg/platform/base.py](https://codecov.io/gh/borgbackup/borg/compare/833d0ab76cbf5af2a253d2c245722e16bdcf9887...cb64746ad34ded088caf42d90fadfc00d278f9cd?src=pr&el=tree#diff-c3JjL2JvcmcvcGxhdGZvcm0vYmFzZS5weQ==) | `72.61% <ø> (-10.72%)` | :x: |
| [src/borg/helpers.py](https://codecov.io/gh/borgbackup/borg/compare/833d0ab76cbf5af2a253d2c245722e16bdcf9887...cb64746ad34ded088caf42d90fadfc00d278f9cd?src=pr&el=tree#diff-c3JjL2JvcmcvaGVscGVycy5weQ==) | `85.89% <ø> (-1.52%)` | :x: |
| [src/borg/remote.py](https://codecov.io/gh/borgbackup/borg/compare/833d0ab76cbf5af2a253d2c245722e16bdcf9887...cb64746ad34ded088caf42d90fadfc00d278f9cd?src=pr&el=tree#diff-c3JjL2JvcmcvcmVtb3RlLnB5) | `77.3% <ø> (-0.23%)` | :x: |
| [src/borg/archive.py](https://codecov.io/gh/borgbackup/borg/compare/833d0ab76cbf5af2a253d2c245722e16bdcf9887...cb64746ad34ded088caf42d90fadfc00d278f9cd?src=pr&el=tree#diff-c3JjL2JvcmcvYXJjaGl2ZS5weQ==) | `82.52% <ø> (+0.5%)` | :white_check_mark: |

------

[Continue to review full report at Codecov](https://codecov.io/gh/borgbackup/borg/pull/2166?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/borgbackup/borg/pull/2166?src=pr&el=footer). Last update [833d0ab...af1cd85](https://codecov.io/gh/borgbackup/borg/compare/833d0ab76cbf5af2a253d2c245722e16bdcf9887...af1cd8500586c496a98bf714881c71cbc3bb2144?el=footer&src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).

Patch:
diff --git a/src/borg/archiver.py b/src/borg/archiver.py
index 5de24d26..9dbbd202 100644
--- a/src/borg/archiver.py
+++ b/src/borg/archiver.py
@@ -1850,9 +1850,8 @@ def process_epilog(epilog):
         subparser.add_argument('location', metavar='REPOSITORY', nargs='?', default='',
                                type=location_validator(archive=False),
                                help='repository to create')
-        subparser.add_argument('-e', '--encryption', dest='encryption',
+        subparser.add_argument('-e', '--encryption', dest='encryption', required=True,
                                choices=('none', 'keyfile', 'repokey', 'keyfile-blake2', 'repokey-blake2', 'authenticated'),
-                               default=None,
                                help='select encryption key mode')
         subparser.add_argument('-a', '--append-only', dest='append_only', action='store_true',
                                help='create an append-only mode repository')
diff --git a/src/borg/key.py b/src/borg/key.py
index e5d736ea..4358c644 100644
--- a/src/borg/key.py
+++ b/src/borg/key.py
@@ -98,8 +98,10 @@ def key_creator(repository, args):
         return Blake2RepoKey.create(repository, args)
     elif args.encryption == 'authenticated':
         return AuthenticatedKey.create(repository, args)
-    else:
+    elif args.encryption == 'none':
         return PlaintextKey.create(repository, args)
+    else:
+        raise ValueError('Invalid encryption mode "%s"' % args.encryption)
 
 
 def key_factory(repository, manifest_data):


Test patch:
diff --git a/src/borg/testsuite/archiver.py b/src/borg/testsuite/archiver.py
index 2b0709be..adf85b7d 100644
--- a/src/borg/testsuite/archiver.py
+++ b/src/borg/testsuite/archiver.py
@@ -79,7 +79,13 @@ def exec_cmd(*args, archiver=None, fork=False, exe=None, **kw):
                 archiver = Archiver()
             archiver.prerun_checks = lambda *args: None
             archiver.exit_code = EXIT_SUCCESS
-            args = archiver.parse_args(list(args))
+            try:
+                args = archiver.parse_args(list(args))
+                # argparse parsing may raise SystemExit when the command line is bad or
+                # actions that abort early (eg. --help) where given. Catch this and return
+                # the error code as-if we invoked a Borg binary.
+            except SystemExit as e:
+                return e.code, output.getvalue()
             ret = archiver.run(args)
             return ret, output.getvalue()
         finally:
@@ -879,16 +885,12 @@ def _create_test_caches(self):
 
     def test_create_without_root(self):
         """test create without a root"""
-        self.cmd('init', self.repository_location)
-        args = ['create', self.repository_location + '::test']
-        if self.FORK_DEFAULT:
-            self.cmd(*args, exit_code=2)
-        else:
-            self.assert_raises(SystemExit, lambda: self.cmd(*args))
+        self.cmd('init', '--encryption=repokey', self.repository_location)
+        self.cmd('create', self.repository_location + '::test', exit_code=2)
 
     def test_create_pattern_root(self):
         """test create with only a root pattern"""
-        self.cmd('init', self.repository_location)
+        self.cmd('init', '--encryption=repokey', self.repository_location)
         self.create_regular_file('file1', size=1024 * 80)
         self.create_regular_file('file2', size=1024 * 80)
         output = self.cmd('create', '-v', '--list', '--pattern=R input', self.repository_location + '::test')
@@ -897,7 +899,7 @@ def test_create_pattern_root(self):
 
     def test_create_pattern(self):
         """test file patterns during create"""
-        self.cmd('init', self.repository_location)
+        self.cmd('init', '--encryption=repokey', self.repository_location)
         self.create_regular_file('file1', size=1024 * 80)
         self.create_regular_file('file2', size=1024 * 80)
         self.create_regular_file('file_important', size=1024 * 80)
@@ -910,7 +912,7 @@ def test_create_pattern(self):
         self.assert_in('x input/file2', output)
 
     def test_extract_pattern_opt(self):
-        self.cmd('init', self.repository_location)
+        self.cmd('init', '--encryption=repokey', self.repository_location)
         self.create_regular_file('file1', size=1024 * 80)
         self.create_regular_file('file2', size=1024 * 80)
         self.create_regular_file('file_important', size=1024 * 80)
@@ -921,9 +923,6 @@ def test_extract_pattern_opt(self):
                      self.repository_location + '::test')
         self.assert_equal(sorted(os.listdir('output/input')), ['file_important'])
 
-    def test_exclude_caches(self):
-        self.cmd('init', self.repository_location)
-
     def _assert_test_caches(self):
         with changedir('output'):
             self.cmd('extract', self.repository_location + '::test')
@@ -1516,12 +1515,8 @@ def test_break_lock(self):
         self.cmd('break-lock', self.repository_location)
 
     def test_usage(self):
-        if self.FORK_DEFAULT:
-            self.cmd(exit_code=0)
-            self.cmd('-h', exit_code=0)
-        else:
-            self.assert_raises(SystemExit, lambda: self.cmd())
-            self.assert_raises(SystemExit, lambda: self.cmd('-h'))
+        self.cmd()
+        self.cmd('-h')
 
     def test_help(self):
         assert 'Borg' in self.cmd('help')
@@ -1777,6 +1772,9 @@ def raise_eof(*args):
             self.cmd('init', '--encryption=repokey', self.repository_location, exit_code=1)
         assert not os.path.exists(self.repository_location)
 
+    def test_init_requires_encryption_option(self):
+        self.cmd('init', self.repository_location, exit_code=2)
+
     def check_cache(self):
         # First run a regular borg check
         self.cmd('check', self.repository_location)
@@ -2476,10 +2474,10 @@ def test_remote_repo_restrict_to_path(self):
         path_prefix = os.path.dirname(self.repository_path)
         # restrict to repo directory's parent directory:
         with patch.object(RemoteRepository, 'extra_test_args', ['--restrict-to-path', path_prefix]):
-            self.cmd('init', self.repository_location + '_2')
+            self.cmd('init', '--encryption=repokey', self.repository_location + '_2')
         # restrict to repo directory's parent directory and another directory:
         with patch.object(RemoteRepository, 'extra_test_args', ['--restrict-to-path', '/foo', '--restrict-to-path', path_prefix]):
-            self.cmd('init', self.repository_location + '_3')
+            self.cmd('init', '--encryption=repokey', self.repository_location + '_3')
 
     @unittest.skip('only works locally')
     def test_debug_put_get_delete_obj(self):
@@ -2682,7 +2680,7 @@ def test_get_args():
     assert args.restrict_to_paths == ['/p1', '/p2']
     # trying to cheat - try to execute different subcommand
     args = archiver.get_args(['borg', 'serve', '--restrict-to-path=/p1', '--restrict-to-path=/p2', ],
-                             'borg init /')
+                             'borg init --encryption=repokey /')
     assert args.func == archiver.do_serve