From d3526968bbdfd78b969abd0f2a47b96ccf9c2930 Mon Sep 17 00:00:00 2001 From: J3vb <192430104+J3vb@users.noreply.github.com> Date: Sun, 9 Aug 2026 11:50:12 +0200 Subject: [PATCH] release: v1.2.0-alpha.2 (#1333) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * docs: add bug-detection improvements plan Plan for mechanical bug detection alongside the agentic hunt: activate the 14 unused Go fuzz harnesses, the configured-but-never-run Stryker setup, and browser-mode vitest; encode recurring bug classes as semgrep rules; add model-based and fault-injected ordering tests; add a persistent seen-ledger and sibling-sweep lens to the hunt. All local-only and on demand - fuzz crashers are working reproducers, and this repo is public. Co-Authored-By: Claude Opus 5 (1M context) * build: add make fuzz target and ignore mutation-test output `go test ./...` runs each Fuzz* function against its committed seed corpus only - one pass per seed, zero generated inputs - so the 17 fuzz harnesses in Server/ have never actually fuzzed. `make fuzz` enumerates every target and runs each with a time budget (Go fuzzes one target per package per invocation, hence the loop). Local-only by design: a crasher is a working reproducer and this repo is public. Also gitignore Client/tauri-client/.stryker-tmp/ and reports/ - a Stryker run left 200+ untracked files, and a surviving-mutant report maps exactly which behaviour nothing tests. Co-Authored-By: Claude Opus 5 (1M context) * test(client): pin reconnect auth-frame and replay-dedup arming Stryker found 14 surviving mutants across ws.ts:413/422/428 - the auth frame built on reconnect. Every condition there could be flipped with all 4777 tests still green: the replay-dedup arming guard, the resume-vs-fresh-connect ternary, and the conditional active_channel_id spread. Seven tests through the public send/isReplaying surface, no new exports. Two isolate each half of the `reconnectAttempt > 0 && lastSeq > 0` AND condition - the combination no existing test reached, and the one an && -> || mutant walked straight through. Verified by flipping the line 413 guard to `if (true)`: 3 of 7 fail, revert restores green. Co-Authored-By: Claude Opus 5 (1M context) * docs: record two fuzz corpus traps Interrupting a fuzz run manufactures a false crasher: Go cannot distinguish a worker that crashed on an input from one killed externally, so it saves the in-flight input to testdata/fuzz/ as a suspect. It looks exactly like a real security finding. Replay before believing it. And committed seed corpus shares the testdata/fuzz// directory with any false crasher, so clearing one by removing the directory deletes the seeds too. Co-Authored-By: Claude Opus 5 (1M context) * feat(client): enforce three prose invariants as ESLint rules CLAUDE.md documents the voice-supersession, E2EE staleness and dispatcher invariants in English. English fails no build, and bug hunts keep rediscovering the same classes. Five rules encode them as an inline flat-config plugin - no new dependency, and `npx eslint src/` is already a blocking CI gate. - no-leave-voice-when-superseded: a global leaveVoice() inside a branch that already confirmed supersession tears down the newer live session - e2ee-epoch-needs-keypair-check: a non-key-holder never bumps the epoch, so an epoch-only staleness guard cannot see a restarted session - e2ee-verified-status-literal: keeps "verified" tied to a hand-written call site that earned it, never a computed status - no-identity-scope-fallback: a `?? 0` placeholder scope mints a keypair under the wrong account - no-store-write-in-ws-on: page-local ws.on handlers may read stores, not write them Each rule proven to fire by reintroducing the historical bug shape and reverting; RuleTester cases cover both the real shapes that must stay clean and the bug shapes that must not. A fourth candidate - await-then-stale-snapshot - was declined as not AST-expressible: whether an await needs a guard, and whether the guard is sufficient, is intent rather than shape, and the rule would flag most of the already-correct guard code in livekitSession.ts. Co-Authored-By: Claude Opus 5 (1M context) * docs: correct dispatcher invariant, record Tier 2 as shipped The client CLAUDE.md claimed ws.on(...) appears only in dispatcher.ts. Eight handlers across main.ts, MainPage.ts and ChannelController.ts say otherwise - page-local UI (ringing, overlays, slow-mode timers) legitimately subscribes. The real invariant is narrower: dispatcher is the single path by which server events WRITE to domain stores. That is what local/no-store-write-in-ws-on enforces, and the doc now matches the code. Also record that Tier 2 shipped as ESLint rules rather than semgrep, and why the fourth candidate was declined. Co-Authored-By: Claude Opus 5 (1M context) * fix(client): move the status-picker dot onto the avatar corner The corner dot on the user bar avatar was a static hardcoded-green div — never reflected real status and did nothing on click. Removed it and relocated the actual StatusPicker trigger dot (real color, opens the status dropdown) to that same corner instead of its own row. The "Online"/"Idle"/... text label under the username is unchanged. Co-Authored-By: Claude Sonnet 5 * fix(client): return the saved password over IPC again The remember-password box saved a password the client could never read back. Hardening had put #[serde(skip)] on CredentialData::password, so load_credential returned a record whose password was always absent and the login form could not prefill it — the box appeared to work and silently did nothing. Drop the skip and carry the field through the TS wrapper, which now maps a non-string password to undefined rather than trusting the payload. Co-Authored-By: Claude Sonnet 5 * feat(client): add an auto-connect checkbox to the login form Auto-connect already existed end to end — ServerProfile.autoConnect, setAutoLogin(), and the boot auto-login block with its cancel overlay — but was only reachable through the zap button on a server card. This surfaces the same state as a checkbox under Remember password, where users look for it. Ticking it forces Remember password on and disables it: boot auto-login replays the stored token, which saveCredential only writes when the password is remembered, so the two cannot be set independently without producing a setting that silently does nothing. Unticking is guarded. setAutoLogin(null) clears autoConnect on every profile, so a bare toggle-off would wipe another server's setting; the clear now only fires when this profile is the current holder. The guard lives in ensureProfileExists, which all four auth paths already route through. Also consume the password restored in the previous commit, so selecting a saved server prefills it instead of leaving the field blank. Co-Authored-By: Claude Sonnet 5 * chore(release): bump client to 1.2.0-alpha.2 The client version is not derived from the tag — release.yml's verify-versions job compares the tag against package.json and tauri.conf.json and fails the release if they drift, so all five manifests (both lockfiles included) move together. Also refreshes the literal version in the README and docs build examples, and closes the Unreleased changelog section as v1.2.0-alpha.2. Co-Authored-By: Claude Sonnet 5 * docs(changelog): record the three bug-hunt sweeps in v1.2.0-alpha.2 PRs #1328, #1331 and #1332 merged to main after v1.2.0-alpha.1 was tagged and closed 233 verified defects between them, but none of the three left an entry in the curated changelog — the generated list covers commits, this file covers behaviour, and nothing bridged the two. Verified unreleased by ancestry rather than by date (none of the three merge commits is an ancestor of v1.2.0-alpha.1), so all of it ships for the first time in alpha.2. Nine entries grouped by subsystem, leading with the changes an operator or user would actually notice: the 24h-retention desync, the avatar- deleting orphan sweep, the zero-byte restore truncation, the six hot-mic paths, and the TOFU re-pin that would have warned every install at once. Co-Authored-By: Claude Sonnet 5 * test(client): drop the e2e assertion for the removed user-bar status dot (#1334) 26b46cc removed the hardcoded-green `.status-dot` div from the user bar avatar and relocated the real StatusPicker trigger dot into that corner, adding "status picker dot sits on the avatar" to cover the new element. The old "user bar has status dot" test was left behind and now fails on an element that no longer exists by design. The replacement test already asserts the corner dot is present and visible, so removing the stale one loses no coverage. Claude-Session: https://claude.ai/code/session_01Rkv9dVo5YEYArqrDRfW41w Co-authored-by: Claude --------- Co-authored-by: Claude Opus 5 (1M context) --- .gitignore | 5 + CHANGELOG.md | 122 +++++- Client/tauri-client/CLAUDE.md | 10 +- Client/tauri-client/eslint-rules.js | 408 ++++++++++++++++++ Client/tauri-client/eslint.config.js | 51 ++- Client/tauri-client/package-lock.json | 4 +- Client/tauri-client/package.json | 2 +- Client/tauri-client/src-tauri/Cargo.lock | 2 +- Client/tauri-client/src-tauri/Cargo.toml | 2 +- .../tauri-client/src-tauri/src/credentials.rs | 12 +- Client/tauri-client/src-tauri/tauri.conf.json | 2 +- Client/tauri-client/src/components/UserBar.ts | 12 +- Client/tauri-client/src/lib/credentials.ts | 4 +- Client/tauri-client/src/main.ts | 47 +- Client/tauri-client/src/pages/ConnectPage.ts | 15 +- .../src/pages/connect-page/LoginForm.ts | 40 ++ .../src/pages/connect-page/ServerPanel.ts | 6 +- Client/tauri-client/src/styles/app.css | 17 +- .../tauri-client/tests/e2e/user-bar.spec.ts | 12 +- .../tests/unit/connect-page.test.ts | 76 +++- .../tests/unit/credentials.test.ts | 20 +- .../tests/unit/eslint-rules.test.ts | 286 ++++++++++++ .../tests/unit/server-panel.test.ts | 28 +- .../tests/unit/ws-active-channel.test.ts | 250 +++++++++++ README.md | 4 +- Server/Makefile | 27 +- docs/api.md | 2 +- docs/deployment.md | 4 +- docs/plans/bug-detection-improvements.md | 265 ++++++++++++ docs/quick-start.md | 4 +- 30 files changed, 1648 insertions(+), 91 deletions(-) create mode 100644 Client/tauri-client/eslint-rules.js create mode 100644 Client/tauri-client/tests/unit/eslint-rules.test.ts create mode 100644 Client/tauri-client/tests/unit/ws-active-channel.test.ts create mode 100644 docs/plans/bug-detection-improvements.md diff --git a/.gitignore b/.gitignore index 1eab611b..cc3c0bcd 100644 --- a/.gitignore +++ b/.gitignore @@ -27,6 +27,11 @@ docs/research/ docs/superpowers/ /skills/ +# Mutation-testing output (npm run test:mutate). Local-only by design: a +# surviving-mutant report maps exactly which behaviour nothing tests. +Client/tauri-client/.stryker-tmp/ +Client/tauri-client/reports/ + # Server runtime artifacts Server/chatserver.exe Server/chatserver.exe~ diff --git a/CHANGELOG.md b/CHANGELOG.md index 0b8ab47d..3e83cf18 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,8 +5,128 @@ tooling (`npm run changelog`) auto-generates entries from commit messages on each release; this file is the curated counterpart that calls out behavioural changes operators must know about. -## Unreleased +## v1.2.0-alpha.2 +- **feat(client):** the login form has an **Auto connect** checkbox under + Remember password. Ticking it makes that server connect automatically on + launch — the same setting as the auto-login button on a server card, so + the two stay in sync, and as before only one server can be auto-connect + at a time. + Ticking it also forces Remember password on and locks it: auto-connect + replays the stored token, which is only written when the password is + remembered, so the two cannot be set independently without producing a + setting that silently does nothing. +- **fix(client):** Remember password works again. The password was saved to + the OS keyring but never returned to the client over IPC, so the login + form could not prefill it — the box appeared to work and did nothing. +- **fix:** three bug-hunt sweeps closed **233 verified defects** since + `v1.2.0-alpha.1` — 26 in #1328, 107 in #1331, 100 in #1332 — each fixed + test-first, with the failing assertion watched red against the unpatched + code before the patch landed. The behavioural consequences worth knowing + about are listed in the nine entries below. +- **server:** WS hub reconnect and replay hardening (#1328, #1331). + Cold-tier replay used to truncate silently instead of forcing a full + ready, and a retention-pruned event log was accepted outright as a + complete resume — the highest-impact fix in #1331, since any client whose + reconnect gap crossed the 24h retention default was permanently desynced. + Resume also silently dropped the focused channel's topic subscription, + stopping message delivery until the user manually switched channels; it + is now restored during the handshake. `visibilityChangeSeq` can now only + move forward across its three writers — it previously could regress and + skip a required resync. +- **server:** voice/E2EE key-holder election and audience gating (#1328, + #1331) — three key-holder desync bugs (no client demotion path, peer keys + cleared on reconnect, missing re-election on the webhook and + fresh-reconnect paths), plus re-election wired into the sweep and + channel-cleanup paths. Voice events were READ-filtered while membership + is CONNECT-only, so participants in that gap silently missed + `voice_leave`, stalling key-holder election and forward-secrecy rotation. + Deleting a channel now evicts its voice participants first — the cleanup + function existed but had zero production callers, so the FK cascade used + to strand them silently. Moderator mute/deafen now survives a + voice-channel switch; joins to non-voice channels are rejected; archived + channels are read-only and unjoinable. +- **security(server):** roles/permissions (#1328, #1331) — `UpdateRole` + allowed position collisions that `CreateRole` already rejected, so tied + positions could read as equal rank in every hierarchy comparison; it now + matches `CreateRole`'s validation. `can_send` is now recomputed per client + on every role/override change, so a permission change takes effect for + connected clients immediately rather than waiting on a reconnect. +- **server:** attachments and admin data-safety (#1331) — migration **030** + unlinks attachments on message delete instead of cascading, so a cascaded + channel/DM delete no longer strands uploaded files on disk with no + reclamation path. The 15-minute orphan-attachment sweep was deleting every + avatar in the instance (avatars are, by design, attachments with no + message link) on its first tick past the grace period, permanently 404ing + every profile picture; a second bug in the same sweep collapsed the + one-hour grace period to effectively zero, from a TEXT-comparison mismatch + between an RFC3339 cutoff and SQLite's own timestamp format. A failed + backup restore used to truncate the live database to zero bytes with no + rollback, while the server kept answering requests against the now-closed + DB and falsely claimed a restart was underway — it now restores the + pre-restore safety copy on failure and requests the restart honestly. + Also fixed: personal data is cleared on account deletion, banned users are + excluded from owner lookup, the silent 1000-member roster cap is gone, and + a sender's own read state now advances on send. Migration applies + automatically on first start; no operator action needed. +- **protocol:** a new READ-gated `active_channel_id` auth field (#1331) + restores the focused-channel subscription during the reconnect handshake + itself, closing the window before the post-`auth_ok` `channel_focus` round + trip lands. `protocol.md` also corrects the presence table, which had + incorrectly documented all presence events as sequenced. Older + clients/servers are unaffected — it is a new, ignorable field. +- **security(client):** identity/TOFU and transport (#1332) — an in-flight + change to scope the identity keypair by host *and* user id would have + re-minted a fresh key on every existing install, firing the TOFU "verify + out-of-band" re-pin warning at the entire alpha population simultaneously, + exactly the pattern that teaches users to click through the one warning + meant to matter. The legacy host-only key is now adopted into the scoped + name instead, saving before deleting so a partial failure cannot strand a + user with neither key. Switching hosts carried the previous server's + bearer token forward into the next login request; `api.setConfig` now + drops it when the host changes without a replacement. A hand-copied, + un-lowercased host normalizer in `main.ts` meant an uppercase hostname's + cert-mismatch *reject* path skipped `disconnect()`/`clearAuth()`, leaving + a user who refused a changed certificate still connected to that server — + the single lowercased implementation in `ws.ts` is now shared everywhere. +- **fix(client):** voice mic/camera reliability (#1331, #1332) — six + separate paths could republish the microphone without checking the user's + mute state (the audio-device fallback, selecting "Default" input, + un-deafening, `retryMicPermission`, a stale PTT ownership latch, and + auto-reconnect's `restoreLocalVoiceState`), each producing a hot mic while + every remote UI still showed the user muted; all now route through + `isMicPolicyGated()`. Camera and screenshare kept publishing to the SFU + after the user turned them off during the OS device picker. Enhanced Noise + Suppression silently disabled the input-volume slider and VAD gate because + `livekit-client`'s own `replaceTrack` call landed after ours. A key-holder + promotion arriving mid voice-setup was clobbered, ejecting the joiner + after a timeout only it could have resolved. +- **fix(client):** messaging and store reliability (#1328, #1331, #1332) — + sequenced DMs could jump the FIFO ahead of `sendHigh`, permanently losing + an event dropped before flush. A full-ready resync left every loaded + channel with a permanent hole in its history, because that tier never + replays `chat_message` frames; loaded windows are now invalidated and the + active channel refetched. The WS error handler only bannered + `RATE_LIMITED` and `FORBIDDEN`, so every other server error code — for + example a rejected `chat_edit` — was dropped in silence while the + optimistic "Message edited" toast still fired. A message whose + `chat_send_ok` was lost to the same disconnect that forced a resync could + render twice; the optimistic row's id-based dedup now shares the + content-based match predicate `addMessage` already used. Replay detection + compared the server's `created_at` against the client's own clock, so a + self-hosted server without NTP made every live message after a reconnect + look like a replay and silently killed its notification; both sides now + use an estimated server-time skew. +- **fix(client):** UI defects (#1331, #1332) — the quick-switcher could + mount a second overlay, orphaning a body-mounted backdrop that blocked all + input until reload. The status-picker stylesheet targeted a root element + the component never toggles; a same-branch repair then left the status dot + itself 0×0 and unclickable, now fixed together with a test pinning the + stylesheet to the classes the component actually emits. The attachment + remove button and the failed-send Retry/Discard buttons did nothing; + drag-reorder's phantom-drag latch and permission gate are fixed; keyboard + Tab could escape every modal because hidden (`display: none`) controls + were still counted as focusable. - **fix(client):** the user profile popup is styled correctly again (`a308f81`). - **fix(client):** Vite no longer watches `src-tauri/`, so a running dev diff --git a/Client/tauri-client/CLAUDE.md b/Client/tauri-client/CLAUDE.md index ad19bf0f..b759ffc5 100644 --- a/Client/tauri-client/CLAUDE.md +++ b/Client/tauri-client/CLAUDE.md @@ -18,8 +18,14 @@ Rust backend in `src-tauri/` for native APIs only. LiveKit handles voice/video. Native Web Storage shadows jsdom's `localStorage` and fails ~478 tests that have nothing to do with your change. That is a local toolchain artifact, not a regression — do not "fix" those failures. CI pins Node 20. -- `src/lib/dispatcher.ts` is the single WS-event entry point: server events - reach the stores only through a `ws.on(...)` subscription registered there. +- `src/lib/dispatcher.ts` is the single WS-event entry point **into the + stores**: server events reach domain stores only through a `ws.on(...)` + subscription registered there. Other modules do register their own + `ws.on(...)` handlers for page-local UI (`main.ts`, `MainPage.ts`, + `ChannelController.ts` — ringing, overlays, slow-mode timers); that is fine + as long as they only *read* store state. Writing a store from one of those + handlers is the violation, and `local/no-store-write-in-ws-on` now fails the + build on it. - Voice sessions are superseded, not cancelled. `LiveKitSession` re-entry points check whether a newer attempt owns the shared state before tearing anything down, so cleanup in an aborted path must be scoped to that attempt's diff --git a/Client/tauri-client/eslint-rules.js b/Client/tauri-client/eslint-rules.js new file mode 100644 index 00000000..c16188b0 --- /dev/null +++ b/Client/tauri-client/eslint-rules.js @@ -0,0 +1,408 @@ +// Custom ESLint rules that turn three of the invariants documented in prose in +// CLAUDE.md into enforced, test-covered lint rules. Each rule is scoped (via +// `files:` in eslint.config.js) to only the module(s) its invariant governs — +// see the per-rule `meta.docs.description` for the invariant it encodes and +// tests/unit/eslint-rules.test.ts for the real-code shapes it was proven +// against (both the shapes that must stay clean and the historical bug shapes +// it must catch). +// +// Plain JS, ESM, no build step — eslint.config.js imports this directly. + +/** True when `node` is a `this.(...)` call. */ +function isThisMethodCall(node, methodName) { + return ( + node !== null && + node.type === "CallExpression" && + node.callee.type === "MemberExpression" && + node.callee.object.type === "ThisExpression" && + !node.callee.computed && + node.callee.property.type === "Identifier" && + node.callee.property.name === methodName + ); +} + +/** True when `node` is a `this.` member access. */ +function isThisMember(node, propertyName) { + return ( + node !== null && + node.type === "MemberExpression" && + node.object.type === "ThisExpression" && + !node.computed && + node.property.type === "Identifier" && + node.property.name === propertyName + ); +} + +function isFunctionNode(node) { + return ( + node.type === "FunctionDeclaration" || + node.type === "FunctionExpression" || + node.type === "ArrowFunctionExpression" + ); +} + +// ───────────────────────────────────────────────────────────────────────── +// Rule: no-leave-voice-when-superseded +// +// Invariant (CLAUDE.md): "Voice sessions are superseded, not cancelled. +// LiveKitSession re-entry points check whether a newer attempt owns the +// shared state before tearing anything down, so cleanup in an aborted path +// must be scoped to that attempt's own room — a global leaveVoice() there +// kills the live session." +// +// livekitSession.ts encodes "this attempt was superseded" with exactly two +// guard predicates, always used the same way: `this.reconnectSuperseded(...)` +// (true = superseded) and `!this.isStateConnected(...)` (negated = true when +// superseded). Once either guard has confirmed supersession, the historical +// bug (see the fix that introduced disconnectSupersededLocalRoom / +// generation-guarded leaveVoice calls) was calling the global +// `this.leaveVoice()` inside that same branch, tearing down whichever session +// currently owns the shared state — which, once superseded, is a newer +// attempt's live session, not this one. +// ───────────────────────────────────────────────────────────────────────── + +/** True when `test` (walking through &&/||) asserts "this attempt IS + * superseded" via one of the two named guards used throughout the file. */ +function testSignalsSuperseded(test) { + if (test === null) return false; + if (test.type === "LogicalExpression") { + return testSignalsSuperseded(test.left) || testSignalsSuperseded(test.right); + } + if (isThisMethodCall(test, "reconnectSuperseded")) return true; + if (test.type === "UnaryExpression" && test.operator === "!") { + return isThisMethodCall(test.argument, "isStateConnected"); + } + return false; +} + +const noLeaveVoiceWhenSuperseded = { + meta: { + type: "problem", + docs: { + description: + "Disallow this.leaveVoice() inside a branch that already confirmed this connect/reconnect " + + "attempt was superseded. Voice sessions are superseded, not cancelled — once reconnectSuperseded() " + + "or !isStateConnected() is true, `_state` may already belong to a newer, live attempt, and " + + "leaveVoice() there tears that live session down instead of the aborted one.", + }, + schema: [], + messages: { + unsafeLeaveVoice: + "this.leaveVoice() must not run once this attempt is known to be superseded — it acts on " + + "whichever session currently owns `_state`, which may now be a newer, live attempt. Disconnect " + + "only this attempt's own room instead (e.g. disconnectSupersededLocalRoom(localRoom) / " + + "localRoom.disconnect()), or simply return without calling it.", + }, + }, + create(context) { + return { + CallExpression(node) { + if (!isThisMethodCall(node, "leaveVoice")) return; + let child = node; + let parent = node.parent; + while (parent) { + if (isFunctionNode(parent)) return; // left the enclosing method — stop + if ( + parent.type === "IfStatement" && + child === parent.consequent && + testSignalsSuperseded(parent.test) + ) { + context.report({ node, messageId: "unsafeLeaveVoice" }); + return; + } + child = parent; + parent = parent.parent; + } + }, + }; + }, +}; + +// ───────────────────────────────────────────────────────────────────────── +// Rule: e2ee-epoch-needs-keypair-check +// +// Invariant (CLAUDE.md): "Anything touching livekitE2EE.ts ... must preserve +// the epoch/keypair staleness guards." +// +// Every async E2EE operation that resumes after an await re-checks it is +// still the current attempt before writing shared state. The historical bug +// (see the fix for handleOfferInner / handleAnnounceInner) compared only +// `this._e2eeEpoch !== epochBefore` — insufficient, because a non-key-holder +// never bumps the epoch, so a torn-down-then-restarted session can resume +// with the epoch unchanged in both the old and new session. The fix requires +// ALSO comparing keypair identity (`this._ecdhKeyPair !== keypair`). This +// rule requires both checks to appear together in the same guard. +// ───────────────────────────────────────────────────────────────────────── + +/** True when `test` (walking through &&/||) contains `this. !== X` + * (in either operand order). */ +function containsStrictInequality(test, prop) { + if (test === null) return false; + if (test.type === "LogicalExpression") { + return containsStrictInequality(test.left, prop) || containsStrictInequality(test.right, prop); + } + if (test.type === "BinaryExpression" && test.operator === "!==") { + return isThisMember(test.left, prop) || isThisMember(test.right, prop); + } + return false; +} + +const e2eeEpochNeedsKeypairCheck = { + meta: { + type: "problem", + docs: { + description: + "Require this._ecdhKeyPair identity checks alongside this._e2eeEpoch staleness checks. A " + + "non-key-holder session never bumps the epoch, so an epoch-only comparison cannot detect a " + + "torn-down-then-restarted session resuming after an await — only the keypair identity can.", + }, + schema: [], + messages: { + missingKeypairCheck: + "This staleness check compares this._e2eeEpoch but not this._ecdhKeyPair. A non-key-holder " + + "session never advances the epoch, so this guard alone cannot detect a torn-down-then-restarted " + + "session — add `|| this._ecdhKeyPair !== ` to the condition.", + }, + }, + create(context) { + return { + IfStatement(node) { + if ( + containsStrictInequality(node.test, "_e2eeEpoch") && + !containsStrictInequality(node.test, "_ecdhKeyPair") + ) { + context.report({ node: node.test, messageId: "missingKeypairCheck" }); + } + }, + }; + }, +}; + +// ───────────────────────────────────────────────────────────────────────── +// Rule: e2ee-verified-status-literal +// +// Invariant (CLAUDE.md): "Anything touching livekitE2EE.ts ... must never +// report an unverified peer as verified." +// +// verifyPeerAnnounce's every write of peer-verification state goes through +// setPeerVerification/setPeerVerificationIfCurrent, and "verified" is reached +// exactly once, only after a real signature check. This rule keeps that +// structurally true: the `status` field at every call site must be a literal +// the author typed by hand at that call site, never a variable/expression — +// which would let a status be computed (and potentially manipulated) instead +// of asserted at the one audited call site that earned it. +// ───────────────────────────────────────────────────────────────────────── + +function getCalleeName(node) { + if (node.callee.type === "Identifier") return node.callee.name; + if ( + node.callee.type === "MemberExpression" && + !node.callee.computed && + node.callee.property.type === "Identifier" + ) { + return node.callee.property.name; + } + return null; +} + +const VERIFICATION_SETTERS = new Set(["setPeerVerification", "setPeerVerificationIfCurrent"]); + +const e2eeVerifiedStatusLiteral = { + meta: { + type: "problem", + docs: { + description: + "Require the `status` field passed to setPeerVerification/setPeerVerificationIfCurrent to be a " + + "string literal. A peer must never be reported verified via a computed/derived status — each " + + "verification outcome is a distinct, hand-written call site that earned its status inline.", + }, + schema: [], + messages: { + dynamicStatus: + "The `status` passed here must be a string literal ('verified' | 'unverified' | 'mismatch' | " + + "'unknown'), not a computed expression. Add a new literal call site for this outcome instead of " + + "deriving the status dynamically — that is what keeps 'verified' provably tied to a real signature check.", + }, + }, + create(context) { + return { + CallExpression(node) { + const name = getCalleeName(node); + if (name === null || !VERIFICATION_SETTERS.has(name)) return; + const objArg = node.arguments[node.arguments.length - 1]; + if (objArg === undefined || objArg.type !== "ObjectExpression") return; + const statusProp = objArg.properties.find( + (p) => + p.type === "Property" && + !p.computed && + p.key.type === "Identifier" && + p.key.name === "status", + ); + if (statusProp === undefined) return; + const value = statusProp.value; + if (value.type !== "Literal" || typeof value.value !== "string") { + context.report({ node: statusProp, messageId: "dynamicStatus" }); + } + }, + }; + }, +}; + +// ───────────────────────────────────────────────────────────────────────── +// Rule: no-identity-scope-fallback +// +// Invariant (CLAUDE.md): "Anything touching livekitE2EE.ts or identity.ts +// must preserve the epoch/keypair staleness guards." (Identity-scoping +// analogue: a documented, previously-real bug — see identity.ts's +// `identityKeyPairCache` comment — where a missing user id fell back to a +// placeholder scope like `?? 0`, silently minting/adopting a keypair under +// the wrong account and permanently desyncing the published key from the +// announce-signing key for every peer.) +// +// getOrCreateIdentityKeyPair's userId argument must come from a value that +// was already checked for `undefined` (the pattern both call sites use), not +// a `??`/`||` fallback that would substitute a placeholder id. +// ───────────────────────────────────────────────────────────────────────── + +const noIdentityScopeFallback = { + meta: { + type: "problem", + docs: { + description: + "Disallow a ??/|| placeholder fallback as the userId argument to getOrCreateIdentityKeyPair. A " + + "missing user id must abort (see the `userId === undefined` guards at both call sites), never " + + "substitute a placeholder scope — that mints or adopts a keypair under the wrong account and " + + "permanently desyncs the published key from the announce-signing key.", + }, + schema: [], + messages: { + placeholderFallback: + "Do not fall back with ??/|| when passing the user id to getOrCreateIdentityKeyPair — a missing " + + "id must abort instead (check `=== undefined` and return, as both existing call sites do). A " + + "placeholder id mints/adopts a keypair under the wrong account and desyncs it from the signing key.", + }, + }, + create(context) { + return { + CallExpression(node) { + if ( + node.callee.type !== "Identifier" || + node.callee.name !== "getOrCreateIdentityKeyPair" + ) { + return; + } + const userIdArg = node.arguments[1]; + if (userIdArg === undefined) return; + if ( + userIdArg.type === "LogicalExpression" && + (userIdArg.operator === "??" || userIdArg.operator === "||") + ) { + context.report({ node: userIdArg, messageId: "placeholderFallback" }); + } + }, + }; + }, +}; + +// ───────────────────────────────────────────────────────────────────────── +// Rule: no-store-write-in-ws-on +// +// Invariant (CLAUDE.md): "src/lib/dispatcher.ts is the single WS-event entry +// point: server events reach the stores only through a ws.on(...) +// subscription registered there." +// +// Other modules DO register their own ws.on(...) handlers (page-local UI: +// slow-mode timers, the connected overlay, incoming-call ringing) — that +// itself is not the violation. What must never happen outside dispatcher.ts +// is one of those handlers writing to a domain store directly, bypassing the +// dispatcher. Store *reads* (`fooStore.getState()`) are unaffected; this only +// flags calls to an imported store-mutator function (set/add/update/... from +// a `*/stores/*` module) reached from inside a `ws.on(...)` callback. +// ───────────────────────────────────────────────────────────────────────── + +const STORE_MUTATOR_PREFIX = + /^(set|add|remove|update|increment|clear|toggle|open|close|join|leave|mark|confirm|bulk|rollback|reset|prepend|reattach|invalidate|load)[A-Z_]/; + +function isStoreModuleSource(source) { + // Matches both the "@stores/..." alias and relative "../stores/..." paths. + return typeof source === "string" && /(?:^|\/)@?stores\//.test(source); +} + +function isWsOnCall(node) { + return ( + node !== null && + node !== undefined && + node.type === "CallExpression" && + node.callee.type === "MemberExpression" && + !node.callee.computed && + node.callee.object.type === "Identifier" && + node.callee.object.name === "ws" && + node.callee.property.type === "Identifier" && + node.callee.property.name === "on" && + node.arguments.length >= 2 + ); +} + +const noStoreWriteInWsOn = { + meta: { + type: "problem", + docs: { + description: + "Disallow calling an imported store-mutator (set*/add*/update*/... from a stores/ module) from " + + "inside a ws.on(...) callback outside dispatcher.ts. dispatcher.ts is the single place server " + + "events are allowed to write into domain stores; a page-local ws.on(...) handler may read store " + + "state and drive its own local UI, but must not mutate a domain store itself.", + }, + schema: [], + messages: { + storeWriteOutsideDispatcher: + "'{{name}}' is a store mutator called from a ws.on(...) handler outside dispatcher.ts. " + + "dispatcher.ts is the single WS-event entry point that may write to stores — move this update " + + "into a dispatcher.ts handler for this message type, or have this handler read the store instead " + + "of writing it.", + }, + }, + create(context) { + const storeMutatorImports = new Set(); + + return { + ImportDeclaration(node) { + if (!isStoreModuleSource(node.source.value)) return; + for (const spec of node.specifiers) { + if (spec.type === "ImportSpecifier" && STORE_MUTATOR_PREFIX.test(spec.local.name)) { + storeMutatorImports.add(spec.local.name); + } + } + }, + CallExpression(node) { + if (node.callee.type !== "Identifier" || !storeMutatorImports.has(node.callee.name)) return; + let parent = node.parent; + while (parent) { + if ( + isFunctionNode(parent) && + isWsOnCall(parent.parent) && + parent.parent.arguments[1] === parent + ) { + context.report({ + node, + messageId: "storeWriteOutsideDispatcher", + data: { name: node.callee.name }, + }); + return; + } + parent = parent.parent; + } + }, + }; + }, +}; + +export default { + rules: { + "no-leave-voice-when-superseded": noLeaveVoiceWhenSuperseded, + "e2ee-epoch-needs-keypair-check": e2eeEpochNeedsKeypairCheck, + "e2ee-verified-status-literal": e2eeVerifiedStatusLiteral, + "no-identity-scope-fallback": noIdentityScopeFallback, + "no-store-write-in-ws-on": noStoreWriteInWsOn, + }, +}; diff --git a/Client/tauri-client/eslint.config.js b/Client/tauri-client/eslint.config.js index b667dbca..d166ec0c 100644 --- a/Client/tauri-client/eslint.config.js +++ b/Client/tauri-client/eslint.config.js @@ -1,5 +1,6 @@ import eslint from "@eslint/js"; import tseslint from "typescript-eslint"; +import localRules from "./eslint-rules.js"; export default tseslint.config( eslint.configs.recommended, @@ -32,10 +33,7 @@ export default tseslint.config( // Empty functions are used for no-op callbacks "@typescript-eslint/no-empty-function": "off", // Project uses void for fire-and-forget promises intentionally - "@typescript-eslint/no-misused-promises": [ - "error", - { checksVoidReturn: false }, - ], + "@typescript-eslint/no-misused-promises": ["error", { checksVoidReturn: false }], // Allow require() in config files "@typescript-eslint/no-require-imports": "off", // Unbound methods used in singleton export pattern (bind at export) @@ -67,14 +65,43 @@ export default tseslint.config( "consistent-return": "off", }, }, + // --- Local rules: three CLAUDE.md invariants enforced as lint rules --- + // See eslint-rules.js for each rule's rationale and the historical bug + // shape it catches. Each is scoped to only the module(s) its invariant + // governs. { - ignores: [ - "dist/", - "src-tauri/", - "node_modules/", - "public/", - "*.js", - "*.cjs", - ], + files: ["src/lib/livekitSession.ts"], + plugins: { local: localRules }, + rules: { + "local/no-leave-voice-when-superseded": "error", + }, + }, + { + files: ["src/lib/livekitE2EE.ts"], + plugins: { local: localRules }, + rules: { + "local/e2ee-epoch-needs-keypair-check": "error", + "local/e2ee-verified-status-literal": "error", + "local/no-identity-scope-fallback": "error", + }, + }, + { + files: ["src/lib/identity.ts"], + plugins: { local: localRules }, + rules: { + "local/no-identity-scope-fallback": "error", + }, + }, + { + // dispatcher.ts IS the allowed entry point, so it is exempt from its own rule. + files: ["src/**/*.ts"], + ignores: ["src/lib/dispatcher.ts"], + plugins: { local: localRules }, + rules: { + "local/no-store-write-in-ws-on": "error", + }, + }, + { + ignores: ["dist/", "src-tauri/", "node_modules/", "public/", "*.js", "*.cjs"], }, ); diff --git a/Client/tauri-client/package-lock.json b/Client/tauri-client/package-lock.json index 2a3c6f4c..41b41bc1 100644 --- a/Client/tauri-client/package-lock.json +++ b/Client/tauri-client/package-lock.json @@ -1,12 +1,12 @@ { "name": "owncord-client", - "version": "1.2.0-alpha.1", + "version": "1.2.0-alpha.2", "lockfileVersion": 3, "requires": true, "packages": { "": { "name": "owncord-client", - "version": "1.2.0-alpha.1", + "version": "1.2.0-alpha.2", "dependencies": { "@jitsi/rnnoise-wasm": "^0.2.1", "@tauri-apps/api": "^2.10.1", diff --git a/Client/tauri-client/package.json b/Client/tauri-client/package.json index 8754bb98..7cfa2a3d 100644 --- a/Client/tauri-client/package.json +++ b/Client/tauri-client/package.json @@ -1,7 +1,7 @@ { "name": "owncord-client", "private": true, - "version": "1.2.0-alpha.1", + "version": "1.2.0-alpha.2", "type": "module", "scripts": { "dev": "vite", diff --git a/Client/tauri-client/src-tauri/Cargo.lock b/Client/tauri-client/src-tauri/Cargo.lock index 27e266c5..1cc14547 100644 --- a/Client/tauri-client/src-tauri/Cargo.lock +++ b/Client/tauri-client/src-tauri/Cargo.lock @@ -3021,7 +3021,7 @@ dependencies = [ [[package]] name = "owncord-client" -version = "1.2.0-alpha.1" +version = "1.2.0-alpha.2" dependencies = [ "base64 0.22.1", "device_query", diff --git a/Client/tauri-client/src-tauri/Cargo.toml b/Client/tauri-client/src-tauri/Cargo.toml index 0238af4f..2586b9c5 100644 --- a/Client/tauri-client/src-tauri/Cargo.toml +++ b/Client/tauri-client/src-tauri/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "owncord-client" -version = "1.2.0-alpha.1" +version = "1.2.0-alpha.2" edition = "2021" # Effective minimum: tauri 2.11 declares rust-version = "1.77.2", so the crate # cannot build below it. Declaring it here enables Cargo's MSRV-aware resolver diff --git a/Client/tauri-client/src-tauri/src/credentials.rs b/Client/tauri-client/src-tauri/src/credentials.rs index 726fed8c..c0fdbb22 100644 --- a/Client/tauri-client/src-tauri/src/credentials.rs +++ b/Client/tauri-client/src-tauri/src/credentials.rs @@ -9,9 +9,9 @@ use crate::secret_store::{self, Backend}; pub struct CredentialData { pub username: String, pub token: String, - // Password is stored in the credential blob for re-authentication but - // is never serialized back to the frontend over IPC to limit exposure. - #[serde(skip)] + // Password is stored in the credential blob for re-authentication and is + // serialized back to the frontend over IPC so the login form can prefill + // it when the user ticked "Remember password". pub password: Option, } @@ -398,15 +398,15 @@ mod tests { } #[test] - fn credential_data_skips_password_in_json() { + fn credential_data_serializes_password_for_prefill() { let data = CredentialData { username: "alice".into(), token: "tok".into(), password: Some("pw".into()), }; let json = serde_json::to_string(&data).unwrap(); - assert!(!json.contains("password")); - assert!(!json.contains("pw")); + assert!(json.contains("password")); + assert!(json.contains("pw")); } /// B4-3 follow-up: all 7 commands moved to `#[tauri::command(async)]`, diff --git a/Client/tauri-client/src-tauri/tauri.conf.json b/Client/tauri-client/src-tauri/tauri.conf.json index 3c881716..c2872b35 100644 --- a/Client/tauri-client/src-tauri/tauri.conf.json +++ b/Client/tauri-client/src-tauri/tauri.conf.json @@ -1,6 +1,6 @@ { "productName": "OwnCord", - "version": "1.2.0-alpha.1", + "version": "1.2.0-alpha.2", "identifier": "com.owncord.client", "build": { "frontendDist": "../dist", diff --git a/Client/tauri-client/src/components/UserBar.ts b/Client/tauri-client/src/components/UserBar.ts index 10142789..0034026f 100644 --- a/Client/tauri-client/src/components/UserBar.ts +++ b/Client/tauri-client/src/components/UserBar.ts @@ -120,19 +120,15 @@ export function createUserBar(options?: UserBarOptions): MountableComponent { }); avatarTextEl = createElement("span", {}); avatarEl.appendChild(avatarTextEl); - const statusDot = createElement("div", { - class: "status-dot", - style: - "background: var(--green); width: 10px; height: 10px; border-radius: 50%; position: absolute; bottom: 0; right: 0;", - }); - avatarEl.appendChild(statusDot); const info = createElement("div", { class: "ub-info" }); nameEl = createElement("span", { class: "ub-name", "data-testid": "user-bar-name" }); statusEl = createElement("span", { class: "ub-status" }); appendChildren(info, nameEl, statusEl); - // Status picker — anchored below username, opens upward + // Status picker — the dot itself lives in the avatar's corner (same spot + // the old plain status indicator occupied) so it doubles as the status + // display and its click target; the dropdown still opens upward from there. const statusPickerWrap = createElement("div", { class: "ub-status-picker-wrap", "data-testid": "status-picker-wrap", @@ -203,7 +199,7 @@ export function createUserBar(options?: UserBarOptions): MountableComponent { () => updatePickerDisabled(), ); - info.appendChild(statusPickerWrap); + avatarEl.appendChild(statusPickerWrap); const buttons = createElement("div", { class: "ub-controls" }); diff --git a/Client/tauri-client/src/lib/credentials.ts b/Client/tauri-client/src/lib/credentials.ts index 64399281..65897818 100644 --- a/Client/tauri-client/src/lib/credentials.ts +++ b/Client/tauri-client/src/lib/credentials.ts @@ -11,8 +11,7 @@ const log = createLogger("credentials"); export interface SavedCredential { readonly username: string; readonly token: string; - // Note: password is no longer returned from the Rust backend over IPC - // to limit credential exposure in the JS heap. + readonly password?: string; } /** Dynamically import Tauri invoke to avoid errors in test/browser. */ @@ -93,6 +92,7 @@ export async function loadCredential(host: string): Promise { } // Auto-save a profile for a host after successful login (if not already saved) - function ensureProfileExists(host: string, username: string, rememberPassword: boolean): void { + function ensureProfileExists( + host: string, + username: string, + rememberPassword: boolean, + autoConnect: boolean, + ): void { const existing = profileManager.getAll().find((p) => p.host === host); if (existing) { // Update username, rememberPassword preference, and lastConnected @@ -469,6 +474,18 @@ async function renderPage(pageId: "connect" | "main"): Promise { }); profileManager.setLastConnected(created.id); } + + // Re-find: the profile may have just been created above. + const profile = profileManager.getAll().find((p) => p.host === host); + if (profile) { + if (autoConnect) { + profileManager.setAutoLogin(profile.id); + } else if (profile.autoConnect) { + // Only clear when this profile is the current holder — setAutoLogin(null) + // clears auto-login on every profile, not just this one. + profileManager.setAutoLogin(null); + } + } persistProfiles(); } @@ -487,7 +504,7 @@ async function renderPage(pageId: "connect" | "main"): Promise { if (result.token) { const remember = connectPage.getRememberPassword(); const savedPassword = remember ? password : undefined; - ensureProfileExists(host, username, remember); + ensureProfileExists(host, username, remember, connectPage.getAutoConnect()); wirePostAuth(host, result.token, username, savedPassword, remember); } }, @@ -496,7 +513,7 @@ async function renderPage(pageId: "connect" | "main"): Promise { const result = await api.register(username, password, inviteCode); const remember = connectPage.getRememberPassword(); const savedPassword = remember ? password : undefined; - ensureProfileExists(host, username, remember); + ensureProfileExists(host, username, remember, connectPage.getAutoConnect()); wirePostAuth(host, result.token, username, savedPassword, remember); }, async onTotpSubmit(code) { @@ -509,7 +526,12 @@ async function renderPage(pageId: "connect" | "main"): Promise { if (result.token) { const remember = connectPage.getRememberPassword(); const savedPassword = remember ? connectPage.getPassword() : undefined; - ensureProfileExists(pendingTotpHost, pendingTotpUsername, remember); + ensureProfileExists( + pendingTotpHost, + pendingTotpUsername, + remember, + connectPage.getAutoConnect(), + ); wirePostAuth( pendingTotpHost, result.token, @@ -615,7 +637,11 @@ async function renderPage(pageId: "connect" | "main"): Promise { if (quickSwitchTarget !== null) { sessionStorage.removeItem("owncord:quick-switch-target"); const targetProfile = profileManager.getAll().find((p) => p.host === quickSwitchTarget); - connectPage.selectServer(quickSwitchTarget, targetProfile?.username ?? undefined); + connectPage.selectServer( + quickSwitchTarget, + targetProfile?.username ?? undefined, + targetProfile?.autoConnect === true, + ); return; // Skip auto-login when switching servers } @@ -640,7 +666,9 @@ async function renderPage(pageId: "connect" | "main"): Promise { try { const cred = await loadCredential(autoProfile.host); if (cred?.username && cred?.token && !autoLoginCancelled) { - connectPage.selectServer(autoProfile.host, cred.username); + // Pass autoConnect so the checkbox still reads correctly if the + // user cancels and lands back on the form. + connectPage.selectServer(autoProfile.host, cred.username, autoProfile.autoConnect); connectPage.showAutoConnecting(autoProfile.name); if (autoLoginCancelled) return; @@ -656,7 +684,12 @@ async function renderPage(pageId: "connect" | "main"): Promise { // remember (save_credential only carries the password key // `if let Some(...)`, so a None wipes it — see credentials.rs). api.setConfig({ host: autoProfile.host }); - ensureProfileExists(autoProfile.host, cred.username, autoProfile.rememberPassword); + ensureProfileExists( + autoProfile.host, + cred.username, + autoProfile.rememberPassword, + autoProfile.autoConnect, + ); wirePostAuth(autoProfile.host, cred.token, cred.username, undefined, false); return; } diff --git a/Client/tauri-client/src/pages/ConnectPage.ts b/Client/tauri-client/src/pages/ConnectPage.ts index 35b8deba..b6bdd66f 100644 --- a/Client/tauri-client/src/pages/ConnectPage.ts +++ b/Client/tauri-client/src/pages/ConnectPage.ts @@ -51,11 +51,13 @@ export function createConnectPage( resetToIdle(): void; updateHealthStatus(host: string, status: HealthStatus): void; getRememberPassword(): boolean; + /** Whether the auto-connect checkbox is ticked. */ + getAutoConnect(): boolean; getPassword(): string; /** Re-render the server profile list with updated data. */ refreshProfiles(profiles: readonly SimpleProfile[]): void; /** Pre-select a server by host — fills the login form and loads saved credentials. */ - selectServer(host: string, username?: string): void; + selectServer(host: string, username?: string, autoConnect?: boolean): void; /** Pre-fill + switch to register mode from an owncord:// invite deep link. */ applyInviteLink(code: string, host?: string): void; } { @@ -80,11 +82,12 @@ export function createConnectPage( const serverPanel = createServerPanel( { signal, - onServerClick(host: string, username?: string) { + onServerClick(host: string, username?: string, autoConnect?: boolean) { loginForm.setHost(host); if (username) { loginForm.setCredentials(username); } + loginForm.setAutoConnect(autoConnect === true); }, onCredentialLoaded(host: string, username: string, password?: string) { // Guard: user may have clicked a different profile while loading @@ -313,22 +316,24 @@ export function createConnectPage( updateHealthStatus: (host: string, status: HealthStatus) => serverPanel.updateHealthStatus(host, status), getRememberPassword: () => loginForm.getRememberPassword(), + getAutoConnect: () => loginForm.getAutoConnect(), getPassword: () => loginForm.getPassword(), refreshProfiles(profiles: readonly SimpleProfile[]): void { serverPanel.renderProfiles(profiles); }, - selectServer(host: string, username?: string): void { + selectServer(host: string, username?: string, autoConnect?: boolean): void { loginForm.setHost(host); if (username) { loginForm.setCredentials(username); } + loginForm.setAutoConnect(autoConnect === true); // Load saved credentials asynchronously (same flow as clicking a server card) void (async () => { try { const cred = await loadCredential(host); if (cred && loginForm.getHost() === host) { - // Password is no longer returned from credential store over IPC - loginForm.setCredentials(cred.username); + // Prefill the saved password so the user isn't retyping it. + loginForm.setCredentials(cred.username, cred.password); } } catch { // Credential loading is best-effort; user can type manually diff --git a/Client/tauri-client/src/pages/connect-page/LoginForm.ts b/Client/tauri-client/src/pages/connect-page/LoginForm.ts index 3bb065e5..a9a39bd6 100644 --- a/Client/tauri-client/src/pages/connect-page/LoginForm.ts +++ b/Client/tauri-client/src/pages/connect-page/LoginForm.ts @@ -53,6 +53,10 @@ export interface LoginFormApi { showError(message: string): void; resetToIdle(): void; getRememberPassword(): boolean; + /** Whether the auto-connect checkbox is ticked. */ + getAutoConnect(): boolean; + /** Set the auto-connect checkbox (also forces remember-password on). */ + setAutoConnect(enabled: boolean): void; getPassword(): string; /** Set the host input value (called when ServerPanel clicks a server). */ setHost(host: string): void; @@ -93,6 +97,7 @@ export function createLoginForm(opts: LoginFormOptions): LoginFormApi { let totpInput: HTMLInputElement; let totpSubmitBtn: HTMLButtonElement; let rememberPasswordCheckbox: HTMLInputElement; + let autoConnectCheckbox: HTMLInputElement; let autoConnectServerName: HTMLSpanElement; // --------------------------------------------------------------------------- @@ -220,6 +225,30 @@ export function createLoginForm(opts: LoginFormOptions): LoginFormApi { ); appendChildren(rememberGroup, rememberPasswordCheckbox, rememberLabel); + // Auto connect checkbox + const autoConnectGroup = createElement("div", { class: "form-group remember-password-group" }); + autoConnectCheckbox = createElement("input", { type: "checkbox", id: "auto-connect" }); + const autoConnectLabel = createElement( + "label", + { + for: "auto-connect", + class: "remember-password-label", + }, + "Auto connect", + ); + appendChildren(autoConnectGroup, autoConnectCheckbox, autoConnectLabel); + + autoConnectCheckbox.addEventListener( + "change", + () => { + // Auto-connect replays the saved token, which only exists when the + // password is remembered — so the pairing is enforced, not suggested. + if (autoConnectCheckbox.checked) rememberPasswordCheckbox.checked = true; + rememberPasswordCheckbox.disabled = autoConnectCheckbox.checked; + }, + { signal }, + ); + // Invite code (register only, hidden by default) inviteGroup = buildFormGroup("invite", "Invite Code", "text", ""); inviteGroup.classList.add("form-group--hidden"); @@ -247,6 +276,7 @@ export function createLoginForm(opts: LoginFormOptions): LoginFormApi { usernameGroup, passwordGroup, rememberGroup, + autoConnectGroup, inviteGroup, submitBtn, formSwitch, @@ -680,6 +710,16 @@ export function createLoginForm(opts: LoginFormOptions): LoginFormApi { return rememberPasswordCheckbox?.checked ?? false; }, + getAutoConnect(): boolean { + return autoConnectCheckbox?.checked ?? false; + }, + + setAutoConnect(enabled: boolean): void { + autoConnectCheckbox.checked = enabled; + if (enabled) rememberPasswordCheckbox.checked = true; + rememberPasswordCheckbox.disabled = enabled; + }, + getPassword(): string { return passwordInput?.value ?? ""; }, diff --git a/Client/tauri-client/src/pages/connect-page/ServerPanel.ts b/Client/tauri-client/src/pages/connect-page/ServerPanel.ts index 3fbf9c58..038a1b45 100644 --- a/Client/tauri-client/src/pages/connect-page/ServerPanel.ts +++ b/Client/tauri-client/src/pages/connect-page/ServerPanel.ts @@ -47,7 +47,7 @@ function getIconInitials(name: string): string { export interface ServerPanelOptions { readonly signal: AbortSignal; /** Called immediately when the user clicks a server profile. */ - readonly onServerClick: (host: string, username?: string) => void; + readonly onServerClick: (host: string, username?: string, autoConnect?: boolean) => void; /** Called after async credential lookup succeeds (may set password). */ readonly onCredentialLoaded: (host: string, username: string, password?: string) => void; readonly onAddProfile?: (name: string, host: string) => void; @@ -206,13 +206,13 @@ export function createServerPanel( "click", () => { // Immediately fill host + username from profile - onServerClick(profile.host, fullProfile.username); + onServerClick(profile.host, fullProfile.username, fullProfile.autoConnect === true); // Auto-fill credentials from credential store (async) const requestedHost = profile.host; void (async () => { const cred = await loadCredential(requestedHost); if (cred) { - onCredentialLoaded(requestedHost, cred.username, undefined); + onCredentialLoaded(requestedHost, cred.username, cred.password); } })(); }, diff --git a/Client/tauri-client/src/styles/app.css b/Client/tauri-client/src/styles/app.css index 63419357..0a024017 100644 --- a/Client/tauri-client/src/styles/app.css +++ b/Client/tauri-client/src/styles/app.css @@ -657,14 +657,14 @@ color: white; cursor: pointer; } -.user-bar .status-dot { +/* The status picker's own trigger dot now lives here instead of a separate + static dot — same corner, but it's the real thing: colored per status and + clickable to open the picker. Sizing/border override lives with the rest + of the status-picker rules below. */ +.user-bar .ub-status-picker-wrap { position: absolute; bottom: -1px; right: -1px; - width: 12px; - height: 12px; - border-radius: var(--radius-circle); - border: 3px solid rgba(17, 18, 20, 0.6); } .user-bar .ub-info { flex: 1; @@ -750,6 +750,13 @@ outline: 2px solid var(--accent); outline-offset: 2px; } +/* In the user bar the dot sits on the avatar corner, not inline next to + text — bigger, with a ring so it reads against any avatar picture. */ +.user-bar .status-picker-dot { + width: 12px; + height: 12px; + border: 3px solid rgba(17, 18, 20, 0.6); +} .status-picker-option { display: flex; align-items: center; diff --git a/Client/tauri-client/tests/e2e/user-bar.spec.ts b/Client/tauri-client/tests/e2e/user-bar.spec.ts index d6d4baa9..6b5a8b94 100644 --- a/Client/tauri-client/tests/e2e/user-bar.spec.ts +++ b/Client/tauri-client/tests/e2e/user-bar.spec.ts @@ -36,6 +36,13 @@ test.describe("User Bar", () => { await expect(status).toHaveText("Online"); }); + test("status picker dot sits on the avatar", async ({ page }) => { + const dot = page.locator( + "[data-testid='user-bar'] .ub-avatar [data-testid='status-picker-wrap'] .status-picker-dot", + ); + await expect(dot).toBeVisible(); + }); + test("user bar has settings button with correct label", async ({ page }) => { const controls = page.locator("[data-testid='user-bar'] .ub-controls"); await expect(controls).toBeVisible(); @@ -54,9 +61,4 @@ test.describe("User Bar", () => { // UserBar renders settings + optionally disconnect (no mute/deafen in user bar) expect(count).toBeGreaterThanOrEqual(1); }); - - test("user bar has status dot", async ({ page }) => { - const statusDot = page.locator("[data-testid='user-bar'] .status-dot"); - await expect(statusDot).toBeAttached(); - }); }); diff --git a/Client/tauri-client/tests/unit/connect-page.test.ts b/Client/tauri-client/tests/unit/connect-page.test.ts index 3a15e616..ce364336 100644 --- a/Client/tauri-client/tests/unit/connect-page.test.ts +++ b/Client/tauri-client/tests/unit/connect-page.test.ts @@ -288,9 +288,9 @@ describe("ConnectPage", () => { expect(usernameInput.value).toBe("saveduser"); }); - // Password is no longer returned from credential store over IPC (security hardening) + // The stored password prefills the field so the user isn't retyping it. const passwordInput = container.querySelector("#password") as HTMLInputElement; - expect(passwordInput.value).toBe(""); + expect(passwordInput.value).toBe("savedpass"); page.destroy?.(); }); @@ -457,6 +457,57 @@ describe("ConnectPage", () => { page.destroy?.(); }); + // --- getAutoConnect --- + + it("getAutoConnect returns checkbox state", () => { + const page = createConnectPage(makeCallbacks(), testProfiles); + page.mount(container); + + expect(page.getAutoConnect()).toBe(false); + + const checkbox = container.querySelector("#auto-connect") as HTMLInputElement; + checkbox.checked = true; + expect(page.getAutoConnect()).toBe(true); + + page.destroy?.(); + }); + + it("ticking auto-connect forces and disables remember password", () => { + const page = createConnectPage(makeCallbacks(), testProfiles); + page.mount(container); + + const autoConnectCheckbox = container.querySelector("#auto-connect") as HTMLInputElement; + const rememberCheckbox = container.querySelector("#remember-password") as HTMLInputElement; + + autoConnectCheckbox.checked = true; + autoConnectCheckbox.dispatchEvent(new Event("change")); + + expect(rememberCheckbox.checked).toBe(true); + expect(rememberCheckbox.disabled).toBe(true); + expect(page.getRememberPassword()).toBe(true); + + page.destroy?.(); + }); + + it("unticking auto-connect re-enables remember password", () => { + const page = createConnectPage(makeCallbacks(), testProfiles); + page.mount(container); + + const autoConnectCheckbox = container.querySelector("#auto-connect") as HTMLInputElement; + const rememberCheckbox = container.querySelector("#remember-password") as HTMLInputElement; + + autoConnectCheckbox.checked = true; + autoConnectCheckbox.dispatchEvent(new Event("change")); + + autoConnectCheckbox.checked = false; + autoConnectCheckbox.dispatchEvent(new Event("change")); + + expect(rememberCheckbox.disabled).toBe(false); + expect(rememberCheckbox.checked).toBe(true); + + page.destroy?.(); + }); + it("getPassword returns password input value", () => { const page = createConnectPage(makeCallbacks(), testProfiles); page.mount(container); @@ -940,25 +991,20 @@ describe("ConnectPage", () => { // --- setCredentials with password sets remember checkbox --- - it("setCredentials with password checks the remember password checkbox", () => { + it("setCredentials with password checks the remember password checkbox", async () => { + mockLoadCredential.mockResolvedValue({ username: "user", token: "tok", password: "pass123" }); const page = createConnectPage(makeCallbacks(), testProfiles); page.mount(container); - // Use selectServer which calls setCredentials internally - mockLoadCredential.mockResolvedValue({ username: "user", token: "tok", password: "pass123" }); - page.selectServer("localhost:8443"); - - // Wait for credential loading isn't needed for checking setCredentials behavior - // Let's check via the sync path: onServerClick doesn't set a password - // We need to verify that setCredentials with password enables remember - // Simulating by using credential loaded callback - // Actually let's verify through server panel click path const serverItem = container.querySelector(".server-item") as HTMLElement; serverItem.click(); - // The rememberPassword should eventually be true after cred loaded - // For now, let's just verify getRememberPassword baseline - expect(page.getRememberPassword()).toBe(false); + await vi.waitFor(() => { + expect(page.getRememberPassword()).toBe(true); + }); + + const passwordInput = container.querySelector("#password") as HTMLInputElement; + expect(passwordInput.value).toBe("pass123"); page.destroy?.(); }); diff --git a/Client/tauri-client/tests/unit/credentials.test.ts b/Client/tauri-client/tests/unit/credentials.test.ts index d1f0bf4c..a021c51c 100644 --- a/Client/tauri-client/tests/unit/credentials.test.ts +++ b/Client/tauri-client/tests/unit/credentials.test.ts @@ -136,15 +136,27 @@ describe("loadCredential", () => { expect(invoke).toHaveBeenCalledWith("load_credential", { host: "h.example" }); }); + it("returns the stored password so the login form can prefill it", async () => { + invoke.mockResolvedValue({ username: "alice", token: "tok", password: "pass123" }); + + await expect(loadCredential("h.example")).resolves.toEqual({ + username: "alice", + token: "tok", + password: "pass123", + }); + }); + it("drops any extra fields the backend returns", async () => { - // The Rust side deliberately stopped returning the password over IPC; if it - // ever regresses, the password must not make it into the JS heap. - invoke.mockResolvedValue({ username: "alice", token: "tok", password: "leaked" }); + // Only the known fields should survive reconstruction — an unrecognised + // field must not make it into the JS heap. + invoke.mockResolvedValue({ username: "alice", token: "tok", bogus: "x" }); const got = await loadCredential("h.example"); + // toEqual ignores the explicit `password: undefined`, so this still pins + // the exact shape and catches any unknown field, not just `bogus`. expect(got).toEqual({ username: "alice", token: "tok" }); - expect(got).not.toHaveProperty("password"); + expect(got).not.toHaveProperty("bogus"); }); it("returns null when nothing is stored", async () => { diff --git a/Client/tauri-client/tests/unit/eslint-rules.test.ts b/Client/tauri-client/tests/unit/eslint-rules.test.ts new file mode 100644 index 00000000..dde5dd6d --- /dev/null +++ b/Client/tauri-client/tests/unit/eslint-rules.test.ts @@ -0,0 +1,286 @@ +// Tests for the custom ESLint rules in eslint-rules.js that enforce three of +// the invariants documented in prose in CLAUDE.md. Each `describe` block +// covers one rule: `valid` cases are real shapes from the actual codebase +// that must NOT be flagged, `invalid` cases are the historical bug shape +// each rule exists to catch (see eslint-rules.js for the git-history context +// on each one). +import { describe, it } from "vitest"; +import { RuleTester } from "eslint"; +// eslint-rules.js is plain JS with no type declarations; the import below is +// only used to drive RuleTester, not type-checked against a `.d.ts`. +// @ts-expect-error -- no type declarations for the plain-JS rules module +import localRules from "../../eslint-rules.js"; + +const ruleTester = new RuleTester({ + languageOptions: { ecmaVersion: 2022, sourceType: "module" }, +}); + +describe("eslint-rules", () => { + it("no-leave-voice-when-superseded", () => { + ruleTester.run( + "no-leave-voice-when-superseded", + localRules.rules["no-leave-voice-when-superseded"], + { + valid: [ + // Entry-point cleanup before starting a new attempt — not a + // supersession check, so leaveVoice() here is fine. + `class LiveKitSession { + connectAndSetup() { + if (this._room !== null) this.leaveVoice(false); + } + }`, + // Reconnect give-up path: the guard returns early on supersession; + // leaveVoice() runs afterward as a sibling statement, never nested + // inside the superseded branch. + `class LiveKitSession { + async attemptAutoReconnect(signal, channelId) { + if (this.reconnectSuperseded(signal, channelId)) { + log.info("give up — superseded"); + return; + } + this.leaveVoice(true); + leaveVoiceChannel(); + } + }`, + // Positive-confirmed pattern: leaveVoice() only runs when the + // generation check confirms this attempt is STILL current. + `class LiveKitSession { + async connectAndSetup(myGeneration) { + if ( + this._state.type === "connecting" && + this._state.joinGeneration === myGeneration && + this._state.pendingJoin === null + ) { + this.leaveVoice(true); + leaveVoiceChannel(); + } + return false; + } + }`, + // isStateConnected() checkpoint: the safe cleanup scopes to the + // attempt's own room instead of calling the global leaveVoice(). + `class LiveKitSession { + async connectAndSetup(channelId, localRoom) { + if (!this.isStateConnected(channelId)) { + this.disconnectSupersededLocalRoom(localRoom); + return "superseded"; + } + } + }`, + // e2ee-timeout shape: a nested joinGeneration guard returns early; + // leaveVoice() is a sibling statement after it, not nested inside. + `class LiveKitSession { + async connectAndSetup(channelId, myGeneration, keyExchangeOk) { + if (!keyExchangeOk) { + if (this._state.type !== "connecting" || this._state.joinGeneration !== myGeneration) { + return "superseded"; + } + this.leaveVoice(true); + leaveVoiceChannel(); + return false; + } + } + }`, + ], + invalid: [ + // The historical bug shape: leaveVoice() called directly inside a + // reconnectSuperseded() branch — tears down whatever session + // currently owns _state, which may be a newer, live attempt. + { + code: `class LiveKitSession { + async attemptAutoReconnect(signal, channelId) { + if (this.reconnectSuperseded(signal, channelId)) { + this.leaveVoice(true); + return; + } + } + }`, + errors: [{ messageId: "unsafeLeaveVoice" }], + }, + // Same bug, via the negated isStateConnected() guard (this is + // exactly what checkpoints 3-5 regressed to before the fix that + // introduced disconnectSupersededLocalRoom). + { + code: `class LiveKitSession { + async connectAndSetup(channelId, localRoom) { + if (!this.isStateConnected(channelId)) { + this.leaveVoice(false); + return "superseded"; + } + } + }`, + errors: [{ messageId: "unsafeLeaveVoice" }], + }, + ], + }, + ); + }); + + it("e2ee-epoch-needs-keypair-check", () => { + ruleTester.run( + "e2ee-epoch-needs-keypair-check", + localRules.rules["e2ee-epoch-needs-keypair-check"], + { + valid: [ + // handleOfferInner / handleAnnounceInner's real (fixed) guard: + // epoch AND keypair identity checked together. + `class E2EEManager { + async handleOfferInner(keypair, epochBefore) { + if (this._e2eeEpoch !== epochBefore || this._ecdhKeyPair !== keypair) { + return; + } + } + }`, + // distributeRoomKey's guard: keypair (+ room key) only, no epoch + // involved at all — the rule must not demand an epoch check here. + `class E2EEManager { + async distributeRoomKey(keypair, roomKey) { + if (this._ecdhKeyPair !== keypair || this._roomKey !== roomKey) { + return; + } + } + }`, + // Unrelated condition — no epoch, no keypair. + `class E2EEManager { + foo(x) { + if (x > 0) { + return true; + } + } + }`, + ], + invalid: [ + // The historical bug: epoch-only staleness check. A non-key-holder + // never bumps the epoch, so this cannot detect a torn-down-then- + // restarted session. + { + code: `class E2EEManager { + async handleOfferInner(epochBefore) { + if (this._e2eeEpoch !== epochBefore) { + return; + } + } + }`, + errors: [{ messageId: "missingKeypairCheck" }], + }, + ], + }, + ); + }); + + it("e2ee-verified-status-literal", () => { + ruleTester.run( + "e2ee-verified-status-literal", + localRules.rules["e2ee-verified-status-literal"], + { + valid: [ + `setPeerVerification({ userId, status: "verified", safetyNumber });`, + `class E2EEManager { + f(myGeneration, userId) { + this.setPeerVerificationIfCurrent(myGeneration, { userId, status: "mismatch", safetyNumber: null }); + } + }`, + `class E2EEManager { + f(myGeneration, userId) { + this.setPeerVerificationIfCurrent(myGeneration, { userId, status: "unverified", safetyNumber: null }); + } + }`, + ], + invalid: [ + // A regression that derives the status instead of asserting it + // inline at a hand-verified call site. + { + code: `class E2EEManager { + f(myGeneration, userId, safetyNumber, ok) { + const computedStatus = ok ? "verified" : "unverified"; + this.setPeerVerificationIfCurrent(myGeneration, { userId, status: computedStatus, safetyNumber }); + } + }`, + errors: [{ messageId: "dynamicStatus" }], + }, + { + code: `setPeerVerification({ userId, status: computeStatus(ok), safetyNumber: null });`, + errors: [{ messageId: "dynamicStatus" }], + }, + ], + }, + ); + }); + + it("no-identity-scope-fallback", () => { + ruleTester.run("no-identity-scope-fallback", localRules.rules["no-identity-scope-fallback"], { + valid: [ + // The real (fixed) pattern at both call sites: a missing user id + // aborts instead of substituting a placeholder scope. + `async function ensureIdentityKeyPair(host) { + const myUserId = authStore.getState().user?.id; + if (myUserId === undefined) return null; + return await getOrCreateIdentityKeyPair(host, myUserId); + }`, + // Unrelated call with the same shape but a different callee — must + // not be flagged just because it also takes two args. + `someOtherFunction(host, userId ?? 0);`, + ], + invalid: [ + { + code: `async function ensureIdentityKeyPair(host, userId) { + return await getOrCreateIdentityKeyPair(host, userId ?? 0); + }`, + errors: [{ messageId: "placeholderFallback" }], + }, + { + code: `getOrCreateIdentityKeyPair(host, userId || 0);`, + errors: [{ messageId: "placeholderFallback" }], + }, + ], + }); + }); + + it("no-store-write-in-ws-on", () => { + ruleTester.run("no-store-write-in-ws-on", localRules.rules["no-store-write-in-ws-on"], { + valid: [ + // A store mutator called from a plain UI callback (not a ws.on() + // handler at all) is unaffected. + `import { setActiveChannel } from "@stores/channels.store"; + function onCancel() { + setActiveChannel(null); + }`, + // A ws.on() handler outside dispatcher.ts that only READS a store + // (channelsStore.getState()) to drive page-local UI — this is the + // real ChannelController.ts shape and must stay legal. + `import { channelsStore } from "@stores/channels.store"; + ws.on("chat_send_ok", (payload, id) => { + const ch = channelsStore.getState().channels.get(1); + startSlowMode(ch.slowMode); + });`, + // A locally-defined function that happens to match the mutator-verb + // naming convention, but was never imported from a stores/ module — + // proves the rule keys off the import source, not the name alone. + `function setLocalThing() {} + ws.on("ready", () => { + setLocalThing(); + });`, + ], + invalid: [ + { + code: `import { setActiveChannel } from "@stores/channels.store"; + ws.on("chat_send_ok", () => { + setActiveChannel(null); + });`, + errors: [{ messageId: "storeWriteOutsideDispatcher" }], + }, + // Reached indirectly through a nested .then() inside the callback — + // still "driven by this ws event", so still flagged. + { + code: `import { updateUser } from "@stores/auth.store"; + ws.on("user_update", () => { + somePromise.then(() => { + updateUser({ username: "x" }); + }); + });`, + errors: [{ messageId: "storeWriteOutsideDispatcher" }], + }, + ], + }); + }); +}); diff --git a/Client/tauri-client/tests/unit/server-panel.test.ts b/Client/tauri-client/tests/unit/server-panel.test.ts index f619dc32..80de714e 100644 --- a/Client/tauri-client/tests/unit/server-panel.test.ts +++ b/Client/tauri-client/tests/unit/server-panel.test.ts @@ -199,7 +199,7 @@ describe("ServerPanel", () => { const item = container.querySelector(".server-item") as HTMLElement; item.click(); - expect(onServerClick).toHaveBeenCalledWith("localhost:8443", undefined); + expect(onServerClick).toHaveBeenCalledWith("localhost:8443", undefined, false); }); it("calls onServerClick with host AND username for full profiles", () => { @@ -211,7 +211,31 @@ describe("ServerPanel", () => { const item = container.querySelector(".server-item") as HTMLElement; item.click(); - expect(onServerClick).toHaveBeenCalledWith("full.example.com:8443", "testuser"); + expect(onServerClick).toHaveBeenCalledWith("full.example.com:8443", "testuser", false); + }); + + it("calls onServerClick with autoConnect true when the profile has it enabled", () => { + const onServerClick = vi.fn(); + const fp = fullProfile({ autoConnect: true }); + const panel = createServerPanel(makeOpts({ onServerClick }), [fp]); + container.appendChild(panel.element); + + const item = container.querySelector(".server-item") as HTMLElement; + item.click(); + + expect(onServerClick).toHaveBeenCalledWith("full.example.com:8443", "testuser", true); + }); + + it("calls onServerClick with autoConnect false when the profile has it disabled", () => { + const onServerClick = vi.fn(); + const fp = fullProfile({ autoConnect: false }); + const panel = createServerPanel(makeOpts({ onServerClick }), [fp]); + container.appendChild(panel.element); + + const item = container.querySelector(".server-item") as HTMLElement; + item.click(); + + expect(onServerClick).toHaveBeenCalledWith("full.example.com:8443", "testuser", false); }); it("attempts to load credentials from credential store on click", async () => { diff --git a/Client/tauri-client/tests/unit/ws-active-channel.test.ts b/Client/tauri-client/tests/unit/ws-active-channel.test.ts new file mode 100644 index 00000000..f8bc5fac --- /dev/null +++ b/Client/tauri-client/tests/unit/ws-active-channel.test.ts @@ -0,0 +1,250 @@ +import { describe, it, expect, vi, beforeEach, afterEach } from "vitest"; + +// vi.mock is hoisted per file; the factories resolve to the shared handles +// exported from ./helpers/ws-mocks (see that module's doc comment). +vi.mock("@tauri-apps/api/core", async () => ({ + invoke: (await import("./helpers/ws-mocks")).mockInvoke, +})); + +vi.mock("@tauri-apps/api/event", async () => ({ + listen: (await import("./helpers/ws-mocks")).mockListen, +})); + +import { mockInvoke, mockListen, eventHandlers, emitTauriEvent } from "./helpers/ws-mocks"; +import { createWsClient, setActiveChannelProvider } from "../../src/lib/ws"; + +/** Parses the payload of the most recent `auth` frame sent via ws_send. */ +function getAuthPayload(): Record { + const authCall = mockInvoke.mock.calls.find( + (c) => + c[0] === "ws_send" && + typeof c[1]?.message === "string" && + (c[1].message as string).includes('"type":"auth"'), + ); + expect(authCall).toBeDefined(); + const parsed = JSON.parse((authCall![1] as { message: string }).message) as { + payload: Record; + }; + return parsed.payload; +} + +describe("auth frame: active_channel_id + reconnect-replay dedup arming", () => { + let client: ReturnType; + + beforeEach(() => { + vi.useFakeTimers(); + mockInvoke.mockReset(); + mockInvoke.mockResolvedValue(undefined); + mockListen.mockClear(); + eventHandlers.clear(); + // activeChannelProvider is a module-level singleton (registered once at + // app bootstrap in dispatcher.ts) — reset it so state doesn't leak across + // tests/files. + setActiveChannelProvider(null); + client = createWsClient(); + }); + + afterEach(() => { + client.disconnect(); + setActiveChannelProvider(null); + vi.useRealTimers(); + }); + + it("fresh connect (reconnectAttempt=0, lastSeq=0): no active_channel_id key even with a provider registered, and dedup is not armed", async () => { + setActiveChannelProvider(() => 99); + client.connect({ host: "localhost:8443", token: "t" }); + await vi.advanceTimersByTimeAsync(10); + emitTauriEvent("ws-state", "open"); + + const payload = getAuthPayload(); + expect(payload.last_seq).toBe(0); + expect(Object.prototype.hasOwnProperty.call(payload, "active_channel_id")).toBe(false); + expect(client.isReplaying()).toBe(false); + }); + + it("reconnect with lastSeq > 0 and a registered provider: active_channel_id carries the provider's id", async () => { + client.connect({ host: "localhost:8443", token: "t" }); + await vi.advanceTimersByTimeAsync(10); + emitTauriEvent("ws-state", "open"); + + emitTauriEvent( + "ws-message", + JSON.stringify({ + type: "auth_ok", + seq: 7, + payload: { + user: { id: 1, username: "a", avatar: null, role: "admin" }, + server_name: "S", + motd: "", + }, + }), + ); + + setActiveChannelProvider(() => 42); + + emitTauriEvent("ws-state", "closed"); + mockInvoke.mockClear(); + await vi.advanceTimersByTimeAsync(1100); + emitTauriEvent("ws-state", "open"); + + const payload = getAuthPayload(); + expect(payload.last_seq).toBe(7); + expect(payload.active_channel_id).toBe(42); + }); + + it("reconnect with lastSeq > 0 and NO provider registered: no active_channel_id key", async () => { + client.connect({ host: "localhost:8443", token: "t" }); + await vi.advanceTimersByTimeAsync(10); + emitTauriEvent("ws-state", "open"); + + emitTauriEvent( + "ws-message", + JSON.stringify({ + type: "auth_ok", + seq: 3, + payload: { + user: { id: 1, username: "a", avatar: null, role: "admin" }, + server_name: "S", + motd: "", + }, + }), + ); + + // No setActiveChannelProvider call — stays null from beforeEach reset. + emitTauriEvent("ws-state", "closed"); + mockInvoke.mockClear(); + await vi.advanceTimersByTimeAsync(1100); + emitTauriEvent("ws-state", "open"); + + const payload = getAuthPayload(); + expect(payload.last_seq).toBe(3); + expect(Object.prototype.hasOwnProperty.call(payload, "active_channel_id")).toBe(false); + }); + + it("reconnect with lastSeq > 0 and a provider that returns null: no active_channel_id key", async () => { + setActiveChannelProvider(() => null); + + client.connect({ host: "localhost:8443", token: "t" }); + await vi.advanceTimersByTimeAsync(10); + emitTauriEvent("ws-state", "open"); + + emitTauriEvent( + "ws-message", + JSON.stringify({ + type: "auth_ok", + seq: 4, + payload: { + user: { id: 1, username: "a", avatar: null, role: "admin" }, + server_name: "S", + motd: "", + }, + }), + ); + + emitTauriEvent("ws-state", "closed"); + mockInvoke.mockClear(); + await vi.advanceTimersByTimeAsync(1100); + emitTauriEvent("ws-state", "open"); + + const payload = getAuthPayload(); + expect(payload.last_seq).toBe(4); + expect(Object.prototype.hasOwnProperty.call(payload, "active_channel_id")).toBe(false); + }); + + it("lastSeq > 0 but reconnectAttempt === 0: the provider is still consulted (gated on lastSeq, not reconnect count) and dedup stays unarmed", async () => { + client.connect({ host: "localhost:8443", token: "t" }); + await vi.advanceTimersByTimeAsync(10); + // First "open": reconnectAttempt=0, lastSeq=0 — irrelevant, just gets us started. + emitTauriEvent("ws-state", "open"); + + // Bump lastSeq WITHOUT ever going through a "closed"/scheduleReconnect + // cycle, so reconnectAttempt never increments off 0. + emitTauriEvent( + "ws-message", + JSON.stringify({ + type: "presence", + seq: 9, + payload: { user_id: 1, status: "idle" }, + }), + ); + + setActiveChannelProvider(() => 5); + mockInvoke.mockClear(); + + // Rust reports "open" again on the same (never-closed) connection. + emitTauriEvent("ws-state", "open"); + + const payload = getAuthPayload(); + expect(payload.last_seq).toBe(9); + expect(payload.active_channel_id).toBe(5); + // Dedup requires reconnectAttempt > 0 too — must still be unarmed. + expect(client.isReplaying()).toBe(false); + }); + + it("reconnectAttempt > 0 but lastSeq === 0: no active_channel_id key and dedup stays unarmed", async () => { + setActiveChannelProvider(() => 5); + + client.connect({ host: "localhost:8443", token: "t" }); + await vi.advanceTimersByTimeAsync(10); + emitTauriEvent("ws-state", "open"); + // No message ever arrives — lastSeq stays 0. + + emitTauriEvent("ws-state", "closed"); + mockInvoke.mockClear(); + await vi.advanceTimersByTimeAsync(1100); + emitTauriEvent("ws-state", "open"); // reconnectAttempt is now 1, lastSeq is still 0 + + const payload = getAuthPayload(); + expect(payload.last_seq).toBe(0); + expect(Object.prototype.hasOwnProperty.call(payload, "active_channel_id")).toBe(false); + expect(client.isReplaying()).toBe(false); + }); + + it("dedup is armed only when BOTH reconnectAttempt > 0 AND lastSeq > 0: a genuine reconnect replay dedups a repeated message (fresh-connect non-arming is covered above)", async () => { + client.connect({ host: "localhost:8443", token: "t" }); + await vi.advanceTimersByTimeAsync(10); + emitTauriEvent("ws-state", "open"); + + emitTauriEvent( + "ws-message", + JSON.stringify({ + type: "auth_ok", + seq: 1, + payload: { + user: { id: 1, username: "a", avatar: null, role: "admin" }, + server_name: "S", + motd: "", + }, + }), + ); + + emitTauriEvent("ws-state", "closed"); + await vi.advanceTimersByTimeAsync(1100); + emitTauriEvent("ws-state", "open"); + + // Both conditions true now: reconnectAttempt=1, lastSeq=1. + expect(client.isReplaying()).toBe(true); + + const replayed: unknown[] = []; + client.on("chat_message", (p) => replayed.push(p)); + + const dupMsg = JSON.stringify({ + type: "chat_message", + seq: 5, + id: "dup-msg", + payload: { + id: 1, + channel_id: 1, + user: { id: 1, username: "a", avatar: null }, + content: "replayed", + reply_to: null, + attachments: [], + timestamp: "2026-01-01T00:00:00Z", + }, + }); + emitTauriEvent("ws-message", dupMsg); + emitTauriEvent("ws-message", dupMsg); + + expect(replayed).toHaveLength(1); + }); +}); diff --git a/README.md b/README.md index 0ccbd511..eff01f91 100644 --- a/README.md +++ b/README.md @@ -138,11 +138,11 @@ Two main components: ```bash # Server (Windows) cd Server -go build -o chatserver.exe -ldflags "-s -w -X main.version=1.2.0-alpha.1" . +go build -o chatserver.exe -ldflags "-s -w -X main.version=1.2.0-alpha.2" . # Server (Linux) cd Server -CGO_ENABLED=0 go build -o chatserver -ldflags "-s -w -X main.version=1.2.0-alpha.1" . +CGO_ENABLED=0 go build -o chatserver -ldflags "-s -w -X main.version=1.2.0-alpha.2" . # Client cd Client/tauri-client diff --git a/Server/Makefile b/Server/Makefile index 1b43994c..05c44839 100644 --- a/Server/Makefile +++ b/Server/Makefile @@ -2,6 +2,8 @@ # # test Run the test suite the way CI does (race + timeout). # test-deadlock Run the deadlock-detection pass CI also runs. +# fuzz Actually fuzz. CI (and plain `go test`) only replays the +# committed seed corpus; this generates new inputs. # cover Per-package coverage (what CI uploads) + a function summary. # cover-all Cross-package coverage — the honest number. See below. # sqlc-generate Regenerate type-safe Go from sqlc.yaml (db/dbgen). @@ -14,7 +16,7 @@ SQLC_VERSION := $(shell cat sqlc.version) -.PHONY: test test-deadlock cover cover-all sqlc-install sqlc-generate sqlc-verify \ +.PHONY: test test-deadlock fuzz cover cover-all sqlc-install sqlc-generate sqlc-verify \ protocol-generate protocol-verify otel-up otel-down test: @@ -23,6 +25,29 @@ test: test-deadlock: go test -tags deadlock -count=1 ./... +# Every Fuzz* target, one at a time. `go test ./...` (and therefore CI) runs a +# Fuzz function against its committed seed corpus only — one pass per seed, +# zero generated inputs — so the harnesses find nothing new until this runs. +# Go fuzzes exactly one target per package per invocation, hence the loop. +# +# Deliberately local-only: a crasher is written to testdata/fuzz// +# and that file IS a working reproducer. This repo is public, so a crasher stays +# uncommitted until its fix exists, then corpus entry and fix land together as +# one regression test. +# +# No make on Windows? The same loop, straight into Git Bash: +# for pkg in $(go list ./...); do for fn in $(go test -list='^Fuzz' $pkg \ +# 2>/dev/null | grep '^Fuzz'); do go test $pkg -run='^$' -fuzz="^$fn$" \ +# -fuzztime=30s || break 2; done; done +FUZZTIME ?= 30s +fuzz: + @for pkg in $$(go list ./...); do \ + for fn in $$(go test -list='^Fuzz' $$pkg 2>/dev/null | grep '^Fuzz'); do \ + echo "── $$pkg $$fn"; \ + go test $$pkg -run='^$$' -fuzz="^$$fn$$" -fuzztime=$(FUZZTIME) || exit 1; \ + done; \ + done + # Matches the CI invocation. Note that `go test ./... -coverprofile` instruments # each package only for itself, so a package whose code is mostly exercised # through another package's tests reports far lower than its real coverage diff --git a/docs/api.md b/docs/api.md index 287a2fcc..97fde7c1 100644 --- a/docs/api.md +++ b/docs/api.md @@ -1973,7 +1973,7 @@ Owner-only self-update from GitHub Releases (minisign/Ed25519-verified; see ```json { - "current": "v1.2.0-alpha.1", + "current": "v1.2.0-alpha.2", "latest": "v1.2.0", "update_available": true, "required_assets_present": true, diff --git a/docs/deployment.md b/docs/deployment.md index 093cc28f..a725c604 100644 --- a/docs/deployment.md +++ b/docs/deployment.md @@ -16,13 +16,13 @@ Production deployment guide for OwnCord server on Windows and Linux. **Windows:** ```bash cd Server -go build -o chatserver.exe -ldflags "-s -w -X main.version=1.2.0-alpha.1" . +go build -o chatserver.exe -ldflags "-s -w -X main.version=1.2.0-alpha.2" . ``` **Linux:** ```bash cd Server -CGO_ENABLED=0 go build -o chatserver -ldflags "-s -w -X main.version=1.2.0-alpha.1" . +CGO_ENABLED=0 go build -o chatserver -ldflags "-s -w -X main.version=1.2.0-alpha.2" . ``` - `-s -w` strips debug info (smaller binary) diff --git a/docs/plans/bug-detection-improvements.md b/docs/plans/bug-detection-improvements.md new file mode 100644 index 00000000..de0ca834 --- /dev/null +++ b/docs/plans/bug-detection-improvements.md @@ -0,0 +1,265 @@ +# Bug-detection improvements — design + +Date: 2026-08-08 +Status: approved, not implemented + +## Problem + +The multi-agent bug hunt finds real defects at a high rate — the 2026-08-08 +client hunt confirmed 88 bugs and the follow-up sweeps fixed 101 — but it is +the only mechanism doing so, it costs a large token budget per run, and it has +never converged. Meanwhile several bug-catching tools are already installed, +configured, and committed to this repository, and none of them execute. + +This design adds mechanical detection alongside the agentic hunt, prioritised +by yield per token spent. + +## What already exists and does not run + +| Asset | State | Gap | +| --- | --- | --- | +| 14 `Fuzz*` harnesses under `Server/**/*_fuzz_test.go` | Committed | `go test ./...` runs a `Fuzz*` function against its **seed corpus only** — one pass per seed, zero generated inputs. `-fuzz` appears nowhere in the repo. | +| Stryker mutation testing | `stryker.config.mjs` + `npm run test:mutate` | Referenced in `ci.yml` only inside an npm-audit comment. Has never run. | +| Browser-mode vitest | `vitest.config.browser.ts` + `npm run test:browser` | CI runs jsdom only. | +| Cross-package coverage | `make cover-all` prints every 0.0%-covered function | Output is not fed to anything. | + +Separately, three of the codebase's sharpest invariants are documented in +`CLAUDE.md` files as prose and asserted nowhere: + +- `ws`: "a frame that skips the queue, or a seq allocated for a frame that is + then dropped, is silently unrecoverable" +- voice: cleanup in an aborted attempt "must be scoped to that attempt's own + room — a global `leaveVoice()` there kills the live session" +- E2EE: "must never report an unverified peer as verified" + +Prose fails no build. + +## Locked decisions + +**Everything in this design runs locally, on demand. Nothing is added to +GitHub Actions.** + +Rationale: `go test -fuzz` writes each crashing input to +`testdata/fuzz//`, and that file *is* a working reproducer. The +root `CLAUDE.md` states: "This repo is public — unfixed defects do not belong +in commits, issues, or PR descriptions." Actions artifacts on a public repo are +downloadable by anyone, and a red scheduled job is itself a public signal that +something is broken. A Stryker surviving-mutant report is a milder version of +the same disclosure: a precise map of which behaviour nobody tests. + +Local-only also means zero new workflow files and zero CI minutes. + +**Corpus discipline.** A crasher stays uncommitted until its fix exists. The +`testdata/fuzz/` corpus entry and the fix are committed together, as one +regression test. This is the same shape as the existing test-first rule. + +**Always replay a crasher before believing it.** Go runs fuzz targets in +separate worker processes. When a worker dies without reporting, the +coordinator cannot tell "crashed on this input" from "was killed externally", +so it saves the in-flight input to `testdata/fuzz/` as a suspected crasher. +Interrupting a fuzz run therefore manufactures a fake reproducer that is +indistinguishable at a glance from a real security finding. Confirm with +`go test ./ -run=''` — a real crasher fails there. Observed +2026-08-08: a 1666-byte malformed JPEG appeared under +`api/testdata/fuzz/FuzzImageDimensions/` purely because the run was killed. + +**Never `rm -r` a `testdata/fuzz//` directory** to clear a false +crasher. Committed seed corpus files live in the same directory — deleting the +directory takes them with it. Remove the single offending file by name. + +**Superseded 2026-08-08: Tier 2 ships as ESLint rules, not semgrep.** Semgrep +has no native Windows support (WSL or Docker only), so on this machine it would +join `make` as tooling that cannot be run locally. ESLint flat config supports +an inline plugin, so custom rules cost no new dependency — and `npx eslint +src/` is already a blocking CI gate, which removes the promotion step entirely. +Rules live in `Client/tauri-client/eslint-rules.js`, tested with `RuleTester` +in `tests/unit/eslint-rules.test.ts`. See "Tier 2 — delivered" below. + +## Tier 1 — Turn on what already exists + +### 1a. `make fuzz` + +Go fuzzes **one target per package per invocation**, so this cannot be a single +`go test -fuzz ./...`. The target enumerates fuzz functions and runs each with +a time budget. + +Add to `Server/Makefile`, and to its header comment block: + +```make +# fuzz Actually fuzz. CI only replays the seed corpus; this generates inputs. +# Override the per-target budget: FUZZTIME=2m make fuzz +FUZZTIME ?= 30s +fuzz: + @for pkg in $$(go list ./...); do \ + for fn in $$(go test -list='^Fuzz' $$pkg 2>/dev/null | grep '^Fuzz'); do \ + echo "── $$pkg $$fn"; \ + go test $$pkg -run='^$$' -fuzz="^$$fn$$" -fuzztime=$(FUZZTIME) || exit 1; \ + done; \ + done +``` + +Add `fuzz` to the `.PHONY` list. + +Default budget 30s per target — a full sweep of 14 targets is about 10 minutes +unattended. `FUZZTIME=2m` for a deep run. + +### 1b. Scoped Stryker runs + +`stryker.config.mjs` already scopes mutation to `src/lib/**` and `src/stores/**` +with `thresholds.break: 50`. A full run over that scope is expensive; a +hotspot run is not: + +```bash +npx stryker run --mutate "src/lib/dispatcher.ts,src/lib/ws.ts,src/lib/livekitE2EE.ts" +``` + +Roughly 25 minutes for three files. Surviving mutants identify lines whose +behaviour can be changed with the entire 4800-test suite still green. + +Treat the result as advisory. Do not gate on `thresholds.break` — the threshold +in the config file applies to a full-scope run and is meaningless for a +three-file subset. + +Target the files the hunt keeps returning to: `dispatcher.ts`, `ws.ts`, +`livekitE2EE.ts`, `identity.ts`, and the voice session module. + +### 1c. Browser-mode vitest + +Run `npm run test:browser` locally. The client `CLAUDE.md` already documents +jsdom diverging from native Web Storage semantics; browser mode is the only +configured surface that observes that class. + +### 1d. Prerequisite + +Confirm `Client/tauri-client/reports/`, `Client/tauri-client/.stryker-tmp/`, +`Server/coverage-all.out`, and `Server/**/testdata/fuzz/` interim output are +covered by `.gitignore` before running any of the above. Add entries where +they are missing. + +## Tier 2 — Bugs to permanent detectors + +Roughly 200 confirmed real bugs have been fixed across the hunt and harvest +runs. Each one currently bought exactly one fix. Encoding the recurring +*classes* converts them into permanent detectors. + +**Sources to mine:** bughunt commit history on `fix/bughunt-*` and +`fix/bughunt-harvest-*` branches, `.superpowers/harvest-med-low-checklist.md`, +and `docs/audit-*.md`. + +**Method:** cluster findings by class, not by symptom. Use the installed +`semgrep-rule-creator` skill, which is test-first — each rule ships with a +positive fixture that must match and a negative fixture that must not. + +### Tier 2 — delivered 2026-08-08 + +Five rules, all scoped to the modules their invariant governs, all proven to +fire by reintroducing the historical bug shape into real source and reverting: + +| Rule | Encodes | +| --- | --- | +| `no-leave-voice-when-superseded` | A global `leaveVoice()` inside a branch that already confirmed supersession tears down the newer live session | +| `e2ee-epoch-needs-keypair-check` | A non-key-holder never bumps the epoch, so an epoch-only staleness guard cannot see a restarted session | +| `e2ee-verified-status-literal` | Keeps `"verified"` tied to a hand-written call site that earned it, never a computed status | +| `no-identity-scope-fallback` | A `?? 0` placeholder scope mints a keypair under the wrong account | +| `no-store-write-in-ws-on` | Page-local `ws.on` handlers may read stores, not write them | + +**Declined: `await`-then-stale-snapshot.** Not AST-expressible. Whether an +await needs a guard — and whether the guard present is sufficient and correctly +placed — is intent, not shape. `livekitSession.ts` alone expresses supersession +guards in at least four different forms, and several awaits legitimately need +no guard. Any rule here would be too narrow to catch real bugs or broad enough +to flag most of the file's already-correct guard code. A rule that misfires on +correct code gets disabled and trains people to ignore the linter. + +**Found while writing these:** the dispatcher invariant in the client +`CLAUDE.md` was factually wrong. It claimed `ws.on(...)` appears only in +`dispatcher.ts`; eight handlers across `main.ts`, `MainPage.ts` and +`ChannelController.ts` say otherwise. The true invariant — dispatcher is the +single path by which server events *write to stores* — is what the rule +encodes, and the doc has been corrected to match. + +**Still open:** the server-side `ws` seq/FIFO invariant, which needs a Go +runtime assertion rather than a lint rule. + +Not every fixed bug becomes a rule. A class earns one when it has recurred at +least twice, or when it corresponds to an invariant already written down in a +`CLAUDE.md`. + +## Tier 3 — Stateful and chaos testing + +Fuzzing and property tests find bad **functions**. Every recurring bug in this +codebase's history is a bad **ordering**: `registerNow` reconnect-transfer, +superseded voice sessions, duplicate-message reconciliation, resync corruption, +the auth-race deep link, the logout/auto-login race. Nothing in the repo +generates orderings. + +### 3a. Client model-based tests + +`fast-check` v4 is already a dependency, and +`tests/unit/*.property.test.ts` establish the house pattern. Use `fc.commands`. + +- **Commands:** `Connect`, `Disconnect`, `RegisterNow`, `Receive(seq)`, + `Supersede`, `Resync`, `Logout`. +- **Model:** a minimal reference implementation of expected store state — not + a second copy of the real logic. +- **Invariants:** message ids never duplicate; per-client seq is monotonic; a + verified peer never flips to unverified and back; an aborted voice attempt + never tears down a live session owned by a newer attempt. + +The shrinking is the point: fast-check reduces a 40-step failure to the minimal +3-step reproducer, which is what makes an ordering bug fixable at all. + +### 3b. Server hub simulation + +A `ws` package test driving random interleavings of subscribe, broadcast, ack +and disconnect under `-race`, asserting the FIFO and seq property already +stated in `Server/CLAUDE.md`. Seeded and therefore replayable. + +### 3c. Fault-injected transport + +A test-only wrapper that drops, reorders, duplicates and delays frames from a +seed. Shared by 3a and 3b. Deterministic: a failing seed reproduces exactly. + +## Tier 4 — Sharpen the hunt + +The 2026-08-08 client hunt fixed 101 bugs and still did not converge. Four +changes, cheapest first: + +1. **Persistent seen-ledger.** Key on `(file, symbol, class)` and persist + *across* runs, not only within one. Each run currently starts cold and + re-derives ground already covered — the most likely reason convergence never + arrives. +2. **Sibling-sweep lens.** For every confirmed bug, enumerate the other callers + of the touched function. This is the root-cause rule turned into a lens: it + converts one finding into its whole family, which is also what stops the + same class reappearing in the next round. +3. **Coverage-guided targeting.** `make cover-all` already prints every + function at 0.0%. Feed that list to the finders as a priority surface. +4. **Anti-pattern priming.** Supply the fixed-bug corpus as "confirmed-real + classes in this codebase — hunt siblings" rather than starting each finder + from a cold read. + +## Order and effort + +| Step | Effort | Runs in | +| --- | --- | --- | +| 1a `make fuzz` | 15 min to write | 10 min/sweep unattended | +| 1b Stryker hotspots | 0 (already configured) | ~25 min for 3 files | +| 1d gitignore check | 5 min | — | +| 2 first four semgrep rules | ~1 afternoon | seconds | +| 4.1 + 4.2 ledger and sibling lens | ~2 hours | within existing hunt | +| 1c browser-mode vitest | 0 | minutes | +| 3 model-based and chaos harnesses | ~1 day | minutes | + +Tier 1a is first because 14 harnesses — the expensive part — are already +written and produce nothing today. + +## Non-goals + +- No new GitHub Actions workflows, jobs, or scheduled runs. +- No gating of any existing CI check on mutation score or fuzz results. +- No change to the existing test suites' assertions. The client suite is green + and stays green. +- No promotion of semgrep to CI in this scope. +- No replacement of the agentic bug hunt. Tier 4 sharpens it; Tiers 1 to 3 run + beside it. diff --git a/docs/quick-start.md b/docs/quick-start.md index db4fc681..1ed92b27 100644 --- a/docs/quick-start.md +++ b/docs/quick-start.md @@ -55,11 +55,11 @@ Full Docker details: [Deployment Guide](deployment.md#docker-linux). ```bash # Server (Windows) cd Server -go build -o chatserver.exe -ldflags "-s -w -X main.version=1.2.0-alpha.1" . +go build -o chatserver.exe -ldflags "-s -w -X main.version=1.2.0-alpha.2" . # Server (Linux) cd Server -CGO_ENABLED=0 go build -o chatserver -ldflags "-s -w -X main.version=1.2.0-alpha.1" . +CGO_ENABLED=0 go build -o chatserver -ldflags "-s -w -X main.version=1.2.0-alpha.2" . # Client cd Client/tauri-client