🛡️ Sentinel: Fix path traversal in LocalJsonStore

.jules/sentinel.md

@@ -0,0 +1,10 @@
+# Sentinel Security Journal
+
+## 2025-05-15 - [Path Traversal in LocalJsonStore]
+**Vulnerability:** `LocalJsonStore` allowed reading and writing files outside of its intended `baseDirectory` via `..` segments in the keys.
+**Learning:** Simple string replacement or filtering of `/` isn't enough to prevent path traversal in Node.js when using `path.join`. Attackers can use `..` or absolute paths.
+**Prevention:**
+1. Disallow absolute paths in keys.
+2. Filter out `.` and `..` segments during key normalization.
+3. Use a cross-platform regex `/[\\\/]/` for splitting path segments.
+4. Always verify the final resolved absolute path starts with the resolved `baseDirectory` (using a trailing separator to prevent prefix bypass attacks like `/data/store` vs `/data/store-leak`).

packages/documents/src/index.ts

@@ -48,13 +48,27 @@ export class LocalJsonStore<T> {
   }
 
   private toPath(key: string) {
-    const relative = key
-      .split("/")
-      .filter(Boolean)
-      .map(sanitizeSegment)
-      .join(path.sep);
+    if (path.isAbsolute(key)) {
+      throw new Error(`Absolute paths are not allowed in JSON store keys: ${key}`);
+    }
+
+    const segments = key.split(/[\\\/]/).filter((s) => s && s !== "." && s !== "..");
+    const relative = segments.map(sanitizeSegment).join(path.sep);
+
+    const filePath = path.join(this.baseDirectory, `${relative}.json`);
+    const absoluteBase = path.resolve(this.baseDirectory);
+    const absoluteResolved = path.resolve(filePath);
+
+    // Ensure the resolved path is actually within the base directory
+    const baseWithTrailingSep = absoluteBase.endsWith(path.sep)
+      ? absoluteBase
+      : `${absoluteBase}${path.sep}`;
+
+    if (!absoluteResolved.startsWith(baseWithTrailingSep) && absoluteResolved !== absoluteBase) {
+      throw new Error(`Path traversal detected in key: ${key}`);
+    }
 
-    return path.join(this.baseDirectory, `${relative}.json`);
+    return filePath;
   }
 
   private readPath(filePath: string) {

packages/documents/src/security.test.ts

@@ -0,0 +1,40 @@
+import { describe, it, expect, beforeEach, afterEach } from "vitest";
+import { LocalJsonStore } from "./index.js";
+import path from "node:path";
+import fs from "node:fs";
+import os from "node:os";
+
+describe("LocalJsonStore Security", () => {
+  let tempDir: string;
+  let store: LocalJsonStore<any>;
+
+  beforeEach(() => {
+    tempDir = fs.mkdtempSync(path.join(os.tmpdir(), "json-store-test-"));
+    const storeDir = path.join(tempDir, "store");
+    store = new LocalJsonStore(storeDir);
+  });
+
+  afterEach(() => {
+    fs.rmSync(tempDir, { recursive: true, force: true });
+  });
+
+  it("should not allow path traversal out of base directory", () => {
+    const secretFile = path.join(tempDir, "secret.json");
+    fs.writeFileSync(secretFile, JSON.stringify({ secret: "data" }));
+
+    // Attempt to read the secret file using path traversal
+    // The store is at tempDir/store
+    // So ../secret should point to tempDir/secret.json
+    const trappedData = store.read("../secret");
+
+    expect(trappedData).toBeUndefined();
+  });
+
+  it("should not allow writing outside of base directory", () => {
+    const traversalKey = "../evil";
+    store.write(traversalKey, { evil: "data" });
+
+    const evilFile = path.join(tempDir, "evil.json");
+    expect(fs.existsSync(evilFile)).toBe(false);
+  });
+});