mirror of
https://github.com/google-gemini/gemini-cli.git
synced 2026-04-20 10:10:56 -07:00
fix(core): ensure robust sandbox cleanup in all process execution paths (#24763)
Co-authored-by: Spencer <spencertang@google.com>
This commit is contained in:
Emily Hedlund
@@ -847,34 +847,40 @@ export const spawnAsync = async (
|
||||
|
||||
const { program: finalCommand, args: finalArgs, env: finalEnv } = prepared;
|
||||
|
||||
return new Promise((resolve, reject) => {
|
||||
const child = spawn(finalCommand, finalArgs, {
|
||||
...options,
|
||||
env: finalEnv,
|
||||
});
|
||||
let stdout = '';
|
||||
let stderr = '';
|
||||
try {
|
||||
return await new Promise((resolve, reject) => {
|
||||
const child = spawn(finalCommand, finalArgs, {
|
||||
...options,
|
||||
env: finalEnv,
|
||||
});
|
||||
let stdout = '';
|
||||
let stderr = '';
|
||||
|
||||
child.stdout.on('data', (data) => {
|
||||
stdout += data.toString();
|
||||
});
|
||||
child.stdout.on('data', (data) => {
|
||||
stdout += data.toString();
|
||||
});
|
||||
|
||||
child.stderr.on('data', (data) => {
|
||||
stderr += data.toString();
|
||||
});
|
||||
child.stderr.on('data', (data) => {
|
||||
stderr += data.toString();
|
||||
});
|
||||
|
||||
child.on('close', (code) => {
|
||||
if (code === 0) {
|
||||
resolve({ stdout, stderr });
|
||||
} else {
|
||||
reject(new Error(`Command failed with exit code ${code}:\n${stderr}`));
|
||||
}
|
||||
});
|
||||
child.on('close', (code) => {
|
||||
if (code === 0) {
|
||||
resolve({ stdout, stderr });
|
||||
} else {
|
||||
reject(
|
||||
new Error(`Command failed with exit code ${code}:\n${stderr}`),
|
||||
);
|
||||
}
|
||||
});
|
||||
|
||||
child.on('error', (err) => {
|
||||
reject(err);
|
||||
child.on('error', (err) => {
|
||||
reject(err);
|
||||
});
|
||||
});
|
||||
});
|
||||
} finally {
|
||||
prepared.cleanup?.();
|
||||
}
|
||||
};
|
||||
|
||||
/**
|
||||
@@ -902,109 +908,115 @@ export async function* execStreaming(
|
||||
env: options?.env ?? process.env,
|
||||
});
|
||||
|
||||
const { program: finalCommand, args: finalArgs, env: finalEnv } = prepared;
|
||||
|
||||
const child = spawn(finalCommand, finalArgs, {
|
||||
...options,
|
||||
env: finalEnv,
|
||||
// ensure we don't open a window on windows if possible/relevant
|
||||
windowsHide: true,
|
||||
});
|
||||
|
||||
const rl = readline.createInterface({
|
||||
input: child.stdout,
|
||||
terminal: false,
|
||||
});
|
||||
|
||||
const errorChunks: Buffer[] = [];
|
||||
let stderrTotalBytes = 0;
|
||||
const MAX_STDERR_BYTES = 20 * 1024; // 20KB limit
|
||||
|
||||
child.stderr.on('data', (chunk) => {
|
||||
if (stderrTotalBytes < MAX_STDERR_BYTES) {
|
||||
errorChunks.push(chunk);
|
||||
stderrTotalBytes += chunk.length;
|
||||
}
|
||||
});
|
||||
|
||||
let error: Error | null = null;
|
||||
child.on('error', (err) => {
|
||||
error = err;
|
||||
});
|
||||
|
||||
const onAbort = () => {
|
||||
// If manually aborted by signal, we kill immediately.
|
||||
if (!child.killed) child.kill();
|
||||
};
|
||||
|
||||
if (options?.signal?.aborted) {
|
||||
onAbort();
|
||||
} else {
|
||||
options?.signal?.addEventListener('abort', onAbort);
|
||||
}
|
||||
|
||||
let finished = false;
|
||||
try {
|
||||
for await (const line of rl) {
|
||||
if (options?.signal?.aborted) break;
|
||||
yield line;
|
||||
}
|
||||
finished = true;
|
||||
} finally {
|
||||
rl.close();
|
||||
options?.signal?.removeEventListener('abort', onAbort);
|
||||
const { program: finalCommand, args: finalArgs, env: finalEnv } = prepared;
|
||||
|
||||
// Ensure process is killed when the generator is closed (consumer breaks loop)
|
||||
let killedByGenerator = false;
|
||||
if (!finished && child.exitCode === null && !child.killed) {
|
||||
try {
|
||||
child.kill();
|
||||
} catch {
|
||||
// ignore error if process is already dead
|
||||
const child = spawn(finalCommand, finalArgs, {
|
||||
...options,
|
||||
env: finalEnv,
|
||||
// ensure we don't open a window on windows if possible/relevant
|
||||
windowsHide: true,
|
||||
});
|
||||
|
||||
const rl = readline.createInterface({
|
||||
input: child.stdout,
|
||||
terminal: false,
|
||||
});
|
||||
|
||||
const errorChunks: Buffer[] = [];
|
||||
let stderrTotalBytes = 0;
|
||||
const MAX_STDERR_BYTES = 20 * 1024; // 20KB limit
|
||||
|
||||
child.stderr.on('data', (chunk) => {
|
||||
if (stderrTotalBytes < MAX_STDERR_BYTES) {
|
||||
errorChunks.push(chunk);
|
||||
stderrTotalBytes += chunk.length;
|
||||
}
|
||||
killedByGenerator = true;
|
||||
});
|
||||
|
||||
let error: Error | null = null;
|
||||
child.on('error', (err) => {
|
||||
error = err;
|
||||
});
|
||||
|
||||
const onAbort = () => {
|
||||
// If manually aborted by signal, we kill immediately.
|
||||
if (!child.killed) child.kill();
|
||||
};
|
||||
|
||||
if (options?.signal?.aborted) {
|
||||
onAbort();
|
||||
} else {
|
||||
options?.signal?.addEventListener('abort', onAbort);
|
||||
}
|
||||
|
||||
// Ensure we wait for the process to exit to check codes
|
||||
await new Promise<void>((resolve, reject) => {
|
||||
// If an error occurred before we got here (e.g. spawn failure), reject immediately.
|
||||
if (error) {
|
||||
reject(error);
|
||||
return;
|
||||
let finished = false;
|
||||
try {
|
||||
for await (const line of rl) {
|
||||
if (options?.signal?.aborted) break;
|
||||
yield line;
|
||||
}
|
||||
finished = true;
|
||||
} finally {
|
||||
rl.close();
|
||||
options?.signal?.removeEventListener('abort', onAbort);
|
||||
|
||||
// Ensure process is killed when the generator is closed (consumer breaks loop)
|
||||
let killedByGenerator = false;
|
||||
if (!finished && child.exitCode === null && !child.killed) {
|
||||
try {
|
||||
child.kill();
|
||||
} catch {
|
||||
// ignore error if process is already dead
|
||||
}
|
||||
killedByGenerator = true;
|
||||
}
|
||||
|
||||
function checkExit(code: number | null) {
|
||||
// If we aborted or killed it manually, we treat it as success (stop waiting)
|
||||
if (options?.signal?.aborted || killedByGenerator) {
|
||||
resolve();
|
||||
// Ensure we wait for the process to exit to check codes
|
||||
await new Promise<void>((resolve, reject) => {
|
||||
// If an error occurred before we got here (e.g. spawn failure), reject immediately.
|
||||
if (error) {
|
||||
reject(error);
|
||||
return;
|
||||
}
|
||||
|
||||
const allowed = options?.allowedExitCodes ?? [0];
|
||||
if (code !== null && allowed.includes(code)) {
|
||||
resolve();
|
||||
} else {
|
||||
// If we have an accumulated error or explicit error event
|
||||
if (error) reject(error);
|
||||
else {
|
||||
const stderr = Buffer.concat(errorChunks).toString('utf8');
|
||||
const truncatedMsg =
|
||||
stderrTotalBytes >= MAX_STDERR_BYTES ? '...[truncated]' : '';
|
||||
reject(
|
||||
new Error(
|
||||
`Process exited with code ${code}: ${stderr}${truncatedMsg}`,
|
||||
),
|
||||
);
|
||||
function checkExit(code: number | null) {
|
||||
// If we aborted or killed it manually, we treat it as success (stop waiting)
|
||||
if (options?.signal?.aborted || killedByGenerator) {
|
||||
resolve();
|
||||
return;
|
||||
}
|
||||
|
||||
const allowed = options?.allowedExitCodes ?? [0];
|
||||
if (code !== null && allowed.includes(code)) {
|
||||
resolve();
|
||||
} else {
|
||||
// If we have an accumulated error or explicit error event
|
||||
if (error) reject(error);
|
||||
else {
|
||||
const stderr = Buffer.concat(errorChunks).toString('utf8');
|
||||
const truncatedMsg =
|
||||
stderrTotalBytes >= MAX_STDERR_BYTES ? '...[truncated]' : '';
|
||||
reject(
|
||||
new Error(
|
||||
`Process exited with code ${code}: ${stderr}${truncatedMsg}`,
|
||||
),
|
||||
);
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
if (child.exitCode !== null) {
|
||||
checkExit(child.exitCode);
|
||||
} else {
|
||||
child.on('close', (code) => checkExit(code));
|
||||
child.on('error', (err) => reject(err));
|
||||
}
|
||||
});
|
||||
if (child.exitCode !== null) {
|
||||
checkExit(child.exitCode);
|
||||
} else {
|
||||
child.on('close', (code) => checkExit(code));
|
||||
child.on('error', (err) => {
|
||||
reject(err);
|
||||
});
|
||||
}
|
||||
});
|
||||
}
|
||||
} finally {
|
||||
prepared.cleanup?.();
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user