From f7377b65b485ffb2230291bb38919d3e58188eb3 Mon Sep 17 00:00:00 2001 From: Ryoichi Izumita Date: Mon, 12 Jan 2026 10:13:30 +0900 Subject: [PATCH 1/2] refactor: unify path traversal detection logic Extract common path traversal detection into shared utility function. - Create isPathTraversal() in src/shared/fs/pathTraversal.ts - Update normalizePathForDenylist() to use shared utility - Update resolveSafePath() to use shared utility - Add comprehensive tests for isPathTraversal() Uses segment-based detection (split on /\\) to avoid false positives on patterns like Next.js catch-all routes [...all]. Closes #215 Co-Authored-By: Claude Opus 4.5 --- src/indexer/cli.ts | 4 +- src/shared/fs/pathTraversal.ts | 36 ++++++++++++ src/shared/fs/safePath.ts | 5 +- tests/shared/fs/pathTraversal.spec.ts | 79 +++++++++++++++++++++++++++ 4 files changed, 122 insertions(+), 2 deletions(-) create mode 100644 src/shared/fs/pathTraversal.ts create mode 100644 tests/shared/fs/pathTraversal.spec.ts diff --git a/src/indexer/cli.ts b/src/indexer/cli.ts index 2ab78b6..eab0ad6 100644 --- a/src/indexer/cli.ts +++ b/src/indexer/cli.ts @@ -8,6 +8,7 @@ import { parse as parseYAML } from "yaml"; import { DuckDBClient } from "../shared/duckdb.js"; import { generateEmbedding } from "../shared/embedding.js"; +import { isPathTraversal } from "../shared/fs/pathTraversal.js"; import { acquireLock, releaseLock, LockfileError, getLockOwner } from "../shared/utils/lockfile.js"; import { normalizeDbPath, @@ -1512,7 +1513,8 @@ export function normalizePathForDenylist(filePath: string): string { normalized = normalized.slice(1); } // パストラバーサル検出（セキュリティ） - if (normalized.split(/[/\\]/).includes("..")) { + // @see Issue #215: 共通ユーティリティに統合 + if (isPathTraversal(normalized)) { throw new Error(`Path traversal detected: ${filePath}`); } return normalized; diff --git a/src/shared/fs/pathTraversal.ts b/src/shared/fs/pathTraversal.ts new file mode 100644 index 0000000..129b28c --- /dev/null +++ b/src/shared/fs/pathTraversal.ts @@ -0,0 +1,36 @@ +/** + * Path Traversal Detection Utility + * + * Unified path traversal detection logic extracted from: + * - normalizePathForDenylist (src/indexer/cli.ts) + * - resolveSafePath (src/shared/fs/safePath.ts) + * + * Uses segment-based detection (not substring) to avoid false positives + * on patterns like Next.js catch-all routes [...all]. + * + * @see Issue #215: パストラバーサル検出ロジックの統合 + * @see PR #214: improve path traversal detection accuracy + */ + +/** + * Check if a path contains path traversal attempts + * + * Uses segment-based detection to avoid false positives on patterns like: + * - Next.js catch-all routes: [...all], [[...slug]] + * - File names containing ".." : bar..baz.ts + * - Directory names with partial "..": [..bar] + * + * @param path - The path to check (supports both Unix and Windows separators) + * @returns true if path traversal is detected + * + * @example + * isPathTraversal("../etc/passwd") // true + * isPathTraversal("foo/../bar") // true + * isPathTraversal("[...all]/route.ts") // false (Next.js pattern) + * isPathTraversal("foo/bar..baz.ts") // false (filename with ..) + */ +export function isPathTraversal(path: string): boolean { + // Split on both forward slash and backslash (Windows support) + const segments = path.split(/[/\\]/); + return segments.includes(".."); +} diff --git a/src/shared/fs/safePath.ts b/src/shared/fs/safePath.ts index a9ddf1d..11e6ce4 100644 --- a/src/shared/fs/safePath.ts +++ b/src/shared/fs/safePath.ts @@ -1,6 +1,8 @@ import { resolve, relative, sep } from "node:path"; import process from "node:process"; +import { isPathTraversal } from "./pathTraversal.js"; + interface SafePathOptions { baseDir?: string; allowOutsideBase?: boolean; @@ -26,7 +28,8 @@ export function resolveSafePath(inputPath: string, options?: SafePathOptions): s return resolved; } - if (relativePath.startsWith(`..${sep}`) || relativePath === "..") { + // @see Issue #215: 共通ユーティリティに統合 + if (isPathTraversal(relativePath)) { throw new Error(`Path traversal attempt detected: ${inputPath}`); } diff --git a/tests/shared/fs/pathTraversal.spec.ts b/tests/shared/fs/pathTraversal.spec.ts new file mode 100644 index 0000000..e6c5a6c --- /dev/null +++ b/tests/shared/fs/pathTraversal.spec.ts @@ -0,0 +1,79 @@ +import { describe, expect, it } from "vitest"; + +import { isPathTraversal } from "../../../src/shared/fs/pathTraversal.js"; + +describe("isPathTraversal", () => { + describe("パストラバーサル検出", () => { + it("先頭の .. を検出する", () => { + expect(isPathTraversal("../etc/passwd")).toBe(true); + }); + + it("中間の .. を検出する", () => { + expect(isPathTraversal("foo/../bar.ts")).toBe(true); + }); + + it("末尾の .. を検出する", () => { + expect(isPathTraversal("foo/bar/..")).toBe(true); + }); + + it("複数の .. を検出する", () => { + expect(isPathTraversal("../../secret")).toBe(true); + }); + + it("Windows形式の .. を検出する", () => { + expect(isPathTraversal("foo\\..\\bar")).toBe(true); + }); + + it("単独の .. を検出する", () => { + expect(isPathTraversal("..")).toBe(true); + }); + }); + + describe("誤検知回避（PR #214 修正対象）", () => { + it("Next.js catch-all ルート [...all] を許可する", () => { + expect(isPathTraversal("api/auth/[...all]/route.ts")).toBe(false); + }); + + it("ファイル名内の .. を許可する", () => { + expect(isPathTraversal("foo/bar..baz.ts")).toBe(false); + }); + + it("ディレクトリ名内の .. を許可する（部分マッチ）", () => { + expect(isPathTraversal("foo/[..bar]/baz.ts")).toBe(false); + }); + + it("三点 ... を許可する", () => { + expect(isPathTraversal("foo/.../bar.ts")).toBe(false); + }); + + it("末尾三点のファイル名を許可する", () => { + expect(isPathTraversal("foo/bar.../baz.ts")).toBe(false); + }); + + it("Next.js optional catch-all [[...slug]] を許可する", () => { + expect(isPathTraversal("[[...slug]]/page.ts")).toBe(false); + }); + }); + + describe("正常パス", () => { + it("通常のパスは false を返す", () => { + expect(isPathTraversal("foo/bar.ts")).toBe(false); + }); + + it("単一のドット . を許可する", () => { + expect(isPathTraversal(".")).toBe(false); + }); + + it("三点のみ ... を許可する", () => { + expect(isPathTraversal("...")).toBe(false); + }); + + it("空文字を許可する", () => { + expect(isPathTraversal("")).toBe(false); + }); + + it("深いネストのパスを許可する", () => { + expect(isPathTraversal("a/b/c/d/e/f/g.ts")).toBe(false); + }); + }); +}); From 16abc1a6981e8ed7b977360ecf905609e742388b Mon Sep 17 00:00:00 2001 From: Ryoichi Izumita Date: Mon, 12 Jan 2026 10:47:40 +0900 Subject: [PATCH 2/2] fix: remove unused sep import Co-Authored-By: Claude Opus 4.5 --- src/shared/fs/safePath.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/shared/fs/safePath.ts b/src/shared/fs/safePath.ts index 11e6ce4..7e9c9fe 100644 --- a/src/shared/fs/safePath.ts +++ b/src/shared/fs/safePath.ts @@ -1,4 +1,4 @@ -import { resolve, relative, sep } from "node:path"; +import { resolve, relative } from "node:path"; import process from "node:process"; import { isPathTraversal } from "./pathTraversal.js";