Skip to content

Tighten up some logic around file permissions #662

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
56 changes: 29 additions & 27 deletions Sources/SWBUtil/FSProxy.swift
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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()
}

Expand Down Expand Up @@ -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)
}
}

Expand Down
3 changes: 2 additions & 1 deletion Sources/SWBUtil/PbxCp.swift
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
24 changes: 23 additions & 1 deletion Tests/SWBUtilTests/FSProxyTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
}

Expand Down Expand Up @@ -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
Expand Down