Skip to content

🗝️ fix: Exempt Admin-Trusted Domains from MCP OAuth Validation#12255

Merged
danny-avila merged 8 commits into
devfrom
fix/mcp-oauth-ssrf-allowedDomains-bypass
Mar 16, 2026
Merged

🗝️ fix: Exempt Admin-Trusted Domains from MCP OAuth Validation#12255
danny-avila merged 8 commits into
devfrom
fix/mcp-oauth-ssrf-allowedDomains-bypass

Conversation

@danny-avila

@danny-avila danny-avila commented Mar 16, 2026

Copy link
Copy Markdown
Owner

Summary

Fixes a context-blind SSRF guard that blocked private/internal OAuth endpoints for MCP servers whose hostnames were explicitly listed in mcpSettings.allowedDomains. Adds isOAuthUrlAllowed() and threads allowedDomains through the full MCP connection stack so the OAuth layer can honor admin-configured trust.

  • Adds isOAuthUrlAllowed() to packages/api/src/auth/domain.ts — a synchronous allowlist check that mirrors isDomainAllowedCore logic, matching the full OAuth URL against allowedDomains entries including protocol and explicit port constraints, so a scoped entry like https://auth.internal:8443 cannot inadvertently exempt other ports on the same host.
  • Moves the isOAuthUrlAllowed check to the top of validateOAuthUrl, before new URL() hostname extraction, so admin-trusted URLs return after a single URL parse rather than two.
  • Updates validateOAuthUrl JSDoc to document that the isOAuthUrlAllowed check short-circuits the SSRF/DNS path and honors protocol/port constraints when specified by the admin.
  • Adds allowedDomains?: string[] | null to BasicConnectionOptions in packages/api/src/mcp/types/index.ts.
  • Propagates allowedDomains from MCPServersRegistry.getAllowedDomains() through ConnectionsRepository, MCPManager, UserConnectionManager, and MCPConnectionFactory into every MCPOAuthHandler call site (initiateOAuthFlow, refreshOAuthTokens, revokeOAuthToken, discoverMetadata).
  • Passes allowedDomains from MCPServerInspector into MCPConnectionFactory.create() when creating temporary inspection connections.
  • Retrieves allowedDomains from the registry in UserController.js and forwards it to both revokeOAuthToken calls in maybeUninstallOAuthMCP.
  • Adds a JSDoc comment on completeOAuthFlow explaining the intentional absence of allowedDomains — all URLs originate from MCPOAuthFlowMetadata already validated during initiateOAuthFlow.
  • Adds unit tests for isOAuthUrlAllowed covering exact match, wildcard subdomain, case insensitivity, protocol/port constraint enforcement, and null/empty list guards in domain.spec.ts.
  • Adds integration tests in MCPOAuthSecurity.test.ts covering: pre-configured OAuth bypass for private IPs and wildcard internal domains; rejection when allowedDomains is empty or non-matching; auto-discovery path with a private server allowed via allowedDomains; auto-discovery rejection when allowedDomains doesn't match; refreshOAuthTokens branch 3 (no clientInfo, no pre-configured config) with the bypass active; and revokeOAuthToken with a private revocation endpoint in allowedDomains.
  • Updates mock registry instances in ConnectionsRepository.test.ts, MCPManager.test.ts, MCPOAuthRaceCondition.test.ts, and MCPConnectionFactory.test.ts to include getAllowedDomains.

Change Type

  • Bug fix (non-breaking change which fixes an issue)

Testing

All new behavior is covered by automated tests. Tests exercise real code paths per project conventions — resolveHostnameSSRF (DNS-dependent) is the only mock, keeping isSSRFTarget live so private IP literals are genuinely blocked before the bypass is evaluated.

