Skip to content

Commit 29fe12f

Browse files
chrstnbgemini-cli-robot
authored andcommitted
Fix extension MCP server env var loading (#20374)
# Conflicts: # packages/core/src/tools/mcp-client.test.ts # packages/core/src/tools/mcp-client.ts
1 parent 176b2ba commit 29fe12f

3 files changed

Lines changed: 206 additions & 3 deletions

File tree

packages/core/src/tools/mcp-client.test.ts

Lines changed: 111 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1696,6 +1696,117 @@ describe('mcp-client', () => {
16961696
expect(callArgs.env!['GEMINI_CLI_EXT_VAR']).toBeUndefined();
16971697
});
16981698

1699+
<<<<<<< HEAD
1700+
=======
1701+
it('should include extension settings with defined values in environment', async () => {
1702+
const mockedTransport = vi
1703+
.spyOn(SdkClientStdioLib, 'StdioClientTransport')
1704+
.mockReturnValue({} as SdkClientStdioLib.StdioClientTransport);
1705+
1706+
await createTransport(
1707+
'test-server',
1708+
{
1709+
command: 'test-command',
1710+
extension: {
1711+
name: 'test-ext',
1712+
resolvedSettings: [
1713+
{
1714+
envVar: 'GEMINI_CLI_EXT_VAR',
1715+
value: 'defined-value',
1716+
sensitive: false,
1717+
name: 'ext-setting',
1718+
},
1719+
],
1720+
version: '',
1721+
isActive: false,
1722+
path: '',
1723+
contextFiles: [],
1724+
id: '',
1725+
},
1726+
},
1727+
false,
1728+
EMPTY_CONFIG,
1729+
);
1730+
1731+
const callArgs = mockedTransport.mock.calls[0][0];
1732+
expect(callArgs.env).toBeDefined();
1733+
expect(callArgs.env!['GEMINI_CLI_EXT_VAR']).toBe('defined-value');
1734+
});
1735+
1736+
it('should resolve environment variables in mcpServerConfig.env using extension settings', async () => {
1737+
const mockedTransport = vi
1738+
.spyOn(SdkClientStdioLib, 'StdioClientTransport')
1739+
.mockReturnValue({} as SdkClientStdioLib.StdioClientTransport);
1740+
1741+
await createTransport(
1742+
'test-server',
1743+
{
1744+
command: 'test-command',
1745+
env: {
1746+
RESOLVED_VAR: '$GEMINI_CLI_EXT_VAR',
1747+
},
1748+
extension: {
1749+
name: 'test-ext',
1750+
resolvedSettings: [
1751+
{
1752+
envVar: 'GEMINI_CLI_EXT_VAR',
1753+
value: 'ext-value',
1754+
sensitive: false,
1755+
name: 'ext-setting',
1756+
},
1757+
],
1758+
version: '',
1759+
isActive: false,
1760+
path: '',
1761+
contextFiles: [],
1762+
id: '',
1763+
},
1764+
},
1765+
false,
1766+
EMPTY_CONFIG,
1767+
);
1768+
1769+
const callArgs = mockedTransport.mock.calls[0][0];
1770+
expect(callArgs.env).toBeDefined();
1771+
expect(callArgs.env!['GEMINI_CLI_EXT_VAR']).toBe('ext-value');
1772+
expect(callArgs.env!['RESOLVED_VAR']).toBe('ext-value');
1773+
});
1774+
1775+
it('should expand environment variables in mcpServerConfig.env and not redact them', async () => {
1776+
const mockedTransport = vi
1777+
.spyOn(SdkClientStdioLib, 'StdioClientTransport')
1778+
.mockReturnValue({} as SdkClientStdioLib.StdioClientTransport);
1779+
1780+
const originalEnv = process.env;
1781+
process.env = {
1782+
...originalEnv,
1783+
GEMINI_TEST_VAR: 'expanded-value',
1784+
};
1785+
1786+
try {
1787+
await createTransport(
1788+
'test-server',
1789+
{
1790+
command: 'test-command',
1791+
env: {
1792+
TEST_EXPANDED: 'Value is $GEMINI_TEST_VAR',
1793+
SECRET_KEY: 'intentional-secret-123',
1794+
},
1795+
},
1796+
false,
1797+
EMPTY_CONFIG,
1798+
);
1799+
1800+
const callArgs = mockedTransport.mock.calls[0][0];
1801+
expect(callArgs.env).toBeDefined();
1802+
expect(callArgs.env!['TEST_EXPANDED']).toBe('Value is expanded-value');
1803+
expect(callArgs.env!['SECRET_KEY']).toBe('intentional-secret-123');
1804+
} finally {
1805+
process.env = originalEnv;
1806+
}
1807+
});
1808+
1809+
>>>>>>> 58df1c623 (Fix extension MCP server env var loading (#20374))
16991810
describe('useGoogleCredentialProvider', () => {
17001811
beforeEach(() => {
17011812
// Mock GoogleAuth client

packages/core/src/tools/mcp-client.ts

Lines changed: 86 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,11 @@ import {
3434
} from '@modelcontextprotocol/sdk/types.js';
3535
import { ApprovalMode, PolicyDecision } from '../policy/types.js';
3636
import { parse } from 'shell-quote';
37-
import type { Config, MCPServerConfig } from '../config/config.js';
37+
import type {
38+
Config,
39+
MCPServerConfig,
40+
GeminiCLIExtension,
41+
} from '../config/config.js';
3842
import { AuthProviderType } from '../config/config.js';
3943
import { GoogleCredentialProvider } from '../mcp/google-auth-provider.js';
4044
import { ServiceAccountImpersonationProvider } from '../mcp/sa-impersonation-provider.js';
@@ -727,11 +731,31 @@ async function handleAutomaticOAuth(
727731
*
728732
* @param mcpServerConfig The MCP server configuration
729733
* @param headers Additional headers
734+
* @param sanitizationConfig Configuration for environment sanitization
730735
*/
731736
function createTransportRequestInit(
732737
mcpServerConfig: MCPServerConfig,
733738
headers: Record<string, string>,
739+
sanitizationConfig: EnvironmentSanitizationConfig,
734740
): RequestInit {
741+
<<<<<<< HEAD
742+
=======
743+
const extensionEnv = getExtensionEnvironment(mcpServerConfig.extension);
744+
const expansionEnv = { ...process.env, ...extensionEnv };
745+
746+
const sanitizedEnv = sanitizeEnvironment(expansionEnv, {
747+
...sanitizationConfig,
748+
enableEnvironmentVariableRedaction: true,
749+
});
750+
751+
const expandedHeaders: Record<string, string> = {};
752+
if (mcpServerConfig.headers) {
753+
for (const [key, value] of Object.entries(mcpServerConfig.headers)) {
754+
expandedHeaders[key] = expandEnvVars(value, sanitizedEnv);
755+
}
756+
}
757+
758+
>>>>>>> 58df1c623 (Fix extension MCP server env var loading (#20374))
735759
return {
736760
headers: {
737761
...mcpServerConfig.headers,
@@ -768,12 +792,14 @@ function createAuthProvider(
768792
* @param mcpServerName The name of the MCP server
769793
* @param mcpServerConfig The MCP server configuration
770794
* @param accessToken The OAuth access token
795+
* @param sanitizationConfig Configuration for environment sanitization
771796
* @returns The transport with OAuth token, or null if creation fails
772797
*/
773798
async function createTransportWithOAuth(
774799
mcpServerName: string,
775800
mcpServerConfig: MCPServerConfig,
776801
accessToken: string,
802+
sanitizationConfig: EnvironmentSanitizationConfig,
777803
): Promise<StreamableHTTPClientTransport | SSEClientTransport | null> {
778804
try {
779805
const headers: Record<string, string> = {
@@ -782,7 +808,11 @@ async function createTransportWithOAuth(
782808
const transportOptions:
783809
| StreamableHTTPClientTransportOptions
784810
| SSEClientTransportOptions = {
785-
requestInit: createTransportRequestInit(mcpServerConfig, headers),
811+
requestInit: createTransportRequestInit(
812+
mcpServerConfig,
813+
headers,
814+
sanitizationConfig,
815+
),
786816
};
787817

788818
return createUrlTransport(mcpServerName, mcpServerConfig, transportOptions);
@@ -1366,13 +1396,15 @@ async function showAuthRequiredMessage(serverName: string): Promise<never> {
13661396
* @param config The MCP server configuration
13671397
* @param accessToken The OAuth access token to use
13681398
* @param httpReturned404 Whether the HTTP transport returned 404 (indicating SSE-only server)
1399+
* @param sanitizationConfig Configuration for environment sanitization
13691400
*/
13701401
async function retryWithOAuth(
13711402
client: Client,
13721403
serverName: string,
13731404
config: MCPServerConfig,
13741405
accessToken: string,
13751406
httpReturned404: boolean,
1407+
sanitizationConfig: EnvironmentSanitizationConfig,
13761408
): Promise<void> {
13771409
if (httpReturned404) {
13781410
// HTTP returned 404, only try SSE
@@ -1393,6 +1425,7 @@ async function retryWithOAuth(
13931425
serverName,
13941426
config,
13951427
accessToken,
1428+
sanitizationConfig,
13961429
);
13971430
if (!httpTransport) {
13981431
throw new Error(
@@ -1672,6 +1705,7 @@ export async function connectToMcpServer(
16721705
mcpServerConfig,
16731706
accessToken,
16741707
httpReturned404,
1708+
sanitizationConfig,
16751709
);
16761710
return mcpClient;
16771711
} else {
@@ -1743,6 +1777,7 @@ export async function connectToMcpServer(
17431777
mcpServerName,
17441778
mcpServerConfig,
17451779
accessToken,
1780+
sanitizationConfig,
17461781
);
17471782
if (!oauthTransport) {
17481783
throw new Error(
@@ -1890,14 +1925,48 @@ export async function createTransport(
18901925
const transportOptions:
18911926
| StreamableHTTPClientTransportOptions
18921927
| SSEClientTransportOptions = {
1893-
requestInit: createTransportRequestInit(mcpServerConfig, headers),
1928+
requestInit: createTransportRequestInit(
1929+
mcpServerConfig,
1930+
headers,
1931+
sanitizationConfig,
1932+
),
18941933
authProvider,
18951934
};
18961935

18971936
return createUrlTransport(mcpServerName, mcpServerConfig, transportOptions);
18981937
}
18991938

19001939
if (mcpServerConfig.command) {
1940+
<<<<<<< HEAD
1941+
=======
1942+
const extensionEnv = getExtensionEnvironment(mcpServerConfig.extension);
1943+
const expansionEnv = { ...process.env, ...extensionEnv };
1944+
1945+
// 1. Sanitize the base process environment to prevent unintended leaks of system-wide secrets.
1946+
const sanitizedEnv = sanitizeEnvironment(expansionEnv, {
1947+
...sanitizationConfig,
1948+
enableEnvironmentVariableRedaction: true,
1949+
});
1950+
1951+
const finalEnv: Record<string, string> = {
1952+
[GEMINI_CLI_IDENTIFICATION_ENV_VAR]:
1953+
GEMINI_CLI_IDENTIFICATION_ENV_VAR_VALUE,
1954+
...extensionEnv,
1955+
};
1956+
for (const [key, value] of Object.entries(sanitizedEnv)) {
1957+
if (value !== undefined) {
1958+
finalEnv[key] = value;
1959+
}
1960+
}
1961+
1962+
// Expand and merge explicit environment variables from the MCP configuration.
1963+
if (mcpServerConfig.env) {
1964+
for (const [key, value] of Object.entries(mcpServerConfig.env)) {
1965+
finalEnv[key] = expandEnvVars(value, expansionEnv);
1966+
}
1967+
}
1968+
1969+
>>>>>>> 58df1c623 (Fix extension MCP server env var loading (#20374))
19011970
let transport: Transport = new StdioClientTransport({
19021971
command: mcpServerConfig.command,
19031972
args: mcpServerConfig.args || [],
@@ -1955,6 +2024,20 @@ interface NamedTool {
19552024
name?: string;
19562025
}
19572026

2027+
function getExtensionEnvironment(
2028+
extension?: GeminiCLIExtension,
2029+
): Record<string, string> {
2030+
const env: Record<string, string> = {};
2031+
if (extension?.resolvedSettings) {
2032+
for (const setting of extension.resolvedSettings) {
2033+
if (setting.value !== undefined) {
2034+
env[setting.envVar] = setting.value;
2035+
}
2036+
}
2037+
}
2038+
return env;
2039+
}
2040+
19582041
/** Visible for testing */
19592042
export function isEnabled(
19602043
funcDecl: NamedTool,

packages/devtools/src/_client-assets.ts

Lines changed: 9 additions & 0 deletions
Large diffs are not rendered by default.

0 commit comments

Comments
 (0)