Skip to content
26 changes: 20 additions & 6 deletions core/src/code_executors/unsafe_local_code_executor.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
* SPDX-License-Identifier: Apache-2.0
*/

import {spawn} from 'child_process';
import {spawn} from 'node:child_process';
import * as fs from 'node:fs/promises';
import * as os from 'node:os';
import * as path from 'node:path';
Expand All @@ -20,6 +20,14 @@ import {

const IS_WINDOWS = os.platform() === 'win32';

/**
* Whether `commandPath` names Windows PowerShell (`powershell`) or PowerShell
* 7+ (`pwsh`). `path.win32` splits on both separators on every platform.
*/
function isPowerShellCommand(commandPath: string): boolean {
return /^(powershell|pwsh)(\.exe)?$/i.test(path.win32.basename(commandPath));
}

/**
* Options for UnsafeLocalCodeExecutor.
*/
Expand All @@ -38,14 +46,17 @@ export interface UnsafeLocalCodeExecutorOptions {
pythonCommandPath?: string;
/**
* The command to run Shell code. Default is `bash`.
*
* When it names `powershell` or `pwsh` (with or without `.exe`) the script
* is written as `.ps1` and run with `-NoLogo -ExecutionPolicy Bypass -File`.
*/
shellCommandPath?: string;
}

async function createTempScriptFile(
code: string,
language: CodeExecutionLanguage,
shellCommandPath?: string,

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why changed to be required?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch - reverted in 449368a, both createTempScriptFile and getExtensionForLanguage take shellCommandPath?: string again.

The reasoning was that the only call site is executeCode, which always passes this.shellCommandPath, and the constructor defaults that to 'powershell'/'bash' - so the optional made the undefined case unreachable. But that is a pre-existing cleanup and unrelated to this bug fix, so it did not belong in this diff. The new PowerShell check now guards the same way the cmd check on the next line already does:

if (shellCommandPath && isPowerShellCommand(shellCommandPath)) {
  return '.ps1';
}

The diff is now only the pwsh detection fix. Happy to send the signature tightening separately if you want it.

shellCommandPath: string,
): Promise<{filePath: string; tempDir: string}> {
const tempDir = path.join(
os.tmpdir(),
Expand All @@ -63,7 +74,7 @@ async function createTempScriptFile(

function getExtensionForLanguage(
language: CodeExecutionLanguage,
shellCommandPath?: string,
shellCommandPath: string,
): string | undefined {
if (language === CodeExecutionLanguage.JAVASCRIPT) {
return '.js';
Expand All @@ -82,8 +93,11 @@ function getExtensionForLanguage(
}

if (language === CodeExecutionLanguage.SHELL) {
if (isPowerShellCommand(shellCommandPath)) {
return '.ps1';
}
if (IS_WINDOWS) {
if (shellCommandPath && shellCommandPath.toLowerCase().includes('cmd')) {
if (shellCommandPath.toLowerCase().includes('cmd')) {
return '.bat';
}
return '.ps1';
Expand All @@ -101,7 +115,7 @@ function getExtensionForLanguage(
* **Execution Details**:
* - **JavaScript**: Executed via `node` (defaults to `process.execPath`).
* - **Python**: Executed via `python3` on Unix, and `python` on Windows.
* - **Shell**: Executed via `bash` on Unix, and defaults to `powershell` (injecting ExecutionPolicy Bypass) or `cmd.exe` on Windows.
* - **Shell**: Executed via `bash` on Unix, and defaults to `powershell` or `cmd.exe` on Windows. A `powershell` or `pwsh` command injects ExecutionPolicy Bypass.
*
* WARNING: This executor runs code in the local environment without sandboxing or security restrictions.
* Use with caution and only for trusted code.
Expand Down Expand Up @@ -171,7 +185,7 @@ export class UnsafeLocalCodeExecutor extends BaseCodeExecutor {
command = this.pythonCommandPath;
} else if (language === CodeExecutionLanguage.SHELL) {
command = this.shellCommandPath;
if (this.shellCommandPath.toLowerCase().includes('powershell')) {
if (isPowerShellCommand(this.shellCommandPath)) {
args = ['-NoLogo', '-ExecutionPolicy', 'Bypass', '-File', filePath];
} else if (this.shellCommandPath.toLowerCase().includes('cmd')) {
args = ['/c', filePath];
Expand Down
60 changes: 59 additions & 1 deletion core/test/code_executors/unsafe_local_code_executor_test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,13 @@ import {
UnsafeLocalCodeExecutor,
createSession,
} from '@google/adk';
import {beforeEach, describe, expect, it} from 'vitest';
import * as childProcess from 'node:child_process';
import {beforeEach, describe, expect, it, vi} from 'vitest';

// Records the spawn arguments while still running the real `spawn`, so the
// surrounding tests keep executing actual child processes.
vi.mock('node:child_process', {spy: true});
const spawnSpy = vi.mocked(childProcess.spawn);

function createMockInvocationContext(): InvocationContext {
const agent = new LlmAgent({
Expand Down Expand Up @@ -308,4 +314,56 @@ describe('UnsafeLocalCodeExecutor', () => {
expect(result.outputFiles![0].contentEncoding).toBe('utf-8');
expect(result.outputFiles![0].mimeType).toBe('application/json');
});

// Runners that ship PowerShell really launch it, and a cold start there
// exceeds the default 5s test timeout.
describe('shell command detection', {timeout: 30_000}, () => {
const POWERSHELL_FLAGS = ['-NoLogo', '-ExecutionPolicy', 'Bypass', '-File'];

async function captureShellSpawn(shellCommandPath: string) {
spawnSpy.mockClear();
const customExecutor = new UnsafeLocalCodeExecutor({shellCommandPath});
await customExecutor.executeCode({
invocationContext,
codeExecutionInput: {
code: 'echo "test"',
language: CodeExecutionLanguage.SHELL,
inputFiles: [],
},
});
expect(spawnSpy).toHaveBeenCalledTimes(1);
return spawnSpy.mock.calls[0][1];
}

it.each([
'pwsh',
'pwsh.exe',
'/usr/bin/pwsh',
'C:\\Program Files\\PowerShell\\7\\pwsh.exe',
'PWSH',
'powershell',
'powershell.exe',
])('runs a .ps1 script with PowerShell flags for %s', async (shell) => {
const args = await captureShellSpawn(shell);

expect(args).toEqual(expect.arrayContaining(POWERSHELL_FLAGS));
expect(args.at(-2)).toBe('-File');
expect(args.at(-1)).toMatch(/script\.ps1$/);
});

it.each([
'/opt/pwsh-tools/bin/bash',
'/usr/local/powershell-helpers/run.sh',
])('does not treat %s as PowerShell', async (shell) => {
const args = await captureShellSpawn(shell);

expect(args).toHaveLength(1);
});

it.each(['cmd', 'cmd.exe'])('invokes %s with /c', async (shell) => {
const args = await captureShellSpawn(shell);

expect(args).toEqual(['/c', expect.any(String)]);
});
});
});
Loading