From af8071bf04ba2ecbb846e5be316b0a52f1fcf6cb Mon Sep 17 00:00:00 2001 From: Chris George Date: Fri, 8 May 2026 12:58:02 -0700 Subject: [PATCH] Use SystemPath for PacketFilter. (#1518) - Closes #1517. - Using URL for filesystem paths is bad practice. FilePath is safer and more ergonomic. - Sibling of #1480 (HostDNSResolver), same pattern. --- .../Client/PacketFilter.swift | 61 ++++++++++--------- .../PacketFilterTest.swift | 18 +++--- 2 files changed, 41 insertions(+), 38 deletions(-) diff --git a/Sources/Services/ContainerAPIService/Client/PacketFilter.swift b/Sources/Services/ContainerAPIService/Client/PacketFilter.swift index 86feb1b0..bd4035d2 100644 --- a/Sources/Services/ContainerAPIService/Client/PacketFilter.swift +++ b/Sources/Services/ContainerAPIService/Client/PacketFilter.swift @@ -18,18 +18,19 @@ import ContainerizationError import ContainerizationExtras import DNSServer import Foundation +import SystemPackage public struct PacketFilter { public static let anchor = "com.apple.container" - public static let defaultConfigPath = URL(filePath: "/etc/pf.conf") - public static let defaultAnchorsPath = URL(filePath: "/etc/pf.anchors") + public static let defaultConfigPath = FilePath("/etc/pf.conf") + public static let defaultAnchorsPath = FilePath("/etc/pf.anchors") - private let configURL: URL - private let anchorsURL: URL + private let configPath: FilePath + private let anchorsPath: FilePath - public init(configURL: URL = Self.defaultConfigPath, anchorsURL: URL = Self.defaultAnchorsPath) { - self.configURL = configURL - self.anchorsURL = anchorsURL + public init(configPath: FilePath = Self.defaultConfigPath, anchorsPath: FilePath = Self.defaultAnchorsPath) { + self.configPath = configPath + self.anchorsPath = anchorsPath } public func createRedirectRule(from: IPAddress, to: IPAddress, domain: DNSName) throws { @@ -39,7 +40,7 @@ public struct PacketFilter { let fm: FileManager = FileManager.default - let anchorURL = self.anchorsURL.appending(path: Self.anchor) + let anchorPath = self.anchorsPath.appending(Self.anchor) let inet: String switch from { @@ -49,8 +50,8 @@ public struct PacketFilter { let redirectRule = "rdr \(inet) from any to \(from.description) -> \(to.description) # \(domain.pqdn)" var content = "" - if fm.fileExists(atPath: anchorURL.path) { - content = try String(contentsOfFile: anchorURL.path, encoding: .utf8) + if fm.fileExists(atPath: anchorPath.string) { + content = try String(contentsOfFile: anchorPath.string, encoding: .utf8) } else { try addAnchorToConfig() } @@ -60,7 +61,7 @@ public struct PacketFilter { lines.insert(redirectRule, at: lines.endIndex - 1) } - try lines.joined(separator: "\n").write(toFile: anchorURL.path, atomically: true, encoding: .utf8) + try lines.joined(separator: "\n").write(toFile: anchorPath.string, atomically: true, encoding: .utf8) } public func removeRedirectRule(from: IPAddress, to: IPAddress, domain: DNSName) throws { @@ -70,7 +71,7 @@ public struct PacketFilter { let fm: FileManager = FileManager.default - let anchorURL = self.anchorsURL.appending(path: Self.anchor) + let anchorPath = self.anchorsPath.appending(Self.anchor) let inet: String switch from { @@ -79,11 +80,11 @@ public struct PacketFilter { } let redirectRule = "rdr \(inet) from any to \(from.description) -> \(to.description) # \(domain.pqdn)" - guard fm.fileExists(atPath: anchorURL.path) else { + guard fm.fileExists(atPath: anchorPath.string) else { return } - let content = try String(contentsOfFile: anchorURL.path, encoding: .utf8) + let content = try String(contentsOfFile: anchorPath.string, encoding: .utf8) let lines = content.components(separatedBy: .newlines) let removedLines = lines.filter { l in @@ -91,28 +92,28 @@ public struct PacketFilter { } if removedLines == [""] { - try fm.removeItem(atPath: anchorURL.path) + try fm.removeItem(atPath: anchorPath.string) try removeAnchorFromConfig() } else { - try removedLines.joined(separator: "\n").write(toFile: anchorURL.path, atomically: true, encoding: .utf8) + try removedLines.joined(separator: "\n").write(toFile: anchorPath.string, atomically: true, encoding: .utf8) } } private func addAnchorToConfig() throws { let fm: FileManager = FileManager.default - let anchorURL = self.anchorsURL.appending(path: Self.anchor) + let anchorPath = self.anchorsPath.appending(Self.anchor) /* PF requires strict ordering of anchors: scrub-anchor, nat-anchor, rdr-anchor, dummynet-anchor, anchor, load anchor */ let anchorKeywords = ["scrub-anchor", "nat-anchor", "rdr-anchor", "dummynet-anchor", "anchor", "load anchor"] - let loadAnchorText = "load anchor \"\(Self.anchor)\" from \"\(anchorURL.path)\"" + let loadAnchorText = "load anchor \"\(Self.anchor)\" from \"\(anchorPath.string)\"" var content: String = "" var lines: [String] = [] - if fm.fileExists(atPath: self.configURL.path) { - content = try String(contentsOfFile: self.configURL.path, encoding: .utf8) + if fm.fileExists(atPath: self.configPath.string) { + content = try String(contentsOfFile: self.configPath.string, encoding: .utf8) } lines = content.components(separatedBy: .newlines) @@ -134,28 +135,28 @@ public struct PacketFilter { } do { - try lines.joined(separator: "\n").write(toFile: self.configURL.path, atomically: true, encoding: .utf8) + try lines.joined(separator: "\n").write(toFile: self.configPath.string, atomically: true, encoding: .utf8) } catch { - throw ContainerizationError(.invalidState, message: "failed to write \"\(self.configURL.path)\"") + throw ContainerizationError(.invalidState, message: "failed to write \"\(self.configPath.string)\"") } } private func removeAnchorFromConfig() throws { let fm: FileManager = FileManager.default - guard fm.fileExists(atPath: configURL.path) else { + guard fm.fileExists(atPath: configPath.string) else { return } - let content = try String(contentsOfFile: configURL.path, encoding: .utf8) + let content = try String(contentsOfFile: configPath.string, encoding: .utf8) let lines = content.components(separatedBy: .newlines) let removedLines = lines.filter { l in !l.contains(Self.anchor) } do { - try removedLines.joined(separator: "\n").write(toFile: configURL.path, atomically: true, encoding: .utf8) + try removedLines.joined(separator: "\n").write(toFile: configPath.string, atomically: true, encoding: .utf8) } catch { - throw ContainerizationError(.invalidState, message: "failed to write \"\(configURL.path)\"") + throw ContainerizationError(.invalidState, message: "failed to write \"\(configPath.string)\"") } } @@ -165,7 +166,7 @@ public struct PacketFilter { let checkProcess = Foundation.Process() var checkStatus: Int32 checkProcess.executableURL = URL(fileURLWithPath: "/sbin/pfctl") - checkProcess.arguments = ["-n", "-f", configURL.path] + checkProcess.arguments = ["-n", "-f", configPath.string] checkProcess.standardOutput = null checkProcess.standardError = null @@ -178,14 +179,14 @@ public struct PacketFilter { checkProcess.waitUntilExit() checkStatus = checkProcess.terminationStatus guard checkStatus == 0 else { - throw ContainerizationError(.internalError, message: "invalid pf config \"\(configURL.path)\"") + throw ContainerizationError(.internalError, message: "invalid pf config \"\(configPath.string)\"") } let reloadProcess = Foundation.Process() var reloadStatus: Int32 reloadProcess.executableURL = URL(fileURLWithPath: "/sbin/pfctl") - reloadProcess.arguments = ["-f", configURL.path] + reloadProcess.arguments = ["-f", configPath.string] reloadProcess.standardOutput = null reloadProcess.standardError = null @@ -197,7 +198,7 @@ public struct PacketFilter { reloadProcess.waitUntilExit() reloadStatus = reloadProcess.terminationStatus guard reloadStatus == 0 else { - throw ContainerizationError(.invalidState, message: "pfctl -f \"\(configURL.path)\" failed with status \(reloadStatus)") + throw ContainerizationError(.invalidState, message: "pfctl -f \"\(configPath.string)\" failed with status \(reloadStatus)") } } } diff --git a/Tests/ContainerAPIClientTests/PacketFilterTest.swift b/Tests/ContainerAPIClientTests/PacketFilterTest.swift index 79dfe0a0..48cf049a 100644 --- a/Tests/ContainerAPIClientTests/PacketFilterTest.swift +++ b/Tests/ContainerAPIClientTests/PacketFilterTest.swift @@ -18,6 +18,7 @@ import ContainerizationError import ContainerizationExtras import DNSServer import Foundation +import SystemPackage import Testing @testable import ContainerAPIClient @@ -32,17 +33,18 @@ struct PacketFilterTest { appropriateFor: .temporaryDirectory, create: true ) + let tempPath = FilePath(tempURL.path) defer { try? FileManager.default.removeItem(at: tempURL) } - let configURL = tempURL.appending(path: "pf.conf") + let configPath = tempPath.appending("pf.conf") - let pf = PacketFilter(configURL: configURL, anchorsURL: tempURL) + let pf = PacketFilter(configPath: configPath, anchorsPath: tempPath) let from1 = try! IPAddress("203.0.113.113") let domain1 = try! DNSName("aaa.com") let to = try! IPAddress("127.0.0.1") try pf.createRedirectRule(from: from1, to: to, domain: domain1) - let anchorURL = tempURL.appending(path: "com.apple.container") - var actualAnchorText = try String(contentsOf: anchorURL, encoding: .utf8) + let anchorPath = tempPath.appending("com.apple.container") + var actualAnchorText = try String(contentsOfFile: anchorPath.string, encoding: .utf8) var expectedAnchorTest = """ rdr inet from any to \(from1) -> \(to) # \(domain1.pqdn)\n """ @@ -53,13 +55,13 @@ struct PacketFilterTest { let domain2 = try! DNSName("bbb.com") try pf.createRedirectRule(from: from2, to: to, domain: domain2) - actualAnchorText = try String(contentsOf: anchorURL, encoding: .utf8) + actualAnchorText = try String(contentsOfFile: anchorPath.string, encoding: .utf8) expectedAnchorTest += """ rdr inet from any to \(from2) -> \(to) # \(domain2.pqdn)\n """ #expect(actualAnchorText == expectedAnchorTest) - let actualConfigText = try String(contentsOf: configURL, encoding: .utf8) + let actualConfigText = try String(contentsOfFile: configPath.string, encoding: .utf8) let expectedConfigText = try Regex( #""" scrub-anchor "([^"]+)" @@ -76,8 +78,8 @@ struct PacketFilterTest { try pf.removeRedirectRule(from: from1, to: to, domain: domain1) try pf.removeRedirectRule(from: from2, to: to, domain: domain2) - #expect(!fm.fileExists(atPath: anchorURL.path)) - let configText = try String(contentsOf: configURL, encoding: .utf8) + #expect(!fm.fileExists(atPath: anchorPath.string)) + let configText = try String(contentsOfFile: configPath.string, encoding: .utf8) #expect(configText == "") }