Repository navigation
feat: MCP Events behind the events flag - #136
Merged
Merged
Conversation
Six v1 events, each declared as data: arguments, Appwrite pattern templates, an IDs-only payload schema and the status filter the ingress will apply. Arguments are checked against a dot- and wildcard-free Appwrite ID pattern before they are spliced into an Appwrite event pattern, so a subscription can never widen past the named resource. Error codes sit behind constants because SEP-3415 renumbers them. The MCP_EVENTS flag gates the feature (off by default).
The SDK drops unknown capability keys (python-sdk#3640), so a Server.middleware adds capabilities.events for ChatGPT and the SEP-3415 extension entry to the discover and initialize results. Only on the HTTP transport with MCP_EVENTS on. Tests assert on raw HTTP through the hosted app because the SDK client models drop the key too.
Boot the real Starlette app under uvicorn and drive it over HTTP like ChatGPT, so the wire is what is asserted. Unit tests the e2e flow now covers are removed; only what HTTP cannot reach before subscribe stays.
It needs no credentials, so unlike integration it also runs for forks.
Each MCP Events subscription is stored only in its Appwrite webhook. The envelope seals the delivery state into authPassword with AES-256-GCM, bound to the subscription id and project, and the webhook signing key is derived so the ingress can verify deliveries without any storage.
Round trips, tampering, binding, rotation and expiry are covered through the ingress e2e flows in the layer above. What stays is the PHP-computed Appwrite signature vector, subscription id determinism and charset until subscribe has an e2e flow, and the size budget for appwrite/appwrite#14293.
valid_subscription_id accepted any 32-char Appwrite-safe suffix, so ids like sub_zzzz... passed as ours although subscription_id() only ever emits 32 lowercase hex characters. The ingress route and managed-webhook recognition use it to tell our ids from foreign ones, so they now reject anything else.
…file The custom v1.<kid>.<AES-GCM> envelope becomes a Subscription Record v1 sealed with the package's mcp-subscription+jwe profile, bound to this server, the project and the webhook id. The adapter keeps what the profile leaves to us: subscription ids, the principal digest, the derived Appwrite signing key and the authPassword size budget.
… repr The published vector's lease ends in 2030, so Keyring.record and open take an optional clock that the vector test pins, as the package's own tests do. A Subscription's whsec_ secret is no longer part of its repr.
Callback URLs are client-chosen, so every webhook POST is an attacker-steerable request. Add an SSRF-safe egress path that pins the connection to a checked global address while keeping SNI and Host, and a Standard Webhooks dispatcher for verification handshakes and bounded, re-signed event delivery retries.
Signing, retries, final statuses, TLS and redirect handling, timeouts and per-host limits are covered through the ingress e2e flows (#134). What stays needs a controlled resolver or address table, runs only from subscribe (PR 5), or waits on the production retry schedule.
allow_loopback widened the allowed schemes to http for every host, so an embedder enabling it could send signed event data unencrypted to a public callback. Plain http now needs a loopback host (localhost or a loopback IP) whose every resolved address is loopback. The per-host slot was acquired before the request deadline started, so a request queued behind slow ones to the same host could wait without bound and go out with a signature timestamped long before it was sent. One deadline now covers the slot wait and the request, and Egress.post takes a headers factory that is called only once the slot is held, right before sending.
A subscription record carries one delivery.secret, so a Callback holds one secret and signs once; the multi-secret path had nothing left to feed it. The secret is no longer part of a Callback's repr.
delivery.Reason duplicated the CallbackFailure enum from errors.py value for value. Keep one source so the subscribe handler can pass CallbackError.reason straight into EventsError.callback_endpoint.
Mount POST /appwrite/webhooks/{id} behind the events flag. The Appwrite signature authenticates it: the ingress rebuilds the registered URL from MCP_PUBLIC_URL, derives the webhook secret from the envelope's key id, hashes the body as it streams, then opens the envelope for the webhook and project.
Bodies are projected onto the event's payload schema and never forwarded raw. Anything from the subscriber's own webhook gets a 2xx (expired, filtered, unknown, retired key, oversized) so Appwrite never pauses it and emails the owner; only failed authentication gets 401.
Deliveries run in a task group owned by the app lifespan with one shared Egress and Dispatcher, and the request returns at once. Adds mcp.events.ingress and mcp.events.deliveries counters, and drops the section-header comments in telemetry.py.
Replace the TestClient and MockTransport ingress tests with flows through the real hosted server: Appwrite deliveries signed like the webhooks worker, deliveries verified by the official standardwebhooks library at a real HTTPS receiver, and counters read from the server's OTLP export. This also covers the signing, retry, TLS, redirect and per-host limit behavior the delivery and egress unit tests no longer check.
Each accepted webhook starts a delivery that can live for minutes of retries, with no admission bound, so a steady stream of events to failing callbacks could exhaust memory. The ingress now holds at most 1024 deliveries in flight per process and 64 per subscription; past either limit it still answers 200 (Appwrite never pauses the webhook) and drops the event as backlog_full. Two replicas refreshing one subscription during a rolling key rotation can interleave their writes and leave the new key's envelope next to the old key's signing secret, failing every delivery until the next refresh. The signature is now checked against the secret of every key in the ring. That keeps the webhook format and the rotation procedure as they are, unlike a separate non-rotating signing root, which would need a second secret to configure and could never be rotated. The e2e flows also cover what the stack's earlier unit tests did: status filters for completed executions and building deployments, envelopes that are truncated, not base64, or decrypt but do not match the id, foreign subscription ids, and the egress deadline including the per-host slot wait.
The 2 MiB limit only bounded what was kept; the public route kept reading and hashing, under every ring key, until the client closed the stream, so an unauthenticated caller could hold connections and CPU with an endless or stalled body. A request without a signature is now refused before its body is read, and a body past 16 MiB (declared or streamed) or still arriving after 10 seconds is acknowledged as unread and never verified or delivered.
The ingress reads the key id from the JWE header, opens the envelope for this server, the routed webhook id and the Appwrite project header, and treats the profile's expiry as the 2xx expired drop. A subscription carries one delivery secret, so the dual-secret flow goes; the forged-delivery flow now also covers an envelope sealed for another server and one with an unsupported header.
Body reads come before the signature check, so anyone who knows a subscription id and a key id could hold memory and sockets with concurrent forged bodies. A process now reads at most READS (32) bodies at once; a request waits for a slot within its read timeout. Every read ends by its own deadline, so a stalled forger delays real deliveries but cannot starve them.
Each subscription is one managed Appwrite webhook in the subscriber's project, written with their own OAuth token, so the hosted server stays stateless. Subscribe validates, authorizes with a resource read, cleans up the caller's expired webhooks, verifies the callback and upserts the webhook; unsubscribe deletes it by the principal-bound id.
An Appwrite REST emulator joins the e2e harness, and the webhooks worker now fires the webhooks the server actually stored, closing the loop from subscribe to signed delivery to unsubscribe. The unit tests that only existed because nothing called the catalog, whsec_ validation, the handshake or subscription ids over HTTP are gone.
Covers the TTL policy, the webhook label used for cleanup, Free-plan limits, required OAuth scopes and secret rotation, and keeps the events flag framed as a testing flag.
events/unsubscribe for a project that does not exist reached the webhook
error fallback as -32603. A 404 project_not_found now maps to -32012 like
subscribe, so project ids cannot be probed; a missing webhook stays an
idempotent {}.
Expired-webhook cleanup decided from the earlier listing and deleted
unconditionally, so a concurrent refresh of that subscription could have its
now-live webhook deleted. Appwrite has no conditional delete, so cleanup now
only considers webhooks expired for over 10 minutes and re-reads each one
right before deleting it, skipping it when its name or $updatedAt changed.
The e2e flows add the Appwrite emulator's interleaving hook and use it for a
refresh landing mid-cleanup and for two replicas refreshing one subscription
across a key rotation, and check that the server refuses exactly what the
published input schemas reject.
A token that could list webhooks but not write them passed the listing and got the callback a signed verification POST before the write failed. The token's scope claim is now checked first: without project:webhooks.write, project:all or all it is -32012 before Appwrite or the callback is reached. A refresh whose secret update failed deleted the webhook. The previous signing key was derived from a key still in the ring, and the ingress accepts every ring key, so the webhook keeps delivering; the error is still reported so the client retries. Rotation is documented as three deploys (add the key behind the active one, then move it first, then drop the old one), so no replica ever receives a delivery sealed with a key it does not hold.
events/subscribe seals one delivery secret and the granted lease (expiresAt, refreshBefore) into the record, bound to this server through Ingress.server, and answers -32602 for a delivery the record refuses. The subscribe flow now opens the stored envelope and checks its key id, callback, secret and lease.
A failed second write left a new envelope with a signing key derived from the previous sealing key, which could be retired while that envelope was still live. Writing the signing key first bounds a stale pairing by the previous envelope's own expiry.
…y replica Concurrent refreshes during step 2 can pair a new-key envelope with an old-key signing secret. Only step-1 replicas write that secret, so counting the TTL from the end of step 2 guarantees such envelopes expire before the old key leaves the ring. The e2e now produces that pairing and delivers through both replicas.
After a key rotation an orphaned webhook can pair an expired envelope from the new key with a signing secret from the retired one. Checking the signature first answered 401, so Appwrite paused the webhook and emailed the owner. Opening the envelope first authenticates it and drops the expired lease with 200.
feat: advertise MCP events and list the event catalog
feat: seal MCP event subscriptions into an envelope
feat: sign and deliver MCP events safely
feat: receive Appwrite webhooks and forward MCP events
feat: serve events/subscribe and events/unsubscribe
🔵 Tier A · Mergeable after minor fixes
Adds an opt-in MCP Events surface for hosted HTTP, including event discovery, webhook-backed subscriptions, signed delivery, and an Appwrite webhook ingress. Adds end-to-end coverage and documentation for the feature flag, envelope/key handling, and delivery behavior. 26 of 43 changed files were too large to include in full.
Note Not approving while a bug finding is open: Do not report a complete subscription for an unsupported cursor
Fix with agent prompt### Issue 1
src/mcp_server_appwrite/events/subscriptions.py:294-295
**Do not report a complete subscription for an unsupported cursor**
When a client resubscribes with a non-null cursor, this path still returns `truncated: false` and creates a fresh Appwrite webhook without replaying anything. Since Appwrite cannot replay, events between that cursor and webhook creation are silently lost; reject non-null cursors or indicate the gap.
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.📂 Walkthrough · 8
Reviewed |
![hansi-codes[bot]](https://avatars.githubusercontent.com/in/5046327?s=60&v=4){kind=small}
Comment on lines
+294
to
+295
| "cursor": None, | ||
| "truncated": False, |
There was a problem hiding this comment.
Do not report a complete subscription for an unsupported cursor
When a client resubscribes with a non-null cursor, this path still returns truncated: false and creates a fresh Appwrite webhook without replaying anything. Since Appwrite cannot replay, events between that cursor and webhook creation are silently lost; reject non-null cursors or indicate the gap.
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/mcp_server_appwrite/events/subscriptions.py
Line: 294-295
Comment:
**Do not report a complete subscription for an unsupported cursor**
When a client resubscribes with a non-null cursor, this path still returns `truncated: false` and creates a fresh Appwrite webhook without replaying anything. Since Appwrite cannot replay, events between that cursor and webhook creation are silently lost; reject non-null cursors or indicate the gap.
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.🟡 Minor · bug · Reply if this doesn't apply.
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.
Summary
Adds MCP Events (webhook delivery, as ChatGPT uses it) to the hosted server, behind the
eventstesting flag (MCP_EVENTS, default off). With the flag off, nothing changes: no capability is advertised, theevents/*methods return-32601, and/appwrite/webhooks/*is not mounted (covered byEventsDisabledFlow).Closes nothing yet; tracks #127. The flag stays off until appwrite/appwrite#14293 is on Cloud, because the sealed subscription envelope (about 1.1–1.6k characters) does not fit Appwrite's current
authPasswordstorage.Reviewed and merged as a stack into
feat/events:eventsflag, capability and extension advertisement (with theserver/discoversieve workaround),events/list, error codes, e2e harness and CI jobdir+A256GCM), sealing key ring and rotation, derived Appwrite signing keysPOST /appwrite/webhooks/{id}, signature and envelope checks, projection, bounded backlog, telemetryDesign
The server keeps no state. Each subscription is one Appwrite webhook in the subscriber's project, whose id is the subscription id and whose Basic auth password is the sealed envelope. See
docs/events.md.Known limits
Enabling later
MCP_EVENTS_SEALING_KEYS(seedocs/development.md) andMCP_EVENTS=1on the deployment.Test plan