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