From c5c3919c7863aa8a50505c89180cf65524bbe960 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E9=9B=B7=E7=94=B5=E8=8A=BD=E8=A1=A3?= Date: Thu, 16 Apr 2026 21:57:29 -0400 Subject: [PATCH] fix(skills): bound local skill walk with depth, file count, and symlink loop protection --- src/main/skills/import.ts | 130 ++++++++++++++++++++++++++++++-------- 1 file changed, 105 insertions(+), 25 deletions(-) diff --git a/src/main/skills/import.ts b/src/main/skills/import.ts index 09ac600a..d492574e 100644 --- a/src/main/skills/import.ts +++ b/src/main/skills/import.ts @@ -1,5 +1,5 @@ import { randomUUID } from "node:crypto"; -import { existsSync, readdirSync, readFileSync, realpathSync, statSync } from "node:fs"; +import { existsSync, readdirSync, readFileSync, realpathSync, statSync, type Dirent } from "node:fs"; import { mkdir, writeFile, rm } from "node:fs/promises"; import { join, resolve, basename, dirname, relative } from "node:path"; import type { AppDatabase } from "../database"; @@ -21,6 +21,9 @@ import { createGitHubImporter } from "./import-github"; const BLOCKED_PATH_PREFIXES_UNIX = ["/etc", "/var", "/proc", "/sys", "/dev", "/boot", "/sbin", "/usr/sbin"]; const BLOCKED_PATH_PREFIXES_WIN = ["C:\\Windows", "C:\\Program Files", "C:\\ProgramData"]; +const DEFAULT_SKILL_WALK_MAX_DEPTH = 8; +const DEFAULT_SKILL_WALK_MAX_FILES = 256; + function assertNotSystemPath(realPath: string): void { const normalized = realPath.replace(/\\/g, "/"); const blocked = process.platform === "win32" ? BLOCKED_PATH_PREFIXES_WIN : BLOCKED_PATH_PREFIXES_UNIX; @@ -312,48 +315,125 @@ export function createSkillImporter({ db, profileDir: _profileDir, logger, skill return results; } - function findSkillMdFiles(rootDir: string): string[] { + function findSkillMdFiles( + rootDir: string, + opts?: { maxDepth?: number; maxFiles?: number }, + ): string[] { + const maxDepth = opts?.maxDepth ?? DEFAULT_SKILL_WALK_MAX_DEPTH; + const maxFiles = opts?.maxFiles ?? DEFAULT_SKILL_WALK_MAX_FILES; const found: string[] = []; - function walk(dir: string) { + const visitedRealPaths = new Set(); + let filesSeen = 0; + + function walk(dir: string, depth: number) { + if (depth > maxDepth) { + throw new Error( + `Skill directory too deep, refusing to walk beyond ${maxDepth} levels`, + ); + } + let realDir: string; try { - const entries = readdirSync(dir, { withFileTypes: true }); - for (const entry of entries) { - const fullPath = join(dir, entry.name); - if (entry.isDirectory()) { - walk(fullPath); - } else if (entry.name === "SKILL.md") { - found.push(fullPath); - } - } + realDir = realpathSync(dir); } catch { - // Skip unreadable directories + return; + } + if (visitedRealPaths.has(realDir)) { + return; + } + visitedRealPaths.add(realDir); + + let entries: Dirent[]; + try { + entries = readdirSync(dir, { withFileTypes: true }); + } catch { + return; + } + for (const entry of entries) { + filesSeen++; + if (filesSeen > maxFiles) { + throw new Error( + `Skill directory contains too many files, refusing after ${maxFiles} entries`, + ); + } + const fullPath = join(dir, entry.name); + if (entry.isDirectory()) { + walk(fullPath, depth + 1); + } else if (entry.name === "SKILL.md") { + found.push(fullPath); + } } } - walk(rootDir); + walk(rootDir, 0); return found; } - function buildLocalInventory(skillDir: string, skillMdPath: string): FileInventoryEntry[] { + function buildLocalInventory( + skillDir: string, + skillMdPath: string, + opts?: { maxDepth?: number; maxFiles?: number }, + ): FileInventoryEntry[] { + const maxDepth = opts?.maxDepth ?? DEFAULT_SKILL_WALK_MAX_DEPTH; + const maxFiles = opts?.maxFiles ?? DEFAULT_SKILL_WALK_MAX_FILES; const inventory: FileInventoryEntry[] = [ { path: "SKILL.md", kind: "skill" }, ]; - try { - const entries = readdirSync(skillDir, { withFileTypes: true, recursive: true }); + const visitedRealPaths = new Set(); + let filesSeen = 0; + + function walk(dir: string, depth: number) { + if (depth > maxDepth) { + throw new Error( + `Skill directory too deep, refusing to walk beyond ${maxDepth} levels`, + ); + } + let realDir: string; + try { + realDir = realpathSync(dir); + } catch { + return; + } + if (visitedRealPaths.has(realDir)) { + return; + } + visitedRealPaths.add(realDir); + + let entries: Dirent[]; + try { + entries = readdirSync(dir, { withFileTypes: true }); + } catch { + return; + } for (const entry of entries) { + filesSeen++; + if (filesSeen > maxFiles) { + throw new Error( + `Skill directory contains too many files, refusing after ${maxFiles} entries`, + ); + } + const fullPath = join(dir, entry.name); + if (entry.isDirectory()) { + walk(fullPath, depth + 1); + continue; + } if (!entry.isFile()) continue; - const entryPath = entry.parentPath - ? join(entry.parentPath, entry.name) - : join(skillDir, entry.name); - const fullPath = resolve(entryPath); - if (fullPath === skillMdPath) continue; - const relativePath = relative(skillDir, fullPath).replace(/\\/g, "/"); + const resolvedPath = resolve(fullPath); + if (resolvedPath === skillMdPath) continue; + const relativePath = relative(skillDir, resolvedPath).replace(/\\/g, "/"); inventory.push({ path: relativePath, kind: classifyInventoryKind(relativePath), }); } - } catch { - // If directory listing fails, return just the SKILL.md entry + } + + try { + walk(skillDir, 0); + } catch (err) { + // Re-throw limit errors so caller can surface them; silently ignore others. + const msg = err instanceof Error ? err.message : String(err); + if (msg.startsWith("Skill directory too deep") || msg.startsWith("Skill directory contains too many files")) { + throw err; + } } return inventory; }