From 67741cc3a3e7f8ab8f9d4da8a87ed7fab4e650f8 Mon Sep 17 00:00:00 2001 From: vorotamoroz Date: Sun, 12 Jul 2026 09:40:17 +0000 Subject: [PATCH] Enforce rooted storage path contracts --- .../{ => _test}/storageAdapterContract.ts | 31 +++++++++++++++++++ src/apps/cli/adapters/NodeStorageAdapter.ts | 25 ++++++++------- .../adapters/NodeStorageAdapter.unit.spec.ts | 5 +-- src/apps/storagePath.ts | 26 ++++++++++++++++ .../webapp/adapters/FSAPIStorageAdapter.ts | 13 +++++--- .../adapters/FSAPIStorageAdapter.unit.spec.ts | 2 +- 6 files changed, 81 insertions(+), 21 deletions(-) rename src/apps/{ => _test}/storageAdapterContract.ts (68%) create mode 100644 src/apps/storagePath.ts diff --git a/src/apps/storageAdapterContract.ts b/src/apps/_test/storageAdapterContract.ts similarity index 68% rename from src/apps/storageAdapterContract.ts rename to src/apps/_test/storageAdapterContract.ts index 21880ea..5693e19 100644 --- a/src/apps/storageAdapterContract.ts +++ b/src/apps/_test/storageAdapterContract.ts @@ -16,6 +16,15 @@ function assertEqual(actual: unknown, expected: unknown, message: string): void } } +async function assertRejects(operation: () => Promise, message: string): Promise { + try { + await operation(); + } catch { + return; + } + throw new Error(message); +} + /** Passing baseline shared by Node, FSAPI, and future storage adapters. */ export const storageAdapterContractCases: readonly StorageAdapterContractCase[] = [ { @@ -74,4 +83,26 @@ export const storageAdapterContractCases: readonly StorageAdapterContractCase[] assertEqual(await adapter.exists("remove/folder"), false, "directory tree should be removed"); }, }, + { + name: "keeps operations inside the configured root", + async run(adapter) { + await assertRejects(() => adapter.exists("../outside"), "parent traversal should be rejected"); + await assertRejects(() => adapter.write("nested/../outside", "content"), "nested traversal should be rejected"); + await assertRejects(() => adapter.read("/absolute"), "absolute paths should be rejected"); + await assertRejects(() => adapter.read("C:\\absolute"), "drive-qualified paths should be rejected"); + await assertRejects(() => adapter.read("nested\\outside"), "backslash-separated paths should be rejected"); + await assertRejects(() => adapter.remove(""), "removing the configured root should be rejected"); + }, + }, + { + name: "uses the empty path only for root-safe operations", + async run(adapter) { + await adapter.mkdir(""); + assertEqual(await adapter.exists(""), true, "the configured root should exist"); + assertEqual((await adapter.stat(""))?.type, "folder", "the configured root should be a folder"); + assertEqual(await adapter.list(""), { files: [], folders: [] }, "the configured root should be listable"); + await assertRejects(() => adapter.write("", "content"), "writing over the configured root should be rejected"); + await assertRejects(() => adapter.append("", "content"), "appending to the configured root should be rejected"); + }, + }, ]; diff --git a/src/apps/cli/adapters/NodeStorageAdapter.ts b/src/apps/cli/adapters/NodeStorageAdapter.ts index 49a3f84..2f48f6c 100644 --- a/src/apps/cli/adapters/NodeStorageAdapter.ts +++ b/src/apps/cli/adapters/NodeStorageAdapter.ts @@ -2,20 +2,22 @@ import type { UXDataWriteOptions } from "@lib/common/types"; import type { IStorageAdapter } from "@lib/serviceModules/adapters"; import type { NodeStat } from "./NodeTypes"; import { fsPromises as fs, path } from "@/apps/cli/node-compat"; +import { validateStoragePath } from "@/apps/storagePath"; /** * Storage adapter implementation for Node.js */ export class NodeStorageAdapter implements IStorageAdapter { - constructor(private basePath: string) {} + constructor(private readonly basePath: string) {} - private resolvePath(p: string): string { - return path.join(this.basePath, p); + private resolvePath(p: string, allowRoot: boolean = true): string { + return path.join(this.basePath, validateStoragePath(p, allowRoot)); } async exists(p: string): Promise { + const fullPath = this.resolvePath(p); try { - await fs.access(this.resolvePath(p)); + await fs.access(fullPath); return true; } catch { return false; @@ -23,8 +25,9 @@ export class NodeStorageAdapter implements IStorageAdapter { } async trystat(p: string): Promise { + const fullPath = this.resolvePath(p); try { - const stat = await fs.stat(this.resolvePath(p)); + const stat = await fs.stat(fullPath); return { size: stat.size, mtime: Math.floor(stat.mtimeMs), @@ -45,7 +48,7 @@ export class NodeStorageAdapter implements IStorageAdapter { } async remove(p: string): Promise { - const fullPath = this.resolvePath(p); + const fullPath = this.resolvePath(p, false); const stat = await fs.stat(fullPath); if (stat.isDirectory()) { await fs.rm(fullPath, { recursive: true, force: true }); @@ -55,17 +58,17 @@ export class NodeStorageAdapter implements IStorageAdapter { } async read(p: string): Promise { - return await fs.readFile(this.resolvePath(p), "utf-8"); + return await fs.readFile(this.resolvePath(p, false), "utf-8"); } async readBinary(p: string): Promise { - const buffer = await fs.readFile(this.resolvePath(p)); + const buffer = await fs.readFile(this.resolvePath(p, false)); // eslint-disable-next-line @typescript-eslint/no-unnecessary-type-assertion -- required in environments where Buffer.buffer is ArrayBufferLike return buffer.buffer.slice(buffer.byteOffset, buffer.byteOffset + buffer.byteLength) as ArrayBuffer; } async write(p: string, data: string, options?: UXDataWriteOptions): Promise { - const fullPath = this.resolvePath(p); + const fullPath = this.resolvePath(p, false); await fs.mkdir(path.dirname(fullPath), { recursive: true }); await fs.writeFile(fullPath, data, "utf-8"); @@ -77,7 +80,7 @@ export class NodeStorageAdapter implements IStorageAdapter { } async writeBinary(p: string, data: ArrayBuffer, options?: UXDataWriteOptions): Promise { - const fullPath = this.resolvePath(p); + const fullPath = this.resolvePath(p, false); await fs.mkdir(path.dirname(fullPath), { recursive: true }); await fs.writeFile(fullPath, new Uint8Array(data)); @@ -89,7 +92,7 @@ export class NodeStorageAdapter implements IStorageAdapter { } async append(p: string, data: string, options?: UXDataWriteOptions): Promise { - const fullPath = this.resolvePath(p); + const fullPath = this.resolvePath(p, false); await fs.mkdir(path.dirname(fullPath), { recursive: true }); await fs.appendFile(fullPath, data, "utf-8"); diff --git a/src/apps/cli/adapters/NodeStorageAdapter.unit.spec.ts b/src/apps/cli/adapters/NodeStorageAdapter.unit.spec.ts index c3ad462..c32486e 100644 --- a/src/apps/cli/adapters/NodeStorageAdapter.unit.spec.ts +++ b/src/apps/cli/adapters/NodeStorageAdapter.unit.spec.ts @@ -2,7 +2,7 @@ import * as fs from "node:fs/promises"; import * as os from "node:os"; import * as path from "node:path"; import { afterEach, describe, expect, it } from "vitest"; -import { storageAdapterContractCases } from "@/apps/storageAdapterContract"; +import { storageAdapterContractCases } from "@/apps/_test/storageAdapterContract"; import { NodeStorageAdapter } from "./NodeStorageAdapter"; describe("NodeStorageAdapter", () => { @@ -44,7 +44,4 @@ describe("NodeStorageAdapter", () => { expect(result.byteLength).toBe(expected.byteLength); expect(Array.from(new Uint8Array(result))).toEqual([0x10, 0x20, 0x30]); }); - - it.todo("rejects paths that escape the configured root"); - it.todo("rejects removing the configured root through an empty path"); }); diff --git a/src/apps/storagePath.ts b/src/apps/storagePath.ts new file mode 100644 index 0000000..06bcda4 --- /dev/null +++ b/src/apps/storagePath.ts @@ -0,0 +1,26 @@ +/** + * Validate the platform-neutral path vocabulary used by rooted storage adapters. + * + * Paths are slash-separated and relative to the root bound to the adapter. Root + * selection and authorisation happen before the adapter is constructed. + */ +export function validateStoragePath(storagePath: string, allowRoot: boolean = true): string { + if (storagePath === "") { + if (allowRoot) return storagePath; + throw new Error("The storage root is not a valid entry path"); + } + + if (storagePath.startsWith("/") || storagePath.startsWith("\\") || /^[A-Za-z]:/.test(storagePath)) { + throw new Error(`Storage paths must be relative to the configured root: ${storagePath}`); + } + if (storagePath.includes("\\")) { + throw new Error(`Storage paths must use forward slashes: ${storagePath}`); + } + + const segments = storagePath.split("/"); + if (segments.some((segment) => segment === "." || segment === "..")) { + throw new Error(`Storage paths must not contain traversal segments: ${storagePath}`); + } + + return storagePath; +} diff --git a/src/apps/webapp/adapters/FSAPIStorageAdapter.ts b/src/apps/webapp/adapters/FSAPIStorageAdapter.ts index f48a20d..259d656 100644 --- a/src/apps/webapp/adapters/FSAPIStorageAdapter.ts +++ b/src/apps/webapp/adapters/FSAPIStorageAdapter.ts @@ -1,12 +1,13 @@ import type { UXDataWriteOptions } from "@lib/common/types"; import type { IStorageAdapter } from "@lib/serviceModules/adapters"; import type { FSAPIStat } from "./FSAPITypes"; +import { validateStoragePath } from "@/apps/storagePath"; /** * Storage adapter implementation for FileSystem API */ export class FSAPIStorageAdapter implements IStorageAdapter { - constructor(private rootHandle: FileSystemDirectoryHandle) {} + constructor(private readonly rootHandle: FileSystemDirectoryHandle) {} /** * Resolve a path to directory and file handles @@ -15,11 +16,9 @@ export class FSAPIStorageAdapter implements IStorageAdapter { dirHandle: FileSystemDirectoryHandle; fileName: string; } | null> { + validateStoragePath(p, false); try { const parts = p.split("/").filter((part) => part !== ""); - if (parts.length === 0) { - return null; - } let currentHandle = this.rootHandle; const fileName = parts[parts.length - 1]; @@ -39,6 +38,8 @@ export class FSAPIStorageAdapter implements IStorageAdapter { * Get file handle for a given path */ private async getFileHandle(p: string): Promise { + validateStoragePath(p); + if (p === "") return null; const resolved = await this.resolvePath(p); if (!resolved) return null; @@ -53,6 +54,7 @@ export class FSAPIStorageAdapter implements IStorageAdapter { * Get directory handle for a given path */ private async getDirectoryHandle(p: string): Promise { + validateStoragePath(p); try { const parts = p.split("/").filter((part) => part !== ""); if (parts.length === 0) { @@ -75,8 +77,8 @@ export class FSAPIStorageAdapter implements IStorageAdapter { dirHandle: FileSystemDirectoryHandle; fileName: string; } | null> { + validateStoragePath(p, false); const parts = p.split("/").filter((part) => part !== ""); - if (parts.length === 0) return null; const fileName = parts.pop()!; const parentPath = parts.join("/"); await this.mkdir(parentPath); @@ -124,6 +126,7 @@ export class FSAPIStorageAdapter implements IStorageAdapter { } async mkdir(p: string): Promise { + validateStoragePath(p); const parts = p.split("/").filter((part) => part !== ""); let currentHandle = this.rootHandle; diff --git a/src/apps/webapp/adapters/FSAPIStorageAdapter.unit.spec.ts b/src/apps/webapp/adapters/FSAPIStorageAdapter.unit.spec.ts index f741820..dde5f91 100644 --- a/src/apps/webapp/adapters/FSAPIStorageAdapter.unit.spec.ts +++ b/src/apps/webapp/adapters/FSAPIStorageAdapter.unit.spec.ts @@ -1,5 +1,5 @@ import { describe, it } from "vitest"; -import { storageAdapterContractCases } from "@/apps/storageAdapterContract"; +import { storageAdapterContractCases } from "@/apps/_test/storageAdapterContract"; import { FSAPIStorageAdapter } from "./FSAPIStorageAdapter"; class MemoryFileHandle {