diff --git a/docs/proposals/v3-identity-bundle-design.md b/docs/proposals/v3-identity-bundle-design.md index c393a73cd..3a05a9ab0 100644 --- a/docs/proposals/v3-identity-bundle-design.md +++ b/docs/proposals/v3-identity-bundle-design.md @@ -21,6 +21,12 @@ These compose. (1) proves an agent's *cryptographic identity*; (2) asserts the *commercial relationship*; (3) verifies *per-brand authorization*. All three checks gate a request. +**Status note:** code names below (`AGENT_SUSPENDED`, `AGENT_BLOCKED`, +`REQUEST_AUTH_UNRECOGNIZED_AGENT`, `INVALID_BILLING_MODEL`) were +superseded in PR #393 by the spec-conformant `PERMISSION_DENIED` / +`BILLING_NOT_PERMITTED_FOR_AGENT` shapes. See the test suite in +`tests/test_tier2_spec_conformance.py` for current behavior. + **Status:** RFC, awaiting review. **Companions:** - JS `BuyerAgentRegistry` design — [adcp-client#1269](https://github.com/adcontextprotocol/adcp-client/issues/1269) diff --git a/examples/buyer_agent_registry_sqlalchemy.py b/examples/buyer_agent_registry_sqlalchemy.py index e50ab6497..8ef652f32 100644 --- a/examples/buyer_agent_registry_sqlalchemy.py +++ b/examples/buyer_agent_registry_sqlalchemy.py @@ -368,7 +368,11 @@ def session_factory() -> Session: print("\n== suspended agent ==") suspended = await signing.resolve_by_agent_url("https://suspended-buyer.example/") print(f" status: {suspended.status!r}") - print(" → framework dispatch raises AdcpError(code='AGENT_SUSPENDED')") + print( + " → framework dispatch raises " + "AdcpError(code='PERMISSION_DENIED', " + "details={'scope': 'agent', 'status': 'suspended', ...})" + ) if __name__ == "__main__": diff --git a/examples/v3_reference_seller/src/buyer_registry.py b/examples/v3_reference_seller/src/buyer_registry.py index 68ed937e7..a4c8dd1d5 100644 --- a/examples/v3_reference_seller/src/buyer_registry.py +++ b/examples/v3_reference_seller/src/buyer_registry.py @@ -45,8 +45,9 @@ class TenantScopedBuyerAgentRegistry: Resolves to ``None`` when the request has no tenant context (i.e., :func:`current_tenant` returns ``None`` because the middleware bypassed routing or the host wasn't registered) — the - framework's dispatch then rejects with - ``REQUEST_AUTH_UNRECOGNIZED_AGENT``. + framework's dispatch then rejects with ``PERMISSION_DENIED`` + (with ``details`` omitted so the unrecognized-agent path is + wire-indistinguishable from a recognized-but-denied response). Implements both methods of the :class:`adcp.decisioning.BuyerAgentRegistry` Protocol so the diff --git a/examples/v3_reference_seller/tests/test_smoke.py b/examples/v3_reference_seller/tests/test_smoke.py index 2cd59fa43..3e22a5d45 100644 --- a/examples/v3_reference_seller/tests/test_smoke.py +++ b/examples/v3_reference_seller/tests/test_smoke.py @@ -100,7 +100,9 @@ def scalar_one_or_none(self): async def test_buyer_registry_returns_none_without_tenant() -> None: """Without a tenant context (ContextVar unset), the registry returns None — the framework dispatch then rejects with - REQUEST_AUTH_UNRECOGNIZED_AGENT.""" + PERMISSION_DENIED (with no ``details`` so the unrecognized-agent + path is wire-indistinguishable from a recognized-but-denied + response).""" from src.buyer_registry import TenantScopedBuyerAgentRegistry from adcp.decisioning import ApiKeyCredential diff --git a/src/adcp/decisioning/context.py b/src/adcp/decisioning/context.py index 962ad7775..ff605bb02 100644 --- a/src/adcp/decisioning/context.py +++ b/src/adcp/decisioning/context.py @@ -73,8 +73,8 @@ class AuthInfo: ``credential=``. * **4.5.0** — synthesis is removed; flat-field-only construction stops auto-populating ``credential``, and the registry dispatch - will reject the request with - ``REQUEST_AUTH_UNRECOGNIZED_AGENT``. Adopters must construct + will reject the request with ``PERMISSION_DENIED``. Adopters + must construct the typed credential explicitly: ``AuthInfo(credential=ApiKeyCredential(kind="api_key", key_id=...))`` or use the bundled signed-request verifier middleware. diff --git a/src/adcp/decisioning/handler.py b/src/adcp/decisioning/handler.py index 2b488dce1..34b3db5ef 100644 --- a/src/adcp/decisioning/handler.py +++ b/src/adcp/decisioning/handler.py @@ -331,14 +331,41 @@ async def _resolve_buyer_agent( * :class:`ApiKeyCredential` / :class:`OAuthCredential` → :meth:`BuyerAgentRegistry.resolve_by_credential`. * No credential at all (unauthenticated dev fixture, ``derived`` - auth) → ``REQUEST_AUTH_UNRECOGNIZED_AGENT``. Adopters running - a registry have implicitly opted out of unauthenticated traffic. - - :raises AdcpError: ``REQUEST_AUTH_UNRECOGNIZED_AGENT`` (registry - miss / no credential), ``AGENT_SUSPENDED`` (status=suspended), - or ``AGENT_BLOCKED`` (status=blocked). All ``recovery=terminal`` - — the buyer cannot retry their way out of a commercial-state - rejection. + auth) → ``PERMISSION_DENIED`` (no ``details.scope``). Adopters + running a registry have implicitly opted out of unauthenticated + traffic. + + All four denial paths surface as ``code="PERMISSION_DENIED"`` to + match the spec enum and prevent the cross-tenant onboarding-oracle + risk: an attacker watching the wire MUST NOT be able to + distinguish "this agent_url is unrecognized at this seller" from + "this agent_url is recognized but currently denied". The + discriminator is in ``details``: + + * recognized + suspended → + ``details = {scope: "agent", status: "suspended", agent_url: ...}`` + * recognized + blocked → + ``details = {scope: "agent", status: "blocked", agent_url: ...}`` + * unrecognized (registry miss / no credential / unknown status) → + ``details`` OMITTED — scope MUST NOT be set on the unestablished- + identity path (omit-on-unestablished-identity rule). + + Note on parity: the *latency / headers / side-effects* parity + contract between the recognized and unrecognized paths is tracked + as a follow-up — the eager-raise pattern below still completes the + unrecognized path on a different code path than the recognized + one. Renaming closes the wire-code mismatch; folding all four + paths through a common emit point with deliberate latency padding + and identical audit/metric side-effects is the next step. + + :raises AdcpError: ``PERMISSION_DENIED`` (all four denial paths). + Recovery is ``correctable`` per the spec's ``enumMetadata`` + for ``PERMISSION_DENIED``. The wire-level recovery hint is + independent of the resolution channel: the buyer cannot + auto-retry a commercial-identity rejection, but the + ``details.scope == "agent"`` discriminator (when present) is + the signal callers surface to a human operator rather than + loop on the request. """ from adcp.decisioning.registry import ( ApiKeyCredential, @@ -372,62 +399,68 @@ async def _resolve_buyer_agent( recovery="terminal", ) + # Generic message used on every denial path — MUST be identical + # across the unrecognized and the recognized-but-denied paths so + # the wire-level error.message is not itself a side channel + # leaking which agent_urls are onboarded with which sellers. The + # discriminator (when present at all) is in details, only on the + # recognized-but-denied paths. + _denied_message = ( + "Buyer agent is not authorized for this seller. The seller's " + "commercial allowlist did not authorize this credential. " + "Resolve out-of-band via the seller's onboarding contact; this " + "is not a request-side error the buyer can correct." + ) + if agent is None: + # Registry miss / no credential. ``details`` is OMITTED — the + # spec's omit-on-unestablished-identity rule says the + # unrecognized-agent path MUST be indistinguishable on the + # wire from the recognized-but-denied path, and ``scope`` + # would itself be the discriminator. raise AdcpError( - "REQUEST_AUTH_UNRECOGNIZED_AGENT", - message=( - "BuyerAgentRegistry returned no match for the request's " - "credential. The registry is the seller's commercial " - "allowlist — adopters reject auth that's cryptographically " - "valid but not commercially recognized (no onboarding row, " - "revoked, or wrong credential kind for the registry's " - "posture). Check that the agent has been onboarded into the " - "registry's backing store." - ), - recovery="terminal", + "PERMISSION_DENIED", + message=_denied_message, + recovery="correctable", ) if agent.status == "active": return agent if agent.status == "suspended": raise AdcpError( - "AGENT_SUSPENDED", - message=( - f"Buyer agent {agent.agent_url!r} is suspended. Suspension " - "is a temporary commercial pause (credit, compliance review, " - "ops hold) — the seller restores it via their durable " - "store. Retry once the seller restores the agent; escalate " - "through the account contact if the pause is unexpected." - ), - recovery="transient", - details={"agent_url": agent.agent_url, "status": agent.status}, + "PERMISSION_DENIED", + message=_denied_message, + recovery="correctable", + details={ + "scope": "agent", + "status": "suspended", + "agent_url": agent.agent_url, + }, ) if agent.status == "blocked": raise AdcpError( - "AGENT_BLOCKED", - message=( - f"Buyer agent {agent.agent_url!r} is blocked. Blocked is " - "a hard cutoff (terms violation, fraud, enforcement) — " - "no retry path. Buyer must contact the seller directly." - ), - recovery="terminal", - details={"agent_url": agent.agent_url, "status": agent.status}, + "PERMISSION_DENIED", + message=_denied_message, + recovery="correctable", + details={ + "scope": "agent", + "status": "blocked", + "agent_url": agent.agent_url, + }, ) # Default-reject any non-active status the framework doesn't # recognize (typo, future enum value, adopter-custom string). A # silent fall-through to "active" would leak commercial state - # past the gate. + # past the gate. ``details`` is OMITTED for the same reason as + # the registry-miss branch — the framework treats unknown statuses + # as the unrecognized-identity path (the row is in the registry + # but the framework cannot interpret it, which is operationally + # equivalent to "not authorized" without a defensible status + # claim to project on the wire). raise AdcpError( - "REQUEST_AUTH_UNRECOGNIZED_AGENT", - message=( - f"Buyer agent {agent.agent_url!r} has unrecognized status " - f"{agent.status!r}. The framework only treats ``active`` as " - "live; ``suspended`` / ``blocked`` raise their own structured " - "errors. Unknown statuses are rejected by default to prevent " - "silent fall-through past the commercial-identity gate." - ), - recovery="terminal", - details={"agent_url": agent.agent_url, "status": agent.status}, + "PERMISSION_DENIED", + message=_denied_message, + recovery="correctable", ) @@ -627,10 +660,13 @@ async def _resolve_account( BEFORE calling ``AccountStore.resolve`` and stashes the result on ``ctx.metadata['adcp.buyer_agent']`` for :meth:`_build_ctx` to read into the typed :class:`RequestContext`. Suspended / - blocked agents are rejected here with structured error codes - — buyers see ``AGENT_SUSPENDED`` / ``AGENT_BLOCKED`` / - ``REQUEST_AUTH_UNRECOGNIZED_AGENT`` instead of the registry - miss leaking into the AccountStore as ``ACCOUNT_NOT_FOUND``. + blocked / unrecognized agents are rejected here with + ``PERMISSION_DENIED`` (recognized-but-denied paths carry + ``details.scope="agent"`` + ``details.status``; the + unrecognized-agent path omits ``details`` so the wire shape + does not enumerate which ``agent_url``s are onboarded with + this seller) instead of the registry miss leaking into the + AccountStore as ``ACCOUNT_NOT_FOUND``. """ auth_info = self._extract_auth_info(ctx) if self._buyer_agent_registry is not None: diff --git a/src/adcp/decisioning/pg/buyer_agent_registry.py b/src/adcp/decisioning/pg/buyer_agent_registry.py index 6463871e1..3db56e5af 100644 --- a/src/adcp/decisioning/pg/buyer_agent_registry.py +++ b/src/adcp/decisioning/pg/buyer_agent_registry.py @@ -239,7 +239,9 @@ async def resolve_by_agent_url(self, agent_url: str) -> BuyerAgent | None: The framework has already validated the RFC 9421 signature before this point — the registry's only job is the commercial lookup. Returns ``None`` when the agent isn't recognized; - the framework converts that to ``REQUEST_AUTH_UNRECOGNIZED_AGENT``. + the framework converts that to ``PERMISSION_DENIED`` (with + ``details`` omitted so the unrecognized-agent path is + wire-indistinguishable from recognized-but-denied). """ return await asyncio.to_thread(self._sync_lookup_by_agent_url, agent_url) diff --git a/src/adcp/decisioning/registry.py b/src/adcp/decisioning/registry.py index 6e697a8ca..ff76278f2 100644 --- a/src/adcp/decisioning/registry.py +++ b/src/adcp/decisioning/registry.py @@ -18,7 +18,8 @@ Today the framework can't enforce that and the resulting commercial drift is invisible to buyers. With :class:`BuyerAgent.billing_capabilities` the framework rejects mismatched ``billing`` values with a structured -:class:`adcp.decisioning.AdcpError` ``code="INVALID_BILLING_MODEL"``. +:class:`adcp.decisioning.AdcpError` +``code="BILLING_NOT_PERMITTED_FOR_AGENT"``. Per :issue:`adcp-client#1269` and the v3-identity-bundle RFC, the registry is *durable* infrastructure — it stays useful even after @@ -197,8 +198,11 @@ class BuyerAgentRegistry(Protocol): * :meth:`resolve_by_credential` for bearer / API-key / OAuth — the adopter looks up against their existing key table. - Returning ``None`` rejects the request with - ``REQUEST_AUTH_UNRECOGNIZED_AGENT``. Adopters typically construct + Returning ``None`` rejects the request with ``PERMISSION_DENIED`` + (``details`` omitted — the unrecognized-agent path MUST be + indistinguishable on the wire from the recognized-but-denied path + to prevent cross-tenant onboarding enumeration). Adopters + typically construct via the :func:`signing_only_registry` / :func:`bearer_only_registry` / :func:`mixed_registry` factories rather than implementing the Protocol directly — the factories carry the posture choice in @@ -286,7 +290,8 @@ def signing_only_registry( Adopter supplies an async function that maps a verified ``agent_url`` to a :class:`BuyerAgent` (or ``None`` to reject). - Bearer traffic gets ``REQUEST_AUTH_UNRECOGNIZED_AGENT`` at the + Bearer traffic gets ``PERMISSION_DENIED`` (with ``details`` + omitted — wire-indistinguishable from any other denial) at the framework's dispatch layer — the registry deliberately doesn't implement bearer lookup. """ @@ -301,8 +306,8 @@ def bearer_only_registry( Adopter supplies an async function that maps an :class:`ApiKeyCredential` or :class:`OAuthCredential` to a :class:`BuyerAgent` (or ``None`` to reject). Signed traffic gets - ``REQUEST_AUTH_UNRECOGNIZED_AGENT`` — adopt :func:`mixed_registry` - once signed onboarding is wired. + ``PERMISSION_DENIED`` — adopt :func:`mixed_registry` once signed + onboarding is wired. """ return _BearerOnlyRegistry(_resolve_by_credential=resolve_by_credential) @@ -332,13 +337,22 @@ def validate_billing_for_agent( requested_billing: BillingMode, agent: BuyerAgent, ) -> None: - """Raise :class:`adcp.decisioning.AdcpError` ``INVALID_BILLING_MODEL`` - when ``requested_billing`` is not in ``agent.billing_capabilities``. + """Raise :class:`adcp.decisioning.AdcpError` + ``BILLING_NOT_PERMITTED_FOR_AGENT`` when ``requested_billing`` is + not in ``agent.billing_capabilities``. Called by the framework's ``sync_accounts`` shim before invoking the platform method. Adopters needn't call this directly; the framework enforces. Re-exported so platform methods that branch on billing mode can short-circuit to the same structured error. + + The wire ``details`` payload deliberately carries only + ``rejected_billing`` (and an optional ``suggested_billing``) — it + MUST NOT carry the agent's full ``permitted_billing`` subset. The + full subset is the agent's commercial relationship with the + seller; surfacing it on every rejected request would let a + misconfigured buyer probe and exfiltrate the matrix one mode at + a time. """ if requested_billing in agent.billing_capabilities: return @@ -346,25 +360,30 @@ def validate_billing_for_agent( # close on import-load order). from adcp.decisioning.types import AdcpError + # Suggest a single permitted mode (deterministic — the + # alphabetically-first permitted mode) when the agent has any + # capability at all. We do NOT enumerate the full set; suggesting + # one mode is sufficient remediation hint without leaking the + # subset shape on every failed request. + suggested = sorted(agent.billing_capabilities)[0] if agent.billing_capabilities else None + details: dict[str, Any] = {"rejected_billing": requested_billing} + if suggested is not None: + details["suggested_billing"] = suggested + raise AdcpError( - "INVALID_BILLING_MODEL", + "BILLING_NOT_PERMITTED_FOR_AGENT", message=( - f"Buyer agent '{agent.agent_url}' is not authorized for " - f"billing={requested_billing!r}; permitted modes are " - f"{sorted(agent.billing_capabilities)!r}. Common cause: " - "this agent has no payments relationship with the seller " - "(passthrough only) — accounts under this agent must be " - "operator-billed. Sellers extending the agent's billing " - "capabilities update the BuyerAgent.billing_capabilities " - "frozenset in their durable store." + f"Buyer agent {agent.agent_url!r} is not authorized for " + f"billing={requested_billing!r}. Common cause: this agent " + "has no payments relationship with the seller (passthrough " + "only) — accounts under this agent must be operator-billed. " + "Sellers extending the agent's billing capabilities update " + "the BuyerAgent.billing_capabilities frozenset in their " + "durable store." ), field="billing", - recovery="terminal", - details={ - "agent_url": agent.agent_url, - "requested_billing": requested_billing, - "permitted_billing": sorted(agent.billing_capabilities), - }, + recovery="correctable", + details=details, ) diff --git a/src/adcp/decisioning/serve.py b/src/adcp/decisioning/serve.py index 3097c244b..03c872b83 100644 --- a/src/adcp/decisioning/serve.py +++ b/src/adcp/decisioning/serve.py @@ -147,9 +147,12 @@ def create_adcp_server_from_platform( identity layer. When wired, the framework calls the registry BEFORE :meth:`AccountStore.resolve` to gate every request on the seller's commercial allowlist. Suspended / blocked / - unknown agents are rejected with structured - ``AGENT_SUSPENDED`` / ``AGENT_BLOCKED`` / - ``REQUEST_AUTH_UNRECOGNIZED_AGENT`` errors. The resolved + unrecognized agents are rejected with structured + ``PERMISSION_DENIED`` errors (recognized-but-denied paths + carry ``details.scope="agent"`` + ``details.status``; the + unrecognized-agent path omits ``details`` so the wire shape + does not enumerate which ``agent_url``s are onboarded with + this seller). The resolved :class:`adcp.decisioning.BuyerAgent` is threaded onto :attr:`RequestContext.buyer_agent` so platform methods can read commercial context (billing capabilities, default terms, @@ -351,7 +354,11 @@ def serve( spec-compliance storyboards) pass ``True``. :param serve_kwargs: Forwarded to :func:`adcp.server.serve`. Use for ``host``, ``port``, ``transport``, ``test_controller``, - ``context_factory``, ``middleware``, etc. + ``context_factory``, ``middleware``, ``validation``, etc. + Pass ``validation=ValidationHookConfig(requests="strict", + responses="strict")`` to enable schema-driven request/response + validation against the bundled AdCP JSON schemas — sellers who + want their server to enforce wire conformance turn it on here. """ # Local import to avoid a circular at module-load time. Adopter # serves never run during foundation imports anyway. diff --git a/tests/test_buyer_agent_registry.py b/tests/test_buyer_agent_registry.py index fa00e4933..04f093796 100644 --- a/tests/test_buyer_agent_registry.py +++ b/tests/test_buyer_agent_registry.py @@ -6,7 +6,9 @@ * Three implementer postures (signing-only / bearer-only / mixed) reject the off-posture credential type by returning ``None``. * :func:`validate_billing_for_agent` accepts permitted modes and - raises ``INVALID_BILLING_MODEL`` on others. + raises ``BILLING_NOT_PERMITTED_FOR_AGENT`` on others, with details + scoped to ``rejected_billing`` (and optional ``suggested_billing``) + — the full ``permitted_billing`` subset is never leaked. * ``BuyerAgent`` defaults match the pre-trust beta passthrough-only posture (no payments relationship — accounts must be operator-billed). * Discriminated :data:`Credential` union pattern-matches cleanly. @@ -292,13 +294,20 @@ def test_validate_billing_rejects_passthrough_only_with_agent_billing() -> None: ) with pytest.raises(AdcpError) as exc: validate_billing_for_agent(requested_billing="agent", agent=agent) - assert exc.value.code == "INVALID_BILLING_MODEL" + assert exc.value.code == "BILLING_NOT_PERMITTED_FOR_AGENT" assert exc.value.field == "billing" - assert exc.value.recovery == "terminal" + assert exc.value.recovery == "correctable" details = exc.value.details - assert details["agent_url"] == "https://passthrough/" - assert details["requested_billing"] == "agent" - assert details["permitted_billing"] == ["operator"] + # ``rejected_billing`` is required. + assert details["rejected_billing"] == "agent" + # Suggested mode is the alphabetically-first permitted mode. + assert details["suggested_billing"] == "operator" + # Critical: the full ``permitted_billing`` subset MUST NOT leak — + # surfacing it on every rejected request would let a misconfigured + # buyer probe and exfiltrate the matrix one mode at a time. + assert "permitted_billing" not in details + # The agent_url is also a leak vector and is not echoed back. + assert "agent_url" not in details def test_validate_billing_rejects_advertiser_when_not_in_capabilities() -> None: @@ -310,5 +319,7 @@ def test_validate_billing_rejects_advertiser_when_not_in_capabilities() -> None: ) with pytest.raises(AdcpError) as exc: validate_billing_for_agent(requested_billing="advertiser", agent=agent) - assert exc.value.code == "INVALID_BILLING_MODEL" + assert exc.value.code == "BILLING_NOT_PERMITTED_FOR_AGENT" assert "advertiser" in str(exc.value) + # Sanity: with a non-empty permitted set, suggested_billing is set. + assert exc.value.details["suggested_billing"] in {"agent", "operator"} diff --git a/tests/test_decisioning_buyer_agent_dispatch.py b/tests/test_decisioning_buyer_agent_dispatch.py index 4fa751a2a..ab13f7763 100644 --- a/tests/test_decisioning_buyer_agent_dispatch.py +++ b/tests/test_decisioning_buyer_agent_dispatch.py @@ -14,10 +14,13 @@ * :class:`PlatformHandler` calls the registry BEFORE :meth:`AccountStore.resolve` when one is wired; the resolved :class:`BuyerAgent` is threaded onto :attr:`RequestContext.buyer_agent`. -* Suspended / blocked / unknown-status agents reject with structured - error codes (``AGENT_SUSPENDED`` transient, ``AGENT_BLOCKED`` - terminal, ``REQUEST_AUTH_UNRECOGNIZED_AGENT`` for missing / - unknown-status) instead of leaking into ``ACCOUNT_NOT_FOUND``. +* Suspended / blocked / unknown-status agents reject with the + spec-conformant ``PERMISSION_DENIED`` code instead of leaking into + ``ACCOUNT_NOT_FOUND``. Recognized-but-denied paths carry + ``details.scope="agent"`` + ``details.status``; the unrecognized + paths (registry miss, no credential, unknown status) omit + ``details`` so the wire shape is indistinguishable per the + cross-tenant onboarding-oracle clamp. * No registry wired → existing dispatch path runs unchanged (back-compat for pre-trust beta adopters). """ @@ -614,7 +617,7 @@ async def test_signed_kind_without_explicit_credential_is_rejected( ``kind="signed_request"`` but no explicit ``credential`` cannot reach the signed-traffic registry path. Synthesis is disabled for this kind, so ``_resolve_buyer_agent`` sees ``credential is - None`` and rejects with ``REQUEST_AUTH_UNRECOGNIZED_AGENT``. + None`` and rejects with ``PERMISSION_DENIED`` (no ``details``). Without this, an upstream middleware that wrote ``kind="signed_request"`` without doing RFC 9421 verification would silently escalate to the verified path.""" @@ -647,16 +650,21 @@ async def get_products(self, req, ctx): GetProductsRequest(buying_mode="brief", brief="any"), tool_ctx, ) - assert exc_info.value.code == "REQUEST_AUTH_UNRECOGNIZED_AGENT" + assert exc_info.value.code == "PERMISSION_DENIED" + # Unrecognized-agent path MUST NOT carry details — the spec's + # omit-on-unestablished-identity rule. + assert exc_info.value.details == {} @pytest.mark.asyncio -async def test_registry_miss_raises_request_auth_unrecognized_agent( +async def test_registry_miss_raises_permission_denied_no_details( executor, ) -> None: - """Registry returns None → ``REQUEST_AUTH_UNRECOGNIZED_AGENT``, + """Registry returns None → ``PERMISSION_DENIED`` (no ``details``), NOT ``ACCOUNT_NOT_FOUND`` (which would mask the commercial-allowlist - miss as an account-resolution problem).""" + miss as an account-resolution problem). ``details`` is OMITTED so + the wire shape is indistinguishable from the recognized-but-denied + paths per the cross-tenant onboarding-oracle clamp.""" from adcp.types import GetProductsRequest async def lookup(_url: str) -> BuyerAgent | None: @@ -680,16 +688,19 @@ async def get_products(self, req, ctx): GetProductsRequest(buying_mode="brief", brief="any"), tool_ctx, ) - assert exc_info.value.code == "REQUEST_AUTH_UNRECOGNIZED_AGENT" - assert exc_info.value.recovery == "terminal" + assert exc_info.value.code == "PERMISSION_DENIED" + assert exc_info.value.recovery == "correctable" + assert exc_info.value.details == {} @pytest.mark.asyncio -async def test_suspended_agent_raises_agent_suspended_transient(executor) -> None: - """Status=suspended is a *retryable* commercial pause - (``recovery="transient"``). Buyer agents can retry once the - seller restores the agent — distinct from blocked, which is - terminal.""" +async def test_suspended_agent_raises_permission_denied_terminal(executor) -> None: + """Status=suspended is rejected as ``PERMISSION_DENIED`` with + ``details.scope="agent"`` + ``details.status="suspended"``. + Wire-level ``recovery`` is ``correctable`` per the spec's + ``enumMetadata`` for ``PERMISSION_DENIED``; the + ``details.scope == "agent"`` discriminator is the signal callers + surface to a human operator rather than auto-retry.""" from adcp.types import GetProductsRequest suspended = BuyerAgent( @@ -719,16 +730,21 @@ async def get_products(self, req, ctx): GetProductsRequest(buying_mode="brief", brief="any"), tool_ctx, ) - assert exc_info.value.code == "AGENT_SUSPENDED" - assert exc_info.value.recovery == "transient" + assert exc_info.value.code == "PERMISSION_DENIED" + assert exc_info.value.recovery == "correctable" + assert exc_info.value.details["scope"] == "agent" + assert exc_info.value.details["status"] == "suspended" assert exc_info.value.details["agent_url"] == "https://suspended/" @pytest.mark.asyncio -async def test_blocked_agent_raises_agent_blocked_terminal(executor) -> None: - """Status=blocked is a hard cutoff - (``recovery="terminal"``) — buyer cannot retry their way out, - must contact seller directly.""" +async def test_blocked_agent_raises_permission_denied_terminal(executor) -> None: + """Status=blocked is rejected as ``PERMISSION_DENIED`` with + ``details.scope="agent"`` + ``details.status="blocked"``. + Wire-level ``recovery`` is ``correctable`` per the spec's + ``enumMetadata``; the ``details.scope == "agent"`` discriminator + signals callers to surface to a human operator rather than + auto-retry.""" from adcp.types import GetProductsRequest blocked = BuyerAgent( @@ -758,8 +774,10 @@ async def get_products(self, req, ctx): GetProductsRequest(buying_mode="brief", brief="any"), tool_ctx, ) - assert exc_info.value.code == "AGENT_BLOCKED" - assert exc_info.value.recovery == "terminal" + assert exc_info.value.code == "PERMISSION_DENIED" + assert exc_info.value.recovery == "correctable" + assert exc_info.value.details["scope"] == "agent" + assert exc_info.value.details["status"] == "blocked" @pytest.mark.asyncio @@ -767,7 +785,11 @@ async def test_unknown_agent_status_default_rejects(executor) -> None: """Defense-in-depth: a row with a typo'd or future-enum-value status must not silently fall through to ``active`` past the commercial-identity gate. Anything not in ``{active, suspended, - blocked}`` raises ``REQUEST_AUTH_UNRECOGNIZED_AGENT``.""" + blocked}`` raises ``PERMISSION_DENIED`` with NO ``details`` — + the framework cannot project an unknown status as a defensible + discriminator on the wire, so it routes the unknown-status case + through the same omit-on-unestablished-identity path as the + registry-miss branch.""" from adcp.types import GetProductsRequest weird = BuyerAgent.__new__(BuyerAgent) @@ -803,8 +825,10 @@ async def get_products(self, req, ctx): GetProductsRequest(buying_mode="brief", brief="any"), tool_ctx, ) - assert exc_info.value.code == "REQUEST_AUTH_UNRECOGNIZED_AGENT" - assert exc_info.value.details["status"] == "deleted" + assert exc_info.value.code == "PERMISSION_DENIED" + # Unknown-status path MUST omit details — same wire shape as + # registry miss / no credential. The status is not surfaced. + assert exc_info.value.details == {} @pytest.mark.asyncio @@ -863,7 +887,7 @@ async def get_products(self, req, ctx): GetProductsRequest(buying_mode="brief", brief="any"), ToolContext(), ) - assert exc_info.value.code == "REQUEST_AUTH_UNRECOGNIZED_AGENT" + assert exc_info.value.code == "PERMISSION_DENIED" @pytest.mark.asyncio @@ -939,7 +963,7 @@ async def get_products(self, req, ctx): @pytest.mark.asyncio async def test_mixed_registry_signed_miss_rejects(executor) -> None: """Mixed-registry posture, signed traffic, agent_url not in the - seller's allowlist → REQUEST_AUTH_UNRECOGNIZED_AGENT. The bearer + seller's allowlist → PERMISSION_DENIED. The bearer path is never consulted (signed credentials don't fall through to bearer lookup).""" from adcp.types import GetProductsRequest @@ -975,5 +999,5 @@ async def get_products(self, req, ctx): GetProductsRequest(buying_mode="brief", brief="any"), ToolContext(metadata={"adcp.auth_info": _signed_auth_info("https://unknown/")}), ) - assert exc_info.value.code == "REQUEST_AUTH_UNRECOGNIZED_AGENT" + assert exc_info.value.code == "PERMISSION_DENIED" assert bearer_consulted == [] diff --git a/tests/test_tier2_spec_conformance.py b/tests/test_tier2_spec_conformance.py new file mode 100644 index 000000000..b28375b5b --- /dev/null +++ b/tests/test_tier2_spec_conformance.py @@ -0,0 +1,361 @@ +"""Tier 2 commercial-identity gate — AdCP error-code spec conformance. + +The four denial paths in :func:`adcp.decisioning.handler._resolve_buyer_agent` +and :func:`adcp.decisioning.registry.validate_billing_for_agent` previously +raised four SDK-invented error codes (``AGENT_SUSPENDED``, ``AGENT_BLOCKED``, +``REQUEST_AUTH_UNRECOGNIZED_AGENT``, ``INVALID_BILLING_MODEL``) absent from +the spec's :file:`schemas/cache/enums/error-code.json` 51-entry vocabulary. + +This file pins the spec-conformant wire shape: + +* All four denial paths surface a code from the spec vocabulary + (``PERMISSION_DENIED`` for the three commercial-identity paths; + ``BILLING_NOT_PERMITTED_FOR_AGENT`` for the billing-capability path — + see PR notes for the spec status of the billing code). +* Recognized-but-denied paths (suspended / blocked) carry + ``details.scope="agent"`` + ``details.status``. +* Unrecognized paths (registry miss / no credential / unknown status) + OMIT ``details`` so the wire shape is indistinguishable from a + recognized-but-denied response per the cross-tenant onboarding-oracle + clamp. +* The billing-capability path's ``details`` carries ``rejected_billing`` + (and an optional ``suggested_billing``) — the full + ``permitted_billing`` subset MUST NOT leak. + +The four old codes MUST NOT be raised by any framework path. A regression +test here is the load-bearing CI signal that an adopter doesn't +accidentally re-introduce them via copy-paste. + +The latency / headers / side-effects parity contract between the +unrecognized-agent path and the recognized-but-denied path is tracked as +a separate follow-up (see issue #375 and the parity-contract follow-up +referenced in the PR body). This file pins the wire-shape conformance +only — the parity refactor needs a single emit point with deliberate +latency padding and identical audit/metric side-effects, which is a +larger dispatch-path refactor than fits in the rename PR. +""" + +from __future__ import annotations + +from concurrent.futures import ThreadPoolExecutor + +import pytest + +from adcp.decisioning import ( + AdcpError, + AuthInfo, + BuyerAgent, + DecisioningCapabilities, + DecisioningPlatform, + HttpSigCredential, + InMemoryTaskRegistry, + SingletonAccounts, + signing_only_registry, +) +from adcp.decisioning.handler import PlatformHandler +from adcp.decisioning.registry import validate_billing_for_agent +from adcp.server.base import ToolContext + +# Codes the framework MUST NOT raise from the Tier 2 commercial-identity +# gate after this PR. Adopters who match on these on the wire need to +# migrate to the new shape per the CHANGELOG. +_REMOVED_CODES = frozenset( + { + "AGENT_SUSPENDED", + "AGENT_BLOCKED", + "REQUEST_AUTH_UNRECOGNIZED_AGENT", + "INVALID_BILLING_MODEL", + } +) + + +@pytest.fixture +def executor(): + pool = ThreadPoolExecutor(max_workers=4, thread_name_prefix="tier2-spec-") + yield pool + pool.shutdown(wait=True) + + +def _signed_auth_info(agent_url: str) -> AuthInfo: + return AuthInfo( + kind="http_sig", + credential=HttpSigCredential( + kind="http_sig", + keyid="kid-1", + agent_url=agent_url, + verified_at=1700000000.0, + ), + ) + + +def _make_handler(platform: DecisioningPlatform, executor, registry) -> PlatformHandler: + return PlatformHandler( + platform, + executor=executor, + registry=InMemoryTaskRegistry(), + buyer_agent_registry=registry, + ) + + +class _RejectingPlatform(DecisioningPlatform): + """Test platform whose every method asserts it's not invoked. + + The Tier 2 gate runs BEFORE the platform method, so any platform + method invocation here means the gate let the request through + when it shouldn't have. + """ + + capabilities = DecisioningCapabilities() + accounts = SingletonAccounts(account_id="acct-1") + + async def get_products(self, req, ctx): # pragma: no cover - asserts not called + raise AssertionError("Tier 2 gate should have rejected before this") + + +# --------------------------------------------------------------------------- +# All four removed codes are gone from the four denial paths. +# --------------------------------------------------------------------------- + + +@pytest.mark.asyncio +async def test_registry_miss_does_not_raise_legacy_code(executor) -> None: + async def lookup(_url: str) -> BuyerAgent | None: + return None + + handler = _make_handler(_RejectingPlatform(), executor, signing_only_registry(lookup)) + from adcp.types import GetProductsRequest + + with pytest.raises(AdcpError) as exc: + await handler.get_products( + GetProductsRequest(buying_mode="brief", brief="any"), + ToolContext(metadata={"adcp.auth_info": _signed_auth_info("https://x/")}), + ) + assert exc.value.code not in _REMOVED_CODES + + +@pytest.mark.asyncio +async def test_suspended_agent_does_not_raise_legacy_code(executor) -> None: + suspended = BuyerAgent(agent_url="https://s/", display_name="S", status="suspended") + + async def lookup(_: str) -> BuyerAgent | None: + return suspended + + handler = _make_handler(_RejectingPlatform(), executor, signing_only_registry(lookup)) + from adcp.types import GetProductsRequest + + with pytest.raises(AdcpError) as exc: + await handler.get_products( + GetProductsRequest(buying_mode="brief", brief="any"), + ToolContext(metadata={"adcp.auth_info": _signed_auth_info("https://s/")}), + ) + assert exc.value.code not in _REMOVED_CODES + + +@pytest.mark.asyncio +async def test_blocked_agent_does_not_raise_legacy_code(executor) -> None: + blocked = BuyerAgent(agent_url="https://b/", display_name="B", status="blocked") + + async def lookup(_: str) -> BuyerAgent | None: + return blocked + + handler = _make_handler(_RejectingPlatform(), executor, signing_only_registry(lookup)) + from adcp.types import GetProductsRequest + + with pytest.raises(AdcpError) as exc: + await handler.get_products( + GetProductsRequest(buying_mode="brief", brief="any"), + ToolContext(metadata={"adcp.auth_info": _signed_auth_info("https://b/")}), + ) + assert exc.value.code not in _REMOVED_CODES + + +def test_billing_validation_does_not_raise_legacy_code() -> None: + agent = BuyerAgent( + agent_url="https://passthrough/", + display_name="Passthrough", + status="active", + ) + with pytest.raises(AdcpError) as exc: + validate_billing_for_agent(requested_billing="agent", agent=agent) + assert exc.value.code not in _REMOVED_CODES + + +# --------------------------------------------------------------------------- +# Recognized-but-denied paths carry details.scope="agent" + details.status. +# --------------------------------------------------------------------------- + + +@pytest.mark.asyncio +async def test_suspended_carries_scope_and_status_details(executor) -> None: + suspended = BuyerAgent(agent_url="https://s/", display_name="S", status="suspended") + + async def lookup(_: str) -> BuyerAgent | None: + return suspended + + handler = _make_handler(_RejectingPlatform(), executor, signing_only_registry(lookup)) + from adcp.types import GetProductsRequest + + with pytest.raises(AdcpError) as exc: + await handler.get_products( + GetProductsRequest(buying_mode="brief", brief="any"), + ToolContext(metadata={"adcp.auth_info": _signed_auth_info("https://s/")}), + ) + assert exc.value.code == "PERMISSION_DENIED" + assert exc.value.recovery == "correctable" + assert exc.value.details["scope"] == "agent" + assert exc.value.details["status"] == "suspended" + + +@pytest.mark.asyncio +async def test_blocked_carries_scope_and_status_details(executor) -> None: + blocked = BuyerAgent(agent_url="https://b/", display_name="B", status="blocked") + + async def lookup(_: str) -> BuyerAgent | None: + return blocked + + handler = _make_handler(_RejectingPlatform(), executor, signing_only_registry(lookup)) + from adcp.types import GetProductsRequest + + with pytest.raises(AdcpError) as exc: + await handler.get_products( + GetProductsRequest(buying_mode="brief", brief="any"), + ToolContext(metadata={"adcp.auth_info": _signed_auth_info("https://b/")}), + ) + assert exc.value.code == "PERMISSION_DENIED" + assert exc.value.recovery == "correctable" + assert exc.value.details["scope"] == "agent" + assert exc.value.details["status"] == "blocked" + + +# --------------------------------------------------------------------------- +# Unrecognized paths OMIT details (omit-on-unestablished-identity rule). +# --------------------------------------------------------------------------- + + +@pytest.mark.asyncio +async def test_registry_miss_omits_details(executor) -> None: + """Registry returned None — the unrecognized-agent path. ``details`` + MUST be empty so the wire shape is indistinguishable from the + recognized-but-denied paths. ``scope`` would itself be the + discriminator that leaks onboarding state to an external attacker. + """ + + async def lookup(_url: str) -> BuyerAgent | None: + return None + + handler = _make_handler(_RejectingPlatform(), executor, signing_only_registry(lookup)) + from adcp.types import GetProductsRequest + + with pytest.raises(AdcpError) as exc: + await handler.get_products( + GetProductsRequest(buying_mode="brief", brief="any"), + ToolContext(metadata={"adcp.auth_info": _signed_auth_info("https://x/")}), + ) + assert exc.value.code == "PERMISSION_DENIED" + assert exc.value.recovery == "correctable" + # The critical no-leak property: NO scope, NO status, NO agent_url. + assert exc.value.details == {} + + +@pytest.mark.asyncio +async def test_no_credential_omits_details(executor) -> None: + """No credential at all on the request — the framework's + unauthenticated-with-registry path. Same wire shape as registry + miss; ``details`` empty.""" + + async def lookup(_: str) -> BuyerAgent | None: + return None + + handler = _make_handler(_RejectingPlatform(), executor, signing_only_registry(lookup)) + from adcp.types import GetProductsRequest + + with pytest.raises(AdcpError) as exc: + await handler.get_products( + GetProductsRequest(buying_mode="brief", brief="any"), + ToolContext(), + ) + assert exc.value.code == "PERMISSION_DENIED" + assert exc.value.details == {} + + +@pytest.mark.asyncio +async def test_unknown_status_omits_details(executor) -> None: + """Defense-in-depth: a registry row whose status the framework + cannot interpret (typo, future enum value, adopter-custom string) + is routed through the omit-on-unestablished-identity path. The + framework cannot defensibly project the unknown status string on + the wire (it might encode commercial state the framework doesn't + understand), so ``details`` is omitted.""" + weird = BuyerAgent.__new__(BuyerAgent) + object.__setattr__(weird, "agent_url", "https://w/") + object.__setattr__(weird, "display_name", "W") + object.__setattr__(weird, "status", "deleted") + object.__setattr__(weird, "billing_capabilities", frozenset({"operator"})) + object.__setattr__(weird, "default_account_terms", None) + object.__setattr__(weird, "allowed_brands", None) + object.__setattr__(weird, "ext", {}) + + async def lookup(_: str) -> BuyerAgent | None: + return weird + + handler = _make_handler(_RejectingPlatform(), executor, signing_only_registry(lookup)) + from adcp.types import GetProductsRequest + + with pytest.raises(AdcpError) as exc: + await handler.get_products( + GetProductsRequest(buying_mode="brief", brief="any"), + ToolContext(metadata={"adcp.auth_info": _signed_auth_info("https://w/")}), + ) + assert exc.value.code == "PERMISSION_DENIED" + assert exc.value.details == {} + + +# --------------------------------------------------------------------------- +# Billing capability path — rejected_billing required, no permitted leak. +# --------------------------------------------------------------------------- + + +def test_billing_validation_carries_rejected_billing() -> None: + agent = BuyerAgent( + agent_url="https://passthrough/", + display_name="Passthrough", + status="active", + ) + with pytest.raises(AdcpError) as exc: + validate_billing_for_agent(requested_billing="agent", agent=agent) + assert exc.value.code == "BILLING_NOT_PERMITTED_FOR_AGENT" + assert exc.value.recovery == "correctable" + assert exc.value.details["rejected_billing"] == "agent" + + +def test_billing_validation_carries_suggested_billing_when_permitted_nonempty() -> None: + agent = BuyerAgent( + agent_url="https://x/", + display_name="X", + status="active", + billing_capabilities=frozenset({"operator", "agent"}), + ) + with pytest.raises(AdcpError) as exc: + validate_billing_for_agent(requested_billing="advertiser", agent=agent) + assert exc.value.details["suggested_billing"] in {"agent", "operator"} + + +def test_billing_validation_does_not_leak_permitted_subset() -> None: + """Critical no-leak property: the full ``permitted_billing`` subset + is the agent's commercial relationship with the seller. Surfacing + it on every rejected request would let a misconfigured buyer probe + and exfiltrate the matrix one mode at a time. ``details`` carries + ``rejected_billing`` and an optional ``suggested_billing`` — not + the whole set.""" + agent = BuyerAgent( + agent_url="https://x/", + display_name="X", + status="active", + billing_capabilities=frozenset({"operator", "agent"}), + ) + with pytest.raises(AdcpError) as exc: + validate_billing_for_agent(requested_billing="advertiser", agent=agent) + details = exc.value.details + assert "permitted_billing" not in details + # Defense-in-depth: agent_url is also a leak vector and is not echoed. + assert "agent_url" not in details