diff --git a/apps/macos/Sources/OpenClaw/ExecApprovals.swift b/apps/macos/Sources/OpenClaw/ExecApprovals.swift index 73aa3899d82..cb6d0e7d7a7 100644 --- a/apps/macos/Sources/OpenClaw/ExecApprovals.swift +++ b/apps/macos/Sources/OpenClaw/ExecApprovals.swift @@ -226,6 +226,7 @@ enum ExecApprovalsStore { private static let defaultAsk: ExecAsk = .onMiss private static let defaultAskFallback: ExecSecurity = .deny private static let defaultAutoAllowSkills = false + private static let secureStateDirPermissions = 0o700 static func fileURL() -> URL { OpenClawPaths.stateDirURL.appendingPathComponent("exec-approvals.json") @@ -332,6 +333,7 @@ enum ExecApprovalsStore { encoder.outputFormatting = [.prettyPrinted, .sortedKeys] let data = try encoder.encode(file) let url = self.fileURL() + self.ensureSecureStateDirectory() try FileManager().createDirectory( at: url.deletingLastPathComponent(), withIntermediateDirectories: true) @@ -343,6 +345,7 @@ enum ExecApprovalsStore { } static func ensureFile() -> ExecApprovalsFile { + self.ensureSecureStateDirectory() let url = self.fileURL() let existed = FileManager().fileExists(atPath: url.path) let loaded = self.loadFile() @@ -524,6 +527,18 @@ enum ExecApprovalsStore { self.saveFile(file) } + private static func ensureSecureStateDirectory() { + let url = OpenClawPaths.stateDirURL + do { + try FileManager().createDirectory(at: url, withIntermediateDirectories: true) + try FileManager().setAttributes( + [.posixPermissions: self.secureStateDirPermissions], + ofItemAtPath: url.path) + } catch { + self.logger.warning("exec approvals state dir permission hardening failed: \(error.localizedDescription, privacy: .public)") + } + } + private static func generateToken() -> String { var bytes = [UInt8](repeating: 0, count: 24) let status = SecRandomCopyBytes(kSecRandomDefault, bytes.count, &bytes) diff --git a/apps/macos/Sources/OpenClaw/ExecApprovalsSocket.swift b/apps/macos/Sources/OpenClaw/ExecApprovalsSocket.swift index 2c308b3eeb6..1ce962eb3ff 100644 --- a/apps/macos/Sources/OpenClaw/ExecApprovalsSocket.swift +++ b/apps/macos/Sources/OpenClaw/ExecApprovalsSocket.swift @@ -544,6 +544,106 @@ private enum ExecHostExecutor { } } +enum ExecApprovalsSocketPathKind: Equatable { + case missing + case directory + case socket + case symlink + case other +} + +enum ExecApprovalsSocketPathGuardError: LocalizedError { + case lstatFailed(path: String, code: Int32) + case parentPathInvalid(path: String, kind: ExecApprovalsSocketPathKind) + case socketPathInvalid(path: String, kind: ExecApprovalsSocketPathKind) + case unlinkFailed(path: String, code: Int32) + case createParentDirectoryFailed(path: String, message: String) + case setParentDirectoryPermissionsFailed(path: String, message: String) + + var errorDescription: String? { + switch self { + case let .lstatFailed(path, code): + "lstat failed for \(path) (errno \(code))" + case let .parentPathInvalid(path, kind): + "socket parent path invalid (\(kind)) at \(path)" + case let .socketPathInvalid(path, kind): + "socket path invalid (\(kind)) at \(path)" + case let .unlinkFailed(path, code): + "unlink failed for \(path) (errno \(code))" + case let .createParentDirectoryFailed(path, message): + "socket parent directory create failed at \(path): \(message)" + case let .setParentDirectoryPermissionsFailed(path, message): + "socket parent directory chmod failed at \(path): \(message)" + } + } +} + +enum ExecApprovalsSocketPathGuard { + static let parentDirectoryPermissions = 0o700 + + static func pathKind(at path: String) throws -> ExecApprovalsSocketPathKind { + var status = stat() + let result = lstat(path, &status) + if result != 0 { + if errno == ENOENT { + return .missing + } + throw ExecApprovalsSocketPathGuardError.lstatFailed(path: path, code: errno) + } + + let fileType = status.st_mode & mode_t(S_IFMT) + if fileType == mode_t(S_IFDIR) { return .directory } + if fileType == mode_t(S_IFSOCK) { return .socket } + if fileType == mode_t(S_IFLNK) { return .symlink } + return .other + } + + static func hardenParentDirectory(for socketPath: String) throws { + let parentURL = URL(fileURLWithPath: socketPath).deletingLastPathComponent() + let parentPath = parentURL.path + + switch try self.pathKind(at: parentPath) { + case .missing, .directory: + break + case let kind: + throw ExecApprovalsSocketPathGuardError.parentPathInvalid(path: parentPath, kind: kind) + } + + do { + try FileManager().createDirectory(at: parentURL, withIntermediateDirectories: true) + } catch { + throw ExecApprovalsSocketPathGuardError.createParentDirectoryFailed( + path: parentPath, + message: error.localizedDescription) + } + + do { + try FileManager().setAttributes( + [.posixPermissions: self.parentDirectoryPermissions], + ofItemAtPath: parentPath) + } catch { + throw ExecApprovalsSocketPathGuardError.setParentDirectoryPermissionsFailed( + path: parentPath, + message: error.localizedDescription) + } + } + + static func removeExistingSocket(at socketPath: String) throws { + let kind = try self.pathKind(at: socketPath) + switch kind { + case .missing: + return + case .socket: + break + case .directory, .symlink, .other: + throw ExecApprovalsSocketPathGuardError.socketPathInvalid(path: socketPath, kind: kind) + } + if unlink(socketPath) != 0, errno != ENOENT { + throw ExecApprovalsSocketPathGuardError.unlinkFailed(path: socketPath, code: errno) + } + } +} + private final class ExecApprovalsSocketServer: @unchecked Sendable { private let logger = Logger(subsystem: "ai.openclaw", category: "exec-approvals.socket") private let socketPath: String @@ -583,7 +683,11 @@ private final class ExecApprovalsSocketServer: @unchecked Sendable { self.socketFD = -1 } if !self.socketPath.isEmpty { - unlink(self.socketPath) + do { + try ExecApprovalsSocketPathGuard.removeExistingSocket(at: self.socketPath) + } catch { + self.logger.warning("exec approvals socket cleanup failed: \(error.localizedDescription, privacy: .public)") + } } } @@ -618,7 +722,14 @@ private final class ExecApprovalsSocketServer: @unchecked Sendable { self.logger.error("exec approvals socket create failed") return -1 } - unlink(self.socketPath) + do { + try ExecApprovalsSocketPathGuard.hardenParentDirectory(for: self.socketPath) + try ExecApprovalsSocketPathGuard.removeExistingSocket(at: self.socketPath) + } catch { + self.logger.error("exec approvals socket path hardening failed: \(error.localizedDescription, privacy: .public)") + close(fd) + return -1 + } var addr = sockaddr_un() addr.sun_family = sa_family_t(AF_UNIX) let maxLen = MemoryLayout.size(ofValue: addr.sun_path) @@ -645,12 +756,18 @@ private final class ExecApprovalsSocketServer: @unchecked Sendable { close(fd) return -1 } + if chmod(self.socketPath, 0o600) != 0 { + self.logger.error("exec approvals socket chmod failed") + close(fd) + try? ExecApprovalsSocketPathGuard.removeExistingSocket(at: self.socketPath) + return -1 + } if listen(fd, 16) != 0 { self.logger.error("exec approvals socket listen failed") close(fd) + try? ExecApprovalsSocketPathGuard.removeExistingSocket(at: self.socketPath) return -1 } - chmod(self.socketPath, 0o600) self.logger.info("exec approvals socket listening at \(self.socketPath, privacy: .public)") return fd } diff --git a/apps/macos/Tests/OpenClawIPCTests/ExecApprovalsSocketPathGuardTests.swift b/apps/macos/Tests/OpenClawIPCTests/ExecApprovalsSocketPathGuardTests.swift new file mode 100644 index 00000000000..64194a0dd97 --- /dev/null +++ b/apps/macos/Tests/OpenClawIPCTests/ExecApprovalsSocketPathGuardTests.swift @@ -0,0 +1,75 @@ +import Foundation +import Testing +@testable import OpenClaw + +@Suite(.serialized) +struct ExecApprovalsSocketPathGuardTests { + @Test + func hardenParentDirectoryCreatesDirectoryWith0700Permissions() throws { + let root = FileManager().temporaryDirectory + .appendingPathComponent("openclaw-socket-guard-\(UUID().uuidString)", isDirectory: true) + defer { try? FileManager().removeItem(at: root) } + let socketPath = root + .appendingPathComponent("nested", isDirectory: true) + .appendingPathComponent("exec-approvals.sock", isDirectory: false) + .path + + try ExecApprovalsSocketPathGuard.hardenParentDirectory(for: socketPath) + + let parent = URL(fileURLWithPath: socketPath).deletingLastPathComponent() + #expect(FileManager().fileExists(atPath: parent.path)) + let attrs = try FileManager().attributesOfItem(atPath: parent.path) + let permissions = (attrs[.posixPermissions] as? NSNumber)?.intValue ?? -1 + #expect(permissions & 0o777 == 0o700) + } + + @Test + func removeExistingSocketRejectsSymlinkPath() throws { + let root = FileManager().temporaryDirectory + .appendingPathComponent("openclaw-socket-guard-\(UUID().uuidString)", isDirectory: true) + defer { try? FileManager().removeItem(at: root) } + try FileManager().createDirectory(at: root, withIntermediateDirectories: true) + + let target = root.appendingPathComponent("target.txt") + _ = FileManager().createFile(atPath: target.path, contents: Data("x".utf8)) + let symlink = root.appendingPathComponent("exec-approvals.sock") + try FileManager().createSymbolicLink(at: symlink, withDestinationURL: target) + + do { + try ExecApprovalsSocketPathGuard.removeExistingSocket(at: symlink.path) + Issue.record("Expected symlink socket path rejection") + } catch let error as ExecApprovalsSocketPathGuardError { + switch error { + case let .socketPathInvalid(path, kind): + #expect(path == symlink.path) + #expect(kind == .symlink) + default: + Issue.record("Unexpected error: \(error)") + } + } + } + + @Test + func removeExistingSocketRejectsRegularFilePath() throws { + let root = FileManager().temporaryDirectory + .appendingPathComponent("openclaw-socket-guard-\(UUID().uuidString)", isDirectory: true) + defer { try? FileManager().removeItem(at: root) } + try FileManager().createDirectory(at: root, withIntermediateDirectories: true) + + let regularFile = root.appendingPathComponent("exec-approvals.sock") + _ = FileManager().createFile(atPath: regularFile.path, contents: Data("x".utf8)) + + do { + try ExecApprovalsSocketPathGuard.removeExistingSocket(at: regularFile.path) + Issue.record("Expected non-socket path rejection") + } catch let error as ExecApprovalsSocketPathGuardError { + switch error { + case let .socketPathInvalid(path, kind): + #expect(path == regularFile.path) + #expect(kind == .other) + default: + Issue.record("Unexpected error: \(error)") + } + } + } +} diff --git a/apps/macos/Tests/OpenClawIPCTests/ExecApprovalsStoreRefactorTests.swift b/apps/macos/Tests/OpenClawIPCTests/ExecApprovalsStoreRefactorTests.swift index fa9eef87881..6363139bb3a 100644 --- a/apps/macos/Tests/OpenClawIPCTests/ExecApprovalsStoreRefactorTests.swift +++ b/apps/macos/Tests/OpenClawIPCTests/ExecApprovalsStoreRefactorTests.swift @@ -64,6 +64,22 @@ struct ExecApprovalsStoreRefactorTests { } } + @Test + func ensureFileHardensStateDirectoryPermissions() async throws { + let stateDir = FileManager().temporaryDirectory + .appendingPathComponent("openclaw-state-\(UUID().uuidString)", isDirectory: true) + defer { try? FileManager().removeItem(at: stateDir) } + try FileManager().createDirectory(at: stateDir, withIntermediateDirectories: true) + try FileManager().setAttributes([.posixPermissions: 0o755], ofItemAtPath: stateDir.path) + + try await TestIsolation.withEnvValues(["OPENCLAW_STATE_DIR": stateDir.path]) { + _ = ExecApprovalsStore.ensureFile() + let attrs = try FileManager().attributesOfItem(atPath: stateDir.path) + let permissions = (attrs[.posixPermissions] as? NSNumber)?.intValue ?? -1 + #expect(permissions & 0o777 == 0o700) + } + } + private static func modificationDate(at url: URL) throws -> Date { let attributes = try FileManager().attributesOfItem(atPath: url.path) guard let date = attributes[.modificationDate] as? Date else {