Skip to content
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
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
56 changes: 56 additions & 0 deletions packages/cli/src/ui/hooks/atCommandProcessor.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ import {
type Mock,
} from 'vitest';
import {
checkPermissions,
handleAtCommand,
escapeAtSymbols,
unescapeLiteralAt,
Expand All @@ -35,6 +36,7 @@ import {
import * as core from '@google/gemini-cli-core';
import * as os from 'node:os';
import type { UseHistoryManagerReturn } from './useHistoryManager.js';
import * as fs from 'node:fs';
import * as fsPromises from 'node:fs/promises';
import * as path from 'node:path';

Expand Down Expand Up @@ -1540,3 +1542,57 @@ describe('unescapeLiteralAt', () => {
expect(unescapeLiteralAt(escapeAtSymbols(input))).toBe(input);
});
});

describe('checkPermissions', () => {
let testRootDir: string;
let mockConfig: Config;

beforeEach(async () => {
vi.restoreAllMocks();
testRootDir = await fsPromises.mkdtemp(
path.join(os.tmpdir(), 'check-permissions-test-'),
);

mockConfig = {
getTargetDir: () => testRootDir,
getAgentRegistry: () => ({
getDefinition: () => undefined,
}),
getResourceRegistry: () => ({
findResourceByUri: () => undefined,
getAllResources: () => [],
}),
validatePathAccess: () => null,
} as unknown as Config;
});

afterEach(async () => {
await fsPromises.rm(testRootDir, { recursive: true, force: true });
});

// Regression for #22029 (and related #25910 / #25923): when a user pastes
// a JSON-like blob after an @, the @-command regex greedily captures it.
// The resolved string is longer than NAME_MAX, so fs.realpathSync throws
// ENAMETOOLONG. Previously this bubbled up as an unhandled rejection and
// crashed the CLI.
it('skips @-mentions whose path is too long to be a real filesystem entry', async () => {
const longSegment = 'a'.repeat(8192);
const query = `@${longSegment}`;
await expect(checkPermissions(query, mockConfig)).resolves.toEqual([]);
});

it('still surfaces real @-mentioned files when a sibling @-mention is unresolvable', async () => {
// A real file alongside a giant pasted-blob mention: the bogus mention
// should be skipped, the real one should still appear in the result.
const realFile = path.join(testRootDir, 'real.txt');
await fsPromises.writeFile(realFile, 'hello');
const resolvedRealFile = fs.realpathSync(realFile);
mockConfig.validatePathAccess = () =>
'permission required' as unknown as null;
const longSegment = 'b'.repeat(8192);
const query = `@real.txt and @${longSegment}`;
await expect(checkPermissions(query, mockConfig)).resolves.toEqual([
resolvedRealFile,
]);
});
});
17 changes: 14 additions & 3 deletions packages/cli/src/ui/hooks/atCommandProcessor.ts
Original file line number Diff line number Diff line change
Expand Up @@ -188,9 +188,20 @@ export async function checkPermissions(
const pathName = part.content.substring(1);
if (!pathName) continue;

const resolvedPathName = resolveToRealPath(
path.resolve(config.getTargetDir(), pathName),
);
let resolvedPathName: string;
try {
resolvedPathName = resolveToRealPath(
path.resolve(config.getTargetDir(), pathName),
);
} catch {
// The @-command regex is greedy and can capture pasted content (e.g.
Comment thread
scidomino marked this conversation as resolved.
Outdated

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Remove this giant comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Done in 3c21e10 — trimmed to the suggested one-liner. Thanks!

// a JSON blob) that is not actually a path. Resolving such content
// through fs.realpathSync throws ENAMETOOLONG / EINVAL, which would
// otherwise propagate as an unhandled rejection and crash the CLI.
// Skip the entry — downstream rendering will surface a clearer error
// if the user really meant to reference a file.
continue;
}

if (config.validatePathAccess(resolvedPathName, 'read')) {
if (await fileExists(resolvedPathName)) {
Expand Down