fix: harden absolute directory segment validation

This commit is contained in:
Peter Steinberger 2026-05-07 10:52:57 +01:00
parent fb06663ac6
commit a431bfc3b8
No known key found for this signature in database
2 changed files with 182 additions and 29 deletions

View File

@ -34,6 +34,13 @@ type DirectoryGuardCheckResult = { ok: true } | EnsureAbsoluteDirectoryFailure;
type DirectoryGuardCreateResult =
| { ok: true; guard: AsyncDirectoryGuard }
| EnsureAbsoluteDirectoryFailure;
type DirectoryPrefixResult =
| {
ok: true;
ancestorPath: string;
missingSegments: string[];
}
| EnsureAbsoluteDirectoryFailure;
function ensureDirectoryFailure(
code: FsSafeErrorCode,
@ -49,13 +56,14 @@ function ensureDirectoryFailure(
async function assertGuardResult(
guard: AsyncDirectoryGuard,
scopeLabel: string,
): Promise<DirectoryGuardCheckResult> {
try {
await assertAsyncDirectoryGuard(guard);
return { ok: true };
} catch (err) {
if (err instanceof FsSafeError) {
return { ok: false, code: err.code, error: err };
return await directoryGuardFailure(err, guard.dir, scopeLabel);
}
throw err;
}
@ -63,17 +71,141 @@ async function assertGuardResult(
async function createDirectoryGuardResult(
dir: string,
scopeLabel: string,
): Promise<DirectoryGuardCreateResult> {
try {
return { ok: true, guard: await createAsyncDirectoryGuard(dir) };
} catch (err) {
if (err instanceof FsSafeError) {
return { ok: false, code: err.code, error: err };
return await directoryGuardFailure(err, dir, scopeLabel);
}
throw err;
}
}
function classifyDirectoryLookupError(
err: unknown,
scopeLabel: string,
): EnsureAbsoluteDirectoryFailure | null {
const code = (err as NodeJS.ErrnoException).code;
if (code === "ENOENT") {
return ensureDirectoryFailure(
"not-found",
`directory path must have a real existing ancestor within ${scopeLabel}`,
err,
);
}
if (code === "ENOTDIR") {
return ensureDirectoryFailure(
"not-file",
`path must be a real directory within ${scopeLabel}`,
err,
);
}
return null;
}
function classifyExistingDirectorySegment(
stat: Stats,
scopeLabel: string,
): EnsureAbsoluteDirectoryFailure | null {
if (stat.isSymbolicLink()) {
return ensureDirectoryFailure(
"symlink",
`directory path traverses a symlink within ${scopeLabel}`,
);
}
if (!stat.isDirectory()) {
return ensureDirectoryFailure("not-file", `path must be a real directory within ${scopeLabel}`);
}
return null;
}
async function directoryGuardFailure(
err: FsSafeError,
dir: string,
scopeLabel: string,
): Promise<EnsureAbsoluteDirectoryFailure> {
if (err.code !== "not-file") {
return { ok: false, code: err.code, error: err };
}
try {
const stat = await fs.lstat(dir);
const failure = classifyExistingDirectorySegment(stat, scopeLabel);
if (failure) {
return failure;
}
} catch (lookupErr) {
const failure = classifyDirectoryLookupError(lookupErr, scopeLabel);
if (failure) {
return failure;
}
throw lookupErr;
}
return { ok: false, code: err.code, error: err };
}
async function resolveTrustedDirectoryPrefix(
targetPath: string,
scopeLabel: string,
): Promise<DirectoryPrefixResult> {
const root = path.parse(targetPath).root;
let current = root;
let currentStat: Stats;
try {
currentStat = await fs.lstat(current);
} catch (err) {
const failure = classifyDirectoryLookupError(err, scopeLabel);
if (failure) {
return failure;
}
throw err;
}
const rootFailure = classifyExistingDirectorySegment(currentStat, scopeLabel);
if (rootFailure) {
return rootFailure;
}
// Walk forward with lstat. Looking backward for the "nearest existing
// ancestor" can cross an existing suffix through a symlinked parent before
// this helper gets a chance to reject that parent.
const segments = path.relative(root, targetPath).split(path.sep).filter(Boolean);
for (let index = 0; index < segments.length; index += 1) {
const segment = segments[index];
if (!segment) {
continue;
}
const next = path.join(current, segment);
try {
const nextStat = await fs.lstat(next);
const segmentFailure = classifyExistingDirectorySegment(nextStat, scopeLabel);
if (segmentFailure) {
return segmentFailure;
}
current = next;
currentStat = nextStat;
} catch (err) {
const code = (err as NodeJS.ErrnoException).code;
if (code === "ENOENT") {
return {
ok: true,
ancestorPath: current,
missingSegments: segments.slice(index),
};
}
const failure = classifyDirectoryLookupError(err, scopeLabel);
if (failure) {
return failure;
}
throw err;
}
}
return { ok: true, ancestorPath: current, missingSegments: [] };
}
export function assertAbsolutePathInput(filePath: string): string {
if (!filePath) {
throw new FsSafeError("invalid-path", "path is required");
@ -136,33 +268,21 @@ export async function ensureAbsoluteDirectory(
throw err;
}
const ancestor = await findExistingAncestorWithStat(targetPath);
if (!ancestor) {
return ensureDirectoryFailure(
"not-found",
`directory path must have a real existing ancestor within ${scopeLabel}`,
);
const prefix = await resolveTrustedDirectoryPrefix(targetPath, scopeLabel);
if (!prefix.ok) {
return prefix;
}
if (ancestor.stat.isSymbolicLink()) {
return ensureDirectoryFailure("symlink", `directory path traverses a symlink within ${scopeLabel}`);
let current = prefix.ancestorPath;
const initialGuard = await createDirectoryGuardResult(prefix.ancestorPath, scopeLabel);
if (!initialGuard.ok) {
return initialGuard;
}
if (!ancestor.stat.isDirectory()) {
return ensureDirectoryFailure("not-file", `path must be a real directory within ${scopeLabel}`);
}
const ancestorDir = ancestor.path;
const relativeDir = path.relative(ancestorDir, targetPath);
let current = ancestorDir;
let currentGuard: AsyncDirectoryGuard = {
dir: ancestorDir,
realPath: await fs.realpath(ancestorDir),
stat: ancestor.stat,
};
for (const segment of relativeDir.split(path.sep).filter(Boolean)) {
let currentGuard: AsyncDirectoryGuard = initialGuard.guard;
for (const segment of prefix.missingSegments) {
current = path.join(current, segment);
while (true) {
const guardResult = await assertGuardResult(currentGuard);
const guardResult = await assertGuardResult(currentGuard, scopeLabel);
if (!guardResult.ok) {
return guardResult;
}
@ -185,7 +305,7 @@ export async function ensureAbsoluteDirectory(
if ((err as NodeJS.ErrnoException).code !== "ENOENT") {
throw err;
}
const parentStillValid = await assertGuardResult(currentGuard);
const parentStillValid = await assertGuardResult(currentGuard, scopeLabel);
if (!parentStillValid.ok) {
return parentStillValid;
}
@ -199,18 +319,18 @@ export async function ensureAbsoluteDirectory(
}
}
}
const nextGuard = await createDirectoryGuardResult(current);
const nextGuard = await createDirectoryGuardResult(current, scopeLabel);
if (!nextGuard.ok) {
return nextGuard;
}
const previousGuardStillValid = await assertGuardResult(currentGuard);
const previousGuardStillValid = await assertGuardResult(currentGuard, scopeLabel);
if (!previousGuardStillValid.ok) {
return previousGuardStillValid;
}
currentGuard = nextGuard.guard;
}
const finalGuardResult = await assertGuardResult(currentGuard);
const finalGuardResult = await assertGuardResult(currentGuard, scopeLabel);
if (!finalGuardResult.ok) {
return finalGuardResult;
}

View File

@ -63,6 +63,39 @@ describe("ensureAbsoluteDirectory", () => {
},
);
it.runIf(process.platform !== "win32")(
"rejects symlinked parents even when the requested suffix already exists",
async () => {
const root = await fs.realpath(await tempRoot("fs-safe-absolute-dir-link-existing-"));
const outside = await fs.realpath(
await tempRoot("fs-safe-absolute-dir-link-existing-outside-"),
);
const existing = path.join(outside, "existing");
const linkDir = path.join(root, "link");
await fs.mkdir(existing);
await fs.symlink(outside, linkDir);
await expect(
ensureAbsoluteDirectory(path.join(linkDir, "existing", "new"), {
scopeLabel: "output directory",
}),
).resolves.toMatchObject({ ok: false, code: "symlink" });
await expect(fs.stat(path.join(existing, "new"))).rejects.toMatchObject({ code: "ENOENT" });
},
);
it("returns a policy failure when an intermediate component is a file", async () => {
const root = await fs.realpath(await tempRoot("fs-safe-absolute-dir-file-component-"));
const filePath = path.join(root, "file");
await fs.writeFile(filePath, "file", "utf8");
await expect(
ensureAbsoluteDirectory(path.join(filePath, "child"), {
scopeLabel: "output directory",
}),
).resolves.toMatchObject({ ok: false, code: "not-file" });
});
it.runIf(process.platform !== "win32")(
"rejects absolute directory creation when an existing parent is swapped before mkdir",
async () => {
@ -123,7 +156,7 @@ describe("ensureAbsoluteDirectory", () => {
try {
await expect(
ensureAbsoluteDirectory(targetDir, { scopeLabel: "output directory" }),
).resolves.toMatchObject({ ok: false, code: "not-file" });
).resolves.toMatchObject({ ok: false, code: "symlink" });
} finally {
realpathSpy.mockRestore();
}