fix: read DIDComm mailbox via the local gateway, not the public endpoint - #646
Merged
Conversation
receiveDidComm/mediateDidComm resolved the identity's *published* DIDCommMessaging endpoint and dialed it directly to read the owner's own mailbox. When that endpoint is the auto-discovered Tor onion (no ARCHON_DRAWBRIDGE_PUBLIC_HOST), a client with no Tor fails with a bare "fetch failed" — and even with Tor it pointlessly routes out through the onion to reach the owner's own relay. Reading your own mailbox is a local-gateway operation, the mirror of Phase 8 sends. Factor the node-URL derivation out of sendDidComm into didcommGatewayBase / _didcomm_gateway_base and use it in all three: mailbox reads now go to <nodeURL>/didcomm (Drawbridge's /didcomm proxy → relay), never the published public endpoint. An explicit --endpoint still overrides. The published endpoint stays what it is for — *others* sending to you. Also guard the challenge fetch in receive/mediate so a non-2xx gateway response is a clean KeymasterError, not a raw .json() throw. JS + Python in parity; design doc updated. Validated: JS e2e 7/7, JS didcomm units 62, Python didcomm 17; live CLI receive-didcomm through the gateway confirmed. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Pull request overview
This PR fixes receive-didcomm / mediate-didcomm to read an identity’s own DIDComm mailbox through the local node gateway (<nodeURL>/didcomm) instead of dialing the identity’s published public DIDCommMessaging endpoint (which may be an unreachable .onion and is conceptually for others sending to you).
Changes:
- Factor gateway base URL derivation into a shared helper in both TypeScript and Python, and reuse it for send/receive/mediate flows.
- Switch receive/mediate mailbox reads to use the local gateway base (with
--endpointstill overriding). - Add clearer gateway challenge error handling and update DIDComm design documentation accordingly.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| python/keymaster/src/keymaster/core.py | Adds _didcomm_gateway_base() and routes DIDComm send/receive/mediate gateway access through it; improves challenge error handling. |
| packages/keymaster/src/keymaster.ts | Adds didcommGatewayBase() and switches DIDComm gateway callers to use it; improves challenge error handling in receive/mediate. |
| docs/didcomm-design.md | Documents the corrected “read your own mailbox via local gateway” behavior and the shared derivation helper. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
…iew) Address Copilot review on #646: - didcommGatewayBase / _didcomm_gateway_base: strip trailing slashes from the node URL before appending `/didcomm`, so a configured `ARCHON_GATEKEEPER_URL` ending in `/` no longer yields `host//didcomm`. - Python challenge guards now fail on any non-2xx (`not response.is_success`) instead of only `>= 400`, matching the TypeScript `response.ok` behavior. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
macterra
added a commit
that referenced
this pull request
Aug 20, 2026
… node Sending to your own identity left the node and came back. With an auto-discovered endpoint that is the Tor onion, so a message to yourself meant a full Tor round trip out and in again -- and on a node running no Tor it failed outright with a 502. Recognise a recipient whose mailbox lives here, by comparing their published endpoint against the one this node advertises (the value publishDidComm already auto-discovers), and deposit straight into the local relay. No challenge, no egress, no Tor. Excluded when a mediator is in the path: that Forward envelope is addressed to the mediator, so depositing it here would leave an envelope this node cannot unpack in a local mailbox. This is the send-side counterpart to PR #646, which made reading your own mailbox a local-gateway operation for the same reason. Also carry the service's error body into a failed delivery. A bare 502 could not distinguish a missing Tor proxy from a recipient that rejected the envelope; the relay says which in the body, so stop discarding it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
macterra
added a commit
that referenced
this pull request
Aug 20, 2026
* feat(keymaster): Expose the node capability manifest to clients Wallet UIs need the same signal the capability gates use, so they can hide an optional-service surface instead of offering it and failing. Make `getNodeCapabilities` public on Keymaster and carry it through the client contract, the REST API, and both Python ports. Null (a node that serves no manifest) keeps meaning "unknown", which callers treat as allowed — an older node or a bare gatekeeper must not have every optional surface hidden. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * feat(wallet): Add a DIDComm endpoint tab to the wallets Phase 1: publish and unpublish a DIDComm messaging endpoint, and show what the current identity's DID document actually advertises. Blank endpoint asks the node for its own relay, which is the path most identities want; an explicit endpoint overrides it, and routing keys switch publishDidComm to the object service form for delivery via a mediator. Both wallet code paths are covered: react-wallet gets a DidCommTab gated on hasDidComm, and the shared KeymasterUI gets the same screen so the demo wallets exercise it through KeymasterClient over REST. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * feat(wallet): Add a DIDComm inbox to the wallets Phase 2: read the mailbox, dismiss messages, and answer a trust ping. Reads with ack: false, so the node keeps every message until it is explicitly dismissed. That also means each poll returns the whole mailbox, so the list is replaced rather than accumulated -- what is on screen is exactly what is still on the node. Messages that fail to unpack stay there and never appear. Polling runs only while the screen is open, at the interval already configured in Settings, and a poll that fails stays quiet; only an explicit refresh raises a snackbar. Overlap is guarded with a ref, since an interval callback closes over the render that created it. Basic messages render their text, trust pings offer a one-click response, and anything else falls back to its JSON body. Sender and authentication are shown separately: anoncrypt hides the sender, so "who sent this" and "is that verified" are different questions. Also exports @didcid/keymaster/didcomm-protocols so a consumer can build and recognise protocol messages without reaching into the package internals. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * feat(wallet): Add DIDComm compose and send to the wallets Phase 3: send a basic message or a trust ping to any agent that publishes a messaging endpoint, from either wallet code path. The recipient is resolved before packing. An alias must not travel in the envelope, which addresses recipients by DID, and resolving first turns an unknown name into a clear error before any crypto happens. Anoncrypt is offered as "send anonymously", with the trade stated rather than implied: the recipient cannot tell who sent it and cannot reply. Authcrypt stays the default. A basic message in the inbox now offers Reply, which prefills the recipient and switches to Compose. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(keymaster): Deliver locally when the recipient mailbox is on this node Sending to your own identity left the node and came back. With an auto-discovered endpoint that is the Tor onion, so a message to yourself meant a full Tor round trip out and in again -- and on a node running no Tor it failed outright with a 502. Recognise a recipient whose mailbox lives here, by comparing their published endpoint against the one this node advertises (the value publishDidComm already auto-discovers), and deposit straight into the local relay. No challenge, no egress, no Tor. Excluded when a mediator is in the path: that Forward envelope is addressed to the mediator, so depositing it here would leave an envelope this node cannot unpack in a local mailbox. This is the send-side counterpart to PR #646, which made reading your own mailbox a local-gateway operation for the same reason. Also carry the service's error body into a failed delivery. A bare 502 could not distinguish a missing Tor proxy from a recipient that rejected the envelope; the relay says which in the body, so stop discarding it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(wallet): Trust the DIDComm envelope's sender, and the current identity's mail Two review findings on the DIDComm inbox. **Sender binding.** Authcrypt authenticates the sender through the envelope's `skid`. The plaintext `from` header is a claim carried inside that envelope, and nothing checked the two agree -- so an authenticated sender could name any DID as `from`, and every consumer trusting that header addressed the wrong party. In the wallet that meant a spoofed DID displayed beside an "Authenticated" badge, with Reply and the trust-ping response both aimed at the claimed party. Reject the mismatch in unpackDidComm, which covers every surface at once -- CLI, MCP, SDK and any external consumer -- rather than only the wallets. An anoncrypt `from` has no authenticated sender to contradict, so it stays an unverified claim rather than an error. The wallets apply the same rule at the second line: the sender comes from metadata.sender, an unverified `from` renders as such instead of masquerading, and Reply/Respond are offered only when there is an authenticated party to address. packDidComm already stripped a caller-supplied `from`, so the vector was never our own send path; it is a foreign agent, and anyone may send us DIDComm. **Identity race.** The inbox kept the previous identity's mail on screen while the new read ran, and the overlap guard was a bare boolean: a read already in flight would skip the new identity's read and then commit its own result under the new name -- permanently, with polling off. The guard is now keyed by identity, a completion whose identity is no longer active does not commit, and the list is cleared on identity change. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
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.
Problem (field report)
keymaster receive-didcommfailed with a bare "fetch failed".Root cause:
receiveDidComm/mediateDidCommresolved the identity's publishedDIDCommMessagingendpoint and dialed it directly to read the owner's own mailbox. With noARCHON_DRAWBRIDGE_PUBLIC_HOSTset, auto-discovery publishes that endpoint as the Tor onion (…onion:4222/didcomm) — which a client with no Tor can't reach (undici's generic "fetch failed"). Even with Tor it's wrong: it routes out through the onion to reach your own relay.Fix
Reading your own mailbox is a local-gateway operation — the mirror of Phase 8 sends. The node-URL derivation is factored out of
sendDidComminto a shared helper (didcommGatewayBase/_didcomm_gateway_base) and used by all three:<nodeURL>/didcomm(Drawbridge's/didcommproxy → relay), derived from the single node URL the keymaster already uses — never the published public endpoint.--endpointstill overrides (e.g.--endpoint http://localhost:4236).fetchin receive/mediate so a non-2xx gateway response is a cleanKeymasterErrorinstead of a raw.json()throw.Parity & validation
docs/didcomm-design.mdupdated.keymaster receive-didcomm(no flags) reads the mailbox through the gateway and unpacks an authcrypt Basic Message.🤖 Generated with Claude Code