Skip to content

Synapse v2.1.13 — "one stream, one grant"

Patch release closing two defects found by reading the SSE reconnect loop and the MCP OAuth PKCE check during weekly maintenance. No schema migrations, no new endpoints, no API changes, no new configuration keys.

Like 2.1.8 through 2.1.12, neither came from a live failure report, and both are the same familiar shape: a guard that does not guard. One is a boolean asked to answer a question it cannot represent; the other is a validation gate written as if its inputs were already known-good, when they are raw form fields. In both cases the surrounding code states the right intent — the defect is that the intent is not what the code does.

Fixed

A remount during the reconnect backoff left a second, unstoppable /events stream running

eventsStore.ts runs one reader loop over the GET /events SSE channel, with reconnect on an exponential backoff. Whether that loop should keep going is decided by a single module-level boolean:

let controller: AbortController | null = null;
let stopped = true;

const runLoop = async (): Promise<void> => {
  while (!stopped) {
    const ctrl = new AbortController();
    controller = ctrl;
    ...
  }
};

stop() sets stopped = true and aborts controller; start() sets it back to false and starts a loop. That works for one loop. It stops working the moment there can transiently be two — because stopped cannot distinguish "the stream is stopped" from "a newer loop owns the stream now".

Root cause. One shared flag used as the exit condition for what is not always a single loop.

The window is the reconnect backoff. A loop waiting there is suspended, not finished:

const delay = backoffDelayMs(consecutiveFailures - 1);
await new Promise<void>((resolve) => { ... });
// ← resumes here, then re-tests `while (!stopped)`

A stop() immediately followed by a start() — React StrictMode's double-effect, a Vite HMR reload, any AppShell remount — flips stopped true and then false again before the suspended loop resumes. When it does resume it re-tests the condition, finds !stopped, and carries on as though nothing happened: it opens its own connection alongside the one the new loop already opened, and overwrites controller, which only ever tracks the most recent.

From that point the older loop is unreachable. stop() aborts a controller that is no longer its own, so the orphan:

  • holds a server-side /events generator open for the life of the tab;
  • applies every data_version and queue frame to statusStore / activityStore a second time;
  • and is joined by one more on each subsequent remount.

The backoff is not a rare place to be caught. It is entered on every reconnect, including the clean one the backend forces on every connection at EVENTS_MAX_STREAM_SECONDS (30 minutes by default), and it stretches to 30 seconds whenever the backend is unreachable — which is precisely when a developer is reloading the frontend most often.

Fix. A generation token. Each loop captures the generation it was started with and exits as soon as a newer one supersedes it, aborting its own controller on the way out:

let generation = 0;
...
const myGeneration = ++generation;
const isCurrent = (): boolean => !stopped && myGeneration === generation;

stop() bumps the generation as well, so a suspended loop cannot be revived by a start() that flips stopped back before it resumes.

The guard sits at both fall-through points, not only the catch. A clean server-side close leaves the reader via done, not via an exception, and reaches the same reconnect path:

if (!isCurrent()) {
  ctrl.abort();
  return;
}

This is the same shape as the latent pollChain defect recorded in ROADMAP-3.0-IDEAS.md on 2026-08-28, which was deliberately left alone because nothing could reach it. The difference here is that this one is reachable today, and the regression test (T-SSE-009) drives it deterministically: it asserts exactly two connection attempts across a stop/start straddling the backoff, and fails with three against the old code.

A non-ASCII PKCE value answered 500 instead of invalid_grant

_verify_pkce (app/mcp/oauth.py) is the gate that decides whether an authorization code may be exchanged for a token. Both halves of its comparison refuse non-ASCII input by raising:

digest = hashlib.sha256(code_verifier.encode("ascii")).digest()   # UnicodeEncodeError
computed = base64.urlsafe_b64encode(digest).rstrip(b"=").decode("ascii")
return secrets.compare_digest(computed, code_challenge)           # TypeError

