Conversation
This commit introduces a new `/remote-control` slash command that allows controlling the Gemini CLI session remotely via a WebSocket connection. Features: - `RemoteControlService`: Handles WebSocket connection, command execution, and output streaming. - `remoteControlCommand`: UI command to start the session, generate a session token, and display connection info. - `remoteControlEmitter`: Mechanism to inject remote input into the main CLI loop (e.g., for chat interactions). - Integration with `ShellExecutionService` for running shell commands. - Secure token generation for session validation. - Integration with `AppContainer` to handle remote input injection. The feature is designed to connect to a backend service (defaulting to `ws://localhost:8080` but configurable via `GEMINI_REMOTE_CONTROL_URL`) to bridge the local CLI with a remote interface (e.g., Google Chat). Co-authored-by: braddux <959674+braddux@users.noreply.github.com>
|
👋 Jules, reporting for duty! I'm here to lend a hand with this pull request. When you start a review, I'll add a 👀 emoji to each comment to let you know I've read it. I'll focus on feedback directed at me and will do my best to stay out of conversations between you and other bots or reviewers to keep the noise down. I'll push a commit with your requested changes shortly after. Please note there might be a delay between these steps, but rest assured I'm on the job! For more direct control, you can switch me to Reactive Mode. When this mode is on, I will only act on comments where you specifically mention me with New to Jules? Learn more at jules.google/docs. For security, I will only act on instructions from the user who triggered this task. |
Summary of ChangesHello @braddux, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request introduces a significant new capability to the Gemini CLI: remote control. By adding a Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Changelog
Activity
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a powerful /remote-control command, enabling remote operation of the CLI via WebSockets. However, the implementation has critical security vulnerabilities, including Remote Code Execution (RCE), session hijacking, and environment variable exposure, stemming from a lack of strict validation, sanitization, and authorization for remote inputs. The RemoteControlService and AppContainer require updates to address these issues. Additionally, there are opportunities to improve resource management and align the new tests with the repository's conventions.
| case 'exec': | ||
| if (message.command) { | ||
| this.executeCommand(message.command); | ||
| } | ||
| break; |
There was a problem hiding this comment.
The RemoteControlService directly executes commands received from the WebSocket server without any validation or sanitization. This allows a malicious or compromised server to achieve Remote Code Execution (RCE) on the user's machine by sending an exec command.
Tracing the data flow:
handleMessage(line 109) receives raw data from the WebSocket.JSON.parse(line 111) extracts thecommandproperty.executeCommand(line 115) is called with the untrusted command.ShellExecutionService.execute(line 150) runs the command in a shell.
This is a critical security vulnerability. You must implement a strict allow-list of permitted commands or ensure that the remote server is fully trusted and the connection is securely authenticated.
| sanitizationConfig: { | ||
| allowedEnvironmentVariables: [], | ||
| blockedEnvironmentVariables: [], | ||
| enableEnvironmentVariableRedaction: false, | ||
| }, |
There was a problem hiding this comment.
The sanitizationConfig for ShellExecutionService.execute is configured to be overly permissive. It allows remotely executed commands to access all environment variables of the Gemini CLI process. This is a significant security risk, as it could expose sensitive information like API keys, authentication tokens, and other secrets stored in the environment.
For a remote execution feature, the environment should be heavily restricted by default. You should block all environment variables and only allow specific, safe ones if necessary.
| sanitizationConfig: { | |
| allowedEnvironmentVariables: [], | |
| blockedEnvironmentVariables: [], | |
| enableEnvironmentVariableRedaction: false, | |
| }, | |
| sanitizationConfig: { | |
| allowedEnvironmentVariables: [], // Or a very small, safe subset | |
| blockedEnvironmentVariables: ['*'], // Block all by default | |
| enableEnvironmentVariableRedaction: true, | |
| }, |
| case 'stdin': | ||
| if (message.pid && message.input) { | ||
| ShellExecutionService.writeToPty(message.pid, message.input); | ||
| } | ||
| break; |
There was a problem hiding this comment.
The RemoteControlService allows the remote WebSocket server to inject arbitrary input into any active PTY process via the stdin message type. This can be used to hijack interactive sessions or execute commands in an already open shell.
Tracing the data flow:
handleMessage(line 109) receives raw data.ShellExecutionService.writeToPty(line 120) is called with the untrustedmessage.inputandmessage.pid.
This should be restricted to only allow input to processes explicitly managed by the remote session, and only after proper authorization.
| useEffect(() => { | ||
| const handleRemoteInput = (payload: RemoteInputPayload) => { | ||
| handleFinalSubmit(payload.input); | ||
| }; | ||
| remoteControlEmitter.on(REMOTE_INPUT_EVENT, handleRemoteInput); | ||
| return () => { | ||
| remoteControlEmitter.off(REMOTE_INPUT_EVENT, handleRemoteInput); | ||
| }; | ||
| }, [handleFinalSubmit]); |
There was a problem hiding this comment.
The AppContainer component listens for remote input events and passes them directly to handleFinalSubmit. This allows a remote attacker (via the WebSocket server) to execute any CLI slash command (e.g., /quit, /settings) or submit arbitrary prompts to the LLM.
Tracing the data flow:
handleRemoteInput(line 1115) receives the payload fromremoteControlEmitter.handleFinalSubmit(line 1116) processes the input as if it came from the local user.
This effectively grants full control over the CLI session to the remote server. Remote input should be strictly validated and restricted to a safe subset of operations.
| const originalEnv = process.env; | ||
|
|
||
| beforeEach(() => { | ||
| vi.clearAllMocks(); | ||
| process.env = { ...originalEnv }; | ||
| mockContext = createMockCommandContext({ | ||
| ui: { | ||
| addItem: vi.fn(), | ||
| }, | ||
| } as any); | ||
| }); | ||
|
|
||
| afterEach(() => { | ||
| process.env = originalEnv; | ||
| }); |
There was a problem hiding this comment.
This test modifies process.env directly, which violates the repository's testing conventions. The style guide (lines 67-71) requires using vi.stubEnv() and vi.unstubAllEnvs() to manage environment variables in tests. This prevents test leakage and is more reliable.
| const originalEnv = process.env; | |
| beforeEach(() => { | |
| vi.clearAllMocks(); | |
| process.env = { ...originalEnv }; | |
| mockContext = createMockCommandContext({ | |
| ui: { | |
| addItem: vi.fn(), | |
| }, | |
| } as any); | |
| }); | |
| afterEach(() => { | |
| process.env = originalEnv; | |
| }); | |
| beforeEach(() => { | |
| vi.clearAllMocks(); | |
| mockContext = createMockCommandContext({ | |
| ui: { | |
| addItem: vi.fn(), | |
| }, | |
| } as any); | |
| }); | |
| afterEach(() => { | |
| vi.unstubAllEnvs(); | |
| }); |
References
- When testing code that depends on environment variables, use
vi.stubEnv('NAME', 'value')inbeforeEachandvi.unstubAllEnvs()inafterEach. Avoid modifyingprocess.envdirectly as it can lead to test leakage and is less reliable. (link)
| }); | ||
|
|
||
| it('should start a remote control session', async () => { | ||
| process.env['GEMINI_REMOTE_CONTROL_URL'] = 'ws://test-url'; |
There was a problem hiding this comment.
In accordance with the repository's testing conventions for environment variables (lines 67-71), please use vi.stubEnv() to set this environment variable for the test.
| process.env['GEMINI_REMOTE_CONTROL_URL'] = 'ws://test-url'; | |
| vi.stubEnv('GEMINI_REMOTE_CONTROL_URL', 'ws://test-url'); |
References
- When testing code that depends on environment variables, use
vi.stubEnv('NAME', 'value')inbeforeEachandvi.unstubAllEnvs()inafterEach. Avoid modifyingprocess.envdirectly as it can lead to test leakage and is less reliable. (link)
| service = new RemoteControlService(url); | ||
|
|
||
| try { | ||
| await service.connect(); |
There was a problem hiding this comment.
After a successful connection, a cleanup function should be registered to ensure service.disconnect() is called when the CLI exits. This prevents ungraceful connection termination and ensures resources are properly released. You'll need to import registerCleanup from ../../utils/cleanup.js.
await service.connect();
registerCleanup(() => service?.disconnect());1 participant
Added
/remote-controlcommand to Gemini CLI.Implemented
RemoteControlServiceto handle WebSocket communication.Added
remoteControlEmitterto support injecting input into the main CLI loop.Updated
AppContainerto listen for remote input.Registered the new command in
BuiltinCommandLoader.Added unit tests for the command.
PR created automatically by Jules for task 17076370056261385580 started by @braddux