chore: pr feedbacks
This commit is contained in:
parent
49abcc5a12
commit
d650c53c69
15 changed files with 127 additions and 70 deletions
|
|
@ -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;
|
||||
|
|
|
|||
|
|
@ -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;
|
||||
|
|
|
|||
|
|
@ -53,10 +53,10 @@ function Button({
|
|||
|
||||
return (
|
||||
<Comp
|
||||
{...props}
|
||||
data-slot="button"
|
||||
className={cn(buttonVariants({ variant, size, className }), "transition-all")}
|
||||
disabled={disabled || loading || isLoading}
|
||||
{...props}
|
||||
>
|
||||
<Loader2 className={cn("h-4 w-4 animate-spin absolute", { invisible: !isLoading })} />
|
||||
<div className={cn("flex items-center justify-center", { invisible: isLoading })}>{props.children}</div>
|
||||
|
|
|
|||
|
|
@ -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);
|
||||
|
||||
|
|
|
|||
|
|
@ -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, "");
|
||||
|
|
|
|||
|
|
@ -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<Uint8Array>;
|
||||
|
||||
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",
|
||||
},
|
||||
});
|
||||
|
|
|
|||
|
|
@ -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<ReturnType<typeof restic.dump>> | 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;
|
||||
}
|
||||
|
|
|
|||
|
|
@ -11,7 +11,7 @@ const successfulRestoreSummary = JSON.stringify({
|
|||
|
||||
let lastSafeSpawnArgs: string[] = [];
|
||||
|
||||
const safeSpawnMock = mock((params: Parameters<typeof spawnModule.safeSpawn>[0]) => {
|
||||
const safeSpawnMock = mock((params: spawnModule.SafeSpawnParams) => {
|
||||
lastSafeSpawnArgs = params.args;
|
||||
|
||||
return Promise.resolve({
|
||||
|
|
|
|||
|
|
@ -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<void>((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();
|
||||
|
|
|
|||
|
|
@ -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";
|
||||
};
|
||||
|
|
|
|||
|
|
@ -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<typeof spawn>) => 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<SpawnResult>;
|
||||
export function safeSpawn(params: SafeSpawnParamsRaw): Promise<SpawnResult>;
|
||||
export function safeSpawn(params: SafeSpawnParams): Promise<SpawnResult> {
|
||||
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) => {
|
|||
});
|
||||
});
|
||||
});
|
||||
};
|
||||
}
|
||||
|
|
|
|||
39
app/utils/__tests__/path.test.ts
Normal file
39
app/utils/__tests__/path.test.ts
Normal file
|
|
@ -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");
|
||||
});
|
||||
});
|
||||
40
app/utils/path.ts
Normal file
40
app/utils/path.ts
Normal file
|
|
@ -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 || "/";
|
||||
};
|
||||
6
bun.lock
6
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=="],
|
||||
|
|
|
|||
|
|
@ -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",
|
||||
|
|
|
|||
Loading…
Reference in a new issue