Skip to content
Closed
3 changes: 3 additions & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -64,3 +64,6 @@ gemini-debug.log
evals/logs/

temp_agents/
PR.md
changes.md
pr_body.md
31 changes: 29 additions & 2 deletions packages/cli/src/ui/hooks/atCommandProcessor.ts
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

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
Original file line number Diff line number Diff line change
Expand Up @@ -194,8 +194,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
1 change: 1 addition & 0 deletions packages/core/src/utils/fileUtils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -287,6 +287,7 @@ export async function isBinaryFile(filePath: string): Promise<boolean> {
try {
fh = await fs.promises.open(filePath, 'r');
const stats = await fh.stat();
if (stats.isDirectory()) return false;

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.

const fileSize = stats.size;
if (fileSize === 0) return false; // empty is not binary

Expand Down
Loading