feat: sign and deliver MCP events safely

[ChiragAgg5k]

Stack

feat/events ← #131 ← #132 ← #133 ← #134 ← #135 (events/subscribe / events/unsubscribe)

Merges into feat/events; feat/events → main lands as one feature after appwrite/appwrite#14293. Each PR's diff shows only its own layer, and every layer passes the full checklist on its own.

Part of #127 (PR 2 of 4). A self-contained library; nothing is wired into server.py or http_app.py yet.

Summary

• events/egress.py: the only outbound HTTP path for webhook callbacks (verification and delivery). https only, resolves DNS itself, refuses non-global addresses, connects to the checked IP while keeping SNI and Host, never follows redirects, one 10 s deadline per request that also covers the wait for a per-host slot, 16 KiB response cap, 4 concurrent requests per destination host. Headers, including the Standard Webhooks timestamp and signature, are built only once the slot is held.
• events/delivery.py: Standard Webhooks signing (webhook-id, webhook-timestamp, webhook-signature: v1,..., one signature: a subscription has one secret), whsec_ secret validation, the verification handshake, and single-event delivery with bounded retries.
• Declares httpcore explicitly since egress.py imports it (already locked as an httpx dependency; no new packages).

Design notes

SSRF. egress.Backend is an httpcore.AsyncNetworkBackend plugged into the connection pool of an httpx.AsyncHTTPTransport subclass. httpcore calls connect_tcp(hostname, port); the backend resolves the hostname, rejects the request if any answer is not permitted, and opens the socket to the checked IP string. httpcore then starts TLS with server_hostname taken from the request origin, so SNI and certificate validation use the hostname, and httpx builds Host from the URL. The hostname is never resolved a second time, so rebinding between check and connect is impossible, and the check runs on every connection, not only at subscribe time. permitted() combines ipaddress.is_global and the negative predicates with an explicit blocklist (0/8, 10/8, 100.64/10, 127/8, 169.254/16, 172.16/12, 192.0.0/24, documentation and benchmarking ranges, multicast, 240/4, ::/96, 100::/64, 2001::/23 incl. Teredo, 2001:db8::/32, 2002::/16 6to4, 3fff::/20, fc00::/7, fe80::/10, fec0::/10, ff00::/8, local-use NAT64). IPv4-mapped and well-known NAT64 (64:ff9b::/96) addresses are judged by the embedded IPv4. Proxies and trust_env are off, because a proxy would resolve the name itself. Responses are read raw with Accept-Encoding: identity, so a compressed reply can't inflate past the cap. The only loosening is the explicit allow_loopback=True constructor flag, which only tests use and nothing reads from the environment. It admits loopback addresses, and plain http only for a loopback host (localhost, *.localhost or a loopback IP) whose every resolved address is loopback. Public destinations stay https-only.

Deadline and signing time. Egress.post(url, body, headers) takes a headers factory. One fail_after wraps both the per-host slot wait and the request, and the factory runs once the slot is held. A request queued behind slow ones to the same host therefore ends within the timeout, and it is never sent with a signature timestamped back when it was queued.

Verification. A signed {"type":"verification","challenge":<32 random bytes, urlsafe>} with webhook-id: msg_verification_<random>. The endpoint must return a 2xx JSON object whose challenge matches, compared with hmac.compare_digest. Failures raise CallbackError(reason): 4xx is http_4xx, 5xx is http_5xx, 3xx or a missing or wrong echo is challenge_failed, a timeout is timeout, an ssl.SSLError anywhere in the cause chain is tls_error, and a forbidden destination or other connection failure is connection_refused. Raw endpoint responses are never surfaced.

Retry policy. One event per POST, body {eventId, name, timestamp, data, cursor: null} serialized once as compact JSON, capped at 256 KiB before sending (PayloadTooLargeError). RetryPolicy defaults to 4 attempts with waits of 30 s, 2 min and 8 min, each jittered ±20%. The worst case is about 12.6 min of waiting plus 4 × 10 s timeouts, which is under 15 min. Every attempt is re-signed with a fresh timestamp, and webhook-id stays the event id. Any non-2xx except 410/413 is retried, as are timeouts and connection errors (the sketch says retry on non-2xx). 410/413 and a forbidden destination are final (rejected). Exhausted retries are abandoned with the last error category. Retries run in-process via an injectable sleep, and nothing is persisted.

Tests

Delivery and egress are covered end to end by the ingress flows in #134 (tests/e2e/test_events_ingress.py). There, the server's real Dispatcher and Egress deliver to a local HTTPS receiver that checks every request with the official standardwebhooks library:

• Signing: every delivery verifies with standardwebhooks. Body is exactly {eventId, name, timestamp, data, cursor: null}, webhook-id equals eventId, X-MCP-Subscription-Id and Content-Type are set. - Retries with a short injected policy (0.3 s, 0.3 s, 0.6 s; 1 s timeout): 500, then a timeout, then 302 (the Location is never requested), then 200. That gives four attempts with the same webhook-id, re-signed with non-decreasing and finally newer timestamps. 410 and 413 get exactly one attempt each (rejected / http_4xx), and 503 is abandoned after 4 attempts (http_5xx).
• TLS: a receiver whose certificate is not trusted gets no request, the handshake fails, and the delivery is abandoned as tls_error.
• Address pinning: the egress dials 127.0.0.1, while the receiver sees SNI localhost and Host: localhost:<port>. The certificate has only a DNS SAN, so it validated against the hostname.
• The default (production) egress refuses a loopback callback (rejected / connection_refused), and the receiver gets nothing.
• A burst of 8 events to one host peaks at 4 in-flight requests. With 0.6 s replies and a 1 s timeout, the 4 deliveries queued behind the first four time out instead of going out late, and succeed on retry: 12 attempts in total. The old deadline order sends only 8.
• An event over 256 KiB is not sent (too_large).

