From e7ba1c2a896fb930d680409d2d78bd61334cbfd4 Mon Sep 17 00:00:00 2001 From: Juan Denis Date: Sat, 24 Oct 2026 04:31:28 -0400 Subject: [PATCH] refactor: route file operations through capability-checked adapters --- .../__tests__/fileAdapter.test.js | 60 ++++++++++++++ .../components/file-manager/fileAdapter.js | 49 +++++++++++ frontend/src/pages/FileManager.jsx | 81 ++++--------------- 3 files changed, 125 insertions(+), 65 deletions(-) create mode 100644 frontend/src/components/file-manager/__tests__/fileAdapter.test.js create mode 100644 frontend/src/components/file-manager/fileAdapter.js diff --git a/frontend/src/components/file-manager/__tests__/fileAdapter.test.js b/frontend/src/components/file-manager/__tests__/fileAdapter.test.js new file mode 100644 index 00000000..bf218f12 --- /dev/null +++ b/frontend/src/components/file-manager/__tests__/fileAdapter.test.js @@ -0,0 +1,60 @@ +import assert from 'node:assert/strict'; +import test from 'node:test'; +import { createFileAdapter } from '../fileAdapter.js'; + +function fixture(target) { + const calls = []; + const api = new Proxy({}, { get: (_, name) => (...args) => { + calls.push([name, ...args]); + return Promise.resolve({ success: true, data: { files: [] } }); + } }); + return { adapter: createFileAdapter(api, target), calls }; +} + +test('agent operations use its server id and unwrap older envelopes', async () => { + const { adapter, calls } = fixture({ kind: 'agent', server_id: 7 }); + assert.deepEqual(await adapter.browse('/home', true), { files: [] }); + await adapter.read('/home/a'); + await adapter.write('/home/a', 'edited'); + assert.deepEqual(calls, [['browseRemoteFiles', 7, '/home'], + ['readRemoteFile', 7, '/home/a'], ['writeRemoteFile', 7, '/home/a', 'edited']]); +}); + +for (const kind of ['agent', 's3']) { + test(`${kind} unsupported operations never reach the panel host`, () => { + const { adapter, calls } = fixture({ kind, server_id: 7 }); + const unsupported = ['search', 'create folder', 'rename', 'change permissions']; + if (kind === 'agent') unsupported.push('delete', 'download', 'upload'); + for (const operation of unsupported) { + assert.equal(adapter.supports(operation), false); + assert.throws(() => adapter[operation]('/home/a'), /unavailable/); + } + assert.deepEqual(calls, []); + }); +} + +test('S3 transfer and mutation operations stay in the bucket', async () => { + const { adapter, calls } = fixture({ kind: 's3' }); + const progress = () => {}; + await adapter.upload('/folder', 'file', progress); + await adapter.write('/a', 'edited'); + await adapter.del('/a'); + adapter.download({ path: '/b' }); + assert.deepEqual(calls, [['uploadS3', '/folder', 'file', progress], + ['writeS3', '/a', 'edited'], ['deleteS3', '/a'], ['downloadS3File', '/b']]); +}); + +test('local calls retain hidden flag and mutation arguments', async () => { + const { adapter, calls } = fixture({ kind: 'local' }); + await adapter.browse('/home', true); + await adapter['create folder']('/home/assets'); + await adapter.rename('/home/assets', 'new'); + await adapter['change permissions']('/home/assets', '700'); + assert.deepEqual(calls, [['browseFiles', '/home', true], + ['createDirectory', '/home/assets'], ['renameFile', '/home/assets', 'new'], + ['changeFilePermissions', '/home/assets', '700']]); +}); + +test('unknown targets fail closed', () => { + assert.throws(() => fixture({ kind: 'typo' }), /Unknown file target/); +}); diff --git a/frontend/src/components/file-manager/fileAdapter.js b/frontend/src/components/file-manager/fileAdapter.js new file mode 100644 index 00000000..6949c432 --- /dev/null +++ b/frontend/src/components/file-manager/fileAdapter.js @@ -0,0 +1,49 @@ +// All file operations cross this boundary. Unsupported targets never fall back +// to the panel host, even if a caller forgets to disable a control. +const OPERATIONS = ['browse', 'read', 'write', 'delete', 'download', 'upload', + 'search', 'create folder', 'rename', 'change permissions']; + +function unwrapAgentData(result) { + return result && typeof result === 'object' && 'success' in result && 'data' in result + ? result.data : result; +} + +export function createFileAdapter(api, target) { + const local = { + browse: (path, hidden) => api.browseFiles(path, hidden), + read: (path) => api.readFile(path), + write: (path, content) => api.writeFile(path, content), + delete: (path) => api.deleteFile(path), + download: (entry) => api.downloadFile(entry.path), + upload: (path, file, progress) => api.uploadFile(path, file, progress), + search: (path, query) => api.searchFiles(path, query), + 'create folder': (path) => api.createDirectory(path), + rename: (path, name) => api.renameFile(path, name), + 'change permissions': (path, mode) => api.changeFilePermissions(path, mode), + }; + const agent = { + browse: async (path) => unwrapAgentData(await api.browseRemoteFiles(target.server_id, path)), + read: async (path) => unwrapAgentData(await api.readRemoteFile(target.server_id, path)), + write: async (path, content) => unwrapAgentData(await api.writeRemoteFile(target.server_id, path, content)), + }; + const s3 = { + browse: (path) => api.browseS3(path), + read: (path) => api.readS3(path), + write: (path, content) => api.writeS3(path, content), + delete: (path) => api.deleteS3(path), + download: (entry) => api.downloadS3File(entry.path), + upload: (path, file, progress) => api.uploadS3(path, file, progress), + }; + const methods = { local, agent, s3 }[target.kind]; + if (!methods) throw new Error(`Unknown file target: ${target.kind}`); + const supports = (operation) => Object.hasOwn(methods, operation); + const adapter = { supports }; + for (const operation of OPERATIONS) { + adapter[operation] = (...args) => { + if (!supports(operation)) throw new Error(`${operation} is unavailable on ${target.kind}`); + return methods[operation](...args); + }; + } + adapter.del = adapter.delete; + return Object.freeze(adapter); +} diff --git a/frontend/src/pages/FileManager.jsx b/frontend/src/pages/FileManager.jsx index 7aa8d399..eb1c1481 100644 --- a/frontend/src/pages/FileManager.jsx +++ b/frontend/src/pages/FileManager.jsx @@ -1,5 +1,6 @@ import { useState, useEffect, useRef, useMemo, useCallback } from 'react'; import { api } from '../services/api'; +import { createFileAdapter } from '../components/file-manager/fileAdapter'; import { useToast } from '../contexts/useToast.js'; import ConfirmDialog from '../components/ConfirmDialog'; import EmptyState from '../components/EmptyState'; @@ -68,16 +69,6 @@ const QUICK_ACCESS_AGENT = { ], }; -// File manager operations that the agent can serve over file:* commands. -// Anything else (mkdir, delete, rename, copy, chmod, search, disk usage, -// upload/download) is panel-host-only until the matching agent verbs land. -const REMOTE_SUPPORTED = new Set(['browse', 'read', 'write']); - -// Operations the S3 target can't serve (no real directories, permissions, or -// in-place rename). Everything else — browse/read/write/delete/upload/download — -// works against the bucket. -const S3_BLOCKED = new Set(['create file', 'create folder', 'rename', 'change permissions']); - function deriveParent(path) { if (!path || path === '/' || path === '') return null; // A Windows drive root ("C:/" or "C:") sits directly under the agent's @@ -94,16 +85,6 @@ function deriveParent(path) { return parent; } -function unwrapAgentData(res) { - // Remote endpoints return the agent payload directly - // (RemoteFileService → _agent_result unwraps {data}). Defensive - // unwrap covers both shapes for older agent responses. - if (res && typeof res === 'object' && 'success' in res && 'data' in res) { - return res.data; - } - return res; -} - function mountDisplayName(mount) { if (!mount?.mountpoint || mount.mountpoint === '/') return null; const parts = mount.mountpoint.split('/').filter(Boolean); @@ -310,50 +291,19 @@ function FileManager() { // eslint-disable-next-line react-hooks/exhaustive-deps }, [currentPath]); - // ─── file API adapter ──────────────────────────────── - // Routes the three verbs the agent supports through the remote - // endpoints when target is an agent; falls back to the panel-local - // FileService otherwise. Other ops keep going to panel-local — the - // remoteGuard helper short-circuits them with a toast when the user - // is on a remote target so we don't accidentally write to the panel - // host while they think they're editing a remote server. - const fileApi = useMemo(() => ({ - browse: async (path, hidden) => { - if (isS3) return api.browseS3(path); - if (isRemote) { - return unwrapAgentData(await api.browseRemoteFiles(target.server_id, path)); - } - return api.browseFiles(path, hidden); - }, - read: async (path) => { - if (isS3) return api.readS3(path); - if (isRemote) { - return unwrapAgentData(await api.readRemoteFile(target.server_id, path)); - } - return api.readFile(path); - }, - write: async (path, content) => { - if (isS3) return api.writeS3(path, content); - if (isRemote) { - return unwrapAgentData(await api.writeRemoteFile(target.server_id, path, content)); - } - return api.writeFile(path, content); - }, - del: async (path) => (isS3 ? api.deleteS3(path) : api.deleteFile(path)), - download: (entry) => (isS3 ? api.downloadS3File(entry.path) : api.downloadFile(entry.path)), - }), [isRemote, isS3, target]); + const fileApi = useMemo(() => createFileAdapter(api, target), [target]); const remoteGuard = useCallback((op) => { - if (isRemote && !REMOTE_SUPPORTED.has(op)) { + if (isRemote && !fileApi.supports(op)) { toast.error(t('app.fileManager.isNotYetSupportedOnRemote', '{{op}} is not yet supported on remote agents', { op: op })); return true; } - if (isS3 && S3_BLOCKED.has(op)) { + if (isS3 && !fileApi.supports(op)) { toast.error(t('app.fileManager.isnTAvailableOnS3Buckets', '{{op}} isn\'t available on S3 buckets', { op: op })); return true; } return false; - }, [isRemote, isS3, t, toast]); + }, [isRemote, isS3, fileApi, t, toast]); // When the user switches to a remote target, jump to its first // advertised allowed_path so they don't see a "panel /home" view @@ -497,7 +447,7 @@ function FileManager() { if (!searchQuery.trim()) { setSearchResults(null); return; } setLoading(true); try { - const data = await api.searchFiles(currentPath, searchQuery); + const data = await fileApi.search(currentPath, searchQuery); setSearchResults(data.results || []); } catch (error) { toast.error(t('app.fileManager.searchFailed', "Couldn't search. {{message}}", { message: errorReason(error) })); @@ -590,7 +540,7 @@ function FileManager() { const handleCreateFolder = async (name) => { if (remoteGuard('create folder')) return; try { - await api.createDirectory(`${currentPath}/${name}`); + await fileApi['create folder'](`${currentPath}/${name}`); toast.success(t('app.fileManager.folderCreated', 'Folder created')); closeDialog(); loadDirectory(currentPath); @@ -598,7 +548,7 @@ function FileManager() { const parent = currentPath; if (treeCache.has(parent)) { try { - const data = await api.browseFiles(parent, false); + const data = await fileApi.browse(parent, false); const folders = (data.entries || []).filter((e) => e.is_dir).map((e) => ({ path: e.path, name: e.name })); setTreeCache((c) => { const n = new Map(c); n.set(parent, folders); return n; }); } catch { /* ignore */ } @@ -643,7 +593,7 @@ function FileManager() { const handleRename = async (entry, name) => { if (remoteGuard('rename')) return; try { - await api.renameFile(entry.path, name); + await fileApi.rename(entry.path, name); toast.success(t('app.fileManager.renamed', 'Renamed')); closeDialog(); loadDirectory(currentPath); @@ -655,7 +605,7 @@ function FileManager() { const handleChangePermissions = async (entry, mode) => { if (remoteGuard('change permissions')) return; try { - await api.changeFilePermissions(entry.path, mode); + await fileApi['change permissions'](entry.path, mode); toast.success(t('app.fileManager.permissionsUpdated', 'Permissions updated')); closeDialog(); loadDirectory(currentPath); @@ -687,7 +637,7 @@ function FileManager() { const itemId = queue[i].id; try { setUploads((p) => p.map((u) => u.id === itemId ? { ...u, status: 'uploading' } : u)); - const doUpload = isS3 ? api.uploadS3 : api.uploadFile; + const doUpload = fileApi.upload; await doUpload(currentPath, file, (progress) => { setUploads((p) => p.map((u) => u.id === itemId ? { ...u, progress } : u)); }); @@ -871,7 +821,8 @@ function FileManager() { const rowOpen = useStableHandler(handleOpen); const rowToggleSelect = useStableHandler(handleToggleSelect); const rowContext = useStableHandler(openContextMenu); - const rowDownload = useStableHandler((entry) => fileApi.download(entry)); + const handleDownload = (entry) => { if (!remoteGuard('download')) fileApi.download(entry); }; + const rowDownload = useStableHandler(handleDownload); const rowRename = useStableHandler(openRenameModal); const rowPermissions = useStableHandler(openPermissionsModal); const rowDelete = useStableHandler((entry) => handleDelete(entry)); @@ -879,7 +830,7 @@ function FileManager() { const copyPathToClipboard = (path) => copyText(path); const downloadSelected = () => { - selectedEntries.filter((e) => !e.is_dir).forEach((e) => fileApi.download(e)); + if (!remoteGuard('download')) selectedEntries.filter((e) => !e.is_dir).forEach((e) => fileApi.download(e)); }; const getDiskColor = (percent) => { @@ -1384,7 +1335,7 @@ function FileManager() { onCancelEdit={() => setEditing(false)} onSave={handleSaveFile} onClose={() => { setPreviewFile(null); setEditing(false); }} - onDownload={(e) => fileApi.download(e)} + onDownload={handleDownload} onRename={openRenameModal} onPermissions={openPermissionsModal} onCopyPath={copyPathToClipboard} @@ -1434,7 +1385,7 @@ function FileManager() { selectionCount={selectedEntries.length} onClose={() => setContextMenu(null)} onOpen={handleOpen} - onDownload={(e) => fileApi.download(e)} + onDownload={handleDownload} onRename={openRenameModal} onPermissions={openPermissionsModal} onCopyPath={copyPathToClipboard}