Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions .jules/sentinel.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
## 2024-06-07 - Path Traversal Vulnerability in file export
**Vulnerability:** The `exportGherkin` command allowed specifying a custom `--output` path without ensuring it resided within the project boundaries, leading to potential path traversal vulnerabilities.
**Learning:** Functions dealing with output paths provided by users must strictly validate boundaries to ensure files are written within authorized directories (`config.root`).
**Prevention:** Construct absolute paths using `path.resolve()` and verify bounds using `const rel = path.relative(root, target)`. Check both `rel.startsWith("..")` and `path.isAbsolute(rel)`.
29 changes: 23 additions & 6 deletions src/export/gherkin.ts
Original file line number Diff line number Diff line change
@@ -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 {
Expand All @@ -21,8 +22,12 @@ export function renderGherkin(useCase: ParsedUseCase): string {
for (const extension of useCase.extensions) {
lines.push("");
lines.push(` Scenario: ${extension.point} ${extension.condition}`);
if (extension.point.startsWith("*")) lines.push(" Given main success reaches any step");
else lines.push(` Given main success reaches step ${extension.point.match(/^(\d+)/)?.[1] ?? extension.point}`);
if (extension.point.startsWith("*"))
lines.push(" Given main success reaches any step");
else
lines.push(
` Given main success reaches step ${extension.point.match(/^(\d+)/)?.[1] ?? extension.point}`,
);
for (const step of extension.steps) {
lines.push(` When ${step.actor} ${trimSentence(step.action)}`);
}
Expand All @@ -32,14 +37,26 @@ export function renderGherkin(useCase: ParsedUseCase): string {
return `${lines.join("\n")}\n`;
}

export function exportGherkin(args: { key: string; output?: string; cwd?: string }) {
export function exportGherkin(args: {
key: string;
output?: string;
cwd?: string;
}) {
const config = readConfig(args.cwd ?? process.cwd());
if (!config) throw new Error("NOT_INITIALIZED");
const source = findUseCaseFile(config.root, args.key);
if (!source) throw new Error("KEY_NOT_FOUND");
const text = renderGherkin(parseUseCaseMarkdown(readFileSync(source, "utf8")));
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", "Path traversal detected");
}

mkdirSync(dirname(outputPath), { recursive: true });
writeFileSync(outputPath, text);
return { key: args.key, text, path: relativePath(outputPath, config.root) };
Expand Down
63 changes: 58 additions & 5 deletions tests/gherkin.test.ts
Original file line number Diff line number Diff line change
@@ -1,13 +1,66 @@
import { readFileSync } from "node:fs";
import { readFileSync, writeFileSync, mkdirSync, rmSync } from "node:fs";
import { join } from "node:path";
import { describe, expect, it } from "vitest";
import { renderGherkin } from "../src/export/gherkin.js";
import { tmpdir } from "node:os";
import { describe, expect, it, beforeEach, afterEach } from "vitest";
import { renderGherkin, exportGherkin } from "../src/export/gherkin.js";
import { parseUseCaseMarkdown } from "../src/format/parse.js";
import { VspecError } from "../src/errors.js";

describe("gherkin export", () => {
let tempRoot: string;

beforeEach(() => {
tempRoot = join(tmpdir(), `vspec-gherkin-${crypto.randomUUID()}`);
mkdirSync(tempRoot, { recursive: true });
});

afterEach(() => {
rmSync(tempRoot, { recursive: true, force: true });
});

it("renders the golden feature byte-for-byte", () => {
const useCase = parseUseCaseMarkdown(readFileSync(join(import.meta.dirname, "fixtures/export/VSPEC-010-export-gherkin.md"), "utf8"));
const expected = readFileSync(join(import.meta.dirname, "fixtures/export/VSPEC-010.feature"), "utf8");
const useCase = parseUseCaseMarkdown(
readFileSync(
join(
import.meta.dirname,
"fixtures/export/VSPEC-010-export-gherkin.md",
),
"utf8",
),
);
const expected = readFileSync(
join(import.meta.dirname, "fixtures/export/VSPEC-010.feature"),
"utf8",
);
expect(renderGherkin(useCase)).toBe(expected);
});

it("throws VspecError on path traversal attempt", () => {
mkdirSync(join(tempRoot, ".vspec"), { recursive: true });
writeFileSync(
join(tempRoot, ".vspec/config.json"),
'{"vspec_format": 1, "key_prefix": "VSPEC"}',
);
mkdirSync(join(tempRoot, "specs/usecases"), { recursive: true });

// Copy the golden fixture to the temp dir so exportGherkin can read it
const md = readFileSync(
join(import.meta.dirname, "fixtures/export/VSPEC-010-export-gherkin.md"),
"utf8",
);
writeFileSync(
join(tempRoot, "specs/usecases/VSPEC-010-export-gherkin.md"),
md,
);

expect(() => {
exportGherkin({
key: "VSPEC-010",
output: "../../../etc/passwd",
cwd: tempRoot,
});
}).toThrowError(
new VspecError("INVALID_ARGUMENT", "Path traversal detected"),
);
});
});