From 464d8e37e5d2ad0b0776ab96863f212222085b6f Mon Sep 17 00:00:00 2001 From: Brett Heap <1513478+brettheap@users.noreply.github.com> Date: Tue, 29 Sep 2026 11:38:22 +0000 Subject: [PATCH 01/11] Broker path: a minted token travels only by a private route (follows T080) A broker binding over plain http:// to any host but this one is now refused when it is declared, with the one ENDPOINT_NOT_PRIVATE sentence a built-in credential earns on the same route. `mint` asks the same predicate before it asks the broker for anything, as the built-in resolver does before it reads, so a binding forced past the record still mints nothing. Only the auth kind `none`, which presents no credential, keeps a route that is not private. This is the first of the four broker-path gaps openDox-code#63 listed for Brett. His word of 2026-09-29, given in-session on #63's closing question: "Yes, separate phase-3 draft (Recommended)". The T080 ruling it extends is recorded at openxFactory#656 comment 5880893901. ENDPOINT_NOT_PRIVATE now names both credentials. T080's two route lists become shared parametrize marks, so the broker cases reuse them and T080's node ids do not change. T080's case that pinned the old scope (a broker binding declared on these routes) keeps only its `none` half, over all nine. Tests: 17 broker cases and 9 `none` cases added, 2 removed. With T080's head 3f14bb96 as the source, the 11 gap cases fail (DID NOT RAISE) and the 15 controls pass. Arc: neutral-product-standalone-operability Lane: openxfactory-4 (openXfactory-4-openDox_extraction) Co-Authored-By: Claude Opus 5.5 (1M context) --- src/opendox/doxbench_binding.py | 63 ++++++++------- src/opendox/doxbench_provider.py | 16 +++- tests/test_model_provider_broker.py | 119 +++++++++++++++++++++++----- 3 files changed, 148 insertions(+), 50 deletions(-) diff --git a/src/opendox/doxbench_binding.py b/src/opendox/doxbench_binding.py index 57359d2..58493e6 100644 --- a/src/opendox/doxbench_binding.py +++ b/src/opendox/doxbench_binding.py @@ -37,14 +37,19 @@ * the BUILT-IN RESOLVER, for an `env:NAME` or `keyring:SERVICE/USERNAME` reference. It reads the reference at call time, inside `doxbench_provider` only (RULED R1Q17 (b)). Such a record needs no broker, and one given - beside it is refused, so no record has two resolvers. What it reads is a - LONG-LIVED key, so its endpoint must be a PRIVATE ROUTE: `https://`, or - `http://` to 127.0.0.1, ::1 or localhost (Brett Heap's ruling of - 2026-09-28, "Refuse unless loopback"; `is_a_private_route`); + beside it is refused, so no record has two resolvers; * NONE, for an endpoint that takes no credential. It declares the auth kind `none` rather than leaving a field out, and `credential_ref` and `broker_argv` are forbidden under it (RULED R1Q18 (a)). +WHATEVER A RECORD PRESENTS TRAVELS ONLY BY A PRIVATE ROUTE: `https://`, or +`http://` to 127.0.0.1, ::1 or localhost (`is_a_private_route`). That holds for +the built-in resolver's LONG-LIVED key (Brett Heap's ruling of 2026-09-28, +"Refuse unless loopback") and for a broker's minted token alike (his word of +2026-09-29 on openDox-code#63's closing question, "Yes, separate phase-3 +draft"). Only the auth kind `none`, which presents nothing, keeps the route it +declares. + This module classifies a reference's FORM when a binding is declared. It never reads what a reference names. @@ -213,15 +218,15 @@ class BuiltInReference(NamedTuple): #: on-this-host proxy posture an operator may legitimately run; a scheme this #: tuple does not name is refused at declaration, because `file://` or a bare #: host is not something a provider client should discover at dispatch time. -#: For a credential the built-in resolver reads, "on this host" is ENFORCED -#: (`is_a_private_route`). A broker's minted token and the auth kind `none` -#: keep the posture this tuple gives them, as the 2026-09-28 ruling leaves it. +#: For every record that presents a credential, "on this host" is ENFORCED +#: (`is_a_private_route`). The auth kind `none` presents none, so it keeps the +#: posture this tuple gives it. ENDPOINT_SCHEMES: tuple[str, ...] = ("https://", "http://") -#: The hosts a credential the built-in resolver reads may reach over plain -#: `http://`: this host, spelled exactly as Brett Heap's ruling of 2026-09-28 -#: names it ("Refuse unless loopback"). No other spelling of these addresses, -#: and no other address of the loopback range, is one of them. +#: The hosts a credential may reach over plain `http://`: this host, spelled +#: exactly as Brett Heap's ruling of 2026-09-28 names it ("Refuse unless +#: loopback"). No other spelling of these addresses, and no other address of +#: the loopback range, is one of them. LOOPBACK_HOSTS: tuple[str, ...] = ("127.0.0.1", "::1", "localhost") @@ -248,23 +253,24 @@ def is_a_private_route(endpoint: object) -> bool: """Whether `endpoint` keeps a credential from crossing a network in cleartext: `https://`, or `http://` to this host (`LOOPBACK_HOSTS`). - ONE PREDICATE. The record asks it when a binding is declared, and - `doxbench_provider`'s built-in resolver asks it again before it reads - anything.""" + ONE PREDICATE. The record asks it when a binding is declared. + `doxbench_provider` asks it again before the built-in resolver reads + anything, and before `mint` asks a broker for a token.""" return (isinstance(endpoint, str) and _PRIVATE_ROUTE.match(endpoint) is not None) -#: The refusal a credential the built-in resolver reads earns on a route that -#: is not private (Brett Heap's ruling of 2026-09-28, "Refuse unless -#: loopback"). That resolver reads a LONG-LIVED key, where a broker mints a -#: short-lived token, so plain `http://` carries one only to this host. A -#: fixed sentence, and it repeats nothing of the endpoint. +#: The refusal a record that presents a credential earns on a route that is +#: not private. Brett Heap ruled it on 2026-09-28 ("Refuse unless loopback") +#: for the built-in resolver's LONG-LIVED key. His word of 2026-09-29 gave a +#: broker's minted token the same rule. A short-lived token is still a +#: credential: sent in cleartext, it can be replayed by whoever reads it until +#: it expires. A fixed sentence, and it repeats nothing of the endpoint. ENDPOINT_NOT_PRIVATE = ( - "a credential the built-in resolver reads (an env: or keyring: reference) " - "is sent only over https://, or over http:// to this host (127.0.0.1, ::1 " - "or localhost), and this endpoint is neither; declare an https:// " - "endpoint, or a loopback one") + "a credential (a broker's minted token, or the key an env: or keyring: " + "reference names) is sent only over https://, or over http:// to this host " + "(127.0.0.1, ::1 or localhost), and this endpoint is neither; declare an " + "https:// endpoint, or a loopback one") #: The refusal a key inside the endpoint URL earns (#1144 box 16.3). Measured #: before 16.3: this record checked the endpoint's scheme and nothing else, so @@ -563,11 +569,12 @@ def _require_a_declarable_endpoint(self) -> None: f"{ENDPOINT_SCHEMES}") def _require_a_private_route(self) -> None: - """A CREDENTIAL THE BUILT-IN RESOLVER READS TRAVELS ONLY BY A PRIVATE - ROUTE (Brett Heap's ruling of 2026-09-28, "Refuse unless loopback"; - `is_a_private_route`). A broker's minted token and the auth kind - `none` keep the route they had, as the ruling leaves them.""" - if (self.credential_source() == CREDENTIAL_FROM_BUILT_IN_RESOLVER + """A CREDENTIAL TRAVELS ONLY BY A PRIVATE ROUTE (`is_a_private_route`), + whichever resolver answers it: the built-in resolver's key (Brett + Heap's ruling of 2026-09-28, "Refuse unless loopback") or a broker's + minted token (his word of 2026-09-29). The auth kind `none` presents + no credential, so its route is its own.""" + if (self.credential_source() != NO_CREDENTIAL and not is_a_private_route(self.endpoint)): raise BindingRefused(ENDPOINT_NOT_PRIVATE) diff --git a/src/opendox/doxbench_provider.py b/src/opendox/doxbench_provider.py index ab0956d..ab94f8e 100644 --- a/src/opendox/doxbench_provider.py +++ b/src/opendox/doxbench_provider.py @@ -628,7 +628,21 @@ def mint(binding, *, retry_of: str | None = None, WHERE the token is good and WHAT GRAMMAR that endpoint speaks come from the BINDING, not from this answer: the declaration emits neither and says why — the broker is provider-agnostic about the request grammar and will not name - an endpoint it would then be accountable for (0.2 FINDING 3).""" + an endpoint it would then be accountable for (0.2 FINDING 3). + + NO TOKEN IS ASKED FOR ON A ROUTE THAT IS NOT PRIVATE (Brett Heap's word of + 2026-09-29: a broker's minted token keeps the rules a built-in credential + keeps). The record refuses such a binding when it is declared + (`doxbench_binding.ENDPOINT_NOT_PRIVATE`), so no declared binding reaches + this check. It is repeated before the broker is asked all the same, as the + built-in resolver repeats it before it reads, because this is the function + that obtains the token. What reaches it is a programming error, and + nothing has been minted when it is raised.""" + if not binding_mod.is_a_private_route(binding.endpoint): + raise AssertionError( + f"binding {binding.id!r} would present a minted token over a " + "route that is not private, which the record refuses when it is " + "declared; nothing was minted") answer = runner(broker_operation_argv(binding, OPERATION_MINT, retry_of=retry_of)) document = _answer_document(answer, BROKER_MINT_KIND, MINT_FIELDS) diff --git a/tests/test_model_provider_broker.py b/tests/test_model_provider_broker.py index bfd7eda..7b49fdf 100644 --- a/tests/test_model_provider_broker.py +++ b/tests/test_model_provider_broker.py @@ -25,7 +25,9 @@ 034 phase 3, slice P3-B). 16.1 is the OpenAI-compatible dialect (T078), 16.2 is the model name the provider receives (T079), and 16.3 is the credential staying a reference: a key in the URL or an extra field refused, the built-in -`env:` and keyring resolver, and the auth kind `none` (T080). +`env:` and keyring resolver, and the auth kind `none` (T080). Its last section +holds a broker's minted token to the rules T080 gave a built-in credential +(the broker path's hardening, Brett Heap's word of 2026-09-29). THE FAKE BROKER SPEAKS THE DECLARED CONTRACT (task 2.6). It was this repository's own invented stdin/stdout protocol until the reconciliation, which @@ -2161,8 +2163,10 @@ def test_a_binding_no_broker_answers_has_no_broker_operation(): f"keyring:{KEYRING_SERVICE}/{KEYRING_USER}") BACKSLASH = chr(92) - -@pytest.mark.parametrize("endpoint", [ +#: The routes a credential may take, and the routes it may not. One list of +#: each, shared by the built-in resolver's cases here and the broker path's +#: cases below, because the rule is one rule. +ON_A_PRIVATE_ROUTE = pytest.mark.parametrize("endpoint", [ "https://api.example.invalid/v1/chat/completions", "http://127.0.0.1:8080/v1/chat/completions", "http://[::1]:8080/v1/chat/completions", @@ -2171,15 +2175,7 @@ def test_a_binding_no_broker_answers_has_no_broker_operation(): "http://localhost", ], ids=["https", "ipv4-loopback", "ipv6-loopback", "localhost", "localhost-in-capitals", "no-path"]) -def test_a_built_in_credential_is_declared_on_a_private_route(endpoint): - for reference in BUILT_IN_REFERENCES: - binding = _built_in_binding(reference, endpoint=endpoint) - assert binding.endpoint == endpoint - assert binding.credential_source() == ( - binding_mod.CREDENTIAL_FROM_BUILT_IN_RESOLVER) - - -@pytest.mark.parametrize("endpoint", [ +NOT_ON_A_PRIVATE_ROUTE = pytest.mark.parametrize("endpoint", [ "http://api.example.invalid/v1/chat/completions", "http://localhost.evil.com/v1/chat/completions", "http://127.0.0.1.evil.com/v1/chat/completions", @@ -2193,6 +2189,18 @@ def test_a_built_in_credential_is_declared_on_a_private_route(endpoint): "localhost-only-in-the-path", "a-loopback-address-not-named", "another-spelling-of-ipv6-loopback", "trailing-dot", "percent-encoded-dot", "unspecified-address"]) + + +@ON_A_PRIVATE_ROUTE +def test_a_built_in_credential_is_declared_on_a_private_route(endpoint): + for reference in BUILT_IN_REFERENCES: + binding = _built_in_binding(reference, endpoint=endpoint) + assert binding.endpoint == endpoint + assert binding.credential_source() == ( + binding_mod.CREDENTIAL_FROM_BUILT_IN_RESOLVER) + + +@NOT_ON_A_PRIVATE_ROUTE def test_a_built_in_credential_over_http_to_another_host_is_refused(endpoint): """Refused when it is declared, by the constructor and from a stored record alike, with the one fixed sentence.""" @@ -2305,15 +2313,12 @@ def test_a_broker_reference_in_a_built_in_form_is_malformed(tmp_path): assert caught.value.diagnostic == provider_mod.DIAG_BROKER_MALFORMED -@pytest.mark.parametrize("endpoint", [ - "http://api.example.invalid/turn", "http://localhost.evil.com/turn"]) -def test_the_loopback_rule_is_the_built_in_resolvers_alone(endpoint): - """The ruling leaves the broker path as it is today: a broker's minted - token may still be declared over plain http:// to any host, which is the - pre-existing gap the PR notes. The auth kind `none` presents no - credential, so it keeps its route too.""" - assert _binding(endpoint=endpoint).credential_source() == ( - binding_mod.CREDENTIAL_FROM_BROKER) +@NOT_ON_A_PRIVATE_ROUTE +def test_a_none_binding_keeps_a_route_that_is_not_private(endpoint): + """The auth kind `none` presents no credential, so it is the one kind + that keeps such a route. Until the broker path's hardening (the last + section of this file), this case also declared a broker binding on these + routes, pinning T080's scope. That half is now refused.""" assert _none_binding(endpoint=endpoint).credential_source() == ( binding_mod.NO_CREDENTIAL) @@ -2903,3 +2908,75 @@ def test_set_credential_refuses_a_binding_no_broker_answers(tmp_path, capsys, args, source=_UnreadableSource()) == 1 assert "names no broker" in capsys.readouterr().err assert store.get(binding.id) == binding, "nothing changed" + + +# --- the broker path keeps the same rules (follows T080) ----------------- +# Brett Heap's word of 2026-09-29, answering openDox-code#63's closing +# question ("Should the broker path follow it?"): "Yes, separate phase-3 +# draft". A broker's minted token keeps every rule T080 gave a credential the +# built-in resolver reads. At `main`, and at T080's head, the broker path had +# four gaps, each measured over real sockets and a real broker child: +# +# 1. the token could be declared over plain http:// to any host; +# 2. a redirect, or an environment proxy, carried it elsewhere; +# 3. a provider-unreachable refusal chained urllib's error, whose frames +# held the token in their locals; +# 4. a token that cannot be presented went to urllib as it was, so one +# outside latin-1 failed there as DIAG_PROVIDER_UNREACHABLE. +# +# Each gap's cases below fail at T080's head, and the controls beside them +# (a private route declared, a presentable token presented) pass there too. +# Every token here is an obvious fake. + + +@ON_A_PRIVATE_ROUTE +def test_a_broker_token_is_declared_on_a_private_route(endpoint): + binding = _binding(endpoint=endpoint, dialect=OPENAI_CHAT) + assert binding.endpoint == endpoint + assert binding.credential_source() == binding_mod.CREDENTIAL_FROM_BROKER + + +@NOT_ON_A_PRIVATE_ROUTE +def test_a_broker_token_over_http_to_another_host_is_refused(endpoint): + """Gap 1. Refused when it is declared, by the constructor and from a + stored record alike, with the fixed sentence a built-in credential + earns on the same route.""" + with pytest.raises(binding_mod.BindingRefused) as caught: + _binding(endpoint=endpoint, dialect=OPENAI_CHAT) + assert str(caught.value) == binding_mod.ENDPOINT_NOT_PRIVATE + record = dict(_binding().as_record(), endpoint=endpoint) + with pytest.raises(binding_mod.BindingRefused) as caught: + binding_mod.ModelProviderBinding.from_record(record) + assert str(caught.value) == binding_mod.ENDPOINT_NOT_PRIVATE + + +def test_mint_asks_no_broker_for_a_token_on_a_route_that_is_not_private( + tmp_path): + """Gap 1, in `mint` itself, as the built-in resolver checks before it + reads. The record refuses such a binding when it is declared, so this + one is forced past that check, as no declaration can do. It still + cannot make a broker mint.""" + script = _write_broker(tmp_path) + binding = _broker_binding(script) + object.__setattr__(binding, "endpoint", "http://api.example.invalid/v1") + with pytest.raises(AssertionError) as caught: + provider_mod.mint(binding) + assert "nothing was minted" in str(caught.value) + assert _seen_all(script) == [], "the broker was never asked" + + +def test_the_cli_refuses_a_broker_binding_over_http_to_another_host( + tmp_path, capsys): + checkout = tmp_path / "checkout" + checkout.mkdir() + args = cli_mod.build_parser().parse_args([ + "model-binding", "add", "--repo-root", str(checkout), + "--id", "cleartext-broker", "--label", "L", "--provider", "local", + "--credential-ref", FAKE_REFERENCE, "--auth-kind", "api_key", + "--credential-approver", "brett@opensoft.one", + "--endpoint", "http://api.example.invalid/v1/chat/completions", + "--dialect", OPENAI_CHAT, "--", "openprofiler-broker"]) + assert args.func(args) == 1 + assert binding_mod.ENDPOINT_NOT_PRIVATE in capsys.readouterr().err + store = binding_mod.BindingStore(binding_mod.bindings_path(checkout)) + assert store.list() == (), "nothing is stored" From 823dc0332aabfb7a79c3760d0ab1d8de615b36da Mon Sep 17 00:00:00 2001 From: Brett Heap <1513478+brettheap@users.noreply.github.com> Date: Tue, 29 Sep 2026 11:43:59 +0000 Subject: [PATCH 02/11] Broker path: a minted token follows no redirect, uses no proxy, and chains nothing Gaps 2 and 3 of the four broker-path gaps openDox-code#63 listed for Brett, closed with T080's own rules for a built-in credential (his word of 2026-09-29, given in-session). One port method, `_call_provider`, now makes every provider request, on both paths. A request that carries a credential (a broker's minted token, or what the built-in resolver read) goes through `_open_with_a_credential`, which is T080's opener renamed. It declines every redirect, and over plain http:// it uses no proxy. Its refusal is raised afresh, with no cause and no context. The auth kind `none` presents nothing, so it keeps the default opener and its refusal's chain, as before. The broker branch of `dispatch` answers an expiry outside every handler. The one re-mint and the one paid retry of the 2026-08-26 ruling do what they did, and a refusal raised by either keeps no context. DIAG_PROVIDER_REDIRECTED and the redirect handler now name both credentials. Measured at T080's head 3f14bb96, over real sockets and a real broker child: - a POST answered 301, 302 or 303 reached the redirect's target as a GET that carried the token; - with http_proxy set, a request to 127.0.0.1 went to the proxy with it; - a refused connection chained urllib's URLError, and do_open.headers, _send_request.headers, _send_output.msg, send.data and request.headers held the token. Tests: 12 broker cases. With 3f14bb96 as the source all 12 fail, and T080's two refactored cases pass. T080's redirect and proxy cases now share their scaffolds with the broker's (`_a_provider_that_redirects`, `_an_environment_proxy`). The proxy scaffold drops urlopen's cached global opener, so the proxy environment is read the way a process started with it reads it. The ENDPOINT_NOT_PRIVATE sentence is rewrapped, with the same text. Arc: neutral-product-standalone-operability Lane: openxfactory-4 (openXfactory-4-openDox_extraction) Co-Authored-By: Claude Opus 5.5 (1M context) --- src/opendox/doxbench_binding.py | 6 +- src/opendox/doxbench_provider.py | 143 ++++++++++++++---------- tests/test_model_provider_broker.py | 167 +++++++++++++++++++++++++--- 3 files changed, 239 insertions(+), 77 deletions(-) diff --git a/src/opendox/doxbench_binding.py b/src/opendox/doxbench_binding.py index 58493e6..477690f 100644 --- a/src/opendox/doxbench_binding.py +++ b/src/opendox/doxbench_binding.py @@ -268,9 +268,9 @@ def is_a_private_route(endpoint: object) -> bool: #: it expires. A fixed sentence, and it repeats nothing of the endpoint. ENDPOINT_NOT_PRIVATE = ( "a credential (a broker's minted token, or the key an env: or keyring: " - "reference names) is sent only over https://, or over http:// to this host " - "(127.0.0.1, ::1 or localhost), and this endpoint is neither; declare an " - "https:// endpoint, or a loopback one") + "reference names) is sent only over https://, or over http:// to this " + "host (127.0.0.1, ::1 or localhost), and this endpoint is neither; " + "declare an https:// endpoint, or a loopback one") #: The refusal a key inside the endpoint URL earns (#1144 box 16.3). Measured #: before 16.3: this record checked the endpoint's scheme and nothing else, so diff --git a/src/opendox/doxbench_provider.py b/src/opendox/doxbench_provider.py index ab94f8e..08e0123 100644 --- a/src/opendox/doxbench_provider.py +++ b/src/opendox/doxbench_provider.py @@ -251,19 +251,19 @@ "the OS keyring could not be read by this process, so the keyring " "reference could not be resolved") -#: The answer to a redirect of a request that carried a credential the -#: built-in resolver read. That request follows no redirect (see -#: `_DeclineRedirects`), so the credential went to the declared endpoint and -#: nowhere else, and the sentence says what to declare instead. +#: The answer to a redirect of a request that carried a credential: a broker's +#: minted token, or what the built-in resolver read. That request follows no +#: redirect (see `_DeclineRedirects`), so the credential went to the declared +#: endpoint and nowhere else, and the sentence says what to declare instead. DIAG_PROVIDER_REDIRECTED = ( - "the provider answered with a redirect, which a credential the built-in " - "resolver reads does not follow, so it was sent nowhere else; declare " - "the endpoint the provider redirects to") + "the provider answered with a redirect, which a request carrying a " + "credential does not follow, so the credential was sent nowhere else; " + "declare the endpoint the provider redirects to") #: The closed set, so a test can assert no other sentence can be raised. #: ELEVEN: the eight the reconciliation left, the built-in resolver's two -#: (#1144 box 16.3), and the redirect a request carrying a built-in -#: credential declines. `DIAG_DIALECT_UNKNOWN` is gone because the fact it guarded +#: (#1144 box 16.3), and the redirect a request carrying a credential +#: declines. `DIAG_DIALECT_UNKNOWN` is gone because the fact it guarded #: moved: the dialect is the BINDING's, validated against the closed vocabulary #: when the operator declares it #: (`doxbench_binding.ModelProviderBinding.__post_init__`), so an unknown @@ -915,8 +915,8 @@ def authorization(self) -> str: class _Redirected(Exception): - """A provider answered a request carrying a built-in credential with a - redirect, and the redirect was declined. + """A provider answered a request carrying a credential with a redirect, + and the redirect was declined. PRIVATE and never raised out of this module: the port answers it with `DIAG_PROVIDER_REDIRECTED` before any caller sees anything.""" @@ -929,20 +929,23 @@ class _DeclineRedirects(urllib.request.HTTPRedirectHandler): `urllib`'s own handler re-sends a request's headers, all but the content ones, to whatever `Location` the provider names, whatever its host and scheme. Measured: a POST answered 301, 302 or 303 reaches the redirect's - target as a GET that still carries `Authorization: Bearer ...`. The - loopback ruling of 2026-09-28 sends a credential the built-in resolver - reads only by a private route, and a followed redirect would send it by - any route. So a request that carries one declines every redirect, with - the redirect's answer closed unread.""" + target as a GET that still carries `Authorization: Bearer ...`, whether + the bearer is a built-in credential or a broker's minted token. A + credential travels only by a private route (the loopback ruling of + 2026-09-28, which Brett Heap's word of 2026-09-29 gave a minted token + too), and a followed redirect would send it by any route. So a request + that carries one declines every redirect, with the redirect's answer + closed unread.""" def redirect_request(self, req, fp, code, msg, headers, newurl): fp.close() raise _Redirected -def _open_for_a_built_in_credential(request, *, timeout): - """`urllib.request.urlopen` for a request that carries a credential the - built-in resolver read. It changes two things, and nothing else. +def _open_with_a_credential(request, *, timeout): + """`urllib.request.urlopen` for a request that carries a credential: a + broker's minted token, or what the built-in resolver read. It changes two + things, and nothing else. * EVERY REDIRECT IS DECLINED (`_DeclineRedirects`). * A PLAIN-`http://` REQUEST GOES DIRECT, whatever proxy the environment @@ -1201,7 +1204,14 @@ def dispatch(self, prompt_envelope: object) -> object: what every request sent before the field existed, byte for byte. A RECORD NO BROKER ANSWERS takes `_dispatch_without_a_broker` instead - (#1144 box 16.3): no mint, no ledger event, and no retry.""" + (#1144 box 16.3): no mint, no ledger event, and no retry. + + EITHER WAY, THE PROVIDER IS CALLED THROUGH `_call_provider`, so a + broker's minted token keeps every rule a built-in credential keeps: no + redirect, no proxy over plain `http://`, and a refusal that chains + nothing (Brett Heap's word of 2026-09-29). The re-mint and the retry + above therefore happen outside every handler, so a refusal raised by + either keeps no context either.""" handle = getattr(prompt_envelope, "model_id", None) if not isinstance(handle, str) or not handle: entries = self._declared_catalog.entries @@ -1215,13 +1225,10 @@ def dispatch(self, prompt_envelope: object) -> object: model=model, prompt=prompt), "proposals": []} token = self._current_token(REASON_FIRST_MINT) - try: - prose = _post_to_provider( - endpoint=token.endpoint, dialect=token.dialect, - credential=_PresentedCredential(token.token), model=model, - prompt=prompt, timeout=self._timeout_seconds, - opener=self._opener) - except _TokenExpired: + prose = self._call_provider( + _PresentedCredential(token.token), endpoint=token.endpoint, + dialect=token.dialect, model=model, prompt=prompt) + if prose is None: # PER-TURN STATE, and no longer than the turn: the expired mint's # own audit reference, read before the token is dropped, so the # re-mint can name what it replaces. @@ -1231,15 +1238,12 @@ def dispatch(self, prompt_envelope: object) -> object: token = self._current_token(REASON_EXPIRY_REMINT, retry_of=replaced) self._record(REASON_PAID_RETRY) - try: - prose = _post_to_provider( - endpoint=token.endpoint, dialect=token.dialect, - credential=_PresentedCredential(token.token), model=model, - prompt=prompt, timeout=self._timeout_seconds, - opener=self._opener) - except _TokenExpired: + prose = self._call_provider( + _PresentedCredential(token.token), endpoint=token.endpoint, + dialect=token.dialect, model=model, prompt=prompt) + if prose is None: self._forget_token() - raise BrokerRefused(DIAG_TOKEN_EXPIRED_TWICE) from None + raise BrokerRefused(DIAG_TOKEN_EXPIRED_TWICE) return {"assistant_prose": prose, "proposals": []} def _dispatch_without_a_broker(self, *, model: str, prompt: str) -> str: @@ -1255,21 +1259,11 @@ def _dispatch_without_a_broker(self, *, model: str, prompt: str) -> str: A 401 HERE IS A REFUSAL, NOT AN EXPIRY. The 2026-08-26 retry ruling is about a MINTED token outliving its turn, and here there is no mint to repeat: the reference names the same value on a second read, so a - retry would buy a second paid call for the same refusal. + retry would buy a second paid call for the same refusal. It is raised + outside every handler, so it keeps no context. - A REQUEST CARRYING A BUILT-IN CREDENTIAL FOLLOWS NO REDIRECT AND, OVER - PLAIN `http://`, USES NO PROXY. The default opener does both, and - sends the credential header along each time, so such a request uses - `_open_for_a_built_in_credential` in its place. An opener a caller - injected is that caller's own seam and is used as given. The auth - kind `none` sends no credential, and a broker's minted token keeps - the default opener, as the 2026-09-28 ruling leaves that path. - - A REFUSAL OF A REQUEST THAT CARRIED A BUILT-IN CREDENTIAL CHAINS - NOTHING. The credential stays wrapped in a `_PresentedCredential` in - every frame here, and the refusal is raised afresh, with no cause and - no context, so no traceback it carries reaches a frame inside - `urllib` whose locals hold the request's headers.""" + The request itself is made through `_call_provider`, which keeps the + rules for a request that carries a credential.""" credential = None if (self._binding.credential_source() == binding_mod.CREDENTIAL_FROM_BUILT_IN_RESOLVER): @@ -1283,27 +1277,58 @@ def _dispatch_without_a_broker(self, *, model: str, prompt: str) -> str: raise with self._lock: self._available = True + prose = self._call_provider( + credential, endpoint=self._binding.endpoint, + dialect=self._binding.dialect, model=model, prompt=prompt) + if prose is None: + raise BrokerRefused(DIAG_PROVIDER_REFUSED) + return prose + + def _call_provider(self, credential: _PresentedCredential | None, *, + endpoint: str, dialect: str, model: str, + prompt: str) -> str | None: + """ONE provider call, under the rules a request that carries a + credential keeps, whichever resolver answered it: a broker's minted + token, or what the built-in resolver read. Returns the prose, or None + when the provider said the credential is no longer valid (a 401), which + each path answers in its own way. + + A REQUEST THAT CARRIES A CREDENTIAL FOLLOWS NO REDIRECT AND, OVER + PLAIN `http://`, USES NO PROXY. The default opener does both, and + sends the credential header along each time, so such a request uses + `_open_with_a_credential` in its place. An opener a caller injected is + that caller's own seam, and it is used as given. + + A REFUSAL OF A REQUEST THAT CARRIED A CREDENTIAL CHAINS NOTHING. The + credential stays wrapped in a `_PresentedCredential` in every frame + here, and the refusal is raised afresh, with no cause and no context, + so no traceback it carries reaches a frame inside `urllib` whose locals + hold the request's headers (Copilot's review of openDox-code#63 at + `d240fd50`). + + T080 gave these rules to the built-in resolver's key. Brett Heap's + word of 2026-09-29 gave them to a broker's minted token too. The auth + kind `none` presents nothing, so its request keeps the default opener, + and its refusal is raised as `_post_to_provider` raised it.""" opener = self._opener if credential is not None and opener is urllib.request.urlopen: - opener = _open_for_a_built_in_credential - failure = None + opener = _open_with_a_credential try: return _post_to_provider( - endpoint=self._binding.endpoint, dialect=self._binding.dialect, - credential=credential, model=model, prompt=prompt, - timeout=self._timeout_seconds, opener=opener) + endpoint=endpoint, dialect=dialect, credential=credential, + model=model, prompt=prompt, timeout=self._timeout_seconds, + opener=opener) except _TokenExpired: - failure = DIAG_PROVIDER_REFUSED + return None except _Redirected: failure = DIAG_PROVIDER_REDIRECTED except BrokerRefused as refusal: if credential is None: raise failure = refusal.diagnostic - # RAISED HERE, OUTSIDE EVERY HANDLER, so the refusal chains nothing - # (Copilot's review of openDox-code#63 at `d240fd50`). A cause chained - # from inside `urllib` keeps frames whose locals hold the request's - # headers, and so the credential. + # RAISED HERE, OUTSIDE EVERY HANDLER, so the refusal chains nothing. A + # cause chained from inside `urllib` keeps frames whose locals hold the + # request's headers, and so the credential. raise BrokerRefused(failure) # -- token custody ------------------------------------------------------ diff --git a/tests/test_model_provider_broker.py b/tests/test_model_provider_broker.py index 7b49fdf..ec98a35 100644 --- a/tests/test_model_provider_broker.py +++ b/tests/test_model_provider_broker.py @@ -60,6 +60,7 @@ import time import types import urllib.error +import urllib.request from datetime import datetime, timezone from pathlib import Path @@ -891,14 +892,15 @@ def _expired_error(): def _port(tmp_path, *outcomes, expires=None, notice=None, clock=time.time, endpoint=ENDPOINT, dialect=binding_mod.DIALECT_XFACTORY_PROMPT_V1, - model=None): - script = _write_broker(tmp_path, expires=expires) + model=None, token=SENTINEL_TOKEN, + runner=provider_mod.subprocess_broker_runner): + script = _write_broker(tmp_path, expires=expires, token=token) binding = _broker_binding(script, endpoint=endpoint, dialect=dialect, model=model) opener = _Opener(*outcomes) port = provider_mod.BrokeredProviderPort( binding, install_mod.brokered_catalog(binding), - opener=opener, clock=clock, + runner=runner, opener=opener, clock=clock, notice=notice if notice is not None else (lambda _text: None)) return port, opener @@ -2641,6 +2643,40 @@ def log_message(self, *_args): return +@contextlib.contextmanager +def _a_provider_that_redirects(monkeypatch, code): + """A stand-in provider that answers every request with a `code` redirect + to a second stand-in, `_ElsewhereHandler`, which records whatever it is + sent. It yields the first one's base URL.""" + monkeypatch.setattr(_ElsewhereHandler, "seen", []) + monkeypatch.setattr(_RedirectingHandler, "code", code) + with _stand_in_provider(_ElsewhereHandler) as elsewhere, \ + _stand_in_provider(_RedirectingHandler) as base: + monkeypatch.setattr(_RedirectingHandler, "location", + f"{elsewhere}/v1/chat/completions") + yield base + + +@contextlib.contextmanager +def _an_environment_proxy(monkeypatch): + """A stand-in proxy that `http_proxy` names, recording into + `_ElsewhereHandler.seen`, for the length of one test. + + `urlopen`'s global opener is dropped first. `urlopen` builds it on first + use and reads the proxy environment then, so a request made through + `urlopen` here reads this environment, as it would in a process started + with the variable set. Without that, a proxy case would pass or fail by + whichever earlier test built the opener.""" + monkeypatch.setattr(_ElsewhereHandler, "seen", []) + monkeypatch.setattr(urllib.request, "_opener", None) + for name in ("no_proxy", "NO_PROXY"): + monkeypatch.delenv(name, raising=False) + with _stand_in_provider(_ElsewhereHandler) as proxy: + for name in ("http_proxy", "HTTP_PROXY"): + monkeypatch.setenv(name, proxy) + yield proxy + + @pytest.mark.parametrize("code", [301, 302, 303, 307, 308]) def test_a_built_in_credential_follows_no_redirect(monkeypatch, code): """Copilot's review of openDox-code#63 at `4abc6d4d`, over real sockets. @@ -2648,12 +2684,7 @@ def test_a_built_in_credential_follows_no_redirect(monkeypatch, code): GET to the `Location`, with the credential header still on it (measured). A request that carries a built-in credential declines the redirect, and the second server hears nothing at all.""" - monkeypatch.setattr(_ElsewhereHandler, "seen", []) - monkeypatch.setattr(_RedirectingHandler, "code", code) - with _stand_in_provider(_ElsewhereHandler) as elsewhere, \ - _stand_in_provider(_RedirectingHandler) as base: - monkeypatch.setattr(_RedirectingHandler, "location", - f"{elsewhere}/v1/chat/completions") + with _a_provider_that_redirects(monkeypatch, code) as base: binding = _built_in_binding(endpoint=f"{base}/v1/chat/completions") port = provider_mod.BrokeredProviderPort( binding, install_mod.brokered_catalog(binding), @@ -2673,14 +2704,9 @@ def test_a_built_in_credential_over_http_to_this_host_uses_no_proxy( `http_proxy` set, urllib's default opener sends a request addressed to `127.0.0.1` to the proxy, credential header and all (measured). This request goes direct, and the stand-in proxy hears nothing.""" - monkeypatch.setattr(_ElsewhereHandler, "seen", []) monkeypatch.setattr(_ChatCompletionsHandler, "seen", {}) - for name in ("no_proxy", "NO_PROXY"): - monkeypatch.delenv(name, raising=False) - with _stand_in_provider(_ElsewhereHandler) as proxy, \ + with _an_environment_proxy(monkeypatch), \ _stand_in_provider(_ChatCompletionsHandler) as base: - for name in ("http_proxy", "HTTP_PROXY"): - monkeypatch.setenv(name, proxy) binding = _built_in_binding(endpoint=f"{base}/v1/chat/completions") port = provider_mod.BrokeredProviderPort( binding, install_mod.brokered_catalog(binding), @@ -2980,3 +3006,114 @@ def test_the_cli_refuses_a_broker_binding_over_http_to_another_host( assert binding_mod.ENDPOINT_NOT_PRIVATE in capsys.readouterr().err store = binding_mod.BindingStore(binding_mod.bindings_path(checkout)) assert store.list() == (), "nothing is stored" + + +def _minting_port(tmp_path, endpoint, *, token=SENTINEL_TOKEN): + """A port as a served install builds one, for a binding at `endpoint` + whose broker, a real child, mints `token`. It keeps the default opener, + so nothing stands between the port and the socket.""" + binding = _broker_binding(_write_broker(tmp_path, token=token), + endpoint=endpoint, dialect=OPENAI_CHAT) + return provider_mod.BrokeredProviderPort( + binding, install_mod.brokered_catalog(binding), + notice=lambda _text: None) + + +def _refused_turn(port) -> provider_mod.BrokerRefused: + """The refusal one turn on `port` ends in.""" + envelope = _Envelope() + with pytest.raises(provider_mod.BrokerRefused) as caught: + port.dispatch(envelope) + return caught.value + + +@pytest.mark.parametrize("code", [301, 302, 303, 307, 308]) +def test_a_broker_token_follows_no_redirect(tmp_path, monkeypatch, code): + """Gap 2, over real sockets. At T080's head a POST answered 301, 302 or + 303 reached the redirect's target as a GET that still carried the minted + token, and the turn was answered from there. A 307 or 308 read as the + provider refusing. The redirect is declined, the second server hears + nothing, and the refusal chains nothing.""" + with _a_provider_that_redirects(monkeypatch, code) as base: + refusal = _refused_turn( + _minting_port(tmp_path, f"{base}/v1/chat/completions")) + assert refusal.diagnostic == provider_mod.DIAG_PROVIDER_REDIRECTED + assert _ElsewhereHandler.seen == [], "the token went nowhere else" + assert refusal.__cause__ is None + assert refusal.__context__ is None + + +def test_a_broker_token_over_http_to_this_host_uses_no_proxy(tmp_path, + monkeypatch): + """Gap 2, over real sockets. With `http_proxy` set, T080's head sent a + request that carried a minted token to the proxy, even one addressed to + `127.0.0.1` (measured with a fresh global opener, as in a process + started with the variable set). The request goes direct, and the + stand-in proxy hears nothing.""" + monkeypatch.setattr(_ChatCompletionsHandler, "seen", {}) + with _an_environment_proxy(monkeypatch), \ + _stand_in_provider(_ChatCompletionsHandler) as base: + answer = _minting_port( + tmp_path, f"{base}/v1/chat/completions").dispatch(_Envelope()) + assert answer["assistant_prose"] == "answered in the chat grammar" + assert _ChatCompletionsHandler.seen["authorization"] == ( + f"Bearer {SENTINEL_TOKEN}") + assert _ElsewhereHandler.seen == [], "the proxy heard nothing" + + +def test_a_refused_connection_keeps_no_frame_that_holds_the_token(tmp_path): + """Gap 3, over the real transport. At T080's head this refusal chained + urllib's `URLError`, and five frames it kept held the token in their + locals (measured: `do_open.headers`, `_send_request.headers`, + `_send_output.msg`, `send.data` and `request.headers`). The refusal + chains nothing, and no frame it keeps holds the token.""" + with _a_closed_loopback_port() as closed: + refusal = _refused_turn(_minting_port( + tmp_path, f"http://127.0.0.1:{closed}/v1/chat/completions")) + assert refusal.diagnostic == provider_mod.DIAG_PROVIDER_UNREACHABLE + assert refusal.__cause__ is None + assert refusal.__context__ is None + assert _locals_holding(refusal, SENTINEL_TOKEN) == [] + + +@pytest.mark.parametrize("outcomes,expected", [ + ((urllib.error.URLError("down"),), + provider_mod.DIAG_PROVIDER_UNREACHABLE), + ((urllib.error.HTTPError(ENDPOINT, 500, "boom", {}, + io.BytesIO(b"provider detail")),), + provider_mod.DIAG_PROVIDER_REFUSED), + ((b"not json",), provider_mod.DIAG_PROVIDER_MALFORMED), + ((_expired_error(), _expired_error()), + provider_mod.DIAG_TOKEN_EXPIRED_TWICE), +], ids=["unreachable", "refused", "malformed", "expired-twice"]) +def test_a_refusal_of_a_turn_that_presented_a_token_chains_nothing( + tmp_path, outcomes, expected): + """Gap 3, for each refusal a presented token can meet. At T080's head + each of them kept urllib's error, or the expiry, as its cause or its + context.""" + port, _opener = _port(tmp_path, *outcomes) + refusal = _refused_turn(port) + assert refusal.diagnostic == expected + assert refusal.__cause__ is None + assert refusal.__context__ is None + + +def test_a_re_mint_the_broker_refuses_after_an_expiry_keeps_no_context( + tmp_path): + """The one re-mint the 2026-08-26 ruling allows happens outside every + handler, so the broker's refusal of it keeps no context. At T080's head + it kept the expiry, which kept urllib's error.""" + asked: list = [] + + def refuses_a_second_mint(argv, **kwargs): + asked.append(argv) + if len(asked) > 1: + raise provider_mod.BrokerRefused(provider_mod.DIAG_BROKER_REFUSED) + return provider_mod.subprocess_broker_runner(argv, **kwargs) + + port, opener = _port(tmp_path, _expired_error(), + runner=refuses_a_second_mint) + refusal = _refused_turn(port) + assert refusal.diagnostic == provider_mod.DIAG_BROKER_REFUSED + assert len(opener.requests) == 1, "no paid retry without a token" + assert refusal.__context__ is None From 7d465b6a291781275819e60b042e3aed0db63012 Mon Sep 17 00:00:00 2001 From: Brett Heap <1513478+brettheap@users.noreply.github.com> Date: Tue, 29 Sep 2026 11:47:44 +0000 Subject: [PATCH 03/11] Broker path: a minted token must be presentable, and no mint refusal keeps it Gap 4 of the four broker-path gaps openDox-code#63 listed for Brett, closed with T080's own rule for a built-in credential (his word of 2026-09-29, given in-session). `mint` now asks `_presentable`, the built-in resolver's own test, of the token: non-empty printable ASCII with no whitespace. Any other token is a malformed answer (DIAG_BROKER_MALFORMED). It is refused before the port holds it or any provider is contacted, so the catalog reads unavailable and no mint is recorded. At T080's head 3f14bb96: - a token outside latin-1 failed inside urllib as DIAG_PROVIDER_UNREACHABLE, which names the wrong party, with do_open.headers and putheader.values holding it; - one with a space or another non-ASCII character was sent as it was. The refusal keeps no frame that holds the token, as the built-in resolver's refusal of an unpresentable value keeps none. `mint` reads the answer through `_minted_token` and raises any refusal again, afresh, after the answer has left its frame. That covers every refusal of the mint answer, which goes one step past the fourth gap's letter: a malformed answer beside a good token also kept it at 3f14bb96 (mint.answer, mint.document, _answer_document.text and _answer_document.document). Not changed: the broker runner's own refusals (a non-zero exit, an answer past the bound, a timeout) still keep what a misbehaving broker wrote. The runner is shared by all four broker operations and sits outside the provider call the ruling names. The PR flags it for Brett. Tests: 15 cases. With 3f14bb96 as the source the 13 gap cases fail, and the 2 controls pass (every printable ASCII character but the space is presented unchanged; the declared answer mints). Arc: neutral-product-standalone-operability Lane: openxfactory-4 (openXfactory-4-openDox_extraction) Co-Authored-By: Claude Opus 5.5 (1M context) --- src/opendox/doxbench_provider.py | 52 +++++++++++-- tests/test_model_provider_broker.py | 116 ++++++++++++++++++++++++++++ 2 files changed, 160 insertions(+), 8 deletions(-) diff --git a/src/opendox/doxbench_provider.py b/src/opendox/doxbench_provider.py index 08e0123..d9c3f10 100644 --- a/src/opendox/doxbench_provider.py +++ b/src/opendox/doxbench_provider.py @@ -637,7 +637,20 @@ def mint(binding, *, retry_of: str | None = None, this check. It is repeated before the broker is asked all the same, as the built-in resolver repeats it before it reads, because this is the function that obtains the token. What reaches it is a programming error, and - nothing has been minted when it is raised.""" + nothing has been minted when it is raised. + + A TOKEN THAT CANNOT BE PRESENTED AS IT IS, IS REFUSED, with + `DIAG_BROKER_MALFORMED`. The test is the built-in resolver's own + (`_presentable`), and the refusal comes before the port holds the token + or any provider is contacted. Measured at T080's head: a token outside + latin-1 failed inside `urllib` as `DIAG_PROVIDER_UNREACHABLE`, which + names the wrong party, and one with a space or another non-ASCII + character was sent as it was. + + NO REFUSAL OF THE ANSWER KEEPS IT. The answer carries the token, so every + refusal raised once it is read is raised again here, afresh, after the + answer has left this frame. It keeps no frame, cause or context that + holds the token, as the built-in resolver lets go of what it read.""" if not binding_mod.is_a_private_route(binding.endpoint): raise AssertionError( f"binding {binding.id!r} would present a minted token over a " @@ -645,9 +658,28 @@ def mint(binding, *, retry_of: str | None = None, "declared; nothing was minted") answer = runner(broker_operation_argv(binding, OPERATION_MINT, retry_of=retry_of)) + try: + return _minted_token(answer, binding) + except BrokerRefused as refusal: + failure = refusal.diagnostic + del answer + raise BrokerRefused(failure) + + +def _minted_token(answer: object, binding) -> MintedToken: + """A mint answer, read EXACTLY (`_answer_document`), as a `MintedToken`. + + Every refusal is `DIAG_BROKER_MALFORMED`: an answer of another shape, a + token that cannot be presented as it is (`_presentable`, which also + refuses one that is not a string or is blank), or an expiry or an audit + reference the declaration does not allow. `mint` raises each one again, + holding nothing of the answer.""" document = _answer_document(answer, BROKER_MINT_KIND, MINT_FIELDS) + token = document["token"] + if not _presentable(token): + raise BrokerRefused(DIAG_BROKER_MALFORMED) return MintedToken( - token=_declared_string(document, "token"), + token=token, expires_at=_parse_expires_at(document["expires_at"]), endpoint=binding.endpoint, dialect=binding.dialect, @@ -691,8 +723,9 @@ def list_references(binding, *, runner=subprocess_broker_runner) -> list: # --------------------------------------------------------------------------- def _presentable(value: object) -> bool: - """Whether a resolved value can be presented AS IT IS, as the bearer - credential of the request's Authorization header. + """Whether a credential can be presented AS IT IS, as the bearer + credential of the request's Authorization header. It is asked of a value + the built-in resolver read, and of a token a broker minted (`mint`). It must be a non-empty string of printable ASCII with no whitespace, which a bearer credential is by its grammar (RFC 6750's `b64token` is narrower @@ -706,7 +739,8 @@ def _presentable(value: object) -> bool: * any other non-ASCII character, and an embedded space, is SENT, as a credential the grammar does not allow. Trimming or re-encoding the value would present a credential other than - the one the reference names, so the value is refused instead.""" + the one the reference names or the broker minted, so the value is + refused instead.""" return (isinstance(value, str) and value != "" and all("!" <= character <= "~" for character in value)) @@ -895,9 +929,11 @@ class _PresentedCredential: of openDox-code#63 at `d240fd50`). A traceback keeps the frames it passes through, and an error reporter that records a frame's locals records them by their repr. So in this module a raw credential is a local of no frame - except the one that reads it: `resolve_credential_reference`, until it - returns. Every frame that carries a credential to the provider carries - this wrapper instead.""" + except the ones that read it, until they return: + `resolve_credential_reference`, and `subprocess_broker_runner`, `mint` + and `_minted_token` reading a broker's answer. Every frame that carries a + credential to the provider carries this wrapper instead, or a + `MintedToken`.""" __slots__ = ("_value",) diff --git a/tests/test_model_provider_broker.py b/tests/test_model_provider_broker.py index ec98a35..84c12f8 100644 --- a/tests/test_model_provider_broker.py +++ b/tests/test_model_provider_broker.py @@ -3117,3 +3117,119 @@ def refuses_a_second_mint(argv, **kwargs): assert refusal.diagnostic == provider_mod.DIAG_BROKER_REFUSED assert len(opener.requests) == 1, "no paid retry without a token" assert refusal.__context__ is None + + +@pytest.mark.parametrize("token", [ + f"{SENTINEL_TOKEN}€", f"{SENTINEL_TOKEN}é", + "mint-stand-in NOT-A-TOKEN", "mint-stand-in\tNOT-A-TOKEN", + f"{SENTINEL_TOKEN}\n", f"{SENTINEL_TOKEN}\x00", f"{SENTINEL_TOKEN}\x7f", + f"{SENTINEL_TOKEN} ", +], ids=["outside-latin-1", "latin-1-not-ascii", "embedded-space", "tab", + "line-break", "nul", "delete", "trailing-space"]) +def test_an_unpresentable_token_is_refused_before_any_request(tmp_path, + token): + """Gap 4. A bearer credential is printable ASCII with no whitespace, and + the minted token is held to the test the built-in resolver's value + meets. At T080's head each of these reached the opener as it was. A + refused answer is no mint: nothing is held, and nothing is recorded.""" + port, opener = _port(tmp_path, _chat_completion(), dialect=OPENAI_CHAT, + token=token) + refusal = _refused_turn(port) + assert refusal.diagnostic == provider_mod.DIAG_BROKER_MALFORMED + assert opener.requests == [], "no provider was contacted" + assert port.catalog().entries[0].available is False + assert port.ledger == [] + + +def test_a_token_outside_latin_1_is_refused_before_any_header_is_built( + tmp_path, monkeypatch): + """Gap 4, over a real socket, because the failure was `urllib`'s. At + T080's head such a token failed while the header was encoded, and the + turn read `DIAG_PROVIDER_UNREACHABLE`, which names the wrong party. It + is now the broker's answer that is refused, nothing is chained, and no + request is sent.""" + monkeypatch.setattr(_ChatCompletionsHandler, "seen", {}) + with _stand_in_provider(_ChatCompletionsHandler) as base: + refusal = _refused_turn(_minting_port( + tmp_path, f"{base}/v1/chat/completions", + token=f"{SENTINEL_TOKEN}€")) + assert refusal.diagnostic == provider_mod.DIAG_BROKER_MALFORMED + assert refusal.__cause__ is None + assert refusal.__context__ is None + assert _ChatCompletionsHandler.seen == {}, "no request reached the server" + + +def test_a_token_of_printable_ascii_is_presented_as_it_is(tmp_path): + """The control for gap 4: the check refuses what a bearer credential + cannot be and nothing more, so every printable ASCII character but the + space is presented unchanged.""" + token = "mint-" + "".join(chr(code) for code in range(0x21, 0x7F)) + port, opener = _port(tmp_path, _chat_completion("a"), dialect=OPENAI_CHAT, + token=token) + assert port.dispatch(_Envelope())["assistant_prose"] == "a" + assert opener.requests[0].get_header("Authorization") == f"Bearer {token}" + + +def test_an_unpresentable_token_leaves_no_frame_that_holds_it(tmp_path): + """A token refused as unpresentable can still be most of a token, such + as one with a line break after it. The refusal keeps no frame that holds + it, as the built-in resolver's refusal of an unpresentable value keeps + none.""" + port, _opener = _port(tmp_path, _chat_completion(), dialect=OPENAI_CHAT, + token=f"{SENTINEL_TOKEN}\n") + refusal = _refused_turn(port) + assert refusal.diagnostic == provider_mod.DIAG_BROKER_MALFORMED + assert _locals_holding(refusal, SENTINEL_TOKEN) == [] + + +def _mint_answer(**changes) -> dict: + """A mint answer in the declared shape, carrying `SENTINEL_TOKEN`, with + `changes` applied.""" + answer = { + "schema_version": 1, "kind": "openprofiler_broker_mint", + "reference": FAKE_REFERENCE, "binding": "openprofiler-demo", + "provider": "demo-provider", "auth_kind": "api_key", + "token": SENTINEL_TOKEN, "token_type": "api_key", + "issued_at": "2026-08-26T14:07:52Z", + "expires_at": _iso(time.time() + 300), "expires_in_seconds": 300, + "scope": [], "issued_by": "openprofiler-broker/0.1.4-fake", + "approved_by": "brett@opensoft.one", + "audit_ref": "opaud-" + "0" * 24, "retry_of": None, + "enforcement": {"expiry": "broker_bookkeeping", "scope": "declared"}} + answer.update(changes) + return answer + + +def _broker_answering(tmp_path, text: str) -> Path: + """A broker that answers every operation with `text`, verbatim.""" + script = tmp_path / "answering-broker.py" + script.write_text(f"import sys\nsys.stdout.write({text!r})\n", + encoding="utf-8") + return script + + +def test_the_declared_mint_answer_mints(tmp_path): + """The control for the case below: this answer, unchanged, mints.""" + script = _broker_answering(tmp_path, json.dumps(_mint_answer())) + assert provider_mod.mint(_broker_binding(script)).token == SENTINEL_TOKEN + + +@pytest.mark.parametrize("text", [ + json.dumps(_mint_answer(expires_at="not-an-instant")), + json.dumps(_mint_answer(debug_note="a key the declaration does not name")), + json.dumps(_mint_answer())[:-1], +], ids=["expiry-malformed", "undeclared-key", "not-json"]) +def test_a_malformed_mint_answer_keeps_no_frame_that_holds_its_token( + tmp_path, text): + """The same rule for every refusal of the answer that carried the token. + At T080's head the answer stayed in the refusal's frames (measured: + `mint.answer`, `mint.document`, `_answer_document.text` and + `_answer_document.document`). Its refusal keeps no frame, cause or + context that holds the token.""" + binding = _broker_binding(_broker_answering(tmp_path, text)) + with pytest.raises(provider_mod.BrokerRefused) as caught: + provider_mod.mint(binding) + assert caught.value.diagnostic == provider_mod.DIAG_BROKER_MALFORMED + assert caught.value.__cause__ is None + assert caught.value.__context__ is None + assert _locals_holding(caught.value, SENTINEL_TOKEN) == [] From af457e6659ef8377a353a08aaa23119db57b36c5 Mon Sep 17 00:00:00 2001 From: Brett Heap <1513478+brettheap@users.noreply.github.com> Date: Tue, 29 Sep 2026 12:00:54 +0000 Subject: [PATCH 04/11] Broker path tests: two escapes and three docstrings The two unpresentable-token literals outside ASCII are written as backslash-u escapes, as the rest of this module writes them, so the source adds no non-ASCII character. The gap-1 control and the operator door's case each gain a line saying what they hold. The `none` case says how T080 once pinned its scope there (two routes, with a broker binding). No test changes what it runs. Arc: neutral-product-standalone-operability Lane: openxfactory-4 (openXfactory-4-openDox_extraction) Co-Authored-By: Claude Opus 5.5 (1M context) --- tests/test_model_provider_broker.py | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/tests/test_model_provider_broker.py b/tests/test_model_provider_broker.py index 84c12f8..0213fbd 100644 --- a/tests/test_model_provider_broker.py +++ b/tests/test_model_provider_broker.py @@ -2318,9 +2318,9 @@ def test_a_broker_reference_in_a_built_in_form_is_malformed(tmp_path): @NOT_ON_A_PRIVATE_ROUTE def test_a_none_binding_keeps_a_route_that_is_not_private(endpoint): """The auth kind `none` presents no credential, so it is the one kind - that keeps such a route. Until the broker path's hardening (the last - section of this file), this case also declared a broker binding on these - routes, pinning T080's scope. That half is now refused.""" + that keeps such a route. Before the broker path's hardening (the last + section of this file), T080 pinned its scope here by declaring a broker + binding on two such routes as well. That half is now refused.""" assert _none_binding(endpoint=endpoint).credential_source() == ( binding_mod.NO_CREDENTIAL) @@ -2957,6 +2957,7 @@ def test_set_credential_refuses_a_binding_no_broker_answers(tmp_path, capsys, @ON_A_PRIVATE_ROUTE def test_a_broker_token_is_declared_on_a_private_route(endpoint): + """The control for gap 1: every route a built-in credential may take.""" binding = _binding(endpoint=endpoint, dialect=OPENAI_CHAT) assert binding.endpoint == endpoint assert binding.credential_source() == binding_mod.CREDENTIAL_FROM_BROKER @@ -2993,6 +2994,7 @@ def test_mint_asks_no_broker_for_a_token_on_a_route_that_is_not_private( def test_the_cli_refuses_a_broker_binding_over_http_to_another_host( tmp_path, capsys): + """Gap 1, through the operator door: refused, and nothing is stored.""" checkout = tmp_path / "checkout" checkout.mkdir() args = cli_mod.build_parser().parse_args([ @@ -3120,7 +3122,7 @@ def refuses_a_second_mint(argv, **kwargs): @pytest.mark.parametrize("token", [ - f"{SENTINEL_TOKEN}€", f"{SENTINEL_TOKEN}é", + f"{SENTINEL_TOKEN}\u20ac", f"{SENTINEL_TOKEN}\u00e9", "mint-stand-in NOT-A-TOKEN", "mint-stand-in\tNOT-A-TOKEN", f"{SENTINEL_TOKEN}\n", f"{SENTINEL_TOKEN}\x00", f"{SENTINEL_TOKEN}\x7f", f"{SENTINEL_TOKEN} ", @@ -3152,7 +3154,7 @@ def test_a_token_outside_latin_1_is_refused_before_any_header_is_built( with _stand_in_provider(_ChatCompletionsHandler) as base: refusal = _refused_turn(_minting_port( tmp_path, f"{base}/v1/chat/completions", - token=f"{SENTINEL_TOKEN}€")) + token=f"{SENTINEL_TOKEN}\u20ac")) assert refusal.diagnostic == provider_mod.DIAG_BROKER_MALFORMED assert refusal.__cause__ is None assert refusal.__context__ is None From a2c838a09ced781814a8672737775f8c54461d48 Mon Sep 17 00:00:00 2001 From: Brett Heap <1513478+brettheap@users.noreply.github.com> Date: Tue, 29 Sep 2026 12:16:40 +0000 Subject: [PATCH 05/11] Broker path tests: the redirect case says which codes urllib followed Copilot's review of openDox-code#64 at af457e66 (thread 4133295457): the docstring's "301, 302 or 303" wrapped so that it read as repeating 303, and "a 307 or 308 read as the provider refusing" was hard to parse. It now says that at T080's head urllib followed only the 301, 302 and 303 codes, with a GET that still carried the token, and refused a 307 or a 308 with DIAG_PROVIDER_REFUSED, as the base probe measured. No test changes what it runs. Arc: neutral-product-standalone-operability Lane: openxfactory-4 (openXfactory-4-openDox_extraction) Co-Authored-By: Claude Opus 5.5 (1M context) --- tests/test_model_provider_broker.py | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/tests/test_model_provider_broker.py b/tests/test_model_provider_broker.py index 0213fbd..8b2df2a 100644 --- a/tests/test_model_provider_broker.py +++ b/tests/test_model_provider_broker.py @@ -3031,11 +3031,15 @@ def _refused_turn(port) -> provider_mod.BrokerRefused: @pytest.mark.parametrize("code", [301, 302, 303, 307, 308]) def test_a_broker_token_follows_no_redirect(tmp_path, monkeypatch, code): - """Gap 2, over real sockets. At T080's head a POST answered 301, 302 or - 303 reached the redirect's target as a GET that still carried the minted - token, and the turn was answered from there. A 307 or 308 read as the - provider refusing. The redirect is declined, the second server hears - nothing, and the refusal chains nothing.""" + """Gap 2, over real sockets, for each redirect code. + + At T080's head, urllib followed only the 301, 302 and 303 codes. For + those it sent a GET that still carried the minted token to the + redirect's target, and the turn was answered from there. It did not + follow a 307 or a 308, and the turn refused with + `DIAG_PROVIDER_REFUSED`, as though the provider had refused. Now every + redirect is declined, the second server hears nothing, and the refusal + chains nothing.""" with _a_provider_that_redirects(monkeypatch, code) as base: refusal = _refused_turn( _minting_port(tmp_path, f"{base}/v1/chat/completions")) From 788d764b569208c8e92b5235498a462e3b7f6b27 Mon Sep 17 00:00:00 2001 From: Brett Heap <1513478+brettheap@users.noreply.github.com> Date: Tue, 29 Sep 2026 12:26:05 +0000 Subject: [PATCH 06/11] Broker path: a mint answer whose expiry is no finite number is malformed Copilot's review of openDox-code#64 at a2c838a0 (thread 4133360345): `_parse_expires_at` let an integer past a float's range escape as an OverflowError, and took NaN and the infinities as an expiry. `mint` re-raises only a BrokerRefused afresh, so the OverflowError escaped with the answer still in its frames. That defeats this PR's rule that no refusal of a mint keeps the answer. A NaN or infinite expiry also minted a token that would never expire, or would always have expired. An expiry must now be a finite number, or an ISO-8601 instant as before. Anything else is a malformed answer (DIAG_BROKER_MALFORMED), which `mint` raises again holding nothing. The same measurement found one more escape of the same class: arrays nested past the recursion limit fit inside the 64 KiB answer bound (30,000 of them in 60 KB), and json.loads' RecursionError escaped `_answer_document`. That is now malformed too, for all four broker operations. Tests: 5 cases join the malformed-answer case (an expiry past a float, NaN, Infinity, -Infinity, and an answer nested past the limit), and each answer is held inside the runner's bound. With 3f14bb96 as the source all 8 of that case's params fail. Three mutants are killed: the finite check removed (4 fail), the overflow left to float() (1), and RecursionError not caught (1). Arc: neutral-product-standalone-operability Lane: openxfactory-4 (openXfactory-4-openDox_extraction) Co-Authored-By: Claude Opus 5.5 (1M context) --- src/opendox/doxbench_provider.py | 25 ++++++++++++++++++++++--- tests/test_model_provider_broker.py | 28 +++++++++++++++++++++++++--- 2 files changed, 47 insertions(+), 6 deletions(-) diff --git a/src/opendox/doxbench_provider.py b/src/opendox/doxbench_provider.py index d9c3f10..475dc8a 100644 --- a/src/opendox/doxbench_provider.py +++ b/src/opendox/doxbench_provider.py @@ -99,6 +99,7 @@ import dataclasses import json +import math import os import shutil import subprocess @@ -359,11 +360,26 @@ def _parse_expires_at(value: object) -> float: Accepts an ISO-8601 instant (the spelling `credential-contracts` uses for its own `expires_at`) or a plain number of epoch seconds. A naive instant is read as UTC — the alternative, reading it in the console host's local zone, - would make a token's life depend on where the operator lives.""" + would make a token's life depend on where the operator lives. + + A NUMBER MUST BE FINITE (Copilot's review of openDox-code#64 at + `a2c838a0`). JSON can carry an integer too large for a float, which + `float()` refuses with an `OverflowError`. Python's JSON reader also takes + `NaN`, `Infinity` and `-Infinity`, and none of them is an instant: the + token would never expire, or would always have expired. Each is a + malformed answer, refused with the fixed sentence, so no token whose + expiry cannot be read is held, and `mint` raises the refusal again, + holding nothing of the answer.""" if isinstance(value, bool): raise BrokerRefused(DIAG_BROKER_MALFORMED) if isinstance(value, (int, float)): - return float(value) + try: + seconds = float(value) + except OverflowError: + seconds = math.inf + if not math.isfinite(seconds): + raise BrokerRefused(DIAG_BROKER_MALFORMED) + return seconds if not isinstance(value, str) or not value.strip(): raise BrokerRefused(DIAG_BROKER_MALFORMED) text = value.strip() @@ -546,7 +562,10 @@ def _answer_document(text: object, kind: str, fields) -> dict: raise BrokerRefused(DIAG_BROKER_MALFORMED) try: document = json.loads(text) - except (ValueError, TypeError) as error: + # A RecursionError too: arrays nested past the interpreter's limit fit + # well inside MAX_BROKER_ANSWER_BYTES (measured: 30,000 of them in 60 KB), + # and such an answer is malformed, not a crash that escapes with it. + except (ValueError, TypeError, RecursionError) as error: raise BrokerRefused(DIAG_BROKER_MALFORMED) from error if not isinstance(document, dict): raise BrokerRefused(DIAG_BROKER_MALFORMED) diff --git a/tests/test_model_provider_broker.py b/tests/test_model_provider_broker.py index 8b2df2a..c9917e4 100644 --- a/tests/test_model_provider_broker.py +++ b/tests/test_model_provider_broker.py @@ -3220,18 +3220,40 @@ def test_the_declared_mint_answer_mints(tmp_path): assert provider_mod.mint(_broker_binding(script)).token == SENTINEL_TOKEN +#: A mint answer nested past the interpreter's recursion limit, and still well +#: inside the broker's answer bound. +_NESTED_PAST_THE_LIMIT = (json.dumps(_mint_answer())[:-1] + ', "deep": ' + + "[" * 30_000 + "]" * 30_000 + "}") + + @pytest.mark.parametrize("text", [ json.dumps(_mint_answer(expires_at="not-an-instant")), json.dumps(_mint_answer(debug_note="a key the declaration does not name")), json.dumps(_mint_answer())[:-1], -], ids=["expiry-malformed", "undeclared-key", "not-json"]) + json.dumps(_mint_answer(expires_at=10 ** 400)), + json.dumps(_mint_answer(expires_at=float("nan"))), + json.dumps(_mint_answer(expires_at=float("inf"))), + json.dumps(_mint_answer(expires_at=float("-inf"))), + _NESTED_PAST_THE_LIMIT, +], ids=["expiry-malformed", "undeclared-key", "not-json", + "expiry-past-a-float", "expiry-nan", "expiry-infinite", + "expiry-minus-infinite", "nested-past-the-recursion-limit"]) def test_a_malformed_mint_answer_keeps_no_frame_that_holds_its_token( tmp_path, text): """The same rule for every refusal of the answer that carried the token. At T080's head the answer stayed in the refusal's frames (measured: `mint.answer`, `mint.document`, `_answer_document.text` and - `_answer_document.document`). Its refusal keeps no frame, cause or - context that holds the token.""" + `_answer_document.document`). + + Some answers escaped `mint` outright (Copilot's review of + openDox-code#64 at `a2c838a0`): an expiry past a float's range, as an + `OverflowError`, and an answer nested past the recursion limit, as a + `RecursionError`. An expiry of `NaN` or an infinity minted a token that + would never expire, or would always have expired. Each is now a + malformed answer, and its refusal keeps no frame, cause or context that + holds the token.""" + assert len(text.encode("utf-8")) <= provider_mod.MAX_BROKER_ANSWER_BYTES, \ + "a case for the answer's parser, not for the runner's bound" binding = _broker_binding(_broker_answering(tmp_path, text)) with pytest.raises(provider_mod.BrokerRefused) as caught: provider_mod.mint(binding) From 25788f91d3f4fddb3578558df196c04f09b88fbc Mon Sep 17 00:00:00 2001 From: Brett Heap <1513478+brettheap@users.noreply.github.com> Date: Wed, 30 Sep 2026 00:19:41 +0000 Subject: [PATCH 07/11] Broker runner: a misbehaving broker's refusal keeps nothing it wrote Brett Heap's word of 2026-09-29 (openxFactory#656, comment 5901112350): "Yes, add to #64". When a broker misbehaves (it exits non-zero, answers past the size bound, or times out after writing), the shared runner's refusal carries no broker output, no cause and no context, for all four operations. The refusal names the operation and the failure class, and never the bytes. What was measured at 788d764b, with a broker that wrote a fake token first: - exit non-zero, or past the bound: the token stayed in the runner's frame (answer, and the child's _fileobj2output). Past the bound read as MALFORMED. - timeout: the refusal chained the TimeoutExpired, whose output and whose frames inside subprocess held the token. - an answer that is not UTF-8: a UnicodeDecodeError escaped holding the token, and no refusal was raised at all. - no refusal named its operation, and two of the sentences said "so no token could be minted" for intake, revoke and list too. The runner's work moves to _run_broker, which returns an answer or a sentence and raises no refusal of its own. subprocess_broker_runner raises the refusal after that call returns, so it is outside every handler and in a frame that never held the child. A decode error is caught, and so is a decode error while a refused child is reaped. The four operations now ask through one wrapper, _broker_operation. It raises each refusal again, afresh, with the operation named, so an injected runner is covered too. BrokerRefused takes operation=, but only from OPERATIONS and only beside a broker's sentence (BROKER_DIAGNOSTICS). Its message reads "broker : ", and .diagnostic is unchanged. The broker's sentences no longer name an operation. An answer past the bound has its own sentence, DIAG_BROKER_OVERSIZE, so there are twelve fixed diagnostics. Tests: 36 new cases. They cover the shared runner for five misbehaviours, each of the four operations for each misbehaviour, a broker that cannot be started, an injected runner, the operation vocabulary, and the operator's set-credential door. With 788d764b as the source, 37 cases fail: the 36 new ones except the one guard, plus the two tests that were updated. 14 new mutants are killed, and all 16 earlier ones are still killed. Arc: neutral-product-standalone-operability Lane: openxfactory-4 (openXfactory-4-openDox_extraction) Co-Authored-By: Claude Opus 5.5 (1M context) --- src/opendox/doxbench_provider.py | 234 +++++++++++++++++++----- tests/test_model_provider_broker.py | 268 ++++++++++++++++++++++++++-- 2 files changed, 442 insertions(+), 60 deletions(-) diff --git a/src/opendox/doxbench_provider.py b/src/opendox/doxbench_provider.py index 475dc8a..15f1f7f 100644 --- a/src/opendox/doxbench_provider.py +++ b/src/opendox/doxbench_provider.py @@ -70,7 +70,10 @@ this module raises carries one of the FIXED sentences below, composed from nothing the broker or the provider said, so `doxbench_model.dispatch_turn` maps it onto the same redacted `model_failed` every other adapter failure - already maps onto. + already maps onto. A broker's refusal keeps nothing the broker wrote: no + cause, no context, and no frame that holds its answer (Brett Heap's word + of 2026-09-29). It names the operation that failed, one of the declared + four, and the failure class. THE PROGRAM IS DECLARED; THE VERBS ARE THE DECLARATION'S (task 2.6). This was written while openProfiler was unbuilt, so it named the operation in a JSON @@ -224,14 +227,32 @@ #: the provider said. A broker's stderr, a provider's error body and an #: exception's text are all dropped unread at the boundary that observes them, #: exactly as `dispatch_turn` drops a provider exception's text. +#: +#: THE BROKER'S SENTENCES NAME A FAILURE CLASS AND NO OPERATION. They are +#: shared by all four operations, and a refusal raised by one of them names +#: that operation beside the sentence (`BrokerRefused.operation`). At +#: `788d764b` two of them said "so no token could be minted" whichever +#: operation had failed, and a broker that answered past the bound was refused +#: as MALFORMED, beside every answer of the wrong shape. DIAG_BROKER_UNREACHABLE = ( - "the credential broker could not be started, so no token could be minted") + "the credential broker could not be started") DIAG_BROKER_REFUSED = ( - "the credential broker refused, so no token could be minted") + "the credential broker exited non-zero, and its answer is withheld by " + "design") DIAG_BROKER_MALFORMED = ( - "the credential broker's answer did not match the declared mint contract") + "the credential broker's answer did not match the operation's declared " + "contract, and it is withheld by design") DIAG_BROKER_TIMEOUT = ( - "the credential broker did not answer within the declared timeout") + "the credential broker did not answer within the declared timeout, and " + "anything it wrote is withheld by design") +#: `MAX_BROKER_ANSWER_BYTES` exceeded. The provider's bound stays on +#: `DIAG_PROVIDER_MALFORMED` (see `MAX_PROVIDER_ANSWER_BYTES`). The broker's +#: has its own sentence: Brett Heap's word of 2026-09-29 names three ways a +#: broker misbehaves, this is one of them, and a refusal names its class. It +#: says only that the bound was passed, never by how much. +DIAG_BROKER_OVERSIZE = ( + "the credential broker's answer was larger than the declared bound, and " + "it is withheld by design") DIAG_PROVIDER_UNREACHABLE = ( "the provider could not be reached and its details are withheld by design") DIAG_PROVIDER_REFUSED = ( @@ -262,9 +283,10 @@ "declare the endpoint the provider redirects to") #: The closed set, so a test can assert no other sentence can be raised. -#: ELEVEN: the eight the reconciliation left, the built-in resolver's two -#: (#1144 box 16.3), and the redirect a request carrying a credential -#: declines. `DIAG_DIALECT_UNKNOWN` is gone because the fact it guarded +#: TWELVE: the eight the reconciliation left, the built-in resolver's two +#: (#1144 box 16.3), the redirect a request carrying a credential declines, +#: and a broker answer past the bound (2026-09-29). +#: `DIAG_DIALECT_UNKNOWN` is gone because the fact it guarded #: moved: the dialect is the BINDING's, validated against the closed vocabulary #: when the operator declares it #: (`doxbench_binding.ModelProviderBinding.__post_init__`), so an unknown @@ -276,7 +298,14 @@ DIAG_BROKER_TIMEOUT, DIAG_PROVIDER_UNREACHABLE, DIAG_PROVIDER_REFUSED, DIAG_PROVIDER_MALFORMED, DIAG_TOKEN_EXPIRED_TWICE, DIAG_REFERENCE_UNRESOLVED, DIAG_KEYRING_UNAVAILABLE, - DIAG_PROVIDER_REDIRECTED, + DIAG_PROVIDER_REDIRECTED, DIAG_BROKER_OVERSIZE, +}) + +#: The sentences a BROKER's failure is stated in, the only ones a refusal may +#: name an operation beside. +BROKER_DIAGNOSTICS: frozenset[str] = frozenset({ + DIAG_BROKER_UNREACHABLE, DIAG_BROKER_REFUSED, DIAG_BROKER_MALFORMED, + DIAG_BROKER_TIMEOUT, DIAG_BROKER_OVERSIZE, }) @@ -286,16 +315,33 @@ class BrokerRefused(RuntimeError): Deliberately carries no payload, no status code, no stderr and no response body: there is no attribute a caller could log that discloses provider or - broker detail, which is the same discipline `TurnDispatchFailure` keeps.""" - - def __init__(self, diagnostic: str) -> None: + broker detail, which is the same discipline `TurnDispatchFailure` keeps. + + A BROKER'S REFUSAL NAMES ITS OPERATION, so that a refusal holding + nothing a broker wrote (Brett Heap's word of 2026-09-29, openxFactory#656) + still says what failed. `operation` is one of `OPERATIONS`, given only + beside a broker's sentence (`BROKER_DIAGNOSTICS`), and the message reads + "broker : ". Both halves come from this module's + closed vocabularies, so the message still holds nothing a broker wrote. + `diagnostic` stays the sentence alone, and `operation` is None for a + provider's refusal and for one the runner raises by itself.""" + + def __init__(self, diagnostic: str, *, + operation: str | None = None) -> None: if diagnostic not in FIXED_DIAGNOSTICS: raise AssertionError( "a broker refusal carries a FIXED diagnostic; composing one " "from what the broker or the provider said is exactly what " "this class exists to prevent") - super().__init__(diagnostic) + if operation is not None and (operation not in OPERATIONS + or diagnostic not in BROKER_DIAGNOSTICS): + raise AssertionError( + "a refusal names an operation only beside a broker's " + "sentence, and only one of the declared four") + super().__init__(diagnostic if operation is None + else f"broker {operation}: {diagnostic}") self.diagnostic = diagnostic + self.operation = operation class _TokenExpired(Exception): @@ -439,7 +485,44 @@ def subprocess_broker_runner(argv, *, source=None, broker's own words must never reach a caller, inheriting this process's stderr would put them on the console, and capturing them into a pipe would make this process's memory a function of how noisy a declared program - chooses to be. The kernel drops them instead, unread by construction.""" + chooses to be. The kernel drops them instead, unread by construction. + + A BROKER THAT MISBEHAVES IS REFUSED WITH NOTHING IT WROTE (Brett Heap's + word of 2026-09-29, openxFactory#656), for all four operations. That is a + broker that exits non-zero, answers past `MAX_BROKER_ANSWER_BYTES`, times + out, or writes an answer that is not UTF-8. Each refusal is raised here, + after `_run_broker` has returned: outside every handler, so it keeps no + cause and no context, and from the one frame that never held the child or + its answer. Measured at #64's `788d764b`, a broker that wrote a token and + then exited non-zero, or wrote past the bound, left it in this frame's + `answer` and in the child's buffers. One that wrote it and then timed out + chained the `TimeoutExpired` that holds it. One that wrote it beside a + byte that is not UTF-8 escaped as a `UnicodeDecodeError` that holds it, + and was no refusal at all.""" + answer, failure = _run_broker(argv, source=source, timeout=timeout) + if failure is not None: + raise BrokerRefused(failure) + return answer + + +def _reap(child) -> None: + """Kill a child this runner is refusing and finish its communication, + dropping whatever it wrote. The drop includes an answer that is not + UTF-8, whose `UnicodeDecodeError` would otherwise escape from the + handler that called this, holding that answer.""" + child.kill() + try: + child.communicate() + except UnicodeDecodeError: + pass + + +def _run_broker(argv, *, source, + timeout: float) -> tuple[str | None, str | None]: + """The work of `subprocess_broker_runner`: `(answer, None)`, or + `(None, sentence)` for a refusal. It raises no refusal itself, so no + refusal keeps its frame, which holds the child and what the child + wrote.""" try: child = subprocess.Popen( # noqa: S603 - argv from a declared binding plus the declared subcommand, never a shell string list(argv), @@ -449,8 +532,8 @@ def subprocess_broker_runner(argv, *, source=None, env=bridge_mod.child_environment(os.environ), text=True, ) - except (OSError, ValueError) as error: - raise BrokerRefused(DIAG_BROKER_UNREACHABLE) from error + except (OSError, ValueError): + return None, DIAG_BROKER_UNREACHABLE try: try: if source is not None: @@ -475,20 +558,26 @@ def subprocess_broker_runner(argv, *, source=None, # it is also the last place in this process that could have held the # pipe the credential travelled down. child.stdin = None - answer, _dropped_stderr = child.communicate(timeout=timeout) - except subprocess.TimeoutExpired as error: - child.kill() - child.communicate() - raise BrokerRefused(DIAG_BROKER_TIMEOUT) from error - except OSError as error: - child.kill() - child.communicate() - raise BrokerRefused(DIAG_BROKER_UNREACHABLE) from error + try: + output, _dropped_stderr = child.communicate(timeout=timeout) + except UnicodeDecodeError: + # An answer that is not UTF-8. `communicate` decodes only after + # it has waited for the child, so the exit code is read below + # as for any other answer. + output = None + except subprocess.TimeoutExpired: + _reap(child) + return None, DIAG_BROKER_TIMEOUT + except OSError: + _reap(child) + return None, DIAG_BROKER_UNREACHABLE if child.returncode != 0: - raise BrokerRefused(DIAG_BROKER_REFUSED) - if len(answer.encode("utf-8")) > MAX_BROKER_ANSWER_BYTES: - raise BrokerRefused(DIAG_BROKER_MALFORMED) - return answer + return None, DIAG_BROKER_REFUSED + if output is None: + return None, DIAG_BROKER_MALFORMED + if len(output.encode("utf-8")) > MAX_BROKER_ANSWER_BYTES: + return None, DIAG_BROKER_OVERSIZE + return output, None def broker_operation_argv(binding, operation: str, *, @@ -585,6 +674,37 @@ def _declared_string(document: Mapping, field: str) -> str: return value +def _broker_operation(binding, operation: str, read, *, runner, + source=None, retry_of: str | None = None): + """Run one declared `operation` through `runner`, and return what `read` + makes of its answer. It is the one way each of the four operations asks + the broker. + + EVERY REFUSAL KEEPS NOTHING THE BROKER WROTE (Brett Heap's word of + 2026-09-29, openxFactory#656), AND NAMES THE OPERATION. A refusal from the + runner, or from `read`, is raised again here with the operation named + (`BrokerRefused.operation`). It is raised afresh, outside the handler and + after the answer has left this frame, so it keeps no cause, no context + and no frame that holds the answer, whichever runner was injected. The + answer is dropped even when it carries no secret, since a broker that + misbehaves may write anything into it. + + `source` is given to the runner only when there is one, which is + `intake`'s case. Every other operation reads no standard input.""" + argv = broker_operation_argv(binding, operation, retry_of=retry_of) + answer = None + try: + if source is None: + answer = runner(argv) + else: + answer = runner(argv, source=source) + return read(answer) + except BrokerRefused as refusal: + failure = refusal.diagnostic + del answer + raise BrokerRefused(failure, operation=operation) + + # --------------------------------------------------------------------------- # the credential hand-off (task 1.3) # --------------------------------------------------------------------------- @@ -617,9 +737,16 @@ def hand_off_credential(binding, source, *, beside the broker that holds the credential, which is two resolvers. The record refuses that, and it would do so where neither entry point expects a refusal. So such an answer is malformed, and it is refused - here, where both entry points already catch a broker's refusal.""" - answer = runner(broker_operation_argv(binding, OPERATION_INTAKE), - source=source) + here, where both entry points already catch a broker's refusal. + + A refusal names `intake` and keeps nothing the broker wrote + (`_broker_operation`).""" + return _broker_operation(binding, OPERATION_INTAKE, _intake_reference, + runner=runner, source=source) + + +def _intake_reference(answer: object) -> str: + """An intake answer, read EXACTLY, as the `reference` it returns.""" document = _answer_document(answer, BROKER_INTAKE_KIND, INTAKE_FIELDS) reference = _declared_string(document, "reference") if binding_mod.names_a_built_in_form(reference): @@ -667,22 +794,19 @@ def mint(binding, *, retry_of: str | None = None, character was sent as it was. NO REFUSAL OF THE ANSWER KEEPS IT. The answer carries the token, so every - refusal raised once it is read is raised again here, afresh, after the - answer has left this frame. It keeps no frame, cause or context that - holds the token, as the built-in resolver lets go of what it read.""" + refusal is raised again, afresh, after the answer has left the frame that + read it (`_broker_operation`), and it names `mint`. It keeps no frame, + cause or context that holds the token, as the built-in resolver lets go + of what it read.""" if not binding_mod.is_a_private_route(binding.endpoint): raise AssertionError( f"binding {binding.id!r} would present a minted token over a " "route that is not private, which the record refuses when it is " "declared; nothing was minted") - answer = runner(broker_operation_argv(binding, OPERATION_MINT, - retry_of=retry_of)) - try: - return _minted_token(answer, binding) - except BrokerRefused as refusal: - failure = refusal.diagnostic - del answer - raise BrokerRefused(failure) + return _broker_operation( + binding, OPERATION_MINT, + lambda answer: _minted_token(answer, binding), + runner=runner, retry_of=retry_of) def _minted_token(answer: object, binding) -> MintedToken: @@ -691,8 +815,8 @@ def _minted_token(answer: object, binding) -> MintedToken: Every refusal is `DIAG_BROKER_MALFORMED`: an answer of another shape, a token that cannot be presented as it is (`_presentable`, which also refuses one that is not a string or is blank), or an expiry or an audit - reference the declaration does not allow. `mint` raises each one again, - holding nothing of the answer.""" + reference the declaration does not allow. `_broker_operation` raises each + one again, holding nothing of the answer.""" document = _answer_document(answer, BROKER_MINT_KIND, MINT_FIELDS) token = document["token"] if not _presentable(token): @@ -712,8 +836,14 @@ def revoke(binding, *, runner=subprocess_broker_runner) -> str: trail through a revocation and refuses an unknown reference rather than answering silently, so "there was nothing there" and "it is gone now" stay different answers — and both reach a caller here as the same fixed refusal - or the same returned reference, never as the broker's own words.""" - answer = runner(broker_operation_argv(binding, OPERATION_REVOKE)) + or the same returned reference, never as the broker's own words. A + refusal names `revoke` (`_broker_operation`).""" + return _broker_operation(binding, OPERATION_REVOKE, _revocation_audit_ref, + runner=runner) + + +def _revocation_audit_ref(answer: object) -> str: + """A revocation answer, read EXACTLY, as its `audit_ref`.""" document = _answer_document(answer, BROKER_REVOCATION_KIND, REVOCATION_FIELDS) if document["revoked"] is not True: @@ -727,8 +857,14 @@ def list_references(binding, *, runner=subprocess_broker_runner) -> list: Safe to read and safe to print: `list` never opens a custody file, and the index it reads carries no credential material. Returned as the declaration's own list of entries rather than reshaped, because a consumer that reshapes - an index it does not own invents a second contract for it.""" - answer = runner(broker_operation_argv(binding, OPERATION_LIST)) + an index it does not own invents a second contract for it. A refusal + names `list` (`_broker_operation`).""" + return _broker_operation(binding, OPERATION_LIST, _reference_index, + runner=runner) + + +def _reference_index(answer: object) -> list: + """A reference-index answer, read EXACTLY, as its list of entries.""" document = _answer_document(answer, BROKER_REFERENCE_LIST_KIND, REFERENCE_LIST_FIELDS) references = document["references"] diff --git a/tests/test_model_provider_broker.py b/tests/test_model_provider_broker.py index c9917e4..7641560 100644 --- a/tests/test_model_provider_broker.py +++ b/tests/test_model_provider_broker.py @@ -50,6 +50,7 @@ import contextlib import dataclasses +import functools import http.server import io import json @@ -1141,12 +1142,19 @@ def test_the_fixed_diagnostics_are_all_reachable_and_no_more(): declaration-time refusal; keeping an unraisable sentence would be a refusal nobody can trigger. ELEVEN since #1144 box 16.3: the built-in resolver's two joined, and so did the redirect a built-in credential declines. - Section (f) below reaches each of the three.""" - assert len(provider_mod.FIXED_DIAGNOSTICS) == 11 + Section (f) below reaches each of the three. TWELVE since Brett Heap's + word of 2026-09-29: a broker answer past the bound has its own sentence, + and the last section reaches it.""" + assert len(provider_mod.FIXED_DIAGNOSTICS) == 12 assert {provider_mod.DIAG_REFERENCE_UNRESOLVED, provider_mod.DIAG_KEYRING_UNAVAILABLE, - provider_mod.DIAG_PROVIDER_REDIRECTED} <= \ + provider_mod.DIAG_PROVIDER_REDIRECTED, + provider_mod.DIAG_BROKER_OVERSIZE} <= \ provider_mod.FIXED_DIAGNOSTICS + assert provider_mod.BROKER_DIAGNOSTICS == { + provider_mod.DIAG_BROKER_UNREACHABLE, + provider_mod.DIAG_BROKER_REFUSED, provider_mod.DIAG_BROKER_MALFORMED, + provider_mod.DIAG_BROKER_TIMEOUT, provider_mod.DIAG_BROKER_OVERSIZE} assert not hasattr(provider_mod, "DIAG_DIALECT_UNKNOWN") @@ -1351,14 +1359,19 @@ def test_a_broker_that_hangs_is_refused_at_the_declared_timeout(tmp_path): def test_the_broker_answer_is_bounded(tmp_path): - script = tmp_path / "loud-broker.py" - script.write_text( - "import sys\n" - f"sys.stdout.write('x' * {provider_mod.MAX_BROKER_ANSWER_BYTES + 1})\n", - encoding="utf-8") - with pytest.raises(provider_mod.BrokerRefused) as caught: - provider_mod.mint(_broker_binding(script)) - assert caught.value.diagnostic == provider_mod.DIAG_BROKER_MALFORMED + """One byte past the bound has its own sentence since Brett Heap's word + of 2026-09-29. It was `DIAG_BROKER_MALFORMED`, beside every answer of + the wrong shape. An answer at the bound is read, and refused only as + what it is: here, not JSON.""" + bound = provider_mod.MAX_BROKER_ANSWER_BYTES + for size, expected in ((bound + 1, provider_mod.DIAG_BROKER_OVERSIZE), + (bound, provider_mod.DIAG_BROKER_MALFORMED)): + script = tmp_path / f"loud-broker-{size}.py" + script.write_text(f"import sys\nsys.stdout.write('x' * {size})\n", + encoding="utf-8") + with pytest.raises(provider_mod.BrokerRefused) as caught: + provider_mod.mint(_broker_binding(script)) + assert caught.value.diagnostic == expected def test_the_subprocess_runner_never_uses_a_shell(tmp_path): @@ -3261,3 +3274,236 @@ def test_a_malformed_mint_answer_keeps_no_frame_that_holds_its_token( assert caught.value.__cause__ is None assert caught.value.__context__ is None assert _locals_holding(caught.value, SENTINEL_TOKEN) == [] + + +# --- a broker that misbehaves is refused with nothing it wrote ----------- +# Brett Heap's word of 2026-09-29 (openxFactory#656, the lane's latest RULED +# comment): "Yes, add to #64". When a broker misbehaves, the shared runner's +# refusal carries no broker output, no cause and no context, for all four +# operations. So that it stays useful, it names the operation and the failure +# class, and never the bytes. Measured at #64's `788d764b`, with a broker that +# wrote the token before it misbehaved: +# +# * exits non-zero: the runner's frame held the token, in `answer` and in +# the child's `_fileobj2output`; +# * answers past the bound: the same, and the refusal read as MALFORMED; +# * times out: the refusal chained the `TimeoutExpired`, whose `output` +# and whose frames inside `subprocess` held the token; +# * answers in bytes that are not UTF-8: a `UnicodeDecodeError` escaped +# holding the token in `object`, and no refusal was raised at all. +# +# No refusal named its operation, and two of the sentences said no token +# could be minted, whichever operation had failed. Every case below fails at +# `788d764b`. Every token here is an obvious fake. + +#: Long enough for a Python child to start and write, well before it expires. +_MISBEHAVING_TIMEOUT = 1.0 + +#: What each broker below does once it has written `SENTINEL_TOKEN`, and the +#: failure class its refusal names. The sentence is named here, and read +#: only once the refusal has been checked for what it keeps. +_MISBEHAVIOURS = { + "exits-non-zero": ( + "sys.stdout.write(TOKEN)\nwrote()\nsys.exit(3)\n", + "DIAG_BROKER_REFUSED"), + "answers-past-the-bound": ( + "sys.stdout.write(TOKEN + 'x' * BOUND)\nwrote()\n", + "DIAG_BROKER_OVERSIZE"), + "times-out": ( + "sys.stdout.write(TOKEN)\nwrote()\ntime.sleep(30)\n", + "DIAG_BROKER_TIMEOUT"), + "answers-in-no-utf-8": ( + "sys.stdout.buffer.write(TOKEN.encode() + b'\\xff')\nwrote()\n", + "DIAG_BROKER_MALFORMED"), + "answers-in-no-utf-8-and-times-out": ( + "sys.stdout.buffer.write(TOKEN.encode() + b'\\xff')\nwrote()\n" + "time.sleep(30)\n", + "DIAG_BROKER_TIMEOUT"), +} + +#: The broker's preamble. `wrote()` flushes, then leaves a mark beside the +#: script, so a test can show the token was written before the misbehaviour. +_MISBEHAVING_PREAMBLE = ( + "import pathlib, sys, time\n" + "TOKEN = {token!r}\n" + "BOUND = {bound!r}\n" + "def wrote():\n" + " sys.stdout.flush()\n" + " pathlib.Path(sys.argv[0] + '.wrote').touch()\n") + + +def _misbehaving_broker(tmp_path, misbehaviour: str) -> Path: + body, _sentence = _MISBEHAVIOURS[misbehaviour] + script = tmp_path / f"{misbehaviour}-broker.py" + script.write_text(_MISBEHAVING_PREAMBLE.format( + token=SENTINEL_TOKEN, bound=provider_mod.MAX_BROKER_ANSWER_BYTES) + + body, encoding="utf-8") + return script + + +def _sentence_for(misbehaviour: str) -> str: + return getattr(provider_mod, _MISBEHAVIOURS[misbehaviour][1]) + + +def _wrote(script: Path) -> bool: + return Path(str(script) + ".wrote").is_file() + + +def _kept_anywhere(exception, secret: str) -> list[str]: + """`_locals_holding`, and one level deeper. The attributes of each local + are searched too, since that is where a `Popen` keeps what its child + wrote (`_fileobj2output`). So are the refusal's own arguments and + attributes.""" + found = set(_locals_holding(exception, secret)) + for frame in _frames_kept_by(exception): + for name, value in list(frame.f_locals.items()): + attributes = getattr(value, "__dict__", None) + if (isinstance(attributes, dict) + and secret in _safe_repr(attributes)): + found.add(f"{frame.f_code.co_name}.{name}.__dict__") + if (secret in _safe_repr(exception.args) + or secret in _safe_repr(vars(exception))): + found.add("the refusal itself") + return sorted(found) + + +#: Each operation, asked through its own function, with the real runner. +_OPERATIONS_ASKED = { + provider_mod.OPERATION_INTAKE: lambda binding, runner: ( + provider_mod.hand_off_credential( + binding, io.StringIO("sk-stand-in-intake-NOT-A-KEY"), + runner=runner)), + provider_mod.OPERATION_MINT: lambda binding, runner: ( + provider_mod.mint(binding, runner=runner)), + provider_mod.OPERATION_REVOKE: lambda binding, runner: ( + provider_mod.revoke(binding, runner=runner)), + provider_mod.OPERATION_LIST: lambda binding, runner: ( + provider_mod.list_references(binding, runner=runner)), +} + + +def test_every_operation_is_asked_here(): + assert tuple(_OPERATIONS_ASKED) == provider_mod.OPERATIONS + + +@pytest.mark.parametrize("misbehaviour", sorted(_MISBEHAVIOURS)) +def test_the_shared_runner_refuses_a_misbehaving_broker_keeping_nothing( + tmp_path, misbehaviour): + """The runner itself, called directly. Its refusal keeps no cause, no + context, and no frame or attribute that holds what the broker wrote. + It names the failure class. It is not told the operation, so it names + none.""" + script = _misbehaving_broker(tmp_path, misbehaviour) + argv = provider_mod.broker_operation_argv( + _broker_binding(script), provider_mod.OPERATION_MINT) + with pytest.raises(provider_mod.BrokerRefused) as caught: + provider_mod.subprocess_broker_runner( + argv, timeout=_MISBEHAVING_TIMEOUT) + refusal = caught.value + assert _wrote(script), "the broker wrote the token before it misbehaved" + assert refusal.__cause__ is None + assert refusal.__context__ is None + assert _kept_anywhere(refusal, SENTINEL_TOKEN) == [] + expected = _sentence_for(misbehaviour) + assert refusal.diagnostic == expected + assert refusal.operation is None + assert str(refusal) == expected + + +@pytest.mark.parametrize("operation", provider_mod.OPERATIONS) +@pytest.mark.parametrize("misbehaviour", sorted(_MISBEHAVIOURS)) +def test_a_misbehaving_broker_is_refused_naming_the_operation( + tmp_path, misbehaviour, operation): + """Each of the four operations, through the real runner. The refusal + names the operation and the failure class, and keeps nothing the broker + wrote.""" + script = _misbehaving_broker(tmp_path, misbehaviour) + runner = functools.partial(provider_mod.subprocess_broker_runner, + timeout=_MISBEHAVING_TIMEOUT) + with pytest.raises(provider_mod.BrokerRefused) as caught: + _OPERATIONS_ASKED[operation](_broker_binding(script), runner) + refusal = caught.value + assert _wrote(script), "the broker wrote the token before it misbehaved" + assert refusal.__cause__ is None + assert refusal.__context__ is None + assert _kept_anywhere(refusal, SENTINEL_TOKEN) == [] + expected = _sentence_for(misbehaviour) + assert refusal.diagnostic == expected + assert refusal.operation == operation + assert str(refusal) == f"broker {operation}: {expected}" + + +@pytest.mark.parametrize("operation", provider_mod.OPERATIONS) +def test_a_broker_that_cannot_be_started_chains_nothing(tmp_path, operation): + """A program that does not exist wrote nothing, and its refusal chains + nothing either. At `788d764b` it chained the `FileNotFoundError`, by the + runner and by the operation alike.""" + binding = _binding(broker_argv=(str(tmp_path / "no-such-broker"),)) + with pytest.raises(provider_mod.BrokerRefused) as caught: + provider_mod.subprocess_broker_runner( + provider_mod.broker_operation_argv(binding, operation)) + assert caught.value.__cause__ is None + assert caught.value.__context__ is None + assert caught.value.diagnostic == provider_mod.DIAG_BROKER_UNREACHABLE + with pytest.raises(provider_mod.BrokerRefused) as caught: + _OPERATIONS_ASKED[operation](binding, + provider_mod.subprocess_broker_runner) + assert caught.value.__cause__ is None + assert caught.value.__context__ is None + assert caught.value.operation == operation + assert str(caught.value) == ( + f"broker {operation}: {provider_mod.DIAG_BROKER_UNREACHABLE}") + + +@pytest.mark.parametrize("operation", provider_mod.OPERATIONS) +def test_an_injected_runners_refusal_is_named_too(tmp_path, operation): + """The operation is named by the function that asked, so a runner that + was injected is covered as the default one is.""" + def refusing(argv, **_kwargs): + raise provider_mod.BrokerRefused(provider_mod.DIAG_BROKER_REFUSED) + + with pytest.raises(provider_mod.BrokerRefused) as caught: + _OPERATIONS_ASKED[operation](_binding(), refusing) + assert caught.value.operation == operation + assert caught.value.diagnostic == provider_mod.DIAG_BROKER_REFUSED + assert caught.value.__context__ is None + + +def test_only_a_broker_sentence_names_a_declared_operation(): + refusal = provider_mod.BrokerRefused( + provider_mod.DIAG_BROKER_TIMEOUT, + operation=provider_mod.OPERATION_REVOKE) + assert str(refusal) == f"broker revoke: {provider_mod.DIAG_BROKER_TIMEOUT}" + assert refusal.diagnostic == provider_mod.DIAG_BROKER_TIMEOUT + assert refusal.operation == provider_mod.OPERATION_REVOKE + with pytest.raises(AssertionError): + provider_mod.BrokerRefused(provider_mod.DIAG_BROKER_REFUSED, + operation="exfiltrate") + with pytest.raises(AssertionError): + provider_mod.BrokerRefused(provider_mod.DIAG_PROVIDER_REFUSED, + operation=provider_mod.OPERATION_MINT) + provider_refusal = provider_mod.BrokerRefused( + provider_mod.DIAG_PROVIDER_REFUSED) + assert provider_refusal.operation is None + assert str(provider_refusal) == provider_mod.DIAG_PROVIDER_REFUSED + + +def test_the_operator_door_names_the_operation_and_withholds_the_answer( + tmp_path, capsys): + """What an operator reads when `set-credential` meets a broker that wrote + and then exited non-zero: the operation and the failure class, and none + of what it wrote.""" + script = _misbehaving_broker(tmp_path, "exits-non-zero") + checkout = tmp_path / "checkout" + (checkout / "ideation" / "dashboard").mkdir(parents=True) + store = binding_mod.BindingStore(binding_mod.bindings_path(checkout)) + store.add(_broker_binding(script, credential_ref="opref-" + "0" * 24)) + args = cli_mod.build_parser().parse_args([ + "model-binding", "set-credential", "--repo-root", str(checkout), + "--id", "openprofiler-demo"]) + assert cli_mod.cmd_model_binding_set_credential( + args, source=io.StringIO("sk-stand-in-intake-NOT-A-KEY")) == 1 + captured = capsys.readouterr() + assert captured.err == ( + f"broker intake: {provider_mod.DIAG_BROKER_REFUSED}\n") + assert SENTINEL_TOKEN not in captured.out + captured.err From b847ef3d33910e6752cc4322f27e2310b1f39464 Mon Sep 17 00:00:00 2001 From: Brett Heap <1513478+brettheap@users.noreply.github.com> Date: Wed, 30 Sep 2026 00:56:06 +0000 Subject: [PATCH 08/11] Broker runner: the answer's bound limits what is read Copilot's review of #64 at 25788f91 (high): communicate() read all of a broker's output before MAX_BROKER_ANSWER_BYTES was checked. So the bound limited nothing in memory, and a broker that wrote without end was read until the timeout and refused as a timeout. _run_broker now reads the answer on a reader thread, at most one byte past the bound. A broker that writes that byte is refused with DIAG_BROKER_OVERSIZE and killed at once. The thread starts before the credential is written, so the answer drains while intake's stdin is streamed. The runner then waits for the child's exit within what is left of the timeout. The answer is decoded as UTF-8 (JSON's encoding; the locale's before), and an answer that is not UTF-8 is malformed. Every refusal is still raised by subprocess_broker_runner after _run_broker has returned, so it keeps no cause, no context and nothing the broker wrote. Tests: a broker that writes without end is refused at the bound, well inside a 5 s timeout. At 25788f91 it read until the timeout (5.1 s measured), and that is the one case that fails there. A sixth misbehaviour, a broker that closes its output and then hangs, is refused as a timeout by the runner and by each operation. With 788d764b as the source, all six new cases fail too (43 of the 44 runner cases). The runner's mutants are rewritten for the new code. All 31 mutants are killed, among them the whole output read as at 25788f91, and no deadline on the child's exit. Arc: neutral-product-standalone-operability Lane: openxfactory-4 (openXfactory-4-openDox_extraction) Co-Authored-By: Claude Opus 5.5 (1M context) --- src/opendox/doxbench_provider.py | 121 +++++++++++++++++----------- tests/test_model_provider_broker.py | 39 ++++++++- 2 files changed, 110 insertions(+), 50 deletions(-) diff --git a/src/opendox/doxbench_provider.py b/src/opendox/doxbench_provider.py index 15f1f7f..944fc29 100644 --- a/src/opendox/doxbench_provider.py +++ b/src/opendox/doxbench_provider.py @@ -505,24 +505,44 @@ def subprocess_broker_runner(argv, *, source=None, return answer -def _reap(child) -> None: - """Kill a child this runner is refusing and finish its communication, - dropping whatever it wrote. The drop includes an answer that is not - UTF-8, whose `UnicodeDecodeError` would otherwise escape from the - handler that called this, holding that answer.""" - child.kill() +def _read_at_most(stream, limit: int, into: list) -> None: + """A reader thread's work: at most `limit` bytes of the child's standard + output, or fewer if it ends first, appended to `into`. A read that fails + appends nothing.""" try: - child.communicate() - except UnicodeDecodeError: + into.append(stream.read(limit)) + except (OSError, ValueError): pass +def _reap(child, reader) -> None: + """Kill a child this runner is refusing, wait for it, and let its reader + finish. Whatever the child wrote is dropped with the reader.""" + child.kill() + child.wait() + reader.join() + child.stdout.close() + + def _run_broker(argv, *, source, timeout: float) -> tuple[str | None, str | None]: """The work of `subprocess_broker_runner`: `(answer, None)`, or `(None, sentence)` for a refusal. It raises no refusal itself, so no refusal keeps its frame, which holds the child and what the child - wrote.""" + wrote. + + THE BOUND IS A BOUND ON WHAT IS READ (Copilot's review of + openDox-code#64 at `25788f91`). A reader thread reads at most one byte + past `MAX_BROKER_ANSWER_BYTES`, and a broker that writes that byte is + refused and killed there. At `25788f91` the whole of the child's output + was read before the bound was checked, so a broker that wrote without + end filled this process's memory until the timeout, and was refused as + a timeout. The thread also drains the answer while the credential is + still being written, as `communicate` did not. + + The answer is decoded as UTF-8, JSON's own encoding, where + `communicate` used the locale's. An answer that is not UTF-8 is + malformed.""" try: child = subprocess.Popen( # noqa: S603 - argv from a declared binding plus the declared subcommand, never a shell string list(argv), @@ -534,50 +554,59 @@ def _run_broker(argv, *, source, ) except (OSError, ValueError): return None, DIAG_BROKER_UNREACHABLE + received: list[bytes] = [] + reader = threading.Thread( + target=_read_at_most, + args=(child.stdout.buffer, MAX_BROKER_ANSWER_BYTES + 1, received), + name="broker-answer", daemon=True) + reader.start() try: + if source is not None: + # The credential's ONLY path through this process: handle to + # pipe, in chunks, never assembled. + shutil.copyfileobj(source, child.stdin) + child.stdin.close() + except BrokenPipeError: + # THE REFUSAL ARRIVING. Nothing is raised here; the exit code and + # the child's own answer are read below, exactly as the declaration + # instructs. The close is still attempted so the descriptor is not + # left to a garbage collector, and its own broken pipe is dropped + # for the same reason the first one was. try: - if source is not None: - # The credential's ONLY path through this process: handle to - # pipe, in chunks, never assembled. - shutil.copyfileobj(source, child.stdin) child.stdin.close() - except BrokenPipeError: - # THE REFUSAL ARRIVING. Nothing is raised here; the exit code and - # the child's own answer are read below, exactly as the declaration - # instructs. The close is still attempted so the descriptor is not - # left to a garbage collector, and its own broken pipe is dropped - # for the same reason the first one was. - try: - child.stdin.close() - except OSError: - pass - # `communicate` flushes `child.stdin` before reading, which raises on a - # handle this function has already closed — and closing it IS the - # signal a streamed credential's end of file needs. Dropping the - # reference is the documented way to say "stdin is finished with", and - # it is also the last place in this process that could have held the - # pipe the credential travelled down. - child.stdin = None - try: - output, _dropped_stderr = child.communicate(timeout=timeout) - except UnicodeDecodeError: - # An answer that is not UTF-8. `communicate` decodes only after - # it has waited for the child, so the exit code is read below - # as for any other answer. - output = None - except subprocess.TimeoutExpired: - _reap(child) - return None, DIAG_BROKER_TIMEOUT + except OSError: + pass except OSError: - _reap(child) + _reap(child, reader) return None, DIAG_BROKER_UNREACHABLE - if child.returncode != 0: + # Closing `child.stdin` IS the signal a streamed credential's end of file + # needs. Dropping the reference says "stdin is finished with", and it was + # the last place in this process that could have held the pipe the + # credential travelled down. + child.stdin = None + deadline = time.monotonic() + timeout + reader.join(timeout) + if reader.is_alive(): + _reap(child, reader) + return None, DIAG_BROKER_TIMEOUT + if not received: + _reap(child, reader) + return None, DIAG_BROKER_UNREACHABLE + if len(received[0]) > MAX_BROKER_ANSWER_BYTES: + _reap(child, reader) + return None, DIAG_BROKER_OVERSIZE + try: + returncode = child.wait(timeout=max(0.0, deadline - time.monotonic())) + except subprocess.TimeoutExpired: + _reap(child, reader) + return None, DIAG_BROKER_TIMEOUT + child.stdout.close() + if returncode != 0: return None, DIAG_BROKER_REFUSED - if output is None: + try: + return received[0].decode("utf-8"), None + except UnicodeDecodeError: return None, DIAG_BROKER_MALFORMED - if len(output.encode("utf-8")) > MAX_BROKER_ANSWER_BYTES: - return None, DIAG_BROKER_OVERSIZE - return output, None def broker_operation_argv(binding, operation: str, *, diff --git a/tests/test_model_provider_broker.py b/tests/test_model_provider_broker.py index 7641560..aaeaa93 100644 --- a/tests/test_model_provider_broker.py +++ b/tests/test_model_provider_broker.py @@ -3287,8 +3287,9 @@ def test_a_malformed_mint_answer_keeps_no_frame_that_holds_its_token( # * exits non-zero: the runner's frame held the token, in `answer` and in # the child's `_fileobj2output`; # * answers past the bound: the same, and the refusal read as MALFORMED; -# * times out: the refusal chained the `TimeoutExpired`, whose `output` -# and whose frames inside `subprocess` held the token; +# * times out, with its output open or closed: the refusal chained the +# `TimeoutExpired`, whose `output` and whose frames inside `subprocess` +# held the token; # * answers in bytes that are not UTF-8: a `UnicodeDecodeError` escaped # holding the token in `object`, and no refusal was raised at all. # @@ -3307,11 +3308,14 @@ def test_a_malformed_mint_answer_keeps_no_frame_that_holds_its_token( "sys.stdout.write(TOKEN)\nwrote()\nsys.exit(3)\n", "DIAG_BROKER_REFUSED"), "answers-past-the-bound": ( - "sys.stdout.write(TOKEN + 'x' * BOUND)\nwrote()\n", + "sys.stdout.write(TOKEN)\nwrote()\nsys.stdout.write('x' * BOUND)\n", "DIAG_BROKER_OVERSIZE"), "times-out": ( "sys.stdout.write(TOKEN)\nwrote()\ntime.sleep(30)\n", "DIAG_BROKER_TIMEOUT"), + "closes-its-output-and-times-out": ( + "sys.stdout.write(TOKEN)\nwrote()\nos.close(1)\ntime.sleep(30)\n", + "DIAG_BROKER_TIMEOUT"), "answers-in-no-utf-8": ( "sys.stdout.buffer.write(TOKEN.encode() + b'\\xff')\nwrote()\n", "DIAG_BROKER_MALFORMED"), @@ -3324,7 +3328,7 @@ def test_a_malformed_mint_answer_keeps_no_frame_that_holds_its_token( #: The broker's preamble. `wrote()` flushes, then leaves a mark beside the #: script, so a test can show the token was written before the misbehaviour. _MISBEHAVING_PREAMBLE = ( - "import pathlib, sys, time\n" + "import os, pathlib, sys, time\n" "TOKEN = {token!r}\n" "BOUND = {bound!r}\n" "def wrote():\n" @@ -3410,6 +3414,33 @@ def test_the_shared_runner_refuses_a_misbehaving_broker_keeping_nothing( assert str(refusal) == expected +def test_a_broker_that_writes_without_end_is_refused_at_the_bound(tmp_path): + """Copilot's review of openDox-code#64 at `25788f91`: the bound was + checked only once the whole answer had been read, so it bounded nothing + in memory. A broker that writes without end is now refused as soon as it + passes the bound, well inside the timeout, and it is killed there. At + `25788f91`, and at `788d764b`, the runner read it until the timeout and + refused it as a timeout.""" + script = tmp_path / "endless-broker.py" + script.write_text( + "import sys, time\n" + f"sys.stdout.write({SENTINEL_TOKEN!r})\n" + "while True:\n" + " sys.stdout.write('x' * 65536)\n" + " sys.stdout.flush()\n" + " time.sleep(0.01)\n", encoding="utf-8") + argv = provider_mod.broker_operation_argv( + _broker_binding(script), provider_mod.OPERATION_MINT) + started = time.monotonic() + with pytest.raises(provider_mod.BrokerRefused) as caught: + provider_mod.subprocess_broker_runner(argv, timeout=5.0) + assert time.monotonic() - started < 2.5, "refused at the bound" + assert caught.value.__cause__ is None + assert caught.value.__context__ is None + assert _kept_anywhere(caught.value, SENTINEL_TOKEN) == [] + assert caught.value.diagnostic == provider_mod.DIAG_BROKER_OVERSIZE + + @pytest.mark.parametrize("operation", provider_mod.OPERATIONS) @pytest.mark.parametrize("misbehaviour", sorted(_MISBEHAVIOURS)) def test_a_misbehaving_broker_is_refused_naming_the_operation( From e3eec6b18be7e6727f1e92f8e54fe9cd00006dda Mon Sep 17 00:00:00 2001 From: Brett Heap <1513478+brettheap@users.noreply.github.com> Date: Wed, 30 Sep 2026 15:06:01 +0000 Subject: [PATCH 09/11] Broker runner: reap the child when anything else escapes b847ef3d's reader is a daemon thread. When the credential's own source failed mid-copy (the operator's input, which the ruling does not reach), the error escaped with the child still running and the reader blocked on it, and the interpreter aborted at exit ("Fatal Python error: _enter_buffered_busy"), measured 3 of 3 times. _run_broker now reaps the child before any other exception goes on, and drops what the child wrote. The work after the reader starts moves to _answer_of, unchanged. The runner's tests also assert that no refusal keeps a frame holding the child. That kills three mutants that the reap had made equivalent: a refusal raised where it is seen, for the exit, the bound and the timeout. Tests: the failing source in a child interpreter exits 1 with its own UnicodeDecodeError and no fatal error. At b847ef3d it aborts. In this process, with a broker that wrote first, what escapes keeps nothing the broker wrote. All 33 mutants are killed. Arc: neutral-product-standalone-operability Lane: openxfactory-4 (openXfactory-4-openDox_extraction) Co-Authored-By: Claude Opus 5.5 (1M context) --- src/opendox/doxbench_provider.py | 19 +++++++ tests/test_model_provider_broker.py | 83 +++++++++++++++++++++++++++-- 2 files changed, 99 insertions(+), 3 deletions(-) diff --git a/src/opendox/doxbench_provider.py b/src/opendox/doxbench_provider.py index 944fc29..cd94602 100644 --- a/src/opendox/doxbench_provider.py +++ b/src/opendox/doxbench_provider.py @@ -560,6 +560,25 @@ def _run_broker(argv, *, source, args=(child.stdout.buffer, MAX_BROKER_ANSWER_BYTES + 1, received), name="broker-answer", daemon=True) reader.start() + try: + return _answer_of(child, reader, received, source=source, + timeout=timeout) + except BaseException: + # Anything else that escapes, such as the credential's own source + # failing while it is copied (the operator's input, not the broker's + # output), goes on as it was. The child is reaped first, and what it + # wrote is dropped: a reader left blocked in its daemon thread would + # abort the interpreter when it exits. + _reap(child, reader) + received.clear() + raise + + +def _answer_of(child, reader, received: list, *, source, + timeout: float) -> tuple[str | None, str | None]: + """`_run_broker`'s work once the child and its reader are running: the + credential, if any, then the answer, within the bound and the + timeout.""" try: if source is not None: # The credential's ONLY path through this process: handle to diff --git a/tests/test_model_provider_broker.py b/tests/test_model_provider_broker.py index aaeaa93..2b7dfc6 100644 --- a/tests/test_model_provider_broker.py +++ b/tests/test_model_provider_broker.py @@ -3371,6 +3371,16 @@ def _kept_anywhere(exception, secret: str) -> list[str]: return sorted(found) +def _children_kept_by(exception) -> list[str]: + """The frame locals that hold a broker child (`subprocess.Popen`). A + child holds the pipe its answer came down, and whatever that pipe's + buffers still hold, so no refusal keeps one.""" + return sorted({f"{frame.f_code.co_name}.{name}" + for frame in _frames_kept_by(exception) + for name, value in list(frame.f_locals.items()) + if isinstance(value, subprocess.Popen)}) + + #: Each operation, asked through its own function, with the real runner. _OPERATIONS_ASKED = { provider_mod.OPERATION_INTAKE: lambda binding, runner: ( @@ -3394,9 +3404,9 @@ def test_every_operation_is_asked_here(): def test_the_shared_runner_refuses_a_misbehaving_broker_keeping_nothing( tmp_path, misbehaviour): """The runner itself, called directly. Its refusal keeps no cause, no - context, and no frame or attribute that holds what the broker wrote. - It names the failure class. It is not told the operation, so it names - none.""" + context, no frame or attribute that holds what the broker wrote, and no + frame that holds the child. It names the failure class. It is not told + the operation, so it names none.""" script = _misbehaving_broker(tmp_path, misbehaviour) argv = provider_mod.broker_operation_argv( _broker_binding(script), provider_mod.OPERATION_MINT) @@ -3408,6 +3418,7 @@ def test_the_shared_runner_refuses_a_misbehaving_broker_keeping_nothing( assert refusal.__cause__ is None assert refusal.__context__ is None assert _kept_anywhere(refusal, SENTINEL_TOKEN) == [] + assert _children_kept_by(refusal) == [] expected = _sentence_for(misbehaviour) assert refusal.diagnostic == expected assert refusal.operation is None @@ -3438,6 +3449,7 @@ def test_a_broker_that_writes_without_end_is_refused_at_the_bound(tmp_path): assert caught.value.__cause__ is None assert caught.value.__context__ is None assert _kept_anywhere(caught.value, SENTINEL_TOKEN) == [] + assert _children_kept_by(caught.value) == [] assert caught.value.diagnostic == provider_mod.DIAG_BROKER_OVERSIZE @@ -3458,6 +3470,7 @@ def test_a_misbehaving_broker_is_refused_naming_the_operation( assert refusal.__cause__ is None assert refusal.__context__ is None assert _kept_anywhere(refusal, SENTINEL_TOKEN) == [] + assert _children_kept_by(refusal) == [] expected = _sentence_for(misbehaviour) assert refusal.diagnostic == expected assert refusal.operation == operation @@ -3486,6 +3499,70 @@ def test_a_broker_that_cannot_be_started_chains_nothing(tmp_path, operation): f"broker {operation}: {provider_mod.DIAG_BROKER_UNREACHABLE}") +def test_a_credential_source_that_fails_leaves_no_broker_running(tmp_path): + """The credential's own source is the operator's input, not the + broker's output, so the ruling does not reach it. A source that fails + while it is copied still escapes as it did. But the broker must not be + left running with its reader blocked on it, which aborted the + interpreter at exit at `b847ef3d` ("Fatal Python error: + _enter_buffered_busy"). A child interpreter runs it, so that its exit + is what is measured.""" + binding = _broker_binding(_write_broker(tmp_path)) + fields = {field.name: getattr(binding, field.name) + for field in dataclasses.fields(binding)} + program = ( + "from opendox import doxbench_binding as b\n" + "from opendox import doxbench_provider as p\n" + "class Failing:\n" + " parts = ['sk-stand-in-input-side-NOT-A-KEY']\n" + " def read(self, _size=-1):\n" + " if self.parts:\n" + " return self.parts.pop()\n" + " raise UnicodeDecodeError('utf-8', b'x', 0, 1, 'stand-in')\n" + f"binding = b.ModelProviderBinding(**{fields!r})\n" + "p.hand_off_credential(binding, Failing())\n") + run = subprocess.run([sys.executable, "-c", program], cwd=tmp_path, + capture_output=True, text=True, timeout=60, + check=False) + assert "Fatal Python error" not in run.stderr + assert run.returncode == 1 + assert "UnicodeDecodeError" in run.stderr + assert "sk-stand-in-input-side-NOT-A-KEY" not in run.stderr + + +class _SourceFailingOnceMarked: + """A credential source that fails on its second read, once `mark` + exists, so the broker has written before it fails.""" + + def __init__(self, mark: Path) -> None: + self.parts = ["sk-stand-in-input-side-NOT-A-KEY"] + self.mark = mark + + def read(self, _size=-1): + if self.parts: + return self.parts.pop() + deadline = time.monotonic() + 10 + while not self.mark.exists() and time.monotonic() < deadline: + time.sleep(0.01) + raise UnicodeDecodeError("utf-8", b"x", 0, 1, "stand-in") + + +def test_a_failing_credential_source_escapes_with_no_broker_output(tmp_path): + """The same escape, in this process, from a broker that wrote the + token before it read its standard input. What escapes keeps nothing the + broker wrote, as a refusal would not.""" + script = tmp_path / "early-writing-broker.py" + script.write_text(_MISBEHAVING_PREAMBLE.format( + token=SENTINEL_TOKEN, bound=provider_mod.MAX_BROKER_ANSWER_BYTES) + + "sys.stdout.write(TOKEN)\nwrote()\nsys.stdin.read()\n", + encoding="utf-8") + source = _SourceFailingOnceMarked(Path(str(script) + ".wrote")) + with pytest.raises(UnicodeDecodeError) as caught: + provider_mod.hand_off_credential(_broker_binding(script), source) + assert _wrote(script), "the broker wrote before the source failed" + assert _kept_anywhere(caught.value, SENTINEL_TOKEN) == [] + + @pytest.mark.parametrize("operation", provider_mod.OPERATIONS) def test_an_injected_runners_refusal_is_named_too(tmp_path, operation): """The operation is named by the function that asked, so a runner that From a603a032739071f53da58f661538d7ec9a5ca9d3 Mon Sep 17 00:00:00 2001 From: Brett Heap <1513478+brettheap@users.noreply.github.com> Date: Wed, 30 Sep 2026 15:46:31 +0000 Subject: [PATCH 10/11] Broker runner: a refusal kills the broker's process group, and waits boundedly Copilot's review of #64 at b847ef3d: a descendant that inherits the broker's standard output kept the pipe open after the broker was killed, so the reader, and the refusal, waited forever. That was measured at e3eec6b1: a broker with such a descendant and a 0.5 s timeout was still not refused after 10 s. The broker now starts in its own process group (process_group=0; the session and terminal stay this process's). A refusal kills the whole group, and waits at most BROKER_REAP_SECONDS (2 s) for the reader. A descendant that left the group is left to the daemon reader, and the refusal does not wait on it. The reader reads the descriptor with os.read, so a reader still blocked at interpreter exit holds no buffered lock that finalization needs. Tests: a descendant in the group, refused within the timeout plus the grace, and one that left it (setsid), refused within the grace plus a margin. Both hang at e3eec6b1. The failing-source case now also asserts that the broker is not left running. All 36 mutants are killed, among them the broker killed alone, no group of its own, and an unbounded wait for the reader. Arc: neutral-product-standalone-operability Lane: openxfactory-4 (openXfactory-4-openDox_extraction) Co-Authored-By: Claude Opus 5.5 (1M context) --- src/opendox/doxbench_provider.py | 59 +++++++++++++++++++++++++---- tests/test_model_provider_broker.py | 57 +++++++++++++++++++++++++++- 2 files changed, 106 insertions(+), 10 deletions(-) diff --git a/src/opendox/doxbench_provider.py b/src/opendox/doxbench_provider.py index cd94602..82d1a24 100644 --- a/src/opendox/doxbench_provider.py +++ b/src/opendox/doxbench_provider.py @@ -105,6 +105,7 @@ import math import os import shutil +import signal import subprocess import sys import threading @@ -203,6 +204,11 @@ #: cannot answer. BROKER_TIMEOUT_SECONDS = 30.0 +#: How long a refused broker's reader is waited for once the broker's process +#: group has been killed. A descendant that left the group can still hold the +#: answer's pipe open, and the refusal does not wait on it past this. +BROKER_REAP_SECONDS = 2.0 + #: The largest answer a broker may write. A bound, not a policy: an unbounded #: read of a child's stdout is a way to spend this process's memory by #: misconfiguring a binding. @@ -508,20 +514,53 @@ def subprocess_broker_runner(argv, *, source=None, def _read_at_most(stream, limit: int, into: list) -> None: """A reader thread's work: at most `limit` bytes of the child's standard output, or fewer if it ends first, appended to `into`. A read that fails - appends nothing.""" + appends nothing. + + It reads the descriptor itself (`os.read`), not the buffered file, so a + reader still blocked when the interpreter exits holds no lock that + finalization needs. The thread keeps `stream`, so the descriptor is not + closed and reused under it.""" + chunks: list[bytes] = [] + size = 0 try: - into.append(stream.read(limit)) + descriptor = stream.fileno() + while size < limit: + chunk = os.read(descriptor, min(65_536, limit - size)) + if not chunk: + break + chunks.append(chunk) + size += len(chunk) except (OSError, ValueError): - pass + return + into.append(b"".join(chunks)) -def _reap(child, reader) -> None: - """Kill a child this runner is refusing, wait for it, and let its reader - finish. Whatever the child wrote is dropped with the reader.""" +def _kill_the_group(child) -> None: + """Kill the broker and every descendant still in its process group. A + descendant holding the answer's pipe open would otherwise keep the + reader, and so the refusal, waiting. Where process groups do not exist, + the broker alone is killed.""" + killpg = getattr(os, "killpg", None) + if killpg is not None: + try: + killpg(child.pid, signal.SIGKILL) + return + except OSError: + pass child.kill() + + +def _reap(child, reader) -> None: + """Kill a child this runner is refusing, and its process group, wait for + it, and let its reader finish, for at most `BROKER_REAP_SECONDS`. + Whatever the child wrote is dropped with the reader. A reader still + blocked then (a descendant that left the group holds the pipe) is left + to its daemon thread, with the descriptor it reads.""" + _kill_the_group(child) child.wait() - reader.join() - child.stdout.close() + reader.join(BROKER_REAP_SECONDS) + if not reader.is_alive(): + child.stdout.close() def _run_broker(argv, *, source, @@ -551,6 +590,10 @@ def _run_broker(argv, *, source, stderr=subprocess.DEVNULL, env=bridge_mod.child_environment(os.environ), text=True, + # Its own process group, so a refusal can kill its descendants + # too (`_kill_the_group`). The session, and so the terminal, is + # this process's. + process_group=0 if hasattr(os, "killpg") else None, ) except (OSError, ValueError): return None, DIAG_BROKER_UNREACHABLE diff --git a/tests/test_model_provider_broker.py b/tests/test_model_provider_broker.py index 2b7dfc6..da900f0 100644 --- a/tests/test_model_provider_broker.py +++ b/tests/test_model_provider_broker.py @@ -54,6 +54,7 @@ import http.server import io import json +import os import socket import subprocess import sys @@ -3530,6 +3531,52 @@ def test_a_credential_source_that_fails_leaves_no_broker_running(tmp_path): assert "sk-stand-in-input-side-NOT-A-KEY" not in run.stderr +@pytest.mark.parametrize("leaves_the_group", [False, True], + ids=["descendant-in-its-group", + "descendant-that-left-it"]) +def test_a_broker_whose_descendant_holds_its_output_is_still_refused_in_time( + tmp_path, leaves_the_group): + """Copilot's review of openDox-code#64 at `b847ef3d`: a descendant that + inherits the broker's standard output kept the pipe open after the + broker was killed, so the reader, and the refusal, waited forever. The + broker now has its own process group, which a refusal kills whole. A + descendant that left the group is waited for no longer than + `BROKER_REAP_SECONDS`.""" + script = tmp_path / "forking-broker.py" + script.write_text( + "import os, subprocess, sys, time\n" + "subprocess.Popen([sys.executable, '-c', " + f"'import os, time\\n{'os.setsid()' if leaves_the_group else 'pass'}" + "\\ntime.sleep(10)'])\n" + f"sys.stdout.write({SENTINEL_TOKEN!r})\n" + "sys.stdout.flush()\n" + "time.sleep(30)\n", encoding="utf-8") + argv = provider_mod.broker_operation_argv( + _broker_binding(script), provider_mod.OPERATION_MINT) + caught: list = [] + + def run(): + try: + provider_mod.subprocess_broker_runner(argv, timeout=0.5) + except provider_mod.BrokerRefused as refusal: + caught.append(refusal) + + runner = threading.Thread(target=run, daemon=True) + started = time.monotonic() + runner.start() + runner.join(10) + assert not runner.is_alive(), "the refusal waited on the descendant" + elapsed = time.monotonic() - started + [refusal] = caught + grace = provider_mod.BROKER_REAP_SECONDS + assert elapsed < 0.5 + grace + 3 + if not leaves_the_group: + assert elapsed < 0.5 + grace, \ + "the whole group is killed, so nothing is left to wait on" + assert refusal.diagnostic == provider_mod.DIAG_BROKER_TIMEOUT + assert _kept_anywhere(refusal, SENTINEL_TOKEN) == [] + + class _SourceFailingOnceMarked: """A credential source that fails on its second read, once `mark` exists, so the broker has written before it fails.""" @@ -3550,17 +3597,23 @@ def read(self, _size=-1): def test_a_failing_credential_source_escapes_with_no_broker_output(tmp_path): """The same escape, in this process, from a broker that wrote the token before it read its standard input. What escapes keeps nothing the - broker wrote, as a refusal would not.""" + broker wrote, as a refusal would not, and the broker is not left + running, where it would read the end of its input and store whatever + part of the credential had reached it.""" script = tmp_path / "early-writing-broker.py" script.write_text(_MISBEHAVING_PREAMBLE.format( token=SENTINEL_TOKEN, bound=provider_mod.MAX_BROKER_ANSWER_BYTES) - + "sys.stdout.write(TOKEN)\nwrote()\nsys.stdin.read()\n", + + "pathlib.Path(sys.argv[0] + '.pid').write_text(str(os.getpid()))\n" + "sys.stdout.write(TOKEN)\nwrote()\nsys.stdin.read()\n", encoding="utf-8") source = _SourceFailingOnceMarked(Path(str(script) + ".wrote")) with pytest.raises(UnicodeDecodeError) as caught: provider_mod.hand_off_credential(_broker_binding(script), source) assert _wrote(script), "the broker wrote before the source failed" assert _kept_anywhere(caught.value, SENTINEL_TOKEN) == [] + pid = int(Path(str(script) + ".pid").read_text(encoding="utf-8")) + with pytest.raises(ProcessLookupError): + os.kill(pid, 0) @pytest.mark.parametrize("operation", provider_mod.OPERATIONS) From e75900ff31eb6a5c7206de793424d37f93799b72 Mon Sep 17 00:00:00 2001 From: Brett Heap <1513478+brettheap@users.noreply.github.com> Date: Wed, 30 Sep 2026 16:39:00 +0000 Subject: [PATCH 11/11] Broker runner: one selector loop, one deadline, no reader thread Copilot's review of #64 at a603a032: a descendant that left the broker's process group left the timed-out reader thread blocked, with its pipe descriptor, behind every refusal, and that reader could still add to what had been read after received was cleared. The reader thread is gone. One selector loop in the calling thread streams the credential to the broker's stdin in PIPE_BUF writes and reads the answer, as communicate does on POSIX, under one deadline that now covers the credential's streaming too. A refusal kills the broker's process group and closes this process's ends of both pipes. So a descendant that left the group costs no thread and no descriptor, and nothing can add to the answer once the loop stops. The answer is read straight into received, so no other local holds it. BROKER_REAP_SECONDS is gone. Measured at a603a032: - a broker whose descendant left the group was refused only after the 2 s reap grace (2.5 s with a 0.5 s timeout), and left its reader behind; - a broker that never read a 1 MB credential held the refusal past 10 s, because the timeout began only after the credential was written. Tests: the descendant case now also asserts that no thread and no descriptor is left behind, and that an in-group descendant is killed. A broker that reads 5000 bytes of a 1 MB credential is refused at its 0.5 s timeout. The failing-source case now lets the answer be read first. That exposed a local, chunk, holding the answer in the escaping traceback, and it is removed. All 35 mutants are killed. Arc: neutral-product-standalone-operability Lane: openxfactory-4 (openXfactory-4-openDox_extraction) Co-Authored-By: Claude Opus 5.5 (1M context) --- src/opendox/doxbench_provider.py | 191 ++++++++++++++-------------- tests/test_model_provider_broker.py | 109 ++++++++++++---- 2 files changed, 179 insertions(+), 121 deletions(-) diff --git a/src/opendox/doxbench_provider.py b/src/opendox/doxbench_provider.py index 82d1a24..2364551 100644 --- a/src/opendox/doxbench_provider.py +++ b/src/opendox/doxbench_provider.py @@ -100,11 +100,13 @@ from __future__ import annotations +import codecs import dataclasses import json import math import os -import shutil +import select +import selectors import signal import subprocess import sys @@ -204,10 +206,8 @@ #: cannot answer. BROKER_TIMEOUT_SECONDS = 30.0 -#: How long a refused broker's reader is waited for once the broker's process -#: group has been killed. A descendant that left the group can still hold the -#: answer's pipe open, and the refusal does not wait on it past this. -BROKER_REAP_SECONDS = 2.0 +#: How much of the credential is read from its source at a time. +_STDIN_CHUNK = 8192 #: The largest answer a broker may write. A bound, not a policy: an unbounded #: read of a child's stdout is a way to spend this process's memory by @@ -462,8 +462,8 @@ def subprocess_broker_runner(argv, *, source=None, operation reads no standard input at all, and this function closes the pipe immediately for them, which is what the declaration says a caller may do. - The credential is STREAMED, not read: `shutil.copyfileobj` moves it in - chunks from the operator's handle to the child's pipe, so the whole value + The credential is STREAMED, not read: `_answer_of` moves it in chunks + from the operator's handle to the child's pipe, so the whole value never becomes a string in this process and there is no variable holding it to outlive the call. @@ -511,35 +511,11 @@ def subprocess_broker_runner(argv, *, source=None, return answer -def _read_at_most(stream, limit: int, into: list) -> None: - """A reader thread's work: at most `limit` bytes of the child's standard - output, or fewer if it ends first, appended to `into`. A read that fails - appends nothing. - - It reads the descriptor itself (`os.read`), not the buffered file, so a - reader still blocked when the interpreter exits holds no lock that - finalization needs. The thread keeps `stream`, so the descriptor is not - closed and reused under it.""" - chunks: list[bytes] = [] - size = 0 - try: - descriptor = stream.fileno() - while size < limit: - chunk = os.read(descriptor, min(65_536, limit - size)) - if not chunk: - break - chunks.append(chunk) - size += len(chunk) - except (OSError, ValueError): - return - into.append(b"".join(chunks)) - - def _kill_the_group(child) -> None: """Kill the broker and every descendant still in its process group. A descendant holding the answer's pipe open would otherwise keep the - reader, and so the refusal, waiting. Where process groups do not exist, - the broker alone is killed.""" + answer from ending. Where process groups do not exist, the broker alone + is killed.""" killpg = getattr(os, "killpg", None) if killpg is not None: try: @@ -550,17 +526,25 @@ def _kill_the_group(child) -> None: child.kill() -def _reap(child, reader) -> None: +def _reap(child) -> None: """Kill a child this runner is refusing, and its process group, wait for - it, and let its reader finish, for at most `BROKER_REAP_SECONDS`. - Whatever the child wrote is dropped with the reader. A reader still - blocked then (a descendant that left the group holds the pipe) is left - to its daemon thread, with the descriptor it reads.""" + it, and close both of this process's ends of its pipes. Nothing is left + reading them. A descendant that left the group keeps only its own copy + of the pipe, which nothing here waits on.""" _kill_the_group(child) child.wait() - reader.join(BROKER_REAP_SECONDS) - if not reader.is_alive(): - child.stdout.close() + _close_quietly(child.stdin) + child.stdin = None + child.stdout.close() + + +def _close_quietly(stream) -> None: + if stream is None: + return + try: + stream.close() + except OSError: + pass def _run_broker(argv, *, source, @@ -571,13 +555,20 @@ def _run_broker(argv, *, source, wrote. THE BOUND IS A BOUND ON WHAT IS READ (Copilot's review of - openDox-code#64 at `25788f91`). A reader thread reads at most one byte - past `MAX_BROKER_ANSWER_BYTES`, and a broker that writes that byte is + openDox-code#64 at `25788f91`). At most one byte past + `MAX_BROKER_ANSWER_BYTES` is read, and a broker that writes that byte is refused and killed there. At `25788f91` the whole of the child's output was read before the bound was checked, so a broker that wrote without - end filled this process's memory until the timeout, and was refused as - a timeout. The thread also drains the answer while the credential is - still being written, as `communicate` did not. + end filled this process's memory until the timeout. + + ONE LOOP, IN THIS THREAD, AND ONE DEADLINE (Copilot's reviews at + `b847ef3d` and `a603a032`). The credential is written and the answer is + read by one selector loop, as `communicate` does on POSIX, and the + timeout covers both. There is no reader thread to abandon. A refusal + kills the broker's process group and closes this process's ends of both + pipes, so a descendant that left the group, and holds the answer's pipe + open, costs no thread and no descriptor here and can add nothing to + what was read. The answer is decoded as UTF-8, JSON's own encoding, where `communicate` used the locale's. An answer that is not UTF-8 is @@ -598,75 +589,83 @@ def _run_broker(argv, *, source, except (OSError, ValueError): return None, DIAG_BROKER_UNREACHABLE received: list[bytes] = [] - reader = threading.Thread( - target=_read_at_most, - args=(child.stdout.buffer, MAX_BROKER_ANSWER_BYTES + 1, received), - name="broker-answer", daemon=True) - reader.start() try: - return _answer_of(child, reader, received, source=source, - timeout=timeout) + return _answer_of(child, received, source=source, timeout=timeout) except BaseException: # Anything else that escapes, such as the credential's own source # failing while it is copied (the operator's input, not the broker's # output), goes on as it was. The child is reaped first, and what it - # wrote is dropped: a reader left blocked in its daemon thread would - # abort the interpreter when it exits. - _reap(child, reader) + # wrote is dropped. Nothing else can add to it once this loop has + # stopped. + _reap(child) received.clear() raise -def _answer_of(child, reader, received: list, *, source, +def _answer_of(child, received: list, *, source, timeout: float) -> tuple[str | None, str | None]: - """`_run_broker`'s work once the child and its reader are running: the - credential, if any, then the answer, within the bound and the - timeout.""" - try: - if source is not None: - # The credential's ONLY path through this process: handle to - # pipe, in chunks, never assembled. - shutil.copyfileobj(source, child.stdin) - child.stdin.close() - except BrokenPipeError: - # THE REFUSAL ARRIVING. Nothing is raised here; the exit code and - # the child's own answer are read below, exactly as the declaration - # instructs. The close is still attempted so the descriptor is not - # left to a garbage collector, and its own broken pipe is dropped - # for the same reason the first one was. - try: - child.stdin.close() - except OSError: - pass - except OSError: - _reap(child, reader) - return None, DIAG_BROKER_UNREACHABLE - # Closing `child.stdin` IS the signal a streamed credential's end of file - # needs. Dropping the reference says "stdin is finished with", and it was - # the last place in this process that could have held the pipe the - # credential travelled down. - child.stdin = None + """`_run_broker`'s work once the child is running: the credential, if + any, streamed to its standard input, and its answer read, within the + bound and the timeout.""" deadline = time.monotonic() + timeout - reader.join(timeout) - if reader.is_alive(): - _reap(child, reader) - return None, DIAG_BROKER_TIMEOUT - if not received: - _reap(child, reader) - return None, DIAG_BROKER_UNREACHABLE - if len(received[0]) > MAX_BROKER_ANSWER_BYTES: - _reap(child, reader) - return None, DIAG_BROKER_OVERSIZE + encoder = codecs.getincrementalencoder(child.stdin.encoding)() + pending = b"" + source_done = source is None + size = 0 + with selectors.DefaultSelector() as selector: + selector.register(child.stdout, selectors.EVENT_READ) + selector.register(child.stdin, selectors.EVENT_WRITE) + while selector.get_map(): + remaining = deadline - time.monotonic() + if remaining <= 0: + _reap(child) + return None, DIAG_BROKER_TIMEOUT + for key, _events in selector.select(remaining): + if key.fileobj is child.stdin: + if not pending and not source_done: + # The credential's ONLY path through this process: + # handle to pipe, in chunks, never assembled. + text = source.read(_STDIN_CHUNK) + pending = encoder.encode(text, final=not text) + source_done = not text + try: + written = os.write(child.stdin.fileno(), + pending[:select.PIPE_BUF]) + except BrokenPipeError: + # THE REFUSAL ARRIVING. Nothing is raised here; the + # exit code and the child's own answer are read + # below, exactly as the declaration instructs. + written, pending, source_done = 0, b"", True + pending = pending[written:] + if source_done and not pending: + # Closing it IS the signal a streamed credential's + # end of file needs. + selector.unregister(child.stdin) + _close_quietly(child.stdin) + child.stdin = None + continue + # Read straight into `received`, so no other name in this + # frame holds what the broker wrote. + received.append(os.read(child.stdout.fileno(), + MAX_BROKER_ANSWER_BYTES + 1 - size)) + if not received[-1]: + received.pop() + selector.unregister(child.stdout) + continue + size += len(received[-1]) + if size > MAX_BROKER_ANSWER_BYTES: + _reap(child) + return None, DIAG_BROKER_OVERSIZE try: returncode = child.wait(timeout=max(0.0, deadline - time.monotonic())) except subprocess.TimeoutExpired: - _reap(child, reader) + _reap(child) return None, DIAG_BROKER_TIMEOUT child.stdout.close() if returncode != 0: return None, DIAG_BROKER_REFUSED try: - return received[0].decode("utf-8"), None + return b"".join(received).decode("utf-8"), None except UnicodeDecodeError: return None, DIAG_BROKER_MALFORMED diff --git a/tests/test_model_provider_broker.py b/tests/test_model_provider_broker.py index da900f0..e30e674 100644 --- a/tests/test_model_provider_broker.py +++ b/tests/test_model_provider_broker.py @@ -3531,23 +3531,42 @@ def test_a_credential_source_that_fails_leaves_no_broker_running(tmp_path): assert "sk-stand-in-input-side-NOT-A-KEY" not in run.stderr +def _still_running(pid: int, *, within: float = 2.0) -> bool: + """Whether `pid` is still running after `within` seconds. A zombie, + which a container's first process may never reap, has stopped.""" + deadline = time.monotonic() + within + while True: + try: + state = Path(f"/proc/{pid}/stat").read_text().rsplit(")", 1)[1] + except OSError: + return False + if state.split()[0] in ("Z", "X"): + return False + if time.monotonic() > deadline: + return True + time.sleep(0.05) + + @pytest.mark.parametrize("leaves_the_group", [False, True], ids=["descendant-in-its-group", "descendant-that-left-it"]) def test_a_broker_whose_descendant_holds_its_output_is_still_refused_in_time( tmp_path, leaves_the_group): - """Copilot's review of openDox-code#64 at `b847ef3d`: a descendant that - inherits the broker's standard output kept the pipe open after the - broker was killed, so the reader, and the refusal, waited forever. The - broker now has its own process group, which a refusal kills whole. A - descendant that left the group is waited for no longer than - `BROKER_REAP_SECONDS`.""" + """Copilot's reviews of openDox-code#64 at `b847ef3d` and `a603a032`. + A descendant that inherits the broker's standard output kept the pipe + open after the broker was killed. At `e3eec6b1` the refusal waited on it + forever. At `a603a032` a descendant that left the group left a reader + thread blocked, and its descriptor open, behind every refusal. The + broker now has its own process group, which a refusal kills whole, and + one loop in the calling thread reads the answer. So the refusal comes + at the timeout, and leaves no thread and no descriptor behind.""" script = tmp_path / "forking-broker.py" script.write_text( "import os, subprocess, sys, time\n" - "subprocess.Popen([sys.executable, '-c', " + "descendant = subprocess.Popen([sys.executable, '-c', " f"'import os, time\\n{'os.setsid()' if leaves_the_group else 'pass'}" "\\ntime.sleep(10)'])\n" + "open(sys.argv[0] + '.pid', 'w').write(str(descendant.pid))\n" f"sys.stdout.write({SENTINEL_TOKEN!r})\n" "sys.stdout.flush()\n" "time.sleep(30)\n", encoding="utf-8") @@ -3561,6 +3580,8 @@ def run(): except provider_mod.BrokerRefused as refusal: caught.append(refusal) + threads = threading.active_count() + descriptors = len(os.listdir("/proc/self/fd")) runner = threading.Thread(target=run, daemon=True) started = time.monotonic() runner.start() @@ -3568,43 +3589,81 @@ def run(): assert not runner.is_alive(), "the refusal waited on the descendant" elapsed = time.monotonic() - started [refusal] = caught - grace = provider_mod.BROKER_REAP_SECONDS - assert elapsed < 0.5 + grace + 3 + assert elapsed < 0.5 + 2, "refused at the timeout" + assert threading.active_count() == threads, "no thread is left behind" + assert len(os.listdir("/proc/self/fd")) == descriptors, \ + "no descriptor is left behind" + descendant = int(Path(str(script) + ".pid").read_text(encoding="utf-8")) if not leaves_the_group: - assert elapsed < 0.5 + grace, \ - "the whole group is killed, so nothing is left to wait on" + assert not _still_running(descendant), \ + "the broker's whole process group is killed" assert refusal.diagnostic == provider_mod.DIAG_BROKER_TIMEOUT assert _kept_anywhere(refusal, SENTINEL_TOKEN) == [] +def test_a_broker_that_never_reads_the_credential_is_refused_in_time( + tmp_path): + """The timeout covers the credential's streaming too. At `a603a032` the + credential was written before the timeout began, so a broker that never + read a credential larger than its pipe held the refusal until the broker + itself exited.""" + script = tmp_path / "deaf-broker.py" + # It reads a little and then stops, so the pipe has room for some of + # the credential but not for all of it. + script.write_text("import os, time\nos.read(0, 5000)\ntime.sleep(30)\n", + encoding="utf-8") + binding = _broker_binding(script) + runner = functools.partial(provider_mod.subprocess_broker_runner, + timeout=0.5) + caught: list = [] + + def run(): + try: + provider_mod.hand_off_credential( + binding, io.StringIO("k" * 1_000_000), runner=runner) + except provider_mod.BrokerRefused as refusal: + caught.append(refusal) + + thread = threading.Thread(target=run, daemon=True) + started = time.monotonic() + thread.start() + thread.join(10) + assert not thread.is_alive(), "the refusal waited on the broker" + assert time.monotonic() - started < 0.5 + 2 + [refusal] = caught + assert refusal.diagnostic == provider_mod.DIAG_BROKER_TIMEOUT + assert refusal.operation == provider_mod.OPERATION_INTAKE + + class _SourceFailingOnceMarked: - """A credential source that fails on its second read, once `mark` - exists, so the broker has written before it fails.""" + """A credential source that gives a small part at each read, and fails + a few reads after `mark` exists. So the broker has written, and its + answer has been read, before the source fails.""" def __init__(self, mark: Path) -> None: - self.parts = ["sk-stand-in-input-side-NOT-A-KEY"] self.mark = mark + self.reads_since_marked = 0 def read(self, _size=-1): - if self.parts: - return self.parts.pop() - deadline = time.monotonic() + 10 - while not self.mark.exists() and time.monotonic() < deadline: - time.sleep(0.01) - raise UnicodeDecodeError("utf-8", b"x", 0, 1, "stand-in") + time.sleep(0.02) + if self.mark.exists(): + self.reads_since_marked += 1 + if self.reads_since_marked > 5: + raise UnicodeDecodeError("utf-8", b"x", 0, 1, "stand-in") + return "sk-stand-in-input-side-NOT-A-KEY" def test_a_failing_credential_source_escapes_with_no_broker_output(tmp_path): """The same escape, in this process, from a broker that wrote the - token before it read its standard input. What escapes keeps nothing the - broker wrote, as a refusal would not, and the broker is not left - running, where it would read the end of its input and store whatever - part of the credential had reached it.""" + token while the credential was still streaming. What escapes keeps + nothing the broker wrote, as a refusal would not. The broker is not + left running, where it would read the end of its input and store + whatever part of the credential had reached it.""" script = tmp_path / "early-writing-broker.py" script.write_text(_MISBEHAVING_PREAMBLE.format( token=SENTINEL_TOKEN, bound=provider_mod.MAX_BROKER_ANSWER_BYTES) + "pathlib.Path(sys.argv[0] + '.pid').write_text(str(os.getpid()))\n" - "sys.stdout.write(TOKEN)\nwrote()\nsys.stdin.read()\n", + "sys.stdout.write(TOKEN)\nwrote()\ntime.sleep(30)\n", encoding="utf-8") source = _SourceFailingOnceMarked(Path(str(script) + ".wrote")) with pytest.raises(UnicodeDecodeError) as caught: