officerdev/platform
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

step 4/4: the docs say permissions too, and capability means one thing again

Browse Source 
44 files of prose — CLAUDE.md, AGENTS.md, TODO.md, 20 docs, both plugin design
documents, and the comment surface the earlier steps could not reach.

Applied against an explicit keep-list, not swept, because the word turned out to
have SIX meanings in this repository rather than the three the offscale doc
recorded:

  permissions          renamed (steps 1–2)
  $OFFICER_ROOT/capabilities/  KEPT — the item store, and now the only thing
                               the word means that is ours
  sidecar routing keys renamed to `handles` (step 3)
  Lightning wallet     KEPT — a domain term, and on the wire to the mobile apps
  terminfo queries     KEPT — XTGETTCAP, in the pty sidecar
  InvoiceShelf         KEPT — per-resource { write, bulkDelete } flags

The sweep still falsified two things, both caught by checking rather than by
review, and both in prose that discusses more than one meaning at once:

CLAUDE.md began claiming the item store lives at `$OFFICER_ROOT/permissions`.
It does not; that directory is on disk and full of skills and tools.

And the offscale doc's own note about the collision became
"Named `permissions`, NOT `permissions`" — a sentence that had eaten the thing
it existed to warn about.

Both restored, and the note rewritten to say what is now true: capability means
one thing of ours, and three that belong to somebody else's vocabulary.

Verified live after restart: self and admin permission endpoints 200, gated
route 200, agent-status 200, 9 grants intact with 6 permissions offered.
tsgo clean, 797 tests, 787 pass, same 7.

