From 24a52c6fdddb0f700bed6aabba2991429959c37e Mon Sep 17 00:00:00 2001 From: Iliya Date: Mon, 1 Jun 2026 03:46:26 -0500 Subject: [PATCH 1/4] Add CleanFilter: drop items the current process couldn't trash MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Pure access(W_OK)-based probe in MacCleanKit. An item is cleanable iff: 1. Its parent directory is writable by us (unlink/trash is a directory-modification syscall; permission lives on the parent). 2. If the item is a directory, the directory itself is also writable (recursive descent needs to modify contents). 3. The path still exists. This single probe catches two distinct macOS realities that both surface as 'you don't have permission' errors today: - Root-owned children of writable system dirs (/Library/Caches/*, /Library/Logs/*, /private/var/log/*) — POSIX denies via EACCES. - Data-vaulted Apple caches in the user's own home (~/Library/Caches/com.apple.containermanagerd/, …ap.adprivacyd/, …Safari.SafeBrowsing/) — kernel denies via EPERM (UF_DATAVAULT) regardless of FDA, ownership, or root. 6 spec tests cover the contract using POSIX permission stripping to simulate the data-vault case (the actual UF_DATAVAULT flag can't be set from userland). --- Sources/MacCleanKit/CleanFilter.swift | 55 +++++++++++ Tests/MacCleanKitTests/CleanFilterTests.swift | 99 +++++++++++++++++++ 2 files changed, 154 insertions(+) create mode 100644 Sources/MacCleanKit/CleanFilter.swift create mode 100644 Tests/MacCleanKitTests/CleanFilterTests.swift 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/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) + } +} From b51929aeb199a7bfad3f3cef63ebd41a25fe99c1 Mon Sep 17 00:00:00 2001 From: Iliya Date: Mon, 1 Jun 2026 03:46:38 -0500 Subject: [PATCH 2/4] Wire CleanFilter into ScanCoordinator: drop un-cleanable items before UI MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Single insertion point in scanModules(): every module's results flow through here, so every result list gets filtered exactly once. Items the current process couldn't trash never reach the ViewModel, never render in the file list, never count toward selectable bytes, never get queued for clean. Users no longer see 'X errors' on the completion screen for items nothing on this machine could clean. This intentionally affects every module — including Optimization / Maintenance / Updater whose items aren't trashed via FileManager. Items in those modules don't typically live under root-owned dirs in practice, but if they do, surfacing them in the UI without an actionable path was already misleading. Two ScanCoordinator/SmartScan tests previously used non-existent fake URLs (/tmp/f0, /tmp/a.cache); the filter correctly dropped them, breaking aggregation assertions. Updated to materialize real files in per-test temp dirs so the aggregation contract is tested against actual cleanable inputs. --- .../Core/Scanner/ScanCoordinator.swift | 15 +++++++++- .../MacCleanTests/ScanCoordinatorTests.swift | 30 ++++++++++++++++--- Tests/MacCleanTests/SmartScanE2ETests.swift | 20 +++++++++++-- 3 files changed, 57 insertions(+), 8 deletions(-) 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/Tests/MacCleanTests/ScanCoordinatorTests.swift b/Tests/MacCleanTests/ScanCoordinatorTests.swift index ff0d36c..df77a18 100644 --- a/Tests/MacCleanTests/ScanCoordinatorTests.swift +++ b/Tests/MacCleanTests/ScanCoordinatorTests.swift @@ -8,6 +8,20 @@ import MacCleanKit @MainActor final class ScanCoordinatorTests: XCTestCase { + private var tmpRoot: URL! + + override func setUpWithError() throws { + try super.setUpWithError() + tmpRoot = URL(fileURLWithPath: NSTemporaryDirectory()) + .appending(path: "ScanCoordinatorTests-\(UUID().uuidString)") + try FileManager.default.createDirectory(at: tmpRoot, withIntermediateDirectories: true) + } + + override func tearDownWithError() throws { + try? FileManager.default.removeItem(at: tmpRoot) + try super.tearDownWithError() + } + // MARK: - Fake modules struct FakeModule: ScanModule { @@ -34,11 +48,19 @@ final class ScanCoordinatorTests: XCTestCase { } } + /// Materializes real files of the requested size in `tmpRoot` so the + /// CleanFilter (which drops non-existent / non-writable paths) keeps + /// them in the aggregated results. private func makeItems(count: Int, eachSize: UInt64) -> [FileItem] { - (0.. Date: Mon, 1 Jun 2026 03:46:46 -0500 Subject: [PATCH 3/4] =?UTF-8?q?Bump=201.5.0=20=E2=86=92=201.5.1:=20drop=20?= =?UTF-8?q?un-cleanable=20items=20at=20scan=20time?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Patch bump. UX fix: the completion screen no longer lists 'X errors' for items the current process was never going to be able to trash in the first place — root-owned children of /Library/Caches/, /Library/Logs/, /private/var/log/, and macOS data-vaulted Apple caches under ~/Library/Caches/com.apple.*. Items disappear from the scan results upfront via a single access(W_OK) probe in ScanCoordinator. --- Sources/MacCleanKit/Constants.swift | 2 +- VERSION | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) 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/VERSION b/VERSION index bc80560..26ca594 100644 --- a/VERSION +++ b/VERSION @@ -1 +1 @@ -1.5.0 +1.5.1 From 76319eb5bd85ac2fb61d6c30c68ad0341a99a5fe Mon Sep 17 00:00:00 2001 From: Iliya Date: Mon, 1 Jun 2026 03:54:17 -0500 Subject: [PATCH 4/4] Move per-test tmp dirs inline: avoid Swift 6 strict concurrency violation CI macos-15 runner hit: error: main actor-isolated property 'tmpRoot' can not be mutated from a nonisolated context XCTestCase declares setUp(WithError) as nonisolated; an @MainActor subclass can't mutate its own @MainActor stored property from those inherited entry points without an explicit hop. Sidestep by creating the per-test sandbox inside each test (which IS @MainActor-isolated in this class) via a static factory + defer for cleanup. No class- level mutable state means no actor mismatch to resolve. Local Swift 6.3.2 happened to let this through; the CI toolchain catches it. Now both pass. --- .../MacCleanTests/ScanCoordinatorTests.swift | 44 ++++++++++--------- 1 file changed, 23 insertions(+), 21 deletions(-) diff --git a/Tests/MacCleanTests/ScanCoordinatorTests.swift b/Tests/MacCleanTests/ScanCoordinatorTests.swift index df77a18..71e6a3c 100644 --- a/Tests/MacCleanTests/ScanCoordinatorTests.swift +++ b/Tests/MacCleanTests/ScanCoordinatorTests.swift @@ -8,18 +8,16 @@ import MacCleanKit @MainActor final class ScanCoordinatorTests: XCTestCase { - private var tmpRoot: URL! - - override func setUpWithError() throws { - try super.setUpWithError() - tmpRoot = URL(fileURLWithPath: NSTemporaryDirectory()) - .appending(path: "ScanCoordinatorTests-\(UUID().uuidString)") - try FileManager.default.createDirectory(at: tmpRoot, withIntermediateDirectories: true) - } - - override func tearDownWithError() throws { - try? FileManager.default.removeItem(at: tmpRoot) - try super.tearDownWithError() + // 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 @@ -48,12 +46,12 @@ final class ScanCoordinatorTests: XCTestCase { } } - /// Materializes real files of the requested size in `tmpRoot` so the + /// Materializes real files of the requested size in `inRoot` so the /// CleanFilter (which drops non-existent / non-writable paths) keeps /// them in the aggregated results. - private func makeItems(count: Int, eachSize: UInt64) -> [FileItem] { + private func makeItems(count: Int, eachSize: UInt64, inRoot: URL) -> [FileItem] { (0..