[platform] Remove chmod from platform.h

include/multipass/platform.h

@@ -58,8 +58,7 @@ class Platform : public Singleton<Platform>
     virtual bool is_remote_supported(const std::string& remote) const;
     virtual bool is_backend_supported(const QString& backend) const; // temporary (?)
     virtual int chown(const char* path, unsigned int uid, unsigned int gid) const;
-    virtual int chmod(const char* path, unsigned int mode) const;
-    virtual bool set_permissions(const multipass::Path path, const QFileDevice::Permissions permissions) const;
+    virtual bool set_permissions(const multipass::Path& path, const QFileDevice::Permissions permissions) const;
     virtual bool link(const char* target, const char* link) const;
     virtual bool symlink(const char* target, const char* link, bool is_dir) const;
     virtual int utime(const char* path, int atime, int mtime) const;

src/platform/platform_unix.cpp

@@ -58,12 +58,7 @@ int mp::platform::Platform::chown(const char* path, unsigned int uid, unsigned i
     return ::lchown(path, uid, gid);
 }
 
-int mp::platform::Platform::chmod(const char* path, unsigned int mode) const
-{
-    return ::chmod(path, mode);
-}
-
-bool mp::platform::Platform::set_permissions(const mp::Path path, const QFileDevice::Permissions permissions) const
+bool mp::platform::Platform::set_permissions(const mp::Path& path, const QFileDevice::Permissions permissions) const
 {
     return QFile::setPermissions(path, permissions);
 }
@@ -108,7 +103,7 @@ void mp::platform::Platform::set_server_socket_restrictions(const std::string& s
         return;
 
     int gid{0};
-    int mode{S_IRUSR | S_IWUSR | S_IRGRP | S_IWGRP};
+    auto mode{QFileDevice::ReadUser | QFileDevice::WriteUser | QFileDevice::ReadGroup | QFileDevice::WriteGroup};
 
     if (restricted)
     {
@@ -124,14 +119,14 @@ void mp::platform::Platform::set_server_socket_restrictions(const std::string& s
     }
     else
     {
-        mode |= S_IROTH | S_IWOTH;
+        mode |= QFileDevice::ReadOther | QFileDevice::WriteOther;
     }
 
     const auto socket_path = tokens[1];
     if (chown(socket_path.c_str(), 0, gid) == -1)
         throw std::runtime_error(fmt::format("Could not set ownership of the multipass socket: {}", strerror(errno)));
 
-    if (chmod(socket_path.c_str(), mode) == -1)
+    if (!set_permissions(socket_path.c_str(), mode))
         throw std::runtime_error(
             fmt::format("Could not set permissions for the multipass socket: {}", strerror(errno)));
 }

tests/mock_platform.h

@@ -40,7 +40,7 @@ class MockPlatform : public platform::Platform
     MOCK_METHOD(bool, is_remote_supported, (const std::string&), (const, override));
     MOCK_METHOD(bool, is_backend_supported, (const QString&), (const, override));
     MOCK_METHOD(bool, is_alias_supported, (const std::string&, const std::string&), (const, override));
-    MOCK_METHOD(int, chmod, (const char*, unsigned int), (const, override));
+    MOCK_METHOD(bool, set_permissions, (const multipass::Path&, const QFileDevice::Permissions), (const, override));
     MOCK_METHOD(int, chown, (const char*, unsigned int, unsigned int), (const, override));
     MOCK_METHOD(bool, link, (const char*, const char*), (const, override));
     MOCK_METHOD(bool, symlink, (const char*, const char*, bool), (const, override));

tests/unix/test_platform_unix.cpp

@@ -36,6 +36,11 @@ namespace
 struct TestPlatformUnix : public Test
 {
     mpt::TempFile file;
+
+    static constexpr auto restricted_permissions{QFileDevice::ReadUser | QFileDevice::WriteUser |
+                                                 QFileDevice::ReadGroup | QFileDevice::WriteGroup};
+    static constexpr auto relaxed_permissions{restricted_permissions | QFileDevice::ReadOther |
+                                              QFileDevice::WriteOther};
 };
 } // namespace
 
