Skip to content

Commit ed4c47c

Browse files
jgamblinclaude
andcommitted
fix: address HIGH review findings — system_cache allowlist, per-module enabled, severity validation
Fix #1: system_cache too aggressive - Changed from wiping entire ~/Library/Caches to explicit allowlist - Now only cleans: Xcode, Homebrew, pip, yarn, nsurlsessiond caches - Prevents destruction of app caches that don't regenerate gracefully (Outlook, Teams, browsers) - Updated scan() to only report allowlisted directories - Updated tests to reflect new allowlist approach Fix #2: No per-module enabled config key - Added "enabled": True to all module sections in DEFAULT_CONFIG - Git remains "enabled": False by default - Updated get_all_modules() to check cfg.get("enabled", True) for each module - Modules with enabled=False are now excluded from the module list - Applies to all 13 modules including those without explicit config Fix #3: Finding.severity validation - Added __post_init__ validation to Finding dataclass - Raises ValueError if severity not in {"pass", "info", "warn", "fail"} - Catches typos without requiring full enum migration - Maintains Finding.severity as str for backward compatibility All tests pass (186/186 excluding pre-existing wifi test failures). Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
1 parent 083c94e commit ed4c47c

11 files changed

Lines changed: 281 additions & 142 deletions

File tree

src/macos_maid/cli.py

Lines changed: 52 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@
1414
from macos_maid.audit_log import AuditLog
1515
from macos_maid.config import generate_default_config_yaml, load_config
1616
from macos_maid.modules import get_all_modules
17-
from macos_maid.modules.base import CleanResult, ScanResult
17+
from macos_maid.modules.base import CleanResult, Module, ScanResult
1818
from macos_maid.reporter import Reporter
1919
from macos_maid.runner import ModuleRunner
2020
from macos_maid.system import detect_platform
@@ -23,6 +23,49 @@
2323
DEFAULT_LOG_DIR = Path.home() / ".maid"
2424

2525

