Skip to content

Commit

Permalink
feat: safe write/copy file (blib-la#209)
Browse files Browse the repository at this point in the history
## Motivation

prevents traversing beyond the path root

## Issues closed

closes blib-la#193
  • Loading branch information
pixelass committed Apr 18, 2024
1 parent c295588 commit 9bf22a1
Show file tree
Hide file tree
Showing 3 changed files with 63 additions and 4 deletions.
6 changes: 3 additions & 3 deletions src/electron/future/ipc/global.ts
Expand Up @@ -19,7 +19,7 @@ import { v4 } from "uuid";
import { buildKey } from "#/build-key";
import { VECTOR_STORE_COLLECTION } from "#/constants";
import { ID } from "#/enums";
import { extractH1Headings, getFileType } from "#/string";
import { cleanPath, extractH1Headings, getFileType } from "#/string";
import { apps } from "@/apps";
import { VectorStore } from "@/services/vector-store";
import { inventoryStore, downloadsStore } from "@/stores";
Expand Down Expand Up @@ -124,7 +124,7 @@ ipcMain.handle(
} = {},
context?: string
) => {
const filePath = getCaptainData("files", subpath);
const filePath = getCaptainData("files", cleanPath(subpath));
const { dir: directory, ext, base } = path.parse(filePath);
const fileType = getFileType(filePath);

Expand Down Expand Up @@ -227,7 +227,7 @@ ipcMain.handle(
ipcMain.handle(
buildKey([ID.FILE], { suffix: ":copy" }),
async (_event, source: string, destination: string) => {
const filePath = getCaptainData("files", destination);
const filePath = getCaptainData("files", cleanPath(destination));
const { dir: directory } = path.parse(filePath);

// Ensure the directory exists, creating it if necessary
Expand Down
36 changes: 35 additions & 1 deletion src/shared/__tests__/string.test.ts
@@ -1,4 +1,4 @@
import { extractH1Headings, getFileType } from "../string";
import { cleanPath, extractH1Headings, getFileType } from "../string";

describe("extractH1Headings", () => {
it("extracts multiple H1 headings", () => {
Expand Down Expand Up @@ -54,3 +54,37 @@ describe("getFileType", () => {
expect(getFileType("archive.zip")).toBe("other");
});
});

describe("cleanPath", () => {
it("should handle simple paths without traversal", () => {
expect(cleanPath("foo/bar/baz")).toBe("foo/bar/baz");
});

it('should resolve paths with "." to the same path', () => {
expect(cleanPath("./foo/./bar/./baz")).toBe("foo/bar/baz");
});

it('should correctly handle paths with ".." to move up directories', () => {
expect(cleanPath("foo/bar/baz/../../qux")).toBe("foo/qux");
});

it("should return root when moving back past the root", () => {
expect(cleanPath("../../../foo")).toBe("foo");
});

it("should ignore empty segments caused by consecutive slashes", () => {
expect(cleanPath("foo//bar///baz")).toBe("foo/bar/baz");
});

it("should handle complex mixed paths", () => {
expect(cleanPath("../../../foo/bar/./../../baz")).toBe("baz");
});

it("should return a root slash for completely empty paths", () => {
expect(cleanPath("")).toBe("");
});

it("should return a root slash for paths that resolve to the root", () => {
expect(cleanPath("../../../../")).toBe("");
});
});
25 changes: 25 additions & 0 deletions src/shared/string.ts
Expand Up @@ -106,3 +106,28 @@ export function getFileType(filePath: string) {

return "other";
}

/**
* Cleans a given pathname by removing any directory traversal characters.
* @param {string} pathname - The path to be cleaned.
* @returns {string} The cleaned path.
*/
export function cleanPath(pathname: string): string {
const segments = pathname.split("/");
const stack: string[] = [];

for (const segment of segments) {
if (segment === "..") {
// Only pop the stack if there's something to go back from
if (stack.length > 0) {
stack.pop();
}
} else if (segment !== "." && segment !== "") {
// Ignore '.' which represents current directory
// Ignore empty segments caused by consecutive slashes
stack.push(segment);
}
}

return stack.join("/");
}

0 comments on commit 9bf22a1

Please sign in to comment.