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