@@ -44,8 +49,7 @@ TEST_F(TestPlatformUnix, setServerSocketRestrictionsNotRestrictedIsCorrect)
     auto [mock_platform, guard] = mpt::MockPlatform::inject();
 
     EXPECT_CALL(*mock_platform, chown(_, 0, 0)).WillOnce(Return(0));
-    EXPECT_CALL(*mock_platform, chmod(_, S_IRUSR | S_IWUSR | S_IRGRP | S_IWGRP | S_IROTH | S_IWOTH))
-        .WillOnce(Return(0));
+    EXPECT_CALL(*mock_platform, set_permissions(_, relaxed_permissions)).WillOnce(Return(true));
 
     EXPECT_NO_THROW(MP_PLATFORM.Platform::set_server_socket_restrictions(fmt::format("unix:{}", file.name()), false));
 }
@@ -58,7 +62,7 @@ TEST_F(TestPlatformUnix, setServerSocketRestrictionsRestrictedIsCorrect)
     group.gr_gid = gid;
 
     EXPECT_CALL(*mock_platform, chown(_, 0, gid)).WillOnce(Return(0));
-    EXPECT_CALL(*mock_platform, chmod(_, S_IRUSR | S_IWUSR | S_IRGRP | S_IWGRP)).WillOnce(Return(0));
+    EXPECT_CALL(*mock_platform, set_permissions(_, restricted_permissions)).WillOnce(Return(true));
 
     REPLACE(getgrnam, [&group](auto) { return &group; });
 
@@ -77,7 +81,7 @@ TEST_F(TestPlatformUnix, setServerSocketRestrictionsNonUnixTypeReturns)
     auto [mock_platform, guard] = mpt::MockPlatform::inject();
 
     EXPECT_CALL(*mock_platform, chown).Times(0);
-    EXPECT_CALL(*mock_platform, chmod).Times(0);
+    EXPECT_CALL(*mock_platform, set_permissions).Times(0);
 
     EXPECT_NO_THROW(MP_PLATFORM.Platform::set_server_socket_restrictions(fmt::format("dns:{}", file.name()), false));
 }
@@ -102,11 +106,10 @@ TEST_F(TestPlatformUnix, setServerSocketRestrictionsChmodFailsThrows)
     auto [mock_platform, guard] = mpt::MockPlatform::inject();
 
     EXPECT_CALL(*mock_platform, chown(_, 0, 0)).WillOnce(Return(0));
-    EXPECT_CALL(*mock_platform, chmod(_, S_IRUSR | S_IWUSR | S_IRGRP | S_IWGRP | S_IROTH | S_IWOTH))
-        .WillOnce([](auto...) {
-            errno = EPERM;
-            return -1;
-        });
+    EXPECT_CALL(*mock_platform, set_permissions(_, relaxed_permissions)).WillOnce([](auto...) {
+        errno = EPERM;
+        return false;
+    });
 
     MP_EXPECT_THROW_THAT(
         MP_PLATFORM.Platform::set_server_socket_restrictions(fmt::format("unix:{}", file.name()), false),
@@ -119,7 +122,7 @@ TEST_F(TestPlatformUnix, chmodSetsFileModsAndReturns)
     auto perms = QFileInfo(file.name()).permissions();
     ASSERT_EQ(perms, QFileDevice::ReadOwner | QFileDevice::WriteOwner | QFileDevice::ReadUser | QFileDevice::WriteUser);
 
-    EXPECT_EQ(MP_PLATFORM.chmod(file.name().toStdString().c_str(), S_IRUSR | S_IWUSR | S_IRGRP | S_IWGRP), 0);
+    EXPECT_EQ(MP_PLATFORM.set_permissions(file.name(), restricted_permissions), true);
 
     perms = QFileInfo(file.name()).permissions();