diff --git a/src/skill-command.ts b/src/skill-command.ts index 79c1246..a830de2 100644 --- a/src/skill-command.ts +++ b/src/skill-command.ts @@ -15,7 +15,7 @@ import path from 'node:path'; import { unzipSync } from 'fflate'; import { ensureDir, remove, writeFile, pathExists } from './utils/fs.js'; -import { assertSafeResourceName } from './utils/path-safety.js'; +import { assertSafeResourceName, assertWithinRoot } from './utils/path-safety.js'; import { log } from './utils/logger.js'; /** Command types in the iWiki/clawpro contract. */ @@ -137,7 +137,6 @@ export async function installSkillZip( await remove(destRoot); await ensureDir(destRoot); - const resolvedRoot = path.resolve(destRoot); for (const [entryPath, bytes] of Object.entries(entries)) { // For a nested layout, only extract the chosen subtree; for a flat layout // (prefix === '') every entry belongs to the skill. @@ -148,9 +147,7 @@ export async function installSkillZip( const rel = prefix ? entryPath.slice(prefix.length) : entryPath; if (!rel) continue; const outPath = path.resolve(destRoot, rel); - if (outPath !== resolvedRoot && !outPath.startsWith(resolvedRoot + path.sep)) { - throw new Error(`path traversal detected in skill package: ${entryPath}`); - } + assertWithinRoot(destRoot, outPath, `path traversal detected in skill package: ${entryPath}`); await ensureDir(path.dirname(outPath)); await writeFile(outPath, Buffer.from(bytes).toString('utf-8')); } diff --git a/src/source-http.ts b/src/source-http.ts index 5c9f8d1..3dfc3c9 100644 --- a/src/source-http.ts +++ b/src/source-http.ts @@ -16,6 +16,7 @@ import path from 'node:path'; import { writeFile, remove, ensureDir } from './utils/fs.js'; +import { assertWithinRoot } from './utils/path-safety.js'; import { executeSkillCommand, type SkillCommand } from './skill-command.js'; import { log } from './utils/logger.js'; @@ -85,11 +86,8 @@ export async function fetchRepoSnapshot(baseUrl: string, apiKey: string): Promis * Throws if the resolved destination escapes localPath. */ async function writeRepoFile(localPath: string, file: RepoFile): Promise { - const resolvedRoot = path.resolve(localPath); const outPath = path.resolve(localPath, file.path); - if (outPath !== resolvedRoot && !outPath.startsWith(resolvedRoot + path.sep)) { - throw new Error(`path traversal detected in /repo file: ${file.path}`); - } + assertWithinRoot(localPath, outPath, `path traversal detected in /repo file: ${file.path}`); await ensureDir(path.dirname(outPath)); await writeFile(outPath, file.content); } diff --git a/src/status-report.ts b/src/status-report.ts index d716f41..0ea4133 100644 --- a/src/status-report.ts +++ b/src/status-report.ts @@ -26,6 +26,8 @@ import { pathExists, readFileSafe, ensureDir, + writeFile, + remove, } from './utils/fs.js'; import { resolveBaseDir, type LocalConfig, type TeamaiConfig } from './types.js'; import { getMachineId, deriveLocalAgentId } from './machine-id.js'; @@ -122,21 +124,18 @@ async function readSkillMeta(skillMdPath: string): Promise<{ name: string; versi */ export async function scanReportableSkills(skillsDir: string): Promise { if (!(await pathExists(skillsDir))) return []; - const dirs = await listDirs(skillsDir); - const skills: ReportedSkill[] = []; - for (const slug of dirs) { - if (slug.startsWith('.')) continue; - const skillMd = path.join(skillsDir, slug, 'SKILL.md'); - if (!(await pathExists(skillMd))) continue; - const meta = await readSkillMeta(skillMd); - skills.push({ - slug, - version: meta.version, - display_name: meta.name || slug, - }); - } - skills.sort((a, b) => a.slug.localeCompare(b.slug)); - return skills; + const dirs = (await listDirs(skillsDir)).filter((slug) => !slug.startsWith('.')); + const skills = await Promise.all( + dirs.map(async (slug): Promise => { + const skillMd = path.join(skillsDir, slug, 'SKILL.md'); + if (!(await pathExists(skillMd))) return null; + const meta = await readSkillMeta(skillMd); + return { slug, version: meta.version, display_name: meta.name || slug }; + }), + ); + return skills + .filter((s): s is ReportedSkill => s !== null) + .sort((a, b) => a.slug.localeCompare(b.slug)); } // ─── Offline queue ────────────────────────────────────────── @@ -177,11 +176,10 @@ async function flushQueue(apiKey: string): Promise { remaining.push(line); } } - const fse = await import('fs-extra'); if (remaining.length === 0) { - await fse.default.remove(p); + await remove(p); } else { - await fse.default.writeFile(p, remaining.join('\n') + '\n', 'utf-8'); + await writeFile(p, remaining.join('\n') + '\n'); } } diff --git a/src/team-push.ts b/src/team-push.ts index e6a7880..ddec132 100644 --- a/src/team-push.ts +++ b/src/team-push.ts @@ -4,7 +4,7 @@ import { readUsageEvents, truncateUsageAfterReport } from './usage-tracker.js'; import { aggregateUsage } from './stats.js'; import { readEvents, aggregateSessionInterventions } from './dashboard-collector.js'; import { createGit, pushRepoDirectly, pullRepo, resetToCleanMaster } from './utils/git.js'; -import { writeFile, readFileSafe, ensureDir, pathExists, listFiles } from './utils/fs.js'; +import { writeFile, readFileSafe, ensureDir, pathExists, listFiles, readJson, writeJson } from './utils/fs.js'; import { log } from './utils/logger.js'; import type { UserStats, UserInterventionStats } from './types.js'; import { VOTES_LOCAL_DIR } from './types.js'; @@ -111,21 +111,13 @@ function getReportedInterventionsPath(): string { } async function readReportedInterventions(): Promise { - try { - const content = await readFileSafe(getReportedInterventionsPath()); - if (!content) return {}; - const parsed = JSON.parse(content); - return parsed && typeof parsed === 'object' ? parsed : {}; - } catch { - return {}; - } + const parsed = await readJson(getReportedInterventionsPath()); + return parsed && typeof parsed === 'object' ? parsed : {}; } async function writeReportedInterventions(data: ReportedInterventions): Promise { try { - const p = getReportedInterventionsPath(); - await ensureDir(path.dirname(p)); - await writeFile(p, JSON.stringify(data)); + await writeJson(getReportedInterventionsPath(), data); } catch (e) { log.error(`Failed to persist reported interventions: ${(e as Error).message}`); } diff --git a/src/utils/path-safety.ts b/src/utils/path-safety.ts index 87c7d2a..89faa47 100644 --- a/src/utils/path-safety.ts +++ b/src/utils/path-safety.ts @@ -27,6 +27,28 @@ export function assertSafePath(target: string, allowedRoots: string[]): void { ); } +/** + * Assert that `candidate` stays within `root`, comparing resolved paths WITHOUT + * following symlinks on either side. + * + * Use this for "write a new file under root" guards: the candidate need not exist + * yet, and because neither side is symlink-resolved the check stays consistent + * even when `root` is reached through a symlink (e.g. macOS `/var` → `/private/var` + * tmpdirs). When symlink resolution IS required, use {@link assertSafePath}. + * + * @param root The directory the candidate must stay inside. + * @param candidate The path to validate. + * @param message Optional custom error message thrown on violation. + * @throws Error if `candidate` resolves outside `root`. + */ +export function assertWithinRoot(root: string, candidate: string, message?: string): void { + const resolvedRoot = path.resolve(root); + const resolvedCandidate = path.resolve(candidate); + if (resolvedCandidate !== resolvedRoot && !resolvedCandidate.startsWith(resolvedRoot + path.sep)) { + throw new Error(message ?? `path traversal detected: "${candidate}" is outside "${root}"`); + } +} + /** * Resolve a path to its real absolute form. *