From bc55cf4134cf3bbee61db0cef50149f632f88018 Mon Sep 17 00:00:00 2001 From: Will Gordon Date: Fri, 31 Jul 2026 08:06:41 -0400 Subject: [PATCH 01/33] feat(cli): adds init/add-host scaffold with sops-nix safety gates Implements the foundational scaffold PR of the migration-mvp plan: a shared Nix value-rendering module (python_to_nix/nix_string/mkdefault, custom-delimiter Jinja2 environment) plus mac2nix init (host-less nix-darwin+home-manager+sops-nix framework scaffold) and mac2nix add-host (per-host registration with its own sops-nix age key, mandatory backup confirmation, and wholesale regeneration of flake.nix and .sops.yaml from on-disk metadata). Adds real nix build and VM-based nix-darwin switch integration tests, a shared Tart VM pytest fixture, an idempotent Nix bootstrap in the pre-existing Validator, and matching CI jobs. --- .github/workflows/pr-checks.yaml | 97 ++++ Makefile | 29 +- pyproject.toml | 5 +- scripts/prewarm_vm.py | 99 ++++ src/mac2nix/cli.py | 93 +++- src/mac2nix/generators/__init__.py | 2 + src/mac2nix/generators/_nix_render.py | 75 +++ src/mac2nix/generators/scaffold.py | 458 ++++++++++++++++++ src/mac2nix/templates/scaffold/flake.nix | 49 ++ src/mac2nix/templates/scaffold/gitignore | 9 + .../scaffold/hosts/darwin/configuration.nix | 19 + .../templates/scaffold/lib/helpers.nix | 50 ++ .../scaffold/modules/darwin/default.nix | 7 + .../scaffold/modules/darwin/homebrew.nix | 18 + .../scaffold/modules/home-manager/default.nix | 6 + src/mac2nix/templates/scaffold/users/user.nix | 13 + src/mac2nix/vm/manager.py | 49 ++ src/mac2nix/vm/validator.py | 126 +++-- tests/_scaffold_helpers.py | 32 ++ tests/cli/__init__.py | 0 tests/cli/test_add_host.py | 136 ++++++ tests/cli/test_init.py | 64 +++ tests/generators/__init__.py | 0 tests/generators/test_nix_render.py | 128 +++++ tests/generators/test_scaffold.py | 394 +++++++++++++++ tests/generators/test_scaffold_integration.py | 72 +++ tests/test_cli_vm.py | 59 ++- tests/vm/conftest.py | 13 +- tests/vm/test_manager.py | 77 ++- tests/vm/test_scaffold_vm.py | 121 +++++ tests/vm/test_validator.py | 43 +- tests/vm/test_vm_fixtures.py | 48 ++ tests/vm_fixtures.py | 98 ++++ 33 files changed, 2424 insertions(+), 65 deletions(-) create mode 100644 scripts/prewarm_vm.py create mode 100644 src/mac2nix/generators/__init__.py create mode 100644 src/mac2nix/generators/_nix_render.py create mode 100644 src/mac2nix/generators/scaffold.py create mode 100644 src/mac2nix/templates/scaffold/flake.nix create mode 100644 src/mac2nix/templates/scaffold/gitignore create mode 100644 src/mac2nix/templates/scaffold/hosts/darwin/configuration.nix create mode 100644 src/mac2nix/templates/scaffold/lib/helpers.nix create mode 100644 src/mac2nix/templates/scaffold/modules/darwin/default.nix create mode 100644 src/mac2nix/templates/scaffold/modules/darwin/homebrew.nix create mode 100644 src/mac2nix/templates/scaffold/modules/home-manager/default.nix create mode 100644 src/mac2nix/templates/scaffold/users/user.nix create mode 100644 tests/_scaffold_helpers.py create mode 100644 tests/cli/__init__.py create mode 100644 tests/cli/test_add_host.py create mode 100644 tests/cli/test_init.py create mode 100644 tests/generators/__init__.py create mode 100644 tests/generators/test_nix_render.py create mode 100644 tests/generators/test_scaffold.py create mode 100644 tests/generators/test_scaffold_integration.py create mode 100644 tests/vm/test_scaffold_vm.py create mode 100644 tests/vm/test_vm_fixtures.py create mode 100644 tests/vm_fixtures.py diff --git a/.github/workflows/pr-checks.yaml b/.github/workflows/pr-checks.yaml index 1680de0..9eee6f1 100644 --- a/.github/workflows/pr-checks.yaml +++ b/.github/workflows/pr-checks.yaml @@ -56,3 +56,100 @@ jobs: env: MAC2NIX_BASE_VM: macos-tahoe-base run: make test-integration + + # Runs on every pull_request, unconditional (not gated like the VM-based + # `integration` job above) — this plan's Key Decisions require every + # deliverable validated before merge, not after. No `needs:` on + # lint-and-test — runs in parallel. Inherits the workflow's default + # read-only GITHUB_TOKEN (no elevated permissions) since this job runs + # `nix build` against PR-supplied template content, including from forks. + # + # NOTE FOR WHOEVER MERGES THIS PR: this job needs to be added to the + # repository's branch-protection required-status-checks list manually via + # GitHub repo settings — a one-time change outside this workflow file's scope. + nix-integration: + if: github.event_name == 'pull_request' + runs-on: macos-latest + steps: + - uses: actions/checkout@9c091bb21b7c1c1d1991bb908d89e4e9dddfe3e0 # v7 + + - uses: astral-sh/setup-uv@11f9893b081a58869d3b5fccaea48c9e9e46f990 # v8.3.2 + with: + python-version: "3.13" + + - name: Install dependencies + run: make install + + - uses: DeterminateSystems/nix-installer-action@ef8a148080ab6020fd15196c2084a2eea5ff2d25 # v22 + + - name: Install sops and age + run: brew install sops age + + - name: Nix build integration tests + run: make test-nix + + # Job-level (not workflow-level `on.pull_request.paths`) scoping: a workflow-level + # path filter leaves a required status check permanently "Waiting for status to be + # reported" on any PR that doesn't touch the filtered paths. A job-level skip + # reports a real, non-blocking "success" status instead. + detect-changes: + if: github.event_name == 'pull_request' + runs-on: macos-latest + outputs: + vm-relevant: ${{ steps.check.outputs.vm-relevant }} + steps: + - uses: actions/checkout@9c091bb21b7c1c1d1991bb908d89e4e9dddfe3e0 # v7 + + - name: Detect VM-relevant changes + id: check + run: | + git fetch origin "${{ github.base_ref }}" --depth=1 + changed=$(git diff --name-only "origin/${{ github.base_ref }}" HEAD) + echo "$changed" + if echo "$changed" | grep -qE '^(src/mac2nix/generators/|src/mac2nix/templates/|src/mac2nix/vm/|tests/generators/|tests/vm/|tests/vm_fixtures\.py)'; then + echo "vm-relevant=true" >> "$GITHUB_OUTPUT" + else + echo "vm-relevant=false" >> "$GITHUB_OUTPUT" + fi + + # Expensive (VM boot + fresh Nix install every run) and gated: only runs when + # detect-changes says the PR touches VM-relevant paths, and even then waits for a + # maintainer's manual approval via the vm-validated environment's required reviewer. + # + # NOTE FOR WHOEVER MERGES THIS PR: the `vm-validated` GitHub Environment and its + # required reviewer need to be configured once via repo settings — this is outside + # this workflow file's scope. + vm-integration: + needs: detect-changes + if: needs.detect-changes.outputs.vm-relevant == 'true' + runs-on: macos-latest + environment: vm-validated + steps: + - uses: actions/checkout@9c091bb21b7c1c1d1991bb908d89e4e9dddfe3e0 # v7 + + - uses: astral-sh/setup-uv@11f9893b081a58869d3b5fccaea48c9e9e46f990 # v8.3.2 + with: + python-version: "3.13" + + - name: Install dependencies + run: make install + + - name: Install tart + run: brew install cirruslabs/cli/tart + + - name: Install sshpass + run: | + brew tap hudochenkov/sshpass + brew install sshpass + + - name: Install sops and age + run: brew install sops age + + # No Nix install needed on the CI host itself — Validator's whole flow (copy + # flake via SCP, bootstrap Nix, switch, re-scan) happens over SSH inside the + # VM, never locally. This deliberately does not use `make prewarm-vm`: per + # this project's own CI scope decision, it clones the raw macos-tahoe-base + # image and lets Validator._bootstrap_nix_darwin() install Nix fresh inside + # the VM every run. + - name: VM integration tests + run: make test-vm diff --git a/Makefile b/Makefile index 28c5ceb..cce7b17 100644 --- a/Makefile +++ b/Makefile @@ -1,5 +1,5 @@ .DEFAULT_GOAL := all -.PHONY: install lint format typecheck test test-integration test-quick clean all prek-install prek +.PHONY: install lint format typecheck test test-integration test-vm test-nix prewarm-vm pull-base-vm test-quick clean all prek-install prek install: uv sync @@ -18,10 +18,33 @@ typecheck: test: uv run pytest -test-integration: - tart list | grep -q "$${MAC2NIX_BASE_VM:-macos-tahoe-base}" || tart pull ghcr.io/cirruslabs/macos-tahoe-base@sha256:a8e1c8305758643f513fdccdd829c2243687c60791083dea42f73f0b7aeb435c # latest +# Pull-if-missing, shared by test-integration/test-vm rather than each duplicating +# the pinned digest. Uses mac2nix.vm.manager.pull_base_image_if_missing() (exact-name +# match + `tart clone `) rather than a plain-text `tart list | grep`, which +# false-positives: an OCI pull caches under its full registry/repo@digest string, which +# contains "macos-tahoe-base" as a substring without actually being named that. +# Always operates on the one canonical pinned name/digest pair (its own defaults) — +# deliberately ignores MAC2NIX_BASE_VM, since passing only `name=` without a matching +# `image_ref=` would silently clone the unrelated pinned macos-tahoe-base image and +# tag it under whatever name MAC2NIX_BASE_VM names (e.g. mac2nix-nix-base), producing +# a VM that looks prewarmed by name but has no Nix installed. A prewarmed image is +# only ever created for real by `make prewarm-vm`; this target's only job is to +# guarantee the fallback pinned image exists. +pull-base-vm: + uv run python -c "import asyncio; from mac2nix.vm.manager import pull_base_image_if_missing; asyncio.run(pull_base_image_if_missing())" + +test-integration: pull-base-vm uv run pytest -m integration --tb=long +test-vm: pull-base-vm + uv run pytest -m nix_vm --tb=long + +test-nix: + uv run pytest -m nix_build --tb=long + +prewarm-vm: + uv run python scripts/prewarm_vm.py + test-quick: uv run pytest -x --no-header -q diff --git a/pyproject.toml b/pyproject.toml index 09c63ac..ecb58c0 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -82,10 +82,13 @@ python_functions = "test_*" addopts = [ "--strict-markers", "--tb=short", - "-m", "not integration", + "-m", "not integration and not nix_vm and not nix_build", ] markers = [ "integration: real VM tests — require tart + sshpass + base VM image", + "nix: nix-instantiate syntax validation — require Nix on PATH", + "nix_vm: real VM-based apply-and-verify tests — require tart", + "nix_build: real nix flake lock/build tests — never skipped, require nix + age + sops + network", ] cache_dir = ".cache/pytest" diff --git a/scripts/prewarm_vm.py b/scripts/prewarm_vm.py new file mode 100644 index 0000000..76df6bf --- /dev/null +++ b/scripts/prewarm_vm.py @@ -0,0 +1,99 @@ +"""Pre-warm a Nix-enabled Tart base image for fast local VM-based validation. + +Every ``Validator.validate()`` run installs Nix from scratch inside a fresh +VM clone — correct, but slow when repeated often during local development. +This script bakes a customized, persistent base image once: pull the pinned +``macos-tahoe-base`` image if missing, clone it into ``mac2nix-nix-base``, +install Nix inside it, then stop (without deleting). Any later clone of +``mac2nix-nix-base`` already has Nix installed, and +``Validator._bootstrap_nix_darwin()``'s own idempotency check skips +reinstalling it. + +Rerun this whenever you want to refresh the baked-in Nix version. + +Usage: ``uv run python scripts/prewarm_vm.py`` (or ``make prewarm-vm``). +""" + +from __future__ import annotations + +import asyncio +import logging +import sys + +from mac2nix.vm._utils import VMError +from mac2nix.vm.manager import BASE_IMAGE_NAME, BASE_IMAGE_REF, TartVMManager, pull_base_image_if_missing +from mac2nix.vm.validator import NIX_INSTALLER_URL + +logging.basicConfig(level=logging.INFO, format="%(message)s") +logger = logging.getLogger(__name__) + +PREWARMED_VM_NAME = "mac2nix-nix-base" + + +async def _install_nix(vm: TartVMManager) -> None: + """Run the same Nix-install steps as Validator._bootstrap_nix_darwin(). + + Reuses NIX_INSTALLER_URL from validator.py rather than a second copy, but + calls exec_command() directly (this is a one-shot bake, not the + per-validate-run path Validator itself owns). + """ + installer_path = "/tmp/nix-installer.sh" # noqa: S108 + + logger.info("Downloading Nix installer...") + # Two things collide here, verified empirically against a real VM: (1) + # curl on this VM's own build rejects the combined "--proto=https" form + # outright ("option --proto=https: is unknown"); (2) the separate-argv + # form ["--proto", "=https"] hits the VM's remote login shell (zsh), + # which treats a bare leading-"=" word as its own command-path-expansion + # syntax and fails with "https not found" before curl ever runs. + # Wrapping in `bash -c` with `=https` single-quoted sidesteps both: bash + # has no equals-expansion, and curl accepts the quoted, space-separated + # two-word form. + download_cmd = f"curl --proto '=https' --tlsv1.2 -sSf -L {NIX_INSTALLER_URL} -o {installer_path}" + ok, _out, err = await vm.exec_command(["bash", "-c", download_cmd], timeout=60) + if not ok: + raise VMError(f"Failed to download Nix installer: {err.strip()}") + + ok, _out, err = await vm.exec_command(["chmod", "+x", installer_path]) + if not ok: + raise VMError(f"chmod +x nix-installer.sh failed: {err.strip()}") + + logger.info("Installing Nix (this can take a few minutes)...") + ok, _out, err = await vm.exec_command([installer_path, "install", "--no-confirm"], timeout=300) + if not ok: + raise VMError(f"Nix installation failed: {err.strip()}") + + logger.info("Nix installed successfully") + + +async def _prewarm() -> None: + if not TartVMManager.is_available(): + raise VMError("tart CLI is not available — install tart to prewarm a VM") + + await pull_base_image_if_missing(name=BASE_IMAGE_NAME, image_ref=BASE_IMAGE_REF) + + logger.info("Cloning %r -> %r", BASE_IMAGE_NAME, PREWARMED_VM_NAME) + vm = TartVMManager(BASE_IMAGE_NAME) + await vm.clone(PREWARMED_VM_NAME) + await vm.start() + try: + await _install_nix(vm) + finally: + # Deliberately not deleted — the whole point is a persistent, reusable + # disk other clones can be made from. + await vm.stop() + + logger.info("%r is ready. Pass --base-vm %s to skip the Nix install wait.", PREWARMED_VM_NAME, PREWARMED_VM_NAME) + + +def main() -> int: + try: + asyncio.run(_prewarm()) + except VMError as exc: + logger.error("prewarm failed: %s", exc) + return 1 + return 0 + + +if __name__ == "__main__": + sys.exit(main()) diff --git a/src/mac2nix/cli.py b/src/mac2nix/cli.py index 0879656..a22aead 100644 --- a/src/mac2nix/cli.py +++ b/src/mac2nix/cli.py @@ -3,6 +3,8 @@ from __future__ import annotations import asyncio +import getpass +import re import time import uuid from collections import Counter @@ -16,6 +18,7 @@ from rich.table import Table from rich.text import Text +from mac2nix.generators.scaffold import add_host, 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 @@ -204,6 +207,84 @@ def _vm_options(f: click.decorators.FC) -> click.decorators.FC: ) +@main.command() +@click.argument("output_dir", type=click.Path(path_type=Path)) +def init(output_dir: Path) -> None: + """Scaffold a host-less nix-darwin + home-manager + sops-nix framework.""" + try: + init_framework(output_dir) + except Exception as exc: + raise click.ClickException(str(exc)) from exc + + click.echo(f"Framework scaffolded at {output_dir}") + click.echo("Next: mac2nix add-host --hostname [--username ] to register a machine.") + + +_HOSTNAME_RE = re.compile(r"^[a-z][a-z0-9-]*$") +_USERNAME_RE = re.compile(r"^[a-z_][a-z0-9_-]*$") + + +def _validate_hostname(_ctx: click.Context, _param: click.Parameter, value: str) -> str: + if not _HOSTNAME_RE.match(value): + msg = "hostname must match ^[a-z][a-z0-9-]*$ (lowercase alphanumeric and hyphens, starting with a letter)" + raise click.BadParameter(msg) + return value + + +def _validate_username(_ctx: click.Context, _param: click.Parameter, value: str) -> str: + if not _USERNAME_RE.match(value): + msg = "username must match ^[a-z_][a-z0-9_-]*$" + raise click.BadParameter(msg) + return value + + +@main.command("add-host") +@click.argument("output_dir", type=click.Path(exists=True, file_okay=False, path_type=Path)) +@click.option( + "--hostname", + required=True, + callback=_validate_hostname, + help="Hostname to register (lowercase alphanumeric + hyphens, starting with a letter).", +) +@click.option( + "--username", + default=getpass.getuser, + callback=_validate_username, + show_default="current OS user", + help="Account username for this host.", +) +@click.option( + "--system", + default="aarch64-darwin", + show_default=True, + type=click.Choice(["aarch64-darwin", "x86_64-darwin"]), + help="Darwin system double.", +) +def add_host_cmd(output_dir: Path, hostname: str, username: str, system: str) -> None: + """Register a host with an existing mac2nix-scaffolded framework.""" + + def _confirm_backup(fingerprint: str) -> bool: + click.echo(f"Public key fingerprint: {fingerprint}") + return ( + click.prompt("Type CONFIRMED once the private key has been backed up to a password manager", default="") + == "CONFIRMED" + ) + + try: + fingerprint = add_host(output_dir, hostname, username, system, confirm_backup=_confirm_backup) + except Exception as exc: + raise click.ClickException(str(exc)) from exc + + key_path = f"/Users/{username}/.config/sops/age/keys.txt" + click.echo(f"Host {hostname!r} registered (age key fingerprint: {fingerprint}).") + click.echo(f"Age key stored at {key_path} — make sure it's backed up somewhere safe.") + click.echo(f"Next: run `nix flake lock` inside {output_dir} before the first build for this host.") + click.echo( + "Reminder: push this repo as a PRIVATE GitHub repo — scan-derived configuration isn't vetted " + "for public-repo exposure the way sops-encrypted secrets are." + ) + + @main.command() def generate() -> None: """Generate nix-darwin configuration from a scan snapshot.""" @@ -223,10 +304,18 @@ def generate() -> None: type=click.Path(exists=True, dir_okay=False, path_type=Path), help="Source SystemState JSON produced by 'mac2nix scan'.", ) +@click.option( + "--mac2nix-source", + default="github:gordon-code/mac2nix", + show_default=True, + help="mac2nix source the VM re-scans with — a flake ref, or a local checkout path (e.g. '.') " + "to validate not-yet-published code.", +) @_vm_options -def validate( +def validate( # noqa: PLR0913 flake_path: Path, scan_file: Path, + mac2nix_source: str, base_vm: str, vm_user: str, vm_password: str, @@ -245,7 +334,7 @@ async def _run() -> None: clone_name = f"mac2nix-validate-{uuid.uuid4().hex[:8]}" await vm.clone(clone_name) await vm.start() - result = await Validator(vm).validate(flake_path, source_state) + result = await Validator(vm, mac2nix_source=mac2nix_source).validate(flake_path, source_state) if result.errors: click.echo("Validation errors:", err=True) diff --git a/src/mac2nix/generators/__init__.py b/src/mac2nix/generators/__init__.py new file mode 100644 index 0000000..3599ca6 --- /dev/null +++ b/src/mac2nix/generators/__init__.py @@ -0,0 +1,2 @@ +class Mac2NixError(Exception): + """Base for every mac2nix-raised, user-facing error across scaffold.py and generate_all().""" diff --git a/src/mac2nix/generators/_nix_render.py b/src/mac2nix/generators/_nix_render.py new file mode 100644 index 0000000..c19c168 --- /dev/null +++ b/src/mac2nix/generators/_nix_render.py @@ -0,0 +1,75 @@ +"""Shared Nix value rendering and Jinja2 template plumbing for generators.""" + +from __future__ import annotations + +import re +from typing import Any + +import jinja2 + +_BARE_KEY_RE = re.compile(r"^[A-Za-z_][A-Za-z0-9_'-]*$") + + +def nix_string(s: str) -> str: + """Render a Python string as a double-quoted Nix string literal.""" + escaped = s.replace("\\", "\\\\").replace('"', '\\"').replace("${", "\\${") + return f'"{escaped}"' + + +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, (int, float)): + return str(value) + if value is None: + return "null" + if isinstance(value, str): + return nix_string(value) + if isinstance(value, list): + return "[ " + " ".join(python_to_nix(v) for v in value) + " ]" + if isinstance(value, dict): + items = " ".join( + f"{key if _BARE_KEY_RE.match(key) else nix_string(key)} = {python_to_nix(val)};" + for key, val in value.items() + ) + return "{ " + items + " }" + msg = f"cannot render {type(value).__name__!r} as a Nix literal" + raise TypeError(msg) + + +def nix_mkdefault(nix_expr: str) -> str: + """Wrap a rendered Nix expression in `lib.mkDefault`.""" + return f"lib.mkDefault {nix_expr}" + + +def setup_jinja_env(loader: jinja2.BaseLoader | None = None) -> jinja2.Environment: + """Build the Jinja2 environment shared by every mac2nix Nix template. + + Uses `<% %>`/`<< >>` delimiters instead of Jinja2's defaults so they don't + collide with Nix's `{ }` attribute-set syntax, which appears throughout + every template. + """ + env = jinja2.Environment( + loader=loader or jinja2.PackageLoader("mac2nix", "templates/modules"), + block_start_string="<%", + block_end_string="%>", + variable_start_string="<<", + variable_end_string=">>", + trim_blocks=True, + lstrip_blocks=True, + autoescape=False, # noqa: S701 -- output is Nix, not HTML; escaping would corrupt syntax + ) + env.filters["nix_value"] = python_to_nix + env.filters["nix_str"] = nix_string + env.filters["mkdefault"] = nix_mkdefault + return env + + +def render_template( + template_name: str, + context: dict[str, Any], + loader: jinja2.BaseLoader | None = None, +) -> str: + """Render a named template through the shared mac2nix Jinja2 environment.""" + return setup_jinja_env(loader).get_template(template_name).render(**context) diff --git a/src/mac2nix/generators/scaffold.py b/src/mac2nix/generators/scaffold.py new file mode 100644 index 0000000..19de62d --- /dev/null +++ b/src/mac2nix/generators/scaffold.py @@ -0,0 +1,458 @@ +"""Scaffold generation — init_framework() (host-less framework) and add_host() (per-host registration).""" + +from __future__ import annotations + +import hashlib +import importlib.resources +import json +import logging +import os +import re +import shutil +import stat +import subprocess +from collections.abc import Callable +from pathlib import Path +from typing import Any + +import yaml + +from mac2nix.generators import Mac2NixError +from mac2nix.generators._nix_render import nix_string + +logger = logging.getLogger(__name__) + +_HOSTS_BEGIN = "# MAC2NIX:HOSTS:BEGIN" +_HOSTS_END = "# MAC2NIX:HOSTS:END" + +_META_FILENAME = ".mac2nix-meta.json" +_STATE_FILENAME = ".mac2nix-state.json" + +_TEMPLATES_ROOT = ("templates", "scaffold") + + +class ScaffoldError(Mac2NixError): + """Raised for scaffold-generation failures — init_framework(), add_host().""" + + +def init_framework(output_dir: Path) -> None: + """Scaffold a host-less nix-darwin + home-manager + sops-nix framework at *output_dir*. + + Copies the static framework templates byte-for-byte — no hostname, + username, or system is bound yet; registering a host is `add_host()`'s job. + + `templates/scaffold/hosts/` and `templates/scaffold/users/` hold + `add_host()`'s own per-host/per-user templates (with unsubstituted + `__HOSTNAME__`/`__USERNAME__` placeholders) — they share the same + template root as the framework-level files but must never be copied by + `init`, which is host-less by design. + + Raises :exc:`ScaffoldError` if *output_dir* already exists and is non-empty. + """ + if output_dir.exists() and any(output_dir.iterdir()): + msg = f"{output_dir} already exists and is not empty — init never overwrites an existing repo" + raise ScaffoldError(msg) + + output_dir.mkdir(parents=True, exist_ok=True) + + scaffold_root = importlib.resources.files("mac2nix").joinpath("templates", "scaffold") + with importlib.resources.as_file(scaffold_root) as real_scaffold_root: + shutil.copytree( + real_scaffold_root, + output_dir, + dirs_exist_ok=True, + ignore=shutil.ignore_patterns("hosts", "users"), + ) + + gitignore_path = output_dir / "gitignore" + if gitignore_path.is_file(): + gitignore_path.rename(output_dir / ".gitignore") + + +# --------------------------------------------------------------------------- +# add_host() and its Step 4-6 helpers +# --------------------------------------------------------------------------- + + +def _read_template(*parts: str) -> str: + return importlib.resources.files("mac2nix").joinpath(*_TEMPLATES_ROOT, *parts).read_text() + + +def _render_placeholders(text: str, hostname: str, username: str) -> str: + return text.replace("__HOSTNAME__", hostname).replace("__USERNAME__", username) + + +def _age_key_path(username: str, key_dir: Path | None = None) -> Path: + return (key_dir or Path(f"/Users/{username}") / ".config" / "sops" / "age") / "keys.txt" + + +def _write_host_config(host_dir: Path, hostname: str, username: str) -> None: + host_dir.mkdir(parents=True) + template = _read_template("hosts", "darwin", "configuration.nix") + (host_dir / "configuration.nix").write_text(_render_placeholders(template, hostname, username)) + + +def _write_user_file(user_file: Path, hostname: str, username: str) -> None: + template = _read_template("users", "user.nix") + user_file.parent.mkdir(parents=True, exist_ok=True) + user_file.write_text(_render_placeholders(template, hostname, username)) + + +def generate_age_key(username: str, *, key_dir: Path | None = None) -> str: + """Generate a new sops-nix age key for *username* and return its public fingerprint. + + `key_dir` is a keyword-only test seam — real callers always leave it + `None`, which resolves the key's parent directory from `username` itself + (never `Path.home()`), matching the path `lib/helpers.nix` interpolates + for `sops.age.keyFile`. + + Raises :exc:`ScaffoldError` if `age-keygen` is unavailable, or if a key + already exists at the target path — an existing key is never regenerated + or overwritten, since doing so orphans every secret already encrypted to it. + """ + if shutil.which("age-keygen") is None: + msg = "age-keygen is not available — install age (e.g. `nix shell nixpkgs#age`) to use add-host" + raise ScaffoldError(msg) + + key_path = _age_key_path(username, key_dir) + + if key_path.exists(): + msg = ( + f"age key already exists at {key_path} — add-host will not regenerate or overwrite it, " + "since doing so orphans every secret already encrypted to the old key" + ) + raise ScaffoldError(msg) + + key_path.parent.mkdir(parents=True, exist_ok=True) + + result = subprocess.run( # noqa: S603 + ["age-keygen", "-o", str(key_path)], # noqa: S607 + capture_output=True, + text=True, + check=False, + ) + if result.returncode != 0: + raise ScaffoldError(f"age-keygen failed: {result.stderr.strip()}") + + # From here on, a real private key file exists at key_path. Self-clean on + # any failure so this function's contract is "either returns a valid + # fingerprint or leaves no file behind" — independent of whether the + # caller correctly tracks that generation happened. + try: + key_path.chmod(0o600) + actual_mode = stat.S_IMODE(key_path.stat().st_mode) + if actual_mode != 0o600: + raise ScaffoldError(f"failed to set age key file permissions to 0600 (got {oct(actual_mode)})") + return _parse_age_public_key(key_path) + except Exception: + key_path.unlink(missing_ok=True) + raise + + +def _parse_age_public_key(key_path: Path) -> str: + for line in key_path.read_text().splitlines(): + if line.startswith("# public key:"): + return line.removeprefix("# public key:").strip() + raise ScaffoldError(f"could not find a '# public key:' comment line in {key_path}") + + +def _load_all_host_metadata(output_dir: Path) -> list[dict[str, Any]]: + meta_paths = sorted((output_dir / "hosts" / "darwin").glob(f"*/{_META_FILENAME}")) + return [json.loads(p.read_text()) for p in meta_paths] + + +def _warn_if_hand_edited(output_dir: Path, current_inner: str) -> None: + state_path = output_dir / _STATE_FILENAME + if not state_path.is_file(): + return + try: + stored_hash = json.loads(state_path.read_text())["flake_hosts_block_hash"] + except (json.JSONDecodeError, KeyError, OSError): + return + + if hashlib.sha256(current_inner.encode()).hexdigest() != stored_hash: + logger.warning( + "flake.nix's MAC2NIX:HOSTS block doesn't match what add-host last wrote there " + "(likely a hand-edit, or corrupted/manually-deleted host metadata) — " + "this regeneration will overwrite it." + ) + + +def _store_flake_hosts_hash(output_dir: Path, inner: str) -> None: + state_path = output_dir / _STATE_FILENAME + new_hash = hashlib.sha256(inner.encode()).hexdigest() + state_path.write_text(json.dumps({"flake_hosts_block_hash": new_hash}, indent=2)) + + +def _regenerate_flake_hosts_block(output_dir: Path, metas: list[dict[str, Any]]) -> None: + """Wholesale-regenerate flake.nix's sentinel-bounded darwinConfigurations block. + + Rebuilds from *metas* (every `.mac2nix-meta.json` sidecar currently on + disk, sorted — see `_load_all_host_metadata()`), never by patching the + file in place — correct regardless of call order or prior state. + """ + flake_path = output_dir / "flake.nix" + content = flake_path.read_text() + + # Anchor to the end of the BEGIN sentinel's own line (not just past the + # bare "# MAC2NIX:HOSTS:BEGIN" substring) — the real line also carries a + # trailing "-- generated by ...; do not edit by hand" comment, which must + # survive regeneration verbatim rather than being treated as + # regeneratable content and discarded on the very first add-host call. + begin_marker_end = content.index("\n", content.index(_HOSTS_BEGIN)) + 1 + end_marker_start = content.index(_HOSTS_END) + old_inner = content[begin_marker_end:end_marker_start] + + host_lines = [ + f" {nix_string(m['hostname'])} = mkDarwinSystem {{ hostname = {nix_string(m['hostname'])}; " + f"system = {nix_string(m['system'])}; users = [ {nix_string(m['username'])} ]; }};" + for m in metas + ] + new_inner = "".join(f"{line}\n" for line in host_lines) + " " + + _warn_if_hand_edited(output_dir, old_inner) + + flake_path.write_text(content[:begin_marker_end] + new_inner + content[end_marker_start:]) + _store_flake_hosts_hash(output_dir, new_inner) + + +def _regenerate_sops_yaml(output_dir: Path, metas: list[dict[str, Any]]) -> None: + """Fully rewrite .sops.yaml from *metas* (every host's `.mac2nix-meta.json` sidecar), then verify it. + + Writes first, then reads the result back and verifies it structurally — + a real, load-bearing check, not just a test hook: raises `ScaffoldError` + (aborting the whole `add_host()` call, which triggers rollback) if two + hosts share an age key, or if any rule's `path_regex`/`key_groups` don't + scope correctly to their own host. + """ + creation_rules = [ + { + "path_regex": f"secrets/{re.escape(m['hostname'])}\\.yaml$", + "key_groups": [{"age": [m["age_public_key"]]}], + } + for m in metas + ] + + sops_path = output_dir / ".sops.yaml" + sops_path.write_text(yaml.safe_dump({"creation_rules": creation_rules}, sort_keys=False)) + + _verify_sops_yaml(sops_path, metas) + + +def _verify_sops_yaml(sops_path: Path, metas: list[dict[str, Any]]) -> None: + try: + parsed = yaml.safe_load(sops_path.read_text()) + except yaml.YAMLError as exc: + raise ScaffoldError(f"regenerated .sops.yaml is not valid YAML: {exc}") from exc + + rules = parsed.get("creation_rules", []) if parsed else [] + if len(rules) != len(metas): + msg = f".sops.yaml verification failed: expected {len(metas)} creation_rules, found {len(rules)}" + raise ScaffoldError(msg) + + fingerprints = [m["age_public_key"] for m in metas] + if len(set(fingerprints)) != len(fingerprints): + msg = "two or more hosts share the same age public key — aborting .sops.yaml regeneration" + raise ScaffoldError(msg) + + by_hostname = {m["hostname"]: m for m in metas} + for rule in rules: + key_groups = rule.get("key_groups", []) + if len(key_groups) != 1 or set(key_groups[0].keys()) != {"age"} or len(key_groups[0]["age"]) != 1: + raise ScaffoldError(f".sops.yaml verification failed: malformed key_groups shape in rule {rule!r}") + fingerprint = key_groups[0]["age"][0] + + path_regex = rule.get("path_regex", "") + matched_hosts = [m["hostname"] for m in metas if re.search(path_regex, f"secrets/{m['hostname']}.yaml")] + if len(matched_hosts) != 1: + msg = ( + f".sops.yaml verification failed: path_regex {path_regex!r} matches " + f"{matched_hosts}, expected exactly one host" + ) + raise ScaffoldError(msg) + + expected_fingerprint = by_hostname[matched_hosts[0]]["age_public_key"] + if fingerprint != expected_fingerprint: + msg = f".sops.yaml verification failed: rule for {matched_hosts[0]!r} has a mismatched fingerprint" + raise ScaffoldError(msg) + + +def _run_sops(cmd: list[str], cwd: Path, env: dict[str, str], step: str) -> subprocess.CompletedProcess[str]: + result = subprocess.run(cmd, cwd=cwd, env=env, capture_output=True, text=True, check=False) # noqa: S603 + if result.returncode != 0: + raise ScaffoldError(f"sops {step} failed: {result.stderr.strip()}") + return result + + +def _create_host_secrets_file(output_dir: Path, hostname: str, age_key_path: Path) -> None: + """Create+smoke-test `secrets/{hostname}.yaml`, then overwrite it with an empty document. + + Uses SOPS_AGE_KEY_FILE (never argv) to scope the sops subprocess to this + host's own key, exercising the `path_regex` entry `_regenerate_sops_yaml()` + just wrote. + """ + if shutil.which("sops") is None: + msg = "sops is not available — install sops (e.g. `nix shell nixpkgs#sops`) to use add-host" + raise ScaffoldError(msg) + + secrets_dir = output_dir / "secrets" + secrets_dir.mkdir(exist_ok=True) + secrets_path = secrets_dir / f"{hostname}.yaml" + + env = {**os.environ, "SOPS_AGE_KEY_FILE": str(age_key_path)} + + secrets_path.write_text(yaml.safe_dump({"_mac2nix_smoke_test": "placeholder"})) + _run_sops(["sops", "--encrypt", "--in-place", str(secrets_path)], output_dir, env, "smoke-test encrypt") + + decrypted = _run_sops(["sops", "--decrypt", str(secrets_path)], output_dir, env, "smoke-test decrypt") + if "_mac2nix_smoke_test" not in decrypted.stdout: + msg = f"sops decrypt round-trip for {secrets_path} did not return the expected smoke-test key" + raise ScaffoldError(msg) + + secrets_path.write_text(yaml.safe_dump({})) + _run_sops(["sops", "--encrypt", "--in-place", str(secrets_path)], output_dir, env, "final empty-document encrypt") + + +def _cleanup_after_failed_add_host( # noqa: PLR0913 + output_dir: Path, + hostname: str, + host_dir: Path, + user_file: Path, + *, + user_file_created: bool, + age_key_generated: bool, + username: str, +) -> None: + """Remove only what this failed `add_host()` call itself created, then self-heal flake.nix/.sops.yaml. + + Re-invokes `_regenerate_flake_hosts_block()`/`_regenerate_sops_yaml()` + against the now-reduced host set so neither file ends up referencing a + host whose directory no longer exists. That re-invocation's own failure + is logged, never raised — the original exception is always what + propagates, since preserving the real root cause matters more than a + secondary cleanup-time failure. + """ + shutil.rmtree(host_dir, ignore_errors=True) + + if user_file_created: + user_file.unlink(missing_ok=True) + + if age_key_generated: + _age_key_path(username).unlink(missing_ok=True) + + (output_dir / "secrets" / f"{hostname}.yaml").unlink(missing_ok=True) + + try: + metas = _load_all_host_metadata(output_dir) + _regenerate_flake_hosts_block(output_dir, metas) + _regenerate_sops_yaml(output_dir, metas) + except Exception: + logger.warning( + "Cleanup after failed add-host for %r could not fully regenerate flake.nix/.sops.yaml — " + "these files may still reference the removed host and require manual inspection.", + hostname, + ) + + +def add_host( + output_dir: Path, + hostname: str, + username: str, + system: str = "aarch64-darwin", + *, + confirm_backup: Callable[[str], bool], +) -> str: + """Register a host with the framework at *output_dir*. + + Generates this host's own sops-nix age key, writes its per-host + configuration/user files, and wholesale-regenerates flake.nix's host + block and .sops.yaml from every registered host's metadata. + + `confirm_backup` is invoked once the new age key exists, receiving its + public fingerprint; returning `False` aborts before flake/sops + regeneration ever runs. + + Trust boundary: *hostname*/*username* are used directly in path + construction (`output_dir / "hosts" / "darwin" / hostname`) and are + **not** validated against a character allowlist here — that check is the + CLI layer's job (`^[a-z][a-z0-9-]*$`/`^[a-z_][a-z0-9_-]*$`), applied + before this function is ever called. A hostname like `"../../evil"` + passed directly to this function would escape `output_dir`. `nix_string()` + escaping in `_regenerate_flake_hosts_block()` is a separate, independent + layer guarding against Nix-syntax corruption — it does not substitute for + the CLI's own path-traversal check. + + Returns the new host's age key public fingerprint. + Raises :exc:`ScaffoldError` on any failure — anything this specific call + created is rolled back, and flake.nix/.sops.yaml are re-regenerated + against the resulting, unchanged host set. + + Not safe for concurrent invocations against the same *output_dir* — + its read-glob-write cycle on flake.nix/.sops.yaml assumes single-operator, + sequential use, matching this tool's actual CLI-driven usage pattern. + """ + # Resolve to absolute up front: _create_host_secrets_file() runs sops + # with cwd=output_dir *and* a secrets-file argument derived from + # output_dir — if output_dir were relative, sops would resolve that + # argument a second time against its own (already output_dir-rooted) + # cwd, doubling the path and failing with "non-existent file". Verified + # empirically: `mac2nix add-host some/relative/path ...` hit exactly + # this before this fix. + output_dir = output_dir.resolve() + + flake_path = output_dir / "flake.nix" + if not flake_path.is_file(): + 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: + msg = f"{output_dir} is not a mac2nix-scaffolded framework — run `mac2nix init` first" + raise ScaffoldError(msg) + + host_dir = output_dir / "hosts" / "darwin" / hostname + if host_dir.exists(): + raise ScaffoldError(f"host {hostname!r} is already registered") + + user_file = output_dir / "users" / f"{username}.nix" + user_file_created = False + age_key_generated = False + + try: + _write_host_config(host_dir, hostname, username) + + if not user_file.exists(): + user_file_created = True + _write_user_file(user_file, hostname, username) + + key_path = _age_key_path(username) + if key_path.resolve().is_relative_to(output_dir.resolve()): + msg = f"refusing to generate an age key inside {output_dir} — keys must live outside the framework repo" + raise ScaffoldError(msg) + + fingerprint = generate_age_key(username) + age_key_generated = True + + if not confirm_backup(fingerprint): + msg = "age key backup not confirmed — aborting before flake/sops regeneration" + raise ScaffoldError(msg) + + meta = {"hostname": hostname, "username": username, "system": system, "age_public_key": fingerprint} + (host_dir / _META_FILENAME).write_text(json.dumps(meta, indent=2)) + + metas = _load_all_host_metadata(output_dir) + _regenerate_flake_hosts_block(output_dir, metas) + _regenerate_sops_yaml(output_dir, metas) + _create_host_secrets_file(output_dir, hostname, key_path) + except Exception: + _cleanup_after_failed_add_host( + output_dir, + hostname, + host_dir, + user_file, + user_file_created=user_file_created, + age_key_generated=age_key_generated, + username=username, + ) + raise + + return fingerprint diff --git a/src/mac2nix/templates/scaffold/flake.nix b/src/mac2nix/templates/scaffold/flake.nix new file mode 100644 index 0000000..f073a59 --- /dev/null +++ b/src/mac2nix/templates/scaffold/flake.nix @@ -0,0 +1,49 @@ +{ + description = "mac2nix-managed nix-darwin configuration"; + + inputs = { + nixpkgs.url = "github:NixOS/nixpkgs/nixos-unstable"; + + darwin = { + url = "github:nix-darwin/nix-darwin"; + inputs.nixpkgs.follows = "nixpkgs"; + }; + + home-manager = { + url = "github:nix-community/home-manager"; + inputs.nixpkgs.follows = "nixpkgs"; + }; + + nix-homebrew.url = "github:zhaofengli/nix-homebrew"; + + homebrew-core = { + url = "github:homebrew/homebrew-core"; + flake = false; + }; + + homebrew-cask = { + url = "github:homebrew/homebrew-cask"; + flake = false; + }; + + mac-app-util.url = "github:hraban/mac-app-util"; + + determinate.url = "https://flakehub.com/f/DeterminateSystems/determinate/3"; + + sops-nix = { + url = "github:Mic92/sops-nix"; + inputs.nixpkgs.follows = "nixpkgs"; + }; + }; + + outputs = inputs: + let + mkDarwinSystem = import ./lib/helpers.nix { inherit inputs; }; + in + { + darwinConfigurations = { + # MAC2NIX:HOSTS:BEGIN -- generated by `mac2nix add-host`; do not edit by hand + # MAC2NIX:HOSTS:END + }; + }; +} diff --git a/src/mac2nix/templates/scaffold/gitignore b/src/mac2nix/templates/scaffold/gitignore new file mode 100644 index 0000000..24ce4bc --- /dev/null +++ b/src/mac2nix/templates/scaffold/gitignore @@ -0,0 +1,9 @@ +.direnv/ +result +result-* + +# Defense-in-depth — real age keys are always generated outside this repo +# (see `mac2nix add-host`), but these patterns guard against ever +# accidentally checking one in. +age/keys.txt +*.agekey diff --git a/src/mac2nix/templates/scaffold/hosts/darwin/configuration.nix b/src/mac2nix/templates/scaffold/hosts/darwin/configuration.nix new file mode 100644 index 0000000..5ad649a --- /dev/null +++ b/src/mac2nix/templates/scaffold/hosts/darwin/configuration.nix @@ -0,0 +1,19 @@ +# mac2nix: host configuration for __HOSTNAME__ +{ config, lib, pkgs, ... }: + +{ + # nix-darwin doesn't manage real macOS accounts and has no default for + # users.users..home — but home-manager's own darwin integration reads + # it unconditionally (config.users.users..home) to derive + # home.homeDirectory, so leaving it unset makes that derivation null and + # fails the module system's type check. Set explicitly, not inferred. + users.users.__USERNAME__.shell = pkgs.zsh; + users.users.__USERNAME__.home = "/Users/__USERNAME__"; + + # Required by nix-darwin — set once per host at onboarding time and never + # changed afterward (see the assertion message this satisfies). + system.stateVersion = 7; + + # MAC2NIX:GENERATE:BEGIN -- generated by `mac2nix generate`; do not edit by hand + # MAC2NIX:GENERATE:END +} diff --git a/src/mac2nix/templates/scaffold/lib/helpers.nix b/src/mac2nix/templates/scaffold/lib/helpers.nix new file mode 100644 index 0000000..ae68350 --- /dev/null +++ b/src/mac2nix/templates/scaffold/lib/helpers.nix @@ -0,0 +1,50 @@ +{ inputs }: + +{ hostname, system, users }: + +let + inherit (inputs) darwin home-manager nix-homebrew mac-app-util determinate sops-nix homebrew-core homebrew-cask; + primaryUser = builtins.elemAt users 0; +in +darwin.lib.darwinSystem { + specialArgs = { inherit inputs hostname; }; + modules = [ + sops-nix.darwinModules.sops + nix-homebrew.darwinModules.nix-homebrew + mac-app-util.darwinModules.default + determinate.darwinModules.default + home-manager.darwinModules.home-manager + + { + nixpkgs.hostPlatform = system; + # Required by nix-darwin whenever an option that used to apply to the + # invoking user (e.g. homebrew.enable) is set, now that system + # activation always runs as root — per host, from its own user list. + system.primaryUser = primaryUser; + + sops.defaultSopsFile = ../secrets + "/${hostname}.yaml"; + sops.age.keyFile = "/Users/${primaryUser}/.config/sops/age/keys.txt"; + + nix-homebrew = { + enable = true; + user = primaryUser; + taps = { + "homebrew/homebrew-core" = homebrew-core; + "homebrew/homebrew-cask" = homebrew-cask; + }; + mutableTaps = false; + }; + + home-manager.useGlobalPkgs = true; + home-manager.useUserPackages = true; + home-manager.extraSpecialArgs = { inherit inputs hostname; }; + home-manager.sharedModules = [ ../modules/home-manager/default.nix ]; + home-manager.users = builtins.listToAttrs ( + map (u: { name = u; value = import (../users + "/${u}.nix"); }) users + ); + } + + ../modules/darwin/default.nix + (../hosts/darwin + "/${hostname}/configuration.nix") + ]; +} diff --git a/src/mac2nix/templates/scaffold/modules/darwin/default.nix b/src/mac2nix/templates/scaffold/modules/darwin/default.nix new file mode 100644 index 0000000..37d8e2d --- /dev/null +++ b/src/mac2nix/templates/scaffold/modules/darwin/default.nix @@ -0,0 +1,7 @@ +{ ... }: + +{ + imports = [ + ./homebrew.nix + ]; +} diff --git a/src/mac2nix/templates/scaffold/modules/darwin/homebrew.nix b/src/mac2nix/templates/scaffold/modules/darwin/homebrew.nix new file mode 100644 index 0000000..0c96811 --- /dev/null +++ b/src/mac2nix/templates/scaffold/modules/darwin/homebrew.nix @@ -0,0 +1,18 @@ +{ ... }: + +{ + # Shared Homebrew activation policy — not scanned cask/brew data, which is + # per-host and lives under hosts/darwin// once `mac2nix generate` + # has run. + homebrew = { + enable = true; + onActivation = { + autoUpdate = true; + upgrade = true; + cleanup = "zap"; + }; + caskArgs = { + require_sha = true; + }; + }; +} diff --git a/src/mac2nix/templates/scaffold/modules/home-manager/default.nix b/src/mac2nix/templates/scaffold/modules/home-manager/default.nix new file mode 100644 index 0000000..fca5958 --- /dev/null +++ b/src/mac2nix/templates/scaffold/modules/home-manager/default.nix @@ -0,0 +1,6 @@ +{ ... }: + +{ + imports = [ + ]; +} diff --git a/src/mac2nix/templates/scaffold/users/user.nix b/src/mac2nix/templates/scaffold/users/user.nix new file mode 100644 index 0000000..c8e451f --- /dev/null +++ b/src/mac2nix/templates/scaffold/users/user.nix @@ -0,0 +1,13 @@ +{ pkgs, lib, hostname, ... }: + +{ + imports = lib.optional (builtins.pathExists (../hosts/darwin + "/${hostname}/packages.nix")) ( + ../hosts/darwin + "/${hostname}/packages.nix" + ); + + home.username = "__USERNAME__"; + home.homeDirectory = "/Users/__USERNAME__"; + home.stateVersion = "26.05"; + + programs.home-manager.enable = true; +} diff --git a/src/mac2nix/vm/manager.py b/src/mac2nix/vm/manager.py index ba160d3..6b4b1cf 100644 --- a/src/mac2nix/vm/manager.py +++ b/src/mac2nix/vm/manager.py @@ -4,6 +4,7 @@ import asyncio import contextlib +import json import logging import shutil @@ -27,6 +28,54 @@ "no route to host", ) +# Pinned Tart base image used by this project's real VM integration tests. +# Kept here, alongside TartVMManager, so scripts/prewarm_vm.py and +# tests/vm_fixtures.py share one source of truth instead of re-deriving the +# digest (mirrors the Makefile's own test-integration target). +BASE_IMAGE_NAME = "macos-tahoe-base" +BASE_IMAGE_REF = ( + "ghcr.io/cirruslabs/macos-tahoe-base@sha256:a8e1c8305758643f513fdccdd829c2243687c60791083dea42f73f0b7aeb435c" +) + + +async def _local_vm_names() -> set[str]: + """Return the exact set of locally-known Tart VM/image names. + + Uses ``tart list --format json`` rather than the plain-text table — + verified empirically that a plain substring check against the table + (mirroring the Makefile's own ``grep -q``) false-positives on + ``BASE_IMAGE_NAME`` ("macos-tahoe-base"), since that string is itself a + substring of the full ``registry/repo@digest`` name an OCI pull is + actually cached under. + """ + returncode, stdout, _stderr = await async_run_command(["tart", "list", "--format", "json"]) + if returncode != 0: + return set() + entries = json.loads(stdout) + return {entry["Name"] for entry in entries} + + +async def pull_base_image_if_missing(name: str = BASE_IMAGE_NAME, image_ref: str = BASE_IMAGE_REF) -> None: + """Ensure a locally-named *name* VM exists, pulling *image_ref* under that name if missing. + + Uses ``tart clone `` rather than ``tart pull + `` — verified empirically that ``tart pull`` caches the OCI + blob under its own full ``registry/repo@digest`` string, not a short + alias, so a bare ``tart clone ...`` against that short name fails + with "the specified VM does not exist" even right after a successful + pull. ``tart clone`` accepts a remote reference as its source and pulls + it on a cache miss, so this both fetches and aliases it in one step. + + Raises :exc:`VMError` if the clone/pull fails. + """ + if name in await _local_vm_names(): + return + + logger.debug("Pulling base image %r as %r", image_ref, name) + returncode, _stdout, stderr = await async_run_command(["tart", "clone", image_ref, name], timeout=600) + if returncode != 0: + raise VMError(f"tart clone {image_ref!r} -> {name!r} failed (exit {returncode}): {stderr.strip()}") + class TartVMManager: """Async lifecycle manager for a single Tart VM clone. diff --git a/src/mac2nix/vm/validator.py b/src/mac2nix/vm/validator.py index 4e40fca..7fbc030 100644 --- a/src/mac2nix/vm/validator.py +++ b/src/mac2nix/vm/validator.py @@ -15,6 +15,14 @@ logger = logging.getLogger(__name__) +# Sourcing the Nix profile script is required before any non-interactive SSH +# command can see Nix binaries — a bare command has no guarantee that a login +# shell (and its profile.d sourcing) ran first. +_NIX_PROFILE_SOURCE = ". /nix/var/nix/profiles/default/etc/profile.d/nix-daemon.sh" + +# Shared with scripts/prewarm_vm.py so the two Nix-install invocations can't drift apart. +NIX_INSTALLER_URL = "https://install.determinate.systems/nix" + # --------------------------------------------------------------------------- # Models (co-located per architect decision) # --------------------------------------------------------------------------- @@ -179,9 +187,20 @@ class Validator: # Remote paths inside the VM. _REMOTE_FLAKE_DIR = "/tmp/mac2nix-flake" # noqa: S108 _REMOTE_SCAN_PATH = "/tmp/mac2nix-state.json" # noqa: S108 + _REMOTE_SOURCE_DIR = "/tmp/mac2nix-source" # noqa: S108 + + # Default preserves today's `mac2nix validate` CLI behavior exactly — only + # this plan's own nix_vm tests pass a local checkout path instead. + _DEFAULT_MAC2NIX_SOURCE = "github:gordon-code/mac2nix" - def __init__(self, vm: TartVMManager) -> None: + # 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"}) + + def __init__(self, vm: TartVMManager, mac2nix_source: str = _DEFAULT_MAC2NIX_SOURCE) -> None: self._vm = vm + self._mac2nix_source = mac2nix_source async def validate(self, flake_path: Path, source_state: SystemState) -> ValidationResult: """Run the full validation pipeline. @@ -224,20 +243,37 @@ async def validate(self, flake_path: Path, source_state: SystemState) -> Validat errors=[], ) - async def _copy_flake_to_vm(self, flake_path: Path) -> None: - """SCP the flake directory into the VM at :attr:`_REMOTE_FLAKE_DIR`. + async def _copy_flake_to_vm( + self, + local_path: Path, + remote_dir: str | None = None, + exclude: frozenset[str] = frozenset(), + what: str = "flake", + ) -> None: + """SCP *local_path* into the VM at *remote_dir* (default :attr:`_REMOTE_FLAKE_DIR`). + + When *exclude* is non-empty, only *local_path*'s top-level entries not + named in *exclude* are copied — used by :meth:`_scan_vm`'s local-source + override to scope the copy to package content, never dev-machine-only + directories. Uses sshpass + scp with argument lists (no shell=True). Raises :exc:`VMError` if scp fails or the VM has no IP. """ + remote_dir = remote_dir or self._REMOTE_FLAKE_DIR ip = await self._vm.get_ip() if not ip: - raise VMError("Cannot copy flake — VM has no IP address") + raise VMError(f"Cannot copy {what} — VM has no IP address") # Ensure remote destination exists. - ok, _out, err = await self._vm.exec_command(["mkdir", "-p", self._REMOTE_FLAKE_DIR]) + ok, _out, err = await self._vm.exec_command(["mkdir", "-p", remote_dir]) if not ok: - raise VMError(f"mkdir {self._REMOTE_FLAKE_DIR!r} failed: {err.strip()}") + raise VMError(f"mkdir {remote_dir!r} failed: {err.strip()}") + + if exclude: + sources = [str(p) for p in sorted(local_path.iterdir()) if p.name not in exclude] + else: + sources = [str(local_path) + "/."] # scp -r user@ip: — uses sshpass -e for password auth. # Password passed via SSHPASS env var to avoid exposure in ps aux. @@ -252,42 +288,51 @@ async def _copy_flake_to_vm(self, flake_path: Path) -> None: "-o", "LogLevel=ERROR", "-r", - str(flake_path) + "/.", - f"{self._vm.vm_user}@{ip}:{self._REMOTE_FLAKE_DIR}", + *sources, + f"{self._vm.vm_user}@{ip}:{remote_dir}", ] returncode, _stdout, stderr = await async_run_command( scp_cmd, timeout=120, env={"SSHPASS": self._vm.vm_password} ) if returncode != 0: - raise VMError(f"scp flake to VM failed (exit {returncode}): {stderr.strip()}") + raise VMError(f"scp {what} to VM failed (exit {returncode}): {stderr.strip()}") - logger.debug("Flake copied to VM at %s", self._REMOTE_FLAKE_DIR) + logger.debug("%s copied to VM at %s", what, remote_dir) async def _bootstrap_nix_darwin(self) -> None: - """Install Nix and nix-darwin inside the VM. + """Install Nix inside the VM, skipping the install if Nix is already present. + + The idempotency check sources the Nix profile script first, matching + :meth:`_rebuild_switch`/:meth:`_scan_vm`'s own pattern — a bare + ``which nix`` over a non-interactive SSH session has no guarantee that + profile.d was sourced, and would report "not found" even against a + pre-warmed, Nix-already-installed image (see ``scripts/prewarm_vm.py``). Raises :exc:`VMError` if any bootstrap step fails. """ + check_cmd = f"{_NIX_PROFILE_SOURCE} && which nix" + already_installed, _out, _err = await self._vm.exec_command(["bash", "-c", check_cmd]) + if already_installed: + logger.debug("Nix already present in VM — skipping install") + return + logger.debug("Bootstrapping Nix in VM") installer_path = "/tmp/nix-installer.sh" # noqa: S108 # Step 1: Download the Determinate Systems nix installer to a file. - ok, _out, err = await self._vm.exec_command( - [ - "curl", - "--proto", - "=https", - "--tlsv1.2", - "-sSf", - "-L", - "https://install.determinate.systems/nix", - "-o", - installer_path, - ], - timeout=60, - ) + # Two things collide here, verified empirically against a real VM: + # (1) curl on this VM's own build rejects the combined "--proto=https" + # form outright ("option --proto=https: is unknown"); (2) the + # separate-argv form ["--proto", "=https"] hits the VM's remote login + # shell (zsh), which treats a bare leading-"=" word as its own + # command-path-expansion syntax and fails with "https not found" + # before curl ever runs. Wrapping in `bash -c` with `=https` + # single-quoted sidesteps both: bash has no equals-expansion, and + # curl accepts the quoted, space-separated two-word form. + download_cmd = f"curl --proto '=https' --tlsv1.2 -sSf -L {NIX_INSTALLER_URL} -o {installer_path}" + ok, _out, err = await self._vm.exec_command(["bash", "-c", download_cmd], timeout=60) if not ok: raise VMError(f"Failed to download Nix installer: {err.strip()}") @@ -321,11 +366,7 @@ async def _rebuild_switch(self) -> str: Raises :exc:`VMError` if the rebuild fails. """ logger.debug("Running nix-darwin switch") - cmd = ( - f"cd {self._REMOTE_FLAKE_DIR}" - " && . /nix/var/nix/profiles/default/etc/profile.d/nix-daemon.sh" - " && nix run nix-darwin -- switch --flake ." - ) + cmd = f"cd {self._REMOTE_FLAKE_DIR} && {_NIX_PROFILE_SOURCE} && nix run nix-darwin -- switch --flake ." ok, out, err = await self._vm.exec_command(["bash", "-c", cmd], timeout=600) combined = (out + "\n" + err).strip() if not ok: @@ -337,15 +378,30 @@ async def _rebuild_switch(self) -> str: async def _scan_vm(self) -> SystemState: """Run mac2nix inside the VM via nix run, SCP the result back, parse it. + When :attr:`_mac2nix_source` is the published default, runs the + GitHub-hosted flake directly — this is the existing, unchanged + behavior of the `mac2nix validate` CLI. Otherwise, `_mac2nix_source` + is treated as a local checkout path: it's SCPed into the VM (scoped + via :attr:`_LOCAL_SOURCE_EXCLUDE`) and run from there instead — used + by this plan's own nix_vm tests, whose generator code isn't published + to GitHub yet. + Raises :exc:`VMError` if any step fails or the result cannot be parsed. """ logger.debug("Running mac2nix scan in VM via nix run") - # Run mac2nix directly from GitHub using nix run — no pip needed. - nix_run_cmd = ( - ". /nix/var/nix/profiles/default/etc/profile.d/nix-daemon.sh" - f" && nix run github:gordon-code/mac2nix -- scan -o {self._REMOTE_SCAN_PATH}" - ) + if self._mac2nix_source == self._DEFAULT_MAC2NIX_SOURCE: + run_target = self._mac2nix_source + else: + await self._copy_flake_to_vm( + Path(self._mac2nix_source), + remote_dir=self._REMOTE_SOURCE_DIR, + exclude=self._LOCAL_SOURCE_EXCLUDE, + what="mac2nix source", + ) + run_target = self._REMOTE_SOURCE_DIR + + nix_run_cmd = f"{_NIX_PROFILE_SOURCE} && nix run {run_target} -- scan -o {self._REMOTE_SCAN_PATH}" ok, _out, err = await self._vm.exec_command(["bash", "-c", nix_run_cmd], timeout=300) if not ok: raise VMError(f"mac2nix scan failed: {err.strip()}") diff --git a/tests/_scaffold_helpers.py b/tests/_scaffold_helpers.py new file mode 100644 index 0000000..cabcf62 --- /dev/null +++ b/tests/_scaffold_helpers.py @@ -0,0 +1,32 @@ +"""Shared test-support helpers for add_host()'s real age-keygen/sops integration tests. + +Not a test module itself (pytest's python_files pattern doesn't match this +name) — imported by tests/cli/test_add_host.py and tests/generators/test_scaffold.py. +""" + +from __future__ import annotations + +import contextlib +import shutil +from collections.abc import Iterator +from pathlib import Path +from unittest.mock import patch + + +def _has_add_host_crypto_deps() -> bool: + return shutil.which("age-keygen") is not None and shutil.which("sops") is not None + + +@contextlib.contextmanager +def _redirect_age_keys(key_root: Path) -> Iterator[None]: + """Redirect generate_age_key()'s real key file location from the real + `/Users//...` tree to a tmp-based directory, so real + age-keygen/sops can run safely in tests without touching the actual + filesystem's /Users tree. + """ + + def fake_age_key_path(username: str, key_dir: Path | None = None) -> Path: + return (key_dir or key_root / username) / "keys.txt" + + with patch("mac2nix.generators.scaffold._age_key_path", side_effect=fake_age_key_path): + yield diff --git a/tests/cli/__init__.py b/tests/cli/__init__.py new file mode 100644 index 0000000..e69de29 diff --git a/tests/cli/test_add_host.py b/tests/cli/test_add_host.py new file mode 100644 index 0000000..3b776d4 --- /dev/null +++ b/tests/cli/test_add_host.py @@ -0,0 +1,136 @@ +"""Tests for the mac2nix add-host CLI command.""" + +from __future__ import annotations + +from pathlib import Path +from unittest.mock import patch + +import pytest +from click.testing import CliRunner + +from mac2nix.cli import main +from mac2nix.generators.scaffold import init_framework +from tests._scaffold_helpers import _has_add_host_crypto_deps, _redirect_age_keys + +require_add_host_crypto_deps = pytest.mark.skipif( + not _has_add_host_crypto_deps(), reason="age-keygen and/or sops not on PATH" +) + + +@require_add_host_crypto_deps +class TestAddHostCommand: + def test_registered(self) -> None: + runner = CliRunner() + result = runner.invoke(main, ["--help"]) + assert result.exit_code == 0 + assert "add-host" in result.output + + def test_succeeds_end_to_end_with_confirmed_input(self, tmp_path: Path) -> None: + output_dir = tmp_path / "repo" + init_framework(output_dir) + + runner = CliRunner() + with _redirect_age_keys(tmp_path / "age-keys"): + result = runner.invoke( + main, + ["add-host", str(output_dir), "--hostname", "myhost", "--username", "alice"], + input="CONFIRMED\n", + ) + + assert result.exit_code == 0, result.output + assert (output_dir / "hosts" / "darwin" / "myhost" / "configuration.nix").is_file() + assert (output_dir / "hosts" / "darwin" / "myhost" / ".mac2nix-meta.json").is_file() + assert (output_dir / "users" / "alice.nix").is_file() + assert (output_dir / "secrets" / "myhost.yaml").is_file() + assert "myhost" in (output_dir / "flake.nix").read_text() + assert "myhost" in (output_dir / ".sops.yaml").read_text() + + def test_declining_confirmation_aborts_with_no_files_left(self, tmp_path: Path) -> None: + output_dir = tmp_path / "repo" + init_framework(output_dir) + + runner = CliRunner() + with _redirect_age_keys(tmp_path / "age-keys"): + result = runner.invoke( + main, + ["add-host", str(output_dir), "--hostname", "myhost", "--username", "alice"], + input="nope\n", + ) + + assert result.exit_code != 0 + assert not (output_dir / "hosts" / "darwin" / "myhost").exists() + assert not (output_dir / "users" / "alice.nix").exists() + assert not (output_dir / "secrets" / "myhost.yaml").exists() + assert "myhost" not in (output_dir / "flake.nix").read_text() + + def test_rerun_against_same_hostname_fails_cleanly(self, tmp_path: Path) -> None: + output_dir = tmp_path / "repo" + init_framework(output_dir) + runner = CliRunner() + + with _redirect_age_keys(tmp_path / "age-keys"): + first = runner.invoke( + main, + ["add-host", str(output_dir), "--hostname", "myhost", "--username", "alice"], + input="CONFIRMED\n", + ) + assert first.exit_code == 0, first.output + + second = runner.invoke( + main, + ["add-host", str(output_dir), "--hostname", "myhost", "--username", "alice"], + input="CONFIRMED\n", + ) + + assert second.exit_code != 0 + assert "already registered" in second.output + + def test_non_init_directory_fails_cleanly(self, tmp_path: Path) -> None: + output_dir = tmp_path / "not-a-framework" + output_dir.mkdir() + + runner = CliRunner() + result = runner.invoke( + main, + ["add-host", str(output_dir), "--hostname", "myhost", "--username", "alice"], + input="CONFIRMED\n", + ) + + assert result.exit_code != 0 + assert "mac2nix" in result.output.lower() or "framework" in result.output.lower() + + +class TestAddHostValidation: + """--hostname/--username character-allowlist rejection — no crypto tools required.""" + + def test_invalid_hostname_rejected_before_any_crypto_call(self, tmp_path: Path) -> None: + output_dir = tmp_path / "repo" + init_framework(output_dir) + + runner = CliRunner() + with patch("mac2nix.generators.scaffold.subprocess.run") as mock_run: + result = runner.invoke( + main, + ["add-host", str(output_dir), "--hostname", "../evil", "--username", "alice"], + ) + + assert result.exit_code != 0 + assert "hostname" in result.output.lower() + mock_run.assert_not_called() + assert not (output_dir / "hosts").exists() + + def test_invalid_username_rejected_before_any_crypto_call(self, tmp_path: Path) -> None: + output_dir = tmp_path / "repo" + init_framework(output_dir) + + runner = CliRunner() + with patch("mac2nix.generators.scaffold.subprocess.run") as mock_run: + result = runner.invoke( + main, + ["add-host", str(output_dir), "--hostname", "myhost", "--username", 'foo"bar'], + ) + + assert result.exit_code != 0 + assert "username" in result.output.lower() + mock_run.assert_not_called() + assert not (output_dir / "hosts").exists() diff --git a/tests/cli/test_init.py b/tests/cli/test_init.py new file mode 100644 index 0000000..97b7f01 --- /dev/null +++ b/tests/cli/test_init.py @@ -0,0 +1,64 @@ +"""Tests for the mac2nix init CLI command.""" + +from __future__ import annotations + +from pathlib import Path + +from click.testing import CliRunner + +from mac2nix.cli import main + + +class TestInitCommand: + def test_registered(self) -> None: + runner = CliRunner() + result = runner.invoke(main, ["--help"]) + assert result.exit_code == 0 + assert "init" in result.output + + def test_succeeds_on_empty_directory(self, tmp_path: Path) -> None: + output_dir = tmp_path / "repo" + + runner = CliRunner() + result = runner.invoke(main, ["init", str(output_dir)]) + + assert result.exit_code == 0, result.output + assert (output_dir / "flake.nix").is_file() + assert (output_dir / ".gitignore").is_file() + assert (output_dir / "lib" / "helpers.nix").is_file() + + def test_succeeds_on_nonexistent_directory(self, tmp_path: Path) -> None: + """output_dir doesn't need to exist yet — init creates it.""" + output_dir = tmp_path / "does-not-exist-yet" + + runner = CliRunner() + result = runner.invoke(main, ["init", str(output_dir)]) + + assert result.exit_code == 0, result.output + assert output_dir.is_dir() + + def test_prints_next_step_guidance(self, tmp_path: Path) -> None: + output_dir = tmp_path / "repo" + + runner = CliRunner() + result = runner.invoke(main, ["init", str(output_dir)]) + + assert result.exit_code == 0 + assert "add-host" in result.output + + def test_rerun_against_populated_directory_fails_cleanly(self, tmp_path: Path) -> None: + output_dir = tmp_path / "repo" + runner = CliRunner() + + first = runner.invoke(main, ["init", str(output_dir)]) + assert first.exit_code == 0, first.output + + before = sorted(p.relative_to(output_dir) for p in output_dir.rglob("*")) + + second = runner.invoke(main, ["init", str(output_dir)]) + + assert second.exit_code != 0 + assert "not empty" in second.output + + after = sorted(p.relative_to(output_dir) for p in output_dir.rglob("*")) + assert before == after, "a failed re-run must not modify the existing directory" diff --git a/tests/generators/__init__.py b/tests/generators/__init__.py new file mode 100644 index 0000000..e69de29 diff --git a/tests/generators/test_nix_render.py b/tests/generators/test_nix_render.py new file mode 100644 index 0000000..499ba47 --- /dev/null +++ b/tests/generators/test_nix_render.py @@ -0,0 +1,128 @@ +"""Tests for mac2nix.generators._nix_render: Python-to-Nix literal rendering and Jinja2 plumbing.""" + +from __future__ import annotations + +import shutil +import subprocess +from pathlib import Path +from typing import Any + +import jinja2 +import pytest + +from mac2nix.generators._nix_render import ( + nix_mkdefault, + nix_string, + python_to_nix, + render_template, + setup_jinja_env, +) + +_BACKSLASH = chr(92) +_DQUOTE = chr(34) + + +@pytest.mark.parametrize( + ("value", "expected"), + [ + (True, "true"), + (False, "false"), + (42, "42"), + (3.14, "3.14"), + (None, "null"), + ("hello", '"hello"'), + (["a", 1, True], '[ "a" 1 true ]'), + ({"key": "value"}, '{ key = "value"; }'), + ({"outer": {"inner": 1}}, "{ outer = { inner = 1; }; }"), + ({"has space": 1}, '{ "has space" = 1; }'), + ], + ids=[ + "bool_true", + "bool_false", + "int", + "float", + "none", + "str", + "list", + "dict_bare_key", + "nested_dict", + "dict_key_needs_quoting", + ], +) +def test_python_to_nix(value: Any, expected: str) -> None: + assert python_to_nix(value) == expected + + +def test_python_to_nix_raises_typeerror_for_unsupported_type() -> None: + with pytest.raises(TypeError): + python_to_nix((1, 2, 3)) + + +def test_nix_string_escapes_backslash() -> None: + value = "a" + _BACKSLASH + "b" + assert nix_string(value) == _DQUOTE + "a" + _BACKSLASH + _BACKSLASH + "b" + _DQUOTE + + +def test_nix_string_escapes_double_quote() -> None: + value = "say " + _DQUOTE + "hi" + _DQUOTE + expected = _DQUOTE + "say " + _BACKSLASH + _DQUOTE + "hi" + _BACKSLASH + _DQUOTE + _DQUOTE + assert nix_string(value) == expected + + +def test_nix_string_escapes_dollar_brace_interpolation() -> None: + value = "${danger}" + assert nix_string(value) == _DQUOTE + _BACKSLASH + "${danger}" + _DQUOTE + + +def test_nix_mkdefault_wraps_expression() -> None: + assert nix_mkdefault("true") == "lib.mkDefault true" + + +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) + template = env.get_template("fixture.nix.j2") + + assert template.render(x=True, y=5) == "{ y = 5; }" + assert template.render(x=False, y=5) == "" + + +def test_render_template_delegates_to_environment() -> None: + loader = jinja2.DictLoader({"fixture.nix.j2": "<< value|nix_str >>"}) + result = render_template("fixture.nix.j2", {"value": "hi"}, loader=loader) + assert result == '"hi"' + + +@pytest.fixture +def require_nix_instantiate() -> None: + if shutil.which("nix-instantiate") is None: + pytest.skip("nix-instantiate not on PATH") + + +@pytest.mark.nix +@pytest.mark.parametrize( + "value", + [ + pytest.param({"a": {"b": 1, "c": "x"}}, id="nested_dict"), + pytest.param(["a", "b", "c"], id="list_of_strings"), + pytest.param('has "quotes" and $dollar', id="string_with_quote_and_dollar"), + ], +) +def test_nix_instantiate_parses_rendered_value( + require_nix_instantiate: None, + value: Any, + tmp_path: Path, +) -> None: + rendered = python_to_nix(value) + module_source = f"{{ config, lib, pkgs, ... }}: {{ test = {rendered}; }}" + 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 diff --git a/tests/generators/test_scaffold.py b/tests/generators/test_scaffold.py new file mode 100644 index 0000000..c662297 --- /dev/null +++ b/tests/generators/test_scaffold.py @@ -0,0 +1,394 @@ +"""Tests for mac2nix.generators.scaffold: init_framework(), add_host(), and ScaffoldError.""" + +from __future__ import annotations + +import contextlib +import json +import os +import re +import stat +import subprocess +from pathlib import Path +from unittest.mock import patch + +import pytest +import yaml + +from mac2nix.generators import Mac2NixError +from mac2nix.generators.scaffold import ScaffoldError, add_host, generate_age_key, init_framework +from tests._scaffold_helpers import _has_add_host_crypto_deps, _redirect_age_keys + +_EXPECTED_FRAMEWORK_FILES = [ + ".gitignore", + "flake.nix", + "lib/helpers.nix", + "modules/darwin/default.nix", + "modules/darwin/homebrew.nix", + "modules/home-manager/default.nix", +] + + +def test_scaffold_error_is_mac2nix_error() -> None: + assert issubclass(ScaffoldError, Mac2NixError) + + +class TestInitFramework: + def test_produces_expected_framework_files(self, tmp_path: Path) -> None: + output_dir = tmp_path / "repo" + init_framework(output_dir) + + for rel_path in _EXPECTED_FRAMEWORK_FILES: + assert (output_dir / rel_path).is_file(), f"missing {rel_path}" + + def test_gitignore_renamed_bare_gitignore_absent(self, tmp_path: Path) -> None: + output_dir = tmp_path / "repo" + init_framework(output_dir) + + assert (output_dir / ".gitignore").is_file() + assert not (output_dir / "gitignore").exists() + + def test_flake_nix_has_empty_host_sentinels(self, tmp_path: Path) -> None: + output_dir = tmp_path / "repo" + init_framework(output_dir) + + lines = (output_dir / "flake.nix").read_text().splitlines() + begin_line = next(i for i, line in enumerate(lines) if "MAC2NIX:HOSTS:BEGIN" in line) + end_line = next(i for i, line in enumerate(lines) if "MAC2NIX:HOSTS:END" in line) + + # The BEGIN marker's own line carries trailing explanatory comment text + # ("-- generated by ... do not edit by hand") — only lines strictly + # between the two sentinel lines are the actual host-block content. + between_lines = lines[begin_line + 1 : end_line] + assert all(line.strip() == "" for line in between_lines), "sentinel block must be empty right after init" + + def test_produces_no_host_or_secret_state(self, tmp_path: Path) -> None: + """init_framework() is host-less — hosts/, users/, .sops.yaml, and secrets/ + are add_host()'s responsibility, not init's.""" + output_dir = tmp_path / "repo" + init_framework(output_dir) + + assert not (output_dir / "hosts").exists() + assert not (output_dir / "users").exists() + assert not (output_dir / ".sops.yaml").exists() + assert not (output_dir / "secrets").exists() + + def test_creates_missing_parent_directories(self, tmp_path: Path) -> None: + output_dir = tmp_path / "nested" / "repo" + init_framework(output_dir) + assert (output_dir / "flake.nix").is_file() + + def test_raises_on_nonempty_directory(self, tmp_path: Path) -> None: + output_dir = tmp_path / "repo" + output_dir.mkdir() + (output_dir / "unrelated.txt").write_text("pre-existing content") + + with pytest.raises(ScaffoldError, match="not empty"): + init_framework(output_dir) + + def test_raises_on_nonempty_directory_writes_nothing(self, tmp_path: Path) -> None: + output_dir = tmp_path / "repo" + output_dir.mkdir() + (output_dir / "unrelated.txt").write_text("pre-existing content") + + before = sorted(p.relative_to(output_dir) for p in output_dir.rglob("*")) + + with pytest.raises(ScaffoldError): + init_framework(output_dir) + + after = sorted(p.relative_to(output_dir) for p in output_dir.rglob("*")) + assert before == after + + +# --------------------------------------------------------------------------- +# generate_age_key() +# --------------------------------------------------------------------------- + + +class TestGenerateAgeKey: + def test_permissions_and_fingerprint(self, tmp_path: Path) -> None: + key_dir = tmp_path / "keys" + fingerprint = generate_age_key("testuser", key_dir=key_dir) + + key_path = key_dir / "keys.txt" + assert key_path.is_file() + assert stat.S_IMODE(key_path.stat().st_mode) == 0o600 + + public_key_line = next(line for line in key_path.read_text().splitlines() if line.startswith("# public key:")) + assert public_key_line.removeprefix("# public key:").strip() == fingerprint + + def test_second_call_raises_without_modifying_existing_key(self, tmp_path: Path) -> None: + key_dir = tmp_path / "keys" + generate_age_key("testuser", key_dir=key_dir) + + key_path = key_dir / "keys.txt" + before_mtime = key_path.stat().st_mtime_ns + before_content = key_path.read_text() + + with pytest.raises(ScaffoldError, match="already exists"): + generate_age_key("testuser", key_dir=key_dir) + + assert key_path.stat().st_mtime_ns == before_mtime + assert key_path.read_text() == before_content + + +# --------------------------------------------------------------------------- +# add_host() — file/directory orchestration and rollback (crypto mocked) +# --------------------------------------------------------------------------- + + +@contextlib.contextmanager +def _mocked_crypto(*fingerprints: str): + """Isolate add_host()'s file/directory orchestration from real age-keygen/sops. + + generate_age_key() is patched to return *fingerprints* in call order + (each add_host() call inside the `with` block consumes the next one); + _create_host_secrets_file() (the only sops-shelling step) is a no-op. + Real sops/age integration is TestAddHostSops's job, not this one's. + """ + with ( + patch("mac2nix.generators.scaffold.generate_age_key", side_effect=list(fingerprints)), + patch("mac2nix.generators.scaffold._create_host_secrets_file"), + ): + yield + + +class TestAddHostFiles: + def test_first_host_creates_files_with_placeholders_substituted(self, tmp_path: Path) -> None: + output_dir = tmp_path / "repo" + init_framework(output_dir) + + with _mocked_crypto("age1hosta-fingerprint"): + fingerprint = add_host(output_dir, "hosta", "alice", confirm_backup=lambda _: True) + + assert fingerprint == "age1hosta-fingerprint" + + config = (output_dir / "hosts" / "darwin" / "hosta" / "configuration.nix").read_text() + assert "__HOSTNAME__" not in config + assert "__USERNAME__" not in config + assert "hosta" in config + assert "alice" in config + + meta = json.loads((output_dir / "hosts" / "darwin" / "hosta" / ".mac2nix-meta.json").read_text()) + assert meta == { + "hostname": "hosta", + "username": "alice", + "system": "aarch64-darwin", + "age_public_key": "age1hosta-fingerprint", + } + + user_config = (output_dir / "users" / "alice.nix").read_text() + assert "__USERNAME__" not in user_config + assert "alice" in user_config + + def test_second_host_same_username_leaves_user_file_untouched(self, tmp_path: Path) -> None: + output_dir = tmp_path / "repo" + init_framework(output_dir) + + with _mocked_crypto("age1hosta-fingerprint", "age1hostb-fingerprint"): + add_host(output_dir, "hosta", "shareduser", confirm_backup=lambda _: True) + + user_file = output_dir / "users" / "shareduser.nix" + before_mtime = user_file.stat().st_mtime_ns + before_content = user_file.read_text() + + add_host(output_dir, "hostb", "shareduser", confirm_backup=lambda _: True) + + assert user_file.stat().st_mtime_ns == before_mtime + assert user_file.read_text() == before_content + assert (output_dir / "hosts" / "darwin" / "hosta").is_dir() + assert (output_dir / "hosts" / "darwin" / "hostb").is_dir() + + def test_duplicate_hostname_raises_and_writes_nothing(self, tmp_path: Path) -> None: + output_dir = tmp_path / "repo" + init_framework(output_dir) + + with _mocked_crypto("age1hosta-fingerprint"): + add_host(output_dir, "hosta", "alice", confirm_backup=lambda _: True) + + before = sorted(p.relative_to(output_dir) for p in output_dir.rglob("*")) + + with ( + _mocked_crypto("age1hosta-fingerprint-again"), + pytest.raises(ScaffoldError, match="already registered"), + ): + add_host(output_dir, "hosta", "alice", confirm_backup=lambda _: True) + + after = sorted(p.relative_to(output_dir) for p in output_dir.rglob("*")) + assert before == after + + def test_non_framework_directory_missing_flake_raises(self, tmp_path: Path) -> None: + output_dir = tmp_path / "not-a-framework" + output_dir.mkdir() + + with pytest.raises(ScaffoldError, match="not a mac2nix-scaffolded framework"): + add_host(output_dir, "hosta", "alice", confirm_backup=lambda _: True) + + def test_non_framework_directory_missing_sentinel_raises(self, tmp_path: Path) -> None: + output_dir = tmp_path / "not-a-framework" + output_dir.mkdir() + (output_dir / "flake.nix").write_text("{ }") # a flake.nix, but not mac2nix's + + with pytest.raises(ScaffoldError, match="not a mac2nix-scaffolded framework"): + add_host(output_dir, "hosta", "alice", confirm_backup=lambda _: True) + + def test_flake_nix_missing_end_sentinel_raises_clean_error(self, tmp_path: Path) -> None: + """BEGIN present but END damaged/removed must raise ScaffoldError, not a raw ValueError.""" + output_dir = tmp_path / "repo" + init_framework(output_dir) + + flake_path = output_dir / "flake.nix" + flake_path.write_text(flake_path.read_text().replace("# MAC2NIX:HOSTS:END", "")) + + with pytest.raises(ScaffoldError, match="not a mac2nix-scaffolded framework"): + add_host(output_dir, "hosta", "alice", confirm_backup=lambda _: True) + + def test_confirm_backup_false_leaves_no_host_artifacts(self, tmp_path: Path) -> None: + output_dir = tmp_path / "repo" + init_framework(output_dir) + + with _mocked_crypto("age1hosta-fingerprint"), pytest.raises(ScaffoldError, match="not confirmed"): + add_host(output_dir, "hosta", "alice", confirm_backup=lambda _: False) + + assert not (output_dir / "hosts" / "darwin" / "hosta").exists() + assert not (output_dir / "users" / "alice.nix").exists() + assert not (output_dir / "secrets" / "hosta.yaml").exists() + + flake_content = (output_dir / "flake.nix").read_text() + assert "hosta" not in flake_content + # The cleanup handler's self-heal always regenerates .sops.yaml (even to an + # empty creation_rules list) — its presence isn't the point, its content is. + if (output_dir / ".sops.yaml").is_file(): + assert "hosta" not in (output_dir / ".sops.yaml").read_text() + + def test_late_sops_failure_rolls_back_and_reverts_flake_and_sops(self, tmp_path: Path) -> None: + """A failure in the sops-shelling step (Step 6) must undo everything Step 4-6 already + wrote for the failing host — including the flake.nix/.sops.yaml regeneration that + already happened by that point — while leaving an earlier, successfully-registered + host completely untouched.""" + output_dir = tmp_path / "repo" + init_framework(output_dir) + + with ( + patch( + "mac2nix.generators.scaffold.generate_age_key", + side_effect=["age1hosta-fingerprint", "age1hostb-fingerprint"], + ), + patch( + "mac2nix.generators.scaffold._create_host_secrets_file", + side_effect=[None, ScaffoldError("sops failed for testing")], + ), + ): + add_host(output_dir, "hosta", "alice", confirm_backup=lambda _: True) + + with pytest.raises(ScaffoldError, match="sops failed for testing"): + add_host(output_dir, "hostb", "bob", confirm_backup=lambda _: True) + + assert not (output_dir / "hosts" / "darwin" / "hostb").exists() + assert not (output_dir / "users" / "bob.nix").exists() + assert not (output_dir / "secrets" / "hostb.yaml").exists() + + flake_content = (output_dir / "flake.nix").read_text() + assert "hostb" not in flake_content + assert "hosta" in flake_content + + sops_config = yaml.safe_load((output_dir / ".sops.yaml").read_text()) + rules = sops_config["creation_rules"] + assert len(rules) == 1 + assert rules[0]["key_groups"] == [{"age": ["age1hosta-fingerprint"]}] + + +# --------------------------------------------------------------------------- +# add_host() — real sops/age integration (nix-marked) +# --------------------------------------------------------------------------- + + +@pytest.fixture +def require_add_host_crypto_deps() -> None: + if not _has_add_host_crypto_deps(): + pytest.skip("age-keygen and/or sops not on PATH") + + +@pytest.mark.nix +@pytest.mark.usefixtures("require_add_host_crypto_deps") +class TestAddHostSops: + def test_host_a_produces_correctly_scoped_sops_and_secrets(self, tmp_path: Path) -> None: + output_dir = tmp_path / "repo" + init_framework(output_dir) + + with _redirect_age_keys(tmp_path / "age-keys"): + fingerprint_a = add_host(output_dir, "hosta", "alice", confirm_backup=lambda _: True) + + sops_config = yaml.safe_load((output_dir / ".sops.yaml").read_text()) + rules = sops_config["creation_rules"] + assert len(rules) == 1 + assert rules[0]["key_groups"] == [{"age": [fingerprint_a]}] + assert re.search(rules[0]["path_regex"], "secrets/hosta.yaml") + assert not re.search(rules[0]["path_regex"], "secrets/hostb.yaml") + + assert (output_dir / "secrets" / "hosta.yaml").is_file() + + def test_host_b_added_without_disturbing_host_a_and_cannot_decrypt_its_secrets(self, tmp_path: Path) -> None: + output_dir = tmp_path / "repo" + init_framework(output_dir) + key_root = tmp_path / "age-keys" + + with _redirect_age_keys(key_root): + add_host(output_dir, "hosta", "alice", confirm_backup=lambda _: True) + + meta_a_path = output_dir / "hosts" / "darwin" / "hosta" / ".mac2nix-meta.json" + secrets_a_path = output_dir / "secrets" / "hosta.yaml" + meta_a_before = meta_a_path.read_text() + secrets_a_before = secrets_a_path.read_text() + + add_host(output_dir, "hostb", "bob", confirm_backup=lambda _: True) + + # A's own files are untouched by B's registration. + assert meta_a_path.read_text() == meta_a_before + assert secrets_a_path.read_text() == secrets_a_before + + sops_config = yaml.safe_load((output_dir / ".sops.yaml").read_text()) + flake_content = (output_dir / "flake.nix").read_text() + assert len(sops_config["creation_rules"]) == 2 + assert "hosta" in flake_content + assert "hostb" in flake_content + + # B's key cannot decrypt A's secrets — a real sops --decrypt attempt. + b_key_path = key_root / "bob" / "keys.txt" + env = {**os.environ, "SOPS_AGE_KEY_FILE": str(b_key_path)} + result = subprocess.run( # noqa: S603 + ["sops", "--decrypt", str(secrets_a_path)], # noqa: S607 + cwd=output_dir, + env=env, + capture_output=True, + text=True, + check=False, + ) + assert result.returncode != 0 + + def test_duplicate_key_metadata_triggers_uniqueness_error_and_rolls_back(self, tmp_path: Path) -> None: + output_dir = tmp_path / "repo" + init_framework(output_dir) + key_root = tmp_path / "age-keys" + + with _redirect_age_keys(key_root): + fingerprint_a = add_host(output_dir, "hosta", "alice", confirm_backup=lambda _: True) + + with ( + patch("mac2nix.generators.scaffold.generate_age_key", return_value=fingerprint_a), + pytest.raises(ScaffoldError, match="share the same age public key"), + ): + add_host(output_dir, "hostc", "carol", confirm_backup=lambda _: True) + + # Rollback: hostc leaves no trace, flake.nix/.sops.yaml revert to hosta only. + assert not (output_dir / "hosts" / "darwin" / "hostc").exists() + assert not (output_dir / "users" / "carol.nix").exists() + assert not (output_dir / "secrets" / "hostc.yaml").exists() + + flake_content = (output_dir / "flake.nix").read_text() + assert "hostc" not in flake_content + assert "hosta" in flake_content + + sops_config = yaml.safe_load((output_dir / ".sops.yaml").read_text()) + rules = sops_config["creation_rules"] + assert len(rules) == 1 + assert rules[0]["key_groups"] == [{"age": [fingerprint_a]}] diff --git a/tests/generators/test_scaffold_integration.py b/tests/generators/test_scaffold_integration.py new file mode 100644 index 0000000..c47ad65 --- /dev/null +++ b/tests/generators/test_scaffold_integration.py @@ -0,0 +1,72 @@ +"""Real `nix flake check`/`nix build` integration test against a freshly-scaffolded, hosted framework. + +Marked `nix_build` — excluded from the default `pytest`/`make test` run, +invoked via `make test-nix`. Unlike the parse-only `nix` marker (Task 3), +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. A silently-skipped integration test would defeat the entire point +of this PR. + +Known, accepted tradeoff: there is no committed `flake.lock` for this +throwaway scaffold, so `nix flake lock` resolves `nixpkgs` (tracked as +`nixos-unstable`) to whatever commit is current at test-run time. A failure +here can be caused by an unrelated upstream nixpkgs regression, not a bug +in mac2nix's own templates — this is a deliberate scope boundary (pinning +the *shipped* scaffold's own `nixpkgs` input would carry that same +staleness into every real user's deployed flake), not an oversight. This +test requires live network access and is not fully hermetic. + +Real-environment note: `add_host()`'s age key path is derived from +*username* alone (`/Users/{username}/.config/sops/age/keys.txt`), with no +override at that level — by design, see `scaffold.py`. Rather than touching +that real path (which would either collide with a developer's own real sops +key, or fail outright on a machine that already has one configured — exactly +the audience most likely to have nix/age/sops all on PATH and be running +this never-skip test), this test redirects the key to `tmp_path` via the +same `_redirect_age_keys()` monkeypatch used by `test_scaffold.py`/ +`test_add_host.py`. It still exercises real `age-keygen`/`sops` end-to-end — +just at a location this test fully owns. +""" + +from __future__ import annotations + +import getpass +import subprocess +from pathlib import Path + +import pytest + +from mac2nix.generators.scaffold import add_host, init_framework +from tests._scaffold_helpers import _redirect_age_keys + +pytestmark = pytest.mark.nix_build + +_HOSTNAME = "mac2nix-nix-build-test" + + +def test_scaffold_builds_for_real(tmp_path: Path) -> None: + """init_framework() + add_host() must produce a flake that actually `nix build`s.""" + output_dir = tmp_path / "mac2nix-scaffold" + username = getpass.getuser() + + init_framework(output_dir) + with _redirect_age_keys(tmp_path / "age-keys"): + add_host(output_dir, _HOSTNAME, username, confirm_backup=lambda _fingerprint: True) + + lock_result = subprocess.run( + ["nix", "flake", "lock"], # 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"], # 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/test_cli_vm.py b/tests/test_cli_vm.py index 610a2ab..5f8b962 100644 --- a/tests/test_cli_vm.py +++ b/tests/test_cli_vm.py @@ -5,14 +5,14 @@ import inspect import json from pathlib import Path -from unittest.mock import patch +from unittest.mock import AsyncMock, MagicMock, patch from click.testing import CliRunner from mac2nix.cli import main from mac2nix.models.system_state import SystemState from mac2nix.vm.discovery import DiscoveryResult -from mac2nix.vm.validator import DomainScore, FidelityReport +from mac2nix.vm.validator import DomainScore, FidelityReport, ValidationResult # --------------------------------------------------------------------------- # Helpers @@ -156,6 +156,61 @@ def test_missing_scan_file_exits_nonzero(self, tmp_path: Path) -> None: assert result.exit_code != 0 +# --------------------------------------------------------------------------- +# validate command — --mac2nix-source option +# --------------------------------------------------------------------------- + + +class TestValidateMac2nixSourceOption: + def test_help_shows_option_with_default(self) -> None: + runner = CliRunner() + result = runner.invoke(main, ["validate", "--help"]) + assert result.exit_code == 0 + assert "--mac2nix-source" in result.output + assert "github:gordon-code/mac2nix" in result.output + + def _invoke_validate(self, tmp_path: Path, extra_args: list[str] | None = None): + scan_file = tmp_path / "state.json" + scan_file.write_text(_make_state().to_json()) + flake_dir = tmp_path / "flake" + flake_dir.mkdir() + + mock_vm = MagicMock() + mock_vm.__aenter__ = AsyncMock(return_value=mock_vm) + mock_vm.__aexit__ = AsyncMock(return_value=False) + mock_vm.clone = AsyncMock() + mock_vm.start = AsyncMock() + + mock_validator_instance = MagicMock() + mock_validator_instance.validate = AsyncMock( + return_value=ValidationResult(success=True, fidelity=None, build_output="", errors=[]) + ) + mock_validator_cls = MagicMock(return_value=mock_validator_instance) + + runner = CliRunner() + with ( + patch("mac2nix.vm.manager.TartVMManager.is_available", return_value=True), + patch("mac2nix.cli.TartVMManager", return_value=mock_vm), + patch("mac2nix.cli.Validator", mock_validator_cls), + ): + args = ["validate", "--flake-path", str(flake_dir), "--scan-file", str(scan_file), *(extra_args or [])] + result = runner.invoke(main, args) + + return result, mock_validator_cls + + def test_default_reaches_validator(self, tmp_path: Path) -> None: + result, mock_validator_cls = self._invoke_validate(tmp_path) + assert result.exit_code == 0, result.output + _, kwargs = mock_validator_cls.call_args + assert kwargs["mac2nix_source"] == "github:gordon-code/mac2nix" + + def test_override_reaches_validator(self, tmp_path: Path) -> None: + result, mock_validator_cls = self._invoke_validate(tmp_path, ["--mac2nix-source", "."]) + assert result.exit_code == 0, result.output + _, kwargs = mock_validator_cls.call_args + assert kwargs["mac2nix_source"] == "." + + # --------------------------------------------------------------------------- # discover command — registration and help # --------------------------------------------------------------------------- diff --git a/tests/vm/conftest.py b/tests/vm/conftest.py index bffe5c2..be74666 100644 --- a/tests/vm/conftest.py +++ b/tests/vm/conftest.py @@ -12,6 +12,7 @@ import pytest from mac2nix.vm.manager import TartVMManager +from tests.vm_fixtures import local_vm_names, nix_darwin_vm # noqa: F401 -- re-exported for tests/vm/*.py BASE_VM = os.environ.get("MAC2NIX_BASE_VM", "macos-tahoe-base") VM_USER = os.environ.get("MAC2NIX_VM_USER", "admin") @@ -22,17 +23,7 @@ def _can_run_integration() -> bool: """Check that tart, sshpass, and the base VM image are all available.""" if shutil.which("tart") is None or shutil.which("sshpass") is None: return False - try: - result = subprocess.run( - ["tart", "list"], # noqa: S607 - capture_output=True, - text=True, - timeout=10, - check=False, - ) - except (subprocess.TimeoutExpired, OSError): - return False - return result.returncode == 0 and BASE_VM in result.stdout + return BASE_VM in local_vm_names() skip_missing_deps = pytest.mark.skipif( diff --git a/tests/vm/test_manager.py b/tests/vm/test_manager.py index 293170e..709df5b 100644 --- a/tests/vm/test_manager.py +++ b/tests/vm/test_manager.py @@ -4,12 +4,13 @@ import asyncio import contextlib +import json from unittest.mock import AsyncMock, MagicMock, patch import pytest from mac2nix.vm._utils import VMConnectionError, VMError, VMTimeoutError -from mac2nix.vm.manager import TartVMManager +from mac2nix.vm.manager import BASE_IMAGE_NAME, BASE_IMAGE_REF, TartVMManager, pull_base_image_if_missing # --------------------------------------------------------------------------- # Helpers @@ -1010,3 +1011,77 @@ async def _run() -> None: # After exit, cleanup was called (stop + delete ran) asyncio.run(_run()) + + +# --------------------------------------------------------------------------- +# pull_base_image_if_missing() +# --------------------------------------------------------------------------- + + +class TestPullBaseImageIfMissing: + def test_skips_pull_when_name_present(self) -> None: + list_output = json.dumps([{"Name": BASE_IMAGE_NAME}, {"Name": "other-vm"}]) + captured: list[list[str]] = [] + + async def recording_run(cmd: list[str], **_kw) -> tuple[int, str, str]: + captured.append(cmd) + return (0, list_output, "") + + async def _run() -> None: + with patch("mac2nix.vm.manager.async_run_command", side_effect=recording_run): + await pull_base_image_if_missing(name=BASE_IMAGE_NAME, image_ref=BASE_IMAGE_REF) + + asyncio.run(_run()) + assert captured == [["tart", "list", "--format", "json"]] # no clone call made + + def test_pulls_when_name_absent(self) -> None: + list_output = json.dumps([{"Name": "other-vm"}]) + captured: list[list[str]] = [] + + async def recording_run(cmd: list[str], **_kw) -> tuple[int, str, str]: + captured.append(cmd) + if cmd[:2] == ["tart", "list"]: + return (0, list_output, "") + return (0, "", "") + + async def _run() -> None: + with patch("mac2nix.vm.manager.async_run_command", side_effect=recording_run): + await pull_base_image_if_missing(name=BASE_IMAGE_NAME, image_ref=BASE_IMAGE_REF) + + asyncio.run(_run()) + assert captured[-1] == ["tart", "clone", BASE_IMAGE_REF, BASE_IMAGE_NAME] + + def test_pulls_when_only_full_digest_variant_present(self) -> None: + """A locally-cached OCI pull is named by its full `registry/repo@digest` + string, which contains BASE_IMAGE_NAME as a substring without actually + being it — matching must be exact-name, not substring.""" + list_output = json.dumps([{"Name": f"ghcr.io/cirruslabs/{BASE_IMAGE_NAME}@sha256:deadbeef"}]) + captured: list[list[str]] = [] + + async def recording_run(cmd: list[str], **_kw) -> tuple[int, str, str]: + captured.append(cmd) + if cmd[:2] == ["tart", "list"]: + return (0, list_output, "") + return (0, "", "") + + async def _run() -> None: + with patch("mac2nix.vm.manager.async_run_command", side_effect=recording_run): + await pull_base_image_if_missing(name=BASE_IMAGE_NAME, image_ref=BASE_IMAGE_REF) + + asyncio.run(_run()) + assert captured[-1] == ["tart", "clone", BASE_IMAGE_REF, BASE_IMAGE_NAME] + + def test_raises_vm_error_on_clone_failure(self) -> None: + list_output = json.dumps([]) + + async def recording_run(cmd: list[str], **_kw) -> tuple[int, str, str]: + if cmd[:2] == ["tart", "list"]: + return (0, list_output, "") + return (1, "", "no space left on device") + + async def _run() -> None: + with patch("mac2nix.vm.manager.async_run_command", side_effect=recording_run): + await pull_base_image_if_missing(name=BASE_IMAGE_NAME, image_ref=BASE_IMAGE_REF) + + with pytest.raises(VMError, match="tart clone"): + asyncio.run(_run()) diff --git a/tests/vm/test_scaffold_vm.py b/tests/vm/test_scaffold_vm.py new file mode 100644 index 0000000..09e3010 --- /dev/null +++ b/tests/vm/test_scaffold_vm.py @@ -0,0 +1,121 @@ +"""Real VM-based apply-and-verify test for the bare, host-registered scaffold — no domains yet. + +Marked `nix_vm` (Step 10's shared marker, registered in pyproject.toml): the +`nix_darwin_vm` fixture skips this test if `tart` is unavailable, otherwise +it must run to completion and pass, no further internal skipping. + +This is the first point in the whole plan that any generated nix-darwin +configuration is actually applied to a running system, however minimal — it +proves the scaffold's own module wiring (`lib/helpers.nix`'s +`mkDarwinSystem`, `modules/darwin/default.nix`'s imports, sops-nix's +`sops.age.keyFile`/`sops.defaultSopsFile` wiring, `modules/darwin/homebrew.nix`'s +activation-policy stub) actually activates cleanly on its own, independent +of any later domain generator's correctness. No `Validator`/fidelity +comparison is needed here — there is no scanned domain data yet for any +generator to reproduce; this is a simple pass/fail activation check. +""" + +from __future__ import annotations + +import asyncio +from pathlib import Path + +import pytest + +from mac2nix.generators.scaffold import add_host, init_framework +from mac2nix.vm._utils import VMError, async_run_command +from mac2nix.vm.manager import TartVMManager +from mac2nix.vm.validator import Validator + +pytestmark = pytest.mark.nix_vm + +_HOSTNAME = "mac2nix-scaffold-vm-test" + +# Tart's base images ship with a real, pre-existing "admin" account — the +# same default TartVMManager itself uses for SSH (vm_user="admin"). Using it +# here too means the macOS account nix-darwin configures via +# users.users. and the account the age key is placed under both refer +# to a real account that actually exists inside the VM. +_VM_USERNAME = "admin" + + +async def _copy_age_key_to_vm(vm: TartVMManager, local_key_path: Path, username: str) -> None: + """SCP the local age key into the VM at the exact path `lib/helpers.nix` expects. + + Mirrors ``Validator._copy_flake_to_vm()``'s sshpass/scp security pattern + (SSHPASS env var, never argv, no shell=True) for a single file rather + than a directory tree. Never logs key content — only file paths, which + aren't secret. + """ + ip = await vm.get_ip() + if not ip: + raise VMError("Cannot copy age key — VM has no IP address") + + remote_dir = f"/Users/{username}/.config/sops/age" + ok, _out, err = await vm.exec_command(["mkdir", "-p", remote_dir]) + if not ok: + raise VMError(f"mkdir {remote_dir!r} failed: {err.strip()}") + + scp_cmd = [ + "sshpass", + "-e", + "scp", + "-o", + "StrictHostKeyChecking=no", + "-o", + "UserKnownHostsFile=/dev/null", + "-o", + "LogLevel=ERROR", + str(local_key_path), + f"{vm.vm_user}@{ip}:{remote_dir}/keys.txt", + ] + returncode, _stdout, stderr = await async_run_command(scp_cmd, timeout=30, env={"SSHPASS": vm.vm_password}) + if returncode != 0: + raise VMError(f"scp age key to VM failed (exit {returncode}): {stderr.strip()}") + + # age-keygen already chmod 0600'd the local file, but scp doesn't + # guarantee preserving that mode remotely — set it explicitly, matching + # generate_age_key()'s own "never assume the tool set it correctly" rule. + ok, _out, err = await vm.exec_command(["chmod", "600", f"{remote_dir}/keys.txt"]) + if not ok: + raise VMError(f"chmod age key in VM failed: {err.strip()}") + + +def test_scaffold_switches_for_real( + nix_darwin_vm: TartVMManager, tmp_path: Path, monkeypatch: pytest.MonkeyPatch +) -> None: + """init_framework() + add_host() must produce a config that really `nix run nix-darwin -- switch`es.""" + # add_host()'s real age-key path always resolves to /Users//... on + # whatever machine runs this test, with no override at that level (by + # design). Redirect it to a scratch dir so this never touches the actual + # host machine's real home directory — the VM-side path (what the + # deployed config actually references) is unaffected, since that's a + # separate, explicit SCP destination below. + 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) + + local_key_path = _fake_age_key_path(_VM_USERNAME) + + async def _run() -> tuple[bool, str, str]: + validator = Validator(nix_darwin_vm) + await validator._copy_flake_to_vm(output_dir) + await _copy_age_key_to_vm(nix_darwin_vm, local_key_path, _VM_USERNAME) + await validator._bootstrap_nix_darwin() + + switch_cmd = ( + f"cd {validator._REMOTE_FLAKE_DIR}" + " && . /nix/var/nix/profiles/default/etc/profile.d/nix-daemon.sh" + f" && nix run nix-darwin -- switch --flake .#{_HOSTNAME}" + ) + return await nix_darwin_vm.exec_command(["bash", "-c", switch_cmd], timeout=900) + + ok, out, err = asyncio.run(_run()) + assert ok, f"nix run nix-darwin -- switch failed:\nstdout:\n{out}\nstderr:\n{err}" diff --git a/tests/vm/test_validator.py b/tests/vm/test_validator.py index f537f00..627eb78 100644 --- a/tests/vm/test_validator.py +++ b/tests/vm/test_validator.py @@ -15,6 +15,8 @@ from mac2nix.models.system_state import SystemState from mac2nix.vm._utils import VMError from mac2nix.vm.validator import ( + _NIX_PROFILE_SOURCE, + NIX_INSTALLER_URL, DomainScore, FidelityReport, Mismatch, @@ -467,6 +469,28 @@ async def _run() -> None: assert captured_env[0] == {"SSHPASS": "admin"} +# --------------------------------------------------------------------------- +# Validator._bootstrap_nix_darwin() — idempotency +# --------------------------------------------------------------------------- + + +class TestBootstrapIdempotency: + def test_skips_install_when_nix_already_present(self) -> None: + """When the profile-sourced `which nix` check succeeds, the install + commands (curl/chmod/installer) must never be invoked.""" + vm = _make_vm(exec_result=(True, "", "")) + + async def _run() -> None: + v = Validator(vm) + await v._bootstrap_nix_darwin() + + asyncio.run(_run()) + + calls = [call.args[0] for call in vm.exec_command.call_args_list] + assert calls == [["bash", "-c", f"{_NIX_PROFILE_SOURCE} && which nix"]] + assert not any(NIX_INSTALLER_URL in " ".join(c) for c in calls) + + # --------------------------------------------------------------------------- # Validator.validate() — full pipeline # --------------------------------------------------------------------------- @@ -540,17 +564,16 @@ async def _run() -> ValidationResult: assert any("bootstrap" in e for e in result.errors) def test_rebuild_failure_returns_early(self) -> None: - # All bootstrap steps succeed, darwin-rebuild switch fails - call_count = 0 - + # 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. async def exec_side_effect(cmd, **_kw): - nonlocal call_count - call_count += 1 - # mkdir(1) + curl(2) + chmod(3) + installer(4) + nix-darwin bootstrap(5) succeed - # darwin-rebuild switch (6th call) fails - if call_count <= 5: - return (True, "admin", "") - return (False, "", "nix-darwin build error") + joined = " ".join(cmd) + if "which nix" in joined: + return (True, "", "") # idempotency check succeeds + if "nix-darwin -- switch" in joined: + return (False, "", "nix-darwin build error") + return (True, "admin", "") # mkdir and any other step succeeds vm = _make_vm() vm.exec_command = AsyncMock(side_effect=exec_side_effect) diff --git a/tests/vm/test_vm_fixtures.py b/tests/vm/test_vm_fixtures.py new file mode 100644 index 0000000..b509a51 --- /dev/null +++ b/tests/vm/test_vm_fixtures.py @@ -0,0 +1,48 @@ +"""Fixture-level tests for tests/vm_fixtures.py's shared nix_darwin_vm fixture. + +Two complementary tests, not redundant with each other: + +- test_nix_darwin_vm_yields_usable_manager uses normal pytest fixture + injection (the same path every real consumer, e.g. test_scaffold_vm.py, + actually uses) to verify the fixture provides a usable, cloned, started + TartVMManager. +- test_nix_darwin_vm_tears_down_even_on_generator_exit drives the fixture's + underlying generator directly (via its `__wrapped__` attribute, since + pytest fixture functions can't be called directly) specifically to + observe state *after* its teardown runs — something a normally-injected + consumer test structurally cannot do, since the fixture's own teardown + always runs after the consuming test's body has already returned. +""" + +from __future__ import annotations + +import pytest + +from mac2nix.vm.manager import TartVMManager +from tests.vm_fixtures import local_vm_names, nix_darwin_vm + +pytestmark = pytest.mark.nix_vm + + +def test_nix_darwin_vm_yields_usable_manager(nix_darwin_vm: TartVMManager) -> None: + assert isinstance(nix_darwin_vm, TartVMManager) + clone_name = nix_darwin_vm._current_clone + assert clone_name is not None + assert clone_name in local_vm_names() + + +@pytest.mark.skipif(not TartVMManager.is_available(), reason="tart not available") +def test_nix_darwin_vm_tears_down_even_on_generator_exit() -> None: + gen = nix_darwin_vm.__wrapped__() + mgr = next(gen) + clone_name = mgr._current_clone + + try: + assert isinstance(mgr, TartVMManager) + assert clone_name is not None + assert clone_name in local_vm_names() + finally: + with pytest.raises(StopIteration): + next(gen) + + assert clone_name not in local_vm_names() diff --git a/tests/vm_fixtures.py b/tests/vm_fixtures.py new file mode 100644 index 0000000..4105311 --- /dev/null +++ b/tests/vm_fixtures.py @@ -0,0 +1,98 @@ +"""Shared nix_darwin_vm fixture — real Tart VM lifecycle for nix_vm-marked tests. + +Reused by this PR's own tests/vm/test_scaffold_vm.py, and by Tasks 5, 6, and 7's +VM-based generator tests, so each doesn't reinvent VM clone/start/teardown. +""" + +from __future__ import annotations + +import asyncio +import contextlib +import json +import os +import subprocess +import uuid + +import pytest + +from mac2nix.vm.manager import BASE_IMAGE_NAME, BASE_IMAGE_REF, TartVMManager, pull_base_image_if_missing + +_PREWARMED_VM_NAME = "mac2nix-nix-base" + + +def local_vm_names() -> set[str]: + """Exact local Tart VM/image names, via JSON — not a plain-text substring check. + + An OCI pull caches under its full registry/repo@digest string, which can + contain a short name like "macos-tahoe-base" as a substring without + actually being named that (mirrors mac2nix.vm.manager's own fix for the + same false-positive pattern). Shared by tests/vm/conftest.py, which + re-exports this rather than keeping its own copy. + """ + try: + result = subprocess.run( + ["tart", "list", "--format", "json"], # noqa: S607 + capture_output=True, + text=True, + timeout=10, + check=False, + ) + except (subprocess.TimeoutExpired, OSError): + return set() + if result.returncode != 0: + return set() + return {entry["Name"] for entry in json.loads(result.stdout)} + + +def _resolve_base_vm() -> str: + """Pick a base VM: MAC2NIX_BASE_VM env override, else a locally pre-warmed + mac2nix-nix-base (see scripts/prewarm_vm.py), else the pinned macos-tahoe-base. + """ + env_base_vm = os.environ.get("MAC2NIX_BASE_VM") + if env_base_vm: + return env_base_vm + + if _PREWARMED_VM_NAME in local_vm_names(): + return _PREWARMED_VM_NAME + return BASE_IMAGE_NAME + + +@pytest.fixture +def nix_darwin_vm(): + """Function-scoped, real Tart VM clone — cloned and started per test, always torn down. + + Skips immediately if `tart` isn't on PATH. Cleanup uses sync subprocess + calls rather than TartVMManager.cleanup() — the setup event loop is + closed by the time teardown runs, and the VM's background process handle + is tied to that closed loop (mirrors tests/vm/conftest.py's shared_vm + fixture, which hit this same issue first). + """ + if not TartVMManager.is_available(): + pytest.skip("tart not available") + + base_vm = _resolve_base_vm() + mgr = TartVMManager(base_vm) + clone_name = f"mac2nix-test-{uuid.uuid4().hex[:8]}" + + async def _setup() -> None: + await pull_base_image_if_missing(name=base_vm, image_ref=BASE_IMAGE_REF) + await mgr.clone(clone_name) + await mgr.start() + + def _sync_cleanup() -> None: + if mgr._vm_process is not None: + with contextlib.suppress(ProcessLookupError, OSError): + mgr._vm_process.kill() + mgr._vm_process = None + subprocess.run(["tart", "stop", clone_name], capture_output=True, check=False) # noqa: S603, S607 + subprocess.run(["tart", "delete", clone_name], capture_output=True, check=False) # noqa: S603, S607 + + try: + asyncio.run(_setup()) + except Exception: + _sync_cleanup() + raise + + yield mgr + + _sync_cleanup() From 45d04dfad36f1554d25c7d009bcd9858df57f140 Mon Sep 17 00:00:00 2001 From: Will Gordon Date: Sun, 2 Aug 2026 13:30:30 -0400 Subject: [PATCH 02/33] fix(tests): skips TestGenerateAgeKey when age-keygen is unavailable MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit lint-and-test's CI job never installed age (only nix-integration and vm-integration do), so these tests failed there with "age-keygen is not available" — harmless before branch protection existed, but a real merge blocker now that lint-and-test is a required status check. Gates the class the same way TestAddHostSops already gates on sops/age together, via a narrower age-keygen-only check. --- tests/_scaffold_helpers.py | 6 +++++- tests/generators/test_scaffold.py | 9 ++++++++- 2 files changed, 13 insertions(+), 2 deletions(-) diff --git a/tests/_scaffold_helpers.py b/tests/_scaffold_helpers.py index cabcf62..0e9ccb3 100644 --- a/tests/_scaffold_helpers.py +++ b/tests/_scaffold_helpers.py @@ -13,8 +13,12 @@ from unittest.mock import patch +def _has_age_keygen() -> bool: + return shutil.which("age-keygen") is not None + + def _has_add_host_crypto_deps() -> bool: - return shutil.which("age-keygen") is not None and shutil.which("sops") is not None + return _has_age_keygen() and shutil.which("sops") is not None @contextlib.contextmanager diff --git a/tests/generators/test_scaffold.py b/tests/generators/test_scaffold.py index c662297..ed6a33b 100644 --- a/tests/generators/test_scaffold.py +++ b/tests/generators/test_scaffold.py @@ -16,7 +16,7 @@ from mac2nix.generators import Mac2NixError from mac2nix.generators.scaffold import ScaffoldError, add_host, generate_age_key, init_framework -from tests._scaffold_helpers import _has_add_host_crypto_deps, _redirect_age_keys +from tests._scaffold_helpers import _has_add_host_crypto_deps, _has_age_keygen, _redirect_age_keys _EXPECTED_FRAMEWORK_FILES = [ ".gitignore", @@ -104,6 +104,13 @@ def test_raises_on_nonempty_directory_writes_nothing(self, tmp_path: Path) -> No # --------------------------------------------------------------------------- +@pytest.fixture +def require_age_keygen() -> None: + if not _has_age_keygen(): + pytest.skip("age-keygen not on PATH") + + +@pytest.mark.usefixtures("require_age_keygen") class TestGenerateAgeKey: def test_permissions_and_fingerprint(self, tmp_path: Path) -> None: key_dir = tmp_path / "keys" From 40cd962c0256d02465df4ba47a7137f087ac18c5 Mon Sep 17 00:00:00 2001 From: Will Gordon Date: Sun, 2 Aug 2026 14:38:52 -0400 Subject: [PATCH 03/33] fix(ci): trusts cirruslabs/cli tap before installing tart --- .github/workflows/pr-checks.yaml | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/.github/workflows/pr-checks.yaml b/.github/workflows/pr-checks.yaml index 9eee6f1..bd39c75 100644 --- a/.github/workflows/pr-checks.yaml +++ b/.github/workflows/pr-checks.yaml @@ -45,7 +45,10 @@ jobs: run: make install - name: Install tart - run: brew install cirruslabs/cli/tart + run: | + brew tap cirruslabs/cli + brew trust cirruslabs/cli + brew install cirruslabs/cli/tart - name: Install sshpass run: | @@ -135,7 +138,10 @@ jobs: run: make install - name: Install tart - run: brew install cirruslabs/cli/tart + run: | + brew tap cirruslabs/cli + brew trust cirruslabs/cli + brew install cirruslabs/cli/tart - name: Install sshpass run: | From 271d3b45e5310845f41f99c31fc419cdbcb2ffd1 Mon Sep 17 00:00:00 2001 From: Will Gordon Date: Sun, 2 Aug 2026 14:55:04 -0400 Subject: [PATCH 04/33] refactor(ci): merges integration job into vm-integration MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit integration ran post-merge-only with no gate and no path-scoping — a holdover from before this PR's pre-merge validation tiers existed, not a deliberate design choice. Its tests (raw TartVMManager lifecycle, FileSystemComparator diffing) are already covered by detect-changes's path filter (src/mac2nix/vm/, tests/vm/), so folding them into vm-integration gets them the same pre-merge timing and approval gate as the new scaffold tests, without paying for a second job's tart/sshpass/sops/age setup. Each suite still manages its own VM independently — this merges the job, not the fixtures. --- .github/workflows/pr-checks.yaml | 43 ++++++++------------------------ 1 file changed, 11 insertions(+), 32 deletions(-) diff --git a/.github/workflows/pr-checks.yaml b/.github/workflows/pr-checks.yaml index bd39c75..9cbecf0 100644 --- a/.github/workflows/pr-checks.yaml +++ b/.github/workflows/pr-checks.yaml @@ -30,38 +30,8 @@ jobs: - name: Test run: make test - integration: - needs: lint-and-test - if: github.event_name == 'push' && github.ref == 'refs/heads/main' - runs-on: macos-latest - steps: - - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 - - - uses: astral-sh/setup-uv@11f9893b081a58869d3b5fccaea48c9e9e46f990 # v8.3.2 - with: - python-version: "3.13" - - - name: Install dependencies - run: make install - - - name: Install tart - run: | - brew tap cirruslabs/cli - brew trust cirruslabs/cli - brew install cirruslabs/cli/tart - - - name: Install sshpass - run: | - brew tap hudochenkov/sshpass - brew install sshpass - - - name: Integration tests - env: - MAC2NIX_BASE_VM: macos-tahoe-base - run: make test-integration - # Runs on every pull_request, unconditional (not gated like the VM-based - # `integration` job above) — this plan's Key Decisions require every + # `vm-integration` job below) — this plan's Key Decisions require every # deliverable validated before merge, not after. No `needs:` on # lint-and-test — runs in parallel. Inherits the workflow's default # read-only GITHUB_TOKEN (no elevated permissions) since this job runs @@ -119,6 +89,15 @@ jobs: # detect-changes says the PR touches VM-relevant paths, and even then waits for a # maintainer's manual approval via the vm-validated environment's required reviewer. # + # Runs both the `integration`-marked tests (raw TartVMManager lifecycle and + # FileSystemComparator diffing — pre-existing VM-control-layer tests, formerly + # their own post-merge-only, ungated job) and the `nix_vm`-marked tests (real + # apply-and-verify: does a *generated* nix-darwin config actually activate). + # Each suite still clones/manages its own VM independently (different fixtures, + # not sharing one boot) — merged into one job so both get the identical + # tart/sshpass/sops/age setup, one approval gate, and pre-merge timing, instead + # of `integration` being the sole remaining post-merge-only, ungated tier. + # # NOTE FOR WHOEVER MERGES THIS PR: the `vm-validated` GitHub Environment and its # required reviewer need to be configured once via repo settings — this is outside # this workflow file's scope. @@ -158,4 +137,4 @@ jobs: # image and lets Validator._bootstrap_nix_darwin() install Nix fresh inside # the VM every run. - name: VM integration tests - run: make test-vm + run: make test-vm test-integration From e85dbc781894cdfca5ea64a997f92efae83cc086 Mon Sep 17 00:00:00 2001 From: Will Gordon Date: Mon, 3 Aug 2026 08:50:09 -0400 Subject: [PATCH 05/33] fix(ci): applies nix-darwin switch natively, drops Tart from CI MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Nested macOS virtualization is categorically unsupported on any GitHub-hosted runner — confirmed via GitHub's own docs, an Apple Virtualization Framework limitation, not a Tart-specific one. `tart run` exits immediately with no boot at all on macos-latest, so the vm-integration job (and the old integration job's tests, which it absorbed) could never have passed there. Renames the job to nix-darwin-switch and applies the generated config directly to the runner itself instead of inside a VM — safe only because the runner is already fully disposable. Adds nix_darwin_switch, a new pytest marker gated to skip unless GITHUB_ACTIONS=true, so it can never run against a real developer machine by accident. test_integration.py's TartVMManager-lifecycle/FileSystemComparator tests test the VM-control layer itself and so can never run on any GitHub-hosted runner regardless of this fix — they stay a local-only check (make test-integration). Also sets nix-homebrew.autoMigrate = true in the scaffold template: without it, nix-homebrew expects to own an empty Homebrew prefix and fails against the runner's already-populated one — but this is also the objectively correct setting independent of CI, since mac2nix's whole premise is migrating a Mac that's already running Homebrew. --- .github/workflows/pr-checks.yaml | 59 +++++----- Makefile | 7 +- pyproject.toml | 3 +- .../templates/scaffold/lib/helpers.nix | 5 + .../generators/test_scaffold_switch_native.py | 104 ++++++++++++++++++ 5 files changed, 144 insertions(+), 34 deletions(-) create mode 100644 tests/generators/test_scaffold_switch_native.py diff --git a/.github/workflows/pr-checks.yaml b/.github/workflows/pr-checks.yaml index 9cbecf0..1e96f53 100644 --- a/.github/workflows/pr-checks.yaml +++ b/.github/workflows/pr-checks.yaml @@ -30,8 +30,8 @@ jobs: - name: Test run: make test - # Runs on every pull_request, unconditional (not gated like the VM-based - # `vm-integration` job below) — this plan's Key Decisions require every + # Runs on every pull_request, unconditional (not gated like the + # `nix-darwin-switch` job below) — this plan's Key Decisions require every # deliverable validated before merge, not after. No `needs:` on # lint-and-test — runs in parallel. Inherits the workflow's default # read-only GITHUB_TOKEN (no elevated permissions) since this job runs @@ -85,23 +85,33 @@ jobs: echo "vm-relevant=false" >> "$GITHUB_OUTPUT" fi - # Expensive (VM boot + fresh Nix install every run) and gated: only runs when - # detect-changes says the PR touches VM-relevant paths, and even then waits for a - # maintainer's manual approval via the vm-validated environment's required reviewer. + # Real, invasive (applies a genuine nix-darwin switch to the runner itself) and + # gated: only runs when detect-changes says the PR touches VM/generator-relevant + # paths, and even then waits for a maintainer's manual approval via the + # vm-validated environment's required reviewer. # - # Runs both the `integration`-marked tests (raw TartVMManager lifecycle and - # FileSystemComparator diffing — pre-existing VM-control-layer tests, formerly - # their own post-merge-only, ungated job) and the `nix_vm`-marked tests (real - # apply-and-verify: does a *generated* nix-darwin config actually activate). - # Each suite still clones/manages its own VM independently (different fixtures, - # not sharing one boot) — merged into one job so both get the identical - # tart/sshpass/sops/age setup, one approval gate, and pre-merge timing, instead - # of `integration` being the sole remaining post-merge-only, ungated tier. + # Nested macOS virtualization is categorically unsupported on GitHub-hosted + # runners (confirmed via GitHub's own docs — an Apple Virtualization Framework + # limitation, not a Tart-specific one: `tart run` exits immediately with no + # boot at all), so a Tart-VM-based apply-and-verify check — what this job + # originally did, and what `tests/vm/test_scaffold_vm.py` still does for local + # development on real Apple Silicon hardware — cannot run here. Instead this + # job applies the switch directly to the runner itself, which is safe only + # because the runner is already fully disposable (destroyed after the job). + # See tests/generators/test_scaffold_switch_native.py's own docstring for the + # full safety gating (skips unless GITHUB_ACTIONS=true, fails loudly rather + # than reusing a real key that shouldn't exist on a fresh runner). + # + # test_integration.py's TartVMManager-lifecycle/FileSystemComparator tests are + # deliberately NOT run here — they test the VM-control layer itself, which + # needs an actual VM boot regardless of what's being tested, so they can never + # run on any GitHub-hosted runner. They remain a local-only check + # (`make test-integration`, requires real Tart-capable hardware). # # NOTE FOR WHOEVER MERGES THIS PR: the `vm-validated` GitHub Environment and its # required reviewer need to be configured once via repo settings — this is outside # this workflow file's scope. - vm-integration: + nix-darwin-switch: needs: detect-changes if: needs.detect-changes.outputs.vm-relevant == 'true' runs-on: macos-latest @@ -116,25 +126,10 @@ jobs: - name: Install dependencies run: make install - - name: Install tart - run: | - brew tap cirruslabs/cli - brew trust cirruslabs/cli - brew install cirruslabs/cli/tart - - - name: Install sshpass - run: | - brew tap hudochenkov/sshpass - brew install sshpass + - uses: DeterminateSystems/nix-installer-action@ef8a148080ab6020fd15196c2084a2eea5ff2d25 # v22 - name: Install sops and age run: brew install sops age - # No Nix install needed on the CI host itself — Validator's whole flow (copy - # flake via SCP, bootstrap Nix, switch, re-scan) happens over SSH inside the - # VM, never locally. This deliberately does not use `make prewarm-vm`: per - # this project's own CI scope decision, it clones the raw macos-tahoe-base - # image and lets Validator._bootstrap_nix_darwin() install Nix fresh inside - # the VM every run. - - name: VM integration tests - run: make test-vm test-integration + - name: Real nix-darwin switch (applied to this disposable runner) + run: make test-nix-darwin-switch diff --git a/Makefile b/Makefile index cce7b17..57cb0c8 100644 --- a/Makefile +++ b/Makefile @@ -1,5 +1,5 @@ .DEFAULT_GOAL := all -.PHONY: install lint format typecheck test test-integration test-vm test-nix prewarm-vm pull-base-vm test-quick clean all prek-install prek +.PHONY: install lint format typecheck test test-integration test-vm test-nix test-nix-darwin-switch prewarm-vm pull-base-vm test-quick clean all prek-install prek install: uv sync @@ -42,6 +42,11 @@ test-vm: pull-base-vm test-nix: uv run pytest -m nix_build --tb=long +# CI-only — skips unless GITHUB_ACTIONS=true (see the test module's own docstring). +# Never invoke this on a real machine; it applies a genuine nix-darwin switch. +test-nix-darwin-switch: + uv run pytest -m nix_darwin_switch --tb=long + prewarm-vm: uv run python scripts/prewarm_vm.py diff --git a/pyproject.toml b/pyproject.toml index ecb58c0..7811339 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -82,13 +82,14 @@ python_functions = "test_*" addopts = [ "--strict-markers", "--tb=short", - "-m", "not integration and not nix_vm and not nix_build", + "-m", "not integration and not nix_vm and not nix_build and not nix_darwin_switch", ] markers = [ "integration: real VM tests — require tart + sshpass + base VM image", "nix: nix-instantiate syntax validation — require Nix on PATH", "nix_vm: real VM-based apply-and-verify tests — require tart", "nix_build: real nix flake lock/build tests — never skipped, require nix + age + sops + network", + "nix_darwin_switch: real nix-darwin switch applied to the running machine — CI-only, skips unless GITHUB_ACTIONS=true", ] cache_dir = ".cache/pytest" diff --git a/src/mac2nix/templates/scaffold/lib/helpers.nix b/src/mac2nix/templates/scaffold/lib/helpers.nix index ae68350..b45121f 100644 --- a/src/mac2nix/templates/scaffold/lib/helpers.nix +++ b/src/mac2nix/templates/scaffold/lib/helpers.nix @@ -33,6 +33,11 @@ darwin.lib.darwinSystem { "homebrew/homebrew-cask" = homebrew-cask; }; mutableTaps = false; + # This host's own Homebrew may already exist and be populated — + # mac2nix's whole premise is migrating a Mac that's already running + # Homebrew, not a bare machine. Without this, nix-homebrew expects to + # own an empty prefix and fails against any pre-existing install. + autoMigrate = true; }; home-manager.useGlobalPkgs = true; diff --git a/tests/generators/test_scaffold_switch_native.py b/tests/generators/test_scaffold_switch_native.py new file mode 100644 index 0000000..b528dd4 --- /dev/null +++ b/tests/generators/test_scaffold_switch_native.py @@ -0,0 +1,104 @@ +"""Real `nix-darwin switch` applied directly to the machine running this test — no VM. + +Marked `nix_darwin_switch`, excluded from the default `pytest`/`make test` +run. Unlike `test_scaffold_vm.py` (the Tart-VM-based equivalent, kept for +local development on real Apple Silicon hardware), this test applies the +generated config to whatever real system runs it — safe only because a +GitHub Actions runner is itself fully disposable, destroyed after the job +completes. It must NEVER run on a developer's actual machine: the gate below +skips unless `GITHUB_ACTIONS=true`, GitHub Actions' own environment variable +(not something set on a real dev machine by accident), so a bare `pytest -m +nix_darwin_switch` locally skips safely by default rather than mutating a +real system. + +Nested macOS virtualization is categorically unsupported on GitHub-hosted +runners (confirmed via GitHub's own documentation — an Apple Virtualization +Framework limitation, not a Tart-specific one), so `test_scaffold_vm.py`'s +Tart-based approach cannot run there at all. Running natively sidesteps that +wall entirely, at the cost of testing against an already-provisioned CI +image rather than a pristine machine — `lib/helpers.nix`'s +`nix-homebrew.autoMigrate = true` exists specifically to let this test +(and any real user migrating an already-Homebrew Mac) apply cleanly despite +that. + +The age key must land at the exact real path (`/Users//.config/ +sops/age/keys.txt`) `lib/helpers.nix`'s `sops.age.keyFile` expects — unlike +`test_scaffold_builds_for_real`'s redirected key (fine for a build-only +check), a real switch reads that literal path at apply time and cannot be +redirected. Fails loudly if a key already exists there rather than silently +reusing or overwriting it — should never happen on a fresh CI runner, and +if it does, that's worth knowing about rather than masking. +""" + +from __future__ import annotations + +import asyncio +import getpass +import os +import shutil +from collections.abc import Iterator +from pathlib import Path + +import pytest + +from mac2nix.generators.scaffold import add_host, init_framework + +pytestmark = pytest.mark.nix_darwin_switch + +_HOSTNAME = "mac2nix-native-switch-test" + + +def _is_github_actions() -> bool: + return os.environ.get("GITHUB_ACTIONS") == "true" + + +@pytest.fixture +def real_age_key() -> Iterator[Path]: + if not _is_github_actions(): + pytest.skip("only runs under GITHUB_ACTIONS=true — never against a real developer machine") + + username = getpass.getuser() + key_path = Path(f"/Users/{username}/.config/sops/age/keys.txt") + if key_path.exists(): + pytest.fail( + f"a real sops age key already exists at {key_path} — refusing to overwrite or reuse it. " + "This should never happen on a fresh CI runner." + ) + + try: + yield key_path + finally: + key_path.unlink(missing_ok=True) + + +def test_scaffold_switches_for_real_natively(real_age_key: Path, tmp_path: Path) -> None: + """init_framework() + add_host() must produce a config that really `nix run nix-darwin -- switch`es.""" + username = getpass.getuser() + output_dir = tmp_path / "mac2nix-scaffold" + + init_framework(output_dir) + add_host(output_dir, _HOSTNAME, username, confirm_backup=lambda _fingerprint: True) + + assert real_age_key.is_file(), "add_host() should have written the real age key to the real expected path" + + nix_bin = shutil.which("nix") + assert nix_bin is not None, "nix must be installed on this runner before this test can apply anything" + + async def _run() -> tuple[int, str, str]: + proc = await asyncio.create_subprocess_exec( + nix_bin, + "run", + "nix-darwin", + "--", + "switch", + "--flake", + f".#{_HOSTNAME}", + cwd=output_dir, + stdout=asyncio.subprocess.PIPE, + stderr=asyncio.subprocess.PIPE, + ) + stdout, stderr = await asyncio.wait_for(proc.communicate(), timeout=900) + return proc.returncode or 0, stdout.decode(), stderr.decode() + + returncode, out, err = asyncio.run(_run()) + assert returncode == 0, f"nix run nix-darwin -- switch failed (exit {returncode}):\nstdout:\n{out}\nstderr:\n{err}" From 9fa58e05c9aed426ee9d4356f3edb9cec96f509f Mon Sep 17 00:00:00 2001 From: Will Gordon Date: Mon, 3 Aug 2026 08:58:17 -0400 Subject: [PATCH 06/33] fix(tests): runs nix-darwin switch under sudo -n with absolute nix path --- tests/generators/test_scaffold_switch_native.py | 15 ++++++++++++++- 1 file changed, 14 insertions(+), 1 deletion(-) diff --git a/tests/generators/test_scaffold_switch_native.py b/tests/generators/test_scaffold_switch_native.py index b528dd4..4c6d860 100644 --- a/tests/generators/test_scaffold_switch_native.py +++ b/tests/generators/test_scaffold_switch_native.py @@ -83,9 +83,20 @@ def test_scaffold_switches_for_real_natively(real_age_key: Path, tmp_path: Path) nix_bin = shutil.which("nix") assert nix_bin is not None, "nix must be installed on this runner before this test can apply anything" + sudo_bin = shutil.which("sudo") + assert sudo_bin is not None, "sudo must be available to run nix-darwin's system activation" async def _run() -> tuple[int, str, str]: + # nix-darwin's system activation now always runs as root (per + # `system.primaryUser`'s own purpose) — `-n` fails fast with a clear + # error instead of hanging for the full timeout if passwordless sudo + # isn't actually available, rather than silently waiting on a prompt + # that will never come. The absolute `nix_bin` path (not a bare `nix` + # on PATH) avoids sudo's restricted `secure_path` not including + # wherever Nix installed its own binaries. proc = await asyncio.create_subprocess_exec( + sudo_bin, + "-n", nix_bin, "run", "nix-darwin", @@ -101,4 +112,6 @@ async def _run() -> tuple[int, str, str]: return proc.returncode or 0, stdout.decode(), stderr.decode() returncode, out, err = asyncio.run(_run()) - assert returncode == 0, f"nix run nix-darwin -- switch failed (exit {returncode}):\nstdout:\n{out}\nstderr:\n{err}" + assert returncode == 0, ( + f"sudo nix run nix-darwin -- switch failed (exit {returncode}):\nstdout:\n{out}\nstderr:\n{err}" + ) From 53cdba6bc6d246c022c1b45edddac4d4d559d934 Mon Sep 17 00:00:00 2001 From: Will Gordon Date: Mon, 3 Aug 2026 09:06:51 -0400 Subject: [PATCH 07/33] fix(ci): moves pre-existing nix.custom.conf before nix-darwin switch --- .github/workflows/pr-checks.yaml | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/.github/workflows/pr-checks.yaml b/.github/workflows/pr-checks.yaml index 1e96f53..57ac160 100644 --- a/.github/workflows/pr-checks.yaml +++ b/.github/workflows/pr-checks.yaml @@ -131,5 +131,20 @@ jobs: - name: Install sops and age run: brew install sops age + # nix-darwin refuses to overwrite any /etc file it doesn't already manage + # and finds with unrecognized content — a safety guard against silently + # clobbering pre-existing system config, not a mac2nix-specific problem. + # DeterminateSystems/nix-installer-action itself writes nix.custom.conf, + # which nix-darwin's own Nix management then wants to own — exactly the + # scenario nix-darwin's own error message describes, with its own + # suggested fix (rename it out of the way first). A real user migrating a + # Mac that already has Nix installed independently of mac2nix would hit + # this identically and need the same one-time manual step. + - name: Move pre-existing /etc/nix/nix.custom.conf out of nix-darwin's way + run: | + if [ -f /etc/nix/nix.custom.conf ]; then + sudo mv /etc/nix/nix.custom.conf /etc/nix/nix.custom.conf.before-nix-darwin + fi + - name: Real nix-darwin switch (applied to this disposable runner) run: make test-nix-darwin-switch From 061f0266dceebd8c0782c9e86e2b626b70f63c6e Mon Sep 17 00:00:00 2001 From: Will Gordon Date: Mon, 3 Aug 2026 09:16:33 -0400 Subject: [PATCH 08/33] fix(ci): removes pre-existing Homebrew Taps before autoMigrate --- .github/workflows/pr-checks.yaml | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/.github/workflows/pr-checks.yaml b/.github/workflows/pr-checks.yaml index 57ac160..f381b83 100644 --- a/.github/workflows/pr-checks.yaml +++ b/.github/workflows/pr-checks.yaml @@ -146,5 +146,17 @@ jobs: sudo mv /etc/nix/nix.custom.conf /etc/nix/nix.custom.conf.before-nix-darwin fi + # nix-homebrew's autoMigrate takes over an existing Homebrew install by + # replacing Library/Taps with its own managed symlinks — a known rough + # edge (reported by other autoMigrate users) when a real, pre-existing + # Taps directory is already there, which this runner's pre-provisioned + # Homebrew always has. Unlike the /etc/nix conflict above, nix-darwin's + # own activation already self-heals an existing Library/Homebrew (logs + # a Warning, not an Error) — only Taps needs removing first. A real user + # migrating a Mac with several existing taps would hit this identically; + # worth carrying into a future migration runbook, not just this CI step. + - name: Remove pre-existing Homebrew Taps directory before autoMigrate + run: sudo rm -rf /opt/homebrew/Library/Taps + - name: Real nix-darwin switch (applied to this disposable runner) run: make test-nix-darwin-switch From afa11b37495602531239f9eae271c3fe1e8a87be Mon Sep 17 00:00:00 2001 From: Will Gordon Date: Mon, 3 Aug 2026 09:26:47 -0400 Subject: [PATCH 09/33] fix(ci): wipes Homebrew before switch, installs sops/age via nix --- .github/workflows/pr-checks.yaml | 33 +++++++++++++++++++------------- 1 file changed, 20 insertions(+), 13 deletions(-) diff --git a/.github/workflows/pr-checks.yaml b/.github/workflows/pr-checks.yaml index f381b83..6d7c249 100644 --- a/.github/workflows/pr-checks.yaml +++ b/.github/workflows/pr-checks.yaml @@ -128,8 +128,11 @@ jobs: - uses: DeterminateSystems/nix-installer-action@ef8a148080ab6020fd15196c2084a2eea5ff2d25 # v22 - - name: Install sops and age - run: brew install sops age + # Installed via Nix, not Homebrew, so they survive the Homebrew removal + # below — add_host() needs sops/age-keygen on PATH independent of + # whatever happens to Homebrew during the switch. + - name: Install sops and age via nix + run: nix profile install nixpkgs#sops nixpkgs#age # nix-darwin refuses to overwrite any /etc file it doesn't already manage # and finds with unrecognized content — a safety guard against silently @@ -146,17 +149,21 @@ jobs: sudo mv /etc/nix/nix.custom.conf /etc/nix/nix.custom.conf.before-nix-darwin fi - # nix-homebrew's autoMigrate takes over an existing Homebrew install by - # replacing Library/Taps with its own managed symlinks — a known rough - # edge (reported by other autoMigrate users) when a real, pre-existing - # Taps directory is already there, which this runner's pre-provisioned - # Homebrew always has. Unlike the /etc/nix conflict above, nix-darwin's - # own activation already self-heals an existing Library/Homebrew (logs - # a Warning, not an Error) — only Taps needs removing first. A real user - # migrating a Mac with several existing taps would hit this identically; - # worth carrying into a future migration runbook, not just this CI step. - - name: Remove pre-existing Homebrew Taps directory before autoMigrate - run: sudo rm -rf /opt/homebrew/Library/Taps + # nix-homebrew's autoMigrate adopts an existing Homebrew install, but + # this runner's pre-provisioned Homebrew has real content from dozens of + # formulae/casks across several non-default taps (Azure, AWS, etc.) — + # migration kept cascading through one conflict after another (an + # existing Library/Taps directory, then a formula whose originating tap + # had just been removed). Rather than keep chasing individual + # conflicts on an image this heavily provisioned, remove Homebrew + # entirely first: nix-homebrew then does a normal fresh install against + # an empty prefix, sidestepping autoMigrate's adoption path altogether. + # autoMigrate = true stays set in the scaffold template regardless — + # it's still the objectively correct setting for a real user's actual + # migration target, which (unlike this disposable runner) genuinely + # needs its existing Homebrew adopted rather than wiped. + - name: Remove pre-existing Homebrew installation + run: sudo rm -rf /opt/homebrew - name: Real nix-darwin switch (applied to this disposable runner) run: make test-nix-darwin-switch From 6758698349e40546a712598cb0914d476238adc2 Mon Sep 17 00:00:00 2001 From: Will Gordon Date: Mon, 3 Aug 2026 11:15:07 -0400 Subject: [PATCH 10/33] fix(ci): authenticates nix's github flake-input fetches MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit nix run nix-darwin -- switch failed with "API rate limit exceeded" resolving github:nix-darwin/nix-darwin — Nix's own flake-input fetching hits GitHub's REST API unauthenticated by default, capped at 60/hr per egress IP, shared across every unrelated customer on the same NAT'd GitHub Actions IP pool, not scoped to our own traffic. Adds a shared _nix_extra_access_tokens_args() helper that threads GITHUB_TOKEN through as an explicit --extra-access-tokens CLI argument, used by both nix-integration and nix-darwin-switch's real nix invocations. Passed as a literal argument rather than via NIX_CONFIG: sudo strips the calling environment by default, and multi-user/daemon Nix installs don't reliably forward client-side NIX_CONFIG to the daemon's own fetches — a CLI flag applies to the invoked process regardless of either concern. Fails open (empty arg list) if GITHUB_TOKEN isn't set, matching this project's other real-network tests. --- .github/workflows/pr-checks.yaml | 14 ++++++++++++ tests/_scaffold_helpers.py | 22 +++++++++++++++++++ tests/generators/test_scaffold_integration.py | 9 ++++---- .../generators/test_scaffold_switch_native.py | 13 ++++++++++- 4 files changed, 53 insertions(+), 5 deletions(-) diff --git a/.github/workflows/pr-checks.yaml b/.github/workflows/pr-checks.yaml index 6d7c249..9b2d39b 100644 --- a/.github/workflows/pr-checks.yaml +++ b/.github/workflows/pr-checks.yaml @@ -58,7 +58,14 @@ jobs: - name: Install sops and age run: brew install sops age + # Authenticates nix's own github: flake-input fetches (see + # tests/_scaffold_helpers.py's _nix_extra_access_tokens_args() for why + # this matters on a shared, unauthenticated-rate-limited runner IP + # pool). Default read-only GITHUB_TOKEN — no elevated permissions, + # matches this job's existing security posture against fork PRs. - name: Nix build integration tests + env: + GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} run: make test-nix # Job-level (not workflow-level `on.pull_request.paths`) scoping: a workflow-level @@ -165,5 +172,12 @@ jobs: - name: Remove pre-existing Homebrew installation run: sudo rm -rf /opt/homebrew + # See the nix-integration job's own comment on GITHUB_TOKEN — same + # reasoning applies here, for this job's own github: flake-input + # fetches during the real switch (which sudo's env-stripping means + # can't just be set via NIX_CONFIG at the job level; the test itself + # threads this through as an explicit nix CLI argument instead). - name: Real nix-darwin switch (applied to this disposable runner) + env: + GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} run: make test-nix-darwin-switch diff --git a/tests/_scaffold_helpers.py b/tests/_scaffold_helpers.py index 0e9ccb3..2ebfccc 100644 --- a/tests/_scaffold_helpers.py +++ b/tests/_scaffold_helpers.py @@ -7,12 +7,34 @@ from __future__ import annotations import contextlib +import os import shutil from collections.abc import Iterator from pathlib import Path from unittest.mock import patch +def _nix_extra_access_tokens_args() -> list[str]: + """Extra `nix` CLI args authenticating `github:` flake-input fetches. + + Without this, Nix's own flake-input resolution hits GitHub's REST API + unauthenticated (e.g. resolving `github:nix-darwin/nix-darwin` to a + commit) — capped at 60/hr *per shared egress IP*, not per caller, so on + GitHub-hosted runners this pool is exhausted by unrelated traffic from + other customers on the same NAT'd IP, independent of how many requests + *we've* made. Passed as an explicit CLI flag rather than via NIX_CONFIG + or a nix.conf file: `sudo` strips environment variables by default + (would need `-E`, itself a broader change), and multi-user/daemon Nix + installs don't reliably pick up client-side NIX_CONFIG for the daemon's + own fetches. A CLI flag applies directly to the invoked process either + way. Returns an empty list (fails open, not closed — matches this + project's other real-network tests) if GITHUB_TOKEN isn't set, since + that should only happen outside GitHub Actions. + """ + token = os.environ.get("GITHUB_TOKEN") + return ["--extra-access-tokens", f"github.com={token}"] if token else [] + + def _has_age_keygen() -> bool: return shutil.which("age-keygen") is not None diff --git a/tests/generators/test_scaffold_integration.py b/tests/generators/test_scaffold_integration.py index c47ad65..fa71c9a 100644 --- a/tests/generators/test_scaffold_integration.py +++ b/tests/generators/test_scaffold_integration.py @@ -37,7 +37,7 @@ import pytest from mac2nix.generators.scaffold import add_host, init_framework -from tests._scaffold_helpers import _redirect_age_keys +from tests._scaffold_helpers import _nix_extra_access_tokens_args, _redirect_age_keys pytestmark = pytest.mark.nix_build @@ -48,13 +48,14 @@ def test_scaffold_builds_for_real(tmp_path: Path) -> None: """init_framework() + add_host() must produce a flake that actually `nix build`s.""" 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) - lock_result = subprocess.run( - ["nix", "flake", "lock"], # noqa: S607 + lock_result = subprocess.run( # noqa: S603 + ["nix", "flake", "lock", *token_args], # noqa: S607 cwd=output_dir, capture_output=True, text=True, @@ -63,7 +64,7 @@ def test_scaffold_builds_for_real(tmp_path: Path) -> None: 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"], # noqa: S607 + ["nix", "build", f".#darwinConfigurations.{_HOSTNAME}.system", "--no-link", *token_args], # noqa: S607 cwd=output_dir, capture_output=True, text=True, diff --git a/tests/generators/test_scaffold_switch_native.py b/tests/generators/test_scaffold_switch_native.py index 4c6d860..3c72a12 100644 --- a/tests/generators/test_scaffold_switch_native.py +++ b/tests/generators/test_scaffold_switch_native.py @@ -42,6 +42,7 @@ import pytest from mac2nix.generators.scaffold import add_host, init_framework +from tests._scaffold_helpers import _nix_extra_access_tokens_args pytestmark = pytest.mark.nix_darwin_switch @@ -86,6 +87,12 @@ def test_scaffold_switches_for_real_natively(real_age_key: Path, tmp_path: Path) sudo_bin = shutil.which("sudo") assert sudo_bin is not None, "sudo must be available to run nix-darwin's system activation" + # Placed immediately after nix_bin, before the `run` subcommand and well + # before `--` — --extra-access-tokens is a nix-level common flag, and + # anything after `--` belongs to the invoked nix-darwin program instead, + # not to nix itself, so it must not land there. + token_args = _nix_extra_access_tokens_args() + async def _run() -> tuple[int, str, str]: # nix-darwin's system activation now always runs as root (per # `system.primaryUser`'s own purpose) — `-n` fails fast with a clear @@ -93,11 +100,15 @@ async def _run() -> tuple[int, str, str]: # isn't actually available, rather than silently waiting on a prompt # that will never come. The absolute `nix_bin` path (not a bare `nix` # on PATH) avoids sudo's restricted `secure_path` not including - # wherever Nix installed its own binaries. + # wherever Nix installed its own binaries. sudo strips the calling + # environment by default, so --extra-access-tokens is passed as an + # explicit argument rather than via NIX_CONFIG, which wouldn't survive + # the elevation. proc = await asyncio.create_subprocess_exec( sudo_bin, "-n", nix_bin, + *token_args, "run", "nix-darwin", "--", From 28c9fc90c6506ecc9d030c7a1c2c6f1b1e295dbd Mon Sep 17 00:00:00 2001 From: Will Gordon Date: Mon, 3 Aug 2026 11:51:07 -0400 Subject: [PATCH 11/33] fix(ci): propagates nix github token through sudo and wrapper scripts --- tests/_scaffold_helpers.py | 26 +++++++++++++++++++ .../generators/test_scaffold_switch_native.py | 20 +++++++------- 2 files changed, 35 insertions(+), 11 deletions(-) diff --git a/tests/_scaffold_helpers.py b/tests/_scaffold_helpers.py index 2ebfccc..be9d5a8 100644 --- a/tests/_scaffold_helpers.py +++ b/tests/_scaffold_helpers.py @@ -35,6 +35,32 @@ def _nix_extra_access_tokens_args() -> list[str]: return ["--extra-access-tokens", f"github.com={token}"] if token else [] +def _nix_config_env_prefix_args() -> list[str]: + """`env NIX_CONFIG=...` argv prefix authenticating nix invocations reached through `sudo`. + + `_nix_extra_access_tokens_args()`'s CLI flag only configures the single + `nix` process it's passed to. `sudo nix run nix-darwin -- switch` execs + `darwin-rebuild`, which spawns its own separate, fresh `nix build`/`nix + flake lock` subprocess to resolve the *target flake's own* inputs + (nixpkgs, home-manager, sops-nix) — that child process never sees a CLI + flag given to its parent. An environment variable does survive fork/exec + down that whole chain, which is why this returns an `env NAME=value` + argv prefix (to place directly after `sudo -n`, before the real command) + instead of a plain env var: `sudo` resets almost the entire environment + of the process *it* execs by default, but running `env` as the thing sudo + execs sidesteps that — `env` is just a normal program that sets a var in + *its own* clean environment before exec-ing the real command, and that + var then flows down normally to every descendant from there. + """ + token = os.environ.get("GITHUB_TOKEN") + if not token: + return [] + env_bin = shutil.which("env") + if env_bin is None: + return [] + return [env_bin, f"NIX_CONFIG=extra-access-tokens = github.com={token}"] + + def _has_age_keygen() -> bool: return shutil.which("age-keygen") is not None diff --git a/tests/generators/test_scaffold_switch_native.py b/tests/generators/test_scaffold_switch_native.py index 3c72a12..19c9dde 100644 --- a/tests/generators/test_scaffold_switch_native.py +++ b/tests/generators/test_scaffold_switch_native.py @@ -42,7 +42,7 @@ import pytest from mac2nix.generators.scaffold import add_host, init_framework -from tests._scaffold_helpers import _nix_extra_access_tokens_args +from tests._scaffold_helpers import _nix_config_env_prefix_args pytestmark = pytest.mark.nix_darwin_switch @@ -87,11 +87,12 @@ def test_scaffold_switches_for_real_natively(real_age_key: Path, tmp_path: Path) sudo_bin = shutil.which("sudo") assert sudo_bin is not None, "sudo must be available to run nix-darwin's system activation" - # Placed immediately after nix_bin, before the `run` subcommand and well - # before `--` — --extra-access-tokens is a nix-level common flag, and - # anything after `--` belongs to the invoked nix-darwin program instead, - # not to nix itself, so it must not land there. - token_args = _nix_extra_access_tokens_args() + # See _nix_config_env_prefix_args()'s own docstring for why this must be + # an `env NAME=value` argv prefix (surviving both sudo's env-stripping + # and the separate nix subprocess darwin-rebuild spawns internally to + # resolve this flake's own inputs) rather than a --extra-access-tokens + # CLI flag, which only configures the single process it's passed to. + nix_config_prefix = _nix_config_env_prefix_args() async def _run() -> tuple[int, str, str]: # nix-darwin's system activation now always runs as root (per @@ -100,15 +101,12 @@ async def _run() -> tuple[int, str, str]: # isn't actually available, rather than silently waiting on a prompt # that will never come. The absolute `nix_bin` path (not a bare `nix` # on PATH) avoids sudo's restricted `secure_path` not including - # wherever Nix installed its own binaries. sudo strips the calling - # environment by default, so --extra-access-tokens is passed as an - # explicit argument rather than via NIX_CONFIG, which wouldn't survive - # the elevation. + # wherever Nix installed its own binaries. proc = await asyncio.create_subprocess_exec( sudo_bin, "-n", + *nix_config_prefix, nix_bin, - *token_args, "run", "nix-darwin", "--", From d39054e5f14b608a79ca35f6a614a0381343d8db Mon Sep 17 00:00:00 2001 From: Will Gordon Date: Wed, 5 Aug 2026 17:04:10 -0400 Subject: [PATCH 12/33] feat(cli): reworks add-host confirmation flow per UAT feedback Replaces the free-text "type CONFIRMED" prompt with a y/N confirmation that reprompts on "no" instead of aborting, giving the operator time to actually back up the key without losing the just-generated one. Adds optional --op-vault 1Password backup via a new mac2nix.onepassword module, verified by reading the stored item back and comparing content byte-for-byte before ever skipping the manual confirmation. Adds prompts to run `nix flake lock` immediately and to register additional hosts in the same invocation, both defaulting to declined on EOF so an already-registered host is never rolled back by an exhausted prompt. Rewords the private-repo reminder: add-host introduces identifying hostname/username metadata, not scan-derived configuration (that belongs with the still-unimplemented `generate` command). Adds a public age_key_path() accessor to scaffold.py so the CLI and the new onepassword module share one path construction instead of each duplicating the /Users//.config/sops/age/keys.txt literal. --- src/mac2nix/cli.py | 127 ++++++++++++++++++++---- src/mac2nix/generators/scaffold.py | 11 +++ src/mac2nix/onepassword.py | 78 +++++++++++++++ tests/cli/test_add_host.py | 153 +++++++++++++++++++++++++++-- tests/test_onepassword.py | 141 ++++++++++++++++++++++++++ 5 files changed, 483 insertions(+), 27 deletions(-) create mode 100644 src/mac2nix/onepassword.py create mode 100644 tests/test_onepassword.py diff --git a/src/mac2nix/cli.py b/src/mac2nix/cli.py index a22aead..68a15eb 100644 --- a/src/mac2nix/cli.py +++ b/src/mac2nix/cli.py @@ -5,6 +5,8 @@ import asyncio import getpass import re +import shutil +import subprocess import time import uuid from collections import Counter @@ -18,7 +20,8 @@ from rich.table import Table from rich.text import Text -from mac2nix.generators.scaffold import add_host, init_framework +from mac2nix import onepassword +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 from mac2nix.scan_report import ScannerOutcome, ScannerStatus, capture_scanner_logs, get_remediation_hint @@ -222,22 +225,65 @@ def init(output_dir: Path) -> None: _HOSTNAME_RE = re.compile(r"^[a-z][a-z0-9-]*$") _USERNAME_RE = re.compile(r"^[a-z_][a-z0-9_-]*$") +_SYSTEM_CHOICES = ("aarch64-darwin", "x86_64-darwin") -def _validate_hostname(_ctx: click.Context, _param: click.Parameter, value: str) -> str: +def _check_hostname(value: str) -> str: if not _HOSTNAME_RE.match(value): msg = "hostname must match ^[a-z][a-z0-9-]*$ (lowercase alphanumeric and hyphens, starting with a letter)" raise click.BadParameter(msg) return value -def _validate_username(_ctx: click.Context, _param: click.Parameter, value: str) -> str: +def _check_username(value: str) -> str: if not _USERNAME_RE.match(value): msg = "username must match ^[a-z_][a-z0-9_-]*$" raise click.BadParameter(msg) return value +def _validate_hostname(_ctx: click.Context, _param: click.Parameter, value: str) -> str: + return _check_hostname(value) + + +def _validate_username(_ctx: click.Context, _param: click.Parameter, value: str) -> str: + return _check_username(value) + + +def _confirm_or_default(prompt: str, *, default: bool) -> bool: + """Like `click.confirm()`, but resolves to *default* on EOF instead of aborting. + + For the two trailing, non-destructive prompts (register another host, + run `nix flake lock` now) — by the time these run, any host registered + earlier in this invocation is already fully committed, so an exhausted + stdin here should behave like silently declining, not like aborting a + command that already did its real work. + """ + try: + return click.confirm(prompt, default=default) + except EOFError: + return default + + +def _run_nix_flake_lock(output_dir: Path) -> None: + """Run `nix flake lock` in *output_dir*, streaming its real output live. + + A small, separately-named wrapper (rather than inlining the subprocess + call at its call site) so tests can patch just this one call without + touching the shared `subprocess` module object — patching + `subprocess.run` globally would also intercept scaffold.py's real + `age-keygen`/`sops` calls made earlier in the same command invocation. + + Raises :exc:`click.ClickException` if `nix` isn't on PATH or the lock + fails. + """ + if shutil.which("nix") is None: + raise click.ClickException("nix is not installed or not on PATH") + result = subprocess.run(["nix", "flake", "lock"], cwd=output_dir, check=False) # noqa: S607 + if result.returncode != 0: + raise click.ClickException(f"nix flake lock failed (exit {result.returncode})") + + @main.command("add-host") @click.argument("output_dir", type=click.Path(exists=True, file_okay=False, path_type=Path)) @click.option( @@ -257,31 +303,70 @@ def _validate_username(_ctx: click.Context, _param: click.Parameter, value: str) "--system", default="aarch64-darwin", show_default=True, - type=click.Choice(["aarch64-darwin", "x86_64-darwin"]), + type=click.Choice(_SYSTEM_CHOICES), help="Darwin system double.", ) -def add_host_cmd(output_dir: Path, hostname: str, username: str, system: str) -> None: +@click.option( + "--op-vault", + default=None, + help="Back up the new age key to this 1Password vault via `op` (verified by read-back) instead of a manual " + "confirmation. Falls back to the manual confirmation if `op` is unavailable, unauthenticated, or the " + "backup/verify fails.", +) +def add_host_cmd(output_dir: Path, hostname: str, username: str, system: str, op_vault: str | None) -> None: """Register a host with an existing mac2nix-scaffolded framework.""" - def _confirm_backup(fingerprint: str) -> bool: + def _confirm_backup(fingerprint: str, current_username: str, current_hostname: str) -> bool: click.echo(f"Public key fingerprint: {fingerprint}") - return ( - click.prompt("Type CONFIRMED once the private key has been backed up to a password manager", default="") - == "CONFIRMED" - ) - - try: - fingerprint = add_host(output_dir, hostname, username, system, confirm_backup=_confirm_backup) - except Exception as exc: - raise click.ClickException(str(exc)) from exc + if op_vault: + title = f"mac2nix age key — {current_hostname}/{current_username}" + try: + item_id = onepassword.store_age_key(age_key_path(current_username), vault=op_vault, title=title) + except onepassword.OnePasswordError as exc: + click.echo(f"1Password backup failed ({exc}) — falling back to manual confirmation.") + else: + click.echo(f"Backed up to 1Password vault {op_vault!r} (item {item_id}), verified by read-back.") + return True + while not click.confirm("Have you backed up the private key to a password manager?", default=False): + click.echo("This key cannot be recovered if lost — please back it up before continuing.") + return True + + def _register_one(current_hostname: str, current_username: str, current_system: str) -> str: + try: + fingerprint = add_host( + output_dir, + current_hostname, + current_username, + current_system, + confirm_backup=lambda fp: _confirm_backup(fp, current_username, current_hostname), + ) + except EOFError: + raise + except Exception as exc: + raise click.ClickException(str(exc)) from exc + + key_path = age_key_path(current_username) + click.echo(f"Host {current_hostname!r} registered (age key fingerprint: {fingerprint}).") + click.echo(f"Age key stored at {key_path} — make sure it's backed up somewhere safe.") + return fingerprint + + _register_one(hostname, username, system) + + while _confirm_or_default("Register another host now?", default=False): + next_hostname = click.prompt("Hostname", value_proc=_check_hostname) + next_username = click.prompt("Username", default=getpass.getuser(), value_proc=_check_username) + next_system = click.prompt("System", default=system, type=click.Choice(_SYSTEM_CHOICES), show_default=True) + _register_one(next_hostname, next_username, next_system) + + if _confirm_or_default("Run `nix flake lock` now?", default=False): + _run_nix_flake_lock(output_dir) + else: + click.echo(f"Next: run `nix flake lock` inside {output_dir} before the first build for this host.") - key_path = f"/Users/{username}/.config/sops/age/keys.txt" - click.echo(f"Host {hostname!r} registered (age key fingerprint: {fingerprint}).") - click.echo(f"Age key stored at {key_path} — make sure it's backed up somewhere safe.") - click.echo(f"Next: run `nix flake lock` inside {output_dir} before the first build for this host.") click.echo( - "Reminder: push this repo as a PRIVATE GitHub repo — scan-derived configuration isn't vetted " - "for public-repo exposure the way sops-encrypted secrets are." + "Reminder: push this repo as a PRIVATE GitHub repo — sops-nix keeps the actual secrets encrypted, " + "but each host's hostname/username are embedded in flake.nix/configuration.nix as plain, " + "unencrypted metadata." ) diff --git a/src/mac2nix/generators/scaffold.py b/src/mac2nix/generators/scaffold.py index 19de62d..f6ed41d 100644 --- a/src/mac2nix/generators/scaffold.py +++ b/src/mac2nix/generators/scaffold.py @@ -86,6 +86,17 @@ def _age_key_path(username: str, key_dir: Path | None = None) -> Path: return (key_dir or Path(f"/Users/{username}") / ".config" / "sops" / "age") / "keys.txt" +def age_key_path(username: str) -> Path: + """Public accessor for *username*'s real sops-nix age key path. + + Exists so callers outside this module (the CLI, `mac2nix.onepassword`) + share this exact construction instead of independently duplicating the + `/Users//.config/sops/age/keys.txt` literal and risking the + two silently drifting apart. + """ + return _age_key_path(username) + + def _write_host_config(host_dir: Path, hostname: str, username: str) -> None: host_dir.mkdir(parents=True) template = _read_template("hosts", "darwin", "configuration.nix") diff --git a/src/mac2nix/onepassword.py b/src/mac2nix/onepassword.py new file mode 100644 index 0000000..c68da23 --- /dev/null +++ b/src/mac2nix/onepassword.py @@ -0,0 +1,78 @@ +"""Optional 1Password CLI (`op`) integration for backing up sops-nix age keys. + +Used by `mac2nix add-host --op-vault` as an alternative to the manual +"have you backed this up?" confirmation. A write that's never read back can +silently fail or land wrong (locked vault, stale item, truncated upload) — +`store_age_key()` always verifies by reading the item back and comparing +its content byte-for-byte against the key file on disk before reporting +success, so a caller never has to trust the write alone. +""" + +from __future__ import annotations + +import json +import shutil +import subprocess +from pathlib import Path + + +class OnePasswordError(Exception): + """Raised when `op` is unavailable, unauthenticated, or a write/verify fails.""" + + +def is_available() -> bool: + return shutil.which("op") is not None + + +def is_signed_in() -> bool: + """Check sign-in state without ever triggering an interactive prompt. + + `op signin` blocks on user interaction if not already authenticated; + `op whoami` never does — it just fails fast with a nonzero exit. + """ + result = subprocess.run(["op", "whoami"], capture_output=True, text=True, check=False) # noqa: S607 + return result.returncode == 0 + + +def store_age_key(key_path: Path, *, vault: str, title: str) -> str: + """Upload *key_path* to 1Password as a Document item, then read it back to verify. + + Returns the created item's ID on success. Raises :exc:`OnePasswordError` + if `op` is missing, not signed in, the write fails, or the read-back + content doesn't match the on-disk key byte-for-byte — the last case is + the entire reason this function exists instead of a bare `op document + create` call: a write that "succeeds" but silently stores the wrong + (or no) content is worse than no backup, since it looks safe. + """ + if not is_available(): + raise OnePasswordError("`op` is not installed or not on PATH") + if not is_signed_in(): + raise OnePasswordError("`op` is not signed in — run `op signin` first") + + create = subprocess.run( # noqa: S603 + ["op", "document", "create", str(key_path), "--title", title, "--vault", vault, "--format", "json"], # noqa: S607 + capture_output=True, + text=True, + check=False, + ) + if create.returncode != 0: + raise OnePasswordError(f"op document create failed: {create.stderr.strip()}") + + try: + item_id = json.loads(create.stdout)["id"] + except (json.JSONDecodeError, KeyError) as exc: + raise OnePasswordError(f"op document create returned unexpected output: {create.stdout.strip()}") from exc + + verify = subprocess.run( # noqa: S603 + ["op", "document", "get", item_id, "--vault", vault], # noqa: S607 + capture_output=True, + check=False, + ) + if verify.returncode != 0: + stderr = verify.stderr.decode(errors="replace").strip() + raise OnePasswordError(f"op document get (verify) failed: {stderr}") + + if verify.stdout != key_path.read_bytes(): + raise OnePasswordError("1Password read-back did not match the on-disk age key — treating backup as failed") + + return item_id diff --git a/tests/cli/test_add_host.py b/tests/cli/test_add_host.py index 3b776d4..ad17ec1 100644 --- a/tests/cli/test_add_host.py +++ b/tests/cli/test_add_host.py @@ -5,9 +5,11 @@ from pathlib import Path from unittest.mock import patch +import click import pytest from click.testing import CliRunner +from mac2nix import onepassword from mac2nix.cli import main from mac2nix.generators.scaffold import init_framework from tests._scaffold_helpers import _has_add_host_crypto_deps, _redirect_age_keys @@ -34,7 +36,7 @@ def test_succeeds_end_to_end_with_confirmed_input(self, tmp_path: Path) -> None: result = runner.invoke( main, ["add-host", str(output_dir), "--hostname", "myhost", "--username", "alice"], - input="CONFIRMED\n", + input="y\nn\nn\n", ) assert result.exit_code == 0, result.output @@ -45,7 +47,8 @@ def test_succeeds_end_to_end_with_confirmed_input(self, tmp_path: Path) -> None: assert "myhost" in (output_dir / "flake.nix").read_text() assert "myhost" in (output_dir / ".sops.yaml").read_text() - def test_declining_confirmation_aborts_with_no_files_left(self, tmp_path: Path) -> None: + def test_declining_confirmation_reprompts_then_aborts_on_eof(self, tmp_path: Path) -> None: + """Answering "n" must reprompt (not abort outright) — only exhausting stdin aborts.""" output_dir = tmp_path / "repo" init_framework(output_dir) @@ -54,15 +57,33 @@ def test_declining_confirmation_aborts_with_no_files_left(self, tmp_path: Path) result = runner.invoke( main, ["add-host", str(output_dir), "--hostname", "myhost", "--username", "alice"], - input="nope\n", + input="n\n", ) assert result.exit_code != 0 + assert "please back it up before continuing" in result.output assert not (output_dir / "hosts" / "darwin" / "myhost").exists() assert not (output_dir / "users" / "alice.nix").exists() assert not (output_dir / "secrets" / "myhost.yaml").exists() assert "myhost" not in (output_dir / "flake.nix").read_text() + def test_declining_confirmation_reprompts_until_confirmed(self, tmp_path: Path) -> None: + """Two "n" answers must not abort — only when the caller eventually answers "y".""" + output_dir = tmp_path / "repo" + init_framework(output_dir) + + runner = CliRunner() + with _redirect_age_keys(tmp_path / "age-keys"): + result = runner.invoke( + main, + ["add-host", str(output_dir), "--hostname", "myhost", "--username", "alice"], + input="n\nn\ny\nn\nn\n", + ) + + assert result.exit_code == 0, result.output + assert result.output.count("please back it up before continuing") == 2 + assert (output_dir / "hosts" / "darwin" / "myhost").exists() + def test_rerun_against_same_hostname_fails_cleanly(self, tmp_path: Path) -> None: output_dir = tmp_path / "repo" init_framework(output_dir) @@ -72,14 +93,14 @@ def test_rerun_against_same_hostname_fails_cleanly(self, tmp_path: Path) -> None first = runner.invoke( main, ["add-host", str(output_dir), "--hostname", "myhost", "--username", "alice"], - input="CONFIRMED\n", + input="y\nn\nn\n", ) assert first.exit_code == 0, first.output second = runner.invoke( main, ["add-host", str(output_dir), "--hostname", "myhost", "--username", "alice"], - input="CONFIRMED\n", + input="y\nn\nn\n", ) assert second.exit_code != 0 @@ -93,12 +114,132 @@ def test_non_init_directory_fails_cleanly(self, tmp_path: Path) -> None: result = runner.invoke( main, ["add-host", str(output_dir), "--hostname", "myhost", "--username", "alice"], - input="CONFIRMED\n", + input="y\nn\nn\n", ) assert result.exit_code != 0 assert "mac2nix" in result.output.lower() or "framework" in result.output.lower() + def test_register_another_host_loop_registers_second_host(self, tmp_path: Path) -> None: + output_dir = tmp_path / "repo" + init_framework(output_dir) + + runner = CliRunner() + with _redirect_age_keys(tmp_path / "age-keys"): + result = runner.invoke( + main, + ["add-host", str(output_dir), "--hostname", "myhost", "--username", "alice"], + input="y\ny\nmyhost2\nbob\naarch64-darwin\ny\nn\nn\n", + ) + + assert result.exit_code == 0, result.output + assert (output_dir / "hosts" / "darwin" / "myhost").exists() + assert (output_dir / "hosts" / "darwin" / "myhost2").exists() + assert (output_dir / "users" / "bob.nix").exists() + assert "myhost2" in (output_dir / "flake.nix").read_text() + + def test_register_another_host_rejects_invalid_hostname_then_reprompts(self, tmp_path: Path) -> None: + output_dir = tmp_path / "repo" + init_framework(output_dir) + + runner = CliRunner() + with _redirect_age_keys(tmp_path / "age-keys"): + result = runner.invoke( + main, + ["add-host", str(output_dir), "--hostname", "myhost", "--username", "alice"], + input="y\ny\n../evil\nmyhost2\nbob\naarch64-darwin\ny\nn\nn\n", + ) + + assert result.exit_code == 0, result.output + assert (output_dir / "hosts" / "darwin" / "myhost2").exists() + + def test_op_vault_success_skips_manual_confirmation(self, tmp_path: Path) -> None: + output_dir = tmp_path / "repo" + init_framework(output_dir) + + runner = CliRunner() + with ( + _redirect_age_keys(tmp_path / "age-keys"), + patch("mac2nix.cli.onepassword.store_age_key", return_value="item123") as mock_store, + ): + result = runner.invoke( + main, + ["add-host", str(output_dir), "--hostname", "myhost", "--username", "alice", "--op-vault", "Private"], + input="n\nn\n", + ) + + assert result.exit_code == 0, result.output + assert "Backed up to 1Password vault 'Private' (item item123)" in result.output + assert "Have you backed up the private key" not in result.output + mock_store.assert_called_once() + kwargs = mock_store.call_args.kwargs + assert kwargs["vault"] == "Private" + assert "myhost" in kwargs["title"] + assert "alice" in kwargs["title"] + assert (output_dir / "hosts" / "darwin" / "myhost").exists() + + def test_op_vault_failure_falls_back_to_manual_confirmation(self, tmp_path: Path) -> None: + output_dir = tmp_path / "repo" + init_framework(output_dir) + + runner = CliRunner() + with ( + _redirect_age_keys(tmp_path / "age-keys"), + patch( + "mac2nix.cli.onepassword.store_age_key", + side_effect=onepassword.OnePasswordError("not signed in"), + ), + ): + result = runner.invoke( + main, + ["add-host", str(output_dir), "--hostname", "myhost", "--username", "alice", "--op-vault", "Private"], + input="y\nn\nn\n", + ) + + assert result.exit_code == 0, result.output + assert "1Password backup failed (not signed in)" in result.output + assert "Have you backed up the private key" in result.output + assert (output_dir / "hosts" / "darwin" / "myhost").exists() + + def test_flake_lock_prompt_invokes_nix_when_confirmed(self, tmp_path: Path) -> None: + output_dir = tmp_path / "repo" + init_framework(output_dir) + + runner = CliRunner() + with ( + _redirect_age_keys(tmp_path / "age-keys"), + patch("mac2nix.cli._run_nix_flake_lock") as mock_lock, + ): + result = runner.invoke( + main, + ["add-host", str(output_dir), "--hostname", "myhost", "--username", "alice"], + input="y\nn\ny\n", + ) + + assert result.exit_code == 0, result.output + mock_lock.assert_called_once_with(output_dir) + + def test_flake_lock_prompt_reports_failure(self, tmp_path: Path) -> None: + output_dir = tmp_path / "repo" + init_framework(output_dir) + + runner = CliRunner() + with ( + _redirect_age_keys(tmp_path / "age-keys"), + patch( + "mac2nix.cli._run_nix_flake_lock", + side_effect=click.ClickException("nix flake lock failed (exit 1)"), + ), + ): + result = runner.invoke( + main, + ["add-host", str(output_dir), "--hostname", "myhost", "--username", "alice"], + input="y\nn\ny\n", + ) + + assert result.exit_code != 0 + assert "nix flake lock failed" in result.output + class TestAddHostValidation: """--hostname/--username character-allowlist rejection — no crypto tools required.""" diff --git a/tests/test_onepassword.py b/tests/test_onepassword.py new file mode 100644 index 0000000..436e1f8 --- /dev/null +++ b/tests/test_onepassword.py @@ -0,0 +1,141 @@ +"""Tests for the mac2nix.onepassword module — all `op` calls mocked, no real CLI needed.""" + +from __future__ import annotations + +import json +from pathlib import Path +from unittest.mock import patch + +import pytest + +from mac2nix import onepassword + + +class TestAvailability: + def test_is_available_true_when_on_path(self) -> None: + with patch("mac2nix.onepassword.shutil.which", return_value="/usr/local/bin/op"): + assert onepassword.is_available() is True + + def test_is_available_false_when_missing(self) -> None: + with patch("mac2nix.onepassword.shutil.which", return_value=None): + assert onepassword.is_available() is False + + def test_is_signed_in_true_on_zero_exit(self) -> None: + with patch("mac2nix.onepassword.subprocess.run") as mock_run: + mock_run.return_value.returncode = 0 + assert onepassword.is_signed_in() is True + + def test_is_signed_in_false_on_nonzero_exit(self) -> None: + with patch("mac2nix.onepassword.subprocess.run") as mock_run: + mock_run.return_value.returncode = 1 + assert onepassword.is_signed_in() is False + + +class TestStoreAgeKey: + def test_raises_when_op_not_available(self, tmp_path: Path) -> None: + key_path = tmp_path / "keys.txt" + key_path.write_text("age-secret-key") + + with ( + patch("mac2nix.onepassword.is_available", return_value=False), + pytest.raises(onepassword.OnePasswordError, match="not installed"), + ): + onepassword.store_age_key(key_path, vault="Private", title="t") + + def test_raises_when_not_signed_in(self, tmp_path: Path) -> None: + key_path = tmp_path / "keys.txt" + key_path.write_text("age-secret-key") + + with ( + patch("mac2nix.onepassword.is_available", return_value=True), + patch("mac2nix.onepassword.is_signed_in", return_value=False), + pytest.raises(onepassword.OnePasswordError, match="not signed in"), + ): + onepassword.store_age_key(key_path, vault="Private", title="t") + + def test_raises_when_create_fails(self, tmp_path: Path) -> None: + key_path = tmp_path / "keys.txt" + key_path.write_text("age-secret-key") + + with ( + patch("mac2nix.onepassword.is_available", return_value=True), + patch("mac2nix.onepassword.is_signed_in", return_value=True), + patch("mac2nix.onepassword.subprocess.run") as mock_run, + ): + mock_run.return_value.returncode = 1 + mock_run.return_value.stderr = "vault is locked" + with pytest.raises(onepassword.OnePasswordError, match="vault is locked"): + onepassword.store_age_key(key_path, vault="Private", title="t") + + def test_raises_when_create_output_is_not_json(self, tmp_path: Path) -> None: + key_path = tmp_path / "keys.txt" + key_path.write_text("age-secret-key") + + with ( + patch("mac2nix.onepassword.is_available", return_value=True), + patch("mac2nix.onepassword.is_signed_in", return_value=True), + patch("mac2nix.onepassword.subprocess.run") as mock_run, + ): + mock_run.return_value.returncode = 0 + mock_run.return_value.stdout = "not json" + with pytest.raises(onepassword.OnePasswordError, match="unexpected output"): + onepassword.store_age_key(key_path, vault="Private", title="t") + + def test_raises_when_verify_read_fails(self, tmp_path: Path) -> None: + key_path = tmp_path / "keys.txt" + key_path.write_text("age-secret-key") + + create_result = type("R", (), {"returncode": 0, "stdout": json.dumps({"id": "item123"})})() + verify_result = type("R", (), {"returncode": 1, "stderr": b"item not found"})() + + with ( + patch("mac2nix.onepassword.is_available", return_value=True), + patch("mac2nix.onepassword.is_signed_in", return_value=True), + patch("mac2nix.onepassword.subprocess.run", side_effect=[create_result, verify_result]), + pytest.raises(onepassword.OnePasswordError, match="item not found"), + ): + onepassword.store_age_key(key_path, vault="Private", title="t") + + def test_raises_when_readback_content_mismatches(self, tmp_path: Path) -> None: + key_path = tmp_path / "keys.txt" + key_path.write_text("age-secret-key") + + create_result = type("R", (), {"returncode": 0, "stdout": json.dumps({"id": "item123"})})() + verify_result = type("R", (), {"returncode": 0, "stdout": b"wrong-content"})() + + with ( + patch("mac2nix.onepassword.is_available", return_value=True), + patch("mac2nix.onepassword.is_signed_in", return_value=True), + patch("mac2nix.onepassword.subprocess.run", side_effect=[create_result, verify_result]), + pytest.raises(onepassword.OnePasswordError, match="did not match"), + ): + onepassword.store_age_key(key_path, vault="Private", title="t") + + def test_succeeds_when_readback_matches(self, tmp_path: Path) -> None: + key_path = tmp_path / "keys.txt" + key_path.write_text("age-secret-key") + + create_result = type("R", (), {"returncode": 0, "stdout": json.dumps({"id": "item123"})})() + verify_result = type("R", (), {"returncode": 0, "stdout": key_path.read_bytes()})() + + with ( + patch("mac2nix.onepassword.is_available", return_value=True), + patch("mac2nix.onepassword.is_signed_in", return_value=True), + patch("mac2nix.onepassword.subprocess.run", side_effect=[create_result, verify_result]) as mock_run, + ): + item_id = onepassword.store_age_key(key_path, vault="Private", title="my title") + + assert item_id == "item123" + create_call = mock_run.call_args_list[0] + assert create_call.args[0] == [ + "op", + "document", + "create", + str(key_path), + "--title", + "my title", + "--vault", + "Private", + "--format", + "json", + ] From 81a4e13da957cceaa5f9d1f7e0674573d24202db Mon Sep 17 00:00:00 2001 From: Will Gordon Date: Thu, 6 Aug 2026 10:04:06 -0400 Subject: [PATCH 13/33] fix(vm): points guest DNS at a public resolver, bypassing tart gateway MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Root-causes the "known limitation" flagged in PR1's UAT comment (attributed there to a local dnsmasq/Tart networking conflict). Live reproduction shows the actual cause is unrelated to dnsmasq: Tart's own vmnet-provided gateway can fail to answer DNS queries from the guest outright (a connection failure, not a slow upstream), while the same guest resolves instantly via a direct public resolver. Verified against the exact originally-failing command (downloading the Nix installer) — it now succeeds. TartVMManager.start() calls a new _ensure_dns_resolves() right after wait_ready() confirms SSH is up, running `networksetup -setdnsservers Ethernet 1.1.1.1` inside the guest before any caller can run a real network operation there. Every VM this project boots only ever needs public-hostname resolution (nixpkgs, github, flakehub, the Nix installer), never anything unique to Tart's own DHCP-provided resolver, so this is safe unconditionally. --- src/mac2nix/vm/manager.py | 28 ++++++++++++++++++ tests/vm/test_manager.py | 61 +++++++++++++++++++++++++++++++++++++++ 2 files changed, 89 insertions(+) diff --git a/src/mac2nix/vm/manager.py b/src/mac2nix/vm/manager.py index 6b4b1cf..9a0a13b 100644 --- a/src/mac2nix/vm/manager.py +++ b/src/mac2nix/vm/manager.py @@ -184,6 +184,34 @@ async def start(self) -> None: ) logger.debug("VM %r process started (pid=%d)", clone, self._vm_process.pid) await self.wait_ready() + await self._ensure_dns_resolves() + + async def _ensure_dns_resolves(self) -> None: + """Point the guest's DNS at a public resolver, bypassing Tart's own gateway. + + Verified empirically against a real VM: Tart's vmnet-provided gateway + (the DHCP-assigned nameserver, e.g. 192.168.64.1) can fail to answer + DNS queries from the guest at all — not slow, an outright connection + failure — while the exact same guest resolves instantly via a direct + public resolver. Every real network operation this project runs + inside a VM (downloading the Nix installer, fetching flake inputs) + needs public-hostname resolution, never anything the gateway's own + DHCP-provided resolver would uniquely know, so unconditionally + overriding it here is safe for this project's actual VM usage. + + "Ethernet" is Tart's macOS base images' one consistent network + service name (a single virtio-net interface) — not dynamically + discovered, since every base image this project targets uses it. + Raises :exc:`VMError` if the override itself fails to apply, since a + broken guest resolver would otherwise surface later as a much more + confusing "could not resolve host" failure from whatever real + command runs next. + """ + success, _out, err = await self.exec_command( + ["sudo", "networksetup", "-setdnsservers", "Ethernet", "1.1.1.1"], timeout=15 + ) + if not success: + raise VMError(f"Failed to configure guest DNS resolver: {err.strip()}") async def wait_ready(self, max_attempts: int = 10) -> None: """Poll until the VM has an IP and accepts SSH connections. diff --git a/tests/vm/test_manager.py b/tests/vm/test_manager.py index 709df5b..5f43ac6 100644 --- a/tests/vm/test_manager.py +++ b/tests/vm/test_manager.py @@ -218,6 +218,7 @@ async def _run() -> None: new=AsyncMock(return_value=bg_proc), ), patch.object(mgr, "wait_ready", new=AsyncMock()), + patch.object(mgr, "_ensure_dns_resolves", new=AsyncMock()), ): await mgr.start() @@ -239,6 +240,7 @@ async def _run() -> None: patch("mac2nix.vm.manager.shutil.which", return_value="/usr/local/bin/tart"), patch("mac2nix.vm.manager.asyncio.create_subprocess_exec", side_effect=recording_exec), patch.object(mgr, "wait_ready", new=AsyncMock()), + patch.object(mgr, "_ensure_dns_resolves", new=AsyncMock()), ): await mgr.start() @@ -267,6 +269,7 @@ async def mock_wait_ready(**_kw) -> None: new=AsyncMock(return_value=bg_proc), ), patch.object(mgr, "wait_ready", side_effect=mock_wait_ready), + patch.object(mgr, "_ensure_dns_resolves", new=AsyncMock()), ): await mgr.start() @@ -291,6 +294,64 @@ async def _run() -> None: with pytest.raises(VMError, match="tart CLI is not available"): asyncio.run(_run()) + def test_calls_ensure_dns_resolves_after_wait_ready(self) -> None: + bg_proc = _make_bg_proc() + call_order: list[str] = [] + + async def _run() -> None: + mgr = _cloned_manager("test-vm") + + async def mock_wait_ready(**_kw) -> None: + call_order.append("wait_ready") + + async def mock_ensure_dns() -> None: + call_order.append("ensure_dns") + + with ( + patch("mac2nix.vm.manager.shutil.which", return_value="/usr/local/bin/tart"), + patch( + "mac2nix.vm.manager.asyncio.create_subprocess_exec", + new=AsyncMock(return_value=bg_proc), + ), + patch.object(mgr, "wait_ready", side_effect=mock_wait_ready), + patch.object(mgr, "_ensure_dns_resolves", side_effect=mock_ensure_dns), + ): + await mgr.start() + + asyncio.run(_run()) + assert call_order == ["wait_ready", "ensure_dns"] + + +# --------------------------------------------------------------------------- +# _ensure_dns_resolves() +# --------------------------------------------------------------------------- + + +class TestEnsureDnsResolves: + def test_success_sets_guest_dns_via_networksetup(self) -> None: + captured_cmd: list[str] = [] + + async def fake_exec_command(cmd: list[str], **_kw: object) -> tuple[bool, str, str]: + captured_cmd.extend(cmd) + return True, "", "" + + async def _run() -> None: + mgr = _cloned_manager("dns-vm") + with patch.object(mgr, "exec_command", side_effect=fake_exec_command): + await mgr._ensure_dns_resolves() + + asyncio.run(_run()) + assert captured_cmd == ["sudo", "networksetup", "-setdnsservers", "Ethernet", "1.1.1.1"] + + def test_failure_raises_vm_error(self) -> None: + async def _run() -> None: + mgr = _cloned_manager("dns-vm") + with patch.object(mgr, "exec_command", new=AsyncMock(return_value=(False, "", "no such service"))): + await mgr._ensure_dns_resolves() + + with pytest.raises(VMError, match="Failed to configure guest DNS resolver"): + asyncio.run(_run()) + # --------------------------------------------------------------------------- # wait_ready() From 8f01df315e6f64bb4570b6b9912165a6607a9f0f Mon Sep 17 00:00:00 2001 From: Will Gordon Date: Thu, 6 Aug 2026 10:07:40 -0400 Subject: [PATCH 14/33] fix(vm): forces password-only ssh auth to avoid MaxAuthTries exhaustion Reproduced against a real VM while re-verifying the DNS fix: without PreferredAuthentications=password + PubkeyAuthentication=no, ssh tries every other auth method first, including whatever default identity files happen to exist in the calling machine's ~/.ssh/, before ever offering the password sshpass supplies. Each failed attempt counts against the server's MaxAuthTries, so a machine with enough default keys present can exhaust that limit ("Too many authentication failures") before password auth is ever tried. Applies to all three ssh/scp invocations (async_ssh_exec, and validator.py's two direct scp commands), since scp negotiates auth the same way ssh does. --- src/mac2nix/vm/_utils.py | 14 ++++++++++++++ src/mac2nix/vm/validator.py | 13 +++++++++++++ tests/vm/test_vm_utils.py | 4 ++++ 3 files changed, 31 insertions(+) diff --git a/src/mac2nix/vm/_utils.py b/src/mac2nix/vm/_utils.py index 69f6e6d..b96a445 100644 --- a/src/mac2nix/vm/_utils.py +++ b/src/mac2nix/vm/_utils.py @@ -146,6 +146,16 @@ async def async_ssh_exec( # StrictHostKeyChecking=no and UserKnownHostsFile=/dev/null: safe because # target VMs are ephemeral Tart clones on localhost — no persistent host # identity to verify, and the IP/key changes on every clone. + # + # PreferredAuthentications=password + PubkeyAuthentication=no: without + # these, ssh tries every other auth method first — including whatever + # default identity files happen to exist in the calling machine's + # ~/.ssh/ — before ever offering the password sshpass supplies. Each + # failed attempt counts against the *server's* MaxAuthTries, so on a + # machine with enough default keys present, ssh can exhaust that limit + # ("Too many authentication failures") before password auth is ever + # tried — reproduced empirically. Forcing password-only makes this + # deterministic regardless of what any given contributor's machine has. ssh_cmd = [ "sshpass", "-e", @@ -157,6 +167,10 @@ async def async_ssh_exec( "-o", "LogLevel=ERROR", "-o", + "PreferredAuthentications=password", + "-o", + "PubkeyAuthentication=no", + "-o", f"ConnectTimeout={max(timeout // 2, 5)}", f"{user}@{ip}", "--", diff --git a/src/mac2nix/vm/validator.py b/src/mac2nix/vm/validator.py index 7fbc030..c192a82 100644 --- a/src/mac2nix/vm/validator.py +++ b/src/mac2nix/vm/validator.py @@ -277,6 +277,9 @@ async def _copy_flake_to_vm( # scp -r user@ip: — uses sshpass -e for password auth. # Password passed via SSHPASS env var to avoid exposure in ps aux. + # PreferredAuthentications/PubkeyAuthentication: see async_ssh_exec()'s + # own comment in _utils.py — same "Too many authentication failures" + # footgun applies here since scp shares ssh's auth negotiation. scp_cmd = [ "sshpass", "-e", @@ -287,6 +290,10 @@ async def _copy_flake_to_vm( "UserKnownHostsFile=/dev/null", "-o", "LogLevel=ERROR", + "-o", + "PreferredAuthentications=password", + "-o", + "PubkeyAuthentication=no", "-r", *sources, f"{self._vm.vm_user}@{ip}:{remote_dir}", @@ -416,6 +423,8 @@ async def _scan_vm(self) -> SystemState: local_path = Path(tmp.name) try: + # PreferredAuthentications/PubkeyAuthentication: see + # async_ssh_exec()'s comment in _utils.py. scp_cmd = [ "sshpass", "-e", @@ -426,6 +435,10 @@ async def _scan_vm(self) -> SystemState: "UserKnownHostsFile=/dev/null", "-o", "LogLevel=ERROR", + "-o", + "PreferredAuthentications=password", + "-o", + "PubkeyAuthentication=no", f"{self._vm.vm_user}@{ip}:{self._REMOTE_SCAN_PATH}", str(local_path), ] diff --git a/tests/vm/test_vm_utils.py b/tests/vm/test_vm_utils.py index f5412c7..40e31c2 100644 --- a/tests/vm/test_vm_utils.py +++ b/tests/vm/test_vm_utils.py @@ -343,6 +343,10 @@ async def _run() -> None: "-o", "LogLevel=ERROR", "-o", + "PreferredAuthentications=password", + "-o", + "PubkeyAuthentication=no", + "-o", "ConnectTimeout=15", # max(30 // 2, 5) — SSH timeout is half the process timeout "admin@192.168.64.10", "--", From be9c36abe3227a638128e8c76d22ed819dd217c5 Mon Sep 17 00:00:00 2001 From: Will Gordon Date: Thu, 6 Aug 2026 10:23:52 -0400 Subject: [PATCH 15/33] fix(vm): sudos the real switch and retries transient DNS-setup failures MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two more real bugs surfaced only once the DNS and SSH-auth fixes let a real VM run get far enough to hit them. test_scaffold_vm.py's plain `nix run nix-darwin -- switch` failed with "system activation must now be run as root" — sudos it (matching the same pattern already used in CI's native-switch test), resolving nix's absolute path via `command -v` in the profile-sourced shell first since sudo's own secure_path won't include wherever the nix-daemon profile put it on PATH. TartVMManager._ensure_dns_resolves() also got "Permission denied" once, moments after wait_ready()'s own SSH check had just succeeded with the same credentials on a freshly-booted VM. wait_ready() already tolerates exactly this class of boot-timing flakiness via retries; a single-shot DNS-setup call right after it was throwing that resilience away for one more mandatory SSH round-trip. Now retries the same way. --- src/mac2nix/vm/manager.py | 34 ++++++++++++++++++++++++---------- tests/vm/test_manager.py | 27 +++++++++++++++++++++++++-- tests/vm/test_scaffold_vm.py | 10 +++++++++- 3 files changed, 58 insertions(+), 13 deletions(-) diff --git a/src/mac2nix/vm/manager.py b/src/mac2nix/vm/manager.py index 9a0a13b..2e1fa11 100644 --- a/src/mac2nix/vm/manager.py +++ b/src/mac2nix/vm/manager.py @@ -186,7 +186,7 @@ async def start(self) -> None: await self.wait_ready() await self._ensure_dns_resolves() - async def _ensure_dns_resolves(self) -> None: + async def _ensure_dns_resolves(self, max_attempts: int = 3) -> None: """Point the guest's DNS at a public resolver, bypassing Tart's own gateway. Verified empirically against a real VM: Tart's vmnet-provided gateway @@ -202,16 +202,30 @@ async def _ensure_dns_resolves(self) -> None: "Ethernet" is Tart's macOS base images' one consistent network service name (a single virtio-net interface) — not dynamically discovered, since every base image this project targets uses it. - Raises :exc:`VMError` if the override itself fails to apply, since a - broken guest resolver would otherwise surface later as a much more - confusing "could not resolve host" failure from whatever real - command runs next. + + Retries on failure like :meth:`wait_ready` does for the same reason: + reproduced empirically, a VM can briefly reject the very same SSH + credentials moments after `wait_ready()`'s own check just succeeded + with them (macOS first-boot account/password setup finishing shortly + after sshd starts accepting connections) — a single-shot call here + would throw away `wait_ready()`'s own tolerance for that exact class + of boot-timing flakiness. Raises :exc:`VMError` if every attempt + fails, since a broken guest resolver would otherwise surface later as + a much more confusing "could not resolve host" failure from whatever + real command runs next. """ - success, _out, err = await self.exec_command( - ["sudo", "networksetup", "-setdnsservers", "Ethernet", "1.1.1.1"], timeout=15 - ) - if not success: - raise VMError(f"Failed to configure guest DNS resolver: {err.strip()}") + err = "" + for attempt in range(max_attempts): + success, _out, err = await self.exec_command( + ["sudo", "networksetup", "-setdnsservers", "Ethernet", "1.1.1.1"], timeout=15 + ) + if success: + return + logger.debug("DNS configuration attempt %d/%d failed: %s", attempt + 1, max_attempts, err.strip()) + if attempt < max_attempts - 1: + await asyncio.sleep(5) + + raise VMError(f"Failed to configure guest DNS resolver: {err.strip()}") async def wait_ready(self, max_attempts: int = 10) -> None: """Poll until the VM has an IP and accepts SSH connections. diff --git a/tests/vm/test_manager.py b/tests/vm/test_manager.py index 5f43ac6..16ce9d9 100644 --- a/tests/vm/test_manager.py +++ b/tests/vm/test_manager.py @@ -343,15 +343,38 @@ async def _run() -> None: asyncio.run(_run()) assert captured_cmd == ["sudo", "networksetup", "-setdnsservers", "Ethernet", "1.1.1.1"] - def test_failure_raises_vm_error(self) -> None: + def test_failure_raises_vm_error_after_exhausting_retries(self) -> None: async def _run() -> None: mgr = _cloned_manager("dns-vm") - with patch.object(mgr, "exec_command", new=AsyncMock(return_value=(False, "", "no such service"))): + with ( + patch.object(mgr, "exec_command", new=AsyncMock(return_value=(False, "", "no such service"))), + patch("mac2nix.vm.manager.asyncio.sleep", new=AsyncMock()), + ): await mgr._ensure_dns_resolves() with pytest.raises(VMError, match="Failed to configure guest DNS resolver"): asyncio.run(_run()) + def test_retries_and_succeeds_after_transient_failure(self) -> None: + attempts: list[int] = [] + + async def flaky_exec_command(cmd: list[str], **_kw: object) -> tuple[bool, str, str]: + attempts.append(1) + if len(attempts) < 2: + return False, "", "Permission denied, please try again." + return True, "", "" + + async def _run() -> None: + mgr = _cloned_manager("dns-vm") + with ( + patch.object(mgr, "exec_command", side_effect=flaky_exec_command), + patch("mac2nix.vm.manager.asyncio.sleep", new=AsyncMock()), + ): + await mgr._ensure_dns_resolves() + + asyncio.run(_run()) + assert len(attempts) == 2 + # --------------------------------------------------------------------------- # wait_ready() diff --git a/tests/vm/test_scaffold_vm.py b/tests/vm/test_scaffold_vm.py index 09e3010..e673bc6 100644 --- a/tests/vm/test_scaffold_vm.py +++ b/tests/vm/test_scaffold_vm.py @@ -110,10 +110,18 @@ async def _run() -> tuple[bool, str, str]: await _copy_age_key_to_vm(nix_darwin_vm, local_key_path, _VM_USERNAME) await validator._bootstrap_nix_darwin() + # nix-darwin's system activation now always runs as root (per + # system.primaryUser's own purpose) — plain `nix run` fails with + # "system activation must now be run as root". `sudo -n` fails fast + # rather than hanging on a password prompt if passwordless sudo isn't + # actually available. `$(command -v nix)` resolves nix's absolute + # path in the current (profile-sourced) shell *before* handing it to + # sudo, since sudo's own secure_path won't include wherever the + # nix-daemon profile put it on PATH. switch_cmd = ( f"cd {validator._REMOTE_FLAKE_DIR}" " && . /nix/var/nix/profiles/default/etc/profile.d/nix-daemon.sh" - f" && nix run nix-darwin -- switch --flake .#{_HOSTNAME}" + f" && sudo -n $(command -v nix) run nix-darwin -- switch --flake .#{_HOSTNAME}" ) return await nix_darwin_vm.exec_command(["bash", "-c", switch_cmd], timeout=900) From 0c6e8dbb2b2002867b06c00d68197d3d44e90ec8 Mon Sep 17 00:00:00 2001 From: Will Gordon Date: Thu, 6 Aug 2026 10:36:38 -0400 Subject: [PATCH 16/33] fix(vm): moves aside nix.custom.conf before the real vm switch test The real switch got much further this time (building the full 126-drv darwin system, not just darwin-rebuild itself) but hit the same /etc/nix/nix.custom.conf ownership conflict already fixed in CI's nix-darwin-switch job: nix-darwin refuses to overwrite an /etc file it doesn't manage and finds with unrecognized content, and the Determinate Nix installer run by _bootstrap_nix_darwin() writes exactly that file. test_scaffold_vm.py never had the CI job's equivalent workaround since it's a separate test predating that fix. Same move-aside, same reason. --- tests/vm/test_scaffold_vm.py | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/tests/vm/test_scaffold_vm.py b/tests/vm/test_scaffold_vm.py index e673bc6..c6563db 100644 --- a/tests/vm/test_scaffold_vm.py +++ b/tests/vm/test_scaffold_vm.py @@ -110,6 +110,21 @@ async def _run() -> tuple[bool, str, str]: await _copy_age_key_to_vm(nix_darwin_vm, local_key_path, _VM_USERNAME) await validator._bootstrap_nix_darwin() + # nix-darwin refuses to overwrite any /etc file it doesn't already + # manage and finds with unrecognized content — the Determinate Nix + # installer run by _bootstrap_nix_darwin() above writes its own + # /etc/nix/nix.custom.conf, which nix-darwin's own Nix management + # then wants to own. Same conflict, same fix as the CI + # nix-darwin-switch job's own step for this exact reason. + 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 nix_darwin_vm.exec_command(["bash", "-c", move_cmd]) + if not ok: + raise VMError(f"Failed to move aside /etc/nix/nix.custom.conf: {err.strip()}") + # nix-darwin's system activation now always runs as root (per # system.primaryUser's own purpose) — plain `nix run` fails with # "system activation must now be run as root". `sudo -n` fails fast From bbc7ee0795e622f95cfbccb75b1fe385be4937f4 Mon Sep 17 00:00:00 2001 From: Will Gordon Date: Thu, 6 Aug 2026 10:48:49 -0400 Subject: [PATCH 17/33] fix(vm): wipes pre-existing homebrew before the real vm switch test MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Got past the nix.custom.conf conflict and much further this time (networking/firewall/power/fonts/nvram all configured) but hit the exact same cascading Homebrew conflict already root-caused and fixed in CI's nix-darwin-switch job: macos-tahoe-base ships with a real, multi-tap Homebrew install that nix-homebrew's autoMigrate can't cleanly adopt (Library/Taps already exists, then a formula whose originating tap was just removed). Wiping it first lets nix-homebrew do a normal fresh install instead — nothing in this test needs Homebrew itself, Nix comes from _bootstrap_nix_darwin() independently. --- tests/vm/test_scaffold_vm.py | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/tests/vm/test_scaffold_vm.py b/tests/vm/test_scaffold_vm.py index c6563db..2a848b6 100644 --- a/tests/vm/test_scaffold_vm.py +++ b/tests/vm/test_scaffold_vm.py @@ -125,6 +125,18 @@ async def _run() -> tuple[bool, str, str]: if not ok: raise VMError(f"Failed to move aside /etc/nix/nix.custom.conf: {err.strip()}") + # macos-tahoe-base ships with a pre-existing Homebrew install. + # nix-homebrew's autoMigrate can't cleanly adopt one with real content + # across multiple taps — same cascading conflict already root-caused + # and fixed the same way in CI's nix-darwin-switch job (an existing + # Library/Taps directory, then a formula whose originating tap had + # just been removed). Wiping it first lets nix-homebrew do a normal + # fresh install instead. Nothing in this test needs Homebrew itself — + # Nix comes from _bootstrap_nix_darwin() above, independent of brew. + ok, _out, err = await nix_darwin_vm.exec_command(["sudo", "rm", "-rf", "/opt/homebrew"], timeout=60) + if not ok: + raise VMError(f"Failed to remove pre-existing Homebrew: {err.strip()}") + # nix-darwin's system activation now always runs as root (per # system.primaryUser's own purpose) — plain `nix run` fails with # "system activation must now be run as root". `sudo -n` fails fast From 4b241ff24e3fd310b28b8688f283501cdc84ff63 Mon Sep 17 00:00:00 2001 From: Will Gordon Date: Thu, 6 Aug 2026 10:53:00 -0400 Subject: [PATCH 18/33] fix(vm): requires 2 consecutive ssh successes before vm is ready --- src/mac2nix/vm/manager.py | 30 +++++++++++++++++++++++------- tests/vm/test_manager.py | 31 +++++++++++++++++++++++++++++-- 2 files changed, 52 insertions(+), 9 deletions(-) diff --git a/src/mac2nix/vm/manager.py b/src/mac2nix/vm/manager.py index 2e1fa11..603dd8f 100644 --- a/src/mac2nix/vm/manager.py +++ b/src/mac2nix/vm/manager.py @@ -228,10 +228,19 @@ async def _ensure_dns_resolves(self, max_attempts: int = 3) -> None: raise VMError(f"Failed to configure guest DNS resolver: {err.strip()}") async def wait_ready(self, max_attempts: int = 10) -> None: - """Poll until the VM has an IP and accepts SSH connections. - - Sleeps 5 seconds between attempts. Raises :exc:`VMTimeoutError` when - *max_attempts* is exhausted without a successful SSH handshake. + """Poll until the VM has an IP and accepts two consecutive SSH connections. + + Sleeps 5 seconds between attempts. Requires the SSH check to succeed + twice in a row (2s apart) before declaring the VM ready — reproduced + empirically, a single successful `whoami` isn't a reliable enough + signal: a freshly-booted VM's account/SSH state can still be settling + at that exact moment, and the very next SSH-dependent call from a + caller (a second `exec_command`, an `scp`) can spuriously get + "Permission denied" moments later even though the credentials are + correct and a subsequent retry would succeed. One confirmation check + closes that window for every caller, rather than needing its own + retry logic at each call site. Raises :exc:`VMTimeoutError` when + *max_attempts* is exhausted without two consecutive successes. """ clone = self._require_clone() logger.debug("Waiting for VM %r to be ready (%d attempts)", clone, max_attempts) @@ -251,9 +260,16 @@ async def wait_ready(self, max_attempts: int = 10) -> None: ip, self._vm_user, self._vm_password, ["whoami"], timeout=10 ) if success and self._vm_user in out: - logger.debug("VM %r is ready at %s", clone, ip) - return - logger.debug("SSH not yet ready for %r: %r", clone, _err.strip()) + await asyncio.sleep(2) + confirm_success, confirm_out, confirm_err = await async_ssh_exec( + ip, self._vm_user, self._vm_password, ["whoami"], timeout=10 + ) + if confirm_success and self._vm_user in confirm_out: + logger.debug("VM %r is ready at %s", clone, ip) + return + logger.debug("SSH readiness confirmation failed for %r: %r", clone, confirm_err.strip()) + else: + logger.debug("SSH not yet ready for %r: %r", clone, _err.strip()) except (VMConnectionError, VMError): logger.debug("SSH attempt %d failed for %r", attempt + 1, clone) else: diff --git a/tests/vm/test_manager.py b/tests/vm/test_manager.py index 16ce9d9..adc14a7 100644 --- a/tests/vm/test_manager.py +++ b/tests/vm/test_manager.py @@ -447,8 +447,8 @@ async def flaky_ssh(ip, user, pw, cmd, *, timeout): ): await mgr.wait_ready(max_attempts=5) - asyncio.run(_run()) # Should not raise; succeeds on attempt 3 - assert call_count == 3 + asyncio.run(_run()) # Should not raise; succeeds on attempt 3, then confirmed on a 4th call + assert call_count == 4 def test_sleeps_between_attempts(self) -> None: sleep_calls: list[float] = [] @@ -469,6 +469,33 @@ async def _run() -> None: # 3 attempts → 2 sleeps (no sleep after last attempt) assert len(sleep_calls) == 2 + def test_confirmation_failure_after_first_success_retries_not_returns(self) -> None: + """A spurious failure on the confirmation check must not be treated as ready.""" + call_count = 0 + + async def flaky_confirmation(ip, user, pw, cmd, *, timeout): + nonlocal call_count + call_count += 1 + # First check succeeds every attempt; only the *second* (confirmation) + # call in the whole test fails once, then everything succeeds. + if call_count == 2: + return (False, "", "Permission denied, please try again.") + return (True, user, "") + + async def _run() -> None: + mgr = _cloned_manager("confirm-vm") + with ( + patch.object(mgr, "get_ip", new=AsyncMock(return_value="10.0.0.1")), + patch("mac2nix.vm.manager.async_ssh_exec", side_effect=flaky_confirmation), + patch("mac2nix.vm.manager.asyncio.sleep", new=AsyncMock()), + ): + await mgr.wait_ready(max_attempts=5) + + asyncio.run(_run()) # Should not raise + # Attempt 1: first check succeeds (call 1), confirmation fails (call 2) → retry. + # Attempt 2: first check succeeds (call 3), confirmation succeeds (call 4) → ready. + assert call_count == 4 + def test_requires_clone(self) -> None: async def _run() -> None: mgr = _make_manager() From c14c5134800fb7ced650f0545f068c21f1d9ccf6 Mon Sep 17 00:00:00 2001 From: Will Gordon Date: Thu, 6 Aug 2026 14:15:28 -0400 Subject: [PATCH 19/33] fix(cli): stops pre-gating op backup on op whoami MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `op whoami` has a known failure mode where it reports "not signed in" while the local vault is still genuinely readable/writable (confirmed against a real machine this session — session-state can desync from what `op` actually has access to). Pre-checking is_signed_in() before ever attempting the real `op document create` meant a stale whoami result could force an unnecessary manual-confirmation fallback even though the backup would have succeeded. Attempts the write directly now and surfaces its own error message instead — a more reliable signal than a separate pre-flight check that can itself be wrong. is_signed_in() stays available as its own function, just no longer used to gate store_age_key(). --- src/mac2nix/onepassword.py | 21 ++++++++++++++------- tests/test_onepassword.py | 21 +++++---------------- 2 files changed, 19 insertions(+), 23 deletions(-) diff --git a/src/mac2nix/onepassword.py b/src/mac2nix/onepassword.py index c68da23..4a9def6 100644 --- a/src/mac2nix/onepassword.py +++ b/src/mac2nix/onepassword.py @@ -38,16 +38,23 @@ def store_age_key(key_path: Path, *, vault: str, title: str) -> str: """Upload *key_path* to 1Password as a Document item, then read it back to verify. Returns the created item's ID on success. Raises :exc:`OnePasswordError` - if `op` is missing, not signed in, the write fails, or the read-back - content doesn't match the on-disk key byte-for-byte — the last case is - the entire reason this function exists instead of a bare `op document - create` call: a write that "succeeds" but silently stores the wrong - (or no) content is worse than no backup, since it looks safe. + if `op` is missing, the write fails, or the read-back content doesn't + match the on-disk key byte-for-byte — the last case is the entire reason + this function exists instead of a bare `op document create` call: a + write that "succeeds" but silently stores the wrong (or no) content is + worse than no backup, since it looks safe. + + Deliberately does not pre-check `is_signed_in()` before attempting the + write — `op whoami` has a known failure mode where it reports "not + signed in" while the local vault is still genuinely readable/writable + (session-state can desync from what `op` actually has access to). + Attempting the real operation and surfacing its own error message is a + more reliable signal than a separate pre-flight check that can itself + be wrong; `is_signed_in()` is still available as its own function for + callers that want a best-effort availability check. """ if not is_available(): raise OnePasswordError("`op` is not installed or not on PATH") - if not is_signed_in(): - raise OnePasswordError("`op` is not signed in — run `op signin` first") create = subprocess.run( # noqa: S603 ["op", "document", "create", str(key_path), "--title", title, "--vault", vault, "--format", "json"], # noqa: S607 diff --git a/tests/test_onepassword.py b/tests/test_onepassword.py index 436e1f8..59e3135 100644 --- a/tests/test_onepassword.py +++ b/tests/test_onepassword.py @@ -42,24 +42,17 @@ def test_raises_when_op_not_available(self, tmp_path: Path) -> None: ): onepassword.store_age_key(key_path, vault="Private", title="t") - def test_raises_when_not_signed_in(self, tmp_path: Path) -> None: - key_path = tmp_path / "keys.txt" - key_path.write_text("age-secret-key") - - with ( - patch("mac2nix.onepassword.is_available", return_value=True), - patch("mac2nix.onepassword.is_signed_in", return_value=False), - pytest.raises(onepassword.OnePasswordError, match="not signed in"), - ): - onepassword.store_age_key(key_path, vault="Private", title="t") - def test_raises_when_create_fails(self, tmp_path: Path) -> None: + """Covers the not-signed-in case too, since store_age_key() no longer pre-checks + is_signed_in() — see its own docstring for why (op whoami can report "not signed + in" while the vault is still genuinely usable). The real `op document create` + call's own error message is the source of truth instead. + """ key_path = tmp_path / "keys.txt" key_path.write_text("age-secret-key") with ( patch("mac2nix.onepassword.is_available", return_value=True), - patch("mac2nix.onepassword.is_signed_in", return_value=True), patch("mac2nix.onepassword.subprocess.run") as mock_run, ): mock_run.return_value.returncode = 1 @@ -73,7 +66,6 @@ def test_raises_when_create_output_is_not_json(self, tmp_path: Path) -> None: with ( patch("mac2nix.onepassword.is_available", return_value=True), - patch("mac2nix.onepassword.is_signed_in", return_value=True), patch("mac2nix.onepassword.subprocess.run") as mock_run, ): mock_run.return_value.returncode = 0 @@ -90,7 +82,6 @@ def test_raises_when_verify_read_fails(self, tmp_path: Path) -> None: with ( patch("mac2nix.onepassword.is_available", return_value=True), - patch("mac2nix.onepassword.is_signed_in", return_value=True), patch("mac2nix.onepassword.subprocess.run", side_effect=[create_result, verify_result]), pytest.raises(onepassword.OnePasswordError, match="item not found"), ): @@ -105,7 +96,6 @@ def test_raises_when_readback_content_mismatches(self, tmp_path: Path) -> None: with ( patch("mac2nix.onepassword.is_available", return_value=True), - patch("mac2nix.onepassword.is_signed_in", return_value=True), patch("mac2nix.onepassword.subprocess.run", side_effect=[create_result, verify_result]), pytest.raises(onepassword.OnePasswordError, match="did not match"), ): @@ -120,7 +110,6 @@ def test_succeeds_when_readback_matches(self, tmp_path: Path) -> None: with ( patch("mac2nix.onepassword.is_available", return_value=True), - patch("mac2nix.onepassword.is_signed_in", return_value=True), patch("mac2nix.onepassword.subprocess.run", side_effect=[create_result, verify_result]) as mock_run, ): item_id = onepassword.store_age_key(key_path, vault="Private", title="my title") From cf392d09ca42caeee34ef5d3c80a843a40ff2562 Mon Sep 17 00:00:00 2001 From: Will Gordon Date: Fri, 7 Aug 2026 11:49:35 -0400 Subject: [PATCH 20/33] fix(cli): tightens add-host prompts per UAT review MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Drops "to a password manager" from the backup-confirmation prompt — unnecessarily presumptuous about where the key ends up. Removes the early "Public key fingerprint" echo since the final "Host registered (age key fingerprint: ...)" line already reports it once; printing it twice in the same short run was noise, not confirmation. Defaults the "Run `nix flake lock` now?" prompt to yes — locking isn't destructive and is the natural next step after registering a host, so a bare Enter should do it rather than silently skip it. --- src/mac2nix/cli.py | 7 +++---- tests/cli/test_add_host.py | 21 +++++++++++++++++++++ 2 files changed, 24 insertions(+), 4 deletions(-) diff --git a/src/mac2nix/cli.py b/src/mac2nix/cli.py index 68a15eb..397316f 100644 --- a/src/mac2nix/cli.py +++ b/src/mac2nix/cli.py @@ -316,8 +316,7 @@ def _run_nix_flake_lock(output_dir: Path) -> None: def add_host_cmd(output_dir: Path, hostname: str, username: str, system: str, op_vault: str | None) -> None: """Register a host with an existing mac2nix-scaffolded framework.""" - def _confirm_backup(fingerprint: str, current_username: str, current_hostname: str) -> bool: - click.echo(f"Public key fingerprint: {fingerprint}") + def _confirm_backup(_fingerprint: str, current_username: str, current_hostname: str) -> bool: if op_vault: title = f"mac2nix age key — {current_hostname}/{current_username}" try: @@ -327,7 +326,7 @@ def _confirm_backup(fingerprint: str, current_username: str, current_hostname: s else: click.echo(f"Backed up to 1Password vault {op_vault!r} (item {item_id}), verified by read-back.") return True - while not click.confirm("Have you backed up the private key to a password manager?", default=False): + while not click.confirm("Have you backed up the private key?", default=False): click.echo("This key cannot be recovered if lost — please back it up before continuing.") return True @@ -358,7 +357,7 @@ def _register_one(current_hostname: str, current_username: str, current_system: next_system = click.prompt("System", default=system, type=click.Choice(_SYSTEM_CHOICES), show_default=True) _register_one(next_hostname, next_username, next_system) - if _confirm_or_default("Run `nix flake lock` now?", default=False): + if _confirm_or_default("Run `nix flake lock` now?", default=True): _run_nix_flake_lock(output_dir) else: click.echo(f"Next: run `nix flake lock` inside {output_dir} before the first build for this host.") diff --git a/tests/cli/test_add_host.py b/tests/cli/test_add_host.py index ad17ec1..44d094d 100644 --- a/tests/cli/test_add_host.py +++ b/tests/cli/test_add_host.py @@ -44,6 +44,7 @@ def test_succeeds_end_to_end_with_confirmed_input(self, tmp_path: Path) -> None: assert (output_dir / "hosts" / "darwin" / "myhost" / ".mac2nix-meta.json").is_file() assert (output_dir / "users" / "alice.nix").is_file() assert (output_dir / "secrets" / "myhost.yaml").is_file() + assert result.output.count("age key fingerprint:") == 1 assert "myhost" in (output_dir / "flake.nix").read_text() assert "myhost" in (output_dir / ".sops.yaml").read_text() @@ -219,6 +220,26 @@ def test_flake_lock_prompt_invokes_nix_when_confirmed(self, tmp_path: Path) -> N assert result.exit_code == 0, result.output mock_lock.assert_called_once_with(output_dir) + def test_flake_lock_prompt_defaults_to_yes_on_bare_enter(self, tmp_path: Path) -> None: + """Pressing Enter (no explicit y/n) at the flake-lock prompt must run it, not skip it.""" + output_dir = tmp_path / "repo" + init_framework(output_dir) + + runner = CliRunner() + with ( + _redirect_age_keys(tmp_path / "age-keys"), + patch("mac2nix.cli._run_nix_flake_lock") as mock_lock, + ): + result = runner.invoke( + main, + ["add-host", str(output_dir), "--hostname", "myhost", "--username", "alice"], + input="y\nn\n\n", + ) + + assert result.exit_code == 0, result.output + assert "Run `nix flake lock` now? [Y/n]" in result.output + mock_lock.assert_called_once_with(output_dir) + def test_flake_lock_prompt_reports_failure(self, tmp_path: Path) -> None: output_dir = tmp_path / "repo" init_framework(output_dir) From 7fcddff7b652d30d006ac117d3c45cdb641bea8c Mon Sep 17 00:00:00 2001 From: Will Gordon Date: Fri, 7 Aug 2026 11:59:34 -0400 Subject: [PATCH 21/33] fix(cli): reads op document create's real "uuid" field, not "id" MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Caught by an actual `op document create --format json` invocation: the real response is `{"uuid": ..., "createdAt": ..., "updatedAt": ..., "vaultUuid": ...}` — no "id" field at all, despite "id" being what `op item ...` commands use elsewhere. store_age_key() was parsing the wrong key, so every real backup attempt that got past authentication would have failed at this step. A mock-based test can only ever verify its own guess at a schema; this needed a real op call to surface. --- src/mac2nix/onepassword.py | 7 ++++++- tests/test_onepassword.py | 23 ++++++++++++++++++++--- 2 files changed, 26 insertions(+), 4 deletions(-) diff --git a/src/mac2nix/onepassword.py b/src/mac2nix/onepassword.py index 4a9def6..e09174d 100644 --- a/src/mac2nix/onepassword.py +++ b/src/mac2nix/onepassword.py @@ -65,8 +65,13 @@ def store_age_key(key_path: Path, *, vault: str, title: str) -> str: if create.returncode != 0: raise OnePasswordError(f"op document create failed: {create.stderr.strip()}") + # Verified against a real `op document create --format json` response: + # the created item's identifier comes back as "uuid", not "id" — despite + # "id" being the field name `op item ...` commands use elsewhere. Caught + # by an actual op invocation; a schema this function's own tests can't + # verify by construction, since a mock only ever asserts its own guess. try: - item_id = json.loads(create.stdout)["id"] + item_id = json.loads(create.stdout)["uuid"] except (json.JSONDecodeError, KeyError) as exc: raise OnePasswordError(f"op document create returned unexpected output: {create.stdout.strip()}") from exc diff --git a/tests/test_onepassword.py b/tests/test_onepassword.py index 59e3135..54abb65 100644 --- a/tests/test_onepassword.py +++ b/tests/test_onepassword.py @@ -73,11 +73,28 @@ def test_raises_when_create_output_is_not_json(self, tmp_path: Path) -> None: with pytest.raises(onepassword.OnePasswordError, match="unexpected output"): onepassword.store_age_key(key_path, vault="Private", title="t") + def test_raises_when_create_output_is_missing_uuid_field(self, tmp_path: Path) -> None: + """A real `op document create --format json` response uses "uuid", not "id" — + regression guard for that exact field-name mismatch (caught via a real op + invocation, since a mock can only ever assert its own guess at the schema). + """ + key_path = tmp_path / "keys.txt" + key_path.write_text("age-secret-key") + + with ( + patch("mac2nix.onepassword.is_available", return_value=True), + patch("mac2nix.onepassword.subprocess.run") as mock_run, + ): + mock_run.return_value.returncode = 0 + mock_run.return_value.stdout = json.dumps({"id": "item123"}) + with pytest.raises(onepassword.OnePasswordError, match="unexpected output"): + onepassword.store_age_key(key_path, vault="Private", title="t") + def test_raises_when_verify_read_fails(self, tmp_path: Path) -> None: key_path = tmp_path / "keys.txt" key_path.write_text("age-secret-key") - create_result = type("R", (), {"returncode": 0, "stdout": json.dumps({"id": "item123"})})() + create_result = type("R", (), {"returncode": 0, "stdout": json.dumps({"uuid": "item123"})})() verify_result = type("R", (), {"returncode": 1, "stderr": b"item not found"})() with ( @@ -91,7 +108,7 @@ def test_raises_when_readback_content_mismatches(self, tmp_path: Path) -> None: key_path = tmp_path / "keys.txt" key_path.write_text("age-secret-key") - create_result = type("R", (), {"returncode": 0, "stdout": json.dumps({"id": "item123"})})() + create_result = type("R", (), {"returncode": 0, "stdout": json.dumps({"uuid": "item123"})})() verify_result = type("R", (), {"returncode": 0, "stdout": b"wrong-content"})() with ( @@ -105,7 +122,7 @@ def test_succeeds_when_readback_matches(self, tmp_path: Path) -> None: key_path = tmp_path / "keys.txt" key_path.write_text("age-secret-key") - create_result = type("R", (), {"returncode": 0, "stdout": json.dumps({"id": "item123"})})() + create_result = type("R", (), {"returncode": 0, "stdout": json.dumps({"uuid": "item123"})})() verify_result = type("R", (), {"returncode": 0, "stdout": key_path.read_bytes()})() with ( From cd54e1336c25335ec4dc283a82726dbb55862005 Mon Sep 17 00:00:00 2001 From: Will Gordon Date: Fri, 7 Aug 2026 14:23:13 -0400 Subject: [PATCH 22/33] fix(cli): reports 1password item by title, not raw uuid MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The success message printed a raw item uuid ("item 3xem4ns7v..."), which isn't something you can search for in the 1Password app — the title is. Report the human-readable title (already constructed for the op document create call itself) instead. --- src/mac2nix/cli.py | 6 ++++-- tests/cli/test_add_host.py | 2 +- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/src/mac2nix/cli.py b/src/mac2nix/cli.py index 397316f..2d69fcd 100644 --- a/src/mac2nix/cli.py +++ b/src/mac2nix/cli.py @@ -320,11 +320,13 @@ def _confirm_backup(_fingerprint: str, current_username: str, current_hostname: if op_vault: title = f"mac2nix age key — {current_hostname}/{current_username}" try: - item_id = onepassword.store_age_key(age_key_path(current_username), vault=op_vault, title=title) + onepassword.store_age_key(age_key_path(current_username), vault=op_vault, title=title) except onepassword.OnePasswordError as exc: click.echo(f"1Password backup failed ({exc}) — falling back to manual confirmation.") else: - click.echo(f"Backed up to 1Password vault {op_vault!r} (item {item_id}), verified by read-back.") + # The item's title, not its raw uuid — a uuid isn't something + # you can search for in the 1Password app, the title is. + click.echo(f"Backed up to 1Password vault {op_vault!r} as {title!r}, verified by read-back.") return True while not click.confirm("Have you backed up the private key?", default=False): click.echo("This key cannot be recovered if lost — please back it up before continuing.") diff --git a/tests/cli/test_add_host.py b/tests/cli/test_add_host.py index 44d094d..201eb93 100644 --- a/tests/cli/test_add_host.py +++ b/tests/cli/test_add_host.py @@ -170,7 +170,7 @@ def test_op_vault_success_skips_manual_confirmation(self, tmp_path: Path) -> Non ) assert result.exit_code == 0, result.output - assert "Backed up to 1Password vault 'Private' (item item123)" in result.output + assert "Backed up to 1Password vault 'Private' as 'mac2nix age key — myhost/alice'" in result.output assert "Have you backed up the private key" not in result.output mock_store.assert_called_once() kwargs = mock_store.call_args.kwargs From a2d0037278cb36915ca1d9fa019caec499a380f2 Mon Sep 17 00:00:00 2001 From: Will Gordon Date: Fri, 7 Aug 2026 15:36:13 -0400 Subject: [PATCH 23/33] docs(ci): trims comment bloat and removes stale merge notes MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit pr-checks.yaml had grown to ~40% comments, much of it restating detail already covered in the referenced test files' own docstrings. Condenses every block to its essential non-obvious fact and drops the two "NOTE FOR WHOEVER MERGES THIS PR" comments outright — both described manual setup (required-status-checks, the vm-validated environment) that's already declared and applied via khepri/tofu (gordon-code/mac2nix.tf), confirmed live by this session's own repeated successful use of the vm-validated environment's approval gate. --- .github/workflows/pr-checks.yaml | 105 +++++++++---------------------- 1 file changed, 31 insertions(+), 74 deletions(-) diff --git a/.github/workflows/pr-checks.yaml b/.github/workflows/pr-checks.yaml index 9b2d39b..5e9d9d4 100644 --- a/.github/workflows/pr-checks.yaml +++ b/.github/workflows/pr-checks.yaml @@ -30,16 +30,9 @@ jobs: - name: Test run: make test - # Runs on every pull_request, unconditional (not gated like the - # `nix-darwin-switch` job below) — this plan's Key Decisions require every - # deliverable validated before merge, not after. No `needs:` on - # lint-and-test — runs in parallel. Inherits the workflow's default - # read-only GITHUB_TOKEN (no elevated permissions) since this job runs - # `nix build` against PR-supplied template content, including from forks. - # - # NOTE FOR WHOEVER MERGES THIS PR: this job needs to be added to the - # repository's branch-protection required-status-checks list manually via - # GitHub repo settings — a one-time change outside this workflow file's scope. + # Unconditional and ungated (unlike nix-darwin-switch below) — every PR must + # build before merge. Read-only default GITHUB_TOKEN is safe even against + # fork PRs since this only runs `nix build`, no elevated permissions needed. nix-integration: if: github.event_name == 'pull_request' runs-on: macos-latest @@ -58,20 +51,16 @@ jobs: - name: Install sops and age run: brew install sops age - # Authenticates nix's own github: flake-input fetches (see - # tests/_scaffold_helpers.py's _nix_extra_access_tokens_args() for why - # this matters on a shared, unauthenticated-rate-limited runner IP - # pool). Default read-only GITHUB_TOKEN — no elevated permissions, - # matches this job's existing security posture against fork PRs. + # Authenticates nix's github: flake-input fetches against the shared, + # rate-limited runner IP pool (see _nix_extra_access_tokens_args()). - name: Nix build integration tests env: GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} run: make test-nix - # Job-level (not workflow-level `on.pull_request.paths`) scoping: a workflow-level - # path filter leaves a required status check permanently "Waiting for status to be - # reported" on any PR that doesn't touch the filtered paths. A job-level skip - # reports a real, non-blocking "success" status instead. + # Job-level scoping, not workflow-level `on.pull_request.paths` — a + # workflow-level filter leaves required checks stuck "Waiting for status" + # on PRs that don't touch these paths; a job-level skip reports success. detect-changes: if: github.event_name == 'pull_request' runs-on: macos-latest @@ -92,32 +81,17 @@ jobs: echo "vm-relevant=false" >> "$GITHUB_OUTPUT" fi - # Real, invasive (applies a genuine nix-darwin switch to the runner itself) and - # gated: only runs when detect-changes says the PR touches VM/generator-relevant - # paths, and even then waits for a maintainer's manual approval via the - # vm-validated environment's required reviewer. - # - # Nested macOS virtualization is categorically unsupported on GitHub-hosted - # runners (confirmed via GitHub's own docs — an Apple Virtualization Framework - # limitation, not a Tart-specific one: `tart run` exits immediately with no - # boot at all), so a Tart-VM-based apply-and-verify check — what this job - # originally did, and what `tests/vm/test_scaffold_vm.py` still does for local - # development on real Apple Silicon hardware — cannot run here. Instead this - # job applies the switch directly to the runner itself, which is safe only - # because the runner is already fully disposable (destroyed after the job). - # See tests/generators/test_scaffold_switch_native.py's own docstring for the - # full safety gating (skips unless GITHUB_ACTIONS=true, fails loudly rather - # than reusing a real key that shouldn't exist on a fresh runner). - # - # test_integration.py's TartVMManager-lifecycle/FileSystemComparator tests are - # deliberately NOT run here — they test the VM-control layer itself, which - # needs an actual VM boot regardless of what's being tested, so they can never - # run on any GitHub-hosted runner. They remain a local-only check - # (`make test-integration`, requires real Tart-capable hardware). + # Invasive (applies a real nix-darwin switch to the runner) and gated: + # only runs when detect-changes flags VM/generator-relevant paths, and + # even then needs a maintainer's approval via the vm-validated environment. # - # NOTE FOR WHOEVER MERGES THIS PR: the `vm-validated` GitHub Environment and its - # required reviewer need to be configured once via repo settings — this is outside - # this workflow file's scope. + # Runs natively, not via Tart, because nested macOS virtualization is + # unsupported on GitHub-hosted runners (see + # test_scaffold_switch_native.py's own docstring for the full rationale + # and safety gating) — safe here only because the runner itself is + # disposable. test_integration.py's VM-control-layer tests still can't + # run here regardless (they need an actual VM boot) and remain local-only + # via `make test-integration`. nix-darwin-switch: needs: detect-changes if: needs.detect-changes.outputs.vm-relevant == 'true' @@ -135,48 +109,31 @@ jobs: - uses: DeterminateSystems/nix-installer-action@ef8a148080ab6020fd15196c2084a2eea5ff2d25 # v22 - # Installed via Nix, not Homebrew, so they survive the Homebrew removal - # below — add_host() needs sops/age-keygen on PATH independent of - # whatever happens to Homebrew during the switch. + # Via Nix, not Homebrew, so these survive the Homebrew removal below — + # add_host() needs sops/age-keygen on PATH regardless of Homebrew's state. - name: Install sops and age via nix run: nix profile install nixpkgs#sops nixpkgs#age - # nix-darwin refuses to overwrite any /etc file it doesn't already manage - # and finds with unrecognized content — a safety guard against silently - # clobbering pre-existing system config, not a mac2nix-specific problem. - # DeterminateSystems/nix-installer-action itself writes nix.custom.conf, - # which nix-darwin's own Nix management then wants to own — exactly the - # scenario nix-darwin's own error message describes, with its own - # suggested fix (rename it out of the way first). A real user migrating a - # Mac that already has Nix installed independently of mac2nix would hit - # this identically and need the same one-time manual step. + # nix-darwin refuses to overwrite /etc files it doesn't manage with + # unrecognized content — the nix-installer-action writes its own + # nix.custom.conf, which nix-darwin then wants to own. A real user + # migrating an existing Nix install would hit this identically. - name: Move pre-existing /etc/nix/nix.custom.conf out of nix-darwin's way run: | if [ -f /etc/nix/nix.custom.conf ]; then sudo mv /etc/nix/nix.custom.conf /etc/nix/nix.custom.conf.before-nix-darwin fi - # nix-homebrew's autoMigrate adopts an existing Homebrew install, but - # this runner's pre-provisioned Homebrew has real content from dozens of - # formulae/casks across several non-default taps (Azure, AWS, etc.) — - # migration kept cascading through one conflict after another (an - # existing Library/Taps directory, then a formula whose originating tap - # had just been removed). Rather than keep chasing individual - # conflicts on an image this heavily provisioned, remove Homebrew - # entirely first: nix-homebrew then does a normal fresh install against - # an empty prefix, sidestepping autoMigrate's adoption path altogether. - # autoMigrate = true stays set in the scaffold template regardless — - # it's still the objectively correct setting for a real user's actual - # migration target, which (unlike this disposable runner) genuinely - # needs its existing Homebrew adopted rather than wiped. + # nix-homebrew's autoMigrate can't cleanly adopt this runner's heavily + # provisioned Homebrew (real content across many non-default taps) — + # migration cascaded through conflict after conflict. Wiping it first + # lets nix-homebrew do a normal fresh install instead. autoMigrate + # stays true in the template — real migration targets need it. - name: Remove pre-existing Homebrew installation run: sudo rm -rf /opt/homebrew - # See the nix-integration job's own comment on GITHUB_TOKEN — same - # reasoning applies here, for this job's own github: flake-input - # fetches during the real switch (which sudo's env-stripping means - # can't just be set via NIX_CONFIG at the job level; the test itself - # threads this through as an explicit nix CLI argument instead). + # Same GITHUB_TOKEN reasoning as nix-integration — sudo's env-stripping + # means this is threaded through as an explicit nix CLI arg, not NIX_CONFIG. - name: Real nix-darwin switch (applied to this disposable runner) env: GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} From 9d58159a9292529befc08e46cdb6772d8b279ba3 Mon Sep 17 00:00:00 2001 From: Will Gordon Date: Fri, 7 Aug 2026 15:43:19 -0400 Subject: [PATCH 24/33] docs: trims remaining comment bloat found in a full audit --- src/mac2nix/vm/manager.py | 58 ++++++++++++++---------------------- tests/vm/test_scaffold_vm.py | 31 ++++++------------- 2 files changed, 32 insertions(+), 57 deletions(-) diff --git a/src/mac2nix/vm/manager.py b/src/mac2nix/vm/manager.py index 603dd8f..b4dc608 100644 --- a/src/mac2nix/vm/manager.py +++ b/src/mac2nix/vm/manager.py @@ -189,30 +189,21 @@ async def start(self) -> None: async def _ensure_dns_resolves(self, max_attempts: int = 3) -> None: """Point the guest's DNS at a public resolver, bypassing Tart's own gateway. - Verified empirically against a real VM: Tart's vmnet-provided gateway - (the DHCP-assigned nameserver, e.g. 192.168.64.1) can fail to answer - DNS queries from the guest at all — not slow, an outright connection - failure — while the exact same guest resolves instantly via a direct - public resolver. Every real network operation this project runs - inside a VM (downloading the Nix installer, fetching flake inputs) - needs public-hostname resolution, never anything the gateway's own - DHCP-provided resolver would uniquely know, so unconditionally - overriding it here is safe for this project's actual VM usage. - - "Ethernet" is Tart's macOS base images' one consistent network - service name (a single virtio-net interface) — not dynamically - discovered, since every base image this project targets uses it. - - Retries on failure like :meth:`wait_ready` does for the same reason: - reproduced empirically, a VM can briefly reject the very same SSH - credentials moments after `wait_ready()`'s own check just succeeded - with them (macOS first-boot account/password setup finishing shortly - after sshd starts accepting connections) — a single-shot call here - would throw away `wait_ready()`'s own tolerance for that exact class - of boot-timing flakiness. Raises :exc:`VMError` if every attempt - fails, since a broken guest resolver would otherwise surface later as - a much more confusing "could not resolve host" failure from whatever - real command runs next. + Verified against a real VM: Tart's vmnet gateway (e.g. 192.168.64.1) + can outright fail to answer guest DNS queries, while the same guest + resolves instantly via a direct public resolver. Every real network + op this project runs in a VM only needs public-hostname resolution, + so overriding unconditionally is safe here. + + "Ethernet" is Tart's one consistent network service name across the + base images this project targets — not dynamically discovered. + + Retries for the same reason as :meth:`wait_ready`: a VM can briefly + reject the same SSH credentials moments after `wait_ready()`'s own + check just succeeded (account/password setup still settling). Raises + :exc:`VMError` after exhausting attempts, so a broken resolver + surfaces clearly instead of as a confusing "could not resolve host" + from whatever real command runs next. """ err = "" for attempt in range(max_attempts): @@ -230,17 +221,14 @@ async def _ensure_dns_resolves(self, max_attempts: int = 3) -> None: async def wait_ready(self, max_attempts: int = 10) -> None: """Poll until the VM has an IP and accepts two consecutive SSH connections. - Sleeps 5 seconds between attempts. Requires the SSH check to succeed - twice in a row (2s apart) before declaring the VM ready — reproduced - empirically, a single successful `whoami` isn't a reliable enough - signal: a freshly-booted VM's account/SSH state can still be settling - at that exact moment, and the very next SSH-dependent call from a - caller (a second `exec_command`, an `scp`) can spuriously get - "Permission denied" moments later even though the credentials are - correct and a subsequent retry would succeed. One confirmation check - closes that window for every caller, rather than needing its own - retry logic at each call site. Raises :exc:`VMTimeoutError` when - *max_attempts* is exhausted without two consecutive successes. + Sleeps 5 seconds between attempts. A single successful `whoami` isn't + reliable enough — a freshly-booted VM's account/SSH state can still be + settling, so the very next SSH-dependent call (another exec_command, + an scp) can spuriously get "Permission denied" moments later with + correct credentials. Two successes 2s apart closes that window for + every caller instead of needing retry logic at each call site. Raises + :exc:`VMTimeoutError` when *max_attempts* is exhausted without two + consecutive successes. """ clone = self._require_clone() logger.debug("Waiting for VM %r to be ready (%d attempts)", clone, max_attempts) diff --git a/tests/vm/test_scaffold_vm.py b/tests/vm/test_scaffold_vm.py index 2a848b6..185ea8d 100644 --- a/tests/vm/test_scaffold_vm.py +++ b/tests/vm/test_scaffold_vm.py @@ -110,12 +110,9 @@ async def _run() -> tuple[bool, str, str]: await _copy_age_key_to_vm(nix_darwin_vm, local_key_path, _VM_USERNAME) await validator._bootstrap_nix_darwin() - # nix-darwin refuses to overwrite any /etc file it doesn't already - # manage and finds with unrecognized content — the Determinate Nix - # installer run by _bootstrap_nix_darwin() above writes its own - # /etc/nix/nix.custom.conf, which nix-darwin's own Nix management - # then wants to own. Same conflict, same fix as the CI - # nix-darwin-switch job's own step for this exact reason. + # nix-darwin refuses to overwrite an unrecognized /etc/nix/nix.custom.conf + # (written by the Nix installer above) — same conflict/fix as the CI + # nix-darwin-switch job. 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; " @@ -125,26 +122,16 @@ async def _run() -> tuple[bool, str, str]: if not ok: raise VMError(f"Failed to move aside /etc/nix/nix.custom.conf: {err.strip()}") - # macos-tahoe-base ships with a pre-existing Homebrew install. - # nix-homebrew's autoMigrate can't cleanly adopt one with real content - # across multiple taps — same cascading conflict already root-caused - # and fixed the same way in CI's nix-darwin-switch job (an existing - # Library/Taps directory, then a formula whose originating tap had - # just been removed). Wiping it first lets nix-homebrew do a normal - # fresh install instead. Nothing in this test needs Homebrew itself — - # Nix comes from _bootstrap_nix_darwin() above, independent of brew. + # macos-tahoe-base's pre-existing Homebrew can't be cleanly adopted by + # nix-homebrew's autoMigrate — same conflict/fix as CI's nix-darwin-switch + # job. Nothing here needs Homebrew itself; Nix comes from the bootstrap above. ok, _out, err = await nix_darwin_vm.exec_command(["sudo", "rm", "-rf", "/opt/homebrew"], timeout=60) if not ok: raise VMError(f"Failed to remove pre-existing Homebrew: {err.strip()}") - # nix-darwin's system activation now always runs as root (per - # system.primaryUser's own purpose) — plain `nix run` fails with - # "system activation must now be run as root". `sudo -n` fails fast - # rather than hanging on a password prompt if passwordless sudo isn't - # actually available. `$(command -v nix)` resolves nix's absolute - # path in the current (profile-sourced) shell *before* handing it to - # sudo, since sudo's own secure_path won't include wherever the - # nix-daemon profile put it on PATH. + # nix-darwin's activation runs as root; `sudo -n` fails fast instead of + # hanging if passwordless sudo isn't available. `$(command -v nix)` + # resolves nix's path before sudo, since sudo's secure_path won't include it. switch_cmd = ( f"cd {validator._REMOTE_FLAKE_DIR}" " && . /nix/var/nix/profiles/default/etc/profile.d/nix-daemon.sh" From 8008c9039ffd567a0a7bbfdba9df70da8b7828b4 Mon Sep 17 00:00:00 2001 From: Will Gordon Date: Fri, 7 Aug 2026 16:38:32 -0400 Subject: [PATCH 25/33] refactor(generators): passes primaryUser instead of inferring it helpers.nix derived system.primaryUser via `builtins.elemAt users 0`, silently trusting list order even though `users` is really scoped to home-manager account generation and could grow past one entry via a hand-edit. Python already knows the single canonical username per host from .mac2nix-meta.json -- the same value used for `users` itself and for configuration.nix's __USERNAME__ substitution -- so add-host now emits it as its own explicit primaryUser field instead of making Nix re-derive it positionally. --- src/mac2nix/generators/scaffold.py | 3 ++- src/mac2nix/templates/scaffold/lib/helpers.nix | 7 ++++--- 2 files changed, 6 insertions(+), 4 deletions(-) diff --git a/src/mac2nix/generators/scaffold.py b/src/mac2nix/generators/scaffold.py index f6ed41d..a56d59c 100644 --- a/src/mac2nix/generators/scaffold.py +++ b/src/mac2nix/generators/scaffold.py @@ -216,7 +216,8 @@ def _regenerate_flake_hosts_block(output_dir: Path, metas: list[dict[str, Any]]) host_lines = [ f" {nix_string(m['hostname'])} = mkDarwinSystem {{ hostname = {nix_string(m['hostname'])}; " - f"system = {nix_string(m['system'])}; users = [ {nix_string(m['username'])} ]; }};" + f"system = {nix_string(m['system'])}; users = [ {nix_string(m['username'])} ]; " + f"primaryUser = {nix_string(m['username'])}; }};" for m in metas ] new_inner = "".join(f"{line}\n" for line in host_lines) + " " diff --git a/src/mac2nix/templates/scaffold/lib/helpers.nix b/src/mac2nix/templates/scaffold/lib/helpers.nix index b45121f..f73b73b 100644 --- a/src/mac2nix/templates/scaffold/lib/helpers.nix +++ b/src/mac2nix/templates/scaffold/lib/helpers.nix @@ -1,10 +1,9 @@ { inputs }: -{ hostname, system, users }: +{ hostname, system, users, primaryUser }: let inherit (inputs) darwin home-manager nix-homebrew mac-app-util determinate sops-nix homebrew-core homebrew-cask; - primaryUser = builtins.elemAt users 0; in darwin.lib.darwinSystem { specialArgs = { inherit inputs hostname; }; @@ -19,7 +18,9 @@ darwin.lib.darwinSystem { nixpkgs.hostPlatform = system; # Required by nix-darwin whenever an option that used to apply to the # invoking user (e.g. homebrew.enable) is set, now that system - # activation always runs as root — per host, from its own user list. + # activation always runs as root. Passed explicitly per host rather + # than inferred from `users` — that list also feeds home-manager and + # may grow past one entry without changing who owns Homebrew/sops. system.primaryUser = primaryUser; sops.defaultSopsFile = ../secrets + "/${hostname}.yaml"; From f25672757eb1e44c7d931b44ff56e87773f02ca3 Mon Sep 17 00:00:00 2001 From: Will Gordon Date: Sat, 8 Aug 2026 10:52:37 -0400 Subject: [PATCH 26/33] fix(cli): distinguishes real interrupt from eof in confirm prompts --- src/mac2nix/cli.py | 17 ++++++++-- tests/cli/test_add_host.py | 64 +++++++++++++++++++++++++++++++++++++- 2 files changed, 78 insertions(+), 3 deletions(-) diff --git a/src/mac2nix/cli.py b/src/mac2nix/cli.py index 2d69fcd..e0eb0c2 100644 --- a/src/mac2nix/cli.py +++ b/src/mac2nix/cli.py @@ -258,10 +258,23 @@ def _confirm_or_default(prompt: str, *, default: bool) -> bool: earlier in this invocation is already fully committed, so an exhausted stdin here should behave like silently declining, not like aborting a command that already did its real work. + + Note: `click.confirm()` catches EOFError internally and re-raises it as + `click.Abort` — that's what must be caught here, not EOFError. But + `click.confirm()` converts a real KeyboardInterrupt to `click.Abort` the + exact same way, so catching `click.Abort` unconditionally would also + swallow a genuine Ctrl-C at these prompts (defaulting to `True` on the + "run nix flake lock now?" prompt would execute a real subprocess despite + the user's explicit interrupt). `Abort()` is raised via `from None` + inside `except (KeyboardInterrupt, EOFError):`, which suppresses the + traceback but still sets `__context__` to the real underlying exception — + check it to tell the two apart and let a real interrupt actually abort. """ try: return click.confirm(prompt, default=default) - except EOFError: + except click.Abort as exc: + if isinstance(exc.__context__, KeyboardInterrupt): + raise return default @@ -341,7 +354,7 @@ def _register_one(current_hostname: str, current_username: str, current_system: current_system, confirm_backup=lambda fp: _confirm_backup(fp, current_username, current_hostname), ) - except EOFError: + except click.Abort: raise except Exception as exc: raise click.ClickException(str(exc)) from exc diff --git a/tests/cli/test_add_host.py b/tests/cli/test_add_host.py index 201eb93..775ca17 100644 --- a/tests/cli/test_add_host.py +++ b/tests/cli/test_add_host.py @@ -10,7 +10,7 @@ from click.testing import CliRunner from mac2nix import onepassword -from mac2nix.cli import main +from mac2nix.cli import _confirm_or_default, _run_nix_flake_lock, main from mac2nix.generators.scaffold import init_framework from tests._scaffold_helpers import _has_add_host_crypto_deps, _redirect_age_keys @@ -296,3 +296,65 @@ def test_invalid_username_rejected_before_any_crypto_call(self, tmp_path: Path) assert "username" in result.output.lower() mock_run.assert_not_called() assert not (output_dir / "hosts").exists() + + +class TestRunNixFlakeLockGuard: + """_run_nix_flake_lock's own shutil.which("nix") guard, exercised directly — every + add-host test that reaches the flake-lock prompt mocks _run_nix_flake_lock wholesale + and never triggers this guard.""" + + def test_raises_when_nix_not_on_path(self, tmp_path: Path) -> None: + with ( + patch("mac2nix.cli.shutil.which", return_value=None), + pytest.raises(click.ClickException, match="nix is not installed or not on PATH"), + ): + _run_nix_flake_lock(tmp_path) + + +class TestConfirmOrDefault: + """_confirm_or_default must resolve to *default* on real EOF, but still let a + genuine Ctrl-C abort — both surface as click.Abort from click.confirm(), so the + two must be told apart via the suppressed exception's __context__.""" + + def test_eof_resolves_to_default(self) -> None: + with patch("mac2nix.cli.click.confirm", side_effect=click.exceptions.Abort()): + assert _confirm_or_default("proceed?", default=True) is True + assert _confirm_or_default("proceed?", default=False) is False + + def test_keyboard_interrupt_propagates_instead_of_defaulting(self) -> None: + def _raise_from_keyboard_interrupt(*_args: object, **_kwargs: object) -> bool: + try: + raise KeyboardInterrupt + except KeyboardInterrupt: + raise click.exceptions.Abort from None + + with ( + patch("mac2nix.cli.click.confirm", side_effect=_raise_from_keyboard_interrupt), + pytest.raises(click.Abort), + ): + _confirm_or_default("proceed?", default=True) + + def test_real_eof_at_trailing_prompts_defaults_and_completes(self, tmp_path: Path) -> None: + """Exhausting stdin at the two optional trailing prompts (register-another, + run-flake-lock) must complete successfully with defaults applied, not abort + a command that already registered a host — the end-to-end regression this + fix targets.""" + output_dir = tmp_path / "repo" + init_framework(output_dir) + + runner = CliRunner() + with ( + _redirect_age_keys(tmp_path / "age-keys"), + patch("mac2nix.cli._run_nix_flake_lock") as mock_lock, + ): + result = runner.invoke( + main, + ["add-host", str(output_dir), "--hostname", "myhost", "--username", "alice"], + input="y\n", + ) + + assert result.exit_code == 0, result.output + assert (output_dir / "hosts" / "darwin" / "myhost").is_dir() + # "Register another host?" defaults False on EOF (no second host); "Run nix + # flake lock now?" defaults True on EOF, so the (mocked) lock step still runs. + mock_lock.assert_called_once() From 6708091775ba071d90139edcbf5f27a4e80cdedd Mon Sep 17 00:00:00 2001 From: Will Gordon Date: Sat, 8 Aug 2026 10:52:58 -0400 Subject: [PATCH 27/33] fix(generators): hardens age key directory permissions to 0700 --- src/mac2nix/generators/scaffold.py | 4 + tests/generators/test_scaffold.py | 137 ++++++++++++++++++++++++++++- 2 files changed, 140 insertions(+), 1 deletion(-) diff --git a/src/mac2nix/generators/scaffold.py b/src/mac2nix/generators/scaffold.py index a56d59c..8ccc59b 100644 --- a/src/mac2nix/generators/scaffold.py +++ b/src/mac2nix/generators/scaffold.py @@ -135,6 +135,10 @@ def generate_age_key(username: str, *, key_dir: Path | None = None) -> str: raise ScaffoldError(msg) key_path.parent.mkdir(parents=True, exist_ok=True) + key_path.parent.chmod(0o700) + actual_dir_mode = stat.S_IMODE(key_path.parent.stat().st_mode) + if actual_dir_mode != 0o700: + raise ScaffoldError(f"failed to set age key directory permissions to 0700 (got {oct(actual_dir_mode)})") result = subprocess.run( # noqa: S603 ["age-keygen", "-o", str(key_path)], # noqa: S607 diff --git a/tests/generators/test_scaffold.py b/tests/generators/test_scaffold.py index ed6a33b..5eaed33 100644 --- a/tests/generators/test_scaffold.py +++ b/tests/generators/test_scaffold.py @@ -4,6 +4,7 @@ import contextlib import json +import logging import os import re import stat @@ -15,7 +16,8 @@ import yaml from mac2nix.generators import Mac2NixError -from mac2nix.generators.scaffold import ScaffoldError, add_host, generate_age_key, init_framework +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 tests._scaffold_helpers import _has_add_host_crypto_deps, _has_age_keygen, _redirect_age_keys _EXPECTED_FRAMEWORK_FILES = [ @@ -119,6 +121,7 @@ def test_permissions_and_fingerprint(self, tmp_path: Path) -> None: key_path = key_dir / "keys.txt" assert key_path.is_file() assert stat.S_IMODE(key_path.stat().st_mode) == 0o600 + assert stat.S_IMODE(key_dir.stat().st_mode) == 0o700 public_key_line = next(line for line in key_path.read_text().splitlines() if line.startswith("# public key:")) assert public_key_line.removeprefix("# public key:").strip() == fingerprint @@ -138,6 +141,10 @@ def test_second_call_raises_without_modifying_existing_key(self, tmp_path: Path) assert key_path.read_text() == before_content +def test_age_key_path_matches_internal_construction() -> None: + assert age_key_path("alice") == Path("/Users/alice") / ".config" / "sops" / "age" / "keys.txt" + + # --------------------------------------------------------------------------- # add_host() — file/directory orchestration and rollback (crypto mocked) # --------------------------------------------------------------------------- @@ -303,6 +310,134 @@ def test_late_sops_failure_rolls_back_and_reverts_flake_and_sops(self, tmp_path: assert len(rules) == 1 assert rules[0]["key_groups"] == [{"age": ["age1hosta-fingerprint"]}] + def test_second_host_same_username_failure_leaves_shared_user_file_untouched(self, tmp_path: Path) -> None: + """A second host sharing an existing username must not have its shared + users/.nix rolled back if that second host's add_host() call fails — + user_file_created is False for it, since the file predates this call.""" + output_dir = tmp_path / "repo" + init_framework(output_dir) + + with _mocked_crypto("age1hosta-fingerprint"): + add_host(output_dir, "hosta", "shareduser", confirm_backup=lambda _: True) + + user_file = output_dir / "users" / "shareduser.nix" + before_mtime = user_file.stat().st_mtime_ns + before_content = user_file.read_text() + + with ( + patch("mac2nix.generators.scaffold.generate_age_key", return_value="age1hostb-fingerprint"), + patch( + "mac2nix.generators.scaffold._create_host_secrets_file", + side_effect=ScaffoldError("sops failed for testing"), + ), + pytest.raises(ScaffoldError, match="sops failed for testing"), + ): + add_host(output_dir, "hostb", "shareduser", confirm_backup=lambda _: True) + + assert not (output_dir / "hosts" / "darwin" / "hostb").exists() + assert user_file.stat().st_mtime_ns == before_mtime + assert user_file.read_text() == before_content + + def test_cleanup_time_regeneration_failure_does_not_mask_original_exception(self, tmp_path: Path) -> None: + """If cleanup's own re-invocation of _regenerate_sops_yaml() fails, the + ORIGINAL add_host() failure must still propagate — not the cleanup-time one.""" + output_dir = tmp_path / "repo" + init_framework(output_dir) + + real_regenerate_sops_yaml = scaffold_module._regenerate_sops_yaml + calls: list[int] = [] + + def flaky_regenerate_sops_yaml(output_dir: Path, metas: list[dict]) -> None: + calls.append(1) + if len(calls) == 2: + raise RuntimeError("cleanup-time regen failure") + real_regenerate_sops_yaml(output_dir, metas) + + with ( + patch("mac2nix.generators.scaffold.generate_age_key", return_value="age1hosta-fingerprint"), + patch( + "mac2nix.generators.scaffold._create_host_secrets_file", + side_effect=ScaffoldError("sops failed for testing"), + ), + patch("mac2nix.generators.scaffold._regenerate_sops_yaml", side_effect=flaky_regenerate_sops_yaml), + pytest.raises(ScaffoldError, match="sops failed for testing"), + ): + add_host(output_dir, "hosta", "alice", confirm_backup=lambda _: True) + + def test_age_key_path_inside_output_dir_raises_before_generating_key(self, tmp_path: Path) -> None: + """Guards against a resolved age-key path landing inside output_dir — a real + secret-leak risk since the CLI instructs users to push output_dir to a + (potentially public) git remote.""" + output_dir = tmp_path / "repo" + init_framework(output_dir) + + with ( + _redirect_age_keys(output_dir), + patch("mac2nix.generators.scaffold.generate_age_key") as mock_generate, + pytest.raises(ScaffoldError, match="refusing to generate an age key inside"), + ): + add_host(output_dir, "hosta", "alice", confirm_backup=lambda _: True) + + mock_generate.assert_not_called() + assert not (output_dir / "hosts" / "darwin" / "hosta").exists() + assert not (output_dir / "users" / "alice.nix").exists() + + def test_missing_state_file_produces_no_warning_and_creates_state_file( + self, tmp_path: Path, caplog: pytest.LogCaptureFixture + ) -> None: + output_dir = tmp_path / "repo" + init_framework(output_dir) + assert not (output_dir / ".mac2nix-state.json").exists() + + with ( + _mocked_crypto("age1hosta-fingerprint"), + caplog.at_level(logging.WARNING, logger="mac2nix.generators.scaffold"), + ): + add_host(output_dir, "hosta", "alice", confirm_backup=lambda _: True) + + assert not caplog.records + stored = json.loads((output_dir / ".mac2nix-state.json").read_text()) + assert "flake_hosts_block_hash" in stored + + def test_hand_edited_flake_hosts_block_triggers_warning_on_next_add_host( + self, tmp_path: Path, caplog: pytest.LogCaptureFixture + ) -> None: + output_dir = tmp_path / "repo" + init_framework(output_dir) + + with _mocked_crypto("age1hosta-fingerprint"): + add_host(output_dir, "hosta", "alice", confirm_backup=lambda _: True) + + flake_path = output_dir / "flake.nix" + flake_path.write_text(flake_path.read_text().replace("hosta", "HOSTA-HAND-EDITED")) + + with ( + _mocked_crypto("age1hostb-fingerprint"), + caplog.at_level(logging.WARNING, logger="mac2nix.generators.scaffold"), + ): + add_host(output_dir, "hostb", "bob", confirm_backup=lambda _: True) + + assert "doesn't match what add-host last wrote" in caplog.text + + def test_corrupt_state_file_handled_gracefully(self, tmp_path: Path, caplog: pytest.LogCaptureFixture) -> None: + output_dir = tmp_path / "repo" + init_framework(output_dir) + + with _mocked_crypto("age1hosta-fingerprint"): + add_host(output_dir, "hosta", "alice", confirm_backup=lambda _: True) + + (output_dir / ".mac2nix-state.json").write_text("{not valid json") + + with ( + _mocked_crypto("age1hostb-fingerprint"), + caplog.at_level(logging.WARNING, logger="mac2nix.generators.scaffold"), + ): + add_host(output_dir, "hostb", "bob", confirm_backup=lambda _: True) + + assert not caplog.records + stored = json.loads((output_dir / ".mac2nix-state.json").read_text()) + assert "flake_hosts_block_hash" in stored + # --------------------------------------------------------------------------- # add_host() — real sops/age integration (nix-marked) From 43b08d2d746c2989ba20fd265a2557452ba02641 Mon Sep 17 00:00:00 2001 From: Will Gordon Date: Sat, 8 Aug 2026 10:53:22 -0400 Subject: [PATCH 28/33] refactor(cli): privatizes unused onepassword sign-in check Also adds an opt-in real-op-CLI integration test tier (op_cli-marked, gated on MAC2NIX_TEST_OP_VAULT) and switches remaining ad-hoc mock objects to MagicMock, matching the rest of the test suite. --- src/mac2nix/onepassword.py | 6 +- tests/test_onepassword.py | 129 +++++++++++++++++++++++++++++++++---- 2 files changed, 121 insertions(+), 14 deletions(-) diff --git a/src/mac2nix/onepassword.py b/src/mac2nix/onepassword.py index e09174d..5385bff 100644 --- a/src/mac2nix/onepassword.py +++ b/src/mac2nix/onepassword.py @@ -24,7 +24,7 @@ def is_available() -> bool: return shutil.which("op") is not None -def is_signed_in() -> bool: +def _is_signed_in() -> bool: """Check sign-in state without ever triggering an interactive prompt. `op signin` blocks on user interaction if not already authenticated; @@ -44,13 +44,13 @@ def store_age_key(key_path: Path, *, vault: str, title: str) -> str: write that "succeeds" but silently stores the wrong (or no) content is worse than no backup, since it looks safe. - Deliberately does not pre-check `is_signed_in()` before attempting the + Deliberately does not pre-check `_is_signed_in()` before attempting the write — `op whoami` has a known failure mode where it reports "not signed in" while the local vault is still genuinely readable/writable (session-state can desync from what `op` actually has access to). Attempting the real operation and surfacing its own error message is a more reliable signal than a separate pre-flight check that can itself - be wrong; `is_signed_in()` is still available as its own function for + be wrong; `_is_signed_in()` is still available as its own function for callers that want a best-effort availability check. """ if not is_available(): diff --git a/tests/test_onepassword.py b/tests/test_onepassword.py index 54abb65..1190872 100644 --- a/tests/test_onepassword.py +++ b/tests/test_onepassword.py @@ -1,10 +1,19 @@ -"""Tests for the mac2nix.onepassword module — all `op` calls mocked, no real CLI needed.""" +"""Tests for the mac2nix.onepassword module. + +Most tests mock `op` entirely. TestStoreAgeKeyRealOp additionally exercises +the real `op` CLI, opted into via MAC2NIX_TEST_OP_VAULT, so a real 1Password +CLI JSON-schema change (e.g. the "uuid" vs "id" field) gets caught even +though the mocked tests above can only assert their own guess at the schema. +""" from __future__ import annotations import json +import os +import subprocess +import uuid from pathlib import Path -from unittest.mock import patch +from unittest.mock import MagicMock, patch import pytest @@ -23,12 +32,12 @@ def test_is_available_false_when_missing(self) -> None: def test_is_signed_in_true_on_zero_exit(self) -> None: with patch("mac2nix.onepassword.subprocess.run") as mock_run: mock_run.return_value.returncode = 0 - assert onepassword.is_signed_in() is True + assert onepassword._is_signed_in() is True def test_is_signed_in_false_on_nonzero_exit(self) -> None: with patch("mac2nix.onepassword.subprocess.run") as mock_run: mock_run.return_value.returncode = 1 - assert onepassword.is_signed_in() is False + assert onepassword._is_signed_in() is False class TestStoreAgeKey: @@ -44,7 +53,7 @@ def test_raises_when_op_not_available(self, tmp_path: Path) -> None: def test_raises_when_create_fails(self, tmp_path: Path) -> None: """Covers the not-signed-in case too, since store_age_key() no longer pre-checks - is_signed_in() — see its own docstring for why (op whoami can report "not signed + _is_signed_in() — see its own docstring for why (op whoami can report "not signed in" while the vault is still genuinely usable). The real `op document create` call's own error message is the source of truth instead. """ @@ -94,8 +103,8 @@ def test_raises_when_verify_read_fails(self, tmp_path: Path) -> None: key_path = tmp_path / "keys.txt" key_path.write_text("age-secret-key") - create_result = type("R", (), {"returncode": 0, "stdout": json.dumps({"uuid": "item123"})})() - verify_result = type("R", (), {"returncode": 1, "stderr": b"item not found"})() + create_result = MagicMock(returncode=0, stdout=json.dumps({"uuid": "item123"})) + verify_result = MagicMock(returncode=1, stderr=b"item not found") with ( patch("mac2nix.onepassword.is_available", return_value=True), @@ -108,8 +117,8 @@ def test_raises_when_readback_content_mismatches(self, tmp_path: Path) -> None: key_path = tmp_path / "keys.txt" key_path.write_text("age-secret-key") - create_result = type("R", (), {"returncode": 0, "stdout": json.dumps({"uuid": "item123"})})() - verify_result = type("R", (), {"returncode": 0, "stdout": b"wrong-content"})() + create_result = MagicMock(returncode=0, stdout=json.dumps({"uuid": "item123"})) + verify_result = MagicMock(returncode=0, stdout=b"wrong-content") with ( patch("mac2nix.onepassword.is_available", return_value=True), @@ -122,8 +131,8 @@ def test_succeeds_when_readback_matches(self, tmp_path: Path) -> None: key_path = tmp_path / "keys.txt" key_path.write_text("age-secret-key") - create_result = type("R", (), {"returncode": 0, "stdout": json.dumps({"uuid": "item123"})})() - verify_result = type("R", (), {"returncode": 0, "stdout": key_path.read_bytes()})() + create_result = MagicMock(returncode=0, stdout=json.dumps({"uuid": "item123"})) + verify_result = MagicMock(returncode=0, stdout=key_path.read_bytes()) with ( patch("mac2nix.onepassword.is_available", return_value=True), @@ -145,3 +154,101 @@ def test_succeeds_when_readback_matches(self, tmp_path: Path) -> None: "--format", "json", ] + + +# --------------------------------------------------------------------------- +# store_age_key() — real 1Password CLI integration (opt-in, op_cli-marked) +# --------------------------------------------------------------------------- + + +class _OpTestVaultUnavailableError(Exception): + """Raised by _resolve_op_test_vault() when a precondition is missing. + + Split out from the op_test_vault fixture below so each of the three skip + conditions is independently unit-testable (TestResolveOpTestVault) without + pytest.skip() aborting the very test that's checking the branch logic. + """ + + +def _resolve_op_test_vault() -> str: + """Order matters: op must be on PATH before checking sign-in, which must be + checked before the vault env var — each check assumes the previous one passed.""" + if not onepassword.is_available(): + raise _OpTestVaultUnavailableError("op CLI not on PATH") + if not onepassword._is_signed_in(): + raise _OpTestVaultUnavailableError("not signed in to 1Password CLI (op whoami failed)") + vault = os.environ.get("MAC2NIX_TEST_OP_VAULT") + if not vault: + raise _OpTestVaultUnavailableError("MAC2NIX_TEST_OP_VAULT not set — skipping real op CLI integration test") + return vault + + +class TestResolveOpTestVault: + """Runs in the default suite (not op_cli-marked) — unlike the real op_cli tier, + where only whichever precondition is actually missing on a given machine gets + exercised, these force each of the three skip branches independently.""" + + def test_skips_when_op_not_available(self) -> None: + with ( + patch("tests.test_onepassword.onepassword.is_available", return_value=False), + pytest.raises(_OpTestVaultUnavailableError, match="not on PATH"), + ): + _resolve_op_test_vault() + + def test_skips_when_not_signed_in(self) -> None: + with ( + patch("tests.test_onepassword.onepassword.is_available", return_value=True), + patch("tests.test_onepassword.onepassword._is_signed_in", return_value=False), + pytest.raises(_OpTestVaultUnavailableError, match="not signed in"), + ): + _resolve_op_test_vault() + + def test_skips_when_vault_env_var_unset(self, monkeypatch: pytest.MonkeyPatch) -> None: + monkeypatch.delenv("MAC2NIX_TEST_OP_VAULT", raising=False) + with ( + patch("tests.test_onepassword.onepassword.is_available", return_value=True), + patch("tests.test_onepassword.onepassword._is_signed_in", return_value=True), + pytest.raises(_OpTestVaultUnavailableError, match="MAC2NIX_TEST_OP_VAULT not set"), + ): + _resolve_op_test_vault() + + def test_returns_vault_when_all_preconditions_met(self) -> None: + with ( + patch("tests.test_onepassword.onepassword.is_available", return_value=True), + patch("tests.test_onepassword.onepassword._is_signed_in", return_value=True), + patch.dict(os.environ, {"MAC2NIX_TEST_OP_VAULT": "mac2nix-test"}), + ): + assert _resolve_op_test_vault() == "mac2nix-test" + + +@pytest.fixture +def op_test_vault() -> str: + """Skip unless a real, signed-in `op` CLI and a caller-designated test + vault are both available. This tier writes a real item into a real + 1Password vault, so it must never run against someone's account by + accident — MAC2NIX_TEST_OP_VAULT must be set explicitly to the name of a + vault that's safe to use for throwaway test items (e.g. a dedicated + "mac2nix-test" vault, never "Private"). + """ + try: + return _resolve_op_test_vault() + except _OpTestVaultUnavailableError as exc: + pytest.skip(str(exc)) + + +@pytest.mark.op_cli +class TestStoreAgeKeyRealOp: + def test_round_trip_against_real_vault(self, tmp_path: Path, op_test_vault: str) -> None: + key_path = tmp_path / "keys.txt" + key_path.write_text("age-secret-key-for-mac2nix-test\n") + title = f"mac2nix-test-{uuid.uuid4()}" + + item_id = onepassword.store_age_key(key_path, vault=op_test_vault, title=title) + try: + assert item_id + finally: + subprocess.run( # noqa: S603 + ["op", "document", "delete", item_id, "--vault", op_test_vault], # noqa: S607 + capture_output=True, + check=False, + ) From abf14529dad203decbb5a9032819edaa55175da8 Mon Sep 17 00:00:00 2001 From: Will Gordon Date: Sat, 8 Aug 2026 10:53:38 -0400 Subject: [PATCH 29/33] style(templates): standardizes nix module signatures across scaffold --- src/mac2nix/templates/scaffold/modules/darwin/default.nix | 2 +- src/mac2nix/templates/scaffold/modules/darwin/homebrew.nix | 2 +- src/mac2nix/templates/scaffold/modules/home-manager/default.nix | 2 +- src/mac2nix/templates/scaffold/users/user.nix | 2 +- 4 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/mac2nix/templates/scaffold/modules/darwin/default.nix b/src/mac2nix/templates/scaffold/modules/darwin/default.nix index 37d8e2d..29bccb3 100644 --- a/src/mac2nix/templates/scaffold/modules/darwin/default.nix +++ b/src/mac2nix/templates/scaffold/modules/darwin/default.nix @@ -1,4 +1,4 @@ -{ ... }: +{ config, lib, pkgs, ... }: { imports = [ diff --git a/src/mac2nix/templates/scaffold/modules/darwin/homebrew.nix b/src/mac2nix/templates/scaffold/modules/darwin/homebrew.nix index 0c96811..18bf2b2 100644 --- a/src/mac2nix/templates/scaffold/modules/darwin/homebrew.nix +++ b/src/mac2nix/templates/scaffold/modules/darwin/homebrew.nix @@ -1,4 +1,4 @@ -{ ... }: +{ config, lib, pkgs, ... }: { # Shared Homebrew activation policy — not scanned cask/brew data, which is diff --git a/src/mac2nix/templates/scaffold/modules/home-manager/default.nix b/src/mac2nix/templates/scaffold/modules/home-manager/default.nix index fca5958..720a861 100644 --- a/src/mac2nix/templates/scaffold/modules/home-manager/default.nix +++ b/src/mac2nix/templates/scaffold/modules/home-manager/default.nix @@ -1,4 +1,4 @@ -{ ... }: +{ config, lib, pkgs, ... }: { imports = [ diff --git a/src/mac2nix/templates/scaffold/users/user.nix b/src/mac2nix/templates/scaffold/users/user.nix index c8e451f..75e0c3c 100644 --- a/src/mac2nix/templates/scaffold/users/user.nix +++ b/src/mac2nix/templates/scaffold/users/user.nix @@ -1,4 +1,4 @@ -{ pkgs, lib, hostname, ... }: +{ config, lib, pkgs, hostname, ... }: { imports = lib.optional (builtins.pathExists (../hosts/darwin + "/${hostname}/packages.nix")) ( From 2ead2c66b212b27c6cc75c663d47d421b4ac654d Mon Sep 17 00:00:00 2001 From: Will Gordon Date: Sat, 8 Aug 2026 10:54:01 -0400 Subject: [PATCH 30/33] fix(vm): cleans up orphaned clone when prewarm fails Also moves logging.basicConfig() out of module scope, since the new tests/test_prewarm_vm.py imports this module and a module-scope call would mutate the root logger for the whole pytest process. --- scripts/prewarm_vm.py | 14 +++++-- tests/test_prewarm_vm.py | 85 ++++++++++++++++++++++++++++++++++++++++ 2 files changed, 96 insertions(+), 3 deletions(-) create mode 100644 tests/test_prewarm_vm.py diff --git a/scripts/prewarm_vm.py b/scripts/prewarm_vm.py index 76df6bf..bc22519 100644 --- a/scripts/prewarm_vm.py +++ b/scripts/prewarm_vm.py @@ -24,7 +24,6 @@ from mac2nix.vm.manager import BASE_IMAGE_NAME, BASE_IMAGE_REF, TartVMManager, pull_base_image_if_missing from mac2nix.vm.validator import NIX_INSTALLER_URL -logging.basicConfig(level=logging.INFO, format="%(message)s") logger = logging.getLogger(__name__) PREWARMED_VM_NAME = "mac2nix-nix-base" @@ -74,8 +73,12 @@ async def _prewarm() -> None: logger.info("Cloning %r -> %r", BASE_IMAGE_NAME, PREWARMED_VM_NAME) vm = TartVMManager(BASE_IMAGE_NAME) - await vm.clone(PREWARMED_VM_NAME) - await vm.start() + try: + await vm.clone(PREWARMED_VM_NAME) + await vm.start() + except VMError: + await vm.cleanup() + raise try: await _install_nix(vm) finally: @@ -87,6 +90,11 @@ async def _prewarm() -> None: def main() -> int: + # Configured here, not at module scope — this module is imported by + # tests/test_prewarm_vm.py (via pythonpath), and a module-scope + # basicConfig() would mutate the root logger for the whole pytest + # process the moment that test module is collected. + logging.basicConfig(level=logging.INFO, format="%(message)s") try: asyncio.run(_prewarm()) except VMError as exc: diff --git a/tests/test_prewarm_vm.py b/tests/test_prewarm_vm.py new file mode 100644 index 0000000..4e94824 --- /dev/null +++ b/tests/test_prewarm_vm.py @@ -0,0 +1,85 @@ +"""Tests for scripts/prewarm_vm.py — specifically the clone/start cleanup-on-failure path.""" + +from __future__ import annotations + +import asyncio +from unittest.mock import AsyncMock, MagicMock, patch + +import prewarm_vm +import pytest + +from mac2nix.vm._utils import VMError + + +def _make_vm(*, clone_error: Exception | None = None, start_error: Exception | None = None) -> MagicMock: + vm = MagicMock() + vm.clone = AsyncMock(side_effect=clone_error) + vm.start = AsyncMock(side_effect=start_error) + vm.cleanup = AsyncMock() + vm.stop = AsyncMock() + return vm + + +class TestPrewarmCleanupOnFailure: + def test_clone_failure_triggers_cleanup_and_reraises(self) -> None: + vm = _make_vm(clone_error=VMError("tart clone failed")) + + async def _run() -> None: + with ( + patch("prewarm_vm.TartVMManager.is_available", return_value=True), + patch("prewarm_vm.pull_base_image_if_missing", new=AsyncMock()), + patch("prewarm_vm.TartVMManager", return_value=vm), + pytest.raises(VMError, match="tart clone failed"), + ): + await prewarm_vm._prewarm() + + asyncio.run(_run()) + vm.cleanup.assert_awaited_once() + vm.stop.assert_not_awaited() # the post-install stop() is never reached + + def test_start_failure_triggers_cleanup_and_reraises(self) -> None: + vm = _make_vm(start_error=VMError("VM never became reachable")) + + async def _run() -> None: + with ( + patch("prewarm_vm.TartVMManager.is_available", return_value=True), + patch("prewarm_vm.pull_base_image_if_missing", new=AsyncMock()), + patch("prewarm_vm.TartVMManager", return_value=vm), + pytest.raises(VMError, match="VM never became reachable"), + ): + await prewarm_vm._prewarm() + + asyncio.run(_run()) + vm.clone.assert_awaited_once() + vm.cleanup.assert_awaited_once() + vm.stop.assert_not_awaited() + + def test_install_failure_still_stops_vm_without_calling_cleanup(self) -> None: + """A failure AFTER clone+start succeed (during Nix install) must still stop + the VM via the pre-existing finally block — cleanup() (which deletes the + clone) must NOT run here, since the whole point of prewarming is to keep + the clone around even if this particular install attempt failed.""" + vm = _make_vm() + + async def _run() -> None: + with ( + patch("prewarm_vm.TartVMManager.is_available", return_value=True), + patch("prewarm_vm.pull_base_image_if_missing", new=AsyncMock()), + patch("prewarm_vm.TartVMManager", return_value=vm), + patch("prewarm_vm._install_nix", new=AsyncMock(side_effect=VMError("install failed"))), + pytest.raises(VMError, match="install failed"), + ): + await prewarm_vm._prewarm() + + asyncio.run(_run()) + vm.clone.assert_awaited_once() + vm.stop.assert_awaited_once() + vm.cleanup.assert_not_awaited() + + def test_main_returns_1_and_logs_on_vm_error(self) -> None: + with ( + patch("prewarm_vm._prewarm", new=AsyncMock(side_effect=VMError("boom"))), + patch("prewarm_vm.logger") as mock_logger, + ): + assert prewarm_vm.main() == 1 + mock_logger.error.assert_called_once() From 742a3c3c154f05522a93e44ac3db86397fcbd3c9 Mon Sep 17 00:00:00 2001 From: Will Gordon Date: Sat, 8 Aug 2026 10:54:16 -0400 Subject: [PATCH 31/33] test(vm): covers validator's local source copy behavior --- tests/vm/test_validator.py | 109 +++++++++++++++++++++++++++++++++++++ 1 file changed, 109 insertions(+) diff --git a/tests/vm/test_validator.py b/tests/vm/test_validator.py index 627eb78..0ecb30d 100644 --- a/tests/vm/test_validator.py +++ b/tests/vm/test_validator.py @@ -468,6 +468,115 @@ async def _run() -> None: assert "admin" not in cmd # password must not appear in argv assert captured_env[0] == {"SSHPASS": "admin"} + 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"): + (tmp_path / name).mkdir() + (tmp_path / "flake.nix").touch() + + vm = _make_vm(exec_result=(True, "", "")) + captured: list[list[str]] = [] + + async def recording_run(cmd: list[str], **_kw) -> tuple[int, str, str]: + captured.append(cmd) + return (0, "", "") + + async def _run() -> None: + v = Validator(vm) + with patch("mac2nix.vm.validator.async_run_command", side_effect=recording_run): + await v._copy_flake_to_vm(tmp_path, exclude=Validator._LOCAL_SOURCE_EXCLUDE) + + asyncio.run(_run()) + cmd = captured[0] + sources = cmd[14:-1] # scp_cmd layout: 14 fixed args, *sources, dest + source_names = {Path(src).name for src in sources} + assert source_names == {"flake.nix"} + + +# --------------------------------------------------------------------------- +# Validator._scan_vm() — mac2nix_source selection +# --------------------------------------------------------------------------- + + +class TestScanVMSourceSelection: + def test_default_source_runs_published_flake_without_extra_copy(self) -> 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, "", "")) + vm_state = _base_state(shell=ShellConfig(shell_type="fish")) + exec_calls: list[list[str]] = [] + + async def exec_side_effect(cmd, **_kw): + exec_calls.append(cmd) + return (True, "", "") + + vm.exec_command = AsyncMock(side_effect=exec_side_effect) + + async def _run() -> SystemState: + v = Validator(vm) # default mac2nix_source + with ( + patch("mac2nix.vm.validator.async_run_command", new=AsyncMock(return_value=(0, "", ""))), + patch.object(SystemState, "from_json", return_value=vm_state), + ): + return await v._scan_vm() + + 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 + 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: + """A non-default mac2nix_source is SCPed into the VM (excluding dev-only + dirs) and `nix run` targets the remote source dir, not a github: URL.""" + (tmp_path / "flake.nix").touch() + (tmp_path / ".git").mkdir() + + vm = _make_vm(exec_result=(True, "", "")) + vm_state = _base_state(shell=ShellConfig(shell_type="fish")) + exec_calls: list[list[str]] = [] + + async def exec_side_effect(cmd, **_kw): + exec_calls.append(cmd) + return (True, "", "") + + vm.exec_command = AsyncMock(side_effect=exec_side_effect) + scp_calls: list[list[str]] = [] + + async def recording_run(cmd: list[str], **_kw) -> tuple[int, str, str]: + scp_calls.append(cmd) + return (0, "", "") + + async def _run() -> SystemState: + v = Validator(vm, mac2nix_source=str(tmp_path)) + with ( + patch("mac2nix.vm.validator.async_run_command", side_effect=recording_run), + patch.object(SystemState, "from_json", return_value=vm_state), + ): + return await v._scan_vm() + + result = asyncio.run(_run()) + assert result is vm_state + + # mkdir for the source copy, then the nix-run scan invocation. + assert len(exec_calls) == 2 + mkdir_cmd, nix_run_cmd = exec_calls + assert mkdir_cmd == ["mkdir", "-p", Validator._REMOTE_SOURCE_DIR] + + joined = nix_run_cmd[2] + assert f"nix run {Validator._REMOTE_SOURCE_DIR} --" in joined + assert str(tmp_path) not in joined + + # The scp for the local source copy excludes ".git". + assert scp_calls, "expected an scp invocation for the local source copy" + source_scp_cmd = scp_calls[0] + sources = source_scp_cmd[14:-1] + source_names = {Path(src).name for src in sources} + assert source_names == {"flake.nix"} + # --------------------------------------------------------------------------- # Validator._bootstrap_nix_darwin() — idempotency From 31a17820c52e48073e6f7e0f49fee375ecfbc9a0 Mon Sep 17 00:00:00 2001 From: Will Gordon Date: Sat, 8 Aug 2026 10:54:38 -0400 Subject: [PATCH 32/33] chore(deps): scopes pytest collection and adds op-cli test tier Restricts collection to tests/ (previously unset, so pytest scanned the whole repo tree including gitignored scratch scripts) and adds scripts/ to pythonpath so tests/test_prewarm_vm.py can import it directly. Also registers the new op_cli marker and excludes it from the default run. --- Makefile | 7 ++++++- pyproject.toml | 6 ++++-- 2 files changed, 10 insertions(+), 3 deletions(-) diff --git a/Makefile b/Makefile index 57cb0c8..47803be 100644 --- a/Makefile +++ b/Makefile @@ -1,5 +1,5 @@ .DEFAULT_GOAL := all -.PHONY: install lint format typecheck test test-integration test-vm test-nix test-nix-darwin-switch prewarm-vm pull-base-vm test-quick clean all prek-install prek +.PHONY: install lint format typecheck test test-integration test-vm test-nix test-nix-darwin-switch test-op-cli prewarm-vm pull-base-vm test-quick clean all prek-install prek install: uv sync @@ -47,6 +47,11 @@ test-nix: test-nix-darwin-switch: uv run pytest -m nix_darwin_switch --tb=long +# Requires a real, signed-in `op` CLI and MAC2NIX_TEST_OP_VAULT set to a disposable +# test vault — skips otherwise (see tests/test_onepassword.py's op_test_vault fixture). +test-op-cli: + uv run pytest -m op_cli --tb=long + prewarm-vm: uv run python scripts/prewarm_vm.py diff --git a/pyproject.toml b/pyproject.toml index 7811339..2934887 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -75,14 +75,15 @@ include = ["src"] exclude = ["**/__pycache__"] [tool.pytest.ini_options] -pythonpath = ["src"] +testpaths = ["tests"] +pythonpath = ["src", "scripts"] python_files = "test_*.py" python_classes = "Test*" python_functions = "test_*" addopts = [ "--strict-markers", "--tb=short", - "-m", "not integration and not nix_vm and not nix_build and not nix_darwin_switch", + "-m", "not integration and not nix_vm and not nix_build and not nix_darwin_switch and not op_cli", ] markers = [ "integration: real VM tests — require tart + sshpass + base VM image", @@ -90,6 +91,7 @@ markers = [ "nix_vm: real VM-based apply-and-verify tests — require tart", "nix_build: real nix flake lock/build tests — never skipped, require nix + age + sops + network", "nix_darwin_switch: real nix-darwin switch applied to the running machine — CI-only, skips unless GITHUB_ACTIONS=true", + "op_cli: real 1Password CLI integration — requires op on PATH, signed in, and MAC2NIX_TEST_OP_VAULT set", ] cache_dir = ".cache/pytest" From 72c0df291022f50a4a5bbf6a7fa08002f81b9e56 Mon Sep 17 00:00:00 2001 From: Will Gordon Date: Sat, 8 Aug 2026 11:14:53 -0400 Subject: [PATCH 33/33] fix(cli): mocks crypto in confirm-or-default eof test MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit test_real_eof_at_trailing_prompts_defaults_and_completes used _redirect_age_keys, which still shells out to the real age-keygen binary — not installed on the lint-and-test CI runner (only nix-integration/nix-darwin-switch install it), so the test failed there while passing locally. This test verifies CLI prompt/exception handling, not crypto behavior, so it should never have needed a real binary in the first place; switches to the same generate_age_key/ _create_host_secrets_file mocking the rest of the unguarded suite uses. --- tests/cli/test_add_host.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/tests/cli/test_add_host.py b/tests/cli/test_add_host.py index 775ca17..9b9a6cd 100644 --- a/tests/cli/test_add_host.py +++ b/tests/cli/test_add_host.py @@ -344,7 +344,8 @@ def test_real_eof_at_trailing_prompts_defaults_and_completes(self, tmp_path: Pat runner = CliRunner() with ( - _redirect_age_keys(tmp_path / "age-keys"), + patch("mac2nix.generators.scaffold.generate_age_key", return_value="age1myhost-fingerprint"), + patch("mac2nix.generators.scaffold._create_host_secrets_file"), patch("mac2nix.cli._run_nix_flake_lock") as mock_lock, ): result = runner.invoke(