fix(security): keep paths on a short leash

.changeset/tidy-path-leash.md

@@ -0,0 +1,5 @@
+---
+"@fission-ai/openspec": patch
+---
+
+Keep generated files, specs, archive moves, and local state inside their intended security boundaries without breaking linked monorepo workflows.

src/commands/change.ts

@@ -9,6 +9,7 @@ import type { RootOutput } from '../core/root-selection.js';
 import { isInteractive } from '../utils/interactive.js';
 import { getActiveChangeIds } from '../utils/item-discovery.js';
 import { getTaskProgressForChange } from '../utils/task-progress.js';
+import { FileSystemUtils } from '../utils/file-system.js';
 
 /**
  * True only when `target` is definitively absent. An EACCES or I/O failure
@@ -106,15 +107,18 @@ export class ChangeCommand {
       }
       throw new Error(`Change "${changeName}" not found at ${proposalPath}`);
     }
+    FileSystemUtils.assertPathWithin(path.dirname(proposalPath), proposalPath);
 
     if (options?.json) {
+      FileSystemUtils.assertPathWithin(changeDir, proposalPath);
       const jsonOutput = await this.converter.convertChangeToJson(proposalPath);
 
       if (options.requirementsOnly) {
         console.error('Flag --requirements-only is deprecated; use --deltas-only instead.');
       }
 
       const parsed: Change = JSON.parse(jsonOutput);
+      FileSystemUtils.assertPathWithin(changeDir, proposalPath);
       const contentForTitle = await fs.readFile(proposalPath, 'utf-8');
       const title = this.extractTitle(contentForTitle, changeName);
       const id = parsed.name;
@@ -129,6 +133,7 @@ export class ChangeCommand {
       };
       console.log(JSON.stringify(output, null, 2));
     } else {
+      FileSystemUtils.assertPathWithin(changeDir, proposalPath);
       const content = await fs.readFile(proposalPath, 'utf-8');
       console.log(content);
     }
@@ -168,6 +173,7 @@ export class ChangeCommand {
           }
 
           try {
+            FileSystemUtils.assertPathWithin(changeDir, proposalPath);
             const content = await fs.readFile(proposalPath, 'utf-8');
             const parser = new ChangeParser(content, changeDir);
             const change = await parser.parseChangeWithDeltas(changeName);
@@ -209,6 +215,7 @@ export class ChangeCommand {
           continue;
         }
         try {
+          FileSystemUtils.assertPathWithin(changeDir, proposalPath);
           const content = await fs.readFile(proposalPath, 'utf-8');
           const title = this.extractTitle(content, changeName);
           const parser = new ChangeParser(content, changeDir);
@@ -248,7 +255,9 @@ export class ChangeCommand {
     }
 
     const changeDir = path.join(changesPath, changeName);
-
+    if (!isChangeDirectoryName(changesPath, changeDir)) {
+      throw new Error(`Change "${changeName}" not found at ${changeDir}`);
+    }
     try {
       await fs.access(changeDir);
     } catch {

src/commands/schema.ts

@@ -13,6 +13,7 @@ import {
 } from '../core/artifact-graph/resolver.js';
 import { parseSchema, SchemaValidationError } from '../core/artifact-graph/schema.js';
 import type { SchemaYaml, Artifact } from '../core/artifact-graph/types.js';
+import { FileSystemUtils } from '../utils/file-system.js';
 
 /**
  * Schema source location type
@@ -196,22 +197,31 @@ function validateSchema(
     return { valid: false, issues };
   }
 
-  // Check template files exist
-  // Templates can be in schemaDir directly or in a templates/ subdirectory
+  // Check template files exist in the same directory used at runtime.
   if (verbose) {
     console.log('  Checking template files...');
   }
   for (const artifact of schema.artifacts) {
-    // Try templates subdirectory first (standard location), then root
-    const templatePathInTemplates = path.join(schemaDir, 'templates', artifact.template);
-    const templatePathInRoot = path.join(schemaDir, artifact.template);
+    const templatesDir = path.join(schemaDir, 'templates');
+    const existingTemplatePath = path.join(templatesDir, artifact.template);
 
-    if (!fs.existsSync(templatePathInTemplates) && !fs.existsSync(templatePathInRoot)) {
+    if (!fs.existsSync(existingTemplatePath)) {
       issues.push({
         level: 'error',
         path: `artifacts.${artifact.id}.template`,
         message: `Template file '${artifact.template}' not found for artifact '${artifact.id}'`,
       });
+      continue;
+    }
+
+    try {
+      FileSystemUtils.assertPathWithin(templatesDir, existingTemplatePath);
+    } catch {
+      issues.push({
+        level: 'error',
+        path: `artifacts.${artifact.id}.template`,
+        message: `Template file '${artifact.template}' points outside the schema templates directory`,
+      });
     }
   }
 
@@ -234,19 +244,83 @@ function isValidSchemaName(name: string): boolean {
 /**
  * Copy a directory recursively.
  */
