refactor: dedup path-traversal guard, reuse fs/json helpers

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'));
   }

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<void> {
-  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);
 }

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<ReportedSkill[]> {
   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<ReportedSkill | null> => {
+      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<void> {
       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');
   }
 }
 

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<ReportedInterventions> {
-  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<ReportedInterventions>(getReportedInterventionsPath());
+  return parsed && typeof parsed === 'object' ? parsed : {};
 }
 
 async function writeReportedInterventions(data: ReportedInterventions): Promise<void> {
   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}`);
   }

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.
  *