From f3954fb225fdda96d619f83e8f3321a840c4ef4a Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Marek=20Marczykowski-G=C3=B3recki?=
 <marmarek@invisiblethingslab.com>
Date: Fri, 29 Jan 2021 22:54:24 +0100
Subject: [PATCH] qvm-template: download templates to a temporary directory

Avoid risk of conflicting downloads to the same directory, reusing
partial downloads, leaving broken files etc. Move template package out
of temporary directory only after its verified.

QubesOS/qubes-issues#2534
---
 qubesadmin/tests/tools/qvm_template.py | 26 +++++++++++---------------
 qubesadmin/tools/qvm_template.py       | 20 +++++++++++---------
 2 files changed, 22 insertions(+), 24 deletions(-)

diff --git a/qubesadmin/tests/tools/qvm_template.py b/qubesadmin/tests/tools/qvm_template.py
index a150c1b..365199c 100644
--- a/qubesadmin/tests/tools/qvm_template.py
+++ b/qubesadmin/tests/tools/qvm_template.py
@@ -243,14 +243,12 @@ class TC_00_qvm_template(qubesadmin.tests.QubesTestCase):
             mock.call(args, self.app, version_selector=selector)
         ])
         # Nothing downloaded
-        self.assertEqual(mock_dl.mock_calls, [
-            mock.call(args, self.app, path_override='/var/cache/qvm-template',
-                dl_list={}, suffix='.unverified', version_selector=selector)
-        ])
+        mock_dl.assert_called_with(args, self.app,
+            path_override='/var/tmp/qvm-template-tmpdir',
+            dl_list={}, suffix='.unverified', version_selector=selector)
         # Package is extracted
-        self.assertEqual(mock_extract.mock_calls, [
-            mock.call('test-vm', path, '/var/tmp/qvm-template-tmpdir')
-        ])
+        mock_extract.assert_called_with('test-vm', path,
+            '/var/tmp/qvm-template-tmpdir')
         # No packages overwritten, so no confirm needed
         self.assertEqual(mock_confirm.mock_calls, [])
         # qvm-template-postprocess is called
@@ -363,14 +361,12 @@ class TC_00_qvm_template(qubesadmin.tests.QubesTestCase):
             mock.call(args, self.app, version_selector=selector)
         ])
         # Nothing downloaded
-        self.assertEqual(mock_dl.mock_calls, [
-            mock.call(args, self.app, path_override='/var/cache/qvm-template',
-                dl_list={}, suffix='.unverified', version_selector=selector)
-        ])
+        mock_dl.assert_called_with(args, self.app,
+            path_override='/var/tmp/qvm-template-tmpdir',
+            dl_list={}, suffix='.unverified', version_selector=selector)
         # Package is extracted
-        self.assertEqual(mock_extract.mock_calls, [
-            mock.call('test-vm', path, '/var/tmp/qvm-template-tmpdir')
-        ])
+        mock_extract.assert_called_with('test-vm', path,
+            '/var/tmp/qvm-template-tmpdir')
         # No packages overwritten, so no confirm needed
         self.assertEqual(mock_confirm.mock_calls, [])
         # qvm-template-postprocess is called
@@ -531,7 +527,7 @@ class TC_00_qvm_template(qubesadmin.tests.QubesTestCase):
         ])
         # Nothing downloaded
         self.assertEqual(mock_dl.mock_calls, [
-            mock.call(args, self.app, path_override='/var/cache/qvm-template',
+            mock.call(args, self.app, path_override='/var/tmp/qvm-template-tmpdir',
                 dl_list={}, suffix='.unverified', version_selector=selector)
         ])
         # Should not be executed:
diff --git a/qubesadmin/tools/qvm_template.py b/qubesadmin/tools/qvm_template.py
index aef780b..f16fdde 100644
--- a/qubesadmin/tools/qvm_template.py
+++ b/qubesadmin/tools/qvm_template.py
@@ -781,11 +781,12 @@ def install(
 
         unverified_rpm_list = [] # rpmfile, reponame
         verified_rpm_list = []
-        def verify(rpmfile, reponame):
+        def verify(rpmfile, reponame, dl_dir=None):
             """Verify package signature and version, remove "unverified"
             suffix, and parse package header."""
-            if reponame != '@commandline':
-                path = rpmfile + UNVERIFIED_SUFFIX
+            if dl_dir:
+                path = os.path.join(
+                    dl_dir, os.path.basename(rpmfile) + UNVERIFIED_SUFFIX)
             else:
                 path = rpmfile
 
@@ -892,13 +893,14 @@ def install(
                 'This will override changes made in the following VMs:',
                 override_tpls)
 
-        download(args, app, path_override=args.cachedir,
-            dl_list=dl_list, suffix=UNVERIFIED_SUFFIX,
-            version_selector=version_selector)
+        with tempfile.TemporaryDirectory(dir=args.cachedir) as dl_dir:
+            download(args, app, path_override=dl_dir,
+                dl_list=dl_list, suffix=UNVERIFIED_SUFFIX,
+                version_selector=version_selector)
 
-        # Verify downloaded templates
-        for rpmfile, reponame in unverified_rpm_list:
-            verify(rpmfile, reponame)
+            # Verify downloaded templates
+            for rpmfile, reponame in unverified_rpm_list:
+                verify(rpmfile, reponame, dl_dir=dl_dir)
         unverified_rpm_list = []
 
         # Unpack and install