From c91ab16df1ba79f67dcc86dd2653bb921166acb6 Mon Sep 17 00:00:00 2001 From: seonghobae <8172694+seonghobae@users.noreply.github.com> Date: Tue, 16 Jun 2026 05:23:35 +0000 Subject: [PATCH] =?UTF-8?q?=F0=9F=9B=A1=EF=B8=8F=20Sentinel:=20[CRITICAL]?= =?UTF-8?q?=20Fix=20path=20traversal=20in=20export=20command?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - `exportGherkin` 명령어에서 `--output` 옵션으로 전달된 경로를 처리할 때 발생할 수 있는 경로 조작(Path Traversal) 취약점을 수정했습니다. - `path.resolve`와 `path.relative`를 사용하여 최종 출력 경로가 프로젝트의 루트 디렉토리 내에 안전하게 위치하는지 검증합니다. - 취약점 발생 시 안전하게 `VspecError`를 던지도록 처리했습니다. - 검증 및 회귀 방지를 위한 통합 테스트(`tests/gherkin-security.test.ts`)를 추가했습니다. --- .jules/sentinel.md | 4 +++ src/export/gherkin.ts | 9 +++++-- tests/gherkin-security.test.ts | 49 ++++++++++++++++++++++++++++++++++ 3 files changed, 60 insertions(+), 2 deletions(-) create mode 100644 .jules/sentinel.md create mode 100644 tests/gherkin-security.test.ts diff --git a/.jules/sentinel.md b/.jules/sentinel.md new file mode 100644 index 0000000..be8dde2 --- /dev/null +++ b/.jules/sentinel.md @@ -0,0 +1,4 @@ +## 2024-06-16 - Prevent Path Traversal in Export Path +**Vulnerability:** The `exportGherkin` command accepted arbitrary paths via the `--output` option and concatenated them with the project root using `join()`, allowing path traversal (e.g., `../../../../tmp/hacked.feature`) to write files outside the intended project directory boundaries. +**Learning:** In CLI applications that write to user-specified paths, using `join()` alone is insufficient for preventing traversal attacks. Path boundaries must be explicitly validated. +**Prevention:** Always use `resolve()` to compute the absolute target path, use `relative(root, target)` to calculate the distance, and check for `..`, `startsWith(".." + sep)`, or `isAbsolute(rel)` to ensure the resolved path stays within the intended directory boundary. diff --git a/src/export/gherkin.ts b/src/export/gherkin.ts index 4710ea9..3a250d5 100644 --- a/src/export/gherkin.ts +++ b/src/export/gherkin.ts @@ -1,5 +1,6 @@ import { mkdirSync, readFileSync, writeFileSync } from "node:fs"; -import { dirname, join } from "node:path"; +import { dirname, isAbsolute, join, relative, resolve, sep } from "node:path"; +import { VspecError } from "../errors.js"; import { findUseCaseFile, readConfig, relativePath } from "../files.js"; import { parseUseCaseMarkdown } from "../format/parse.js"; import type { ParsedUseCase } from "../domain/types.js"; @@ -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 === ".." || rel.startsWith(".." + sep) || isAbsolute(rel)) { + throw new VspecError("INVALID_ARGUMENT", "Invalid output path: Path traversal detected."); + } mkdirSync(dirname(outputPath), { recursive: true }); writeFileSync(outputPath, text); return { key: args.key, text, path: relativePath(outputPath, config.root) }; diff --git a/tests/gherkin-security.test.ts b/tests/gherkin-security.test.ts new file mode 100644 index 0000000..def7b50 --- /dev/null +++ b/tests/gherkin-security.test.ts @@ -0,0 +1,49 @@ +import { join } from "node:path"; +import { describe, expect, it, beforeEach, afterEach } from "vitest"; +import { mkdirSync, rmSync, writeFileSync } from "node:fs"; +import { tmpdir } from "node:os"; +import { randomUUID } from "node:crypto"; +import { exportGherkin } from "../src/export/gherkin.js"; +import { initProject } from "../src/project.js"; + +describe("gherkin security", () => { + let cwd: string; + + beforeEach(() => { + cwd = join(tmpdir(), randomUUID()); + mkdirSync(cwd, { recursive: true }); + initProject({ key: "TEST", root: cwd }); + mkdirSync(join(cwd, "specs/usecases"), { recursive: true }); + + // Create a dummy use case file + const md = `--- +vspec_format: 1 +type: usecase +key: TEST-001 +title: Test use case +scope: test_project +level: USER_GOAL +primary_actor: system +priority: P1 +status: DRAFT +format: BRIEF +--- +`; + writeFileSync(join(cwd, "specs/usecases/TEST-001-test.md"), md); + }); + + afterEach(() => { + rmSync(cwd, { recursive: true, force: true }); + }); + + it("prevents path traversal in output path", () => { + expect(() => + exportGherkin({ key: "TEST-001", output: "../../../../../tmp/hacked.feature", cwd }) + ).toThrowError("Invalid output path: Path traversal detected."); + }); + + it("allows valid output paths inside the project", () => { + const result = exportGherkin({ key: "TEST-001", output: "tests/valid.feature", cwd }); + expect(result.path).toBe("tests/valid.feature"); + }); +});