From 4514a9ef2011e5d4a8fdf47fcd88807dcb7cb53f Mon Sep 17 00:00:00 2001 From: Brett Heap <1513478+brettheap@users.noreply.github.com> Date: Mon, 28 Sep 2026 18:22:38 +0000 Subject: [PATCH 01/20] T078: 16.1, the OpenAI-compatible dialect joins DIALECTS (plan 034) `openai-chat-v1` joins doxbench_binding.DIALECTS as the second member, after `xfactory-prompt-v1`, which stays first. Its request is the chat-completions grammar (`model`, `messages`, the assembled prompt as one message in the user role), and its answer is read at `choices[0].message.content`. Both are spoken by one arm in doxbench_provider.py alone (`_DIALECT_ARMS`), beside the prompt grammar's arm, which sends the bytes it always sent. An unknown dialect is still refused when a binding is declared, and a test holds the arm table's keys equal to the vocabulary. Falsifier: F16.1's dialect assertion, `"openai-chat-v1" in b.DIALECTS`. Ruled: R1Q22 (a), openxFactory#656 comment 5817152735. doxbench_binding.py is a moved_verbatim row, and editing it needs no declared-edit act. Drafted ahead of T063 under Brett's phase-3 word ("Only the independent ones"). It does not land before T063. 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 | 30 ++-- src/opendox/doxbench_provider.py | 106 ++++++++++-- tests/test_model_provider_broker.py | 249 +++++++++++++++++++++++++++- 3 files changed, 358 insertions(+), 27 deletions(-) diff --git a/src/opendox/doxbench_binding.py b/src/opendox/doxbench_binding.py index fdd9e2ff..3b1e2131 100644 --- a/src/opendox/doxbench_binding.py +++ b/src/opendox/doxbench_binding.py @@ -93,21 +93,31 @@ AUTH_KINDS: tuple[str, ...] = (AUTH_KIND_API_KEY, AUTH_KIND_OAUTH) #: The CLOSED dialect vocabulary a binding may declare — the request grammar the -#: provider client speaks at the declared endpoint. ONE member today: this -#: repository's own already-declared turn shape, a prompt in and an -#: `assistant_prose` out, which is the shape `doxbench_model.dispatch_turn` -#: validates on the way back, so no second response grammar exists to keep -#: honest. CLOSED rather than open because an UNKNOWN dialect must REFUSE rather -#: than be guessed at: sending an assembled prompt to an endpoint whose grammar -#: this client does not know is a paid call that cannot succeed. A second member -#: joins here and an arm joins beside the first in `doxbench_provider`; the check -#: is never loosened. +#: provider client speaks at the declared endpoint. CLOSED rather than open +#: because an UNKNOWN dialect must REFUSE rather than be guessed at: sending an +#: assembled prompt to an endpoint whose grammar this client does not know is a +#: paid call that cannot succeed. A member joins here and an arm joins beside the +#: others in `doxbench_provider`; the check is never loosened. +#: +#: TWO MEMBERS, and the second joined exactly that way (#1144 box 16.1; plan 034 +#: T078): +#: +#: * `xfactory-prompt-v1` — this repository's own already-declared turn shape, +#: a POST of a model and a prompt answered by an `assistant_prose`, which is +#: the shape `doxbench_model.dispatch_turn` validates on the way back. It +#: stays FIRST, and it is unchanged byte for byte; +#: * `openai-chat-v1` — the OpenAI-compatible chat-completions grammar: a +#: request of a model and a list of messages, answered by the content of +#: the first choice's message. It is what a hosted API and the usual local +#: server both speak, which the first member does not. Its arm is in +#: `doxbench_provider` alone, beside the first one's. #: #: THE VOCABULARY LIVES HERE, on the record that declares it, and #: `doxbench_provider` reads it from this module — so an unknown dialect is #: refused when an operator DECLARES the binding rather than when a turn fails. DIALECT_XFACTORY_PROMPT_V1 = "xfactory-prompt-v1" -DIALECTS: tuple[str, ...] = (DIALECT_XFACTORY_PROMPT_V1,) +DIALECT_OPENAI_CHAT_V1 = "openai-chat-v1" +DIALECTS: tuple[str, ...] = (DIALECT_XFACTORY_PROMPT_V1, DIALECT_OPENAI_CHAT_V1) #: The URL schemes a declared endpoint may carry. `http://` is permitted for the #: on-this-host proxy posture an operator may legitimately run; a scheme this diff --git a/src/opendox/doxbench_provider.py b/src/opendox/doxbench_provider.py index fcff458b..42e9ad9e 100644 --- a/src/opendox/doxbench_provider.py +++ b/src/opendox/doxbench_provider.py @@ -34,7 +34,10 @@ console process. Brett's ruling of 2026-08-08: the broker mints, doxBench calls, because a broker in the request path adds a hop to every turn and to every chunk of a streamed one. WHERE to call and WHAT GRAMMAR to speak are - the BINDING's — the broker's declaration emits neither, deliberately; + the BINDING's — the broker's declaration emits neither, deliberately. Each + grammar the binding may declare has one arm here (`_DIALECT_ARMS`): this + repository's own prompt grammar, and the OpenAI-compatible chat-completions + grammar (#1144 box 16.1); * EXPIRY is handled by the 2026-08-26 ruling: re-mint and retry ONCE, with the re-mint and the paid retry visibly recorded, and a second expiry inside one turn surfaces the standard refusal rather than buying a third call. The @@ -175,8 +178,9 @@ #: it is the record that validates it). Aliased rather than respelled so the two #: modules cannot drift into two vocabularies. An unknown dialect is refused #: when an operator DECLARES the binding — earlier than a mint, and earlier than -#: a paid call. +#: a paid call. Each member has exactly one ARM below (`_DIALECT_ARMS`). DIALECT_XFACTORY_PROMPT_V1 = binding_mod.DIALECT_XFACTORY_PROMPT_V1 +DIALECT_OPENAI_CHAT_V1 = binding_mod.DIALECT_OPENAI_CHAT_V1 DIALECTS: tuple[str, ...] = binding_mod.DIALECTS #: How long a broker invocation may take. A mint is a local process doing local @@ -616,13 +620,27 @@ def list_references(binding, *, runner=subprocess_broker_runner) -> list: # the provider transport # --------------------------------------------------------------------------- -#: The provider request's own field names, in the ONE dialect this client -#: speaks. Named constants rather than inline literals so the boundary test can -#: assert they exist only here. +#: The provider request's own field names, per dialect. Named constants rather +#: than inline literals so the boundary test can assert they exist only here. +#: `model` is the one field both grammars share. PROVIDER_REQUEST_MODEL_FIELD = "model" + +#: `xfactory-prompt-v1`: a model and a prompt in, an `assistant_prose` out. PROVIDER_REQUEST_PROMPT_FIELD = "prompt" PROVIDER_RESPONSE_PROSE_FIELD = "assistant_prose" +#: `openai-chat-v1` (#1144 box 16.1; plan 034 T078): the chat-completions +#: request, a model and a list of messages, and its answer, the content of the +#: first choice's message. The assembled prompt travels as ONE message in the +#: user role. Prompt assembly is on the other side of the port (D14), so this +#: arm carries the text it was given and composes no message of its own. +PROVIDER_REQUEST_MESSAGES_FIELD = "messages" +CHAT_MESSAGE_ROLE_FIELD = "role" +CHAT_MESSAGE_CONTENT_FIELD = "content" +CHAT_ROLE_USER = "user" +CHAT_RESPONSE_CHOICES_FIELD = "choices" +CHAT_RESPONSE_MESSAGE_FIELD = "message" + #: The status a provider returns when the presented token is no longer good. #: 401 only: a 403 is an authorization verdict about what the token may do, #: which re-minting the same scope cannot change, and retrying it would buy a @@ -630,6 +648,62 @@ def list_references(binding, *, runner=subprocess_broker_runner) -> list: PROVIDER_STATUS_TOKEN_EXPIRED = 401 +def _prompt_request(model: str, prompt: str) -> dict: + """`xfactory-prompt-v1`'s request, exactly as it has always been sent.""" + return {PROVIDER_REQUEST_MODEL_FIELD: model, + PROVIDER_REQUEST_PROMPT_FIELD: prompt} + + +def _prompt_answer(document: dict) -> str: + """`xfactory-prompt-v1`'s answer: its `assistant_prose`, a string.""" + prose = document.get(PROVIDER_RESPONSE_PROSE_FIELD) + if not isinstance(prose, str): + raise BrokerRefused(DIAG_PROVIDER_MALFORMED) + return prose + + +def _chat_request(model: str, prompt: str) -> dict: + """`openai-chat-v1`'s request: the model, and the prompt as one message in + the user role.""" + return {PROVIDER_REQUEST_MODEL_FIELD: model, + PROVIDER_REQUEST_MESSAGES_FIELD: [ + {CHAT_MESSAGE_ROLE_FIELD: CHAT_ROLE_USER, + CHAT_MESSAGE_CONTENT_FIELD: prompt}]} + + +def _chat_answer(document: dict) -> str: + """`openai-chat-v1`'s answer: `choices[0].message.content`, a string. + + Read at exactly that path and nowhere else. A body with no first choice, a + choice with no message, or a message whose content is not text (a tool-call + answer carries null there) is not an answer this seam can hand back as + prose. Each lands on the fixed `DIAG_PROVIDER_MALFORMED` that every other + unusable answer lands on. Nothing past the first choice is read: the + request asks for one.""" + choices = document.get(CHAT_RESPONSE_CHOICES_FIELD) + if not isinstance(choices, list) or not choices: + raise BrokerRefused(DIAG_PROVIDER_MALFORMED) + first = choices[0] + message = (first.get(CHAT_RESPONSE_MESSAGE_FIELD) + if isinstance(first, dict) else None) + content = (message.get(CHAT_MESSAGE_CONTENT_FIELD) + if isinstance(message, dict) else None) + if not isinstance(content, str): + raise BrokerRefused(DIAG_PROVIDER_MALFORMED) + return content + + +#: ONE ARM PER DECLARED DIALECT: the function that builds its request and the +#: function that reads its answer. The record's closed vocabulary +#: (`doxbench_binding.DIALECTS`) refuses any other member at declaration, and a +#: test holds this table's keys equal to that vocabulary, so a member cannot +#: join one without the other. +_DIALECT_ARMS: dict[str, tuple] = { + DIALECT_XFACTORY_PROMPT_V1: (_prompt_request, _prompt_answer), + DIALECT_OPENAI_CHAT_V1: (_chat_request, _chat_answer), +} + + def _post_to_provider(token: MintedToken, *, model_id: str, prompt: str, timeout: float, opener) -> str: """The ONE place a provider is contacted. Returns the assistant prose. @@ -638,6 +712,11 @@ def _post_to_provider(token: MintedToken, *, model_id: str, prompt: str, it is not in the URL (which a proxy logs), not in the body (which an error handler might echo), and not in this function's return value. + THE GRAMMAR IS THE BINDING'S DIALECT (#1144 box 16.1), which the token + carries from the binding. Its arm in `_DIALECT_ARMS` builds the request + body and reads the answer. The route, the header, the bound, the expiry + status and every refusal below are the same for both dialects. + THE ANSWER IS BOUNDED (PR #392 review note b). `response.read()` with no argument reads until the peer stops sending, which makes the memory of this process a function of what a declared endpoint chooses to send — and the @@ -645,10 +724,14 @@ def _post_to_provider(token: MintedToken, *, model_id: str, prompt: str, byte over `MAX_PROVIDER_ANSWER_BYTES` is read deliberately, so an answer that is exactly at the bound is still honoured while one past it is detected rather than truncated into a shorter document that would parse.""" - body = json.dumps({ - PROVIDER_REQUEST_MODEL_FIELD: model_id, - PROVIDER_REQUEST_PROMPT_FIELD: prompt, - }).encode("utf-8") + arm = _DIALECT_ARMS.get(token.dialect) + if arm is None: + raise AssertionError( + f"{token.dialect!r} is outside the declared dialect vocabulary " + f"{DIALECTS}; the binding refuses it at declaration, so no turn " + "can carry one") + build_request, read_answer = arm + body = json.dumps(build_request(model_id, prompt)).encode("utf-8") request = urllib.request.Request( # noqa: S310 - endpoint declared on the binding by its operator, carried on the minted token token.endpoint, data=body, method="POST") request.add_header("Content-Type", "application/json") @@ -680,10 +763,7 @@ def _post_to_provider(token: MintedToken, *, model_id: str, prompt: str, raise BrokerRefused(DIAG_PROVIDER_MALFORMED) from error if not isinstance(document, dict): raise BrokerRefused(DIAG_PROVIDER_MALFORMED) - prose = document.get(PROVIDER_RESPONSE_PROSE_FIELD) - if not isinstance(prose, str): - raise BrokerRefused(DIAG_PROVIDER_MALFORMED) - return prose + return read_answer(document) # --------------------------------------------------------------------------- diff --git a/tests/test_model_provider_broker.py b/tests/test_model_provider_broker.py index 4a5fbaea..9fe6099e 100644 --- a/tests/test_model_provider_broker.py +++ b/tests/test_model_provider_broker.py @@ -21,6 +21,9 @@ redacted refusal `doxbench_model.dispatch_turn` already defines, and the UNCONFIGURED posture is byte-for-byte what it was before this change. +A SIXTH LAYER, (f), holds #1144 Group 16's binding and provider boxes (plan +034 phase 3, slice P3-B). 16.1 is the OpenAI-compatible dialect (T078). + THE FAKE BROKER SPEAKS THE DECLARED CONTRACT (task 2.6). It was this repository's own invented stdin/stdout protocol until the reconciliation, which meant every test here agreed with a broker that does not exist. It now takes the @@ -40,6 +43,7 @@ from __future__ import annotations +import contextlib import dataclasses import http.server import io @@ -145,8 +149,12 @@ def test_the_dialect_vocabulary_is_closed_and_refuses_at_declaration(): there; openProfiler's declaration emits no dialect at all, so the fact is the BINDING's and the refusal happens when an operator DECLARES one — before any broker is invoked and long before a paid call. Closed, still: an unknown - grammar refuses rather than being guessed at.""" - assert binding_mod.DIALECTS == ("xfactory-prompt-v1",) + grammar refuses rather than being guessed at. + + TWO MEMBERS since #1144 box 16.1 (plan 034 T078), and the order is pinned: + the prompt grammar stays first, and the OpenAI-compatible chat grammar + joins after it. The refusal below is the same refusal it always was.""" + assert binding_mod.DIALECTS == ("xfactory-prompt-v1", "openai-chat-v1") assert provider_mod.DIALECTS is binding_mod.DIALECTS, \ "one vocabulary, read from the record that declares it" with pytest.raises(binding_mod.BindingRefused) as caught: @@ -866,9 +874,9 @@ def _expired_error(): def _port(tmp_path, *outcomes, expires=None, notice=None, clock=time.time, - endpoint=ENDPOINT): + endpoint=ENDPOINT, dialect=binding_mod.DIALECT_XFACTORY_PROMPT_V1): script = _write_broker(tmp_path, expires=expires) - binding = _broker_binding(script, endpoint=endpoint) + binding = _broker_binding(script, endpoint=endpoint, dialect=dialect) opener = _Opener(*outcomes) port = provider_mod.BrokeredProviderPort( binding, install_mod.brokered_catalog(binding), @@ -1336,3 +1344,236 @@ def test_the_subprocess_runner_never_uses_a_shell(tmp_path): assert "shell=True" not in source assert "os.system" not in source assert subprocess.Popen is subprocess.Popen # the module spawns, nothing else + + +# =========================================================================== +# (f) CHAT'S MODEL CONFIGURATION (#1144 Group 16; plan 034 phase 3, P3-B) +# =========================================================================== +# +# 16.1, the OpenAI-compatible dialect (T078). `openai-chat-v1` is the second +# `DIALECTS` member. Its request is the chat-completions grammar (`model`, +# `messages`), and its answer is read at `choices[0].message.content`. Both are +# spoken by one arm in `doxbench_provider`, beside the prompt grammar's arm. + +OPENAI_CHAT = binding_mod.DIALECT_OPENAI_CHAT_V1 + + +def _chat_completion(content="the chat answer"): + """A chat-completions answer in that grammar's own shape. The keys around + `choices` are what a real server sends, and nothing here reads them.""" + return {"id": "chatcmpl-stand-in", "object": "chat.completion", + "model": "stand-in-model", + "choices": [{"index": 0, "finish_reason": "stop", + "message": {"role": "assistant", + "content": content}}]} + + +def test_f16_1_the_openai_compatible_dialect_is_declared(): + """F16.1's dialect assertion, as #1144 writes it: + `assert "openai-chat-v1" in b.DIALECTS`. A binding may declare it.""" + assert "openai-chat-v1" in binding_mod.DIALECTS, ( + f"no OpenAI-compatible dialect: {binding_mod.DIALECTS}") + assert OPENAI_CHAT == "openai-chat-v1" + assert _binding(dialect=OPENAI_CHAT).dialect == OPENAI_CHAT + assert provider_mod.DIALECT_OPENAI_CHAT_V1 is OPENAI_CHAT, \ + "one spelling, read from the record that declares it" + + +def test_every_declared_dialect_has_exactly_one_arm_in_the_provider_module(): + """A member cannot join the vocabulary without an arm, or an arm exist + for a member the record would refuse.""" + assert set(provider_mod._DIALECT_ARMS) == set(binding_mod.DIALECTS) + + +def test_a_chat_turn_speaks_the_chat_completions_grammar(tmp_path): + port, opener = _port(tmp_path, _chat_completion("the answer"), + dialect=OPENAI_CHAT) + assert port.dispatch(_Envelope()) == {"assistant_prose": "the answer", + "proposals": []} + request = opener.requests[0] + assert request.get_method() == "POST" + assert request.get_full_url() == ENDPOINT + assert json.loads(request.data.decode("utf-8")) == { + "model": "openprofiler-demo", + "messages": [{"role": "user", "content": "assembled prompt"}]} + assert request.get_header("Content-type") == "application/json" + # the token travels in the header, exactly as it does for the prompt grammar + assert request.get_header("Authorization") == f"Bearer {SENTINEL_TOKEN}" + assert SENTINEL_TOKEN not in request.get_full_url() + assert SENTINEL_TOKEN not in request.data.decode("utf-8") + + +def test_the_prompt_dialect_is_unchanged_byte_for_byte(tmp_path): + """The first member's request is the bytes it always was: the arm table + moved the code, and nothing it sends.""" + port, opener = _port(tmp_path, {"assistant_prose": "a"}) + assert port.dispatch(_Envelope())["assistant_prose"] == "a" + assert opener.requests[0].data == json.dumps( + {"model": "openprofiler-demo", "prompt": "assembled prompt"} + ).encode("utf-8") + + +@pytest.mark.parametrize("answer", [ + {}, + {"choices": []}, + {"choices": "not a list"}, + {"choices": ["not an object"]}, + {"choices": [{}]}, + {"choices": [{"message": "not an object"}]}, + {"choices": [{"message": {"role": "assistant"}}]}, + {"choices": [{"message": {"role": "assistant", "content": None}}]}, + {"choices": [{"message": {"role": "assistant", "content": 7}}]}, + {"assistant_prose": "the prompt grammar's answer, not this one's"}, +], ids=["empty", "no-choice", "choices-not-a-list", "choice-not-an-object", + "no-message", "message-not-an-object", "no-content", "null-content", + "content-not-text", "the-other-grammar"]) +def test_a_chat_answer_off_the_declared_path_is_malformed(tmp_path, answer): + port, _opener = _port(tmp_path, answer, dialect=OPENAI_CHAT) + with pytest.raises(provider_mod.BrokerRefused) as caught: + port.dispatch(_Envelope()) + assert caught.value.diagnostic == provider_mod.DIAG_PROVIDER_MALFORMED + + +def test_a_chat_shaped_answer_is_not_the_prompt_grammars_answer(tmp_path): + """Each arm reads its own grammar and no other.""" + port, _opener = _port(tmp_path, _chat_completion()) + with pytest.raises(provider_mod.BrokerRefused) as caught: + port.dispatch(_Envelope()) + assert caught.value.diagnostic == provider_mod.DIAG_PROVIDER_MALFORMED + + +def test_only_the_first_choice_is_read(tmp_path): + answer = _chat_completion("first") + answer["choices"].append({"index": 1, "finish_reason": "stop", + "message": {"role": "assistant", + "content": "second"}}) + port, _opener = _port(tmp_path, answer, dialect=OPENAI_CHAT) + assert port.dispatch(_Envelope())["assistant_prose"] == "first" + + +def test_the_expiry_ruling_holds_for_the_chat_grammar(tmp_path): + """The 2026-08-26 ruling is the port's, not a dialect's: a mid-turn expiry + re-mints and retries once, visibly, in either grammar.""" + printed: list[str] = [] + port, opener = _port(tmp_path, _expired_error(), + _chat_completion("the retried answer"), + notice=printed.append, dialect=OPENAI_CHAT) + assert port.dispatch(_Envelope())["assistant_prose"] == "the retried answer" + assert len(opener.requests) == 2, "exactly one paid retry" + assert [event.reason for event in port.ledger] == [ + provider_mod.REASON_FIRST_MINT, + provider_mod.REASON_EXPIRY_REMINT, + provider_mod.REASON_PAID_RETRY, + ] + assert printed and "re-minted once and retried" in printed[0] + + +def test_the_answer_bound_holds_for_the_chat_grammar(tmp_path): + bound = provider_mod.MAX_PROVIDER_ANSWER_BYTES + oversize = json.dumps(_chat_completion("x" * bound)).encode("utf-8") + port, _opener = _port(tmp_path, oversize, dialect=OPENAI_CHAT) + with pytest.raises(provider_mod.BrokerRefused) as caught: + port.dispatch(_Envelope()) + assert caught.value.diagnostic == provider_mod.DIAG_PROVIDER_MALFORMED + + +def test_a_chat_provider_refusal_lands_on_the_fixed_sentence(tmp_path): + port, _opener = _port( + tmp_path, + urllib.error.HTTPError(ENDPOINT, 400, "Bad Request", {}, + io.BytesIO(b'{"error":{"message":"leaky"}}')), + dialect=OPENAI_CHAT) + with pytest.raises(provider_mod.BrokerRefused) as caught: + port.dispatch(_Envelope()) + assert caught.value.diagnostic == provider_mod.DIAG_PROVIDER_REFUSED + assert "leaky" not in str(caught.value) + + +@contextlib.contextmanager +def _stand_in_provider(handler_class): + """A stand-in provider on loopback for the length of one test. It yields + the server's base URL, and it is shut down and joined however the test + ends.""" + server = http.server.ThreadingHTTPServer(("127.0.0.1", 0), handler_class) + thread = threading.Thread(target=server.serve_forever, daemon=True) + thread.start() + try: + host, prt = server.server_address[:2] + yield f"http://{host}:{prt}" + finally: + server.shutdown() + server.server_close() + thread.join(timeout=5) + + +def _answer_json(handler, document) -> None: + """Answer one stand-in request with `document` as a JSON body.""" + payload = json.dumps(document).encode("utf-8") + handler.send_response(200) + handler.send_header("Content-Type", "application/json") + handler.send_header("Content-Length", str(len(payload))) + handler.end_headers() + handler.wfile.write(payload) + + +class _ChatCompletionsHandler(http.server.BaseHTTPRequestHandler): + """A stand-in OpenAI-compatible server on loopback. It records each + request and answers in the chat-completions grammar.""" + + seen: dict = {} + + def do_POST(self): # noqa: N802 - BaseHTTPRequestHandler's own spelling + length = int(self.headers.get("Content-Length", "0")) + _ChatCompletionsHandler.seen = { + "path": self.path, + "authorization": self.headers.get("Authorization"), + "content_type": self.headers.get("Content-Type"), + "body": json.loads(self.rfile.read(length).decode("utf-8")), + } + _answer_json(self, _chat_completion("answered in the chat grammar")) + + def log_message(self, *_args): + return + + +def test_a_chat_turn_reaches_a_stand_in_chat_completions_server(tmp_path): + """The real `urllib` path, in the chat grammar: a stand-in server on + loopback receives the request at the binding's declared endpoint, in that + grammar, with the token in the authorization header and nowhere else.""" + with _stand_in_provider(_ChatCompletionsHandler) as base: + binding = _broker_binding(_write_broker(tmp_path), + endpoint=f"{base}/v1/chat/completions", + dialect=OPENAI_CHAT) + port = provider_mod.BrokeredProviderPort( + binding, install_mod.brokered_catalog(binding), + notice=lambda _text: None) + assert port.dispatch(_Envelope()) == { + "assistant_prose": "answered in the chat grammar", + "proposals": []} + + seen = _ChatCompletionsHandler.seen + assert seen["path"] == "/v1/chat/completions" + assert seen["authorization"] == f"Bearer {SENTINEL_TOKEN}" + assert seen["content_type"] == "application/json" + assert seen["body"] == { + "model": "openprofiler-demo", + "messages": [{"role": "user", "content": "assembled prompt"}]} + assert SENTINEL_TOKEN not in json.dumps(seen["body"]) + + +def test_the_cli_declares_a_chat_binding(tmp_path, capsys): + """The operator door offers the dialect, because its choices are read from + the record's vocabulary rather than respelled.""" + checkout = tmp_path / "checkout" + checkout.mkdir() + args = cli_mod.build_parser().parse_args([ + "model-binding", "add", "--repo-root", str(checkout), + "--id", "local-chat", "--label", "Local chat", "--provider", "local", + "--credential-ref", FAKE_REFERENCE, "--auth-kind", "api_key", + "--credential-approver", "brett@opensoft.one", + "--endpoint", "http://127.0.0.1:9/v1/chat/completions", + "--dialect", OPENAI_CHAT, "--", "openprofiler-broker"]) + assert args.func(args) == 0 + capsys.readouterr() + store = binding_mod.BindingStore(binding_mod.bindings_path(checkout)) + assert store.get("local-chat").dialect == OPENAI_CHAT From 7c83c2cd0bbabc8a6005ee524a8ed018f9c9c807 Mon Sep 17 00:00:00 2001 From: Brett Heap <1513478+brettheap@users.noreply.github.com> Date: Mon, 28 Sep 2026 18:29:46 +0000 Subject: [PATCH 02/20] T079: 16.2, a `model` field the provider receives (plan 034) The binding record gains `model`, the model name the provider receives as the request's model, in either dialect. It sits after `dialect`, with the route it belongs to, so BINDING_FIELDS grows from nine to ten, and still no field can hold a secret. `model-binding add|edit --model` sets it, and `model-binding list` shows it. A binding that declares no model keeps the meaning it had: the request names the catalog handle, the binding's `id`, byte for byte. So `model` is keyword-only and defaults to None, and a stored record may leave it out (OPTIONAL_BINDING_FIELDS). Every construction written before the field existed builds the binding it built, and every nine-field document reads. The console's intake route in serve_workbench.py builds a binding without a model, and it keeps working unedited. The catalog handle stays the binding's id, and `model` is not an argv placeholder. Falsifier: F16.1's field assertion, `"model" in b.BINDING_FIELDS`. Ruled: R1Q22 (a), openxFactory#656 comment 5817152735. Drafted ahead of T063 under Brett's phase-3 word ("Only the independent ones"). It does not land before T063. Arc: neutral-product-standalone-operability Lane: openxfactory-4 (openXfactory-4-openDox_extraction) Co-Authored-By: Claude Opus 5.5 (1M context) --- src/opendox/cli_model_binding.py | 21 ++- src/opendox/doxbench_binding.py | 47 +++++-- src/opendox/doxbench_provider.py | 26 ++-- tests/test_model_provider_broker.py | 208 +++++++++++++++++++++++++++- 4 files changed, 280 insertions(+), 22 deletions(-) diff --git a/src/opendox/cli_model_binding.py b/src/opendox/cli_model_binding.py index 2331034d..f5c15ad9 100644 --- a/src/opendox/cli_model_binding.py +++ b/src/opendox/cli_model_binding.py @@ -50,7 +50,14 @@ def _declared_binding(args: argparse.Namespace) -> "binding_mod.ModelProviderBin id=args.id, label=args.label, provider=args.provider, credential_ref=args.credential_ref, auth_kind=args.auth_kind, approved_by=args.approved_by, endpoint=args.endpoint, - dialect=args.dialect, broker_argv=tuple(args.broker_argv)) + dialect=args.dialect, model=args.model, + broker_argv=tuple(args.broker_argv)) + + +#: What `list` prints for a binding that declares no model (#1144 box 16.2). +#: The request then names the binding's id, as every request did before the +#: field existed, and the operator reading the list should see that. +NO_MODEL_DECLARED = "(none declared: the request names this binding's id)" def cmd_model_binding_list(args: argparse.Namespace) -> int: @@ -77,6 +84,9 @@ def cmd_model_binding_list(args: argparse.Namespace) -> int: print(f" credential ref {record['credential_ref']}") print(f" endpoint {record['endpoint']}") print(f" dialect {record['dialect']}") + model = record["model"] + print(f" model " + f"{model if model is not None else NO_MODEL_DECLARED}") print(f" broker argv {record['broker_argv']}") print(f" custody {record['credential_custody']}") return 0 @@ -203,6 +213,15 @@ def _add_binding_declaration_args(parser: argparse.ArgumentParser) -> None: parser.add_argument("--dialect", required=True, choices=list(binding_mod.DIALECTS), help="the request grammar that endpoint speaks") + # THE MODEL THE PROVIDER RECEIVES (#1144 box 16.2), the route's third fact. + # OPTIONAL, and that keeps a binding declared without it meaning what it + # always meant: the request names the binding's id. `edit` replaces the + # whole binding, as it always has, so an edit that omits `--model` declares + # none. + parser.add_argument("--model", default=None, + help="the model name the provider receives in each " + "request (default: none declared, and the " + "request names this binding's id)") # A POSITIONAL, taken after a bare `--`, and that is the fix for a real # trap rather than a style choice: a broker invocation is full of # option-shaped members (`--binding`, `--ref`), and as a flag's value they diff --git a/src/opendox/doxbench_binding.py b/src/opendox/doxbench_binding.py index 3b1e2131..c76ca89a 100644 --- a/src/opendox/doxbench_binding.py +++ b/src/opendox/doxbench_binding.py @@ -31,7 +31,9 @@ would then be accountable for. So provider routing is the CONSUMER's fact, and the consumer's declared record is where a fact the consumer owns belongs. Declaring a route is not holding a transport: nothing here opens a socket, and -the module still names no provider host of its own. +the module still names no provider host of its own. #1144 box 16.2 gave the +route a third fact, `model`: the model name the provider receives. It is the +consumer's fact for the same reason. THE BROKER INVOCATION IS DECLARED, NOT WRITTEN INTO CODE — the program and its fixed leading arguments. `broker_argv` is the BASE invocation and names no @@ -133,9 +135,14 @@ #: `provider` and `approved_by` are REQUIRED flags of the declared `intake` #: (`--provider`, `--approved-by`; the second because `credential-contracts` #: holds that a grant without an approver is invalid), and `endpoint`/`dialect` -#: are the provider route the mint answer deliberately does not carry. STILL NO -#: SECRET FIELD: nine fields, and the absence of a tenth is the same point the -#: absence of a sixth was. +#: are the provider route the mint answer deliberately does not carry. +#: +#: AND FROM NINE TO TEN BY #1144 box 16.2 (plan 034 T079): `model`, the model +#: name the provider receives as the request's model. It sits with the route +#: it belongs to, after `dialect`. Before it, the request named the catalog +#: handle, which is this binding's `id`, so no provider model could be named. +#: STILL NO SECRET FIELD: ten fields, and the absence of an eleventh is the +#: same point the absence of a sixth was. BINDING_FIELDS: tuple[str, ...] = ( "id", "label", @@ -145,16 +152,25 @@ "approved_by", "endpoint", "dialect", + "model", "broker_argv", ) +#: The one field a stored record may leave out. A record without `model` was +#: declared before the field existed, and it keeps the meaning it had: the +#: request names the catalog handle, this binding's `id`. Every other field is +#: required, as it always was. +OPTIONAL_BINDING_FIELDS: tuple[str, ...] = ("model",) + #: The CLOSED placeholder vocabulary an argv template may name. Every member is #: a field of the binding itself, which is the property that matters: a template #: can only ever be filled with facts the binding already discloses, so no #: substitution can smuggle a value the record does not carry. A template naming #: anything outside this set is refused at construction rather than at #: execution — an operator finds out when they declare the binding, not when a -#: turn fails. +#: turn fails. `model` is not a member: a broker's invocation is about custody, +#: never about which model a turn asks for, and an undeclared model has no +#: value to fill a placeholder with. ARGV_PLACEHOLDERS: tuple[str, ...] = ( "binding_id", "label", "provider", "credential_ref", "auth_kind", "approved_by", "endpoint", "dialect") @@ -195,12 +211,19 @@ def _require_non_blank_str(field: str, value: object) -> str: class ModelProviderBinding: """ONE model provider, as settings hold it. - Nine fields, and the absence of a tenth is the point (see the module + Ten fields, and the absence of an eleventh is the point (see the module docstring). `broker_argv` is the DECLARED BASE invocation as a tuple of argv members — argv, never a shell string, so no operator's label and no credential reference can ever be read as shell syntax. It names the program and its fixed leading arguments and NOT the operation: the operation is a declared subcommand `doxbench_provider` appends. + + `model` (#1144 box 16.2) is the model name the provider receives as the + request's model. It is KEYWORD-ONLY and defaults to None, so every + construction written before it existed still builds the binding it built. + That binding keeps its old meaning: with no model declared, the request + names the catalog handle, which is the binding's `id`, exactly as before. + A declared model is a non-blank string. """ id: str @@ -211,12 +234,15 @@ class ModelProviderBinding: approved_by: str endpoint: str dialect: str + model: str | None = dataclasses.field(default=None, kw_only=True) broker_argv: tuple[str, ...] def __post_init__(self) -> None: for field in ("id", "label", "provider", "credential_ref", "auth_kind", "approved_by", "endpoint", "dialect"): _require_non_blank_str(field, getattr(self, field)) + if self.model is not None: + _require_non_blank_str("model", self.model) if self.auth_kind not in AUTH_KINDS: raise BindingRefused( f"auth_kind {self.auth_kind!r} is outside the closed " @@ -259,7 +285,8 @@ def __post_init__(self) -> None: def as_record(self) -> dict: """The STORED record: the record kind, then exactly ``BINDING_FIELDS`` in order. `broker_argv` becomes a list because that is what YAML round - trips; nothing else changes shape.""" + trips. An undeclared `model` is written as null, so every stored + record carries all ten keys; nothing else changes shape.""" return { "kind": BINDING_KIND, "id": self.id, @@ -270,6 +297,7 @@ def as_record(self) -> dict: "approved_by": self.approved_by, "endpoint": self.endpoint, "dialect": self.dialect, + "model": self.model, "broker_argv": list(self.broker_argv), } @@ -334,7 +362,9 @@ def from_record(cls, record: object) -> "ModelProviderBinding": raise BindingRefused( f"a binding record declares kind {declared_kind!r}, not " f"{BINDING_KIND!r}") - missing = [field for field in BINDING_FIELDS if field not in record] + missing = [field for field in BINDING_FIELDS + if field not in record + and field not in OPTIONAL_BINDING_FIELDS] if missing: raise BindingRefused( f"a binding record is missing {missing}") @@ -347,6 +377,7 @@ def from_record(cls, record: object) -> "ModelProviderBinding": approved_by=record["approved_by"], endpoint=record["endpoint"], dialect=record["dialect"], + model=record.get("model"), broker_argv=record["broker_argv"], ) diff --git a/src/opendox/doxbench_provider.py b/src/opendox/doxbench_provider.py index 42e9ad9e..9317a995 100644 --- a/src/opendox/doxbench_provider.py +++ b/src/opendox/doxbench_provider.py @@ -704,10 +704,14 @@ def _chat_answer(document: dict) -> str: } -def _post_to_provider(token: MintedToken, *, model_id: str, prompt: str, +def _post_to_provider(token: MintedToken, *, model: str, prompt: str, timeout: float, opener) -> str: """The ONE place a provider is contacted. Returns the assistant prose. + `model` is the model name the request carries, which the port chose (the + binding's declared `model`, or the catalog handle for a binding that + declares none). + The token travels in the request's authorization header and nowhere else; it is not in the URL (which a proxy logs), not in the body (which an error handler might echo), and not in this function's return value. @@ -731,7 +735,7 @@ def _post_to_provider(token: MintedToken, *, model_id: str, prompt: str, f"{DIALECTS}; the binding refuses it at declaration, so no turn " "can carry one") build_request, read_answer = arm - body = json.dumps(build_request(model_id, prompt)).encode("utf-8") + body = json.dumps(build_request(model, prompt)).encode("utf-8") request = urllib.request.Request( # noqa: S310 - endpoint declared on the binding by its operator, carried on the minted token token.endpoint, data=body, method="POST") request.add_header("Content-Type", "application/json") @@ -909,15 +913,21 @@ def dispatch(self, prompt_envelope: object) -> object: unrelated issuances. The expired mint's reference is read off the token this turn is holding and lives no longer than the turn; * a SECOND expiry inside the same turn raises the standard refusal. - No third call is bought.""" - model_id = getattr(prompt_envelope, "model_id", None) - if not isinstance(model_id, str) or not model_id: + No third call is bought. + + THE REQUEST'S MODEL IS THE BINDING'S DECLARED `model` (#1144 box + 16.2). A binding that declares none sends the catalog handle, which is + what every request sent before the field existed, byte for byte.""" + handle = getattr(prompt_envelope, "model_id", None) + if not isinstance(handle, str) or not handle: entries = self._declared_catalog.entries - model_id = entries[0].model_id if entries else "" + handle = entries[0].model_id if entries else "" + declared_model = self._binding.model + model = declared_model if declared_model is not None else handle prompt = bridge_mod.render_prompt_message(prompt_envelope) token = self._current_token(REASON_FIRST_MINT) try: - prose = _post_to_provider(token, model_id=model_id, prompt=prompt, + prose = _post_to_provider(token, model=model, prompt=prompt, timeout=self._timeout_seconds, opener=self._opener) except _TokenExpired: @@ -932,7 +942,7 @@ def dispatch(self, prompt_envelope: object) -> object: self._record(REASON_PAID_RETRY) try: prose = _post_to_provider( - token, model_id=model_id, prompt=prompt, + token, model=model, prompt=prompt, timeout=self._timeout_seconds, opener=self._opener) except _TokenExpired: self._forget_token() diff --git a/tests/test_model_provider_broker.py b/tests/test_model_provider_broker.py index 9fe6099e..82bf9a11 100644 --- a/tests/test_model_provider_broker.py +++ b/tests/test_model_provider_broker.py @@ -22,7 +22,8 @@ UNCONFIGURED posture is byte-for-byte what it was before this change. A SIXTH LAYER, (f), holds #1144 Group 16's binding and provider boxes (plan -034 phase 3, slice P3-B). 16.1 is the OpenAI-compatible dialect (T078). +034 phase 3, slice P3-B). 16.1 is the OpenAI-compatible dialect (T078), and +16.2 is the model name the provider receives (T079). THE FAKE BROKER SPEAKS THE DECLARED CONTRACT (task 2.6). It was this repository's own invented stdin/stdout protocol until the reconciliation, which @@ -120,8 +121,9 @@ def test_the_binding_declares_exactly_the_fields_the_seam_needs(): "secret", "api_key", "token", "credential", "value", "password"]) def test_no_secret_field_exists_in_the_shape_to_populate(secret_field): """NOT OPTIONAL — ABSENT. The dataclass is slotted and frozen, so a secret - cannot be passed in and cannot be attached afterwards. Nine fields now - rather than five, and the absence of a tenth is the same claim.""" + cannot be passed in and cannot be attached afterwards. Ten fields now + rather than five (#1144 box 16.2 added `model`), and the absence of an + eleventh is the same claim.""" with pytest.raises(TypeError): _binding(**{secret_field: SENTINEL_CREDENTIAL}) binding = _binding() @@ -874,9 +876,11 @@ def _expired_error(): def _port(tmp_path, *outcomes, expires=None, notice=None, clock=time.time, - endpoint=ENDPOINT, dialect=binding_mod.DIALECT_XFACTORY_PROMPT_V1): + endpoint=ENDPOINT, dialect=binding_mod.DIALECT_XFACTORY_PROMPT_V1, + model=None): script = _write_broker(tmp_path, expires=expires) - binding = _broker_binding(script, endpoint=endpoint, dialect=dialect) + binding = _broker_binding(script, endpoint=endpoint, dialect=dialect, + model=model) opener = _Opener(*outcomes) port = provider_mod.BrokeredProviderPort( binding, install_mod.brokered_catalog(binding), @@ -1577,3 +1581,197 @@ def test_the_cli_declares_a_chat_binding(tmp_path, capsys): capsys.readouterr() store = binding_mod.BindingStore(binding_mod.bindings_path(checkout)) assert store.get("local-chat").dialect == OPENAI_CHAT + + +# 16.2, the model name the provider receives (T079). The record gains `model`, +# sent as the request's model and set by `model-binding add|edit --model`. The +# field list grows from nine to ten, and still no field can hold a secret. A +# binding that declares no model sends the catalog handle, its `id`, exactly as +# every request did before the field existed. + +DECLARED_MODEL = "stand-in-model-7b" + + +def test_f16_1_the_record_names_a_model(): + """F16.1's field assertion, as #1144 writes it: + `assert "model" in b.BINDING_FIELDS`. Ten fields, in their declared order, + with `model` beside the route it belongs to.""" + assert "model" in binding_mod.BINDING_FIELDS, ( + f"the record names no model: {binding_mod.BINDING_FIELDS}") + assert binding_mod.BINDING_FIELDS == ( + "id", "label", "provider", "credential_ref", "auth_kind", + "approved_by", "endpoint", "dialect", "model", "broker_argv") + assert binding_mod.OPTIONAL_BINDING_FIELDS == ("model",) + + +def test_the_model_is_keyword_only_and_undeclared_by_default(): + """Every construction written before the field existed builds the + binding it built, which declares no model.""" + import inspect + parameter = inspect.signature( + binding_mod.ModelProviderBinding).parameters["model"] + assert parameter.kind is inspect.Parameter.KEYWORD_ONLY + assert parameter.default is None + assert _binding().model is None + assert _binding(model=DECLARED_MODEL).model == DECLARED_MODEL + + +@pytest.mark.parametrize("bad", ["", " ", 7, ["a-model"]]) +def test_a_declared_model_is_non_blank_text(bad): + with pytest.raises(binding_mod.BindingRefused): + _binding(model=bad) + + +@pytest.mark.parametrize("dialect,answer,grammar_key", [ + (binding_mod.DIALECT_XFACTORY_PROMPT_V1, {"assistant_prose": "a"}, "prompt"), + (OPENAI_CHAT, _chat_completion("a"), "messages"), +]) +def test_the_declared_model_is_what_the_provider_receives(tmp_path, dialect, + answer, grammar_key): + port, opener = _port(tmp_path, answer, dialect=dialect, + model=DECLARED_MODEL) + assert port.dispatch(_Envelope())["assistant_prose"] == "a" + body = json.loads(opener.requests[0].data.decode("utf-8")) + assert body["model"] == DECLARED_MODEL + assert set(body) == {"model", grammar_key} + + +@pytest.mark.parametrize("dialect,answer", [ + (binding_mod.DIALECT_XFACTORY_PROMPT_V1, {"assistant_prose": "a"}), + (OPENAI_CHAT, _chat_completion("a")), +]) +def test_a_binding_with_no_model_still_sends_the_catalog_handle(tmp_path, + dialect, + answer): + """What every request sent before #1144 box 16.2, byte for byte.""" + port, opener = _port(tmp_path, answer, dialect=dialect) + port.dispatch(_Envelope()) + assert json.loads(opener.requests[0].data.decode("utf-8"))["model"] == \ + "openprofiler-demo" + + +def test_the_catalog_handle_stays_the_bindings_id(tmp_path): + """The model is what the PROVIDER receives. The menu's handle is still the + binding's id, so a chosen entry still resolves back to its binding.""" + binding = _binding(model=DECLARED_MODEL) + entries = install_mod.brokered_catalog(binding).entries + assert [entry.model_id for entry in entries] == [binding.id] + + +def test_a_declared_model_survives_the_expiry_retry(tmp_path): + port, opener = _port(tmp_path, _expired_error(), _chat_completion("b"), + dialect=OPENAI_CHAT, model=DECLARED_MODEL) + assert port.dispatch(_Envelope())["assistant_prose"] == "b" + assert [json.loads(request.data.decode("utf-8"))["model"] + for request in opener.requests] == [DECLARED_MODEL] * 2 + + +def test_the_model_is_not_an_argv_placeholder(): + """A broker's invocation is about custody, never about the model a turn + asks for, so the closed placeholder vocabulary does not grow.""" + assert "model" not in binding_mod.ARGV_PLACEHOLDERS + with pytest.raises(binding_mod.BindingRefused): + _binding(model=DECLARED_MODEL, + broker_argv=("openprofiler-broker", "--for", "{model}")) + + +def test_a_stored_record_carries_its_model_and_round_trips(tmp_path): + store = _store(tmp_path) + store.add(_binding(model=DECLARED_MODEL)) + store.add(_binding(id="undeclared", label="No model")) + import yaml + document = yaml.safe_load(store.path.read_text(encoding="utf-8")) + first, second = document["bindings"] + assert list(first) == ["kind", *binding_mod.BINDING_FIELDS] + assert first["model"] == DECLARED_MODEL + assert second["model"] is None, "an undeclared model is written as null" + assert store.get("openprofiler-demo").model == DECLARED_MODEL + assert store.get("undeclared").model is None + assert store.read_back()["bindings"][0]["model"] == DECLARED_MODEL + + +def test_a_record_declared_before_the_field_existed_still_reads(tmp_path): + """A nine-field record, as every stored document held until #1144 box + 16.2, reads as a binding that declares no model.""" + record = _binding().as_record() + del record["model"] + assert set(record) == {"kind", *binding_mod.BINDING_FIELDS} - {"model"} + path = tmp_path / "bindings.yaml" + path.write_text(json.dumps({"schema_version": 1, + "kind": binding_mod.BINDINGS_KIND, + "bindings": [record]}), encoding="utf-8") + (binding,) = binding_mod.BindingStore(path).list() + assert binding.model is None + assert binding == _binding() + + +def test_a_record_missing_a_required_field_still_refuses(): + """`model` is the one field a record may leave out, and only that one.""" + for field in binding_mod.BINDING_FIELDS: + if field in binding_mod.OPTIONAL_BINDING_FIELDS: + continue + record = _binding().as_record() + del record[field] + with pytest.raises(binding_mod.BindingRefused) as caught: + binding_mod.ModelProviderBinding.from_record(record) + assert field in str(caught.value) + + +def test_the_cli_sets_the_model_on_add_and_edit(tmp_path, capsys): + checkout = tmp_path / "checkout" + checkout.mkdir() + parser = cli_mod.build_parser() + + def run(*argv) -> int: + args = parser.parse_args(list(argv)) + return args.func(args) + + root = ["--repo-root", str(checkout)] + declaration = ["--id", "local-chat", "--label", "Local chat", + "--provider", "local", "--credential-ref", FAKE_REFERENCE, + "--auth-kind", "api_key", + "--credential-approver", "brett@opensoft.one", + "--endpoint", "http://127.0.0.1:9/v1/chat/completions", + "--dialect", OPENAI_CHAT] + store = binding_mod.BindingStore(binding_mod.bindings_path(checkout)) + + assert run("model-binding", "add", *root, *declaration, + "--model", DECLARED_MODEL, "--", "openprofiler-broker") == 0 + capsys.readouterr() + assert store.get("local-chat").model == DECLARED_MODEL + assert run("model-binding", "list", *root) == 0 + assert f"model {DECLARED_MODEL}" in capsys.readouterr().out + + assert run("model-binding", "edit", *root, *declaration, + "--model", "another-model", "--", "openprofiler-broker") == 0 + capsys.readouterr() + assert store.get("local-chat").model == "another-model" + + # `edit` replaces the whole binding, so an edit without `--model` declares + # none, and the list says what the request then names + assert run("model-binding", "edit", *root, *declaration, + "--", "openprofiler-broker") == 0 + capsys.readouterr() + assert store.get("local-chat").model is None + assert run("model-binding", "list", *root) == 0 + from opendox import cli_model_binding as cmb + assert cmb.NO_MODEL_DECLARED in capsys.readouterr().out + + # a blank model refuses THROUGH THE VERB, not only through the record + assert run("model-binding", "edit", *root, *declaration, + "--model", " ", "--", "openprofiler-broker") == 1 + capsys.readouterr() + assert store.get("local-chat").model is None + + +def test_a_stand_in_chat_server_receives_the_declared_model(tmp_path): + with _stand_in_provider(_ChatCompletionsHandler) as base: + binding = _broker_binding( + _write_broker(tmp_path), endpoint=f"{base}/v1/chat/completions", + dialect=OPENAI_CHAT, model=DECLARED_MODEL) + provider_mod.BrokeredProviderPort( + binding, install_mod.brokered_catalog(binding), + notice=lambda _text: None).dispatch(_Envelope()) + assert _ChatCompletionsHandler.seen["body"] == { + "model": DECLARED_MODEL, + "messages": [{"role": "user", "content": "assembled prompt"}]} From e9ef9514a5a164812d5d11a067334b69a384f327 Mon Sep 17 00:00:00 2001 From: Brett Heap <1513478+brettheap@users.noreply.github.com> Date: Mon, 28 Sep 2026 18:49:03 +0000 Subject: [PATCH 03/20] T080: 16.3, the credential stays a reference; a raw key is refused (plan 034) A key inside the endpoint URL is refused when a binding is declared, by the product's own detector, runtime/local_git_adapter's carries_a_credential. It runs before the scheme check, so no refusal repeats the URL. The refusal is one fixed sentence. A key in an extra field is refused as an unknown key, as it always was. Each record now has ONE resolver, and credential_source() says which: - a BROKER, for any other reference, as before; - the BUILT-IN RESOLVER, for `env:NAME` and `keyring:SERVICE/USERNAME` references (R1Q17 (b)). doxbench_provider reads the reference at call time, once per request, and keeps nothing: no mint, no ledger event, no retry on a 401. Such a record needs no broker, and a broker_argv given beside it is refused. That refusal is the plan's fail-closed reading (analyze round 2, V2-21), recorded as standing in openxFactory#656 comment 5851950767; no answer rules it. The OS keyring is read through the `keyring` package, imported at call time. It is not a dependency: without it, a keyring reference refuses with a fixed sentence. Two fixed diagnostics join (eight to ten); - NONE, for an endpoint that takes no credential: the auth kind `none` (R1Q18 (a)), under which credential_ref and broker_argv are forbidden. It joins AUTH_KINDS after api_key and oauth, so AUTH_KINDS[0] is unchanged, and its requests carry no authorization header. The record parses a reference's form, once, for both sides, and never reads what it names. A stored record may leave out the fields its resolver forbids or does not need. Each read-back and each removal states the custody sentence that is true of its resolver. The operator door takes `--auth-kind none`, an omitted `--credential-ref` and an empty broker invocation, and `set-credential` refuses a binding no broker answers without reading its standard input. Falsifier: F16.1's three refusals and its control, plus tests of the resolver and of `none`. Ruled: R1Q22 (a), openxFactory#656 comment 5817152735; R1Q17 (b) and R1Q18 (a), comment 5850003126. After: T079, and T007 (batch H), which has NOT landed on openxFactory main (e369cb25): #1144's 16.3 has no addendum yet. This is built to plan 034's text. Drafted ahead of T063 under Brett's phase-3 word ("Only the independent ones"). It does not land before T063. Arc: neutral-product-standalone-operability Lane: openxfactory-4 (openXfactory-4-openDox_extraction) Co-Authored-By: Claude Opus 5.5 (1M context) --- src/opendox/cli_model_binding.py | 75 +++- src/opendox/doxbench_binding.py | 306 ++++++++++++-- src/opendox/doxbench_provider.py | 264 +++++++++--- tests/test_model_provider_broker.py | 633 +++++++++++++++++++++++++++- 4 files changed, 1183 insertions(+), 95 deletions(-) diff --git a/src/opendox/cli_model_binding.py b/src/opendox/cli_model_binding.py index f5c15ad9..51a9709e 100644 --- a/src/opendox/cli_model_binding.py +++ b/src/opendox/cli_model_binding.py @@ -51,7 +51,7 @@ def _declared_binding(args: argparse.Namespace) -> "binding_mod.ModelProviderBin credential_ref=args.credential_ref, auth_kind=args.auth_kind, approved_by=args.approved_by, endpoint=args.endpoint, dialect=args.dialect, model=args.model, - broker_argv=tuple(args.broker_argv)) + broker_argv=tuple(args.broker_argv or ())) #: What `list` prints for a binding that declares no model (#1144 box 16.2). @@ -59,6 +59,19 @@ def _declared_binding(args: argparse.Namespace) -> "binding_mod.ModelProviderBin #: field existed, and the operator reading the list should see that. NO_MODEL_DECLARED = "(none declared: the request names this binding's id)" +#: What `list` prints for a field the record's resolver forbids or does not +#: need (#1144 box 16.3): the reference under the auth kind `none`, and the +#: broker invocation of a record no broker answers. The custody line beside it +#: says which resolver answers instead. +NOT_DECLARED = "(none)" + +#: What `set-credential` says of a binding no broker answers (#1144 box 16.3). +#: There is no broker to hand a credential to: the built-in resolver reads the +#: reference at call time, or the endpoint takes none. +NO_BROKER_TO_HAND_TO = ( + "binding {binding_id!r} names no broker, so there is nothing to hand a " + "credential to: {custody}") + def cmd_model_binding_list(args: argparse.Namespace) -> int: """DISCLOSE every declared binding (task 1.2's read-back). @@ -81,13 +94,16 @@ def cmd_model_binding_list(args: argparse.Namespace) -> int: print(f" provider {record['provider']}") print(f" auth kind {record['auth_kind']}") print(f" approved by {record['approved_by']}") - print(f" credential ref {record['credential_ref']}") + reference = record["credential_ref"] + print(f" credential ref " + f"{reference if reference is not None else NOT_DECLARED}") print(f" endpoint {record['endpoint']}") print(f" dialect {record['dialect']}") model = record["model"] print(f" model " f"{model if model is not None else NO_MODEL_DECLARED}") - print(f" broker argv {record['broker_argv']}") + argv = record["broker_argv"] + print(f" broker argv {argv if argv else NOT_DECLARED}") print(f" custody {record['credential_custody']}") return 0 @@ -100,7 +116,7 @@ def cmd_model_binding_add(args: argparse.Namespace) -> int: print(str(exc), file=sys.stderr) return 1 print(f" declared {binding.id} in {store.path}") - print(f" {binding_mod.CUSTODY_NOTICE}") + print(f" {binding.custody_notice()}") return 0 @@ -123,7 +139,7 @@ def cmd_model_binding_remove(args: argparse.Namespace) -> int: print(str(exc), file=sys.stderr) return 1 print(f" retired {binding.id} from {store.path}") - print(f" {binding_mod.REMOVAL_NOTICE}") + print(f" {binding.removal_notice()}") return 0 @@ -137,7 +153,12 @@ def cmd_model_binding_set_credential(args: argparse.Namespace, *, standard input. No variable in this function ever holds the credential, so none can outlive the call, be echoed in a message, or reach an exception. It is deliberately NOT a command-line argument: an argv is visible in the - process table and lands in a shell history.""" + process table and lands in a shell history. + + A BINDING NO BROKER ANSWERS IS REFUSED, and its standard input is left + unread (#1144 box 16.3). Its credential is where its `env:` or `keyring:` + reference names, or it takes none, so there is no custodian to hand a + value to, and reading one here would be holding it for nothing.""" from opendox import doxbench_provider as provider_mod store = _binding_store(args) @@ -146,6 +167,10 @@ def cmd_model_binding_set_credential(args: argparse.Namespace, *, if binding is None: raise binding_mod.BindingRefused( f"no binding with id {args.id!r} is declared") + if (binding.credential_source() + != binding_mod.CREDENTIAL_FROM_BROKER): + raise binding_mod.BindingRefused(NO_BROKER_TO_HAND_TO.format( + binding_id=binding.id, custody=binding.custody_notice())) reference = provider_mod.hand_off_credential( binding, source if source is not None else sys.stdin) store.edit(dataclasses.replace(binding, credential_ref=reference)) @@ -171,15 +196,25 @@ def _add_binding_declaration_args(parser: argparse.ArgumentParser) -> None: parser.add_argument("--label", required=True, help="the label the model menu shows") parser.add_argument("--provider", required=True, - help="the provider name the broker takes custody for " - "(the broker's declared `--provider`)") - parser.add_argument("--credential-ref", required=True, + help="the provider's name; where a broker holds the " + "credential, the one it takes custody for (the " + "broker's declared `--provider`)") + # A REFERENCE, NEVER THE CREDENTIAL (#1144 box 16.3). NOT REQUIRED by the + # parser, because the auth kind `none` forbids it. The binding itself + # refuses a missing reference for every kind that takes a credential, and + # says why. + parser.add_argument("--credential-ref", default=None, dest="credential_ref", - help="the reference the broker resolves; NEVER the " - "credential itself") + help="the credential's REFERENCE, NEVER the credential " + "itself: env:NAME or keyring:SERVICE/USERNAME, " + "which the built-in resolver reads at call time, " + "or a reference the broker resolves; omitted " + f"for --auth-kind {binding_mod.AUTH_KIND_NONE}") parser.add_argument("--auth-kind", required=True, dest="auth_kind", choices=list(binding_mod.AUTH_KINDS), - help="the authentication kind the broker holds") + help="the authentication kind of the credential; " + f"{binding_mod.AUTH_KIND_NONE} for an endpoint " + "that takes none") # REQUIRED because the broker requires it: `credential-contracts` holds # that a grant without an approver is invalid, and the broker's `intake` # refuses without an approver flag. A binding that could not name one could @@ -208,8 +243,10 @@ def _add_binding_declaration_args(parser: argparse.ArgumentParser) -> None: # neither an endpoint nor a dialect from a mint, deliberately, so both are # declared here — see doxbench_binding's module docstring. parser.add_argument("--endpoint", required=True, - help="the provider endpoint this binding's minted " - "token is presented at") + help="the provider endpoint this binding's requests " + "are sent to; a URL carrying a credential is " + "refused, so name the credential by its reference " + "instead") parser.add_argument("--dialect", required=True, choices=list(binding_mod.DIALECTS), help="the request grammar that endpoint speaks") @@ -230,11 +267,17 @@ def _add_binding_declaration_args(parser: argparse.ArgumentParser) -> None: # rewriting the operator's store path and truncating their template. The # subparsers below also set `allow_abbrev=False`, so the two defences are # independent. + # + # ZERO OR MORE since #1144 box 16.3: a binding the built-in resolver or the + # auth kind `none` answers names no broker, and one given beside either is + # refused by the binding itself. A broker's reference still needs its + # invocation, and the binding still refuses one without it. parser.add_argument( - "broker_argv", nargs="+", metavar="-- BROKER ARGV", + "broker_argv", nargs="*", metavar="-- BROKER ARGV", help="the broker invocation, as argv members, after a bare `--`. " f"Placeholders {binding_mod.ARGV_PLACEHOLDERS} are filled from " - "this binding's own fields") + "this binding's own fields. Omitted for an env: or keyring: " + f"reference and for --auth-kind {binding_mod.AUTH_KIND_NONE}") def _add_model_binding_parser(sub) -> None: diff --git a/src/opendox/doxbench_binding.py b/src/opendox/doxbench_binding.py index c76ca89a..4ac3abc6 100644 --- a/src/opendox/doxbench_binding.py +++ b/src/opendox/doxbench_binding.py @@ -18,10 +18,30 @@ not a validator that a later edit could soften. WHAT IS NOT IN THIS MODULE, deliberately: the broker itself, the credential -hand-off, the minted token, and every provider TRANSPORT. All four live in -`doxbench_provider`, the ONE module this repository permits to hold them, and -the structural boundary test names that module by name. This one holds records -and a file, reaches no network, spawns no process, and never sees a credential. +hand-off, the minted token, the built-in resolver, and every provider +TRANSPORT. All five live in `doxbench_provider`, the ONE module this repository +permits to hold them, and the structural boundary test names that module by +name. This one holds records and a file, reaches no network, spawns no process, +and never sees a credential. + +THE CREDENTIAL STAYS A REFERENCE, AND A KEY IS REFUSED WHEN IT IS DECLARED +(#1144 box 16.3; plan 034 T080). A key inside the endpoint URL is refused by the +product's own detector, `runtime/local_git_adapter.carries_a_credential`, and +the refusal never repeats the URL it refused. A key in an extra field is +refused as an unknown key, as it always was. EACH RECORD HAS ONE RESOLVER, and +the record says which: + + * the BROKER the record names, for any other reference (as before); + * 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; + * 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)). + +This module classifies a reference's FORM when a binding is declared. It never +reads what a reference names. IT DOES NOW DECLARE THE PROVIDER ROUTE, and that is a reconciliation rather than a widening (task 2.6, 0.2 FINDING 3). The binding carries `endpoint` and @@ -59,6 +79,7 @@ from __future__ import annotations import dataclasses +import re from collections.abc import Iterable, Mapping from pathlib import Path @@ -82,17 +103,62 @@ # the closed vocabularies # --------------------------------------------------------------------------- -#: A long-lived API key the broker takes custody of. +#: A long-lived API key. The broker the binding names takes custody of it, or, +#: for an `env:` or `keyring:` reference, the built-in resolver reads it at call +#: time (#1144 box 16.3, RULED R1Q17 (b)). AUTH_KIND_API_KEY = "api_key" #: An OAuth grant the broker holds and refreshes. AUTH_KIND_OAUTH = "oauth" +#: An endpoint that takes NO credential, which is the usual local server (#1144 +#: box 16.3; RULED R1Q18 (a), openxFactory#656 comment 5850003126). It says so +#: EXPLICITLY, as a kind, and never by a field left out. Under it +#: `credential_ref` and `broker_argv` are FORBIDDEN: there is no credential to +#: refer to and no broker to hold one. +AUTH_KIND_NONE = "none" + #: The CLOSED authentication-kind vocabulary. Closed because a free-text kind #: riding a record that neither declares nor forbids it is unenforceable and #: invisible to every consumer — the same argument `credential-contracts` makes #: for its own `issuance_preconditions` vocabulary. -AUTH_KINDS: tuple[str, ...] = (AUTH_KIND_API_KEY, AUTH_KIND_OAUTH) +#: +#: THREE MEMBERS, and the ORDER IS PINNED: `none` joins AFTER the two kinds that +#: take a credential (R1Q18 (a)), so `AUTH_KINDS[0]`, which #1144's F16.1 reads, +#: still names a kind that takes one. +AUTH_KINDS: tuple[str, ...] = (AUTH_KIND_API_KEY, AUTH_KIND_OAUTH, + AUTH_KIND_NONE) + +#: THE BUILT-IN RESOLVER'S REFERENCE FORMS (#1144 box 16.3; RULED R1Q17 (b)). A +#: standalone install has no broker to resolve a reference, so a reference in +#: one of these forms is resolved by a resolver built into `doxbench_provider`, +#: at call time, in that module only. This module holds the FORMS, as it holds +#: the dialects: it classifies a reference when a binding is declared, and +#: never reads what the reference names. +#: +#: * `env:NAME` — the environment variable NAME of the serving process. NAME +#: is a portable variable name: a letter or `_`, then letters, digits or +#: `_`; +#: * `keyring:SERVICE/USERNAME` — the OS keyring's entry for that service and +#: user name. The split is at the LAST `/`, so a service name may contain +#: one, and a user name may not. +#: +#: Any other reference is a BROKER's, as every reference was before 16.3. +CREDENTIAL_REF_ENV = "env:" +CREDENTIAL_REF_KEYRING = "keyring:" +BUILT_IN_REFERENCE_FORMS: tuple[str, ...] = (CREDENTIAL_REF_ENV, + CREDENTIAL_REF_KEYRING) + +_ENV_NAME = re.compile(r"[A-Za-z_][A-Za-z0-9_]*") + +#: The three answers to "what resolves this record's credential", one per +#: record (see the module docstring). `ModelProviderBinding.credential_source` +#: returns one of them. +CREDENTIAL_FROM_BROKER = "broker" +CREDENTIAL_FROM_BUILT_IN_RESOLVER = "built-in-resolver" +NO_CREDENTIAL = "no-credential" +CREDENTIAL_SOURCES: tuple[str, ...] = ( + CREDENTIAL_FROM_BROKER, CREDENTIAL_FROM_BUILT_IN_RESOLVER, NO_CREDENTIAL) #: The CLOSED dialect vocabulary a binding may declare — the request grammar the #: provider client speaks at the declared endpoint. CLOSED rather than open @@ -127,6 +193,35 @@ #: host is not something a provider client should discover at dispatch time. ENDPOINT_SCHEMES: tuple[str, ...] = ("https://", "http://") +#: 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 +#: `https://user:@…` and `…?api_key=` were both ACCEPTED, into a file +#: this module calls safe to commit. A FIXED sentence, composed from nothing +#: the operator typed: the URL it refuses carries the key, and a refusal that +#: repeated the URL would print the key to a terminal, a log, or, through the +#: console's intake route, a browser. +ENDPOINT_CARRIES_A_CREDENTIAL = ( + "the endpoint carries a credential (a user name or password in the URL, or " + "a credential-shaped query or fragment parameter), and a binding is safe to " + "commit only because it holds none; declare the endpoint without it, and " + "name the credential by its reference in credential_ref") + + +def _carries_a_credential(text: str) -> bool: + """The product's ONE detector, `runtime/local_git_adapter. + carries_a_credential`, which #1144 box 16.3 names: it flags a URL with + userinfo and a URL with a credential-shaped parameter, and passes a clean + one. + + Asked here rather than re-derived, so the record and the repository act + cannot disagree about the same bytes. Imported where it is asked, as + `authoring.py` imports from the same module, so this module stays light at + import time. `local_git_adapter` is stdlib-only by the runtime package's + own import-weight contract, so the lean hosted image imports it too.""" + from opendox.runtime.local_git_adapter import carries_a_credential + + return carries_a_credential(text) + #: The exact, ordered field list a binding declares. Nothing else may appear in #: a stored record, and nothing else appears in a read-back. #: @@ -156,10 +251,12 @@ "broker_argv", ) -#: The one field a stored record may leave out. A record without `model` was +#: The one field EVERY stored record may leave out. A record without `model` was #: declared before the field existed, and it keeps the meaning it had: the -#: request names the catalog handle, this binding's `id`. Every other field is -#: required, as it always was. +#: request names the catalog handle, this binding's `id`. A record may also +#: leave out the fields its own resolver forbids or does not need (#1144 box +#: 16.3; `_fields_its_resolver_leaves_out`). Every other field is required, as +#: it always was. OPTIONAL_BINDING_FIELDS: tuple[str, ...] = ("model",) #: The CLOSED placeholder vocabulary an argv template may name. Every member is @@ -183,6 +280,20 @@ "the credential itself is held by the broker this binding names; this " "dashboard stores only the reference above and can disclose nothing more") +#: The same sentence for a record the BUILT-IN RESOLVER answers (#1144 box +#: 16.3). The broker sentence would be false there, and a read-back states +#: custody PLAINLY, so each resolver has its own sentence. +BUILT_IN_CUSTODY_NOTICE = ( + "the credential itself stays where the reference above names, in this " + "process's environment or the OS keyring; it is read at call time for each " + "request and never stored, and this dashboard stores only the reference") + +#: ...and for a record whose endpoint takes no credential (the auth kind +#: `none`). +NO_CREDENTIAL_NOTICE = ( + "this endpoint takes no credential (auth kind none), so there is nothing " + "to hold and nothing to disclose") + #: Where a checkout's bindings live when nothing said otherwise. Beside the gate #: records, under the served checkout, because a binding IS safe to commit and #: an operator reading their repository should be able to see what their install @@ -207,6 +318,59 @@ def _require_non_blank_str(field: str, value: object) -> str: return value +def names_a_built_in_form(credential_ref: object) -> bool: + """Whether a reference is in one of `BUILT_IN_REFERENCE_FORMS`, by its + prefix alone (#1144 box 16.3). A reference that is not is a broker's.""" + return (isinstance(credential_ref, str) + and credential_ref.startswith(BUILT_IN_REFERENCE_FORMS)) + + +def built_in_reference_parts(credential_ref: str) -> tuple[str, ...] | None: + """A reference the built-in resolver takes, split into what it looks up: + `("env:", NAME)` or `("keyring:", SERVICE, USERNAME)`. None for a broker's + reference. + + ONE PARSER, which the record calls when a binding is declared and + `doxbench_provider`'s resolver calls at call time, so the two cannot + disagree about a reference's form. A reference that names a built-in form + but is malformed is REFUSED, at declaration. The refusal does not repeat + the reference: a key pasted where its reference belongs would otherwise be + printed by the very check that refused it.""" + if credential_ref.startswith(CREDENTIAL_REF_ENV): + name = credential_ref[len(CREDENTIAL_REF_ENV):] + if not _ENV_NAME.fullmatch(name): + raise BindingRefused( + "credential_ref uses the env: form, and what follows env: is " + "not an environment variable name (a letter or _, then " + "letters, digits or _)") + return (CREDENTIAL_REF_ENV, name) + if credential_ref.startswith(CREDENTIAL_REF_KEYRING): + service, separator, username = ( + credential_ref[len(CREDENTIAL_REF_KEYRING):].rpartition("/")) + if not separator or not service.strip() or not username.strip(): + raise BindingRefused( + "credential_ref uses the keyring: form, and it does not read " + "keyring:SERVICE/USERNAME with both parts present") + return (CREDENTIAL_REF_KEYRING, service, username) + return None + + +def _fields_its_resolver_leaves_out(record: Mapping) -> set[str]: + """The fields a stored record may leave out because its own resolver + forbids or does not need them (#1144 box 16.3). Under the auth kind `none` + these are `credential_ref` and `broker_argv`. Beside a reference the + built-in resolver takes, it is `broker_argv`. + + Leaving a field out is not declaring it, which is why a record may. A + record that DECLARES one is refused by the binding itself, which is the + rule. This only says which absences are lawful.""" + if record.get("auth_kind") == AUTH_KIND_NONE: + return {"credential_ref", "broker_argv"} + if names_a_built_in_form(record.get("credential_ref")): + return {"broker_argv"} + return set() + + @dataclasses.dataclass(frozen=True, slots=True) class ModelProviderBinding: """ONE model provider, as settings hold it. @@ -224,12 +388,19 @@ class ModelProviderBinding: That binding keeps its old meaning: with no model declared, the request names the catalog handle, which is the binding's `id`, exactly as before. A declared model is a non-blank string. + + ONE RESOLVER PER RECORD (#1144 box 16.3; see the module docstring). Under + the auth kind `none`, `credential_ref` is None and `broker_argv` is empty, + and giving either is refused. A reference the built-in resolver takes + needs no broker, so `broker_argv` is empty there, and a broker given + beside it is refused. Any other reference is a broker's, and it needs its + `broker_argv`, as it always did. """ id: str label: str provider: str - credential_ref: str + credential_ref: str | None auth_kind: str approved_by: str endpoint: str @@ -238,8 +409,8 @@ class ModelProviderBinding: broker_argv: tuple[str, ...] def __post_init__(self) -> None: - for field in ("id", "label", "provider", "credential_ref", "auth_kind", - "approved_by", "endpoint", "dialect"): + for field in ("id", "label", "provider", "auth_kind", "approved_by", + "endpoint", "dialect"): _require_non_blank_str(field, getattr(self, field)) if self.model is not None: _require_non_blank_str("model", self.model) @@ -252,6 +423,10 @@ def __post_init__(self) -> None: f"dialect {self.dialect!r} is outside the closed vocabulary " f"{DIALECTS}; an unknown request grammar is refused at " "DECLARATION rather than guessed at on a paid call") + # A KEY INSIDE THE URL IS REFUSED FIRST (#1144 box 16.3), so no later + # refusal, the scheme's among them, can repeat a URL that carries one. + if _carries_a_credential(self.endpoint): + raise BindingRefused(ENDPOINT_CARRIES_A_CREDENTIAL) if not self.endpoint.startswith(ENDPOINT_SCHEMES): raise BindingRefused( f"endpoint {self.endpoint!r} does not name one of " @@ -268,10 +443,6 @@ def __post_init__(self) -> None: "broker_argv must be an iterable of argv members, got " f"{type(self.broker_argv).__name__}") from error object.__setattr__(self, "broker_argv", argv) - if not argv: - raise BindingRefused( - "broker_argv must name the broker command; an empty invocation " - "is a binding that can never mint") for member in argv: _require_non_blank_str("broker_argv member", member) for name in _placeholder_names(member): @@ -279,14 +450,84 @@ def __post_init__(self) -> None: raise BindingRefused( f"broker_argv names the placeholder {{{name}}}, which " f"is outside the closed vocabulary {ARGV_PLACEHOLDERS}") + self._require_one_resolver(argv) + + def _require_one_resolver(self, argv: tuple[str, ...]) -> None: + """#1144 box 16.3: exactly one thing answers this record's credential. + + THE `none` HALF IS RULED (R1Q18 (a)): an endpoint that takes no + credential declares `none`, and under it the reference and the broker + are both forbidden. THE BUILT-IN HALF is RULED in part: R1Q17 (b) says + a record whose reference the built-in resolver takes needs no broker. + Refusing a broker given BESIDE such a reference is plan 034's own + fail-closed reading, which no answer rules (its analyze round 2, + V2-21), and openxFactory#656 comment 5851950767 records it as standing, + not overruled. A record with two resolvers would leave which one + answers to whoever reads it next.""" + if self.auth_kind == AUTH_KIND_NONE: + if self.credential_ref is not None: + raise BindingRefused( + f"auth_kind {AUTH_KIND_NONE!r} declares an endpoint that " + "takes no credential, so credential_ref is forbidden " + "under it") + if argv: + raise BindingRefused( + f"auth_kind {AUTH_KIND_NONE!r} declares an endpoint that " + "takes no credential, so broker_argv is forbidden under " + "it: there is no credential for a broker to hold") + return + if self.credential_ref is None: + raise BindingRefused( + f"auth_kind {self.auth_kind!r} takes a credential, so " + "credential_ref must name its reference; an endpoint that " + f"takes no credential declares the auth kind " + f"{AUTH_KIND_NONE!r} instead") + _require_non_blank_str("credential_ref", self.credential_ref) + if built_in_reference_parts(self.credential_ref) is not None: + if argv: + raise BindingRefused( + "credential_ref is a reference the built-in resolver " + "takes, so this binding needs no broker; a broker_argv " + "beside it would give one record two resolvers") + return + if not argv: + raise BindingRefused( + "broker_argv must name the broker command; an empty invocation " + "is a binding that can never mint") + + def credential_source(self) -> str: + """Which of `CREDENTIAL_SOURCES` answers this record's credential: the + broker it names, the built-in resolver, or nothing (the auth kind + `none`). `doxbench_provider` routes a turn by it, and the read-back + states custody by it.""" + if self.auth_kind == AUTH_KIND_NONE: + return NO_CREDENTIAL + if names_a_built_in_form(self.credential_ref): + return CREDENTIAL_FROM_BUILT_IN_RESOLVER + return CREDENTIAL_FROM_BROKER + + def custody_notice(self) -> str: + """The fixed custody sentence that is TRUE of this record.""" + return {CREDENTIAL_FROM_BROKER: CUSTODY_NOTICE, + CREDENTIAL_FROM_BUILT_IN_RESOLVER: BUILT_IN_CUSTODY_NOTICE, + NO_CREDENTIAL: NO_CREDENTIAL_NOTICE}[self.credential_source()] + + def removal_notice(self) -> str: + """The fixed sentence a removal of this record states.""" + return {CREDENTIAL_FROM_BROKER: REMOVAL_NOTICE, + CREDENTIAL_FROM_BUILT_IN_RESOLVER: BUILT_IN_REMOVAL_NOTICE, + NO_CREDENTIAL: NO_CREDENTIAL_REMOVAL_NOTICE, + }[self.credential_source()] # -- projections -------------------------------------------------------- def as_record(self) -> dict: """The STORED record: the record kind, then exactly ``BINDING_FIELDS`` in order. `broker_argv` becomes a list because that is what YAML round - trips. An undeclared `model` is written as null, so every stored - record carries all ten keys; nothing else changes shape.""" + trips. An undeclared `model` is written as null, as is the absent + `credential_ref` of an auth-kind-`none` record, and a record that + names no broker writes an empty `broker_argv`. So every stored record + carries all ten keys; nothing else changes shape.""" return { "kind": BINDING_KIND, "id": self.id, @@ -308,9 +549,11 @@ def as_read_back(self) -> dict: There is no credential material to redact here, which is the whole claim: a read-back cannot leak a secret it was never able to hold. The sentence says so in words rather than leaving the absence to be - inferred from a missing key.""" + inferred from a missing key. It is the sentence that is true of THIS + record's resolver (#1144 box 16.3): the broker's, the built-in + resolver's, or none.""" disclosed = self.as_record() - disclosed["credential_custody"] = CUSTODY_NOTICE + disclosed["credential_custody"] = self.custody_notice() return disclosed def substituted_argv(self) -> tuple[str, ...]: @@ -362,23 +605,27 @@ def from_record(cls, record: object) -> "ModelProviderBinding": raise BindingRefused( f"a binding record declares kind {declared_kind!r}, not " f"{BINDING_KIND!r}") + may_leave_out = {*OPTIONAL_BINDING_FIELDS, + *_fields_its_resolver_leaves_out(record)} missing = [field for field in BINDING_FIELDS - if field not in record - and field not in OPTIONAL_BINDING_FIELDS] + if field not in record and field not in may_leave_out] if missing: raise BindingRefused( f"a binding record is missing {missing}") + broker_argv = record.get("broker_argv") return cls( id=record["id"], label=record["label"], provider=record["provider"], - credential_ref=record["credential_ref"], + credential_ref=record.get("credential_ref"), auth_kind=record["auth_kind"], approved_by=record["approved_by"], endpoint=record["endpoint"], dialect=record["dialect"], model=record.get("model"), - broker_argv=record["broker_argv"], + # an absent or null `broker_argv` names no broker. Whether that is + # lawful is the binding's own rule (`_require_one_resolver`) + broker_argv=() if broker_argv is None else broker_argv, ) @@ -591,6 +838,17 @@ def _save(self, bindings: Iterable[ModelProviderBinding]) -> None: "the binding is retired from this checkout; the credential it referenced " "remains in the broker's custody and is not revoked by this act") +#: The removal sentence for a record the built-in resolver answers, and for one +#: that takes no credential (#1144 box 16.3). As with custody, each resolver has +#: its own sentence, so no removal says something that is not so. +BUILT_IN_REMOVAL_NOTICE = ( + "the binding is retired from this checkout; the credential its reference " + "named stays where it is, in the environment or the OS keyring, and is not " + "removed by this act") +NO_CREDENTIAL_REMOVAL_NOTICE = ( + "the binding is retired from this checkout; it referred to no credential, " + "so there is nothing to revoke") + def bindings_path(checkout_root: Path | str) -> Path: """The default store path for a checkout. ONE rule, so two entrypoints diff --git a/src/opendox/doxbench_provider.py b/src/opendox/doxbench_provider.py index 9317a995..201c1b61 100644 --- a/src/opendox/doxbench_provider.py +++ b/src/opendox/doxbench_provider.py @@ -38,6 +38,12 @@ grammar the binding may declare has one arm here (`_DIALECT_ARMS`): this repository's own prompt grammar, and the OpenAI-compatible chat-completions grammar (#1144 box 16.1); + * OR NO BROKER AT ALL (#1144 box 16.3). A record whose reference is + `env:NAME` or `keyring:SERVICE/USERNAME` is answered by the BUILT-IN + RESOLVER below, at call time and in this module only (RULED R1Q17 (b)). + The reference is read for the one request that presents it, and nothing + is minted, cached or stored. A record with the auth kind `none` presents + no credential at all (RULED R1Q18 (a)); * EXPIRY is handled by the 2026-08-26 ruling: re-mint and retry ONCE, with the re-mint and the paid retry visibly recorded, and a second expiry inside one turn surfaces the standard refusal rather than buying a third call. The @@ -47,10 +53,15 @@ WHAT NEVER HAPPENS HERE: - * a credential is never held. `hand_off_credential` streams the human's value - from an open source straight into the broker's standard input and never - materialises it as a value of its own — no variable that outlives the call, - no file, no echo in a return value, and nothing in any exception; + * a credential is never held beyond the one call that uses it. + `hand_off_credential` streams the human's value from an open source + straight into the broker's standard input and never materialises it as a + value of its own — no variable that outlives the call, no file, no echo in + a return value, and nothing in any exception. The built-in resolver's + value, the one exception 16.3 makes, is read at call time, presented in + that request's authorization header, and dropped with the call. It is + never cached on the port, written, logged, returned, or put in an + exception; * a minted token is never written to a file, never placed in a response, never logged, and never survives the process. `MintedToken` carries a redacting `__repr__`, so even a traceback frame or a debugger `print` of the @@ -230,18 +241,30 @@ "the minted token expired twice within one turn; a further paid call is " "not made on a turn that has already been retried once") +#: The built-in resolver's two refusals (#1144 box 16.3). Fixed, like every +#: other sentence here: they name no variable, no service and no user, so a +#: reference an operator mistyped is not repeated wherever the refusal goes. +DIAG_REFERENCE_UNRESOLVED = ( + "the credential reference resolved to no usable credential, so none was " + "presented") +DIAG_KEYRING_UNAVAILABLE = ( + "the OS keyring could not be read by this process, so the keyring " + "reference could not be resolved") + #: The closed set, so a test can assert no other sentence can be raised. -#: EIGHT, not the nine this set held before the reconciliation. -#: `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 grammar can no longer reach a mint. Keeping a sentence here that no -#: path can raise would be a refusal nobody can trigger, asserted by a test that -#: proves nothing. +#: TEN: the eight the reconciliation left, and the built-in resolver's two +#: (#1144 box 16.3). `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 +#: grammar can no longer reach a mint. Keeping a sentence here that no path can +#: raise would be a refusal nobody can trigger, asserted by a test that proves +#: nothing. FIXED_DIAGNOSTICS: frozenset[str] = frozenset({ DIAG_BROKER_UNREACHABLE, DIAG_BROKER_REFUSED, DIAG_BROKER_MALFORMED, DIAG_BROKER_TIMEOUT, DIAG_PROVIDER_UNREACHABLE, DIAG_PROVIDER_REFUSED, DIAG_PROVIDER_MALFORMED, DIAG_TOKEN_EXPIRED_TWICE, + DIAG_REFERENCE_UNRESOLVED, DIAG_KEYRING_UNAVAILABLE, }) @@ -455,11 +478,22 @@ def broker_operation_argv(binding, operation: str, *, careful: every value comes from a field of the binding, the binding has no secret field to read, and the declaration refuses a credential-shaped flag on every command with its own `secret_in_argv` code. Two independent - refusals, agreeing.""" + refusals, agreeing. + + A BINDING THAT NAMES NO BROKER HAS NO BROKER OPERATION (#1144 box 16.3). + The built-in resolver or the auth kind `none` answers it, and its empty + base invocation would otherwise run the SUBCOMMAND as a program. So that + is a programming error here, like an undeclared operation. The operator + door refuses it in words first (`cli_model_binding`).""" if operation not in OPERATIONS: raise AssertionError( f"{operation!r} is outside the broker's declared operation " f"vocabulary {OPERATIONS}") + if binding.credential_source() != binding_mod.CREDENTIAL_FROM_BROKER: + raise AssertionError( + f"binding {binding.id!r} names no broker, so it has no broker " + "operation: the built-in resolver or the auth kind " + f"{binding_mod.AUTH_KIND_NONE!r} answers it") argv = list(binding.substituted_argv()) if operation == OPERATION_INTAKE: argv += [OPERATION_INTAKE, @@ -616,6 +650,74 @@ def list_references(binding, *, runner=subprocess_broker_runner) -> list: return references +# --------------------------------------------------------------------------- +# the built-in resolver (#1144 box 16.3; RULED R1Q17 (b), `5850003126`) +# --------------------------------------------------------------------------- + +#: What a resolved value may not carry. A line break or a NUL cannot travel in +#: a request header as it is, and trimming one out would present a credential +#: other than the one the reference names, so such a value is refused. +_UNPRESENTABLE_CHARACTERS = ("\r", "\n", "\x00") + + +def _os_keyring(): + """The OS keyring, through the `keyring` package, imported at call time. + + NOT A DEPENDENCY OF THIS PACKAGE, and that is deliberate. An install that + never names a keyring reference never needs it, and one that does installs + it beside openDox. Without it, a keyring reference refuses with the fixed + `DIAG_KEYRING_UNAVAILABLE` rather than raising an import error out of a + turn.""" + try: + import keyring + except ImportError: + raise BrokerRefused(DIAG_KEYRING_UNAVAILABLE) from None + return keyring + + +def resolve_credential_reference(binding, *, environ=None, + keyring_backend=None) -> str: + """THE BUILT-IN RESOLVER: the credential an `env:NAME` or + `keyring:SERVICE/USERNAME` reference names, read NOW. + + AT CALL TIME, IN THIS MODULE ONLY (R1Q17 (b)). The port calls it once per + request and hands the answer straight to `_post_to_provider`, and nothing + keeps it. So a key rotated in the keyring is the key the next request + presents, and no credential lives on the port between turns. The + reference's FORM is parsed by `doxbench_binding.built_in_reference_parts`, + which the record already ran when the binding was declared, so the two + cannot disagree about it. + + `environ` and `keyring_backend` are seams for tests. Production passes + neither, and so reads this process's own environment and the OS keyring. + + Every failure is a FIXED refusal, raised before any provider is contacted. + An unset or blank variable, an absent keyring entry, or a value that could + not travel in a header is `DIAG_REFERENCE_UNRESOLVED`. A keyring that + cannot be read is `DIAG_KEYRING_UNAVAILABLE`. A keyring backend's own error + is dropped unread, like a broker's or a provider's.""" + parts = binding_mod.built_in_reference_parts(binding.credential_ref) + if parts is None: + raise AssertionError( + f"binding {binding.id!r} names a broker's reference, which the " + "broker resolves; the built-in resolver takes only the " + f"{binding_mod.BUILT_IN_REFERENCE_FORMS} forms") + if parts[0] == binding_mod.CREDENTIAL_REF_ENV: + value = (os.environ if environ is None else environ).get(parts[1]) + else: + backend = (keyring_backend if keyring_backend is not None + else _os_keyring()) + try: + value = backend.get_password(parts[1], parts[2]) + except Exception: # noqa: BLE001 - a keyring backend's own error, of any class, never reaches a caller + raise BrokerRefused(DIAG_KEYRING_UNAVAILABLE) from None + if (not isinstance(value, str) or not value.strip() + or any(character in value + for character in _UNPRESENTABLE_CHARACTERS)): + raise BrokerRefused(DIAG_REFERENCE_UNRESOLVED) + return value + + # --------------------------------------------------------------------------- # the provider transport # --------------------------------------------------------------------------- @@ -704,22 +806,26 @@ def _chat_answer(document: dict) -> str: } -def _post_to_provider(token: MintedToken, *, model: str, prompt: str, - timeout: float, opener) -> str: +def _post_to_provider(*, endpoint: str, dialect: str, credential: str | None, + model: str, prompt: str, timeout: float, opener) -> str: """The ONE place a provider is contacted. Returns the assistant prose. - `model` is the model name the request carries, which the port chose (the - binding's declared `model`, or the catalog handle for a binding that - declares none). + `endpoint` and `dialect` are the binding's route. `model` is the model + name the request carries, which the port chose (the binding's declared + `model`, or the catalog handle for a binding that declares none). + `credential` is what this request presents (#1144 box 16.3): the token a + broker minted (a `MintedToken`'s), the value the built-in resolver read for + this call, or None under the auth kind `none`. With None the request + carries no authorization header at all. - The token travels in the request's authorization header and nowhere else; - it is not in the URL (which a proxy logs), not in the body (which an error - handler might echo), and not in this function's return value. + The credential travels in the request's authorization header and nowhere + else; it is not in the URL (which a proxy logs), not in the body (which an + error handler might echo), and not in this function's return value. - THE GRAMMAR IS THE BINDING'S DIALECT (#1144 box 16.1), which the token - carries from the binding. Its arm in `_DIALECT_ARMS` builds the request - body and reads the answer. The route, the header, the bound, the expiry - status and every refusal below are the same for both dialects. + THE GRAMMAR IS THE BINDING'S DIALECT (#1144 box 16.1). Its arm in + `_DIALECT_ARMS` builds the request body and reads the answer. The route, + the header, the bound, the expiry status and every refusal below are the + same for both dialects. THE ANSWER IS BOUNDED (PR #392 review note b). `response.read()` with no argument reads until the peer stops sending, which makes the memory of this @@ -728,18 +834,19 @@ def _post_to_provider(token: MintedToken, *, model: str, prompt: str, byte over `MAX_PROVIDER_ANSWER_BYTES` is read deliberately, so an answer that is exactly at the bound is still honoured while one past it is detected rather than truncated into a shorter document that would parse.""" - arm = _DIALECT_ARMS.get(token.dialect) + arm = _DIALECT_ARMS.get(dialect) if arm is None: raise AssertionError( - f"{token.dialect!r} is outside the declared dialect vocabulary " + f"{dialect!r} is outside the declared dialect vocabulary " f"{DIALECTS}; the binding refuses it at declaration, so no turn " "can carry one") build_request, read_answer = arm body = json.dumps(build_request(model, prompt)).encode("utf-8") - request = urllib.request.Request( # noqa: S310 - endpoint declared on the binding by its operator, carried on the minted token - token.endpoint, data=body, method="POST") + request = urllib.request.Request( # noqa: S310 - endpoint declared on the binding by its operator + endpoint, data=body, method="POST") request.add_header("Content-Type", "application/json") - request.add_header("Authorization", f"Bearer {token.token}") + if credential is not None: + request.add_header("Authorization", f"Bearer {credential}") try: with opener(request, timeout=timeout) as response: payload = response.read(MAX_PROVIDER_ANSWER_BYTES + 1) @@ -827,29 +934,37 @@ def as_dict(self) -> dict: class BrokeredProviderPort: - """A `doxbench_model.WorkbenchModelPort` backed by a broker-minted token. + """A `doxbench_model.WorkbenchModelPort` backed by the binding's + credential: a broker-minted token, or, since #1144 box 16.3, a reference + the built-in resolver reads at call time, or no credential at all under + the auth kind `none`. The record's `credential_source()` says which, and + the name is kept because the entry points construct this class by it. THREE MEMBERS AND NO FOURTH, exactly like every other adapter this seam accepts: `timeout_seconds`, `catalog()`, `dispatch(envelope)`. Everything - below them — minting, expiry, the retry ruling, the provider call — is this - class's business and reaches the seam as one opaque dispatch. + below them — minting, expiry, the retry ruling, the built-in resolver, the + provider call — is this class's business and reaches the seam as one + opaque dispatch. ONE INSTANCE PER PROCESS, for the same reason the harness bridge is: `_workbench_model_port` resolves per REQUEST, and a port constructed per call would mint a fresh token for every turn and throw away a perfectly live one. The token is guarded by a lock because the server is a - `ThreadingHTTPServer`. + `ThreadingHTTPServer`. A record no broker answers holds NO credential on + the port: the resolver reads it for each request. - `catalog()` NEVER MINTS. A menu is not a paid call, and a console that - minted a token to render one would spend a mint on every capabilities - probe.""" + `catalog()` NEVER MINTS AND NEVER RESOLVES. A menu is not a paid call, and + a console that minted a token or read a key to render one would do so on + every capabilities probe.""" def __init__(self, binding, catalog, *, timeout_seconds: float = 60.0, runner=subprocess_broker_runner, opener=urllib.request.urlopen, clock=time.time, - notice=None) -> None: + notice=None, + environ=None, + keyring_backend=None) -> None: if not isinstance(binding, binding_mod.ModelProviderBinding): raise TypeError( "binding must be a ModelProviderBinding, got " @@ -865,9 +980,13 @@ def __init__(self, binding, catalog, *, self._opener = opener self._clock = clock self._notice = notice if notice is not None else sys.stderr.write + # The built-in resolver's seams (#1144 box 16.3). None reads this + # process's own environment and the OS keyring, AT CALL TIME. + self._environ = environ + self._keyring_backend = keyring_backend self._lock = threading.Lock() self._token: MintedToken | None = None - self._mintable = True + self._available = True self.ledger: list[MintEvent] = [] # -- the three port members -------------------------------------------- @@ -878,12 +997,14 @@ def timeout_seconds(self) -> float: def catalog(self) -> model_mod.ModelCatalog: """The install's declared catalog, marked unavailable once this port - knows it cannot mint. + knows it cannot present a credential: a broker that refused to mint, + or a reference the built-in resolver could not resolve. The same honesty the harness bridge keeps: a declaration is available until something is measured, and a broker that has refused is measured. - Nothing here contacts the broker to find out.""" - if self._mintable: + Nothing here contacts the broker, or reads a reference, to find out. + A later turn that succeeds makes the entry available again.""" + if self._available: return self._declared_catalog return model_mod.ModelCatalog.from_entries([ dataclasses.replace(entry, available=False) @@ -917,7 +1038,10 @@ def dispatch(self, prompt_envelope: object) -> object: THE REQUEST'S MODEL IS THE BINDING'S DECLARED `model` (#1144 box 16.2). A binding that declares none sends the catalog handle, which is - what every request sent before the field existed, byte for byte.""" + 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.""" handle = getattr(prompt_envelope, "model_id", None) if not isinstance(handle, str) or not handle: entries = self._declared_catalog.entries @@ -925,11 +1049,17 @@ def dispatch(self, prompt_envelope: object) -> object: declared_model = self._binding.model model = declared_model if declared_model is not None else handle prompt = bridge_mod.render_prompt_message(prompt_envelope) + if (self._binding.credential_source() + != binding_mod.CREDENTIAL_FROM_BROKER): + return {"assistant_prose": self._dispatch_without_a_broker( + model=model, prompt=prompt), + "proposals": []} token = self._current_token(REASON_FIRST_MINT) try: - prose = _post_to_provider(token, model=model, prompt=prompt, - timeout=self._timeout_seconds, - opener=self._opener) + prose = _post_to_provider( + endpoint=token.endpoint, dialect=token.dialect, + credential=token.token, model=model, prompt=prompt, + timeout=self._timeout_seconds, opener=self._opener) except _TokenExpired: # PER-TURN STATE, and no longer than the turn: the expired mint's # own audit reference, read before the token is dropped, so the @@ -942,13 +1072,49 @@ def dispatch(self, prompt_envelope: object) -> object: self._record(REASON_PAID_RETRY) try: prose = _post_to_provider( - token, model=model, prompt=prompt, + endpoint=token.endpoint, dialect=token.dialect, + credential=token.token, model=model, prompt=prompt, timeout=self._timeout_seconds, opener=self._opener) except _TokenExpired: self._forget_token() raise BrokerRefused(DIAG_TOKEN_EXPIRED_TWICE) from None return {"assistant_prose": prose, "proposals": []} + def _dispatch_without_a_broker(self, *, model: str, prompt: str) -> str: + """One turn for a record NO BROKER answers (#1144 box 16.3). + + NOTHING IS MINTED AND NOTHING IS KEPT. Under the built-in resolver the + credential is read now, for this one request (RULED R1Q17 (b)), and it + is dropped when this call returns. Under the auth kind `none` there is + no credential, and the request carries no authorization header (RULED + R1Q18 (a)). A resolution that fails refuses before any provider is + contacted, and marks the catalog unavailable as a refused mint does. + + 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.""" + credential = None + if (self._binding.credential_source() + == binding_mod.CREDENTIAL_FROM_BUILT_IN_RESOLVER): + try: + credential = resolve_credential_reference( + self._binding, environ=self._environ, + keyring_backend=self._keyring_backend) + except BrokerRefused: + with self._lock: + self._available = False + raise + with self._lock: + self._available = True + try: + return _post_to_provider( + endpoint=self._binding.endpoint, dialect=self._binding.dialect, + credential=credential, model=model, prompt=prompt, + timeout=self._timeout_seconds, opener=self._opener) + except _TokenExpired: + raise BrokerRefused(DIAG_PROVIDER_REFUSED) from None + # -- token custody ------------------------------------------------------ def _current_token(self, reason: str, *, @@ -971,9 +1137,9 @@ def _current_token(self, reason: str, *, minted = mint(self._binding, retry_of=retry_of, runner=self._runner) except BrokerRefused: - self._mintable = False + self._available = False raise - self._mintable = True + self._available = True self._token = minted self._record(reason, audit_ref=minted.audit_ref) return minted @@ -992,5 +1158,5 @@ def _record(self, reason: str, *, audit_ref: str | None = None) -> None: def __repr__(self) -> str: return (f"BrokeredProviderPort(binding={self._binding.id!r}, " - f"mintable={self._mintable}, " + f"available={self._available}, " f"token={'held' if self._token is not None else 'none'})") diff --git a/tests/test_model_provider_broker.py b/tests/test_model_provider_broker.py index 82bf9a11..a2ff186e 100644 --- a/tests/test_model_provider_broker.py +++ b/tests/test_model_provider_broker.py @@ -22,8 +22,10 @@ UNCONFIGURED posture is byte-for-byte what it was before this change. A SIXTH LAYER, (f), holds #1144 Group 16's binding and provider boxes (plan -034 phase 3, slice P3-B). 16.1 is the OpenAI-compatible dialect (T078), and -16.2 is the model name the provider receives (T079). +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). THE FAKE BROKER SPEAKS THE DECLARED CONTRACT (task 2.6). It was this repository's own invented stdin/stdout protocol until the reconciliation, which @@ -137,9 +139,14 @@ def test_no_secret_field_exists_in_the_shape_to_populate(secret_field): def test_the_auth_kind_vocabulary_is_closed(): - assert binding_mod.AUTH_KINDS == ("api_key", "oauth") - for kind in binding_mod.AUTH_KINDS: + """THREE KINDS since #1144 box 16.3 (RULED R1Q18 (a)), in a pinned order: + `none` joins after the two that take a credential. A `none` binding names + no reference and no broker, which is why it is built apart here.""" + assert binding_mod.AUTH_KINDS == ("api_key", "oauth", "none") + for kind in (binding_mod.AUTH_KIND_API_KEY, binding_mod.AUTH_KIND_OAUTH): assert _binding(auth_kind=kind).auth_kind == kind + assert _binding(auth_kind=binding_mod.AUTH_KIND_NONE, credential_ref=None, + broker_argv=()).auth_kind == "none" with pytest.raises(binding_mod.BindingRefused): _binding(auth_kind="whatever_the_broker_likes") @@ -1123,8 +1130,12 @@ def test_a_refusal_cannot_be_composed_from_what_a_broker_said(): def test_the_fixed_diagnostics_are_all_reachable_and_no_more(): """The closed set shed the dialect sentence when the dialect became a declaration-time refusal; keeping an unraisable sentence would be a refusal - nobody can trigger.""" - assert len(provider_mod.FIXED_DIAGNOSTICS) == 8 + nobody can trigger. TEN since #1144 box 16.3: the built-in resolver's two + joined, and section (f) below reaches each of them.""" + assert len(provider_mod.FIXED_DIAGNOSTICS) == 10 + assert {provider_mod.DIAG_REFERENCE_UNRESOLVED, + provider_mod.DIAG_KEYRING_UNAVAILABLE} <= \ + provider_mod.FIXED_DIAGNOSTICS assert not hasattr(provider_mod, "DIAG_DIALECT_UNKNOWN") @@ -1775,3 +1786,613 @@ def test_a_stand_in_chat_server_receives_the_declared_model(tmp_path): assert _ChatCompletionsHandler.seen["body"] == { "model": DECLARED_MODEL, "messages": [{"role": "user", "content": "assembled prompt"}]} + + +# 16.3, the credential stays a reference (T080). +# +# * A key in the endpoint URL or in an extra field is refused when the +# binding is declared. The URL check is the product's own detector, +# `runtime/local_git_adapter.carries_a_credential`. +# * The built-in resolver takes `env:NAME` and OS-keyring references, at call +# time and inside `doxbench_provider` only (RULED R1Q17 (b)). Such a record +# needs no broker, and one given beside it is refused. That refusal is the +# plan's fail-closed reading (analyze round 2, V2-21), which no answer +# rules. The tests below hold both halves. +# * An endpoint that takes no credential declares the auth kind `none`, +# under which `broker_argv` and `credential_ref` are forbidden (RULED +# R1Q18 (a)). It joins after the two kinds that exist. +# +# Every key here is an obvious fake. + +KEY_SENTINEL = "sk-stand-in-7c1e5a90d3b24f68-NOT-A-KEY" +ENV_NAME = "STAND_IN_PROVIDER_KEY" +KEYRING_SERVICE = "https://api.example.invalid" +KEYRING_USER = "brett" + + +def _f16_1_record(): + """F16.1's clean control record, built as #1144's falsifier builds it.""" + rec = {f: "stand-in" for f in binding_mod.BINDING_FIELDS} + rec.update(auth_kind=binding_mod.AUTH_KINDS[0], dialect="openai-chat-v1", + endpoint="http://127.0.0.1:9/v1/chat/completions") + if "broker_argv" in rec: + rec["broker_argv"] = ["stand-in-broker"] + return rec + + +def test_f16_1_a_raw_key_is_refused_in_a_field_and_in_the_url(): + """F16.1's refusal block, as #1144 writes it: the clean control record is + accepted, and each of its three raw keys is refused.""" + rec = _f16_1_record() + binding_mod.ModelProviderBinding.from_record(rec) # the control + for bad in (dict(rec, endpoint="https://user:sk-stand-in@api.example.invalid/v1"), + dict(rec, endpoint="https://api.example.invalid/v1?api_key=sk-stand-in"), + dict(rec, api_key="sk-stand-in")): + with pytest.raises(binding_mod.BindingRefused): + binding_mod.ModelProviderBinding.from_record(bad) + + +def test_f16_1_the_first_auth_kind_still_takes_a_credential(): + """`none` joined AFTER the two kinds that exist (R1Q18 (a)), so the + `AUTH_KINDS[0]` F16.1 builds its control from is unchanged.""" + assert binding_mod.AUTH_KINDS[0] == binding_mod.AUTH_KIND_API_KEY + assert binding_mod.AUTH_KINDS[-1] == binding_mod.AUTH_KIND_NONE == "none" + + +@pytest.mark.parametrize("endpoint", [ + f"https://user:{KEY_SENTINEL}@api.example.invalid/v1", + f"https://{KEY_SENTINEL}@api.example.invalid/v1", + f"https://api.example.invalid/v1?api_key={KEY_SENTINEL}", + f"https://api.example.invalid/v1?key={KEY_SENTINEL}", + f"https://api.example.invalid/v1?token={KEY_SENTINEL}", + f"https://api.example.invalid/v1?%61pi_key={KEY_SENTINEL}", + f"https://api.example.invalid/v1#token={KEY_SENTINEL}", + f"ftp://user:{KEY_SENTINEL}@api.example.invalid/v1", +], ids=["userinfo", "bare-userinfo", "query-api-key", "query-key", + "query-token", "percent-encoded-name", "fragment", "keyed-bad-scheme"]) +def test_a_key_in_the_endpoint_is_refused_and_never_repeated(endpoint): + """The refusal is FIXED, so the URL it refused is repeated nowhere. The + detector runs before the scheme check, which would have echoed it.""" + with pytest.raises(binding_mod.BindingRefused) as caught: + _binding(endpoint=endpoint) + assert str(caught.value) == binding_mod.ENDPOINT_CARRIES_A_CREDENTIAL + assert KEY_SENTINEL not in str(caught.value) + + +@pytest.mark.parametrize("endpoint", [ + "http://127.0.0.1:9/v1/chat/completions", + "http://localhost:11434/v1/chat/completions", + "https://api.example.invalid/v1/chat/completions", + "https://api.example.invalid/v1?model=stand-in&stream=false", +]) +def test_a_clean_endpoint_is_accepted(endpoint): + assert _binding(endpoint=endpoint, dialect=OPENAI_CHAT).endpoint == endpoint + + +def test_a_key_in_an_extra_field_is_refused_as_it_always_was(): + for field in ("api_key", "secret", "token", "password", "key"): + record = dict(_binding().as_record(), **{field: KEY_SENTINEL}) + with pytest.raises(binding_mod.BindingRefused) as caught: + binding_mod.ModelProviderBinding.from_record(record) + assert KEY_SENTINEL not in str(caught.value) + + +def test_a_stored_document_whose_endpoint_carries_a_key_does_not_read( + tmp_path): + """A record written before 16.3 with a key in its URL no longer reads, + and the entry point's fallback still resolves the harness declaration. + The key is not in the refusal it prints.""" + record = _binding().as_record() + record["endpoint"] = f"https://user:{KEY_SENTINEL}@api.example.invalid/v1" + path = tmp_path / "bindings.yaml" + path.write_text(json.dumps({"schema_version": 1, + "kind": binding_mod.BINDINGS_KIND, + "bindings": [record]}), encoding="utf-8") + with pytest.raises(binding_mod.BindingRefused) as caught: + binding_mod.BindingStore(path).list() + assert KEY_SENTINEL not in str(caught.value) + + +# --- one resolver per record --------------------------------------------- + + +def _none_binding(**overrides): + fields = dict(auth_kind=binding_mod.AUTH_KIND_NONE, credential_ref=None, + broker_argv=(), endpoint="http://127.0.0.1:9/v1/chat/completions", + dialect=OPENAI_CHAT, model=DECLARED_MODEL) + fields.update(overrides) + return _binding(**fields) + + +def _built_in_binding(credential_ref=f"env:{ENV_NAME}", **overrides): + fields = dict(credential_ref=credential_ref, broker_argv=(), + dialect=OPENAI_CHAT, model=DECLARED_MODEL) + fields.update(overrides) + return _binding(**fields) + + +def test_the_none_kind_forbids_the_reference_and_the_broker(): + """R1Q18 (a), both halves: declared explicitly, and both fields + forbidden under it. A blank reference is still a reference given.""" + binding = _none_binding() + assert binding.credential_ref is None and binding.broker_argv == () + assert binding.credential_source() == binding_mod.NO_CREDENTIAL + for given in (FAKE_REFERENCE, f"env:{ENV_NAME}", ""): + with pytest.raises(binding_mod.BindingRefused) as caught: + _none_binding(credential_ref=given) + assert "credential_ref is forbidden" in str(caught.value) + with pytest.raises(binding_mod.BindingRefused) as caught: + _none_binding(broker_argv=("openprofiler-broker",)) + assert "broker_argv is forbidden" in str(caught.value) + + +@pytest.mark.parametrize("kind", [binding_mod.AUTH_KIND_API_KEY, + binding_mod.AUTH_KIND_OAUTH]) +def test_a_kind_that_takes_a_credential_must_name_its_reference(kind): + """Never "no credential" by a field left out: the kind says it.""" + with pytest.raises(binding_mod.BindingRefused) as caught: + _binding(auth_kind=kind, credential_ref=None) + assert "'none'" in str(caught.value) + + +@pytest.mark.parametrize("reference", [ + f"env:{ENV_NAME}", f"keyring:{KEYRING_SERVICE}/{KEYRING_USER}"]) +def test_a_built_in_reference_needs_no_broker_and_refuses_one_beside_it( + reference): + """BOTH HALVES, in one test, as T080's text asks. R1Q17 (b): a record + whose reference the built-in resolver takes needs no broker. The plan's + fail-closed reading: a broker given beside it is refused, so the record + has one resolver.""" + binding = _built_in_binding(reference) + assert binding.broker_argv == () + assert binding.credential_source() == \ + binding_mod.CREDENTIAL_FROM_BUILT_IN_RESOLVER + with pytest.raises(binding_mod.BindingRefused) as caught: + _built_in_binding(reference, broker_argv=("openprofiler-broker",)) + assert "two resolvers" in str(caught.value) + + +def test_a_brokers_reference_still_needs_its_broker(): + binding = _binding() + assert binding.credential_source() == binding_mod.CREDENTIAL_FROM_BROKER + with pytest.raises(binding_mod.BindingRefused) as caught: + _binding(broker_argv=()) + assert "must name the broker command" in str(caught.value) + + +def test_the_reference_forms_are_parsed_once(): + parts = binding_mod.built_in_reference_parts + assert parts(f"env:{ENV_NAME}") == ("env:", ENV_NAME) + # the split is at the LAST `/`, so a service may carry one + assert parts(f"keyring:{KEYRING_SERVICE}/{KEYRING_USER}") == ( + "keyring:", KEYRING_SERVICE, KEYRING_USER) + assert parts(FAKE_REFERENCE) is None + assert binding_mod.BUILT_IN_REFERENCE_FORMS == ("env:", "keyring:") + + +@pytest.mark.parametrize("reference", [ + "env:", "env:1BAD", "env:A-B", "env: SPACED", "env:NAME\n", + f"env:{KEY_SENTINEL}", + "keyring:", "keyring:service-only", "keyring:/user", "keyring:service/", + "keyring: /user", f"keyring:{KEY_SENTINEL}", +]) +def test_a_malformed_built_in_reference_is_refused_and_never_repeated( + reference): + """Refused at declaration, and the refusal names the FORM, not the value: + a key pasted where its reference belongs is not printed back.""" + with pytest.raises(binding_mod.BindingRefused) as caught: + _built_in_binding(reference) + message = str(caught.value) + after_the_form = reference.split(":", 1)[1] + if after_the_form.strip(): + assert after_the_form not in message + assert KEY_SENTINEL not in message + + +def test_a_none_record_round_trips_and_may_leave_its_forbidden_fields_out(): + binding = _none_binding() + record = binding.as_record() + assert list(record) == ["kind", *binding_mod.BINDING_FIELDS] + assert record["credential_ref"] is None and record["broker_argv"] == [] + assert binding_mod.ModelProviderBinding.from_record(record) == binding + del record["credential_ref"], record["broker_argv"] + assert binding_mod.ModelProviderBinding.from_record(record) == binding + for key, given in (("credential_ref", FAKE_REFERENCE), + ("broker_argv", ["openprofiler-broker"])): + with pytest.raises(binding_mod.BindingRefused): + binding_mod.ModelProviderBinding.from_record( + dict(record, **{key: given})) + + +def test_a_built_in_record_may_leave_out_its_broker_argv(): + binding = _built_in_binding() + record = binding.as_record() + assert record["broker_argv"] == [] + for absent in ("deleted", None): + candidate = dict(record) + if absent == "deleted": + del candidate["broker_argv"] + else: + candidate["broker_argv"] = None + assert binding_mod.ModelProviderBinding.from_record(candidate) == \ + binding + with pytest.raises(binding_mod.BindingRefused): + binding_mod.ModelProviderBinding.from_record( + dict(record, broker_argv=["openprofiler-broker"])) + + +def test_each_read_back_states_the_custody_that_is_true_of_it(tmp_path): + store = _store(tmp_path) + store.add(_binding()) + store.add(_built_in_binding(id="env-bound", label="Env")) + store.add(_none_binding(id="local", label="Local")) + custody = {record["id"]: record["credential_custody"] + for record in store.read_back()["bindings"]} + assert custody == { + "openprofiler-demo": binding_mod.CUSTODY_NOTICE, + "env-bound": binding_mod.BUILT_IN_CUSTODY_NOTICE, + "local": binding_mod.NO_CREDENTIAL_NOTICE, + } + assert [store.get(i).removal_notice() + for i in ("openprofiler-demo", "env-bound", "local")] == [ + binding_mod.REMOVAL_NOTICE, binding_mod.BUILT_IN_REMOVAL_NOTICE, + binding_mod.NO_CREDENTIAL_REMOVAL_NOTICE] + + +def test_the_consoles_intake_shaped_binding_still_builds(): + """`serve_workbench`'s intake route builds its binding by keyword, with a + placeholder reference and the declared broker. That construction is + unchanged and still valid. A `none` binding cannot be enrolled that way, + because the placeholder is a reference and `none` forbids one.""" + shaped = dict(id="enrolled", label="Enrolled", provider="p", + credential_ref="pending-broker-intake", + approved_by="brett", endpoint=ENDPOINT, + dialect=binding_mod.DIALECT_XFACTORY_PROMPT_V1, + broker_argv=("openprofiler-broker",)) + for kind in (binding_mod.AUTH_KIND_API_KEY, binding_mod.AUTH_KIND_OAUTH): + assert binding_mod.ModelProviderBinding(auth_kind=kind, **shaped) + with pytest.raises(binding_mod.BindingRefused): + binding_mod.ModelProviderBinding(auth_kind=binding_mod.AUTH_KIND_NONE, + **shaped) + + +def test_a_binding_no_broker_answers_has_no_broker_operation(): + for binding in (_none_binding(), _built_in_binding()): + for operation in provider_mod.OPERATIONS: + with pytest.raises(AssertionError): + provider_mod.broker_operation_argv(binding, operation) + with pytest.raises(AssertionError): + provider_mod.hand_off_credential(binding, io.StringIO("x")) + + +# --- the built-in resolver, at call time --------------------------------- + + +class _FakeKeyring: + """A stand-in for the `keyring` package: the one call the resolver makes, + recorded.""" + + def __init__(self, entries=None, error=None): + self.entries = dict(entries or {}) + self.error = error + self.asked: list[tuple[str, str]] = [] + + def get_password(self, service, username): + self.asked.append((service, username)) + if self.error is not None: + raise self.error + return self.entries.get((service, username)) + + +def _refusing_runner(argv, **_kwargs): + raise AssertionError(f"a binding no broker answers spawned {argv!r}") + + +def _unbrokered_port(binding, *outcomes, environ=None, keyring_backend=None, + notice=None): + opener = _Opener(*outcomes) + port = provider_mod.BrokeredProviderPort( + binding, install_mod.brokered_catalog(binding), + runner=_refusing_runner, opener=opener, + notice=notice if notice is not None else (lambda _text: None), + environ=environ, keyring_backend=keyring_backend) + return port, opener + + +def test_an_env_reference_is_read_at_call_time_and_presented_as_the_bearer(): + """R1Q17 (b): no broker, no mint, and the value read for each request, so + a rotated value is the one the next request presents.""" + environ = {ENV_NAME: KEY_SENTINEL} + port, opener = _unbrokered_port(_built_in_binding(), + _chat_completion("a"), + _chat_completion("b"), environ=environ) + assert port.dispatch(_Envelope())["assistant_prose"] == "a" + environ[ENV_NAME] = "sk-rotated-stand-in-NOT-A-KEY" + assert port.dispatch(_Envelope())["assistant_prose"] == "b" + assert [request.get_header("Authorization") + for request in opener.requests] == [ + f"Bearer {KEY_SENTINEL}", "Bearer sk-rotated-stand-in-NOT-A-KEY"] + for request in opener.requests: + assert KEY_SENTINEL not in request.get_full_url() + assert KEY_SENTINEL not in request.data.decode("utf-8") + assert port.ledger == [], "nothing was minted" + + +def test_production_reads_this_processs_own_environment(monkeypatch): + monkeypatch.setenv(ENV_NAME, KEY_SENTINEL) + port, opener = _unbrokered_port(_built_in_binding(), + _chat_completion("a")) + port.dispatch(_Envelope()) + assert opener.requests[0].get_header("Authorization") == \ + f"Bearer {KEY_SENTINEL}" + + +@pytest.mark.parametrize("environ", [ + {}, {ENV_NAME: ""}, {ENV_NAME: " "}, {ENV_NAME: "sk-stand-in\nX-Other: 1"}, + {ENV_NAME: "sk-stand-in\x00"}], + ids=["unset", "empty", "blank", "line-break", "nul"]) +def test_an_unusable_env_value_refuses_before_any_request(environ): + port, opener = _unbrokered_port(_built_in_binding(), _chat_completion(), + environ=environ) + with pytest.raises(provider_mod.BrokerRefused) as caught: + port.dispatch(_Envelope()) + assert caught.value.diagnostic == provider_mod.DIAG_REFERENCE_UNRESOLVED + assert opener.requests == [], "no provider was contacted" + assert port.catalog().entries[0].available is False + + +def test_a_reference_that_resolves_again_makes_the_entry_available_again(): + environ: dict[str, str] = {} + port, _opener = _unbrokered_port(_built_in_binding(), + _chat_completion("a"), environ=environ) + with pytest.raises(provider_mod.BrokerRefused): + port.dispatch(_Envelope()) + assert port.catalog().entries[0].available is False + environ[ENV_NAME] = KEY_SENTINEL + assert port.dispatch(_Envelope())["assistant_prose"] == "a" + assert port.catalog().entries[0].available is True + + +def test_a_keyring_reference_reads_the_os_keyring_at_call_time(): + backend = _FakeKeyring({(KEYRING_SERVICE, KEYRING_USER): KEY_SENTINEL}) + port, opener = _unbrokered_port( + _built_in_binding(f"keyring:{KEYRING_SERVICE}/{KEYRING_USER}"), + _chat_completion("a"), _chat_completion("b"), keyring_backend=backend) + port.dispatch(_Envelope()) + port.dispatch(_Envelope()) + assert backend.asked == [(KEYRING_SERVICE, KEYRING_USER)] * 2, \ + "read for each request, and nothing cached" + assert opener.requests[0].get_header("Authorization") == \ + f"Bearer {KEY_SENTINEL}" + + +def test_an_absent_keyring_entry_refuses_unresolved(): + port, opener = _unbrokered_port( + _built_in_binding(f"keyring:{KEYRING_SERVICE}/{KEYRING_USER}"), + _chat_completion(), keyring_backend=_FakeKeyring()) + with pytest.raises(provider_mod.BrokerRefused) as caught: + port.dispatch(_Envelope()) + assert caught.value.diagnostic == provider_mod.DIAG_REFERENCE_UNRESOLVED + assert opener.requests == [] + + +def test_a_keyring_that_cannot_be_read_refuses_and_says_nothing_of_its_own(): + backend = _FakeKeyring(error=RuntimeError("backend detail that leaks")) + port, opener = _unbrokered_port( + _built_in_binding(f"keyring:{KEYRING_SERVICE}/{KEYRING_USER}"), + _chat_completion(), keyring_backend=backend) + with pytest.raises(provider_mod.BrokerRefused) as caught: + port.dispatch(_Envelope()) + assert caught.value.diagnostic == provider_mod.DIAG_KEYRING_UNAVAILABLE + assert "leaks" not in str(caught.value) + assert caught.value.__cause__ is None + assert opener.requests == [] + + +def test_without_the_keyring_package_a_keyring_reference_refuses(monkeypatch): + """`keyring` is not a dependency of this package. Without it, the + production path refuses with the fixed sentence, not an import error.""" + monkeypatch.setitem(sys.modules, "keyring", None) + port, opener = _unbrokered_port( + _built_in_binding(f"keyring:{KEYRING_SERVICE}/{KEYRING_USER}"), + _chat_completion()) + with pytest.raises(provider_mod.BrokerRefused) as caught: + port.dispatch(_Envelope()) + assert caught.value.diagnostic == provider_mod.DIAG_KEYRING_UNAVAILABLE + assert opener.requests == [] + + +def test_the_production_keyring_path_imports_the_package_at_call_time( + monkeypatch): + backend = _FakeKeyring({(KEYRING_SERVICE, KEYRING_USER): KEY_SENTINEL}) + monkeypatch.setitem(sys.modules, "keyring", backend) + port, opener = _unbrokered_port( + _built_in_binding(f"keyring:{KEYRING_SERVICE}/{KEYRING_USER}"), + _chat_completion("a")) + assert port.dispatch(_Envelope())["assistant_prose"] == "a" + assert backend.asked == [(KEYRING_SERVICE, KEYRING_USER)] + + +def test_a_none_binding_presents_no_credential_and_spawns_no_broker(): + port, opener = _unbrokered_port(_none_binding(), _chat_completion("a")) + assert port.dispatch(_Envelope())["assistant_prose"] == "a" + request = opener.requests[0] + assert not request.has_header("Authorization") + assert json.loads(request.data.decode("utf-8"))["model"] == DECLARED_MODEL + assert port.ledger == [] + + +@pytest.mark.parametrize("which", ["built-in", "none"]) +def test_a_401_without_a_broker_is_a_refusal_not_a_retry(which): + """The 2026-08-26 retry ruling is about a MINTED token. Here there is + nothing to re-mint, so a retry would buy a second paid call for the same + refusal.""" + binding = _built_in_binding() if which == "built-in" else _none_binding() + port, opener = _unbrokered_port(binding, _expired_error(), + _chat_completion("never reached"), + environ={ENV_NAME: KEY_SENTINEL}) + with pytest.raises(provider_mod.BrokerRefused) as caught: + port.dispatch(_Envelope()) + assert caught.value.diagnostic == provider_mod.DIAG_PROVIDER_REFUSED + assert len(opener.requests) == 1, "no second paid call" + assert port.ledger == [] + + +def test_the_resolved_credential_reaches_no_response_log_repr_or_disk( + tmp_path): + printed: list[str] = [] + port, _opener = _unbrokered_port(_built_in_binding(), + _chat_completion("the answer"), + environ={ENV_NAME: KEY_SENTINEL}, + notice=printed.append) + answer = port.dispatch(_Envelope()) + assert KEY_SENTINEL not in json.dumps(answer) + assert KEY_SENTINEL not in repr(port) + assert KEY_SENTINEL not in "".join(printed) + assert KEY_SENTINEL not in repr(port.ledger) + port_state = {name: getattr(port, name) for name in dir(port) + if name.startswith("_") and not name.startswith("__") + and name != "_environ"} + assert KEY_SENTINEL not in repr(port_state), \ + "the port keeps no credential between turns" + assert not [path for path in tmp_path.rglob("*") if path.is_file() + and KEY_SENTINEL in path.read_text(encoding="utf-8", + errors="replace")] + + +def test_an_unresolved_reference_maps_onto_the_seams_fixed_model_failed(): + port, _opener = _unbrokered_port(_built_in_binding(), environ={}) + entry = port.catalog().entries[0] + ticks = iter([0.0, 0.1]) + outcome = model_mod.dispatch_turn(port, _Envelope(), entry=entry, + clock=lambda: next(ticks)) + assert isinstance(outcome, model_mod.TurnDispatchFailure) + assert outcome.error == model_mod.DISPATCH_ERR_MODEL_FAILED + assert KEY_SENTINEL not in json.dumps(str(outcome)) + + +@pytest.mark.parametrize("which,expected", [ + ("built-in", f"Bearer {KEY_SENTINEL}"), ("none", None)]) +def test_a_stand_in_server_sees_the_resolved_bearer_or_no_header(which, + expected): + with _stand_in_provider(_ChatCompletionsHandler) as base: + endpoint = f"{base}/v1/chat/completions" + binding = (_built_in_binding(endpoint=endpoint) if which == "built-in" + else _none_binding(endpoint=endpoint)) + port = provider_mod.BrokeredProviderPort( + binding, install_mod.brokered_catalog(binding), + runner=_refusing_runner, notice=lambda _text: None, + environ={ENV_NAME: KEY_SENTINEL}) + assert port.dispatch(_Envelope())["assistant_prose"] == \ + "answered in the chat grammar" + assert _ChatCompletionsHandler.seen["authorization"] == expected + assert _ChatCompletionsHandler.seen["body"]["model"] == DECLARED_MODEL + + +def test_the_resolver_lives_in_the_provider_module_alone(): + """R1Q17 (b): "inside `doxbench_provider.py` only". The record parses a + reference's FORM and reads no environment and no keyring. No other module + of the package reads the OS keyring.""" + package = REPO_ROOT / "src" / "opendox" + binding_source = (package / "doxbench_binding.py").read_text( + encoding="utf-8") + for reach in ("os.environ", "getenv", "get_password", "import keyring", + "import os"): + assert reach not in binding_source, reach + holders = sorted(path.relative_to(package).as_posix() + for path in package.rglob("*.py") + if "get_password" in path.read_text(encoding="utf-8") + or "import keyring" in path.read_text(encoding="utf-8")) + assert holders == [provider_mod.PROVIDER_CLIENT_MODULE], holders + + +# --- the operator door ---------------------------------------------------- + + +def test_the_cli_declares_a_binding_for_each_resolver(tmp_path, capsys): + checkout = tmp_path / "checkout" + checkout.mkdir() + parser = cli_mod.build_parser() + + def run(*argv) -> int: + args = parser.parse_args(list(argv)) + return args.func(args) + + root = ["--repo-root", str(checkout)] + route = ["--label", "L", "--provider", "local", + "--credential-approver", "brett@opensoft.one", + "--endpoint", "http://127.0.0.1:9/v1/chat/completions", + "--dialect", OPENAI_CHAT, "--model", DECLARED_MODEL] + store = binding_mod.BindingStore(binding_mod.bindings_path(checkout)) + + # an endpoint that takes no credential: no reference, and no broker + assert run("model-binding", "add", *root, "--id", "local", *route, + "--auth-kind", "none") == 0 + assert binding_mod.NO_CREDENTIAL_NOTICE in capsys.readouterr().out + assert store.get("local").credential_source() == binding_mod.NO_CREDENTIAL + + # a reference the built-in resolver takes: no broker + assert run("model-binding", "add", *root, "--id", "env-bound", *route, + "--auth-kind", "api_key", + "--credential-ref", f"env:{ENV_NAME}") == 0 + assert binding_mod.BUILT_IN_CUSTODY_NOTICE in capsys.readouterr().out + + # ...and one given a broker beside it is refused, through the verb + assert run("model-binding", "add", *root, "--id", "two-resolvers", *route, + "--auth-kind", "api_key", "--credential-ref", f"env:{ENV_NAME}", + "--", "openprofiler-broker") == 1 + assert "two resolvers" in capsys.readouterr().err + + # a kind that takes a credential, with no reference, is refused + assert run("model-binding", "add", *root, "--id", "no-ref", *route, + "--auth-kind", "api_key") == 1 + assert "'none'" in capsys.readouterr().err + + # a key inside the URL is refused, and the refusal does not repeat it + keyed = list(route) + keyed[keyed.index("--endpoint") + 1] = ( + f"https://user:{KEY_SENTINEL}@api.example.invalid/v1") + assert run("model-binding", "add", *root, "--id", "keyed", *keyed, + "--auth-kind", "none") == 1 + captured = capsys.readouterr() + assert binding_mod.ENDPOINT_CARRIES_A_CREDENTIAL in captured.err + assert KEY_SENTINEL not in captured.err + captured.out + assert store.get("keyed") is None, "nothing is stored" + + assert run("model-binding", "list", *root) == 0 + listed = capsys.readouterr().out + from opendox import cli_model_binding as cmb + assert f"credential ref {cmb.NOT_DECLARED}" in listed + assert f"broker argv {cmb.NOT_DECLARED}" in listed + assert f"credential ref env:{ENV_NAME}" in listed + + assert run("model-binding", "remove", *root, "--id", "env-bound") == 0 + assert binding_mod.BUILT_IN_REMOVAL_NOTICE in capsys.readouterr().out + + +class _UnreadableSource: + """A standard input that must never be read.""" + + def read(self, *_args): + raise AssertionError("set-credential read a credential it had no " + "custodian for") + + readline = read + + +@pytest.mark.parametrize("binding_factory", [_none_binding, _built_in_binding], + ids=["none", "built-in"]) +def test_set_credential_refuses_a_binding_no_broker_answers(tmp_path, capsys, + binding_factory): + checkout = tmp_path / "checkout" + (checkout / "ideation" / "dashboard").mkdir(parents=True) + store = binding_mod.BindingStore(binding_mod.bindings_path(checkout)) + binding = binding_factory() + store.add(binding) + args = cli_mod.build_parser().parse_args([ + "model-binding", "set-credential", "--repo-root", str(checkout), + "--id", binding.id]) + assert cli_mod.cmd_model_binding_set_credential( + args, source=_UnreadableSource()) == 1 + assert "names no broker" in capsys.readouterr().err + assert store.get(binding.id) == binding, "nothing changed" From 6e1b8942913f650a182a3bd5999fcd04586f689f Mon Sep 17 00:00:00 2001 From: Brett Heap <1513478+brettheap@users.noreply.github.com> Date: Mon, 28 Sep 2026 19:10:09 +0000 Subject: [PATCH 04/20] T078: one throwing call per pytest.raises block in the new tests (SonarCloud) SonarCloud's analysis of openDox-code#61 flagged five of T078's new test lines: four `pytest.raises` blocks whose envelope was built inside the block (S5778), and one composite assertion (S9073). The envelope is now built before each block and the assertion is split in two. No assertion changes meaning, and the case count is unchanged. 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 | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-) diff --git a/tests/test_model_provider_broker.py b/tests/test_model_provider_broker.py index 9fe6099e..76cbd2ec 100644 --- a/tests/test_model_provider_broker.py +++ b/tests/test_model_provider_broker.py @@ -1429,16 +1429,18 @@ def test_the_prompt_dialect_is_unchanged_byte_for_byte(tmp_path): "content-not-text", "the-other-grammar"]) def test_a_chat_answer_off_the_declared_path_is_malformed(tmp_path, answer): port, _opener = _port(tmp_path, answer, dialect=OPENAI_CHAT) + envelope = _Envelope() with pytest.raises(provider_mod.BrokerRefused) as caught: - port.dispatch(_Envelope()) + port.dispatch(envelope) assert caught.value.diagnostic == provider_mod.DIAG_PROVIDER_MALFORMED def test_a_chat_shaped_answer_is_not_the_prompt_grammars_answer(tmp_path): """Each arm reads its own grammar and no other.""" port, _opener = _port(tmp_path, _chat_completion()) + envelope = _Envelope() with pytest.raises(provider_mod.BrokerRefused) as caught: - port.dispatch(_Envelope()) + port.dispatch(envelope) assert caught.value.diagnostic == provider_mod.DIAG_PROVIDER_MALFORMED @@ -1465,15 +1467,17 @@ def test_the_expiry_ruling_holds_for_the_chat_grammar(tmp_path): provider_mod.REASON_EXPIRY_REMINT, provider_mod.REASON_PAID_RETRY, ] - assert printed and "re-minted once and retried" in printed[0] + assert printed + assert "re-minted once and retried" in printed[0] def test_the_answer_bound_holds_for_the_chat_grammar(tmp_path): bound = provider_mod.MAX_PROVIDER_ANSWER_BYTES oversize = json.dumps(_chat_completion("x" * bound)).encode("utf-8") port, _opener = _port(tmp_path, oversize, dialect=OPENAI_CHAT) + envelope = _Envelope() with pytest.raises(provider_mod.BrokerRefused) as caught: - port.dispatch(_Envelope()) + port.dispatch(envelope) assert caught.value.diagnostic == provider_mod.DIAG_PROVIDER_MALFORMED @@ -1483,8 +1487,9 @@ def test_a_chat_provider_refusal_lands_on_the_fixed_sentence(tmp_path): urllib.error.HTTPError(ENDPOINT, 400, "Bad Request", {}, io.BytesIO(b'{"error":{"message":"leaky"}}')), dialect=OPENAI_CHAT) + envelope = _Envelope() with pytest.raises(provider_mod.BrokerRefused) as caught: - port.dispatch(_Envelope()) + port.dispatch(envelope) assert caught.value.diagnostic == provider_mod.DIAG_PROVIDER_REFUSED assert "leaky" not in str(caught.value) From 053e207a7b6b60c121e5803bead0c91599ff9733 Mon Sep 17 00:00:00 2001 From: Brett Heap <1513478+brettheap@users.noreply.github.com> Date: Mon, 28 Sep 2026 19:15:17 +0000 Subject: [PATCH 05/20] T079: the intake module's docstring stops counting the binding's fields Copilot's overview of openDox-code#62 noted that doxbench_intake.py's module docstring still called the binding a "closed nine-field record" and the intake declaration "not a tenth field on it". T079 made the record ten fields, so both phrases were false. The paragraph now names each count by its tuple, as it already did for the catalog entry, and records that the binding grew once, by `model` (#1144 box 16.2). The declaration is "not a field on it". This is a docstring only: no code, no test and no behaviour changes. Arc: neutral-product-standalone-operability Lane: openxfactory-4 (openXfactory-4-openDox_extraction) Co-Authored-By: Claude Opus 5.5 (1M context) --- src/opendox/doxbench_intake.py | 19 ++++++++++--------- 1 file changed, 10 insertions(+), 9 deletions(-) diff --git a/src/opendox/doxbench_intake.py b/src/opendox/doxbench_intake.py index caaeb4a6..a51d9cfa 100644 --- a/src/opendox/doxbench_intake.py +++ b/src/opendox/doxbench_intake.py @@ -20,21 +20,22 @@ never sees a credential — which is why it is safe for it to be the surface a browser talks to. -WHAT IT DOES NOT WIDEN, also deliberately: the BINDING's closed nine-field -record (`doxbench_binding.BINDING_FIELDS`) and the CATALOG entry's closed public -shape (`doxbench_model.DECLARABLE_ENTRY_FIELDS`). The count is named by that -tuple rather than restated here, because the shape has grown twice by governing -release since this module was written — the routing declaration at -contract-v1.38 and the input-modality declaration at contract-v2.2 — and this -paragraph's claim is that THIS module widens nothing, which is unchanged by -either. Proposed-versus- +WHAT IT DOES NOT WIDEN, also deliberately: the BINDING's closed record +(`doxbench_binding.BINDING_FIELDS`) and the CATALOG entry's closed public +shape (`doxbench_model.DECLARABLE_ENTRY_FIELDS`). Each count is named by its +tuple rather than restated here, because both shapes have grown since this +module was written. The catalog's grew twice by governing release (the routing +declaration at contract-v1.38 and the input-modality declaration at +contract-v2.2), and the binding's grew once, by `model` (#1144 box 16.2, plan +034 T079). This paragraph's claim is that THIS module widens nothing, which is +unchanged by any of them. Proposed-versus- approved is a SERVER-SIDE distinction and a pending declaration is simply not in the catalog, so NEITHER SHAPE GAINS A FIELD FROM THIS MODULE and this module needs no release act. (Both statements are scoped to this module deliberately. The catalog shape HAS gained fields — by the governing releases named above — and each of those was a release act; what has never happened, and is what this paragraph promises, is this module widening either shape.) The declaration is a -SECOND record beside the binding, not a tenth field on it. +SECOND record beside the binding, not a field on it. WHY PENDING-NESS IS A DECLARED FACT AND NOT A DEFAULT. A binding this document says nothing about is UNAFFECTED: it resolves exactly as it resolved before this From f92fca47e934d1979f08bad7c836111b0d87f73a Mon Sep 17 00:00:00 2001 From: Brett Heap <1513478+brettheap@users.noreply.github.com> Date: Mon, 28 Sep 2026 19:30:16 +0000 Subject: [PATCH 06/20] T080: bound the endpoint before the detector; present only what a bearer can carry Two remarks in Copilot's overview of openDox-code#63, both measured here. The endpoint reached the credential detector unbounded. The detector is quadratic in a parameter name's length, and an endpoint arrives from the command line or from the console's intake route. The binding now refuses an endpoint longer than runtime/config.MAX_REMOTE_URL_CHARS before the detector is asked. That is the product's own URL bound, which the repository act applies to a remote for the same reason. It is read when the binding is declared, and the refusal is a fixed sentence that repeats nothing of the endpoint. A resolved credential could be one no header carries. The resolver checked only for CR, LF and NUL. Measured against urllib: - a character outside latin-1 failed while the header was encoded, as DIAG_PROVIDER_UNREACHABLE, which names the wrong party, and the UnicodeEncodeError it chained held the whole header, credential included; - any other non-ASCII character, or an embedded space, was sent. A resolved value must now be non-empty printable ASCII with no whitespace, or it refuses with DIAG_REFERENCE_UNRESOLVED before any provider is contacted. SonarCloud's findings on the same PR: - built_in_reference_parts returns one shape for both forms, a BuiltInReference named tuple (S8495); - the variable-name pattern reads [A-Za-z_]\w* under re.ASCII (S6353); - the broad except carries a bare noqa code, with its reason on the line above (S7632); - each pytest.raises block in T080's tests holds one throwing call (S5778), and two composite assertions are split (S9073). Sixteen new cases: the bound (past it, at it, read from config), six unpresentable values and a printable-ASCII control, a real-socket case for a value outside latin-1, three unusable keyring answers, and two non-ASCII variable names. Five mutants of the fix were each 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_binding.py | 65 ++++++++++-- src/opendox/doxbench_provider.py | 49 +++++---- tests/test_model_provider_broker.py | 152 +++++++++++++++++++++++++--- 3 files changed, 225 insertions(+), 41 deletions(-) diff --git a/src/opendox/doxbench_binding.py b/src/opendox/doxbench_binding.py index 4ac3abc6..ed4db9b0 100644 --- a/src/opendox/doxbench_binding.py +++ b/src/opendox/doxbench_binding.py @@ -27,9 +27,11 @@ THE CREDENTIAL STAYS A REFERENCE, AND A KEY IS REFUSED WHEN IT IS DECLARED (#1144 box 16.3; plan 034 T080). A key inside the endpoint URL is refused by the product's own detector, `runtime/local_git_adapter.carries_a_credential`, and -the refusal never repeats the URL it refused. A key in an extra field is -refused as an unknown key, as it always was. EACH RECORD HAS ONE RESOLVER, and -the record says which: +the refusal never repeats the URL it refused. An endpoint longer than the +product's URL bound is refused before the detector is asked. A key in an extra +field is refused as an unknown key, as it always was. + +EACH RECORD HAS ONE RESOLVER, and the record says which: * the BROKER the record names, for any other reference (as before); * the BUILT-IN RESOLVER, for an `env:NAME` or `keyring:SERVICE/USERNAME` @@ -82,6 +84,7 @@ import re from collections.abc import Iterable, Mapping from pathlib import Path +from typing import NamedTuple # --------------------------------------------------------------------------- # the record's identity (the workspace rule: every YAML carries both) @@ -149,7 +152,23 @@ BUILT_IN_REFERENCE_FORMS: tuple[str, ...] = (CREDENTIAL_REF_ENV, CREDENTIAL_REF_KEYRING) -_ENV_NAME = re.compile(r"[A-Za-z_][A-Za-z0-9_]*") +#: A portable variable name. `re.ASCII` keeps `\w` to letters, digits and `_` +#: of ASCII alone, so a name no other shell could export is refused. +_ENV_NAME = re.compile(r"[A-Za-z_]\w*", re.ASCII) + + +class BuiltInReference(NamedTuple): + """A reference the built-in resolver takes, split into what it looks up. + + One shape for both forms. `form` is one of `BUILT_IN_REFERENCE_FORMS`. + For `env:NAME`, `name` is the variable and `user` is None. For + `keyring:SERVICE/USERNAME`, `name` is the service and `user` is the user + name.""" + + form: str + name: str + user: str | None + #: The three answers to "what resolves this record's credential", one per #: record (see the module docstring). `ModelProviderBinding.credential_source` @@ -206,6 +225,30 @@ "commit only because it holds none; declare the endpoint without it, and " "name the credential by its reference in credential_ref") +#: The refusal an endpoint longer than the product's URL bound earns (#1144 box +#: 16.3; Copilot's overview of openDox-code#63). The detector below is +#: quadratic in a parameter name's length, and an endpoint reaches it from an +#: operator's command line or from the console's intake route, so the +#: endpoint's length is checked before the detector is asked. The bound is the +#: product's own, `runtime/config.MAX_REMOTE_URL_CHARS`, which the repository +#: act applies to a remote for the same reason. Like the refusal above, this +#: one never repeats the endpoint. The only number in it is the product's +#: bound. +ENDPOINT_TOO_LONG = ( + "the endpoint is longer than {bound} characters and is refused unread: " + "the credential check is quadratic in what it is given, and no provider " + "endpoint is this long") + + +def _endpoint_bound() -> int: + """`runtime/config.MAX_REMOTE_URL_CHARS`, read where it is asked. It is + imported there for the same reason as the detector below, so this module + stays light at import time. `runtime/config` is stdlib-only by the runtime + package's import-weight contract.""" + from opendox.runtime import config + + return config.MAX_REMOTE_URL_CHARS + def _carries_a_credential(text: str) -> bool: """The product's ONE detector, `runtime/local_git_adapter. @@ -325,9 +368,10 @@ def names_a_built_in_form(credential_ref: object) -> bool: and credential_ref.startswith(BUILT_IN_REFERENCE_FORMS)) -def built_in_reference_parts(credential_ref: str) -> tuple[str, ...] | None: +def built_in_reference_parts(credential_ref: str) -> BuiltInReference | None: """A reference the built-in resolver takes, split into what it looks up: - `("env:", NAME)` or `("keyring:", SERVICE, USERNAME)`. None for a broker's + `BuiltInReference("env:", NAME, None)` or + `BuiltInReference("keyring:", SERVICE, USERNAME)`. None for a broker's reference. ONE PARSER, which the record calls when a binding is declared and @@ -343,7 +387,7 @@ def built_in_reference_parts(credential_ref: str) -> tuple[str, ...] | None: "credential_ref uses the env: form, and what follows env: is " "not an environment variable name (a letter or _, then " "letters, digits or _)") - return (CREDENTIAL_REF_ENV, name) + return BuiltInReference(CREDENTIAL_REF_ENV, name, None) if credential_ref.startswith(CREDENTIAL_REF_KEYRING): service, separator, username = ( credential_ref[len(CREDENTIAL_REF_KEYRING):].rpartition("/")) @@ -351,7 +395,7 @@ def built_in_reference_parts(credential_ref: str) -> tuple[str, ...] | None: raise BindingRefused( "credential_ref uses the keyring: form, and it does not read " "keyring:SERVICE/USERNAME with both parts present") - return (CREDENTIAL_REF_KEYRING, service, username) + return BuiltInReference(CREDENTIAL_REF_KEYRING, service, username) return None @@ -425,6 +469,11 @@ def __post_init__(self) -> None: "DECLARATION rather than guessed at on a paid call") # A KEY INSIDE THE URL IS REFUSED FIRST (#1144 box 16.3), so no later # refusal, the scheme's among them, can repeat a URL that carries one. + # The length is checked before that, because the detector's work grows + # with the square of what it is given. + if len(self.endpoint) > _endpoint_bound(): + raise BindingRefused(ENDPOINT_TOO_LONG.format( + bound=_endpoint_bound())) if _carries_a_credential(self.endpoint): raise BindingRefused(ENDPOINT_CARRIES_A_CREDENTIAL) if not self.endpoint.startswith(ENDPOINT_SCHEMES): diff --git a/src/opendox/doxbench_provider.py b/src/opendox/doxbench_provider.py index 201c1b61..5cc56be7 100644 --- a/src/opendox/doxbench_provider.py +++ b/src/opendox/doxbench_provider.py @@ -654,10 +654,25 @@ def list_references(binding, *, runner=subprocess_broker_runner) -> list: # the built-in resolver (#1144 box 16.3; RULED R1Q17 (b), `5850003126`) # --------------------------------------------------------------------------- -#: What a resolved value may not carry. A line break or a NUL cannot travel in -#: a request header as it is, and trimming one out would present a credential -#: other than the one the reference names, so such a value is refused. -_UNPRESENTABLE_CHARACTERS = ("\r", "\n", "\x00") +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. + + 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 + still). Any other value is refused, before any provider is contacted + (Copilot's overview of openDox-code#63). Measured against `urllib`: + * a line break or a NUL cannot travel in a header at all; + * a character outside latin-1 fails while the header is encoded. The + refusal from there would read `DIAG_PROVIDER_UNREACHABLE`, which names + the wrong party, and the `UnicodeEncodeError` it chains holds the + whole header, credential included; + * 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.""" + return (isinstance(value, str) and value != "" + and all("!" <= character <= "~" for character in value)) def _os_keyring(): @@ -692,28 +707,28 @@ def resolve_credential_reference(binding, *, environ=None, neither, and so reads this process's own environment and the OS keyring. Every failure is a FIXED refusal, raised before any provider is contacted. - An unset or blank variable, an absent keyring entry, or a value that could - not travel in a header is `DIAG_REFERENCE_UNRESOLVED`. A keyring that - cannot be read is `DIAG_KEYRING_UNAVAILABLE`. A keyring backend's own error - is dropped unread, like a broker's or a provider's.""" - parts = binding_mod.built_in_reference_parts(binding.credential_ref) - if parts is None: + An unset variable, an absent keyring entry, or a value that cannot be + presented as it is (`_presentable`) is `DIAG_REFERENCE_UNRESOLVED`. A + keyring that cannot be read is `DIAG_KEYRING_UNAVAILABLE`. A keyring + backend's own error is dropped unread, like a broker's or a provider's.""" + reference = binding_mod.built_in_reference_parts(binding.credential_ref) + if reference is None: raise AssertionError( f"binding {binding.id!r} names a broker's reference, which the " "broker resolves; the built-in resolver takes only the " f"{binding_mod.BUILT_IN_REFERENCE_FORMS} forms") - if parts[0] == binding_mod.CREDENTIAL_REF_ENV: - value = (os.environ if environ is None else environ).get(parts[1]) + if reference.form == binding_mod.CREDENTIAL_REF_ENV: + value = (os.environ if environ is None else environ).get( + reference.name) else: backend = (keyring_backend if keyring_backend is not None else _os_keyring()) try: - value = backend.get_password(parts[1], parts[2]) - except Exception: # noqa: BLE001 - a keyring backend's own error, of any class, never reaches a caller + value = backend.get_password(reference.name, reference.user) + # A keyring backend's own error, of any class, never reaches a caller. + except Exception: # noqa: BLE001 raise BrokerRefused(DIAG_KEYRING_UNAVAILABLE) from None - if (not isinstance(value, str) or not value.strip() - or any(character in value - for character in _UNPRESENTABLE_CHARACTERS)): + if not _presentable(value): raise BrokerRefused(DIAG_REFERENCE_UNRESOLVED) return value diff --git a/tests/test_model_provider_broker.py b/tests/test_model_provider_broker.py index 151c6121..be8391f9 100644 --- a/tests/test_model_provider_broker.py +++ b/tests/test_model_provider_broker.py @@ -68,6 +68,8 @@ from opendox import doxbench_install as install_mod from opendox import doxbench_model as model_mod from opendox import doxbench_provider as provider_mod +from opendox.runtime import config as runtime_config +from opendox.runtime import local_git_adapter as git_adapter_mod # The credential a human types. A SENTINEL: long, unique, and impossible to # produce by accident, so a sweep that finds it has found the real thing. @@ -1874,6 +1876,58 @@ def test_a_clean_endpoint_is_accepted(endpoint): assert _binding(endpoint=endpoint, dialect=OPENAI_CHAT).endpoint == endpoint +def _endpoint_of_length(length: int, head: str) -> str: + endpoint = head + "a" * (length - len(head)) + assert len(endpoint) == length + return endpoint + + +def test_an_endpoint_past_the_url_bound_is_refused_before_the_detector( + monkeypatch): + """Copilot's overview of openDox-code#63. The detector is quadratic in a + parameter name's length, so the endpoint's length is checked first, + against the product's own bound, and the detector is not asked. The + refusal repeats nothing of the endpoint, and so nothing of a key in it.""" + bound = runtime_config.MAX_REMOTE_URL_CHARS + endpoint = _endpoint_of_length( + bound + 1, + f"https://api.example.invalid/v1?api_key={KEY_SENTINEL}&pad=") + + def _not_asked(_text): + raise AssertionError("the detector was asked about an endpoint past " + "the bound") + + monkeypatch.setattr(git_adapter_mod, "carries_a_credential", _not_asked) + with pytest.raises(binding_mod.BindingRefused) as caught: + _binding(endpoint=endpoint) + message = str(caught.value) + assert message == binding_mod.ENDPOINT_TOO_LONG.format(bound=bound) + assert KEY_SENTINEL not in message + assert "api.example.invalid" not in message + + +def test_an_endpoint_at_the_url_bound_is_declared(): + bound = runtime_config.MAX_REMOTE_URL_CHARS + endpoint = _endpoint_of_length( + bound, "https://api.example.invalid/v1/chat/completions?pad=") + binding = _binding(endpoint=endpoint, dialect=OPENAI_CHAT) + assert binding.endpoint == endpoint + + +def test_the_url_bound_is_the_products_own_read_when_it_is_asked( + monkeypatch): + """The number is `runtime/config`'s, read at declaration, so the record + and the repository act cannot come to hold different bounds.""" + monkeypatch.setattr(runtime_config, "MAX_REMOTE_URL_CHARS", 40) + head = "https://api.example.invalid/" + at_the_bound = _endpoint_of_length(40, head) + past_the_bound = _endpoint_of_length(41, head) + assert _binding(endpoint=at_the_bound).endpoint == at_the_bound + with pytest.raises(binding_mod.BindingRefused) as caught: + _binding(endpoint=past_the_bound) + assert str(caught.value) == binding_mod.ENDPOINT_TOO_LONG.format(bound=40) + + def test_a_key_in_an_extra_field_is_refused_as_it_always_was(): for field in ("api_key", "secret", "token", "password", "key"): record = dict(_binding().as_record(), **{field: KEY_SENTINEL}) @@ -1893,8 +1947,9 @@ def test_a_stored_document_whose_endpoint_carries_a_key_does_not_read( path.write_text(json.dumps({"schema_version": 1, "kind": binding_mod.BINDINGS_KIND, "bindings": [record]}), encoding="utf-8") + store = binding_mod.BindingStore(path) with pytest.raises(binding_mod.BindingRefused) as caught: - binding_mod.BindingStore(path).list() + store.list() assert KEY_SENTINEL not in str(caught.value) @@ -1920,7 +1975,8 @@ def test_the_none_kind_forbids_the_reference_and_the_broker(): """R1Q18 (a), both halves: declared explicitly, and both fields forbidden under it. A blank reference is still a reference given.""" binding = _none_binding() - assert binding.credential_ref is None and binding.broker_argv == () + assert binding.credential_ref is None + assert binding.broker_argv == () assert binding.credential_source() == binding_mod.NO_CREDENTIAL for given in (FAKE_REFERENCE, f"env:{ENV_NAME}", ""): with pytest.raises(binding_mod.BindingRefused) as caught: @@ -1966,17 +2022,27 @@ def test_a_brokers_reference_still_needs_its_broker(): def test_the_reference_forms_are_parsed_once(): + """One parser and ONE SHAPE for both forms, so no caller has to count + what it was given before it reads it.""" parts = binding_mod.built_in_reference_parts - assert parts(f"env:{ENV_NAME}") == ("env:", ENV_NAME) + env = parts(f"env:{ENV_NAME}") + assert env == binding_mod.BuiltInReference("env:", ENV_NAME, None) + keyring = parts(f"keyring:{KEYRING_SERVICE}/{KEYRING_USER}") # the split is at the LAST `/`, so a service may carry one - assert parts(f"keyring:{KEYRING_SERVICE}/{KEYRING_USER}") == ( + assert keyring == binding_mod.BuiltInReference( + "keyring:", KEYRING_SERVICE, KEYRING_USER) + assert (keyring.form, keyring.name, keyring.user) == ( "keyring:", KEYRING_SERVICE, KEYRING_USER) + assert len(env) == len(keyring) assert parts(FAKE_REFERENCE) is None assert binding_mod.BUILT_IN_REFERENCE_FORMS == ("env:", "keyring:") @pytest.mark.parametrize("reference", [ "env:", "env:1BAD", "env:A-B", "env: SPACED", "env:NAME\n", + # `\w` is held to ASCII: a letter or a digit of another script is not a + # portable variable name + "env:NAMÉ", "env:KEY١", f"env:{KEY_SENTINEL}", "keyring:", "keyring:service-only", "keyring:/user", "keyring:service/", "keyring: /user", f"keyring:{KEY_SENTINEL}", @@ -1998,7 +2064,8 @@ def test_a_none_record_round_trips_and_may_leave_its_forbidden_fields_out(): binding = _none_binding() record = binding.as_record() assert list(record) == ["kind", *binding_mod.BINDING_FIELDS] - assert record["credential_ref"] is None and record["broker_argv"] == [] + assert record["credential_ref"] is None + assert record["broker_argv"] == [] assert binding_mod.ModelProviderBinding.from_record(record) == binding del record["credential_ref"], record["broker_argv"] assert binding_mod.ModelProviderBinding.from_record(record) == binding @@ -2066,8 +2133,9 @@ def test_a_binding_no_broker_answers_has_no_broker_operation(): for operation in provider_mod.OPERATIONS: with pytest.raises(AssertionError): provider_mod.broker_operation_argv(binding, operation) + stdin = io.StringIO("x") with pytest.raises(AssertionError): - provider_mod.hand_off_credential(binding, io.StringIO("x")) + provider_mod.hand_off_credential(binding, stdin) # --- the built-in resolver, at call time --------------------------------- @@ -2134,24 +2202,67 @@ def test_production_reads_this_processs_own_environment(monkeypatch): @pytest.mark.parametrize("environ", [ {}, {ENV_NAME: ""}, {ENV_NAME: " "}, {ENV_NAME: "sk-stand-in\nX-Other: 1"}, - {ENV_NAME: "sk-stand-in\x00"}], - ids=["unset", "empty", "blank", "line-break", "nul"]) + {ENV_NAME: "sk-stand-in\x00"}, + # a bearer credential is printable ASCII with no whitespace (Copilot's + # overview of openDox-code#63), so nothing else is presented + {ENV_NAME: f"{KEY_SENTINEL}€"}, {ENV_NAME: f"{KEY_SENTINEL}é"}, + {ENV_NAME: "sk-stand-in NOT-A-KEY"}, {ENV_NAME: "sk-stand-in\tNOT-A-KEY"}, + {ENV_NAME: f"{KEY_SENTINEL} "}, {ENV_NAME: f"{KEY_SENTINEL}\x7f"}], + ids=["unset", "empty", "blank", "line-break", "nul", "outside-latin-1", + "latin-1-not-ascii", "embedded-space", "tab", "trailing-space", + "delete"]) def test_an_unusable_env_value_refuses_before_any_request(environ): port, opener = _unbrokered_port(_built_in_binding(), _chat_completion(), environ=environ) + envelope = _Envelope() with pytest.raises(provider_mod.BrokerRefused) as caught: - port.dispatch(_Envelope()) + port.dispatch(envelope) assert caught.value.diagnostic == provider_mod.DIAG_REFERENCE_UNRESOLVED assert opener.requests == [], "no provider was contacted" assert port.catalog().entries[0].available is False +def test_a_value_outside_latin_1_is_refused_before_any_header_is_built(): + """Over a real socket, because the failure was `urllib`'s. Before the + check, such a value failed while the header was encoded: the refusal read + `DIAG_PROVIDER_UNREACHABLE`, and it chained a `UnicodeEncodeError` whose + `object` held the whole header, credential included (measured). Now it is + the resolver's own fixed refusal, nothing is chained, and no request is + sent.""" + _ChatCompletionsHandler.seen = {} + with _stand_in_provider(_ChatCompletionsHandler) as base: + binding = _built_in_binding(endpoint=f"{base}/v1/chat/completions") + port = provider_mod.BrokeredProviderPort( + binding, install_mod.brokered_catalog(binding), + runner=_refusing_runner, notice=lambda _text: None, + environ={ENV_NAME: f"{KEY_SENTINEL}€"}) + envelope = _Envelope() + with pytest.raises(provider_mod.BrokerRefused) as caught: + port.dispatch(envelope) + assert caught.value.diagnostic == provider_mod.DIAG_REFERENCE_UNRESOLVED + assert caught.value.__cause__ is None + assert caught.value.__context__ is None + assert _ChatCompletionsHandler.seen == {}, "no request reached the server" + + +def test_a_value_of_printable_ascii_is_presented_as_it_is(): + """The check refuses what a bearer credential cannot be and nothing more: + every printable ASCII character but the space is presented unchanged.""" + value = "sk-" + "".join(chr(code) for code in range(0x21, 0x7F)) + port, opener = _unbrokered_port(_built_in_binding(), + _chat_completion("a"), + environ={ENV_NAME: value}) + assert port.dispatch(_Envelope())["assistant_prose"] == "a" + assert opener.requests[0].get_header("Authorization") == f"Bearer {value}" + + def test_a_reference_that_resolves_again_makes_the_entry_available_again(): environ: dict[str, str] = {} port, _opener = _unbrokered_port(_built_in_binding(), _chat_completion("a"), environ=environ) + envelope = _Envelope() with pytest.raises(provider_mod.BrokerRefused): - port.dispatch(_Envelope()) + port.dispatch(envelope) assert port.catalog().entries[0].available is False environ[ENV_NAME] = KEY_SENTINEL assert port.dispatch(_Envelope())["assistant_prose"] == "a" @@ -2171,12 +2282,18 @@ def test_a_keyring_reference_reads_the_os_keyring_at_call_time(): f"Bearer {KEY_SENTINEL}" -def test_an_absent_keyring_entry_refuses_unresolved(): +@pytest.mark.parametrize("stored", [ + None, b"sk-stand-in-NOT-A-KEY", "", f"{KEY_SENTINEL}€"], + ids=["absent", "not-text", "empty", "outside-latin-1"]) +def test_an_absent_or_unusable_keyring_entry_refuses_unresolved(stored): + entries = ({} if stored is None + else {(KEYRING_SERVICE, KEYRING_USER): stored}) port, opener = _unbrokered_port( _built_in_binding(f"keyring:{KEYRING_SERVICE}/{KEYRING_USER}"), - _chat_completion(), keyring_backend=_FakeKeyring()) + _chat_completion(), keyring_backend=_FakeKeyring(entries)) + envelope = _Envelope() with pytest.raises(provider_mod.BrokerRefused) as caught: - port.dispatch(_Envelope()) + port.dispatch(envelope) assert caught.value.diagnostic == provider_mod.DIAG_REFERENCE_UNRESOLVED assert opener.requests == [] @@ -2186,8 +2303,9 @@ def test_a_keyring_that_cannot_be_read_refuses_and_says_nothing_of_its_own(): port, opener = _unbrokered_port( _built_in_binding(f"keyring:{KEYRING_SERVICE}/{KEYRING_USER}"), _chat_completion(), keyring_backend=backend) + envelope = _Envelope() with pytest.raises(provider_mod.BrokerRefused) as caught: - port.dispatch(_Envelope()) + port.dispatch(envelope) assert caught.value.diagnostic == provider_mod.DIAG_KEYRING_UNAVAILABLE assert "leaks" not in str(caught.value) assert caught.value.__cause__ is None @@ -2201,8 +2319,9 @@ def test_without_the_keyring_package_a_keyring_reference_refuses(monkeypatch): port, opener = _unbrokered_port( _built_in_binding(f"keyring:{KEYRING_SERVICE}/{KEYRING_USER}"), _chat_completion()) + envelope = _Envelope() with pytest.raises(provider_mod.BrokerRefused) as caught: - port.dispatch(_Envelope()) + port.dispatch(envelope) assert caught.value.diagnostic == provider_mod.DIAG_KEYRING_UNAVAILABLE assert opener.requests == [] @@ -2236,8 +2355,9 @@ def test_a_401_without_a_broker_is_a_refusal_not_a_retry(which): port, opener = _unbrokered_port(binding, _expired_error(), _chat_completion("never reached"), environ={ENV_NAME: KEY_SENTINEL}) + envelope = _Envelope() with pytest.raises(provider_mod.BrokerRefused) as caught: - port.dispatch(_Envelope()) + port.dispatch(envelope) assert caught.value.diagnostic == provider_mod.DIAG_PROVIDER_REFUSED assert len(opener.requests) == 1, "no second paid call" assert port.ledger == [] From 68b6e412446c4611a6fa3cd1590aac0f2b6f4893 Mon Sep 17 00:00:00 2001 From: Brett Heap <1513478+brettheap@users.noreply.github.com> Date: Mon, 28 Sep 2026 19:42:37 +0000 Subject: [PATCH 07/20] T080: the stand-in server's record is reset through monkeypatch (SonarCloud) SonarCloud's analysis of openDox-code#63 at f92fca47 flagged one line of the new real-socket test: it reset the stand-in handler's class-level record by assignment (S8997). The test now takes monkeypatch, so the record is restored when the test ends. The test file's four new non-ASCII literals are written as \u escapes, so the source shows which code point each case uses. The string values are unchanged, and the case count is unchanged. 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 | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/tests/test_model_provider_broker.py b/tests/test_model_provider_broker.py index be8391f9..225441ae 100644 --- a/tests/test_model_provider_broker.py +++ b/tests/test_model_provider_broker.py @@ -2042,7 +2042,7 @@ def test_the_reference_forms_are_parsed_once(): "env:", "env:1BAD", "env:A-B", "env: SPACED", "env:NAME\n", # `\w` is held to ASCII: a letter or a digit of another script is not a # portable variable name - "env:NAMÉ", "env:KEY١", + "env:NAM\u00c9", "env:KEY\u0661", f"env:{KEY_SENTINEL}", "keyring:", "keyring:service-only", "keyring:/user", "keyring:service/", "keyring: /user", f"keyring:{KEY_SENTINEL}", @@ -2205,7 +2205,7 @@ def test_production_reads_this_processs_own_environment(monkeypatch): {ENV_NAME: "sk-stand-in\x00"}, # a bearer credential is printable ASCII with no whitespace (Copilot's # overview of openDox-code#63), so nothing else is presented - {ENV_NAME: f"{KEY_SENTINEL}€"}, {ENV_NAME: f"{KEY_SENTINEL}é"}, + {ENV_NAME: f"{KEY_SENTINEL}\u20ac"}, {ENV_NAME: f"{KEY_SENTINEL}\u00e9"}, {ENV_NAME: "sk-stand-in NOT-A-KEY"}, {ENV_NAME: "sk-stand-in\tNOT-A-KEY"}, {ENV_NAME: f"{KEY_SENTINEL} "}, {ENV_NAME: f"{KEY_SENTINEL}\x7f"}], ids=["unset", "empty", "blank", "line-break", "nul", "outside-latin-1", @@ -2222,20 +2222,21 @@ def test_an_unusable_env_value_refuses_before_any_request(environ): assert port.catalog().entries[0].available is False -def test_a_value_outside_latin_1_is_refused_before_any_header_is_built(): +def test_a_value_outside_latin_1_is_refused_before_any_header_is_built( + monkeypatch): """Over a real socket, because the failure was `urllib`'s. Before the check, such a value failed while the header was encoded: the refusal read `DIAG_PROVIDER_UNREACHABLE`, and it chained a `UnicodeEncodeError` whose `object` held the whole header, credential included (measured). Now it is the resolver's own fixed refusal, nothing is chained, and no request is sent.""" - _ChatCompletionsHandler.seen = {} + monkeypatch.setattr(_ChatCompletionsHandler, "seen", {}) with _stand_in_provider(_ChatCompletionsHandler) as base: binding = _built_in_binding(endpoint=f"{base}/v1/chat/completions") port = provider_mod.BrokeredProviderPort( binding, install_mod.brokered_catalog(binding), runner=_refusing_runner, notice=lambda _text: None, - environ={ENV_NAME: f"{KEY_SENTINEL}€"}) + environ={ENV_NAME: f"{KEY_SENTINEL}\u20ac"}) envelope = _Envelope() with pytest.raises(provider_mod.BrokerRefused) as caught: port.dispatch(envelope) @@ -2283,7 +2284,7 @@ def test_a_keyring_reference_reads_the_os_keyring_at_call_time(): @pytest.mark.parametrize("stored", [ - None, b"sk-stand-in-NOT-A-KEY", "", f"{KEY_SENTINEL}€"], + None, b"sk-stand-in-NOT-A-KEY", "", f"{KEY_SENTINEL}\u20ac"], ids=["absent", "not-text", "empty", "outside-latin-1"]) def test_an_absent_or_unusable_keyring_entry_refuses_unresolved(stored): entries = ({} if stored is None From 146b5a22530acfe9daec7219b3712df87e2d1027 Mon Sep 17 00:00:00 2001 From: Brett Heap <1513478+brettheap@users.noreply.github.com> Date: Mon, 28 Sep 2026 20:06:09 +0000 Subject: [PATCH 08/20] T080: the console flow's kinds are the ones a broker enrols, said and pinned With `none` in AUTH_KINDS, the console's intake flow offers a subset of the vocabulary: api_key and oauth, the kinds a broker enrols. A `none` binding holds no credential, so the flow, which exists to hand one to a broker, has nothing to collect for it. The operator declares one with `model-binding add --auth-kind none` instead. doxbench_intake.auth_kind_disclosure's docstring now says so, and a new test pins the relationship, in the vocabulary's order. The disclosure's code is unchanged. The docstring sits outside P3-B's row; the PR body flags it. The test named "this_processs_own_environment" is renamed to "the_serving_process_environment". Arc: neutral-product-standalone-operability Lane: openxfactory-4 (openXfactory-4-openDox_extraction) Co-Authored-By: Claude Opus 5.5 (1M context) --- src/opendox/doxbench_intake.py | 6 +++++- tests/test_model_provider_broker.py | 12 +++++++++++- 2 files changed, 16 insertions(+), 2 deletions(-) diff --git a/src/opendox/doxbench_intake.py b/src/opendox/doxbench_intake.py index a51d9cfa..de6f4faa 100644 --- a/src/opendox/doxbench_intake.py +++ b/src/opendox/doxbench_intake.py @@ -233,7 +233,11 @@ def auth_kind_disclosure() -> list[dict]: """The authentication kinds this flow offers, as a surface may disclose them. READ FROM `doxbench_binding.AUTH_KINDS`, never respelled, so the flow and the - record it writes cannot drift into two vocabularies. `accepts_secret` is the + record it writes cannot drift into two vocabularies. The flow offers the + kinds a broker enrols, which is every member but `none` (#1144 box 16.3). A + `none` binding holds no credential, so this flow, which exists to hand one + to a broker, has nothing to collect for it; the operator declares it with + `model-binding add --auth-kind none` instead. `accepts_secret` is the fact a renderer actually needs: it is what decides whether a field that would take a credential is presented at all, and it is stated here — on the server, beside the vocabulary — rather than inferred in a browser from the kind's diff --git a/tests/test_model_provider_broker.py b/tests/test_model_provider_broker.py index 225441ae..f7a648ed 100644 --- a/tests/test_model_provider_broker.py +++ b/tests/test_model_provider_broker.py @@ -66,6 +66,7 @@ from opendox import cli as cli_mod from opendox import doxbench_binding as binding_mod from opendox import doxbench_install as install_mod +from opendox import doxbench_intake as intake_mod from opendox import doxbench_model as model_mod from opendox import doxbench_provider as provider_mod from opendox.runtime import config as runtime_config @@ -1846,6 +1847,15 @@ def test_f16_1_the_first_auth_kind_still_takes_a_credential(): assert binding_mod.AUTH_KINDS[-1] == binding_mod.AUTH_KIND_NONE == "none" +def test_the_console_flow_offers_every_kind_a_broker_enrols(): + """The console's intake flow hands a credential to a broker, so it offers + every member of `AUTH_KINDS` but `none`, in the vocabulary's order. A + `none` binding holds no credential and is declared at the operator door.""" + offered = [entry["kind"] for entry in intake_mod.auth_kind_disclosure()] + assert offered == [kind for kind in binding_mod.AUTH_KINDS + if kind != binding_mod.AUTH_KIND_NONE] + + @pytest.mark.parametrize("endpoint", [ f"https://user:{KEY_SENTINEL}@api.example.invalid/v1", f"https://{KEY_SENTINEL}@api.example.invalid/v1", @@ -2191,7 +2201,7 @@ def test_an_env_reference_is_read_at_call_time_and_presented_as_the_bearer(): assert port.ledger == [], "nothing was minted" -def test_production_reads_this_processs_own_environment(monkeypatch): +def test_production_reads_the_serving_process_environment(monkeypatch): monkeypatch.setenv(ENV_NAME, KEY_SENTINEL) port, opener = _unbrokered_port(_built_in_binding(), _chat_completion("a")) From 4abc6d4de4480f4d31376b876b30062d153e641f Mon Sep 17 00:00:00 2001 From: Brett Heap <1513478+brettheap@users.noreply.github.com> Date: Mon, 28 Sep 2026 22:37:20 +0000 Subject: [PATCH 09/20] T080: a built-in credential travels only over https:// or to this host Brett Heap ruled on the question openDox-code#63 raised, on 2026-09-28, choosing "Refuse unless loopback (Recommended)"; the lane's holder relayed the ruling. A credential the built-in resolver reads (an env: or keyring: reference) is a long-lived key, so it is sent only over https://, or over http:// to 127.0.0.1, ::1 or localhost. Any other http:// endpoint is refused before resolution. - doxbench_binding.is_a_private_route is the one predicate. It matches the endpoint as written, case-blind, against https:// or http:// to one of LOOPBACK_HOSTS with an optional port. A URL parser's reading is not used, because it and the HTTP client read "http://evil.example\@localhost/" as two different hosts. - The record refuses a built-in reference on any other route when it is declared, from the command line or a stored record, with one fixed sentence, ENDPOINT_NOT_PRIVATE, that repeats nothing of the endpoint. - The built-in resolver asks the same predicate before it reads anything. No declared binding reaches that check, so what does is a programming error, raised as an AssertionError with nothing read, as the resolver already does for a broker's reference. - The broker path is left as it is, as the ruling says: a minted token may still be declared over plain http:// to any host. The auth kind none presents no credential and keeps its route too. 32 new cases, including IPv6 loopback, the lookalike host "localhost.evil.com" and mixed-case schemes. Seven mutants of the rule were each 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_binding.py | 62 ++++++++++- src/opendox/doxbench_provider.py | 17 ++- tests/test_model_provider_broker.py | 156 ++++++++++++++++++++++++++++ 3 files changed, 233 insertions(+), 2 deletions(-) diff --git a/src/opendox/doxbench_binding.py b/src/opendox/doxbench_binding.py index ed4db9b0..cf3ef971 100644 --- a/src/opendox/doxbench_binding.py +++ b/src/opendox/doxbench_binding.py @@ -37,7 +37,10 @@ * 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; + 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`); * 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)). @@ -210,8 +213,59 @@ 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. 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. +LOOPBACK_HOSTS: tuple[str, ...] = ("127.0.0.1", "::1", "localhost") + + +def _authority(host: str) -> str: + """`host` as a URL's authority spells it: an IPv6 literal is bracketed.""" + return f"[{host}]" if ":" in host else host + + +#: A PRIVATE ROUTE: `https://` to any host, or `http://` to one of +#: `LOOPBACK_HOSTS`, with an optional port, then the path, the query, the +#: fragment or nothing at all. It is matched against the endpoint AS WRITTEN, +#: not against a parser's reading of it. A URL parser and the HTTP client read +#: `http://evil.example\@localhost/` as two different hosts, and only the +#: client's reading decides where the key would go. It is case-blind, because +#: a scheme and a host name are: `HTTP://` is `http://`. +_PRIVATE_ROUTE = re.compile( + r"https://|http://(?:" + + "|".join(re.escape(_authority(host)) for host in LOOPBACK_HOSTS) + + r")(?::[0-9]{1,5})?(?:[/?#]|\Z)", + re.IGNORECASE | re.ASCII) + + +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.""" + 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. +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") + #: 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 #: `https://user:@…` and `…?api_key=` were both ACCEPTED, into a file @@ -500,6 +554,12 @@ def __post_init__(self) -> None: f"broker_argv names the placeholder {{{name}}}, which " f"is outside the closed vocabulary {ARGV_PLACEHOLDERS}") self._require_one_resolver(argv) + # A CREDENTIAL THE BUILT-IN RESOLVER READS TRAVELS ONLY BY A PRIVATE + # ROUTE (the 2026-09-28 ruling). A broker's minted token and the auth + # kind `none` keep the route they had. + if (self.credential_source() == CREDENTIAL_FROM_BUILT_IN_RESOLVER + and not is_a_private_route(self.endpoint)): + raise BindingRefused(ENDPOINT_NOT_PRIVATE) def _require_one_resolver(self, argv: tuple[str, ...]) -> None: """#1144 box 16.3: exactly one thing answers this record's credential. diff --git a/src/opendox/doxbench_provider.py b/src/opendox/doxbench_provider.py index 5cc56be7..c5555669 100644 --- a/src/opendox/doxbench_provider.py +++ b/src/opendox/doxbench_provider.py @@ -710,13 +710,28 @@ def resolve_credential_reference(binding, *, environ=None, An unset variable, an absent keyring entry, or a value that cannot be presented as it is (`_presentable`) is `DIAG_REFERENCE_UNRESOLVED`. A keyring that cannot be read is `DIAG_KEYRING_UNAVAILABLE`. A keyring - backend's own error is dropped unread, like a broker's or a provider's.""" + backend's own error is dropped unread, like a broker's or a provider's. + + NOTHING IS READ FOR A ROUTE THAT IS NOT PRIVATE (Brett Heap's ruling of + 2026-09-28, "Refuse unless loopback"). What this function reads is a + long-lived key, sent only over `https://` or over `http://` to this host. + The record refuses any other endpoint when the binding is declared + (`doxbench_binding.ENDPOINT_NOT_PRIVATE`), so no declared binding reaches + that check here. The check is repeated before the first read all the + same, because this is the function that holds the key. What reaches it + is a programming error, like a broker's reference, and nothing has been + read when it is raised.""" reference = binding_mod.built_in_reference_parts(binding.credential_ref) if reference is None: raise AssertionError( f"binding {binding.id!r} names a broker's reference, which the " "broker resolves; the built-in resolver takes only the " f"{binding_mod.BUILT_IN_REFERENCE_FORMS} forms") + if not binding_mod.is_a_private_route(binding.endpoint): + raise AssertionError( + f"binding {binding.id!r} routes a credential the built-in " + "resolver reads over a route that is not private, which the " + "record refuses when it is declared; nothing was read") if reference.form == binding_mod.CREDENTIAL_REF_ENV: value = (os.environ if environ is None else environ).get( reference.name) diff --git a/tests/test_model_provider_broker.py b/tests/test_model_provider_broker.py index f7a648ed..c7ce29a7 100644 --- a/tests/test_model_provider_broker.py +++ b/tests/test_model_provider_broker.py @@ -55,6 +55,7 @@ import sys import threading import time +import types import urllib.error from datetime import datetime, timezone from pathlib import Path @@ -2148,6 +2149,147 @@ def test_a_binding_no_broker_answers_has_no_broker_operation(): provider_mod.hand_off_credential(binding, stdin) +# --- a built-in credential travels by a private route -------------------- +# Brett Heap's ruling of 2026-09-28 on this PR's question, "Refuse unless +# loopback": a credential the built-in resolver reads is sent only over +# https://, or over http:// to 127.0.0.1, ::1 or localhost. + +BUILT_IN_REFERENCES = (f"env:{ENV_NAME}", + f"keyring:{KEYRING_SERVICE}/{KEYRING_USER}") +BACKSLASH = chr(92) + + +@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", + "http://localhost:11434/v1/chat/completions", + "http://LOCALHOST:11434/v1/chat/completions", + "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", [ + "http://api.example.invalid/v1/chat/completions", + "http://localhost.evil.com/v1/chat/completions", + "http://127.0.0.1.evil.com/v1/chat/completions", + "http://evil.com/localhost", + "http://127.0.0.2:8080/v1", + "http://[0:0:0:0:0:0:0:1]:8080/v1", + "http://localhost./v1", + "http://localhost%2eevil.com/v1", + "http://0.0.0.0:8080/v1", +], ids=["another-host", "resembles-localhost", "resembles-127", + "localhost-only-in-the-path", "a-loopback-address-not-named", + "another-spelling-of-ipv6-loopback", "trailing-dot", + "percent-encoded-dot", "unspecified-address"]) +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.""" + for reference in BUILT_IN_REFERENCES: + with pytest.raises(binding_mod.BindingRefused) as caught: + _built_in_binding(reference, endpoint=endpoint) + assert str(caught.value) == binding_mod.ENDPOINT_NOT_PRIVATE + record = dict(_built_in_binding(reference).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 + + +@pytest.mark.parametrize("endpoint,private", [ + ("HTTP://api.example.invalid/v1", False), + ("Http://localhost.evil.com/v1", False), + ("hTTp://127.0.0.1:8080/v1", True), + ("HTTP://[::1]:8080/v1", True), + ("http://LocalHost:11434/v1", True), + ("HTTPS://api.example.invalid/v1", True), + ("hTtPs://api.example.invalid/v1", True), + (f"http://evil.example{BACKSLASH}@localhost/v1", False), + (" http://localhost/v1", False), + ("ftp://localhost/v1", False), +], ids=["capital-http-to-another-host", "mixed-case-http-to-a-lookalike", + "mixed-case-http-to-127", "capital-http-to-ipv6-loopback", + "mixed-case-localhost", "capital-https", "mixed-case-https", + "backslash-before-localhost", "leading-space", "another-scheme"]) +def test_a_private_route_is_read_case_blind_and_as_written(endpoint, + private): + """A scheme and a host name are case-blind, so `HTTP://` to another host + is not private and `HTTP://` to this one is. The route is read AS + WRITTEN: a URL parser reads the backslash case's host as `localhost`, and + the HTTP client reads it as the whole authority.""" + assert binding_mod.is_a_private_route(endpoint) is private + + +class _RecordingEnviron(dict): + """An environment that records every name read from it.""" + + def __init__(self, *args): + super().__init__(*args) + self.read: list[str] = [] + + def get(self, name, default=None): + self.read.append(name) + return super().get(name, default) + + +@pytest.mark.parametrize("endpoint", [ + "http://api.example.invalid/v1", "HTTP://api.example.invalid/v1", + "http://localhost.evil.com/v1", + f"http://evil.example{BACKSLASH}@localhost/v1", +], ids=["another-host", "mixed-case-scheme", "resembles-localhost", + "backslash"]) +def test_the_resolver_reads_nothing_for_a_route_that_is_not_private( + endpoint): + """BEFORE RESOLUTION, in the resolver itself. The record refuses such a + binding when it is declared, so this is a binding-shaped object that was + never declared, and it still cannot make the resolver read a key.""" + for reference in BUILT_IN_REFERENCES: + shaped = types.SimpleNamespace(id="undeclared", + credential_ref=reference, + endpoint=endpoint) + environ = _RecordingEnviron({ENV_NAME: KEY_SENTINEL}) + backend = _FakeKeyring({(KEYRING_SERVICE, KEYRING_USER): KEY_SENTINEL}) + with pytest.raises(AssertionError) as caught: + provider_mod.resolve_credential_reference( + shaped, environ=environ, keyring_backend=backend) + assert "nothing was read" in str(caught.value) + assert environ.read == [] + assert backend.asked == [] + + +def test_the_resolver_reads_a_key_for_a_private_route(): + """The control for the case above: the same shape on IPv6 loopback is + read.""" + shaped = types.SimpleNamespace(id="undeclared", + credential_ref=f"env:{ENV_NAME}", + endpoint="http://[::1]:8080/v1") + environ = _RecordingEnviron({ENV_NAME: KEY_SENTINEL}) + assert provider_mod.resolve_credential_reference( + shaped, environ=environ) == KEY_SENTINEL + assert environ.read == [ENV_NAME] + + +@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) + assert _none_binding(endpoint=endpoint).credential_source() == ( + binding_mod.NO_CREDENTIAL) + + # --- the built-in resolver, at call time --------------------------------- @@ -2495,6 +2637,20 @@ def run(*argv) -> int: assert KEY_SENTINEL not in captured.err + captured.out assert store.get("keyed") is None, "nothing is stored" + # a built-in credential over http:// to another host is refused (the + # 2026-09-28 loopback ruling), and a `none` binding to it is declared + cleartext = list(route) + cleartext[cleartext.index("--endpoint") + 1] = ( + "http://api.example.invalid/v1/chat/completions") + assert run("model-binding", "add", *root, "--id", "cleartext", + *cleartext, "--auth-kind", "api_key", + "--credential-ref", f"env:{ENV_NAME}") == 1 + assert binding_mod.ENDPOINT_NOT_PRIVATE in capsys.readouterr().err + assert store.get("cleartext") is None, "nothing is stored" + assert run("model-binding", "add", *root, "--id", "cleartext-none", + *cleartext, "--auth-kind", "none") == 0 + capsys.readouterr() + assert run("model-binding", "list", *root) == 0 listed = capsys.readouterr().out from opendox import cli_model_binding as cmb From d240fd50b521b78411e89ecc8a6f12c4b95b1bd7 Mon Sep 17 00:00:00 2001 From: Brett Heap <1513478+brettheap@users.noreply.github.com> Date: Mon, 28 Sep 2026 22:46:49 +0000 Subject: [PATCH 10/20] T080: the endpoint's checks and the private-route rule move into two helpers (SonarCloud) SonarCloud's analysis of openDox-code#63 at 4abc6d4d measured the binding's __post_init__ at a cognitive complexity of 17, over the 15 allowed (S3776). The endpoint's own checks (the length bound, the key detector and the scheme, in that order) move into _require_a_declarable_endpoint, and the loopback rule into _require_a_private_route. The order of every check is unchanged, and so is every refusal. The whole suite and the seven mutants of the rule give the same results as at 4abc6d4d. 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 | 42 ++++++++++++++++++++------------- 1 file changed, 26 insertions(+), 16 deletions(-) diff --git a/src/opendox/doxbench_binding.py b/src/opendox/doxbench_binding.py index cf3ef971..57359d2e 100644 --- a/src/opendox/doxbench_binding.py +++ b/src/opendox/doxbench_binding.py @@ -521,19 +521,7 @@ def __post_init__(self) -> None: f"dialect {self.dialect!r} is outside the closed vocabulary " f"{DIALECTS}; an unknown request grammar is refused at " "DECLARATION rather than guessed at on a paid call") - # A KEY INSIDE THE URL IS REFUSED FIRST (#1144 box 16.3), so no later - # refusal, the scheme's among them, can repeat a URL that carries one. - # The length is checked before that, because the detector's work grows - # with the square of what it is given. - if len(self.endpoint) > _endpoint_bound(): - raise BindingRefused(ENDPOINT_TOO_LONG.format( - bound=_endpoint_bound())) - if _carries_a_credential(self.endpoint): - raise BindingRefused(ENDPOINT_CARRIES_A_CREDENTIAL) - if not self.endpoint.startswith(ENDPOINT_SCHEMES): - raise BindingRefused( - f"endpoint {self.endpoint!r} does not name one of " - f"{ENDPOINT_SCHEMES}") + self._require_a_declarable_endpoint() if isinstance(self.broker_argv, (str, bytes)): raise BindingRefused( "broker_argv must be a sequence of argv members, not a single " @@ -554,9 +542,31 @@ def __post_init__(self) -> None: f"broker_argv names the placeholder {{{name}}}, which " f"is outside the closed vocabulary {ARGV_PLACEHOLDERS}") self._require_one_resolver(argv) - # A CREDENTIAL THE BUILT-IN RESOLVER READS TRAVELS ONLY BY A PRIVATE - # ROUTE (the 2026-09-28 ruling). A broker's minted token and the auth - # kind `none` keep the route they had. + self._require_a_private_route() + + def _require_a_declarable_endpoint(self) -> None: + """The endpoint's own checks, in the order that keeps a key out of + every refusal (#1144 box 16.3). + + The length is checked first, because the detector's work grows with + the square of what it is given. A KEY INSIDE THE URL IS REFUSED NEXT, + so no later refusal, the scheme's among them, can repeat a URL that + carries one.""" + if len(self.endpoint) > _endpoint_bound(): + raise BindingRefused(ENDPOINT_TOO_LONG.format( + bound=_endpoint_bound())) + if _carries_a_credential(self.endpoint): + raise BindingRefused(ENDPOINT_CARRIES_A_CREDENTIAL) + if not self.endpoint.startswith(ENDPOINT_SCHEMES): + raise BindingRefused( + f"endpoint {self.endpoint!r} does not name one of " + 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 and not is_a_private_route(self.endpoint)): raise BindingRefused(ENDPOINT_NOT_PRIVATE) From 5167084cf4f147e071ad4bed17a35104a03cb59e Mon Sep 17 00:00:00 2001 From: Brett Heap <1513478+brettheap@users.noreply.github.com> Date: Mon, 28 Sep 2026 22:59:31 +0000 Subject: [PATCH 11/20] T080: a request carrying a built-in credential follows no redirect Copilot's review of openDox-code#63 at 4abc6d4d (high severity): the default urllib opener follows redirects, and it re-sends every header but the content ones to the Location. Measured with two loopback servers: a POST answered 301, 302 or 303 reached the redirect's target as a GET that still carried "Authorization: Bearer ...". So a private endpoint could hand a long-lived key to any host and any scheme, which the loopback ruling of 2026-09-28 forbids. A request that carries a credential the built-in resolver read now uses _open_without_redirects. Its _DeclineRedirects handler closes the redirect's answer unread and declines, and the port answers with a new fixed diagnostic, DIAG_PROVIDER_REDIRECTED. FIXED_DIAGNOSTICS goes from ten to eleven. An injected opener is still used as given. The auth kind none sends no credential, and the broker path keeps the default opener, as the ruling leaves that path. Five new cases over real sockets (301, 302, 303, 307 and 308). The second server hears nothing. Two mutants were each killed: the opener swap removed, and the redirect declined silently. 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 | 66 ++++++++++++++++++++++++-- tests/test_model_provider_broker.py | 73 +++++++++++++++++++++++++++-- 2 files changed, 131 insertions(+), 8 deletions(-) diff --git a/src/opendox/doxbench_provider.py b/src/opendox/doxbench_provider.py index c5555669..ae61664e 100644 --- a/src/opendox/doxbench_provider.py +++ b/src/opendox/doxbench_provider.py @@ -251,9 +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. +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 closed set, so a test can assert no other sentence can be raised. -#: TEN: the eight the reconciliation left, and the built-in resolver's two -#: (#1144 box 16.3). `DIAG_DIALECT_UNKNOWN` is gone because the fact it guarded +#: 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 #: 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 @@ -265,6 +275,7 @@ 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, }) @@ -836,6 +847,40 @@ def _chat_answer(document: dict) -> str: } +class _Redirected(Exception): + """A provider answered a request carrying a built-in 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.""" + + +class _DeclineRedirects(urllib.request.HTTPRedirectHandler): + """A redirect handler that follows NO redirect (Copilot's review of + openDox-code#63 at `4abc6d4d`). + + `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.""" + + def redirect_request(self, req, fp, code, msg, headers, newurl): + fp.close() + raise _Redirected + + +def _open_without_redirects(request, *, timeout): + """`urllib.request.urlopen`, but every redirect is declined. The opener + is built per call, as `urlopen` builds its own on first use, so the + proxy environment is read when a request is made.""" + return urllib.request.build_opener(_DeclineRedirects).open( + request, timeout=timeout) + + def _post_to_provider(*, endpoint: str, dialect: str, credential: str | None, model: str, prompt: str, timeout: float, opener) -> str: """The ONE place a provider is contacted. Returns the assistant prose. @@ -1123,7 +1168,15 @@ 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. + + A REQUEST CARRYING A BUILT-IN CREDENTIAL FOLLOWS NO REDIRECT. The + default opener follows redirects and re-sends the credential header + (see `_DeclineRedirects`), so such a request swaps it for + `_open_without_redirects`. 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.""" credential = None if (self._binding.credential_source() == binding_mod.CREDENTIAL_FROM_BUILT_IN_RESOLVER): @@ -1137,13 +1190,18 @@ def _dispatch_without_a_broker(self, *, model: str, prompt: str) -> str: raise with self._lock: self._available = True + opener = self._opener + if credential is not None and opener is urllib.request.urlopen: + opener = _open_without_redirects try: return _post_to_provider( endpoint=self._binding.endpoint, dialect=self._binding.dialect, credential=credential, model=model, prompt=prompt, - timeout=self._timeout_seconds, opener=self._opener) + timeout=self._timeout_seconds, opener=opener) except _TokenExpired: raise BrokerRefused(DIAG_PROVIDER_REFUSED) from None + except _Redirected: + raise BrokerRefused(DIAG_PROVIDER_REDIRECTED) from None # -- token custody ------------------------------------------------------ diff --git a/tests/test_model_provider_broker.py b/tests/test_model_provider_broker.py index c7ce29a7..8d44f5e8 100644 --- a/tests/test_model_provider_broker.py +++ b/tests/test_model_provider_broker.py @@ -1134,11 +1134,13 @@ def test_a_refusal_cannot_be_composed_from_what_a_broker_said(): def test_the_fixed_diagnostics_are_all_reachable_and_no_more(): """The closed set shed the dialect sentence when the dialect became a declaration-time refusal; keeping an unraisable sentence would be a refusal - nobody can trigger. TEN since #1144 box 16.3: the built-in resolver's two - joined, and section (f) below reaches each of them.""" - assert len(provider_mod.FIXED_DIAGNOSTICS) == 10 + 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 assert {provider_mod.DIAG_REFERENCE_UNRESOLVED, - provider_mod.DIAG_KEYRING_UNAVAILABLE} <= \ + provider_mod.DIAG_KEYRING_UNAVAILABLE, + provider_mod.DIAG_PROVIDER_REDIRECTED} <= \ provider_mod.FIXED_DIAGNOSTICS assert not hasattr(provider_mod, "DIAG_DIALECT_UNKNOWN") @@ -2567,6 +2569,69 @@ def test_a_stand_in_server_sees_the_resolved_bearer_or_no_header(which, assert _ChatCompletionsHandler.seen["body"]["model"] == DECLARED_MODEL +class _ElsewhereHandler(http.server.BaseHTTPRequestHandler): + """A second stand-in, where a redirect would lead. It records every + request it is sent, of any method.""" + + seen: list = [] + + def _record(self): + _ElsewhereHandler.seen.append( + (self.command, self.headers.get("Authorization"))) + _answer_json(self, _chat_completion("followed a redirect")) + + def do_GET(self): # noqa: N802 - BaseHTTPRequestHandler's own spelling + self._record() + + def do_POST(self): # noqa: N802 - BaseHTTPRequestHandler's own spelling + self._record() + + def log_message(self, *_args): + return + + +class _RedirectingHandler(http.server.BaseHTTPRequestHandler): + """A stand-in provider that answers every request with a redirect.""" + + code = 302 + location = "" + + def do_POST(self): # noqa: N802 - BaseHTTPRequestHandler's own spelling + self.rfile.read(int(self.headers.get("Content-Length", "0"))) + self.send_response(_RedirectingHandler.code) + self.send_header("Location", _RedirectingHandler.location) + self.send_header("Content-Length", "0") + self.end_headers() + + def log_message(self, *_args): + return + + +@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. + `urllib`'s default opener answers a POST's 301, 302 or 303 by sending a + 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") + binding = _built_in_binding(endpoint=f"{base}/v1/chat/completions") + port = provider_mod.BrokeredProviderPort( + binding, install_mod.brokered_catalog(binding), + runner=_refusing_runner, notice=lambda _text: None, + environ={ENV_NAME: KEY_SENTINEL}) + envelope = _Envelope() + with pytest.raises(provider_mod.BrokerRefused) as caught: + port.dispatch(envelope) + assert caught.value.diagnostic == provider_mod.DIAG_PROVIDER_REDIRECTED + assert _ElsewhereHandler.seen == [], "the credential went nowhere else" + + def test_the_resolver_lives_in_the_provider_module_alone(): """R1Q17 (b): "inside `doxbench_provider.py` only". The record parses a reference's FORM and reads no environment and no keyring. No other module From 1b0fb3f4d9e543f36e99f0eccf1232d98e6682f4 Mon Sep 17 00:00:00 2001 From: Brett Heap <1513478+brettheap@users.noreply.github.com> Date: Mon, 28 Sep 2026 23:09:49 +0000 Subject: [PATCH 12/20] T080: no frame a refusal keeps holds a raw credential Copilot's review of openDox-code#63 at d240fd50 (high severity): _post_to_provider took the credential as a raw string. At main the provider-call frame held a MintedToken, whose repr redacts. A traceback keeps its frames, and an error reporter that records a frame's locals records them by their repr, so the raw string was one repr away from a log. - The credential now travels as _PresentedCredential, whose repr and str say nothing, on both paths. The minted token is wrapped at its two call sites and the built-in value as it is read. So every frame of this module that carries a credential to the provider holds only the wrapper, as it held a MintedToken at main. - A refusal of a request that carried a built-in credential is raised afresh, outside every handler, with no cause and no context. Measured with a refused connection: urllib's own frames (do_open.headers, _send_request.headers, send.data and others) hold the bearer in their locals, and a chained cause keeps those frames. - A value refused as unpresentable is deleted from the reading frame before the refusal is raised. It can still be most of a key. The broker path keeps its chained cause, and so keeps urllib's frames, as the 2026-09-28 ruling leaves that path. The PR body records that. Three new cases: over a real refused socket, for an unpresentable value, and for the broker path's frame. Each walks every frame the refusal keeps, through its causes and contexts. Three mutants were each killed: the wrapper's repr disclosing, the chained cause kept, and the unpresentable value kept. 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 | 75 +++++++++++++++++++---- tests/test_model_provider_broker.py | 95 +++++++++++++++++++++++++++++ 2 files changed, 158 insertions(+), 12 deletions(-) diff --git a/src/opendox/doxbench_provider.py b/src/opendox/doxbench_provider.py index ae61664e..9e692db8 100644 --- a/src/opendox/doxbench_provider.py +++ b/src/opendox/doxbench_provider.py @@ -755,6 +755,10 @@ def resolve_credential_reference(binding, *, environ=None, except Exception: # noqa: BLE001 raise BrokerRefused(DIAG_KEYRING_UNAVAILABLE) from None if not _presentable(value): + # The refusal's traceback keeps this frame, so what was read leaves + # it first: a value that cannot be presented can still be most of a + # key. + del value raise BrokerRefused(DIAG_REFERENCE_UNRESOLVED) return value @@ -847,6 +851,32 @@ def _chat_answer(document: dict) -> str: } +class _PresentedCredential: + """A credential on its way into ONE request's authorization header. + + Its repr and its str say nothing, as `MintedToken`'s do (Copilot's review + 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.""" + + __slots__ = ("_value",) + + def __init__(self, value: str) -> None: + self._value = value + + def __repr__(self) -> str: + return "_PresentedCredential()" + + __str__ = __repr__ + + def authorization(self) -> str: + """The authorization header's value, built as the header is set.""" + return f"Bearer {self._value}" + + class _Redirected(Exception): """A provider answered a request carrying a built-in credential with a redirect, and the redirect was declined. @@ -881,7 +911,8 @@ def _open_without_redirects(request, *, timeout): request, timeout=timeout) -def _post_to_provider(*, endpoint: str, dialect: str, credential: str | None, +def _post_to_provider(*, endpoint: str, dialect: str, + credential: _PresentedCredential | None, model: str, prompt: str, timeout: float, opener) -> str: """The ONE place a provider is contacted. Returns the assistant prose. @@ -891,7 +922,9 @@ def _post_to_provider(*, endpoint: str, dialect: str, credential: str | None, `credential` is what this request presents (#1144 box 16.3): the token a broker minted (a `MintedToken`'s), the value the built-in resolver read for this call, or None under the auth kind `none`. With None the request - carries no authorization header at all. + carries no authorization header at all. A credential arrives wrapped in a + `_PresentedCredential`, so this frame holds no raw value for a traceback + to keep. The credential travels in the request's authorization header and nowhere else; it is not in the URL (which a proxy logs), not in the body (which an @@ -921,7 +954,7 @@ def _post_to_provider(*, endpoint: str, dialect: str, credential: str | None, endpoint, data=body, method="POST") request.add_header("Content-Type", "application/json") if credential is not None: - request.add_header("Authorization", f"Bearer {credential}") + request.add_header("Authorization", credential.authorization()) try: with opener(request, timeout=timeout) as response: payload = response.read(MAX_PROVIDER_ANSWER_BYTES + 1) @@ -1133,8 +1166,9 @@ def dispatch(self, prompt_envelope: object) -> object: try: prose = _post_to_provider( endpoint=token.endpoint, dialect=token.dialect, - credential=token.token, model=model, prompt=prompt, - timeout=self._timeout_seconds, opener=self._opener) + credential=_PresentedCredential(token.token), model=model, + prompt=prompt, timeout=self._timeout_seconds, + opener=self._opener) except _TokenExpired: # PER-TURN STATE, and no longer than the turn: the expired mint's # own audit reference, read before the token is dropped, so the @@ -1148,8 +1182,9 @@ def dispatch(self, prompt_envelope: object) -> object: try: prose = _post_to_provider( endpoint=token.endpoint, dialect=token.dialect, - credential=token.token, model=model, prompt=prompt, - timeout=self._timeout_seconds, opener=self._opener) + credential=_PresentedCredential(token.token), model=model, + prompt=prompt, timeout=self._timeout_seconds, + opener=self._opener) except _TokenExpired: self._forget_token() raise BrokerRefused(DIAG_TOKEN_EXPIRED_TWICE) from None @@ -1176,14 +1211,20 @@ def _dispatch_without_a_broker(self, *, model: str, prompt: str) -> str: `_open_without_redirects`. 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.""" + 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.""" credential = None if (self._binding.credential_source() == binding_mod.CREDENTIAL_FROM_BUILT_IN_RESOLVER): try: - credential = resolve_credential_reference( + credential = _PresentedCredential(resolve_credential_reference( self._binding, environ=self._environ, - keyring_backend=self._keyring_backend) + keyring_backend=self._keyring_backend)) except BrokerRefused: with self._lock: self._available = False @@ -1193,15 +1234,25 @@ def _dispatch_without_a_broker(self, *, model: str, prompt: str) -> str: opener = self._opener if credential is not None and opener is urllib.request.urlopen: opener = _open_without_redirects + failure = None 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) except _TokenExpired: - raise BrokerRefused(DIAG_PROVIDER_REFUSED) from None + failure = DIAG_PROVIDER_REFUSED except _Redirected: - raise BrokerRefused(DIAG_PROVIDER_REDIRECTED) from None + 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. + raise BrokerRefused(failure) # -- token custody ------------------------------------------------------ diff --git a/tests/test_model_provider_broker.py b/tests/test_model_provider_broker.py index 8d44f5e8..73a9d010 100644 --- a/tests/test_model_provider_broker.py +++ b/tests/test_model_provider_broker.py @@ -51,6 +51,7 @@ import http.server import io import json +import socket import subprocess import sys import threading @@ -2632,6 +2633,100 @@ def test_a_built_in_credential_follows_no_redirect(monkeypatch, code): assert _ElsewhereHandler.seen == [], "the credential went nowhere else" +def _safe_repr(value) -> str: + try: + return repr(value) + # A repr that fails discloses nothing, whatever it raised. + except Exception: # noqa: BLE001 + return "" + + +def _frames_kept_by(exception): + """Every frame a refusal's tracebacks keep, through its causes and its + contexts, suppressed or not, except this test file's own frames.""" + seen: set[int] = set() + pending = [exception] + while pending: + current = pending.pop() + if current is None or id(current) in seen: + continue + seen.add(id(current)) + traceback = current.__traceback__ + while traceback is not None: + if traceback.tb_frame.f_code.co_filename != __file__: + yield traceback.tb_frame + traceback = traceback.tb_next + pending.extend((current.__cause__, current.__context__)) + + +def _locals_holding(exception, secret: str) -> list[str]: + """The frame locals, by their repr, that disclose `secret`, which is what + an error reporter that records locals would send on.""" + 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 secret in _safe_repr(value)}) + + +@contextlib.contextmanager +def _a_closed_loopback_port(): + """A loopback port that nothing listens on. It is held for the test, so + no other process can take it.""" + holder = socket.socket() + try: + holder.bind(("127.0.0.1", 0)) + yield holder.getsockname()[1] + finally: + holder.close() + + +def test_a_refused_connection_keeps_no_frame_that_holds_the_key(monkeypatch): + """Copilot's review of openDox-code#63 at `d240fd50`, over the real + transport. A cause chained from inside `urllib` keeps frames whose locals + hold the request's headers, and so the key. The refusal chains nothing, + and no frame it keeps holds the key.""" + monkeypatch.setenv(ENV_NAME, KEY_SENTINEL) + with _a_closed_loopback_port() as closed: + binding = _built_in_binding( + endpoint=f"http://127.0.0.1:{closed}/v1/chat/completions") + port = provider_mod.BrokeredProviderPort( + binding, install_mod.brokered_catalog(binding), + runner=_refusing_runner, notice=lambda _text: None) + envelope = _Envelope() + with pytest.raises(provider_mod.BrokerRefused) as caught: + port.dispatch(envelope) + assert caught.value.diagnostic == provider_mod.DIAG_PROVIDER_UNREACHABLE + assert caught.value.__cause__ is None + assert caught.value.__context__ is None + assert _locals_holding(caught.value, KEY_SENTINEL) == [] + + +def test_an_unpresentable_value_leaves_no_frame_that_holds_it(monkeypatch): + """A value refused as unpresentable can still be most of a key, such as + a key with a line break after it. The frame that read it lets it go + before the refusal is raised.""" + monkeypatch.setenv(ENV_NAME, KEY_SENTINEL + "\n") + port, opener = _unbrokered_port(_built_in_binding(), _chat_completion()) + envelope = _Envelope() + with pytest.raises(provider_mod.BrokerRefused) as caught: + port.dispatch(envelope) + assert caught.value.diagnostic == provider_mod.DIAG_REFERENCE_UNRESOLVED + assert opener.requests == [] + assert _locals_holding(caught.value, KEY_SENTINEL) == [] + + +def test_a_broker_refusal_keeps_no_frame_that_holds_the_token(tmp_path): + """The minted token travels as the same wrapper, so the provider-call + frame holds no raw token, as it held none when that frame took a + `MintedToken`.""" + port, _opener = _port(tmp_path, OSError("unreachable")) + envelope = _Envelope() + with pytest.raises(provider_mod.BrokerRefused) as caught: + port.dispatch(envelope) + assert caught.value.diagnostic == provider_mod.DIAG_PROVIDER_UNREACHABLE + assert _locals_holding(caught.value, SENTINEL_TOKEN) == [] + + def test_the_resolver_lives_in_the_provider_module_alone(): """R1Q17 (b): "inside `doxbench_provider.py` only". The record parses a reference's FORM and reads no environment and no keyring. No other module From 286655f3082a8e7048c713794531a57de68b5d7e Mon Sep 17 00:00:00 2001 From: Brett Heap <1513478+brettheap@users.noreply.github.com> Date: Mon, 28 Sep 2026 23:21:55 +0000 Subject: [PATCH 13/20] T080: a built-in credential over plain http:// uses no proxy Copilot's review of openDox-code#63 at 1b0fb3f4: the opener for a built-in credential still installed urllib's ProxyHandler, which reads the environment's proxies. Measured: with http_proxy set, a request addressed to 127.0.0.1 went to the proxy, credential header and all. A plain-http route is private only because it stays on this host, so the loopback ruling of 2026-09-28 is broken by any proxy. _open_without_redirects becomes _open_for_a_built_in_credential. It still declines every redirect, and a plain-http request now goes direct through ProxyHandler({}), whatever the environment names. An https:// request may still use the environment's proxy, because a proxy reaches it only by CONNECT and the credential stays inside TLS. The broker path keeps the default opener, as the ruling leaves that path. One new case over real sockets, with a stand-in proxy that hears nothing. With the bypass removed, the stand-in proxy answered the turn, so the mutant was 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 | 41 ++++++++++++++++++++--------- tests/test_model_provider_broker.py | 33 ++++++++++++++++++++--- 2 files changed, 58 insertions(+), 16 deletions(-) diff --git a/src/opendox/doxbench_provider.py b/src/opendox/doxbench_provider.py index 9e692db8..39a440cb 100644 --- a/src/opendox/doxbench_provider.py +++ b/src/opendox/doxbench_provider.py @@ -903,11 +903,26 @@ def redirect_request(self, req, fp, code, msg, headers, newurl): raise _Redirected -def _open_without_redirects(request, *, timeout): - """`urllib.request.urlopen`, but every redirect is declined. The opener - is built per call, as `urlopen` builds its own on first use, so the - proxy environment is read when a request is made.""" - return urllib.request.build_opener(_DeclineRedirects).open( +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. + + * EVERY REDIRECT IS DECLINED (`_DeclineRedirects`). + * A PLAIN-`http://` REQUEST GOES DIRECT, whatever proxy the environment + names (Copilot's review of openDox-code#63 at `1b0fb3f4`). Such a + route is private only because it stays on this host, and a proxy + would carry it, in cleartext, to wherever the proxy is. Measured: with + `http_proxy` set, urllib's default opener sends a request addressed + to `127.0.0.1` to the proxy, credential header and all. An `https://` + request may still use the environment's proxy, because a proxy + reaches it only by CONNECT, and the credential stays inside TLS. + + The opener is built per call, as `urlopen` builds its own on first use, + so the proxy environment is read when a request is made.""" + handlers: list = [_DeclineRedirects] + if request.type == "http": + handlers.insert(0, urllib.request.ProxyHandler({})) + return urllib.request.build_opener(*handlers).open( request, timeout=timeout) @@ -1205,13 +1220,13 @@ def _dispatch_without_a_broker(self, *, model: str, prompt: str) -> str: repeat: the reference names the same value on a second read, so a retry would buy a second paid call for the same refusal. - A REQUEST CARRYING A BUILT-IN CREDENTIAL FOLLOWS NO REDIRECT. The - default opener follows redirects and re-sends the credential header - (see `_DeclineRedirects`), so such a request swaps it for - `_open_without_redirects`. 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 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 @@ -1233,7 +1248,7 @@ def _dispatch_without_a_broker(self, *, model: str, prompt: str) -> str: self._available = True opener = self._opener if credential is not None and opener is urllib.request.urlopen: - opener = _open_without_redirects + opener = _open_for_a_built_in_credential failure = None try: return _post_to_provider( diff --git a/tests/test_model_provider_broker.py b/tests/test_model_provider_broker.py index 73a9d010..5943b157 100644 --- a/tests/test_model_provider_broker.py +++ b/tests/test_model_provider_broker.py @@ -2571,15 +2571,15 @@ def test_a_stand_in_server_sees_the_resolved_bearer_or_no_header(which, class _ElsewhereHandler(http.server.BaseHTTPRequestHandler): - """A second stand-in, where a redirect would lead. It records every - request it is sent, of any method.""" + """A second stand-in, where a redirect or a proxy would lead. It records + every request it is sent, of any method.""" seen: list = [] def _record(self): _ElsewhereHandler.seen.append( (self.command, self.headers.get("Authorization"))) - _answer_json(self, _chat_completion("followed a redirect")) + _answer_json(self, _chat_completion("answered from elsewhere")) def do_GET(self): # noqa: N802 - BaseHTTPRequestHandler's own spelling self._record() @@ -2633,6 +2633,33 @@ def test_a_built_in_credential_follows_no_redirect(monkeypatch, code): assert _ElsewhereHandler.seen == [], "the credential went nowhere else" +def test_a_built_in_credential_over_http_to_this_host_uses_no_proxy( + monkeypatch): + """Copilot's review of openDox-code#63 at `1b0fb3f4`, over real sockets. + A plain-http route is private only because it stays on this host. With + `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, \ + _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), + runner=_refusing_runner, notice=lambda _text: None, + environ={ENV_NAME: KEY_SENTINEL}) + answer = port.dispatch(_Envelope()) + assert answer["assistant_prose"] == "answered in the chat grammar" + assert _ChatCompletionsHandler.seen["authorization"] == ( + f"Bearer {KEY_SENTINEL}") + assert _ElsewhereHandler.seen == [], "the proxy heard nothing" + + def _safe_repr(value) -> str: try: return repr(value) From 3f14bb961982ebdde13284d52115238a732c3ee5 Mon Sep 17 00:00:00 2001 From: Brett Heap <1513478+brettheap@users.noreply.github.com> Date: Mon, 28 Sep 2026 23:36:31 +0000 Subject: [PATCH 14/20] T080: a broker's reference may not take a built-in form; keyring failures keep no context Two remarks in Copilot's overview of openDox-code#63 at 286655f3. Its threads were none. - "Broker-returned reserved references can raise uncaught errors." T080 reserves the env: and keyring: forms for the built-in resolver. A broker whose intake answer carried a reference in one of them would make the next declaration of the record see two resolvers. The console's intake route replaces the reference outside the handler that catches a refused binding, so there it would be an uncaught error. hand_off_credential now refuses such an answer as malformed (DIAG_BROKER_MALFORMED), which both entry points already catch. serve_workbench.py is left alone. - "Keyring failures retain exception context." The resolver raised its keyring refusal inside the handler, "from None", which hides the backend's error but keeps it as __context__, together with the backend's frames. The refusal is now raised after the handler, with no context. One new case with two real broker scripts, and one assertion added to the failing-backend case. Two mutants were each 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 | 29 ++++++++++++++++++++++++++--- tests/test_model_provider_broker.py | 28 ++++++++++++++++++++++++++++ 2 files changed, 54 insertions(+), 3 deletions(-) diff --git a/src/opendox/doxbench_provider.py b/src/opendox/doxbench_provider.py index 39a440cb..ab0956dd 100644 --- a/src/opendox/doxbench_provider.py +++ b/src/opendox/doxbench_provider.py @@ -590,11 +590,22 @@ def hand_off_credential(binding, source, *, Returns the `reference` the broker gives back — the declaration's own field name (0.2 FINDING 4). That reference is the only thing that then lives in a - binding, in a file, in a log or in a review.""" + binding, in a file, in a log or in a review. + + THE BUILT-IN FORMS ARE RESERVED (#1144 box 16.3; Copilot's overview of + openDox-code#63 at `286655f3`). A broker's reference in the `env:` or + `keyring:` form would make the record read it as the built-in resolver's, + 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) document = _answer_document(answer, BROKER_INTAKE_KIND, INTAKE_FIELDS) - return _declared_string(document, "reference") + reference = _declared_string(document, "reference") + if binding_mod.names_a_built_in_form(reference): + raise BrokerRefused(DIAG_BROKER_MALFORMED) + return reference # --------------------------------------------------------------------------- @@ -686,6 +697,12 @@ def _presentable(value: object) -> bool: and all("!" <= character <= "~" for character in value)) +#: What a keyring backend's failure reads as, inside the resolver only. A +#: sentinel, not None, because None is what a backend answers for an absent +#: entry, which is a different refusal. +_UNREADABLE = object() + + def _os_keyring(): """The OS keyring, through the `keyring` package, imported at call time. @@ -753,7 +770,13 @@ def resolve_credential_reference(binding, *, environ=None, value = backend.get_password(reference.name, reference.user) # A keyring backend's own error, of any class, never reaches a caller. except Exception: # noqa: BLE001 - raise BrokerRefused(DIAG_KEYRING_UNAVAILABLE) from None + value = _UNREADABLE + if value is _UNREADABLE: + # Raised outside the handler, so the refusal keeps no context + # (Copilot's overview of openDox-code#63 at `286655f3`): the + # backend's own frames may hold what it was decoding when it + # failed. + raise BrokerRefused(DIAG_KEYRING_UNAVAILABLE) if not _presentable(value): # The refusal's traceback keeps this frame, so what was read leaves # it first: a value that cannot be presented can still be most of a diff --git a/tests/test_model_provider_broker.py b/tests/test_model_provider_broker.py index 5943b157..bfd7eda5 100644 --- a/tests/test_model_provider_broker.py +++ b/tests/test_model_provider_broker.py @@ -2280,6 +2280,31 @@ def test_the_resolver_reads_a_key_for_a_private_route(): assert environ.read == [ENV_NAME] +def test_a_broker_reference_in_a_built_in_form_is_malformed(tmp_path): + """The built-in forms are reserved. A broker's reference in one would + make the record read it as the built-in resolver's, beside the broker + that holds the credential: two resolvers, refused where neither entry + point expects a refusal (Copilot's overview of openDox-code#63 at + `286655f3`). So the broker's answer is malformed, and it is refused as + one, which both entry points already catch.""" + for index, reserved in enumerate(BUILT_IN_REFERENCES): + script = tmp_path / f"reserved-broker-{index}.py" + script.write_text( + "import json,sys\nsys.stdin.read()\n" + "print(json.dumps({'schema_version':1," + "'kind':'openprofiler_broker_intake','reference':" + + repr(reserved) + "," + "'binding':'b','provider':'p','auth_kind':'api_key','label':None," + "'created_at':'x','max_lifetime_seconds':300,'issued_by':'i'," + "'approved_by':'a','audit_ref':'opaud-x'}))\n", + encoding="utf-8") + binding = _broker_binding(script) + stdin = io.StringIO("x") + with pytest.raises(provider_mod.BrokerRefused) as caught: + provider_mod.hand_off_credential(binding, stdin) + 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): @@ -2465,6 +2490,9 @@ def test_a_keyring_that_cannot_be_read_refuses_and_says_nothing_of_its_own(): assert caught.value.diagnostic == provider_mod.DIAG_KEYRING_UNAVAILABLE assert "leaks" not in str(caught.value) assert caught.value.__cause__ is None + # no context either: the backend's own frames are not kept (Copilot's + # overview of openDox-code#63 at `286655f3`) + assert caught.value.__context__ is None assert opener.requests == [] From 645280aca24ccae90341de470a20c3b2a7062545 Mon Sep 17 00:00:00 2001 From: Brett Heap <1513478+brettheap@users.noreply.github.com> Date: Sat, 3 Oct 2026 00:08:48 +0000 Subject: [PATCH 15/20] T080: refuse a raw key's shape, fix the scheme refusal, map http.client errors The adversarial review of openDox-code#63 at 4948e6dd found four ways a key still reached a stored record, a refusal or a kept frame. M2. A key given as --credential-ref was taken as a broker's reference: stored, printed by list, and put in the broker's argv at each mint. The record now asks the product's detector and a raw key's SHAPE of every reference (has_a_raw_key_shape: a mixed-case base62 run of 32, a two-class run of 40, a known key prefix with 16 key characters after it, a Google API key, an AWS access key id, a JSON Web Token), and refuses one that has either with a fixed sentence, CREDENTIAL_REF_IS_A_RAW_KEY. A reference a broker hands back is held to the same rule, as a malformed answer. The detector is quadratic in what it is given (an opref- reference of 65,536 characters took it 4.3 s), and a reference had no length bound. So a reference is held to the endpoint's bound first, with its own fixed sentence, CREDENTIAL_REF_TOO_LONG, and carries_a_raw_key never asks the detector of a longer text. M3. The scheme refusal repeated the endpoint, so --endpoint , ftp:// and " https://...?q=" printed the key to the terminal, startup's standard error and the browser. It is a fixed sentence now, ENDPOINT_SCHEME_REFUSED, composed from ENDPOINT_SCHEMES alone. M5. A key in the endpoint's path or fragment passed the detector, which reads a URL's userinfo and its parameters' names. The endpoint is asked the same shape, before the scheme, and refused with the fixed sentence. L4. A status line, a protocol, a header, a body or a path http.client cannot read or send raises an http.client.HTTPException, which is no OSError, so it escaped dispatch with the request's headers, and the key, in do_open's frame. The transport maps it to the unreachable sentence, and the built-in path raises that afresh as before. A case per subclass (BadStatusLine, UnknownProtocol, LineTooLong, HTTPException, IncompleteRead, InvalidURL) runs over a real loopback server. Every new case fails at 4948e6dd, and every mutant of the new checks is killed by the broker module. 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 | 140 ++++++++++- src/opendox/doxbench_provider.py | 14 +- tests/test_model_provider_broker.py | 348 ++++++++++++++++++++++++++++ 3 files changed, 490 insertions(+), 12 deletions(-) diff --git a/src/opendox/doxbench_binding.py b/src/opendox/doxbench_binding.py index 57359d2e..dfe5f36a 100644 --- a/src/opendox/doxbench_binding.py +++ b/src/opendox/doxbench_binding.py @@ -274,10 +274,47 @@ def is_a_private_route(endpoint: object) -> bool: #: repeated the URL would print the key to a terminal, a log, or, through the #: console's intake route, a browser. ENDPOINT_CARRIES_A_CREDENTIAL = ( - "the endpoint carries a credential (a user name or password in the URL, or " - "a credential-shaped query or fragment parameter), and a binding is safe to " - "commit only because it holds none; declare the endpoint without it, and " - "name the credential by its reference in credential_ref") + "the endpoint carries a credential (a user name or password in the URL, a " + "credential-shaped query or fragment parameter, or a run with a raw key's " + "shape anywhere in it), and a binding is safe to commit only because it " + "holds none; declare the endpoint without it, and name the credential by " + "its reference in credential_ref") + +#: The refusal an endpoint outside `ENDPOINT_SCHEMES` earns. FIXED, like the +#: two around it, because a value in the wrong field can be a key: at T080's +#: `4948e6dd` this refusal repeated the endpoint, so `--endpoint `, +#: `ftp://` and `" https://…?q="` each printed the key to the +#: terminal, to startup's standard error and, through the console's intake +#: route, to a browser (the adversarial review of openDox-code#63, M3). It is +#: composed from `ENDPOINT_SCHEMES` alone. +ENDPOINT_SCHEME_REFUSED = ( + "the endpoint does not begin with one of " + " or ".join(ENDPOINT_SCHEMES) + + ", and it is not repeated here, since a value in the wrong field can be " + "a key; declare an https:// endpoint, or an http:// one on this host") + +#: The refusal a reference with a raw key's shape earns (#1144 box 16.3: a +#: reference is never a raw key). At T080's `4948e6dd` any value that was not +#: a built-in form was taken as a broker's reference, so +#: `--credential-ref ` stored the key in the bindings file, `list` printed +#: it, and each mint put it in the broker's argv, which `/proc//cmdline` +#: shows to every local user (the adversarial review of openDox-code#63, M2). +#: FIXED, so the refusal does not print the key either. +CREDENTIAL_REF_IS_A_RAW_KEY = ( + "credential_ref has the shape of a raw key, not of a reference, and it is " + "not repeated here; give the key to its custodian (the broker's intake, " + "through model-binding set-credential, or the environment or keyring the " + "built-in resolver reads), and declare the reference it is known by") + +#: The refusal a reference longer than the product's URL bound earns. Asking +#: the detector of a reference (M2 above) asks a quadratic check of a value +#: that was unbounded: measured locally, an `opref-` reference of 65,536 +#: characters took the detector 4.3 s. So a reference is held to the +#: endpoint's bound, for the endpoint's reason, before it is asked. Like the +#: two above, this refusal repeats nothing of the value. +CREDENTIAL_REF_TOO_LONG = ( + "credential_ref is longer than {bound} characters and is refused unread: " + "the credential check is quadratic in what it is given, and a reference " + "names a credential in fewer") #: The refusal an endpoint longer than the product's URL bound earns (#1144 box #: 16.3; Copilot's overview of openDox-code#63). The detector below is @@ -304,6 +341,81 @@ def _endpoint_bound() -> int: return config.MAX_REMOTE_URL_CHARS +#: A RAW KEY'S SHAPE (the adversarial review of openDox-code#63 at `4948e6dd`, +#: M2 and M5). The detector below reads a URL's userinfo and its parameters' +#: names, and nothing else. So a key pasted as a broker's reference, or +#: carried in an endpoint's path or fragment, passed it and was stored. A key +#: has no grammar, so this is a SHAPE, and it errs toward refusing. A text has +#: one if any of these is in it: +#: * a run of 32 or more letters and digits that mixes upper case, lower +#: case and digits, as a base62 secret does; +#: * a run of 40 or more letters and digits that mixes two of those +#: three, as a long hex secret does; +#: * a widely used key prefix (`sk-`, `ghp_`, `xoxb-`, `hf_` and the rest of +#: `_KEY_PREFIXED`) with 16 or more key characters after it, which mix +#: all three, or two where the prefix begins a word. So `bot` and a key +#: glued together are still refused, and a word that merely ends in a +#: prefix, as `benchmark-` ends in `rk-`, is not; +#: * a Google API key, an AWS access key id, or a JSON Web Token. +#: It passes the shapes references and endpoints are known to take: this +#: product's own `opref-` references (24 lowercase hex), UUIDs, model and +#: deployment names under 32 characters, and the 32-character lowercase hex +#: ids that gateways put in their paths and key vaults in their secrets' +#: versions. The tests list both sides. +_KEY_RUN = re.compile(r"[A-Za-z0-9]{32,}", re.ASCII) +_KEY_PREFIXED = re.compile( + r"(?:sk|rk|ghp|gho|ghu|ghs|ghr|github_pat|glpat|xox[abprs]|hf|gsk|pplx" + r"|nvapi|xai)[-_](?P[A-Za-z0-9_-]{16,})", re.ASCII) +_KEY_FORMATS = tuple(re.compile(pattern, re.ASCII) for pattern in ( + r"AIza[A-Za-z0-9_-]{35}", # a Google API key + r"(?:AKIA|ASIA)[A-Z0-9]{16}", # an AWS access key id + r"eyJ[A-Za-z0-9_-]{10,}\.[A-Za-z0-9_-]{2,}\.", # a JSON Web Token +)) + + +def _character_classes(text: str) -> int: + """How many of upper case, lower case and digits `text` holds.""" + return sum(any(test(c) for c in text) + for test in (str.isupper, str.islower, str.isdigit)) + + +def _begins_a_word(text: str, at: int) -> bool: + return at == 0 or not text[at - 1].isalnum() + + +def has_a_raw_key_shape(text: object) -> bool: + """Whether `text` holds a raw key's shape (the rules above `_KEY_RUN`).""" + if not isinstance(text, str): + return False + for match in _KEY_RUN.finditer(text): + classes = _character_classes(match.group()) + if classes == 3 or (classes == 2 and len(match.group()) >= 40): + return True + for match in _KEY_PREFIXED.finditer(text): + needed = 2 if _begins_a_word(text, match.start()) else 3 + if _character_classes(match.group("tail")) >= needed: + return True + return any(pattern.search(text) for pattern in _KEY_FORMATS) + + +def carries_a_raw_key(text: object) -> bool: + """Whether `text` carries a credential by either test the record asks: + the product's URL detector (`_carries_a_credential`), or a raw key's + shape anywhere in it (`has_a_raw_key_shape`). The record asks it of the + endpoint and of the reference, and `doxbench_provider` asks it of a + reference a broker hands back. + + A TEXT PAST THE PRODUCT'S URL BOUND IS TAKEN TO CARRY ONE, and the + detector is not asked: its work grows with the square of what it is + given (`CREDENTIAL_REF_TOO_LONG`). The record refuses such a value with + its own sentence first, so this is the floor for any other caller.""" + if not isinstance(text, str): + return False + if len(text) > _endpoint_bound(): + return True + return _carries_a_credential(text) or has_a_raw_key_shape(text) + + def _carries_a_credential(text: str) -> bool: """The product's ONE detector, `runtime/local_git_adapter. carries_a_credential`, which #1144 box 16.3 names: it flags a URL with @@ -550,17 +662,17 @@ def _require_a_declarable_endpoint(self) -> None: The length is checked first, because the detector's work grows with the square of what it is given. A KEY INSIDE THE URL IS REFUSED NEXT, - so no later refusal, the scheme's among them, can repeat a URL that - carries one.""" + whether the detector finds it or its shape does, in the path and the + fragment as well (the adversarial review of openDox-code#63, M5). + Every refusal here is a fixed sentence, the scheme's too (M3), so + none can repeat a URL that carries one.""" if len(self.endpoint) > _endpoint_bound(): raise BindingRefused(ENDPOINT_TOO_LONG.format( bound=_endpoint_bound())) - if _carries_a_credential(self.endpoint): + if carries_a_raw_key(self.endpoint): raise BindingRefused(ENDPOINT_CARRIES_A_CREDENTIAL) if not self.endpoint.startswith(ENDPOINT_SCHEMES): - raise BindingRefused( - f"endpoint {self.endpoint!r} does not name one of " - f"{ENDPOINT_SCHEMES}") + raise BindingRefused(ENDPOINT_SCHEME_REFUSED) def _require_a_private_route(self) -> None: """A CREDENTIAL THE BUILT-IN RESOLVER READS TRAVELS ONLY BY A PRIVATE @@ -602,6 +714,14 @@ def _require_one_resolver(self, argv: tuple[str, ...]) -> None: f"takes no credential declares the auth kind " f"{AUTH_KIND_NONE!r} instead") _require_non_blank_str("credential_ref", self.credential_ref) + # A REFERENCE IS NEVER A RAW KEY (the adversarial review of + # openDox-code#63, M2), in either form. Its length is checked first, + # as the endpoint's is, and both refusals are fixed. + if len(self.credential_ref) > _endpoint_bound(): + raise BindingRefused(CREDENTIAL_REF_TOO_LONG.format( + bound=_endpoint_bound())) + if carries_a_raw_key(self.credential_ref): + raise BindingRefused(CREDENTIAL_REF_IS_A_RAW_KEY) if built_in_reference_parts(self.credential_ref) is not None: if argv: raise BindingRefused( diff --git a/src/opendox/doxbench_provider.py b/src/opendox/doxbench_provider.py index ab0956dd..bb5f75fb 100644 --- a/src/opendox/doxbench_provider.py +++ b/src/opendox/doxbench_provider.py @@ -98,6 +98,7 @@ from __future__ import annotations import dataclasses +import http.client import json import os import shutil @@ -603,7 +604,11 @@ def hand_off_credential(binding, source, *, source=source) document = _answer_document(answer, BROKER_INTAKE_KIND, INTAKE_FIELDS) reference = _declared_string(document, "reference") - if binding_mod.names_a_built_in_form(reference): + # A reference with a raw key's shape is malformed too, for the same + # reason: the record refuses it (the adversarial review of + # openDox-code#63, M2), where neither entry point expects a refusal. + if (binding_mod.names_a_built_in_form(reference) + or binding_mod.carries_a_raw_key(reference)): raise BrokerRefused(DIAG_BROKER_MALFORMED) return reference @@ -1005,7 +1010,12 @@ def _post_to_provider(*, endpoint: str, dialect: str, if status == PROVIDER_STATUS_TOKEN_EXPIRED: raise _TokenExpired from None raise BrokerRefused(DIAG_PROVIDER_REFUSED) from None - except (urllib.error.URLError, OSError, ValueError) as error: + # An `http.client.HTTPException` too: a status line, a protocol or a + # header line `http.client` cannot read raises one, which is no OSError, + # and it escaped with the request's headers in `do_open`'s frame (the + # adversarial review of openDox-code#63, L4). + except (urllib.error.URLError, http.client.HTTPException, OSError, + ValueError) as error: raise BrokerRefused(DIAG_PROVIDER_UNREACHABLE) from error if not isinstance(payload, (bytes, bytearray)): raise BrokerRefused(DIAG_PROVIDER_MALFORMED) diff --git a/tests/test_model_provider_broker.py b/tests/test_model_provider_broker.py index bfd7eda5..b5102b7d 100644 --- a/tests/test_model_provider_broker.py +++ b/tests/test_model_provider_broker.py @@ -48,6 +48,7 @@ import contextlib import dataclasses +import http.client import http.server import io import json @@ -58,6 +59,7 @@ import time import types import urllib.error +import urllib.request from datetime import datetime, timezone from pathlib import Path @@ -1967,6 +1969,221 @@ def test_a_stored_document_whose_endpoint_carries_a_key_does_not_read( assert KEY_SENTINEL not in str(caught.value) +# --- a raw key's shape (the adversarial review of openDox-code#63) ------- +# +# The adversarial review at `4948e6dd` found three ways a key still reached +# the record or a refusal. M2: a key given as a broker's reference was taken +# as one, stored, printed by `list`, and put in the broker's argv at each +# mint. M3: the scheme refusal repeated the endpoint, key and all. M5: a key +# in the endpoint's path or fragment passed the detector, which reads a URL's +# userinfo and its parameters' names. Every value below is an obvious fake, +# built from fragments, so no line of this file holds a key's format whole. + +_STAND_IN_BASE62 = "StandIn0NotAKey0" * 2 # 32 characters, all three classes +_STAND_IN_HEX = "0123456789abcdef" * 2 # 32 characters, two classes + +#: Each holds a raw key's shape by one rule of `has_a_raw_key_shape` alone, +#: so a rule that is dropped or narrowed is a case that fails. +RAW_KEY_SHAPES = { + "base62-run-of-32": _STAND_IN_BASE62, + "hex-run-of-40": _STAND_IN_HEX + "01234567", + "prefix-and-hex": "sk" + "-" + _STAND_IN_HEX, + "prefix-and-a-tail-of-16": "sk" + "-" + _STAND_IN_HEX[:16], + "prefix-and-letters": "hf" + "_" + "StandInNotAKeyStandIn", + "prefix-glued-to-a-word": "bot" + "sk" + "-" + "Stand-In-0000-NOT-A-KEY", + "google-api-key": "AIza" + "-stand-in-NOT-a-KEY-0000-0000-00000", + "aws-access-key-id": "AKIA" + "STANDIN0NOTAKEY0", + "json-web-token": "eyJ" + "hbGciOiJub25lIn0" + "." + "e30" + ".", +} + +#: The shapes references and endpoints are known to take, and each rule's +#: edge. None of them is refused. +NOT_RAW_KEY_SHAPES = { + "opendox-reference": FAKE_REFERENCE, + "zeroed-reference": "opref-" + "0" * 24, + "console-placeholder": "pending-broker-intake", + "uuid": "123e4567-e89b-12d3-a456-426614174000", + "model-name-under-32": "GPT4oMiniProduction2024", + "hex-run-of-32": _STAND_IN_HEX, + "hex-run-of-39": (_STAND_IN_HEX + "01234567")[:39], + "base62-run-of-31": _STAND_IN_BASE62[:31], + "a-word-ending-in-a-prefix": "benchmark-runner-" + _STAND_IN_HEX[:16], + "prefix-and-a-tail-of-15": "sk" + "-" + _STAND_IN_HEX[:15], + "secret-manager-reference": "op://dev/5vtmcbtqbkxhvdl3ezm2l3lvsa/password", +} + +#: A key as a provider issues one: a prefix, and a base62 body. +_STAND_IN_PROVIDER_KEY = "sk" + "-proj-" + _STAND_IN_BASE62 + + +@pytest.mark.parametrize("shape", sorted(RAW_KEY_SHAPES)) +def test_a_raw_keys_shape_is_read_by_each_rule(shape): + assert binding_mod.has_a_raw_key_shape(RAW_KEY_SHAPES[shape]) is True + assert binding_mod.carries_a_raw_key(RAW_KEY_SHAPES[shape]) is True + + +@pytest.mark.parametrize("shape", sorted(NOT_RAW_KEY_SHAPES)) +def test_the_shapes_references_take_are_not_a_raw_keys(shape): + """The other side, at each rule's edge: a run one character short, a + tail one short, a prefix that only ends a word, and the ids gateways and + secret managers use.""" + assert binding_mod.has_a_raw_key_shape(NOT_RAW_KEY_SHAPES[shape]) is False + assert binding_mod.carries_a_raw_key(NOT_RAW_KEY_SHAPES[shape]) is False + + +def test_only_text_has_a_raw_keys_shape(): + for value in (None, 0, _STAND_IN_BASE62.encode("ascii"), + [_STAND_IN_BASE62]): + assert binding_mod.has_a_raw_key_shape(value) is False + assert binding_mod.carries_a_raw_key(value) is False + + +@pytest.mark.parametrize("shape", sorted(RAW_KEY_SHAPES)) +def test_a_reference_with_a_raw_keys_shape_is_refused_and_never_repeated( + shape): + """M2. A reference is never a raw key (#1144 box 16.3), so a value with + a raw key's shape is refused as a broker's reference and inside each + built-in form, and the refusal is a fixed sentence.""" + key = RAW_KEY_SHAPES[shape] + for declare in (lambda: _binding(credential_ref=key), + lambda: _built_in_binding(f"env:{key}"), + lambda: _built_in_binding( + f"keyring:{KEYRING_SERVICE}/{key}")): + with pytest.raises(binding_mod.BindingRefused) as caught: + declare() + assert str(caught.value) == binding_mod.CREDENTIAL_REF_IS_A_RAW_KEY + assert key not in str(caught.value) + + +@pytest.mark.parametrize("shape", sorted( + name for name, value in NOT_RAW_KEY_SHAPES.items() + if not binding_mod.names_a_built_in_form(value))) +def test_a_reference_shaped_value_is_still_a_brokers_reference(shape): + reference = NOT_RAW_KEY_SHAPES[shape] + assert _binding(credential_ref=reference).credential_ref == reference + + +def test_a_reference_past_the_url_bound_is_refused_before_the_detector( + monkeypatch): + """Asking the detector of a reference (M2) asks a quadratic check of a + value that had no bound, so a reference is held to the endpoint's bound + first, and the detector is never asked of a longer one. The refusal + repeats nothing of it.""" + bound = runtime_config.MAX_REMOTE_URL_CHARS + at_the_bound = "opref-" + "0" * (bound - len("opref-")) + past_the_bound = at_the_bound + "0" + asked = [] + detector = git_adapter_mod.carries_a_credential + + def _recording(text): + asked.append(text) + return detector(text) + + monkeypatch.setattr(git_adapter_mod, "carries_a_credential", _recording) + with pytest.raises(binding_mod.BindingRefused) as caught: + _binding(credential_ref=past_the_bound) + assert str(caught.value) == binding_mod.CREDENTIAL_REF_TOO_LONG.format( + bound=bound) + assert past_the_bound not in asked + assert _binding(credential_ref=at_the_bound).credential_ref == ( + at_the_bound) + assert at_the_bound in asked + + +def test_a_text_past_the_url_bound_carries_a_key_unasked(monkeypatch): + """The predicate's own floor, for a caller that does not bound what it + asks about, as `doxbench_provider` asks of a broker's reference.""" + def _not_asked(_text): + raise AssertionError("the detector was asked about a text past the " + "bound") + + monkeypatch.setattr(git_adapter_mod, "carries_a_credential", _not_asked) + past_the_bound = "x" * (runtime_config.MAX_REMOTE_URL_CHARS + 1) + assert binding_mod.has_a_raw_key_shape(past_the_bound) is False + assert binding_mod.carries_a_raw_key(past_the_bound) is True + + +def test_a_stored_document_whose_reference_is_a_raw_key_does_not_read( + tmp_path): + """M2 for a record written before the rule: it no longer reads, as one + whose endpoint carries a key does not, and the refusal does not repeat + the key.""" + record = _binding().as_record() + record["credential_ref"] = _STAND_IN_PROVIDER_KEY + path = tmp_path / "bindings.yaml" + path.write_text(json.dumps({"schema_version": 1, + "kind": binding_mod.BINDINGS_KIND, + "bindings": [record]}), encoding="utf-8") + with pytest.raises(binding_mod.BindingRefused) as caught: + binding_mod.BindingStore(path).list() + assert str(caught.value) == binding_mod.CREDENTIAL_REF_IS_A_RAW_KEY + assert _STAND_IN_PROVIDER_KEY not in str(caught.value) + + +@pytest.mark.parametrize("endpoint", [ + "provider.invalid/turn", + "file:///etc/passwd", + "ftp://provider.invalid/turn", + " https://provider.invalid/turn", + # a short key in the wrong field, which no shape rule knows + "sk" + "-" + "stand-in", +]) +def test_the_scheme_refusal_is_a_fixed_sentence_that_repeats_nothing( + endpoint): + """M3. The scheme refusal repeated the endpoint, and a value in the + wrong field can be a key, a key no shape rule knows among them. It is a + fixed sentence now, composed from `ENDPOINT_SCHEMES` alone.""" + with pytest.raises(binding_mod.BindingRefused) as caught: + _binding(endpoint=endpoint) + message = str(caught.value) + assert message == binding_mod.ENDPOINT_SCHEME_REFUSED + assert endpoint.strip() not in message + for scheme in binding_mod.ENDPOINT_SCHEMES: + assert scheme in message + + +#: Where a key was carried past the detector (M5), and the reviewer's M3 +#: examples, where a key in the endpoint field reached the scheme refusal. +_KEYED_ENDPOINTS = { + "a-path-segment": "https://api.example.invalid/v1/{key}/chat/completions", + "glued-to-a-path-word": "https://api.example.invalid/bot{key}/v1", + "the-fragment": "https://api.example.invalid/v1/chat/completions#{key}", + "a-query-value": "https://api.example.invalid/v1/chat/completions?q={key}", + "the-whole-field": "{key}", + "behind-another-scheme": "ftp://{key}", + "behind-a-space": " https://api.example.invalid/v1?q={key}", +} + + +@pytest.mark.parametrize("key", [_STAND_IN_BASE62, _STAND_IN_PROVIDER_KEY], + ids=["base62-run", "provider-key"]) +@pytest.mark.parametrize("place", sorted(_KEYED_ENDPOINTS)) +def test_a_key_anywhere_in_the_endpoint_is_refused_and_never_repeated( + place, key): + """M5, and M3's examples. The shape is checked with the detector, before + the scheme, so a key in the path, glued to a path word, in the fragment, + in a parameter with an innocent name, or in place of the URL, is refused + with the fixed sentence.""" + with pytest.raises(binding_mod.BindingRefused) as caught: + _binding(endpoint=_KEYED_ENDPOINTS[place].format(key=key)) + assert str(caught.value) == binding_mod.ENDPOINT_CARRIES_A_CREDENTIAL + assert key not in str(caught.value) + + +@pytest.mark.parametrize("endpoint", [ + "https://stand-in.openai.azure.invalid/openai/deployments/GPT4oMini2024" + "/chat/completions?api-version=2024-02-01", + "https://gateway.ai.cloudflare.invalid/v1/" + _STAND_IN_HEX + + "/stand-in/openai/chat/completions", + "https://api.example.invalid/v1/projects/" + "123e4567-e89b-12d3-a456-426614174000/chat/completions", + "https://api.example.invalid/v1/chat/completions#section-2", +], ids=["deployment-name", "gateway-account-id", "uuid", "fragment"]) +def test_the_ids_an_endpoint_carries_are_not_keys(endpoint): + binding = _binding(endpoint=endpoint, dialect=OPENAI_CHAT) + assert binding.endpoint == endpoint + + # --- one resolver per record --------------------------------------------- @@ -2304,6 +2521,31 @@ def test_a_broker_reference_in_a_built_in_form_is_malformed(tmp_path): provider_mod.hand_off_credential(binding, stdin) assert caught.value.diagnostic == provider_mod.DIAG_BROKER_MALFORMED +@pytest.mark.parametrize("which", ["raw-key-shape", "past-the-url-bound"]) +def test_a_broker_reference_the_record_would_refuse_is_malformed(tmp_path, + which): + """M2 at the hand-off: the reference a broker hands back is held to the + record's rule before anything stores it, a key's shape and the length + bound alike, so one that breaks it is a malformed answer, which both + entry points already catch. Nothing of it is repeated.""" + reference = (_STAND_IN_PROVIDER_KEY if which == "raw-key-shape" + else "opref-" + "0" * runtime_config.MAX_REMOTE_URL_CHARS) + script = tmp_path / "keyed-reference-broker.py" + script.write_text( + "import json,sys\nsys.stdin.read()\n" + "print(json.dumps({'schema_version':1," + "'kind':'openprofiler_broker_intake','reference':" + + repr(reference) + "," + "'binding':'b','provider':'p','auth_kind':'api_key','label':None," + "'created_at':'x','max_lifetime_seconds':300,'issued_by':'i'," + "'approved_by':'a','audit_ref':'opaud-x'}))\n", + encoding="utf-8") + with pytest.raises(provider_mod.BrokerRefused) as caught: + provider_mod.hand_off_credential(_broker_binding(script), + io.StringIO("x")) + assert caught.value.diagnostic == provider_mod.DIAG_BROKER_MALFORMED + assert reference not in str(caught.value) + @pytest.mark.parametrize("endpoint", [ "http://api.example.invalid/turn", "http://localhost.evil.com/turn"]) @@ -2756,6 +2998,82 @@ def test_a_refused_connection_keeps_no_frame_that_holds_the_key(monkeypatch): assert _locals_holding(caught.value, KEY_SENTINEL) == [] +class _UnreadableAnswerHandler(http.server.BaseHTTPRequestHandler): + """A stand-in provider that reads one whole request and answers it with + `answer`, bytes `http.client` cannot read as a response.""" + + answer = b"" + + def do_POST(self): # noqa: N802 - BaseHTTPRequestHandler's own spelling + self.rfile.read(int(self.headers.get("Content-Length", "0"))) + self.wfile.write(self.answer) + self.close_connection = True + + def log_message(self, *_args): + return + + +#: The answers of the adversarial review of openDox-code#63 (L4), each under +#: the `http.client.HTTPException` it raises, with the path it is asked at. +#: `InvalidURL` needs no answer: `http.client` raises it for a path it cannot +#: send, a path the record accepts, before anything is sent. +UNREADABLE_ANSWERS = { + "BadStatusLine": (b"GARBAGE\r\n\r\n", "/v1/chat/completions"), + "UnknownProtocol": (b"HTTP/2.0 200 OK\r\nContent-Length: 2\r\n\r\n{}", + "/v1/chat/completions"), + "LineTooLong": (b"HTTP/1.1 200 OK\r\nX-Stand-In: " + b"a" * 70_000 + + b"\r\n\r\n", "/v1/chat/completions"), + "HTTPException": (b"HTTP/1.1 200 OK\r\n" + + b"".join(b"X-Stand-In-%d: y\r\n" % number + for number in range(120)) + b"\r\n", + "/v1/chat/completions"), + "IncompleteRead": (b"HTTP/1.1 200 OK\r\nTransfer-Encoding: chunked\r\n" + b"\r\n10\r\n{\"choices\":", "/v1/chat/completions"), + "InvalidURL": (b"", "/v1/chat completions"), +} + + +def _read_by_urllib_alone(url: str) -> None: + request = urllib.request.Request(url, data=b"{}", method="POST") + opener = urllib.request.build_opener(urllib.request.ProxyHandler({})) + with opener.open(request, timeout=10) as response: + response.read() + + +@pytest.mark.parametrize("resolver", ["built-in", "none"]) +@pytest.mark.parametrize("raised", sorted(UNREADABLE_ANSWERS)) +def test_an_answer_http_client_cannot_read_is_unreachable_and_keeps_no_key( + raised, resolver): + """The adversarial review of openDox-code#63 at `4948e6dd`, L4. A status + line, a protocol, a header, a body or a path that `http.client` cannot + read or send raises an `http.client.HTTPException`, which is no + `OSError`. So it escaped `dispatch`, with the request's headers, and the + key, in `do_open`'s frame. Each is the fixed unreachable refusal now, + raised afresh where a built-in credential was presented.""" + answer, path = UNREADABLE_ANSWERS[raised] + handler = type(f"_{raised}Answer", (_UnreadableAnswerHandler,), + {"answer": answer}) + with _stand_in_provider(handler) as base: + # the case is what it is named for: urllib alone raises exactly it + with pytest.raises(http.client.HTTPException) as unread: + _read_by_urllib_alone(base + path) + assert type(unread.value) is getattr(http.client, raised) + binding = (_built_in_binding(endpoint=base + path) + if resolver == "built-in" + else _none_binding(endpoint=base + path)) + port = provider_mod.BrokeredProviderPort( + binding, install_mod.brokered_catalog(binding), + runner=_refusing_runner, notice=lambda _text: None, + environ={ENV_NAME: KEY_SENTINEL}) + with pytest.raises(provider_mod.BrokerRefused) as caught: + port.dispatch(_Envelope()) + assert caught.value.diagnostic == provider_mod.DIAG_PROVIDER_UNREACHABLE + if resolver == "built-in": + assert caught.value.__cause__ is None + assert caught.value.__context__ is None + assert _locals_holding(caught.value, KEY_SENTINEL) == [] + + def test_an_unpresentable_value_leaves_no_frame_that_holds_it(monkeypatch): """A value refused as unpresentable can still be most of a key, such as a key with a line break after it. The frame that read it lets it go @@ -2877,6 +3195,36 @@ def run(*argv) -> int: assert binding_mod.BUILT_IN_REMOVAL_NOTICE in capsys.readouterr().out +def test_the_cli_refuses_a_raw_key_as_a_reference_and_repeats_nothing( + tmp_path, capsys): + """M2 at the operator door: `model-binding add --credential-ref ` + stores nothing, so `list` has nothing of it to print, and neither stream + repeats it.""" + checkout = tmp_path / "checkout" + checkout.mkdir() + parser = cli_mod.build_parser() + + def run(*argv) -> int: + args = parser.parse_args(list(argv)) + return args.func(args) + + root = ["--repo-root", str(checkout)] + assert run("model-binding", "add", *root, "--id", "keyed", "--label", "L", + "--provider", "p", "--credential-approver", + "brett@opensoft.one", + "--endpoint", "https://api.example.invalid/v1/chat/completions", + "--dialect", OPENAI_CHAT, "--auth-kind", "api_key", + "--credential-ref", _STAND_IN_PROVIDER_KEY, + "--", "openprofiler-broker") == 1 + captured = capsys.readouterr() + assert binding_mod.CREDENTIAL_REF_IS_A_RAW_KEY in captured.err + assert _STAND_IN_PROVIDER_KEY not in captured.err + captured.out + store = binding_mod.BindingStore(binding_mod.bindings_path(checkout)) + assert store.list() == () + assert run("model-binding", "list", *root) == 0 + assert _STAND_IN_PROVIDER_KEY not in capsys.readouterr().out + + class _UnreadableSource: """A standard input that must never be read.""" From e7f3a7b3415a1aec1371d297c926258d7f10c936 Mon Sep 17 00:00:00 2001 From: Brett Heap <1513478+brettheap@users.noreply.github.com> Date: Sat, 3 Oct 2026 00:20:19 +0000 Subject: [PATCH 16/20] T079: doxbench_intake's docstring names each shape that grew (Copilot at cb059d5d) Copilot's review of openDox-code#62 at cb059d5d read "the catalog's grew" and "the binding's grew" as possessives used as verbs. Each was elliptical for the shape it named, and the review shows a reader can miss that, so the paragraph now names the noun: the catalog entry's shape, and the binding's record. A docstring only. Arc: neutral-product-standalone-operability Lane: openxfactory-4 (openXfactory-4-openDox_extraction) Co-Authored-By: Claude Opus 5.5 (1M context) --- src/opendox/doxbench_intake.py | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/opendox/doxbench_intake.py b/src/opendox/doxbench_intake.py index a51d9cfa..74fead6f 100644 --- a/src/opendox/doxbench_intake.py +++ b/src/opendox/doxbench_intake.py @@ -24,11 +24,11 @@ (`doxbench_binding.BINDING_FIELDS`) and the CATALOG entry's closed public shape (`doxbench_model.DECLARABLE_ENTRY_FIELDS`). Each count is named by its tuple rather than restated here, because both shapes have grown since this -module was written. The catalog's grew twice by governing release (the routing -declaration at contract-v1.38 and the input-modality declaration at -contract-v2.2), and the binding's grew once, by `model` (#1144 box 16.2, plan -034 T079). This paragraph's claim is that THIS module widens nothing, which is -unchanged by any of them. Proposed-versus- +module was written. The catalog entry's shape grew twice by governing release +(the routing declaration at contract-v1.38 and the input-modality declaration +at contract-v2.2), and the binding's record grew once, by `model` (#1144 box +16.2, plan 034 T079). This paragraph's claim is that THIS module widens +nothing, which is unchanged by any of them. Proposed-versus- approved is a SERVER-SIDE distinction and a pending declaration is simply not in the catalog, so NEITHER SHAPE GAINS A FIELD FROM THIS MODULE and this module needs no release act. (Both statements are scoped to this module deliberately. From 82ec9a20cb49c3aef898522ad77fa19da0805f7c Mon Sep 17 00:00:00 2001 From: Brett Heap <1513478+brettheap@users.noreply.github.com> Date: Sat, 3 Oct 2026 01:01:13 +0000 Subject: [PATCH 17/20] T080: the scheme refusal says nothing of hosts (Copilot at abbb05d4) Copilot's review of openDox-code#63 at abbb05d4: ENDPOINT_SCHEME_REFUSED told every resolver to declare "an http:// one on this host". Only a built-in credential is held to a private route, and ENDPOINT_NOT_PRIVATE says so, so the advice was false for a broker's binding and for a none binding, which may still name an http:// endpoint on another host. The sentence now names the schemes alone, and a case pins it for each resolver, with the other host still declared for the two it allows. 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 | 8 ++++++-- tests/test_model_provider_broker.py | 17 +++++++++++++++++ 2 files changed, 23 insertions(+), 2 deletions(-) diff --git a/src/opendox/doxbench_binding.py b/src/opendox/doxbench_binding.py index dfe5f36a..ab589c41 100644 --- a/src/opendox/doxbench_binding.py +++ b/src/opendox/doxbench_binding.py @@ -286,11 +286,15 @@ def is_a_private_route(endpoint: object) -> bool: #: `ftp://` and `" https://…?q="` each printed the key to the #: terminal, to startup's standard error and, through the console's intake #: route, to a browser (the adversarial review of openDox-code#63, M3). It is -#: composed from `ENDPOINT_SCHEMES` alone. +#: composed from `ENDPOINT_SCHEMES` alone. AND IT SAYS NOTHING OF HOSTS +#: (Copilot's review of openDox-code#63 at `abbb05d4`): every resolver meets +#: it, and only a built-in credential is held to a private route, which +#: `ENDPOINT_NOT_PRIVATE` says, so a broker's or a `none` binding may still +#: name an `http://` endpoint on another host. ENDPOINT_SCHEME_REFUSED = ( "the endpoint does not begin with one of " + " or ".join(ENDPOINT_SCHEMES) + ", and it is not repeated here, since a value in the wrong field can be " - "a key; declare an https:// endpoint, or an http:// one on this host") + "a key; declare the endpoint's URL with one of those schemes") #: The refusal a reference with a raw key's shape earns (#1144 box 16.3: a #: reference is never a raw key). At T080's `4948e6dd` any value that was not diff --git a/tests/test_model_provider_broker.py b/tests/test_model_provider_broker.py index b5102b7d..138b6d97 100644 --- a/tests/test_model_provider_broker.py +++ b/tests/test_model_provider_broker.py @@ -2142,6 +2142,23 @@ def test_the_scheme_refusal_is_a_fixed_sentence_that_repeats_nothing( assert scheme in message +def test_the_scheme_refusal_is_route_neutral(): + """Copilot's review of openDox-code#63 at `abbb05d4`. Every resolver + meets the scheme refusal, and only a built-in credential is held to a + private route (`ENDPOINT_NOT_PRIVATE`), so the refusal names the schemes + and nothing about hosts: a broker's or a `none` binding may still name + an `http://` endpoint on another host.""" + for declare in (_binding, _built_in_binding, _none_binding): + with pytest.raises(binding_mod.BindingRefused) as caught: + declare(endpoint="ftp://provider.invalid/turn") + assert str(caught.value) == binding_mod.ENDPOINT_SCHEME_REFUSED + for word in ("host", "loopback", "localhost", "127.0.0.1"): + assert word not in binding_mod.ENDPOINT_SCHEME_REFUSED + for declare in (_binding, _none_binding): + endpoint = "http://api.example.invalid/v1/chat/completions" + assert declare(endpoint=endpoint).endpoint == endpoint + + #: Where a key was carried past the detector (M5), and the reviewer's M3 #: examples, where a key in the endpoint field reached the scheme refusal. _KEYED_ENDPOINTS = { From 67d00617b433ea816e4f0c52930328275c8ec9ca Mon Sep 17 00:00:00 2001 From: Brett Heap <1513478+brettheap@users.noreply.github.com> Date: Sat, 3 Oct 2026 01:20:56 +0000 Subject: [PATCH 18/20] T080: SonarCloud at 82ec9a20 (S5713 in the transport, S5778 x4 in tests) S5713: urllib.error.URLError is an OSError, so listing both in the transport's except tuple named one class twice. The tuple keeps OSError, and its comment says URLError is caught through it. No behaviour moves. S5778 x4: four of the adversarial review's cases built an argument inside their pytest.raises block, so a second call there could have raised the refusal under test. Each argument is built before the block. 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 | 10 +++++----- tests/test_model_provider_broker.py | 14 +++++++++----- 2 files changed, 14 insertions(+), 10 deletions(-) diff --git a/src/opendox/doxbench_provider.py b/src/opendox/doxbench_provider.py index bb5f75fb..dfd1bf86 100644 --- a/src/opendox/doxbench_provider.py +++ b/src/opendox/doxbench_provider.py @@ -1010,12 +1010,12 @@ def _post_to_provider(*, endpoint: str, dialect: str, if status == PROVIDER_STATUS_TOKEN_EXPIRED: raise _TokenExpired from None raise BrokerRefused(DIAG_PROVIDER_REFUSED) from None - # An `http.client.HTTPException` too: a status line, a protocol or a - # header line `http.client` cannot read raises one, which is no OSError, - # and it escaped with the request's headers in `do_open`'s frame (the + # `urllib.error.URLError` is an `OSError`, so it is caught here too. And + # an `http.client.HTTPException`: a status line, a protocol or a header + # line `http.client` cannot read raises one, which is no OSError, and it + # escaped with the request's headers in `do_open`'s frame (the # adversarial review of openDox-code#63, L4). - except (urllib.error.URLError, http.client.HTTPException, OSError, - ValueError) as error: + except (http.client.HTTPException, OSError, ValueError) as error: raise BrokerRefused(DIAG_PROVIDER_UNREACHABLE) from error if not isinstance(payload, (bytes, bytearray)): raise BrokerRefused(DIAG_PROVIDER_MALFORMED) diff --git a/tests/test_model_provider_broker.py b/tests/test_model_provider_broker.py index 138b6d97..9fab139a 100644 --- a/tests/test_model_provider_broker.py +++ b/tests/test_model_provider_broker.py @@ -2114,8 +2114,9 @@ def test_a_stored_document_whose_reference_is_a_raw_key_does_not_read( path.write_text(json.dumps({"schema_version": 1, "kind": binding_mod.BINDINGS_KIND, "bindings": [record]}), encoding="utf-8") + store = binding_mod.BindingStore(path) with pytest.raises(binding_mod.BindingRefused) as caught: - binding_mod.BindingStore(path).list() + store.list() assert str(caught.value) == binding_mod.CREDENTIAL_REF_IS_A_RAW_KEY assert _STAND_IN_PROVIDER_KEY not in str(caught.value) @@ -2181,8 +2182,9 @@ def test_a_key_anywhere_in_the_endpoint_is_refused_and_never_repeated( the scheme, so a key in the path, glued to a path word, in the fragment, in a parameter with an innocent name, or in place of the URL, is refused with the fixed sentence.""" + endpoint = _KEYED_ENDPOINTS[place].format(key=key) with pytest.raises(binding_mod.BindingRefused) as caught: - _binding(endpoint=_KEYED_ENDPOINTS[place].format(key=key)) + _binding(endpoint=endpoint) assert str(caught.value) == binding_mod.ENDPOINT_CARRIES_A_CREDENTIAL assert key not in str(caught.value) @@ -2557,9 +2559,10 @@ def test_a_broker_reference_the_record_would_refuse_is_malformed(tmp_path, "'created_at':'x','max_lifetime_seconds':300,'issued_by':'i'," "'approved_by':'a','audit_ref':'opaud-x'}))\n", encoding="utf-8") + binding = _broker_binding(script) + source = io.StringIO("x") with pytest.raises(provider_mod.BrokerRefused) as caught: - provider_mod.hand_off_credential(_broker_binding(script), - io.StringIO("x")) + provider_mod.hand_off_credential(binding, source) assert caught.value.diagnostic == provider_mod.DIAG_BROKER_MALFORMED assert reference not in str(caught.value) @@ -3082,8 +3085,9 @@ def test_an_answer_http_client_cannot_read_is_unreachable_and_keeps_no_key( binding, install_mod.brokered_catalog(binding), runner=_refusing_runner, notice=lambda _text: None, environ={ENV_NAME: KEY_SENTINEL}) + envelope = _Envelope() with pytest.raises(provider_mod.BrokerRefused) as caught: - port.dispatch(_Envelope()) + port.dispatch(envelope) assert caught.value.diagnostic == provider_mod.DIAG_PROVIDER_UNREACHABLE if resolver == "built-in": assert caught.value.__cause__ is None From 44582f8fcf9c7e78471fdfd3f23de0963390501f Mon Sep 17 00:00:00 2001 From: Brett Heap <1513478+brettheap@users.noreply.github.com> Date: Sat, 3 Oct 2026 01:20:56 +0000 Subject: [PATCH 19/20] T080: a keyring package that fails as it is imported refuses like a failed read (Copilot at 82ec9a20) Copilot's review of openDox-code#63 at 82ec9a20: "Keyring import failures beyond ImportError can escape without the required fixed diagnostic." An import runs the package's own code, and a backend can fail there as it can when it is read. The resolver already drops a read's error of any class; the import now drops its own the same way, and the refusal, DIAG_KEYRING_UNAVAILABLE, is raised outside the handler, so it keeps no context. A case pins it with a RuntimeError at import, and fails with 82ec9a20's ImportError-only catch. 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 | 15 ++++++++++++--- tests/test_model_provider_broker.py | 28 ++++++++++++++++++++++++++++ 2 files changed, 40 insertions(+), 3 deletions(-) diff --git a/src/opendox/doxbench_provider.py b/src/opendox/doxbench_provider.py index dfd1bf86..19ac5b6c 100644 --- a/src/opendox/doxbench_provider.py +++ b/src/opendox/doxbench_provider.py @@ -715,11 +715,20 @@ def _os_keyring(): never names a keyring reference never needs it, and one that does installs it beside openDox. Without it, a keyring reference refuses with the fixed `DIAG_KEYRING_UNAVAILABLE` rather than raising an import error out of a - turn.""" + turn. + + AND A PACKAGE THAT FAILS AS IT IS IMPORTED REFUSES THE SAME WAY (Copilot's + review of openDox-code#63 at `82ec9a20`). An import runs the package's own + code, and a backend can fail there as it can when it is read, so its + error, of any class, is dropped as a read's is. The refusal is raised + outside the handler, so it keeps no context either.""" try: import keyring - except ImportError: - raise BrokerRefused(DIAG_KEYRING_UNAVAILABLE) from None + # The package's own failure, of any class, never reaches a caller. + except Exception: # noqa: BLE001 + keyring = None + if keyring is None: + raise BrokerRefused(DIAG_KEYRING_UNAVAILABLE) return keyring diff --git a/tests/test_model_provider_broker.py b/tests/test_model_provider_broker.py index 9fab139a..3eca2b5a 100644 --- a/tests/test_model_provider_broker.py +++ b/tests/test_model_provider_broker.py @@ -46,6 +46,7 @@ from __future__ import annotations +import builtins import contextlib import dataclasses import http.client @@ -2772,6 +2773,33 @@ def test_without_the_keyring_package_a_keyring_reference_refuses(monkeypatch): assert opener.requests == [] +def test_a_keyring_package_that_fails_as_it_is_imported_refuses_the_same_way( + monkeypatch): + """Copilot's review of openDox-code#63 at `82ec9a20`. An import runs the + package's own code, and a backend can fail there as it can when it is + read. Whatever it raises, the reference refuses with the fixed sentence, + before any request, and the refusal carries none of what was raised.""" + importing = builtins.__import__ + + def _failing_import(name, *args, **kwargs): + if name == "keyring": + raise RuntimeError(f"a backend failed at import: {KEY_SENTINEL}") + return importing(name, *args, **kwargs) + + monkeypatch.setattr(builtins, "__import__", _failing_import) + port, opener = _unbrokered_port( + _built_in_binding(f"keyring:{KEYRING_SERVICE}/{KEYRING_USER}"), + _chat_completion()) + envelope = _Envelope() + with pytest.raises(provider_mod.BrokerRefused) as caught: + port.dispatch(envelope) + assert caught.value.diagnostic == provider_mod.DIAG_KEYRING_UNAVAILABLE + assert caught.value.__cause__ is None + assert caught.value.__context__ is None + assert opener.requests == [] + assert _locals_holding(caught.value, KEY_SENTINEL) == [] + + def test_the_production_keyring_path_imports_the_package_at_call_time( monkeypatch): backend = _FakeKeyring({(KEYRING_SERVICE, KEYRING_USER): KEY_SENTINEL}) From 47c9da9a8ecf1f35ac4495a1780758d9a0ca9842 Mon Sep 17 00:00:00 2001 From: Brett Heap <1513478+brettheap@users.noreply.github.com> Date: Sat, 3 Oct 2026 01:30:09 +0000 Subject: [PATCH 20/20] T080: the L4 case runs a broker's turn too (Copilot at 44582f8f) Copilot's review of openDox-code#63 at 44582f8f: the shared transport's handler changes the broker path's failure, which the description said this PR leaves alone. It does: an http.client.HTTPException on a broker's turn escaped dispatch, and now lands on the fixed unreachable sentence, as on the built-in and none paths. The L4 case runs a real fake broker's mint for each subclass and pins that. Raising it afresh on the broker path, with no token in a kept frame, is openDox-code#64's, as the ruling leaves that path to it, and the description now says so. 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 | 26 +++++++++++++++++++------- 1 file changed, 19 insertions(+), 7 deletions(-) diff --git a/tests/test_model_provider_broker.py b/tests/test_model_provider_broker.py index 3eca2b5a..bc4d2c18 100644 --- a/tests/test_model_provider_broker.py +++ b/tests/test_model_provider_broker.py @@ -3088,16 +3088,22 @@ def _read_by_urllib_alone(url: str) -> None: response.read() -@pytest.mark.parametrize("resolver", ["built-in", "none"]) +@pytest.mark.parametrize("resolver", ["built-in", "none", "broker"]) @pytest.mark.parametrize("raised", sorted(UNREADABLE_ANSWERS)) def test_an_answer_http_client_cannot_read_is_unreachable_and_keeps_no_key( - raised, resolver): + tmp_path, raised, resolver): """The adversarial review of openDox-code#63 at `4948e6dd`, L4. A status line, a protocol, a header, a body or a path that `http.client` cannot read or send raises an `http.client.HTTPException`, which is no `OSError`. So it escaped `dispatch`, with the request's headers, and the key, in `do_open`'s frame. Each is the fixed unreachable refusal now, - raised afresh where a built-in credential was presented.""" + raised afresh where a built-in credential was presented. + + THE TRANSPORT IS SHARED, so a broker's turn lands on the same sentence + (Copilot's review of openDox-code#63 at `44582f8f`), where it escaped + before. That is the one change this PR makes to the broker path's + failure. Raising it afresh there is openDox-code#64's, as the ruling + leaves that path to it.""" answer, path = UNREADABLE_ANSWERS[raised] handler = type(f"_{raised}Answer", (_UnreadableAnswerHandler,), {"answer": answer}) @@ -3106,12 +3112,18 @@ def test_an_answer_http_client_cannot_read_is_unreachable_and_keeps_no_key( with pytest.raises(http.client.HTTPException) as unread: _read_by_urllib_alone(base + path) assert type(unread.value) is getattr(http.client, raised) - binding = (_built_in_binding(endpoint=base + path) - if resolver == "built-in" - else _none_binding(endpoint=base + path)) + runner = _refusing_runner + if resolver == "built-in": + binding = _built_in_binding(endpoint=base + path) + elif resolver == "none": + binding = _none_binding(endpoint=base + path) + else: + binding = _broker_binding(_write_broker(tmp_path), + endpoint=base + path) + runner = provider_mod.subprocess_broker_runner port = provider_mod.BrokeredProviderPort( binding, install_mod.brokered_catalog(binding), - runner=_refusing_runner, notice=lambda _text: None, + runner=runner, notice=lambda _text: None, environ={ENV_NAME: KEY_SENTINEL}) envelope = _Envelope() with pytest.raises(provider_mod.BrokerRefused) as caught: