From 929438a2b7e8a7e7570ad9013449e57780b63b20 Mon Sep 17 00:00:00 2001 From: J3vb <192430104+J3vb@users.noreply.github.com> Date: Sat, 8 Aug 2026 20:51:43 +0200 Subject: [PATCH] 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) --- Client/tauri-client/CLAUDE.md | 10 ++++- docs/plans/bug-detection-improvements.md | 48 +++++++++++++++++------- 2 files changed, 43 insertions(+), 15 deletions(-) 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/docs/plans/bug-detection-improvements.md b/docs/plans/bug-detection-improvements.md index af428fa7..de0ca834 100644 --- a/docs/plans/bug-detection-improvements.md +++ b/docs/plans/bug-detection-improvements.md @@ -67,9 +67,13 @@ indistinguishable at a glance from a real security finding. Confirm with crasher. Committed seed corpus files live in the same directory — deleting the directory takes them with it. Remove the single offending file by name. -**Semgrep is the one candidate for later promotion to public CI.** Its rules -are deterministic and its failures are regressions of already-fixed bugs, not -disclosures of novel ones. Promotion is out of scope here. +**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 @@ -146,18 +150,36 @@ and `docs/audit-*.md`. `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. -**Rules to write first:** +### Tier 2 — delivered 2026-08-08 -| Class | Detector | Source | -| --- | --- | --- | -| `await` between reading a store snapshot and `registerNow` | semgrep | recurring reconnect-transfer hotspot | -| global `leaveVoice()` inside a superseded-attempt path | semgrep | client `CLAUDE.md` invariant | -| peer marked verified without an epoch/keypair staleness check | semgrep | client `CLAUDE.md` invariant | -| seq allocated for a frame that can subsequently be dropped | Go runtime assertion under `-tags deadlock` | server `CLAUDE.md` invariant | +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: -**Location:** rules in `.semgrep/owncord/*.yaml`, fixtures alongside as -`.semgrep/owncord/.test.{ts,go}`. Run via `npx semgrep --config -.semgrep/owncord`. +| 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