🛡️ Sentinel: Fix path traversal in LocalJsonStore

.jules/sentinel.md

@@ -0,0 +1,4 @@
+## 2025-05-15 - [Path Traversal in LocalJsonStore]
+**Vulnerability:** `LocalJsonStore` allowed directory traversal via `..` segments in keys and did not block absolute paths, enabling file access outside the `baseDirectory`.
+**Learning:** Even with segment sanitization, failing to filter `..` or validate the final resolved path against a trusted root allows path traversal. Prefix-based validation needs a trailing separator to prevent "prefix bypass" (e.g., `/data` matching `/data-secret`).
+**Prevention:** Always filter `.` and `..` from user-controlled path segments. Use `path.resolve` and verify the result starts with the base directory followed by a separator. Disallow absolute paths if they are intended to be relative to a root.

packages/documents/src/index.ts

@@ -48,13 +48,28 @@ export class LocalJsonStore<T> {
   }
 
   private toPath(key: string) {
+    if (path.isAbsolute(key)) {
+      throw new Error(`Invalid store key: ${key} (absolute paths not allowed)`);
+    }
+
     const relative = key
-      .split("/")
-      .filter(Boolean)
+      .split(/[\\\/]/)
+      .filter((segment) => segment && segment !== "." && segment !== "..")
       .map(sanitizeSegment)
       .join(path.sep);
 
-    return path.join(this.baseDirectory, `${relative}.json`);
+    const resolvedBase = path.resolve(this.baseDirectory);
+    const absolutePath = path.resolve(resolvedBase, `${relative}.json`);
+
+    const baseWithTrailingSep = resolvedBase.endsWith(path.sep)
+      ? resolvedBase
+      : `${resolvedBase}${path.sep}`;
+
+    if (!absolutePath.startsWith(baseWithTrailingSep)) {
+      throw new Error(`Invalid store key: ${key}`);
+    }
+
+    return absolutePath;
   }
 
   private readPath(filePath: string) {

packages/documents/src/security.test.ts

@@ -0,0 +1,58 @@
+import { describe, it, expect, beforeEach, afterEach } from "vitest";
+import { LocalJsonStore } from "./index.js";
+import path from "node:path";
+import fs from "node:fs";
+
+describe("LocalJsonStore Security", () => {
+  const baseDir = "./test-store";
+
+  beforeEach(() => {
+    if (fs.existsSync(baseDir)) {
+      fs.rmSync(baseDir, { recursive: true, force: true });
+    }
+  });
+
+  afterEach(() => {
+    if (fs.existsSync(baseDir)) {
+      fs.rmSync(baseDir, { recursive: true, force: true });
+    }
+    const traversalFile = path.resolve(baseDir, "../traversal-test.json");
+    if (fs.existsSync(traversalFile)) {
+        fs.unlinkSync(traversalFile);
+    }
+  });
+
+  it("should not allow path traversal via .. in key", () => {
+    const store = new LocalJsonStore(baseDir);
+    const traversalKey = "../traversal-test";
+
+    // This should ideally throw or at least not write outside baseDir
+    try {
+        store.write(traversalKey, { pwned: true });
+    } catch (_e) {
+        // If it throws, that's also a form of protection
+    }
+
+    const traversalFile = path.resolve(baseDir, "../traversal-test.json");
+    expect(fs.existsSync(traversalFile)).toBe(false);
+  });
+
+  it("should block absolute path traversal", () => {
+    const store = new LocalJsonStore(baseDir);
+    const absoluteTraversalKey = path.resolve("/tmp/traversal-test");
+
+    expect(() => {
+        store.write(absoluteTraversalKey, { pwned: true });
+    }).toThrow();
+  });
+
+  it("should allow deep keys within base directory", () => {
+    const store = new LocalJsonStore(baseDir);
+    const deepKey = "deep/nested/key";
+
+    store.write(deepKey, { success: true });
+
+    const record = store.read(deepKey);
+    expect(record).toEqual({ success: true });
+  });
+});