Skip to content

Fix EISDIR warnings and Max Stack Size errors for issue #21527 #25444

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

Closed
ProthamD wants to merge 9 commits into from
Closed

Fix EISDIR warnings and Max Stack Size errors for issue #21527 #25444

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
657e630
Fix EISDIR warnings and Infinite Recursion on atCommandProcessor #21527
ProthamD Apr 15, 2026
d3481c9
Fix EISDIR read errors and Max Stack Size infinite recursion for issu…
ProthamD Apr 15, 2026
ea51c67
Fix incomplete recursion depth for atCommandProcessor by parsing @ ta…
ProthamD Apr 15, 2026
96a80eb
fix(core): correct isBinaryFile to detect invalid UTF-8 byte sequence…
ProthamD Apr 15, 2026
382e95b
Merge branch 'main' into fix/issue-21527
ProthamD Apr 22, 2026
f22a6de
Update packages/cli/src/ui/hooks/atCommandProcessor.ts
ProthamD Apr 22, 2026
a66b75e
fix(core): restore isbinaryfile usage, validate paths, and fix observ…
ProthamD Apr 23, 2026
d798185
Merge branch 'main' into fix/issue-21527
ProthamD Apr 23, 2026
8ca46e3
fix(cli): handle non-array PartListUnion in nested @command expansion
ProthamD May 12, 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
46 changes: 44 additions & 2 deletions packages/cli/src/ui/hooks/atCommandProcessor.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,7 @@ interface HandleAtCommandParams {
messageId: number;
signal: AbortSignal;
escapePastedAtSymbols?: boolean;
depth?: number;
}

