Skip to content

feat(types): Account v3 projection helpers — bank-details write-only guard (#356)#371

Merged
bokelley merged 1 commit intomainfrom
bokelley/account-bank-guard
May 2, 2026
Merged

feat(types): Account v3 projection helpers — bank-details write-only guard (#356)#371
bokelley merged 1 commit intomainfrom
bokelley/account-bank-guard

Conversation

@bokelley
Copy link
Copy Markdown
Contributor

@bokelley bokelley commented May 2, 2026

Closes #356.

Summary

New module `adcp.types.projections` with response-shape projection types that close the bank-details-on-read leak path on `Account` responses.

Per AdCP 3.0.x `core/business-entity.json`, `BusinessEntity.bank` carries `writeOnly: true` — IBAN, BIC, routing/account numbers flow into the seller during account setup but MUST NOT be echoed in responses. Pydantic round-trips everything by default, so an adopter who reuses an internal `Account` model on the response path leaks bank details unknowingly.

Design choice — Option 2 (projection types)

Per security-reviewer on the issue, Option 1 (model_validator strip) has multiple silent bypass paths: `model_copy()`, direct `BusinessEntity` serialization, idempotency cache replay. Option 2 (projection types) is structurally correct — the field is type-narrowed to `None`, so an adopter holding the projection type literally cannot construct one with bank set.

Public surface

```python
from adcp.types import (
AccountResponse,
BusinessEntityResponse,
to_account_response,
)

Convert an internal Account (with bank populated) to a safe response:

response = to_account_response(internal_account)

response.billing_entity.bank is None; serialization omits the field.

Or construct directly:

resp = AccountResponse(
account_id="acct-1", name="Acme", status="active",
billing_entity={"legal_name": "Acme Corp"}, # bank=... raises ValidationError
)
```

Behavior pinned by tests

  • `BusinessEntityResponse(bank=...)` → `ValidationError`
  • Other fields (`legal_name`, `vat_id`, `address`, `contacts`) round-trip unchanged
  • `model_dump()` excludes `bank` even if smuggled past the type system (defense in depth)
  • `AccountResponse.billing_entity` annotation references `BusinessEntityResponse` (the whole guard collapses if this binding regresses)
  • `AccountResponse.model_validate({...with nested bank...})` raises
  • `to_account_response(internal_account)` strips bank, preserves everything else
  • `reporting_bucket` is NOT write-only (per ad-tech-protocol-expert clarification — seller-provisioned, buyer-readable for offline-delivery coordinates) and MUST NOT be stripped — pinned by a regression test.

Out of scope (track separately)

  • `GovernanceAgent.authentication` is also `writeOnly` but lives in a generated nested type with a different adopter contract.
  • `sync_accounts_response.Account` is built via the `SyncAccountsResponse1`/`SyncAccountsResponse2` discriminator branches and doesn't share the `billing_entity` wire shape with `core.Account`. Different leak surface, different fix.
  • The triage flagged a docstring example `request_snapshot = req.model_dump()` that prescribes writing IBAN into adopter stores; orthogonal to projections, deferred.

Local gates

  • `uv run ruff check src/` — clean
  • `uv run mypy src/adcp/` — Success: no issues found in 744 source files
  • `uv run pytest tests/` — all green; `test_public_api_surface_matches_snapshot` updated for the 3 new exports.

Test plan

  • `uv run pytest tests/test_account_projections.py -v` — 7 passed
  • `uv run pytest tests/test_public_api.py -q` — snapshot regenerated and matches
  • Full suite green

🤖 Generated with Claude Code

…guard

Adds adcp.types.projections — projection types that strip write-only
fields on the response edge. Closes #356.

Per AdCP 3.0.x spec, BusinessEntity.bank carries writeOnly: true:
buyers send bank coordinates as part of account setup, sellers store
them, but sellers MUST NOT echo them back. Pydantic's default
serialization round-trips everything, so an adopter who reuses an
internal Account model on the response path can leak IBAN/routing
numbers without realizing it.

Per security-reviewer's pre-PR findings on issue #356, Option 2
(projection types) is structurally safer than Option 1 (model_validator)
because it closes the model_copy / direct-serialization / idempotency-
cache bypass paths a strip-only validator leaves open.

What ships:

- BusinessEntityResponse: subclass that rejects bank=... at construction
  (ValidationError) and excludes the field from serialization regardless.
- AccountResponse: subclass binding billing_entity to
  BusinessEntityResponse so the guard travels with Account-level
  serialization.
- to_account_response(account): adopter helper that projects an internal
  Account to the response shape, dropping bank along the way. Preserves
  reporting_bucket (NOT write-only — buyer-readable for offline delivery
  coordinates per ad-tech-protocol-expert clarification on the issue).

Tests: 7 behavioral assertions covering construction guard, serialization
guard, type binding, AccountResponse rejection, internal-Account strip
helper, and reporting_bucket preservation.

Out of scope (track separately): GovernanceAgent.authentication is
also write-only but lives in a generated nested type with a different
adopter contract; sync_accounts_response.Account is constructed via
SyncAccountsResponse1/2 discriminator branches that don't share
billing_entity wire shape with core.Account.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@bokelley bokelley merged commit e4415e3 into main May 2, 2026
12 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

feat(types): Account v3 projection helpers (billing_entity write-only bank-details guard, reporting_bucket)

1 participant