Readd permission setting to new files/folders

include/multipass/utils.h

@@ -202,14 +202,18 @@ class Utils : public Singleton<Utils>
 public:
     Utils(const Singleton<Utils>::PrivatePass&) noexcept;
 
+    static constexpr QFileDevice::Permissions default_permissions =
+        QFile::ReadOwner | QFile::WriteOwner | QFile::ExeOwner;
+
     virtual qint64 filesystem_bytes_available(const QString& data_directory) const;
     virtual void exit(int code);
     virtual std::string contents_of(const multipass::Path& file_path) const;
     virtual void make_file_with_content(const std::string& file_name, const std::string& content,
                                         const bool& overwrite = false);
-    virtual Path make_dir(const QDir& a_dir, const QString& name,
-                          QFileDevice::Permissions permissions = QFileDevice::Permissions(0));
-    virtual Path make_dir(const QDir& dir, QFileDevice::Permissions permissions = QFileDevice::Permissions(0));
+    virtual Path make_dir(const QDir& a_dir,
+                          const QString& name,
+                          QFileDevice::Permissions permissions = default_permissions);
+    virtual Path make_dir(const QDir& dir, QFileDevice::Permissions permissions = default_permissions);
 
     // command and process helpers
     virtual std::string run_cmd_for_output(const QString& cmd, const QStringList& args,

src/cert/client_cert_store.cpp

@@ -26,6 +26,7 @@
 #include <QFile>
 #include <QSaveFile>
 
+#include <multipass/utils/permission_utils.h>
 #include <stdexcept>
 
 namespace mp = multipass;
@@ -76,7 +77,7 @@ void mp::ClientCertStore::add_cert(const std::string& pem_cert)
     if (!MP_FILEOPS.open(file, QIODevice::WriteOnly))
         throw std::runtime_error("failed to create file to store certificate");
 
-    file.setPermissions(QFile::ReadOwner | QFile::WriteOwner);
+    MP_PERMISSIONS.restrict_permissions(file.fileName().toStdU16String());
 
     // QIODevice::Append is not supported in QSaveFile, so must write out all of the
     // existing clients certs each time.

src/iso/cloud_init_iso.cpp

@@ -538,6 +538,8 @@ void mp::CloudInitIso::write_to(const Path& path)
         throw std::runtime_error{fmt::format(
             "Failed to open file for writing during cloud-init generation: {}; path: {}", f.errorString(), path)};
 
+    MP_PERMISSIONS.restrict_permissions(path.toStdU16String());
+
     const uint32_t num_reserved_bytes = 32768u;
     const uint32_t num_reserved_blocks = num_blocks(num_reserved_bytes);
     f.seek(num_reserved_bytes);

src/platform/backends/lxd/lxd_vm_image_vault.cpp

@@ -94,6 +94,8 @@ QString post_process_downloaded_image(const QString& image_path, const mp::Progr
         mp::vault::delete_file(original_image_path);
     }
 
+    MP_PERMISSIONS.restrict_permissions(new_image_path.toStdU16String());
+
     return new_image_path;
 }
 

src/utils/vm_image_vault_utils.cpp

@@ -48,6 +48,8 @@ QString mp::vault::copy(const QString& file_name, const QDir& output_dir)
     auto new_path = output_dir.filePath(source_name);
     QFile::copy(file_name, new_path);
 
+    MP_PERMISSIONS.restrict_permissions(new_path.toStdU16String());
+
     return new_path;
 }
 
@@ -95,6 +97,8 @@ QString mp::vault::extract_image(const mp::Path& image_path, const mp::ProgressM
 
     mp::vault::delete_file(image_path);
 
+    MP_PERMISSIONS.restrict_permissions(new_image_path.toStdU16String());
+
     return new_image_path;
 }
 

tests/lxd/test_lxd_image_vault.cpp

@@ -22,6 +22,7 @@
 #include "tests/common.h"
 #include "tests/mock_image_host.h"
 #include "tests/mock_logger.h"
