diff --git a/Sources/MacClean/Core/Scanner/ScanCoordinator.swift b/Sources/MacClean/Core/Scanner/ScanCoordinator.swift index 5706436..4052109 100644 --- a/Sources/MacClean/Core/Scanner/ScanCoordinator.swift +++ b/Sources/MacClean/Core/Scanner/ScanCoordinator.swift @@ -94,7 +94,20 @@ public final class ScanCoordinator: @unchecked Sendable { ) let start = Date() - let scanResults = await module.scan() + let rawResults = await module.scan() + // Drop items the current process couldn't trash even if it + // tried — root-owned children, data-vaulted Apple caches, + // stale paths. Users never see them, never click them, + // never see "X errors" on the completion screen for things + // nothing on this machine could clean anyway. See + // CleanFilter for the syscall-level reasoning. + let scanResults: [ScanResult] = rawResults.map { result in + ScanResult( + category: result.category, + items: result.items.filter { CleanFilter.isCleanableByCurrentProcess($0.url) }, + autoSelect: result.autoSelect + ) + } let duration = Date().timeIntervalSince(start) let moduleResult = ModuleScanResult( diff --git a/Sources/MacCleanKit/CleanFilter.swift b/Sources/MacCleanKit/CleanFilter.swift new file mode 100644 index 0000000..679bab9 --- /dev/null +++ b/Sources/MacCleanKit/CleanFilter.swift @@ -0,0 +1,55 @@ +import Foundation +import Darwin + +/// Drops scan items that our process couldn't trash even if it tried — +/// before they reach the UI. Two categories of un-cleanable items show +/// up routinely on real Macs and used to surface as "X errors" on the +/// completion screen, even though there's nothing the user can do about +/// them: +/// +/// * **Root-owned children** under writable parents. `/Library/Caches/` +/// is writable, but `/Library/Caches/com.apple.InferenceProviderService/` +/// is mode 700 root:wheel — `unlink` from inside it returns EACCES. +/// Likewise `/private/var/log/com.apple.xpc.launchd/` (root-owned), +/// `/Library/Logs/PaloAltoNetworks/...` (third-party root daemons). +/// +/// * **Data-vault dirs** in the user's own home. `~/Library/Caches/` +/// is owned by the user, but `~/Library/Caches/com.apple.containermanagerd/`, +/// `…com.apple.ap.adprivacyd/`, `…com.apple.Safari.SafeBrowsing/` carry +/// the `UF_DATAVAULT` flag — macOS rejects writes to these subtrees +/// regardless of FDA, regardless of ownership, regardless of root. +/// Only Apple-signed daemons can touch them. +/// +/// Both surface as `access(W_OK)` denials at the syscall level (EACCES +/// for POSIX-permission denial, EPERM for data-vault denial), so a single +/// probe covers both. +public enum CleanFilter { + public static func isCleanableByCurrentProcess(_ url: URL) -> Bool { + let path = url.path(percentEncoded: false) + + // `trashItem`/`unlink` is a directory-modification op — it + // requires write permission on the PARENT directory. Files in + // root-owned dirs fail here even if the file is "user-readable". + let parent = (path as NSString).deletingLastPathComponent + guard access(parent, W_OK) == 0 else { return false } + + // If the item doesn't exist any more (stale scan results from a + // previous run, cache churn between scan and clean), drop it + // here instead of surfacing it as a UI row that produces a + // benign skip at clean time. + var isDir: ObjCBool = false + guard FileManager.default.fileExists(atPath: path, isDirectory: &isDir) else { + return false + } + + // Directory deletion has to descend into the tree. This is the + // check that catches `~/Library/Caches/com.apple.*` — the + // parent is writable, but the dir itself carries UF_DATAVAULT + // and macOS rejects writes inside it. + if isDir.boolValue { + guard access(path, W_OK) == 0 else { return false } + } + + return true + } +} diff --git a/Sources/MacCleanKit/Constants.swift b/Sources/MacCleanKit/Constants.swift index b921bbb..9f54d66 100644 --- a/Sources/MacCleanKit/Constants.swift +++ b/Sources/MacCleanKit/Constants.swift @@ -159,5 +159,5 @@ public enum MCConstants { // plugin was tried (commit history) but doesn't work under multi-arch // `swift build --arch arm64 --arch x86_64` because xcbuild doesn't // execute plugins. - public static let appVersion = "1.5.0" + public static let appVersion = "1.5.1" } diff --git a/Tests/MacCleanKitTests/CleanFilterTests.swift b/Tests/MacCleanKitTests/CleanFilterTests.swift new file mode 100644 index 0000000..094bb6a --- /dev/null +++ b/Tests/MacCleanKitTests/CleanFilterTests.swift @@ -0,0 +1,99 @@ +import XCTest +@testable import MacCleanKit + +final class CleanFilterTests: XCTestCase { + private var tmpRoot: URL! + + override func setUpWithError() throws { + try super.setUpWithError() + tmpRoot = URL(fileURLWithPath: NSTemporaryDirectory()) + .appending(path: "CleanFilterTests-\(UUID().uuidString)") + try FileManager.default.createDirectory(at: tmpRoot, withIntermediateDirectories: true) + } + + override func tearDownWithError() throws { + // Restore perms before delete so the test sandbox doesn't leak. + try? FileManager.default.setAttributes( + [.posixPermissions: 0o755], ofItemAtPath: tmpRoot.path) + try? FileManager.default.removeItem(at: tmpRoot) + try super.tearDownWithError() + } + + // MARK: Cleanable cases + + func testFileInUserOwnedWritableDirIsCleanable() throws { + let file = tmpRoot.appending(path: "cache.bin") + FileManager.default.createFile(atPath: file.path, contents: Data([1, 2, 3])) + XCTAssertTrue(CleanFilter.isCleanableByCurrentProcess(file)) + } + + func testWritableDirectoryIsCleanable() throws { + let dir = tmpRoot.appending(path: "subdir") + try FileManager.default.createDirectory(at: dir, withIntermediateDirectories: true) + XCTAssertTrue(CleanFilter.isCleanableByCurrentProcess(dir)) + } + + // MARK: Non-cleanable cases + + func testNonexistentPathIsNotCleanable() { + let ghost = tmpRoot.appending(path: "does-not-exist-\(UUID().uuidString)") + XCTAssertFalse(CleanFilter.isCleanableByCurrentProcess(ghost)) + } + + func testFileWhoseParentIsNotWritableIsNotCleanable() throws { + // Simulates the `/Library/Caches/com.apple.InferenceProviderService/foo.cache` + // case: parent dir exists and the file exists, but we don't have + // write permission on the parent so `unlink` would fail. + let lockedParent = tmpRoot.appending(path: "locked") + try FileManager.default.createDirectory(at: lockedParent, withIntermediateDirectories: true) + let file = lockedParent.appending(path: "trapped.txt") + FileManager.default.createFile(atPath: file.path, contents: Data()) + // Remove write bit on the parent (read+execute only). + try FileManager.default.setAttributes( + [.posixPermissions: 0o555], ofItemAtPath: lockedParent.path) + + XCTAssertFalse(CleanFilter.isCleanableByCurrentProcess(file)) + + // Restore so tearDown can clean up. + try FileManager.default.setAttributes( + [.posixPermissions: 0o755], ofItemAtPath: lockedParent.path) + } + + func testDirectoryWhoseContentsAreNotWritableIsNotCleanable() throws { + // Simulates `~/Library/Caches/com.apple.containermanagerd/`: we own + // the parent, but the directory itself rejects writes. This is the + // closest portable analogue of the data-vault case — we can't + // actually set UF_DATAVAULT from userland, but stripping our own + // write bit on the dir reproduces the syscall-level denial that + // `isCleanableByCurrentProcess` keys on. + let dataVaultLike = tmpRoot.appending(path: "vaulted") + try FileManager.default.createDirectory(at: dataVaultLike, withIntermediateDirectories: true) + try FileManager.default.setAttributes( + [.posixPermissions: 0o555], ofItemAtPath: dataVaultLike.path) + + XCTAssertFalse(CleanFilter.isCleanableByCurrentProcess(dataVaultLike)) + + try FileManager.default.setAttributes( + [.posixPermissions: 0o755], ofItemAtPath: dataVaultLike.path) + } + + func testFilesInsideUnwritableDirectoryAreNotCleanable() throws { + // Even individual files inside an unwritable parent should be + // dropped — this is what catches every leaf inside a root-owned + // junk directory before the user sees them. + let parent = tmpRoot.appending(path: "rootlike") + try FileManager.default.createDirectory(at: parent, withIntermediateDirectories: true) + let leaf1 = parent.appending(path: "a.log") + let leaf2 = parent.appending(path: "b.log") + FileManager.default.createFile(atPath: leaf1.path, contents: Data()) + FileManager.default.createFile(atPath: leaf2.path, contents: Data()) + try FileManager.default.setAttributes( + [.posixPermissions: 0o555], ofItemAtPath: parent.path) + + XCTAssertFalse(CleanFilter.isCleanableByCurrentProcess(leaf1)) + XCTAssertFalse(CleanFilter.isCleanableByCurrentProcess(leaf2)) + + try FileManager.default.setAttributes( + [.posixPermissions: 0o755], ofItemAtPath: parent.path) + } +} diff --git a/Tests/MacCleanTests/ScanCoordinatorTests.swift b/Tests/MacCleanTests/ScanCoordinatorTests.swift index ff0d36c..71e6a3c 100644 --- a/Tests/MacCleanTests/ScanCoordinatorTests.swift +++ b/Tests/MacCleanTests/ScanCoordinatorTests.swift @@ -8,6 +8,18 @@ import MacCleanKit @MainActor final class ScanCoordinatorTests: XCTestCase { + // Per-test sandbox. The classic setUpWithError/tearDownWithError + // approach trips Swift 6 strict concurrency on the CI macos-15 + // runner — XCTestCase declares those as nonisolated, but reaching + // an @MainActor stored property from them violates isolation. We + // create+destroy the tmp dir inline per test instead. + private static func makeTmpRoot(for name: String = #function) throws -> URL { + let root = URL(fileURLWithPath: NSTemporaryDirectory()) + .appending(path: "ScanCoordinatorTests-\(name)-\(UUID().uuidString)") + try FileManager.default.createDirectory(at: root, withIntermediateDirectories: true) + return root + } + // MARK: - Fake modules struct FakeModule: ScanModule { @@ -34,11 +46,19 @@ final class ScanCoordinatorTests: XCTestCase { } } - private func makeItems(count: Int, eachSize: UInt64) -> [FileItem] { - (0.. [FileItem] { + (0..