To reproduce the fixed behavior manually: configure an MCP server in librechat.yaml with a private IP OAuth endpoint (e.g., authorization_url: http://10.0.0.1/authorize) and add 10.0.0.1 to mcpSettings.allowedDomains. Prior to this fix, initiating the OAuth flow would throw OAuth authorization_url targets a blocked address; after this fix it proceeds normally.

Test Configuration:

  • Run cd packages/api && npx jest MCPOAuthSecurity for the OAuth SSRF and bypass integration tests.
  • Run cd packages/api && npx jest domain.spec for the isOAuthUrlAllowed unit tests.

Checklist

  • My code adheres to this project's style guidelines
  • I have performed a self-review of my own code
  • I have commented in any complex areas of my code
  • My changes do not introduce new warnings
  • I have written tests demonstrating that my changes are effective or that my feature works
  • Local unit tests pass with my changes

The SSRF guard in validateOAuthUrl was context-blind — it blocked
private/internal OAuth endpoints even for admin-trusted MCP servers
listed in mcpSettings.allowedDomains. Add isHostnameAllowed() to
domain.ts and skip SSRF checks in validateOAuthUrl when the OAuth
endpoint hostname matches an allowed domain.
Pass allowedDomains from MCPServersRegistry through BasicConnectionOptions,
MCPConnectionFactory, and into MCPOAuthHandler method calls so the OAuth
layer can exempt admin-trusted domains from SSRF validation.
Add isHostnameAllowed unit tests (exact, wildcard, case-insensitive,
private IPs). Add MCPOAuthSecurity tests covering the allowedDomains
bypass for initiateOAuthFlow, refreshOAuthTokens, and revokeOAuthToken.
Update registry mocks to include getAllowedDomains.
Copilot AI review requested due to automatic review settings March 16, 2026 00:14

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR updates the MCP OAuth SSRF protection logic to respect the admin-configured allowedDomains allowlist, so that OAuth discovery/token/revocation URLs on trusted/internal hosts aren’t blocked, and threads allowedDomains through the MCP connection/inspection flow.

Changes:

  • Add allowedDomains to MCP connection option plumbing (registry → manager/factory/inspector → OAuth handler).
  • Exempt OAuth URLs from SSRF checks when their hostname matches the configured allowedDomains (via new isHostnameAllowed helper).
  • Add/extend Jest coverage for the allowlist exemption and update registry mocks accordingly.

Reviewed changes

Copilot reviewed 14 out of 14 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
packages/api/src/mcp/types/index.ts Adds allowedDomains to BasicConnectionOptions for propagation.
packages/api/src/mcp/registry/MCPServerInspector.ts Passes allowedDomains into connection creation during inspection.
packages/api/src/mcp/oauth/handler.ts Threads allowedDomains through OAuth flows and skips SSRF checks for allowlisted hostnames.
packages/api/src/mcp/tests/MCPOAuthSecurity.test.ts Adds tests validating allowlist-based SSRF exemption behavior.
packages/api/src/mcp/tests/MCPOAuthRaceCondition.test.ts Updates registry mocks to include getAllowedDomains().
packages/api/src/mcp/tests/MCPManager.test.ts Updates registry mock to include getAllowedDomains().
packages/api/src/mcp/tests/ConnectionsRepository.test.ts Updates registry mock and expected connection options to include allowedDomains.
packages/api/src/mcp/UserConnectionManager.ts Passes registry allowedDomains into connection creation.
packages/api/src/mcp/MCPManager.ts Includes allowedDomains in the BasicConnectionOptions passed to the factory.
packages/api/src/mcp/MCPConnectionFactory.ts Stores and forwards allowedDomains into OAuth handler calls.
packages/api/src/mcp/ConnectionsRepository.ts Passes registry allowedDomains into connection creation.
packages/api/src/auth/domain.ts Introduces isHostnameAllowed() for hostname-level allowlist matching.
packages/api/src/auth/domain.spec.ts Adds unit tests for isHostnameAllowed().
api/server/controllers/UserController.js Passes registry allowedDomains into OAuth token revocation calls.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

You can also share your feedback on Copilot code review. Take the survey.

Comment thread packages/api/src/auth/domain.ts Outdated
Comment thread packages/api/src/auth/domain.ts Outdated
Replace isHostnameAllowed (hostname-only check) with isOAuthUrlAllowed
which parses the full OAuth URL and matches against allowedDomains
entries including protocol and explicit port constraints — mirroring
isDomainAllowedCore's allowlist logic. Prevents a port-scoped entry
like 'https://auth.internal:8443' from also exempting other ports.
…ains

Add three new integration tests using a real OAuth test server:
- auto-discovered OAuth endpoints allowed when server IP is in allowedDomains
- auto-discovered endpoints rejected when allowedDomains doesn't match
- refreshOAuthTokens branch 3 (no clientInfo/config) with allowedDomains bypass

Also rename describe block from ephemeral issue number to durable name.
Prevents future contributors from assuming a missing parameter during
security audits — URLs are pre-validated during initiateOAuthFlow.
Move isOAuthUrlAllowed check before the hostname extraction so
admin-trusted URLs short-circuit with a single URL parse instead
of two. The hostname extraction (new URL) is now deferred to the
SSRF-check path where it's actually needed.
@danny-avila danny-avila linked an issue Mar 16, 2026 that may be closed by this pull request
1 task
@danny-avila danny-avila changed the title Fix/mcp oauth ssrf allowed domains bypass 🗝️ fix: Exempt Admin-Trusted Domains from MCP OAuth Validation Mar 16, 2026
@danny-avila danny-avila merged commit acd07e8 into dev Mar 16, 2026
10 checks passed
@danny-avila danny-avila deleted the fix/mcp-oauth-ssrf-allowedDomains-bypass branch March 16, 2026 03:03
jcbartle pushed a commit to jcbartle/LibreChat that referenced this pull request May 11, 2026
…-avila#12255)

* fix: exempt allowedDomains from MCP OAuth SSRF checks (danny-avila#12254)

The SSRF guard in validateOAuthUrl was context-blind — it blocked
private/internal OAuth endpoints even for admin-trusted MCP servers
listed in mcpSettings.allowedDomains. Add isHostnameAllowed() to
domain.ts and skip SSRF checks in validateOAuthUrl when the OAuth
endpoint hostname matches an allowed domain.

* refactor: thread allowedDomains through MCP connection stack

Pass allowedDomains from MCPServersRegistry through BasicConnectionOptions,
MCPConnectionFactory, and into MCPOAuthHandler method calls so the OAuth
layer can exempt admin-trusted domains from SSRF validation.

* test: add allowedDomains bypass tests and fix registry mocks

Add isHostnameAllowed unit tests (exact, wildcard, case-insensitive,
private IPs). Add MCPOAuthSecurity tests covering the allowedDomains
bypass for initiateOAuthFlow, refreshOAuthTokens, and revokeOAuthToken.
Update registry mocks to include getAllowedDomains.

* fix: enforce protocol/port constraints in OAuth allowedDomains bypass

Replace isHostnameAllowed (hostname-only check) with isOAuthUrlAllowed
which parses the full OAuth URL and matches against allowedDomains
entries including protocol and explicit port constraints — mirroring
isDomainAllowedCore's allowlist logic. Prevents a port-scoped entry
like 'https://auth.internal:8443' from also exempting other ports.

* test: cover auto-discovery and branch-3 refresh paths with allowedDomains

Add three new integration tests using a real OAuth test server:
- auto-discovered OAuth endpoints allowed when server IP is in allowedDomains
- auto-discovered endpoints rejected when allowedDomains doesn't match
- refreshOAuthTokens branch 3 (no clientInfo/config) with allowedDomains bypass

Also rename describe block from ephemeral issue number to durable name.

* docs: explain intentional absence of allowedDomains in completeOAuthFlow

Prevents future contributors from assuming a missing parameter during
security audits — URLs are pre-validated during initiateOAuthFlow.

* test: update initiateOAuthFlow assertion for allowedDomains parameter

* perf: avoid redundant URL parse for admin-trusted OAuth endpoints

Move isOAuthUrlAllowed check before the hostname extraction so
admin-trusted URLs short-circuit with a single URL parse instead
of two. The hostname extraction (new URL) is now deferred to the
SSRF-check path where it's actually needed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug]: MCP Oauth doesn't work to local IP addresses

2 participants