Skip to content

Commit 9798183

Browse files
committed
feat: enhance file matching and session tab management
- Introduced a new constant `MaxGlobMatches` to limit the number of matched files, preventing resource exhaustion during file searches. - Updated `findMatchingFiles` to return an error for invalid base directories and enforce security checks against directory traversal. - Enhanced the session detail page to limit the number of open tabs, ensuring efficient memory usage and improved user experience. - Implemented memoization for workflow results to optimize tab content updates, reducing unnecessary re-renders. These changes improve the robustness and performance of the content workflow and session management interface.
1 parent 9c50f85 commit 9798183

File tree

3 files changed

+161
-40
lines changed

3 files changed

+161
-40
lines changed

components/backend/handlers/content.go

Lines changed: 61 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,9 @@ var StateBaseDir string
2727
// MaxResultFileSize is the maximum size for result files to prevent memory issues
2828
const MaxResultFileSize = 10 * 1024 * 1024 // 10MB
2929

30+
// MaxGlobMatches limits the number of files that can be matched to prevent resource exhaustion
31+
const MaxGlobMatches = 100
32+
3033
// Git operation functions - set by main package during initialization
3134
// These are set to the actual implementations from git package
3235
var (
@@ -688,7 +691,17 @@ func ContentWorkflowResults(c *gin.Context) {
688691

689692
for _, displayName := range displayNames {
690693
pattern := ambientConfig.Results[displayName]
691-
matches := findMatchingFiles(workspaceBase, pattern)
694+
matches, err := findMatchingFiles(workspaceBase, pattern)
695+
696+
if err != nil {
697+
results = append(results, ResultFile{
698+
DisplayName: displayName,
699+
Path: pattern,
700+
Exists: false,
701+
Error: fmt.Sprintf("Pattern error: %v", err),
702+
})
703+
continue
704+
}
692705

693706
if len(matches) == 0 {
694707
results = append(results, ResultFile{
@@ -740,22 +753,63 @@ func ContentWorkflowResults(c *gin.Context) {
740753
}
741754

742755
// findMatchingFiles finds files matching a glob pattern with ** support for recursive matching
743-
func findMatchingFiles(baseDir, pattern string) []string {
756+
// Returns matched files and an error if validation fails or too many matches found
757+
func findMatchingFiles(baseDir, pattern string) ([]string, error) {
758+
// Validate baseDir is absolute and exists
759+
if !filepath.IsAbs(baseDir) {
760+
return nil, fmt.Errorf("baseDir must be absolute path")
761+
}
762+
763+
baseInfo, err := os.Stat(baseDir)
764+
if err != nil {
765+
return nil, fmt.Errorf("baseDir does not exist: %w", err)
766+
}
767+
if !baseInfo.IsDir() {
768+
return nil, fmt.Errorf("baseDir is not a directory")
769+
}
770+
744771
// Use doublestar for glob matching with ** support
745772
fsys := os.DirFS(baseDir)
746773
matches, err := doublestar.Glob(fsys, pattern)
747774
if err != nil {
748-
log.Printf("findMatchingFiles: glob error for pattern %q: %v", pattern, err)
749-
return []string{}
775+
return nil, fmt.Errorf("glob pattern error: %w", err)
750776
}
751777

752-
// Convert relative paths to absolute paths
778+
// Enforce match limit to prevent resource exhaustion
779+
if len(matches) > MaxGlobMatches {
780+
log.Printf("findMatchingFiles: pattern %q matched %d files, limiting to %d", pattern, len(matches), MaxGlobMatches)
781+
matches = matches[:MaxGlobMatches]
782+
}
783+
784+
// Convert relative paths to absolute paths and validate they stay within baseDir
753785
var absolutePaths []string
786+
baseDirAbs, err := filepath.Abs(baseDir)
787+
if err != nil {
788+
return nil, fmt.Errorf("failed to resolve baseDir: %w", err)
789+
}
790+
754791
for _, match := range matches {
755-
absolutePaths = append(absolutePaths, filepath.Join(baseDir, match))
792+
// Join and clean the path
793+
absPath := filepath.Join(baseDirAbs, match)
794+
absPath = filepath.Clean(absPath)
795+
796+
// Security: Ensure resolved path stays within baseDir (prevent directory traversal)
797+
relPath, err := filepath.Rel(baseDirAbs, absPath)
798+
if err != nil {
799+
log.Printf("findMatchingFiles: failed to compute relative path for %q: %v", absPath, err)
800+
continue
801+
}
802+
803+
// Check for directory traversal attempts (paths like "../" or starting with "../")
804+
if strings.HasPrefix(relPath, "..") {
805+
log.Printf("findMatchingFiles: rejected path traversal attempt: %q", absPath)
806+
continue
807+
}
808+
809+
absolutePaths = append(absolutePaths, absPath)
756810
}
757811

758-
return absolutePaths
812+
return absolutePaths, nil
759813
}
760814

761815
// findActiveWorkflowDir finds the active workflow directory for a session

components/frontend/src/app/globals.css

Lines changed: 27 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -121,10 +121,10 @@
121121
@apply bg-background text-foreground;
122122
}
123123

124-
/* Force light scrollbar styling */
124+
/* Scrollbar styling with dark mode support */
125125
* {
126126
scrollbar-width: thin;
127-
scrollbar-color: rgb(203 213 225) rgb(241 245 249);
127+
scrollbar-color: oklch(0.7 0 0) oklch(0.95 0 0);
128128
}
129129

130130
*::-webkit-scrollbar {
@@ -133,34 +133,50 @@
133133
}
134134

135135
*::-webkit-scrollbar-track {
136-
background: rgb(241 245 249);
136+
background: oklch(0.95 0 0);
137137
}
138138

139139
*::-webkit-scrollbar-thumb {
140-
background: rgb(203 213 225);
140+
background: oklch(0.7 0 0);
141141
border-radius: 4px;
142142
}
143143

144144
*::-webkit-scrollbar-thumb:hover {
145-
background: rgb(148 163 184);
145+
background: oklch(0.6 0 0);
146146
}
147147

148-
/* Force light mode for checkboxes in markdown */
148+
.dark * {
149+
scrollbar-color: oklch(0.4 0 0) oklch(0.2 0 0);
150+
}
151+
152+
.dark *::-webkit-scrollbar-track {
153+
background: oklch(0.2 0 0);
154+
}
155+
156+
.dark *::-webkit-scrollbar-thumb {
157+
background: oklch(0.4 0 0);
158+
}
159+
160+
.dark *::-webkit-scrollbar-thumb:hover {
161+
background: oklch(0.5 0 0);
162+
}
163+
164+
/* Checkbox styling for markdown - uses theme colors */
149165
article input[type="checkbox"] {
150166
appearance: none;
151167
-webkit-appearance: none;
152168
width: 1rem;
153169
height: 1rem;
154-
border: 1px solid rgb(209 213 219);
170+
border: 1px solid var(--border);
155171
border-radius: 0.25rem;
156-
background-color: white;
172+
background-color: var(--background);
157173
cursor: not-allowed;
158174
position: relative;
159175
}
160176

161177
article input[type="checkbox"]:checked {
162-
background-color: rgb(37 99 235);
163-
border-color: rgb(37 99 235);
178+
background-color: var(--primary);
179+
border-color: var(--primary);
164180
}
165181

166182
article input[type="checkbox"]:checked::after {
@@ -170,7 +186,7 @@
170186
top: 0.1rem;
171187
width: 0.375rem;
172188
height: 0.625rem;
173-
border: solid white;
189+
border: solid var(--primary-foreground);
174190
border-width: 0 2px 2px 0;
175191
transform: rotate(45deg);
176192
}

components/frontend/src/app/projects/[name]/sessions/[sessionName]/page.tsx

Lines changed: 73 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -82,6 +82,9 @@ export default function ProjectSessionDetailPage({
8282
const [openTabs, setOpenTabs] = useState<Array<{id: string; name: string; path: string; content: string}>>([{id: 'chat', name: 'Chat', path: '', content: ''}]);
8383
const [activeTab, setActiveTab] = useState<string>('chat');
8484

85+
// Maximum number of open tabs to prevent memory issues
86+
const MAX_TABS = 10;
87+
8588
// Directory browser state (unified for artifacts, repos, and workflow)
8689
const [selectedDirectory, setSelectedDirectory] = useState<DirectoryOption>({
8790
type: 'artifacts',
@@ -318,32 +321,48 @@ export default function ProjectSessionDetailPage({
318321
initializedFromSessionRef.current = true;
319322
}, [session, ootbWorkflows, workflowManagement]);
320323

324+
// Memoize results by path for efficient lookup
325+
const resultsByPath = useMemo(() => {
326+
if (!workflowResults?.results) return new Map<string, string>();
327+
const map = new Map<string, string>();
328+
workflowResults.results.forEach(r => {
329+
if (r.exists && r.content) {
330+
map.set(r.path, r.content);
331+
}
332+
});
333+
return map;
334+
}, [workflowResults?.results]);
335+
321336
// Sync open tabs with latest workflow results (auto-refresh tab content)
337+
// Only updates when content actually changes, not on every poll
322338
useEffect(() => {
323-
if (!workflowResults?.results) return;
339+
if (resultsByPath.size === 0) return;
324340

325341
setOpenTabs(prevTabs => {
326-
return prevTabs.map(tab => {
342+
let hasChanges = false;
343+
const updatedTabs = prevTabs.map(tab => {
327344
// Don't update chat tab
328345
if (tab.id === 'chat') return tab;
329346

330-
// Find matching result by path
331-
const matchingResult = workflowResults.results.find(r =>
332-
r.exists && r.path === tab.path
333-
);
347+
// Get latest content for this path
348+
const latestContent = resultsByPath.get(tab.path);
334349

335-
// Update tab content if we found newer data
336-
if (matchingResult?.content && matchingResult.content !== tab.content) {
350+
// Only update if content actually changed
351+
if (latestContent && latestContent !== tab.content) {
352+
hasChanges = true;
337353
return {
338354
...tab,
339-
content: matchingResult.content
355+
content: latestContent
340356
};
341357
}
342358

343359
return tab;
344360
});
361+
362+
// Only trigger state update if something actually changed
363+
return hasChanges ? updatedTabs : prevTabs;
345364
});
346-
}, [workflowResults]);
365+
}, [resultsByPath]);
347366

348367
// Compute directory options
349368
const directoryOptions = useMemo<DirectoryOption[]>(() => {
@@ -1029,15 +1048,47 @@ export default function ProjectSessionDetailPage({
10291048
)}
10301049
onClick={() => {
10311050
if (result.exists && result.content) {
1032-
const tabId = `result-${idx}`;
1033-
if (!openTabs.find(t => t.id === tabId)) {
1034-
setOpenTabs([...openTabs, {
1051+
// Use path-based stable ID instead of array index
1052+
const tabId = `result-${result.path}`;
1053+
1054+
// Check if tab already exists
1055+
const existingTab = openTabs.find(t => t.id === tabId);
1056+
if (existingTab) {
1057+
// Tab exists, just switch to it
1058+
setActiveTab(tabId);
1059+
return;
1060+
}
1061+
1062+
// Enforce tab limit - remove oldest non-chat tab if at limit
1063+
setOpenTabs(prevTabs => {
1064+
const nonChatTabs = prevTabs.filter(t => t.id !== 'chat');
1065+
1066+
let newTabs = [...prevTabs];
1067+
1068+
// If at limit, remove oldest non-chat tab
1069+
if (nonChatTabs.length >= MAX_TABS - 1) {
1070+
// Remove first non-chat tab (oldest)
1071+
const oldestTabId = nonChatTabs[0]?.id;
1072+
if (oldestTabId) {
1073+
newTabs = newTabs.filter(t => t.id !== oldestTabId);
1074+
// If we removed the active tab, switch to chat
1075+
if (activeTab === oldestTabId) {
1076+
setActiveTab('chat');
1077+
}
1078+
}
1079+
}
1080+
1081+
// Add new tab
1082+
newTabs.push({
10351083
id: tabId,
10361084
name: result.displayName,
10371085
path: result.path,
1038-
content: result.content
1039-
}]);
1040-
}
1086+
content: result.content || ''
1087+
});
1088+
1089+
return newTabs;
1090+
});
1091+
10411092
setActiveTab(tabId);
10421093
}
10431094
}}
@@ -1228,15 +1279,15 @@ export default function ProjectSessionDetailPage({
12281279
</div>
12291280

12301281
{isMarkdown ? (
1231-
<article className="prose prose-slate prose-headings:text-gray-900 prose-h1:text-4xl prose-h1:font-bold prose-h2:text-2xl prose-h2:font-semibold prose-h3:text-xl prose-h3:font-semibold prose-p:text-gray-700 prose-li:text-gray-700 prose-strong:text-gray-900 prose-a:text-blue-600 hover:prose-a:text-blue-800 prose-code:text-pink-600 prose-code:bg-gray-100 prose-code:px-1 prose-code:py-0.5 prose-code:rounded max-w-none">
1282+
<article className="prose prose-slate dark:prose-invert max-w-none">
12321283
<ReactMarkdown
12331284
remarkPlugins={[remarkGfm]}
12341285
components={{
1235-
h1: ({children}) => <h1 className="text-3xl font-bold mt-8 mb-4 text-gray-900 border-b pb-2">{children}</h1>,
1236-
h2: ({children}) => <h2 className="text-2xl font-semibold mt-6 mb-3 text-gray-900">{children}</h2>,
1237-
h3: ({children}) => <h3 className="text-xl font-semibold mt-4 mb-2 text-gray-800">{children}</h3>,
1238-
h4: ({children}) => <h4 className="text-lg font-semibold mt-3 mb-2 text-gray-800">{children}</h4>,
1239-
p: ({children}) => <p className="mb-4 text-gray-700 leading-7">{children}</p>,
1286+
h1: ({children}) => <h1 className="text-3xl font-bold mt-8 mb-4 text-gray-900 dark:text-gray-100 border-b pb-2">{children}</h1>,
1287+
h2: ({children}) => <h2 className="text-2xl font-semibold mt-6 mb-3 text-gray-900 dark:text-gray-100">{children}</h2>,
1288+
h3: ({children}) => <h3 className="text-xl font-semibold mt-4 mb-2 text-gray-800 dark:text-gray-200">{children}</h3>,
1289+
h4: ({children}) => <h4 className="text-lg font-semibold mt-3 mb-2 text-gray-800 dark:text-gray-200">{children}</h4>,
1290+
p: ({children}) => <p className="mb-4 text-gray-700 dark:text-gray-300 leading-7">{children}</p>,
12401291
a: ({href, children}) => <a href={href} className="text-blue-600 hover:text-blue-800 underline" target="_blank" rel="noopener noreferrer">{children}</a>,
12411292
ul: ({children, className}) => {
12421293
const isTaskList = className?.includes('contains-task-list');

0 commit comments

Comments
 (0)