From d5081b0868cce02fab3e8057e8acffc4298e07d2 Mon Sep 17 00:00:00 2001 From: Alex Soffronow-Pagonidis Date: Tue, 28 Jul 2026 11:12:42 +0200 Subject: [PATCH 1/2] Auto-migrate legacy config into workspace/config (scrub + backup) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit One idempotent routine copies the legacy layout into /config/, leaving the originals as breadcrumbs, and scrubs secrets on the way: known sensitive keys move to the machine-local overlay and the tracked file gets an ${ENV_VAR} reference in their place. A value that cannot be classified confidently stays local rather than being committed, and migration prints exactly what moved. It runs from upgrade and daemon start, no-ops once migrated, and --dry-run mutates nothing. Migration is isolable — it honours NERVE_HOME instead of reaching for real machine state — crash-safe, and writes owner-only. A config.yaml that is already the machine half is not migrated a second time, and a workflow journal directory is not mistaken for shareable config. Rewriting a file no longer re-permissions it. Co-Authored-By: Claude --- docs/config.md | 61 ++ nerve/agent/backends/codex/ultracode.py | 19 +- nerve/cli.py | 100 ++- nerve/migrate.py | 759 ++++++++++++++++ nerve/utils/fs.py | 118 +++ tests/test_migrate.py | 1078 +++++++++++++++++++++++ tests/test_utils_fs.py | 184 ++++ 7 files changed, 2305 insertions(+), 14 deletions(-) create mode 100644 nerve/migrate.py create mode 100644 nerve/utils/fs.py create mode 100644 tests/test_migrate.py create mode 100644 tests/test_utils_fs.py diff --git a/docs/config.md b/docs/config.md index e5e7b97f..d448afbd 100644 --- a/docs/config.md +++ b/docs/config.md @@ -188,6 +188,67 @@ is too old to read shows up as a startup warning on the instance and in `nerve doctor`. If a key is ever retired upstream, `--strict-keys` flags it here first; drop the key, or install the ref you deploy (`nerve @ git+...@v1.2.3`) instead. +## Migrating an Existing Install + +Installs from before the workspace-config layout are migrated automatically and +idempotently on `nerve upgrade` and daemon start. To run it by hand: + +```bash +nerve migrate --dry-run # show what would change +nerve migrate # apply +``` + +Nothing is deleted and the effective configuration does not change; values are +only relocated. Originals are renamed to `*.migrated` breadcrumbs, and an existing +breadcrumb is never overwritten. + +- `config.yaml` moves to the git-tracked `workspace/config/settings.yaml`. Secret + values are moved to machine-local `config.local.yaml` and replaced with + `${ENV_VAR}` placeholders. The machine-local `workspace` path also stays in + `config.local.yaml`. +- The legacy `~/.nerve/cron/*` moves to `workspace/config/cron/*`, including any + `prompts/` referenced by `prompt_file`. +- `config.local.yaml` and the breadcrumb hold plaintext secrets, so both are + written `0600`. `settings.yaml` is written normally, so a restrictive `umask` is + honored. + +A value is treated as secret when any of these hold: + +- The key name looks like one: `*api_key*`, `*api_hash*`, `api_id`, `*token*`, + `*secret*`, `password*`, `jwt`, `authorization`, `bearer`, `oauth`, + `*access_key*`, `*private_key*`, `dsn`, `pw`, `pat`, `session_string`, + `webhook_url`. A public identifier is not a secret: `client_id` is left alone, + `client_secret` is not. +- The value's shape is a credential whatever the key is called: `sk-…`, `ghp_…`, + `xox…`, `user:password@host`, `?token=…`, `Bearer …`. This fires only when the + whole value is the credential, so a category description that mentions + `postgres://user:pass@host` stays put. +- It sits inside an `env` or `headers` block, including inside lists, where MCP + `headers` entries live. +- It is the argument after a flag that names a credential, so both + `args: ["--token=abc123"]` and the split `args: ["--token", "abc123"]` are + caught. The split form is what `npx` and `uvx` MCP servers use, and the value + on its own is unrecognizable. + +If one item in a list is a secret, the whole list moves, because a merge replaces +a list rather than combining it element-wise. Migration reports this, and the copy +left in `settings.yaml` has no further effect. + +Secret detection is best-effort, so **review `workspace/config/settings.yaml` +before committing it to a shared repo**, then run `nerve config validate` to +confirm the bundle is well-formed. Migration also lists any value it left in the +tracked file that still looks like a credential, for you to judge. + +Migration only runs on a pre-refactor `config.yaml`, meaning one that holds +shareable settings rather than only this box's. A `config.yaml` written by +`nerve init` under the current layout holds only machine-local keys (workspace, +bind address, provider handles), so it is left in place even when the workspace has +no `settings.yaml`. Migration is also a no-op once `settings.yaml` carries real +keys; the `nerve init` scaffold is all comments and counts as empty. A +`config.yaml` with shareable keys sitting next to a populated `settings.yaml` is +reported, because it overrides the tracked file, and you move those keys across by +hand. + ## Config Directory Resolution `nerve` commands locate the config directory via a waterfall, so they work diff --git a/nerve/agent/backends/codex/ultracode.py b/nerve/agent/backends/codex/ultracode.py index 47ddc2f3..ac91dd69 100644 --- a/nerve/agent/backends/codex/ultracode.py +++ b/nerve/agent/backends/codex/ultracode.py @@ -21,6 +21,8 @@ from pathlib import Path from typing import Any +from nerve.utils.fs import atomic_write_text + logger = logging.getLogger(__name__) _MARKETPLACE = "nerve-ultracode" @@ -93,15 +95,6 @@ def managed_dir(home: str | Path) -> Path: return Path(home).expanduser() / "nerve-managed" / "ultracode" -def _atomic_write(path: Path, content: str, mode: int = 0o600) -> None: - path.parent.mkdir(parents=True, exist_ok=True) - tmp = path.with_name(f".{path.name}.{os.getpid()}.tmp") - tmp.write_text(content, encoding="utf-8") - os.chmod(tmp, mode) - tmp.replace(path) - os.chmod(path, mode) - - def materialize_worker_wrapper(home: str | Path) -> Path: """Write an owner-only executable pointing at Nerve's wrapper module.""" path = managed_dir(home) / "codex-worker" @@ -110,7 +103,7 @@ def materialize_worker_wrapper(home: str | Path) -> Path: "from nerve.agent.backends.codex.worker_wrapper import main\n" "raise SystemExit(main())\n" ) - _atomic_write(path, script, mode=0o700) + atomic_write_text(path, script, mode=0o700) return path @@ -123,7 +116,7 @@ def _replace_overlay(path: Path, old: str, new: str) -> None: f"Pinned Ultracode source drifted; policy overlay does not match {path}" ) mode = path.stat().st_mode & 0o777 - _atomic_write(path, content.replace(old, new), mode=mode) + atomic_write_text(path, content.replace(old, new), mode=mode) def _sha256(path: Path) -> str: @@ -156,7 +149,7 @@ def apply_policy_overlay(config: Any, plugin_root: str | Path) -> dict[str, Any] "scripts/ultracode-script-runner.js": _sha256(script_runner), }, } - _atomic_write( + atomic_write_text( managed_dir(config.codex.home_dir) / "policy-overlay.json", json.dumps(marker, indent=2, sort_keys=True) + "\n", ) @@ -318,7 +311,7 @@ async def ensure_installed(config: Any) -> dict[str, Any]: root = managed_dir(config.codex.home_dir) / "marketplace" manifest_path = root / ".agents" / "plugins" / "marketplace.json" - _atomic_write( + atomic_write_text( manifest_path, json.dumps(_marketplace_manifest(config), indent=2) + "\n", ) diff --git a/nerve/cli.py b/nerve/cli.py index cc758fc1..177ff7c4 100644 --- a/nerve/cli.py +++ b/nerve/cli.py @@ -172,7 +172,7 @@ def _docker_compose( # Commands that must keep working when the config itself is broken -- they are # how an operator diagnoses or repairs it. They receive ctx.obj["config"] = None # and ctx.obj["config_error"], and are responsible for reporting. -_SELF_DIAGNOSING_COMMANDS = frozenset({"config", "doctor", "init"}) +_SELF_DIAGNOSING_COMMANDS = frozenset({"config", "doctor", "init", "migrate"}) @click.group() @@ -276,6 +276,25 @@ def start(ctx: click.Context, foreground: bool) -> None: config_dir = Path(ctx.obj["config_dir"]) config = ctx.obj["config"] + # Migrate a legacy install to the workspace/config layout if needed + # (idempotent, best-effort). Non-destructive — originals kept as *.migrated. + if config is not None: + from nerve.migrate import maybe_migrate + report = maybe_migrate(config_dir, workspace=getattr(config, "workspace", None)) + if report is not None and report.did_anything: + # The config in hand was loaded from the old locations, and migration + # has just moved the files out from under it — cron in particular now + # lives in the workspace, so the stale object resolves to job files + # that no longer exist and a --foreground run would schedule nothing. + try: + config = load_config(config_dir) + except Exception as e: + raise click.ClickException( + f"Config could not be reloaded after migration: {e}" + ) from e + set_config(config) + ctx.obj["config"] = config + # Detect fresh install — offer to run setup wizard from nerve.bootstrap import is_fresh_install if is_fresh_install(config_dir): @@ -726,6 +745,41 @@ def upgrade(ctx: click.Context, no_frontend: bool, no_deps: bool, no_pull: bool) if rc != 0: raise click.ClickException("npm run build failed") + # Migrate a legacy config layout to workspace/config (idempotent). + if config is not None: + from nerve.migrate import maybe_migrate + report = maybe_migrate( + Path(ctx.obj["config_dir"]), workspace=getattr(config, "workspace", None) + ) + if report and report.did_anything: + click.echo("\nMigrated config to the workspace layout:") + for action in report.actions: + click.echo(f" - {action}") + if report.error: + # The steps above did apply. Saying so beats a bare success + # message, because the rest has to be finished by hand or by a + # re-run, and the tracked file already exists either way. + click.secho( + f" Migration did not finish: {report.error}\n" + " What is listed above was applied; re-run `nerve migrate` " + "once the cause is fixed.", + fg="yellow", + ) + if report.secrets_moved: + click.echo( + f" (scrubbed {len(report.secrets_moved)} secret(s) into " + "config.local.yaml; review workspace/config/settings.yaml " + "before committing)" + ) + if report.suspect_values: + click.secho( + f" ({len(report.suspect_values)} value(s) left in settings.yaml " + f"still look like credentials: {', '.join(report.suspect_values)})", + fg="yellow", + ) + for warning in report.warnings: + click.secho(f" Note: {warning}", fg="yellow") + click.echo("\nUpgrade complete. Restart Nerve for changes to take effect:") click.echo(" nerve restart") @@ -1092,6 +1146,50 @@ def doctor(ctx: click.Context) -> None: ctx.exit(1) +@main.command() +@click.option("--dry-run", is_flag=True, help="Show what would change without writing.") +@click.pass_context +def migrate(ctx: click.Context, dry_run: bool) -> None: + """Migrate a legacy config layout into the workspace/config subtree. + + Copies config.yaml → workspace/config/settings.yaml (scrubbing secrets into + config.local.yaml as ${ENV_VAR} refs) and ~/.nerve/cron → workspace/config/cron. + Non-destructive (originals kept as *.migrated) and idempotent. + """ + from nerve.migrate import migrate as run_migrate + + config = ctx.obj["config"] + config_dir = Path(ctx.obj["config_dir"]) + workspace = getattr(config, "workspace", None) if config is not None else None + try: + report = run_migrate(config_dir, workspace=workspace, dry_run=dry_run) + except Exception as e: # e.g. a malformed config.local.yaml + raise click.ClickException(f"Migration failed: {e}") from e + + if not report.did_anything: + click.secho("Nothing to migrate — already on the workspace layout.", fg="green") + for warning in report.warnings: + click.secho(f" Note: {warning}", fg="yellow") + return + prefix = "[dry-run] would " if dry_run else "" + for action in report.actions: + click.echo(f" {prefix}{action}") + for warning in report.warnings: + click.secho(f"\n Note: {warning}", fg="yellow") + if report.secrets_moved: + click.echo(f"\n Secrets scrubbed to config.local.yaml: {', '.join(report.secrets_moved)}") + if report.suspect_values: + click.secho( + f"\n {len(report.suspect_values)} value(s) headed for settings.yaml still " + f"look like credentials — review: {', '.join(report.suspect_values)}", + fg="yellow", + ) + if dry_run: + click.secho("\nDry run — no changes written.", fg="yellow") + else: + click.secho("\nMigration complete. Review workspace/config/settings.yaml before committing.", fg="green") + + @main.group(name="config") def config_group() -> None: """Configuration commands.""" diff --git a/nerve/migrate.py b/nerve/migrate.py new file mode 100644 index 00000000..1723aa0f --- /dev/null +++ b/nerve/migrate.py @@ -0,0 +1,759 @@ +"""One-time migration of legacy config into the git-syncable workspace subtree. + +Moves an existing install from the pre-refactor layout: + + /config.yaml (shareable + secrets, gitignored) + /config.local.yaml (secrets, gitignored) + ~/.nerve/cron/{jobs,system}.yaml (cron config) + +to the workspace-centric layout: + + /config/settings.yaml (shareable, git-tracked — SCRUBBED) + /config.local.yaml (secrets, machine-local) + /config/cron/* (cron config, git-tracked) + +Design (confirmed with the user): + +* **Auto on upgrade / daemon start**, via :func:`maybe_migrate` — idempotent, + a no-op once migrated. Also exposed as ``nerve migrate [--dry-run]``. +* **Only a legacy monolith is migrated.** A ``config.yaml`` holding nothing but + the keys the split layout keeps machine-local is left exactly where it is, + however empty the workspace's ``settings.yaml`` looks — see + :func:`_has_portable_content`. +* **Copy + keep as backup** — originals are never deleted; ``config.yaml`` and + the legacy cron files are renamed to ``*.migrated`` breadcrumbs, and the new + location wins. An existing breadcrumb is never overwritten. +* **Auto-scrub secrets** — before writing the *tracked* ``settings.yaml``, secret + values are moved into machine-local ``config.local.yaml`` and replaced with + ``${ENV_VAR}`` placeholders. Scrubbed: values under secret-looking keys + (see :data:`_SECRET_KEY_RE`), values whose *shape* is a credential whatever the + key is called (``sk-…``, ``ghp_…``, ``user:pass@host``, ``?token=…``), *every* + value inside ``env`` / ``headers`` mappings (where arbitrarily-named secrets + live, e.g. MCP ``Authorization`` headers), and secrets nested inside lists. + Migration prints exactly what it moved, plus anything left behind that still + looks credential-shaped; heuristics can't be exhaustive, so **review + settings.yaml before committing**. + +Never destructive and never raises out of :func:`maybe_migrate` (best-effort on +startup). + +**Isolating a migration.** Three roots are read and written, and only two of +them are obvious from the call: + +* ``config_dir`` — the first argument. +* the workspace — the ``workspace`` argument; when omitted it is resolved from + the machine-local config files, falling back to ``paths.default_workspace()``. +* the legacy cron directory — the ``legacy_cron_dir`` argument; when omitted it + is ``paths.cron_dir()``, i.e. under ``NERVE_HOME``. + +Passing ``workspace=`` alone therefore does **not** sandbox anything: the cron +half still reads, copies and *renames* files under the real state directory. +Callers that must not touch the machine — tests, tooling, a dry run against +someone else's tree — have to pass ``legacy_cron_dir=`` as well (or set +``NERVE_HOME``). +""" + +from __future__ import annotations + +import logging +import os +import re +import shutil +from dataclasses import dataclass, field +from pathlib import Path + +import yaml + +from nerve import paths +from nerve.config import ( + _deep_merge, + _expand_path, + _read_yaml_mapping, + workspace_config_dir, + workspace_settings_file, +) +from nerve.utils.fs import atomic_write_text + +logger = logging.getLogger(__name__) + +# Anything holding a credential is owner-only, including the breadcrumb copy of +# the pre-migration config — it still has every secret in plaintext. +_SECRET_FILE_MODE = 0o600 + +# Leaf key names whose values are treated as secrets and scrubbed out of the +# tracked settings file. Matched against the *normalized* key (see +# :func:`_normalize_key`), and every alternative has to cover whole +# underscore-separated runs: a substring match would read ``max_tokens`` (a +# size) as a token and ``client_idle_timeout_minutes`` (a duration) as a client +# id. Keys ending in "_env" hold an env-var *name* — a reference, not a secret — +# and are left alone. +# +# ``client_id`` is deliberately absent: an OAuth client id is public by design, +# and scrubbing it turns a shareable value into a required ``${VAR}`` that no +# other machine can resolve. ``client_secret`` is caught by ``secret``. +_SECRET_KEY_RE = re.compile( + r"(?:^|_)(?:" + r"api_?key|api_?hash|api_?id|access_?key|private_?key|secret_?key" + r"|secret|token|password|passwd|passphrase|credentials?|jwt|authorization" + r"|bearer|oauth|pw|pat|dsn" + r"|session_?(?:string|token|secret|id)" + r"|webhook_?url" + r")(?:_|$)" +) + +# Numbers are credentials far less often than strings are — a config is full of +# sizes, ports and timeouts under names that brush against the list above. So a +# non-string leaf is only scrubbed when the key *ends* in one of these, which +# keeps ``telegram.api_id`` (half of a Telegram credential pair, and an int) +# while leaving ``max_tokens`` and ``default_token_budget`` in place. +_NUMERIC_SECRET_KEY_RE = re.compile( + r"(?:^|_)(?:api_?id|app_?id|api_?key|api_?hash|access_?key|secret|token" + r"|password|passwd|pw|pat)$" +) + +# Values whose *shape* is a credential, whatever the key is called. This is the +# half a key-name list can never do: a key pasted into an ``args`` list, a token +# in a URL's query string, a password inside a DSN. The lookbehind stops the +# provider prefixes from matching mid-word (``task-management-system`` is not an +# ``sk-`` key). +_SECRET_VALUE_RE = re.compile( + r"(? bool: + return self.migrated_config or self.migrated_cron + + +def _env_name(path: tuple[str, ...]) -> str: + """Derive an ENV_VAR name from a dotted config path (auth.jwt_secret → + AUTH_JWT_SECRET).""" + joined = "_".join(path) + return re.sub(r"[^A-Za-z0-9]+", "_", joined).strip("_").upper() + + +def _normalize_key(key) -> str: + """``apiKey`` / ``API-KEY`` / ``api.key`` → ``api_key``. + + Config written by hand (and MCP server blocks copied from vendor docs) uses + every spelling; normalizing once means the pattern lists only have to know + about one. + """ + camel_split = re.sub(r"(?<=[a-z0-9])(?=[A-Z])", "_", str(key)) + return re.sub(r"[^a-z0-9]+", "_", camel_split.lower()).strip("_") + + +def _value_looks_secret(value: str) -> bool: + if _SECRET_VALUE_RE.search(value) or _BEARER_VALUE_RE.search(value): + return True + return not any(c.isspace() for c in value) and bool(_SECRET_VALUE_OPAQUE_RE.search(value)) + + +def _is_cli_flag(item) -> bool: + """True for an argv item that is a flag name rather than a value.""" + return isinstance(item, str) and item.startswith("-") + + +def _is_secret_leaf(key, value, path: tuple[str, ...], force: bool) -> bool: + """True if this leaf should be moved out of the tracked file. + + ``force`` marks a leaf inside a subtree that is sensitive by definition + (``env`` / ``headers``), where the key names are the user's own and tell us + nothing. + """ + # Scalars only. A bool is never a credential, and containers are walked by + # the caller. + if isinstance(value, bool) or not isinstance(value, (str, int, float)): + return False + if isinstance(value, str) and (not value or _FULL_ENV_REF_RE.match(value)): + return False + if ".".join(path) in _SCRUB_EXCLUDE_PATHS: + return False + norm = _normalize_key(key) + if force: + return True + if norm.endswith("_env"): + return False # holds an env-var name, not a secret + if isinstance(value, str): + return bool(_SECRET_KEY_RE.search(norm)) or _value_looks_secret(value) + return bool(_NUMERIC_SECRET_KEY_RE.search(norm)) + + +def _scrub_secrets( + data: dict, path: tuple[str, ...] = (), force: bool = False +) -> tuple[dict, dict, list[str]]: + """Split a config dict into (tracked, secrets, moved_paths). + + ``tracked`` is safe to commit (secret leaf values replaced with + ``${ENV_VAR}``). ``secrets`` is the parallel structure holding the real + values, to be merged into config.local.yaml. ``force`` scrubs every scalar + leaf regardless of key name (used inside sensitive subtrees like env/headers). + """ + tracked: dict = {} + secrets: dict = {} + moved: list[str] = [] + for key, value in data.items(): + p = path + (str(key),) + child_force = force or (_normalize_key(key) in _SENSITIVE_SUBTREE_KEYS) + if isinstance(value, dict): + t, s, m = _scrub_secrets(value, p, force=child_force) + tracked[key] = t + if s: + secrets[key] = s + moved.extend(m) + elif isinstance(value, list): + t_list, has_secret = [], False + after_secret_flag = False + for i, item in enumerate(value): + item_path = p + (str(i),) + if isinstance(item, dict): + t, s, m = _scrub_secrets(item, item_path, force=child_force) + t_list.append(t) + if s or m: + has_secret = True + moved.extend(m) + after_secret_flag = False + continue + # A scalar list item has no key of its own, so it is judged by + # the list's key, by its own shape, or by the item in front of + # it — which is how ``headers: ["Authorization: Bearer …"]``, + # ``args: ["--api-key=…"]`` and ``args: ["--token", "…"]`` are + # all caught. + positional = after_secret_flag and not _is_cli_flag(item) + if _is_secret_leaf(key, item, item_path, child_force or positional): + t_list.append("${" + _env_name(item_path) + "}") + has_secret = True + moved.append(".".join(item_path)) + after_secret_flag = False + else: + t_list.append(item) + # Only a flag arms the next item. Another flag does not: + # ``--token --verbose`` is a malformed command line, not a + # token whose value is ``--verbose``. + after_secret_flag = isinstance(item, str) and bool( + _SECRET_FLAG_RE.match(item) + ) + tracked[key] = t_list + if has_secret: + # The whole real list goes to the overlay, not just the secret + # items. Merging replaces a list rather than combining it + # element-wise, so there is no way for the overlay to supply + # item 3 and let the tracked file keep items 1 and 2 — it is all + # or nothing, and the tracked copy is inert from here on. + # :func:`_relocated_lists` surfaces that so it isn't a surprise. + secrets[key] = value + elif _is_secret_leaf(key, value, p, force): + tracked[key] = "${" + _env_name(p) + "}" + secrets[key] = value + moved.append(".".join(p)) + else: + tracked[key] = value + return tracked, secrets, moved + + +def _suspect_values(data, path: tuple[str, ...] = ()) -> list[str]: + """Dotted paths of tracked values that still look credential-shaped. + + Nothing is moved on this signal — it is too weak to act on and too strong to + swallow. Reporting it is the difference between "scrubbed 3 secrets" (which + a user reads as "and that was all of them") and a prompt to look at the four + opaque strings the key-name rules had no opinion about. + """ + out: list[str] = [] + items = data.items() if isinstance(data, dict) else enumerate(data) + for key, value in items: + p = path + (str(key),) + if isinstance(value, (dict, list)): + out.extend(_suspect_values(value, p)) + elif isinstance(value, str) and _OPAQUE_VALUE_RE.match(value): + if any(c.isdigit() for c in value) and any(c.isalpha() for c in value): + out.append(".".join(p)) + return out + + +def _relocated_lists(secrets: dict, path: tuple[str, ...] = ()) -> list[str]: + """Dotted paths of lists moved to the overlay whole because one item was a + secret. The copy left in the tracked file no longer has any effect.""" + out: list[str] = [] + for key, value in secrets.items(): + p = path + (str(key),) + if isinstance(value, list): + out.append(".".join(p)) + elif isinstance(value, dict): + out.extend(_relocated_lists(value, p)) + return out + + +def _leaf_paths(data, path: tuple[str, ...] = ()) -> list[str]: + """Dotted paths of every value in a config mapping. Lists count as leaves — + the split layout routes a whole list one way or the other.""" + out: list[str] = [] + for key, value in data.items(): + p = path + (str(key),) + if isinstance(value, dict) and value: + out.extend(_leaf_paths(value, p)) + else: + out.append(".".join(p)) + return out + + +def _is_machine_local(dotted: str) -> bool: + return any( + dotted == known or dotted.startswith(known + ".") for known in _MACHINE_LOCAL_PATHS + ) + + +def _has_portable_content(raw: dict) -> bool: + """True if ``config.yaml`` holds anything the shareable layer should own. + + The positive test for "this is a legacy monolith". Emptiness of + ``settings.yaml`` can't answer it: a workspace loses its settings file by + being repointed, moved, emptied, or interrupted mid-init, and in every one of + those cases the ``config.yaml`` sitting next to it is the deliberately + machine-local half — the last thing that should be copied into a file the + docs tell you to commit and push. + """ + return any(not _is_machine_local(p) for p in _leaf_paths(raw)) + + +def _resolve_workspace(config_dir: Path) -> Path: + machine = _deep_merge( + _read_yaml_mapping(config_dir / "config.yaml"), + _read_yaml_mapping(config_dir / "config.local.yaml"), + ) + return _expand_path(machine.get("workspace")) or paths.default_workspace() + + +def _breadcrumb_path(original: Path) -> Path: + """A free ``*.migrated`` name next to ``original``. + + ``Path.rename`` silently overwrites on POSIX (and raises on Windows), and the + cron half of the migration can run more than once — so a fixed suffix could + destroy the only surviving copy of an earlier original. + """ + candidate = original.with_name(original.name + ".migrated") + n = 1 + while candidate.exists(): + candidate = original.with_name(f"{original.name}.migrated.{n}") + n += 1 + return candidate + + +def _restrict(path: Path) -> None: + """Make a file owner-only, best-effort (some filesystems have no modes).""" + try: + os.chmod(path, _SECRET_FILE_MODE) + except OSError as e: + logger.warning("Could not restrict permissions on %s: %s", path, e) + + +def migrate( + config_dir: Path, + workspace: Path | None = None, + dry_run: bool = False, + legacy_cron_dir: Path | None = None, + report: MigrationReport | None = None, +) -> MigrationReport: + """Perform the migration for ``config_dir``. Idempotent; safe to re-run. + + ``workspace`` defaults to the one the machine-local config names, and + ``legacy_cron_dir`` to ``paths.cron_dir()``. Both have to be supplied to + confine the migration to a given tree — see the module docstring. + + Pass ``report`` to keep hold of it if this raises. Migration is not one + transaction: the config half can commit and the cron half then fail on a + directory it cannot write, and a caller that only sees the exception has no + way to know the files already moved. + """ + config_dir = Path(config_dir) + workspace = Path(workspace) if workspace is not None else _resolve_workspace(config_dir) + legacy_cron = Path(legacy_cron_dir) if legacy_cron_dir is not None else paths.cron_dir() + report = MigrationReport(dry_run=dry_run) if report is None else report + + _migrate_config_yaml(config_dir, workspace, report) + _migrate_cron(workspace, legacy_cron, report) + return report + + +def _settings_has_content(settings: Path) -> bool: + """True if the tracked settings file already carries configuration. + + Existence is not the test: ``nerve init`` scaffolds a comments-only + ``settings.yaml``, which parses to an empty mapping. Treating that as + "already migrated" made migration a permanent no-op for every install + created after the scaffold shipped. + + A file that won't parse — or parses to something that isn't a mapping — + counts as content: migration must never overwrite what it cannot read. + """ + if not settings.exists(): + return False + try: + return bool(_read_yaml_mapping(settings, strict=True)) + except Exception: # noqa: BLE001 — unreadable is "leave it alone" + return True + + +def _migrate_config_yaml(config_dir: Path, workspace: Path, report: MigrationReport) -> None: + config_yaml = config_dir / "config.yaml" + settings = workspace_settings_file(workspace) + + if not config_yaml.exists(): + return + + raw = _read_yaml_mapping(config_yaml) + + if _settings_has_content(settings): + # Already on the split layout. If config.yaml still carries shareable + # keys they silently mask the tracked file, which is also what an + # interrupted migration leaves behind — the two states are + # indistinguishable on disk, so say so rather than guess. + if _has_portable_content(raw): + report.warnings.append( + f"{config_yaml} is still present and overrides {settings}; " + "move any shared settings across and remove it" + ) + return + + if not _has_portable_content(raw): + return # a machine-local config.yaml from the split layout, not a legacy one + + # The workspace location is machine-local — it must not live in the tracked + # settings file (circular) but must be preserved so config still resolves the + # workspace after config.yaml is renamed away. Keep it in config.local.yaml. + ws_value = raw.pop("workspace", None) + + # ``lockdown`` is honored only in the tracked settings file — the machine + # layers deliberately ignore it so that a local edit cannot unlock or + # fake-lock an instance (see :func:`nerve.config._load_layers`). Copying it + # across would promote a flag that has never had any effect into the one file + # where it is authoritative, and locking drops the machine layers entirely, + # including the config.local.yaml this migration just moved the secrets into: + # the next start would fail on an unresolved ${VAR} it used to resolve. So it + # is dropped, which preserves what the box does today, and reported. + if raw.pop("lockdown", None) is not None: + report.warnings.append( + f"dropped 'lockdown' from {config_yaml}: it was being ignored there and " + f"is honored only in {settings}, where it also stops the machine-local " + "layers from being read. Set it there deliberately to lock this instance" + ) + + tracked, secrets, moved = _scrub_secrets(raw) + + # Everything bound for the machine-local overlay: scrubbed secrets + the + # workspace path (not a secret, but machine-specific). + local_additions = dict(secrets) + if ws_value is not None: + local_additions["workspace"] = ws_value + + local_path = config_dir / "config.local.yaml" + backup = _breadcrumb_path(config_yaml) + + report.migrated_config = True + report.secrets_moved.extend(moved) + report.suspect_values.extend(_suspect_values(tracked)) + if local_additions: + report.actions.append(f"moved secrets + workspace path into {local_path}") + report.actions.append(f"config.yaml → {settings} (scrubbed {len(moved)} secret(s))") + report.actions.append(f"config.yaml → {backup} (backup)") + for dotted in _relocated_lists(secrets): + report.warnings.append( + f"{dotted} contained a secret, so the whole list moved to " + f"{local_path.name} — a list can't be half-overridden, and the copy " + "left in settings.yaml no longer has any effect" + ) + + if report.dry_run: + return + + # Order matters, and both writes are atomic (temp file + rename), so an + # interruption at any point leaves a working install: + # + # 1. the local overlay first, so the tracked file never references secrets + # that exist nowhere on this machine; + # 2. the tracked settings file; + # 3. only then rename config.yaml away. + # + # Stopping between 2 and 3 leaves config.yaml still shadowing an already + # complete settings.yaml — the instance keeps working, and re-running is a + # no-op. Renaming earlier would invert that: a crash before the settings + # file existed would take every non-secret setting out of the live config + # with no way to retry. + if local_additions: + existing_local = _read_yaml_mapping(local_path) + # Existing local values win — never clobber a value the operator already + # placed there. + merged_local = _deep_merge(local_additions, existing_local) + atomic_write_text( + local_path, + "# Nerve — machine-local secrets & overrides (gitignored).\n\n" + + yaml.safe_dump(merged_local, default_flow_style=False, sort_keys=False), + mode=_SECRET_FILE_MODE, + ) + + atomic_write_text( + settings, + "# Nerve — shareable workspace configuration (migrated).\n" + "# Secrets were moved to config.local.yaml and replaced with\n" + "# ${ENV_VAR} placeholders. Safe to commit.\n\n" + + yaml.safe_dump(tracked, default_flow_style=False, sort_keys=False), + # The shareable file gets whatever mode an ordinary write would have + # produced. Forcing it open would override a restrictive umask on a file + # that scrubbing is not guaranteed to have emptied of credentials. + mode=None, + ) + + # Rename the original as a breadcrumb so it no longer overrides settings.yaml. + # It keeps every secret in plaintext — unscrubbed, unlike the file we just + # locked down — so tighten it first, then move it. + _restrict(config_yaml) + config_yaml.rename(backup) + + +def _migrate_cron(workspace: Path, legacy: Path, report: MigrationReport) -> None: + """Copy the legacy cron directory into the workspace, one file at a time. + + Per file rather than all-or-nothing, because the two sides legitimately + overlap: ``nerve init`` writes ``system.yaml`` and an empty ``gates/`` into + the workspace while copying only ``jobs.yaml`` across (see + :mod:`nerve.bootstrap`). A directory-level "the workspace already has cron + config" test therefore skipped the remainder permanently, and the legacy + directory stops being consulted as soon as the workspace has job files (see + :func:`nerve.config._resolve_cron_dir`) — so ``gates/*.py`` and any + ``prompts/`` a job names by relative path were stranded where nothing reads + them, and custom gates quietly stopped loading. Losing a gate is not a soft + failure either: an unknown gate ``type`` takes the whole job with it. + + A file already in the workspace is never overwritten; that copy is the + reviewed one. The exception worth reporting is a legacy *job* file that loses + to one already in place, because it is real cron config that now runs + nowhere and migration cannot merge two sets of jobs. + """ + if not legacy.is_dir(): + return + ws_cron = workspace_config_dir(workspace) / "cron" + job_files = ("system.yaml", "jobs.yaml") + + copy: list[Path] = [] + shadowed: list[Path] = [] + for src in sorted(legacy.rglob("*")): + if src.is_dir(): + continue + rel = src.relative_to(legacy) + if any(".migrated" in Path(part).suffixes for part in rel.parts): + continue # a breadcrumb from an earlier pass, not cron content + if not (ws_cron / rel).exists(): + copy.append(rel) + elif rel.parent == Path(".") and rel.name in job_files: + shadowed.append(rel) + + # Reported whether or not anything else moves, the way a leftover config.yaml + # is: nothing here can fix it, and the jobs in it are not running. + for rel in shadowed: + report.warnings.append( + f"{legacy / rel} still holds cron jobs, but {ws_cron / rel} is already " + "present and wins; move across what you need and remove it" + ) + if not copy: + return + + report.migrated_cron = True + report.actions.append( + f"cron {legacy}/* → {ws_cron}/ ({len(copy)} file(s), originals kept as *.migrated)" + ) + + if report.dry_run: + return + + for rel in copy: + dst = ws_cron / rel + dst.parent.mkdir(parents=True, exist_ok=True) + shutil.copy2(legacy / rel, dst) + # Breadcrumb only the job files that actually moved, so the legacy directory + # stops reading as "has jobs". Renaming one that lost to a workspace copy + # would hide the operator's only copy of it. + for name in job_files: + if Path(name) in copy: + (legacy / name).rename(_breadcrumb_path(legacy / name)) + + +def is_migrated( + config_dir: Path, + workspace: Path | None = None, + legacy_cron_dir: Path | None = None, +) -> bool: + """True if there is nothing left to migrate for ``config_dir``.""" + return not migrate( + config_dir, workspace=workspace, dry_run=True, legacy_cron_dir=legacy_cron_dir + ).did_anything + + +def maybe_migrate( + config_dir: Path, + workspace: Path | None = None, + legacy_cron_dir: Path | None = None, +) -> MigrationReport | None: + """Run migration if needed. Best-effort: never raises (called on startup). + + Returns the report if migration ran, else None. A failure partway still + returns what was applied, rather than reading as "nothing happened": the + config half commits before the cron half runs, so an exception can leave the + tracked settings file written, the secrets relocated and ``config.yaml`` + renamed away. Callers need that — ``nerve start`` has to reload the config it + is holding, and ``nerve upgrade`` has to print the review prompt for a + tracked file that now exists. + """ + report = MigrationReport(dry_run=False) + try: + migrate( + config_dir, + workspace=workspace, + dry_run=False, + legacy_cron_dir=legacy_cron_dir, + report=report, + ) + except Exception as e: # noqa: BLE001 — must never break upgrade/startup + report.error = str(e) + if report.did_anything: + logger.warning( + "Config migration failed partway and is half-applied (%s). Already " + "done: %s", + e, + "; ".join(report.actions), + ) + else: + logger.warning("Config migration skipped due to error: %s", e) + if report.did_anything: + if not report.error: + logger.info( + "Migrated config to the workspace layout: %s", + "; ".join(report.actions), + ) + if report.suspect_values: + logger.warning( + "Migration left %d value(s) in the tracked settings file that look " + "like credentials — review before committing: %s", + len(report.suspect_values), + ", ".join(report.suspect_values), + ) + for warning in report.warnings: + logger.warning("Config migration: %s", warning) + return report diff --git a/nerve/utils/fs.py b/nerve/utils/fs.py new file mode 100644 index 00000000..c76b6a27 --- /dev/null +++ b/nerve/utils/fs.py @@ -0,0 +1,118 @@ +"""Filesystem helpers shared by the writers that must not lose or leak data. + +Kept dependency-free (stdlib only) so any module can import it. +""" + +from __future__ import annotations + +import os +import tempfile +from pathlib import Path + + +def atomic_write_text(path: Path | str, content: str, *, mode: int | None = 0o600) -> None: + """Replace ``path`` with ``content`` in one step, at ``mode``. + + ``mode=None`` means "whatever ``open(path, 'w')`` would have left behind": + a file that already exists keeps the permissions it already has, and a new + one is created at ``0o666`` minus the process umask. Use it for files that + are meant to be shared; pass an explicit mode for anything holding a + credential. + + Three properties the obvious ``open(path, "w")`` does not have: + + * **A crash never truncates the destination.** The bytes go to a temp file + in the same directory, are flushed and fsynced, and only then does + ``os.replace`` swap them in — an atomic rename on POSIX and on Windows. + An interrupted write (power loss, ENOSPC, SIGKILL) leaves either the + complete old file or the complete new one, never a half of either. + * **The permissions are in place before the content is.** The temp file is + created owner-only by ``mkstemp`` and chmod'd *before* the first write, + so a secret is never briefly readable at the process umask. Chmod'ing + after the write is a race, however short. + * **The rename itself is durable.** The parent directory is fsynced too, + so a crash right after the call cannot resurrect the old file. + + One thing it does *not* preserve: hard links. Swapping in a new file gives + the destination a new inode, so any other name for the old one keeps the + old content. Symlinks are followed (the link's target is rewritten, as a + plain write would do), but a hard-linked destination cannot be updated in + place and atomically at the same time. + + ``path``'s parent is created if missing. The temp file is removed if + anything goes wrong, so a failed write leaves no debris. + """ + # Follow symlinks to the real destination. Otherwise a config file the user + # linked into a dotfiles repo would be *replaced* by a regular file and the + # repo copy would silently keep the old content. + path = Path(os.path.realpath(path)) + path.parent.mkdir(parents=True, exist_ok=True) + if mode is None: + # Only a *new* file takes its mode from the umask; rewriting an existing + # one through ``open(path, "w")`` leaves the permissions alone. This + # helper writes a fresh inode and renames it into place, so the old mode + # has to be carried across by hand or every write silently re-permissions + # the file: a 0o600 config widened to 0o666 under a lax umask, or a + # deliberately group-readable one clamped to 0o600 under a strict one. + # The widening direction is the dangerous one — these are the files that + # hold credentials. + mode = _existing_file_mode(path) + if mode is None: + mode = _default_file_mode() + fd, tmp_name = tempfile.mkstemp(dir=path.parent, prefix=f".{path.name}.", suffix=".tmp") + tmp = Path(tmp_name) + try: + with os.fdopen(fd, "w", encoding="utf-8") as f: + try: + os.fchmod(f.fileno(), mode) + except AttributeError: # no fchmod on Windows + os.chmod(tmp, mode) + f.write(content) + f.flush() + os.fsync(f.fileno()) + os.replace(tmp, path) + except BaseException: + tmp.unlink(missing_ok=True) + raise + _fsync_dir(path.parent) + + +def _existing_file_mode(path: Path) -> int | None: + """The permission bits already on ``path``, or ``None`` if there is no file. + + Only the low nine bits travel. Set-user-ID, set-group-ID and the sticky bit + belong to the inode being discarded, and re-applying them to a file whose + contents this process just wrote is how an ordinary config write turns into + a privilege hand-out. + """ + try: + return os.stat(path).st_mode & 0o777 + except OSError: + return None # absent, or unstattable — fall back to what open() would do + + +def _default_file_mode() -> int: + """The mode ``open()`` would create a file with, honoring the umask. + + There is no getter for the umask, only a setter that returns the previous + value — so it has to be read by writing it. The value parked in the window + is deliberately restrictive: if another thread creates a file in those few + instructions, it lands too tight rather than too loose. + """ + current = os.umask(0o077) + os.umask(current) + return 0o666 & ~current + + +def _fsync_dir(directory: Path) -> None: + """Flush a directory entry, ignoring platforms that can't open one.""" + try: + fd = os.open(directory, os.O_RDONLY) + except OSError: + return # Windows, and some network filesystems + try: + os.fsync(fd) + except OSError: + pass + finally: + os.close(fd) diff --git a/tests/test_migrate.py b/tests/test_migrate.py new file mode 100644 index 00000000..1298a697 --- /dev/null +++ b/tests/test_migrate.py @@ -0,0 +1,1078 @@ +"""Tests for legacy → workspace/config migration.""" + +import os +from pathlib import Path +from unittest.mock import patch + +import pytest +import yaml +from click.testing import CliRunner + +import nerve.migrate +from nerve import paths +from nerve.config import load_config, workspace_settings_file +from nerve.cron.jobs import load_jobs +from nerve.migrate import ( + _scrub_secrets, + _suspect_values, + is_migrated, + maybe_migrate, + migrate, +) + + +def _legacy_install(tmp_path, config_yaml: str, local_yaml: str | None = None): + config_dir = tmp_path / "cfg" + workspace = tmp_path / "ws" + config_dir.mkdir(parents=True) + workspace.mkdir(parents=True) + (config_dir / "config.yaml").write_text( + f"workspace: {workspace}\n" + config_yaml, encoding="utf-8" + ) + if local_yaml is not None: + (config_dir / "config.local.yaml").write_text(local_yaml, encoding="utf-8") + return config_dir, workspace + + +def _legacy_cron(tmp_path, files: dict[str, str]) -> Path: + """A stand-in for the machine-global cron dir, inside the test's tmp tree.""" + d = tmp_path / "legacy-cron" + d.mkdir(parents=True, exist_ok=True) + for name, body in files.items(): + (d / name).write_text(body, encoding="utf-8") + return d + + +def _mode(path: Path) -> int: + return path.stat().st_mode & 0o777 + + +def _snapshot(root: Path) -> dict[str, bytes | None]: + """Every path under ``root`` and its contents (``None`` for directories).""" + return { + str(p.relative_to(root)): None if p.is_dir() else p.read_bytes() + for p in sorted(root.rglob("*")) + } + + +@pytest.fixture +def permissive_umask(): + """Run with the common umask 022, under which a plain write lands 0644.""" + old = os.umask(0o022) + yield + os.umask(old) + + +class TestScrubSecrets: + def test_moves_secret_keys(self): + tracked, secrets, moved = _scrub_secrets({ + "anthropic_api_key": "sk-real", + "auth": {"jwt_secret": "hunter2", "password_hash": "$2b$12$x"}, + "timezone": "UTC", + }) + assert tracked["anthropic_api_key"] == "${ANTHROPIC_API_KEY}" + assert tracked["auth"]["jwt_secret"] == "${AUTH_JWT_SECRET}" + assert tracked["auth"]["password_hash"] == "${AUTH_PASSWORD_HASH}" + assert tracked["timezone"] == "UTC" + assert secrets["anthropic_api_key"] == "sk-real" + assert secrets["auth"]["jwt_secret"] == "hunter2" + assert set(moved) == {"anthropic_api_key", "auth.jwt_secret", "auth.password_hash"} + + def test_leaves_env_refs_and_env_name_keys(self): + tracked, secrets, moved = _scrub_secrets({ + "openai_api_key": "${OPENAI_API_KEY}", # already a ref + "api_key_env": "OPENAI_API_KEY", # a name, not a secret + }) + assert tracked["openai_api_key"] == "${OPENAI_API_KEY}" + assert tracked["api_key_env"] == "OPENAI_API_KEY" + assert secrets == {} + + def test_sizes_and_timeouts_are_not_credentials(self): + """Names that brush against the secret vocabulary but hold quantities.""" + tracked, secrets, moved = _scrub_secrets({ + "max_tokens": 4096, + "default_token_budget": 120000, + "client_idle_timeout_minutes": 30, + "telegram_chat_id": 12345, + }) + assert secrets == {} + assert tracked["max_tokens"] == 4096 + assert tracked["default_token_budget"] == 120000 + assert tracked["client_idle_timeout_minutes"] == 30 + assert tracked["telegram_chat_id"] == 12345 + + def test_numeric_credential_scrubbed(self): + """Half a credential pair is no better than none: Telegram's api_id is an + int, and scrubbing only its api_hash sibling left it in the clear.""" + tracked, secrets, moved = _scrub_secrets({ + "telegram": {"api_id": 2040123, "api_hash": "0123456789abcdef"} + }) + assert tracked["telegram"]["api_id"] == "${TELEGRAM_API_ID}" + assert secrets["telegram"]["api_id"] == 2040123 + assert moved == ["telegram.api_id", "telegram.api_hash"] + + def test_partial_env_ref_is_still_scrubbed(self): + """One ${} reference anywhere used to exempt the whole value.""" + tracked, secrets, _ = _scrub_secrets({ + "github": {"token": "ghp_realsecretvalue${SUFFIX}"} + }) + assert tracked["github"]["token"] == "${GITHUB_TOKEN}" + assert secrets["github"]["token"] == "ghp_realsecretvalue${SUFFIX}" + + def test_scalar_list_items_scrubbed(self): + """A list of strings is a leaf too — inside a sensitive subtree every + item is a secret, and elsewhere the value's own shape decides.""" + tracked, secrets, moved = _scrub_secrets({ + "mcp": { + "headers": ["Authorization: Bearer ghp_AAAABBBBCCCCDDDDEEEE"], + "args": ["--stdio", "--api-key=sk-proj-AAAABBBBCCCCDDDDEEEE"], + } + }) + assert tracked["mcp"]["headers"] == ["${MCP_HEADERS_0}"] + assert tracked["mcp"]["args"] == ["--stdio", "${MCP_ARGS_1}"] + # Lists don't deep-merge — the overlay carries the real list wholesale. + assert secrets["mcp"]["args"][1] == "--api-key=sk-proj-AAAABBBBCCCCDDDDEEEE" + assert set(moved) == {"mcp.headers.0", "mcp.args.1"} + + def test_credential_after_a_flag_is_scrubbed(self): + """``--token`` and its value are separate argv items, which is the form + ``npx``/``uvx`` MCP servers are configured with. The value on its own has + no vendor prefix and no separator to match on, so the flag in front of it + is what marks it — otherwise it goes to the tracked file in plaintext.""" + tracked, secrets, moved = _scrub_secrets({ + "mcp_servers": [ + { + "name": "s", + "command": "npx", + "args": ["-y", "srv", "--token", "tok_abcdefghijklmnop"], + }, + ] + }) + assert moved == ["mcp_servers.0.args.3"] + assert tracked["mcp_servers"][0]["args"] == [ + "-y", "srv", "--token", "${MCP_SERVERS_0_ARGS_3}", + ] + assert secrets["mcp_servers"][0]["args"][3] == "tok_abcdefghijklmnop" + + def test_a_flag_does_not_make_a_following_flag_a_secret(self): + """``--token --verbose`` is a malformed command line, not a token whose + value is ``--verbose``. Scrubbing it would put a required ``${VAR}`` + where a switch used to be, and the server would stop starting.""" + tracked, _, moved = _scrub_secrets( + {"args": ["--token", "--verbose", "--port", "8080"]} + ) + assert moved == [] + assert tracked["args"] == ["--token", "--verbose", "--port", "8080"] + + def test_only_a_credential_naming_flag_arms_the_next_item(self): + """The flag has to name a credential. Every argv list has values after + flags, and treating all of them as secrets would empty the tracked file + into the overlay.""" + tracked, _, moved = _scrub_secrets( + {"args": ["-y", "server-name", "--port", "8080", "--model", "opus"]} + ) + assert moved == [] + assert tracked["args"] == ["-y", "server-name", "--port", "8080", "--model", "opus"] + + def test_credential_shaped_value_under_innocuous_key(self): + tracked, secrets, moved = _scrub_secrets({ + "database": {"url": "postgres://admin:hunter2@db.internal:5432/nerve"}, + "feed": {"url": "https://example.com/all?token=abcdef1234567890"}, + "repo": {"url": "https://github.com/owner/repo.git"}, + }) + assert tracked["database"]["url"] == "${DATABASE_URL}" + assert tracked["feed"]["url"] == "${FEED_URL}" + assert tracked["repo"]["url"] == "https://github.com/owner/repo.git" + + def test_alternate_key_spellings(self): + tracked, secrets, _ = _scrub_secrets({"apiKey": "abc", "API-TOKEN": "def"}) + assert tracked["apiKey"] == "${APIKEY}" + assert tracked["API-TOKEN"] == "${API_TOKEN}" + + def test_sqlite_dsn_not_scrubbed(self): + """"dsn" usually means a credential, but this one is a local file path.""" + tracked, secrets, _ = _scrub_secrets( + {"memory": {"sqlite_dsn": "sqlite:////home/me/.nerve/memu.sqlite"}} + ) + assert tracked["memory"]["sqlite_dsn"].startswith("sqlite:") + assert secrets == {} + + def test_paths_are_not_pats(self): + """Segment-anchored matching: ``pat`` must not swallow ``*_path``.""" + tracked, secrets, _ = _scrub_secrets({ + "archive_path": "/var/lib/nerve", + "redact_patterns": ["sk-[a-z]+"], + "public_key": "ssh-ed25519 AAAAC3NzaC1lZDI1NTE5", + }) + assert secrets == {} + + def test_public_identifiers_are_not_scrubbed(self): + """An OAuth client id is public by design. Scrubbing it turns a + shareable value into a required ${VAR} that hard-fails load_config on + the second machine — the one the tracked layer exists for.""" + tracked, secrets, _ = _scrub_secrets({ + "external_agents": {"codex": {"client_id": "app-12345", "client_secret": "shh"}} + }) + codex = tracked["external_agents"]["codex"] + assert codex["client_id"] == "app-12345" + assert codex["client_secret"].startswith("${") + + def test_prose_mentioning_a_credential_is_not_scrubbed(self): + """Category descriptions and help text are shareable. Only a value that + *is* a credential counts, not a sentence that mentions the shape of one + — the wizard generates descriptions like these.""" + tracked, secrets, _ = _scrub_secrets({ + "memory": {"categories": [ + {"name": "infra", + "description": "Connection strings like postgres://user:pass@host and " + "CLI flags such as --api-key=YOURKEYHERE live here"}, + ]}, + "help": "Append ?token=YOURTOKEN to the URL to authenticate", + }) + assert secrets == {} + assert "postgres://user:pass@host" in tracked["memory"]["categories"][0]["description"] + assert tracked["help"].startswith("Append ?token=") + + def test_suspect_values_flags_what_the_rules_missed(self): + """No rule matches an opaque blob under an innocuous key — but the + operator gets told about it rather than a bare "scrubbed 0 secrets".""" + tracked, _, _ = _scrub_secrets({ + "analytics": {"project": "5f4dcc3b5aa765d61d8327deb882cf99"}, + "agent": {"model": "claude-haiku-4-5-20251001"}, + "timezone": "America/New_York", + }) + assert _suspect_values(tracked) == ["analytics.project"] + + def test_authorization_header_scrubbed(self): + tracked, secrets, moved = _scrub_secrets({ + "mcp_servers": {"gh": {"headers": {"Authorization": "Bearer sk-leak"}}} + }) + hdr = tracked["mcp_servers"]["gh"]["headers"]["Authorization"] + assert hdr.startswith("${") and "sk-leak" not in hdr + assert secrets["mcp_servers"]["gh"]["headers"]["Authorization"] == "Bearer sk-leak" + + def test_env_subtree_scrubbed_regardless_of_key_name(self): + tracked, secrets, moved = _scrub_secrets({ + "mcp_servers": {"x": {"env": {"GH_PAT": "ghp_leak", "OPENAI_KEY": "sk-leak"}}} + }) + env = tracked["mcp_servers"]["x"]["env"] + assert env["GH_PAT"].startswith("${") and env["OPENAI_KEY"].startswith("${") + assert secrets["mcp_servers"]["x"]["env"]["GH_PAT"] == "ghp_leak" + + def test_secret_inside_list_scrubbed(self): + tracked, secrets, moved = _scrub_secrets({ + "external_agents": {"targets": [{"name": "codex", "token": "tok-leak"}]} + }) + assert tracked["external_agents"]["targets"][0]["token"].startswith("${") + # Full real list preserved in the overlay (lists don't deep-merge). + assert secrets["external_agents"]["targets"] == [ + {"name": "codex", "token": "tok-leak"} + ] + + def test_api_hash_scrubbed(self): + tracked, secrets, _ = _scrub_secrets({"sync": {"telegram": {"api_hash": "H"}}}) + assert tracked["sync"]["telegram"]["api_hash"].startswith("${") + + def test_proxy_api_key_not_scrubbed(self): + # A fixed local-loopback token — must stay in the shared file. + tracked, secrets, _ = _scrub_secrets({"proxy": {"api_key": "sk-nerve-local-proxy"}}) + assert tracked["proxy"]["api_key"] == "sk-nerve-local-proxy" + assert secrets == {} + + +class TestMigrateConfig: + def test_creates_scrubbed_settings_and_moves_secrets(self, tmp_path): + config_dir, workspace = _legacy_install( + tmp_path, + "timezone: UTC\nanthropic_api_key: sk-secret\n", + ) + report = migrate(config_dir, workspace=workspace) + assert report.migrated_config + + settings = yaml.safe_load(workspace_settings_file(workspace).read_text()) + assert settings["timezone"] == "UTC" + assert settings["anthropic_api_key"] == "${ANTHROPIC_API_KEY}" + assert "workspace" not in settings # stripped (circular) + + local = yaml.safe_load((config_dir / "config.local.yaml").read_text()) + assert local["anthropic_api_key"] == "sk-secret" + + # Original kept as a breadcrumb; config.yaml no longer present. + assert (config_dir / "config.yaml.migrated").exists() + assert not (config_dir / "config.yaml").exists() + + def test_effective_config_unchanged_after_migration(self, tmp_path): + """The whole point: values are preserved, just relocated.""" + config_dir, workspace = _legacy_install( + tmp_path, "timezone: Europe/Berlin\nanthropic_api_key: sk-xyz\n" + ) + before = load_config(config_dir) + migrate(config_dir, workspace=workspace) + after = load_config(config_dir) + assert after.timezone == before.timezone == "Europe/Berlin" + assert after.anthropic_api_key == before.anthropic_api_key == "sk-xyz" + + def test_lockdown_is_not_promoted_into_the_tracked_file(self, tmp_path): + """``lockdown`` is ignored in config.yaml deliberately, so that a local + edit cannot unlock or fake-lock an instance. Copying it into settings.yaml + would hand a flag that has never had any effect the authority to drop the + machine layers — including the config.local.yaml this migration just moved + the secrets into, leaving the ${VAR}s in the tracked file resolving to + nothing and the instance refusing to load.""" + config_dir, workspace = _legacy_install( + tmp_path, "timezone: UTC\nlockdown: true\nanthropic_api_key: sk-xyz\n" + ) + report = migrate(config_dir, workspace=workspace) + + settings = yaml.safe_load(workspace_settings_file(workspace).read_text()) + local = yaml.safe_load((config_dir / "config.local.yaml").read_text()) + assert "lockdown" not in settings + assert "lockdown" not in local + assert any("lockdown" in w for w in report.warnings) + # Dropping it preserves what the box already did, so the install loads. + assert load_config(config_dir).anthropic_api_key == "sk-xyz" + + def test_existing_local_secret_not_clobbered(self, tmp_path): + config_dir, workspace = _legacy_install( + tmp_path, + "anthropic_api_key: sk-from-config\n", + local_yaml="openai_api_key: sk-existing-local\n", + ) + migrate(config_dir, workspace=workspace) + local = yaml.safe_load((config_dir / "config.local.yaml").read_text()) + assert local["openai_api_key"] == "sk-existing-local" # preserved + assert local["anthropic_api_key"] == "sk-from-config" # added + + def test_idempotent(self, tmp_path): + config_dir, workspace = _legacy_install(tmp_path, "timezone: UTC\n") + assert migrate(config_dir, workspace=workspace).did_anything + # Second run: settings.yaml now exists → no-op. + assert not migrate(config_dir, workspace=workspace).did_anything + assert is_migrated(config_dir, workspace=workspace) + + def test_settings_with_content_not_migrated(self, tmp_path): + """A workspace whose settings file already carries configuration is left + alone — migration must not rename config.yaml out from under it.""" + config_dir, workspace = _legacy_install(tmp_path, "timezone: UTC\n") + (workspace / "config").mkdir(parents=True) + workspace_settings_file(workspace).write_text("timezone: UTC\n", encoding="utf-8") + report = migrate(config_dir, workspace=workspace) + assert not report.migrated_config + assert (config_dir / "config.yaml").exists() # untouched + + def test_comments_only_scaffold_does_not_block_migration(self, tmp_path): + """``nerve init`` scaffolds a settings.yaml that is nothing but comments. + Treating its mere existence as "already migrated" made migration a + permanent no-op for every install created after the scaffold shipped.""" + config_dir, workspace = _legacy_install(tmp_path, "timezone: Europe/Berlin\n") + (workspace / "config").mkdir(parents=True) + workspace_settings_file(workspace).write_text( + "# Nerve — shareable workspace configuration.\n" + "# Uncomment what you want to share.\n" + "# timezone: UTC\n", + encoding="utf-8", + ) + report = migrate(config_dir, workspace=workspace) + assert report.migrated_config + settings = yaml.safe_load(workspace_settings_file(workspace).read_text()) + assert settings["timezone"] == "Europe/Berlin" + assert (config_dir / "config.yaml.migrated").exists() + + def test_unreadable_settings_blocks_migration(self, tmp_path): + """Never overwrite a tracked file we can't parse.""" + config_dir, workspace = _legacy_install(tmp_path, "timezone: UTC\n") + (workspace / "config").mkdir(parents=True) + workspace_settings_file(workspace).write_text("{ not: valid: yaml\n", encoding="utf-8") + report = migrate(config_dir, workspace=workspace) + assert not report.migrated_config + assert (config_dir / "config.yaml").exists() + + def test_dry_run_writes_nothing(self, tmp_path): + config_dir, workspace = _legacy_install( + tmp_path, "anthropic_api_key: sk-secret\n" + ) + legacy_cron = _legacy_cron(tmp_path, {"jobs.yaml": "jobs: []\n"}) + before = _snapshot(tmp_path) + + report = migrate( + config_dir, workspace=workspace, dry_run=True, legacy_cron_dir=legacy_cron + ) + + assert report.migrated_config and report.migrated_cron # would do both + # Not one byte, and not one directory — the workspace config subtree in + # particular must not be conjured into existence by a dry run. + assert _snapshot(tmp_path) == before + assert not (workspace / "config").exists() + + +class TestMigratePermissions: + """The migrated files carry plaintext secrets; the modes have to say so.""" + + def test_new_local_overlay_is_owner_only(self, tmp_path, permissive_umask): + config_dir, workspace = _legacy_install( + tmp_path, "telegram:\n bot_token: 12345:AAisasecret\n" + ) + migrate(config_dir, workspace=workspace) + assert _mode(config_dir / "config.local.yaml") == 0o600 + + def test_backup_of_the_unscrubbed_original_is_owner_only(self, tmp_path, permissive_umask): + """The breadcrumb still holds *every* secret, unscrubbed, right next to + the file we bothered to lock down.""" + config_dir, workspace = _legacy_install( + tmp_path, "auth:\n jwt_secret: hunter2\n" + ) + os.chmod(config_dir / "config.yaml", 0o644) + migrate(config_dir, workspace=workspace) + backup = config_dir / "config.yaml.migrated" + assert "hunter2" in backup.read_text() # unscrubbed, as designed + assert _mode(backup) == 0o600 + + def test_pre_existing_wide_open_local_file_is_tightened(self, tmp_path, permissive_umask): + config_dir, workspace = _legacy_install( + tmp_path, "anthropic_api_key: sk-a\n", local_yaml="openai_api_key: sk-b\n" + ) + os.chmod(config_dir / "config.local.yaml", 0o644) + migrate(config_dir, workspace=workspace) + assert _mode(config_dir / "config.local.yaml") == 0o600 + + @pytest.mark.parametrize("umask,expected", [(0o022, 0o644), (0o077, 0o600), (0o002, 0o664)]) + def test_tracked_settings_follow_the_umask(self, tmp_path, umask, expected): + """The shareable file is readable from a checkout, but an operator who + set a restrictive umask does not get it widened behind their back — and + scrubbing is explicitly not guaranteed to have emptied it of secrets.""" + config_dir, workspace = _legacy_install(tmp_path, "timezone: UTC\n") + old = os.umask(umask) + try: + migrate(config_dir, workspace=workspace) + finally: + os.umask(old) + assert _mode(workspace_settings_file(workspace)) == expected + + def test_scaffolded_settings_file_keeps_its_own_permissions(self, tmp_path, permissive_umask): + """``nerve init`` scaffolds a comments-only settings.yaml, which migration + is free to fill in — but filling it in is a *rewrite*, and a rewrite of an + existing file does not re-permission it. An operator who locked the + scaffold down must not have it opened up by a migration that happened to + run under a laxer umask.""" + config_dir, workspace = _legacy_install(tmp_path, "timezone: UTC\n") + settings = workspace_settings_file(workspace) + settings.parent.mkdir(parents=True, exist_ok=True) + settings.write_text("# Shared settings. No keys yet.\n", encoding="utf-8") + os.chmod(settings, 0o600) + + migrate(config_dir, workspace=workspace) + + assert yaml.safe_load(settings.read_text())["timezone"] == "UTC" + assert _mode(settings) == 0o600 + + def test_secret_files_ignore_a_permissive_umask(self, tmp_path): + config_dir, workspace = _legacy_install(tmp_path, "anthropic_api_key: sk-a\n") + old = os.umask(0o000) + try: + migrate(config_dir, workspace=workspace) + finally: + os.umask(old) + assert _mode(config_dir / "config.local.yaml") == 0o600 + assert _mode(config_dir / "config.yaml.migrated") == 0o600 + + +class TestMigrateCrashSafety: + def test_pre_existing_local_secrets_survive_a_failed_write(self, tmp_path, monkeypatch): + """config.local.yaml holds secrets that exist in no backup — the wizard's + API key, OAuth token and password hash. A migration that dies partway + must not be what removes them.""" + config_dir, workspace = _legacy_install( + tmp_path, + "timezone: UTC\nanthropic_api_key: sk-from-config\n", + local_yaml="openai_api_key: sk-only-copy\nclaude_oauth_token: tok-only-copy\n", + ) + real_write = nerve.migrate.atomic_write_text + + def fail_on_settings(path, content, **kwargs): + if Path(path) == workspace_settings_file(workspace): + raise OSError("No space left on device") + return real_write(path, content, **kwargs) + + monkeypatch.setattr(nerve.migrate, "atomic_write_text", fail_on_settings) + with pytest.raises(OSError): + migrate(config_dir, workspace=workspace) + monkeypatch.undo() + + local = yaml.safe_load((config_dir / "config.local.yaml").read_text()) + assert local["openai_api_key"] == "sk-only-copy" + assert local["claude_oauth_token"] == "tok-only-copy" + + # And the install is still in a state that a retry can finish, because + # the source file was not renamed away first. + assert (config_dir / "config.yaml").exists() + assert migrate(config_dir, workspace=workspace).migrated_config + local = yaml.safe_load((config_dir / "config.local.yaml").read_text()) + assert local["openai_api_key"] == "sk-only-copy" + assert local["anthropic_api_key"] == "sk-from-config" + + def test_settings_is_written_before_the_source_is_renamed(self, tmp_path, monkeypatch): + """Losing config.yaml before its replacement exists would drop every + non-secret setting out of the live config, unrecoverably.""" + config_dir, workspace = _legacy_install(tmp_path, "timezone: UTC\n") + seen = [] + real_rename = Path.rename + + def spy_rename(self, target): + seen.append(workspace_settings_file(workspace).exists()) + return real_rename(self, target) + + monkeypatch.setattr(Path, "rename", spy_rename) + migrate(config_dir, workspace=workspace) + assert seen == [True] + + def test_wholesale_list_relocation_is_reported(self, tmp_path): + """One secret in a list sends the whole list to the overlay, because a + merge replaces a list rather than combining it element-wise. The copy + left in settings.yaml is inert from then on — editing it does nothing, + so the operator has to be told.""" + config_dir, workspace = _legacy_install( + tmp_path, + "mcp_servers:\n" + " targets:\n" + " - name: gh\n" + " token: ghp_AAAABBBBCCCCDDDDEEEEFFFF\n", + ) + report = migrate(config_dir, workspace=workspace) + assert any("mcp_servers.targets" in w and "no longer has any effect" in w + for w in report.warnings) + + def test_existing_breadcrumb_is_never_overwritten(self, tmp_path): + """The cron half re-runs whenever the legacy dir has jobs again, so a + fixed .migrated suffix could destroy the only copy of the first one.""" + config_dir, workspace = _legacy_install(tmp_path, "timezone: UTC\n") + legacy_cron = _legacy_cron(tmp_path, {"jobs.yaml": "jobs: [] # first\n"}) + migrate(config_dir, workspace=workspace, legacy_cron_dir=legacy_cron) + + # A later run of the daemon writes jobs to the legacy dir again, and the + # workspace copy is gone (moved, or a fresh checkout). + (legacy_cron / "jobs.yaml").write_text("jobs: [] # second\n", encoding="utf-8") + (workspace / "config" / "cron" / "jobs.yaml").unlink() + migrate(config_dir, workspace=workspace, legacy_cron_dir=legacy_cron) + + assert "first" in (legacy_cron / "jobs.yaml.migrated").read_text() + assert "second" in (legacy_cron / "jobs.yaml.migrated.1").read_text() + # ...and the breadcrumbs stay in the legacy dir. They're inert, but the + # workspace cron dir is headed for a shared repo. + assert not list((workspace / "config" / "cron").glob("*.migrated*")) + + +class TestAlreadySplitInstall: + """A ``config.yaml`` written by the split layout holds only what must stay on + this box. Copying it into the shareable file would publish exactly the + values the split exists to keep local — and an absent or emptied + ``settings.yaml`` is not evidence to the contrary, since a workspace loses + one by being repointed, moved, or interrupted mid-init.""" + + # Only the genuinely local half: certificate paths, a credential handle, + # whose mailboxes, this box's helper process and mounts. gateway.host/port + # and provider.type/aws_region are deliberately absent — they are shared + # policy now, so a config.yaml holding them has something to migrate. + MACHINE_ONLY = """ +deployment: server +gateway: + ssl: + cert: /etc/nerve/tls/cert.pem + key: /etc/nerve/tls/key.pem +provider: + aws_profile: nerve-prod +sync: + gmail: + accounts: + - alex@example.com +proxy: + enabled: true + port: 8317 +docker: + extra_mounts: [] +""" + + def test_machine_local_config_is_not_migrated(self, tmp_path): + config_dir, workspace = _legacy_install(tmp_path, self.MACHINE_ONLY) + report = migrate(config_dir, workspace=workspace) + assert not report.migrated_config + assert (config_dir / "config.yaml").exists() + assert not workspace_settings_file(workspace).exists() + + def test_one_shareable_key_is_enough_to_migrate(self, tmp_path): + """The test is "is there anything here the shared layer should own", not + "does this look tidy" — a legacy monolith always has something.""" + config_dir, workspace = _legacy_install(tmp_path, self.MACHINE_ONLY + "timezone: UTC\n") + report = migrate(config_dir, workspace=workspace) + assert report.migrated_config + settings = yaml.safe_load(workspace_settings_file(workspace).read_text()) + assert settings["timezone"] == "UTC" + + def test_a_workflow_journal_dir_does_not_make_the_machine_half_look_legacy( + self, tmp_path, + ): + """A runs directory alone is not a reason to migrate. + + The wizard never writes this section, so the coverage test above cannot + see it. Left uncovered, one absolute path is enough to make a post-split + machine-local ``config.yaml`` read as a legacy monolith — and then the + whole machine half, provider and gateway included, is copied into the + file the docs tell you to commit. + """ + machine_half = self.MACHINE_ONLY + ( + "workflows:\n" + " runs_dir: /srv/box-42/workflow-runs\n" + ) + config_dir, workspace = _legacy_install(tmp_path, machine_half) + report = migrate(config_dir, workspace=workspace) + + assert not report.migrated_config + assert (config_dir / "config.yaml").exists() + assert not workspace_settings_file(workspace).exists() + + def test_machine_local_paths_cover_what_the_wizard_writes(self): + """Keeps the list honest: if init starts routing another key to the + machine layer, migration has to learn about it in the same change.""" + from nerve.bootstrap import SetupWizard + + uncovered = set() + for mode in ("personal", "worker"): + for provider in ("anthropic", "bedrock"): + wizard = SetupWizard(Path("/nonexistent")) + wizard.choices.mode = mode + wizard.choices.provider_type = provider + wizard.choices.aws_region = "us-east-1" + wizard.choices.aws_profile = "nerve-prod" + wizard.choices.use_proxy = True + wizard.choices.deployment = "docker" + machine, _portable, _shadowed = wizard._build_config_layers() + uncovered |= { + p for p in nerve.migrate._leaf_paths(machine) + if not nerve.migrate._is_machine_local(p) + } + assert not uncovered, ( + "the wizard keeps these machine-local but migration would copy them " + f"into the git-tracked settings file: {sorted(uncovered)}" + ) + + def test_machine_local_paths_claim_nothing_the_wizard_shares(self): + """The direction nothing checked, and which had gone wrong. + + A path listed in _MACHINE_LOCAL_PATHS is one migration leaves behind in + config.yaml, so an entry the wizard treats as shareable is not a harmless + over-listing: it strands that value in a layer lockdown does not read. + ``gateway`` and ``provider`` were listed as whole subtrees after the + wizard had already moved host/port and type/aws_region to the tracked + file. + """ + from nerve.bootstrap import SetupWizard + + overclaimed = set() + for provider in ("anthropic", "bedrock"): + wizard = SetupWizard(Path("/nonexistent")) + wizard.choices.mode = "personal" + wizard.choices.provider_type = provider + wizard.choices.aws_region = "us-east-1" + wizard.choices.aws_profile = "nerve-prod" + wizard.choices.use_proxy = True + wizard.choices.deployment = "docker" + _machine, portable, _shadowed = wizard._build_config_layers() + overclaimed |= { + p for p in nerve.migrate._leaf_paths(portable) + if nerve.migrate._is_machine_local(p) + } + assert not overclaimed, ( + "the wizard shares these but migration would strand them in " + f"config.yaml, where lockdown never reads them: {sorted(overclaimed)}" + ) + + def test_a_legacy_bind_port_and_provider_are_migrated(self, tmp_path): + """Values the old classification stranded must now move across. + + A pre-split install with a non-default port, or one running on Bedrock, + had both judged machine-local. Migration left them in config.yaml, so + turning on lockdown reverted the box to 0.0.0.0:8900 and the Anthropic + API with no indication that it had. + """ + config_dir, workspace = _legacy_install( + tmp_path, + "timezone: UTC\n" + "gateway:\n host: 127.0.0.1\n port: 9100\n" + " ssl:\n cert: /etc/nerve/tls/cert.pem\n" + "provider:\n type: bedrock\n aws_region: eu-west-1\n" + " aws_profile: nerve-prod\n", + ) + report = migrate(config_dir, workspace=workspace) + assert report.migrated_config + + settings = yaml.safe_load( + workspace_settings_file(workspace).read_text(encoding="utf-8") + ) + assert settings["gateway"]["host"] == "127.0.0.1" + assert settings["gateway"]["port"] == 9100 + assert settings["provider"]["type"] == "bedrock" + assert settings["provider"]["aws_region"] == "eu-west-1" + + def test_migration_still_publishes_machine_local_keys(self, tmp_path): + """Known gap, pinned here so that changing it is a deliberate act. + + ``_MACHINE_LOCAL_PATHS`` only gates whether a config.yaml is a legacy + monolith (``_has_portable_content``); it does not filter what gets copied, + so migration moves the whole file. A pre-split install with TLS paths + publishes them into a git-tracked settings file, and syncing that to a box + without the certificate makes its gateway fail to bind. Narrowing the list + neither caused nor fixes this: the fix is for migration to split the file + instead of renaming it wholesale, which is a larger change. + """ + config_dir, workspace = _legacy_install( + tmp_path, + "timezone: UTC\n" + "gateway:\n port: 9100\n ssl:\n cert: /etc/nerve/tls/cert.pem\n" + "provider:\n type: bedrock\n aws_profile: nerve-prod\n", + ) + migrate(config_dir, workspace=workspace) + settings = yaml.safe_load( + workspace_settings_file(workspace).read_text(encoding="utf-8") + ) + assert settings["gateway"]["ssl"]["cert"] == "/etc/nerve/tls/cert.pem" + assert settings["provider"]["aws_profile"] == "nerve-prod" + + def test_the_external_agents_block_is_machine_local_too(self, tmp_path): + """A second writer appends to config.yaml after the wizard has paired + this box's external agents, so it has to be covered as well.""" + from types import SimpleNamespace + + from nerve.bootstrap import SetupWizard + + (tmp_path / "config.yaml").write_text("deployment: server\n", encoding="utf-8") + wizard = SetupWizard(tmp_path) + wizard._record_external_agents_in_config_yaml([SimpleNamespace(agent="codex")]) + + raw = yaml.safe_load((tmp_path / "config.yaml").read_text()) + assert "external_agents" in raw # the writer really did run + assert not nerve.migrate._has_portable_content(raw) + + def test_lingering_config_yaml_is_reported_not_ignored(self, tmp_path): + """What an interruption at the rename leaves behind, and what a + hand-managed install can look like. Indistinguishable on disk, so say + so instead of guessing.""" + config_dir, workspace = _legacy_install(tmp_path, "timezone: UTC\n") + (workspace / "config").mkdir(parents=True) + workspace_settings_file(workspace).write_text("timezone: Europe/Berlin\n", encoding="utf-8") + + report = migrate(config_dir, workspace=workspace) + + assert not report.migrated_config + assert any("still present" in w for w in report.warnings) + + def test_no_warning_for_an_ordinary_split_install(self, tmp_path): + config_dir, workspace = _legacy_install(tmp_path, self.MACHINE_ONLY) + (workspace / "config").mkdir(parents=True) + workspace_settings_file(workspace).write_text("timezone: UTC\n", encoding="utf-8") + assert migrate(config_dir, workspace=workspace).warnings == [] + + +class TestMigrationIsolation: + """``workspace=`` alone never sandboxed the cron half — it resolved its + source from the machine-global state dir and *renamed* files there.""" + + def test_scoped_migration_leaves_the_machine_cron_dir_untouched(self, tmp_path): + machine_cron = paths.cron_dir() + machine_cron.mkdir(parents=True, exist_ok=True) + (machine_cron / "jobs.yaml").write_text("jobs: [] # real\n", encoding="utf-8") + (machine_cron / "system.yaml").write_text("jobs: [] # real\n", encoding="utf-8") + + config_dir, workspace = _legacy_install(tmp_path, "timezone: UTC\n") + legacy_cron = _legacy_cron(tmp_path, {"jobs.yaml": "jobs: [] # scoped\n"}) + report = migrate(config_dir, workspace=workspace, legacy_cron_dir=legacy_cron) + + assert report.migrated_cron + assert (machine_cron / "jobs.yaml").exists() + assert (machine_cron / "system.yaml").exists() + assert not list(machine_cron.glob("*.migrated")) + # The scoped directory is the one that moved. + assert "scoped" in (workspace / "config" / "cron" / "jobs.yaml").read_text() + + def test_override_threads_through_the_dry_run_helpers(self, tmp_path): + config_dir, workspace = _legacy_install(tmp_path, "timezone: UTC\n") + legacy_cron = _legacy_cron(tmp_path, {"jobs.yaml": "jobs: []\n"}) + empty = tmp_path / "no-cron-here" + assert not is_migrated(config_dir, workspace=workspace, legacy_cron_dir=legacy_cron) + maybe_migrate(config_dir, workspace=workspace, legacy_cron_dir=empty) + # Config migrated, cron did not — the override was honored by both. + assert workspace_settings_file(workspace).exists() + assert not (workspace / "config" / "cron").exists() + assert (legacy_cron / "jobs.yaml").exists() + + +class TestPlantedSecretCorpus: + """A legacy config carrying one of every plausible secret shape. What + matters is the bytes that land in the git-tracked file.""" + + CORPUS = """ +timezone: UTC +anthropic_api_key: sk-ant-api03-AAAABBBBCCCCDDDDEEEEFFFFGGGGHHHH +auth: + jwt_secret: hunter2hunter2hunter2 +github: + token: "ghp_MixedRefAAAABBBBCCCCDDDDEEEE${GH_SUFFIX}" +slack: + webhook_url: https://hooks.slack.com/services/T000/B000/XXXXXXXXXXXXXXXXXXXX +database: + url: postgres://admin:sup3rs3cr3t@db.internal:5432/nerve +mcp_servers: + gh: + args: ["--stdio", "--api-key=sk-proj-AAAABBBBCCCCDDDDEEEEFFFF"] + headers: ["Authorization: Bearer ghp_AAAABBBBCCCCDDDDEEEEFFFFGGGG"] + feed: + url: https://feeds.example.com/all?token=abcdef1234567890abcdef +telegram_sync: + api_id: 2040123 + api_hash: 0123456789abcdef0123456789abcdef +sentry: + dsn: https://0123456789abcdef0123456789abcdef@o12345.ingest.sentry.io/9876 +smtp: + host: smtp.example.com + pw: correct-horse-battery-staple + pat: pat_AAAABBBBCCCCDDDDEEEE +telethon: + session_string: 1BVtsOHYBu4XyZAbCdEfGhIjKlMnOpQrStUvWx0123456789 +worker: + env: ["GH_PAT=ghp_WWWWXXXXYYYYZZZZAAAABBBB"] +""" + + PLANTED = [ + "sk-ant-api03-AAAABBBBCCCCDDDDEEEEFFFFGGGGHHHH", + "hunter2hunter2hunter2", + "ghp_MixedRefAAAABBBBCCCCDDDDEEEE", + "T000/B000/XXXXXXXXXXXXXXXXXXXX", + "sup3rs3cr3t", + "sk-proj-AAAABBBBCCCCDDDDEEEEFFFF", + "ghp_AAAABBBBCCCCDDDDEEEEFFFFGGGG", + "token=abcdef1234567890abcdef", + "2040123", + "0123456789abcdef0123456789abcdef", + "correct-horse-battery-staple", + "pat_AAAABBBBCCCCDDDDEEEE", + "1BVtsOHYBu4XyZAbCdEfGhIjKlMnOpQrStUvWx0123456789", + "ghp_WWWWXXXXYYYYZZZZAAAABBBB", + ] + + def test_no_planted_secret_reaches_the_tracked_file(self, tmp_path): + config_dir, workspace = _legacy_install(tmp_path, self.CORPUS) + migrate(config_dir, workspace=workspace) + + tracked = workspace_settings_file(workspace).read_text() + leaked = [s for s in self.PLANTED if s in tracked] + assert not leaked, f"{len(leaked)} secret(s) leaked into settings.yaml: {leaked}" + + def test_every_planted_secret_is_still_readable_after_migration(self, tmp_path): + """Scrubbing must relocate, not destroy — the daemon still resolves the + real values from the machine-local overlay.""" + config_dir, workspace = _legacy_install(tmp_path, self.CORPUS) + migrate(config_dir, workspace=workspace) + + overlay = (config_dir / "config.local.yaml").read_text() + missing = [s for s in self.PLANTED if s not in overlay] + assert not missing, f"lost in migration: {missing}" + + +class TestMigrateCron: + def test_copies_legacy_cron_to_workspace(self, tmp_path): + config_dir, workspace = _legacy_install(tmp_path, "timezone: UTC\n") + legacy_cron = paths.cron_dir() + legacy_cron.mkdir(parents=True, exist_ok=True) + (legacy_cron / "jobs.yaml").write_text( + "jobs:\n - id: mine\n schedule: 1h\n prompt: hi\n", encoding="utf-8" + ) + report = migrate(config_dir, workspace=workspace) + assert report.migrated_cron + + ws_jobs = workspace / "config" / "cron" / "jobs.yaml" + assert ws_jobs.exists() + assert "mine" in ws_jobs.read_text() + # Original kept as breadcrumb. + assert (legacy_cron / "jobs.yaml.migrated").exists() + + def test_copies_cron_prompts_dir(self, tmp_path): + config_dir, workspace = _legacy_install(tmp_path, "timezone: UTC\n") + legacy_cron = paths.cron_dir() + (legacy_cron / "prompts").mkdir(parents=True, exist_ok=True) + (legacy_cron / "prompts" / "daily.md").write_text("do it", encoding="utf-8") + (legacy_cron / "jobs.yaml").write_text( + "jobs:\n - id: j\n schedule: 1h\n prompt_file: prompts/daily.md\n", + encoding="utf-8", + ) + migrate(config_dir, workspace=workspace) + # The referenced prompt file came along, so the relative path still resolves. + assert (workspace / "config" / "cron" / "prompts" / "daily.md").exists() + + def test_no_cron_migration_when_workspace_has_jobs(self, tmp_path): + config_dir, workspace = _legacy_install(tmp_path, "timezone: UTC\n") + legacy_cron = paths.cron_dir() + legacy_cron.mkdir(parents=True, exist_ok=True) + (legacy_cron / "jobs.yaml").write_text("jobs: []\n", encoding="utf-8") + ws_cron = workspace / "config" / "cron" + ws_cron.mkdir(parents=True) + (ws_cron / "jobs.yaml").write_text("jobs: []\n", encoding="utf-8") + report = migrate(config_dir, workspace=workspace) + assert not report.migrated_cron + assert not (legacy_cron / "jobs.yaml.migrated").exists() + # Silence would leave the operator with two job files, one of which has + # stopped running: the workspace copy wins and the legacy dir is dropped. + assert any("still holds cron jobs" in w for w in report.warnings) + + def test_gates_and_prompts_migrate_past_the_files_nerve_init_wrote(self, tmp_path): + """``nerve init`` writes ``system.yaml`` and an empty ``gates/`` into the + workspace and copies only ``jobs.yaml`` across. A directory-level "the + workspace already has cron config" test therefore skipped everything + else permanently — and the legacy dir is never read again once the + workspace has job files, so custom gates stopped loading. A job whose + gate ``type`` no longer resolves does not run at all.""" + config_dir, workspace = _legacy_install(tmp_path, "timezone: UTC\n") + legacy_cron = paths.cron_dir() + (legacy_cron / "gates").mkdir(parents=True, exist_ok=True) + (legacy_cron / "gates" / "stale_tasks.py").write_text("CUSTOM = 1\n", encoding="utf-8") + (legacy_cron / "prompts").mkdir(parents=True, exist_ok=True) + (legacy_cron / "prompts" / "daily.md").write_text("do it", encoding="utf-8") + (legacy_cron / "jobs.yaml").write_text("jobs: []\n", encoding="utf-8") + + ws_cron = workspace / "config" / "cron" + (ws_cron / "gates").mkdir(parents=True) # the init placeholder + (ws_cron / "system.yaml").write_text("jobs: []\n", encoding="utf-8") + (ws_cron / "jobs.yaml").write_text("jobs: []\n", encoding="utf-8") + + report = migrate(config_dir, workspace=workspace) + + assert report.migrated_cron + assert (ws_cron / "gates" / "stale_tasks.py").read_text() == "CUSTOM = 1\n" + assert (ws_cron / "prompts" / "daily.md").exists() + + def test_an_existing_workspace_file_is_kept_and_the_rest_still_moves(self, tmp_path): + """Never overwrite: the workspace copy is the reviewed one. That has to + hold per file rather than per directory, or one same-named gate blocks + every other gate in the directory.""" + config_dir, workspace = _legacy_install(tmp_path, "timezone: UTC\n") + legacy_cron = paths.cron_dir() + (legacy_cron / "gates").mkdir(parents=True, exist_ok=True) + (legacy_cron / "gates" / "shared.py").write_text("LEGACY = 1\n", encoding="utf-8") + (legacy_cron / "gates" / "only_legacy.py").write_text("OTHER = 1\n", encoding="utf-8") + (legacy_cron / "jobs.yaml").write_text("jobs: []\n", encoding="utf-8") + + ws_cron = workspace / "config" / "cron" + (ws_cron / "gates").mkdir(parents=True) + (ws_cron / "gates" / "shared.py").write_text("REVIEWED = 1\n", encoding="utf-8") + + migrate(config_dir, workspace=workspace) + + assert (ws_cron / "gates" / "shared.py").read_text() == "REVIEWED = 1\n" + assert (ws_cron / "gates" / "only_legacy.py").read_text() == "OTHER = 1\n" + + def test_a_job_file_that_did_not_move_is_not_breadcrumbed(self, tmp_path): + """Renaming a legacy ``jobs.yaml`` that lost to a workspace copy would + hide the operator's only copy of those jobs behind a ``.migrated`` + suffix, while the jobs themselves are already not running.""" + config_dir, workspace = _legacy_install(tmp_path, "timezone: UTC\n") + legacy_cron = paths.cron_dir() + (legacy_cron / "gates").mkdir(parents=True, exist_ok=True) + (legacy_cron / "gates" / "g.py").write_text("G = 1\n", encoding="utf-8") + (legacy_cron / "jobs.yaml").write_text( + "jobs:\n - id: legacy-only\n schedule: 1h\n prompt: hi\n", encoding="utf-8" + ) + ws_cron = workspace / "config" / "cron" + ws_cron.mkdir(parents=True) + (ws_cron / "jobs.yaml").write_text("jobs: []\n", encoding="utf-8") + + report = migrate(config_dir, workspace=workspace) + + assert (ws_cron / "gates" / "g.py").exists() # the gate still moved + assert (legacy_cron / "jobs.yaml").exists() # ...and this stayed put + assert not (legacy_cron / "jobs.yaml.migrated").exists() + assert "legacy-only" in (legacy_cron / "jobs.yaml").read_text() + assert any("still holds cron jobs" in w for w in report.warnings) + + def test_gates_migrate_even_with_no_legacy_job_files(self, tmp_path): + """Job files are not the gate for the directory: an install that pinned + ``cron.jobs_file`` elsewhere still keeps its gate plugins here, and they + are imported from whichever cron dir resolution picks.""" + config_dir, workspace = _legacy_install(tmp_path, "timezone: UTC\n") + legacy_cron = paths.cron_dir() + (legacy_cron / "gates").mkdir(parents=True, exist_ok=True) + (legacy_cron / "gates" / "g.py").write_text("G = 1\n", encoding="utf-8") + + report = migrate(config_dir, workspace=workspace) + + assert report.migrated_cron + assert (workspace / "config" / "cron" / "gates" / "g.py").exists() + + +class TestMaybeMigrate: + def test_never_raises(self, tmp_path): + # A config_dir with nothing — must not raise. + result = maybe_migrate(tmp_path / "nonexistent") + assert result is None or not result.did_anything + + def test_a_failure_partway_still_reports_what_was_applied(self, tmp_path, monkeypatch): + """The config half commits before the cron half runs, so an exception can + leave settings.yaml written, the secrets relocated and config.yaml renamed + away. Reporting that as "nothing happened" is what made a half-applied + migration a silent one: ``upgrade`` printed no review prompt for a tracked + file that now exists and may hold an unscrubbed credential, and ``start`` + skipped the reload it needs because the files moved underneath it.""" + config_dir, workspace = _legacy_install( + tmp_path, "timezone: UTC\nanthropic_api_key: sk-xyz\n" + ) + legacy_cron = _legacy_cron(tmp_path, {"jobs.yaml": "jobs: []\n"}) + + def boom(src, dst, *args, **kwargs): + raise OSError("Read-only file system") + + monkeypatch.setattr(nerve.migrate.shutil, "copy2", boom) + report = maybe_migrate(config_dir, workspace=workspace, legacy_cron_dir=legacy_cron) + + assert report is not None + assert report.did_anything + assert report.error and "Read-only file system" in report.error + # ...and what it reports really did happen. + assert workspace_settings_file(workspace).exists() + assert (config_dir / "config.yaml.migrated").exists() + assert report.secrets_moved == ["anthropic_api_key"] + + +class TestStartAfterMigration: + def test_foreground_start_gets_the_post_migration_config(self, tmp_path): + """``start`` loads config, migrates, then serves. Migration moves cron + into the workspace and renames the legacy files away, so the object + loaded beforehand points at job files that no longer exist — and + ``--foreground`` is the deployed path, where that means the daemon runs + for its whole lifetime with nothing scheduled.""" + from nerve.cli import start + + config_dir, workspace = _legacy_install(tmp_path, "timezone: UTC\n") + legacy_cron = paths.cron_dir() + legacy_cron.mkdir(parents=True, exist_ok=True) + (legacy_cron / "jobs.yaml").write_text( + "jobs:\n - id: mine\n schedule: 1h\n prompt: hi\n", encoding="utf-8" + ) + + stale = load_config(config_dir) + assert stale.cron.jobs_file == legacy_cron / "jobs.yaml" + + served = {} + with ( + patch("nerve.gateway.server.run_server", side_effect=lambda c: served.update(config=c)), + patch("nerve.cli._get_daemon_status", return_value=(False, None)), + patch("nerve.cli._write_pid"), + patch("nerve.cli._remove_pid"), + patch("nerve.bootstrap.is_fresh_install", return_value=False), + ): + result = CliRunner().invoke( + start, + ["--foreground"], + obj={"config": stale, "config_dir": str(config_dir), "verbose": False}, + standalone_mode=False, + ) + + assert result.exit_code == 0, result.output + config = served["config"] + assert config.cron.jobs_file == workspace / "config" / "cron" / "jobs.yaml" + assert [j.id for j in load_jobs(config.cron.jobs_file)] == ["mine"] diff --git a/tests/test_utils_fs.py b/tests/test_utils_fs.py new file mode 100644 index 00000000..170edfde --- /dev/null +++ b/tests/test_utils_fs.py @@ -0,0 +1,184 @@ +"""Tests for the shared atomic file writer.""" + +from __future__ import annotations + +import os +from pathlib import Path + +import pytest + +from nerve.utils.fs import atomic_write_text + + +def _mode(path: Path) -> int: + return path.stat().st_mode & 0o777 + + +@pytest.fixture +def permissive_umask(): + old = os.umask(0o022) + yield + os.umask(old) + + +def test_writes_content_and_creates_parents(tmp_path): + target = tmp_path / "deep" / "nested" / "file.yaml" + atomic_write_text(target, "hello\n") + assert target.read_text(encoding="utf-8") == "hello\n" + + +def test_mode_is_applied_regardless_of_umask(tmp_path, permissive_umask): + target = tmp_path / "secrets.yaml" + atomic_write_text(target, "token: x\n") + assert _mode(target) == 0o600 + + public = tmp_path / "shared.yaml" + atomic_write_text(public, "timezone: UTC\n", mode=0o644) + assert _mode(public) == 0o644 + + +@pytest.mark.parametrize("umask,expected", [(0o022, 0o644), (0o077, 0o600), (0o002, 0o664)]) +def test_mode_none_on_a_new_file_matches_a_plain_write(tmp_path, umask, expected): + """``mode=None`` is for shareable files: same permissions ``open()`` would + have given them, so a restrictive umask is not overridden.""" + old = os.umask(umask) + try: + atomic_write_text(tmp_path / "shared.yaml", "timezone: UTC\n", mode=None) + reference = tmp_path / "reference.yaml" + with open(reference, "w") as f: + f.write("timezone: UTC\n") + finally: + os.umask(old) + assert _mode(tmp_path / "shared.yaml") == expected == _mode(reference) + + +@pytest.mark.parametrize("existing,umask", [(0o600, 0o000), (0o644, 0o077), (0o640, 0o022)]) +def test_mode_none_keeps_an_existing_files_permissions(tmp_path, existing, umask): + """``open(path, "w")`` re-permissions nothing — only a *new* file takes its + mode from the umask. Deriving the mode from the umask on every write would + widen a 0o600 file to 0o666 under a lax umask, silently publishing a file + that had been locked down, and clamp a group-readable one under a strict + one. The rename this helper does must not change what the mode would be.""" + target = tmp_path / "shared.yaml" + reference = tmp_path / "reference.yaml" + for p in (target, reference): + p.write_text("old\n", encoding="utf-8") + os.chmod(p, existing) + + old = os.umask(umask) + try: + atomic_write_text(target, "new\n", mode=None) + with open(reference, "w") as f: + f.write("new\n") + finally: + os.umask(old) + + assert _mode(target) == existing == _mode(reference) + assert target.read_text(encoding="utf-8") == "new\n" + + +def test_explicit_mode_beats_an_existing_files_permissions(tmp_path): + """Preserving what is on disk is only what ``mode=None`` asks for. A file + that has started holding a credential has to be tightened despite being + world-readable a moment ago, and an explicit loosening has to stick too.""" + tightened = tmp_path / "config.local.yaml" + tightened.write_text("timezone: UTC\n", encoding="utf-8") + os.chmod(tightened, 0o644) + atomic_write_text(tightened, "openai_api_key: sk-x\n") + assert _mode(tightened) == 0o600 + + loosened = tmp_path / "shared.yaml" + loosened.write_text("old\n", encoding="utf-8") + os.chmod(loosened, 0o600) + atomic_write_text(loosened, "new\n", mode=0o644) + assert _mode(loosened) == 0o644 + + +def test_symlinked_destination_is_written_through(tmp_path): + """A config file linked into a dotfiles repo must keep being that link — + replacing it with a regular file would leave the repo copy stale and put + the new secret outside the repo.""" + real = tmp_path / "repo" / "config.local.yaml" + real.parent.mkdir() + real.write_text("old\n", encoding="utf-8") + link = tmp_path / "config.local.yaml" + link.symlink_to(real) + + atomic_write_text(link, "new\n") + + assert link.is_symlink() + assert real.read_text(encoding="utf-8") == "new\n" + assert _mode(real) == 0o600 + + +@pytest.mark.skipif( + not hasattr(os, "fchmod"), + reason="no os.fchmod here; the chmod fallback it forces is covered below", +) +def test_mode_is_set_before_the_content_is_written(tmp_path, permissive_umask, monkeypatch): + """Chmod'ing after the write is a race: the secret is already on disk, and + readable, for however far apart the two syscalls land.""" + at_chmod = [] + real_fchmod = os.fchmod + + def spy_fchmod(fd, mode): + at_chmod.append((os.fstat(fd).st_size, mode)) + return real_fchmod(fd, mode) + + monkeypatch.setattr(os, "fchmod", spy_fchmod) + atomic_write_text(tmp_path / "secrets.yaml", "token: hunter2\n") + + # Permissions settled while the file was still empty. + assert at_chmod == [(0, 0o600)] + + +def test_mode_is_set_before_the_content_is_written_without_fchmod( + tmp_path, permissive_umask, monkeypatch +): + """Windows has no ``os.fchmod``, so the writer falls back to ``os.chmod`` on + the temp file's name. Take the attribute away here rather than only on the + platform that lacks it, or the one path that needs checking is the one path + never exercised. + + The mode asked for is 0o644, not 0o600: ``mkstemp`` already creates at 0o600, + so a 0o600 expectation would pass just as happily if the fallback never ran. + """ + monkeypatch.delattr(os, "fchmod", raising=False) + at_chmod = [] + real_chmod = os.chmod + + def spy_chmod(path, mode, **kwargs): + at_chmod.append((os.stat(path).st_size, mode)) + return real_chmod(path, mode, **kwargs) + + monkeypatch.setattr(os, "chmod", spy_chmod) + target = tmp_path / "shared.yaml" + atomic_write_text(target, "timezone: UTC\n", mode=0o644) + + assert at_chmod == [(0, 0o644)] + assert _mode(target) == 0o644 + assert target.read_text(encoding="utf-8") == "timezone: UTC\n" + + +def test_existing_file_survives_a_failed_write(tmp_path, monkeypatch): + target = tmp_path / "config.local.yaml" + target.write_text("openai_api_key: sk-only-copy\n", encoding="utf-8") + + def boom(fd): + raise OSError("No space left on device") + + monkeypatch.setattr(os, "fsync", boom) + with pytest.raises(OSError): + atomic_write_text(target, "clobbered\n") + + assert target.read_text(encoding="utf-8") == "openai_api_key: sk-only-copy\n" + # ...and no debris left behind for the next reader to trip over. + assert [p.name for p in tmp_path.iterdir()] == ["config.local.yaml"] + + +def test_replaces_an_existing_file(tmp_path): + target = tmp_path / "f.txt" + target.write_text("old", encoding="utf-8") + atomic_write_text(target, "new") + assert target.read_text(encoding="utf-8") == "new" + assert list(tmp_path.iterdir()) == [target] From 6f15042a096ddad7fac3f018c0abaf44db8bb129 Mon Sep 17 00:00:00 2001 From: Alex Soffronow-Pagonidis Date: Thu, 30 Jul 2026 16:43:26 +0200 Subject: [PATCH 2/2] Review: split a legacy config.yaml instead of renaming it wholesale MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Migration moved everything except `workspace` and `lockdown` into the git-tracked `settings.yaml`, under a header reading "Safe to commit". A pre-split install therefore published its TLS certificate paths, AWS SSO profile, mail accounts, docker mounts, external-agent targets and workflow journal directory into a file the operator is told to commit, and nothing downstream objected: `nerve config validate --portable-only --strict-keys` passes, and the scaffolded gitleaks step matches credential shapes, not filesystem paths. `_MACHINE_LOCAL_PATHS` was not the cause. It decides whether a `config.yaml` is a legacy monolith at all, and was never consulted when copying — which is why `gateway.ssl` and `provider.aws_profile` were published despite being listed in it. The copy is now partitioned through that same predicate, so the list finally means what its name says. The split is by path depth, not by top-level key: `gateway.ssl` moves the subtree while `gateway.host` and `gateway.port` stay portable. Moving the whole key would send the bind address to a layer lockdown does not read. The machine half is written back to `config.yaml`, which is where `docs/config.md`'s layer table puts these keys and what `nerve init` already produces. Not `config.local.yaml`: that layer has higher precedence, and `routes/external_agents.py` rewrites the `external_agents` block into `config.yaml` on every enable/disable, so an overlay copy would shadow the gateway's own writes and every toggle would silently do nothing. `workspace` keeps going to `config.local.yaml`, which is written before anything is consumed — it is the one key whose loss is unrecoverable. `test_migration_still_publishes_machine_local_keys` pinned this as a known gap and named this fix in its docstring; it now pins the guarantee instead. A new guard parses the layer table out of `docs/config.md` and requires it to agree with `_MACHINE_LOCAL_PATHS` in both directions, and requires every entry to resolve against a default `NerveConfig` so a renamed field cannot sit in the list checking nothing. It found one real drift on its first run: `workflows.runs_dir` was in the code and missing from the table. Co-Authored-By: Claude --- docs/config.md | 39 +++++---- nerve/cli.py | 16 +++- nerve/migrate.py | 111 ++++++++++++++++++++++---- tests/test_migrate.py | 180 +++++++++++++++++++++++++++++++++++++++--- 4 files changed, 301 insertions(+), 45 deletions(-) diff --git a/docs/config.md b/docs/config.md index d448afbd..258ca1cb 100644 --- a/docs/config.md +++ b/docs/config.md @@ -18,11 +18,12 @@ inside `settings.yaml` is ignored. ## Which layer a key belongs in -`nerve init` splits its answers between the first two layers: +The first two layers divide as below. `nerve init` writes its answers this way, +and migration splits a legacy `config.yaml` on the same table: | Layer | Gets | |-------|------| -| `config.yaml` | `workspace`, `deployment`, `provider.aws_profile`, `gateway.ssl.*`, `proxy`, `docker`, `telegram.enabled`, `sync.gmail.accounts`, `external_agents`, `mcp_endpoint` | +| `config.yaml` | `workspace`, `deployment`, `provider.aws_profile`, `gateway.ssl.*`, `proxy`, `docker`, `telegram.enabled`, `sync.gmail.accounts`, `external_agents`, `mcp_endpoint`, `workflows.runs_dir` | | `settings.yaml` | `timezone`, `gateway.host`/`port`, `provider.type`/`aws_region` (incl. the region-scoped Bedrock model IDs), `agent.*`, `memory.*`, `sessions.*`, `sync.*`, `houseofagents.*`, quiet hours, `telegram.dm_policy`/`stream_mode` | The test is whether the value would be wrong on another machine: filesystem @@ -202,15 +203,21 @@ Nothing is deleted and the effective configuration does not change; values are only relocated. Originals are renamed to `*.migrated` breadcrumbs, and an existing breadcrumb is never overwritten. -- `config.yaml` moves to the git-tracked `workspace/config/settings.yaml`. Secret - values are moved to machine-local `config.local.yaml` and replaced with - `${ENV_VAR}` placeholders. The machine-local `workspace` path also stays in - `config.local.yaml`. +- `config.yaml` is split rather than copied. Shareable keys go to the git-tracked + `workspace/config/settings.yaml`; secret values go to machine-local + `config.local.yaml`, replaced with `${ENV_VAR}` placeholders; the machine-local + keys from the table above are rewritten into `config.yaml`, so a certificate + path or an AWS profile handle never travels with the workspace repo. The split + follows the table at whatever depth a key is listed, so `gateway.ssl.*` stays + local while `gateway.host`/`port` move across. Migration prints which keys it + kept. The `workspace` path is the exception that stays in `config.local.yaml`: + it is written before anything is consumed, so an interrupted migration cannot + leave the instance unable to find its workspace. - The legacy `~/.nerve/cron/*` moves to `workspace/config/cron/*`, including any `prompts/` referenced by `prompt_file`. -- `config.local.yaml` and the breadcrumb hold plaintext secrets, so both are - written `0600`. `settings.yaml` is written normally, so a restrictive `umask` is - honored. +- `config.local.yaml`, the rewritten `config.yaml` and the breadcrumb can all hold + plaintext credentials, so all three are written `0600`. `settings.yaml` is + written normally, so a restrictive `umask` is honored. A value is treated as secret when any of these hold: @@ -241,13 +248,13 @@ tracked file that still looks like a credential, for you to judge. Migration only runs on a pre-refactor `config.yaml`, meaning one that holds shareable settings rather than only this box's. A `config.yaml` written by -`nerve init` under the current layout holds only machine-local keys (workspace, -bind address, provider handles), so it is left in place even when the workspace has -no `settings.yaml`. Migration is also a no-op once `settings.yaml` carries real -keys; the `nerve init` scaffold is all comments and counts as empty. A -`config.yaml` with shareable keys sitting next to a populated `settings.yaml` is -reported, because it overrides the tracked file, and you move those keys across by -hand. +`nerve init` — or left behind by an earlier migration — holds only machine-local +keys (workspace, certificate paths, provider handles), so it is left in place even +when the workspace has no `settings.yaml`. Migration is also a no-op once +`settings.yaml` carries real keys; the `nerve init` scaffold is all comments and +counts as empty. A `config.yaml` with shareable keys sitting next to a populated +`settings.yaml` is reported, because it overrides the tracked file, and you move +those keys across by hand. ## Config Directory Resolution diff --git a/nerve/cli.py b/nerve/cli.py index 177ff7c4..f09c6519 100644 --- a/nerve/cli.py +++ b/nerve/cli.py @@ -771,6 +771,12 @@ def upgrade(ctx: click.Context, no_frontend: bool, no_deps: bool, no_pull: bool) "config.local.yaml; review workspace/config/settings.yaml " "before committing)" ) + if report.machine_local_kept: + click.echo( + f" ({len(report.machine_local_kept)} machine-local key(s) kept " + "in config.yaml instead of published: " + f"{', '.join(report.machine_local_kept)})" + ) if report.suspect_values: click.secho( f" ({len(report.suspect_values)} value(s) left in settings.yaml " @@ -1152,8 +1158,9 @@ def doctor(ctx: click.Context) -> None: def migrate(ctx: click.Context, dry_run: bool) -> None: """Migrate a legacy config layout into the workspace/config subtree. - Copies config.yaml → workspace/config/settings.yaml (scrubbing secrets into - config.local.yaml as ${ENV_VAR} refs) and ~/.nerve/cron → workspace/config/cron. + Splits config.yaml: shareable keys → workspace/config/settings.yaml (secrets + scrubbed into config.local.yaml as ${ENV_VAR} refs), machine-local keys stay + in config.yaml. Also moves ~/.nerve/cron → workspace/config/cron. Non-destructive (originals kept as *.migrated) and idempotent. """ from nerve.migrate import migrate as run_migrate @@ -1178,6 +1185,11 @@ def migrate(ctx: click.Context, dry_run: bool) -> None: click.secho(f"\n Note: {warning}", fg="yellow") if report.secrets_moved: click.echo(f"\n Secrets scrubbed to config.local.yaml: {', '.join(report.secrets_moved)}") + if report.machine_local_kept: + click.echo( + "\n Machine-local keys kept in config.yaml: " + f"{', '.join(report.machine_local_kept)}" + ) if report.suspect_values: click.secho( f"\n {len(report.suspect_values)} value(s) headed for settings.yaml still " diff --git a/nerve/migrate.py b/nerve/migrate.py index 1723aa0f..0820edf3 100644 --- a/nerve/migrate.py +++ b/nerve/migrate.py @@ -9,6 +9,7 @@ to the workspace-centric layout: /config/settings.yaml (shareable, git-tracked — SCRUBBED) + /config.yaml (machine-local base — REWRITTEN) /config.local.yaml (secrets, machine-local) /config/cron/* (cron config, git-tracked) @@ -23,6 +24,11 @@ * **Copy + keep as backup** — originals are never deleted; ``config.yaml`` and the legacy cron files are renamed to ``*.migrated`` breadcrumbs, and the new location wins. An existing breadcrumb is never overwritten. +* **Split, don't copy** — a legacy ``config.yaml`` holds both halves, so the keys + :data:`_MACHINE_LOCAL_PATHS` names are rewritten into a fresh ``config.yaml`` + and never reach the tracked file. A certificate path, an AWS profile handle or + a mount list is right on exactly one box; syncing it to the rest of a fleet + points them all at something that isn't there. * **Auto-scrub secrets** — before writing the *tracked* ``settings.yaml``, secret values are moved into machine-local ``config.local.yaml`` and replaced with ``${ENV_VAR}`` placeholders. Scrubbed: values under secret-looking keys @@ -178,22 +184,27 @@ # a credential handle, a certificate path, whose mailboxes this person syncs, # which agent binaries this box has paired. Prefixes match their whole subtree. # -# Migration uses these to tell the two shapes of ``config.yaml`` apart. A legacy -# monolith holds everything — timezone, secrets, agent behaviour — and belongs in -# the tracked layer. A post-split ``config.yaml`` holds *only* these, and copying -# it into a file the docs say to commit would publish exactly the values the -# split exists to keep local. If nothing else is in there, there is nothing to -# migrate. +# Migration uses these twice. +# +# They tell the two shapes of ``config.yaml`` apart: a legacy monolith holds +# everything — timezone, secrets, agent behaviour — and belongs in the tracked +# layer, while a post-split ``config.yaml`` holds *only* these, so if nothing else +# is in there, there is nothing to migrate (see :func:`_has_portable_content`). +# +# And they are what a monolith is split *on*: the machine half is rewritten back +# into ``config.yaml`` rather than copied into a file the docs say to commit (see +# :func:`_partition_machine_local`). # # Entries are scoped to the part that is genuinely local. Listing a whole subtree # here keeps every key under it out of the tracked layer, which is why ``gateway`` # and ``provider`` were wrong as whole subtrees: a legacy monolith's bind port and # provider type stayed in config.yaml, which lockdown does not read. # -# The list mirrors what the wizard routes to the machine layer, and two tests -# check both directions: one drives the wizard and fails if it emits a path this -# does not cover, the other migrates a monolith and fails if a shareable key is -# left behind. +# The list is the ``config.yaml`` row of the layer table in ``docs/config.md``, +# and tests check it from three sides: one parses that table and fails on any +# disagreement (in either direction, and on a path that resolves to no real +# setting), one drives the wizard and fails if it routes a path here that this +# does not cover, one fails if this claims a path the wizard shares. _MACHINE_LOCAL_PATHS = frozenset({ "workspace", "deployment", @@ -227,6 +238,10 @@ class MigrationReport: migrated_cron: bool = False actions: list[str] = field(default_factory=list) secrets_moved: list[str] = field(default_factory=list) + # Dotted paths withheld from the tracked file and rewritten into + # config.yaml. Reported for the same reason the scrubbed secrets are: the + # operator has to be able to see which half of their config went where. + machine_local_kept: list[str] = field(default_factory=list) # Dotted paths left in the tracked file whose value still looks like a # credential. Nothing was done about them — they are for the operator to # look at before committing. @@ -421,6 +436,36 @@ def _is_machine_local(dotted: str) -> bool: ) +def _partition_machine_local(data: dict, path: tuple[str, ...] = ()) -> tuple[dict, dict]: + """Split a config mapping into (portable, machine_local). + + The split happens at whatever depth the path is listed at, not at the top + level: ``gateway.ssl`` takes the ``ssl`` subtree and leaves ``gateway.host`` + and ``gateway.port`` behind. Moving whole top-level keys instead would drag + the bind address off to the machine layer along with the certificate paths, + and lockdown never reads that layer. + + A subtree emptied by the split is dropped rather than left behind as + ``gateway: {}``. An empty mapping in the source is preserved as written — + there is nothing under it to be machine-local. + """ + portable: dict = {} + machine: dict = {} + for key, value in data.items(): + p = path + (str(key),) + if _is_machine_local(".".join(p)): + machine[key] = value + elif isinstance(value, dict) and value: + sub_portable, sub_machine = _partition_machine_local(value, p) + if sub_portable: + portable[key] = sub_portable + if sub_machine: + machine[key] = sub_machine + else: + portable[key] = value + return portable, machine + + def _has_portable_content(raw: dict) -> bool: """True if ``config.yaml`` holds anything the shareable layer should own. @@ -537,8 +582,11 @@ def _migrate_config_yaml(config_dir: Path, workspace: Path, report: MigrationRep return # a machine-local config.yaml from the split layout, not a legacy one # The workspace location is machine-local — it must not live in the tracked - # settings file (circular) but must be preserved so config still resolves the - # workspace after config.yaml is renamed away. Keep it in config.local.yaml. + # settings file (circular). It is the one machine-local key that does not go + # back into config.yaml with the rest: config.local.yaml is written before + # anything is consumed, so no interruption can lose it. Losing it is the only + # unrecoverable outcome here — the instance would resolve the *default* + # workspace and read no settings.yaml at all. ws_value = raw.pop("workspace", None) # ``lockdown`` is honored only in the tracked settings file — the machine @@ -556,7 +604,14 @@ def _migrate_config_yaml(config_dir: Path, workspace: Path, report: MigrationRep "layers from being read. Set it there deliberately to lock this instance" ) - tracked, secrets, moved = _scrub_secrets(raw) + # Split before scrubbing, so a machine-local value never reaches the tracked + # file even as a ${VAR} placeholder. The machine half is left unscrubbed: + # config.yaml is machine-local and gitignored, exactly like the overlay the + # placeholders would point at, so scrubbing it would only add indirection — + # which is why it is written owner-only below. + portable, machine_local = _partition_machine_local(raw) + + tracked, secrets, moved = _scrub_secrets(portable) # Everything bound for the machine-local overlay: scrubbed secrets + the # workspace path (not a secret, but machine-specific). @@ -566,13 +621,17 @@ def _migrate_config_yaml(config_dir: Path, workspace: Path, report: MigrationRep local_path = config_dir / "config.local.yaml" backup = _breadcrumb_path(config_yaml) + kept = _leaf_paths(machine_local) report.migrated_config = True report.secrets_moved.extend(moved) + report.machine_local_kept.extend(kept) report.suspect_values.extend(_suspect_values(tracked)) if local_additions: report.actions.append(f"moved secrets + workspace path into {local_path}") report.actions.append(f"config.yaml → {settings} (scrubbed {len(moved)} secret(s))") + if kept: + report.actions.append(f"kept {len(kept)} machine-local key(s) in {config_yaml}") report.actions.append(f"config.yaml → {backup} (backup)") for dotted in _relocated_lists(secrets): report.warnings.append( @@ -584,19 +643,26 @@ def _migrate_config_yaml(config_dir: Path, workspace: Path, report: MigrationRep if report.dry_run: return - # Order matters, and both writes are atomic (temp file + rename), so an + # Order matters, and every write is atomic (temp file + rename), so an # interruption at any point leaves a working install: # # 1. the local overlay first, so the tracked file never references secrets # that exist nowhere on this machine; # 2. the tracked settings file; - # 3. only then rename config.yaml away. + # 3. rename config.yaml away; + # 4. write the machine-local half back to config.yaml. # # Stopping between 2 and 3 leaves config.yaml still shadowing an already # complete settings.yaml — the instance keeps working, and re-running is a # no-op. Renaming earlier would invert that: a crash before the settings # file existed would take every non-secret setting out of the live config # with no way to retry. + # + # Steps 3 and 4 cannot be one operation — they are the same path — so an + # interruption between them leaves the machine half only in the breadcrumb, + # to be copied back by hand. That window holds nothing that stops the + # instance from loading: the workspace path went to config.local.yaml at + # step 1. if local_additions: existing_local = _read_yaml_mapping(local_path) # Existing local values win — never clobber a value the operator already @@ -627,6 +693,21 @@ def _migrate_config_yaml(config_dir: Path, workspace: Path, report: MigrationRep _restrict(config_yaml) config_yaml.rename(backup) + if machine_local: + # Owner-only, unlike the tracked file: this half is unscrubbed, and the + # subtrees that land in it are where a paired agent's token or a local + # service credential lives. + atomic_write_text( + config_yaml, + "# Nerve — machine-local configuration (migrated).\n" + "# The half of the old config.yaml that describes this box:\n" + "# filesystem paths, credential handles, what this machine has\n" + "# paired. Not for a shared repo. Shareable settings are in\n" + "# /config/settings.yaml, which this file overrides.\n\n" + + yaml.safe_dump(machine_local, default_flow_style=False, sort_keys=False), + mode=_SECRET_FILE_MODE, + ) + def _migrate_cron(workspace: Path, legacy: Path, report: MigrationReport) -> None: """Copy the legacy cron directory into the workspace, one file at a time. diff --git a/tests/test_migrate.py b/tests/test_migrate.py index 1298a697..a99b218d 100644 --- a/tests/test_migrate.py +++ b/tests/test_migrate.py @@ -1,6 +1,7 @@ """Tests for legacy → workspace/config migration.""" import os +import re from pathlib import Path from unittest.mock import patch @@ -712,16 +713,15 @@ def test_a_legacy_bind_port_and_provider_are_migrated(self, tmp_path): assert settings["provider"]["type"] == "bedrock" assert settings["provider"]["aws_region"] == "eu-west-1" - def test_migration_still_publishes_machine_local_keys(self, tmp_path): - """Known gap, pinned here so that changing it is a deliberate act. + def test_machine_local_keys_are_not_published(self, tmp_path): + """A legacy monolith is split on ``_MACHINE_LOCAL_PATHS``, not copied. - ``_MACHINE_LOCAL_PATHS`` only gates whether a config.yaml is a legacy - monolith (``_has_portable_content``); it does not filter what gets copied, - so migration moves the whole file. A pre-split install with TLS paths - publishes them into a git-tracked settings file, and syncing that to a box - without the certificate makes its gateway fail to bind. Narrowing the list - neither caused nor fixes this: the fix is for migration to split the file - instead of renaming it wholesale, which is a larger change. + The machine half is rewritten into config.yaml and never reaches the + git-tracked settings file: syncing a settings.yaml that names a + certificate path present on one box makes every other box's gateway fail + to bind. The split has to happen at the depth the path is listed at — + ``gateway.ssl`` moving must not take ``gateway.port`` with it, which would + strand the bind port in a layer lockdown does not read. """ config_dir, workspace = _legacy_install( tmp_path, @@ -729,12 +729,77 @@ def test_migration_still_publishes_machine_local_keys(self, tmp_path): "gateway:\n port: 9100\n ssl:\n cert: /etc/nerve/tls/cert.pem\n" "provider:\n type: bedrock\n aws_profile: nerve-prod\n", ) - migrate(config_dir, workspace=workspace) + report = migrate(config_dir, workspace=workspace) settings = yaml.safe_load( workspace_settings_file(workspace).read_text(encoding="utf-8") ) - assert settings["gateway"]["ssl"]["cert"] == "/etc/nerve/tls/cert.pem" - assert settings["provider"]["aws_profile"] == "nerve-prod" + assert "ssl" not in settings["gateway"] + assert "aws_profile" not in settings["provider"] + assert settings["gateway"]["port"] == 9100 # portable sibling + assert settings["provider"]["type"] == "bedrock" # portable sibling + + machine = yaml.safe_load((config_dir / "config.yaml").read_text(encoding="utf-8")) + assert machine["gateway"]["ssl"]["cert"] == "/etc/nerve/tls/cert.pem" + assert machine["provider"]["aws_profile"] == "nerve-prod" + assert "timezone" not in machine + assert sorted(report.machine_local_kept) == [ + "gateway.ssl.cert", "provider.aws_profile", + ] + + def test_the_split_preserves_the_effective_config(self, tmp_path): + """Relocated, not dropped: config.yaml is a layer the loader still reads, + so every machine-local value has to survive the split.""" + config_dir, workspace = _legacy_install( + tmp_path, + "timezone: UTC\n" + "gateway:\n port: 9100\n" + " ssl:\n cert: /etc/nerve/tls/cert.pem\n key: /etc/nerve/tls/key.pem\n" + "docker:\n extra_mounts: ['~/code:/code']\n" + "workflows:\n runs_dir: /srv/box-42/workflow-runs\n", + ) + before = load_config(config_dir) + migrate(config_dir, workspace=workspace) + after = load_config(config_dir) + + assert after.gateway.ssl.cert == before.gateway.ssl.cert + assert after.gateway.ssl.key == before.gateway.ssl.key + assert after.gateway.port == before.gateway.port == 9100 + assert after.docker.extra_mounts == before.docker.extra_mounts + assert after.workflows.runs_dir == before.workflows.runs_dir + + def test_the_rewritten_config_yaml_is_owner_only(self, tmp_path, permissive_umask): + """It is the unscrubbed half — an ``external_agents`` target's token and a + local service's credentials are in the subtrees that land here.""" + config_dir, workspace = _legacy_install( + tmp_path, + "timezone: UTC\n" + "external_agents:\n targets:\n - name: codex\n token: tok-secret\n", + ) + migrate(config_dir, workspace=workspace) + config_yaml = config_dir / "config.yaml" + assert _mode(config_yaml) == 0o600 + assert "tok-secret" in config_yaml.read_text(encoding="utf-8") + assert "tok-secret" not in workspace_settings_file(workspace).read_text() + + def test_the_split_half_does_not_look_legacy_on_a_re_run(self, tmp_path): + """The file migration leaves behind is a post-split config.yaml, so a + second pass has to read it as nothing to do rather than as a monolith + shadowing the tracked file.""" + config_dir, workspace = _legacy_install(tmp_path, self.MACHINE_ONLY + "timezone: UTC\n") + assert migrate(config_dir, workspace=workspace).migrated_config + + again = migrate(config_dir, workspace=workspace) + assert not again.did_anything + assert again.warnings == [] + assert is_migrated(config_dir, workspace=workspace) + + def test_no_config_yaml_is_written_when_there_is_no_machine_half(self, tmp_path): + """A wholly portable legacy config leaves nothing behind to shadow the + tracked file.""" + config_dir, workspace = _legacy_install(tmp_path, "timezone: UTC\n") + report = migrate(config_dir, workspace=workspace) + assert report.machine_local_kept == [] + assert not (config_dir / "config.yaml").exists() def test_the_external_agents_block_is_machine_local_too(self, tmp_path): """A second writer appends to config.yaml after the wizard has paired @@ -771,6 +836,97 @@ def test_no_warning_for_an_ordinary_split_install(self, tmp_path): assert migrate(config_dir, workspace=workspace).warnings == [] +_DOCS_CONFIG_MD = Path(__file__).resolve().parents[1] / "docs" / "config.md" +_LAYER_TABLE_HEADING = "## Which layer a key belongs in" +# A sentinel rather than getattr's default of None: a setting whose default *is* +# None (gateway.ssl.cert) has to read as present. +_MISSING = object() + + +def _documented_machine_local_paths() -> set[str]: + """The ``config.yaml`` row of the layer table in ``docs/config.md``. + + Parsed out of the document rather than restated here. A copy of the table + kept in this file would agree with the test forever and with the docs never, + which is the failure mode the guard exists to catch. + """ + text = _DOCS_CONFIG_MD.read_text(encoding="utf-8") + parts = text.split(_LAYER_TABLE_HEADING, 1) + assert len(parts) == 2, ( + f"{_LAYER_TABLE_HEADING!r} is no longer in {_DOCS_CONFIG_MD} — the layer " + "table moved or was renamed, and this guard is now checking nothing" + ) + rows = [ + line for line in parts[1].splitlines() if line.startswith("| `config.yaml` |") + ] + assert len(rows) == 1, ( + f"expected exactly one `config.yaml` row under {_LAYER_TABLE_HEADING!r}, " + f"found {len(rows)}" + ) + tokens = re.findall(r"`([^`]+)`", rows[0].split("|")[2]) + assert tokens, f"the `config.yaml` row names no keys: {rows[0]}" + + paths = set() + for token in tokens: + # ``gateway.ssl.*`` names a subtree; the code lists its root. + assert re.fullmatch(r"[a-z_]+(?:\.[a-z_]+)*(?:\.\*)?", token), ( + f"{token!r} in the layer table is not a plain dotted path. The " + "shorthand the settings.yaml row uses (`a`/`b` for two siblings) " + "would be read as one key here, so spell it out instead" + ) + paths.add(token[:-2] if token.endswith(".*") else token) + return paths + + +class TestMachineLocalPathsMatchTheDocs: + """The layer table in ``docs/config.md`` states which layer owns a key; + ``_MACHINE_LOCAL_PATHS`` is what migration and the wizard actually do. The + list's comment claims to mirror the table and nothing checked it. + + Both directions matter now that the list decides what migration publishes. A + key the table calls machine-local but the list omits is copied into a file the + docs tell you to commit; one the list claims but the table shares is stranded + in a layer lockdown never reads. + """ + + def test_the_list_and_the_table_agree(self): + documented = _documented_machine_local_paths() + assert len(documented) >= 10, ( + f"the parse found only {len(documented)} keys in the layer table — " + "did the table's formatting change?" + ) + listed = set(nerve.migrate._MACHINE_LOCAL_PATHS) + assert documented == listed, ( + "docs/config.md's `config.yaml` row and _MACHINE_LOCAL_PATHS " + "disagree.\n" + f" in the docs, not in the code: {sorted(documented - listed)}\n" + f" in the code, not in the docs: {sorted(listed - documented)}" + ) + + def test_every_listed_path_names_a_real_setting(self): + """A renamed or misspelled entry covers nothing and says nothing. + + It matches no key, so ``_has_portable_content`` counts the value as + shareable and the split hands it to the tracked file — while the list + still reads as though that key were accounted for. + """ + from nerve.config import NerveConfig + + defaults = NerveConfig() + unresolved = [] + for dotted in sorted(nerve.migrate._MACHINE_LOCAL_PATHS): + node = defaults + for part in dotted.split("."): + node = getattr(node, part, _MISSING) + if node is _MISSING: + unresolved.append(dotted) + break + assert not unresolved, ( + "_MACHINE_LOCAL_PATHS entries that resolve to no setting on a default " + f"NerveConfig (renamed? misspelled?): {unresolved}" + ) + + class TestMigrationIsolation: """``workspace=`` alone never sandboxed the cron half — it resolved its source from the machine-global state dir and *renamed* files there."""