Repository navigation
fix(security): container/Fly REQUIRE_API_KEY posture + free-tier CORS leak (#13679 PR E) - #13911
Merged
Merged
Conversation
…e 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
muhamadgalihsaputra
pushed a commit
to niyatna/NiyatnaRoute
that referenced
this pull request
Sep 27, 2026
…e leak (cheaper-inference#13679) (cheaper-inference#13911) PR E of the cheaper-inference#13679 insecure-defaults umbrella (items cheaper-inference#6, cheaper-inference#7; item cheaper-inference#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 (cheaper-inference#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 cheaper-inference#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 cheaper-inference#13679
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
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.
Refs #13679
This is PR E of the #13679 insecure-defaults umbrella (16 findings, split A→F). PRs A, C, D, F
already shipped (#13804, #13813, #13812, #13811). This PR covers items #6, #7 only; item #8
(
outboundUrlGuardPolicylocal-first default) was analyzed as by-design and partially mitigated —no change made, per the plan-file and the task instructions.
Not covered here
src/shared/network/outboundUrlGuardPolicy.ts) — intentionally left unchanged (bydesign: cloud-metadata IPs stay blocked regardless; RFC1918/loopback provider URLs validating by
default is the local-first tradeoff, consistent with [BUG] Unable to properly use and add local OpenAI-compatible provider #5066).
REQUIRE_API_KEY=falsedefault insrc/shared/constants/featureFlagDefinitions.ts— explicitly out of scope per the owner'sdecision on this issue; not touched.
docker-compose.yml— already mitigated (loopback-only bind by default, fix(docker): compose defaults bind 0.0.0.0 while REQUIRE_API_KEY=false leaves /v1 anonymous #12568) and correctlykeeps following the operator's own
.env; not touched.Root cause
Item #6 — keyless-by-default + public listener posture. The published
Dockerfileandfly.tomlshipped with noREQUIRE_API_KEYset, inheriting the npm/CLI local-first default of"false". Combined with:docker run -p 20128:20128 … diegosouzapw/omniroute(the exact one-liner inREADME/QUICK-START — no
--env-file .env), which publishes the port on every interface with noloopback protection, and
fly.toml's[http_service](force_https = true, Fly's edge proxy — inherently public, noloopback-bind option exists on Fly),
...the anonymous
/v1LLM proxy was both keyless AND world-reachable on both published deploymentpaths.
docker-compose.ymlalready closed this for the compose path via loopback-only binding(#12568) and intentionally keeps following the operator's own
.env, so it needed no change.The
openai-to-gemini-sse.ts:289sub-citation in the original audit is the DNS-rebinding angleagainst
/v1beta/models/*:streamGenerateContent— but that line is a plainreturn new Response(...), not where any auth/CORS decision is made. Investigation confirmed the route isalready classified
CLIENT_API(src/server/authz/classify.ts) and fronted byclientApiPolicythrough the central
src/proxy.tsmiddleware before the translator ever runs, and itsCORS-header echo was already hardened fail-closed by #12573
(
tests/unit/gemini-cors-wildcard-bypass.test.ts). The exposure depended entirely on the globalREQUIRE_API_KEYposture — which this PR now defaults totruefor the published container/Flypath, closing it. No code change was needed in the translator or the
/v1betaroute itself; seetests/unit/issue-13679-gemini-sse-requires-api-key.test.tsfor the locking confirmation.Item #7 —
/api/free-tier/summaryCORS + usage leak. The route ships an unconditionalAccess-Control-Allow-Origin: "*"(by design — the community free-tier catalog is meant to bepublicly embeddable) and, until this fix, always included the operator's own local
usedThisMonth/remainingusage numbers regardless of whether the caller was authenticated. Anyreachable origin (including a DNS-rebinding attacker page on a keyless install) could read the
operator's local token-usage stats. Low severity — aggregate usage, not secrets — but still
per-operator data that should not be unconditionally public.
Fix
Dockerfile(runner-base stage): addedENV REQUIRE_API_KEY=true, documented with anexplicit
-e REQUIRE_API_KEY=falseopt-out for an intentionally keyless deployment.fly.toml: addedREQUIRE_API_KEY = "true"to[env].src/app/api/free-tier/summary/route.ts: computeisAuthenticated(req)once and use it togate
usedThisMonth/remaining(returned asnullwhen unauthenticated) while theintentionally-public catalog fields (
perModel,steadyRecurringTokens, …) stay served toeveryone — scoping the leak down rather than 401-ing the whole route, which would have broken
the route's own intended public-catalog use case.
locking regression test instead.
Regression tests
tests/unit/issue-13679-container-posture-require-api-key.test.ts— Dockerfile/fly.toml defaultposture + confirms the npm/CLI local-first default is untouched.
tests/unit/issue-13679-free-tier-summary-usage-leak.test.ts— usage-leak fix.tests/unit/issue-13679-gemini-sse-requires-api-key.test.ts— confirmation lock (already greenpre-fix; documents why no code change was needed there).
RED (Dockerfile/fly.toml, before the fix):
RED (free-tier summary leak, before the fix):
GREEN (after the fix, all three new files):
Existing tests aligned
None weakened. Ran the full existing
tests/unit/free-tier-summary-route.test.ts(5 tests) andtests/unit/free-tier-summary-radar-overlay.test.ts(16 sub-tests) — both still fully greenunmodified: their bare-request/fresh-DB scenario resolves
isAuthenticated()totrue(loopback +setup-incomplete bootstrap path), so
usedThisMonth/remainingstay present exactly as thosetests already assert. Also ran
tests/unit/gemini-cors-wildcard-bypass.test.ts(#12573) andtests/integration/security-hardening.test.ts— all green, confirming the existing CORSfail-closed protection this PR relies on for the gemini item.
Also ran every existing Dockerfile-focused test
(
dockerfile-dashboard-embed-arg-10273,dockerfile-base-path-arg,dockerfile-npm-bundled-cve-patch,dockerfile-build-heap-4076,dockerfile-better-sqlite3-node-gyp-6700,docker-build-memory-budget) — all green.Gates run
npx eslint --suppressions-location config/quality/eslint-suppressions.json <changed files>→exit 0, no output.
npm run typecheck:core→ exit 0 (clean).node scripts/check/check-file-size.mjs→ no violations on touched files (pre-existing drift onchatHelpers.ts,imageGeneration.ts,roundRobinCombo.ts,stream.ts— none touched by thisPR, base tip already over its own frozen values).
node scripts/check/check-complexity.mjs→ OK, no new violations.node scripts/check/check-cognitive-complexity.mjs→ OK — 1279 violations (baseline 1437).node scripts/check/check-test-discovery.mjs→ OK, all 3 new test files discovered.node scripts/check/check-env-doc-sync.mjs→ in sync (REQUIRE_API_KEYalready documented inboth
.env.exampleanddocs/reference/ENVIRONMENT.md; this PR only changes deploymentmanifests, not the variable contract).