Skip to content

fix(core): remove synchronous I/O from shell tool critical path #28397

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
Daksh7785 wants to merge 2 commits into
base: main
Choose a base branch
from
+48 −36
Open

fix(core): remove synchronous I/O from shell tool critical path #28397

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
93bccd1
fix(core): remove synchronous I/O from shell tool critical path
Jul 13, 2026
c53e6f7
Merge branch 'main' into fix/ui-stutter-sync-io
Daksh7785 Jul 13, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
84 changes: 48 additions & 36 deletions packages/core/src/tools/shell.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
*/

import fsPromises from 'node:fs/promises';
import fs from 'node:fs';

import path from 'node:path';
import os from 'node:os';
import { debugLogger } from '../index.js';
Expand Down Expand Up @@ -498,7 +498,9 @@ export class ShellToolInvocation extends BaseToolInvocation<

const onAbort = () => combinedController.abort();
try {
tempDir = fs.mkdtempSync(path.join(os.tmpdir(), 'gemini-shell-'));
tempDir = await fsPromises.mkdtemp(
path.join(os.tmpdir(), 'gemini-shell-'),
);
tempFilePath = path.join(tempDir, 'bgpids.tmp');

// Windows shells do not support the POSIX jobs output used here.
Expand Down Expand Up @@ -906,45 +908,55 @@ export class ShellToolInvocation extends BaseToolInvocation<
}

if (sandboxDenial.filePaths) {
for (const p of sandboxDenial.filePaths) {
try {
// Find an existing parent directory to add instead of a non-existent file
let currentPath = p;
if (currentPath.startsWith('~')) {
currentPath = path.join(os.homedir(), currentPath.slice(1));
}
await Promise.all(
sandboxDenial.filePaths.map(async (p) => {
try {
if (
fs.existsSync(currentPath) &&
fs.statSync(currentPath).isFile()
) {
currentPath = path.dirname(currentPath);
// Find an existing parent directory to add instead of a non-existent file
let currentPath = p;
if (currentPath.startsWith('~')) {
currentPath = path.join(os.homedir(), currentPath.slice(1));
}
} catch {
/* ignore */
}
while (currentPath.length > 1) {
if (fs.existsSync(currentPath)) {
const mode = this.context.config.getApprovalMode();
const isReadonlyMode =
this.context.config.sandboxPolicyManager.getModeConfig(
mode,
)?.readonly ?? false;
const isAllowed =
this.context.config.isPathAllowed(currentPath);

if (!isAllowed || isReadonlyMode) {
writePaths.add(currentPath);
readPaths.add(currentPath);
try {
const exists = await fsPromises
.access(currentPath)
.then(() => true)
.catch(() => false);
if (exists) {
const stat = await fsPromises.stat(currentPath);
if (stat.isFile()) {
currentPath = path.dirname(currentPath);
}
}
break;
} catch {
/* ignore */
}
currentPath = path.dirname(currentPath);
while (currentPath.length > 1) {
const exists = await fsPromises
.access(currentPath)
.then(() => true)
.catch(() => false);
if (exists) {
const mode = this.context.config.getApprovalMode();
const isReadonlyMode =
this.context.config.sandboxPolicyManager.getModeConfig(
mode,
)?.readonly ?? false;
const isAllowed =
this.context.config.isPathAllowed(currentPath);

if (!isAllowed || isReadonlyMode) {
writePaths.add(currentPath);
readPaths.add(currentPath);
}
break;
}
currentPath = path.dirname(currentPath);
}
Comment on lines +933 to +954

@gemini-code-assist gemini-code-assist Bot Jul 13, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

security-high high

An infinite loop / Denial of Service (DoS) vulnerability exists in the sandbox denial path resolution logic on Windows. When traversing up the directory tree using path.dirname(currentPath), if currentPath resolves to a non-existent root path (e.g., C:\ or a non-existent drive like Z:\) or an inaccessible UNC path, fsPromises.access will fail and path.dirname will return the same root path. Since the length of the root path is greater than 1, the loop condition while (currentPath.length > 1) remains true, causing an infinite loop that consumes 100% CPU, freezes the application, and hangs the shell tool execution indefinitely.

To remediate this, check if the parent directory returned by path.dirname(currentPath) is equal to currentPath (indicating that the root directory has been reached and no further traversal is possible) and break the loop.

                  while (currentPath.length > 1) {
                    const exists = await fsPromises
                      .access(currentPath)
                      .then(() => true)
                      .catch(() => false);
                    if (exists) {
                      const mode = this.context.config.getApprovalMode();
                      const isReadonlyMode =
                        this.context.config.sandboxPolicyManager.getModeConfig(
                          mode,
                        )?.readonly ?? false;
                      const isAllowed =
                        this.context.config.isPathAllowed(currentPath);

                      if (!isAllowed || isReadonlyMode) {
                        writePaths.add(currentPath);
                        readPaths.add(currentPath);
                      }
                      break;
                    }
                    const parent = path.dirname(currentPath);
                    if (parent === currentPath) {
                      break;
                    }
                    currentPath = parent;
                  }

} catch {
// ignore
}
} catch {
// ignore
}
}
}),
);
}

const simplifiedRead = this.simplifyPaths(readPaths);
Expand Down
Loading