{{ message }}
Repository navigation
fix(db-sync): websocket auth errors, semantic API 400s, and sync server hardening - #13781
Open
yuxi-liu-wired wants to merge 9 commits into
Open
yuxi-liu-wired wants to merge 9 commits into
yuxi-liu-wired wants to merge 9 commits into
Conversation
This was referenced Oct 16, 2026
Contributor
Contributor
Author
yuxi-liu-wired
force-pushed
the
fix/db-sync-combined
branch
3 times, most recently
from
October 21, 2026 18:05
a549b97 to
8bc75ee
Compare
…thout a string title
…t is not a string
… only an entity id
…rver Security review follow-ups (H1, H3, M1-M4, L1, L2, L4, L5): - Third-party OAuth tokens (logseq/* scopes or additional Cognito client ids) are rejected on the native surface: /sync, /assets, /graphs, /e2ee, the PAT endpoint and the node WebSocket upgrade. The MCP code-mode guard checks the resolved URL origin and pathname. - Graph deletion requires a manager; /sync/<g>/admin/reset is 404 without the admin token; /internal/* tails are blocked in both dispatchers. - DB_SYNC_ALLOW_UNVERIFIED_JWT_CLAIMS falls back only on the JWKS- unreachable error; every other verification failure is a plain 401. - The x-graph-id header is gone; the sync path is authoritative. - Member removal closes the member's sockets on a new DO-internal route; expired tokens close their socket on the next message. - The DO routes the path exactly as received (no /sync prefix stripping), the node adapter blocks /semantic tails, and graph-e2ee is always set explicitly by dispatch. - Error responses carry no internal details (debug-*, error-detail). - Non-image asset types are served as attachment with nosniff; SVG gets a CSP sandbox. Plain-graph asset PUTs verify the declared checksum. - The node adapter no longer sends self-hosted users' tokens to api.logseq.com unless the issuer is Logseq's own pool. Note: the auth-claims catch handler returns false instead of nil and normalizes it, working around a promesa 11 bug: every nil resolution is one shared promise, and a task that throws during its drain rejects the other tasks of that drain.
… routes wait for the user upsert Removing a member fetched /internal/close-sockets without a graph id, and the node server's Durable Object stand-in found the graph only by the graph-id query parameter, so the removed member's socket stayed open there. The request now carries graph-id, and the stand-in falls back to the id from idFromName (the graph id). Index routes again wait for the user upsert (skipped for personal access tokens, as before), so a new user is in D1 before invite-by-email looks them up.
…bject at its internal paths The Durable Object routes the path dispatch forwards to it, unprefixed; the test now does as the others: /pull and /snapshot/download, and it checks the download hands the client the public /sync/<graph> stream URL.
yuxi-liu-wired
force-pushed
the
fix/db-sync-combined
branch
from
October 24, 2026 11:06
8bc75ee to
e01164b
Compare
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.

This PR combines #13692, #13772, #13780, which change the same files, into one branch on current master so they no longer conflict with each other. Each part is unchanged except where two of them touched the same lines.
#13692: fix(db-sync): the node server answers a websocket upgrade whose auth fails
A websocket upgrade to the Node sync server whose authentication fails with an error (a malformed bearer token such as
x.y.z, or a JWKS fetch failure) is never answered: the socket is neither upgraded, refused nor closed, and stays open until the client gives up. The same failure on an HTTP request gets a 500 (node-server-returns-500-when-auth-claims-rejects-test).Cause: the server's
"upgrade"handler runsaccess-allowed?(which callsauth/auth-claims) and the readiness check in a promise chain with nocatch. A token read as no user is dropped (allowed?is false), but a rejectedauth-claimsskips every branch, so nothing is written to the socket. Anyone can hold sockets open on the server this way without valid credentials; no graph data is reached.Fix: the chain gets a
catchthat logs:db-sync/node-ws-upgrade-failedand answers the upgrade with a 500 (then closes the socket), unless the socket is already gone.reject-ws-upgrade!now writes the reason phrase for its status (it always wroteConflict, which was right only for its one caller, the 409).Test:
logseq.db-sync.node-server-test/node-server-answers-ws-upgrade-when-auth-claims-rejects-test: withauth-claimsrejecting (as the existing HTTP test does), a websocket upgrade to/sync/graph-xgets an HTTP 500. On master it gets no answer and the test fails; with the fix it passes, along with the rest oflogseq.db-sync.node-server-testandlogseq.db-sync.worker-handler-ws-test(12 tests, 0 failures). Lint as in CI fordeps/db-syncand the app.Found by mutation testing (triage of
node/server.cljs).#13772: fix(db-sync): the semantic API answers 400 to a write the graph does not allow
The semantic API answers a write the graph does not allow with 500 "server error" and a debug stack. Examples:
POST /semantic/pageswith titlea/b("Page name can't include "/""),POST /semantic/tagswith#tag,PATCHorDELETEof a built-in page or block ("Built-in nodes can't be modified", "Built-in nodes can't be deleted").The outliner rejects such writes by throwing an error meant to notify the user (
ex-data:type :notification, the message in:payload). The semantic handlers check some inputs themselves and answer 400, but these errors come from inside the outliner call and reachhandle-http's catch-all, which answers 500.semantic/handlenow answers 400 with the error's message for an error of:type :notification, synchronous or from a promise. Every other error still reaches the 500 path.The routes that insert a block tree (
/semantic/capture,/semantic/blocks/:block-id/children,/semantic/block-trees) checked thatblocksis a non-empty list but not its blocks: a block whose title is not a string ({"blocks": [{"title": 1}]}, or a child{}) failed an assert,(string? title), answered 500. They now check the whole tree (every block a map with a stringtitleand optionalchildrenof the same shape) and answer 400 as for a missing list.The property routes resolved
:property-idto the ident of any entity it names:PUT /semantic/blocks/:block-id/properties/logseq.class%2FTask(a class, not a property) set it as a property and failed with "No validation for property type nil", answered 500.resolve-property-identnow returns an ident only for a property, so such a request answers 400 "property not found" as for a missing one.PATCH /semantic/properties/:property-idtook anytitle:{"title": {}}failed with "str.indexOf is not a function", answered 500. Atitlethere must now be a non-blank string, as on the other write routes.A value for a ref property (status, priority, tags, node) that is a number went to
d/entityas an entity id. Datascript throws on a number that is not one (-1,1.5: "Expected number or lookup ref for entity id"), answered 500, onPUT /semantic/blocks/:block-id/properties/:property-idandPOST /semantic/block-properties/batch-set. Only a positive integer is now looked up; another number resolves to nothing and answers 400 as for a missing entity.Test:
semantic-write-the-graph-does-not-allow-answers-400-test(a page titleda/b, a tag titled#tag, a capture of a block titled1, a capture of a child without a title, a class named as the property, a property update with an object as title, a status value of-1and of1.5).Found by a fuzzer that sends random semantic API requests to a server on a full graph.
#13780: fix(db-sync): confine third-party OAuth tokens and harden the sync server
fix(db-sync): confine third-party OAuth tokens and harden the sync server
Security review follow-ups (H1, H3, M1-M4, L1, L2, L4, L5):
ids) are rejected on the native surface: /sync, /assets, /graphs, /e2ee,
the PAT endpoint and the node WebSocket upgrade. The MCP code-mode guard
checks the resolved URL origin and pathname.
the admin token; /internal/* tails are blocked in both dispatchers.
unreachable error; every other verification failure is a plain 401.
expired tokens close their socket on the next message.
the node adapter blocks /semantic tails, and graph-e2ee is always set
explicitly by dispatch.
CSP sandbox. Plain-graph asset PUTs verify the declared checksum.
api.logseq.com unless the issuer is Logseq's own pool.
Note: the auth-claims catch handler returns false instead of nil and
normalizes it, working around a promesa 11 bug: every nil resolution is
one shared promise, and a task that throws during its drain rejects the
other tasks of that drain.