diff --git a/app/broker.py b/app/broker.py index ba15291..84d537e 100644 --- a/app/broker.py +++ b/app/broker.py @@ -19,12 +19,17 @@ class HttpxBroker: this is exactly where you'd inject them for a real upstream service. Per the ``BrokerAdapter`` execution contract (spec §4.2) it sends only the - **fingerprinted** action — method + host + path + the declared ``params`` — - and refuses (``BrokerRefusal``) if ``action.url`` carries a query string or - fragment, since through protocol 0.2 the query is outside the fingerprint and - so was never authorised. Forwarding it verbatim would let ``/orders?to=me`` - and ``/orders?to=attacker`` (one fingerprint) reach different upstreams — the - confused-deputy gap the firewall exists to close.""" + **fingerprinted** action — method + host + path + canonicalized query + the + declared ``params``. Since protocol 0.3 the URL's query string is folded into + the fingerprint, so it *is* part of the authorised action: ``/orders?to=me`` + and ``/orders?to=attacker`` carry different fingerprints, and the broker + forwards the query of the authorised action. The only channel the fingerprint + does not represent is the URL ``#fragment``, which the broker refuses + (``BrokerRefusal``) rather than forward — never silently strip. + + *(Pre-0.3 this broker refused* **all** *queries, the 0.2.3 interim defense; + that is now over-strict — the query is fingerprint-bound. Requires delego + ≥ 0.3.0.)*""" name = "httpx" @@ -33,13 +38,17 @@ def __init__(self, timeout: float = 15.0) -> None: timeout=timeout, headers={"User-Agent": "delego-sample-app"} ) - def execute(self, action: ProposedAction) -> dict: - # Fail closed on a query/fragment the fingerprint never represented, - # rather than silently forwarding decision-relevant data (spec §4.2). - if action.has_query: + def execute(self, action: ProposedAction, token: str | None = None) -> dict: + # Fail closed on a #fragment — data outside the fingerprint preimage + # (spec §4.2) — rather than forwarding what the decision never saw. The + # query, now fingerprint-bound, is part of the authorised action and is + # forwarded via ``fingerprinted_url`` below. (``token`` is accepted for + # the §9 profile; this in-process broker trusts the decision, so it does + # not verify — a *separated* gateway would, see delego.verify_token.) + if action.has_fragment: raise BrokerRefusal( - "broker refuses to execute: action.url carries a query string or " - "fragment outside the fingerprint (method+host+path+params), so it " + "broker refuses to execute: action.url carries a #fragment, which " + "is outside the fingerprint (method+host+path+query+params) and so " "was never authorised (spec §4.2). Put decision-relevant values in " "params. Offending url: " + action.url ) @@ -49,7 +58,8 @@ def execute(self, action: ProposedAction) -> dict: # For writes, forward the declared params as the JSON body. if method in ("POST", "PUT", "PATCH"): kwargs["json"] = action.params - # Request only the fingerprinted URL (scheme+host+path); no query is sent. + # Request the fingerprinted URL (scheme+host+path+query); the #fragment + # is never represented in the fingerprint, so it is never sent. resp = self._client.request(method, action.fingerprinted_url, **kwargs) return { "broker": self.name, diff --git a/app/main.py b/app/main.py index 7cd1e5a..b7eefd6 100644 --- a/app/main.py +++ b/app/main.py @@ -121,8 +121,8 @@ def create_app(broker: BrokerAdapter | None = None, home: str | os.PathLike | No @app.exception_handler(BrokerRefusal) def _broker_refused(_request, exc: BrokerRefusal): # The firewall authorised the fingerprinted action, but the broker refused - # to forward decision-relevant data outside that fingerprint (a query - # string; spec §4.2). Surface it as a clear 4xx, not a 500. + # to forward decision-relevant data outside that fingerprint (a URL + # #fragment; spec §4.2). Surface it as a clear 4xx, not a 500. from fastapi.responses import JSONResponse return JSONResponse( diff --git a/requirements.txt b/requirements.txt index 0f342b4..4797423 100644 --- a/requirements.txt +++ b/requirements.txt @@ -1,4 +1,4 @@ -delego>=0.2.3 # 0.2.3 adds BrokerRefusal + the confused-deputy broker fix (C1) this app uses +delego>=0.3.0 # 0.3 folds the query into the fingerprint; the broker forwards it and refuses only #fragments (needs has_fragment + query-bearing fingerprinted_url) fastapi>=0.110 uvicorn[standard]>=0.29 httpx>=0.27 diff --git a/tests/test_broker.py b/tests/test_broker.py new file mode 100644 index 0000000..2157db6 --- /dev/null +++ b/tests/test_broker.py @@ -0,0 +1,69 @@ +"""HttpxBroker query/fragment handling under protocol 0.3. + +Since 0.3 the URL query is folded into the action fingerprint, so the broker +forwards it (it is part of the authorised action) and refuses only a URL +``#fragment`` — which the fingerprint never represents. These tests stub the +httpx client so nothing hits the network. +""" + +from __future__ import annotations + +import pytest +from delego import ProposedAction +from delego.brokers import BrokerRefusal + +from app.broker import HttpxBroker + + +class _FakeResp: + status_code = 200 + text = "ok" + + def __init__(self, url: str) -> None: + self.url = url + + +def _broker(monkeypatch): + """An HttpxBroker whose client records the URL it would request.""" + broker = HttpxBroker() + sent: dict = {} + + def fake_request(method, url, **kwargs): + sent["method"], sent["url"], sent["kwargs"] = method, url, kwargs + return _FakeResp(url) + + monkeypatch.setattr(broker._client, "request", fake_request) + return broker, sent + + +def test_forwards_the_fingerprint_bound_query(monkeypatch): + # /orders?to=me is a distinct, authorised action in 0.3 — the broker sends it. + broker, sent = _broker(monkeypatch) + action = ProposedAction("send to me", "GET", "https://api.example.com/orders?to=me", {}) + out = broker.execute(action) + assert out["http_status"] == 200 + assert sent["url"] == "https://api.example.com/orders?to=me" # query forwarded + + +def test_refuses_a_fragment(monkeypatch): + # A #fragment is outside the fingerprint preimage — refuse, never strip. + broker, sent = _broker(monkeypatch) + action = ProposedAction("read", "GET", "https://api.example.com/orders#smuggled", {}) + with pytest.raises(BrokerRefusal): + broker.execute(action) + assert sent == {} # nothing was sent upstream + + +def test_clean_action_is_sent(monkeypatch): + broker, sent = _broker(monkeypatch) + out = broker.execute(ProposedAction("read", "GET", "https://api.example.com/orders", {})) + assert out["http_status"] == 200 + assert sent["url"] == "https://api.example.com/orders" + + +def test_accepts_the_optional_token_kwarg(monkeypatch): + # The §9 token rides as an optional kwarg; an in-process broker need not + # verify, but must accept it so the firewall can pass it. + broker, sent = _broker(monkeypatch) + out = broker.execute(ProposedAction("read", "GET", "https://api.example.com/orders", {}), token="tok") + assert out["http_status"] == 200