Skip to content

Commit dd14208

Browse files
gundermancProthamD
authored andcommitted
Disallow Object.create() and reflect. (google-gemini#22408)
1 parent 368a71c commit dd14208

File tree

9 files changed

+229
-66
lines changed

9 files changed

+229
-66
lines changed

eslint.config.js

Lines changed: 17 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -51,6 +51,7 @@ export default tseslint.config(
5151
'evals/**',
5252
'packages/test-utils/**',
5353
'.gemini/skills/**',
54+
'**/*.d.ts',
5455
],
5556
},
5657
eslint.configs.recommended,
@@ -206,11 +207,26 @@ export default tseslint.config(
206207
{
207208
// Rules that only apply to product code
208209
files: ['packages/*/src/**/*.{ts,tsx}'],
209-
ignores: ['**/*.test.ts', '**/*.test.tsx'],
210+
ignores: ['**/*.test.ts', '**/*.test.tsx', 'packages/*/src/test-utils/**'],
210211
rules: {
211212
'@typescript-eslint/no-unsafe-type-assertion': 'error',
212213
'@typescript-eslint/no-unsafe-assignment': 'error',
213214
'@typescript-eslint/no-unsafe-return': 'error',
215+
'no-restricted-syntax': [
216+
'error',
217+
...commonRestrictedSyntaxRules,
218+
{
219+
selector:
220+
'CallExpression[callee.object.name="Object"][callee.property.name="create"]',
221+
message:
222+
'Avoid using Object.create() in product code. Use object spread {...obj}, explicit class instantiation, structuredClone(), or copy constructors instead.',
223+
},
224+
{
225+
selector: 'Identifier[name="Reflect"]',
226+
message:
227+
'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).',
228+
},
229+
],
214230
},
215231
},
216232
{

packages/core/src/agents/agent-scheduler.ts

Lines changed: 1 addition & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -57,18 +57,8 @@ export async function scheduleAgentTools(
5757
} = options;
5858

5959
// Create a proxy/override of the config to provide the agent-specific tool registry.
60-
// eslint-disable-next-line @typescript-eslint/no-unsafe-assignment
61-
const agentConfig: Config = Object.create(config);
62-
agentConfig.getToolRegistry = () => toolRegistry;
63-
agentConfig.getMessageBus = () => toolRegistry.messageBus;
64-
// Override toolRegistry property so AgentLoopContext reads the agent-specific registry.
65-
Object.defineProperty(agentConfig, 'toolRegistry', {
66-
get: () => toolRegistry,
67-
configurable: true,
68-
});
69-
7060
const schedulerContext = {
71-
config: agentConfig,
61+
config,
7262
promptId: config.promptId,
7363
toolRegistry,
7464
messageBus: toolRegistry.messageBus,

packages/core/src/agents/local-executor.ts

Lines changed: 1 addition & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,6 @@ import {
2626
} from '../tools/mcp-tool.js';
2727
import { CompressionStatus } from '../core/turn.js';
2828
import { type ToolCallRequestInfo } from '../scheduler/types.js';
29-
import { type Message } from '../confirmation-bus/types.js';
3029
import { ChatCompressionService } from '../services/chatCompressionService.js';
3130
import { getDirectoryContextString } from '../utils/environmentContext.js';
3231
import { promptIdContext } from '../utils/promptIdContext.js';
@@ -128,19 +127,7 @@ export class LocalAgentExecutor<TOutput extends z.ZodTypeAny> {
128127
const parentMessageBus = context.messageBus;
129128

130129
// Create an override object to inject the subagent name into tool confirmation requests
131-
// eslint-disable-next-line @typescript-eslint/no-unsafe-type-assertion
132-
const subagentMessageBus = Object.create(
133-
parentMessageBus,
134-
) as typeof parentMessageBus;
135-
subagentMessageBus.publish = async (message: Message) => {
136-
if (message.type === 'tool-confirmation-request') {
137-
return parentMessageBus.publish({
138-
...message,
139-
subagent: definition.name,
140-
});
141-
}
142-
return parentMessageBus.publish(message);
143-
};
130+
const subagentMessageBus = parentMessageBus.derive(definition.name);
144131

145132
// Create an isolated tool registry for this agent instance.
146133
const agentToolRegistry = new ToolRegistry(

packages/core/src/agents/registry.ts

Lines changed: 49 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -520,23 +520,55 @@ export class AgentRegistry {
520520
return definition;
521521
}
522522

523-
// Use Object.create to preserve lazy getters on the definition object
524-
// eslint-disable-next-line @typescript-eslint/no-unsafe-assignment
525-
const merged: LocalAgentDefinition<TOutput> = Object.create(definition);
526-
527-
if (overrides.runConfig) {
528-
merged.runConfig = {
529-
...definition.runConfig,
530-
...overrides.runConfig,
531-
};
532-
}
533-
534-
if (overrides.modelConfig) {
535-
merged.modelConfig = ModelConfigService.merge(
536-
definition.modelConfig,
537-
overrides.modelConfig,
538-
);
539-
}
523+
// Preserve lazy getters on the definition object by wrapping in a new object with getters
524+
const merged: LocalAgentDefinition<TOutput> = {
525+
get kind() {
526+
return definition.kind;
527+
},
528+
get name() {
529+
return definition.name;
530+
},
531+
get displayName() {
532+
return definition.displayName;
533+
},
534+
get description() {
535+
return definition.description;
536+
},
537+
get experimental() {
538+
return definition.experimental;
539+
},
540+
get metadata() {
541+
return definition.metadata;
542+
},
543+
get inputConfig() {
544+
return definition.inputConfig;
545+
},
546+
get outputConfig() {
547+
return definition.outputConfig;
548+
},
549+
get promptConfig() {
550+
return definition.promptConfig;
551+
},
552+
get toolConfig() {
553+
return definition.toolConfig;
554+
},
555+
get processOutput() {
556+
return definition.processOutput;
557+
},
558+
get runConfig() {
559+
return overrides.runConfig
560+
? { ...definition.runConfig, ...overrides.runConfig }
561+
: definition.runConfig;
562+
},
563+
get modelConfig() {
564+
return overrides.modelConfig
565+
? ModelConfigService.merge(
566+
definition.modelConfig,
567+
overrides.modelConfig,
568+
)
569+
: definition.modelConfig;
570+
},
571+
};
540572

541573
return merged;
542574
}

packages/core/src/confirmation-bus/message-bus.test.ts

Lines changed: 86 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -262,4 +262,90 @@ describe('MessageBus', () => {
262262
);
263263
});
264264
});
265+
266+
describe('derive', () => {
267+
it('should receive responses from parent bus on derived bus', async () => {
268+
vi.spyOn(policyEngine, 'check').mockResolvedValue({
269+
decision: PolicyDecision.ASK_USER,
270+
});
271+
272+
const subagentName = 'test-subagent';
273+
const subagentBus = messageBus.derive(subagentName);
274+
275+
const request: Omit<ToolConfirmationRequest, 'correlationId'> = {
276+
type: MessageBusType.TOOL_CONFIRMATION_REQUEST,
277+
toolCall: { name: 'test-tool', args: {} },
278+
};
279+
280+
const requestPromise = subagentBus.request<
281+
ToolConfirmationRequest,
282+
ToolConfirmationResponse
283+
>(request, MessageBusType.TOOL_CONFIRMATION_RESPONSE, 2000);
284+
285+
// Wait for request on root bus and respond
286+
await new Promise<void>((resolve) => {
287+
messageBus.subscribe<ToolConfirmationRequest>(
288+
MessageBusType.TOOL_CONFIRMATION_REQUEST,
289+
(msg) => {
290+
if (msg.subagent === subagentName) {
291+
void messageBus.publish({
292+
type: MessageBusType.TOOL_CONFIRMATION_RESPONSE,
293+
correlationId: msg.correlationId,
294+
confirmed: true,
295+
});
296+
resolve();
297+
}
298+
},
299+
);
300+
});
301+
302+
await expect(requestPromise).resolves.toEqual(
303+
expect.objectContaining({
304+
type: MessageBusType.TOOL_CONFIRMATION_RESPONSE,
305+
confirmed: true,
306+
}),
307+
);
308+
});
309+
310+
it('should correctly chain subagent names for nested subagents', async () => {
311+
vi.spyOn(policyEngine, 'check').mockResolvedValue({
312+
decision: PolicyDecision.ASK_USER,
313+
});
314+
315+
const subagentBus1 = messageBus.derive('agent1');
316+
const subagentBus2 = subagentBus1.derive('agent2');
317+
318+
const request: Omit<ToolConfirmationRequest, 'correlationId'> = {
319+
type: MessageBusType.TOOL_CONFIRMATION_REQUEST,
320+
toolCall: { name: 'test-tool', args: {} },
321+
};
322+
323+
const requestPromise = subagentBus2.request<
324+
ToolConfirmationRequest,
325+
ToolConfirmationResponse
326+
>(request, MessageBusType.TOOL_CONFIRMATION_RESPONSE, 2000);
327+
328+
await new Promise<void>((resolve) => {
329+
messageBus.subscribe<ToolConfirmationRequest>(
330+
MessageBusType.TOOL_CONFIRMATION_REQUEST,
331+
(msg) => {
332+
if (msg.subagent === 'agent1/agent2') {
333+
void messageBus.publish({
334+
type: MessageBusType.TOOL_CONFIRMATION_RESPONSE,
335+
correlationId: msg.correlationId,
336+
confirmed: true,
337+
});
338+
resolve();
339+
}
340+
},
341+
);
342+
});
343+
344+
await expect(requestPromise).resolves.toEqual(
345+
expect.objectContaining({
346+
confirmed: true,
347+
}),
348+
);
349+
});
350+
});
265351
});

