feat: protect restores that would overwrite important internal path

This commit is contained in:
Nicolas Meienberger 2026-03-13 19:16:31 +01:00
parent 9632c77177
commit b10d5cd802
8 changed files with 133 additions and 11 deletions

View file

@ -7,8 +7,10 @@ export const RESTIC_CACHE_DIR = process.env.RESTIC_CACHE_DIR || "/var/lib/zeroby
export const DATABASE_URL = process.env.ZEROBYTE_DATABASE_URL || "/var/lib/zerobyte/data/zerobyte.db";
export const RESTIC_PASS_FILE = process.env.RESTIC_PASS_FILE || "/var/lib/zerobyte/data/restic.pass";
export const SSH_KEYS_DIR = "/var/lib/zerobyte/ssh";
export const RCLONE_CONFIG_DIR = process.env.RCLONE_CONFIG_DIR || "/root/.config/rclone";
export const RESTORE_BLOCKED_ROOTS = [REPOSITORY_BASE, RESTIC_CACHE_DIR, SSH_KEYS_DIR, RCLONE_CONFIG_DIR, "/app"];
export const DEFAULT_EXCLUDES = [RESTIC_PASS_FILE, REPOSITORY_BASE];

View file

@ -2,7 +2,7 @@ import * as fs from "node:fs/promises";
import * as os from "node:os";
import * as path from "node:path";
import { spawn } from "node:child_process";
import { OPERATION_TIMEOUT } from "../../../core/constants";
import { OPERATION_TIMEOUT, SSH_KEYS_DIR } from "../../../core/constants";
import { cryptoUtils } from "../../../utils/crypto";
import { toMessage } from "../../../utils/errors";
import { logger } from "@zerobyte/core/node";
@ -13,8 +13,6 @@ import type { VolumeBackend } from "../backend";
import { executeUnmount } from "../utils/backend-utils";
import { BACKEND_STATUS, type BackendConfig } from "~/schemas/volumes";
const SSH_KEYS_DIR = "/var/lib/zerobyte/ssh";
const getPrivateKeyPath = (mountPath: string) => {
const name = path.basename(mountPath);
return path.join(SSH_KEYS_DIR, `${name}.key`);

View file

@ -1,3 +1,6 @@
import * as fs from "node:fs/promises";
import * as os from "node:os";
import nodePath from "node:path";
import { randomUUID } from "node:crypto";
import { Readable } from "node:stream";
import { afterEach, beforeEach, describe, expect, mock, spyOn, test } from "bun:test";
@ -349,6 +352,86 @@ describe("repositoriesService.dumpSnapshot", () => {
});
});
describe("repositoriesService.restoreSnapshot", () => {
afterEach(() => {
mock.restore();
});
const setupRestoreSnapshotScenario = async () => {
const { organizationId, user } = await createTestSession();
const repository = await createTestRepository(organizationId);
spyOn(restic, "snapshots").mockResolvedValue([
{
id: "snapshot-restore",
short_id: "snapshot-restore",
time: new Date().toISOString(),
paths: ["/var/lib/zerobyte/volumes/vol123/_data"],
hostname: "host",
},
]);
const restoreMock = mock(() =>
Promise.resolve({
message_type: "summary" as const,
seconds_elapsed: 1,
percent_done: 100,
files_skipped: 0,
total_files: 1,
files_restored: 1,
total_bytes: 1,
bytes_restored: 1,
}),
);
spyOn(restic, "restore").mockImplementation(restoreMock);
return {
organizationId,
userId: user.id,
repositoryShortId: repository.shortId,
restoreMock,
};
};
test("rejects restore targets inside protected roots", async () => {
const { organizationId, userId, repositoryShortId, restoreMock } = await setupRestoreSnapshotScenario();
const targetPath = nodePath.join(os.tmpdir(), "zerobyte-restore-target");
await expect(
withContext({ organizationId, userId }, () =>
repositoriesService.restoreSnapshot(repositoryShortId, "snapshot-restore", { targetPath }),
),
).rejects.toThrow("Restore target path is not allowed");
expect(restoreMock).not.toHaveBeenCalled();
});
test("restores to a custom target outside protected roots", async () => {
const { organizationId, userId, repositoryShortId, restoreMock } = await setupRestoreSnapshotScenario();
const targetPath = await fs.mkdtemp(nodePath.join(process.cwd(), "restore-target-"));
try {
await withContext({ organizationId, userId }, () =>
repositoriesService.restoreSnapshot(repositoryShortId, "snapshot-restore", { targetPath }),
);
} finally {
await fs.rm(targetPath, { recursive: true, force: true });
}
expect(restoreMock).toHaveBeenCalledWith(
expect.objectContaining({
backend: "local",
}),
"snapshot-restore",
targetPath,
expect.objectContaining({
organizationId,
basePath: "/var/lib/zerobyte/volumes/vol123/_data",
}),
);
});
});
describe("repositoriesService.getRetentionCategories", () => {
afterEach(() => {
mock.restore();

View file

@ -1,3 +1,4 @@
import * as os from "node:os";
import nodePath from "node:path";
import { and, eq } from "drizzle-orm";
import { BadRequestError, ConflictError, InternalServerError, NotFoundError } from "http-errors-enhanced";
@ -8,7 +9,9 @@ import {
type ResticStatsDto,
repositoryConfigSchema,
} from "@zerobyte/core/restic";
import { isPathWithin } from "@zerobyte/core/utils";
import { config as appConfig } from "~/server/core/config";
import { DATABASE_URL, RESTORE_BLOCKED_ROOTS, RESTIC_PASS_FILE } from "~/server/core/constants";
import { serverEvents } from "~/server/core/events";
import { getOrganizationId } from "~/server/core/request-context";
import { logger } from "@zerobyte/core/node";
@ -41,6 +44,16 @@ const emptyRepositoryStats: ResticStatsDto = {
snapshots_count: 0,
};
const getBlockedRestoreTargets = () => {
return [
...RESTORE_BLOCKED_ROOTS,
DATABASE_URL === ":memory:" ? undefined : nodePath.dirname(nodePath.resolve(DATABASE_URL)),
nodePath.dirname(nodePath.resolve(RESTIC_PASS_FILE)),
nodePath.resolve(os.tmpdir()),
appConfig.provisioningPath ? nodePath.dirname(nodePath.resolve(appConfig.provisioningPath)) : undefined,
].filter((e) => e !== undefined);
};
const findRepository = async (shortId: ShortId) => {
const organizationId = getOrganizationId();
return await db.query.repositoriesTable.findFirst({
@ -309,6 +322,16 @@ const restoreSnapshot = async (
}
const target = options?.targetPath || "/";
const resolvedTarget = nodePath.resolve(target);
const blockedTargets = getBlockedRestoreTargets();
for (const blockedTarget of blockedTargets) {
if (isPathWithin(blockedTarget, resolvedTarget)) {
throw new BadRequestError(
"Restore target path is not allowed. Restoring to this path could overwrite critical system files or application data.",
);
}
}
const { paths } = await getSnapshotDetails(repository.shortId, snapshotId);
const basePath = findCommonAncestor(paths);

View file

@ -1,5 +1,5 @@
import fs from "node:fs/promises";
import path from "node:path";
import { isPathWithin } from "@zerobyte/core/utils";
type MountInfo = {
mountPoint: string;
@ -12,11 +12,6 @@ export type StatFs = {
free: number;
};
function isPathWithin(base: string, target: string): boolean {
const rel = path.posix.relative(base, target);
return rel === "" || (!rel.startsWith("..") && !path.isAbsolute(rel));
}
function unescapeMount(s: string): string {
return s.replace(/\\([0-7]{3})/g, (_, oct) => String.fromCharCode(parseInt(oct, 8)));
}

View file

@ -1,5 +1,5 @@
import { describe, expect, test } from "bun:test";
import { normalizeAbsolutePath } from "@zerobyte/core/utils";
import { isPathWithin, normalizeAbsolutePath } from "@zerobyte/core/utils";
describe("normalizeAbsolutePath", () => {
test("handles undefined and empty inputs", () => {
@ -37,3 +37,15 @@ describe("normalizeAbsolutePath", () => {
expect(normalizeAbsolutePath("/foo/../../bar")).toBe("/bar");
});
});
describe("isPathWithin", () => {
test("matches the same path and nested paths", () => {
expect(isPathWithin("/var/lib/zerobyte", "/var/lib/zerobyte")).toBe(true);
expect(isPathWithin("/var/lib/zerobyte", "/var/lib/zerobyte/data/restic.pass")).toBe(true);
});
test("does not match sibling or parent-escape paths", () => {
expect(isPathWithin("/var/lib/zerobyte/data", "/var/lib/zerobyte/database")).toBe(false);
expect(isPathWithin("/var/lib/zerobyte/data", "/var/lib/zerobyte/data/../ssh")).toBe(false);
});
});

View file

@ -1,4 +1,4 @@
export { safeJsonParse } from "./json.js";
export { normalizeAbsolutePath } from "./path.js";
export { isPathWithin, normalizeAbsolutePath } from "./path.js";
export { findCommonAncestor } from "./common-ancestor.js";
export { FILE_MODES, writeFileWithMode } from "./fs.js";

View file

@ -47,3 +47,12 @@ export const normalizeAbsolutePath = (value?: string): string => {
return withSingleLeadingSlash || "/";
};
export const isPathWithin = (base: string, target: string): boolean => {
const normalizedBase = normalizeAbsolutePath(base);
const normalizedTarget = normalizeAbsolutePath(target);
return (
normalizedBase === "/" || normalizedTarget === normalizedBase || normalizedTarget.startsWith(`${normalizedBase}/`)
);
};