diff --git a/reports/2026-05-29-gc-audit.md b/reports/2026-05-29-gc-audit.md new file mode 100644 index 0000000..cf7c051 --- /dev/null +++ b/reports/2026-05-29-gc-audit.md @@ -0,0 +1,243 @@ +# GuruConnect Audit Report — 2026-05-29 + +**Auditor:** Claude (claude-opus-4-8, via /gc-audit) +**Passes:** API Surface, Rust Quality, TypeScript/Dashboard, Protocol Integrity, Security & Session, CI/CD Pipeline, Docs Reconciliation +**Previous audit:** First audit (this is the inaugural /gc-audit run — also a dry run to validate the skill) +**Note:** Gitea was down during this run (internal :3000 refused, public 502). Pass G's Gitea-runner checks are COVERAGE GAPS to re-verify; deploy-host (172.16.3.30) live checks succeeded (separate segment). Living docs were NOT auto-edited this run — proposed deltas are listed for review (dry-run posture). + +--- + +## Executive Summary + +| Pass | Total | Critical | High | Medium | Low | +|------|-------|---------|------|--------|-----| +| API Surface | 18 | 0 | 4 | 4 | 0 | +| Rust Quality | 9 | 1 | 2 | 2 | 2 | +| TypeScript/Dashboard | 12 | 2 | 1 | 3 | 4 | +| Protocol Integrity | 7 | 1 | 2 | 1 | 2 | +| Security & Session | 12 | 3 | 3 | 2 | 2 | +| CI/CD Pipeline | 11 | 0 | 3 | 1 | 1 | +| **TOTAL (delta-deduped)** | **~45 unique** | **4** | **9** | **~10** | **~10** | + +**Requires immediate action (CRITICAL):** +1. `validate_agent_api_key` accepts any valid dashboard JWT as an agent API key → viewer→agent impersonation / session-slot seizure (`server/src/relay/mod.rs:224-239`). +2. Viewer WebSocket: any valid JWT attaches to and controls ANY session by UUID — no per-session/per-machine authorization (`server/src/relay/mod.rs:242-288`). +3. Viewer WebSocket skips the token blacklist → revoked/logged-out tokens still grant live remote control until natural expiry (`server/src/relay/mod.rs:260`). +4. Dashboard `lib/protobuf.ts` is a fabricated fixed-offset codec, wire-incompatible with the real protobuf the agent/server speak → React viewer cannot decode frames or send input (`dashboard/src/lib/protobuf.ts:18-150`). *Functional-critical; currently dead code (prod uses static viewer.html) so production impact is deferred until the component library is mounted.* + +--- + +## Pass 1: API Surface + +The server exposes a substantially larger REST surface than the static dashboard consumes. **No broken calls and no method mismatches** — every static-page `fetch` resolves to a real route with the correct method. The gaps are UI-orphaned server capability. + +### [HIGH] Entire `/api/machines` resource is UI-orphaned; Machines tab is session-backed +**File:** `server/src/main.rs:316-319`; `server/static/dashboard.html:818-821` +**Detail:** `GET /api/machines` (list), `GET /api/machines/:agent_id`, `DELETE /api/machines/:agent_id`, `GET /api/machines/:agent_id/history` have no frontend caller. The dashboard "Machines" tab is populated by `loadMachines()` which actually fetches `/api/sessions`. Persistent-machine inventory, history, and deletion have no UI; if the sessions payload lacks `is_online`/`agent_version`, the tab renders empty/Unknown. +**Recommendation:** Repoint the Machines tab to `GET /api/machines`, or document that the dashboard intentionally shows live sessions; add machine history/delete UI. + +### [HIGH] Full `/api/releases` CRUD has no UI +**File:** `server/src/main.rs:326-333` +**Detail:** `GET/POST /api/releases`, `GET/PUT/DELETE /api/releases/:version` have zero frontend callers; no release-management page exists. +**Recommendation:** Build a releases admin page, or confirm releases are CI/API-managed and mark ops-only. + +### [HIGH] `/api/users/:id/clients` (set_client_access) orphaned +**File:** `server/src/main.rs:302` +**Detail:** users.html has a `manage_clients` permission checkbox but never assigns which clients a user may access. Server supports per-user client scoping with no UI to set it. +**Recommendation:** Add a client-assignment UI, or confirm client-access is managed elsewhere. + +### [HIGH] `useRemoteSession.ts` `/ws/viewer` URL omits the required JWT token +**File:** `dashboard/src/hooks/useRemoteSession.ts:92` +**Detail:** The React hook builds `?session_id=...` with no `token` (and no `viewer_name`), but the viewer WS requires a JWT in the `token` query param (static viewer.html includes it). If the component library is used against this server, viewer WS auth fails. +**Recommendation:** Include the JWT token + viewer_name query params. + +### [MEDIUM] `/api/sessions/:id` (GET single), `/api/changelog/:component/:version`, `/api/auth/blacklist/{stats,cleanup}` orphaned (no UI) +**Files:** `main.rs:313`, `:337-340`, `:284-291` +**Detail:** Session-detail GET, changelog display, and blacklist ops endpoints have no UI consumer. Most are reasonable ops/API-only endpoints; flag and document. + +### [INFO] Genuinely-external routes (not dead) +`/api/version` (agent polling), `/ws/agent` (Rust agent), `/health`, `/metrics` are correctly out-of-band. + +**Pass A summary:** All frontend calls resolve correctly; the issue is large UI-orphaned server CRUD — the entire `/api/machines` resource (the Machines tab is actually `/api/sessions`-backed), all of `/api/releases`, changelog, and `/api/users/:id/clients` client-scoping. Highest-impact: missing machine-management UI and the `useRemoteSession` missing-token bug. + +--- + +## Pass 2: Rust Code Quality + +### [CRITICAL] Auth-plane leak — dashboard JWT accepted as agent API key +**File:** `server/src/relay/mod.rs:224-239` +**Detail:** `validate_agent_api_key` returns `true` for any token passing `jwt_config.validate_token`. A viewer-plane credential (dashboard JWT) is thus a valid agent-plane credential — a leaked/low-priv JWT can register/impersonate an agent. The code comment admits "allows using dashboard token for testing." +**Recommendation:** Drop the JWT-accept branch; validate `api_key` only against `AGENT_API_KEY` (and future per-agent keys with `role == "agent"`). *(Also surfaced by Pass E as CRITICAL with session-level impact.)* + +### [HIGH] Initial-admin password logged to tracing on fallback path +**File:** `server/src/main.rs:175` +**Detail:** Primary path writes creds to a 0600 file; if `fs::write` fails, `info!(" Password: {}", password)` emits the cleartext password to the log stream (journald/aggregators). +**Recommendation:** On write failure, print to stdout only or abort startup; never route the secret through the structured logger. + +### [HIGH] Rate limiting disabled and non-compiling +**File:** `server/src/middleware/mod.rs:3-11`, `middleware/rate_limit.rs`, `main.rs:268,303` +**Detail:** `rate_limit` module is commented out ("not yet functional"); `rate_limit.rs` has invalid type signatures; login/change-password/code-validate routes carry only `// TODO: Add rate limiting`. Brute-force protection on auth is absent. *(Cross-ref Pass E, Pass F — SEC-2 not fixed; roadmap `[x]` partially false.)* + +### [MEDIUM] Stub admin endpoint returns 501 +**File:** `server/src/api/auth_logout.rs:99-131` +**Detail:** `POST /api/auth/admin/revoke-user` is registered/reachable but always `NOT_IMPLEMENTED` — admins cannot force-revoke a compromised user's tokens. + +### [MEDIUM] Agent run-loop `.unwrap()` on transport +**File:** `agent/src/session/mod.rs:312,392,465` +**Detail:** `self.transport.as_mut().unwrap()` — in-loop invariant today, but any reconnect refactor turns it into a hard agent crash. Use `ok_or_else(...)?`. + +### [LOW] Support codes logged in cleartext; `/metrics` mutex `.unwrap()` panic +**Files:** `relay/mod.rs:107,134,165,377`, `main.rs:413,415` + +### [INFO] Acceptable panics / correct usage +Startup `JWT_SECRET` presence + ≥32-char enforcement (`main.rs:94-100`) is the required fail-closed behavior; static-literal `parse().unwrap()` in CORS/headers is infallible; agent `println!`/`eprintln!` are user-facing CLI output (version-info/help), not logging violations. sqlx compile-time macros used throughout — correct for GC, not flagged. Logout blacklist IS consulted per-request for REST (`auth/mod.rs:116`); JWT `exp` enforced; Argon2id confirmed; no raw `e.to_string()` leaks to clients. + +**Pass B summary:** Server is broadly disciplined (tracing, anyhow, Argon2id, exp enforcement, per-request blacklist on REST, clean error handling, sqlx macros correctly used). Standout problem is the CRITICAL one-way auth-plane leak; secondary: admin-password-on-fallback logging and fully-disabled rate limiting. + +--- + +## Pass 3: TypeScript / Dashboard Quality + +### [CRITICAL] `lib/protobuf.ts` is not protobuf — fabricated fixed-offset layout, wire-incompatible +**File:** `dashboard/src/lib/protobuf.ts:93-150` (decode), `:18-88` (encode) +**Detail:** The server relays genuine prost-encoded protobuf `Message` bytes verbatim (`relay/mod.rs:449,454`). A real `Message{video_frame}` starts with tag byte `0x52` (field 10 << 3 | 2). The decoder instead checks `getUint8(0) === 10` and reads phantom fixed offsets, so **every real frame returns null** (canvas never renders). The static `server/static/viewer.html` parses the same wire tree correctly — the two implementations have diverged. +**Recommendation:** Delete the hand-rolled codec; port viewer.html's varint/length-delimited parser, or use protobufjs/generated code. + +### [CRITICAL] Encoders emit the same fabricated layout — input events undecodable by the agent +**File:** `dashboard/src/lib/protobuf.ts:18-46, 51-88` +**Detail:** `encodeMouseEvent`/`encodeKeyEvent` write a flat struct with a leading message-id byte. The server decodes viewer input via `proto::Message::decode` (`relay/mod.rs:666`) and rejects these as malformed → clicks/keys silently dropped. + +### [HIGH] Decoder bounds-checking incomplete — short/hostile frames read OOB +**File:** `dashboard/src/lib/protobuf.ts:93-117` +**Detail:** Only `length < 2` guarded, then reads through offset 21. A 3–20-byte frame throws `RangeError` into an uncaught promise (`useRemoteSession.ts:60`). Validate full header length; wrap decode in try/catch. + +### [MEDIUM] `sequence` decoded but out-of-order frames never dropped; WS hook reconnects forever with no backoff/error surface; `fzstd` decompression is a no-op stub +**Files:** `RemoteViewer.tsx:40-87`, `useRemoteSession.ts:105-125`, `protobuf.ts:156-162` + +### [LOW] Single union cast hides out-of-range `encoding`; SessionControls `