Repository navigation
id(web): collab server stores and broadcasts steps it never validates — one bad step poisons the document's history and loops every client #1618
Description
Summary
The collab server accepts, stores, broadcasts and Acks ProseMirror steps without ever checking that they parse or apply — one invalid step poisons a document session for every current and future client.
In pkgs/id/src/web/collab.rs a step is an opaque blob end to end:
Stepis a flattenedserde_json::Value(collab.rs:164-169) — there is no parse or validation constructor.CollabMessage::decode(collab.rs:536-545) accepts a Steps frame as(u8, u64, Vec<serde_json::Value>, u64): any JSON values at all count as "steps".- The Steps handler (
collab.rs:986-1052) checks only that the frame's version equals the current version, then pushes every step intodoc.steps(:1019-1024), bumps the version bysteps.len()(:1028), broadcasts anUpdateto the other clients (:1032-1036) and Acks the sender (:1044-1052). It never applies the steps to a document and never asks whether they are well-formed. - On join, the catch-up path (
collab.rs:787-810) replays all stored steps to the new client, so a stored bad step is re-delivered forever, for the life of theDocument(cleanup only runs an hour after the last client disconnects).
Auth is not a gate: a client that sends no AUTH frame in 5 seconds "proceeds as anonymous" (collab.rs:694-755) and can send Steps. A merely buggy or version-skewed client reaches the same place without malice — e.g. a step built against a diverged local doc, or a node type an older/newer web build's schema does not share.
Evidence (verified on main b2adaf47, 2026-10-28)
collab.rs sha256 55e04270… and collab.ts sha256 0fb75f70…, identical in the sandbox and laptop clones. Two probes ON THE LAPTOP (~/git/fleet/probe-2026-10-28-laneA-collabsteps/; sources + logs in the goal's hidden_files/probe-2026-10-28-laneA-collabsteps/):
Rust verbatim-decode probe (the exact encode/decode tuple shapes from collab.rs:479-482 / :536-545):
CTRL one text step: decode -> Ok(tag=1, version=3, steps=1, client_id=4242)
BOGUS stepType: decode -> Ok(...) {"stepType":"bogusType","from":1,"to":2}
RANGE replace to=999999: decode -> Ok(...)
NON-OBJECT steps (42, null, "hi"): decode -> Ok(steps=3)
TS probe driving the real client receive path (Step.fromJSON + receiveTransaction from prosemirror-transform / prosemirror-collab, as collab.ts:278-279 calls them; basic schema standing in for the mode-aware editor schema):
CTRL_valid: fromJSON OK, apply ok, receiveTransaction OK ("hello world!")
BOGUS_type: Step.fromJSON -> THROW "No step type bogusType defined"
RANGE_replace: Step.fromJSON OK, Step.apply -> THROW "Position 999999 out of range",
receiveTransaction -> THROW
SCHEMA_slice: Step.fromJSON -> THROW "Unknown node type: no_such_node"
The receiving client's only handling for that throw is collab.ts:281-293: log "Failed to apply steps", close the socket with code 4001 and scheduleReconnect() — which rejoins, receives Init + the same catch-up steps, and throws again. One stored step therefore puts every client of that document into a reconnect loop and makes the document uneditable until the session's Document is cleaned up.
Why it matters
The server is the authority for the shared step history — clients trust its Update/catch-up stream to be applicable, because receiveTransaction cannot skip a step in the middle of the version sequence. Storing unvalidated steps breaks that invariant permanently rather than transiently: the failure is not one dropped edit but a poisoned history that every joiner must replay. The version counter also advances by the number of poison steps, so even the version bookkeeping certifies steps that were never applied anywhere.
Acceptance criteria
- A Steps frame containing a step that does not parse against the document's schema, or does not apply to the document at the frame's version, is refused (Error to the sender, no Ack): nothing is stored, the version does not advance, and nothing is broadcast.
- Validation is against the server's actual document state (base doc + previously accepted steps) or an equivalent server-side model — a purely syntactic/shape check that still admits the out-of-range replace above does not satisfy this.
- Valid multi-step frames from a client at the current version keep working exactly as today (stored, version += n, broadcast, Ack), pinned by a test alongside the refusal cases (bogus stepType, unknown node in slice, out-of-range positions, non-object step).
- A client that somehow still receives an unappliable step does not loop forever on the same history: e.g. the reconnect path distinguishes "catch-up itself is poisoned" and surfaces a terminal error for that document instead of rejoining indefinitely — or the server-side guarantee is documented as the sole line of defence with a test that a poisoned history cannot be created.
- The anonymous-client reachability is considered explicitly: whatever validation is added applies before storage for authenticated and anonymous senders alike.
Activity
developing-today commentedon Oct 28, 2026
CODE-642
dezren39 commentedon Oct 28, 2026
agent: new-issue-creator-code-a
state: filed
found_via: filed this run
at: 2026-10-28T07:45:46Z
Filed this run after verification on main
b2adaf47(collab.rs sha25655e04270…, collab.ts0fb75f70…, identical in both clones):Step.fromJSON+receiveTransaction, as collab.ts:278-279 calls them): bogus type and unknown slice node throw at fromJSON; the out-of-range replace throws in receiveTransaction; the control step applies cleanly. The client's only recovery (collab.ts:281-293) is close + reconnect, and catch-up (collab.rs:787-810) replays the same stored steps — so one bad step loops every client and every future joiner.Labels: kind/bug, queue/queued, source/creator, task/triage, status/needs-triage, priority/normal, app/id, effort/s. Queue packet enqueued as a triage task in repo-health-code-pilot (see backlog). Deliberately NOT self-solved: the server has no server-side ProseMirror document model today (steps are opaque values and the doc is never advanced server-side), so adding real apply-validation is an architecture choice for queue-lane triage, not a creator one-liner.
dezren39 commentedon Oct 28, 2026
agent: new-issue-creator-code-a
state: filed
found_via: correction to the filing note, same run
at: 2026-10-28T07:50:00Z
Precision correction to my filing note: the client-side reconnect is not literally unbounded —
scheduleReconnect(collab.ts:149-155) caps at 10 attempts with exponential backoff, then sets statuserrorand stops. The substance is unchanged: every attempt rejoins into the same poisoned catch-up and throws again, the client then gives up, and any fresh page load or new joiner hits the same stored step in catch-up and fails the same way. The document stays uneditable until the server-sideDocumentis cleaned up (1h after the last client disconnects). Read "reconnect loop" in the issue as "reconnect loop up to the 10-attempt cap, then a dead session — repeated on every join".37 remaining items
dezren39 commentedon Nov 3, 2026
🤖 Agent note · picked up
dezren39 commentedon Nov 3, 2026
🤖 Agent note · blocked
packet_defect; no implementation or worktree created.1e3763faf4e3997160814a22f2e268184b1d128c; related open issue id(web/collab): Steps version check and append are not atomic — two clients at the same base version both pass, both append, and the document silently diverges #2968Fresh revalidation: #1618 is open and
gh pr list --state all --search "1618 collab"returned no matching PR. The queued packet is materially incomplete for itssmall_fix/ 45-minute classification: it requires actual schema-aware sequential ProseMirror application plus atomic all-or-nothing commit before storage/version/broadcast, but chooses no authoritative server-side ProseMirror engine or schema-parity strategy and no serialization boundary. On current main,Document.docis JSON andapply_stepsstill stores opaque steps; related #2968 remains open for the non-atomic version check/append. Prior triage also found no Rust ProseMirror engine and called this an architecture choice. Please settle the server-side model and atomic serialization design, then enqueue a bounded implementation task. No branch, worktree, code changes, or tests.developing-today commentedon Nov 3, 2026
agent-note:v1
state: reviewed (coverage loop-back)
Coverage pass 2026-11-03, loop-back review: this issue was re-reviewed (drift / overlap / already-handled / stale / blockers / ordering) so every open issue in the four repos is reviewed and in the system. Verdict: still live; in the system as task 4790 (small_fix, blocked, priority 50) in repo-health-code-pilot. Labels audited and synced. If it is already handled at current main, its triage closes it with evidence; if oversized, the triage files the split; blockers belong on this issue.
Found via: coverage loop-back 2026-11-03