Skip to content

Commit 0ad61da

Browse files
committed
fix(storage): harden backup root validation
1 parent 94c18b8 commit 0ad61da

2 files changed

Lines changed: 53 additions & 5 deletions

File tree

lib/storage.ts

Lines changed: 15 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,12 @@
11
import { AsyncLocalStorage } from "node:async_hooks";
22
import { createHash } from "node:crypto";
3-
import { existsSync, promises as fs, realpathSync, type Dirent } from "node:fs";
3+
import {
4+
existsSync,
5+
lstatSync,
6+
promises as fs,
7+
realpathSync,
8+
type Dirent,
9+
} from "node:fs";
410
import { basename, dirname, isAbsolute, join, relative } from "node:path";
511
import { ACCOUNT_LIMITS } from "./constants.js";
612
import { createLogger } from "./logger.js";
@@ -2026,9 +2032,14 @@ export function assertNamedBackupRestorePath(
20262032
backupRoot: string,
20272033
): string {
20282034
const resolvedPath = resolvePath(path);
2029-
const resolvedBackupRoot = resolvePathForNamedBackupContainment(backupRoot);
2035+
const resolvedBackupRoot = resolvePath(backupRoot);
2036+
if (existsSync(resolvedBackupRoot) && lstatSync(resolvedBackupRoot).isSymbolicLink()) {
2037+
throw new Error("Backup path escapes backup directory");
2038+
}
2039+
const canonicalBackupRoot =
2040+
resolvePathForNamedBackupContainment(resolvedBackupRoot);
20302041
const containedPath = resolvePathForNamedBackupContainment(resolvedPath);
2031-
const relativePath = relative(resolvedBackupRoot, containedPath);
2042+
const relativePath = relative(canonicalBackupRoot, containedPath);
20322043
const firstSegment = relativePath.split(/[\\/]/)[0];
20332044
if (
20342045
relativePath.length === 0 ||
@@ -2037,7 +2048,7 @@ export function assertNamedBackupRestorePath(
20372048
) {
20382049
throw new Error("Backup path escapes backup directory");
20392050
}
2040-
return resolvedPath;
2051+
return containedPath;
20412052
}
20422053

20432054
export function isNamedBackupContainmentError(error: unknown): boolean {

test/storage.test.ts

Lines changed: 38 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1896,6 +1896,41 @@ describe("storage", () => {
18961896
).toThrow(/escapes backup directory/i);
18971897
});
18981898

1899+
it("rejects symlinked backup roots during restore path validation", async () => {
1900+
const canonicalBackupRoot = join(testWorkDir, "canonical-backups");
1901+
const linkedBackupRoot = join(testWorkDir, "linked-backups");
1902+
const backupPath = join(canonicalBackupRoot, "linked-root.json");
1903+
await fs.mkdir(canonicalBackupRoot, { recursive: true });
1904+
await fs.writeFile(
1905+
backupPath,
1906+
JSON.stringify({
1907+
version: 3,
1908+
activeIndex: 0,
1909+
accounts: [
1910+
{
1911+
accountId: "linked-root",
1912+
refreshToken: "ref-linked-root",
1913+
addedAt: 1,
1914+
lastUsed: 1,
1915+
},
1916+
],
1917+
}),
1918+
"utf-8",
1919+
);
1920+
await fs.symlink(
1921+
resolve(canonicalBackupRoot),
1922+
linkedBackupRoot,
1923+
process.platform === "win32" ? "junction" : "dir",
1924+
);
1925+
1926+
expect(() =>
1927+
assertNamedBackupRestorePath(
1928+
join(linkedBackupRoot, "linked-root.json"),
1929+
linkedBackupRoot,
1930+
),
1931+
).toThrow(/escapes backup directory/i);
1932+
});
1933+
18991934
it("rejects named backup listings whose resolved paths escape the backups directory", async () => {
19001935
const backupRoot = join(dirname(testStoragePath), "backups");
19011936
const originalReaddir = fs.readdir.bind(fs);
@@ -2252,7 +2287,9 @@ describe("storage", () => {
22522287
expect(listedBackups).toEqual(
22532288
expect.arrayContaining([
22542289
expect.objectContaining({
2255-
name: "chunk-boundary-08",
2290+
name: `chunk-boundary-${String(
2291+
NAMED_BACKUP_LIST_CONCURRENCY,
2292+
).padStart(2, "0")}`,
22562293
valid: true,
22572294
}),
22582295
]),

0 commit comments

Comments
 (0)