improved fix

brendan-kellam · brendan-kellam · commit 518e8b5f2067 · 2026-01-15T15:38:16.000-08:00
diff --git a/packages/web/src/features/fileTree/api.ts b/packages/web/src/features/fileTree/api.ts
@@ -9,7 +9,7 @@ import { createLogger } from '@sourcebot/shared';
 import path from 'path';
 import { simpleGit } from 'simple-git';
 import { FileTreeItem } from './types';
-import { buildFileTree, normalizePath } from './utils';
+import { buildFileTree, isPathValid, normalizePath } from './utils';
 import { compareFileTreeItems } from './utils';
 
 const logger = createLogger('file-tree');
@@ -36,6 +36,10 @@ export const getTree = async (params: { repoName: string, revisionName: string,
         const { path: repoPath } = getRepoPath(repo);
 
         const git = simpleGit().cwd(repoPath);
+        if (!paths.every(path => isPathValid(path))) {
+            return notFound();
+        }
+
         const normalizedPaths = paths.map(path => normalizePath(path));
 
         let result: string = '';
@@ -100,6 +104,9 @@ export const getFolderContents = async (params: { repoName: string, revisionName
         const { path: repoPath } = getRepoPath(repo);
         const git = simpleGit().cwd(repoPath);
 
+        if (!isPathValid(path)) {
+            return notFound();
+        }
         const normalizedPath = normalizePath(path);
 
         let result: string;
diff --git a/packages/web/src/features/fileTree/utils.test.ts b/packages/web/src/features/fileTree/utils.test.ts
@@ -1,5 +1,5 @@
 import { expect, test } from 'vitest';
-import { buildFileTree, normalizePath } from './utils';
+import { buildFileTree, isPathValid, normalizePath } from './utils';
 
 test('normalizePath adds a trailing slash and strips leading slashes', () => {
     expect(normalizePath('/a/b')).toBe('a/b/');
@@ -13,6 +13,23 @@ test('normalizePath returns empty string for root', () => {
     expect(normalizePath('/')).toBe('');
 });
 
+test('isPathValid rejects traversal and null bytes', () => {
+    expect(isPathValid('a/../b')).toBe(false);
+    expect(isPathValid('a/\0b')).toBe(false);
+});
+
+test('isPathValid allows normal paths', () => {
+    expect(isPathValid('a/b')).toBe(true);
+});
+
+test('isPathValid allows paths with dots', () => {
+    expect(isPathValid('a/b/c.txt')).toBe(true);
+    expect(isPathValid('a/b/c...')).toBe(true);
+    expect(isPathValid('a/b/c.../d')).toBe(true);
+    expect(isPathValid('a/b/..c')).toBe(true);
+    expect(isPathValid('a/b/[..path]')).toBe(true);
+});
+
 test('buildFileTree handles a empty flat list', () => {
     const flatList: { type: string, path: string }[] = [];
     const tree = buildFileTree(flatList);
diff --git a/packages/web/src/features/fileTree/utils.ts b/packages/web/src/features/fileTree/utils.ts
@@ -21,6 +21,17 @@ export const normalizePath = (path: string): string => {
     return normalizedPath;
 }
 
+// @note: we don't allow directory traversal
+// or null bytes in the path.
+export const isPathValid = (path: string) => {
+    const pathSegments = path.split('/');
+    if (pathSegments.some(segment => segment === '..') || path.includes('\0')) {
+        return false;
+    }
+
+    return true;
+}
+
 export const buildFileTree = (flatList: { type: string, path: string }[]): FileTreeNode => {
     const root: FileTreeNode = {
         name: 'root',
