From 0a2e258b796689a15584799820cd6806612b7878 Mon Sep 17 00:00:00 2001 From: Varun Singh <382354+vr000m@users.noreply.github.com> Date: Fri, 26 Jun 2026 21:07:49 -0700 Subject: [PATCH 01/24] docs: add reviewed dev plan for UDS server-side trust-boundary hardening Plan covers two server-side measures for the same-host UDS trust boundary: (1) enforce a 0700 owner-owned socket parent dir before bind, refusing to start otherwise; (2) peer-credential auth (macOS getpeereid via ctypes, Linux SO_PEERCRED) rejecting any peer whose uid != server uid on UDS. Reviewed via five-lens /review-plan; all findings folded in (cross-uid test redesign with test-only dir bypass, _cmd_serve error surface, install-script 0700 co-requisite, fail-closed sock-None guard, verified-vs-assumed split). Marker: reviewed 2026-06-26. --- ...26-feature-uds-trust-boundary-hardening.md | 454 ++++++++++++++++++ docs/dev_plans/README.md | 1 + 2 files changed, 455 insertions(+) create mode 100644 docs/dev_plans/20260626-feature-uds-trust-boundary-hardening.md diff --git a/docs/dev_plans/20260626-feature-uds-trust-boundary-hardening.md b/docs/dev_plans/20260626-feature-uds-trust-boundary-hardening.md new file mode 100644 index 0000000..13b214a --- /dev/null +++ b/docs/dev_plans/20260626-feature-uds-trust-boundary-hardening.md @@ -0,0 +1,454 @@ +# Feature: UDS server-side trust-boundary hardening + +**Status:** Reviewed (2026-06-26) — ready to implement +**Component:** Server Transport +**Assignee:** unassigned +**Priority:** High (security; trust boundary) +**Branch:** `feature/uds-trust-boundary-hardening` +**Created:** 2026-06-26 +**Objective:** Close the two remaining same-host UDS trust-boundary gaps in +`stt_server` entirely on the server side — (1) make the socket un-plantable by +enforcing an owner-only, owner-owned parent directory before bind, and (2) +authenticate the connecting client by kernel-supplied peer credentials so a +foreign local uid cannot connect even if it reaches the socket. + +--- + +## Context + +A sibling client-side change added an inode/permission check that defends the +*client* against a spoofed server. That check is inherently TOCTOU: the client +can narrow the plant/swap window by re-checking on every connect but cannot +eliminate it. The server is better positioned to close the vector for both +sides. Three server-side measures were proposed; one is already shipped: + +| # | Measure | State | +|---|---|---| +| 1 | Un-plantable socket: `0700`, owner-owned parent dir, refuse to start otherwise | **Missing** — this plan | +| 2 | `umask`-at-bind so the socket inode is `0600` from birth | **Already done** — `server.py:152` wraps `ws_unix_serve` in `os.umask(0o077)` + `finally` restore, then `chmod 0o600` at `server.py:162-166`. Out of scope. | +| 3 | Peer-credential auth: reject any peer whose `uid != server uid` | **Missing** — this plan | + +Why #1 is highest-leverage: the `0600` mode on the socket *inode* stops a +foreign uid from `connect()`-ing, but does **not** stop the plant/swap attack. +An attacker with write on the parent directory `unlink()`s our inode and +`bind()`s their own; the client then connects to the attacker's socket. The +only defense is denying others write on the parent — i.e. a `0700`, +owner-owned parent dir, verified at startup. On stock macOS `~/Library/Caches` +is `0700`, but the `pipecat-stt/` subdir we `mkdir` inherits the process umask +(commonly `0755`), and a custom `STT_WS_SOCKET` pointing at a world-writable +location defeats everything. Enforcing at startup makes it robust regardless of +where the path points. + +Why #3 is **defense-in-depth, not the primary boundary** for the same-host case: +once #1 enforces a `0700` owner-owned parent dir, a foreign uid already cannot +even *traverse* to the socket (no `+x` on the dir → `connect()` fails `EACCES` +at path resolution, before the socket mode or any handshake). So #1 alone closes +the same-host foreign-uid vector at the filesystem layer. #3 still earns its +place because the uid it checks is **kernel-supplied and unforgeable**, so it +holds even when the filesystem perms are looser than intended (a misconfigured +dir, a relaxed socket mode, a future abstract/Linux socket, or simply not +trusting file modes as the sole boundary). It is the kernel-authoritative backstop +behind the filesystem boundary — not a strictly-stronger replacement for it. +Corollary on the token: for UDS the bearer token is redundant (the file-perm +boundary + peer-cred both dominate it); we keep it for TCP/remote, which has +neither a file-permission boundary nor peer creds. **This reframing matters for +testing #3 in isolation — see Phase 4.** + +### Scope: server-side only, no client changes + +Both measures are **server-side only and require no client code change**: + +- #1 only touches the directory the server binds into; the client never sees it. +- #3 reads the peer uid from the **kernel** (`SO_PEERCRED` / `getpeereid`), not + from anything the client sends. A legitimate same-uid client passes the check + having done nothing — no new handshake field, no token, no library bump. + +**Precondition (deployment fact, not code):** peer-cred auth assumes the client +and server always run as the **same uid**. The Koda cross-repo contract runs +both as per-user LaunchAgents (same uid), so this holds today. If a future +deployment runs the server as a daemon user and the client as the logged-in +user, #3 would correctly reject it and that deployment would need coordination — +still not a client code change. This precondition is written into Requirements. + +--- + +## Requirements + +1. **R1 — Parent-dir enforcement (blocking, fatal).** Before bind, the server + MUST verify the socket's parent directory is owned by the running uid + (`st_uid == os.geteuid()`) and has mode `0700` (no group/other bits: + `st_mode & 0o077 == 0`). If the server creates the directory it MUST create + it `0700`. If the directory exists but fails either check, the server MUST + refuse to start with an actionable error naming the path and the offending + condition. It MUST NOT silently `chmod`/`chown` a pre-existing directory it + does not own. +2. **R2 — Peer-cred auth (UDS only).** On the UDS transport, the server MUST + reject any connection whose peer uid `!= os.geteuid()` before the WebSocket + handshake completes, returning a `403`. TCP connections are unaffected (no + peer-cred concept) and continue to use Origin + optional bearer-token checks. +3. **R3 — Cross-platform.** Peer-cred resolution MUST work on macOS (primary) + and Linux. macOS has no `socket.SO_PEERCRED`; use `getpeereid(2)` via + `ctypes` (or `LOCAL_PEERCRED`). Linux uses `socket.SO_PEERCRED`. A platform + where neither is available MUST fail closed (reject) with a logged warning, + not silently allow. +4. **R4 — Same-uid precondition.** The same-uid deployment assumption is + documented in code comments and the security docs; behavior under uid + mismatch is "reject," not "warn-and-allow." +5. **R5 — Bearer token unchanged for TCP.** #3 does not remove or weaken the + existing bearer-token path; the token remains the TCP trust mechanism. The + UDS-token-is-now-redundant observation is documented but the token plumbing + is NOT ripped out in this change (keeps the diff minimal and reversible). +6. **R6 — Tests.** Cross-platform tests including the macOS `ctypes getpeereid` + path; same-uid connections succeed, the directory-mode failure refuses to + start, and the peer-cred resolver is unit-tested in isolation. + +--- + +## Implementation Checklist + +### Phase 1 — Parent-directory enforcement + +**Impl files:** `stt_server/server.py`, `stt_server/__main__.py`, `scripts/install_stt_agent.sh` +**Test files:** `tests/test_stt_server.py` +**Test command:** `uv run pytest tests/test_stt_server.py -k "parent_dir or socket_dir or 0700 or owner" -q` + +- [ ] Add a private helper (e.g. `_enforce_socket_dir_secure(path: Path)`) that: + creates the parent `0700` when absent (`mkdir(mode=0o700)`, then re-`stat` and + verify — `mkdir` mode is umask-masked, so verify rather than trust); on an + existing dir, `stat` and require `st_uid == os.geteuid()` and + `st_mode & 0o077 == 0`; raise a clear exception otherwise. +- [ ] Call it in `start()` immediately before the `os.umask(0o077)` block + (replacing the bare `socket_path.parent.mkdir(parents=True, exist_ok=True)` + at `server.py:148-149`). Parents above the immediate dir: create with + `parents=True` but only assert mode/ownership on the immediate parent of the + socket (the bind dir) — document this boundary. +- [ ] **Failure surface — wrap the serve path, not the status probe.** Raise a + `ValueError`/dedicated exception from the helper. The serve entrypoint + `_cmd_serve` (`__main__.py:210-224`) runs `asyncio.run(serve(...))` with **no** + try/except today, so the exception would propagate as a bare traceback (the + cited `__main__.py:298-300` handler is in `_cmd_status`, the probe — it does + NOT cover the serve path). Add `try/except (ValueError, OSError) as exc: + print(f"stt_server: {exc}", file=sys.stderr); raise SystemExit(1)` around the + serve call. This also fixes the latent unguarded `ServerConfig.__post_init__` + `ValueError` on the serve path. Confirm no stack-trace-only failure. +- [ ] **Co-requisite: install script must create the dir `0700` (lands with this + phase).** `scripts/install_stt_agent.sh:100` does `mkdir -p "$(dirname + "$SOCKET_PATH")"` at the install shell umask (commonly `0755`); after this phase + the server would *refuse to start* against that existing `0755` dir. Change to + `mkdir -m 700` (or follow with `chmod 700`), add an upgrade note for + pre-existing `0755` dirs, and cross-check the Koda cross-repo contract socket + path. Without this, fresh installs and upgrades both break at the phase commit. + +### Phase 2 — Peer-credential resolver (cross-platform, isolated + unit-tested) + +**Impl files:** `stt_server/_peercred.py` (new), `stt_server/server.py` +**Test files:** `tests/test_peercred.py` (new) +**Test command:** `uv run pytest tests/test_peercred.py -q` + +- [ ] New module `stt_server/_peercred.py` exposing + `peer_uid(sock: socket.socket) -> int | None`: + - Linux: `sock.getsockopt(SOL_SOCKET, SO_PEERCRED, struct.calcsize("3i"))`, + unpack `(pid, uid, gid)`, return uid. + - macOS: `getpeereid(2)` via `ctypes` — `libc.getpeereid(fd, byref(uid_t), + byref(gid_t))`, `uid_t`/`gid_t` are `c_uint32`; return uid on success. + - Unknown platform / call failure: return `None` (caller fails closed). +- [ ] Keep this module import-light and side-effect-free so it is unit-testable + without binding a server (mirror the existing single `sys.platform == "darwin"` + precedent at `server.py:75-77`; no new abstraction framework). + +### Phase 3 — Wire peer-cred into the handshake (UDS only) + +**Impl files:** `stt_server/server.py` +**Test files:** `tests/test_stt_server.py` +**Test command:** `uv run pytest tests/test_stt_server.py -k "peercred or peer_uid or uds_auth" -q` + +- [ ] In `_process_request` (`server.py:261`), gate on UDS only + (`self._config.socket_path is not None`). Obtain the raw socket via + `connection.transport.get_extra_info("socket")`. **Verified:** + `connection.transport` is set before `_process_request` runs (confirmed in the + websockets 16 source — `connection_made` sets `self.transport` before + `conn_handler` awaits `handshake(process_request, …)`). **Assumed (validate in + this phase):** that `get_extra_info("socket")` returns a non-`None` AF_UNIX + socket at handshake time — this is standard asyncio behavior but is NOT + demonstrated by existing code (the `server.py:866-882` reference is + `_pending_write_bytes`, a *post-handshake* call site, so it is not precedent + for the handshake-time return). Assert `sock is not None and sock.family == + AF_UNIX` in the implementation. +- [ ] **Fail-closed guard (do this before calling the resolver):** if the raw + socket is `None`, return `connection.respond(403, "peer not permitted\n")` and + warn — do NOT call `peer_uid(None)` (it would raise `AttributeError` on + `.getsockopt`/`.fileno`, an uncaught exception, not a guaranteed reject). +- [ ] Call `peer_uid(sock)`; if it returns `None` or `!= os.geteuid()`, return + `connection.respond(403, "peer not permitted\n")`. Order it before/independent + of the bearer-token branch so UDS rejects foreign uids regardless of token. +- [ ] Log a single warning on the fail-closed `None` path (resolver `None` *or* + missing socket) so an unsupported platform / unexpected transport is loud. + +### Phase 4 — Local end-to-end smoke (multi-connection + cross-uid) + +**Impl files:** `scripts/smoke_peercred.py` (new), `justfile` +**Test files:** `tests/test_stt_server.py` (multi-connection same-uid case, CI-safe) +**Test command:** `uv run pytest tests/test_stt_server.py -k "multi_connection or concurrent_uds" -q` +**Validation cmd:** `just smoke-peercred` (local-only; skips/aborts cleanly when not privileged) + +Reuse the established `scripts/smoke_test_parakeet.py` pattern (real server on a +temp UDS, driven through `stt_server.client.TranscriptionClient`). The script +exercises two things a single-uid CI run cannot. + +**Why a test-only dir bypass is required.** To reach `_process_request` (where +peer-cred runs), a foreign uid must defeat **both** filesystem layers: traverse +the parent dir (needs `+x`) **and** open the socket (needs the socket mode). R1 +enforces the parent dir at `0700`, which blocks traversal — so relaxing only the +socket mode to `0o666` is **not enough**; the foreign uid still fails `EACCES` at +the directory before peer-cred is consulted. Since R1's `_enforce_socket_dir_secure` +*refuses to start* on any dir with group/other bits, the smoke must bind into a +deliberately-traversable dir (e.g. `0711`) with dir-enforcement bypassed for that +one path. Add a **narrow, explicit test-only escape hatch** — an internal +parameter/flag (e.g. `ServerConfig(_skip_socket_dir_enforcement=True)`, clearly +named and undocumented in the public CLI) that the smoke sets. This bypasses #1 +*for the harness only* so #3 can be observed in isolation; production paths never +set it. + +- [ ] **Cross-uid rejection (local-only, the real test).** Build + `TranscriptionServer`/`ServerConfig` **directly** (the public `serve()` does + not expose `unix_socket_mode` — `server.py:900` — so the smoke cannot use it) + with `unix_socket_mode=0o666`, the test-only dir-enforcement bypass set, and a + `0711` temp parent dir. Connect the example client under a second uid + (`sudo -u ` / a CI-absent dev user) and assert peer-cred (#3) rejects. + Assert the reject as the existing 401 test does: catch + `websockets.exceptions.InvalidStatus` and check `status_code == 403` and the + `"peer not permitted\n"` body (it is a **pre-handshake HTTP response, not a + protocol JSON envelope** — `docs/protocol.md` documents no reject envelope). +- [ ] **Same-uid multi-connection (also CI-safe).** Open N concurrent + `TranscriptionClient` sessions as the owning uid, assert all complete the + handshake and stream — a regression guard that peer-cred did not break the + normal path under concurrency. Add one assertion that the resolved peer uid + equals `os.geteuid()` via the **real** resolver (not a stub), so a silently- + `None` transport is caught rather than masked. Mirror this as a pytest case. +- [ ] Gate the cross-uid path on availability of a second uid / `sudo` and + `sys.platform`; print a clear "skipped: needs a second local uid" rather than + failing when run unprivileged. `just smoke-peercred` wraps invocation. +- [ ] For the **accept** path, assert `server.hello` fields against the + `server.py:287-307` source of truth (protocol.md lists event *names*, not the + field schema). If protocol.md is to be the field oracle, Phase 5 must add the + `server.hello` field table to it first. + +### Phase 5 — Docs + plan/README sync + +**Impl files:** `docs/` security notes, `docs/protocol.md` (trust-model note), +`docs/dev_plans/README.md`, this plan +**Test files:** n/a +**Test command:** `uv run ruff check && uv run ruff format --check` + +- [ ] Document the same-host UDS trust model: parent-dir `0700` is the primary + filesystem boundary; peer-cred is the kernel-authoritative defense-in-depth + backstop; bearer token retained for TCP only. +- [ ] Note macOS `getpeereid`-via-`ctypes` wrinkle for future maintainers. +- [ ] If the accept-path test is to assert against `docs/protocol.md` rather than + `server.py`, add the `server.hello` field table (`protocol_version`, + `capabilities`, `audio`, `backend`) to `docs/protocol.md` so it becomes a real + field oracle. Otherwise document that protocol.md pins event presence, not + field shape, and the reject is a pre-handshake HTTP `403` (no JSON envelope). +- [ ] Update `docs/dev_plans/README.md` row status on completion. + +--- + +## Technical Specifications + +### Files to modify / create + +| File | Change | +|---|---| +| `stt_server/server.py:148-149` | Replace bare `mkdir` with `_enforce_socket_dir_secure()`; add the helper. | +| `stt_server/server.py` `ServerConfig` (`:88-108`) | Add narrow test-only `_skip_socket_dir_enforcement` field (default `False`) for the Phase 4 smoke. | +| `stt_server/server.py:261-275` | Add UDS-only peer-cred gate in `_process_request` (incl. `sock is None → 403`). | +| `stt_server/_peercred.py` (new) | Cross-platform `peer_uid(sock)` resolver. | +| `stt_server/__main__.py` `_cmd_serve` (`:210-224`) | Wrap `asyncio.run(serve(...))` in `try/except (ValueError, OSError)` → `stt_server: ` + `SystemExit(1)`. NOT the `_cmd_status` handler at `:298-300`. | +| `scripts/install_stt_agent.sh:100` | `mkdir -m 700` the socket dir; upgrade note for existing `0755` dirs. | +| `tests/test_peercred.py` (new) | Unit tests for the resolver incl. macOS ctypes path + forced `sys.platform` branch selection. | +| `tests/test_stt_server.py:516+` | Dir-enforcement (incl. foreign-owner branch), `sock is None`, and UDS peer-cred integration tests. | +| `scripts/smoke_peercred.py` (new), `justfile` | Local cross-uid + multi-connection smoke; `just smoke-peercred` recipe. | +| `docs/…` security notes, `docs/protocol.md`, `docs/dev_plans/README.md` | Trust-model docs + (optional) hello field table + status row. | + +### Interfaces / seams + +**Verified in-repo / library source (grounded):** +- **`_process_request(self, connection, request)` contract** (`server.py:261`) — + websockets 16: return `connection.respond(status, body)` to reject, `None` to + allow; rejection triggers `transport.abort()`. Confirmed against the installed + `websockets/asyncio/server.py`. +- **`connection.transport` is set before `_process_request` runs** — confirmed in + the websockets 16 source: `connection_made` sets `self.transport` before + `conn_handler` awaits `handshake(process_request, …)`. +- **Startup-error precedent:** `ServerConfig.__post_init__` raises `ValueError` + (`server.py:110-114`). NOTE: the `__main__.py:298-300` OSError→`SystemExit` + handler is in `_cmd_status`, the probe — it does **not** wrap the serve path; + `_cmd_serve` (`:210-224`) must get its own try/except (see Phase 1). + +**External / OS facts assumed (NOT verified in-repo — no prior usage; validate +in Phase 2/3 via the `socketpair()` unit test before wiring in):** +- **`get_extra_info("socket")` returns a usable non-`None` AF_UNIX socket at + handshake time** — standard asyncio, but the codebase has no precedent (the + `server.py:866-882` reference is a *post-handshake* call site, not evidence for + the handshake stage). Assert non-`None` + `AF_UNIX` in code; guard `None → 403`. +- **`getpeereid(2)` semantics:** `int getpeereid(int fd, uid_t *euid, gid_t + *egid)`; returns 0 on success; yields the connecting peer's effective uid, + captured at `connect()` time. `uid_t`/`gid_t` = `c_uint32` on Darwin (a wrong + width could fail *open* by comparing equal) — the `socketpair()` test returning + `os.geteuid()` is the gate against a width/signature mistake. +- **`SO_PEERCRED` (Linux):** `struct ucred { pid_t pid; uid_t uid; gid_t gid; }`, + unpack `"3i"`. Untestable on the macOS primary platform — must run on a Linux + CI runner, or be marked "Linux-CI-only, unverified on dev host." + +### Dependency facts + +- `websockets>=13.0` (pyproject.toml:27); installed 16.0. `process_request` + async/sync both accepted. +- `requires-python = ">=3.12"` (pyproject.toml:6). +- `pytest>=8.0.0`, `pytest-asyncio>=0.24.0`, `asyncio_mode = "auto"` + (pyproject.toml:46-47,70) — `async def test_*` needs no marker. +- No existing `conftest.py`; no `platform`/`compat` abstraction module — UDS + tests are inline `async def` using `tempfile.TemporaryDirectory` + (`tests/test_stt_server.py:516-562`). Follow that style. + +### Integration Seams + +| Seam | Contract | Verified by | +|---|---|---| +| `start()` → dir enforcement | Refuse to bind unless parent is `0700` + owner-owned | Phase 1 tests | +| `_process_request` → `peer_uid()` | UDS only; `None` or uid mismatch → `403` before handshake | Phase 3 tests | +| `peer_uid()` → OS | macOS `getpeereid` / Linux `SO_PEERCRED`; unknown → `None` (fail closed) | Phase 2 unit tests | +| TCP path | Unchanged: Origin + bearer token only | existing tests stay green | + +## Architecture & Call Flow + +Single component changes (the server's listener), but the trust decision spans +client → kernel → server, so the accept-time sequence is worth pinning: + +```mermaid +sequenceDiagram + participant C as Client (same uid) + participant K as Kernel + participant S as stt_server (_process_request) + Note over S: start(): enforce parent dir 0700 + owner — else refuse to start + C->>S: connect() over UDS, then WS upgrade + S->>K: get_extra_info("socket") → getpeereid/SO_PEERCRED + K-->>S: peer uid + alt uid == server uid + S-->>C: None (allow) → handshake completes + else uid != server uid OR resolver None + S-->>C: respond(403) → transport.abort() + end +``` + +| Step | Trigger | Enters context | Cleared/persisted | Turn boundary | +|---|---|---|---|---| +| Bind | `start()` | socket_path, parent dir stat | dir verified once at start | startup | +| Accept | client connect | peer uid from kernel | per-connection, not stored | per connection | +| Reject | uid mismatch | 403 response | connection aborted | per connection | + +--- + +## Testing Notes + +- **Resolver unit tests** (`tests/test_peercred.py`): create a connected + `socketpair()` (AF_UNIX), assert `peer_uid()` returns `os.geteuid()` on the + host platform — this is the acceptance gate for the ctypes binding (width + + signature) and for `getpeereid` semantics, both of which are unverified + in-repo. Cover the macOS ctypes path when `sys.platform == "darwin"`; on Linux + assert `SO_PEERCRED`. Additionally **force each branch's selection by + monkeypatching `sys.platform`** (even if the off-host syscall is mocked), so + dispatch logic is covered regardless of CI host. Test the fail-closed `None` + branch (unknown platform / call failure) by monkeypatching. +- **Dir enforcement** (`tests/test_stt_server.py`): (a) server creates parent + `0700` when absent and starts; (b) pre-existing `0755` parent → start refuses + with actionable error (assert the `stt_server: ` + exit-1 surface, not a + bare traceback); (c) verify the created dir's mode after start; (d) + **foreign-owner branch:** monkeypatch `os.stat` to return a foreign `st_uid` + and assert the helper raises **without** calling `os.chmod`/`os.chown` (the + "must not chmod/chown what it does not own" invariant — hard to do with a real + foreign-owned dir on single-uid CI). +- **UDS peer-cred — two layers:** + - *CI (seam):* monkeypatch `peer_uid` → `euid+1`, expect `403`; force + `get_extra_info("socket") → None` and assert `403` (not an exception/allow); + plus a same-uid multi-connection regression case that resolves the **real** + peer uid == `os.geteuid()` (so a silently-`None` transport is caught). + - *Local (real, Phase 4):* `scripts/smoke_peercred.py` / `just smoke-peercred` + runs the example client under a second uid against a server built directly + with `unix_socket_mode=0o666`, a `0711` parent dir, and the test-only + dir-enforcement bypass — defeating **both** filesystem layers so peer-cred is + what rejects. Asserts a real `403` via `InvalidStatus.status_code` + the + `"peer not permitted\n"` body (a pre-handshake HTTP response, **not** a + protocol JSON envelope). Skips cleanly when no second uid / `sudo`. +- Full suite: `uv run pytest -q`. `uv run ruff check && uv run ruff format` before push. + +## Acceptance Criteria + +- [ ] Server refuses to start when the socket parent dir is not `0700` + + owner-owned, with an actionable `stt_server: ` error + `SystemExit(1)` + (not a bare traceback); creates it `0700` when absent. `install_stt_agent.sh` + creates the dir `0700` so fresh installs/upgrades do not break. +- [ ] UDS connections from a foreign uid are rejected with `403` before the + handshake; same-uid connections succeed unchanged. Every fail-closed path + (resolver `None`, missing transport socket, unknown platform) rejects. +- [ ] Local `just smoke-peercred` demonstrates a real cross-uid `403` (asserted + via `InvalidStatus.status_code` + `"peer not permitted\n"` body) against a + server whose `0o666` socket mode **and** `0711` parent dir both permit the peer, + with dir-enforcement bypassed for the harness — so peer-cred is provably what + rejects. N concurrent same-uid sessions all succeed. +- [ ] `peer_uid()` resolves on macOS (ctypes `getpeereid`) and Linux + (`SO_PEERCRED`); branch selection is covered on any host; unknown platforms + fail closed. +- [ ] No client-side change required; existing client connects unmodified. +- [ ] TCP path and bearer-token behavior unchanged. +- [ ] The test-only dir-enforcement bypass is never settable from the public CLI + / `serve()` entrypoint. +- [ ] `uv run pytest -q` green; `ruff check` + `ruff format` clean. +- [ ] Docs describe the same-host trust model and the same-uid precondition. + +## Review Focus + +- **Fail-closed posture:** confirm every error path (resolver `None`, ctypes + failure, unknown platform, missing transport socket) results in *reject*, never + *allow*. +- **TOCTOU on the dir check:** the stat-then-bind is itself a small window; + confirm `0700`-owner-owned makes the window unexploitable (no other uid can + win the race without write on the parent). +- **websockets 16 contract:** verify `connection.transport` is reliably set when + `_process_request` runs and `get_extra_info("socket")` returns the AF_UNIX + socket (not `None`). +- **UDS-vs-TCP gating:** ensure peer-cred runs only for UDS and TCP is untouched. +- **Same-uid precondition (R4):** confirm the deployment assumption is documented + and that uid mismatch rejects rather than warn-allows. +- **Test-only bypass containment:** the `_skip_socket_dir_enforcement` escape + hatch (Phase 4) must be unreachable from the public CLI / `serve()` — it exists + solely so the smoke can observe peer-cred in isolation. Verify no production + path can set it. +- **Defense-in-depth framing:** confirm the docs present `0700` dir as the primary + same-host boundary and peer-cred as the kernel-authoritative backstop — not as a + strictly-stronger replacement (since #1 alone already blocks the same-host + foreign-uid vector at the filesystem layer). + + + + + +## Progress + +- [ ] Phase 1: Parent-directory enforcement +- [ ] Phase 2: Peer-credential resolver +- [ ] Phase 3: Wire peer-cred into the handshake +- [ ] Phase 4: Local end-to-end smoke (multi-connection + cross-uid) +- [ ] Phase 5: Docs + plan/README sync + +## Findings + +- (append findings here as work proceeds) + +## Issues & Solutions + +_(to be filled during implementation)_ + +## Final Results + +[Fill this section when the work is complete] diff --git a/docs/dev_plans/README.md b/docs/dev_plans/README.md index dabbb61..4971740 100644 --- a/docs/dev_plans/README.md +++ b/docs/dev_plans/README.md @@ -9,6 +9,7 @@ update the row in the same change that updates the plan's `**Status**` line. | [Nemotron 3.5 ASR backend (0.3.0)](20260605-nemotron-asr-backend.md) | ASR Backends | ✅ Shipped — PR #7 merged 2026-06-06; packaged as `nemotron` extra in 0.3.2 | | [justfile operator layer for STT LaunchAgents](20260607-feature-stt-agents-justfile.md) | Install & Packaging | ✅ Complete | | [STT vocabulary/prompt biasing on the wire protocol](20260607-feature-stt-prompt-biasing.md) | Wire Protocol / ASR Backends | ⬜ Not started — analysis/handoff only | +| [UDS server-side trust-boundary hardening](20260626-feature-uds-trust-boundary-hardening.md) | Server Transport | ⬜ Reviewed (2026-06-26) — ready to implement | ## Conventions From 505961087488791a3cc6b6b1657d308d95772298 Mon Sep 17 00:00:00 2001 From: Varun Singh <382354+vr000m@users.noreply.github.com> Date: Fri, 26 Jun 2026 21:23:26 -0700 Subject: [PATCH 02/24] docs: harden UDS trust-boundary plan per Codex review MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Independent Codex review of the plan surfaced two HIGH issues the five-lens pass missed, plus four defensive improvements. All folded in: - H1: enforce owner-owned, non-group/other-writable ancestor chain from the socket bind dir through the trusted root (sticky dirs excepted), not just the immediate parent — closes an ancestor-swap TOCTOU. - H2: drop the _skip_socket_dir_enforcement ServerConfig field (reachable via direct construction); use a test-only subclass/monkeypatch seam instead. - M1: wrap peer_uid() in try/except -> 403 (not just None); add raising test. - M2: pin websockets>=16,<17 (handshake API contract); lockfile row added. - L1: ctypes argtypes/restype + use_errno=True on libc.getpeereid. - L2: type resolver against a minimal structural Protocol, not socket.socket. Re-hashed review marker to bless the revised content. --- ...26-feature-uds-trust-boundary-hardening.md | 227 ++++++++++-------- 1 file changed, 128 insertions(+), 99 deletions(-) diff --git a/docs/dev_plans/20260626-feature-uds-trust-boundary-hardening.md b/docs/dev_plans/20260626-feature-uds-trust-boundary-hardening.md index 13b214a..55c5a3d 100644 --- a/docs/dev_plans/20260626-feature-uds-trust-boundary-hardening.md +++ b/docs/dev_plans/20260626-feature-uds-trust-boundary-hardening.md @@ -8,7 +8,7 @@ **Created:** 2026-06-26 **Objective:** Close the two remaining same-host UDS trust-boundary gaps in `stt_server` entirely on the server side — (1) make the socket un-plantable by -enforcing an owner-only, owner-owned parent directory before bind, and (2) +enforcing an owner-only, owner-owned socket directory chain before bind, and (2) authenticate the connecting client by kernel-supplied peer credentials so a foreign local uid cannot connect even if it reaches the socket. @@ -24,7 +24,7 @@ sides. Three server-side measures were proposed; one is already shipped: | # | Measure | State | |---|---|---| -| 1 | Un-plantable socket: `0700`, owner-owned parent dir, refuse to start otherwise | **Missing** — this plan | +| 1 | Un-plantable socket: owner-owned, non-writable ancestor chain through the trusted root, refuse to start otherwise | **Missing** — this plan | | 2 | `umask`-at-bind so the socket inode is `0600` from birth | **Already done** — `server.py:152` wraps `ws_unix_serve` in `os.umask(0o077)` + `finally` restore, then `chmod 0o600` at `server.py:162-166`. Out of scope. | | 3 | Peer-credential auth: reject any peer whose `uid != server uid` | **Missing** — this plan | @@ -32,17 +32,19 @@ Why #1 is highest-leverage: the `0600` mode on the socket *inode* stops a foreign uid from `connect()`-ing, but does **not** stop the plant/swap attack. An attacker with write on the parent directory `unlink()`s our inode and `bind()`s their own; the client then connects to the attacker's socket. The -only defense is denying others write on the parent — i.e. a `0700`, -owner-owned parent dir, verified at startup. On stock macOS `~/Library/Caches` -is `0700`, but the `pipecat-stt/` subdir we `mkdir` inherits the process umask +only defense is denying others write on the path that can replace the socket — +i.e. every ancestor from the socket's bind directory through the trusted root is +owner-owned and not group/other-writable (sticky-bit directories excepted), +verified at startup. On stock macOS `~/Library/Caches` is `0700`, but the +`pipecat-stt/` subdir we `mkdir` inherits the process umask (commonly `0755`), and a custom `STT_WS_SOCKET` pointing at a world-writable location defeats everything. Enforcing at startup makes it robust regardless of where the path points. Why #3 is **defense-in-depth, not the primary boundary** for the same-host case: -once #1 enforces a `0700` owner-owned parent dir, a foreign uid already cannot -even *traverse* to the socket (no `+x` on the dir → `connect()` fails `EACCES` -at path resolution, before the socket mode or any handshake). So #1 alone closes +once #1 enforces the owner-only ancestor chain, a foreign uid already cannot even +*traverse* to the socket (no `+x` on the relevant directory → `connect()` fails +`EACCES` at path resolution, before the socket mode or any handshake). So #1 alone closes the same-host foreign-uid vector at the filesystem layer. #3 still earns its place because the uid it checks is **kernel-supplied and unforgeable**, so it holds even when the filesystem perms are looser than intended (a misconfigured @@ -74,14 +76,16 @@ still not a client code change. This precondition is written into Requirements. ## Requirements -1. **R1 — Parent-dir enforcement (blocking, fatal).** Before bind, the server - MUST verify the socket's parent directory is owned by the running uid - (`st_uid == os.geteuid()`) and has mode `0700` (no group/other bits: - `st_mode & 0o077 == 0`). If the server creates the directory it MUST create - it `0700`. If the directory exists but fails either check, the server MUST - refuse to start with an actionable error naming the path and the offending - condition. It MUST NOT silently `chmod`/`chown` a pre-existing directory it - does not own. +1. **R1 — Socket-dir ancestor enforcement (blocking, fatal).** Before bind, the + server MUST walk every directory component from the socket's bind directory + up to and including the trusted root directory. Every component in that walk + MUST be owned by the running uid (`st_uid == os.geteuid()`) and MUST NOT be + group-writable or other-writable (`st_mode & 0o022 == 0`), except sticky-bit + directories. If the server creates missing socket directories, it MUST create + them `0700` and then re-`stat` the full walk. If any component fails either + check, the server MUST refuse to start with an actionable error naming the + path and offending condition. It MUST NOT silently `chmod`/`chown` a + pre-existing directory it does not own. 2. **R2 — Peer-cred auth (UDS only).** On the UDS transport, the server MUST reject any connection whose peer uid `!= os.geteuid()` before the WebSocket handshake completes, returning a `403`. TCP connections are unaffected (no @@ -99,8 +103,9 @@ still not a client code change. This precondition is written into Requirements. UDS-token-is-now-redundant observation is documented but the token plumbing is NOT ripped out in this change (keeps the diff minimal and reversible). 6. **R6 — Tests.** Cross-platform tests including the macOS `ctypes getpeereid` - path; same-uid connections succeed, the directory-mode failure refuses to - start, and the peer-cred resolver is unit-tested in isolation. + path; same-uid connections succeed, directory-mode/owner failures anywhere in + the ancestor walk refuse to start, `peer_uid` exceptions return `403`, and the + peer-cred resolver is unit-tested in isolation. --- @@ -112,16 +117,18 @@ still not a client code change. This precondition is written into Requirements. **Test files:** `tests/test_stt_server.py` **Test command:** `uv run pytest tests/test_stt_server.py -k "parent_dir or socket_dir or 0700 or owner" -q` -- [ ] Add a private helper (e.g. `_enforce_socket_dir_secure(path: Path)`) that: - creates the parent `0700` when absent (`mkdir(mode=0o700)`, then re-`stat` and - verify — `mkdir` mode is umask-masked, so verify rather than trust); on an - existing dir, `stat` and require `st_uid == os.geteuid()` and - `st_mode & 0o077 == 0`; raise a clear exception otherwise. +- [ ] Add a private helper (e.g. `_enforce_socket_dir_secure(path: Path, + trusted_root: Path)`) that creates missing socket directories `0700` + (`mkdir(mode=0o700)`, then re-`stat` and verify — `mkdir` mode is umask-masked, + so verify rather than trust), then walks every component from the socket's bind + directory up to and including `trusted_root`. Each component must have + `st_uid == os.geteuid()` and no group/other write bits (`st_mode & 0o022 == 0`); + sticky-bit directories are allowed. Raise a clear exception naming the failing + component and condition otherwise. - [ ] Call it in `start()` immediately before the `os.umask(0o077)` block (replacing the bare `socket_path.parent.mkdir(parents=True, exist_ok=True)` - at `server.py:148-149`). Parents above the immediate dir: create with - `parents=True` but only assert mode/ownership on the immediate parent of the - socket (the bind dir) — document this boundary. + at `server.py:148-149`). Resolve and document the trusted root, then verify the + full ancestor walk rather than only the immediate parent of the socket. - [ ] **Failure surface — wrap the serve path, not the status probe.** Raise a `ValueError`/dedicated exception from the helper. The serve entrypoint `_cmd_serve` (`__main__.py:210-224`) runs `asyncio.run(serve(...))` with **no** @@ -146,11 +153,17 @@ still not a client code change. This precondition is written into Requirements. **Test command:** `uv run pytest tests/test_peercred.py -q` - [ ] New module `stt_server/_peercred.py` exposing - `peer_uid(sock: socket.socket) -> int | None`: + `peer_uid(sock: PeerCredSocket) -> int | None`, where `PeerCredSocket` is a + minimal structural `Protocol` containing only the members used by the resolver + (`family`, `fileno()`, `getsockopt()`), rather than `socket.socket` directly. + This decouples the resolver from the concrete socket class and keeps mock + testing simple. - Linux: `sock.getsockopt(SOL_SOCKET, SO_PEERCRED, struct.calcsize("3i"))`, unpack `(pid, uid, gid)`, return uid. - macOS: `getpeereid(2)` via `ctypes` — `libc.getpeereid(fd, byref(uid_t), - byref(gid_t))`, `uid_t`/`gid_t` are `c_uint32`; return uid on success. + byref(gid_t))`, `uid_t`/`gid_t` are `c_uint32`; set `argtypes` and `restype` + explicitly on the libc function and load libc with `use_errno=True` for + safety, portability, and correct errno propagation; return uid on success. - Unknown platform / call failure: return `None` (caller fails closed). - [ ] Keep this module import-light and side-effect-free so it is unit-testable without binding a server (mirror the existing single `sys.platform == "darwin"` @@ -178,11 +191,14 @@ still not a client code change. This precondition is written into Requirements. socket is `None`, return `connection.respond(403, "peer not permitted\n")` and warn — do NOT call `peer_uid(None)` (it would raise `AttributeError` on `.getsockopt`/`.fileno`, an uncaught exception, not a guaranteed reject). -- [ ] Call `peer_uid(sock)`; if it returns `None` or `!= os.geteuid()`, return - `connection.respond(403, "peer not permitted\n")`. Order it before/independent - of the bearer-token branch so UDS rejects foreign uids regardless of token. -- [ ] Log a single warning on the fail-closed `None` path (resolver `None` *or* - missing socket) so an unsupported platform / unexpected transport is loud. +- [ ] Wrap `peer_uid(sock)` in `try/except Exception`; if it raises, log the + exception and return `connection.respond(403, "peer not permitted\n")`. If it + returns `None` or `!= os.geteuid()`, return the same `403`. Order it + before/independent of the bearer-token branch so UDS rejects foreign uids + regardless of token. +- [ ] Log a single warning on each fail-closed path (resolver `None`, resolver + exception, or missing socket) so an unsupported platform / unexpected transport + is loud. ### Phase 4 — Local end-to-end smoke (multi-connection + cross-uid) @@ -195,25 +211,24 @@ Reuse the established `scripts/smoke_test_parakeet.py` pattern (real server on a temp UDS, driven through `stt_server.client.TranscriptionClient`). The script exercises two things a single-uid CI run cannot. -**Why a test-only dir bypass is required.** To reach `_process_request` (where +**Why a test-only dir seam is required.** To reach `_process_request` (where peer-cred runs), a foreign uid must defeat **both** filesystem layers: traverse the parent dir (needs `+x`) **and** open the socket (needs the socket mode). R1 -enforces the parent dir at `0700`, which blocks traversal — so relaxing only the -socket mode to `0o666` is **not enough**; the foreign uid still fails `EACCES` at -the directory before peer-cred is consulted. Since R1's `_enforce_socket_dir_secure` -*refuses to start* on any dir with group/other bits, the smoke must bind into a -deliberately-traversable dir (e.g. `0711`) with dir-enforcement bypassed for that -one path. Add a **narrow, explicit test-only escape hatch** — an internal -parameter/flag (e.g. `ServerConfig(_skip_socket_dir_enforcement=True)`, clearly -named and undocumented in the public CLI) that the smoke sets. This bypasses #1 -*for the harness only* so #3 can be observed in isolation; production paths never -set it. +enforces the ancestor chain, which blocks traversal — so relaxing only the socket +mode to `0o666` is **not enough**; the foreign uid still fails `EACCES` at path +resolution before peer-cred is consulted. Since R1's `_enforce_socket_dir_secure` +*refuses to start* on any group/other-writable component, the smoke must bind +into a deliberately-traversable dir (e.g. `0711`) while replacing the enforcement +helper through a test-only mechanism that is not reachable from `serve()` or +normal `TranscriptionServer` construction (for example, a local subclass or +monkeypatch of `_enforce_socket_dir_secure`). Do not add a `ServerConfig` field +or equivalent public/API-reachable bypass flag. - [ ] **Cross-uid rejection (local-only, the real test).** Build `TranscriptionServer`/`ServerConfig` **directly** (the public `serve()` does not expose `unix_socket_mode` — `server.py:900` — so the smoke cannot use it) - with `unix_socket_mode=0o666`, the test-only dir-enforcement bypass set, and a - `0711` temp parent dir. Connect the example client under a second uid + with `unix_socket_mode=0o666`, a test-only replacement for the dir-enforcement + helper, and a `0711` temp parent dir. Connect the example client under a second uid (`sudo -u ` / a CI-absent dev user) and assert peer-cred (#3) rejects. Assert the reject as the existing 401 test does: catch `websockets.exceptions.InvalidStatus` and check `status_code == 403` and the @@ -240,9 +255,9 @@ set it. **Test files:** n/a **Test command:** `uv run ruff check && uv run ruff format --check` -- [ ] Document the same-host UDS trust model: parent-dir `0700` is the primary - filesystem boundary; peer-cred is the kernel-authoritative defense-in-depth - backstop; bearer token retained for TCP only. +- [ ] Document the same-host UDS trust model: the owner-only ancestor chain is the + primary filesystem boundary; peer-cred is the kernel-authoritative + defense-in-depth backstop; bearer token retained for TCP only. - [ ] Note macOS `getpeereid`-via-`ctypes` wrinkle for future maintainers. - [ ] If the accept-path test is to assert against `docs/protocol.md` rather than `server.py`, add the `server.hello` field table (`protocol_version`, @@ -259,14 +274,14 @@ set it. | File | Change | |---|---| -| `stt_server/server.py:148-149` | Replace bare `mkdir` with `_enforce_socket_dir_secure()`; add the helper. | -| `stt_server/server.py` `ServerConfig` (`:88-108`) | Add narrow test-only `_skip_socket_dir_enforcement` field (default `False`) for the Phase 4 smoke. | -| `stt_server/server.py:261-275` | Add UDS-only peer-cred gate in `_process_request` (incl. `sock is None → 403`). | -| `stt_server/_peercred.py` (new) | Cross-platform `peer_uid(sock)` resolver. | +| `stt_server/server.py:148-149` | Replace bare `mkdir` with `_enforce_socket_dir_secure()`; add the full ancestor-walk helper. | +| `stt_server/server.py:261-275` | Add UDS-only peer-cred gate in `_process_request` (incl. `sock is None` and `peer_uid` exception → `403`). | +| `stt_server/_peercred.py` (new) | Cross-platform `peer_uid(sock)` resolver typed against a minimal structural `Protocol`. | | `stt_server/__main__.py` `_cmd_serve` (`:210-224`) | Wrap `asyncio.run(serve(...))` in `try/except (ValueError, OSError)` → `stt_server: ` + `SystemExit(1)`. NOT the `_cmd_status` handler at `:298-300`. | | `scripts/install_stt_agent.sh:100` | `mkdir -m 700` the socket dir; upgrade note for existing `0755` dirs. | +| `pyproject.toml`, `uv.lock` | Pin `websockets>=16,<17` to keep handshake API assumptions stable across major versions. | | `tests/test_peercred.py` (new) | Unit tests for the resolver incl. macOS ctypes path + forced `sys.platform` branch selection. | -| `tests/test_stt_server.py:516+` | Dir-enforcement (incl. foreign-owner branch), `sock is None`, and UDS peer-cred integration tests. | +| `tests/test_stt_server.py:516+` | Dir-enforcement (incl. foreign-owner ancestor branch), `sock is None`, `peer_uid` raises, and UDS peer-cred integration tests. | | `scripts/smoke_peercred.py` (new), `justfile` | Local cross-uid + multi-connection smoke; `just smoke-peercred` recipe. | | `docs/…` security notes, `docs/protocol.md`, `docs/dev_plans/README.md` | Trust-model docs + (optional) hello field table + status row. | @@ -294,16 +309,19 @@ in Phase 2/3 via the `socketpair()` unit test before wiring in):** - **`getpeereid(2)` semantics:** `int getpeereid(int fd, uid_t *euid, gid_t *egid)`; returns 0 on success; yields the connecting peer's effective uid, captured at `connect()` time. `uid_t`/`gid_t` = `c_uint32` on Darwin (a wrong - width could fail *open* by comparing equal) — the `socketpair()` test returning - `os.geteuid()` is the gate against a width/signature mistake. + width could fail *open* by comparing equal). Set `argtypes`/`restype` on the + `ctypes` function and load libc with `use_errno=True`; the `socketpair()` test + returning `os.geteuid()` is the gate against a width/signature mistake. - **`SO_PEERCRED` (Linux):** `struct ucred { pid_t pid; uid_t uid; gid_t gid; }`, unpack `"3i"`. Untestable on the macOS primary platform — must run on a Linux CI runner, or be marked "Linux-CI-only, unverified on dev host." ### Dependency facts -- `websockets>=13.0` (pyproject.toml:27); installed 16.0. `process_request` - async/sync both accepted. +- Pin `websockets>=16,<17` (replacing `websockets>=13.0` in pyproject.toml:27); + installed 16.0. Rationale: `_process_request` depends on the websockets 16 + handshake/transport API, and major-version drift must not silently change that + contract. - `requires-python = ">=3.12"` (pyproject.toml:6). - `pytest>=8.0.0`, `pytest-asyncio>=0.24.0`, `asyncio_mode = "auto"` (pyproject.toml:46-47,70) — `async def test_*` needs no marker. @@ -315,8 +333,8 @@ in Phase 2/3 via the `socketpair()` unit test before wiring in):** | Seam | Contract | Verified by | |---|---|---| -| `start()` → dir enforcement | Refuse to bind unless parent is `0700` + owner-owned | Phase 1 tests | -| `_process_request` → `peer_uid()` | UDS only; `None` or uid mismatch → `403` before handshake | Phase 3 tests | +| `start()` → dir enforcement | Refuse to bind unless every ancestor through the trusted root is owner-owned and not group/other-writable, except sticky-bit dirs | Phase 1 tests | +| `_process_request` → `peer_uid()` | UDS only; `None`, exception, or uid mismatch → `403` before handshake | Phase 3 tests | | `peer_uid()` → OS | macOS `getpeereid` / Linux `SO_PEERCRED`; unknown → `None` (fail closed) | Phase 2 unit tests | | TCP path | Unchanged: Origin + bearer token only | existing tests stay green | @@ -330,20 +348,20 @@ sequenceDiagram participant C as Client (same uid) participant K as Kernel participant S as stt_server (_process_request) - Note over S: start(): enforce parent dir 0700 + owner — else refuse to start + Note over S: start(): enforce owner-only ancestor chain — else refuse to start C->>S: connect() over UDS, then WS upgrade S->>K: get_extra_info("socket") → getpeereid/SO_PEERCRED K-->>S: peer uid alt uid == server uid S-->>C: None (allow) → handshake completes - else uid != server uid OR resolver None + else uid != server uid OR resolver None/raises S-->>C: respond(403) → transport.abort() end ``` | Step | Trigger | Enters context | Cleared/persisted | Turn boundary | |---|---|---|---|---| -| Bind | `start()` | socket_path, parent dir stat | dir verified once at start | startup | +| Bind | `start()` | socket_path, trusted root, ancestor stats | ancestor chain verified once at start | startup | | Accept | client connect | peer uid from kernel | per-connection, not stored | per connection | | Reject | uid mismatch | 403 response | connection aborted | per connection | @@ -360,76 +378,87 @@ sequenceDiagram monkeypatching `sys.platform`** (even if the off-host syscall is mocked), so dispatch logic is covered regardless of CI host. Test the fail-closed `None` branch (unknown platform / call failure) by monkeypatching. -- **Dir enforcement** (`tests/test_stt_server.py`): (a) server creates parent - `0700` when absent and starts; (b) pre-existing `0755` parent → start refuses - with actionable error (assert the `stt_server: ` + exit-1 surface, not a - bare traceback); (c) verify the created dir's mode after start; (d) - **foreign-owner branch:** monkeypatch `os.stat` to return a foreign `st_uid` - and assert the helper raises **without** calling `os.chmod`/`os.chown` (the - "must not chmod/chown what it does not own" invariant — hard to do with a real +- **Dir enforcement** (`tests/test_stt_server.py`): (a) server creates missing + socket directories `0700` when absent and starts; (b) pre-existing `0755` + component in the ancestor walk → start refuses with actionable error (assert + the `stt_server: ` + exit-1 surface, not a bare traceback); (c) verify + created directory modes after start; (d) **foreign-owner ancestor branch:** + monkeypatch `os.stat` so a grandparent directory reports a foreign `st_uid` and + assert the helper rejects **without** calling `os.chmod`/`os.chown` (the "must + not chmod/chown what it does not own" invariant — hard to do with a real foreign-owned dir on single-uid CI). - **UDS peer-cred — two layers:** - *CI (seam):* monkeypatch `peer_uid` → `euid+1`, expect `403`; force `get_extra_info("socket") → None` and assert `403` (not an exception/allow); - plus a same-uid multi-connection regression case that resolves the **real** - peer uid == `os.geteuid()` (so a silently-`None` transport is caught). + monkeypatch `peer_uid` to raise `OSError` and assert `_process_request` + returns `403` rather than leaking the exception; plus a same-uid + multi-connection regression case that resolves the **real** peer uid == + `os.geteuid()` (so a silently-`None` transport is caught). - *Local (real, Phase 4):* `scripts/smoke_peercred.py` / `just smoke-peercred` runs the example client under a second uid against a server built directly - with `unix_socket_mode=0o666`, a `0711` parent dir, and the test-only - dir-enforcement bypass — defeating **both** filesystem layers so peer-cred is - what rejects. Asserts a real `403` via `InvalidStatus.status_code` + the + with `unix_socket_mode=0o666`, a `0711` parent dir, and a test-only helper + replacement — defeating **both** filesystem layers so peer-cred is what + rejects. Asserts a real `403` via `InvalidStatus.status_code` + the `"peer not permitted\n"` body (a pre-handshake HTTP response, **not** a protocol JSON envelope). Skips cleanly when no second uid / `sudo`. - Full suite: `uv run pytest -q`. `uv run ruff check && uv run ruff format` before push. ## Acceptance Criteria -- [ ] Server refuses to start when the socket parent dir is not `0700` + - owner-owned, with an actionable `stt_server: ` error + `SystemExit(1)` - (not a bare traceback); creates it `0700` when absent. `install_stt_agent.sh` - creates the dir `0700` so fresh installs/upgrades do not break. +- [ ] Server refuses to start when any non-sticky ancestor from the socket bind + directory through the trusted root is not owner-owned or is group/other-writable, + with an actionable `stt_server: ` error + `SystemExit(1)` (not a bare + traceback); creates missing socket directories `0700` when absent. A test where + a grandparent dir is owned by a different uid rejects. `install_stt_agent.sh` + creates the socket dir `0700` so fresh installs/upgrades do not break. - [ ] UDS connections from a foreign uid are rejected with `403` before the handshake; same-uid connections succeed unchanged. Every fail-closed path - (resolver `None`, missing transport socket, unknown platform) rejects. + (resolver `None`, resolver exception such as `OSError`, missing transport + socket, unknown platform) rejects. - [ ] Local `just smoke-peercred` demonstrates a real cross-uid `403` (asserted via `InvalidStatus.status_code` + `"peer not permitted\n"` body) against a server whose `0o666` socket mode **and** `0711` parent dir both permit the peer, - with dir-enforcement bypassed for the harness — so peer-cred is provably what + using a test-only helper replacement that cannot be triggered through `serve()` + or normal `TranscriptionServer` construction — so peer-cred is provably what rejects. N concurrent same-uid sessions all succeed. - [ ] `peer_uid()` resolves on macOS (ctypes `getpeereid`) and Linux - (`SO_PEERCRED`); branch selection is covered on any host; unknown platforms + (`SO_PEERCRED`); the ctypes binding sets `argtypes`, `restype`, and + `use_errno=True`; branch selection is covered on any host; unknown platforms fail closed. +- [ ] `websockets` is pinned to `>=16,<17` and the lockfile reflects the pin. - [ ] No client-side change required; existing client connects unmodified. - [ ] TCP path and bearer-token behavior unchanged. -- [ ] The test-only dir-enforcement bypass is never settable from the public CLI - / `serve()` entrypoint. +- [ ] Any test-only dir-enforcement seam is implemented by subclassing or + monkeypatching the helper, not by a `ServerConfig` field or other public/API- + reachable flag. - [ ] `uv run pytest -q` green; `ruff check` + `ruff format` clean. - [ ] Docs describe the same-host trust model and the same-uid precondition. ## Review Focus -- **Fail-closed posture:** confirm every error path (resolver `None`, ctypes - failure, unknown platform, missing transport socket) results in *reject*, never - *allow*. +- **Fail-closed posture:** confirm every error path (resolver `None`, resolver + exception, ctypes failure, unknown platform, missing transport socket) results + in *reject*, never *allow*. - **TOCTOU on the dir check:** the stat-then-bind is itself a small window; - confirm `0700`-owner-owned makes the window unexploitable (no other uid can - win the race without write on the parent). + confirm the owner-owned, non-writable ancestor walk through the trusted root + makes the window unexploitable (no other uid can win the race without write on + an ancestor component; sticky-bit dirs remain the explicit exception). - **websockets 16 contract:** verify `connection.transport` is reliably set when `_process_request` runs and `get_extra_info("socket")` returns the AF_UNIX socket (not `None`). - **UDS-vs-TCP gating:** ensure peer-cred runs only for UDS and TCP is untouched. - **Same-uid precondition (R4):** confirm the deployment assumption is documented and that uid mismatch rejects rather than warn-allows. -- **Test-only bypass containment:** the `_skip_socket_dir_enforcement` escape - hatch (Phase 4) must be unreachable from the public CLI / `serve()` — it exists - solely so the smoke can observe peer-cred in isolation. Verify no production - path can set it. -- **Defense-in-depth framing:** confirm the docs present `0700` dir as the primary - same-host boundary and peer-cred as the kernel-authoritative backstop — not as a - strictly-stronger replacement (since #1 alone already blocks the same-host - foreign-uid vector at the filesystem layer). - - +- **Test-only seam containment:** the Phase 4 helper replacement must be local to + tests/smoke code and unreachable from the public CLI, `serve()`, or normal + `TranscriptionServer` construction. Verify no production path can trigger it. +- **Defense-in-depth framing:** confirm the docs present the owner-only ancestor + chain as the primary same-host boundary and peer-cred as the + kernel-authoritative backstop — not as a strictly-stronger replacement (since + #1 alone already blocks the same-host foreign-uid vector at the filesystem + layer). + + From 839e718ae187cd427fda93eb16fd79e99d304511 Mon Sep 17 00:00:00 2001 From: Varun Singh <382354+vr000m@users.noreply.github.com> Date: Fri, 26 Jun 2026 22:03:18 -0700 Subject: [PATCH 03/24] =?UTF-8?q?conduct:=20phase=201=20=E2=80=94=20Parent?= =?UTF-8?q?-directory=20enforcement?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- scripts/install_stt_agent.sh | 16 ++- stt_server/__main__.py | 23 ++-- stt_server/server.py | 89 ++++++++++++- tests/test_mlx_teardown_spike.py | 33 ++++- tests/test_stt_server.py | 215 ++++++++++++++++++++++++++++++- 5 files changed, 353 insertions(+), 23 deletions(-) diff --git a/scripts/install_stt_agent.sh b/scripts/install_stt_agent.sh index d8c5053..65c9157 100755 --- a/scripts/install_stt_agent.sh +++ b/scripts/install_stt_agent.sh @@ -97,9 +97,19 @@ fi cmd="${1:-install}" render_plist() { - mkdir -p "$LOG_DIR" "$(dirname "$PLIST_DST")" "$(dirname "$SOCKET_PATH")" - # Lock the socket's parent directory so another local user can't - # pre-create a socket at the same path under a permissive umask. + mkdir -p "$LOG_DIR" "$(dirname "$PLIST_DST")" + # Create the socket's parent directory owner-only (0700) from birth so + # there is no window under a permissive umask (commonly 0755) in which + # another local user could pre-create a socket at the same path. The server + # now *refuses to start* (see _enforce_socket_dir_secure in + # stt_server/server.py) if any ancestor through the trusted root is + # group/other-writable, so 0700 here is load-bearing, not just hygiene. + # + # Upgrade note: installs predating this change created the dir at the + # install shell umask (commonly 0755). The trailing `chmod 700` repairs such + # a pre-existing dir in place so upgrades do not trip the new startup check; + # `mkdir -m 700` covers the fresh-install path with no race window. + mkdir -m 700 -p "$(dirname "$SOCKET_PATH")" chmod 700 "$(dirname "$SOCKET_PATH")" # Delegate to plistlib (via render_stt_plist.py) so XML escaping and # allowlist validation handle hostile values instead of sed string diff --git a/stt_server/__main__.py b/stt_server/__main__.py index c4d0fd6..8552d2d 100644 --- a/stt_server/__main__.py +++ b/stt_server/__main__.py @@ -213,15 +213,22 @@ def _cmd_serve(args: argparse.Namespace) -> None: format="%(asctime)s %(levelname)s %(name)s: %(message)s", ) backend = _make_backend(args.backend, _resolve_model(args.backend, args.model)) - asyncio.run( - serve( - backend, - socket_path=args.socket_path, - host=args.host, - port=args.port, - auth_token=_resolve_auth_token(args.auth_token_file), + try: + asyncio.run( + serve( + backend, + socket_path=args.socket_path, + host=args.host, + port=args.port, + auth_token=_resolve_auth_token(args.auth_token_file), + ) ) - ) + except (ValueError, OSError) as exc: + # Surface startup failures (socket-dir enforcement refusing to bind, + # the ServerConfig.__post_init__ ValueError, bind OSErrors) as an + # actionable one-line message + exit 1 rather than a bare traceback. + print(f"stt_server: {exc}", file=sys.stderr) + raise SystemExit(1) async def _probe_status(args: argparse.Namespace) -> dict: diff --git a/stt_server/server.py b/stt_server/server.py index 5b4de79..3ec73de 100644 --- a/stt_server/server.py +++ b/stt_server/server.py @@ -85,6 +85,84 @@ def _item_id() -> str: return f"item_{uuid.uuid4().hex[:16]}" +def _enforce_socket_dir_secure(path: Path, trusted_root: Path) -> None: + """Refuse to bind unless the socket's directory chain cannot be hijacked. + + The UDS file mode (``0o600``) stops a foreign uid from ``connect()``-ing, + but it does NOT stop a plant/swap: anyone with write on an ancestor + directory can ``unlink()`` our socket and ``bind()`` their own at the same + path, after which a client connects to the attacker's socket. The only + defense is denying others write on every directory that could replace the + socket. We therefore walk every component from the socket's bind directory + up to AND INCLUDING ``trusted_root`` and require each to be owned by the + running uid (``st_uid == os.geteuid()``) and not group/other-writable + (``st_mode & 0o022 == 0``); sticky-bit directories are the explicit + exception (the sticky bit already prevents non-owners from removing our + inode, which is the property we need). + + Missing socket directories are created ``0o700`` and then re-``stat``-ed and + verified rather than trusted: the ``mkdir`` mode is masked by the process + umask, so we confirm the result instead of assuming it. A pre-existing + directory we do not own is never silently ``chmod``/``chown``-ed — we raise + instead, naming the offending component and condition. + + Raises ``ValueError`` on any failure; the serve entrypoint turns that into + ``stt_server: `` + ``SystemExit(1)`` rather than a bare traceback. + """ + euid = os.geteuid() + bind_dir = path.parent + trusted_root = trusted_root.resolve() + resolved_bind = bind_dir.resolve() + + # The socket MUST live under the trusted root. A path outside it (e.g. a + # custom socket path pointing at a world-writable location) has no + # owner-only chain we can vouch for, and walking to the filesystem root + # would only verify root-owned system directories that we do not own. Refuse + # rather than pretend the path is safe. + if resolved_bind != trusted_root and trusted_root not in resolved_bind.parents: + raise ValueError( + f"socket directory {resolved_bind} is not under the trusted root " + f"{trusted_root}; point the socket at a path under {trusted_root}" + ) + + # Create every missing component owner-only. Each is created with its own + # ``mkdir(mode=0o700)`` rather than ``parents=True``: the latter creates + # intermediate parents at the umask default (not ``mode``), which would + # leave a group/other-readable directory in the chain. ``mkdir``'s mode is + # itself umask-masked, but ``0o700`` carries no group/other bits for any + # umask to widen, and the walk below re-stats and verifies regardless. + to_create: list[Path] = [] + cursor = bind_dir + while not cursor.exists(): + to_create.append(cursor) + parent = cursor.parent + if parent == cursor: + break + cursor = parent + for missing in reversed(to_create): + missing.mkdir(mode=0o700) + + component = resolved_bind + while True: + st = os.stat(component) + if st.st_uid != euid: + raise ValueError( + f"socket directory component {component} is owned by uid " + f"{st.st_uid}, not the server uid {euid}; refusing to bind " + "(another user could replace the socket at this path)" + ) + sticky = bool(st.st_mode & 0o1000) + if not sticky and (st.st_mode & 0o022): + raise ValueError( + f"socket directory component {component} is group/other-writable " + f"(mode {oct(st.st_mode & 0o7777)}); refusing to bind " + "(another user could replace the socket at this path)" + ) + if component == trusted_root: + break + component = component.parent + + @dataclass class ServerConfig: """Transport and policy configuration for ``TranscriptionServer``.""" @@ -146,7 +224,16 @@ async def start(self) -> None: await self._backend.start() if self._config.socket_path: socket_path = Path(self._config.socket_path) - socket_path.parent.mkdir(parents=True, exist_ok=True) + # Trusted root for the ancestor walk: the user's home directory. The + # default socket lives under ~/Library/Caches/pipecat-stt (see + # scripts/install_stt_agent.sh), so on stock macOS every component + # from the cache dir up through home is owner-owned and 0700. Enforce + # an owner-only, non-group/other-writable chain from the socket + # directory up to and including home BEFORE bind so the socket cannot + # be planted/swapped by another local uid. This replaces a bare + # mkdir that trusted the path blindly and verifies the full ancestor + # walk, not just the immediate parent. + _enforce_socket_dir_secure(socket_path, Path.home()) # Restrict the socket file to owner-only before bind so the UDS # trust boundary actually holds on multi-user hosts. prior_umask = os.umask(0o077) diff --git a/tests/test_mlx_teardown_spike.py b/tests/test_mlx_teardown_spike.py index e2275e7..679ad49 100644 --- a/tests/test_mlx_teardown_spike.py +++ b/tests/test_mlx_teardown_spike.py @@ -156,11 +156,16 @@ async def _drain_until_updated(client: TranscriptionClient) -> None: # -------------------------------------------------------------------------- -async def test_shutdown_drains_two_concurrent_decodes(): +async def test_shutdown_drains_two_concurrent_decodes(monkeypatch): """Koda's ``me`` + ``them`` shape: two clients each mid-commit against a backend that takes 500 ms. ``shutdown()`` must drain both inside the configured budget (2 s here; real default is 10 s) and call ``backend.close()`` exactly once afterwards.""" + # _start_server binds under /tmp (root-owned), which R1 dir-enforcement + # rejects. This test exercises the drain path, not ancestor-dir enforcement, + # and the temp dir already exists before start(), so neutralise the check + # with a pure no-op via the Phase-4-sanctioned seam. + monkeypatch.setattr("stt_server.server._enforce_socket_dir_secure", lambda *a, **k: None) backend = _SlowBackend(decode_seconds=0.5) srv, sock = await _start_server(backend, drain=2.0) try: @@ -195,10 +200,14 @@ async def test_shutdown_drains_two_concurrent_decodes(): pass -async def test_shutdown_force_cancels_past_drain_timeout(): +async def test_shutdown_force_cancels_past_drain_timeout(monkeypatch): """Backend that never completes a decode — drain budget expires, force cancel path runs (server.py:228-233). ``shutdown()`` must still return promptly instead of wedging on the stuck decode.""" + # /tmp is root-owned so R1 dir-enforcement rejects it; this test exercises + # the force-cancel drain path, not ancestor-dir enforcement, and the temp + # dir already exists before start(), so neutralise with a no-op (Phase-4 seam). + monkeypatch.setattr("stt_server.server._enforce_socket_dir_secure", lambda *a, **k: None) backend = _HangingBackend() srv, sock = await _start_server(backend, drain=0.5) try: @@ -224,10 +233,14 @@ async def test_shutdown_force_cancels_past_drain_timeout(): pass -async def test_shutdown_is_idempotent_under_double_call(): +async def test_shutdown_is_idempotent_under_double_call(monkeypatch): """Simulates SIGTERM arriving twice mid-drain (launchctl does not de-duplicate). Second ``shutdown()`` must be a no-op — no double-close on the listener, no extra ``backend.close()`` call.""" + # /tmp is root-owned so R1 dir-enforcement rejects it; this test exercises + # shutdown idempotency, not ancestor-dir enforcement, and the temp dir + # already exists before start(), so neutralise with a no-op (Phase-4 seam). + monkeypatch.setattr("stt_server.server._enforce_socket_dir_secure", lambda *a, **k: None) backend = _SlowBackend(decode_seconds=0.1) srv, sock = await _start_server(backend, drain=2.0) try: @@ -248,11 +261,16 @@ async def test_shutdown_is_idempotent_under_double_call(): pass -async def test_fresh_server_after_shutdown_accepts_new_connections(): +async def test_fresh_server_after_shutdown_accepts_new_connections(monkeypatch): """LaunchAgent respawn analog: start → shutdown → start a NEW ``TranscriptionServer`` on the same socket path → new client connects cleanly. Catches stale-listener / stale-socket-file regressions that would make ``ThrottleInterval=10`` restarts crash-loop.""" + # Both servers bind under /tmp (root-owned), which R1 dir-enforcement + # rejects. This test exercises respawn-on-same-socket, not ancestor-dir + # enforcement, and the temp dir already exists before start(), so neutralise + # with a no-op via the Phase-4-sanctioned seam. + monkeypatch.setattr("stt_server.server._enforce_socket_dir_secure", lambda *a, **k: None) tmp = tempfile.mkdtemp(prefix="mlx-spike.", dir="/tmp") sock = Path(tmp) / "s" @@ -284,10 +302,15 @@ async def test_fresh_server_after_shutdown_accepts_new_connections(): await srv2.shutdown() -async def test_backend_close_called_exactly_once_on_force_cancel_path(): +async def test_backend_close_called_exactly_once_on_force_cancel_path(monkeypatch): """Even when shutdown hits the force-cancel branch (drain timeout expired with pending decodes), ``backend.close()`` is called exactly once. Regressions here would leave MLX state leaked across respawns.""" + # /tmp is root-owned so R1 dir-enforcement rejects it; this test exercises + # the force-cancel close-once invariant, not ancestor-dir enforcement, and + # the temp dir already exists before start(), so neutralise with a no-op + # via the Phase-4-sanctioned seam. + monkeypatch.setattr("stt_server.server._enforce_socket_dir_secure", lambda *a, **k: None) backend = _HangingBackend() srv, sock = await _start_server(backend, drain=0.2) try: diff --git a/tests/test_stt_server.py b/tests/test_stt_server.py index 5b64c49..76ccec0 100644 --- a/tests/test_stt_server.py +++ b/tests/test_stt_server.py @@ -513,9 +513,13 @@ async def test_bearer_auth_requires_token(): await srv.shutdown() -async def test_unix_socket_has_owner_only_permissions(): +async def test_unix_socket_has_owner_only_permissions(monkeypatch): import stat + # Binds under /tmp (root-owned), which R1 dir-enforcement rejects. This + # test exercises UDS perms, not ancestor-dir enforcement, so neutralise the + # check via the Phase-4-sanctioned seam. + monkeypatch.setattr("stt_server.server._enforce_socket_dir_secure", lambda *a, **k: None) with tempfile.TemporaryDirectory(prefix="stt.", dir="/tmp") as d: sock = Path(d) / "s" srv = TranscriptionServer(EchoBackend(), ServerConfig(socket_path=str(sock))) @@ -528,7 +532,15 @@ async def test_unix_socket_has_owner_only_permissions(): await srv.shutdown() -async def test_unix_socket_start_creates_parent_directory(): +async def test_unix_socket_start_creates_parent_directory(monkeypatch): + # Binds under /tmp (root-owned), which R1 ownership enforcement rejects. + # This test exercises parent-dir *creation*, so the stub keeps the mkdir + # behaviour but drops the ancestor ownership/perms check (the part /tmp + # trips). Sanctioned by the Phase-4 test-only seam. + monkeypatch.setattr( + "stt_server.server._enforce_socket_dir_secure", + lambda path, trusted_root: path.parent.mkdir(mode=0o700, parents=True, exist_ok=True), + ) with tempfile.TemporaryDirectory(prefix="stt.", dir="/tmp") as d: sock = Path(d) / "nested" / "path" / "s" srv = TranscriptionServer(EchoBackend(), ServerConfig(socket_path=str(sock))) @@ -540,8 +552,12 @@ async def test_unix_socket_start_creates_parent_directory(): await srv.shutdown() -async def test_unix_socket_transport(): +async def test_unix_socket_transport(monkeypatch): # AF_UNIX paths on macOS cap at ~104 bytes; use a short /tmp path. + # /tmp is root-owned so R1 dir-enforcement rejects it; this test exercises + # UDS transport, not ancestor-dir enforcement, so neutralise the check via + # the Phase-4-sanctioned seam. + monkeypatch.setattr("stt_server.server._enforce_socket_dir_secure", lambda *a, **k: None) with tempfile.TemporaryDirectory(prefix="stt.", dir="/tmp") as d: sock = Path(d) / "s" srv = TranscriptionServer( @@ -605,8 +621,12 @@ async def test_tcp_without_token_emits_startup_warning(caplog): await srv.shutdown() -async def test_uds_without_token_does_not_warn(caplog): +async def test_uds_without_token_does_not_warn(caplog, monkeypatch): caplog.set_level("WARNING", logger="stt_server.server") + # Binds under a system temp dir (root-owned ancestor) which R1 rejects; + # this test exercises the UDS-vs-TCP token warning, not ancestor-dir + # enforcement, so neutralise the check via the Phase-4-sanctioned seam. + monkeypatch.setattr("stt_server.server._enforce_socket_dir_secure", lambda *a, **k: None) with tempfile.TemporaryDirectory() as tmp: sock = str(Path(tmp) / "stt.sock") srv = TranscriptionServer( @@ -812,8 +832,12 @@ def test_resolve_probe_endpoint_loads_dotenv_even_with_explicit_socket(monkeypat assert _resolve_auth_token(None, client=True) == "from-dotenv" -async def test_cli_status_with_explicit_socket_reads_token_from_dotenv(tmp_path: Path): +async def test_cli_status_with_explicit_socket_reads_token_from_dotenv(tmp_path: Path, monkeypatch): pytest.importorskip("dotenv") + # Binds the in-process server's socket directly under /tmp (root-owned), + # which R1 dir-enforcement rejects; this test exercises the CLI token-probe + # path, not ancestor-dir enforcement, so neutralise via the Phase-4 seam. + monkeypatch.setattr("stt_server.server._enforce_socket_dir_secure", lambda *a, **k: None) sock = Path("/tmp") / f"stt-preflight-{os.getpid()}.sock" sock.unlink(missing_ok=True) srv = TranscriptionServer( @@ -1346,10 +1370,15 @@ async def test_parakeet_24_to_300s_utterance_reaches_backend_intact(): await srv.shutdown() -async def test_cli_status_client_does_not_use_server_only_token(tmp_path: Path): +async def test_cli_status_client_does_not_use_server_only_token(tmp_path: Path, monkeypatch): """P1 regression at the CLI boundary: KODA_STT_AUTH_TOKEN alone must not authenticate the probe — otherwise a health check can report ok while the bot (which only reads STT_WS_TOKEN) still 401s.""" + # Binds the in-process server's socket directly under /tmp (root-owned), + # which R1 dir-enforcement rejects; this test exercises the client-vs-server + # token contract, not ancestor-dir enforcement, so neutralise via the + # Phase-4-sanctioned seam. + monkeypatch.setattr("stt_server.server._enforce_socket_dir_secure", lambda *a, **k: None) sock = Path("/tmp") / f"stt-preflight-p1-{os.getpid()}.sock" sock.unlink(missing_ok=True) srv = TranscriptionServer( @@ -1385,3 +1414,177 @@ async def test_cli_status_client_does_not_use_server_only_token(tmp_path: Path): finally: await srv.shutdown() sock.unlink(missing_ok=True) + + +# --------------------------------------------------------------------------- +# Phase 1 — Parent-directory enforcement (R1). +# +# Pins the contract of the private helper +# _enforce_socket_dir_secure(path: Path, trusted_root: Path) +# which walks every directory component from the socket's bind directory up to +# and including ``trusted_root`` requiring ``st_uid == os.geteuid()`` and no +# group/other write bits (sticky-bit dirs excepted), creating any missing +# socket directories ``0700`` (then re-stat-verifying, since mkdir's mode is +# umask-masked). On any failure it raises a clear exception naming the offending +# component; it MUST NOT chmod/chown a pre-existing directory it does not own. +# +# These call the helper directly (no event loop / bind needed). ``tempfile`` +# creates the trusted root via ``mkdtemp`` which is reliably ``0700``, so the +# root itself passes the walk on single-uid CI. +# --------------------------------------------------------------------------- + + +def _make_trusted_root() -> Path: + """A 0700, owner-owned temp dir suitable as the trusted root of the walk.""" + return Path(tempfile.mkdtemp(prefix="stt-dirsec.")) + + +def test_enforce_socket_dir_creates_missing_socket_dir_0700(): + # (a)+(c): an absent socket bind directory is created 0700 and the helper + # succeeds (no exception). The walk through the trusted root must pass. + import shutil + import stat as _stat + + from stt_server.server import _enforce_socket_dir_secure + + root = _make_trusted_root() + try: + bind_dir = root / "pipecat-stt" + sock = bind_dir / "s" + assert not bind_dir.exists() + + _enforce_socket_dir_secure(sock, root) # must not raise + + assert bind_dir.is_dir(), "helper must create the missing socket directory" + mode = _stat.S_IMODE(bind_dir.stat().st_mode) + assert mode == 0o700, f"socket dir created as {oct(mode)}, expected 0o700" + finally: + shutil.rmtree(root, ignore_errors=True) + + +def test_enforce_socket_dir_created_modes_are_0700_under_loose_umask(): + # (c): even under a permissive process umask the created dirs end up 0700 + # (the helper must verify/repair, not trust the umask-masked mkdir mode). + import shutil + import stat as _stat + + from stt_server.server import _enforce_socket_dir_secure + + root = _make_trusted_root() + prior = os.umask(0o022) + try: + bind_dir = root / "nested" + sock = bind_dir / "s" + + _enforce_socket_dir_secure(sock, root) + + mode = _stat.S_IMODE(bind_dir.stat().st_mode) + assert mode == 0o700, f"created dir is {oct(mode)} under umask 0o022, expected 0o700" + finally: + os.umask(prior) + shutil.rmtree(root, ignore_errors=True) + + +def test_enforce_socket_dir_refuses_group_writable_parent_dir(): + # (b): a pre-existing group/other-writable ancestor in the walk must make + # the helper refuse with an actionable error naming the offending path. + # + # NOTE on the fixture mode: R1 / the Acceptance Criteria define the failure + # as "group/other-writable" (st_mode & 0o022 != 0). A plain 0o755 dir is + # NOT group/other-writable, so we use 0o775 (group-writable) which refuses + # under both the literal `& 0o022` rule and any stricter `& 0o077` reading. + import shutil + + from stt_server.server import _enforce_socket_dir_secure + + root = _make_trusted_root() + try: + loose = root / "loose" + loose.mkdir() + os.chmod(loose, 0o775) # group-writable: defeats the trust boundary + sock = loose / "s" + + with pytest.raises((ValueError, OSError)) as exc: + _enforce_socket_dir_secure(sock, root) + + msg = str(exc.value) + assert str(loose) in msg, f"error must name the offending path; got: {msg!r}" + finally: + shutil.rmtree(root, ignore_errors=True) + + +def test_cli_serve_refuses_group_writable_socket_dir_with_exit1(): + # (b) CLI surface: a group-writable socket parent dir must surface as + # "stt_server: " on stderr + exit code 1, not a bare traceback. + # Uses the echo backend so `serve` does not load a heavy model and fails + # fast at the dir-enforcement check before binding. + import shutil + + root = Path(tempfile.mkdtemp(prefix="stt-cli-dirsec.")) + try: + loose = root / "loose" + loose.mkdir() + os.chmod(loose, 0o777) # world+group-writable parent + sock = loose / "s.sock" + + r = _run_module( + "serve", + "--backend", + "echo", + "--socket-path", + str(sock), + ) + assert r.returncode == 1, f"expected exit 1; stdout={r.stdout!r} stderr={r.stderr!r}" + assert "stt_server:" in r.stderr, f"expected actionable error, got: {r.stderr!r}" + assert "Traceback" not in r.stderr, f"must not leak a bare traceback: {r.stderr!r}" + finally: + shutil.rmtree(root, ignore_errors=True) + + +def test_enforce_socket_dir_foreign_owner_ancestor_rejects_without_chmod_chown(monkeypatch): + # (d): a grandparent dir reporting a foreign st_uid must be rejected, and + # the helper MUST NOT attempt to chmod/chown a directory it does not own. + import shutil + + from stt_server.server import _enforce_socket_dir_secure + + root = _make_trusted_root() + try: + # Pre-create a fully valid 0700 chain so no mkdir happens; the only + # failure is the monkeypatched foreign ownership of `child`. + child = root / "child" + child.mkdir(mode=0o700) + bind_dir = child / "gc" + bind_dir.mkdir(mode=0o700) + sock = bind_dir / "s" + + real_stat = os.stat + foreign_uid = os.geteuid() + 1 + + def fake_stat(path, *args, **kwargs): + st = real_stat(path, *args, **kwargs) + try: + target = os.path.realpath(os.fspath(path)) + except TypeError: + return st # fd-based stat: leave untouched + if target == os.path.realpath(child): + fields = list(st) # the 10 canonical stat fields + fields[4] = foreign_uid # st_uid + return os.stat_result(fields) + return st + + chmod_calls: list = [] + chown_calls: list = [] + monkeypatch.setattr(os, "stat", fake_stat) + monkeypatch.setattr(os, "chmod", lambda *a, **k: chmod_calls.append(a)) + monkeypatch.setattr(os, "chown", lambda *a, **k: chown_calls.append(a)) + + with pytest.raises((ValueError, OSError)): + _enforce_socket_dir_secure(sock, root) + + # Invariant: never touch ownership/mode of a dir we do not own. + assert chmod_calls == [], f"helper chmod'd an unowned dir: {chmod_calls!r}" + assert chown_calls == [], f"helper chown'd an unowned dir: {chown_calls!r}" + finally: + monkeypatch.undo() + shutil.rmtree(root, ignore_errors=True) From 49b34dc421ffa385e2ba2f59ace28cc0e1674d74 Mon Sep 17 00:00:00 2001 From: Varun Singh <382354+vr000m@users.noreply.github.com> Date: Fri, 26 Jun 2026 22:06:23 -0700 Subject: [PATCH 04/24] =?UTF-8?q?conduct:=20phase=202=20=E2=80=94=20Peer-c?= =?UTF-8?q?redential=20resolver?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- stt_server/_peercred.py | 125 ++++++++++++++++++++++++++++++++++++ tests/test_peercred.py | 137 ++++++++++++++++++++++++++++++++++++++++ 2 files changed, 262 insertions(+) create mode 100644 stt_server/_peercred.py create mode 100644 tests/test_peercred.py diff --git a/stt_server/_peercred.py b/stt_server/_peercred.py new file mode 100644 index 0000000..860b325 --- /dev/null +++ b/stt_server/_peercred.py @@ -0,0 +1,125 @@ +"""Cross-platform peer-credential resolution for AF_UNIX sockets. + +The server authenticates a connecting UDS client by the **kernel-supplied** +peer uid rather than anything the client sends. The uid the kernel captures at +``connect()`` time is unforgeable, which is why peer-cred auth is the +kernel-authoritative defense-in-depth backstop behind the owner-only filesystem +ancestor chain (see R3 / R4 in the dev plan). + +``peer_uid(sock)`` returns the connecting peer's uid, or ``None`` when it cannot +be resolved (unknown platform, missing socket, or any syscall failure). The +contract is **fail-closed**: a ``None`` return tells the caller to reject the +connection. This module deliberately raises nothing on the resolution paths — +the caller treats ``None`` as "reject," so a leaked exception would be a worse +failure mode than an explicit ``None``. + +Platform notes: + +- **Linux** uses ``SO_PEERCRED``: ``getsockopt`` returns a ``struct ucred { + pid_t pid; uid_t uid; gid_t gid; }``, unpacked as ``"3i"``. +- **macOS** has no ``SO_PEERCRED``; we call ``getpeereid(2)`` through + ``ctypes``. The wrinkle worth flagging for future maintainers: ``uid_t`` and + ``gid_t`` are ``c_uint32`` on Darwin, and we set ``argtypes``/``restype`` on + the libc function **explicitly**. A wrong width or signature could fail *open* + (e.g. an uninitialized output buffer that happens to compare equal to the + server uid), so the binding is pinned and the ``socketpair()`` unit test that + asserts ``peer_uid() == os.geteuid()`` is the gate against a width/signature + mistake. We also load libc with ``use_errno=True`` for correct errno + propagation. + +The same-uid precondition (R4): the client and server are assumed to run as the +same uid (per the Koda cross-repo contract, both are per-user LaunchAgents). A +uid mismatch is rejected, never warn-and-allowed. + +Kept import-light and side-effect-free so it is unit-testable without binding a +server (mirrors the existing single ``sys.platform == "darwin"`` precedent in +``server.py``; no new abstraction framework). +""" + +from __future__ import annotations + +import logging +import socket +import struct +import sys +from typing import Protocol, runtime_checkable + +logger = logging.getLogger("stt_server") + + +@runtime_checkable +class PeerCredSocket(Protocol): + """Minimal structural socket contract the resolver depends on. + + Only the members ``peer_uid`` actually touches are declared, so callers can + pass the concrete ``socket.socket`` and tests can pass a lightweight mock — + the resolver is decoupled from the concrete socket class. + """ + + family: int + + def fileno(self) -> int: ... + + def getsockopt(self, level: int, optname: int, buflen: int) -> bytes: ... + + +def peer_uid(sock: PeerCredSocket) -> int | None: + """Return the connecting peer's uid, or ``None`` if it cannot be resolved. + + ``None`` is the fail-closed signal: the caller rejects the connection. This + covers unknown platforms and any syscall failure. The actual syscall paths + are wrapped so an unexpected error yields ``None`` rather than propagating. + """ + try: + if sys.platform.startswith("linux"): + return _peer_uid_linux(sock) + if sys.platform == "darwin": + return _peer_uid_darwin(sock) + except Exception: # noqa: BLE001 - fail closed on any resolution failure + logger.warning("peer_uid: failed to resolve peer credentials on %s", sys.platform) + return None + + logger.warning( + "peer_uid: no peer-credential mechanism for platform %s; failing closed", + sys.platform, + ) + return None + + +def _peer_uid_linux(sock: PeerCredSocket) -> int | None: + """Resolve the peer uid via ``SO_PEERCRED`` (``struct ucred``).""" + buf = sock.getsockopt(socket.SOL_SOCKET, socket.SO_PEERCRED, struct.calcsize("3i")) + _pid, uid, _gid = struct.unpack("3i", buf) + return uid + + +def _peer_uid_darwin(sock: PeerCredSocket) -> int | None: + """Resolve the peer uid via ``getpeereid(2)`` through ``ctypes``. + + ``uid_t``/``gid_t`` are ``c_uint32`` on Darwin; ``argtypes``/``restype`` are + set explicitly and libc is loaded with ``use_errno=True`` (see module + docstring for why the binding must be pinned). + """ + import ctypes + + libc = ctypes.CDLL(None, use_errno=True) + getpeereid = libc.getpeereid + getpeereid.argtypes = [ + ctypes.c_int, + ctypes.POINTER(ctypes.c_uint32), + ctypes.POINTER(ctypes.c_uint32), + ] + getpeereid.restype = ctypes.c_int + + uid = ctypes.c_uint32() + gid = ctypes.c_uint32() + rc = getpeereid(sock.fileno(), ctypes.byref(uid), ctypes.byref(gid)) + if rc != 0: + errno = ctypes.get_errno() + logger.warning( + "peer_uid: getpeereid failed (rc=%d, errno=%d); failing closed", + rc, + errno, + ) + return None + return uid.value diff --git a/tests/test_peercred.py b/tests/test_peercred.py new file mode 100644 index 0000000..fdec118 --- /dev/null +++ b/tests/test_peercred.py @@ -0,0 +1,137 @@ +"""Unit tests for the cross-platform peer-credential resolver. + +Covers Phase 2 of the UDS trust-boundary hardening plan: ``peer_uid(sock)`` +in ``stt_server._peercred`` resolves the connecting peer's uid via +``getpeereid(2)`` (macOS) or ``SO_PEERCRED`` (Linux), and fails closed +(returns ``None``) on unknown platforms or syscall failure. + +These are plain synchronous unit tests (no asyncio); they follow the +``monkeypatch`` idiom used by ``tests/test_env_helpers.py`` and do not rely on +a ``conftest.py``. +""" + +from __future__ import annotations + +import os +import socket +import struct +import sys + +import pytest + +from stt_server._peercred import peer_uid + + +# --------------------------------------------------------------------------- +# Host-platform acceptance gate +# +# The real gate that the ctypes binding (width + signature) and getpeereid +# semantics are correct on the dev host (macOS), and that SO_PEERCRED unpacking +# is correct on Linux. A connected AF_UNIX socketpair has both ends owned by +# this process, so the peer uid must equal our effective uid. +# --------------------------------------------------------------------------- + + +def test_peer_uid_getpeereid_on_real_socketpair_returns_geteuid(): + """The host-platform gate: real socketpair peer uid == os.geteuid().""" + if sys.platform not in ("darwin", "linux"): + pytest.skip(f"no peer-cred resolver path for platform {sys.platform!r}") + a, b = socket.socketpair(socket.AF_UNIX, socket.SOCK_STREAM) + try: + assert peer_uid(a) == os.geteuid() + # Symmetric: both ends are this process, so both resolve identically. + assert peer_uid(b) == os.geteuid() + finally: + a.close() + b.close() + + +# --------------------------------------------------------------------------- +# Branch-selection coverage regardless of CI host +# --------------------------------------------------------------------------- + + +def test_linux_branch_unpacks_so_peercred_uid(monkeypatch): + """Force the Linux branch; mock getsockopt to return a packed ucred.""" + monkeypatch.setattr(sys, "platform", "linux") + # ``socket.SO_PEERCRED`` is Linux-only and absent on the macOS dev host; + # provide it so the Linux dispatch branch can be exercised on any host. + # The fake socket ignores the optname, so the concrete value is irrelevant. + if not hasattr(socket, "SO_PEERCRED"): + monkeypatch.setattr(socket, "SO_PEERCRED", 17, raising=False) + + pid, uid, gid = 4321, 1234, 20 + + class FakeSocket: + family = socket.AF_UNIX + + def fileno(self): + return -1 + + def getsockopt(self, level, optname, buflen): + # SO_PEERCRED returns struct ucred { pid_t; uid_t; gid_t } == "3i". + assert buflen == struct.calcsize("3i") + return struct.pack("3i", pid, uid, gid) + + assert peer_uid(FakeSocket()) == uid + + +def test_darwin_branch_selected_on_real_socketpair(monkeypatch): + """Force the darwin branch; on a darwin host this exercises real ctypes.""" + if sys.platform != "darwin": + pytest.skip("darwin ctypes getpeereid path only runs on macOS host") + monkeypatch.setattr(sys, "platform", "darwin") + a, b = socket.socketpair(socket.AF_UNIX, socket.SOCK_STREAM) + try: + assert peer_uid(a) == os.geteuid() + finally: + a.close() + b.close() + + +# --------------------------------------------------------------------------- +# Fail-closed: None branch +# --------------------------------------------------------------------------- + + +def test_unknown_platform_returns_none(monkeypatch): + """Unsupported platform must fail closed (None), never silently allow.""" + monkeypatch.setattr(sys, "platform", "sunos5") + a, b = socket.socketpair(socket.AF_UNIX, socket.SOCK_STREAM) + try: + assert peer_uid(a) is None + finally: + a.close() + b.close() + + +def test_linux_branch_syscall_failure_returns_none(monkeypatch): + """A raising getsockopt on the Linux branch must return None, not raise.""" + monkeypatch.setattr(sys, "platform", "linux") + + class RaisingSocket: + family = socket.AF_UNIX + + def fileno(self): + return -1 + + def getsockopt(self, level, optname, buflen): + raise OSError("getsockopt failed") + + assert peer_uid(RaisingSocket()) is None + + +def test_darwin_branch_call_failure_returns_none(monkeypatch): + """getpeereid failure (bad fd / non-zero return) must fail closed to None.""" + if sys.platform != "darwin": + pytest.skip("darwin ctypes getpeereid path only runs on macOS host") + monkeypatch.setattr(sys, "platform", "darwin") + + class BadFdSocket: + family = socket.AF_UNIX + + def fileno(self): + # An invalid descriptor makes getpeereid(2) fail (EBADF / non-zero). + return -1 + + assert peer_uid(BadFdSocket()) is None From 7773c796438052b0a4431a07d6a8ab41333dbd22 Mon Sep 17 00:00:00 2001 From: Varun Singh <382354+vr000m@users.noreply.github.com> Date: Fri, 26 Jun 2026 22:12:52 -0700 Subject: [PATCH 05/24] =?UTF-8?q?conduct:=20phase=203=20=E2=80=94=20Wire?= =?UTF-8?q?=20peer-cred=20into=20the=20handshake?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- pyproject.toml | 10 +-- stt_server/server.py | 32 ++++++++ tests/test_stt_server.py | 161 +++++++++++++++++++++++++++++++++++++++ uv.lock | 10 +-- 4 files changed, 203 insertions(+), 10 deletions(-) diff --git a/pyproject.toml b/pyproject.toml index 426c7c5..39bdfdf 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -24,22 +24,22 @@ dependencies = [ # hard runtime dependency of the base package. Backend ASR engines live # behind the [mlx] / [parakeet] extras so a client-only consumer (e.g. a # bot that just talks to a running server) does not pull MLX/Whisper. - "websockets>=13.0", + "websockets>=16,<17", ] [project.optional-dependencies] # Pure client deps — what a downstream consumer of `stt_server.client` needs # without pulling any MLX/Whisper runtime. -client = ["websockets>=13.0"] +client = ["websockets>=16,<17"] # Full server install with the MLX Whisper backend. -mlx = ["websockets>=13.0", "mlx-whisper>=0.4.0", "numpy>=1.26"] +mlx = ["websockets>=16,<17", "mlx-whisper>=0.4.0", "numpy>=1.26"] # Full server install with the Parakeet TDT backend (via parakeet-mlx). -parakeet = ["websockets>=13.0", "parakeet-mlx>=0.5.1", "numpy>=1.26"] +parakeet = ["websockets>=16,<17", "parakeet-mlx>=0.5.1", "numpy>=1.26"] # Full server install with the Nemotron 3.5 ASR backend (via mlx-audio). # mlx-audio>=0.4.4 is the first PyPI release carrying Nemotron STT support # (Blaizzy/mlx-audio#774, merged 2026-06-05); earlier releases lacked it, which # previously forced a git-pinned dev group. Now a clean, PyPI-installable extra. -nemotron = ["websockets>=13.0", "mlx-audio>=0.4.4", "numpy>=1.26"] +nemotron = ["websockets>=16,<17", "mlx-audio>=0.4.4", "numpy>=1.26"] [dependency-groups] dev = [ diff --git a/stt_server/server.py b/stt_server/server.py index 3ec73de..f62465f 100644 --- a/stt_server/server.py +++ b/stt_server/server.py @@ -26,6 +26,7 @@ import os import resource import signal +import socket import sys import time import uuid @@ -42,6 +43,7 @@ ) from . import protocol as P +from ._peercred import peer_uid from .backend import BackendStream, EchoBackend, TranscriptionBackend # OpenAI Realtime groups errors into a coarse "type" field (e.g. @@ -346,6 +348,36 @@ async def shutdown(self) -> None: # --- connection handling --- async def _process_request(self, connection, request): + # On a Unix-domain socket the only legitimate peer is a process owned + # by the same effective uid as this server. Enforce that BEFORE (and + # independently of) bearer auth so a foreign uid is rejected even when + # no token is configured. Fail closed: any inability to resolve the + # peer's uid is a 403, not a pass. TCP listeners cannot answer + # SO_PEERCRED-style queries, so this gate is UDS-only. + if self._config.socket_path is not None: + sock = connection.transport.get_extra_info("socket") + if sock is None: + logger.warning("stt_server: UDS connection has no underlying socket; rejecting") + return connection.respond(403, "peer not permitted\n") + if sock.family != socket.AF_UNIX: + logger.warning( + "stt_server: UDS connection on unexpected socket family %r; rejecting", + sock.family, + ) + return connection.respond(403, "peer not permitted\n") + try: + uid = peer_uid(sock) + except Exception as exc: + logger.warning("stt_server: peer uid resolution failed (%s); rejecting", exc) + return connection.respond(403, "peer not permitted\n") + if uid is None or uid != os.geteuid(): + logger.warning( + "stt_server: rejecting UDS peer uid %r (expected %d)", + uid, + os.geteuid(), + ) + return connection.respond(403, "peer not permitted\n") + # Reject unexpected browser Origin headers for non-browser-focused V1, # and enforce optional bearer auth in one place. headers = request.headers diff --git a/tests/test_stt_server.py b/tests/test_stt_server.py index 76ccec0..2ea66ca 100644 --- a/tests/test_stt_server.py +++ b/tests/test_stt_server.py @@ -1588,3 +1588,164 @@ def fake_stat(path, *args, **kwargs): finally: monkeypatch.undo() shutil.rmtree(root, ignore_errors=True) + + +# --------------------------------------------------------------------------- +# Phase 3 — UDS peer-credential gate in _process_request (CI seam) +# +# The server authenticates a UDS peer by its kernel-supplied uid before the +# WebSocket handshake completes (R2). Every fail-closed path — missing +# transport socket, resolver returning None, resolver raising, or a uid that +# mismatches os.geteuid() — must return ``connection.respond(403, "peer not +# permitted\n")`` rather than allow or leak an exception. A same-uid peer +# (the only legitimate case under the same-uid deployment precondition) must +# pass the gate untouched. +# +# Two layers, per the plan's Testing Notes: +# * integration: bind a real UDS server (dir-enforcement neutralised via the +# sanctioned seam so it can bind under /tmp) and connect with the client; +# * unit: drive ``_process_request`` directly with fakes for the cases that +# cannot be staged with a real same-uid socket (None transport socket, +# resolver raising). +# --------------------------------------------------------------------------- + + +class _FakeTransport: + """Minimal asyncio-transport stand-in exposing only get_extra_info.""" + + def __init__(self, sock): + self._sock = sock + + def get_extra_info(self, name, default=None): + if name == "socket": + return self._sock + return default + + +class _FakeConnection: + """Minimal ``ServerConnection`` stand-in for _process_request unit tests. + + ``respond(status, body)`` records the call and returns a sentinel so the + test can assert _process_request returned the reject response (the + websockets contract: returning a respond() result rejects the handshake). + """ + + def __init__(self, sock): + self.transport = _FakeTransport(sock) + self.responses: list[tuple[int, str]] = [] + + def respond(self, status, body): + self.responses.append((status, body)) + return ("RESPOND", status, body) + + +class _FakeRequest: + def __init__(self, headers=None): + self.headers = headers or {} + + +def _uds_server_for_unit_test() -> TranscriptionServer: + """A TranscriptionServer in UDS mode WITHOUT binding. + + ``_process_request`` only reads ``self._config.socket_path`` to decide the + gate runs, so a config with any socket_path is enough — no start() needed. + """ + return TranscriptionServer( + EchoBackend(), + ServerConfig(socket_path="/tmp/peercred-unit-test.sock"), + ) + + +async def test_uds_auth_foreign_uid_rejected_with_403(monkeypatch): + """A UDS peer whose resolved uid != os.geteuid() is rejected pre-handshake. + + Bind a real UDS server (dir-enforcement neutralised via the sanctioned + seam so it can bind under /tmp), force the resolver to report a foreign + uid, and assert the client connect raises InvalidStatus(403). Mirrors the + existing 401 bearer-auth test's InvalidStatus handling. + """ + monkeypatch.setattr("stt_server.server._enforce_socket_dir_secure", lambda *a, **k: None) + # Foreign uid: resolver claims the peer is someone other than us. + monkeypatch.setattr("stt_server.server.peer_uid", lambda sock: os.geteuid() + 1) + with tempfile.TemporaryDirectory(prefix="stt.", dir="/tmp") as d: + sock = Path(d) / "s" + srv = TranscriptionServer(EchoBackend(), ServerConfig(socket_path=str(sock))) + await srv.start() + try: + c = TranscriptionClient(socket_path=str(sock)) + with pytest.raises(websockets.exceptions.InvalidStatus) as exc: + await c.connect() + assert exc.value.response.status_code == 403 + # Body assertion is best-effort: InvalidStatus exposes the response, + # whose body may be bytes or absent depending on websockets version. + # status_code == 403 is the load-bearing assertion (per the plan). + body = getattr(exc.value.response, "body", None) + if body: + body_bytes = body if isinstance(body, (bytes, bytearray)) else body.encode() + assert b"peer not permitted" in bytes(body_bytes) + finally: + await srv.shutdown() + + +async def test_uds_auth_peercred_missing_transport_socket_rejects_with_403(): + """A UDS handshake whose transport has no underlying socket must 403. + + Unit-drives _process_request with a fake connection whose + ``transport.get_extra_info("socket")`` returns None. The gate must return + the 403 respond() result (fail closed) and NOT raise (it must not call + peer_uid(None), which would AttributeError). + """ + srv = _uds_server_for_unit_test() + conn = _FakeConnection(sock=None) + result = await srv._process_request(conn, _FakeRequest()) + assert conn.responses == [(403, "peer not permitted\n")] + assert result == ("RESPOND", 403, "peer not permitted\n") + + +async def test_uds_auth_peer_uid_resolver_raises_rejects_with_403(monkeypatch): + """If the peer_uid resolver raises (e.g. OSError), the gate must 403, not + leak the exception. Provide a non-None AF_UNIX socket so the gate reaches + the resolver call, then force the resolver to raise. + """ + import socket as _socket + + def _raise(_sock): + raise OSError("getpeereid failed") + + monkeypatch.setattr("stt_server.server.peer_uid", _raise) + srv = _uds_server_for_unit_test() + + class _FakeUnixSock: + family = _socket.AF_UNIX + + conn = _FakeConnection(sock=_FakeUnixSock()) + result = await srv._process_request(conn, _FakeRequest()) + assert conn.responses == [(403, "peer not permitted\n")] + assert result == ("RESPOND", 403, "peer not permitted\n") + + +async def test_uds_auth_peer_uid_same_uid_real_resolver_completes_handshake(monkeypatch): + """Same-uid regression with the REAL resolver (no peer_uid stub). + + Bind a real UDS server (dir-enforcement neutralised) and connect as the + owning uid. The handshake must complete and a normal session work — this + catches a silently-None transport socket that would otherwise fail closed. + Kept to a single connection; the N-concurrent case is Phase 4. + """ + monkeypatch.setattr("stt_server.server._enforce_socket_dir_secure", lambda *a, **k: None) + with tempfile.TemporaryDirectory(prefix="stt.", dir="/tmp") as d: + sock = Path(d) / "s" + srv = TranscriptionServer(EchoBackend(), ServerConfig(socket_path=str(sock))) + await srv.start() + try: + c = TranscriptionClient(socket_path=str(sock)) + hello = await c.connect() + assert hello["type"] == P.EVT_SERVER_HELLO + # A real session works end-to-end through the gate. + await c.send_audio(_pcm(800)) + await c.commit() + completed = await _next_event_of_types(c, {P.EVT_TRANSCRIPT_COMPLETED}) + assert completed["transcript"] == "echo:1600" + await c.close() + finally: + await srv.shutdown() diff --git a/uv.lock b/uv.lock index 2705301..c83cf72 100644 --- a/uv.lock +++ b/uv.lock @@ -1137,11 +1137,11 @@ requires-dist = [ { name = "numpy", marker = "extra == 'nemotron'", specifier = ">=1.26" }, { name = "numpy", marker = "extra == 'parakeet'", specifier = ">=1.26" }, { name = "parakeet-mlx", marker = "extra == 'parakeet'", specifier = ">=0.5.1" }, - { name = "websockets", specifier = ">=13.0" }, - { name = "websockets", marker = "extra == 'client'", specifier = ">=13.0" }, - { name = "websockets", marker = "extra == 'mlx'", specifier = ">=13.0" }, - { name = "websockets", marker = "extra == 'nemotron'", specifier = ">=13.0" }, - { name = "websockets", marker = "extra == 'parakeet'", specifier = ">=13.0" }, + { name = "websockets", specifier = ">=16,<17" }, + { name = "websockets", marker = "extra == 'client'", specifier = ">=16,<17" }, + { name = "websockets", marker = "extra == 'mlx'", specifier = ">=16,<17" }, + { name = "websockets", marker = "extra == 'nemotron'", specifier = ">=16,<17" }, + { name = "websockets", marker = "extra == 'parakeet'", specifier = ">=16,<17" }, ] provides-extras = ["client", "mlx", "parakeet", "nemotron"] From 35de5ec7e65d6d1caccec6e54959d1c10207bcab Mon Sep 17 00:00:00 2001 From: Varun Singh <382354+vr000m@users.noreply.github.com> Date: Fri, 26 Jun 2026 22:56:01 -0700 Subject: [PATCH 06/24] =?UTF-8?q?conduct:=20phase=204=20=E2=80=94=20Local?= =?UTF-8?q?=20end-to-end=20smoke=20(multi-connection=20+=20cross-uid)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- justfile | 11 ++ scripts/smoke_peercred.py | 358 ++++++++++++++++++++++++++++++++++++++ tests/test_stt_server.py | 63 +++++++ 3 files changed, 432 insertions(+) create mode 100644 scripts/smoke_peercred.py diff --git a/justfile b/justfile index 9baaa58..df2f0b4 100644 --- a/justfile +++ b/justfile @@ -164,6 +164,17 @@ stt-enable backend: fi echo "stt-enable: bootstrapped + kickstarted $label" +# Local UDS peer-cred smoke: same-uid multi-connection + cross-uid 403. +# Runs an in-process server with both filesystem layers deliberately permissive +# (0711 parent + 0o666 socket) via a test-only helper replacement, so peer-cred +# is provably what rejects a foreign uid. The cross-uid leg needs a second local +# uid reachable via passwordless `sudo`; it skips cleanly (exit 0) when absent, +# while the same-uid leg still runs. +smoke-peercred: + #!/usr/bin/env bash + set -uo pipefail + exec uv run python "{{justfile_directory()}}/scripts/smoke_peercred.py" + # Install an agent — delegates to install_stt_agent.sh (no plist reimplementation). stt-install backend: #!/usr/bin/env bash diff --git a/scripts/smoke_peercred.py b/scripts/smoke_peercred.py new file mode 100644 index 0000000..a29b5ba --- /dev/null +++ b/scripts/smoke_peercred.py @@ -0,0 +1,358 @@ +#!/usr/bin/env python3 +"""Local end-to-end smoke for the UDS peer-credential trust boundary. + +Not part of the pytest suite — a one-command check of the two things a +single-uid CI run structurally cannot exercise: + +1. **Cross-uid rejection (the real test, local-only).** A connection from a + *foreign* local uid must be rejected by peer-cred (#3) with a pre-handshake + HTTP ``403`` and body ``peer not permitted\\n``. To prove peer-cred is what + rejects (and not the filesystem boundary), the server is built so BOTH + filesystem layers permit the peer: a deliberately-traversable ``0711`` parent + dir AND a ``0o666`` socket mode. That combination would normally be refused at + startup by ``_enforce_socket_dir_secure`` (R1), so this script replaces that + helper through a **test-only monkeypatch** that is *not* reachable from + ``serve()`` or normal ``TranscriptionServer`` construction — there is no + ``ServerConfig`` field or public flag for it. The foreign uid is driven via + ``sudo -u ``; with no second uid / passwordless sudo available the path + skips cleanly (exit 0), it does not fail. + +2. **Same-uid multi-connection (CI-safe).** N concurrent ``TranscriptionClient`` + sessions as the owning uid must all complete the handshake and stream — a + regression guard that peer-cred did not break the normal path under + concurrency. It also resolves the peer uid through the **real** + ``stt_server._peercred.peer_uid`` on a live socket and asserts it equals + ``os.geteuid()``, so a silently-``None`` transport is caught rather than + masked. + +Usage:: + + uv run python scripts/smoke_peercred.py + uv run python scripts/smoke_peercred.py --connections 8 + +Run via ``just smoke-peercred``. The server runs IN-PROCESS (not as a +subprocess) so the test-only helper replacement and ``unix_socket_mode=0o666`` +can be applied directly — the public ``serve()`` exposes neither. +""" + +from __future__ import annotations + +import argparse +import asyncio +import contextlib +import os +import pwd +import socket +import subprocess +import sys +import tempfile + +# Run from the repo root so ``stt_server`` imports resolve (also required when +# this file is re-invoked under ``sudo -u`` for the foreign-uid connect role). +sys.path.insert(0, os.path.dirname(os.path.dirname(os.path.abspath(__file__)))) + +import websockets.exceptions # noqa: E402 + +from stt_server import _peercred # noqa: E402 +from stt_server.backend import EchoBackend # noqa: E402 +from stt_server.client import TranscriptionClient # noqa: E402 +from stt_server.protocol import ( # noqa: E402 + AUDIO_CHANNELS, + AUDIO_FORMAT, + AUDIO_SAMPLE_RATE_HZ, + EVT_SERVER_HELLO, + EVT_TRANSCRIPT_COMPLETED, + PROTOCOL_VERSION, +) + +REJECT_BODY = "peer not permitted\n" +# Tokens the foreign-uid child prints on stdout so the parent can classify the +# outcome without parsing a traceback. +CHILD_REJECTED = "PEERCRED_REJECTED_403" +CHILD_ACCEPTED = "PEERCRED_ACCEPTED" +CHILD_OTHER = "PEERCRED_OTHER" + + +# --------------------------------------------------------------------------- +# Child role: connect under whatever uid this process is running as. +# --------------------------------------------------------------------------- +async def _connect_as_peer(sock: str) -> int: + """Attempt one handshake against ``sock``; print a classification token. + + Exit code mirrors the token so a ``sudo`` caller can branch on either. A + peer-cred reject is the SUCCESS case for the cross-uid test, so it exits 0. + """ + client = TranscriptionClient(socket_path=sock) + try: + await client.connect() + except websockets.exceptions.InvalidStatus as exc: + response = exc.response + body = response.body.decode("utf-8", "replace") if response.body else "" + if response.status_code == 403 and body == REJECT_BODY: + print(f"{CHILD_REJECTED} status={response.status_code} body={body!r}") + return 0 + print(f"{CHILD_OTHER} status={response.status_code} body={body!r}") + return 2 + except Exception as exc: # noqa: BLE001 - surface any unexpected failure + print(f"{CHILD_OTHER} exception={type(exc).__name__}: {exc}") + return 2 + else: + with contextlib.suppress(Exception): + await client.close() + print(CHILD_ACCEPTED) + return 1 + + +# --------------------------------------------------------------------------- +# Same-uid multi-connection path (CI-safe). +# --------------------------------------------------------------------------- +def _assert_hello(hello: dict) -> None: + """Assert ``server.hello`` matches what ``server.py`` actually emits.""" + assert hello["type"] == EVT_SERVER_HELLO, hello + assert hello["protocol_version"] == PROTOCOL_VERSION, hello + assert hello["capabilities"] == { + "binary_audio": True, + "base64_audio_append": True, + "server_vad": False, + }, hello + assert hello["audio"] == { + "format": AUDIO_FORMAT, + "rate": AUDIO_SAMPLE_RATE_HZ, + "channels": AUDIO_CHANNELS, + }, hello + # EchoBackend identity: name "echo", model None. + assert hello["backend"]["name"] == "echo", hello + + +async def _one_session(sock: str, index: int) -> str: + """Run one full connect → stream → completed session; return transcript.""" + async with TranscriptionClient(socket_path=sock) as client: + hello = await client.connect() + _assert_hello(hello) + await client.update_session(turn_detection=None) + # 50 ms of silence is enough for EchoBackend (it echoes the byte count). + pcm = b"\x00" * (AUDIO_SAMPLE_RATE_HZ * AUDIO_CHANNELS * 2 // 20) + await client.send_audio(pcm) + await client.commit() + async for ev in client.events(): + if ev.get("type") == EVT_TRANSCRIPT_COMPLETED: + transcript = ev.get("transcript", "") + await client.close_session() + return transcript + raise SystemExit(f"session {index}: closed without a transcript.completed event") + + +def _assert_real_resolver(sock: str) -> None: + """Resolve the peer uid on a live socket via the REAL resolver. + + Connecting a raw AF_UNIX socket to the server gives us a socket whose *peer* + is the server process; ``peer_uid`` on it returns the server's uid. Since the + server runs as us, it must equal ``os.geteuid()``. A ``None`` here would mean + the transport silently yielded no creds — exactly the masked failure this + guards against. + """ + raw = socket.socket(socket.AF_UNIX, socket.SOCK_STREAM) + try: + raw.connect(sock) + resolved = _peercred.peer_uid(raw) + finally: + raw.close() + expected = os.geteuid() + if resolved is None: + raise SystemExit( + "real peer_uid resolver returned None on a live socket — the " + "transport is not exposing peer credentials (silent fail-open risk)" + ) + if resolved != expected: + raise SystemExit(f"real peer_uid resolver returned {resolved}, expected {expected}") + print(f" real resolver: peer_uid == os.geteuid() == {expected} OK") + + +async def _run_same_uid(sock: str, connections: int) -> None: + print(f"\n=== same-uid multi-connection ({connections} concurrent) ===") + results = await asyncio.gather(*(_one_session(sock, i) for i in range(connections))) + assert all(r.startswith("echo:") for r in results), results + print(f" {len(results)} concurrent sessions all completed: {sorted(set(results))}") + _assert_real_resolver(sock) + + +# --------------------------------------------------------------------------- +# Cross-uid path (local-only). +# --------------------------------------------------------------------------- +def _find_second_uid() -> tuple[str, int] | None: + """Return (username, uid) of a real local user reachable via passwordless + ``sudo``, or ``None`` if none is available (the usual CI / dev case).""" + if sys.platform not in ("darwin", "linux"): + return None + if not _have("sudo"): + return None + me = os.geteuid() + # macOS normal users start at 501; Linux at 1000. Keep it conservative and + # skip system accounts and ourselves. + min_uid = 501 if sys.platform == "darwin" else 1000 + for entry in pwd.getpwall(): + if entry.pw_uid == me or entry.pw_uid < min_uid: + continue + if entry.pw_name.startswith("_"): # macOS service accounts + continue + # Passwordless sudo to this user, non-interactive: succeeds silently or + # we move on. ``-n`` never prompts. + try: + probe = subprocess.run( + ["sudo", "-n", "-u", entry.pw_name, "true"], + stdout=subprocess.DEVNULL, + stderr=subprocess.DEVNULL, + timeout=10, + ) + except (OSError, subprocess.TimeoutExpired): + continue + if probe.returncode == 0: + return entry.pw_name, entry.pw_uid + return None + + +def _have(cmd: str) -> bool: + return subprocess.run( + ["command", "-v", cmd], + stdout=subprocess.DEVNULL, + stderr=subprocess.DEVNULL, + ).returncode == 0 or os.path.exists(f"/usr/bin/{cmd}") + + +async def _run_cross_uid(sock: str) -> bool: + """Drive the example client under a second uid; assert peer-cred rejects. + + Returns ``True`` if the cross-uid assertion ran, ``False`` if it was skipped. + """ + second = _find_second_uid() + if second is None: + print("\n=== cross-uid rejection ===") + print(" skipped: needs a second local uid reachable via passwordless sudo") + return False + + username, uid = second + print(f"\n=== cross-uid rejection (peer uid {uid} / {username}) ===") + # Re-invoke THIS file under the second uid in its connect-as-peer role. + cmd = [ + "sudo", + "-n", + "-u", + username, + sys.executable, + os.path.abspath(__file__), + "--connect-as-peer", + sock, + ] + proc = subprocess.run( + cmd, stdout=subprocess.PIPE, stderr=subprocess.PIPE, text=True, timeout=60 + ) + out = (proc.stdout or "").strip() + err = (proc.stderr or "").strip() + if err: + print(f" child stderr: {err}") + print(f" child stdout: {out}") + if CHILD_REJECTED not in out: + raise SystemExit( + f"cross-uid connect was NOT rejected with 403 '{REJECT_BODY.strip()}' " + f"(rc={proc.returncode}, stdout={out!r}) — peer-cred boundary FAILED" + ) + print(" cross-uid connection rejected with 403 'peer not permitted' — peer-cred OK") + return True + + +# --------------------------------------------------------------------------- +# Server lifecycle (in-process). +# --------------------------------------------------------------------------- +@contextlib.asynccontextmanager +async def _running_server(): + """Spin up a TranscriptionServer on a temp UDS with both filesystem layers + deliberately permissive, so peer-cred is the only boundary left. + + The dir-enforcement helper is replaced by a test-only no-op monkeypatch that + is unreachable from ``serve()`` / normal construction (no ServerConfig flag). + """ + import stt_server.server as server_module + + # TEST-ONLY seam: defeat R1's ancestor-chain enforcement so a 0711/0o666 + # socket can be bound. NOT reachable from serve() or public construction. + original_enforce = server_module._enforce_socket_dir_secure + server_module._enforce_socket_dir_secure = lambda *a, **k: None + + tmpdir = tempfile.mkdtemp(prefix="peercred-smoke-") + # Make the parent dir traversable by *other* uids (0711): +x for group/other + # lets a foreign uid path-resolve to the socket. R1 would normally refuse + # this; the monkeypatch above is why it binds. + os.chmod(tmpdir, 0o711) + sock = os.path.join(tmpdir, "p.sock") + + config = server_module.ServerConfig( + socket_path=sock, + # 0o666: socket is connectable by any uid — the other half of defeating + # the filesystem boundary so peer-cred is provably what rejects. + unix_socket_mode=0o666, + reject_browser_origins=False, + ) + srv = server_module.TranscriptionServer(EchoBackend(), config) + await srv.start() + try: + yield sock + finally: + await srv.shutdown() + server_module._enforce_socket_dir_secure = original_enforce + with contextlib.suppress(OSError): + os.unlink(sock) + with contextlib.suppress(OSError): + os.rmdir(tmpdir) + + +async def _run(args: argparse.Namespace) -> int: + if sys.platform not in ("darwin", "linux"): + print(f"skipped: peer-cred smoke needs macOS/Linux, not {sys.platform}") + return 0 + + async with _running_server() as sock: + # Verify socket/parent perms are actually permissive — the whole point is + # that the filesystem does NOT reject, so peer-cred provably does. + import stat as _stat + + sock_mode = _stat.S_IMODE(os.stat(sock).st_mode) + parent_mode = _stat.S_IMODE(os.stat(os.path.dirname(sock)).st_mode) + print("=== UDS peer-cred smoke ===") + print(f" socket : {sock} mode={oct(sock_mode)}") + print(f" parent : {os.path.dirname(sock)} mode={oct(parent_mode)}") + assert sock_mode & 0o006, f"socket not other-accessible: {oct(sock_mode)}" + assert parent_mode & 0o001, f"parent not other-traversable: {oct(parent_mode)}" + + await _run_same_uid(sock, args.connections) + ran_cross = await _run_cross_uid(sock) + + print("\n=== summary ===") + print(" same-uid multi-connection: PASS") + print(f" cross-uid rejection: {'PASS' if ran_cross else 'SKIPPED (no second uid)'}") + return 0 + + +def main() -> None: + parser = argparse.ArgumentParser(description=__doc__) + parser.add_argument( + "--connections", + type=int, + default=5, + help="number of concurrent same-uid sessions (default: 5)", + ) + parser.add_argument( + "--connect-as-peer", + metavar="SOCK", + default=None, + help=argparse.SUPPRESS, # internal: foreign-uid child role (via sudo -u) + ) + args = parser.parse_args() + + if args.connect_as_peer is not None: + raise SystemExit(asyncio.run(_connect_as_peer(args.connect_as_peer))) + + raise SystemExit(asyncio.run(_run(args))) + + +if __name__ == "__main__": + main() diff --git a/tests/test_stt_server.py b/tests/test_stt_server.py index 2ea66ca..527b2ae 100644 --- a/tests/test_stt_server.py +++ b/tests/test_stt_server.py @@ -1749,3 +1749,66 @@ async def test_uds_auth_peer_uid_same_uid_real_resolver_completes_handshake(monk await c.close() finally: await srv.shutdown() + + +async def test_uds_multi_connection_same_uid_all_handshake(monkeypatch): + """Phase 4 CI-safe mirror: N concurrent same-uid sessions all pass the gate. + + Opens N concurrent ``TranscriptionClient`` sessions as the owning uid against + a single real UDS server (dir-enforcement neutralised via the sanctioned seam + so it binds under /tmp). All N must complete the handshake (``server.hello``) + and stream a normal echo exchange — a regression guard that the peer-cred gate + (Phase 3) did not break the normal path under concurrency. + + The REAL resolver runs: ``peer_uid`` is deliberately NOT stubbed, so each + accepted connection had its uid resolved by the kernel-backed resolver and + compared equal to ``os.geteuid()``; a silently-``None`` transport would fail + closed (403) and the handshake would raise rather than succeed. To pin the + "real resolver == geteuid()" invariant directly (not just transitively via a + successful handshake), we also assert ``peer_uid`` on a live AF_UNIX + ``socketpair()`` end returns ``os.geteuid()`` — the same assertion the + resolver unit test uses, exercised here against the production import path. + """ + import socket as _socket + + from stt_server.server import peer_uid as _server_peer_uid + + # Direct real-resolver assertion on a connected AF_UNIX socketpair (both ends + # are this process -> same uid). Catches a width/signature regression or a + # silently-None resolver before we even look at the server. + a, b = _socket.socketpair(_socket.AF_UNIX, _socket.SOCK_STREAM) + try: + assert _server_peer_uid(a) == os.geteuid() + assert _server_peer_uid(b) == os.geteuid() + finally: + a.close() + b.close() + + # Bind one real UDS server; do NOT stub peer_uid so the real gate runs for + # every connection. + monkeypatch.setattr("stt_server.server._enforce_socket_dir_secure", lambda *a, **k: None) + n = 4 + with tempfile.TemporaryDirectory(prefix="stt.", dir="/tmp") as d: + sock = Path(d) / "s" + srv = TranscriptionServer(EchoBackend(), ServerConfig(socket_path=str(sock))) + await srv.start() + try: + + async def _one_session(idx: int) -> None: + c = TranscriptionClient(socket_path=str(sock)) + hello = await c.connect() + assert hello["type"] == P.EVT_SERVER_HELLO + assert hello["protocol_version"] == P.PROTOCOL_VERSION + try: + # A full echo exchange proves the session works post-handshake. + await c.send_audio(_pcm(800)) + await c.commit() + completed = await _next_event_of_types(c, {P.EVT_TRANSCRIPT_COMPLETED}) + assert completed["transcript"] == "echo:1600" + finally: + await c.close() + + # All N concurrently — surfaces any per-connection gate races. + await asyncio.gather(*(_one_session(i) for i in range(n))) + finally: + await srv.shutdown() From 37aa5f2cfc3defaa62301a6fd7d319495b4f1d82 Mon Sep 17 00:00:00 2001 From: Varun Singh <382354+vr000m@users.noreply.github.com> Date: Fri, 26 Jun 2026 22:59:02 -0700 Subject: [PATCH 07/24] =?UTF-8?q?conduct:=20phase=205=20=E2=80=94=20Docs?= =?UTF-8?q?=20+=20plan/README=20sync?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- ...26-feature-uds-trust-boundary-hardening.md | 75 ++++++++++++++-- docs/dev_plans/README.md | 2 +- docs/operations.md | 89 +++++++++++++++++++ docs/protocol.md | 21 +++++ 4 files changed, 178 insertions(+), 9 deletions(-) diff --git a/docs/dev_plans/20260626-feature-uds-trust-boundary-hardening.md b/docs/dev_plans/20260626-feature-uds-trust-boundary-hardening.md index 55c5a3d..61f7f4f 100644 --- a/docs/dev_plans/20260626-feature-uds-trust-boundary-hardening.md +++ b/docs/dev_plans/20260626-feature-uds-trust-boundary-hardening.md @@ -1,6 +1,6 @@ # Feature: UDS server-side trust-boundary hardening -**Status:** Reviewed (2026-06-26) — ready to implement +**Status:** Implemented (2026-06-26) on `feature/uds-trust-boundary-hardening` (phases 1–5) — pending Koda checkout coordination + merge **Component:** Server Transport **Assignee:** unassigned **Priority:** High (security; trust boundary) @@ -464,20 +464,79 @@ sequenceDiagram ## Progress -- [ ] Phase 1: Parent-directory enforcement -- [ ] Phase 2: Peer-credential resolver -- [ ] Phase 3: Wire peer-cred into the handshake -- [ ] Phase 4: Local end-to-end smoke (multi-connection + cross-uid) -- [ ] Phase 5: Docs + plan/README sync +- [x] Phase 1: Parent-directory enforcement +- [x] Phase 2: Peer-credential resolver +- [x] Phase 3: Wire peer-cred into the handshake +- [x] Phase 4: Local end-to-end smoke (multi-connection + cross-uid) +- [x] Phase 5: Docs + plan/README sync ## Findings - (append findings here as work proceeds) +- **Phase 1 (839e718):** trusted root chosen as `Path.home()` (the default socket + lives under `~/Library/Caches/pipecat-stt`; all install paths are under `$HOME`). + Consequence: any test/deployment binding a real UDS under a non-home dir (`/tmp`) + is correctly rejected by R1. Pre-existing UDS integration tests in + `tests/test_stt_server.py` and `tests/test_mlx_teardown_spike.py` were updated to + opt out of enforcement via the plan-sanctioned monkeypatch seam + (`monkeypatch.setattr("stt_server.server._enforce_socket_dir_secure", ...)`), + not by changing the implementation. +- **Pre-existing failure (not this feature):** + `tests/test_justfile_recipes.py::test_justfile_map_mirrors_readme` fails on the + clean base commit too ("README per-ASR table header not found") — unrelated to + UDS hardening; left untouched. Suite is otherwise green. +- **Plan gap to resolve in Phase 3:** the `websockets>=16,<17` pin (acceptance + criterion + Files-to-modify table) is not assigned to any phase's `Impl files` + slot. Folding it into Phase 3 (whose peer-cred wiring depends on the websockets + 16 handshake API), touching `pyproject.toml` + `uv.lock` as a noted deviation. ## Issues & Solutions -_(to be filled during implementation)_ +- **R1 breaks `/tmp`-bound tests (expected).** Enforcing the owner-only ancestor + chain with `trusted_root=Path.home()` means any test binding a real UDS server + under `/tmp` (root-owned) is correctly rejected. Resolved by opting those tests + out of enforcement via the plan-sanctioned monkeypatch seam + (`monkeypatch.setattr("stt_server.server._enforce_socket_dir_secure", ...)`) in + `tests/test_stt_server.py` and `tests/test_mlx_teardown_spike.py` — the + implementation was not weakened. +- **`websockets` pin had no phase home.** The `>=16,<17` pin (acceptance criterion + + Files-to-modify table) was not assigned to any phase's `Impl files` slot. + Folded into Phase 3 (whose peer-cred wiring depends on the websockets 16 + handshake API); `pyproject.toml` + `uv.lock` updated as a noted deviation. +- **`test_branch_diff_does_not_touch_koda_surface` (sibling-feature guard) is red + by design.** This guard (from the agents-justfile feature) forbids any change + under `stt_server/` or to `install_stt_agent.sh`. This security feature + intentionally hardens exactly that surface. Per the cross-repo contract the + guard's *intent* is to protect the imported client + wire protocol (neither of + which this branch touches). Left UNCHANGED per user decision (out of this plan's + file scope). One-line fix if narrowing to the contract's actual pin-bump trigger + is desired: scope the `forbidden` filter to `stt_server/client.py` + + `stt_server/protocol.py` instead of the whole `stt_server/` prefix. +- **`test_justfile_map_mirrors_readme` failure is pre-existing**, unrelated to this + feature (fails on the clean base commit too — "README per-ASR table header not + found"). Not addressed here. ## Final Results -[Fill this section when the work is complete] +Phases 1–5 implemented on `feature/uds-trust-boundary-hardening`: + +| Phase | Commit | Outcome | +|---|---|---| +| 1 — Parent-directory enforcement | `839e718` | `_enforce_socket_dir_secure` (owner-only ancestor walk to `$HOME`, creates `0700`), wired into `start()`; `_cmd_serve` turns startup errors into `stt_server: ` + exit 1; `install_stt_agent.sh` creates the socket dir `0700`. | +| 2 — Peer-credential resolver | `49b34dc` | `stt_server/_peercred.py` — `peer_uid(sock)` (Linux `SO_PEERCRED` / macOS `getpeereid` ctypes), fail-closed; verified on host (`peer_uid == os.geteuid()`). | +| 3 — Wire peer-cred into handshake | `7773c79` | UDS-only fail-closed `403` gate in `_process_request`, independent of the bearer-token branch; TCP untouched; `websockets` pinned `>=16,<17`. | +| 4 — Local end-to-end smoke | `35de5ec` | `scripts/smoke_peercred.py` + `just smoke-peercred` + multi-connection pytest; same-uid concurrency PASS, cross-uid leg skips cleanly without a second uid. | +| 5 — Docs + plan/README sync | (this commit) | Trust-model + socket-security + Koda coordination notes in `operations.md`; pre-handshake reject table in `protocol.md`; status rows updated. | + +**Verification:** each phase's `Test command:` passed; full suite green except the +two documented non-feature reds above. `ruff check` + `ruff format --check` clean. + +**Open items (owner: maintainer):** +- Koda checkout-update coordination (re-run `install_stt_agent.sh` for the + `0755→0700` socket-dir upgrade; confirm `KODA_STT_SOCKET` stays under `$HOME`). + No version/pin bump required — see `docs/operations.md` → "Cross-repo note (Koda)". +- Decide the fate of the `test_branch_diff_does_not_touch_koda_surface` guard + (leave as documented known-failure, or narrow per the one-liner above). +- The cross-uid `403` smoke leg is unverified in single-uid CI/dev; run + `just smoke-peercred` on a host with a second uid / passwordless `sudo` to + exercise it for real. diff --git a/docs/dev_plans/README.md b/docs/dev_plans/README.md index 4971740..3b45dd4 100644 --- a/docs/dev_plans/README.md +++ b/docs/dev_plans/README.md @@ -9,7 +9,7 @@ update the row in the same change that updates the plan's `**Status**` line. | [Nemotron 3.5 ASR backend (0.3.0)](20260605-nemotron-asr-backend.md) | ASR Backends | ✅ Shipped — PR #7 merged 2026-06-06; packaged as `nemotron` extra in 0.3.2 | | [justfile operator layer for STT LaunchAgents](20260607-feature-stt-agents-justfile.md) | Install & Packaging | ✅ Complete | | [STT vocabulary/prompt biasing on the wire protocol](20260607-feature-stt-prompt-biasing.md) | Wire Protocol / ASR Backends | ⬜ Not started — analysis/handoff only | -| [UDS server-side trust-boundary hardening](20260626-feature-uds-trust-boundary-hardening.md) | Server Transport | ⬜ Reviewed (2026-06-26) — ready to implement | +| [UDS server-side trust-boundary hardening](20260626-feature-uds-trust-boundary-hardening.md) | Server Transport | 🟦 Implemented on `feature/uds-trust-boundary-hardening` (phases 1–5) — pending Koda checkout coordination + merge | ## Conventions diff --git a/docs/operations.md b/docs/operations.md index e07ae6b..da05085 100644 --- a/docs/operations.md +++ b/docs/operations.md @@ -143,6 +143,95 @@ from a LaunchAgent keepalive script. The existing `--socket-path`/`--host`/ `--port`/`--auth-token-file` endpoint flags work for both `serve` and `status` subcommands. +## Trust model and socket security (same-host UDS) + +The Unix-domain-socket transport is hardened on the server side by two +independent measures. They are layered, not redundant: + +1. **Primary filesystem boundary — owner-only ancestor chain.** Before bind, + the server walks every directory from the socket's parent up to and + including the trusted root (`$HOME`) and **refuses to start** unless each + component is owned by the running uid and is not group/other-writable + (sticky-bit directories excepted). This makes the socket *un-plantable*: an + attacker cannot `unlink()` our socket and `bind()` their own, because they + have no write access on any directory that could replace it. On stock macOS + the chain (`~/Library/Caches/pipecat-stt` → `~/Library/Caches` → `~/Library` + → `~`) already satisfies this once the socket dir is `0700`. A foreign uid + cannot even *traverse* to the socket (no `+x` → `connect()` fails `EACCES` + at path resolution), so this layer alone closes the same-host foreign-uid + vector at the filesystem layer. +2. **Kernel-authoritative backstop — peer-credential auth.** On every UDS + connection the server reads the peer's uid from the kernel + (`SO_PEERCRED` on Linux, `getpeereid(2)` on macOS) and rejects any peer + whose `uid != server uid` with a pre-handshake `403` before the WebSocket + handshake completes. The uid is kernel-supplied and unforgeable, so this + holds even if the filesystem perms are ever looser than intended. It is + *defense-in-depth behind* the filesystem boundary — not a strictly-stronger + replacement for it. Every failure path (resolver returns `None`, resolver + raises, missing transport socket, unsupported platform) **fails closed** + (rejects), logging one warning. +3. **Bearer token — TCP/remote only.** For UDS the token is redundant (the + filesystem boundary + peer-cred both dominate it), so it is kept but not + relied on. For TCP/remote — which has neither a file-permission boundary nor + peer credentials — the bearer token remains the trust mechanism, alongside + the Origin check. This change does not weaken the TCP path. + +**Same-uid precondition.** Peer-cred auth assumes the client and server run as +the **same uid** (the per-user LaunchAgent deployment satisfies this). A uid +mismatch is treated as **reject**, not warn-and-allow. A future deployment that +runs the server as a daemon user and the client as the logged-in user would be +correctly rejected and would need coordination — but still no client code +change. + +### Socket directory permissions (`0700`) — upgrade note + +`scripts/install_stt_agent.sh` creates the socket's parent directory `0700` +from birth (`mkdir -m 700`) and also `chmod 700`s it, which **self-heals** a +pre-existing `0755` directory left by an older install. Because the new startup +check refuses to bind against a group/other-writable ancestor: + +- **Upgrading an existing host:** re-run `install_stt_agent.sh` (it repairs the + dir in place), or manually `chmod 700 ~/Library/Caches/pipecat-stt`. +- **Custom socket paths:** a `PIPECAT_STT_SOCKET` / `KODA_STT_SOCKET` / + `STT_WS_SOCKET` pointing outside `$HOME` (e.g. `/tmp`, `/var`, a shared dir) + now makes the server **refuse to start** with + `socket directory is not under the trusted root `. Keep custom + socket paths under `$HOME`. + +If the server refuses to start, the error is printed as `stt_server: ` +on stderr (exit 1) — grep the agent's `.err` log for +`is not under the trusted root` or `is group/other-writable`. + +### Cross-repo note (Koda) + +Koda consumes this repo two ways: it pins the **Python client** at an immutable +git SHA, and it runs the **server** from this repo's working **checkout at +HEAD** (not a tagged/PyPI release). Consequences for this hardening: + +- **No version bump gates it, and no client pin bump is needed.** The client + library and wire protocol are unchanged, so Koda's pinned client stays valid + (a pin bump is only required when the imported client/protocol surface + changes). PyPI releases are irrelevant to Koda's coupling. +- **The trigger is the checkout update, not the merge.** The hardening lands on + a Koda host the moment its server checkout is updated — decoupled from + merging to `main` (which touches no machine until something pulls). Fold the + coordination into that window: **re-run `install_stt_agent.sh`** so the socket + dir is `0700`, and confirm `KODA_STT_SOCKET` (if set) stays under `$HOME`. +- Because the startup check is **fail-closed**, the "runtime change takes effect + on checkout update" property is now a startup risk if perms/path are not + squared away in the same step — that is the entire content of the + coordination. + +### Maintainer note — macOS `getpeereid` via `ctypes` + +macOS has no `socket.SO_PEERCRED`, so the resolver in `stt_server/_peercred.py` +calls `getpeereid(2)` through `ctypes`. Two details are load-bearing and must +not be dropped: `uid_t`/`gid_t` are `c_uint32` on Darwin, and the libc function +must have explicit `argtypes`/`restype` set (a wrong width or signature can fail +*open* by comparing equal). libc is loaded with `use_errno=True`. The +`socketpair()` unit test asserting `peer_uid() == os.geteuid()` is the gate that +catches a width/signature regression — keep it. + ## Whisper hallucination suppression (MLX backend) The MLX Whisper backend forwards four decode-time knobs to diff --git a/docs/protocol.md b/docs/protocol.md index 05c0b87..7e06c7f 100644 --- a/docs/protocol.md +++ b/docs/protocol.md @@ -35,3 +35,24 @@ Deviations from the OpenAI Realtime transcription snapshot (2026-04-20): - custom events: `server.hello`, `server.status`, `session.close`, `session.cancel`, `session.closed` +## Connection rejection (pre-handshake) + +This document pins the **presence** of wire events, not their field schema; the +field shape of `server.hello`/`server.status` is defined by the server source +(`stt_server/server.py`) and the table under +[Checking server health](operations.md#checking-server-health). + +Connection-level rejections happen **before** the WebSocket handshake and are +returned as plain HTTP responses, not protocol JSON envelopes (there is no +`error` event for these): + +| Condition | Status | Body | +|---|---|---| +| Disallowed browser `Origin` | `403` | `origin not permitted` | +| UDS peer uid `!=` server uid, or peer-cred fails closed | `403` | `peer not permitted` | +| TCP bearer token missing/incorrect | `401` | `unauthorized` | + +The UDS peer-credential check (`403 peer not permitted`) is server-side only and +requires no client change — see +[Trust model and socket security](operations.md#trust-model-and-socket-security-same-host-uds). + From f1f62cffe825c2719856f755af9880df9c6edfa0 Mon Sep 17 00:00:00 2001 From: Varun Singh <382354+vr000m@users.noreply.github.com> Date: Sat, 27 Jun 2026 00:45:06 -0700 Subject: [PATCH 08/24] feat: onboard backend extras via just stt-install/stt-enable Add a private _ensure-extra helper that probes a backend's import and runs 'uv sync --extra --inexact' when the extra is missing, wired into stt-install and stt-enable. This makes 'just stt-install nemotron' produce a working agent instead of one that crash-loops on ModuleNotFoundError after a bare 'uv run'/'uv sync' prunes the optional extra. --inexact preserves other backends' extras; PIPECAT_STT_SKIP_DEP_SYNC lets operators (and the recipe tests) opt out. Kept in the justfile operator layer, off the Koda-consumed install_stt_agent.sh. --- justfile | 45 ++++++++++++++++++++++++++++++ tests/test_justfile_recipes.py | 50 +++++++++++++++++++++++++++++++++- 2 files changed, 94 insertions(+), 1 deletion(-) diff --git a/justfile b/justfile index df2f0b4..2e01335 100644 --- a/justfile +++ b/justfile @@ -151,6 +151,10 @@ stt-enable backend: echo "stt-enable: no plist at $plist — run 'just stt-install $backend' first" >&2 exit 1 fi + # Self-heal a pruned venv before re-loading: if a bare `uv run`/`uv sync` + # stripped this backend's extra since install, bootstrapping the plist would + # just resume the crash-loop. (Skipped under PIPECAT_STT_SKIP_DEP_SYNC.) + just _ensure-extra "$backend" || exit 1 # Guard each state change: set -uo pipefail does NOT abort on a failed # simple command, so an unguarded failure would be masked by the success # echo (exit 0 while the agent never started). @@ -175,7 +179,47 @@ smoke-peercred: set -uo pipefail exec uv run python "{{justfile_directory()}}/scripts/smoke_peercred.py" +# Ensure a backend's optional Python extra is installed in .venv, additively. +# The server imports its backend lib lazily in backend.start(); a bare +# `uv run`/`uv sync` prunes optional extras, so an agent can be installed yet +# crash-loop on `ModuleNotFoundError`. `stt-install`/`stt-enable` call this so +# onboarding is self-healing. `--inexact` is load-bearing: plain +# `uv sync --extra X` prunes the OTHER backends' extras, breaking a multi-backend +# host. We probe via the venv python directly — NOT `uv run`, which would itself +# re-sync/prune before we could check. Set PIPECAT_STT_SKIP_DEP_SYNC=1 to manage +# extras yourself (the recipe tests set it so they never shell out to `uv sync`). +_ensure-extra backend: + #!/usr/bin/env bash + set -uo pipefail + backend={{quote(backend)}} + # extra == the install backend-name (the 3rd field of `_resolve`); only the + # import-probe name differs from it. Validate before the skip check so an + # unknown backend always errors regardless of PIPECAT_STT_SKIP_DEP_SYNC. + case "$backend" in + whisper) extra="mlx"; probe="mlx_whisper" ;; + parakeet) extra="parakeet"; probe="parakeet_mlx" ;; + nemotron) extra="nemotron"; probe="mlx_audio" ;; + *) echo "error: unknown backend '$backend' (valid: whisper, parakeet, nemotron)" >&2; exit 1 ;; + esac + if [[ -n "${PIPECAT_STT_SKIP_DEP_SYNC:-}" ]]; then + echo "_ensure-extra: PIPECAT_STT_SKIP_DEP_SYNC set — skipping '$extra' extra check" + exit 0 + fi + py="{{justfile_directory()}}/.venv/bin/python" + if [[ -x "$py" ]] && "$py" -c "import $probe" >/dev/null 2>&1; then + echo "_ensure-extra: '$extra' extra already present ($probe importable)" + exit 0 + fi + echo "_ensure-extra: '$probe' missing — installing the '$extra' extra (uv sync --extra $extra --inexact)…" + if ! uv sync --extra "$extra" --inexact; then + echo "_ensure-extra: 'uv sync --extra $extra --inexact' failed; install the '$extra' extra manually" >&2 + exit 1 + fi + echo "_ensure-extra: '$extra' extra ready." + # Install an agent — delegates to install_stt_agent.sh (no plist reimplementation). +# Ensures the backend's Python extra first (see _ensure-extra) so the freshly +# installed agent doesn't immediately crash-loop on a missing backend import. stt-install backend: #!/usr/bin/env bash set -uo pipefail @@ -184,6 +228,7 @@ stt-install backend: # One field per line (so a spaced socket path survives); three reads keep # this bash-3.2-compatible — macOS system bash has no `mapfile`. { read -r label; read -r sock; read -r bk; } <<<"$resolved" + just _ensure-extra "$backend" || exit 1 PIPECAT_STT_LABEL="$label" PIPECAT_STT_SOCKET="$sock" PIPECAT_STT_BACKEND="$bk" \ "{{script}}" install diff --git a/tests/test_justfile_recipes.py b/tests/test_justfile_recipes.py index f1a12eb..187d9b8 100644 --- a/tests/test_justfile_recipes.py +++ b/tests/test_justfile_recipes.py @@ -133,6 +133,11 @@ def _run_just( env = { "HOME": str(home), "PATH": f"{stub_dir}:{os.environ.get('PATH', '/usr/bin:/bin')}", + # stt-install / stt-enable call `_ensure-extra`, which would otherwise + # shell out to a real `uv sync` for backends whose extra is absent from + # the dev/CI venv. Skip that side effect by default; a dedicated test + # overrides this via extra_env to exercise the skip-message path. + "PIPECAT_STT_SKIP_DEP_SYNC": "1", } if extra_env: env.update(extra_env) @@ -180,8 +185,9 @@ def test_just_list_exposes_public_recipes(tmp_path): "stt-uninstall", ): assert recipe in res.stdout - # `_resolve` is private (underscore) — not advertised. + # Private (underscore) helpers are not advertised in --list. assert "_resolve" not in res.stdout + assert "_ensure-extra" not in res.stdout @pytest.mark.parametrize( @@ -473,6 +479,48 @@ def test_install_uninstall_delegate_exact_env(tmp_path, recipe, cmd): assert "BACKEND=parakeet" in logged +# --------------------------------------------------------------------------- # +# _ensure-extra: backend -> Python extra onboarding +# --------------------------------------------------------------------------- # + + +def test_ensure_extra_skips_under_env_flag(tmp_path): + # With PIPECAT_STT_SKIP_DEP_SYNC set (the harness default), the helper must + # short-circuit BEFORE probing or shelling out to `uv sync`. + stub_dir, _ = _make_stub_dir(tmp_path) + home = _home_with_agents(tmp_path, []) + res = _run_just(["_ensure-extra", "nemotron"], home=home, stub_dir=stub_dir) + assert res.returncode == 0, res.stderr + assert "skipping" in res.stdout.lower() + + +def test_ensure_extra_unknown_backend_errors_even_when_skipped(tmp_path): + # Backend validation happens before the skip check, so an unknown backend + # errors regardless of PIPECAT_STT_SKIP_DEP_SYNC. + stub_dir, _ = _make_stub_dir(tmp_path) + home = _home_with_agents(tmp_path, []) + res = _run_just(["_ensure-extra", "bogus"], home=home, stub_dir=stub_dir) + assert res.returncode != 0 + assert "unknown backend" in res.stderr + + +def test_stt_install_invokes_ensure_extra_then_delegates(tmp_path): + # stt-install must run the dep-ensure step AND still delegate to the install + # script. Under the skip flag the ensure step is a no-op message, so we can + # assert both the skip line (proving the wiring) and the delegation. + stub_dir, _ = _make_stub_dir(tmp_path) + home = _home_with_agents(tmp_path, []) + fake_script, env_log = _delegation_stub(tmp_path) + res = _run_just( + [f"script={fake_script}", "stt-install", "nemotron"], + home=home, + stub_dir=stub_dir, + ) + assert res.returncode == 0, res.stderr + assert "skipping" in res.stdout.lower() # _ensure-extra ran + assert "CMD=install" in env_log.read_text() # delegation still happened + + # --------------------------------------------------------------------------- # # Koda-safety: the branch diff stays within the additive file set # --------------------------------------------------------------------------- # From ee1e66e5923b7a853d0d5a2741539e38cdc776f9 Mon Sep 17 00:00:00 2001 From: Varun Singh <382354+vr000m@users.noreply.github.com> Date: Sat, 27 Jun 2026 00:45:16 -0700 Subject: [PATCH 09/24] feat: actionable error when a backend's optional extra is missing Each backend's start() re-raises a missing lazy import as 'the extra is not installed ... run: uv sync --extra --inexact', and _cmd_serve now catches ModuleNotFoundError so it prints 'stt_server: ' + exit 1 instead of a bare traceback crash-looping in the LaunchAgent log. Pairs with the just _ensure-extra onboarding helper. --- stt_server/__main__.py | 7 ++--- stt_server/backends/mlx_whisper.py | 14 ++++++++-- stt_server/backends/nemotron.py | 13 +++++++-- stt_server/backends/parakeet.py | 12 ++++++++- tests/test_backend_missing_extra.py | 42 +++++++++++++++++++++++++++++ 5 files changed, 80 insertions(+), 8 deletions(-) create mode 100644 tests/test_backend_missing_extra.py diff --git a/stt_server/__main__.py b/stt_server/__main__.py index 8552d2d..baa70ac 100644 --- a/stt_server/__main__.py +++ b/stt_server/__main__.py @@ -223,10 +223,11 @@ def _cmd_serve(args: argparse.Namespace) -> None: auth_token=_resolve_auth_token(args.auth_token_file), ) ) - except (ValueError, OSError) as exc: + except (ValueError, OSError, ModuleNotFoundError) as exc: # Surface startup failures (socket-dir enforcement refusing to bind, - # the ServerConfig.__post_init__ ValueError, bind OSErrors) as an - # actionable one-line message + exit 1 rather than a bare traceback. + # the ServerConfig.__post_init__ ValueError, bind OSErrors, and a + # missing backend extra re-raised by backend.start()) as an actionable + # one-line message + exit 1 rather than a bare traceback. print(f"stt_server: {exc}", file=sys.stderr) raise SystemExit(1) diff --git a/stt_server/backends/mlx_whisper.py b/stt_server/backends/mlx_whisper.py index 0499ce2..b62f8b7 100644 --- a/stt_server/backends/mlx_whisper.py +++ b/stt_server/backends/mlx_whisper.py @@ -295,8 +295,18 @@ def _wait_inflight_drained(self, timeout_s: float) -> bool: ) async def start(self) -> None: - # Eager import; fail fast if the extra isn't installed. - import mlx_whisper # type: ignore # noqa: F401 + # Eager import; fail fast if the extra isn't installed. Re-raise a + # missing module as an actionable message (a bare ModuleNotFoundError is + # otherwise a cryptic LaunchAgent crash-loop); _cmd_serve turns this into + # ``stt_server: `` + exit 1, and ``just stt-install whisper`` + # self-heals it via _ensure-extra. + try: + import mlx_whisper # type: ignore # noqa: F401 + except ModuleNotFoundError as exc: + raise ModuleNotFoundError( + f"the 'mlx' extra is not installed (missing module: {exc.name}) " + "— run: uv sync --extra mlx --inexact" + ) from exc async def open_stream(self, *, language: str | None = None) -> "_MLXStream": return _MLXStream(self._model, language, self._decode_lock, self._thread_lock, self) diff --git a/stt_server/backends/nemotron.py b/stt_server/backends/nemotron.py index 4bcbf8f..db77b70 100644 --- a/stt_server/backends/nemotron.py +++ b/stt_server/backends/nemotron.py @@ -258,8 +258,17 @@ def _get_model(self): async def start(self) -> None: # Eager import; fail fast before the socket binds if the ``mlx-audio`` # package is not installed. The model itself is NOT loaded here — see - # ``_get_model``. - from mlx_audio.stt import load # type: ignore # noqa: F401 + # ``_get_model``. Re-raise a missing module as an actionable message — + # the bare ModuleNotFoundError is otherwise a cryptic crash-loop in the + # LaunchAgent log. _cmd_serve turns this into ``stt_server: `` + + # exit 1, and ``just stt-install nemotron`` self-heals it via _ensure-extra. + try: + from mlx_audio.stt import load # type: ignore # noqa: F401 + except ModuleNotFoundError as exc: + raise ModuleNotFoundError( + f"the 'nemotron' extra is not installed (missing module: {exc.name}) " + "— run: uv sync --extra nemotron --inexact" + ) from exc async def open_stream(self, *, language: str | None = None) -> "_NemotronStream": return _NemotronStream(language, self._decode_lock, self._thread_lock, self) diff --git a/stt_server/backends/parakeet.py b/stt_server/backends/parakeet.py index 99a92c4..28caeb1 100644 --- a/stt_server/backends/parakeet.py +++ b/stt_server/backends/parakeet.py @@ -243,7 +243,17 @@ def _get_model(self): async def start(self) -> None: # Eager import; fail fast if the ``parakeet`` extra is not # installed. The model itself is NOT loaded here — see ``_get_model``. - import parakeet_mlx # type: ignore # noqa: F401 + # Re-raise a missing module as an actionable message (a bare + # ModuleNotFoundError is otherwise a cryptic LaunchAgent crash-loop); + # _cmd_serve turns this into ``stt_server: `` + exit 1, and + # ``just stt-install parakeet`` self-heals it via _ensure-extra. + try: + import parakeet_mlx # type: ignore # noqa: F401 + except ModuleNotFoundError as exc: + raise ModuleNotFoundError( + f"the 'parakeet' extra is not installed (missing module: {exc.name}) " + "— run: uv sync --extra parakeet --inexact" + ) from exc async def open_stream(self, *, language: str | None = None) -> "_ParakeetStream": return _ParakeetStream(language, self._decode_lock, self._thread_lock, self) diff --git a/tests/test_backend_missing_extra.py b/tests/test_backend_missing_extra.py new file mode 100644 index 0000000..b7e9299 --- /dev/null +++ b/tests/test_backend_missing_extra.py @@ -0,0 +1,42 @@ +"""Backstop: a backend whose optional extra is not installed must fail with an +actionable message (``run: uv sync --extra --inexact``) rather than a bare +``ModuleNotFoundError`` traceback that crash-loops in the LaunchAgent log. + +The import is forced to fail via a ``builtins.__import__`` shim so the test is +deterministic regardless of which extras happen to be present in the dev/CI venv +(``uv run`` prunes optional extras — see the justfile ``_ensure-extra`` helper). +""" + +import asyncio +import builtins + +import pytest + +from stt_server.backends.mlx_whisper import MLXWhisperBackend +from stt_server.backends.nemotron import NemotronBackend +from stt_server.backends.parakeet import ParakeetBackend + + +@pytest.mark.parametrize( + "factory, missing, extra", + [ + (lambda: MLXWhisperBackend(model="fake"), "mlx_whisper", "mlx"), + (lambda: NemotronBackend(model="fake"), "mlx_audio", "nemotron"), + (lambda: ParakeetBackend(model="fake"), "parakeet_mlx", "parakeet"), + ], +) +def test_backend_missing_extra_raises_actionable_error(monkeypatch, factory, missing, extra): + real_import = builtins.__import__ + + def fake_import(name, *args, **kwargs): + if name == missing or name.startswith(missing + "."): + raise ModuleNotFoundError(f"No module named '{name}'", name=name) + return real_import(name, *args, **kwargs) + + monkeypatch.setattr(builtins, "__import__", fake_import) + backend = factory() + with pytest.raises(ModuleNotFoundError) as excinfo: + asyncio.run(backend.start()) + msg = str(excinfo.value) + assert f"'{extra}' extra is not installed" in msg + assert f"uv sync --extra {extra} --inexact" in msg From 0d9410ed8be9890faa0edb96588682b72c80fb76 Mon Sep 17 00:00:00 2001 From: Varun Singh <382354+vr000m@users.noreply.github.com> Date: Sat, 27 Jun 2026 01:47:43 -0700 Subject: [PATCH 10/24] fix: make cross-uid peercred smoke actually reach the gate MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Addresses the Codex adversarial-review finding: the smoke bound its socket under tempfile's default root ($TMPDIR / /var/folders/.../T on macOS), whose per-user ancestors are 0700. A foreign uid fails at path resolution there BEFORE _process_request runs peer_uid, so the cross-uid test could never go green on macOS and a filesystem-boundary failure was indistinguishable from a peer-cred result. - Bind under /tmp (world-traversable, 1777) so the peer can reach the socket. - _assert_chain_traversable(): verify every ancestor up to / is other-traversable before driving the child; fail with a targeted diagnostic otherwise. - Classify the child outcome three ways — rejected (403, OK), accepted (security failure), or inconclusive (path/socket error, never reached the gate) — so a filesystem error can't masquerade as a pass or a peer-cred failure. --- scripts/smoke_peercred.py | 60 +++++++++++++++++++++++++++++++++++---- 1 file changed, 54 insertions(+), 6 deletions(-) diff --git a/scripts/smoke_peercred.py b/scripts/smoke_peercred.py index a29b5ba..d670542 100644 --- a/scripts/smoke_peercred.py +++ b/scripts/smoke_peercred.py @@ -219,6 +219,31 @@ def _have(cmd: str) -> bool: ).returncode == 0 or os.path.exists(f"/usr/bin/{cmd}") +def _assert_chain_traversable(sock: str) -> None: + """Every directory from the socket's parent up to ``/`` must be + other-traversable (mode & 0o001). Otherwise a foreign uid fails at PATH + RESOLUTION before reaching ``_process_request``, and the cross-uid result + would reflect the filesystem boundary rather than the peer-cred gate — the + exact masking Codex flagged. Raise a targeted setup error if the chain is + not traversable (rather than producing a meaningless cross-uid verdict).""" + import stat as _stat + + d = os.path.dirname(os.path.realpath(sock)) + while True: + mode = _stat.S_IMODE(os.stat(d).st_mode) + if not (mode & 0o001): + raise SystemExit( + f"socket ancestor {d} is not other-traversable (mode={oct(mode)}): a " + "foreign uid would fail at path resolution BEFORE the peer-cred gate, " + "so the cross-uid result would be meaningless. Bind the smoke socket " + "under a world-traversable root (e.g. /tmp)." + ) + parent = os.path.dirname(d) + if parent == d: # reached '/' + break + d = parent + + async def _run_cross_uid(sock: str) -> bool: """Drive the example client under a second uid; assert peer-cred rejects. @@ -232,6 +257,10 @@ async def _run_cross_uid(sock: str) -> bool: username, uid = second print(f"\n=== cross-uid rejection (peer uid {uid} / {username}) ===") + # Guard the invariant the whole cross-uid test depends on: the peer must be + # able to traverse to the socket so peer-cred (not the filesystem) is what + # rejects. Fails with a targeted diagnostic if not. + _assert_chain_traversable(sock) # Re-invoke THIS file under the second uid in its connect-as-peer role. cmd = [ "sudo", @@ -251,13 +280,25 @@ async def _run_cross_uid(sock: str) -> bool: if err: print(f" child stderr: {err}") print(f" child stdout: {out}") - if CHILD_REJECTED not in out: + if CHILD_REJECTED in out: + print(" cross-uid connection rejected with 403 'peer not permitted' — peer-cred OK") + return True + if CHILD_ACCEPTED in out: raise SystemExit( - f"cross-uid connect was NOT rejected with 403 '{REJECT_BODY.strip()}' " - f"(rc={proc.returncode}, stdout={out!r}) — peer-cred boundary FAILED" + "cross-uid connection was ACCEPTED — peer-cred did NOT reject a foreign " + f"uid (rc={proc.returncode}, stdout={out!r}). SECURITY FAILURE." ) - print(" cross-uid connection rejected with 403 'peer not permitted' — peer-cred OK") - return True + # CHILD_OTHER / no token: the child never produced a clean 403-or-accept + # verdict — almost always an OS path-resolution or socket-open error, i.e. it + # failed at the FILESYSTEM boundary before reaching peer-cred. That is an + # INCONCLUSIVE setup failure, not proof the gate is broken — keep it distinct + # from both a pass and a real peer-cred failure. + raise SystemExit( + "cross-uid result INCONCLUSIVE: the child did not reach the peer-cred gate " + f"(rc={proc.returncode}, stdout={out!r}; see child stderr above). The connect " + "likely failed at path resolution / socket open (filesystem boundary), not " + "peer-cred. Ensure the socket parent chain is world-traversable (under /tmp)." + ) # --------------------------------------------------------------------------- @@ -278,7 +319,14 @@ async def _running_server(): original_enforce = server_module._enforce_socket_dir_secure server_module._enforce_socket_dir_secure = lambda *a, **k: None - tmpdir = tempfile.mkdtemp(prefix="peercred-smoke-") + # Bind under a WORLD-TRAVERSABLE root (/tmp, mode 1777) — NOT tempfile's + # default ($TMPDIR, which on macOS is a per-user /var/folders/.../T whose + # ancestors are 0700). Under the default root a foreign uid fails at PATH + # RESOLUTION before the peer-cred gate, so the cross-uid result would reflect + # the filesystem boundary instead of peer-cred and could never go green on + # macOS (Codex adversarial-review finding). _assert_chain_traversable() below + # enforces this invariant before the cross-uid child runs. + tmpdir = tempfile.mkdtemp(prefix="peercred-smoke-", dir="/tmp") # Make the parent dir traversable by *other* uids (0711): +x for group/other # lets a foreign uid path-resolve to the socket. R1 would normally refuse # this; the monkeypatch above is why it binds. From f9e8cdf8431eca20309e6ef79b553ec0480a86d1 Mon Sep 17 00:00:00 2001 From: Varun Singh <382354+vr000m@users.noreply.github.com> Date: Sat, 27 Jun 2026 01:57:56 -0700 Subject: [PATCH 11/24] chore(release): prepare 0.3.3 (UDS trust-boundary hardening) Bump version 0.3.2 -> 0.3.3 and cut the CHANGELOG [0.3.3] section (folding in the already-unreleased README-split docs entry). Patch-level: server-side hardening only, no new user-facing feature, no protocol/client change. The [0.3.3] notes flag the one behavior change that can affect existing hosts (a 0755 socket dir / out-of-$HOME socket path now refuses to start). Tag + GitHub release + 'uvx twine upload' happen at merge time, not here. --- CHANGELOG.md | 56 ++++++++++++++++++++++++++++++++++++++++++++++++++ pyproject.toml | 2 +- uv.lock | 2 +- 3 files changed, 58 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 061537a..731e01a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,49 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +## [0.3.3] - 2026-06-27 + +Server-side hardening of the same-host UDS trust boundary. No new user-facing +features; clients connect unchanged (no protocol or client-API change). + +### Added + +- **Server-side UDS peer-credential authentication.** The server rejects any + connection whose kernel-reported peer uid `!= os.geteuid()` with a + pre-handshake HTTP `403` (`peer not permitted`), before the WebSocket + handshake. Uses macOS `getpeereid(2)` (via `ctypes`) / Linux `SO_PEERCRED`; + every resolver failure path fails closed (rejects). UDS only — TCP keeps the + Origin + bearer-token checks. No client change required. +- **Socket-directory ancestor enforcement.** Before bind, the server walks every + directory from the socket's parent up to and including `$HOME` and refuses to + start unless each is owner-owned and not group/other-writable (sticky-bit dirs + excepted), creating missing dirs `0700`. This makes the socket un-plantable + (no foreign uid can `unlink`+`bind` a replacement). +- **Backend-extra onboarding.** `just stt-install` / `stt-enable` now ensure the + selected backend's optional extra is installed (`uv sync --extra + --inexact`) so a freshly installed agent doesn't crash-loop on a missing + import; `PIPECAT_STT_SKIP_DEP_SYNC=1` opts out. +- `scripts/smoke_peercred.py` + `just smoke-peercred`: a local cross-uid / + multi-connection peer-cred smoke (cross-uid leg skips cleanly without a second + local uid). + +### Changed + +- Pinned `websockets` to `>=16,<17` (the `_process_request` handshake/transport + contract depends on the v16 API). +- `scripts/install_stt_agent.sh` creates the socket directory `0700` (was the + install-shell umask default) and self-heals a pre-existing `0755` dir. +- Startup failures — socket-dir enforcement, the `ServerConfig` `ValueError`, + bind `OSError`s, and a missing backend extra — surface as + `stt_server: ` + exit 1 instead of a bare traceback. + +### Security + +- Closes the same-host UDS plant/swap and foreign-uid-connect vectors: the + owner-only ancestor chain is the primary filesystem boundary and peer-cred is + the kernel-authoritative backstop. The bearer token is retained for TCP/remote + (which has neither boundary). See [`docs/operations.md`](docs/operations.md). + ### Documentation - **Split the 593-line README into a focused top page + `docs/`.** The README now @@ -19,6 +62,19 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 and [`docs/migration.md`](docs/migration.md) (0.1.x → 0.2.0 upgrade). A Documentation index links them; all cross-references were repointed. No content was dropped. +- Documented the same-host UDS trust model and the same-uid precondition in + `docs/operations.md`; added a pre-handshake connection-rejection table to + `docs/protocol.md`. + +### Upgrade notes + +- **May require action on existing hosts.** Because the server now refuses to + start against a group/other-writable socket-dir ancestor, an existing `0755` + socket directory — or a custom `STT_WS_SOCKET` / `KODA_STT_SOCKET` pointing + outside `$HOME` — will block startup. Re-run `scripts/install_stt_agent.sh` + (self-heals the dir to `0700`), or `chmod 700` the dir / move the socket under + `$HOME`. Koda hosts run the server from the checkout, so fold this into the + next checkout update; no client pin bump is needed. ## [0.3.2] - 2026-06-08 diff --git a/pyproject.toml b/pyproject.toml index 39bdfdf..da84651 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -1,6 +1,6 @@ [project] name = "pipecat-local-stt-server" -version = "0.3.2" +version = "0.3.3" description = "Standalone local WebSocket transcription (STT) server, client, and pluggable ASR backends for the Pipecat ecosystem" readme = "README.md" requires-python = ">=3.12" diff --git a/uv.lock b/uv.lock index c83cf72..785190a 100644 --- a/uv.lock +++ b/uv.lock @@ -1095,7 +1095,7 @@ wheels = [ [[package]] name = "pipecat-local-stt-server" -version = "0.3.2" +version = "0.3.3" source = { editable = "." } dependencies = [ { name = "websockets" }, From 9b167e3b0ba0c88cbdcf2301a51f666786c69d85 Mon Sep 17 00:00:00 2001 From: Varun Singh <382354+vr000m@users.noreply.github.com> Date: Sat, 27 Jun 2026 15:19:05 -0700 Subject: [PATCH 12/24] test: add stdlib cross-uid peer-cred verifier (nobody, no venv) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Complements 'just smoke-peercred', whose cross-uid leg skips without a second local uid reachable via passwordless sudo. This verifier drives a stdlib-only probe (no websockets/venv) against the same permissive socket as BOTH the owning uid (expect 101) and 'nobody' (expect 403) — so a 101-vs-403 split proves peer-cred, not the filesystem, is the discriminator. 'nobody' can run it because the probe lives in /tmp and uses the system python3 (the repo/venv under a 0750 home is unreadable to other uids). Run: sudo -v && uv run python scripts/verify_peercred_crossuid.py --- scripts/verify_peercred_crossuid.py | 204 ++++++++++++++++++++++++++++ 1 file changed, 204 insertions(+) create mode 100644 scripts/verify_peercred_crossuid.py diff --git a/scripts/verify_peercred_crossuid.py b/scripts/verify_peercred_crossuid.py new file mode 100644 index 0000000..e013a7a --- /dev/null +++ b/scripts/verify_peercred_crossuid.py @@ -0,0 +1,204 @@ +#!/usr/bin/env python3 +"""Local cross-uid peer-cred verification (the leg `just smoke-peercred` skips +without a real second uid). + +Runs an in-process permissive TranscriptionServer (0711 parent under /tmp + +0o666 socket, dir-enforcement monkeypatched off — the same test-only seam the +smoke uses), then drives a STDLIB-ONLY probe against the SAME socket twice: + + * as the owning uid -> expect 101 Switching Protocols (peer-cred allows) + * as `nobody` -> expect 403 'peer not permitted' (peer-cred rejects) + +Same socket, same perms, only the uid differs — so a 101-vs-403 split proves the +peer-credential gate (not the filesystem boundary) is what discriminates. The +probe needs no venv/websockets, so `nobody` (who cannot read this repo under a +0750 home) can still run it from /tmp via the system /usr/bin/python3. + +Run from the repo root: uv run python +The `nobody` leg uses `sudo -u nobody`; sudo will prompt for your password once +(or run `sudo -v` first to pre-cache). +""" + +from __future__ import annotations + +import asyncio +import contextlib +import os +import stat +import sys +import tempfile + +sys.path.insert(0, os.path.dirname(os.path.dirname(os.path.abspath(__file__)))) +# Resolve repo root robustly even when run from scratchpad: rely on `uv run`'s +# project env for the import below. +import stt_server.server as server_module # noqa: E402 +from stt_server.backend import EchoBackend # noqa: E402 + +SYS_PY = "/usr/bin/python3" # world-executable; reachable by nobody + +# Stdlib-only probe: open the UDS, send a minimal WebSocket upgrade, read the +# pre-handshake HTTP status. Written to /tmp (world-readable) at runtime. +PROBE_SRC = r""" +import socket, sys +sock_path = sys.argv[1] +req = ( + "GET / HTTP/1.1\r\n" + "Host: localhost\r\n" + "Upgrade: websocket\r\n" + "Connection: Upgrade\r\n" + "Sec-WebSocket-Key: dGhlIHNhbXBsZSBub25jZQ==\r\n" + "Sec-WebSocket-Version: 13\r\n" + "\r\n" +) +s = socket.socket(socket.AF_UNIX, socket.SOCK_STREAM) +s.settimeout(10) +try: + s.connect(sock_path) +except OSError as e: + # Failed before reaching the gate => filesystem boundary, NOT peer-cred. + print("CONNECT_ERROR:%s:%s" % (type(e).__name__, e)) + sys.exit(3) +data = b"" +try: + s.sendall(req.encode()) + while b"\r\n\r\n" not in data and len(data) < 65536: + chunk = s.recv(4096) + if not chunk: + break + data += chunk + try: + data += s.recv(4096) + except OSError: + pass +except OSError as e: + print("IO_ERROR:%s:%s" % (type(e).__name__, e)) + sys.exit(3) +finally: + s.close() +text = data.decode("latin-1", "replace") +status = text.split("\r\n", 1)[0] if text else "" +print("STATUS:%s" % status) +print("PEER_NOT_PERMITTED:%s" % ("peer not permitted" in text)) +""" + + +def _classify(out: str) -> str: + if "CONNECT_ERROR" in out or "IO_ERROR" in out: + return "INCONCLUSIVE" # never reached the gate (filesystem boundary) + if "STATUS:" in out and " 403 " in out and "PEER_NOT_PERMITTED:True" in out: + return "REJECTED_403" + if "STATUS:" in out and " 101 " in out: + return "ACCEPTED_101" + return "OTHER" + + +async def _run_probe(sock: str, probe_path: str, as_nobody: bool) -> tuple[str, str, int]: + # MUST be async (not subprocess.run): the server runs in THIS process's event + # loop, so a blocking subprocess.run would freeze the loop and the probe's + # connection would never be serviced (it would time out). create_subprocess_exec + # keeps the loop live so the in-process server can respond. stdin is inherited + # so `sudo` can still prompt on the tty when run interactively. + cmd = [SYS_PY, probe_path, sock] + if as_nobody: + cmd = ["sudo", "-u", "nobody", *cmd] + proc = await asyncio.create_subprocess_exec( + *cmd, stdout=asyncio.subprocess.PIPE, stderr=asyncio.subprocess.PIPE + ) + try: + out_b, err_b = await asyncio.wait_for(proc.communicate(), timeout=60) + except asyncio.TimeoutError: + with contextlib.suppress(ProcessLookupError): + proc.kill() + return "", "timeout", 124 + return out_b.decode().strip(), err_b.decode().strip(), proc.returncode or 0 + + +def _assert_chain_traversable(sock: str) -> None: + d = os.path.dirname(os.path.realpath(sock)) + while True: + mode = stat.S_IMODE(os.stat(d).st_mode) + if not (mode & 0o001): + raise SystemExit( + f"ancestor {d} not other-traversable (mode={oct(mode)}); " + "a foreign uid would fail before the peer-cred gate" + ) + parent = os.path.dirname(d) + if parent == d: + break + d = parent + + +async def main() -> int: + if sys.platform != "darwin" and sys.platform != "linux": + print(f"skipped: needs macOS/Linux, not {sys.platform}") + return 0 + if not os.path.exists(SYS_PY): + raise SystemExit(f"{SYS_PY} not found; need a system python3 reachable by nobody") + + original = server_module._enforce_socket_dir_secure + server_module._enforce_socket_dir_secure = lambda *a, **k: None + tmpdir = tempfile.mkdtemp(prefix="peercred-crossuid-", dir="/tmp") + os.chmod(tmpdir, 0o711) + sock = os.path.join(tmpdir, "p.sock") + probe_path = os.path.join(tmpdir, "probe.py") + with open(probe_path, "w") as f: + f.write(PROBE_SRC) + os.chmod(probe_path, 0o644) + + config = server_module.ServerConfig( + socket_path=sock, unix_socket_mode=0o666, reject_browser_origins=False + ) + srv = server_module.TranscriptionServer(EchoBackend(), config) + await srv.start() + try: + _assert_chain_traversable(sock) + print("=== cross-uid peer-cred verification ===") + print(f" socket : {sock} mode={oct(stat.S_IMODE(os.stat(sock).st_mode))}") + print(f" euid : {os.geteuid()}\n") + + self_out, self_err, _ = await _run_probe(sock, probe_path, as_nobody=False) + self_v = _classify(self_out) + print(f" [self uid={os.geteuid()}] {self_v} ({self_out!r})") + if self_err: + print(f" stderr: {self_err}") + + print("\n driving probe as 'nobody' (sudo will prompt if not cached)…") + nob_out, nob_err, nob_rc = await _run_probe(sock, probe_path, as_nobody=True) + # If sudo itself couldn't run the child (no tty / password required / + # not cached), the probe never executed — that's a setup gap, NOT a + # gate result. Detect it (empty probe output but non-zero exit). + sudo_blocked = not nob_out and nob_rc != 0 + nob_v = "SUDO_UNAVAILABLE" if sudo_blocked else _classify(nob_out) + print(f" [nobody] {nob_v} (rc={nob_rc}, out={nob_out!r})") + if nob_err: + print(f" stderr: {nob_err}") + + print("\n=== verdict ===") + if self_v == "ACCEPTED_101" and nob_v == "REJECTED_403": + print(" PASS: same socket+perms — same-uid accepted (101), foreign uid") + print(" rejected (403). Peer-cred is provably the discriminator.") + return 0 + if nob_v == "SUDO_UNAVAILABLE": + print(" NOT RUN: could not launch the child as 'nobody' — sudo needs a") + print(" password/tty here. Run this in your terminal (sudo will") + print(f" prompt), or `sudo -v` first. Same-uid leg verified: {self_v}.") + return 2 + if nob_v == "INCONCLUSIVE": + print(" INCONCLUSIVE: the nobody probe never reached the gate (filesystem") + print(" boundary). Not proof the gate is broken.") + return 2 + print(f" FAIL: self={self_v}, nobody={nob_v} — expected ACCEPTED_101 / REJECTED_403.") + return 1 + finally: + await srv.shutdown() + server_module._enforce_socket_dir_secure = original + with contextlib.suppress(OSError): + os.unlink(sock) + with contextlib.suppress(OSError): + os.unlink(probe_path) + with contextlib.suppress(OSError): + os.rmdir(tmpdir) + + +if __name__ == "__main__": + raise SystemExit(asyncio.run(main())) From 16ccceaf98f1d391b3a1856eb87c13716618910d Mon Sep 17 00:00:00 2001 From: Varun Singh <382354+vr000m@users.noreply.github.com> Date: Sat, 27 Jun 2026 15:51:15 -0700 Subject: [PATCH 13/24] docs: record cross-uid peer-cred verification (real foreign uid) --- .../20260626-feature-uds-trust-boundary-hardening.md | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/docs/dev_plans/20260626-feature-uds-trust-boundary-hardening.md b/docs/dev_plans/20260626-feature-uds-trust-boundary-hardening.md index 61f7f4f..4f71898 100644 --- a/docs/dev_plans/20260626-feature-uds-trust-boundary-hardening.md +++ b/docs/dev_plans/20260626-feature-uds-trust-boundary-hardening.md @@ -537,6 +537,12 @@ two documented non-feature reds above. `ruff check` + `ruff format --check` clea No version/pin bump required — see `docs/operations.md` → "Cross-repo note (Koda)". - Decide the fate of the `test_branch_diff_does_not_touch_koda_surface` guard (leave as documented known-failure, or narrow per the one-liner above). -- The cross-uid `403` smoke leg is unverified in single-uid CI/dev; run - `just smoke-peercred` on a host with a second uid / passwordless `sudo` to - exercise it for real. +- **Cross-uid `403` — VERIFIED (2026-06-27) on real hardware.** Ran + `scripts/verify_peercred_crossuid.py`: against one permissive socket + (`0o666` / `0711` parent under `/tmp`), the owning uid (501) completed the + handshake (`101`) while `nobody` (uid 4294967294) was rejected (`403 peer not + permitted`). Server logged `stt_server: rejecting UDS peer uid 4294967294 + (expected 501)`. Same socket + perms, only the uid differs → peer-cred (not + the filesystem) is provably the discriminator. Closes the adversarial-review + NO-SHIP finding. (`just smoke-peercred`'s cross-uid leg still skips without + passwordless `sudo`; this verifier uses `nobody` + a stdlib probe instead.) From ed7a9c2a83ea4f9d94b2443e05d0d28e03bfa581 Mon Sep 17 00:00:00 2001 From: Varun Singh <382354+vr000m@users.noreply.github.com> Date: Sat, 27 Jun 2026 15:55:32 -0700 Subject: [PATCH 14/24] test: narrow Koda-surface guard to imported surface; repoint per-ASR mirror MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two fixes that get the suite fully green: 1. Narrow test_branch_diff_does_not_touch_koda_surface to the Koda-consumed *imported* surface (stt_server/client.py + stt_server/protocol.py) instead of the whole stt_server/ prefix + install script. Per the cross-repo contract a pin bump is only triggered by the imported client/protocol; the server runtime and install script are run from a HEAD checkout and coordinated at checkout time, not via the pin. This unblocks server-side hardening (which intentionally touches server.py/__main__.py/install_stt_agent.sh). 2. Repoint the per-ASR mirror parser from README.md to docs/operations.md, where the table moved when the README was split (PR #12). This was a pre-existing failure unrelated to the hardening — the test still read README.md after the table relocated. --- tests/test_justfile_recipes.py | 45 ++++++++++++++++++++++------------ 1 file changed, 30 insertions(+), 15 deletions(-) diff --git a/tests/test_justfile_recipes.py b/tests/test_justfile_recipes.py index 187d9b8..9576143 100644 --- a/tests/test_justfile_recipes.py +++ b/tests/test_justfile_recipes.py @@ -34,7 +34,9 @@ REPO_ROOT = Path(__file__).resolve().parent.parent JUSTFILE = REPO_ROOT / "justfile" -README = REPO_ROOT / "README.md" +# The canonical per-ASR socket table moved from README.md to docs/operations.md +# when the README was split (PR #12); the justfile map mirrors it from there. +PER_ASR_DOC = REPO_ROOT / "docs" / "operations.md" FAKE_UID = "501" BACKENDS = ("whisper", "parakeet", "nemotron") @@ -232,13 +234,14 @@ def test_unknown_backend_fails_fast_and_lists_valid(tmp_path): def _readme_map() -> dict[str, tuple[str, str]]: - """Parse the README per-ASR table into {backend: (label, socket)}. + """Parse the canonical per-ASR table (docs/operations.md) into + {backend: (label, socket)}. Anchors on the table header and asserts its column order before indexing - cells by position, so a future column insertion/reorder in the README fails + cells by position, so a future column insertion/reorder in the doc fails loudly here instead of silently mis-mapping label/socket. """ - lines = README.read_text().splitlines() + lines = PER_ASR_DOC.read_text().splitlines() header_idx = next( ( i @@ -247,10 +250,10 @@ def _readme_map() -> dict[str, tuple[str, str]]: ), None, ) - assert header_idx is not None, "README per-ASR table header not found" + assert header_idx is not None, "per-ASR table header not found in docs/operations.md" header = [c.strip() for c in lines[header_idx].split("|")[1:-1]] assert header[:3] == ["ASR", "LaunchAgent label", "Socket"], ( - f"README per-ASR table column order changed: {header}" + f"per-ASR table column order changed: {header}" ) out: dict[str, tuple[str, str]] = {} for ln in lines[header_idx + 2 :]: # skip the header row + `|---|` separator @@ -526,13 +529,26 @@ def test_stt_install_invokes_ensure_extra_then_delegates(tmp_path): # --------------------------------------------------------------------------- # +# The Koda-consumed *imported* surface: the modules Koda pins at a SHA and +# imports as a library. Per the cross-repo contract, a pin bump is required only +# when these change — the client entrypoints (``TranscriptionClient``, +# ``resolve_endpoint_from_env``, ``is_cleartext_remote``) and the wire-protocol +# module (``PROTOCOL_VERSION`` + event constants). The rest of ``stt_server/`` +# (the server runtime, backends, ``__main__``) and ``scripts/install_stt_agent.sh`` +# are the *runtime* Koda runs from a HEAD checkout — server-side changes there are +# coordinated at checkout-update time, NOT via the pinned import, so they are not +# gated here. A ``server.hello`` / ``server.status`` payload change is still +# caught because it must bump ``PROTOCOL_VERSION`` in ``protocol.py``. +KODA_IMPORT_SURFACE = frozenset({"stt_server/client.py", "stt_server/protocol.py"}) + + def test_branch_diff_does_not_touch_koda_surface(): """No-pin-bump invariant (per the cross-repo contract): this work must not - modify the Koda-consumed surface — anything under ``stt_server/`` (the - imported client + the wire protocol) or ``scripts/install_stt_agent.sh``. - Additive files (justfile, these tests, README, dev-plan docs) are fine; an - unrelated docs-only commit does NOT void Koda safety, so this asserts the - *negative* contract rather than an exact file allowlist.""" + modify the Koda-consumed *imported* surface — ``stt_server/client.py`` or + ``stt_server/protocol.py`` — without a coordinated client pin bump. Changes to + the server runtime / install script (run from a HEAD checkout) and additive + files (justfile, these tests, README, dev-plan docs) are fine; this asserts + the *negative* contract rather than an exact file allowlist.""" if not (REPO_ROOT / ".git").exists(): pytest.skip("not a git checkout") merge_base = subprocess.run( @@ -551,9 +567,8 @@ def test_branch_diff_does_not_touch_koda_surface(): cwd=str(REPO_ROOT), ) changed = {ln for ln in diff.stdout.splitlines() if ln.strip()} - forbidden = sorted( - c for c in changed if c.startswith("stt_server/") or c == "scripts/install_stt_agent.sh" - ) + forbidden = sorted(c for c in changed if c in KODA_IMPORT_SURFACE) assert not forbidden, ( - f"branch modifies the Koda-consumed surface (would require a pin bump): {forbidden}" + f"branch modifies the Koda-consumed IMPORTED surface (would require a " + f"coordinated client pin bump): {forbidden}" ) From 2d87e46c03c134f0849dd3353f5bba17142c6da2 Mon Sep 17 00:00:00 2001 From: Varun Singh <382354+vr000m@users.noreply.github.com> Date: Sat, 27 Jun 2026 16:07:30 -0700 Subject: [PATCH 15/24] fix: address code-review findings (peer-cred unsigned uid, ImportError gap, probe parity) - _peercred: unpack SO_PEERCRED as unsigned ('3I'); a uid >= 2**31 was unpacked signed/negative and would false-reject a legit same-uid peer on Linux. - _peercred: cache the macOS getpeereid ctypes binding once per process instead of reloading libc + re-setting argtypes/restype on every UDS connection. - backends: catch ImportError (not just ModuleNotFoundError) so a present-but- broken extra (e.g. nemotron's 'from mlx_audio.stt import load' on version skew, a plain ImportError) surfaces as the actionable message instead of escaping _cmd_serve as a bare traceback crash-loop. - justfile _ensure-extra: probe the exact import backend.start() does ('from mlx_audio.stt import load' for nemotron), not a coarser 'import mlx_audio' that a partially-installed package would pass. - smoke_peercred: use shutil.which instead of exec'ing 'command' (a shell builtin, not a guaranteed binary). --- justfile | 19 +++++++++------- scripts/smoke_peercred.py | 10 ++++----- stt_server/_peercred.py | 36 +++++++++++++++++++++--------- stt_server/backends/mlx_whisper.py | 10 ++++++--- stt_server/backends/nemotron.py | 12 +++++++--- stt_server/backends/parakeet.py | 10 ++++++--- 6 files changed, 64 insertions(+), 33 deletions(-) diff --git a/justfile b/justfile index 2e01335..5b357b7 100644 --- a/justfile +++ b/justfile @@ -193,12 +193,15 @@ _ensure-extra backend: set -uo pipefail backend={{quote(backend)}} # extra == the install backend-name (the 3rd field of `_resolve`); only the - # import-probe name differs from it. Validate before the skip check so an - # unknown backend always errors regardless of PIPECAT_STT_SKIP_DEP_SYNC. + # import probe differs. The probe MUST match what backend.start() imports + # (nemotron does `from mlx_audio.stt import load`, not `import mlx_audio`) — + # a coarser `import mlx_audio` would pass for a partially-installed package + # while the agent still crash-loops. Validate the backend before the skip + # check so an unknown backend always errors regardless of the skip env. case "$backend" in - whisper) extra="mlx"; probe="mlx_whisper" ;; - parakeet) extra="parakeet"; probe="parakeet_mlx" ;; - nemotron) extra="nemotron"; probe="mlx_audio" ;; + whisper) extra="mlx"; probe="import mlx_whisper" ;; + parakeet) extra="parakeet"; probe="import parakeet_mlx" ;; + nemotron) extra="nemotron"; probe="from mlx_audio.stt import load" ;; *) echo "error: unknown backend '$backend' (valid: whisper, parakeet, nemotron)" >&2; exit 1 ;; esac if [[ -n "${PIPECAT_STT_SKIP_DEP_SYNC:-}" ]]; then @@ -206,11 +209,11 @@ _ensure-extra backend: exit 0 fi py="{{justfile_directory()}}/.venv/bin/python" - if [[ -x "$py" ]] && "$py" -c "import $probe" >/dev/null 2>&1; then - echo "_ensure-extra: '$extra' extra already present ($probe importable)" + if [[ -x "$py" ]] && "$py" -c "$probe" >/dev/null 2>&1; then + echo "_ensure-extra: '$extra' extra already present ($probe)" exit 0 fi - echo "_ensure-extra: '$probe' missing — installing the '$extra' extra (uv sync --extra $extra --inexact)…" + echo "_ensure-extra: '$extra' import check failed ($probe) — installing (uv sync --extra $extra --inexact)…" if ! uv sync --extra "$extra" --inexact; then echo "_ensure-extra: 'uv sync --extra $extra --inexact' failed; install the '$extra' extra manually" >&2 exit 1 diff --git a/scripts/smoke_peercred.py b/scripts/smoke_peercred.py index d670542..add3744 100644 --- a/scripts/smoke_peercred.py +++ b/scripts/smoke_peercred.py @@ -42,6 +42,7 @@ import contextlib import os import pwd +import shutil import socket import subprocess import sys @@ -212,11 +213,10 @@ def _find_second_uid() -> tuple[str, int] | None: def _have(cmd: str) -> bool: - return subprocess.run( - ["command", "-v", cmd], - stdout=subprocess.DEVNULL, - stderr=subprocess.DEVNULL, - ).returncode == 0 or os.path.exists(f"/usr/bin/{cmd}") + # shutil.which does a proper PATH lookup; the old `subprocess.run(["command", + # …])` relied on a `command` *binary* existing (it is normally a shell + # builtin) and would raise FileNotFoundError on a host without one. + return shutil.which(cmd) is not None def _assert_chain_traversable(sock: str) -> None: diff --git a/stt_server/_peercred.py b/stt_server/_peercred.py index 860b325..67c27a9 100644 --- a/stt_server/_peercred.py +++ b/stt_server/_peercred.py @@ -16,7 +16,7 @@ Platform notes: - **Linux** uses ``SO_PEERCRED``: ``getsockopt`` returns a ``struct ucred { - pid_t pid; uid_t uid; gid_t gid; }``, unpacked as ``"3i"``. + pid_t pid; uid_t uid; gid_t gid; }``, unpacked as ``"3I"`` (unsigned). - **macOS** has no ``SO_PEERCRED``; we call ``getpeereid(2)`` through ``ctypes``. The wrinkle worth flagging for future maintainers: ``uid_t`` and ``gid_t`` are ``c_uint32`` on Darwin, and we set ``argtypes``/``restype`` on @@ -38,6 +38,7 @@ from __future__ import annotations +import functools import logging import socket import struct @@ -88,29 +89,42 @@ def peer_uid(sock: PeerCredSocket) -> int | None: def _peer_uid_linux(sock: PeerCredSocket) -> int | None: """Resolve the peer uid via ``SO_PEERCRED`` (``struct ucred``).""" - buf = sock.getsockopt(socket.SOL_SOCKET, socket.SO_PEERCRED, struct.calcsize("3i")) - _pid, uid, _gid = struct.unpack("3i", buf) + # "3I": pid_t/uid_t/gid_t are unsigned in ``struct ucred``. Unpacking as + # signed ("3i") would turn a uid >= 2**31 negative, so it would never equal + # os.geteuid() and a legitimate same-uid peer would be wrongly rejected. + buf = sock.getsockopt(socket.SOL_SOCKET, socket.SO_PEERCRED, struct.calcsize("3I")) + _pid, uid, _gid = struct.unpack("3I", buf) return uid -def _peer_uid_darwin(sock: PeerCredSocket) -> int | None: - """Resolve the peer uid via ``getpeereid(2)`` through ``ctypes``. +@functools.lru_cache(maxsize=1) +def _darwin_getpeereid(): + """Load libc and pin the ``getpeereid`` binding ONCE per process. - ``uid_t``/``gid_t`` are ``c_uint32`` on Darwin; ``argtypes``/``restype`` are - set explicitly and libc is loaded with ``use_errno=True`` (see module - docstring for why the binding must be pinned). + ``_process_request`` runs per UDS connection; re-doing ``CDLL(None)`` (a libc + re-dlopen) plus re-resolving the symbol and re-setting ``argtypes``/``restype`` + on every call is wasted work. The configured callable never changes between + calls, so build it once and cache it. ``uid_t``/``gid_t`` are ``c_uint32`` on + Darwin; libc is loaded with ``use_errno=True`` (see module docstring). """ import ctypes libc = ctypes.CDLL(None, use_errno=True) - getpeereid = libc.getpeereid - getpeereid.argtypes = [ + fn = libc.getpeereid + fn.argtypes = [ ctypes.c_int, ctypes.POINTER(ctypes.c_uint32), ctypes.POINTER(ctypes.c_uint32), ] - getpeereid.restype = ctypes.c_int + fn.restype = ctypes.c_int + return fn + + +def _peer_uid_darwin(sock: PeerCredSocket) -> int | None: + """Resolve the peer uid via ``getpeereid(2)`` through ``ctypes``.""" + import ctypes + getpeereid = _darwin_getpeereid() uid = ctypes.c_uint32() gid = ctypes.c_uint32() rc = getpeereid(sock.fileno(), ctypes.byref(uid), ctypes.byref(gid)) diff --git a/stt_server/backends/mlx_whisper.py b/stt_server/backends/mlx_whisper.py index b62f8b7..537ea69 100644 --- a/stt_server/backends/mlx_whisper.py +++ b/stt_server/backends/mlx_whisper.py @@ -302,10 +302,14 @@ async def start(self) -> None: # self-heals it via _ensure-extra. try: import mlx_whisper # type: ignore # noqa: F401 - except ModuleNotFoundError as exc: + except ImportError as exc: + # ImportError (not just ModuleNotFoundError) so a present-but-broken + # mlx_whisper (e.g. its own transitive import fails) also surfaces as + # an actionable message rather than escaping _cmd_serve as a traceback. + missing = getattr(exc, "name", None) or "mlx_whisper" raise ModuleNotFoundError( - f"the 'mlx' extra is not installed (missing module: {exc.name}) " - "— run: uv sync --extra mlx --inexact" + f"the 'mlx' extra is not installed or failed to import " + f"({missing}) — run: uv sync --extra mlx --inexact" ) from exc async def open_stream(self, *, language: str | None = None) -> "_MLXStream": diff --git a/stt_server/backends/nemotron.py b/stt_server/backends/nemotron.py index db77b70..22bbeee 100644 --- a/stt_server/backends/nemotron.py +++ b/stt_server/backends/nemotron.py @@ -264,10 +264,16 @@ async def start(self) -> None: # exit 1, and ``just stt-install nemotron`` self-heals it via _ensure-extra. try: from mlx_audio.stt import load # type: ignore # noqa: F401 - except ModuleNotFoundError as exc: + except ImportError as exc: + # Catch ImportError, not just ModuleNotFoundError: this is a + # ``from mlx_audio.stt import load``, which raises a plain ImportError + # (not a ModuleNotFoundError) when mlx_audio is present but the + # symbol/submodule is missing (version skew) — that would otherwise + # escape _cmd_serve and crash-loop as a bare traceback. + missing = getattr(exc, "name", None) or "mlx_audio" raise ModuleNotFoundError( - f"the 'nemotron' extra is not installed (missing module: {exc.name}) " - "— run: uv sync --extra nemotron --inexact" + f"the 'nemotron' extra is not installed or failed to import " + f"({missing}) — run: uv sync --extra nemotron --inexact" ) from exc async def open_stream(self, *, language: str | None = None) -> "_NemotronStream": diff --git a/stt_server/backends/parakeet.py b/stt_server/backends/parakeet.py index 28caeb1..6644c16 100644 --- a/stt_server/backends/parakeet.py +++ b/stt_server/backends/parakeet.py @@ -249,10 +249,14 @@ async def start(self) -> None: # ``just stt-install parakeet`` self-heals it via _ensure-extra. try: import parakeet_mlx # type: ignore # noqa: F401 - except ModuleNotFoundError as exc: + except ImportError as exc: + # ImportError (not just ModuleNotFoundError) so a present-but-broken + # parakeet_mlx also surfaces as an actionable message rather than + # escaping _cmd_serve as a traceback. + missing = getattr(exc, "name", None) or "parakeet_mlx" raise ModuleNotFoundError( - f"the 'parakeet' extra is not installed (missing module: {exc.name}) " - "— run: uv sync --extra parakeet --inexact" + f"the 'parakeet' extra is not installed or failed to import " + f"({missing}) — run: uv sync --extra parakeet --inexact" ) from exc async def open_stream(self, *, language: str | None = None) -> "_ParakeetStream": From ed5f8274811ae514982ffa84311b7ce6851617b3 Mon Sep 17 00:00:00 2001 From: Varun Singh <382354+vr000m@users.noreply.github.com> Date: Sat, 27 Jun 2026 17:16:21 -0700 Subject: [PATCH 16/24] docs: sync documentation with feature/uds-trust-boundary-hardening changes - dev plan: tick completed checklist + acceptance-criteria boxes - dev_plans/README: cite PR #13 on the UDS row - CHANGELOG: list scripts/verify_peercred_crossuid.py in [0.3.3] Added - README: document stt-install/stt-enable auto-extra-install + PIPECAT_STT_SKIP_DEP_SYNC, add just smoke-peercred to the recipe list - operations.md: backend-dependency onboarding note + local boundary verification (smoke-peercred + cross-uid verifier) --- CHANGELOG.md | 5 ++ README.md | 8 ++- ...26-feature-uds-trust-boundary-hardening.md | 56 +++++++++---------- docs/dev_plans/README.md | 2 +- docs/operations.md | 27 +++++++++ 5 files changed, 68 insertions(+), 30 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 731e01a..74708f9 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -32,6 +32,11 @@ features; clients connect unchanged (no protocol or client-API change). - `scripts/smoke_peercred.py` + `just smoke-peercred`: a local cross-uid / multi-connection peer-cred smoke (cross-uid leg skips cleanly without a second local uid). +- `scripts/verify_peercred_crossuid.py`: a stdlib-only cross-uid verifier that + drives a probe as both the owning uid and `nobody` against one permissive + socket — proving peer-cred (not the filesystem) is the discriminator. Needs no + venv/`websockets`, so it works with `nobody` where the smoke's `sudo` path + can't. ### Changed diff --git a/README.md b/README.md index c4bf498..508a938 100644 --- a/README.md +++ b/README.md @@ -98,14 +98,20 @@ just stt-list # every pipecat.stt-server* agent: state, pid, live b just stt-status nemotron # wire health probe for one backend just stt-disable whisper # stop until next login (keeps the plist) just stt-enable whisper # re-load it from the existing plist -just stt-install parakeet # delegates to install_stt_agent.sh +just stt-install parakeet # delegates to install_stt_agent.sh (+ ensures the extra) just stt-uninstall parakeet +just smoke-peercred # local UDS peer-cred smoke (cross-uid leg needs a 2nd uid) ``` `` is one of `whisper` / `parakeet` / `nemotron`, mapped to the labels and sockets in the [per-ASR table](docs/operations.md#per-asr-socket-convention) (the justfile map is a checked mirror of that table — a test fails CI on drift). +`stt-install` / `stt-enable` also ensure the backend's optional Python extra is +installed (`uv sync --extra --inexact`) so a freshly installed agent +doesn't crash-loop on a missing import; set `PIPECAT_STT_SKIP_DEP_SYNC=1` to +manage the extras yourself. + `stt-list` prints each agent's `socket:` line in the same `~`-form a consumer's config uses for its endpoint (e.g. onoats' `config.toml` `[stt] ws_socket`), so you can match a config line to a running agent directly. Note whisper's socket is diff --git a/docs/dev_plans/20260626-feature-uds-trust-boundary-hardening.md b/docs/dev_plans/20260626-feature-uds-trust-boundary-hardening.md index 4f71898..cdc1651 100644 --- a/docs/dev_plans/20260626-feature-uds-trust-boundary-hardening.md +++ b/docs/dev_plans/20260626-feature-uds-trust-boundary-hardening.md @@ -117,7 +117,7 @@ still not a client code change. This precondition is written into Requirements. **Test files:** `tests/test_stt_server.py` **Test command:** `uv run pytest tests/test_stt_server.py -k "parent_dir or socket_dir or 0700 or owner" -q` -- [ ] Add a private helper (e.g. `_enforce_socket_dir_secure(path: Path, +- [x] Add a private helper (e.g. `_enforce_socket_dir_secure(path: Path, trusted_root: Path)`) that creates missing socket directories `0700` (`mkdir(mode=0o700)`, then re-`stat` and verify — `mkdir` mode is umask-masked, so verify rather than trust), then walks every component from the socket's bind @@ -125,11 +125,11 @@ still not a client code change. This precondition is written into Requirements. `st_uid == os.geteuid()` and no group/other write bits (`st_mode & 0o022 == 0`); sticky-bit directories are allowed. Raise a clear exception naming the failing component and condition otherwise. -- [ ] Call it in `start()` immediately before the `os.umask(0o077)` block +- [x] Call it in `start()` immediately before the `os.umask(0o077)` block (replacing the bare `socket_path.parent.mkdir(parents=True, exist_ok=True)` at `server.py:148-149`). Resolve and document the trusted root, then verify the full ancestor walk rather than only the immediate parent of the socket. -- [ ] **Failure surface — wrap the serve path, not the status probe.** Raise a +- [x] **Failure surface — wrap the serve path, not the status probe.** Raise a `ValueError`/dedicated exception from the helper. The serve entrypoint `_cmd_serve` (`__main__.py:210-224`) runs `asyncio.run(serve(...))` with **no** try/except today, so the exception would propagate as a bare traceback (the @@ -138,7 +138,7 @@ still not a client code change. This precondition is written into Requirements. print(f"stt_server: {exc}", file=sys.stderr); raise SystemExit(1)` around the serve call. This also fixes the latent unguarded `ServerConfig.__post_init__` `ValueError` on the serve path. Confirm no stack-trace-only failure. -- [ ] **Co-requisite: install script must create the dir `0700` (lands with this +- [x] **Co-requisite: install script must create the dir `0700` (lands with this phase).** `scripts/install_stt_agent.sh:100` does `mkdir -p "$(dirname "$SOCKET_PATH")"` at the install shell umask (commonly `0755`); after this phase the server would *refuse to start* against that existing `0755` dir. Change to @@ -152,7 +152,7 @@ still not a client code change. This precondition is written into Requirements. **Test files:** `tests/test_peercred.py` (new) **Test command:** `uv run pytest tests/test_peercred.py -q` -- [ ] New module `stt_server/_peercred.py` exposing +- [x] New module `stt_server/_peercred.py` exposing `peer_uid(sock: PeerCredSocket) -> int | None`, where `PeerCredSocket` is a minimal structural `Protocol` containing only the members used by the resolver (`family`, `fileno()`, `getsockopt()`), rather than `socket.socket` directly. @@ -165,7 +165,7 @@ still not a client code change. This precondition is written into Requirements. explicitly on the libc function and load libc with `use_errno=True` for safety, portability, and correct errno propagation; return uid on success. - Unknown platform / call failure: return `None` (caller fails closed). -- [ ] Keep this module import-light and side-effect-free so it is unit-testable +- [x] Keep this module import-light and side-effect-free so it is unit-testable without binding a server (mirror the existing single `sys.platform == "darwin"` precedent at `server.py:75-77`; no new abstraction framework). @@ -175,7 +175,7 @@ still not a client code change. This precondition is written into Requirements. **Test files:** `tests/test_stt_server.py` **Test command:** `uv run pytest tests/test_stt_server.py -k "peercred or peer_uid or uds_auth" -q` -- [ ] In `_process_request` (`server.py:261`), gate on UDS only +- [x] In `_process_request` (`server.py:261`), gate on UDS only (`self._config.socket_path is not None`). Obtain the raw socket via `connection.transport.get_extra_info("socket")`. **Verified:** `connection.transport` is set before `_process_request` runs (confirmed in the @@ -187,16 +187,16 @@ still not a client code change. This precondition is written into Requirements. `_pending_write_bytes`, a *post-handshake* call site, so it is not precedent for the handshake-time return). Assert `sock is not None and sock.family == AF_UNIX` in the implementation. -- [ ] **Fail-closed guard (do this before calling the resolver):** if the raw +- [x] **Fail-closed guard (do this before calling the resolver):** if the raw socket is `None`, return `connection.respond(403, "peer not permitted\n")` and warn — do NOT call `peer_uid(None)` (it would raise `AttributeError` on `.getsockopt`/`.fileno`, an uncaught exception, not a guaranteed reject). -- [ ] Wrap `peer_uid(sock)` in `try/except Exception`; if it raises, log the +- [x] Wrap `peer_uid(sock)` in `try/except Exception`; if it raises, log the exception and return `connection.respond(403, "peer not permitted\n")`. If it returns `None` or `!= os.geteuid()`, return the same `403`. Order it before/independent of the bearer-token branch so UDS rejects foreign uids regardless of token. -- [ ] Log a single warning on each fail-closed path (resolver `None`, resolver +- [x] Log a single warning on each fail-closed path (resolver `None`, resolver exception, or missing socket) so an unsupported platform / unexpected transport is loud. @@ -224,7 +224,7 @@ normal `TranscriptionServer` construction (for example, a local subclass or monkeypatch of `_enforce_socket_dir_secure`). Do not add a `ServerConfig` field or equivalent public/API-reachable bypass flag. -- [ ] **Cross-uid rejection (local-only, the real test).** Build +- [x] **Cross-uid rejection (local-only, the real test).** Build `TranscriptionServer`/`ServerConfig` **directly** (the public `serve()` does not expose `unix_socket_mode` — `server.py:900` — so the smoke cannot use it) with `unix_socket_mode=0o666`, a test-only replacement for the dir-enforcement @@ -234,16 +234,16 @@ or equivalent public/API-reachable bypass flag. `websockets.exceptions.InvalidStatus` and check `status_code == 403` and the `"peer not permitted\n"` body (it is a **pre-handshake HTTP response, not a protocol JSON envelope** — `docs/protocol.md` documents no reject envelope). -- [ ] **Same-uid multi-connection (also CI-safe).** Open N concurrent +- [x] **Same-uid multi-connection (also CI-safe).** Open N concurrent `TranscriptionClient` sessions as the owning uid, assert all complete the handshake and stream — a regression guard that peer-cred did not break the normal path under concurrency. Add one assertion that the resolved peer uid equals `os.geteuid()` via the **real** resolver (not a stub), so a silently- `None` transport is caught rather than masked. Mirror this as a pytest case. -- [ ] Gate the cross-uid path on availability of a second uid / `sudo` and +- [x] Gate the cross-uid path on availability of a second uid / `sudo` and `sys.platform`; print a clear "skipped: needs a second local uid" rather than failing when run unprivileged. `just smoke-peercred` wraps invocation. -- [ ] For the **accept** path, assert `server.hello` fields against the +- [x] For the **accept** path, assert `server.hello` fields against the `server.py:287-307` source of truth (protocol.md lists event *names*, not the field schema). If protocol.md is to be the field oracle, Phase 5 must add the `server.hello` field table to it first. @@ -255,16 +255,16 @@ or equivalent public/API-reachable bypass flag. **Test files:** n/a **Test command:** `uv run ruff check && uv run ruff format --check` -- [ ] Document the same-host UDS trust model: the owner-only ancestor chain is the +- [x] Document the same-host UDS trust model: the owner-only ancestor chain is the primary filesystem boundary; peer-cred is the kernel-authoritative defense-in-depth backstop; bearer token retained for TCP only. -- [ ] Note macOS `getpeereid`-via-`ctypes` wrinkle for future maintainers. -- [ ] If the accept-path test is to assert against `docs/protocol.md` rather than +- [x] Note macOS `getpeereid`-via-`ctypes` wrinkle for future maintainers. +- [x] If the accept-path test is to assert against `docs/protocol.md` rather than `server.py`, add the `server.hello` field table (`protocol_version`, `capabilities`, `audio`, `backend`) to `docs/protocol.md` so it becomes a real field oracle. Otherwise document that protocol.md pins event presence, not field shape, and the reject is a pre-handshake HTTP `403` (no JSON envelope). -- [ ] Update `docs/dev_plans/README.md` row status on completion. +- [x] Update `docs/dev_plans/README.md` row status on completion. --- @@ -405,34 +405,34 @@ sequenceDiagram ## Acceptance Criteria -- [ ] Server refuses to start when any non-sticky ancestor from the socket bind +- [x] Server refuses to start when any non-sticky ancestor from the socket bind directory through the trusted root is not owner-owned or is group/other-writable, with an actionable `stt_server: ` error + `SystemExit(1)` (not a bare traceback); creates missing socket directories `0700` when absent. A test where a grandparent dir is owned by a different uid rejects. `install_stt_agent.sh` creates the socket dir `0700` so fresh installs/upgrades do not break. -- [ ] UDS connections from a foreign uid are rejected with `403` before the +- [x] UDS connections from a foreign uid are rejected with `403` before the handshake; same-uid connections succeed unchanged. Every fail-closed path (resolver `None`, resolver exception such as `OSError`, missing transport socket, unknown platform) rejects. -- [ ] Local `just smoke-peercred` demonstrates a real cross-uid `403` (asserted +- [x] Local `just smoke-peercred` demonstrates a real cross-uid `403` (asserted via `InvalidStatus.status_code` + `"peer not permitted\n"` body) against a server whose `0o666` socket mode **and** `0711` parent dir both permit the peer, using a test-only helper replacement that cannot be triggered through `serve()` or normal `TranscriptionServer` construction — so peer-cred is provably what rejects. N concurrent same-uid sessions all succeed. -- [ ] `peer_uid()` resolves on macOS (ctypes `getpeereid`) and Linux +- [x] `peer_uid()` resolves on macOS (ctypes `getpeereid`) and Linux (`SO_PEERCRED`); the ctypes binding sets `argtypes`, `restype`, and `use_errno=True`; branch selection is covered on any host; unknown platforms fail closed. -- [ ] `websockets` is pinned to `>=16,<17` and the lockfile reflects the pin. -- [ ] No client-side change required; existing client connects unmodified. -- [ ] TCP path and bearer-token behavior unchanged. -- [ ] Any test-only dir-enforcement seam is implemented by subclassing or +- [x] `websockets` is pinned to `>=16,<17` and the lockfile reflects the pin. +- [x] No client-side change required; existing client connects unmodified. +- [x] TCP path and bearer-token behavior unchanged. +- [x] Any test-only dir-enforcement seam is implemented by subclassing or monkeypatching the helper, not by a `ServerConfig` field or other public/API- reachable flag. -- [ ] `uv run pytest -q` green; `ruff check` + `ruff format` clean. -- [ ] Docs describe the same-host trust model and the same-uid precondition. +- [x] `uv run pytest -q` green; `ruff check` + `ruff format` clean. +- [x] Docs describe the same-host trust model and the same-uid precondition. ## Review Focus diff --git a/docs/dev_plans/README.md b/docs/dev_plans/README.md index 3b45dd4..c5de077 100644 --- a/docs/dev_plans/README.md +++ b/docs/dev_plans/README.md @@ -9,7 +9,7 @@ update the row in the same change that updates the plan's `**Status**` line. | [Nemotron 3.5 ASR backend (0.3.0)](20260605-nemotron-asr-backend.md) | ASR Backends | ✅ Shipped — PR #7 merged 2026-06-06; packaged as `nemotron` extra in 0.3.2 | | [justfile operator layer for STT LaunchAgents](20260607-feature-stt-agents-justfile.md) | Install & Packaging | ✅ Complete | | [STT vocabulary/prompt biasing on the wire protocol](20260607-feature-stt-prompt-biasing.md) | Wire Protocol / ASR Backends | ⬜ Not started — analysis/handoff only | -| [UDS server-side trust-boundary hardening](20260626-feature-uds-trust-boundary-hardening.md) | Server Transport | 🟦 Implemented on `feature/uds-trust-boundary-hardening` (phases 1–5) — pending Koda checkout coordination + merge | +| [UDS server-side trust-boundary hardening](20260626-feature-uds-trust-boundary-hardening.md) | Server Transport | 🟦 Implemented on `feature/uds-trust-boundary-hardening` (phases 1–5), PR #13 — pending Koda checkout coordination + merge | ## Conventions diff --git a/docs/operations.md b/docs/operations.md index da05085..ce0784e 100644 --- a/docs/operations.md +++ b/docs/operations.md @@ -28,6 +28,16 @@ client-side configuration. ### Two-agent install +> **Backend dependencies.** A backend imports its ASR library lazily at startup, +> and a bare `uv run` / `uv sync` prunes the optional extras, so an installed +> agent can crash-loop on `ModuleNotFoundError`. `just stt-install ` / +> `just stt-enable ` ensure the matching extra is present +> (`uv sync --extra --inexact`, additive — it won't prune other +> backends); set `PIPECAT_STT_SKIP_DEP_SYNC=1` to manage extras yourself. If an +> agent does crash on a missing import, the server now exits with an actionable +> `stt_server: the '' extra is not installed … run: uv sync --extra +> --inexact` instead of a bare traceback. + `scripts/install_stt_agent.sh` is parameterised by `PIPECAT_STT_LABEL` / `PIPECAT_STT_SOCKET` / `PIPECAT_STT_BACKEND` (the legacy `KODA_STT_*` names are still honoured as deprecated aliases) so two LaunchAgents can coexist @@ -232,6 +242,23 @@ must have explicit `argtypes`/`restype` set (a wrong width or signature can fail `socketpair()` unit test asserting `peer_uid() == os.geteuid()` is the gate that catches a width/signature regression — keep it. +### Verifying the boundary locally + +Two local checks exercise what single-uid CI cannot: + +- `just smoke-peercred` (`scripts/smoke_peercred.py`) — opens N concurrent + same-uid sessions (regression guard) and, when a second local uid is reachable + via passwordless `sudo`, drives a foreign-uid connection that must be rejected + `403`. The cross-uid leg skips cleanly when no second uid is available. +- `scripts/verify_peercred_crossuid.py` — a stdlib-only verifier (no venv / + `websockets`) that drives a probe as **both** the owning uid (expect `101`) and + `nobody` (expect `403`) against one permissive socket. Same socket + perms, + only the uid differs, so a `101`-vs-`403` split proves peer-cred — not the + filesystem — is the discriminator. Run with `sudo -v && uv run python + scripts/verify_peercred_crossuid.py`. It binds under `/tmp` (world-traversable) + and asserts every ancestor is traversable, so a filesystem-boundary failure is + reported as inconclusive rather than masquerading as a peer-cred result. + ## Whisper hallucination suppression (MLX backend) The MLX Whisper backend forwards four decode-time knobs to From ffd921f88a04752007a2eb081b7a99ec64247b97 Mon Sep 17 00:00:00 2001 From: Varun Singh <382354+vr000m@users.noreply.github.com> Date: Sat, 27 Jun 2026 17:56:33 -0700 Subject: [PATCH 17/24] fix(server): bind UDS on the verified resolved socket path _enforce_socket_dir_secure now returns resolved_bind / path.name and start() creates, binds, and chmods on that resolved path instead of the literal str(socket_path). The owner/mode ancestor walk verifies the resolved chain, so binding the unresolved path left a symlinked ancestor outside the verified chain repointable in the stat->bind window. Default cache path has no symlink components, so resolved == literal there. Also guard connection.transport is None in the UDS peer-cred gate so a future/edge None fails closed (403) rather than raising AttributeError. Updates the test-only enforcement seam to mirror the new bind-path return contract (returns the socket path, not None). --- stt_server/server.py | 34 ++++++++++++++++++++++++++------ tests/test_mlx_teardown_spike.py | 10 +++++----- tests/test_stt_server.py | 25 ++++++++++++----------- 3 files changed, 46 insertions(+), 23 deletions(-) diff --git a/stt_server/server.py b/stt_server/server.py index f62465f..75c93bd 100644 --- a/stt_server/server.py +++ b/stt_server/server.py @@ -87,7 +87,7 @@ def _item_id() -> str: return f"item_{uuid.uuid4().hex[:16]}" -def _enforce_socket_dir_secure(path: Path, trusted_root: Path) -> None: +def _enforce_socket_dir_secure(path: Path, trusted_root: Path) -> Path: """Refuse to bind unless the socket's directory chain cannot be hijacked. The UDS file mode (``0o600``) stops a foreign uid from ``connect()``-ing, @@ -110,6 +110,13 @@ def _enforce_socket_dir_secure(path: Path, trusted_root: Path) -> None: Raises ``ValueError`` on any failure; the serve entrypoint turns that into ``stt_server: `` + ``SystemExit(1)`` rather than a bare traceback. + + Returns the fully symlink-resolved socket path (``resolved_bind / + path.name``). The caller MUST bind on this returned path, not the literal + ``path``: the owner/mode walk verifies ``resolved_bind``, so binding the + unresolved path would let a symlinked ancestor outside the verified chain be + repointed in the stat->bind window. For the default cache path (no symlink + components) the resolved and literal paths are identical. """ euid = os.geteuid() bind_dir = path.parent @@ -134,7 +141,7 @@ def _enforce_socket_dir_secure(path: Path, trusted_root: Path) -> None: # itself umask-masked, but ``0o700`` carries no group/other bits for any # umask to widen, and the walk below re-stats and verifies regardless. to_create: list[Path] = [] - cursor = bind_dir + cursor = resolved_bind while not cursor.exists(): to_create.append(cursor) parent = cursor.parent @@ -164,6 +171,10 @@ def _enforce_socket_dir_secure(path: Path, trusted_root: Path) -> None: break component = component.parent + # Bind on the verified resolved path, not the literal ``path``: this is the + # chain the walk above vouched for. + return resolved_bind / path.name + @dataclass class ServerConfig: @@ -235,14 +246,18 @@ async def start(self) -> None: # be planted/swapped by another local uid. This replaces a bare # mkdir that trusted the path blindly and verifies the full ancestor # walk, not just the immediate parent. - _enforce_socket_dir_secure(socket_path, Path.home()) + # Bind on the verified resolved path the enforcement returned, not + # the literal configured path: the owner/mode walk vouched for the + # resolved chain, so binding the unresolved path could let a + # symlinked ancestor be repointed in the stat->bind window. + resolved_socket = _enforce_socket_dir_secure(socket_path, Path.home()) # Restrict the socket file to owner-only before bind so the UDS # trust boundary actually holds on multi-user hosts. prior_umask = os.umask(0o077) try: self._server = await ws_unix_serve( self._handle_connection, - path=str(socket_path), + path=str(resolved_socket), max_size=self._config.max_append_bytes, process_request=self._process_request, ) @@ -250,7 +265,7 @@ async def start(self) -> None: os.umask(prior_umask) if self._config.unix_socket_mode is not None: try: - os.chmod(socket_path, self._config.unix_socket_mode) + os.chmod(resolved_socket, self._config.unix_socket_mode) except OSError as exc: logger.warning("stt_server: chmod on UDS failed: %s", exc) else: @@ -355,7 +370,14 @@ async def _process_request(self, connection, request): # peer's uid is a 403, not a pass. TCP listeners cannot answer # SO_PEERCRED-style queries, so this gate is UDS-only. if self._config.socket_path is not None: - sock = connection.transport.get_extra_info("socket") + transport = connection.transport + if transport is None: + # websockets 16 sets ``transport`` before ``process_request`` + # runs; guard defensively so a future/edge None fails closed + # rather than raising AttributeError. + logger.warning("stt_server: UDS connection has no transport; rejecting") + return connection.respond(403, "peer not permitted\n") + sock = transport.get_extra_info("socket") if sock is None: logger.warning("stt_server: UDS connection has no underlying socket; rejecting") return connection.respond(403, "peer not permitted\n") diff --git a/tests/test_mlx_teardown_spike.py b/tests/test_mlx_teardown_spike.py index 679ad49..75aa737 100644 --- a/tests/test_mlx_teardown_spike.py +++ b/tests/test_mlx_teardown_spike.py @@ -165,7 +165,7 @@ async def test_shutdown_drains_two_concurrent_decodes(monkeypatch): # rejects. This test exercises the drain path, not ancestor-dir enforcement, # and the temp dir already exists before start(), so neutralise the check # with a pure no-op via the Phase-4-sanctioned seam. - monkeypatch.setattr("stt_server.server._enforce_socket_dir_secure", lambda *a, **k: None) + monkeypatch.setattr("stt_server.server._enforce_socket_dir_secure", lambda *a, **k: a[0]) backend = _SlowBackend(decode_seconds=0.5) srv, sock = await _start_server(backend, drain=2.0) try: @@ -207,7 +207,7 @@ async def test_shutdown_force_cancels_past_drain_timeout(monkeypatch): # /tmp is root-owned so R1 dir-enforcement rejects it; this test exercises # the force-cancel drain path, not ancestor-dir enforcement, and the temp # dir already exists before start(), so neutralise with a no-op (Phase-4 seam). - monkeypatch.setattr("stt_server.server._enforce_socket_dir_secure", lambda *a, **k: None) + monkeypatch.setattr("stt_server.server._enforce_socket_dir_secure", lambda *a, **k: a[0]) backend = _HangingBackend() srv, sock = await _start_server(backend, drain=0.5) try: @@ -240,7 +240,7 @@ async def test_shutdown_is_idempotent_under_double_call(monkeypatch): # /tmp is root-owned so R1 dir-enforcement rejects it; this test exercises # shutdown idempotency, not ancestor-dir enforcement, and the temp dir # already exists before start(), so neutralise with a no-op (Phase-4 seam). - monkeypatch.setattr("stt_server.server._enforce_socket_dir_secure", lambda *a, **k: None) + monkeypatch.setattr("stt_server.server._enforce_socket_dir_secure", lambda *a, **k: a[0]) backend = _SlowBackend(decode_seconds=0.1) srv, sock = await _start_server(backend, drain=2.0) try: @@ -270,7 +270,7 @@ async def test_fresh_server_after_shutdown_accepts_new_connections(monkeypatch): # rejects. This test exercises respawn-on-same-socket, not ancestor-dir # enforcement, and the temp dir already exists before start(), so neutralise # with a no-op via the Phase-4-sanctioned seam. - monkeypatch.setattr("stt_server.server._enforce_socket_dir_secure", lambda *a, **k: None) + monkeypatch.setattr("stt_server.server._enforce_socket_dir_secure", lambda *a, **k: a[0]) tmp = tempfile.mkdtemp(prefix="mlx-spike.", dir="/tmp") sock = Path(tmp) / "s" @@ -310,7 +310,7 @@ async def test_backend_close_called_exactly_once_on_force_cancel_path(monkeypatc # the force-cancel close-once invariant, not ancestor-dir enforcement, and # the temp dir already exists before start(), so neutralise with a no-op # via the Phase-4-sanctioned seam. - monkeypatch.setattr("stt_server.server._enforce_socket_dir_secure", lambda *a, **k: None) + monkeypatch.setattr("stt_server.server._enforce_socket_dir_secure", lambda *a, **k: a[0]) backend = _HangingBackend() srv, sock = await _start_server(backend, drain=0.2) try: diff --git a/tests/test_stt_server.py b/tests/test_stt_server.py index 527b2ae..71f7338 100644 --- a/tests/test_stt_server.py +++ b/tests/test_stt_server.py @@ -519,7 +519,7 @@ async def test_unix_socket_has_owner_only_permissions(monkeypatch): # Binds under /tmp (root-owned), which R1 dir-enforcement rejects. This # test exercises UDS perms, not ancestor-dir enforcement, so neutralise the # check via the Phase-4-sanctioned seam. - monkeypatch.setattr("stt_server.server._enforce_socket_dir_secure", lambda *a, **k: None) + monkeypatch.setattr("stt_server.server._enforce_socket_dir_secure", lambda *a, **k: a[0]) with tempfile.TemporaryDirectory(prefix="stt.", dir="/tmp") as d: sock = Path(d) / "s" srv = TranscriptionServer(EchoBackend(), ServerConfig(socket_path=str(sock))) @@ -537,10 +537,11 @@ async def test_unix_socket_start_creates_parent_directory(monkeypatch): # This test exercises parent-dir *creation*, so the stub keeps the mkdir # behaviour but drops the ancestor ownership/perms check (the part /tmp # trips). Sanctioned by the Phase-4 test-only seam. - monkeypatch.setattr( - "stt_server.server._enforce_socket_dir_secure", - lambda path, trusted_root: path.parent.mkdir(mode=0o700, parents=True, exist_ok=True), - ) + def _mkdir_only(path, trusted_root): + path.parent.mkdir(mode=0o700, parents=True, exist_ok=True) + return path # mirror the real helper's bind-path return contract + + monkeypatch.setattr("stt_server.server._enforce_socket_dir_secure", _mkdir_only) with tempfile.TemporaryDirectory(prefix="stt.", dir="/tmp") as d: sock = Path(d) / "nested" / "path" / "s" srv = TranscriptionServer(EchoBackend(), ServerConfig(socket_path=str(sock))) @@ -557,7 +558,7 @@ async def test_unix_socket_transport(monkeypatch): # /tmp is root-owned so R1 dir-enforcement rejects it; this test exercises # UDS transport, not ancestor-dir enforcement, so neutralise the check via # the Phase-4-sanctioned seam. - monkeypatch.setattr("stt_server.server._enforce_socket_dir_secure", lambda *a, **k: None) + monkeypatch.setattr("stt_server.server._enforce_socket_dir_secure", lambda *a, **k: a[0]) with tempfile.TemporaryDirectory(prefix="stt.", dir="/tmp") as d: sock = Path(d) / "s" srv = TranscriptionServer( @@ -626,7 +627,7 @@ async def test_uds_without_token_does_not_warn(caplog, monkeypatch): # Binds under a system temp dir (root-owned ancestor) which R1 rejects; # this test exercises the UDS-vs-TCP token warning, not ancestor-dir # enforcement, so neutralise the check via the Phase-4-sanctioned seam. - monkeypatch.setattr("stt_server.server._enforce_socket_dir_secure", lambda *a, **k: None) + monkeypatch.setattr("stt_server.server._enforce_socket_dir_secure", lambda *a, **k: a[0]) with tempfile.TemporaryDirectory() as tmp: sock = str(Path(tmp) / "stt.sock") srv = TranscriptionServer( @@ -837,7 +838,7 @@ async def test_cli_status_with_explicit_socket_reads_token_from_dotenv(tmp_path: # Binds the in-process server's socket directly under /tmp (root-owned), # which R1 dir-enforcement rejects; this test exercises the CLI token-probe # path, not ancestor-dir enforcement, so neutralise via the Phase-4 seam. - monkeypatch.setattr("stt_server.server._enforce_socket_dir_secure", lambda *a, **k: None) + monkeypatch.setattr("stt_server.server._enforce_socket_dir_secure", lambda *a, **k: a[0]) sock = Path("/tmp") / f"stt-preflight-{os.getpid()}.sock" sock.unlink(missing_ok=True) srv = TranscriptionServer( @@ -1378,7 +1379,7 @@ async def test_cli_status_client_does_not_use_server_only_token(tmp_path: Path, # which R1 dir-enforcement rejects; this test exercises the client-vs-server # token contract, not ancestor-dir enforcement, so neutralise via the # Phase-4-sanctioned seam. - monkeypatch.setattr("stt_server.server._enforce_socket_dir_secure", lambda *a, **k: None) + monkeypatch.setattr("stt_server.server._enforce_socket_dir_secure", lambda *a, **k: a[0]) sock = Path("/tmp") / f"stt-preflight-p1-{os.getpid()}.sock" sock.unlink(missing_ok=True) srv = TranscriptionServer( @@ -1664,7 +1665,7 @@ async def test_uds_auth_foreign_uid_rejected_with_403(monkeypatch): uid, and assert the client connect raises InvalidStatus(403). Mirrors the existing 401 bearer-auth test's InvalidStatus handling. """ - monkeypatch.setattr("stt_server.server._enforce_socket_dir_secure", lambda *a, **k: None) + monkeypatch.setattr("stt_server.server._enforce_socket_dir_secure", lambda *a, **k: a[0]) # Foreign uid: resolver claims the peer is someone other than us. monkeypatch.setattr("stt_server.server.peer_uid", lambda sock: os.geteuid() + 1) with tempfile.TemporaryDirectory(prefix="stt.", dir="/tmp") as d: @@ -1732,7 +1733,7 @@ async def test_uds_auth_peer_uid_same_uid_real_resolver_completes_handshake(monk catches a silently-None transport socket that would otherwise fail closed. Kept to a single connection; the N-concurrent case is Phase 4. """ - monkeypatch.setattr("stt_server.server._enforce_socket_dir_secure", lambda *a, **k: None) + monkeypatch.setattr("stt_server.server._enforce_socket_dir_secure", lambda *a, **k: a[0]) with tempfile.TemporaryDirectory(prefix="stt.", dir="/tmp") as d: sock = Path(d) / "s" srv = TranscriptionServer(EchoBackend(), ServerConfig(socket_path=str(sock))) @@ -1786,7 +1787,7 @@ async def test_uds_multi_connection_same_uid_all_handshake(monkeypatch): # Bind one real UDS server; do NOT stub peer_uid so the real gate runs for # every connection. - monkeypatch.setattr("stt_server.server._enforce_socket_dir_secure", lambda *a, **k: None) + monkeypatch.setattr("stt_server.server._enforce_socket_dir_secure", lambda *a, **k: a[0]) n = 4 with tempfile.TemporaryDirectory(prefix="stt.", dir="/tmp") as d: sock = Path(d) / "s" From 62def0cb3e0d381454d390aa0a42ec63a8e23907 Mon Sep 17 00:00:00 2001 From: Varun Singh <382354+vr000m@users.noreply.github.com> Date: Sat, 27 Jun 2026 17:56:44 -0700 Subject: [PATCH 18/24] fix(cli): broaden serve startup handler to ImportError _cmd_serve caught ModuleNotFoundError, so a partially-present backend extra whose import fails on a missing sub-symbol (plain ImportError, not ModuleNotFoundError) escaped as a bare traceback. ImportError is the superset, so the actionable 'stt_server: ' + exit 1 now covers it. --- stt_server/__main__.py | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/stt_server/__main__.py b/stt_server/__main__.py index baa70ac..4c048c1 100644 --- a/stt_server/__main__.py +++ b/stt_server/__main__.py @@ -223,11 +223,13 @@ def _cmd_serve(args: argparse.Namespace) -> None: auth_token=_resolve_auth_token(args.auth_token_file), ) ) - except (ValueError, OSError, ModuleNotFoundError) as exc: + except (ValueError, OSError, ImportError) as exc: # Surface startup failures (socket-dir enforcement refusing to bind, # the ServerConfig.__post_init__ ValueError, bind OSErrors, and a # missing backend extra re-raised by backend.start()) as an actionable - # one-line message + exit 1 rather than a bare traceback. + # one-line message + exit 1 rather than a bare traceback. ``ImportError`` + # (superset of ``ModuleNotFoundError``) also catches a partially-present + # extra whose import fails on a missing sub-symbol. print(f"stt_server: {exc}", file=sys.stderr) raise SystemExit(1) From f24121f53f85ad12790735bfb25dfbb758d9d58f Mon Sep 17 00:00:00 2001 From: Varun Singh <382354+vr000m@users.noreply.github.com> Date: Sat, 27 Jun 2026 17:56:44 -0700 Subject: [PATCH 19/24] test(peercred): mirror unsigned 3I SO_PEERCRED format in mock The Linux resolver reads struct ucred as unsigned (3I) so a uid >= 2**31 stays positive; the mock asserted/packed signed 3i. Both are 12 bytes so the test passed, but it no longer faithfully mirrored the implementation it guards. Match 3I exactly. --- tests/test_peercred.py | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/tests/test_peercred.py b/tests/test_peercred.py index fdec118..8066608 100644 --- a/tests/test_peercred.py +++ b/tests/test_peercred.py @@ -69,9 +69,11 @@ def fileno(self): return -1 def getsockopt(self, level, optname, buflen): - # SO_PEERCRED returns struct ucred { pid_t; uid_t; gid_t } == "3i". - assert buflen == struct.calcsize("3i") - return struct.pack("3i", pid, uid, gid) + # SO_PEERCRED returns struct ucred { pid_t; uid_t; gid_t }. The + # implementation reads it as unsigned ("3I") so a uid >= 2**31 stays + # positive; mirror that exact format here rather than signed "3i". + assert buflen == struct.calcsize("3I") + return struct.pack("3I", pid, uid, gid) assert peer_uid(FakeSocket()) == uid From 8f13cca9f67184105a2f0f552ecc6025745e2952 Mon Sep 17 00:00:00 2001 From: Varun Singh <382354+vr000m@users.noreply.github.com> Date: Sat, 27 Jun 2026 17:56:52 -0700 Subject: [PATCH 20/24] refactor(scripts): extract shared assert_chain_traversable smoke_peercred.py and verify_peercred_crossuid.py each carried a near-identical traversability check that would diverge silently. Move the canonical version to scripts/_smoke_lib.py (pure stdlib) and import it in both. Also updates each script's test-only enforcement seam to the new bind-path return contract (returns the socket path, not None). --- scripts/_smoke_lib.py | 43 +++++++++++++++++++++++++++++ scripts/smoke_peercred.py | 34 ++++------------------- scripts/verify_peercred_crossuid.py | 22 ++++----------- 3 files changed, 54 insertions(+), 45 deletions(-) create mode 100644 scripts/_smoke_lib.py diff --git a/scripts/_smoke_lib.py b/scripts/_smoke_lib.py new file mode 100644 index 0000000..663af33 --- /dev/null +++ b/scripts/_smoke_lib.py @@ -0,0 +1,43 @@ +"""Shared helpers for the peer-cred verification scripts. + +``smoke_peercred.py`` and ``verify_peercred_crossuid.py`` both drive a cross-uid +probe against a deliberately permissive socket to prove that *peer-cred* (not the +filesystem) is what rejects a foreign uid. That assertion is only meaningful if +the foreign uid can actually traverse the path to the socket — otherwise it fails +at PATH RESOLUTION before reaching ``_process_request`` and the verdict reflects +the filesystem boundary, not the peer-cred gate. This module holds the single +canonical traversability check both scripts use so they cannot diverge. + +Pure stdlib (``os``/``stat``) so it imports under both the venv driver and any +system-python context. +""" + +from __future__ import annotations + +import os +import stat + + +def assert_chain_traversable(sock: str) -> None: + """Require every directory from the socket's parent up to ``/`` to be + other-traversable (``mode & 0o001``). + + Raises ``SystemExit`` with a targeted diagnostic if any ancestor is not + other-traversable: a foreign uid would fail at path resolution BEFORE the + peer-cred gate, so the cross-uid result would be meaningless rather than a + real peer-cred verdict. + """ + d = os.path.dirname(os.path.realpath(sock)) + while True: + mode = stat.S_IMODE(os.stat(d).st_mode) + if not (mode & 0o001): + raise SystemExit( + f"socket ancestor {d} is not other-traversable (mode={oct(mode)}): a " + "foreign uid would fail at path resolution BEFORE the peer-cred gate, " + "so the cross-uid result would be meaningless. Bind the socket under a " + "world-traversable root (e.g. /tmp)." + ) + parent = os.path.dirname(d) + if parent == d: # reached '/' + break + d = parent diff --git a/scripts/smoke_peercred.py b/scripts/smoke_peercred.py index add3744..c173875 100644 --- a/scripts/smoke_peercred.py +++ b/scripts/smoke_peercred.py @@ -51,9 +51,12 @@ # Run from the repo root so ``stt_server`` imports resolve (also required when # this file is re-invoked under ``sudo -u`` for the foreign-uid connect role). sys.path.insert(0, os.path.dirname(os.path.dirname(os.path.abspath(__file__)))) +# The scripts dir itself, so the shared ``_smoke_lib`` helper resolves. +sys.path.insert(0, os.path.dirname(os.path.abspath(__file__))) import websockets.exceptions # noqa: E402 +from _smoke_lib import assert_chain_traversable # noqa: E402 from stt_server import _peercred # noqa: E402 from stt_server.backend import EchoBackend # noqa: E402 from stt_server.client import TranscriptionClient # noqa: E402 @@ -219,31 +222,6 @@ def _have(cmd: str) -> bool: return shutil.which(cmd) is not None -def _assert_chain_traversable(sock: str) -> None: - """Every directory from the socket's parent up to ``/`` must be - other-traversable (mode & 0o001). Otherwise a foreign uid fails at PATH - RESOLUTION before reaching ``_process_request``, and the cross-uid result - would reflect the filesystem boundary rather than the peer-cred gate — the - exact masking Codex flagged. Raise a targeted setup error if the chain is - not traversable (rather than producing a meaningless cross-uid verdict).""" - import stat as _stat - - d = os.path.dirname(os.path.realpath(sock)) - while True: - mode = _stat.S_IMODE(os.stat(d).st_mode) - if not (mode & 0o001): - raise SystemExit( - f"socket ancestor {d} is not other-traversable (mode={oct(mode)}): a " - "foreign uid would fail at path resolution BEFORE the peer-cred gate, " - "so the cross-uid result would be meaningless. Bind the smoke socket " - "under a world-traversable root (e.g. /tmp)." - ) - parent = os.path.dirname(d) - if parent == d: # reached '/' - break - d = parent - - async def _run_cross_uid(sock: str) -> bool: """Drive the example client under a second uid; assert peer-cred rejects. @@ -260,7 +238,7 @@ async def _run_cross_uid(sock: str) -> bool: # Guard the invariant the whole cross-uid test depends on: the peer must be # able to traverse to the socket so peer-cred (not the filesystem) is what # rejects. Fails with a targeted diagnostic if not. - _assert_chain_traversable(sock) + assert_chain_traversable(sock) # Re-invoke THIS file under the second uid in its connect-as-peer role. cmd = [ "sudo", @@ -317,14 +295,14 @@ async def _running_server(): # TEST-ONLY seam: defeat R1's ancestor-chain enforcement so a 0711/0o666 # socket can be bound. NOT reachable from serve() or public construction. original_enforce = server_module._enforce_socket_dir_secure - server_module._enforce_socket_dir_secure = lambda *a, **k: None + server_module._enforce_socket_dir_secure = lambda *a, **k: a[0] # Bind under a WORLD-TRAVERSABLE root (/tmp, mode 1777) — NOT tempfile's # default ($TMPDIR, which on macOS is a per-user /var/folders/.../T whose # ancestors are 0700). Under the default root a foreign uid fails at PATH # RESOLUTION before the peer-cred gate, so the cross-uid result would reflect # the filesystem boundary instead of peer-cred and could never go green on - # macOS (Codex adversarial-review finding). _assert_chain_traversable() below + # macOS (Codex adversarial-review finding). assert_chain_traversable() below # enforces this invariant before the cross-uid child runs. tmpdir = tempfile.mkdtemp(prefix="peercred-smoke-", dir="/tmp") # Make the parent dir traversable by *other* uids (0711): +x for group/other diff --git a/scripts/verify_peercred_crossuid.py b/scripts/verify_peercred_crossuid.py index e013a7a..4fde268 100644 --- a/scripts/verify_peercred_crossuid.py +++ b/scripts/verify_peercred_crossuid.py @@ -29,9 +29,12 @@ import tempfile sys.path.insert(0, os.path.dirname(os.path.dirname(os.path.abspath(__file__)))) +# The scripts dir itself, so the shared ``_smoke_lib`` helper resolves. +sys.path.insert(0, os.path.dirname(os.path.abspath(__file__))) # Resolve repo root robustly even when run from scratchpad: rely on `uv run`'s # project env for the import below. import stt_server.server as server_module # noqa: E402 +from _smoke_lib import assert_chain_traversable # noqa: E402 from stt_server.backend import EchoBackend # noqa: E402 SYS_PY = "/usr/bin/python3" # world-executable; reachable by nobody @@ -113,21 +116,6 @@ async def _run_probe(sock: str, probe_path: str, as_nobody: bool) -> tuple[str, return out_b.decode().strip(), err_b.decode().strip(), proc.returncode or 0 -def _assert_chain_traversable(sock: str) -> None: - d = os.path.dirname(os.path.realpath(sock)) - while True: - mode = stat.S_IMODE(os.stat(d).st_mode) - if not (mode & 0o001): - raise SystemExit( - f"ancestor {d} not other-traversable (mode={oct(mode)}); " - "a foreign uid would fail before the peer-cred gate" - ) - parent = os.path.dirname(d) - if parent == d: - break - d = parent - - async def main() -> int: if sys.platform != "darwin" and sys.platform != "linux": print(f"skipped: needs macOS/Linux, not {sys.platform}") @@ -136,7 +124,7 @@ async def main() -> int: raise SystemExit(f"{SYS_PY} not found; need a system python3 reachable by nobody") original = server_module._enforce_socket_dir_secure - server_module._enforce_socket_dir_secure = lambda *a, **k: None + server_module._enforce_socket_dir_secure = lambda *a, **k: a[0] tmpdir = tempfile.mkdtemp(prefix="peercred-crossuid-", dir="/tmp") os.chmod(tmpdir, 0o711) sock = os.path.join(tmpdir, "p.sock") @@ -151,7 +139,7 @@ async def main() -> int: srv = server_module.TranscriptionServer(EchoBackend(), config) await srv.start() try: - _assert_chain_traversable(sock) + assert_chain_traversable(sock) print("=== cross-uid peer-cred verification ===") print(f" socket : {sock} mode={oct(stat.S_IMODE(os.stat(sock).st_mode))}") print(f" euid : {os.geteuid()}\n") From cd66ad9641085b9377175eca44dc3bb81aaa9a1b Mon Sep 17 00:00:00 2001 From: Varun Singh <382354+vr000m@users.noreply.github.com> Date: Sat, 27 Jun 2026 17:56:52 -0700 Subject: [PATCH 21/24] docs(changelog): note websockets>=16 consumer-conflict caveat The floor moved >=13 -> >=16,<17 across every extra (client included), so an environment resolving an older websockets hits a dependency conflict. Wire protocol and stt_server.client API are unchanged, so it stays a patch release; document the caveat in Upgrade notes. --- CHANGELOG.md | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 74708f9..e1efe5f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -40,8 +40,9 @@ features; clients connect unchanged (no protocol or client-API change). ### Changed -- Pinned `websockets` to `>=16,<17` (the `_process_request` handshake/transport - contract depends on the v16 API). +- Pinned `websockets` to `>=16,<17` (was `>=13`; the `_process_request` + handshake/transport contract depends on the v16 API). Applies to every extra, + including `client`. See Upgrade notes for the consumer-conflict caveat. - `scripts/install_stt_agent.sh` creates the socket directory `0700` (was the install-shell umask default) and self-heals a pre-existing `0755` dir. - Startup failures — socket-dir enforcement, the `ServerConfig` `ValueError`, @@ -80,6 +81,13 @@ features; clients connect unchanged (no protocol or client-API change). (self-heals the dir to `0700`), or `chmod 700` the dir / move the socket under `$HOME`. Koda hosts run the server from the checkout, so fold this into the next checkout update; no client pin bump is needed. +- **`websockets>=16` may conflict for library consumers.** The floor moved from + `>=13` to `>=16,<17` across every extra (`client` included), so an environment + that resolves an older `websockets` (or another package capping it `<16`) will + hit a dependency conflict when installing this version. The wire protocol and + `stt_server.client` API are unchanged — only the dependency floor moved — so + this is a packaging bump, not a behavioural one, and stays a patch release + (`0.3.3`). ## [0.3.2] - 2026-06-08 From a50624c0736b444ce8f7c1e7baf78e0cfb7caff6 Mon Sep 17 00:00:00 2001 From: Varun Singh <382354+vr000m@users.noreply.github.com> Date: Sun, 28 Jun 2026 00:38:56 -0700 Subject: [PATCH 22/24] fix(server): verify literal socket-dir chain, reject symlink components Adversarial-review no-ship: the prior fix resolved path.parent and bound the resolved target, but clients and the LaunchAgent still connect via the literal configured path. That split the verified chain from the client-visible one: a symlink in a group/other-writable lexical ancestor of the literal path was skipped by the resolve()-d walk, so a foreign uid who can write that ancestor could repoint the symlink after startup and hijack the path clients connect to while the server stayed bound to a safe resolved target. _enforce_socket_dir_secure now verifies the LITERAL lexical chain clients traverse (no resolve()), rejects any symlink component via os.lstat, and requires an absolute, ..-free path. It returns and the server binds the literal verified path, so verified == bound == client-visible. Default cache path (no symlink components) is unchanged. Adds a regression test: $HOME/writable (group-writable) / link -> $HOME/safe, socket under link -> startup refuses naming the symlink component. Updates the foreign-owner test to patch os.lstat (recursion-safe lexical match). --- CHANGELOG.md | 7 ++-- docs/operations.md | 7 +++- stt_server/server.py | 73 +++++++++++++++++++++++++++------------- tests/test_stt_server.py | 52 +++++++++++++++++++++++++--- 4 files changed, 107 insertions(+), 32 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index e1efe5f..5936de0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -23,8 +23,11 @@ features; clients connect unchanged (no protocol or client-API change). - **Socket-directory ancestor enforcement.** Before bind, the server walks every directory from the socket's parent up to and including `$HOME` and refuses to start unless each is owner-owned and not group/other-writable (sticky-bit dirs - excepted), creating missing dirs `0700`. This makes the socket un-plantable - (no foreign uid can `unlink`+`bind` a replacement). + excepted), creating missing dirs `0700`. The walk is over the **literal** path + clients traverse and **rejects any symlink component** (a symlinked, writable + lexical ancestor would otherwise be repointable post-startup to hijack the + client-visible socket). This makes the socket un-plantable (no foreign uid can + `unlink`+`bind` a replacement). - **Backend-extra onboarding.** `just stt-install` / `stt-enable` now ensure the selected backend's optional extra is installed (`uv sync --extra --inexact`) so a freshly installed agent doesn't crash-loop on a missing diff --git a/docs/operations.md b/docs/operations.md index ce0784e..8307a25 100644 --- a/docs/operations.md +++ b/docs/operations.md @@ -162,7 +162,12 @@ independent measures. They are layered, not redundant: the server walks every directory from the socket's parent up to and including the trusted root (`$HOME`) and **refuses to start** unless each component is owned by the running uid and is not group/other-writable - (sticky-bit directories excepted). This makes the socket *un-plantable*: an + (sticky-bit directories excepted). The walk is over the **literal** socket + path clients traverse — not a symlink-resolved one — and **rejects any + symlink component**: resolving past a symlink would verify the target's chain + while a foreign uid who can write a symlinked-but-writable lexical ancestor + could repoint it after startup and hijack the path clients connect to. This + makes the socket *un-plantable*: an attacker cannot `unlink()` our socket and `bind()` their own, because they have no write access on any directory that could replace it. On stock macOS the chain (`~/Library/Caches/pipecat-stt` → `~/Library/Caches` → `~/Library` diff --git a/stt_server/server.py b/stt_server/server.py index 75c93bd..84b965b 100644 --- a/stt_server/server.py +++ b/stt_server/server.py @@ -27,6 +27,7 @@ import resource import signal import socket +import stat as _stat import sys import time import uuid @@ -111,26 +112,42 @@ def _enforce_socket_dir_secure(path: Path, trusted_root: Path) -> Path: Raises ``ValueError`` on any failure; the serve entrypoint turns that into ``stt_server: `` + ``SystemExit(1)`` rather than a bare traceback. - Returns the fully symlink-resolved socket path (``resolved_bind / - path.name``). The caller MUST bind on this returned path, not the literal - ``path``: the owner/mode walk verifies ``resolved_bind``, so binding the - unresolved path would let a symlinked ancestor outside the verified chain be - repointed in the stat->bind window. For the default cache path (no symlink - components) the resolved and literal paths are identical. + Returns the verified socket path (the literal ``path``). We verify the + LITERAL lexical chain that clients and the LaunchAgent actually traverse — + NOT a ``resolve()``-d one — and reject any symlink component (``lstat``). + The kernel resolves the literal socket path at connect/bind time, so the + directories that could replace the socket are the literal lexical ancestors. + Resolving here would verify a *different* chain than clients use: a symlinked + but group/other-writable lexical ancestor would be skipped, then repointed + after startup to make clients connect to an attacker's socket while the + server stayed bound to the safe resolved target. By rejecting symlinks the + verified chain, the bound path, and the client-visible path are identical. """ euid = os.geteuid() bind_dir = path.parent - trusted_root = trusted_root.resolve() - resolved_bind = bind_dir.resolve() + + # Verify (and bind) the literal path, so we never resolve past a symlink the + # client will still traverse. An absolute, ``..``-free path is required: with + # no ``resolve()`` the containment check below is purely lexical, and a + # ``..`` segment could let a path that escapes the trusted root pass it. + if not bind_dir.is_absolute(): + raise ValueError( + f"socket directory {bind_dir} must be an absolute path under {trusted_root}" + ) + if ".." in bind_dir.parts: + raise ValueError( + f"socket directory {bind_dir} must not contain '..' components; " + f"point the socket at a direct path under {trusted_root}" + ) # The socket MUST live under the trusted root. A path outside it (e.g. a # custom socket path pointing at a world-writable location) has no # owner-only chain we can vouch for, and walking to the filesystem root # would only verify root-owned system directories that we do not own. Refuse # rather than pretend the path is safe. - if resolved_bind != trusted_root and trusted_root not in resolved_bind.parents: + if bind_dir != trusted_root and trusted_root not in bind_dir.parents: raise ValueError( - f"socket directory {resolved_bind} is not under the trusted root " + f"socket directory {bind_dir} is not under the trusted root " f"{trusted_root}; point the socket at a path under {trusted_root}" ) @@ -141,7 +158,7 @@ def _enforce_socket_dir_secure(path: Path, trusted_root: Path) -> Path: # itself umask-masked, but ``0o700`` carries no group/other bits for any # umask to widen, and the walk below re-stats and verifies regardless. to_create: list[Path] = [] - cursor = resolved_bind + cursor = bind_dir while not cursor.exists(): to_create.append(cursor) parent = cursor.parent @@ -151,9 +168,18 @@ def _enforce_socket_dir_secure(path: Path, trusted_root: Path) -> Path: for missing in reversed(to_create): missing.mkdir(mode=0o700) - component = resolved_bind + component = bind_dir while True: - st = os.stat(component) + # ``lstat`` (not ``stat``) so a symlinked component is detected, not + # silently followed. As we walk up, every component eventually becomes + # the final path element, so a symlink anywhere in the chain is caught. + st = os.lstat(component) + if _stat.S_ISLNK(st.st_mode): + raise ValueError( + f"socket directory component {component} is a symlink; refusing " + "to bind (a symlinked ancestor can be repointed by another user " + "to hijack the client-visible socket path)" + ) if st.st_uid != euid: raise ValueError( f"socket directory component {component} is owned by uid " @@ -171,9 +197,10 @@ def _enforce_socket_dir_secure(path: Path, trusted_root: Path) -> Path: break component = component.parent - # Bind on the verified resolved path, not the literal ``path``: this is the - # chain the walk above vouched for. - return resolved_bind / path.name + # Bind on the literal, now-verified path: the chain clients traverse is + # exactly the chain we vouched for (no symlinks), so verified == bound == + # client-visible. + return path @dataclass @@ -245,19 +272,17 @@ async def start(self) -> None: # directory up to and including home BEFORE bind so the socket cannot # be planted/swapped by another local uid. This replaces a bare # mkdir that trusted the path blindly and verifies the full ancestor - # walk, not just the immediate parent. - # Bind on the verified resolved path the enforcement returned, not - # the literal configured path: the owner/mode walk vouched for the - # resolved chain, so binding the unresolved path could let a - # symlinked ancestor be repointed in the stat->bind window. - resolved_socket = _enforce_socket_dir_secure(socket_path, Path.home()) + # walk, not just the immediate parent. The enforcement rejects any + # symlink component and returns the literal verified path, so the + # chain we vouched for is exactly the one clients traverse. + verified_socket = _enforce_socket_dir_secure(socket_path, Path.home()) # Restrict the socket file to owner-only before bind so the UDS # trust boundary actually holds on multi-user hosts. prior_umask = os.umask(0o077) try: self._server = await ws_unix_serve( self._handle_connection, - path=str(resolved_socket), + path=str(verified_socket), max_size=self._config.max_append_bytes, process_request=self._process_request, ) @@ -265,7 +290,7 @@ async def start(self) -> None: os.umask(prior_umask) if self._config.unix_socket_mode is not None: try: - os.chmod(resolved_socket, self._config.unix_socket_mode) + os.chmod(verified_socket, self._config.unix_socket_mode) except OSError as exc: logger.warning("stt_server: chmod on UDS failed: %s", exc) else: diff --git a/tests/test_stt_server.py b/tests/test_stt_server.py index 71f7338..33edde2 100644 --- a/tests/test_stt_server.py +++ b/tests/test_stt_server.py @@ -1559,16 +1559,21 @@ def test_enforce_socket_dir_foreign_owner_ancestor_rejects_without_chmod_chown(m bind_dir.mkdir(mode=0o700) sock = bind_dir / "s" - real_stat = os.stat + # Capture the real lstat BEFORE patching: the walk reads ownership via + # os.lstat, and computing the match via os.path.realpath would call the + # (patched) os.lstat and recurse. The chain has no symlinks, so a lexical + # compare identifies `child` unambiguously. + real_lstat = os.lstat foreign_uid = os.geteuid() + 1 + child_str = os.path.normpath(str(child)) def fake_stat(path, *args, **kwargs): - st = real_stat(path, *args, **kwargs) try: - target = os.path.realpath(os.fspath(path)) + p = os.path.normpath(os.fspath(path)) except TypeError: - return st # fd-based stat: leave untouched - if target == os.path.realpath(child): + return real_lstat(path) # fd-based: leave untouched + st = real_lstat(path) + if p == child_str: fields = list(st) # the 10 canonical stat fields fields[4] = foreign_uid # st_uid return os.stat_result(fields) @@ -1576,7 +1581,11 @@ def fake_stat(path, *args, **kwargs): chmod_calls: list = [] chown_calls: list = [] + # The ancestor walk reads ownership via ``os.lstat`` (so symlink + # components are detected, not followed); patch it as well as ``os.stat`` + # so the injected foreign uid is observed. monkeypatch.setattr(os, "stat", fake_stat) + monkeypatch.setattr(os, "lstat", fake_stat) monkeypatch.setattr(os, "chmod", lambda *a, **k: chmod_calls.append(a)) monkeypatch.setattr(os, "chown", lambda *a, **k: chown_calls.append(a)) @@ -1591,6 +1600,39 @@ def fake_stat(path, *args, **kwargs): shutil.rmtree(root, ignore_errors=True) +def test_enforce_socket_dir_refuses_symlinked_socket_dir_under_writable_ancestor(): + # (e) Adversarial: the client-visible socket path runs through a symlink that + # lives in a group/other-writable lexical ancestor. Resolving the path would + # verify the symlink TARGET's chain (safe) and skip the writable ancestor, + # letting a foreign uid repoint the symlink after startup and hijack the + # path clients connect to. The helper must verify the LITERAL chain and + # refuse to bind on any symlink component. + import shutil + + from stt_server.server import _enforce_socket_dir_secure + + root = _make_trusted_root() # stands in for $HOME (0700, owner-owned) + try: + safe = root / "safe" + safe.mkdir(mode=0o700) # a legitimate 0700 target + writable = root / "writable" + writable.mkdir(mode=0o700) + os.chmod(writable, 0o775) # group-writable: a foreign uid can write here + link = writable / "link" + link.symlink_to(safe) # link -> root/safe, repointable by anyone with + # write on `writable` + sock = link / "s" # client-visible path traverses the symlink + + with pytest.raises((ValueError, OSError)) as exc: + _enforce_socket_dir_secure(sock, root) + + msg = str(exc.value) + assert str(link) in msg, f"error must name the symlink component; got: {msg!r}" + assert "symlink" in msg.lower(), f"error must explain the symlink refusal; got: {msg!r}" + finally: + shutil.rmtree(root, ignore_errors=True) + + # --------------------------------------------------------------------------- # Phase 3 — UDS peer-credential gate in _process_request (CI seam) # From f9f41b6f18bc0b625e781137fc4b4798b682aa93 Mon Sep 17 00:00:00 2001 From: Varun Singh <382354+vr000m@users.noreply.github.com> Date: Sun, 28 Jun 2026 01:14:38 -0700 Subject: [PATCH 23/24] fix(install): validate socket path before mutating the filesystem MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adversarial-review medium: render_plist() mkdir'd + chmod 700'd the socket parent before the server's path rules got a say. A custom PIPECAT_STT_SOCKET outside $HOME, or with a symlinked parent, would have its directory tightened to owner-only and THEN fail at startup when _enforce_socket_dir_secure rejects it — an availability/permissions regression, not a clean fail-closed refusal. Add validate_socket_path() mirroring the server's rules (absolute, ..-free, under $HOME, no symlink component) and call it first in render_plist(), before any mkdir/chmod. A rejected path now exits 1 with an actionable error and zero filesystem mutation. Tests: outside-$HOME and symlinked-parent custom sockets both refuse with exit 1 and leave the operator's directory permissions untouched. --- CHANGELOG.md | 7 +++- scripts/install_stt_agent.sh | 59 ++++++++++++++++++++++++++ tests/test_install_migration.py | 74 +++++++++++++++++++++++++++++++++ 3 files changed, 139 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 5936de0..42f35e3 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -47,7 +47,12 @@ features; clients connect unchanged (no protocol or client-API change). handshake/transport contract depends on the v16 API). Applies to every extra, including `client`. See Upgrade notes for the consumer-conflict caveat. - `scripts/install_stt_agent.sh` creates the socket directory `0700` (was the - install-shell umask default) and self-heals a pre-existing `0755` dir. + install-shell umask default) and self-heals a pre-existing `0755` dir. It now + validates a custom `PIPECAT_STT_SOCKET` against the same rules the server + enforces (absolute, under `$HOME`, no symlink component) **before** any + `mkdir`/`chmod`, so a path the server would reject fails the install cleanly + with no filesystem mutation instead of tightening the directory and then + crash-looping the agent. - Startup failures — socket-dir enforcement, the `ServerConfig` `ValueError`, bind `OSError`s, and a missing backend extra — surface as `stt_server: ` + exit 1 instead of a bare traceback. diff --git a/scripts/install_stt_agent.sh b/scripts/install_stt_agent.sh index 65c9157..7c0efc1 100755 --- a/scripts/install_stt_agent.sh +++ b/scripts/install_stt_agent.sh @@ -96,7 +96,66 @@ fi cmd="${1:-install}" +# Validate the literal socket path against the SAME rules the server enforces in +# _enforce_socket_dir_secure() (stt_server/server.py) BEFORE creating or +# chmod-ing anything. The server refuses to start unless the socket directory is +# an absolute, ``..``-free path under $HOME whose chain contains no symlink +# component. Mirroring that here means a custom PIPECAT_STT_SOCKET the server +# would reject fails the install cleanly — with NO filesystem mutation — instead +# of tightening the operator's directory to 0700 and only then crash-looping the +# agent on startup. Keep this in lockstep with the Python rules. +validate_socket_path() { + local sock="$1" + local dir + dir="$(dirname "$sock")" + local home="${HOME%/}" + + # Absolute path required (lexical containment below assumes it). + case "$dir" in + /*) : ;; + *) + echo "error: socket directory '$dir' must be an absolute path under \$HOME ($home)" >&2 + exit 1 + ;; + esac + + # No ``..`` components: without resolving symlinks, a ``..`` segment could + # escape the trusted root while still passing the lexical check below. + case "/$dir/" in + */../*) + echo "error: socket directory '$dir' must not contain '..' components" >&2 + exit 1 + ;; + esac + + # Must live lexically under the trusted root ($HOME), or be $HOME itself. + if [[ "$dir" != "$home" && "$dir" != "$home/"* ]]; then + echo "error: socket directory '$dir' is not under the trusted root \$HOME ($home); point PIPECAT_STT_SOCKET (or KODA_STT_SOCKET) at a path under \$HOME" >&2 + exit 1 + fi + + # Reject any symlink component from the socket directory up to AND INCLUDING + # $HOME. Only existing components can be symlinks; not-yet-created ones are + # made 0700 below and cannot be. Walking up means a symlink anywhere in the + # chain (not just the immediate parent) is caught before any mkdir/chmod. + local component="$dir" + while :; do + if [[ -L "$component" ]]; then + echo "error: socket directory component '$component' is a symlink; refusing to install (a symlinked ancestor can be repointed by another user to hijack the socket path)" >&2 + exit 1 + fi + [[ "$component" == "$home" ]] && break + local parent + parent="$(dirname "$component")" + [[ "$parent" == "$component" ]] && break + component="$parent" + done +} + render_plist() { + # Validate (and refuse) BEFORE mutating any filesystem state — the socket + # dir chmod below must never tighten a directory the server will then reject. + validate_socket_path "$SOCKET_PATH" mkdir -p "$LOG_DIR" "$(dirname "$PLIST_DST")" # Create the socket's parent directory owner-only (0700) from birth so # there is no window under a permissive umask (commonly 0755) in which diff --git a/tests/test_install_migration.py b/tests/test_install_migration.py index d62ed0f..02da38f 100644 --- a/tests/test_install_migration.py +++ b/tests/test_install_migration.py @@ -456,6 +456,80 @@ def test_nemotron_backend_install_uses_nemotron_default_model(tmp_path: Path): ) +# --------------------------------------------------------------------------- +# (7) Socket-path validation runs BEFORE any filesystem mutation. A custom +# PIPECAT_STT_SOCKET the server would reject (outside $HOME, or with a +# symlinked parent) must fail the install cleanly with NO mkdir/chmod — +# never tighten the operator's directory to 0700 and then crash on startup. +# --------------------------------------------------------------------------- + + +def test_install_refuses_socket_outside_home_without_mutating(tmp_path: Path): + """A socket path outside $HOME is rejected before any filesystem change: + exit 1, an actionable 'not under the trusted root' error, and the operator's + pre-existing directory keeps its original (loose) permissions.""" + stub_dir, _ = _make_stub_dir(tmp_path) + + # An operator-owned dir OUTSIDE the hermetic HOME, deliberately 0755. + outside = tmp_path / "outside" + outside.mkdir() + os.chmod(outside, 0o755) + socket = outside / "stt.sock" + + r = _run_install( + tmp_path, + stub_dir, + env_overrides={"PIPECAT_STT_SOCKET": str(socket)}, + ) + + assert r.returncode == 1, f"expected exit 1; stdout={r.stdout!r} stderr={r.stderr!r}" + assert "not under the trusted root" in r.stderr, ( + f"expected an actionable under-$HOME error; stderr={r.stderr!r}" + ) + # The load-bearing assertion: the install must NOT have chmod'd the + # operator's directory before refusing. + mode = stat.S_IMODE((tmp_path / "outside").stat().st_mode) + assert mode == 0o755, f"install tightened an unsupported dir to {oct(mode)}; must not mutate" + # The default log/cache dirs must not have been created either (validation + # runs first). + assert not (tmp_path / "home" / "Library" / "Logs" / "pipecat-stt").exists(), ( + "install must not create the log dir before validating the socket path" + ) + + +def test_install_refuses_symlinked_socket_parent_without_mutating(tmp_path: Path): + """A socket whose parent is a symlink is rejected before any filesystem + change: exit 1, a 'symlink' error, and the symlink's target keeps its + original permissions (no chmod 700 fired through the link).""" + stub_dir, _ = _make_stub_dir(tmp_path) + home = tmp_path / "home" + (home / "Library" / "LaunchAgents").mkdir(parents=True, exist_ok=True) + + # A real dir under $HOME, deliberately 0755, and a symlink pointing at it. + realdir = home / "realdir" + realdir.mkdir() + os.chmod(realdir, 0o755) + link = home / "link" + link.symlink_to(realdir) + socket = link / "stt.sock" # client-visible path traverses the symlink + + r = _run_install( + tmp_path, + stub_dir, + env_overrides={"PIPECAT_STT_SOCKET": str(socket)}, + ) + + assert r.returncode == 1, f"expected exit 1; stdout={r.stdout!r} stderr={r.stderr!r}" + assert "symlink" in r.stderr.lower(), ( + f"expected an actionable symlink-refusal error; stderr={r.stderr!r}" + ) + # The symlink and its target must be untouched — the chmod 700 must not have + # fired through the link onto the shared target. + assert link.is_symlink(), "the socket-parent symlink must be left in place" + mode = stat.S_IMODE(realdir.stat().st_mode) + assert mode == 0o755, f"install chmod'd the symlink target to {oct(mode)}; must not mutate" + + def test_shutil_which_bash_available(): """Sanity guard: the test harness needs a real ``bash`` to invoke the script — surface a clear failure rather than an opaque subprocess error.""" From c07cd6d6e0e8283913a1b92b914e651631e28ec5 Mon Sep 17 00:00:00 2001 From: Varun Singh <382354+vr000m@users.noreply.github.com> Date: Sun, 28 Jun 2026 01:35:14 -0700 Subject: [PATCH 24/24] docs: sync documentation for 0.3.3 release MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - CHANGELOG: add [0.3.3]/[0.3.2] footer links; set 0.3.3 release date to 2026-06-28. - README: backend-extra callout reflects the actionable 'stt_server: the extra is not installed … --inexact' message instead of a bare ModuleNotFoundError. - docs/operations.md: document the installer's pre-mutation validate_socket_path() (absolute / under $HOME / no symlink). - dev plan: refresh spec rows + Final Results to the final state (literal-chain os.lstat symlink rejection, ImportError handler, validate_socket_path) and record the post-phase-5 code-review fixes. --- CHANGELOG.md | 4 +- README.md | 5 ++- ...26-feature-uds-trust-boundary-hardening.md | 38 ++++++++++++++----- docs/operations.md | 11 ++++-- 4 files changed, 42 insertions(+), 16 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 42f35e3..57e3b25 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,7 +7,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] -## [0.3.3] - 2026-06-27 +## [0.3.3] - 2026-06-28 Server-side hardening of the same-host UDS trust boundary. No new user-facing features; clients connect unchanged (no protocol or client-API change). @@ -309,6 +309,8 @@ import name `stt_server`. - Wire protocol is unchanged: `PROTOCOL_VERSION == "0.1"`; the `server.hello` and `server.status` shapes are stable. +[0.3.3]: https://github.com/vr000m/pipecat-local-stt-server/releases/tag/v0.3.3 +[0.3.2]: https://github.com/vr000m/pipecat-local-stt-server/releases/tag/v0.3.2 [0.3.1]: https://github.com/vr000m/pipecat-local-stt-server/releases/tag/v0.3.1 [0.3.0]: https://github.com/vr000m/pipecat-local-stt-server/releases/tag/v0.3.0 [0.2.0]: https://github.com/vr000m/pipecat-local-stt-server/releases/tag/v0.2.0 diff --git a/README.md b/README.md index 508a938..1471940 100644 --- a/README.md +++ b/README.md @@ -69,8 +69,9 @@ uv run python -m stt_server --host 127.0.0.1 --port 8765 --auth-token-file /path > **Backends need their own install.** Every `--backend` above except `echo` > requires a `uv sync` extra first — `mlx`/`parakeet`/`nemotron` (e.g. -> `uv sync --extra mlx`). Without it you get e.g. `ModuleNotFoundError: No module -> named 'mlx_whisper'`. See [Choosing a backend and model](#choosing-a-backend-and-model) +> `uv sync --extra mlx`). Without it the server exits with an actionable +> `stt_server: the '' extra is not installed … run: uv sync --extra +> --inexact` message. See [Choosing a backend and model](#choosing-a-backend-and-model) > for each backend's install command and `--model` defaults. The CLI accepts both `python -m stt_server ` (the legacy flat form, diff --git a/docs/dev_plans/20260626-feature-uds-trust-boundary-hardening.md b/docs/dev_plans/20260626-feature-uds-trust-boundary-hardening.md index cdc1651..b5e4ec6 100644 --- a/docs/dev_plans/20260626-feature-uds-trust-boundary-hardening.md +++ b/docs/dev_plans/20260626-feature-uds-trust-boundary-hardening.md @@ -121,10 +121,14 @@ still not a client code change. This precondition is written into Requirements. trusted_root: Path)`) that creates missing socket directories `0700` (`mkdir(mode=0o700)`, then re-`stat` and verify — `mkdir` mode is umask-masked, so verify rather than trust), then walks every component from the socket's bind - directory up to and including `trusted_root`. Each component must have - `st_uid == os.geteuid()` and no group/other write bits (`st_mode & 0o022 == 0`); - sticky-bit directories are allowed. Raise a clear exception naming the failing - component and condition otherwise. + directory up to and including `trusted_root`. The walk is over the **literal** + lexical path (no `resolve()`); each component is checked with `os.lstat` so a + symlink is detected rather than followed — a symlinked ancestor can be repointed + post-startup, so it is rejected outright. Each component must have + `st_uid == os.geteuid()`, no group/other write bits (`st_mode & 0o022 == 0`), and + must not be a symlink; sticky-bit directories are allowed. Returns the verified + literal path (the caller binds on this, not a `resolve()`d one). Raise a clear + exception naming the failing component and condition otherwise. - [x] Call it in `start()` immediately before the `os.umask(0o077)` block (replacing the bare `socket_path.parent.mkdir(parents=True, exist_ok=True)` at `server.py:148-149`). Resolve and document the trusted root, then verify the @@ -145,6 +149,10 @@ still not a client code change. This precondition is written into Requirements. `mkdir -m 700` (or follow with `chmod 700`), add an upgrade note for pre-existing `0755` dirs, and cross-check the Koda cross-repo contract socket path. Without this, fresh installs and upgrades both break at the phase commit. + **Post-phase-5 addition (`f9f41b6`):** also added `validate_socket_path()` that + validates a custom `PIPECAT_STT_SOCKET` (absolute, under `$HOME`, no symlink + component) **before** any `mkdir`/`chmod`, so a path the server would reject fails + the install cleanly with no filesystem mutation. ### Phase 2 — Peer-credential resolver (cross-platform, isolated + unit-tested) @@ -274,11 +282,11 @@ or equivalent public/API-reachable bypass flag. | File | Change | |---|---| -| `stt_server/server.py:148-149` | Replace bare `mkdir` with `_enforce_socket_dir_secure()`; add the full ancestor-walk helper. | +| `stt_server/server.py:148-149` | Replace bare `mkdir` with `_enforce_socket_dir_secure()`; add the full ancestor-walk helper. Walk is over the **literal** lexical path; `os.lstat` detects and rejects symlink components; returns the verified literal path for binding (no `resolve()`). | | `stt_server/server.py:261-275` | Add UDS-only peer-cred gate in `_process_request` (incl. `sock is None` and `peer_uid` exception → `403`). | | `stt_server/_peercred.py` (new) | Cross-platform `peer_uid(sock)` resolver typed against a minimal structural `Protocol`. | -| `stt_server/__main__.py` `_cmd_serve` (`:210-224`) | Wrap `asyncio.run(serve(...))` in `try/except (ValueError, OSError)` → `stt_server: ` + `SystemExit(1)`. NOT the `_cmd_status` handler at `:298-300`. | -| `scripts/install_stt_agent.sh:100` | `mkdir -m 700` the socket dir; upgrade note for existing `0755` dirs. | +| `stt_server/__main__.py` `_cmd_serve` | Wrap `asyncio.run(serve(...))` in `try/except (ValueError, OSError, ImportError)` → `stt_server: ` + `SystemExit(1)`. `ImportError` (superset of `ModuleNotFoundError`) also surfaces missing backend extras. NOT the `_cmd_status` handler. | +| `scripts/install_stt_agent.sh` | Add `validate_socket_path()` that validates a custom `PIPECAT_STT_SOCKET` (absolute, under `$HOME`, no symlink component) BEFORE any `mkdir`/`chmod`; then `mkdir -m 700` the socket dir and self-heal a pre-existing `0755`. | | `pyproject.toml`, `uv.lock` | Pin `websockets>=16,<17` to keep handshake API assumptions stable across major versions. | | `tests/test_peercred.py` (new) | Unit tests for the resolver incl. macOS ctypes path + forced `sys.platform` branch selection. | | `tests/test_stt_server.py:516+` | Dir-enforcement (incl. foreign-owner ancestor branch), `sock is None`, `peer_uid` raises, and UDS peer-cred integration tests. | @@ -333,7 +341,7 @@ in Phase 2/3 via the `socketpair()` unit test before wiring in):** | Seam | Contract | Verified by | |---|---|---| -| `start()` → dir enforcement | Refuse to bind unless every ancestor through the trusted root is owner-owned and not group/other-writable, except sticky-bit dirs | Phase 1 tests | +| `start()` → dir enforcement | Refuse to bind unless every literal lexical ancestor through the trusted root is owner-owned, not group/other-writable, and not a symlink (`os.lstat`); sticky-bit dirs excepted; returns verified literal path | Phase 1 tests | | `_process_request` → `peer_uid()` | UDS only; `None`, exception, or uid mismatch → `403` before handshake | Phase 3 tests | | `peer_uid()` → OS | macOS `getpeereid` / Linux `SO_PEERCRED`; unknown → `None` (fail closed) | Phase 2 unit tests | | TCP path | Unchanged: Origin + bearer token only | existing tests stay green | @@ -515,6 +523,18 @@ sequenceDiagram - **`test_justfile_map_mirrors_readme` failure is pre-existing**, unrelated to this feature (fails on the clean base commit too — "README per-ASR table header not found"). Not addressed here. +- **Post-phase-5 code-review fixes** added after the Phase 5 doc sync (commits + `a50624c`, `62def0c`, `ffd921f`, `f9f41b6`): (1) `_enforce_socket_dir_secure` + now walks the **literal** path and rejects any symlink component using `os.lstat` + rather than `os.stat` — a symlinked ancestor can be repointed post-startup, so it + is rejected outright; the function returns the verified literal path and the caller + binds on it (not a `resolve()`d copy). (2) `_cmd_serve` exception handler broadened + from `(ValueError, OSError)` to `(ValueError, OSError, ImportError)` so a missing + backend extra also surfaces as a clean `stt_server: ` message. (3) + `install_stt_agent.sh` gained `validate_socket_path()` that checks a custom + `PIPECAT_STT_SOCKET` against the server's own rules (absolute, under `$HOME`, no + symlink component) **before** any `mkdir`/`chmod`, so the install fails cleanly + rather than tightening the directory and then crash-looping the agent. ## Final Results @@ -522,7 +542,7 @@ Phases 1–5 implemented on `feature/uds-trust-boundary-hardening`: | Phase | Commit | Outcome | |---|---|---| -| 1 — Parent-directory enforcement | `839e718` | `_enforce_socket_dir_secure` (owner-only ancestor walk to `$HOME`, creates `0700`), wired into `start()`; `_cmd_serve` turns startup errors into `stt_server: ` + exit 1; `install_stt_agent.sh` creates the socket dir `0700`. | +| 1 — Parent-directory enforcement | `839e718` | `_enforce_socket_dir_secure` (owner-only **literal** ancestor walk to `$HOME`, `os.lstat` rejects symlink components, returns verified literal path, creates `0700`), wired into `start()`; `_cmd_serve` catches `(ValueError, OSError, ImportError)` → `stt_server: ` + exit 1 (`62def0c` broadened from `ValueError, OSError`); `install_stt_agent.sh` creates the socket dir `0700` and now validates custom socket paths before any filesystem mutation (`f9f41b6`); symlink-chain fix landed in `a50624c`. | | 2 — Peer-credential resolver | `49b34dc` | `stt_server/_peercred.py` — `peer_uid(sock)` (Linux `SO_PEERCRED` / macOS `getpeereid` ctypes), fail-closed; verified on host (`peer_uid == os.geteuid()`). | | 3 — Wire peer-cred into handshake | `7773c79` | UDS-only fail-closed `403` gate in `_process_request`, independent of the bearer-token branch; TCP untouched; `websockets` pinned `>=16,<17`. | | 4 — Local end-to-end smoke | `35de5ec` | `scripts/smoke_peercred.py` + `just smoke-peercred` + multi-connection pytest; same-uid concurrency PASS, cross-uid leg skips cleanly without a second uid. | diff --git a/docs/operations.md b/docs/operations.md index 8307a25..f5f312e 100644 --- a/docs/operations.md +++ b/docs/operations.md @@ -200,10 +200,13 @@ change. ### Socket directory permissions (`0700`) — upgrade note -`scripts/install_stt_agent.sh` creates the socket's parent directory `0700` -from birth (`mkdir -m 700`) and also `chmod 700`s it, which **self-heals** a -pre-existing `0755` directory left by an older install. Because the new startup -check refuses to bind against a group/other-writable ancestor: +`scripts/install_stt_agent.sh` validates a custom `PIPECAT_STT_SOCKET` against +the same rules the server enforces — absolute path, under `$HOME`, no symlink +component — **before** any `mkdir`/`chmod`. A path the server would reject fails +the install cleanly with no filesystem mutation. It then creates the socket's +parent directory `0700` from birth (`mkdir -m 700`) and also `chmod 700`s it, +which **self-heals** a pre-existing `0755` directory left by an older install. +Because the new startup check refuses to bind against a group/other-writable ancestor: - **Upgrading an existing host:** re-run `install_stt_agent.sh` (it repairs the dir in place), or manually `chmod 700 ~/Library/Caches/pipecat-stt`.