From 254a67be5f459ef5a21461da792aa6b08af4be4e Mon Sep 17 00:00:00 2001 From: arpan Date: Wed, 16 Sep 2026 14:28:34 +0530 Subject: [PATCH] Let the operator console speak stdio, with the OS login as the approver MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Desktop assistants, Cursor and the editors launch an MCP server as a subprocess and speak to it on stdin and stdout. This server spoke only loopback HTTP, so the registry listing that went live at 0.12.2 described a server none of those clients could start. `--stdio` opens no socket, which is stricter than §2.1 and not a loosening of it. Every message goes through `handle` exactly as a POST body does: the same parser, the same refusals, the same identity gate, the same store calls. The loop adds only what HTTP carried in headers and a pipe cannot: the revision, negotiated once at `initialize` from the body and only when that initialize was accepted; the mirrored `Mcp-Method` and `Mcp-Name` that 2026-07-28 requires, synthesised from the body they would have to agree with; and a bound on the line that holds on the allocation, not only the decision. §10 excluded stdio because a process the assistant launched has no credential, and every identity the client could offer is asserted by it. That is still true and none of them is used. What it missed is that the client does not choose everything about the process it starts: the kernel chooses the uid, and the account behind it is one the client already holds. `OsLoginIdentityProvider` reads the real uid's login and nothing else: not `getpass.getuser()`, which believes `USER` and `LOGNAME` first, not `clientInfo`, not `SUDO_USER`. T571 forges every one of those and asserts the principal is unmoved. What that does and does not promise is stated rather than implied: a client that can subvert the process can already open the store as that account, so nothing is added to what it can do, only a name on what it did. Root is an account and not a person, so under uid 0 every write is refused and reads still answer. The login carries no roles and no expiry, and the client process therefore holds approve, deny and resolve under it for as long as it runs; the `initialize` instructions and the startup block both say so, because the confirmation the client shows before a write is then the only human step. Every flag that names a header is refused with `--stdio`, by name, as a flag that could not take effect, and `--max-body-bytes` has a floor on both transports. The startup block goes to stderr, because the first stdio client this was tried behind logged "ignoring non-JSON output" for every line of it on stdout. A client that closes stdout ends the loop the way EOF on stdin does. The registry manifest now describes this: `uvx ctrlrun mcp-operator --stdio` with `CTRLRUN_CONFIG`, which is a block a desktop client can install. The old manifest's loopback-HTTP entry was not something any client could start from a listing. Reviewed as an identity change, in a session that did not write it: eleven findings, eight accepted and three declined, recorded in SPEC-mcp-operator §9.5. §2.3 and §3.1 carry the design; §10's exclusion is struck through with the reasoning that replaced it. T570 to T574. Signed-off-by: arpan --- CHANGELOG.md | 25 ++ docs/SPEC-mcp-operator.md | 219 ++++++++++++- server.json | 19 +- src/ctrlrun/cli/main.py | 16 +- src/ctrlrun/gateway/__init__.py | 67 ++-- src/ctrlrun/gateway/mcp.py | 10 +- src/ctrlrun/gateway/operator.py | 326 +++++++++++++++++++- tests/test_mcp_operator.py | 506 ++++++++++++++++++++++++++++++- tests/test_repository_signals.py | 9 + 9 files changed, 1160 insertions(+), 37 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 31dbb566..ec13b5b9 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,31 @@ any change to one appears here. ## [Unreleased] +### Added + +- **`ctrlrun mcp-operator --stdio`: the operator console for the client that launched it.** + Desktop assistants, Cursor and the editors launch an MCP server as a subprocess and speak to it on + stdin and stdout; until now this server spoke only HTTP on loopback, so the registry listing + that went live at 0.12.2 described a server none of those clients could start. `--stdio` opens + no socket at all, which is stricter than the loopback rule and not a loosening of it, and every + message still goes through the same parser, the same refusals and the same store calls as a + POST body does. The approver is the account the process runs as, read from the **real uid** + and from nothing the client sends or sets: not `getpass.getuser()`, which believes the + environment, not `clientInfo`, not `SUDO_USER`. What that does and does not promise is stated + rather than implied: a client that can subvert the process can already open the store as that + account, so nothing is added to what it can do, only a name on what it did. Root is an account + and not a person, so under uid 0 every write is refused and reads still answer. The login + carries no roles and no expiry, and says so: a control naming an `approver_role` refuses over + stdio, and the client process holds approve, deny and resolve under the login for as long as it + runs, which the `initialize` instructions and the startup block both say, because the + confirmation the client shows before a write is then the only human step. Every flag that + names a header is refused with it, by name, and `--max-body-bytes` now has a floor on both + transports. `SPEC-mcp-operator.md` §2.3 and §3.1 carry the + design, and §10's exclusion of stdio is struck through with the reasoning that replaced it + rather than deleted. The registry manifest now describes this: `uvx ctrlrun mcp-operator + --stdio` with `CTRLRUN_CONFIG` pointing at your policy, which is a config block a desktop + client can install. + ## [0.12.2] — the operator's tool descriptions, and the marker the registry reads A patch release with no change to the enforcement path. Two things sat on `main` with no way for diff --git a/docs/SPEC-mcp-operator.md b/docs/SPEC-mcp-operator.md index 392a4650..238426ef 100644 --- a/docs/SPEC-mcp-operator.md +++ b/docs/SPEC-mcp-operator.md @@ -113,11 +113,62 @@ says so (§6). It is the same boundary the store file already has. the whole deployment this exists for — and two servers whose default ports collide produce a bind error at the worst moment, or worse, a client pointed at the wrong one. +### 2.3 stdio — added 2026-09-16 + +`ctrlrun mcp-operator --stdio` speaks MCP on stdin and stdout to the one client that launched +the process, and opens **no socket**. That is what a desktop client does — a desktop assistant, +Cursor, an editor — and it is the shape the registry listing has to describe for any of them to +install this at all. §10 excluded it, and the exclusion is struck below with the reasoning that +replaced it; the short version is that a transport with no port is stricter than §2.1, not a +loosening of it, and that the identity question §10 asked has an answer §3.1 now gives. + +**The framing** is the transport specification's: one JSON-RPC message per line, UTF-8, no +embedded newline. Every line this server writes to stdout is a JSON-RPC message and nothing +else is, because the client parses the stream and a line of text on it is a broken message to +the client rather than information — the first stdio client this was tried behind logged +*"ignoring non-JSON output"* for every line of the §6 block. So over stdio the §6 block, the +observe banner and every log line go to **stderr**, and T574 asserts stdout parses whole. + +**Every message goes through `handle`**, exactly as a POST body does — the same parser, the same +refusals, the same identity gate (§3.3) and the same store calls. What the loop adds is only what +HTTP carried in headers and a pipe cannot: + +| Over HTTP | Over stdio | +|---|---| +| `MCP-Protocol-Version` on every request; an unaccepted one is refused `-32022` | Negotiated **once**, at `initialize`, from `params.protocolVersion`: the client's revision where it is one this server accepts, else `2026-07-28`. The transport specification's rule, not the HTTP path's: a server that does not support the requested version answers with one it does and the client decides, because a desktop client refused outright has no version at all to decide about (T570) | +| `Mcp-Method` and `Mcp-Name`, required on `2026-07-28`, validated against the body (§6.4) | Synthesised **from the body** they would have to agree with, so the validation runs and cannot fail. The check exists because an HTTP proxy can set a header independently of the body; a pipe has no proxy | +| `Origin`, validated against `--allow-origin` | No origin over a pipe. `--allow-origin` is refused with `--stdio` (§9.4) | +| A body over `--max-body-bytes` is HTTP 413, read no further | A line over `--max-body-bytes` is refused **unread**: `readline(limit + 2)` bounds the allocation (two being the longest line ending, so a CRLF client keeps its whole budget), the rest of the line is drained without being decoded, and the client gets `-32600` with a null id. The bound is on the allocation and not only the decision, over this transport as over the other (T573) | +| A refusal is the store's, or `_call`'s, or `-32603` for anything else (§7) | The same, and anything `handle` raises outside `_call`'s own net is `-32603` with the message's id rather than the process dying, so §7's last row holds here too | +| A client that goes away closes the socket | A client that closes stdout is a client that went away: the write fails, the loop returns, the process exits 0, exactly as at EOF on stdin | +| A refusal with no body (403, 413) | Becomes a JSON-RPC error with whatever id the line carried. A request that gets no line is a request the client waits on for ever | +| A notification is HTTP 202, no body | No line | +| `Mcp-Session-Id` never minted | Still never. There is one client and it is the one that launched the process | + +Reaching EOF on stdin is the client going away, and the process exits 0. + +**What stdio costs, and where it is said.** Over HTTP a human's credential is per request and +expires; over stdio the client process holds `approve`, `deny` and `resolve` under the human's +name for as long as it runs, `-41007`, `-41013` (bar root, §3.1) and `-41014` are unreachable, +and the only human step left is the confirmation the client shows before a write — which +CTRLRun does not control and which a user can switch off. That is the auto-approve §1.1 +refuses, reachable by client configuration, and this document does not pretend otherwise. It is +said in three places so that each party sees it: here; in the `initialize` `instructions`, which +the model reads; and in the §6 block, which the person reads. A lifetime after which writes +refuse until relaunch (`--stdio-max-age`) was proposed by the review and is not built: it is a +new flag with its own semantics to specify, and a client that re-launches the process on a +timer defeats it, so it would be a promise with a hole in it. Recorded in §10. + +**What does not change**: the eight tools, their schemas, every refusal code, every store call, +the evidence written. `serve_operator_forever` runs whichever transport the config names, and +the rest of the server cannot tell which it is under. + ## 3. Identity ### 3.1 The provider is the operator's, and there are two of them Exactly one of `--principal-header` or `--identity-jwt` MUST be given. There is no default. +Over `--stdio` neither is accepted, and the provider is the third one, below. **`--principal` is refused**, and this is the second place the operator server is stricter than the gateway. `StaticIdentityProvider` answers with the same `Principal` for every request @@ -130,6 +181,55 @@ at startup rather than discovered in a receipt. (`v0.3 §8.2`), construct exactly the same providers, and carry exactly the same warnings. A header is worth what the proxy that sets it is worth. +**Over `--stdio` the provider is `OsLoginIdentityProvider` — added 2026-09-16.** There are no +headers on a pipe, so neither flag above can apply, and §10's original objection stands as +written: a process launched by the assistant has no credential to verify, and *every identity +the client could offer* is asserted by it — `clientInfo`, an argument, an environment variable. +None of those is used. What is used is the account the process is running as: the login the +password database gives for the process's **real uid**, `pwd.getpwuid(os.getuid())` on POSIX, +and the token's user on Windows. Not `getpass.getuser()`, which believes `USER` and `LOGNAME` +first, and the environment is the client's to set; not `os.getlogin()` on POSIX, which reads a +controlling terminal a desktop-launched process does not have; not `SUDO_USER`, which is the +environment again. The **real** uid and not the effective one, because it names the account +that launched the process rather than what a setuid file grants. T571 sets every one of those +variables to a forged name and asserts the principal is unmoved. + +**And no more than that.** The uid cannot be chosen by the client. The *name the process +reports for it* is only as trustworthy as the process, and a client that controls the +interpreter's environment — `PYTHONPATH` and a `sitecustomize`, a preloaded library, what `uvx` +installs beside the package — controls the process. The review that found this is right, and +the earlier draft's "the one name the client cannot choose" was an overclaim and is gone. What +makes the design sound is the boundary, not the lookup: a client that can do any of that can +already open the store as this account and answer with `ctrlrun approve`, so the attribution +string was already within its reach at the file. Nothing is added to what it can do; what is +added is a name on what it did. + +**Root is an account, not a person.** `sudo ctrlrun mcp-operator --stdio`, or a container +running as uid 0, would record every answer as `root`, which distinguishes nobody — this +document's own objection to `--principal`. So under uid 0 the principal carries no `user`, every +write is refused as `-41013` exactly as a machine credential is, the reads still answer, and the +§6 block says so in capitals. T571 pins it with `SUDO_USER` set, to make the point that it is +ignored. + +It is not `--principal` in another costume, and the difference is where the name comes from. A +static principal is whatever was typed after the flag, so every approval carries a string that +distinguishes nobody. This one distinguishes people at the process boundary: two people on one +host get two logins, and over stdio one client is one process is one login, so the boundary is +the same one a per-request credential draws over HTTP. And it is the boundary the store file +already has — §2.1's own words, "loopback is not a trust boundary against other processes on the +same host" — because a process that can run this as you can already open the store as you and +answer with `ctrlrun approve`. It adds no surface; it opens no port. + +The principal it returns: `agent` and `user` both the login, `issuer` `os-login:`, so +evidence can tell an answer given this way from one behind a proxy's header or a token's `iss` +at a glance. **What it cannot do is stated rather than implied.** An OS login carries no claims, +so no role can be read from it, `--approver-roles-claim` is refused with `--stdio` (§9.4), and +every control naming an `approver_role` refuses over stdio — the fail-closed direction, and +`OperatorServer`'s startup warning names the controls. It has no `expires_at`: a login session is +not a credential with a lifetime the process can see. And a uid with no entry in the password +database refuses to start, naming the uid, on §3.2's logic — a server whose write tools could +never succeed is refused where it can still be fixed. + ### 3.2 The credential must name a human A write tool's principal MUST have a non-`None` `user`. An `agent` with no `user` is a machine @@ -429,6 +529,11 @@ write tools approve, deny, resolve — each needs a credential naming a human, answer is recorded under that name ``` +Over `--stdio` the same block goes to **stderr** (§2.3), the first line reads +`ctrlrun mcp-operator — stdio; no socket, one client, the one that launched this`, the identity +lines name the login the answers will be recorded under and say the client could not choose it, +and the write-tools line names that login rather than describing a credential. + The observe-mode banner (`v0.3 §6.5`) is printed by the CLI before this block, as it is for every command that loads the operator's policy. An operator server against an observing deployment is worth the line: the approvals it lists were requested by a `Control` that is not @@ -454,6 +559,13 @@ enforcing, and answering one changes nothing in the world. | `resolve` on a record that is not `AMBIGUOUS`, or an unknown key | HTTP 200, `-41003`, with the store's reason | | `resolve` with a blank reason | HTTP 200, `-32602` | | The store raises anything else | HTTP 500, `-32603`, and the exception is logged, never returned | +| *Over stdio:* a line over `--max-body-bytes` | `-32600`, id `null`, the line drained unread (§2.3) | +| *Over stdio:* a line that is not JSON | `-32700`, id `null` — never silence, which the client would wait on | +| *Over stdio:* a notification | No line | +| *Over stdio:* EOF on stdin | Exit 0; the client went away | +| *Over stdio:* stdout closed by the client | Exit 0, the same | +| *Over stdio:* `handle` raises outside `_call` | `-32603` with the message's id, logged | +| *Over stdio:* a write under uid 0 | `-41013`; root is an account, not a person (§3.1) | A CTRLRun refusal is a JSON-RPC **error**, never a `result` with `isError: true`, for `v0.2 §6.10`'s reason: `isError` reaches the model as text, and a refusal to let a human's assistant do @@ -572,6 +684,55 @@ the test proves this server actually routes through it rather than reimplementin `ctrlrun stats --json` and `stats` return the same document for the same window. Asserted by equality, not by shape: §1.1's "not a second composer" is worth nothing if the two drift. +### T570 — stdio: initialize, list, read, write, attributed to the OS login + +T191 over the other transport, with T184's attribution half: `initialize` answers with the +client's revision, the notification gets no line, `tools/list` lists eight, a read returns the +pending request, `approve` grants it, and the record's `approver` is `mcp-operator:` +with the principal recorded beside it carrying `issuer` `os-login:`; the receipt the agent +leaves afterwards names the same person. A second case walks the negotiation table of §2.3: an +accepted revision is echoed, an unaccepted or absent one gets `2026-07-28`, and a `tools/list` +after each proves the mirrored headers that revision requires were synthesised. A third case, +against a policy whose control names an `approver_role`: `approve` over stdio is `-41015` and the +request stays pending, which is §3.1's stated cost pinned rather than described. + +### T571 — The login is the real uid and never the environment + +`USER`, `LOGNAME`, `LNAME` and `USERNAME` are all set to a forged name; the control asserts +`getpass.getuser()` now returns it; the provider's login is `pwd.getpwuid(os.getuid()).pw_name` +and is not the forgery. A context naming the forgery in every field it has resolves to the same +principal, with no claims and no expiry. A uid with no password entry refuses to start naming the +uid. `operator_identity_provider` returns this provider for a stdio config. And under uid 0, +with `SUDO_USER` set to a real name, the principal has no `user`, `approve` is `-41013` with the +store byte-identical, and a read still answers. + +### T572 — `--stdio` refuses every flag that names a header, by name + +`--principal-header`, `--user-header`, `--identity-jwt`, `--allow-origin` and +`--approver-roles-claim` each raise `InvalidArgument` naming the flag; so do a `--listen` or +`--path` that is not the default, as flags that cannot take effect; a stray `--identity-jwt-*` +flag is still refused by the shared check. `--max-body-bytes 0` is refused on both transports. +And T183's assertion, repeated beside the new flag: the command has `--stdio` and still has no +`--allow-remote` and no `--principal`. + +### T573 — Stdout carries only JSON-RPC lines + +A notification produces no line, a blank line is skipped, a line that is not JSON is `-32700` +with a null id, and the stream goes on. An oversized line that is a *well-formed request with an +id* is refused with a **null** id — the proof it was never parsed — and the next message is +answered. A last line with no trailing newline is still a message. A message of exactly `limit` +bytes is accepted whether the line ends in LF or CRLF, and one of `limit + 1` is refused either +way. A client that closes stdout ends the loop without a traceback. A tool name shaped like the +header sentinel is an unknown tool, not a header mismatch. A malformed `initialize` naming a +legacy revision is refused and leaves the loop on the revision the client actually negotiated, +observed through the one mechanic that differs between them. + +### T574 — The process speaks JSON on stdout and everything else on stderr + +`ctrlrun mcp-operator --stdio` as a real subprocess fed by pipe: exit 0 at EOF, every line of +stdout parses as JSON-RPC, and the §6 block is on stderr naming the login. A flag that cannot +take effect exits non-zero before the stream opens, with nothing on stdout. + ## 9. Public API and CLI additions (frozen) ### 9.1 The names @@ -582,7 +743,10 @@ equality, not by shape: §1.1's "not a second composer" is worth nothing if the # ctrlrun.gateway.operator.OperatorServer # ctrlrun.gateway.operator.operator_identity_provider # ctrlrun.gateway.operator.build_operator_server -# ctrlrun.gateway.operator.serve_operator_forever +# ctrlrun.gateway.operator.serve_operator_forever — runs whichever transport the config names +# ctrlrun.gateway.operator.serve_operator_stdio — added 2026-09-16 (§2.3) +# ctrlrun.gateway.operator.OsLoginIdentityProvider — added 2026-09-16 (§3.1) +# ctrlrun.gateway.operator.OS_LOGIN_ISSUER — "os-login" # ctrlrun.gateway — the entry point the CLI calls # ctrlrun.gateway.serve_operator(**options) -> None @@ -663,10 +827,17 @@ ctrlrun mcp-operator [--listen HOST:PORT] [--path PATH] [--environment ENV] [--allow-origin ORIGIN]... [--max-body-bytes N] [--store-url URL] ( --principal-header NAME --user-header NAME - | --identity-jwt [--identity-jwt-* ...] ) + | --identity-jwt [--identity-jwt-* ...] + | --stdio ) [--authority PATH] ``` +`--stdio` (added 2026-09-16, §2.3) takes no `--listen`, `--path`, `--allow-origin`, +`--principal-header`, `--user-header`, `--identity-jwt` or `--approver-roles-claim`: each is +refused at startup **by name** as a flag that could not take effect, and a flag the operator +believes took effect is the failure the gateway refuses the same way (`v0.3 §8.2`). It relaxes +no check. It removes a transport, and with it the one thing §2.1 exists to prevent. + Every `--identity-jwt-*` flag is `ctrlrun gateway`'s, spelled identically and meaning the same thing — and validated by the same function, not by a copy of it: `--identity-jwt` requires the four settings that have no safe default, and any `--identity-jwt-*` flag without it is refused. @@ -712,6 +883,27 @@ principal; the attributed name cannot forge a row (`v0.1 §5.3`'s `_approver` re character that could); `GET`/`DELETE` never reach `handle`; and `--since` behaves exactly as it did before the move to `ctrlrun.reporting`, exit code included. +**The stdio review, 2026-09-16.** §2.3 and §3.1 are an identity change, so they got the same +review, in a session that did not write them. Eleven findings; eight accepted, three declined, +none a blocker. Accepted, and each is now in the text above or in a test: "the one name the +client cannot choose" was an overclaim (§3.1, *and no more than that*); the lifetime cost of a +credential with no expiry was mechanism without consequence (§2.3, the `initialize` instructions, +the §6 block); root and `sudo` were unaddressed (§3.1, T571); the `-41015`-over-stdio row was +asserted and not tested (T570); a client closing stdout was a traceback and exit 1 (§7, T573); +the revision moved on a refused `initialize` (T573); the line bound charged a CRLF client two +bytes (§2.3, T573); a tool name shaped like the header sentinel was refused as a mismatch, a +pre-existing gap in `encode_header_value` now fixed at the source (T573); `--max-body-bytes` +had no floor on either transport (§11, T572); and `os.getlogin()` on Windows can raise +`OSError`, now `InvalidArgument`. Declined: `--stdio-max-age` (§2.3 says why); an `os-login` +issuer whose hostname contains a colon, because the prefix is the first segment and unambiguous; +and the body being JSON-parsed three times per message, which at the rate humans answer +approvals is not a cost. The review also confirmed, and it is recorded for the same reason as +the four above: the HTTP path is byte-identical; no stdio path reaches a store write without a +resolved human principal; nothing but JSON-RPC can reach stdout, including under `KeyboardInterrupt`; +each input line yields at most one output line, so ids cannot desynchronise; and the manifest's +`uvx ctrlrun mcp-operator --stdio` resolves to the `ctrlrun` console script with `CTRLRUN_CONFIG` +read by `discover_policy_path`. + ## 10. Explicitly out of scope Everything `v0.6 §11` excludes, plus: @@ -732,10 +924,24 @@ Everything `v0.6 §11` excludes, plus: and are not this server's. And the check is bounded the way `SPEC-v0.8.md` §3.8 bounds it: what it compares is one operator-written string against one claim, and what the kernel later refuses is an approval whose **recorded** entitlement does not cover the role. -- **stdio transport.** An MCP server launched over stdio by the assistant has no credential to +- **stdio transport.** ~~An MCP server launched over stdio by the assistant has no credential to verify — the process is whatever the client started, and every candidate identity is asserted by it. That is `--principal-from-client-info` (`v0.3 §8.1`) in a fourth costume, and it is the - one thing this server cannot afford. HTTP with a proxy is the shape that has an answer. + one thing this server cannot afford. HTTP with a proxy is the shape that has an answer.~~ + **Amended 2026-09-16 by §2.3 and §3.1**, and the strikethrough is deliberate for the reason + the entitlement line above gives: this was true of every release through 0.12.2, and a reader + of an older deployment's documentation should see which sentence applied. + + What was right in it is kept whole: every identity *the client could offer* is asserted by the + client, and none of them is used. What it missed is that the client does not choose everything + about the process it starts. It chooses the command line and the environment; the kernel + chooses the uid, and the account behind that uid is one the client already holds — it can + already open the store as it. That is the identity §3.1 now uses, with what it does not + promise stated beside it. HTTP with a proxy is still the shape for an approver who is not the + person at the keyboard; stdio is the shape for the one who is. +- **A lifetime for the stdio session.** A `--stdio-max-age` after which writes refuse until the + client relaunches the process was proposed by the stdio review and is not built; §2.3 says + why. The cost it would bound is stated in three places instead. - **Notifications, subscriptions or a push of pending approvals.** An approver asks; the server answers. A server that pushed would need a session, and §2 has none. - **Resources or prompts.** Tools only. @@ -756,6 +962,11 @@ Everything `v0.6 §11` excludes, plus: | `--principal-header` without `--user-header` | Refuses to start (§3.2) | | `--identity-jwt` without `--identity-jwt-user-claim` | Refuses to start (§3.2) | | A non-loopback `--listen` | Refuses to start (§2.1) | +| `--stdio` with any of `--principal-header`, `--user-header`, `--identity-jwt`, `--allow-origin`, `--approver-roles-claim`, or a non-default `--listen`/`--path` | Refuses to start, naming the flag (§2.3, §9.4) | +| `--stdio` and the real uid has no login in the password database | Refuses to start, naming the uid (§3.1) | +| `--stdio` under uid 0, a write tool | `-41013`, store unchanged: root is an account, not a person (§3.1) | +| `--max-body-bytes` below 1, either transport | Refuses to start | +| `--stdio`, a write tool, and the cited control names an `approver_role` | `-41015`, store unchanged: an OS login carries no roles (§3.1) | | A write tool, no credential | `-41007`, store unchanged | | A write tool, credential declined or rejected | `-41007`, store unchanged | | A write tool, credential names no human | `-41013`, store unchanged | diff --git a/server.json b/server.json index 1c0e841f..4344bafc 100644 --- a/server.json +++ b/server.json @@ -15,15 +15,28 @@ "registryBaseUrl": "https://pypi.org", "identifier": "ctrlrun", "version": "0.12.2", + "runtimeHint": "uvx", "transport": { - "type": "streamable-http", - "url": "http://127.0.0.1:8901/mcp" + "type": "stdio" }, "packageArguments": [ { "type": "positional", "value": "mcp-operator", - "description": "Serve the operator's own commands as MCP tools, over loopback." + "description": "Serve the operator's own commands as MCP tools." + }, + { + "type": "named", + "name": "--stdio", + "description": "Speak MCP on stdin and stdout, open no socket, and record each answer under the OS login of the person whose client launched this." + } + ], + "environmentVariables": [ + { + "name": "CTRLRUN_CONFIG", + "description": "Path to the ctrlrun.yaml your agents run against. The store is found beside it.", + "isRequired": true, + "format": "filepath" } ] } diff --git a/src/ctrlrun/cli/main.py b/src/ctrlrun/cli/main.py index e6db036e..5437187e 100644 --- a/src/ctrlrun/cli/main.py +++ b/src/ctrlrun/cli/main.py @@ -1882,6 +1882,14 @@ def _delegation_dict(delegation: Delegation) -> dict[str, Any]: @main.command(name="mcp-operator") @click.option("--listen", default="127.0.0.1:8901", show_default=True, help="HOST:PORT.") @click.option("--path", default="/mcp", show_default=True, help="The MCP endpoint path.") +@click.option( + "--stdio", + is_flag=True, + help="Speak MCP on stdin and stdout to the client that launched this process (a " + "desktop assistant, Cursor, an editor). Opens no socket. The approver is the account this " + "process runs as, read from the real uid; takes no header, JWT or origin flag " + "(SPEC-mcp-operator §2.3).", +) @click.option( "--principal-header", default=None, @@ -1963,6 +1971,7 @@ def _delegation_dict(delegation: Delegation) -> dict[str, Any]: def mcp_operator( listen: str, path: str, + stdio: bool, principal_header: str | None, user_header: str | None, environment: str | None, @@ -1987,11 +1996,13 @@ def mcp_operator( identity_jwt_http_timeout: float, store_url: str | None, ) -> None: - """Answer approvals from an MCP client, over loopback (SPEC-mcp-operator.md). + """Answer approvals from an MCP client, over loopback or stdio (SPEC-mcp-operator.md). There is no --principal and no --allow-remote, and both absences are load-bearing: a static principal cannot attribute an answer to a person (§3.1), and a server whose read - tools answer without a credential must not be the one that opens a port (§2.1). + tools answer without a credential must not be the one that opens a port (§2.1). --stdio + opens none at all, and its approver is the one name the launching client cannot set: the + OS login of the process (§2.3). """ host, _, port = listen.rpartition(":") try: @@ -2005,6 +2016,7 @@ def mcp_operator( host=host or "127.0.0.1", port=int(port), path=path, + stdio=stdio, principal_header=principal_header, user_header=user_header, environment=environment, diff --git a/src/ctrlrun/gateway/__init__.py b/src/ctrlrun/gateway/__init__.py index 88683ae8..7079dab8 100644 --- a/src/ctrlrun/gateway/__init__.py +++ b/src/ctrlrun/gateway/__init__.py @@ -283,39 +283,70 @@ def _announce_operator(control: Any, config: Any, identity: Any, store: Any) -> `print` rather than the logger, because this is the CLI's own output and a logger with no configured handler would swallow it — which is the failure mode the block exists to prevent, in miniature. + + **Over stdio the whole block goes to stderr** (§2.3). Stdout is the protocol stream there, + the client parses every line of it as JSON-RPC, and a startup block on it is read as a + broken message rather than as information: the first stdio client this was tried behind + logged "ignoring non-JSON output" for every line of it. """ - print( - f"ctrlrun mcp-operator — listening on {config.host}:{config.port}{config.path}", - flush=True, - ) - print(f"environment {control.environment}", flush=True) + out = sys.stderr if config.stdio else sys.stdout + + def line(text: str) -> None: + print(text, file=out, flush=True) + + if config.stdio: + line("ctrlrun mcp-operator — stdio; no socket, one client, the one that launched this") + else: + line(f"ctrlrun mcp-operator — listening on {config.host}:{config.port}{config.path}") + line(f"environment {control.environment}") # SPEC-mcp-operator §6 — for a server whose whole premise is "both processes on one host # against one store", and which has a `--store-url` that silently changes it, this is the # line an operator most needs. A review found the block printing everything but this. - print(f"store {getattr(store, 'path', store)}", flush=True) - print(f"identity {type(identity).__name__}", flush=True) + line(f"store {getattr(store, 'path', store)}") + line(f"identity {type(identity).__name__}") if config.principal_header is not None: - print( + line( f" trusts the header {config.principal_header!r}: it is worth what " "the proxy that sets it is worth," ) - print( + line( " and that proxy must authenticate the caller and overwrite the " "header on every request (SPEC-v0.3 §3.3)" ) - print( + if config.stdio: + line( + f" the account this process runs as, {identity.login!r}, read from the " + "real uid and from nothing the client" + ) + line( + " sends or sets. It carries no roles and no expiry, so this process " + "holds approve, deny and resolve under" + ) + line( + " that name for as long as it runs; the confirmation the client shows " + "before a write is the only human step" + ) + if identity.is_root: + line( + " RUNNING AS ROOT: an account, not a person. Every write is " + "refused (-41013); reads still answer" + ) + line( "read tools answer without a credential; loopback is not a boundary against " "other processes on this host" ) - print( - "write tools approve, deny, resolve — each needs a credential naming a human, " - "and each answer is recorded under that name" - ) - if control.authority is not None: - print( - f"authority {len(control.authority.grants)} grant(s), evaluated by the agent", - flush=True, + if config.stdio: + line( + "write tools approve, deny, resolve — each answer is recorded under " + f"{identity.login!r}" ) + else: + line( + "write tools approve, deny, resolve — each needs a credential naming a human, " + "and each answer is recorded under that name" + ) + if control.authority is not None: + line(f"authority {len(control.authority.grants)} grant(s), evaluated by the agent") def _observe_the_upstream(control: Any, config: Any) -> None: diff --git a/src/ctrlrun/gateway/mcp.py b/src/ctrlrun/gateway/mcp.py index 1a20eec5..3e10298d 100644 --- a/src/ctrlrun/gateway/mcp.py +++ b/src/ctrlrun/gateway/mcp.py @@ -285,10 +285,18 @@ def _decoded(value: str) -> str | None: def encode_header_value(value: str) -> str: - """Wrap a value in the revision's base64 sentinel where it is not ASCII-safe (§6.4).""" + """Wrap a value in the revision's base64 sentinel where it is not ASCII-safe (§6.4). + + A value that is ASCII but *looks like* the sentinel is wrapped too, or `_decoded` on the far + side would try to decode the bare value and refuse a header that faithfully mirrored the + body. A review found the gap when the operator's stdio loop started mirroring tool names. + """ try: value.encode("ascii") + wrap = value.startswith(_SENTINEL_OPEN) and value.endswith(_SENTINEL_CLOSE) except UnicodeEncodeError: + wrap = True + if wrap: encoded = base64.b64encode(value.encode("utf-8")).decode("ascii") return f"{_SENTINEL_OPEN}{encoded}{_SENTINEL_CLOSE}" return value diff --git a/src/ctrlrun/gateway/operator.py b/src/ctrlrun/gateway/operator.py index 83454c50..6ba35f33 100644 --- a/src/ctrlrun/gateway/operator.py +++ b/src/ctrlrun/gateway/operator.py @@ -26,14 +26,16 @@ import json import logging +import os import socket +import sys import threading from collections.abc import Callable, Iterable, Mapping -from dataclasses import dataclass +from dataclasses import dataclass, field from datetime import UTC, datetime from http.server import BaseHTTPRequestHandler, ThreadingHTTPServer from pathlib import Path -from typing import Any, Final +from typing import IO, Any, Final from ..action import Principal from ..approval import ( @@ -63,7 +65,17 @@ ledger_rows as _ledger_rows, ) from ..state import RESOLUTIONS, StateStore -from .mcp import DEFAULT_MAX_BODY_BYTES, ParsedRequest, Refusal, parse_request +from .mcp import ( + ACCEPTED_REVISIONS, + CURRENT_REVISION, + DEFAULT_MAX_BODY_BYTES, + INVALID_REQUEST, + LEGACY_DEFAULT_REVISION, + ParsedRequest, + Refusal, + encode_header_value, + parse_request, +) from .wire import ( _header, _json, @@ -109,6 +121,11 @@ SERVER_NAME: Final = "ctrlrun-mcp-operator" +#: §3.1 — what `OsLoginIdentityProvider` writes as the principal's issuer: the login was issued +#: by the operating system of the named host, and evidence can tell it from a proxy's header or +#: a token's `iss` at a glance. +OS_LOGIN_ISSUER: Final = "os-login" + #: §9.3 — two codes added to `v0.2 §6.10`'s table, both in the `-410xx` range that release #: reserved, neither reachable from the gateway. NOT_A_HUMAN: Final = (-41013, "ctrlrun.not_a_human", 403) @@ -173,6 +190,10 @@ class OperatorConfig: identity_jwt_leeway: float = 60.0 identity_jwt_jwks_min_refresh: float = 30.0 identity_jwt_http_timeout: float = 5.0 + #: §2.3 — speak MCP on stdin and stdout to the one client that launched this process, and + #: open no socket at all. The approver is then the account the process runs as, read from + #: the real uid and from nothing the client sends or sets (§3.1). + stdio: bool = False def __post_init__(self) -> None: if self.host not in LOOPBACK: @@ -188,6 +209,41 @@ def __post_init__(self) -> None: ) if not self.path.startswith("/"): raise InvalidArgument(f"--path {self.path!r} must start with '/'") + if self.max_body_bytes < 1: + # A bound of zero refuses every message and a negative one reads to EOF over HTTP + # and nothing at all over stdio; neither is a server, and both are cheaper to find + # here. A review found the floor missing on both transports. + raise InvalidArgument("--max-body-bytes must be at least 1") + if self.stdio: + # §2.3 — there are no headers over stdio, so every flag that names one is a flag that + # could not take effect, and a flag the operator believes took effect is the failure + # the gateway refuses by name (`v0.3 §8.2`). `--approver-roles-claim` is in the list + # for the same reason: an OS login carries no claims to read a role from. + offered = [ + flag + for flag, given in ( + ("--principal-header", self.principal_header is not None), + ("--user-header", self.user_header is not None), + ("--identity-jwt", self.identity_jwt), + ("--allow-origin", bool(self.allow_origins)), + ("--approver-roles-claim", self.approver_roles_claim is not None), + ) + if given + ] + if offered: + raise InvalidArgument( + f"--stdio takes no {', '.join(offered)}: there are no headers over stdio. " + "The approver is the OS login of this process, which the client that " + "launched it cannot choose (SPEC-mcp-operator §2.3, §3.1)" + ) + if (self.host, self.port) != DEFAULT_LISTEN or self.path != DEFAULT_PATH: + raise InvalidArgument( + "--stdio opens no socket, so --listen and --path cannot take effect " + "(SPEC-mcp-operator §2.3)" + ) + # A stray `--identity-jwt-*` flag is still refused by name, by the same shared check. + check_jwt_flags(self) + return sources = [self.principal_header is not None, self.identity_jwt] if sum(sources) != 1: # §3.1 — `--principal` is not among them. `StaticIdentityProvider` answers with the @@ -228,13 +284,113 @@ def __post_init__(self) -> None: ) +def _os_account() -> tuple[int | None, str]: + """The **real** uid of this process and its login name, from the system and never from the + environment. + + `getpass.getuser()` reads `LOGNAME`, `USER`, `LNAME` and `USERNAME` before it asks the + system, and every one of those is set by whoever launched the process -- which over stdio + is the client. A name the client can set is `--principal-from-client-info` (`v0.3 §8.1`) + again, and §10 refused that shape for a reason. `os.getlogin()` on POSIX reads the + controlling terminal, which a process launched by a desktop client does not have. What is + left is the password database keyed by the real uid. The **real** uid, not the effective + one: it names the account that launched the process, not what a setuid file grants, and + `SUDO_USER` is ignored for the same reason the rest of the environment is. + + This is the account the client is running as, and no more than that. The uid itself cannot + be chosen by the client; the *name the process reports for it* is only as trustworthy as + the process, and a client that controls the interpreter's environment (`PYTHONPATH`, a + preloaded library, what `uvx` installs beside the package) controls the process. What makes + that acceptable is the boundary, not the lookup: a client that can do any of that can + already open the store as this account and answer with `ctrlrun approve`, so the attribution + string was already in its reach at the file. §3.1 says exactly this and no more. + + On Windows `os.getlogin()` is `GetUserNameW`, the token's user, and there is no uid. + """ + try: + import pwd + except ImportError: # pragma: no cover - Windows + try: + return None, os.getlogin() + except OSError as exc: + raise InvalidArgument( + f"the login of this process could not be read ({exc}), so there is nobody to " + "record an answer under; --stdio refuses to start (SPEC-mcp-operator §3.1)" + ) from exc + uid = os.getuid() + try: + return uid, pwd.getpwuid(uid).pw_name + except KeyError as exc: + raise InvalidArgument( + f"uid {uid} has no login in the password database, so there is nobody to record " + "an answer under; --stdio refuses to start (SPEC-mcp-operator §3.1)" + ) from exc + + +@dataclass(frozen=True) +class OsLoginIdentityProvider: + """The OS login of the process, for `--stdio` (SPEC-mcp-operator §3.1). + + Not `StaticIdentityProvider` in another costume, and the difference is where the name comes + from. A static principal is whatever was typed after `--principal`, so every approval carries + a string that distinguishes nobody. This one is the account the process is running as, read + from the real uid and from nothing the client sends or sets: two people on one host get two + logins. It is the boundary the store file already has -- a process that can run this as you + can already open the store as you -- so it adds no surface, and it is stricter than §2.1 in + one respect: it opens no port. `_os_account` says what that boundary does and does not + promise about the *name*. + + What it cannot do is stated rather than implied. An OS login carries no claims, so no role + can be read from it and every control naming an `approver_role` refuses over stdio (the + startup warning in `OperatorServer` covers it). It has no `expires_at`, because a login + session is not a credential with a lifetime the process can see -- which means the client + process holds `approve`, `deny` and `resolve` under this name for as long as it runs, and + the confirmation the client shows before a write is the only human step left; §2.3 states + that cost and the `initialize` instructions repeat it to the model. And **root is an + account, not a person**: under uid 0 the principal carries no `user`, every write is refused + as `-41013` the way a machine credential is, and reads still answer. + """ + + login: str + host: str + uid: int | None = None + _principal: Principal = field(init=False, repr=False, compare=False) + + @classmethod + def from_process(cls) -> OsLoginIdentityProvider: + uid, login = _os_account() + return cls(login=login, host=socket.gethostname(), uid=uid) + + @property + def is_root(self) -> bool: + return self.uid == 0 + + def __post_init__(self) -> None: + if not self.login: + raise InvalidArgument( + "the OS login is empty; there is nobody to attribute an answer to" + ) + issuer = OS_LOGIN_ISSUER if not self.host else f"{OS_LOGIN_ISSUER}:{self.host}" + user = None if self.is_root else self.login + object.__setattr__( + self, "_principal", Principal(agent=self.login, user=user, issuer=issuer) + ) + + def resolve(self, context: IdentityContext) -> Principal | None: + # The context is ignored on purpose: every field of it that could name a person came + # from the client, and the whole point of this provider is that the client cannot. + return self._principal + + def operator_identity_provider(config: OperatorConfig) -> IdentityProvider: """The provider this server's flags name (SPEC-mcp-operator §3.1). - Two constructors, not the gateway's three. The JWT import is deferred so that `import - ctrlrun` never pulls in `jwt` and an operator who selected the extra without installing it - gets `MissingDependency` naming the command. + Three constructors, not the gateway's three: the OS login for `--stdio`, a header, or a JWT. + The JWT import is deferred so that `import ctrlrun` never pulls in `jwt` and an operator + who selected the extra without installing it gets `MissingDependency` naming the command. """ + if config.stdio: + return OsLoginIdentityProvider.from_process() if config.principal_header is not None: return HeaderIdentityProvider( agent_header=config.principal_header, user_header=config.user_header @@ -649,6 +805,14 @@ def _initialize(self, parsed: ParsedRequest) -> dict[str, Any]: "happened. approve, deny and resolve write, need an authenticated human, and " "record the answer under that person's name. Nothing here can make an agent " "act." + + ( + " Over stdio this process holds approve, deny and resolve under the OS " + "login of whoever launched it for as long as it runs, and the confirmation " + "the client shows before a write is the only human step: it must stay on " + "for these three tools." + if self._config.stdio + else "" + ) ), } @@ -1320,8 +1484,156 @@ def _repeated_identity_header( return None +def serve_operator_stdio( + server: OperatorServer, + *, + stdin: IO[bytes] | None = None, + stdout: IO[bytes] | None = None, +) -> None: + """Speak MCP on stdin and stdout until the client closes them (SPEC-mcp-operator §2.3). + + One JSON-RPC message per line, UTF-8, as the stdio transport specifies; every line written + to `stdout` is a JSON-RPC message and nothing else is, because the client parses the stream + and a stray line of text is a protocol error to it. The startup block and every log line go + to stderr for that reason. + + Every message goes through `handle`, exactly as a POST body does, so the same parser, the + same refusals and the same identity gate apply. What this loop adds is the part HTTP carried + in headers: the protocol revision, negotiated once at `initialize` from the body rather than + read from a header on every message, and the mirrored `Mcp-Method` and `Mcp-Name`, which + `2026-07-28` requires and which are synthesised from the body they would have to agree with. + + Two things HTTP could refuse with a bare status become messages here, because a request with + an id that gets no line would hang the client: an oversized line is refused **unread** -- it + is bounded by `readline(limit + 2)`, two being the longest line ending, and drained without + being decoded -- and any empty-body refusal becomes `-32600` with whatever id the line + carried. Anything `handle` raises outside `_call`'s own net is `-32603` with the id, so §7's + last row holds over this transport too. A client that closes stdout is a client that went + away, and the loop returns rather than dying on the write. + """ + reader = sys.stdin.buffer if stdin is None else stdin + writer = sys.stdout.buffer if stdout is None else stdout + limit = server.config.max_body_bytes + revision = LEGACY_DEFAULT_REVISION + while True: + line = reader.readline(limit + 2) + if not line: + return + truncated = not line.endswith(b"\n") and len(line) == limit + 2 + body = line.rstrip(b"\r\n") + if truncated or len(body) > limit: + if truncated: + _drain_line(reader) + _LOG.warning("refused a line over %d bytes without reading it", limit) + refusal = json_rpc_error( + None, + INVALID_REQUEST, + "ctrlrun.invalid_request", + f"a message over {limit} bytes was discarded unread", + ) + if not _emit(writer, refusal): + return + continue + body = body.strip() + if not body: + continue + document = _loaded(body) + initializing = isinstance(document, dict) and document.get("method") == "initialize" + candidate = _negotiated(document) if initializing else revision + try: + response = server.handle(body, _stdio_headers(document, candidate)) + except Exception: + _LOG.exception("a message could not be handled") + response = _error( + _request_id(body), + _INTERNAL_ERROR, + "ctrlrun.internal_error", + 500, + "the message could not be handled; see the server log", + ) + if initializing and response.status == 200: + # Only a successful initialize moves the revision: a malformed one is refused and + # must not leave the loop on a revision the client never negotiated. + revision = candidate + if response.body: + if not _write_line(writer, response.body): + return + elif response.status not in (200, 202): + refusal = json_rpc_error( + _request_id(body), + INVALID_REQUEST, + "ctrlrun.invalid_request", + f"the message was refused ({response.status})", + ) + if not _emit(writer, refusal): + return + + +def _loaded(body: bytes) -> Any: + try: + return json.loads(body) + except (ValueError, UnicodeDecodeError): + return None + + +def _negotiated(document: Mapping[str, Any]) -> str: + """§2.3 — the client's revision where it is one this server accepts, else the current one. + + The transport specification's rule: a server that does not support the requested version + answers with one it does, and the client decides whether to go on. Refusing outright, which + is what the HTTP path does with an unaccepted header, would leave a desktop client with no + version at all to decide about. + """ + params = document.get("params") + requested = params.get("protocolVersion") if isinstance(params, Mapping) else None + return requested if requested in ACCEPTED_REVISIONS else CURRENT_REVISION + + +def _stdio_headers(document: Any, revision: str) -> dict[str, str]: + """What the HTTP transport would have carried, built from the body it must agree with.""" + headers = {"mcp-protocol-version": revision} + if not isinstance(document, dict): + return headers + method = document.get("method") + if isinstance(method, str) and method: + headers["mcp-method"] = encode_header_value(method) + params = document.get("params") + name = params.get("name") if isinstance(params, Mapping) else None + if isinstance(name, str) and name: + headers["mcp-name"] = encode_header_value(name) + return headers + + +def _drain_line(reader: IO[bytes]) -> None: + """Discard the rest of a line that was too long to read, without holding any of it.""" + while True: + chunk = reader.readline(65536) + if not chunk or chunk.endswith(b"\n"): + return + + +def _emit(writer: IO[bytes], document: Mapping[str, Any]) -> bool: + return _write_line( + writer, json.dumps(document, ensure_ascii=False, separators=(",", ":")).encode() + ) + + +def _write_line(writer: IO[bytes], payload: bytes) -> bool: + """One line to the client, or `False` when there is no client any more (§2.3, §7).""" + try: + writer.write(payload + b"\n") + writer.flush() + except BrokenPipeError: + _LOG.info("the client closed stdout; exiting") + return False + return True + + def serve_operator_forever(server: OperatorServer) -> None: - """Run until interrupted. `ctrlrun mcp-operator` calls this.""" + """Run until interrupted, over whichever transport the config names (§2, §2.3).""" + if server.config.stdio: + serve_operator_stdio(server) + return httpd = build_operator_server(server) thread = threading.Thread(target=httpd.serve_forever, daemon=True) thread.start() diff --git a/tests/test_mcp_operator.py b/tests/test_mcp_operator.py index 66433218..52a231db 100644 --- a/tests/test_mcp_operator.py +++ b/tests/test_mcp_operator.py @@ -1,6 +1,6 @@ # SPDX-FileCopyrightText: 2026 The CTRLRun contributors # SPDX-License-Identifier: Apache-2.0 -"""The operator MCP server. SPEC-mcp-operator.md; acceptance tests T182-T193. +"""The operator MCP server. SPEC-mcp-operator.md; acceptance tests T182-T193 and T570-T574. An approver answers from the assistant they are already talking to, through the same two store calls `ctrlrun approve` and `ctrlrun deny` make. So most of what is under test here is @@ -41,12 +41,15 @@ from ctrlrun.effect import RESOLVED_BY_HUMAN from ctrlrun.gateway.operator import ( LOOPBACK, + OS_LOGIN_ISSUER, OperatorConfig, OperatorServer, + OsLoginIdentityProvider, build_operator_server, operator_identity_provider, + serve_operator_stdio, ) -from ctrlrun.identity import IdentityProvider +from ctrlrun.identity import IdentityContext, IdentityProvider from ctrlrun.receipt import EventType, JSONLEventSink CURRENT = "2026-07-28" @@ -1525,3 +1528,502 @@ def test_each_gateway_module_imports_on_its_own_in_either_order(): text=True, ) assert finished.returncode == 0, f"{first} then {second}: {finished.stderr[-400:]}" + + +# --- T570-T574 — stdio: no socket, and the approver is the OS login ------------------------ +# +# §10 refused stdio for a reason, and the reason is still true: a process launched by the +# assistant has no credential, and every candidate identity *the client could offer* is asserted +# by it. What these tests pin is that none of those is used -- the name comes from the real uid +# and from nothing the client sends or sets -- and that the loop puts nothing on stdout but +# JSON-RPC. What the real uid's name does and does not promise is `_os_account`'s docstring. + + +def _stdio_config(**overrides) -> OperatorConfig: + return OperatorConfig(stdio=True, **overrides) + + +@pytest.fixture +def os_login(): + return OsLoginIdentityProvider.from_process() + + +@pytest.fixture +def stdio_server(control, os_login): + return OperatorServer(_stdio_config(), control, os_login) + + +def _session(server, messages): + """Feed messages down stdin (dicts, or raw bytes for a malformed line) and return every + line that came out of stdout, parsed -- so a line that is not JSON fails here, which is the + property T573 is about.""" + import io + + lines = [m if isinstance(m, bytes) else json.dumps(m).encode() for m in messages] + stdin = io.BytesIO(b"\n".join(lines) + b"\n") + stdout = io.BytesIO() + serve_operator_stdio(server, stdin=stdin, stdout=stdout) + out = stdout.getvalue() + assert out == b"" or out.endswith(b"\n"), out + return [json.loads(line) for line in out.split(b"\n") if line] + + +def _issuer(provider): + return OS_LOGIN_ISSUER if not provider.host else f"{OS_LOGIN_ISSUER}:{provider.host}" + + +def test_T570_initialize_list_read_write_attributed_to_the_os_login( + stdio_server, control, store, os_login +): + """T191 over the other transport, and the attribution half of T184 with it: the answer is + recorded under the OS login, the record carries the principal with an `os-login` issuer so + evidence can tell it from a proxy's header, and the receipt the agent leaves afterwards + names the same person. The notification in the middle gets no line.""" + action, request_id = _pending(control) + + replies = _session( + stdio_server, + [ + { + "jsonrpc": "2.0", + "id": 1, + "method": "initialize", + "params": { + "protocolVersion": "2025-06-18", + "capabilities": {}, + "clientInfo": {"name": "desktop", "version": "1"}, + }, + }, + {"jsonrpc": "2.0", "method": "notifications/initialized"}, + {"jsonrpc": "2.0", "id": 2, "method": "tools/list"}, + { + "jsonrpc": "2.0", + "id": 3, + "method": "tools/call", + "params": {"name": "list_pending_approvals", "arguments": {}}, + }, + { + "jsonrpc": "2.0", + "id": 4, + "method": "tools/call", + "params": {"name": "approve", "arguments": {"request_id": request_id}}, + }, + ], + ) + + assert [reply["id"] for reply in replies] == [1, 2, 3, 4] + assert replies[0]["result"]["protocolVersion"] == "2025-06-18" + assert replies[0]["result"]["serverInfo"]["name"] == "ctrlrun-mcp-operator" + assert len(replies[1]["result"]["tools"]) == 8 + pending = replies[2]["result"]["structuredContent"]["pending"] + assert [entry["request_id"] for entry in pending] == [request_id] + assert replies[3]["result"]["structuredContent"]["status"] == "granted" + + who = f"mcp-operator:{os_login.login}" + record = store.get_approval(request_id) + assert record.status is ApprovalStatus.GRANTED + assert record.approver == who + assert [(w.agent, w.user, w.issuer) for w in record.approvers] == [ + (os_login.login, os_login.login, _issuer(os_login)) + ] + assert os_login.login != "desktop", "clientInfo is the client's word and must not be the name" + + granted = [e for e in store.events() if e.type is EventType.APPROVAL_GRANTED] + assert len(granted) == 1 + assert granted[0].data["approver"] == who + assert granted[0].data["via"] == "mcp-operator" + + with with_approval(request_id): + control.execute(action, lambda: "re_1", "refund:txn_1") + receipt = next(r for r in store.receipts() if r.action_id == action.action_id) + assert receipt.approver == who + + +@pytest.mark.parametrize( + "requested, answered", + [ + ("2026-07-28", "2026-07-28"), + ("2025-03-26", "2025-03-26"), + ("1999-01-01", "2026-07-28"), + (None, "2026-07-28"), + ], +) +def test_T570_the_revision_is_the_clients_where_accepted_and_the_current_one_otherwise( + stdio_server, requested, answered +): + """§2.3. The HTTP path refuses an unaccepted header outright; over stdio the transport's + rule applies instead -- answer with a version this server supports and let the client + decide. The `tools/list` afterwards proves the mirrored headers `2026-07-28` requires were + synthesised from the body rather than left for the client to send over a pipe.""" + params = {} if requested is None else {"protocolVersion": requested} + replies = _session( + stdio_server, + [ + {"jsonrpc": "2.0", "id": 1, "method": "initialize", "params": params}, + {"jsonrpc": "2.0", "id": 2, "method": "tools/list"}, + ], + ) + assert replies[0]["result"]["protocolVersion"] == answered + assert len(replies[1]["result"]["tools"]) == 8 + + +def test_T571_the_login_is_the_real_uid_and_never_the_environment(monkeypatch): + """§3.1's whole argument in one assertion. `getpass.getuser()` believes the environment, + the environment is the launching client's to set, and a client that could set the + approver's name would be `--principal-from-client-info` in a fourth costume -- the thing + §10 refused. The control proves the environment really does say `mallory`.""" + import getpass + import os + + pwd = pytest.importorskip("pwd") + for name in ("USER", "LOGNAME", "LNAME", "USERNAME"): + monkeypatch.setenv(name, "mallory") + assert getpass.getuser() == "mallory" + + provider = OsLoginIdentityProvider.from_process() + real = pwd.getpwuid(os.getuid()).pw_name + assert provider.login == real + assert provider.login != "mallory" + + # Nor from anything in the request: a context naming mallory in every field it has. + principal = provider.resolve( + IdentityContext( + action="mcp-operator.approve", + environment="production", + headers={"x-approver": "mallory", "x-approver-user": "mallory"}, + agent="mallory", + user="mallory", + ) + ) + assert (principal.agent, principal.user, principal.issuer) == (real, real, _issuer(provider)) + assert principal.expires_at is None, "a login session has no lifetime the process can see" + assert dict(principal.claims) == {}, "an OS login carries no claims, so no role can be read" + + +def test_T571_a_uid_with_no_login_refuses_to_start(monkeypatch): + """A server whose write tools could never succeed is refused where it can still be fixed + (§3.2's logic), and the refusal names the uid rather than failing at the first answer.""" + pwd = pytest.importorskip("pwd") + + def nobody(uid): + raise KeyError(uid) + + monkeypatch.setattr(pwd, "getpwuid", nobody) + with pytest.raises(InvalidArgument) as raised: + OsLoginIdentityProvider.from_process() + assert "no login" in str(raised.value) + + +def test_T571_the_config_names_the_os_login_provider_for_stdio(): + provider = operator_identity_provider(_stdio_config()) + assert isinstance(provider, OsLoginIdentityProvider) + assert provider.login + + +@pytest.mark.parametrize( + "flag, settings", + [ + ("--principal-header", {"principal_header": "x-approver", "user_header": "x-u"}), + ("--user-header", {"user_header": "x-u"}), + ("--identity-jwt", dict(JWT_OK)), + ("--allow-origin", {"allow_origins": ("http://localhost",)}), + ("--approver-roles-claim", {"approver_roles_claim": "roles"}), + ], +) +def test_T572_stdio_refuses_every_flag_that_names_a_header_by_name(flag, settings): + """There are no headers over stdio, so each of these is a flag that could not take effect, + and a flag the operator believes took effect is the failure the gateway refuses by name.""" + with pytest.raises(InvalidArgument) as raised: + _stdio_config(**settings) + assert flag in str(raised.value) + + +@pytest.mark.parametrize("settings", [{"host": "localhost"}, {"port": 9000}, {"path": "/other"}]) +def test_T572_stdio_refuses_a_listen_or_path_that_cannot_take_effect(settings): + with pytest.raises(InvalidArgument) as raised: + _stdio_config(**settings) + assert "cannot take effect" in str(raised.value) + + +def test_T572_a_stray_jwt_flag_is_still_refused_with_stdio(): + """The shared `check_jwt_flags` runs on this path too, so `--stdio` cannot become the way + to start a server with an `--identity-jwt-*` flag nobody validated.""" + with pytest.raises(InvalidArgument) as raised: + _stdio_config(identity_jwt_algorithms=("none",)) + assert "needs --identity-jwt" in str(raised.value) + + +def test_T572_the_cli_has_stdio_and_still_no_allow_remote_and_no_principal(): + """T183's assertion, repeated beside the new flag: stdio removed a transport and added no + way to bind, and no way to name the approver from the command line.""" + command = main.commands["mcp-operator"] + flags = {flag for parameter in command.params for flag in parameter.opts} + assert "--stdio" in flags + assert "--allow-remote" not in flags + assert "--principal" not in flags + + +def test_T573_stdout_carries_only_json_rpc_lines(stdio_server): + """A notification gets no line, a blank line is skipped, a line that is not JSON is + `-32700` with a null id rather than silence -- a request the client cannot match to an id + is one it will wait on for ever -- and the stream goes on afterwards.""" + replies = _session( + stdio_server, + [ + {"jsonrpc": "2.0", "method": "notifications/initialized"}, + b"this is not json", + b"", + {"jsonrpc": "2.0", "id": 7, "method": "tools/list"}, + ], + ) + assert [reply.get("id") for reply in replies] == [None, 7] + assert replies[0]["error"]["code"] == -32700 + assert len(replies[1]["result"]["tools"]) == 8 + + +def test_T573_an_oversized_line_is_refused_unread_and_the_stream_goes_on(control, os_login): + """`--max-body-bytes` bounds the allocation and not only the decision (§2), over stdio as + over HTTP. The oversized line is a well-formed request with an id, so had it been read and + parsed the reply would carry `99`; it carries `null`, and the next message is answered.""" + server = OperatorServer(_stdio_config(max_body_bytes=256), control, os_login) + big = {"jsonrpc": "2.0", "id": 99, "method": "tools/list", "params": {"pad": "x" * 1000}} + + replies = _session(server, [big, {"jsonrpc": "2.0", "id": 100, "method": "tools/list"}]) + + assert [reply.get("id") for reply in replies] == [None, 100] + assert replies[0]["error"]["code"] == -32600 + assert "unread" in replies[0]["error"]["message"] + + +def test_T573_a_last_line_with_no_newline_is_still_a_message(stdio_server): + import io + + stdin = io.BytesIO(json.dumps({"jsonrpc": "2.0", "id": 1, "method": "tools/list"}).encode()) + stdout = io.BytesIO() + serve_operator_stdio(stdio_server, stdin=stdin, stdout=stdout) + assert json.loads(stdout.getvalue())["id"] == 1 + + +def test_T574_the_process_speaks_json_on_stdout_and_everything_else_on_stderr(workspace): + """The end-to-end half, as T191 is for HTTP: `ctrlrun mcp-operator --stdio` as a real + subprocess, fed by pipe, exiting cleanly when the pipe closes. Every line of stdout parses; + the startup block, which the HTTP path prints to stdout, is on stderr here and names the + login -- the first stdio client this was tried behind logged "ignoring non-JSON output" + for every line of a block printed to stdout.""" + import os + import subprocess + import sys + + pwd = pytest.importorskip("pwd") + login = pwd.getpwuid(os.getuid()).pw_name + messages = [ + {"jsonrpc": "2.0", "id": 1, "method": "initialize", "params": {"protocolVersion": CURRENT}}, + {"jsonrpc": "2.0", "method": "notifications/initialized"}, + {"jsonrpc": "2.0", "id": 2, "method": "tools/list"}, + ] + finished = subprocess.run( + [sys.executable, "-m", "ctrlrun.cli.main", "mcp-operator", "--stdio"], + input=b"".join(json.dumps(m).encode() + b"\n" for m in messages), + capture_output=True, + cwd=workspace, + timeout=120, + ) + assert finished.returncode == 0, finished.stderr.decode(errors="replace") + + documents = [json.loads(line) for line in finished.stdout.split(b"\n") if line] + assert [document["id"] for document in documents] == [1, 2] + assert documents[0]["result"]["protocolVersion"] == CURRENT + + stderr = finished.stderr.decode() + assert "ctrlrun mcp-operator — stdio" in stderr + assert "identity OsLoginIdentityProvider" in stderr + assert repr(login) in stderr + + +def test_T574_a_flag_that_cannot_take_effect_exits_before_the_stream_opens(workspace): + import subprocess + import sys + + finished = subprocess.run( + [ + sys.executable, + "-m", + "ctrlrun.cli.main", + "mcp-operator", + "--stdio", + "--allow-origin", + "x", + ], + input=b"", + capture_output=True, + cwd=workspace, + timeout=120, + ) + assert finished.returncode != 0 + assert finished.stdout == b"", "nothing but JSON-RPC may reach stdout, refusals included" + assert "--allow-origin" in finished.stderr.decode() + + +def test_T570_a_gated_control_refuses_over_stdio_because_a_login_carries_no_roles(gated, os_login): + """§3.1's stated cost, pinned: an OS login carries no claims, so a control naming an + `approver_role` refuses the answer with `-41015` and the request stays pending. The + fail-closed direction, asserted rather than described.""" + server = OperatorServer(_stdio_config(), gated, os_login) + request_id = _pending_on(gated) + + replies = _session( + server, + [ + { + "jsonrpc": "2.0", + "id": 1, + "method": "tools/call", + "params": {"name": "approve", "arguments": {"request_id": request_id}}, + } + ], + ) + + assert replies[0]["error"]["code"] == -41015 + assert "payments-owner" in replies[0]["error"]["message"] + assert gated.store.get_approval(request_id).status is ApprovalStatus.PENDING + + +def test_T571_root_is_an_account_and_not_a_person(monkeypatch, control, store): + """`sudo ctrlrun mcp-operator --stdio`, or a root container, would record every answer as + `root`, which distinguishes nobody -- §3.1's own objection to `--principal`. So under uid 0 + the principal carries no `user`, a write is refused exactly as a machine credential is, and + a read still answers. `SUDO_USER` is set to make the point that it is ignored.""" + import os + + pytest.importorskip("pwd") + monkeypatch.setattr(os, "getuid", lambda: 0) + monkeypatch.setenv("SUDO_USER", "alice") + provider = OsLoginIdentityProvider.from_process() + assert provider.is_root + principal = provider.resolve(IdentityContext(action="mcp-operator.approve", environment="p")) + assert principal.user is None + + server = OperatorServer(_stdio_config(), control, provider) + _, request_id = _pending(control) + before = len(list(store.events())) + replies = _session( + server, + [ + { + "jsonrpc": "2.0", + "id": 1, + "method": "tools/call", + "params": {"name": "approve", "arguments": {"request_id": request_id}}, + }, + { + "jsonrpc": "2.0", + "id": 2, + "method": "tools/call", + "params": {"name": "list_pending_approvals", "arguments": {}}, + }, + ], + ) + assert replies[0]["error"]["code"] == -41013 + assert store.get_approval(request_id).status is ApprovalStatus.PENDING + assert len(list(store.events())) == before + assert [e["request_id"] for e in replies[1]["result"]["structuredContent"]["pending"]] == [ + request_id + ] + + +@pytest.mark.parametrize("make", [_config, _stdio_config]) +def test_T572_max_body_bytes_has_a_floor_on_both_transports(make): + """Zero refuses every message and a negative value reads to EOF over HTTP and nothing at + all over stdio. Neither is a server. A review found the floor missing on both.""" + with pytest.raises(InvalidArgument) as raised: + make(max_body_bytes=0) + assert "--max-body-bytes" in str(raised.value) + + +def test_T573_the_limit_counts_the_message_and_not_its_line_ending(control, os_login): + """A message of exactly `limit` bytes is accepted whether the line ends in LF or CRLF, and + one of `limit + 1` bytes is refused however it ends. The first version measured the raw + line, so a CRLF client lost two bytes of budget; a review found it.""" + import io + + limit = 200 + server = OperatorServer(_stdio_config(max_body_bytes=limit), control, os_login) + + def padded(size): + stem = b'{"jsonrpc":"2.0","id":1,"method":"tools/list","params":{"pad":"' + return stem + b"x" * (size - len(stem) - 3) + b'"}}' + + exact, over = padded(limit), padded(limit + 1) + assert (len(exact), len(over)) == (limit, limit + 1) + + for ending in (b"\n", b"\r\n"): + stdout = io.BytesIO() + serve_operator_stdio(server, stdin=io.BytesIO(exact + ending), stdout=stdout) + assert json.loads(stdout.getvalue())["id"] == 1, ending + stdout = io.BytesIO() + serve_operator_stdio(server, stdin=io.BytesIO(over + ending), stdout=stdout) + assert json.loads(stdout.getvalue())["error"]["code"] == -32600, ending + + +def test_T573_a_client_that_closes_stdout_ends_the_loop_without_a_traceback(stdio_server): + """The client died or closed the pipe. That is the client going away, the same as EOF on + stdin, and the answer is to return, not to exit 1 with `BrokenPipeError` on stderr.""" + import io + + class Closed(io.BytesIO): + def write(self, data): + raise BrokenPipeError + + stdin = io.BytesIO(json.dumps({"jsonrpc": "2.0", "id": 1, "method": "tools/list"}).encode()) + serve_operator_stdio(stdio_server, stdin=stdin, stdout=Closed()) + + +def test_T573_a_tool_name_shaped_like_the_header_sentinel_is_an_unknown_tool(stdio_server): + """The mirrored `Mcp-Name` is built from the body. A name that happens to look like the + revision's base64 sentinel must round-trip through the encoder and the decoder to the same + string, or the server refuses its own header with `-32020` instead of saying what is true: + no such tool.""" + replies = _session( + stdio_server, + [ + { + "jsonrpc": "2.0", + "id": 1, + "method": "initialize", + "params": {"protocolVersion": CURRENT}, + }, + { + "jsonrpc": "2.0", + "id": 2, + "method": "tools/call", + "params": {"name": "=?base64?not-really?=", "arguments": {}}, + }, + ], + ) + assert replies[1]["error"]["code"] == -32602 + assert "no tool named" in replies[1]["error"]["message"] + + +def test_T573_a_malformed_initialize_does_not_move_the_revision(stdio_server): + """The revision changes only when the initialize it came in on was accepted; a refused one + must not leave the loop on a revision the client never negotiated. Observed through the + legacy revision's one distinguishing mechanic: a JSON-RPC *response* body is permitted on + `2025-03-26` and refused on `2026-07-28`.""" + replies = _session( + stdio_server, + [ + { + "jsonrpc": "2.0", + "id": 1, + "method": "initialize", + "params": {"protocolVersion": CURRENT}, + }, + {"id": 2, "method": "initialize", "params": {"protocolVersion": "2025-03-26"}}, + {"jsonrpc": "2.0", "id": 3, "result": {}}, + ], + ) + assert replies[0]["result"]["protocolVersion"] == CURRENT + assert replies[1]["error"]["code"] == -32600 + assert replies[2]["error"]["code"] == -32600, ( + "still on 2026-07-28, where a response body is refused" + ) diff --git a/tests/test_repository_signals.py b/tests/test_repository_signals.py index cf9d4d0b..58ce2967 100644 --- a/tests/test_repository_signals.py +++ b/tests/test_repository_signals.py @@ -353,6 +353,15 @@ def test_the_registry_manifest_agrees_with_the_version_and_the_readme_marker(): assert packages[0]["registryType"] == "pypi" assert packages[0]["registryBaseUrl"] == "https://pypi.org" assert packages[0]["version"] == version + # The package a registry client installs is the stdio one (SPEC-mcp-operator §2.3): the + # loopback-HTTP mode needs a proxy in front of it and is not something a client can start + # from a manifest. The listing said `streamable-http` on 127.0.0.1 for one release and no + # desktop client could use it. + assert packages[0]["transport"] == {"type": "stdio"} + assert packages[0]["runtimeHint"] == "uvx" + arguments = [a.get("value") or a.get("name") for a in packages[0]["packageArguments"]] + assert arguments == ["mcp-operator", "--stdio"] + assert [v["name"] for v in packages[0]["environmentVariables"]] == ["CTRLRUN_CONFIG"] name = manifest["name"] # The registry's matcher is `strings.Index(description, "mcp-name: " + name)` followed by a