From 62cdf5dcca8340e7621f488de648237cf7fe2953 Mon Sep 17 00:00:00 2001 From: Nicolas Meienberger Date: Mon, 1 Jun 2026 19:15:28 +0200 Subject: [PATCH] fix(backup-config): throw if include patterns have un-supported characters --- .../helpers/__tests__/backup.helpers.test.ts | 29 +++++++--- .../src/commands/helpers/backup.helpers.ts | 22 ++++++-- .../restic/commands/__tests__/backup.test.ts | 55 +++++++++++++++---- packages/core/src/restic/commands/backup.ts | 12 ++-- .../core/src/utils/__tests__/path.test.ts | 21 ++++--- packages/core/src/utils/index.ts | 2 +- packages/core/src/utils/path.ts | 4 +- 7 files changed, 104 insertions(+), 41 deletions(-) diff --git a/apps/agent/src/commands/helpers/__tests__/backup.helpers.test.ts b/apps/agent/src/commands/helpers/__tests__/backup.helpers.test.ts index c657fc4c..7c6cce68 100644 --- a/apps/agent/src/commands/helpers/__tests__/backup.helpers.test.ts +++ b/apps/agent/src/commands/helpers/__tests__/backup.helpers.test.ts @@ -133,19 +133,32 @@ describe("backup path options", () => { test("rejects unsupported characters in selected include paths", () => { const volumePath = "/var/lib/zerobyte/volumes/vol123/_data"; - for (const includePath of ["/Photos\0/etc/passwd", "/Photos\n/etc/passwd", "/Photos\r/etc/passwd"]) { - expect(() => createOptions(createPathOptions({ includePaths: [includePath] }), volumePath)).toThrow( - "Include pattern contains an unsupported path character", - ); - } + 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"; - expect(() => - createOptions(createPathOptions({ includePatterns: ["/Photos\n/etc/passwd"] }), volumePath), - ).toThrow("Include pattern contains an unsupported path character"); + 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", () => { diff --git a/apps/agent/src/commands/helpers/backup.helpers.ts b/apps/agent/src/commands/helpers/backup.helpers.ts index 573290ec..9faf35ac 100644 --- a/apps/agent/src/commands/helpers/backup.helpers.ts +++ b/apps/agent/src/commands/helpers/backup.helpers.ts @@ -1,14 +1,16 @@ import path from "node:path"; import type { BackupRunPayload } from "@zerobyte/contracts/agent-protocol"; -import { hasUnsupportedPathCharacter } from "@zerobyte/core/utils"; +import { hasPathListSeparator } from "@zerobyte/core/utils"; type BackupOptions = BackupRunPayload["options"]; -export const processPattern = (pattern: string, volumePath: string, relative = false) => { - if (relative && hasUnsupportedPathCharacter(pattern)) { - throw new Error(`Include pattern contains an unsupported path character: ${pattern}`); +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) => { const isNegated = pattern.startsWith("!"); const p = isNegated ? pattern.slice(1) : pattern; @@ -46,8 +48,16 @@ export const createBackupOptions = ( signal, exclude: params.options.excludePatterns?.map((p) => processPattern(p, volumePath)) ?? undefined, excludeIfPresent: params.options.excludeIfPresent ?? undefined, - includePaths: params.options.includePaths?.map((p) => processPattern(p, volumePath, true)) ?? undefined, - includePatterns: params.options.includePatterns?.map((p) => processPattern(p, volumePath, true)) ?? undefined, + includePaths: + 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 ?? [], compressionMode: params.options.compressionMode, }); diff --git a/packages/core/src/restic/commands/__tests__/backup.test.ts b/packages/core/src/restic/commands/__tests__/backup.test.ts index bda19790..7c7591e2 100644 --- a/packages/core/src/restic/commands/__tests__/backup.test.ts +++ b/packages/core/src/restic/commands/__tests__/backup.test.ts @@ -158,6 +158,33 @@ describe("backup command", () => { 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(); @@ -179,18 +206,24 @@ describe("backup command", () => { test("rejects unsupported characters before writing include pattern files", async () => { const { getArgs } = setup(); - const error = await runBackupError( - config, - "/mnt/data", - { - organizationId: "org-1", - includePatterns: ["/mnt/data/safe\n/etc/passwd"], - }, - mockDeps, - ); + 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([]); + expect(String(error.message)).toContain("includePatterns contains an unsupported path character"); + expect(getArgs()).toEqual([]); + } }); test("always includes DEFAULT_EXCLUDES as --exclude args", async () => { diff --git a/packages/core/src/restic/commands/backup.ts b/packages/core/src/restic/commands/backup.ts index 9a2561e7..18b69441 100644 --- a/packages/core/src/restic/commands/backup.ts +++ b/packages/core/src/restic/commands/backup.ts @@ -13,16 +13,16 @@ import { validateCustomResticParams } from "../helpers/validate-custom-params"; import { createResticError, isResticError } from "../error"; import { logger, safeSpawn } from "../../node"; import type { ResticDeps } from "../types"; -import { hasUnsupportedPathCharacter, toMessage } from "../../utils"; +import { hasPathListSeparator, toMessage } from "../../utils"; class ResticBackupCommandError extends Data.TaggedError("ResticBackupCommandError")<{ cause: unknown; message: string; }> {} -const validatePatterns = (entries: string[], optionName: string) => { +const validateEntries = (entries: string[], optionName: string, format: "raw" | "text") => { for (const entry of entries) { - if (hasUnsupportedPathCharacter(entry)) { + if (hasPathListSeparator(entry, format)) { throw new Error(`${optionName} contains an unsupported path character: ${entry}`); } } @@ -49,7 +49,6 @@ export const backup = ( return Effect.tryPromise({ try: async () => { const repoUrl = buildRepoUrl(config); - const env = await buildEnv(config, options.organizationId, deps); const args: string[] = ["--repo", repoUrl, "backup", "--compression", options.compressionMode ?? "auto"]; @@ -74,7 +73,7 @@ export const backup = ( (!options.includePatterns || options.includePatterns.length === 0); if (options.includePatterns?.length) { - validatePatterns(options.includePatterns, "includePatterns"); + validateEntries(options.includePatterns, "includePatterns", "text"); const tmp = await fs.mkdtemp(path.join(os.tmpdir(), "zerobyte-restic-include-")); includeFile = path.join(tmp, "include.txt"); @@ -85,7 +84,7 @@ export const backup = ( } if (options.includePaths?.length) { - validatePatterns(options.includePaths, "includePaths"); + validateEntries(options.includePaths, "includePaths", "raw"); const tmp = await fs.mkdtemp(path.join(os.tmpdir(), "zerobyte-restic-include-raw-")); rawIncludeFile = path.join(tmp, "include.raw"); @@ -126,6 +125,7 @@ export const backup = ( } } + const env = await buildEnv(config, options.organizationId, deps); addCommonArgs(args, env, config); if (usesSourceArg) { diff --git a/packages/core/src/utils/__tests__/path.test.ts b/packages/core/src/utils/__tests__/path.test.ts index b46827d3..ae1c1adb 100644 --- a/packages/core/src/utils/__tests__/path.test.ts +++ b/packages/core/src/utils/__tests__/path.test.ts @@ -1,7 +1,7 @@ import path from "node:path"; import fc from "fast-check"; import { describe, expect, test } from "vitest"; -import { hasUnsupportedPathCharacter, isPathWithin, normalizeAbsolutePath } from "../path"; +import { hasPathListSeparator, isPathWithin, normalizeAbsolutePath } from "../path"; const safePathSegmentArb = fc .array(fc.constantFrom("a", "b", "c", "x", "y", "z", "0", "1", "2", "-", "_", ".", " "), { @@ -94,11 +94,18 @@ describe("isPathWithin", () => { }); }); -describe("hasUnsupportedPathCharacter", () => { - test("detects path characters that are not supported by include files", () => { - expect(hasUnsupportedPathCharacter("Photos")).toBe(false); - expect(hasUnsupportedPathCharacter("Photos\0Secrets")).toBe(true); - expect(hasUnsupportedPathCharacter("Photos\nSecrets")).toBe(true); - expect(hasUnsupportedPathCharacter("Photos\rSecrets")).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); }); }); diff --git a/packages/core/src/utils/index.ts b/packages/core/src/utils/index.ts index 47baa93d..e43c4636 100644 --- a/packages/core/src/utils/index.ts +++ b/packages/core/src/utils/index.ts @@ -1,4 +1,4 @@ export { safeJsonParse } from "./json.js"; export { toErrorDetails, toMessage } from "./errors.js"; -export { hasUnsupportedPathCharacter, isPathWithin, normalizeAbsolutePath } from "./path.js"; +export { hasPathListSeparator, isPathWithin, normalizeAbsolutePath } from "./path.js"; export { findCommonAncestor } from "./common-ancestor.js"; diff --git a/packages/core/src/utils/path.ts b/packages/core/src/utils/path.ts index 04cde088..9428d4e5 100644 --- a/packages/core/src/utils/path.ts +++ b/packages/core/src/utils/path.ts @@ -58,5 +58,5 @@ export const isPathWithin = (base: string, target: string): boolean => { ); }; -export const hasUnsupportedPathCharacter = (value: string) => - value.includes("\u0000") || value.includes("\n") || value.includes("\r"); +export const hasPathListSeparator = (value: string, format: "raw" | "text") => + value.includes("\u0000") || (format === "text" && (value.includes("\n") || value.includes("\r")));