Skip to content

Commit 3b14032

Browse files
brendan-kellambrendan-kellam
committed
feedback
1 parent 2b8fce2 commit 3b14032

File tree

4 files changed

+69
-43
lines changed

4 files changed

+69
-43
lines changed

‎packages/web/src/features/fileTree/actions.ts

Lines changed: 48 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -4,9 +4,12 @@ import { sew, withAuth, withOrgMembership } from '@/actions';
44
import { env } from '@/env.mjs';
55
import { OrgRole, Repo } from '@sourcebot/db';
66
import { prisma } from '@/prisma';
7-
import { notFound } from '@/lib/serviceError';
7+
import { notFound, unexpectedError } from '@/lib/serviceError';
88
import { simpleGit } from 'simple-git';
99
import path from 'path';
10+
import { createLogger } from '@sourcebot/logger';
11+
12+
const logger = createLogger('file-tree');
1013

1114
export type FileTreeItem = {
1215
type: string;
@@ -40,16 +43,23 @@ export const getTree = async (params: { repoName: string, revisionName: string }
4043
const { path: repoPath } = getRepoPath(repo);
4144

4245
const git = simpleGit().cwd(repoPath);
43-
const result = await git.raw([
44-
'ls-tree',
45-
revisionName,
46-
// recursive
47-
'-r',
48-
// include trees when recursing
49-
'-t',
50-
// format as output as {type},{path}
51-
'--format=%(objecttype),%(path)',
52-
]);
46+
47+
let result: string;
48+
try {
49+
result = await git.raw([
50+
'ls-tree',
51+
revisionName,
52+
// recursive
53+
'-r',
54+
// include trees when recursing
55+
'-t',
56+
// format as output as {type},{path}
57+
'--format=%(objecttype),%(path)',
58+
]);
59+
} catch (error) {
60+
logger.error('git ls-tree failed.', { error });
61+
return unexpectedError('git ls-tree command failed.');
62+
}
5363

5464
const lines = result.split('\n').filter(line => line.trim());
5565

@@ -91,24 +101,44 @@ export const getFolderContents = async (params: { repoName: string, revisionName
91101

92102
const { path: repoPath } = getRepoPath(repo);
93103

104+
// @note: we don't allow directory traversal
105+
// or null bytes in the path.
106+
if (path.includes('..') || path.includes('\0')) {
107+
return notFound();
108+
}
109+
110+
// Normalize the path by...
94111
let normalizedPath = path;
95112

113+
// ... adding a trailing slash if it doesn't have one.
114+
// This is important since ls-tree won't return the contents
115+
// of a directory if it doesn't have a trailing slash.
96116
if (!normalizedPath.endsWith('/')) {
97117
normalizedPath = `${normalizedPath}/`;
98118
}
99119

120+
// ... removing any leading slashes. This is needed since
121+
// the path is relative to the repository's root, so we
122+
// need a relative path.
100123
if (normalizedPath.startsWith('/')) {
101124
normalizedPath = normalizedPath.slice(1);
102125
}
103126

104127
const git = simpleGit().cwd(repoPath);
105-
const result = await git.raw([
106-
'ls-tree',
107-
revisionName,
108-
// format as output as {type},{path}
109-
'--format=%(objecttype),%(path)',
110-
...(normalizedPath.length === 0 ? [] : [normalizedPath]),
111-
]);
128+
129+
let result: string;
130+
try {
131+
result = await git.raw([
132+
'ls-tree',
133+
revisionName,
134+
// format as output as {type},{path}
135+
'--format=%(objecttype),%(path)',
136+
...(normalizedPath.length === 0 ? [] : [normalizedPath]),
137+
]);
138+
} catch (error) {
139+
logger.error('git ls-tree failed.', { error });
140+
return unexpectedError('git ls-tree command failed.');
141+
}
112142

113143
const lines = result.split('\n').filter(line => line.trim());
114144

‎packages/web/src/features/fileTree/components/fileTreeItemComponent.tsx

Lines changed: 5 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -92,13 +92,11 @@ export const FileTreeItemComponent = ({
9292
className="flex flex-row gap-1 cursor-pointer w-4 h-4 flex-shrink-0"
9393
>
9494
{isCollapseChevronVisible && (
95-
<>
96-
{isCollapsed ? (
97-
<ChevronRightIcon className="w-4 h-4 flex-shrink-0" />
98-
) : (
99-
<ChevronDownIcon className="w-4 h-4 flex-shrink-0" />
100-
)}
101-
</>
95+
isCollapsed ? (
96+
<ChevronRightIcon className="w-4 h-4 flex-shrink-0" />
97+
) : (
98+
<ChevronDownIcon className="w-4 h-4 flex-shrink-0" />
99+
)
102100
)}
103101
</div>
104102
<Icon icon={iconName} className="w-4 h-4 flex-shrink-0" />

‎packages/web/src/features/fileTree/components/fileTreePanel.tsx

Lines changed: 13 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -53,7 +53,7 @@ export const FileTreePanel = ({ order }: FileTreePanelProps) => {
5353
}, domain)
5454
),
5555
});
56-
56+
5757
useHotkeys("mod+b", () => {
5858
if (isFileTreePanelCollapsed) {
5959
fileTreePanelRef.current?.expand();
@@ -182,22 +182,22 @@ const FileTreePanelSkeleton = () => {
182182
<div className="flex items-center gap-2 pl-4">
183183
<Skeleton className="w-4 h-4" />
184184
<Skeleton className="w-4 h-4" />
185-
<Skeleton className="h-4 w-18" />
185+
<Skeleton className="h-4 w-16" />
186186
</div>
187187
<div className="flex items-center gap-2 pl-8">
188188
<Skeleton className="w-4 h-4" />
189189
<Skeleton className="w-4 h-4" />
190-
<Skeleton className="h-4 w-22" />
190+
<Skeleton className="h-4 w-20" />
191191
</div>
192192
<div className="flex items-center gap-2 pl-8">
193193
<Skeleton className="w-4 h-4" />
194194
<Skeleton className="w-4 h-4" />
195-
<Skeleton className="h-4 w-30" />
195+
<Skeleton className="h-4 w-28" />
196196
</div>
197197
<div className="flex items-center gap-2 pl-4">
198198
<Skeleton className="w-4 h-4" />
199199
<Skeleton className="w-4 h-4" />
200-
<Skeleton className="h-4 w-26" />
200+
<Skeleton className="h-4 w-24" />
201201
</div>
202202
<div className="flex items-center gap-2">
203203
<Skeleton className="w-4 h-4" />
@@ -212,7 +212,7 @@ const FileTreePanelSkeleton = () => {
212212
<div className="flex items-center gap-2 pl-4">
213213
<Skeleton className="w-4 h-4" />
214214
<Skeleton className="w-4 h-4" />
215-
<Skeleton className="h-4 w-35" />
215+
<Skeleton className="h-4 w-32" />
216216
</div>
217217
<div className="flex items-center gap-2 pl-8">
218218
<Skeleton className="w-4 h-4" />
@@ -232,7 +232,7 @@ const FileTreePanelSkeleton = () => {
232232
<div className="flex items-center gap-2">
233233
<Skeleton className="w-4 h-4" />
234234
<Skeleton className="w-4 h-4" />
235-
<Skeleton className="h-4 w-22" />
235+
<Skeleton className="h-4 w-20" />
236236
</div>
237237
<div className="flex items-center gap-2 pl-4">
238238
<Skeleton className="w-4 h-4" />
@@ -242,12 +242,12 @@ const FileTreePanelSkeleton = () => {
242242
<div className="flex items-center gap-2 pl-4">
243243
<Skeleton className="w-4 h-4" />
244244
<Skeleton className="w-4 h-4" />
245-
<Skeleton className="h-4 w-18" />
245+
<Skeleton className="h-4 w-16" />
246246
</div>
247247
<div className="flex items-center gap-2">
248248
<Skeleton className="w-4 h-4" />
249249
<Skeleton className="w-4 h-4" />
250-
<Skeleton className="h-4 w-26" />
250+
<Skeleton className="h-4 w-24" />
251251
</div>
252252
<div className="flex items-center gap-2 pl-4">
253253
<Skeleton className="w-4 h-4" />
@@ -257,7 +257,7 @@ const FileTreePanelSkeleton = () => {
257257
<div className="flex items-center gap-2 pl-4">
258258
<Skeleton className="w-4 h-4" />
259259
<Skeleton className="w-4 h-4" />
260-
<Skeleton className="h-4 w-30" />
260+
<Skeleton className="h-4 w-28" />
261261
</div>
262262
<div className="flex items-center gap-2 pl-8">
263263
<Skeleton className="w-4 h-4" />
@@ -272,7 +272,7 @@ const FileTreePanelSkeleton = () => {
272272
<div className="flex items-center gap-2 pl-12">
273273
<Skeleton className="w-4 h-4" />
274274
<Skeleton className="w-4 h-4" />
275-
<Skeleton className="h-4 w-22" />
275+
<Skeleton className="h-4 w-20" />
276276
</div>
277277
<div className="flex items-center gap-2 pl-12">
278278
<Skeleton className="w-4 h-4" />
@@ -282,7 +282,7 @@ const FileTreePanelSkeleton = () => {
282282
<div className="flex items-center gap-2 pl-8">
283283
<Skeleton className="w-4 h-4" />
284284
<Skeleton className="w-4 h-4" />
285-
<Skeleton className="h-4 w-26" />
285+
<Skeleton className="h-4 w-24" />
286286
</div>
287287
<div className="flex items-center gap-2">
288288
<Skeleton className="w-4 h-4" />
@@ -297,7 +297,7 @@ const FileTreePanelSkeleton = () => {
297297
<div className="flex items-center gap-2 pl-4">
298298
<Skeleton className="w-4 h-4" />
299299
<Skeleton className="w-4 h-4" />
300-
<Skeleton className="h-4 w-18" />
300+
<Skeleton className="h-4 w-16" />
301301
</div>
302302
<div className="flex items-center gap-2 pl-4">
303303
<Skeleton className="w-4 h-4" />

‎packages/web/src/features/fileTree/components/pureFileTreePanel.tsx

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -2,11 +2,10 @@
22

33
import { FileTreeNode as RawFileTreeNode } from "../actions";
44
import { ScrollArea, ScrollBar } from "@/components/ui/scroll-area";
5-
import { useCallback, useMemo, useState, useEffect, useRef } from "react";
5+
import React, { useCallback, useMemo, useState, useEffect, useRef } from "react";
66
import { FileTreeItemComponent } from "./fileTreeItemComponent";
77
import { useBrowseNavigation } from "@/app/[domain]/browse/hooks/useBrowseNavigation";
88
import { useBrowseParams } from "@/app/[domain]/browse/hooks/useBrowseParams";
9-
import { useDomain } from "@/hooks/useDomain";
109
import { usePrefetchFileSource } from "@/hooks/usePrefetchFileSource";
1110

1211

@@ -46,7 +45,6 @@ export const PureFileTreePanel = ({ tree: _tree, path }: PureFileTreePanelProps)
4645
const { navigateToPath } = useBrowseNavigation();
4746
const { repoName, revisionName } = useBrowseParams();
4847
const { prefetchFileSource } = usePrefetchFileSource();
49-
const domain = useDomain();
5048

5149
// @note: When `_tree` changes, it indicates that a new tree has been loaded.
5250
// In that case, we need to rebuild the collapsable tree.
@@ -108,7 +106,7 @@ export const PureFileTreePanel = ({ tree: _tree, path }: PureFileTreePanelProps)
108106
<>
109107
{nodes.children.map((node) => {
110108
return (
111-
<>
109+
<React.Fragment key={node.path}>
112110
<FileTreeItemComponent
113111
key={node.path}
114112
node={node}
@@ -121,7 +119,7 @@ export const PureFileTreePanel = ({ tree: _tree, path }: PureFileTreePanelProps)
121119
parentRef={scrollAreaRef}
122120
/>
123121
{node.children.length > 0 && !node.isCollapsed && renderTree(node, depth + 1)}
124-
</>
122+
</React.Fragment>
125123
);
126124
})}
127125
</>

0 commit comments

Comments
 (0)