Write toolkit generator output safely

toolkit-docs-generator/src/cli/index.ts

@@ -13,7 +13,7 @@
 
 import chalk from "chalk";
 import { Command } from "commander";
-import { readdir, readFile, rm } from "fs/promises";
+import { readdir, readFile } from "fs/promises";
 import ora from "ora";
 import { join, resolve } from "path";
 import {
@@ -53,6 +53,10 @@ import {
 } from "../sources/toolkit-data-source.js";
 import { readExclusionList } from "../utils/exclusion-list.js";
 import { readIgnoreList } from "../utils/ignore-list.js";
+import {
+  clearSafeOutputDir,
+  resolveDefaultOutputDir,
+} from "../utils/output-dir.js";
 import {
   createProgressTracker,
   formatToolkitComplete,
@@ -118,17 +122,6 @@ const parseProviders = (input: string): ProviderVersion[] => {
  */
 const getDefaultMockDataDir = (): string => join(process.cwd(), "mock-data");
 
-/**
- * Get the default output directory for docs JSON.
- */
-const getDefaultOutputDir = (): string => {
-  const cwd = process.cwd();
-  if (cwd.endsWith("toolkit-docs-generator")) {
-    return resolve(cwd, "..", "data", "toolkits");
-  }
-  return resolve(cwd, "data", "toolkits");
-};
-
 const getDefaultVerificationDir = (): string => {
   const cwd = process.cwd();
   if (cwd.endsWith("toolkit-docs-generator")) {
@@ -265,12 +258,7 @@ const clearOutputDir = async (
   outputDir: string,
   verbose: boolean
 ): Promise<void> => {
-  const resolvedDir = resolve(outputDir);
-  const repoRoot = resolve(process.cwd());
-  if (resolvedDir === "/" || resolvedDir === repoRoot) {
-    throw new Error(`Refusing to overwrite output directory: ${resolvedDir}`);
-  }
-  await rm(resolvedDir, { recursive: true, force: true });
+  const resolvedDir = await clearSafeOutputDir(outputDir);
   if (verbose) {
     console.log(chalk.dim(`Cleared output directory: ${resolvedDir}`));
   }
@@ -878,7 +866,7 @@ program
     "Path to failed tools report to rerun only impacted toolkits"
   )
   .option("--all", "Generate documentation for all toolkits", false)
-  .option("-o, --output <dir>", "Output directory", getDefaultOutputDir())
+  .option("-o, --output <dir>", "Output directory", resolveDefaultOutputDir())
   .option("--log-dir <dir>", "Directory for run logs", getDefaultLogDir())
   .option("--mock-data-dir <dir>", "Path to mock data directory")
   .option("--metadata-file <file>", "Path to metadata JSON file")
@@ -1924,7 +1912,7 @@ program
 program
   .command("generate-all")
   .description("Generate documentation for all toolkits in mock data")
-  .option("-o, --output <dir>", "Output directory", getDefaultOutputDir())
+  .option("-o, --output <dir>", "Output directory", resolveDefaultOutputDir())
   .option("--mock-data-dir <dir>", "Path to mock data directory")
   .option("--metadata-file <file>", "Path to metadata JSON file")
   .option(
@@ -2627,7 +2615,7 @@ program
 program
   .command("verify-output")
   .description("Verify output directory structure and schema")
-  .option("-o, --output <dir>", "Output directory", getDefaultOutputDir())
+  .option("-o, --output <dir>", "Output directory", resolveDefaultOutputDir())
   .option("--verbose", "Show detailed progress", false)
   .action(async (options: { output: string; verbose: boolean }) => {
     const spinner = ora("Verifying output...").start();
@@ -2667,7 +2655,7 @@ program
   .option(
     "-o, --output <dir>",
     "Previous output directory",
-    getDefaultOutputDir()
+    resolveDefaultOutputDir()
   )
   .option("--log-dir <dir>", "Directory for run logs", getDefaultLogDir())
   .option("--mock-data-dir <dir>", "Path to mock data directory")

toolkit-docs-generator/src/generator/json-generator.ts

@@ -3,7 +3,8 @@
  *
  * Outputs merged toolkit data as JSON files.
  */
-import { mkdir, readFile, stat, writeFile } from "fs/promises";
+import { randomUUID } from "node:crypto";
+import { mkdir, readFile, rename, rm, stat, writeFile } from "fs/promises";
 import { dirname, join } from "path";
 import { parsePreviousToolkitForDiff } from "../diff/previous-output.js";
 import type {
@@ -17,6 +18,72 @@ import {
   type ToolkitReadResult,
 } from "./output-verifier.js";
 
+const SAFE_TOOLKIT_ID = /^[a-z0-9][a-z0-9_-]*$/i;
+const RESERVED_TOOLKIT_ID = "index";
+
+const getToolkitFileName = (toolkitId: string): string => {
+  const normalizedId = toolkitId.toLowerCase();
+  if (normalizedId === RESERVED_TOOLKIT_ID) {
+    throw new Error(`"${toolkitId}" is a reserved toolkit ID`);
+  }
+  if (!SAFE_TOOLKIT_ID.test(toolkitId)) {
+    throw new Error(`Unsafe toolkit ID: "${toolkitId}"`);
+  }
+  return `${normalizedId}.json`;
+};
+
+const validateToolkitFileNames = (
+  toolkits: readonly MergedToolkit[]
+): string[] => {
+  const errors: string[] = [];
+  const toolkitIdsByFile = new Map<string, string[]>();
+
+  for (const toolkit of toolkits) {
+    try {
+      const fileName = getToolkitFileName(toolkit.id);
+      const toolkitIds = toolkitIdsByFile.get(fileName) ?? [];
+      toolkitIdsByFile.set(fileName, [...toolkitIds, toolkit.id]);
+    } catch (error) {
+      errors.push(error instanceof Error ? error.message : String(error));
+    }
+  }
+
+  for (const [fileName, toolkitIds] of toolkitIdsByFile) {
+    if (toolkitIds.length > 1) {
+      errors.push(
+        `Toolkit IDs collide on ${fileName}: ${toolkitIds.join(", ")}`
+      );
+    }
+  }
+
+  return errors;
+};
+
+const compareToolkitIds = (left: string, right: string): number => {
+  const normalizedLeft = left.toLowerCase();
+  const normalizedRight = right.toLowerCase();
+  if (normalizedLeft !== normalizedRight) {
+    return normalizedLeft < normalizedRight ? -1 : 1;
+  }
+  if (left === right) {
+    return 0;
+  }
+  return left < right ? -1 : 1;
+};
+
+const writeFileAtomically = async (
+  filePath: string,
+  content: string
+): Promise<void> => {
+  const tempPath = `${filePath}.${process.pid}.${randomUUID()}.tmp`;
+  try {
+    await writeFile(tempPath, content, "utf-8");
+    await rename(tempPath, filePath);
+  } finally {
+    await rm(tempPath, { force: true });
+  }
+};
+
 // ============================================================================
 // Generator Configuration
 // ============================================================================
@@ -63,6 +130,7 @@ export class JsonGenerator {
   private readonly generateIndex: boolean;
   private readonly indexSource: "current" | "output";
   private readonly validateOutput: boolean;
+  private readonly toolkitIdsByFile = new Map<string, string>();
 
   constructor(config: JsonGeneratorConfig) {
     this.outputDir = config.outputDir;
@@ -86,28 +154,44 @@ export class JsonGenerator {
       }
     }
 
-    const fileName = `${toolkit.id.toLowerCase()}.json`;
+    const fileName = getToolkitFileName(toolkit.id);
     const filePath = join(this.outputDir, fileName);
+    const previousToolkitId = this.toolkitIdsByFile.get(fileName);
+    if (previousToolkitId && previousToolkitId !== toolkit.id) {
+      throw new Error(
+        `Toolkit IDs collide on ${fileName}: ${previousToolkitId}, ${toolkit.id}`
+      );
+    }
+    this.toolkitIdsByFile.set(fileName, toolkit.id);
 
     // Ensure directory exists
-    await mkdir(dirname(filePath), { recursive: true });
+    try {
+      await mkdir(dirname(filePath), { recursive: true });
 
-    // Write file
-    const content = this.prettyPrint
-      ? JSON.stringify(toolkit, null, 2)
-      : JSON.stringify(toolkit);
+      // Write file
+      const content = this.prettyPrint
+        ? JSON.stringify(toolkit, null, 2)
+        : JSON.stringify(toolkit);
 
-    await writeFile(filePath, content, "utf-8");
-    return filePath;
+      await writeFileAtomically(filePath, content);
+      return filePath;
+    } catch (error) {
+      if (previousToolkitId) {
+        this.toolkitIdsByFile.set(fileName, previousToolkitId);
+      } else {
+        this.toolkitIdsByFile.delete(fileName);
+      }
+      throw error;
+    }
   }
 
   /**
    * Check if a toolkit file already exists in the output directory
    */
   async hasToolkitFile(toolkitId: string): Promise<boolean> {
-    const fileName = `${toolkitId.toLowerCase()}.json`;
-    const filePath = join(this.outputDir, fileName);
     try {
+      const fileName = getToolkitFileName(toolkitId);
+      const filePath = join(this.outputDir, fileName);
       const stats = await stat(filePath);
       return stats.isFile();
     } catch {
@@ -137,9 +221,9 @@ export class JsonGenerator {
    * Load an existing toolkit file
    */
   async loadToolkitFile(toolkitId: string): Promise<MergedToolkit | null> {
-    const fileName = `${toolkitId.toLowerCase()}.json`;
-    const filePath = join(this.outputDir, fileName);
     try {
+      const fileName = getToolkitFileName(toolkitId);
+      const filePath = join(this.outputDir, fileName);
       const content = await readFile(filePath, "utf-8");
       const parsed = JSON.parse(content) as unknown;
       const result = MergedToolkitSchema.safeParse(parsed);
@@ -160,7 +244,10 @@ export class JsonGenerator {
     toolkits: readonly MergedToolkit[]
   ): Promise<GeneratorResult> {
     const filesWritten: string[] = [];
-    const errors: string[] = [];
+    const errors = validateToolkitFileNames(toolkits);
+    if (errors.length > 0) {
+      return { filesWritten, errors };
+    }
 
     // Generate per-toolkit files
     for (const toolkit of toolkits) {
@@ -199,15 +286,17 @@ export class JsonGenerator {
   private async generateIndexFile(
     toolkits: readonly MergedToolkit[]
   ): Promise<string> {
-    const entries: ToolkitIndexEntry[] = toolkits.map((t) => ({
-      id: t.id,
-      label: t.label,
-      version: t.version,
-      category: t.metadata.category,
-      type: t.metadata.type,
-      toolCount: t.tools.length,
-      authType: t.auth?.type ?? "none",
-    }));
+    const entries: ToolkitIndexEntry[] = toolkits
+      .map((t) => ({
+        id: t.id,
+        label: t.label,
+        version: t.version,
+        category: t.metadata.category,
+        type: t.metadata.type,
+        toolCount: t.tools.length,
+        authType: t.auth?.type ?? "none",
+      }))
+      .sort((left, right) => compareToolkitIds(left.id, right.id));
 
     const index: ToolkitIndex = {
       generatedAt: new Date().toISOString(),
@@ -223,7 +312,7 @@ export class JsonGenerator {
       ? JSON.stringify(index, null, 2)
       : JSON.stringify(index);
 
-    await writeFile(filePath, content, "utf-8");
+    await writeFileAtomically(filePath, content);
     return filePath;
   }
 

toolkit-docs-generator/src/utils/output-dir.ts

@@ -1,11 +1,27 @@
-import { realpath } from "fs/promises";
-import { isAbsolute, resolve, sep } from "path";
+import { realpath, rm } from "fs/promises";
+import { basename, isAbsolute, resolve, sep } from "path";
 
 type ResolveOptions = {
   repoRoot?: string;
   homeDir?: string;
 };
 
+export const resolveRepositoryRoot = (cwd: string = process.cwd()): string =>
+  basename(cwd) === "toolkit-docs-generator"
+    ? resolve(cwd, "..")
+    : resolve(cwd);
+
+export const resolveDefaultOutputDir = (
+  cwd: string = process.cwd()
+): string => {
+  const repoRoot = resolveRepositoryRoot(cwd);
+  const generatorRoot =
+    basename(cwd) === "toolkit-docs-generator"
+      ? cwd
+      : resolve(repoRoot, "toolkit-docs-generator");
+  return resolve(generatorRoot, "data", "toolkits");
+};
+
 const isSubpath = (parent: string, child: string): boolean => {
   const normalizedParent = parent.endsWith(sep) ? parent : `${parent}${sep}`;
   return child === parent || child.startsWith(normalizedParent);
@@ -15,10 +31,16 @@ export const resolveSafeOutputDir = async (
   outputDir: string,
   options: ResolveOptions = {}
 ): Promise<string> => {
-  const resolvedRepoRoot = resolve(options.repoRoot ?? process.cwd());
+  const resolvedRepoRoot = resolve(
+    options.repoRoot ?? resolveRepositoryRoot(process.cwd())
+  );
   const repoRoot = await realpath(resolvedRepoRoot).catch(
     () => resolvedRepoRoot
   );
+  const resolvedGeneratorRoot = resolve(repoRoot, "toolkit-docs-generator");
+  const generatorRoot = await realpath(resolvedGeneratorRoot).catch(
+    () => resolvedGeneratorRoot
+  );
   const resolvedHomeDir = options.homeDir
     ? resolve(options.homeDir)
     : process.env.HOME
@@ -30,12 +52,10 @@ export const resolveSafeOutputDir = async (
   const resolvedDir = resolve(outputDir);
   const realDir = await realpath(resolvedDir).catch(() => resolvedDir);
 
-  const forbidden = new Set<string>(["/", repoRoot]);
-  if (homeDir) {
-    forbidden.add(homeDir);
-  }
-
-  if (forbidden.has(realDir)) {
+  const containsRepoRoot = isSubpath(realDir, repoRoot);
+  const containsGeneratorRoot = isSubpath(realDir, generatorRoot);
+  const containsHomeDir = homeDir ? isSubpath(realDir, homeDir) : false;
+  if (containsRepoRoot || containsGeneratorRoot || containsHomeDir) {
     throw new Error(`Refusing to delete unsafe output directory: ${realDir}`);
   }
 
@@ -48,3 +68,12 @@ export const resolveSafeOutputDir = async (
 
   return realDir;
 };
+
+export const clearSafeOutputDir = async (
+  outputDir: string,
+  options: ResolveOptions = {}
+): Promise<string> => {
+  const safeDir = await resolveSafeOutputDir(outputDir, options);
+  await rm(resolve(outputDir), { recursive: true, force: true });
+  return safeDir;
+};