Skip to content

fix(registry): enforce identity-not-authorization on property-save (#5749 gap #2)#5750

Merged
bokelley merged 1 commit into
mainfrom
fix/registry-save-identity-not-authorization
Jun 29, 2026
Merged

fix(registry): enforce identity-not-authorization on property-save (#5749 gap #2)#5750
bokelley merged 1 commit into
mainfrom
fix/registry-save-identity-not-authorization

Conversation

@bokelley

@bokelley bokelley commented Jun 28, 2026

Copy link
Copy Markdown
Contributor

Closes gap #2 of RFC #5749.

Problem

POST /api/properties/save (saveProperty) is the community-registry property write surface. It required a caller-supplied authorized_agents array (400 if absent) and stored it verbatim into the persisted community-mirror adagents.json document. That let any authenticated registry caller assert sales authorization (which agent may sell a domain's inventory) for a domain they may not own.

AdCP's invariant: the owner's origin /.well-known/adagents.json is the sole authorization source. Community-registry rows never assert authorization and must carry authorized_agents: []docs/governance/property/adagents.mdx:296: a community/catalog mirror "Sets authorized_agents: [] … the mirror MUST NOT fabricate one." The sibling community-mirror write path (server/src/routes/community-mirrors.ts:190-200) already drops caller authorization and forces []. This brings saveProperty in line.

Why it matters

The registry's authorization view is normally re-derived from a live origin crawl — but for an un-crawled brand-new domain (exactly the population this endpoint creates rows for) there is nothing to overwrite a phantom write, and the SDK's lookupDomain returns authorized_agents to consumers. Combined with the documented TS/Python resolver divergence (adcontextprotocol/adcp-client#2301, where the TS resolver doesn't honor revocation), an un-scrubbed write is a latent sales-authorization spoof. The scrub is now unconditional and structurally guaranteed — the persisted object hard-codes authorized_agents: [] and never spreads the request body.

Change

  • Drop caller-supplied authorized_agents; always persist []. The single adagentsJson object flows into both the create and edit paths, so a previously-stored non-empty array is overwritten to [] on the next save (heals a prior spoof).
  • Stop requiring authorized_agents; mark it optional + documented-as-ignored in the OpenAPI spec; regenerate static/openapi/registry.yaml.
  • Update the registry save-property docs (request body + cURL/JS/Python samples no longer carry authorization).
  • New integration test (registry-property-save-identity.test.ts): caller-supplied authorization is dropped → stored []; omitting the field no longer 400s; the edit path heals a prior non-empty array.

Scope

Scoped to the public REST write surface the RFC names. Both a code review and a security review ran on the diff and came back clean.

The security review surfaced that other caller-supplied writers of hosted_properties.adagents_json carry the same gap — notably the Addie save_property MCP tool (server/src/addie/mcp/property-tools.ts), which auto-publishes (is_public:true, review_status:'approved') and syncs caller-supplied agents into the federated authorization index — plus admin save endpoints in http.ts and mcp-tools.ts. These need per-path review to distinguish caller-supplied from origin-crawl-derived authorization (the latter must not be scrubbed), so they're deliberately not bundled here. Follow-up: #5751.

Gap #1 of the RFC (an authority model / domain-control challenge on the write — already specced at docs/registry/index.mdx:267 but unimplemented) is a separate issue.

Test plan

  • tsc -p server/tsconfig.json --noEmit — clean
  • vitest run server/tests/integration/registry-property-save-identity.test.ts — 3/3 pass
  • npm run build:openapistatic/openapi/registry.yaml regenerated and committed
  • pre-commit (unit + typecheck) and pre-push (broken-links) hooks pass

🤖 Generated with Claude Code

…5749 gap #2)

POST /api/properties/save (saveProperty) is the community-registry property
write surface. It required a caller-supplied `authorized_agents` array (400 if
absent) and stored it verbatim into the persisted community-mirror
adagents.json document — letting any authenticated caller assert sales
authorization for a domain they may not own.

AdCP's invariant: the owner's origin /.well-known/adagents.json is the sole
authorization source; community-registry rows never assert authorization and
must carry `authorized_agents: []` (governance/property/adagents.mdx). The
sibling community-mirror write path (community-mirrors.ts) already drops caller
authorization and forces []. This brings saveProperty in line.

- Drop the caller-supplied `authorized_agents`; always persist []. The single
  adagentsJson object flows into both the create and edit paths, so a
  previously-stored non-empty array is overwritten to [] on the next save.
- Stop requiring `authorized_agents`; mark it optional/ignored in the OpenAPI
  spec and document the identity-only contract.
- Update the registry save-property docs (request body + cURL/JS/Python samples
  no longer carry authorization).
- Add an integration test: caller-supplied authorization is dropped, omitting
  the field no longer 400s, and the edit path heals a prior non-empty array.

Scoped to the public REST write surface named in the RFC. Other caller-supplied
writers of hosted_properties.adagents_json (Addie save_property MCP tool,
admin save endpoints, mcp-tools.ts) carry the same gap and need per-path review
to distinguish caller-supplied from origin-crawl-derived authorization — tracked
as a follow-up, not bundled here.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@mintlify

mintlify Bot commented Jun 28, 2026

Copy link
Copy Markdown

Preview deployment for your docs. Learn more about Mintlify Previews.

Project Status Preview Updated (UTC)
adcp 🟢 Ready View Preview Jun 28, 2026, 10:18 PM

💡 Tip: Enable Workflows to automatically generate PRs for you.

@aao-release-bot aao-release-bot Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Clean, contained security fix. Right principle: identity is not authorization — a community-registry row never asserts who may sell a domain's inventory, and the owner's origin adagents.json stays the sole authorization source.

Things I checked

  • Scrub is structural, not conditional. Single adagentsJson object at registry-api.ts:4476 hard-codes authorized_agents: [] and never spreads the request body. authorized_agents is no longer destructured at :4458, so there's no dangling reference and no path to re-merge caller input. security-reviewer: no High/Medium.
  • Edit path heals a prior spoof. editCommunityProperty (property-db.ts:685-699) does a whole-column adagents_json = $N replacement, not a JSON merge — a pre-fix row holding a non-empty array is overwritten wholesale to [] on the next save. Test 3 proves it via revision_number (edit-branch-only) plus the [] assertion.
  • Matches the sibling write path. Same invariant as community-mirrors.ts:194-198, which already drops caller authorization and forces [].
  • Spec-sound. docs/governance/property/adagents.mdx:296 — a community mirror "Sets authorized_agents: [] … the mirror MUST NOT fabricate one." Empty array plus non-empty properties is a schema-valid catalog-only document (static/schemas/source/adagents.json:712-728). ad-tech-protocol-expert: sound.
  • No drift, no fork. static/openapi/registry.yaml was regenerated from the Zod source, not hand-edited — .optional(), the "Ignored." description, example: [], and the required removal all line up across schema, OpenAPI, and docs/registry/index.mdx.
  • Required→optional flip is non-breaking. Relaxing a request-body field from required to optional is backward-compatible contract widening — every previously-valid body still validates. Patch-level, no major warranted.
  • No changeset required. check-changeset-protocol-scope.cjs scopes the gate to static/schemas/source/, static/registry/, docs/reference/ — none of this PR's paths (static/openapi/, docs/registry/, server/) match. Absence is not a block.

Follow-ups (non-blocking — file as issues)

  • Other un-scrubbed writers. #5751 already tracks the Addie save_property MCP tool (auto-publishes is_public:true, review_status:'approved' and syncs caller agents into the federated index) plus the admin endpoints in http.ts/mcp-tools.ts. Correct to scope those out here — they mix caller-supplied and origin-crawl-derived authorization, and a blanket [] there would wrongly scrub legitimate crawled authorization.
  • Consider a changeset anyway. Not gate-required, but static/openapi/registry.yaml and docs/registry/index.mdx are adopter-visible. A patch changeset would keep the published-surface change tracked. Wording-only — doesn't block.

Minor nits (non-blocking)

  1. Silent drop vs. explicit reject. A caller still sending non-empty authorized_agents gets a 200 and no signal that it was ignored. Defensible — and the OpenAPI description documents it as "Ignored" — but a 400 on non-empty input would be the louder, more spoof-hostile contract. Confirm silent-drop is the intended posture.

The third drift-cleanup-by-regeneration in recent registry work; the source-and-generated-YAML staying in lockstep is the part that makes that boring, which is the goal.

LGTM. Follow-ups noted below.

@bokelley bokelley merged commit 879d112 into main Jun 29, 2026
20 checks passed
@bokelley bokelley deleted the fix/registry-save-identity-not-authorization branch June 29, 2026 03:27
bokelley added a commit that referenced this pull request Jun 30, 2026
#5752)

The community-registry write stays open (anyone can stage a non-authoritative,
private community row — #5750). Authority over a domain is established by
binding an owner ON successful origin verification, not by gating the write.

Flow:
1. An authenticated account claims a domain → AAO issues a claim-specific
   authoritative_location URL (…/adagents.json?adcp_claim=<token>).
2. The account places that single pointer at their own origin (one IT action).
3. verify-origin reads the token and binds workos_organization_id to the
   claim's org, publishes the row, and marks it origin-verified.

The token is the per-account artifact that proves WHICH account owns a domain —
a plain domain-keyed pointer proves only that the origin endorses AAO hosting,
not who the owner is. Binding is driven by the token the origin pointer carries,
never by who triggers verification, so any authenticated caller may trigger it
and a squatter cannot bind a domain they don't control. An existing verified
owner is never overwritten, and issuing a claim for a domain locked to another
org is refused.

- Migration 523: claim_token + claimant_org_id on hosted_properties.
- property-db: issueDomainClaim, bindOwnerFromVerifiedClaim (never changes a
  different owner; publishes the row on bind).
- POST /properties/hosted/:domain/claim returns the claim-specific pointer URL.
- Verifier: extract adcp_claim from the pointer, base-match against the AAO
  hosted URL, bind on success; outcome carries bound_org_id.
- verify-origin handler inverted: no longer requires a pre-bound owner; trigger
  is open, binding is token-driven (replaces the NULL-owner/org-mismatch 403s).
- /properties/save: owner-locked edit — once verified+bound, only the owner may
  edit (community rows stay openly editable). authorized_agents stays [] per
  #5750; owner-authored authz is a deliberate follow-up.
- Regenerated static/openapi/registry.yaml.

Tests: new registry-domain-claim-bind.test.ts (token binds claimant not caller;
no/wrong token verifies but doesn't bind; locked domain refuses re-claim; owner
never overwritten). Updated the verify-origin auth test for the new invariant.
Fixed a discovered_properties cleanup leak + hook-timeout in the #5750 save test.

Follow-ups: periodic TTL re-verify + lapse; DNS-TXT pointer flavor; owner-authored
authorized_agents on a verified-locked record; extend owner-lock to Addie/admin
write paths (#5751).

Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
bokelley added a commit that referenced this pull request Jul 1, 2026
…nts (#5787)

The catalog fact surface (POST /api/registry/resolve, /catalog/disputes) is live
and serves real traffic, but catalog-api.ts registered zero OpenAPI paths — the
whole router was absent from static/openapi/registry.yaml. Since SDK clients
generate their types from the published OpenAPI (adcp-client's types.generated.ts
sources https://agenticadvertising.org/openapi/registry.yaml), the "send us facts"
surface could not be typed. This makes it part of the contract.

- New server/src/schemas/catalog-openapi.ts (mirrors the member-agents-openapi.ts
  pattern — standalone so the generator imports it without route-factory deps).
  Registers: resolveIdentifiers (POST /api/registry/resolve), fileCatalogDispute
  (POST /api/registry/catalog/disputes), getCatalogDispute (GET .../disputes/{id}).
- Schemas mirror the Zod validators + result types in catalog-api.ts /
  catalog-db.ts / catalog-governance.ts exactly: the provenance enum, the
  identifiers[1..10000] cap, ResolveResponse.summary's five fields (incl
  not_found), and the dispute TriageResult ({dispute_id, action_taken, reason}).
- property_rid is documented as a non-authoritative join/match handle, never an
  authorization credential (mirrors the #5750 identity-not-authorization lesson).
- Wired into scripts/generate-openapi.ts + a "Property Catalog" tag;
  regenerated static/openapi/registry.yaml.

Prerequisite for the SDK fact-contribution surface (specs/sdk-fact-contribution.md,
#5782): once this publishes, @adcp/client and adcp regenerate types and add the
reportIdentifiers()/disputeFact() methods. Read-side (browse/sync) OpenAPI is a
fast follow-on. No behavior change — documentation of live endpoints only.

Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
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.

1 participant