packages/core/src/confirmation-bus/message-bus.ts

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,37 @@ export class MessageBus extends EventEmitter {
4040
this.emit(message.type, message);
4141
}
4242

43+
/**
44+
* Derives a child message bus scoped to a specific subagent.
45+
*/
46+
derive(subagentName: string): MessageBus {
47+
const bus = new MessageBus(this.policyEngine, this.debug);
48+
49+
bus.publish = async (message: Message) => {
50+
if (message.type === MessageBusType.TOOL_CONFIRMATION_REQUEST) {
51+
return this.publish({
52+
...message,
53+
subagent: message.subagent
54+
? `${subagentName}/${message.subagent}`
55+
: subagentName,
56+
});
57+
}
58+
return this.publish(message);
59+
};
60+
61+
// Delegate subscription methods to the parent bus
62+
bus.subscribe = this.subscribe.bind(this);
63+
bus.unsubscribe = this.unsubscribe.bind(this);
64+
bus.on = this.on.bind(this);
65+
bus.off = this.off.bind(this);
66+
bus.emit = this.emit.bind(this);
67+
bus.once = this.once.bind(this);
68+
bus.removeListener = this.removeListener.bind(this);
69+
bus.listenerCount = this.listenerCount.bind(this);
70+
71+
return bus;
72+
}
73+
4374
async publish(message: Message): Promise<void> {
4475
if (this.debug) {
4576
debugLogger.debug(`[MESSAGE_BUS] publish: ${safeJsonStringify(message)}`);

packages/core/src/tools/tool-registry.ts

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -233,6 +233,15 @@ export class ToolRegistry {
233233
return this.messageBus;
234234
}
235235

236+
/**
237+
* Creates a shallow clone of the registry and its current known tools.
238+
*/
239+
clone(): ToolRegistry {
240+
const clone = new ToolRegistry(this.config, this.messageBus);
241+
clone.allKnownTools = new Map(this.allKnownTools);
242+
return clone;
243+
}
244+
236245
/**
237246
* Registers a tool definition.
238247
*

packages/core/src/utils/stdio.ts

Lines changed: 32 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -77,43 +77,55 @@ export function patchStdio(): () => void {
7777
};
7878
}
7979

80+
/**
81+
* Type guard to check if a property key exists on an object.
82+
*/
83+
function isKey<T extends object>(
84+
key: string | symbol | number,
85+
obj: T,
86+
): key is keyof T {
87+
return key in obj;
88+
}
89+
8090
/**
8191
* Creates proxies for process.stdout and process.stderr that use the real write methods
8292
* (writeToStdout and writeToStderr) bypassing any monkey patching.
8393
* This is used to write to the real output even when stdio is patched.
8494
*/
8595
export function createWorkingStdio() {
86-
const inkStdout = new Proxy(process.stdout, {
87-
get(target, prop, receiver) {
96+
const stdoutHandler: ProxyHandler<typeof process.stdout> = {
97+
get(target, prop) {
8898
if (prop === 'write') {
8999
return writeToStdout;
90100
}
91-
// eslint-disable-next-line @typescript-eslint/no-unsafe-assignment
92-
const value = Reflect.get(target, prop, receiver);
93-
if (typeof value === 'function') {
94-
// eslint-disable-next-line @typescript-eslint/no-unsafe-return
95-
return value.bind(target);
101+
if (isKey(prop, target)) {
102+
const value = target[prop];
103+
if (typeof value === 'function') {
104+
return value.bind(target);
105+
}
106+
return value;
96107
}
97-
// eslint-disable-next-line @typescript-eslint/no-unsafe-return
98-
return value;
108+
return undefined;
99109
},
100-
});
110+
};
111+
const inkStdout = new Proxy(process.stdout, stdoutHandler);
101112

102-
const inkStderr = new Proxy(process.stderr, {
103-
get(target, prop, receiver) {
113+
const stderrHandler: ProxyHandler<typeof process.stderr> = {
114+
get(target, prop) {
104115
if (prop === 'write') {
105116
return writeToStderr;
106117
}
107-
// eslint-disable-next-line @typescript-eslint/no-unsafe-assignment
108-
const value = Reflect.get(target, prop, receiver);
109-
if (typeof value === 'function') {
110-
// eslint-disable-next-line @typescript-eslint/no-unsafe-return
111-
return value.bind(target);
118+
if (isKey(prop, target)) {
119+
const value = target[prop];
120+
if (typeof value === 'function') {
121+
return value.bind(target);
122+
}
123+
return value;
112124
}
113-
// eslint-disable-next-line @typescript-eslint/no-unsafe-return
114-
return value;
125+
return undefined;
115126
},
116-
});
127+
};
128+
const inkStderr = new Proxy(process.stderr, stderrHandler);
117129

118130
return { stdout: inkStdout, stderr: inkStderr };
119131
}

packages/sdk/src/session.ts

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -243,10 +243,10 @@ export class GeminiCliSession {
243243

244244
const loopContext: AgentLoopContext = this.config;
245245
const originalRegistry = loopContext.toolRegistry;
246-
// eslint-disable-next-line @typescript-eslint/no-unsafe-assignment
247-
const scopedRegistry: ToolRegistry = Object.create(originalRegistry);
246+
const scopedRegistry: ToolRegistry = originalRegistry.clone();
247+
const originalGetTool = scopedRegistry.getTool.bind(scopedRegistry);
248248
scopedRegistry.getTool = (name: string) => {
249-
const tool = originalRegistry.getTool(name);
249+
const tool = originalGetTool(name);
250250
if (tool instanceof SdkTool) {
251251
return tool.bindContext(context);
252252
}

0 commit comments

Comments
 (0)