Skip to content
Merged
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
15 changes: 14 additions & 1 deletion Sources/MacClean/Core/Scanner/ScanCoordinator.swift
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down
55 changes: 55 additions & 0 deletions Sources/MacCleanKit/CleanFilter.swift
Original file line number Diff line number Diff line change
@@ -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
}
}
2 changes: 1 addition & 1 deletion Sources/MacCleanKit/Constants.swift
Original file line number Diff line number Diff line change
Expand Up @@ -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"
}
99 changes: 99 additions & 0 deletions Tests/MacCleanKitTests/CleanFilterTests.swift
Original file line number Diff line number Diff line change
@@ -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)
}
}
46 changes: 35 additions & 11 deletions Tests/MacCleanTests/ScanCoordinatorTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -34,11 +46,19 @@ final class ScanCoordinatorTests: XCTestCase {
}
}

private func makeItems(count: Int, eachSize: UInt64) -> [FileItem] {
(0..<count).map {
FileItem(
url: URL(filePath: "/tmp/f\($0)"),
name: "f\($0)", size: eachSize, allocatedSize: eachSize, isDirectory: false
/// 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, inRoot: URL) -> [FileItem] {
(0..<count).map { idx in
let url = inRoot.appending(path: "f\(idx)")
FileManager.default.createFile(
atPath: url.path,
contents: Data(count: Int(eachSize))
)
return FileItem(
url: url,
name: "f\(idx)", size: eachSize, allocatedSize: eachSize, isDirectory: false
)
}
}
Expand All @@ -52,18 +72,20 @@ final class ScanCoordinatorTests: XCTestCase {
}
}

func testScanAllCompletesAndAggregates() async {
func testScanAllCompletesAndAggregates() async throws {
let tmpRoot = try Self.makeTmpRoot()
defer { try? FileManager.default.removeItem(at: tmpRoot) }
let c = ScanCoordinator()
c.registerModules([
FakeModule(
id: "a", name: "A",
result: [ScanResult(category: .userCaches,
items: makeItems(count: 5, eachSize: 100))]
items: makeItems(count: 5, eachSize: 100, inRoot: tmpRoot))]
),
FakeModule(
id: "b", name: "B",
result: [ScanResult(category: .userLogs,
items: makeItems(count: 3, eachSize: 200))]
items: makeItems(count: 3, eachSize: 200, inRoot: tmpRoot))]
),
])
c.scanAll()
Expand All @@ -82,17 +104,19 @@ final class ScanCoordinatorTests: XCTestCase {
XCTAssertEqual(c.totalSizeFound, 5*100 + 3*200)
}

func testScanAllExcludesHeavyModules() async {
func testScanAllExcludesHeavyModules() async throws {
let tmpRoot = try Self.makeTmpRoot()
defer { try? FileManager.default.removeItem(at: tmpRoot) }
let c = ScanCoordinator()
c.registerModules([
FakeModule(
id: "light", name: "Light",
result: [ScanResult(category: .userCaches, items: makeItems(count: 1, eachSize: 1))]
result: [ScanResult(category: .userCaches, items: makeItems(count: 1, eachSize: 1, inRoot: tmpRoot))]
),
FakeModule(
id: "heavy", name: "Heavy",
includedInSmartScan: false,
result: [ScanResult(category: .duplicates, items: makeItems(count: 100, eachSize: 1))]
result: [ScanResult(category: .duplicates, items: makeItems(count: 100, eachSize: 1, inRoot: tmpRoot))]
),
])
c.scanAll()
Expand Down
20 changes: 17 additions & 3 deletions Tests/MacCleanTests/SmartScanE2ETests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -10,14 +10,28 @@ import MacCleanTestSupport
@MainActor
final class SmartScanE2ETests: XCTestCase {

func testCoordinatorAggregatesAllRegisteredModules() async {
func testCoordinatorAggregatesAllRegisteredModules() async throws {
// Real files so the CleanFilter (which drops un-cleanable
// paths) keeps them in the aggregated results. The previous
// version used `/tmp/a.cache` / `/tmp/b.log` literally — those
// didn't exist, so the filter correctly dropped them and the
// aggregation assertions failed for the wrong reason.
let dir = URL(fileURLWithPath: NSTemporaryDirectory())
.appending(path: "SmartScanE2E-\(UUID().uuidString)")
try FileManager.default.createDirectory(at: dir, withIntermediateDirectories: true)
defer { try? FileManager.default.removeItem(at: dir) }
let aURL = dir.appending(path: "a.cache")
let bURL = dir.appending(path: "b.log")
FileManager.default.createFile(atPath: aURL.path, contents: Data(count: 100))
FileManager.default.createFile(atPath: bURL.path, contents: Data(count: 200))

let coord = ScanCoordinator()
coord.registerModules([
ScanCoordinatorTests.FakeModule(
id: "a", name: "A",
result: [ScanResult(
category: .userCaches,
items: [FileItem(url: URL(filePath: "/tmp/a.cache"),
items: [FileItem(url: aURL,
name: "a.cache", size: 100, allocatedSize: 100,
isDirectory: false)]
)]
Expand All @@ -26,7 +40,7 @@ final class SmartScanE2ETests: XCTestCase {
id: "b", name: "B",
result: [ScanResult(
category: .userLogs,
items: [FileItem(url: URL(filePath: "/tmp/b.log"),
items: [FileItem(url: bURL,
name: "b.log", size: 200, allocatedSize: 200,
isDirectory: false)]
)]
Expand Down
2 changes: 1 addition & 1 deletion VERSION
Original file line number Diff line number Diff line change
@@ -1 +1 @@
1.5.0
1.5.1
Loading