Skip to content

Security audit fixes #13330

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
diegosouzapw merged 8 commits into from
Oct 9, 2026
Merged

Security audit fixes #13330

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
45b54e8
fix(security): close SSRF, plaintext-secret, and MCP-scope gaps found…
JJAbrams-eng Sep 11, 2026
09e1770
test(security): cover resolvePinnedFetch DNS-pin dispatch path and MC…
JJAbrams-eng Sep 16, 2026
35e0c6f
Merge release/v3.8.52 into fix/security-audit-2026-09-10 and redesign…
JJAbrams-eng Sep 29, 2026
2dad946
fix(security): guard operator-supplied provider hosts at connect time…
JJAbrams-eng Sep 29, 2026
3712019
Merge commit '9b768b15bea2a28084d8cc71a85b25eea7e1eeaa' into HEAD
diegosouzapw Oct 8, 2026
3448a4e
test(security): cover the guarded dispatch through execute() and alig…
JJAbrams-eng Oct 8, 2026
77f0e02
fix(security): warn at boot when storage encryption is unset
JJAbrams-eng Oct 9, 2026
c990ec1
fix(i18n): record the MCP server doc hash without claiming the mirror…
JJAbrams-eng Oct 9, 2026
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
2 changes: 1 addition & 1 deletion .i18n-state.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15741,7 +15741,7 @@
}
},
"docs/frameworks/MCP-SERVER.md": {
"source_hash": "e6dad5fa6e9eb6c0c7484edebc4d008255a04bd6058e0cf36464a26dbe114130",
"source_hash": "d52489818c0589192e2cf6601a82962b5b3311ec573f73e6cf6d76fba25c6ec6",
"locales": {
"ar": {
"source_hash": "e6dad5fa6e9eb6c0c7484edebc4d008255a04bd6058e0cf36464a26dbe114130",
Expand Down
16 changes: 16 additions & 0 deletions changelog.d/fixes/13330-security-audit-fixes.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
- **fix(security):** four independent security-audit fixes. The DNS-rebinding guard (GHSA-cmhj-wh2f-9cgx) now also covers the provider dispatch path: for operator-supplied
base URLs (`providerSpecificData.baseUrl`, including `openai-compatible-*` nodes) the connection
is validated at connect time against the same guard mode as the URL check
(`dispatchGuarded()` in `open-sse/executors/dispatchPin.ts`), closing the classic TOCTOU window
on the highest-risk override path; it rides on the ambient patched `fetch` as a dispatcher and is
skipped whenever an outbound proxy, TLS impersonation or the direct sentinel applies, so
configured proxies and request logging are never bypassed. The rate limiter no longer fails fully open when Redis errors — it falls
back to the in-memory limiter instead of allowing every request unconditionally. Boot warns
loudly in production when `STORAGE_ENCRYPTION_KEY` is missing and keeps serving: an empty key is
the documented encryption-disabled contract, so production must not exit. Persisted `JWT_SECRET`/`API_KEY_SECRET`
are encrypted at rest. And the MCP scope-enforcement gap is closed:
`withScopeEnforcement()` now forces scope checks for any caller resolved from a real HTTP
`Authorization` header that does not already hold `manage`/`admin` scope, even when
`OMNIROUTE_MCP_ENFORCE_SCOPES` is off by default — so a key granted only the narrow
`mcp:connect` bypass scope can no longer invoke arbitrary MCP tools once remote access is
enabled ([#13330](https://github.com/diegosouzapw/OmniRoute/pull/13330)) — thanks @JJAbrams-eng
44 changes: 28 additions & 16 deletions docs/frameworks/MCP-SERVER.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -457,28 +457,40 @@ and passes it to the MCP SDK's `transport.handleRequest(req, { authInfo })`, so
the `_meta` and `OMNIROUTE_MCP_SCOPES` env fallback — this only populates that first,
highest-priority source, which was previously unfed over HTTP. When no API key resolves
(no header, invalid key), `authInfo` stays `undefined` and resolution falls through to the
existing `meta`/env chain unchanged. This does NOT flip `OMNIROUTE_MCP_ENFORCE_SCOPES`'s
default — enforcement still has to be explicitly enabled; this change only makes the
per-key path take precedence once it is. stdio has no per-caller identity (see
existing `meta`/env chain unchanged. stdio has no per-caller identity (see
`mcpCallerIdentity.ts`) and is unaffected — it stays on the `_meta`/env fallback chain.

**Enforcement is forced on for narrow-scoped HTTP/SSE callers regardless of
`OMNIROUTE_MCP_ENFORCE_SCOPES`.** `OMNIROUTE_MCP_ENFORCE_SCOPES` defaulting to `false` is only
safe for the local/stdio single-operator flow, where there is no per-caller identity to scope
against. `open-sse/mcp-server/server.ts::withScopeEnforcement()` turns per-tool scope
enforcement on unconditionally (`shouldForceScopeEnforcement()` in `scopeEnforcement.ts`)
whenever `resolveCallerScopeContext()` resolved
`source === "authInfo"` (i.e. a real per-key HTTP Authorization header, HTTP/SSE-only) AND that
key does not hold full `manage`/`admin` scope. This closes the gap where a key holding ONLY
the narrow `mcp:connect` bypass scope — documented above as authorizing nothing but the
`/api/mcp/` LOCAL_ONLY carve-out — could otherwise invoke every MCP tool once an operator
enabled remote/non-loopback MCP access, simply because `OMNIROUTE_MCP_ENFORCE_SCOPES` ships
`false` by default. A full `manage`/`admin` key over HTTP, and every stdio/local caller, keep
the existing `OMNIROUTE_MCP_ENFORCE_SCOPES`-gated behavior unchanged.

---

## Environment Variables

| Variable | Default | Purpose |
| :-------------------------------------- | :--------------------------------- | :----------------------------------------------------------------------------------------------------------------------- |
| `OMNIROUTE_BASE_URL` | `http://localhost:20128` | Base URL the MCP server uses when calling OmniRoute internal APIs |
| `OMNIROUTE_API_KEY` | (empty) | API key forwarded as `Authorization: Bearer` to internal API calls |
| `OMNIROUTE_MCP_ENFORCE_SCOPES` | `false` (only `"true"` enables it) | When enabled, missing scopes deny tool calls and log `scope_denied:<reason>` in audit log |
| `OMNIROUTE_MCP_SCOPES` | (empty) | Comma-separated allowlist of scopes considered "available" by default (used when caller does not provide its own scopes) |
| `OMNIROUTE_MCP_COMPRESS_DESCRIPTIONS` | (unset = on) | When set to `0/false/off/no`, disables MCP description compression at registration time |
| `OMNIROUTE_MCP_DESCRIPTION_COMPRESSION` | (unset = on) | Alternate alias for the same toggle as above |
| `OMNIROUTE_MCP_FETCH_TIMEOUT_MS` | `10000` | Abort budget for internal management reads (health, resilience, combos, quota, usage) |
| `OMNIROUTE_MCP_UPSTREAM_TIMEOUT_MS` | `60000` | Abort budget for hops that wait on a provider (`route_request`, `web_search`, `web_fetch`) |
| `MCP_TOOL_DENY` | (unset = no filter) | Comma-separated tool names to drop from `tools/list` (tool-cardinality reduction — see below) |
| `MCP_TOOL_ALLOW` | (unset = no filter) | Comma-separated tool names to keep exclusively (allow-list mode — see below) |
| `DATA_DIR` | `~/.omniroute` | Heartbeat file is written to `${DATA_DIR}/runtime/mcp-heartbeat.json` |
| Variable | Default | Purpose |
| :-------------------------------------- | :--------------------------------- | :------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- |
| `OMNIROUTE_BASE_URL` | `http://localhost:20128` | Base URL the MCP server uses when calling OmniRoute internal APIs |
| `OMNIROUTE_API_KEY` | (empty) | API key forwarded as `Authorization: Bearer` to internal API calls |
| `OMNIROUTE_MCP_ENFORCE_SCOPES` | `false` (only `"true"` enables it) | When enabled, missing scopes deny tool calls and log `scope_denied:<reason>` in audit log. Enforcement is ALSO forced on regardless of this flag for any HTTP/SSE caller resolved from a per-key Authorization header (`source === "authInfo"`) that lacks full `manage`/`admin` scope — e.g. a key holding only the narrow `mcp:connect` bypass scope — so this default is safe only for the local/stdio single-operator flow, never for remote non-loopback access |
| `OMNIROUTE_MCP_SCOPES` | (empty) | Comma-separated allowlist of scopes considered "available" by default (used when caller does not provide its own scopes) |
| `OMNIROUTE_MCP_COMPRESS_DESCRIPTIONS` | (unset = on) | When set to `0/false/off/no`, disables MCP description compression at registration time |
| `OMNIROUTE_MCP_DESCRIPTION_COMPRESSION` | (unset = on) | Alternate alias for the same toggle as above |
| `OMNIROUTE_MCP_FETCH_TIMEOUT_MS` | `10000` | Abort budget for internal management reads (health, resilience, combos, quota, usage) |
| `OMNIROUTE_MCP_UPSTREAM_TIMEOUT_MS` | `60000` | Abort budget for hops that wait on a provider (`route_request`, `web_search`, `web_fetch`) |
| `MCP_TOOL_DENY` | (unset = no filter) | Comma-separated tool names to drop from `tools/list` (tool-cardinality reduction — see below) |
| `MCP_TOOL_ALLOW` | (unset = no filter) | Comma-separated tool names to keep exclusively (allow-list mode — see below) |
| `DATA_DIR` | `~/.omniroute` | Heartbeat file is written to `${DATA_DIR}/runtime/mcp-heartbeat.json` |

---

Expand Down
37 changes: 17 additions & 20 deletions open-sse/executors/base.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -103,12 +103,7 @@ import {
import { applyPeerTraceHeader } from "@/shared/resilience/peerRouting";
import { applyClineProtocolHeaders } from "@/shared/utils/clineAuth";
import { isProbeContext } from "@/shared/utils/probeOrigin";
import {
parseAndValidatePublicUrl,
parseAndValidateNonMetadataUrl,
} from "@/shared/network/outboundUrlGuard";
import { getProviderValidationGuard } from "@/shared/network/outboundUrlGuardPolicy";
import { isLocalProvider, isSelfHostedChatProvider } from "@/shared/constants/providers";
import { assertDispatchUrlAllowed, dispatchGuarded } from "./dispatchPin.ts";
// Header helpers extracted to a pure leaf; re-exported for external importers
// (executors + tests) that import them from "./base.ts".
export {
Expand Down Expand Up @@ -423,13 +418,7 @@ export class BaseExecutor {
* cloud-metadata IMDS pivot. Throws on a blocked URL.
*/
protected assertOutboundUrlAllowed(url: string): void {
if (!url) return;
if (isLocalProvider(this.provider) || isSelfHostedChatProvider(this.provider)) return;
if (getProviderValidationGuard() === "public-only") {
parseAndValidatePublicUrl(url);
return;
}
parseAndValidateNonMetadataUrl(url);
assertDispatchUrlAllowed(this.provider, url);
}

/**
Expand Down Expand Up @@ -678,12 +667,17 @@ export class BaseExecutor {
}

try {
const response = await fetch(url, {
method: "POST",
headers,
body: JSON.stringify(requestBody),
signal: activeSignal || undefined,
});
const response = await dispatchGuarded(
this.provider,
url,
{
method: "POST",
headers,
body: JSON.stringify(requestBody),
signal: activeSignal || undefined,
},
credentials
);

const text = await response.text();
if (!response.ok) {
Expand Down Expand Up @@ -967,11 +961,14 @@ export class BaseExecutor {
: requestOptions;

try {
// Strict-validation fence (tip) first, then the connect-time DNS-rebinding guard
// for operator-supplied base URLs (#13330) as the transport.
return await validationFetch(
input.validationDispatch,
this.provider,
model,
requestCredentials
requestCredentials,
(url, init) => dispatchGuarded(this.provider, url, init, requestCredentials)
)(requestUrl, optionsWithSignal);
} finally {
if (timeoutId) clearTimeout(timeoutId);
Expand Down
19 changes: 15 additions & 4 deletions open-sse/executors/base/validationDispatch.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -55,13 +55,24 @@ function prepareValidationFetch(
return { ...options, redirect: "error" };
}

/** `fetch` that runs the strict-validation fence first; plain `fetch` when there is no observer. */
type FetchTransport = (url: string, options: RequestInit) => Promise<Response>;

/**
* `fetch` that runs the strict-validation fence first; plain `fetch` when there is no observer.
* `transport` replaces the global `fetch` for the physical request (e.g. the connect-time
* guarded dispatch for operator-supplied base URLs, #13330); it must start the request
* synchronously so nothing awaited separates the fence from the transport.
*/
export function validationFetch(
observer: StrictValidationDispatch | undefined,
provider: string,
model: string,
credentials: ProviderCredentials
): (url: string, options: RequestInit) => Promise<Response> {
credentials: ProviderCredentials,
transport: FetchTransport = (url, options) => fetch(url, options)
): FetchTransport {
return (url, options) =>
fetch(url, prepareValidationFetch(observer, { provider, model, credentials, url }, options));
transport(
url,
prepareValidationFetch(observer, { provider, model, credentials, url }, options)
);
}
Loading