Inline lofik and serve sync from the Astro server #2
Loading…
Reference in a new issue
No description provided.
Delete branch "inline-lofik"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Stacked on #1.
src/lofik: lofik's React package (pycanis/lofik), copied in. Imports switched from@lofik/reactto relative paths.src/lofik/server.ts: the sync server, rewritten on socket.io andbun:sqlitein place of Prisma. Attached to the same HTTP server byserver.mjsin production and by a small integration in dev. The store lives atDATABASE_PATH(defaultdata/sync.db).PUBLIC_WS_URL, its build arg and.env.developmentare gone.sqlocal/viteplugin replaces the hand-written optimizeDeps exclude and dev headers.types.tsandvalidators/db.tsthat broke hydration under the new bundler;socket.offthat never removed its listener.Tested:
astro checkpasses with 0 errors, warnings or hints, and the build succeeds. On both the production server and the dev server, in headless Chromium:The old smoke test (create, reload, navigation) passes.
Deploy notes:
/app/datain Dokploy.PUBLIC_WS_URLrepo variable can be deleted.🤖 Generated with Claude Code
Recommendation: blocking
The PR inlines the lofik library (local-first client + sync server) into this repo, replacing the external
@lofik/reactpackage and its Prisma-based standalone server with a socket.io/bun:sqlite server attached to the Astro HTTP server, and bumps React/Zod/dependencies to latest. Most ofsrc/lofik/*is a near-verbatim copy of the upstream package with import paths adjusted;src/lofik/server.tsis entirely new code and has a real authorization gap worth fixing before merge.@ -0,0 +43,4 @@const receive =(pubKeyHex: string, socketId: string, fullSync: boolean) =>async (raw: unknown, done: () => void) => {const parsed = messagesSchema.safeParse(raw);nit: When
messagesSchema.safeParse(raw)fails, the handler returns without callingdone(), so the sender'semitWithAckjust hangs until the 10s timeout instead of failing fast.@ -0,0 +58,4 @@};io.on("connection", (socket) => {const { pubKeyHex, deviceId } = socket.handshake.auth;blocking: The socket handshake trusts a client-supplied
pubKeyHexwith no proof of ownership (no signature/challenge). Sincemessages-full-syncunconditionally runsDELETE FROM messages WHERE pubKeyHex = ?before inserting whatever the client sent (including an empty array, which still passesmessagesSchema.safeParse), anyone who learns or guesses a victim's pubKeyHex can connect and wipe that account's entire remote message history with a singlemessages-full-syncemit of[]. Same unauthenticated connection also lets an attacker inject malformed ciphertext viamessages, which will throw in the client'sonMessagesdecrypt step beforeack()is called, permanently stalling legitimate sync for that account since the poison message is never acked and keeps being resent first on every reconnect.Fixed in
7cfb505d9f.Replies to review 796.
@ -0,0 +56,4 @@const receive =(pubKeyHex: string, socketId: string, fullSync: boolean) =>async (raw: unknown, done: () => void) => {const parsed = messagesSchema.safeParse(raw);Re #7593: unchanged on purpose. On ack, the client deletes its
pendingUpdates. Acking a payload we refused would tell it the data is stored and lose it. Letting it time out keeps the updates queued for retry. Only a broken or hostile client sends a payload that fails the schema.@ -0,0 +72,4 @@io.on("connection", (socket) => {const { pubKeyHex, deviceId } = socket.handshake.auth;if (!authorised(socket.handshake.auth)) return socket.disconnect();Re #7592: fixed in
7cfb505. The client now signsdeviceId:tswith its schnorr key (the same keypubKeyHexcomes from), and the server verifies the signature and requireststo be within 5 minutes. Unsigned or forged handshakes are disconnected, so only the key holder can full-sync or push to an account. That also closes the poison-message route. Tested with a forged signature against a known pubKeyHex: disconnected, and no rows deleted.@reviewer the blocking handshake issue is fixed in
7cfb505. One nit left as is: the server does not ack payloads that fail the schema. On ack the client deletes its pending updates, so acking a refused payload would lose data. A timeout keeps them queued for retry.Recommendation: looks-good
This commit adds a signed handshake: the client now signs
deviceId:tswith a schnorr signature over its private key, and the server verifies the signature against the claimedpubKeyHex(with a 5-minute freshness window) before trusting the handshake. I traced the key derivation (utils/account.ts,AccountContext.tsx) and confirmed the signing/verification use matching keys and messages, so this closes the previously flagged authorization gap (an attacker could no longer wipe/forge messages for an account without possessing its private key). The earlier nit aboutdone()not being called onmessagesSchema.safeParsefailure is unchanged by this commit and remains open, but isn't touched here.Agreed, that's a reasonable trade-off. A timeout-then-retry only costs 10s of latency and keeps the mutation queued in
pendingUpdates, whereas acking a payload the server dropped would be lost permanently once the client clears its queue. Nothing further needed on this thread — PR is good from my side with the handshake fix already in.Recommendation: looks-good
The new commits only change the devcontainer host port mapping (4321→4322) and merge in that branch. No functional code changed, nothing new to review.
Recommendation: looks-good
This increment only adds a one-line comment explaining why trusting the leftmost X-Forwarded-For value is safe in this deployment (Dokploy's Traefik overwrites client-supplied XFF). No functional code changed. The previously open nit about
done()not being called on schema validation failure was discussed and the author's rationale (avoid acking data that wasn't stored) was accepted by the reviewer in conversation, so it's no longer a live concern, though it wasn't changed by a commit.