Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 12 additions & 0 deletions Dockerfile
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
1 change: 1 addition & 0 deletions changelog.d/fixes/13679-container-posture-cors.md
Original file line number Diff line number Diff line change
@@ -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 changes: 9 additions & 1 deletion fly.toml
Original file line number Diff line number Diff line change
Expand Up @@ -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"
17 changes: 14 additions & 3 deletions src/app/api/free-tier/summary/route.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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),
Expand Down
Original file line number Diff line number Diff line change
@@ -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 changes: 73 additions & 0 deletions tests/unit/issue-13679-free-tier-summary-usage-leak.test.ts
Original file line number Diff line number Diff line change
@@ -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 changes: 72 additions & 0 deletions tests/unit/issue-13679-gemini-sse-requires-api-key.test.ts
Original file line number Diff line number Diff line change
@@ -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;
}
}
});
Loading