str.encode("ascii") raises UnicodeEncodeError on the verifier. secrets.compare_digest() raises TypeError — "comparing strings with non-ASCII characters is not supported" — on the challenge. Neither value had ever been checked for it, and both arrive as untrusted form fields: code_verifier on POST /token, code_challenge on /authorize.

Nothing caught either exception, so a single non-ASCII byte propagated out of POST /token as an unhandled 500 — on an endpoint any client, or any prober, can reach.

Root cause. A validation gate written as if its inputs were already known-good, when they are raw form fields.

Fix. _verify_pkce fails closed on a non-ASCII verifier or challenge rather than raising. RFC 7636 §4.1 confines code_verifier — and therefore the BASE64URL challenge derived from it — to ASCII unreserved characters, so an input the gate cannot even hash is a failed verification, never an exception that escapes the caller:

if not _is_ascii(code_verifier) or not _is_ascii(code_challenge):
    return False

The challenge half is additionally refused earlier, at /authorize. A challenge that no verifier can ever match makes the grant dead on arrival, so there is no reason to mint a pending code for it — still less to render the consent form and ask the operator to type their static MCP token in to approve a code that could only ever fail. It is checked on the POST as well as the GET, since the consent form is not a trust boundary and the POST is reachable directly.

Security posture is unchanged either way: the old behaviour already failed closed in the sense that no token was issued, and the authorization code had already been consumed (single-use) before verification ran. What changes is that a malformed request now gets the 400 it deserves instead of a 500, and stops generating unhandled-exception noise in the operator's logs.

No conforming client is affected: a valid S256 challenge is BASE64URL, which is ASCII by construction.

Also in this release

An entry was added to ROADMAP-3.0-IDEAS.md for a third finding that is not fixed here: ApiTokenCache.find_match() runs a 260 000-iteration PBKDF2 per active token, synchronously, inside the ASGI auth middleware — measured at 175 ms each, blocking the whole event loop (in-flight chat streams, /events, the watcher, graph ticks) for the duration of the scan.

It is reachable unauthenticated. The bootstrap token is compared first with compare_digest, so the PBKDF2 path is taken whenever the presented bearer is not that token — including a bearer of pure garbage. A stream of such requests costs the backend N × 175 ms of blocked event loop each, at no cost to the sender, and presents to the operator as "the backend became inexplicably slow".

The real fix stops scanning linearly: give each token a plaintext identifier (<token_id>.<secret>, as GitHub and Stripe do) so verification is an O(1) lookup followed by exactly one PBKDF2. That changes the on-the-wire format of tokens already issued — breaking, therefore 3.0, and not something to land in an unattended weekly patch that must never lock an operator out of their own backend.

Upgrade notes

Nothing to do. Pull the image (or rebuild) and restart:

docker compose pull && docker compose up -d
  • No schema migration. No SQLAlchemy model changed, so docs/er/schema.mmd is unchanged and no Alembic revision ships with this release.
  • No API change. docs/api/openapi.json differs only in the version string. Request and response shapes for /events, /authorize and /token are identical.
  • No new configuration. EVENTS_MAX_STREAM_SECONDS, EVENTS_POLL_INTERVAL_SECONDS and EVENTS_HEARTBEAT_INTERVAL_SECONDS keep their meanings and defaults; nothing new to set in .env.
  • Two observable differences, both by design:
  • POST /token with a non-ASCII code_verifier now answers 400 (invalid_grant) where it previously answered 500, and /authorize with a non-ASCII code_challenge now answers 400 instead of rendering the consent form. Any connector that completes the flow today is unaffected — a BASE64URL challenge is ASCII by construction.
  • A browser tab that remounts the app shell no longer accumulates extra /events connections. If you were seeing more concurrent SSE connections than open tabs, that is the cause, and it resolves on upgrade with no action.
  • No client-side state to clear. Existing OAuth grants, refresh tokens and API tokens are untouched; nothing needs to be re-approved or re-issued.