diff --git a/.jules/sentinel.md b/.jules/sentinel.md new file mode 100644 index 0000000..b1f6f5b --- /dev/null +++ b/.jules/sentinel.md @@ -0,0 +1,4 @@ +## 2025-06-08 - Path Traversal in Gherkin Export +**Vulnerability:** The `exportGherkin` command allowed path traversal vulnerabilities through the `output` parameter by constructing paths using `join` without ensuring they remain inside the project root, allowing arbitrary file write outside the project structure. +**Learning:** `join` is insufficient for enforcing path boundaries when user input is involved, which could result in overwriting critical files or gaining broader filesystem access. +**Prevention:** Construct absolute paths using `path.resolve()` and verify boundaries using `const rel = path.relative(root, target)`. Always check both `rel.startsWith("..")` and `path.isAbsolute(rel)` to ensure security across all platforms, including Windows. diff --git a/src/export/gherkin.ts b/src/export/gherkin.ts index 4710ea9..708021a 100644 --- a/src/export/gherkin.ts +++ b/src/export/gherkin.ts @@ -1,8 +1,9 @@ import { mkdirSync, readFileSync, writeFileSync } from "node:fs"; -import { dirname, join } from "node:path"; +import { dirname, isAbsolute, join, relative, resolve } from "node:path"; import { findUseCaseFile, readConfig, relativePath } from "../files.js"; import { parseUseCaseMarkdown } from "../format/parse.js"; import type { ParsedUseCase } from "../domain/types.js"; +import { VspecError } from "../errors.js"; export function renderGherkin(useCase: ParsedUseCase): string { const lines: string[] = [ @@ -39,7 +40,14 @@ export function exportGherkin(args: { key: string; output?: string; cwd?: string if (!source) throw new Error("KEY_NOT_FOUND"); const text = renderGherkin(parseUseCaseMarkdown(readFileSync(source, "utf8"))); const output = args.output ?? join("tests", `${args.key}.feature`); - const outputPath = join(config.root, output); + + // Security: Prevent path traversal by ensuring the resolved absolute output + // path remains within the project root directory boundaries. + const outputPath = resolve(config.root, output); + const rel = relative(config.root, outputPath); + if (rel.startsWith("..") || isAbsolute(rel)) { + throw new VspecError("INVALID_ARGUMENT", `Output path "${output}" must be inside the project root.`); + } mkdirSync(dirname(outputPath), { recursive: true }); writeFileSync(outputPath, text); return { key: args.key, text, path: relativePath(outputPath, config.root) }; diff --git a/tests/gherkin.test.ts b/tests/gherkin.test.ts index 1a0c28e..bffe689 100644 --- a/tests/gherkin.test.ts +++ b/tests/gherkin.test.ts @@ -1,8 +1,13 @@ import { readFileSync } from "node:fs"; import { join } from "node:path"; import { describe, expect, it } from "vitest"; -import { renderGherkin } from "../src/export/gherkin.js"; +import { exportGherkin, renderGherkin } from "../src/export/gherkin.js"; import { parseUseCaseMarkdown } from "../src/format/parse.js"; +import { initProject } from "../src/project.js"; +import { createUseCase } from "../src/usecase-commands.js"; +import os from "node:os"; +import crypto from "node:crypto"; +import { mkdirSync, rmSync } from "node:fs"; describe("gherkin export", () => { it("renders the golden feature byte-for-byte", () => { @@ -10,4 +15,13 @@ describe("gherkin export", () => { const expected = readFileSync(join(import.meta.dirname, "fixtures/export/VSPEC-010.feature"), "utf8"); expect(renderGherkin(useCase)).toBe(expected); }); + it("throws INVALID_ARGUMENT when output path escapes project root", () => { + const cwd = join(os.tmpdir(), crypto.randomUUID()); + mkdirSync(cwd, { recursive: true }); + initProject({ root: cwd, key: "TEST" }); + const { key } = createUseCase({ title: "test", primaryActor: "user", cwd }); + + expect(() => exportGherkin({ key, output: "../../../../tmp/hacked.txt", cwd })).toThrowError(/must be inside the project root/); + rmSync(cwd, { recursive: true, force: true }); + }); });