26+
def _warn_if_sudo_needs_password(allow_sudo: bool) -> None:
27+
"""Warn the user if sudo requires a password.
28+
29+
Args:
30+
allow_sudo: Whether sudo operations are allowed
31+
"""
32+
if allow_sudo:
33+
result = subprocess.run(["sudo", "-n", "true"], capture_output=True)
34+
if result.returncode != 0:
35+
click.echo("Warning: sudo requires a password. You may be prompted.")
36+
37+
38+
def _build_runner(
39+
modules: list[Module],
40+
dry_run: bool,
41+
allow_sudo: bool,
42+
audit_log: AuditLog,
43+
categories: list[str] | None = None,
44+
module_names: list[str] | None = None,
45+
) -> ModuleRunner:
46+
"""Build a ModuleRunner with the given configuration.
47+
48+
Args:
49+
modules: List of modules to run
50+
dry_run: Whether to run in dry-run mode
51+
allow_sudo: Whether to allow sudo operations
52+
audit_log: Audit log instance
53+
categories: Optional list of categories to filter by
54+
module_names: Optional list of module names to filter by
55+
56+
Returns:
57+
Configured ModuleRunner instance
58+
"""
59+
return ModuleRunner(
60+
modules=modules,
61+
dry_run=dry_run,
62+
allow_sudo=allow_sudo,
63+
audit_log=audit_log,
64+
categories=categories,
65+
module_names=module_names,
66+
)
67+
68+
2669
@click.group()
2770
def main() -> None:
2871
"""MacOS Maid — macOS cleanup and security auditing tool."""
@@ -86,10 +129,7 @@ def clean(
86129
dry_run = True
87130

88131
# Validate sudo access upfront if needed
89-
if allow_sudo:
90-
result = subprocess.run(["sudo", "-n", "true"], capture_output=True)
91-
if result.returncode != 0:
92-
click.echo("Warning: sudo requires a password. You may be prompted.")
132+
_warn_if_sudo_needs_password(allow_sudo)
93133

94134
platform = detect_platform()
95135
output_fmt = output or config.report.get("output", "terminal")
@@ -104,7 +144,7 @@ def clean(
104144

105145
parsed_modules = module_names.split(",") if module_names else None
106146

107-
runner = ModuleRunner(
147+
runner = _build_runner(
108148
modules=get_all_modules(config),
109149
dry_run=dry_run,
110150
allow_sudo=allow_sudo,
@@ -121,7 +161,7 @@ def clean(
121161
# If not dry-run and not --yes, show preview and prompt for confirmation
122162
if not dry_run and not yes:
123163
# Run scan to preview changes
124-
scan_runner = ModuleRunner(
164+
scan_runner = _build_runner(
125165
modules=get_all_modules(config),
126166
dry_run=True,
127167
allow_sudo=allow_sudo,
@@ -162,17 +202,14 @@ def audit(
162202
config = load_config(cfg_path if cfg_path.exists() else None)
163203

164204
# Validate sudo access upfront if needed
165-
if allow_sudo:
166-
result = subprocess.run(["sudo", "-n", "true"], capture_output=True)
167-
if result.returncode != 0:
168-
click.echo("Warning: sudo requires a password. You may be prompted.")
205+
_warn_if_sudo_needs_password(allow_sudo)
169206

170207
platform = detect_platform()
171208
output_fmt = output or config.report.get("output", "terminal")
172209
reporter = Reporter(platform=platform, output_format=output_fmt)
173210
audit_log = AuditLog()
174211

175-
runner = ModuleRunner(
212+
runner = _build_runner(
176213
modules=get_all_modules(config),
177214
dry_run=False,
178215
allow_sudo=allow_sudo,
@@ -215,17 +252,14 @@ def report(
215252
dry_run = True
216253

217254
# Validate sudo access upfront if needed
218-
if allow_sudo:
219-
result = subprocess.run(["sudo", "-n", "true"], capture_output=True)
220-
if result.returncode != 0:
221-
click.echo("Warning: sudo requires a password. You may be prompted.")
255+
_warn_if_sudo_needs_password(allow_sudo)
222256

223257
platform = detect_platform()
224258
output_fmt = output or config.report.get("output", "terminal")
225259
reporter = Reporter(platform=platform, output_format=output_fmt)
226260
audit_log = AuditLog()
227261

228-
runner = ModuleRunner(
262+
runner = _build_runner(
229263
modules=get_all_modules(config),
230264
dry_run=dry_run,
231265
allow_sudo=allow_sudo,
@@ -235,7 +269,7 @@ def report(
235269
# If not dry-run and not --yes, show preview and prompt for confirmation
236270
if not dry_run and not yes:
237271
# Run scan to preview changes
238-
scan_runner = ModuleRunner(
272+
scan_runner = _build_runner(
239273
modules=get_all_modules(config),
240274
dry_run=True,
241275
allow_sudo=allow_sudo,

src/macos_maid/config.py

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,18 +15,21 @@
1515
"output": "terminal",
1616
},
1717
"homebrew": {
18+
"enabled": True,
1819
"update": False,
1920
"upgrade": False,
2021
"cleanup": True,
2122
"audit_casks": True,
2223
"check_untapped": True,
2324
},
2425
"docker": {
26+
"enabled": True,
2527
"remove_dangling_images": True,
2628
"remove_unused_volumes": True,
2729
"remove_stopped_containers": False,
2830
},
2931
"dev_caches": {
32+
"enabled": True,
3033
"clean": ["pip", "npm", "cargo", "gradle", "cocoapods", "xcode_derived"],
3134
},
3235
"git": {
@@ -38,44 +41,53 @@
3841
"report_large_repos": True,
3942
},
4043
"trash": {
44+
"enabled": True,
4145
"empty": True,
4246
},
4347
"wifi": {
48+
"enabled": True,
4449
"keep_days": 90,
4550
"keep_ssids": [],
4651
},
4752
"network": {
53+
"enabled": True,
4854
"flush_dns": True,
4955
"check_open_ports": True,
5056
"audit_vpn_profiles": True,
5157
"check_firewall": True,
5258
},
5359
"privacy": {
60+
"enabled": True,
5461
"clear_recent_items": True,
5562
"report_old_downloads": True,
5663
"downloads_move_to_trash": False,
5764
"downloads_older_than": 90,
5865
"audit_tcc_permissions": True,
5966
},
6067
"system_integrity": {
68+
"enabled": True,
6169
"check_sip": True,
6270
"check_filevault": True,
6371
"check_gatekeeper": True,
6472
"check_xprotect": True,
6573
},
6674
"app_audit": {
75+
"enabled": True,
6776
"check_unsigned": True,
6877
},
6978
"launch_audit": {
79+
"enabled": True,
7080
"audit_launch_daemons": True,
7181
"audit_launch_agents": True,
7282
"flag_non_apple": True,
7383
},
7484
"system_cache": {
85+
"enabled": True,
7586
"clean_system_logs": True,
7687
"clean_user_caches": True,
7788
},
7889
"tools": {
90+
"enabled": True,
7991
"lynis": True,
8092
"osquery": True,
8193
"knockknock": False,

src/macos_maid/modules/__init__.py

Lines changed: 101 additions & 56 deletions
Original file line numberDiff line numberDiff line change
@@ -30,83 +30,128 @@ def get_all_modules(config: MaidConfig | None = None) -> list[Module]:
3030
config: Optional MaidConfig to configure modules. If None, uses default settings.
3131
3232
Returns:
33-
List of configured module instances
33+
List of configured module instances (only those with enabled=True)
3434
"""
3535
# Get module configurations from config or use defaults
3636
if config is not None:
37+
trash_cfg = config.get_module_config("trash")
3738
wifi_cfg = config.get_module_config("wifi")
3839
privacy_cfg = config.get_module_config("privacy")
3940
tools_cfg = config.get_module_config("tools")
4041
git_cfg = config.get_module_config("git")
4142
homebrew_cfg = config.get_module_config("homebrew")
4243
docker_cfg = config.get_module_config("docker")
44+
dev_caches_cfg = config.get_module_config("dev_caches")
45+
system_integrity_cfg = config.get_module_config("system_integrity")
46+
network_cfg = config.get_module_config("network")
47+
app_audit_cfg = config.get_module_config("app_audit")
48+
launch_audit_cfg = config.get_module_config("launch_audit")
49+
system_cache_cfg = config.get_module_config("system_cache")
4350
else:
51+
trash_cfg = {}
4452
wifi_cfg = {}
4553
privacy_cfg = {}
4654
tools_cfg = {}
4755
git_cfg = {}
4856
homebrew_cfg = {}
4957
docker_cfg = {}
58+
dev_caches_cfg = {}
59+
system_integrity_cfg = {}
60+
network_cfg = {}
61+
app_audit_cfg = {}
62+
launch_audit_cfg = {}
63+
system_cache_cfg = {}
64+
65+
modules: list[Module] = []
66+
67+
# Trash module
68+
if trash_cfg.get("enabled", True):
69+
modules.append(TrashModule())
70+
71+
# Homebrew module
72+
if homebrew_cfg.get("enabled", True):
73+
modules.append(
74+
HomebrewModule(
75+
update=homebrew_cfg.get("update", False),
76+
upgrade=homebrew_cfg.get("upgrade", False),
77+
cleanup=homebrew_cfg.get("cleanup", True),
78+
)
79+
)
80+
81+
# Docker module
82+
if docker_cfg.get("enabled", True):
83+
modules.append(
84+
DockerModule(
85+
remove_dangling_images=docker_cfg.get("remove_dangling_images", True),
86+
remove_unused_volumes=docker_cfg.get("remove_unused_volumes", True),
87+
remove_stopped_containers=docker_cfg.get("remove_stopped_containers", False),
88+
)
89+
)
90+
91+
# Dev caches module
92+
if dev_caches_cfg.get("enabled", True):
93+
modules.append(DevCachesModule())
94+
95+
# Git module
96+
if git_cfg.get("enabled", False):
97+
modules.append(
98+
GitModule(
99+
enabled=git_cfg.get("enabled", False),
100+
repos_dir=git_cfg.get("repos_dir"),
101+
prune_remotes=git_cfg.get("prune_remotes", True),
102+
delete_merged=git_cfg.get("delete_merged_branches", False),
103+
protected_branches=git_cfg.get("protected_branches", ["main", "master", "develop"]),
104+
report_large_repos=git_cfg.get("report_large_repos", True),
105+
)
106+
)
107+
108+
# System integrity module
109+
if system_integrity_cfg.get("enabled", True):
110+
modules.append(SystemIntegrityModule())
111+
112+
# Network module
113+
if network_cfg.get("enabled", True):
114+
modules.append(NetworkModule())
50115

51116
# WiFi module
52-
wifi = WiFiModule(
53-
keep_days=wifi_cfg.get("keep_days", 90),
54-
keep_ssids=wifi_cfg.get("keep_ssids", []),
55-
)
117+
if wifi_cfg.get("enabled", True):
118+
modules.append(
119+
WiFiModule(
120+
keep_days=wifi_cfg.get("keep_days", 90),
121+
keep_ssids=wifi_cfg.get("keep_ssids", []),
122+
)
123+
)
56124

57125
# Privacy module
58-
privacy = PrivacyModule(
59-
clear_recent=privacy_cfg.get("clear_recent_items", True),
60-
downloads_move_to_trash=privacy_cfg.get("downloads_move_to_trash", False),
61-
downloads_older_than=privacy_cfg.get("downloads_older_than", 90),
62-
)
126+
if privacy_cfg.get("enabled", True):
127+
modules.append(
128+
PrivacyModule(
129+
clear_recent=privacy_cfg.get("clear_recent_items", True),
130+
downloads_move_to_trash=privacy_cfg.get("downloads_move_to_trash", False),
131+
downloads_older_than=privacy_cfg.get("downloads_older_than", 90),
132+
)
133+
)
63134

64-
# Tools module
65-
tools = ToolsModule(
66-
lynis_enabled=tools_cfg.get("lynis", True),
67-
osquery_enabled=tools_cfg.get("osquery", True),
68-
knockknock_enabled=tools_cfg.get("knockknock", False),
69-
)
135+
# App audit module
136+
if app_audit_cfg.get("enabled", True):
137+
modules.append(AppAuditModule())
70138

71-
# Git module
72-
git = GitModule(
73-
enabled=git_cfg.get("enabled", False),
74-
repos_dir=git_cfg.get("repos_dir"),
75-
prune_remotes=git_cfg.get("prune_remotes", True),
76-
delete_merged=git_cfg.get("delete_merged_branches", False),
77-
protected_branches=git_cfg.get("protected_branches", ["main", "master", "develop"]),
78-
report_large_repos=git_cfg.get("report_large_repos", True),
79-
)
139+
# Launch audit module
140+
if launch_audit_cfg.get("enabled", True):
141+
modules.append(LaunchAuditModule())
80142

81-
# Homebrew module
82-
homebrew = HomebrewModule(
83-
update=homebrew_cfg.get("update", False),
84-
upgrade=homebrew_cfg.get("upgrade", False),
85-
cleanup=homebrew_cfg.get("cleanup", True),
86-
)
143+
# Tools module
144+
if tools_cfg.get("enabled", True):
145+
modules.append(
146+
ToolsModule(
147+
lynis_enabled=tools_cfg.get("lynis", True),
148+
osquery_enabled=tools_cfg.get("osquery", True),
149+
knockknock_enabled=tools_cfg.get("knockknock", False),
150+
)
151+
)
87152

88-
# Docker module
89-
docker = DockerModule(
90-
remove_dangling_images=docker_cfg.get("remove_dangling_images", True),
91-
remove_unused_volumes=docker_cfg.get("remove_unused_volumes", True),
92-
remove_stopped_containers=docker_cfg.get("remove_stopped_containers", False),
93-
)
94-
95-
return [
96-
# Dev modules
97-
TrashModule(),
98-
homebrew,
99-
docker,
100-
DevCachesModule(),
101-
git,
102-
# Security modules
103-
SystemIntegrityModule(),
104-
NetworkModule(),
105-
wifi,
106-
privacy,
107-
AppAuditModule(),
108-
LaunchAuditModule(),
109-
tools,
110-
# Both
111-
SystemCacheModule(),
112-
]
153+
# System cache module
154+
if system_cache_cfg.get("enabled", True):
155+
modules.append(SystemCacheModule())
156+
157+
return modules

0 commit comments

Comments
 (0)