ReviewOS

also looking at this

stacks/system-cleaner

feat(orphans): scan Application Support and Containers

#29
Merged glennmichael123 wants to merge feat/orphan-scan-application-support into main
3 files +242 -19

Review threads live on the whole diff, not on one commit, so none are shown here - a thread's line means something in the branch's final form, and painting it into an intermediate step would put it on code it is not about.

packages/clean/src/orphans.tsmodified+171-19
Changes to packages/clean/src/orphans.ts
@@ -4,9 +4,16 @@ import { exec, macPaths, pathExists, safeReadDir, safeStat, shellEscape, readApp
44export interface OrphanedItem {
55 path: string
66 bundleId: string
7 type: 'cache' | 'log' | 'saved-state' | 'webkit' | 'http-storage' | 'cookie'
7 type: 'cache' | 'log' | 'saved-state' | 'webkit' | 'http-storage' | 'cookie' | 'app-support' | 'container'
88 sizeBytes: number
99 daysSinceModified: number
10 /**
11 * True for roots that hold real user documents rather than rebuildable
12 * scratch. Callers must not auto-select these — a stale cache is free to
13 * delete, an app's Application Support directory may be the only copy of
14 * something the user cannot regenerate.
15 */
16 holdsUserData: boolean
1017}
1118
1219/** Bundle ID patterns that should NEVER be cleaned as orphans */
@@ -16,27 +23,118 @@ const NEVER_DELETE = [
1623 /com\.apple\./i, /\bloginwindow\b/i, /\bdock\b/i, /\bfinder\b/i, /\bsafari\b/i,
1724]
1825
26/**
27 * Plain-named directories under Application Support that belong to macOS
28 * itself, not to a third-party app. None of these has a matching `.app`, so
29 * any "is the owning app installed?" test reports them as orphaned — and
30 * `MobileSync` alone can be every iPhone backup on the machine.
31 *
32 * Only consulted for the plain-name path; reverse-DNS entries are already
33 * covered by the `com.apple.` rule in NEVER_DELETE.
34 */
35export const SYSTEM_DIR_NAMES = new Set([
36 'addressbook', 'animoji', 'callhistorydb', 'callhistorytransactions', 'clouddocs',
37 'crashreporter', 'differentialprivacy', 'diskimages', 'dock', 'facetime',
38 'fileprovider', 'icloud', 'knowledge', 'mobilesync', 'music', 'sesstorage',
39 'syncservices', 'caches', 'com.apple',
40])
41
42/**
43 * Plain-named Application Support directories mapped to the apps that own
44 * them. Required because the directory name routinely is not the app name:
45 * Chrome writes to `Google`, Firefox to `Mozilla`, Brave to `BraveSoftware`.
46 *
47 * This map is deliberately the *only* source of plain-name orphans. The
48 * obvious alternative — flag any directory whose name matches no installed
49 * app — is unsafe: on the machine this was developed against, `Google` held
50 * 7.7 GB of live Chrome profile (history, bookmarks, saved passwords) and
51 * would have been reported as orphaned data from an uninstalled app.
52 *
53 * An unrecognised directory is therefore never flagged. False negatives cost
54 * disk space; false positives cost the user their data.
55 */
56export const KNOWN_APP_DIRS: Record<string, string[]> = {
57 'Arc': ['Arc'],
58 'BraveSoftware': ['Brave Browser', 'Brave Browser Beta', 'Brave Browser Nightly'],
59 'Chromium': ['Chromium'],
60 'Code': ['Visual Studio Code', 'Code'],
61 'Cursor': ['Cursor'],
62 'Discord': ['Discord'],
63 'Firefox': ['Firefox'],
64 'GitHub Desktop': ['GitHub Desktop'],
65 'Google': ['Google Chrome', 'Google Chrome Canary', 'Google Chrome Beta', 'Google Drive'],
66 'JetBrains': ['IntelliJ IDEA', 'WebStorm', 'PyCharm', 'GoLand', 'RubyMine', 'CLion', 'PhpStorm', 'Rider', 'DataGrip'],
67 'Microsoft': ['Microsoft Edge', 'Microsoft Word', 'Microsoft Excel', 'Microsoft PowerPoint', 'Microsoft Outlook', 'Microsoft Teams', 'OneDrive'],
68 'Mozilla': ['Firefox', 'Firefox Developer Edition', 'Thunderbird'],
69 'Notion': ['Notion'],
70 'Postman': ['Postman'],
71 'Slack': ['Slack'],
72 'Steam': ['Steam'],
73 'Sublime Text': ['Sublime Text'],
74 'TeamViewer': ['TeamViewer'],
75 'Zoom': ['zoom.us'],
76}
77
1978/** Case-insensitive pattern to match reverse-DNS bundle IDs */
2079const BUNDLE_ID_PATTERN = /^(com|org|net|io)\.[a-z]/i
2180
22const ORPHAN_DIRS: { basePath: string, type: OrphanedItem['type'], pattern: RegExp }[] = [
23 { basePath: macPaths.caches, type: 'cache', pattern: BUNDLE_ID_PATTERN },
24 { basePath: macPaths.logs, type: 'log', pattern: BUNDLE_ID_PATTERN },
25 { basePath: macPaths.savedState, type: 'saved-state', pattern: /\.savedState$/i },
26 { basePath: macPaths.webkit, type: 'webkit', pattern: BUNDLE_ID_PATTERN },
27 { basePath: macPaths.httpStorages, type: 'http-storage', pattern: BUNDLE_ID_PATTERN },
28]
81/**
82 * How entries in a root identify their owning app. `bundle-id` names are
83 * unambiguous and resolve against installed bundle identifiers; `app-name`
84 * names resolve through KNOWN_APP_DIRS against installed app names.
85 */
86type NameKind = 'bundle-id' | 'app-name'
87
88interface OrphanDir {
89 basePath: string
90 type: OrphanedItem['type']
91 pattern: RegExp
92 nameKind: NameKind
93 /** Age below which an entry in this root is never reported. */
94 minAgeDays: number
95 holdsUserData: boolean
96}
2997
3098const AGE_THRESHOLD_DAYS = 30
3199
100/**
101 * Application Support and Containers get a longer threshold than caches do.
102 * A cache untouched for a month is dead; an app's documents untouched for a
103 * month may just be an app the user opens quarterly.
104 */
105const USER_DATA_AGE_THRESHOLD_DAYS = 90
106
107const ORPHAN_DIRS: OrphanDir[] = [
108 { basePath: macPaths.caches, type: 'cache', pattern: BUNDLE_ID_PATTERN, nameKind: 'bundle-id', minAgeDays: AGE_THRESHOLD_DAYS, holdsUserData: false },
109 { basePath: macPaths.logs, type: 'log', pattern: BUNDLE_ID_PATTERN, nameKind: 'bundle-id', minAgeDays: AGE_THRESHOLD_DAYS, holdsUserData: false },
110 { basePath: macPaths.savedState, type: 'saved-state', pattern: /\.savedState$/i, nameKind: 'bundle-id', minAgeDays: AGE_THRESHOLD_DAYS, holdsUserData: false },
111 { basePath: macPaths.webkit, type: 'webkit', pattern: BUNDLE_ID_PATTERN, nameKind: 'bundle-id', minAgeDays: AGE_THRESHOLD_DAYS, holdsUserData: false },
112 { basePath: macPaths.httpStorages, type: 'http-storage', pattern: BUNDLE_ID_PATTERN, nameKind: 'bundle-id', minAgeDays: AGE_THRESHOLD_DAYS, holdsUserData: false },
113
114 // Where an uninstalled app leaves gigabytes rather than megabytes. Scanned
115 // twice: once for the reverse-DNS directories, once for the plain-named
116 // ones KNOWN_APP_DIRS can attribute.
117 { basePath: macPaths.applicationSupport, type: 'app-support', pattern: BUNDLE_ID_PATTERN, nameKind: 'bundle-id', minAgeDays: USER_DATA_AGE_THRESHOLD_DAYS, holdsUserData: true },
118 { basePath: macPaths.applicationSupport, type: 'app-support', pattern: /^[A-Z][\w .+-]*$/i, nameKind: 'app-name', minAgeDays: USER_DATA_AGE_THRESHOLD_DAYS, holdsUserData: true },
119
120 // Sandboxed apps. Always bundle-id named, so unambiguous.
121 { basePath: macPaths.containers, type: 'container', pattern: BUNDLE_ID_PATTERN, nameKind: 'bundle-id', minAgeDays: USER_DATA_AGE_THRESHOLD_DAYS, holdsUserData: true },
122
123 // Group Containers are deliberately omitted: they are prefixed with a team
124 // identifier (`9BNSXJN65R.com.example.app`) and shared between several apps
125 // of one vendor, so "the owning app is gone" is not a question a single
126 // directory name can answer.
127]
128
32129/**
33130 * Find orphaned app data — caches, logs, saved states for apps no longer installed.
34131 * Uses shellEscape to prevent command injection from bundle IDs.
35132 */
36133export async function findOrphanedAppData(): Promise<OrphanedItem[]> {
37 const installedBundles = await getInstalledBundleIds()
134 const installed = await getInstalledApps()
38135 const now = Date.now()
39136 const orphans: OrphanedItem[] = []
137 const seen = new Set<string>()
40138
41139 for (const dir of ORPHAN_DIRS) {
42140 if (!pathExists(dir.basePath))
@@ -52,26 +150,62 @@ export async function findOrphanedAppData(): Promise<OrphanedItem[]> {
52150 if (NEVER_DELETE.some(p => p.test(bundleId)))
53151 continue
54152
55 // Skip if app is installed
56 if (installedBundles.has(bundleId) || installedBundles.has(bundleId.toLowerCase()))
57 continue
153 if (dir.nameKind === 'bundle-id') {
154 // Skip if app is installed
155 if (installed.bundleIds.has(bundleId) || installed.bundleIds.has(bundleId.toLowerCase()))
156 continue
157 }
158 else {
159 // If the app index came back empty — Spotlight disabled, mdfind
160 // missing — then *every* mapped directory reads as uninstalled.
161 // Refuse to answer rather than answer wrongly: the bundle-id roots
162 // have NEVER_DELETE and `com.apple.` to fall back on, this path has
163 // nothing but the index.
164 if (installed.appNames.size === 0)
165 continue
166
167 // Plain name. macOS's own directories first — none of them has an
168 // owning `.app`, so every one would otherwise read as orphaned.
169 if (SYSTEM_DIR_NAMES.has(bundleId.toLowerCase()))
170 continue
171
172 // Only directories we can actually attribute. An unknown name is
173 // left alone rather than guessed at; see KNOWN_APP_DIRS.
174 const owners = KNOWN_APP_DIRS[bundleId]
175 if (!owners)
176 continue
177
178 if (owners.some(name => installed.appNames.has(name.toLowerCase())))
179 continue
180 }
58181
59182 const fullPath = path.join(dir.basePath, entry)
183 if (seen.has(fullPath))
184 continue
185
60186 const stat = safeStat(fullPath)
61187 if (!stat)
62188 continue
63189
64190 const daysSinceModified = Math.floor((now - stat.mtimeMs) / 86_400_000)
65 if (daysSinceModified < AGE_THRESHOLD_DAYS)
191 if (daysSinceModified < dir.minAgeDays)
66192 continue
67193
68 // Validate bundle ID format before using in shell command (reverse-DNS only)
69 if (!/^[\w.-]+$/.test(bundleId))
194 // Validate the name before using it in a shell command. Plain app names
195 // may legitimately contain spaces, dots and `+` (e.g. "Sublime Text").
196 const safeName = dir.nameKind === 'bundle-id'
197 ? /^[\w.-]+$/.test(bundleId)
198 : /^[\w .+-]+$/.test(bundleId)
199 if (!safeName)
70200 continue
71201
72 // Final check: use mdfind with shellEscape to verify app is truly gone
202 // Final check against Spotlight: verify the app is truly gone rather
203 // than merely absent from the index we built.
204 const query = dir.nameKind === 'bundle-id'
205 ? `kMDItemCFBundleIdentifier == ${shellEscape(bundleId)}`
206 : `kMDItemFSName == ${shellEscape(`${bundleId}.app`)}`
73207 const mdfindResult = await exec(
74 `mdfind "kMDItemCFBundleIdentifier == ${shellEscape(bundleId)}" 2>/dev/null | head -1`,
208 `mdfind ${shellEscape(query)} 2>/dev/null | head -1`,
75209 { timeout: 3000 },
76210 )
77211 if (mdfindResult.ok && mdfindResult.stdout.trim())
@@ -81,12 +215,14 @@ export async function findOrphanedAppData(): Promise<OrphanedItem[]> {
81215 ? await getDirSizeQuick(fullPath)
82216 : stat.size
83217
218 seen.add(fullPath)
84219 orphans.push({
85220 path: fullPath,
86221 bundleId,
87222 type: dir.type,
88223 sizeBytes,
89224 daysSinceModified,
225 holdsUserData: dir.holdsUserData,
90226 })
91227 }
92228 }
@@ -94,13 +230,20 @@ export async function findOrphanedAppData(): Promise<OrphanedItem[]> {
94230 return orphans.sort((a, b) => b.sizeBytes - a.sizeBytes)
95231}
96232
97async function getInstalledBundleIds(): Promise<Set<string>> {
233interface InstalledApps {
234 bundleIds: Set<string>
235 /** Lowercased app names, from the bundle's own metadata and its filename. */
236 appNames: Set<string>
237}
238
239async function getInstalledApps(): Promise<InstalledApps> {
98240 const result = await exec(
99241 'mdfind "kMDItemContentType == \'com.apple.application-bundle\'" 2>/dev/null',
100242 { timeout: 15_000 },
101243 )
102244
103245 const ids = new Set<string>()
246 const names = new Set<string>()
104247
105248 if (result.ok) {
106249 // No artificial cap. The previous `slice(0, 300)` silently classified
@@ -111,11 +254,20 @@ async function getInstalledBundleIds(): Promise<Set<string>> {
111254 const PLIST_BATCH = 20
112255 for (let i = 0; i < appPaths.length; i += PLIST_BATCH) {
113256 for (const appPath of appPaths.slice(i, i + PLIST_BATCH)) {
257 // The filename is recorded whether or not the plist is readable —
258 // an unreadable bundle is still an installed app, and treating it as
259 // absent is how live data gets reported as orphaned.
260 names.add(path.basename(appPath, '.app').toLowerCase())
114261 try {
115262 const info = readAppInfoPlist(appPath)
116263 const bid = info.CFBundleIdentifier as string
117264 if (bid)
118265 ids.add(bid)
266 for (const key of ['CFBundleName', 'CFBundleDisplayName'] as const) {
267 const value = info[key]
268 if (typeof value === 'string' && value)
269 names.add(value.toLowerCase())
270 }
119271 }
120272 catch { /* skip unreadable bundles */ }
121273 }
@@ -128,7 +280,7 @@ async function getInstalledBundleIds(): Promise<Set<string>> {
128280 for (const id of ['com.apple.Safari', 'com.apple.mail', 'com.apple.finder', 'com.apple.dock'])
129281 ids.add(id)
130282
131 return ids
283 return { bundleIds: ids, appNames: names }
132284}
133285
134286async function getDirSizeQuick(dirPath: string): Promise<number> {
routes/api.tsmodified+3-0
Changes to routes/api.ts
@@ -1756,6 +1756,9 @@ export default async function (router: Router) {
17561756 type: item.type,
17571757 sizeBytes: item.sizeBytes,
17581758 daysSinceModified: item.daysSinceModified,
1759 // Application Support and Containers can hold the only copy of
1760 // something; the UI must not pre-select these the way it can a cache.
1761 holdsUserData: item.holdsUserData,
17591762 protected: protectedPaths.has(path.resolve(item.path)),
17601763 }));
17611764
tests/orphans.test.tsadded+68-0
Changes to tests/orphans.test.ts
@@ -0,0 +1,68 @@
1import { describe, expect, it } from 'bun:test'
2import { findOrphanedAppData, KNOWN_APP_DIRS, SYSTEM_DIR_NAMES } from '@system-cleaner/clean'
3
4/**
5 * These guard the Application Support scan added for #26.
6 *
7 * The risk this feature carries is not missing an orphan — it is reporting
8 * live data as one. Chrome keeps 7.7 GB of profile (history, bookmarks,
9 * saved passwords) in a directory called `Google`, which matches no installed
10 * app name; `MobileSync` holds every iPhone backup on the machine and has no
11 * owning `.app` at all. Both must stay unreachable from this scan.
12 */
13describe('orphan attribution', () => {
14 it('maps vendor directory names to the apps that actually own them', () => {
15 // The directory name is not the app name for any of these, which is the
16 // whole reason the map exists.
17 expect(KNOWN_APP_DIRS.Google).toContain('Google Chrome')
18 expect(KNOWN_APP_DIRS.Mozilla).toContain('Firefox')
19 expect(KNOWN_APP_DIRS.BraveSoftware).toContain('Brave Browser')
20 expect(KNOWN_APP_DIRS.Code).toContain('Visual Studio Code')
21 })
22
23 it('treats macOS-owned directories as system data, not orphans', () => {
24 // None of these has an owning `.app`, so any "is the app installed?"
25 // test reports them as orphaned unless they are excluded outright.
26 for (const name of ['mobilesync', 'addressbook', 'clouddocs', 'knowledge', 'fileprovider'])
27 expect(SYSTEM_DIR_NAMES.has(name)).toBe(true)
28 })
29
30 it('has no directory listed as both system-owned and app-owned', () => {
31 for (const dir of Object.keys(KNOWN_APP_DIRS))
32 expect(SYSTEM_DIR_NAMES.has(dir.toLowerCase())).toBe(false)
33 })
34})
35
36describe('findOrphanedAppData', () => {
37 it('never reports a directory whose owning app is installed', async () => {
38 const items = await findOrphanedAppData()
39 const reported = new Set(items.map(item => item.bundleId))
40
41 // Walk the map rather than hardcoding app names: whatever is installed on
42 // the machine running the suite, its support directory must not appear.
43 for (const [dir, owners] of Object.entries(KNOWN_APP_DIRS)) {
44 const installed = owners.some(owner =>
45 ['/Applications', `${process.env.HOME}/Applications`]
46 .some(base => Bun.file(`${base}/${owner}.app/Contents/Info.plist`).size > 0),
47 )
48 if (installed)
49 expect(reported.has(dir)).toBe(false)
50 }
51 }, 120_000)
52
53 it('never reports a macOS-owned directory', async () => {
54 const items = await findOrphanedAppData()
55 for (const item of items)
56 expect(SYSTEM_DIR_NAMES.has(item.bundleId.toLowerCase())).toBe(false)
57 }, 120_000)
58
59 it('marks Application Support and Container hits as holding user data', async () => {
60 const items = await findOrphanedAppData()
61 for (const item of items) {
62 if (item.type === 'app-support' || item.type === 'container')
63 expect(item.holdsUserData).toBe(true)
64 if (item.type === 'cache' || item.type === 'log')
65 expect(item.holdsUserData).toBe(false)
66 }
67 }, 120_000)
68})