-function copyDirRecursive(src: string, dest: string): void {
+function resolveSchemaCopyPath(allowedRoot: string, sourcePath: string): string {
+  try {
+    const canonicalRoot = fs.realpathSync(allowedRoot);
+    const canonicalPath = fs.realpathSync(sourcePath);
+    FileSystemUtils.assertPathWithin(canonicalRoot, canonicalPath);
+    return canonicalPath;
+  } catch (error) {
+    const detail = error instanceof Error ? error.message : String(error);
+    throw new Error(
+      `Cannot fork schema with linked or unsupported entry: ${sourcePath}: ${detail}`,
+      { cause: error }
+    );
+  }
+}
+
+function copyDirRecursive(
+  src: string,
+  dest: string,
+  allowedRoot = src,
+  ancestors = new Set<string>()
+): void {
+  const canonicalSrc = resolveSchemaCopyPath(allowedRoot, src);
+  if (ancestors.has(canonicalSrc)) {
+    throw new Error(`Cannot fork schema with a linked directory cycle: ${src}`);
+  }
+  ancestors.add(canonicalSrc);
   fs.mkdirSync(dest, { recursive: true });
 
-  const entries = fs.readdirSync(src, { withFileTypes: true });
-  for (const entry of entries) {
-    const srcPath = path.join(src, entry.name);
-    const destPath = path.join(dest, entry.name);
+  try {
+    const entries = fs.readdirSync(src, { withFileTypes: true });
+    for (const entry of entries) {
+      const srcPath = path.join(src, entry.name);
+      const destPath = path.join(dest, entry.name);
+      const canonicalEntry = resolveSchemaCopyPath(allowedRoot, srcPath);
+      const stats = fs.statSync(canonicalEntry);
+
+      if (stats.isDirectory()) {
+        copyDirRecursive(canonicalEntry, destPath, allowedRoot, ancestors);
+      } else if (stats.isFile()) {
+        // Dereference confined links so the fork is an independent schema.
+        fs.copyFileSync(canonicalEntry, destPath);
+      } else {
+        throw new Error(`Cannot fork schema with linked or unsupported entry: ${srcPath}`);
+      }
+    }
+  } finally {
+    ancestors.delete(canonicalSrc);
+  }
+}
+
+/**
+ * Verifies a schema tree before replacing or creating the fork destination.
+ */
+function assertSchemaTreeCanBeCopied(
+  src: string,
+  allowedRoot = src,
+  ancestors = new Set<string>()
+): void {
+  const canonicalSrc = resolveSchemaCopyPath(allowedRoot, src);
+  if (ancestors.has(canonicalSrc)) {
+    throw new Error(`Cannot fork schema with a linked directory cycle: ${src}`);
+  }
+  ancestors.add(canonicalSrc);
 
-    if (entry.isDirectory()) {
-      copyDirRecursive(srcPath, destPath);
-    } else {
-      fs.copyFileSync(srcPath, destPath);
+  try {
+    for (const entry of fs.readdirSync(src, { withFileTypes: true })) {
+      const entryPath = path.join(src, entry.name);
+      const canonicalEntry = resolveSchemaCopyPath(allowedRoot, entryPath);
+      const stats = fs.statSync(canonicalEntry);
+      if (stats.isDirectory()) {
+        assertSchemaTreeCanBeCopied(canonicalEntry, allowedRoot, ancestors);
+      } else if (!stats.isFile()) {
+        throw new Error(`Cannot fork schema with linked or unsupported entry: ${entryPath}`);
+      }
     }
+  } finally {
+    ancestors.delete(canonicalSrc);
   }
 }
 
@@ -481,10 +555,10 @@ export function registerSchemaCommand(program: Command): void {
                 console.log(`    ${issue.level}: ${issue.message}`);
               }
             }
+          }
 
-            if (anyInvalid) {
-              process.exitCode = 1;
-            }
+          if (anyInvalid) {
+            process.exitCode = 1;
           }
           return;
         }
