Repository navigation
🐛 Reject presence whose version is ahead of the doc - #749
Draft
alecgibson wants to merge 1 commit into
Draft
alecgibson wants to merge 1 commit into
alecgibson wants to merge 1 commit into
Conversation
|
coverage: 97.925% (+0.009%) from 97.916% — reject-future-presence-version into main |
alecgibson
commented
Oct 9, 2026
alecgibson
force-pushed
the
reject-future-presence-version
branch
from
October 9, 2026 11:09
7f7fc81 to
ae15459
Compare
There was a problem hiding this comment.
🟡 Changes recommended
Versionless database operations can bypass rejection, while rejected presence can leak subscriptions and cached state.
2 open findings
What changed in this PR
Rejects invalid or future document-presence versions to prevent repeated client fetch amplification.
Changes:
- Requires integer operation and presence versions.
- Validates presence against tracked or fetched document versions.
- Adds presence-version and deleted-operation tests.
| File | Description |
|---|---|
lib/agent.js |
Validates versions and passes tracked versions for presence transformation. |
lib/backend.js |
Detects and rejects future presence versions. |
test/agent.js |
Tests fractional operation-version rejection. |
test/client/presence/doc-presence.js |
Tests presence validation and transformation scenarios. |
🧠 Review effort: Balanced
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Fixes #735 At the moment, the server relays a doc presence whose `v` is ahead of the doc unchanged, which lets one client make every subscriber fetch: 1. Submit presence with `v: 1e9` to a doc with 20 subscribers 2. Each subscriber calls `doc.fetch()`, since `v > doc.version` 3. The presence stays pending, so every later op fetches again This change has the `Agent` pass its tracked doc version through to `transformPresenceToLatestVersion()` (a new argument before `callback`), which reads ops from the lower of that and `v`. The tracker can lag the database but never lead it, so a `v` past the last op read is ahead of the doc. Only presence with no tracked version still reads the snapshot. We also now require an integer `v`, so a fractional one can't sneak past. A rejected presence restores the `Agent`'s previously cached request, so the leave it replays on disconnect isn't rejected too. We deliberately don't bound presence that is far *behind*: a `fetch` with `v: 0` already makes the server read the whole op history. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
alecgibson
force-pushed
the
reject-future-presence-version
branch
from
October 9, 2026 14:43
ae15459 to
df41add
Compare
|
|
||
| Backend.prototype.transformPresenceToLatestVersion = function(agent, presence, callback) { | ||
| Backend.prototype.transformPresenceToLatestVersion = function(agent, presence, knownVersion, callback) { | ||
| if (!presence.c || !presence.d) return callback(null, presence); |
|
|
||
| if (request.a === ACTIONS.op || request.a === ACTIONS.presence) { | ||
| if (request.v != null && (typeof request.v !== 'number' || request.v < 0)) return 'Invalid version'; | ||
| if (request.v != null && !util.isValidVersion(request.v)) return 'Invalid version'; |
Comment on lines
+849
to
+851
| if (error) { | ||
| restorePreviousPresenceRequest(requests, presence, previousRequest); | ||
| return callback(error); |
This branch has not been deployed
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
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.
Fixes #735
At the moment, the server relays a doc presence whose
vis ahead of the doc unchanged, which lets one client make every subscriber fetch:v: 1e9to a doc with 20 subscribersdoc.fetch(), sincev > doc.versionThis change has the
Agentpass its tracked doc version through totransformPresenceToLatestVersion()(a new argument beforecallback), which reads ops from the lower of that andv. The tracker can lag the database but never lead it, so avpast the last op read is ahead of the doc. Only presence with no tracked version still reads the snapshot. We also now require an integerv, so a fractional one can't sneak past. A rejected presence restores theAgent's previously cached request, so the leave it replays on disconnect isn't rejected too.We deliberately don't bound presence that is far behind: a
fetchwithv: 0already makes the server read the whole op history.🤖 Generated with Claude Code
Co-Authored-By: Claude noreply@anthropic.com