155 lines
17 KiB
Markdown
155 lines
17 KiB
Markdown
# App-level access token (leave-the-LAN bar-raiser)
|
||
|
||
> **Feature id:** `w5-access-token` · **Effort: M** · Branch: `develop`
|
||
> Adds an **optional** shared secret (`WEBTERM_TOKEN`) that gates remote HTTP + the WS handshake with a constant-time-compared, `HttpOnly`/`SameSite=Strict` cookie. **Unset ⇒ auth disabled**, preserving today's LAN zero-config. The token is **additive** — it sits *in front of* the existing Origin/CSRF model (`isOriginAllowed` at `src/http/origin.ts:22`, `requireAllowedOrigin` at `src/server.ts:480`), never replaces it.
|
||
|
||
---
|
||
|
||
## Honest tradeoff (read first — belongs in the PR description too)
|
||
|
||
This is a **bar-raiser, not a TLS/Tailscale substitute.** On bare LAN the terminal stream is still `ws://` (plaintext) — see `buildWsUrl` at `public/terminal-session.ts:47`, which only upgrades to `wss` when the *page* is HTTPS. When the token travels over `ws://`/`http://`, **anyone sniffing the LAN sees the cookie/token in cleartext.** The token only meaningfully hardens the **relay/tunnel path**, where the edge terminates TLS and the browser speaks `wss://`/`https://`. It is a *single shared secret* (no per-user identity, no revocation except changing the env var + restart, no lockout beyond rate-limiting). Ship it with that framing; do not let it read as "now it's safe on the public internet."
|
||
|
||
---
|
||
|
||
## Contract (routes / messages / env / types)
|
||
|
||
### Env (new — `src/config.ts`)
|
||
|
||
| Env var | Type | Default | Meaning |
|
||
|---|---|---|---|
|
||
| `WEBTERM_TOKEN` | string \| undefined | **unset** | Shared access token. **Unset/empty ⇒ auth DISABLED** (everything open, exactly as today). When set: **validated at load** — must match `^[A-Za-z0-9._~+/=-]{16,512}$` (cookie/URL-safe charset, min 16 chars). Invalid ⇒ **throw** (fail-fast, like `parsePort`). |
|
||
| `WEBTERM_TOKEN_TTL` | number (sec) | `2592000` (30d) | *(optional, YAGNI-dial)* Cookie `Max-Age`. Parsed via the existing `parseNonNegativeInt` helper (`src/config.ts:83`). Ship the constant first; only wire the env if trivial. |
|
||
|
||
Charset validation is **not cosmetic**: it blocks `Set-Cookie` header/response-splitting injection and query-string ambiguity, and the ≥16 floor multiplies brute-force cost against the rate limiter.
|
||
|
||
### Config type (`src/types.ts`)
|
||
|
||
Add one field to `Config` (interface at `src/types.ts:21`, alongside the other secret-ish fields near lines 44–47 / 82):
|
||
|
||
```
|
||
readonly webtermToken: string | undefined; // WEBTERM_TOKEN; undefined ⇒ auth disabled (SECRET — never log/expose)
|
||
```
|
||
|
||
Follow the `vapidPrivateKey` precedent (`src/config.ts:349`): read as `env['WEBTERM_TOKEN'] || undefined`, **never** log it, **never** return it over `/config/ui`.
|
||
|
||
### New pure module `src/http/auth.ts` (mirrors the shape/discipline of `src/http/origin.ts`)
|
||
|
||
```
|
||
export const AUTH_COOKIE_NAME = 'webterm_auth'
|
||
export function isAuthEnabled(cfg: Config): boolean // webtermToken != null && != ''
|
||
export function parseCookieHeader(header: string | undefined): Record<string, string>
|
||
export function constantTimeEqual(a: string, b: string): boolean // SHA-256 both → timingSafeEqual (fixed-length guard)
|
||
export function cookieIsAuthed(cfg: Config, cookieHeader: string | undefined): boolean
|
||
export function buildSetCookie(cfg: Config, opts: { secure: boolean }): string // the Set-Cookie value
|
||
export function isHttpsRequest(req: IncomingMessage): boolean // x-forwarded-proto==='https' || socket.encrypted
|
||
```
|
||
|
||
- **`constantTimeEqual`** hashes *both* inputs with `crypto.createHash('sha256')` to a fixed 32 bytes, then `crypto.timingSafeEqual`. Hashing-to-fixed-length is the "fixed-length guard": it removes the length side-channel **and** avoids `timingSafeEqual`'s throw-on-length-mismatch. (Present-vs-absent, i.e. undefined/empty cookie, short-circuits to `false` — a missing cookie is not a secret-comparison oracle.)
|
||
- **`buildSetCookie`** returns:
|
||
`webterm_auth=<token>; Path=/; Max-Age=<ttl>; HttpOnly; SameSite=Strict` **+ `; Secure`** only when `opts.secure` is true. Dynamic `Secure` is required: a `Secure` cookie is never sent over `ws://`/`http://`, so forcing it would silently break LAN-over-HTTP auth; over the relay (`x-forwarded-proto: https`) it must be present.
|
||
|
||
### New/changed HTTP routes (`src/server.ts`)
|
||
|
||
| Route | Guard | Behavior |
|
||
|---|---|---|
|
||
| `GET /login` | **always reachable** (registered *before* the gate) | Serves the self-contained `public/login.html`. |
|
||
| `POST /auth` | **always reachable**, rate-limited (`authLimiter`, 10/min/IP) | Body `token` (accepts `urlencoded` *and* `json`, `limit:'1kb'`). Valid ⇒ `Set-Cookie` (`buildSetCookie`) + **302 → `/`** (native-form path) or **204** for XHR. Invalid ⇒ **401** (+ `302 → /login?e=1` for form navigations). Over rate limit ⇒ **429**. |
|
||
| `GET /?token=<t>` | handled **inside the gate**, rate-limited | Bootstrap link. Valid ⇒ `Set-Cookie` + **302 → same path with `token` stripped** (no token left in history; `Referrer-Policy: no-referrer` already set at `src/server.ts:307`). Invalid ⇒ **302 → /login**. |
|
||
| **The global auth gate** (`app.use`, new) | — | Runs after the security-headers middleware (`src/server.ts:304`) and **before** `express.static` (`src/server.ts:317`). See allow-list below. |
|
||
|
||
### WS handshake (`src/server.ts` upgrade handler, `:1130`)
|
||
|
||
After the existing Origin check passes (`:1141–1146`) and **before** `wss.handleUpgrade` (`:1149`), insert:
|
||
|
||
> if `isAuthEnabled(cfg)` and **not** `cookieIsAuthed(cfg, req.headers['cookie'])` → `socket.write('HTTP/1.1 401 Unauthorized\r\n\r\n'); socket.destroy(); return`.
|
||
|
||
The browser auto-sends the `HttpOnly` cookie on the same-origin WS handshake — **no frontend change to `buildWsUrl`/`connect()` is required** for the authed path.
|
||
|
||
### The gate's allow-list (single central policy point — the `origin.ts:13` idiom)
|
||
|
||
In order, the gate:
|
||
1. `!isAuthEnabled(cfg)` → `next()` *(zero-config LAN unchanged)*.
|
||
2. `isLoopback(req.socket.remoteAddress)` (`src/server.ts:160`) → `next()` *(loopback hook ingest — `POST /hook` `:533`, `/hook/permission` `:561`, `/hook/status` `:890` — has no cookie and must keep working; the token is about **remote** access).*
|
||
3. `GET` with `?token=` → validate (rate-limited) → set-cookie+redirect, or → `/login`.
|
||
4. `cookieIsAuthed(...)` → `next()`.
|
||
5. else unauthed: `Accept: text/html` navigation → **302 → /login**; otherwise → **401** JSON `{ error: 'authentication required' }`.
|
||
|
||
**Scope note (decision to surface at review):** this gate is a **superset** of the ROADMAP's stated "WS + `requireAllowedOrigin` routes." It *also* gates the read-only GET side-channels (`/live-sessions`, `/projects`, **`/projects/diff` — which leaks source**, `/sessions` — which leaks prompts, `/config/ui`, etc.). That is deliberate: for a "don't-expose-a-shell-off-LAN" bar-raiser, leaving source/prompt reads open off-LAN is the bigger hole, and one central gate is simpler + DRYer than threading a token check through ~19 route handlers. `requireAllowedOrigin` is left **untouched** so the CSRF layer stays independent (defense in depth: gate = "are you authorized to be here", Origin = "is this request forged cross-site"). If a reviewer wants the strictly-minimal scope instead, the fallback is a `requireToken(req,res)` helper called only inside `requireAllowedOrigin` + the WS check — but the global gate is recommended.
|
||
|
||
---
|
||
|
||
## Files to change
|
||
|
||
| Path | Concrete change |
|
||
|---|---|
|
||
| `src/types.ts` | Add `readonly webtermToken: string \| undefined` to `Config` (interface `:21`, near the secret fields `:44–47`). **Coordination point** — this is the frozen shared contract; do it here, not locally. Optionally add `authRequired?: boolean` to `UiConfig` (`:656`) for the FE expiry hint. |
|
||
| `src/config.ts` | In `loadConfig` (`:271`): read `WEBTERM_TOKEN` (`env['WEBTERM_TOKEN'] \|\| undefined`), validate charset+min-length when present (throw on bad, like `parsePort` `:170`), add `webtermToken` to the frozen return (`:474`). Never log it. *(Optional: parse `WEBTERM_TOKEN_TTL`.)* |
|
||
| `src/http/auth.ts` | **NEW.** Pure, dependency-light helpers listed in Contract (`isAuthEnabled`, `parseCookieHeader`, `constantTimeEqual`, `cookieIsAuthed`, `buildSetCookie`, `isHttpsRequest`, `AUTH_COOKIE_NAME`). Imports `createHash`, `timingSafeEqual` from `node:crypto`. No Express/DOM types — keep it unit-testable like `origin.ts`. |
|
||
| `src/server.ts` | (1) import the auth helpers (near `:36`). (2) Add `AUTH_RATE_MAX = 10` to the rate-limit constants (`:80–86`). (3) Instantiate `const authLimiter = createRateLimiter(AUTH_RATE_MAX, RATE_LIMIT_WINDOW_MS)` (near `:227`). (4) Register `GET /login` + `POST /auth` **then** `app.use(authGate)` between `:313` and `:317`. (5) `authGate` closure implements the allow-list (reuses `isLoopback` `:160`, `requireAllowedOrigin` untouched). (6) WS upgrade: insert the cookie check after Origin (`:1146`, before `:1149`). Serve `login.html` from a startup-cached read of `path.join(publicDir,'login.html')`. |
|
||
| `public/login.html` | **NEW, fully self-contained.** A `<form method="POST" action="/auth">` with a `type="password"` token field + submit. **Inline `<style>` only, NO inline `<script>`** — the CSP at `src/server.ts:310` is `script-src 'self'` (blocks inline JS) but `style-src 'self' 'unsafe-inline'` (allows inline CSS). Native form POST → server 302 → cookie set → app loads; **needs zero JS**. Show an error banner when `?e=1`. |
|
||
| `public/terminal-session.ts` | *(OPTIONAL polish, low priority)* On WS close-before-`attached` while auth is enabled, print a `statusLine` (`:38`) hint like "Locked — reload to sign in" instead of silent reconnect loops. Covers the cookie-expired-mid-session case. |
|
||
| `docs/ROADMAP.md`, `CLAUDE.md`, `.env`/README env list | Tick the ROADMAP item (`:124`); document `WEBTERM_TOKEN` in the env-var list (CLAUDE.md "Planned Commands" block) with the honest-tradeoff sentence. |
|
||
| `docs/PROGRESS_LOG.md` | Orchestrator appends the completion entry (not the builder). |
|
||
|
||
---
|
||
|
||
## TDD steps (ordered; matches repo vitest style — pure-unit + `test/integration/*` real-server)
|
||
|
||
**Write each test RED first, then implement to GREEN.** Follow AAA and descriptive names, per the repo's `test/origin.test.ts` / `test/integration/server.test.ts` conventions.
|
||
|
||
1. **`test/auth.test.ts` (unit, like `origin.test.ts`)**
|
||
- `constantTimeEqual`: equal strings → `true`; unequal same-length → `false`; **different length → `false`** (no throw); empty/`undefined` → `false`.
|
||
- `parseCookieHeader`: `'a=1; webterm_auth=xyz; b=2'` → map; missing/empty header → `{}`; malformed pairs ignored.
|
||
- `isAuthEnabled`: undefined/empty → `false`; set → `true`.
|
||
- `cookieIsAuthed`: correct cookie → `true`; wrong value → `false`; absent cookie → `false`; disabled cfg → design choice (assert `false` and gate short-circuits before calling it).
|
||
- `buildSetCookie`: asserts substrings `HttpOnly`, `SameSite=Strict`, `Path=/`, `Max-Age=`; **`Secure` present iff `opts.secure`**; token value present.
|
||
|
||
2. **`test/config.test.ts` (extend existing)**
|
||
- `WEBTERM_TOKEN` unset → `cfg.webtermToken === undefined`.
|
||
- Valid token (≥16, safe charset) → stored verbatim.
|
||
- Too short (`'abc'`) → `loadConfig` **throws**.
|
||
- Bad charset (contains `;`, space, control char) → **throws**.
|
||
|
||
3. **`test/integration/auth.test.ts` (NEW real-server, pattern from `server.test.ts`)** — extend `makeTestConfig` to accept `WEBTERM_TOKEN`.
|
||
- **Regression / disabled:** no token set → WS connects with Origin only; `DELETE /live-sessions` with valid Origin works; `GET /live-sessions` open. *(Proves zero-config LAN is untouched.)*
|
||
- **WS enabled:** valid Origin + **no cookie → 401** (`waitForOpen` rejects, mirroring the `:262` bad-Origin test); valid Origin + **valid `Cookie: webterm_auth=<t>` → connects → `attached`**; valid Origin + **wrong cookie → 401**.
|
||
- **`POST /auth`:** wrong token → 401; correct token → 302 + `Set-Cookie` (assert flags; **no `Secure` over http**); `>10` bad attempts/min → **429**.
|
||
- **`x-forwarded-proto: https`** on `POST /auth` → `Set-Cookie` **includes `Secure`**.
|
||
- **`GET /?token=<valid>`** → 302 to `/` (Location has no `token`) + `Set-Cookie`; **`?token=<invalid>`** → 302 `/login`, no cookie.
|
||
- **`GET /login`** → 200 HTML, reachable while unauthed.
|
||
- **Unauthed HTML nav** (`GET /`, `Accept: text/html`, no cookie) → 302 `/login`; **unauthed XHR** (`GET /live-sessions`, no cookie) → 401 JSON.
|
||
- **Gate + CSRF stacking:** `DELETE /live-sessions` valid Origin, **no cookie → 401** (gate); with cookie → passes gate, then Origin check as before.
|
||
- **Loopback bypass:** `POST /hook` from 127.0.0.1 with **no cookie** still returns 204 (hooks unaffected). *(Guard against regressing the side-channel.)*
|
||
|
||
4. Run `npm test` (vitest) + `npm run typecheck`; confirm the 80% coverage gate holds for `src/http/auth.ts` and the new server branches.
|
||
|
||
---
|
||
|
||
## Edge cases
|
||
|
||
- **Cookie-less non-browser clients (curl/scripts):** already rejected by the WS Origin check (undefined Origin → `false`, `origin.ts:26`); the token gate adds a second wall for HTTP. Intended.
|
||
- **Cookie expiry mid-session:** loaded FE keeps a live WS but new WS reconnects/`/config/ui` fetches start 401ing → the OPTIONAL `terminal-session.ts` hint tells the user to reload. No crash.
|
||
- **`SameSite=Strict` + bootstrap link:** the `?token=` response *sets* the cookie and 302-redirects; the follow-up top-level navigation to `/` carries it (Strict permits top-level same-site sends). Works.
|
||
- **Token with URL-reserved chars:** prevented by the config-load charset validation (no `%`-encoding ambiguity, no cookie/header injection).
|
||
- **`?token=` on a non-GET or with a body:** gate only intercepts `?token=` for GET; other methods fall through to the cookie check.
|
||
- **PWA offline shell after expiry:** `sw.js` may serve the cached shell offline (same-origin, no data) but the WS still 401s — no data leak.
|
||
- **Rate-limiter memory:** reuses the existing in-memory sliding-window `createRateLimiter` (`:118`); per-IP arrays are pruned per call — no unbounded growth for the auth endpoint beyond active IPs.
|
||
- **Login page assets vs. gate:** `login.html` must reference **no** external CSS/JS (inline-styles-only) so the gate can 401 everything else including `/build/main.js` while unauthed.
|
||
- **Trailing-slash / `/index.html`:** treat both `/` and `/index.html` navigations the same in the "HTML nav → /login" branch.
|
||
|
||
## Security
|
||
|
||
- **Constant-time compare** via SHA-256→`timingSafeEqual` (no length or content timing oracle; no throw). ✔ security.md secret-handling.
|
||
- **Cookie flags:** `HttpOnly` (JS can't read the token → XSS can't exfiltrate it), `SameSite=Strict` (cross-site pages can't ride the cookie — complements the Origin/CSWSH defense and `requireAllowedOrigin`), `Secure`-when-https, `Path=/`.
|
||
- **Rate limiting** on `/auth` and `?token=` (10/min/IP) raises brute-force cost; combined with the ≥16-char token floor this is not trivially guessable. No account lockout (single shared secret) — documented.
|
||
- **Secret hygiene:** `webtermToken` never logged (follows `vapidPrivateKey` at `:349`), never returned by `/config/ui` (`:1099`). Charset validation blocks `Set-Cookie`/response-splitting injection.
|
||
- **Additive, non-breaking:** Origin check (`origin.ts:22`) and `requireAllowedOrigin` (`:480`) are unchanged; the gate is a new earlier layer. Loopback hook ingest is explicitly bypassed so the smart-features side-channel keeps working.
|
||
- **Honest boundary (repeat in code comments + PR):** plaintext on bare `ws://` — token is a relay/tunnel hardener, **not** a TLS/Tailscale replacement. Keep the "never port-forward this raw" guidance from TECH_DOC §7 intact.
|
||
- **Security-review trigger:** this is auth + cookie + crypto code → run the `security-reviewer` before merge per code-review.md.
|
||
|
||
## Effort & dependencies
|
||
|
||
- **Effort: M** (matches ROADMAP `:129`). No new npm deps — `node:crypto` (`timingSafeEqual`/`createHash`) and Express's built-in `urlencoded`/`json` parsers cover it. `~1` new pure module + `~1` new HTML file + surgical server wiring.
|
||
- **Hard dependencies:** none — self-contained; does not block on the fan-out board or Android parity.
|
||
- **Coordination:** touches two shared files — `src/types.ts` (frozen contract; the `Config.webtermToken` add is the coordination point) and `src/server.ts` (shared wiring). If run in parallel with the other Wave-5 tasks, use `isolation: worktree` and land the `types.ts` bump first so the server change type-checks. `PROGRESS_LOG.md` is orchestrator-written.
|
||
- **Cross-cutting risk to watch:** the CSP `script-src 'self'` constraint (`:310`) forces the login page to be JS-free (native form) — easy to get wrong by reaching for inline `<script>`; the integration test `GET /login → 200` plus a manual load catches it. |