@@ -529,9 +603,11 @@ export function registerSchemaCommand(program: Command): void {
             for (const issue of result.issues) {
               console.log(`  ${issue.level}: ${issue.message}`);
             }
-            process.exitCode = 1;
           }
         }
+        if (!result.valid) {
+          process.exitCode = 1;
+        }
       } catch (error) {
         if (options?.json) {
           console.log(JSON.stringify({
@@ -595,6 +671,10 @@ export function registerSchemaCommand(program: Command): void {
         const sourceResolution = getSchemaResolution(source, projectRoot);
         const sourceLocation = sourceResolution?.source || 'package';
 
+        // Validate the complete source before a forced fork removes anything.
+        const trustedSourceDir = fs.realpathSync(sourceDir);
+        assertSchemaTreeCanBeCopied(trustedSourceDir);
+
         // Check destination
         const destinationDir = path.join(getProjectSchemasDir(projectRoot), destinationName);
 
@@ -621,7 +701,7 @@ export function registerSchemaCommand(program: Command): void {
 
         // Copy schema
         if (spinner) spinner.start(`Forking '${source}' to '${destinationName}'...`);
-        copyDirRecursive(sourceDir, destinationDir);
+        copyDirRecursive(trustedSourceDir, destinationDir);
 
         // Update name in schema.yaml
         const destSchemaPath = path.join(destinationDir, 'schema.yaml');

src/commands/spec.ts

@@ -1,16 +1,37 @@
 import { program } from 'commander';
 import { existsSync, readFileSync } from 'fs';
-import { join } from 'path';
+import path, { join } from 'path';
 import { MarkdownParser } from '../core/parsers/markdown-parser.js';
 import { Validator } from '../core/validation/validator.js';
 import type { Spec } from '../core/schemas/index.js';
 import type { RootOutput } from '../core/root-selection.js';
 import { isInteractive } from '../utils/interactive.js';
 import { getSpecIds } from '../utils/item-discovery.js';
 import { discoverSpecFiles } from '../utils/spec-discovery.js';
+import { FileSystemUtils } from '../utils/file-system.js';
 
 const SPECS_DIR = 'openspec/specs';
 
+function assertSpecPath(specsDir: string, specPath: string): void {
+  const relativePath = path.relative(path.resolve(specsDir), path.resolve(specPath));
+  if (
+    relativePath === '..' ||
+    relativePath.startsWith(`..${path.sep}`) ||
+    path.isAbsolute(relativePath)
+  ) {
+    throw new Error(`Path is outside the allowed directory: ${specPath}`);
+  }
+
+  try {
+    // Preserve confined spec.md links, including links to a sibling capability.
+    FileSystemUtils.assertPathWithin(specsDir, specPath);
+  } catch {
+    // A capability directory may intentionally be a monorepo symlink. Treat it
+    // as the trust root while still rejecting a link outside that capability.
+    FileSystemUtils.assertPathWithin(path.dirname(specPath), specPath);
+  }
+}
+
 interface ShowOptions {
   json?: boolean;
   // JSON-only filters (raw-first text has no filters)
@@ -21,7 +42,8 @@ interface ShowOptions {
   rootOutput?: RootOutput;
 }
 
-function parseSpecFromFile(specPath: string, specId: string): Spec {
+function parseSpecFromFile(specsDir: string, specPath: string, specId: string): Spec {
+  assertSpecPath(specsDir, specPath);
   const content = readFileSync(specPath, 'utf-8');
   const parser = new MarkdownParser(content);
   return parser.parseSpec(specId);
@@ -62,7 +84,8 @@ function filterSpec(spec: Spec, options: ShowOptions): Spec {
  * Print the raw markdown content for a spec file without any formatting.
  * Raw-first behavior ensures text mode is a passthrough for deterministic output.
  */
-function printSpecTextRaw(specPath: string): void {
+function printSpecTextRaw(specsDir: string, specPath: string): void {
+  assertSpecPath(specsDir, specPath);
   const content = readFileSync(specPath, 'utf-8');
   console.log(content);
 }
@@ -94,6 +117,7 @@ export class SpecCommand {
     }
 
     const specPath = join(this.specsDir, specId, 'spec.md');
+    assertSpecPath(this.specsDir, specPath);
     if (!existsSync(specPath)) {
       // Root-aware callers get the absolute path; the cwd-based noun form
       // keeps its historical forward-slash relative message on all platforms.
@@ -105,7 +129,7 @@ export class SpecCommand {
       if (options.requirements && options.requirement) {
         throw new Error('Options --requirements and --requirement cannot be used together');
       }
-      const parsed = parseSpecFromFile(specPath, specId);
+      const parsed = parseSpecFromFile(this.specsDir, specPath, specId);
       const filtered = filterSpec(parsed, options);
       const output = {
         id: specId,
@@ -119,7 +143,7 @@ export class SpecCommand {
       console.log(JSON.stringify(output, null, 2));
       return;
     }
-    printSpecTextRaw(specPath);
+    printSpecTextRaw(this.specsDir, specPath);
   }
 }
 
@@ -167,7 +191,8 @@ export function registerSpecCommand(rootProgram: typeof program) {
         const specs = discovered
           .map(({ id, specFile }) => {
             try {
-              const spec = parseSpecFromFile(specFile, id);
+              assertSpecPath(SPECS_DIR, specFile);
+              const spec = parseSpecFromFile(SPECS_DIR, specFile, id);
 
               return {
                 id,
@@ -228,12 +253,14 @@ export function registerSpecCommand(rootProgram: typeof program) {
         }
 
         const specPath = join(SPECS_DIR, specId, 'spec.md');
+        assertSpecPath(SPECS_DIR, specPath);
 
         if (!existsSync(specPath)) {
           throw new Error(`Spec '${specId}' not found at openspec/specs/${specId}/spec.md`);
         }
 
         const validator = new Validator(options.strict);
+        assertSpecPath(SPECS_DIR, specPath);
         const report = await validator.validateSpec(specPath);
 
         if (options.json) {

src/commands/workflow/instructions.ts

@@ -12,6 +12,7 @@ import {
   loadChangeContext,
   generateInstructions,
   resolveSchema,
+  resolveArtifactOutputPath,
   resolveArtifactOutputs,
   type ArtifactInstructions,
 } from '../../core/artifact-graph/index.js';
@@ -415,7 +416,7 @@ export async function generateApplyInstructions(
   let parsedTasks: ParsedTask[] = [];
   let tracksFileExists = false;
   if (tracksFile) {
-    const tracksPath = path.join(changeDir, tracksFile);
+    const tracksPath = resolveArtifactOutputPath(changeDir, tracksFile);
     tracksFileExists = fs.existsSync(tracksPath);
     if (tracksFileExists) {
       const tasksContent = await fs.promises.readFile(tracksPath, 'utf-8');