mirror of
https://github.com/diegosouzapw/OmniRoute.git
synced 2026-07-26 09:52:11 +03:00
fix(security): loopback-gate /api/middleware/* (arbitrary JS via vm.Script) (#6541)
Merged — thank you, @developerjillur! Loopback-gates /api/middleware/* (arbitrary JS via vm.Script) for RCE parity with /api/plugins/*. Integrated into release/v3.8.47.
This commit is contained in:
@@ -11,6 +11,7 @@ _Living section — bullets land here as PRs merge into `release/v3.8.47` (paral
|
||||
### 🐛 Bug Fixes
|
||||
|
||||
- **fix(auth):** an API key restricted via `allowedModels`/`allowedCombos` could bypass that restriction entirely over the Codex Responses-over-WebSocket bridge ([#6564](https://github.com/diegosouzapw/OmniRoute/issues/6564)) — `prepare()` in `src/app/api/internal/codex-responses-ws/route.ts` authenticated the WS bridge's API key (`authenticate()`/`authorizeWebSocketHandshake()`) and honored `allowedConnections`, but never called `enforceApiKeyPolicy()`, the same model/combo policy gate the HTTP `/v1/responses` path enforces via `handleChat()` — so a key scoped to e.g. `combo/model-1.0` could still reach a direct Codex model like `gpt-5.5` through this transport, as long as an eligible Codex OAuth connection existed. The bridge's WS auth token arrives via query params (`api_key`/`token`/`access_token`), not a normal `Authorization` header, so a new `enforceCodexWsApiKeyPolicy()` builds an equivalent `Request` carrying an explicit `Authorization: Bearer <apiKey>` header and calls `enforceApiKeyPolicy()` against the CLIENT-requested model, before any Codex-specific model remapping or credential selection. Regression guard: `tests/unit/codex-ws-policy-enforcement-6564.test.ts` (a model-restricted key is rejected 403 before reaching credential selection; a combo-restricted key is rejected 403 requesting a disallowed combo; a key that DOES allow the requested model still proceeds past policy).
|
||||
- **fix(security):** loopback-gate `/api/middleware/*` so a leaked JWT over a tunnel can't install or trigger a middleware hook — middleware hooks compile + run arbitrary JS via `new vm.Script` on the request hot path (`src/lib/middleware/registry.ts`), the same RCE class as the already-gated `/api/plugins/*`; `/api/middleware/` is now in `LOCAL_ONLY_API_PREFIXES` so loopback enforcement runs unconditionally before any auth check (Hard Rules #15 + #17). Regression guard: `tests/unit/route-guard-middleware-local-only.test.ts`. ([#6541](https://github.com/diegosouzapw/OmniRoute/pull/6541)) — see PR. (thanks @developerjillur)
|
||||
- **fix(startup):** AgentBridge's MITM server no longer fails to start with `ROUTER_API_KEY is required` on a normal install ([#6403](https://github.com/diegosouzapw/OmniRoute/issues/6403)) — `POST /api/tools/agent-bridge/server` resolved the spawned MITM child's router key from only an explicit `apiKey` body field (never sent by the AgentBridge UI — the schema has no such field) and the `ROUTER_API_KEY` env var (unset by default), so `startMitm()` always received `""` and the child hard-exited, even though OmniRoute already had a usable API key in its own DB. A new `resolveRouterApiKey()` now falls back to `pickApiKeyForInternalUse()` (the same DB-backed selector the combo-health-check / cloud-sync internal probes use), resolving in order: explicit key → `ROUTER_API_KEY` env → an existing DB key. Regression guard: `tests/unit/agentbridge-mitm-router-key-6403.test.ts`.
|
||||
- **fix(providers):** deploying a Cloudflare relay Worker from Dashboard → System → Proxy pool → Cloudflare relay failed immediately with `Cloudflare Worker upload failed: Content-Type must be one of: application/javascript, text/javascript, multipart/form-data`, even with a valid token/account ([#6416](https://github.com/diegosouzapw/OmniRoute/issues/6416)) — the Worker-script upload built a native `FormData` and let `fetch` derive the multipart Content-Type automatically, but in production `globalThis.fetch` is patched with `node_modules/undici`'s own fetch (`open-sse/utils/proxyFetch.ts`), whose `FormData`/`Request` classes differ from the runtime's global `FormData` (same cross-realm class mismatch already fixed once for image edits in #3273); passing a native `FormData` instance through undici's patched fetch made it serialize the body as the literal string `"[object FormData]"` with `Content-Type: text/plain;charset=UTF-8`, which Cloudflare rejects outright. `buildCloudflareWorkerUploadRequest()` (`src/lib/proxyRelay/cloudflareWorkerScript.ts`) now builds the multipart body as a raw `Buffer` with an explicit boundary and `Content-Type: multipart/form-data; boundary=…` header, accepted verbatim by any fetch implementation. Regression guard: `tests/unit/cloudflare-worker-upload-content-type-6416.test.ts` + updated `tests/unit/relay-deploy-5128.test.ts`.
|
||||
- **fix(startup):** AgentBridge's MITM proxy served a mismatched cert for 3 of the 4 antigravity/cloudcode-pa hosts it terminates TLS for, breaking interception ([#6494](https://github.com/diegosouzapw/OmniRoute/issues/6494)) — `src/mitm/server.cjs`'s `TARGET_HOSTS` decrypts all 4 hosts locally (`daily-cloudcode-pa.googleapis.com`, `cloudcode-pa.googleapis.com`, `daily-cloudcode-pa.sandbox.googleapis.com`, `autopush-cloudcode-pa.sandbox.googleapis.com`), but `src/mitm/cert/generate.ts`'s self-signed cert only carried a SAN entry for the first host — a request to any of the other 3 got served a cert whose CN/SAN didn't match (confirmed via `curl -k https://cloudcode-pa.googleapis.com/` showing `CN=daily-cloudcode-pa.googleapis.com`). `generateCert()` now sources its host list from `ANTIGRAVITY_TARGET.hosts` (`src/mitm/targets/antigravity.ts`, the single authoritative registry already kept in lock-step with `server.cjs`/`dnsConfig.ts`/`mitmToolHosts.ts` by their own drift tests) and emits a SAN entry for all 4 hosts instead of hard-coding a second, incomplete copy. Regression guard: `tests/unit/agentbridge-antigravity-cert-hosts-6494.test.ts` (asserts the host list covers all 4 hosts and that the real generated cert's SAN includes each one).
|
||||
|
||||
@@ -36,6 +36,7 @@ export const LOCAL_ONLY_API_PREFIXES: ReadonlyArray<string> = [
|
||||
"/api/tools/traffic-inspector/", // Traffic Inspector: http-proxy listener + system proxy (Hard Rules #15 + #17)
|
||||
"/api/plugins/", // plugins: load/execute via worker_threads + child_process (Hard Rules #15 + #17)
|
||||
"/api/plugins", // bare path: GET list + POST install also trigger plugin loading
|
||||
"/api/middleware/", // SECURITY_AUDIT M8: middleware hooks compile+run arbitrary JS via new vm.Script (src/lib/middleware/registry.ts) on the request hot path — same code-exec class as /api/plugins/, so loopback-gate it for parity (Hard Rules #15 + #17)
|
||||
"/api/system/version", // auto-update: spawns git checkout + npm install — RCE-via-tunnel surface (Hard Rules #15 + #17, found by 6A.8 route-guard gate)
|
||||
"/api/db-backups/exportAll", // spawns tar for export archive (Hard Rules #15 + #17, found by 6A.8 route-guard gate)
|
||||
"/api/local/", // T-12: 1-click local service launchers (Redis today; spawns podman/docker) — loopback-enforced by isLocalRequestAllowed() in src/lib/security/localEndpoints.ts (Hard Rules #15 + #17)
|
||||
|
||||
31
tests/unit/route-guard-middleware-local-only.test.ts
Normal file
31
tests/unit/route-guard-middleware-local-only.test.ts
Normal file
@@ -0,0 +1,31 @@
|
||||
/**
|
||||
* Security regression: /api/middleware/* routes must be classified as LOCAL_ONLY
|
||||
* so loopback enforcement runs unconditionally before any auth check.
|
||||
*
|
||||
* Middleware hooks compile + run arbitrary JS via `new vm.Script` on the request
|
||||
* hot path (src/lib/middleware/registry.ts) — the same remote-code-execution class
|
||||
* as /api/plugins/* (which is already LOCAL_ONLY). A leaked JWT over a
|
||||
* Cloudflared/Ngrok tunnel must not be able to install or trigger a middleware
|
||||
* hook. Hard Rules #15 + #17. See docs/security/ROUTE_GUARD_TIERS.md.
|
||||
*/
|
||||
import test from "node:test";
|
||||
import assert from "node:assert/strict";
|
||||
import { isLocalOnlyPath } from "../../src/server/authz/routeGuard.ts";
|
||||
|
||||
test("/api/middleware/ prefix (trailing slash) is LOCAL_ONLY", () => {
|
||||
assert.equal(isLocalOnlyPath("/api/middleware/"), true);
|
||||
});
|
||||
|
||||
test("/api/middleware/hooks is LOCAL_ONLY (list + install compile arbitrary JS)", () => {
|
||||
assert.equal(isLocalOnlyPath("/api/middleware/hooks"), true);
|
||||
});
|
||||
|
||||
test("/api/middleware/hooks/[name] is LOCAL_ONLY (compile + run via vm.Script)", () => {
|
||||
assert.equal(isLocalOnlyPath("/api/middleware/hooks/my-hook"), true);
|
||||
});
|
||||
|
||||
test("non-middleware paths are NOT LOCAL_ONLY (no over-match)", () => {
|
||||
assert.equal(isLocalOnlyPath("/api/combos"), false);
|
||||
assert.equal(isLocalOnlyPath("/api/providers"), false);
|
||||
assert.equal(isLocalOnlyPath("/api/keys"), false);
|
||||
});
|
||||
Reference in New Issue
Block a user