Go SDK: update low tier - #32
Merged
Merged
Conversation
The Connect client exposed two verbs, Discover and Execute. The agent integration role needs six, and nothing asserted that the client covered the protocol, so the gap stayed invisible while the reference implementation re-implemented all four by hand. Adds ReportUsage, Dispute, Fetch and a separate broker client carrying Resolve. No .proto change, so no regeneration and no new dependencies. A usage report reaches the Exchange that ISSUED the offer. The domain comes off the report message and the endpoint from that Exchange's own well-known manifest; there is deliberately no option to supply an endpoint, so a configured origin cannot become the default. Five checks precede the send — plain hostname, manifest lookup, same-host endpoint, SSRF guard on the report itself, no redirects — behind an LRU-bounded per-origin client pool, since which Exchanges appear is driven by incoming offers. Fixes two defects in the verbs that already shipped. Execute built a request carrying neither Requester nor AgentAcceptance, so a reference Exchange refuses it; it now fills both, signing through the injected Signer so the SDK never holds a key. Discover read only the flat offers list and dropped offer_groups, returning an empty result with no error against a server that leaves the flat list empty as the contract asks. Both discovery verbs now return one group per requested URI. A flat list has nowhere to put the typed absence reason, and a refused URI vanishes entirely — an empty group has no offer to carry its identity back. The reasons are different agent actions, so collapsing them is the trial-and-error the vocabulary exists to prevent. This changes Discover's return type; nothing outside this repo consumes the client. Supporting L1 work: the agent-binding proof signer, lifted rather than rewritten and taking a Signer plus the public half so custody stays with the application; the two host predicates; URL redaction; and the delivery edge's refusal vocabulary mapped onto the typed enum. The shared proof vectors are now generated by the shipped signer instead of a test-internal copy, byte-identically — the negative vector stays hand-built, because the shipped signer refuses that input by design. Signed legs refuse redirects where the guarded client follows them. Following one would re-sign an RPC for a target the peer chose after the endpoint check had passed, or hand a fresh proof of possession of the agent's key to whatever host the first hop named. Transport errors are rebuilt before surfacing: the HTTP client wraps them in a value carrying the full URL, query included, which leaks a credential past the SDK's own redaction. The API-surface gate would not have run on this change. Its workflow paths covered only two Go packages, so a change adding exports elsewhere and editing the symbol map to match triggered no job at all. Widened to every Go SDK package plus the map, the matrix and its generator. The parity entry recording the typed client as a resolved runtime-native divergence is rewritten as an open decision: the API-surface design governs and specifies a thin unary client for the other two languages with the same verb names, so what diverges is the transport, not the API.
A review of the verb work found the same shape five times: a careful posture was built for two verbs and not extended to the other four, or to the surface it is reached through. Nothing here changes a decision; it finishes applying the ones already made. The leg that dials an Exchange an offer named was unbounded three ways, and in each the SDK was weaker than the downstream code it exists to replace. Connect treats an unset read cap as "any size" while compressing every exchange, so a hostile peer's response decompressed without limit into the caller's memory; there was no deadline, so an Exchange that accepted a connection and never answered held a call, a goroutine and a socket indefinitely; and the endpoint cache beneath the client pool grew without bound over the same caller-influenced host space the pool is already bounded for. All three now carry a bound, and a caller can tune the Connect options through a seam that mirrors the server face. The SSRF guard is no longer removable by an option. The transport seam was documented as the content fetch's but also replaced the base of the offer-derived RPC pool, so a caller injecting a tracing or mTLS transport silently disarmed both the address and scheme guards on signed calls. A supplied transport now composes UNDERNEATH the guard; the only opt-out stays the deployment-level flags. The tests took that seam, which is why nothing proved the guard was installed at all — they now opt out the way a deployment does, and a private endpoint refused at dial time is asserted. The client offered a separately-custodied acceptance key the protocol has no slot for. One agent identity runs through the whole flow: the thumbprint of the request-signing key is what an Exchange verifies the detached acceptance against and what a delivery URL is bound to, so a second key is refused at execute and could never fetch what it bought. That option is gone; the public half of the one key remains, because a bound fetch presents it and a Signer cannot yield it. Two verbs could not be used against a reference service. Both the Exchange and the Broker resolve the calling agent from the requester and refuse a request that names none, while the client held that identity and applied it only to a purchase. Discovery now fills the requester and the protocol version on a clone, and only where the caller left them empty. The same fill-when-empty rule replaces an unconditional overwrite of the idempotency key on the report and dispute paths. Minting a default is intended; discarding a value the caller put in a required field is not — it turns each of their retries into a fresh action, which is the double-counting the field exists to prevent. Failures are classified by cause rather than by position. Every failure to reach a manifest was reported as a refusal to send, so a momentary outage read as a permanent verdict and a caller following that would drop a usage report for good; and after the checks passed the raw transport error came back, so one method answered with two unrelated error types depending on where it failed. A synthesized error detail now names the failing surface rather than the fetched URL, which is what makes the field a grouping key rather than an unbounded one. Host anchoring compares hostnames, not origins. Including the port left an Exchange advertising a non-default port permanently unable to receive a usage report, refused by a check that was never protecting anything there — TLS binds hostnames. The label-boundary rule is unchanged. Also: an unparseable delivery URL no longer reaches an error string, where a parse failure would print the live credential in its query; the redirect refusal and the whole public accessor surface of the error type gain the coverage they lacked; and the durable reasoning behind the anchoring rule, the bounds, the single agent identity, and what the manifest fetch does not guarantee is recorded beside the rest.
…nt cache Composing a caller's transport underneath the SSRF guard left one way through. net/http prefers a transport's OWN TLS dialer over DialContext whenever the scheme is https — which is every RAMP leg — so a base carrying DialTLSContext took the dial around the address pin, with no error to say the pin never ran. Reproduced on both legs the seam feeds: a signed usage report reached a loopback address, and a bound content fetch retrieved the bytes. Both TLS dialer fields are now cleared beside the proxy that was already cleared there, for the reason already stated in that function: each routes the dial around the address check rather than under it, so each is mutually exclusive with the pin by construction. TLSClientConfig is kept, so client certificates — the customisation the seam advertises — still work, and only the dialer is refused. The clear goes in the exported guard rather than its caller so it also covers consumers outside this repo; in-repo callers on the previous release all passed a nil base, so the field could not be populated and the gap was unreachable there. The endpoint resolver's two per-host structures grew without bound over the same offer-supplied host space the client pool above it is already bounded for, which the design record and the previous commit message both described as handled when it was not. The cache now evicts least-recently-used at a fixed cap, and the hand-rolled per-host mutex map — which grew even for hosts whose fetch failed — is a singleflight group, matching the sibling resolver in the same package and holding a host only while its fetch is in flight. The parity gate walked the live Go surface and asked whether each symbol was mapped; nothing asked the reverse, so an entry left behind when its symbol was deleted stayed in the generated matrix describing a surface that no longer existed. Removed the one such entry and added the mirror walk, so the next removal fails the gate instead of being published. Each guard fix carries a test that fails against the previous commit. The test that covered this seam passed an empty transport, which cannot carry a dialer — which is why it proved less than it appeared to.
Three knobs the tier below the client already had, with no way to reach them through NewClient. Each was configured by the application this client exists to replace, so adopting it would have read the value from the environment and silently stopped acting on it. The RFC 9421 request signature was pinned to the package default of five minutes. WithSignWindow moves it. Two reasons an application supplies its own: a deployment with a shorter freshness policy, and a peer that screens replays on (key id, signature) — signature timestamps have one-second resolution, so two identical requests inside one second sign to the same bytes, and a monotonic window is what keeps them distinct. The option is threaded through the one place the package builds a signing transport, so the home Exchange, the Broker face and the offer-derived leg all sign on identical terms. Deliberately narrow rather than a pass-through for the transport's whole option set. That set also carries a predicate that skips signing outright and a mode that co-signs every request as a relay does; neither belongs on an agent client by default. Widening is a per-option decision with its own reason. The delivery fetch was fixed at the tier defaults for both its deadline and its body cap. WithContentTimeout and WithMaxContentBytes move them. The body cap matters beyond tuning: an application carrying its own per-item budget gets wrong accounting when the two disagree, and an over-cap body is refused rather than truncated. Two scalars rather than one options struct, because that struct also carries the base transport, which arrives through its own option — a second way to set it would be a field that had to be silently ignored. Each test asserts the value on the wire rather than that an option set a field: the supplied window has to appear in the emitted Signature-Input, and the supplied cap has to produce the refusal, since a client that accepted the option and dropped it would pass a field assertion.
…mise The two caching tiers had hand-rolled the same bounded map, down to the same rationale paragraph copied word for word, and the two failure types rendered themselves with byte-identical code. Duplication in this codebase has already been observed to drift within a single commit, so each is now one implementation: a generic least-recently-used cache and a shared renderer, both internal because a bounded map and error prose are not protocol concepts and have no cross-language face to mirror. Four copies of the clone-and-type-assert preamble collapse into one helper, and the fill-when-empty envelope rule the two discovery verbs each stated separately now has one home. A full generic send helper for the two offer-routed verbs was considered and not taken: stamping the envelope needs settable field pointers, which generics cannot express without a per-type adapter that would reintroduce what it removed. Three defects behind those seams: The coalesced manifest fetch inherited the winning caller's context, so one caller walking away failed every other caller waiting on the same host, each reading it as the Exchange being unreachable. The shared fetch now outlives whoever triggered it, bounded by the resolver's own timeout. A missing signer was a malformed request on the purchase path and an unsignable one on the fetch path — the same misconfiguration under two classes a caller is told to branch on. It is unsignable on both. The agent-binding proof wrote its method and target URI verbatim into a line-delimited signature base with no check, so a control byte could add or split a component line and the signed bytes would stop describing the request a verifier reconstructs. Refused rather than escaped. Coverage now reaches the branches whose comments warn loudest about the consequence of getting them wrong: a transient resolve failure classified apart from a verdict, an idempotency key the caller put on the message surviving to the wire, and the proof window reaching the delivery signature. A test also pins that the two failure vocabularies keep spelling their shared classes the same word, which the bridge between them assumed and nothing enforced. A Broker resolve now refuses locally when no requester is configured, matching the purchase path: the peer declines such a request anyway, and naming the remedy beats relaying the refusal from a round trip away. Smaller corrections: a stale comment claimed a synthesized error detail names the fetched host, which the code deliberately does not do, and the test pinning it asserted only that a constant lacked a substring; the agent public key is copied rather than aliased; a failed requester clone falls back to nil rather than keeping a previous agent's identity; the read-cap constant takes the Default prefix its three siblings carry; and the deferred-SDK banner, pasted into seven pages in two variants, is two shared components — verified to keep its link under the build's link validator, which an .astro component would have silently stopped covering. The testing sample on the fetch-flow page imported the real client package while calling an API that package does not have; it names a clearly hypothetical path again, matching the banner that already declares the whole page deferred design.
…es it An Exchange's well-known manifest advertises where its service lives, and nothing said that address had to stay on the Exchange's own domain. The field read only "Exchange-only. ExchangeService endpoint URL", so no operator was ever told, and the rule existed in one client's code and nowhere else. It is now stated where an implementer will find it: the endpoint MUST be on the host that serves the manifest, or a subdomain of it, and MUST NOT carry userinfo. The manifest is only as trustworthy as the host that served it, so an endpoint naming an unrelated host would let whoever answers for the manifest redirect a signed call to a party the offer's signature never covered — and a dial-time address guard has no objection, because the destination is an ordinary public host. The match is on a full dot-delimited label boundary, so evil-a.com is not a subdomain of a.com. The port is not compared: TLS binds hostnames, not ports. Enforcement moves down a tier, from the client to the shared endpoint resolver. The check is a property of reading an endpoint out of a manifest, not of any one caller's plans for it, and every consumer needs it while none can be relied on to remember. Consumers that resolve through this package inherit it without changing a line. The client keeps its own check, because the resolver is an injectable seam and a signed call cannot be made conditional on a stranger's implementation having remembered the rule. Refusal carries its own sentinel rather than reusing the no-endpoint one. The distinction is load-bearing: a caller classifying retryability reads a transport failure as worth another try, and this is a verdict — the Exchange answered, and the answer is unusable. Userinfo is refused for a different reason with the same shape: the host comparison reads the authority and ignores any credentials before it, so an endpoint carrying them would pass and then have net/http stamp an Authorization header the SDK never chose, on a leg that already carries the agent's own signature. Marked breaking for implementers rather than for the wire: no field, message or encoding changes, and any Exchange already serving from its own domain is unaffected. One serving its API from a separate domain must front it under a subdomain. Three resolver tests advertised exactly that cross-host shape and now advertise themselves.
Both sides changed the same proto file, the same manifest page, and the same changelog pair: main added the registration schema and its failure detail, this branch added the low-tier client verbs and the endpoint host binding. Neither touches the other's fields. Resolution ledger: - mechanical: gen/descriptor.binpb, gen/go/ramp/v1/ramp.pb.go, gen/python/wire/models.py, gen/ts/wire/schemas.ts, gen/python/wire/unique.py, conformance/corpus/cases.json, docs/sdk-parity-matrix.md -> regenerated from the merged proto, never content-merged. Two of them had conflicted and two had auto-merged; both outcomes were discarded and rebuilt, since a line-merged descriptor or generated model is meaningless either way. - semantic: proto/ramp/v1/ramp.proto -> union, verified by diffing the result against each parent: it adds only the endpoint comment to main and only the registration schema to this branch. - semantic: website/.../protocol/exchange-manifest.mdx -> union, verified the same way (our endpoint-host-binding section, main's registration_schema rows). - semantic: proto/CHANGELOG.md and website/.../reference/changelog.mdx -> union. Both sides appended a new Unreleased entry and neither subsumes the other; each file is a pure addition against both parents. Resolved as one artifact rather than per-file so the pair cannot drift, though note the website copy already abridges older entries.
The endpoint host binding was checked in two places and only one of them applied the whole rule. The resolver refused an endpoint on an unrelated host AND one carrying credentials; the client re-check, which exists precisely because the resolver is a seam a caller can replace, refused only the first. So an injected resolver could return an endpoint with userinfo, the client would send there, and net/http would stamp an Authorization header the SDK never chose — inside the agent's own signed covered set. There is now one predicate behind both call sites, each wrapping it in its own tier's vocabulary. Stated twice it drifts, which this codebase has already watched happen to a host predicate. Coalesced endpoint resolution honoured nobody's context. Dropping the leader's cancellation so a caller walking away could not fail the whole burst also dropped the leader's DEADLINE — that helper returns a context with no deadline at all — and the coalescing group is not context-aware, so every waiter blocked until the shared fetch finished. With an injected client carrying no timeout, which the options struct accepts and the constructor doc invites, that was indefinite. Each waiter now selects on its own context while the shared fetch runs under a bound of its own, so a caller's deadline ends that caller's call and nobody else's. The proof signer refuses control bytes in the target URI in Python and TypeScript now, not only in Go. The signature base is line-delimited and the target is written into it verbatim, so a control byte adds or splits a component line and the signed bytes stop describing the request a verifier reconstructs. Both mirrors always sign GET, so the method arm does not apply to them. The parity records were wrong about all of this. Three claimed Python and TypeScript have no endpoint resolver, or keep private equivalents of the Go host predicates. Both languages ship an endpoint resolver, neither enforces the rule, and their private near-namesakes are the WBA variant, which compares WITH the port where the shared one deliberately does not — so they are not counterparts and wiring them in would refuse an endpoint Go accepts. The records now say that, and the generated matrix carries it. A durable record contradicting the code is the failure this branch has now hit twice. The normative wording anchored to the wrong thing in one place. The field table bound the endpoint to the manifest's own `domain` member — a value inside the document being validated, so a hostile manifest could set it to match its endpoint and pass. The proto, the page's prose and the code all anchor to the host that SERVED the document; the table does now too, and the page says why the distinction exists. The changelog called a conformance-affecting change documentation-only. The wire is genuinely untouched, but an Exchange serving its API from a separate domain is conformant today and will be refused, and the entry now says so — under a classifier that does not borrow "breaking", which in this changelog has only ever marked a wire delta. It also names the sentinel a re-pinning consumer must add to its retry classifier, since that entry is what a re-pin reads. Also: the design-history record still said the rule lived only in code with no proto comment and no published page, which stopped being true when it became normative; and the two per-caller cache tests asserted only that they stayed at or under the cap, so neither pinned that its own call site passed its own constant.
…llation a refusal The endpoint resolver concatenates its caller's host into a well-known URL but never checked the value was a bare host, so a caller outside the connect tier could choose the path that got fetched and have the result cached under the raw string. The check moves in beside the one that vets the advertised endpoint, for the reason that one already gives: it is a property of building this URL, not of any one caller's plans for it. Its coalescing had a leak. The shared fetch's timeout context was built before singleflight ran, so every coalesced follower allocated a timer whose cancel func only the leader's closure ever called — one live timer per waiter, held to full expiry, and invisible to vet's lostcancel because the traced path does cancel it. Deriving it inside the closure fixes that and keeps both properties the outer construction was there for. A locally cancelled call was classified as a peer refusal. connect-go stamps CodeCanceled on a context the caller cancelled, and the default arm read that as the final "the peer said no" verdict — telling a caller that gave up that the Exchange had declined a request it may never have finished reading. Also: - The EndpointResolver seam states the contract it is judged by. Only ErrNoEndpoint and ErrEndpointRefused are read as final, so an injected resolver that returns a bare error has its refusal retried indefinitely. - The Broker's requester-less refusal gets the test it never had. It was deletable with every test still green; the new one also asserts no round trip happened, which is what makes it a test of the LOCAL refusal. - Four parity-map reasons claimed Python and TS "carry the same default inline in their client". Neither ships a Connect client or a content fetcher. They now describe what exists. - The manifest endpoint's proto comment is one paragraph. A blank line routes the first into a JSON-Schema title that the Pydantic/Zod export drops, which had cost the field its identity line in both generated types while Go kept it. - The control-byte offset is reported in bytes by all three signers; Python and TS were counting code points under identical wording. - Smaller: errors.New for constant strings, no cookie jar on the offer-derived leg, the README names both resolver sentinels, the changelog's claim about every prior use of "breaking" is narrowed to what those entries actually say, the shared manifest-server helper covers its last call site, and the Python control-byte test is parametrized so a regression names the case that broke.
…the manifest The same-host predicate existed twice in Go and the copies disagreed about ports: the endpoint rule stripped the port, the WBA revocation check compared it. Rather than align them on the looser reading, the port now counts everywhere — what is being anchored is a place a signed call is sent, and another port is another service that the party publishing the anchor need not control. This tightens the newer check rather than loosening the live one, so no existing behaviour relaxes. The duplication ends: wbaHostAnchored keeps only what is local to WBA — a bool answer, and the requirement that a revocation_url be absolute, since the shared predicate reads a schemeless value as https and one branch there returns before the scheme is ever compared. A default port and an omitted port are the SAME port. url.Parse does not materialize an implicit port, so comparing raw authorities would refuse an operator who merely wrote :443 out in full — a spelling check wearing a security check's clothes. The folding is scheme-relative, which is also what stops the port rule from quietly becoming a scheme check: http://x and https://x still anchor, rather than diverging on 80 versus 443. The scheme remains the guarded transport's decision, made in one place from one flag. Hostname and port are compared as two values, not one joined string. Joined, the label-boundary match would have to find ".a.com" at the end of "sub.a.com:8443" and would refuse a subdomain for carrying a port — the right answer reached through the wrong comparison. Three SSRF-guard tests advertised https://localhost:1 against an anchor of localhost. The routing check now refuses that before the dial, so they had stopped proving the dial-time guard was installed at all; they name the port on both sides now, which keeps them testing the guard rather than the rule. Every surface that stated the old rule is updated together — the proto comment and its four generated artifacts, both changelogs, the manifest page, the design history and the parity map — because a durable record contradicting the code is the failure this branch has spent three rounds removing.
Anchoring folds a scheme's default port into an omitted one, and that folding needs a scheme on both sides — but both anchors in this SDK arrive without one. A WBA directory's authority and an Offer.exchange host are bare host[:port] values, read as https so a bare domain can be told apart from a path. That assumption broke plaintext deployments. An anchor of "a.example:80" kept its port, because 80 is not https's default, while the candidate "http://a.example:80" folded the same port away — one authority reaching two answers. Every plaintext WBA directory that spelled :80 in full stopped anchoring its own revocation_url, and a revocation poll that is skipped leaves a revoked key resolving, which is a great deal worse than the spelling it was refusing. A side that named no scheme now borrows the other's. That decides only WHICH port is the default, never whether two different ports are equal: an anchor of x:443 still refuses http://x:80, since 443 is not http's default — a pairing the previous code wrongly accepted by folding both sides to nothing. A panic from an injected seam no longer takes the process with it. When a coalesced call has waiting channels singleflight re-raises on a fresh goroutine, which no caller's recover can reach, so the closure recovers for itself; WellKnownOptions.HTTP and .Now are application code, making one failed lookup the right blast radius. A malformed host now classifies as final rather than retryable. The resolver checks the host itself and answers with ErrInvalidHost, which the routing tier's own documented contract did not list among the sentinels it reads as a verdict — so a value that will never become a host was scheduled for retry forever. And the records catch up with the rule. The manifest page still stated the port-exclusive binding in its field table, its MUST sentence and its embedded proto excerpt; the changelog's migration paragraph still told operators of single-domain Exchanges they were unaffected, when a single domain across two ports is precisely what now stops conforming. Both name the port shape, and the changelog gives a remedy that fits it.
signature-agent is one of the five RFC 9421 components a RAMP request always covers, so the header is signed whether or not a value was supplied — and the client had no way to supply one. Every call it made signed an EMPTY Signature-Agent. That is not a cosmetic gap. A peer resolves the caller's key by fetching the WBA directory at that origin, so an empty value leaves it nothing to resolve and the call is refused at verification: a 401 from an otherwise healthy Exchange, after the request was routed, signed and sent, with none of the routing checks able to see it coming. A deployment where each agent has its own directory cannot use the client at all. The option it needed already existed a tier down. What was missing was the wiring, and the reason it was missing is worth keeping: signingOptions returned early on the first knob it found, so a second option added later was silently dropped whenever the first was unset. It accumulates now. The value is stamped set-if-absent, so a relay forwarding an originating agent's request keeps the value that agent's own signature covers. The test drives a real RPC and reads what arrived, rather than asserting the option set a field: the header must carry the configured directory, Signature-Input must cover it, and the call must verify — a value that reached the wire uncovered would pass the first check and fail the third. Both client faces are pinned, since they reach the wire through the same plumbing and a second construction path is where a knob gets dropped. The changelog gains the entry this client surface never had. The verbs and the five options all landed on this branch without one, while the file's own convention is that a new exported SDK face gets recorded — and that entry is what a re-pinning consumer reads.
The delivery GET carried no X-Request-ID and had no way to. The two RPC legs correlate through a Connect interceptor, which a plain GET never traverses, and ContentFetchOptions had no header hook — nor could a caller patch around it, since the base-transport seam takes a concrete *http.Transport rather than a RoundTripper. So an edge that mints its own id when the header is absent recorded a refusal under a value nothing on this side could join it to, on the one leg where delivery failures are diagnosed. The hook is a plain func() string rather than the transport tier's named mint type, so this package grows no dependency on that tier for one alias; a named type is assignable to it at the call site. Nil sends no header: this tier holds no clock and no random source of its own, and inventing an id the caller cannot correlate against would be worse than omitting it. The client resolves the mint through one function now, used by both the interceptor and the fetcher. The legs do not share a mechanism, so two nil checks would be two places for the default to drift — and the leg that drifts silently is the one carrying nothing. Separately, the per-domain offer-key cache is bounded. Its key is a domain off Offer.exchange, so which entries appear is driven by incoming offers, and an entry's expiry is a freshness check rather than a removal — a stale entry held its slot indefinitely. Two structures over that same key space were already capped at 256 with the reason written out; this third one was not. It now shares the same bounded map, which carries its own lock, so the type holds no mutex and concurrent misses for one domain still both fetch exactly as before. Reaching the unbounded case needed a resolvable host serving a valid directory per domain, so it was narrow rather than open. The bound is the same either way.
The content fetch derives its deadline before building the request, because building it mints the proof and a ProofSigner is application code that may reach a custody backend bounded only by that backend's own client. Derived after instead, "bounds one content fetch" stops being true against a degraded custody service, and a batch pays that cost once per item. Nothing tested it. Moving the WithTimeout below the request build — re-attaching the deadlined context to the request, so the round trip stayed bounded — left the whole Go SDK suite green. The existing option test does not cover it and cannot: it blocks in the HANDLER, so the deadline it observes is the transport's. Only a signer that never answers tells the two apart, which is what the retired downstream test used and what this adds. It asserts the whole of what the failure class documents — the classification, the deadline as the cause rather than any signer error, and that no request left. The call runs off the test goroutine because the regression it guards against is not a slow fetch but one that never returns: inline, a reversal would hang until the suite panicked ten minutes later; here it is a named failure in five seconds.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Go SDK: the four missing low-tier client verbs
The Go SDK's Connect client exposed two verbs,
DiscoverandExecute. The agentintegration role needs six. This adds the missing four, fixes defects in the two
that already shipped, closes the gaps that let both go unnoticed, and — after the
downstream team tried to adopt it — makes reachable three knobs the tier below the
client already had.
One protocol change, and it is a comment: the well-known manifest's
endpointfield now states the rule its consumers were already enforcing. No field, message
or encoding change, and no new dependencies.
Six commits: the verbs, then three passes of review remediation, the adoption
gaps, and the endpoint rule.
New verbs
ReportUsage— files a usage report with the Exchange that issued theoffer. The destination is read off
UsageReport.exchangeand resolved from thatExchange's own
/.well-known/ramp.json; there is deliberately no option tosupply an endpoint, so a configured origin cannot become the default. Five
checks precede the send: plain hostname, manifest lookup, same-host endpoint,
SSRF guard on the report itself, no redirects. Behind an LRU-bounded per-origin
client pool.
Dispute— same routing and vetting. Takes the exchange domain as anargument because
DisputeRequestcarries no field to read it from.Fetch— retrieves content from a signed delivery URL, presenting proof ofpossession. Dials through the SSRF guard, refuses redirects, sends the URL
verbatim, caps the body.
Resolve— a separateNewBrokerClient, sharing the exchange client'ssigning transport, interceptors and offer Verifier. A Broker is not an Exchange,
so the two cannot share a base URL.
Fixes to the verbs that already shipped
Executecould not buy. It built a request carrying neitherRequesternorAgentAcceptance, which a conforming Exchange refuses. It now fills both,signing the acceptance through the injected
Signerso the SDK never holds a key.Discoversilently returned nothing. It read only the flatofferslist anddropped
offer_groups, so against a server that follows the contract — leave theflat list empty when groups are populated — it returned an empty result with no
error.
Exchange and the Broker each resolve the calling agent from the requester and
refuse a request that names none, while the client held that identity and applied
it only to a purchase. Discovery now fills the requester and the protocol version
on a clone, and only where the caller left them empty. A Broker resolve with no
requester configured is refused locally rather than sent to be declined.
Discovery results are grouped per URI
Both discovery verbs return one group per requested URI, each carrying the
fail-closed
{verified, rejected}split plus the typed absence reason. A flat listhas nowhere to put that reason, and a refused URI vanishes entirely — an empty
group has no offer to carry its identity back. The reasons are different agent
actions ("give up" / "acquire an entitlement and retry" / "never retry"), so
collapsing them is the trial-and-error the vocabulary exists to prevent.
Breaking change to
Discover's return type; nothing outside this repo consumes theclient.
A manifest's endpoint is bound to the host that serves it
The field said only "Exchange-only. ExchangeService endpoint URL", so no operator
was ever told the address had to stay on their own domain — the rule lived in one
client's code and nowhere else.
It is now stated where an implementer will find it, in the proto and on the
manifest page: the endpoint MUST be on the host that serves the manifest or a
subdomain of it, and MUST NOT carry userinfo. The manifest is only as
trustworthy as the host that served it, so an endpoint naming an unrelated host
would let whoever answers for it redirect a signed call to a party the offer's
signature never covered — and a dial-time address guard has no objection, because
the destination is an ordinary public host. The match is on a full dot-delimited
label boundary, so
evil-a.comis not a subdomain ofa.com. The port iscompared too — another port is another service the manifest's publisher need not
control — with a scheme's default port and an omitted port treated as the same,
so writing
:443out in full is not a refusal.Enforcement moved down a tier, from the client into the shared endpoint
resolver. The check is a property of reading an endpoint out of a manifest, not of
any one caller's plans for it, and every consumer needs it while none can be relied
on to remember. Consumers that resolve through this package inherit it without
changing a line. The client keeps its own check, because the resolver is an
injectable seam and a signed call cannot be made conditional on a stranger's
implementation having remembered the rule.
Refusal carries its own sentinel rather than reusing the no-endpoint one. That
distinction is load-bearing: a caller classifying retryability reads a transport
failure as worth another try, and this is a verdict — the Exchange answered and the
answer is unusable.
Breaking for implementers, not for the wire — the changelog entry carries a
classifier that says so rather than borrowing
(breaking), which in thischangelog has only ever meant a
buf breakingdelta. An Exchange already servingfrom its own domain is unaffected; one serving its API from a separate domain must
front it under a subdomain. Three resolver tests advertised exactly that cross-host
shape and now advertise themselves.
The anchor is the host that served the manifest, not the self-asserted
domainmember inside it — anchoring to that member would let a hostile document name
whatever endpoint it liked and validate itself.
One SDK enforces it. Python and TypeScript ship endpoint resolvers that do not,
and the predicate they would need does not exist there — their private
near-namesakes compare with the port, so they are not drop-in counterparts. The
parity records now say that plainly instead of claiming, as they did, that neither
language had an endpoint resolver at all. Closing the gap is tracked separately,
and the TypeScript/Python client work is marked as blocked by it.
The offer-derived leg is bounded, and its guard is not optional
ReportUsage,DisputeandFetchdial an address another party named, so thatleg is bounded in every dimension:
exchange, so a hostile response decompressed without limit into the caller's
memory. There is now a default cap, plus a
WithClientOptionsseam mirroring theserver face for callers who need to tune it.
answered held a call, a goroutine and a socket indefinitely.
the same open-ended, caller-influenced host space the pool is already bounded for.
It now evicts least-recently-used at a fixed cap, and the per-host coalescing it
sat beside — a map of mutexes that grew even for hosts whose fetch failed — is a
single-flight group, which holds a host only while its fetch is in flight.
The SSRF guard is not removable through the transport seam. That seam was
documented as the content fetch's but also replaced the base of the offer-derived
RPC pool, so a caller injecting a tracing or mTLS transport silently disarmed both
the address and scheme guards on signed calls. A supplied transport now composes
underneath the guard; the only opt-out through it is the deployment-level
environment flags. The tests took that seam — which is why nothing proved the guard
was installed at all — and now opt out the way a deployment does, with a private
endpoint refused at dial time asserted directly.
Composing underneath left one way through, closed here.
net/httpprefers atransport's own TLS dialer over the guarded one whenever the scheme is https —
which is every leg — so a base carrying
DialTLSContexttook the dial around theaddress pin with no error to say so. Reproduced on both affected legs: a signed
report reached a loopback address, and a bound fetch retrieved the bytes. Both
dialer fields are now cleared where the proxy already was, for the same stated
reason — each routes the dial around the check rather than under it.
TLSClientConfigis kept, so client certificates, which is what the seamadvertises, still work. The regression tests fail against the previous commit; the
one that covered this seam passed an empty transport, which cannot carry a dialer
and so proved less than it appeared to.
One agent identity
The client offered a separately-custodied acceptance key the protocol has no slot
for. One identity runs through the whole flow: the thumbprint of the
request-signing key is what an Exchange verifies the detached acceptance against
and what a delivery URL is bound to, so a second key is refused at execute and
could never fetch what it bought. That option is gone. The public half of the one
key remains, because a bound fetch presents it in a header and a
Signercannotyield it — and it is now copied rather than aliased, so a caller reusing its slice
cannot change which key later fetches present.
Three knobs the tier below already had
Found while repointing an existing consumer at this client. Each was configured
there from the environment, and adopting the SDK would have kept reading the value
and silently stopped acting on it.
WithSignWindowsets the freshness window on every outbound requestsignature, which was pinned to the package default of five minutes. Two reasons
an application supplies its own: a shorter freshness policy, and a peer that
screens replays on (key id, signature) — timestamps have one-second resolution,
so two identical requests inside one second sign to the same bytes, and a
monotonic window is what keeps them distinct. Threaded through the one place the
package builds a signing transport, so all three faces sign on identical terms.
WithContentTimeout/WithMaxContentBytesmove the delivery fetch'sdeadline and body cap. The cap matters beyond tuning: an application carrying its
own per-item budget gets wrong accounting when the two disagree.
The signing option is deliberately narrow rather than a pass-through for the
transport's whole option set — that set also carries a predicate that skips signing
outright and a mode that co-signs as a relay, neither of which belongs on an agent
client by default. The two fetch bounds are separate scalars rather than one
options struct, because that struct also carries the base transport, which arrives
through its own option; a second way to set it would be a field that had to be
silently ignored.
Errors
Failures are classified by cause rather than by position. Every failure to reach a
manifest was reported as a refusal to send, so a momentary outage read as a
permanent verdict — and a caller following that guidance would drop a usage report
for good. After the checks passed, the raw transport error came back, so one method
answered with two unrelated error types depending on where it failed. A missing
signer was a malformed request on the purchase path and an unsignable one on the
fetch path — the same misconfiguration under two classes a caller is told to branch
on; it is unsignable on both. A synthesized error detail names the failing surface
rather than the fetched URL, which is what makes that field a grouping key rather
than an unbounded one.
One implementation, not two
The two caching tiers had hand-rolled the same bounded map, down to the same
rationale paragraph copied word for word, and the two failure types rendered
themselves with byte-identical code. Duplication here has already been observed to
drift within a single commit, so each is now one implementation — a generic
least-recently-used cache and a shared renderer, both internal, because a bounded
map and error prose are not protocol concepts and have no cross-language face to
mirror. Four copies of the clone-and-type-assert preamble collapse into one helper,
and the fill-when-empty envelope rule the two discovery verbs each stated
separately now has one home.
A full generic send helper for the two offer-routed verbs was considered and not
taken: stamping the envelope needs settable field pointers, which generics cannot
express without a per-type adapter that would reintroduce what it removed.
Two defects behind those seams. The coalesced manifest fetch inherited the winning
caller's context, so one caller walking away failed every other caller waiting on
the same host, each reading it as the Exchange being unreachable — the shared fetch
now outlives whoever triggered it, bounded by the resolver's own timeout. And the
agent-binding proof wrote its method and target URI verbatim into a line-delimited
signature base with no check, so a control byte could add or split a component line
and the signed bytes would stop describing the request a verifier reconstructs.
Other corrections
Exchange advertising a non-default port permanently unable to receive a usage
report, refused by a check that was never protecting anything there.
intended; overwriting a value the caller put in a required field turns each of
their retries into a fresh action, which is the double-counting the field exists
to prevent.
failure would print the live credential in its query.
identity, and the read-cap constant takes the
Defaultprefix its three siblingscarry.
Supporting work
host predicates; URL redaction; the delivery edge's refusal vocabulary mapped
onto the typed enum.
rather than a test-internal copy, byte-identically. The one negative vector stays
hand-built, because the shipped signer refuses that input by design.
tier holds no key material.
Gates and docs
unchanged. Matrix regenerated by script.
and asks whether each symbol is mapped; nothing asked the reverse, so an entry
left behind when its symbol was deleted stayed in the published matrix claiming a
surface that no longer existed. This change is the repo's first export removal
and it left exactly that — one phantom row. Removed, and the mirror walk added,
so the next removal is caught rather than published.
DECISION resolved". It is now recorded as open: the API-surface design governs
and specifies a thin unary client for TypeScript and Python with the same verb
names, so what diverges is the transport, not the API.
covered only two Go packages, so a change adding exports elsewhere — and editing
the symbol map to match — triggered no job at all. Widened.
results are grouped, why signed legs refuse redirects where the guarded client
follows them, why a report's destination comes off the message, the single-identity
rule, what the manifest fetch does not guarantee, and which transport settings the
guard drops rather than carries. The record of what the guard covers is scoped to
the seam it governs — a caller that injects a whole HTTP client or its own resolver
has taken ownership of that fetch, which is a different bargain.
shared components. Kept as MDX rather than Astro after verifying that the build's
link validator harvests links at Markdown compile time — an Astro component would
have silently stopped covering the one link the short banner carries.
calling an API it does not have; it names a clearly hypothetical path again,
matching the banner that already declares the page deferred design.
Verification
go build/vet/test ./... -raceand-shuffle=on, the full local gate, the CI-onlyAPI-surface gate, the Python SDK suite (540), the TypeScript SDK suite (522), and
the website build (79 pages, links valid) all pass. Regenerating the shared vectors
produces no diff, and the four generated artifacts a field-comment change touches
are regenerated and committed.
mainis merged in. Both sides had edited the same proto file, the same manifestpage and both changelogs; the generated artifacts were rebuilt from the merged
proto rather than line-merged, and the union was verified by diffing the result
against each parent.
main's own suites account for part of the counts above.Each guard fix carries a test that fails against the previous commit. A guard test
that passes before and after proves nothing, which is how the TLS-dialer bypass
reached review in the first place.
Notes for review
guidance covers only the report leg, so that inference is worth a second look.
Executeposts to the configured base URL rather than the offer's Exchange. Thatis correct for the direct topology, and buying through a Broker is a relay the
client is deliberately not asked to route — the missing piece is a broker-side
execute path, tracked separately.
Exchange advertises an endpoint on a different domain will start being refused,
which is the intent, but it is worth a look before this ships.
agent-key header could be mapped rather than recorded as a divergence it is not.