diff --git a/.jules/sentinel.md b/.jules/sentinel.md new file mode 100644 index 0000000..52dafe9 --- /dev/null +++ b/.jules/sentinel.md @@ -0,0 +1,4 @@ +## 2024-06-09 - Path Traversal Vulnerability in `exportGherkin` +**Vulnerability:** The `--output` argument in `vspec export gherkin` does not sanitize the `output` file path. It is concatenated with the config root directly (`join(config.root, output)`). +**Learning:** This allows an attacker to provide a path like `../../../../etc/passwd` to write Gherkin export data to unauthorized locations outside the project directory. +**Prevention:** Implement strict input validation on constructed paths to verify they remain within the expected directory bounds. Use `path.resolve` and check if the resolved relative path starts with `..` or is an absolute path. diff --git a/src/export/gherkin.ts b/src/export/gherkin.ts index 4710ea9..0c64651 100644 --- a/src/export/gherkin.ts +++ b/src/export/gherkin.ts @@ -1,7 +1,8 @@ import { mkdirSync, readFileSync, writeFileSync } from "node:fs"; -import { dirname, join } from "node:path"; +import { dirname, join, resolve, relative, isAbsolute } from "node:path"; import { findUseCaseFile, readConfig, relativePath } from "../files.js"; import { parseUseCaseMarkdown } from "../format/parse.js"; +import { VspecError } from "../errors.js"; import type { ParsedUseCase } from "../domain/types.js"; export function renderGherkin(useCase: ParsedUseCase): string { @@ -39,7 +40,11 @@ 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); + const outputPath = resolve(config.root, output); + const rel = relative(config.root, outputPath); + if (rel.startsWith("..") || isAbsolute(rel)) { + throw new VspecError("INVALID_ARGUMENT", "Output path must be within 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..a8cadf1 100644 --- a/tests/gherkin.test.ts +++ b/tests/gherkin.test.ts @@ -1,8 +1,9 @@ import { readFileSync } from "node:fs"; import { join } from "node:path"; import { describe, expect, it } from "vitest"; -import { renderGherkin } from "../src/export/gherkin.js"; +import { renderGherkin, exportGherkin } from "../src/export/gherkin.js"; import { parseUseCaseMarkdown } from "../src/format/parse.js"; +import { VspecError } from "../src/errors.js"; describe("gherkin export", () => { it("renders the golden feature byte-for-byte", () => { @@ -10,4 +11,13 @@ describe("gherkin export", () => { const expected = readFileSync(join(import.meta.dirname, "fixtures/export/VSPEC-010.feature"), "utf8"); expect(renderGherkin(useCase)).toBe(expected); }); + + it("prevents path traversal vulnerabilities", () => { + // VSPEC-001 is present in the mock project root (tests/fixtures/doctor/clean) so it would succeed + // if not for the traversal in the --output path. + const cwd = join(import.meta.dirname, "fixtures/doctor/clean"); + expect(() => + exportGherkin({ key: "VSPEC-001", output: "../../../tmp/hacked.feature", cwd }) + ).toThrowError(new VspecError("INVALID_ARGUMENT", "Output path must be within the project root.")); + }); });