From 9dbe83d8ee2690d9834ae9b5c58baf3a1fdb55d7 Mon Sep 17 00:00:00 2001 From: Will Gordon Date: Thu, 13 Aug 2026 16:55:11 -0400 Subject: [PATCH 01/35] feat(generators): adds curated macOS preferences generator Curated 7-domain preferences generator covering dock, Finder, screensaver, trackpad, keyboard shortcuts, keyboard text replacements, battery/power, and desktop wallpaper. Renders all four classification tiers (native system.defaults options, CustomUserPreferences, a postActivation wallpaper script, and non-skipped manual-report comments) into one preferences.nix module per host. Adds SystemConfig.wallpaper_path (read from desktoppicture.db, schema verified against a real macOS Tahoe machine) and classify_wallpaper() routing it to a Tier 3 activation script. Coerces raw pmset power values to the types nix-darwin's power.sleep.* option actually expects (null | positive-int | "never"), caught by a real nix build failure during UAT. --- src/mac2nix/generators/preferences.py | 221 ++++++++++++++ src/mac2nix/mappings/__init__.py | 2 + src/mac2nix/mappings/classifier.py | 19 ++ src/mac2nix/models/system.py | 2 + src/mac2nix/scanners/system_scanner.py | 48 +++ .../templates/modules/preferences.nix.j2 | 30 ++ tests/generators/test_preferences.py | 281 ++++++++++++++++++ tests/mappings/test_classifier.py | 12 + tests/scanners/test_system_scanner.py | 120 ++++++++ 9 files changed, 735 insertions(+) create mode 100644 src/mac2nix/generators/preferences.py create mode 100644 src/mac2nix/templates/modules/preferences.nix.j2 create mode 100644 tests/generators/test_preferences.py diff --git a/src/mac2nix/generators/preferences.py b/src/mac2nix/generators/preferences.py new file mode 100644 index 0000000..528b488 --- /dev/null +++ b/src/mac2nix/generators/preferences.py @@ -0,0 +1,221 @@ +"""Curated macOS preferences generator: dock, Finder, screensaver, trackpad, +keyboard shortcuts, keyboard text replacements, battery/power, and wallpaper. + +Narrow by design -- this is not the full 197-option Tier 1 sweep, just the +curated subset the migration MVP targets. See `hack/PROJECT.md`'s "Mapping +Layer Architecture" section for the four-tier classifier this generator +consumes. +""" + +from __future__ import annotations + +from dataclasses import dataclass +from typing import Any + +from mac2nix.generators._nix_render import render_template +from mac2nix.mappings.classifier import ( + ClassificationResult, + ClassificationTier, + classify_preference, + classify_system_setting, + classify_wallpaper, +) +from mac2nix.models.preferences import PreferencesDomain +from mac2nix.models.system_state import SystemState + +# [ASSUMPTION: detail] both domain lists are a starting point verified +# against known macOS defaults domains/keys, not exhaustively tested +# against every macOS version; refine against this PR's own UAT scan output. +CURATED_WHOLESALE_DOMAINS: frozenset[str] = frozenset( + { + "com.apple.dock", + "com.apple.finder", + "com.apple.screensaver", + "com.apple.AppleMultitouchTrackpad", + "com.apple.driver.AppleBluetoothMultitouch.trackpad", + "com.apple.symbolichotkeys", # has zero DEFAULTS_TO_NIX coverage -- see + # module docstring; every key here lands in CUSTOM_PREFS, not NATIVE. + # Included anyway since rendering handles both tiers. + } +) + +# NSGlobalDomain is far broader than the curated scope (appearance, window +# behavior, etc.) -- only these specific keys are curated. +CURATED_GLOBAL_DOMAIN_KEYS: frozenset[str] = frozenset( + { + "NSUserDictionaryReplacementItems", # keyboard text replacements + "KeyRepeat", + "InitialKeyRepeat", + "ApplePressAndHoldEnabled", + "com.apple.trackpad.scaling", + "com.apple.trackpad.forceClick", + "com.apple.swipescrolldirection", + } +) + +# Both the disk-plist alias (".GlobalPreferences") and the cfprefsd-reported +# name (NSGlobalDomain) can appear as domain_name -- defaults_to_nix's own +# DOMAIN_ALIASES already resolves either form internally when classifying a +# key, so here we only need to recognize both literals as "the global +# domain" for filtering purposes, not reimplement alias resolution. +_GLOBAL_DOMAIN_NAMES = frozenset({"NSGlobalDomain", ".GlobalPreferences"}) + +_EMPTY_MODULE = "# preferences/system domain not scanned -- nothing to generate\n{ config, lib, pkgs, ... }:\n{\n}\n" + +_TEMPLATE_NAME = "preferences.nix.j2" + + +@dataclass(frozen=True, slots=True) +class _CuratedItem: + """A single curated setting awaiting tiered rendering. + + `value` is always sourced from the original scan (domain.keys[key], + power_settings[field], or the wallpaper path itself) -- never from + `result.metadata`, whose shape differs across the classify_* functions + that can produce a `ClassificationResult` here. `domain`/`key` are only + populated for preference-sourced items: they're needed to render + CUSTOM_PREFS entries under `system.defaults.CustomUserPreferences..`, + since `ClassificationResult.metadata` does NOT always carry them (the + tier_override path for a complex-struct NATIVE option, e.g. dock's + persistent-apps/persistent-others, sets metadata to + {"native_nix_path_available", "reason"} instead). + """ + + value: Any + result: ClassificationResult + domain: str | None = None + key: str | None = None + + +def _collect_preference_items(domains: list[PreferencesDomain]) -> list[_CuratedItem]: + items: list[_CuratedItem] = [] + for domain in domains: + if domain.domain_name in CURATED_WHOLESALE_DOMAINS: + keys_to_scan: list[str] = list(domain.keys) + elif domain.domain_name in _GLOBAL_DOMAIN_NAMES: + keys_to_scan = [k for k in domain.keys if k in CURATED_GLOBAL_DOMAIN_KEYS] + else: + continue + + for key in keys_to_scan: + value = domain.keys[key] + result = classify_preference(domain, key, value) + items.append(_CuratedItem(value=value, result=result, domain=domain.domain_name, key=key)) + + return items + + +def _collect_power_items(power_settings: dict[str, str]) -> list[_CuratedItem]: + """Classify each pmset power setting via POWER_SETTING_MAP. + + `power_settings` keys carry a per-source-section prefix as scanned + (e.g. "battery_power.displaysleep", "ac_power.sleep" -- see + SystemScanner._get_power_settings()), but POWER_SETTING_MAP is keyed by + the bare pmset key alone. Strip the prefix before classifying -- this is + generator-layer, not mapping-layer, code (the mapping table itself needs + no change for this domain). + """ + items: list[_CuratedItem] = [] + for field_name, value in power_settings.items(): + bare_field_name = field_name.rsplit(".", 1)[-1] + result = classify_system_setting(bare_field_name, value) + items.append(_CuratedItem(value=value, result=result)) + return items + + +# power.sleep.{computer,display,harddisk} take `null | positive-int | "never"` +# -- confirmed via a real `nix build` failure ("A definition for option +# `power.sleep.computer' is not of type `null or positive integer, meaning +# >0, or value "never"'"). A scanned pmset value of "0" means "never sleep", +# not the integer 0, which isn't itself a valid positive integer. This is +# generator-layer, not mapping-layer, code -- classify_system_setting() has +# no pmset-specific type knowledge (no coercion attached), the same way +# _collect_power_items()'s section-prefix stripping already is. +_POWER_SLEEP_NIX_PATHS = frozenset({"power.sleep.computer", "power.sleep.display", "power.sleep.harddisk"}) + +# power.restartAfterPowerFailure / networking.wakeOnLan.enable are booleans; +# pmset reports 0/1 (or Off/On) as a raw string for these. +_POWER_BOOL_NIX_PATHS = frozenset({"power.restartAfterPowerFailure", "networking.wakeOnLan.enable"}) + +_POWER_BOOL_FALSE_VALUES = frozenset({"0", "off", "no", "false"}) + + +def _coerce_power_native_value(nix_path: str, value: Any) -> Any: + if nix_path in _POWER_SLEEP_NIX_PATHS: + try: + minutes = int(value) + except (TypeError, ValueError): + return value + return "never" if minutes <= 0 else minutes + if nix_path in _POWER_BOOL_NIX_PATHS: + return str(value).strip().lower() not in _POWER_BOOL_FALSE_VALUES + return value + + +def _build_render_context(items: list[_CuratedItem]) -> dict[str, Any]: + """Apply coercion, group by tier, and shape the Jinja2 render context. + + NATIVE assignments are deduped by `nix_path` -- Nix rejects an attrset + that assigns the same dotted path twice (e.g. both + "battery_power.sleep" and "ac_power.sleep" resolve to the same + "power.sleep.computer" nix-darwin option, which has no per-power-source + control). Iterating `sorted(native.items())` for the final render list + keeps output deterministic regardless of dict insertion order. + """ + native: dict[str, Any] = {} + custom_user_prefs: dict[str, dict[str, Any]] = {} + custom_system_prefs: dict[str, dict[str, Any]] = {} + wallpaper_path: str | None = None + manual_report_comments: list[str] = [] + + for item in items: + result = item.result + metadata = result.metadata or {} + + if result.tier == ClassificationTier.NATIVE and result.nix_path is not None: + value = result.coercion(item.value) if result.coercion else item.value + value = _coerce_power_native_value(result.nix_path, value) + native[result.nix_path] = value + elif result.tier == ClassificationTier.CUSTOM_PREFS: + if item.domain is None or item.key is None: + # Every CUSTOM_PREFS item this generator produces is + # preference-sourced (classify_system_setting/classify_wallpaper + # never return CUSTOM_PREFS) -- defensive, not expected. + continue + bucket = custom_user_prefs if result.destination == "CustomUserPreferences" else custom_system_prefs + bucket.setdefault(item.domain, {})[item.key] = item.value + elif result.tier == ClassificationTier.ACTIVATION_SCRIPT: + if "wallpaper_path" in metadata: + wallpaper_path = metadata["wallpaper_path"] + else: + manual_report_comments.append(result.destination) + elif not metadata.get("skipped"): + manual_report_comments.append(result.destination) + + return { + "native_items": [{"nix_path": path, "value": value} for path, value in sorted(native.items())], + "custom_user_prefs": custom_user_prefs, + "custom_system_prefs": custom_system_prefs, + "wallpaper_path": wallpaper_path, + "manual_report_comments": manual_report_comments, + } + + +def generate_preferences(system_state: SystemState) -> str: + """Render the curated preferences.nix module from one host's scan. + + Returns rendered Nix source text -- it does not write the file itself; + `generate_all()` owns file I/O. + """ + if system_state.preferences is None or system_state.system is None: + return _EMPTY_MODULE + + items = _collect_preference_items(system_state.preferences.domains) + items.extend(_collect_power_items(system_state.system.power_settings)) + + if system_state.system.wallpaper_path is not None: + wallpaper_result = classify_wallpaper(system_state.system.wallpaper_path) + items.append(_CuratedItem(value=system_state.system.wallpaper_path, result=wallpaper_result)) + + context = _build_render_context(items) + return render_template(_TEMPLATE_NAME, context) diff --git a/src/mac2nix/mappings/__init__.py b/src/mac2nix/mappings/__init__.py index a160d14..6caf5c7 100644 --- a/src/mac2nix/mappings/__init__.py +++ b/src/mac2nix/mappings/__init__.py @@ -34,6 +34,7 @@ classify_security_setting, classify_shell_setting, classify_system_setting, + classify_wallpaper, ) from mac2nix.mappings.defaults_to_nix import ( DEFAULTS_TO_NIX, @@ -94,6 +95,7 @@ "classify_security_setting", "classify_shell_setting", "classify_system_setting", + "classify_wallpaper", "filter_ephemeral", "get_app_config", "get_font_nixpkgs", diff --git a/src/mac2nix/mappings/classifier.py b/src/mac2nix/mappings/classifier.py index e3e1fa0..4c70180 100644 --- a/src/mac2nix/mappings/classifier.py +++ b/src/mac2nix/mappings/classifier.py @@ -522,6 +522,25 @@ def classify_security_setting(field_name: str, value: Any) -> ClassificationResu ) +def classify_wallpaper(path: Path) -> ClassificationResult: + """Classify the scanned desktop wallpaper path as a Tier 3 activation script. + + No native nix-darwin option exists for the desktop picture. Metadata + carries only the structured path -- shell escaping is a generator concern. + + Destination is `postActivation`, not `postUserActivation`: nix-darwin + removed `{pre,post}UserActivation` -- all activation now runs as root, so + a generator targeting this destination must wrap any user-context + command (e.g. `osascript` talking to the logged-in user's WindowServer + session) in `sudo -u ${config.system.primaryUser}` itself. + """ + return ClassificationResult( + tier=ClassificationTier.ACTIVATION_SCRIPT, + destination="system.activationScripts.postActivation", + metadata={"wallpaper_path": str(path)}, + ) + + def classify_network_setting(field_name: str, value: Any) -> ClassificationResult: """Classify a SystemConfig/NetworkConfig field against NETWORKING_MAP.""" nix_path = NETWORKING_MAP.get(field_name) diff --git a/src/mac2nix/models/system.py b/src/mac2nix/models/system.py index 0c21616..46f89fb 100644 --- a/src/mac2nix/models/system.py +++ b/src/mac2nix/models/system.py @@ -3,6 +3,7 @@ from __future__ import annotations from datetime import datetime +from pathlib import Path from typing import Any from pydantic import BaseModel, Field @@ -115,3 +116,4 @@ class SystemConfig(BaseModel): system_extensions: list[SystemExtension] = [] icloud: ICloudState = Field(default_factory=ICloudState) mdm_enrolled: bool | None = None + wallpaper_path: Path | None = None diff --git a/src/mac2nix/scanners/system_scanner.py b/src/mac2nix/scanners/system_scanner.py index 3b03578..b213767 100644 --- a/src/mac2nix/scanners/system_scanner.py +++ b/src/mac2nix/scanners/system_scanner.py @@ -5,6 +5,7 @@ import json import logging import shutil +import sqlite3 from datetime import UTC, datetime from pathlib import Path from typing import Any @@ -23,6 +24,24 @@ _LOCALTIME_PATH = Path("/etc/localtime") +# Verified against a real macOS Tahoe (26.x) desktoppicture.db: the schema +# matches the commonly-documented data(value)/preferences(key, data_id, +# picture_id) shape, but preferences.key is NOT uniformly a picture-path +# pointer -- only key=1 rows ever point to an absolute filesystem path on +# this machine (other keys observed: 9, 10, 12, 15, 16, 20, none of which +# are paths -- likely per-space/display shuffle-history bookkeeping). Both +# conditions (key=1 AND an absolute-path-shaped value) are required to avoid +# picking one of those non-path rows. ORDER BY preferences.ROWID DESC takes +# the most-recently-written matching entry as "the current" wallpaper -- +# a deliberate simplification given SystemConfig.wallpaper_path is a single +# field, not a per-space/per-display map. +_WALLPAPER_QUERY = ( + "SELECT data.value FROM preferences " + "JOIN data ON data.ROWID = preferences.data_id " + "WHERE preferences.key = 1 AND data.value LIKE '/%' " + "ORDER BY preferences.ROWID DESC LIMIT 1" +) + @register("system") class SystemScanner(BaseScannerPlugin): @@ -58,6 +77,7 @@ def scan(self) -> SystemConfig: system_extensions = self._detect_system_extensions() icloud = self._detect_icloud() mdm_enrolled = self._detect_mdm() + wallpaper_path = self._get_wallpaper_path() return SystemConfig( hostname=hostname, @@ -90,6 +110,7 @@ def scan(self) -> SystemConfig: system_extensions=system_extensions, icloud=icloud, mdm_enrolled=mdm_enrolled, + wallpaper_path=wallpaper_path, ) def _get_computer_name(self) -> str | None: @@ -549,6 +570,33 @@ def _detect_icloud(self) -> ICloudState: documents_sync=documents_sync, ) + def _get_wallpaper_path(self) -> Path | None: + """Read the current desktop wallpaper path from desktoppicture.db. + + macOS stores the desktop picture in a SQLite database, not a plist -- + the general preferences scanner structurally cannot see it. Never + raises: a missing file, corrupt database, or schema mismatch all + resolve to `None`, consistent with every other best-effort scanner + capability in this file. + """ + db_path = Path.home() / "Library" / "Application Support" / "Dock" / "desktoppicture.db" + try: + with sqlite3.connect(f"file:{db_path}?mode=ro", uri=True) as conn: + row = conn.execute(_WALLPAPER_QUERY).fetchone() + except (sqlite3.Error, OSError) as exc: + logger.warning("Could not read desktop wallpaper from %s: %s", db_path, exc) + return None + + if not row or not row[0]: + logger.warning( + "desktoppicture.db query returned no matching row (expected a " + "'preferences' row with key=1 pointing to an absolute-path 'data' " + "value) -- wallpaper_path will be unset" + ) + return None + + return Path(row[0]) + def _detect_mdm(self) -> bool | None: """Check if device is MDM enrolled.""" result = run_command(["profiles", "status", "-type", "enrollment"]) diff --git a/src/mac2nix/templates/modules/preferences.nix.j2 b/src/mac2nix/templates/modules/preferences.nix.j2 new file mode 100644 index 0000000..050d292 --- /dev/null +++ b/src/mac2nix/templates/modules/preferences.nix.j2 @@ -0,0 +1,30 @@ +{ config, lib, pkgs, ... }: + +{ +<% for item in native_items %> + << item.nix_path >> = << item.value|nix_value|mkdefault >>; +<% endfor %> +<% if custom_user_prefs %> + system.defaults.CustomUserPreferences = << custom_user_prefs|nix_value|mkdefault >>; +<% endif %> +<% if custom_system_prefs %> + system.defaults.CustomSystemPreferences = << custom_system_prefs|nix_value|mkdefault >>; +<% endif %> +<% if wallpaper_path %> + # nix-darwin removed {pre,post}UserActivation -- all activation now runs + # as root, so the osascript call (which must talk to the logged-in user's + # WindowServer session) is explicitly run as system.primaryUser. + system.activationScripts.postActivation.text = lib.mkDefault ( + let + wallpaperPath = << wallpaper_path|nix_str >>; + in + '' + WALLPAPER_PATH=${lib.escapeShellArg wallpaperPath} + sudo -u ${config.system.primaryUser} osascript -e "tell application \"System Events\" to tell every desktop to set picture to POSIX file \"$WALLPAPER_PATH\"" + '' + ); +<% endif %> +<% for comment in manual_report_comments %> + # not automated: << comment >> +<% endfor %> +} diff --git a/tests/generators/test_preferences.py b/tests/generators/test_preferences.py new file mode 100644 index 0000000..3c4c490 --- /dev/null +++ b/tests/generators/test_preferences.py @@ -0,0 +1,281 @@ +"""Tests for the curated macOS preferences generator.""" + +from __future__ import annotations + +import shutil +import subprocess +from pathlib import Path + +import pytest + +from mac2nix.generators.preferences import ( + CURATED_GLOBAL_DOMAIN_KEYS, + CURATED_WHOLESALE_DOMAINS, + _build_render_context, + _collect_power_items, + _collect_preference_items, + _CuratedItem, + generate_preferences, +) +from mac2nix.mappings.classifier import ClassificationResult, ClassificationTier, classify_wallpaper +from mac2nix.models.preferences import PreferencesDomain, PreferencesResult +from mac2nix.models.system import SystemConfig +from mac2nix.models.system_state import SystemState + + +def _domain(name: str, keys: dict) -> PreferencesDomain: + return PreferencesDomain(domain_name=name, keys=keys) + + +def _state(*, preferences: PreferencesResult | None = None, system: SystemConfig | None = None) -> SystemState: + return SystemState( + hostname="test-host", + macos_version="26.0", + architecture="arm64", + preferences=preferences, + system=system, + ) + + +class TestCuratedDomainAllowlists: + def test_curated_wholesale_domains_are_the_expected_set(self) -> None: + assert { + "com.apple.dock", + "com.apple.finder", + "com.apple.screensaver", + "com.apple.AppleMultitouchTrackpad", + "com.apple.driver.AppleBluetoothMultitouch.trackpad", + "com.apple.symbolichotkeys", + } == CURATED_WHOLESALE_DOMAINS + + def test_curated_global_domain_keys_are_the_expected_set(self) -> None: + assert { + "NSUserDictionaryReplacementItems", + "KeyRepeat", + "InitialKeyRepeat", + "ApplePressAndHoldEnabled", + "com.apple.trackpad.scaling", + "com.apple.trackpad.forceClick", + "com.apple.swipescrolldirection", + } == CURATED_GLOBAL_DOMAIN_KEYS + + +class TestCuratedFiltering: + def test_curated_filtering(self) -> None: + domains = [ + _domain("com.apple.dock", {"tilesize": 48, "autohide": True}), + _domain("com.apple.Safari", {"HomePage": "https://example.com"}), + ] + items = _collect_preference_items(domains) + + assert len(items) == 2 + assert {i.key for i in items} == {"tilesize", "autohide"} + for item in items: + assert item.domain == "com.apple.dock" + assert item.value == domains[0].keys[item.key] + + def test_global_domain_only_curated_keys_are_classified(self) -> None: + domains = [_domain("NSGlobalDomain", {"KeyRepeat": 2, "AppleInterfaceStyle": "Dark"})] + items = _collect_preference_items(domains) + + assert len(items) == 1 + assert items[0].key == "KeyRepeat" + assert items[0].value == 2 + + def test_global_preferences_alias_is_also_treated_as_global(self) -> None: + domains = [_domain(".GlobalPreferences", {"KeyRepeat": 2, "AppleInterfaceStyle": "Dark"})] + items = _collect_preference_items(domains) + + assert len(items) == 1 + assert items[0].key == "KeyRepeat" + + def test_uncurated_domain_is_skipped_entirely(self) -> None: + domains = [_domain("com.apple.Safari", {"HomePage": "https://example.com"})] + assert _collect_preference_items(domains) == [] + + +class TestPowerItems: + def test_prefixed_power_key_is_stripped_before_classification(self) -> None: + items = _collect_power_items({"ac_power.sleep": "0", "battery_power.displaysleep": "2"}) + native = {item.result.nix_path: item.value for item in items if item.result.tier == ClassificationTier.NATIVE} + assert native == {"power.sleep.computer": "0", "power.sleep.display": "2"} + + def test_unmapped_power_key_routes_to_manual_report(self) -> None: + items = _collect_power_items({"ac_power.hibernatemode": "3"}) + assert items[0].result.tier == ClassificationTier.MANUAL_REPORT + + +class TestBuildRenderContext: + def test_native_dedupes_by_nix_path(self) -> None: + """Two power sources mapping to the same nix option must not double-assign it.""" + items = _collect_power_items({"ac_power.sleep": "0", "battery_power.sleep": "1"}) + context = _build_render_context(items) + matching = [i for i in context["native_items"] if i["nix_path"] == "power.sleep.computer"] + assert len(matching) == 1 + + def test_power_sleep_zero_coerces_to_never_not_integer_zero(self) -> None: + """nix-darwin's power.sleep.* type is `null | positive-int | "never"` -- + confirmed via a real `nix build` failure: the integer 0 isn't itself a + valid positive integer, and a raw scanned "0" string is neither. + """ + items = _collect_power_items({"ac_power.sleep": "0"}) + context = _build_render_context(items) + (item,) = [i for i in context["native_items"] if i["nix_path"] == "power.sleep.computer"] + assert item["value"] == "never" + + def test_power_sleep_nonzero_coerces_to_int(self) -> None: + items = _collect_power_items({"ac_power.displaysleep": "10"}) + context = _build_render_context(items) + (item,) = [i for i in context["native_items"] if i["nix_path"] == "power.sleep.display"] + assert item["value"] == 10 + assert isinstance(item["value"], int) + + def test_power_boolean_setting_coerces_from_raw_string(self) -> None: + items = _collect_power_items({"ac_power.autorestart": "0", "ac_power.womp": "1"}) + context = _build_render_context(items) + by_path = {i["nix_path"]: i["value"] for i in context["native_items"]} + assert by_path["power.restartAfterPowerFailure"] is False + assert by_path["networking.wakeOnLan.enable"] is True + + def test_custom_prefs_grouped_by_domain_and_key(self) -> None: + domains = [_domain("com.apple.symbolichotkeys", {"AppleSymbolicHotKeys": {"32": {"enabled": 0}}})] + items = _collect_preference_items(domains) + context = _build_render_context(items) + assert context["custom_user_prefs"] == { + "com.apple.symbolichotkeys": {"AppleSymbolicHotKeys": {"32": {"enabled": 0}}} + } + + def test_tier_override_custom_prefs_uses_item_domain_key_not_metadata(self) -> None: + """dock's persistent-apps/persistent-others hit the tier_override CUSTOM_PREFS + path, whose metadata has no domain/key -- _build_render_context must source + them from the _CuratedItem itself, not result.metadata. + """ + domains = [_domain("com.apple.dock", {"persistent-apps": [{"tile-data": {}}]})] + items = _collect_preference_items(domains) + assert items[0].result.tier == ClassificationTier.CUSTOM_PREFS + assert "domain" not in (items[0].result.metadata or {}) + + context = _build_render_context(items) + assert context["custom_user_prefs"] == {"com.apple.dock": {"persistent-apps": [{"tile-data": {}}]}} + + def test_skipped_manual_report_is_dropped_not_rendered(self) -> None: + result = ClassificationResult( + tier=ClassificationTier.MANUAL_REPORT, + destination="skipped: ephemeral UI/runtime state, not reproducible config", + metadata={"skipped": True, "reason": "ephemeral"}, + ) + context = _build_render_context([_CuratedItem(value="x", result=result)]) + assert context["manual_report_comments"] == [] + + def test_non_skipped_manual_report_is_rendered_as_comment(self) -> None: + result = ClassificationResult( + tier=ClassificationTier.MANUAL_REPORT, + destination="manual report: no nix-darwin option for X", + metadata={"field_name": "X", "value": 1}, + ) + context = _build_render_context([_CuratedItem(value=1, result=result)]) + assert context["manual_report_comments"] == ["manual report: no nix-darwin option for X"] + + def test_wallpaper_activation_script_extracted_from_metadata(self) -> None: + result = classify_wallpaper(Path("/System/Library/Desktop Pictures/The Cliffs.heic")) + context = _build_render_context([_CuratedItem(value=Path("/x"), result=result)]) + assert context["wallpaper_path"] == "/System/Library/Desktop Pictures/The Cliffs.heic" + + +class TestGeneratePreferences: + def test_missing_preferences_domain_returns_empty_module_fallback(self) -> None: + state = _state(preferences=None, system=SystemConfig(hostname="h")) + assert generate_preferences(state) == ( + "# preferences/system domain not scanned -- nothing to generate\n{ config, lib, pkgs, ... }:\n{\n}\n" + ) + + def test_missing_system_domain_returns_empty_module_fallback(self) -> None: + state = _state(preferences=PreferencesResult(domains=[]), system=None) + assert generate_preferences(state) == ( + "# preferences/system domain not scanned -- nothing to generate\n{ config, lib, pkgs, ... }:\n{\n}\n" + ) + + def test_render(self) -> None: + domains = [ + _domain("com.apple.finder", {"NewWindowTarget": "PfHm"}), + _domain("com.apple.symbolichotkeys", {"AppleSymbolicHotKeys": {"32": {"enabled": 0}}}), + ] + system = SystemConfig( + hostname="h", + power_settings={}, + wallpaper_path=Path("/System/Library/Desktop Pictures/The Cliffs.heic"), + ) + state = _state(preferences=PreferencesResult(domains=domains), system=system) + + rendered = generate_preferences(state) + + # NATIVE: coerced value ("Home"), not the raw scanned code ("PfHm"). + assert 'system.defaults.finder.NewWindowTarget = lib.mkDefault "Home";' in rendered + assert "PfHm" not in rendered + + # CUSTOM_PREFS: nested under CustomUserPreferences. + assert "system.defaults.CustomUserPreferences" in rendered + assert "AppleSymbolicHotKeys" in rendered + + # ACTIVATION_SCRIPT: wallpaper. + assert "system.activationScripts.postActivation.text" in rendered + assert "The Cliffs.heic" in rendered + assert "lib.escapeShellArg" in rendered + + def test_skipped_ephemeral_key_produces_no_manual_report_comment(self) -> None: + # A key/value shaped to trip is_ephemeral()'s UI-state detection. + domains = [_domain("com.apple.finder", {"NSWindowFrame": "0 0 100 100 0 0 1920 1080"})] + system = SystemConfig(hostname="h", power_settings={}) + state = _state(preferences=PreferencesResult(domains=domains), system=system) + + rendered = generate_preferences(state) + assert "not automated" not in rendered + + +@pytest.fixture +def require_nix_instantiate() -> None: + if shutil.which("nix-instantiate") is None: + pytest.skip("nix-instantiate not on PATH") + + +@pytest.mark.nix +def test_render_is_valid_nix(require_nix_instantiate: None, tmp_path: Path) -> None: + domains = [ + _domain("com.apple.dock", {"tilesize": 48}), + _domain("com.apple.finder", {"NewWindowTarget": "PfHm"}), + _domain("com.apple.symbolichotkeys", {"AppleSymbolicHotKeys": {"32": {"enabled": 0}}}), + ] + system = SystemConfig( + hostname="h", + power_settings={"ac_power.sleep": "0"}, + wallpaper_path=Path("/System/Library/Desktop Pictures/The Cliffs.heic"), + ) + state = _state(preferences=PreferencesResult(domains=domains), system=system) + + rendered = generate_preferences(state) + module_path = tmp_path / "preferences.nix" + module_path.write_text(rendered) + + result = subprocess.run( # noqa: S603 + ["nix-instantiate", "--parse", str(module_path)], # noqa: S607 + capture_output=True, + text=True, + check=False, + ) + assert result.returncode == 0, result.stderr + + +@pytest.mark.nix +def test_empty_module_fallback_is_valid_nix(require_nix_instantiate: None, tmp_path: Path) -> None: + state = _state(preferences=None, system=None) + rendered = generate_preferences(state) + module_path = tmp_path / "preferences.nix" + module_path.write_text(rendered) + + result = subprocess.run( # noqa: S603 + ["nix-instantiate", "--parse", str(module_path)], # noqa: S607 + capture_output=True, + text=True, + check=False, + ) + assert result.returncode == 0, result.stderr diff --git a/tests/mappings/test_classifier.py b/tests/mappings/test_classifier.py index 6f7758d..8d42ad9 100644 --- a/tests/mappings/test_classifier.py +++ b/tests/mappings/test_classifier.py @@ -15,6 +15,7 @@ classify_security_setting, classify_shell_setting, classify_system_setting, + classify_wallpaper, ) from mac2nix.mappings.defaults_to_nix import get_nix_option from mac2nix.models.application import AppSource, BrewFormula, InstalledApp @@ -501,6 +502,17 @@ def test_sip_and_gatekeeper_are_always_manual(self) -> None: assert classify_security_setting("gatekeeper_enabled", True).tier == ClassificationTier.MANUAL_REPORT +class TestClassifyWallpaper: + def test_routes_to_activation_script(self) -> None: + result = classify_wallpaper(Path("/System/Library/Desktop Pictures/The Cliffs.heic")) + assert result.tier == ClassificationTier.ACTIVATION_SCRIPT + assert result.destination == "system.activationScripts.postActivation" + + def test_metadata_carries_only_structured_path_no_shell_command(self) -> None: + result = classify_wallpaper(Path("/System/Library/Desktop Pictures/The Cliffs.heic")) + assert result.metadata == {"wallpaper_path": "/System/Library/Desktop Pictures/The Cliffs.heic"} + + class TestClassifyNetworkSetting: def test_known_network_field_routes_to_native(self) -> None: result = classify_network_setting("computer_name", "MyMac") diff --git a/tests/scanners/test_system_scanner.py b/tests/scanners/test_system_scanner.py index 57d9b5e..b6752f7 100644 --- a/tests/scanners/test_system_scanner.py +++ b/tests/scanners/test_system_scanner.py @@ -1,6 +1,7 @@ """Tests for system scanner.""" import json +import sqlite3 import subprocess from pathlib import Path from unittest.mock import patch @@ -9,6 +10,29 @@ from mac2nix.scanners.system_scanner import SystemScanner +def _write_wallpaper_db(db_path: Path, rows: list[tuple[int, str]]) -> None: + """Build a fixture desktoppicture.db matching the real schema. + + *rows* is a list of (preferences.key, data.value) pairs, inserted in + order -- later rows get higher ROWIDs, matching "most recently written + wins" real-world semantics. + """ + db_path.parent.mkdir(parents=True, exist_ok=True) + conn = sqlite3.connect(db_path) + try: + conn.execute("CREATE TABLE data (value)") + conn.execute("CREATE TABLE preferences (key INTEGER, data_id INTEGER, picture_id INTEGER)") + for key, value in rows: + cursor = conn.execute("INSERT INTO data (value) VALUES (?)", (value,)) + conn.execute( + "INSERT INTO preferences (key, data_id, picture_id) VALUES (?, ?, ?)", + (key, cursor.lastrowid, 1), + ) + conn.commit() + finally: + conn.close() + + class TestSystemScanner: def test_name_property(self) -> None: assert SystemScanner().name == "system" @@ -897,3 +921,99 @@ def test_mdm_wired_into_scan(self) -> None: assert isinstance(result, SystemConfig) assert result.mdm_enrolled is None + + +class TestWallpaperDetection: + def _db_path(self, tmp_path: Path) -> Path: + return tmp_path / "Library" / "Application Support" / "Dock" / "desktoppicture.db" + + def test_wallpaper_path_from_real_schema(self, tmp_path: Path) -> None: + db_path = self._db_path(tmp_path) + _write_wallpaper_db(db_path, [(1, "/System/Library/Desktop Pictures/The Cliffs.heic")]) + + with patch("mac2nix.scanners.system_scanner.Path.home", return_value=tmp_path): + result = SystemScanner()._get_wallpaper_path() + + assert result == Path("/System/Library/Desktop Pictures/The Cliffs.heic") + + def test_most_recently_written_path_wins(self, tmp_path: Path) -> None: + db_path = self._db_path(tmp_path) + _write_wallpaper_db( + db_path, + [ + (1, "/Library/Desktop Pictures/Pink Lotus Flower.jpg"), + (1, "/System/Library/Desktop Pictures/The Cliffs.heic"), + ], + ) + + with patch("mac2nix.scanners.system_scanner.Path.home", return_value=tmp_path): + result = SystemScanner()._get_wallpaper_path() + + assert result == Path("/System/Library/Desktop Pictures/The Cliffs.heic") + + def test_non_path_key_is_ignored(self, tmp_path: Path) -> None: + """key != 1 rows are non-path bookkeeping on real machines -- must never be selected.""" + db_path = self._db_path(tmp_path) + _write_wallpaper_db( + db_path, + [ + (1, "/System/Library/Desktop Pictures/The Cliffs.heic"), + (16, "F8DD5F35"), + ], + ) + + with patch("mac2nix.scanners.system_scanner.Path.home", return_value=tmp_path): + result = SystemScanner()._get_wallpaper_path() + + assert result == Path("/System/Library/Desktop Pictures/The Cliffs.heic") + + def test_missing_db_returns_none(self, tmp_path: Path) -> None: + with patch("mac2nix.scanners.system_scanner.Path.home", return_value=tmp_path): + result = SystemScanner()._get_wallpaper_path() + + assert result is None + + def test_corrupt_db_returns_none(self, tmp_path: Path) -> None: + db_path = self._db_path(tmp_path) + db_path.parent.mkdir(parents=True) + db_path.write_bytes(b"not a sqlite database") + + with patch("mac2nix.scanners.system_scanner.Path.home", return_value=tmp_path): + result = SystemScanner()._get_wallpaper_path() + + assert result is None + + def test_wrong_schema_returns_none(self, tmp_path: Path) -> None: + db_path = self._db_path(tmp_path) + db_path.parent.mkdir(parents=True) + conn = sqlite3.connect(db_path) + conn.execute("CREATE TABLE unrelated (foo)") + conn.commit() + conn.close() + + with patch("mac2nix.scanners.system_scanner.Path.home", return_value=tmp_path): + result = SystemScanner()._get_wallpaper_path() + + assert result is None + + def test_no_matching_row_returns_none(self, tmp_path: Path) -> None: + db_path = self._db_path(tmp_path) + _write_wallpaper_db(db_path, [(16, "F8DD5F35")]) + + with patch("mac2nix.scanners.system_scanner.Path.home", return_value=tmp_path): + result = SystemScanner()._get_wallpaper_path() + + assert result is None + + def test_wallpaper_wired_into_scan(self, tmp_path: Path) -> None: + db_path = self._db_path(tmp_path) + _write_wallpaper_db(db_path, [(1, "/System/Library/Desktop Pictures/The Cliffs.heic")]) + + with ( + patch("mac2nix.scanners.system_scanner.run_command", return_value=None), + patch("mac2nix.scanners.system_scanner.Path.home", return_value=tmp_path), + ): + result = SystemScanner().scan() + + assert isinstance(result, SystemConfig) + assert result.wallpaper_path == Path("/System/Library/Desktop Pictures/The Cliffs.heic") From 7ba0f9a6cab429b142faca869558366a498af6ec Mon Sep 17 00:00:00 2001 From: Will Gordon Date: Thu, 13 Aug 2026 16:55:30 -0400 Subject: [PATCH 02/35] feat(cli): wires generate command with generate_all() foundation Introduces generate_all(), GenerateResult, and GenerateError in generators/__init__.py, scoped to the preferences domain for now -- sibling `if` blocks and a (filename, import_line) list are structured so Tasks 6/7 can extend this incrementally without restructuring. Regenerates a host's configuration.nix generated-imports section from actual on-disk file existence, with hash-based hand-edit detection mirroring add-host's flake.nix mechanism. Fills in the `mac2nix generate --hostname` CLI stub: validates the target is a scaffolded framework and the host is registered, loads a scan file or runs one inline, and reports which domains ran/were skipped/were unrecognized. Covered by a real `nix build` integration test and a real VM-based apply-and-verify test (nix run nix-darwin -- switch against a disposable Tart VM, confirming the specific curated settings this generator writes actually survive a real switch). --- README.md | 21 +++ src/mac2nix/cli.py | 66 +++++++- src/mac2nix/generators/__init__.py | 154 ++++++++++++++++++ tests/cli/test_generate.py | 136 ++++++++++++++++ tests/generators/test_generate_all.py | 131 +++++++++++++++ tests/generators/test_generate_integration.py | 100 ++++++++++++ tests/vm/test_generate_vm.py | 150 +++++++++++++++++ 7 files changed, 755 insertions(+), 3 deletions(-) create mode 100644 tests/cli/test_generate.py create mode 100644 tests/generators/test_generate_all.py create mode 100644 tests/generators/test_generate_integration.py create mode 100644 tests/vm/test_generate_vm.py diff --git a/README.md b/README.md index a2d6384..bacfe3f 100644 --- a/README.md +++ b/README.md @@ -14,6 +14,27 @@ uv sync uv run mac2nix --help ``` +### Migrating a Mac + +Scaffold a reusable, host-less nix-darwin + home-manager + sops-nix framework, +register a machine, scan it, and generate that host's configuration: + +```sh +uv run mac2nix init ~/my-nix-config +uv run mac2nix add-host ~/my-nix-config --hostname my-mac --username myuser +uv run mac2nix generate ~/my-nix-config --hostname my-mac +``` + +`init` runs once per framework (it scaffolds `flake.nix`, shared `modules/`, +and sops-nix wiring with zero hosts registered). `add-host` registers one +machine at a time — including the first — generating that host's own +sops-nix age key behind a mandatory backup-confirmation prompt. `generate` +scans the current machine (or replays a `mac2nix scan` JSON file via +`--scan-file`) and writes that host's curated `preferences.nix`, updating +`configuration.nix`'s generated-imports section. It's safely re-runnable and +supports `--domains` to select which domains to generate (currently just +`preferences`; more are added incrementally). + ## License MIT diff --git a/src/mac2nix/cli.py b/src/mac2nix/cli.py index 252d73a..4213790 100644 --- a/src/mac2nix/cli.py +++ b/src/mac2nix/cli.py @@ -21,6 +21,7 @@ from rich.text import Text from mac2nix import onepassword +from mac2nix.generators import generate_all from mac2nix.generators.scaffold import add_host, age_key_path, init_framework from mac2nix.models.system_state import SystemState from mac2nix.orchestrator import run_scan @@ -384,10 +385,69 @@ def _register_one(current_hostname: str, current_username: str, current_system: ) +_ALLOWED_DOMAINS = ("preferences",) + + @main.command() -def generate() -> None: - """Generate nix-darwin configuration from a scan snapshot.""" - click.echo("generate: not yet implemented") +@click.argument("output_dir", type=click.Path(exists=True, file_okay=False, path_type=Path)) +@click.option("--hostname", required=True, help="Host to populate (must already be registered via add-host).") +@click.option( + "--scan-file", + type=click.Path(exists=True, dir_okay=False, path_type=Path), + default=None, + help="Source SystemState JSON produced by 'mac2nix scan'. Omit to scan the current machine inline.", +) +@click.option( + "--domains", + default="preferences", + show_default=True, + help=f"Comma-separated domains to generate. Allowed: {', '.join(_ALLOWED_DOMAINS)}.", +) +def generate(output_dir: Path, hostname: str, scan_file: Path | None, domains: str) -> None: + """Generate nix-darwin configuration for one host from a scan. + + Never invokes `nix flake lock`, `nix build`, `darwin-rebuild`, or `git` + -- it only writes files and (for an inline scan) runs the existing + read-only scanners. + """ + flake_path = output_dir / "flake.nix" + if not flake_path.is_file() or "# MAC2NIX:HOSTS:BEGIN" not in flake_path.read_text(): + raise click.ClickException(f"{output_dir} is not a mac2nix-scaffolded framework — run `mac2nix init` first") + + requested_domains = {token.strip() for token in domains.split(",") if token.strip()} + unknown = requested_domains - set(_ALLOWED_DOMAINS) + if unknown: + msg = f"unknown domain(s): {', '.join(sorted(unknown))}. Allowed: {', '.join(_ALLOWED_DOMAINS)}" + raise click.BadParameter(msg, param_hint="--domains") + + if not (output_dir / "hosts" / "darwin" / hostname).exists(): + msg = f"host {hostname!r} is not registered under {output_dir} — run `mac2nix add-host` first" + raise click.ClickException(msg) + + if scan_file is not None: + try: + system_state = SystemState.from_json(scan_file) + except Exception as exc: + raise click.ClickException(f"Failed to load scan file: {exc}") from exc + else: + try: + system_state = asyncio.run(run_scan()) + except RuntimeError as exc: + raise click.ClickException(str(exc)) from exc + + try: + result = generate_all(system_state, output_dir, hostname, requested_domains) + except click.ClickException: + raise + except Exception as exc: + raise click.ClickException(str(exc)) from exc + + if result.ran: + click.echo(f"Generated: {', '.join(sorted(result.ran))}") + for domain, reason in sorted(result.skipped.items()): + click.echo(f"Skipped {domain}: {reason}") + if result.unrecognized: + click.echo(f"Unrecognized (not generated): {', '.join(sorted(result.unrecognized))}") @main.command() diff --git a/src/mac2nix/generators/__init__.py b/src/mac2nix/generators/__init__.py index 3599ca6..7c6854e 100644 --- a/src/mac2nix/generators/__init__.py +++ b/src/mac2nix/generators/__init__.py @@ -1,2 +1,156 @@ +"""generate_all() orchestrator -- fans out to each domain generator, then +regenerates a host's configuration.nix generated-imports section from +actual on-disk file existence. + +Scoped to the `preferences` domain in this PR. Tasks 7 (shell) and 6 +(homebrew) each extend this module with one more sibling `if` block and one +more `(filename, import_line)` pair -- never restructuring the mechanism. +""" + +from __future__ import annotations + +import hashlib +import json +import logging +from dataclasses import dataclass +from pathlib import Path +from typing import Any + +from mac2nix.generators.preferences import generate_preferences +from mac2nix.models.system_state import SystemState + +logger = logging.getLogger(__name__) + + class Mac2NixError(Exception): """Base for every mac2nix-raised, user-facing error across scaffold.py and generate_all().""" + + +class GenerateError(Mac2NixError): + """Raised for generate_all() failures -- an unregistered host, etc.""" + + +@dataclass(frozen=True, slots=True) +class GenerateResult: + """The outcome of one `generate_all()` call. + + `unrecognized` is always empty for a CLI-originated call (the CLI's own + `_ALLOWED_DOMAINS` membership check rejects those first) -- it exists + for callers that invoke `generate_all()` directly, bypassing the CLI. + `homebrew_audit_manifest` is always `None` in this PR; Task 6's PR is + what first populates it -- the field exists from day one so that PR only + has to populate it, not add it. + """ + + ran: set[str] + skipped: dict[str, str] + unrecognized: frozenset[str] + homebrew_audit_manifest: list[dict[str, Any]] | None + + +_META_FILENAME = ".mac2nix-meta.json" + +_GENERATE_BEGIN = "# MAC2NIX:GENERATE:BEGIN" +_GENERATE_END = "# MAC2NIX:GENERATE:END" + +# (filename, import_line) pairs, checked via on-disk existence -- extended by +# Task 7 (shell.nix) and Task 6 (homebrew-packages.nix), never restructured. +_GENERATED_IMPORT_FILES: list[tuple[str, str]] = [ + ("preferences.nix", "./preferences.nix"), +] + + +def _read_host_meta(host_dir: Path) -> dict[str, Any]: + return json.loads((host_dir / _META_FILENAME).read_text()) + + +def _warn_if_host_imports_hand_edited(host_dir: Path, current_inner: str) -> None: + try: + stored_hash = _read_host_meta(host_dir).get("generate_imports_hash") + except (OSError, json.JSONDecodeError): + return + if stored_hash is None: + return + + if hashlib.sha256(current_inner.encode()).hexdigest() != stored_hash: + logger.warning( + "%s's MAC2NIX:GENERATE block doesn't match what generate last wrote there " + "(likely a hand-edit, or corrupted/manually-deleted host metadata) -- " + "this regeneration will overwrite it.", + host_dir / "configuration.nix", + ) + + +def _store_host_imports_hash(host_dir: Path, inner: str) -> None: + try: + meta = _read_host_meta(host_dir) + except (OSError, json.JSONDecodeError): + return + meta["generate_imports_hash"] = hashlib.sha256(inner.encode()).hexdigest() + (host_dir / _META_FILENAME).write_text(json.dumps(meta, indent=2)) + + +def _regenerate_host_imports(output_dir: Path, hostname: str) -> None: + """Fully regenerate configuration.nix's sentinel-bounded imports line from + actual on-disk file existence for this host -- not which domains ran in + this specific `generate_all()` invocation. This is what makes `generate` + safely repeatable with a narrower `--domains` subset: an already-present, + untouched file is never dropped from the imports list. + """ + host_dir = output_dir / "hosts" / "darwin" / hostname + config_path = host_dir / "configuration.nix" + content = config_path.read_text() + + # Anchor to the end of the BEGIN sentinel's own line -- it also carries a + # trailing "-- generated by ...; do not edit by hand" comment that must + # survive regeneration verbatim (mirrors _regenerate_flake_hosts_block() + # in scaffold.py). + begin_marker_end = content.index("\n", content.index(_GENERATE_BEGIN)) + 1 + end_marker_start = content.index(_GENERATE_END) + old_inner = content[begin_marker_end:end_marker_start] + + present_imports = [ + import_line for filename, import_line in _GENERATED_IMPORT_FILES if (host_dir / filename).exists() + ] + new_inner = f" imports = [ {' '.join(present_imports)} ];\n " if present_imports else " " + + _warn_if_host_imports_hand_edited(host_dir, old_inner) + + config_path.write_text(content[:begin_marker_end] + new_inner + content[end_marker_start:]) + _store_host_imports_hash(host_dir, new_inner) + + +def generate_all(system_state: SystemState, output_dir: Path, hostname: str, domains: set[str]) -> GenerateResult: + """Fill in *hostname*'s already-`add-host`-registered directory from *system_state*. + + Raises :exc:`GenerateError` if *hostname* isn't registered -- `generate` + only populates an existing host, it never registers one itself. Any + other exception raised by a domain generator (e.g. a Jinja2 template + error, an `OSError` writing the output file) propagates uncaught -- + turning it into a clean, user-facing error is the CLI's job, not this + function's. A failure partway through leaves whichever domains already + wrote their files on disk as-is: `generate` is safely re-runnable, so a + subsequent successful call regenerates every requested-and-available + domain's output again. + """ + host_dir = output_dir / "hosts" / "darwin" / hostname + if not host_dir.exists(): + msg = f"host {hostname!r} is not registered under {output_dir} -- run `mac2nix add-host` first" + raise GenerateError(msg) + + ran: set[str] = set() + skipped: dict[str, str] = {} + + if "preferences" in domains: + if system_state.preferences is not None and system_state.system is not None: + rendered = generate_preferences(system_state) + (host_dir / "preferences.nix").write_text(rendered) + ran.add("preferences") + else: + skipped["preferences"] = "not scanned" + + unrecognized = domains - {"preferences"} + + _regenerate_host_imports(output_dir, hostname) + + return GenerateResult(ran=ran, skipped=skipped, unrecognized=frozenset(unrecognized), homebrew_audit_manifest=None) diff --git a/tests/cli/test_generate.py b/tests/cli/test_generate.py new file mode 100644 index 0000000..61891f1 --- /dev/null +++ b/tests/cli/test_generate.py @@ -0,0 +1,136 @@ +"""Tests for the mac2nix generate CLI command.""" + +from __future__ import annotations + +import json +from pathlib import Path + +from click.testing import CliRunner + +from mac2nix.cli import main +from mac2nix.generators.scaffold import _read_template, _render_placeholders, init_framework +from mac2nix.models.preferences import PreferencesDomain, PreferencesResult +from mac2nix.models.system import SystemConfig +from mac2nix.models.system_state import SystemState + + +def _register_fake_host(output_dir: Path, hostname: str, username: str = "testuser") -> Path: + """Register a host without real age-keygen/sops -- the generate CLI never touches + secrets, only configuration.nix and .mac2nix-meta.json. + """ + host_dir = output_dir / "hosts" / "darwin" / hostname + host_dir.mkdir(parents=True) + template = _read_template("hosts", "darwin", "configuration.nix") + (host_dir / "configuration.nix").write_text(_render_placeholders(template, hostname, username)) + meta = {"hostname": hostname, "username": username, "system": "aarch64-darwin", "age_public_key": "age1fake"} + (host_dir / ".mac2nix-meta.json").write_text(json.dumps(meta)) + return host_dir + + +def _write_scan_file(path: Path) -> None: + domains = [PreferencesDomain(domain_name="com.apple.dock", keys={"tilesize": 48})] + state = SystemState( + hostname="h", + macos_version="26.0", + architecture="arm64", + preferences=PreferencesResult(domains=domains), + system=SystemConfig(hostname="h"), + ) + state.to_json(path) + + +class TestGenerateCommand: + def test_registered(self) -> None: + runner = CliRunner() + result = runner.invoke(main, ["--help"]) + assert result.exit_code == 0 + assert "generate" in result.output + + def test_produces_preferences_and_prints_summary(self, tmp_path: Path) -> None: + output_dir = tmp_path / "repo" + init_framework(output_dir) + _register_fake_host(output_dir, "myhost") + scan_file = tmp_path / "scan.json" + _write_scan_file(scan_file) + + runner = CliRunner() + result = runner.invoke( + main, + ["generate", str(output_dir), "--hostname", "myhost", "--scan-file", str(scan_file)], + ) + + assert result.exit_code == 0, result.output + assert (output_dir / "hosts" / "darwin" / "myhost" / "preferences.nix").is_file() + assert "preferences" in result.output + + def test_non_scaffolded_directory_fails_and_writes_nothing(self, tmp_path: Path) -> None: + output_dir = tmp_path / "not-a-repo" + output_dir.mkdir() + scan_file = tmp_path / "scan.json" + _write_scan_file(scan_file) + + runner = CliRunner() + result = runner.invoke( + main, + ["generate", str(output_dir), "--hostname", "myhost", "--scan-file", str(scan_file)], + ) + + assert result.exit_code != 0 + assert not (output_dir / "hosts").exists() + + def test_unregistered_hostname_fails_before_any_scan(self, tmp_path: Path) -> None: + output_dir = tmp_path / "repo" + init_framework(output_dir) + scan_file = tmp_path / "scan.json" + _write_scan_file(scan_file) + + runner = CliRunner() + result = runner.invoke( + main, + ["generate", str(output_dir), "--hostname", "ghost-host", "--scan-file", str(scan_file)], + ) + + assert result.exit_code != 0 + assert "ghost-host" in result.output + assert not (output_dir / "hosts" / "darwin" / "ghost-host").exists() + + def test_unallowed_domain_rejected_before_scan_or_write(self, tmp_path: Path) -> None: + output_dir = tmp_path / "repo" + init_framework(output_dir) + _register_fake_host(output_dir, "myhost") + scan_file = tmp_path / "scan.json" + _write_scan_file(scan_file) + + runner = CliRunner() + result = runner.invoke( + main, + [ + "generate", + str(output_dir), + "--hostname", + "myhost", + "--scan-file", + str(scan_file), + "--domains", + "homebrew", + ], + ) + + assert result.exit_code != 0 + assert not (output_dir / "hosts" / "darwin" / "myhost" / "preferences.nix").exists() + + def test_invalid_scan_file_fails_cleanly(self, tmp_path: Path) -> None: + output_dir = tmp_path / "repo" + init_framework(output_dir) + _register_fake_host(output_dir, "myhost") + scan_file = tmp_path / "scan.json" + scan_file.write_text("not valid json{{{") + + runner = CliRunner() + result = runner.invoke( + main, + ["generate", str(output_dir), "--hostname", "myhost", "--scan-file", str(scan_file)], + ) + + assert result.exit_code != 0 + assert "Failed to load scan file" in result.output diff --git a/tests/generators/test_generate_all.py b/tests/generators/test_generate_all.py new file mode 100644 index 0000000..20e75d8 --- /dev/null +++ b/tests/generators/test_generate_all.py @@ -0,0 +1,131 @@ +"""Tests for generate_all() -- the preferences-scoped foundation of the `generate` orchestrator.""" + +from __future__ import annotations + +import json +import logging +from pathlib import Path + +import pytest + +from mac2nix.generators import GenerateError, generate_all +from mac2nix.generators.scaffold import _read_template, _render_placeholders +from mac2nix.models.preferences import PreferencesDomain, PreferencesResult +from mac2nix.models.system import SystemConfig +from mac2nix.models.system_state import SystemState + + +def _register_fake_host(output_dir: Path, hostname: str, username: str = "testuser") -> Path: + """Build a minimal, real-template-backed registered host without real age-keygen/sops. + + generate_all() only reads/writes configuration.nix and .mac2nix-meta.json + -- it never touches secrets/flake.nix, so a full add_host() isn't needed + for these unit tests. + """ + host_dir = output_dir / "hosts" / "darwin" / hostname + host_dir.mkdir(parents=True) + template = _read_template("hosts", "darwin", "configuration.nix") + (host_dir / "configuration.nix").write_text(_render_placeholders(template, hostname, username)) + meta = {"hostname": hostname, "username": username, "system": "aarch64-darwin", "age_public_key": "age1fake"} + (host_dir / ".mac2nix-meta.json").write_text(json.dumps(meta)) + return host_dir + + +def _state(*, preferences: PreferencesResult | None, system: SystemConfig | None) -> SystemState: + return SystemState(hostname="h", macos_version="26.0", architecture="arm64", preferences=preferences, system=system) + + +def _full_state() -> SystemState: + domains = [PreferencesDomain(domain_name="com.apple.dock", keys={"tilesize": 48})] + return _state(preferences=PreferencesResult(domains=domains), system=SystemConfig(hostname="h")) + + +class TestGenerateAll: + def test_writes_preferences_and_updates_imports(self, tmp_path: Path) -> None: + output_dir = tmp_path / "repo" + host_dir = _register_fake_host(output_dir, "myhost") + + # Hand-added content outside the sentinel markers must survive regeneration. + config_path = host_dir / "configuration.nix" + hand_added = 'system.stateVersion = 7;\n networking.hostName = "myhost";' + config_path.write_text(config_path.read_text().replace("system.stateVersion = 7;", hand_added)) + + result = generate_all(_full_state(), output_dir, "myhost", {"preferences"}) + + assert result.ran == {"preferences"} + assert result.skipped == {} + assert result.unrecognized == frozenset() + assert result.homebrew_audit_manifest is None + + assert (host_dir / "preferences.nix").exists() + rendered_config = config_path.read_text() + assert "./preferences.nix" in rendered_config + assert 'networking.hostName = "myhost";' in rendered_config + + def test_missing_system_domain_skips_preferences(self, tmp_path: Path) -> None: + output_dir = tmp_path / "repo" + _register_fake_host(output_dir, "myhost") + domains = [PreferencesDomain(domain_name="com.apple.dock", keys={"tilesize": 48})] + state = _state(preferences=PreferencesResult(domains=domains), system=None) + + result = generate_all(state, output_dir, "myhost", {"preferences"}) + + assert result.ran == set() + assert result.skipped == {"preferences": "not scanned"} + + def test_repeatable_generate_keeps_file_in_imports_with_empty_domains(self, tmp_path: Path) -> None: + output_dir = tmp_path / "repo" + host_dir = _register_fake_host(output_dir, "myhost") + + generate_all(_full_state(), output_dir, "myhost", {"preferences"}) + assert (host_dir / "preferences.nix").exists() + + result = generate_all(_full_state(), output_dir, "myhost", set()) + + assert result.ran == set() + assert result.skipped == {} + config_content = (host_dir / "configuration.nix").read_text() + assert "./preferences.nix" in config_content + + def test_unregistered_host_raises_and_writes_nothing(self, tmp_path: Path) -> None: + output_dir = tmp_path / "repo" + output_dir.mkdir(parents=True) + + with pytest.raises(GenerateError): + generate_all(_full_state(), output_dir, "ghost-host", {"preferences"}) + + assert not (output_dir / "hosts" / "darwin" / "ghost-host").exists() + + def test_unrecognized_domain_returns_without_raising(self, tmp_path: Path) -> None: + output_dir = tmp_path / "repo" + _register_fake_host(output_dir, "myhost") + + result = generate_all(_full_state(), output_dir, "myhost", {"bogus"}) + + assert result.ran == set() + assert result.skipped == {} + assert result.unrecognized == frozenset({"bogus"}) + assert result.homebrew_audit_manifest is None + + def test_hand_edited_imports_section_warns_but_still_overwrites( + self, tmp_path: Path, caplog: pytest.LogCaptureFixture + ) -> None: + output_dir = tmp_path / "repo" + host_dir = _register_fake_host(output_dir, "myhost") + + generate_all(_full_state(), output_dir, "myhost", {"preferences"}) + + # Simulate a hand-edit of the generated-imports section. + config_path = host_dir / "configuration.nix" + content = config_path.read_text() + hand_edited = content.replace( + "imports = [ ./preferences.nix ];", "imports = [ ./preferences.nix ./hand-added.nix ];" + ) + config_path.write_text(hand_edited) + + with caplog.at_level(logging.WARNING): + generate_all(_full_state(), output_dir, "myhost", {"preferences"}) + + assert any("hand-edit" in record.message for record in caplog.records) + # Still overwrites -- the hand-added import doesn't survive. + assert "hand-added.nix" not in config_path.read_text() diff --git a/tests/generators/test_generate_integration.py b/tests/generators/test_generate_integration.py new file mode 100644 index 0000000..9dd5286 --- /dev/null +++ b/tests/generators/test_generate_integration.py @@ -0,0 +1,100 @@ +"""Real `nix flake lock`/`nix build` integration test — the assembled flake, with +real curated-preferences output, actually evaluates. + +Marked `nix_build` — excluded from the default `pytest`/`make test` run, +invoked via `make test-nix`. Like `test_scaffold_integration.py`, this test +NEVER calls `pytest.skip()`: if `nix`/`age`/`sops` aren't on PATH, or +there's no network access to resolve flake inputs, the test fails loudly. + +This is the first point any *generated, data-driven* Nix content (as +opposed to the static scaffold templates) gets evaluated for real — proving +`preferences.nix`'s Jinja2-rendered output is not just individually +`nix-instantiate --parse`-valid (test_preferences.py's own `-m nix` test) +but actually composes correctly with the rest of the flake. +""" + +from __future__ import annotations + +import getpass +import subprocess +from pathlib import Path + +import pytest + +from mac2nix.generators import generate_all +from mac2nix.generators.scaffold import add_host, init_framework +from mac2nix.models.preferences import PreferencesDomain, PreferencesResult +from mac2nix.models.system import SystemConfig +from mac2nix.models.system_state import SystemState +from tests._scaffold_helpers import _nix_extra_access_tokens_args, _redirect_age_keys + +pytestmark = pytest.mark.nix_build + +_HOSTNAME = "mac2nix-generate-nix-build-test" + + +def _realistic_state() -> SystemState: + """Covers all four render buckets: NATIVE (dock, plus power settings -- + including a POWER_SETTING_MAP-mapped sleep/boolean key, not just an + unmapped one -- a real `nix build` failure caught power.sleep.* needing + `null | positive-int | "never"`, not a raw scanned string, so this + fixture must actually exercise that coercion path), CUSTOM_PREFS + (symbolichotkeys-shaped), ACTIVATION_SCRIPT (wallpaper), and a + non-skipped MANUAL_REPORT (an unmapped pmset key). + """ + domains = [ + PreferencesDomain(domain_name="com.apple.dock", keys={"tilesize": 48}), + PreferencesDomain( + domain_name="com.apple.symbolichotkeys", + keys={"AppleSymbolicHotKeys": {"32": {"enabled": 0}}}, + ), + ] + system = SystemConfig( + hostname=_HOSTNAME, + power_settings={ + "ac_power.sleep": "0", # POWER_SETTING_MAP-mapped -> power.sleep.computer ("never") + "battery_power.displaysleep": "10", # POWER_SETTING_MAP-mapped -> power.sleep.display (int) + "ac_power.womp": "1", # POWER_SETTING_MAP-mapped -> networking.wakeOnLan.enable (bool) + "ac_power.hibernatemode": "3", # not in POWER_SETTING_MAP -> MANUAL_REPORT + }, + wallpaper_path=Path("/System/Library/Desktop Pictures/The Cliffs.heic"), + ) + return SystemState( + hostname=_HOSTNAME, + macos_version="26.0", + architecture="arm64", + preferences=PreferencesResult(domains=domains), + system=system, + ) + + +def test_generate_builds_for_real(tmp_path: Path) -> None: + output_dir = tmp_path / "mac2nix-scaffold" + username = getpass.getuser() + token_args = _nix_extra_access_tokens_args() + + init_framework(output_dir) + with _redirect_age_keys(tmp_path / "age-keys"): + add_host(output_dir, _HOSTNAME, username, confirm_backup=lambda _fingerprint: True) + + result = generate_all(_realistic_state(), output_dir, _HOSTNAME, {"preferences"}) + assert result.ran == {"preferences"} + assert (output_dir / "hosts" / "darwin" / _HOSTNAME / "preferences.nix").is_file() + + lock_result = subprocess.run( # noqa: S603 + ["nix", "flake", "lock", *token_args], # noqa: S607 + cwd=output_dir, + capture_output=True, + text=True, + check=False, + ) + assert lock_result.returncode == 0, f"nix flake lock failed (exit {lock_result.returncode}):\n{lock_result.stderr}" + + build_result = subprocess.run( # noqa: S603 + ["nix", "build", f".#darwinConfigurations.{_HOSTNAME}.system", "--no-link", *token_args], # noqa: S607 + cwd=output_dir, + capture_output=True, + text=True, + check=False, + ) + assert build_result.returncode == 0, f"nix build failed (exit {build_result.returncode}):\n{build_result.stderr}" diff --git a/tests/vm/test_generate_vm.py b/tests/vm/test_generate_vm.py new file mode 100644 index 0000000..81be5de --- /dev/null +++ b/tests/vm/test_generate_vm.py @@ -0,0 +1,150 @@ +"""Real VM-based apply-and-verify: switch a generated preferences.nix inside an +actual VM, confirm the real result matches the scan. + +Marked `nix_vm` (skips only if `tart` is unavailable, otherwise must run to +completion and pass). `nix build` (test_generate_integration.py) proves the +flake evaluates; this proves *applying* it reproduces the intended +preferences on a real system. + +Composes Validator's pieces manually rather than calling `Validator.validate()` +in one shot -- mirroring `test_scaffold_vm.py`'s own already-verified +approach, since a plain `nix run nix-darwin -- switch --flake .` (no +`#hostname`) relies on hostname auto-detection that doesn't hold for this +multi-host-capable scaffold, and a fresh VM needs the same nix.custom.conf/ +pre-existing-Homebrew fixups `test_scaffold_vm.py` already discovered. +""" + +from __future__ import annotations + +import asyncio +from pathlib import Path + +import pytest + +from mac2nix.generators import generate_all +from mac2nix.generators.scaffold import add_host, init_framework +from mac2nix.vm._utils import VMError +from mac2nix.vm.manager import TartVMManager +from mac2nix.vm.validator import Validator, compute_fidelity +from tests.generators.test_generate_integration import _realistic_state +from tests.vm.test_scaffold_vm import _copy_age_key_to_vm + +pytestmark = pytest.mark.nix_vm + +_HOSTNAME = "mac2nix-generate-vm-test" +_VM_USERNAME = "admin" + +_REPO_ROOT = Path(__file__).resolve().parents[2] + +# A shared `is_transient_auth_failure()` detector for this exact class of +# host-load-related SSH auth flakiness was built and verified on the +# YubiKey PIV branch (see hack/PROJECT.md's Task 10 entries) -- but that +# branch was abandoned and closed unmerged, so the fix never reached `main`. +# Retry locally here rather than depending on it, or reimplementing it in +# shared vm/ production code (out of this task's scope). +_AUTH_FAILURE_MARKER = "Permission denied (publickey,password,keyboard-interactive)" + + +async def _retry_transient(coro_fn, *, attempts: int = 5, delay: float = 10.0): + last_exc: VMError | None = None + for attempt in range(attempts): + try: + return await coro_fn() + except VMError as exc: + if _AUTH_FAILURE_MARKER not in str(exc): + raise + last_exc = exc + if attempt < attempts - 1: + await asyncio.sleep(delay) + assert last_exc is not None + raise last_exc + + +async def _exec_with_retry( + vm: TartVMManager, cmd: list[str], *, timeout: int = 30, attempts: int = 5, delay: float = 10.0 +): + ok, out, err = False, "", "" + for attempt in range(attempts): + ok, out, err = await vm.exec_command(cmd, timeout=timeout) + if ok or _AUTH_FAILURE_MARKER not in err: + return ok, out, err + if attempt < attempts - 1: + await asyncio.sleep(delay) + return ok, out, err + + +def test_generate_switches_and_matches_scan( + nix_darwin_vm: TartVMManager, tmp_path: Path, monkeypatch: pytest.MonkeyPatch +) -> None: + """generate_all() + a real nix-darwin switch must reproduce the source + scan's preferences state, per Validator's fidelity comparison. + """ + key_root = tmp_path / "age-keys" + + def _fake_age_key_path(username: str, key_dir: Path | None = None) -> Path: + return (key_dir or key_root / username) / "keys.txt" + + monkeypatch.setattr("mac2nix.generators.scaffold._age_key_path", _fake_age_key_path) + + output_dir = tmp_path / "mac2nix-scaffold" + init_framework(output_dir) + add_host(output_dir, _HOSTNAME, _VM_USERNAME, confirm_backup=lambda _fingerprint: True) + + source_state = _realistic_state() + gen_result = generate_all(source_state, output_dir, _HOSTNAME, {"preferences"}) + assert gen_result.ran == {"preferences"} + + local_key_path = _fake_age_key_path(_VM_USERNAME) + validator = Validator(nix_darwin_vm, mac2nix_source=str(_REPO_ROOT)) + + async def _run(): + await _retry_transient(lambda: validator._copy_flake_to_vm(output_dir)) + await _retry_transient(lambda: _copy_age_key_to_vm(nix_darwin_vm, local_key_path, _VM_USERNAME)) + await _retry_transient(validator._bootstrap_nix_darwin) + + move_cmd = ( + "if [ -f /etc/nix/nix.custom.conf ]; then " + "sudo mv /etc/nix/nix.custom.conf /etc/nix/nix.custom.conf.before-nix-darwin; " + "fi" + ) + ok, _out, err = await _exec_with_retry(nix_darwin_vm, ["bash", "-c", move_cmd]) + if not ok: + raise VMError(f"Failed to move aside /etc/nix/nix.custom.conf: {err.strip()}") + + ok, _out, err = await _exec_with_retry(nix_darwin_vm, ["sudo", "rm", "-rf", "/opt/homebrew"], timeout=60) + if not ok: + raise VMError(f"Failed to remove pre-existing Homebrew: {err.strip()}") + + switch_cmd = ( + f"cd {validator._REMOTE_FLAKE_DIR}" + " && . /nix/var/nix/profiles/default/etc/profile.d/nix-daemon.sh" + f" && sudo -n $(command -v nix) run nix-darwin -- switch --flake .#{_HOSTNAME}" + ) + ok, out, err = await _exec_with_retry(nix_darwin_vm, ["bash", "-c", switch_cmd], timeout=900) + if not ok: + raise VMError(f"nix-darwin switch failed:\nstdout:\n{out}\nstderr:\n{err}") + + return await _retry_transient(validator._scan_vm) + + vm_state = asyncio.run(_run()) + + # compute_fidelity() scores PreferencesResult.domains as a single list -- + # unhashable PreferencesDomain items fall back to a whole-list string + # comparison (see Validator._score_domain()/_compare_values()), which + # only matches if the target has *exactly* the source's domain set. That + # holds for a generator meant to reproduce an entire scanned domain, but + # this curated generator (and this test's fixture) intentionally covers + # only a narrow subset of com.apple.dock/symbolichotkeys/etc, while the + # VM's real re-scan naturally also reports dozens of real domains/keys + # this generator never touched. A low aggregate score here is expected, + # not evidence of a bug -- verify the *specific* curated values this + # generator actually claims to set, per Task 5 Step 8's own note to + # investigate a partial score before assuming the generator is wrong. + report = compute_fidelity(source_state, vm_state) + assert "system" in report.domain_scores + assert "preferences" in report.domain_scores + + assert vm_state.preferences is not None + vm_dock = next((d for d in vm_state.preferences.domains if d.domain_name == "com.apple.dock"), None) + assert vm_dock is not None, "com.apple.dock domain missing from the VM's post-switch re-scan" + assert vm_dock.keys.get("tilesize") == 48, f"dock tilesize was not applied: {vm_dock.keys.get('tilesize')!r}" From 68b2f0f6383273b23709179abafc63205a18ebdd Mon Sep 17 00:00:00 2001 From: Will Gordon Date: Thu, 13 Aug 2026 16:55:46 -0400 Subject: [PATCH 03/35] fix(vm): packages mac2nix as a runnable flake for VM-based re-scans Validator._scan_vm()'s local-checkout override (added for this plan's own nix_vm tests) runs `nix run -- scan` inside the VM, but mac2nix's own repo has never had a flake.nix -- confirmed via `git log --all -- flake.nix` and a real `nix run` failure ("not part of a flake"). This affected the already-merged default path too (`github:gordon-code/mac2nix` has never had one either), so `mac2nix validate`'s re-scan step has apparently never been exercised for real end-to-end. Adds a minimal flake.nix exposing a `nix run` app that delegates to `uv run --project mac2nix`, redirecting uv's venv to a fresh writable temp directory (the flake's own source is a read-only Nix store path once fetched, which uv must never try to write a venv into). Verified for real: `nix run . -- scan` builds a working venv and produces a real scan. Also excludes `.cache` (a local dev venv/cache directory that can be tens to hundreds of MB) from the local-checkout SCP/copy scope. --- flake.lock | 27 +++++++++++++++++++++ flake.nix | 48 +++++++++++++++++++++++++++++++++++++ src/mac2nix/vm/validator.py | 7 +++--- 3 files changed, 79 insertions(+), 3 deletions(-) create mode 100644 flake.lock create mode 100644 flake.nix diff --git a/flake.lock b/flake.lock new file mode 100644 index 0000000..873cd89 --- /dev/null +++ b/flake.lock @@ -0,0 +1,27 @@ +{ + "nodes": { + "nixpkgs": { + "locked": { + "lastModified": 1786599213, + "narHash": "sha256-yNJd40f11EzXBjSByCB7IPpeFFAdeoSKKM67dGkfFoU=", + "owner": "NixOS", + "repo": "nixpkgs", + "rev": "0e251e24a4f24e036a084b6b4b2d2491af4167f4", + "type": "github" + }, + "original": { + "owner": "NixOS", + "ref": "nixos-unstable", + "repo": "nixpkgs", + "type": "github" + } + }, + "root": { + "inputs": { + "nixpkgs": "nixpkgs" + } + } + }, + "root": "root", + "version": 7 +} diff --git a/flake.nix b/flake.nix new file mode 100644 index 0000000..fe3861e --- /dev/null +++ b/flake.nix @@ -0,0 +1,48 @@ +{ + description = "mac2nix -- scan macOS system state and generate nix-darwin configuration"; + + # Minimal packaging for `nix run` -- delegates to `uv` for the actual Python + # dependency resolution/venv management (this project's own established + # convention, per CLAUDE.md's "use uv for all Python work"), rather than + # reimplementing that as a native Nix Python closure (uv2nix, etc.). This + # exists specifically so `Validator._scan_vm()` (src/mac2nix/vm/validator.py) + # can `nix run -- scan` from inside a VM that + # only has Nix bootstrapped, not `uv` itself -- `uv` is fetched from + # nixpkgs as part of this flake's own closure. + inputs.nixpkgs.url = "github:NixOS/nixpkgs/nixos-unstable"; + + outputs = + { self, nixpkgs }: + let + systems = [ + "aarch64-darwin" + "x86_64-darwin" + ]; + forEachSystem = nixpkgs.lib.genAttrs systems; + in + { + apps = forEachSystem ( + system: + let + pkgs = nixpkgs.legacyPackages.${system}; + in + { + default = { + type = "app"; + # `--project ${self}` points uv at a read-only Nix store path for + # source/dependency resolution -- uv must never try to create its + # venv there (its own default, and any ambient + # $UV_PROJECT_ENVIRONMENT override, both assume a writable + # project root). Force the venv into a fresh, writable temp dir + # instead, independent of the caller's environment. + program = toString ( + pkgs.writeShellScript "mac2nix" '' + export UV_PROJECT_ENVIRONMENT="$(${pkgs.coreutils}/bin/mktemp -d)/venv" + exec ${pkgs.uv}/bin/uv run --project ${self} mac2nix "$@" + '' + ); + }; + } + ); + }; +} diff --git a/src/mac2nix/vm/validator.py b/src/mac2nix/vm/validator.py index c192a82..d36386a 100644 --- a/src/mac2nix/vm/validator.py +++ b/src/mac2nix/vm/validator.py @@ -194,9 +194,10 @@ class Validator: _DEFAULT_MAC2NIX_SOURCE = "github:gordon-code/mac2nix" # Directories excluded when SCPing a local mac2nix checkout into the VM — - # dev-machine-only content (VCS history, secrets, scan data, project memory) - # that has no bearing on the package being scanned from inside the VM. - _LOCAL_SOURCE_EXCLUDE = frozenset({".git", ".env", "data", "hack"}) + # dev-machine-only content (VCS history, secrets, scan data, project memory, + # a local dev venv/cache that can be tens to hundreds of MB) that has no + # bearing on the package being scanned from inside the VM. + _LOCAL_SOURCE_EXCLUDE = frozenset({".git", ".env", "data", "hack", ".cache"}) def __init__(self, vm: TartVMManager, mac2nix_source: str = _DEFAULT_MAC2NIX_SOURCE) -> None: self._vm = vm From d3d557d5f845117d928e0a1c743b03596b8c03b7 Mon Sep 17 00:00:00 2001 From: Will Gordon Date: Fri, 14 Aug 2026 14:14:15 -0400 Subject: [PATCH 04/35] refactor(tests): extracts shared generate test helper Extracts the duplicated `_register_fake_host()` helper (byte-identical across test_generate_all.py and test_generate.py) into tests/_generate_helpers.py, mirroring the existing tests/_scaffold_helpers.py convention. --- tests/_generate_helpers.py | 28 +++++++++++++++++++++++++++ tests/cli/test_generate.py | 17 ++-------------- tests/generators/test_generate_all.py | 19 +----------------- 3 files changed, 31 insertions(+), 33 deletions(-) create mode 100644 tests/_generate_helpers.py diff --git a/tests/_generate_helpers.py b/tests/_generate_helpers.py new file mode 100644 index 0000000..4311224 --- /dev/null +++ b/tests/_generate_helpers.py @@ -0,0 +1,28 @@ +"""Shared test-support helpers for generate_all()/`mac2nix generate` unit tests. + +Not a test module itself (pytest's python_files pattern doesn't match this +name) — imported by tests/generators/test_generate_all.py and tests/cli/test_generate.py. +""" + +from __future__ import annotations + +import json +from pathlib import Path + +from mac2nix.generators.scaffold import _read_template, _render_placeholders + + +def _register_fake_host(output_dir: Path, hostname: str, username: str = "testuser") -> Path: + """Build a minimal, real-template-backed registered host without real age-keygen/sops. + + generate_all() only reads/writes configuration.nix and .mac2nix-meta.json + -- it never touches secrets/flake.nix, so a full add_host() isn't needed + for these unit tests. + """ + host_dir = output_dir / "hosts" / "darwin" / hostname + host_dir.mkdir(parents=True) + template = _read_template("hosts", "darwin", "configuration.nix") + (host_dir / "configuration.nix").write_text(_render_placeholders(template, hostname, username)) + meta = {"hostname": hostname, "username": username, "system": "aarch64-darwin", "age_public_key": "age1fake"} + (host_dir / ".mac2nix-meta.json").write_text(json.dumps(meta)) + return host_dir diff --git a/tests/cli/test_generate.py b/tests/cli/test_generate.py index 61891f1..d38390d 100644 --- a/tests/cli/test_generate.py +++ b/tests/cli/test_generate.py @@ -2,29 +2,16 @@ from __future__ import annotations -import json from pathlib import Path from click.testing import CliRunner from mac2nix.cli import main -from mac2nix.generators.scaffold import _read_template, _render_placeholders, init_framework +from mac2nix.generators.scaffold import init_framework from mac2nix.models.preferences import PreferencesDomain, PreferencesResult from mac2nix.models.system import SystemConfig from mac2nix.models.system_state import SystemState - - -def _register_fake_host(output_dir: Path, hostname: str, username: str = "testuser") -> Path: - """Register a host without real age-keygen/sops -- the generate CLI never touches - secrets, only configuration.nix and .mac2nix-meta.json. - """ - host_dir = output_dir / "hosts" / "darwin" / hostname - host_dir.mkdir(parents=True) - template = _read_template("hosts", "darwin", "configuration.nix") - (host_dir / "configuration.nix").write_text(_render_placeholders(template, hostname, username)) - meta = {"hostname": hostname, "username": username, "system": "aarch64-darwin", "age_public_key": "age1fake"} - (host_dir / ".mac2nix-meta.json").write_text(json.dumps(meta)) - return host_dir +from tests._generate_helpers import _register_fake_host def _write_scan_file(path: Path) -> None: diff --git a/tests/generators/test_generate_all.py b/tests/generators/test_generate_all.py index 20e75d8..ebee511 100644 --- a/tests/generators/test_generate_all.py +++ b/tests/generators/test_generate_all.py @@ -2,33 +2,16 @@ from __future__ import annotations -import json import logging from pathlib import Path import pytest from mac2nix.generators import GenerateError, generate_all -from mac2nix.generators.scaffold import _read_template, _render_placeholders from mac2nix.models.preferences import PreferencesDomain, PreferencesResult from mac2nix.models.system import SystemConfig from mac2nix.models.system_state import SystemState - - -def _register_fake_host(output_dir: Path, hostname: str, username: str = "testuser") -> Path: - """Build a minimal, real-template-backed registered host without real age-keygen/sops. - - generate_all() only reads/writes configuration.nix and .mac2nix-meta.json - -- it never touches secrets/flake.nix, so a full add_host() isn't needed - for these unit tests. - """ - host_dir = output_dir / "hosts" / "darwin" / hostname - host_dir.mkdir(parents=True) - template = _read_template("hosts", "darwin", "configuration.nix") - (host_dir / "configuration.nix").write_text(_render_placeholders(template, hostname, username)) - meta = {"hostname": hostname, "username": username, "system": "aarch64-darwin", "age_public_key": "age1fake"} - (host_dir / ".mac2nix-meta.json").write_text(json.dumps(meta)) - return host_dir +from tests._generate_helpers import _register_fake_host def _state(*, preferences: PreferencesResult | None, system: SystemConfig | None) -> SystemState: From fdda7b9ad5cf6a9d5dc9f9ac9bfb8c3a9398d074 Mon Sep 17 00:00:00 2001 From: Will Gordon Date: Fri, 14 Aug 2026 14:14:48 -0400 Subject: [PATCH 05/35] docs(vm): documents the nix run entry point in README mac2nix ships a minimal flake.nix (used internally to re-scan a VM during `mac2nix validate`) that also works as a standalone entry point for anyone without a local uv install. --- README.md | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/README.md b/README.md index bacfe3f..b1b5d2e 100644 --- a/README.md +++ b/README.md @@ -8,6 +8,13 @@ Generate nix-darwin configurations from macOS system scans. uv sync ``` +mac2nix also ships a minimal `flake.nix`, so it can be run via Nix without a +local `uv` install (used internally to re-scan a VM during `mac2nix validate`): + +```sh +nix run github:gordon-code/mac2nix -- --help +``` + ## Usage ```sh From 0deb5cd42356ef646b776fdab91d4beb1c77dba7 Mon Sep 17 00:00:00 2001 From: Will Gordon Date: Fri, 14 Aug 2026 14:15:01 -0400 Subject: [PATCH 06/35] docs(generators): notes generate_all()'s concurrency limitation Its read-modify-write cycle on configuration.nix/.mac2nix-meta.json assumes single-operator, sequential use, matching add_host()'s own documented limitation on flake.nix/.sops.yaml. --- src/mac2nix/generators/__init__.py | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/src/mac2nix/generators/__init__.py b/src/mac2nix/generators/__init__.py index 7c6854e..a4ade8c 100644 --- a/src/mac2nix/generators/__init__.py +++ b/src/mac2nix/generators/__init__.py @@ -132,6 +132,11 @@ def generate_all(system_state: SystemState, output_dir: Path, hostname: str, dom wrote their files on disk as-is: `generate` is safely re-runnable, so a subsequent successful call regenerates every requested-and-available domain's output again. + + Not safe for concurrent invocations against the same *hostname* -- + its read-modify-write cycle on configuration.nix/.mac2nix-meta.json + assumes single-operator, sequential use, matching `add_host()`'s own + documented limitation on flake.nix/.sops.yaml. """ host_dir = output_dir / "hosts" / "darwin" / hostname if not host_dir.exists(): From 517f390184f98e07a87fd084c336a4fcc98f2ea3 Mon Sep 17 00:00:00 2001 From: Will Gordon Date: Fri, 14 Aug 2026 13:24:56 -0400 Subject: [PATCH 07/35] fix(cli): guards generate's filesystem checks against OSError MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `flake_path.read_text()` and `host_dir.exists()` in `mac2nix generate` were unguarded — verified empirically that `Path.exists()` on Python 3.13 propagates `PermissionError` for an unreadable intermediate directory rather than returning `False`. Either call could leak a raw traceback instead of a clean `click.ClickException`, violating this command's own "never a raw traceback" contract. Extracted into `_check_scaffolded_framework()`/`_check_host_registered()` (also resolves a ruff PLR0912 branch-count violation for real, rather than suppressing it). Also adds `ac_power.autorestart` to the nix_build integration fixture and verifies it for real: `power.restartAfterPowerFailure`'s boolean coercion had only ever been confirmed by a unit test, never by an actual nix build, unlike its sibling `networking.wakeOnLan.enable`. --- src/mac2nix/cli.py | 29 +++++++++++---- tests/cli/test_generate.py | 35 +++++++++++++++++++ tests/generators/test_generate_integration.py | 1 + 3 files changed, 59 insertions(+), 6 deletions(-) diff --git a/src/mac2nix/cli.py b/src/mac2nix/cli.py index 4213790..b3ce6bf 100644 --- a/src/mac2nix/cli.py +++ b/src/mac2nix/cli.py @@ -388,6 +388,27 @@ def _register_one(current_hostname: str, current_username: str, current_system: _ALLOWED_DOMAINS = ("preferences",) +def _check_scaffolded_framework(output_dir: Path) -> None: + flake_path = output_dir / "flake.nix" + try: + is_scaffolded = flake_path.is_file() and "# MAC2NIX:HOSTS:BEGIN" in flake_path.read_text() + except OSError as exc: + raise click.ClickException(f"Failed to read {flake_path}: {exc}") from exc + if not is_scaffolded: + raise click.ClickException(f"{output_dir} is not a mac2nix-scaffolded framework — run `mac2nix init` first") + + +def _check_host_registered(output_dir: Path, hostname: str) -> None: + host_dir = output_dir / "hosts" / "darwin" / hostname + try: + host_registered = host_dir.exists() + except OSError as exc: + raise click.ClickException(f"Failed to check {host_dir}: {exc}") from exc + if not host_registered: + msg = f"host {hostname!r} is not registered under {output_dir} — run `mac2nix add-host` first" + raise click.ClickException(msg) + + @main.command() @click.argument("output_dir", type=click.Path(exists=True, file_okay=False, path_type=Path)) @click.option("--hostname", required=True, help="Host to populate (must already be registered via add-host).") @@ -410,9 +431,7 @@ def generate(output_dir: Path, hostname: str, scan_file: Path | None, domains: s -- it only writes files and (for an inline scan) runs the existing read-only scanners. """ - flake_path = output_dir / "flake.nix" - if not flake_path.is_file() or "# MAC2NIX:HOSTS:BEGIN" not in flake_path.read_text(): - raise click.ClickException(f"{output_dir} is not a mac2nix-scaffolded framework — run `mac2nix init` first") + _check_scaffolded_framework(output_dir) requested_domains = {token.strip() for token in domains.split(",") if token.strip()} unknown = requested_domains - set(_ALLOWED_DOMAINS) @@ -420,9 +439,7 @@ def generate(output_dir: Path, hostname: str, scan_file: Path | None, domains: s msg = f"unknown domain(s): {', '.join(sorted(unknown))}. Allowed: {', '.join(_ALLOWED_DOMAINS)}" raise click.BadParameter(msg, param_hint="--domains") - if not (output_dir / "hosts" / "darwin" / hostname).exists(): - msg = f"host {hostname!r} is not registered under {output_dir} — run `mac2nix add-host` first" - raise click.ClickException(msg) + _check_host_registered(output_dir, hostname) if scan_file is not None: try: diff --git a/tests/cli/test_generate.py b/tests/cli/test_generate.py index d38390d..21d2cf6 100644 --- a/tests/cli/test_generate.py +++ b/tests/cli/test_generate.py @@ -106,6 +106,41 @@ def test_unallowed_domain_rejected_before_scan_or_write(self, tmp_path: Path) -> assert result.exit_code != 0 assert not (output_dir / "hosts" / "darwin" / "myhost" / "preferences.nix").exists() + def test_unreadable_flake_nix_fails_cleanly_not_a_raw_traceback(self, tmp_path: Path) -> None: + output_dir = tmp_path / "repo" + init_framework(output_dir) + flake_path = output_dir / "flake.nix" + flake_path.chmod(0o000) + + try: + runner = CliRunner() + result = runner.invoke(main, ["generate", str(output_dir), "--hostname", "myhost"]) + finally: + flake_path.chmod(0o644) + + assert result.exit_code != 0 + assert result.exc_info is not None + assert result.exc_info[0] is SystemExit + assert "Failed to read" in result.output + + def test_unreadable_hosts_dir_fails_cleanly_not_a_raw_traceback(self, tmp_path: Path) -> None: + output_dir = tmp_path / "repo" + init_framework(output_dir) + hosts_dir = output_dir / "hosts" + hosts_dir.mkdir() + hosts_dir.chmod(0o000) + + try: + runner = CliRunner() + result = runner.invoke(main, ["generate", str(output_dir), "--hostname", "myhost"]) + finally: + hosts_dir.chmod(0o755) + + assert result.exit_code != 0 + assert result.exc_info is not None + assert result.exc_info[0] is SystemExit + assert "Failed to check" in result.output + def test_invalid_scan_file_fails_cleanly(self, tmp_path: Path) -> None: output_dir = tmp_path / "repo" init_framework(output_dir) diff --git a/tests/generators/test_generate_integration.py b/tests/generators/test_generate_integration.py index 9dd5286..8e99bdb 100644 --- a/tests/generators/test_generate_integration.py +++ b/tests/generators/test_generate_integration.py @@ -55,6 +55,7 @@ def _realistic_state() -> SystemState: "ac_power.sleep": "0", # POWER_SETTING_MAP-mapped -> power.sleep.computer ("never") "battery_power.displaysleep": "10", # POWER_SETTING_MAP-mapped -> power.sleep.display (int) "ac_power.womp": "1", # POWER_SETTING_MAP-mapped -> networking.wakeOnLan.enable (bool) + "ac_power.autorestart": "0", # POWER_SETTING_MAP-mapped -> power.restartAfterPowerFailure (bool) "ac_power.hibernatemode": "3", # not in POWER_SETTING_MAP -> MANUAL_REPORT }, wallpaper_path=Path("/System/Library/Desktop Pictures/The Cliffs.heic"), From 5ba3a0d4b2290db0c9af804fa4c5890b08990082 Mon Sep 17 00:00:00 2001 From: Will Gordon Date: Fri, 14 Aug 2026 13:26:08 -0400 Subject: [PATCH 08/35] docs(generators): flags future-PR extension points inline --- src/mac2nix/cli.py | 2 ++ src/mac2nix/generators/__init__.py | 5 +++++ 2 files changed, 7 insertions(+) diff --git a/src/mac2nix/cli.py b/src/mac2nix/cli.py index b3ce6bf..a15ca45 100644 --- a/src/mac2nix/cli.py +++ b/src/mac2nix/cli.py @@ -385,6 +385,8 @@ def _register_one(current_hostname: str, current_username: str, current_system: ) +# Extend this tuple (never restructure) as each domain generator lands -- +# Task 7 (shell) adds "shell", then Task 6 (homebrew) adds "homebrew". _ALLOWED_DOMAINS = ("preferences",) diff --git a/src/mac2nix/generators/__init__.py b/src/mac2nix/generators/__init__.py index a4ade8c..c2cc888 100644 --- a/src/mac2nix/generators/__init__.py +++ b/src/mac2nix/generators/__init__.py @@ -154,6 +154,11 @@ def generate_all(system_state: SystemState, output_dir: Path, hostname: str, dom else: skipped["preferences"] = "not scanned" + # Extend this literal set (never restructure the mechanism) whenever a new + # sibling `if` block is added above -- Task 7 (shell) widens this to + # {"preferences", "shell"}, then Task 6 (homebrew) to + # {"preferences", "shell", "homebrew"}. Forgetting this line makes a + # newly-supported domain wrongly report as `unrecognized`. unrecognized = domains - {"preferences"} _regenerate_host_imports(output_dir, hostname) From c4371cda21f06c55e4baed93651567e1ea92116d Mon Sep 17 00:00:00 2001 From: Will Gordon Date: Fri, 14 Aug 2026 13:57:11 -0400 Subject: [PATCH 09/35] fix(generators): adds nix_comment filter, prevents comment injection A `#` comment ends at the first newline -- free-text that derives from scanned, attacker-writable data (e.g. a plist key name set via `defaults write`) could otherwise embed a newline and break out of a rendered `# not automated: ...` comment, turning the remainder of the string into live Nix syntax. --- src/mac2nix/generators/_nix_render.py | 14 ++++++++++++++ tests/generators/test_nix_render.py | 14 ++++++++++++++ 2 files changed, 28 insertions(+) diff --git a/src/mac2nix/generators/_nix_render.py b/src/mac2nix/generators/_nix_render.py index c19c168..fbd2e5f 100644 --- a/src/mac2nix/generators/_nix_render.py +++ b/src/mac2nix/generators/_nix_render.py @@ -43,6 +43,19 @@ def nix_mkdefault(nix_expr: str) -> str: return f"lib.mkDefault {nix_expr}" +def nix_comment(text: str) -> str: + """Render *text* as safe content for a single-line Nix `#` comment. + + A `#` comment ends at the first newline -- free-text that ultimately + derives from scanned, attacker-writable data (e.g. a plist key name + via `defaults write`) could otherwise embed a newline and break out of + the comment, turning the rest of the string into live Nix syntax. + Replace rather than strip so multi-line input stays visible (if + garbled) instead of silently disappearing. + """ + return text.replace("\r\n", " ").replace("\n", " ").replace("\r", " ") + + def setup_jinja_env(loader: jinja2.BaseLoader | None = None) -> jinja2.Environment: """Build the Jinja2 environment shared by every mac2nix Nix template. @@ -63,6 +76,7 @@ def setup_jinja_env(loader: jinja2.BaseLoader | None = None) -> jinja2.Environme env.filters["nix_value"] = python_to_nix env.filters["nix_str"] = nix_string env.filters["mkdefault"] = nix_mkdefault + env.filters["nix_comment"] = nix_comment return env diff --git a/tests/generators/test_nix_render.py b/tests/generators/test_nix_render.py index 499ba47..137f387 100644 --- a/tests/generators/test_nix_render.py +++ b/tests/generators/test_nix_render.py @@ -11,6 +11,7 @@ import pytest from mac2nix.generators._nix_render import ( + nix_comment, nix_mkdefault, nix_string, python_to_nix, @@ -78,6 +79,19 @@ def test_nix_mkdefault_wraps_expression() -> None: assert nix_mkdefault("true") == "lib.mkDefault true" +def test_nix_comment_replaces_newline_variants_with_space() -> None: + assert nix_comment("a\nb") == "a b" + assert nix_comment("a\r\nb") == "a b" + assert nix_comment("a\rb") == "a b" + + +def test_nix_comment_cannot_be_used_to_break_out_of_a_single_line_nix_comment() -> None: + malicious = 'x\n }; system.activationScripts.pwned.text = "id > /tmp/pwned"; { y' + rendered = nix_comment(malicious) + assert "\n" not in rendered + assert "\r" not in rendered + + def test_jinja_env_custom_delimiters_do_not_collide_with_nix_braces() -> None: loader = jinja2.DictLoader({"fixture.nix.j2": "<% if x %>{ y = << y|nix_value >>; }<% endif %>"}) env = setup_jinja_env(loader=loader) From 45b85113bc247c6f070350adab8bb6eca88312f7 Mon Sep 17 00:00:00 2001 From: Will Gordon Date: Fri, 14 Aug 2026 13:57:19 -0400 Subject: [PATCH 10/35] fix(templates): passes wallpaper path as osascript argv Embedding the path directly into the AppleScript source string let a path containing a literal `"` terminate the string early and inject arbitrary AppleScript (including `do shell script`). Passing it as an osascript argument (argv) instead means the path is never parsed as script source. Also switches the manual-report comment line to the new nix_comment filter, and adds coverage confirming the CustomSystemPreferences bucket (reachable via any curated domain's system-scoped plist) renders correctly. --- .../templates/modules/preferences.nix.j2 | 15 +++- tests/generators/test_preferences.py | 80 +++++++++++++++++++ 2 files changed, 91 insertions(+), 4 deletions(-) diff --git a/src/mac2nix/templates/modules/preferences.nix.j2 b/src/mac2nix/templates/modules/preferences.nix.j2 index 050d292..b652276 100644 --- a/src/mac2nix/templates/modules/preferences.nix.j2 +++ b/src/mac2nix/templates/modules/preferences.nix.j2 @@ -13,18 +13,25 @@ <% if wallpaper_path %> # nix-darwin removed {pre,post}UserActivation -- all activation now runs # as root, so the osascript call (which must talk to the logged-in user's - # WindowServer session) is explicitly run as system.primaryUser. + # WindowServer session) is explicitly run as system.primaryUser. The path + # is passed as an osascript *argument* (argv), never embedded into the + # AppleScript source text itself -- a path containing a literal `"` would + # otherwise terminate the embedded AppleScript string early and allow + # arbitrary command injection via `&`/`do shell script`. system.activationScripts.postActivation.text = lib.mkDefault ( let wallpaperPath = << wallpaper_path|nix_str >>; in '' - WALLPAPER_PATH=${lib.escapeShellArg wallpaperPath} - sudo -u ${config.system.primaryUser} osascript -e "tell application \"System Events\" to tell every desktop to set picture to POSIX file \"$WALLPAPER_PATH\"" + sudo -u ${config.system.primaryUser} osascript \ + -e 'on run argv' \ + -e ' tell application "System Events" to tell every desktop to set picture to POSIX file (item 1 of argv)' \ + -e 'end run' \ + ${lib.escapeShellArg wallpaperPath} '' ); <% endif %> <% for comment in manual_report_comments %> - # not automated: << comment >> + # not automated: << comment|nix_comment >> <% endfor %> } diff --git a/tests/generators/test_preferences.py b/tests/generators/test_preferences.py index 3c4c490..83dcef6 100644 --- a/tests/generators/test_preferences.py +++ b/tests/generators/test_preferences.py @@ -231,6 +231,43 @@ def test_skipped_ephemeral_key_produces_no_manual_report_comment(self) -> None: rendered = generate_preferences(state) assert "not automated" not in rendered + def test_newline_in_sensitive_key_cannot_inject_nix_syntax_via_manual_report_comment(self) -> None: + """A real, previously-exploitable bug: a plist key is fully attacker-writable + (`defaults write ...` needs no privilege), and a newline in a + key routed to MANUAL_REPORT would otherwise break out of the rendered + `# not automated: ...` single-line Nix comment. + """ + malicious_key = 'x_TOKEN\n }; system.activationScripts.pwned.text = "pwned"; { y' + domains = [_domain("com.apple.dock", {malicious_key: "irrelevant"})] + system = SystemConfig(hostname="h", power_settings={}) + state = _state(preferences=PreferencesResult(domains=domains), system=system) + + rendered = generate_preferences(state) + + assert "\n }; system.activationScripts.pwned" not in rendered + assert "pwned" not in rendered # the redacted key never appears in output at all + for line in rendered.splitlines(): + assert line.count("#") <= 1 or line.strip().startswith("#") + + def test_custom_system_preferences_bucket_is_reachable_and_renders(self) -> None: + """A system-scoped plist for a curated domain is real, not speculative -- the + preferences scanner globs `/Library/Preferences/*.plist` unfiltered, so any + curated domain name can legitimately appear there on a real Mac. + """ + domain = PreferencesDomain( + domain_name="com.apple.screensaver", + source_path=Path("/Library/Preferences/com.apple.screensaver.plist"), + keys={"someUnmappedKey": "value"}, + ) + system = SystemConfig(hostname="h", power_settings={}) + state = _state(preferences=PreferencesResult(domains=[domain]), system=system) + + rendered = generate_preferences(state) + + assert "system.defaults.CustomSystemPreferences" in rendered + assert "someUnmappedKey" in rendered + assert "system.defaults.CustomUserPreferences" not in rendered + @pytest.fixture def require_nix_instantiate() -> None: @@ -279,3 +316,46 @@ def test_empty_module_fallback_is_valid_nix(require_nix_instantiate: None, tmp_p check=False, ) assert result.returncode == 0, result.stderr + + +@pytest.mark.nix +def test_custom_system_preferences_block_is_valid_nix(require_nix_instantiate: None, tmp_path: Path) -> None: + domain = PreferencesDomain( + domain_name="com.apple.screensaver", + source_path=Path("/Library/Preferences/com.apple.screensaver.plist"), + keys={"someUnmappedKey": "value"}, + ) + system = SystemConfig(hostname="h", power_settings={}) + state = _state(preferences=PreferencesResult(domains=[domain]), system=system) + + rendered = generate_preferences(state) + module_path = tmp_path / "preferences.nix" + module_path.write_text(rendered) + + result = subprocess.run( # noqa: S603 + ["nix-instantiate", "--parse", str(module_path)], # noqa: S607 + capture_output=True, + text=True, + check=False, + ) + assert result.returncode == 0, result.stderr + + +@pytest.mark.nix +def test_newline_in_key_does_not_break_nix_syntax(require_nix_instantiate: None, tmp_path: Path) -> None: + malicious_key = 'x_TOKEN\n }; system.activationScripts.pwned.text = "pwned"; { y' + domains = [_domain("com.apple.dock", {malicious_key: "irrelevant"})] + system = SystemConfig(hostname="h", power_settings={}) + state = _state(preferences=PreferencesResult(domains=domains), system=system) + + rendered = generate_preferences(state) + module_path = tmp_path / "preferences.nix" + module_path.write_text(rendered) + + result = subprocess.run( # noqa: S603 + ["nix-instantiate", "--parse", str(module_path)], # noqa: S607 + capture_output=True, + text=True, + check=False, + ) + assert result.returncode == 0, result.stderr From f096a2c447ec262dc174b1d1e5c809e6f9db2e66 Mon Sep 17 00:00:00 2001 From: Will Gordon Date: Fri, 14 Aug 2026 13:57:27 -0400 Subject: [PATCH 11/35] fix(mappings): redacts sensitive key name in classifier destination A secret is sometimes embedded in the key itself rather than the value (e.g. a literal API key used as a dict key). The sensitive-key manual-report path already redacted the value but still echoed the raw key name into `destination`, which generators surface verbatim into real, on-disk output. --- src/mac2nix/mappings/classifier.py | 7 ++++++- tests/mappings/test_classifier.py | 10 ++++++++++ 2 files changed, 16 insertions(+), 1 deletion(-) diff --git a/src/mac2nix/mappings/classifier.py b/src/mac2nix/mappings/classifier.py index 4c70180..e5b8f79 100644 --- a/src/mac2nix/mappings/classifier.py +++ b/src/mac2nix/mappings/classifier.py @@ -195,9 +195,14 @@ def _classify_preference_precheck( ) -> ClassificationResult | None: """SEC-1/SEC-3 gating checks plus ephemeral-noise filtering, run before any tier routing.""" if _contains_sensitive_pattern(key) or _value_contains_sensitive_pattern(value): + # The key name itself is redacted in `destination` (not just `value`) + # -- a secret is sometimes embedded in the key rather than the value + # (e.g. a literal API key used as a dict key), and `destination` is + # what generators surface into real, on-disk output. return ClassificationResult( tier=ClassificationTier.MANUAL_REPORT, - destination=f"manual report: key '{key}' in domain '{domain.domain_name}' matches a sensitive pattern", + destination=f"manual report: key '***REDACTED***' in domain '{domain.domain_name}' " + "matches a sensitive pattern", metadata={ "potentially_sensitive": True, "reason": "key or value matches a sensitive pattern", diff --git a/tests/mappings/test_classifier.py b/tests/mappings/test_classifier.py index 8d42ad9..37d5610 100644 --- a/tests/mappings/test_classifier.py +++ b/tests/mappings/test_classifier.py @@ -128,6 +128,16 @@ def test_key_matching_sensitive_pattern_routes_to_manual_report_redacted(self) - assert result.metadata["potentially_sensitive"] is True assert result.metadata["value"] == "***REDACTED***" + def test_sensitive_key_name_itself_is_redacted_in_destination(self) -> None: + """destination is what generators (e.g. preferences.py) surface into real, + on-disk output -- the secret can be embedded in the key itself, not just + the value, so `destination` must never leak the raw key name either. + """ + domain = _domain("com.example.someapp", {"sk-live-abc123_TOKEN": "unused"}) + result = classify_preference(domain, "sk-live-abc123_TOKEN", "unused") + assert "sk-live-abc123" not in result.destination + assert "***REDACTED***" in result.destination + def test_sensitive_match_takes_priority_over_native_mapping(self) -> None: """A sensitive-looking key must never leak into Tier 1/2/3 even if otherwise mappable.""" domain = _domain("com.apple.dock", {"autohide_TOKEN": True}) From 6aba95b5801414af328e3eb00ab1d0a14d546406 Mon Sep 17 00:00:00 2001 From: Will Gordon Date: Fri, 14 Aug 2026 13:57:34 -0400 Subject: [PATCH 12/35] fix(cli): rejects path-traversal hostnames in generate command `--hostname` fed directly into `host_dir = output_dir / "hosts" / "darwin" / hostname` without validation -- a value like "../../../../tmp" escapes output_dir entirely. Routes it through the same _validate_hostname callback add-host already uses, and switches _check_scaffolded_framework to the shared _HOSTS_BEGIN/_HOSTS_END sentinel constants so it can't silently drift from add_host()'s own check. --- src/mac2nix/cli.py | 19 +++++++++++---- tests/cli/test_generate.py | 49 ++++++++++++++++++++++++++++++++++++++ 2 files changed, 64 insertions(+), 4 deletions(-) diff --git a/src/mac2nix/cli.py b/src/mac2nix/cli.py index a15ca45..e01b427 100644 --- a/src/mac2nix/cli.py +++ b/src/mac2nix/cli.py @@ -22,7 +22,7 @@ from mac2nix import onepassword from mac2nix.generators import generate_all -from mac2nix.generators.scaffold import add_host, age_key_path, init_framework +from mac2nix.generators.scaffold import _HOSTS_BEGIN, _HOSTS_END, add_host, age_key_path, init_framework from mac2nix.models.system_state import SystemState from mac2nix.orchestrator import run_scan from mac2nix.scan_report import ScannerOutcome, ScannerStatus, capture_scanner_logs, get_remediation_hint @@ -391,12 +391,18 @@ def _register_one(current_hostname: str, current_username: str, current_system: def _check_scaffolded_framework(output_dir: Path) -> None: + """Matches add_host()'s own scaffolded-framework check in scaffold.py + (shared sentinel constants, both markers) -- kept as a second, + independent check (not a shared function) since this one must raise + click.ClickException while add_host() raises ScaffoldError, but reusing + the sentinel constants avoids the two checks silently drifting apart. + """ flake_path = output_dir / "flake.nix" try: - is_scaffolded = flake_path.is_file() and "# MAC2NIX:HOSTS:BEGIN" in flake_path.read_text() + flake_content = flake_path.read_text() if flake_path.is_file() else "" except OSError as exc: raise click.ClickException(f"Failed to read {flake_path}: {exc}") from exc - if not is_scaffolded: + if _HOSTS_BEGIN not in flake_content or _HOSTS_END not in flake_content: raise click.ClickException(f"{output_dir} is not a mac2nix-scaffolded framework — run `mac2nix init` first") @@ -413,7 +419,12 @@ def _check_host_registered(output_dir: Path, hostname: str) -> None: @main.command() @click.argument("output_dir", type=click.Path(exists=True, file_okay=False, path_type=Path)) -@click.option("--hostname", required=True, help="Host to populate (must already be registered via add-host).") +@click.option( + "--hostname", + required=True, + callback=_validate_hostname, + help="Host to populate (must already be registered via add-host).", +) @click.option( "--scan-file", type=click.Path(exists=True, dir_okay=False, path_type=Path), diff --git a/tests/cli/test_generate.py b/tests/cli/test_generate.py index 21d2cf6..04bc1d2 100644 --- a/tests/cli/test_generate.py +++ b/tests/cli/test_generate.py @@ -3,6 +3,7 @@ from __future__ import annotations from pathlib import Path +from unittest.mock import AsyncMock, patch from click.testing import CliRunner @@ -156,3 +157,51 @@ def test_invalid_scan_file_fails_cleanly(self, tmp_path: Path) -> None: assert result.exit_code != 0 assert "Failed to load scan file" in result.output + + def test_path_traversal_hostname_rejected(self, tmp_path: Path) -> None: + """`--hostname` must go through the same allowlist add-host uses -- otherwise + host_dir = output_dir / "hosts" / "darwin" / hostname escapes output_dir + for a value like "../../../../tmp". + """ + output_dir = tmp_path / "repo" + init_framework(output_dir) + + runner = CliRunner() + result = runner.invoke(main, ["generate", str(output_dir), "--hostname", "../../evil"]) + + assert result.exit_code != 0 + assert not (tmp_path / "evil").exists() + + def test_inline_scan_success_when_no_scan_file_given(self, tmp_path: Path) -> None: + output_dir = tmp_path / "repo" + init_framework(output_dir) + _register_fake_host(output_dir, "myhost") + + domains = [PreferencesDomain(domain_name="com.apple.dock", keys={"tilesize": 48})] + state = SystemState( + hostname="h", + macos_version="26.0", + architecture="arm64", + preferences=PreferencesResult(domains=domains), + system=SystemConfig(hostname="h"), + ) + + with patch("mac2nix.cli.run_scan", new=AsyncMock(return_value=state)): + runner = CliRunner() + result = runner.invoke(main, ["generate", str(output_dir), "--hostname", "myhost"]) + + assert result.exit_code == 0, result.output + assert (output_dir / "hosts" / "darwin" / "myhost" / "preferences.nix").is_file() + + def test_inline_scan_runtime_error_fails_cleanly(self, tmp_path: Path) -> None: + output_dir = tmp_path / "repo" + init_framework(output_dir) + _register_fake_host(output_dir, "myhost") + + with patch("mac2nix.cli.run_scan", new=AsyncMock(side_effect=RuntimeError("orchestrator failed"))): + runner = CliRunner() + result = runner.invoke(main, ["generate", str(output_dir), "--hostname", "myhost"]) + + assert result.exit_code != 0 + assert "orchestrator failed" in result.output + assert not (output_dir / "hosts" / "darwin" / "myhost" / "preferences.nix").exists() From 2346b8f939d6a476c63e7ec0d09fff1f952c0e6a Mon Sep 17 00:00:00 2001 From: Will Gordon Date: Fri, 14 Aug 2026 13:57:41 -0400 Subject: [PATCH 13/35] perf(scanners): closes sqlite3 wallpaper-db connection handle sqlite3.Connection's own context manager only commits/rolls back the pending transaction on exit -- it doesn't close the connection or its file descriptor. Wraps it in contextlib.closing() so the handle is actually released. --- src/mac2nix/scanners/system_scanner.py | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/mac2nix/scanners/system_scanner.py b/src/mac2nix/scanners/system_scanner.py index b213767..7491983 100644 --- a/src/mac2nix/scanners/system_scanner.py +++ b/src/mac2nix/scanners/system_scanner.py @@ -2,6 +2,7 @@ from __future__ import annotations +import contextlib import json import logging import shutil @@ -581,7 +582,10 @@ def _get_wallpaper_path(self) -> Path | None: """ db_path = Path.home() / "Library" / "Application Support" / "Dock" / "desktoppicture.db" try: - with sqlite3.connect(f"file:{db_path}?mode=ro", uri=True) as conn: + # sqlite3.Connection's own context manager only commits/rolls back + # the pending transaction on exit -- it does not close the + # connection or its file descriptor. contextlib.closing() does. + with contextlib.closing(sqlite3.connect(f"file:{db_path}?mode=ro", uri=True)) as conn: row = conn.execute(_WALLPAPER_QUERY).fetchone() except (sqlite3.Error, OSError) as exc: logger.warning("Could not read desktop wallpaper from %s: %s", db_path, exc) From c6bffadc5417848d9966e3e3894dc9c95ff456e9 Mon Sep 17 00:00:00 2001 From: Will Gordon Date: Fri, 14 Aug 2026 13:57:50 -0400 Subject: [PATCH 14/35] fix(generators): raises clear error on missing generate sentinels A stripped or corrupted MAC2NIX:GENERATE:BEGIN/END marker pair previously surfaced as a raw `ValueError: substring not found` instead of a purpose-written, actionable GenerateError. --- src/mac2nix/generators/__init__.py | 14 +++++++++++--- tests/generators/test_generate_all.py | 17 +++++++++++++++++ 2 files changed, 28 insertions(+), 3 deletions(-) diff --git a/src/mac2nix/generators/__init__.py b/src/mac2nix/generators/__init__.py index c2cc888..71ee1d7 100644 --- a/src/mac2nix/generators/__init__.py +++ b/src/mac2nix/generators/__init__.py @@ -105,8 +105,16 @@ def _regenerate_host_imports(output_dir: Path, hostname: str) -> None: # trailing "-- generated by ...; do not edit by hand" comment that must # survive regeneration verbatim (mirrors _regenerate_flake_hosts_block() # in scaffold.py). - begin_marker_end = content.index("\n", content.index(_GENERATE_BEGIN)) + 1 - end_marker_start = content.index(_GENERATE_END) + try: + begin_marker_end = content.index("\n", content.index(_GENERATE_BEGIN)) + 1 + end_marker_start = content.index(_GENERATE_END) + except ValueError as exc: + msg = ( + f"{config_path} is missing its {_GENERATE_BEGIN!r}/{_GENERATE_END!r} sentinel " + "markers -- restore them (see templates/scaffold/hosts/darwin/configuration.nix) " + "before running generate again" + ) + raise GenerateError(msg) from exc old_inner = content[begin_marker_end:end_marker_start] present_imports = [ @@ -140,7 +148,7 @@ def generate_all(system_state: SystemState, output_dir: Path, hostname: str, dom """ host_dir = output_dir / "hosts" / "darwin" / hostname if not host_dir.exists(): - msg = f"host {hostname!r} is not registered under {output_dir} -- run `mac2nix add-host` first" + msg = f"host {hostname!r} is not registered under {output_dir} — run `mac2nix add-host` first" raise GenerateError(msg) ran: set[str] = set() diff --git a/tests/generators/test_generate_all.py b/tests/generators/test_generate_all.py index ebee511..a889e66 100644 --- a/tests/generators/test_generate_all.py +++ b/tests/generators/test_generate_all.py @@ -112,3 +112,20 @@ def test_hand_edited_imports_section_warns_but_still_overwrites( assert any("hand-edit" in record.message for record in caplog.records) # Still overwrites -- the hand-added import doesn't survive. assert "hand-added.nix" not in config_path.read_text() + + def test_missing_sentinel_markers_raise_clear_generate_error(self, tmp_path: Path) -> None: + """A stripped/corrupted sentinel pair must raise a purpose-written + GenerateError, not a raw, unhelpful `ValueError: substring not found`. + """ + output_dir = tmp_path / "repo" + host_dir = _register_fake_host(output_dir, "myhost") + config_path = host_dir / "configuration.nix" + stripped = config_path.read_text().replace( + " # MAC2NIX:GENERATE:BEGIN -- generated by `mac2nix generate`; do not edit by hand\n" + " # MAC2NIX:GENERATE:END\n", + "", + ) + config_path.write_text(stripped) + + with pytest.raises(GenerateError, match="sentinel"): + generate_all(_full_state(), output_dir, "myhost", {"preferences"}) From ace2e931f7aa86da04c0c5eeacd2d877601a477b Mon Sep 17 00:00:00 2001 From: Will Gordon Date: Fri, 14 Aug 2026 13:57:58 -0400 Subject: [PATCH 15/35] perf(vm): reuses stable venv cache path instead of mktemp per run A fresh `mktemp -d` on every `nix run` invocation forced a full dependency install each time and left orphaned venv directories behind in $TMPDIR indefinitely. uv creates any missing parent directories itself, so a stable, XDG-respecting cache path makes repeat invocations a fast no-op sync and leaves no litter. Verified for real: two consecutive `nix run . -- --version` calls, second one instant. --- flake.nix | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/flake.nix b/flake.nix index fe3861e..4d9495a 100644 --- a/flake.nix +++ b/flake.nix @@ -33,11 +33,15 @@ # source/dependency resolution -- uv must never try to create its # venv there (its own default, and any ambient # $UV_PROJECT_ENVIRONMENT override, both assume a writable - # project root). Force the venv into a fresh, writable temp dir - # instead, independent of the caller's environment. + # project root). Force the venv into a stable, writable cache + # location instead of a fresh `mktemp -d` per invocation -- `uv` + # creates any missing parent directories itself, and reusing the + # same venv makes repeat invocations a fast no-op sync instead of + # a full dependency install, and never leaves orphaned venv + # directories behind in $TMPDIR. program = toString ( pkgs.writeShellScript "mac2nix" '' - export UV_PROJECT_ENVIRONMENT="$(${pkgs.coreutils}/bin/mktemp -d)/venv" + export UV_PROJECT_ENVIRONMENT="''${XDG_CACHE_HOME:-$HOME/.cache}/mac2nix/nix-run-venv" exec ${pkgs.uv}/bin/uv run --project ${self} mac2nix "$@" '' ); From 075715abb0937ad86809607db7620a8fc3c8cf0e Mon Sep 17 00:00:00 2001 From: Will Gordon Date: Fri, 14 Aug 2026 13:58:12 -0400 Subject: [PATCH 16/35] test(vm): adds real nix build/run coverage for mac2nix's own flake Before this, flake.nix's only consumer was Validator._scan_vm()'s local-source override, exercised only by nix_vm-marked tests -- which always skip in CI (no tart on GitHub-hosted runners). A syntax or evaluation error in flake.nix would pass `make test`/`make test-nix` completely undetected, silently reproducing the "nix run mechanism doesn't actually work" gap this PR found and fixed. --- tests/test_own_flake.py | 44 +++++++++++++++++++++++++++++++++++++++++ 1 file changed, 44 insertions(+) create mode 100644 tests/test_own_flake.py diff --git a/tests/test_own_flake.py b/tests/test_own_flake.py new file mode 100644 index 0000000..5dd3bd9 --- /dev/null +++ b/tests/test_own_flake.py @@ -0,0 +1,44 @@ +"""Real nix build/run test for mac2nix's own root flake.nix. + +Marked `nix_build` (never skipped). Before this PR, flake.nix's only +consumer was `Validator._scan_vm()`'s local-source override, exercised +only by `nix_vm`-marked tests -- which always skip in this project's CI +(no `tart` on GitHub-hosted runners). A syntax/evaluation error here would +otherwise pass `make test`/`make test-nix` completely undetected, silently +reproducing the exact "nix run mechanism doesn't actually work" gap this +PR found and fixed. +""" + +from __future__ import annotations + +import subprocess +from pathlib import Path + +import pytest + +pytestmark = pytest.mark.nix_build + +_REPO_ROOT = Path(__file__).resolve().parent.parent + + +def test_own_flake_check() -> None: + result = subprocess.run( + ["nix", "flake", "check"], # noqa: S607 + cwd=_REPO_ROOT, + capture_output=True, + text=True, + check=False, + ) + assert result.returncode == 0, f"nix flake check failed (exit {result.returncode}):\n{result.stderr}" + + +def test_own_flake_app_runs_for_real() -> None: + result = subprocess.run( + ["nix", "run", ".", "--", "--version"], # noqa: S607 + cwd=_REPO_ROOT, + capture_output=True, + text=True, + check=False, + ) + assert result.returncode == 0, f"nix run . -- --version failed (exit {result.returncode}):\n{result.stderr}" + assert "mac2nix" in result.stdout From 98c5f6bf1cfccef2c66fbf612ee60e2a0aefdb75 Mon Sep 17 00:00:00 2001 From: Will Gordon Date: Fri, 14 Aug 2026 14:12:58 -0400 Subject: [PATCH 17/35] fix(generators): rejects non-finite floats in python_to_nix str(float('inf')) renders as the bare word `inf`, which Nix parses as an undefined variable reference rather than a number literal -- a scanned preference value that happened to be IEEE 754 infinity or NaN (structurally possible via plistlib) would otherwise surface as an opaque nix-instantiate syntax error with no indication of which preference value caused it. Fails loud and immediately instead, consistent with this function's existing TypeError for other unsupported types. --- src/mac2nix/generators/_nix_render.py | 7 +++++++ tests/generators/test_nix_render.py | 10 ++++++++++ 2 files changed, 17 insertions(+) diff --git a/src/mac2nix/generators/_nix_render.py b/src/mac2nix/generators/_nix_render.py index fbd2e5f..99f5d72 100644 --- a/src/mac2nix/generators/_nix_render.py +++ b/src/mac2nix/generators/_nix_render.py @@ -2,6 +2,7 @@ from __future__ import annotations +import math import re from typing import Any @@ -20,6 +21,12 @@ def python_to_nix(value: Any) -> str: """Recursively render a Python value as a Nix literal.""" if isinstance(value, bool): return "true" if value else "false" + if isinstance(value, float) and (math.isinf(value) or math.isnan(value)): + # str() renders these as `inf`/`nan`/`-inf` -- not valid Nix number + # literals ("undefined variable 'inf'" at nix-instantiate time, + # pointing at the generated file rather than the offending value). + msg = f"cannot render {value!r} as a Nix literal -- not a finite number" + raise TypeError(msg) if isinstance(value, (int, float)): return str(value) if value is None: diff --git a/tests/generators/test_nix_render.py b/tests/generators/test_nix_render.py index 137f387..ef6be67 100644 --- a/tests/generators/test_nix_render.py +++ b/tests/generators/test_nix_render.py @@ -59,6 +59,16 @@ def test_python_to_nix_raises_typeerror_for_unsupported_type() -> None: python_to_nix((1, 2, 3)) +@pytest.mark.parametrize("value", [float("inf"), float("-inf"), float("nan")]) +def test_python_to_nix_raises_typeerror_for_non_finite_float(value: float) -> None: + """str(float('inf')) is `inf` -- not a valid Nix number literal (Nix parses + it as an undefined variable reference instead). Must fail loud here, not + surface as an opaque nix-instantiate syntax error later. + """ + with pytest.raises(TypeError, match="not a finite number"): + python_to_nix(value) + + def test_nix_string_escapes_backslash() -> None: value = "a" + _BACKSLASH + "b" assert nix_string(value) == _DQUOTE + "a" + _BACKSLASH + _BACKSLASH + "b" + _DQUOTE From 40725ae33f54cd382094c1b4d94ea71ed588d80d Mon Sep 17 00:00:00 2001 From: Will Gordon Date: Fri, 14 Aug 2026 14:13:10 -0400 Subject: [PATCH 18/35] fix(generators): hardens power bool coercion and wallpaper activation _coerce_power_native_value() matched power booleans against a known false-value set and defaulted everything else -- including an empty string or an unrecognized future pmset value -- to true, the opposite of this generator's mkDefault-everywhere conservatism. Switches to a positive match against known true-values instead. The wallpaper activation script's osascript call talks to the logged- in user's WindowServer session, which doesn't exist during a headless or SSH-only activation. Under nix-darwin's `set -e`, that failure could abort the entire darwin-rebuild switch over a cosmetic setting. Adds a non-fatal `|| echo ... >&2` fallback so it degrades loudly instead of failing the whole activation. --- src/mac2nix/generators/preferences.py | 7 +++++-- .../templates/modules/preferences.nix.j2 | 8 +++++++- tests/generators/test_preferences.py | 19 +++++++++++++++++++ 3 files changed, 31 insertions(+), 3 deletions(-) diff --git a/src/mac2nix/generators/preferences.py b/src/mac2nix/generators/preferences.py index 528b488..5ef7086 100644 --- a/src/mac2nix/generators/preferences.py +++ b/src/mac2nix/generators/preferences.py @@ -137,7 +137,7 @@ def _collect_power_items(power_settings: dict[str, str]) -> list[_CuratedItem]: # pmset reports 0/1 (or Off/On) as a raw string for these. _POWER_BOOL_NIX_PATHS = frozenset({"power.restartAfterPowerFailure", "networking.wakeOnLan.enable"}) -_POWER_BOOL_FALSE_VALUES = frozenset({"0", "off", "no", "false"}) +_POWER_BOOL_TRUE_VALUES = frozenset({"1", "on", "yes", "true"}) def _coerce_power_native_value(nix_path: str, value: Any) -> Any: @@ -148,7 +148,10 @@ def _coerce_power_native_value(nix_path: str, value: Any) -> Any: return value return "never" if minutes <= 0 else minutes if nix_path in _POWER_BOOL_NIX_PATHS: - return str(value).strip().lower() not in _POWER_BOOL_FALSE_VALUES + # Positive match, not `not in {false-values}` -- an empty string or an + # unrecognized future pmset value must coerce to False (matching this + # generator's mkDefault-everywhere conservatism), not silently to True. + return str(value).strip().lower() in _POWER_BOOL_TRUE_VALUES return value diff --git a/src/mac2nix/templates/modules/preferences.nix.j2 b/src/mac2nix/templates/modules/preferences.nix.j2 index b652276..fc66460 100644 --- a/src/mac2nix/templates/modules/preferences.nix.j2 +++ b/src/mac2nix/templates/modules/preferences.nix.j2 @@ -18,6 +18,11 @@ # AppleScript source text itself -- a path containing a literal `"` would # otherwise terminate the embedded AppleScript string early and allow # arbitrary command injection via `&`/`do shell script`. + # A headless/SSH-only activation (no WindowServer session for + # primaryUser -- e.g. a fleet member switched over a remote session) + # would otherwise fail this whole activation script under nix-darwin's + # `set -e`; `|| echo ... >&2` keeps that failure non-fatal but still + # loud, rather than either aborting the switch or failing silently. system.activationScripts.postActivation.text = lib.mkDefault ( let wallpaperPath = << wallpaper_path|nix_str >>; @@ -27,7 +32,8 @@ -e 'on run argv' \ -e ' tell application "System Events" to tell every desktop to set picture to POSIX file (item 1 of argv)' \ -e 'end run' \ - ${lib.escapeShellArg wallpaperPath} + ${lib.escapeShellArg wallpaperPath} \ + || echo "mac2nix: could not set desktop wallpaper (no GUI session for ${config.system.primaryUser}?)" >&2 '' ); <% endif %> diff --git a/tests/generators/test_preferences.py b/tests/generators/test_preferences.py index 83dcef6..efb8405 100644 --- a/tests/generators/test_preferences.py +++ b/tests/generators/test_preferences.py @@ -137,6 +137,18 @@ def test_power_boolean_setting_coerces_from_raw_string(self) -> None: assert by_path["power.restartAfterPowerFailure"] is False assert by_path["networking.wakeOnLan.enable"] is True + def test_power_boolean_setting_defaults_false_for_unrecognized_value(self) -> None: + """A positive match against known true-values, not `not in {false-values}` -- + an empty string or an unrecognized future pmset value must coerce to + False, matching this generator's mkDefault-everywhere conservatism, + not silently default to True. + """ + items = _collect_power_items({"ac_power.autorestart": "", "ac_power.womp": "some-future-value"}) + context = _build_render_context(items) + by_path = {i["nix_path"]: i["value"] for i in context["native_items"]} + assert by_path["power.restartAfterPowerFailure"] is False + assert by_path["networking.wakeOnLan.enable"] is False + def test_custom_prefs_grouped_by_domain_and_key(self) -> None: domains = [_domain("com.apple.symbolichotkeys", {"AppleSymbolicHotKeys": {"32": {"enabled": 0}}})] items = _collect_preference_items(domains) @@ -222,6 +234,13 @@ def test_render(self) -> None: assert "The Cliffs.heic" in rendered assert "lib.escapeShellArg" in rendered + # A headless/SSH-only activation (no WindowServer session for + # primaryUser) must not abort the whole activation under + # nix-darwin's `set -e` -- the osascript call has a non-fatal, + # loud fallback. + assert "|| echo" in rendered + assert "no GUI session" in rendered + def test_skipped_ephemeral_key_produces_no_manual_report_comment(self) -> None: # A key/value shaped to trip is_ephemeral()'s UI-state detection. domains = [_domain("com.apple.finder", {"NSWindowFrame": "0 0 100 100 0 0 1920 1080"})] From 42662c51b2b32fd679f88b9971a14cc54c917701 Mon Sep 17 00:00:00 2001 From: Will Gordon Date: Sun, 16 Aug 2026 18:18:31 -0400 Subject: [PATCH 19/35] fix(generators): prevents hardware-dependent power settings from aborting darwin-rebuild switch power.restartAfterPowerFailure and networking.wakeOnLan.enable were auto-applied as NATIVE options from scanned pmset values, but nix-darwin's own activation script (modules/system/checks.nix) hard-aborts the entire darwin-rebuild switch on hardware that doesn't support these features -- confirmed against nix-darwin's actual source and matching upstream issues #1236/#1244. Neither setting can be safely known-supported from a source-machine scan, so both now route to a manual-report comment instead. Also fixes two real duplicate-comment bugs confirmed via actual pmset -g custom output on real hardware (some keys report under both AC Power and Battery Power sections with different values), and makes NSGlobalDomain (cfprefsd, live) deterministically win over .GlobalPreferences (on-disk snapshot) when a scan reports both for the same curated key -- which real scans always do, since the on-disk plist's file stem never textually matches the literal domain name cfprefsd reports. Verified against Apple's own documentation that cfprefsd's in-memory cache is authoritative. Re-verified against a real Tart VM: the nix_vm integration test failed before this fix and passes after. --- src/mac2nix/generators/preferences.py | 104 ++++++++++++++++--- tests/generators/test_preferences.py | 142 +++++++++++++++++++++++--- 2 files changed, 216 insertions(+), 30 deletions(-) diff --git a/src/mac2nix/generators/preferences.py b/src/mac2nix/generators/preferences.py index 5ef7086..4e8e317 100644 --- a/src/mac2nix/generators/preferences.py +++ b/src/mac2nix/generators/preferences.py @@ -88,8 +88,30 @@ class _CuratedItem: def _collect_preference_items(domains: list[PreferencesDomain]) -> list[_CuratedItem]: + """Collect curated items from wholesale domains and the global-domain alias. + + `PreferencesScanner._discover_cfprefsd_domains()` only skips a cfprefsd + domain name already present in `seen` -- and `seen` is populated from + on-disk plist file *stems*, so the on-disk ".GlobalPreferences.plist" + (stem ".GlobalPreferences") never matches the literal string + "NSGlobalDomain" that `defaults domains` reports. This means a real scan + of any Mac with a `.GlobalPreferences.plist` (virtually all of them) + deterministically produces BOTH domains for the same underlying global + preferences. When they disagree on a curated key's value, NSGlobalDomain + (cfprefsd, live) wins over .GlobalPreferences (on-disk) explicitly here + -- not by incidental scan-order -- because cfprefsd's in-memory cache is + documented by Apple as the authoritative source: it batches writes to + disk asynchronously, so the on-disk plist can lag the live value by + design (the same reason `killall cfprefsd` is a real troubleshooting + step for "my defaults write didn't take effect"). + """ + domain_names = {d.domain_name for d in domains} + skip_stale_global_preferences_alias = "NSGlobalDomain" in domain_names + items: list[_CuratedItem] = [] for domain in domains: + if domain.domain_name == ".GlobalPreferences" and skip_stale_global_preferences_alias: + continue if domain.domain_name in CURATED_WHOLESALE_DOMAINS: keys_to_scan: list[str] = list(domain.keys) elif domain.domain_name in _GLOBAL_DOMAIN_NAMES: @@ -133,11 +155,27 @@ def _collect_power_items(power_settings: dict[str, str]) -> list[_CuratedItem]: # _collect_power_items()'s section-prefix stripping already is. _POWER_SLEEP_NIX_PATHS = frozenset({"power.sleep.computer", "power.sleep.display", "power.sleep.harddisk"}) -# power.restartAfterPowerFailure / networking.wakeOnLan.enable are booleans; -# pmset reports 0/1 (or Off/On) as a raw string for these. -_POWER_BOOL_NIX_PATHS = frozenset({"power.restartAfterPowerFailure", "networking.wakeOnLan.enable"}) - -_POWER_BOOL_TRUE_VALUES = frozenset({"1", "on", "yes", "true"}) +# power.restartAfterPowerFailure / networking.wakeOnLan.enable are gated by +# nix-darwin's own systemsetup-backed activation scripts -- whether the +# TARGET machine's hardware supports either feature can't be known from a +# SOURCE-machine scan. Confirmed via a real `nix_vm` integration-test +# failure (not review): nix-darwin's modules/system/checks.nix ships +# `restartAfterPowerFailureIsSupported`, which fires whenever +# `config.power.restartAfterPowerFailure != null` -- true OR false, either +# one -- and calls `exit 2` inside the single `set -e` master activation +# script (modules/system/activation-scripts.nix), aborting the ENTIRE +# `darwin-rebuild switch`, not just this one setting. There is no safe +# boolean value; only leaving the option unset (its own `null` default) +# avoids the check. `networking.wakeOnLan.enable` has no equivalent +# nix-darwin pre-check at all, but its own activation script calls the +# same `systemsetup` family with no `|| true` guard under the identical +# `set -e` wrapper -- a documented real-world failure on hardware/drivers +# that report Wake-on-LAN as unsupported (nix-darwin's own option +# docstring: "Battery powered devices may require being connected to +# power."). Never render either option natively -- this generator has no +# mechanism to detect target-hardware capability at generate or apply +# time, so both are downgraded to a manual-report comment instead. +_POWER_HARDWARE_DEPENDENT_NIX_PATHS = frozenset({"power.restartAfterPowerFailure", "networking.wakeOnLan.enable"}) def _coerce_power_native_value(nix_path: str, value: Any) -> Any: @@ -147,11 +185,6 @@ def _coerce_power_native_value(nix_path: str, value: Any) -> Any: except (TypeError, ValueError): return value return "never" if minutes <= 0 else minutes - if nix_path in _POWER_BOOL_NIX_PATHS: - # Positive match, not `not in {false-values}` -- an empty string or an - # unrecognized future pmset value must coerce to False (matching this - # generator's mkDefault-everywhere conservatism), not silently to True. - return str(value).strip().lower() in _POWER_BOOL_TRUE_VALUES return value @@ -164,21 +197,48 @@ def _build_render_context(items: list[_CuratedItem]) -> dict[str, Any]: "power.sleep.computer" nix-darwin option, which has no per-power-source control). Iterating `sorted(native.items())` for the final render list keeps output deterministic regardless of dict insertion order. + + MANUAL_REPORT comments get two further, independent dedup passes for the + same underlying reason (pmset reports some keys under both "AC Power:" + and "Battery Power:"): a `nix_path`-keyed, first-occurrence-wins dedup + for the hardware-dependent power/networking settings (whose comment text + embeds the scanned value, so two different values must still collapse to + one entry), and a final whole-list `dict.fromkeys()` pass for unmapped + fields whose destination string never varies by value (so an + exact-string dedup is sufficient there). """ native: dict[str, Any] = {} custom_user_prefs: dict[str, dict[str, Any]] = {} custom_system_prefs: dict[str, dict[str, Any]] = {} wallpaper_path: str | None = None manual_report_comments: list[str] = [] + reported_hardware_dependent_paths: set[str] = set() for item in items: result = item.result metadata = result.metadata or {} - if result.tier == ClassificationTier.NATIVE and result.nix_path is not None: - value = result.coercion(item.value) if result.coercion else item.value - value = _coerce_power_native_value(result.nix_path, value) - native[result.nix_path] = value + if result.tier == ClassificationTier.NATIVE: + if result.nix_path in _POWER_HARDWARE_DEPENDENT_NIX_PATHS: + # pmset reports some keys (e.g. "autorestart", "womp") under + # both the "AC Power:" and "Battery Power:" sections even + # though the underlying setting isn't actually + # per-power-source -- the same duplication `native`'s + # dict-write already dedupes for NATIVE paths. Dedupe here + # too, or a real scan produces two identical manual-report + # comments for the same nix_path. + if result.nix_path not in reported_hardware_dependent_paths: + reported_hardware_dependent_paths.add(result.nix_path) + manual_report_comments.append( + f"manual report: {result.nix_path} (scanned value {item.value!r}) not applied -- " + "target-hardware support for this setting can't be verified from a source-machine " + "scan; setting it on unsupported hardware aborts the entire darwin-rebuild switch. " + "Verify with `systemsetup -get...` on the target Mac and set manually if supported." + ) + elif result.nix_path is not None: + value = result.coercion(item.value) if result.coercion else item.value + value = _coerce_power_native_value(result.nix_path, value) + native[result.nix_path] = value elif result.tier == ClassificationTier.CUSTOM_PREFS: if item.domain is None or item.key is None: # Every CUSTOM_PREFS item this generator produces is @@ -191,10 +251,26 @@ def _build_render_context(items: list[_CuratedItem]) -> dict[str, Any]: if "wallpaper_path" in metadata: wallpaper_path = metadata["wallpaper_path"] else: + # This generator only implements the wallpaper case for + # ACTIVATION_SCRIPT; any other Tier-3 result (e.g. a + # binary-data plist value) intentionally falls back to a + # manual-report comment instead of a real activation + # script, since synthesizing an arbitrary `defaults write` + # script for binary data is out of this narrow generator's + # scope. manual_report_comments.append(result.destination) elif not metadata.get("skipped"): manual_report_comments.append(result.destination) + # A real, confirmed-on-hardware case: pmset reports some keys (e.g. + # "hibernatemode") under both "AC Power:" and "Battery Power:" with + # different values, but classify_system_setting()'s MANUAL_REPORT + # destination string for an unmapped field doesn't include the value -- + # so two source-prefixed keys for the same unmapped field produce two + # identical comment strings. dict.fromkeys() dedupes exact-duplicate + # strings while preserving first-occurrence order. + manual_report_comments = list(dict.fromkeys(manual_report_comments)) + return { "native_items": [{"nix_path": path, "value": value} for path, value in sorted(native.items())], "custom_user_prefs": custom_user_prefs, diff --git a/tests/generators/test_preferences.py b/tests/generators/test_preferences.py index efb8405..9e141d6 100644 --- a/tests/generators/test_preferences.py +++ b/tests/generators/test_preferences.py @@ -113,6 +113,54 @@ def test_native_dedupes_by_nix_path(self) -> None: matching = [i for i in context["native_items"] if i["nix_path"] == "power.sleep.computer"] assert len(matching) == 1 + def test_unmapped_field_from_two_power_sources_produces_one_manual_report_comment(self) -> None: + """Confirmed on real hardware via `pmset -g custom`: an unmapped key like + 'hibernatemode' can appear under both "AC Power:" and "Battery Power:" + with DIFFERENT values, but classify_system_setting()'s MANUAL_REPORT + destination string for an unmapped field never includes the value -- + two source-prefixed keys must still produce exactly one comment, not two + identical duplicates. + """ + items = _collect_power_items({"ac_power.hibernatemode": "3", "battery_power.hibernatemode": "0"}) + context = _build_render_context(items) + matching = [c for c in context["manual_report_comments"] if "hibernatemode" in c] + assert len(matching) == 1 + + @pytest.mark.parametrize( + "domains", + [ + pytest.param( + [_domain("NSGlobalDomain", {"KeyRepeat": 2}), _domain(".GlobalPreferences", {"KeyRepeat": 6})], + id="nsglobaldomain-first", + ), + pytest.param( + [_domain(".GlobalPreferences", {"KeyRepeat": 6}), _domain("NSGlobalDomain", {"KeyRepeat": 2})], + id="globalpreferences-first", + ), + ], + ) + def test_global_domain_alias_conflict_nsglobaldomain_wins_deterministically( + self, domains: list[PreferencesDomain] + ) -> None: + """Both 'NSGlobalDomain' and '.GlobalPreferences' are guaranteed to appear as + domain_name in the same real scan: PreferencesScanner._discover_cfprefsd_domains() + only skips a cfprefsd domain already in `seen`, and `seen` is populated from + on-disk plist file *stems* -- ".GlobalPreferences.plist"'s stem never matches the + literal string "NSGlobalDomain" that `defaults domains` reports, so any Mac with + a `.GlobalPreferences.plist` (virtually all of them) produces both. When they + disagree, NSGlobalDomain (cfprefsd, live) must win over .GlobalPreferences + (on-disk) -- Apple's own documentation states cfprefsd's in-memory cache is + authoritative and the on-disk plist is only asynchronously, eventually + reconciled with it. This must hold regardless of scan/list order -- parametrized + both ways to prove it's not an incidental artifact of iteration order. + """ + items = _collect_preference_items(domains) + context = _build_render_context(items) + + matching = [i for i in context["native_items"] if i["nix_path"] == "system.defaults.NSGlobalDomain.KeyRepeat"] + assert len(matching) == 1 + assert matching[0]["value"] == 2 + def test_power_sleep_zero_coerces_to_never_not_integer_zero(self) -> None: """nix-darwin's power.sleep.* type is `null | positive-int | "never"` -- confirmed via a real `nix build` failure: the integer 0 isn't itself a @@ -130,24 +178,59 @@ def test_power_sleep_nonzero_coerces_to_int(self) -> None: assert item["value"] == 10 assert isinstance(item["value"], int) - def test_power_boolean_setting_coerces_from_raw_string(self) -> None: - items = _collect_power_items({"ac_power.autorestart": "0", "ac_power.womp": "1"}) + @pytest.mark.parametrize( + "power_settings", + [ + pytest.param({"ac_power.autorestart": "0", "ac_power.womp": "1"}, id="typical-values"), + pytest.param({"ac_power.autorestart": "", "ac_power.womp": "some-future-value"}, id="edge-case-values"), + ], + ) + def test_power_hardware_dependent_settings_never_render_as_native(self, power_settings: dict[str, str]) -> None: + """Confirmed via a real `nix_vm` integration-test failure: nix-darwin's own + modules/system/checks.nix aborts the ENTIRE `darwin-rebuild switch` whenever + `power.restartAfterPowerFailure` is set at all (true OR false) on hardware that + doesn't support it, and `networking.wakeOnLan.enable` carries the same + unsupported-hardware risk with no nix-darwin guard at all. Neither can be + safely auto-applied from a source-machine scan -- both must route to a + manual-report comment instead of `context["native_items"]`, regardless of the + scanned value (classify_system_setting() decides tier/nix_path purely from + field_name, never from value, so this holds for typical and edge-case values + alike -- parametrized rather than duplicated as two near-identical tests). + """ + items = _collect_power_items(power_settings) context = _build_render_context(items) - by_path = {i["nix_path"]: i["value"] for i in context["native_items"]} - assert by_path["power.restartAfterPowerFailure"] is False - assert by_path["networking.wakeOnLan.enable"] is True - - def test_power_boolean_setting_defaults_false_for_unrecognized_value(self) -> None: - """A positive match against known true-values, not `not in {false-values}` -- - an empty string or an unrecognized future pmset value must coerce to - False, matching this generator's mkDefault-everywhere conservatism, - not silently default to True. + native_paths = {i["nix_path"] for i in context["native_items"]} + assert "power.restartAfterPowerFailure" not in native_paths + assert "networking.wakeOnLan.enable" not in native_paths + assert any("power.restartAfterPowerFailure" in c for c in context["manual_report_comments"]) + assert any("networking.wakeOnLan.enable" in c for c in context["manual_report_comments"]) + + def test_power_hardware_dependent_settings_dedupe_across_power_sources(self) -> None: + """pmset reports `autorestart`/`womp` under both the "AC Power:" and + "Battery Power:" sections even though the underlying setting isn't + actually per-power-source -- two source-prefixed keys resolving to the + same nix_path must produce exactly one manual-report comment, not two, + mirroring the dedup NATIVE items already get via dict-write. Uses + DIFFERING values across sources (confirmed real via `pmset -g custom` on + real hardware, which reports different autorestart/womp values per + section) specifically because the rendered comment embeds the scanned + value -- a naive whole-list string dedup would NOT catch two differing + values for the same nix_path, so this proves the nix_path-keyed dedup + mechanism itself, not just incidental string equality. """ - items = _collect_power_items({"ac_power.autorestart": "", "ac_power.womp": "some-future-value"}) + items = _collect_power_items( + { + "ac_power.autorestart": "0", + "battery_power.autorestart": "1", + "ac_power.womp": "1", + "battery_power.womp": "0", + } + ) context = _build_render_context(items) - by_path = {i["nix_path"]: i["value"] for i in context["native_items"]} - assert by_path["power.restartAfterPowerFailure"] is False - assert by_path["networking.wakeOnLan.enable"] is False + restart_comments = [c for c in context["manual_report_comments"] if "power.restartAfterPowerFailure" in c] + wol_comments = [c for c in context["manual_report_comments"] if "networking.wakeOnLan.enable" in c] + assert len(restart_comments) == 1 + assert len(wol_comments) == 1 def test_custom_prefs_grouped_by_domain_and_key(self) -> None: domains = [_domain("com.apple.symbolichotkeys", {"AppleSymbolicHotKeys": {"32": {"enabled": 0}}})] @@ -193,6 +276,25 @@ def test_wallpaper_activation_script_extracted_from_metadata(self) -> None: context = _build_render_context([_CuratedItem(value=Path("/x"), result=result)]) assert context["wallpaper_path"] == "/System/Library/Desktop Pictures/The Cliffs.heic" + def test_binary_data_activation_script_without_wallpaper_falls_back_to_manual_report(self) -> None: + """classify_preference's binary-sentinel precheck routes a `` value to + ACTIVATION_SCRIPT with metadata {command_type, domain, key, value_type, value} -- no + "wallpaper_path" key. _build_render_context's ACTIVATION_SCRIPT branch only extracts + wallpaper_path; this generator has no support for rendering an arbitrary `defaults + write` activation script, so any other ACTIVATION_SCRIPT item must fall back to a + manual-report comment instead of being silently dropped. + """ + domains = [_domain("com.apple.dock", {"some-binary-pref": ""})] + items = _collect_preference_items(domains) + assert items[0].result.tier == ClassificationTier.ACTIVATION_SCRIPT + assert "wallpaper_path" not in (items[0].result.metadata or {}) + + context = _build_render_context(items) + assert context["manual_report_comments"] == [ + "activationScripts: defaults write for com.apple.dock some-binary-pref (binary data)" + ] + assert context["wallpaper_path"] is None + class TestGeneratePreferences: def test_missing_preferences_domain_returns_empty_module_fallback(self) -> None: @@ -303,7 +405,7 @@ def test_render_is_valid_nix(require_nix_instantiate: None, tmp_path: Path) -> N ] system = SystemConfig( hostname="h", - power_settings={"ac_power.sleep": "0"}, + power_settings={"ac_power.sleep": "0", "ac_power.autorestart": "0", "ac_power.womp": "1"}, wallpaper_path=Path("/System/Library/Desktop Pictures/The Cliffs.heic"), ) state = _state(preferences=PreferencesResult(domains=domains), system=system) @@ -312,6 +414,14 @@ def test_render_is_valid_nix(require_nix_instantiate: None, tmp_path: Path) -> N module_path = tmp_path / "preferences.nix" module_path.write_text(rendered) + # The two hardware-dependent settings' manual-report comment embeds the + # scanned value via !r -- confirm it actually rendered (not silently + # dropped) before the nix-instantiate check below, so this test would + # fail loudly if that branch stopped firing rather than just passing + # trivially on an empty comment section. + assert "power.restartAfterPowerFailure" in rendered + assert "networking.wakeOnLan.enable" in rendered + result = subprocess.run( # noqa: S603 ["nix-instantiate", "--parse", str(module_path)], # noqa: S607 capture_output=True, From ade051a4f6d3ae95440bb82b7c606aed3e4e1e47 Mon Sep 17 00:00:00 2001 From: Will Gordon Date: Sun, 16 Aug 2026 18:19:00 -0400 Subject: [PATCH 20/35] fix(scanners): percent-encodes wallpaper db path before building sqlite URI An unescaped '?' or '#' in the desktoppicture.db path (e.g. from an unusual macOS username) would be misparsed as the start of the file: URI's query string or fragment, silently truncating the path. Percent-encoding via urllib.parse.quote(path, safe="/") prevents this while still round-tripping through sqlite3's URI decoder correctly. Also downgrades the two soft-failure log calls in this method from warning to debug, matching this file's existing convention for every other best-effort scanner capability (missing file, corrupt db, and schema mismatch are all expected outcomes, not warning-worthy ones). --- src/mac2nix/scanners/system_scanner.py | 11 ++++++++--- tests/scanners/test_system_scanner.py | 16 ++++++++++++++++ 2 files changed, 24 insertions(+), 3 deletions(-) diff --git a/src/mac2nix/scanners/system_scanner.py b/src/mac2nix/scanners/system_scanner.py index 7491983..8392ac8 100644 --- a/src/mac2nix/scanners/system_scanner.py +++ b/src/mac2nix/scanners/system_scanner.py @@ -10,6 +10,7 @@ from datetime import UTC, datetime from pathlib import Path from typing import Any +from urllib.parse import quote from mac2nix.models.system import ( ICloudState, @@ -585,14 +586,18 @@ def _get_wallpaper_path(self) -> Path | None: # sqlite3.Connection's own context manager only commits/rolls back # the pending transaction on exit -- it does not close the # connection or its file descriptor. contextlib.closing() does. - with contextlib.closing(sqlite3.connect(f"file:{db_path}?mode=ro", uri=True)) as conn: + # The path must be percent-encoded before being embedded in a + # file: URI -- an unescaped '?' or '#' in the path would otherwise + # be misparsed as the start of the URI's query string/fragment. + db_uri = quote(str(db_path), safe="/") + with contextlib.closing(sqlite3.connect(f"file:{db_uri}?mode=ro", uri=True)) as conn: row = conn.execute(_WALLPAPER_QUERY).fetchone() except (sqlite3.Error, OSError) as exc: - logger.warning("Could not read desktop wallpaper from %s: %s", db_path, exc) + logger.debug("Could not read desktop wallpaper from %s: %s", db_path, exc) return None if not row or not row[0]: - logger.warning( + logger.debug( "desktoppicture.db query returned no matching row (expected a " "'preferences' row with key=1 pointing to an absolute-path 'data' " "value) -- wallpaper_path will be unset" diff --git a/tests/scanners/test_system_scanner.py b/tests/scanners/test_system_scanner.py index b6752f7..fb46457 100644 --- a/tests/scanners/test_system_scanner.py +++ b/tests/scanners/test_system_scanner.py @@ -936,6 +936,22 @@ def test_wallpaper_path_from_real_schema(self, tmp_path: Path) -> None: assert result == Path("/System/Library/Desktop Pictures/The Cliffs.heic") + def test_home_path_with_uri_reserved_characters_still_resolves(self, tmp_path: Path) -> None: + """A literal '?' or '#' in the path (e.g. an unusual macOS username) must not be + misparsed as the start of the file: URI's query string/fragment -- without + percent-encoding, sqlite3 would silently truncate the path there and either + fail to open the real db or open the wrong location. + """ + home = tmp_path / "AC?DC#1" + home.mkdir() + db_path = home / "Library" / "Application Support" / "Dock" / "desktoppicture.db" + _write_wallpaper_db(db_path, [(1, "/System/Library/Desktop Pictures/The Cliffs.heic")]) + + with patch("mac2nix.scanners.system_scanner.Path.home", return_value=home): + result = SystemScanner()._get_wallpaper_path() + + assert result == Path("/System/Library/Desktop Pictures/The Cliffs.heic") + def test_most_recently_written_path_wins(self, tmp_path: Path) -> None: db_path = self._db_path(tmp_path) _write_wallpaper_db( From 9208f144bc86c66194e1dd4b3a3b608e0ed6aad5 Mon Sep 17 00:00:00 2001 From: Will Gordon Date: Sun, 16 Aug 2026 18:19:29 -0400 Subject: [PATCH 21/35] fix(vm): excludes dev-only content from copied user flakes, documents source risk Validator.validate()'s first step copied the caller's real flake directory into the VM with no exclusion -- .git (full history, sometimes with secrets committed before later encryption) and .env were copied wholesale, right before an unpinned re-scan source runs in that same VM, which also has outbound network access (TartVMManager configures a public DNS resolver). A compromised upstream main would have had both read access to the copied flake and a network path to exfiltrate it. Fixed with a new _USER_FLAKE_EXCLUDE = frozenset({".git", ".env"}) -- deliberately narrower than the existing _LOCAL_SOURCE_EXCLUDE, which is scoped to mac2nix's own dev-tree conventions ("data", "hack" are this project's own directory names, not general nix-darwin flake conventions). Reusing the broader set for an arbitrary caller's real flake risked silently dropping a legitimately-named user directory. _LOCAL_SOURCE_EXCLUDE remains unchanged for _scan_vm()'s own local-source-checkout copy. Also rewrites the SECURITY comment on _DEFAULT_MAC2NIX_SOURCE to state the risk accurately (the VM is not network-isolated, and "the user owns the upstream repo" only holds for the mac2nix maintainer) and raises the unpinned-source disclosure from a debug log to a warning, since it's a real supply-chain fact the caller should see by default. --- src/mac2nix/vm/validator.py | 67 +++++++++++++++++++++++++++++++++++-- tests/vm/test_validator.py | 65 ++++++++++++++++++++++++----------- 2 files changed, 111 insertions(+), 21 deletions(-) diff --git a/src/mac2nix/vm/validator.py b/src/mac2nix/vm/validator.py index d36386a..988a10a 100644 --- a/src/mac2nix/vm/validator.py +++ b/src/mac2nix/vm/validator.py @@ -191,14 +191,55 @@ class Validator: # Default preserves today's `mac2nix validate` CLI behavior exactly — only # this plan's own nix_vm tests pass a local checkout path instead. + # + # SECURITY: intentionally unpinned (no ?ref=/?rev=). `nix run` resolves this + # to whatever upstream/main's HEAD is at call time and executes it inside the + # Tart VM. This is a known, deliberate gap, not an oversight — it cannot be + # fixed by hardcoding a commit SHA here: `upstream/main` has no flake.nix at + # all today (this PR's own root flake.nix is the first commit that will ever + # add one), so pinning to any commit that exists right now would make + # `nix run` fail immediately with "not part of a flake". There are also no + # tags/releases on gordon-code/mac2nix yet to pin to instead. + # + # This is NOT fully mitigated. The VM is not network-isolated (see + # TartVMManager's DNS setup) and, for a real `validate()` call, + # `_copy_flake_to_vm(flake_path, exclude=_USER_FLAKE_EXCLUDE)` already + # copies the caller's real flake into the same VM *before* this unpinned + # source ever runs there — so a compromised upstream `main` would have + # both read access to that flake and outbound network access to + # exfiltrate it. "The user owns the upstream repo" only holds for the + # mac2nix maintainer; for any other user of `mac2nix validate` (this + # tool is designed to be generic, portable boilerplate, not + # maintainer-only), the default trusts a third-party repository they do + # not control. Sandboxing the VM limits the blast radius to that VM's + # own lifetime — it does not prevent exfiltration during that lifetime. + # TODO: once a tagged release exists on gordon-code/mac2nix (post-merge of + # this PR), pin this default to that tag + # (`github:gordon-code/mac2nix?ref=vX.Y.Z`) instead of a floating branch ref. + # Meanwhile, the only real mitigation is `--mac2nix-source` (a local + # checkout, or any other flake ref/rev/tag) to opt out of the default. _DEFAULT_MAC2NIX_SOURCE = "github:gordon-code/mac2nix" # Directories excluded when SCPing a local mac2nix checkout into the VM — # dev-machine-only content (VCS history, secrets, scan data, project memory, # a local dev venv/cache that can be tens to hundreds of MB) that has no - # bearing on the package being scanned from inside the VM. + # bearing on the package being scanned from inside the VM. Scoped to + # *this project's own* directory conventions ("data", "hack") -- do not + # reuse for an arbitrary caller-owned flake (see _USER_FLAKE_EXCLUDE + # below), since a real nix-darwin flake could legitimately have a + # top-level directory with either of those names that nix-darwin + # actually needs to build/switch. _LOCAL_SOURCE_EXCLUDE = frozenset({".git", ".env", "data", "hack", ".cache"}) + # Directories excluded when SCPing the *caller's* flake into the VM for + # `validate()` -- deliberately narrower than _LOCAL_SOURCE_EXCLUDE above. + # Only VCS history and secrets are universally unsafe/unneeded regardless + # of what any given nix-darwin flake is structured like; "data"/"hack" + # are this project's own conventions, not a general flake convention, so + # excluding them here could silently drop content nix-darwin actually + # needs from someone else's real flake. + _USER_FLAKE_EXCLUDE = frozenset({".git", ".env"}) + def __init__(self, vm: TartVMManager, mac2nix_source: str = _DEFAULT_MAC2NIX_SOURCE) -> None: self._vm = vm self._mac2nix_source = mac2nix_source @@ -213,7 +254,18 @@ async def validate(self, flake_path: Path, source_state: SystemState) -> Validat build_output = "" try: - await self._copy_flake_to_vm(flake_path) + # Exclude VCS history/secrets -- the caller's real flake directory + # can contain .git (full history, sometimes with secrets + # committed before later encryption) and .env, neither of which + # nix-darwin needs to build/switch. Without this, a compromised + # unpinned _scan_vm() re-scan (see the SECURITY comment on + # _DEFAULT_MAC2NIX_SOURCE below) would have both read access to + # this content inside the VM and outbound network access (see + # TartVMManager's DNS setup) to exfiltrate it. Uses the narrower + # _USER_FLAKE_EXCLUDE, not _LOCAL_SOURCE_EXCLUDE -- this is an + # arbitrary caller-owned flake, not mac2nix's own dev tree, so + # only universally-unsafe names are excluded here. + await self._copy_flake_to_vm(flake_path, exclude=self._USER_FLAKE_EXCLUDE) except VMError as exc: errors.append(f"copy_flake failed: {exc}") return ValidationResult(success=False, fidelity=None, build_output="", errors=errors) @@ -400,6 +452,17 @@ async def _scan_vm(self) -> SystemState: if self._mac2nix_source == self._DEFAULT_MAC2NIX_SOURCE: run_target = self._mac2nix_source + # WARNING (not debug): this is a real supply-chain disclosure the + # caller should see by default, not only under verbose logging -- + # see the SECURITY comment on _DEFAULT_MAC2NIX_SOURCE. + logger.warning( + "Using unpinned default mac2nix source %r — resolves to " + "upstream main's current HEAD at nix-run time and has already " + "received the caller's flake contents in this same VM; " + "override with --mac2nix-source to pin a specific rev/tag " + "once one exists", + run_target, + ) else: await self._copy_flake_to_vm( Path(self._mac2nix_source), diff --git a/tests/vm/test_validator.py b/tests/vm/test_validator.py index 0ecb30d..d3c7f26 100644 --- a/tests/vm/test_validator.py +++ b/tests/vm/test_validator.py @@ -470,7 +470,7 @@ async def _run() -> None: def test_exclude_omits_dev_only_directories(self, tmp_path: Path) -> None: """When exclude is non-empty, only top-level entries not in exclude are copied.""" - for name in (".git", ".env", "data", "hack"): + for name in (".git", ".env", "data", "hack", ".cache"): (tmp_path / name).mkdir() (tmp_path / "flake.nix").touch() @@ -616,7 +616,7 @@ def _success_vm(self, _vm_json: str) -> MagicMock: # We patch async_run_command and from_json separately. return vm - def test_success_returns_validation_result(self) -> None: + def test_success_returns_validation_result(self, tmp_path: Path) -> None: vm = _make_vm(exec_result=(True, "admin", "")) source = _minimal_source_state() vm_state = _base_state(shell=ShellConfig(shell_type="fish")) @@ -627,7 +627,7 @@ async def _run() -> ValidationResult: patch("mac2nix.vm.validator.async_run_command", new=AsyncMock(return_value=(0, "", ""))), patch.object(SystemState, "from_json", return_value=vm_state), ): - return await v.validate(Path("/tmp/flake"), source) + return await v.validate(tmp_path, source) result = asyncio.run(_run()) assert isinstance(result, ValidationResult) @@ -648,7 +648,34 @@ async def _run() -> ValidationResult: assert any("copy_flake" in e for e in result.errors) assert result.fidelity is None - def test_bootstrap_failure_returns_early(self) -> None: + def test_validate_excludes_dev_only_content_from_the_copied_flake(self, tmp_path: Path) -> None: + """The caller's real flake directory can contain .git (full history, + sometimes with secrets committed before later encryption) and .env -- + validate() must scope the copy with _USER_FLAKE_EXCLUDE (deliberately + narrower than _LOCAL_SOURCE_EXCLUDE, which is scoped to mac2nix's own + dev-tree conventions like "data"/"hack" -- an arbitrary caller's real + flake could legitimately have a top-level directory with either name + that nix-darwin actually needs), or .git/.env is copied wholesale into + a VM that also runs an unpinned re-scan source with outbound network + access. + """ + vm = _make_vm(exec_result=(True, "admin", "")) + source = _minimal_source_state() + vm_state = _base_state(shell=ShellConfig(shell_type="fish")) + + async def _run() -> None: + v = Validator(vm) + with ( + patch.object(v, "_copy_flake_to_vm", new=AsyncMock()) as mock_copy, + patch("mac2nix.vm.validator.async_run_command", new=AsyncMock(return_value=(0, "", ""))), + patch.object(SystemState, "from_json", return_value=vm_state), + ): + await v.validate(tmp_path, source) + mock_copy.assert_any_call(tmp_path, exclude=Validator._USER_FLAKE_EXCLUDE) + + asyncio.run(_run()) + + def test_bootstrap_failure_returns_early(self, tmp_path: Path) -> None: # mkdir succeeds, then first exec_command call in bootstrap fails call_count = 0 @@ -666,13 +693,13 @@ async def exec_side_effect(cmd, **_kw): async def _run() -> ValidationResult: v = Validator(vm) with patch("mac2nix.vm.validator.async_run_command", new=AsyncMock(return_value=(0, "", ""))): - return await v.validate(Path("/tmp/flake"), source) + return await v.validate(tmp_path, source) result = asyncio.run(_run()) assert result.success is False assert any("bootstrap" in e for e in result.errors) - def test_rebuild_failure_returns_early(self) -> None: + def test_rebuild_failure_returns_early(self, tmp_path: Path) -> None: # Bootstrap succeeds (Nix reports already installed); rebuild switch fails. # Keyed off command content, not call position, so it's insensitive to how # many exec_command calls bootstrap itself makes. @@ -691,13 +718,13 @@ async def exec_side_effect(cmd, **_kw): async def _run() -> ValidationResult: v = Validator(vm) with patch("mac2nix.vm.validator.async_run_command", new=AsyncMock(return_value=(0, "", ""))): - return await v.validate(Path("/tmp/flake"), source) + return await v.validate(tmp_path, source) result = asyncio.run(_run()) assert result.success is False assert any("nix-darwin" in e or "darwin-rebuild" in e for e in result.errors) - def test_scan_failure_returns_early(self) -> None: + def test_scan_failure_returns_early(self, tmp_path: Path) -> None: # All VM exec_commands succeed, but mac2nix scan fails call_count = 0 @@ -717,13 +744,13 @@ async def exec_side_effect(cmd, **_kw): async def _run() -> ValidationResult: v = Validator(vm) with patch("mac2nix.vm.validator.async_run_command", new=AsyncMock(return_value=(0, "", ""))): - return await v.validate(Path("/tmp/flake"), source) + return await v.validate(tmp_path, source) result = asyncio.run(_run()) assert result.success is False assert any("scan" in e for e in result.errors) - def test_scan_parse_failure_returns_error(self) -> None: + def test_scan_parse_failure_returns_error(self, tmp_path: Path) -> None: """If SCP back succeeds but JSON parse fails, validate returns failure.""" vm = _make_vm(exec_result=(True, "admin", "")) source = _minimal_source_state() @@ -734,13 +761,13 @@ async def _run() -> ValidationResult: patch("mac2nix.vm.validator.async_run_command", new=AsyncMock(return_value=(0, "", ""))), patch.object(SystemState, "from_json", side_effect=ValueError("bad json")), ): - return await v.validate(Path("/tmp/flake"), source) + return await v.validate(tmp_path, source) result = asyncio.run(_run()) assert result.success is False assert any("scan" in e for e in result.errors) - def test_success_build_output_captured(self) -> None: + def test_success_build_output_captured(self, tmp_path: Path) -> None: vm = _make_vm() vm.exec_command = AsyncMock(return_value=(True, "build output text", "")) source = _minimal_source_state() @@ -752,14 +779,14 @@ async def _run() -> ValidationResult: patch("mac2nix.vm.validator.async_run_command", new=AsyncMock(return_value=(0, "", ""))), patch.object(SystemState, "from_json", return_value=vm_state), ): - return await v.validate(Path("/tmp/flake"), source) + return await v.validate(tmp_path, source) result = asyncio.run(_run()) assert result.success is True # build_output is the combined stdout+stderr from darwin-rebuild assert isinstance(result.build_output, str) - def test_success_fidelity_report_populated(self) -> None: + def test_success_fidelity_report_populated(self, tmp_path: Path) -> None: vm = _make_vm(exec_result=(True, "admin", "")) source = _base_state(shell=ShellConfig(shell_type="fish")) vm_state = _base_state(shell=ShellConfig(shell_type="fish")) @@ -770,14 +797,14 @@ async def _run() -> ValidationResult: patch("mac2nix.vm.validator.async_run_command", new=AsyncMock(return_value=(0, "", ""))), patch.object(SystemState, "from_json", return_value=vm_state), ): - return await v.validate(Path("/tmp/flake"), source) + return await v.validate(tmp_path, source) result = asyncio.run(_run()) assert result.fidelity is not None assert isinstance(result.fidelity, FidelityReport) assert result.fidelity.overall_score == 1.0 - def test_fidelity_reflects_vm_state_difference(self) -> None: + def test_fidelity_reflects_vm_state_difference(self, tmp_path: Path) -> None: """Fidelity < 1.0 when VM state differs from source.""" vm = _make_vm(exec_result=(True, "admin", "")) source = _base_state(shell=ShellConfig(shell_type="fish")) @@ -789,14 +816,14 @@ async def _run() -> ValidationResult: patch("mac2nix.vm.validator.async_run_command", new=AsyncMock(return_value=(0, "", ""))), patch.object(SystemState, "from_json", return_value=vm_state), ): - return await v.validate(Path("/tmp/flake"), source) + return await v.validate(tmp_path, source) result = asyncio.run(_run()) assert result.success is True assert result.fidelity is not None assert result.fidelity.overall_score < 1.0 - def test_scp_result_back_no_ip_returns_scan_error(self) -> None: + def test_scp_result_back_no_ip_returns_scan_error(self, tmp_path: Path) -> None: """If VM has no IP when SCPing result back, scan fails gracefully.""" get_ip_calls = 0 vm = _make_vm() @@ -814,7 +841,7 @@ async def get_ip_side_effect(): async def _run() -> ValidationResult: v = Validator(vm) with patch("mac2nix.vm.validator.async_run_command", new=AsyncMock(return_value=(0, "", ""))): - return await v.validate(Path("/tmp/flake"), source) + return await v.validate(tmp_path, source) result = asyncio.run(_run()) assert result.success is False From ea1bafde83c8ef9755e3e8a7e76262b3c7a2c5c5 Mon Sep 17 00:00:00 2001 From: Will Gordon Date: Sun, 16 Aug 2026 18:19:48 -0400 Subject: [PATCH 22/35] refactor(cli): extracts has_hosts_sentinels() instead of importing private scaffold constants cli.py imported scaffold.py's private _HOSTS_BEGIN/_HOSTS_END sentinel constants directly, the only cross-module import of an underscore-prefixed name anywhere in the codebase. Adds a public has_hosts_sentinels() accessor in scaffold.py, mirroring the existing age_key_path() public-wrapper pattern, and uses it from both add_host() and cli.py's _check_scaffolded_framework() -- removing a literal duplicated boolean check between the two in the process. --- src/mac2nix/cli.py | 8 ++++---- src/mac2nix/generators/scaffold.py | 14 +++++++++++++- tests/generators/test_scaffold.py | 18 +++++++++++++++++- 3 files changed, 34 insertions(+), 6 deletions(-) diff --git a/src/mac2nix/cli.py b/src/mac2nix/cli.py index e01b427..803cd97 100644 --- a/src/mac2nix/cli.py +++ b/src/mac2nix/cli.py @@ -22,7 +22,7 @@ from mac2nix import onepassword from mac2nix.generators import generate_all -from mac2nix.generators.scaffold import _HOSTS_BEGIN, _HOSTS_END, add_host, age_key_path, init_framework +from mac2nix.generators.scaffold import add_host, age_key_path, has_hosts_sentinels, init_framework from mac2nix.models.system_state import SystemState from mac2nix.orchestrator import run_scan from mac2nix.scan_report import ScannerOutcome, ScannerStatus, capture_scanner_logs, get_remediation_hint @@ -392,17 +392,17 @@ def _register_one(current_hostname: str, current_username: str, current_system: def _check_scaffolded_framework(output_dir: Path) -> None: """Matches add_host()'s own scaffolded-framework check in scaffold.py - (shared sentinel constants, both markers) -- kept as a second, + (shared `has_hosts_sentinels()` predicate) -- kept as a second, independent check (not a shared function) since this one must raise click.ClickException while add_host() raises ScaffoldError, but reusing - the sentinel constants avoids the two checks silently drifting apart. + the shared predicate avoids the two checks silently drifting apart. """ flake_path = output_dir / "flake.nix" try: flake_content = flake_path.read_text() if flake_path.is_file() else "" except OSError as exc: raise click.ClickException(f"Failed to read {flake_path}: {exc}") from exc - if _HOSTS_BEGIN not in flake_content or _HOSTS_END not in flake_content: + if not has_hosts_sentinels(flake_content): raise click.ClickException(f"{output_dir} is not a mac2nix-scaffolded framework — run `mac2nix init` first") diff --git a/src/mac2nix/generators/scaffold.py b/src/mac2nix/generators/scaffold.py index 8ccc59b..9ac9e21 100644 --- a/src/mac2nix/generators/scaffold.py +++ b/src/mac2nix/generators/scaffold.py @@ -25,6 +25,18 @@ _HOSTS_BEGIN = "# MAC2NIX:HOSTS:BEGIN" _HOSTS_END = "# MAC2NIX:HOSTS:END" + +def has_hosts_sentinels(flake_content: str) -> bool: + """Return whether *flake_content* contains both MAC2NIX:HOSTS sentinel markers. + + Public accessor so callers outside this module (the CLI) can check + whether a `flake.nix` looks like a mac2nix-scaffolded framework without + importing this module's private `_HOSTS_BEGIN`/`_HOSTS_END` sentinel + constants directly -- mirrors `age_key_path()`'s "public wrapper" pattern. + """ + return _HOSTS_BEGIN in flake_content and _HOSTS_END in flake_content + + _META_FILENAME = ".mac2nix-meta.json" _STATE_FILENAME = ".mac2nix-state.json" @@ -421,7 +433,7 @@ def add_host( msg = f"{output_dir} is not a mac2nix-scaffolded framework — run `mac2nix init` first" raise ScaffoldError(msg) flake_content = flake_path.read_text() - if _HOSTS_BEGIN not in flake_content or _HOSTS_END not in flake_content: + if not has_hosts_sentinels(flake_content): msg = f"{output_dir} is not a mac2nix-scaffolded framework — run `mac2nix init` first" raise ScaffoldError(msg) diff --git a/tests/generators/test_scaffold.py b/tests/generators/test_scaffold.py index 5eaed33..e62c29b 100644 --- a/tests/generators/test_scaffold.py +++ b/tests/generators/test_scaffold.py @@ -17,7 +17,14 @@ from mac2nix.generators import Mac2NixError from mac2nix.generators import scaffold as scaffold_module -from mac2nix.generators.scaffold import ScaffoldError, add_host, age_key_path, generate_age_key, init_framework +from mac2nix.generators.scaffold import ( + ScaffoldError, + add_host, + age_key_path, + generate_age_key, + has_hosts_sentinels, + init_framework, +) from tests._scaffold_helpers import _has_add_host_crypto_deps, _has_age_keygen, _redirect_age_keys _EXPECTED_FRAMEWORK_FILES = [ @@ -145,6 +152,15 @@ def test_age_key_path_matches_internal_construction() -> None: assert age_key_path("alice") == Path("/Users/alice") / ".config" / "sops" / "age" / "keys.txt" +def test_has_hosts_sentinels_true_when_both_markers_present() -> None: + assert has_hosts_sentinels("# MAC2NIX:HOSTS:BEGIN\n...\n# MAC2NIX:HOSTS:END\n") is True + + +def test_has_hosts_sentinels_false_when_a_marker_is_missing() -> None: + assert has_hosts_sentinels("# MAC2NIX:HOSTS:BEGIN\n...\n") is False + assert has_hosts_sentinels("not a mac2nix flake at all") is False + + # --------------------------------------------------------------------------- # add_host() — file/directory orchestration and rollback (crypto mocked) # --------------------------------------------------------------------------- From d89a295c3ed537c6020088c0b9fbba06a7d09e26 Mon Sep 17 00:00:00 2001 From: Will Gordon Date: Sun, 16 Aug 2026 18:20:17 -0400 Subject: [PATCH 23/35] test(generators): adds coverage for generate_all()'s error paths and idempotency guarantees - Asserts preferences.nix survives on disk when a later sentinel-parsing failure aborts generate_all(), verifying the documented partial-write guarantee - Adds a corrupt .mac2nix-meta.json test mirroring the existing test_corrupt_state_file_handled_gracefully coverage for add_host()'s analogous mechanism - Adds a CLI-level test driving a GenerateError through to a clean ClickException, exercising the one exception-wrap branch no existing test reached - Tightens a CLI happy-path assertion from a bare "preferences" substring (which passes identically for both the success and skip code paths) to the literal "Generated: preferences" success string - Updates test_generate_integration.py's fixture comments to describe the corrected MANUAL_REPORT routing for hardware-dependent power/networking settings --- tests/cli/test_generate.py | 32 ++++++++++++++++++- tests/generators/test_generate_all.py | 26 +++++++++++++++ tests/generators/test_generate_integration.py | 20 ++++++------ 3 files changed, 68 insertions(+), 10 deletions(-) diff --git a/tests/cli/test_generate.py b/tests/cli/test_generate.py index 04bc1d2..5e30af7 100644 --- a/tests/cli/test_generate.py +++ b/tests/cli/test_generate.py @@ -49,7 +49,7 @@ def test_produces_preferences_and_prints_summary(self, tmp_path: Path) -> None: assert result.exit_code == 0, result.output assert (output_dir / "hosts" / "darwin" / "myhost" / "preferences.nix").is_file() - assert "preferences" in result.output + assert "Generated: preferences" in result.output def test_non_scaffolded_directory_fails_and_writes_nothing(self, tmp_path: Path) -> None: output_dir = tmp_path / "not-a-repo" @@ -205,3 +205,33 @@ def test_inline_scan_runtime_error_fails_cleanly(self, tmp_path: Path) -> None: assert result.exit_code != 0 assert "orchestrator failed" in result.output assert not (output_dir / "hosts" / "darwin" / "myhost" / "preferences.nix").exists() + + def test_generate_all_failure_wraps_as_click_exception(self, tmp_path: Path) -> None: + """generate_all() can raise after _check_host_registered has already passed -- + e.g. GenerateError from corrupted sentinel markers (mirrors + test_generate_all.py::test_missing_sentinel_markers_raise_clear_generate_error). + This must surface as a clean ClickException through the CLI, not a raw traceback. + """ + output_dir = tmp_path / "repo" + init_framework(output_dir) + host_dir = _register_fake_host(output_dir, "myhost") + config_path = host_dir / "configuration.nix" + stripped = config_path.read_text().replace( + " # MAC2NIX:GENERATE:BEGIN -- generated by `mac2nix generate`; do not edit by hand\n" + " # MAC2NIX:GENERATE:END\n", + "", + ) + config_path.write_text(stripped) + scan_file = tmp_path / "scan.json" + _write_scan_file(scan_file) + + runner = CliRunner() + result = runner.invoke( + main, + ["generate", str(output_dir), "--hostname", "myhost", "--scan-file", str(scan_file)], + ) + + assert result.exit_code != 0 + assert result.exc_info is not None + assert result.exc_info[0] is SystemExit + assert "sentinel" in result.output diff --git a/tests/generators/test_generate_all.py b/tests/generators/test_generate_all.py index a889e66..9f991e4 100644 --- a/tests/generators/test_generate_all.py +++ b/tests/generators/test_generate_all.py @@ -129,3 +129,29 @@ def test_missing_sentinel_markers_raise_clear_generate_error(self, tmp_path: Pat with pytest.raises(GenerateError, match="sentinel"): generate_all(_full_state(), output_dir, "myhost", {"preferences"}) + + # generate_all()'s documented partial-failure guarantee: a domain + # generator that already ran and wrote its file before the later + # sentinel-parsing failure must have that file survive on disk. + assert (host_dir / "preferences.nix").exists() + + def test_corrupt_meta_file_handled_gracefully(self, tmp_path: Path, caplog: pytest.LogCaptureFixture) -> None: + """A hand-corrupted `.mac2nix-meta.json` must not crash generate_all() or + spuriously warn -- mirrors test_scaffold.py's + test_corrupt_state_file_handled_gracefully for add_host()'s analogous + `.mac2nix-state.json` mechanism. + """ + output_dir = tmp_path / "repo" + host_dir = _register_fake_host(output_dir, "myhost") + + generate_all(_full_state(), output_dir, "myhost", {"preferences"}) + + (host_dir / ".mac2nix-meta.json").write_text("{not valid json") + + with caplog.at_level(logging.WARNING): + result = generate_all(_full_state(), output_dir, "myhost", {"preferences"}) + + assert not caplog.records + assert result.ran == {"preferences"} + assert result.skipped == {} + assert (host_dir / "preferences.nix").exists() diff --git a/tests/generators/test_generate_integration.py b/tests/generators/test_generate_integration.py index 8e99bdb..07156a5 100644 --- a/tests/generators/test_generate_integration.py +++ b/tests/generators/test_generate_integration.py @@ -34,13 +34,15 @@ def _realistic_state() -> SystemState: - """Covers all four render buckets: NATIVE (dock, plus power settings -- - including a POWER_SETTING_MAP-mapped sleep/boolean key, not just an - unmapped one -- a real `nix build` failure caught power.sleep.* needing - `null | positive-int | "never"`, not a raw scanned string, so this - fixture must actually exercise that coercion path), CUSTOM_PREFS - (symbolichotkeys-shaped), ACTIVATION_SCRIPT (wallpaper), and a - non-skipped MANUAL_REPORT (an unmapped pmset key). + """Covers all four render buckets: NATIVE (dock, plus a POWER_SETTING_MAP-mapped + sleep key, not just an unmapped one -- a real `nix build` failure caught + power.sleep.* needing `null | positive-int | "never"`, not a raw scanned + string, so this fixture must actually exercise that coercion path), + CUSTOM_PREFS (symbolichotkeys-shaped), ACTIVATION_SCRIPT (wallpaper), and + non-skipped MANUAL_REPORT entries (an unmapped pmset key, plus the two + hardware-dependent power/networking keys that a real `nix_vm` test + failure proved unsafe to auto-apply -- see preferences.py's + `_POWER_HARDWARE_DEPENDENT_NIX_PATHS`). """ domains = [ PreferencesDomain(domain_name="com.apple.dock", keys={"tilesize": 48}), @@ -54,8 +56,8 @@ def _realistic_state() -> SystemState: power_settings={ "ac_power.sleep": "0", # POWER_SETTING_MAP-mapped -> power.sleep.computer ("never") "battery_power.displaysleep": "10", # POWER_SETTING_MAP-mapped -> power.sleep.display (int) - "ac_power.womp": "1", # POWER_SETTING_MAP-mapped -> networking.wakeOnLan.enable (bool) - "ac_power.autorestart": "0", # POWER_SETTING_MAP-mapped -> power.restartAfterPowerFailure (bool) + "ac_power.womp": "1", # -> networking.wakeOnLan.enable (MANUAL_REPORT, hardware-dependent) + "ac_power.autorestart": "0", # -> power.restartAfterPowerFailure (MANUAL_REPORT, hardware-dependent) "ac_power.hibernatemode": "3", # not in POWER_SETTING_MAP -> MANUAL_REPORT }, wallpaper_path=Path("/System/Library/Desktop Pictures/The Cliffs.heic"), From 2b8f83d2e9832e8fa3e4fb683b6a74e30704d4c5 Mon Sep 17 00:00:00 2001 From: Will Gordon Date: Sun, 16 Aug 2026 18:20:50 -0400 Subject: [PATCH 24/35] docs(contributing): adds tests scope to conventional commit types A prior commit on this branch used scope "tests" (refactor(tests): extracts shared generate test helper), which wasn't in the documented scope list. Extends the list to match real, already-in-use usage rather than rewording pushed history. --- CONTRIBUTING.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index d20c5df..22c49f4 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -39,6 +39,7 @@ This project follows [Conventional Commits](https://www.conventionalcommits.org/ | `drift` | Configuration drift detection | | `deps` | Dependency updates | | `ci` | CI/CD pipeline | +| `tests` | Test-only infrastructure/helpers | ### Examples From 03d22e816769c2051e8fecedea5e30b3d552cdc0 Mon Sep 17 00:00:00 2001 From: Will Gordon Date: Wed, 19 Aug 2026 12:32:04 -0400 Subject: [PATCH 25/35] fix(generators): guards generate_all against missing config --- src/mac2nix/generators/__init__.py | 35 ++++++++++++++++++--------- tests/generators/test_generate_all.py | 34 ++++++++++++++++++++++++++ 2 files changed, 57 insertions(+), 12 deletions(-) diff --git a/src/mac2nix/generators/__init__.py b/src/mac2nix/generators/__init__.py index 71ee1d7..3827ea8 100644 --- a/src/mac2nix/generators/__init__.py +++ b/src/mac2nix/generators/__init__.py @@ -4,7 +4,8 @@ Scoped to the `preferences` domain in this PR. Tasks 7 (shell) and 6 (homebrew) each extend this module with one more sibling `if` block and one -more `(filename, import_line)` pair -- never restructuring the mechanism. +more `(domain, filename, import_line)` triple -- never restructuring the +mechanism. """ from __future__ import annotations @@ -53,10 +54,14 @@ class GenerateResult: _GENERATE_BEGIN = "# MAC2NIX:GENERATE:BEGIN" _GENERATE_END = "# MAC2NIX:GENERATE:END" -# (filename, import_line) pairs, checked via on-disk existence -- extended by -# Task 7 (shell.nix) and Task 6 (homebrew-packages.nix), never restructured. -_GENERATED_IMPORT_FILES: list[tuple[str, str]] = [ - ("preferences.nix", "./preferences.nix"), +# (domain, filename, import_line) triples -- `domain` is this module's single +# source of truth for "which domains actually run" (drives both the +# unrecognized-domain check and the on-disk import regeneration below). +# Extended by Task 7 (adds ("shell", "shell.nix", "./shell.nix")) and Task 6 +# (adds ("homebrew", "homebrew-packages.nix", "./homebrew-packages.nix")), +# never restructured. +_GENERATED_IMPORT_FILES: list[tuple[str, str, str]] = [ + ("preferences", "preferences.nix", "./preferences.nix"), ] @@ -96,9 +101,19 @@ def _regenerate_host_imports(output_dir: Path, hostname: str) -> None: this specific `generate_all()` invocation. This is what makes `generate` safely repeatable with a narrower `--domains` subset: an already-present, untouched file is never dropped from the imports list. + + Raises :exc:`GenerateError` if configuration.nix is missing or is not a + regular file, or if it's missing its sentinel markers. """ host_dir = output_dir / "hosts" / "darwin" / hostname config_path = host_dir / "configuration.nix" + if not config_path.is_file(): + msg = ( + f"{config_path} is missing or is not a regular file -- the host directory is " + "registered but its configuration.nix isn't usable; restore it (see " + "templates/scaffold/hosts/darwin/configuration.nix) before running generate again" + ) + raise GenerateError(msg) content = config_path.read_text() # Anchor to the end of the BEGIN sentinel's own line -- it also carries a @@ -118,7 +133,7 @@ def _regenerate_host_imports(output_dir: Path, hostname: str) -> None: old_inner = content[begin_marker_end:end_marker_start] present_imports = [ - import_line for filename, import_line in _GENERATED_IMPORT_FILES if (host_dir / filename).exists() + import_line for _domain, filename, import_line in _GENERATED_IMPORT_FILES if (host_dir / filename).exists() ] new_inner = f" imports = [ {' '.join(present_imports)} ];\n " if present_imports else " " @@ -162,12 +177,8 @@ def generate_all(system_state: SystemState, output_dir: Path, hostname: str, dom else: skipped["preferences"] = "not scanned" - # Extend this literal set (never restructure the mechanism) whenever a new - # sibling `if` block is added above -- Task 7 (shell) widens this to - # {"preferences", "shell"}, then Task 6 (homebrew) to - # {"preferences", "shell", "homebrew"}. Forgetting this line makes a - # newly-supported domain wrongly report as `unrecognized`. - unrecognized = domains - {"preferences"} + known_domains = {domain for domain, _filename, _import_line in _GENERATED_IMPORT_FILES} + unrecognized = domains - known_domains _regenerate_host_imports(output_dir, hostname) diff --git a/tests/generators/test_generate_all.py b/tests/generators/test_generate_all.py index 9f991e4..f86c560 100644 --- a/tests/generators/test_generate_all.py +++ b/tests/generators/test_generate_all.py @@ -135,6 +135,40 @@ def test_missing_sentinel_markers_raise_clear_generate_error(self, tmp_path: Pat # sentinel-parsing failure must have that file survive on disk. assert (host_dir / "preferences.nix").exists() + def test_missing_configuration_nix_raises_clear_generate_error(self, tmp_path: Path) -> None: + """A registered host whose configuration.nix was deleted must raise a + purpose-written GenerateError, not a raw `FileNotFoundError`. + """ + output_dir = tmp_path / "repo" + host_dir = _register_fake_host(output_dir, "myhost") + config_path = host_dir / "configuration.nix" + config_path.unlink() + + with pytest.raises(GenerateError, match="is missing or is not a regular file"): + generate_all(_full_state(), output_dir, "myhost", {"preferences"}) + + # generate_all()'s documented partial-failure guarantee: a domain + # generator that already ran and wrote its file before the later + # missing-configuration.nix failure must have that file survive on disk. + assert (host_dir / "preferences.nix").exists() + + def test_configuration_nix_replaced_with_directory_raises_clear_generate_error(self, tmp_path: Path) -> None: + """A registered host whose configuration.nix was replaced with a directory + must raise the same purpose-written GenerateError, not a raw + `IsADirectoryError` -- `is_file()` is False for both this and the + deleted-file case, so both must be guarded identically. + """ + output_dir = tmp_path / "repo" + host_dir = _register_fake_host(output_dir, "myhost") + config_path = host_dir / "configuration.nix" + config_path.unlink() + config_path.mkdir() + + with pytest.raises(GenerateError, match="is missing or is not a regular file"): + generate_all(_full_state(), output_dir, "myhost", {"preferences"}) + + assert (host_dir / "preferences.nix").exists() + def test_corrupt_meta_file_handled_gracefully(self, tmp_path: Path, caplog: pytest.LogCaptureFixture) -> None: """A hand-corrupted `.mac2nix-meta.json` must not crash generate_all() or spuriously warn -- mirrors test_scaffold.py's From 578d4f9c381071c012d7b442efc423d858f1857d Mon Sep 17 00:00:00 2001 From: Will Gordon Date: Wed, 19 Aug 2026 12:32:44 -0400 Subject: [PATCH 26/35] fix(vm): records SHA-pin decision, tests unpinned-source warning --- src/mac2nix/vm/validator.py | 13 ++++++++++--- tests/vm/test_validator.py | 11 +++++++++-- 2 files changed, 19 insertions(+), 5 deletions(-) diff --git a/src/mac2nix/vm/validator.py b/src/mac2nix/vm/validator.py index 988a10a..4eabddb 100644 --- a/src/mac2nix/vm/validator.py +++ b/src/mac2nix/vm/validator.py @@ -213,9 +213,16 @@ class Validator: # maintainer-only), the default trusts a third-party repository they do # not control. Sandboxing the VM limits the blast radius to that VM's # own lifetime — it does not prevent exfiltration during that lifetime. - # TODO: once a tagged release exists on gordon-code/mac2nix (post-merge of - # this PR), pin this default to that tag - # (`github:gordon-code/mac2nix?ref=vX.Y.Z`) instead of a floating branch ref. + # POST-MERGE TODO (decided, not optional — do this immediately after this + # PR merges, do not wait for a tagged release): pin this default to the + # merge commit's own SHA, not a future tag. Run: + # git log -1 --format=%H origin/main # (or the actual merge commit) + # and update the line below to: + # _DEFAULT_MAC2NIX_SOURCE = "github:gordon-code/mac2nix?rev=" + # This eliminates the floating-`main`-HEAD trust window addressed by the + # SECURITY comment above without waiting for a formal release tag. The SHA + # cannot be known while this PR is still open (it is only assigned once + # the merge commit itself is created), which is why it is not set here. # Meanwhile, the only real mitigation is `--mac2nix-source` (a local # checkout, or any other flake ref/rev/tag) to opt out of the default. _DEFAULT_MAC2NIX_SOURCE = "github:gordon-code/mac2nix" diff --git a/tests/vm/test_validator.py b/tests/vm/test_validator.py index d3c7f26..4b94fe9 100644 --- a/tests/vm/test_validator.py +++ b/tests/vm/test_validator.py @@ -4,6 +4,7 @@ import asyncio import json +import logging from pathlib import Path from unittest.mock import AsyncMock, MagicMock, patch @@ -499,7 +500,7 @@ async def _run() -> None: class TestScanVMSourceSelection: - def test_default_source_runs_published_flake_without_extra_copy(self) -> None: + def test_default_source_runs_published_flake_without_extra_copy(self, caplog: pytest.LogCaptureFixture) -> None: """Default mac2nix_source runs the GitHub flake directly — regression check that today's `mac2nix validate` behavior (no local-source SCP) is unchanged.""" vm = _make_vm(exec_result=(True, "", "")) @@ -520,13 +521,19 @@ async def _run() -> SystemState: ): return await v._scan_vm() - result = asyncio.run(_run()) + with caplog.at_level(logging.WARNING, logger="mac2nix.vm.validator"): + result = asyncio.run(_run()) assert result is vm_state # Only the `nix run` scan invocation — no mkdir/scp for a source copy. assert len(exec_calls) == 1 nix_run_cmd = exec_calls[0] joined = nix_run_cmd[2] assert f"nix run {Validator._DEFAULT_MAC2NIX_SOURCE} --" in joined + # The unpinned-default supply-chain disclosure must surface as a + # visible warning, not only under verbose/debug logging. + assert any( + r.levelno == logging.WARNING and "unpinned default mac2nix source" in r.message for r in caplog.records + ) assert Validator._REMOTE_SOURCE_DIR not in joined def test_local_source_triggers_scp_and_runs_from_remote_source_dir(self, tmp_path: Path) -> None: From 701bc93cefd7e122ba714bac409de0b7eba669f2 Mon Sep 17 00:00:00 2001 From: Will Gordon Date: Wed, 19 Aug 2026 12:33:04 -0400 Subject: [PATCH 27/35] test(cli): covers generate's skipped and unrecognized domain output --- tests/cli/test_generate.py | 63 ++++++++++++++++++++++++++++++++++++++ 1 file changed, 63 insertions(+) diff --git a/tests/cli/test_generate.py b/tests/cli/test_generate.py index 5e30af7..04c2f14 100644 --- a/tests/cli/test_generate.py +++ b/tests/cli/test_generate.py @@ -8,6 +8,7 @@ from click.testing import CliRunner from mac2nix.cli import main +from mac2nix.generators import GenerateResult from mac2nix.generators.scaffold import init_framework from mac2nix.models.preferences import PreferencesDomain, PreferencesResult from mac2nix.models.system import SystemConfig @@ -235,3 +236,65 @@ def test_generate_all_failure_wraps_as_click_exception(self, tmp_path: Path) -> assert result.exc_info is not None assert result.exc_info[0] is SystemExit assert "sentinel" in result.output + + def test_skipped_domain_prints_reason(self, tmp_path: Path) -> None: + """Mirrors test_generate_all.py::test_missing_system_domain_skips_preferences, + but asserts on the CLI's own echo formatting for result.skipped (cli.py:477-478), + which had no coverage at the CLI layer. + """ + output_dir = tmp_path / "repo" + init_framework(output_dir) + _register_fake_host(output_dir, "myhost") + scan_file = tmp_path / "scan.json" + domains = [PreferencesDomain(domain_name="com.apple.dock", keys={"tilesize": 48})] + state = SystemState( + hostname="h", + macos_version="26.0", + architecture="arm64", + preferences=PreferencesResult(domains=domains), + system=None, + ) + state.to_json(scan_file) + + runner = CliRunner() + result = runner.invoke( + main, + ["generate", str(output_dir), "--hostname", "myhost", "--scan-file", str(scan_file)], + ) + + assert result.exit_code == 0, result.output + assert "Skipped preferences: not scanned" in result.output + assert "Generated:" not in result.output + assert not (output_dir / "hosts" / "darwin" / "myhost" / "preferences.nix").exists() + + def test_unrecognized_domain_prints_when_reported_by_generate_all(self, tmp_path: Path) -> None: + """result.unrecognized is always empty for a real CLI call today -- the CLI's own + _ALLOWED_DOMAINS membership check (cli.py:450-453) rejects any non-'preferences' + domain before generate_all() ever runs, per GenerateResult's own docstring. The + echo branch at cli.py:479-480 is otherwise unreachable from a real invocation, so + this patches generate_all() directly to simulate a future domain that's allowed by + the CLI but not yet handled by generate_all()'s own if-block (mirrors + test_generate_all.py::test_unrecognized_domain_returns_without_raising at the + generate_all() unit level). + """ + output_dir = tmp_path / "repo" + init_framework(output_dir) + _register_fake_host(output_dir, "myhost") + scan_file = tmp_path / "scan.json" + _write_scan_file(scan_file) + + fake_result = GenerateResult( + ran=set(), + skipped={}, + unrecognized=frozenset({"future_domain"}), + homebrew_audit_manifest=None, + ) + with patch("mac2nix.cli.generate_all", return_value=fake_result): + runner = CliRunner() + result = runner.invoke( + main, + ["generate", str(output_dir), "--hostname", "myhost", "--scan-file", str(scan_file)], + ) + + assert result.exit_code == 0, result.output + assert "Unrecognized (not generated): future_domain" in result.output From a1e0f3a472f4793120f2bbf4425c7ea6180011a3 Mon Sep 17 00:00:00 2001 From: Will Gordon Date: Wed, 19 Aug 2026 12:33:16 -0400 Subject: [PATCH 28/35] docs(contributing): adds contributing as a valid commit scope --- CONTRIBUTING.md | 29 +++++++++++++++-------------- 1 file changed, 15 insertions(+), 14 deletions(-) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 22c49f4..965f7d4 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -26,20 +26,21 @@ This project follows [Conventional Commits](https://www.conventionalcommits.org/ ### Scopes -| Scope | Description | -|--------------|------------------------------------| -| `cli` | CLI commands and options | -| `scanners` | macOS system scanners | -| `generators` | Nix configuration generators | -| `mappings` | macOS-to-nix mapping tables | -| `models` | Data models | -| `reports` | Scan reports and diffs | -| `templates` | Jinja2 nix templates | -| `vm` | Tart VM integration | -| `drift` | Configuration drift detection | -| `deps` | Dependency updates | -| `ci` | CI/CD pipeline | -| `tests` | Test-only infrastructure/helpers | +| Scope | Description | +|----------------|--------------------------------------| +| `cli` | CLI commands and options | +| `scanners` | macOS system scanners | +| `generators` | Nix configuration generators | +| `mappings` | macOS-to-nix mapping tables | +| `models` | Data models | +| `reports` | Scan reports and diffs | +| `templates` | Jinja2 nix templates | +| `vm` | Tart VM integration | +| `drift` | Configuration drift detection | +| `deps` | Dependency updates | +| `ci` | CI/CD pipeline | +| `tests` | Test-only infrastructure/helpers | +| `contributing` | Contributing guide and conventions | ### Examples From 4ba1ed9e73f4819cd903266b9d6819df96de2680 Mon Sep 17 00:00:00 2001 From: Will Gordon Date: Wed, 19 Aug 2026 18:00:23 -0400 Subject: [PATCH 29/35] feat(generators): combines scripts into one postActivation hook --- src/mac2nix/generators/_nix_render.py | 46 ++++++++++++++ tests/_generate_helpers.py | 87 +++++++++++++++++++++++++++ tests/generators/test_nix_render.py | 35 +++++++++++ 3 files changed, 168 insertions(+) diff --git a/src/mac2nix/generators/_nix_render.py b/src/mac2nix/generators/_nix_render.py index 99f5d72..b097b00 100644 --- a/src/mac2nix/generators/_nix_render.py +++ b/src/mac2nix/generators/_nix_render.py @@ -50,6 +50,52 @@ def nix_mkdefault(nix_expr: str) -> str: return f"lib.mkDefault {nix_expr}" +def nix_post_activation_script(bodies: list[str]) -> str: + """Wrap one or more caller-built shell-body expressions into a single + `system.activationScripts.postActivation.text` assignment. + + nix-darwin's own activation-scripts.nix module concatenates only a + fixed, hardcoded sequence of named entries (preActivation, etcChecks, + ..., defaults, launchd, ..., postActivation) into the real script that + `darwin-rebuild switch` runs -- confirmed against nix-darwin's real + source and a real Tart-VM switch where an arbitrary custom key (e.g. + `system.activationScripts.mac2nixWallpaper.text`) evaluated to valid + Nix and built successfully but was silently never executed, since + nix-darwin's own script-assembly code never references it. + `postActivation` is one of the few real, always-concatenated hook + points. Every caller-built body (already a complete Nix expression + evaluating to a string, typically `let ... in ''...''`) must route + through this single function -- there is no other valid destination + for a custom activation script. + + `bodies` is rendered as a Nix list joined with `lib.concatStringsSep` + rather than string-concatenated in Python, so each body stays visually + separate in the generated source for readability/debugging. Each body + is wrapped in parens: a bare `let ... in ''...''` is not itself a valid + list-literal element in Nix's grammar (list elements must be + application-level terms), so the parens are required, not cosmetic. + + Deliberately NOT wrapped in `lib.mkDefault`. `postActivation.text` is a + `types.lines` option: multiple definitions at the SAME priority merge + (concatenate) via that type's own merge function, but the NixOS module + system first discards every definition that isn't at the single lowest + priority number present across ALL modules -- it does not merge across + priority tiers. `lib.mkDefault` sets priority 1000 (lower precedence); + home-manager's own nix-darwin integration + (`home-manager/nix-darwin/default.nix`) sets this exact same option + with a plain, unwrapped assignment (priority 100, higher precedence). + Confirmed for real: with `mkDefault`, home-manager's definition won + outright and this module's entire activation-script content was + silently discarded -- not merged, not overridden-with-a-warning, just + absent -- verified by building the real system derivation and grepping + its `activate` script, which contained zero trace of any + mac2nix-authored command. A plain assignment here merges at the same + priority as home-manager's own fragment instead of losing to it. + """ + items = "\n ".join(f"({body})" for body in bodies) + return f'system.activationScripts.postActivation.text = (\n lib.concatStringsSep "\\n" [\n {items}\n ]\n);' + + def nix_comment(text: str) -> str: """Render *text* as safe content for a single-line Nix `#` comment. diff --git a/tests/_generate_helpers.py b/tests/_generate_helpers.py index 4311224..a55bb67 100644 --- a/tests/_generate_helpers.py +++ b/tests/_generate_helpers.py @@ -7,8 +7,13 @@ from __future__ import annotations import json +import shutil +import subprocess from pathlib import Path +import pytest + +from mac2nix.generators._nix_render import nix_string from mac2nix.generators.scaffold import _read_template, _render_placeholders @@ -26,3 +31,85 @@ def _register_fake_host(output_dir: Path, hostname: str, username: str = "testus meta = {"hostname": hostname, "username": username, "system": "aarch64-darwin", "age_public_key": "age1fake"} (host_dir / ".mac2nix-meta.json").write_text(json.dumps(meta)) return host_dir + + +def _quoted_spans(text: str) -> list[tuple[int, int]]: + """Return (start, end) index spans of Nix double-quoted string literals in `text`. + + Approximate scanner (treats `\\"` as a non-terminating escaped quote, + everything else literally) -- good enough for the structural injection + check below, not a full Nix parser. + """ + spans: list[tuple[int, int]] = [] + in_string = False + start = 0 + i = 0 + n = len(text) + while i < n: + ch = text[i] + if not in_string: + if ch == '"': + in_string = True + start = i + i += 1 + elif ch == "\\": + i += 2 + elif ch == '"': + spans.append((start, i + 1)) + in_string = False + i += 1 + else: + i += 1 + return spans + + +def assert_activation_script_neutralizes_shell_metacharacters( + rendered_body: str, injected_marker: str, tmp_path: Path +) -> None: + """Assert an activation-script body safely neutralizes an adversarial dynamic value. + + Every activation-script code path built from scanned (untrusted) data -- + the wallpaper path (Step 9/11) and any case promoted by Steps 13/14 -- + must pass an adversarial value containing a backtick, `$()`, `;`, a + literal `"`, and a newline through this helper before shipping. + + Two checks: + (a) the body still parses as valid Nix once wrapped in a minimal module + -- no Nix-syntax breakout via the injected marker. + (b) the marker's escaped form (as produced by this codebase's own + `nix_string()`) appears only inside a Nix double-quoted string + literal in the rendered text -- a marker sitting outside any quoted + span would mean it reached shell/Nix source unescaped. + + `[ASSUMPTION: detail]` This is a structural approximation, not a full + round-trip proof (it doesn't execute the shell body). Acceptable for + this generator's current cases; revisit if a future case's shell logic + gets meaningfully more complex. Callers should mark their test + `@pytest.mark.nix`. + """ + if shutil.which("nix-instantiate") is None: + pytest.skip("nix-instantiate not on PATH") + + escaped_inner = nix_string(injected_marker)[1:-1] + assert escaped_inner in rendered_body, "fixture bug: escaped marker not present in rendered body" + + module_source = f"{{ config, lib, pkgs, ... }}:\n{{\n {rendered_body}\n}}\n" + module_path = tmp_path / "activation_fixture.nix" + module_path.write_text(module_source) + + result = subprocess.run( # noqa: S603 + ["nix-instantiate", "--parse", str(module_path)], # noqa: S607 + capture_output=True, + text=True, + check=False, + ) + assert result.returncode == 0, result.stderr + + spans = _quoted_spans(rendered_body) + pos = rendered_body.find(escaped_inner) + while pos != -1: + assert any(start <= pos < end for start, end in spans), ( + f"marker at position {pos} is not inside a quoted Nix string literal -- " + "possible injection: value may reach shell/Nix source unescaped" + ) + pos = rendered_body.find(escaped_inner, pos + 1) diff --git a/tests/generators/test_nix_render.py b/tests/generators/test_nix_render.py index ef6be67..93c0557 100644 --- a/tests/generators/test_nix_render.py +++ b/tests/generators/test_nix_render.py @@ -13,11 +13,13 @@ from mac2nix.generators._nix_render import ( nix_comment, nix_mkdefault, + nix_post_activation_script, nix_string, python_to_nix, render_template, setup_jinja_env, ) +from tests._generate_helpers import assert_activation_script_neutralizes_shell_metacharacters _BACKSLASH = chr(92) _DQUOTE = chr(34) @@ -89,6 +91,30 @@ def test_nix_mkdefault_wraps_expression() -> None: assert nix_mkdefault("true") == "lib.mkDefault true" +def test_nix_post_activation_script_wraps_bodies_in_concat_list() -> None: + """Deliberately NOT `lib.mkDefault`-wrapped -- confirmed via a real built + system derivation that home-manager's own nix-darwin integration sets + this exact option with a plain (higher-precedence) assignment, which + would silently discard an `mkDefault`-priority definition entirely + rather than merge with it. + """ + rendered = nix_post_activation_script(["''\n echo hi\n''"]) + assert rendered == ( + "system.activationScripts.postActivation.text = (\n" + ' lib.concatStringsSep "\\n" [\n' + " (''\n echo hi\n'')\n" + " ]\n" + ");" + ) + assert "mkDefault" not in rendered + + +def test_nix_post_activation_script_combines_multiple_bodies() -> None: + rendered = nix_post_activation_script(["''one''", "''two''"]) + assert "(''one'')" in rendered + assert "(''two'')" in rendered + + def test_nix_comment_replaces_newline_variants_with_space() -> None: assert nix_comment("a\nb") == "a b" assert nix_comment("a\r\nb") == "a b" @@ -117,6 +143,15 @@ def test_render_template_delegates_to_environment() -> None: assert result == '"hi"' +@pytest.mark.nix +def test_nix_post_activation_script_adversarial_value_stays_quoted(tmp_path: Path) -> None: + marker = 'inject`ed $(rm -rf /) ; "quoted"\nnewline' + body = f"let\n wallpaperPath = {nix_string(marker)};\nin\n''\n ${{lib.escapeShellArg wallpaperPath}}\n''" + rendered = nix_post_activation_script([body]) + + assert_activation_script_neutralizes_shell_metacharacters(rendered, marker, tmp_path) + + @pytest.fixture def require_nix_instantiate() -> None: if shutil.which("nix-instantiate") is None: From 94a07d8bc471df3c17498ea9ced2b33385b8fa0b Mon Sep 17 00:00:00 2001 From: Will Gordon Date: Wed, 19 Aug 2026 18:01:01 -0400 Subject: [PATCH 30/35] feat(scanners): distinguishes wallpaper scan failure from no-wallpaper --- src/mac2nix/models/system.py | 1 + src/mac2nix/scanners/system_scanner.py | 18 ++++--- tests/scanners/test_system_scanner.py | 72 +++++++++++++++++++------- 3 files changed, 64 insertions(+), 27 deletions(-) diff --git a/src/mac2nix/models/system.py b/src/mac2nix/models/system.py index 46f89fb..3f82e99 100644 --- a/src/mac2nix/models/system.py +++ b/src/mac2nix/models/system.py @@ -117,3 +117,4 @@ class SystemConfig(BaseModel): icloud: ICloudState = Field(default_factory=ICloudState) mdm_enrolled: bool | None = None wallpaper_path: Path | None = None + wallpaper_scan_error: str | None = None diff --git a/src/mac2nix/scanners/system_scanner.py b/src/mac2nix/scanners/system_scanner.py index 8392ac8..c4f7ac3 100644 --- a/src/mac2nix/scanners/system_scanner.py +++ b/src/mac2nix/scanners/system_scanner.py @@ -79,7 +79,7 @@ def scan(self) -> SystemConfig: system_extensions = self._detect_system_extensions() icloud = self._detect_icloud() mdm_enrolled = self._detect_mdm() - wallpaper_path = self._get_wallpaper_path() + wallpaper_path, wallpaper_scan_error = self._get_wallpaper_path() return SystemConfig( hostname=hostname, @@ -113,6 +113,7 @@ def scan(self) -> SystemConfig: icloud=icloud, mdm_enrolled=mdm_enrolled, wallpaper_path=wallpaper_path, + wallpaper_scan_error=wallpaper_scan_error, ) def _get_computer_name(self) -> str | None: @@ -572,14 +573,17 @@ def _detect_icloud(self) -> ICloudState: documents_sync=documents_sync, ) - def _get_wallpaper_path(self) -> Path | None: + def _get_wallpaper_path(self) -> tuple[Path | None, str | None]: """Read the current desktop wallpaper path from desktoppicture.db. macOS stores the desktop picture in a SQLite database, not a plist -- the general preferences scanner structurally cannot see it. Never raises: a missing file, corrupt database, or schema mismatch all - resolve to `None`, consistent with every other best-effort scanner - capability in this file. + resolve to a `None` path. Returns `(path, scan_error)` -- `scan_error` + is populated only for a genuine read/schema failure (something is + actually wrong); a legitimate "no wallpaper row found" result (the + query ran fine, there's just no picture set) leaves it `None`. These + are NOT the same case and must stay distinguishable downstream. """ db_path = Path.home() / "Library" / "Application Support" / "Dock" / "desktoppicture.db" try: @@ -594,7 +598,7 @@ def _get_wallpaper_path(self) -> Path | None: row = conn.execute(_WALLPAPER_QUERY).fetchone() except (sqlite3.Error, OSError) as exc: logger.debug("Could not read desktop wallpaper from %s: %s", db_path, exc) - return None + return None, f"could not read {db_path.name} ({exc})" if not row or not row[0]: logger.debug( @@ -602,9 +606,9 @@ def _get_wallpaper_path(self) -> Path | None: "'preferences' row with key=1 pointing to an absolute-path 'data' " "value) -- wallpaper_path will be unset" ) - return None + return None, None - return Path(row[0]) + return Path(row[0]), None def _detect_mdm(self) -> bool | None: """Check if device is MDM enrolled.""" diff --git a/tests/scanners/test_system_scanner.py b/tests/scanners/test_system_scanner.py index fb46457..b117ee5 100644 --- a/tests/scanners/test_system_scanner.py +++ b/tests/scanners/test_system_scanner.py @@ -932,9 +932,10 @@ def test_wallpaper_path_from_real_schema(self, tmp_path: Path) -> None: _write_wallpaper_db(db_path, [(1, "/System/Library/Desktop Pictures/The Cliffs.heic")]) with patch("mac2nix.scanners.system_scanner.Path.home", return_value=tmp_path): - result = SystemScanner()._get_wallpaper_path() + path, error = SystemScanner()._get_wallpaper_path() - assert result == Path("/System/Library/Desktop Pictures/The Cliffs.heic") + assert path == Path("/System/Library/Desktop Pictures/The Cliffs.heic") + assert error is None def test_home_path_with_uri_reserved_characters_still_resolves(self, tmp_path: Path) -> None: """A literal '?' or '#' in the path (e.g. an unusual macOS username) must not be @@ -948,9 +949,10 @@ def test_home_path_with_uri_reserved_characters_still_resolves(self, tmp_path: P _write_wallpaper_db(db_path, [(1, "/System/Library/Desktop Pictures/The Cliffs.heic")]) with patch("mac2nix.scanners.system_scanner.Path.home", return_value=home): - result = SystemScanner()._get_wallpaper_path() + path, error = SystemScanner()._get_wallpaper_path() - assert result == Path("/System/Library/Desktop Pictures/The Cliffs.heic") + assert path == Path("/System/Library/Desktop Pictures/The Cliffs.heic") + assert error is None def test_most_recently_written_path_wins(self, tmp_path: Path) -> None: db_path = self._db_path(tmp_path) @@ -963,9 +965,10 @@ def test_most_recently_written_path_wins(self, tmp_path: Path) -> None: ) with patch("mac2nix.scanners.system_scanner.Path.home", return_value=tmp_path): - result = SystemScanner()._get_wallpaper_path() + path, error = SystemScanner()._get_wallpaper_path() - assert result == Path("/System/Library/Desktop Pictures/The Cliffs.heic") + assert path == Path("/System/Library/Desktop Pictures/The Cliffs.heic") + assert error is None def test_non_path_key_is_ignored(self, tmp_path: Path) -> None: """key != 1 rows are non-path bookkeeping on real machines -- must never be selected.""" @@ -979,27 +982,34 @@ def test_non_path_key_is_ignored(self, tmp_path: Path) -> None: ) with patch("mac2nix.scanners.system_scanner.Path.home", return_value=tmp_path): - result = SystemScanner()._get_wallpaper_path() + path, error = SystemScanner()._get_wallpaper_path() - assert result == Path("/System/Library/Desktop Pictures/The Cliffs.heic") + assert path == Path("/System/Library/Desktop Pictures/The Cliffs.heic") + assert error is None - def test_missing_db_returns_none(self, tmp_path: Path) -> None: + def test_missing_db_populates_scan_error(self, tmp_path: Path) -> None: with patch("mac2nix.scanners.system_scanner.Path.home", return_value=tmp_path): - result = SystemScanner()._get_wallpaper_path() + path, error = SystemScanner()._get_wallpaper_path() - assert result is None + assert path is None + assert error is not None - def test_corrupt_db_returns_none(self, tmp_path: Path) -> None: + def test_corrupt_db_populates_scan_error(self, tmp_path: Path) -> None: db_path = self._db_path(tmp_path) db_path.parent.mkdir(parents=True) db_path.write_bytes(b"not a sqlite database") with patch("mac2nix.scanners.system_scanner.Path.home", return_value=tmp_path): - result = SystemScanner()._get_wallpaper_path() + path, error = SystemScanner()._get_wallpaper_path() - assert result is None + assert path is None + assert error is not None - def test_wrong_schema_returns_none(self, tmp_path: Path) -> None: + def test_wrong_schema_populates_scan_error(self, tmp_path: Path) -> None: + """A genuine schema-mismatch read failure -- distinct from a legitimate + zero-matching-rows result (test_no_matching_row_leaves_scan_error_none) -- + must surface a non-None reason so a real problem isn't silently swallowed. + """ db_path = self._db_path(tmp_path) db_path.parent.mkdir(parents=True) conn = sqlite3.connect(db_path) @@ -1008,18 +1018,24 @@ def test_wrong_schema_returns_none(self, tmp_path: Path) -> None: conn.close() with patch("mac2nix.scanners.system_scanner.Path.home", return_value=tmp_path): - result = SystemScanner()._get_wallpaper_path() + path, error = SystemScanner()._get_wallpaper_path() - assert result is None + assert path is None + assert error is not None - def test_no_matching_row_returns_none(self, tmp_path: Path) -> None: + def test_no_matching_row_leaves_scan_error_none(self, tmp_path: Path) -> None: + """Zero matching rows is a legitimate "no wallpaper set" outcome, not a + failure -- nothing is wrong here, so scan_error must stay None (distinct + from test_wrong_schema_populates_scan_error's genuine failure case). + """ db_path = self._db_path(tmp_path) _write_wallpaper_db(db_path, [(16, "F8DD5F35")]) with patch("mac2nix.scanners.system_scanner.Path.home", return_value=tmp_path): - result = SystemScanner()._get_wallpaper_path() + path, error = SystemScanner()._get_wallpaper_path() - assert result is None + assert path is None + assert error is None def test_wallpaper_wired_into_scan(self, tmp_path: Path) -> None: db_path = self._db_path(tmp_path) @@ -1033,3 +1049,19 @@ def test_wallpaper_wired_into_scan(self, tmp_path: Path) -> None: assert isinstance(result, SystemConfig) assert result.wallpaper_path == Path("/System/Library/Desktop Pictures/The Cliffs.heic") + assert result.wallpaper_scan_error is None + + def test_wallpaper_scan_error_wired_into_scan(self, tmp_path: Path) -> None: + db_path = self._db_path(tmp_path) + db_path.parent.mkdir(parents=True) + db_path.write_bytes(b"not a sqlite database") + + with ( + patch("mac2nix.scanners.system_scanner.run_command", return_value=None), + patch("mac2nix.scanners.system_scanner.Path.home", return_value=tmp_path), + ): + result = SystemScanner().scan() + + assert isinstance(result, SystemConfig) + assert result.wallpaper_path is None + assert result.wallpaper_scan_error is not None From 9647dac9dd139599909dd80f003d0efa8990ca26 Mon Sep 17 00:00:00 2001 From: Will Gordon Date: Wed, 19 Aug 2026 18:01:18 -0400 Subject: [PATCH 31/35] feat(mappings): tags manual-report reasons by category --- src/mac2nix/mappings/classifier.py | 13 ++++++++++--- tests/mappings/test_classifier.py | 20 ++++++++++++++++++++ 2 files changed, 30 insertions(+), 3 deletions(-) diff --git a/src/mac2nix/mappings/classifier.py b/src/mac2nix/mappings/classifier.py index e5b8f79..bb90413 100644 --- a/src/mac2nix/mappings/classifier.py +++ b/src/mac2nix/mappings/classifier.py @@ -201,7 +201,7 @@ def _classify_preference_precheck( # what generators surface into real, on-disk output. return ClassificationResult( tier=ClassificationTier.MANUAL_REPORT, - destination=f"manual report: key '***REDACTED***' in domain '{domain.domain_name}' " + destination=f"[sensitive] manual report: key '***REDACTED***' in domain '{domain.domain_name}' " "matches a sensitive pattern", metadata={ "potentially_sensitive": True, @@ -499,7 +499,7 @@ def classify_system_setting(field_name: str, value: Any) -> ClassificationResult if nix_path is None: return ClassificationResult( tier=ClassificationTier.MANUAL_REPORT, - destination=f"manual report: no nix-darwin option for system setting '{field_name}'", + destination=f"[coverage gap] manual report: no nix-darwin option for system setting '{field_name}'", metadata={"field_name": field_name, "value": value}, ) return ClassificationResult( @@ -537,7 +537,14 @@ def classify_wallpaper(path: Path) -> ClassificationResult: removed `{pre,post}UserActivation` -- all activation now runs as root, so a generator targeting this destination must wrap any user-context command (e.g. `osascript` talking to the logged-in user's WindowServer - session) in `sudo -u ${config.system.primaryUser}` itself. + session) in `sudo -u ${config.system.primaryUser}` itself. It's also not + a custom `mac2nixWallpaper`-style key: nix-darwin's own + activation-scripts.nix module only ever concatenates a fixed, hardcoded + set of named entries into the script `darwin-rebuild switch` actually + runs -- an arbitrary custom key evaluates and builds fine but is + silently never executed (confirmed via a real Tart-VM switch and + nix-darwin's own GitHub issue #663). `postActivation` is one of the few + real hook points. """ return ClassificationResult( tier=ClassificationTier.ACTIVATION_SCRIPT, diff --git a/tests/mappings/test_classifier.py b/tests/mappings/test_classifier.py index 37d5610..72dc5c1 100644 --- a/tests/mappings/test_classifier.py +++ b/tests/mappings/test_classifier.py @@ -497,6 +497,26 @@ def test_timezone_routes_to_native_time_timezone(self) -> None: assert result.nix_path == "time.timeZone" +class TestManualReportCategoryTags: + """Each MANUAL_REPORT destination is prefixed with a short category tag at + the point it's constructed, reflecting *why* it's manual: `[sensitive]` + (deliberately never captured), `[coverage gap]` (no nix-darwin mapping + exists yet). Constructed here, not via post-hoc string matching in a + generator, which would be fragile. + """ + + def test_manual_report_category_sensitive_key_is_tagged(self) -> None: + domain = _domain("com.example.someapp", {"API_TOKEN": "sk-live-abc123"}) + result = classify_preference(domain, "API_TOKEN", "sk-live-abc123") + assert result.tier == ClassificationTier.MANUAL_REPORT + assert result.destination.startswith("[sensitive] ") + + def test_manual_report_category_unmapped_system_setting_is_tagged(self) -> None: + result = classify_system_setting("hibernatemode", "3") + assert result.tier == ClassificationTier.MANUAL_REPORT + assert result.destination.startswith("[coverage gap] ") + + class TestClassifySecuritySetting: def test_known_security_field_routes_to_native(self) -> None: result = classify_security_setting("firewall_enabled", True) From 9e6341c32c547cc476ba8013825a530edffa3d71 Mon Sep 17 00:00:00 2001 From: Will Gordon Date: Wed, 19 Aug 2026 18:02:19 -0400 Subject: [PATCH 32/35] chore(tests): exempts ARG002 for tests, matching ARG001 --- pyproject.toml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pyproject.toml b/pyproject.toml index 2934887..7f39eb4 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -63,7 +63,7 @@ ignore = [ ] [tool.ruff.lint.per-file-ignores] -"**/tests/**/*.py" = ["S101", "S105", "S106", "S107", "S108", "S314", "SLF001", "ARG001"] # assert + test data + credentials + XML parsing + private access + unused mock args OK in tests +"**/tests/**/*.py" = ["S101", "S105", "S106", "S107", "S108", "S314", "SLF001", "ARG001", "ARG002"] # assert + test data + credentials + XML parsing + private access + unused mock/fixture args (functions and methods) OK in tests [tool.ruff.lint.isort] known-first-party = ["mac2nix"] From b75531ab472464a0a065eaaad2da413399a97ff6 Mon Sep 17 00:00:00 2001 From: Will Gordon Date: Wed, 19 Aug 2026 18:02:30 -0400 Subject: [PATCH 33/35] feat(generators): bundles wallpaper images with self-guarding scripts --- src/mac2nix/generators/__init__.py | 47 +- src/mac2nix/generators/preferences.py | 521 ++++++++++++++++-- .../templates/modules/preferences.nix.j2 | 28 +- tests/generators/test_generate_all.py | 87 +++ tests/generators/test_generate_integration.py | 35 +- tests/generators/test_preferences.py | 465 ++++++++++++++-- 6 files changed, 1057 insertions(+), 126 deletions(-) diff --git a/src/mac2nix/generators/__init__.py b/src/mac2nix/generators/__init__.py index 3827ea8..8178b2d 100644 --- a/src/mac2nix/generators/__init__.py +++ b/src/mac2nix/generators/__init__.py @@ -17,7 +17,7 @@ from pathlib import Path from typing import Any -from mac2nix.generators.preferences import generate_preferences +from mac2nix.generators.preferences import WallpaperAsset, generate_preferences from mac2nix.models.system_state import SystemState logger = logging.getLogger(__name__) @@ -95,6 +95,45 @@ def _store_host_imports_hash(host_dir: Path, inner: str) -> None: (host_dir / _META_FILENAME).write_text(json.dumps(meta, indent=2)) +def _write_wallpaper_asset(host_dir: Path, asset: WallpaperAsset) -> None: + """Write a bundled wallpaper asset under `host_dir/assets/`. + + Mirrors `_warn_if_host_imports_hand_edited`/`_store_host_imports_hash`'s + hash-tracking pattern: warns if the on-disk file was hand-edited since + mac2nix last wrote it, and skips a gratuitous rewrite when the content + hasn't actually changed. Orphaned assets left behind by a wallpaper + change/removal are an accepted non-goal, consistent with this module's + existing never-deletes-stale-files behavior for `.nix` outputs. + """ + dest = host_dir / "assets" / asset.filename + + try: + meta: dict[str, Any] | None = _read_host_meta(host_dir) + except (OSError, json.JSONDecodeError): + meta = None + + if dest.is_file(): + on_disk = dest.read_bytes() + if meta is not None and meta.get("wallpaper_asset_filename") == asset.filename: + stored_hash = meta.get("wallpaper_asset_hash") + if stored_hash is not None and hashlib.sha256(on_disk).hexdigest() != stored_hash: + logger.warning( + "%s doesn't match what generate last wrote there (likely a hand-edit) -- " + "this regeneration will overwrite it.", + dest, + ) + if on_disk == asset.data: + return + + dest.parent.mkdir(parents=True, exist_ok=True) + dest.write_bytes(asset.data) + + if meta is not None: + meta["wallpaper_asset_filename"] = asset.filename + meta["wallpaper_asset_hash"] = hashlib.sha256(asset.data).hexdigest() + (host_dir / _META_FILENAME).write_text(json.dumps(meta, indent=2)) + + def _regenerate_host_imports(output_dir: Path, hostname: str) -> None: """Fully regenerate configuration.nix's sentinel-bounded imports line from actual on-disk file existence for this host -- not which domains ran in @@ -171,8 +210,10 @@ def generate_all(system_state: SystemState, output_dir: Path, hostname: str, dom if "preferences" in domains: if system_state.preferences is not None and system_state.system is not None: - rendered = generate_preferences(system_state) - (host_dir / "preferences.nix").write_text(rendered) + generated = generate_preferences(system_state) + (host_dir / "preferences.nix").write_text(generated.rendered) + if generated.asset is not None: + _write_wallpaper_asset(host_dir, generated.asset) ran.add("preferences") else: skipped["preferences"] = "not scanned" diff --git a/src/mac2nix/generators/preferences.py b/src/mac2nix/generators/preferences.py index 4e8e317..bb9675f 100644 --- a/src/mac2nix/generators/preferences.py +++ b/src/mac2nix/generators/preferences.py @@ -10,9 +10,10 @@ from __future__ import annotations from dataclasses import dataclass +from pathlib import Path from typing import Any -from mac2nix.generators._nix_render import render_template +from mac2nix.generators._nix_render import nix_post_activation_script, nix_string, render_template from mac2nix.mappings.classifier import ( ClassificationResult, ClassificationTier, @@ -173,11 +174,344 @@ def _collect_power_items(power_settings: dict[str, str]) -> list[_CuratedItem]: # that report Wake-on-LAN as unsupported (nix-darwin's own option # docstring: "Battery powered devices may require being connected to # power."). Never render either option natively -- this generator has no -# mechanism to detect target-hardware capability at generate or apply -# time, so both are downgraded to a manual-report comment instead. +# way to make nix-darwin's own option skip its unconditional check. Step 14 +# instead renders each as its own self-guarding activation script that +# probes target support directly (mirroring nix-darwin's own +# modules/system/checks.nix grep-for-"Not supported" pattern) and only +# applies via `systemsetup` if supported, bypassing the native option +# (and its unconditional check) entirely. _POWER_HARDWARE_DEPENDENT_NIX_PATHS = frozenset({"power.restartAfterPowerFailure", "networking.wakeOnLan.enable"}) +def _coerce_hardware_dependent_setting(value: Any) -> str | None: + """Coerce a scanned pmset boolean-like value (autorestart/womp) to exactly "on" or "off". + + Per security consultation: coerce to a closed set rather than escaping + an open one -- anything that isn't exactly the scanned pmset literal + "1" or "0" fails closed to the manual-report path, never passed + through as a raw scanned string. + """ + if value == "1": + return "on" + if value == "0": + return "off" + return None + + +def _build_hardware_dependent_activation_script( + probe_flag: str, apply_flag: str, value: str, setting_label: str +) -> str: + """Build a self-guarding activation-script body for a hardware-dependent power/networking setting. + + Mirrors nix-darwin's own modules/system/checks.nix grep-for-"Not + supported" probe pattern instead of relying on its unconditional + native-option check. Unlike wallpaper's AppleScript call, neither + setting needs `sudo -u ${config.system.primaryUser}` -- `systemsetup` + operates at the system level and nix-darwin activation already runs as + root. `value` is already one of exactly "on"/"off" (never a raw + scanned string) by the time it reaches here; still routed through + `lib.escapeShellArg` as defense-in-depth against a future coercion bug. + + Returns a bare body expression -- the caller collects these into a list + for `nix_post_activation_script()`, which is the only function that + wraps them into the one real `system.activationScripts.postActivation` + hook nix-darwin actually executes. + """ + return ( + "let\n" + f" settingValue = {nix_string(value)};\n" + " in\n" + " ''\n" + f' if systemsetup {probe_flag} | grep -q "Not supported"; then\n' + f' echo "mac2nix: {setting_label} not supported on this hardware, skipped" >&2\n' + " else\n" + f" systemsetup {apply_flag} ${{lib.escapeShellArg settingValue}}\n" + " fi\n" + " ''" + ) + + +def _build_restart_after_power_failure_activation_script(value: str) -> str: + return _build_hardware_dependent_activation_script( + "-getRestartPowerFailure", "-setRestartPowerFailure", value, "restart-after-power-failure" + ) + + +def _build_wake_on_lan_activation_script(value: str) -> str: + """`-getwakeonnetworkaccess`/`-setwakeonnetworkaccess` are the real `systemsetup` + flags for Wake-on-LAN -- confirmed against a real `systemsetup -help` on a Tart + VM. `-get/setRemoteWakeUp` (an earlier, incorrect guess) is not a valid + `systemsetup` command at all; the tool prints an "is not a valid command" + error but still exits 0, which is why a real VM switch never surfaced this as + a failure -- it silently no-op'd instead of erroring or applying anything. + """ + return _build_hardware_dependent_activation_script( + "-getwakeonnetworkaccess", "-setwakeonnetworkaccess", value, "wake-on-LAN" + ) + + +# Step 13's curated-domain audit: of every `[coverage gap]` manual-report +# case reachable from this generator (preference domain/key items always +# route to NATIVE or CUSTOM_PREFS when unmapped, never MANUAL_REPORT -- see +# classify_preference()'s fallthrough -- so `[coverage gap]` only ever +# comes from classify_system_setting()'s no-mapping fallback for pmset +# keys), these three are promotable: no native nix-darwin option exists, +# but unlike restartAfterPowerFailure/wakeOnLan (systemsetup, hard-fails on +# unsupported hardware), `pmset -a ` silently no-ops a setting +# that doesn't apply to the current hardware (e.g. `lidwake` on a Mac with +# no lid) rather than erroring -- safe to apply unconditionally, no probe +# needed. [ASSUMPTION: detail] this enumeration is the audit itself, not +# exhaustive -- a real scan may surface more `[coverage gap]` pmset keys; +# these three are the ones this audit could confirm safe without one. Well +# under the plan's own ~5-case scope circuit-breaker. +_PROMOTED_POWER_SETTING_KEYS = frozenset({"hibernatemode", "standby", "lidwake"}) + + +def _coerce_promoted_power_setting_value(value: Any) -> int | None: + """Coerce a scanned pmset value to a plain non-negative int, or None if it can't be. + + Fails closed like Step 14's power-boolean coercion: anything that + doesn't cleanly coerce is never passed through as a raw scanned string + -- the caller falls back to the manual-report path instead. + """ + try: + coerced = int(value) + except (TypeError, ValueError): + return None + return coerced if coerced >= 0 else None + + +def _build_promoted_power_setting_activation_script(pmset_key: str, value: int) -> str: + """Build a self-contained activation-script body for a promoted `[coverage gap]` pmset setting. + + `pmset_key` is always one of the fixed, module-controlled + `_PROMOTED_POWER_SETTING_KEYS` literals -- never attacker/scan-derived + data -- so it's safe to embed directly in the shell command. `value` is + already a coerced Python int by the time it reaches here (never a raw + scanned string); still routed through `lib.escapeShellArg` as + defense-in-depth against a future coercion bug, matching Step 14's + same discipline. + + Returns a bare body expression -- see + `_build_hardware_dependent_activation_script()`'s docstring for why. + """ + return ( + "let\n" + f" settingValue = toString {value};\n" + " in\n" + " ''\n" + f" pmset -a {pmset_key} ${{lib.escapeShellArg settingValue}}\n" + " ''" + ) + + +def _handle_manual_report_item( + item: _CuratedItem, activation_scripts: dict[str, str], manual_report_comments: list[str] +) -> None: + """Route a MANUAL_REPORT-tier result to a promoted activation script or a plain comment. + + A promoted case (its `field_name` in `_PROMOTED_POWER_SETTING_KEYS`, + with a cleanly-coercible value) never also appears as a comment -- + promotion here is the only path for that field_name, mirroring the + same never-appears-in-both-lists mechanism Step 14 uses for + hardware-dependent settings. + + `activation_scripts` is keyed by an internal dedup identifier only + (never a real Nix attribute name) -- see `_build_render_context()`'s + docstring for why there is no longer a per-case Nix key. + """ + result = item.result + metadata = result.metadata or {} + field_name = metadata.get("field_name") + + if field_name in _PROMOTED_POWER_SETTING_KEYS: + coerced = _coerce_promoted_power_setting_value(item.value) + if coerced is not None: + activation_scripts[f"pmset_{field_name}"] = _build_promoted_power_setting_activation_script( + field_name, coerced + ) + return + + if not metadata.get("skipped"): + manual_report_comments.append(result.destination) + + +# A path whose resolved parent falls here ships identically on every macOS +# install -- portable across machines as an absolute path reference, +# no bundling needed. Anything else is a personal file unique to the +# scanned machine (_prepare_wallpaper_asset() decides whether it's safe to +# bundle it into the generated output instead). +_WALLPAPER_ASSET_ALLOWLIST = frozenset( + { + Path("/System/Library/Desktop Pictures"), + Path("/Library/Desktop Pictures"), + } +) + + +def _is_portable_wallpaper_path(path: Path) -> bool: + """A path anywhere under an allowlisted OS-asset directory is portable. + + macOS ships stock wallpapers in nested subdirectories (e.g. + "Solid Colors/Black.png", ".wallpapers/Sonoma Horizon/Sonoma Horizon.heic") + -- an exact-parent-match check would misclassify these as personal files + and route a genuinely portable OS-shipped wallpaper through the bundling + path, where it fails every allowlist there too and silently drops the + wallpaper automation entirely (found by a fresh-context adversarial + review, confirmed live against real subdirectories on this machine). + Allowlist entries are resolved too, matching + `_is_bundleable_wallpaper_location`'s same rationale for a symlinked + ancestor. + """ + resolved = path.resolve() + return any(resolved.is_relative_to(d.resolve()) for d in _WALLPAPER_ASSET_ALLOWLIST) + + +# Independent from _WALLPAPER_ASSET_ALLOWLIST above: that allowlist decides +# "is this path portable as-is"; this one decides "is bundling THIS +# personal path actually safe" -- a personal file outside both allowlists +# (e.g. under Downloads or an external volume) falls back to manual-report +# instead of bundling. [ASSUMPTION: detail] a starting guess at realistic +# personal-wallpaper locations, not exhaustively researched -- widen if a +# real scan's UAT run shows the bundling path rarely triggers. +def _wallpaper_bundle_source_allowlist() -> tuple[Path, ...]: + home = Path.home() + return (home / "Pictures", home / "Library" / "Application Support" / "Dock") + + +_WALLPAPER_EXTENSION_ALLOWLIST = frozenset({".heic", ".jpg", ".jpeg", ".png", ".tiff"}) +_WALLPAPER_MAX_BYTES = 20 * 1024 * 1024 + +# ISO-BMFF (HEIC/HEIF) brand codes at bytes[8:12] of a "....ftyp" box. +_HEIC_FTYP_BRANDS = frozenset({b"heic", b"heix", b"hevc", b"hevx", b"mif1", b"msf1"}) + + +def _sniff_image_extension(data: bytes) -> str | None: + """Identify a real image format from magic bytes -- never trust a claimed extension. + + `imghdr` was removed in Python 3.13 (PEP 594); this covers exactly the + formats in `_WALLPAPER_EXTENSION_ALLOWLIST`, nothing more. + """ + if data.startswith(b"\xff\xd8\xff"): + return ".jpg" + if data.startswith(b"\x89PNG\r\n\x1a\n"): + return ".png" + if data.startswith((b"II*\x00", b"MM\x00*")): + return ".tiff" + if len(data) >= 12 and data[4:8] == b"ftyp" and data[8:12] in _HEIC_FTYP_BRANDS: + return ".heic" + return None + + +def _is_bundleable_wallpaper_location(resolved: Path) -> bool: + """Path/location checks only -- content checks (size, magic bytes) happen separately. + + `resolved` must already be the fully symlink-resolved real path: a + symlink inside an allowlisted directory that points outside it must be + rejected, so the check has to apply to the real target, not the + pre-symlink path. The allowlist dirs are resolved too, not just + `resolved` -- otherwise a symlinked tmpdir (or any symlinked component + of $HOME) compares an already-resolved path against an unresolved one, + spuriously rejecting a genuinely-allowlisted file. + """ + if not resolved.is_file(): + return False + if resolved.suffix.lower() not in _WALLPAPER_EXTENSION_ALLOWLIST: + return False + return any(resolved.is_relative_to(d.resolve()) for d in _wallpaper_bundle_source_allowlist()) + + +@dataclass(frozen=True, slots=True) +class WallpaperAsset: + """A bundled personal wallpaper image awaiting disk write by `generate_all()`. + + `filename` is a bare filename fragment (e.g. "wallpaper.heic"), not a + full path -- this module has no `hostname` to build + `hosts/darwin//assets/...` itself; `generate_all()` owns + joining it under that host's own `assets/` directory and owns all file + I/O, per this module's existing contract (it performs no I/O itself). + """ + + filename: str + data: bytes + + +def _prepare_wallpaper_asset(source_path: Path) -> WallpaperAsset | None: + """Validate and read a personal wallpaper file for bundling into generated output. + + Fails closed: any validation failure (missing file, wrong location, + disguised content, oversized) returns None, routing the caller to a + manual-report fallback instead of bundling. The wallpaper path was + captured at *scan* time -- by the time `generate` actually runs + (possibly much later, against a saved --scan-file), the file may no + longer exist; `resolve(strict=True)`'s FileNotFoundError is caught like + any other validation failure here, never left to crash `generate` over + one stale wallpaper reference. + """ + try: + resolved = source_path.resolve(strict=True) + except (OSError, FileNotFoundError): + return None + + if not _is_bundleable_wallpaper_location(resolved): + return None + + try: + if resolved.stat().st_size > _WALLPAPER_MAX_BYTES: + return None + data = resolved.read_bytes() + except OSError: + return None + + ext = _sniff_image_extension(data) + return None if ext is None else WallpaperAsset(filename=f"wallpaper{ext}", data=data) + + +def _build_wallpaper_activation_script(wallpaper_path_nix_expr: str) -> str: + """Build the wallpaper self-guarding activation-script body. + + `wallpaper_path_nix_expr` is a complete Nix expression for the + `wallpaperPath` let-binding's right-hand side -- either a `nix_str`- + quoted absolute path (a portable, OS-shipped wallpaper) or a + `toString ./assets/` path literal (a bundled personal image, + resolved relative to the importing module and content-addressed into + the Nix store). Returns a bare body expression -- see + `_build_hardware_dependent_activation_script()`'s docstring for why. + + nix-darwin removed {pre,post}UserActivation -- all activation now runs + as root, so the osascript call (which must talk to the logged-in user's + WindowServer session) is explicitly run as system.primaryUser. The path + is passed as an osascript *argument* (argv), never embedded into the + AppleScript source text itself -- a path containing a literal `"` would + otherwise terminate the embedded AppleScript string early and allow + arbitrary command injection via `&`/`do shell script`. A headless/SSH-only + activation (no WindowServer session for primaryUser -- e.g. a fleet + member switched over a remote session) would otherwise fail this whole + activation script under nix-darwin's `set -e`; `|| echo ... >&2` keeps + that failure non-fatal but still loud, rather than either aborting the + switch or failing silently. + """ + set_picture_applescript = ( + 'tell application "System Events" to tell every desktop to set picture to POSIX file (item 1 of argv)' + ) + no_gui_session_fallback = ( + 'echo "mac2nix: could not set desktop wallpaper (no GUI session for ${config.system.primaryUser}?)" >&2' + ) + return ( + "let\n" + f" wallpaperPath = {wallpaper_path_nix_expr};\n" + " in\n" + " ''\n" + " sudo -u ${config.system.primaryUser} osascript \\\n" + " -e 'on run argv' \\\n" + f" -e ' {set_picture_applescript}' \\\n" + " -e 'end run' \\\n" + " ${lib.escapeShellArg wallpaperPath} \\\n" + f" || {no_gui_session_fallback}\n" + " ''" + ) + + def _coerce_power_native_value(nix_path: str, value: Any) -> Any: if nix_path in _POWER_SLEEP_NIX_PATHS: try: @@ -188,6 +522,89 @@ def _coerce_power_native_value(nix_path: str, value: Any) -> Any: return value +def _render_activation_script_item( + result: ClassificationResult, metadata: dict[str, Any] +) -> tuple[str | None, WallpaperAsset | None, str | None]: + """Route an ACTIVATION_SCRIPT-tier result to wallpaper handling or a manual-report fallback. + + Returns `(activation_script, asset, manual_report_comment)`. + `manual_report_comment` is non-None only when the other two are both + None (the out-of-scope/bundling-failed fallback paths); `activation_script` + and `asset` can BOTH be non-None together (a successfully bundled personal + wallpaper returns its activation-script body alongside the asset to + write), so this is not a strict one-of-three. This generator only + implements the wallpaper case for ACTIVATION_SCRIPT; any other Tier-3 result (e.g. a + binary-data plist value) intentionally falls back to a manual-report + comment instead of a real activation script, since synthesizing an + arbitrary `defaults write` activation script is out of this narrow + generator's scope. Tagged `[out of scope]` here, not in + classify_preference() -- the classifier's own tier assignment for this + case (ACTIVATION_SCRIPT, automatable in principle) is correct; only + this generator's own scope decision demotes it to a comment, so the + "why" is only known at this level. + """ + if "wallpaper_path" not in metadata: + return None, None, f"[out of scope] {result.destination}" + + wallpaper_path = Path(metadata["wallpaper_path"]) + if _is_portable_wallpaper_path(wallpaper_path): + return _build_wallpaper_activation_script(nix_string(str(wallpaper_path))), None, None + + asset = _prepare_wallpaper_asset(wallpaper_path) + if asset is not None: + return _build_wallpaper_activation_script(f"toString ./assets/{asset.filename}"), asset, None + + comment = ( + f"[coverage gap] wallpaper: personal image at {wallpaper_path} could not be " + "bundled (missing, oversized, an unrecognized format, or outside " + "~/Pictures / ~/Library/Application Support/Dock) -- copy it manually and " + "reference it from a custom activation script if needed." + ) + return None, None, comment + + +def _handle_native_item( + item: _CuratedItem, + reported_hardware_dependent_paths: set[str], + native: dict[str, Any], + activation_scripts: dict[str, str], + manual_report_comments: list[str], +) -> None: + """Route a NATIVE-tier result to a native assignment, a hardware-dependent + self-guarding activation script, or (if uncoercible) a manual-report comment. + + A hardware-dependent nix_path already reported once is silently + dropped: pmset reports some keys under both "AC Power:" and "Battery + Power:" sections even though the underlying setting isn't actually + per-power-source, so the first occurrence wins, mirroring `native`'s + own dict-write dedup -- a duplicate activation script for the same + setting must be prevented exactly like the duplicate comment was. + """ + result = item.result + if result.nix_path in _POWER_HARDWARE_DEPENDENT_NIX_PATHS: + if result.nix_path in reported_hardware_dependent_paths: + return + reported_hardware_dependent_paths.add(result.nix_path) + coerced = _coerce_hardware_dependent_setting(item.value) + if coerced is None: + manual_report_comments.append( + f"[hardware-dependent] manual report: {result.nix_path} (scanned value {item.value!r}) " + "not applied -- value could not be safely coerced to on/off. Verify with " + "`systemsetup -get...` on the target Mac and set manually if supported." + ) + return + if result.nix_path == "power.restartAfterPowerFailure": + key, script = "restart_after_power_failure", _build_restart_after_power_failure_activation_script(coerced) + else: + key, script = "wake_on_lan", _build_wake_on_lan_activation_script(coerced) + activation_scripts[key] = script + return + if result.nix_path is not None: + value = result.coercion(item.value) if result.coercion else item.value + value = _coerce_power_native_value(result.nix_path, value) + native[result.nix_path] = value + + def _build_render_context(items: list[_CuratedItem]) -> dict[str, Any]: """Apply coercion, group by tier, and shape the Jinja2 render context. @@ -206,39 +623,35 @@ def _build_render_context(items: list[_CuratedItem]) -> dict[str, Any]: one entry), and a final whole-list `dict.fromkeys()` pass for unmapped fields whose destination string never varies by value (so an exact-string dedup is sufficient there). + + `activation_scripts` is keyed by an internal dedup identifier only + (never a real Nix attribute name -- nix-darwin's own activation-scripts.nix + module only ever concatenates a fixed, hardcoded set of named entries + into the script `darwin-rebuild switch` actually runs; an arbitrary + custom key like "mac2nixWallpaper" is valid Nix and builds successfully + but is silently never executed, confirmed via a real Tart-VM switch and + nix-darwin's own GitHub issue #663). Every body collected here gets + combined into the one real hook, `system.activationScripts.postActivation`, + by `nix_post_activation_script()` at the end of this function. Dict-keying + dedupes the same way `native` does, and iterating + `sorted(activation_scripts.items())` keeps render order deterministic. """ native: dict[str, Any] = {} custom_user_prefs: dict[str, dict[str, Any]] = {} custom_system_prefs: dict[str, dict[str, Any]] = {} - wallpaper_path: str | None = None + activation_scripts: dict[str, str] = {} manual_report_comments: list[str] = [] reported_hardware_dependent_paths: set[str] = set() + wallpaper_asset: WallpaperAsset | None = None for item in items: result = item.result metadata = result.metadata or {} if result.tier == ClassificationTier.NATIVE: - if result.nix_path in _POWER_HARDWARE_DEPENDENT_NIX_PATHS: - # pmset reports some keys (e.g. "autorestart", "womp") under - # both the "AC Power:" and "Battery Power:" sections even - # though the underlying setting isn't actually - # per-power-source -- the same duplication `native`'s - # dict-write already dedupes for NATIVE paths. Dedupe here - # too, or a real scan produces two identical manual-report - # comments for the same nix_path. - if result.nix_path not in reported_hardware_dependent_paths: - reported_hardware_dependent_paths.add(result.nix_path) - manual_report_comments.append( - f"manual report: {result.nix_path} (scanned value {item.value!r}) not applied -- " - "target-hardware support for this setting can't be verified from a source-machine " - "scan; setting it on unsupported hardware aborts the entire darwin-rebuild switch. " - "Verify with `systemsetup -get...` on the target Mac and set manually if supported." - ) - elif result.nix_path is not None: - value = result.coercion(item.value) if result.coercion else item.value - value = _coerce_power_native_value(result.nix_path, value) - native[result.nix_path] = value + _handle_native_item( + item, reported_hardware_dependent_paths, native, activation_scripts, manual_report_comments + ) elif result.tier == ClassificationTier.CUSTOM_PREFS: if item.domain is None or item.key is None: # Every CUSTOM_PREFS item this generator produces is @@ -248,19 +661,15 @@ def _build_render_context(items: list[_CuratedItem]) -> dict[str, Any]: bucket = custom_user_prefs if result.destination == "CustomUserPreferences" else custom_system_prefs bucket.setdefault(item.domain, {})[item.key] = item.value elif result.tier == ClassificationTier.ACTIVATION_SCRIPT: - if "wallpaper_path" in metadata: - wallpaper_path = metadata["wallpaper_path"] - else: - # This generator only implements the wallpaper case for - # ACTIVATION_SCRIPT; any other Tier-3 result (e.g. a - # binary-data plist value) intentionally falls back to a - # manual-report comment instead of a real activation - # script, since synthesizing an arbitrary `defaults write` - # script for binary data is out of this narrow generator's - # scope. - manual_report_comments.append(result.destination) - elif not metadata.get("skipped"): - manual_report_comments.append(result.destination) + script, asset, comment = _render_activation_script_item(result, metadata) + if script is not None: + activation_scripts["wallpaper"] = script + if asset is not None: + wallpaper_asset = asset + if comment is not None: + manual_report_comments.append(comment) + else: + _handle_manual_report_item(item, activation_scripts, manual_report_comments) # A real, confirmed-on-hardware case: pmset reports some keys (e.g. # "hibernatemode") under both "AC Power:" and "Battery Power:" with @@ -271,23 +680,41 @@ def _build_render_context(items: list[_CuratedItem]) -> dict[str, Any]: # strings while preserving first-occurrence order. manual_report_comments = list(dict.fromkeys(manual_report_comments)) + post_activation_script = ( + nix_post_activation_script([body for _, body in sorted(activation_scripts.items())]) + if activation_scripts + else None + ) + return { "native_items": [{"nix_path": path, "value": value} for path, value in sorted(native.items())], "custom_user_prefs": custom_user_prefs, "custom_system_prefs": custom_system_prefs, - "wallpaper_path": wallpaper_path, + "post_activation_script": post_activation_script, "manual_report_comments": manual_report_comments, + "wallpaper_asset": wallpaper_asset, } -def generate_preferences(system_state: SystemState) -> str: +@dataclass(frozen=True, slots=True) +class GeneratePreferencesResult: + """Result of `generate_preferences()`: rendered Nix source plus an optional + bundled wallpaper asset. + """ + + rendered: str + asset: WallpaperAsset | None = None + + +def generate_preferences(system_state: SystemState) -> GeneratePreferencesResult: """Render the curated preferences.nix module from one host's scan. - Returns rendered Nix source text -- it does not write the file itself; - `generate_all()` owns file I/O. + Returns rendered Nix source text (plus an optional bundled wallpaper + asset) -- it does not write anything itself; `generate_all()` owns file + I/O. """ if system_state.preferences is None or system_state.system is None: - return _EMPTY_MODULE + return GeneratePreferencesResult(rendered=_EMPTY_MODULE) items = _collect_preference_items(system_state.preferences.domains) items.extend(_collect_power_items(system_state.system.power_settings)) @@ -297,4 +724,12 @@ def generate_preferences(system_state: SystemState) -> str: items.append(_CuratedItem(value=system_state.system.wallpaper_path, result=wallpaper_result)) context = _build_render_context(items) - return render_template(_TEMPLATE_NAME, context) + asset = context.pop("wallpaper_asset") + + if system_state.system.wallpaper_scan_error is not None: + context["manual_report_comments"].append( + f"[coverage gap] wallpaper: {system_state.system.wallpaper_scan_error}" + ) + + rendered = render_template(_TEMPLATE_NAME, context) + return GeneratePreferencesResult(rendered=rendered, asset=asset) diff --git a/src/mac2nix/templates/modules/preferences.nix.j2 b/src/mac2nix/templates/modules/preferences.nix.j2 index fc66460..53e551f 100644 --- a/src/mac2nix/templates/modules/preferences.nix.j2 +++ b/src/mac2nix/templates/modules/preferences.nix.j2 @@ -10,32 +10,8 @@ <% if custom_system_prefs %> system.defaults.CustomSystemPreferences = << custom_system_prefs|nix_value|mkdefault >>; <% endif %> -<% if wallpaper_path %> - # nix-darwin removed {pre,post}UserActivation -- all activation now runs - # as root, so the osascript call (which must talk to the logged-in user's - # WindowServer session) is explicitly run as system.primaryUser. The path - # is passed as an osascript *argument* (argv), never embedded into the - # AppleScript source text itself -- a path containing a literal `"` would - # otherwise terminate the embedded AppleScript string early and allow - # arbitrary command injection via `&`/`do shell script`. - # A headless/SSH-only activation (no WindowServer session for - # primaryUser -- e.g. a fleet member switched over a remote session) - # would otherwise fail this whole activation script under nix-darwin's - # `set -e`; `|| echo ... >&2` keeps that failure non-fatal but still - # loud, rather than either aborting the switch or failing silently. - system.activationScripts.postActivation.text = lib.mkDefault ( - let - wallpaperPath = << wallpaper_path|nix_str >>; - in - '' - sudo -u ${config.system.primaryUser} osascript \ - -e 'on run argv' \ - -e ' tell application "System Events" to tell every desktop to set picture to POSIX file (item 1 of argv)' \ - -e 'end run' \ - ${lib.escapeShellArg wallpaperPath} \ - || echo "mac2nix: could not set desktop wallpaper (no GUI session for ${config.system.primaryUser}?)" >&2 - '' - ); +<% if post_activation_script %> + << post_activation_script >> <% endif %> <% for comment in manual_report_comments %> # not automated: << comment|nix_comment >> diff --git a/tests/generators/test_generate_all.py b/tests/generators/test_generate_all.py index f86c560..4df971f 100644 --- a/tests/generators/test_generate_all.py +++ b/tests/generators/test_generate_all.py @@ -4,6 +4,7 @@ import logging from pathlib import Path +from unittest.mock import patch import pytest @@ -23,6 +24,15 @@ def _full_state() -> SystemState: return _state(preferences=PreferencesResult(domains=domains), system=SystemConfig(hostname="h")) +def _state_with_wallpaper(wallpaper_path: Path) -> SystemState: + domains = [PreferencesDomain(domain_name="com.apple.dock", keys={"tilesize": 48})] + system = SystemConfig(hostname="h", wallpaper_path=wallpaper_path) + return _state(preferences=PreferencesResult(domains=domains), system=system) + + +_JPEG_MAGIC = b"\xff\xd8\xff\xe0" + b"\x00" * 50 + + class TestGenerateAll: def test_writes_preferences_and_updates_imports(self, tmp_path: Path) -> None: output_dir = tmp_path / "repo" @@ -189,3 +199,80 @@ def test_corrupt_meta_file_handled_gracefully(self, tmp_path: Path, caplog: pyte assert result.ran == {"preferences"} assert result.skipped == {} assert (host_dir / "preferences.nix").exists() + + +class TestGenerateAllWallpaperAsset: + def _bundleable_wallpaper_source(self, tmp_path: Path) -> tuple[Path, Path]: + home = tmp_path / "home" + (home / "Pictures").mkdir(parents=True) + source = home / "Pictures" / "sunset.jpg" + source.write_bytes(_JPEG_MAGIC) + return home, source + + def test_writes_bundled_wallpaper_asset_alongside_preferences(self, tmp_path: Path) -> None: + output_dir = tmp_path / "repo" + host_dir = _register_fake_host(output_dir, "myhost") + home, source = self._bundleable_wallpaper_source(tmp_path) + + with patch("mac2nix.generators.preferences.Path.home", return_value=home): + result = generate_all(_state_with_wallpaper(source), output_dir, "myhost", {"preferences"}) + + assert result.ran == {"preferences"} + asset_path = host_dir / "assets" / "wallpaper.jpg" + assert asset_path.is_file() + assert asset_path.read_bytes() == _JPEG_MAGIC + assert "toString ./assets/wallpaper.jpg" in (host_dir / "preferences.nix").read_text() + + def test_second_generate_with_unchanged_wallpaper_does_not_rewrite_asset(self, tmp_path: Path) -> None: + output_dir = tmp_path / "repo" + host_dir = _register_fake_host(output_dir, "myhost") + home, source = self._bundleable_wallpaper_source(tmp_path) + + with patch("mac2nix.generators.preferences.Path.home", return_value=home): + generate_all(_state_with_wallpaper(source), output_dir, "myhost", {"preferences"}) + asset_path = host_dir / "assets" / "wallpaper.jpg" + mtime_before = asset_path.stat().st_mtime_ns + + generate_all(_state_with_wallpaper(source), output_dir, "myhost", {"preferences"}) + + assert asset_path.stat().st_mtime_ns == mtime_before + + def test_hand_edited_asset_warns_but_still_overwrites( + self, tmp_path: Path, caplog: pytest.LogCaptureFixture + ) -> None: + output_dir = tmp_path / "repo" + host_dir = _register_fake_host(output_dir, "myhost") + home, source = self._bundleable_wallpaper_source(tmp_path) + + with patch("mac2nix.generators.preferences.Path.home", return_value=home): + generate_all(_state_with_wallpaper(source), output_dir, "myhost", {"preferences"}) + asset_path = host_dir / "assets" / "wallpaper.jpg" + asset_path.write_bytes(b"hand-edited content") + + with caplog.at_level(logging.WARNING): + generate_all(_state_with_wallpaper(source), output_dir, "myhost", {"preferences"}) + + assert any("hand-edit" in record.message for record in caplog.records) + assert asset_path.read_bytes() == _JPEG_MAGIC + + def test_corrupt_meta_file_still_writes_asset(self, tmp_path: Path) -> None: + """`_write_wallpaper_asset`'s `_read_host_meta` exception path (`meta = None` + on a corrupt/unreadable `.mac2nix-meta.json`) is only reached when a wallpaper + asset actually needs writing -- `test_corrupt_meta_file_handled_gracefully` + (this module) covers the exception path in isolation but never with a + wallpaper present, so the `meta = None` fallback inside this specific function + was untested. The asset write itself must still succeed even though the + hash-tracking/hand-edit-warning half of the function can't run without meta. + """ + output_dir = tmp_path / "repo" + host_dir = _register_fake_host(output_dir, "myhost") + home, source = self._bundleable_wallpaper_source(tmp_path) + (host_dir / ".mac2nix-meta.json").write_text("{not valid json") + + with patch("mac2nix.generators.preferences.Path.home", return_value=home): + result = generate_all(_state_with_wallpaper(source), output_dir, "myhost", {"preferences"}) + + assert result.ran == {"preferences"} + asset_path = host_dir / "assets" / "wallpaper.jpg" + assert asset_path.is_file() + assert asset_path.read_bytes() == _JPEG_MAGIC diff --git a/tests/generators/test_generate_integration.py b/tests/generators/test_generate_integration.py index 07156a5..cfc97fb 100644 --- a/tests/generators/test_generate_integration.py +++ b/tests/generators/test_generate_integration.py @@ -38,11 +38,11 @@ def _realistic_state() -> SystemState: sleep key, not just an unmapped one -- a real `nix build` failure caught power.sleep.* needing `null | positive-int | "never"`, not a raw scanned string, so this fixture must actually exercise that coercion path), - CUSTOM_PREFS (symbolichotkeys-shaped), ACTIVATION_SCRIPT (wallpaper), and - non-skipped MANUAL_REPORT entries (an unmapped pmset key, plus the two - hardware-dependent power/networking keys that a real `nix_vm` test - failure proved unsafe to auto-apply -- see preferences.py's - `_POWER_HARDWARE_DEPENDENT_NIX_PATHS`). + CUSTOM_PREFS (symbolichotkeys-shaped), ACTIVATION_SCRIPT (wallpaper, the + two hardware-dependent power/networking keys Step 14 renders as their + own self-guarding `systemsetup`-probing scripts instead of a native + assignment, and `hibernatemode`, one of Step 13's promoted `pmset -a` + cases), and a genuinely-unmapped pmset key that stays MANUAL_REPORT. """ domains = [ PreferencesDomain(domain_name="com.apple.dock", keys={"tilesize": 48}), @@ -56,9 +56,10 @@ def _realistic_state() -> SystemState: power_settings={ "ac_power.sleep": "0", # POWER_SETTING_MAP-mapped -> power.sleep.computer ("never") "battery_power.displaysleep": "10", # POWER_SETTING_MAP-mapped -> power.sleep.display (int) - "ac_power.womp": "1", # -> networking.wakeOnLan.enable (MANUAL_REPORT, hardware-dependent) - "ac_power.autorestart": "0", # -> power.restartAfterPowerFailure (MANUAL_REPORT, hardware-dependent) - "ac_power.hibernatemode": "3", # not in POWER_SETTING_MAP -> MANUAL_REPORT + "ac_power.womp": "1", # -> networking.wakeOnLan.enable (ACTIVATION_SCRIPT, hardware-dependent) + "ac_power.autorestart": "0", # -> power.restartAfterPowerFailure (ACTIVATION_SCRIPT, hardware-dependent) + "ac_power.hibernatemode": "3", # promoted (Step 13) -> ACTIVATION_SCRIPT via `pmset -a` + "ac_power.gpuswitch": "2", # not in POWER_SETTING_MAP or promoted -> MANUAL_REPORT }, wallpaper_path=Path("/System/Library/Desktop Pictures/The Cliffs.heic"), ) @@ -71,7 +72,7 @@ def _realistic_state() -> SystemState: ) -def test_generate_builds_for_real(tmp_path: Path) -> None: +def test_generate_with_hardware_dependent_settings_builds_for_real(tmp_path: Path) -> None: output_dir = tmp_path / "mac2nix-scaffold" username = getpass.getuser() token_args = _nix_extra_access_tokens_args() @@ -82,7 +83,21 @@ def test_generate_builds_for_real(tmp_path: Path) -> None: result = generate_all(_realistic_state(), output_dir, _HOSTNAME, {"preferences"}) assert result.ran == {"preferences"} - assert (output_dir / "hosts" / "darwin" / _HOSTNAME / "preferences.nix").is_file() + preferences_path = output_dir / "hosts" / "darwin" / _HOSTNAME / "preferences.nix" + assert preferences_path.is_file() + + # Step 14's two hardware-dependent activation scripts, plus Step 13's + # promoted hibernatemode case, must actually be part of what gets built + # below -- not silently dropped before this point. All three combine + # into the single real `postActivation` hook nix-darwin's own + # activation-scripts.nix module actually concatenates and executes; an + # arbitrary custom activationScripts key (e.g. "mac2nixWallpaper") is + # valid Nix and builds successfully but is silently never run. + rendered = preferences_path.read_text() + assert "system.activationScripts.postActivation.text" in rendered + assert "restart-after-power-failure" in rendered + assert "wake-on-LAN" in rendered + assert "pmset -a hibernatemode" in rendered lock_result = subprocess.run( # noqa: S603 ["nix", "flake", "lock", *token_args], # noqa: S607 diff --git a/tests/generators/test_preferences.py b/tests/generators/test_preferences.py index 9e141d6..008654c 100644 --- a/tests/generators/test_preferences.py +++ b/tests/generators/test_preferences.py @@ -5,22 +5,26 @@ import shutil import subprocess from pathlib import Path +from unittest.mock import patch import pytest from mac2nix.generators.preferences import ( CURATED_GLOBAL_DOMAIN_KEYS, CURATED_WHOLESALE_DOMAINS, + WallpaperAsset, _build_render_context, _collect_power_items, _collect_preference_items, _CuratedItem, + _prepare_wallpaper_asset, generate_preferences, ) from mac2nix.mappings.classifier import ClassificationResult, ClassificationTier, classify_wallpaper from mac2nix.models.preferences import PreferencesDomain, PreferencesResult from mac2nix.models.system import SystemConfig from mac2nix.models.system_state import SystemState +from tests._generate_helpers import assert_activation_script_neutralizes_shell_metacharacters def _domain(name: str, keys: dict) -> PreferencesDomain: @@ -115,15 +119,16 @@ def test_native_dedupes_by_nix_path(self) -> None: def test_unmapped_field_from_two_power_sources_produces_one_manual_report_comment(self) -> None: """Confirmed on real hardware via `pmset -g custom`: an unmapped key like - 'hibernatemode' can appear under both "AC Power:" and "Battery Power:" + 'ttyskeepawake' can appear under both "AC Power:" and "Battery Power:" with DIFFERENT values, but classify_system_setting()'s MANUAL_REPORT destination string for an unmapped field never includes the value -- two source-prefixed keys must still produce exactly one comment, not two - identical duplicates. + identical duplicates. Uses a field NOT in `_PROMOTED_POWER_SETTING_KEYS` + (Step 13) -- a promoted field is covered by its own dedicated test class. """ - items = _collect_power_items({"ac_power.hibernatemode": "3", "battery_power.hibernatemode": "0"}) + items = _collect_power_items({"ac_power.ttyskeepawake": "3", "battery_power.ttyskeepawake": "0"}) context = _build_render_context(items) - matching = [c for c in context["manual_report_comments"] if "hibernatemode" in c] + matching = [c for c in context["manual_report_comments"] if "ttyskeepawake" in c] assert len(matching) == 1 @pytest.mark.parametrize( @@ -180,43 +185,52 @@ def test_power_sleep_nonzero_coerces_to_int(self) -> None: @pytest.mark.parametrize( "power_settings", - [ - pytest.param({"ac_power.autorestart": "0", "ac_power.womp": "1"}, id="typical-values"), - pytest.param({"ac_power.autorestart": "", "ac_power.womp": "some-future-value"}, id="edge-case-values"), - ], + [pytest.param({"ac_power.autorestart": "0", "ac_power.womp": "1"}, id="typical-values")], ) - def test_power_hardware_dependent_settings_never_render_as_native(self, power_settings: dict[str, str]) -> None: + def test_hardware_dependent_activation_renders_script_not_native(self, power_settings: dict[str, str]) -> None: """Confirmed via a real `nix_vm` integration-test failure: nix-darwin's own modules/system/checks.nix aborts the ENTIRE `darwin-rebuild switch` whenever `power.restartAfterPowerFailure` is set at all (true OR false) on hardware that doesn't support it, and `networking.wakeOnLan.enable` carries the same unsupported-hardware risk with no nix-darwin guard at all. Neither can be - safely auto-applied from a source-machine scan -- both must route to a - manual-report comment instead of `context["native_items"]`, regardless of the - scanned value (classify_system_setting() decides tier/nix_path purely from - field_name, never from value, so this holds for typical and edge-case values - alike -- parametrized rather than duplicated as two near-identical tests). + safely rendered as a plain native assignment -- both must route to + `context["post_activation_script"]` (Step 14's self-guarding probe-then-apply + scripts, combined into the one real `postActivation` hook nix-darwin + actually executes) instead of `context["native_items"]`. """ items = _collect_power_items(power_settings) context = _build_render_context(items) native_paths = {i["nix_path"] for i in context["native_items"]} assert "power.restartAfterPowerFailure" not in native_paths assert "networking.wakeOnLan.enable" not in native_paths + assert context["post_activation_script"] is not None + assert "restart-after-power-failure" in context["post_activation_script"] + assert "wake-on-LAN" in context["post_activation_script"] + assert context["manual_report_comments"] == [] + + def test_hardware_dependent_activation_uncoercible_value_falls_back(self) -> None: + """A value that isn't cleanly "0"/"1" fails closed (Step 14): never passed + through as a raw scanned string, routed to manual-report instead of an + activation script. + """ + items = _collect_power_items({"ac_power.autorestart": "", "ac_power.womp": "some-future-value"}) + context = _build_render_context(items) + + assert context["post_activation_script"] is None assert any("power.restartAfterPowerFailure" in c for c in context["manual_report_comments"]) assert any("networking.wakeOnLan.enable" in c for c in context["manual_report_comments"]) + assert all(c.startswith("[hardware-dependent] ") for c in context["manual_report_comments"]) - def test_power_hardware_dependent_settings_dedupe_across_power_sources(self) -> None: + def test_hardware_dependent_activation_dedupes_across_power_sources(self) -> None: """pmset reports `autorestart`/`womp` under both the "AC Power:" and "Battery Power:" sections even though the underlying setting isn't actually per-power-source -- two source-prefixed keys resolving to the - same nix_path must produce exactly one manual-report comment, not two, + same nix_path must produce exactly one activation script entry, not two, mirroring the dedup NATIVE items already get via dict-write. Uses DIFFERING values across sources (confirmed real via `pmset -g custom` on real hardware, which reports different autorestart/womp values per - section) specifically because the rendered comment embeds the scanned - value -- a naive whole-list string dedup would NOT catch two differing - values for the same nix_path, so this proves the nix_path-keyed dedup - mechanism itself, not just incidental string equality. + section) to prove the nix_path-keyed dedup mechanism applies before + either value is even coerced -- the first-encountered source wins. """ items = _collect_power_items( { @@ -227,10 +241,37 @@ def test_power_hardware_dependent_settings_dedupe_across_power_sources(self) -> } ) context = _build_render_context(items) - restart_comments = [c for c in context["manual_report_comments"] if "power.restartAfterPowerFailure" in c] - wol_comments = [c for c in context["manual_report_comments"] if "networking.wakeOnLan.enable" in c] - assert len(restart_comments) == 1 - assert len(wol_comments) == 1 + combined = context["post_activation_script"] + assert combined is not None + assert combined.count("restart-after-power-failure not supported") == 1 + assert combined.count("wake-on-LAN not supported") == 1 + + @pytest.mark.nix + def test_hardware_dependent_activation_renders_valid_nix( + self, require_nix_instantiate: None, tmp_path: Path + ) -> None: + """Step 9's adversarial-escaping helper needs an injectable string value -- + these settings have none, since `_coerce_hardware_dependent_setting` only + ever accepts exactly "on"/"off" (see + test_hardware_dependent_activation_uncoercible_value_falls_back for the + fail-closed proof). A real `nix-instantiate --parse` check is the + meaningful equivalent here, matching Step 13's promoted-case tests. + """ + items = _collect_power_items({"ac_power.autorestart": "0", "ac_power.womp": "1"}) + context = _build_render_context(items) + assert context["post_activation_script"] is not None + + module_source = "{ config, lib, pkgs, ... }:\n{\n " + context["post_activation_script"] + "\n}\n" + module_path = tmp_path / "fixture.nix" + module_path.write_text(module_source) + + result = subprocess.run( # noqa: S603 + ["nix-instantiate", "--parse", str(module_path)], # noqa: S607 + capture_output=True, + text=True, + check=False, + ) + assert result.returncode == 0, result.stderr def test_custom_prefs_grouped_by_domain_and_key(self) -> None: domains = [_domain("com.apple.symbolichotkeys", {"AppleSymbolicHotKeys": {"32": {"enabled": 0}}})] @@ -274,7 +315,9 @@ def test_non_skipped_manual_report_is_rendered_as_comment(self) -> None: def test_wallpaper_activation_script_extracted_from_metadata(self) -> None: result = classify_wallpaper(Path("/System/Library/Desktop Pictures/The Cliffs.heic")) context = _build_render_context([_CuratedItem(value=Path("/x"), result=result)]) - assert context["wallpaper_path"] == "/System/Library/Desktop Pictures/The Cliffs.heic" + assert context["post_activation_script"] is not None + assert "system.activationScripts.postActivation.text" in context["post_activation_script"] + assert "The Cliffs.heic" in context["post_activation_script"] def test_binary_data_activation_script_without_wallpaper_falls_back_to_manual_report(self) -> None: """classify_preference's binary-sentinel precheck routes a `` value to @@ -291,21 +334,355 @@ def test_binary_data_activation_script_without_wallpaper_falls_back_to_manual_re context = _build_render_context(items) assert context["manual_report_comments"] == [ - "activationScripts: defaults write for com.apple.dock some-binary-pref (binary data)" + "[out of scope] activationScripts: defaults write for com.apple.dock some-binary-pref (binary data)" ] - assert context["wallpaper_path"] is None + assert context["post_activation_script"] is None + + +_JPEG_MAGIC = b"\xff\xd8\xff\xe0" + b"\x00" * 50 +_PNG_MAGIC = b"\x89PNG\r\n\x1a\n" + b"\x00" * 50 +_TIFF_LE_MAGIC = b"II*\x00" + b"\x00" * 50 +_TIFF_BE_MAGIC = b"MM\x00*" + b"\x00" * 50 +_HEIC_MAGIC = b"\x00\x00\x00\x18ftypheic" + b"\x00" * 50 +_TEXT_DISGUISED_AS_JPEG = b"this is plain text padded out to look like a real file, not a jpeg" + + +class TestWallpaperPortability: + def test_os_asset_path_renders_absolute_path_unchanged(self) -> None: + result = classify_wallpaper(Path("/System/Library/Desktop Pictures/The Cliffs.heic")) + context = _build_render_context([_CuratedItem(value=Path("/x"), result=result)]) + + assert context["post_activation_script"] is not None + assert '"/System/Library/Desktop Pictures/The Cliffs.heic"' in context["post_activation_script"] + assert context["wallpaper_asset"] is None + + def test_os_asset_path_in_nested_subdirectory_is_still_portable(self) -> None: + """macOS ships stock wallpapers in nested subdirectories (e.g. "Solid + Colors/Black.png", ".wallpapers/Sonoma Horizon/Sonoma Horizon.heic") -- + an exact-parent-match allowlist check would misclassify these as personal + files, routing a genuinely portable OS-shipped wallpaper through the + bundling path, where it fails every bundling allowlist too and silently + drops wallpaper automation entirely (found by a fresh-context adversarial + review, confirmed live against real subdirectories on the reviewing + machine). + """ + result = classify_wallpaper(Path("/System/Library/Desktop Pictures/Solid Colors/Black.png")) + context = _build_render_context([_CuratedItem(value=Path("/x"), result=result)]) + + assert context["post_activation_script"] is not None + assert '"/System/Library/Desktop Pictures/Solid Colors/Black.png"' in context["post_activation_script"] + assert context["wallpaper_asset"] is None + assert context["manual_report_comments"] == [] + + @pytest.mark.nix + def test_os_asset_path_adversarial_value_stays_quoted(self, tmp_path: Path) -> None: + """The wallpaper path is scanned (untrusted) data reaching a shell command via + `_build_wallpaper_activation_script` -- per `assert_activation_script_neutralizes_shell_metacharacters`'s + own docstring, it must be verified through that helper like every other + activation-script code path built from untrusted data. This was missing: the + only existing adversarial-injection test for the shared `nix_post_activation_script` + renderer used a synthetic body, never the real classify_wallpaper() -> + _build_render_context() pipeline. + """ + # No literal '/' in the marker -- unlike a generic string value, a wallpaper + # path is a filesystem path, so a '/' would just describe a different + # directory rather than exercise injection into the shell/Nix layers below. + marker = 'inject`ed $(id) ; "quoted"\nnewline' + adversarial_path = Path(f"/System/Library/Desktop Pictures/{marker}.heic") + result = classify_wallpaper(adversarial_path) + context = _build_render_context([_CuratedItem(value=adversarial_path, result=result)]) + + assert context["post_activation_script"] is not None + assert_activation_script_neutralizes_shell_metacharacters( + context["post_activation_script"], str(adversarial_path), tmp_path + ) + + def test_personal_path_routes_to_bundling_path(self, tmp_path: Path) -> None: + home = tmp_path / "home" + pictures = home / "Pictures" + pictures.mkdir(parents=True) + source = pictures / "sunset.jpg" + source.write_bytes(_JPEG_MAGIC) + + with patch("mac2nix.generators.preferences.Path.home", return_value=home): + result = classify_wallpaper(source) + context = _build_render_context([_CuratedItem(value=source, result=result)]) + + assert context["wallpaper_asset"] == WallpaperAsset(filename="wallpaper.jpg", data=_JPEG_MAGIC) + assert context["post_activation_script"] is not None + assert "toString ./assets/wallpaper.jpg" in context["post_activation_script"] + + def test_personal_path_outside_bundle_allowlist_falls_back_to_manual_report(self, tmp_path: Path) -> None: + """A personal path outside both bundling allowlist directories (e.g. Downloads) + must fall back to a manual-report comment, not silently drop the wallpaper. + """ + home = tmp_path / "home" + downloads = home / "Downloads" + downloads.mkdir(parents=True) + source = downloads / "sunset.jpg" + source.write_bytes(_JPEG_MAGIC) + + with patch("mac2nix.generators.preferences.Path.home", return_value=home): + result = classify_wallpaper(source) + context = _build_render_context([_CuratedItem(value=source, result=result)]) + + assert context["wallpaper_asset"] is None + assert context["post_activation_script"] is None + assert any("[coverage gap] wallpaper" in c for c in context["manual_report_comments"]) + + def test_wallpaper_scan_error_produces_manual_report_comment(self) -> None: + system = SystemConfig( + hostname="h", power_settings={}, wallpaper_scan_error="could not read desktoppicture.db (corrupt)" + ) + state = _state(preferences=PreferencesResult(domains=[]), system=system) + + generated = generate_preferences(state) + + assert "[coverage gap] wallpaper: could not read desktoppicture.db (corrupt)" in generated.rendered + + +class TestPrepareWallpaperAsset: + def _home_with_pictures(self, tmp_path: Path) -> Path: + home = tmp_path / "home" + (home / "Pictures").mkdir(parents=True) + return home + + def test_valid_image_bundles_with_fixed_destination_name(self, tmp_path: Path) -> None: + home = self._home_with_pictures(tmp_path) + source = home / "Pictures" / "my-photo.jpg" + source.write_bytes(_JPEG_MAGIC) + + with patch("mac2nix.generators.preferences.Path.home", return_value=home): + result = _prepare_wallpaper_asset(source) + + assert result == WallpaperAsset(filename="wallpaper.jpg", data=_JPEG_MAGIC) + + def test_extension_lying_about_content_is_rejected(self, tmp_path: Path) -> None: + home = self._home_with_pictures(tmp_path) + source = home / "Pictures" / "fake.jpg" + source.write_bytes(_TEXT_DISGUISED_AS_JPEG) + + with patch("mac2nix.generators.preferences.Path.home", return_value=home): + result = _prepare_wallpaper_asset(source) + + assert result is None + + def test_oversized_file_is_rejected(self, tmp_path: Path) -> None: + home = self._home_with_pictures(tmp_path) + source = home / "Pictures" / "huge.jpg" + source.write_bytes(_JPEG_MAGIC + b"\x00" * (20 * 1024 * 1024)) + + with patch("mac2nix.generators.preferences.Path.home", return_value=home): + result = _prepare_wallpaper_asset(source) + + assert result is None + + def test_path_outside_allowlist_is_rejected(self, tmp_path: Path) -> None: + home = tmp_path / "home" + downloads = home / "Downloads" + downloads.mkdir(parents=True) + source = downloads / "sunset.jpg" + source.write_bytes(_JPEG_MAGIC) + + with patch("mac2nix.generators.preferences.Path.home", return_value=home): + result = _prepare_wallpaper_asset(source) + + assert result is None + + def test_symlink_inside_allowlist_pointing_outside_is_rejected(self, tmp_path: Path) -> None: + """A symlink physically located inside ~/Pictures but pointing outside it must + still be rejected -- the check applies to the resolved real target, not the + pre-symlink path the symlink itself sits at. + """ + home = self._home_with_pictures(tmp_path) + outside_target = tmp_path / "outside" / "real.jpg" + outside_target.parent.mkdir(parents=True) + outside_target.write_bytes(_JPEG_MAGIC) + symlink_source = home / "Pictures" / "sneaky.jpg" + symlink_source.symlink_to(outside_target) + + with patch("mac2nix.generators.preferences.Path.home", return_value=home): + result = _prepare_wallpaper_asset(symlink_source) + + assert result is None + + def test_deleted_file_falls_back_gracefully_not_uncaught_error(self, tmp_path: Path) -> None: + """The wallpaper path is captured at scan time -- by the time `generate` runs + (possibly against a saved --scan-file, much later), the file may be gone. + """ + home = self._home_with_pictures(tmp_path) + source = home / "Pictures" / "gone.jpg" + + with patch("mac2nix.generators.preferences.Path.home", return_value=home): + result = _prepare_wallpaper_asset(source) + + assert result is None + + @pytest.mark.parametrize( + ("filename", "magic", "expected_ext"), + [ + pytest.param("photo.png", _PNG_MAGIC, ".png", id="png"), + pytest.param("photo.tiff", _TIFF_LE_MAGIC, ".tiff", id="tiff-little-endian"), + pytest.param("photo.tiff", _TIFF_BE_MAGIC, ".tiff", id="tiff-big-endian"), + pytest.param("photo.heic", _HEIC_MAGIC, ".heic", id="heic"), + ], + ) + def test_every_allowlisted_format_is_recognized_by_magic_bytes( + self, tmp_path: Path, filename: str, magic: bytes, expected_ext: str + ) -> None: + """`_sniff_image_extension` has a distinct branch per format in + `_WALLPAPER_EXTENSION_ALLOWLIST` -- every prior test in this class only ever + used JPEG magic bytes, leaving the PNG/TIFF(x2)/HEIC branches dead as far as + tests could tell. + """ + home = self._home_with_pictures(tmp_path) + source = home / "Pictures" / filename + source.write_bytes(magic) + + with patch("mac2nix.generators.preferences.Path.home", return_value=home): + result = _prepare_wallpaper_asset(source) + + assert result == WallpaperAsset(filename=f"wallpaper{expected_ext}", data=magic) + + def test_heic_ftyp_box_shorter_than_length_guard_is_rejected(self, tmp_path: Path) -> None: + """`_sniff_image_extension`'s HEIC branch guards with `len(data) >= 12` before + indexing `data[8:12]` -- a file too short to contain a full ftyp brand must be + rejected, not raise an IndexError-adjacent slicing bug. + """ + home = self._home_with_pictures(tmp_path) + source = home / "Pictures" / "truncated.heic" + source.write_bytes(b"\x00\x00\x00\x18ftyphe") # 10 bytes, brand field cut short + + with patch("mac2nix.generators.preferences.Path.home", return_value=home): + result = _prepare_wallpaper_asset(source) + + assert result is None + + def test_uppercase_extension_still_bundles(self, tmp_path: Path) -> None: + """`_is_bundleable_wallpaper_location` deliberately lowercases the suffix + before the allowlist check -- prove a `.JPG`-suffixed file (a real macOS + filename case, not hypothetical) still bundles rather than silently failing + an exact-case string comparison. + """ + home = self._home_with_pictures(tmp_path) + source = home / "Pictures" / "photo.JPG" + source.write_bytes(_JPEG_MAGIC) + + with patch("mac2nix.generators.preferences.Path.home", return_value=home): + result = _prepare_wallpaper_asset(source) + + assert result == WallpaperAsset(filename="wallpaper.jpg", data=_JPEG_MAGIC) + + def test_second_allowlist_directory_also_bundles(self, tmp_path: Path) -> None: + """`_wallpaper_bundle_source_allowlist()` returns two directories -- every + other test in this class only ever placed its fixture under `~/Pictures`, + leaving `~/Library/Application Support/Dock` untested. + """ + home = tmp_path / "home" + dock_dir = home / "Library" / "Application Support" / "Dock" + dock_dir.mkdir(parents=True) + source = dock_dir / "sunset.jpg" + source.write_bytes(_JPEG_MAGIC) + + with patch("mac2nix.generators.preferences.Path.home", return_value=home): + result = _prepare_wallpaper_asset(source) + + assert result == WallpaperAsset(filename="wallpaper.jpg", data=_JPEG_MAGIC) + + +class TestActivationScriptPromotion: + """Step 13's audit: `[coverage gap]` pmset keys with no native nix-darwin + option, promotable because `pmset -a` (unlike `systemsetup`) silently + no-ops on hardware where a setting doesn't apply rather than + hard-failing -- no capability probe needed. + """ + + @pytest.mark.parametrize("field_name", ["hibernatemode", "standby", "lidwake"]) + def test_activation_script_promotion_renders_script_not_comment(self, field_name: str) -> None: + items = _collect_power_items({f"ac_power.{field_name}": "1"}) + context = _build_render_context(items) + + assert context["post_activation_script"] is not None + assert f"pmset -a {field_name}" in context["post_activation_script"] + assert not any(field_name in c for c in context["manual_report_comments"]) + + def test_activation_script_promotion_non_promoted_case_still_renders_as_comment(self) -> None: + """A `[coverage gap]` field NOT in `_PROMOTED_POWER_SETTING_KEYS` (no safe CLI + path confirmed by this audit) must stay a comment, not be force-fit into an + activation script. + """ + items = _collect_power_items({"ac_power.gpuswitch": "2"}) + context = _build_render_context(items) + + assert context["post_activation_script"] is None + assert any("[coverage gap]" in c and "gpuswitch" in c for c in context["manual_report_comments"]) + + @pytest.mark.parametrize("field_name", ["hibernatemode", "standby", "lidwake"]) + def test_activation_script_promotion_uncoercible_value_falls_back(self, field_name: str) -> None: + """Fails closed like Step 14's power-boolean coercion: a value that doesn't + cleanly coerce to a non-negative int is never passed through raw. + """ + items = _collect_power_items({f"ac_power.{field_name}": "not-a-number"}) + context = _build_render_context(items) + + assert context["post_activation_script"] is None + assert any(field_name in c for c in context["manual_report_comments"]) + + @pytest.mark.parametrize("field_name", ["hibernatemode", "standby", "lidwake"]) + def test_activation_script_promotion_negative_value_falls_back(self, field_name: str) -> None: + """`_coerce_promoted_power_setting_value` requires a *non-negative* int -- a + value like "-1" parses cleanly via `int()` (so the `except (TypeError, + ValueError)` branch alone would miss it) and must still be rejected by the + `coerced >= 0` guard, not passed through raw. + """ + items = _collect_power_items({f"ac_power.{field_name}": "-1"}) + context = _build_render_context(items) + + assert context["post_activation_script"] is None + assert any(field_name in c for c in context["manual_report_comments"]) + + @pytest.mark.nix + @pytest.mark.parametrize("field_name", ["hibernatemode", "standby", "lidwake"]) + def test_activation_script_promotion_renders_valid_nix( + self, field_name: str, require_nix_instantiate: None, tmp_path: Path + ) -> None: + """Step 9's adversarial-escaping helper needs an injectable string value to + assert against -- these settings have no such value, since + `_coerce_promoted_power_setting_value` only ever accepts a non-negative int + (see test_activation_script_promotion_uncoercible_value_falls_back for the + fail-closed proof) and `pmset_key` is always one of the fixed, + module-controlled `_PROMOTED_POWER_SETTING_KEYS` literals, never scanned + data. A real `nix-instantiate --parse` check is the meaningful equivalent + here. + """ + items = _collect_power_items({f"ac_power.{field_name}": "1"}) + context = _build_render_context(items) + script = context["post_activation_script"] + assert script is not None + + module_source = f"{{ config, lib, pkgs, ... }}:\n{{\n {script}\n}}\n" + module_path = tmp_path / "fixture.nix" + module_path.write_text(module_source) + + result = subprocess.run( # noqa: S603 + ["nix-instantiate", "--parse", str(module_path)], # noqa: S607 + capture_output=True, + text=True, + check=False, + ) + assert result.returncode == 0, result.stderr class TestGeneratePreferences: def test_missing_preferences_domain_returns_empty_module_fallback(self) -> None: state = _state(preferences=None, system=SystemConfig(hostname="h")) - assert generate_preferences(state) == ( + assert generate_preferences(state).rendered == ( "# preferences/system domain not scanned -- nothing to generate\n{ config, lib, pkgs, ... }:\n{\n}\n" ) def test_missing_system_domain_returns_empty_module_fallback(self) -> None: state = _state(preferences=PreferencesResult(domains=[]), system=None) - assert generate_preferences(state) == ( + assert generate_preferences(state).rendered == ( "# preferences/system domain not scanned -- nothing to generate\n{ config, lib, pkgs, ... }:\n{\n}\n" ) @@ -321,7 +698,7 @@ def test_render(self) -> None: ) state = _state(preferences=PreferencesResult(domains=domains), system=system) - rendered = generate_preferences(state) + rendered = generate_preferences(state).rendered # NATIVE: coerced value ("Home"), not the raw scanned code ("PfHm"). assert 'system.defaults.finder.NewWindowTarget = lib.mkDefault "Home";' in rendered @@ -349,7 +726,7 @@ def test_skipped_ephemeral_key_produces_no_manual_report_comment(self) -> None: system = SystemConfig(hostname="h", power_settings={}) state = _state(preferences=PreferencesResult(domains=domains), system=system) - rendered = generate_preferences(state) + rendered = generate_preferences(state).rendered assert "not automated" not in rendered def test_newline_in_sensitive_key_cannot_inject_nix_syntax_via_manual_report_comment(self) -> None: @@ -363,7 +740,7 @@ def test_newline_in_sensitive_key_cannot_inject_nix_syntax_via_manual_report_com system = SystemConfig(hostname="h", power_settings={}) state = _state(preferences=PreferencesResult(domains=domains), system=system) - rendered = generate_preferences(state) + rendered = generate_preferences(state).rendered assert "\n }; system.activationScripts.pwned" not in rendered assert "pwned" not in rendered # the redacted key never appears in output at all @@ -383,7 +760,7 @@ def test_custom_system_preferences_bucket_is_reachable_and_renders(self) -> None system = SystemConfig(hostname="h", power_settings={}) state = _state(preferences=PreferencesResult(domains=[domain]), system=system) - rendered = generate_preferences(state) + rendered = generate_preferences(state).rendered assert "system.defaults.CustomSystemPreferences" in rendered assert "someUnmappedKey" in rendered @@ -410,17 +787,17 @@ def test_render_is_valid_nix(require_nix_instantiate: None, tmp_path: Path) -> N ) state = _state(preferences=PreferencesResult(domains=domains), system=system) - rendered = generate_preferences(state) + rendered = generate_preferences(state).rendered module_path = tmp_path / "preferences.nix" module_path.write_text(rendered) - # The two hardware-dependent settings' manual-report comment embeds the - # scanned value via !r -- confirm it actually rendered (not silently - # dropped) before the nix-instantiate check below, so this test would - # fail loudly if that branch stopped firing rather than just passing - # trivially on an empty comment section. - assert "power.restartAfterPowerFailure" in rendered - assert "networking.wakeOnLan.enable" in rendered + # The two hardware-dependent settings now render as self-guarding + # activation scripts (Step 14) -- confirm they actually rendered (not + # silently dropped) before the nix-instantiate check below, so this + # test would fail loudly if that branch stopped firing rather than + # just passing trivially on empty activation-script output. + assert "restart-after-power-failure" in rendered + assert "wake-on-LAN" in rendered result = subprocess.run( # noqa: S603 ["nix-instantiate", "--parse", str(module_path)], # noqa: S607 @@ -434,7 +811,7 @@ def test_render_is_valid_nix(require_nix_instantiate: None, tmp_path: Path) -> N @pytest.mark.nix def test_empty_module_fallback_is_valid_nix(require_nix_instantiate: None, tmp_path: Path) -> None: state = _state(preferences=None, system=None) - rendered = generate_preferences(state) + rendered = generate_preferences(state).rendered module_path = tmp_path / "preferences.nix" module_path.write_text(rendered) @@ -457,7 +834,7 @@ def test_custom_system_preferences_block_is_valid_nix(require_nix_instantiate: N system = SystemConfig(hostname="h", power_settings={}) state = _state(preferences=PreferencesResult(domains=[domain]), system=system) - rendered = generate_preferences(state) + rendered = generate_preferences(state).rendered module_path = tmp_path / "preferences.nix" module_path.write_text(rendered) @@ -477,7 +854,7 @@ def test_newline_in_key_does_not_break_nix_syntax(require_nix_instantiate: None, system = SystemConfig(hostname="h", power_settings={}) state = _state(preferences=PreferencesResult(domains=domains), system=system) - rendered = generate_preferences(state) + rendered = generate_preferences(state).rendered module_path = tmp_path / "preferences.nix" module_path.write_text(rendered) From f6f9a2ae820903fca46725ebf7e3c4d828a820d6 Mon Sep 17 00:00:00 2001 From: Will Gordon Date: Wed, 19 Aug 2026 18:02:44 -0400 Subject: [PATCH 34/35] test(vm): verifies hardware-dependent scripts on real hardware --- tests/vm/test_generate_vm.py | 24 +++++++++++++++++++++--- 1 file changed, 21 insertions(+), 3 deletions(-) diff --git a/tests/vm/test_generate_vm.py b/tests/vm/test_generate_vm.py index 81be5de..70b9596 100644 --- a/tests/vm/test_generate_vm.py +++ b/tests/vm/test_generate_vm.py @@ -124,9 +124,27 @@ async def _run(): if not ok: raise VMError(f"nix-darwin switch failed:\nstdout:\n{out}\nstderr:\n{err}") - return await _retry_transient(validator._scan_vm) - - vm_state = asyncio.run(_run()) + return await _retry_transient(validator._scan_vm), err + + vm_state, switch_err = asyncio.run(_run()) + + # Step 14 regression test: this PR's own UAT already found once, for + # real, that nix-darwin's native `power.restartAfterPowerFailure` + # option aborts the ENTIRE `darwin-rebuild switch` on hardware (this + # same Tart VM) that reports the feature as unsupported -- that's + # exactly why this task originally downgraded it to a manual-report + # comment. The switch above already completed successfully (or this + # test would have raised VMError before reaching this point) -- the + # self-guarding activation script's own probe-then-skip message must + # be what actually fired, not a lucky coincidence, since restart- + # after-power-failure is confirmed unsupported on this VM. Written to + # a file rather than relying on pytest's own truncated assertion diff + # for a multi-KB string -- a real prior run's failure message elided + # the middle of this exact string, which cost real debugging time. + (tmp_path / "switch_stderr.log").write_text(switch_err) + assert "mac2nix: restart-after-power-failure not supported on this hardware, skipped" in switch_err, ( + f"full stderr written to {tmp_path / 'switch_stderr.log'}" + ) # compute_fidelity() scores PreferencesResult.domains as a single list -- # unhashable PreferencesDomain items fall back to a whole-list string From c42f38eb1a3567255df073b4c46eacff9d9b026e Mon Sep 17 00:00:00 2001 From: Will Gordon Date: Wed, 19 Aug 2026 18:02:58 -0400 Subject: [PATCH 35/35] docs(generators): documents wallpaper bundling and report tags --- README.md | 17 +++++++++++++---- 1 file changed, 13 insertions(+), 4 deletions(-) diff --git a/README.md b/README.md index b1b5d2e..39c0656 100644 --- a/README.md +++ b/README.md @@ -37,10 +37,19 @@ and sops-nix wiring with zero hosts registered). `add-host` registers one machine at a time — including the first — generating that host's own sops-nix age key behind a mandatory backup-confirmation prompt. `generate` scans the current machine (or replays a `mac2nix scan` JSON file via -`--scan-file`) and writes that host's curated `preferences.nix`, updating -`configuration.nix`'s generated-imports section. It's safely re-runnable and -supports `--domains` to select which domains to generate (currently just -`preferences`; more are added incrementally). +`--scan-file`) and writes that host's curated `preferences.nix` — bundling a +personal desktop wallpaper image under that host's `assets/` directory when +one is set — updating `configuration.nix`'s generated-imports section. It's +safely re-runnable and supports `--domains` to select which domains to +generate (currently just `preferences`; more are added incrementally). + +Anything `generate` can't apply automatically shows up as a `# not automated:` +comment in the generated file, prefixed with why: `[sensitive]` (a value +that's deliberately never captured), `[coverage gap]` (no nix-darwin option +exists yet), `[hardware-dependent]` (the target machine's support for the +setting can't be known until activation time), or `[out of scope]` (outside +this generator's current scope). Review these and apply them manually where +needed. ## License