mirror of
https://github.com/google-gemini/gemini-cli.git
synced 2026-08-01 04:31:03 -07:00
Compare commits
18 Commits
| Author | SHA1 | Date | |
|---|---|---|---|
| 94ccdc75c2 | |||
| 5e80faf389 | |||
| c2e92125c3 | |||
| 302b48a0b3 | |||
| 8fa4585ebf | |||
| 5d2983f12a | |||
| 9912ea8686 | |||
| 508aaeda89 | |||
| a6256cdabf | |||
| c316fc6c51 | |||
| 7e6e40c814 | |||
| b25c8137f6 | |||
| 03a8fc3113 | |||
| 393e9a171f | |||
| 59702f913d | |||
| b3439e1458 | |||
| b21b289e26 | |||
| 56a63a35e2 |
@@ -124,7 +124,9 @@ jobs:
|
||||
- name: 'Link Checker'
|
||||
uses: 'lycheeverse/lychee-action@885c65f3dc543b57c898c8099f4e08c8afd178a2' # ratchet: lycheeverse/lychee-action@v2.6.1
|
||||
with:
|
||||
args: '--verbose --accept 200,503 ./**/*.md'
|
||||
# Exclude GEMINI.md because the absolute GitHub URL in CONTRIBUTING.md (which is symlinked)
|
||||
# causes intermittent 429 Too Many Requests errors from GitHub API rate limits.
|
||||
args: '--verbose --accept 200,503 --exclude "GEMINI\\.md" ./**/*.md'
|
||||
fail: true
|
||||
test_linux:
|
||||
name: 'Test (Linux) - ${{ matrix.node-version }}, ${{ matrix.shard }}'
|
||||
|
||||
@@ -22,4 +22,6 @@ jobs:
|
||||
id: 'lychee'
|
||||
uses: 'lycheeverse/lychee-action@885c65f3dc543b57c898c8099f4e08c8afd178a2' # ratchet: lycheeverse/lychee-action@v2.6.1
|
||||
with:
|
||||
args: '--verbose --no-progress --accept 200,503 ./**/*.md'
|
||||
# Exclude GEMINI.md because the absolute GitHub URL in CONTRIBUTING.md (which is symlinked)
|
||||
# causes intermittent 429 Too Many Requests errors from GitHub API rate limits.
|
||||
args: '--verbose --no-progress --accept 200,503 --exclude "GEMINI\\.md" ./**/*.md'
|
||||
|
||||
@@ -55,7 +55,7 @@ describe.skip('ACP Environment and Auth', () => {
|
||||
|
||||
const bundlePath = join(import.meta.dirname, '..', 'bundle/gemini.js');
|
||||
|
||||
child = spawn('node', [bundlePath, '--experimental-acp'], {
|
||||
child = spawn('node', [bundlePath, '--acp'], {
|
||||
cwd: rig.homeDir!,
|
||||
stdio: ['pipe', 'pipe', 'inherit'],
|
||||
env: {
|
||||
@@ -120,7 +120,7 @@ describe.skip('ACP Environment and Auth', () => {
|
||||
|
||||
const bundlePath = join(import.meta.dirname, '..', 'bundle/gemini.js');
|
||||
|
||||
child = spawn('node', [bundlePath, '--experimental-acp'], {
|
||||
child = spawn('node', [bundlePath, '--acp'], {
|
||||
cwd: rig.homeDir!,
|
||||
stdio: ['pipe', 'pipe', 'inherit'],
|
||||
env: {
|
||||
|
||||
@@ -58,7 +58,7 @@ describe('ACP telemetry', () => {
|
||||
'node',
|
||||
[
|
||||
bundlePath,
|
||||
'--experimental-acp',
|
||||
'--acp',
|
||||
'--fake-responses',
|
||||
join(rig.testDir!, 'fake-responses.json'),
|
||||
],
|
||||
|
||||
@@ -104,7 +104,7 @@ describe('extension reloading', () => {
|
||||
return (
|
||||
output.includes(
|
||||
'test-server (from test-extension) - Ready (1 tool)',
|
||||
) && output.includes('- hello')
|
||||
) && output.includes('- mcp_test-server_hello')
|
||||
);
|
||||
},
|
||||
30000, // 30s timeout
|
||||
@@ -148,7 +148,7 @@ describe('extension reloading', () => {
|
||||
return (
|
||||
output.includes(
|
||||
'test-server (from test-extension) - Ready (1 tool)',
|
||||
) && output.includes('- goodbye')
|
||||
) && output.includes('- mcp_test-server_goodbye')
|
||||
);
|
||||
},
|
||||
30000,
|
||||
|
||||
Generated
+9
-9
@@ -1,12 +1,12 @@
|
||||
{
|
||||
"name": "@google/gemini-cli",
|
||||
"version": "0.33.0-nightly.20260228.1ca5c05d0",
|
||||
"version": "0.33.0-preview.9",
|
||||
"lockfileVersion": 3,
|
||||
"requires": true,
|
||||
"packages": {
|
||||
"": {
|
||||
"name": "@google/gemini-cli",
|
||||
"version": "0.33.0-nightly.20260228.1ca5c05d0",
|
||||
"version": "0.33.0-preview.9",
|
||||
"workspaces": [
|
||||
"packages/*"
|
||||
],
|
||||
@@ -17303,7 +17303,7 @@
|
||||
},
|
||||
"packages/a2a-server": {
|
||||
"name": "@google/gemini-cli-a2a-server",
|
||||
"version": "0.33.0-nightly.20260228.1ca5c05d0",
|
||||
"version": "0.33.0-preview.9",
|
||||
"dependencies": {
|
||||
"@a2a-js/sdk": "^0.3.8",
|
||||
"@google-cloud/storage": "^7.16.0",
|
||||
@@ -17361,7 +17361,7 @@
|
||||
},
|
||||
"packages/cli": {
|
||||
"name": "@google/gemini-cli",
|
||||
"version": "0.33.0-nightly.20260228.1ca5c05d0",
|
||||
"version": "0.33.0-preview.9",
|
||||
"license": "Apache-2.0",
|
||||
"dependencies": {
|
||||
"@agentclientprotocol/sdk": "^0.12.0",
|
||||
@@ -17444,7 +17444,7 @@
|
||||
},
|
||||
"packages/core": {
|
||||
"name": "@google/gemini-cli-core",
|
||||
"version": "0.33.0-nightly.20260228.1ca5c05d0",
|
||||
"version": "0.33.0-preview.9",
|
||||
"license": "Apache-2.0",
|
||||
"dependencies": {
|
||||
"@a2a-js/sdk": "^0.3.8",
|
||||
@@ -17709,7 +17709,7 @@
|
||||
},
|
||||
"packages/devtools": {
|
||||
"name": "@google/gemini-cli-devtools",
|
||||
"version": "0.33.0-nightly.20260228.1ca5c05d0",
|
||||
"version": "0.33.0-preview.9",
|
||||
"license": "Apache-2.0",
|
||||
"dependencies": {
|
||||
"ws": "^8.16.0"
|
||||
@@ -17724,7 +17724,7 @@
|
||||
},
|
||||
"packages/sdk": {
|
||||
"name": "@google/gemini-cli-sdk",
|
||||
"version": "0.33.0-nightly.20260228.1ca5c05d0",
|
||||
"version": "0.33.0-preview.9",
|
||||
"license": "Apache-2.0",
|
||||
"dependencies": {
|
||||
"@google/gemini-cli-core": "file:../core",
|
||||
@@ -17741,7 +17741,7 @@
|
||||
},
|
||||
"packages/test-utils": {
|
||||
"name": "@google/gemini-cli-test-utils",
|
||||
"version": "0.33.0-nightly.20260228.1ca5c05d0",
|
||||
"version": "0.33.0-preview.9",
|
||||
"license": "Apache-2.0",
|
||||
"dependencies": {
|
||||
"@google/gemini-cli-core": "file:../core",
|
||||
@@ -17758,7 +17758,7 @@
|
||||
},
|
||||
"packages/vscode-ide-companion": {
|
||||
"name": "gemini-cli-vscode-ide-companion",
|
||||
"version": "0.33.0-nightly.20260228.1ca5c05d0",
|
||||
"version": "0.33.0-preview.9",
|
||||
"license": "LICENSE",
|
||||
"dependencies": {
|
||||
"@modelcontextprotocol/sdk": "^1.23.0",
|
||||
|
||||
+2
-2
@@ -1,6 +1,6 @@
|
||||
{
|
||||
"name": "@google/gemini-cli",
|
||||
"version": "0.33.0-nightly.20260228.1ca5c05d0",
|
||||
"version": "0.33.0-preview.9",
|
||||
"engines": {
|
||||
"node": ">=20.0.0"
|
||||
},
|
||||
@@ -14,7 +14,7 @@
|
||||
"url": "git+https://github.com/google-gemini/gemini-cli.git"
|
||||
},
|
||||
"config": {
|
||||
"sandboxImageUri": "us-docker.pkg.dev/gemini-code-dev/gemini-cli/sandbox:0.33.0-nightly.20260228.1ca5c05d0"
|
||||
"sandboxImageUri": "us-docker.pkg.dev/gemini-code-dev/gemini-cli/sandbox:0.33.0-preview.9"
|
||||
},
|
||||
"scripts": {
|
||||
"start": "cross-env NODE_ENV=development node scripts/start.js",
|
||||
|
||||
@@ -1,6 +1,6 @@
|
||||
{
|
||||
"name": "@google/gemini-cli-a2a-server",
|
||||
"version": "0.33.0-nightly.20260228.1ca5c05d0",
|
||||
"version": "0.33.0-preview.9",
|
||||
"description": "Gemini CLI A2A Server",
|
||||
"repository": {
|
||||
"type": "git",
|
||||
|
||||
@@ -1,6 +1,6 @@
|
||||
{
|
||||
"name": "@google/gemini-cli",
|
||||
"version": "0.33.0-nightly.20260228.1ca5c05d0",
|
||||
"version": "0.33.0-preview.9",
|
||||
"description": "Gemini CLI",
|
||||
"license": "Apache-2.0",
|
||||
"repository": {
|
||||
@@ -26,7 +26,7 @@
|
||||
"dist"
|
||||
],
|
||||
"config": {
|
||||
"sandboxImageUri": "us-docker.pkg.dev/gemini-code-dev/gemini-cli/sandbox:0.33.0-nightly.20260228.1ca5c05d0"
|
||||
"sandboxImageUri": "us-docker.pkg.dev/gemini-code-dev/gemini-cli/sandbox:0.33.0-preview.9"
|
||||
},
|
||||
"dependencies": {
|
||||
"@agentclientprotocol/sdk": "^0.12.0",
|
||||
|
||||
+54
-2
@@ -14,7 +14,7 @@ import {
|
||||
type Mock,
|
||||
type Mocked,
|
||||
} from 'vitest';
|
||||
import { GeminiAgent, Session } from './zedIntegration.js';
|
||||
import { GeminiAgent, Session } from './acpClient.js';
|
||||
import type { CommandHandler } from './commandHandler.js';
|
||||
import * as acp from '@agentclientprotocol/sdk';
|
||||
import {
|
||||
@@ -208,7 +208,16 @@ describe('GeminiAgent', () => {
|
||||
});
|
||||
|
||||
expect(response.protocolVersion).toBe(acp.PROTOCOL_VERSION);
|
||||
expect(response.authMethods).toHaveLength(3);
|
||||
expect(response.authMethods).toHaveLength(4);
|
||||
const gatewayAuth = response.authMethods?.find(
|
||||
(m) => m.id === AuthType.GATEWAY,
|
||||
);
|
||||
expect(gatewayAuth?._meta).toEqual({
|
||||
gateway: {
|
||||
protocol: 'google',
|
||||
restartRequired: 'false',
|
||||
},
|
||||
});
|
||||
const geminiAuth = response.authMethods?.find(
|
||||
(m) => m.id === AuthType.USE_GEMINI,
|
||||
);
|
||||
@@ -228,6 +237,8 @@ describe('GeminiAgent', () => {
|
||||
expect(mockConfig.refreshAuth).toHaveBeenCalledWith(
|
||||
AuthType.LOGIN_WITH_GOOGLE,
|
||||
undefined,
|
||||
undefined,
|
||||
undefined,
|
||||
);
|
||||
expect(mockSettings.setValue).toHaveBeenCalledWith(
|
||||
SettingScope.User,
|
||||
@@ -247,6 +258,8 @@ describe('GeminiAgent', () => {
|
||||
expect(mockConfig.refreshAuth).toHaveBeenCalledWith(
|
||||
AuthType.USE_GEMINI,
|
||||
'test-api-key',
|
||||
undefined,
|
||||
undefined,
|
||||
);
|
||||
expect(mockSettings.setValue).toHaveBeenCalledWith(
|
||||
SettingScope.User,
|
||||
@@ -255,6 +268,45 @@ describe('GeminiAgent', () => {
|
||||
);
|
||||
});
|
||||
|
||||
it('should authenticate correctly with gateway method', async () => {
|
||||
await agent.authenticate({
|
||||
methodId: AuthType.GATEWAY,
|
||||
_meta: {
|
||||
gateway: {
|
||||
baseUrl: 'https://example.com',
|
||||
headers: { Authorization: 'Bearer token' },
|
||||
},
|
||||
},
|
||||
} as unknown as acp.AuthenticateRequest);
|
||||
|
||||
expect(mockConfig.refreshAuth).toHaveBeenCalledWith(
|
||||
AuthType.GATEWAY,
|
||||
undefined,
|
||||
'https://example.com',
|
||||
{ Authorization: 'Bearer token' },
|
||||
);
|
||||
expect(mockSettings.setValue).toHaveBeenCalledWith(
|
||||
SettingScope.User,
|
||||
'security.auth.selectedType',
|
||||
AuthType.GATEWAY,
|
||||
);
|
||||
});
|
||||
|
||||
it('should throw acp.RequestError when gateway payload is malformed', async () => {
|
||||
await expect(
|
||||
agent.authenticate({
|
||||
methodId: AuthType.GATEWAY,
|
||||
_meta: {
|
||||
gateway: {
|
||||
// Invalid baseUrl
|
||||
baseUrl: 123,
|
||||
headers: { Authorization: 'Bearer token' },
|
||||
},
|
||||
},
|
||||
} as unknown as acp.AuthenticateRequest),
|
||||
).rejects.toThrow(/Malformed gateway payload/);
|
||||
});
|
||||
|
||||
it('should create a new session', async () => {
|
||||
vi.useFakeTimers();
|
||||
mockConfig.getContentGeneratorConfig = vi.fn().mockReturnValue({
|
||||
+58
-4
@@ -70,7 +70,7 @@ import { runExitCleanup } from '../utils/cleanup.js';
|
||||
import { SessionSelector } from '../utils/sessionUtils.js';
|
||||
|
||||
import { CommandHandler } from './commandHandler.js';
|
||||
export async function runZedIntegration(
|
||||
export async function runAcpClient(
|
||||
config: Config,
|
||||
settings: LoadedSettings,
|
||||
argv: CliArgs,
|
||||
@@ -98,6 +98,8 @@ export class GeminiAgent {
|
||||
private sessions: Map<string, Session> = new Map();
|
||||
private clientCapabilities: acp.ClientCapabilities | undefined;
|
||||
private apiKey: string | undefined;
|
||||
private baseUrl: string | undefined;
|
||||
private customHeaders: Record<string, string> | undefined;
|
||||
|
||||
constructor(
|
||||
private config: Config,
|
||||
@@ -131,6 +133,17 @@ export class GeminiAgent {
|
||||
name: 'Vertex AI',
|
||||
description: 'Use an API key with Vertex AI GenAI API',
|
||||
},
|
||||
{
|
||||
id: AuthType.GATEWAY,
|
||||
name: 'AI API Gateway',
|
||||
description: 'Use a custom AI API Gateway',
|
||||
_meta: {
|
||||
gateway: {
|
||||
protocol: 'google',
|
||||
restartRequired: 'false',
|
||||
},
|
||||
},
|
||||
},
|
||||
];
|
||||
|
||||
await this.config.initialize();
|
||||
@@ -179,7 +192,38 @@ export class GeminiAgent {
|
||||
if (apiKey) {
|
||||
this.apiKey = apiKey;
|
||||
}
|
||||
await this.config.refreshAuth(method, apiKey ?? this.apiKey);
|
||||
|
||||
// Extract gateway details if present
|
||||
const gatewaySchema = z.object({
|
||||
baseUrl: z.string().optional(),
|
||||
headers: z.record(z.string()).optional(),
|
||||
});
|
||||
|
||||
let baseUrl: string | undefined;
|
||||
let headers: Record<string, string> | undefined;
|
||||
|
||||
if (meta?.['gateway']) {
|
||||
const result = gatewaySchema.safeParse(meta['gateway']);
|
||||
if (result.success) {
|
||||
baseUrl = result.data.baseUrl;
|
||||
headers = result.data.headers;
|
||||
} else {
|
||||
throw new acp.RequestError(
|
||||
-32602,
|
||||
`Malformed gateway payload: ${result.error.message}`,
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
this.baseUrl = baseUrl;
|
||||
this.customHeaders = headers;
|
||||
|
||||
await this.config.refreshAuth(
|
||||
method,
|
||||
apiKey ?? this.apiKey,
|
||||
baseUrl,
|
||||
headers,
|
||||
);
|
||||
} catch (e) {
|
||||
throw new acp.RequestError(-32000, getAcpErrorMessage(e));
|
||||
}
|
||||
@@ -209,7 +253,12 @@ export class GeminiAgent {
|
||||
let isAuthenticated = false;
|
||||
let authErrorMessage = '';
|
||||
try {
|
||||
await config.refreshAuth(authType, this.apiKey);
|
||||
await config.refreshAuth(
|
||||
authType,
|
||||
this.apiKey,
|
||||
this.baseUrl,
|
||||
this.customHeaders,
|
||||
);
|
||||
isAuthenticated = true;
|
||||
|
||||
// Extra validation for Gemini API key
|
||||
@@ -371,7 +420,12 @@ export class GeminiAgent {
|
||||
// This satisfies the security requirement to verify the user before executing
|
||||
// potentially unsafe server definitions.
|
||||
try {
|
||||
await config.refreshAuth(selectedAuthType, this.apiKey);
|
||||
await config.refreshAuth(
|
||||
selectedAuthType,
|
||||
this.apiKey,
|
||||
this.baseUrl,
|
||||
this.customHeaders,
|
||||
);
|
||||
} catch (e) {
|
||||
debugLogger.error(`Authentication failed: ${e}`);
|
||||
throw acp.RequestError.authRequired();
|
||||
+1
-1
@@ -13,7 +13,7 @@ import {
|
||||
type Mocked,
|
||||
type Mock,
|
||||
} from 'vitest';
|
||||
import { GeminiAgent } from './zedIntegration.js';
|
||||
import { GeminiAgent } from './acpClient.js';
|
||||
import * as acp from '@agentclientprotocol/sdk';
|
||||
import {
|
||||
ApprovalMode,
|
||||
@@ -81,7 +81,8 @@ export interface CliArgs {
|
||||
policy: string[] | undefined;
|
||||
allowedMcpServerNames: string[] | undefined;
|
||||
allowedTools: string[] | undefined;
|
||||
experimentalAcp: boolean | undefined;
|
||||
acp?: boolean;
|
||||
experimentalAcp?: boolean;
|
||||
extensions: string[] | undefined;
|
||||
listExtensions: boolean | undefined;
|
||||
resume: string | typeof RESUME_LATEST | undefined;
|
||||
@@ -177,10 +178,15 @@ export async function parseArguments(
|
||||
.filter(Boolean),
|
||||
),
|
||||
})
|
||||
.option('experimental-acp', {
|
||||
.option('acp', {
|
||||
type: 'boolean',
|
||||
description: 'Starts the agent in ACP mode',
|
||||
})
|
||||
.option('experimental-acp', {
|
||||
type: 'boolean',
|
||||
description:
|
||||
'Starts the agent in ACP mode (deprecated, use --acp instead)',
|
||||
})
|
||||
.option('allowed-mcp-server-names', {
|
||||
type: 'array',
|
||||
string: true,
|
||||
@@ -632,6 +638,7 @@ export async function loadCliConfig(
|
||||
// -i/--prompt-interactive forces interactive mode with an initial prompt
|
||||
const interactive =
|
||||
!!argv.promptInteractive ||
|
||||
!!argv.acp ||
|
||||
!!argv.experimentalAcp ||
|
||||
(!isHeadlessMode({ prompt: argv.prompt, query: argv.query }) &&
|
||||
!argv.isCommand);
|
||||
@@ -758,6 +765,7 @@ export async function loadCliConfig(
|
||||
}
|
||||
|
||||
return new Config({
|
||||
acpMode: !!argv.acp || !!argv.experimentalAcp,
|
||||
sessionId,
|
||||
clientVersion: await getVersion(),
|
||||
embeddingModel: DEFAULT_GEMINI_EMBEDDING_MODEL,
|
||||
@@ -821,7 +829,7 @@ export async function loadCliConfig(
|
||||
bugCommand: settings.advanced?.bugCommand,
|
||||
model: resolvedModel,
|
||||
maxSessionTurns: settings.model?.maxSessionTurns,
|
||||
experimentalZedIntegration: argv.experimentalAcp || false,
|
||||
|
||||
listExtensions: argv.listExtensions || false,
|
||||
listSessions: argv.listSessions || false,
|
||||
deleteSession: argv.deleteSession,
|
||||
@@ -867,6 +875,7 @@ export async function loadCliConfig(
|
||||
fakeResponses: argv.fakeResponses,
|
||||
recordResponses: argv.recordResponses,
|
||||
retryFetchErrors: settings.general?.retryFetchErrors,
|
||||
billing: settings.billing,
|
||||
maxAttempts: settings.general?.maxAttempts,
|
||||
ptyInfo: ptyInfo?.name,
|
||||
disableLLMCorrection: settings.tools?.disableLLMCorrection,
|
||||
|
||||
@@ -89,13 +89,13 @@ describe('Policy Engine Integration Tests', () => {
|
||||
// Tools from allowed server should be allowed
|
||||
// Tools from allowed server should be allowed
|
||||
expect(
|
||||
(await engine.check({ name: 'allowed-server__tool1' }, undefined))
|
||||
(await engine.check({ name: 'mcp_allowed-server_tool1' }, undefined))
|
||||
.decision,
|
||||
).toBe(PolicyDecision.ALLOW);
|
||||
expect(
|
||||
(
|
||||
await engine.check(
|
||||
{ name: 'allowed-server__another_tool' },
|
||||
{ name: 'mcp_allowed-server_another_tool' },
|
||||
undefined,
|
||||
)
|
||||
).decision,
|
||||
@@ -103,13 +103,13 @@ describe('Policy Engine Integration Tests', () => {
|
||||
|
||||
// Tools from trusted server should be allowed
|
||||
expect(
|
||||
(await engine.check({ name: 'trusted-server__tool1' }, undefined))
|
||||
(await engine.check({ name: 'mcp_trusted-server_tool1' }, undefined))
|
||||
.decision,
|
||||
).toBe(PolicyDecision.ALLOW);
|
||||
expect(
|
||||
(
|
||||
await engine.check(
|
||||
{ name: 'trusted-server__special_tool' },
|
||||
{ name: 'mcp_trusted-server_special_tool' },
|
||||
undefined,
|
||||
)
|
||||
).decision,
|
||||
@@ -117,17 +117,17 @@ describe('Policy Engine Integration Tests', () => {
|
||||
|
||||
// Tools from blocked server should be denied
|
||||
expect(
|
||||
(await engine.check({ name: 'blocked-server__tool1' }, undefined))
|
||||
(await engine.check({ name: 'mcp_blocked-server_tool1' }, undefined))
|
||||
.decision,
|
||||
).toBe(PolicyDecision.DENY);
|
||||
expect(
|
||||
(await engine.check({ name: 'blocked-server__any_tool' }, undefined))
|
||||
(await engine.check({ name: 'mcp_blocked-server_any_tool' }, undefined))
|
||||
.decision,
|
||||
).toBe(PolicyDecision.DENY);
|
||||
|
||||
// Tools from unknown servers should use default
|
||||
expect(
|
||||
(await engine.check({ name: 'unknown-server__tool' }, undefined))
|
||||
(await engine.check({ name: 'mcp_unknown-server_tool' }, undefined))
|
||||
.decision,
|
||||
).toBe(PolicyDecision.ASK_USER);
|
||||
});
|
||||
@@ -147,12 +147,16 @@ describe('Policy Engine Integration Tests', () => {
|
||||
|
||||
// ANY tool with a server name should be allowed
|
||||
expect(
|
||||
(await engine.check({ name: 'mcp-server__tool' }, 'mcp-server'))
|
||||
(await engine.check({ name: 'mcp_mcp-server_tool' }, 'mcp-server'))
|
||||
.decision,
|
||||
).toBe(PolicyDecision.ALLOW);
|
||||
expect(
|
||||
(await engine.check({ name: 'another-server__tool' }, 'another-server'))
|
||||
.decision,
|
||||
(
|
||||
await engine.check(
|
||||
{ name: 'mcp_another-server_tool' },
|
||||
'another-server',
|
||||
)
|
||||
).decision,
|
||||
).toBe(PolicyDecision.ALLOW);
|
||||
|
||||
// Built-in tools should NOT be allowed by the MCP wildcard
|
||||
@@ -167,7 +171,7 @@ describe('Policy Engine Integration Tests', () => {
|
||||
allowed: ['my-server'],
|
||||
},
|
||||
tools: {
|
||||
exclude: ['my-server__dangerous-tool'],
|
||||
exclude: ['mcp_my-server_dangerous-tool'],
|
||||
},
|
||||
};
|
||||
|
||||
@@ -180,20 +184,24 @@ describe('Policy Engine Integration Tests', () => {
|
||||
// MCP server allowed (priority 4.1) provides general allow for server
|
||||
// MCP server allowed (priority 4.1) provides general allow for server
|
||||
expect(
|
||||
(await engine.check({ name: 'my-server__safe-tool' }, undefined))
|
||||
(await engine.check({ name: 'mcp_my-server_safe-tool' }, undefined))
|
||||
.decision,
|
||||
).toBe(PolicyDecision.ALLOW);
|
||||
// But specific tool exclude (priority 4.4) wins over server allow
|
||||
expect(
|
||||
(await engine.check({ name: 'my-server__dangerous-tool' }, undefined))
|
||||
.decision,
|
||||
(
|
||||
await engine.check(
|
||||
{ name: 'mcp_my-server_dangerous-tool' },
|
||||
undefined,
|
||||
)
|
||||
).decision,
|
||||
).toBe(PolicyDecision.DENY);
|
||||
});
|
||||
|
||||
it('should handle complex mixed configurations', async () => {
|
||||
const settings: Settings = {
|
||||
tools: {
|
||||
allowed: ['custom-tool', 'my-server__special-tool'],
|
||||
allowed: ['custom-tool', 'mcp_my-server_special-tool'],
|
||||
exclude: ['glob', 'dangerous-tool'],
|
||||
},
|
||||
mcp: {
|
||||
@@ -238,21 +246,21 @@ describe('Policy Engine Integration Tests', () => {
|
||||
(await engine.check({ name: 'custom-tool' }, undefined)).decision,
|
||||
).toBe(PolicyDecision.ALLOW);
|
||||
expect(
|
||||
(await engine.check({ name: 'my-server__special-tool' }, undefined))
|
||||
(await engine.check({ name: 'mcp_my-server_special-tool' }, undefined))
|
||||
.decision,
|
||||
).toBe(PolicyDecision.ALLOW);
|
||||
|
||||
// MCP server tools
|
||||
expect(
|
||||
(await engine.check({ name: 'allowed-server__tool' }, undefined))
|
||||
(await engine.check({ name: 'mcp_allowed-server_tool' }, undefined))
|
||||
.decision,
|
||||
).toBe(PolicyDecision.ALLOW);
|
||||
expect(
|
||||
(await engine.check({ name: 'trusted-server__tool' }, undefined))
|
||||
(await engine.check({ name: 'mcp_trusted-server_tool' }, undefined))
|
||||
.decision,
|
||||
).toBe(PolicyDecision.ALLOW);
|
||||
expect(
|
||||
(await engine.check({ name: 'blocked-server__tool' }, undefined))
|
||||
(await engine.check({ name: 'mcp_blocked-server_tool' }, undefined))
|
||||
.decision,
|
||||
).toBe(PolicyDecision.DENY);
|
||||
|
||||
@@ -479,7 +487,7 @@ describe('Policy Engine Integration Tests', () => {
|
||||
expect(blockedToolRule?.priority).toBe(4.4); // Command line exclude
|
||||
|
||||
const blockedServerRule = rules.find(
|
||||
(r) => r.toolName === 'blocked-server__*',
|
||||
(r) => r.toolName === 'mcp_blocked-server_*',
|
||||
);
|
||||
expect(blockedServerRule?.priority).toBe(4.9); // MCP server exclude
|
||||
|
||||
@@ -489,11 +497,13 @@ describe('Policy Engine Integration Tests', () => {
|
||||
expect(specificToolRule?.priority).toBe(4.3); // Command line allow
|
||||
|
||||
const trustedServerRule = rules.find(
|
||||
(r) => r.toolName === 'trusted-server__*',
|
||||
(r) => r.toolName === 'mcp_trusted-server_*',
|
||||
);
|
||||
expect(trustedServerRule?.priority).toBe(4.2); // MCP trusted server
|
||||
|
||||
const mcpServerRule = rules.find((r) => r.toolName === 'mcp-server__*');
|
||||
const mcpServerRule = rules.find(
|
||||
(r) => r.toolName === 'mcp_mcp-server_*',
|
||||
);
|
||||
expect(mcpServerRule?.priority).toBe(4.1); // MCP allowed server
|
||||
|
||||
const readOnlyToolRule = rules.find((r) => r.toolName === 'glob');
|
||||
@@ -505,18 +515,19 @@ describe('Policy Engine Integration Tests', () => {
|
||||
(await engine.check({ name: 'blocked-tool' }, undefined)).decision,
|
||||
).toBe(PolicyDecision.DENY);
|
||||
expect(
|
||||
(await engine.check({ name: 'blocked-server__any' }, undefined))
|
||||
(await engine.check({ name: 'mcp_blocked-server_any' }, undefined))
|
||||
.decision,
|
||||
).toBe(PolicyDecision.DENY);
|
||||
expect(
|
||||
(await engine.check({ name: 'specific-tool' }, undefined)).decision,
|
||||
).toBe(PolicyDecision.ALLOW);
|
||||
expect(
|
||||
(await engine.check({ name: 'trusted-server__any' }, undefined))
|
||||
(await engine.check({ name: 'mcp_trusted-server_any' }, undefined))
|
||||
.decision,
|
||||
).toBe(PolicyDecision.ALLOW);
|
||||
expect(
|
||||
(await engine.check({ name: 'mcp-server__any' }, undefined)).decision,
|
||||
(await engine.check({ name: 'mcp_mcp-server_any' }, undefined))
|
||||
.decision,
|
||||
).toBe(PolicyDecision.ALLOW);
|
||||
expect((await engine.check({ name: 'glob' }, undefined)).decision).toBe(
|
||||
PolicyDecision.ALLOW,
|
||||
@@ -545,7 +556,7 @@ describe('Policy Engine Integration Tests', () => {
|
||||
|
||||
// Exclusion (195) should win over trust (90)
|
||||
expect(
|
||||
(await engine.check({ name: 'conflicted-server__tool' }, undefined))
|
||||
(await engine.check({ name: 'mcp_conflicted-server_tool' }, undefined))
|
||||
.decision,
|
||||
).toBe(PolicyDecision.DENY);
|
||||
});
|
||||
@@ -556,7 +567,7 @@ describe('Policy Engine Integration Tests', () => {
|
||||
excluded: ['my-server'], // Priority 195 - DENY
|
||||
},
|
||||
tools: {
|
||||
allowed: ['my-server__special-tool'], // Priority 100 - ALLOW
|
||||
allowed: ['mcp_my-server_special-tool'], // Priority 100 - ALLOW
|
||||
},
|
||||
};
|
||||
|
||||
@@ -569,11 +580,11 @@ describe('Policy Engine Integration Tests', () => {
|
||||
// Server exclusion (195) wins over specific tool allow (100)
|
||||
// This might be counterintuitive but follows the priority system
|
||||
expect(
|
||||
(await engine.check({ name: 'my-server__special-tool' }, undefined))
|
||||
(await engine.check({ name: 'mcp_my-server_special-tool' }, undefined))
|
||||
.decision,
|
||||
).toBe(PolicyDecision.DENY);
|
||||
expect(
|
||||
(await engine.check({ name: 'my-server__other-tool' }, undefined))
|
||||
(await engine.check({ name: 'mcp_my-server_other-tool' }, undefined))
|
||||
.decision,
|
||||
).toBe(PolicyDecision.DENY);
|
||||
});
|
||||
@@ -643,13 +654,13 @@ describe('Policy Engine Integration Tests', () => {
|
||||
const tool3Rule = rules.find((r) => r.toolName === 'tool3');
|
||||
expect(tool3Rule?.priority).toBe(4.4); // Excluded tools (user tier)
|
||||
|
||||
const server2Rule = rules.find((r) => r.toolName === 'server2__*');
|
||||
const server2Rule = rules.find((r) => r.toolName === 'mcp_server2_*');
|
||||
expect(server2Rule?.priority).toBe(4.9); // Excluded servers (user tier)
|
||||
|
||||
const tool1Rule = rules.find((r) => r.toolName === 'tool1');
|
||||
expect(tool1Rule?.priority).toBe(4.3); // Allowed tools (user tier)
|
||||
|
||||
const server1Rule = rules.find((r) => r.toolName === 'server1__*');
|
||||
const server1Rule = rules.find((r) => r.toolName === 'mcp_server1_*');
|
||||
expect(server1Rule?.priority).toBe(4.1); // Allowed servers (user tier)
|
||||
|
||||
const globRule = rules.find((r) => r.toolName === 'glob');
|
||||
|
||||
@@ -79,7 +79,7 @@ import {
|
||||
type InitializationResult,
|
||||
} from './core/initializer.js';
|
||||
import { validateAuthMethod } from './config/auth.js';
|
||||
import { runZedIntegration } from './zed-integration/zedIntegration.js';
|
||||
import { runAcpClient } from './acp/acpClient.js';
|
||||
import { validateNonInteractiveAuth } from './validateNonInterActiveAuth.js';
|
||||
import { checkForUpdates } from './ui/utils/updateCheck.js';
|
||||
import { handleAutoUpdate } from './utils/handleAutoUpdate.js';
|
||||
@@ -672,8 +672,8 @@ export async function main() {
|
||||
await getOauthClient(settings.merged.security.auth.selectedType, config);
|
||||
}
|
||||
|
||||
if (config.getExperimentalZedIntegration()) {
|
||||
return runZedIntegration(config, settings, argv);
|
||||
if (config.getAcpMode()) {
|
||||
return runAcpClient(config, settings, argv);
|
||||
}
|
||||
|
||||
let input = config.getQuestion();
|
||||
|
||||
@@ -179,7 +179,7 @@ describe('gemini.tsx main function cleanup', () => {
|
||||
vi.restoreAllMocks();
|
||||
});
|
||||
|
||||
it('should log error when cleanupExpiredSessions fails', async () => {
|
||||
it.skip('should log error when cleanupExpiredSessions fails', async () => {
|
||||
const { loadCliConfig, parseArguments } = await import(
|
||||
'./config/config.js'
|
||||
);
|
||||
@@ -216,7 +216,7 @@ describe('gemini.tsx main function cleanup', () => {
|
||||
getMcpServers: () => ({}),
|
||||
getMcpClientManager: vi.fn(),
|
||||
getIdeMode: vi.fn(() => false),
|
||||
getExperimentalZedIntegration: vi.fn(() => true),
|
||||
getAcpMode: vi.fn(() => true),
|
||||
getScreenReader: vi.fn(() => false),
|
||||
getGeminiMdFileCount: vi.fn(() => 0),
|
||||
getProjectRoot: vi.fn(() => '/'),
|
||||
|
||||
@@ -42,7 +42,7 @@ export const createMockConfig = (overrides: Partial<Config> = {}): Config =>
|
||||
setSessionId: vi.fn(),
|
||||
getSessionId: vi.fn().mockReturnValue('mock-session-id'),
|
||||
getContentGeneratorConfig: vi.fn(() => ({ authType: 'google' })),
|
||||
getExperimentalZedIntegration: vi.fn(() => false),
|
||||
getAcpMode: vi.fn(() => false),
|
||||
isBrowserLaunchSuppressed: vi.fn(() => false),
|
||||
setRemoteAdminSettings: vi.fn(),
|
||||
isYoloModeDisabled: vi.fn(() => false),
|
||||
|
||||
@@ -2544,136 +2544,6 @@ describe('AppContainer State Management', () => {
|
||||
});
|
||||
});
|
||||
|
||||
describe('Expansion Persistence', () => {
|
||||
let rerender: () => void;
|
||||
let unmount: () => void;
|
||||
let stdin: ReturnType<typeof render>['stdin'];
|
||||
|
||||
const setupExpansionPersistenceTest = async (
|
||||
HighPriorityChild?: React.FC,
|
||||
) => {
|
||||
const getTree = () => (
|
||||
<SettingsContext.Provider value={mockSettings}>
|
||||
<KeypressProvider config={mockConfig}>
|
||||
<OverflowProvider>
|
||||
<AppContainer
|
||||
config={mockConfig}
|
||||
version="1.0.0"
|
||||
initializationResult={mockInitResult}
|
||||
/>
|
||||
{HighPriorityChild && <HighPriorityChild />}
|
||||
</OverflowProvider>
|
||||
</KeypressProvider>
|
||||
</SettingsContext.Provider>
|
||||
);
|
||||
|
||||
const renderResult = render(getTree());
|
||||
stdin = renderResult.stdin;
|
||||
await act(async () => {
|
||||
vi.advanceTimersByTime(100);
|
||||
});
|
||||
rerender = () => renderResult.rerender(getTree());
|
||||
unmount = () => renderResult.unmount();
|
||||
};
|
||||
|
||||
const writeStdin = async (sequence: string) => {
|
||||
await act(async () => {
|
||||
stdin.write(sequence);
|
||||
// Advance timers to allow escape sequence parsing and broadcasting
|
||||
vi.advanceTimersByTime(100);
|
||||
});
|
||||
rerender();
|
||||
};
|
||||
|
||||
beforeEach(() => {
|
||||
vi.useFakeTimers();
|
||||
});
|
||||
|
||||
afterEach(() => {
|
||||
vi.useRealTimers();
|
||||
vi.restoreAllMocks();
|
||||
});
|
||||
|
||||
it('should reset expansion when a key is NOT handled by anyone', async () => {
|
||||
await setupExpansionPersistenceTest();
|
||||
|
||||
// Expand first
|
||||
act(() => capturedUIActions.setConstrainHeight(false));
|
||||
rerender();
|
||||
expect(capturedUIState.constrainHeight).toBe(false);
|
||||
|
||||
// Press a random key that no one handles (hits Low priority fallback)
|
||||
await writeStdin('x');
|
||||
|
||||
// Should be reset to true (collapsed)
|
||||
expect(capturedUIState.constrainHeight).toBe(true);
|
||||
|
||||
unmount();
|
||||
});
|
||||
|
||||
it('should toggle expansion when Ctrl+O is pressed', async () => {
|
||||
await setupExpansionPersistenceTest();
|
||||
|
||||
// Initial state is collapsed
|
||||
expect(capturedUIState.constrainHeight).toBe(true);
|
||||
|
||||
// Press Ctrl+O to expand (Ctrl+O is sequence \x0f)
|
||||
await writeStdin('\x0f');
|
||||
expect(capturedUIState.constrainHeight).toBe(false);
|
||||
|
||||
// Press Ctrl+O again to collapse
|
||||
await writeStdin('\x0f');
|
||||
expect(capturedUIState.constrainHeight).toBe(true);
|
||||
|
||||
unmount();
|
||||
});
|
||||
|
||||
it('should NOT collapse when a high-priority component handles the key (e.g., up/down arrows)', async () => {
|
||||
const NavigationHandler = () => {
|
||||
// use real useKeypress
|
||||
useKeypress(
|
||||
(key: Key) => {
|
||||
if (key.name === 'up' || key.name === 'down') {
|
||||
return true; // Handle navigation
|
||||
}
|
||||
return false;
|
||||
},
|
||||
{ isActive: true, priority: true }, // High priority
|
||||
);
|
||||
return null;
|
||||
};
|
||||
|
||||
await setupExpansionPersistenceTest(NavigationHandler);
|
||||
|
||||
// Expand first
|
||||
act(() => capturedUIActions.setConstrainHeight(false));
|
||||
rerender();
|
||||
expect(capturedUIState.constrainHeight).toBe(false);
|
||||
|
||||
// 1. Simulate Up arrow (handled by high priority child)
|
||||
// CSI A is Up arrow
|
||||
await writeStdin('\u001b[A');
|
||||
|
||||
// Should STILL be expanded
|
||||
expect(capturedUIState.constrainHeight).toBe(false);
|
||||
|
||||
// 2. Simulate Down arrow (handled by high priority child)
|
||||
// CSI B is Down arrow
|
||||
await writeStdin('\u001b[B');
|
||||
|
||||
// Should STILL be expanded
|
||||
expect(capturedUIState.constrainHeight).toBe(false);
|
||||
|
||||
// 3. Sanity check: press an unhandled key
|
||||
await writeStdin('x');
|
||||
|
||||
// Should finally collapse
|
||||
expect(capturedUIState.constrainHeight).toBe(true);
|
||||
|
||||
unmount();
|
||||
});
|
||||
});
|
||||
|
||||
describe('Shortcuts Help Visibility', () => {
|
||||
let handleGlobalKeypress: (key: Key) => boolean;
|
||||
let mockedUseKeypress: Mock;
|
||||
|
||||
@@ -1873,10 +1873,7 @@ Logging in with Google... Restarting Gemini CLI to continue.
|
||||
],
|
||||
);
|
||||
|
||||
useKeypress(handleGlobalKeypress, {
|
||||
isActive: true,
|
||||
priority: KeypressPriority.Low,
|
||||
});
|
||||
useKeypress(handleGlobalKeypress, { isActive: true, priority: true });
|
||||
|
||||
useKeypress(
|
||||
() => {
|
||||
|
||||
@@ -110,7 +110,6 @@ async function handleOverageMenu(
|
||||
isDialogPending,
|
||||
setOverageMenuRequest,
|
||||
setModelSwitchedFromQuotaError,
|
||||
historyManager,
|
||||
} = args;
|
||||
|
||||
logBillingEvent(
|
||||
@@ -155,13 +154,6 @@ async function handleOverageMenu(
|
||||
setModelSwitchedFromQuotaError(false);
|
||||
config.setQuotaErrorOccurred(false);
|
||||
config.setOverageStrategy('always');
|
||||
historyManager.addItem(
|
||||
{
|
||||
type: MessageType.INFO,
|
||||
text: `Using AI Credits for this request.`,
|
||||
},
|
||||
Date.now(),
|
||||
);
|
||||
return 'retry_with_credits';
|
||||
|
||||
case 'use_fallback':
|
||||
|
||||
@@ -271,6 +271,7 @@ describe('useGeminiStream', () => {
|
||||
addHistory: vi.fn(),
|
||||
getSessionId: vi.fn(() => 'test-session-id'),
|
||||
setQuotaErrorOccurred: vi.fn(),
|
||||
resetBillingTurnState: vi.fn(),
|
||||
getQuotaErrorOccurred: vi.fn(() => false),
|
||||
getModel: vi.fn(() => 'gemini-2.5-pro'),
|
||||
getContentGeneratorConfig: vi.fn(() => ({
|
||||
|
||||
@@ -1374,6 +1374,9 @@ export const useGeminiStream = (
|
||||
if (!options?.isContinuation) {
|
||||
setModelSwitchedFromQuotaError(false);
|
||||
config.setQuotaErrorOccurred(false);
|
||||
config.resetBillingTurnState(
|
||||
settings.merged.billing?.overageStrategy,
|
||||
);
|
||||
suppressedToolErrorCountRef.current = 0;
|
||||
suppressedToolErrorNoteShownRef.current = false;
|
||||
lowVerbosityFailureNoteShownRef.current = false;
|
||||
@@ -1534,6 +1537,7 @@ export const useGeminiStream = (
|
||||
setThought,
|
||||
maybeAddSuppressedToolErrorNote,
|
||||
maybeAddLowVerbosityFailureNote,
|
||||
settings.merged.billing?.overageStrategy,
|
||||
],
|
||||
);
|
||||
|
||||
|
||||
@@ -67,14 +67,6 @@ export function useQuotaAndFallback({
|
||||
const isDialogPending = useRef(false);
|
||||
const isValidationPending = useRef(false);
|
||||
|
||||
// Initial overage strategy from settings; runtime value read from config at call time.
|
||||
const initialOverageStrategy =
|
||||
(settings.merged.billing?.overageStrategy as
|
||||
| 'ask'
|
||||
| 'always'
|
||||
| 'never'
|
||||
| undefined) ?? 'ask';
|
||||
|
||||
// Set up Flash fallback handler
|
||||
useEffect(() => {
|
||||
const fallbackHandler: FallbackModelHandler = async (
|
||||
@@ -109,9 +101,7 @@ export function useQuotaAndFallback({
|
||||
? getResetTimeMessage(error.retryDelayMs)
|
||||
: undefined;
|
||||
|
||||
const overageStrategy =
|
||||
config.getBillingSettings().overageStrategy ??
|
||||
initialOverageStrategy;
|
||||
const overageStrategy = config.getBillingSettings().overageStrategy;
|
||||
|
||||
const creditsResult = await handleCreditsFlow({
|
||||
config,
|
||||
@@ -209,7 +199,6 @@ export function useQuotaAndFallback({
|
||||
userTier,
|
||||
paidTier,
|
||||
settings,
|
||||
initialOverageStrategy,
|
||||
setModelSwitchedFromQuotaError,
|
||||
onShowAuthSelection,
|
||||
errorVerbosity,
|
||||
|
||||
@@ -1,6 +1,6 @@
|
||||
{
|
||||
"name": "@google/gemini-cli-core",
|
||||
"version": "0.33.0-nightly.20260228.1ca5c05d0",
|
||||
"version": "0.33.0-preview.9",
|
||||
"description": "Gemini CLI Core",
|
||||
"license": "Apache-2.0",
|
||||
"repository": {
|
||||
|
||||
@@ -17,10 +17,7 @@ import { debugLogger } from '../utils/debugLogger.js';
|
||||
import { LocalAgentExecutor, type ActivityCallback } from './local-executor.js';
|
||||
import { makeFakeConfig } from '../test-utils/config.js';
|
||||
import { ToolRegistry } from '../tools/tool-registry.js';
|
||||
import {
|
||||
DiscoveredMCPTool,
|
||||
MCP_QUALIFIED_NAME_SEPARATOR,
|
||||
} from '../tools/mcp-tool.js';
|
||||
import { DiscoveredMCPTool } from '../tools/mcp-tool.js';
|
||||
import { LSTool } from '../tools/ls.js';
|
||||
import { LS_TOOL_NAME, READ_FILE_TOOL_NAME } from '../tools/tool-names.js';
|
||||
import {
|
||||
@@ -504,7 +501,7 @@ describe('LocalAgentExecutor', () => {
|
||||
it('should automatically qualify MCP tools in agent definitions', async () => {
|
||||
const serverName = 'mcp-server';
|
||||
const toolName = 'mcp-tool';
|
||||
const qualifiedName = `${serverName}${MCP_QUALIFIED_NAME_SEPARATOR}${toolName}`;
|
||||
const qualifiedName = `mcp_${serverName}_${toolName}`;
|
||||
|
||||
const mockMcpTool = {
|
||||
tool: vi.fn(),
|
||||
|
||||
@@ -229,14 +229,14 @@ describe('billing', () => {
|
||||
expect(isOverageEligibleModel('gemini-3.1-pro-preview')).toBe(true);
|
||||
});
|
||||
|
||||
it('should return true for gemini-3.1-pro-preview-customtools', () => {
|
||||
it('should return false for gemini-3.1-pro-preview-customtools', () => {
|
||||
expect(isOverageEligibleModel('gemini-3.1-pro-preview-customtools')).toBe(
|
||||
false,
|
||||
);
|
||||
});
|
||||
|
||||
it('should return false for gemini-3-flash-preview', () => {
|
||||
expect(isOverageEligibleModel('gemini-3-flash-preview')).toBe(false);
|
||||
it('should return true for gemini-3-flash-preview', () => {
|
||||
expect(isOverageEligibleModel('gemini-3-flash-preview')).toBe(true);
|
||||
});
|
||||
|
||||
it('should return false for gemini-2.5-pro', () => {
|
||||
|
||||
@@ -12,6 +12,7 @@ import type {
|
||||
import {
|
||||
PREVIEW_GEMINI_MODEL,
|
||||
PREVIEW_GEMINI_3_1_MODEL,
|
||||
PREVIEW_GEMINI_FLASH_MODEL,
|
||||
} from '../config/models.js';
|
||||
|
||||
/**
|
||||
@@ -32,6 +33,7 @@ export const G1_CREDIT_TYPE: CreditType = 'GOOGLE_ONE_AI';
|
||||
export const OVERAGE_ELIGIBLE_MODELS = new Set([
|
||||
PREVIEW_GEMINI_MODEL,
|
||||
PREVIEW_GEMINI_3_1_MODEL,
|
||||
PREVIEW_GEMINI_FLASH_MODEL,
|
||||
]);
|
||||
|
||||
/**
|
||||
|
||||
@@ -95,7 +95,8 @@ const mockConfig = {
|
||||
getNoBrowser: () => false,
|
||||
getProxy: () => 'http://test.proxy.com:8080',
|
||||
isBrowserLaunchSuppressed: () => false,
|
||||
getExperimentalZedIntegration: () => false,
|
||||
getAcpMode: () => false,
|
||||
isInteractive: () => true,
|
||||
} as unknown as Config;
|
||||
|
||||
// Mock fetch globally
|
||||
|
||||
@@ -271,8 +271,8 @@ async function initOauthClient(
|
||||
|
||||
await triggerPostAuthCallbacks(client.credentials);
|
||||
} else {
|
||||
// In Zed integration, we skip the interactive consent and directly open the browser
|
||||
if (!config.getExperimentalZedIntegration()) {
|
||||
// In ACP mode, we skip the interactive consent and directly open the browser
|
||||
if (!config.getAcpMode()) {
|
||||
const userConsent = await getConsentForOauth('');
|
||||
if (!userConsent) {
|
||||
throw new FatalCancellationError('Authentication cancelled by user.');
|
||||
|
||||
@@ -47,7 +47,9 @@ import {
|
||||
isOverageEligibleModel,
|
||||
shouldAutoUseCredits,
|
||||
} from '../billing/billing.js';
|
||||
|
||||
import { logBillingEvent } from '../telemetry/loggers.js';
|
||||
import { coreEvents } from '../utils/events.js';
|
||||
import { CreditsUsedEvent } from '../telemetry/billingEvents.js';
|
||||
import type {
|
||||
CaCountTokenResponse,
|
||||
@@ -102,6 +104,11 @@ export class CodeAssistServer implements ContentGenerator {
|
||||
const modelIsEligible = isOverageEligibleModel(req.model);
|
||||
const shouldEnableCredits = modelIsEligible && autoUse;
|
||||
|
||||
if (shouldEnableCredits && !this.config?.getCreditsNotificationShown()) {
|
||||
this.config?.setCreditsNotificationShown(true);
|
||||
coreEvents.emitFeedback('info', 'Using AI Credits for this request.');
|
||||
}
|
||||
|
||||
const enabledCreditTypes = shouldEnableCredits
|
||||
? ([G1_CREDIT_TYPE] as string[])
|
||||
: undefined;
|
||||
|
||||
@@ -500,6 +500,8 @@ describe('Server Config (config.ts)', () => {
|
||||
config,
|
||||
authType,
|
||||
undefined,
|
||||
undefined,
|
||||
undefined,
|
||||
);
|
||||
// Verify that contentGeneratorConfig is updated
|
||||
expect(config.getContentGeneratorConfig()).toEqual(mockContentConfig);
|
||||
@@ -2395,6 +2397,65 @@ describe('Availability Service Integration', () => {
|
||||
config.resetTurn();
|
||||
expect(spy).toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('resetTurn does NOT reset billing state', () => {
|
||||
const config = new Config({
|
||||
...baseParams,
|
||||
billing: { overageStrategy: 'ask' },
|
||||
});
|
||||
|
||||
// Simulate accepting credits mid-turn
|
||||
config.setOverageStrategy('always');
|
||||
config.setCreditsNotificationShown(true);
|
||||
|
||||
// resetTurn should leave billing state intact
|
||||
config.resetTurn();
|
||||
expect(config.getBillingSettings().overageStrategy).toBe('always');
|
||||
expect(config.getCreditsNotificationShown()).toBe(true);
|
||||
});
|
||||
|
||||
it('resetBillingTurnState resets overageStrategy to configured value', () => {
|
||||
const config = new Config({
|
||||
...baseParams,
|
||||
billing: { overageStrategy: 'ask' },
|
||||
});
|
||||
|
||||
config.setOverageStrategy('always');
|
||||
expect(config.getBillingSettings().overageStrategy).toBe('always');
|
||||
|
||||
config.resetBillingTurnState('ask');
|
||||
expect(config.getBillingSettings().overageStrategy).toBe('ask');
|
||||
});
|
||||
|
||||
it('resetBillingTurnState preserves overageStrategy when configured as always', () => {
|
||||
const config = new Config({
|
||||
...baseParams,
|
||||
billing: { overageStrategy: 'always' },
|
||||
});
|
||||
|
||||
config.resetBillingTurnState('always');
|
||||
expect(config.getBillingSettings().overageStrategy).toBe('always');
|
||||
});
|
||||
|
||||
it('resetBillingTurnState defaults to ask when no strategy provided', () => {
|
||||
const config = new Config({
|
||||
...baseParams,
|
||||
billing: { overageStrategy: 'always' },
|
||||
});
|
||||
|
||||
config.resetBillingTurnState();
|
||||
expect(config.getBillingSettings().overageStrategy).toBe('ask');
|
||||
});
|
||||
|
||||
it('resetBillingTurnState resets creditsNotificationShown', () => {
|
||||
const config = new Config(baseParams);
|
||||
|
||||
config.setCreditsNotificationShown(true);
|
||||
expect(config.getCreditsNotificationShown()).toBe(true);
|
||||
|
||||
config.resetBillingTurnState();
|
||||
expect(config.getCreditsNotificationShown()).toBe(false);
|
||||
});
|
||||
});
|
||||
|
||||
describe('Hooks configuration', () => {
|
||||
|
||||
@@ -6,7 +6,6 @@
|
||||
|
||||
import * as fs from 'node:fs';
|
||||
import * as path from 'node:path';
|
||||
import * as os from 'node:os';
|
||||
import { inspect } from 'node:util';
|
||||
import process from 'node:process';
|
||||
import type {
|
||||
@@ -135,7 +134,7 @@ import { SkillManager, type SkillDefinition } from '../skills/skillManager.js';
|
||||
import { startupProfiler } from '../telemetry/startupProfiler.js';
|
||||
import type { AgentDefinition } from '../agents/types.js';
|
||||
import { fetchAdminControls } from '../code_assist/admin/admin_controls.js';
|
||||
import { isSubpath } from '../utils/paths.js';
|
||||
import { isSubpath, resolveToRealPath } from '../utils/paths.js';
|
||||
import { UserHintService } from './userHintService.js';
|
||||
import { WORKSPACE_POLICY_TIER } from '../policy/config.js';
|
||||
import { loadPoliciesFromToml } from '../policy/toml-loader.js';
|
||||
@@ -503,7 +502,7 @@ export interface ConfigParameters {
|
||||
model: string;
|
||||
disableLoopDetection?: boolean;
|
||||
maxSessionTurns?: number;
|
||||
experimentalZedIntegration?: boolean;
|
||||
acpMode?: boolean;
|
||||
listSessions?: boolean;
|
||||
deleteSession?: string;
|
||||
listExtensions?: boolean;
|
||||
@@ -671,6 +670,7 @@ export class Config implements McpContext {
|
||||
fallbackModelHandler?: FallbackModelHandler;
|
||||
validationHandler?: ValidationHandler;
|
||||
private quotaErrorOccurred: boolean = false;
|
||||
private creditsNotificationShown: boolean = false;
|
||||
private modelQuotas: Map<
|
||||
string,
|
||||
{ remaining: number; limit: number; resetTime?: string }
|
||||
@@ -699,7 +699,7 @@ export class Config implements McpContext {
|
||||
private readonly summarizeToolOutput:
|
||||
| Record<string, SummarizeToolOutputSettings>
|
||||
| undefined;
|
||||
private readonly experimentalZedIntegration: boolean = false;
|
||||
private readonly acpMode: boolean = false;
|
||||
private readonly loadMemoryFromIncludeDirectories: boolean = false;
|
||||
private readonly includeDirectoryTree: boolean = true;
|
||||
private readonly importFormat: 'tree' | 'flat';
|
||||
@@ -894,8 +894,7 @@ export class Config implements McpContext {
|
||||
DEFAULT_PROTECT_LATEST_TURN,
|
||||
};
|
||||
this.maxSessionTurns = params.maxSessionTurns ?? -1;
|
||||
this.experimentalZedIntegration =
|
||||
params.experimentalZedIntegration ?? false;
|
||||
this.acpMode = params.acpMode ?? false;
|
||||
this.listSessions = params.listSessions ?? false;
|
||||
this.deleteSession = params.deleteSession;
|
||||
this.listExtensions = params.listExtensions ?? false;
|
||||
@@ -1148,7 +1147,7 @@ export class Config implements McpContext {
|
||||
}
|
||||
});
|
||||
|
||||
if (!this.interactive || this.experimentalZedIntegration) {
|
||||
if (!this.interactive || this.acpMode) {
|
||||
await this.mcpInitializationPromise;
|
||||
}
|
||||
|
||||
@@ -1191,7 +1190,12 @@ export class Config implements McpContext {
|
||||
return this.contentGenerator;
|
||||
}
|
||||
|
||||
async refreshAuth(authMethod: AuthType, apiKey?: string) {
|
||||
async refreshAuth(
|
||||
authMethod: AuthType,
|
||||
apiKey?: string,
|
||||
baseUrl?: string,
|
||||
customHeaders?: Record<string, string>,
|
||||
) {
|
||||
// Reset availability service when switching auth
|
||||
this.modelAvailabilityService.reset();
|
||||
|
||||
@@ -1218,6 +1222,8 @@ export class Config implements McpContext {
|
||||
this,
|
||||
authMethod,
|
||||
apiKey,
|
||||
baseUrl,
|
||||
customHeaders,
|
||||
);
|
||||
this.contentGenerator = await createContentGenerator(
|
||||
newContentGeneratorConfig,
|
||||
@@ -1432,6 +1438,12 @@ export class Config implements McpContext {
|
||||
this.modelAvailabilityService.resetTurn();
|
||||
}
|
||||
|
||||
/** Resets billing state (overageStrategy, creditsNotificationShown) once per user prompt. */
|
||||
resetBillingTurnState(overageStrategy?: OverageStrategy): void {
|
||||
this.creditsNotificationShown = false;
|
||||
this.billing.overageStrategy = overageStrategy ?? 'ask';
|
||||
}
|
||||
|
||||
getMaxSessionTurns(): number {
|
||||
return this.maxSessionTurns;
|
||||
}
|
||||
@@ -1444,6 +1456,14 @@ export class Config implements McpContext {
|
||||
return this.quotaErrorOccurred;
|
||||
}
|
||||
|
||||
setCreditsNotificationShown(value: boolean): void {
|
||||
this.creditsNotificationShown = value;
|
||||
}
|
||||
|
||||
getCreditsNotificationShown(): boolean {
|
||||
return this.creditsNotificationShown;
|
||||
}
|
||||
|
||||
setQuota(
|
||||
remaining: number | undefined,
|
||||
limit: number | undefined,
|
||||
@@ -2205,8 +2225,8 @@ export class Config implements McpContext {
|
||||
return this.usageStatisticsEnabled;
|
||||
}
|
||||
|
||||
getExperimentalZedIntegration(): boolean {
|
||||
return this.experimentalZedIntegration;
|
||||
getAcpMode(): boolean {
|
||||
return this.acpMode;
|
||||
}
|
||||
|
||||
async waitForMcpInit(): Promise<void> {
|
||||
@@ -2339,17 +2359,7 @@ export class Config implements McpContext {
|
||||
* @returns true if the path is allowed, false otherwise.
|
||||
*/
|
||||
isPathAllowed(absolutePath: string): boolean {
|
||||
const realpath = (p: string) => {
|
||||
let resolved: string;
|
||||
try {
|
||||
resolved = fs.realpathSync(p);
|
||||
} catch {
|
||||
resolved = path.resolve(p);
|
||||
}
|
||||
return os.platform() === 'win32' ? resolved.toLowerCase() : resolved;
|
||||
};
|
||||
|
||||
const resolvedPath = realpath(absolutePath);
|
||||
const resolvedPath = resolveToRealPath(absolutePath);
|
||||
|
||||
const workspaceContext = this.getWorkspaceContext();
|
||||
if (workspaceContext.isPathWithinWorkspace(resolvedPath)) {
|
||||
@@ -2357,7 +2367,7 @@ export class Config implements McpContext {
|
||||
}
|
||||
|
||||
const projectTempDir = this.storage.getProjectTempDir();
|
||||
const resolvedTempDir = realpath(projectTempDir);
|
||||
const resolvedTempDir = resolveToRealPath(projectTempDir);
|
||||
|
||||
return isSubpath(resolvedTempDir, resolvedPath);
|
||||
}
|
||||
|
||||
@@ -24,7 +24,7 @@ vi.mock('fs', async (importOriginal) => {
|
||||
});
|
||||
|
||||
import { Storage } from './storage.js';
|
||||
import { GEMINI_DIR, homedir } from '../utils/paths.js';
|
||||
import { GEMINI_DIR, homedir, resolveToRealPath } from '../utils/paths.js';
|
||||
import { ProjectRegistry } from './projectRegistry.js';
|
||||
import { StorageMigration } from './storageMigration.js';
|
||||
|
||||
@@ -279,8 +279,7 @@ describe('Storage – additional helpers', () => {
|
||||
name: 'custom absolute path outside throws',
|
||||
customDir: '/absolute/path/to/plans',
|
||||
expected: '',
|
||||
expectedError:
|
||||
"Custom plans directory '/absolute/path/to/plans' resolves to '/absolute/path/to/plans', which is outside the project root '/tmp/project'.",
|
||||
expectedError: `Custom plans directory '/absolute/path/to/plans' resolves to '/absolute/path/to/plans', which is outside the project root '${resolveToRealPath(projectRoot)}'.`,
|
||||
},
|
||||
{
|
||||
name: 'absolute path that happens to be inside project root',
|
||||
@@ -306,8 +305,7 @@ describe('Storage – additional helpers', () => {
|
||||
name: 'escaping relative path throws',
|
||||
customDir: '../escaped-plans',
|
||||
expected: '',
|
||||
expectedError:
|
||||
"Custom plans directory '../escaped-plans' resolves to '/tmp/escaped-plans', which is outside the project root '/tmp/project'.",
|
||||
expectedError: `Custom plans directory '../escaped-plans' resolves to '${resolveToRealPath(path.resolve(projectRoot, '../escaped-plans'))}', which is outside the project root '${resolveToRealPath(projectRoot)}'.`,
|
||||
},
|
||||
{
|
||||
name: 'hidden directory starting with ..',
|
||||
|
||||
@@ -103,7 +103,7 @@ The following tools are available in Plan Mode:
|
||||
<tool>\`exit_plan_mode\`</tool>
|
||||
<tool>\`write_file\`</tool>
|
||||
<tool>\`replace\`</tool>
|
||||
<tool>\`read_data\` (readonly-server)</tool>
|
||||
<tool>\`mcp_readonly-server_read_data\` (readonly-server)</tool>
|
||||
</available_tools>
|
||||
|
||||
## Rules
|
||||
@@ -271,7 +271,7 @@ The following tools are available in Plan Mode:
|
||||
<tool>\`exit_plan_mode\`</tool>
|
||||
<tool>\`write_file\`</tool>
|
||||
<tool>\`replace\`</tool>
|
||||
<tool>\`read_data\` (readonly-server)</tool>
|
||||
<tool>\`mcp_readonly-server_read_data\` (readonly-server)</tool>
|
||||
</available_tools>
|
||||
|
||||
## Rules
|
||||
@@ -558,7 +558,7 @@ The following tools are available in Plan Mode:
|
||||
<tool>\`exit_plan_mode\`</tool>
|
||||
<tool>\`write_file\`</tool>
|
||||
<tool>\`replace\`</tool>
|
||||
<tool>\`read_data\` (readonly-server)</tool>
|
||||
<tool>\`mcp_readonly-server_read_data\` (readonly-server)</tool>
|
||||
</available_tools>
|
||||
|
||||
## Rules
|
||||
|
||||
@@ -59,6 +59,7 @@ export enum AuthType {
|
||||
USE_VERTEX_AI = 'vertex-ai',
|
||||
LEGACY_CLOUD_SHELL = 'cloud-shell',
|
||||
COMPUTE_ADC = 'compute-default-credentials',
|
||||
GATEWAY = 'gateway',
|
||||
}
|
||||
|
||||
/**
|
||||
@@ -93,12 +94,16 @@ export type ContentGeneratorConfig = {
|
||||
vertexai?: boolean;
|
||||
authType?: AuthType;
|
||||
proxy?: string;
|
||||
baseUrl?: string;
|
||||
customHeaders?: Record<string, string>;
|
||||
};
|
||||
|
||||
export async function createContentGeneratorConfig(
|
||||
config: Config,
|
||||
authType: AuthType | undefined,
|
||||
apiKey?: string,
|
||||
baseUrl?: string,
|
||||
customHeaders?: Record<string, string>,
|
||||
): Promise<ContentGeneratorConfig> {
|
||||
const geminiApiKey =
|
||||
apiKey ||
|
||||
@@ -115,6 +120,8 @@ export async function createContentGeneratorConfig(
|
||||
const contentGeneratorConfig: ContentGeneratorConfig = {
|
||||
authType,
|
||||
proxy: config?.getProxy(),
|
||||
baseUrl,
|
||||
customHeaders,
|
||||
};
|
||||
|
||||
// If we are using Google auth or we are in Cloud Shell, there is nothing else to validate for now
|
||||
@@ -203,9 +210,13 @@ export async function createContentGenerator(
|
||||
|
||||
if (
|
||||
config.authType === AuthType.USE_GEMINI ||
|
||||
config.authType === AuthType.USE_VERTEX_AI
|
||||
config.authType === AuthType.USE_VERTEX_AI ||
|
||||
config.authType === AuthType.GATEWAY
|
||||
) {
|
||||
let headers: Record<string, string> = { ...baseHeaders };
|
||||
if (config.customHeaders) {
|
||||
headers = { ...headers, ...config.customHeaders };
|
||||
}
|
||||
if (gcConfig?.getUsageStatisticsEnabled()) {
|
||||
const installationManager = new InstallationManager();
|
||||
const installationId = installationManager.getInstallationId();
|
||||
@@ -214,7 +225,14 @@ export async function createContentGenerator(
|
||||
'x-gemini-api-privileged-user-id': `${installationId}`,
|
||||
};
|
||||
}
|
||||
const httpOptions = { headers };
|
||||
const httpOptions: {
|
||||
baseUrl?: string;
|
||||
headers: Record<string, string>;
|
||||
} = { headers };
|
||||
|
||||
if (config.baseUrl) {
|
||||
httpOptions.baseUrl = config.baseUrl;
|
||||
}
|
||||
|
||||
const googleGenAI = new GoogleGenAI({
|
||||
apiKey: config.apiKey === '' ? undefined : config.apiKey,
|
||||
|
||||
@@ -709,7 +709,7 @@ describe('estimateContextBreakdown', () => {
|
||||
{
|
||||
functionDeclarations: [
|
||||
{
|
||||
name: 'myserver__search',
|
||||
name: 'mcp_myserver_search',
|
||||
description: 'Search via MCP',
|
||||
parameters: {},
|
||||
},
|
||||
@@ -747,8 +747,7 @@ describe('estimateContextBreakdown', () => {
|
||||
expect(builtinOnly.mcp_servers).toBe(0);
|
||||
});
|
||||
|
||||
it('should not classify tools with __ in the middle of a segment as MCP', () => {
|
||||
// "__" at start or end (not a valid server__tool pattern) should not be MCP
|
||||
it('should not classify tools without mcp_ prefix as MCP', () => {
|
||||
const config = {
|
||||
tools: [
|
||||
{
|
||||
@@ -842,7 +841,7 @@ describe('estimateContextBreakdown', () => {
|
||||
functionDeclarations: [
|
||||
{ name: 'read_file', description: 'Read', parameters: {} },
|
||||
{
|
||||
name: 'myserver__search',
|
||||
name: 'mcp_myserver_search',
|
||||
description: 'MCP search',
|
||||
parameters: {},
|
||||
},
|
||||
@@ -858,7 +857,7 @@ describe('estimateContextBreakdown', () => {
|
||||
expect(result.history).toBeGreaterThan(0);
|
||||
// tool_calls should only contain non-MCP tools
|
||||
expect(result.tool_calls['read_file']).toBeGreaterThan(0);
|
||||
expect(result.tool_calls['myserver__search']).toBeUndefined();
|
||||
expect(result.tool_calls['mcp_myserver_search']).toBeUndefined();
|
||||
// MCP tokens are only in mcp_servers
|
||||
expect(result.mcp_servers).toBeGreaterThan(0);
|
||||
});
|
||||
@@ -870,7 +869,7 @@ describe('estimateContextBreakdown', () => {
|
||||
parts: [
|
||||
{
|
||||
functionCall: {
|
||||
name: 'myserver__search',
|
||||
name: 'mcp_myserver_search',
|
||||
args: { query: 'test' },
|
||||
},
|
||||
},
|
||||
@@ -881,7 +880,7 @@ describe('estimateContextBreakdown', () => {
|
||||
parts: [
|
||||
{
|
||||
functionResponse: {
|
||||
name: 'myserver__search',
|
||||
name: 'mcp_myserver_search',
|
||||
response: { results: [] },
|
||||
},
|
||||
},
|
||||
@@ -890,7 +889,7 @@ describe('estimateContextBreakdown', () => {
|
||||
];
|
||||
const result = estimateContextBreakdown(contents);
|
||||
// MCP tool calls should NOT appear in tool_calls
|
||||
expect(result.tool_calls['myserver__search']).toBeUndefined();
|
||||
expect(result.tool_calls['mcp_myserver_search']).toBeUndefined();
|
||||
// MCP call tokens should only be counted in mcp_servers
|
||||
expect(result.mcp_servers).toBeGreaterThan(0);
|
||||
});
|
||||
@@ -908,7 +907,7 @@ describe('estimateContextBreakdown', () => {
|
||||
},
|
||||
{
|
||||
functionCall: {
|
||||
name: 'myserver__search',
|
||||
name: 'mcp_myserver_search',
|
||||
args: { q: 'hello' },
|
||||
},
|
||||
},
|
||||
@@ -919,7 +918,7 @@ describe('estimateContextBreakdown', () => {
|
||||
// Non-MCP tools should be in tool_calls
|
||||
expect(result.tool_calls['read_file']).toBeGreaterThan(0);
|
||||
// MCP tools should NOT be in tool_calls
|
||||
expect(result.tool_calls['myserver__search']).toBeUndefined();
|
||||
expect(result.tool_calls['mcp_myserver_search']).toBeUndefined();
|
||||
// MCP tool calls should only be in mcp_servers
|
||||
expect(result.mcp_servers).toBeGreaterThan(0);
|
||||
});
|
||||
|
||||
@@ -465,9 +465,13 @@ describe('Core System Prompt (prompts.ts)', () => {
|
||||
const prompt = getCoreSystemPrompt(mockConfig);
|
||||
expect(prompt).toContain('# Active Approval Mode: Plan');
|
||||
// Read-only MCP tool should appear with server name
|
||||
expect(prompt).toContain('`read_data` (readonly-server)');
|
||||
expect(prompt).toContain(
|
||||
'`mcp_readonly-server_read_data` (readonly-server)',
|
||||
);
|
||||
// Non-read-only MCP tool should not appear (excluded by policy)
|
||||
expect(prompt).not.toContain('`write_data` (nonreadonly-server)');
|
||||
expect(prompt).not.toContain(
|
||||
'`mcp_nonreadonly-server_write_data` (nonreadonly-server)',
|
||||
);
|
||||
expect(prompt).toMatchSnapshot();
|
||||
});
|
||||
|
||||
@@ -485,8 +489,12 @@ describe('Core System Prompt (prompts.ts)', () => {
|
||||
|
||||
const prompt = getCoreSystemPrompt(mockConfig);
|
||||
|
||||
expect(prompt).toContain('`read_data` (readonly-server)');
|
||||
expect(prompt).not.toContain('`write_data` (nonreadonly-server)');
|
||||
expect(prompt).toContain(
|
||||
'`mcp_readonly-server_read_data` (readonly-server)',
|
||||
);
|
||||
expect(prompt).not.toContain(
|
||||
'`mcp_nonreadonly-server_write_data` (nonreadonly-server)',
|
||||
);
|
||||
});
|
||||
|
||||
it('should only list available tools in PLAN mode', () => {
|
||||
|
||||
@@ -202,8 +202,7 @@ describe('createPolicyEngineConfig', () => {
|
||||
'/tmp/mock/default/policies',
|
||||
);
|
||||
const rule = config.rules?.find(
|
||||
(r) =>
|
||||
r.toolName === 'my-server__*' && r.decision === PolicyDecision.ALLOW,
|
||||
(r) => r.mcpName === 'my-server' && r.decision === PolicyDecision.ALLOW,
|
||||
);
|
||||
expect(rule).toBeDefined();
|
||||
expect(rule?.priority).toBe(4.1); // MCP allowed server
|
||||
@@ -220,8 +219,7 @@ describe('createPolicyEngineConfig', () => {
|
||||
'/tmp/mock/default/policies',
|
||||
);
|
||||
const rule = config.rules?.find(
|
||||
(r) =>
|
||||
r.toolName === 'my-server__*' && r.decision === PolicyDecision.DENY,
|
||||
(r) => r.mcpName === 'my-server' && r.decision === PolicyDecision.DENY,
|
||||
);
|
||||
expect(rule).toBeDefined();
|
||||
expect(rule?.priority).toBe(4.9); // MCP excluded server
|
||||
@@ -247,8 +245,7 @@ describe('createPolicyEngineConfig', () => {
|
||||
|
||||
const trustedRule = config.rules?.find(
|
||||
(r) =>
|
||||
r.toolName === 'trusted-server__*' &&
|
||||
r.decision === PolicyDecision.ALLOW,
|
||||
r.mcpName === 'trusted-server' && r.decision === PolicyDecision.ALLOW,
|
||||
);
|
||||
expect(trustedRule).toBeDefined();
|
||||
expect(trustedRule?.priority).toBe(4.2); // MCP trusted server
|
||||
@@ -256,8 +253,7 @@ describe('createPolicyEngineConfig', () => {
|
||||
// Untrusted server should not have an allow rule
|
||||
const untrustedRule = config.rules?.find(
|
||||
(r) =>
|
||||
r.toolName === 'untrusted-server__*' &&
|
||||
r.decision === PolicyDecision.ALLOW,
|
||||
r.mcpName === 'untrusted-server' && r.decision === PolicyDecision.ALLOW,
|
||||
);
|
||||
expect(untrustedRule).toBeUndefined();
|
||||
});
|
||||
@@ -284,8 +280,7 @@ describe('createPolicyEngineConfig', () => {
|
||||
// Check allowed server
|
||||
const allowedRule = config.rules?.find(
|
||||
(r) =>
|
||||
r.toolName === 'allowed-server__*' &&
|
||||
r.decision === PolicyDecision.ALLOW,
|
||||
r.mcpName === 'allowed-server' && r.decision === PolicyDecision.ALLOW,
|
||||
);
|
||||
expect(allowedRule).toBeDefined();
|
||||
expect(allowedRule?.priority).toBe(4.1); // MCP allowed server
|
||||
@@ -293,8 +288,7 @@ describe('createPolicyEngineConfig', () => {
|
||||
// Check trusted server
|
||||
const trustedRule = config.rules?.find(
|
||||
(r) =>
|
||||
r.toolName === 'trusted-server__*' &&
|
||||
r.decision === PolicyDecision.ALLOW,
|
||||
r.mcpName === 'trusted-server' && r.decision === PolicyDecision.ALLOW,
|
||||
);
|
||||
expect(trustedRule).toBeDefined();
|
||||
expect(trustedRule?.priority).toBe(4.2); // MCP trusted server
|
||||
@@ -302,8 +296,7 @@ describe('createPolicyEngineConfig', () => {
|
||||
// Check excluded server
|
||||
const excludedRule = config.rules?.find(
|
||||
(r) =>
|
||||
r.toolName === 'excluded-server__*' &&
|
||||
r.decision === PolicyDecision.DENY,
|
||||
r.mcpName === 'excluded-server' && r.decision === PolicyDecision.DENY,
|
||||
);
|
||||
expect(excludedRule).toBeDefined();
|
||||
expect(excludedRule?.priority).toBe(4.9); // MCP excluded server
|
||||
@@ -368,7 +361,7 @@ describe('createPolicyEngineConfig', () => {
|
||||
const { createPolicyEngineConfig } = await import('./config.js');
|
||||
const settings: PolicySettings = {
|
||||
mcp: { excluded: ['my-server'] },
|
||||
tools: { allowed: ['my-server__specific-tool'] },
|
||||
tools: { allowed: ['mcp_my-server_specific-tool'] },
|
||||
};
|
||||
const config = await createPolicyEngineConfig(
|
||||
settings,
|
||||
@@ -377,12 +370,11 @@ describe('createPolicyEngineConfig', () => {
|
||||
);
|
||||
|
||||
const serverDenyRule = config.rules?.find(
|
||||
(r) =>
|
||||
r.toolName === 'my-server__*' && r.decision === PolicyDecision.DENY,
|
||||
(r) => r.mcpName === 'my-server' && r.decision === PolicyDecision.DENY,
|
||||
);
|
||||
const toolAllowRule = config.rules?.find(
|
||||
(r) =>
|
||||
r.toolName === 'my-server__specific-tool' &&
|
||||
r.toolName === 'mcp_my-server_specific-tool' &&
|
||||
r.decision === PolicyDecision.ALLOW,
|
||||
);
|
||||
|
||||
@@ -404,7 +396,7 @@ describe('createPolicyEngineConfig', () => {
|
||||
trust: true,
|
||||
},
|
||||
},
|
||||
tools: { exclude: ['my-server__dangerous-tool'] },
|
||||
tools: { exclude: ['mcp_my-server_dangerous-tool'] },
|
||||
};
|
||||
const config = await createPolicyEngineConfig(
|
||||
settings,
|
||||
@@ -413,12 +405,11 @@ describe('createPolicyEngineConfig', () => {
|
||||
);
|
||||
|
||||
const serverAllowRule = config.rules?.find(
|
||||
(r) =>
|
||||
r.toolName === 'my-server__*' && r.decision === PolicyDecision.ALLOW,
|
||||
(r) => r.mcpName === 'my-server' && r.decision === PolicyDecision.ALLOW,
|
||||
);
|
||||
const toolDenyRule = config.rules?.find(
|
||||
(r) =>
|
||||
r.toolName === 'my-server__dangerous-tool' &&
|
||||
r.toolName === 'mcp_my-server_dangerous-tool' &&
|
||||
r.decision === PolicyDecision.DENY,
|
||||
);
|
||||
|
||||
@@ -432,8 +423,8 @@ describe('createPolicyEngineConfig', () => {
|
||||
it('should handle complex priority scenarios correctly', async () => {
|
||||
const settings: PolicySettings = {
|
||||
tools: {
|
||||
allowed: ['my-server__tool1', 'other-tool'], // Priority 4.3
|
||||
exclude: ['my-server__tool2', 'glob'], // Priority 4.4
|
||||
allowed: ['mcp_trusted-server_tool1', 'other-tool'], // Priority 4.3
|
||||
exclude: ['mcp_trusted-server_tool2', 'glob'], // Priority 4.4
|
||||
},
|
||||
mcp: {
|
||||
allowed: ['allowed-server'], // Priority 4.1
|
||||
@@ -564,13 +555,12 @@ describe('createPolicyEngineConfig', () => {
|
||||
// Neither server should have an allow rule
|
||||
const noTrustRule = config.rules?.find(
|
||||
(r) =>
|
||||
r.toolName === 'no-trust-property__*' &&
|
||||
r.mcpName === 'no-trust-property' &&
|
||||
r.decision === PolicyDecision.ALLOW,
|
||||
);
|
||||
const explicitFalseRule = config.rules?.find(
|
||||
(r) =>
|
||||
r.toolName === 'explicit-false__*' &&
|
||||
r.decision === PolicyDecision.ALLOW,
|
||||
r.mcpName === 'explicit-false' && r.decision === PolicyDecision.ALLOW,
|
||||
);
|
||||
|
||||
expect(noTrustRule).toBeUndefined();
|
||||
|
||||
@@ -31,6 +31,7 @@ import { debugLogger } from '../utils/debugLogger.js';
|
||||
import { SHELL_TOOL_NAMES } from '../utils/shell-utils.js';
|
||||
import { SHELL_TOOL_NAME } from '../tools/tool-names.js';
|
||||
import { isNodeError } from '../utils/errors.js';
|
||||
import { MCP_TOOL_PREFIX } from '../tools/mcp-tool.js';
|
||||
|
||||
import { isDirectorySecure } from '../utils/security.js';
|
||||
|
||||
@@ -342,7 +343,11 @@ export async function createPolicyEngineConfig(
|
||||
if (settings.mcp?.excluded) {
|
||||
for (const serverName of settings.mcp.excluded) {
|
||||
rules.push({
|
||||
toolName: `${serverName}__*`,
|
||||
toolName:
|
||||
serverName === '*'
|
||||
? `${MCP_TOOL_PREFIX}*`
|
||||
: `${MCP_TOOL_PREFIX}${serverName}_*`,
|
||||
mcpName: serverName,
|
||||
decision: PolicyDecision.DENY,
|
||||
priority: MCP_EXCLUDED_PRIORITY,
|
||||
source: 'Settings (MCP Excluded)',
|
||||
@@ -423,9 +428,10 @@ export async function createPolicyEngineConfig(
|
||||
)) {
|
||||
if (serverConfig.trust) {
|
||||
// Trust all tools from this MCP server
|
||||
// Using pattern matching for MCP tool names which are formatted as "serverName__toolName"
|
||||
// Using explicit mcpName metadata and FQN mcp_{serverName}_*
|
||||
rules.push({
|
||||
toolName: `${serverName}__*`,
|
||||
toolName: `${MCP_TOOL_PREFIX}${serverName}_*`,
|
||||
mcpName: serverName,
|
||||
decision: PolicyDecision.ALLOW,
|
||||
priority: TRUSTED_MCP_SERVER_PRIORITY,
|
||||
source: 'Settings (MCP Trusted)',
|
||||
@@ -439,7 +445,11 @@ export async function createPolicyEngineConfig(
|
||||
if (settings.mcp?.allowed) {
|
||||
for (const serverName of settings.mcp.allowed) {
|
||||
rules.push({
|
||||
toolName: `${serverName}__*`,
|
||||
toolName:
|
||||
serverName === '*'
|
||||
? `${MCP_TOOL_PREFIX}*`
|
||||
: `${MCP_TOOL_PREFIX}${serverName}_*`,
|
||||
mcpName: serverName,
|
||||
decision: PolicyDecision.ALLOW,
|
||||
priority: ALLOWED_MCP_SERVER_PRIORITY,
|
||||
source: 'Settings (MCP Allowed)',
|
||||
|
||||
@@ -51,3 +51,8 @@ priority = 50
|
||||
toolName = "google_web_search"
|
||||
decision = "allow"
|
||||
priority = 50
|
||||
|
||||
[[rule]]
|
||||
toolName = ["codebase_investigator", "cli_help"]
|
||||
decision = "allow"
|
||||
priority = 50
|
||||
@@ -150,7 +150,8 @@ describe('PolicyEngine', () => {
|
||||
it('should match unqualified tool names with qualified rules when serverName is provided', async () => {
|
||||
const rules: PolicyRule[] = [
|
||||
{
|
||||
toolName: 'my-server__tool',
|
||||
toolName: 'mcp_my-server_tool',
|
||||
mcpName: 'my-server',
|
||||
decision: PolicyDecision.ALLOW,
|
||||
},
|
||||
];
|
||||
@@ -159,23 +160,9 @@ describe('PolicyEngine', () => {
|
||||
|
||||
// Match with qualified name (standard)
|
||||
expect(
|
||||
(await engine.check({ name: 'my-server__tool' }, 'my-server')).decision,
|
||||
(await engine.check({ name: 'mcp_my-server_tool' }, 'my-server'))
|
||||
.decision,
|
||||
).toBe(PolicyDecision.ALLOW);
|
||||
|
||||
// Match with unqualified name + serverName (the fix)
|
||||
expect((await engine.check({ name: 'tool' }, 'my-server')).decision).toBe(
|
||||
PolicyDecision.ALLOW,
|
||||
);
|
||||
|
||||
// Should NOT match with unqualified name but NO serverName
|
||||
expect((await engine.check({ name: 'tool' }, undefined)).decision).toBe(
|
||||
PolicyDecision.ASK_USER,
|
||||
);
|
||||
|
||||
// Should NOT match with unqualified name but WRONG serverName
|
||||
expect(
|
||||
(await engine.check({ name: 'tool' }, 'wrong-server')).decision,
|
||||
).toBe(PolicyDecision.ASK_USER);
|
||||
});
|
||||
|
||||
it('should match by args pattern', async () => {
|
||||
@@ -476,61 +463,41 @@ describe('PolicyEngine', () => {
|
||||
(await engine.check({ name: 'read_file' }, undefined)).decision,
|
||||
).toBe(PolicyDecision.ALLOW);
|
||||
expect(
|
||||
(await engine.check({ name: 'my-server__tool' }, 'my-server')).decision,
|
||||
(await engine.check({ name: 'mcp_my-server_tool' }, 'my-server'))
|
||||
.decision,
|
||||
).toBe(PolicyDecision.ALLOW);
|
||||
});
|
||||
|
||||
it('should match any MCP tool when toolName is *__*', async () => {
|
||||
it('should match any MCP tool when toolName is mcp_*', async () => {
|
||||
engine = new PolicyEngine({
|
||||
rules: [
|
||||
{ toolName: '*__*', decision: PolicyDecision.ALLOW, priority: 10 },
|
||||
{ toolName: 'mcp_*', decision: PolicyDecision.ALLOW, priority: 10 },
|
||||
],
|
||||
defaultDecision: PolicyDecision.DENY,
|
||||
});
|
||||
|
||||
expect((await engine.check({ name: 'mcp__tool' }, 'mcp')).decision).toBe(
|
||||
PolicyDecision.ALLOW,
|
||||
);
|
||||
expect(
|
||||
(await engine.check({ name: 'other__tool' }, 'other')).decision,
|
||||
(await engine.check({ name: 'mcp_mcp_tool' }, 'mcp')).decision,
|
||||
).toBe(PolicyDecision.ALLOW);
|
||||
expect(
|
||||
(await engine.check({ name: 'mcp_other_tool' }, 'other')).decision,
|
||||
).toBe(PolicyDecision.ALLOW);
|
||||
expect(
|
||||
(await engine.check({ name: 'read_file' }, undefined)).decision,
|
||||
).toBe(PolicyDecision.DENY);
|
||||
});
|
||||
|
||||
it('should match specific tool across all servers when using *__tool', async () => {
|
||||
engine = new PolicyEngine({
|
||||
rules: [
|
||||
{
|
||||
toolName: '*__search',
|
||||
decision: PolicyDecision.ALLOW,
|
||||
priority: 10,
|
||||
},
|
||||
],
|
||||
defaultDecision: PolicyDecision.DENY,
|
||||
});
|
||||
|
||||
expect((await engine.check({ name: 'ws__search' }, 'ws')).decision).toBe(
|
||||
PolicyDecision.ALLOW,
|
||||
);
|
||||
expect((await engine.check({ name: 'gh__search' }, 'gh')).decision).toBe(
|
||||
PolicyDecision.ALLOW,
|
||||
);
|
||||
expect((await engine.check({ name: 'gh__list' }, 'gh')).decision).toBe(
|
||||
PolicyDecision.DENY,
|
||||
);
|
||||
});
|
||||
|
||||
it('should match MCP server wildcard patterns', async () => {
|
||||
const rules: PolicyRule[] = [
|
||||
{
|
||||
toolName: 'my-server__*',
|
||||
toolName: 'mcp_my-server_*',
|
||||
mcpName: 'my-server',
|
||||
decision: PolicyDecision.ALLOW,
|
||||
priority: 10,
|
||||
},
|
||||
{
|
||||
toolName: 'blocked-server__*',
|
||||
toolName: 'mcp_blocked-server_*',
|
||||
mcpName: 'blocked-server',
|
||||
decision: PolicyDecision.DENY,
|
||||
priority: 20,
|
||||
},
|
||||
@@ -540,19 +507,23 @@ describe('PolicyEngine', () => {
|
||||
|
||||
// Should match my-server tools
|
||||
expect(
|
||||
(await engine.check({ name: 'my-server__tool1' }, 'my-server'))
|
||||
(await engine.check({ name: 'mcp_my-server_tool1' }, 'my-server'))
|
||||
.decision,
|
||||
).toBe(PolicyDecision.ALLOW);
|
||||
expect(
|
||||
(await engine.check({ name: 'my-server__another_tool' }, 'my-server'))
|
||||
.decision,
|
||||
(
|
||||
await engine.check(
|
||||
{ name: 'mcp_my-server_another_tool' },
|
||||
'my-server',
|
||||
)
|
||||
).decision,
|
||||
).toBe(PolicyDecision.ALLOW);
|
||||
|
||||
// Should match blocked-server tools
|
||||
expect(
|
||||
(
|
||||
await engine.check(
|
||||
{ name: 'blocked-server__tool1' },
|
||||
{ name: 'mcp_blocked-server_tool1' },
|
||||
'blocked-server',
|
||||
)
|
||||
).decision,
|
||||
@@ -560,7 +531,7 @@ describe('PolicyEngine', () => {
|
||||
expect(
|
||||
(
|
||||
await engine.check(
|
||||
{ name: 'blocked-server__dangerous' },
|
||||
{ name: 'mcp_blocked-server_dangerous' },
|
||||
'blocked-server',
|
||||
)
|
||||
).decision,
|
||||
@@ -568,7 +539,7 @@ describe('PolicyEngine', () => {
|
||||
|
||||
// Should not match other patterns
|
||||
expect(
|
||||
(await engine.check({ name: 'other-server__tool' }, 'other-server'))
|
||||
(await engine.check({ name: 'mcp_other-server_tool' }, 'other-server'))
|
||||
.decision,
|
||||
).toBe(PolicyDecision.ASK_USER);
|
||||
expect(
|
||||
@@ -582,12 +553,14 @@ describe('PolicyEngine', () => {
|
||||
it('should prioritize specific tool rules over server wildcards', async () => {
|
||||
const rules: PolicyRule[] = [
|
||||
{
|
||||
toolName: 'my-server__*',
|
||||
toolName: 'mcp_my-server_*',
|
||||
mcpName: 'my-server',
|
||||
decision: PolicyDecision.ALLOW,
|
||||
priority: 10,
|
||||
},
|
||||
{
|
||||
toolName: 'my-server__dangerous-tool',
|
||||
toolName: 'mcp_my-server_dangerous-tool',
|
||||
mcpName: 'my-server',
|
||||
decision: PolicyDecision.DENY,
|
||||
priority: 20,
|
||||
},
|
||||
@@ -597,33 +570,38 @@ describe('PolicyEngine', () => {
|
||||
|
||||
// Specific tool deny should override server allow
|
||||
expect(
|
||||
(await engine.check({ name: 'my-server__dangerous-tool' }, 'my-server'))
|
||||
.decision,
|
||||
(
|
||||
await engine.check(
|
||||
{ name: 'mcp_my-server_dangerous-tool' },
|
||||
'my-server',
|
||||
)
|
||||
).decision,
|
||||
).toBe(PolicyDecision.DENY);
|
||||
expect(
|
||||
(await engine.check({ name: 'my-server__safe-tool' }, 'my-server'))
|
||||
(await engine.check({ name: 'mcp_my-server_safe-tool' }, 'my-server'))
|
||||
.decision,
|
||||
).toBe(PolicyDecision.ALLOW);
|
||||
});
|
||||
|
||||
it('should NOT match spoofed server names when using wildcards', async () => {
|
||||
// Vulnerability: A rule for 'prefix__*' matches 'prefix__suffix__tool'
|
||||
// effectively allowing a server named 'prefix__suffix' to spoof 'prefix'.
|
||||
// Vulnerability: A rule for 'mcp_prefix_*' matches 'mcp_prefix__suffix_tool'
|
||||
// effectively allowing a server named 'mcp_prefix_suffix' to spoof 'prefix'.
|
||||
const rules: PolicyRule[] = [
|
||||
{
|
||||
toolName: 'safe_server__*',
|
||||
toolName: 'mcp_safe_server_*',
|
||||
mcpName: 'safe_server',
|
||||
decision: PolicyDecision.ALLOW,
|
||||
},
|
||||
];
|
||||
engine = new PolicyEngine({ rules });
|
||||
|
||||
// A tool from a different server 'safe_server__malicious'
|
||||
const spoofedToolCall = { name: 'safe_server__malicious__tool' };
|
||||
// A tool from a different server 'mcp_safe_server_malicious'
|
||||
const spoofedToolCall = { name: 'mcp_mcp_safe_server_malicious_tool' };
|
||||
|
||||
// CURRENT BEHAVIOR (FIXED): Matches because it starts with 'safe_server__' BUT serverName doesn't match 'safe_server'
|
||||
// We expect this to FAIL matching the ALLOW rule, thus falling back to default (ASK_USER)
|
||||
expect(
|
||||
(await engine.check(spoofedToolCall, 'safe_server__malicious'))
|
||||
(await engine.check(spoofedToolCall, 'mcp_safe_server_malicious'))
|
||||
.decision,
|
||||
).toBe(PolicyDecision.ASK_USER);
|
||||
});
|
||||
@@ -631,14 +609,15 @@ describe('PolicyEngine', () => {
|
||||
it('should verify tool name prefix even if serverName matches', async () => {
|
||||
const rules: PolicyRule[] = [
|
||||
{
|
||||
toolName: 'safe_server__*',
|
||||
toolName: 'mcp_safe_server_*',
|
||||
mcpName: 'safe_server',
|
||||
decision: PolicyDecision.ALLOW,
|
||||
},
|
||||
];
|
||||
engine = new PolicyEngine({ rules });
|
||||
|
||||
// serverName matches, but tool name does not start with prefix
|
||||
const invalidToolCall = { name: 'other_server__tool' };
|
||||
const invalidToolCall = { name: 'mcp_other_server_tool' };
|
||||
expect(
|
||||
(await engine.check(invalidToolCall, 'safe_server')).decision,
|
||||
).toBe(PolicyDecision.ASK_USER);
|
||||
@@ -647,13 +626,14 @@ describe('PolicyEngine', () => {
|
||||
it('should allow when both serverName and tool name prefix match', async () => {
|
||||
const rules: PolicyRule[] = [
|
||||
{
|
||||
toolName: 'safe_server__*',
|
||||
toolName: 'mcp_safe_server_*',
|
||||
mcpName: 'safe_server',
|
||||
decision: PolicyDecision.ALLOW,
|
||||
},
|
||||
];
|
||||
engine = new PolicyEngine({ rules });
|
||||
|
||||
const validToolCall = { name: 'safe_server__tool' };
|
||||
const validToolCall = { name: 'mcp_safe_server_tool' };
|
||||
expect((await engine.check(validToolCall, 'safe_server')).decision).toBe(
|
||||
PolicyDecision.ALLOW,
|
||||
);
|
||||
@@ -2007,11 +1987,16 @@ describe('PolicyEngine', () => {
|
||||
|
||||
it('should support wildcard patterns for checkers', async () => {
|
||||
const rules: PolicyRule[] = [
|
||||
{ toolName: 'server__tool', decision: PolicyDecision.ALLOW },
|
||||
{
|
||||
toolName: 'mcp_server_tool',
|
||||
mcpName: 'server',
|
||||
decision: PolicyDecision.ALLOW,
|
||||
},
|
||||
];
|
||||
const wildcardChecker: SafetyCheckerRule = {
|
||||
checker: { type: 'external', name: 'wildcard' },
|
||||
toolName: 'server__*',
|
||||
toolName: 'mcp_server_*',
|
||||
mcpName: 'server',
|
||||
};
|
||||
|
||||
engine = new PolicyEngine(
|
||||
@@ -2023,7 +2008,7 @@ describe('PolicyEngine', () => {
|
||||
decision: SafetyCheckDecision.ALLOW,
|
||||
});
|
||||
|
||||
await engine.check({ name: 'server__tool' }, 'server');
|
||||
await engine.check({ name: 'mcp_server_tool' }, 'server');
|
||||
|
||||
expect(mockCheckerRunner.runChecker).toHaveBeenCalledWith(
|
||||
expect.anything(),
|
||||
@@ -2137,6 +2122,8 @@ describe('PolicyEngine', () => {
|
||||
rules: PolicyRule[];
|
||||
approvalMode?: ApprovalMode;
|
||||
nonInteractive?: boolean;
|
||||
allToolNames?: string[];
|
||||
metadata?: Map<string, Record<string, unknown>>;
|
||||
expected: string[];
|
||||
}
|
||||
|
||||
@@ -2144,11 +2131,13 @@ describe('PolicyEngine', () => {
|
||||
{
|
||||
name: 'should return empty set when no rules provided',
|
||||
rules: [],
|
||||
allToolNames: ['tool1'],
|
||||
expected: [],
|
||||
},
|
||||
{
|
||||
name: 'should apply rules without explicit modes to all modes',
|
||||
rules: [{ toolName: 'tool1', decision: PolicyDecision.DENY }],
|
||||
allToolNames: ['tool1', 'tool2'],
|
||||
expected: ['tool1'],
|
||||
},
|
||||
{
|
||||
@@ -2168,6 +2157,7 @@ describe('PolicyEngine', () => {
|
||||
modes: [ApprovalMode.DEFAULT],
|
||||
},
|
||||
],
|
||||
allToolNames: ['tool1'],
|
||||
expected: [],
|
||||
},
|
||||
{
|
||||
@@ -2184,6 +2174,7 @@ describe('PolicyEngine', () => {
|
||||
modes: [ApprovalMode.DEFAULT],
|
||||
},
|
||||
],
|
||||
allToolNames: ['tool1', 'tool2', 'tool3'],
|
||||
expected: ['tool1'],
|
||||
},
|
||||
{
|
||||
@@ -2202,6 +2193,7 @@ describe('PolicyEngine', () => {
|
||||
modes: [ApprovalMode.DEFAULT],
|
||||
},
|
||||
],
|
||||
allToolNames: ['tool1'],
|
||||
expected: ['tool1'],
|
||||
},
|
||||
{
|
||||
@@ -2220,6 +2212,7 @@ describe('PolicyEngine', () => {
|
||||
modes: [ApprovalMode.DEFAULT],
|
||||
},
|
||||
],
|
||||
allToolNames: ['tool1'],
|
||||
expected: [],
|
||||
},
|
||||
{
|
||||
@@ -2232,7 +2225,8 @@ describe('PolicyEngine', () => {
|
||||
},
|
||||
],
|
||||
nonInteractive: true,
|
||||
expected: [],
|
||||
allToolNames: ['tool1'],
|
||||
expected: ['tool1'],
|
||||
},
|
||||
{
|
||||
name: 'should ignore rules with argsPattern',
|
||||
@@ -2244,6 +2238,7 @@ describe('PolicyEngine', () => {
|
||||
modes: [ApprovalMode.DEFAULT],
|
||||
},
|
||||
],
|
||||
allToolNames: ['tool1'],
|
||||
expected: [],
|
||||
},
|
||||
{
|
||||
@@ -2256,6 +2251,7 @@ describe('PolicyEngine', () => {
|
||||
},
|
||||
],
|
||||
approvalMode: ApprovalMode.PLAN,
|
||||
allToolNames: ['tool1'],
|
||||
expected: ['tool1'],
|
||||
},
|
||||
{
|
||||
@@ -2268,6 +2264,7 @@ describe('PolicyEngine', () => {
|
||||
},
|
||||
],
|
||||
approvalMode: ApprovalMode.DEFAULT,
|
||||
allToolNames: ['tool1'],
|
||||
expected: [],
|
||||
},
|
||||
{
|
||||
@@ -2286,36 +2283,55 @@ describe('PolicyEngine', () => {
|
||||
},
|
||||
],
|
||||
approvalMode: ApprovalMode.YOLO,
|
||||
allToolNames: ['dangerous-tool', 'safe-tool'],
|
||||
expected: [],
|
||||
},
|
||||
{
|
||||
name: 'should respect server wildcard DENY',
|
||||
rules: [
|
||||
{
|
||||
toolName: 'server__*',
|
||||
toolName: 'mcp_server_*',
|
||||
mcpName: 'server',
|
||||
decision: PolicyDecision.DENY,
|
||||
modes: [ApprovalMode.DEFAULT],
|
||||
},
|
||||
],
|
||||
expected: ['server__*'],
|
||||
allToolNames: [
|
||||
'mcp_server_tool1',
|
||||
'mcp_server_tool2',
|
||||
'mcp_other_tool',
|
||||
],
|
||||
metadata: new Map([
|
||||
['mcp_server_tool1', { _serverName: 'server' }],
|
||||
['mcp_server_tool2', { _serverName: 'server' }],
|
||||
['mcp_other_tool', { _serverName: 'other' }],
|
||||
]),
|
||||
expected: ['mcp_server_tool1', 'mcp_server_tool2'],
|
||||
},
|
||||
{
|
||||
name: 'should expand server wildcard for specific tools if already processed',
|
||||
rules: [
|
||||
{
|
||||
toolName: 'server__*',
|
||||
toolName: 'mcp_server_*',
|
||||
mcpName: 'server',
|
||||
decision: PolicyDecision.DENY,
|
||||
priority: 100,
|
||||
modes: [ApprovalMode.DEFAULT],
|
||||
},
|
||||
{
|
||||
toolName: 'server__tool1',
|
||||
decision: PolicyDecision.DENY,
|
||||
toolName: 'mcp_server_tool1',
|
||||
mcpName: 'server',
|
||||
decision: PolicyDecision.DENY, // redundant but tests ordering
|
||||
priority: 10,
|
||||
modes: [ApprovalMode.DEFAULT],
|
||||
},
|
||||
],
|
||||
expected: ['server__*', 'server__tool1'],
|
||||
allToolNames: ['mcp_server_tool1', 'mcp_server_tool2'],
|
||||
metadata: new Map([
|
||||
['mcp_server_tool1', { _serverName: 'server' }],
|
||||
['mcp_server_tool2', { _serverName: 'server' }],
|
||||
]),
|
||||
expected: ['mcp_server_tool1', 'mcp_server_tool2'],
|
||||
},
|
||||
{
|
||||
name: 'should exclude run_shell_command but NOT write_file in simulated Plan Mode',
|
||||
@@ -2342,24 +2358,29 @@ describe('PolicyEngine', () => {
|
||||
priority: 10,
|
||||
},
|
||||
],
|
||||
expected: ['run_shell_command'],
|
||||
allToolNames: ['write_file', 'run_shell_command', 'read_file'],
|
||||
expected: ['run_shell_command', 'read_file'],
|
||||
},
|
||||
{
|
||||
name: 'should NOT exclude tool if covered by a higher priority wildcard ALLOW',
|
||||
rules: [
|
||||
{
|
||||
toolName: 'server__*',
|
||||
toolName: 'mcp_server_*',
|
||||
mcpName: 'server',
|
||||
decision: PolicyDecision.ALLOW,
|
||||
priority: 100,
|
||||
modes: [ApprovalMode.DEFAULT],
|
||||
},
|
||||
{
|
||||
toolName: 'server__tool1',
|
||||
toolName: 'mcp_server_tool1',
|
||||
mcpName: 'server',
|
||||
decision: PolicyDecision.DENY,
|
||||
priority: 10,
|
||||
modes: [ApprovalMode.DEFAULT],
|
||||
},
|
||||
],
|
||||
allToolNames: ['mcp_server_tool1'],
|
||||
metadata: new Map([['mcp_server_tool1', { _serverName: 'server' }]]),
|
||||
expected: [],
|
||||
},
|
||||
{
|
||||
@@ -2371,41 +2392,63 @@ describe('PolicyEngine', () => {
|
||||
priority: 10,
|
||||
},
|
||||
],
|
||||
expected: ['*'],
|
||||
allToolNames: ['toolA', 'toolB', 'mcp_server_toolC'],
|
||||
expected: ['toolA', 'toolB', 'mcp_server_toolC'], // all tools denied by *
|
||||
},
|
||||
{
|
||||
name: 'should handle MCP category wildcard *__* in getExcludedTools',
|
||||
rules: [
|
||||
{
|
||||
toolName: '*__*',
|
||||
toolName: 'mcp_*',
|
||||
decision: PolicyDecision.DENY,
|
||||
priority: 10,
|
||||
},
|
||||
],
|
||||
expected: ['*__*'],
|
||||
allToolNames: ['localTool', 'mcp_myserver_mytool'],
|
||||
metadata: new Map([
|
||||
['mcp_myserver_mytool', { _serverName: 'myserver' }],
|
||||
]),
|
||||
expected: ['mcp_myserver_mytool'],
|
||||
},
|
||||
{
|
||||
name: 'should handle tool wildcard *__search in getExcludedTools',
|
||||
name: 'should handle tool wildcard mcp_server_* in getExcludedTools',
|
||||
rules: [
|
||||
{
|
||||
toolName: '*__search',
|
||||
toolName: 'mcp_server_*',
|
||||
decision: PolicyDecision.DENY,
|
||||
priority: 10,
|
||||
},
|
||||
],
|
||||
expected: ['*__search'],
|
||||
allToolNames: [
|
||||
'localTool',
|
||||
'mcp_server_search',
|
||||
'mcp_otherserver_read',
|
||||
],
|
||||
metadata: new Map([
|
||||
['mcp_server_search', { _serverName: 'server' }],
|
||||
['mcp_otherserver_read', { _serverName: 'otherserver' }],
|
||||
]),
|
||||
expected: ['mcp_server_search'],
|
||||
},
|
||||
];
|
||||
|
||||
it.each(testCases)(
|
||||
'$name',
|
||||
({ rules, approvalMode, nonInteractive, expected }) => {
|
||||
({
|
||||
rules,
|
||||
approvalMode,
|
||||
nonInteractive,
|
||||
allToolNames,
|
||||
metadata,
|
||||
expected,
|
||||
}) => {
|
||||
engine = new PolicyEngine({
|
||||
rules,
|
||||
approvalMode: approvalMode ?? ApprovalMode.DEFAULT,
|
||||
nonInteractive: nonInteractive ?? false,
|
||||
});
|
||||
const excluded = engine.getExcludedTools();
|
||||
const toolsSet = allToolNames ? new Set(allToolNames) : undefined;
|
||||
const excluded = engine.getExcludedTools(metadata, toolsSet);
|
||||
expect(Array.from(excluded).sort()).toEqual(expected.sort());
|
||||
},
|
||||
);
|
||||
@@ -2420,7 +2463,10 @@ describe('PolicyEngine', () => {
|
||||
},
|
||||
],
|
||||
});
|
||||
const excluded = engine.getExcludedTools();
|
||||
const excluded = engine.getExcludedTools(
|
||||
undefined,
|
||||
new Set(['dangerous_tool']),
|
||||
);
|
||||
expect(Array.from(excluded)).toEqual([]);
|
||||
});
|
||||
|
||||
@@ -2438,7 +2484,10 @@ describe('PolicyEngine', () => {
|
||||
['dangerous_tool', { destructiveHint: true }],
|
||||
['safe_tool', { readOnlyHint: true }],
|
||||
]);
|
||||
const excluded = engine.getExcludedTools(metadata);
|
||||
const excluded = engine.getExcludedTools(
|
||||
metadata,
|
||||
new Set(['dangerous_tool', 'safe_tool']),
|
||||
);
|
||||
expect(Array.from(excluded)).toEqual(['dangerous_tool']);
|
||||
});
|
||||
|
||||
@@ -2455,7 +2504,10 @@ describe('PolicyEngine', () => {
|
||||
const metadata = new Map<string, Record<string, unknown>>([
|
||||
['safe_tool', { readOnlyHint: true }],
|
||||
]);
|
||||
const excluded = engine.getExcludedTools(metadata);
|
||||
const excluded = engine.getExcludedTools(
|
||||
metadata,
|
||||
new Set(['safe_tool']),
|
||||
);
|
||||
expect(Array.from(excluded)).toEqual([]);
|
||||
});
|
||||
|
||||
@@ -2463,7 +2515,8 @@ describe('PolicyEngine', () => {
|
||||
engine = new PolicyEngine({
|
||||
rules: [
|
||||
{
|
||||
toolName: 'server__*',
|
||||
toolName: 'mcp_server_*',
|
||||
mcpName: 'server',
|
||||
toolAnnotations: { destructiveHint: true },
|
||||
decision: PolicyDecision.DENY,
|
||||
priority: 10,
|
||||
@@ -2471,12 +2524,25 @@ describe('PolicyEngine', () => {
|
||||
],
|
||||
});
|
||||
const metadata = new Map<string, Record<string, unknown>>([
|
||||
['server__dangerous_tool', { destructiveHint: true }],
|
||||
['other__dangerous_tool', { destructiveHint: true }],
|
||||
['server__safe_tool', { readOnlyHint: true }],
|
||||
[
|
||||
'mcp_server_dangerous_tool',
|
||||
{ destructiveHint: true, _serverName: 'server' },
|
||||
],
|
||||
[
|
||||
'mcp_other_dangerous_tool',
|
||||
{ destructiveHint: true, _serverName: 'other' },
|
||||
],
|
||||
['mcp_server_safe_tool', { readOnlyHint: true, _serverName: 'server' }],
|
||||
]);
|
||||
const excluded = engine.getExcludedTools(metadata);
|
||||
expect(Array.from(excluded)).toEqual(['server__dangerous_tool']);
|
||||
const excluded = engine.getExcludedTools(
|
||||
metadata,
|
||||
new Set([
|
||||
'mcp_server_dangerous_tool',
|
||||
'mcp_other_dangerous_tool',
|
||||
'mcp_server_safe_tool',
|
||||
]),
|
||||
);
|
||||
expect(Array.from(excluded)).toEqual(['mcp_server_dangerous_tool']);
|
||||
});
|
||||
|
||||
it('should exclude unprocessed tools from allToolNames when global DENY is active', () => {
|
||||
@@ -2493,8 +2559,8 @@ describe('PolicyEngine', () => {
|
||||
priority: 70,
|
||||
},
|
||||
{
|
||||
// Simulates plan.toml: mcpName="*" → toolName="*__*"
|
||||
toolName: '*__*',
|
||||
// Simulates plan.toml: mcpName="*" → toolName="mcp_*"
|
||||
toolName: 'mcp_*',
|
||||
toolAnnotations: { readOnlyHint: true },
|
||||
decision: PolicyDecision.ASK_USER,
|
||||
priority: 70,
|
||||
@@ -2505,36 +2571,42 @@ describe('PolicyEngine', () => {
|
||||
},
|
||||
],
|
||||
});
|
||||
// MCP tools are registered with unqualified names in ToolRegistry
|
||||
// MCP tools are registered with qualified names in ToolRegistry
|
||||
const allToolNames = new Set([
|
||||
'glob',
|
||||
'read_file',
|
||||
'shell',
|
||||
'web_fetch',
|
||||
'read_mcp_tool',
|
||||
'write_mcp_tool',
|
||||
'mcp_my-server_read_mcp_tool',
|
||||
'mcp_my-server_write_mcp_tool',
|
||||
]);
|
||||
// buildToolMetadata() includes _serverName for MCP tools
|
||||
const toolMetadata = new Map<string, Record<string, unknown>>([
|
||||
['read_mcp_tool', { readOnlyHint: true, _serverName: 'my-server' }],
|
||||
['write_mcp_tool', { readOnlyHint: false, _serverName: 'my-server' }],
|
||||
[
|
||||
'mcp_my-server_read_mcp_tool',
|
||||
{ readOnlyHint: true, _serverName: 'my-server' },
|
||||
],
|
||||
[
|
||||
'mcp_my-server_write_mcp_tool',
|
||||
{ readOnlyHint: false, _serverName: 'my-server' },
|
||||
],
|
||||
]);
|
||||
const excluded = engine.getExcludedTools(toolMetadata, allToolNames);
|
||||
expect(excluded.has('shell')).toBe(true);
|
||||
expect(excluded.has('web_fetch')).toBe(true);
|
||||
// Non-read-only MCP tool excluded by catch-all DENY
|
||||
expect(excluded.has('write_mcp_tool')).toBe(true);
|
||||
expect(excluded.has('mcp_my-server_write_mcp_tool')).toBe(true);
|
||||
expect(excluded.has('glob')).toBe(false);
|
||||
expect(excluded.has('read_file')).toBe(false);
|
||||
// Read-only MCP tool allowed by annotation rule
|
||||
expect(excluded.has('read_mcp_tool')).toBe(false);
|
||||
expect(excluded.has('mcp_my-server_read_mcp_tool')).toBe(false);
|
||||
});
|
||||
|
||||
it('should match already-qualified MCP tool names without _serverName', () => {
|
||||
it('should match MCP wildcard rules when explicitly mapped with _serverName', () => {
|
||||
engine = new PolicyEngine({
|
||||
rules: [
|
||||
{
|
||||
toolName: '*__*',
|
||||
toolName: 'mcp_*',
|
||||
toolAnnotations: { readOnlyHint: true },
|
||||
decision: PolicyDecision.ASK_USER,
|
||||
priority: 70,
|
||||
@@ -2547,17 +2619,23 @@ describe('PolicyEngine', () => {
|
||||
});
|
||||
// Tool registered with qualified name (collision case)
|
||||
const allToolNames = new Set([
|
||||
'myserver__read_tool',
|
||||
'myserver__write_tool',
|
||||
'mcp_myserver_read_tool',
|
||||
'mcp_myserver_write_tool',
|
||||
]);
|
||||
const toolMetadata = new Map<string, Record<string, unknown>>([
|
||||
['myserver__read_tool', { readOnlyHint: true }],
|
||||
['myserver__write_tool', { readOnlyHint: false }],
|
||||
[
|
||||
'mcp_myserver_read_tool',
|
||||
{ readOnlyHint: true, _serverName: 'myserver' },
|
||||
],
|
||||
[
|
||||
'mcp_myserver_write_tool',
|
||||
{ readOnlyHint: false, _serverName: 'myserver' },
|
||||
],
|
||||
]);
|
||||
const excluded = engine.getExcludedTools(toolMetadata, allToolNames);
|
||||
// Qualified name already contains __, matched directly without _serverName
|
||||
expect(excluded.has('myserver__read_tool')).toBe(false);
|
||||
expect(excluded.has('myserver__write_tool')).toBe(true);
|
||||
// Qualified name matched using explicit _serverName
|
||||
expect(excluded.has('mcp_myserver_read_tool')).toBe(false);
|
||||
expect(excluded.has('mcp_myserver_write_tool')).toBe(true);
|
||||
});
|
||||
|
||||
it('should not exclude unprocessed tools when allToolNames is not provided (backward compat)', () => {
|
||||
@@ -2648,7 +2726,7 @@ describe('PolicyEngine', () => {
|
||||
modes: [ApprovalMode.PLAN],
|
||||
},
|
||||
{
|
||||
toolName: '*__*',
|
||||
toolName: 'mcp_*',
|
||||
toolAnnotations: { readOnlyHint: true },
|
||||
decision: PolicyDecision.ASK_USER,
|
||||
priority: 70,
|
||||
@@ -2679,13 +2757,19 @@ describe('PolicyEngine', () => {
|
||||
'write_todos',
|
||||
'memory',
|
||||
'save_memory',
|
||||
'read_tool',
|
||||
'write_tool',
|
||||
'mcp_mcp-server_read_tool',
|
||||
'mcp_mcp-server_write_tool',
|
||||
]);
|
||||
// buildToolMetadata() includes _serverName for MCP tools
|
||||
const toolMetadata = new Map<string, Record<string, unknown>>([
|
||||
['read_tool', { readOnlyHint: true, _serverName: 'mcp-server' }],
|
||||
['write_tool', { readOnlyHint: false, _serverName: 'mcp-server' }],
|
||||
[
|
||||
'mcp_mcp-server_read_tool',
|
||||
{ readOnlyHint: true, _serverName: 'mcp-server' },
|
||||
],
|
||||
[
|
||||
'mcp_mcp-server_write_tool',
|
||||
{ readOnlyHint: false, _serverName: 'mcp-server' },
|
||||
],
|
||||
]);
|
||||
const excluded = engine.getExcludedTools(toolMetadata, allToolNames);
|
||||
// These should be excluded (caught by catch-all DENY)
|
||||
@@ -2698,7 +2782,7 @@ describe('PolicyEngine', () => {
|
||||
expect(excluded.has('write_file')).toBe(true);
|
||||
expect(excluded.has('replace')).toBe(true);
|
||||
// Non-read-only MCP tool excluded by catch-all DENY
|
||||
expect(excluded.has('write_tool')).toBe(true);
|
||||
expect(excluded.has('mcp_mcp-server_write_tool')).toBe(true);
|
||||
// These should NOT be excluded (explicitly allowed)
|
||||
expect(excluded.has('glob')).toBe(false);
|
||||
expect(excluded.has('grep_search')).toBe(false);
|
||||
@@ -2710,7 +2794,7 @@ describe('PolicyEngine', () => {
|
||||
expect(excluded.has('exit_plan_mode')).toBe(false);
|
||||
expect(excluded.has('save_memory')).toBe(false);
|
||||
// Read-only MCP tool allowed by annotation rule (matched via _serverName)
|
||||
expect(excluded.has('read_tool')).toBe(false);
|
||||
expect(excluded.has('mcp_mcp-server_read_tool')).toBe(false);
|
||||
});
|
||||
});
|
||||
|
||||
@@ -3058,13 +3142,13 @@ describe('PolicyEngine', () => {
|
||||
engine = new PolicyEngine({
|
||||
rules: [
|
||||
{
|
||||
toolName: '*__*',
|
||||
toolName: 'mcp_*',
|
||||
toolAnnotations: { experimental: true },
|
||||
decision: PolicyDecision.DENY,
|
||||
priority: 20,
|
||||
},
|
||||
{
|
||||
toolName: '*__*',
|
||||
toolName: 'mcp_*',
|
||||
decision: PolicyDecision.ALLOW,
|
||||
priority: 10,
|
||||
},
|
||||
@@ -3073,14 +3157,14 @@ describe('PolicyEngine', () => {
|
||||
|
||||
expect(
|
||||
(
|
||||
await engine.check({ name: 'mcp__test' }, 'mcp', {
|
||||
await engine.check({ name: 'mcp_mcp_test' }, 'mcp', {
|
||||
experimental: true,
|
||||
})
|
||||
).decision,
|
||||
).toBe(PolicyDecision.DENY);
|
||||
expect(
|
||||
(
|
||||
await engine.check({ name: 'mcp__stable' }, 'mcp', {
|
||||
await engine.check({ name: 'mcp_mcp_stable' }, 'mcp', {
|
||||
experimental: false,
|
||||
})
|
||||
).decision,
|
||||
|
||||
@@ -25,6 +25,11 @@ import {
|
||||
hasRedirection,
|
||||
} from '../utils/shell-utils.js';
|
||||
import { getToolAliases } from '../tools/tool-names.js';
|
||||
import {
|
||||
MCP_TOOL_PREFIX,
|
||||
isMcpToolAnnotation,
|
||||
parseMcpToolName,
|
||||
} from '../tools/mcp-tool.js';
|
||||
|
||||
function isWildcardPattern(name: string): boolean {
|
||||
return name === '*' || name.includes('*');
|
||||
@@ -32,7 +37,7 @@ function isWildcardPattern(name: string): boolean {
|
||||
|
||||
/**
|
||||
* Checks if a tool call matches a wildcard pattern.
|
||||
* Supports global (*) and composite (server__*, *__tool, *__*) patterns.
|
||||
* Supports global (*) and the explicit MCP (*mcp_serverName_**) format.
|
||||
*/
|
||||
function matchesWildcard(
|
||||
pattern: string,
|
||||
@@ -43,59 +48,25 @@ function matchesWildcard(
|
||||
return true;
|
||||
}
|
||||
|
||||
if (pattern.includes('__')) {
|
||||
return matchesCompositePattern(pattern, toolName, serverName);
|
||||
if (pattern === `${MCP_TOOL_PREFIX}*`) {
|
||||
return serverName !== undefined;
|
||||
}
|
||||
|
||||
if (pattern.startsWith(MCP_TOOL_PREFIX) && pattern.endsWith('_*')) {
|
||||
const expectedServerName = pattern.slice(MCP_TOOL_PREFIX.length, -2);
|
||||
// 1. Must be an MCP tool call (has serverName)
|
||||
// 2. Server name must match
|
||||
// 3. Tool name must be properly qualified by that server
|
||||
if (serverName === undefined || serverName !== expectedServerName) {
|
||||
return false;
|
||||
}
|
||||
return toolName.startsWith(`${MCP_TOOL_PREFIX}${expectedServerName}_`);
|
||||
}
|
||||
|
||||
// Not a recognized wildcard pattern, fallback to exact match just in case
|
||||
return toolName === pattern;
|
||||
}
|
||||
|
||||
/**
|
||||
* Matches composite patterns like "server__*", "*__tool", or "*__*".
|
||||
*/
|
||||
function matchesCompositePattern(
|
||||
pattern: string,
|
||||
toolName: string,
|
||||
serverName: string | undefined,
|
||||
): boolean {
|
||||
const parts = pattern.split('__');
|
||||
if (parts.length !== 2) return false;
|
||||
const [patternServer, patternTool] = parts;
|
||||
|
||||
// 1. Identify the tool's components
|
||||
const { actualServer, actualTool } = getToolMetadata(toolName, serverName);
|
||||
|
||||
// 2. Composite patterns require a server context
|
||||
if (actualServer === undefined) {
|
||||
return false;
|
||||
}
|
||||
|
||||
// 3. Robustness: if serverName is provided, toolName MUST be qualified by it.
|
||||
// This prevents "malicious-server" from spoofing "trusted-server" by naming itself "trusted-server__malicious".
|
||||
if (serverName !== undefined && !toolName.startsWith(serverName + '__')) {
|
||||
return false;
|
||||
}
|
||||
|
||||
// 4. Match components
|
||||
const serverMatch = patternServer === '*' || patternServer === actualServer;
|
||||
const toolMatch = patternTool === '*' || patternTool === actualTool;
|
||||
|
||||
return serverMatch && toolMatch;
|
||||
}
|
||||
|
||||
/**
|
||||
* Extracts the server and unqualified tool name from a tool call context.
|
||||
*/
|
||||
function getToolMetadata(toolName: string, serverName: string | undefined) {
|
||||
const sepIndex = toolName.indexOf('__');
|
||||
const isQualified = sepIndex !== -1;
|
||||
return {
|
||||
actualServer:
|
||||
serverName ?? (isQualified ? toolName.substring(0, sepIndex) : undefined),
|
||||
actualTool: isQualified ? toolName.substring(sepIndex + 2) : toolName,
|
||||
};
|
||||
}
|
||||
|
||||
function ruleMatches(
|
||||
rule: PolicyRule | SafetyCheckerRule,
|
||||
toolCall: FunctionCall,
|
||||
@@ -111,9 +82,20 @@ function ruleMatches(
|
||||
}
|
||||
}
|
||||
|
||||
// Strictly enforce mcpName identity if the rule dictates it
|
||||
if (rule.mcpName) {
|
||||
if (rule.mcpName === '*') {
|
||||
// Rule requires it to be ANY MCP tool
|
||||
if (serverName === undefined) return false;
|
||||
} else {
|
||||
// Rule requires it to be a specific MCP server
|
||||
if (serverName !== rule.mcpName) return false;
|
||||
}
|
||||
}
|
||||
|
||||
// Check tool name if specified
|
||||
if (rule.toolName) {
|
||||
// Support wildcard patterns: "serverName__*" matches "serverName__anyTool"
|
||||
// Support wildcard patterns: "mcp_serverName_*" matches "mcp_serverName_anyTool"
|
||||
if (rule.toolName === '*') {
|
||||
// Match all tools
|
||||
} else if (isWildcardPattern(rule.toolName)) {
|
||||
@@ -371,6 +353,22 @@ export class PolicyEngine {
|
||||
serverName: string | undefined,
|
||||
toolAnnotations?: Record<string, unknown>,
|
||||
): Promise<CheckResult> {
|
||||
// Case 1: Metadata injection is the primary and safest way to identify an MCP server.
|
||||
// If we have explicit `_serverName` metadata (usually injected by tool-registry for active tools), use it.
|
||||
if (!serverName && isMcpToolAnnotation(toolAnnotations)) {
|
||||
serverName = toolAnnotations._serverName;
|
||||
}
|
||||
|
||||
// Case 2: Fallback for static FQN strings (e.g. from TOML policies or allowed/excluded settings strings).
|
||||
// These strings don't have active metadata objects associated with them during policy generation,
|
||||
// so we must extract the server name from the qualified `mcp_{server}_{tool}` format.
|
||||
if (!serverName && toolCall.name) {
|
||||
const parsed = parseMcpToolName(toolCall.name);
|
||||
if (parsed.serverName) {
|
||||
serverName = parsed.serverName;
|
||||
}
|
||||
}
|
||||
|
||||
let stringifiedArgs: string | undefined;
|
||||
// Compute stringified args once before the loop
|
||||
if (
|
||||
@@ -404,20 +402,12 @@ export class PolicyEngine {
|
||||
let matchedRule: PolicyRule | undefined;
|
||||
let decision: PolicyDecision | undefined;
|
||||
|
||||
// For tools with a server name, we want to try matching both the
|
||||
// original name and the fully qualified name (server__tool).
|
||||
// We also want to check legacy aliases for the tool name.
|
||||
const toolNamesToTry = toolCall.name ? getToolAliases(toolCall.name) : [];
|
||||
|
||||
const toolCallsToTry: FunctionCall[] = [];
|
||||
for (const name of toolNamesToTry) {
|
||||
toolCallsToTry.push({ ...toolCall, name });
|
||||
if (serverName && !name.includes('__')) {
|
||||
toolCallsToTry.push({
|
||||
...toolCall,
|
||||
name: `${serverName}__${name}`,
|
||||
});
|
||||
}
|
||||
}
|
||||
|
||||
for (const rule of this.rules) {
|
||||
@@ -654,145 +644,69 @@ export class PolicyEngine {
|
||||
allToolNames?: Set<string>,
|
||||
): Set<string> {
|
||||
const excludedTools = new Set<string>();
|
||||
const processedTools = new Set<string>();
|
||||
let globalVerdict: PolicyDecision | undefined;
|
||||
|
||||
for (const rule of this.rules) {
|
||||
if (rule.argsPattern) {
|
||||
if (rule.toolName && rule.decision !== PolicyDecision.DENY) {
|
||||
processedTools.add(rule.toolName);
|
||||
}
|
||||
continue;
|
||||
}
|
||||
|
||||
// Check if rule applies to current approval mode
|
||||
if (rule.modes && rule.modes.length > 0) {
|
||||
if (!rule.modes.includes(this.approvalMode)) {
|
||||
continue;
|
||||
}
|
||||
}
|
||||
|
||||
// Handle annotation-based rules
|
||||
if (rule.toolAnnotations) {
|
||||
if (!toolMetadata) {
|
||||
// Without metadata, we can't evaluate annotation rules — skip (conservative fallback)
|
||||
continue;
|
||||
}
|
||||
// Iterate over all known tools and check if their annotations match this rule
|
||||
for (const [toolName, annotations] of toolMetadata) {
|
||||
if (processedTools.has(toolName)) {
|
||||
continue;
|
||||
}
|
||||
// Check if annotations match the rule's toolAnnotations (partial match)
|
||||
let annotationsMatch = true;
|
||||
for (const [key, value] of Object.entries(rule.toolAnnotations)) {
|
||||
if (annotations[key] !== value) {
|
||||
annotationsMatch = false;
|
||||
break;
|
||||
}
|
||||
}
|
||||
if (!annotationsMatch) {
|
||||
continue;
|
||||
}
|
||||
// Check if the tool name matches the rule's toolName pattern (if any)
|
||||
if (rule.toolName) {
|
||||
if (isWildcardPattern(rule.toolName)) {
|
||||
// For composite patterns (e.g. "*__*"), construct a qualified
|
||||
// name from metadata so matchesWildcard can resolve it.
|
||||
const rawServerName = annotations['_serverName'];
|
||||
const serverName =
|
||||
typeof rawServerName === 'string' ? rawServerName : undefined;
|
||||
const qualifiedName =
|
||||
serverName && !toolName.includes('__')
|
||||
? `${serverName}__${toolName}`
|
||||
: toolName;
|
||||
if (!matchesWildcard(rule.toolName, qualifiedName, undefined)) {
|
||||
continue;
|
||||
}
|
||||
} else if (toolName !== rule.toolName) {
|
||||
continue;
|
||||
}
|
||||
}
|
||||
// Determine decision considering global verdict
|
||||
let decision: PolicyDecision;
|
||||
if (globalVerdict !== undefined) {
|
||||
decision = globalVerdict;
|
||||
} else {
|
||||
decision = rule.decision;
|
||||
}
|
||||
if (decision === PolicyDecision.DENY) {
|
||||
excludedTools.add(toolName);
|
||||
}
|
||||
processedTools.add(toolName);
|
||||
}
|
||||
continue;
|
||||
}
|
||||
|
||||
// Handle Global Rules
|
||||
if (!rule.toolName) {
|
||||
if (globalVerdict === undefined) {
|
||||
globalVerdict = rule.decision;
|
||||
if (globalVerdict !== PolicyDecision.DENY) {
|
||||
// Global ALLOW/ASK found.
|
||||
// Since rules are sorted by priority, this overrides any lower-priority rules.
|
||||
// We can stop processing because nothing else will be excluded.
|
||||
break;
|
||||
}
|
||||
// If Global DENY, we continue to find specific tools to add to excluded set
|
||||
}
|
||||
continue;
|
||||
}
|
||||
|
||||
const toolName = rule.toolName;
|
||||
|
||||
// Check if already processed (exact match)
|
||||
if (processedTools.has(toolName)) {
|
||||
continue;
|
||||
}
|
||||
|
||||
// Check if covered by a processed wildcard
|
||||
let coveredByWildcard = false;
|
||||
for (const processed of processedTools) {
|
||||
if (
|
||||
isWildcardPattern(processed) &&
|
||||
matchesWildcard(processed, toolName, undefined)
|
||||
) {
|
||||
// It's covered by a higher-priority wildcard rule.
|
||||
// If that wildcard rule resulted in exclusion, this tool should also be excluded.
|
||||
if (excludedTools.has(processed)) {
|
||||
excludedTools.add(toolName);
|
||||
}
|
||||
coveredByWildcard = true;
|
||||
break;
|
||||
}
|
||||
}
|
||||
if (coveredByWildcard) {
|
||||
continue;
|
||||
}
|
||||
|
||||
processedTools.add(toolName);
|
||||
|
||||
// Determine decision
|
||||
let decision: PolicyDecision;
|
||||
if (globalVerdict !== undefined) {
|
||||
decision = globalVerdict;
|
||||
} else {
|
||||
decision = rule.decision;
|
||||
}
|
||||
|
||||
if (decision === PolicyDecision.DENY) {
|
||||
excludedTools.add(toolName);
|
||||
}
|
||||
if (!allToolNames) {
|
||||
return excludedTools;
|
||||
}
|
||||
|
||||
// If there's a global DENY and we know all tool names, exclude any tool
|
||||
// that wasn't explicitly allowed by a higher-priority rule.
|
||||
if (globalVerdict === PolicyDecision.DENY && allToolNames) {
|
||||
for (const name of allToolNames) {
|
||||
if (!processedTools.has(name)) {
|
||||
excludedTools.add(name);
|
||||
for (const toolName of allToolNames) {
|
||||
const annotations = toolMetadata?.get(toolName);
|
||||
const serverName = isMcpToolAnnotation(annotations)
|
||||
? annotations._serverName
|
||||
: undefined;
|
||||
|
||||
let staticallyExcluded = false;
|
||||
let matchFound = false;
|
||||
|
||||
// Evaluate rules in priority order (they are already sorted in constructor)
|
||||
for (const rule of this.rules) {
|
||||
// Create a copy of the rule without argsPattern to see if it targets the tool
|
||||
// regardless of the runtime arguments it might receive.
|
||||
const ruleWithoutArgs: PolicyRule = { ...rule, argsPattern: undefined };
|
||||
const toolCall: FunctionCall = { name: toolName, args: {} };
|
||||
|
||||
const appliesToTool = ruleMatches(
|
||||
ruleWithoutArgs,
|
||||
toolCall,
|
||||
undefined, // stringifiedArgs
|
||||
serverName,
|
||||
this.approvalMode,
|
||||
annotations,
|
||||
);
|
||||
|
||||
if (appliesToTool) {
|
||||
if (rule.argsPattern) {
|
||||
// Exclusions only apply statically before arguments are known.
|
||||
if (rule.decision !== PolicyDecision.DENY) {
|
||||
// Conditionally allowed/asked based on args. Therefore NOT statically excluded.
|
||||
staticallyExcluded = false;
|
||||
matchFound = true;
|
||||
break;
|
||||
}
|
||||
// If it's conditionally DENIED based on args, it means it's not unconditionally denied.
|
||||
// We must keep evaluating lower priority rules to see the default/unconditional state.
|
||||
continue;
|
||||
} else {
|
||||
// Unconditional rule for this tool
|
||||
const decision = this.applyNonInteractiveMode(rule.decision);
|
||||
staticallyExcluded = decision === PolicyDecision.DENY;
|
||||
matchFound = true;
|
||||
break;
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
if (!matchFound) {
|
||||
// Fallback to default decision if no rule matches
|
||||
const defaultDec = this.applyNonInteractiveMode(this.defaultDecision);
|
||||
if (defaultDec === PolicyDecision.DENY) {
|
||||
staticallyExcluded = true;
|
||||
}
|
||||
}
|
||||
|
||||
if (staticallyExcluded) {
|
||||
excludedTools.add(toolName);
|
||||
}
|
||||
}
|
||||
|
||||
return excludedTools;
|
||||
|
||||
@@ -129,7 +129,7 @@ priority = 10
|
||||
`);
|
||||
|
||||
expect(result.rules).toHaveLength(1);
|
||||
expect(result.rules[0].toolName).toBe('*__*');
|
||||
expect(result.rules[0].toolName).toBe('mcp_*');
|
||||
expect(result.rules[0].decision).toBe(PolicyDecision.ASK_USER);
|
||||
expect(result.errors).toHaveLength(0);
|
||||
});
|
||||
@@ -144,7 +144,7 @@ priority = 10
|
||||
`);
|
||||
|
||||
expect(result.rules).toHaveLength(1);
|
||||
expect(result.rules[0].toolName).toBe('*__search');
|
||||
expect(result.rules[0].toolName).toBe('mcp_*_search');
|
||||
expect(result.errors).toHaveLength(0);
|
||||
});
|
||||
|
||||
@@ -215,8 +215,12 @@ priority = 100
|
||||
`);
|
||||
|
||||
expect(result.rules).toHaveLength(2);
|
||||
expect(result.rules[0].toolName).toBe('google-workspace__calendar.list');
|
||||
expect(result.rules[1].toolName).toBe('google-workspace__calendar.get');
|
||||
expect(result.rules[0].toolName).toBe(
|
||||
'mcp_google-workspace_calendar.list',
|
||||
);
|
||||
expect(result.rules[1].toolName).toBe(
|
||||
'mcp_google-workspace_calendar.get',
|
||||
);
|
||||
expect(result.errors).toHaveLength(0);
|
||||
});
|
||||
|
||||
@@ -678,12 +682,12 @@ priority = 100
|
||||
it('should not warn for MCP format tool names', async () => {
|
||||
const result = await runLoadPoliciesFromToml(`
|
||||
[[rule]]
|
||||
toolName = "my-server__my-tool"
|
||||
toolName = "mcp_my-server_my-tool"
|
||||
decision = "allow"
|
||||
priority = 100
|
||||
|
||||
[[rule]]
|
||||
toolName = "my-server__*"
|
||||
toolName = "mcp_my-server_*"
|
||||
decision = "allow"
|
||||
priority = 100
|
||||
`);
|
||||
@@ -822,7 +826,7 @@ priority = 100
|
||||
annotationRule,
|
||||
'Should have loaded a rule with toolAnnotations',
|
||||
).toBeDefined();
|
||||
expect(annotationRule!.toolName).toBe('*__*');
|
||||
expect(annotationRule!.toolName).toBe('mcp_*');
|
||||
expect(annotationRule!.toolAnnotations).toEqual({
|
||||
readOnlyHint: true,
|
||||
});
|
||||
@@ -863,7 +867,7 @@ priority = 100
|
||||
|
||||
// 4. MCP tool WITHOUT annotations should be DENIED
|
||||
const denyResult = await engine.check(
|
||||
{ name: 'github__create_issue' },
|
||||
{ name: 'mcp_github_create_issue' },
|
||||
'github',
|
||||
undefined,
|
||||
);
|
||||
@@ -874,7 +878,7 @@ priority = 100
|
||||
|
||||
// 5. MCP tool with readOnlyHint=false should also be DENIED
|
||||
const denyResult2 = await engine.check(
|
||||
{ name: 'github__delete_issue' },
|
||||
{ name: 'mcp_github_delete_issue' },
|
||||
'github',
|
||||
{ readOnlyHint: false },
|
||||
);
|
||||
@@ -883,9 +887,9 @@ priority = 100
|
||||
'MCP tool with readOnlyHint=false should be DENIED in Plan Mode',
|
||||
).toBe(PolicyDecision.DENY);
|
||||
|
||||
// 6. Test with qualified tool name format (server__tool) but no separate serverName
|
||||
// 6. Test with qualified tool name format (mcp_server_tool) but no separate serverName
|
||||
const qualifiedResult = await engine.check(
|
||||
{ name: 'github__list_repos' },
|
||||
{ name: 'mcp_github_list_repos' },
|
||||
undefined,
|
||||
{ readOnlyHint: true },
|
||||
);
|
||||
@@ -990,7 +994,8 @@ priority = 100
|
||||
['people.getMe', 'calendar.list', 'calendar.get'],
|
||||
[
|
||||
{
|
||||
toolName: 'google-workspace__people.getxMe',
|
||||
toolName: 'mcp_google-workspace_people.getxMe',
|
||||
mcpName: 'google-workspace',
|
||||
source: 'User: workspace.toml',
|
||||
},
|
||||
],
|
||||
@@ -1007,8 +1012,14 @@ priority = 100
|
||||
'google-workspace',
|
||||
['people.getMe', 'calendar.list'],
|
||||
[
|
||||
{ toolName: 'google-workspace__people.getMe' },
|
||||
{ toolName: 'google-workspace__calendar.list' },
|
||||
{
|
||||
toolName: 'mcp_google-workspace_people.getMe',
|
||||
mcpName: 'google-workspace',
|
||||
},
|
||||
{
|
||||
toolName: 'mcp_google-workspace_calendar.list',
|
||||
mcpName: 'google-workspace',
|
||||
},
|
||||
],
|
||||
);
|
||||
|
||||
@@ -1019,7 +1030,7 @@ priority = 100
|
||||
const warnings = validateMcpPolicyToolNames(
|
||||
'my-server',
|
||||
['tool1', 'tool2'],
|
||||
[{ toolName: 'my-server__*' }],
|
||||
[{ toolName: 'mcp_my-server_*', mcpName: 'my-server' }],
|
||||
);
|
||||
|
||||
expect(warnings).toHaveLength(0);
|
||||
@@ -1029,7 +1040,7 @@ priority = 100
|
||||
const warnings = validateMcpPolicyToolNames(
|
||||
'server-a',
|
||||
['tool1'],
|
||||
[{ toolName: 'server-b__toolx' }],
|
||||
[{ toolName: 'mcp_server-b_toolx', mcpName: 'server-b' }],
|
||||
);
|
||||
|
||||
expect(warnings).toHaveLength(0);
|
||||
@@ -1039,7 +1050,12 @@ priority = 100
|
||||
const warnings = validateMcpPolicyToolNames(
|
||||
'my-server',
|
||||
['tool1', 'tool2'],
|
||||
[{ toolName: 'my-server__completely_different_name' }],
|
||||
[
|
||||
{
|
||||
toolName: 'mcp_my-server_completely_different_name',
|
||||
mcpName: 'my-server',
|
||||
},
|
||||
],
|
||||
);
|
||||
|
||||
expect(warnings).toHaveLength(0);
|
||||
@@ -1059,7 +1075,13 @@ priority = 100
|
||||
const warnings = validateMcpPolicyToolNames(
|
||||
'my-server',
|
||||
['tool1'],
|
||||
[{ toolName: 'my-server__tol1', source: 'User: custom.toml' }],
|
||||
[
|
||||
{
|
||||
toolName: 'mcp_my-server_tol1',
|
||||
mcpName: 'my-server',
|
||||
source: 'User: custom.toml',
|
||||
},
|
||||
],
|
||||
);
|
||||
|
||||
expect(warnings).toHaveLength(1);
|
||||
|
||||
@@ -24,6 +24,7 @@ import path from 'node:path';
|
||||
import toml from '@iarna/toml';
|
||||
import { z, type ZodError } from 'zod';
|
||||
import { isNodeError } from '../utils/errors.js';
|
||||
import { MCP_TOOL_PREFIX, formatMcpToolName } from '../tools/mcp-tool.js';
|
||||
|
||||
/**
|
||||
* Maximum Levenshtein distance to consider a name a likely typo of a built-in tool.
|
||||
@@ -262,11 +263,15 @@ function validateShellCommandSyntax(
|
||||
* tool name, or null if valid or not close to any built-in name.
|
||||
*/
|
||||
function validateToolName(name: string, ruleIndex: number): string | null {
|
||||
if (name.includes('__')) {
|
||||
return `Rule #${ruleIndex + 1}: The "__" syntax for MCP tools is strictly deprecated. Please use the 'mcpName = "..."' property or the 'mcp_server_tool' format instead.`;
|
||||
}
|
||||
|
||||
// A name that looks like an MCP tool (e.g., "re__ad") could be a typo of a
|
||||
// built-in tool ("read_file"). We should let such names fall through to the
|
||||
// Levenshtein distance check below. Non-MCP-like names that are valid can
|
||||
// be safely skipped.
|
||||
if (isValidToolName(name, { allowWildcards: true }) && !name.includes('__')) {
|
||||
if (isValidToolName(name, { allowWildcards: true })) {
|
||||
return null;
|
||||
}
|
||||
|
||||
@@ -402,8 +407,8 @@ export async function loadPoliciesFromToml(
|
||||
// Validate tool names in rules
|
||||
for (let i = 0; i < tomlRules.length; i++) {
|
||||
const rule = tomlRules[i];
|
||||
// Skip MCP-scoped rules — MCP tool names are server-defined and dynamic
|
||||
if (rule.mcpName) continue;
|
||||
// We no longer skip MCP-scoped rules because we need to specifically
|
||||
// warn users if they use deprecated "__" syntax for MCP tool names
|
||||
|
||||
const toolNames: string[] = rule.toolName
|
||||
? Array.isArray(rule.toolName)
|
||||
@@ -447,18 +452,19 @@ export async function loadPoliciesFromToml(
|
||||
|
||||
// Create a policy rule for each tool name
|
||||
return toolNames.map((toolName) => {
|
||||
// Transform mcpName field to composite toolName format
|
||||
let effectiveToolName: string | undefined;
|
||||
if (rule.mcpName && toolName) {
|
||||
effectiveToolName = `${rule.mcpName}__${toolName}`;
|
||||
} else if (rule.mcpName) {
|
||||
effectiveToolName = `${rule.mcpName}__*`;
|
||||
} else {
|
||||
effectiveToolName = toolName;
|
||||
let effectiveToolName: string | undefined = toolName;
|
||||
const mcpName = rule.mcpName;
|
||||
|
||||
if (mcpName) {
|
||||
effectiveToolName = formatMcpToolName(
|
||||
mcpName,
|
||||
effectiveToolName,
|
||||
);
|
||||
}
|
||||
|
||||
const policyRule: PolicyRule = {
|
||||
toolName: effectiveToolName,
|
||||
mcpName: rule.mcpName,
|
||||
decision: rule.decision,
|
||||
priority: transformPriority(rule.priority, tier),
|
||||
modes: rule.modes,
|
||||
@@ -563,15 +569,16 @@ export async function loadPoliciesFromToml(
|
||||
return toolNames.map((toolName) => {
|
||||
let effectiveToolName: string | undefined;
|
||||
if (checker.mcpName && toolName) {
|
||||
effectiveToolName = `${checker.mcpName}__${toolName}`;
|
||||
effectiveToolName = `${MCP_TOOL_PREFIX}${checker.mcpName}_${toolName}`;
|
||||
} else if (checker.mcpName) {
|
||||
effectiveToolName = `${checker.mcpName}__*`;
|
||||
effectiveToolName = `${MCP_TOOL_PREFIX}${checker.mcpName}_*`;
|
||||
} else {
|
||||
effectiveToolName = toolName;
|
||||
}
|
||||
|
||||
const safetyCheckerRule: SafetyCheckerRule = {
|
||||
toolName: effectiveToolName,
|
||||
mcpName: checker.mcpName,
|
||||
priority: transformPriority(checker.priority, tier),
|
||||
// eslint-disable-next-line @typescript-eslint/no-unsafe-type-assertion
|
||||
checker: checker.checker as SafetyCheckerConfig,
|
||||
@@ -655,16 +662,28 @@ export async function loadPoliciesFromToml(
|
||||
export function validateMcpPolicyToolNames(
|
||||
serverName: string,
|
||||
discoveredToolNames: string[],
|
||||
policyRules: ReadonlyArray<{ toolName?: string; source?: string }>,
|
||||
policyRules: ReadonlyArray<{
|
||||
toolName?: string;
|
||||
mcpName?: string;
|
||||
source?: string;
|
||||
}>,
|
||||
): string[] {
|
||||
const prefix = `${serverName}__`;
|
||||
const prefix = `${MCP_TOOL_PREFIX}${serverName}_`;
|
||||
const warnings: string[] = [];
|
||||
|
||||
for (const rule of policyRules) {
|
||||
if (!rule.toolName) continue;
|
||||
if (!rule.toolName.startsWith(prefix)) continue;
|
||||
|
||||
const toolPart = rule.toolName.slice(prefix.length);
|
||||
let toolPart: string | undefined;
|
||||
|
||||
// The toolName is typically transformed into an FQN if mcpName was used.
|
||||
if (rule.mcpName === serverName && rule.toolName.startsWith(prefix)) {
|
||||
toolPart = rule.toolName.slice(prefix.length);
|
||||
} else if (rule.toolName.startsWith(prefix)) {
|
||||
toolPart = rule.toolName.slice(prefix.length);
|
||||
} else {
|
||||
continue;
|
||||
}
|
||||
|
||||
// Skip wildcards
|
||||
if (toolPart === '*') continue;
|
||||
|
||||
@@ -110,6 +110,13 @@ export interface PolicyRule {
|
||||
*/
|
||||
toolName?: string;
|
||||
|
||||
/**
|
||||
* Identifies the MCP server this rule applies to.
|
||||
* Enables precise rule matching against `serverName` metadata instead
|
||||
* of parsing composite string names.
|
||||
*/
|
||||
mcpName?: string;
|
||||
|
||||
/**
|
||||
* Pattern to match against tool arguments.
|
||||
* Can be used for more fine-grained control.
|
||||
@@ -166,6 +173,11 @@ export interface SafetyCheckerRule {
|
||||
*/
|
||||
toolName?: string;
|
||||
|
||||
/**
|
||||
* Identifies the MCP server this rule applies to.
|
||||
*/
|
||||
mcpName?: string;
|
||||
|
||||
/**
|
||||
* Pattern to match against tool arguments.
|
||||
* Can be used for more fine-grained control.
|
||||
|
||||
@@ -153,7 +153,7 @@ describe('PromptProvider', () => {
|
||||
const provider = new PromptProvider();
|
||||
const prompt = provider.getCoreSystemPrompt(mockConfig);
|
||||
|
||||
expect(prompt).toContain('`mcp_read` (my-mcp-server)');
|
||||
expect(prompt).toContain('`mcp_my-mcp-server_mcp_read` (my-mcp-server)');
|
||||
});
|
||||
|
||||
it('should include write constraint message in plan mode prompt', () => {
|
||||
|
||||
@@ -8,9 +8,12 @@
|
||||
import type { Mocked } from 'vitest';
|
||||
import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest';
|
||||
import { safeJsonStringify } from '../utils/safeJsonStringify.js';
|
||||
import { DiscoveredMCPTool, generateValidName } from './mcp-tool.js'; // Added getStringifiedResultForDisplay
|
||||
import type { ToolResult } from './tools.js';
|
||||
import { ToolConfirmationOutcome } from './tools.js'; // Added ToolConfirmationOutcome
|
||||
import {
|
||||
DiscoveredMCPTool,
|
||||
generateValidName,
|
||||
formatMcpToolName,
|
||||
} from './mcp-tool.js'; // Added getStringifiedResultForDisplay
|
||||
import { ToolConfirmationOutcome, type ToolResult } from './tools.js';
|
||||
import type { CallableTool, Part } from '@google/genai';
|
||||
import { ToolErrorType } from './tool-error.js';
|
||||
import {
|
||||
@@ -43,23 +46,23 @@ const createSdkResponse = (
|
||||
|
||||
describe('generateValidName', () => {
|
||||
it('should return a valid name for a simple function', () => {
|
||||
expect(generateValidName('myFunction')).toBe('myFunction');
|
||||
expect(generateValidName('myFunction')).toBe('mcp_myFunction');
|
||||
});
|
||||
|
||||
it('should replace invalid characters with underscores', () => {
|
||||
expect(generateValidName('invalid-name with spaces')).toBe(
|
||||
'invalid-name_with_spaces',
|
||||
'mcp_invalid-name_with_spaces',
|
||||
);
|
||||
});
|
||||
|
||||
it('should truncate long names', () => {
|
||||
expect(generateValidName('x'.repeat(80))).toBe(
|
||||
'xxxxxxxxxxxxxxxxxxxxxxxxxxxxxx...xxxxxxxxxxxxxxxxxxxxxxxxxxxxxx',
|
||||
'mcp_xxxxxxxxxxxxxxxxxxxxxxxxxx...xxxxxxxxxxxxxxxxxxxxxxxxxxxxxx',
|
||||
);
|
||||
});
|
||||
|
||||
it('should handle names with only invalid characters', () => {
|
||||
expect(generateValidName('!@#$%^&*()')).toBe('__________');
|
||||
expect(generateValidName('!@#$%^&*()')).toBe('mcp___________');
|
||||
});
|
||||
|
||||
it.each([
|
||||
@@ -74,6 +77,30 @@ describe('generateValidName', () => {
|
||||
);
|
||||
});
|
||||
|
||||
describe('formatMcpToolName', () => {
|
||||
it('should format a fully qualified name', () => {
|
||||
expect(formatMcpToolName('github', 'list_repos')).toBe(
|
||||
'mcp_github_list_repos',
|
||||
);
|
||||
});
|
||||
|
||||
it('should handle global wildcards', () => {
|
||||
expect(formatMcpToolName('*')).toBe('mcp_*');
|
||||
});
|
||||
|
||||
it('should handle tool-level wildcards', () => {
|
||||
expect(formatMcpToolName('github', '*')).toBe('mcp_github_*');
|
||||
});
|
||||
|
||||
it('should handle undefined toolName as a tool-level wildcard', () => {
|
||||
expect(formatMcpToolName('github')).toBe('mcp_github_*');
|
||||
});
|
||||
|
||||
it('should format explicitly global wildcard with specific tool', () => {
|
||||
expect(formatMcpToolName('*', 'list_repos')).toBe('mcp_*_list_repos');
|
||||
});
|
||||
});
|
||||
|
||||
describe('DiscoveredMCPTool', () => {
|
||||
const serverName = 'mock-mcp-server';
|
||||
const serverToolName = 'actual-server-tool-name';
|
||||
@@ -110,8 +137,10 @@ describe('DiscoveredMCPTool', () => {
|
||||
|
||||
describe('constructor', () => {
|
||||
it('should set properties correctly', () => {
|
||||
expect(tool.name).toBe(serverToolName);
|
||||
expect(tool.schema.name).toBe(serverToolName);
|
||||
expect(tool.name).toBe('mcp_mock-mcp-server_actual-server-tool-name');
|
||||
expect(tool.schema.name).toBe(
|
||||
'mcp_mock-mcp-server_actual-server-tool-name',
|
||||
);
|
||||
expect(tool.schema.description).toBe(baseDescription);
|
||||
expect(tool.schema.parameters).toBeUndefined();
|
||||
expect(tool.schema.parametersJsonSchema).toEqual(inputSchema);
|
||||
@@ -937,31 +966,35 @@ describe('DiscoveredMCPTool', () => {
|
||||
describe('MCP Tool Naming Regression Fixes', () => {
|
||||
describe('generateValidName', () => {
|
||||
it('should replace spaces with underscores', () => {
|
||||
expect(generateValidName('My Tool')).toBe('My_Tool');
|
||||
expect(generateValidName('My Tool')).toBe('mcp_My_Tool');
|
||||
});
|
||||
|
||||
it('should allow colons', () => {
|
||||
expect(generateValidName('namespace:tool')).toBe('namespace:tool');
|
||||
expect(generateValidName('namespace:tool')).toBe('mcp_namespace:tool');
|
||||
});
|
||||
|
||||
it('should ensure name starts with a letter or underscore', () => {
|
||||
expect(generateValidName('123-tool')).toBe('_123-tool');
|
||||
expect(generateValidName('-tool')).toBe('_-tool');
|
||||
expect(generateValidName('.tool')).toBe('_.tool');
|
||||
expect(generateValidName('valid_tool_name')).toBe('mcp_valid_tool_name');
|
||||
expect(generateValidName('alsoValid-123.name')).toBe(
|
||||
'mcp_alsoValid-123.name',
|
||||
);
|
||||
expect(generateValidName('another:valid:name')).toBe(
|
||||
'mcp_another:valid:name',
|
||||
);
|
||||
});
|
||||
|
||||
it('should handle very long names by truncating in the middle', () => {
|
||||
const longName = 'a'.repeat(40) + '__' + 'b'.repeat(40);
|
||||
const result = generateValidName(longName);
|
||||
expect(result.length).toBeLessThanOrEqual(63);
|
||||
expect(result).toMatch(/^a{30}\.\.\.b{30}$/);
|
||||
expect(result).toMatch(/^mcp_a{26}\.\.\.b{30}$/);
|
||||
});
|
||||
|
||||
it('should handle very long names starting with a digit', () => {
|
||||
const longName = '1' + 'a'.repeat(80);
|
||||
const result = generateValidName(longName);
|
||||
expect(result.length).toBeLessThanOrEqual(63);
|
||||
expect(result.startsWith('_1')).toBe(true);
|
||||
expect(result.startsWith('mcp_1')).toBe(true);
|
||||
});
|
||||
});
|
||||
|
||||
@@ -977,7 +1010,7 @@ describe('MCP Tool Naming Regression Fixes', () => {
|
||||
);
|
||||
|
||||
const qn = tool.getFullyQualifiedName();
|
||||
expect(qn).toBe('My_Server__my-tool');
|
||||
expect(qn).toBe('mcp_My_Server_my-tool');
|
||||
});
|
||||
|
||||
it('should handle long server and tool names in qualified name', () => {
|
||||
@@ -1008,7 +1041,7 @@ describe('MCP Tool Naming Regression Fixes', () => {
|
||||
);
|
||||
|
||||
const qn = tool.getFullyQualifiedName();
|
||||
expect(qn).toBe('_123-server__tool');
|
||||
expect(qn).toBe('mcp_123-server_tool');
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
@@ -5,6 +5,7 @@
|
||||
*/
|
||||
|
||||
import { safeJsonStringify } from '../utils/safeJsonStringify.js';
|
||||
import { debugLogger } from '../utils/debugLogger.js';
|
||||
import type {
|
||||
ToolCallConfirmationDetails,
|
||||
ToolInvocation,
|
||||
@@ -25,18 +26,92 @@ import type { McpContext } from './mcp-client.js';
|
||||
|
||||
/**
|
||||
* The separator used to qualify MCP tool names with their server prefix.
|
||||
* e.g. "server_name__tool_name"
|
||||
* e.g. "mcp_server_name_tool_name"
|
||||
*/
|
||||
export const MCP_QUALIFIED_NAME_SEPARATOR = '__';
|
||||
export const MCP_QUALIFIED_NAME_SEPARATOR = '_';
|
||||
|
||||
/**
|
||||
* Returns true if `name` matches the MCP qualified name format: "server__tool",
|
||||
* i.e. exactly two non-empty parts separated by the MCP_QUALIFIED_NAME_SEPARATOR.
|
||||
* The strict prefix that all MCP tools must start with.
|
||||
*/
|
||||
export const MCP_TOOL_PREFIX = 'mcp_';
|
||||
|
||||
/**
|
||||
* Returns true if `name` matches the MCP qualified name format: "mcp_server_tool",
|
||||
* i.e. starts with the "mcp_" prefix.
|
||||
*/
|
||||
export function isMcpToolName(name: string): boolean {
|
||||
if (!name.includes(MCP_QUALIFIED_NAME_SEPARATOR)) return false;
|
||||
const parts = name.split(MCP_QUALIFIED_NAME_SEPARATOR);
|
||||
return parts.length === 2 && parts[0].length > 0 && parts[1].length > 0;
|
||||
return name.startsWith(MCP_TOOL_PREFIX);
|
||||
}
|
||||
|
||||
/**
|
||||
* Extracts the server name and tool name from a fully qualified MCP tool name.
|
||||
* Expected format: `mcp_{server_name}_{tool_name}`
|
||||
* @param name The fully qualified tool name.
|
||||
* @returns An object containing the extracted `serverName` and `toolName`, or
|
||||
* `undefined` properties if the name doesn't match the expected format.
|
||||
*/
|
||||
export function parseMcpToolName(name: string): {
|
||||
serverName?: string;
|
||||
toolName?: string;
|
||||
} {
|
||||
if (!isMcpToolName(name)) {
|
||||
return {};
|
||||
}
|
||||
// Remove the prefix
|
||||
const withoutPrefix = name.slice(MCP_TOOL_PREFIX.length);
|
||||
// The first segment is the server name, the rest is the tool name
|
||||
const match = withoutPrefix.match(/^([^_]+)_(.+)$/);
|
||||
if (match) {
|
||||
return {
|
||||
serverName: match[1],
|
||||
toolName: match[2],
|
||||
};
|
||||
}
|
||||
return {};
|
||||
}
|
||||
|
||||
/**
|
||||
* Assembles a fully qualified MCP tool name (or wildcard pattern) from its server and tool components.
|
||||
*
|
||||
* @param serverName The backend MCP server name (can be '*' for global wildcards).
|
||||
* @param toolName The name of the tool (can be undefined or '*' for tool-level wildcards).
|
||||
* @returns The fully qualified name (e.g., `mcp_server_tool`, `mcp_*`, `mcp_server_*`).
|
||||
*/
|
||||
export function formatMcpToolName(
|
||||
serverName: string,
|
||||
toolName?: string,
|
||||
): string {
|
||||
if (serverName === '*' && !toolName) {
|
||||
return `${MCP_TOOL_PREFIX}*`;
|
||||
} else if (serverName === '*') {
|
||||
return `${MCP_TOOL_PREFIX}*_${toolName}`;
|
||||
} else if (!toolName) {
|
||||
return `${MCP_TOOL_PREFIX}${serverName}_*`;
|
||||
} else {
|
||||
return `${MCP_TOOL_PREFIX}${serverName}_${toolName}`;
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Interface representing metadata annotations specific to an MCP tool.
|
||||
* Ensures strongly-typed access to server-level properties.
|
||||
*/
|
||||
export interface McpToolAnnotation extends Record<string, unknown> {
|
||||
_serverName: string;
|
||||
}
|
||||
|
||||
/**
|
||||
* Type guard to check if tool annotations implement McpToolAnnotation.
|
||||
*/
|
||||
export function isMcpToolAnnotation(
|
||||
annotation: unknown,
|
||||
): annotation is McpToolAnnotation {
|
||||
return (
|
||||
typeof annotation === 'object' &&
|
||||
annotation !== null &&
|
||||
// eslint-disable-next-line @typescript-eslint/no-unsafe-type-assertion
|
||||
typeof (annotation as Record<string, unknown>)['_serverName'] === 'string'
|
||||
);
|
||||
}
|
||||
|
||||
type ToolParams = Record<string, unknown>;
|
||||
@@ -276,7 +351,10 @@ export class DiscoveredMCPTool extends BaseDeclarativeTool<
|
||||
private readonly _toolAnnotations?: Record<string, unknown>,
|
||||
) {
|
||||
super(
|
||||
generateValidName(nameOverride ?? serverToolName),
|
||||
nameOverride ??
|
||||
generateValidName(
|
||||
`${serverName}${MCP_QUALIFIED_NAME_SEPARATOR}${serverToolName}`,
|
||||
),
|
||||
`${serverToolName} (${serverName} MCP Server)`,
|
||||
description,
|
||||
Kind.Other,
|
||||
@@ -304,7 +382,9 @@ export class DiscoveredMCPTool extends BaseDeclarativeTool<
|
||||
}
|
||||
|
||||
getFullyQualifiedPrefix(): string {
|
||||
return `${this.serverName}${MCP_QUALIFIED_NAME_SEPARATOR}`;
|
||||
return generateValidName(
|
||||
`${this.serverName}${MCP_QUALIFIED_NAME_SEPARATOR}`,
|
||||
);
|
||||
}
|
||||
|
||||
getFullyQualifiedName(): string {
|
||||
@@ -495,8 +575,12 @@ const MAX_FUNCTION_NAME_LENGTH = 64;
|
||||
|
||||
/** Visible for testing */
|
||||
export function generateValidName(name: string) {
|
||||
// Replace invalid characters (based on 400 error message from Gemini API) with underscores
|
||||
let validToolname = name.replace(/[^a-zA-Z0-9_.:-]/g, '_');
|
||||
// Enforce the mcp_ prefix for all generated MCP tool names
|
||||
let validToolname = name.startsWith('mcp_') ? name : `mcp_${name}`;
|
||||
|
||||
// Replace invalid characters with underscores to conform to Gemini API:
|
||||
// ^[a-zA-Z_][a-zA-Z0-9_\-.:]{0,63}$
|
||||
validToolname = validToolname.replace(/[^a-zA-Z0-9_\-.:]/g, '_');
|
||||
|
||||
// Ensure it starts with a letter or underscore
|
||||
if (/^[^a-zA-Z_]/.test(validToolname)) {
|
||||
@@ -507,6 +591,9 @@ export function generateValidName(name: string) {
|
||||
// Note: We use 63 instead of 64 to be safe, as some environments have off-by-one behaviors.
|
||||
const safeLimit = MAX_FUNCTION_NAME_LENGTH - 1;
|
||||
if (validToolname.length > safeLimit) {
|
||||
debugLogger.warn(
|
||||
`Truncating MCP tool name "${validToolname}" to fit within the 64 character limit. This tool may require user approval.`,
|
||||
);
|
||||
validToolname =
|
||||
validToolname.slice(0, 30) + '...' + validToolname.slice(-30);
|
||||
}
|
||||
|
||||
@@ -13,9 +13,12 @@ import { ApprovalMode } from '../policy/types.js';
|
||||
|
||||
import { ToolRegistry, DiscoveredTool } from './tool-registry.js';
|
||||
import { DISCOVERED_TOOL_PREFIX } from './tool-names.js';
|
||||
import { DiscoveredMCPTool, MCP_QUALIFIED_NAME_SEPARATOR } from './mcp-tool.js';
|
||||
import type { FunctionDeclaration, CallableTool } from '@google/genai';
|
||||
import { mcpToTool } from '@google/genai';
|
||||
import { DiscoveredMCPTool } from './mcp-tool.js';
|
||||
import {
|
||||
mcpToTool,
|
||||
type FunctionDeclaration,
|
||||
type CallableTool,
|
||||
} from '@google/genai';
|
||||
import { spawn } from 'node:child_process';
|
||||
|
||||
import fs from 'node:fs';
|
||||
@@ -307,7 +310,7 @@ describe('ToolRegistry', () => {
|
||||
{
|
||||
name: 'should match qualified MCP tool names when qualified or unqualified',
|
||||
tools: [mcpTool, mcpTool.asFullyQualifiedTool()],
|
||||
excludedTools: [`${mcpTool.getFullyQualifiedPrefix()}${mcpTool.name}`],
|
||||
excludedTools: [mcpTool.name],
|
||||
},
|
||||
{
|
||||
name: 'should match class names',
|
||||
@@ -395,7 +398,7 @@ describe('ToolRegistry', () => {
|
||||
// Assert that the returned array contains all tool names, with MCP qualified
|
||||
expect(toolNames).toContain('c-tool');
|
||||
expect(toolNames).toContain('a-tool');
|
||||
expect(toolNames).toContain('my-server__my-tool');
|
||||
expect(toolNames).toContain('mcp_my-server_my-tool');
|
||||
expect(toolNames).toHaveLength(3);
|
||||
});
|
||||
|
||||
@@ -409,7 +412,7 @@ describe('ToolRegistry', () => {
|
||||
toolRegistry.registerTool(mcpTool.asFullyQualifiedTool());
|
||||
|
||||
const toolNames = toolRegistry.getAllToolNames();
|
||||
expect(toolNames).toEqual([`${serverName}__${toolName}`]);
|
||||
expect(toolNames).toEqual([`mcp_${serverName}_${toolName}`]);
|
||||
});
|
||||
});
|
||||
|
||||
@@ -439,7 +442,11 @@ describe('ToolRegistry', () => {
|
||||
|
||||
// Assert that the array has the correct tools and is sorted by name
|
||||
expect(toolsFromServer1).toHaveLength(3);
|
||||
expect(toolNames).toEqual(['apple-tool', 'banana-tool', 'zebra-tool']);
|
||||
expect(toolNames).toEqual([
|
||||
'mcp_mcp-server-uno_apple-tool',
|
||||
'mcp_mcp-server-uno_banana-tool',
|
||||
'mcp_mcp-server-uno_zebra-tool',
|
||||
]);
|
||||
|
||||
// Assert that all returned tools are indeed from the correct server
|
||||
for (const tool of toolsFromServer1) {
|
||||
@@ -481,8 +488,8 @@ describe('ToolRegistry', () => {
|
||||
'builtin-1',
|
||||
'builtin-2',
|
||||
DISCOVERED_TOOL_PREFIX + 'discovered-1',
|
||||
'apple-server__mcp-apple',
|
||||
'zebra-server__mcp-zebra',
|
||||
'mcp_apple-server_mcp-apple',
|
||||
'mcp_zebra-server_mcp-zebra',
|
||||
]);
|
||||
});
|
||||
});
|
||||
@@ -598,25 +605,20 @@ describe('ToolRegistry', () => {
|
||||
});
|
||||
|
||||
describe('getTool', () => {
|
||||
it('should retrieve an MCP tool by its fully qualified name even if registered with simple name', () => {
|
||||
it('should retrieve an MCP tool by its fully qualified name', () => {
|
||||
const serverName = 'my-server';
|
||||
const toolName = 'my-tool';
|
||||
const mcpTool = createMCPTool(serverName, toolName, 'description');
|
||||
|
||||
// Register tool (will be registered as 'my-tool' since no conflict)
|
||||
// Register tool
|
||||
toolRegistry.registerTool(mcpTool);
|
||||
|
||||
// Verify it is available as 'my-tool'
|
||||
expect(toolRegistry.getTool('my-tool')).toBeDefined();
|
||||
expect(toolRegistry.getTool('my-tool')?.name).toBe('my-tool');
|
||||
|
||||
// Verify it is available as 'my-server__my-tool'
|
||||
const fullyQualifiedName = `${serverName}__${toolName}`;
|
||||
// Verify it is available as 'mcp_my-server_my-tool'
|
||||
const fullyQualifiedName = `mcp_${serverName}_${toolName}`;
|
||||
const retrievedTool = toolRegistry.getTool(fullyQualifiedName);
|
||||
|
||||
expect(retrievedTool).toBeDefined();
|
||||
// The returned tool object is the same, so its name property is still 'my-tool'
|
||||
expect(retrievedTool?.name).toBe('my-tool');
|
||||
expect(retrievedTool?.name).toBe(fullyQualifiedName);
|
||||
});
|
||||
|
||||
it('should retrieve an MCP tool by its fully qualified name when tool name has special characters', () => {
|
||||
@@ -626,19 +628,15 @@ describe('ToolRegistry', () => {
|
||||
const validToolName = 'my_tool';
|
||||
const mcpTool = createMCPTool(serverName, toolName, 'description');
|
||||
|
||||
// Register tool (will be registered as sanitized name)
|
||||
// Register tool
|
||||
toolRegistry.registerTool(mcpTool);
|
||||
|
||||
// Verify it is available as sanitized name
|
||||
expect(toolRegistry.getTool(validToolName)).toBeDefined();
|
||||
expect(toolRegistry.getTool(validToolName)?.name).toBe(validToolName);
|
||||
|
||||
// Verify it is available as 'my-server__my_tool'
|
||||
const fullyQualifiedName = `${serverName}__${validToolName}`;
|
||||
// Verify it is available as 'mcp_my-server_my_tool'
|
||||
const fullyQualifiedName = `mcp_${serverName}_${validToolName}`;
|
||||
const retrievedTool = toolRegistry.getTool(fullyQualifiedName);
|
||||
|
||||
expect(retrievedTool).toBeDefined();
|
||||
expect(retrievedTool?.name).toBe(validToolName);
|
||||
expect(retrievedTool?.name).toBe(fullyQualifiedName);
|
||||
});
|
||||
|
||||
it('should resolve qualified names in getFunctionDeclarationsFiltered', () => {
|
||||
@@ -648,13 +646,13 @@ describe('ToolRegistry', () => {
|
||||
|
||||
toolRegistry.registerTool(mcpTool);
|
||||
|
||||
const fullyQualifiedName = `${serverName}${MCP_QUALIFIED_NAME_SEPARATOR}${toolName}`;
|
||||
const fullyQualifiedName = `mcp_${serverName}_${toolName}`;
|
||||
const declarations = toolRegistry.getFunctionDeclarationsFiltered([
|
||||
fullyQualifiedName,
|
||||
]);
|
||||
|
||||
expect(declarations).toHaveLength(1);
|
||||
expect(declarations[0].name).toBe(toolName);
|
||||
expect(declarations[0].name).toBe(fullyQualifiedName);
|
||||
});
|
||||
|
||||
it('should retrieve a tool using its legacy alias', async () => {
|
||||
@@ -685,7 +683,7 @@ describe('ToolRegistry', () => {
|
||||
|
||||
const declarations = toolRegistry.getFunctionDeclarations();
|
||||
expect(declarations).toHaveLength(1);
|
||||
expect(declarations[0].name).toBe(`${serverName}__${toolName}`);
|
||||
expect(declarations[0].name).toBe(`mcp_${serverName}_${toolName}`);
|
||||
});
|
||||
|
||||
it('should deduplicate MCP tools in declarations', () => {
|
||||
@@ -699,7 +697,7 @@ describe('ToolRegistry', () => {
|
||||
|
||||
const declarations = toolRegistry.getFunctionDeclarations();
|
||||
expect(declarations).toHaveLength(1);
|
||||
expect(declarations[0].name).toBe(`${serverName}__${toolName}`);
|
||||
expect(declarations[0].name).toBe(`mcp_${serverName}_${toolName}`);
|
||||
});
|
||||
});
|
||||
|
||||
@@ -752,7 +750,7 @@ describe('ToolRegistry', () => {
|
||||
|
||||
const allTools = toolRegistry.getAllTools();
|
||||
const toolNames = allTools.map((t) => t.name);
|
||||
expect(toolNames).toContain('read-only-tool');
|
||||
expect(toolNames).toContain('mcp_test-server_read-only-tool');
|
||||
});
|
||||
|
||||
it('should exclude non-read-only MCP tools when denied by policy in plan mode', () => {
|
||||
@@ -769,7 +767,7 @@ describe('ToolRegistry', () => {
|
||||
|
||||
const allTools = toolRegistry.getAllTools();
|
||||
const toolNames = allTools.map((t) => t.name);
|
||||
expect(toolNames).not.toContain('write-mcp-tool');
|
||||
expect(toolNames).not.toContain('mcp_test-server_write-mcp-tool');
|
||||
});
|
||||
});
|
||||
|
||||
|
||||
@@ -443,13 +443,15 @@ export class ToolRegistry {
|
||||
private buildToolMetadata(): Map<string, Record<string, unknown>> {
|
||||
const toolMetadata = new Map<string, Record<string, unknown>>();
|
||||
for (const [name, tool] of this.allKnownTools) {
|
||||
if (tool.toolAnnotations) {
|
||||
const metadata: Record<string, unknown> = { ...tool.toolAnnotations };
|
||||
// Include server name so the policy engine can resolve composite
|
||||
// wildcard patterns (e.g. "*__*") against unqualified tool names.
|
||||
if (tool instanceof DiscoveredMCPTool) {
|
||||
metadata['_serverName'] = tool.serverName;
|
||||
}
|
||||
const metadata: Record<string, unknown> = tool.toolAnnotations
|
||||
? { ...tool.toolAnnotations }
|
||||
: {};
|
||||
// Include server name so the policy engine can resolve composite
|
||||
// wildcard patterns (e.g. "*__*") against unqualified tool names.
|
||||
if (tool instanceof DiscoveredMCPTool) {
|
||||
metadata['_serverName'] = tool.serverName;
|
||||
}
|
||||
if (Object.keys(metadata).length > 0) {
|
||||
toolMetadata.set(name, metadata);
|
||||
}
|
||||
}
|
||||
|
||||
@@ -484,6 +484,10 @@ describe('shortenPath', () => {
|
||||
});
|
||||
|
||||
describe('resolveToRealPath', () => {
|
||||
afterEach(() => {
|
||||
vi.restoreAllMocks();
|
||||
});
|
||||
|
||||
it.each([
|
||||
{
|
||||
description:
|
||||
@@ -510,9 +514,11 @@ describe('resolveToRealPath', () => {
|
||||
expect(resolveToRealPath(input)).toBe(expected);
|
||||
});
|
||||
|
||||
it('should return decoded path even if fs.realpathSync fails', () => {
|
||||
it('should return decoded path even if fs.realpathSync fails with ENOENT', () => {
|
||||
vi.spyOn(fs, 'realpathSync').mockImplementationOnce(() => {
|
||||
throw new Error('File not found');
|
||||
const err = new Error('File not found') as NodeJS.ErrnoException;
|
||||
err.code = 'ENOENT';
|
||||
throw err;
|
||||
});
|
||||
|
||||
const p = path.resolve('path', 'to', 'New Project');
|
||||
@@ -521,6 +527,47 @@ describe('resolveToRealPath', () => {
|
||||
|
||||
expect(resolveToRealPath(input)).toBe(expected);
|
||||
});
|
||||
|
||||
it('should recursively resolve symlinks for non-existent child paths', () => {
|
||||
const parentPath = path.resolve('/some/parent/path');
|
||||
const resolvedParentPath = path.resolve('/resolved/parent/path');
|
||||
const childPath = path.resolve(parentPath, 'child', 'file.txt');
|
||||
const expectedPath = path.resolve(resolvedParentPath, 'child', 'file.txt');
|
||||
|
||||
vi.spyOn(fs, 'realpathSync').mockImplementation((p) => {
|
||||
const pStr = p.toString();
|
||||
if (pStr === parentPath) {
|
||||
return resolvedParentPath;
|
||||
}
|
||||
const err = new Error('ENOENT') as NodeJS.ErrnoException;
|
||||
err.code = 'ENOENT';
|
||||
throw err;
|
||||
});
|
||||
|
||||
expect(resolveToRealPath(childPath)).toBe(expectedPath);
|
||||
});
|
||||
|
||||
it('should prevent infinite recursion on malicious symlink structures', () => {
|
||||
const maliciousPath = path.resolve('malicious', 'symlink');
|
||||
|
||||
vi.spyOn(fs, 'realpathSync').mockImplementation(() => {
|
||||
const err = new Error('ENOENT') as NodeJS.ErrnoException;
|
||||
err.code = 'ENOENT';
|
||||
throw err;
|
||||
});
|
||||
|
||||
vi.spyOn(fs, 'lstatSync').mockImplementation(
|
||||
() => ({ isSymbolicLink: () => true }) as fs.Stats,
|
||||
);
|
||||
|
||||
vi.spyOn(fs, 'readlinkSync').mockImplementation(() =>
|
||||
['..', 'malicious', 'symlink'].join(path.sep),
|
||||
);
|
||||
|
||||
expect(() => resolveToRealPath(maliciousPath)).toThrow(
|
||||
/Infinite recursion detected/,
|
||||
);
|
||||
});
|
||||
});
|
||||
|
||||
describe('normalizePath', () => {
|
||||
|
||||
@@ -359,8 +359,8 @@ export function isSubpath(parentPath: string, childPath: string): boolean {
|
||||
* @param pathStr The path string to resolve.
|
||||
* @returns The resolved real path.
|
||||
*/
|
||||
export function resolveToRealPath(path: string): string {
|
||||
let resolvedPath = path;
|
||||
export function resolveToRealPath(pathStr: string): string {
|
||||
let resolvedPath = pathStr;
|
||||
|
||||
try {
|
||||
if (resolvedPath.startsWith('file://')) {
|
||||
@@ -372,11 +372,44 @@ export function resolveToRealPath(path: string): string {
|
||||
// Ignore error (e.g. malformed URI), keep path from previous step
|
||||
}
|
||||
|
||||
return robustRealpath(path.resolve(resolvedPath));
|
||||
}
|
||||
|
||||
function robustRealpath(p: string, visited = new Set<string>()): string {
|
||||
const key = process.platform === 'win32' ? p.toLowerCase() : p;
|
||||
if (visited.has(key)) {
|
||||
throw new Error(`Infinite recursion detected in robustRealpath: ${p}`);
|
||||
}
|
||||
visited.add(key);
|
||||
try {
|
||||
return fs.realpathSync(resolvedPath);
|
||||
} catch (_e) {
|
||||
// If realpathSync fails, it might be because the path doesn't exist.
|
||||
// In that case, we can fall back to the path processed.
|
||||
return resolvedPath;
|
||||
return fs.realpathSync(p);
|
||||
} catch (e: unknown) {
|
||||
if (e && typeof e === 'object' && 'code' in e && e.code === 'ENOENT') {
|
||||
try {
|
||||
const stat = fs.lstatSync(p);
|
||||
if (stat.isSymbolicLink()) {
|
||||
const target = fs.readlinkSync(p);
|
||||
const resolvedTarget = path.resolve(path.dirname(p), target);
|
||||
return robustRealpath(resolvedTarget, visited);
|
||||
}
|
||||
} catch (lstatError: unknown) {
|
||||
// Not a symlink, or lstat failed. Re-throw if it's not an expected
|
||||
// ENOENT (e.g., a permissions error), otherwise resolve parent.
|
||||
if (
|
||||
!(
|
||||
lstatError &&
|
||||
typeof lstatError === 'object' &&
|
||||
'code' in lstatError &&
|
||||
lstatError.code === 'ENOENT'
|
||||
)
|
||||
) {
|
||||
throw lstatError;
|
||||
}
|
||||
}
|
||||
const parent = path.dirname(p);
|
||||
if (parent === p) return p;
|
||||
return path.join(robustRealpath(parent, visited), path.basename(p));
|
||||
}
|
||||
throw e;
|
||||
}
|
||||
}
|
||||
|
||||
@@ -4,10 +4,10 @@
|
||||
* SPDX-License-Identifier: Apache-2.0
|
||||
*/
|
||||
|
||||
import { isNodeError } from '../utils/errors.js';
|
||||
import * as fs from 'node:fs';
|
||||
import * as path from 'node:path';
|
||||
import { debugLogger } from './debugLogger.js';
|
||||
import { resolveToRealPath } from './paths.js';
|
||||
|
||||
export type Unsubscribe = () => void;
|
||||
|
||||
@@ -227,22 +227,7 @@ export class WorkspaceContext {
|
||||
* if it did exist.
|
||||
*/
|
||||
private fullyResolvedPath(pathToCheck: string): string {
|
||||
try {
|
||||
return fs.realpathSync(path.resolve(this.targetDir, pathToCheck));
|
||||
} catch (e: unknown) {
|
||||
if (
|
||||
isNodeError(e) &&
|
||||
e.code === 'ENOENT' &&
|
||||
e.path &&
|
||||
// realpathSync does not set e.path correctly for symlinks to
|
||||
// non-existent files.
|
||||
!this.isFileSymlink(e.path)
|
||||
) {
|
||||
// If it doesn't exist, e.path contains the fully resolved path.
|
||||
return e.path;
|
||||
}
|
||||
throw e;
|
||||
}
|
||||
return resolveToRealPath(path.resolve(this.targetDir, pathToCheck));
|
||||
}
|
||||
|
||||
/**
|
||||
@@ -262,15 +247,4 @@ export class WorkspaceContext {
|
||||
!path.isAbsolute(relative)
|
||||
);
|
||||
}
|
||||
|
||||
/**
|
||||
* Checks if a file path is a symbolic link that points to a file.
|
||||
*/
|
||||
private isFileSymlink(filePath: string): boolean {
|
||||
try {
|
||||
return !fs.readlinkSync(filePath).endsWith('/');
|
||||
} catch (_error) {
|
||||
return false;
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
@@ -1,6 +1,6 @@
|
||||
{
|
||||
"name": "@google/gemini-cli-devtools",
|
||||
"version": "0.33.0-nightly.20260228.1ca5c05d0",
|
||||
"version": "0.33.0-preview.9",
|
||||
"license": "Apache-2.0",
|
||||
"type": "module",
|
||||
"main": "dist/src/index.js",
|
||||
|
||||
@@ -1,6 +1,6 @@
|
||||
{
|
||||
"name": "@google/gemini-cli-sdk",
|
||||
"version": "0.33.0-nightly.20260228.1ca5c05d0",
|
||||
"version": "0.33.0-preview.9",
|
||||
"description": "Gemini CLI SDK",
|
||||
"license": "Apache-2.0",
|
||||
"repository": {
|
||||
|
||||
@@ -1,6 +1,6 @@
|
||||
{
|
||||
"name": "@google/gemini-cli-test-utils",
|
||||
"version": "0.33.0-nightly.20260228.1ca5c05d0",
|
||||
"version": "0.33.0-preview.9",
|
||||
"private": true,
|
||||
"main": "src/index.ts",
|
||||
"license": "Apache-2.0",
|
||||
|
||||
@@ -2,7 +2,7 @@
|
||||
"name": "gemini-cli-vscode-ide-companion",
|
||||
"displayName": "Gemini CLI Companion",
|
||||
"description": "Enable Gemini CLI with direct access to your IDE workspace.",
|
||||
"version": "0.33.0-nightly.20260228.1ca5c05d0",
|
||||
"version": "0.33.0-preview.9",
|
||||
"publisher": "google",
|
||||
"icon": "assets/icon.png",
|
||||
"repository": {
|
||||
|
||||
Reference in New Issue
Block a user