diff --git a/packages/documents/src/index.ts b/packages/documents/src/index.ts index afd98de..11598d0 100644 --- a/packages/documents/src/index.ts +++ b/packages/documents/src/index.ts @@ -49,12 +49,28 @@ export class LocalJsonStore { private toPath(key: string) { const relative = key - .split("/") - .filter(Boolean) + .split(/[\\/]/) + .filter((segment) => { + const s = segment.trim(); + return s && s !== "." && s !== ".."; + }) .map(sanitizeSegment) .join(path.sep); - return path.join(this.baseDirectory, `${relative}.json`); + const absoluteBase = path.resolve(this.baseDirectory); + const absoluteResolved = path.resolve(absoluteBase, `${relative}.json`); + + // Ensure the resolved path stays within the base directory. + // Use path.sep to prevent prefix bypass (e.g. /data vs /data-secrets) + const baseWithTrailingSep = absoluteBase.endsWith(path.sep) + ? absoluteBase + : `${absoluteBase}${path.sep}`; + + if (!absoluteResolved.startsWith(baseWithTrailingSep) && absoluteResolved !== absoluteBase) { + throw new Error(`Path traversal detected: "${key}" resolves outside the base directory.`); + } + + return absoluteResolved; } private readPath(filePath: string) { diff --git a/packages/documents/src/security.test.ts b/packages/documents/src/security.test.ts new file mode 100644 index 0000000..23788a4 --- /dev/null +++ b/packages/documents/src/security.test.ts @@ -0,0 +1,63 @@ +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 baseDir: string; + let store: LocalJsonStore; + + beforeEach(() => { + baseDir = fs.mkdtempSync(path.join(os.tmpdir(), "jeanbot-test-store-")); + store = new LocalJsonStore(baseDir); + }); + + afterEach(() => { + fs.rmSync(baseDir, { recursive: true, force: true }); + }); + + it("should prevent path traversal when reading", () => { + const secretFile = path.resolve(baseDir, "..", "jeanbot-secret.json"); + fs.writeFileSync(secretFile, JSON.stringify({ secret: "stolen" })); + + try { + const data = store.read("../jeanbot-secret"); + expect(data).toBeUndefined(); + } finally { + if (fs.existsSync(secretFile)) { + fs.unlinkSync(secretFile); + } + } + }); + + it("should prevent path traversal when writing", () => { + const maliciousKey = "../malicious"; + const data = { evil: true }; + + store.write(maliciousKey, data); + + const maliciousPath = path.resolve(baseDir, "..", "malicious.json"); + const exists = fs.existsSync(maliciousPath); + + if (exists) { + fs.unlinkSync(maliciousPath); + } + + expect(exists).toBe(false); + }); + + it("should prevent path traversal when deleting", () => { + const victimFile = path.resolve(baseDir, "..", "victim.json"); + fs.writeFileSync(victimFile, JSON.stringify({ data: "stay" })); + + try { + store.delete("../victim"); + expect(fs.existsSync(victimFile)).toBe(true); + } finally { + if (fs.existsSync(victimFile)) { + fs.unlinkSync(victimFile); + } + } + }); +});