mirror of
https://github.com/google-gemini/gemini-cli.git
synced 2026-02-01 22:48:03 +00:00
fix(core): enhance tool aliasing to support multiple legacy names per tool
This commit is contained in:
@@ -44,15 +44,39 @@ vi.mock('../utils/shell-utils.js', async (importOriginal) => {
|
||||
});
|
||||
|
||||
// Mock tool-names to provide a consistent alias for testing
|
||||
|
||||
vi.mock('../tools/tool-names.js', async (importOriginal) => {
|
||||
const actual =
|
||||
await importOriginal<typeof import('../tools/tool-names.js')>();
|
||||
|
||||
const mockedAliases: Record<string, string> = {
|
||||
...actual.TOOL_LEGACY_ALIASES,
|
||||
|
||||
legacy_test_tool: 'current_test_tool',
|
||||
|
||||
another_legacy_test_tool: 'current_test_tool',
|
||||
};
|
||||
|
||||
return {
|
||||
...actual,
|
||||
TOOL_LEGACY_ALIASES: {
|
||||
...actual.TOOL_LEGACY_ALIASES,
|
||||
legacy_test_tool: 'current_test_tool',
|
||||
},
|
||||
|
||||
TOOL_LEGACY_ALIASES: mockedAliases,
|
||||
|
||||
getToolAliases: vi.fn().mockImplementation((name: string) => {
|
||||
const aliases = new Set<string>([name]);
|
||||
|
||||
const canonicalName = mockedAliases[name] ?? name;
|
||||
|
||||
aliases.add(canonicalName);
|
||||
|
||||
for (const [legacyName, currentName] of Object.entries(mockedAliases)) {
|
||||
if (currentName === canonicalName) {
|
||||
aliases.add(legacyName);
|
||||
}
|
||||
}
|
||||
|
||||
return Array.from(aliases);
|
||||
}),
|
||||
};
|
||||
});
|
||||
|
||||
@@ -230,6 +254,22 @@ describe('PolicyEngine', () => {
|
||||
expect(decision).toBe(PolicyDecision.ALLOW);
|
||||
});
|
||||
|
||||
it('should match tool call using one legacy name against policy for another legacy name (same canonical tool)', async () => {
|
||||
const legacyName1 = 'legacy_test_tool';
|
||||
const legacyName2 = 'another_legacy_test_tool';
|
||||
|
||||
const rules: PolicyRule[] = [
|
||||
{ toolName: legacyName2, decision: PolicyDecision.DENY },
|
||||
];
|
||||
|
||||
engine = new PolicyEngine({ rules });
|
||||
|
||||
// Call using legacyName1, should be denied because legacyName2 has a deny rule
|
||||
// and they both point to the same canonical tool.
|
||||
const { decision } = await engine.check({ name: legacyName1 }, undefined);
|
||||
expect(decision).toBe(PolicyDecision.DENY);
|
||||
});
|
||||
|
||||
it('should apply wildcard rules (no toolName)', async () => {
|
||||
const rules: PolicyRule[] = [
|
||||
{ decision: PolicyDecision.DENY }, // Applies to all tools
|
||||
|
||||
@@ -24,7 +24,7 @@ import {
|
||||
splitCommands,
|
||||
hasRedirection,
|
||||
} from '../utils/shell-utils.js';
|
||||
import { TOOL_LEGACY_ALIASES } from '../tools/tool-names.js';
|
||||
import { getToolAliases } from '../tools/tool-names.js';
|
||||
|
||||
function ruleMatches(
|
||||
rule: PolicyRule | SafetyCheckerRule,
|
||||
@@ -312,20 +312,9 @@ export class PolicyEngine {
|
||||
// We also want to check legacy aliases for the tool name.
|
||||
const toolNamesToTry = new Set<string>();
|
||||
if (toolCall.name) {
|
||||
toolNamesToTry.add(toolCall.name);
|
||||
|
||||
// Add legacy names that point to this tool
|
||||
for (const [legacyName, currentName] of Object.entries(
|
||||
TOOL_LEGACY_ALIASES,
|
||||
)) {
|
||||
if (currentName === toolCall.name) {
|
||||
toolNamesToTry.add(legacyName);
|
||||
}
|
||||
}
|
||||
|
||||
// Add the tool name's own legacy alias if it has one (for forward compatibility of old skills)
|
||||
if (TOOL_LEGACY_ALIASES[toolCall.name]) {
|
||||
toolNamesToTry.add(TOOL_LEGACY_ALIASES[toolCall.name]);
|
||||
const aliases = getToolAliases(toolCall.name);
|
||||
for (const alias of aliases) {
|
||||
toolNamesToTry.add(alias);
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -7,6 +7,7 @@
|
||||
import { describe, it, expect, vi } from 'vitest';
|
||||
import {
|
||||
isValidToolName,
|
||||
getToolAliases,
|
||||
ALL_BUILTIN_TOOL_NAMES,
|
||||
DISCOVERED_TOOL_PREFIX,
|
||||
LS_TOOL_NAME,
|
||||
@@ -18,6 +19,7 @@ vi.mock('./tool-names.js', async (importOriginal) => {
|
||||
const mockedAliases: Record<string, string> = {
|
||||
...actual.TOOL_LEGACY_ALIASES,
|
||||
legacy_test_tool: 'current_test_tool',
|
||||
another_legacy_test_tool: 'current_test_tool',
|
||||
};
|
||||
return {
|
||||
...actual,
|
||||
@@ -26,6 +28,17 @@ vi.mock('./tool-names.js', async (importOriginal) => {
|
||||
if (mockedAliases[name]) return true;
|
||||
return actual.isValidToolName(name, options);
|
||||
}),
|
||||
getToolAliases: vi.fn().mockImplementation((name: string) => {
|
||||
const aliases = new Set<string>([name]);
|
||||
const canonicalName = mockedAliases[name] ?? name;
|
||||
aliases.add(canonicalName);
|
||||
for (const [legacyName, currentName] of Object.entries(mockedAliases)) {
|
||||
if (currentName === canonicalName) {
|
||||
aliases.add(legacyName);
|
||||
}
|
||||
}
|
||||
return Array.from(aliases);
|
||||
}),
|
||||
};
|
||||
});
|
||||
|
||||
@@ -78,4 +91,25 @@ describe('tool-names', () => {
|
||||
);
|
||||
});
|
||||
});
|
||||
|
||||
describe('getToolAliases', () => {
|
||||
it('should return all associated names for a current tool', () => {
|
||||
const aliases = getToolAliases('current_test_tool');
|
||||
expect(aliases).toContain('current_test_tool');
|
||||
expect(aliases).toContain('legacy_test_tool');
|
||||
expect(aliases).toContain('another_legacy_test_tool');
|
||||
});
|
||||
|
||||
it('should return all associated names for a legacy tool', () => {
|
||||
const aliases = getToolAliases('legacy_test_tool');
|
||||
expect(aliases).toContain('current_test_tool');
|
||||
expect(aliases).toContain('legacy_test_tool');
|
||||
expect(aliases).toContain('another_legacy_test_tool');
|
||||
});
|
||||
|
||||
it('should return only the name itself if no aliases exist', () => {
|
||||
const aliases = getToolAliases('unknown_tool');
|
||||
expect(aliases).toEqual(['unknown_tool']);
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
@@ -35,6 +35,28 @@ export const TOOL_LEGACY_ALIASES: Record<string, string> = {
|
||||
// 'search_file_content': GREP_TOOL_NAME,
|
||||
};
|
||||
|
||||
/**
|
||||
* Returns all associated names for a tool (including legacy aliases and current name).
|
||||
* This ensures that if multiple legacy names point to the same tool, we consider all of them
|
||||
* for policy application.
|
||||
*/
|
||||
export function getToolAliases(name: string): string[] {
|
||||
const aliases = new Set<string>([name]);
|
||||
|
||||
// Determine the canonical (current) name
|
||||
const canonicalName = TOOL_LEGACY_ALIASES[name] ?? name;
|
||||
aliases.add(canonicalName);
|
||||
|
||||
// Find all other legacy aliases that point to the same canonical name
|
||||
for (const [legacyName, currentName] of Object.entries(TOOL_LEGACY_ALIASES)) {
|
||||
if (currentName === canonicalName) {
|
||||
aliases.add(legacyName);
|
||||
}
|
||||
}
|
||||
|
||||
return Array.from(aliases);
|
||||
}
|
||||
|
||||
/** Prefix used for tools discovered via the tool DiscoveryCommand. */
|
||||
export const DISCOVERED_TOOL_PREFIX = 'discovered_tool_';
|
||||
|
||||
|
||||
Reference in New Issue
Block a user