diff --git a/AGENTS.md b/AGENTS.md index 157060f..66dfca6 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -118,14 +118,15 @@ the CLI's query + view layer, so they return the same JSON view models. URL/toke come from the active profile (same `--profile` / `--config` flags). JSON-RPC is on stdout; logs go to stderr. Read tools are annotated `read_only_hint = true`. -**Write tools (Pattern 2, opt-in):** `nbox_plan_write` and `nbox_apply_write` are -available when `nbox serve --http --allow-writes` is set and a `[serve.vault]` -config maps each caller's OIDC `sub` to a per-user NetBox token (env var). They -reuse the CLI's ADR-0001 safe-write engine: plan-first, confirm-token-gated, -per-user identity. Writes require the HTTP transport (OIDC identity); stdio -stays read-only. `nbox serve --print-config` prints the -paste-ready `mcpServers` JSON (absolute binary path, echoed `--profile`/`--config`, -placeholder token) and exits — no server start, no NetBox connection. +**Write tools (opt-in):** `nbox_plan_write` and `nbox_apply_write` reuse the +CLI's ADR-0001 safe-write engine: plan-first, confirm-token-gated, and backed by +the server-issued plan store. Local stdio writes require `nbox serve +--local-writes` / `[serve].local_writes` and use the active profile token. Shared +HTTP/OIDC writes require `nbox serve --http --allow-writes` plus a +`[serve.vault]` mapping each caller's OIDC `sub` to a per-user NetBox token (env +var) and the caller's `nbox:write` scope. `nbox serve --print-config` prints the +paste-ready `mcpServers` JSON (absolute binary path, echoed `--profile`/`--config`/ +`--local-writes`, placeholder token) and exits — no server start, no NetBox connection. | Tool | Purpose | | ---- | ------- | @@ -140,8 +141,8 @@ placeholder token) and exits — no server start, no NetBox connection. | `nbox_list_tags` | List tags (name, slug, color, usage count) — valid `tag` values for `nbox_search`. | | `nbox_tagged` | Objects carrying a tag, across kinds (NetBox 4.3+); `tag` (id\|name\|slug). Cross-kind reverse lookup — unlike `nbox_search --tag`, which narrows a free-text search per-endpoint. | | `nbox_cache_clear` | Drop nbox's local read cache so the next lookups fetch fresh from NetBox (read-only w.r.t. NetBox; idempotent). | -| `nbox_plan_write` | Plan a write operation (interface description, device status, IP/prefix/IP-range reserve, tag add/remove). Builds a `MutationPlan` with a before/after diff and confirm token, without mutating. Review the plan, then call `nbox_apply_write`. Requires `--allow-writes` + `[serve.vault]` per-user credential mapping. | -| `nbox_apply_write` | Apply a previously planned write. Pass the full `MutationPlan` JSON from `nbox_plan_write`. The confirm token is verified, then the write executes under the caller's per-user NetBox identity. Returns a `MutationReceipt`. Requires `--allow-writes`. | +| `nbox_plan_write` | Plan a write operation (interface description, device status, IP/prefix/IP-range reserve, tag add/remove). Builds a `MutationPlan` with a before/after diff and confirm token, without mutating. Review the plan, then call `nbox_apply_write`. Requires local stdio `--local-writes`, or shared HTTP/OIDC `--allow-writes` + `[serve.vault]` per-user credential mapping. | +| `nbox_apply_write` | Apply a previously planned write. Pass the full `MutationPlan` JSON from `nbox_plan_write`. The confirm token looks up the server-stored plan; forged, edited, or replayed plans are rejected. Returns a `MutationReceipt`. Same gating as `nbox_plan_write`. | The same objects are also exposed as MCP **resources** via one template, `nbox://{kind}/{ref}` (e.g. `nbox://device/edge01`, `nbox://ip/10.0.0.1`), for @@ -238,7 +239,7 @@ nbox device edge01 --json | jq '.primary_ip4' `--count N` to reserve N IPs atomically in one call (a single list-body POST — NetBox allocates all N or zero; `count` is bound into the confirmation token); any failure leaves nothing created and exits 1, and the receipt's `object` is a - JSON array of the N created IPs. The MCP server is read-only by default; write tools (`nbox_plan_write`/`nbox_apply_write`) require `--allow-writes` + the per-user vault (Pattern 2). + JSON array of the N created IPs. The MCP server is read-only by default; write tools (`nbox_plan_write`/`nbox_apply_write`) require either local stdio `--local-writes` or shared HTTP/OIDC `--allow-writes` + the per-user vault. - Filters that an object type can't satisfy cause that type to be skipped in `search` (nbox does not send NetBox unknown query params). - `owner` (NetBox 4.5+): a native owner field (user or group) surfaced on most diff --git a/CHANGELOG.md b/CHANGELOG.md index cacab71..a406b8a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,14 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Added +- **Local stdio MCP writes (ADR-0002).** `nbox serve --local-writes` and + `[serve].local_writes = true` enable the existing safe MCP write tools + (`nbox_plan_write` / `nbox_apply_write`) for the common local subprocess setup, + using the active profile token and the MCP host's approval prompt as the human + gate. The server-issued plan store remains authoritative: forged, edited, or + replayed plans are rejected. Shared/network HTTP writes remain on the existing + `--allow-writes` + OIDC + `[serve.vault]` path, and HTTP profile-token writes + are intentionally deferred. - **Structured exports, two more on the same surface.** Alongside `nbox export prometheus-sd`, the export surface now covers the `netbox-lists` niche as one fast binary: diff --git a/README.md b/README.md index 09eeaf1..57ced17 100644 --- a/README.md +++ b/README.md @@ -59,7 +59,7 @@ See [Installation](#installation) below for setup instructions. - **IPAM-aware** — IP → most-specific parent prefix → VLAN → scope resolution, prefix utilization and children, a navigable prefix tree, and `next-ip` / `next-prefix` to preview free addresses and blocks (read-only). To actually claim one, `nbox ip reserve ` allocates the next available IP from a prefix, `nbox prefix reserve ` allocates the next available child prefix, and `nbox ip-range reserve ` allocates the next available IP from an IP range (safe writes — ADR-0001). - **A k9s-style TUI** — a three-pane home (navigation rail → results → live preview), an overview dashboard, a hierarchical prefix tree, cross-object navigation between related objects (every detail's related-object tabs are navigable — open an interface from a device and see its cable path drawn as an A↔Z diagram), fuzzy filters, server-side name filtering on the browse list, recents, and an in-app profile + settings editor. Twelve themes; `NO_COLOR` honored. -- **Agent-ready** — `nbox serve` is a read-first MCP server (read-only by default): the same lookups exposed as eleven read tools (plus every object as an `nbox://{kind}/{ref}` resource), returning the exact JSON view models the CLI does, so AI agents (Claude Code, Claude Desktop, …) query NetBox safely. Two opt-in write tools (`nbox_plan_write` / `nbox_apply_write`) are exposed only behind `--allow-writes` + a per-user OIDC vault. Stdio for a local subprocess, or a loopback HTTP transport with OIDC resource-server auth for a network-reachable deployment. Being one static binary with zero runtime, it cold-starts MCP-ready in ~9 ms — before a Python NetBox MCP server finishes importing its dependencies ([benchmarks](docs/BENCHMARKS.md)). See [docs/MCP.md](docs/MCP.md); [SKILL.md](SKILL.md) is an installable Agent Skill that drives the CLI on matching requests. +- **Agent-ready** — `nbox serve` is a read-first MCP server (read-only by default): the same lookups exposed as eleven read tools (plus every object as an `nbox://{kind}/{ref}` resource), returning the exact JSON view models the CLI does, so AI agents (Claude Code, Claude Desktop, …) query NetBox safely. Two opt-in write tools (`nbox_plan_write` / `nbox_apply_write`) support local stdio writes with `--local-writes` (profile token) or shared HTTP/OIDC writes with `--allow-writes` + a per-user vault. Being one static binary with zero runtime, it cold-starts MCP-ready in ~9 ms — before a Python NetBox MCP server finishes importing its dependencies ([benchmarks](docs/BENCHMARKS.md)). See [docs/MCP.md](docs/MCP.md); [SKILL.md](SKILL.md) is an installable Agent Skill that drives the CLI on matching requests. - **Scriptable** — `-o plain|json|csv`, `--fields`, `--raw`, versioned `--envelope`, and stable exit codes; stdout stays clean for piping, logs go to stderr. Plus structured exports — `nbox export prometheus-sd | address-list | device-inventory` emit fixed shapes for Prometheus SD, firewall/blocklist address lists, and device inventories. See [docs/SCRIPTING.md](docs/SCRIPTING.md). - **Fast on repeat** — a small in-memory read cache (per profile, ~30s) keeps TUI back-navigation and chatty agents from re-hitting NetBox; the detail footer shows "cached Ns ago" and `nbox_cache_clear` busts it. - **Multi-instance** — profiles for several NetBox instances (switch live in the TUI), journals folded into detail lookups, open-in-browser and copy, and tag listing. @@ -129,8 +129,9 @@ claude mcp add nbox -e NBOX_TOKEN=nbt_xxx.yyy -- nbox serve ``` `nbox serve` is a read-first MCP server (read-only by default; writes are opt-in -behind `--allow-writes` + a per-user OIDC vault); an MCP host launches it as a -subprocess and gets the same JSON view models the CLI returns. See [docs/MCP.md](docs/MCP.md). +with local stdio `--local-writes` or shared HTTP/OIDC `--allow-writes` + a +per-user vault); an MCP host launches it as a subprocess and gets the same JSON +view models the CLI returns. See [docs/MCP.md](docs/MCP.md). ## Installation @@ -309,7 +310,8 @@ nbox export # structured read-only exports (Prometheus SD JSON, firewall/blocklist # address list, device inventory) — fixed shapes for scripting nbox serve [--http ] # read-first MCP server for AI agents (read-only by default; - # --allow-writes enables write tools over OIDC HTTP); stdio, or loopback/OIDC HTTP + # --local-writes enables local stdio writes; + # --allow-writes enables shared HTTP/OIDC writes) # --print-config prints the paste-ready mcpServers JSON and exits nbox config # token: status reports the resolved source (never echoed) nbox profile @@ -510,13 +512,13 @@ flags. JSON-RPC goes to stdout; all logging stays on stderr. The eleven read tools below are annotated read-only (`read_only_hint = true`). Two write tools — `nbox_plan_write` / `nbox_apply_write` — are **always registered** (so -they show up in `tools/list`), but execution is gated: a call is rejected unless -writes are enabled (`--allow-writes` / `[serve].allow_writes`) AND the caller is an -OIDC-authenticated identity carrying the `nbox:write` scope with a `[serve.vault]` -entry mapping its `sub` to a per-user NetBox token. The write then runs as that user -(never over stdio; without writes enabled the tools reject with "writes disabled"). -`nbox_apply_write` applies the plan the server stored at plan time — looked up by the -`confirm_token` you pass back — not the plan JSON itself, so a forged plan is rejected. +they show up in `tools/list`), but execution is gated. Local stdio writes require +`--local-writes` / `[serve].local_writes` and run under the active profile token. +Shared HTTP/OIDC writes require `--allow-writes` / `[serve].allow_writes`, an +OIDC-authenticated caller carrying `nbox:write`, and a `[serve.vault]` entry mapping +the caller's `sub` to a per-user NetBox token. `nbox_apply_write` applies the plan +the server stored at plan time — looked up by the `confirm_token` you pass back — +not the plan JSON itself, so a forged plan is rejected. | Tool | What | |------|------| @@ -557,7 +559,7 @@ claude mcp add nbox -- nbox serve Or skip the hand-editing — `nbox serve --print-config` prints the ready-to-paste `mcpServers` object (with the absolute binary path and any `--profile`/`--config` -you passed echoed into `args`); see [docs/MCP.md](docs/MCP.md) for the exact +or `--local-writes` you passed echoed into `args`); see [docs/MCP.md](docs/MCP.md) for the exact config-file path per host (Claude Code, Claude Desktop, Cursor). Both reuse the same `config.toml` / `NBOX_TOKEN` as the CLI. Prefer driving the CLI @@ -592,11 +594,12 @@ single profile token, so scope that token read-only. An audit log (`nbox::audit`) and an optional per-caller rate limit (`--rate-limit`) round it out. Full setup, security model, and IdP notes: [docs/MCP.md](docs/MCP.md). -Per-user NetBox identity bridging (so NetBox sees the real caller) and the safe -MCP write tools have shipped: a `[serve.vault]` mapping each caller's OIDC `sub` -to a per-user NetBox token (via `token_env`), and `nbox_plan_write` / -`nbox_apply_write` behind `--allow-writes`. Only a raw escape-hatch MCP tool comes -later. +For local single-user MCP writes, use stdio plus `--local-writes`; the write tools +reuse the profile token and the MCP host's tool-approval prompt is the human gate. +For shared/network writes where NetBox must see the real caller, use +`--allow-writes` plus `[serve.vault]` to map each caller's OIDC `sub` to a per-user +NetBox token (via `token_env`). HTTP profile-token writes are intentionally +deferred; only a raw escape-hatch MCP tool comes later. ## NetBox Compatibility diff --git a/ROADMAP.md b/ROADMAP.md index 45e2ca8..e719a1a 100644 --- a/ROADMAP.md +++ b/ROADMAP.md @@ -499,7 +499,12 @@ the read tool proves out in practice. Consolidated future scope: - ☑ **Write-capable MCP tools + per-user credential vault (Pattern 2)** — `nbox_plan_write` and `nbox_apply_write` tools reuse the CLI's ADR-0001 safe-write engine; the `[serve.vault]` config maps OIDC `sub` → per-user NetBox token (env var), bridged at request time via `NetBoxClient::with_token`. - Writes require `--allow-writes` + HTTP transport (OIDC identity); stdio stays read-only. + Shared HTTP writes require `--allow-writes`, OIDC identity, `nbox:write`, and a vault entry. +- ☑ **Local stdio MCP writes (ADR-0002)** — `nbox serve --local-writes` / + `[serve].local_writes = true` enables the same `nbox_plan_write` / + `nbox_apply_write` tools for the local subprocess case, using the active + profile token and binding plans to a synthetic local actor. HTTP local writes + remain deferred until a separate guard decision. - ☐ TUI edit mode (`e` / `d` / confirm). - ☐ `nbox raw POST|PATCH|DELETE`; OPTIONS write-capability discovery (optional `schema` command; would also enable value-level filter validation beyond today's typed allowlist, netbox#6489). diff --git a/docs/ARCHITECTURE.md b/docs/ARCHITECTURE.md index 3bd9ce8..c123c13 100644 --- a/docs/ARCHITECTURE.md +++ b/docs/ARCHITECTURE.md @@ -49,9 +49,9 @@ behavior should get an ADR before it becomes a public contract. - **`mcp/`** — the MCP server (`nbox serve`, read-only by default): stdio plus a loopback HTTP transport (OIDC resource-server auth, audit log, per-caller rate limit), exposing the same query + view layer as thirteen tools (eleven read - tools plus the `nbox_plan_write`/`nbox_apply_write` write pair, the latter - enabled only with `--allow-writes`/`[serve].allow_writes` + a per-user vault - and never over stdio), `nbox://{kind}/{ref}` resources, and a prompts catalog. + tools plus the `nbox_plan_write`/`nbox_apply_write` write pair, enabled only by + local stdio `--local-writes` or shared HTTP/OIDC `--allow-writes` plus a + per-user vault), `nbox://{kind}/{ref}` resources, and a prompts catalog. ## Data flow diff --git a/docs/COMPARISON.md b/docs/COMPARISON.md index ddc7e1c..4bcc694 100644 --- a/docs/COMPARISON.md +++ b/docs/COMPARISON.md @@ -61,8 +61,8 @@ Notes: - Feeding an AI agent — `nbox serve` is read-only by default (11 read tools, stdio or loopback HTTP with OIDC) returning the same JSON view models the CLI does, and can expose 2 write tools (`nbox_plan_write` / `nbox_apply_write`) - when `[serve].allow_writes` is set over the authenticated HTTP/OIDC transport; - stdio and unauthenticated transports stay read-only. + with local stdio `--local-writes` or shared HTTP/OIDC `[serve].allow_writes` + plus a per-user vault; unauthenticated HTTP stays read-only. ### The NetBox web UI diff --git a/docs/CONFIG.md b/docs/CONFIG.md index f7a7618..fe7d5bc 100644 --- a/docs/CONFIG.md +++ b/docs/CONFIG.md @@ -230,6 +230,8 @@ Absent ⇒ stdio (no HTTP). The matching CLI flags always override these keys. | `jwks_url` | JWKS URL override; absent ⇒ discover from the issuer. | `--oidc-jwks-url` | | `allowed_hosts` | Extra hostnames for the DNS-rebinding allow-list (OIDC/routable mode only), on top of the audience host + loopback. | `--allowed-host` | | `rate_limit` | Per-caller request cap on `/mcp`, in requests per minute. Absent / `0` ⇒ disabled. | `--rate-limit` | +| `local_writes` | Enable local single-user MCP writes over stdio using the active profile token. Separate from `allow_writes`; rejected with HTTP in this release. | `--local-writes` | +| `allow_writes` | Enable shared HTTP/OIDC MCP writes when paired with `[serve.vault]` and callers carrying `nbox:write`. Does not imply `local_writes`. | `--allow-writes` | ```toml [serve] @@ -242,6 +244,10 @@ http = "127.0.0.1:8080" # jwks_url = "https://idp.example.com/keys" # allowed_hosts = ["nbox.example.com"] # rate_limit = 120 +# local stdio writes: +# local_writes = true +# shared HTTP/OIDC writes: +# allow_writes = true ``` See [docs/MCP.md](MCP.md) for the full server story. diff --git a/docs/FEATURES.md b/docs/FEATURES.md index dbab8b5..f1d35b8 100644 --- a/docs/FEATURES.md +++ b/docs/FEATURES.md @@ -7,10 +7,10 @@ description` and `device … set status` (`PATCH`), `ip reserve ` / `prefix reserve ` / `ip-range reserve ` (three `allocate` `POST`s), and `tag add`/`remove ` (a `PATCH` to the `tags` array on any object). The same safe-write foundation is also exposed as MCP -write tools (`nbox_plan_write`/`nbox_apply_write`) over the authenticated -HTTP+OIDC transport when `[serve].allow_writes` is set, gated by the per-user -credential vault and running as the calling user; stdio and unauthenticated -transports stay read-only. +write tools (`nbox_plan_write`/`nbox_apply_write`) in two explicit modes: local +stdio with `[serve].local_writes` / `--local-writes` using the active profile +token, or shared HTTP/OIDC with `[serve].allow_writes` / `--allow-writes` plus +the per-user credential vault. ## Lookups @@ -159,10 +159,11 @@ Twelve themes (`NO_COLOR` honored); `?`/`F1` shows the full keymap. subprocess and speaks JSON-RPC over stdin/stdout; the tools reuse the CLI's query + view layer and return the same JSON view models. JSON-RPC on stdout, logs on stderr; URL/token from the active profile (same `-p`/`--config` flags). The read -tools are annotated read-only; the write tools (`nbox_plan_write`/`nbox_apply_write`, -exposed only over HTTP+OIDC with `--allow-writes`) are not. `nbox serve --print-config` prints the paste-ready -`mcpServers` JSON (absolute binary path, echoed `--profile`/`--config`, placeholder -token) and exits — no server start, no connection; see docs/MCP.md for the per-host +tools are annotated read-only; the write tools (`nbox_plan_write`/`nbox_apply_write`) +are not and execute only with local stdio `--local-writes` or shared HTTP/OIDC +`--allow-writes` plus a vault. `nbox serve --print-config` prints the paste-ready +`mcpServers` JSON (absolute binary path, echoed `--profile`/`--config`/ +`--local-writes`, placeholder token) and exits — no server start, no connection; see docs/MCP.md for the per-host config-file path. | Tool | What | @@ -178,8 +179,8 @@ config-file path. | `nbox_list_tags` | List tags. | | `nbox_tagged` | Objects carrying a tag, across kinds (NetBox 4.3+); `tag` (id\|name\|slug). Cross-kind reverse lookup. | | `nbox_cache_clear` | Drop nbox's local read cache so the next lookups fetch fresh (read-only w.r.t. NetBox). | -| `nbox_plan_write` | Plan a safe write (interface description, device status, IP/prefix/IP-range reserve, tag add/remove): builds a before/after diff and a confirm token without mutating. Requires `--allow-writes`, the caller's `nbox:write` scope, and a `[serve.vault]` mapping for the caller's OIDC `sub`; rejected over stdio. | -| `nbox_apply_write` | Apply a previously planned write (verifies the confirm token, then executes under the caller's per-user NetBox identity). Same gating as `nbox_plan_write`. | +| `nbox_plan_write` | Plan a safe write (interface description, device status, IP/prefix/IP-range reserve, tag add/remove): builds a before/after diff and a confirm token without mutating. Requires local stdio `--local-writes`, or shared HTTP/OIDC `--allow-writes` plus `nbox:write` and a `[serve.vault]` mapping for the caller's OIDC `sub`. | +| `nbox_apply_write` | Apply a previously planned write (verifies the confirm token, then executes the server-stored plan under the same write mode). Same gating as `nbox_plan_write`. | A loopback HTTP transport ships in the default build (behind the `http` cargo feature, on by default; `--no-default-features` for stdio-only): @@ -189,9 +190,10 @@ feature, on by default; `--no-default-features` for stdio-only): Protected Resource Metadata, routable bind allowed) — accountability, not per-user RBAC. See [docs/MCP.md](MCP.md). The same objects are also exposed as MCP **resources** via one `nbox://{kind}/{ref}` template (the view `nbox_get` -returns). Per-user NetBox identity bridging ships as the credential vault -(`[serve.vault.]` maps each caller's OIDC `sub` to a per-user NetBox -token), and the MCP **prompts** catalog ships a curated set of read-only +returns). Local stdio MCP writes ship behind `--local-writes`; per-user NetBox +identity bridging ships as the credential vault (`[serve.vault.]` maps each +caller's OIDC `sub` to a per-user NetBox token) for shared HTTP/OIDC writes. The +MCP **prompts** catalog ships a curated set of read-only investigation prompts (`ip_utilization_audit`, `cable_path_trace`, `find_stale_prefixes`, `object_change_review`). A raw escape-hatch MCP tool is later. diff --git a/docs/MCP.md b/docs/MCP.md index 425fbb4..819d179 100644 --- a/docs/MCP.md +++ b/docs/MCP.md @@ -5,9 +5,10 @@ Context Protocol) server that is read-only by default, over the stdio transport. An MCP host launches the `nbox` binary as a subprocess and speaks JSON-RPC over its stdin/stdout; the tools reuse the same NetBox query + view layer as the CLI, so they return the same JSON view models. The default tools never write. Opt-in -write tools are available over the HTTP+OIDC transport with `--allow-writes` and -a per-user credential vault — see the **Writes** subsection under -[OIDC resource-server auth](#oidc-resource-server-auth-network-reachable). +write tools support two explicit modes: local stdio writes with `--local-writes` +(the active profile token), or shared HTTP/OIDC writes with `--allow-writes` and +a per-user credential vault. HTTP profile-token writes are not enabled in this +first cut, even on loopback. ## Prerequisites @@ -76,7 +77,9 @@ host's MCP documentation; the object shape above is what they consume. Rather than hand-write the block, ask nbox for it — `--print-config` emits the `mcpServers` JSON to stdout and exits, without starting the server or connecting -to NetBox (so it works before you've even set a token): +to NetBox (so it works before you've even set a token). It echoes any +`--profile`, `--config`, or `--local-writes` flag you passed into the generated +`args`: ```bash $ nbox serve --print-config @@ -277,17 +280,19 @@ Run **reads** in this mode only for a **trusted, read-only, ideally single-team* deployment. Use a NetBox token scoped to exactly what an agent should see (read-only); that token is the real privilege boundary for reads. -**Writes** are a separate, opt-in **Pattern 2** path (DESIGN §24): per-user +**Writes** are a separate opt-in. Local single-user MCP writes use stdio plus +`nbox serve --local-writes` (or `[serve].local_writes = true`) and run under the +active profile token, with the MCP host's tool-approval prompt as the human gate. +Shared/network writes remain the **Pattern 2** path (DESIGN §24): per-user identity → NetBox-token bridging via a credential vault keyed by the OIDC `sub`, so NetBox's object permissions and changelog attribute each write to the real -user — not the service account. Writes are enabled only when **all** of the -following hold, and fail closed otherwise: `nbox serve --allow-writes` (or -`[serve].allow_writes = true`); the caller's token carries the `nbox:write` -scope; and a `[serve.vault.""]` entry maps the caller's `sub` to an env var -holding that user's NetBox token. The read-only service token is never used for a -write, and writes are unavailable over stdio / loopback static-bearer auth (no -per-user identity to bridge). The `nbox_plan_write` / `nbox_apply_write` tools -expose the same plan → confirm-token → apply lifecycle as the CLI. +user. Shared HTTP writes are enabled only when **all** of the following hold, and +fail closed otherwise: `nbox serve --allow-writes` (or `[serve].allow_writes = +true`); the caller's token carries the `nbox:write` scope; and a +`[serve.vault.""]` entry maps the caller's `sub` to an env var holding that +user's NetBox token. HTTP/static-bearer transports cannot use `local_writes` in +this release. The `nbox_plan_write` / `nbox_apply_write` tools expose the same +plan → confirm-token → apply lifecycle as the CLI. ## Operations (HTTP transport) @@ -411,26 +416,21 @@ through unchanged (it's the one kind whose spelling differs between the two). | Tool | Purpose | | ---- | ------- | | `nbox_plan_write` | Plan a NetBox write (interface description, device status, IP/prefix/IP-range reserve, tag add/remove). Builds a `MutationPlan` — a before/after diff plus a confirm token — without mutating NetBox. Annotated `read_only_hint = false`. | -| `nbox_apply_write` | Apply a previously planned write. Pass back the `MutationPlan` from `nbox_plan_write`; its `confirm_token` looks up the plan the server stored at plan time and applies **that** stored plan (the submitted contents are not trusted — a forged or edited plan, or one issued for a different caller, is rejected). Runs under the caller's per-user NetBox identity, returning a `MutationReceipt`. Annotated `read_only_hint = false`. | +| `nbox_apply_write` | Apply a previously planned write. Pass back the `MutationPlan` from `nbox_plan_write`; its `confirm_token` looks up the plan the server stored at plan time and applies **that** stored plan (the submitted contents are not trusted — a forged or edited plan, one issued for a different actor, or a replay is rejected). Runs under the configured write mode, returning a `MutationReceipt`. Annotated `read_only_hint = false`. | Both write tools are **always registered** (discoverable in `tools/list`); what's -gated is execution. A call is rejected unless writes are enabled (`--allow-writes` -or `[serve].allow_writes`) over the HTTP+OIDC transport AND the caller carries the -`nbox:write` scope with a `[serve.vault.""]` entry mapping the caller's OIDC -`sub` to that user's NetBox token. Without writes enabled they reject with -"writes disabled"; they are rejected over stdio / unauthenticated transports (no -per-user identity to bridge). See the **Writes** subsection under -[OIDC resource-server auth](#oidc-resource-server-auth-network-reachable) for the -full gating. - -There is no MCP write path without OIDC — the per-user identity is the point, so -the shared profile token is never used to write. For **local, single-user -writes** (the common stdio `claude mcp add nbox -- nbox serve` setup), use the -equivalent **CLI** command instead — `nbox ip reserve …`, `nbox interface … set -description …`, `nbox device … set status …` — which writes with the local -profile token behind `--allow-writes` + confirmation, no IdP required. The MCP -write tools are for a multi-user, network-reachable deployment where NetBox must -attribute each change to a real user. +gated is execution: + +- **Local stdio:** `nbox serve --local-writes` or `[serve].local_writes = true`. + The tools use the active profile token and bind plans to a synthetic local actor. +- **Shared HTTP/OIDC:** `nbox serve --http … --allow-writes` or + `[serve].allow_writes = true`, the caller carries `nbox:write`, and + `[serve.vault.""]` maps the caller's OIDC `sub` to that user's NetBox token. + +Without one of those modes the tools reject before touching NetBox. If an OIDC +identity is present, the shared/vault path always wins; `local_writes` never +bypasses `nbox:write` or the vault. HTTP local writes, including loopback +profile-token writes, are deferred until a separate guard decision. ## Resources @@ -483,11 +483,11 @@ prompts. ## Security and behavior -- **Use a read-only NetBox token.** By default the server exposes no write path; - writes are opt-in (`--allow-writes`, the `nbox:write` scope, and a per-user - vault) and run as the calling user, never the read-only service token. For the - default read-only deployment the token is the real safety boundary — scope it to - what you want an agent to see. +- **Use a read-only NetBox token by default.** By default the server exposes no + write path. Local stdio writes are opt-in (`--local-writes`) and run as the + active profile token; shared HTTP writes are opt-in (`--allow-writes`, + `nbox:write`, and a per-user vault) and run as the calling user. Scope every + token to what the agent may read or change. - **stdout carries only the JSON-RPC stream.** All logging goes to stderr, so it never corrupts the protocol. - **The token is never logged.** Request logging shows only the auth scheme diff --git a/docs/SCRIPTING.md b/docs/SCRIPTING.md index 9a38a50..0a496e6 100644 --- a/docs/SCRIPTING.md +++ b/docs/SCRIPTING.md @@ -169,11 +169,10 @@ docker run --rm -e NBOX_TOKEN=... \ and speaks JSON-RPC over stdin/stdout (logs stay on stderr). The tools reuse the CLI's query and view layer, so they return the exact same JSON view models the CLI prints — an agent gets the structured data, not screen-scraped text. Run -with `--allow-writes` (HTTP + OIDC only) to additionally expose the -`nbox_plan_write`/`nbox_apply_write` tools, each running under the caller's -per-user NetBox identity via the `[serve.vault]` mapping; stdio and -unauthenticated transports stay read-only. See [MCP.md](MCP.md) for the -write/vault details. +with `--local-writes` for local stdio writes using the active profile token, or +with `--allow-writes` (HTTP + OIDC) to expose the `nbox_plan_write`/ +`nbox_apply_write` tools under the caller's per-user NetBox identity via the +`[serve.vault]` mapping. See [MCP.md](MCP.md) for the write details. Local stdio host (Claude Code): @@ -212,8 +211,8 @@ The eleven read tools, all annotated read-only: | `nbox_cache_clear` | Drop nbox's local read cache so the next reads fetch fresh. | Two more tools — `nbox_plan_write` and `nbox_apply_write` (`read_only_hint = -false`) — are exposed only with `--allow-writes` over the HTTP + OIDC transport; -see [MCP.md](MCP.md). +false`) — execute only with local stdio `--local-writes` or shared HTTP/OIDC +`--allow-writes`; see [MCP.md](MCP.md). The same objects are also exposed as MCP resources via one template, `nbox://{kind}/{ref}` (e.g. `nbox://device/edge01`) — reading one returns the diff --git a/docs/adr/0001-safe-write-foundation.md b/docs/adr/0001-safe-write-foundation.md index 429d5bb..4b91312 100644 --- a/docs/adr/0001-safe-write-foundation.md +++ b/docs/adr/0001-safe-write-foundation.md @@ -148,35 +148,40 @@ Current nbox constraints also matter: and no `nbox raw POST|PATCH|DELETE` should ship until they are built on the same planner, diff, confirmation, concurrency, and audit contracts. -7. **MCP writes require per-user identity (Pattern 2).** ✅ **Implemented.** The - `nbox_plan_write` and `nbox_apply_write` MCP tools are backed by the per-user - credential vault (`src/mcp/vault.rs`). The validated OIDC `Identity` (`sub` + - scopes) is plumbed from the auth gate into the request `Parts`, which the - Streamable-HTTP transport nests into the rmcp `RequestContext`; the write tools - read the caller's `sub` from there and the vault maps it to a per-user NetBox - token (env var), bridged at request time via `NetBoxClient::with_token` so the - write `PATCH`/`POST` hits NetBox under the caller's identity. The design - follows the constraints below: +7. **MCP writes use an explicit write mode.** ✅ **Implemented, amended by + ADR-0002.** The `nbox_plan_write` and `nbox_apply_write` MCP tools are backed + by the same safe-write engine as the CLI and execute only in one of two + explicit modes. Shared HTTP/OIDC deployments use the per-user credential vault + (`src/mcp/vault.rs`): the validated OIDC `Identity` (`sub` + scopes) is + plumbed from the auth gate into the request `Parts`, the Streamable-HTTP + transport nests it into the rmcp `RequestContext`, and the vault maps the + caller's `sub` to a per-user NetBox token (env var), bridged at request time + via `NetBoxClient::with_token` so the write `PATCH`/`POST` hits NetBox under + the caller's identity. Local stdio deployments can instead opt into + `[serve].local_writes` / `--local-writes`, which uses the active profile token + and binds the plan to a synthetic local actor. The design follows the + constraints below: - ✅ operation-specific, not a generic raw mutation tool; - - ✅ require `--allow-writes` on `nbox serve` **and** the caller's token - carrying the `nbox:write` scope. Scope is enforced **in the write tools** - (per request), not the HTTP gate — the gate deliberately does not parse the + - ✅ require either local stdio `--local-writes` / `[serve].local_writes`, or + shared HTTP/OIDC `--allow-writes` plus the caller's token carrying the + `nbox:write` scope. HTTP scope is enforced **in the write tools** (per + request), not the HTTP gate — the gate deliberately does not parse the JSON-RPC method (that would mean buffering the streaming body), so it cannot tell a read tool from a write one. Both `nbox_plan_write` and `nbox_apply_write` are advertised `read_only_hint = false`: planning is part - of the write flow (it requires the same gate + scope + per-user identity), - so a host's "auto-run read-only tools" policy must not auto-invoke it. + of the write flow, so a host's "auto-run read-only tools" policy must not + auto-invoke it. - ✅ return a plan/diff first and require an explicit apply call carrying the confirmation token; - ✅ use the same mutation engine as the CLI; - - ✅ require Pattern 2 per-user NetBox credential bridging. Authorization fails - closed, in order: writes disabled (`--allow-writes` / `[serve.vault]` absent) - → no authenticated caller identity (stdio / loopback static-bearer carry - none — writes need HTTP+OIDC) → missing `nbox:write` scope → no - `[serve.vault]` entry for the caller's `sub`. The service token is never used - for writes, and the no-identity case is its own distinct error that never - suggests mapping a placeholder `sub`. + - ✅ fail closed by mode. With an authenticated OIDC caller, authorization is: + shared writes enabled (`--allow-writes` / `[serve.vault]`) → `nbox:write` + scope → vault entry for the caller's `sub`. With no caller, authorization is + stdio transport plus explicit `--local-writes` / `[serve].local_writes`; + HTTP and static-bearer transports remain rejected in ADR-0002's first cut. + The service/profile token is never used for shared HTTP writes, only for + explicit local stdio writes. 8. **Use clear audit and status wording.** Write logs and user-visible messages should describe facts, not magic: diff --git a/docs/adr/0002-local-single-user-mcp-writes.md b/docs/adr/0002-local-single-user-mcp-writes.md index 6f850d6..2bf0180 100644 --- a/docs/adr/0002-local-single-user-mcp-writes.md +++ b/docs/adr/0002-local-single-user-mcp-writes.md @@ -1,6 +1,6 @@ # ADR-0002: Local single-user MCP writes -**Status:** Proposed +**Status:** Accepted **Date:** 2026-06-29 **Amends:** ADR-0001 §7 (MCP writes require per-user identity) @@ -36,8 +36,9 @@ documented — local users are told to use the equivalent CLI command. Add an explicit, opt-in **single-user local write mode**, gated by a new, distinct config key **`[serve].local_writes`** (with a `--local-writes` CLI flag). When -enabled, `nbox serve` accepts MCP writes that carry **no per-user identity** and -executes them under the **profile token** — the same token reads already use. +enabled on the **stdio** transport, `nbox serve` accepts MCP writes that carry +**no per-user identity** and executes them under the **profile token** — the same +token reads already use. It is deliberately separate from the Pattern 2 path (`[serve].allow_writes` + `[serve.vault]` + OIDC), which is unchanged. A write is authorized if **either**: @@ -45,8 +46,7 @@ It is deliberately separate from the Pattern 2 path (`[serve].allow_writes` + - **Pattern 2 (multi-user, unchanged):** an OIDC caller `sub` carrying the `nbox:write` scope with a `[serve.vault]` entry → that user's per-user token; or - **Single-user local (new):** `[serve].local_writes = true`, the request carries - **no** OIDC identity, and the transport is **stdio or a loopback HTTP bind** → - the profile token. + **no** OIDC identity, and the transport is **stdio** → the profile token. The presence of an OIDC identity disambiguates the two: an authenticated request always takes the Pattern 2 path (and still needs the scope + a vault entry); @@ -55,10 +55,11 @@ multi-user path. Hard safety rules: -1. **Loopback / stdio only.** `local_writes` never applies to a non-loopback - bind. A routable HTTP server already *requires* OIDC; profile-token writes must - never be reachable over the network. `--http ` with `local_writes` - and no OIDC is a **startup usage error**. +1. **Stdio only in the first cut.** `local_writes` never applies to HTTP, even + loopback. A routable HTTP server already *requires* OIDC; profile-token writes + must never be reachable over the network. Loopback local writes are deferred + until there is a separate HTTP-local guard decision (for example requiring a + static bearer plus loopback). 2. **Explicit, off by default.** `local_writes` is its own key, never implied by `allow_writes`. An operator turns it on deliberately. There is no token/scope to check in this mode — the opt-in flag *is* the authorization, and the host's @@ -73,7 +74,7 @@ Hard safety rules: This amends ADR-0001 §7: "the service token is never used for writes" becomes "the service token is never used for writes **except** in the explicit, -loopback/stdio-only `local_writes` single-user mode." +stdio-only `local_writes` single-user mode." ## Consequences @@ -92,31 +93,33 @@ Negative / trade-offs: scope that token to what the agent may change (the same defense-in-depth advice as for reads). - No per-user attribution in this mode — acceptable by definition (one user). The - write audit records `surface=mcp`, the profile, and the operation, with `sub` - recorded as `local`. + write audit records `surface=mcp`, the profile, and the operation. Internally + the server-issued plan store binds local plans to the stable pseudo-principal + `local`. - Two write-enable knobs (`allow_writes`, `local_writes`), kept distinct on purpose so the multi-user and single-user paths never blur. Neutral: -- A non-loopback bind is unaffected (still OIDC-only). -- Not in 0.14.0 — ships as a focused fast-follow so it gets its own review. +- HTTP is unaffected (still OIDC/vault-only for writes). ## Implementation sketch - `config.rs`: add `ServeConfig.local_writes: bool`. `cli.rs`: add `--local-writes`. -- Startup (`http.rs`): a non-loopback bind with `local_writes` and no OIDC is a - usage error (exit 2), mirroring the existing non-loopback-needs-OIDC check. +- Startup: any HTTP bind with `local_writes` is a usage error (exit 2) in this + first cut; use stdio for local writes or OIDC/vault for HTTP writes. - `write.rs::bridged_client`: when the caller is `None` (no OIDC identity), - `local_writes` is enabled, and the transport is stdio / loopback, return the + `local_writes` is enabled, and the transport is stdio, return the profile client (`self.client` clone) instead of rejecting. The `Some(caller)` (Pattern 2) path is unchanged. The plan store, scope-on-the-Pattern-2-path, and audit are untouched. -- Audit: record `sub = "local"` for the single-user path. +- Plan store: bind plans to an internal write actor (`sub:` or + `local`) so local plans remain one-shot and cannot be replayed by another + actor. - Tests: `local_writes` enables a stdio write end to end through the plan store; a - non-loopback bind + `local_writes` is a startup error; `local_writes` does not - bypass the plan store (a forged plan is still rejected); the OIDC/Pattern 2 path - is unchanged; with no `local_writes`, stdio writes still reject. + HTTP bind + `local_writes` is a startup error; `local_writes` does not bypass + the plan store (a forged plan is still rejected); the OIDC/Pattern 2 path is + unchanged; with no `local_writes`, stdio writes still reject. ## Alternatives considered diff --git a/docs/adr/README.md b/docs/adr/README.md index bc2fff7..e0d479b 100644 --- a/docs/adr/README.md +++ b/docs/adr/README.md @@ -9,7 +9,7 @@ contributors understand why the design exists, not just what changed. | ADR | Title | Status | Date | |-----|-------|--------|------| | [0001](0001-safe-write-foundation.md) | Safe write foundation | Accepted | 2026-06-26 | -| [0002](0002-local-single-user-mcp-writes.md) | Local single-user MCP writes | Proposed | 2026-06-29 | +| [0002](0002-local-single-user-mcp-writes.md) | Local single-user MCP writes | Accepted | 2026-06-29 | ## Template diff --git a/skills/serve/SKILL.md b/skills/serve/SKILL.md index fa9762a..ef046b5 100644 --- a/skills/serve/SKILL.md +++ b/skills/serve/SKILL.md @@ -65,8 +65,9 @@ tokens (unknown keys ignored). ## Install recipe `nbox serve --print-config` prints the paste-ready `mcpServers` JSON (absolute -binary path, echoed `--profile`/`--config`, placeholder token) and exits — no -server start, no NetBox connection. Drop it into the host's MCP config. +binary path, echoed `--profile`/`--config`/`--local-writes`, placeholder token) +and exits — no server start, no NetBox connection. Drop it into the host's MCP +config. ```bash nbox serve --print-config # paste-ready mcpServers JSON, then exit @@ -78,14 +79,13 @@ nbox serve --http 127.0.0.1:8080 # loopback HTTP, read-only The MCP server is read-only by default. The write tools (`nbox_plan_write` / `nbox_apply_write`) are always registered, but a call only **executes** with -`nbox serve --http --allow-writes` plus the caller's `nbox:write` scope and a -`[serve.vault]` entry mapping their OIDC `sub` to a per-user NetBox token — writes -require the HTTP+OIDC transport; stdio stays read-only. `nbox_apply_write` applies -the plan the server stored at plan time (looked up by the `confirm_token` from -`nbox_plan_write`), not the plan you resubmit. For local, single-user writes (the -stdio setup), use the equivalent **CLI** command instead — it writes with the -local profile token, no IdP. For that lifecycle, see the -[safe writes](../writes/SKILL.md) skill. +one explicit write mode: local stdio `nbox serve --local-writes`, which uses the +active profile token, or shared HTTP/OIDC `nbox serve --http --allow-writes` plus +the caller's `nbox:write` scope and a `[serve.vault]` entry mapping their OIDC +`sub` to a per-user NetBox token. HTTP local writes are deferred in this release. +`nbox_apply_write` applies the plan the server stored at plan time (looked up by +the `confirm_token` from `nbox_plan_write`), not the plan you resubmit. For that +lifecycle, see the [safe writes](../writes/SKILL.md) skill. ## Reference diff --git a/skills/writes/SKILL.md b/skills/writes/SKILL.md index b998495..f94d1d5 100644 --- a/skills/writes/SKILL.md +++ b/skills/writes/SKILL.md @@ -69,13 +69,13 @@ token never appears in the audit log. ## MCP writes are a separate opt-in The MCP server is read-only by default. The same planner/applier backs two MCP -write tools, `nbox_plan_write` and `nbox_apply_write`, exposed **only** with -`nbox serve --http --allow-writes` plus a `[serve.vault]` config mapping each -caller's OIDC `sub` to a per-user NetBox token (each write runs as the calling -user, and the caller's token must carry the `nbox:write` scope). Stdio and -unauthenticated transports stay read-only; without `--allow-writes` the write -tools reject with "writes disabled". See the [serve skill](../serve/SKILL.md) -and ADR-0001 Decision 7 (Implemented). +write tools, `nbox_plan_write` and `nbox_apply_write`. They execute only in one +explicit mode: local stdio `nbox serve --local-writes`, which uses the active +profile token, or shared HTTP/OIDC `nbox serve --http --allow-writes` plus a +`[serve.vault]` config mapping each caller's OIDC `sub` to a per-user NetBox +token (each shared write runs as the calling user, and the caller's token must +carry the `nbox:write` scope). HTTP local writes are deferred. See the +[serve skill](../serve/SKILL.md), ADR-0001, and ADR-0002. ## Reference diff --git a/src/cli.rs b/src/cli.rs index 251f62e..ec3dc94 100644 --- a/src/cli.rs +++ b/src/cli.rs @@ -617,6 +617,12 @@ pub enum Command { #[arg(long = "allow-writes")] allow_writes: bool, + /// Enable local single-user MCP writes over stdio. Uses the active + /// profile token and the MCP host's tool approval; not valid for HTTP + /// in this first cut. Also read from `[serve].local_writes`. + #[arg(long = "local-writes")] + local_writes: bool, + /// Print a copy-paste MCP server config (the `mcpServers` JSON object /// most hosts read) to stdout and exit, without starting the server or /// connecting to NetBox. The `command` is the absolute path to this @@ -1423,6 +1429,8 @@ mod tests { oidc_jwks_url: None, ref allowed_host, rate_limit: None, + local_writes: false, + allow_writes: false, print_config: false, .. }) if allowed_host.is_empty() @@ -1442,6 +1450,27 @@ mod tests { Some(Command::Serve { http: Some(a), http_token: Some(t), print_config: false, .. }) if a == "127.0.0.1:8080" && t == "abc123" )); + + let local = Cli::try_parse_from(["nbox", "serve", "--local-writes"]).unwrap(); + assert!(matches!( + local.command, + Some(Command::Serve { + local_writes: true, + allow_writes: false, + http: None, + .. + }) + )); + + let shared = Cli::try_parse_from(["nbox", "serve", "--allow-writes"]).unwrap(); + assert!(matches!( + shared.command, + Some(Command::Serve { + allow_writes: true, + local_writes: false, + .. + }) + )); } #[test] diff --git a/src/config.rs b/src/config.rs index ccdc298..b3d9de8 100644 --- a/src/config.rs +++ b/src/config.rs @@ -146,6 +146,12 @@ pub struct ServeConfig { #[serde(default, skip_serializing_if = "std::ops::Not::not")] pub allow_writes: bool, + /// Whether to enable local single-user MCP writes over stdio. This is + /// separate from `allow_writes`: it uses the active profile token and never + /// enables HTTP writes. Overridden by `--local-writes`. + #[serde(default, skip_serializing_if = "std::ops::Not::not")] + pub local_writes: bool, + /// Per-user credential vault: maps OIDC `sub` → env var name holding a /// per-user NetBox token. See [`crate::mcp::vault::CredentialVault`]. Only /// consulted when `allow_writes` is `true`. @@ -169,6 +175,7 @@ impl std::fmt::Debug for ServeConfig { .field("allowed_hosts", &self.allowed_hosts) .field("rate_limit", &self.rate_limit) .field("allow_writes", &self.allow_writes) + .field("local_writes", &self.local_writes) .field("vault_entries", &self.vault.len()) .finish() } @@ -1558,6 +1565,8 @@ search = "graphql" assert_eq!(bare.serve.jwks_url, None); assert!(bare.serve.allowed_hosts.is_empty()); assert_eq!(bare.serve.rate_limit, None); + assert!(!bare.serve.allow_writes); + assert!(!bare.serve.local_writes); // A present `[serve]` populates the fields. let with: Config = toml::from_str( @@ -1567,6 +1576,7 @@ search = "graphql" http = \"127.0.0.1:8080\"\n\ http_token = \"local-secret\"\n\ rate_limit = 120\n\ + local_writes = true\n\ \n\ [profiles.work]\n\ url = \"https://netbox.example.com\"\n", @@ -1575,6 +1585,24 @@ search = "graphql" assert_eq!(with.serve.http.as_deref(), Some("127.0.0.1:8080")); assert_eq!(with.serve.http_token.as_deref(), Some("local-secret")); assert_eq!(with.serve.rate_limit, Some(120)); + assert!(with.serve.local_writes); + assert!(!with.serve.allow_writes); + + let allow_only: Config = toml::from_str( + "active_profile = \"work\"\n\ + \n\ + [serve]\n\ + allow_writes = true\n\ + \n\ + [profiles.work]\n\ + url = \"https://netbox.example.com\"\n", + ) + .unwrap(); + assert!(allow_only.serve.allow_writes); + assert!( + !allow_only.serve.local_writes, + "allow_writes must not imply local_writes" + ); // The OIDC resource-server fields parse onto the same section. let oidc: Config = toml::from_str( diff --git a/src/lib.rs b/src/lib.rs index 916b493..b907cb2 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -599,6 +599,7 @@ pub async fn run(cli: Cli) -> Result<()> { allowed_host, rate_limit, allow_writes, + local_writes, print_config, }) => { run_serve( @@ -612,6 +613,7 @@ pub async fn run(cli: Cli) -> Result<()> { allowed_host, rate_limit, allow_writes, + local_writes, print_config, }, ) @@ -750,6 +752,9 @@ struct ServeFlags { /// `--allow-writes`: enable MCP write tools (Pattern 2). `false` (the /// default) keeps the server read-only. allow_writes: bool, + /// `--local-writes`: enable local single-user MCP writes over stdio using + /// the active profile token. Separate from `--allow-writes`. + local_writes: bool, print_config: bool, } @@ -765,7 +770,7 @@ async fn run_serve(ctx: &Ctx, flags: ServeFlags) -> Result<()> { // before resolving any serve config or connecting to NetBox. Works with no // config file and no token — run it anytime to get the paste-ready block. if flags.print_config { - let cfg = build_mcp_config(ctx); + let cfg = build_mcp_config(ctx, flags.local_writes); // stdout carries only the JSON (data); pretty-printed for pasteability. serde_json::to_writer_pretty(std::io::stdout(), &cfg).context("writing MCP config")?; println!(); @@ -789,9 +794,12 @@ async fn run_serve(ctx: &Ctx, flags: ServeFlags) -> Result<()> { // Per-caller rate limit: flag wins, then config, then off (0). Absent / 0 = // disabled, so existing behavior is unchanged unless the operator opts in. let rate_limit = flags.rate_limit.or(serve_cfg.rate_limit).unwrap_or(0); - // Write tools require both the flag/config gate AND the HTTP transport - // (writes need OIDC identity → per-user token resolution via the vault). + // Shared HTTP writes require both the flag/config gate and the HTTP + // transport (OIDC identity → per-user token resolution via the vault). let allow_writes = flags.allow_writes || serve_cfg.allow_writes; + // Local single-user writes are stdio-only in ADR-0002's first cut. Keep the + // knob distinct from allow_writes: neither implies the other. + let local_writes = flags.local_writes || serve_cfg.local_writes; // OIDC resource-server mode is enabled by the issuer's presence; the audience // is then required (RFC 8707 — without an expected `aud`, nbox can't bind a @@ -811,15 +819,23 @@ async fn run_serve(ctx: &Ctx, flags: ServeFlags) -> Result<()> { (None, _) => None, }; + if http.is_some() && local_writes { + return Err(error::NboxError::Usage( + "`--local-writes` / [serve].local_writes is only supported on the stdio MCP \ + transport in this release; HTTP writes require --allow-writes plus OIDC/vault" + .to_string(), + ) + .into()); + } + let client = connect(ctx)?; // The long-lived server shares a read cache across tool calls (chatty agents // re-read the same object graph); agents can drop it with `nbox_cache_clear`. let cache = serve_cache(ctx, &client); - // Build the per-user credential vault when writes are enabled. The vault - // maps OIDC `sub` → env var name holding a per-user NetBox token. Writes - // require the HTTP transport (OIDC identity); stdio has no caller identity, - // so writes on stdio are rejected by the vault being `None`. + // Build the per-user credential vault for the shared HTTP/OIDC write path. + // The vault maps OIDC `sub` → env var name holding a per-user NetBox token. + // Local stdio writes intentionally do not use the vault. let vault = if allow_writes && http.is_some() { Some(mcp::vault::CredentialVault::new(serve_cfg.vault, true)) } else { @@ -827,7 +843,7 @@ async fn run_serve(ctx: &Ctx, flags: ServeFlags) -> Result<()> { }; match http { - None => mcp::serve(client, cache).await, + None => mcp::serve(client, cache, local_writes).await, Some(addr) => { serve_http_or_explain( client, @@ -858,13 +874,16 @@ async fn run_serve(ctx: &Ctx, flags: ServeFlags) -> Result<()> { /// so the snippet reproduces the invocation. `env.NBOX_TOKEN` is a placeholder /// — the operator sets it (or removes the block if `nbox config init` holds the /// token). Never echoes a real token. -fn build_mcp_config(ctx: &Ctx) -> serde_json::Value { +fn build_mcp_config(ctx: &Ctx, local_writes: bool) -> serde_json::Value { let command = std::env::current_exe() .ok() .and_then(|p| p.to_str().map(str::to_string)) .unwrap_or_else(|| "nbox".to_string()); let mut args = vec!["serve".to_string()]; + if local_writes { + args.push("--local-writes".to_string()); + } if let Some(profile) = &ctx.profile { args.push("--profile".to_string()); args.push(profile.clone()); @@ -3216,7 +3235,7 @@ mod tests { #[test] fn build_mcp_config_emits_stdio_recipe_with_placeholder_token() { - let cfg = build_mcp_config(&ctx_for(None, None)); + let cfg = build_mcp_config(&ctx_for(None, None), false); let nbox = &cfg["mcpServers"]["nbox"]; // `command` is an absolute path to this binary (resolves in the test run). let command = nbox["command"].as_str().expect("command present"); @@ -3230,7 +3249,7 @@ mod tests { #[test] fn build_mcp_config_echoes_profile_and_config_flags() { - let cfg = build_mcp_config(&ctx_for(Some("work"), Some("/tmp/nb.toml"))); + let cfg = build_mcp_config(&ctx_for(Some("work"), Some("/tmp/nb.toml")), false); let args: Vec = nbox_args(&cfg); assert_eq!( args, @@ -3241,11 +3260,28 @@ mod tests { #[test] fn build_mcp_config_omits_profile_when_unset() { // Only `--config` set → no `--profile` pair in args. - let cfg = build_mcp_config(&ctx_for(None, Some("/tmp/nb.toml"))); + let cfg = build_mcp_config(&ctx_for(None, Some("/tmp/nb.toml")), false); let args: Vec = nbox_args(&cfg); assert_eq!(args, vec!["serve", "--config", "/tmp/nb.toml"]); } + #[test] + fn build_mcp_config_echoes_local_writes_when_requested() { + let cfg = build_mcp_config(&ctx_for(Some("work"), Some("/tmp/nb.toml")), true); + let args: Vec = nbox_args(&cfg); + assert_eq!( + args, + vec![ + "serve", + "--local-writes", + "--profile", + "work", + "--config", + "/tmp/nb.toml", + ] + ); + } + fn nbox_args(cfg: &serde_json::Value) -> Vec { cfg["mcpServers"]["nbox"]["args"] .as_array() diff --git a/src/mcp/mod.rs b/src/mcp/mod.rs index 2f4f53f..397f9aa 100644 --- a/src/mcp/mod.rs +++ b/src/mcp/mod.rs @@ -5,7 +5,8 @@ //! domain-view layer the CLI handlers use. Each tool is a thin adapter: it calls //! the same query helpers, builds the same view model, and returns it as //! structured JSON. Two opt-in write tools (`nbox_plan_write` / `nbox_apply_write`) -//! are gated by `--allow-writes` plus the per-user credential vault (Pattern 2). +//! are gated either by local stdio `--local-writes` (single-user profile-token +//! mode) or by `--allow-writes` plus the per-user credential vault (Pattern 2). //! //! stdout carries the JSON-RPC stream and nothing else — logging goes to stderr //! (see [`crate::init_logging`]), and the connect path here prints nothing. @@ -40,7 +41,7 @@ use crate::netbox::client::NetBoxClient; use crate::netbox::search::{SearchFilters, SearchRequest, SearchResult}; use crate::netbox::status::AuthCheck; -/// The NetBox MCP server (read-only by default; write tools gated by the vault). +/// The NetBox MCP server (read-only by default; write tools are explicitly gated). #[derive(Clone)] pub struct NboxMcp { client: Arc, @@ -49,8 +50,11 @@ pub struct NboxMcp { /// drop it with `nbox_cache_clear`. cache: Cache, /// Per-user credential vault for write tools (Pattern 2). Absent ⇒ writes - /// are disabled (the read-only default). See [`vault::CredentialVault`]. + /// cannot use the shared HTTP/OIDC path. See [`vault::CredentialVault`]. vault: Option>, + /// Transport/write-mode facts for the write tools. Local writes are allowed + /// only for stdio servers with `local_writes = true`. + write_mode: write::WriteMode, /// The active profile name — bound into the confirmation token so a plan /// from one profile can't be applied under another. Empty for stdio. profile: String, @@ -564,11 +568,34 @@ impl NboxMcp { cache: Cache, vault: Option, profile: String, + ) -> Self { + Self::new_with_write_mode(client, cache, vault, profile, write::WriteMode::Http) + } + + /// Build a stdio server. `local_writes` is the ADR-0002 single-user mode: + /// when true, write tools with no OIDC identity use the active profile token. + pub fn new_stdio(client: NetBoxClient, cache: Cache, local_writes: bool) -> Self { + Self::new_with_write_mode( + client, + cache, + None, + String::new(), + write::WriteMode::stdio(local_writes), + ) + } + + fn new_with_write_mode( + client: NetBoxClient, + cache: Cache, + vault: Option, + profile: String, + write_mode: write::WriteMode, ) -> Self { Self { client: Arc::new(client), cache, vault: vault.map(Arc::new), + write_mode, profile, plans: Arc::new(Mutex::new(write::PlanStore::default())), tool_router: Self::tool_router(), @@ -881,10 +908,10 @@ impl NboxMcp { /// Plan a write operation. Builds a `MutationPlan` (the reviewable diff + /// confirm token) without mutating NetBox. The agent reviews the plan, - /// then calls `nbox_apply_write` to execute it. Requires writes enabled. + /// then calls `nbox_apply_write` to execute it. Requires one write mode. #[tool( name = "nbox_plan_write", - description = "Plan a NetBox write operation (interface description, device status, IP/prefix/IP-range reserve, tag add/remove). Builds a MutationPlan with a before/after diff and a confirm token, without mutating. Review the plan, then call nbox_apply_write to execute. Requires writes enabled (--allow-writes), the caller's token carrying the nbox:write scope, and a [serve.vault] mapping for the caller's OIDC sub; rejected over stdio (no per-user identity).", + description = "Plan a NetBox write operation (interface description, device status, IP/prefix/IP-range reserve, tag add/remove). Builds a MutationPlan with a before/after diff and a confirm token, without mutating. Review the plan, then call nbox_apply_write to execute. Execution is gated: local stdio servers need --local-writes / [serve].local_writes and use the active profile token; shared HTTP/OIDC servers need --allow-writes, the caller's nbox:write scope, and a [serve.vault] mapping for the caller's OIDC sub.", annotations(read_only_hint = false) )] async fn nbox_plan_write( @@ -897,11 +924,11 @@ impl NboxMcp { /// Apply a previously planned write. The submitted plan's `confirm_token` /// looks up the plan this server stored at plan time, and that stored plan is - /// executed under the caller's per-user NetBox identity — the submitted plan - /// contents are not trusted. Returns a `MutationReceipt`. + /// executed under the same write actor — the submitted plan contents are not + /// trusted. Returns a `MutationReceipt`. #[tool( name = "nbox_apply_write", - description = "Apply a previously planned write. Pass back the MutationPlan from nbox_plan_write; its confirm_token looks up the plan this server stored at plan time and applies THAT (the plan contents you submit are not trusted — a forged or edited plan, or one issued for a different caller, is rejected). Executes under the caller's per-user NetBox identity, returning a MutationReceipt. Same gating as nbox_plan_write: writes enabled (--allow-writes), the caller's nbox:write scope, and a [serve.vault] entry for the caller's OIDC sub; rejected over stdio.", + description = "Apply a previously planned write. Pass back the MutationPlan from nbox_plan_write; its confirm_token looks up the plan this server stored at plan time and applies THAT (the plan contents you submit are not trusted — a forged or edited plan, one issued for a different actor, or a replay is rejected). Same gating as nbox_plan_write: local stdio uses --local-writes / [serve].local_writes and the active profile token; shared HTTP/OIDC uses --allow-writes, the caller's nbox:write scope, and a [serve.vault] entry for the caller's OIDC sub.", annotations(read_only_hint = false) )] async fn nbox_apply_write( @@ -1125,10 +1152,16 @@ impl ServerHandler for NboxMcp { .enable_resources() .enable_prompts() .build(); - // The write note reflects THIS instance: write tools are advertised only - // when writes are enabled (a vault is configured); otherwise the server - // is read-only and says so. - let write_note = if self.vault.is_some() { + // The write note reflects THIS instance: tools are always advertised, + // but execution depends on the selected write mode. + let write_note = if matches!( + self.write_mode, + write::WriteMode::Stdio { local_writes: true } + ) { + "Local writes are enabled: nbox_plan_write reviews the before/after diff + confirm \ + token, then nbox_apply_write mutates NetBox under the active profile token; forged \ + or replayed plans are rejected." + } else if self.vault.is_some() { "Writes are opt-in: a caller with the nbox:write scope and a per-user vault entry can \ nbox_plan_write (review the before/after diff + confirm token) then nbox_apply_write \ to mutate NetBox under their own identity; everything else is read-only." @@ -1207,8 +1240,8 @@ impl ServerHandler for NboxMcp { /// Serve the MCP server over stdio until the client disconnects. /// /// stdout is reserved for the JSON-RPC stream; this prints nothing else. -pub async fn serve(client: NetBoxClient, cache: Cache) -> anyhow::Result<()> { - let service = NboxMcp::new(client, cache, None, String::new()) +pub async fn serve(client: NetBoxClient, cache: Cache, local_writes: bool) -> anyhow::Result<()> { + let service = NboxMcp::new_stdio(client, cache, local_writes) .serve(stdio()) .await?; service.waiting().await?; diff --git a/src/mcp/tests.rs b/src/mcp/tests.rs index 4142aa5..23b4c84 100644 --- a/src/mcp/tests.rs +++ b/src/mcp/tests.rs @@ -7,7 +7,7 @@ use rmcp::ErrorData; use rmcp::handler::server::wrapper::{Json, Parameters}; use rmcp::model::{ErrorCode, ResourceContents}; use serde_json::json; -use wiremock::matchers::{method, path, query_param}; +use wiremock::matchers::{header, method, path, query_param}; use wiremock::{Mock, MockServer, ResponseTemplate}; use super::{ @@ -3871,6 +3871,77 @@ fn write_server_for(mock: &MockServer) -> NboxMcp { ) } +/// A stdio-mode server with ADR-0002 local writes toggled, using `token` as the +/// active profile token. This exercises the local single-user path (no OIDC +/// identity, no vault). +fn local_stdio_server_for(mock: &MockServer, local_writes: bool, token: &str) -> NboxMcp { + let profile = ProfileConfig { + url: mock.uri(), + ..Default::default() + }; + NboxMcp::new_stdio( + NetBoxClient::new(&profile, Some(token.to_string())).unwrap(), + crate::cache::Cache::disabled(), + local_writes, + ) +} + +async fn mount_device_status_update_requiring_token(mock: &MockServer, token: &str) { + let auth = format!("Bearer {token}"); + Mock::given(method("GET")) + .and(path("/api/dcim/devices/")) + .and(header("authorization", auth.as_str())) + .respond_with(ResponseTemplate::new(200).set_body_json(json!({ + "count": 1, "next": null, "previous": null, + "results": [{ + "id": 1, "name": "edge01", "slug": "edge01", + "status": {"value": "planned", "label": "Planned"}, + "display": "edge01", "url": "u", "custom_fields": {} + }] + }))) + .mount(mock) + .await; + Mock::given(method("OPTIONS")) + .and(path("/api/dcim/devices/")) + .and(header("authorization", auth.as_str())) + .respond_with(ResponseTemplate::new(200).set_body_json(json!({ + "name": "Device", + "actions": {"POST": {"status": {"type": "choice", "label": "Status", + "choices": [{"value": "active", "display": "Active"}, + {"value": "planned", "display": "Planned"}]}}} + }))) + .mount(mock) + .await; + Mock::given(method("GET")) + .and(path("/api/dcim/devices/1/")) + .and(header("authorization", auth.as_str())) + .respond_with( + ResponseTemplate::new(200) + .set_body_json(json!({ + "id": 1, "name": "edge01", "slug": "edge01", + "status": {"value": "planned", "label": "Planned"}, + "display": "edge01", "url": "u", "custom_fields": {} + })) + .insert_header("ETag", "\"v1\""), + ) + .mount(mock) + .await; + Mock::given(method("PATCH")) + .and(path("/api/dcim/devices/1/")) + .and(header("authorization", auth.as_str())) + .respond_with( + ResponseTemplate::new(200) + .set_body_json(json!({ + "id": 1, "name": "edge01", "slug": "edge01", + "status": {"value": "active", "label": "Active"}, + "display": "edge01", "url": "u", "custom_fields": {} + })) + .insert_header("ETag", "\"v2\""), + ) + .mount(mock) + .await; +} + /// A write caller (the per-request authz facts the tool layer extracts from the /// OIDC identity), for driving `plan_write_impl`/`apply_write_impl` directly. fn caller(sub: &str, has_write_scope: bool) -> crate::mcp::write::WriteCaller { @@ -3987,6 +4058,213 @@ async fn plan_write_rejects_when_no_caller_identity() { ); } +#[tokio::test] +async fn plan_write_rejects_stdio_without_local_writes() { + // Stdio without ADR-0002 local_writes stays read-only. No OIDC identity, no + // vault, and no local fallback means no NetBox request is made. + let mock = MockServer::start().await; + let server = local_stdio_server_for(&mock, false, "nbt_profile.key"); + let result = server + .plan_write_impl( + crate::mcp::write::PlanWriteArgs { + operation: crate::mcp::write::WriteOperation::DeviceStatus { + device: "edge01".into(), + status: "active".into(), + }, + }, + None, + ) + .await; + let err = result + .err() + .expect("stdio without local_writes must reject"); + assert!( + err.message.contains("local stdio writes"), + "error should point at local_writes: {}", + err.message + ); + assert!( + mock.received_requests() + .await + .unwrap_or_default() + .is_empty(), + "a local-writes gate refusal must not touch NetBox" + ); +} + +#[tokio::test] +async fn local_stdio_plan_apply_uses_profile_token() { + // ADR-0002 local mode: no OIDC caller and no vault, but local_writes=true + // lets stdio writes use the active profile token. Every mock endpoint + // requires that token, so the test fails if the path tries a vault token or + // no token. + let mock = MockServer::start().await; + mount_device_status_update_requiring_token(&mock, "nbt_profile.key").await; + let server = local_stdio_server_for(&mock, true, "nbt_profile.key"); + + let plan = server + .plan_write_impl( + crate::mcp::write::PlanWriteArgs { + operation: crate::mcp::write::WriteOperation::DeviceStatus { + device: "edge01".into(), + status: "active".into(), + }, + }, + None, + ) + .await + .expect("local stdio plan should use profile token") + .0; + assert_eq!(plan.target.kind, "device"); + + let receipt = server + .apply_write_impl(crate::mcp::write::ApplyWriteArgs { plan }, None) + .await + .expect("local stdio apply should use the stored plan") + .0; + assert!(receipt.applied); + assert_eq!(receipt.status, 200); +} + +#[tokio::test] +async fn local_stdio_forged_plan_is_rejected() { + // Local writes get the same plan-store protection as OIDC writes: a + // self-consistent, never-issued plan is rejected before any NetBox write. + let mock = MockServer::start().await; + let server = local_stdio_server_for(&mock, true, "nbt_profile.key"); + + use crate::netbox::mutation::{ + MutationPlan, Operation, PlanTarget, Precondition, confirm_token, parse_iso_utc_to_epoch, + }; + let target = PlanTarget { + kind: "device".into(), + r#ref: "edge01".into(), + id: 1, + display: "edge01".into(), + endpoint: "/api/dcim/devices/1/".into(), + profile: String::new(), + }; + let precondition = Precondition::Etag { + etag: "\"v1\"".into(), + }; + let patch = json!({"name": "pwned"}); + let expires_at = "2999-01-01T00:00:00Z"; + let epoch = parse_iso_utc_to_epoch(expires_at).unwrap(); + let token = confirm_token( + &target, + Operation::Update, + &precondition, + &patch, + &None, + 1, + epoch, + ); + let forged = MutationPlan { + schema_version: 1, + operation: Operation::Update, + target, + precondition, + fields: vec![], + patch, + no_op: false, + warnings: vec![], + errors: vec![], + changelog_message: None, + count: 1, + confirm_token: token, + expires_at: expires_at.into(), + }; + assert!(forged.verify().is_ok(), "forged plan is self-consistent"); + + let result = server + .apply_write_impl(crate::mcp::write::ApplyWriteArgs { plan: forged }, None) + .await; + let err = result.err().expect("forged local plan must be rejected"); + assert!( + err.message.contains("no write plan matches"), + "unexpected error: {}", + err.message + ); + assert!( + mock.received_requests() + .await + .unwrap_or_default() + .is_empty(), + "forged plan rejection must not touch NetBox" + ); +} + +#[tokio::test] +async fn local_stdio_plan_is_one_shot() { + let mock = MockServer::start().await; + mount_device_status_update_requiring_token(&mock, "nbt_profile.key").await; + let server = local_stdio_server_for(&mock, true, "nbt_profile.key"); + + let plan = server + .plan_write_impl( + crate::mcp::write::PlanWriteArgs { + operation: crate::mcp::write::WriteOperation::DeviceStatus { + device: "edge01".into(), + status: "active".into(), + }, + }, + None, + ) + .await + .expect("local plan") + .0; + + server + .apply_write_impl( + crate::mcp::write::ApplyWriteArgs { plan: plan.clone() }, + None, + ) + .await + .expect("first apply succeeds"); + let replay = server + .apply_write_impl(crate::mcp::write::ApplyWriteArgs { plan }, None) + .await; + let err = replay.err().expect("second apply must be rejected"); + assert!( + err.message.contains("no write plan matches"), + "replay should miss the consumed plan: {}", + err.message + ); +} + +#[tokio::test] +async fn local_writes_do_not_bypass_oidc_vault_path() { + // Presence of an OIDC caller disambiguates to Pattern 2. Even on a stdio + // server with local_writes=true, a caller identity must satisfy the vault + // path; it must not silently fall back to the profile token. + let mock = MockServer::start().await; + let server = local_stdio_server_for(&mock, true, "nbt_profile.key"); + let result = server + .plan_write_impl( + crate::mcp::write::PlanWriteArgs { + operation: crate::mcp::write::WriteOperation::DeviceStatus { + device: "edge01".into(), + status: "active".into(), + }, + }, + Some(caller("alice", true)), + ) + .await; + let err = result.err().expect("OIDC caller must require vault"); + assert!( + err.message.contains("shared writes are not enabled"), + "identity-present path should not use local_writes: {}", + err.message + ); + assert!( + mock.received_requests() + .await + .unwrap_or_default() + .is_empty(), + "vault refusal must not touch NetBox" + ); +} + #[tokio::test] async fn plan_write_rejects_when_missing_write_scope() { // A caller with a valid sub + vault entry but no `nbox:write` scope is diff --git a/src/mcp/write.rs b/src/mcp/write.rs index c86cfa3..2d1124a 100644 --- a/src/mcp/write.rs +++ b/src/mcp/write.rs @@ -1,4 +1,5 @@ -//! MCP write tools (Pattern 2, DESIGN §24) — plan-first, per-user identity. +//! MCP write tools — plan-first writes over either local stdio or Pattern 2 +//! per-user identity. //! //! Two operation-specific tools mirror the CLI's two-step safe-write flow: //! @@ -8,13 +9,15 @@ //! 2. `nbox_apply_write` — verifies the plan's confirm token and applies it, //! returning a [`MutationReceipt`]. //! -//! Per-user identity bridging: the caller's OIDC `sub` is resolved to a -//! per-user NetBox token via [`crate::mcp::vault::CredentialVault`], then -//! bridged into a temporary [`NetBoxClient`] via [`NetBoxClient::with_token`] -//! so the write hits NetBox under the caller's identity. +//! In shared HTTP/OIDC mode, the caller's OIDC `sub` is resolved to a per-user +//! NetBox token via [`crate::mcp::vault::CredentialVault`], then bridged into a +//! temporary [`NetBoxClient`] via [`NetBoxClient::with_token`] so the write hits +//! NetBox under the caller's identity. In ADR-0002 local stdio mode, there is no +//! OIDC caller; the write uses the active profile token and binds the stored plan +//! to a synthetic local actor. //! //! The tools reuse the exact same `plan_*`/`apply_*` engine the CLI uses -//! (ADR-0001) — no separate write path. The vault is the only new layer. +//! (ADR-0001) — no separate write path. use std::collections::HashMap; @@ -50,6 +53,39 @@ pub(crate) enum ApplierKind { Tag, } +/// The principal a write plan is issued to. OIDC writes bind to the caller's +/// stable subject; local stdio writes bind to one synthetic single-user +/// principal. The key is internal to the plan store, not exposed as a credential. +#[derive(Clone, Debug, PartialEq, Eq)] +pub(crate) enum WriteActor { + Oidc { sub: String }, + Local, +} + +impl WriteActor { + pub(crate) fn key(&self) -> String { + match self { + WriteActor::Oidc { sub } => format!("sub:{sub}"), + WriteActor::Local => "local".to_string(), + } + } +} + +/// Transport/write-mode facts known by the server instance. ADR-0002's first +/// cut enables local writes only for stdio; HTTP no-identity writes remain +/// rejected, even on loopback. +#[derive(Clone, Copy, Debug, PartialEq, Eq)] +pub(crate) enum WriteMode { + Http, + Stdio { local_writes: bool }, +} + +impl WriteMode { + pub(crate) fn stdio(local_writes: bool) -> Self { + Self::Stdio { local_writes } + } +} + /// Server-issued write plans awaiting `nbox_apply_write`. /// /// A plan's `confirm_token` is a non-secret SHA over the plan's own fields @@ -70,8 +106,8 @@ pub(crate) struct PlanStore { struct StoredPlan { plan: MutationPlan, - /// The OIDC `sub` that planned it — apply must come from the same caller. - sub: String, + /// Internal write-actor key that planned it — apply must come from the same actor. + actor_key: String, applier: ApplierKind, /// Monotonic issue order, for capacity eviction (oldest first). seq: u64, @@ -90,7 +126,7 @@ const PLAN_STORE_CAP: usize = 256; impl PlanStore { /// Record a freshly issued plan, evicting the oldest when at capacity. - pub(crate) fn record(&mut self, plan: MutationPlan, sub: String, applier: ApplierKind) { + pub(crate) fn record(&mut self, plan: MutationPlan, actor: &WriteActor, applier: ApplierKind) { if self.issued.len() >= PLAN_STORE_CAP && !self.issued.contains_key(&plan.confirm_token) && let Some(oldest) = self @@ -107,26 +143,27 @@ impl PlanStore { plan.confirm_token.clone(), StoredPlan { plan, - sub, + actor_key: actor.key(), applier, seq, }, ); } - /// Consume the plan this server issued for `token`, requiring the same caller - /// `sub`. One-shot: a matched plan is removed (no replay). The caller's own + /// Consume the plan this server issued for `token`, requiring the same write + /// actor. One-shot: a matched plan is removed (no replay). The caller's own /// plan contents are never trusted — only its token keys the lookup. pub(crate) fn consume( &mut self, token: &str, - sub: &str, + actor: &WriteActor, ) -> Result<(MutationPlan, ApplierKind), ConsumeError> { + let actor_key = actor.key(); match self.issued.get(token) { None => Err(ConsumeError::NotFound), - // A mismatched caller must not consume (the rightful caller can still + // A mismatched actor must not consume (the rightful actor can still // apply) — reject without removing. - Some(s) if s.sub != sub => Err(ConsumeError::WrongCaller), + Some(s) if s.actor_key != actor_key => Err(ConsumeError::WrongCaller), Some(_) => { let s = self.issued.remove(token).expect("just checked present"); Ok((s.plan, s.applier)) @@ -145,7 +182,7 @@ impl ConsumeError { None, ), ConsumeError::WrongCaller => ErrorData::invalid_params( - "this write plan was issued for a different caller identity; re-plan with \ + "this write plan was issued for a different write actor; re-plan with \ nbox_plan_write", None, ), @@ -276,54 +313,70 @@ pub(crate) struct WriteCaller { } impl NboxMcp { - /// Resolve the caller's per-user NetBox client via the vault, or reject - /// with a clear error if writes are disabled or the caller has no vault - /// entry. Returns a short-lived `NetBoxClient` clone with the per-user - /// token swapped in. - /// Resolve the caller's per-user NetBox client, enforcing the full write - /// authorization ladder — fail-closed at every step (ADR-0001 §7): + /// Resolve the NetBox client and write actor for an MCP write, enforcing the + /// appropriate gate for the request shape: /// - /// 1. writes enabled at all (`[serve].allow_writes` → a vault is present); - /// 2. the request carried an authenticated caller (HTTP+OIDC; stdio and - /// loopback static-bearer have no per-user identity → rejected); - /// 3. the caller's token carries the `nbox:write` scope; - /// 4. the caller's `sub` maps to a provisioned per-user NetBox token. + /// - OIDC caller present: Pattern 2, unchanged — require `nbox:write`, a + /// vault entry, and use the per-user NetBox token. + /// - No caller + stdio + `local_writes`: ADR-0002 local single-user mode — + /// use the active profile token and bind the plan to the synthetic `local` + /// actor. + /// - Everything else rejects clearly before touching NetBox. /// - /// The service token is never used for writes. A `None` caller (no identity) - /// gets a distinct error that never suggests mapping a placeholder `sub`. - fn bridged_client(&self, caller: Option) -> Result { + /// The profile token is used for writes only in explicit stdio local mode. + fn write_client( + &self, + caller: Option, + ) -> Result<(NetBoxClient, WriteActor), ErrorData> { + if let Some(caller) = caller { + let vault = self.vault.as_ref().ok_or_else(|| { + ErrorData::invalid_params( + "MCP shared writes are not enabled on this nbox serve instance; \ + set [serve].allow_writes = true or pass --allow-writes, \ + and provision [serve.vault] entries for each caller's OIDC sub", + None, + ) + })?; + if !caller.has_write_scope { + return Err(ErrorData::invalid_params( + format!( + "the caller's token is missing the required `{}` scope for MCP writes", + crate::mcp::SCOPE_WRITE + ), + None, + )); + } + let token = vault + .resolve(&caller.sub) + .map_err(|e| ErrorData::invalid_params(e.to_string(), None))?; + return Ok(( + (*self.client) + .clone() + .with_token(token.as_str().to_string()), + WriteActor::Oidc { sub: caller.sub }, + )); + } + + if matches!(self.write_mode, WriteMode::Stdio { local_writes: true }) { + return Ok(((*self.client).clone(), WriteActor::Local)); + } + let vault = self.vault.as_ref().ok_or_else(|| { ErrorData::invalid_params( "MCP writes are not enabled on this nbox serve instance; \ - set [serve].allow_writes = true or pass --allow-writes, \ - and provision [serve.vault] entries for each caller's OIDC sub", + for local stdio writes set [serve].local_writes = true or pass --local-writes; \ + for shared HTTP writes set [serve].allow_writes = true or pass --allow-writes \ + and provision [serve.vault] entries", None, ) })?; - let caller = caller.ok_or_else(|| { - ErrorData::invalid_params( - "MCP writes require an authenticated OIDC caller identity; this request \ - carried none. Writes are unavailable over the stdio transport and over \ - loopback static-bearer auth — use the HTTP transport with OIDC so each \ - write is attributed to a real NetBox user.", - None, - ) - })?; - if !caller.has_write_scope { - return Err(ErrorData::invalid_params( - format!( - "the caller's token is missing the required `{}` scope for MCP writes", - crate::mcp::SCOPE_WRITE - ), - None, - )); - } - let token = vault - .resolve(&caller.sub) - .map_err(|e| ErrorData::invalid_params(e.to_string(), None))?; - Ok((*self.client) - .clone() - .with_token(token.as_str().to_string())) + let _ = vault; + Err(ErrorData::invalid_params( + "MCP shared writes require an authenticated OIDC caller identity; this request \ + carried none. HTTP and static-bearer transports cannot use local_writes in this \ + release.", + None, + )) } /// Plan a write operation. Builds a `MutationPlan` without mutating. @@ -332,10 +385,7 @@ impl NboxMcp { args: PlanWriteArgs, caller: Option, ) -> Result, ErrorData> { - // Capture the planner's identity before the caller is consumed by the - // auth ladder — apply requires the same `sub` to apply this plan. - let planner_sub = caller.as_ref().map(|c| c.sub.clone()); - let client = self.bridged_client(caller)?; + let (client, actor) = self.write_client(caller)?; let profile = self.profile.as_str(); let (applier, plan_result) = match args.operation { WriteOperation::InterfaceDescription { @@ -461,12 +511,10 @@ impl NboxMcp { let plan = plan_result.map_err(super::to_mcp_error)?; // Record the server-issued plan so apply can trust it — the caller's // submitted plan contents are never used beyond the confirm_token. - if let Some(sub) = planner_sub { - self.plans - .lock() - .expect("plan store mutex poisoned") - .record(plan.clone(), sub, applier); - } + self.plans + .lock() + .expect("plan store mutex poisoned") + .record(plan.clone(), &actor, applier); Ok(Json(plan)) } @@ -479,12 +527,7 @@ impl NboxMcp { args: ApplyWriteArgs, caller: Option, ) -> Result, ErrorData> { - // Capture the caller's identity, then enforce the full write - // authorization ladder (writes enabled, authenticated caller, write - // scope, vault entry) and bind the per-user NetBox token. - let caller_sub = caller.as_ref().map(|c| c.sub.clone()); - let client = self.bridged_client(caller)?; - let sub = caller_sub.expect("bridged_client succeeded ⇒ caller present"); + let (client, actor) = self.write_client(caller)?; // Apply the plan this server issued for the token — never the caller's // contents. A forged/tampered plan has no matching server-stored entry. @@ -492,7 +535,7 @@ impl NboxMcp { .plans .lock() .expect("plan store mutex poisoned") - .consume(&args.plan.confirm_token, &sub) + .consume(&args.plan.confirm_token, &actor) .map_err(ConsumeError::into_mcp)?; // Defense in depth: the stored plan must still pass its own integrity + diff --git a/tests/mcp_serve_http_tests.rs b/tests/mcp_serve_http_tests.rs index 1dec155..c719720 100644 --- a/tests/mcp_serve_http_tests.rs +++ b/tests/mcp_serve_http_tests.rs @@ -631,6 +631,67 @@ async fn mount_jwks(idp: &TestIdp) -> MockServer { jwks } +#[test] +fn http_local_writes_is_rejected_without_oidc() { + // ADR-0002 first cut is stdio-only. Even loopback HTTP must not get + // profile-token writes from --local-writes. + let port = free_port(); + let addr = format!("127.0.0.1:{port}"); + let mut config = NamedTempFile::new().expect("create temp config"); + write!( + config, + "active_profile = \"test\"\n\ + \n\ + [profiles.test]\n\ + url = \"http://127.0.0.1:1/\"\n" + ) + .expect("write temp config"); + config.flush().expect("flush temp config"); + + let mut child = Command::new(env!("CARGO_BIN_EXE_nbox")) + .arg("--config") + .arg(config.path()) + .arg("serve") + .arg("--http") + .arg(&addr) + .arg("--local-writes") + .env("NBOX_TOKEN", "dummy") + .env_remove("NBOX_SERVE_TOKEN") + .env_remove("NBOX_LOG") + .env_remove("RUST_LOG") + .stdin(Stdio::null()) + .stdout(Stdio::piped()) + .stderr(Stdio::piped()) + .spawn() + .expect("spawn nbox serve --http --local-writes"); + + for _ in 0..100 { + if child.try_wait().expect("poll child").is_some() { + let out = child.wait_with_output().expect("collect output"); + assert_eq!(out.status.code(), Some(2), "stderr: {:?}", out.stderr); + let stderr = String::from_utf8_lossy(&out.stderr); + assert!( + stderr.contains("--local-writes") && stderr.contains("stdio"), + "stderr should explain stdio-only local writes: {stderr}" + ); + assert!( + out.stdout.is_empty(), + "usage failure must keep stdout clean: {:?}", + out.stdout + ); + return; + } + std::thread::sleep(Duration::from_millis(20)); + } + let _ = child.kill(); + let out = child.wait_with_output().expect("collect killed output"); + panic!( + "nbox serve --http --local-writes did not fail fast; status={:?} stderr={}", + out.status, + String::from_utf8_lossy(&out.stderr) + ); +} + #[tokio::test] async fn oidc_write_bridges_caller_identity_to_per_user_token() { let idp = make_idp(); diff --git a/tests/mcp_serve_tests.rs b/tests/mcp_serve_tests.rs index 4b0d6c0..7ec78e3 100644 --- a/tests/mcp_serve_tests.rs +++ b/tests/mcp_serve_tests.rs @@ -27,6 +27,8 @@ use std::time::Duration; use serde_json::{Value, json}; use tempfile::NamedTempFile; +use wiremock::matchers::{header, method, path}; +use wiremock::{Mock, MockServer, ResponseTemplate}; /// The protocol version the server advertises (`ProtocolVersion::LATEST` in /// `src/mcp/mod.rs`). The server negotiates down to the client's version if it @@ -74,25 +76,32 @@ impl ServeChild { /// URL is unreachable on purpose: the initialize/tools/list handshake never /// makes a network call, so the bogus URL is fine and keeps the test offline. fn spawn() -> Self { + Self::spawn_with("http://127.0.0.1:1/", &[], "dummy") + } + + /// Spawn `nbox serve` against a caller-provided NetBox base URL and extra + /// serve args. Used by tests that need the real stdio transport plus a + /// wiremock NetBox. + fn spawn_with(url: &str, serve_args: &[&str], token: &str) -> Self { let mut config = NamedTempFile::new().expect("create temp config"); write!( config, "active_profile = \"test\"\n\ \n\ [profiles.test]\n\ - url = \"http://127.0.0.1:1/\"\n\ + url = \"{url}\"\n\ token_env = \"NBOX_TEST_TOKEN_UNUSED\"\n" ) .expect("write temp config"); config.flush().expect("flush temp config"); - let mut child = Command::new(env!("CARGO_BIN_EXE_nbox")) - .arg("--config") - .arg(config.path()) - .arg("serve") - // The direct-override token env so `connect()` finds a token without - // a real secret; nothing authenticates during the handshake. - .env("NBOX_TOKEN", "dummy") + let mut command = Command::new(env!("CARGO_BIN_EXE_nbox")); + command.arg("--config").arg(config.path()).arg("serve"); + command.args(serve_args); + let mut child = command + // The direct-override token env so `connect()` finds a token. Tests + // that hit wiremock can require this exact token on every request. + .env("NBOX_TOKEN", token) // Pin logging quiet and to stderr regardless of the caller's env, so // the stdout-cleanliness assertion isn't perturbed by NBOX_LOG/RUST_LOG. .env_remove("NBOX_LOG") @@ -196,6 +205,105 @@ impl Drop for ServeChild { } } +async fn mount_device_status_update_requiring_token(mock: &MockServer, token: &str) { + let auth = format!("Bearer {token}"); + Mock::given(method("GET")) + .and(path("/api/dcim/devices/")) + .and(header("authorization", auth.as_str())) + .respond_with(ResponseTemplate::new(200).set_body_json(json!({ + "count": 1, + "next": null, + "previous": null, + "results": [{ + "id": 1, + "name": "edge01", + "slug": "edge01", + "status": {"value": "planned", "label": "Planned"}, + "display": "edge01", + "url": "u", + "custom_fields": {} + }] + }))) + .mount(mock) + .await; + Mock::given(method("OPTIONS")) + .and(path("/api/dcim/devices/")) + .and(header("authorization", auth.as_str())) + .respond_with(ResponseTemplate::new(200).set_body_json(json!({ + "name": "Device", + "actions": { + "POST": { + "status": { + "type": "choice", + "label": "Status", + "choices": [ + {"value": "active", "display": "Active"}, + {"value": "planned", "display": "Planned"} + ] + } + } + } + }))) + .mount(mock) + .await; + Mock::given(method("GET")) + .and(path("/api/dcim/devices/1/")) + .and(header("authorization", auth.as_str())) + .respond_with( + ResponseTemplate::new(200) + .set_body_json(json!({ + "id": 1, + "name": "edge01", + "slug": "edge01", + "status": {"value": "planned", "label": "Planned"}, + "display": "edge01", + "url": "u", + "custom_fields": {} + })) + .insert_header("ETag", "\"v1\""), + ) + .mount(mock) + .await; + Mock::given(method("PATCH")) + .and(path("/api/dcim/devices/1/")) + .and(header("authorization", auth.as_str())) + .respond_with( + ResponseTemplate::new(200) + .set_body_json(json!({ + "id": 1, + "name": "edge01", + "slug": "edge01", + "status": {"value": "active", "label": "Active"}, + "display": "edge01", + "url": "u", + "custom_fields": {} + })) + .insert_header("ETag", "\"v2\""), + ) + .mount(mock) + .await; +} + +/// Extract a successful tool's structured payload (the `Json` it returns). +fn tool_payload(msg: &Value) -> Value { + assert!(msg.get("error").is_none(), "tool returned an error: {msg}"); + let result = &msg["result"]; + assert_ne!( + result["isError"], + json!(true), + "tool execution error: {msg}" + ); + if let Some(sc) = result.get("structuredContent") + && !sc.is_null() + { + return sc.clone(); + } + let text = result["content"][0]["text"] + .as_str() + .unwrap_or_else(|| panic!("tool result has no structuredContent/text: {msg}")); + serde_json::from_str(text).unwrap_or_else(|_| panic!("tool result text not JSON: {text}")) +} + #[test] fn serve_handshake_lists_all_tools_with_clean_stdout() { let mut server = ServeChild::spawn(); @@ -378,3 +486,76 @@ fn serve_handshake_lists_all_tools_with_clean_stdout() { // Close stdin and make sure the process exits (killed as a backstop). server.shutdown(); } + +#[tokio::test(flavor = "multi_thread")] +async fn local_stdio_writes_plan_and_apply_through_json_rpc() { + let netbox = MockServer::start().await; + mount_device_status_update_requiring_token(&netbox, "nbt_profile.key").await; + let mut server = ServeChild::spawn_with(&netbox.uri(), &["--local-writes"], "nbt_profile.key"); + + server.send(&json!({ + "jsonrpc": "2.0", + "id": 1, + "method": "initialize", + "params": { + "protocolVersion": PROTOCOL_VERSION, + "capabilities": {}, + "clientInfo": { "name": "nbox-local-write-e2e-test", "version": "0.0.0" } + } + })); + let init = server.read_response(1); + assert!( + init["result"]["instructions"] + .as_str() + .unwrap_or_default() + .contains("Local writes are enabled"), + "initialize instructions should describe local writes: {init}" + ); + server.send(&json!({ + "jsonrpc": "2.0", + "method": "notifications/initialized" + })); + + server.send(&json!({ + "jsonrpc": "2.0", + "id": 2, + "method": "tools/call", + "params": { + "name": "nbox_plan_write", + "arguments": { + "operation": { + "kind": "device_status", + "device": "edge01", + "status": "active" + } + } + } + })); + let plan_msg = server.read_response(2); + let plan = tool_payload(&plan_msg); + assert_eq!(plan["target"]["kind"], "device", "plan: {plan}"); + assert_eq!(plan["patch"], json!({"status": "active"}), "plan: {plan}"); + assert!( + !plan["confirm_token"] + .as_str() + .unwrap_or_default() + .is_empty(), + "plan carries a confirm token" + ); + + server.send(&json!({ + "jsonrpc": "2.0", + "id": 3, + "method": "tools/call", + "params": { + "name": "nbox_apply_write", + "arguments": { "plan": plan } + } + })); + let receipt_msg = server.read_response(3); + let receipt = tool_payload(&receipt_msg); + assert_eq!(receipt["applied"], true, "receipt: {receipt}"); + assert_eq!(receipt["status"], 200, "receipt: {receipt}"); + + server.shutdown(); +}