Unit tests kept, and why they are not e2e:

• tests/unit/test_events_egress.py:
  • The blocklist table (44 addresses incl. IPv6, IPv4-mapped, NAT64, 6to4 and Teredo), the allow table and the loopback flag's scope. A test machine cannot route to most of these addresses.
  • Mixed public/private DNS answers, DNS rebinding between check and connect, and the connect-target spy. These need a resolver that answers differently on demand.
  • URL shapes (scheme, credentials, missing host) that subscribe (PR 5) will reject before any delivery.
  • The 16 KiB response body cap. Deliveries discard response bodies, so the cap is invisible on the wire.
  • LoopbackHttpTest: plain http needs a loopback host even with allow_loopback, and localhost resolving to a public address (or a mix) never connects. No test can safely prove the refusal by sending to a public http URL.
  • DeadlineTest: the reviewer's repro. Four 80 ms requests, one slot and a 100 ms timeout give exactly one success in under 200 ms (the old order: ~330 ms, all four succeed). Headers are built within 30 ms of sending, not at queue time. The wire's whole-second webhook-timestamp is too coarse to show this.
• tests/unit/test_events_delivery.py:
  • whsec_ validation and the verification handshake: success, single-use challenges, mismatch, malformed bodies, 4xx/5xx, a redirect not followed, timeout, connect error and a forbidden destination. Only events/subscribe (PR 5) calls them, and nothing in this stack does yet. Replace them with its e2e flow.
  • The production retry schedule: 30 s / 2 min / 8 min with ±20% jitter, 4 attempts, worst case under 15 minutes. Its real-time waits are too long to run end to end.

Removed: the Standard Webhooks vector test (superseded by verifying every e2e delivery with the official library), plus delivery body/headers, retry and re-signing, 410/413, transport errors, dual signing (since removed, see 656ac20), the payload cap, timeouts, per-host concurrency and the local-TLS tests (SNI/Host, redirect, loopback without the flag, untrusted certificate). The e2e flows cover all of them.

Review changes

2d9cf2e:

• Hansi, allow_loopback http scope: plain http is now limited to loopback hosts that resolve only to loopback.

• Hansi and code review, semaphore wait outside the deadline: the deadline now starts before the per-host slot. Headers and the signature are built after the slot is acquired.

• 321f72b: rebased onto feat: seal MCP event subscriptions into an envelope #132's switch to the mcp-event-subscriptions JWE envelope; pyproject.toml / uv.lock keep httpcore next to the new dependency.

• 656ac20 (follows feat: seal MCP event subscriptions into an envelope #132, and Hansi on Callback's repr): a subscription record holds one delivery.secret, so Callback holds one secret (kept out of repr, like its derived key) and sign() produces one signature. The multi-secret path had nothing left to feed it, and its only coverage was a two-secret ingress flow that cannot exist with one secret per record. New unit test CallbackTest.test_repr_hides_the_secret (no wire output shows a repr).

Verification

Run locally on Python 3.12.8, lockfile written with uv 0.11.22 (the CI version):

• uv run --group dev ruff check src tests: pass
• uv run --group dev black --check src tests: pass
• uv run --group dev pyright: 0 errors
• uv run python -m unittest discover -s tests/unit: 309 tests OK
• uv run --group e2e python -m unittest discover -s tests/e2e: 2 tests OK (about 4 s)
• docker build -t appwrite-mcp:jwe .: builds

[hansi-codes]

🟢 Tier S · Ready to merge

Adds a webhook delivery layer with Standard Webhooks signing, verification handshakes, bounded retries, and an SSRF-conscious HTTP egress transport. It also declares httpcore as a direct dependency and adds unit coverage for delivery and egress behavior.

Verdict	New comments	Fixed	Still open
✅ Approved	0	1	0

📂 Walkthrough · 5

File	Change
pyproject.toml	Adds httpcore as an explicit runtime dependency.
src/mcp_server_appwrite/events/delivery.py	Implements webhook signing, verification, and retryable event delivery.
src/mcp_server_appwrite/events/egress.py	Adds the guarded outbound HTTP transport, DNS checks, deadlines, and per-host limiting.
tests/unit/test_events_delivery.py	Tests secret validation, verification, and retry scheduling.
tests/unit/test_events_egress.py	Tests address policy, URL validation, request deadlines, and response limits.

✅ Fixed since the last review · 1

• Hide delivery secrets from Callback repr · src/mcp_server_appwrite/events/delivery.py:153

🔇 Filtered out · 1

Findings Hansi considered but did not post.

Finding

Why

Emit multiple signatures during secret rotation

Verifier: The advertised rotation concerns server-side MCP_EVENTS_SEALING_KEYS, not the client's whsec_ delivery secret (docs/development.md:40-45 and events/envelope.py:29-32). Keyring.record opens envelopes using all retained keys (envelope.py:437-443), while the subscription record preserves its independent delivery.secret (300-303); sealing-key rotation therefore does not change the receiver's signing secret. delivery.py:8 explicitly specifies one secret/signature per subscription. No callback-secret rotation path or promise requiring simultaneous old/new delivery signatures exists in the code.

---

_{Reviewed 656ac20 · Details · Comment @hansi-codes review to re-run, or mention @hansi-codes with a question.}

[hansi-codes]

🔵 Tier A · See the inline comments. Summary

[hansi-codes]

🟢 Tier S · Looks good to merge. Summary

[hansi-codes]

🔵 Tier A · See the inline comments. Summary

[hansi-codes]

🟢 Tier S · Looks good to merge. Summary