Enforce request authentication on the Tower local API - #1421
Open
amrmelsayed wants to merge 40 commits into
Open
Enforce request authentication on the Tower local API#1421amrmelsayed wants to merge 40 commits into
amrmelsayed wants to merge 40 commits into
Conversation
Mechanics-free hardening plan for the Tower local HTTP + WebSocket API (advisory GHSA-xvjp-7748-v88v). Five core layers: HTTP key enforcement, WebSocket key enforcement (Sec-WebSocket-Protocol), CORS origin allowlist, cross-client rollout, and BRIDGE_MODE. Key rotation noted as a deferred follow-up (out of scope).
Re-init under a porch-compatible project id (secfix-1); rename the plan file to match. Plan content unchanged. Tower auth hardening lane.
Shared header + WebSocket subprotocol names for the Tower request-auth control (advisory GHSA-xvjp-7748-v88v), so server and clients use one source of truth.
Advisory GHSA-xvjp-7748-v88v hardening. Require the shared local key on every non-public HTTP route (single choke point, constant-time compare, fail closed, 401) and on the WebSocket upgrade (Sec-WebSocket-Protocol, validated before any attach, Origin-independent). Replace reflect-any-https CORS with a fixed origin allowlist. Ensure the key at boot; make enforcement mandatory under BRIDGE_MODE and document the TLS requirement.
Standardize the codev-web-key header (web dashboard, tower.html) and carry the key as a WebSocket subprotocol (vscode, web dashboard), replacing the prior in-band frame. Serve tower.html its key via same-origin injection.
Negative/positive-path coverage for the HTTP key check, public-route allowlist, CORS allowlist, constant-time compare, and the WebSocket upgrade gate. Update the route suites to exercise the real auth helpers.
Add terminalWsProtocols() beside the wire constants so both WS clients share one source of truth for the Sec-WebSocket-Protocol shape (advisory GHSA-xvjp-7748-v88v).
Inject the shared key into every served HTML shell (dashboard, SPA index, annotator) so a direct workspace entry can authenticate; strip CORS from those key-bearing shells so a cross-origin page cannot read the key. Add a Host allowlist (restores the DNS-rebinding guard). Keep the annotator's data/media routes keyed while its shell + vendor libs are public. Repair an existing key file's permissions to 0600 on read.
Read the injected key (window.__CODEV_WEB_KEY__) in the web dashboard and annotator; move dashboard SSE from EventSource to fetch+ReadableStream so it can send the key header; fetch annotator media as authenticated blobs and carry the key on the 3D model loader. Use the shared terminalWsProtocols() builder.
Relax the Host allowlist under BRIDGE_MODE (deliberate network exposure; the key check still guards keyed routes) while keeping the strict loopback guard for the localhost bind. Set the tunnel WS upgrade Host to localhost so tunneled terminals pass the guard. Serve the SPA shell for a bare /workspace/<enc> (no slash). Only embed a hex-validated key in shells (stored-XSS guard). Log rejected requests (Host vs key) for diagnosability.
sse-client.ts calls fetch directly for /api/events (not via TowerClient); send the codev-web-key header so the extension keeps live updates.
Vendor three@0.160.0 (+ STL/3MF/Trackball loaders, fflate) so no remote CDN code runs in the key-injected 3D viewer. Add pagehide object-URL cleanup and guard the tower.html 401 handler against a reload loop when no key was sent.
… sinks Bridge Host relaxation now accepts only IP-literal Hosts (plus loopback + configured), so DNS rebinding (which needs a hostname) stays blocked even in bridge mode; the key remains mandatory and separate. Encode every attacker-influenceable value written into the key-bearing annotator shells (advisory GHSA-xvjp-7748-v88v): HTML-escape filename/path/lang, escape the JSON-in-script filepath against </script>, and integer-validate ?line — so a maliciously-named file cannot XSS the page and read the injected key. Guard the pagehide object-URL cleanup against bfcache-persisted pages.
This change added the first runtime (value) import of codev-types into the Tower server (the WEB_KEY_HEADER / WS_* wire constants). codev-types was only a devDependency — correct while it was type-imported only (erased at build), but a devDependency is not installed for a published/deployed package, so the installed Tower crashed at module load with 'Cannot find module @cluesmith/codev-types' before any logging (surfacing as a 30s startup timeout). Move it to dependencies. Verified with 'pnpm deploy --prod' that a production install now resolves it.
codev now imports codev-types at runtime (request-auth wire constants), so it must be installed as a real dependency. The local-install script packed only core/sdk/codev; add codev-types to the pack + npm-install lists (and the uninstall/rm cleanup). Verified: installing all four tarballs into a throwaway prefix resolves codev-types and the Tower boot module loads.
'afx tower start' polls GET /api/status to detect readiness, but this change made /api/status a keyed route — so the probe 401'd forever and startup timed out after 30s (Tower booted fine; the launcher just never saw 'ready'). Send the shared local key on the probe, like any other trusted local client. /api/status keeps its boot-gated readiness semantics; only the missing credential is added.
…ccept) Standardize the request-auth header on codev-tower-key (matching the already 'tower'-named WS subprotocol), plus the localStorage key and the injected window global. The server accepts the legacy codev-web-key for one release (dual-accept) so an already-installed VS Code / Stream Deck client bundling an older sdk keeps authenticating while it updates. Wire name lives in codev-types (TOWER_KEY_HEADER + LEGACY_WEB_KEY_HEADER); the on-disk key file and WS subprotocol are unchanged. Follow-up: drop the codev-web-key fallback next release.
Pre-existing lint warning (from Spec 1370), not from the auth work — braces added via eslint --fix at the reviewer's request while verifying the branch.
Retrospective for secfix-1 (advisory GHSA-xvjp-7748-v88v), mechanics-free. Routed one arch invariant (Tower API auth enforcement) into arch.md and three lessons (runtime-dep packaging, readiness-probe allowlist, key-bearing-page XSS) into lessons-learned.md.
Codex REQUEST_CHANGES + Claude COMMENT dispositions: document BRIDGE_MODE auth/TLS + CODEV_TOWER_ALLOWED_ORIGINS (agent-farm.md); extract a testable selectWsSubprotocol (asserts the marker is echoed, never the key token) and add a vscode test asserting the WS subprotocols. Reasoned deviations (header rename + dual-accept, CORS env var, bridge key auto-generate) rebutted in the review's consultation-dispositions section.
Two CI failures introduced by the request-authentication change: - Package Install Verification ran afx --help against an install that omitted @cluesmith/codev-types, whose wire constants server-utils now imports at module load, so the CLI crashed at boot. Build + pack the types tarball and pass it to verify-install (mirrors local-install.sh). - Tower Integration Tests hit the now-authenticated Tower with keyless fetch/WebSocket calls and got 401. Inject the codev-tower-key header for loopback Tower requests via a new e2e-only global fetch wrapper (vitest-e2e-setup.ts), and carry the key on terminal/message sockets via a towerWsProtocols() helper.
The 3D viewer must run zero remote code (it holds the injected Tower key to fetch the keyed api/model route), so three.js is served same-origin from templates/vendor/ rather than a CDN. That security property does not require committing ~58k lines of library source. Add three@0.160.0 as an exact-pinned devDependency and a build step (scripts/copy-three.mjs) that regenerates the five vendor files from the package at build time. Output is byte-identical to the previously committed files (four copied verbatim; 3MFLoader's relative fflate import rewritten to the bare importmap specifier), so the importmap, filenames, and viewer behaviour are unchanged. The generated files are gitignored and ship in the npm tarball via the files allowlist, exactly like skeleton/ and dashboard-dist/. Shrinks the PR diff by ~58k lines with no functional change.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
PIR Review: Enforce request authentication on the Tower local API
Private security lane (advisory GHSA-xvjp-7748-v88v). This retrospective, the PR body, and every
committed artifact are deliberately mechanics-free: they describe the hardening, never any
exploit mechanics or attack scenario, and refer to the advisory by id only.
No linked GitHub issue — this was an ad-hoc security task; the advisory is the source of record.
Summary
Tower's local HTTP + WebSocket API performed no server-side request authentication. This PR makes
every non-public route require the shared local key (
~/.agent-farm/local-key), fail closed, andtightens the surrounding controls, then rolls the key transport across all clients. It implements
the advisory's five-layer remediation: HTTP key enforcement at the single request choke point,
WebSocket key enforcement at the upgrade, a fixed CORS origin allowlist, cross-client key
transport, and mandatory enforcement under
BRIDGE_MODE. Key rotation (the advisory's 6th layer)is a deferred follow-up.
Files Changed
(Excludes the vendored three.js assets — see the Architecture note.)
packages/types/src/websocket.ts(+36),packages/types/src/index.ts(+5) — shared wirecontracts (header name, WS subprotocols, the terminal-WS subprotocol builder).
packages/codev/src/agent-farm/utils/server-utils.ts(+280) — the auth helpers: HTTP key check,WS key check, public-route allowlist, constant-time compare, CORS + Host allowlists, cached key.
packages/codev/src/agent-farm/servers/tower-routes.ts(+163) — front-door ordering (preflightbefore the key check, 401), CORS allowlist, same-origin key injection into served HTML shells,
annotator sink encoding, rejected-request logging.
packages/codev/src/agent-farm/servers/tower-websocket.ts(+37) — upgrade-time key gate(Origin-independent), clean reject shapes.
packages/codev/src/agent-farm/servers/tower-server.ts(+30) — ensure the key at boot;mandatory enforcement + fail-closed-if-no-key under
BRIDGE_MODE; subprotocol echo.packages/codev/src/agent-farm/lib/reconnect-backoff.ts(+11) — an app-range WS close code.packages/codev/src/agent-farm/lib/tunnel-client.ts(+10) — tunnel WS upgrade sends a loopbackHostso it passes the Host guard.packages/codev/src/agent-farm/commands/tower.ts(+12) — theafx tower startreadiness probeauthenticates (the probe polls a now-keyed route).
packages/core/src/auth.ts(+11) — repair an existing key file's permissions to0600on read.apps/vscode/src/{terminal-adapter,sse-client,connection-manager}.ts,apps/web/src/{lib/api,hooks/useSSE,components/Terminal}.ts,packages/codev/templates/{tower,open,3d-viewer}.html— send the key on HTTP/SSE/WS; browserpages consume the same-origin-injected key; the annotator loads media as authenticated blobs.
packages/codev/package.json(+codev-typesas a runtime dep),scripts/local-install.sh(pack + install
codev-types),pnpm-lock.yaml— packaging fixes (see Lessons).request-auth.test.ts(+260) plus updates totower-routes,tower-websocket,inbox-routes,spec-761-api-state,tower-cron-routes, sdktower-client, and the apps/websuites.
Plus 5 vendored three.js files (
packages/codev/templates/vendor/three*.js, ~58k lines) — aone-time local vendoring so no remote CDN code runs in a key-bearing page.
Commits
git log main..HEAD --oneline(implementation commits; porch scaffolding omitted):e6181262ftypes: add request-authentication wire contracts37814ee5fserver: enforce request authentication on the Tower API3a882b303clients: transport the shared key on HTTP and WebSocketb0d711f24tests: request-authentication enforcement40a080954types: centralize the terminal WS subprotocol builder5f4cef9dcserver: same-origin key delivery, Host guard, CORS shell isolationf98f42e56clients: web dashboard + annotator key transport9761070b7tests: Host guard + annotator allowlist coverage19ae5013aserver: bridge/tunnel Host handling, injection safety, diagnosticsc7ebde935vscode: authenticate the SSE client0220b46a9annotator: vendor three.js locally; template hardening34f259a76tests: bridge Host, no-slash workspace, SSE-stream + mock updates32dc43411harden: narrow bridge Host to IP-literals; encode annotator XSS sinksf0f4afe2afix: make @cluesmith/codev-types a runtime dependency of codeve8795de95fix(local-install): pack + install @cluesmith/codev-types0ff32b63dfix: authenticate the Tower startup readiness probe11978b516rename the auth header codev-web-key -> codev-tower-key (dual-accept)f36a134ecchore(vscode): satisfy eslint curly rule in tunnel.tsTest Results
pnpm --filter @cluesmith/codev build: ✓ passrequest-authsuite +the dual-accept test)
@cluesmith/codev-sdk: ✓ 98 passed (incl. the import-boundary tests)apps/web: ✓ 335 passed;apps/vscode: ✓ 794 passedapps/webtsc -b✓,apps/vscodemain tsconfig ✓pnpm deploy --prodand a throwaway-prefixnpm installof all fourtarballs (types/core/sdk/codev) — the boot module loads and resolves
codev-types.dev-approvalgate after running the branch (Tower boot +cross-client paths).
Architecture Updates
Routed one system-shape invariant into COLD
codev/resources/arch.md§ Invariants & Constraints(invariant #9): Tower's local API now enforces request authentication — non-public routes
require the shared key and fail closed, and any new Tower route must decide public-vs-keyed via the
isPublicRouteallowlist. Mechanics-free wording (advisory by id). Routed cold rather than into thehot always-injected tier to avoid churning that capped file in a security PR; a future MAINTAIN pass
can promote it if the hot tier warrants it. (
arch.md/lessons-learned.mdare our user-evolvedinstance docs, not
codev-skeletonframework files, so no skeleton mirror applies.)Lessons Learned Updates
Routed three entries into COLD
codev/resources/lessons-learned.md:must move that dep
devDependencies→dependencies, or the published/deployed build crashesat module load (
Cannot find module), invisible to build+test (the monorepo symlinkseverything). Verify the packaged artifact with
pnpm deploy --prod/ a throwaway-prefix install;also update any local-install script that packs a hand-picked subset of workspace packages.
readiness/uptime probes, or startup detection breaks (a keyed
/api/status401'd theafx tower startreadiness probe, so a healthy Tower was killed by its own launcher).an XSS there reads it and yields full API access, so every attacker-influenceable sink in those
pages must be encoded and any
src-loaded media route re-plumbed to an authenticated blob fetch.Things to Look At During PR Review
Security-sensitive spots worth focused attention:
isPublicRoute) — the one place a wrong entry either blocks apre-auth path or exposes a data route. Note the GET-only rule, the
/workspace/<enc>/staticcarve-out that excludes
api//ws//file, and the annotator shell+vendor carve-out that keepsevery data/media sub-route keyed.
codev-tower-keyand the legacycodev-web-keyfor one release, so a not-yet-updated VS Code / Stream Deck keeps working;there's a
# drop the fallback next releasefollow-up.BRIDGE_MODE— strict loopback for the localhost bind; in bridge mode it alsoaccepts IP-literal Hosts but still rejects hostNAMEs (the rebinding guard stays on). Confirm the
relaxation never weakens the (separate, still-mandatory) key check.
strip
Access-Control-Allow-Origin, so a cross-origin page cannot read the injected key; the keyis validated as hex before embedding.
window.__CODEV_TOWER_KEY__.the server echoes only the non-secret marker subprotocol, never the key token.
setRequestHeader; attacker-influenceable values interpolated into the key-bearing shell areencoded at their sink; three.js is vendored locally so no remote code runs in that page.
timingSafeEqual; fail-closed when the key isunavailable.
How to Test Locally
For reviewers pulling the branch:
pnpm -w run local-install(packs + installs types/core/sdk/codev), then the branchTower runs on
:4100; or run the built entry on a spare port.on a direct
/workspace/<enc>/entry; VS Code terminals + gate/comments + live updates;tower.html; the annotator for a text file, an image/video/pdf (authenticated blobs), and an
STL/3MF (the vendored 3D viewer); LAN access under
BRIDGE_MODE; a tunneled terminal; and ano-key / wrong-key request → clean 401 (not a hang).
Flaky Tests
None skipped. Two pre-existing, out-of-scope items noted (not caused by this diff):
apps/vscodewebview tsconfig (tsconfig.webview.json) reports errors insrc/markdown-preview/webview/main.tswhen@cluesmith/codev-artifact-canvasisn't built in theworktree — a build-ordering artifact, untouched by this PR (the main tsconfig, which contains the
changed
terminal-adapter.ts, passes clean).path is covered by fast unit/integration tests in this PR, and updating the e2e suites is a
fast-follow.