From c80c54eefb9c3a7847378e701eb9951687f7faba Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ren=C3=A9=20Mulder?= Date: Mon, 8 Jun 2026 03:39:44 +0200 Subject: [PATCH] =?UTF-8?q?=F0=9F=94=90=20fix:=20Resolve=20Env=20Variables?= =?UTF-8?q?=20in=20MCP=20OAuth=20URL=20Fields=20(#13573)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * 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. --- packages/data-provider/specs/mcp.spec.ts | 161 +++++++++++++++++++++++ packages/data-provider/src/mcp.ts | 33 ++++- 2 files changed, 188 insertions(+), 6 deletions(-) diff --git a/packages/data-provider/specs/mcp.spec.ts b/packages/data-provider/specs/mcp.spec.ts index e5b7d6566a..a52e2f59e9 100644 --- a/packages/data-provider/specs/mcp.spec.ts +++ b/packages/data-provider/specs/mcp.spec.ts @@ -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', () => { diff --git a/packages/data-provider/src/mcp.ts b/packages/data-provider/src/mcp.ts index 430cd4332b..bf9953d51c 100644 --- a/packages/data-provider/src/mcp.ts +++ b/packages/data-provider/src/mcp.ts @@ -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 = >(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);