The rename is done. Four steps, no data lost, no client break that survived
the step it was introduced in.
This commit is contained in:
pastilhas
2026-08-15 16:31:11 +00:00
parent f9fd002ff4
commit 027b10bd6e
45 changed files with 721 additions and 694 deletions
Download Patch File Download Diff File Expand all files Collapse all files
docs/sidecar-audit-2026-07.md
+140 -140
View File
@@ -31,10 +31,10 @@ Three consequences worth stating explicitly, because the audit turned on the thi
`src/servers/api/slskd/` is **70 lines total** and does exactly the two things it should:
| File | Lines | Role |
|---|---|---|
| `router.ts` | 51 | `all('/*')` catch-all. Forwards subpath + query + body, injects `X-Officer-User`, streams the response back. No routes of its own. |
| `sidecar-server.ts` | 19 | Remembers the port the sidecar reports on connect (`slskd:server`). Nothing else. |
| File | Lines | Role |
| ------------------- | ----- | ---------------------------------------------------------------------------------------------------------------------------------- |
| `router.ts` | 51 | `all('/*')` catch-all. Forwards subpath + query + body, injects `X-Officer-User`, streams the response back. No routes of its own. |
| `sidecar-server.ts` | 19 | Remembers the port the sidecar reports on connect (`slskd:server`). Nothing else. |
- `SLSKD_URL` / `SLSKD_API_KEY` are read in **exactly one file**: `src/servers/sidecar/slskd/upstream.ts`.
The platform never sees either.
@@ -49,24 +49,24 @@ Today's three commits (`8032c8b`, `b7b91a2`, `dea9ee2`) touched **zero** platfor
The `soulseek_*` tables live in the shared `officer_db` package (`schema/soulseek.ts`,
`queries/soulseek.ts`) rather than in the sidecar. Only the sidecar reads them — this was a
deliberate call (one database, schema isolated in its own file, `soulseek_` prefix) and it stands.
The cost to remember: `bun db:push` diffs the *whole* schema, which is why soulseek DDL is
The cost to remember: `bun db:push` diffs the _whole_ schema, which is why soulseek DDL is
hand-applied.
## The smell: the frontend speaks slskd
**37 raw `/slskd/api/v0/…` calls from React, against 10 `/slskd/_officer/…` calls.**
| File | Raw slskd calls |
|---|---|
| `SoulseekTransfers.tsx` | 8 |
| `SoulseekRooms.tsx` | 6 |
| `SoulseekChat.tsx` | 5 |
| `SoulseekDashboard.tsx` | 4 |
| `SearchView.tsx` | 4 |
| `SoulseekSystem.tsx` | 3 |
| `SearchResults.tsx` | 3 |
| `useSoulseekUser.ts` | 2 |
| `SoulseekUploads.tsx` | 2 |
| File | Raw slskd calls |
| ----------------------- | --------------- |
| `SoulseekTransfers.tsx` | 8 |
| `SoulseekRooms.tsx` | 6 |
| `SoulseekChat.tsx` | 5 |
| `SoulseekDashboard.tsx` | 4 |
| `SearchView.tsx` | 4 |
| `SoulseekSystem.tsx` | 3 |
| `SearchResults.tsx` | 3 |
| `useSoulseekUser.ts` | 2 |
| `SoulseekUploads.tsx` | 2 |
The proxy stays thin, so rule (1) and (2) hold. Rule (3) does not: `shared.ts` defines slskd's wire
types (`SlskdUserStatus`, `SlskdUserInfo`, `SlskdTransferUser`, …), and the panels know slskd's URL
@@ -117,13 +117,13 @@ Under that line, the three files above are the work. The other six are a naming/
`src/servers/sidecar/slskd/` — what "the sidecar owns its job" already looks like:
| File | Role |
|---|---|
| `index.ts` | Reverse proxy to slskd on a random loopback port; documents the whole `/api/slskd/*` contract; reports its port to the platform. |
| `upstream.ts` | The only holder of `SLSKD_URL` / `SLSKD_API_KEY`. |
| `officer.ts` | The `/_officer/*` routes — favourites, browse snapshots, tree levels, filtered search, downloads. Features slskd has no concept of. |
| `browse.ts` | Multi-minute background share-tree fetch, tree built at ingest. Outlives any request. |
| `download.ts` | Expands a browsed folder into a file list from cache and enqueues it. `MAX_ENQUEUE` guard. |
| File | Role |
| ------------- | ----------------------------------------------------------------------------------------------------------------------------------- |
| `index.ts` | Reverse proxy to slskd on a random loopback port; documents the whole `/api/slskd/*` contract; reports its port to the platform. |
| `upstream.ts` | The only holder of `SLSKD_URL` / `SLSKD_API_KEY`. |
| `officer.ts` | The `/_officer/*` routes — favourites, browse snapshots, tree levels, filtered search, downloads. Features slskd has no concept of. |
| `browse.ts` | Multi-minute background share-tree fetch, tree built at ingest. Outlives any request. |
| `download.ts` | Expands a browsed folder into a file list from cache and enqueues it. `MAX_ENQUEUE` guard. |
Other sidecars for comparison: `claude`, `email`, `music`, `opencode`, `vault`, `vnc`.
@@ -137,16 +137,16 @@ re-derived later.
The eight sidecars, from `ecosystem.config.cjs`:
| PM2 process | Entry point |
|---|---|
| `officer-claude` | `src/servers/sidecar/claude/index.ts` |
| `officer-opencode` | `src/servers/sidecar/opencode/index.ts` |
| `officer-email` | `src/servers/sidecar/email/index.ts` |
| `officer-pty` | `src/servers/api/terminal/pty-sidecar.mjs` ← note the path |
| `officer-vnc` | `src/servers/sidecar/vnc/index.ts` |
| `officer-music` | `src/servers/sidecar/music/index.ts` |
| `officer-vault` | `src/servers/sidecar/vault/index.ts` |
| `officer-slskd` | `src/servers/sidecar/slskd/index.ts` |
| PM2 process | Entry point |
| ------------------ | ---------------------------------------------------------- |
| `officer-claude` | `src/servers/sidecar/claude/index.ts` |
| `officer-opencode` | `src/servers/sidecar/opencode/index.ts` |
| `officer-email` | `src/servers/sidecar/email/index.ts` |
| `officer-pty` | `src/servers/api/terminal/pty-sidecar.mjs` ← note the path |
| `officer-vnc` | `src/servers/sidecar/vnc/index.ts` |
| `officer-music` | `src/servers/sidecar/music/index.ts` |
| `officer-vault` | `src/servers/sidecar/vault/index.ts` |
| `officer-slskd` | `src/servers/sidecar/slskd/index.ts` |
## music — partially compliant
@@ -154,15 +154,15 @@ The eight sidecars, from `ecosystem.config.cjs`:
**cliamp audio subsystem**: the same domain — playing local audio to the browser — implemented
entirely in the main process, with no sidecar owning any of it.
| Surface | Lines |
|---|---|
| Compliant proxy: `api/music/router.ts` + `api/music/sidecar-server.ts` | 88 |
| `hono.ts` (3) + `protocol.ts` (1) | 4 |
| `api/cliamp/websocket.ts` | 201 |
| `api/cliamp/audio-ws.ts` | 91 |
| `server.tsx` PulseAudio bootstrap | 46 |
| `api/cliamp/asoundrc` + `server.tsx` cliamp wiring | 18 |
| **Audio domain in the main process** | **~356** |
| Surface | Lines |
| ---------------------------------------------------------------------- | -------- |
| Compliant proxy: `api/music/router.ts` + `api/music/sidecar-server.ts` | 88 |
| `hono.ts` (3) + `protocol.ts` (1) | 4 |
| `api/cliamp/websocket.ts` | 201 |
| `api/cliamp/audio-ws.ts` | 91 |
| `server.tsx` PulseAudio bootstrap | 46 |
| `api/cliamp/asoundrc` + `server.tsx` cliamp wiring | 18 |
| **Audio domain in the main process** | **~356** |
1. **cliamp player process management** — `api/cliamp/websocket.ts:1-201`. Locates the `cliamp` binary
by probing `Bun.which` plus three GOPATH candidates (`:48-66`), resolves and traversal-validates the
@@ -171,20 +171,20 @@ entirely in the main process, with no sidecar owning any of it.
`PULSE_SINK: 'virtual_out'` and `ALSA_CONFIG_PATH` injected. Pumps stdout/stderr into JSON frames
(`:124-160`), forwards `{type:'input'}` to stdin (`:173-185`), kills the child on close (`:187-198`).
Child processes are held in a module-level `Map` (`:22`).
*Belongs in* `sidecar/music/`, which already runs its own loopback HTTP server
(`sidecar/music/index.ts:140`). *Obstacle:* a browser-held WebSocket with bidirectional keystroke
_Belongs in_ `sidecar/music/`, which already runs its own loopback HTTP server
(`sidecar/music/index.ts:140`). _Obstacle:_ a browser-held WebSocket with bidirectional keystroke
traffic — but the relay pattern already exists twice (`server.tsx:164-228` for dev-server,
`server.tsx:323-326` for vault).
2. **PulseAudio host-daemon bootstrap** — `server.tsx:391-436`. A startup IIFE that locates
`pulseaudio`/`pactl`, runs `pulseaudio --start -D` if the daemon is down (`:401-411`), then greps
`pactl list short sinks` and loads `module-null-sink sink_name=virtual_out` if absent (`:414-435`).
Runs unconditionally at every boot even if nobody opens the player.
*Belongs in* the music sidecar's startup. *Obstacle:* none technical — same host, `pactl` works
_Belongs in_ the music sidecar's startup. _Obstacle:_ none technical — same host, `pactl` works
identically. Must move together with (1) and (3), since the sink must exist before they start.
3. **Host audio capture → browser PCM** — `api/cliamp/audio-ws.ts:1-91`. Spawns
`parec --format=s16le --rate=44100 --channels=2 -d virtual_out.monitor` (`:28-33`) and pushes each
chunk to the browser as a binary frame (`:44-73`). Hardcoded format, sample rate, channel count and
monitor device name — pipeline domain knowledge. *Obstacle:* continuous binary PCM, so a relay hop
monitor device name — pipeline domain knowledge. _Obstacle:_ continuous binary PCM, so a relay hop
costs a copy per chunk.
4. **ALSA config shipped inside the API tree** — `api/cliamp/asoundrc:1-9`, passed via
`ALSA_CONFIG_PATH` (`websocket.ts:8`, `:112`). Upstream config in the thin-proxy process. Moves for
@@ -231,25 +231,26 @@ sidecar owns, but it belongs to the file-browser contract.
The transport proxy is right; the platform owns the entire Vaultwarden **auth/session/key-custody**
domain. This is the worst offender of the eight, and the one where placement has real consequences.
| Surface | Lines |
|---|---|
| `api/vault/router.ts` | 169 |
| `api/vault/websocket.ts` | 164 |
| `api/vault/broker.ts` | 79 |
| `api/vault/token-store.ts` | 34 |
| `api/vault/proxy-util.ts` | 29 |
| `api/vault/sidecar-server.ts` | 24 |
| **`api/vault/` total** | **499** |
| `hono.ts` (7) + `server.tsx` (8) + `protocol.ts` (2) + auth handlers (8) | 25 |
| `officerdb` vault layer the platform calls: `queries/vault.ts` + `crypto.ts` + `schema/vault.ts` | 176 |
| **Total** | **~702** (~525 excluding the shared DB package) |
| Surface | Lines |
| ------------------------------------------------------------------------------------------------ | ----------------------------------------------- |
| `api/vault/router.ts` | 169 |
| `api/vault/websocket.ts` | 164 |
| `api/vault/broker.ts` | 79 |
| `api/vault/token-store.ts` | 34 |
| `api/vault/proxy-util.ts` | 29 |
| `api/vault/sidecar-server.ts` | 24 |
| **`api/vault/` total** | **499** |
| `hono.ts` (7) + `server.tsx` (8) + `protocol.ts` (2) + auth handlers (8) | 25 |
| `officerdb` vault layer the platform calls: `queries/vault.ts` + `crypto.ts` + `schema/vault.ts` | 176 |
| **Total** | **~702** (~525 excluding the shared DB package) |
For scale: the sidecar itself is 295 lines and is a genuine dumb pass-through
(`sidecar/vault/index.ts:153-183` streams bodies verbatim; `upstream.ts:12-22` is the only reader of
`VAULTWARDEN_URL`).
Two structural notes before the findings:
- `api/vault/router.ts:123` *is* an `all('/*')` catch-all, but it is not thin — it **replaces** the
- `api/vault/router.ts:123` _is_ an `all('/*')` catch-all, but it is not thin — it **replaces** the
`Authorization` header with a platform-held upstream credential (`:137-141`) and implements
401-refresh-retry (`:158-165`).
- It does **not** inject `X-Officer-User` (contrast `api/slskd/router.ts:34`,
@@ -274,10 +275,10 @@ Two structural notes before the findings:
every proxied request.
3. **Notifications WebSocket proxied twice, with token injection** — `api/vault/websocket.ts:1-164`.
`injectToken` (`:45-52`) rewrites the SignalR query string: drops `token`, sets `access_token=<vw
token>`. `open` (`:55-118`) verifies the platform JWT, fetches the upstream token, dials
token>`. `open` (`:55-118`) verifies the platform JWT, fetches the upstream token, dials
`ws://127.0.0.1:<sidecarPort>` and runs a full buffered bidirectional pipe — **which the sidecar
already implements** (`sidecar/vault/index.ts:45-114`, `:143-151`). Frames are relayed twice.
*Obstacle:* Bun requires a synchronous upgrade, hence the deferred validation at `:60-70`; that
_Obstacle:_ Bun requires a synchronous upgrade, hence the deferred validation at `:60-70`; that
pattern stays, the token lookup at `:73` should not.
4. **The platform is the vault's key escrow** — `api/vault/router.ts:107-120`. `PUT /unlock-key`
persists a `wrappedKey` (`:111`); `GET /unlock-key` hands it back to any owner session (`:117-119`).
@@ -290,7 +291,7 @@ Two structural notes before the findings:
`queries/vault.ts:22-23,34-35,67-68,83,88`. Derives an AES-256-GCM key as
`SHA-256(VAULT_STORE_KEY)` (`:12-20`) and runs `createCipheriv`/`createDecipheriv` (`:23-39`).
Because the vault router imports `officerdb` (`router.ts:10`, `token-store.ts:1`), all of this runs
inside `officer`. *Obstacle:* `crypto.ts` lives in the shared package, so it is importable from
inside `officer`. _Obstacle:_ `crypto.ts` lives in the shared package, so it is importable from
anywhere; moving it means moving the vault queries out of the shared package or enforcing a
sidecar-only import boundary. No config obstacle — both processes read the same `.env`.
6. **Vault tables are read/written by the platform, not the sidecar** — `queries/vault.ts:18-101`,
@@ -299,7 +300,7 @@ Two structural notes before the findings:
imports **no** DB module at all. Exact inverse of the intended ownership.
7. **Auth flows reach into vault storage directly** — `api/auth/signout.ts:10`,
`revoke-handler.ts:18-19`, `panic-handler.ts:15-16`. Signout deletes the token row; distress and
panic also burn the protector key. The *policy* is platform-level; the *mechanism* — direct DELETEs
panic also burn the protector key. The _policy_ is platform-level; the _mechanism_ — direct DELETEs
against the sidecar's tables — is not. All three are already best-effort `.catch(() => {})`, so
failure semantics wouldn't worsen behind a sidecar call.
8. **Dead weight** — `router.ts:38-48` is a hand-written `GET /_health` passthrough the catch-all
@@ -309,13 +310,13 @@ Two structural notes before the findings:
strings — fix (2) and it's unnecessary.
9. **Mounted outside the protected tree** — `hono.ts:73-77`. `route('/api/vault', …)` sits outside
`protectedRouter`, so the router re-implements its own stack (`router.ts:31-33`: `originMiddleware`,
`userMiddleware`, `ownerGate`). *Real constraint, probably why:* it deliberately avoids
`userMiddleware`, `ownerGate`). _Real constraint, probably why:_ it deliberately avoids
`bodyParser()` so bodies stream (`router.ts:14`), and `protectedRouter` would inherit it from
`hono.ts:88` and buffer vault attachments.
10. **Stale comments on a security boundary** — `hono.ts:73-76` and
`origin-validation.ts:36-39,61-64` both claim vault requests "carry their own Bitwarden bearer
token, not a platform session JWT" and that `userMiddleware` would 401 them. Untrue since
`router.ts:32-33` requires a valid platform JWT *and* owner status on every request. Also,
`router.ts:32-33` requires a valid platform JWT _and_ owner status on every request. Also,
`VAULT_AUTH_SPEC.md` (cited at `router.ts:12`, `schema/vault.ts:4`) and
`BITWARDEN_SIDECAR_PROMPT.md` (cited at `sidecar/vault/upstream.ts:3`) **do not exist** in the repo.
Not logic, but exactly the drift that makes someone loosen a gate by mistake.
@@ -327,7 +328,7 @@ every `officerdb` vault export across `src/servers` and `src/databases`; the onl
User-key derivation is genuinely client-side (the platform only relays `Kdf*` params at
`router.ts:96-101`) — the one credential decision that is correctly placed.
*Shortest path to compliance (inferred, not attempted):* move `broker.ts`, `token-store.ts`,
_Shortest path to compliance (inferred, not attempted):_ move `broker.ts`, `token-store.ts`,
`/session/login`, `/unlock-key`, the vault queries and `crypto.ts` into `sidecar/vault/`; have the
router inject `X-Officer-User` instead of `Authorization`; reduce `websocket.ts` to
origin-check + verify + upgrade + dumb pipe; delete `/_health` and `proxy-util.ts`; replace the three
@@ -342,7 +343,7 @@ The largest violation after email, and the one with the worst consequences, beca
for survivability. They overlap deliberately.
There is **no `/api/claude` mount, no proxy router, and no `X-Officer-User` anywhere on this path.**
Nothing here is shaped like slskd. The platform does not forward to the claude sidecar; it *drives* it,
Nothing here is shaped like slskd. The platform does not forward to the claude sidecar; it _drives_ it,
over a typed RPC vocabulary, and interprets everything that comes back.
A structural fact worth stating before the list, because it inverts the usual reading: **the sidecar
@@ -363,7 +364,7 @@ than the owner of it. Every other item below is downstream of that.
the machinery that makes a restart lossy — it is the durable writer, and it sits on the far side
of the socket from the process producing the events.
3. **`api/chat/claude-sessions.ts:1-361` — a reimplementation of Claude's transcript format.** The
platform reads and *writes* `~/.claude/projects/<slug>/<uuid>.jsonl` directly: the slug encoding
platform reads and _writes_ `~/.claude/projects/<slug>/<uuid>.jsonl` directly: the slug encoding
(`:39`), the entry schema (`:69-78`), content-block decoding (`:143-227`), listing (`:350-361`),
delete-by-unlink (`:253-258`), a 32KB `readSync` plus a `"cwd":"…"` regex to recover a session's
directory (`:294-306`), and — the sharpest example — **rename implemented by appending a
@@ -378,7 +379,7 @@ than the owner of it. Every other item below is downstream of that.
`spawnAndWaitForRegistration`: a per-email `Bun.spawn` of `user-instance.ts` with
`stdout: 'inherit', stderr: 'inherit'` (`:240-241`), a `claudeProcs` Map, a `claudeSpawnWaiters`
Map, a 15s timeout and a 50ms registration poll (`:259-267`). Plus the claude verbs at `:306-358`
and a broadcast fallback at `:339-346`. `:88-90` uses `capabilities.includes('proxy')` as a
and a broadcast fallback at `:339-346`. `:88-90` uses `permissions.includes('proxy')` as a
stand-in for "is this the claude sidecar", which is only true by accident of naming.
6. **`generate-container-context.ts:135-182` (+ `:50-133`) — the platform writes the CLI's config.**
It authors `~/.claude/settings.json`: a `Stop` hook curling
@@ -387,7 +388,7 @@ than the owner of it. Every other item below is downstream of that.
(`:163-179`). Called from `users/provision.ts:28-38`. Two notes: the hook points at the platform,
so it fails during exactly the restart window that matters; and the permission posture is a
deliberate documented choice (`platform/CLAUDE.md`: agents run unsandboxed as the owner) that is
being *implemented in the wrong process*, not a mistake.
being _implemented in the wrong process_, not a mistake.
7. **`api/activity/router.ts:1-191` — the platform walks the agent's scratch tree.** Reads
`/tmp/claude-<uid>/<encoded-cwd>/tasks/<id>.output` (`:24-61`, keyed on
`startsWith('claude-')` at `:34`) and tails it over SSE (`:116-191`). Another private layout the
@@ -419,7 +420,7 @@ than the owner of it. Every other item below is downstream of that.
`sk-ant-api03-<uuid>` keys (`:10`) — the live copy is `sidecar/claude/proxy.ts:135`. A stale second
implementation of the credential path is worth deleting on security grounds alone, not just tidiness.
*What the sidecar already has right:* the Anthropic proxy genuinely lives in the PM2-managed sidecar
_What the sidecar already has right:_ the Anthropic proxy genuinely lives in the PM2-managed sidecar
(`sidecar/claude/index.ts:20`), so the platform never holds an API key at rest, and `ANTHROPIC_BASE_URL`
points at the sidecar (`sidecar-registry.ts:234`). The credential path is roughly correct. It is the
process topology, the transport direction and the domain logic that are not.
@@ -428,7 +429,7 @@ process topology, the transport direction and the domain logic that are not.
The worst of the eight by volume, and the only one where the arrow points backwards end to end:
**≈3,238 platform lines** (2,875 of them in seven files) against a **314-line sidecar** — and the
sidecar *imports platform code back out* (`sidecar/email/email-idle.ts:3` imports
sidecar _imports platform code back out_ (`sidecar/email/email-idle.ts:3` imports
`../../api/email/resync`). There is no proxy router, no `email:server` port event, and no forwarding of
any kind. `emailRouter` implements 18 concrete endpoints itself.
@@ -460,7 +461,7 @@ Read plainly: the sidecar is a cron/IDLE trigger, and the platform is the mail c
work.** `gmailResync` (`:43-63`), `imapResync` (`:148-264`), `resolveImapAuth` (`:118-146`),
`refreshCredentials` (`:21-41`), and `performResync` (`:276-284`) which coalesces concurrent
resyncs through an **in-process Map**. It is imported by both `sidecar/email/email-cron.ts:2` and
`email-idle.ts:3` *and* by `accounts.ts:159` — i.e. by two different processes. Each gets its own
`email-idle.ts:3` _and_ by `accounts.ts:159` — i.e. by two different processes. Each gets its own
copy of the Map, so the coalescing silently does nothing across the boundary. This is what
"importing platform code back out" costs.
6. **`api/email/accounts.ts:1-259` — account setup does live IMAP.** Validation by real connection on
@@ -486,7 +487,7 @@ Read plainly: the sidecar is a cron/IDLE trigger, and the platform is the mail c
12. **`src/servers/sidecar/email-cron.ts` — 92 dead lines**, imported by nothing (the live one is
`sidecar/email/email-cron.ts`).
*Shortest path (inferred):* this one is a rewrite, not a move. The realistic first step is not
_Shortest path (inferred):_ this one is a rewrite, not a move. The realistic first step is not
relocating `email-db.ts` — it is deleting the duplicate clients (items 7 and 8) and moving the two
queue handlers (items 2 and 3) into the sidecar so sync stops dying with `officer`. The store itself
can follow later, behind a proxy router.
@@ -521,7 +522,7 @@ already does the same job. That makes this the cheapest of the non-compliant sur
6. **`channels/send-opencode.ts:29-66` — the terminal-event set (`:33-37`) and a resume policy keyed
on the `ses_` id prefix (`:42-45`).** Protocol knowledge encoded as a string prefix, in the
platform.
7. **`api/chat/list-models.ts:2, 11-59` — fetches `/config/providers` and then invents capability
7. **`api/chat/list-models.ts:2, 11-59` — fetches `/config/providers` and then invents permission
metadata for the results (`:42-45`).**
8. **`api/chat/chat.ts:13-19, 44, 56-57, 68, 80-81, 91` — CRUD dispatch on `isOpenCodeSessionId`**
(`opencode-sessions.ts:112`, a `startsWith('ses_')` test).
@@ -573,11 +574,11 @@ already exists in the same codebase.
backoff table (`:37` vs `connect.ts:22`). Its types are JSDoc (`:39`), so `protocol.ts:145-156` is
unenforced against it. The actual blocker to moving it is mundane: sibling `templates/` files on
disk (`:27-29, 61, 66, 71` — `.zshrc`, `.tmux.conf`, `starship-officer.toml`, and an unused
`.zshenv`). So a `git mv`, not a rewrite. *(Inferred: the `.mjs`/node choice is probably a
node-pty native-addon workaround — corroborated by the comment at `api/cliamp/websocket.ts:100`.)*
`.zshenv`). So a `git mv`, not a rewrite. _(Inferred: the `.mjs`/node choice is probably a
node-pty native-addon workaround — corroborated by the comment at `api/cliamp/websocket.ts:100`.)_
3. **Every PTY byte transits the main process, double-JSON-encoded.** Plus terminal-specific query
parsing in the shared upgrade handler (`server.tsx:248-252`, `WSData:51-52`) and wiring at `:7, 38,
143, 234, 335`. Auth at `:236-246` is correct. Identity ships **inside the payload** as
143, 234, 335`. Auth at `:236-246` is correct. Identity ships **inside the payload** as
`userLabel` / `sessionId` (`websocket.ts:56, 65`) instead of as `X-Officer-User`.
4. **`sidecar-registry.ts:395-407` plus PTY types threaded through generic plumbing** at
`:12-13, 35, 93, 125, 153, 171, 180, 194`. One subtlety to preserve: the 30s
@@ -588,7 +589,7 @@ already exists in the same codebase.
than `getOwnerHomeDir` (`data-path.ts:34`), unlike the eight other host-executing surfaces. Same
result on this machine (`HOME_DIR` is set and equals `HOME`), divergent anywhere it isn't.
*Shortest path (inferred):* `git mv` the sidecar into `src/servers/sidecar/pty/` with its templates,
_Shortest path (inferred):_ `git mv` the sidecar into `src/servers/sidecar/pty/` with its templates,
switch it to `connect.ts`, move the `PtyInitConfig` construction and cwd resolution into it, and
replace `websocket.ts` with the `devServerWebsocket` relay shape. The detach-on-disconnect policy moves
with it.
@@ -625,7 +626,7 @@ pile of leaked logic.
7. Wiring at `server.tsx:13, 45, 149, 234, 339` is fine, and **`hono.ts:37, 122` is already
reference-shaped** (two lines).
*Adjacent, and its own domain rather than a vnc violation:* the browser relay —
_Adjacent, and its own domain rather than a vnc violation:_ the browser relay —
`server.tsx:369, 371` plus `api/browser/relay.ts` (677 lines), `api/browser/router.ts` (198, including
`Bun.spawn(['zip', …])` at `:26-30`), `cdp.ts` (99) and `relay-auth.ts` (42); and
`api/scrape/scrape.ts:9-19, 49+` launches chromium in-process. Noted for a future pass; not counted
@@ -640,19 +641,19 @@ eight times.
Sorted by how far each is from the reference. This is the whole audit in one view:
| sidecar | platform lines | verdict |
|---|---:|---|
| slskd | 70 | ✅ reference |
| music | 88 | ✅ compliant (the cliamp subsystem beside it is not) |
| pty | 169 | ✗ ~all of it is sidecar logic |
| vnc | 170 | ✗ blocked on a missing listener |
| vault | 499 | ◐ partial, security-relevant |
| opencode | 478 | ✗ (~330 lines deletable as dead) |
| claude | ~2,300 | ✗ |
| email | ~2,875 | ✗ no proxy exists at all |
| sidecar | platform lines | verdict |
| -------- | -------------: | ---------------------------------------------------- |
| slskd | 70 | ✅ reference |
| music | 88 | ✅ compliant (the cliamp subsystem beside it is not) |
| pty | 169 | ✗ ~all of it is sidecar logic |
| vnc | 170 | ✗ blocked on a missing listener |
| vault | 499 | ◐ partial, security-relevant |
| opencode | 478 | ✗ (~330 lines deletable as dead) |
| claude | ~2,300 | ✗ |
| email | ~2,875 | ✗ no proxy exists at all |
`hono.ts` mounts **36 routers. Three are thin sidecar proxies** — `:106` (music), `:107` (slskd), and
`:77` (vault, mounted *outside* `protectedRouter`).
`:77` (vault, mounted _outside_ `protectedRouter`).
For contrast, sidecar-side LOC: music 1,630 · claude 1,523 · slskd 653 · opencode 427 · vnc 326 ·
email 314 · vault 295. Note the inversion on email: 314 sidecar lines to 2,875 platform lines.
@@ -664,7 +665,7 @@ file-browser 1,465 · server-settings 1,452 · browser 1,105 · auth 607 · syst
### 2. The protocol is not a transport
`sidecar/protocol.ts` is a **closed union of ~34 message types: 7 transport, 25+ domain.** Every new
sidecar capability requires editing a shared platform file — which is why domain knowledge keeps
sidecar permission requires editing a shared platform file — which is why domain knowledge keeps
landing there (CLI flags, `display`/`pid`, `proxySecret`, spawn params).
Two specific consequences:
@@ -675,7 +676,7 @@ Two specific consequences:
string test (`server.tsx:80`, `sidecar/email/index.ts:31, 38`). So the "closed" union is already
being bypassed where it was inconvenient — evidence that the closed shape is the wrong shape.
By contrast `registration-protocol.ts` (16 lines: `name` + `capabilities: string[]`) is genuinely
By contrast `registration-protocol.ts` (16 lines: `name` + `permissions: string[]`) is genuinely
generic. The registration handshake got this right; the command channel did not.
### 3. Ten WebSocket providers, and only three are tunnels
@@ -708,10 +709,10 @@ is item 2 of the email section arriving from a different direction.
### 5. Registry bugs that will bite during any migration
- **`unregisterSidecar` (`sidecar-registry.ts:80-85`) rejects the entire global pending-command map
when *any single* sidecar disconnects.** So restarting `officer-music` fails in-flight claude, pty and
when _any single_ sidecar disconnects.** So restarting `officer-music` fails in-flight claude, pty and
vault commands. This will look like random unrelated breakage the moment sidecars restart
independently — which is the entire goal.
- **`:88-90` treats `capabilities.includes('proxy')` as "is this claude"** — true only by accident of
- **`:88-90` treats `permissions.includes('proxy')` as "is this claude"** — true only by accident of
the naming confusion documented in `CLAUDE_SIDECAR_ISOLATION.md`.
### 6. What the database says (the clearest signal in the audit)
@@ -726,8 +727,8 @@ Table ownership tracks compliance exactly:
- **`queries/email-accounts.ts` — split**, with `api/chat/websocket.ts:11, 61-62` reaching across
domains into it.
**A useful rule falls out of this:** *if a table is read by exactly one sidecar and nothing else, that
sidecar is probably compliant. If the platform reads it, the platform probably owns logic it shouldn't.*
**A useful rule falls out of this:** _if a table is read by exactly one sidecar and nothing else, that
sidecar is probably compliant. If the platform reads it, the platform probably owns logic it shouldn't._
Cheaper to check than reading 3,000 lines.
---
@@ -739,7 +740,7 @@ mirroring the slskd findings at the top of this document.
Same rule, applied one layer out. The question here is not "what logic runs in `officer`" but **"does
the browser know things only the sidecar should know?"** — upstream URL shapes, wire formats, session-id
conventions, retry and reconnect policy, capability catalogues.
conventions, retry and reconnect policy, permission catalogues.
The slskd case at the top of this document is the template: **37 raw `/slskd/api/v0/…` calls against 10
`/slskd/_officer/…` calls**, meaning the browser is a second client of the upstream API rather than a
@@ -785,7 +786,7 @@ disconnected UI — a red "Disconnected" indicator (`ChatDetailPanel.tsx:38-52`)
(`InputArea.tsx:84`), model switching locked (`ModelSelector.tsx:67`).
**So `seq` + `resume-cursor` already exist end to end.** Pass 1 found the matching backend half at
`chat/websocket.ts:612-629` (`getChatEventsSince`). The protocol is not missing; the *writer* is simply
`chat/websocket.ts:612-629` (`getChatEventsSince`). The protocol is not missing; the _writer_ is simply
on the wrong side of the socket. That makes the durability stage of `CLAUDE_SIDECAR_ISOLATION.md`
substantially smaller than I estimated — a relocation, not a new mechanism.
@@ -807,16 +808,16 @@ check when the writer moves.
string, in the task runner. This one silently goes stale.
3. **The CLI invocation string is in the browser.** `apps/Terminal/index.tsx:32-33` —
`command="claude --dangerously-skip-permissions"`, `statePrefix="claude-code"`. The browser decides
how the agent binary is invoked, including its permission flag. *(The unsandboxed posture is
how the agent binary is invoked, including its permission flag. _(The unsandboxed posture is
deliberate per `platform/CLAUDE.md`; the objection is only to where the decision lives — the
browser is the furthest possible place from the sidecar that owns it.)*
4. **Capability metadata crosses to the client.** `Chat/types.ts:11-19` types `contextWindow`,
browser is the furthest possible place from the sidecar that owns it.)_
4. **Permission metadata crosses to the client.** `Chat/types.ts:11-19` types `contextWindow`,
`maxTokens` and `reasoning?`, and `ModelSelector.tsx:116` branches the UI on `reasoning`. The
browser doesn't compute these, so this is acceptable *if* they come from the sidecar — but Pass 1
browser doesn't compute these, so this is acceptable _if_ they come from the sidecar — but Pass 1
found them hardcoded in the platform at `api/chat/list-models.ts:5-9`, so today the numbers
originate two layers away from the thing they describe.
5. **Claude CLI session conventions are documented in the browser.**
`state/src/useClaudeSessions.ts:8-9` comments that the id *is* the transcript filename;
`state/src/useClaudeSessions.ts:8-9` comments that the id _is_ the transcript filename;
`SessionList.tsx:10-11` explains that clicking a session continues it "via --resume"; `:15` types
`harness?: 'claude' | 'opencode'`. And the magic string **`'general_chat_sessions'`** — Claude's
own default directory bucket — appears as a literal in `PwdSelector.tsx:7, 22, 62`,
@@ -851,7 +852,7 @@ check when the writer moves.
## email — routes are compliant, payloads and realtime are not
The mirror image of chat: **every one of the 20 API paths is Officer-shaped** — there is no
`/imap/uid/…` anywhere — but the request *bodies* carry IMAP configuration, the compose path builds
`/imap/uid/…` anywhere — but the request _bodies_ carry IMAP configuration, the compose path builds
MIME, and the realtime channel cannot recover from a restart at all.
There is no windowed panel app; the email UI is screen-level under
@@ -884,9 +885,10 @@ out of `IntegrationsSettings/index.tsx:11, 71-79`), `EmailScreen.tsx` (60), `typ
- `GoogleOAuthConfig.tsx:197-210` — `GET /integrations/google/config` returns `clientSecret` in
plaintext; held in `useState` (`:184`), shown at `:255-262`.
Neither is a mail credential *the sidecar owns*, and both are the owner's own secrets on the
Neither is a mail credential _the sidecar owns_, and both are the owner's own secrets on the
owner's own machine — but "GET returns the secret so the form can prefill" is the pattern worth
changing, since a write-only field would work identically.
5. **The session bearer token is passed in a URL.** `EmailList.tsx:110-112` builds
`new EventSource('/api/email/events?token=' + …)` from `localStorage`. Unavoidable for `EventSource`
(it can't set headers), but it puts the JWT into browser history and any proxy access log. Worth
@@ -895,7 +897,7 @@ out of `IntegrationsSettings/index.tsx:11, 71-79`), `EmailScreen.tsx` (60), `typ
`EmailList.tsx:109-125` opens the SSE stream, expects `{ type: 'new-mail' }`, invalidates three
query keys, and closes on unmount. There is **no `es.onerror`, no backoff, no reconnect, and no
`Last-Event-ID` handling.** And the server never sends an `id:` field — Pass 1's
`api/email/email.ts:101` emits only `data: {"type":"new-mail"}` — so even the browser's *native*
`api/email/email.ts:101` emits only `data: {"type":"new-mail"}` — so even the browser's _native_
`EventSource` retry cannot request replay. Any `new-mail` event emitted during a restart is lost
silently until the next event arrives or the user hits Sync manually (`:134-154`).
**Direct contrast with chat, in the same codebase: one channel has cursor-based replay, the other
@@ -925,18 +927,18 @@ out of `IntegrationsSettings/index.tsx:11, 71-79`), `EmailScreen.tsx` (60), `typ
- **No charset, quoted-printable, base64 or RFC-2047 decoding in the browser** — it receives decoded
`text`/`html`/`snippet`. Reading is compliant; only composing leaks.
- **No Gmail label ids and no Gmail query syntax constructed client-side.** The search box passes `q=`
through untouched (`EmailList.tsx:83-85`); `:309`'s placeholder only *hints* at the syntax.
through untouched (`EmailList.tsx:83-85`); `:309`'s placeholder only _hints_ at the syntax.
- **Mail credentials are write-only.** The password is POSTed at `EmailAccounts.tsx:132` and never read
back — `GET /email/accounts` returns no credential field. OAuth tokens never reach the browser at
all: `:88-107` either redirects the page to `/api/integrations/google/authorize` or POSTs
`credentials: { userIntegrationId: true }`, a boolean. This is the right shape, and it is worth
noting that the *account* credential path is stricter than the *settings* ones in item 4.
noting that the _account_ credential path is stricter than the _settings_ ones in item 4.
- **No Message-Id handling** — and `Compose.tsx:382-384` documents the absence, noting `m.id` is a local
hash and that threading currently leans on `Re:` + participants.
## opencode — the most compliant frontend of the eight
Genuinely surprising given Pass 1 found ≈792 non-compliant *backend* lines. **The string `opencode`
Genuinely surprising given Pass 1 found ≈792 non-compliant _backend_ lines. **The string `opencode`
appears in exactly four frontend files, and only one of those is logic.** Everything the backend leaks
— the `ses_` prefix, the `opencode/<modelID>` id shape, `metadata.officer`, `auth.json`,
`models.json`, the version pin — stops at the server. Verified by exhaustive grep: **zero frontend hits
@@ -971,7 +973,7 @@ it just always sends `cwd`.
(`value.slice(0,3) + '...' + value.slice(-3)`), and the browser uses the result only as a
placeholder (`AIHarnessesSection.tsx:467`). A freshly typed key lives transiently in
`keyInputs` state (`:74`) and is **deleted after the PUT** (`:121-125`). Never in `localStorage`,
`sessionStorage`, or the query cache. Local-provider config returns the auth *type* only, never key
`sessionStorage`, or the query cache. Local-provider config returns the auth _type_ only, never key
material. **This is the pattern the email settings surface (Pass 2, email item 4) should copy.**
5. **No hardcoded model catalogue.** `state/src/useModels.ts:30-51` fetches everything from
`/chat/models`. The only hardcoded data is display-name maps — `ModelSelector.tsx:7-23` (14 pairs)
@@ -987,16 +989,16 @@ it just always sends `cwd`.
(routed at `App.tsx:38-40`), `useClaudeSessions.ts`, `useEmbeddableChat.ts`. All reachable.
**One thing the frontend displays that isn't real, and the cause is in the backend.**
`api/chat/list-models.ts:24` stubs *every* opencode-routed model with constant metadata —
`api/chat/list-models.ts:24` stubs _every_ opencode-routed model with constant metadata —
`contextWindow: 200000, maxTokens: 8192, reasoning: false, images: true`, with the comment "metadata is
left at neutral defaults for now". The browser faithfully renders these (`ModelSelector.tsx:116`
branches the thinking toggle on `reasoning`). So the capability numbers shown to the user for opencode
branches the thinking toggle on `reasoning`). So the permission numbers shown to the user for opencode
models are placeholders, and `reasoning: false` will suppress the thinking toggle for models that do
support it. A backend defect, surfaced by a compliant frontend.
## terminal / pty — the browser reconnects, and then loses the session anyway
The mirror of the backend result. Pass 1 called pty the least compliant *backend* surface; the frontend
The mirror of the backend result. Pass 1 called pty the least compliant _backend_ surface; the frontend
is mostly well-behaved, has real reconnect logic, and yet contains **one bug that defeats the entire
detach-not-kill design.**
@@ -1056,9 +1058,9 @@ between `pty-sidecar.mjs:37` and `connect.ts:22` (Pass 1). Four backoff policies
sidecar keeps a capped 50KB buffer (`pty-sidecar.mjs:36`, `BUFFER_MAX`) and re-emits it on re-init
(`:99-101`); the bridge forwards it as an ordinary `output` frame
(`api/terminal/websocket.ts:71-79`), and `Terminal.tsx:181` `term.write()`s it indistinguishably from
live output. No dedup, no historical marker. It works, passively. *(INFERRED: survival across a hard
live output. No dedup, no historical marker. It works, passively. _(INFERRED: survival across a hard
page reload depends on React cleanup not running during navigation teardown — standard behaviour, but
not verified against `pagehide` here.)*
not verified against `pagehide` here.)_
Session ids are **chosen by the browser** and persisted server-side through `useDashboardState` →
`GET/PATCH /dashboards` (React Query key `['DASHBOARD_STATE']`, `staleTime: Infinity`), so they survive
@@ -1072,9 +1074,7 @@ with an ephemeral `` `run-cmd-${Date.now()}` `` in local state — deliberate fo
1. **The browser composes shell commands by string concatenation, unescaped.**
```ts
// Terminal.tsx:187-190
const wrapped = onCommandDoneRef.current
? `${commandRef.current}; echo "${EXIT_MARKER}$?__"`
: commandRef.current;
const wrapped = onCommandDoneRef.current ? `${commandRef.current}; echo "${EXIT_MARKER}$?__"` : commandRef.current;
ws.send(JSON.stringify({ type: 'input', data: wrapped + '\r' }));
```
That assumes a POSIX shell (`;`, `$?`, `echo`) and does not escape `command`. Same pattern at
@@ -1104,7 +1104,7 @@ with an ephemeral `` `run-cmd-${Date.now()}` `` in local state — deliberate fo
- **The backend's `cwd` handler is unreachable.** Pass 1 flagged
`api/terminal/websocket.ts:129-138` for synthesizing `` `cd ${JSON.stringify(msg.path)}\r` ``.
Repo-wide grep finds **zero** frontend senders of `{type:'cwd'}` — the browser does its own `cd`
composition instead (item 1 above). So that branch is dead, and the capability it implements is
composition instead (item 1 above). So that branch is dead, and the permission it implements is
duplicated in the client.
- **`detached` is dead in the other direction.** `Terminal.tsx:225-226` handles a `'detached'` message
and writes `[Session taken over]`, but **no backend code ever emits it** — the only `detached` in
@@ -1266,16 +1266,16 @@ settled before any of that code is moved: who is actually meant to talk to the v
The single most useful thing in this pass. Ranked by frontend compliance:
| sidecar | backend verdict (Pass 1) | frontend verdict (Pass 2) |
|---|---|---|
| slskd | ✅ compliant, 70 lines | ✗ **worst** — 37 raw upstream calls vs 10 Officer routes |
| music | ✅ compliant, 88 lines | ✅ 12 routes, all Officer-owned |
| opencode | ✗ ≈792 lines | ✅ **best** — 4 mentions, 1 of them logic |
| claude | ✗ ≈2,300 lines | ✅ mostly clean; vocabulary leaks only |
| email | ✗ ≈2,875 lines, no proxy | ◐ routes clean, payloads and realtime not |
| pty | ✗ ~all of 169 lines | ◐ clean boundary, but shell composition + a session leak |
| vnc | ✗ blocked on a missing listener | ✅ 149 lines, one route, no internals |
| vault | ◐ partial | — no frontend exists |
| sidecar | backend verdict (Pass 1) | frontend verdict (Pass 2) |
| -------- | ------------------------------- | -------------------------------------------------------- |
| slskd | ✅ compliant, 70 lines | ✗ **worst** — 37 raw upstream calls vs 10 Officer routes |
| music | ✅ compliant, 88 lines | ✅ 12 routes, all Officer-owned |
| opencode | ✗ ≈792 lines | ✅ **best** — 4 mentions, 1 of them logic |
| claude | ✗ ≈2,300 lines | ✅ mostly clean; vocabulary leaks only |
| email | ✗ ≈2,875 lines, no proxy | ◐ routes clean, payloads and realtime not |
| pty | ✗ ~all of 169 lines | ◐ clean boundary, but shell composition + a session leak |
| vnc | ✗ blocked on a missing listener | ✅ 149 lines, one route, no internals |
| vault | ◐ partial | — no frontend exists |
**A thin proxy alone does not produce a clean frontend — it relocates the problem.** When the platform
translates (claude, opencode, email), the browser receives Officer-shaped data and stays clean at the
@@ -1285,7 +1285,7 @@ because **the sidecar exposes Officer-shaped routes** — the `/_officer/*` name
proxying the upstream one.
So the rule as stated ("main server is a thin proxy") is necessary but not sufficient. The complete
version is: *the sidecar owns the contract the browser consumes.* Thinning a router without adding
version is: _the sidecar owns the contract the browser consumes._ Thinning a router without adding
`/_officer/*` routes to the sidecar just moves domain logic from the platform into the browser, which is
strictly worse — it is further from the data and unversioned.
@@ -1294,13 +1294,13 @@ strictly worse — it is further from the data and unversioned.
Pass 1 found an architecture problem. Pass 2 mostly finds a **resilience** problem — and it is
per-socket rather than systemic:
| channel | reconnect | replay |
|---|---|---|
| chat WS | ✅ `min(5000, 300 × retry)` | ✅ `seq` + `resume-cursor` (best in repo) |
| terminal / cliamp control WS | ✅ 5-entry table + visibility trigger | ◐ passive 50 KB sidecar buffer; browser unaware |
| cliamp audio WS | ✗ none | — n/a (live capture) |
| desktop / VNC WS | ✗ **none at all** | — n/a |
| email SSE | ✗ none, and no `id:` field server-side | ✗ so native `EventSource` retry can't replay either |
| channel | reconnect | replay |
| ---------------------------- | -------------------------------------- | --------------------------------------------------- |
| chat WS | ✅ `min(5000, 300 × retry)` | ✅ `seq` + `resume-cursor` (best in repo) |
| terminal / cliamp control WS | ✅ 5-entry table + visibility trigger | ◐ passive 50 KB sidecar buffer; browser unaware |
| cliamp audio WS | ✗ none | — n/a (live capture) |
| desktop / VNC WS | ✗ **none at all** | — n/a |
| email SSE | ✗ none, and no `id:` field server-side | ✗ so native `EventSource` retry can't replay either |
Two channels have thought-through recovery; three have none. The chat implementation is the template
and it already works — see the isolation document, where this finding shortened the plan.
@@ -1368,9 +1368,9 @@ Recorded because they surfaced during the audit, not because they're in scope:
2. **Terminals don't re-fit after a resize.** `fitAddon.fit()` runs once per `connect()`
(`Terminal.tsx:158`); there is no `ResizeObserver` or window listener, so dragging a splitter leaves
the pty on stale dimensions until the next reconnect.
3. **opencode model capabilities shown to the user are placeholder constants.**
3. **opencode model permissions shown to the user are placeholder constants.**
`api/chat/list-models.ts:24` stubs every opencode model at `contextWindow: 200000, maxTokens: 8192,
reasoning: false`, and `ModelSelector.tsx:116` hides the thinking toggle based on that `false`.
reasoning: false`, and `ModelSelector.tsx:116` hides the thinking toggle based on that `false`.
---