diff --git a/app/client/components/file-browsers/local-file-browser.tsx b/app/client/components/file-browsers/local-file-browser.tsx index 2e55e13f..920ed891 100644 --- a/app/client/components/file-browsers/local-file-browser.tsx +++ b/app/client/components/file-browsers/local-file-browser.tsx @@ -3,13 +3,7 @@ import { browseFilesystemOptions } from "~/client/api-client/@tanstack/react-que import { FileBrowser, type FileBrowserUiProps } from "~/client/components/file-browsers/file-browser"; import { useFileBrowser } from "~/client/hooks/use-file-browser"; import { parseError } from "~/client/lib/errors"; - -const normalizeAbsolutePath = (path?: string): string => { - if (!path) return "/"; - const withLeadingSlash = path.startsWith("/") ? path : `/${path}`; - const trimmed = withLeadingSlash.replace(/\/+$/, ""); - return trimmed || "/"; -}; +import { normalizeAbsolutePath } from "~/utils/path"; type LocalFileBrowserProps = FileBrowserUiProps & { initialPath?: string; diff --git a/app/client/components/file-browsers/snapshot-tree-browser.tsx b/app/client/components/file-browsers/snapshot-tree-browser.tsx index af8228ec..b4aa69bb 100644 --- a/app/client/components/file-browsers/snapshot-tree-browser.tsx +++ b/app/client/components/file-browsers/snapshot-tree-browser.tsx @@ -4,13 +4,7 @@ import { listSnapshotFilesOptions } from "~/client/api-client/@tanstack/react-qu import { FileBrowser, type FileBrowserUiProps } from "~/client/components/file-browsers/file-browser"; import { useFileBrowser } from "~/client/hooks/use-file-browser"; import { parseError } from "~/client/lib/errors"; - -const normalizeAbsolutePath = (path?: string): string => { - if (!path) return "/"; - const withLeadingSlash = path.startsWith("/") ? path : `/${path}`; - const trimmed = withLeadingSlash.replace(/\/+$/, ""); - return trimmed || "/"; -}; +import { normalizeAbsolutePath } from "~/utils/path"; type SnapshotTreeBrowserProps = FileBrowserUiProps & { repositoryId: string; diff --git a/app/client/components/ui/button.tsx b/app/client/components/ui/button.tsx index d8580808..48f37a75 100644 --- a/app/client/components/ui/button.tsx +++ b/app/client/components/ui/button.tsx @@ -53,10 +53,10 @@ function Button({ return (
{props.children}
diff --git a/app/client/hooks/useMinimumDuration.ts b/app/client/hooks/useMinimumDuration.ts index 2721f965..d77dc11a 100644 --- a/app/client/hooks/useMinimumDuration.ts +++ b/app/client/hooks/useMinimumDuration.ts @@ -13,7 +13,7 @@ export function useMinimumDuration(isActive: boolean, minimumDuration: number): } startTimeRef.current = Date.now(); setDisplayActive(true); - } else if (!isActive && startTimeRef.current !== null) { + } else if (startTimeRef.current !== null) { const elapsed = Date.now() - startTimeRef.current; const remaining = Math.max(0, minimumDuration - elapsed); diff --git a/app/server/modules/repositories/helpers/dump.ts b/app/server/modules/repositories/helpers/dump.ts index 78306e31..cbe85e96 100644 --- a/app/server/modules/repositories/helpers/dump.ts +++ b/app/server/modules/repositories/helpers/dump.ts @@ -1,30 +1,7 @@ import { BadRequestError } from "http-errors-enhanced"; import path from "node:path"; import { findCommonAncestor } from "~/utils/common-ancestor"; - -const normalizeAbsolutePath = (value?: string): string => { - const trimmed = value?.trim(); - if (!trimmed) return "/"; - - const normalizedInput = decodeURIComponent(trimmed).replace(/\\+/g, "/"); - const withLeadingSlash = normalizedInput.startsWith("/") ? normalizedInput : `/${normalizedInput}`; - const normalized = path.posix.normalize(withLeadingSlash); - - if (!normalized || normalized === "." || normalized.startsWith("..")) { - return "/"; - } - - const withoutTrailingSlash = normalized.replace(/\/+$/, ""); - if (!withoutTrailingSlash) { - return "/"; - } - - const withSingleLeadingSlash = withoutTrailingSlash.startsWith("/") - ? `/${withoutTrailingSlash.replace(/^\/+/, "")}` - : `/${withoutTrailingSlash}`; - - return withSingleLeadingSlash || "/"; -}; +import { normalizeAbsolutePath } from "~/utils/path"; const sanitizeFilenamePart = (value: string): string => { const sanitized = value.replace(/[^a-zA-Z0-9._-]/g, "_").replace(/^_+|_+$/g, ""); diff --git a/app/server/modules/repositories/repositories.controller.ts b/app/server/modules/repositories/repositories.controller.ts index 4b1e4cd9..9c316106 100644 --- a/app/server/modules/repositories/repositories.controller.ts +++ b/app/server/modules/repositories/repositories.controller.ts @@ -2,6 +2,7 @@ import { Readable } from "node:stream"; import { Hono } from "hono"; import { validator } from "hono-openapi"; import { streamSSE } from "hono/streaming"; +import contentDisposition from "content-disposition"; import { createRepositoryBody, createRepositoryDto, @@ -51,7 +52,6 @@ import { repositoriesService } from "./repositories.service"; import { getRcloneRemoteInfo, listRcloneRemotes } from "../../utils/rclone"; import { requireAuth, requireOrgAdmin } from "../auth/auth.middleware"; import { toMessage } from "~/server/utils/errors"; -import { sanitizeContentDispositionFilename } from "~/server/utils/sanitize"; import { requireDevPanel } from "../auth/dev-panel.middleware"; import { getSnapshotDuration } from "../../utils/snapshots"; @@ -182,15 +182,13 @@ export const repositoriesController = new Hono() signal.addEventListener("abort", () => dumpStream.abort(), { once: true }); } - const safeFilename = sanitizeContentDispositionFilename(dumpStream.filename); - const webStream = Readable.toWeb(dumpStream.stream) as unknown as ReadableStream; return new Response(webStream, { status: 200, headers: { "Content-Type": "application/x-tar", - "Content-Disposition": `attachment; filename="${safeFilename}"`, + "Content-Disposition": contentDisposition(dumpStream.filename || "snapshot.tar"), "X-Content-Type-Options": "nosniff", }, }); diff --git a/app/server/modules/repositories/repositories.service.ts b/app/server/modules/repositories/repositories.service.ts index f5ac8f5c..db9fee9e 100644 --- a/app/server/modules/repositories/repositories.service.ts +++ b/app/server/modules/repositories/repositories.service.ts @@ -399,11 +399,12 @@ const dumpSnapshot = async (shortId: string, snapshotId: string, path?: string) } const releaseLock = await repoMutex.acquireShared(repository.id, `dump:${snapshotId}`); + let dumpStream: Awaited> | undefined = undefined; try { const snapshot = await getSnapshotDetails(repository.shortId, snapshotId); const preparedDump = prepareSnapshotDump({ snapshotId, snapshotPaths: snapshot.paths, requestedPath: path }); - const dumpStream = await restic.dump(repository.config, preparedDump.snapshotRef, { + dumpStream = await restic.dump(repository.config, preparedDump.snapshotRef, { organizationId, path: preparedDump.path, }); @@ -421,6 +422,9 @@ const dumpSnapshot = async (shortId: string, snapshotId: string, path?: string) return { ...dumpStream, completion, filename: preparedDump.filename }; } catch (error) { + if (dumpStream) { + dumpStream.abort(); + } releaseLock(); throw error; } diff --git a/app/server/utils/restic.test.ts b/app/server/utils/restic.test.ts index d099f53d..5f15b91e 100644 --- a/app/server/utils/restic.test.ts +++ b/app/server/utils/restic.test.ts @@ -11,7 +11,7 @@ const successfulRestoreSummary = JSON.stringify({ let lastSafeSpawnArgs: string[] = []; -const safeSpawnMock = mock((params: Parameters[0]) => { +const safeSpawnMock = mock((params: spawnModule.SafeSpawnParams) => { lastSafeSpawnArgs = params.args; return Promise.resolve({ diff --git a/app/server/utils/restic.ts b/app/server/utils/restic.ts index e1809bcc..b1ce330b 100644 --- a/app/server/utils/restic.ts +++ b/app/server/utils/restic.ts @@ -1,4 +1,5 @@ import crypto from "node:crypto"; +import { normalizeAbsolutePath } from "~/utils/path"; import fs from "node:fs/promises"; import os from "node:os"; import path from "node:path"; @@ -570,10 +571,7 @@ const normalizeDumpPath = (pathToDump?: string): string => { } })(); - const withLeadingSlash = decodedPath.startsWith("/") ? decodedPath : `/${decodedPath}`; - const normalizedPath = withLeadingSlash.replace(/\/+$/, ""); - - return normalizedPath || "/"; + return normalizeAbsolutePath(decodedPath); }; const dump = async ( @@ -643,6 +641,9 @@ const dump = async ( await cleanup(); }); + completion.catch(() => {}); + const completionPromise = new Promise((res, rej) => completion.then(res, rej)); + if (!stream) { await cleanup(); throw new Error("Failed to initialize restic dump stream"); @@ -650,7 +651,7 @@ const dump = async ( return { stream, - completion, + completion: completionPromise, abort: () => { if (abortController) { abortController.abort(); diff --git a/app/server/utils/sanitize.ts b/app/server/utils/sanitize.ts index 13828d3c..d72e2b7e 100644 --- a/app/server/utils/sanitize.ts +++ b/app/server/utils/sanitize.ts @@ -20,15 +20,3 @@ export const sanitizeSensitiveData = (text: string): string => { return sanitized; }; - -/** - * Sanitizes a filename for use in HTTP Content-Disposition header - * Removes control characters and replaces special characters to prevent header injection - */ -export const sanitizeContentDispositionFilename = (filename: string): string => { - const sanitized = filename - .replace(/[\r\n]/g, "") - .replace(/["\\]/g, "_") - .trim(); - return sanitized || "snapshot.tar"; -}; diff --git a/app/server/utils/spawn.ts b/app/server/utils/spawn.ts index d3c605b2..e5a3ec9b 100644 --- a/app/server/utils/spawn.ts +++ b/app/server/utils/spawn.ts @@ -32,25 +32,39 @@ export const exec = async ({ command, args = [], env = {}, ...rest }: ExecProps) } }; -export interface SafeSpawnParams { +export interface SafeSpawnParamsBase { command: string; args: string[]; env?: NodeJS.ProcessEnv; signal?: AbortSignal; - stdoutMode?: "lines" | "raw"; - onStdout?: (line: string) => void; onStderr?: (error: string) => void; onSpawn?: (child: ReturnType) => void; } -type SpawnResult = { +export interface SafeSpawnParamsLines extends SafeSpawnParamsBase { + stdoutMode?: "lines"; + onStdout?: (line: string) => void; +} + +export interface SafeSpawnParamsRaw extends SafeSpawnParamsBase { + stdoutMode: "raw"; + onStdout?: never; +} + +export type SafeSpawnParams = SafeSpawnParamsLines | SafeSpawnParamsRaw; + +export type SpawnResult = { exitCode: number; summary: string; error: string; }; -export const safeSpawn = (params: SafeSpawnParams) => { - const { command, args, env = {}, signal, stdoutMode = "lines", onStdout, onStderr, onSpawn } = params; +export function safeSpawn(params: SafeSpawnParamsLines): Promise; +export function safeSpawn(params: SafeSpawnParamsRaw): Promise; +export function safeSpawn(params: SafeSpawnParams): Promise { + const { command, args, env = {}, signal, onStderr, onSpawn } = params; + const stdoutMode = params.stdoutMode ?? "lines"; + const onStdout = stdoutMode === "lines" ? params.onStdout : undefined; let lastStdout = ""; let lastStderr = ""; @@ -106,4 +120,4 @@ export const safeSpawn = (params: SafeSpawnParams) => { }); }); }); -}; +} diff --git a/app/utils/__tests__/path.test.ts b/app/utils/__tests__/path.test.ts new file mode 100644 index 00000000..89d7d681 --- /dev/null +++ b/app/utils/__tests__/path.test.ts @@ -0,0 +1,39 @@ +import { describe, expect, test } from "bun:test"; +import { normalizeAbsolutePath } from "../path"; + +describe("normalizeAbsolutePath", () => { + test("handles undefined and empty inputs", () => { + expect(normalizeAbsolutePath()).toBe("/"); + expect(normalizeAbsolutePath("")).toBe("/"); + expect(normalizeAbsolutePath(" ")).toBe("/"); + }); + + test("normalizes posix paths", () => { + expect(normalizeAbsolutePath("/foo/bar")).toBe("/foo/bar"); + expect(normalizeAbsolutePath("foo/bar")).toBe("/foo/bar"); + expect(normalizeAbsolutePath("/foo//bar")).toBe("/foo/bar"); + expect(normalizeAbsolutePath("/foo/./bar")).toBe("/foo/bar"); + expect(normalizeAbsolutePath("/foo/../bar")).toBe("/bar"); + }); + + test("trims trailing slashes", () => { + expect(normalizeAbsolutePath("/foo/bar/")).toBe("/foo/bar"); + expect(normalizeAbsolutePath("/foo/bar//")).toBe("/foo/bar"); + }); + + test("handles windows style paths from URI", () => { + expect(normalizeAbsolutePath("foo\\\\bar")).toBe("/foo/bar"); + expect(normalizeAbsolutePath("foo\\\\bar\\\\")).toBe("/foo/bar"); + }); + + test("handles URI encoded paths", () => { + expect(normalizeAbsolutePath("/foo%20bar")).toBe("/foo bar"); + expect(normalizeAbsolutePath("foo%2Fbar")).toBe("/foo/bar"); + }); + + test("prevents parent traversal beyond root", () => { + expect(normalizeAbsolutePath("..")).toBe("/"); + expect(normalizeAbsolutePath("/..")).toBe("/"); + expect(normalizeAbsolutePath("/foo/../../bar")).toBe("/bar"); + }); +}); diff --git a/app/utils/path.ts b/app/utils/path.ts new file mode 100644 index 00000000..0011a196 --- /dev/null +++ b/app/utils/path.ts @@ -0,0 +1,40 @@ +export const normalizeAbsolutePath = (value?: string): string => { + const trimmed = value?.trim(); + if (!trimmed) return "/"; + + const normalizedInput = decodeURIComponent(trimmed).replace(/\\+/g, "/"); + const withLeadingSlash = normalizedInput.startsWith("/") ? normalizedInput : `/${normalizedInput}`; + + const parts = withLeadingSlash.split("/"); + const stack: string[] = []; + + for (const part of parts) { + if (part === "" || part === ".") { + continue; + } + if (part === "..") { + if (stack.length > 0) { + stack.pop(); + } + } else { + stack.push(part); + } + } + + let normalized = "/" + stack.join("/"); + + if (!normalized || normalized === "." || normalized.startsWith("..")) { + return "/"; + } + + const withoutTrailingSlash = normalized.replace(/\/+$/, ""); + if (!withoutTrailingSlash) { + return "/"; + } + + const withSingleLeadingSlash = withoutTrailingSlash.startsWith("/") + ? `/${withoutTrailingSlash.replace(/^\/+/, "")}` + : `/${withoutTrailingSlash}`; + + return withSingleLeadingSlash || "/"; +}; diff --git a/bun.lock b/bun.lock index 09909e79..66e1cf32 100644 --- a/bun.lock +++ b/bun.lock @@ -39,6 +39,7 @@ "clsx": "^2.1.1", "commander": "^14.0.2", "consola": "^3.4.2", + "content-disposition": "^1.0.1", "cron-parser": "^5.5.0", "date-fns": "^4.1.0", "dither-plugin": "^1.1.1", @@ -81,6 +82,7 @@ "@testing-library/react": "^16.3.2", "@total-typescript/shoehorn": "^0.1.2", "@types/bun": "^1.3.6", + "@types/content-disposition": "^0.5.9", "@types/node": "^25.2.3", "@types/react": "^19.2.14", "@types/react-dom": "^19.2.3", @@ -831,6 +833,8 @@ "@types/bun": ["@types/bun@1.3.9", "", { "dependencies": { "bun-types": "1.3.9" } }, "sha512-KQ571yULOdWJiMH+RIWIOZ7B2RXQGpL1YQrBtLIV3FqDcCu6FsbFUBwhdKUlCKUpS3PJDsHlJ1QKlpxoVR+xtw=="], + "@types/content-disposition": ["@types/content-disposition@0.5.9", "", {}, "sha512-8uYXI3Gw35MhiVYhG3s295oihrxRyytcRHjSjqnqZVDDy/xcGBRny7+Xj1Wgfhv5QzRtN2hB2dVRBUX9XW3UcQ=="], + "@types/d3-array": ["@types/d3-array@3.2.2", "", {}, "sha512-hOLWVbm7uRza0BYXpIIW5pxfrKe0W+D5lrFiAEYR+pb6w3N2SwSMaJbXdUfSEv+dT4MfHBLtn5js0LAWaO6otw=="], "@types/d3-color": ["@types/d3-color@3.1.3", "", {}, "sha512-iO90scth9WAbmgv7ogoq57O9YpKmFBbmoEoCHDB2xMBY0+/KVrqAaCDyCE16dUspeOvIxFFRI+0sEtqDqy2b4A=="], @@ -989,6 +993,8 @@ "consola": ["consola@3.4.2", "", {}, "sha512-5IKcdX0nnYavi6G7TtOhwkYzyjfJlatbjMjuLSfE2kYT5pMDOilZ4OvMhi637CcDICTmz3wARPoyhqyX1Y+XvA=="], + "content-disposition": ["content-disposition@1.0.1", "", {}, "sha512-oIXISMynqSqm241k6kcQ5UwttDILMK4BiurCfGEREw6+X9jkkpEe5T9FZaApyLGGOnFuyMWZpdolTXMtvEJ08Q=="], + "convert-source-map": ["convert-source-map@2.0.0", "", {}, "sha512-Kvp459HrV2FEJ1CAsi1Ku+MY3kasH19TFykTz2xWmMeq6bk2NU3XXvfJ+Q61m0xktWwt+1HSYf3JZsTms3aRJg=="], "cookie-es": ["cookie-es@2.0.0", "", {}, "sha512-RAj4E421UYRgqokKUmotqAwuplYw15qtdXfY+hGzgCJ/MBjCVZcSoHK/kH9kocfjRjcDME7IiDWR/1WX1TM2Pg=="], diff --git a/package.json b/package.json index 77239f05..c3e9d32f 100644 --- a/package.json +++ b/package.json @@ -60,6 +60,7 @@ "clsx": "^2.1.1", "commander": "^14.0.2", "consola": "^3.4.2", + "content-disposition": "^1.0.1", "cron-parser": "^5.5.0", "date-fns": "^4.1.0", "dither-plugin": "^1.1.1", @@ -102,6 +103,7 @@ "@testing-library/react": "^16.3.2", "@total-typescript/shoehorn": "^0.1.2", "@types/bun": "^1.3.6", + "@types/content-disposition": "^0.5.9", "@types/node": "^25.2.3", "@types/react": "^19.2.14", "@types/react-dom": "^19.2.3",