🛡️ Sentinel: [CRITICAL] Fix path traversal in LocalJsonStore

.jules/sentinel.md

@@ -0,0 +1,4 @@
+## 2025-05-26 - Path Traversal in LocalJsonStore
+**Vulnerability:** Path traversal via storage keys in `LocalJsonStore`.
+**Learning:** Simple path joining of user-controlled keys is insufficient even with basic sanitization. `path.resolve` combined with `path.relative` or a strict prefix check is necessary to ensure the final path remains within the intended base directory.
+**Prevention:** Always use `path.relative(base, resolved)` and check if it starts with `..` to verify boundary compliance.

packages/documents/src/index.ts

@@ -50,11 +50,20 @@ export class LocalJsonStore<T> {
   private toPath(key: string) {
     const relative = key
       .split("/")
-      .filter(Boolean)
+      .filter((s) => s && s !== "." && s !== "..")
       .map(sanitizeSegment)
       .join(path.sep);
 
-    return path.join(this.baseDirectory, `${relative}.json`);
+    const resolved = path.join(this.baseDirectory, `${relative}.json`);
+    const absoluteBase = path.resolve(this.baseDirectory);
+    const absoluteResolved = path.resolve(resolved);
+
+    const relativePath = path.relative(absoluteBase, absoluteResolved);
+    if (relativePath.startsWith("..") || path.isAbsolute(relativePath)) {
+      throw new Error(`Invalid store key: ${key}`);
+    }
+
+    return resolved;
   }
 
   private readPath(filePath: string) {

packages/documents/src/security.test.ts

@@ -0,0 +1,51 @@
+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 = path.resolve("./tmp/test-store");
+  let store: LocalJsonStore<any>;
+
+  beforeEach(() => {
+    if (!fs.existsSync(baseDir)) {
+      fs.mkdirSync(baseDir, { recursive: true });
+    }
+    store = new LocalJsonStore(baseDir);
+  });
+
+  afterEach(() => {
+    if (fs.existsSync(baseDir)) {
+      fs.rmSync(baseDir, { recursive: true, force: true });
+    }
+    const leakedFile = path.resolve("./tmp/leaked.json");
+    if (fs.existsSync(leakedFile)) {
+        fs.unlinkSync(leakedFile);
+    }
+  });
+
+  it("should prevent path traversal when writing", () => {
+    const maliciousKey = "../leaked";
+    const data = { secret: "leaked" };
+
+    // This is expected to fail or at least NOT write outside baseDir after the fix
+    try {
+        store.write(maliciousKey, data);
+    } catch (e) {
+        // If it throws, that's also a way to prevent it
+    }
+
+    const leakedPath = path.resolve(baseDir, "..", "leaked.json");
+    expect(fs.existsSync(leakedPath)).toBe(false);
+  });
+
+  it("should prevent path traversal when reading", () => {
+    const maliciousKey = "../leaked";
+    // Manually create a file outside
+    const leakedPath = path.resolve(baseDir, "..", "leaked.json");
+    fs.writeFileSync(leakedPath, JSON.stringify({ secret: "internal" }));
+
+    const data = store.read(maliciousKey);
+    expect(data).toBeUndefined();
+  });
+});