diff --git a/docs/config.md b/docs/config.md index e9705f6b..a5d7d738 100644 --- a/docs/config.md +++ b/docs/config.md @@ -95,6 +95,8 @@ type, including `int | None`, `list[int]` and `list[str]`. So `port: ${PORT}` an - An unrecognized value is logged with the field that owns it, and that field keeps its documented default. Integers are parsed with `int()`, so `"1.5"` and `"1e3"` are rejected rather than truncated. +- [`lockdown`](#lockdown-remote-only-read-only) is the exception: its default is + "unprotected", so an unreadable or empty value there is a hard error instead. - Defaults are not re-scanned: `${A:-${B}}` yields the literal `${B}` when `A` is unset. Use a single reference instead. @@ -104,11 +106,12 @@ type, including `int | None`, `list[int]` and `list[str]`. So `port: ${PORT}` an error, so it can gate a config repo in CI: ```bash -nerve config validate # the active install's config -nerve config validate --workspace . # a checked-out config repo -nerve config validate --portable-only # ignore this machine's config.yaml layers -nerve config validate --strict-keys # unknown keys become errors -nerve config validate --strict-env # every ${ENV_VAR} must be set +nerve config validate # the active install's config +nerve config validate --workspace . # a checked-out config repo +nerve config validate --portable-only # ignore this machine's config.yaml layers +nerve config validate --strict-keys # unknown keys become errors +nerve config validate --strict-env # every ${ENV_VAR} must be set +nerve config validate --assume-lockdown # check the view a locked box loads ``` It runs even when the config cannot otherwise load, so a missing secret does not @@ -180,6 +183,9 @@ jobs: # nerve is not published to PyPI: the bare name is an unrelated project. - run: uv pip install --system "nerve @ git+https://github.com/ClickHouse/nerve" - run: nerve config validate --workspace . --portable-only --strict-keys + # Add --assume-lockdown if any instance served by this repo is locked and + # the flag comes from the environment. Without it CI checks the unlocked + # view and the lockdown checks never run. See "Lockdown" below. ``` Installing from the default branch keeps the validator at or ahead of your @@ -222,17 +228,21 @@ config, so the merged change takes effect immediately. CI on the PR is still the first line of defense. The remote and credentials come from git itself, so configure `git remote` and auth in the workspace as usual. -**Keep the config subtree clean.** Sync refuses to merge while -`/config/` has local changes: an edited or deleted tracked file, a -staged change, or an untracked file. Validation judges a clean checkout of the -fetched commit, but the merge lands in your working tree, and `--ff-only` only -refuses when the incoming commit touches the same path. Anything else would survive -the merge unchecked, leaving a bundle on disk that is not the one that passed. The -case that matters most is an untracked `config/cron/gates/*.py`: the daemon imports -and runs gate plugins and validation never loads them, so a box meant to run only -reviewed config would be running local code. Commit, discard or push local edits; -the failure message names the paths. Files matched by `.gitignore` inside `config/` -are warnings rather than refusals, since the shared repo can never carry them. +**Keep the reviewed files clean.** Sync refuses to merge while the workspace's +reviewed files have local changes: an edited or deleted tracked file, a staged +change, or an untracked file. That is `/config/`, `/skills/` +and the root instruction files (`AGENTS.md`, `SOUL.md`, `IDENTITY.md`, `USER.md`, +`TOOLS.md`) — the same set lockdown's write guard refuses. Validation judges a clean +checkout of the fetched commit, but the merge lands in your working tree, and +`--ff-only` only refuses when the incoming commit touches the same path. Anything +else would survive the merge unchecked, leaving a bundle on disk that is not the one +that passed. The case that matters most is an untracked `config/cron/gates/*.py`: +the daemon imports and runs gate plugins and validation never loads them, so a box +meant to run only reviewed config would be running local code. Commit, discard or +push local edits; the failure message names the paths. So a skill the agent created +at runtime, or a locally edited `SOUL.md`, blocks the merge until it is committed or +dropped. Files matched by `.gitignore` in there are warnings rather than refusals, +since the shared repo can never carry them. Sync validates more strictly than CI: an unset required `${VAR}` blocks the merge. CI has no secrets, so it reports those as info, but the daemon does have them, and a @@ -249,6 +259,225 @@ config object every cycle, so it adds no staleness of its own, but nothing refre that object while the process runs. Turning `enabled` on needs a restart in any case, because the sync task is only created at startup. +## Lockdown (remote-only, read-only) + +Lockdown guarantees that the configuration an instance runs came from the tracked +workspace repo, and that the runtime cannot change it. The machine-local layers are +dropped, the write paths refuse, and config changes arrive only as a reviewed, +merged commit that sync pulls in. + +It is a config-integrity control, not a sandbox. It raises the bar against the +agent rewriting its own configuration; it does not confine the agent. Read +[What lockdown does not cover](#what-lockdown-does-not-cover) before relying on +it. + +### Before you turn it on + +**Clone the config repo as the workspace first.** A locked instance takes config +changes only as a merged commit that sync pulls in, so a workspace with no git +remote has nowhere to receive one from and refuses to start. `git remote -v` in the +workspace is the check. + +**Move any required secrets into the environment first.** `config.local.yaml` is +ignored when locked, so a secret that lives only there stops being read, and the +feature depending on it breaks on the next restart. Supply each one as `${ENV_VAR}` +referenced from `settings.yaml` before you lock the box. The usual ones: +`auth.jwt_secret`, `auth.password_hash`, `telegram.bot_token`, +`anthropic_api_key`/`openai_api_key`, `xmemory.api_key`. + +`auth.jwt_secret` is the one to get right. A locked instance that ends up without +it does not fall back to the unauthenticated dev mode an unlocked box would — the +gateway refuses every request with a 503 and websockets are declined — so the box +comes up unusable rather than open. Note also that a `${VAR}` left unresolved +survives as its literal text, which is a perfectly usable signing key and one +published in the config repo, so check that the variable is actually set on the box. + +### Turning it on + +Set `lockdown: true` in the tracked `workspace/config/settings.yaml`, so the +remote is the authority: + +```yaml +# workspace/config/settings.yaml +lockdown: true +``` + +The value may be an environment reference, `lockdown: ${NERVE_LOCKDOWN}`, so one +shared repo can serve a fleet where only some boxes are locked. Unlike every other +boolean, this one is not lenient: a value that is neither true nor false is a hard +error and the instance refuses to start, because the only default it could fall +back to is "unlocked". An empty value counts as unreadable, so +`${NERVE_LOCKDOWN}` with the variable unset or blank is refused rather than read +as off. Write `${NERVE_LOCKDOWN:-false}` when you want an explicit unlocked +default. `nerve config validate` reports the same error, so a bad value fails the +PR instead of the box. + +### Better: set it in the environment + +Setting the flag in `settings.yaml` alone leaves a way around it. Which +`settings.yaml` gets read is decided by `workspace:` in the machine-local +`config.yaml`, so editing that one line repoints the instance at a tree that never +mentions lockdown — the flag reads false and the machine-local layers come back, +`auth.jwt_secret` included. + +Closing that means putting both values in the environment, set where the service is +defined. That is the one place neither a config edit nor the agent can reach: + +```ini +# /etc/systemd/system/nerve.service +[Service] +Environment=NERVE_LOCKDOWN=1 +Environment=NERVE_WORKSPACE=/srv/nerve-workspace +``` + +```bash +docker run -e NERVE_LOCKDOWN=1 -e NERVE_WORKSPACE=/root/nerve-workspace ... +``` + +Set both, not just one. `NERVE_LOCKDOWN` without `NERVE_WORKSPACE` is refused at +startup: it would lock the instance onto whatever tree `config.yaml` happens to +name, which is worse than not locking it at all. Once `NERVE_WORKSPACE` is set, +`workspace:` in `config.yaml` is ignored. + +Two things to know about `NERVE_LOCKDOWN`: + +- **It can only lock, never unlock.** The instance is locked if the variable says so + *or* the tracked `settings.yaml` does. `NERVE_LOCKDOWN=false`, empty, and unset + all mean the environment expresses no opinion; none of them force an unlock. So no + file arriving later can undo it, and equally you cannot use the environment to + escape a locked tracked config — unlocking takes a merged change. A value that is + neither true nor false is refused at startup rather than read as no opinion. +- **It is the same switch as `lockdown: ${NERVE_LOCKDOWN}`.** With the variable set + you need not mention `lockdown` in the tracked file at all, and a fleet repo that + already writes `${NERVE_LOCKDOWN:-false}` gets the same protection, along with the + `NERVE_WORKSPACE` requirement on the boxes where it resolves true. + +### When locked + +- **Config is remote-only.** Only `workspace/config/` and `${ENV_VAR}` are used. + The machine-local `config.yaml` and `config.local.yaml` and the legacy + `~/.nerve/cron` are ignored, and secrets come from the environment. +- **The lever is remote-owned.** Lockdown is read only from the tracked + `settings.yaml`, so a local edit to `config.yaml` or `config.local.yaml` cannot + unlock the instance or fake-lock it. With `NERVE_WORKSPACE` set, neither can + repointing `workspace:`. +- **What lockdown protects is the workspace's *reviewed surface*.** That is + `config/`, `skills/`, and the root instruction files `AGENTS.md`, `SOUL.md`, + `IDENTITY.md`, `USER.md` and `TOOLS.md`. `config/` is the declarative half; a + `SKILL.md` is model-invocable text with its own `allowed-tools` frontmatter that + the skill index picks up on the next reload, and the root files are the system + prompt the agent starts every turn from. `memory/`, `tasks/`, `TASK.md` and + ordinary workspace files are outside it and stay writable. +- **Runtime edits to the reviewed surface are blocked.** Creating, updating, + deleting or toggling a skill, Telegram pairing, writing a workspace file that + lands in that surface, and other config mutations fail with a "locked" error + (HTTP 403). Change config by opening a PR against the workspace repo and letting + sync apply the merge. The rest of the workspace is unaffected: it is also the + agent's working directory, and lockdown covers what the box was reviewed to run + rather than the whole box. +- **The agent's own `Write`/`Edit` are refused across that surface.** Every + non-interactive tool is otherwise auto-approved, so without this the ordinary way + an agent edits a file would never meet the guards above — and the skill + endpoints' 403 would be beside the point, since the index picks up whatever is in + `skills/` on the next reload however it got there. The refusal names the PR flow, + so a capable agent routes to it instead of retrying. Writes elsewhere (memory, + task files, scratch files) are unaffected. `Bash` is not covered; see below. +- **The workspace must be a git repository with a remote.** Every local change to + the reviewed surface is refused, so a merged commit that sync pulls in is the only + way a config change can arrive. A workspace that is not a repository, or is one + with no remote, has no route at all: the instance would keep the configuration it + happens to hold, and nothing would report it. A locked instance without one + refuses to start rather than running unlocked — removing a remote is a + machine-local change, and a machine-local change does not unlock a box. Any remote + counts, and `workspace_sync.enabled: false` is not part of this: sync follows the + branch's own upstream when `workspace_sync.branch` is unset, and `nerve config + sync` is a manual route that works with the periodic loop off. +- **The reviewed subtrees must really be in the workspace.** `/config` + and `/skills` have to resolve inside ``, and each has to be + a real directory rather than a symlink. If one is a symlink out, nothing under it + is part of the reviewed repo, `settings.yaml` included. If it is a symlink to a + sibling inside the workspace, git does not descend into it, so nothing under it is + tracked and sync reports the workspace clean whatever it holds. Either way the + instance refuses to start. Where the workspace itself lives stays a machine-local + decision, symlink included. The root instruction files get no such startup check: + a locked instance whose `SOUL.md` is a symlink starts. What covers that case is + the write guard, which resolves the link and refuses its target too, so the file + the prompt is built from is not writable under another name. +- **`settings.yaml` must be inside that subtree.** `/config/settings.yaml` + has to resolve inside `/config/`, so a symlink there cannot source the + lockdown flag, the auth secret and the cron paths from a file no reviewer saw — an + ordinary workspace file the agent can write included. A link to another file + *within* the subtree is fine; both ends are tracked. The write guard refuses the + path `config/settings.yaml` by name as well as by where it resolves, so a symlink + cannot turn a reviewed name into an allowed write. +- **Cron cannot be pointed out of the workspace.** `cron.jobs_file`, + `cron.system_file` and `cron.gate_plugins_dir` must resolve inside + `/config/`. One that does not is ignored, with a warning, in favour of + the in-workspace default. `..`, absolute paths and symlinks out of the tree are + all caught, because the resolved path is what is checked. This matters most for + `gate_plugins_dir`, whose `.py` files the daemon imports: without the check, a + pure-YAML change to a reviewed file would be enough to get on-disk code executed. + When the in-workspace default is itself what escapes (`config/cron`, or + `config/cron/gates` committed as a symlink, which needs no config key at all) + there is nothing contained left to fall back to and the instance refuses to + start. +- **Sync is stricter about local files.** [Workspace + sync](#git-backed-workspace-sync) already refuses to merge when the reviewed + surface has local changes; on a locked instance a `.gitignore`d file in there is + a refusal too rather than a warning, and so is a submodule, which a validation + checkout never initializes and a fast-forward never updates. An ignored + `config/cron/gates/*.py` is local code the daemon runs, invisible to both the + reviewer and the validator. + +Run `nerve config validate --workspace .` before merging; it validates the locked +view when `lockdown: true`, which is where the checks above are decided. It also +names the machine-local keys the bundle strands, so a key that quietly falls back +to its default on every locked box is visible in the PR rather than at boot. The +remote check is the one that judges the checkout the command runs in rather than +the bundle: `actions/checkout` sets `origin`, but a run against an unpacked +tarball reports it missing. + +**If the flag comes from the environment, add `--assume-lockdown` in CI.** With +`lockdown: ${NERVE_LOCKDOWN:-false}` the validator resolves false wherever the +variable is unset, which is every CI runner, so it checks the unlocked view and +none of the checks above have anything to fire on — the only machine to find out is +the locked one, at boot. `--assume-lockdown` forces the locked view. A plain run +warns when it sees an env-controlled flag resolve to false, so the gap is at least +visible. + +**Flipping the flag needs the config reloaded.** A sync does that, and so does +`POST /api/config/reload`. The write guards, gateway authentication and the sync +loop pick the change up immediately; cron jobs and gates already built follow on the +next cron reload, and anything captured at startup needs a restart. A hand edit on +the box refreshes nothing, and under lockdown it is ignored anyway. If a sync merges +a config the daemon then cannot load, it reports `ok: false` with `apply_error` +rather than claiming the change took. + +### What lockdown does not cover + +Lockdown is a config-integrity control, not a sandbox and not a permission system. + +**The agent can still reach the files through the shell.** `Bash` is auto-approved +like every other non-interactive tool, so refusing `Write` and `Edit` closes the +path an agent takes by default and by accident, not the one it takes deliberately. +Nerve does not filter command strings — `sh -c`, redirection, `python -c` and every +editor defeat such a filter, and a filter that looks like a boundary without being +one is worse than a documented gap. If your threat model includes a deliberately +hostile agent, run the daemon as a user that cannot write the workspace repo, or +mount the reviewed subtrees read-only. + +**The codex backend is weaker still.** Codex's sandbox is a mode, not a path list, +so "everything except the reviewed surface" cannot be expressed to it. Nerve +declines a codex file change landing in that surface, but only through codex's +approval request, and the shipped defaults (`sandbox: danger-full-access`, +`approval_policy: never`) never ask. Treat the `Write`/`Edit` protection above as +absent there unless you have configured approvals. + +**Operator CLI commands are out of scope.** `nerve init`, `nerve migrate` and +`nerve restore` write config without consulting the flag: they are how an operator +sets a box up or repairs one, and anyone who can run them can already edit the +files directly. + ## Migrating an Existing Install Installs from before the workspace-config layout are migrated automatically and @@ -336,6 +565,7 @@ from any working directory: |-----|------|---------|-------------| | `workspace` | path | `~/nerve-workspace` | Path to workspace directory | | `timezone` | string | `America/New_York` | Local timezone for scheduling | +| `lockdown` | bool | `false` | [Remote-only, read-only mode](#lockdown-remote-only-read-only). Honored only in `workspace/config/settings.yaml`; an unreadable value is an error, not a default. | | `deployment` | string | `server` | `server` (bare metal) or `docker`. Set during `nerve init`; determines whether CLI commands run directly or proxy to `docker compose`. | > **Note:** The _mode_ (personal vs worker) is not a config field — it's determined at `nerve init` time and expressed through which workspace templates, cron jobs, and memory categories are active. There's no `mode` key in config. diff --git a/docs/cron.md b/docs/cron.md index eaa3b8cc..8ec49a15 100644 --- a/docs/cron.md +++ b/docs/cron.md @@ -18,7 +18,11 @@ git-syncable workspace subtree): > `/config/cron/`. Installs that still have the legacy > `~/.nerve/cron/` keep working: if the workspace location doesn't exist yet, > Nerve reads from `~/.nerve/cron/` automatically. You can also pin the paths -> explicitly with `cron.jobs_file` / `cron.system_file` / `cron.gate_plugins_dir`. +> explicitly with `cron.jobs_file` / `cron.system_file` / `cron.gate_plugins_dir` +> — except under [lockdown](config.md#lockdown-remote-only-read-only), where all +> three must resolve inside `/config/`. An escaping path is ignored in +> favour of the in-workspace default; if the default is what escapes (a symlinked +> `config/cron` or `config/cron/gates`) the instance refuses to start. Both files use the same format. On startup, CronService loads and merges both: - If a job ID appears in both files, the **user version wins** (with a warning in the log). diff --git a/nerve/agent/backends/claude.py b/nerve/agent/backends/claude.py index a451e8ae..61aa05ee 100644 --- a/nerve/agent/backends/claude.py +++ b/nerve/agent/backends/claude.py @@ -216,6 +216,42 @@ def _translate_tool_result( ) +def lockdown_denial(tool_name: str, tool_input: dict[str, Any]) -> str | None: + """Why a locked instance refuses this file-writing tool call, if it does. + + A locked instance promises that the config it runs came from a reviewed, + merged change. The write guards on the REST and MCP surfaces enforce that for + callers coming in over the network; the agent's own ``Write``/``Edit`` did not + go through any of them, and every non-interactive tool here is auto-approved. + This is where that gap closes. + + **Scope, precisely.** It covers the tools that name a path — ``Write``, + ``Edit``, ``NotebookEdit`` — and nothing else. It is emphatically not a + sandbox: ``Bash`` is auto-approved on the same code path, so an agent that + means to write ``/config/`` can still do it through a shell. That + is a known and documented limitation. Filtering command strings is not the + answer — quoting, ``sh -c``, redirection, ``python -c``, ``tee`` and every + editor defeat it — and a filter that looks like a boundary without being one + is worse than a gap someone can read about. Real sandboxing is separate work. + + What this does buy is that the ordinary way an agent edits a file no longer + silently rewrites the config the box is promising to run, and that the refusal + says what to do instead. + """ + if tool_name not in FILE_MODIFY_TOOLS: + return None + path = tool_input.get("file_path") or tool_input.get("notebook_path") + if not path: + return None + from nerve.config import tracked_config_write_refusal + + try: + return tracked_config_write_refusal(path) + except Exception as e: # noqa: BLE001 — a broken guard must not kill the turn + logger.warning("lockdown write check failed for %r: %s", path, e) + return None + + # ------------------------------------------------------------------ # # can_use_tool adapter # # ------------------------------------------------------------------ # @@ -241,6 +277,18 @@ async def can_use_tool( context: ToolPermissionContext, ) -> PermissionResult: hub = self._hub + # Checked before the snapshot: a refused write has nothing to snapshot, + # and marking it snapshotted would suppress the snapshot of a later, + # allowed write to the same path. + denial = lockdown_denial(tool_name, tool_input) + if denial: + logger.info( + "Session %s: denying %s under lockdown (%s)", + hub.session_id, tool_name, + tool_input.get("file_path") or tool_input.get("notebook_path"), + ) + return PermissionResultDeny(message=denial) + # Capture pre-execution file snapshot for diff tracking # (also done via PreToolUse hook in the backend as primary path). if hub.snapshot_fn and tool_name in FILE_MODIFY_TOOLS: @@ -712,10 +760,27 @@ async def _grant_permission_hook(hook_input, tool_use_id, context): approved via ``can_use_tool``, which never fires for them, so the exclusion protected nothing. The image validator still runs for Read and its deny wins over this allow. + + The lockdown check is repeated here rather than left to + ``can_use_tool``: for a background sub-agent this hook is the only + thing that runs, so the allow issued here is the entire decision. + It only ever refuses the path-naming write tools, so the Read + parity above is unaffected by it. """ tool_name = hook_input.get("tool_name", "") if tool_name in INTERACTIVE_TOOLS: return {"hookSpecificOutput": {"hookEventName": "PreToolUse"}} + denial = lockdown_denial( + tool_name, hook_input.get("tool_input", {}) or {}, + ) + if denial: + return { + "hookSpecificOutput": { + "hookEventName": "PreToolUse", + "permissionDecision": "deny", + "permissionDecisionReason": denial, + } + } return { "hookSpecificOutput": { "hookEventName": "PreToolUse", diff --git a/nerve/agent/backends/codex/backend.py b/nerve/agent/backends/codex/backend.py index 87543766..3d502c0e 100644 --- a/nerve/agent/backends/codex/backend.py +++ b/nerve/agent/backends/codex/backend.py @@ -1479,9 +1479,12 @@ async def _handle_server_request(self, method: str, params: dict) -> dict: "command_approval", self._approval_payload(params), ) if method == "item/fileChange/requestApproval": - return await self._approval( - "file_approval", self._approval_payload(params), - ) + payload = self._approval_payload(params) + refusal = self._lockdown_refusal(payload) + if refusal: + logger.warning("codex: declining file change — %s", refusal) + return {"decision": "decline"} + return await self._approval("file_approval", payload) if method == "item/permissions/requestApproval": # The response type requires a constructed GrantedPermissionProfile # — there is no decline variant to express. Unsupported in v1 @@ -1507,6 +1510,39 @@ async def _handle_server_request(self, method: str, params: dict) -> dict: logger.warning("codex: unknown server request %s — empty reply", method) return {} + def _lockdown_refusal(self, payload: dict) -> str | None: + """Why a locked instance declines this file change, if it does. + + **This is a partial control and the shape of the gap matters.** Codex's + sandbox is a mode — ``read-only`` / ``workspace-write`` / + ``danger-full-access`` — not a path list, so there is no way to express + "everything except ``/config/``" to the app-server. The one + in-band hook is this approval request, and it only fires when + ``approval_policy`` asks for approvals; nerve ships ``never`` with + ``danger-full-access``, so under the default configuration codex never + asks and this never runs. Wired up anyway, because an operator who does + run codex with approvals should get the same answer the Claude backend + gives, and because a control that exists is easier to reach for than one + that has to be invented later. Documented as a gap rather than presented + as a boundary. + """ + from nerve.config import tracked_config_write_refusal + + item = payload.get("item") + changes = self._changes(item) if isinstance(item, dict) else [] + for change in changes: + path = str(change.get("path") or "") + if not path: + continue + try: + refusal = tracked_config_write_refusal(path) + except Exception as e: # noqa: BLE001 — never break the turn + logger.warning("lockdown write check failed for %r: %s", path, e) + continue + if refusal: + return refusal + return None + def _approval_payload(self, params: dict) -> dict: """Attach the originating item's context (the raw request carries only ids/reason — the UI wants the command / changed files).""" diff --git a/nerve/agent/tools/handlers/tasks.py b/nerve/agent/tools/handlers/tasks.py index cc7eea23..5990640a 100644 --- a/nerve/agent/tools/handlers/tasks.py +++ b/nerve/agent/tools/handlers/tasks.py @@ -30,6 +30,7 @@ TASK_UPDATE_SCHEMA, TASK_WRITE_SCHEMA, ) +from nerve.config import ensure_path_not_tracked_config from nerve.db.task_statuses import ( DEFAULT_STATUS, STATUS_NAME_RE, @@ -315,6 +316,7 @@ async def task_update_handler(ctx: ToolContext, args: dict) -> ToolResult: if ctx.workspace and (note or deadline or raw_tags or new_title): file_path = ctx.workspace / task["file_path"] + ensure_path_not_tracked_config(file_path, "write") if file_path.exists(): content = await asyncio.to_thread( file_path.read_text, encoding="utf-8", @@ -371,6 +373,11 @@ async def task_read_handler(ctx: ToolContext, args: dict) -> ToolResult: if ctx.workspace: file_path = ctx.workspace / task["file_path"] + # Deliberately unguarded. Lockdown makes the tracked config subtree + # unwritable, not unreadable — it arrives from a repo the agent can + # already read, and the HTTP GET for a task doesn't guard either. + # Guarding here would refuse a read with a "Cannot write" message + # the caller has no way to act on. if file_path.exists(): content = await asyncio.to_thread( file_path.read_text, encoding="utf-8", @@ -404,6 +411,7 @@ async def task_write_handler(ctx: ToolContext, args: dict) -> ToolResult: return ToolResult.text("Workspace not configured.") file_path = ctx.workspace / task["file_path"] + ensure_path_not_tracked_config(file_path, "write") await asyncio.to_thread(file_path.write_text, new_content, encoding="utf-8") from nerve.tasks.models import ( @@ -441,6 +449,13 @@ async def task_done_handler(ctx: ToolContext, args: dict) -> ToolResult: if not task: return ToolResult.text(f"Task not found: {task_id}") + # Done is a write like any other — it copies the file into done/ and + # unlinks the source, so a stored ``file_path`` inside the tracked config + # subtree would delete config. Refuse before the status flip, or a + # refusal leaves a task marked done whose file never moved. + if ctx.workspace: + ensure_path_not_tracked_config(ctx.workspace / task["file_path"], "move") + await ctx.db.update_task_status(task_id, "done") # Mark any implementing plans for this task as done diff --git a/nerve/backup.py b/nerve/backup.py index 9311adce..60fc6829 100644 --- a/nerve/backup.py +++ b/nerve/backup.py @@ -107,6 +107,18 @@ WORKSPACE_INCLUDE_DIRS: tuple[str, ...] = ("config", "memory", "scripts", "skills") WORKSPACE_INCLUDE_FILE_GLOBS: tuple[str, ...] = ("*.md",) +# What restore may write back, which is deliberately *not* what backup collects. +# The asymmetry is the point, and it is one directory wide: `config` is captured +# so a lost machine loses nothing, and never written back, because the authority +# for tracked config is the git remote rather than a tarball. Writing it back +# would replace reviewed config on a locked instance, and on any instance leave +# the checkout dirty enough that the next sync refuses to merge. +# +# These must stay separate constants. Defining the restore side as "whatever +# backup collects" is what silently opened that hole the moment `config` was +# added above — one edit, two opposite meanings. +WORKSPACE_RESTORE_DIRS: tuple[str, ...] = ("memory", "scripts", "skills") + # Directory names always pruned while walking the included workspace dirs. _PRUNE_DIR_NAMES: frozenset[str] = frozenset({ ".git", "node_modules", ".venv", "venv", "__pycache__", @@ -117,6 +129,30 @@ # missed something (e.g. a build artifact landed under memory/). WORKSPACE_WARN_BYTES = 2 * 1024 * 1024 * 1024 # 2 GB + +def _restorable_workspace_path(rel: Path) -> bool: + """Whether a workspace path from a bundle may be written back on restore. + + Restore applies :data:`WORKSPACE_RESTORE_DIRS`, not the collect-side + allowlist. Bundles this module writes *do* carry ``config/`` — deliberately, + so a lost machine loses no settings or schedule — and a bundle it is handed + can carry anything at all. Either way, writing ``config/settings.yaml`` or + ``config/cron/gates/x.py`` back would let a tarball replace the git-tracked + config subtree: on a locked instance that displaces the reviewed remote + config the whole mode exists to guarantee, and on any instance it leaves the + checkout dirty enough that the next sync refuses to merge. + + So the config subtree travels in the bundle and is never written back. It is + still there to be read out and applied through review if a restore really + needs it, which the skip warning says. + """ + import fnmatch + + parts = rel.parts + if len(parts) == 1: + return any(fnmatch.fnmatch(parts[0], g) for g in WORKSPACE_INCLUDE_FILE_GLOBS) + return parts[0] in WORKSPACE_RESTORE_DIRS + # Retention prune only ever touches files matching this exact pattern, so it # can never delete an unrelated file that happens to share the directory. # An optional ``-`` disambiguates bundles created within the same second. @@ -876,13 +912,33 @@ def restore_bundle( staged_ws = staging / "workspace" if staged_ws.is_dir(): workspace.mkdir(parents=True, exist_ok=True) + refused: list[str] = [] for src in sorted(staged_ws.rglob("*")): if not src.is_file(): continue rel = src.relative_to(staged_ws) + if not _restorable_workspace_path(rel): + refused.append(str(rel)) + continue dst = workspace / rel dst.parent.mkdir(parents=True, exist_ok=True) shutil.copy2(src, dst) + if refused: + logger.warning( + "Restore: skipped %d workspace file(s) not restorable (%s): " + "%s. The config subtree is intentionally among these — it " + "travels in the bundle but is never written back, because " + "tracked config comes from the git remote. Read it out of " + "the bundle and apply it through review if you need it.", + len(refused), ", ".join(WORKSPACE_RESTORE_DIRS), + ", ".join(refused[:10]), + ) + report.warnings.append( + f"skipped {len(refused)} workspace file(s) this bundle " + f"carries that restore does not write back (the config " + f"subtree is deliberately one of them) — including " + f"{refused[0]!r}" + ) logger.info("Restore complete: %s → %s", path.name, nerve_dir) return report diff --git a/nerve/channels/telegram.py b/nerve/channels/telegram.py index 81d66fae..e58473cd 100644 --- a/nerve/channels/telegram.py +++ b/nerve/channels/telegram.py @@ -1023,10 +1023,29 @@ async def _handle_pair(self, update: Update, context: Any) -> None: ) return + # Refuse to pair under lockdown — allowed users must come from the + # tracked remote config, not a local runtime edit. Check BEFORE the + # in-memory add so lockdown isn't bypassed for the current run. + from nerve.config import LockdownError, is_locked + + if is_locked(): + await update.message.reply_text( + "This instance is locked (remote-only). Add your Telegram user " + "to telegram.allowed_users in the workspace config repo instead." + ) + return + # Success: authorize in memory and persist to config.local.yaml self._allowed_users.add(user_id) try: append_telegram_allowed_user(self.config.config_dir, user_id) + except LockdownError: + # Belt-and-suspenders: config was locked between the check and here. + self._allowed_users.discard(user_id) + await update.message.reply_text( + "This instance is locked (remote-only) — pairing is disabled." + ) + return except Exception: logger.exception("Paired user %d but failed to persist to config", user_id) await update.message.reply_text( diff --git a/nerve/cli.py b/nerve/cli.py index 336b1d04..1e43451a 100644 --- a/nerve/cli.py +++ b/nerve/cli.py @@ -955,6 +955,19 @@ def doctor_report(config, config_source: str = "", check_api: bool = False) -> s lines.append("[--] Anthropic API key not set (using proxy)") elif config.anthropic_api_key: lines.append(f"[OK] Anthropic API key: ...{config.anthropic_api_key[-4:]}") + elif config.lockdown: + # The generic message names config.local.yaml, which a locked instance + # never reads. The cause is also often upstream of the key: if the + # provider block was only in config.yaml, this instance has already + # reverted to the Anthropic default and is failing for a key it would not + # otherwise need. + errors.append( + "[ERR] No Anthropic API key. Lockdown does not read " + "config.local.yaml, so supply it as ${ANTHROPIC_API_KEY} in " + "workspace/config/settings.yaml. If this box should be using " + "Bedrock, provider.type is absent from the tracked settings and this " + "instance has fallen back to 'anthropic'" + ) else: errors.append("[ERR] Anthropic API key not set and proxy not enabled (config.local.yaml)") @@ -1239,6 +1252,7 @@ def config_sync( config.workspace, ctx.obj["config_dir"], branch=branch, validate=not no_validate, strict_env=config.workspace_sync.strict_env and not no_strict_env, + locked=config.lockdown, ) for warning in result.validation_warnings: click.secho(f" [WARN] {warning}", fg="yellow") @@ -1273,10 +1287,17 @@ def config_sync( help="Ignore this machine's config.yaml / config.local.yaml and validate " "only the portable workspace config — what a shared repo carries.", ) +@click.option( + "--assume-lockdown", "assume_locked", is_flag=True, + help="Validate the locked view whatever this bundle's lockdown flag resolves " + "to here. A fleet repo writes `lockdown: ${NERVE_LOCKDOWN:-false}`, so " + "CI resolves it to false and checks a config no locked box will run. Use " + "this in CI on any repo one of whose instances is locked.", +) @click.pass_context def config_validate( ctx: click.Context, workspace: str | None, strict_env: bool, strict_keys: bool, - portable_only: bool, + portable_only: bool, assume_locked: bool, ) -> None: """Validate the configuration bundle. Non-zero exit on any error (CI-ready).""" from nerve.config_validate import validate_config_bundle @@ -1285,7 +1306,7 @@ def config_validate( result = validate_config_bundle( config_dir, workspace_override=workspace, strict_env=strict_env, strict_keys=strict_keys, - portable_only=portable_only, + portable_only=portable_only, assume_locked=assume_locked, ) for msg in result.info: click.echo(f"[info] {msg}") diff --git a/nerve/config.py b/nerve/config.py index 99661e98..83917e89 100644 --- a/nerve/config.py +++ b/nerve/config.py @@ -15,6 +15,8 @@ from typing import Any from nerve import paths +from nerve.coerce import FALSY, TRUTHY +from nerve.coerce import as_bool as _as_bool from nerve.coerce import coerced as _coerced from nerve.coerce import lenient_int as _lenient_int @@ -240,7 +242,11 @@ def _read_yaml_mapping(path: Path, *, strict: bool = False) -> dict[str, Any]: except yaml.YAMLError as e: raise ConfigError(f"Failed to parse {path}: {e}") from e except (OSError, UnicodeDecodeError) as e: - raise ConfigError(f"Failed to read {path}: {e}") from e + # A file that exists but cannot be read — mode 000, a directory in its + # place, a dead mount — or one whose bytes are not the encoding we pinned. + # Same promise as a parse failure: the callers that route around a broken + # config report it, they don't show a traceback. + raise ConfigError(f"Cannot read {path}: {e}") from e if data is None: return {} if not isinstance(data, dict): @@ -259,6 +265,43 @@ def workspace_config_dir(workspace: Path) -> Path: return Path(workspace) / "config" +# The reviewed surface of a workspace: what a locked instance takes as +# instruction rather than as data, and therefore what may only arrive as a +# reviewed, merged commit. +# +# It is not one directory because the layout is not one directory. ``config/`` +# is the declarative half. ``skills/`` is executable policy in all but name: a +# SKILL.md is model-invocable text carrying its own ``allowed-tools`` +# frontmatter, and ``SkillManager.discover`` indexes whatever is on disk at +# startup and on every reload without consulting lockdown, so a file dropped +# there is live on the next sync. The skill endpoints already refuse under +# lockdown; refusing the endpoint and not the file it would have written is not +# a refusal. The root instruction files are the system prompt itself — the +# standing orders every turn starts from. +# +# Not a discovery list, and deliberately not derived from one: +# ``nerve.agent.prompts.PROMPT_FILES`` also names TASK.md, which is runtime state +# the agent maintains and has to go on being able to write. +REVIEWED_DIRS = ("config", "skills") +REVIEWED_ROOT_FILES = frozenset({ + "AGENTS.md", "SOUL.md", "IDENTITY.md", "USER.md", "TOOLS.md", +}) + + +def workspace_reviewed_paths(workspace: Path) -> list[Path]: + """Every member of the reviewed surface of ``workspace``, subtrees first. + + One list so the write guard and sync's dirty-state check ask about the same + files. They answer different questions — may this be written, has this been + edited locally — and when the two disagreed about what the surface *is*, a + skill the reviewer never saw was both writable and invisible to sync. + """ + ws = Path(workspace) + return [ws / name for name in REVIEWED_DIRS] + [ + ws / name for name in sorted(REVIEWED_ROOT_FILES) + ] + + def workspace_settings_file(workspace: Path) -> Path: """Shareable settings file inside a workspace (``/config/settings.yaml``).""" return workspace_config_dir(workspace) / "settings.yaml" @@ -285,6 +328,221 @@ def _load_workspace_settings(workspace: Path) -> dict[str, Any]: return data +# The literal an unresolved ``${VAR}`` reference leaves behind. Interpolation is +# a string substitution, so a reference nothing resolved is still sitting in the +# value — which is how a "present" secret can turn out to be nothing of the kind. +_UNRESOLVED_REF = "${" + +# The two variables that anchor lockdown outside every file on the box. Set in +# the service definition (systemd ``Environment=``, docker ``-e``), which is the +# one place neither the agent nor a config edit reaches. +LOCKDOWN_ANCHOR_ENV = "NERVE_LOCKDOWN" +WORKSPACE_ANCHOR_ENV = "NERVE_WORKSPACE" + + +def lockdown_anchor() -> bool: + """Whether the environment forces this instance into lockdown. + + Hardening the flag's *value* does nothing about which file supplies it, and + that file is chosen by ``workspace:`` in the machine-local ``config.yaml``. + One local edit repoints the workspace at a tree whose ``settings.yaml`` says + nothing, lockdown evaluates to false, the machine layers come back, and with + them ``auth.jwt_secret``. Every guarantee in this module rests on a file the + thing being guarded against can rewrite. The anchor is the way out: an + environment variable set by whatever starts the daemon is outside the config + tree, outside the workspace, and outside anything the agent can edit. + + **The anchor can only lock.** The effective state is this OR the tracked + flag, so ``NERVE_LOCKDOWN=false`` and an unset variable both mean "the + environment has no opinion", never "force unlocked". Monotonicity is the + property worth having: once an operator has anchored a box, nothing that + arrives in a file can take it back — including a file the agent wrote. It + also means the anchor cannot be used to *escape* a locked tracked config; + unlocking still takes a reviewed, merged change. + + An empty value is "no opinion" rather than an error, unlike an empty + ``lockdown:`` in the tracked file. ``Environment=NERVE_LOCKDOWN=`` and a + bare ``docker -e NERVE_LOCKDOWN`` both produce it, and under the OR above it + can only ever fail to *add* a restriction — the tracked flag still decides. + An unreadable value is refused, because the only other reading is "no + opinion", which would silently discard an operator's intent to lock. + """ + raw = os.environ.get(LOCKDOWN_ANCHOR_ENV) + if raw is None: + return False + text = raw.strip().lower() + if not text or text in FALSY: + return False + if text in TRUTHY: + return True + raise ConfigError( + f"{LOCKDOWN_ANCHOR_ENV}={raw!r} is neither true nor false. Accepted " + f"spellings are true/false, 1/0, yes/no, on/off (case-insensitive); " + f"unset or empty means the environment has no opinion and the tracked " + f"settings decide. Refused rather than assumed, because assuming would " + f"mean discarding an instruction to lock this instance." + ) + + +def _anchored_workspace() -> Path: + """The workspace an env-anchored instance must use. + + Anchoring the flag alone buys nothing. ``workspace:`` still comes from the + machine-local ``config.yaml``, so an attacker who can write that file + repoints the workspace and gets an instance that is locked *onto config they + supplied* — strictly worse than an unlocked one, because it now treats that + tree as its sole authority and stops reading anything else. The anchor has + to cover both, so the two variables are a single deployment unit: whatever + sets ``NERVE_LOCKDOWN`` sets ``NERVE_WORKSPACE`` beside it. + + Refused rather than defaulted. Falling back to ``~/nerve-workspace`` would + quietly re-admit a path that is a machine-local decision, and defaulting is + how an operator ends up believing an anchor holds when it does not. + """ + raw = os.environ.get(WORKSPACE_ANCHOR_ENV, "").strip() + if not raw: + raise ConfigError( + f"{LOCKDOWN_ANCHOR_ENV} anchors this instance in lockdown, but " + f"{WORKSPACE_ANCHOR_ENV} is not set. Anchoring the flag alone is not " + f"an anchor: the workspace — which selects the settings.yaml that is " + f"then the only source of truth — would still come from the " + f"machine-local config.yaml. Set both in the service definition " + f"(systemd Environment=, docker -e)." + ) + workspace = _expand_path(raw) + if workspace is None or not workspace.is_absolute(): + raise ConfigError( + f"{WORKSPACE_ANCHOR_ENV}={raw!r} must be an absolute path — a " + f"relative one means a different directory in every process that " + f"reads it." + ) + return workspace + + +def _as_lockdown(value: Any) -> bool: + """Parse the ``lockdown`` flag, refusing anything that isn't a plain yes/no. + + Every other boolean in this module is coerced *leniently*: a value nobody can + parse falls back to the field's declared default, so one typo in a section + that isn't even switched on cannot stop the daemon from booting. That trade + is the wrong way round here, because this field's default is ``False``, and + ``False`` is the position in which nothing else is protected — the + machine-local layers come back, the legacy cron directory comes back, and + runtime writes to tracked config are allowed again. Guessing "off" from a + value we could not read would quietly undo every guarantee the flag exists to + make, and it would do it on a box nobody is watching. So an unreadable value + is a hard :class:`ConfigError` instead: the instance does not start, and + ``nerve config validate`` reports it before the change is ever merged. + + ``None`` (a bare ``lockdown:`` in YAML, or the key absent) is off — that is + the file declining to set the flag, which is what the default is for. An + *empty string* is not: it arrives from an environment variable that exists + and carries no value, and while blanking a variable is a perfectly good way + to switch an ordinary feature off, the one switch that governs whether + anything else is enforced must not be flipped by a value that says nothing. + Spell a default-unlocked box ``${VAR:-false}``. + """ + if isinstance(value, bool): + return value + if value is None: + return False + shown = repr(value) + if isinstance(value, str): + # The flag decides which layers get merged, so it is read before the + # single post-merge interpolation pass and has to resolve its own + # reference — otherwise `lockdown: ${NERVE_LOCKDOWN}` is judged as the + # literal string it still is, which is truthy whatever the variable says. + resolved = _interpolate_str(value, []).strip() + if resolved.lower() in TRUTHY: + return True + if resolved.lower() in FALSY: + return False + if resolved != value.strip(): + shown = f"{value!r} (which resolved to {resolved!r})" + if _UNRESOLVED_REF in resolved: + raise ConfigError( + f"lockdown: {shown} names an environment variable that is not " + f"set, so whether this instance is locked cannot be determined. " + f"Set it, or write ${{VAR:-false}} to declare the unlocked " + f"default explicitly." + ) + raise ConfigError( + f"lockdown must be true or false, got {shown}. Accepted spellings are " + f"true/false, 1/0, yes/no, on/off (case-insensitive); an empty value is " + f"not one of them. Refused rather than assumed, because assuming would " + f"mean assuming 'unlocked' — the setting that drops every restriction " + f"lockdown exists to impose." + ) + + +def _resolved(path: Path) -> Path: + """``path`` with symlinks followed; the path itself if it cannot be resolved.""" + try: + return path.resolve() + except (OSError, RuntimeError, ValueError): + return path + + +def _is_within(path: Path, root: Path) -> bool: + """True if ``path`` resolves inside ``root``, symlinks followed. + + Both sides are resolved, so ``..`` segments, an absolute path elsewhere, a + symlink pointing out of the tree, and a path inside the workspace but + outside its ``config/`` subtree all answer False. An unreadable path (a + symlink loop, a permission refusal partway up) answers False too: not being + able to establish that something is contained is not containment. + + Note what resolving ``root`` too means: containment is judged *relative to + wherever root actually is*. If ``root`` is itself a symlink out of the tree, + everything under it is contained and this answers True — correctly, for the + question it is asked. Whether ``root`` is the tree it claims to be is a + separate question, and :func:`lockdown_workspace_problems` is where it is + asked. + """ + try: + return path.resolve().is_relative_to(root.resolve()) + except (OSError, RuntimeError, ValueError): + return False + + +def _is_lexically_within(path: Path, root: Path) -> bool: + """True if ``path`` *names* a location under ``root``, symlinks not followed. + + The counterpart to :func:`_is_within`, and needed because the two disagree + exactly where a symlink is. ``_is_within`` answers "where does this path end + up", which is the right question for a path that reaches into the subtree + from outside. It is the wrong question for a reviewed *name*: put a symlink + at ``config/settings.yaml`` and the resolved path leaves the subtree, so a + guard that only resolves concludes the write is none of its business and + permits it — writing, through the link, the file the daemon then loads as + tracked config. + + ``normpath`` collapses ``.`` and ``..`` textually, without touching the + filesystem, which is the point: a lexical comparison a symlink can influence + would answer the resolved question again. Collapsing ``..`` across a + symlinked directory can name a location the OS would not, so this can + over-refuse; callers pair it with :func:`_is_within` and only ever add + refusals that way. + """ + try: + return Path(os.path.normpath(path)).is_relative_to(os.path.normpath(root)) + except (OSError, RuntimeError, ValueError): + return False + + +def read_machine_layers(config_dir: Path) -> dict[str, Any]: + """The machine-local overlay on its own, for diagnostics. + + Lockdown discards these layers, so reporting what was discarded needs a way + to read them afterwards. Two small YAML reads, not memoized: a caller asking + for this wants the current contents of the files. + """ + return _deep_merge( + _read_yaml_mapping(config_dir / "config.yaml"), + _read_yaml_mapping(config_dir / "config.local.yaml"), + ) + + def _read_config_sources(config_dir: Path) -> dict[str, Any]: """Merge all config layers for ``config_dir`` and resolve ${ENV_VAR} refs. @@ -294,22 +552,57 @@ def _read_config_sources(config_dir: Path) -> dict[str, Any]: base = _read_yaml_mapping(config_dir / "config.yaml") local = _read_yaml_mapping(config_dir / "config.local.yaml") - # The workspace path comes from the machine-local config (or the default), - # never from the tracked settings file — resolve it first. Best-effort - # ${VAR}/${VAR:-default} interpolation is applied so an env-based workspace - # path is honored when locating settings.yaml; an unresolved required ref is - # left intact here and surfaces later in the full _resolve_env_refs pass. machine = _deep_merge(base, local) - ws_raw = machine.get("workspace") - if isinstance(ws_raw, str) and "${" in ws_raw: - ws_raw = _interpolate_str(ws_raw, []) - workspace = _expand_path(ws_raw) or paths.default_workspace() + # An env-anchored instance takes its workspace from the environment too, so + # that the file which decides everything else cannot decide *which* file that + # is. Without the anchor nothing changes: the workspace comes from the + # machine-local config, which is where it has always come from and which is + # the right place for a per-machine path. + anchored = lockdown_anchor() + if anchored: + workspace = _anchored_workspace() + else: + # The workspace path comes from the machine-local config (or the + # default), never from the tracked settings file — resolve it first. + # Best-effort ${VAR}/${VAR:-default} interpolation is applied so an + # env-based workspace path is honored when locating settings.yaml; an + # unresolved required ref is left intact here and surfaces later in the + # full _resolve_env_refs pass. + ws_raw = machine.get("workspace") + if isinstance(ws_raw, str) and "${" in ws_raw: + ws_raw = _interpolate_str(ws_raw, []) + workspace = _expand_path(ws_raw) or paths.default_workspace() ws_settings = _load_workspace_settings(workspace) - # Precedence, lowest first: workspace settings < config.yaml < config.local.yaml - merged = _deep_merge(ws_settings, base) - merged = _deep_merge(merged, local) + # Lockdown is owned by the *tracked* settings file only, so a local edit to + # config.yaml/config.local.yaml can't unlock (or fake-lock) an instance — the + # remote is the authority. When locked, the machine layers are dropped + # entirely: config comes only from workspace/config + ${ENV_VAR} (secrets from + # the environment), never from local drop-in files. + # OR, not override: the environment can add lockdown but never remove it. + # The tracked flag is still evaluated (and still refuses an unreadable value) + # so an anchored box doesn't quietly run a bundle nobody can parse. + locked = _as_lockdown(ws_settings.get("lockdown")) or anchored + if "lockdown" in machine: + # Mirrors the warning for a `workspace` key in settings.yaml. Ignoring it + # is the point of the feature, but ignoring it *in silence* leaves someone + # who wrote it in the wrong file believing the box is locked when nothing + # about it is, and no other signal would ever tell them. + logger.warning( + "config: ignoring 'lockdown' in config.yaml/config.local.yaml — the " + "flag is honored only in workspace/config/settings.yaml, so that a " + "local edit cannot unlock or fake-lock an instance. This instance is " + "%s.", "LOCKED" if locked else "not locked", + ) + if locked: + merged = dict(ws_settings) + merged["workspace"] = str(workspace) # keep the machine-local workspace path + else: + # Precedence, lowest first: settings < config.yaml < config.local.yaml + merged = _deep_merge(_deep_merge(ws_settings, base), local) + # Authoritative: the effective lockdown flag matches the resolution decision. + merged["lockdown"] = locked return _resolve_env_refs(merged) @@ -562,7 +855,7 @@ class TelegramConfig: @classmethod @_coerced - def from_dict(cls, d: dict) -> TelegramConfig: + def from_dict(cls, d: dict, locked: bool = False) -> TelegramConfig: dm_policy = d.get("dm_policy", "pairing") if dm_policy not in ("pairing", "open"): logger.warning( @@ -571,8 +864,27 @@ def from_dict(cls, d: dict) -> TelegramConfig: dm_policy, ) dm_policy = "pairing" + # Whether a given box answers Telegram DMs is a per-machine decision, so + # `nerve init` writes this key to the machine-local config.yaml. Lockdown + # drops that layer, which would leave the declared default — on — + # deciding for it: a box where Telegram was switched off would start + # serving DMs, with full agent access, the moment the shared settings + # carried a token this machine's environment can resolve. + # + # So the fallback direction is chosen by lockdown, and parsed here rather + # than left to @_coerced. The decorator falls back to the *declared* + # default, which is True, so `enabled: ${TG_ON:-nope}` — an env-reference + # spelling this file's own docs recommend for per-box fleet config — would + # turn the bot on everywhere one bad value reached. Deliberately still + # lenient rather than fatal: an unreadable value in one section must not + # stop the daemon booting, and off is the safe end of the guess. + enabled = ( + _as_bool(d["enabled"], not locked, label="TelegramConfig.enabled") + if "enabled" in d + else not locked + ) return cls( - enabled=d.get("enabled", True), + enabled=enabled, bot_token=d.get("bot_token", ""), # Deliberately uncast. The declared list[int] is converted after # construction, which logs an unconvertible entry and keeps it @@ -917,7 +1229,7 @@ def _cron_dir_has_jobs(d: Path) -> bool: return (d / "jobs.yaml").exists() or (d / "system.yaml").exists() -def _resolve_cron_dir(workspace: Path | None) -> Path: +def _resolve_cron_dir(workspace: Path | None, locked: bool = False) -> Path: """Effective directory holding cron config (jobs/system/gates). Prefers the git-syncable ``workspace/config/cron`` so cron definitions live @@ -929,11 +1241,16 @@ def _resolve_cron_dir(workspace: Path | None) -> Path: * If the workspace location has job files → use it. * Else, if the legacy location has job files → use it (un-migrated install). * Else → the workspace location (new install; may be about to be populated). + + Under ``locked`` (lockdown), the legacy ``~/.nerve/cron`` fallback is disabled + entirely — cron config comes only from the workspace. """ legacy = paths.cron_dir() if workspace is None: return legacy ws_cron = workspace_config_dir(workspace) / "cron" + if locked: + return ws_cron if _cron_dir_has_jobs(ws_cron): return ws_cron if _cron_dir_has_jobs(legacy): @@ -941,6 +1258,49 @@ def _resolve_cron_dir(workspace: Path | None) -> Path: return ws_cron +def _locked_cron_path(configured: Path, default: Path, root: Path, key: str) -> Path: + """Keep one locked cron path inside the tracked config subtree. + + A locked instance runs only what the reviewed workspace repo carries, and + these three keys decide which files it reads — including, for + ``gate_plugins_dir``, which ``.py`` files it imports and *executes* at + start-up and on every cron reload. A tracked ``settings.yaml`` that pointed + any of them elsewhere would hand the box job definitions and code that never + went through review, using nothing but a YAML edit. So a path that escapes is + dropped in favour of the in-workspace default and reported. + + Called only when locked. An unmigrated install is entitled to point cron at + the legacy ``~/.nerve/cron``, and that is the whole reason the machine-local + layers exist. + + The default is verified too, and the instance refuses to load when it also + escapes. That is not a defensive afterthought — it is the case with no YAML + key in it at all. ``/config/cron/gates`` is a name a config repo + can carry as a *symlink*, and git tracks symlinks, so a reviewed and merged + bundle can point the default itself anywhere. Substituting the default there + would hand back the exact path just rejected, log a warning that contradicts + itself, and import whatever was on the other end. + """ + if _is_within(configured, root): + return configured + if _is_within(default, root): + logger.warning( + "cron.%s (%s) resolves outside %s — ignoring it and using %s " + "instead, because a locked instance takes its cron config, and its " + "gate plugins, only from the tracked workspace", + key, configured, root, default, + ) + return default + also_default = "" if configured == default else f" — as does the default {default}" + raise ConfigError( + f"lockdown: cron.{key} resolves outside the tracked config subtree " + f"{root}: {configured} points at {_resolved(configured)}{also_default}. " + f"A locked instance reads its cron config, and imports its gate plugins, " + f"only from the reviewed workspace, and there is no contained path left " + f"to fall back to." + ) + + @dataclass class CronConfig: # Bare defaults (no workspace context) point at the legacy machine-local @@ -953,12 +1313,33 @@ class CronConfig: @classmethod @_coerced - def from_dict(cls, d: dict, workspace: Path | None = None) -> CronConfig: - base = _resolve_cron_dir(workspace) + def from_dict(cls, d: dict, workspace: Path | None = None, locked: bool = False) -> CronConfig: + base = _resolve_cron_dir(workspace, locked=locked) + jobs_file = _expand_path(d.get("jobs_file")) or base / "jobs.yaml" + system_file = _expand_path(d.get("system_file")) or base / "system.yaml" + gate_plugins_dir = _expand_path(d.get("gate_plugins_dir")) or base / "gates" + if locked and workspace is not None: + root = workspace_config_dir(workspace) + if not _is_within(base, root): + # The subtree itself leaves the workspace — a symlinked + # config/cron, say. No substitution fixes that, since every + # default is derived from it, so refuse to run rather than read + # cron config from a directory outside the reviewed tree. + raise ConfigError( + f"lockdown: the cron directory {base} resolves outside the " + f"tracked config subtree {root}. A locked instance must read " + f"its cron config, and import its gate plugins, only from the " + f"reviewed workspace." + ) + jobs_file = _locked_cron_path(jobs_file, base / "jobs.yaml", root, "jobs_file") + system_file = _locked_cron_path(system_file, base / "system.yaml", root, "system_file") + gate_plugins_dir = _locked_cron_path( + gate_plugins_dir, base / "gates", root, "gate_plugins_dir", + ) return cls( - jobs_file=_expand_path(d.get("jobs_file")) or base / "jobs.yaml", - system_file=_expand_path(d.get("system_file")) or base / "system.yaml", - gate_plugins_dir=_expand_path(d.get("gate_plugins_dir")) or base / "gates", + jobs_file=jobs_file, + system_file=system_file, + gate_plugins_dir=gate_plugins_dir, ) @@ -1913,6 +2294,11 @@ def from_dict(cls, d: dict) -> "XmemoryConfig": class NerveConfig: workspace: Path = field(default_factory=paths.default_workspace) timezone: str = "America/New_York" + # Remote-only, read-only mode. When set (in workspace/config/settings.yaml — + # the tracked file the remote controls), config comes ONLY from the workspace + # + ${ENV_VAR}; machine config.yaml/config.local.yaml overrides and legacy + # ~/.nerve/cron are ignored, and runtime edits to tracked config are blocked. + lockdown: bool = False deployment: str = "server" # "server" or "docker" quiet_start: str = "02:00" # HH:MM — start of quiet period (local timezone) quiet_end: str = "08:00" # HH:MM — end of quiet period (local timezone) @@ -2116,19 +2502,25 @@ def _build_from_dict(cls, d: dict) -> NerveConfig: # Resolve the workspace once so workspace-aware sub-configs (e.g. cron, # which now lives in workspace/config/cron) can be located relative to it. workspace = _expand_path(d.get("workspace")) or paths.default_workspace() + # Parsed here rather than left to @_coerced, which would fall back to the + # field's default — i.e. to "unlocked" — for a value it cannot read. It is + # also needed *before* construction, because it changes how the cron paths + # and the Telegram switch resolve. See :func:`_as_lockdown`. + locked = _as_lockdown(d.get("lockdown")) return cls( workspace=workspace, timezone=d.get("timezone", "America/New_York"), + lockdown=locked, deployment=d.get("deployment", "server"), quiet_start=d.get("quiet_start", "02:00"), quiet_end=d.get("quiet_end", "08:00"), provider=ProviderConfig.from_dict(d.get("provider", {})), gateway=GatewayConfig.from_dict(d.get("gateway", {})), agent=AgentConfig.from_dict(d.get("agent", {})), - telegram=TelegramConfig.from_dict(d.get("telegram", {})), + telegram=TelegramConfig.from_dict(d.get("telegram", {}), locked=locked), sync=SyncConfig.from_dict(d.get("sync", {})), memory=MemoryConfig.from_dict(d.get("memory", {})), - cron=CronConfig.from_dict(d.get("cron", {}), workspace=workspace), + cron=CronConfig.from_dict(d.get("cron", {}), workspace=workspace, locked=locked), workspace_sync=WorkspaceSyncConfig.from_dict(d.get("workspace_sync", {})), backup=BackupConfig.from_dict(d.get("backup", {})), sessions=SessionsConfig.from_dict(d.get("sessions", {})), @@ -2271,9 +2663,247 @@ def load_config(config_dir: Path | None = None) -> NerveConfig: config = NerveConfig.from_dict(merged) config.config_dir = Path(config_dir) + problems = lockdown_workspace_problems(config.workspace) if config.lockdown else [] + for problem in problems: + # Fail closed: don't boot a locked instance that would take its "tracked" + # config from somewhere the config repo never saw. + raise ConfigError(problem) + for note in lockdown_machine_local_notes( + merged, read_machine_layers(config_dir) if config.lockdown else None + ): + logger.warning("config: %s", note) return config +def lockdown_workspace_problems(workspace: Path) -> list[str]: + """Ways a locked instance's reviewed subtrees aren't the trees they claim. + + Everything else here judges a path against ``/config`` or + ``/skills``. That settles whether a path stays inside a subtree, + and says nothing about whether the subtree is the one the config repo was + reviewed in. If ``config`` is a symlink out of the workspace, every path + under it is contained relative to itself, every containment check passes, and + the whole tracked layer — settings.yaml, the cron files, the gate plugins the + daemon imports — comes from an untracked directory somewhere else on the box, + with lockdown itself read from there and reporting that all is well. A + redirected ``skills`` is the same arrangement one directory over: whatever is + there gets indexed as model-invocable skills on the next reload. + + So each subtree is judged against the workspace instead, once, and a locked + instance that fails does not start. The workspace *path* may still be + anything, including a symlink: where a machine keeps its workspace is a + machine-local decision and the one thing lockdown never took away. + + Two more ways the same claim fails, checked here for the same reason. A + subtree can stay inside the workspace and still not be the reviewed tree — + ``config`` as a symlink to a sibling directory — and ``settings.yaml`` alone + can leave it while everything around it stays put. + + One problem here is not about a subtree at all: whether the workspace has a + git remote. It belongs with these because it is the same claim from the other + side — the reviewed surface is only reviewed if reviewed changes can reach it. + See :func:`_workspace_remote_problem`. + + Every problem found is reported rather than the first: an operator fixing a + layout wants the whole list, and the caller that refuses to boot only needs + one of them. + """ + problems: list[str] = [] + config_root = workspace_config_dir(workspace) + for reviewed in (Path(workspace) / name for name in REVIEWED_DIRS): + if not _is_within(reviewed, workspace): + problems.append( + f"lockdown is enabled but the reviewed subtree {reviewed} resolves " + f"to {_resolved(reviewed)}, outside the workspace " + f"{_resolved(workspace)} — so nothing it holds is part of the " + f"reviewed repo. A locked instance reads {reviewed.name}/ only " + f"from the workspace itself." + ) + elif reviewed.is_symlink(): + # Contained, and still not the reviewed tree. git does not descend + # into a symlinked directory, so `git status -- /config` reports + # nothing whatever is under there: sync calls the workspace clean + # while the daemon reads settings.yaml, and imports cron/gates/*.py, + # from a directory the config repo has never seen a single file of. + # A redirected skills/ is the same arrangement, with the skill index + # reading whatever the link points at. + problems.append( + f"lockdown is enabled but the reviewed subtree {reviewed} is a " + f"symlink to {_resolved(reviewed)}. git does not track anything " + f"through a symlinked directory, so no file under it is part of " + f"the reviewed repo and a sync would report the workspace clean " + f"regardless of what it holds. A locked instance's " + f"{reviewed.name}/ must be a real directory." + ) + + # The subtree can be the reviewed tree and this one file still not be part of + # it. settings.yaml carries the lockdown flag itself, the auth secret and + # every cron path, so a symlink here is the whole tracked layer arriving from + # somewhere the reviewer never looked — including from an ordinary workspace + # file the agent may write, which is why "still inside the workspace" is not + # good enough. The write guard refuses the name; this refuses the instance, + # because a file that is already in place was never written through the guard. + settings = workspace_settings_file(workspace) + if not _is_within(settings, config_root): + problems.append( + f"lockdown is enabled but the tracked settings file {settings} resolves " + f"to {_resolved(settings)}, outside the tracked config subtree " + f"{_resolved(config_root)} — so the file that states lockdown, and " + f"everything else this instance runs on, is not part of the reviewed " + f"repo. A locked instance reads its settings only from the workspace's " + f"own config/ directory." + ) + + remote = _workspace_remote_problem(Path(workspace)) + if remote: + problems.append(remote) + return problems + + +def _workspace_remote_problem(workspace: Path) -> str | None: + """Why this locked workspace has nowhere to receive reviewed config from. + + Lockdown refuses every local change to the reviewed surface and its refusals + say so, naming the PR flow as the way to change config. That flow ends in a + git pull: sync is the only route left in. A locked workspace that is not a + repository, or is one with no remote, has no route at all — every local + change is refused and no remote change can arrive, so the instance keeps the + configuration it happens to hold and nothing on the box says why. Nothing + else reports it, because every other check here judges a tree that is present + and this one is about a tree that can never change. + + Refused rather than ignored, for the reason the other two rules exist: + ``lockdown`` in config.yaml/config.local.yaml is not read, and + ``NERVE_LOCKDOWN`` can lock but never unlock, both so that a machine-local + change cannot turn lockdown off. ``git remote remove origin`` is a + machine-local change. Honoring the flag only when a remote happens to be + configured would make it one more way to unlock the box. + + Any remote counts. Which one to fetch is sync's decision, not this one's: + with ``workspace_sync.branch`` unset it follows the current branch's own + upstream, which need not be named ``origin``, so demanding that name would + refuse a workspace that syncs today. Whether the periodic loop is enabled is + not asked either — ``nerve config sync`` and ``POST /api/config/sync`` are + manual routes that work on a locked box with the loop off, so a remote is a + route regardless. + + Git being unusable is treated as no remote. The answer decides whether a + locked instance starts, and "could not tell" is not "yes"; a box that cannot + run git cannot sync either. + """ + # Imported inside the function because sync_service imports this module. Its + # git runner is the one the sync route itself uses: captured, timed out and + # incapable of raising, which is what a call on the config load path needs. + from nerve.sync_service import _git, is_git_repo + + if not is_git_repo(workspace): + found = f"the workspace {workspace} is not a git repository" + remedy = "Clone the config repo as the workspace" + else: + listed = _git(["remote"], workspace) + if listed.returncode == 0 and listed.stdout.strip(): + return None + if listed.returncode == 0: + found = f"the workspace repository {workspace} has no git remote" + remedy = "Add the config repo as a remote (git remote add origin )" + else: + found = ( + f"git could not list the remotes of the workspace repository " + f"{workspace}: {listed.stderr.strip() or listed.stdout.strip()}" + ) + remedy = "Fix the error git reports" + return ( + f"lockdown is enabled but {found}, so reviewed config has no way to reach " + f"this instance. A locked instance refuses every local change to its " + f"reviewed surface and takes changes only as a merged commit that sync " + f"pulls in, so it would keep the config it currently holds for good. " + f"{remedy}, or drop lockdown from the tracked settings." + ) + + +def _has_dotted(data: dict[str, Any], dotted: str) -> bool: + """Whether ``dotted`` names a key present in ``data``. + + Presence, not truthiness: ``enabled: false`` and ``accounts: []`` are stated + values, and reporting them as unstated would be wrong. + """ + node: Any = data + for part in dotted.split("."): + if not isinstance(node, dict) or part not in node: + return False + node = node[part] + return True + + +def lockdown_machine_local_notes( + merged: dict[str, Any], machine: dict[str, Any] | None = None +) -> list[str]: + """Machine-local settings a locked bundle leaves for the defaults to decide. + + Lockdown reads only the tracked settings, so a key whose natural home is + ``config.yaml`` has nowhere left to be said — and the declared default + silently answers for it on every box in the fleet. That is invisible in the + diff of the change that turned lockdown on, which is exactly when someone + could still do something about it, so name it while the config is being + loaded and while it is being validated. + + The per-box answer belongs in the tracked settings as an ``${ENV_VAR}`` + reference; that is the only channel a locked instance still has. + + ``machine`` is the dropped ``config.yaml``/``config.local.yaml`` overlay, + which callers still hold at the point lockdown discards it. Without it this + function can only report that a key is absent from the tracked settings, and + most machine-local keys are legitimately absent with an acceptable default — + so warning on all of them is noise. With it, the note fires only when a local + file states something that lockdown is about to ignore. + """ + if not _as_lockdown(merged.get("lockdown")): + return [] + notes: list[str] = [] + + if machine: + # The list of machine-local paths lives next to the wizard split it + # mirrors, and is guarded by tests in both directions. Imported inside the + # function because nerve.migrate imports this module. + from nerve.migrate import _is_machine_local, _leaf_paths + + # `workspace` is the one key lockdown keeps, so it is never dropped. + dropped = sorted( + path + for path in _leaf_paths(machine) + if path != "workspace" and not _has_dotted(merged, path) + ) + shareable = [p for p in dropped if not _is_machine_local(p)] + local_only = [p for p in dropped if _is_machine_local(p)] + if shareable: + notes.append( + f"{', '.join(shareable)} are set in config.yaml/config.local.yaml" + " and not in the tracked settings. Lockdown does not read those" + " files, so this instance is using the declared defaults." + " These values are shareable: move them to" + " workspace/config/settings.yaml." + ) + if local_only: + notes.append( + f"{', '.join(local_only)} are machine-local, so lockdown leaves" + " them at their declared defaults. If a default is not the right" + " answer for this box, state the key in the tracked settings as" + " a ${VAR} reference." + ) + + telegram = merged.get("telegram") + telegram = telegram if isinstance(telegram, dict) else {} + if telegram.get("bot_token") and "enabled" not in telegram: + notes.append( + "workspace/config/settings.yaml sets telegram.bot_token but not " + "telegram.enabled, and lockdown does not read config.yaml (where " + "`nerve init` puts it) — so the Telegram bot stays OFF on this " + "instance. Set telegram.enabled in the tracked settings, using " + "${VAR} if the answer differs per machine." + ) + return notes + + # --- Unknown-key validation --- # YAML keys that are intentionally not dataclass fields — keyed by dotted @@ -2348,7 +2978,11 @@ def append_telegram_allowed_user(config_dir: Path, user_id: int) -> bool: Used by the pairing flow. Reads, merges, and rewrites the local config (config.local.yaml is generated — comment loss is acceptable there). Returns True if the file was updated (False if the ID was already present). + + Blocked under lockdown: config.local.yaml isn't even loaded when locked, and + tracked config is remote-only — pair by updating the workspace repo instead. """ + ensure_not_locked("add a Telegram user") local_path = Path(config_dir) / "config.local.yaml" data: dict[str, Any] = {} if local_path.exists(): @@ -2392,3 +3026,109 @@ def set_config(config: NerveConfig) -> None: """Override the global config (for testing or CLI-driven loading).""" global _config _config = config + + +class LockdownError(RuntimeError): + """Raised when a runtime edit to tracked config is attempted under lockdown.""" + + +def is_locked() -> bool: + """True if the *loaded* config puts this instance in lockdown. + + A statement about the process, not about the box: with no config loaded there + is no lockdown state and this answers False. That is a fail-open, and it is + reachable — the CLI hands ``config=None`` to the commands that exist to repair + a config which failed to load, and never calls :func:`set_config` on that + path. Left as it is deliberately, because every caller of the guards below is + a server-side write path that runs with a loaded config, and making this read + files would put I/O behind a predicate called on every skill toggle to protect + a case none of them are in. + + What that does mean is that a guard cannot be *added* by calling this from a + command on the ``config=None`` path and expecting it to hold. Such a caller + has to be given the flag explicitly. + """ + return _config is not None and bool(_config.lockdown) + + +def ensure_not_locked(action: str = "modify configuration") -> None: + """Raise :class:`LockdownError` if locked. Call before any runtime edit to + tracked config (skills, cron, pairing, ...).""" + if is_locked(): + raise LockdownError( + f"Cannot {action}: this instance is in lockdown (remote-only, " + "read-only). Change config via a PR to the workspace repo instead." + ) + + +def ensure_path_not_tracked_config(path: Path, action: str = "write") -> None: + """Refuse, under lockdown, to touch a path in the workspace's reviewed surface. + + The other guards sit in front of a specific operation — creating a skill, + persisting a pairing — and know what they are about to change. This one is + for the endpoints that take a caller-supplied path anywhere under the + workspace: whether they are editing tracked config is a property of the + argument, not of the endpoint. Left unguarded, ``/config/`` is + reachable through them, which means both ``settings.yaml`` (including the + ``lockdown`` flag itself) and ``cron/gates/*.py``, which the daemon imports + and runs. Either one turns lockdown off from inside the box it is meant to be + protecting. ``skills/`` and the root instruction files are reachable the same + way and decide what the agent does next, so they are judged the same. + + A path outside that surface is none of lockdown's business — the workspace is + also the agent's working directory, and normal files there are the point. + """ + reason = tracked_config_write_refusal(path) + if reason: + raise LockdownError(f"Cannot {action} {path}: {reason}") + + +def tracked_config_write_refusal(path: Path | str) -> str | None: + """Why a locked instance refuses to write ``path``, or ``None`` to allow it. + + The same rule as :func:`ensure_path_not_tracked_config`, phrased as a value + rather than an exception, because the agent backends express a refusal by + returning one — a raise there aborts the turn instead of telling the model + what to do differently. The wording is aimed at the model for that reason: + it names the alternative, so a capable agent routes to the review flow rather + than retrying the same write with a different tool. + + A relative path is taken as relative to the workspace, which is the agent's + working directory. That can in principle over-refuse for an agent that has + changed directory into a subtree of its own with a ``config/`` in it; erring + that way costs a clear message and a retry, and erring the other way is the + hole this exists to close. + + Every member of :func:`workspace_reviewed_paths` is judged, not ``config/`` + alone. ``skills/`` and the root instruction files are refused by the named + operations already — ``create_skill`` and friends — and leaving them writable + here left the whole point of those refusals reachable by the plainest tool + the agent has: write ``skills/x/SKILL.md``, wait for the reload that indexes + it, and no 403 was ever involved. + + Both views of the path are checked, because a symlink is the case where they + disagree and each direction of the disagreement is a way through. Resolving + catches a path that arrives from outside and lands in the surface — a link in + the workspace pointing at ``config/``, a directory link, a ``..`` climb. The + lexical name catches the reverse: a symlink sitting *at* a reviewed path + resolves elsewhere, and a guard that only resolved would decide the write was + outside the surface and allow it, while the daemon goes on loading that name + as tracked config. + """ + if not is_locked(): + return None + workspace = get_config().workspace + target = Path(path) + if not target.is_absolute(): + target = workspace / target + for reviewed in workspace_reviewed_paths(workspace): + if _is_within(target, reviewed) or _is_lexically_within(target, reviewed): + return ( + f"this instance is in lockdown (remote-only, read-only) and " + f"{target} is part of {reviewed}, which only a reviewed, merged " + f"change to the workspace repo may alter. Open a pull request " + f"against that repo instead; the instance will pick the change up " + f"when it syncs. Memory, task files and the rest of the workspace " + f"are still writable." + ) + return None diff --git a/nerve/config_validate.py b/nerve/config_validate.py index 6b6c98ec..1eee8d1b 100644 --- a/nerve/config_validate.py +++ b/nerve/config_validate.py @@ -43,6 +43,11 @@ class ValidationResult: errors: list[str] = field(default_factory=list) warnings: list[str] = field(default_factory=list) info: list[str] = field(default_factory=list) + #: Names of ``${VAR}`` references the bundle needs and the environment does + #: not have. Kept as data rather than only as prose, because which bucket the + #: prose lands in depends on ``strict_env`` — and a caller that tolerates + #: unset variables still needs to know the daemon will refuse this config. + unresolved_env: list[str] = field(default_factory=list) @property def ok(self) -> bool: @@ -55,6 +60,7 @@ def validate_config_bundle( strict_env: bool = False, strict_keys: bool = False, portable_only: bool = False, + assume_locked: bool = False, ) -> ValidationResult: """Validate the config bundle rooted at ``config_dir``. @@ -71,6 +77,15 @@ def validate_config_bundle( running the check. Pair it with ``workspace_override``: with no machine config left to read the workspace location from, it falls back to the default one. + + ``assume_locked`` judges the locked view whatever the bundle's own + ``lockdown`` flag resolves to here. A fleet repo states the flag as + ``${NERVE_LOCKDOWN:-false}`` so that one bundle can serve locked and unlocked + boxes — which means CI, where the variable is unset, resolves it to false and + validates the view *no locked box will ever run*. The lockdown checks then + have nothing to fire on and the only machine that finds out is the locked one, + at boot. This is how a config repo whose members include a locked instance + checks the configuration that instance will actually load. """ result = ValidationResult() config_dir = Path(config_dir) @@ -91,8 +106,28 @@ def validate_config_bundle( base, local = layers[0] or {}, layers[1] or {} machine = cfg._deep_merge(base, local) + # An environment anchor forces lockdown regardless of the files, so a run in + # the daemon's environment has to judge the same thing the daemon will. + # Errors from a malformed anchor are reported rather than raised — the + # validator's job is to say what is wrong, not to become unusable. + try: + anchored = cfg.lockdown_anchor() + except cfg.ConfigError as e: + result.errors.append(str(e)) + anchored = False + if workspace_override is not None: workspace = Path(workspace_override).expanduser() + elif anchored: + # Same rule as the loader: the anchor selects the workspace too, so an + # unpinned run in an anchored environment looks at the tree the daemon + # will. An explicit --workspace still wins — pointing the validator at a + # checkout is the whole point of the flag, and CI is not the anchored box. + try: + workspace = cfg._anchored_workspace() + except cfg.ConfigError as e: + result.errors.append(str(e)) + workspace = cfg.paths.default_workspace() else: # Resolve the workspace the same way load_config does, incl. best-effort # ${VAR}/${VAR:-default} interpolation of the path itself. Under @@ -105,9 +140,50 @@ def validate_config_bundle( workspace = cfg._expand_path(ws_raw) or cfg.paths.default_workspace() ws_settings = _read_workspace_settings(workspace, result) - merged = cfg._deep_merge(cfg._deep_merge(ws_settings, base), local) - # Before the pin below overwrites the bundle's own ``workspace`` value with - # the resolved one — that value is part of what is under review. + # Mirror _read_config_sources: when the tracked settings lock the instance, + # validate the LOCKED view (workspace-only), since that's what production runs. + # An unreadable flag is refused rather than assumed, exactly as at load time — + # reported here, before the change is merged, which is the whole point. That + # error is already fatal, so nothing is fail-open in carrying on with the + # unlocked view; judging the locked one instead would bury the real problem + # under a pile of errors derived from a lockdown the bundle never asked for. + raw_lockdown = ws_settings.get("lockdown") + try: + locked = cfg._as_lockdown(raw_lockdown) + except cfg.ConfigError as e: + result.errors.append(str(e)) + locked = False + if anchored and not locked: + result.info.append( + f"validating the LOCKED view: {cfg.LOCKDOWN_ANCHOR_ENV} is set in " + f"this environment, which locks the instance whatever the tracked " + f"settings say" + ) + locked = True + if assume_locked and not locked: + result.info.append( + "validating the LOCKED view on request (--assume-lockdown); this " + "bundle's own lockdown flag resolves to false in this environment" + ) + locked = True + elif not locked and isinstance(raw_lockdown, str) and "${" in raw_lockdown: + # The flag is env-controlled and this environment says off, so everything + # below judges a view no locked member of the fleet will run. Say so: + # nobody would otherwise think to ask for the other one. + result.warnings.append( + f"lockdown is set from the environment ({raw_lockdown}) and resolves " + f"to false here, so the locked view was NOT validated — a locked " + f"instance loads a different config than this. Re-run with " + f"--assume-lockdown to check it." + ) + if locked: + merged = dict(ws_settings) + else: + merged = cfg._deep_merge(cfg._deep_merge(ws_settings, base), local) + merged["lockdown"] = locked + # Judge the same view the instance will run, so a locked bundle is checked on + # its tracked values alone. Before the pin below overwrites the bundle's own + # ``workspace`` value with the resolved one — that value is under review too. _validate_working_dir_paths(merged, result) # Pin the workspace so cron paths resolve against the validated workspace. merged["workspace"] = str(workspace) @@ -116,6 +192,7 @@ def validate_config_bundle( missing: list[str] = [] merged = cfg._interpolate_env(merged, missing) env_names = ", ".join(sorted(set(missing))) + result.unresolved_env = sorted(set(missing)) if missing: msg = f"references unset environment variable(s): {env_names}" # Say which layer asked for them. The refs are collected from the merged @@ -196,6 +273,19 @@ def validate_config_bundle( # be env-caused, with a real one behind it. result.errors.append(f"config error: {residual}") + if locked: + # Judged on the workspace the run was pointed at, so a candidate bundle + # is checked against its own checkout rather than this machine's tree. + result.errors.extend(cfg.lockdown_workspace_problems(workspace)) + if config is not None: + # The machine overlay read at the top of this function is what a locked + # instance discards, so pass it through: with it the note can name the + # keys this bundle strands rather than describing the hazard in general. + # Empty under portable_only, which the callee guards against. + result.warnings.extend( + cfg.lockdown_machine_local_notes(merged, machine if locked else None) + ) + # Resolve the cron locations even if full typed construction failed above. if config is not None: cron_files = ( @@ -203,7 +293,7 @@ def validate_config_bundle( ("jobs", config.cron.jobs_file), ) else: - base_cron = cfg._resolve_cron_dir(workspace) + base_cron = cfg._resolve_cron_dir(workspace, locked=locked) cron_files = ( ("system", base_cron / "system.yaml"), ("jobs", base_cron / "jobs.yaml"), diff --git a/nerve/cron/jobs.py b/nerve/cron/jobs.py index d28e3670..c43592d2 100644 --- a/nerve/cron/jobs.py +++ b/nerve/cron/jobs.py @@ -345,7 +345,16 @@ def load_jobs( def save_jobs(jobs: list[CronJob], jobs_file: Path) -> None: - """Save cron jobs to a YAML file.""" + """Save cron jobs to a YAML file. + + Guarded even though nothing calls it today: the file it writes is + ``/config/cron/jobs.yaml`` on a migrated install, which is tracked + config, and a guard added when the first caller appears is a guard that was + missing for however long the caller took to notice. + """ + from nerve.config import ensure_path_not_tracked_config + + ensure_path_not_tracked_config(jobs_file, "save cron jobs to") jobs_file.parent.mkdir(parents=True, exist_ok=True) data = {"jobs": []} for job in jobs: diff --git a/nerve/cron/service.py b/nerve/cron/service.py index 9e67f41f..aa7ac4cc 100644 --- a/nerve/cron/service.py +++ b/nerve/cron/service.py @@ -16,6 +16,7 @@ from apscheduler.triggers.cron import CronTrigger from apscheduler.triggers.interval import IntervalTrigger +from nerve import paths from nerve.agent.engine import AgentEngine from nerve.config import ConfigError, NerveConfig from nerve.cron.jobs import ( @@ -268,6 +269,16 @@ async def start(self) -> None: # Load job definitions from both files self._jobs = self._load_merged_jobs() + # Under lockdown the legacy machine-local cron fallback is disabled, so + # an unpopulated workspace cron dir means zero user crons — surface it. + if self.config.lockdown and not any( + j.metadata.get("_source") == "user" for j in self._jobs + ): + logger.warning( + "Lockdown: no user crons found in %s (legacy %s is ignored when " + "locked)", self.config.cron.jobs_file.parent, paths.path_label("cron"), + ) + # Register cron jobs with persistent timer alignment for job in self._jobs: if not job.enabled: diff --git a/nerve/gateway/auth.py b/nerve/gateway/auth.py index 3ef61956..e6a90149 100644 --- a/nerve/gateway/auth.py +++ b/nerve/gateway/auth.py @@ -141,6 +141,14 @@ async def require_auth(request: Request) -> dict: """FastAPI dependency: require valid authentication.""" config = get_config() if not config.auth.jwt_secret: + if config.lockdown: + # Fail closed: a locked instance must never fall into the open + # dev-mode bypass just because its jwt_secret wasn't supplied via env. + raise HTTPException( + status_code=503, + detail="Locked instance has no auth.jwt_secret (set it via " + "${ENV_VAR} in settings.yaml or the environment).", + ) # Auth not configured — allow access (development mode) return {"sub": "user"} @@ -156,6 +164,8 @@ async def authenticate_websocket(websocket: WebSocket) -> bool: """ config = get_config() if not config.auth.jwt_secret: + if config.lockdown: + return False # Fail closed under lockdown — never open the socket return True # Dev mode # Check query parameter diff --git a/nerve/gateway/routes/config.py b/nerve/gateway/routes/config.py index 36bb9657..733f0394 100644 --- a/nerve/gateway/routes/config.py +++ b/nerve/gateway/routes/config.py @@ -2,6 +2,8 @@ from __future__ import annotations +from pathlib import Path + from fastapi import APIRouter, Depends, HTTPException from nerve.gateway.auth import require_auth @@ -33,6 +35,7 @@ async def sync_workspace_route(user: dict = Depends(require_auth)): branch=config.workspace_sync.branch, validate=config.workspace_sync.validate, strict_env=config.workspace_sync.strict_env, + locked=config.lockdown, ) if not result.ok: raise HTTPException( @@ -43,12 +46,20 @@ async def sync_workspace_route(user: dict = Depends(require_auth)): "warnings": result.validation_warnings, }, ) + apply_error = None if result.changed: deps = get_deps() - await _apply_sync(deps.engine, _cron_service) + config_dir = Path(config.config_dir) if config.config_dir else Path(config.workspace) + apply_error = await _apply_sync(deps.engine, _cron_service, config_dir) + # The merge is a real state change, so this is not a 4xx — but a merge whose + # config the daemon then refused to load has applied nothing, and reporting + # ok for it would tell an operator who just enabled lockdown that the box is + # locked while its write guards are still open. return { - "ok": True, + "ok": apply_error is None, "changed": result.changed, + "applied": result.changed and apply_error is None, + "apply_error": apply_error, "message": result.message, "old_rev": result.old_rev, "new_rev": result.new_rev, diff --git a/nerve/gateway/routes/external_agents.py b/nerve/gateway/routes/external_agents.py index 0834d8bb..0bd4c401 100644 --- a/nerve/gateway/routes/external_agents.py +++ b/nerve/gateway/routes/external_agents.py @@ -22,7 +22,7 @@ from fastapi import APIRouter, Depends, HTTPException from pydantic import BaseModel -from nerve.config import get_config, set_config +from nerve.config import ensure_not_locked, get_config, set_config from nerve.external_agents.registry import AGENT_REGISTRY from nerve.gateway.auth import require_auth from nerve.gateway.routes._deps import get_deps @@ -125,6 +125,7 @@ async def remove_agent(name: str, user: dict = Depends(require_auth)) -> dict[st user can re-add later without re-bootstrapping. To wipe the files too, they should delete them manually. """ + ensure_not_locked("remove an external agent") config = get_config() before = len(config.external_agents.targets) config.external_agents.targets = [ @@ -139,6 +140,7 @@ async def remove_agent(name: str, user: dict = Depends(require_auth)) -> dict[st def _toggle_agent(name: str, *, enabled: bool) -> ToggleResponse: + ensure_not_locked("change external agents") config = get_config() target = next( (t for t in config.external_agents.targets if t.name == name), diff --git a/nerve/gateway/routes/memory.py b/nerve/gateway/routes/memory.py index 59220482..d38a3665 100644 --- a/nerve/gateway/routes/memory.py +++ b/nerve/gateway/routes/memory.py @@ -12,7 +12,11 @@ from fastapi import APIRouter, Depends, HTTPException, Response from pydantic import BaseModel -from nerve.config import get_config +from nerve.config import ( + ensure_path_not_tracked_config, + get_config, + tracked_config_write_refusal, +) from nerve.gateway.auth import require_auth from nerve.gateway.routes._deps import get_deps @@ -24,7 +28,14 @@ # --- Memory files --- def _list_memory_files_sync(workspace: Path) -> list[dict]: - """Walk + stat memory markdown files. Sync — call via asyncio.to_thread.""" + """Walk + stat memory markdown files. Sync — call via asyncio.to_thread. + + Each entry carries ``read_only``, answered by the same guard the write route + enforces. The listing includes the workspace's root markdown, which is where + SOUL.md and AGENTS.md live, and those are reviewed files a locked instance + will not write — so without this the UI offers an edit whose save comes back + a 403. + """ files = [] memory_dir = workspace / "memory" if memory_dir.exists(): @@ -36,6 +47,7 @@ def _list_memory_files_sync(workspace: Path) -> list[dict]: "name": f.name, "size": st.st_size, "modified": datetime.fromtimestamp(st.st_mtime, timezone.utc).isoformat(), + "read_only": tracked_config_write_refusal(f) is not None, }) # Also include root-level md files @@ -46,6 +58,7 @@ def _list_memory_files_sync(workspace: Path) -> list[dict]: "name": f.name, "size": st.st_size, "modified": datetime.fromtimestamp(st.st_mtime, timezone.utc).isoformat(), + "read_only": tracked_config_write_refusal(f) is not None, }) return files @@ -71,7 +84,13 @@ async def read_memory_file(file_path: str, user: dict = Depends(require_auth)): if not full_path.exists() or not full_path.is_file(): raise HTTPException(status_code=404, detail="File not found") content = await asyncio.to_thread(full_path.read_text, encoding="utf-8") - return {"path": file_path, "content": content} + # An editor that opened this file needs the same answer the listing gave, or + # it re-offers the edit the PUT below refuses. + return { + "path": file_path, + "content": content, + "read_only": tracked_config_write_refusal(full_path) is not None, + } class FileWriteRequest(BaseModel): @@ -86,6 +105,10 @@ async def write_memory_file(file_path: str, req: FileWriteRequest, user: dict = full_path.resolve().relative_to(config.workspace.resolve()) except ValueError: raise HTTPException(status_code=403, detail="Access denied") + # This endpoint writes anywhere under the workspace, which includes the + # tracked config subtree — settings.yaml and the gate plugins the daemon + # executes. A locked instance must not be editable through it. + ensure_path_not_tracked_config(full_path, "write") def _write() -> None: full_path.parent.mkdir(parents=True, exist_ok=True) diff --git a/nerve/gateway/routes/tasks.py b/nerve/gateway/routes/tasks.py index 396faa21..128febdc 100644 --- a/nerve/gateway/routes/tasks.py +++ b/nerve/gateway/routes/tasks.py @@ -7,7 +7,7 @@ from fastapi import APIRouter, Depends, HTTPException from pydantic import BaseModel -from nerve.config import get_config +from nerve.config import ensure_path_not_tracked_config, get_config from nerve.db.task_statuses import STATUS_NAME_RE, normalize_color from nerve.gateway.auth import require_auth from nerve.gateway.routes._deps import ( @@ -129,6 +129,11 @@ async def update_task(task_id: str, req: TaskUpdateRequest, user: dict = Depends if req.content: config = get_config() file_path = config.workspace / task["file_path"] + # file_path comes from the DB row, which the task indexer only ever fills + # from a glob of tasks/ — but it is still a stored path being joined to + # the workspace and written to, so it gets the same guard as every other + # caller-influenced write. + ensure_path_not_tracked_config(file_path, "write") if file_path.exists(): await asyncio.to_thread( file_path.write_text, req.content, encoding="utf-8", diff --git a/nerve/gateway/server.py b/nerve/gateway/server.py index 717ded36..9564d374 100644 --- a/nerve/gateway/server.py +++ b/nerve/gateway/server.py @@ -690,6 +690,23 @@ def create_app() -> FastAPI: lifespan=lifespan, ) + # Render a lockdown violation as a clean 403 instead of an opaque 500. + from fastapi.responses import JSONResponse + + from nerve.config import LockdownError + + @app.exception_handler(LockdownError) + async def _lockdown_handler(request, exc: LockdownError): # noqa: ANN001 + return JSONResponse(status_code=403, content={"detail": str(exc)}) + + # A malformed skill id is the caller's mistake, not a server fault, and the + # id arrives as a path segment so it is trivially reachable. + from nerve.skills.manager import SkillIdError + + @app.exception_handler(SkillIdError) + async def _skill_id_handler(request, exc: SkillIdError): # noqa: ANN001 + return JSONResponse(status_code=400, content={"detail": str(exc)}) + # CORS for development app.add_middleware( CORSMiddleware, diff --git a/nerve/memory/manager.py b/nerve/memory/manager.py index b94c5f37..d24d4c9a 100644 --- a/nerve/memory/manager.py +++ b/nerve/memory/manager.py @@ -116,13 +116,21 @@ def read_file(self, relative_path: str) -> str | None: return None def write_file(self, relative_path: str, content: str) -> bool: - """Write a file in the workspace by relative path.""" + """Write a file in the workspace by relative path. + + Raises :class:`~nerve.config.LockdownError` for a path inside the tracked + config subtree on a locked instance — the caller supplies the path, so + whether this is a config edit is a property of the argument. + """ + from nerve.config import ensure_path_not_tracked_config + full_path = self.workspace / relative_path try: full_path.resolve().relative_to(self.workspace.resolve()) except ValueError: logger.warning("Attempted path traversal: %s", relative_path) return False + ensure_path_not_tracked_config(full_path, "write") full_path.parent.mkdir(parents=True, exist_ok=True) full_path.write_text(content, encoding="utf-8") diff --git a/nerve/skills/manager.py b/nerve/skills/manager.py index e73e257c..9225abe1 100644 --- a/nerve/skills/manager.py +++ b/nerve/skills/manager.py @@ -26,6 +26,7 @@ import yaml +from nerve.config import ensure_not_locked from nerve.db import Database logger = logging.getLogger(__name__) @@ -65,6 +66,41 @@ def _slugify(name: str) -> str: return slug[:60] or "unnamed-skill" +# One path component and nothing else: no separator, on either platform, and no +# drive or UNC prefix. Deliberately wider than what _slugify emits, because the +# filesystem is the source of truth here and discover() adopts whatever directory +# names it finds — a pre-existing `My_Skill` must stay editable. +_SKILL_ID_RE = re.compile(r"[A-Za-z0-9][A-Za-z0-9._+-]{0,127}\Z") + + +class SkillIdError(ValueError): + """Raised for a skill id that could not name a directory under ``skills/``.""" + + +def _skill_id(skill_id: str) -> str: + """Validate a caller-supplied skill id before it is joined to a path. + + ``_slugify`` runs on *create* only; every other entry point takes the id + straight from an HTTP path segment or a tool argument and joins it to + ``skills_dir``. That made ``..`` a working path component — and since delete + removes the whole directory tree it names, ``DELETE /api/skills/../config`` + took out the workspace's tracked config subtree. + + Checked as a *shape* rather than by containing the result afterwards: a skill + id names one directory directly under ``skills/``, so anything that is not a + single path component is not a skill id, whatever it would have resolved to. + A leading dot is refused too, which keeps ``.`` and ``..`` out without + special-casing them. + """ + if not isinstance(skill_id, str) or not _SKILL_ID_RE.match(skill_id): + raise SkillIdError( + f"invalid skill id {skill_id!r}: a skill id names a single directory " + f"under skills/ — letters, digits, '.', '_', '+' and '-', not " + f"starting with a dot" + ) + return skill_id + + def _parse_skill_md(raw: str) -> tuple[dict, str]: """Parse SKILL.md into (frontmatter_dict, body_content). @@ -219,7 +255,7 @@ def _scan_skill_dirs() -> list[tuple[Path, str]]: async def get_skill(self, skill_id: str) -> SkillContent | None: """Load full SKILL.md content + metadata for a skill.""" - skill_dir = self.skills_dir / skill_id + skill_dir = self.skills_dir / _skill_id(skill_id) skill_md = skill_dir / "SKILL.md" if not skill_md.exists(): return None @@ -266,8 +302,9 @@ async def create_skill( version: str = "1.0.0", ) -> SkillMeta: """Create a new skill directory + SKILL.md and index in DB.""" + ensure_not_locked("create a skill") skill_id = _slugify(name) - skill_dir = self.skills_dir / skill_id + skill_dir = self.skills_dir / _skill_id(skill_id) # Build SKILL.md raw = _build_skill_md(name, description, content, version) @@ -293,7 +330,8 @@ def _write_skill() -> None: async def update_skill(self, skill_id: str, content: str) -> SkillMeta | None: """Update SKILL.md content (full raw file), re-parse frontmatter, sync DB.""" - skill_dir = self.skills_dir / skill_id + ensure_not_locked("update a skill") + skill_dir = self.skills_dir / _skill_id(skill_id) skill_md = skill_dir / "SKILL.md" if not skill_md.exists(): return None @@ -325,7 +363,8 @@ async def update_skill(self, skill_id: str, content: str) -> SkillMeta | None: async def delete_skill(self, skill_id: str) -> bool: """Remove skill directory and DB record.""" - skill_dir = self.skills_dir / skill_id + ensure_not_locked("delete a skill") + skill_dir = self.skills_dir / _skill_id(skill_id) if skill_dir.exists(): await asyncio.to_thread(shutil.rmtree, skill_dir) await self.db.delete_skill_row(skill_id) @@ -334,6 +373,7 @@ async def delete_skill(self, skill_id: str) -> bool: async def toggle_skill(self, skill_id: str, enabled: bool) -> bool: """Enable or disable a skill.""" + ensure_not_locked("toggle a skill") existing = await self.db.get_skill_row(skill_id) if not existing: return False @@ -344,7 +384,7 @@ async def toggle_skill(self, skill_id: str, enabled: bool) -> bool: async def list_references(self, skill_id: str) -> list[str]: """List reference files in a skill's references/ directory.""" - refs_dir = self.skills_dir / skill_id / "references" + refs_dir = self.skills_dir / _skill_id(skill_id) / "references" if not refs_dir.is_dir(): return [] @@ -359,7 +399,7 @@ def _walk() -> list[str]: async def read_reference(self, skill_id: str, rel_path: str) -> str | None: """Read a reference file from a skill.""" - ref_file = self.skills_dir / skill_id / "references" / rel_path + ref_file = self.skills_dir / _skill_id(skill_id) / "references" / rel_path # Prevent path traversal try: ref_file.resolve().relative_to((self.skills_dir / skill_id).resolve()) @@ -371,7 +411,7 @@ async def read_reference(self, skill_id: str, rel_path: str) -> str | None: async def run_script(self, skill_id: str, rel_path: str, args: str = "") -> str: """Execute a script from a skill's scripts/ directory.""" - script_file = self.skills_dir / skill_id / "scripts" / rel_path + script_file = self.skills_dir / _skill_id(skill_id) / "scripts" / rel_path # Prevent path traversal try: script_file.resolve().relative_to((self.skills_dir / skill_id).resolve()) @@ -399,7 +439,7 @@ async def run_script(self, skill_id: str, rel_path: str, args: str = "") -> str: capture_output=True, text=True, timeout=30, - cwd=str(self.skills_dir / skill_id), + cwd=str(self.skills_dir / _skill_id(skill_id)), ) output = result.stdout if result.returncode != 0: diff --git a/nerve/sync_service.py b/nerve/sync_service.py index de4c54ec..9880aae4 100644 --- a/nerve/sync_service.py +++ b/nerve/sync_service.py @@ -111,17 +111,22 @@ def _rev(ref: str, cwd: Path) -> str: return r.stdout.strip() if r.returncode == 0 else "" -def _config_pathspec(workspace: Path) -> str: - """The git pathspec for the portable config subtree of ``workspace``.""" - from nerve.config import workspace_config_dir +def _reviewed_pathspec(workspace: Path) -> list[str]: + """The git pathspecs for the reviewed surface of ``workspace``. - return str(workspace_config_dir(workspace)) + The same list the write guard refuses, so the two cannot disagree about what + a reviewed file is. A path this misses is one sync merges over without ever + reporting that the live copy differs from the one that passed validation. + """ + from nerve.config import workspace_reviewed_paths + + return [str(p) for p in workspace_reviewed_paths(workspace)] def _local_config_divergence( - workspace: Path, rev: str, + workspace: Path, rev: str, locked: bool = False, ) -> tuple[list[str], list[str]]: - """Local state in the config subtree that the validated rev does not contain. + """Local state in the reviewed surface that the validated rev does not contain. Validation runs against a clean detached checkout of ``rev``; the merge lands in the live working tree. ``git merge --ff-only`` only refuses when the @@ -140,20 +145,30 @@ def _local_config_divergence( about what ends up on disk, and it cannot make that promise about a tree somebody else is also editing. - Scoped to the config subtree on purpose. A nerve workspace is also the + Scoped to the reviewed surface on purpose. A nerve workspace is also the agent's working directory, so uncommitted notes and scratch files elsewhere in it are normal and none of sync's business — and where they *would* break the fast-forward, git says so itself. + ``locked`` promotes ``.gitignore``d files inside the surface from a warning to + a refusal. On an ordinary box those files are the operator's own, kept + deliberately out of the shared repo, and refusing a merge over them would be + sync passing judgement on what a machine may hold locally. A locked instance + has already made that judgement: its stated contract is that only reviewed, + merged remote config runs there, and an ignored ``cron/gates/*.py`` is local + unreviewed code the daemon executes, invisible to both the reviewer and the + validator. Merging on top of it would report success over a bundle that is + not the one that passed. + Returns ``(blocking, warnings)``. """ blocking: list[str] = [] warnings: list[str] = [] - pathspec = _config_pathspec(workspace) + pathspec = _reviewed_pathspec(workspace) status = _git([ *_QUOTEPATH_OFF, "status", "--porcelain", - "--untracked-files=all", "--ignored=matching", "--", pathspec, + "--untracked-files=all", "--ignored=matching", "--", *pathspec, ], workspace) if status.returncode != 0: # Fail closed: unable to establish that the tree is clean is not the @@ -168,30 +183,43 @@ def _local_config_divergence( continue code, path = line[:2], line[3:] if code == "!!": - # An ignored file inside a *tracked config subtree* is a layout - # mistake — it is config the shared repo can never carry and no - # reviewer will ever see. Worth saying; not worth refusing a merge - # over, since refusing would be a policy decision about what the - # box is allowed to have locally rather than a statement about - # whether the merge is sound. - warnings.append(f"ignored file inside the tracked config subtree: {path}") + # An ignored file inside the *reviewed surface* is a layout mistake — + # it is config the shared repo can never carry and no reviewer will + # ever see. Worth saying; on an unlocked box not worth refusing a + # merge over, since refusing would be a policy decision about what + # the machine is allowed to have locally rather than a statement + # about whether the merge is sound. Under lockdown that policy + # decision has already been made. + if locked: + blocking.append(f"!! {path} (gitignored)") + else: + warnings.append( + f"ignored file inside the reviewed surface: {path}" + ) else: blocking.append(f"{code.strip() or '??'} {path}") - # A submodule under the config subtree is a third case: `git worktree add` + # A submodule inside the reviewed surface is a third case: `git worktree add` # does not initialize submodules, so validation saw an empty directory, and # the fast-forward leaves the live checkout on its old commit. Neither the # old contents nor the new ones were checked. - tree = _git([*_QUOTEPATH_OFF, "ls-tree", "-r", rev, "--", pathspec], workspace) + # + # ``locked`` promotes it for the same reason as an ignored file: unvalidated + # content the daemon reads anyway is exactly what a locked box says cannot be + # there, and the submodule case is the worse of the two, since the working + # copy is whatever the box happens to have checked out and no remote change + # ever moves it. + tree = _git([*_QUOTEPATH_OFF, "ls-tree", "-r", rev, "--", *pathspec], workspace) if tree.returncode == 0: for line in tree.stdout.splitlines(): if line.startswith("160000 "): sub = line.split("\t", 1)[-1] - warnings.append( - f"submodule {sub!r} in the config subtree was not validated " + note = ( + f"submodule {sub!r} in the reviewed surface was not validated " f"(a validation checkout does not initialize submodules) and " f"a fast-forward does not update it" ) + (blocking if locked else warnings).append(note) return blocking, warnings @@ -292,6 +320,7 @@ def sync_workspace( branch: str = "", validate: bool = True, strict_env: bool = True, + locked: bool = False, ) -> SyncResult: """Fetch the workspace remote, validate the fetched bundle, then ff-merge it. @@ -301,8 +330,8 @@ def sync_workspace( :class:`SyncResult`. That guarantee is about the tree the daemon will read, not merely about the - commit that was checked, so a merge is also refused when the live config - subtree has local changes of its own — see + commit that was checked, so a merge is also refused when the live reviewed + files have local changes of their own — see :func:`_local_config_divergence`. ``changed`` reports whether the live tree actually moved; a refusal leaves it exactly where it was. @@ -311,6 +340,9 @@ def sync_workspace( refuses to load on its next restart. Turn it off only when running somewhere that legitimately lacks the daemon's environment, e.g. an operator's shell. + ``locked`` tightens the local-changes check for an instance that has promised + to run only reviewed remote config — see :func:`_local_config_divergence`. + "Never raises" is the whole contract, not an aspiration: the HTTP route turns anything that escapes into a 500 and the daemon's loop would report a stack trace instead of a config problem. The guard here is deliberately @@ -325,6 +357,7 @@ def sync_workspace( with _sync_lock: return _sync_workspace( Path(workspace), Path(config_dir), branch, validate, strict_env, + locked, ) except Exception as e: # noqa: BLE001 — see the contract above logger.warning("Workspace sync failed unexpectedly: %s", e, exc_info=True) @@ -337,6 +370,7 @@ def _sync_workspace( branch: str, validate: bool, strict_env: bool, + locked: bool = False, ) -> SyncResult: """The fetch → validate → merge sequence. See :func:`sync_workspace`.""" if not is_git_repo(workspace): @@ -362,13 +396,13 @@ def _sync_workspace( if new == old: return SyncResult(ok=True, changed=False, old_rev=old, new_rev=new, message="up to date") - blocking, warnings = _local_config_divergence(workspace, new) + blocking, warnings = _local_config_divergence(workspace, new, locked) if blocking: return SyncResult( ok=False, changed=False, old_rev=old, new_rev=new, validation_warnings=warnings, message=( - f"fetched {new[:8]} but the workspace config subtree has local " + f"fetched {new[:8]} but the workspace's reviewed files have local " f"changes — not applying, because they would survive the " f"fast-forward without ever having been validated: " f"{_describe_paths(blocking)}. Commit, discard or push them." @@ -384,6 +418,19 @@ def _sync_workspace( # gate type nothing recognizes is how a cron job quietly starts running # unconditionally, and the only place that is visible is here. warnings += report.warnings + if report.unresolved_env and not strict_env: + # With strict_env on this is already an error and the merge is + # refused. With it off the validator files it as *info*, which + # nothing here propagates — so the gate would see the one thing that + # predicts a failed post-merge reload and drop it on the floor. The + # merge still goes ahead: switching strict_env off is exactly a + # request to tolerate this. Saying so is not. + warnings.append( + f"the fetched bundle references unset environment variable(s): " + f"{', '.join(report.unresolved_env)} — workspace_sync.strict_env " + f"is off so the merge proceeds, but this daemon will refuse to " + f"load the merged config" + ) if report.errors: return SyncResult( ok=False, changed=False, old_rev=old, new_rev=new, @@ -430,14 +477,13 @@ async def run_periodic_sync(config, engine, cron_service, stop_event: asyncio.Ev Every cycle re-reads the process-wide config object rather than working from a reference captured before the loop, so the loop can never be the reason a - setting is stuck. **On its own that changes nothing today:** nothing refreshes - that object after start-up — it is loaded once and only replaced by the CLI - at process start — so editing ``workspace_sync`` still requires a restart to - take effect. Once something does refresh it, ``branch``, ``validate``, + setting is stuck. What refreshes that object is :func:`_apply_sync`, i.e. a + sync that actually merged something — after which ``branch``, ``validate``, ``strict_env``, ``interval_minutes``, the workspace location and turning sync - off will apply from the following cycle with no further work here. Turning - sync *on* will still need a restart regardless: this task is created at - start-up only when sync is already enabled, and nothing creates it later. + off all apply from the following cycle. Nothing *else* refreshes it, so a + hand-edited ``config.yaml`` on the box still needs a restart. Turning sync + *on* needs one regardless: this task is created at start-up only when sync is + already enabled, and nothing creates it later. """ from nerve.config import get_config @@ -471,6 +517,7 @@ async def run_periodic_sync(config, engine, cron_service, stop_event: asyncio.Ev sync_workspace, config.workspace, config.config_dir or config.workspace, branch=cfg.branch, validate=cfg.validate, strict_env=cfg.strict_env, + locked=config.lockdown, ) for warning in result.validation_warnings: logger.warning("Workspace sync: config warning: %s", warning) @@ -481,21 +528,65 @@ async def run_periodic_sync(config, engine, cron_service, stop_event: asyncio.Ev continue if result.changed: logger.info("Workspace sync: %s — applying", result.message) - await _apply_sync(engine, cron_service) + apply_error = await _apply_sync( + engine, cron_service, config.config_dir or config.workspace, + ) + if apply_error: + # The merge landed but the daemon is still running the old + # config. Logged at WARNING beside the INFO line above, which + # otherwise reads as "applied". + logger.warning( + "Workspace sync: merged %s but the new config could not " + "be loaded, so NOTHING was applied and this daemon is " + "still running the previous configuration: %s", + result.new_rev[:8], apply_error, + ) except Exception as e: # noqa: BLE001 — never let the loop die logger.warning("Workspace sync cycle failed: %s", e) continue -async def _apply_sync(engine, cron_service) -> None: - """Hot-reload the subsystems affected by a workspace pull.""" +async def _apply_sync(engine, cron_service, config_dir) -> str | None: + """Hot-reload the subsystems affected by a workspace pull. + + Re-reads the typed config and re-points the singleton, so a synced settings + change engages here without a restart for everything that reads the singleton + per use: the lockdown write guards (``is_locked``), gateway authentication, + and — because it re-reads the singleton every cycle — the sync loop itself. + ``cron_service.config`` and ``engine.config`` are re-pointed too. What is + *not*: cron jobs and gates already built, which follow on the next + ``cron_service.reload()``, and anything captured in a closure or a dataclass + at start-up, which follows on a restart. This is also the only path that + refreshes any of it — a hand edit to a config file on the box is not picked + up, by design for a locked instance and by omission otherwise. + + Returns the config-reload error, if any. A merge that lands config the daemon + cannot load has applied nothing, however well the merge itself went, and the + caller must not report success for it: the case that makes this concrete is a + pull that turns ``lockdown`` on, where reporting success tells the operator a + box is locked while its write guards are still open. + """ + from nerve.config import load_config, set_config + + config_error: str | None = None + try: + new_config = load_config(config_dir) + set_config(new_config) + if cron_service is not None: + cron_service.config = new_config + if engine is not None and hasattr(engine, "config"): + engine.config = new_config + except Exception as e: # noqa: BLE001 — a bad reload must not kill the loop + config_error = f"{type(e).__name__}: {e}" + logger.warning("config reload after sync failed: %s", e, exc_info=True) if cron_service is not None: try: await cron_service.reload() except Exception as e: # noqa: BLE001 - logger.warning("cron reload after sync failed: %s", e) + logger.warning("cron reload after sync failed: %s", e, exc_info=True) if engine is not None: try: await engine.reload_mcp_config() except Exception as e: # noqa: BLE001 - logger.warning("MCP reload after sync failed: %s", e) + logger.warning("MCP reload after sync failed: %s", e, exc_info=True) + return config_error diff --git a/nerve/tasks/manager.py b/nerve/tasks/manager.py index 7c00e721..a9a41a10 100644 --- a/nerve/tasks/manager.py +++ b/nerve/tasks/manager.py @@ -7,6 +7,7 @@ from datetime import datetime, timezone from pathlib import Path +from nerve.config import ensure_path_not_tracked_config from nerve.db import Database from nerve.tasks.models import Task, TaskStatus, parse_task_frontmatter, parse_task_title @@ -76,12 +77,21 @@ async def get_task(self, task_id: str) -> Task | None: return task async def mark_done(self, task_id: str) -> bool: - """Mark a task as done and move its file.""" + """Mark a task as done and move its file. + + Raises :class:`~nerve.config.LockdownError` on a locked instance when the + stored ``file_path`` lands inside the tracked config subtree. + """ row = await self.db.get_task(task_id) if not row: return False src = self.workspace / row["file_path"] + # Done is a write like any other — it copies the file into done/ and + # unlinks the source, so a stored ``file_path`` inside the tracked config + # subtree would delete config. Refuse before any of it runs, or the + # refusal still leaves that config file mirrored into done/. + ensure_path_not_tracked_config(src, "move") if src.exists(): dst = self.done_dir / src.name content = await asyncio.to_thread(src.read_text, encoding="utf-8") diff --git a/nerve/templates/config/settings.yaml b/nerve/templates/config/settings.yaml index 6aa498ef..98190bbf 100644 --- a/nerve/templates/config/settings.yaml +++ b/nerve/templates/config/settings.yaml @@ -21,6 +21,25 @@ # # Everything below is commented out; uncomment and edit what you want to share. +# Remote-only, read-only mode. When true, config comes ONLY from this workspace +# + ${ENV_VAR}; machine config.yaml/config.local.yaml overrides are ignored and +# runtime edits are blocked. Secrets (incl. auth.jwt_secret) have to be supplied +# via ${ENV_VAR} here or the environment — config.local.yaml is not read when +# locked, so a secret that lives only there stops being read. +# May be an env reference (lockdown: ${NERVE_LOCKDOWN}) so one repo can serve a +# fleet where only some boxes are locked; a value that is neither true nor false — +# an unset or blank variable included — is refused rather than read as unlocked, +# so spell a default-unlocked box ${NERVE_LOCKDOWN:-false}. +# NERVE_LOCKDOWN can also be set in the environment directly: put it (with +# NERVE_WORKSPACE beside it) in the service definition and the box is locked +# whatever any file says, which is what stops a machine-local edit from repointing +# `workspace:` at a tree that isn't locked. Setting it here alone does not survive +# that. +# Anything `nerve init` writes to config.yaml has to be restated here to survive +# locking: notably telegram.enabled, which a locked instance treats as OFF unless +# this file says otherwise. +# lockdown: true + # timezone: America/New_York # Where the gateway listens. A box needing a different port overrides in its own diff --git a/tests/test_backup.py b/tests/test_backup.py index 74a779f2..87c77bf1 100644 --- a/tests/test_backup.py +++ b/tests/test_backup.py @@ -267,6 +267,44 @@ def test_backup_excludes_junk(nerve_dir, workspace, config_dir, tmp_path): assert any(m.endswith("workspace/scripts/helper.py") for m in members) +def test_config_travels_in_the_bundle_but_is_not_written_back( + nerve_dir, workspace, config_dir, tmp_path, +): + """The two halves of the config asymmetry, on an ordinary bundle. + + The other restore test smuggles entries in to prove a hostile bundle is + refused. This one uses a bundle this module wrote itself, because the + surprising half is that our *own* config/ is not written back either — it is + captured so nothing is lost, and withheld because tracked config comes from + the git remote, not a tarball. + """ + cfg = workspace / "config" + (cfg / "cron").mkdir(parents=True) + (cfg / "settings.yaml").write_text("timezone: UTC\n") + (cfg / "cron" / "jobs.yaml").write_text("jobs:\n - id: nightly\n") + + out = tmp_path / "out" + result = backup_mod.create_backup(nerve_dir, workspace, out, config_dir=config_dir) + + # Captured. + members = _bundle_members(result.path) + assert any(m.endswith("workspace/config/settings.yaml") for m in members) + assert any(m.endswith("workspace/config/cron/jobs.yaml") for m in members) + + # Not written back — and what is already on disk survives untouched. + ws_out = tmp_path / "ws_restored" + (ws_out / "config").mkdir(parents=True) + (ws_out / "config" / "settings.yaml").write_text("timezone: Europe/Berlin\n") + report = backup_mod.restore_bundle(result.path, tmp_path / "target", ws_out) + + assert (ws_out / "config" / "settings.yaml").read_text() == "timezone: Europe/Berlin\n" + assert not (ws_out / "config" / "cron").exists() + # The rest of the brain did come back. + assert (ws_out / "SOUL.md").read_text() == "soul" + assert (ws_out / "skills" / "s1" / "SKILL.md").exists() + assert any("skipped" in w for w in report.warnings), report.warnings + + def test_backup_captures_tracked_workspace_config( nerve_dir, workspace, config_dir, tmp_path, ): @@ -421,6 +459,54 @@ def test_restore_force_relocates_old_dir(nerve_dir, workspace, config_dir, tmp_p assert "old_marker.txt" not in [p.name for p in target.iterdir()] +def test_restore_refuses_workspace_paths_the_backup_would_never_take( + nerve_dir, workspace, config_dir, tmp_path, +): + """The workspace allowlist is applied when a bundle is *collected*; restore + has to apply it again when a bundle is *read*. + + "config/ is never in a bundle" is true of bundles this module writes and says + nothing about bundles it is handed. One carrying config/settings.yaml or a gate + plugin would overwrite the git-tracked config subtree from a tarball — which on + a locked instance replaces the reviewed remote config outright, and on any + instance leaves a checkout dirty enough that the next sync refuses to merge. + """ + out = tmp_path / "out" + result = backup_mod.create_backup(nerve_dir, workspace, out, config_dir=config_dir) + compression = backup_mod._compression_for(result.path) + + # Re-pack the bundle with two extra workspace entries the collector would + # never have produced, using the module's own reader/writer so it stays a + # bundle this code accepts. + staging = tmp_path / "repack" + staging.mkdir() + with backup_mod._tar_reader(result.path, compression) as tf: + tf.extractall(staging) + smuggled = staging / "workspace" / "config" / "cron" / "gates" + smuggled.mkdir(parents=True) + (smuggled / "evil.py").write_text("MARKER = 1\n") + (staging / "workspace" / "config" / "settings.yaml").write_text("lockdown: false\n") + result.path.unlink() + with backup_mod._tar_writer(result.path, compression) as tf: + for entry in sorted(staging.iterdir()): + tf.add(entry, arcname=entry.name) + + target = tmp_path / "restored" + ws_out = tmp_path / "ws_restored" + ws_out.mkdir() + (ws_out / "config").mkdir() + (ws_out / "config" / "settings.yaml").write_text("lockdown: true\n") + + report = backup_mod.restore_bundle(result.path, target, ws_out) + + # The tracked subtree is untouched; the allowlisted brain still came back. + assert (ws_out / "config" / "settings.yaml").read_text() == "lockdown: true\n" + assert not (ws_out / "config" / "cron").exists() + assert (ws_out / "SOUL.md").read_text() == "soul" + assert (ws_out / "memory" / "people.md").exists() + assert any("skipped" in w for w in report.warnings), report.warnings + + # --------------------------------------------------------------------------- # # 5. Schema-version guard # # --------------------------------------------------------------------------- # diff --git a/tests/test_lockdown.py b/tests/test_lockdown.py new file mode 100644 index 00000000..fed09ec8 --- /dev/null +++ b/tests/test_lockdown.py @@ -0,0 +1,1981 @@ +"""Tests for lockdown / remote-only read-only mode.""" + +import os +import shutil +import subprocess +from pathlib import Path + +import pytest + +import nerve.config as cfg +from nerve.config import ( + ConfigError, + LockdownError, + NerveConfig, + _resolve_cron_dir, + ensure_not_locked, + is_locked, + load_config, + lockdown_workspace_problems, + tracked_config_write_refusal, + workspace_settings_file, +) + + +def _repo(ws: Path, *, remote: str | None = "origin") -> Path: + """Make ``ws`` what a locked workspace is on a real box: a git repository + with a remote to receive reviewed config from. + + A locked instance with nowhere to receive it from refuses to start — see + :class:`TestLockdownNeedsSomewhereToReceiveConfigFrom` — so every locked + workspace here that is expected to load has one. The remote is never + contacted; the check asks whether one is configured. ``remote=None`` leaves + the repository without one. + """ + if not shutil.which("git"): + pytest.skip("git not available") + runs = [["init", "-q"]] + if remote: + runs.append(["remote", "add", remote, "https://example.invalid/config.git"]) + for args in runs: + subprocess.run(["git", *args], cwd=str(ws), check=True, capture_output=True) + return ws + + +def _install(tmp_path, *, settings="", base="", local=""): + config_dir = tmp_path / "cfg" + workspace = tmp_path / "ws" + config_dir.mkdir(parents=True) + (workspace / "config").mkdir(parents=True) + _repo(workspace) + (config_dir / "config.yaml").write_text(f"workspace: {workspace}\n" + base, encoding="utf-8") + if local: + (config_dir / "config.local.yaml").write_text(local, encoding="utf-8") + if settings: + workspace_settings_file(workspace).write_text(settings, encoding="utf-8") + return config_dir, workspace + + +_JWT = "auth:\n jwt_secret: test-secret\n" + + +class TestLockdownResolution: + _JWT = "auth:\n jwt_secret: test-secret\n" + + def test_locked_drops_machine_overrides(self, tmp_path): + # settings says UTC + locked; config.yaml tries to override timezone. + config_dir, ws = _install( + tmp_path, + settings="lockdown: true\ntimezone: UTC\n" + self._JWT, + base="timezone: America/New_York\n", + local="timezone: Europe/Berlin\n", + ) + c = load_config(config_dir) + assert c.lockdown is True + assert c.timezone == "UTC" # machine layers ignored; only settings applies + + def test_local_cannot_override_lockdown(self, tmp_path): + # Tamper attempt: config.yaml/local set lockdown:false, settings sets true. + config_dir, ws = _install( + tmp_path, + settings="lockdown: true\n" + self._JWT, + base="lockdown: false\n", + local="lockdown: false\n", + ) + c = load_config(config_dir) + assert c.lockdown is True # the remote (settings.yaml) is authoritative + + def test_lockdown_not_settable_from_config_yaml(self, tmp_path): + # Only the tracked settings file controls lockdown. + config_dir, ws = _install(tmp_path, base="lockdown: true\n") + c = load_config(config_dir) + assert c.lockdown is False + + def test_not_locked_merges_normally(self, tmp_path): + config_dir, ws = _install( + tmp_path, settings="timezone: UTC\n", base="timezone: America/New_York\n" + ) + c = load_config(config_dir) + assert c.lockdown is False + assert c.timezone == "America/New_York" # config.yaml wins when unlocked + + def test_locked_still_resolves_env(self, tmp_path, monkeypatch): + monkeypatch.setenv("SECRET_TZ", "Asia/Tokyo") + config_dir, ws = _install( + tmp_path, settings="lockdown: true\ntimezone: ${SECRET_TZ}\n" + self._JWT, + ) + c = load_config(config_dir) + assert c.timezone == "Asia/Tokyo" # secrets still come from env + + +class TestLockdownAuthFailClosed: + @pytest.mark.asyncio + async def test_require_auth_denies_when_locked_no_secret(self, monkeypatch): + from fastapi import HTTPException + + from nerve.gateway.auth import require_auth + + c = NerveConfig(lockdown=True) # jwt_secret empty + monkeypatch.setattr(cfg, "_config", c) + with pytest.raises(HTTPException) as ei: + await require_auth(request=None) + assert ei.value.status_code == 503 + + @pytest.mark.asyncio + async def test_websocket_denies_when_locked_no_secret(self, monkeypatch): + from nerve.gateway.auth import authenticate_websocket + + monkeypatch.setattr(cfg, "_config", NerveConfig(lockdown=True)) + assert await authenticate_websocket(websocket=None) is False + + +class TestValidateRespectsLockdown: + def test_validate_uses_locked_view(self, tmp_path): + from nerve.config_validate import validate_config_bundle + + # settings locks + provides jwt; config.yaml has an unknown key that must + # be ignored under lockdown (machine layers dropped). + config_dir = tmp_path / "cfg" + ws = tmp_path / "ws" + config_dir.mkdir(parents=True) + (ws / "config").mkdir(parents=True) + (config_dir / "config.yaml").write_text( + f"workspace: {ws}\ntiimezone: UTC\n", encoding="utf-8" + ) + workspace_settings_file(ws).write_text( + "lockdown: true\nauth:\n jwt_secret: x\n", encoding="utf-8" + ) + result = validate_config_bundle(config_dir, workspace_override=ws, strict_keys=True) + # config.yaml's typo is dropped under lockdown → not reported. + assert not any("tiimezone" in e for e in result.errors) + + +class TestLockdownCron: + def test_locked_forces_workspace_cron_no_legacy(self, tmp_path): + from nerve import paths + + workspace = tmp_path / "ws" + legacy = paths.cron_dir() + legacy.mkdir(parents=True, exist_ok=True) + (legacy / "jobs.yaml").write_text("jobs: []\n", encoding="utf-8") + # Unlocked would fall back to legacy (workspace has no jobs); locked won't. + assert _resolve_cron_dir(workspace, locked=False) == legacy + assert _resolve_cron_dir(workspace, locked=True) == workspace / "config" / "cron" + + +class TestLockdownGuards: + def test_is_locked_reflects_config(self, monkeypatch): + monkeypatch.setattr(cfg, "_config", NerveConfig(lockdown=True)) + assert is_locked() is True + monkeypatch.setattr(cfg, "_config", NerveConfig(lockdown=False)) + assert is_locked() is False + + def test_ensure_not_locked_raises_when_locked(self, monkeypatch): + monkeypatch.setattr(cfg, "_config", NerveConfig(lockdown=True)) + with pytest.raises(LockdownError): + ensure_not_locked("do a thing") + + def test_ensure_not_locked_noop_when_unlocked(self, monkeypatch): + monkeypatch.setattr(cfg, "_config", NerveConfig(lockdown=False)) + ensure_not_locked("do a thing") # no raise + + def test_telegram_write_blocked_when_locked(self, tmp_path, monkeypatch): + from nerve.config import append_telegram_allowed_user + + monkeypatch.setattr(cfg, "_config", NerveConfig(lockdown=True)) + with pytest.raises(LockdownError): + append_telegram_allowed_user(tmp_path, 42) + + +class TestLockdownSkillWrites: + @pytest.mark.asyncio + async def test_skill_writes_blocked_when_locked(self, tmp_path, db, monkeypatch): + from nerve.skills.manager import SkillManager + + monkeypatch.setattr(cfg, "_config", NerveConfig(lockdown=True)) + mgr = SkillManager(tmp_path / "ws", db) + with pytest.raises(LockdownError): + await mgr.create_skill("Test", "desc") + with pytest.raises(LockdownError): + await mgr.update_skill("x", "content") + with pytest.raises(LockdownError): + await mgr.delete_skill("x") + with pytest.raises(LockdownError): + await mgr.toggle_skill("x", True) + + @pytest.mark.asyncio + async def test_skill_create_works_when_unlocked(self, tmp_path, db, monkeypatch): + from nerve.skills.manager import SkillManager + + monkeypatch.setattr(cfg, "_config", NerveConfig(lockdown=False)) + mgr = SkillManager(tmp_path / "ws", db) + meta = await mgr.create_skill("Test Skill", "a description") + assert meta.id == "test-skill" + + +class TestLockdownFlagFromEnvironment: + """The flag is read before the post-merge interpolation pass, so it has to + resolve its own ``${VAR}`` — otherwise it is judged as the literal reference + text, which is truthy no matter what the variable says. + + Both directions of getting that wrong are here. Reading ``True`` when the + config says ``false`` locks a fleet by accident. Reading ``False`` when the + config says ``true`` is an authentication and integrity bypass, so a value + that cannot be read is refused rather than resolved to the safer-looking + default. + """ + + def test_env_ref_false_leaves_the_instance_unlocked(self, tmp_path, monkeypatch): + monkeypatch.setenv("NERVE_LOCKDOWN", "false") + config_dir, ws = _install( + tmp_path, + settings="lockdown: ${NERVE_LOCKDOWN}\ntimezone: UTC\n", + base="timezone: America/New_York\n", + ) + c = load_config(config_dir) + assert c.lockdown is False + # Not just the flag: the machine-local layer is merged again, which is + # the behavior that was actually lost. + assert c.timezone == "America/New_York" + + def test_env_ref_true_locks(self, tmp_path, monkeypatch): + # NERVE_LOCKDOWN is also the environment anchor, so setting it truthy + # requires NERVE_WORKSPACE alongside — see TestLockdownEnvironmentAnchor. + monkeypatch.setenv("NERVE_LOCKDOWN", "true") + monkeypatch.setenv("NERVE_WORKSPACE", str(tmp_path / "ws")) + config_dir, ws = _install( + tmp_path, + settings="lockdown: ${NERVE_LOCKDOWN}\ntimezone: UTC\n" + _JWT, + base="timezone: America/New_York\n", + ) + c = load_config(config_dir) + assert c.lockdown is True + assert c.timezone == "UTC" + + def test_the_tracked_reference_and_the_anchor_are_one_switch(self, tmp_path, monkeypatch): + """``lockdown: ${NERVE_LOCKDOWN}`` and the anchor read the same variable + on purpose: "this box is locked" should have one spelling. With the anchor + the tracked file need not mention the flag at all, and a fleet repo that + does mention it gets the anchor's protection for free.""" + monkeypatch.setenv("NERVE_LOCKDOWN", "1") + monkeypatch.setenv("NERVE_WORKSPACE", str(tmp_path / "ws")) + config_dir, ws = _install(tmp_path, settings=_JWT) # no lockdown key + assert load_config(config_dir).lockdown is True + + def test_optional_ref_default_false_leaves_it_unlocked(self, tmp_path, monkeypatch): + monkeypatch.delenv("NERVE_LOCKDOWN", raising=False) + config_dir, ws = _install( + tmp_path, settings="lockdown: ${NERVE_LOCKDOWN:-false}\n", + ) + assert load_config(config_dir).lockdown is False + + def test_unparseable_value_is_refused_not_read_as_unlocked(self, tmp_path): + # jwt_secret is supplied so the only thing left to complain about is the + # flag itself — an unreadable value must not resolve to either position. + config_dir, ws = _install(tmp_path, settings="lockdown: yess\n" + _JWT) + with pytest.raises(ConfigError) as ei: + load_config(config_dir) + assert "lockdown must be true or false" in str(ei.value) + + def test_unset_required_ref_is_refused(self, tmp_path, monkeypatch): + monkeypatch.delenv("NERVE_LOCKDOWN", raising=False) + config_dir, ws = _install( + tmp_path, settings="lockdown: ${NERVE_LOCKDOWN}\n" + _JWT, + ) + with pytest.raises(ConfigError) as ei: + load_config(config_dir) + assert "NERVE_LOCKDOWN" in str(ei.value) + + def test_empty_env_var_is_refused(self, tmp_path, monkeypatch): + """``FLAG=`` switches an ordinary feature off; it must not switch this + one off, because a variable that failed to be populated would silently + drop every restriction the flag imposes.""" + monkeypatch.setenv("NERVE_LOCKDOWN", "") + config_dir, ws = _install( + tmp_path, settings="lockdown: ${NERVE_LOCKDOWN}\n" + _JWT, + ) + with pytest.raises(ConfigError) as ei: + load_config(config_dir) + assert "lockdown must be true or false" in str(ei.value) + + def test_bare_key_is_off(self, tmp_path): + config_dir, ws = _install(tmp_path, settings="lockdown:\ntimezone: UTC\n") + assert load_config(config_dir).lockdown is False + + def test_machine_layer_cannot_unlock_with_an_env_ref(self, tmp_path, monkeypatch): + """The tracked file stays the only authority once the flag is a reference: + a local layer that resolves to false must not reach it.""" + monkeypatch.setenv("NERVE_LOCKDOWN", "false") + config_dir, ws = _install( + tmp_path, + settings="lockdown: true\n" + _JWT, + base="lockdown: ${NERVE_LOCKDOWN}\n", + local="lockdown: false\n", + ) + assert load_config(config_dir).lockdown is True + + def test_from_dict_parses_a_string_flag(self): + """``NerveConfig.from_dict`` is called with an already-merged dict by the + validator and by tests, so it must agree with the loader.""" + assert NerveConfig.from_dict({"lockdown": "false"}).lockdown is False + assert NerveConfig.from_dict({"lockdown": "1"}).lockdown is True + with pytest.raises(ConfigError): + NerveConfig.from_dict({"lockdown": "sure"}) + + def test_validator_reports_an_unreadable_flag(self, tmp_path): + from nerve.config_validate import validate_config_bundle + + config_dir, ws = _install(tmp_path, settings="lockdown: perhaps\n" + _JWT) + result = validate_config_bundle(config_dir, workspace_override=ws) + assert any("lockdown must be true or false" in e for e in result.errors) + + def test_machine_layer_lockdown_is_warned_about(self, tmp_path, caplog): + """Ignoring it is the feature; ignoring it in silence leaves whoever put + it in the wrong file believing the box is locked.""" + import logging + + config_dir, ws = _install(tmp_path, base="lockdown: true\n") + with caplog.at_level(logging.WARNING, logger="nerve.config"): + assert load_config(config_dir).lockdown is False + assert any( + "lockdown" in r.message and "settings.yaml" in r.message + for r in caplog.records + ), [r.message for r in caplog.records] + + def test_unreadable_settings_file_is_a_config_error(self, tmp_path): + """``_read_yaml_mapping`` promises ConfigError rather than a traceback for + a file it cannot use; that covered a parse failure but not a read one.""" + config_dir, ws = _install(tmp_path, settings="lockdown: false\n") + settings = workspace_settings_file(ws) + settings.unlink() + settings.mkdir() # a directory where the file should be + with pytest.raises(ConfigError) as ei: + load_config(config_dir) + assert "Cannot read" in str(ei.value) + + +class TestValidatingTheLockedViewInCi: + """A fleet repo writes ``lockdown: ${NERVE_LOCKDOWN:-false}`` so one bundle can + serve locked and unlocked boxes. CI has no such variable, so it resolved false + and validated the view no locked box will ever run — leaving the lockdown + checks with nothing to fire on and the locked instance to find out at boot. + """ + + _FLEET = "lockdown: ${NERVE_LOCKDOWN:-false}\ntimezone: UTC\n" + + def test_env_controlled_flag_passes_by_default(self, tmp_path, monkeypatch): + from nerve.config_validate import validate_config_bundle + + monkeypatch.delenv("NERVE_LOCKDOWN", raising=False) + config_dir, ws = _install(tmp_path, settings=self._FLEET) + # No auth.jwt_secret anywhere: broken for a locked box, fine for this one. + result = validate_config_bundle( + config_dir, workspace_override=ws, strict_env=True, + ) + assert result.ok + # ...but the gap is at least named, which is how anyone learns to ask. + assert any("locked view was NOT validated" in w for w in result.warnings) + + def test_assume_lockdown_catches_it(self, tmp_path, monkeypatch): + """A locked-only error — here a config subtree symlinked out of the + workspace — is invisible to the default run and fatal under the flag.""" + from nerve.config_validate import validate_config_bundle + + monkeypatch.delenv("NERVE_LOCKDOWN", raising=False) + config_dir, ws = _install(tmp_path, settings=self._FLEET) + outside = tmp_path / "elsewhere" + outside.mkdir() + (outside / "settings.yaml").write_text(self._FLEET, encoding="utf-8") + (ws / "config").rename(tmp_path / "discarded") + (ws / "config").symlink_to(outside) + + assert validate_config_bundle(config_dir, workspace_override=ws).ok + result = validate_config_bundle( + config_dir, workspace_override=ws, assume_locked=True, + ) + assert not result.ok + assert any("outside the workspace" in e for e in result.errors), result.errors + + def test_assume_lockdown_says_it_is_assuming(self, tmp_path, monkeypatch): + from nerve.config_validate import validate_config_bundle + + monkeypatch.delenv("NERVE_LOCKDOWN", raising=False) + config_dir, ws = _install(tmp_path, settings=self._FLEET + _JWT) + result = validate_config_bundle( + config_dir, workspace_override=ws, assume_locked=True, + ) + assert result.ok + assert any("LOCKED view on request" in i for i in result.info) + + def test_a_literal_false_is_not_warned_about(self, tmp_path): + """Only an env-controlled flag leaves a locked view unchecked. A bundle + that says `lockdown: false` outright has no locked view to check.""" + from nerve.config_validate import validate_config_bundle + + config_dir, ws = _install(tmp_path, settings="lockdown: false\n") + result = validate_config_bundle(config_dir, workspace_override=ws) + assert not any("locked view" in w for w in result.warnings) + + +class TestLockdownCronContainment: + """Cron's three path keys decide which files a locked instance reads, and + ``gate_plugins_dir`` decides which ``.py`` files it *executes*. A tracked + ``settings.yaml`` is pure YAML that a reviewer may wave through, so pointing + those keys out of the reviewed tree would turn a config edit into arbitrary + on-disk code execution. + """ + + def _locked(self, tmp_path, cron_yaml, *, workspace=None): + config_dir, ws = _install( + tmp_path, settings="lockdown: true\n" + _JWT + cron_yaml, + ) + (ws / "config" / "cron").mkdir(parents=True, exist_ok=True) + return load_config(config_dir), ws + + def test_gate_plugins_dir_outside_the_workspace_is_dropped(self, tmp_path): + outside = tmp_path / "outside" + outside.mkdir() + c, ws = self._locked(tmp_path, f"cron:\n gate_plugins_dir: {outside}\n") + assert c.cron.gate_plugins_dir == ws / "config" / "cron" / "gates" + + def test_the_redirect_no_longer_gets_code_executed(self, tmp_path): + """The end that matters: with the path contained, the plugin loader never + sees the outside directory, so nothing in it runs.""" + from nerve.cron.gate_plugins import load_gate_plugins + + outside = tmp_path / "outside" + outside.mkdir() + marker = tmp_path / "executed" + (outside / "evil.py").write_text( + f"open({str(marker)!r}, 'w').write('ran')\n", encoding="utf-8", + ) + c, ws = self._locked(tmp_path, f"cron:\n gate_plugins_dir: {outside}\n") + assert load_gate_plugins(c.cron.gate_plugins_dir) == 0 + assert not marker.exists() + + def test_jobs_and_system_files_outside_are_dropped(self, tmp_path): + c, ws = self._locked( + tmp_path, + f"cron:\n" + f" jobs_file: {tmp_path}/elsewhere/jobs.yaml\n" + f" system_file: {ws_escape(tmp_path)}\n", + ) + assert c.cron.jobs_file == ws / "config" / "cron" / "jobs.yaml" + assert c.cron.system_file == ws / "config" / "cron" / "system.yaml" + + def test_inside_the_workspace_but_outside_config_is_dropped(self, tmp_path): + """Containment is to ``/config``, not to the workspace: the + workspace is also the agent's working directory, and a path it can write + to freely is not a reviewed one.""" + c, ws = self._locked( + tmp_path, f"cron:\n gate_plugins_dir: {tmp_path}/ws/scratch/gates\n", + ) + assert c.cron.gate_plugins_dir == ws / "config" / "cron" / "gates" + + def test_a_symlink_out_of_the_tree_is_dropped(self, tmp_path): + """A path that *is* inside the subtree by name but resolves out of it — + containment is judged on the resolved path for exactly this reason.""" + outside = tmp_path / "outside" + outside.mkdir() + config_dir, ws = _install(tmp_path, settings="lockdown: true\n" + _JWT) + (ws / "config" / "cron").mkdir(parents=True) + link = ws / "config" / "cron" / "borrowed-gates" + link.symlink_to(outside, target_is_directory=True) + workspace_settings_file(ws).write_text( + "lockdown: true\n" + _JWT + + f"cron:\n gate_plugins_dir: {link}\n", encoding="utf-8", + ) + c = load_config(config_dir) + assert c.cron.gate_plugins_dir == ws / "config" / "cron" / "gates" + + def test_a_symlinked_cron_directory_refuses_to_load(self, tmp_path): + """No substitution can fix this one — every default is derived from the + escaping directory — so a locked instance declines to start.""" + outside = tmp_path / "outside" + outside.mkdir() + config_dir, ws = _install(tmp_path, settings="lockdown: true\n" + _JWT) + (ws / "config" / "cron").symlink_to(outside, target_is_directory=True) + with pytest.raises(ConfigError) as ei: + load_config(config_dir) + assert "outside the tracked config subtree" in str(ei.value) + + @pytest.mark.parametrize("name", ["gates", "jobs.yaml", "system.yaml"]) + def test_the_default_name_itself_as_a_symlink_refuses_to_load(self, tmp_path, name): + """The case with no config key in it at all. + + The fallback for an escaping path is the in-workspace default, so when the + *default's own name* is the symlink there is nothing contained to fall back + to: substituting it hands back the path just rejected, with a warning that + contradicts itself. Git tracks symlinks, so this needs no local write — + a reviewed, merged config repo is enough. + """ + outside = tmp_path / "outside" + outside.mkdir() + (outside / "x.py").write_text("MARKER = 1\n", encoding="utf-8") + config_dir, ws = _install(tmp_path, settings="lockdown: true\n" + _JWT) + cron = ws / "config" / "cron" + cron.mkdir(parents=True) + target = outside if name == "gates" else outside / "x.py" + (cron / name).symlink_to(target, target_is_directory=(name == "gates")) + with pytest.raises(ConfigError) as ei: + load_config(config_dir) + assert "outside the tracked config subtree" in str(ei.value) + + def test_a_symlinked_gates_default_never_gets_code_executed(self, tmp_path): + """The same case, ended at the consequence rather than the exception.""" + from nerve.cron.gate_plugins import load_gate_plugins + + outside = tmp_path / "unreviewed_gates" + outside.mkdir() + marker = tmp_path / "executed" + (outside / "evil.py").write_text( + f"open({str(marker)!r}, 'w').write('ran')\n", encoding="utf-8", + ) + config_dir, ws = _install(tmp_path, settings="lockdown: true\n" + _JWT) + (ws / "config" / "cron").mkdir(parents=True) + (ws / "config" / "cron" / "gates").symlink_to(outside, target_is_directory=True) + with pytest.raises(ConfigError): + config = load_config(config_dir) + load_gate_plugins(config.cron.gate_plugins_dir) + assert not marker.exists() + + +class TestLockdownTrackedSubtreeIsInTheWorkspace: + """One level above cron: ``/config`` itself. + + Every other containment check judges a path against that directory, so if it + is a symlink out of the workspace they all pass while nothing underneath is in + the reviewed repo — settings.yaml and the gate plugins included, with lockdown + read from there and reporting that all is well. + """ + + def _elsewhere(self, tmp_path, settings): + config_dir = tmp_path / "cfg" + ws = tmp_path / "ws" + elsewhere = tmp_path / "local-config" + config_dir.mkdir() + ws.mkdir() + (elsewhere / "cron").mkdir(parents=True) + (config_dir / "config.yaml").write_text( + f"workspace: {ws}\n", encoding="utf-8", + ) + (ws / "config").symlink_to(elsewhere, target_is_directory=True) + (elsewhere / "settings.yaml").write_text(settings, encoding="utf-8") + return config_dir, ws + + def test_config_symlinked_out_of_the_workspace_refuses_to_load(self, tmp_path): + config_dir, ws = self._elsewhere(tmp_path, "lockdown: true\n" + _JWT) + with pytest.raises(ConfigError) as ei: + load_config(config_dir) + assert "outside the workspace" in str(ei.value) + + def test_unlocked_is_unaffected(self, tmp_path): + """A symlinked config/ is a legitimate machine-local arrangement; only a + locked instance, which claims that tree *is* the reviewed repo, cannot.""" + config_dir, ws = self._elsewhere(tmp_path, "timezone: UTC\n") + assert load_config(config_dir).timezone == "UTC" + + def test_the_validator_reports_it(self, tmp_path): + from nerve.config_validate import validate_config_bundle + + config_dir, ws = self._elsewhere(tmp_path, "lockdown: true\n" + _JWT) + result = validate_config_bundle(config_dir, workspace_override=ws) + assert any("outside the workspace" in e for e in result.errors) + + def test_a_symlinked_workspace_is_still_fine(self, tmp_path): + """Where a machine keeps its workspace stays a machine-local decision.""" + real = tmp_path / "real-ws" + (real / "config").mkdir(parents=True) + _repo(real) + link = tmp_path / "ws" + link.symlink_to(real, target_is_directory=True) + config_dir = tmp_path / "cfg" + config_dir.mkdir() + (config_dir / "config.yaml").write_text( + f"workspace: {link}\n", encoding="utf-8", + ) + (real / "config" / "settings.yaml").write_text( + "lockdown: true\n" + _JWT, encoding="utf-8", + ) + assert load_config(config_dir).lockdown is True + + def test_unlocked_overrides_are_still_honored(self, tmp_path): + """An unmigrated install is entitled to point cron anywhere — that is what + the machine-local layers are for.""" + outside = tmp_path / "outside" + outside.mkdir() + config_dir, ws = _install( + tmp_path, base=f"cron:\n gate_plugins_dir: {outside}\n", + ) + assert load_config(config_dir).cron.gate_plugins_dir == outside + + def test_validation_contains_paths_against_the_candidate_workspace(self, tmp_path): + """The validator pins the workspace to the tree under review, so the same + containment has to be judged there — a candidate bundle must not be able + to make validation read from outside its own checkout.""" + from nerve.config_validate import validate_config_bundle + + config_dir = tmp_path / "cfg" + candidate = tmp_path / "candidate" + outside = tmp_path / "outside" + config_dir.mkdir() + (candidate / "config" / "cron").mkdir(parents=True) + _repo(candidate) + outside.mkdir() + (outside / "jobs.yaml").write_text("jobs: not-a-list\n", encoding="utf-8") + (candidate / "config" / "cron" / "jobs.yaml").write_text( + "jobs: []\n", encoding="utf-8", + ) + workspace_settings_file(candidate).write_text( + "lockdown: true\n" + _JWT + + f"cron:\n jobs_file: {outside}/jobs.yaml\n", encoding="utf-8", + ) + result = validate_config_bundle(config_dir, workspace_override=candidate) + assert result.ok, result.errors + assert any(str(candidate) in i and "cron jobs" in i for i in result.info) + + def _sibling(self, tmp_path, settings): + """``config`` as a symlink to a sibling directory *inside* the workspace.""" + config_dir = tmp_path / "cfg" + ws = tmp_path / "ws" + config_dir.mkdir() + (ws / "notes" / "cfg" / "cron" / "gates").mkdir(parents=True) + (config_dir / "config.yaml").write_text(f"workspace: {ws}\n", encoding="utf-8") + (ws / "config").symlink_to(Path("notes") / "cfg", target_is_directory=True) + (ws / "notes" / "cfg" / "settings.yaml").write_text(settings, encoding="utf-8") + return config_dir, ws + + def test_config_symlinked_to_a_sibling_directory_refuses_to_load(self, tmp_path): + """Containment alone does not make the subtree the reviewed one. The link + target is inside the workspace, so every check that resolves passes.""" + config_dir, ws = self._sibling(tmp_path, "lockdown: true\n" + _JWT) + with pytest.raises(ConfigError) as ei: + load_config(config_dir) + assert "must be a real directory" in str(ei.value) + + def test_a_symlinked_config_stays_fine_unlocked(self, tmp_path): + config_dir, ws = self._sibling(tmp_path, "timezone: UTC\n") + assert load_config(config_dir).timezone == "UTC" + + @pytest.mark.skipif(not shutil.which("git"), reason="git not available") + def test_git_reports_nothing_under_a_symlinked_config(self, tmp_path): + """Why the symlink is refused rather than tolerated: git does not descend + into it, so sync's divergence check — the thing that keeps unreviewed + ``cron/gates/*.py`` off a locked box — sees an empty subtree and calls the + workspace clean while the daemon imports what is there. + """ + from nerve.sync_service import _local_config_divergence + + config_dir, ws = self._sibling(tmp_path, "lockdown: true\n" + _JWT) + env = {"GIT_AUTHOR_NAME": "t", "GIT_AUTHOR_EMAIL": "t@t", + "GIT_COMMITTER_NAME": "t", "GIT_COMMITTER_EMAIL": "t@t", + "HOME": str(tmp_path), "PATH": os.environ["PATH"]} + for args in (["init", "-b", "main"], ["add", "-A"], ["commit", "-m", "init"]): + subprocess.run(["git", *args], cwd=str(ws), check=True, + capture_output=True, text=True, env=env) + (ws / "notes" / "cfg" / "cron" / "gates" / "evil.py").write_text( + "MARKER = 1\n", encoding="utf-8", + ) + # Untracked code the daemon would import, and git says the subtree is clean. + assert _local_config_divergence(ws, "HEAD", True) == ([], []) + # So the layout has to be refused at the only place that can see it. + with pytest.raises(ConfigError): + load_config(config_dir) + + +class TestLockdownSettingsFileIsInTheSubtree: + """The subtree can be the reviewed tree and ``settings.yaml`` still not be + part of it. It carries the ``lockdown`` flag, the auth secret and the cron + paths, so a symlink there is the whole tracked layer arriving from a file no + reviewer saw — and, when it points at an ordinary workspace file, from one the + agent may write with the tools lockdown leaves it. + """ + + def _linked(self, tmp_path, target, settings="lockdown: true\n" + _JWT): + config_dir = tmp_path / "cfg" + ws = tmp_path / "ws" + config_dir.mkdir() + (ws / "config").mkdir(parents=True) + (ws / "notes").mkdir() + (config_dir / "config.yaml").write_text(f"workspace: {ws}\n", encoding="utf-8") + real = ws / "notes" / "settings.yaml" if target == "inside" else tmp_path / "outside.yaml" + real.write_text(settings, encoding="utf-8") + link = Path("..") / "notes" / "settings.yaml" if target == "inside" else real + workspace_settings_file(ws).symlink_to(link) + return config_dir, ws + + def test_settings_symlinked_out_of_the_workspace_refuses_to_load(self, tmp_path): + config_dir, ws = self._linked(tmp_path, "outside") + with pytest.raises(ConfigError) as ei: + load_config(config_dir) + assert "outside the tracked config subtree" in str(ei.value) + + def test_settings_symlinked_to_an_ordinary_workspace_file_refuses_to_load(self, tmp_path): + """A relative link that never leaves the workspace, which is the harder + case: nothing about it escapes, and the target is a file the agent writes + the same way it writes its notes.""" + config_dir, ws = self._linked( + tmp_path, "inside", "lockdown: true\nauth:\n jwt_secret: attacker\n", + ) + with pytest.raises(ConfigError) as ei: + load_config(config_dir) + assert "outside the tracked config subtree" in str(ei.value) + + def test_the_validator_reports_it(self, tmp_path): + from nerve.config_validate import validate_config_bundle + + config_dir, ws = self._linked(tmp_path, "inside") + result = validate_config_bundle(config_dir, workspace_override=ws) + assert any("outside the tracked config subtree" in e for e in result.errors) + + def test_unlocked_is_unaffected(self, tmp_path): + config_dir, ws = self._linked(tmp_path, "outside", "timezone: UTC\n") + assert load_config(config_dir).timezone == "UTC" + + def test_a_real_settings_file_loads(self, tmp_path): + config_dir, ws = _install(tmp_path, settings="lockdown: true\ntimezone: UTC\n" + _JWT) + assert lockdown_workspace_problems(ws) == [] + assert load_config(config_dir).timezone == "UTC" + + def test_an_absent_settings_file_is_not_a_problem(self, tmp_path): + """Nothing to point anywhere. A locked instance can be anchored by the + environment with no tracked settings file at all.""" + ws = tmp_path / "ws" + (ws / "config").mkdir(parents=True) + _repo(ws) + assert lockdown_workspace_problems(ws) == [] + + def test_a_link_within_the_subtree_is_fine(self, tmp_path): + """Both ends are tracked and reviewed, so the name is a layout choice the + config repo is entitled to make. The check is containment, not a ban on + symlinks.""" + config_dir, ws = _install(tmp_path) + (ws / "config" / "shared.yaml").write_text( + "lockdown: true\ntimezone: UTC\n" + _JWT, encoding="utf-8", + ) + workspace_settings_file(ws).symlink_to(Path("shared.yaml")) + assert load_config(config_dir).timezone == "UTC" + + def test_every_problem_is_reported_not_just_the_first(self, tmp_path): + """An operator fixing a layout wants the list; the loader only needs one.""" + ws = tmp_path / "ws" + (ws / "notes" / "cfg").mkdir(parents=True) + _repo(ws) + (ws / "notes" / "settings.yaml").write_text("lockdown: true\n", encoding="utf-8") + (ws / "config").symlink_to(Path("notes") / "cfg", target_is_directory=True) + (ws / "notes" / "cfg" / "settings.yaml").symlink_to( + Path("..") / "settings.yaml", + ) + problems = lockdown_workspace_problems(ws) + assert len(problems) == 2 + assert any("must be a real directory" in p for p in problems) + assert any("outside the tracked config subtree" in p for p in problems) + + +@pytest.mark.skipif(not shutil.which("git"), reason="git not available") +class TestLockdownNeedsSomewhereToReceiveConfigFrom: + """A locked instance refuses every local change to the reviewed surface and + tells the operator to open a PR instead. That flow ends in a git pull, so + sync is the only route left in, and a workspace with no remote has no route + at all: nothing local may change the config and nothing remote can arrive. + + Refused rather than ignored. Honoring the flag only where a remote happens to + be configured would make ``git remote remove origin`` an unlock — a + machine-local one, which is what ``lockdown`` being unreadable from + config.yaml and ``NERVE_LOCKDOWN`` never unlocking already exist to prevent. + """ + + def _workspace(self, tmp_path, settings="lockdown: true\n" + _JWT): + config_dir = tmp_path / "cfg" + ws = tmp_path / "ws" + config_dir.mkdir() + (ws / "config").mkdir(parents=True) + (config_dir / "config.yaml").write_text(f"workspace: {ws}\n", encoding="utf-8") + workspace_settings_file(ws).write_text(settings, encoding="utf-8") + return config_dir, ws + + def test_a_workspace_that_is_not_a_repository_refuses_to_load(self, tmp_path): + config_dir, ws = self._workspace(tmp_path) + with pytest.raises(ConfigError) as ei: + load_config(config_dir) + assert "is not a git repository" in str(ei.value) + + def test_a_repository_with_no_remote_refuses_to_load(self, tmp_path): + config_dir, ws = self._workspace(tmp_path) + _repo(ws, remote=None) + with pytest.raises(ConfigError) as ei: + load_config(config_dir) + assert "has no git remote" in str(ei.value) + + def test_a_remote_is_all_it_takes(self, tmp_path): + config_dir, ws = self._workspace(tmp_path) + _repo(ws) + assert load_config(config_dir).lockdown is True + + def test_the_remote_need_not_be_named_origin(self, tmp_path): + """Which remote to fetch is sync's decision, not this check's: with + ``workspace_sync.branch`` unset it follows the current branch's own + upstream, which need not be ``origin``. Demanding the name would refuse a + workspace that syncs today.""" + config_dir, ws = self._workspace(tmp_path) + _repo(ws, remote="upstream") + assert load_config(config_dir).lockdown is True + + def test_dropping_the_remote_does_not_unlock_the_box(self, tmp_path): + """The fail-open version of this check, stated as what it would do. A box + that came up locked must not come up unlocked after one local command.""" + config_dir, ws = self._workspace(tmp_path) + _repo(ws) + assert load_config(config_dir).lockdown is True + subprocess.run(["git", "remote", "remove", "origin"], cwd=str(ws), + check=True, capture_output=True) + with pytest.raises(ConfigError) as ei: + load_config(config_dir) + assert "has no git remote" in str(ei.value) + + def test_unlocked_is_unaffected(self, tmp_path): + """Most workspaces are not config repos at all. Where the config came from + is only a question for an instance that claims it came from review.""" + config_dir, ws = self._workspace(tmp_path, "timezone: UTC\n") + assert load_config(config_dir).timezone == "UTC" + + def test_the_validator_reports_it(self, tmp_path): + from nerve.config_validate import validate_config_bundle + + config_dir, ws = self._workspace(tmp_path) + result = validate_config_bundle(config_dir, workspace_override=ws) + assert any("no way to reach this instance" in e for e in result.errors) + + def test_git_being_unusable_is_not_a_pass(self, tmp_path, monkeypatch): + """Not being able to tell is not a yes, and a box that cannot run git + cannot sync either.""" + import nerve.sync_service as sync + + config_dir, ws = self._workspace(tmp_path) + _repo(ws) + monkeypatch.setattr(sync, "_git", lambda args, cwd: subprocess.CompletedProcess( + args, 1, "", "could not run git: [Errno 2] No such file or directory: 'git'", + )) + with pytest.raises(ConfigError) as ei: + load_config(config_dir) + assert "could not list the remotes" in str(ei.value) + + +class TestLockdownWriteGuardJudgesBothViewsOfAPath: + """A symlink is where "where does this path end up" and "what does this path + name" stop agreeing, and each direction of the disagreement is a way through. + Both are checked, and both directions are pinned here so a later refactor + cannot drop one for the other. + """ + + def _locked(self, tmp_path, monkeypatch): + ws = tmp_path / "ws" + (ws / "config" / "cron" / "gates").mkdir(parents=True) + (ws / "notes").mkdir() + monkeypatch.setattr(cfg, "_config", NerveConfig(lockdown=True, workspace=ws)) + return ws + + def test_a_symlink_at_the_reviewed_name_is_refused(self, tmp_path, monkeypatch): + """The lexical direction. Resolving alone puts the target outside the + subtree, which reads as "not lockdown's business" — while the write goes + through the link to the file the daemon loads as tracked config.""" + ws = self._locked(tmp_path, monkeypatch) + outside = tmp_path / "outside.yaml" + outside.write_text("lockdown: true\n", encoding="utf-8") + (ws / "config" / "settings.yaml").symlink_to(outside) + assert tracked_config_write_refusal("config/settings.yaml") + assert tracked_config_write_refusal(ws / "config" / "settings.yaml") + + def test_a_relative_link_that_stays_in_the_workspace_is_refused_too(self, tmp_path, monkeypatch): + ws = self._locked(tmp_path, monkeypatch) + (ws / "notes" / "settings.yaml").write_text("lockdown: true\n", encoding="utf-8") + (ws / "config" / "settings.yaml").symlink_to( + Path("..") / "notes" / "settings.yaml", + ) + assert tracked_config_write_refusal("config/settings.yaml") + + @pytest.mark.parametrize("path", [ + "notes/settings-link.yaml", # a file symlink into the subtree + "notes/cfgdir/settings.yaml", # through a directory symlink + "notes/cfgdir/cron/gates/new.py", # a path that does not exist yet + "notes/../config/settings.yaml", # .. traversal back in + ]) + def test_a_path_that_reaches_into_the_subtree_is_refused(self, tmp_path, monkeypatch, path): + """The resolving direction, which the lexical check cannot see. Dropping + it in favour of comparing names would reopen every one of these.""" + ws = self._locked(tmp_path, monkeypatch) + (ws / "config" / "settings.yaml").write_text("lockdown: true\n", encoding="utf-8") + (ws / "notes" / "settings-link.yaml").symlink_to(ws / "config" / "settings.yaml") + (ws / "notes" / "cfgdir").symlink_to(ws / "config", target_is_directory=True) + assert tracked_config_write_refusal(path) + + def test_ordinary_workspace_files_stay_writable(self, tmp_path, monkeypatch): + """Including the target of the link above. The workspace is the agent's + working directory and lockdown does not make it read-only; what keeps a + link at ``config/settings.yaml`` from mattering is that a locked instance + with one refuses to start — see + :class:`TestLockdownSettingsFileIsInTheSubtree`. + """ + ws = self._locked(tmp_path, monkeypatch) + (ws / "config" / "settings.yaml").symlink_to( + Path("..") / "notes" / "settings.yaml", + ) + assert tracked_config_write_refusal("notes/settings.yaml") is None + assert tracked_config_write_refusal("memory/notes.md") is None + + +class TestLockdownReviewedSurface: + """``config/`` is not the whole of what a locked instance was reviewed for. + + ``skills/`` reaches the model as instructions with their own + ``allowed-tools``, indexed by ``SkillManager.discover`` at startup and on + every reload, and the root instruction files are the system prompt. The skill + endpoints refuse under lockdown already, which only meant the 403 could be + walked around with the plainest tool the agent has. + """ + + def _locked(self, tmp_path, monkeypatch): + ws = tmp_path / "ws" + (ws / "config").mkdir(parents=True) + monkeypatch.setattr(cfg, "_config", NerveConfig(lockdown=True, workspace=ws)) + return ws + + @pytest.mark.parametrize("path", [ + "skills/backdoor/SKILL.md", + "skills/backdoor/scripts/run.sh", + "skills", + "AGENTS.md", + "SOUL.md", + "IDENTITY.md", + "USER.md", + "TOOLS.md", + ]) + def test_the_reviewed_surface_is_refused(self, tmp_path, monkeypatch, path): + self._locked(tmp_path, monkeypatch) + refusal = tracked_config_write_refusal(path) + assert refusal, path + assert "pull request" in refusal + + @pytest.mark.parametrize("path", [ + "memory/notes.md", + "tasks/active/t1.md", + "README.md", + # In PROMPT_FILES beside SOUL.md and read into the same prompt, and + # deliberately not reviewed: it is the running task list the agent keeps. + "TASK.md", + # Neighbouring names, to pin that the match is on path components. + "skills.md", + "SOUL.md.bak", + "memory/SOUL.md", + ]) + def test_the_rest_of_the_workspace_stays_writable(self, tmp_path, monkeypatch, path): + self._locked(tmp_path, monkeypatch) + assert tracked_config_write_refusal(path) is None, path + + def test_a_symlink_at_a_reviewed_root_file_is_refused(self, tmp_path, monkeypatch): + """The lexical half, one directory up from ``config/``: writing through + the link changes the file the prompt is built from. + + The link's target goes with it, unlike the ``config/settings.yaml`` case + where the reviewed root is the directory and a link at a file inside it + leaves the target an ordinary workspace file. Here the reviewed root *is* + the file, so resolving it names the target — and that is the answer this + one needs: nothing refuses a locked instance whose ``SOUL.md`` is a + symlink, so the write guard is the only thing standing between the agent + and its own instructions. + """ + ws = self._locked(tmp_path, monkeypatch) + (ws / "notes").mkdir() + (ws / "notes" / "soul.md").write_text("you are helpful\n", encoding="utf-8") + (ws / "SOUL.md").symlink_to(Path("notes") / "soul.md") + assert tracked_config_write_refusal("SOUL.md") + assert tracked_config_write_refusal("notes/soul.md") + + def test_a_path_that_reaches_into_skills_is_refused(self, tmp_path, monkeypatch): + ws = self._locked(tmp_path, monkeypatch) + (ws / "skills" / "x").mkdir(parents=True) + (ws / "notes").mkdir() + (ws / "notes" / "skilldir").symlink_to(ws / "skills", target_is_directory=True) + assert tracked_config_write_refusal("notes/skilldir/x/SKILL.md") + assert tracked_config_write_refusal("notes/../skills/x/SKILL.md") + + def test_unlocked_writes_skills_freely(self, tmp_path, monkeypatch): + ws = tmp_path / "ws" + (ws / "config").mkdir(parents=True) + monkeypatch.setattr(cfg, "_config", NerveConfig(lockdown=False, workspace=ws)) + assert tracked_config_write_refusal("skills/x/SKILL.md") is None + assert tracked_config_write_refusal("SOUL.md") is None + + def test_a_symlinked_skills_directory_refuses_to_load(self, tmp_path): + """Same layout, same reason as a symlinked ``config/``: git does not + descend into it, so nothing under it is tracked while ``discover`` indexes + whatever is there.""" + config_dir = tmp_path / "cfg" + ws = tmp_path / "ws" + config_dir.mkdir() + (ws / "config").mkdir(parents=True) + (ws / "notes" / "skills").mkdir(parents=True) + (config_dir / "config.yaml").write_text(f"workspace: {ws}\n", encoding="utf-8") + workspace_settings_file(ws).write_text("lockdown: true\n" + _JWT, encoding="utf-8") + (ws / "skills").symlink_to(Path("notes") / "skills", target_is_directory=True) + + with pytest.raises(ConfigError) as ei: + load_config(config_dir) + assert "must be a real directory" in str(ei.value) + assert "skills" in str(ei.value) + + def test_skills_symlinked_out_of_the_workspace_refuses_to_load(self, tmp_path): + config_dir = tmp_path / "cfg" + ws = tmp_path / "ws" + elsewhere = tmp_path / "local-skills" + config_dir.mkdir() + elsewhere.mkdir() + (ws / "config").mkdir(parents=True) + (config_dir / "config.yaml").write_text(f"workspace: {ws}\n", encoding="utf-8") + workspace_settings_file(ws).write_text("lockdown: true\n" + _JWT, encoding="utf-8") + (ws / "skills").symlink_to(elsewhere, target_is_directory=True) + + with pytest.raises(ConfigError) as ei: + load_config(config_dir) + assert "outside the workspace" in str(ei.value) + + def test_an_absent_skills_directory_is_not_a_problem(self, tmp_path): + """Most workspaces have no skills at all, and a missing directory is not + a redirected one.""" + ws = tmp_path / "ws" + (ws / "config").mkdir(parents=True) + _repo(ws) + assert lockdown_workspace_problems(ws) == [] + + def test_a_symlinked_skills_directory_stays_fine_unlocked(self, tmp_path): + config_dir = tmp_path / "cfg" + ws = tmp_path / "ws" + elsewhere = tmp_path / "local-skills" + config_dir.mkdir() + elsewhere.mkdir() + (ws / "config").mkdir(parents=True) + (config_dir / "config.yaml").write_text(f"workspace: {ws}\n", encoding="utf-8") + workspace_settings_file(ws).write_text("timezone: UTC\n", encoding="utf-8") + (ws / "skills").symlink_to(elsewhere, target_is_directory=True) + assert load_config(config_dir).timezone == "UTC" + + +# Representative workspace-relative paths, and whether each is part of the +# reviewed surface. Hand-written on purpose: it is the statement the three +# implementations below are measured against, and deriving it from any of them +# would only make each agree with itself. +_SURFACE_TABLE: dict[str, bool] = { + "config/settings.yaml": True, + "config/cron/jobs.yaml": True, + "config/cron/gates/gate.py": True, + "skills/x/SKILL.md": True, + "AGENTS.md": True, + "SOUL.md": True, + "IDENTITY.md": True, + "USER.md": True, + "TOOLS.md": True, + "memory/notes.md": False, + "tasks/active/t1.md": False, + "TASK.md": False, + "README.md": False, + "../outside.md": False, +} + + +class TestReviewedSurfaceAgreement: + """Three implementations, one surface, and nothing that made them agree. + + The write guard decides what a locked instance may not write, sync's + divergence check decides what a local edit blocks a merge over, and the + startup check decides which subtrees have to really be in the workspace. + They were written at different times against ``config/`` alone, and the + branch that started treating ``skills/`` as reviewed changed one of them: + ``create_skill`` refused while ``Write`` did not, so the same file was + forbidden through the endpoint, allowed through the tool, and invisible to + sync. + + Each row of ``_SURFACE_TABLE`` is put to all three. What is asserted is + agreement with the table, not agreement with each other — three + implementations can be uniformly wrong. + """ + + def test_the_table_covers_the_declared_surface(self): + """The table is the artifact the rest of this class trusts, so it is + checked against the constants rather than against any of the guards.""" + from nerve.config import REVIEWED_DIRS, REVIEWED_ROOT_FILES + + reviewed = [p for p, want in _SURFACE_TABLE.items() if want] + dirs_covered = {p.split("/")[0] for p in reviewed if "/" in p} + files_covered = {p for p in reviewed if "/" not in p} + assert set(REVIEWED_DIRS) <= dirs_covered, ( + "a reviewed directory no row of the table names — every guard below " + "would pass without ever looking at it" + ) + assert REVIEWED_ROOT_FILES <= files_covered, ( + "a reviewed root file no row of the table names" + ) + stray = [ + p for p in reviewed + if p.split("/")[0] not in REVIEWED_DIRS and p not in REVIEWED_ROOT_FILES + ] + assert not stray, f"rows claiming to be reviewed that nothing declares: {stray}" + + @pytest.mark.parametrize("path,reviewed", sorted(_SURFACE_TABLE.items())) + def test_the_write_guard_agrees(self, tmp_path, monkeypatch, path, reviewed): + ws = tmp_path / "ws" + (ws / "config").mkdir(parents=True) + monkeypatch.setattr(cfg, "_config", NerveConfig(lockdown=True, workspace=ws)) + assert (tracked_config_write_refusal(path) is not None) is reviewed, path + + @pytest.mark.skipif(not shutil.which("git"), reason="git not available") + def test_the_sync_pathspec_agrees(self, tmp_path): + """Asked of git, not of the pathspec: an entry that is spelled wrong, or + that git reads as something other than a path, matches nothing and would + pass a comparison against the list itself. + """ + from nerve.sync_service import _local_config_divergence + + ws = tmp_path / "ws" + ws.mkdir() + env = {"GIT_AUTHOR_NAME": "t", "GIT_AUTHOR_EMAIL": "t@t", + "GIT_COMMITTER_NAME": "t", "GIT_COMMITTER_EMAIL": "t@t", + "HOME": str(tmp_path), "PATH": os.environ["PATH"]} + (ws / ".keep").write_text("", encoding="utf-8") + for args in (["init", "-b", "main"], ["add", "-A"], ["commit", "-m", "init"]): + subprocess.run(["git", *args], cwd=str(ws), check=True, + capture_output=True, text=True, env=env) + + # Outside the repository, so git has nothing to say about it either way. + skipped = [p for p in _SURFACE_TABLE if p.startswith("../")] + assert all(not _SURFACE_TABLE[p] for p in skipped), ( + "a reviewed path outside the workspace makes no sense; the sync check " + "could never see it" + ) + for path in _SURFACE_TABLE: + if path in skipped: + continue + target = ws / path + target.parent.mkdir(parents=True, exist_ok=True) + target.write_text("local\n", encoding="utf-8") + + blocking, _warnings = _local_config_divergence(ws, "HEAD") + named = {entry.split(" ", 1)[1] for entry in blocking} + for path, reviewed in _SURFACE_TABLE.items(): + if path in skipped: + continue + assert (path in named) is reviewed, (path, sorted(named)) + + def test_the_startup_check_agrees(self, tmp_path): + """Redirect the subtree each path lives in and ask whether a locked + instance still calls the layout sound. + + The construction needs a subtree to redirect, so rows naming a file at + the workspace root, or a path outside it, have nothing to build — see the + skip assertion. What that leaves uncovered is real and named in + ``docs/config.md``: a locked instance whose ``SOUL.md`` is a symlink + starts, and it is the write guard, refusing the link's target as well, + that keeps the file the prompt is built from unwritable. + """ + from nerve.config import REVIEWED_DIRS + + skipped = [p for p in _SURFACE_TABLE if "/" not in p or p.startswith("../")] + assert all( + "/" not in p or p.startswith("../") for p in skipped + ), "only a path with no subtree of its own may be skipped here" + assert set(REVIEWED_DIRS) <= { + p.split("/")[0] for p, want in _SURFACE_TABLE.items() + if want and p not in skipped + }, "a reviewed directory that the skip rule would let past unchecked" + + for i, (path, reviewed) in enumerate(sorted(_SURFACE_TABLE.items())): + if path in skipped: + continue + ws = tmp_path / f"ws{i}" + (ws / "config").mkdir(parents=True) + _repo(ws) + workspace_settings_file(ws).write_text( + "lockdown: true\n" + _JWT, encoding="utf-8", + ) + subtree = path.split("/")[0] + elsewhere = tmp_path / f"elsewhere{i}" + elsewhere.mkdir() + if subtree == "config": + # Already a real directory; replace it with the redirect. + shutil.rmtree(ws / "config") + (elsewhere / "settings.yaml").write_text( + "lockdown: true\n" + _JWT, encoding="utf-8", + ) + (ws / subtree).symlink_to(elsewhere, target_is_directory=True) + + problems = lockdown_workspace_problems(ws) + assert bool(problems) is reviewed, (path, problems) + + +def ws_escape(tmp_path: Path) -> str: + """A ``..``-traversal path that climbs out of the workspace config subtree.""" + return f"{tmp_path}/ws/config/cron/../../../system.yaml" + + +class TestLockdownTelegramDefault: + """``telegram.enabled`` is a per-machine decision, so ``nerve init`` writes it + to ``config.yaml`` — the layer lockdown drops. Left to its declared default + that would read as "on", so a box where Telegram was switched off would start + answering DMs, with full agent access, as soon as the shared settings carried + a token its environment can resolve. + """ + + _TOKEN = "telegram:\n bot_token: 12345:abc\n" + + def test_locked_leaves_telegram_off_when_the_tracked_file_is_silent(self, tmp_path): + config_dir, ws = _install( + tmp_path, + settings="lockdown: true\n" + _JWT + self._TOKEN, + base="telegram:\n enabled: false\n", + ) + c = load_config(config_dir) + assert c.telegram.enabled is False + assert c.telegram.bot_token # the token is there; the switch is not + + def test_locked_honors_an_explicit_enable(self, tmp_path): + config_dir, ws = _install( + tmp_path, + settings="lockdown: true\n" + _JWT + + "telegram:\n enabled: true\n bot_token: 12345:abc\n", + ) + assert load_config(config_dir).telegram.enabled is True + + def test_locked_honors_an_env_ref_per_machine(self, tmp_path, monkeypatch): + monkeypatch.setenv("TG_ON", "true") + config_dir, ws = _install( + tmp_path, + settings="lockdown: true\n" + _JWT + + "telegram:\n enabled: ${TG_ON}\n bot_token: 12345:abc\n", + ) + assert load_config(config_dir).telegram.enabled is True + + def test_unlocked_default_is_unchanged(self, tmp_path): + config_dir, ws = _install(tmp_path, base=self._TOKEN) + assert load_config(config_dir).telegram.enabled is True + + @pytest.mark.parametrize( + "spelling", ["yess", "disabled", "${TG_ON:-nope}", "[]"], + ) + def test_locked_reads_an_unparseable_value_as_off(self, tmp_path, spelling): + """Being unstated was not the only way to reach the wrong answer. + + A value the parser cannot read falls back to a default, and coercion's + default is the field's *declared* one — ``True``. So the whole guard was + one typo wide, and `${TG_ON:-nope}` is the very spelling the docs + recommend for a per-box fleet setting: one bad env value would have turned + the bot on across the fleet. + """ + config_dir, ws = _install( + tmp_path, + settings="lockdown: true\n" + _JWT + + f"telegram:\n enabled: {spelling}\n bot_token: 12345:abc\n", + base="telegram:\n enabled: false\n", + ) + assert load_config(config_dir).telegram.enabled is False + + def test_unlocked_unparseable_value_keeps_the_declared_default(self, tmp_path): + """Only the fallback *direction* is lockdown's business. Unlocked, the + machine layer is where this is normally stated and reverting to the + documented default is what an operator expects.""" + config_dir, ws = _install( + tmp_path, base="telegram:\n enabled: yess\n bot_token: 12345:abc\n", + ) + assert load_config(config_dir).telegram.enabled is True + + def test_validator_names_the_setting_that_has_nowhere_to_live(self, tmp_path): + from nerve.config_validate import validate_config_bundle + + config_dir, ws = _install( + tmp_path, settings="lockdown: true\n" + _JWT + self._TOKEN, + ) + result = validate_config_bundle(config_dir, workspace_override=ws) + assert result.ok + assert any("telegram.enabled" in w for w in result.warnings) + + def test_every_dropped_key_is_named_not_just_telegram(self, tmp_path): + """The note previously covered one key of the seventeen on the list. + + The uncovered case that mattered was a locked box reverting from Bedrock + to the Anthropic default: nothing reported it, and the only symptom was a + missing API key the box would not otherwise have needed. + """ + from nerve.config_validate import validate_config_bundle + + config_dir, ws = _install( + tmp_path, + settings="lockdown: true\n" + _JWT, + base=( + "gateway:\n port: 9100\n ssl:\n cert: /etc/tls/c.pem\n" + "provider:\n type: bedrock\n aws_region: eu-west-1\n" + " aws_profile: prod\n" + "sync:\n gmail:\n accounts: [me@example.com]\n" + ), + ) + notes = " ".join( + validate_config_bundle(config_dir, workspace_override=ws).warnings + ) + # Shareable values that should not have been stranded in config.yaml. + for path in ("gateway.port", "provider.type", "provider.aws_region"): + assert path in notes, path + # ...reported separately from the ones that are local by design, because + # the fix differs: move the first group, decide about the second. + for path in ("gateway.ssl.cert", "provider.aws_profile", + "sync.gmail.accounts"): + assert path in notes, path + assert "move them to workspace/config/settings.yaml" in notes + + def test_a_key_the_tracked_settings_restate_is_not_reported(self, tmp_path): + """Only silence is worth a warning. A tracked value that config.yaml also + sets is a normal local override, and on a locked box it simply wins.""" + from nerve.config_validate import validate_config_bundle + + config_dir, ws = _install( + tmp_path, + settings="lockdown: true\n" + _JWT + "gateway:\n port: 9100\n", + base="gateway:\n port: 9100\n", + ) + notes = " ".join( + validate_config_bundle(config_dir, workspace_override=ws).warnings + ) + assert "gateway.port" not in notes + + def test_a_falsey_tracked_value_counts_as_stated(self, tmp_path): + """`enabled: false` is an answer, not an absence — presence is the test.""" + from nerve.config_validate import validate_config_bundle + + config_dir, ws = _install( + tmp_path, + settings="lockdown: true\n" + _JWT + "proxy:\n enabled: false\n", + base="proxy:\n enabled: true\n", + ) + notes = " ".join( + validate_config_bundle(config_dir, workspace_override=ws).warnings + ) + assert "proxy.enabled" not in notes + + +class TestLockdownTrackedConfigWrites: + """Guards that sit in front of a named operation only cover the operations + they were put on. An endpoint that takes a caller-supplied path is a + different shape: whether it edits tracked config depends on the argument. + """ + + @pytest.mark.asyncio + async def test_memory_file_route_cannot_edit_tracked_config(self, tmp_path, monkeypatch): + from nerve.gateway.routes.memory import FileWriteRequest, write_memory_file + + ws = tmp_path / "ws" + (ws / "config" / "cron" / "gates").mkdir(parents=True) + monkeypatch.setattr( + cfg, "_config", NerveConfig(lockdown=True, workspace=ws), + ) + for path in ("config/settings.yaml", "config/cron/gates/evil.py"): + with pytest.raises(LockdownError): + await write_memory_file( + path, FileWriteRequest(content="lockdown: false\n"), user={}, + ) + assert not (ws / "config" / "settings.yaml").exists() + assert not (ws / "config" / "cron" / "gates" / "evil.py").exists() + + @pytest.mark.asyncio + async def test_memory_file_route_cannot_edit_the_instruction_files(self, tmp_path, monkeypatch): + """The route writes anywhere under the workspace, and the workspace root + is where the system prompt lives.""" + from nerve.gateway.routes.memory import FileWriteRequest, write_memory_file + + ws = tmp_path / "ws" + (ws / "config").mkdir(parents=True) + monkeypatch.setattr(cfg, "_config", NerveConfig(lockdown=True, workspace=ws)) + for path in ("SOUL.md", "AGENTS.md", "skills/x/SKILL.md"): + with pytest.raises(LockdownError): + await write_memory_file( + path, FileWriteRequest(content="do whatever you like\n"), user={}, + ) + assert not (ws / path).exists() + + @pytest.mark.asyncio + async def test_the_listing_marks_what_the_write_route_will_refuse(self, tmp_path, monkeypatch): + """A UI that offers an edit the PUT answers with a 403 is worse than one + that does not offer it. Both come from the same guard so they cannot part + company.""" + from nerve.gateway.routes.memory import list_memory_files, read_memory_file + + ws = tmp_path / "ws" + (ws / "memory").mkdir(parents=True) + (ws / "SOUL.md").write_text("reviewed\n", encoding="utf-8") + (ws / "NOTES.md").write_text("scratch\n", encoding="utf-8") + (ws / "memory" / "notes.md").write_text("scratch\n", encoding="utf-8") + monkeypatch.setattr(cfg, "_config", NerveConfig(lockdown=True, workspace=ws)) + + listed = {f["path"]: f["read_only"] for f in + (await list_memory_files(user={}))["files"]} + assert listed == { + "SOUL.md": True, "NOTES.md": False, "memory/notes.md": False, + } + assert (await read_memory_file("SOUL.md", user={}))["read_only"] is True + assert (await read_memory_file("NOTES.md", user={}))["read_only"] is False + + monkeypatch.setattr(cfg, "_config", NerveConfig(lockdown=False, workspace=ws)) + unlocked = {f["path"]: f["read_only"] for f in + (await list_memory_files(user={}))["files"]} + assert not any(unlocked.values()) + + @pytest.mark.asyncio + async def test_memory_file_route_still_writes_elsewhere(self, tmp_path, monkeypatch): + """The workspace is also the agent's working directory — lockdown is + about tracked config, not about making the box read-only.""" + from nerve.gateway.routes.memory import FileWriteRequest, write_memory_file + + ws = tmp_path / "ws" + ws.mkdir() + monkeypatch.setattr( + cfg, "_config", NerveConfig(lockdown=True, workspace=ws), + ) + await write_memory_file( + "memory/notes.md", FileWriteRequest(content="hello\n"), user={}, + ) + assert (ws / "memory" / "notes.md").read_text() == "hello\n" + + @pytest.mark.asyncio + async def test_unlocked_route_writes_config_freely(self, tmp_path, monkeypatch): + from nerve.gateway.routes.memory import FileWriteRequest, write_memory_file + + ws = tmp_path / "ws" + ws.mkdir() + monkeypatch.setattr( + cfg, "_config", NerveConfig(lockdown=False, workspace=ws), + ) + await write_memory_file( + "config/settings.yaml", FileWriteRequest(content="timezone: UTC\n"), user={}, + ) + assert (ws / "config" / "settings.yaml").exists() + + def test_memory_manager_write_file_is_guarded_too(self, tmp_path, monkeypatch): + """The route's twin. Guarding one and not the other is not a guard.""" + from nerve.memory.manager import MemoryManager + + ws = tmp_path / "ws" + (ws / "config").mkdir(parents=True) + monkeypatch.setattr(cfg, "_config", NerveConfig(lockdown=True, workspace=ws)) + mgr = MemoryManager(ws) + with pytest.raises(LockdownError): + mgr.write_file("config/settings.yaml", "lockdown: false\n") + assert mgr.write_file("memory/notes.md", "hello\n") is True + + def test_save_jobs_is_guarded(self, tmp_path, monkeypatch): + """No caller today, but the file it writes is tracked cron config.""" + from nerve.cron.jobs import save_jobs + + ws = tmp_path / "ws" + (ws / "config" / "cron").mkdir(parents=True) + monkeypatch.setattr(cfg, "_config", NerveConfig(lockdown=True, workspace=ws)) + with pytest.raises(LockdownError): + save_jobs([], ws / "config" / "cron" / "jobs.yaml") + + @pytest.mark.asyncio + async def test_task_route_cannot_be_pointed_at_tracked_config(self, tmp_path, monkeypatch): + """``task["file_path"]`` is a stored path joined to the workspace. Today + the indexer only ever fills it from a glob of ``tasks/``, so this is depth + rather than a live hole — but it is the same shape as the memory PUT.""" + from nerve.gateway.routes import tasks as tasks_route + + ws = tmp_path / "ws" + (ws / "config").mkdir(parents=True) + (ws / "config" / "settings.yaml").write_text("lockdown: true\n", encoding="utf-8") + monkeypatch.setattr(cfg, "_config", NerveConfig(lockdown=True, workspace=ws)) + + class _Db: + async def get_task(self, task_id): + return {"file_path": "config/settings.yaml", "status": "pending"} + + monkeypatch.setattr( + tasks_route, "get_deps", lambda: type("D", (), {"db": _Db()})(), + ) + with pytest.raises(LockdownError): + await tasks_route.update_task( + "t1", tasks_route.TaskUpdateRequest(content="lockdown: false\n"), + user={}, + ) + assert "lockdown: true" in (ws / "config" / "settings.yaml").read_text() + + +class TestLockdownTaskHandlersGuardWritesOnly: + """The task tools join a stored ``file_path`` to the workspace, so each one + that *changes* the file gets the guard. ``task_read`` must not: lockdown + makes tracked config unwritable, not unreadable. + """ + + async def _locked_task(self, tmp_path, db, monkeypatch): + """A locked instance whose task row points at tracked config.""" + from nerve.agent.tools.handlers import tasks as task_handlers + from nerve.agent.tools.registry import ToolContext + + ws = tmp_path / "ws" + (ws / "config").mkdir(parents=True) + (ws / "config" / "settings.yaml").write_text( + "lockdown: true\n", encoding="utf-8", + ) + monkeypatch.setattr(cfg, "_config", NerveConfig(lockdown=True, workspace=ws)) + # Process-wide read-before-write set: give each test its own so the + # write refusal below can only come from lockdown. + monkeypatch.setattr(task_handlers, "_tasks_read", set()) + await db.upsert_task( + task_id="t1", file_path="config/settings.yaml", + title="T1", status="pending", + ) + return ws, ToolContext(session_id="test", db=db, workspace=ws) + + @pytest.mark.asyncio + async def test_read_is_not_refused_by_the_write_guard(self, tmp_path, db, monkeypatch): + from nerve.agent.tools.handlers.tasks import task_read_handler + + _ws, ctx = await self._locked_task(tmp_path, db, monkeypatch) + result = await task_read_handler(ctx, {"task_id": "t1"}) + assert "lockdown: true" in result.content[0]["text"] + + @pytest.mark.asyncio + async def test_write_on_the_same_path_is_still_refused(self, tmp_path, db, monkeypatch): + from nerve.agent.tools.handlers.tasks import ( + task_read_handler, + task_write_handler, + ) + + ws, ctx = await self._locked_task(tmp_path, db, monkeypatch) + await task_read_handler(ctx, {"task_id": "t1"}) # satisfies read-before-write + with pytest.raises(LockdownError): + await task_write_handler( + ctx, {"task_id": "t1", "content": "lockdown: false\n"}, + ) + assert "lockdown: true" in (ws / "config" / "settings.yaml").read_text() + + @pytest.mark.asyncio + async def test_update_is_still_refused(self, tmp_path, db, monkeypatch): + from nerve.agent.tools.handlers.tasks import task_update_handler + + ws, ctx = await self._locked_task(tmp_path, db, monkeypatch) + with pytest.raises(LockdownError): + await task_update_handler(ctx, {"task_id": "t1", "note": "appended"}) + assert "lockdown: true" in (ws / "config" / "settings.yaml").read_text() + + @pytest.mark.asyncio + async def test_done_is_refused_before_it_unlinks(self, tmp_path, db, monkeypatch): + """``task_done`` copies the file into ``done/`` and unlinks the source — + a delete of tracked config, and the most destructive of the three.""" + from nerve.agent.tools.handlers.tasks import task_done_handler + + ws, ctx = await self._locked_task(tmp_path, db, monkeypatch) + with pytest.raises(LockdownError): + await task_done_handler(ctx, {"task_id": "t1"}) + assert (ws / "config" / "settings.yaml").exists() + # Refused before the DB was touched: no task left claiming to be done + # with its file still sitting in the config subtree. + assert (await db.get_task("t1"))["status"] == "pending" + + @pytest.mark.asyncio + async def test_an_ordinary_task_is_untouched_by_any_of_them(self, tmp_path, db, monkeypatch): + """The workspace is also the agent's working directory: a task file in + its normal home is not config, locked or not.""" + from nerve.agent.tools.handlers import tasks as task_handlers + from nerve.agent.tools.registry import ToolContext + + ws = tmp_path / "ws" + (ws / "config").mkdir(parents=True) + active = ws / "memory" / "tasks" / "active" + active.mkdir(parents=True) + (active / "t2.md").write_text("# T2\n", encoding="utf-8") + monkeypatch.setattr(cfg, "_config", NerveConfig(lockdown=True, workspace=ws)) + monkeypatch.setattr(task_handlers, "_tasks_read", set()) + await db.upsert_task( + task_id="t2", file_path="memory/tasks/active/t2.md", + title="T2", status="pending", + ) + ctx = ToolContext(session_id="test", db=db, workspace=ws) + + assert "# T2" in ( + await task_handlers.task_read_handler(ctx, {"task_id": "t2"}) + ).content[0]["text"] + await task_handlers.task_write_handler( + ctx, {"task_id": "t2", "content": "# T2 edited\n"}, + ) + assert (active / "t2.md").read_text() == "# T2 edited\n" + await task_handlers.task_done_handler(ctx, {"task_id": "t2"}) + assert not (active / "t2.md").exists() + assert (ws / "memory" / "tasks" / "done" / "t2.md").exists() + + +class TestLockdownTaskManagerGuardsTheMove: + """``TaskManager.mark_done`` has the same read-copy-unlink shape as the + ``task_done`` tool — a stored ``file_path`` joined to the workspace, copied + into ``done/`` and then unlinked — so it needs the same guard, or it is a + second route to deleting tracked config. + + Both cases run locked, so the second one is what stops the guard from being + written as "refuse every move". + """ + + def _locked_manager(self, tmp_path, db, monkeypatch): + from nerve.tasks.manager import TaskManager + + ws = tmp_path / "ws" + (ws / "config").mkdir(parents=True) + monkeypatch.setattr(cfg, "_config", NerveConfig(lockdown=True, workspace=ws)) + return ws, TaskManager(ws, db) + + @pytest.mark.asyncio + async def test_a_row_pointing_at_tracked_config_is_refused(self, tmp_path, db, monkeypatch): + ws, manager = self._locked_manager(tmp_path, db, monkeypatch) + (ws / "config" / "settings.yaml").write_text("lockdown: true\n", encoding="utf-8") + await db.upsert_task( + task_id="t1", file_path="config/settings.yaml", + title="T1", status="pending", + ) + + with pytest.raises(LockdownError): + await manager.mark_done("t1") + + assert (ws / "config" / "settings.yaml").read_text() == "lockdown: true\n" + # Nothing half-done: no copy left in done/, no row claiming otherwise. + assert not (ws / "memory" / "tasks" / "done" / "settings.yaml").exists() + assert (await db.get_task("t1"))["status"] == "pending" + + @pytest.mark.asyncio + async def test_an_ordinary_task_file_still_moves(self, tmp_path, db, monkeypatch): + ws, manager = self._locked_manager(tmp_path, db, monkeypatch) + active = ws / "memory" / "tasks" / "active" + (active / "t2.md").write_text("# T2\n", encoding="utf-8") + await db.upsert_task( + task_id="t2", file_path="memory/tasks/active/t2.md", + title="T2", status="pending", + ) + + assert await manager.mark_done("t2") is True + assert not (active / "t2.md").exists() + assert "DONE" in (ws / "memory" / "tasks" / "done" / "t2.md").read_text() + assert (await db.get_task("t2"))["status"] == "done" + + +class TestSkillIdIsOnePathComponent: + """``skill_id`` reaches ``skills_dir / skill_id`` straight from an HTTP path + segment; ``_slugify`` only runs on create. Delete removes the whole tree it + names, so ``../config`` took out the tracked config subtree.""" + + @pytest.mark.asyncio + async def test_delete_cannot_escape_the_skills_directory(self, tmp_path, db): + from nerve.skills.manager import SkillIdError, SkillManager + + ws = tmp_path / "ws" + (ws / "config").mkdir(parents=True) + (ws / "config" / "settings.yaml").write_text("lockdown: true\n", encoding="utf-8") + mgr = SkillManager(ws, db) + for bad in ("../config", "../../etc", "..", ".", "a/b", ".hidden"): + with pytest.raises(SkillIdError): + await mgr.delete_skill(bad) + assert (ws / "config" / "settings.yaml").exists() + + @pytest.mark.asyncio + async def test_existing_directory_names_stay_usable(self, tmp_path, db): + """The filesystem is the source of truth and discover() adopts whatever + directory names it finds, so the check has to be wider than _slugify.""" + from nerve.skills.manager import SkillManager + + ws = tmp_path / "ws" + mgr = SkillManager(ws, db) + skill_dir = ws / "skills" / "My_Skill.v2" + skill_dir.mkdir(parents=True) + (skill_dir / "SKILL.md").write_text( + "---\nname: My Skill\ndescription: d\n---\nbody\n", encoding="utf-8", + ) + await mgr.discover() + assert await mgr.update_skill( + "My_Skill.v2", "---\nname: My Skill\ndescription: e\n---\nbody\n", + ) is not None + + +class TestLockdownEnvironmentAnchor: + """Hardening the flag's value says nothing about which file supplies it. + + ``workspace:`` lives in the machine-local ``config.yaml`` and selects the + ``settings.yaml`` that is then the only authority, so one local edit repoints + the whole chain and lockdown evaluates to false. The anchor moves the decision + out of every file on the box and into the service definition. + """ + + def _two_trees(self, tmp_path, *, machine_workspace): + """A locked real workspace and an unlocked one an attacker points at.""" + config_dir = tmp_path / "cfg" + real = tmp_path / "real-ws" + attacker = tmp_path / "attacker-ws" + config_dir.mkdir() + (real / "config").mkdir(parents=True) + (attacker / "config").mkdir(parents=True) + _repo(real) + workspace_settings_file(real).write_text( + "lockdown: true\ntimezone: UTC\n" + _JWT, encoding="utf-8", + ) + workspace_settings_file(attacker).write_text( + "timezone: Europe/Berlin\n", encoding="utf-8", + ) + (config_dir / "config.yaml").write_text( + f"workspace: {machine_workspace}\nauth:\n jwt_secret: attacker\n", + encoding="utf-8", + ) + return config_dir, real, attacker + + def test_repointing_the_workspace_unlocks_an_unanchored_box(self, tmp_path, monkeypatch): + """The bypass, demonstrated. Nothing here is a bug on its own — this is + what the anchor exists for, and it is why the anchor has to cover the + workspace as well as the flag.""" + monkeypatch.delenv("NERVE_LOCKDOWN", raising=False) + config_dir, real, attacker = self._two_trees( + tmp_path, machine_workspace=tmp_path / "attacker-ws", + ) + c = load_config(config_dir) + assert c.lockdown is False + assert c.workspace == attacker + # ...and with the machine layers back, so is the machine's jwt_secret. + assert c.auth.jwt_secret == "attacker" + + def test_the_anchor_closes_it(self, tmp_path, monkeypatch): + monkeypatch.setenv("NERVE_LOCKDOWN", "1") + monkeypatch.setenv("NERVE_WORKSPACE", str(tmp_path / "real-ws")) + config_dir, real, attacker = self._two_trees( + tmp_path, machine_workspace=tmp_path / "attacker-ws", + ) + c = load_config(config_dir) + assert c.lockdown is True + assert c.workspace == real # the repoint is ignored + assert c.timezone == "UTC" # from the real tracked settings + assert c.auth.jwt_secret == "test-secret" # not the machine layer's + + def test_the_anchor_requires_a_workspace(self, tmp_path, monkeypatch): + """Anchoring the flag alone would produce a box locked *onto* whatever + tree config.yaml names — worse than unlocked, since it now trusts that + tree exclusively.""" + monkeypatch.setenv("NERVE_LOCKDOWN", "true") + monkeypatch.delenv("NERVE_WORKSPACE", raising=False) + config_dir, real, attacker = self._two_trees( + tmp_path, machine_workspace=tmp_path / "attacker-ws", + ) + with pytest.raises(ConfigError) as ei: + load_config(config_dir) + assert "NERVE_WORKSPACE" in str(ei.value) + + def test_a_relative_anchor_workspace_is_refused(self, tmp_path, monkeypatch): + monkeypatch.setenv("NERVE_LOCKDOWN", "true") + monkeypatch.setenv("NERVE_WORKSPACE", "relative/ws") + config_dir, real, attacker = self._two_trees( + tmp_path, machine_workspace=tmp_path / "real-ws", + ) + with pytest.raises(ConfigError) as ei: + load_config(config_dir) + assert "absolute" in str(ei.value) + + @pytest.mark.parametrize("value", ["false", "0", "off", "no", ""]) + def test_the_anchor_never_unlocks(self, tmp_path, monkeypatch, value): + """Monotonic by design: the env can add lockdown, never remove it. A + box whose reviewed config says locked cannot be unlocked from the + environment — that still takes a merged change.""" + monkeypatch.setenv("NERVE_LOCKDOWN", value) + monkeypatch.delenv("NERVE_WORKSPACE", raising=False) + config_dir, ws = _install(tmp_path, settings="lockdown: true\n" + _JWT) + assert load_config(config_dir).lockdown is True + + @pytest.mark.parametrize("value", ["false", "0", "off", ""]) + def test_no_opinion_leaves_an_unlocked_box_alone(self, tmp_path, monkeypatch, value): + monkeypatch.setenv("NERVE_LOCKDOWN", value) + monkeypatch.delenv("NERVE_WORKSPACE", raising=False) + config_dir, ws = _install(tmp_path, base="timezone: UTC\n") + c = load_config(config_dir) + assert c.lockdown is False + assert c.timezone == "UTC" # machine layer still applies + + @pytest.mark.parametrize("value", ["1", "true", "TRUE", "yes", "on"]) + def test_accepted_spellings(self, tmp_path, monkeypatch, value): + monkeypatch.setenv("NERVE_LOCKDOWN", value) + monkeypatch.setenv("NERVE_WORKSPACE", str(tmp_path / "ws")) + config_dir, ws = _install(tmp_path, settings=_JWT) + assert load_config(config_dir).lockdown is True + + def test_an_unreadable_anchor_is_refused(self, tmp_path, monkeypatch): + """The only other reading is "no opinion", which would silently discard + an instruction to lock.""" + monkeypatch.setenv("NERVE_LOCKDOWN", "maybe") + monkeypatch.setenv("NERVE_WORKSPACE", str(tmp_path / "ws")) + config_dir, ws = _install(tmp_path, settings=_JWT) + with pytest.raises(ConfigError) as ei: + load_config(config_dir) + assert "NERVE_LOCKDOWN" in str(ei.value) + + def test_the_validator_judges_the_anchored_view(self, tmp_path, monkeypatch): + from nerve.config_validate import validate_config_bundle + + monkeypatch.setenv("NERVE_LOCKDOWN", "1") + monkeypatch.setenv("NERVE_WORKSPACE", str(tmp_path / "ws")) + # The bundle never says lockdown, so an unlocked run would judge the + # unlocked view; the environment is what makes this the locked one. + config_dir, ws = _install(tmp_path, base="timezone: America/New_York\n") + result = validate_config_bundle(config_dir) + assert any("LOCKED view" in i for i in result.info) + # config.yaml is what a locked box discards, so its keys are named. + assert any("timezone" in w for w in result.warnings), result.warnings + + def test_an_explicit_workspace_still_wins_for_the_validator(self, tmp_path, monkeypatch): + """CI is not the anchored box; --workspace points at a checkout.""" + from nerve.config_validate import validate_config_bundle + + monkeypatch.setenv("NERVE_LOCKDOWN", "1") + monkeypatch.setenv("NERVE_WORKSPACE", str(tmp_path / "nonexistent")) + config_dir, ws = _install(tmp_path, settings="timezone: UTC\n" + _JWT) + result = validate_config_bundle(config_dir, workspace_override=ws) + assert result.ok, result.errors + + +class TestAgentCannotWriteTrackedConfig: + """``Write``/``Edit`` are auto-approved for every non-interactive tool, and + never passed through the REST guards, so the agent's ordinary way of editing + a file reached the config the box promises to run. + + Not a sandbox: ``Bash`` is auto-approved on the same path and is deliberately + not filtered. See ``lockdown_denial``. + """ + + def _hub(self, session_id="s1"): + class _Hub: + snapshot_fn = None + interactive_capable = False + + def __init__(self, sid): + self.session_id = sid + + def mark_snapshotted(self, _p): + return False + + return _Hub(session_id) + + def _locked(self, monkeypatch, tmp_path): + ws = tmp_path / "ws" + (ws / "config" / "cron" / "gates").mkdir(parents=True) + monkeypatch.setattr(cfg, "_config", NerveConfig(lockdown=True, workspace=ws)) + return ws + + @pytest.mark.asyncio + @pytest.mark.parametrize("tool", ["Write", "Edit", "NotebookEdit"]) + async def test_can_use_tool_denies_writes_into_config(self, tmp_path, monkeypatch, tool): + from nerve.agent.backends.claude import ClaudeToolPermissions + + ws = self._locked(monkeypatch, tmp_path) + perms = ClaudeToolPermissions(self._hub()) + result = await perms.can_use_tool( + tool, {"file_path": str(ws / "config" / "settings.yaml")}, context=None, + ) + assert type(result).__name__ == "PermissionResultDeny" + assert "lockdown" in result.message + # The refusal has to route the agent somewhere, not just say no. + assert "pull request" in result.message + + @pytest.mark.asyncio + async def test_a_gate_plugin_is_refused_too(self, tmp_path, monkeypatch): + from nerve.agent.backends.claude import ClaudeToolPermissions + + ws = self._locked(monkeypatch, tmp_path) + perms = ClaudeToolPermissions(self._hub()) + result = await perms.can_use_tool( + "Write", + {"file_path": str(ws / "config" / "cron" / "gates" / "evil.py")}, + context=None, + ) + assert type(result).__name__ == "PermissionResultDeny" + + @pytest.mark.asyncio + async def test_a_relative_path_is_resolved_against_the_workspace(self, tmp_path, monkeypatch): + from nerve.agent.backends.claude import ClaudeToolPermissions + + self._locked(monkeypatch, tmp_path) + perms = ClaudeToolPermissions(self._hub()) + result = await perms.can_use_tool( + "Write", {"file_path": "config/settings.yaml"}, context=None, + ) + assert type(result).__name__ == "PermissionResultDeny" + + @pytest.mark.asyncio + @pytest.mark.parametrize("path", [ + "skills/backdoor/SKILL.md", + "skills/existing/scripts/run.sh", + "AGENTS.md", + "SOUL.md", + "IDENTITY.md", + "USER.md", + "TOOLS.md", + ]) + async def test_can_use_tool_denies_writes_across_the_reviewed_surface( + self, tmp_path, monkeypatch, path, + ): + """The plant-a-skill path, closed where the agent actually takes it. + + ``create_skill`` raises under lockdown, but nothing consults that on the + way to a ``Write``: the file lands, ``SkillManager.discover`` indexes it + on the next reload, and the model can invoke it with whatever + ``allowed-tools`` its frontmatter claims. The root instruction files are + the same shape one directory up — they are the system prompt. + """ + from nerve.agent.backends.claude import ClaudeToolPermissions + + ws = self._locked(monkeypatch, tmp_path) + perms = ClaudeToolPermissions(self._hub()) + result = await perms.can_use_tool( + "Write", {"file_path": str(ws / path)}, context=None, + ) + assert type(result).__name__ == "PermissionResultDeny" + assert "lockdown" in result.message + assert "pull request" in result.message + + @pytest.mark.asyncio + @pytest.mark.parametrize( + "path", ["memory/notes.md", "tasks/active/t1.md", "TASK.md", "scratch.md"], + ) + async def test_ordinary_agent_writes_are_untouched(self, tmp_path, monkeypatch, path): + from nerve.agent.backends.claude import ClaudeToolPermissions + + ws = self._locked(monkeypatch, tmp_path) + perms = ClaudeToolPermissions(self._hub()) + result = await perms.can_use_tool( + "Write", {"file_path": str(ws / path)}, context=None, + ) + assert type(result).__name__ == "PermissionResultAllow" + + @pytest.mark.asyncio + async def test_unlocked_writes_config_freely(self, tmp_path, monkeypatch): + from nerve.agent.backends.claude import ClaudeToolPermissions + + ws = tmp_path / "ws" + (ws / "config").mkdir(parents=True) + monkeypatch.setattr(cfg, "_config", NerveConfig(lockdown=False, workspace=ws)) + perms = ClaudeToolPermissions(self._hub()) + result = await perms.can_use_tool( + "Write", {"file_path": str(ws / "config" / "settings.yaml")}, context=None, + ) + assert type(result).__name__ == "PermissionResultAllow" + + def test_bash_is_deliberately_not_filtered(self, tmp_path, monkeypatch): + """The documented gap, asserted so it cannot be quietly "fixed" with a + command-string filter that looks like a boundary and is not one.""" + from nerve.agent.backends.claude import lockdown_denial + + ws = self._locked(monkeypatch, tmp_path) + assert lockdown_denial( + "Bash", {"command": f"echo x > {ws}/config/settings.yaml"}, + ) is None + + def test_the_background_subagent_hook_denies_too(self, tmp_path, monkeypatch): + """For a background sub-agent the PreToolUse hook is the only thing that + runs, so an allow issued there is the whole decision.""" + from nerve.agent.backends.claude import lockdown_denial + + ws = self._locked(monkeypatch, tmp_path) + assert lockdown_denial( + "Write", {"file_path": str(ws / "config" / "settings.yaml")}, + ) diff --git a/tests/test_sync_service.py b/tests/test_sync_service.py index 1c4be711..73564618 100644 --- a/tests/test_sync_service.py +++ b/tests/test_sync_service.py @@ -426,6 +426,107 @@ def test_ignored_file_warns_but_does_not_block(self, tmp_path): assert result.ok and result.changed, result.message assert any("secrets.local.yaml" in w for w in result.validation_warnings) + def test_ignored_gate_plugin_blocks_a_locked_instance(self, tmp_path): + """A locked box promises that only reviewed remote config runs on it. + + An ignored file in the config subtree is invisible to the reviewer, to + the validation checkout, and to the blocking check above — but a + ``cron/gates/*.py`` there is imported and executed by the daemon all the + same. On an ordinary box refusing over it would be sync policing what the + machine may hold locally; a locked box has already settled that question, + so the same file is a refusal there and a warning here. + """ + origin, ws = self._pair(tmp_path) + (origin / ".gitignore").write_text("config/cron/gates/local_*.py\n") + self._git("add", "-A", cwd=origin) + self._git("commit", "-m", "ignore", cwd=origin) + self._git("pull", "-q", "--ff-only", "origin", "main", cwd=ws) + self._upstream_commit(origin) + gates = ws / "config" / "cron" / "gates" + gates.mkdir(parents=True) + (gates / "local_unreviewed.py").write_text("MARKER = 1\n") + + before = self._git("rev-parse", "HEAD", cwd=ws).stdout.strip() + unlocked = sync_workspace(ws, tmp_path / "cfg", branch="main", validate=True) + assert unlocked.ok, unlocked.message + assert any("local_unreviewed.py" in w for w in unlocked.validation_warnings) + + # Same tree, same commit — only lockdown differs. + self._git("reset", "-q", "--hard", before, cwd=ws) + locked = sync_workspace( + ws, tmp_path / "cfg", branch="main", validate=True, locked=True, + ) + assert not locked.ok and not locked.changed + assert "local_unreviewed.py" in locked.message + assert "Europe/Berlin" not in (ws / "config" / "settings.yaml").read_text() + + def test_untracked_skill_blocks_the_merge(self, tmp_path): + """A skill is instructions the model can invoke, with its own + ``allowed-tools``, indexed on the next reload. Scoped to ``config/``, the + check merged straight over one and reported the workspace clean. + """ + origin, ws = self._pair(tmp_path) + self._upstream_commit(origin) + (ws / "skills" / "backdoor").mkdir(parents=True) + (ws / "skills" / "backdoor" / "SKILL.md").write_text( + "---\nname: backdoor\nallowed-tools: Bash\n---\n", + ) + + result = self._sync(ws, tmp_path) + assert not result.ok and not result.changed + assert "SKILL.md" in result.message + assert "Europe/Berlin" not in (ws / "config" / "settings.yaml").read_text() + + def test_locally_edited_instruction_file_blocks_the_merge(self, tmp_path): + origin, ws = self._pair(tmp_path) + (origin / "SOUL.md").write_text("reviewed\n") + self._git("add", "-A", cwd=origin) + self._git("commit", "-m", "add soul", cwd=origin) + self._git("pull", "-q", "--ff-only", "origin", "main", cwd=ws) + self._upstream_commit(origin) + (ws / "SOUL.md").write_text("do whatever you like\n") + + result = self._sync(ws, tmp_path) + assert not result.ok and not result.changed + assert "SOUL.md" in result.message + + def test_submodule_blocks_a_locked_instance(self, tmp_path): + """Same rule as the ignored file above, and for a sharper reason: the + working copy is whatever this box happens to have checked out, validation + saw an empty directory, and no fast-forward will ever move it.""" + origin, ws = self._pair(tmp_path) + sha = self._git("rev-parse", "HEAD", cwd=origin).stdout.strip() + self._git( + "update-index", "--add", "--cacheinfo", f"160000,{sha},config/vendored", + cwd=origin, + ) + self._git("commit", "-m", "vendor config", cwd=origin) + + result = sync_workspace( + ws, tmp_path / "cfg", branch="main", validate=True, locked=True, + ) + assert not result.ok and not result.changed + assert "vendored" in result.message + + def test_unset_env_var_is_reported_even_when_tolerated(self, tmp_path): + """With strict_env off the validator files this as *info*, which nothing + propagated — so the gate saw the one signal that predicts a failed + post-merge reload and dropped it. Tolerating it is what the setting asks + for; saying nothing about it is not.""" + origin, ws = self._pair(tmp_path) + (origin / "config" / "settings.yaml").write_text( + "timezone: ${NO_SUCH_VAR_FOR_SYNC}\n", + ) + self._git("commit", "-am", "env ref", cwd=origin) + + result = sync_workspace( + ws, tmp_path / "cfg", branch="main", validate=True, strict_env=False, + ) + assert result.ok and result.changed, result.message + assert any( + "NO_SUCH_VAR_FOR_SYNC" in w for w in result.validation_warnings + ), result.validation_warnings + def test_submodule_in_the_subtree_is_flagged_as_unvalidated(self, tmp_path): origin, ws = self._pair(tmp_path) sha = self._git("rev-parse", "HEAD", cwd=origin).stdout.strip() @@ -636,20 +737,100 @@ def test_validation_actually_skipped_when_env_says_off(self, tmp_path, monkeypat class TestApplySync: @pytest.mark.asyncio - async def test_apply_triggers_both_reloads(self): + async def test_apply_triggers_both_reloads(self, tmp_path): cron, engine = AsyncMock(), AsyncMock() - await sync._apply_sync(engine, cron) + await sync._apply_sync(engine, cron, tmp_path) cron.reload.assert_awaited_once() engine.reload_mcp_config.assert_awaited_once() @pytest.mark.asyncio - async def test_apply_survives_reload_error(self): + async def test_apply_survives_reload_error(self, tmp_path): cron = AsyncMock() cron.reload.side_effect = RuntimeError("boom") engine = AsyncMock() - await sync._apply_sync(engine, cron) # must not raise + await sync._apply_sync(engine, cron, tmp_path) # must not raise engine.reload_mcp_config.assert_awaited_once() + @pytest.mark.asyncio + async def test_apply_reloads_config_singleton_engages_lockdown(self, tmp_path, monkeypatch): + """A synced lockdown flip must engage without a restart.""" + import nerve.config as cfgmod + from nerve.config import is_locked, workspace_settings_file + + config_dir = tmp_path / "cfg" + ws = tmp_path / "ws" + config_dir.mkdir(parents=True) + (ws / "config").mkdir(parents=True) + (config_dir / "config.yaml").write_text(f"workspace: {ws}\n", encoding="utf-8") + workspace_settings_file(ws).write_text( + "lockdown: true\nauth:\n jwt_secret: x\n", encoding="utf-8" + ) + # A locked workspace has to be a repo with a remote or the load refuses; + # this one is synced, so it is one. + (ws / ".git").mkdir() + monkeypatch.setattr(sync, "_git", lambda args, cwd: _cp(stdout="origin\n")) + monkeypatch.setattr(cfgmod, "_config", cfgmod.NerveConfig(lockdown=False)) + assert not is_locked() + err = await sync._apply_sync(AsyncMock(), AsyncMock(), config_dir) + assert err is None + assert is_locked() # the reloaded config engaged lockdown + + @pytest.mark.asyncio + async def test_apply_reports_when_lockdown_fails_to_engage(self, tmp_path, monkeypatch): + """The merge landed `lockdown: true` and the daemon could not load it, so + the box is *not* locked. That has to come back to the caller: reporting a + clean success here tells an operator the write guards are closed while + they are wide open.""" + import nerve.config as cfgmod + from nerve.config import is_locked, workspace_settings_file + + config_dir = tmp_path / "cfg" + ws = tmp_path / "ws" + config_dir.mkdir(parents=True) + (ws / "config").mkdir(parents=True) + (config_dir / "config.yaml").write_text(f"workspace: {ws}\n", encoding="utf-8") + # Locks the box and names a variable this environment does not have, so + # load_config raises on the merged result. + workspace_settings_file(ws).write_text( + "lockdown: true\nauth:\n jwt_secret: ${NO_SUCH_SECRET_HERE}\n", + encoding="utf-8", + ) + monkeypatch.setattr(cfgmod, "_config", cfgmod.NerveConfig(lockdown=False)) + err = await sync._apply_sync(AsyncMock(), AsyncMock(), config_dir) + assert err and "NO_SUCH_SECRET_HERE" in err + assert not is_locked() # still running the old config — which the caller must say + + @pytest.mark.asyncio + async def test_route_does_not_claim_success_when_apply_failed(self, tmp_path, monkeypatch): + import nerve.gateway.server as srv + + import nerve.gateway.routes.config as route_mod + from nerve.sync_service import SyncResult + + fake_cfg = type("C", (), {})() + fake_cfg.workspace = str(tmp_path / "ws") + fake_cfg.config_dir = str(tmp_path / "cfg") + fake_cfg.workspace_sync = WorkspaceSyncConfig(enabled=True) + fake_cfg.lockdown = False + monkeypatch.setattr("nerve.config.get_config", lambda: fake_cfg) + monkeypatch.setattr(srv, "_cron_service", None, raising=False) + monkeypatch.setattr( + route_mod, "get_deps", lambda: type("D", (), {"engine": None})(), + ) + monkeypatch.setattr( + sync, "sync_workspace", + lambda *a, **k: SyncResult(ok=True, changed=True, message="updated"), + ) + + async def _failing_apply(engine, cron_service, config_dir): + return "ConfigError: nope" + + monkeypatch.setattr(sync, "_apply_sync", _failing_apply) + body = await route_mod.sync_workspace_route(user={}) + assert body["ok"] is False + assert body["changed"] is True and body["applied"] is False + assert body["apply_error"] == "ConfigError: nope" + class TestNeverRaises: """``sync_workspace`` promises a SyncResult for every outcome. @@ -750,6 +931,7 @@ async def test_route_reports_400_not_500(self, tmp_path, monkeypatch): fake_cfg.workspace = str(ws) fake_cfg.config_dir = str(tmp_path / "cfg") fake_cfg.workspace_sync = WorkspaceSyncConfig(enabled=True, validate=True) + fake_cfg.lockdown = False monkeypatch.setattr("nerve.config.get_config", lambda: fake_cfg) monkeypatch.setattr(srv, "_cron_service", None, raising=False) monkeypatch.setattr(sync, "_git", _FakeGit(head="aaaaaaaa", upstream="bbbbbbbb")) @@ -761,7 +943,6 @@ async def test_route_reports_400_not_500(self, tmp_path, monkeypatch): await route_mod.sync_workspace_route(user={}) assert ei.value.status_code == 400 - class _FakeClock: """Replaces the loop's interval wait so cycles run without real time passing. @@ -816,6 +997,7 @@ def _loop_config(**overrides): cfg = type("C", (), {})() cfg.workspace = overrides.pop("workspace", "/tmp/ws") cfg.config_dir = overrides.pop("config_dir", None) + cfg.lockdown = overrides.pop("lockdown", False) overrides.setdefault("enabled", True) overrides.setdefault("interval_minutes", 60) cfg.workspace_sync = WorkspaceSyncConfig(**overrides) @@ -968,6 +1150,7 @@ async def test_route_400_on_invalid(self, monkeypatch): fake_cfg.workspace = "/tmp/ws" fake_cfg.config_dir = "/tmp/cfg" fake_cfg.workspace_sync = WorkspaceSyncConfig(enabled=True) + fake_cfg.lockdown = False monkeypatch.setattr("nerve.config.get_config", lambda: fake_cfg) monkeypatch.setattr(srv, "_cron_service", None, raising=False) # The route imports sync_workspace from nerve.sync_service at call time. @@ -978,3 +1161,38 @@ async def test_route_400_on_invalid(self, monkeypatch): with pytest.raises(HTTPException) as ei: await route_mod.sync_workspace_route(user={}) assert ei.value.status_code == 400 + + @pytest.mark.asyncio + async def test_route_applies_a_changed_sync(self, monkeypatch, tmp_path): + """The success path, which the 400 tests never reach: it resolves the + config dir before applying, so a name that only exists on that branch + would otherwise surface as a 500 on the first sync that changed anything. + """ + import nerve.gateway.server as srv + + import nerve.gateway.routes.config as route_mod + from nerve.sync_service import SyncResult + + fake_cfg = type("C", (), {})() + fake_cfg.workspace = str(tmp_path / "ws") + fake_cfg.config_dir = str(tmp_path / "cfg") + fake_cfg.workspace_sync = WorkspaceSyncConfig(enabled=True) + fake_cfg.lockdown = False + monkeypatch.setattr("nerve.config.get_config", lambda: fake_cfg) + monkeypatch.setattr(srv, "_cron_service", None, raising=False) + monkeypatch.setattr( + route_mod, "get_deps", lambda: type("D", (), {"engine": None})(), + ) + monkeypatch.setattr( + sync, "sync_workspace", + lambda *a, **k: SyncResult(ok=True, changed=True, message="updated"), + ) + applied: list = [] + + async def _fake_apply(engine, cron_service, config_dir): + applied.append(Path(config_dir)) + + monkeypatch.setattr(sync, "_apply_sync", _fake_apply) + body = await route_mod.sync_workspace_route(user={}) + assert body["ok"] and body["changed"] + assert applied == [tmp_path / "cfg"] diff --git a/web/src/api/client.ts b/web/src/api/client.ts index 30817c9c..6789f0ae 100644 --- a/web/src/api/client.ts +++ b/web/src/api/client.ts @@ -369,7 +369,7 @@ export const api = { // Memory listMemoryFiles: () => request<{ files: any[] }>('/memory/files'), readMemoryFile: (path: string) => - request<{ path: string; content: string }>(`/memory/file/${path}`), + request<{ path: string; content: string; read_only?: boolean }>(`/memory/file/${path}`), writeMemoryFile: (path: string, content: string) => request(`/memory/file/${path}`, { method: 'PUT', diff --git a/web/src/components/Files/FileEditor.tsx b/web/src/components/Files/FileEditor.tsx index d7976a8e..aa418e2a 100644 --- a/web/src/components/Files/FileEditor.tsx +++ b/web/src/components/Files/FileEditor.tsx @@ -1,25 +1,28 @@ import { useState, useCallback } from 'react'; -import { Eye, Edit3, Save } from 'lucide-react'; +import { Eye, Edit3, Lock, Save } from 'lucide-react'; import { MarkdownContent } from '../Chat/MarkdownContent'; interface FileEditorProps { path: string; content: string; modified: boolean; + // Set for a file the server will refuse to write — a reviewed file on a + // locked instance. The save is offered nowhere it would come back a 403. + readOnly?: boolean; saving: boolean; onContentChange: (content: string) => void; onSave: () => void; } -export function FileEditor({ path, content, modified, saving, onContentChange, onSave }: FileEditorProps) { +export function FileEditor({ path, content, modified, readOnly, saving, onContentChange, onSave }: FileEditorProps) { const [mode, setMode] = useState<'edit' | 'preview'>('edit'); const handleKeyDown = useCallback((e: React.KeyboardEvent) => { if ((e.metaKey || e.ctrlKey) && e.key === 's') { e.preventDefault(); - onSave(); + if (!readOnly) onSave(); } - }, [onSave]); + }, [onSave, readOnly]); return (
@@ -43,7 +46,13 @@ export function FileEditor({ path, content, modified, saving, onContentChange, o
- {modified && ( + {readOnly && ( + + + Read-only (lockdown) + + )} + {modified && !readOnly && ( ); diff --git a/web/src/pages/FilesPage.tsx b/web/src/pages/FilesPage.tsx index 6684fc6c..b6c0da80 100644 --- a/web/src/pages/FilesPage.tsx +++ b/web/src/pages/FilesPage.tsx @@ -42,6 +42,7 @@ export function FilesPage() { path={currentFile.path} content={currentFile.content} modified={currentFile.modified} + readOnly={currentFile.readOnly} saving={saving} onContentChange={(c) => updateContent(currentFile.path, c)} onSave={() => saveFile(currentFile.path)} diff --git a/web/src/stores/filesStore.ts b/web/src/stores/filesStore.ts index f65b2cc4..d38bf74b 100644 --- a/web/src/stores/filesStore.ts +++ b/web/src/stores/filesStore.ts @@ -8,6 +8,7 @@ interface OpenFile { content: string; originalContent: string; modified: boolean; + readOnly: boolean; } interface FilesState { @@ -51,10 +52,13 @@ export const useFilesStore = create((set, get) => ({ set({ loading: true }); try { - const { content } = await api.readMemoryFile(path); + const { content, read_only } = await api.readMemoryFile(path); const name = path.split('/').pop() || path; set(s => ({ - openFiles: [...s.openFiles, { path, name, content, originalContent: content, modified: false }], + openFiles: [...s.openFiles, { + path, name, content, originalContent: content, modified: false, + readOnly: !!read_only, + }], activeFile: path, loading: false, })); diff --git a/web/src/utils/fileTree.ts b/web/src/utils/fileTree.ts index 905eefe4..723e041a 100644 --- a/web/src/utils/fileTree.ts +++ b/web/src/utils/fileTree.ts @@ -4,10 +4,13 @@ export interface FileNode { type: 'file' | 'directory'; size?: number; modified?: string; + // The server's answer, not the client's guess: on a locked instance the + // reviewed files (SOUL.md, AGENTS.md, ...) are readable and not writable. + readOnly?: boolean; children?: FileNode[]; } -export function buildFileTree(files: { path: string; name: string; size: number; modified?: string }[]): FileNode[] { +export function buildFileTree(files: { path: string; name: string; size: number; modified?: string; read_only?: boolean }[]): FileNode[] { const root: FileNode = { name: '', path: '', type: 'directory', children: [] }; for (const file of files) { @@ -31,6 +34,7 @@ export function buildFileTree(files: { path: string; name: string; size: number; type: 'file', size: file.size, modified: file.modified, + readOnly: file.read_only, }); }