+#include "tests/mock_permission_utils.h"
 #include "tests/mock_process_factory.h"
 #include "tests/stub_url_downloader.h"
 #include "tests/temp_dir.h"
@@ -60,6 +61,9 @@ struct LXDImageVault : public Test
 
     mpt::MockLogger::Scope logger_scope = mpt::MockLogger::inject();
     std::unique_ptr<NiceMock<mpt::MockNetworkAccessManager>> mock_network_access_manager;
+    const mpt::MockPermissionUtils::GuardedMock mock_permission_utils_injection =
+        mpt::MockPermissionUtils::inject<NiceMock>();
+    mpt::MockPermissionUtils& mock_permission_utils = *mock_permission_utils_injection.first;
     std::vector<mp::VMImageHost*> hosts;
     NiceMock<mpt::MockImageHost> host;
     QUrl base_url{"unix:///[email protected]"};

tests/test_base_virtual_machine_factory.cpp

@@ -17,6 +17,7 @@
 
 #include "common.h"
 #include "mock_logger.h"
+#include "mock_permission_utils.h"
 #include "mock_platform.h"
 #include "stub_ssh_key_provider.h"
 #include "stub_url_downloader.h"
@@ -123,6 +124,9 @@ TEST_F(BaseFactory, networks_throws)
 // at this time.  Instead, just make sure an ISO image is created and has the expected path.
 TEST_F(BaseFactory, creates_cloud_init_iso_image)
 {
+    auto [mock_permission_utils, permission_utils_guard] = mpt::MockPermissionUtils::inject();
+    EXPECT_CALL(*mock_permission_utils, restrict_permissions(_));
+
     MockBaseFactory factory;
     const std::string name{"foo"};
     const YAML::Node metadata{YAML::Load({fmt::format("name: {}", name)})}, vendor_data{metadata}, user_data{metadata},

tests/test_client_cert_store.cpp

@@ -19,6 +19,7 @@
 #include "file_operations.h"
 #include "mock_file_ops.h"
 #include "mock_logger.h"
+#include "mock_permission_utils.h"
 #include "mock_utils.h"
 #include "temp_dir.h"
 
@@ -99,6 +100,9 @@ TEST_F(ClientCertStore, add_cert_throws_on_invalid_data)
 
 TEST_F(ClientCertStore, add_cert_stores_certificate)
 {
+    auto [mock_permission_utils, permission_utils_guard] = mpt::MockPermissionUtils::inject();
+    EXPECT_CALL(*mock_permission_utils, restrict_permissions(_));
+
     mp::ClientCertStore cert_store{temp_dir.path()};
     EXPECT_NO_THROW(cert_store.add_cert(cert_data));
 
@@ -151,6 +155,9 @@ TEST_F(ClientCertStore, addCertAlreadyExistingDoesNotAddAgain)
 
 TEST_F(ClientCertStore, addCertWithExistingCertPersistsCerts)
 {
+    auto [mock_permission_utils, permission_utils_guard] = mpt::MockPermissionUtils::inject();
+    EXPECT_CALL(*mock_permission_utils, restrict_permissions(_));
+
     const QDir dir{cert_dir};
     const auto cert_path = dir.filePath("multipass_client_certs.pem");
     mpt::make_file_with_content(cert_path, cert_data);
@@ -191,6 +198,9 @@ TEST_F(ClientCertStore, openingFileForWritingFailsAndThrows)
     auto [mock_file_ops, guard] = mpt::MockFileOps::inject();
     EXPECT_CALL(*mock_file_ops, open(_, _)).WillOnce(Return(false));
 
+    auto [mock_permission_utils, permission_utils_guard] = mpt::MockPermissionUtils::inject();
+    EXPECT_CALL(*mock_permission_utils, set_permissions(_, _));
+
     mp::ClientCertStore cert_store{temp_dir.path()};
 
     MP_EXPECT_THROW_THAT(cert_store.add_cert(cert_data), std::runtime_error,
@@ -206,6 +216,9 @@ TEST_F(ClientCertStore, writingFileFailsAndThrows)
     EXPECT_CALL(*mock_file_ops, write(_, _)).WillOnce(Return(-1));
     EXPECT_CALL(*mock_file_ops, commit).WillOnce(Return(false));
 
+    auto [mock_permission_utils, permission_utils_guard] = mpt::MockPermissionUtils::inject();
+    EXPECT_CALL(*mock_permission_utils, restrict_permissions(_));
+
     mp::ClientCertStore cert_store{temp_dir.path()};
 
     MP_EXPECT_THROW_THAT(cert_store.add_cert(cert_data), std::runtime_error,

tests/test_cloud_init_iso.cpp

@@ -17,6 +17,7 @@
 
 #include "common.h"
 #include "mock_file_ops.h"
+#include "mock_permission_utils.h"
 #include "temp_dir.h"
 
 #include <multipass/cloud_init_iso.h>
@@ -75,6 +76,9 @@ struct CloudInitIso : public Test
     mpt::TempDir temp_dir;
     QString iso_path;
     std::filesystem::path std_iso_path;
+
+    const mpt::MockPermissionUtils::GuardedMock attr{mpt::MockPermissionUtils::inject<NiceMock>()};
+    mpt::MockPermissionUtils& mock_permission_utils = *attr.first;
 };
 
 TEST_F(CloudInitIso, check_contains_false)
@@ -136,6 +140,8 @@ TEST_F(CloudInitIso, check_index_operator_found_key)
 
 TEST_F(CloudInitIso, creates_iso_file)
 {
+    EXPECT_CALL(mock_permission_utils, restrict_permissions(_));
+
     mp::CloudInitIso iso;
     iso.add_file("test", "test data");
     iso.write_to(iso_path);
@@ -147,6 +153,8 @@ TEST_F(CloudInitIso, creates_iso_file)
 
 TEST_F(CloudInitIso, reads_iso_file_failed_to_open_file)
 {
+    EXPECT_CALL(mock_permission_utils, restrict_permissions(_));
+
     mp::CloudInitIso original_iso;
     original_iso.write_to(iso_path);
 
@@ -160,6 +168,8 @@ TEST_F(CloudInitIso, reads_iso_file_failed_to_open_file)
 
 TEST_F(CloudInitIso, reads_iso_file_failed_to_read_single_bytes)
 {
+    EXPECT_CALL(mock_permission_utils, restrict_permissions(_));
+
     mp::CloudInitIso original_iso;
     original_iso.write_to(iso_path);
 
@@ -178,6 +188,8 @@ TEST_F(CloudInitIso, reads_iso_file_failed_to_read_single_bytes)
 
 TEST_F(CloudInitIso, reads_iso_file_failed_to_check_it_has_Joliet_volume_descriptor)
 {
+    EXPECT_CALL(mock_permission_utils, restrict_permissions(_));
+
     mp::CloudInitIso original_iso;
     original_iso.write_to(iso_path);
 
@@ -201,6 +213,8 @@ TEST_F(CloudInitIso, reads_iso_file_failed_to_check_it_has_Joliet_volume_descrip
 
 TEST_F(CloudInitIso, reads_iso_file_Joliet_volume_descriptor_malformed)
 {
+    EXPECT_CALL(mock_permission_utils, restrict_permissions(_));
+
     mp::CloudInitIso original_iso;
     original_iso.write_to(iso_path);
 
@@ -226,6 +240,8 @@ TEST_F(CloudInitIso, reads_iso_file_Joliet_volume_descriptor_malformed)
 
 TEST_F(CloudInitIso, reads_iso_file_failed_to_read_array)
 {
+    EXPECT_CALL(mock_permission_utils, restrict_permissions(_));
+
     mp::CloudInitIso original_iso;
     original_iso.write_to(iso_path);
 
@@ -247,6 +263,8 @@ TEST_F(CloudInitIso, reads_iso_file_failed_to_read_array)
 
 TEST_F(CloudInitIso, reads_iso_file_failed_to_check_root_dir_record_data)
 {
+    EXPECT_CALL(mock_permission_utils, restrict_permissions(_));
+
     mp::CloudInitIso original_iso;
     original_iso.write_to(iso_path);
 
@@ -274,6 +292,8 @@ TEST_F(CloudInitIso, reads_iso_file_failed_to_check_root_dir_record_data)
 
 TEST_F(CloudInitIso, reads_iso_file_failed_to_read_vec)
 {
+    EXPECT_CALL(mock_permission_utils, restrict_permissions(_));
+
     mp::CloudInitIso original_iso;
     // At least one actual file entry is need to reach the read_bytes_to_vec call
     original_iso.add_file("test1", "test data1");
@@ -299,6 +319,8 @@ TEST_F(CloudInitIso, reads_iso_file_failed_to_read_vec)
 
 TEST_F(CloudInitIso, reads_iso_file_encoded_file_name_is_not_even_length)
 {
+    EXPECT_CALL(mock_permission_utils, restrict_permissions(_));
+
     mp::CloudInitIso original_iso;
     // At least one actual file entry is need to reach the convert_u16_name_back call
     original_iso.add_file("test1", "test data1");
@@ -334,6 +356,8 @@ TEST_F(CloudInitIso, reads_iso_file_encoded_file_name_is_not_even_length)
 
 TEST_F(CloudInitIso, reads_iso_file_with_random_string_data)
 {
+    EXPECT_CALL(mock_permission_utils, restrict_permissions(_));
+
     mp::CloudInitIso original_iso;
 
     original_iso.add_file("test1", "test data1");
@@ -371,6 +395,8 @@ timezone: Europe/Amsterdam
     content: "multipass/version/1.14.0-dev.1209+g5b2c7f7d # written by Multipass\nmultipass/driver/qemu-8.0.4 # written by Multipass\nmultipass/host/ubuntu-23.10 # written by Multipass\nmultipass/alias/default # written by Multipass\n"
 )";
 
+    EXPECT_CALL(mock_permission_utils, restrict_permissions(_));
+
     mp::CloudInitIso original_iso;
 
     original_iso.add_file("meta-data", default_meta_data_content);
@@ -386,6 +412,8 @@ timezone: Europe/Amsterdam
 
 TEST_F(CloudInitIso, updateCloudInitWithNewNonEmptyExtraInterfaces)
 {
+    EXPECT_CALL(mock_permission_utils, restrict_permissions(std_iso_path)).Times(2);
+
     mp::CloudInitIso original_iso;
 
     original_iso.add_file("meta-data", default_meta_data_content);
@@ -412,6 +440,8 @@ TEST_F(CloudInitIso, updateCloudInitWithNewNonEmptyExtraInterfaces)
 
 TEST_F(CloudInitIso, updateCloudInitWithNewEmptyExtraInterfaces)
 {
+    EXPECT_CALL(mock_permission_utils, restrict_permissions(std_iso_path)).Times(2);
+
     mp::CloudInitIso original_iso;
     original_iso.add_file("meta-data", default_meta_data_content);
     original_iso.add_file("network-config", "dummy_data");
@@ -435,6 +465,8 @@ TEST_F(CloudInitIso, updateCloneCloudInitSrcFileWithExtraInterfaces)
     const std::string src_network_config_data_content =
         fmt::format(network_config_data_content_template, "00:00:00:00:00:00", "00:00:00:00:00:01");
 
+    EXPECT_CALL(mock_permission_utils, restrict_permissions(std_iso_path)).Times(2);
+
     mp::CloudInitIso original_iso;
     original_iso.add_file("meta-data", src_meta_data_content);
     original_iso.add_file("network-config", src_network_config_data_content);
@@ -460,6 +492,8 @@ TEST_F(CloudInitIso, updateCloneCloudInitSrcFileWithExtraInterfaces)
 
 TEST_F(CloudInitIso, addExtraInterfaceToCloudInit)
 {
+    EXPECT_CALL(mock_permission_utils, restrict_permissions(std_iso_path)).Times(2);
+
     mp::CloudInitIso original_iso;
     original_iso.add_file("meta-data", default_meta_data_content);
     original_iso.write_to(iso_path);
@@ -471,6 +505,8 @@ TEST_F(CloudInitIso, addExtraInterfaceToCloudInit)
 
 TEST_F(CloudInitIso, getInstanceIdFromCloudInit)
 {
+    EXPECT_CALL(mock_permission_utils, restrict_permissions(_));
+
     mp::CloudInitIso original_iso;
     original_iso.add_file("meta-data", default_meta_data_content);
     original_iso.write_to(iso_path);

tests/test_image_vault.cpp

@@ -21,6 +21,7 @@
 #include "mock_image_host.h"
 #include "mock_json_utils.h"
 #include "mock_logger.h"
+#include "mock_permission_utils.h"
 #include "mock_platform.h"
 #include "mock_process_factory.h"
 #include "path.h"
@@ -167,6 +168,9 @@ struct ImageVault : public testing::Test
     NiceMock<mpt::MockImageHost> host;
     mpt::MockJsonUtils::GuardedMock mock_json_utils_injection = mpt::MockJsonUtils::inject<NiceMock>();
     mpt::MockJsonUtils& mock_json_utils = *mock_json_utils_injection.first;
+    const mpt::MockPermissionUtils::GuardedMock mock_permission_utils_injection =
+        mpt::MockPermissionUtils::inject<NiceMock>();
+    mpt::MockPermissionUtils& mock_permission_utils = *mock_permission_utils_injection.first;
     mp::ProgressMonitor stub_monitor{[](int, int) { return true; }};
     mp::VMImageVault::PrepareAction stub_prepare{
         [](const mp::VMImage& source_image) -> mp::VMImage { return source_image; }};
@@ -414,6 +418,8 @@ TEST_F(ImageVault, remembers_prepared_images)
 
 TEST_F(ImageVault, uses_image_from_prepare)
 {
+    EXPECT_CALL(mock_permission_utils, restrict_permissions(_));
+
     constexpr auto expected_data = "12345-pied-piper-rats";
 
     QDir dir{cache_dir.path()};
@@ -509,6 +515,8 @@ TEST_F(ImageVault, invalid_image_dir_is_removed)
 
 TEST_F(ImageVault, DISABLE_ON_WINDOWS_AND_MACOS(file_based_fetch_copies_image_and_returns_expected_info))
 {
+    EXPECT_CALL(mock_permission_utils, restrict_permissions(_));
+
     mpt::TempFile file;
     mp::DefaultVMImageVault vault{hosts, &url_downloader, cache_dir.path(), data_dir.path(), mp::days{0}};
     auto query = default_query;
@@ -740,6 +748,8 @@ TEST_F(ImageVault, minimum_image_size_returns_expected_size)
 
 TEST_F(ImageVault, DISABLE_ON_WINDOWS_AND_MACOS(file_based_minimum_size_returns_expected_size))
 {
+    EXPECT_CALL(mock_permission_utils, restrict_permissions(_));
+
     const mp::MemorySize image_size{"2097152"};
     const mp::ProcessState qemuimg_exit_status{0, std::nullopt};
     const QByteArray qemuimg_output(fake_img_info(image_size));

tests/test_utils.cpp

@@ -750,6 +750,9 @@ TEST(Utils, check_filesystem_bytes_available_returns_non_negative)
 
 TEST(VaultUtils, copy_creates_new_file_and_returned_path_exists)
 {
+    auto [mock_permission_utils, permission_utils_guard] = mpt::MockPermissionUtils::inject();
+    EXPECT_CALL(*mock_permission_utils, restrict_permissions(_));
+
     mpt::TempDir temp_dir1, temp_dir2;
     auto orig_file_path = QDir(temp_dir1.path()).filePath("test_file");