From 7ea002a8ddf78646bd5171ffbdaa87a54424b664 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 | 18 ++++++++ .../src/commands/helpers/backup.helpers.ts | 5 +++ .../restic/commands/__tests__/backup.test.ts | 42 ++++++++++++++++++- packages/core/src/restic/commands/backup.ts | 14 ++++++- .../core/src/utils/__tests__/path.test.ts | 25 ++++++++--- packages/core/src/utils/index.ts | 2 +- packages/core/src/utils/path.ts | 7 +++- 7 files changed, 102 insertions(+), 11 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 ebd00118..c657fc4c 100644 --- a/apps/agent/src/commands/helpers/__tests__/backup.helpers.test.ts +++ b/apps/agent/src/commands/helpers/__tests__/backup.helpers.test.ts @@ -130,6 +130,24 @@ describe("backup path options", () => { ).toThrow("Include pattern escapes volume root"); }); + 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", + ); + } + }); + + 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"); + }); + test("anchors generated include patterns under the volume path", () => { const volumePath = "/var/lib/zerobyte/volumes/vol123/_data"; diff --git a/apps/agent/src/commands/helpers/backup.helpers.ts b/apps/agent/src/commands/helpers/backup.helpers.ts index 33fac06b..573290ec 100644 --- a/apps/agent/src/commands/helpers/backup.helpers.ts +++ b/apps/agent/src/commands/helpers/backup.helpers.ts @@ -1,9 +1,14 @@ import path from "node:path"; import type { BackupRunPayload } from "@zerobyte/contracts/agent-protocol"; +import { hasUnsupportedPathCharacter } 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 isNegated = pattern.startsWith("!"); const p = isNegated ? pattern.slice(1) : pattern; diff --git a/packages/core/src/restic/commands/__tests__/backup.test.ts b/packages/core/src/restic/commands/__tests__/backup.test.ts index 1121d291..bda19790 100644 --- a/packages/core/src/restic/commands/__tests__/backup.test.ts +++ b/packages/core/src/restic/commands/__tests__/backup.test.ts @@ -158,6 +158,41 @@ describe("backup command", () => { expect(patternIncludeContent).toBe("/mnt/data/**/*.zip"); }); + 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(); + + const error = await runBackupError( + config, + "/mnt/data", + { + organizationId: "org-1", + includePatterns: ["/mnt/data/safe\n/etc/passwd"], + }, + mockDeps, + ); + + expect(String(error.message)).toContain("includePatterns contains an unsupported path character"); + expect(getArgs()).toEqual([]); + }); + test("always includes DEFAULT_EXCLUDES as --exclude args", async () => { const { getOptionValues } = setup(); await runBackup(config, "/mnt/data", { organizationId: "org-1" }, mockDeps); @@ -232,7 +267,9 @@ describe("backup command", () => { const error = await runBackupError(config, "/mnt/data", { organizationId: "org-1" }, mockDeps); 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( "Permissions 0755 for '/tmp/zerobyte-ssh-key' are too open.\nThis private key will be ignored.", ); @@ -327,7 +364,8 @@ describe("backup command", () => { test("ignores valid JSON lines that do not match the progress schema", async () => { const progressUpdates: unknown[] = []; 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( diff --git a/packages/core/src/restic/commands/backup.ts b/packages/core/src/restic/commands/backup.ts index 6bb9bad2..9a2561e7 100644 --- a/packages/core/src/restic/commands/backup.ts +++ b/packages/core/src/restic/commands/backup.ts @@ -13,13 +13,21 @@ import { validateCustomResticParams } from "../helpers/validate-custom-params"; import { createResticError, isResticError } from "../error"; import { logger, safeSpawn } from "../../node"; import type { ResticDeps } from "../types"; -import { toMessage } from "../../utils"; +import { hasUnsupportedPathCharacter, toMessage } from "../../utils"; class ResticBackupCommandError extends Data.TaggedError("ResticBackupCommandError")<{ cause: unknown; message: string; }> {} +const validatePatterns = (entries: string[], optionName: string) => { + for (const entry of entries) { + if (hasUnsupportedPathCharacter(entry)) { + throw new Error(`${optionName} contains an unsupported path character: ${entry}`); + } + } +}; + export const backup = ( config: RepositoryConfig, source: string, @@ -66,6 +74,8 @@ export const backup = ( (!options.includePatterns || options.includePatterns.length === 0); if (options.includePatterns?.length) { + validatePatterns(options.includePatterns, "includePatterns"); + const tmp = await fs.mkdtemp(path.join(os.tmpdir(), "zerobyte-restic-include-")); includeFile = path.join(tmp, "include.txt"); @@ -75,6 +85,8 @@ export const backup = ( } if (options.includePaths?.length) { + validatePatterns(options.includePaths, "includePaths"); + const tmp = await fs.mkdtemp(path.join(os.tmpdir(), "zerobyte-restic-include-raw-")); rawIncludeFile = path.join(tmp, "include.raw"); diff --git a/packages/core/src/utils/__tests__/path.test.ts b/packages/core/src/utils/__tests__/path.test.ts index b8aa0dba..b46827d3 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 { isPathWithin, normalizeAbsolutePath } from "../path"; +import { hasUnsupportedPathCharacter, isPathWithin, normalizeAbsolutePath } from "../path"; const safePathSegmentArb = fc .array(fc.constantFrom("a", "b", "c", "x", "y", "z", "0", "1", "2", "-", "_", ".", " "), { @@ -80,12 +80,25 @@ describe("isPathWithin", () => { test("matches descendants created under the same normalized base", () => { fc.assert( - fc.property(fc.string({ maxLength: 80 }), fc.array(safePathSegmentArb, { maxLength: 5 }), (base, segments) => { - const normalizedBase = normalizeAbsolutePath(base); - const descendant = path.posix.join(normalizedBase, ...segments); + fc.property( + fc.string({ maxLength: 80 }), + fc.array(safePathSegmentArb, { maxLength: 5 }), + (base, segments) => { + const normalizedBase = normalizeAbsolutePath(base); + const descendant = path.posix.join(normalizedBase, ...segments); - expect(isPathWithin(base, descendant)).toBe(true); - }), + expect(isPathWithin(base, descendant)).toBe(true); + }, + ), ); }); }); + +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); + }); +}); diff --git a/packages/core/src/utils/index.ts b/packages/core/src/utils/index.ts index 8431333e..47baa93d 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 { isPathWithin, normalizeAbsolutePath } from "./path.js"; +export { hasUnsupportedPathCharacter, 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 53d76878..04cde088 100644 --- a/packages/core/src/utils/path.ts +++ b/packages/core/src/utils/path.ts @@ -52,6 +52,11 @@ export const isPathWithin = (base: string, target: string): boolean => { const normalizedTarget = normalizeAbsolutePath(target); return ( - normalizedBase === "/" || normalizedTarget === normalizedBase || normalizedTarget.startsWith(`${normalizedBase}/`) + normalizedBase === "/" || + normalizedTarget === normalizedBase || + normalizedTarget.startsWith(`${normalizedBase}/`) ); }; + +export const hasUnsupportedPathCharacter = (value: string) => + value.includes("\u0000") || value.includes("\n") || value.includes("\r");