Merge 0ff3b46ebd into 8ed0bcf5ca

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

@@ -132,7 +132,7 @@ describe('redactServerSecrets', () => {
     expect(redacted.oauth?.client_id).toBe('cid');
   });
 
-  it('should exclude headers from SSE configs', () => {
+  it('should exclude headers from YAML/non-UI configs (secretHeaderKeys undefined)', () => {
     const config: ParsedServerConfig = {
       type: 'sse',
       url: 'https://example.com/mcp',
@@ -147,6 +147,75 @@ describe('redactServerSecrets', () => {
     expect(redacted.title).toBe('SSE Server');
   });
 
+  it('should expose non-secret headers for UI-created servers (secretHeaderKeys defined)', () => {
+    const config: ParsedServerConfig = {
+      type: 'sse',
+      url: 'https://example.com/mcp',
+      title: 'SSE Server',
+      dbId: '507f1f77bcf86cd799439011',
+      secretHeaderKeys: [],
+    };
+    (config as ParsedServerConfig & { headers: Record<string, string> }).headers = {
+      'X-Index-Name': 'my-index',
+      'X-Top': '{{INDEX_TOP}}',
+    };
+    const redacted = redactServerSecrets(config);
+    expect(
+      (redacted as Record<string, unknown> & { headers: Record<string, string> }).headers,
+    ).toEqual({ 'X-Index-Name': 'my-index', 'X-Top': '{{INDEX_TOP}}' });
+    expect(redacted.secretHeaderKeys).toEqual([]);
+  });
+
+  it('should mask secret header values as empty string and expose non-secret values', () => {
+    const config: ParsedServerConfig = {
+      type: 'streamable-http',
+      url: 'https://example.com/mcp',
+      dbId: '507f1f77bcf86cd799439012',
+      secretHeaderKeys: ['X-Secret-Token'],
+    };
+    (config as ParsedServerConfig & { headers: Record<string, string> }).headers = {
+      'X-Secret-Token': 'super-secret-value',
+      'X-Public-Header': 'public-value',
+    };
+    const redacted = redactServerSecrets(config);
+    const headers = (redacted as Record<string, unknown> & { headers: Record<string, string> })
+      .headers;
+    expect(headers['X-Secret-Token']).toBe('');
+    expect(headers['X-Public-Header']).toBe('public-value');
+    expect(redacted.secretHeaderKeys).toEqual(['X-Secret-Token']);
+  });
+
+  it('should expose secretHeaderKeys: [] for UI servers with all non-secret headers', () => {
+    const config: ParsedServerConfig = {
+      type: 'sse',
+      url: 'https://example.com/mcp',
+      dbId: '507f1f77bcf86cd799439013',
+      secretHeaderKeys: [],
+    };
+    (config as ParsedServerConfig & { headers: Record<string, string> }).headers = {
+      'X-Custom': 'value',
+    };
+    const redacted = redactServerSecrets(config);
+    expect(redacted.secretHeaderKeys).toEqual([]);
+  });
+
+  it('should exclude headers from YAML configs even with secretHeaderKeys present', () => {
+    const config: ParsedServerConfig = {
+      type: 'sse',
+      url: 'https://example.com/mcp',
+      title: 'YAML Server',
+      secretHeaderKeys: ['X-Api-Key'],
+    };
+    (config as ParsedServerConfig & { headers: Record<string, string> }).headers = {
+      'X-Api-Key': '${API_KEY_ENV}',
+      'X-Index': '${INDEX_NAME}',
+    };
+    const redacted = redactServerSecrets(config);
+    expect((redacted as Record<string, unknown>).headers).toBeUndefined();
+    expect((redacted as Record<string, unknown>).secretHeaderKeys).toBeUndefined();
+    expect(redacted.title).toBe('YAML Server');
+  });
+
   it('should exclude env from stdio configs', () => {
     const config: ParsedServerConfig = {
       type: 'stdio',
@@ -225,6 +294,56 @@ describe('redactServerSecrets', () => {
     expect(redacted.customUserVars).toEqual(config.customUserVars);
   });
 
+  it('should preserve serverInstructions: true', () => {
+    const config: ParsedServerConfig = {
+      type: 'sse',
+      url: 'https://example.com/mcp',
+      serverInstructions: true,
+    };
+    const redacted = redactServerSecrets(config);
+    expect(redacted.serverInstructions).toBe(true);
+  });
+
+  it('should preserve serverInstructions as a custom string', () => {
+    const config: ParsedServerConfig = {
+      type: 'sse',
+      url: 'https://example.com/mcp',
+      serverInstructions: 'Always respond concisely.',
+    };
+    const redacted = redactServerSecrets(config);
+    expect(redacted.serverInstructions).toBe('Always respond concisely.');
+  });
+
+  it('should omit serverInstructions when not set', () => {
+    const config: ParsedServerConfig = {
+      type: 'sse',
+      url: 'https://example.com/mcp',
+    };
+    const redacted = redactServerSecrets(config);
+    expect(redacted.serverInstructions).toBeUndefined();
+    expect(Object.prototype.hasOwnProperty.call(redacted, 'serverInstructions')).toBe(false);
+  });
+
+  it('should preserve chatMenu: false', () => {
+    const config: ParsedServerConfig = {
+      type: 'sse',
+      url: 'https://example.com/mcp',
+      chatMenu: false,
+    };
+    const redacted = redactServerSecrets(config);
+    expect(redacted.chatMenu).toBe(false);
+  });
+
+  it('should omit chatMenu when not set', () => {
+    const config: ParsedServerConfig = {
+      type: 'sse',
+      url: 'https://example.com/mcp',
+    };
+    const redacted = redactServerSecrets(config);
+    expect(redacted.chatMenu).toBeUndefined();
+    expect(Object.prototype.hasOwnProperty.call(redacted, 'chatMenu')).toBe(false);
+  });
+
   it('should pass URLs through unchanged', () => {
     const config: ParsedServerConfig = {
       type: 'sse',

packages/api/src/mcp/registry/__tests__/ServerConfigsDB.test.ts

@@ -507,6 +507,185 @@ describe('ServerConfigsDB', () => {
       expect(retrievedWithHeaders?.headers?.['X-My-Api-Key']).toBe('{{MCP_API_KEY}}');
       expect(retrievedWithHeaders?.headers?.Authorization).toBeUndefined();
     });
+
+    it('should encrypt secret header values when saving to database', async () => {
+      const config: ParsedServerConfig & {
+        headers?: Record<string, string>;
+        secretHeaderKeys?: string[];
+      } = {
+        type: 'sse',
+        url: 'https://example.com/mcp',
+        title: 'Secret Header Encryption Test',
+        headers: {
+          'X-Public-Header': 'public-value',
+          'X-Secret-Token': 'super-secret-token',
+        },
+        secretHeaderKeys: ['X-Secret-Token'],
+      };
+      const created = await serverConfigsDB.add('temp-name', config, userId);
+
+      // Verify the secret header value is encrypted in DB (not plaintext)
+      const MCPServer = mongoose.models.MCPServer;
+      const server = await MCPServer.findOne({ serverName: created.serverName });
+      const dbConfig = server?.config as ParsedServerConfig & { headers?: Record<string, string> };
+      expect(dbConfig?.headers?.['X-Secret-Token']).not.toBe('super-secret-token');
+      expect(dbConfig?.headers?.['X-Public-Header']).toBe('public-value');
+      expect(dbConfig?.secretHeaderKeys).toEqual(['X-Secret-Token']);
+
+      // Verify the secret is decrypted when accessed via get()
+      const retrieved = (await serverConfigsDB.get(
+        created.serverName,
+        userId,
+      )) as ParsedServerConfig & {
+        headers?: Record<string, string>;
+      };
+      expect(retrieved?.headers?.['X-Secret-Token']).toBe('super-secret-token');
+      expect(retrieved?.headers?.['X-Public-Header']).toBe('public-value');
+    });
+
+    it('should preserve secret header values when not provided in update', async () => {
+      const config: ParsedServerConfig & {
+        headers?: Record<string, string>;
+        secretHeaderKeys?: string[];
+      } = {
+        type: 'sse',
+        url: 'https://example.com/mcp',
+        title: 'Secret Header Preserve Test',
+        headers: {
+          'X-Public-Header': 'public-value',
+          'X-Secret-Token': 'original-secret',
+        },
+        secretHeaderKeys: ['X-Secret-Token'],
+      };
+      const created = await serverConfigsDB.add('temp-name', config, userId);
+
+      // Update without providing the secret header value (empty string means "preserve existing")
+      const updatedConfig: ParsedServerConfig & {
+        headers?: Record<string, string>;
+        secretHeaderKeys?: string[];
+      } = {
+        type: 'sse',
+        url: 'https://example.com/mcp',
+        title: 'Secret Header Preserve Test',
+        description: 'Updated description',
+        headers: {
+          'X-Public-Header': 'updated-public-value',
+          'X-Secret-Token': '', // Empty means preserve existing
+        },
+        secretHeaderKeys: ['X-Secret-Token'],
+      };
+      await serverConfigsDB.update(created.serverName, updatedConfig, userId);
+
+      // Verify the secret is still encrypted in DB (preserved, not plaintext)
+      const MCPServer = mongoose.models.MCPServer;
+      const server = await MCPServer.findOne({ serverName: created.serverName });
+      const dbConfig = server?.config as ParsedServerConfig & { headers?: Record<string, string> };
+      expect(dbConfig?.headers?.['X-Secret-Token']).not.toBe('original-secret');
+      expect(dbConfig?.headers?.['X-Public-Header']).toBe('updated-public-value');
+
+      // Verify the secret is decrypted when accessed via get()
+      const retrieved = (await serverConfigsDB.get(
+        created.serverName,
+        userId,
+      )) as ParsedServerConfig & {
+        headers?: Record<string, string>;
+      };
+      expect(retrieved?.headers?.['X-Secret-Token']).toBe('original-secret');
+      expect(retrieved?.headers?.['X-Public-Header']).toBe('updated-public-value');
+      expect(retrieved?.description).toBe('Updated description');
+    });
+
+    it('should allow updating secret header value when explicitly provided', async () => {
+      const config: ParsedServerConfig & {
+        headers?: Record<string, string>;
+        secretHeaderKeys?: string[];
+      } = {
+        type: 'sse',
+        url: 'https://example.com/mcp',
+        title: 'Secret Header Update Test',
+        headers: {
+          'X-Secret-Token': 'old-secret',
+        },
+        secretHeaderKeys: ['X-Secret-Token'],
+      };
+      const created = await serverConfigsDB.add('temp-name', config, userId);
+
+      // Update with new secret value
+      const updatedConfig: ParsedServerConfig & {
+        headers?: Record<string, string>;
+        secretHeaderKeys?: string[];
+      } = {
+        type: 'sse',
+        url: 'https://example.com/mcp',
+        title: 'Secret Header Update Test',
+        headers: {
+          'X-Secret-Token': 'new-secret',
+        },
+        secretHeaderKeys: ['X-Secret-Token'],
+      };
+      await serverConfigsDB.update(created.serverName, updatedConfig, userId);
+
+      // Verify the secret is encrypted in DB (not plaintext)
+      const MCPServer = mongoose.models.MCPServer;
+      const server = await MCPServer.findOne({ serverName: created.serverName });
+      const dbConfig = server?.config as ParsedServerConfig & { headers?: Record<string, string> };
+      expect(dbConfig?.headers?.['X-Secret-Token']).not.toBe('new-secret');
+
+      // Verify the secret is decrypted to the new value when accessed via get()
+      const retrieved = (await serverConfigsDB.get(
+        created.serverName,
+        userId,
+      )) as ParsedServerConfig & {
+        headers?: Record<string, string>;
+      };
+      expect(retrieved?.headers?.['X-Secret-Token']).toBe('new-secret');
+    });
+
+    it('should handle multiple secret headers with mixed preservation', async () => {
+      const config: ParsedServerConfig & {
+        headers?: Record<string, string>;
+        secretHeaderKeys?: string[];
+      } = {
+        type: 'sse',
+        url: 'https://example.com/mcp',
+        title: 'Multiple Secret Headers Test',
+        headers: {
+          'X-Secret-One': 'secret-one',
+          'X-Secret-Two': 'secret-two',
+          'X-Public': 'public',
+        },
+        secretHeaderKeys: ['X-Secret-One', 'X-Secret-Two'],
+      };
+      const created = await serverConfigsDB.add('temp-name', config, userId);
+
+      // Update: preserve X-Secret-One (empty), update X-Secret-Two, update X-Public
+      const updatedConfig: ParsedServerConfig & {
+        headers?: Record<string, string>;
+        secretHeaderKeys?: string[];
+      } = {
+        type: 'sse',
+        url: 'https://example.com/mcp',
+        title: 'Multiple Secret Headers Test',
+        headers: {
+          'X-Secret-One': '', // Preserve existing
+          'X-Secret-Two': 'new-secret-two', // Update
+          'X-Public': 'updated-public',
+        },
+        secretHeaderKeys: ['X-Secret-One', 'X-Secret-Two'],
+      };
+      await serverConfigsDB.update(created.serverName, updatedConfig, userId);
+
+      // Verify via get()
+      const retrieved = (await serverConfigsDB.get(
+        created.serverName,
+        userId,
+      )) as ParsedServerConfig & {
+        headers?: Record<string, string>;
+      };
+      expect(retrieved?.headers?.['X-Secret-One']).toBe('secret-one'); // Preserved
+      expect(retrieved?.headers?.['X-Secret-Two']).toBe('new-secret-two'); // Updated
+      expect(retrieved?.headers?.['X-Public']).toBe('updated-public');
+    });
   });
 
   describe('credential placeholder sanitization', () => {

packages/api/src/mcp/registry/db/ServerConfigsDB.ts

@@ -217,6 +217,45 @@ export class ServerConfigsDB implements IServerConfigsRepositoryInterface {
       };
     }
 
+    // Retain existing encrypted values for secret headers that were submitted with blank values.
+    // A blank value means "don't change this secret header" (same pattern as oauth.client_secret).
+    // Only preserve when:
+    // 1. The key is explicitly present with an empty string (not omitted entirely)
+    // 2. The key was already secret in the existing config (to avoid copying plaintext as encrypted)
+    const newSecretKeys = configToSave.secretHeaderKeys;
+    if (newSecretKeys?.length && existingServer?.config) {
+      const existingHeaders = (
+        existingServer.config as ParsedServerConfig & { headers?: Record<string, string> }
+      ).headers;
+      const existingSecretKeys = new Set(
+        (existingServer.config as ParsedServerConfig & { secretHeaderKeys?: string[] })
+          .secretHeaderKeys ?? [],
+      );
+      const currentHeaders = (
+        configToSave as ParsedServerConfig & { headers?: Record<string, string> }
+      ).headers;
+      if (existingHeaders && currentHeaders) {
+        const retainedHeaders = { ...currentHeaders };
+        for (const secretKey of newSecretKeys) {
+          // Check if the header key exists in the payload with an explicitly blank string value.
+          // If the key is omitted entirely, allow removal; only preserve on blank string (masked).
+          // Also verify this key was already secret to avoid treating plaintext as encrypted.
+          if (
+            Object.prototype.hasOwnProperty.call(retainedHeaders, secretKey) &&
+            retainedHeaders[secretKey] === '' &&
+            existingSecretKeys.has(secretKey)
+          ) {
+            const existingValue = existingHeaders[secretKey];
+            if (existingValue) {
+              retainedHeaders[secretKey] = existingValue;
+            }
+          }
+        }
+        (configToSave as ParsedServerConfig & { headers?: Record<string, string> }).headers =
+          retainedHeaders;
+      }
+    }
+
     await this._dbMethods.updateMCPServer(serverName, { config: configToSave });
   }
 
@@ -489,7 +528,8 @@ export class ServerConfigsDB implements IServerConfigsRepositoryInterface {
 
   /**
    * Encrypts sensitive fields in config before database storage.
-   * Encrypts oauth.client_secret and apiKey.key (when source === 'admin').
+   * Encrypts oauth.client_secret, apiKey.key (when source === 'admin'),
+   * and header values listed in secretHeaderKeys.
    * Throws on failure to prevent storing plaintext secrets.
    */
   private async encryptConfig(config: ParsedServerConfig): Promise<ParsedServerConfig> {
@@ -522,12 +562,41 @@ export class ServerConfigsDB implements IServerConfigsRepositoryInterface {
       }
     }
 
+    const secretHeaderKeys = result.secretHeaderKeys;
+    if (secretHeaderKeys?.length) {
+      const resultWithHeaders = result as ParsedServerConfig & { headers?: Record<string, string> };
+      if (resultWithHeaders.headers) {
+        const encryptedHeaders = { ...resultWithHeaders.headers };
+        // Deduplicate to avoid double-encryption
+        const uniqueSecretKeys = Array.from(new Set(secretHeaderKeys));
+        for (const secretKey of uniqueSecretKeys) {
+          const val = encryptedHeaders[secretKey];
+          if (val) {
+            try {
+              encryptedHeaders[secretKey] = await encryptV2(val);
+            } catch (error) {
+              logger.error(
+                `[ServerConfigsDB.encryptConfig] Failed to encrypt header "${secretKey}"`,
+                error,
+              );
+              throw new Error('Failed to encrypt MCP server configuration');
+            }
+          }
+        }
+        resultWithHeaders.headers = encryptedHeaders;
+        // Normalize secretHeaderKeys to the deduped list for consistency
+        result.secretHeaderKeys = uniqueSecretKeys;
+        result = resultWithHeaders as ParsedServerConfig;
+      }
+    }
+
     return result;
   }
 
   /**
    * Decrypts sensitive fields in config after database retrieval.
-   * Decrypts oauth.client_secret and apiKey.key (when source === 'admin').
+   * Decrypts oauth.client_secret, apiKey.key (when source === 'admin'),
+   * and header values listed in secretHeaderKeys.
    * Returns config without secret on failure (graceful degradation).
    */
   private async decryptConfig(config: ParsedServerConfig): Promise<ParsedServerConfig> {
@@ -574,6 +643,34 @@ export class ServerConfigsDB implements IServerConfigsRepositoryInterface {
       }
     }
 
+    const secretHeaderKeys = result.secretHeaderKeys;
+    if (secretHeaderKeys?.length) {
+      const resultWithHeaders = result as ParsedServerConfig & { headers?: Record<string, string> };
+      if (resultWithHeaders.headers) {
+        const decryptedHeaders = { ...resultWithHeaders.headers };
+        // Deduplicate to avoid double-decryption
+        const uniqueSecretKeys = Array.from(new Set(secretHeaderKeys));
+        for (const secretKey of uniqueSecretKeys) {
+          const val = decryptedHeaders[secretKey];
+          if (val) {
+            try {
+              decryptedHeaders[secretKey] = await decryptV2(val);
+            } catch (error) {
+              logger.warn(
+                `[ServerConfigsDB.decryptConfig] Failed to decrypt header "${secretKey}", returning empty value`,
+                error,
+              );
+              decryptedHeaders[secretKey] = '';
+            }
+          }
+        }
+        resultWithHeaders.headers = decryptedHeaders;
+        // Normalize secretHeaderKeys to the deduped list for consistency
+        result.secretHeaderKeys = uniqueSecretKeys;
+        result = resultWithHeaders as ParsedServerConfig;
+      }
+    }
+
     return result;
   }
 }

packages/api/src/mcp/types/index.ts

@@ -168,6 +168,12 @@ export type ParsedServerConfig = MCPOptions & {
   consumeOnly?: boolean;
   /** True when inspection failed at startup; the server is known but not fully initialized */
   inspectionFailed?: boolean;
+  /**
+   * Keys in `headers` whose values are encrypted at rest for DB-stored server configs
+   * (and masked in API responses); YAML-defined configs remain plaintext on disk and
+   * are not exposed via API responses.
+   */
+  secretHeaderKeys?: string[];
 };
 
 export type AddServerResult = {

packages/api/src/mcp/utils.ts

@@ -61,6 +61,38 @@ export function redactServerSecrets(config: ParsedServerConfig): Partial<ParsedS
     safe.oauth = safeOAuth;
   }
 
+  //
+  // Only DB-backed configs should have headers round-trip through the API. YAML configs
+  // (even those with secretHeaderKeys via SSEOptionsSchema/StreamableHTTPOptionsSchema)
+  // are excluded to prevent exposing env-resolved header values in API responses.
+  const rawHeaders = (config as ParsedServerConfig & { headers?: Record<string, string> }).headers;
+  const hasHeaders = !!rawHeaders && Object.keys(rawHeaders).length > 0;
+  const isDbBacked = config.dbId !== undefined;
+  const isLegacyDbHeaders = isDbBacked && hasHeaders && config.secretHeaderKeys === undefined;
+  // Treat as "UI-managed" only if DB-backed AND has headers or secretHeaderKeys.
+  const isUiManaged = isDbBacked && (config.secretHeaderKeys !== undefined || hasHeaders);
+
+  if (isUiManaged && rawHeaders) {
+    const maskedHeaders: Record<string, string> = {};
+    if (isLegacyDbHeaders) {
+      // Legacy DB configs with headers but no secretHeaderKeys are treated as non-secret
+      // headers to avoid data loss on edit/save round-trips. Pass headers through unchanged
+      // and intentionally do NOT emit secretHeaderKeys so the client doesn't treat them as secrets.
+      for (const [k, v] of Object.entries(rawHeaders)) {
+        maskedHeaders[k] = v;
+      }
+      (safe as ParsedServerConfig & { headers?: Record<string, string> }).headers = maskedHeaders;
+    } else {
+      // Modern configs with secretHeaderKeys: mask secret values, pass through non-secrets.
+      const secretKeys = new Set(config.secretHeaderKeys ?? []);
+      for (const [k, v] of Object.entries(rawHeaders)) {
+        maskedHeaders[k] = secretKeys.has(k) ? '' : v;
+      }
+      (safe as ParsedServerConfig & { headers?: Record<string, string> }).headers = maskedHeaders;
+      safe.secretHeaderKeys = config.secretHeaderKeys ?? [];
+    }
+  }
+
   return Object.fromEntries(
     Object.entries(safe).filter(([, v]) => v !== undefined),
   ) as Partial<ParsedServerConfig>;