From 4f51abddf41dfe45198f780107eae199f50d5651 Mon Sep 17 00:00:00 2001 From: Bohdan Triapitsyn Date: Tue, 28 Apr 2026 14:06:25 +0300 Subject: [PATCH] fix: improve external file and path handling Open external context files read-only Preserve leading-dot paths in UI Keep workspace write operations guarded --- .../src/components/chat/ChangedFilesList.tsx | 2 +- .../chat/message/parts/ProgressiveGroup.tsx | 1 + .../chat/message/parts/ToolPart.tsx | 5 +- packages/ui/src/components/views/DiffView.tsx | 6 +-- .../ui/src/components/views/FilesView.tsx | 46 +++++++++++++------ .../ui/src/components/views/git/ChangeRow.tsx | 4 +- .../ui/src/contexts/RuntimeAPIProvider.tsx | 19 ++++---- packages/ui/src/lib/api/types.ts | 10 ++-- packages/ui/src/lib/contextFileOpenGuard.ts | 5 +- packages/web/server/lib/fs/routes.js | 27 +++++++++-- packages/web/src/api/files.ts | 16 +++++-- 11 files changed, 98 insertions(+), 43 deletions(-) diff --git a/packages/ui/src/components/chat/ChangedFilesList.tsx b/packages/ui/src/components/chat/ChangedFilesList.tsx index bc8f670a..9527fea1 100644 --- a/packages/ui/src/components/chat/ChangedFilesList.tsx +++ b/packages/ui/src/components/chat/ChangedFilesList.tsx @@ -37,7 +37,7 @@ export const ChangedFilesList: React.FC = ({ files, curre <> {dirPart} diff --git a/packages/ui/src/components/chat/message/parts/ProgressiveGroup.tsx b/packages/ui/src/components/chat/message/parts/ProgressiveGroup.tsx index 806aeee1..26aadcae 100644 --- a/packages/ui/src/components/chat/message/parts/ProgressiveGroup.tsx +++ b/packages/ui/src/components/chat/message/parts/ProgressiveGroup.tsx @@ -271,6 +271,7 @@ const renderReadFilePath = (displayPath: string) => { color: 'var(--tools-description)', direction: 'rtl', textAlign: 'left', + unicodeBidi: 'plaintext', }} > {displayDir} diff --git a/packages/ui/src/components/chat/message/parts/ToolPart.tsx b/packages/ui/src/components/chat/message/parts/ToolPart.tsx index 2db7172a..8e784596 100644 --- a/packages/ui/src/components/chat/message/parts/ToolPart.tsx +++ b/packages/ui/src/components/chat/message/parts/ToolPart.tsx @@ -1299,7 +1299,7 @@ const renderPathLikeGitChanges = (path: string, grow = true) => { return ( {path} @@ -1315,7 +1315,7 @@ const renderPathLikeGitChanges = (path: string, grow = true) => { return ( {hasAbsoluteRoot ? / : null} - + {displayDir} @@ -1360,6 +1360,7 @@ const renderAnimatedPathWithIcon = (path: string, _animate = true, grow = true, color: 'var(--tools-description)', direction: 'rtl', textAlign: 'left', + unicodeBidi: 'plaintext', }} > {displayDir} diff --git a/packages/ui/src/components/views/DiffView.tsx b/packages/ui/src/components/views/DiffView.tsx index 796cf9d7..a606f571 100644 --- a/packages/ui/src/components/views/DiffView.tsx +++ b/packages/ui/src/components/views/DiffView.tsx @@ -362,7 +362,7 @@ const FileList = React.memo(({ {file.path} @@ -813,7 +813,7 @@ const MultiFileDiffEntry = React.memo(({ return ( {file.path} @@ -827,7 +827,7 @@ const MultiFileDiffEntry = React.memo(({ {dir} diff --git a/packages/ui/src/components/views/FilesView.tsx b/packages/ui/src/components/views/FilesView.tsx index b1888398..f435458a 100644 --- a/packages/ui/src/components/views/FilesView.tsx +++ b/packages/ui/src/components/views/FilesView.tsx @@ -597,6 +597,12 @@ export const FilesView: React.FC = ({ mode = 'full' }) => { const openFiles = React.useMemo(() => openPaths.map(toFileNode), [openPaths, toFileNode]); const effectiveSelectedPath = React.useMemo(() => selectedPath ?? openPaths[0] ?? null, [openPaths, selectedPath]); const selectedFile = React.useMemo(() => (effectiveSelectedPath ? toFileNode(effectiveSelectedPath) : null), [effectiveSelectedPath, toFileNode]); + const selectedFilePath = selectedFile?.path ?? ''; + const selectedFileIsOutsideWorkspace = Boolean(root && selectedFilePath && !isPathWithinRoot(selectedFilePath, root)); + const selectedFileReadOptions = React.useMemo( + () => ({ allowOutsideWorkspace: mode === 'editor-only' && selectedFileIsOutsideWorkspace }), + [mode, selectedFileIsOutsideWorkspace], + ); // Editor tabs horizontal scroll fades const editorTabsScrollRef = React.useRef(null); @@ -1217,13 +1223,17 @@ export const FilesView: React.FC = ({ mode = 'full' }) => { }; }, [currentDirectory, debouncedSearchQuery, searchFiles, showHidden, showGitignored]); - const readFile = React.useCallback(async (path: string): Promise => { + const readFile = React.useCallback(async (path: string, options?: { allowOutsideWorkspace?: boolean }): Promise => { if (files.readFile) { - const result = await files.readFile(path); + const result = await files.readFile(path, options); return result.content ?? ''; } - const response = await fetch(`/api/fs/read?path=${encodeURIComponent(path)}`); + const params = new URLSearchParams({ path }); + if (options?.allowOutsideWorkspace) { + params.set('allowOutsideWorkspace', 'true'); + } + const response = await fetch(`/api/fs/read?${params.toString()}`); if (!response.ok) { const error = await response.json().catch(() => ({ error: response.statusText })); throw new Error((error as { error?: string }).error || t('filesView.error.readFileFailed')); @@ -1231,9 +1241,9 @@ export const FilesView: React.FC = ({ mode = 'full' }) => { return response.text(); }, [files, t]); - const readFileStat = React.useCallback(async (path: string): Promise => { + const readFileStat = React.useCallback(async (path: string, options?: { allowOutsideWorkspace?: boolean }): Promise => { if (files.statFile) { - const result = await files.statFile(path); + const result = await files.statFile(path, options); return { path: result.path, size: result.size, @@ -1402,14 +1412,16 @@ export const FilesView: React.FC = ({ mode = 'full' }) => { setFileLoading(true); - await readFile(node.path) + const readOptions = { allowOutsideWorkspace: mode === 'editor-only' && Boolean(root) && !isPathWithinRoot(node.path, root) }; + + await readFile(node.path, readOptions) .then((content) => { setFileContent(content); setDraftContent(content.length > MAX_VIEW_CHARS ? `${content.slice(0, MAX_VIEW_CHARS)}\n\n… truncated …` : content); setLoadedFilePath(node.path); - void readFileStat(node.path) + void readFileStat(node.path, readOptions) .then((stat) => { if (stat) { lastLoadedFileStatRef.current = stat; @@ -1455,7 +1467,7 @@ export const FilesView: React.FC = ({ mode = 'full' }) => { .finally(() => { setFileLoading(false); }); - }, [expandPaths, isMobile, loadDirectory, readFile, readFileStat, root, runtime.isDesktop, searchQuery, setSelectedPath, t]); + }, [expandPaths, isMobile, loadDirectory, mode, readFile, readFileStat, root, runtime.isDesktop, searchQuery, setSelectedPath, t]); const ensurePathVisible = React.useCallback(async (targetPath: string, includeTarget: boolean) => { if (!root) { @@ -1549,7 +1561,7 @@ export const FilesView: React.FC = ({ mode = 'full' }) => { return; } - void readFileStat(selectedFile.path) + void readFileStat(selectedFile.path, selectedFileReadOptions) .then((latestStat) => { if (cancelled || !latestStat) { return; @@ -1585,7 +1597,7 @@ export const FilesView: React.FC = ({ mode = 'full' }) => { cancelled = true; window.clearInterval(interval); }; - }, [loadedFilePath, readFileStat, selectedFile?.path]); + }, [loadedFilePath, readFileStat, selectedFile?.path, selectedFileReadOptions]); const discardAndContinue = React.useCallback(() => { const nextFile = pendingSelectFileRef.current; @@ -1820,7 +1832,6 @@ export const FilesView: React.FC = ({ mode = 'full' }) => { const isSelectedImage = Boolean(selectedFile?.path && isImageFile(selectedFile.path)); const isSelectedSvg = Boolean(selectedFile?.path && selectedFile.path.toLowerCase().endsWith('.svg')); - const selectedFilePath = selectedFile?.path ?? ''; const pendingNavigationTargetPath = React.useMemo( () => normalizePath(pendingFileNavigation?.path ?? ''), [pendingFileNavigation?.path], @@ -1841,7 +1852,7 @@ export const FilesView: React.FC = ({ mode = 'full' }) => { const canCopy = Boolean(selectedFile && (!isSelectedImage || isSelectedSvg) && fileContent.length > 0); const canCopyPath = Boolean(selectedFile && displaySelectedPath.length > 0); - const canEdit = Boolean(selectedFile && !isSelectedImage && files.writeFile && fileContent.length <= MAX_VIEW_CHARS); + const canEdit = Boolean(selectedFile && !selectedFileIsOutsideWorkspace && !isSelectedImage && files.writeFile && fileContent.length <= MAX_VIEW_CHARS); const isMarkdown = Boolean(selectedFile?.path && isMarkdownFile(selectedFile.path)); const isJson = Boolean(selectedFile?.path && isJsonFile(selectedFile.path)); const isHtml = Boolean(selectedFile?.path && isHtmlFile(selectedFile.path)); @@ -2301,7 +2312,10 @@ export const FilesView: React.FC = ({ mode = 'full' }) => { : desktopImageSrc) : (isSelectedSvg ? `data:${getImageMimeType(selectedFile.path)};utf8,${encodeURIComponent(fileContent)}` - : `/api/fs/raw?path=${encodeURIComponent(selectedFile.path)}`)) + : `/api/fs/raw?${new URLSearchParams({ + path: selectedFile.path, + ...(selectedFileReadOptions.allowOutsideWorkspace ? { allowOutsideWorkspace: 'true' } : {}), + }).toString()}`)) : ''; @@ -2319,7 +2333,7 @@ export const FilesView: React.FC = ({ mode = 'full' }) => { setFileError(null); const srcPromise = files.readFileBinary - ? files.readFileBinary(selectedFile.path).then((result) => result.dataUrl) + ? files.readFileBinary(selectedFile.path, selectedFileReadOptions).then((result) => result.dataUrl) : Promise.resolve(convertFileSrc(selectedFile.path, 'asset')); await srcPromise @@ -2348,7 +2362,7 @@ export const FilesView: React.FC = ({ mode = 'full' }) => { return () => { cancelled = true; }; - }, [files, isSelectedImage, isSelectedSvg, runtime.isDesktop, selectedFile?.path, t]); + }, [files, isSelectedImage, isSelectedSvg, runtime.isDesktop, selectedFile?.path, selectedFileReadOptions, t]); const renderDialogs = () => ( !open && setActiveDialog(null)}> @@ -3004,6 +3018,7 @@ export const FilesView: React.FC = ({ mode = 'full' }) => { = ({ mode = 'full' }) => { { diff --git a/packages/ui/src/components/views/git/ChangeRow.tsx b/packages/ui/src/components/views/git/ChangeRow.tsx index 5f143d56..2bbbcb0f 100644 --- a/packages/ui/src/components/views/git/ChangeRow.tsx +++ b/packages/ui/src/components/views/git/ChangeRow.tsx @@ -124,7 +124,7 @@ export const ChangeRow = React.memo(function ChangeRow({ return ( {file.path} @@ -137,7 +137,7 @@ export const ChangeRow = React.memo(function ChangeRow({ {dir} diff --git a/packages/ui/src/contexts/RuntimeAPIProvider.tsx b/packages/ui/src/contexts/RuntimeAPIProvider.tsx index 01c80fdb..a982498f 100644 --- a/packages/ui/src/contexts/RuntimeAPIProvider.tsx +++ b/packages/ui/src/contexts/RuntimeAPIProvider.tsx @@ -47,14 +47,14 @@ function withContentCache(files: FilesAPI): FilesAPI { return result; }; - const readFreshFile = async (path: string): Promise<{ content: string; path: string }> => { + const readFreshFile = async (path: string, options?: Parameters>[1]): Promise<{ content: string; path: string }> => { // stat → read → stat to avoid TOCTOU: // if the file changes between read and either stat, metadata won't match and we retry. - const statBefore = await files.statFile?.(path).catch(() => null); + const statBefore = await files.statFile?.(path, options).catch(() => null); - const result = await files.readFile!(path); + const result = await files.readFile!(path, options); - const statAfter = await files.statFile?.(path).catch(() => null); + const statAfter = await files.statFile?.(path, options).catch(() => null); // If both stats are available and agree, the read was atomic with respect to file changes. if (statBefore && statAfter && statBefore.isFile && statAfter.isFile) { @@ -62,9 +62,9 @@ function withContentCache(files: FilesAPI): FilesAPI { return syncCacheEntry(path, result, statAfter); } // File changed during read — discard and re-read once. - const retryStatBefore = await files.statFile?.(path).catch(() => null); - const retry = await files.readFile!(path); - const retryStat = await files.statFile?.(path).catch(() => null); + const retryStatBefore = await files.statFile?.(path, options).catch(() => null); + const retry = await files.readFile!(path, options); + const retryStat = await files.statFile?.(path, options).catch(() => null); // Accept retry only if file was stable across the read. if (retryStatBefore && retryStat && retryStatBefore.isFile && retryStat.isFile && retryStatBefore.size === retryStat.size && retryStatBefore.mtimeMs === retryStat.mtimeMs) { @@ -78,7 +78,10 @@ function withContentCache(files: FilesAPI): FilesAPI { }; const cachedReadFile: FilesAPI['readFile'] = files.readFile - ? async (path: string) => { + ? async (path: string, options) => { + if (options?.allowOutsideWorkspace) { + return readFreshFile(path, options); + } const hit = cache.get(path); if (hit) { // Validate cached entry is still fresh diff --git a/packages/ui/src/lib/api/types.ts b/packages/ui/src/lib/api/types.ts index a7fe91b6..2a6b6e96 100644 --- a/packages/ui/src/lib/api/types.ts +++ b/packages/ui/src/lib/api/types.ts @@ -509,13 +509,17 @@ export interface ListDirectoryOptions { respectGitignore?: boolean; } +export interface FileReadOptions { + allowOutsideWorkspace?: boolean; +} + export interface FilesAPI { listDirectory(path: string, options?: ListDirectoryOptions): Promise; search(payload: FileSearchQuery): Promise; createDirectory(path: string): Promise<{ success: boolean; path: string }>; - statFile?(path: string): Promise<{ path: string; isFile: boolean; size: number; mtimeMs?: number }>; - readFile?(path: string): Promise<{ content: string; path: string }>; - readFileBinary?(path: string): Promise<{ dataUrl: string; path: string }>; + statFile?(path: string, options?: FileReadOptions): Promise<{ path: string; isFile: boolean; size: number; mtimeMs?: number }>; + readFile?(path: string, options?: FileReadOptions): Promise<{ content: string; path: string }>; + readFileBinary?(path: string, options?: FileReadOptions): Promise<{ dataUrl: string; path: string }>; writeFile?(path: string, content: string): Promise<{ success: boolean; path: string }>; delete?(path: string): Promise<{ success: boolean }>; rename?(oldPath: string, newPath: string): Promise<{ success: boolean; path: string }>; diff --git a/packages/ui/src/lib/contextFileOpenGuard.ts b/packages/ui/src/lib/contextFileOpenGuard.ts index 315b6791..0775dffd 100644 --- a/packages/ui/src/lib/contextFileOpenGuard.ts +++ b/packages/ui/src/lib/contextFileOpenGuard.ts @@ -26,11 +26,12 @@ const classifyReadError = (error: unknown): ContextFileOpenFailureReason => { const readFileContent = async (files: FilesAPI, path: string): Promise => { if (files.readFile) { - const result = await files.readFile(path); + const result = await files.readFile(path, { allowOutsideWorkspace: true }); return result.content ?? ''; } - const response = await fetch(`/api/fs/read?path=${encodeURIComponent(path)}`); + const params = new URLSearchParams({ path, allowOutsideWorkspace: 'true' }); + const response = await fetch(`/api/fs/read?${params.toString()}`); if (!response.ok) { const errorPayload = await response.json().catch(() => ({ error: response.statusText })); throw new Error((errorPayload as { error?: string }).error || 'Failed to read file'); diff --git a/packages/web/server/lib/fs/routes.js b/packages/web/server/lib/fs/routes.js index 3e1f08de..2d704ae7 100644 --- a/packages/web/server/lib/fs/routes.js +++ b/packages/web/server/lib/fs/routes.js @@ -95,6 +95,27 @@ const resolveWorkspacePathFromContext = async ({ req, targetPath, resolveProject }); }; +const resolveReadPathFromContext = async ({ req, targetPath, resolveProjectDirectory, path, os, normalizeDirectoryPath, openchamberUserConfigRoot }) => { + if (req.query?.allowOutsideWorkspace === 'true') { + const normalized = normalizeDirectoryPath(targetPath); + if (!normalized || typeof normalized !== 'string') { + return { ok: false, error: 'Path is required' }; + } + const resolved = path.resolve(normalized); + return { ok: true, base: path.dirname(resolved), resolved }; + } + + return resolveWorkspacePathFromContext({ + req, + targetPath, + resolveProjectDirectory, + path, + os, + normalizeDirectoryPath, + openchamberUserConfigRoot, + }); +}; + const runCommandInDirectory = ({ shell, shellFlag, command, resolvedCwd, spawn, buildAugmentedPath, commandTimeoutMs }) => { return new Promise((resolve) => { let stdout = ''; @@ -290,7 +311,7 @@ export const registerFsRoutes = (app, dependencies) => { } try { - const resolved = await resolveWorkspacePathFromContext({ + const resolved = await resolveReadPathFromContext({ req, targetPath: filePath, resolveProjectDirectory, @@ -338,7 +359,7 @@ export const registerFsRoutes = (app, dependencies) => { } try { - const resolved = await resolveWorkspacePathFromContext({ + const resolved = await resolveReadPathFromContext({ req, targetPath: filePath, resolveProjectDirectory, @@ -387,7 +408,7 @@ export const registerFsRoutes = (app, dependencies) => { } try { - const resolved = await resolveWorkspacePathFromContext({ + const resolved = await resolveReadPathFromContext({ req, targetPath: filePath, resolveProjectDirectory, diff --git a/packages/web/src/api/files.ts b/packages/web/src/api/files.ts index f3bb20d5..b8f0cd73 100644 --- a/packages/web/src/api/files.ts +++ b/packages/web/src/api/files.ts @@ -110,9 +110,13 @@ export const createWebFilesAPI = (): FilesAPI => ({ }; }, - async statFile(path: string): Promise<{ path: string; isFile: boolean; size: number; mtimeMs?: number }> { + async statFile(path: string, options): Promise<{ path: string; isFile: boolean; size: number; mtimeMs?: number }> { const target = normalizePath(path); - const response = await fetch(`/api/fs/stat?path=${encodeURIComponent(target)}`); + const params = new URLSearchParams({ path: target }); + if (options?.allowOutsideWorkspace) { + params.set('allowOutsideWorkspace', 'true'); + } + const response = await fetch(`/api/fs/stat?${params.toString()}`); if (!response.ok) { const error = await response.json().catch(() => ({ error: response.statusText })); @@ -128,9 +132,13 @@ export const createWebFilesAPI = (): FilesAPI => ({ }; }, - async readFile(path: string): Promise<{ content: string; path: string }> { + async readFile(path: string, options): Promise<{ content: string; path: string }> { const target = normalizePath(path); - const response = await fetch(`/api/fs/read?path=${encodeURIComponent(target)}`); + const params = new URLSearchParams({ path: target }); + if (options?.allowOutsideWorkspace) { + params.set('allowOutsideWorkspace', 'true'); + } + const response = await fetch(`/api/fs/read?${params.toString()}`); if (!response.ok) { const error = await response.json().catch(() => ({ error: response.statusText }));