mirror of
https://github.com/google-gemini/gemini-cli.git
synced 2026-03-16 17:11:04 -07:00
Disallow Object.create() and reflect. (#22408)
This commit is contained in:
committed by
GitHub
parent
e3df87cf1a
commit
ef5627eece
@@ -51,6 +51,7 @@ export default tseslint.config(
|
||||
'evals/**',
|
||||
'packages/test-utils/**',
|
||||
'.gemini/skills/**',
|
||||
'**/*.d.ts',
|
||||
],
|
||||
},
|
||||
eslint.configs.recommended,
|
||||
@@ -206,11 +207,26 @@ export default tseslint.config(
|
||||
{
|
||||
// Rules that only apply to product code
|
||||
files: ['packages/*/src/**/*.{ts,tsx}'],
|
||||
ignores: ['**/*.test.ts', '**/*.test.tsx'],
|
||||
ignores: ['**/*.test.ts', '**/*.test.tsx', 'packages/*/src/test-utils/**'],
|
||||
rules: {
|
||||
'@typescript-eslint/no-unsafe-type-assertion': 'error',
|
||||
'@typescript-eslint/no-unsafe-assignment': 'error',
|
||||
'@typescript-eslint/no-unsafe-return': 'error',
|
||||
'no-restricted-syntax': [
|
||||
'error',
|
||||
...commonRestrictedSyntaxRules,
|
||||
{
|
||||
selector:
|
||||
'CallExpression[callee.object.name="Object"][callee.property.name="create"]',
|
||||
message:
|
||||
'Avoid using Object.create() in product code. Use object spread {...obj}, explicit class instantiation, structuredClone(), or copy constructors instead.',
|
||||
},
|
||||
{
|
||||
selector: 'Identifier[name="Reflect"]',
|
||||
message:
|
||||
'Avoid using Reflect namespace in product code. Do not use reflection to make copies. Instead, use explicit object copying or cloning (structuredClone() for values, new instance/clone function for classes).',
|
||||
},
|
||||
],
|
||||
},
|
||||
},
|
||||
{
|
||||
|
||||
@@ -57,18 +57,8 @@ export async function scheduleAgentTools(
|
||||
} = options;
|
||||
|
||||
// Create a proxy/override of the config to provide the agent-specific tool registry.
|
||||
// eslint-disable-next-line @typescript-eslint/no-unsafe-assignment
|
||||
const agentConfig: Config = Object.create(config);
|
||||
agentConfig.getToolRegistry = () => toolRegistry;
|
||||
agentConfig.getMessageBus = () => toolRegistry.messageBus;
|
||||
// Override toolRegistry property so AgentLoopContext reads the agent-specific registry.
|
||||
Object.defineProperty(agentConfig, 'toolRegistry', {
|
||||
get: () => toolRegistry,
|
||||
configurable: true,
|
||||
});
|
||||
|
||||
const schedulerContext = {
|
||||
config: agentConfig,
|
||||
config,
|
||||
promptId: config.promptId,
|
||||
toolRegistry,
|
||||
messageBus: toolRegistry.messageBus,
|
||||
|
||||
@@ -26,7 +26,6 @@ import {
|
||||
} from '../tools/mcp-tool.js';
|
||||
import { CompressionStatus } from '../core/turn.js';
|
||||
import { type ToolCallRequestInfo } from '../scheduler/types.js';
|
||||
import { type Message } from '../confirmation-bus/types.js';
|
||||
import { ChatCompressionService } from '../services/chatCompressionService.js';
|
||||
import { getDirectoryContextString } from '../utils/environmentContext.js';
|
||||
import { promptIdContext } from '../utils/promptIdContext.js';
|
||||
@@ -128,19 +127,7 @@ export class LocalAgentExecutor<TOutput extends z.ZodTypeAny> {
|
||||
const parentMessageBus = context.messageBus;
|
||||
|
||||
// Create an override object to inject the subagent name into tool confirmation requests
|
||||
// eslint-disable-next-line @typescript-eslint/no-unsafe-type-assertion
|
||||
const subagentMessageBus = Object.create(
|
||||
parentMessageBus,
|
||||
) as typeof parentMessageBus;
|
||||
subagentMessageBus.publish = async (message: Message) => {
|
||||
if (message.type === 'tool-confirmation-request') {
|
||||
return parentMessageBus.publish({
|
||||
...message,
|
||||
subagent: definition.name,
|
||||
});
|
||||
}
|
||||
return parentMessageBus.publish(message);
|
||||
};
|
||||
const subagentMessageBus = parentMessageBus.derive(definition.name);
|
||||
|
||||
// Create an isolated tool registry for this agent instance.
|
||||
const agentToolRegistry = new ToolRegistry(
|
||||
|
||||
@@ -520,23 +520,55 @@ export class AgentRegistry {
|
||||
return definition;
|
||||
}
|
||||
|
||||
// Use Object.create to preserve lazy getters on the definition object
|
||||
// eslint-disable-next-line @typescript-eslint/no-unsafe-assignment
|
||||
const merged: LocalAgentDefinition<TOutput> = Object.create(definition);
|
||||
|
||||
if (overrides.runConfig) {
|
||||
merged.runConfig = {
|
||||
...definition.runConfig,
|
||||
...overrides.runConfig,
|
||||
};
|
||||
}
|
||||
|
||||
if (overrides.modelConfig) {
|
||||
merged.modelConfig = ModelConfigService.merge(
|
||||
definition.modelConfig,
|
||||
overrides.modelConfig,
|
||||
);
|
||||
}
|
||||
// Preserve lazy getters on the definition object by wrapping in a new object with getters
|
||||
const merged: LocalAgentDefinition<TOutput> = {
|
||||
get kind() {
|
||||
return definition.kind;
|
||||
},
|
||||
get name() {
|
||||
return definition.name;
|
||||
},
|
||||
get displayName() {
|
||||
return definition.displayName;
|
||||
},
|
||||
get description() {
|
||||
return definition.description;
|
||||
},
|
||||
get experimental() {
|
||||
return definition.experimental;
|
||||
},
|
||||
get metadata() {
|
||||
return definition.metadata;
|
||||
},
|
||||
get inputConfig() {
|
||||
return definition.inputConfig;
|
||||
},
|
||||
get outputConfig() {
|
||||
return definition.outputConfig;
|
||||
},
|
||||
get promptConfig() {
|
||||
return definition.promptConfig;
|
||||
},
|
||||
get toolConfig() {
|
||||
return definition.toolConfig;
|
||||
},
|
||||
get processOutput() {
|
||||
return definition.processOutput;
|
||||
},
|
||||
get runConfig() {
|
||||
return overrides.runConfig
|
||||
? { ...definition.runConfig, ...overrides.runConfig }
|
||||
: definition.runConfig;
|
||||
},
|
||||
get modelConfig() {
|
||||
return overrides.modelConfig
|
||||
? ModelConfigService.merge(
|
||||
definition.modelConfig,
|
||||
overrides.modelConfig,
|
||||
)
|
||||
: definition.modelConfig;
|
||||
},
|
||||
};
|
||||
|
||||
return merged;
|
||||
}
|
||||
|
||||
@@ -262,4 +262,90 @@ describe('MessageBus', () => {
|
||||
);
|
||||
});
|
||||
});
|
||||
|
||||
describe('derive', () => {
|
||||
it('should receive responses from parent bus on derived bus', async () => {
|
||||
vi.spyOn(policyEngine, 'check').mockResolvedValue({
|
||||
decision: PolicyDecision.ASK_USER,
|
||||
});
|
||||
|
||||
const subagentName = 'test-subagent';
|
||||
const subagentBus = messageBus.derive(subagentName);
|
||||
|
||||
const request: Omit<ToolConfirmationRequest, 'correlationId'> = {
|
||||
type: MessageBusType.TOOL_CONFIRMATION_REQUEST,
|
||||
toolCall: { name: 'test-tool', args: {} },
|
||||
};
|
||||
|
||||
const requestPromise = subagentBus.request<
|
||||
ToolConfirmationRequest,
|
||||
ToolConfirmationResponse
|
||||
>(request, MessageBusType.TOOL_CONFIRMATION_RESPONSE, 2000);
|
||||
|
||||
// Wait for request on root bus and respond
|
||||
await new Promise<void>((resolve) => {
|
||||
messageBus.subscribe<ToolConfirmationRequest>(
|
||||
MessageBusType.TOOL_CONFIRMATION_REQUEST,
|
||||
(msg) => {
|
||||
if (msg.subagent === subagentName) {
|
||||
void messageBus.publish({
|
||||
type: MessageBusType.TOOL_CONFIRMATION_RESPONSE,
|
||||
correlationId: msg.correlationId,
|
||||
confirmed: true,
|
||||
});
|
||||
resolve();
|
||||
}
|
||||
},
|
||||
);
|
||||
});
|
||||
|
||||
await expect(requestPromise).resolves.toEqual(
|
||||
expect.objectContaining({
|
||||
type: MessageBusType.TOOL_CONFIRMATION_RESPONSE,
|
||||
confirmed: true,
|
||||
}),
|
||||
);
|
||||
});
|
||||
|
||||
it('should correctly chain subagent names for nested subagents', async () => {
|
||||
vi.spyOn(policyEngine, 'check').mockResolvedValue({
|
||||
decision: PolicyDecision.ASK_USER,
|
||||
});
|
||||
|
||||
const subagentBus1 = messageBus.derive('agent1');
|
||||
const subagentBus2 = subagentBus1.derive('agent2');
|
||||
|
||||
const request: Omit<ToolConfirmationRequest, 'correlationId'> = {
|
||||
type: MessageBusType.TOOL_CONFIRMATION_REQUEST,
|
||||
toolCall: { name: 'test-tool', args: {} },
|
||||
};
|
||||
|
||||
const requestPromise = subagentBus2.request<
|
||||
ToolConfirmationRequest,
|
||||
ToolConfirmationResponse
|
||||
>(request, MessageBusType.TOOL_CONFIRMATION_RESPONSE, 2000);
|
||||
|
||||
await new Promise<void>((resolve) => {
|
||||
messageBus.subscribe<ToolConfirmationRequest>(
|
||||
MessageBusType.TOOL_CONFIRMATION_REQUEST,
|
||||
(msg) => {
|
||||
if (msg.subagent === 'agent1/agent2') {
|
||||
void messageBus.publish({
|
||||
type: MessageBusType.TOOL_CONFIRMATION_RESPONSE,
|
||||
correlationId: msg.correlationId,
|
||||
confirmed: true,
|
||||
});
|
||||
resolve();
|
||||
}
|
||||
},
|
||||
);
|
||||
});
|
||||
|
||||
await expect(requestPromise).resolves.toEqual(
|
||||
expect.objectContaining({
|
||||
confirmed: true,
|
||||
}),
|
||||
);
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
@@ -40,6 +40,37 @@ export class MessageBus extends EventEmitter {
|
||||
this.emit(message.type, message);
|
||||
}
|
||||
|
||||
/**
|
||||
* Derives a child message bus scoped to a specific subagent.
|
||||
*/
|
||||
derive(subagentName: string): MessageBus {
|
||||
const bus = new MessageBus(this.policyEngine, this.debug);
|
||||
|
||||
bus.publish = async (message: Message) => {
|
||||
if (message.type === MessageBusType.TOOL_CONFIRMATION_REQUEST) {
|
||||
return this.publish({
|
||||
...message,
|
||||
subagent: message.subagent
|
||||
? `${subagentName}/${message.subagent}`
|
||||
: subagentName,
|
||||
});
|
||||
}
|
||||
return this.publish(message);
|
||||
};
|
||||
|
||||
// Delegate subscription methods to the parent bus
|
||||
bus.subscribe = this.subscribe.bind(this);
|
||||
bus.unsubscribe = this.unsubscribe.bind(this);
|
||||
bus.on = this.on.bind(this);
|
||||
bus.off = this.off.bind(this);
|
||||
bus.emit = this.emit.bind(this);
|
||||
bus.once = this.once.bind(this);
|
||||
bus.removeListener = this.removeListener.bind(this);
|
||||
bus.listenerCount = this.listenerCount.bind(this);
|
||||
|
||||
return bus;
|
||||
}
|
||||
|
||||
async publish(message: Message): Promise<void> {
|
||||
if (this.debug) {
|
||||
debugLogger.debug(`[MESSAGE_BUS] publish: ${safeJsonStringify(message)}`);
|
||||
|
||||
@@ -233,6 +233,15 @@ export class ToolRegistry {
|
||||
return this.messageBus;
|
||||
}
|
||||
|
||||
/**
|
||||
* Creates a shallow clone of the registry and its current known tools.
|
||||
*/
|
||||
clone(): ToolRegistry {
|
||||
const clone = new ToolRegistry(this.config, this.messageBus);
|
||||
clone.allKnownTools = new Map(this.allKnownTools);
|
||||
return clone;
|
||||
}
|
||||
|
||||
/**
|
||||
* Registers a tool definition.
|
||||
*
|
||||
|
||||
@@ -77,43 +77,55 @@ export function patchStdio(): () => void {
|
||||
};
|
||||
}
|
||||
|
||||
/**
|
||||
* Type guard to check if a property key exists on an object.
|
||||
*/
|
||||
function isKey<T extends object>(
|
||||
key: string | symbol | number,
|
||||
obj: T,
|
||||
): key is keyof T {
|
||||
return key in obj;
|
||||
}
|
||||
|
||||
/**
|
||||
* Creates proxies for process.stdout and process.stderr that use the real write methods
|
||||
* (writeToStdout and writeToStderr) bypassing any monkey patching.
|
||||
* This is used to write to the real output even when stdio is patched.
|
||||
*/
|
||||
export function createWorkingStdio() {
|
||||
const inkStdout = new Proxy(process.stdout, {
|
||||
get(target, prop, receiver) {
|
||||
const stdoutHandler: ProxyHandler<typeof process.stdout> = {
|
||||
get(target, prop) {
|
||||
if (prop === 'write') {
|
||||
return writeToStdout;
|
||||
}
|
||||
// eslint-disable-next-line @typescript-eslint/no-unsafe-assignment
|
||||
const value = Reflect.get(target, prop, receiver);
|
||||
if (typeof value === 'function') {
|
||||
// eslint-disable-next-line @typescript-eslint/no-unsafe-return
|
||||
return value.bind(target);
|
||||
if (isKey(prop, target)) {
|
||||
const value = target[prop];
|
||||
if (typeof value === 'function') {
|
||||
return value.bind(target);
|
||||
}
|
||||
return value;
|
||||
}
|
||||
// eslint-disable-next-line @typescript-eslint/no-unsafe-return
|
||||
return value;
|
||||
return undefined;
|
||||
},
|
||||
});
|
||||
};
|
||||
const inkStdout = new Proxy(process.stdout, stdoutHandler);
|
||||
|
||||
const inkStderr = new Proxy(process.stderr, {
|
||||
get(target, prop, receiver) {
|
||||
const stderrHandler: ProxyHandler<typeof process.stderr> = {
|
||||
get(target, prop) {
|
||||
if (prop === 'write') {
|
||||
return writeToStderr;
|
||||
}
|
||||
// eslint-disable-next-line @typescript-eslint/no-unsafe-assignment
|
||||
const value = Reflect.get(target, prop, receiver);
|
||||
if (typeof value === 'function') {
|
||||
// eslint-disable-next-line @typescript-eslint/no-unsafe-return
|
||||
return value.bind(target);
|
||||
if (isKey(prop, target)) {
|
||||
const value = target[prop];
|
||||
if (typeof value === 'function') {
|
||||
return value.bind(target);
|
||||
}
|
||||
return value;
|
||||
}
|
||||
// eslint-disable-next-line @typescript-eslint/no-unsafe-return
|
||||
return value;
|
||||
return undefined;
|
||||
},
|
||||
});
|
||||
};
|
||||
const inkStderr = new Proxy(process.stderr, stderrHandler);
|
||||
|
||||
return { stdout: inkStdout, stderr: inkStderr };
|
||||
}
|
||||
|
||||
@@ -243,10 +243,10 @@ export class GeminiCliSession {
|
||||
|
||||
const loopContext: AgentLoopContext = this.config;
|
||||
const originalRegistry = loopContext.toolRegistry;
|
||||
// eslint-disable-next-line @typescript-eslint/no-unsafe-assignment
|
||||
const scopedRegistry: ToolRegistry = Object.create(originalRegistry);
|
||||
const scopedRegistry: ToolRegistry = originalRegistry.clone();
|
||||
const originalGetTool = scopedRegistry.getTool.bind(scopedRegistry);
|
||||
scopedRegistry.getTool = (name: string) => {
|
||||
const tool = originalRegistry.getTool(name);
|
||||
const tool = originalGetTool(name);
|
||||
if (tool instanceof SdkTool) {
|
||||
return tool.bindContext(context);
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user