fix(files): pass directory into context panel binary open guard
Scope context-file open validation to the active project directory so binary opens resolve against the correct workspace root. Co-authored-by: Serhii Dziupin <makeittech@users.noreply.github.com>
This commit is contained in:
co-authored by
Serhii Dziupin
parent
4f30a00814
commit
d35cf957ac
@@ -868,7 +868,7 @@ export const SidebarFilesTree: React.FC = () => {
|
|||||||
const handleOpenFile = React.useCallback(async (node: FileNode) => {
|
const handleOpenFile = React.useCallback(async (node: FileNode) => {
|
||||||
if (!root) return;
|
if (!root) return;
|
||||||
|
|
||||||
const openValidation = await validateContextFileOpen(files, node.path);
|
const openValidation = await validateContextFileOpen(files, node.path, { directory: root });
|
||||||
if (!openValidation.ok) {
|
if (!openValidation.ok) {
|
||||||
toast.error(getContextFileOpenFailureMessage(openValidation.reason));
|
toast.error(getContextFileOpenFailureMessage(openValidation.reason));
|
||||||
return;
|
return;
|
||||||
|
|||||||
@@ -455,7 +455,7 @@ export const CommandPalette: React.FC = () => {
|
|||||||
const handleOpenFile = React.useCallback(
|
const handleOpenFile = React.useCallback(
|
||||||
async (filePath: string) => {
|
async (filePath: string) => {
|
||||||
if (!currentRoot) return;
|
if (!currentRoot) return;
|
||||||
const validation = await validateContextFileOpen(filesApi, filePath);
|
const validation = await validateContextFileOpen(filesApi, filePath, { directory: currentRoot });
|
||||||
if (!validation.ok) {
|
if (!validation.ok) {
|
||||||
toast.error(getContextFileOpenFailureMessage(validation.reason));
|
toast.error(getContextFileOpenFailureMessage(validation.reason));
|
||||||
return;
|
return;
|
||||||
|
|||||||
@@ -1537,7 +1537,7 @@ export const DiffView: React.FC<DiffViewProps> = ({
|
|||||||
: getFirstChangedModifiedLine(diffForNavigation.original, diffForNavigation.modified));
|
: getFirstChangedModifiedLine(diffForNavigation.original, diffForNavigation.modified));
|
||||||
|
|
||||||
const absolutePath = toAbsolutePath(effectiveDirectory, filePath);
|
const absolutePath = toAbsolutePath(effectiveDirectory, filePath);
|
||||||
const openValidation = await validateContextFileOpen(files, absolutePath);
|
const openValidation = await validateContextFileOpen(files, absolutePath, { directory: effectiveDirectory });
|
||||||
if (!openValidation.ok) {
|
if (!openValidation.ok) {
|
||||||
toast.error(getContextFileOpenFailureMessage(openValidation.reason));
|
toast.error(getContextFileOpenFailureMessage(openValidation.reason));
|
||||||
return;
|
return;
|
||||||
|
|||||||
@@ -3,34 +3,35 @@ import { describe, expect, test } from 'bun:test';
|
|||||||
import type { FilesAPI } from '@/lib/api/types';
|
import type { FilesAPI } from '@/lib/api/types';
|
||||||
import { validateContextFileOpen } from './contextFileOpenGuard';
|
import { validateContextFileOpen } from './contextFileOpenGuard';
|
||||||
|
|
||||||
const filesApi = (content: string): FilesAPI => ({
|
const filesApi = (content: string): FilesAPI =>
|
||||||
listDirectory: async () => ({ path: '/', entries: [] }),
|
({
|
||||||
readFile: async () => ({ content, path: '/x' }),
|
listDirectory: async () => ({ directory: '/', entries: [] }),
|
||||||
});
|
readFile: async () => ({ content, path: '/x' }),
|
||||||
|
}) as unknown as FilesAPI;
|
||||||
|
|
||||||
describe('validateContextFileOpen', () => {
|
describe('validateContextFileOpen', () => {
|
||||||
test('allows known binaries through without reading text', async () => {
|
test('allows known binaries through without reading text', async () => {
|
||||||
const files: FilesAPI = {
|
const files = {
|
||||||
listDirectory: async () => ({ path: '/', entries: [] }),
|
listDirectory: async () => ({ directory: '/', entries: [] }),
|
||||||
readFile: async () => {
|
readFile: async () => {
|
||||||
throw new Error('should not read binary as text');
|
throw new Error('should not read binary as text');
|
||||||
},
|
},
|
||||||
};
|
} as unknown as FilesAPI;
|
||||||
|
|
||||||
await expect(validateContextFileOpen(files, '/repo/docs/report.pdf')).resolves.toEqual({ ok: true });
|
expect(await validateContextFileOpen(files, '/repo/docs/report.pdf')).toEqual({ ok: true });
|
||||||
await expect(validateContextFileOpen(files, '/repo/docs/report.docx')).resolves.toEqual({ ok: true });
|
expect(await validateContextFileOpen(files, '/repo/docs/report.docx')).toEqual({ ok: true });
|
||||||
await expect(validateContextFileOpen(files, '/repo/docs/pixel.png')).resolves.toEqual({ ok: true });
|
expect(await validateContextFileOpen(files, '/repo/docs/pixel.png')).toEqual({ ok: true });
|
||||||
await expect(validateContextFileOpen(files, '/repo/bin/archive.zip')).resolves.toEqual({ ok: true });
|
expect(await validateContextFileOpen(files, '/repo/bin/archive.zip')).toEqual({ ok: true });
|
||||||
});
|
});
|
||||||
|
|
||||||
test('rejects text payloads that look binary', async () => {
|
test('rejects text payloads that look binary', async () => {
|
||||||
await expect(validateContextFileOpen(filesApi('%PDF-1.7\nbinary'), '/repo/mystery.bin.bak')).resolves.toEqual({
|
expect(await validateContextFileOpen(filesApi('%PDF-1.7\nbinary'), '/repo/mystery.bin.bak')).toEqual({
|
||||||
ok: false,
|
ok: false,
|
||||||
reason: 'binary',
|
reason: 'binary',
|
||||||
});
|
});
|
||||||
});
|
});
|
||||||
|
|
||||||
test('allows ordinary text files', async () => {
|
test('allows ordinary text files', async () => {
|
||||||
await expect(validateContextFileOpen(filesApi('hello\nworld\n'), '/repo/notes.txt')).resolves.toEqual({ ok: true });
|
expect(await validateContextFileOpen(filesApi('hello\nworld\n'), '/repo/notes.txt')).toEqual({ ok: true });
|
||||||
});
|
});
|
||||||
});
|
});
|
||||||
|
|||||||
@@ -2,11 +2,11 @@ import type { FilesAPI } from '@/lib/api/types';
|
|||||||
import { MAX_OPEN_FILE_LINES, countLinesWithLimit } from '@/lib/fileOpenLimits';
|
import { MAX_OPEN_FILE_LINES, countLinesWithLimit } from '@/lib/fileOpenLimits';
|
||||||
import { getCurrentIntlLocale } from '@/lib/i18n';
|
import { getCurrentIntlLocale } from '@/lib/i18n';
|
||||||
import { formatMessage, useI18nStore } from '@/lib/i18n/store';
|
import { formatMessage, useI18nStore } from '@/lib/i18n/store';
|
||||||
|
import { runtimeFetch } from '@/lib/runtime-fetch';
|
||||||
|
import { isBinaryFile, isImageFile, isPdfFile, looksLikeBinaryText } from '@/lib/toolHelpers';
|
||||||
|
|
||||||
const t = (key: Parameters<typeof formatMessage>[1], params?: Parameters<typeof formatMessage>[2]) =>
|
const t = (key: Parameters<typeof formatMessage>[1], params?: Parameters<typeof formatMessage>[2]) =>
|
||||||
formatMessage(useI18nStore.getState().dictionary, key, params);
|
formatMessage(useI18nStore.getState().dictionary, key, params);
|
||||||
import { runtimeFetch } from '@/lib/runtime-fetch';
|
|
||||||
import { isBinaryFile, isImageFile, isPdfFile, looksLikeBinaryText } from '@/lib/toolHelpers';
|
|
||||||
|
|
||||||
export type ContextFileOpenFailureReason = 'too-large' | 'missing' | 'unreadable' | 'binary';
|
export type ContextFileOpenFailureReason = 'too-large' | 'missing' | 'unreadable' | 'binary';
|
||||||
|
|
||||||
@@ -14,6 +14,10 @@ export type ContextFileOpenValidationResult =
|
|||||||
| { ok: true }
|
| { ok: true }
|
||||||
| { ok: false; reason: ContextFileOpenFailureReason };
|
| { ok: false; reason: ContextFileOpenFailureReason };
|
||||||
|
|
||||||
|
export type ContextFileOpenOptions = {
|
||||||
|
directory?: string;
|
||||||
|
};
|
||||||
|
|
||||||
const classifyReadError = (error: unknown): ContextFileOpenFailureReason => {
|
const classifyReadError = (error: unknown): ContextFileOpenFailureReason => {
|
||||||
const message = error instanceof Error ? error.message : String(error ?? '');
|
const message = error instanceof Error ? error.message : String(error ?? '');
|
||||||
const normalized = message.toLowerCase();
|
const normalized = message.toLowerCase();
|
||||||
@@ -31,16 +35,27 @@ const classifyReadError = (error: unknown): ContextFileOpenFailureReason => {
|
|||||||
return 'unreadable';
|
return 'unreadable';
|
||||||
};
|
};
|
||||||
|
|
||||||
const readFileContent = async (files: FilesAPI, path: string): Promise<string> => {
|
const readFileContent = async (
|
||||||
|
files: FilesAPI,
|
||||||
|
path: string,
|
||||||
|
options?: ContextFileOpenOptions,
|
||||||
|
): Promise<string> => {
|
||||||
if (files.readFile) {
|
if (files.readFile) {
|
||||||
const result = await files.readFile(path, { optional: true });
|
const result = await files.readFile(path, {
|
||||||
|
optional: true,
|
||||||
|
directory: options?.directory,
|
||||||
|
});
|
||||||
return result.content ?? '';
|
return result.content ?? '';
|
||||||
}
|
}
|
||||||
|
|
||||||
const params = new URLSearchParams({ path, optional: 'true' });
|
const params = new URLSearchParams({ path, optional: 'true' });
|
||||||
|
if (options?.directory) {
|
||||||
|
params.set('directory', options.directory);
|
||||||
|
}
|
||||||
const response = await runtimeFetch(`/api/fs/read?${params.toString()}`, {
|
const response = await runtimeFetch(`/api/fs/read?${params.toString()}`, {
|
||||||
// Avoid conditional requests (304 + empty body).
|
// Avoid conditional requests (304 + empty body).
|
||||||
cache: 'no-store',
|
cache: 'no-store',
|
||||||
|
headers: options?.directory ? { 'x-opencode-directory': options.directory } : undefined,
|
||||||
});
|
});
|
||||||
if (!response.ok) {
|
if (!response.ok) {
|
||||||
const errorPayload = await response.json().catch(() => ({ error: response.statusText }));
|
const errorPayload = await response.json().catch(() => ({ error: response.statusText }));
|
||||||
@@ -55,13 +70,17 @@ const readFileContent = async (files: FilesAPI, path: string): Promise<string> =
|
|||||||
* Previewable/non-text binaries are allowed through so FilesView can show image/PDF
|
* Previewable/non-text binaries are allowed through so FilesView can show image/PDF
|
||||||
* preview or the cannot-preview empty state — never by decoding them as editable text here.
|
* preview or the cannot-preview empty state — never by decoding them as editable text here.
|
||||||
*/
|
*/
|
||||||
export const validateContextFileOpen = async (files: FilesAPI, path: string): Promise<ContextFileOpenValidationResult> => {
|
export const validateContextFileOpen = async (
|
||||||
|
files: FilesAPI,
|
||||||
|
path: string,
|
||||||
|
options?: ContextFileOpenOptions,
|
||||||
|
): Promise<ContextFileOpenValidationResult> => {
|
||||||
if (isBinaryFile(path) || isPdfFile(path) || isImageFile(path)) {
|
if (isBinaryFile(path) || isPdfFile(path) || isImageFile(path)) {
|
||||||
return { ok: true };
|
return { ok: true };
|
||||||
}
|
}
|
||||||
|
|
||||||
try {
|
try {
|
||||||
const content = await readFileContent(files, path);
|
const content = await readFileContent(files, path, options);
|
||||||
if (looksLikeBinaryText(content)) {
|
if (looksLikeBinaryText(content)) {
|
||||||
return { ok: false, reason: 'binary' };
|
return { ok: false, reason: 'binary' };
|
||||||
}
|
}
|
||||||
|
|||||||
Reference in New Issue
Block a user