mirror of
https://github.com/diegosouzapw/OmniRoute.git
synced 2026-09-17 20:32:25 +03:00
fix(security): container/Fly REQUIRE_API_KEY posture + free-tier usage leak (#13679)
PR E of the #13679 insecure-defaults umbrella (items #6, #7; item #8 analyzed as by-design, no change). The published Docker image and fly.toml shipped without REQUIRE_API_KEY set, so a bare `docker run` (README/QUICK-START one-liners, no --env-file) or a `fly deploy` combined "keyless" with "world-reachable" for the anonymous /v1 LLM proxy. docker-compose.yml already mitigates this via loopback-only binding (#12568) and correctly keeps following the operator's own .env, so it is untouched. The npm/CLI local-first REQUIRE_API_KEY=false default in featureFlagDefinitions.ts is also untouched per the owner's decision. /api/free-tier/summary ships an unconditional Access-Control-Allow-Origin: "*" and always included the operator's own local usedThisMonth/remaining usage regardless of auth — a low-severity info leak to any reachable origin. Both fields are now withheld from unauthenticated callers while the intentionally public catalog data stays served to everyone. The gemini-SSE (openai-to-gemini-sse.ts) sub-finding needed no code change: /v1beta/models/*:streamGenerateContent is already classified CLIENT_API and fronted by clientApiPolicy through src/proxy.ts before the translator ever runs, and its CORS-header echo was already hardened fail-closed by #12573. REQUIRE_API_KEY=true (this PR's container/Fly default) closes the dependency that finding cited. Added a locking regression test confirming this chain. Regression tests: - tests/unit/issue-13679-container-posture-require-api-key.test.ts - tests/unit/issue-13679-free-tier-summary-usage-leak.test.ts - tests/unit/issue-13679-gemini-sse-requires-api-key.test.ts (confirmation) Refs #13679
This commit is contained in:
12
Dockerfile
12
Dockerfile
@@ -227,6 +227,18 @@ ENV NODE_OPTIONS="--max-old-space-size=${OMNIROUTE_MEMORY_MB}"
|
||||
ENV DATA_DIR=/app/data
|
||||
RUN mkdir -p /app/data
|
||||
|
||||
# #13679: default the PUBLISHED image to requiring an API key. A bare
|
||||
# `docker run -p 20128:20128 … diegosouzapw/omniroute` (README/QUICK-START
|
||||
# one-liners) does not pass `--env-file .env`, so without this default the
|
||||
# anonymous /v1 LLM proxy would be both keyless AND world-reachable on the
|
||||
# published container. This does NOT change the npm/CLI local-dev default
|
||||
# (`REQUIRE_API_KEY` stays `"false"` in featureFlagDefinitions.ts) — only the
|
||||
# shipped deployment artifact's posture. docker-compose.yml is unaffected: it
|
||||
# loads the operator's own `.env` (env_file:) which overrides this ENV, and
|
||||
# already binds loopback-only by default (#12568). Override with
|
||||
# `-e REQUIRE_API_KEY=false` for an intentionally keyless deployment.
|
||||
ENV REQUIRE_API_KEY=true
|
||||
|
||||
# `npm run build` (build-next-isolated → assembleStandalone) bundles ALL runtime
|
||||
# files into .build/next/standalone/ — .next, node_modules, migrations, scripts,
|
||||
# docs, and the previously hand-COPY'd modules below (@swc/helpers, pino-*, split2,
|
||||
|
||||
1
changelog.d/fixes/13679-container-posture-cors.md
Normal file
1
changelog.d/fixes/13679-container-posture-cors.md
Normal file
@@ -0,0 +1 @@
|
||||
- **fix(security):** default the published Docker image and `fly.toml` deployment to `REQUIRE_API_KEY=true` (npm/CLI local-dev default unchanged), and stop `/api/free-tier/summary`'s wildcard CORS from leaking the operator's local token usage to unauthenticated callers (#13679)
|
||||
10
fly.toml
10
fly.toml
@@ -37,8 +37,16 @@ primary_region = 'sin'
|
||||
|
||||
[env]
|
||||
TZ = "Asia/Shanghai"
|
||||
|
||||
|
||||
# Bind to all interfaces for Fly runtime networking.
|
||||
HOST = "0.0.0.0"
|
||||
HOSTNAME = "0.0.0.0"
|
||||
BIND = "0.0.0.0"
|
||||
|
||||
# #13679: a Fly deployment is reachable over the public internet by design
|
||||
# (force_https + Fly's edge proxy in [http_service] above) — unlike the
|
||||
# docker-compose path, there is no loopback-bind option here. Require an
|
||||
# API key by default so a `fly deploy` from this manifest is never keyless
|
||||
# AND world-reachable at once. This does not change the npm/CLI local-dev
|
||||
# default; it is specific to this published Fly manifest.
|
||||
REQUIRE_API_KEY = "true"
|
||||
|
||||
@@ -79,7 +79,14 @@ export async function GET(req: Request): Promise<Response> {
|
||||
meta.generatedAt !== null &&
|
||||
meta.generatedAt.slice(0, 10) >= FREE_CATALOG_CURATED_AT;
|
||||
|
||||
const serveOverlay = overlayIsFresh && (meta.tier !== "live" || (await isAuthenticated(req)));
|
||||
// #13679: computed once and reused below to also gate the operator-specific
|
||||
// usage fields — this endpoint ships an unconditional wildcard CORS header
|
||||
// (the community free-tier catalog is intentionally public), so anything
|
||||
// that is NOT meant to be world-readable must be withheld here rather than
|
||||
// relying on Origin checks the browser CORS model doesn't actually enforce
|
||||
// for a same-origin-looking (DNS-rebinding) caller.
|
||||
const authed = await isAuthenticated(req);
|
||||
const serveOverlay = overlayIsFresh && (meta.tier !== "live" || authed);
|
||||
|
||||
// Withheld only because it is stale: drop the feed, keep the operator's own
|
||||
// local state. Falling back to the raw baseline here would resurrect models the
|
||||
@@ -95,11 +102,15 @@ export async function GET(req: Request): Promise<Response> {
|
||||
})
|
||||
: computeFreeModelTotals({ excludeTosAvoid });
|
||||
|
||||
const usedThisMonth = sumUsageTokensThisMonth();
|
||||
// #13679: usedThisMonth/remaining reveal the operator's own local usage —
|
||||
// unlike the catalog totals, that is not meant to be public. Withhold both
|
||||
// from unauthenticated callers instead of gating the whole route, so the
|
||||
// intentionally-public catalog fields stay served to everyone.
|
||||
const usedThisMonth = authed ? sumUsageTokensThisMonth() : null;
|
||||
const body = {
|
||||
...totals,
|
||||
usedThisMonth,
|
||||
remaining: Math.max(0, totals.steadyRecurringTokens - usedThisMonth),
|
||||
remaining: authed ? Math.max(0, totals.steadyRecurringTokens - usedThisMonth!) : null,
|
||||
// Which source answered, and the date of what was actually served — the
|
||||
// feed's own build date when the overlay answers (null when a cache row
|
||||
// predates build-date tracking; never the download time standing in),
|
||||
|
||||
@@ -0,0 +1,60 @@
|
||||
/**
|
||||
* Issue #13679 (PR E, item #6) — published container/Fly deployment posture.
|
||||
*
|
||||
* A default `docker run … diegosouzapw/omniroute` (README/QUICK-START one-liners,
|
||||
* which do NOT pass `--env-file .env`) and the shipped `fly.toml` both publish
|
||||
* the app on a public interface (Fly always; a bare `docker run -p 20128:20128`
|
||||
* binds 0.0.0.0 on the host) while `REQUIRE_API_KEY` defaulted to unset/false —
|
||||
* combining "world-reachable" with "keyless" for the anonymous `/v1` LLM proxy.
|
||||
*
|
||||
* `docker-compose.yml` already mitigates this for the compose path (loopback
|
||||
* bind by default, #12568) and intentionally still follows the operator's own
|
||||
* `.env` (own `REQUIRE_API_KEY` choice) — this test does NOT touch that file.
|
||||
*
|
||||
* The npm/CLI local-dev default in `src/shared/constants/featureFlagDefinitions.ts`
|
||||
* (`REQUIRE_API_KEY` defaultValue `"false"`) is explicitly OUT of scope and must
|
||||
* stay unchanged — see AGENTS.md Hard Rule #20 precedent and the owner decision
|
||||
* on #13679 PR E.
|
||||
*/
|
||||
import test from "node:test";
|
||||
import assert from "node:assert/strict";
|
||||
import { readFileSync } from "node:fs";
|
||||
import path from "node:path";
|
||||
|
||||
const ROOT = path.resolve(import.meta.dirname, "../..");
|
||||
|
||||
function read(relPath: string): string {
|
||||
return readFileSync(path.join(ROOT, relPath), "utf-8");
|
||||
}
|
||||
|
||||
test("Dockerfile ships REQUIRE_API_KEY=true by default for the published image", () => {
|
||||
const dockerfile = read("Dockerfile");
|
||||
assert.match(
|
||||
dockerfile,
|
||||
/^ENV REQUIRE_API_KEY=true$/m,
|
||||
"the published Dockerfile must default to REQUIRE_API_KEY=true so `docker run` without " +
|
||||
"an --env-file does not ship keyless AND world-reachable at once — override with " +
|
||||
"`-e REQUIRE_API_KEY=false` for an intentionally keyless deployment"
|
||||
);
|
||||
});
|
||||
|
||||
test("fly.toml ships REQUIRE_API_KEY=true in [env] for the always-public Fly path", () => {
|
||||
const flyToml = read("fly.toml");
|
||||
assert.match(
|
||||
flyToml,
|
||||
/^\s*REQUIRE_API_KEY\s*=\s*"true"\s*$/m,
|
||||
"fly.toml's [env] block must set REQUIRE_API_KEY=true — a Fly deployment is reachable " +
|
||||
"over the public internet by design (force_https + Fly's edge proxy), unlike the " +
|
||||
"docker-compose path which binds loopback-only by default (#12568)"
|
||||
);
|
||||
});
|
||||
|
||||
test("the npm/CLI local-first REQUIRE_API_KEY default is untouched (#13679 PR E constraint)", () => {
|
||||
const definitions = read("src/shared/constants/featureFlagDefinitions.ts");
|
||||
assert.match(
|
||||
definitions,
|
||||
/key:\s*"REQUIRE_API_KEY"[\s\S]{0,200}?defaultValue:\s*"false"/,
|
||||
"the local-first npm/CLI default must stay REQUIRE_API_KEY=false — only the shipped " +
|
||||
"container/Fly deployment manifests change posture, per the #13679 owner decision"
|
||||
);
|
||||
});
|
||||
73
tests/unit/issue-13679-free-tier-summary-usage-leak.test.ts
Normal file
73
tests/unit/issue-13679-free-tier-summary-usage-leak.test.ts
Normal file
@@ -0,0 +1,73 @@
|
||||
/**
|
||||
* Issue #13679 (PR E, item #7) — `/api/free-tier/summary` ships an unconditional
|
||||
* `Access-Control-Allow-Origin: "*"` and, until this fix, always included the
|
||||
* operator's own local usage numbers (`usedThisMonth` / `remaining`) regardless
|
||||
* of whether the caller was authenticated. Combined with the CORS wildcard, any
|
||||
* reachable origin's browser JS could read the operator's local token-usage
|
||||
* stats — a low-severity but real info leak (the plan-file rates this "low":
|
||||
* aggregate usage, not secrets, but still per-operator data that should not be
|
||||
* unconditionally public).
|
||||
*
|
||||
* The catalog itself (perModel, steadyRecurringTokens, …) is intentionally
|
||||
* public — the community free-tier catalog is served to everyone by design
|
||||
* (see the route's own `serveOverlay` comment) — so this fix scopes down only
|
||||
* the operator-specific usage fields, gated behind the same `isAuthenticated()`
|
||||
* check the route already uses for the "live" tier, rather than 401-ing the
|
||||
* whole endpoint (which would break the intended public-catalog use case).
|
||||
*/
|
||||
import test from "node:test";
|
||||
import assert from "node:assert/strict";
|
||||
import { mkdtempSync } from "node:fs";
|
||||
import { tmpdir } from "node:os";
|
||||
import { join } from "node:path";
|
||||
import { PEER_IP_HEADER, VIA_PROXY_HEADER } from "../../src/server/authz/headers.ts";
|
||||
|
||||
process.env.DATA_DIR = mkdtempSync(join(tmpdir(), "omniroute-freetier-leak-"));
|
||||
|
||||
const { GET } = await import("../../src/app/api/free-tier/summary/route.ts");
|
||||
|
||||
const STAMP_TOKEN = "issue-13679-free-tier-stamp-token";
|
||||
|
||||
/**
|
||||
* A remote (non-loopback, non-management-session) caller with no credentials —
|
||||
* the same shape a DNS-rebinding / cross-origin attacker page would present.
|
||||
* Uses the trusted peer-stamp header (like tests/unit/api-auth.test.ts) so the
|
||||
* verdict is deterministic and not dependent on real socket info.
|
||||
*/
|
||||
function remoteUnauthenticatedRequest(): Request {
|
||||
process.env.OMNIROUTE_PEER_STAMP_TOKEN = STAMP_TOKEN;
|
||||
const headers = new Headers();
|
||||
headers.set(PEER_IP_HEADER, `${STAMP_TOKEN}|203.0.113.5`);
|
||||
headers.set(VIA_PROXY_HEADER, `${STAMP_TOKEN}|0`);
|
||||
return new Request("http://localhost/api/free-tier/summary", { headers });
|
||||
}
|
||||
|
||||
test("issue #13679: unauthenticated remote GET must not leak the operator's local usedThisMonth/remaining", async () => {
|
||||
const res = await GET(remoteUnauthenticatedRequest());
|
||||
assert.equal(res.status, 200);
|
||||
const body = await res.json();
|
||||
|
||||
assert.equal(
|
||||
body.usedThisMonth,
|
||||
null,
|
||||
"an unauthenticated remote caller must not learn the operator's local token usage " +
|
||||
`(got ${JSON.stringify(body.usedThisMonth)})`
|
||||
);
|
||||
assert.equal(
|
||||
body.remaining,
|
||||
null,
|
||||
"remaining is derived from usedThisMonth — must also be withheld from unauthenticated callers"
|
||||
);
|
||||
|
||||
// The public catalog itself stays public — this endpoint is intentionally
|
||||
// served to everyone for the community free-tier catalog.
|
||||
assert.ok(
|
||||
Array.isArray(body.perModel) && body.perModel.length > 0,
|
||||
"the public catalog data must still be served to unauthenticated callers"
|
||||
);
|
||||
assert.ok(body.steadyRecurringTokens > 0);
|
||||
});
|
||||
|
||||
test.after(() => {
|
||||
delete process.env.OMNIROUTE_PEER_STAMP_TOKEN;
|
||||
});
|
||||
72
tests/unit/issue-13679-gemini-sse-requires-api-key.test.ts
Normal file
72
tests/unit/issue-13679-gemini-sse-requires-api-key.test.ts
Normal file
@@ -0,0 +1,72 @@
|
||||
/**
|
||||
* Issue #13679 (PR E, item #6, gemini-SSE sub-finding) — CONFIRMATION test,
|
||||
* not a code fix.
|
||||
*
|
||||
* The audit cited `open-sse/translator/response/openai-to-gemini-sse.ts:289`,
|
||||
* but that line is a plain `return new Response(...)` — it is not where any
|
||||
* auth/CORS decision is made (verified by reading the file; also noted in the
|
||||
* plan-file). The actual DNS-rebinding-style exposure the audit is pointing
|
||||
* at depends entirely on the global `REQUIRE_API_KEY` posture: with it
|
||||
* enabled, `/v1beta/models/{model}:streamGenerateContent` (this route) is
|
||||
* classified CLIENT_API (`src/server/authz/classify.ts`) and fronted by
|
||||
* `clientApiPolicy` (`src/server/authz/policies/clientApi.ts`) through the
|
||||
* central `src/proxy.ts` middleware BEFORE the route handler (and therefore
|
||||
* the translator) ever runs — an anonymous/credential-less request is
|
||||
* rejected with 401 regardless of what Origin/Referer it presents (which is
|
||||
* exactly what defeats DNS rebinding: the attacker page can forge Origin but
|
||||
* not a valid bearer token). The CORS-header echo for this same route was
|
||||
* already hardened fail-closed by #12573 (tests/unit/gemini-cors-wildcard-
|
||||
* bypass.test.ts).
|
||||
*
|
||||
* This test locks that chain end-to-end so a future change to classify.ts /
|
||||
* clientApiPolicy.ts cannot silently reopen it. #13679 PR E closes the
|
||||
* "keyless AND world-reachable" combination the finding actually depends on
|
||||
* by shipping REQUIRE_API_KEY=true as the default posture for the published
|
||||
* container/Fly deployment (Dockerfile + fly.toml — see
|
||||
* issue-13679-container-posture-require-api-key.test.ts) — this test is what
|
||||
* proves that default, once applied, actually closes the gemini-SSE path.
|
||||
*/
|
||||
import test from "node:test";
|
||||
import assert from "node:assert/strict";
|
||||
import { classifyRoute } from "../../src/server/authz/classify.ts";
|
||||
import { clientApiPolicy } from "../../src/server/authz/policies/clientApi.ts";
|
||||
|
||||
const GEMINI_STREAM_PATH = "/v1beta/models/gemini-2.5-flash:streamGenerateContent";
|
||||
|
||||
test("issue #13679: the gemini-SSE route is classified CLIENT_API (fronted by clientApiPolicy)", () => {
|
||||
const classification = classifyRoute(GEMINI_STREAM_PATH, "POST");
|
||||
assert.equal(classification.routeClass, "CLIENT_API");
|
||||
});
|
||||
|
||||
test("issue #13679: an anonymous/DNS-rebinding-style request to the gemini-SSE route is rejected when REQUIRE_API_KEY=true", async () => {
|
||||
const originalRequireApiKey = process.env.REQUIRE_API_KEY;
|
||||
process.env.REQUIRE_API_KEY = "true";
|
||||
|
||||
try {
|
||||
const classification = classifyRoute(GEMINI_STREAM_PATH, "POST");
|
||||
|
||||
// A DNS-rebinding attacker page can forge whatever Origin/Referer it
|
||||
// likes — it presents none of the credentials clientApiPolicy checks.
|
||||
const forgedOriginRequest = new Request(`http://localhost${GEMINI_STREAM_PATH}`, {
|
||||
method: "POST",
|
||||
headers: { Origin: "http://attacker.example", Referer: "http://attacker.example/exploit" },
|
||||
});
|
||||
|
||||
const outcome = await clientApiPolicy.evaluate({
|
||||
request: forgedOriginRequest,
|
||||
classification,
|
||||
requestId: "issue-13679-test",
|
||||
});
|
||||
|
||||
assert.equal(outcome.allow, false);
|
||||
if (!outcome.allow) {
|
||||
assert.equal(outcome.status, 401);
|
||||
}
|
||||
} finally {
|
||||
if (originalRequireApiKey === undefined) {
|
||||
delete process.env.REQUIRE_API_KEY;
|
||||
} else {
|
||||
process.env.REQUIRE_API_KEY = originalRequireApiKey;
|
||||
}
|
||||
}
|
||||
});
|
||||
Reference in New Issue
Block a user