fix(backup-config): throw if include patterns have un-supported chars (#931)
This commit is contained in:
parent
d479bfaddc
commit
00d1dac515
7 changed files with 102 additions and 11 deletions
|
|
@ -130,6 +130,24 @@ 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";
|
||||||
|
|
||||||
|
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", () => {
|
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";
|
||||||
|
|
||||||
|
|
|
||||||
|
|
@ -1,9 +1,14 @@
|
||||||
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 { hasUnsupportedPathCharacter } from "@zerobyte/core/utils";
|
||||||
|
|
||||||
type BackupOptions = BackupRunPayload["options"];
|
type BackupOptions = BackupRunPayload["options"];
|
||||||
|
|
||||||
export const processPattern = (pattern: string, volumePath: string, relative = false) => {
|
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 isNegated = pattern.startsWith("!");
|
||||||
const p = isNegated ? pattern.slice(1) : pattern;
|
const p = isNegated ? pattern.slice(1) : pattern;
|
||||||
|
|
||||||
|
|
|
||||||
|
|
@ -158,6 +158,41 @@ describe("backup command", () => {
|
||||||
expect(patternIncludeContent).toBe("/mnt/data/**/*.zip");
|
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 () => {
|
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 +267,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 +364,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(
|
||||||
|
|
|
||||||
|
|
@ -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 { hasUnsupportedPathCharacter, toMessage } from "../../utils";
|
||||||
|
|
||||||
class ResticBackupCommandError extends Data.TaggedError("ResticBackupCommandError")<{
|
class ResticBackupCommandError extends Data.TaggedError("ResticBackupCommandError")<{
|
||||||
cause: unknown;
|
cause: unknown;
|
||||||
message: string;
|
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 = (
|
export const backup = (
|
||||||
config: RepositoryConfig,
|
config: RepositoryConfig,
|
||||||
source: string,
|
source: string,
|
||||||
|
|
@ -66,6 +74,8 @@ export const backup = (
|
||||||
(!options.includePatterns || options.includePatterns.length === 0);
|
(!options.includePatterns || options.includePatterns.length === 0);
|
||||||
|
|
||||||
if (options.includePatterns?.length) {
|
if (options.includePatterns?.length) {
|
||||||
|
validatePatterns(options.includePatterns, "includePatterns");
|
||||||
|
|
||||||
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 +85,8 @@ export const backup = (
|
||||||
}
|
}
|
||||||
|
|
||||||
if (options.includePaths?.length) {
|
if (options.includePaths?.length) {
|
||||||
|
validatePatterns(options.includePaths, "includePaths");
|
||||||
|
|
||||||
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");
|
||||||
|
|
||||||
|
|
|
||||||
|
|
@ -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 { hasUnsupportedPathCharacter, 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,25 @@ 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(
|
||||||
const normalizedBase = normalizeAbsolutePath(base);
|
fc.string({ maxLength: 80 }),
|
||||||
const descendant = path.posix.join(normalizedBase, ...segments);
|
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);
|
||||||
|
});
|
||||||
|
});
|
||||||
|
|
|
||||||
|
|
@ -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 { hasUnsupportedPathCharacter, isPathWithin, normalizeAbsolutePath } from "./path.js";
|
||||||
export { findCommonAncestor } from "./common-ancestor.js";
|
export { findCommonAncestor } from "./common-ancestor.js";
|
||||||
|
|
|
||||||
|
|
@ -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 hasUnsupportedPathCharacter = (value: string) =>
|
||||||
|
value.includes("\u0000") || value.includes("\n") || value.includes("\r");
|
||||||
|
|
|
||||||
Loading…
Reference in a new issue