Tighten up some logic around file permissions

Fix a regression in PbxCp which caused files to inappropriately gain the execute bit. `isExecutable` is conceptually a higher level operation which (per POSIX) can return true even if the file has no execute permission bits set:

"If execute permission is requested, access shall be granted if execute permission is granted to at least one user by the file permission bits or by an alternate access control mechanism"

No test because it's not clear what the concrete conditions are under which this occurs. That said, copying now checks against the owner permission bit specifically, as that's the actual intent of what the code is meaning to do and matches the original behavior.

Additionally, fix some missing permission information in getFileInfo for the PseudoFS implementation and increase test coverage a bit. Avoids it coming back to bite later.

rdar://154663442

Author: jakepetroules

Sources/SWBUtil/FSProxy.swift

@@ -160,7 +160,11 @@ public protocol FSProxy: AnyObject, Sendable {
     // FIXME: Need to document behavior w.r.t. error handling.
     func isDirectory(_ path: Path) -> Bool
 
-    /// Checks whether the given path has the execute bit (which on Windows is determined by the file extension).
+    /// Checks whether the given path is executable.
+    ///
+    /// On Windows, this is determined by the file extension (based on `SHGetFileInfo`), while on Unix it's determined by `access`, which means a file may be deemed executable even if no execute bit is set in the POSIX permissions.
+    ///
+    /// - seealso: [_stat, _stat32, _stat64, _stati64, _stat32i64, _stat64i32, _wstat, _wstat32, _wstat64, _wstati64, _wstat32i64, _wstat64i32](https://learn.microsoft.com/en-us/cpp/c-runtime-library/reference/stat-functions)
     func isExecutable(_ path: Path) throws -> Bool
 
     /// Checks whether the given path is a symlink, also returning whether the linked file exists.
@@ -309,6 +313,10 @@ public extension FSProxy {
     func getFileSize(_ path: Path) throws -> ByteCount {
         try ByteCount(Int64(getFileInfo(path).size))
     }
+
+    func getFilePermissions(_ path: Path) throws -> FilePermissions {
+        try FilePermissions(rawValue: CModeT(getFilePermissions(path)))
+    }
 }
 
 fileprivate extension FSProxy {
@@ -853,7 +861,7 @@ public class PseudoFS: FSProxy, @unchecked Sendable {
         }
 
         // Write the symlink.
-        directory.contents[path.basename] = Node(.symlink(target), permissions: 0o644, timestamp: getTimestamp(), inode: nextInode())
+        directory.contents[path.basename] = Node(.symlink(target), permissions: 0o755, timestamp: getTimestamp(), inode: nextInode())
         parent.timestamp = getTimestamp()
     }
 
@@ -1237,35 +1245,29 @@ public class PseudoFS: FSProxy, @unchecked Sendable {
     public func getFileInfo(_ path: Path) throws -> FileInfo {
         return try queue.blocking_sync {
             guard let node = getNode(path) else { throw POSIXError(ENOENT) }
+
+            let type: FileAttributeType
+            let size: Int
             switch node.contents {
             case .file(let contents):
-                let info: [FileAttributeKey: any Sendable] = [
-                    .modificationDate : Date(timeIntervalSince1970: TimeInterval(node.timestamp)),
-                    .type: FileAttributeType.typeRegular,
-                    .size: contents.bytes.count,
-                    .posixPermissions: 0,
-                    .systemNumber: node.device,
-                    .systemFileNumber: node.inode]
-                return createFileInfo(info)
+                type = .typeRegular
+                size = contents.bytes.count
             case .directory(let dir):
-                let info: [FileAttributeKey: any Sendable] = [
-                    .modificationDate: Date(timeIntervalSince1970: TimeInterval(node.timestamp)),
-                    .type: FileAttributeType.typeDirectory,
-                    .size: dir.contents.count,
-                    .posixPermissions: 0,
-                    .systemNumber: node.device,
-                    .systemFileNumber: node.inode]
-                return createFileInfo(info)
-            case .symlink(_):
-                let info: [FileAttributeKey: any Sendable] = [
-                    .modificationDate: Date(timeIntervalSince1970: TimeInterval(node.timestamp)),
-                    .type: FileAttributeType.typeSymbolicLink,
-                    .size: 0,
-                    .posixPermissions: 0,
-                    .systemNumber: node.device,
-                    .systemFileNumber: node.inode]
-                return createFileInfo(info)
+                type = .typeDirectory
+                size = dir.contents.count
+            case .symlink(let destination):
+                type = .typeSymbolicLink
+                size = destination.str.utf8.count
             }
+
+            let info: [FileAttributeKey: any Sendable] = [
+                .modificationDate : Date(timeIntervalSince1970: TimeInterval(node.timestamp)),
+                .type: type,
+                .size: size,
+                .posixPermissions: node.permissions,
+                .systemNumber: node.device,
+                .systemFileNumber: node.inode]
+            return createFileInfo(info)
         }
     }
 

Sources/SWBUtil/PbxCp.swift

@@ -298,8 +298,9 @@ fileprivate func copyRegular(_ srcPath: Path, _ srcParentPath: Path, _ dstPath:
 
 func _copyFile(_ srcPath: Path, _ dstPath: Path) throws {
     do {
+        let existingPermissions: FilePermissions = try localFS.getFilePermissions(srcPath)
         var permissions: FilePermissions = [.ownerRead, .ownerWrite, .groupRead, .groupWrite, .otherRead, .otherWrite]
-        if try localFS.isExecutable(srcPath) {
+        if existingPermissions.contains(.ownerExecute) {
             permissions.insert([.ownerExecute, .groupExecute, .otherExecute])
         }
         let dstFd = try FileDescriptor.open(FilePath(dstPath.str), .writeOnly, options: [.create, .truncate], permissions: permissions)

Tests/SWBUtilTests/FSProxyTests.swift

@@ -350,11 +350,21 @@ import SWBTestSupport
             // Test setting file permissions.
             let execPath = tmpDir.join("script.sh")
             try localFS.write(execPath, contents: [])
+            #expect(try localFS.getFilePermissions(execPath) == 0o644)
+            #expect(try localFS.getFileInfo(execPath).permissions == 0o644)
             #expect(try !localFS.isExecutable(execPath))
-
             try localFS.setFilePermissions(execPath, permissions: 0o755)
             #expect(try localFS.getFilePermissions(execPath) == 0o755)
+            #expect(try localFS.getFileInfo(execPath).permissions == 0o755)
             #expect(try localFS.isExecutable(execPath))
+
+            let linkPath = tmpDir.join("script")
+            try localFS.symlink(linkPath, target: Path("script.sh"))
+            #expect(try localFS.getFilePermissions(linkPath) == 0o755)
+            #expect(try localFS.getFileInfo(linkPath).permissions == 0o755)
+            try localFS.setFilePermissions(linkPath, permissions: 0o644)
+            #expect(try localFS.getFilePermissions(linkPath) == 0o644)
+            #expect(try localFS.getFileInfo(linkPath).permissions == 0o644)
         }
     }
 
@@ -804,16 +814,28 @@ import SWBTestSupport
 
         // Test default permissions.
         #expect(try fs.getFilePermissions(.root) == 0o755)
+        #expect(try fs.getFileInfo(.root).permissions == 0o755)
         let filePath = Path.root.join("file.txt")
         try fs.write(filePath, contents: [])
         #expect(try fs.getFilePermissions(filePath) == 0o644)
+        #expect(try fs.getFileInfo(filePath).permissions == 0o644)
 
         // Test setting file permissions.
         let execPath = Path.root.join("script.sh")
         try fs.write(execPath, contents: [])
         #expect(try fs.getFilePermissions(execPath) == 0o644)
+        #expect(try fs.getFileInfo(execPath).permissions == 0o644)
         try fs.setFilePermissions(execPath, permissions: 0o755)
         #expect(try fs.getFilePermissions(execPath) == 0o755)
+        #expect(try fs.getFileInfo(execPath).permissions == 0o755)
+
+        let linkPath = Path.root.join("script")
+        try fs.symlink(linkPath, target: Path("script.sh"))
+        #expect(try fs.getFilePermissions(linkPath) == 0o755)
+        #expect(try fs.getFileInfo(linkPath).permissions == 0o755)
+        try fs.setFilePermissions(linkPath, permissions: 0o644)
+        #expect(try fs.getFilePermissions(linkPath) == 0o644)
+        #expect(try fs.getFileInfo(linkPath).permissions == 0o644)
     }
 
     @Test