Skip to content

Commit 95854ff

Browse files
refactor: improve file browser path resolution and navigation
- Add query parameter support for file paths to handle special characters - Enable navigation above base path with proper validation - Centralize URL encoding logic for file API calls - Update path validation to use workspace base instead of repos base - Add normalizePath utility for consistent path handling
1 parent 40a2af7 commit 95854ff

10 files changed

Lines changed: 153 additions & 50 deletions

File tree

backend/src/routes/files.ts

Lines changed: 16 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,18 @@
1-
import { Hono } from 'hono'
1+
import { Hono, type Context } from 'hono'
22
import type { ContentfulStatusCode } from 'hono/utils/http-status'
33
import * as fileService from '../services/files'
44
import * as archiveService from '../services/archive'
55
import { logger } from '../utils/logger'
66
import { getErrorMessage, getStatusCode } from '../utils/error-utils'
77

8+
function decodeFilePath(path: string): string {
9+
return decodeURIComponent(path)
10+
}
11+
12+
function getFilePathFromRequest(c: Context, fallbackPath: string): string {
13+
return c.req.query('path') ?? decodeFilePath(fallbackPath)
14+
}
15+
816
export function createFileRoutes() {
917
const app = new Hono()
1018

@@ -13,7 +21,7 @@ export function createFileRoutes() {
1321

1422
if (path.endsWith('/download-zip')) {
1523
const match = path.match(/\/api\/files\/(.+?)\/download-zip$/)
16-
const userPath = match?.[1]
24+
const userPath = match?.[1] ? decodeFilePath(match[1]) : undefined
1725

1826
if (!userPath) {
1927
return c.json({ error: 'No path provided' }, 400)
@@ -61,7 +69,7 @@ export function createFileRoutes() {
6169
}
6270

6371
if (path.endsWith('/ignored-paths')) {
64-
const userPath = path.replace(/\/api\/files\/(.+?)\/ignored-paths$/, '$1')
72+
const userPath = decodeFilePath(path.replace(/\/api\/files\/(.+?)\/ignored-paths$/, '$1'))
6573

6674
if (!userPath || userPath === '/ignored-paths') {
6775
return c.json({ error: 'No path provided' }, 400)
@@ -77,7 +85,7 @@ export function createFileRoutes() {
7785
}
7886

7987
try {
80-
const userPath = path.replace(/^\/api\/files\//, '') || ''
88+
const userPath = getFilePathFromRequest(c, path.replace(/^\/api\/files\//, '') || '')
8189
const download = c.req.query('download') === 'true'
8290
const raw = c.req.query('raw') === 'true'
8391
const startLineParam = c.req.query('startLine')
@@ -127,7 +135,7 @@ export function createFileRoutes() {
127135

128136
app.post('/*', async (c) => {
129137
try {
130-
const path = c.req.path.replace(/^\/api\/files\//, '') || ''
138+
const path = getFilePathFromRequest(c, c.req.path.replace(/^\/api\/files\//, '') || '')
131139
const body = await c.req.parseBody()
132140

133141
const file = body.file as File
@@ -146,7 +154,7 @@ export function createFileRoutes() {
146154

147155
app.put('/*', async (c) => {
148156
try {
149-
const path = c.req.path.replace(/^\/api\/files\//, '') || ''
157+
const path = getFilePathFromRequest(c, c.req.path.replace(/^\/api\/files\//, '') || '')
150158
const body = await c.req.json()
151159

152160
const result = await fileService.createFileOrFolder(path, body)
@@ -159,7 +167,7 @@ export function createFileRoutes() {
159167

160168
app.delete('/*', async (c) => {
161169
try {
162-
const path = c.req.path.replace(/^\/api\/files\//, '') || ''
170+
const path = getFilePathFromRequest(c, c.req.path.replace(/^\/api\/files\//, '') || '')
163171

164172
await fileService.deleteFileOrFolder(path)
165173
return c.json({ success: true })
@@ -171,7 +179,7 @@ export function createFileRoutes() {
171179

172180
app.patch('/*', async (c) => {
173181
try {
174-
const path = c.req.path.replace(/^\/api\/files\//, '') || ''
182+
const path = getFilePathFromRequest(c, c.req.path.replace(/^\/api\/files\//, '') || '')
175183
const body = await c.req.json()
176184

177185
if (body.patches && Array.isArray(body.patches)) {

backend/src/services/files.ts

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -13,11 +13,12 @@ import {
1313
getFileStats,
1414
listDirectory
1515
} from './file-operations'
16-
import { getReposPath, FILE_LIMITS } from '@opencode-manager/shared/config/env'
16+
import { getReposPath, getWorkspacePath, FILE_LIMITS } from '@opencode-manager/shared/config/env'
1717
import { ALLOWED_MIME_TYPES, type AllowedMimeType } from '@opencode-manager/shared'
1818
import type { ChunkedFileInfo, PatchOperation } from '@opencode-manager/shared'
1919

2020
const SHARED_WORKSPACE_BASE = getReposPath()
21+
const WORKSPACE_BASE = getWorkspacePath()
2122

2223
interface FileInfo {
2324
name: string
@@ -234,12 +235,12 @@ export async function renameOrMoveFile(userPath: string, body: { newPath: string
234235

235236
function validatePath(userPath: string): string {
236237
const trimmed = userPath.trim()
237-
const normalized = path.normalize(trimmed).replace(/^(\.\.(\/|\\|$))+/, '')
238+
const normalized = path.normalize(trimmed || '.')
238239
const fullPath = path.join(SHARED_WORKSPACE_BASE, normalized)
239240
const resolved = path.resolve(fullPath)
240241

241-
const basePath = path.resolve(SHARED_WORKSPACE_BASE)
242-
if (!resolved.startsWith(basePath)) {
242+
const basePath = path.resolve(WORKSPACE_BASE)
243+
if (resolved !== basePath && !resolved.startsWith(`${basePath}${path.sep}`)) {
243244
throw { message: 'Path traversal detected', statusCode: 403 }
244245
}
245246

@@ -369,4 +370,4 @@ export async function applyFilePatches(userPath: string, patches: PatchOperation
369370
await fs.writeFile(validatedPath, lines.join('\n'), 'utf8')
370371

371372
return { success: true, totalLines: lines.length }
372-
}
373+
}

frontend/src/api/files.ts

Lines changed: 11 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -3,8 +3,16 @@ import { fetchWrapper, fetchWrapperBlob } from './fetchWrapper'
33
import { API_BASE_URL } from '@/config'
44
import type { FileInfo, ChunkedFileInfo, PatchOperation } from '@/types/files'
55

6+
function getFileApiUrl(path: string): string {
7+
if (path.includes('..')) {
8+
return `${API_BASE_URL}/api/files/?path=${encodeURIComponent(path)}`
9+
}
10+
11+
return `${API_BASE_URL}/api/files/${path.split('/').map(encodeURIComponent).join('/')}`
12+
}
13+
614
async function fetchFile(path: string): Promise<FileInfo> {
7-
return fetchWrapper(`${API_BASE_URL}/api/files/${path}`)
15+
return fetchWrapper(getFileApiUrl(path))
816
}
917

1018
export function useFile(path: string | undefined) {
@@ -16,13 +24,13 @@ export function useFile(path: string | undefined) {
1624
}
1725

1826
export async function fetchFileRange(path: string, startLine: number, endLine: number): Promise<ChunkedFileInfo> {
19-
return fetchWrapper(`${API_BASE_URL}/api/files/${path}`, {
27+
return fetchWrapper(getFileApiUrl(path), {
2028
params: { startLine, endLine },
2129
})
2230
}
2331

2432
export async function applyFilePatches(path: string, patches: PatchOperation[]): Promise<{ success: boolean; totalLines: number }> {
25-
return fetchWrapper(`${API_BASE_URL}/api/files/${path}`, {
33+
return fetchWrapper(getFileApiUrl(path), {
2634
method: 'PATCH',
2735
headers: { 'Content-Type': 'application/json' },
2836
body: JSON.stringify({ patches }),

frontend/src/components/file-browser/FileBrowser.tsx

Lines changed: 79 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@ interface FileBrowserProps {
2727
initialSelectedFile?: string
2828
onDirectoryLoad?: (info: { workspaceRoot?: string; currentPath: string }) => void
2929
onPreviewStateChange?: (isOpen: boolean) => void
30+
allowNavigateAboveBase?: boolean
3031
}
3132

3233
interface UploadItem {
@@ -122,7 +123,7 @@ function getUploadItemsFromFileList(fileList: FileList): UploadItem[] {
122123
return items
123124
}
124125

125-
export const FileBrowser = forwardRef<FileBrowserHandle, FileBrowserProps>(function FileBrowser({ basePath = '', onFileSelect, embedded = false, initialSelectedFile, onDirectoryLoad, onPreviewStateChange }, ref) {
126+
export const FileBrowser = forwardRef<FileBrowserHandle, FileBrowserProps>(function FileBrowser({ basePath = '', onFileSelect, embedded = false, initialSelectedFile, onDirectoryLoad, onPreviewStateChange, allowNavigateAboveBase = false }, ref) {
126127
const [currentPath, setCurrentPath] = useState(basePath)
127128
const [files, setFiles] = useState<FileInfo | null>(null)
128129
const [selectedFile, setSelectedFile] = useState<FileInfo | null>(null)
@@ -137,7 +138,7 @@ export const FileBrowser = forwardRef<FileBrowserHandle, FileBrowserProps>(funct
137138
const uploadCancelledRef = useRef(false)
138139
const isMobile = useMobile()
139140

140-
const { data: initialFileData, error: initialFileError } = useFile(initialSelectedFile)
141+
const { data: initialFileData, error: initialFileError } = useFile(initialSelectedFile)
141142

142143
useEffect(() => {
143144
if (initialFileData) {
@@ -155,12 +156,25 @@ useEffect(() => {
155156
}
156157
}, [initialFileError])
157158

159+
const getFileApiUrl = useCallback((path: string) => {
160+
if (path.includes('..')) {
161+
return `${API_BASE_URL}/api/files/?path=${encodeURIComponent(path)}`
162+
}
163+
164+
const encodedPath = path
165+
.split('/')
166+
.map(segment => encodeURIComponent(segment))
167+
.join('/')
168+
169+
return `${API_BASE_URL}/api/files/${encodedPath}`
170+
}, [])
171+
158172
const loadFiles = useCallback(async (path: string) => {
159173
setLoading(true)
160174
setError(null)
161175

162176
try {
163-
const response = await fetch(`${API_BASE_URL}/api/files/${path}`)
177+
const response = await fetch(getFileApiUrl(path))
164178
if (!response.ok) {
165179
throw new Error(`Failed to load files: ${response.statusText}`)
166180
}
@@ -174,28 +188,64 @@ useEffect(() => {
174188
} finally {
175189
setLoading(false)
176190
}
177-
}, [onDirectoryLoad])
191+
}, [getFileApiUrl, onDirectoryLoad])
192+
193+
const normalizePath = useCallback((path: string) => {
194+
const normalized = path
195+
.trim()
196+
.replace(/\\/g, '/')
197+
.replace(/\/+/g, '/')
198+
.replace(/\/+$/, '')
199+
200+
if (normalized === '.' || normalized === './') {
201+
return ''
202+
}
203+
204+
if (normalized.startsWith('./')) {
205+
return normalized.slice(2)
206+
}
207+
208+
return normalized
209+
}, [])
210+
211+
const getPathParts = useCallback((path: string) => normalizePath(path).split('/').filter(Boolean), [normalizePath])
212+
213+
const canNavigateUp = useCallback(() => {
214+
if (allowNavigateAboveBase && normalizePath(currentPath) === '') {
215+
return true
216+
}
217+
218+
if (normalizePath(currentPath) === '..') {
219+
return false
220+
}
221+
222+
const pathParts = getPathParts(currentPath)
223+
if (allowNavigateAboveBase) {
224+
return pathParts.length > 0
225+
}
178226

179-
const getPathParts = useCallback((path: string) => path.split('/').filter(Boolean), [])
227+
return pathParts.length > 0 && normalizePath(currentPath) !== normalizePath(basePath)
228+
}, [allowNavigateAboveBase, basePath, currentPath, getPathParts, normalizePath])
180229

181230
const goToParentDirectory = useCallback(() => {
231+
if (allowNavigateAboveBase && normalizePath(currentPath) === '') {
232+
loadFiles('..')
233+
return
234+
}
235+
182236
const pathParts = getPathParts(currentPath)
183237
if (pathParts.length > 0) {
184238
pathParts.pop()
185239
const parentPath = pathParts.join('/')
186-
loadFiles(parentPath || basePath)
240+
loadFiles(allowNavigateAboveBase ? parentPath : parentPath || basePath)
187241
}
188-
}, [currentPath, basePath, loadFiles, getPathParts])
242+
}, [allowNavigateAboveBase, currentPath, basePath, loadFiles, getPathParts, normalizePath])
189243

190244
useImperativeHandle(ref, () => ({
191245
goBack: goToParentDirectory,
192-
canGoBack: () => {
193-
const pathParts = getPathParts(currentPath)
194-
const joinedPath = pathParts.join('/')
195-
return pathParts.length > 0 && joinedPath !== basePath
196-
},
246+
canGoBack: canNavigateUp,
197247
getCurrentPath: () => currentPath,
198-
}), [currentPath, basePath, goToParentDirectory, getPathParts])
248+
}), [currentPath, goToParentDirectory, canNavigateUp])
199249

200250
const handleFileSelect = useCallback(async (file: FileInfo) => {
201251
if (file.isDirectory) {
@@ -206,7 +256,7 @@ useEffect(() => {
206256
// Fetch the full file content when selecting a file
207257
setLoading(true)
208258
try {
209-
const response = await fetch(`${API_BASE_URL}/api/files/${file.path}`)
259+
const response = await fetch(getFileApiUrl(file.path))
210260
if (!response.ok) {
211261
throw new Error(`Failed to load file: ${response.statusText}`)
212262
}
@@ -226,7 +276,7 @@ useEffect(() => {
226276
} finally {
227277
setLoading(false)
228278
}
229-
}, [onFileSelect, isMobile, onPreviewStateChange])
279+
}, [getFileApiUrl, onFileSelect, isMobile, onPreviewStateChange])
230280

231281
const handleCloseModal = useCallback(() => {
232282
setIsPreviewModalOpen(false)
@@ -248,7 +298,7 @@ useEffect(() => {
248298
formData.append('relativePath', item.relativePath)
249299

250300
try {
251-
const response = await fetch(`${API_BASE_URL}/api/files/${currentPath}`, {
301+
const response = await fetch(getFileApiUrl(currentPath), {
252302
method: 'POST',
253303
body: formData,
254304
})
@@ -262,7 +312,7 @@ useEffect(() => {
262312
} catch (err) {
263313
return err instanceof Error ? err.message : 'Upload failed'
264314
}
265-
}, [currentPath])
315+
}, [currentPath, getFileApiUrl])
266316

267317
const handleUploadItems = useCallback(async (items: UploadItem[]) => {
268318
if (items.length === 0) return
@@ -318,7 +368,7 @@ useEffect(() => {
318368

319369
const handleCreateFile = useCallback(async (name: string, type: 'file' | 'folder') => {
320370
try {
321-
const response = await fetch(`${API_BASE_URL}/api/files/${currentPath}/${name}`, {
371+
const response = await fetch(getFileApiUrl([currentPath, name].filter(Boolean).join('/')), {
322372
method: 'PUT',
323373
headers: { 'Content-Type': 'application/json' },
324374
body: JSON.stringify({ type, content: type === 'file' ? '' : undefined }),
@@ -332,11 +382,11 @@ useEffect(() => {
332382
} catch (err) {
333383
setError(err instanceof Error ? err.message : 'Create failed')
334384
}
335-
}, [currentPath, loadFiles])
385+
}, [currentPath, getFileApiUrl, loadFiles])
336386

337387
const handleDelete = useCallback(async (path: string) => {
338388
try {
339-
const response = await fetch(`${API_BASE_URL}/api/files/${path}`, {
389+
const response = await fetch(getFileApiUrl(path), {
340390
method: 'DELETE',
341391
})
342392

@@ -349,11 +399,11 @@ useEffect(() => {
349399
} catch (err) {
350400
setError(err instanceof Error ? err.message : 'Delete failed')
351401
}
352-
}, [currentPath, loadFiles])
402+
}, [currentPath, getFileApiUrl, loadFiles])
353403

354404
const handleRename = useCallback(async (oldPath: string, newPath: string) => {
355405
try {
356-
const response = await fetch(`${API_BASE_URL}/api/files/${oldPath}`, {
406+
const response = await fetch(getFileApiUrl(oldPath), {
357407
method: 'PATCH',
358408
headers: { 'Content-Type': 'application/json' },
359409
body: JSON.stringify({ newPath }),
@@ -367,7 +417,7 @@ useEffect(() => {
367417
} catch (err) {
368418
setError(err instanceof Error ? err.message : 'Rename failed')
369419
}
370-
}, [currentPath, loadFiles])
420+
}, [currentPath, getFileApiUrl, loadFiles])
371421

372422
const handleDragEnter = (e: React.DragEvent) => {
373423
e.preventDefault()
@@ -427,6 +477,10 @@ useEffect(() => {
427477
return () => document.removeEventListener('keydown', handleEscape)
428478
}, [isPreviewModalOpen, handleCloseModal])
429479

480+
const showNavigateUp = allowNavigateAboveBase && normalizePath(currentPath) !== '..'
481+
? true
482+
: canNavigateUp()
483+
430484
const filteredFiles = files?.children?.filter(file =>
431485
file.name.toLowerCase().includes(searchQuery.toLowerCase())
432486
)
@@ -557,6 +611,7 @@ useEffect(() => {
557611
currentPath={currentPath}
558612
basePath={basePath}
559613
onNavigateUp={goToParentDirectory}
614+
canNavigateUp={showNavigateUp}
560615
/>
561616
)}
562617
</div>
@@ -657,6 +712,7 @@ useEffect(() => {
657712
currentPath={currentPath}
658713
basePath={basePath}
659714
onNavigateUp={goToParentDirectory}
715+
canNavigateUp={showNavigateUp}
660716
/>
661717
</div>
662718
)}

0 commit comments

Comments
 (0)