interface HandleAtCommandResult {
Expand Down Expand Up @@ -504,6 +505,9 @@ async function readLocalFiles(
config: Config,
signal: AbortSignal,
userMessageTimestamp: number,
depth: number,
addItem: UseHistoryManagerReturn['addItem'],
onDebugMessage: (message: string) => void,
): Promise<{
parts: PartUnion[];
display?: IndividualToolCallDisplay;
Expand Down Expand Up @@ -580,7 +584,32 @@ async function readLocalFiles(
parts.push({
text: `\nContent from @${displayPath}:\n`,
});
parts.push({ text: fileActualContent });

if (
depth < 2 &&
typeof fileActualContent === 'string' &&
fileActualContent.includes('@')
) {
const nestedResult = await handleAtCommand({
query: fileActualContent,
config,
addItem: () => 0, // Mock addItem to prevent history pollution.
onDebugMessage,
messageId: userMessageTimestamp,
signal,
depth: depth + 1,
});
if (nestedResult.processedQuery) {
const queryParts = Array.isArray(nestedResult.processedQuery)
? nestedResult.processedQuery
: [nestedResult.processedQuery];
parts.push(...queryParts);
} else {
parts.push({ text: fileActualContent });
}
} else {
parts.push({ text: fileActualContent });
}
} else {
parts.push({ text: part });
}
Expand Down Expand Up @@ -667,7 +696,12 @@ export async function handleAtCommand({
messageId: userMessageTimestamp,
signal,
escapePastedAtSymbols = false,
depth = 0,
}: HandleAtCommandParams): Promise<HandleAtCommandResult> {
if (depth > 2) {
return { processedQuery: [{ text: query }] };
}
Comment on lines +701 to +703

@gemini-code-assist gemini-code-assist Bot Apr 15, 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.

high

The depth parameter and the check if (depth > 2) have been added to prevent a RangeError: Maximum call stack size exceeded, but the implementation appears incomplete. There is no recursive call to handleAtCommand within this function, nor is the depth parameter incremented and passed to any other function that might call it back. Consequently, depth will always remain at its default value of 0, and the check will never trigger. If the recursion is intended to happen within this function (e.g., to expand nested @ commands in the content of read files), the recursive call is missing. If the recursion happens in the caller, the caller must be updated to pass an incremented depth value.

References
  1. A recursive error/reconnect handler is acceptable as long as it includes a mechanism to limit the maximum number of retry attempts to prevent infinite loops.


const commandParts = parseAllAtCommands(query, escapePastedAtSymbols);

const { agentParts, resourceParts, fileParts } = categorizeAtCommands(
Expand Down Expand Up @@ -710,7 +744,15 @@ export async function handleAtCommand({

const [mcpResult, fileResult] = await Promise.all([
readMcpResources(resourceParts, config, signal),
readLocalFiles(resolvedFiles, config, signal, userMessageTimestamp),
readLocalFiles(
resolvedFiles,
config,
signal,
userMessageTimestamp,
depth,
addItem,
onDebugMessage,
),
]);

const hasContent = mcpResult.parts.length > 0 || fileResult.parts.length > 0;
Expand Down
4 changes: 2 additions & 2 deletions packages/core/src/tools/read-many-files.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -199,8 +199,8 @@ ${finalExclusionPatternsForDescription
const fullPath = path.join(dir, normalizedP);
let exists = false;
try {
await fsPromises.access(fullPath);
exists = true;
const st = await fsPromises.stat(fullPath);
exists = st.isFile();
} catch {
exists = false;
}
Expand Down
9 changes: 6 additions & 3 deletions packages/core/src/utils/fileUtils.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,13 +8,14 @@ import fs from 'node:fs';
import fsPromises from 'node:fs/promises';
import path from 'node:path';
import type { PartUnion } from '@google/genai';
import { isBinaryFile as isBinaryFileCheck } from 'isbinaryfile';
import mime from 'mime/lite';

@gemini-code-assist gemini-code-assist Bot Apr 22, 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.

high

The isbinaryfile package should be retained as it provides a battle-tested heuristic for binary detection. The EISDIR issue can be addressed more simply and safely by adding a directory check before calling the library function, rather than re-implementing the entire heuristic manually.

Suggested change
import mime from 'mime/lite';
import { isBinaryFile as isBinaryFileCheck } from 'isbinaryfile';

import type { FileSystemService } from '../services/fileSystemService.js';
import { ToolErrorType } from '../tools/tool-error.js';
import { BINARY_EXTENSIONS } from './ignorePatterns.js';
import { createRequire as createModuleRequire } from 'node:module';
import { debugLogger } from './debugLogger.js';
import { resolveToRealPath } from './paths.js';
import { isBinaryFile as isBinaryFileCheck } from 'isbinaryfile';
import {
DEFAULT_MAX_LINES_TEXT_FILE,
MAX_LINE_LENGTH_TEXT_FILE,
Expand Down Expand Up @@ -346,11 +347,13 @@ export async function isEmpty(filePath: string): Promise<boolean> {

/**
* Heuristic: determine if a file is likely binary.
* Delegates to the `isbinaryfile` package for UTF-8-aware detection.
*/
export async function isBinaryFile(filePath: string): Promise<boolean> {
try {
return await isBinaryFileCheck(filePath);
const realPath = resolveToRealPath(filePath);
const stats = await fsPromises.stat(realPath);
if (stats.isDirectory()) return false;

@gemini-code-assist gemini-code-assist Bot May 12, 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

A Denial of Service (DoS) vulnerability exists in the isBinaryFile function. If the resolved path points to a special file (e.g., named pipe, device file) and is not verified as a regular file before being passed to isBinaryFileCheck, the subsequent read operation could block indefinitely. Additionally, ensure you use asynchronous file system operations (e.g., fs.promises.realpath) instead of synchronous ones to avoid blocking the event loop. This utility should validate its path inputs internally and continue to use resolveToRealPath for consistent path resolution.

Suggested change
if (stats.isDirectory()) return false;
if (!stats.isFile()) return false;
References
  1. Use asynchronous file system operations (e.g., fs.promises.readFile) instead of synchronous ones (e.g., fs.readFileSync) to avoid blocking the event loop.
  2. Utility functions that perform file system operations should validate their path inputs internally to prevent path traversal vulnerabilities, rather than relying solely on callers to perform validation.
  3. Ensure consistent path resolution by using a single, robust function (e.g., resolveToRealPath) for all related path validations, including internal validations in components like WorkspaceContext.

return await isBinaryFileCheck(realPath);
} catch (error) {
debugLogger.warn(
`Failed to check if file is binary: ${filePath}`,
Expand Down
Loading