From 060604e02803495b152db9c89d1fe75f76823b36 Mon Sep 17 00:00:00 2001
From: Ro <ro@freedom.press>
Date: Thu, 14 Mar 2024 20:19:20 -0400
Subject: [PATCH] Ensure files on USB have read permissions, not just enclosing
 directory.

---
 export/securedrop_export/disk/cli.py |  5 +++-
 export/tests/disk/test_cli.py        | 41 ++++++++++++++++++++++++++++
 2 files changed, 45 insertions(+), 1 deletion(-)

diff --git a/export/securedrop_export/disk/cli.py b/export/securedrop_export/disk/cli.py
index e8275e627..88087dfad 100644
--- a/export/securedrop_export/disk/cli.py
+++ b/export/securedrop_export/disk/cli.py
@@ -430,7 +430,10 @@ def write_data_to_device(
             logger.debug("Copying file to {}".format(archive_target_dirname))
 
             subprocess.check_call(["cp", "-r", export_data, target_path])
-            logger.info("File copied successfully to {}".format(archive_target_dirname))
+            logger.info("File copied successfully to {}".format(target_path))
+
+            subprocess.check_call(["sudo", "chmod", "-R", "ugo+r", target_path])
+            logger.info("Read permissions granted on {}".format(target_path))
 
         except (subprocess.CalledProcessError, OSError) as ex:
             logger.error(ex)
diff --git a/export/tests/disk/test_cli.py b/export/tests/disk/test_cli.py
index c046a1f95..d3d1950c8 100644
--- a/export/tests/disk/test_cli.py
+++ b/export/tests/disk/test_cli.py
@@ -1,10 +1,14 @@
+import os
 import re
+import stat
 import subprocess
+from tempfile import TemporaryDirectory
 from unittest import mock
 
 import pytest
 
 from securedrop_export.archive import Archive
+from securedrop_export.directory import safe_mkdir
 from securedrop_export.disk.cli import CLI
 from securedrop_export.disk.status import Status
 from securedrop_export.disk.volume import EncryptionScheme, MountedVolume, Volume
@@ -454,3 +458,40 @@ def test_parse_correct_mountpoint_from_pexpect(self, mock_pexpect):
         )
         assert mv.unlocked_name == "/dev/dm-0"
         assert mv.mountpoint == "/media/usb"
+
+    def test__write_data_correct_permissions(self):
+        # This is the permission set when extracting the archive
+        old_umask = os.umask(0o77)
+
+        try:
+            with mock.patch.object(
+                self.cli, "cleanup"
+            ), TemporaryDirectory() as archive_dir, TemporaryDirectory() as mock_mountpoint:
+                safe_mkdir(archive_dir, "export_data")
+                export_dir = os.path.join(archive_dir, "export_data")
+                export_filepath = os.path.join(export_dir, "export_file.txt")
+                export_file = open(export_filepath, "w+")
+                export_file.write("Export me!")
+                export_file.close()
+
+                # Sanity
+                stat_result = oct(stat.S_IMODE(os.stat(export_filepath).st_mode))
+                assert stat_result == "0o600"
+
+                fake_volume = MountedVolume(
+                    "fake", "fake", EncryptionScheme.LUKS, mountpoint=mock_mountpoint
+                )
+                self.cli.write_data_to_device(fake_volume, archive_dir, "sd-export")
+
+                export_dir = os.path.join(mock_mountpoint, "sd-export")
+                export_datadir = os.path.join(export_dir, "export_data")
+                export_file = os.path.join(export_datadir, "export_file.txt")
+
+                assert os.path.exists(export_dir)
+                assert os.path.exists(export_file)
+
+                stat_result = oct(stat.S_IMODE(os.stat(export_file).st_mode))
+                assert stat_result == "0o644"
+
+        finally:
+            os.umask(old_umask)