Compare commits

...

1 commit

Author SHA1 Message Date
Nicolas Meienberger
ca4c74fcb3
fix(backup-config): throw if include patterns have un-supported characters 2026-06-01 19:39:15 +02:00
7 changed files with 168 additions and 14 deletions

View file

@ -130,6 +130,37 @@ describe("backup path options", () => {
).toThrow("Include pattern escapes volume root"); ).toThrow("Include pattern escapes volume root");
}); });
test("rejects unsupported characters in selected include paths", () => {
const volumePath = "/var/lib/zerobyte/volumes/vol123/_data";
expect(() => createOptions(createPathOptions({ includePaths: ["/Photos\0/etc/passwd"] }), volumePath)).toThrow(
"Include path contains an unsupported path character",
);
});
test("allows line breaks in selected include paths", () => {
const volumePath = "/var/lib/zerobyte/volumes/vol123/_data";
const options = createOptions(
createPathOptions({ includePaths: ["/Photos\n2026", "/Photos\r2025"] }),
volumePath,
);
expect(options.includePaths).toEqual([
path.join(volumePath, "Photos\n2026"),
path.join(volumePath, "Photos\r2025"),
]);
});
test("rejects unsupported characters in include patterns", () => {
const volumePath = "/var/lib/zerobyte/volumes/vol123/_data";
for (const includePattern of ["/Photos\0/etc/passwd", "/Photos\n/etc/passwd", "/Photos\r/etc/passwd"]) {
expect(() => createOptions(createPathOptions({ includePatterns: [includePattern] }), volumePath)).toThrow(
"Include pattern contains an unsupported path character",
);
}
});
test("anchors generated include patterns under the volume path", () => { test("anchors generated include patterns under the volume path", () => {
const volumePath = "/var/lib/zerobyte/volumes/vol123/_data"; const volumePath = "/var/lib/zerobyte/volumes/vol123/_data";

View file

@ -1,8 +1,15 @@
import path from "node:path"; import path from "node:path";
import type { BackupRunPayload } from "@zerobyte/contracts/agent-protocol"; import type { BackupRunPayload } from "@zerobyte/contracts/agent-protocol";
import { hasPathListSeparator } from "@zerobyte/core/utils";
type BackupOptions = BackupRunPayload["options"]; type BackupOptions = BackupRunPayload["options"];
const validateIncludeEntry = (entry: string, name: string, format: "raw" | "text") => {
if (hasPathListSeparator(entry, format)) {
throw new Error(`${name} contains an unsupported path character: ${entry}`);
}
};
export const processPattern = (pattern: string, volumePath: string, relative = false) => { export const processPattern = (pattern: string, volumePath: string, relative = false) => {
const isNegated = pattern.startsWith("!"); const isNegated = pattern.startsWith("!");
const p = isNegated ? pattern.slice(1) : pattern; const p = isNegated ? pattern.slice(1) : pattern;
@ -41,8 +48,16 @@ export const createBackupOptions = (
signal, signal,
exclude: params.options.excludePatterns?.map((p) => processPattern(p, volumePath)) ?? undefined, exclude: params.options.excludePatterns?.map((p) => processPattern(p, volumePath)) ?? undefined,
excludeIfPresent: params.options.excludeIfPresent ?? undefined, excludeIfPresent: params.options.excludeIfPresent ?? undefined,
includePaths: params.options.includePaths?.map((p) => processPattern(p, volumePath, true)) ?? undefined, includePaths:
includePatterns: params.options.includePatterns?.map((p) => processPattern(p, volumePath, true)) ?? undefined, params.options.includePaths?.map((p) => {
validateIncludeEntry(p, "Include path", "raw");
return processPattern(p, volumePath, true);
}) ?? undefined,
includePatterns:
params.options.includePatterns?.map((p) => {
validateIncludeEntry(p, "Include pattern", "text");
return processPattern(p, volumePath, true);
}) ?? undefined,
customResticParams: params.options.customResticParams ?? [], customResticParams: params.options.customResticParams ?? [],
compressionMode: params.options.compressionMode, compressionMode: params.options.compressionMode,
}); });

View file

@ -158,6 +158,74 @@ describe("backup command", () => {
expect(patternIncludeContent).toBe("/mnt/data/**/*.zip"); expect(patternIncludeContent).toBe("/mnt/data/**/*.zip");
}); });
test("writes raw include paths containing line breaks as single entries", async () => {
const lineBreakDir = "/mnt/data/photos\n2026";
const carriageReturnDir = "/mnt/data/photos\r2025";
let rawIncludeContent = "";
setup({
onSpawnCall: async (params) => {
const rawIncludeIndex = params.args.indexOf("--files-from-raw");
if (rawIncludeIndex > -1) {
rawIncludeContent = await Bun.file(params.args[rawIncludeIndex + 1]!).text();
}
},
});
await runBackup(
config,
"/mnt/data",
{
organizationId: "org-1",
includePaths: [lineBreakDir, carriageReturnDir],
},
mockDeps,
);
expect(rawIncludeContent).toBe(`${lineBreakDir}\0${carriageReturnDir}\0`);
});
test("rejects unsupported characters before writing raw include files", async () => {
const { getArgs } = setup();
const error = await runBackupError(
config,
"/mnt/data",
{
organizationId: "org-1",
includePaths: ["/mnt/data/safe\0/etc/passwd"],
includePatterns: ["/mnt/data/**/*.zip"],
},
mockDeps,
);
expect(String(error.message)).toContain("includePaths contains an unsupported path character");
expect(getArgs()).toEqual([]);
});
test("rejects unsupported characters before writing include pattern files", async () => {
const { getArgs } = setup();
for (const includePattern of [
"/mnt/data/safe\0/etc/passwd",
"/mnt/data/safe\n/etc/passwd",
"/mnt/data/safe\r/etc/passwd",
]) {
const error = await runBackupError(
config,
"/mnt/data",
{
organizationId: "org-1",
includePatterns: [includePattern],
},
mockDeps,
);
expect(String(error.message)).toContain("includePatterns contains an unsupported path character");
expect(getArgs()).toEqual([]);
}
});
test("always includes DEFAULT_EXCLUDES as --exclude args", async () => { test("always includes DEFAULT_EXCLUDES as --exclude args", async () => {
const { getOptionValues } = setup(); const { getOptionValues } = setup();
await runBackup(config, "/mnt/data", { organizationId: "org-1" }, mockDeps); await runBackup(config, "/mnt/data", { organizationId: "org-1" }, mockDeps);
@ -232,7 +300,9 @@ describe("backup command", () => {
const error = await runBackupError(config, "/mnt/data", { organizationId: "org-1" }, mockDeps); const error = await runBackupError(config, "/mnt/data", { organizationId: "org-1" }, mockDeps);
expect(error).toBeInstanceOf(ResticError); expect(error).toBeInstanceOf(ResticError);
expect((error as ResticError).summary).toBe("Command failed: An error occurred while executing the command."); expect((error as ResticError).summary).toBe(
"Command failed: An error occurred while executing the command.",
);
expect((error as ResticError).details).toBe( expect((error as ResticError).details).toBe(
"Permissions 0755 for '/tmp/zerobyte-ssh-key' are too open.\nThis private key will be ignored.", "Permissions 0755 for '/tmp/zerobyte-ssh-key' are too open.\nThis private key will be ignored.",
); );
@ -327,7 +397,8 @@ describe("backup command", () => {
test("ignores valid JSON lines that do not match the progress schema", async () => { test("ignores valid JSON lines that do not match the progress schema", async () => {
const progressUpdates: unknown[] = []; const progressUpdates: unknown[] = [];
setup({ setup({
onSpawnCall: (params) => params.onStdout?.(JSON.stringify({ message_type: "verbose_status", action: "scan" })), onSpawnCall: (params) =>
params.onStdout?.(JSON.stringify({ message_type: "verbose_status", action: "scan" })),
}); });
await runBackup( await runBackup(

View file

@ -13,13 +13,21 @@ import { validateCustomResticParams } from "../helpers/validate-custom-params";
import { createResticError, isResticError } from "../error"; import { createResticError, isResticError } from "../error";
import { logger, safeSpawn } from "../../node"; import { logger, safeSpawn } from "../../node";
import type { ResticDeps } from "../types"; import type { ResticDeps } from "../types";
import { toMessage } from "../../utils"; import { hasPathListSeparator, toMessage } from "../../utils";
class ResticBackupCommandError extends Data.TaggedError("ResticBackupCommandError")<{ class ResticBackupCommandError extends Data.TaggedError("ResticBackupCommandError")<{
cause: unknown; cause: unknown;
message: string; message: string;
}> {} }> {}
const validateEntries = (entries: string[], optionName: string, format: "raw" | "text") => {
for (const entry of entries) {
if (hasPathListSeparator(entry, format)) {
throw new Error(`${optionName} contains an unsupported path character: ${entry}`);
}
}
};
export const backup = ( export const backup = (
config: RepositoryConfig, config: RepositoryConfig,
source: string, source: string,
@ -41,7 +49,6 @@ export const backup = (
return Effect.tryPromise({ return Effect.tryPromise({
try: async () => { try: async () => {
const repoUrl = buildRepoUrl(config); const repoUrl = buildRepoUrl(config);
const env = await buildEnv(config, options.organizationId, deps);
const args: string[] = ["--repo", repoUrl, "backup", "--compression", options.compressionMode ?? "auto"]; const args: string[] = ["--repo", repoUrl, "backup", "--compression", options.compressionMode ?? "auto"];
@ -66,6 +73,8 @@ export const backup = (
(!options.includePatterns || options.includePatterns.length === 0); (!options.includePatterns || options.includePatterns.length === 0);
if (options.includePatterns?.length) { if (options.includePatterns?.length) {
validateEntries(options.includePatterns, "includePatterns", "text");
const tmp = await fs.mkdtemp(path.join(os.tmpdir(), "zerobyte-restic-include-")); const tmp = await fs.mkdtemp(path.join(os.tmpdir(), "zerobyte-restic-include-"));
includeFile = path.join(tmp, "include.txt"); includeFile = path.join(tmp, "include.txt");
@ -75,6 +84,8 @@ export const backup = (
} }
if (options.includePaths?.length) { if (options.includePaths?.length) {
validateEntries(options.includePaths, "includePaths", "raw");
const tmp = await fs.mkdtemp(path.join(os.tmpdir(), "zerobyte-restic-include-raw-")); const tmp = await fs.mkdtemp(path.join(os.tmpdir(), "zerobyte-restic-include-raw-"));
rawIncludeFile = path.join(tmp, "include.raw"); rawIncludeFile = path.join(tmp, "include.raw");
@ -114,6 +125,7 @@ export const backup = (
} }
} }
const env = await buildEnv(config, options.organizationId, deps);
addCommonArgs(args, env, config); addCommonArgs(args, env, config);
if (usesSourceArg) { if (usesSourceArg) {

View file

@ -1,7 +1,7 @@
import path from "node:path"; import path from "node:path";
import fc from "fast-check"; import fc from "fast-check";
import { describe, expect, test } from "vitest"; import { describe, expect, test } from "vitest";
import { isPathWithin, normalizeAbsolutePath } from "../path"; import { hasPathListSeparator, isPathWithin, normalizeAbsolutePath } from "../path";
const safePathSegmentArb = fc const safePathSegmentArb = fc
.array(fc.constantFrom("a", "b", "c", "x", "y", "z", "0", "1", "2", "-", "_", ".", " "), { .array(fc.constantFrom("a", "b", "c", "x", "y", "z", "0", "1", "2", "-", "_", ".", " "), {
@ -80,12 +80,32 @@ describe("isPathWithin", () => {
test("matches descendants created under the same normalized base", () => { test("matches descendants created under the same normalized base", () => {
fc.assert( fc.assert(
fc.property(fc.string({ maxLength: 80 }), fc.array(safePathSegmentArb, { maxLength: 5 }), (base, segments) => { fc.property(
fc.string({ maxLength: 80 }),
fc.array(safePathSegmentArb, { maxLength: 5 }),
(base, segments) => {
const normalizedBase = normalizeAbsolutePath(base); const normalizedBase = normalizeAbsolutePath(base);
const descendant = path.posix.join(normalizedBase, ...segments); const descendant = path.posix.join(normalizedBase, ...segments);
expect(isPathWithin(base, descendant)).toBe(true); expect(isPathWithin(base, descendant)).toBe(true);
}), },
),
); );
}); });
}); });
describe("path list character support", () => {
test("allows line breaks in raw path lists", () => {
expect(hasPathListSeparator("Photos", "raw")).toBe(false);
expect(hasPathListSeparator("Photos\nSecrets", "raw")).toBe(false);
expect(hasPathListSeparator("Photos\rSecrets", "raw")).toBe(false);
expect(hasPathListSeparator("Photos\0Secrets", "raw")).toBe(true);
});
test("rejects line breaks in text path lists", () => {
expect(hasPathListSeparator("Photos", "text")).toBe(false);
expect(hasPathListSeparator("Photos\0Secrets", "text")).toBe(true);
expect(hasPathListSeparator("Photos\nSecrets", "text")).toBe(true);
expect(hasPathListSeparator("Photos\rSecrets", "text")).toBe(true);
});
});

View file

@ -1,4 +1,4 @@
export { safeJsonParse } from "./json.js"; export { safeJsonParse } from "./json.js";
export { toErrorDetails, toMessage } from "./errors.js"; export { toErrorDetails, toMessage } from "./errors.js";
export { isPathWithin, normalizeAbsolutePath } from "./path.js"; export { hasPathListSeparator, isPathWithin, normalizeAbsolutePath } from "./path.js";
export { findCommonAncestor } from "./common-ancestor.js"; export { findCommonAncestor } from "./common-ancestor.js";

View file

@ -52,6 +52,11 @@ export const isPathWithin = (base: string, target: string): boolean => {
const normalizedTarget = normalizeAbsolutePath(target); const normalizedTarget = normalizeAbsolutePath(target);
return ( return (
normalizedBase === "/" || normalizedTarget === normalizedBase || normalizedTarget.startsWith(`${normalizedBase}/`) normalizedBase === "/" ||
normalizedTarget === normalizedBase ||
normalizedTarget.startsWith(`${normalizedBase}/`)
); );
}; };
export const hasPathListSeparator = (value: string, format: "raw" | "text") =>
value.includes("\u0000") || (format === "text" && (value.includes("\n") || value.includes("\r")));