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 4 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
31 changes: 29 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,25 @@ 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: () => {},
Comment thread
ProthamD marked this conversation as resolved.
Outdated
messageId: userMessageTimestamp,
signal,
depth: depth + 1,
});
if (nestedResult.processedQuery) {
parts.push(...nestedResult.processedQuery);
} else {
parts.push({ text: fileActualContent });
}
} else {
parts.push({ text: fileActualContent });
}
} else {
parts.push({ text: part });
}
Expand Down Expand Up @@ -667,7 +689,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 +737,7 @@ 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
68 changes: 65 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,7 +8,6 @@ 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';
Expand Down Expand Up @@ -346,11 +345,74 @@ 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);
let fh;
try {
fh = await fs.promises.open(filePath, 'r');
const stats = await fh.stat();
if (stats.isDirectory()) return false;
const fileSize = stats.size;
if (fileSize === 0) return false; // empty is not binary

// Sample up to 4KB from the head
const sampleSize = Math.min(4096, fileSize);
const buf = Buffer.alloc(sampleSize);
const { bytesRead } = await fh.read(buf, 0, sampleSize, 0);
if (bytesRead === 0) return false;

// BOM → text (avoid false positives for UTF‑16/32 with nulls)
const bom = detectBOM(buf.subarray(0, Math.min(4, bytesRead)));
if (bom) return false;

let nonPrintableCount = 0;
let i = 0;
while (i < bytesRead) {
const byte = buf[i];
if (byte === 0) return true; // null byte → strong binary signal
if (byte < 9 || (byte > 13 && byte < 32)) {
nonPrintableCount++;
i++;
} else if (byte >= 0x80) {
// Determine expected UTF-8 sequence length from the lead byte
let seqLen = 0;
if (byte >= 0xf0 && byte <= 0xf7) seqLen = 4;
else if (byte >= 0xe0 && byte <= 0xef) seqLen = 3;
else if (byte >= 0xc2 && byte <= 0xdf) seqLen = 2;
// 0x80–0xBF are continuation bytes without a leading byte → invalid
// 0xC0–0xC1 are overlong encodings → invalid
// 0xF8–0xFF are invalid UTF-8

if (seqLen > 0 && i + seqLen <= bytesRead) {
// Verify continuation bytes (0x80–0xBF)
let valid = true;
for (let j = 1; j < seqLen; j++) {
if ((buf[i + j] & 0xc0) !== 0x80) {
valid = false;
break;
}
}
if (valid) {
i += seqLen; // skip valid multi-byte sequence
} else {
nonPrintableCount++;
i++;
}
} else {
// lone continuation byte, partial sequence, or invalid lead byte
nonPrintableCount++;
i++;
}
} else {
i++;
}
}
// If >30% non-printable characters, consider it binary
return nonPrintableCount / bytesRead > 0.3;
} finally {
if (fh) await fh.close();
}

@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.

security-high high

Re-implementing the binary detection heuristic adds significant complexity and risk of regressions. A more maintainable approach is to use the established isbinaryfile library after verifying that the path is not a directory. Furthermore, as this is a utility function performing file system operations, it must validate path inputs internally (e.g., using a robust function like resolveToRealPath) to prevent path traversal vulnerabilities.

    const stats = await fsPromises.stat(filePath);
    if (stats.isDirectory()) return false;
    return await isBinaryFileCheck(filePath);
References
  1. Ensure consistent path resolution by using a single, robust function (e.g., resolveToRealPath) for all related path validations.
  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.

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