🪤 fix: Fail-Closed MCP Domain Validation for Unparseable URLs (#12245)

* 🛡️ fix: Fail-closed MCP domain validation for unparseable URLs

`isMCPDomainAllowed` returned true (allow) when `extractMCPServerDomain`
could not parse the URL, treating it identically to a stdio transport.
A URL containing template placeholders or invalid syntax bypassed the
domain allowlist, then `processMCPEnv` resolved it to a valid—and
potentially disallowed—host at connection time.

Distinguish "no URL" (stdio, allowed) from "has URL but unparseable"
(rejected when an allowlist is active) by checking whether `config.url`
is an explicit non-empty string before falling through to the stdio path.

When no allowlist is configured the guard does not fire—unparseable URLs
fall through to connection-level SSRF protection via
`createSSRFSafeUndiciConnect`, preserving legitimate `customUserVars`
template-URL configs.

* test: Expand MCP domain validation coverage for invalid/templated URLs

Cover all branches of the fail-closed guard:
- Invalid/templated URLs rejected when allowlist is configured
- Invalid/templated URLs allowed when no allowlist (null/undefined/[])
- Whitespace-only and empty-string URLs treated as absent across all
  allowedDomains configurations
- Stdio configs (no url property) remain allowed

packages/api/src/auth/domain.spec.ts

@@ -1046,8 +1046,37 @@ describe('isMCPDomainAllowed', () => {
   });
 
   describe('invalid URL handling', () => {
-    it('should allow config with invalid URL (treated as stdio)', async () => {
+    it('should reject invalid URL when allowlist is configured', async () => {
       const config = { url: 'not-a-valid-url' };
+      expect(await isMCPDomainAllowed(config, ['example.com'])).toBe(false);
+    });
+
+    it('should reject templated URL when allowlist is configured', async () => {
+      const config = { url: 'http://{{CUSTOM_HOST}}/mcp' };
+      expect(await isMCPDomainAllowed(config, ['example.com'])).toBe(false);
+    });
+
+    it('should allow invalid URL when no allowlist is configured (defers to connection-level SSRF)', async () => {
+      const config = { url: 'http://{{CUSTOM_HOST}}/mcp' };
+      expect(await isMCPDomainAllowed(config, null)).toBe(true);
+      expect(await isMCPDomainAllowed(config, undefined)).toBe(true);
+      expect(await isMCPDomainAllowed(config, [])).toBe(true);
+    });
+
+    it('should allow config with whitespace-only URL (treated as absent)', async () => {
+      const config = { url: '   ' };
+      expect(await isMCPDomainAllowed(config, [])).toBe(true);
+      expect(await isMCPDomainAllowed(config, ['example.com'])).toBe(true);
+      expect(await isMCPDomainAllowed(config, null)).toBe(true);
+    });
+
+    it('should allow config with empty string URL (treated as absent)', async () => {
+      const config = { url: '' };
+      expect(await isMCPDomainAllowed(config, ['example.com'])).toBe(true);
+    });
+
+    it('should allow config with no url property (stdio)', async () => {
+      const config = { command: 'node', args: ['server.js'] };
       expect(await isMCPDomainAllowed(config, ['example.com'])).toBe(true);
     });
   });

packages/api/src/auth/domain.ts

@@ -442,7 +442,10 @@ export async function isActionDomainAllowed(
 /**
  * Extracts full domain spec (protocol://hostname:port) from MCP server config URL.
  * Returns the full origin for proper protocol/port matching against allowedDomains.
- * Returns null for stdio transports (no URL) or invalid URLs.
+ * @returns The full origin string, or null when:
+ *   - No `url` property, non-string, or empty (stdio transport — always allowed upstream)
+ *   - URL string present but cannot be parsed (rejected fail-closed upstream when allowlist active)
+ *   Callers must distinguish these two null cases; see {@link isMCPDomainAllowed}.
  * @param config - MCP server configuration (accepts any config with optional url field)
  */
 export function extractMCPServerDomain(config: Record<string, unknown>): string | null {
@@ -466,6 +469,11 @@ export function extractMCPServerDomain(config: Record<string, unknown>): string
  * Validates MCP server domain against allowedDomains.
  * Supports HTTP, HTTPS, WS, and WSS protocols (per MCP specification).
  * Stdio transports (no URL) are always allowed.
+ * Configs with a non-empty URL that cannot be parsed are rejected fail-closed when an
+ * allowlist is active, preventing template placeholders (e.g. `{{HOST}}`) from bypassing
+ * domain validation after `processMCPEnv` resolves them at connection time.
+ * When no allowlist is configured, unparseable URLs fall through to connection-level
+ * SSRF protection (`createSSRFSafeUndiciConnect`).
  * @param config - MCP server configuration with optional url field
  * @param allowedDomains - List of allowed domains (with wildcard support)
  */
@@ -474,8 +482,16 @@ export async function isMCPDomainAllowed(
   allowedDomains?: string[] | null,
 ): Promise<boolean> {
   const domain = extractMCPServerDomain(config);
+  const hasAllowlist = Array.isArray(allowedDomains) && allowedDomains.length > 0;
 
-  // Stdio transports don't have domains - always allowed
+  const hasExplicitUrl =
+    Object.hasOwn(config, 'url') && typeof config.url === 'string' && config.url.trim().length > 0;
+
+  if (!domain && hasExplicitUrl && hasAllowlist) {
+    return false;
+  }
+
+  // Stdio transports (no URL) are always allowed
   if (!domain) {
     return true;
   }