mirror of
https://github.com/J3vb/OwnCord.git
synced 2026-09-03 03:50:00 +03:00
fix(b2-8): close the nine B2-tagged ledger findings (#1436)
* docs(b2-8): record the B2-1 pre-squash head and ledger the voice_join ordering hazard - B2-1 evidence block: PR #1435 pre-squash head069412db(refs/pull/1435/head), squash1fe3df79- Ledger OC-0349 (open, low): the joiner's own voice_state takes the hub queue while the rest of the join burst is written directly, so its position on the joiner's socket is not ordered (documented in docs/protocol.md during B2-1, not yet fixed) * fix(client): 2 defect(s) (OC-0311, OC-0315) OC-0311: scope the voice_leave E2EE participant-left notification to this client's own voice channel — the frame is broadcast to the whole channel read audience, so a peer leaving a channel we merely read could delete their key, clear their verification, and trigger a room-key rotation in our live session. OC-0315: parse server timestamps with parseTimestamp instead of Date.parse in the reconnect replay gate and the clock-skew sample — the wire form is naive UTC with no 'Z', which Date.parse reads as local time, so an east-of-UTC viewer silently swallowed genuinely live messages. * fix(voice): 1 defect(s) (OC-0316) * fix(client): 1 defect(s) (OC-0317) * fix(plugin): 1 defect(s) (OC-0318) @ Route every directory-manifest resolution through loadManifestFromDir so InstallFromZip and scanPluginDirectory apply identical plugin.toml over plugin.json precedence. A TOML-only zip now installs, and a zip carrying both manifests is rejected rather than validating one while the loader later obeys the other. @ * fix(client): 1 defect(s) (OC-0322) * fix(ws): 1 defect(s) (OC-0337) liveVoiceEventsSince's cold-tier fallback handed a cap-truncated window to resuming clients as if it were complete. The query is oldest-first with a LIMIT, so a full result means the NEWEST rows were dropped - for a voice room, quite possibly a peer's voice_leave. Degrade to nil (the documented best-effort miss) on a cap hit, matching reconnectSelectReplay's guard. * fix(plugin): 1 defect(s) (OC-0338) * fix(client): 1 defect(s) (OC-0328) * docs(b2-8): ledger records, plan evidence and count claims for the nine fixes - Ledger: OC-0311/0315/0316/0317/0318/0322/0328/0337/0338 -> fixed, each with commit, pinning test and revertProof: pass (verify-fixes.mjs 8/8, plus a hand RED/GREEN of the wazero-tagged OC-0318 parity test) - Plan: B2-8 evidence block and status line (B2-0, B2-1, B2-8 landed; B2-2 next) - Count claims in README, b0-baseline, hp-0-scorecard and the issue register follow the ledger (315 fixed / 30 open / 3 declined / 1 duplicate = 349) * fix(ws): replay a complete cap-sized voice window instead of skipping it (OC-0337 follow-up) Codex review on #1436: liveVoiceEventsSince decided truncation by len(persisted) >= coldCap, so a complete window of exactly coldCap rows was treated as truncated and the supplement returned nil. Fetch coldCap+1 rows and discard only when the extra row exists. Test-first: the exact-cap case fails before the change and passes after; the over-cap case still degrades to nil.
This commit is contained in:
J3vb
@@ -1,5 +1,5 @@
|
||||
{
|
||||
"nextId": 349,
|
||||
"nextId": 350,
|
||||
"findings": [
|
||||
{
|
||||
"id": "OC-0001",
|
||||
@@ -7335,13 +7335,19 @@
|
||||
"why": "The VOICE_LEAVE handler has payload.channel_id in hand (it uses it two lines above for shouldTeardownSession) but calls handleParticipantLeft(payload.user_id) with no channel. E2EEManager.handleParticipantLeft then unconditionally deletes that user from _peerPublicKeys/_peerOfferEpochs, clears their verification badge, retires their key, and re-runs the key-holder election against the client's OWN voice channel — even though the leave was for a different channel entirely. voice_leave is broadcast to channelReadAudience(thatChannel), i.e. every client with READ_MESSAGES on it, not just the room's participants.",
|
||||
"repro": "I am in voice channel A and hold the room key; I also have READ_MESSAGES on voice channel B. Peer P (in B) switches to A. Server order: voice_leave(B,P) and voice_state(A,P) are enqueued on the buffered h.broadcast queue by P's voice_join; P's voice_token is sent directly, so P connects and its voice_e2ee_announce is relayed to me via pubsub.Publish. With the broadcast goroutine backed up, my socket sees: (1) announce(P,K) -> I verify P, store K, offer P the current room key; (2) voice_leave(B,P) -> handleParticipantLeft(P) with no channel filter: hadPeerKey=true, P deleted from _peerPublicKeys, clearPeerVerification(P) wipes the verified badge, and because A's roster does not list P yet, retirePeerKey(P,K) retires P's LIVE key; then the `wasKeyHolder && hadPeerKey` branch rotates the room key excluding P; (3) voice_state(A,P) -> P appears in my voice widget. Result: P is visibly in my call but holds a superseded key — nothing I send decrypts for them and nothing they send decrypts for me. Nothing heals it: mid-call peers never re-announce, my 5-minute rotation iterates _peerPublicKeys (P is gone), and any later replay of P's stored key (e.g. sendVoicePeerKeys on my WS reconnect, hub.go:664-666) is rejected by the retirement guard at livekitE2EE.ts:782. Passing payload.channel_id and ignoring leaves for other channels fixes it; the ready-resync call site at dispatcher.ts:374-384 already scopes by channel.",
|
||||
"evidence": "dispatcher.ts:1071-1079:\n const shouldTeardownSession =\n isSelf && voiceStore.getState().currentChannelId === payload.channel_id;\n void livekitSession().then(({ handleParticipantLeft, leaveVoice }) => {\n void handleParticipantLeft(payload.user_id); // <- payload.channel_id dropped\n\nlivekitE2EE.ts:1246-1253 (no channel parameter; acts on this._channelId):\n async handleParticipantLeft(userId: number): Promise<void> {\n const departingKey = this._peerPublicKeys.get(userId);\n const hadPeerKey = departingKey !== undefined;\n this._peerPublicKeys.delete(userId);\n this._peerOfferEpochs.delete(userId);\n clearPeerVerification(userId);\n const channelId = this._channelId ?? this.deps.getCurrentChannelId();\n\nAudience proof, hub_broadcast.go:126-143: broadcastVoiceEventWithLeaver resolves h.channelReadAudience(ctx, channelID) — everyone with READ on the channel, regardless of voice membership.\n\nReordering proof (already documented in-tree): voice_leave goes through the async hub queue (hub_broadcast.go:160-168 `h.broadcast <- bm`), while voice_e2ee_announce is published straight into the recipient's send queue from the announcer's read pump (voice_e2ee.go:270-272 `h.pubsub.Publish(VoiceTopic(channelID), ...)`) — the same hazard livekitE2EE.ts:1268-1272 cites for OC-0213.",
|
||||
"status": "open",
|
||||
"status": "fixed",
|
||||
"found": "2026-08-22",
|
||||
"hunt": "general-2026-08-22-b",
|
||||
"lens": "voice-e2ee",
|
||||
"suggestedFix": "Scope the E2EE notification to this client's own voice channel, mirroring removeVoiceUser and the shouldTeardownSession comparison already computed in the same handler. In dispatcher.ts, reuse the pre-leaveVoiceChannel store read: `const sameChannel = voiceStore.getState().currentChannelId === payload.channel_id;` (the value shouldTeardownSession already derives at :1071-1072), then at :1077 call it only when it matches — `if (sameChannel) void handleParticipantLeft(payload.user_id);` — leaving `if (shouldTeardownSession) void leaveVoice(false);` untouched. This keeps the one-argument call shape that dispatcher.test.ts:2728 asserts, and needs no change in livekitE2EE.ts. (Threading payload.channel_id into handleParticipantLeft and early-returning on mismatch against `this._channelId ?? this.deps.getCurrentChannelId()` is the alternative single-guard form, but it breaks that arity assertion and would require updating it to toHaveBeenCalledWith(7, 3).) The other caller, the ready-resync at dispatcher.ts:374-384, is already channel-scoped and unaffected.",
|
||||
"confidence": "medium",
|
||||
"finder": "opus"
|
||||
"finder": "opus",
|
||||
"fixed": "2026-08-28",
|
||||
"fix": {
|
||||
"commit": "a231108f",
|
||||
"test": "Client/tests/unit/dispatcher.test.ts",
|
||||
"revertProof": "pass"
|
||||
}
|
||||
},
|
||||
{
|
||||
"id": "OC-0312",
|
||||
@@ -7403,13 +7409,19 @@
|
||||
"why": "`payload.timestamp` is the raw SQLite `datetime('now')` string (\"2026-08-22 12:00:01\" — UTC, no zone designator; Server/migrations/001_initial_schema.sql:81, passed through verbatim by service/message_crud.go:79). `Date.parse` treats it as LOCAL time, so the parsed epoch is off by the viewer's UTC offset. The codebase already has `parseTimestamp()` (components/message-list/formatting.ts:25-33) that exists solely to append the missing \"Z\"; this comparison bypasses it. The bias cancels once `serverClockSkewMs` has been sampled (line 765 uses the same biased parse), but it is 0 until the first accepted live message — so the very first reconnect of a session is decided by the viewer's timezone instead of by the timestamp.",
|
||||
"repro": "Cold-skew case (serverClockSkewMs still 0 — no chat_message received since login, i.e. a quiet channel).\nEast of UTC, e.g. viewer at UTC+2: socket blips and reconnects at wall time H; 1 s later a peer posts a genuinely LIVE message. Date.parse(ts) = T - 2h, so `T - 2h < H - 0` is true → isReplayFrame = true → notifyIncomingMessage is skipped (no desktop notification, no sound, no taskbar flash). Worse, line 700-702 computes `isMention = ... && !(mentions_here && isReplayFrame)`, so a live `@here` that names the viewer raises no mention badge at all — and the reconnect tier sends no follow-up `ready` to correct it (OC-0271), so the badge is lost permanently.\nWest of UTC, e.g. viewer at UTC-5: Date.parse(ts) = T + 5h, so the test is false for every frame → the entire replayed burst is classified live and fires one desktop notification + sound per already-seen message, which is exactly what the gate was added to prevent.\nNot caught by tests: every timestamp in tests/unit/dispatcher.test.ts (lines 515, 541, 597, 622, 672, 779, 810…) is an ISO `Z` string, a form the server never emits.",
|
||||
"evidence": "685: const isReplayFrame =\n686: lastReconnectHandshakeAt !== null &&\n687: Date.now() - lastReconnectHandshakeAt < REPLAY_GATE_WINDOW_MS &&\n688: Date.parse(payload.timestamp) < lastReconnectHandshakeAt - serverClockSkewMs;\n...\n765: serverClockSkewMs = Date.now() - Date.parse(payload.timestamp);\n\n(the helper that exists for exactly this, formatting.ts:31-34:)\n const date = !raw.endsWith(\"Z\") && !raw.includes(\"+\") && !/T\\d{2}:\\d{2}:\\d{2}[+-]/.test(raw)\n ? new Date(raw.replace(\" \", \"T\") + \"Z\") : new Date(raw);",
|
||||
"status": "open",
|
||||
"status": "fixed",
|
||||
"found": "2026-08-22",
|
||||
"hunt": "general-2026-08-22-b",
|
||||
"lens": "ordering-boundary",
|
||||
"suggestedFix": "Route both parses through the existing UTC-normalizing helper instead of bare Date.parse — e.g. import { parseTimestamp } from \"@components/message-list/formatting\" (or lift it into @lib) and use `parseTimestamp(payload.timestamp).getTime()` at dispatcher.ts:688 and :765. One shared helper at both sites, no per-caller guards.",
|
||||
"confidence": "high",
|
||||
"finder": "opus"
|
||||
"finder": "opus",
|
||||
"fixed": "2026-08-28",
|
||||
"fix": {
|
||||
"commit": "a231108f",
|
||||
"test": "Client/tests/unit/dispatcher.test.ts",
|
||||
"revertProof": "pass"
|
||||
}
|
||||
},
|
||||
{
|
||||
"id": "OC-0316",
|
||||
@@ -7420,13 +7432,19 @@
|
||||
"why": "registerNow's resume-time E2EE resync (OC-0276) only pushes other participants' stored ECDH public keys *to* the resuming client. The room key itself travels the other way, as a targeted unsequenced voice_e2ee_offer, and one sent while the socket was down is dropped outright. Nothing on either side re-runs the exchange after the resume: the server never re-offers, and the client's only re-announce paths (setupKeyExchange, reannounceForReconnect) are both driven by the LiveKit room, not by the WebSocket, so a pure WS blip leaves a non-key-holder holding the pre-rotation key with no signal and no retry.",
|
||||
"repro": "Users A (lower user id, key holder) and B are in a voice call; both LiveKit sessions are healthy. B's WebSocket drops (WiFi blip / proxy restart) but its LiveKit room stays up — nothing tears voice down on a socket drop alone (dispatcher.ts READY comment, livekitSession.ts). The server has not yet observed B's TCP close, so B's old *Client is still in h.clients. While B is offline a third participant C leaves (or A's 5-minute KEY_ROTATION_INTERVAL_MS timer fires, livekitE2EE.ts:109): A rotates the room key and sends a voice_e2ee_offer for B. sendToUserIfInVoiceChannel queues it onto B's dead client and it is lost. B reconnects with last_seq > 0; handleReconnect replays the sequenced voice_state/voice_leave frames, registerNow transfers B's voice state and calls sendVoicePeerKeys — so B's roster and peer-key map are correct — but B's keyProvider still holds the pre-rotation key. From that moment A and B cannot decrypt each other's frames: both hear silence while VoiceWidget still shows \"Secured\", and the only recovery is A's next 5-minute periodic rotation.",
|
||||
"evidence": "Server/ws/hub.go:664-666 (registerNow tail):\n if voiceChID := c.getVoiceChID(); voiceChID != 0 {\n h.sendVoicePeerKeys(c, voiceChID)\n }\nsendVoicePeerKeys (Server/ws/voice_e2ee.go:344-350) only sends buildVoiceE2EEAnnounce(uid, pubKey, sig) for every *other* participant — no room-key material.\n\nThe offer path drops silently while the socket is down (Server/ws/voice_e2ee.go:239-259):\n target, ok := h.clients[targetUserID]\n if !ok { slog.Debug(\"e2ee: key offer dropped, target not connected\", ...); return }\n ...\n target.sendMsg(msg)\n(and while the dead old *Client is still registered, sendMsg queues into a send buffer that registerNow's old.closeSend() then discards).\n\nClient side, the only two re-announce entry points are LiveKit-driven:\n Client/src/lib/livekitE2EE.ts:159 setupKeyExchange <- called only from livekitSession.ts:1103 (connectAndSetup)\n Client/src/lib/livekitE2EE.ts:346 reannounceForReconnect <- called only from livekitSession.ts:564 (attemptAutoReconnect)\nNeither is reachable from a WS resume: dispatcher.ts's AUTH_OK handler (lines 270-290) does exactly setAuth() + one channel_focus send, and the READY handler's E2EE work (OC-0201, dispatcher.ts:360-384) runs only on the full-resync tier, which a successful replay resume never takes.\n\nreannounceForReconnect's own comment states the assumption that is unmet here: \"the key holder will send a fresh offer if the key was rotated during our absence\" (livekitE2EE.ts:343-344) — true only because that path re-announces; the WS-resume path does not.",
|
||||
"status": "open",
|
||||
"status": "fixed",
|
||||
"found": "2026-08-22",
|
||||
"hunt": "general-2026-08-22-b",
|
||||
"lens": "flow-reconnect",
|
||||
"suggestedFix": "One server-side addition at registerNow's resync call site (Server/ws/hub.go:664-666) — do not put it inside sendVoicePeerKeys, since voice_join.go:531 shares that function and the joiner's own announce already comes from its client there:\n\n if voiceChID := c.getVoiceChID(); voiceChID != 0 {\n h.sendVoicePeerKeys(c, voiceChID)\n // Re-relay THIS client's own stored key back onto VoiceTopic so the\n // key holder's duplicate-announce branch re-wraps the CURRENT room\n // key for us — a rotation offer sent while this socket was down was\n // dropped and no replay tier can recover it.\n if key, sig := c.getE2EEPubKey(); key != \"\" {\n h.sendToVoiceChannelExcept(voiceChID, c.userID,\n buildVoiceE2EEAnnounce(c.userID, key, sig))\n }\n }\n\nThis needs no client change: handleAnnounceInner's dedup branch (livekitE2EE.ts:~812, \"duplicate announce — will re-send offer if key holder\") deliberately falls through to the wrap-and-offer branch on an identical key, so the holder re-offers the live room key. The announce is not blocked by _retiredPeerKeys (that set holds only keys a peer has been moved OFF of, never the live one) and re-runs verifyPeerAnnounce exactly as reannounceForReconnect's announce already does. Guard it on c.lastSeq > 0 if you want it strictly on the resume path.",
|
||||
"confidence": "high",
|
||||
"finder": "opus"
|
||||
"finder": "opus",
|
||||
"fixed": "2026-08-28",
|
||||
"fix": {
|
||||
"commit": "cd4cc850",
|
||||
"test": "Server/ws/oc_0316_voice_e2ee_resume_rotation_test.go",
|
||||
"revertProof": "pass"
|
||||
}
|
||||
},
|
||||
{
|
||||
"id": "OC-0317",
|
||||
@@ -7437,13 +7455,19 @@
|
||||
"why": "The replay branch suppresses the unread/mention increment but still writes `lastMessageId: messageId` unconditionally. When the redelivered id is *lower* than the stored watermark, the watermark is rolled backwards, so the very next frame in the same replay burst no longer looks like a replay and is counted as new. Its sibling `updateDmLastMessagePreview` (lines 186-190) documents this exact hazard (OC-0301) and returns `prev` instead — `updateDmLastMessage` never got the same treatment.",
|
||||
"repro": "DM channel 5. Two messages (ids 101 then 102) are delivered by the server in the registerNow→buildReady window, so `ready` lands with unreadCount=2 / lastMessageId=102, and both frames are then drained from the queue as `chat_message` (dispatcher.ts:740 calls updateDmLastMessage for each, since the DM is neither own-message nor active).\n1. frame 101: isReplay = (101 <= 102) = true → unreadCount stays 2, but lastMessageId is overwritten with 101.\n2. frame 102: isReplay = (102 <= 101) = false → unreadCount = 3, and mentionCount = +1 if the message mentioned the reader.\nThe DM sidebar badge shows 3 unread (and a phantom mention) for 2 messages, and it survives until the next full `ready`. Nothing in tests/unit/dm-store.test.ts asserts lastMessageId after a stale call, so the behavior is not locked.",
|
||||
"evidence": "const isReplay = updated.lastMessageId !== null && messageId <= updated.lastMessageId;\nreturn { channels: [ { ...updated,\n lastMessageId: messageId, // <-- regresses the watermark on a replay\n lastMessage: content,\n lastMessageAt: timestamp,\n unreadCount: isReplay ? updated.unreadCount : updated.unreadCount + 1,\n mentionCount: isMention && !isReplay ? updated.mentionCount + 1 : updated.mentionCount,\n}, ...rest ] };",
|
||||
"status": "open",
|
||||
"status": "fixed",
|
||||
"found": "2026-08-22",
|
||||
"hunt": "general-2026-08-22-b",
|
||||
"lens": "hotspot-client-tauri-client-src-components",
|
||||
"suggestedFix": "Mirror the sibling: in updateDmLastMessage's setState, replace the isReplay ternaries with an early `if (isReplay) return prev;` right after the isReplay computation (dm.store.ts:149). One guard in the shared function; the equal-id case is the same message ready already previewed, so nothing visible is lost.",
|
||||
"confidence": "high",
|
||||
"finder": "opus"
|
||||
"finder": "opus",
|
||||
"fixed": "2026-08-28",
|
||||
"fix": {
|
||||
"commit": "7c159c11",
|
||||
"test": "Client/tests/unit/dm-store.test.ts",
|
||||
"revertProof": "pass"
|
||||
}
|
||||
},
|
||||
{
|
||||
"id": "OC-0318",
|
||||
@@ -7454,13 +7478,19 @@
|
||||
"why": "Two sources of truth for one plugin directory, read with opposite precedence by the two paths that consume it. `installZipStagedManifest` reads *only* `plugin.json` from the staged zip, and that is the manifest that is validated, shown to the admin, persisted to `plugins.manifest_json`, and used to activate the instance. `scanPluginDirectory` — the path that runs on every server start — prefers `plugin.toml` and only falls back to `plugin.json` when the TOML file is absent. A zip may contain both files (installZipExtract rejects only symlinks and path escapes, not extra regular files), so the manifest that governs the plugin after a restart is one that was never examined at install time. The manifest is the per-plugin ACL (manifest.go:64-67, errors.go:18-22: \"the manifest — not the guest module — is the authority ... so an admin can see the full command surface before enabling the plugin\"), so this defeats exactly the review it exists for.",
|
||||
"repro": "Server built with `-tags wazero` (the build where plugins actually execute and where TOML is parsed). Upload a zip through POST /api/v1/admin/plugins/install containing plugin.json with `\"permissions\": [\"commands\"]`, `\"commands\": [{\"name\":\"hello\"}]`, `\"entrypoint\":\"hello.wasm\"` — plus a plugin.toml at the same root declaring `permissions = [\"commands\",\"http\",\"storage\",\"ui\"]`, extra `[[commands]]` entries, and `entrypoint = \"other.wasm\"`. Install succeeds; installZipStagedManifest parses only the JSON, so the admin list, the stored manifest_json, and the immediately-activated instance all show the narrow JSON surface. Restart the server: LoadAll → scanPluginDirectory (loader.go:64) picks plugin.toml, installFromDisk upserts *that* manifest, and activateAll brings the plugin up with the broader capability set, the undeclared-at-review commands, and a different .wasm entrypoint — with no new admin action and no log line noting that the effective manifest changed. The same mechanism bites non-maliciously: an author who ships both files and later edits only plugin.json sees the stale TOML silently win after every restart while the freshly installed process used the JSON.",
|
||||
"evidence": "registry.go:421-427 (install path)\n\tmanifestPath := filepath.Join(stageAbs, \"plugin.json\")\n\traw, err := os.ReadFile(manifestPath)\n\tif err != nil { return nil, fmt.Errorf(\"plugin zip: missing plugin.json at root: %w\", err) }\n\tmanifest, err := ParseManifest(raw)\n\nloader.go:63-86 (load path)\n\t// Prefer plugin.toml (wazero build) over plugin.json.\n\tmanifest, ok, tomlErr := tryLoadPluginTOML(pluginDir)\n\t...\n\tif !ok { /* only now read plugin.json */ }\n\nregistry.go:286-296 — the staged tree (including any plugin.toml) is promoted verbatim into finalDir and registered with the JSON manifest.\n`grep -rn \"plugin.toml\" Server/ --include=*.go` matches only manifest_toml.go and the loader comment: nothing in the install path ever looks at it.",
|
||||
"status": "open",
|
||||
"status": "fixed",
|
||||
"found": "2026-08-22",
|
||||
"hunt": "general-2026-08-22-b",
|
||||
"lens": "explore-2",
|
||||
"suggestedFix": "Make both paths resolve the manifest through one function instead of guarding each caller. Extract loader.go:62-86's precedence into a shared helper and call it from the install path too:\n\n\t// loader.go\n\tfunc loadManifestFromDir(dir string) (*Manifest, error) {\n\t\tif m, ok, err := tryLoadPluginTOML(dir); err != nil {\n\t\t\treturn nil, err\n\t\t} else if ok {\n\t\t\treturn m, nil\n\t\t}\n\t\traw, err := os.ReadFile(filepath.Join(dir, \"plugin.json\"))\n\t\tif err != nil {\n\t\t\treturn nil, err\n\t\t}\n\t\treturn ParseManifest(raw)\n\t}\n\nThen replace registry.go:422-427 with `manifest, err := loadManifestFromDir(stageAbs)` (keeping the existing \"missing plugin.json at root\" wrapping for os.IsNotExist) and have scanPluginDirectory call the same helper. The manifest the admin's install validates is then byte-for-byte the one the next restart loads, in both build tags. If keeping JSON-only at install is preferred, the equally small alternative is to reject the ambiguity at the single install site — after extraction, `if _, err := os.Stat(filepath.Join(stageAbs, \"plugin.toml\")); err == nil { return nil, fmt.Errorf(\"plugin zip: must not contain both plugin.json and plugin.toml\") }` — but the shared-helper version also fixes the plain on-disk case where an author edits only one of the two files.",
|
||||
"confidence": "high",
|
||||
"finder": "opus"
|
||||
"finder": "opus",
|
||||
"fixed": "2026-08-28",
|
||||
"fix": {
|
||||
"commit": "bbbaeed4",
|
||||
"test": "Server/plugin/registry_test.go, Server/plugin/registry_zip_toml_wazero_test.go",
|
||||
"revertProof": "pass"
|
||||
}
|
||||
},
|
||||
{
|
||||
"id": "OC-0319",
|
||||
@@ -7522,13 +7552,19 @@
|
||||
"why": "The DNS-name branch uses `[\\w.-]+`, and JS `\\w` is `[A-Za-z0-9_]` — so a host containing `_` passes. Both `http_proxy::validate_remote_host` and `livekit_proxy::validate_remote_host` allow only `is_ascii_alphanumeric() || '.' | '-' | ':' | '[' | ']'` and reject `_`. Since every REST call routes through `ensureHttpProxy` (api.ts:88), an underscore host is accepted by the Add Server modal and by `api.setConfig`, then fails 100% of REST traffic. The file's own header comment and ServerPanel.ts:310-313 both state the invariant that this validator mirrors the Rust one (\"an address accepted here is also accepted by the actual connection path, and vice versa\").",
|
||||
"repro": "Connect page -> \"Add Server\" -> address `my_server.lan:8443`. ServerPanel.ts:314 `isValidHost(addr)` returns true (JS `\\w` matches `_`), so the profile is saved. The connect page then health-checks it: `api.getHealth(\"my_server.lan:8443\")` -> `ensureHttpProxy(host)` -> `invoke(\"start_http_proxy\", {remoteHost})` -> http_proxy.rs:101 `validate_remote_host` -> Err(\"remote_host contains unexpected characters\"). Every REST call fails identically, so login is impossible and the profile shows permanently unreachable; `start_livekit_proxy` rejects the same host, so voice is dead too. The WS proxy has no charset check, so `wss://my_server.lan/api/v1/ws` would have connected — the client accepts an address that only one of its three transports can use.",
|
||||
"evidence": "hostValidation.ts:33 return /^[\\w.-]+(:\\d+)?$/.test(host); // \\w includes '_'\n\nhttp_proxy.rs:83-88\n if !remote_host\n .chars()\n .all(|c| c.is_ascii_alphanumeric() || matches!(c, '.' | '-' | ':' | '[' | ']'))\n {\n return Err(\"remote_host contains unexpected characters\".into());\n }\n\nlivekit_proxy.rs:110-116 (identical charset, same rejection)\n\napi.ts:87-89\n async function baseUrl(): Promise<string> {\n return `${await ensureHttpProxy(config.host)}/api/v1`;\n }\n\ncommands.rs tests pin the Rust side as deliberate:\n (\"underscore\", \"chat_example.com\".into()), // expected to be rejected\n\ntests/unit/host-validation.test.ts has no underscore case, so nothing locks the TS behavior.",
|
||||
"status": "open",
|
||||
"status": "fixed",
|
||||
"found": "2026-08-22",
|
||||
"hunt": "general-2026-08-22-b",
|
||||
"lens": "tauri-rust",
|
||||
"suggestedFix": "One character-class change in the shared validator, hostValidation.ts:33: replace `\\w` with an explicit ASCII class so the DNS branch matches the Rust charset — `return /^[A-Za-z0-9.-]+(:\\d+)?$/.test(host);`. Add an underscore rejection case to tests/unit/host-validation.test.ts mirroring commands.rs:364 so the two validators stay pinned together.",
|
||||
"confidence": "high",
|
||||
"finder": "opus"
|
||||
"finder": "opus",
|
||||
"fixed": "2026-08-28",
|
||||
"fix": {
|
||||
"commit": "e95c57a4",
|
||||
"test": "Client/tests/unit/host-validation.test.ts",
|
||||
"revertProof": "pass"
|
||||
}
|
||||
},
|
||||
{
|
||||
"id": "OC-0323",
|
||||
@@ -7624,13 +7660,19 @@
|
||||
"why": "incrementUnread/incrementMention bump unconditionally. `Channel.lastMessageId` is declared (line 30) and filled from `ready`'s `last_message_id` (line 100), but no call site anywhere in the client reads it — the identical registerNow->buildReady double-delivery window that OC-0242 fixed for DMs (dm.store.ts) is unguarded for server channels.",
|
||||
"repro": "A message is broadcast into channel #general between registerNow (Server/ws/serve.go:853, which subscribes the socket) and buildReady (serve.go:884) on a fresh connect or a full resync. The server counts it in read_states.unread_count, so `ready` carries unread_count = 1 and last_message_id = <that id>; `ready` is written straight to the connection by handshakeWrite while the broadcast waits in the client's send queue. setChannels applies unreadCount = 1, then writePump drains the queued chat_message and dispatcher.ts:711 calls incrementUnread -> the sidebar shows 2 unread for 1 message, and an @mention in it shows a mention count of 2. dm.store.ts:148 guards this exact case for DMs with `messageId <= updated.lastMessageId`; the channel path has no equivalent.",
|
||||
"evidence": "channels.store.ts:346-363\n export function incrementUnread(channelId: number, evenIfActive = false): void {\n channelsStore.setState((prev) => {\n if (prev.activeChannelId === channelId && !evenIfActive) return prev;\n const existing = prev.channels.get(channelId);\n if (existing === undefined) return prev;\n const updated: Channel = { ...existing, unreadCount: existing.unreadCount + 1 };\n ...\n\nchannels.store.ts:30 / :100 — the watermark is stored and never consulted\n readonly lastMessageId: number | null;\n lastMessageId: ch.last_message_id ?? null,\n\n`grep -rn lastMessageId` over src/ shows the only readers are dm.store.ts and SidebarDmHelpers.ts — nothing reads Channel.lastMessageId.\n\ncaller: dispatcher.ts:706-715\n if ((payload.channel_id !== activeId || isDetached) && !isOwnMessage) {\n incrementUnread(payload.channel_id, isDetached);\n if (isMention) incrementMention(payload.channel_id, isDetached);\n }",
|
||||
"status": "open",
|
||||
"status": "fixed",
|
||||
"found": "2026-08-22",
|
||||
"hunt": "general-2026-08-22-b",
|
||||
"lens": "hotspot-client-tauri-client-src-lib",
|
||||
"suggestedFix": "Mirror the DM shape in the one shared store function rather than at the call site: give channels.store a single guarded entry point, e.g. `noteChannelMessage(channelId, messageId, isMention, evenIfActive)`, whose setState computes `const isReplay = existing.lastMessageId !== null && messageId <= existing.lastMessageId;` and writes `unreadCount: isReplay ? existing.unreadCount : existing.unreadCount + 1`, `mentionCount: isMention && !isReplay ? existing.mentionCount + 1 : existing.mentionCount`, and `lastMessageId: Math.max(messageId, existing.lastMessageId ?? 0)` — both counters behind ONE watermark read, exactly as OC-0242 required for updateDmLastMessage (a guard split across the two functions cannot work: the first call would already have advanced the watermark). Then replace the pair at dispatcher.ts:710/713 with the single call; it is the only production caller of incrementUnread/incrementMention, so the existing exports can stay for the tests.",
|
||||
"confidence": "medium",
|
||||
"finder": "opus"
|
||||
"finder": "opus",
|
||||
"fixed": "2026-08-28",
|
||||
"fix": {
|
||||
"commit": "3e74c968",
|
||||
"test": "Client/tests/unit/channels.store.test.ts, Client/tests/unit/dispatcher.test.ts",
|
||||
"revertProof": "pass"
|
||||
}
|
||||
},
|
||||
{
|
||||
"id": "OC-0329",
|
||||
@@ -7777,13 +7819,19 @@
|
||||
"why": "The cold-tier query is `ORDER BY seq ASC LIMIT n`, so when the range exceeds the cap it is the newest rows that are discarded. The main replay path 220 lines above (reconnectSelectReplay, serve.go:422 and serve.go:442) explicitly detects both of that query's failure modes — `len(persisted) >= coldCap` (cap hit, newest dropped) and a retention-pruned prefix (oldest-seq probe) — and forces a full ready. liveVoiceEventsSince calls the identical db.GetEventsSinceForChannels with the identical cap and has neither guard, so a truncated window is handed to the client as if it were the complete voice history for that room. Worse, the cap is spent on UNFILTERED rows: the query is `channel_id = 0 OR channel_id IN (chID)`, so global broadcasts and the DM's ordinary chat messages consume the budget, and the voice_state/voice_leave filter at serve.go:655 only runs on whatever survived. A peer's voice_leave that falls in the dropped tail is never delivered and never re-sent (the client tracks only max(seq)), so the resumed client renders a participant who has left; symmetrically, a dropped voice_state hides a peer who is really in the call, which also starves that peer of the E2EE announce/offer exchange keyed on the roster.",
|
||||
"repro": "Config: event_persistence.enabled = true, event_persistence.replay_cold_limit = 50 (a legal value; ConfigureReplay accepts any positive int, Server/ws/hub.go:751). Alice and Bob are both in a voice call inside a 1:1 DM that Alice has since closed, so the DM id is outside Alice's allowedChannelIDs (computeAllowedChannels sources DM ids from dm_open_state) and handleReconnect takes the liveVoiceChID supplement branch at serve.go:286. Alice's socket drops. While she is offline: (1) Bob posts 60 messages into that DM — each is a persisted event on that channel_id — and then (2) Bob leaves voice, emitting voice_leave. Alice's readable channels stay quiet, so the main cold-tier replay at serve.go:417 returns well under 50 rows and succeeds (tier \"db\"), and the ring buffer no longer covers her last_seq. liveVoiceEventsSince then runs GetEventsSinceForChannels(lastSeq, [dmID], 50), which returns the OLDEST 50 rows — the first 50 chat messages — and drops the remaining 10 rows including Bob's voice_leave. Alice's client resumes with Bob still listed in the voice roster and never receives a correction; the same window would equally have swallowed a voice_state for a peer who joined late, leaving that peer invisible to her for the rest of the call.",
|
||||
"evidence": "// Server/ws/serve.go:637-649 (liveVoiceEventsSince)\nif buf := h.ReplayBuffer().EventsSinceFiltered(afterSeq, only); buf != nil {\n\traw = buf\n} else if esp := h.eventStore.Load(); esp != nil {\n\tes := *esp\n\tpersisted, err := es.GetEventsSinceForChannels(ctx, int64(afterSeq), []int64{chID}, h.maxColdReplayLimit())\n\tif err != nil {\n\t\treturn nil\n\t}\n\traw = make([][]byte, 0, len(persisted))\n\tfor _, p := range persisted {\n\t\traw = append(raw, p.Payload)\n\t}\n}\n// no `len(persisted) >= coldCap` check, no oldest-seq retention probe — compare\n// Server/ws/serve.go:422-453, which has both for the same query:\n// case len(persisted) >= coldCap: \"...the NEWEST events were dropped...forcing full ready\"\n// case len(oldest) == 0 || uint64(oldest[0].Seq) > lastSeq+1: \"retention pruning left a gap...forcing full ready\"\n//\n// Server/db/event_queries.go:136-144 — the cap is applied before any type filter:\n// WHERE seq > ? AND (channel_id = 0 OR channel_id IN (...)) ORDER BY seq ASC LIMIT ?\n// Server/ws/serve.go:653-659 — voice filtering happens only on the truncated result.",
|
||||
"status": "open",
|
||||
"status": "fixed",
|
||||
"found": "2026-08-22",
|
||||
"hunt": "general-2026-08-22-b",
|
||||
"lens": "explore-1",
|
||||
"suggestedFix": "Stop spending the cap on non-voice rows and stop replaying a truncated window. Smallest change: give this call its own store method that applies the type filter in SQL — `WHERE seq > ? AND channel_id = ? AND event_type IN ('voice_state','voice_leave') ORDER BY seq ASC LIMIT ?` — so chat and global broadcasts can no longer evict voice events from the budget, and in liveVoiceEventsSince add the sibling's cap check: `if len(persisted) >= cap { slog.Warn(\"live voice supplement hit the row cap, skipping truncated window\"); return nil }`. Returning nil is the correct degradation here (a full ready is no longer available — registerNow already ran at serve.go:268 before the supplement at serve.go:287), and it restores the documented best-effort miss instead of installing a join whose matching leave was discarded. Do not simply raise the limit: that leaves the same silent-truncation hole one order of magnitude further out.",
|
||||
"confidence": "medium",
|
||||
"finder": "opus"
|
||||
"finder": "opus",
|
||||
"fixed": "2026-08-28",
|
||||
"fix": {
|
||||
"commit": "7aeab0ed",
|
||||
"test": "Server/ws/reconnect_voice_supplement_coldtier_test.go",
|
||||
"revertProof": "pass"
|
||||
}
|
||||
},
|
||||
{
|
||||
"id": "OC-0338",
|
||||
@@ -7794,13 +7842,19 @@
|
||||
"why": "`toml.Decode` resolves a TOML key to a struct field via the `toml` struct tag, or, when absent, the Go field name matched with `strings.EqualFold`. `Manifest`/`Resources` declare only `json` tags, so `max_memory_mb` and `cpu_budget_ms` never match `MaxMemoryMB` / `CPUBudgetMs` (underscores break EqualFold) and are left undecoded with no error. Every other manifest key happens to be a single word (`name`, `version`, `entrypoint`, `permissions`, `commands`, `ui`, `asset`, …) and case-folds fine, which is why the breakage is invisible — only the two snake_case resource keys are silently discarded, and `Validate()` only checks `>= 0`, so zero passes.",
|
||||
"repro": "Build with `-tags wazero` (the only build where plugin.toml is parsed at all — manifest_nottoml.go:9 stubs it out). Ship `plugins/foo/plugin.toml`:\n\n name = \"foo\"\n version = \"1.0.0\"\n entrypoint = \"foo.wasm\"\n permissions = [\"commands\"]\n [[commands]]\n name = \"foo\"\n [resources]\n cpu_budget_ms = 2000\n max_memory_mb = 128\n\nscanPluginDirectory (loader.go:64) loads it via tryLoadPluginTOML; `Manifest.Resources` is `{0, 0}`. Invoke `/foo`: sandbox_wazero.go:317 falls through to `r.cfg.CPUBudgetMs` (config default 100), so a command the author budgeted 2000 ms for is killed at 100 ms with \"command exceeded CPU budget of 100ms\". The byte-identical plugin.json (`\"resources\": {\"cpu_budget_ms\": 2000}`) behaves correctly, so the same plugin works as JSON and misbehaves as TOML. installFromDisk then serializes the zeroed Resources back into `plugins.manifest_json` (loader.go:132-138, registry.go:192-196), so the admin plugin list also reports a budget the author never wrote. No test covers TOML decoding (`grep -rn toml Server/plugin/*_test.go` is empty), so nothing locks this in as intended.",
|
||||
"evidence": "manifest_toml.go:31-38\n\tvar m Manifest\n\tif _, err := toml.Decode(string(raw), &m); err != nil { ... }\n\tif err := m.Validate(); err != nil { ... }\n\nmanifest.go:77-80\ntype Resources struct {\n\tMaxMemoryMB int `json:\"max_memory_mb\"`\n\tCPUBudgetMs int `json:\"cpu_budget_ms\"`\n}\n\ntoml@v1.6.0/decode.go:311-318 — `if ff.name == key { ... }` else `if f == nil && strings.EqualFold(ff.name, key) { f = ff }`\ntoml@v1.6.0/type_fields.go:108-113 — `name := opts.name; if name == \"\" { name = sf.Name }`, where `opts` comes from `tag.Get(\"toml\")` (encode.go:647-648).\n\nConsumer: sandbox_wazero.go:317-323\n\tbudgetMs := inst.Manifest.Resources.CPUBudgetMs\n\tif budgetMs <= 0 { budgetMs = r.cfg.CPUBudgetMs }\n\tif budgetMs <= 0 { budgetMs = 100 }",
|
||||
"status": "open",
|
||||
"status": "fixed",
|
||||
"found": "2026-08-22",
|
||||
"hunt": "general-2026-08-22-b",
|
||||
"lens": "explore-2",
|
||||
"suggestedFix": "Add toml tags to the two snake_case fields in Server/plugin/manifest.go:77-80:\n\ttype Resources struct {\n\t\tMaxMemoryMB int `json:\"max_memory_mb\" toml:\"max_memory_mb\"`\n\t\tCPUBudgetMs int `json:\"cpu_budget_ms\" toml:\"cpu_budget_ms\"`\n\t}\nThat is the minimal fix and is safe for the JSON path (encoding/json ignores the toml tag). Optionally harden the shared decode site instead of every future field: in tryLoadPluginTOML (manifest_toml.go:31) keep the MetaData and reject leftovers — `md, err := toml.Decode(...)`; `if u := md.Undecoded(); len(u) > 0 { return nil, false, fmt.Errorf(\"plugin.toml: unknown keys %v\", u) }` — which turns any future tag/name mismatch or manifest typo into a loud load error rather than a silent zero.",
|
||||
"confidence": "high",
|
||||
"finder": "opus"
|
||||
"finder": "opus",
|
||||
"fixed": "2026-08-28",
|
||||
"fix": {
|
||||
"commit": "073e8799",
|
||||
"test": "Server/plugin/manifest_test.go",
|
||||
"revertProof": "pass"
|
||||
}
|
||||
},
|
||||
{
|
||||
"id": "OC-0339",
|
||||
@@ -7971,6 +8025,23 @@
|
||||
"suggestedFix": "Tighten the single shared selector rather than its caller — in members.store.ts getOnlineMembers, change the predicate to `if (member.status !== \"offline\" && member.status !== \"invisible\")`, matching MemberList's isAwayStatus.",
|
||||
"confidence": "high",
|
||||
"finder": "opus"
|
||||
},
|
||||
{
|
||||
"id": "OC-0349",
|
||||
"title": "voice_join relays the joiner’s own voice_state through the asynchronous hub queue while the rest of the join burst is written directly, so its position on the joiner’s socket is unordered",
|
||||
"file": "Server/ws/voice_join.go",
|
||||
"line": 498,
|
||||
"severity": "low",
|
||||
"why": "voiceJoinComplete writes voice_token (voice_join.go:445), each existing participant’s voice_state (:523), the peers’ voice_e2ee_announce relays (sendVoicePeerKeys, :531) and voice_config (:546) straight into the joiner’s send queue with c.sendMsg, but the joiner’s OWN voice_state goes through h.broadcastVoiceEvent (:498), which hub_broadcast.go:95-168 enqueues on the buffered h.broadcast channel for the broadcast goroutine to fan out later. The joiner is part of that audience (channelReadAudience), so nothing orders its own sequenced voice_state against the four direct frames: it can land before voice_token, between the existing participants’ states, or after voice_config, depending on how backed up the broadcast goroutine is. The B2-1 fixture capture could not pin the join burst and had to document the order as unspecified (docs/protocol.md:927-929) and exclude that one frame from the epoch-1 transcript’s ordered comparison (protocol_epoch1_contract_test.go:69-75, 636-648). A client that treats the reply as an ordered burst — e.g. takes its own voice_state as the signal that the roster before it is complete, or that the join finished before voice_config — reads a state that is right most of the time and wrong under load.",
|
||||
"repro": "cd Server && go test -tags deadlock ./ws -run TestEpoch1Fixtures -count=30 at 1fe3df79 with the voice-join journey’s own-voice_state exclusion removed (protocol_epoch1_contract_test.go:636-648): roughly 1 run in 30 records the joiner’s own voice_state after the existing participants’ states or after voice_config instead of directly after voice_token; the default build reorders less often but is not immune. Equivalently, keep the broadcast goroutine busy (a burst of chat_send into another channel from a second client) while a client sends voice_join and watch the joiner’s own voice_state trail voice_config on its socket.",
|
||||
"evidence": "Server/ws/voice_join.go:445 c.sendMsg(buildVoiceToken(channelID, token, \"/livekit\", h.livekit.URL(), isKeyHolder)) // direct\nServer/ws/voice_join.go:498 h.broadcastVoiceEvent(ctx, channelID, buildVoiceState(*state)) // hub queue, joiner in audience\nServer/ws/voice_join.go:523 c.sendMsg(buildVoiceState(vs)) // direct, per existing participant\nServer/ws/voice_join.go:531 h.sendVoicePeerKeys(c, channelID) // direct\nServer/ws/voice_join.go:546 c.sendMsg(buildVoiceConfig(channelID, quality, bitrate, maxUsers)) // direct\nServer/ws/hub_broadcast.go:95-168 broadcastVoiceEvent -> h.broadcast <- bm (buffered channel; deliverBroadcast fans out on the hub goroutine)\ndocs/protocol.md:927-929 \"Items 1, 3 and 4 are written directly and keep that relative order; item 2 travels through the hub’s broadcast queue, so its position relative to the other three on the joiner’s own socket is not guaranteed.\"\nServer/ws/protocol_epoch1_contract_test.go:69-75, 636-648 the epoch-1 transcript deliberately records the joiner’s own voice_state out of the ordered comparison because under -tags deadlock it was observed arriving after the direct frames.",
|
||||
"status": "open",
|
||||
"found": "2026-08-28",
|
||||
"hunt": "b2-1-fixture-capture-2026-08-28",
|
||||
"lens": "ordering-boundary",
|
||||
"suggestedFix": "Write the joiner’s own voice_state to the joiner directly (c.sendMsg, in program order at :498, so the whole burst on the joiner’s socket is one goroutine’s program order) and broadcast it to everyone except the joiner — broadcastVoiceEventWithLeaver already carries an exclude-user path in hub_broadcast.go, so the fix is one call-site change plus a helper, not a new fan-out. Check the seq contract first: deliverBroadcast is where the sequenced copy is stamped and appended to the replay buffer, so the joiner’s direct copy must carry the same seq (or be documented as the unsequenced form like the relayed existing states) rather than double-stamping. Behaviour change within epoch 1 (same frame set, deterministic position): regenerate the epoch-1 fixture in the same PR (`go test ./ws -run TestEpoch1Fixtures -update`), drop the contract test’s exclusion so the order is asserted, and tighten docs/protocol.md:927-929.",
|
||||
"confidence": "high",
|
||||
"finder": "fable"
|
||||
}
|
||||
]
|
||||
}
|
||||
|
||||
@@ -13,8 +13,7 @@ import {
|
||||
addChannel,
|
||||
updateChannel,
|
||||
removeChannel,
|
||||
incrementUnread,
|
||||
incrementMention,
|
||||
noteChannelMessage,
|
||||
} from "@stores/channels.store";
|
||||
import { channelsStore } from "@stores/channels.store";
|
||||
import {
|
||||
@@ -75,6 +74,7 @@ import type { DmChannelPayload } from "./types";
|
||||
import { isTextLikeChannel } from "./types";
|
||||
import type { ApiClient } from "./api";
|
||||
import { invalidateReactionUsers } from "@components/message-list/reaction-tooltip";
|
||||
import { parseTimestamp } from "@components/message-list/formatting";
|
||||
import { notifyIncomingMessage } from "./notifications";
|
||||
import { mentionsCurrentUser } from "./mentions";
|
||||
import { ensureIdentityKeyPublished } from "@lib/identity";
|
||||
@@ -501,8 +501,8 @@ export function wireDispatcher(
|
||||
// offline keeps a phantom row here (closeDmLocally fixes this exact
|
||||
// shape for the live dm_channel_close path; this is its ready-time
|
||||
// equivalent), and a DM read elsewhere keeps a stale unread/mention
|
||||
// count (incrementUnread/incrementMention bump the mirror in parallel
|
||||
// with dmStore once it exists, but only dmStore is restated above).
|
||||
// count (noteChannelMessage bumps the mirror in parallel with dmStore
|
||||
// once it exists, but only dmStore is restated above).
|
||||
// Reconcile every dm-typed row against the just-restated payload.
|
||||
channelsStore.setState((prev) => {
|
||||
const dmById = new Map(dmPayloads.map((d) => [d.channel_id, d]));
|
||||
@@ -675,7 +675,7 @@ export function wireDispatcher(
|
||||
// ones — the burst is exactly the messages missed while away (a
|
||||
// full-ready resume sends no burst at all; ready's unread_count values
|
||||
// are authoritative there). DM channel IDs are not in channelsStore
|
||||
// (they use dmStore), so incrementUnread is a no-op for DMs, but the
|
||||
// (they use dmStore), so noteChannelMessage is a no-op for DMs, but the
|
||||
// own-message guard is applied here for defence-in-depth.
|
||||
//
|
||||
// isReplayFrame is computed here (rather than only below, where the
|
||||
@@ -684,7 +684,7 @@ export function wireDispatcher(
|
||||
const isReplayFrame =
|
||||
lastReconnectHandshakeAt !== null &&
|
||||
Date.now() - lastReconnectHandshakeAt < REPLAY_GATE_WINDOW_MS &&
|
||||
Date.parse(payload.timestamp) < lastReconnectHandshakeAt - serverClockSkewMs;
|
||||
parseTimestamp(payload.timestamp).getTime() < lastReconnectHandshakeAt - serverClockSkewMs;
|
||||
// highlightsCurrentUser (mentions.ts) treats @everyone and @here as one
|
||||
// bit, because the wire carries only one: mentions_everyone. But the
|
||||
// server's applyMentionCounts (mentions.go) narrows an @here fan-out to
|
||||
@@ -702,15 +702,15 @@ export function wireDispatcher(
|
||||
const isDetached = isWindowDetached(payload.channel_id);
|
||||
|
||||
if ((payload.channel_id !== activeId || isDetached) && !isOwnMessage) {
|
||||
// incrementUnread/incrementMention skip the active channel by
|
||||
// default — evenIfActive (isDetached here) is a no-op for a
|
||||
// genuinely non-active channel, since their internal guard only
|
||||
// fires when channelId IS the active one.
|
||||
incrementUnread(payload.channel_id, isDetached);
|
||||
// A mention is an unread too — the mention badge just outranks it.
|
||||
if (isMention) {
|
||||
incrementMention(payload.channel_id, isDetached);
|
||||
}
|
||||
// noteChannelMessage skips the active channel by default —
|
||||
// evenIfActive (isDetached here) is a no-op for a genuinely
|
||||
// non-active channel, since its internal guard only fires when
|
||||
// channelId IS the active one. It also guards both counters behind
|
||||
// payload.id vs. the channel's lastMessageId watermark (OC-0328), so
|
||||
// a message already reflected in a `ready` snapshot (delivered
|
||||
// between the server's registerNow and buildReady, then redelivered
|
||||
// as a queued chat_message) does not double-count.
|
||||
noteChannelMessage(payload.channel_id, payload.id, isMention, isDetached);
|
||||
}
|
||||
|
||||
// Update DM store last message if this message belongs to a DM channel.
|
||||
@@ -729,7 +729,7 @@ export function wireDispatcher(
|
||||
);
|
||||
} else {
|
||||
// The DM badge reads dmStore's mentionCount (mute-immune, rendered
|
||||
// by DmSidebar) — incrementMention above no-ops for DM ids, which
|
||||
// by DmSidebar) — noteChannelMessage above no-ops for DM ids, which
|
||||
// are absent from channelsStore. isMention is passed through so the
|
||||
// mention bump sits behind updateDmLastMessage's own message-id
|
||||
// guard (OC-0242) — a separate unconditional increment here would
|
||||
@@ -761,7 +761,7 @@ export function wireDispatcher(
|
||||
notifyIncomingMessage(payload);
|
||||
// Refresh the skew estimate from this accepted-as-live frame so it
|
||||
// stays current for the next reconnect.
|
||||
serverClockSkewMs = Date.now() - Date.parse(payload.timestamp);
|
||||
serverClockSkewMs = Date.now() - parseTimestamp(payload.timestamp).getTime();
|
||||
}
|
||||
}),
|
||||
);
|
||||
@@ -1060,13 +1060,19 @@ export function wireDispatcher(
|
||||
// late-arriving voice_leave for a channel we already left (and rejoined
|
||||
// elsewhere) must not kill a newer join. Read the store before
|
||||
// leaveVoiceChannel() below clears currentChannelId.
|
||||
const shouldTeardownSession =
|
||||
isSelf && voiceStore.getState().currentChannelId === payload.channel_id;
|
||||
const sameChannel = voiceStore.getState().currentChannelId === payload.channel_id;
|
||||
const shouldTeardownSession = isSelf && sameChannel;
|
||||
// Notify E2EE state machine so key holder can rotate the room key, and
|
||||
// (when applicable) tear down the media session — both through one lazy
|
||||
// import so the two effects cannot land in different ticks.
|
||||
// OC-0311: voice_leave is broadcast to the whole channelReadAudience,
|
||||
// i.e. everyone with READ_MESSAGES on THAT channel — not just its
|
||||
// voice participants. Scope the E2EE notification to this client's own
|
||||
// voice channel so a peer leaving a channel we merely read (and never
|
||||
// shared a call with) cannot delete their key, clear their
|
||||
// verification, or trigger a room-key rotation in our live session.
|
||||
void livekitSession().then(({ handleParticipantLeft, leaveVoice }) => {
|
||||
void handleParticipantLeft(payload.user_id);
|
||||
if (sameChannel) void handleParticipantLeft(payload.user_id);
|
||||
if (shouldTeardownSession) void leaveVoice(false);
|
||||
});
|
||||
// Clear local voice state only for the same channel-match case as the
|
||||
|
||||
@@ -29,6 +29,8 @@ export function isValidHost(host: string): boolean {
|
||||
// one colon means the whole string is the address — a single colon is
|
||||
// reserved for the host:port separator below.
|
||||
if ((host.match(/:/g) ?? []).length > 1 && /^[0-9A-Fa-f:.]+$/.test(host)) return true;
|
||||
// DNS name or IPv4 literal, optionally with a port.
|
||||
return /^[\w.-]+(:\d+)?$/.test(host);
|
||||
// DNS name or IPv4 literal, optionally with a port. Explicit ASCII class
|
||||
// (not `\w`, which wrongly includes '_') to match the Rust proxies'
|
||||
// charset exactly.
|
||||
return /^[A-Za-z0-9.-]+(:\d+)?$/.test(host);
|
||||
}
|
||||
|
||||
@@ -388,6 +388,51 @@ export function incrementMention(channelId: number, evenIfActive = false): void
|
||||
});
|
||||
}
|
||||
|
||||
/**
|
||||
* Record an incoming channel message: bumps unread (and mention, when
|
||||
* `isMention`) unless `messageId` is already reflected in the channel's
|
||||
* watermark — mirrors dm.store's `updateDmLastMessage` (OC-0242).
|
||||
*
|
||||
* OC-0328: a message delivered between the server's registerNow and
|
||||
* buildReady is both counted in `ready`'s snapshot (unread_count/
|
||||
* last_message_id already advanced) AND redelivered as a queued
|
||||
* chat_message once the socket drains. Both counters must sit behind the
|
||||
* SAME watermark read in one setState — splitting the guard across
|
||||
* incrementUnread/incrementMention can't work, since the first call would
|
||||
* already have advanced lastMessageId before the second one checked it.
|
||||
*
|
||||
* `evenIfActive` mirrors incrementUnread's escape hatch — see its doc.
|
||||
*/
|
||||
export function noteChannelMessage(
|
||||
channelId: number,
|
||||
messageId: number,
|
||||
isMention: boolean,
|
||||
evenIfActive = false,
|
||||
): void {
|
||||
channelsStore.setState((prev) => {
|
||||
if (prev.activeChannelId === channelId && !evenIfActive) {
|
||||
return prev;
|
||||
}
|
||||
const existing = prev.channels.get(channelId);
|
||||
if (existing === undefined) {
|
||||
return prev;
|
||||
}
|
||||
const isReplay = existing.lastMessageId !== null && messageId <= existing.lastMessageId;
|
||||
if (isReplay) {
|
||||
return prev;
|
||||
}
|
||||
const updated: Channel = {
|
||||
...existing,
|
||||
unreadCount: existing.unreadCount + 1,
|
||||
mentionCount: isMention ? existing.mentionCount + 1 : existing.mentionCount,
|
||||
lastMessageId: messageId,
|
||||
};
|
||||
const next = new Map(prev.channels);
|
||||
next.set(channelId, updated);
|
||||
return { ...prev, channels: next };
|
||||
});
|
||||
}
|
||||
|
||||
/** Clear the unread and mention counts for a channel — they clear together. */
|
||||
export function clearUnread(channelId: number): void {
|
||||
channelsStore.setState((prev) => {
|
||||
|
||||
@@ -146,6 +146,7 @@ export function updateDmLastMessage(
|
||||
if (updated === undefined) return prev;
|
||||
const rest = prev.channels.filter((c) => c.channelId !== channelId);
|
||||
const isReplay = updated.lastMessageId !== null && messageId <= updated.lastMessageId;
|
||||
if (isReplay) return prev;
|
||||
return {
|
||||
channels: [
|
||||
{
|
||||
@@ -153,8 +154,8 @@ export function updateDmLastMessage(
|
||||
lastMessageId: messageId,
|
||||
lastMessage: content,
|
||||
lastMessageAt: timestamp,
|
||||
unreadCount: isReplay ? updated.unreadCount : updated.unreadCount + 1,
|
||||
mentionCount: isMention && !isReplay ? updated.mentionCount + 1 : updated.mentionCount,
|
||||
unreadCount: updated.unreadCount + 1,
|
||||
mentionCount: isMention ? updated.mentionCount + 1 : updated.mentionCount,
|
||||
},
|
||||
...rest,
|
||||
],
|
||||
|
||||
@@ -16,6 +16,7 @@ import {
|
||||
UNCATEGORIZED_VOICE_CATEGORY,
|
||||
incrementUnread,
|
||||
incrementMention,
|
||||
noteChannelMessage,
|
||||
clearUnread,
|
||||
getUnreadOnOpen,
|
||||
resetChannelsStore,
|
||||
@@ -625,6 +626,67 @@ describe("channels store", () => {
|
||||
});
|
||||
});
|
||||
|
||||
// OC-0328: a message delivered between the server's registerNow and
|
||||
// buildReady is both counted in `ready` (unread_count/last_message_id
|
||||
// already advanced) and redelivered as a queued chat_message — mirrors
|
||||
// dm.store's updateDmLastMessage guard (OC-0242).
|
||||
describe("noteChannelMessage", () => {
|
||||
it("increments unread and advances the watermark for a new message", () => {
|
||||
setChannels([{ ...readyChannels[0]!, unread_count: 1, last_message_id: 100 }]);
|
||||
|
||||
noteChannelMessage(1, 101, false);
|
||||
|
||||
const ch = channelsStore.getState().channels.get(1);
|
||||
expect(ch?.unreadCount).toBe(2);
|
||||
expect(ch?.lastMessageId).toBe(101);
|
||||
});
|
||||
|
||||
it("also increments mention when isMention is set", () => {
|
||||
setChannels([{ ...readyChannels[0]!, unread_count: 1, last_message_id: 100 }]);
|
||||
|
||||
noteChannelMessage(1, 101, true);
|
||||
|
||||
expect(channelsStore.getState().channels.get(1)?.mentionCount).toBe(1);
|
||||
});
|
||||
|
||||
it("does not bump either counter for a message id already reflected in lastMessageId", () => {
|
||||
setChannels([{ ...readyChannels[0]!, unread_count: 1, last_message_id: 100 }]);
|
||||
|
||||
noteChannelMessage(1, 100, true); // same id ready already counted
|
||||
|
||||
const ch = channelsStore.getState().channels.get(1);
|
||||
expect(ch?.unreadCount).toBe(1); // unchanged
|
||||
expect(ch?.mentionCount).toBe(0); // unchanged
|
||||
});
|
||||
|
||||
it("skips increment for the active channel", () => {
|
||||
setChannels([{ ...readyChannels[0]!, unread_count: 1, last_message_id: 100 }]);
|
||||
setActiveChannel(1);
|
||||
|
||||
noteChannelMessage(1, 101, false);
|
||||
|
||||
expect(channelsStore.getState().channels.get(1)?.unreadCount).toBe(0);
|
||||
});
|
||||
|
||||
it("bumps the active channel when evenIfActive is set", () => {
|
||||
setChannels([{ ...readyChannels[0]!, unread_count: 1, last_message_id: 100 }]);
|
||||
setActiveChannel(1);
|
||||
|
||||
noteChannelMessage(1, 101, false, true);
|
||||
|
||||
expect(channelsStore.getState().channels.get(1)?.unreadCount).toBe(1);
|
||||
});
|
||||
|
||||
it("is a no-op for an unknown channel id", () => {
|
||||
setChannels(readyChannels);
|
||||
const before = channelsStore.getState();
|
||||
|
||||
noteChannelMessage(999, 1, false);
|
||||
|
||||
expect(channelsStore.getState()).toBe(before);
|
||||
});
|
||||
});
|
||||
|
||||
describe("clearUnread", () => {
|
||||
it("resets unread count to 0", () => {
|
||||
setChannels(readyChannels);
|
||||
|
||||
@@ -434,6 +434,54 @@ describe("WS Dispatcher", () => {
|
||||
expect(ch?.unreadCount).toBe(1);
|
||||
});
|
||||
|
||||
// OC-0328: mirrors the DM-side "does not double-count" test below. A
|
||||
// channel message delivered between the server's registerNow and
|
||||
// buildReady is both counted in `ready`'s snapshot (lastMessageId already
|
||||
// advanced to its id) AND redelivered as a queued chat_message once the
|
||||
// socket drains — the channel path had no replay guard at all.
|
||||
it("does not double-count a channel unread/mention whose id is already reflected in lastMessageId", () => {
|
||||
authStore.setState((prev) => ({
|
||||
...prev,
|
||||
user: { id: 5, username: "me", avatar: null, role: "member" },
|
||||
}));
|
||||
channelsStore.setState((prev) => {
|
||||
const ch = new Map(prev.channels);
|
||||
ch.set(5, {
|
||||
id: 5,
|
||||
name: "off-topic",
|
||||
type: "text" as const,
|
||||
category: null,
|
||||
position: 0,
|
||||
unreadCount: 1,
|
||||
mentionCount: 1,
|
||||
lastMessageId: 200, // already reflects message 200 via `ready`
|
||||
canSend: true,
|
||||
topic: "",
|
||||
slowMode: 0,
|
||||
nsfw: false,
|
||||
voiceMaxUsers: 0,
|
||||
voiceMaxVideo: 0,
|
||||
});
|
||||
return { ...prev, channels: ch, activeChannelId: 1 }; // active is channel 1
|
||||
});
|
||||
|
||||
// The same message redelivered as a queued chat_message.
|
||||
mock.dispatch("chat_message", {
|
||||
id: 200,
|
||||
channel_id: 5,
|
||||
user: { id: 2, username: "bob", avatar: null },
|
||||
content: "hey @me",
|
||||
mentions: [5],
|
||||
reply_to: null,
|
||||
attachments: [],
|
||||
timestamp: "2026-03-15T10:00:00Z",
|
||||
});
|
||||
|
||||
const ch = channelsStore.getState().channels.get(5);
|
||||
expect(ch?.unreadCount).toBe(1);
|
||||
expect(ch?.mentionCount).toBe(1);
|
||||
});
|
||||
|
||||
// OC-0204: "active channel" normally means "the user is watching the live
|
||||
// tail", so skipping the unread bump there is correct — until a jump to an
|
||||
// old permalink/reply/search hit leaves the SAME active channel showing a
|
||||
@@ -679,6 +727,84 @@ describe("WS Dispatcher", () => {
|
||||
});
|
||||
});
|
||||
|
||||
// OC-0315: payload.timestamp is the raw SQLite datetime('now') string —
|
||||
// naive UTC, no 'Z' suffix (the server never emits one). Date.parse (used
|
||||
// by the replay-gate comparison, unlike the parseTimestamp helper built
|
||||
// for exactly this) interprets that as LOCAL time. On a viewer whose zone
|
||||
// is east of UTC, the parsed epoch reads *earlier* than the true instant,
|
||||
// so a genuinely live message can look like it predates the reconnect
|
||||
// handshake and gets silently swallowed by the replay gate — worst when
|
||||
// serverClockSkewMs is still 0 (nothing has been sampled yet this
|
||||
// session), since nothing else offsets the bias. Pin a real east-of-UTC
|
||||
// zone to observe it; skip where the pin isn't honored (see the probe in
|
||||
// renderers.test.ts's DST block for why a worker-thread pool can't).
|
||||
const oc0315OriginalTZ = process.env.TZ;
|
||||
process.env.TZ = "Asia/Tokyo";
|
||||
const oc0315PinHonored = new Date(2026, 0, 15).getTimezoneOffset() === -540;
|
||||
if (oc0315OriginalTZ === undefined) {
|
||||
delete process.env.TZ;
|
||||
} else {
|
||||
process.env.TZ = oc0315OriginalTZ;
|
||||
}
|
||||
|
||||
describe.skipIf(!oc0315PinHonored)(
|
||||
"[OC-0315] naive-UTC server timestamps vs a non-UTC viewer clock",
|
||||
() => {
|
||||
const originalTZ = process.env.TZ;
|
||||
|
||||
beforeEach(() => {
|
||||
process.env.TZ = "Asia/Tokyo";
|
||||
vi.mocked(mockNotifyIncomingMessage).mockClear();
|
||||
});
|
||||
|
||||
afterEach(() => {
|
||||
if (originalTZ === undefined) {
|
||||
delete process.env.TZ;
|
||||
} else {
|
||||
process.env.TZ = originalTZ;
|
||||
}
|
||||
});
|
||||
|
||||
it("does not misclassify a live message as a replay when serverClockSkewMs is still 0 (cold, never sampled)", () => {
|
||||
// Sanity: really pinned east of UTC (Tokyo has no DST, so this is
|
||||
// stable year-round, unlike the America/New_York probe elsewhere).
|
||||
expect(new Date(2026, 0, 15).getTimezoneOffset()).toBe(-540);
|
||||
|
||||
mock.dispatch("auth_ok", {
|
||||
user: { id: 1, username: "alex", avatar: null, role: "admin" },
|
||||
server_name: "TestServer",
|
||||
motd: "",
|
||||
});
|
||||
const handshakeAt = Date.now();
|
||||
// Second auth_ok in the same dispatcher lifetime = a reconnect.
|
||||
mock.dispatch("auth_ok", {
|
||||
user: { id: 1, username: "alex", avatar: null, role: "admin" },
|
||||
server_name: "TestServer",
|
||||
motd: "",
|
||||
});
|
||||
|
||||
// A genuinely live message, 1s after the handshake, stamped by the
|
||||
// server in its real wire form: naive UTC, no 'Z'.
|
||||
vi.setSystemTime(handshakeAt + 1000);
|
||||
const naiveUtcTimestamp = new Date(Date.now())
|
||||
.toISOString()
|
||||
.replace("T", " ")
|
||||
.replace(/\.\d{3}Z$/, "");
|
||||
mock.dispatch("chat_message", {
|
||||
id: 1,
|
||||
channel_id: 1,
|
||||
user: { id: 2, username: "bob", avatar: null },
|
||||
content: "live now, naive-UTC timestamp",
|
||||
reply_to: null,
|
||||
attachments: [],
|
||||
timestamp: naiveUtcTimestamp,
|
||||
});
|
||||
|
||||
expect(mockNotifyIncomingMessage).toHaveBeenCalledTimes(1);
|
||||
});
|
||||
},
|
||||
);
|
||||
|
||||
describe("mention counts", () => {
|
||||
function seedChannel(): void {
|
||||
channelsStore.setState((prev) => {
|
||||
@@ -2712,6 +2838,37 @@ describe("WS Dispatcher", () => {
|
||||
expect(mockHandleParticipantLeft).toHaveBeenCalledWith(7);
|
||||
});
|
||||
|
||||
// OC-0311: voice_leave is broadcast to channelReadAudience(channel), i.e.
|
||||
// everyone with READ_MESSAGES on THAT channel — not just its voice
|
||||
// participants. A client can read channel B (and so receive B's
|
||||
// voice_leave frames) while its own live voice session is in channel A.
|
||||
// Without a channel guard, a peer leaving a channel this client merely
|
||||
// reads mutates this client's own E2EE peer state (deletes the peer's key,
|
||||
// clears their verification badge, retires their key, and can trigger a
|
||||
// room-key rotation) for a call that peer was never part of.
|
||||
it("[OC-0311] does not touch E2EE peer state for a voice_leave from a channel this client is not in", async () => {
|
||||
vi.mocked(mockHandleParticipantLeft).mockClear();
|
||||
authStore.setState((prev) => ({
|
||||
...prev,
|
||||
user: { id: 5, username: "me", avatar: null, role: "member" },
|
||||
}));
|
||||
// This client's live voice session is channel 3.
|
||||
voiceStore.setState((prev) => ({
|
||||
...prev,
|
||||
currentChannelId: 3,
|
||||
}));
|
||||
|
||||
// A peer leaves channel 99, which this client can merely read (hence
|
||||
// seeing the broadcast) but is not the client's own voice channel.
|
||||
mock.dispatch("voice_leave", {
|
||||
channel_id: 99,
|
||||
user_id: 7,
|
||||
});
|
||||
await vi.runAllTimersAsync();
|
||||
|
||||
expect(mockHandleParticipantLeft).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("mirrors a moderator mute/deafen into the local flags and honors it", async () => {
|
||||
authStore.setState((prev) => ({
|
||||
...prev,
|
||||
|
||||
@@ -357,6 +357,36 @@ describe("dmStore", () => {
|
||||
const ch = dmStore.getState().channels[0]!;
|
||||
expect(ch.unreadCount).toBe(2);
|
||||
});
|
||||
|
||||
// OC-0317: a replayed/stale id must not regress the lastMessageId
|
||||
// watermark either — its sibling updateDmLastMessagePreview already
|
||||
// returns `prev` untouched on replay (OC-0301); this function must do
|
||||
// the same so a later, genuinely-new frame in the same burst is still
|
||||
// correctly recognized as new instead of looking like another replay.
|
||||
it("does not regress lastMessageId, lastMessage, or lastMessageAt on a replay", () => {
|
||||
setDmChannels([
|
||||
makeDm({
|
||||
channelId: 5,
|
||||
unreadCount: 1,
|
||||
lastMessageId: 102,
|
||||
lastMessage: "second",
|
||||
lastMessageAt: "2026-03-28T12:00:02Z",
|
||||
}),
|
||||
]);
|
||||
updateDmLastMessage(5, 101, "stale-replay", "2026-03-28T12:00:01Z");
|
||||
const ch = dmStore.getState().channels[0]!;
|
||||
expect(ch.lastMessageId).toBe(102);
|
||||
expect(ch.lastMessage).toBe("second");
|
||||
expect(ch.lastMessageAt).toBe("2026-03-28T12:00:02Z");
|
||||
expect(ch.unreadCount).toBe(1);
|
||||
|
||||
// The next genuinely-new frame must still be counted as new, not
|
||||
// treated as a second replay because the watermark got rolled back.
|
||||
updateDmLastMessage(5, 103, "third", "2026-03-28T12:00:03Z");
|
||||
const after = dmStore.getState().channels[0]!;
|
||||
expect(after.lastMessageId).toBe(103);
|
||||
expect(after.unreadCount).toBe(2);
|
||||
});
|
||||
});
|
||||
|
||||
// ── updateDmLastMessagePreview ──────────────────────────
|
||||
|
||||
@@ -27,6 +27,15 @@ describe("isValidHost", () => {
|
||||
expect(isValidHost("chat.example.com:8443")).toBe(true);
|
||||
});
|
||||
|
||||
it("rejects a host containing an underscore (OC-0322: Rust proxies reject it)", () => {
|
||||
// http_proxy::validate_remote_host and livekit_proxy::validate_remote_host
|
||||
// only allow is_ascii_alphanumeric() || '.' | '-' | ':' | '[' | ']' -- JS
|
||||
// `\w` wrongly includes '_', which would let the client save/accept a
|
||||
// host neither Rust proxy can ever connect to.
|
||||
expect(isValidHost("chat_example.com")).toBe(false);
|
||||
expect(isValidHost("my_server.lan:8443")).toBe(false);
|
||||
});
|
||||
|
||||
it("accepts an IPv4 literal, optionally with a port", () => {
|
||||
expect(isValidHost("192.168.1.1")).toBe(true);
|
||||
expect(isValidHost("192.168.1.1:8443")).toBe(true);
|
||||
|
||||
+43
-29
@@ -11,8 +11,9 @@
|
||||
// detector.wasm
|
||||
// assets/...
|
||||
//
|
||||
// Loader walks the directory, parses every plugin.json, and returns a slice
|
||||
// of foundPlugin records. The Registry then persists each into the store.
|
||||
// Loader walks the directory, parses every plugin.toml (wazero build) or
|
||||
// plugin.json manifest via loadManifestFromDir, and returns a slice of
|
||||
// foundPlugin records. The Registry then persists each into the store.
|
||||
|
||||
package plugin
|
||||
|
||||
@@ -30,11 +31,38 @@ type foundPlugin struct {
|
||||
WASMPath string
|
||||
}
|
||||
|
||||
// scanPluginDirectory walks dir non-recursively and parses plugin.json from
|
||||
// every immediate subdirectory. A per-plugin failure (malformed manifest,
|
||||
// missing or symlinked entrypoint, a stray symlink anywhere in that plugin's
|
||||
// tree) is recorded and that one subdirectory is skipped — it does not stop
|
||||
// the scan. The returned error is non-nil whenever at least one subdirectory
|
||||
// loadManifestFromDir resolves the manifest that governs one plugin
|
||||
// directory, preferring plugin.toml (wazero build) over plugin.json — the
|
||||
// precedence scanPluginDirectory has always applied on the on-disk restart
|
||||
// path. Every caller that resolves a plugin manifest from a directory must
|
||||
// route through here, so the manifest a zip install validates is
|
||||
// byte-for-byte the one the next restart loads (OC-0318): two callers
|
||||
// disagreeing on precedence let an admin approve a narrow manifest while a
|
||||
// broader one, never reviewed, silently takes over after the next restart.
|
||||
//
|
||||
// Returns an error satisfying os.IsNotExist when neither manifest file is
|
||||
// present, so callers that treat "not a plugin directory" as non-fatal
|
||||
// (scanPluginDirectory, walking arbitrary subdirectories) can tell that
|
||||
// apart from a real parse failure.
|
||||
func loadManifestFromDir(dir string) (*Manifest, error) {
|
||||
if m, ok, err := tryLoadPluginTOML(dir); err != nil {
|
||||
return nil, err
|
||||
} else if ok {
|
||||
return m, nil
|
||||
}
|
||||
raw, err := os.ReadFile(filepath.Join(dir, "plugin.json"))
|
||||
if err != nil {
|
||||
return nil, err
|
||||
}
|
||||
return ParseManifest(raw)
|
||||
}
|
||||
|
||||
// scanPluginDirectory walks dir non-recursively and parses a manifest
|
||||
// (plugin.toml or plugin.json, via loadManifestFromDir) from every immediate
|
||||
// subdirectory. A per-plugin failure (malformed manifest, missing or
|
||||
// symlinked entrypoint, a stray symlink anywhere in that plugin's tree) is
|
||||
// recorded and that one subdirectory is skipped — it does not stop the
|
||||
// scan. The returned error is non-nil whenever at least one subdirectory
|
||||
// was skipped, joining every such failure, but `found` still holds every
|
||||
// plugin that scanned cleanly. Callers that need the scan to be all-or-
|
||||
// nothing should check the returned error before using `found`; LoadAll
|
||||
@@ -60,30 +88,16 @@ func scanPluginDirectory(dir string) ([]foundPlugin, error) {
|
||||
}
|
||||
pluginDir := filepath.Join(dir, e.Name())
|
||||
|
||||
// Prefer plugin.toml (wazero build) over plugin.json.
|
||||
manifest, ok, tomlErr := tryLoadPluginTOML(pluginDir)
|
||||
if tomlErr != nil {
|
||||
scanErr = errors.Join(scanErr, fmt.Errorf("plugin %q: %w", e.Name(), tomlErr))
|
||||
manifest, manifestErr := loadManifestFromDir(pluginDir)
|
||||
if manifestErr != nil {
|
||||
if os.IsNotExist(manifestErr) {
|
||||
// Neither plugin.toml nor plugin.json present — not a plugin
|
||||
// directory, skip silently.
|
||||
continue
|
||||
}
|
||||
scanErr = errors.Join(scanErr, fmt.Errorf("plugin %q: %w", e.Name(), manifestErr))
|
||||
continue
|
||||
}
|
||||
if !ok {
|
||||
// Fall back to plugin.json.
|
||||
manifestPath := filepath.Join(pluginDir, "plugin.json")
|
||||
raw, rdErr := os.ReadFile(manifestPath)
|
||||
if rdErr != nil {
|
||||
if os.IsNotExist(rdErr) {
|
||||
continue
|
||||
}
|
||||
scanErr = errors.Join(scanErr, fmt.Errorf("plugin %q: read plugin.json: %w", e.Name(), rdErr))
|
||||
continue
|
||||
}
|
||||
var parseErr error
|
||||
manifest, parseErr = ParseManifest(raw)
|
||||
if parseErr != nil {
|
||||
scanErr = errors.Join(scanErr, fmt.Errorf("plugin %q: %w", e.Name(), parseErr))
|
||||
continue
|
||||
}
|
||||
}
|
||||
// Reject any symlinks anywhere in the plugin directory tree. The asset
|
||||
// handler enforces that resolved paths stay rooted at pluginDir, but
|
||||
// http.ServeFile / os.Open follow symlinks transparently — a malicious
|
||||
|
||||
@@ -75,8 +75,8 @@ type CommandSpec struct {
|
||||
// Resources caps the plugin's runtime budget. Zero means "use the runtime
|
||||
// default from PluginsConfig".
|
||||
type Resources struct {
|
||||
MaxMemoryMB int `json:"max_memory_mb"`
|
||||
CPUBudgetMs int `json:"cpu_budget_ms"`
|
||||
MaxMemoryMB int `json:"max_memory_mb" toml:"max_memory_mb"`
|
||||
CPUBudgetMs int `json:"cpu_budget_ms" toml:"cpu_budget_ms"`
|
||||
}
|
||||
|
||||
// UISpec describes the optional client-side rendering surface.
|
||||
|
||||
@@ -8,6 +8,8 @@ package plugin
|
||||
import (
|
||||
"strings"
|
||||
"testing"
|
||||
|
||||
"github.com/BurntSushi/toml"
|
||||
)
|
||||
|
||||
func TestPluginNameRegexp(t *testing.T) {
|
||||
@@ -143,3 +145,34 @@ func TestManifestValidateRejectsUnknownPermission(t *testing.T) {
|
||||
t.Fatal("expected validation failure for unknown permission")
|
||||
}
|
||||
}
|
||||
|
||||
// TestManifestTOMLDecodesResourceFields pins OC-0338: BurntSushi/toml
|
||||
// resolves a TOML key to a struct field via the `toml` tag, or — absent that
|
||||
// tag — the Go field name matched with strings.EqualFold. Resources' fields
|
||||
// only carried `json` tags, so snake_case keys like max_memory_mb never
|
||||
// matched MaxMemoryMB (underscores break EqualFold) and silently decoded to
|
||||
// zero. This test decodes directly with the toml package (no build tag, no
|
||||
// -tags wazero needed) so it runs in the default `go test ./...` build that
|
||||
// CI always exercises, even though tryLoadPluginTOML itself only compiles
|
||||
// under -tags wazero.
|
||||
func TestManifestTOMLDecodesResourceFields(t *testing.T) {
|
||||
const src = `
|
||||
name = "foo"
|
||||
version = "1.0.0"
|
||||
entrypoint = "foo.wasm"
|
||||
|
||||
[resources]
|
||||
cpu_budget_ms = 2000
|
||||
max_memory_mb = 128
|
||||
`
|
||||
var m Manifest
|
||||
if _, err := toml.Decode(src, &m); err != nil {
|
||||
t.Fatalf("toml.Decode: %v", err)
|
||||
}
|
||||
if m.Resources.CPUBudgetMs != 2000 {
|
||||
t.Errorf("Resources.CPUBudgetMs = %d, want 2000", m.Resources.CPUBudgetMs)
|
||||
}
|
||||
if m.Resources.MaxMemoryMB != 128 {
|
||||
t.Errorf("Resources.MaxMemoryMB = %d, want 128", m.Resources.MaxMemoryMB)
|
||||
}
|
||||
}
|
||||
|
||||
+23
-10
@@ -148,7 +148,8 @@ func (r *Registry) Sink() *EventSink {
|
||||
return r.sink
|
||||
}
|
||||
|
||||
// LoadAll scans cfg.Directory and persists every plugin.json found into the
|
||||
// LoadAll scans cfg.Directory and persists every plugin manifest found
|
||||
// (plugin.toml or plugin.json, via loadManifestFromDir) into the
|
||||
// PluginStore. In the wazero-tagged build it then compiles each entrypoint
|
||||
// into a runnable module; the default build stops at the persistence step.
|
||||
func (r *Registry) LoadAll(ctx context.Context) error {
|
||||
@@ -231,7 +232,9 @@ func (r *Registry) installFromDisk(ctx context.Context, found foundPlugin) error
|
||||
// then renames it into the plugin directory and registers it via
|
||||
// installFromDisk. Returns the new plugin name on success.
|
||||
//
|
||||
// The zip must contain a top-level plugin.json. The plugin's directory name
|
||||
// The zip must contain a top-level plugin.json or plugin.toml (not both —
|
||||
// see installZipStagedManifest), resolved via the same precedence
|
||||
// scanPluginDirectory applies on every restart. The plugin's directory name
|
||||
// is taken from manifest.Name (validated by Manifest.Validate to a strict
|
||||
// charset). Re-installing an existing plugin replaces it.
|
||||
const (
|
||||
@@ -415,17 +418,27 @@ func installZipWriteEntry(f *zip.File, destAbs string, remaining int64) (int64,
|
||||
return n, nil
|
||||
}
|
||||
|
||||
// installZipStagedManifest parses the staged plugin.json and holds the staged
|
||||
// tree to the same rules scanPluginDirectory applies to an on-disk plugin (no
|
||||
// symlinks anywhere, entrypoint present and not a symlink).
|
||||
// installZipStagedManifest resolves the staged plugin's manifest via the
|
||||
// same loadManifestFromDir precedence scanPluginDirectory uses (plugin.toml
|
||||
// preferred over plugin.json), and holds the staged tree to the same rules
|
||||
// scanPluginDirectory applies to an on-disk plugin (no symlinks anywhere,
|
||||
// entrypoint present and not a symlink).
|
||||
//
|
||||
// A zip carrying both plugin.json and plugin.toml is rejected outright
|
||||
// rather than silently picking one: shipping both is never intentional, and
|
||||
// letting one win invites exactly the admin-approved-a-different-manifest
|
||||
// bug OC-0318 was filed for.
|
||||
func installZipStagedManifest(stageAbs string) (*Manifest, error) {
|
||||
manifestPath := filepath.Join(stageAbs, "plugin.json")
|
||||
raw, err := os.ReadFile(manifestPath)
|
||||
if err != nil {
|
||||
return nil, fmt.Errorf("plugin zip: missing plugin.json at root: %w", err)
|
||||
if _, jsonErr := os.Stat(filepath.Join(stageAbs, "plugin.json")); jsonErr == nil {
|
||||
if _, tomlErr := os.Stat(filepath.Join(stageAbs, "plugin.toml")); tomlErr == nil {
|
||||
return nil, fmt.Errorf("plugin zip: must not contain both plugin.json and plugin.toml")
|
||||
}
|
||||
}
|
||||
manifest, err := ParseManifest(raw)
|
||||
manifest, err := loadManifestFromDir(stageAbs)
|
||||
if err != nil {
|
||||
if os.IsNotExist(err) {
|
||||
return nil, fmt.Errorf("plugin zip: missing plugin.json at root: %w", err)
|
||||
}
|
||||
return nil, err
|
||||
}
|
||||
// Validate the staged contents the same way scanPluginDirectory does.
|
||||
|
||||
@@ -693,6 +693,26 @@ func TestRegistry_InstallFromZip_Rejections(t *testing.T) {
|
||||
}
|
||||
}
|
||||
|
||||
// OC-0318: a zip carrying both plugin.json and plugin.toml is ambiguous —
|
||||
// installZipStagedManifest used to validate only the JSON while the on-disk
|
||||
// loader (scanPluginDirectory) prefers TOML, so the manifest an admin
|
||||
// approved at install time was not necessarily the one that governed the
|
||||
// plugin after the next restart. Reject the ambiguity outright instead of
|
||||
// silently picking one file over the other.
|
||||
func TestRegistry_InstallFromZip_RejectsBothManifests(t *testing.T) {
|
||||
r, _, _ := newRegistryWithDir(t)
|
||||
|
||||
zipBytes := buildZip(t, map[string]string{
|
||||
"plugin.json": simpleManifest("dual"),
|
||||
"plugin.toml": "name = \"dual\"\nversion = \"1.0.0\"\nentrypoint = \"dual.wasm\"\n",
|
||||
"dual.wasm": "\x00asm\x01\x00\x00\x00",
|
||||
})
|
||||
|
||||
if _, err := r.InstallFromZip(context.Background(), zipBytes); err == nil {
|
||||
t.Fatal("InstallFromZip accepted a zip containing both plugin.json and plugin.toml; want a rejection of the ambiguity")
|
||||
}
|
||||
}
|
||||
|
||||
func TestRegistry_InstallFromZip_NotConfigured(t *testing.T) {
|
||||
store := openPluginTestDB(t)
|
||||
r, err := NewRegistry(Config{Store: store}) // no Directory
|
||||
|
||||
@@ -0,0 +1,95 @@
|
||||
//go:build wazero
|
||||
|
||||
// OC-0318 (wazero build): installZipStagedManifest used to read only
|
||||
// plugin.json, so a zip containing solely a plugin.toml failed to install
|
||||
// at all — even though the on-disk loader (scanPluginDirectory) happily
|
||||
// parses plugin.toml on every restart. The two paths must resolve the
|
||||
// identical manifest for the same directory contents: this pins that a
|
||||
// TOML-only zip now installs, and that scanning the resulting on-disk
|
||||
// directory finds the same manifest fields InstallFromZip itself
|
||||
// registered — the precedence parity OC-0318 requires.
|
||||
package plugin
|
||||
|
||||
import (
|
||||
"archive/zip"
|
||||
"bytes"
|
||||
"context"
|
||||
"testing"
|
||||
)
|
||||
|
||||
// buildTomlOnlyZip builds a zip containing only a plugin.toml (no
|
||||
// plugin.json) plus a placeholder entrypoint file.
|
||||
func buildTomlOnlyZip(t *testing.T, name string) []byte {
|
||||
t.Helper()
|
||||
var buf bytes.Buffer
|
||||
zw := zip.NewWriter(&buf)
|
||||
files := map[string]string{
|
||||
"plugin.toml": "name = \"" + name + "\"\n" +
|
||||
"version = \"1.0.0\"\n" +
|
||||
"entrypoint = \"" + name + ".wasm\"\n" +
|
||||
"permissions = [\"commands\"]\n\n" +
|
||||
"[[commands]]\n" +
|
||||
"name = \"hello\"\n",
|
||||
name + ".wasm": "\x00asm\x01\x00\x00\x00",
|
||||
}
|
||||
for fname, content := range files {
|
||||
w, err := zw.Create(fname)
|
||||
if err != nil {
|
||||
t.Fatalf("zip create %s: %v", fname, err)
|
||||
}
|
||||
if _, err := w.Write([]byte(content)); err != nil {
|
||||
t.Fatalf("zip write %s: %v", fname, err)
|
||||
}
|
||||
}
|
||||
if err := zw.Close(); err != nil {
|
||||
t.Fatalf("zip close: %v", err)
|
||||
}
|
||||
return buf.Bytes()
|
||||
}
|
||||
|
||||
func TestRegistry_InstallFromZip_TomlOnlyMatchesScan(t *testing.T) {
|
||||
dir := t.TempDir()
|
||||
r, _ := newWazeroTestRegistry(t, dir)
|
||||
ctx := context.Background()
|
||||
|
||||
zipBytes := buildTomlOnlyZip(t, "tomlplug")
|
||||
name, err := r.InstallFromZip(ctx, zipBytes)
|
||||
if err != nil {
|
||||
t.Fatalf("InstallFromZip with a TOML-only plugin: %v", err)
|
||||
}
|
||||
if name != "tomlplug" {
|
||||
t.Fatalf("name = %q, want %q", name, "tomlplug")
|
||||
}
|
||||
|
||||
installed := r.List()
|
||||
if len(installed) != 1 {
|
||||
t.Fatalf("List() = %d instances, want 1", len(installed))
|
||||
}
|
||||
installedManifest := installed[0].Manifest
|
||||
|
||||
// scanPluginDirectory must resolve the identical manifest from the same
|
||||
// on-disk directory InstallFromZip just promoted into — this is the
|
||||
// precedence-parity OC-0318 requires: whatever the admin validated at
|
||||
// install time must be exactly what the next restart loads.
|
||||
found, scanErr := scanPluginDirectory(dir)
|
||||
if scanErr != nil {
|
||||
t.Fatalf("scanPluginDirectory: %v", scanErr)
|
||||
}
|
||||
if len(found) != 1 {
|
||||
t.Fatalf("scanPluginDirectory found %d plugins, want 1", len(found))
|
||||
}
|
||||
scanned := found[0].Manifest
|
||||
|
||||
if scanned.Name != installedManifest.Name {
|
||||
t.Errorf("scanned.Name = %q, installed.Name = %q", scanned.Name, installedManifest.Name)
|
||||
}
|
||||
if scanned.Entrypoint != installedManifest.Entrypoint {
|
||||
t.Errorf("scanned.Entrypoint = %q, installed.Entrypoint = %q", scanned.Entrypoint, installedManifest.Entrypoint)
|
||||
}
|
||||
if len(scanned.Permissions) != len(installedManifest.Permissions) {
|
||||
t.Errorf("scanned.Permissions = %v, installed.Permissions = %v", scanned.Permissions, installedManifest.Permissions)
|
||||
}
|
||||
if len(scanned.Commands) != len(installedManifest.Commands) {
|
||||
t.Errorf("scanned.Commands = %v, installed.Commands = %v", scanned.Commands, installedManifest.Commands)
|
||||
}
|
||||
}
|
||||
@@ -663,6 +663,21 @@ func (h *Hub) registerNow(c *Client, readableChannelIDs map[int64]bool) {
|
||||
// currently in a voice channel, which is the common case (fresh login).
|
||||
if voiceChID := c.getVoiceChID(); voiceChID != 0 {
|
||||
h.sendVoicePeerKeys(c, voiceChID)
|
||||
// Re-relay THIS client's own stored key back onto VoiceTopic (OC-0316).
|
||||
// voice_e2ee_offer (the room-key-bearing message) is a targeted,
|
||||
// unsequenced send that is silently dropped if this socket was down
|
||||
// when it went out (sendToUserIfInVoiceChannel, voice_e2ee.go) — and
|
||||
// unlike voice_e2ee_announce it has no reconnect-replay recovery
|
||||
// path either. A key rotation sent during the outage otherwise
|
||||
// strands this client on a dead key with no signal and no retry
|
||||
// until the key holder's next periodic rotation. The client's
|
||||
// duplicate-announce handling already re-wraps and re-offers the
|
||||
// CURRENT room key whenever it sees a peer announce a key it
|
||||
// already knows, so re-announcing our own (unchanged) key is enough
|
||||
// to make the key holder re-offer — no client change needed.
|
||||
if key, sig := c.getE2EEPubKey(); key != "" {
|
||||
h.sendToVoiceChannelExcept(voiceChID, c.userID, buildVoiceE2EEAnnounce(c.userID, key, sig))
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -0,0 +1,100 @@
|
||||
package ws
|
||||
|
||||
// oc_0316_voice_e2ee_resume_rotation_test.go — regression test for finding
|
||||
// OC-0316.
|
||||
//
|
||||
// registerNow's resume-time E2EE resync (OC-0276, sendVoicePeerKeys) only
|
||||
// pushes OTHER participants' stored ECDH public keys TO the resuming client.
|
||||
// It never re-relays the resuming client's OWN key back onto the voice
|
||||
// channel. voice_e2ee_offer (the room-key-bearing message) is a targeted,
|
||||
// unsequenced send (sendToUserIfInVoiceChannel) that is silently dropped if
|
||||
// the target's socket is down — so a key rotation that lands while a
|
||||
// participant's WebSocket is blipped is lost forever, and no reconnect
|
||||
// replay tier (buffer or DB) can recover it, because the offer never gets a
|
||||
// seq.
|
||||
//
|
||||
// The client's duplicate-announce handling (handleAnnounceInner) already
|
||||
// re-wraps and re-offers the CURRENT room key whenever it sees an announce
|
||||
// carrying a peer's already-known key — that path exists for exactly this
|
||||
// recovery, but nothing on the server ever triggers it after a resume. This
|
||||
// test pins that the server closes the loop: on a resumed connection that is
|
||||
// still in a voice channel, registerNow must re-broadcast the resuming
|
||||
// client's own stored key onto VoiceTopic (excluding itself) so the key
|
||||
// holder's duplicate-announce branch fires and re-offers the live room key.
|
||||
import (
|
||||
"encoding/json"
|
||||
"testing"
|
||||
|
||||
"github.com/J3vb/OwnCord/Server/auth"
|
||||
)
|
||||
|
||||
func TestRegisterNow_ReannouncesOwnKeyOnResume(t *testing.T) {
|
||||
database := newTeardownTestDB(t)
|
||||
hub := NewHub(database, auth.NewRateLimiter(), nil)
|
||||
|
||||
const (
|
||||
chanID = int64(500)
|
||||
peerID = int64(1) // the key holder (lower user id)
|
||||
userID = int64(2) // the resuming client
|
||||
)
|
||||
|
||||
// Peer (the key holder) is already connected and in the voice channel,
|
||||
// subscribed to VoiceTopic like any real voice participant.
|
||||
peerSend := make(chan []byte, 8)
|
||||
peer := &Client{userID: peerID, send: peerSend, sendHigh: peerSend, sendLow: peerSend}
|
||||
peer.voiceChID = chanID
|
||||
peer.e2eePubKey = "peer-pub-key-b64"
|
||||
hub.mu.Lock()
|
||||
hub.clients[peerID] = peer
|
||||
hub.mu.Unlock()
|
||||
hub.pubsub.Subscribe(peer, VoiceTopic(chanID))
|
||||
|
||||
// userID's PREVIOUS connection, still registered, with a completed voice
|
||||
// join and a stored E2EE key — exactly what registerNow transfers onto a
|
||||
// resuming connection per OC-0270.
|
||||
oldSend := make(chan []byte, 8)
|
||||
old := &Client{userID: userID, send: oldSend, sendHigh: oldSend, sendLow: oldSend}
|
||||
old.voiceChID = chanID
|
||||
old.voiceJoinToken = "join-token"
|
||||
old.voiceJoinCompleted = true
|
||||
old.e2eePubKey = "user-pub-key-b64"
|
||||
old.e2eeSignature = "user-sig"
|
||||
hub.mu.Lock()
|
||||
hub.clients[userID] = old
|
||||
hub.mu.Unlock()
|
||||
|
||||
// The resuming connection: lastSeq > 0 marks this as a network reconnect
|
||||
// (registerNow only transfers voice state on this path) rather than a
|
||||
// fresh login.
|
||||
newSend := make(chan []byte, 8)
|
||||
newC := &Client{userID: userID, send: newSend, sendHigh: newSend, sendLow: newSend, lastSeq: 1}
|
||||
|
||||
hub.registerNow(newC, nil)
|
||||
|
||||
if got := newC.getVoiceChID(); got != chanID {
|
||||
t.Fatalf("precondition failed: resumed client's voice state was not transferred, got channel %d want %d", got, chanID)
|
||||
}
|
||||
|
||||
close(peerSend)
|
||||
var gotReannounce bool
|
||||
for msg := range peerSend {
|
||||
var env struct {
|
||||
Type string `json:"type"`
|
||||
Payload struct {
|
||||
UserID int64 `json:"user_id"`
|
||||
PublicKey string `json:"public_key"`
|
||||
} `json:"payload"`
|
||||
}
|
||||
if err := json.Unmarshal(msg, &env); err != nil {
|
||||
t.Fatalf("unmarshal message sent to peer: %v (raw=%s)", err, msg)
|
||||
}
|
||||
if env.Type == MsgTypeVoiceE2EEAnnounceBC && env.Payload.UserID == userID && env.Payload.PublicKey == "user-pub-key-b64" {
|
||||
gotReannounce = true
|
||||
}
|
||||
}
|
||||
if !gotReannounce {
|
||||
t.Error("resumed client's own E2EE key was never re-broadcast onto the voice channel — " +
|
||||
"OC-0316: the key holder never re-offers the room key after a peer's WS-only resume, " +
|
||||
"so a rotation that lands during the outage strands the resumed client on a dead key")
|
||||
}
|
||||
}
|
||||
@@ -0,0 +1,99 @@
|
||||
package ws
|
||||
|
||||
// reconnect_voice_supplement_coldtier_test.go — regression test for OC-0337.
|
||||
//
|
||||
// liveVoiceEventsSince's cold-tier fallback (serve.go) runs the identical
|
||||
// GetEventsSinceForChannels query as reconnectSelectReplay's cold-tier branch,
|
||||
// with the identical row cap — but reconnectSelectReplay detects a cap hit
|
||||
// ("ORDER BY seq ASC LIMIT n" means a full result silently dropped the NEWEST
|
||||
// rows) and forces a full ready, while liveVoiceEventsSince had no such guard
|
||||
// and handed the truncated window to the client as if it were complete. Since
|
||||
// the query is oldest-first, the dropped rows are always the newest — which,
|
||||
// for a voice room, is exactly where a peer's voice_leave would land after a
|
||||
// run of earlier voice_state joins. The resumed client would then render a
|
||||
// participant who has actually left, with no correction ever sent (the client
|
||||
// tracks only max(seq)).
|
||||
//
|
||||
// This test seeds more persisted voice events on a channel than the
|
||||
// configured cold-tier cap, with a voice_leave as the newest (and therefore
|
||||
// dropped) row, and asserts the supplement degrades to nil — the documented
|
||||
// best-effort miss — rather than returning a window that omits the leave.
|
||||
// A second test pins the boundary the guard must not cross: a complete window
|
||||
// of EXACTLY coldCap rows is not truncated and must be returned in full.
|
||||
|
||||
import (
|
||||
"context"
|
||||
"fmt"
|
||||
"testing"
|
||||
|
||||
"github.com/J3vb/OwnCord/Server/auth"
|
||||
)
|
||||
|
||||
func TestLiveVoiceEventsSince_ColdTierCapHit_DegradesToNil(t *testing.T) {
|
||||
database := newTeardownTestDB(t)
|
||||
ctx := context.Background()
|
||||
|
||||
const chID = int64(555)
|
||||
const coldCap = 3
|
||||
|
||||
// Four persisted voice events on chID, but the cap is 3: the cold-tier
|
||||
// query ("ORDER BY seq ASC LIMIT 3") returns only the three oldest —
|
||||
// three voice_state joins — and silently drops the fourth, a
|
||||
// voice_leave, which is exactly the row a resuming client needs most.
|
||||
types := []string{MsgTypeVoiceState, MsgTypeVoiceState, MsgTypeVoiceState, MsgTypeVoiceLeaveBC}
|
||||
for i, evtType := range types {
|
||||
seq := int64(i + 1)
|
||||
payload := fmt.Appendf(nil, `{"seq":%d,"type":%q,"payload":{"channel_id":%d}}`, seq, evtType, chID)
|
||||
if err := database.PersistEvent(ctx, seq, evtType, chID, payload); err != nil {
|
||||
t.Fatalf("PersistEvent seq=%d: %v", seq, err)
|
||||
}
|
||||
}
|
||||
|
||||
hub := NewHub(database, auth.NewRateLimiter(), nil)
|
||||
hub.SetEventStore(database)
|
||||
hub.ConfigureReplay(0, coldCap) // must run before Run(); this test never calls Run()
|
||||
|
||||
// Ring buffer is untouched (nothing pushed), so EventsSinceFiltered
|
||||
// returns nil and liveVoiceEventsSince must fall through to the cold tier.
|
||||
got := hub.liveVoiceEventsSince(ctx, 0, chID)
|
||||
if got != nil {
|
||||
t.Fatalf("liveVoiceEventsSince: cap-hit cold-tier window must degrade to nil (best-effort miss), got %d event(s) — a truncated window silently drops the newest row (the voice_leave), installing a join whose matching leave was discarded", len(got))
|
||||
}
|
||||
}
|
||||
|
||||
// Codex review on #1436 (P2): a complete window of EXACTLY coldCap rows is
|
||||
// not truncated. Deciding truncation by `len == cap` mistakes it for one and
|
||||
// skips the only reconciliation available for a room outside
|
||||
// allowedChannelIDs, leaving the roster stale although every required event
|
||||
// was returned. The query fetches coldCap+1 rows so truncation is decided by
|
||||
// the presence of the extra row, and a cap-sized complete window replays.
|
||||
func TestLiveVoiceEventsSince_ColdTierExactCap_ReturnsCompleteWindow(t *testing.T) {
|
||||
database := newTeardownTestDB(t)
|
||||
ctx := context.Background()
|
||||
|
||||
const chID = int64(556)
|
||||
const coldCap = 3
|
||||
|
||||
// Exactly three persisted voice events: two joins and the leave that
|
||||
// completes the window. Nothing newer exists, so nothing was dropped.
|
||||
types := []string{MsgTypeVoiceState, MsgTypeVoiceState, MsgTypeVoiceLeaveBC}
|
||||
for i, evtType := range types {
|
||||
seq := int64(i + 1)
|
||||
payload := fmt.Appendf(nil, `{"seq":%d,"type":%q,"payload":{"channel_id":%d}}`, seq, evtType, chID)
|
||||
if err := database.PersistEvent(ctx, seq, evtType, chID, payload); err != nil {
|
||||
t.Fatalf("PersistEvent seq=%d: %v", seq, err)
|
||||
}
|
||||
}
|
||||
|
||||
hub := NewHub(database, auth.NewRateLimiter(), nil)
|
||||
hub.SetEventStore(database)
|
||||
hub.ConfigureReplay(0, coldCap) // must run before Run(); this test never calls Run()
|
||||
|
||||
got := hub.liveVoiceEventsSince(ctx, 0, chID)
|
||||
if len(got) != len(types) {
|
||||
t.Fatalf("liveVoiceEventsSince: a complete window of exactly coldCap=%d rows must be returned in full, got %d event(s) — nil means the guard mistook a complete window for a truncated one", coldCap, len(got))
|
||||
}
|
||||
if last := extractEventType(got[len(got)-1]); last != MsgTypeVoiceLeaveBC {
|
||||
t.Fatalf("last replayed event = %q, want %q", last, MsgTypeVoiceLeaveBC)
|
||||
}
|
||||
}
|
||||
+21
-1
@@ -638,10 +638,30 @@ func (h *Hub) liveVoiceEventsSince(ctx context.Context, afterSeq uint64, chID in
|
||||
raw = buf
|
||||
} else if esp := h.eventStore.Load(); esp != nil {
|
||||
es := *esp
|
||||
persisted, err := es.GetEventsSinceForChannels(ctx, int64(afterSeq), []int64{chID}, h.maxColdReplayLimit()) //nolint:gosec // afterSeq is a sequence counter bounded well below MaxInt64
|
||||
coldCap := h.maxColdReplayLimit()
|
||||
// Fetch one row past the cap so truncation is decided by the presence
|
||||
// of that extra row, not by len == cap: a complete window of exactly
|
||||
// coldCap rows is not truncated and must replay in full (Codex review
|
||||
// on #1436). A result of at most coldCap rows is therefore complete.
|
||||
persisted, err := es.GetEventsSinceForChannels(ctx, int64(afterSeq), []int64{chID}, coldCap+1) //nolint:gosec // afterSeq is a sequence counter bounded well below MaxInt64
|
||||
if err != nil {
|
||||
return nil
|
||||
}
|
||||
if len(persisted) > coldCap {
|
||||
// Same failure mode reconnectSelectReplay guards against above:
|
||||
// the query is "ORDER BY seq ASC LIMIT n", so a result past the
|
||||
// cap means the range exceeds it and any cap-sized window would
|
||||
// have silently dropped the NEWEST rows — for a voice room, quite
|
||||
// possibly the peer's voice_leave. Replaying a truncated window
|
||||
// would install a join whose matching leave was discarded, which
|
||||
// is worse than the documented best-effort miss this function
|
||||
// already returns on a plain lookup failure. A full ready isn't
|
||||
// available here (registerNow already ran before this supplement
|
||||
// runs), so nil is the correct degradation.
|
||||
slog.Warn("ws liveVoiceEventsSince: cold-tier supplement exceeds the row cap, skipping truncated window",
|
||||
"chID", chID, "after_seq", afterSeq, "cap", coldCap)
|
||||
return nil
|
||||
}
|
||||
raw = make([][]byte, 0, len(persisted))
|
||||
for _, p := range persisted {
|
||||
raw = append(raw, p.Payload)
|
||||
|
||||
@@ -63,7 +63,7 @@ Planning documents are not trackers. Do not read a defect count out of one.
|
||||
| Phase order and gates | [repo-health-roadmap-2026-08-23](repo-health-roadmap-2026-08-23.md) |
|
||||
| Current measured baseline | [b0-baseline-2026-08-25](b0-baseline-2026-08-25.md) |
|
||||
|
||||
Ledger at 2026-08-25: **306 fixed / 38 open / 3 declined / 1 duplicate = 348**.
|
||||
Ledger at 2026-08-25: **315 fixed / 30 open / 3 declined / 1 duplicate = 349**.
|
||||
All 38 open records still resolved to a live `file:line` at
|
||||
`5cc0888964e26276d1aca145e83270a2c1b9febd` when that sweep was run — it was a
|
||||
manual pass, not something a command reproduces. What the tooling does check:
|
||||
|
||||
@@ -216,7 +216,7 @@ authority over the leftovers listed below. B1 is unblocked.
|
||||
which also refutes its own header note: repository-settings writes were **not**
|
||||
blocked from the agent sandbox.
|
||||
- Step 8: individual adjudication of the 38 open `OC-*` records. The count was
|
||||
verified as **306 fixed / 38 open / 3 declined / 1 duplicate = 348**, matching
|
||||
verified as **315 fixed / 30 open / 3 declined / 1 duplicate = 349**, matching
|
||||
the register, and a staleness pass confirmed **all 38 still resolve to a live
|
||||
`file:line`** at this commit — none is superseded by later work, so all 38 are
|
||||
genuinely open (11 medium, 27 low, all from hunt `general-2026-08-22-b`).
|
||||
|
||||
@@ -4,9 +4,9 @@
|
||||
**Base commit:** `64d2e108` (`dev`, post-PR #1425); `main` @ `b7d388a3` =
|
||||
`v1.2.0-alpha.4` — claims verified at `64d2e108`; the branch was rebased
|
||||
onto `dd7ed091` (#1432) before merge
|
||||
**Status:** in progress — entry gate 1 of 3 met at draft time (see below); B2-0
|
||||
landed 2026-08-28 (evidence in its section); B2-1 is next. Update this line,
|
||||
not only the step table, when a step lands.
|
||||
**Status:** in progress — entry gate 1 of 3 met at draft time (see below); B2-0,
|
||||
B2-1 and B2-8 landed 2026-08-28 (evidence in their sections); B2-2 is next.
|
||||
Update this line, not only the step table, when a step lands.
|
||||
|
||||
Primary inputs:
|
||||
|
||||
@@ -196,10 +196,10 @@ runs the new test under `go test ./...`), `npm run check:client`.
|
||||
|
||||
**Evidence, 2026-08-28** — HP-2 question 1 cites this block:
|
||||
|
||||
- Branch `feat/b2-1-epoch1-fixtures` from `dev` `fb6b51a0`; PR #1435 to `dev`.
|
||||
Record the pre-squash head at merge time:
|
||||
`gh api repos/J3vb/OwnCord/pulls/1435 --jq .head.sha` (before) or
|
||||
`git ls-remote origin refs/pull/1435/head` (after).
|
||||
- Branch `feat/b2-1-epoch1-fixtures` from `dev` `fb6b51a0`; PR #1435 to `dev`,
|
||||
squash-merged 2026-08-28 as `1fe3df79`. Pre-squash head at merge time:
|
||||
`069412db` (`git ls-remote origin refs/pull/1435/head` →
|
||||
`069412dbbfb9fa11318a8a6f16af251563e78d09`); HP-2 question 1 cites it.
|
||||
- Pre-squash commits: retirement `dd638f1c` (own commit, before capture);
|
||||
fixtures `54cae614` (capture), `c0719519` (end-of-journey barriers,
|
||||
present-form optionals), `d5fe06e5` (null forms of `auth_ok`/`member_join`
|
||||
@@ -426,6 +426,48 @@ Run `bughunt-fix` on exactly these nine (test-first, per-file agents, one
|
||||
platform seam) is re-tagged in the issue register with the reason, per the roadmap's
|
||||
phase execution pattern, not silently skipped.
|
||||
|
||||
**Evidence, 2026-08-28** — branch `fix/b2-8-findings-2026-08-28` from `dev`
|
||||
`1fe3df79`; PR #1436 to `dev`.
|
||||
|
||||
- Re-verified against HEAD first, one read-only agent per finding, before any
|
||||
fix: all nine still open; none needs B7 — the `B2/B7` tags in the issue
|
||||
register are scheduling hints, not a platform-seam dependency, so nothing
|
||||
was re-tagged. Two coordinates had drifted after #1435
|
||||
(`dispatcher.ts` 1077→1069 and 688→687); the rest were exact.
|
||||
- Two `bughunt-fix` waves so the same-run overlap guard never fired: wave 1
|
||||
(OC-0311/0315, 0316, 0317, 0318, 0322, 0337, 0338 — seven file clusters),
|
||||
wave 2 (OC-0328, whose fix also edits the `dispatcher.ts` call site that
|
||||
wave 1's first cluster owned). 9/9 fixed test-first in 8 commits
|
||||
(`a231108f`, `cd4cc850`, `7c159c11`, `bbbaeed4`, `e95c57a4`,
|
||||
`7aeab0ed`, `073e8799`, `3e74c968`), each revert-proven by its prove
|
||||
agent and then independently by `verify-fixes.mjs`: 8/8 PASS, red then green (4 client, 4 server), plus a hand RED/GREEN of the wazero-tagged OC-0318 parity test that the untagged run cannot exercise.
|
||||
- Test-design facts worth keeping: OC-0315's RED needs a pinned non-UTC zone
|
||||
(Asia/Tokyo via the `renderers.test.ts` probe pattern) because
|
||||
`Date.parse` and `parseTimestamp` agree under CI's UTC; OC-0338's pinning
|
||||
test decodes with `BurntSushi/toml` directly from an untagged file because
|
||||
`tryLoadPluginTOML` is `//go:build wazero`; OC-0318's default-build RED is
|
||||
the both-manifests rejection, and its install/scan precedence parity is a
|
||||
wazero-tagged test (CI runs `go test -tags wazero ./plugin/...`).
|
||||
- Also in this PR: the B2-1 evidence block above records pre-squash head
|
||||
`069412db`, and ledger OC-0349 (open, low) records the `voice_join`
|
||||
ordering hazard B2-1 found (`Server/ws/voice_join.go:498` vs
|
||||
`:445/:523/:546`) — a behaviour change left for a later fix batch.
|
||||
- Gates at `3e74c968`: `check:server` plus `go vet -tags wazero ./...` and
|
||||
`go test -tags wazero -count=1 ./plugin/...`, `check:client` plus
|
||||
`npx knip` (blocking in CI since 2026-08-04 but not part of
|
||||
`check:client`), `check:docs`, `check:hygiene` — all exit 0.
|
||||
- Codex review on #1436 (P2): OC-0337’s cap guard treated a complete window of
|
||||
exactly `coldCap` rows as truncated and skipped the reconciliation; fixed
|
||||
test-first in the follow-up commit by fetching `coldCap+1` rows and discarding
|
||||
only when the extra row exists. The sibling `reconnectSelectReplay` keeps its
|
||||
`>=` form deliberately — there the over-approximation only costs a full
|
||||
`ready`, never data.
|
||||
- Found, not fixed (workflow tooling, not B2): the `bughunt-fix` gate list
|
||||
still runs `npm run format:check` from `Client/` (removed in B1-3; the
|
||||
formatting gate is root-scoped) and omits `knip`, so every run ends with a
|
||||
phantom `gate: FAIL` — every real command passed. Recorded in the skill
|
||||
observation log for the workflow script.
|
||||
|
||||
## B2-9 — Security owners and acceptance tests
|
||||
|
||||
The seven local reports in `docs/security-findings/` (gitignored, never
|
||||
|
||||
@@ -37,7 +37,7 @@ baseline is **truthful, reproducible, and sufficient to begin B1**.
|
||||
| Docker build + boot smoke | unavailable | pass | **pass**, 50.1 MB, boots `:8443` | `ENV-02` closed |
|
||||
| Largest lazy chunk | — | budget in B7 | 1,998.25 kB min / 1,344.96 kB gzip | measured |
|
||||
| Generated/doc drift | refresh in B0 | 0 | **0** — `sqlc-verify`, `protocol-verify` green | CI |
|
||||
| Ledger path resolution | — | 0 dead | **0 dead paths / 348 records** | re-verified at `6a1561fa` |
|
||||
| Ledger path resolution | — | 0 dead | **0 dead paths / 349 records** | re-verified at `6a1561fa` |
|
||||
| Desktop/browser/device matrix | incomplete | 100% by B10 | **incomplete** | B6–B8 |
|
||||
| 250/100/25 capacity profile | unproven | met by B6 | **unproven** | `S-14`, B6 |
|
||||
| Upgrade/rollback/restore | unproven | green by B6 | **unproven** | B6 |
|
||||
@@ -67,11 +67,11 @@ Open ledger, re-verified at `6a1561fa`:
|
||||
|
||||
| Status | Count |
|
||||
| --------- | ------- |
|
||||
| fixed | 306 |
|
||||
| open | **38** |
|
||||
| fixed | 315 |
|
||||
| open | **30** |
|
||||
| declined | 3 |
|
||||
| duplicate | 1 |
|
||||
| **total** | **348** |
|
||||
| **total** | **349** |
|
||||
|
||||
Of the 38 open records:
|
||||
|
||||
|
||||
@@ -76,11 +76,11 @@ together as if each row were a unique defect:
|
||||
|
||||
| Status | Count |
|
||||
| --------- | ------: |
|
||||
| Fixed | 306 |
|
||||
| Open | 38 |
|
||||
| Fixed | 315 |
|
||||
| Open | 30 |
|
||||
| Declined | 3 |
|
||||
| Duplicate | 1 |
|
||||
| **Total** | **348** |
|
||||
| **Total** | **349** |
|
||||
|
||||
All 38 open records are listed below. Closing a planning row does not close an
|
||||
`OC-*` record: the implementation, regression test, focused verification, full
|
||||
|
||||
Reference in New Issue
Block a user