🥽 fix: Restrict MCP Server URL Disclosure to Admins, Owners, and Editors (#13784)

* 🥽 fix: Redact Non-User-Sourced MCP Server URLs by ACL Edit Permission

GET /api/mcp/servers and GET /api/mcp/servers/:serverName return MCP server configs to any caller with MCP-use permission. For user-sourced configs (DB-stored, UI-submitted), the URL is the caller's own and is intentionally disclosed. For non-user-sourced configs (YAML or config-tier, operator-defined), the URL and OAuth flow endpoints (authorization_url, token_url) are operator-sensitive: they can encode internal infrastructure hostnames and are not editable through the API.

This change redacts those fields on non-user-sourced configs unless the caller has edit authority on the resource, using the same ACL check (PermissionBits.EDIT) that the PATCH and DELETE routes already enforce via canAccessMCPServerResource. Callers with broad MANAGE_MCP_SERVERS capability bypass the per-resource check, matching the existing capability bypass in canAccessResource. customUserVars is intentionally not redacted: its values are UI hint metadata (title, description, sensitive), not user-supplied secrets; blanking it would give non-editor callers a Configure form with no field labels.

* 🥽 fix: Correct getResourcePermissionsMap import path + tighten redact comments

The MCP server redaction commit imported getResourcePermissionsMap from ~/server/controllers/PermissionsController, but that controller is a consumer of the helper, not its exporter. The canonical export lives in ~/server/services/PermissionService (which controllers/agents/v1.js already imports from). Fixes the runtime getResourcePermissionsMap is not a function failure on GET /api/mcp/servers and the four downstream route-spec failures whose config mocks lacked a source field and were therefore wrongly treated as non-user-sourced; mocks now reflect the real registry behavior (addServer/updateServer tag DB-stored configs with source: 'user'). Trims narrating JSDoc on the redact helpers and resorts the librechat-data-provider destructure by length.

* chore: import order

* 🥽 fix: Redact OAuth Revocation Endpoint Alongside Authorization And Token URLs

The OAuth-URL strip path only dropped authorization_url and token_url. The UserOAuthOptionsSchema in packages/data-provider/src/mcp.ts (line 146) accepts revocation_endpoint as another operator-configurable URL, and the OAuth handler uses it to revoke tokens; it can hold the same internal IdP hostnames the existing strip is trying to hide. Adds revocation_endpoint to the destructure so a non-user-sourced YAML/config MCP server config no longer leaks the revocation URL to non-editor callers. The existing strip url and oauth flow URLs spec is extended with a revocation_endpoint value to lock in the new field.

* 🥽 fix: Gate Shared DB Server URL Disclosure On ACL Edit Permission

source-driven URL disclosure was incorrect for shared DB-backed MCP servers. ServerConfigsDB.mapDBServerToParsedConfig (packages/api/src/mcp/registry/db/ServerConfigsDB.ts:465) sets source: 'user' on every DB-stored config it returns, regardless of who is accessing it. A user with only VIEW share on a DB server, or with agent-mediated access, was therefore treated by the redaction layer as if they owned the URL, and GET /api/mcp/servers disclosed the owner's URL and OAuth flow URLs to viewers who could not edit the resource.

The redaction is now driven purely by ACL edit authority: computeCanEditByServer routes every dbId-bearing config through PermissionBits.EDIT regardless of source; redactServerSecrets strips on !canEdit regardless of source. POST and PATCH controllers explicitly pass canEdit: true since both endpoints establish edit authority (POST creates the resource, PATCH is gated on the EDIT middleware). Legacy/ephemeral configs without a dbId still fall back to the source heuristic.

* 📝 docs: correct redactServerSecrets URL-disclosure comment

---------

Co-authored-by: Danny Avila <danny@librechat.ai>

api/server/controllers/mcp.js

@@ -5,9 +5,10 @@
  * @import { MCPServerRegistry } from '@librechat/api'
  * @import { MCPServerDocument } from 'librechat-data-provider'
  */
-const { logger } = require('@librechat/data-schemas');
+const { logger, SystemCapabilities } = require('@librechat/data-schemas');
 const {
   checkAccess,
+  isUserSourced,
   MCPErrorCodes,
   redactServerSecrets,
   redactAllServerSecrets,
@@ -17,9 +18,11 @@ const {
 const {
   Constants,
   Permissions,
+  ResourceType,
+  PermissionBits,
   PermissionTypes,
-  MCPServerUserInputSchema,
   MCP_USER_INPUT_FIELDS,
+  MCPServerUserInputSchema,
 } = require('librechat-data-provider');
 const {
   resolveConfigServers,
@@ -27,6 +30,8 @@ const {
   resolveAllMcpConfigs,
 } = require('~/server/services/MCP');
 const { cacheMCPServerTools, getMCPServerTools } = require('~/server/services/Config');
+const { getResourcePermissionsMap } = require('~/server/services/PermissionService');
+const { hasCapability } = require('~/server/middleware/roles/capabilities');
 const { getMCPManager, getMCPServersRegistry } = require('~/config');
 const db = require('~/models');
 
@@ -196,6 +201,55 @@ const getMCPTools = async (req, res) => {
     res.status(500).json({ message: error.message });
   }
 };
+/** Mirrors canAccessResource's capability bypass plus per-resource ACL EDIT check. */
+async function computeCanEditByServer(req, serverConfigs) {
+  const canEditByServer = new Map();
+  let bypass = false;
+  try {
+    bypass = await hasCapability(req.user, SystemCapabilities.MANAGE_MCP_SERVERS);
+  } catch (err) {
+    logger.warn(`[computeCanEditByServer] Capability bypass check failed: ${err.message}`);
+  }
+  if (bypass) {
+    for (const name of Object.keys(serverConfigs)) {
+      canEditByServer.set(name, true);
+    }
+    return canEditByServer;
+  }
+  const dbIdsToCheck = [];
+  const dbIdToServerName = new Map();
+  for (const [name, config] of Object.entries(serverConfigs)) {
+    if (config.dbId) {
+      dbIdsToCheck.push(config.dbId);
+      dbIdToServerName.set(String(config.dbId), name);
+      continue;
+    }
+    canEditByServer.set(name, isUserSourced(config));
+  }
+  if (dbIdsToCheck.length > 0) {
+    try {
+      const permsMap = await getResourcePermissionsMap({
+        userId: req.user.id,
+        role: req.user.role,
+        resourceType: ResourceType.MCPSERVER,
+        resourceIds: dbIdsToCheck,
+      });
+      for (const [dbIdStr, name] of dbIdToServerName) {
+        const bits = permsMap.get(dbIdStr) ?? 0;
+        canEditByServer.set(name, (bits & PermissionBits.EDIT) !== 0);
+      }
+    } catch (err) {
+      logger.warn(
+        `[computeCanEditByServer] ACL lookup failed, defaulting to no edit: ${err.message}`,
+      );
+      for (const name of dbIdToServerName.values()) {
+        canEditByServer.set(name, false);
+      }
+    }
+  }
+  return canEditByServer;
+}
+
 /**
  * Get all MCP servers with permissions
  * @route GET /api/mcp/servers
@@ -208,7 +262,8 @@ const getMCPServersList = async (req, res) => {
     }
 
     const serverConfigs = await resolveAllMcpConfigs(userId, req.user);
-    return res.json(redactAllServerSecrets(serverConfigs));
+    const canEditByServer = await computeCanEditByServer(req, serverConfigs);
+    return res.json(redactAllServerSecrets(serverConfigs, { canEditByServer }));
   } catch (error) {
     logger.error('[getMCPServersList]', error);
     res.status(500).json({ error: error.message });
@@ -306,7 +361,7 @@ const createMCPServerController = async (req, res) => {
     );
     res.status(201).json({
       serverName: result.serverName,
-      ...redactServerSecrets(result.config),
+      ...redactServerSecrets(result.config, { canEdit: true }),
     });
   } catch (error) {
     logger.error('[createMCPServer]', error);
@@ -339,7 +394,9 @@ const getMCPServerById = async (req, res) => {
       return res.status(404).json({ message: 'MCP server not found' });
     }
 
-    res.status(200).json(redactServerSecrets(parsedConfig));
+    const canEditMap = await computeCanEditByServer(req, { [serverName]: parsedConfig });
+    const canEdit = canEditMap.get(serverName) ?? false;
+    res.status(200).json(redactServerSecrets(parsedConfig, { canEdit }));
   } catch (error) {
     logger.error('[getMCPServerById]', error);
     res.status(500).json({ message: error.message });
@@ -400,7 +457,7 @@ const updateMCPServerController = async (req, res) => {
       userId,
     );
 
-    res.status(200).json(redactServerSecrets(parsedConfig));
+    res.status(200).json(redactServerSecrets(parsedConfig, { canEdit: true }));
   } catch (error) {
     logger.error('[updateMCPServer]', error);
     const mcpErrorResponse = handleMCPError(error, res);

api/server/routes/__tests__/mcp.spec.js

@@ -2600,11 +2600,13 @@ describe('MCP Routes', () => {
           type: 'sse',
           url: 'http://server1.com/sse',
           title: 'Server 1',
+          source: 'user',
         },
         'server-2': {
           type: 'sse',
           url: 'http://server2.com/sse',
           title: 'Server 2',
+          source: 'user',
         },
       };
 
@@ -2676,7 +2678,7 @@ describe('MCP Routes', () => {
 
       mockRegistryInstance.addServer.mockResolvedValue({
         serverName: 'test-sse-server',
-        config: validConfig,
+        config: { ...validConfig, source: 'user' },
       });
 
       const response = await request(app).post('/api/mcp/servers').send({ config: validConfig });
@@ -3096,6 +3098,7 @@ describe('MCP Routes', () => {
         type: 'sse',
         url: 'https://mcp-server.example.com/sse',
         title: 'Test Server',
+        source: 'user',
       };
 
       mockRegistryInstance.getServerConfig.mockResolvedValue(mockConfig);
@@ -3164,7 +3167,7 @@ describe('MCP Routes', () => {
         description: 'Updated description',
       };
 
-      mockRegistryInstance.updateServer.mockResolvedValue(updatedConfig);
+      mockRegistryInstance.updateServer.mockResolvedValue({ ...updatedConfig, source: 'user' });
 
       const response = await request(app)
         .patch('/api/mcp/servers/test-server')

packages/api/src/mcp/__tests__/utils.test.ts

@@ -236,13 +236,93 @@ describe('redactServerSecrets', () => {
     expect(redacted.customUserVars).toEqual(config.customUserVars);
   });
 
-  it('should pass URLs through unchanged', () => {
+  it('should pass URLs through unchanged when caller has edit authority', () => {
     const config: ParsedServerConfig = {
       type: 'sse',
-      url: 'https://mcp.example.com/sse?param=value',
+      url: 'https://infra.internal/mcp',
+      source: 'config',
+    };
+    const redacted = redactServerSecrets(config, { canEdit: true });
+    expect(redacted.url).toBe('https://infra.internal/mcp');
+  });
+
+  it('should strip url and oauth flow URLs for non-user-sourced configs without edit authority', () => {
+    const config: ParsedServerConfig = {
+      type: 'sse',
+      url: 'https://infra.internal/mcp',
+      source: 'config',
+      oauth: {
+        client_id: 'cid',
+        authorization_url: 'https://infra.internal/oauth/authorize',
+        token_url: 'https://infra.internal/oauth/token',
+        revocation_endpoint: 'https://infra.internal/oauth/revoke',
+        scope: 'read',
+      },
     };
     const redacted = redactServerSecrets(config);
-    expect(redacted.url).toBe('https://mcp.example.com/sse?param=value');
+    expect(redacted.url).toBeUndefined();
+    expect(redacted.oauth?.authorization_url).toBeUndefined();
+    expect(redacted.oauth?.token_url).toBeUndefined();
+    expect(redacted.oauth?.revocation_endpoint).toBeUndefined();
+    expect(redacted.oauth?.client_id).toBe('cid');
+    expect(redacted.oauth?.scope).toBe('read');
+  });
+
+  it('should strip url for a VIEW-shared user-sourced config when caller lacks edit', () => {
+    const config: ParsedServerConfig = {
+      type: 'sse',
+      url: 'https://owner-private.example.com/mcp',
+      source: 'user',
+      dbId: 'abc123',
+      oauth: {
+        client_id: 'cid',
+        authorization_url: 'https://owner-private.example.com/oauth/authorize',
+        token_url: 'https://owner-private.example.com/oauth/token',
+      },
+    };
+    const redacted = redactServerSecrets(config);
+    expect(redacted.url).toBeUndefined();
+    expect(redacted.oauth?.authorization_url).toBeUndefined();
+    expect(redacted.oauth?.token_url).toBeUndefined();
+    expect(redacted.oauth?.client_id).toBe('cid');
+  });
+
+  it('should retain non-sensitive oauth fields when stripping oauth flow URLs', () => {
+    const config: ParsedServerConfig = {
+      type: 'sse',
+      url: 'https://infra.internal/mcp',
+      source: 'config',
+      oauth: {
+        client_id: 'cid',
+        client_secret: 'csecret',
+        authorization_url: 'https://infra.internal/oauth/authorize',
+        token_url: 'https://infra.internal/oauth/token',
+        scope: 'read',
+        redirect_uri: 'https://app.example.com/callback',
+      },
+    };
+    const redacted = redactServerSecrets(config);
+    expect(redacted.oauth?.client_secret).toBeUndefined();
+    expect(redacted.oauth?.authorization_url).toBeUndefined();
+    expect(redacted.oauth?.token_url).toBeUndefined();
+    expect(redacted.oauth?.client_id).toBe('cid');
+    expect(redacted.oauth?.scope).toBe('read');
+    expect(redacted.oauth?.redirect_uri).toBe('https://app.example.com/callback');
+  });
+
+  it('should preserve customUserVars metadata for non-user-sourced configs without edit authority', () => {
+    const config: ParsedServerConfig = {
+      type: 'sse',
+      url: 'https://infra.internal/mcp',
+      source: 'config',
+      customUserVars: {
+        API_KEY: { title: 'API Key', description: 'Your key', sensitive: true },
+      },
+    };
+    const redacted = redactServerSecrets(config);
+    expect(redacted.customUserVars).toEqual({
+      API_KEY: { title: 'API Key', description: 'Your key', sensitive: true },
+    });
   });
 
   it('should only include explicitly allowlisted fields', () => {
@@ -295,6 +375,55 @@ describe('redactAllServerSecrets', () => {
     expect(redacted['server-b'].oauth?.client_id).toBe('cid-b');
     expect((redacted['server-c'] as Record<string, unknown>).command).toBeUndefined();
   });
+
+  it('should pass canEdit through per-server via canEditByServer map', () => {
+    const configs: Record<string, ParsedServerConfig> = {
+      'config-server': {
+        type: 'sse',
+        url: 'https://infra.internal/mcp',
+        source: 'config',
+        oauth: {
+          client_id: 'cid',
+          authorization_url: 'https://infra.internal/oauth/authorize',
+        },
+      },
+      'user-server-owner': {
+        type: 'sse',
+        url: 'https://owner.example.com/mcp',
+        source: 'user',
+        dbId: 'owner-id',
+      },
+      'user-server-shared': {
+        type: 'sse',
+        url: 'https://shared.example.com/mcp',
+        source: 'user',
+        dbId: 'shared-id',
+      },
+    };
+    const canEditByServer = new Map<string, boolean>([
+      ['config-server', false],
+      ['user-server-owner', true],
+      ['user-server-shared', false],
+    ]);
+    const redacted = redactAllServerSecrets(configs, { canEditByServer });
+    expect(redacted['config-server'].url).toBeUndefined();
+    expect(redacted['config-server'].oauth?.authorization_url).toBeUndefined();
+    expect(redacted['user-server-owner'].url).toBe('https://owner.example.com/mcp');
+    expect(redacted['user-server-shared'].url).toBeUndefined();
+  });
+
+  it('should expose URL for non-user-sourced configs when canEditByServer is true', () => {
+    const configs: Record<string, ParsedServerConfig> = {
+      'config-server': {
+        type: 'sse',
+        url: 'https://infra.internal/mcp',
+        source: 'config',
+      },
+    };
+    const canEditByServer = new Map<string, boolean>([['config-server', true]]);
+    const redacted = redactAllServerSecrets(configs, { canEditByServer });
+    expect(redacted['config-server'].url).toBe('https://infra.internal/mcp');
+  });
 });
 
 describe('isInvalidClientMessage', () => {

packages/api/src/mcp/utils.ts

@@ -247,11 +247,19 @@ export function isUserSourced(config: Pick<ParsedServerConfig, 'source' | 'dbId'
  * Allowlist-based sanitization for API responses. Only explicitly listed fields are included;
  * new fields added to ParsedServerConfig are excluded by default until allowlisted here.
  *
- * URLs are returned as-is: DB-stored configs reject ${VAR} patterns at validation time
- * (MCPServerUserInputSchema), and YAML configs are admin-managed. Env variable resolution
- * is handled at the schema/input boundary, not the output boundary.
+ * `url` and the oauth flow URLs (`authorization_url`, `token_url`, `revocation_endpoint`) can
+ * encode internal infrastructure, so they are stripped unless `canEdit` is set: the same
+ * disclosure threshold the PATCH route enforces. Callers derive `canEdit` per server (operator
+ * YAML/config-tier servers from the MANAGE_MCP_SERVERS capability, user-sourced servers from
+ * per-resource ACL EDIT), so a user-sourced config shared view-only does not disclose its URL
+ * to the viewer.
+ * DB-stored configs reject ${VAR} patterns at validation time (MCPServerUserInputSchema);
+ * env variable resolution is handled at the schema/input boundary.
  */
-export function redactServerSecrets(config: ParsedServerConfig): Partial<ParsedServerConfig> {
+export function redactServerSecrets(
+  config: ParsedServerConfig,
+  options?: { canEdit?: boolean },
+): Partial<ParsedServerConfig> {
   const safe: Partial<ParsedServerConfig> = {
     type: config.type,
     url: config.url,
@@ -291,6 +299,19 @@ export function redactServerSecrets(config: ParsedServerConfig): Partial<ParsedS
     safe.obo = config.obo;
   }
 
+  if (!options?.canEdit) {
+    delete safe.url;
+    if (safe.oauth) {
+      const {
+        authorization_url: _au,
+        token_url: _tu,
+        revocation_endpoint: _re,
+        ...restOAuth
+      } = safe.oauth;
+      safe.oauth = restOAuth;
+    }
+  }
+
   return Object.fromEntries(
     Object.entries(safe).filter(([, v]) => v !== undefined),
   ) as Partial<ParsedServerConfig>;
@@ -299,10 +320,12 @@ export function redactServerSecrets(config: ParsedServerConfig): Partial<ParsedS
 /** Applies allowlist-based sanitization to a map of server configs. */
 export function redactAllServerSecrets(
   configs: Record<string, ParsedServerConfig>,
+  options?: { canEditByServer?: ReadonlyMap<string, boolean> },
 ): Record<string, Partial<ParsedServerConfig>> {
   const result: Record<string, Partial<ParsedServerConfig>> = {};
   for (const [key, config] of Object.entries(configs)) {
-    result[key] = redactServerSecrets(config);
+    const canEdit = options?.canEditByServer?.get(key) ?? false;
+    result[key] = redactServerSecrets(config, { canEdit });
   }
   return result;
 }