fix(discover): skip malformed capability events instead of aborting discovery - #8
Merged
Merged
Conversation
…iscovery A capability event with a malformed `price` tag made discover() throw and abort the whole batch, dropping every valid capability with it. One hostile relay publishing one bad event could DoS discovery for every agent. Parse each authenticated event independently: skip and warn (loudly, never silently) on a malformed event, and return the rest. Includes a mutation-verified regression test that places the poison event mid-batch so a fix surviving only a trailing bad event would still fail. Fail-open audit ledger item 41. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01CYXrdWWjKo5AyGqFAfrdgK
…ger 41)
The price-tag fix closed the parse step, but two more single-hostile-event
DoS vectors sat upstream of it in discover(), still able to abort the whole
batch:
A. A relay event dict missing a committed field (pubkey/created_at/kind/
tags/content) made NostrEvent.verify() -> compute_id() raise KeyError out
of _is_event_authentic()/_filter_authentic(). _is_event_authentic now
drops the offending event (warn, return False) while STILL propagating a
RuntimeError crypto-backend fault (CryptoBackendUnavailableError is a
RuntimeError subclass) so a backend outage fails closed and loudly rather
than masquerading as "nothing found".
B. A non-dict relay payload (str/list) made _query_relays do `.get` on a
str, raising AttributeError. _query_relays now skips non-dict payloads
with a warning before the `.get`.
Both are mutation-verified with poison events placed mid-batch. The existing
crypto-backend-unavailable / verification-error-propagation tests continue to
pass: Runtime(backend) faults propagate; hostile-event parse faults (KeyError/
TypeError/ValueError) are dropped.
Fail-open audit ledger item 41.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01CYXrdWWjKo5AyGqFAfrdgK
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
1 participant
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.
What
Fail-open audit ledger item 41: a single capability event with a malformed
pricetag madediscover()throw and abort the entire batch. One hostile relay publishing one bad event could DoS discovery for every agent, dropping every valid capability with it. Remotely triggerable, live on published SDKs.Fix
discover()now parses each authenticated event independently. A malformed event is skipped and warned — loudly, never silently swallowed — and all valid capabilities are still returned. The parse is wrapped narrowly (only the untrusted-event parse; authenticity/crypto already ran before it, no I/O inside), catching any parse failure while never swallowing the timeout/cancellation signal.Test
Mutation-verified regression test that places the poison event mid-batch, so a fix surviving only a trailing bad event would still fail. RED before the fix, GREEN after; reverting only the production change turns it RED again. It also asserts the skip is loud (a warning naming the offending event id).
Cross-port consistency
One of three identical fixes shipping together — Python / .NET / TypeScript. All three converge on the same contract: parse each event independently, broad catch on the untrusted-event parse → skip, warn naming the event id, poison-mid-batch test asserting loudness. (Verified against the port-drift oracle: the .NET port was widened from an enumerated
FormatException/JsonExceptioncatch to match the others, since enumerating throw types on a hostile-input boundary leaves every unlisted throw vector live.)🤖 Generated with Claude Code
https://claude.ai/code/session_01CYXrdWWjKo5AyGqFAfrdgK