Repository navigation
[security-critical] refactor(node): Always set the catalog maps on discovered peers - #1284
alexanderludwig wants to merge 3 commits into
Conversation
…alog The proxy tests now build peers with the node's real announcement converter instead of hand-made PeerInfo objects. They check what the buyer shows after a refresh: /v1/models and /_antseed/peers drop a withdrawn service, and drop the categories, protocols, billing models and capabilities a seller stops announcing. A negotiator test checks that a seller with no providers gets no pricing map. Co-Authored-By: Claude <noreply@anthropic.com>
_lookupResultToPeerInfo now sets providerPricing and the four
providerService* maps on every peer, as {} when the seller announces
nothing for them. A newer announcement then replaces an older one in a
plain spread merge, and a catalog field added later works the same way.
The negotiator's pricing-map guard checked for a missing
providerPricing. It now checks for an empty one, so a seller that
announces no providers still gets no pricing map and the buyer signs
what it signed before.
Co-Authored-By: Claude <noreply@anthropic.com>
Discovered peers now carry every catalog map, so the spread merge in _replacePeers replaces a withdrawn catalog by itself. PEER_CATALOG_FIELDS and the loop that deleted missing fields go. Co-Authored-By: Claude <noreply@anthropic.com>
Antseed Review SummaryThe PR makes discovery consistently populate all five provider catalog maps, using empty objects when sellers announce no entries. The CLI buyer now relies on those maps to replace cached catalog data during refresh, while the payment negotiator treats an empty pricing map like an absent one. Regression tests cover catalog withdrawal, persistence across restart, removal of individual catalog details, and negotiation without advertised pricing.
Confidence Score: 5/5Safe to merge; no actionable issues found. Reviewed every diff and the complete changed core functions, tracing discovery, cache refresh, persistence, pricing negotiation, and relevant catalog consumers. The provided test runner passed all 45 buyer-core tests, all 62 targeted node tests, and the CLI suite with 1374 passed, one skipped, and no failures; required builds also succeeded. Empty maps preserve the intended withdrawal and payment-pricing behavior without changing wire formats or requiring persisted-state migrations. Live peer-network and on-chain integration were not exercised.
|
| Filename | Overview |
|---|---|
packages/node/src/node.ts |
Always returns all five catalog maps from discovery conversion, allowing fresh empty announcements to replace cached entries. |
packages/node/src/types/peer.ts |
Documents the discovery invariant while keeping catalog fields optional for legacy persisted and manually constructed peers. |
apps/cli/src/proxy/buyer-proxy.ts |
Removes explicit catalog-field deletion and relies on fresh discovery maps during peer merging; inspected callers support this behavior. |
packages/buyer-core/src/buyer-payment-negotiator.ts |
Checks pricing-map entries rather than object presence, preserving the absent-pricing behavior for newly normalized empty maps. |
apps/cli/src/proxy/buyer-proxy.test.ts |
Adds converter-backed tests for complete catalog withdrawal, persisted withdrawal, and removal of optional service details. |
packages/node/tests/buyer-payment-negotiator.test.ts |
Verifies that discovery with no providers passes no pricing map into spending authorization. |
packages/node/tests/public-address-override.test.ts |
Checks consistent discovered-peer fields and updates the legacy metadata expectation to an empty billing-model map. |
CHANGELOG.md |
Describes the catalog-map normalization, externally visible JSON changes, and payment-negotiation compatibility. |
Diagram
flowchart TD
A[Seller metadata] --> B[Node discovery conversion]
B --> C[Peer with all five catalog maps]
C --> D[Buyer proxy cache merge]
D --> E[Fresh empty maps replace withdrawn catalog]
E --> F[Peer API and persisted state]
C --> G[Payment negotiator]
G --> H[Empty pricing map and zero defaults]
H --> I[Authorize without a pricing map]
Core files read in full: packages/node/src/node.ts, packages/node/src/types/peer.ts, packages/buyer-core/src/buyer-payment-negotiator.ts, apps/cli/src/proxy/buyer-proxy.ts
Reviews (1): Last reviewed commit: [security-critical] refactor(node): Always set the catalog m (cbe748091b) | requested by @alexanderludwig | Tag @reviewant with "re-review" to run again
Why
When a seller stops announcing part of its catalog, the buyer must drop it on the next refresh. #1201 fixed this in the buyer proxy with a list of the five catalog fields to delete when an announcement leaves them out. A catalog field added later had to join that list, or the stale data came back. This PR fixes it where peers are built (Option B in #1251). The node always sets the five maps, as
{}when the seller announces nothing, so the proxy's plain spread merge replaces the old catalog.What changed
_lookupResultToPeerInfoalways setsproviderPricing,providerServiceCategories,providerServiceApiProtocols,providerServiceUnitBillingModelsandproviderServiceCapabilities. ThePeerInfodoc tells consumers to check for entries, not for the field._buildPricingMapin the buyer negotiator checks for an emptyproviderPricinginstead of a missing one. A seller that announces no providers still gets no pricing map, so the buyer signs what it signed before.PEER_CATALOG_FIELDSand its delete loop leave_replacePeers.Scope
_replacePeersnow clears a withdrawn catalog only when the incoming peer carries the maps. Its three callers all pass discovered peers or the cache itself, but nothing in the type enforces that. Peers restored from abuyer.state.jsonwritten before this change, or built by hand, can still leave the fields out. The type keeps them optional for that reason, and because router plugins receivePeerInfo.capabilitieslist. It has the same omit-then-merge pattern, so a withdrawn protocol capability stays cached. Fixing it changes behavior: an empty list would stop the fallback tometadata.capabilities. That needs its own change.Tradeoffs
PeerInfowould break every plugin and test that builds aPeerInfo, and saved state would still need a default. Optional plus a documented contract is the smaller change. A narrower type for discovery output (all five required) would let the compiler hold the contract for_replacePeerstoo. It is a reasonable follow-up, but it widens the published API.Blast Radius
@antseed/nodeconsumers now get{}where they got a missing field. Every in-tree reader either iterates, indexes with?., or is the negotiator guard fixed here.GET /_antseed/peers,antseed network browse --jsonandantseed network peer --jsonprint{}for an empty map, andbuyer.state.jsonsaves{}instead ofnull. The CHANGELOG says so. Buyer-core is in the diff, so this is security-critical. Its only change is the guard above: for peers the node builds, what the buyer signs or reserves does not change. A direct caller of the buyer-core negotiator that passesproviderPricing: {}with zero default prices now gets no pricing map, as it would for a missing field.Verification
buyer.state.jsonthe proxy saved back throughparsePersistedPeersand checks the withdrawn catalog stays gone. With the old delete loop disabled, the proxy pins fail (actual: ['video-model'] expected: []). With only the node change, the negotiator pin fails (expected { defaults: {…}, services: {} } to be undefined). A node test requires every field of a fully announced peer, exceptcapabilities, to stay present when the seller announces nothing, so a catalog map added later with a conditional spread fails it once that test's announcement fills in the new field.unit-test-verifiedfor this head and ran the package checks. Tests and typecheck pass for@antseed/node(1411 tests),@antseed/buyer-core,@antseed/router-local,@antseed/web-sdkand@antseed/protocol, and the node payment and signing tests by name. The CLI passes typecheck and all tests but one. The desktop passes its main build, main tests (377) and renderer tests (503). Three failures also happen onmainin the build environment: a CLI state-file test that writes under/tmp, the desktop setuid packaging test, and a desktop renderer type error (freeModelsonTrustBreakdown).--chain local, onmainand on this branch:chat-basicandpayment-flowprintPASSEDon both.issue-1200-spendingauth-catchupfails on both, the same way:FAIL buyer signed exactly the delivered work after the catch-up(signed 500018, delivered 54, overSigned 499964). It is the red scenario for Buyer over-signs SpendingAuth by minBudgetPerRequest ($0.50) on 402 catch-up #1200, which is still open and unfixed onmain. It is a known failure, not a pass, and this change does not affect it. The branch run was at an earlier head. Since then only tests, the CHANGELOG and doc comments changed, and the verifier found all 443 emitted JavaScript files in node, buyer-core and the CLI identical between the two heads.Catalog fields after a seller drops from a full catalog to pricing only. The author made this transcript with a script that runs the real
BuyerProxyandAntseedNodeagainst a stubbed seller announcement:Security-critical paths touched:
packages/buyer-core/src/buyer-payment-negotiator.ts.Closes #1251.
🤖 Generated with Claude Code
Closes #1251