mirror of
https://github.com/openclaw/openclaw.git
synced 2026-06-06 05:51:15 +08:00
fix(memory): block extra path symlink traversal
## Considered and deferred - packages/memory-host-sdk/src/host/read-file.ts:77 [BOT-SCOPE]: Fully race-proof parent traversal would need a lower-level pinned/openat-style primitive; this diff fixes static symlink traversal and rejects symlink components before read.
This commit is contained in:
committed by
Peter Steinberger
parent
d21c47d711
commit
c6748a8eeb
@@ -1,7 +1,8 @@
|
||||
import { configureFsSafePython } from "@openclaw/fs-safe/config";
|
||||
export { root } from "@openclaw/fs-safe/root";
|
||||
export { isPathInside } from "@openclaw/fs-safe/path";
|
||||
export { isPathInside, isPathInsideWithRealpath } from "@openclaw/fs-safe/path";
|
||||
export {
|
||||
assertNoSymlinkParents,
|
||||
readRegularFile,
|
||||
statRegularFile,
|
||||
type RegularFileStatResult,
|
||||
|
||||
99
packages/memory-host-sdk/src/host/read-file.test.ts
Normal file
99
packages/memory-host-sdk/src/host/read-file.test.ts
Normal file
@@ -0,0 +1,99 @@
|
||||
import fs from "node:fs/promises";
|
||||
import os from "node:os";
|
||||
import path from "node:path";
|
||||
import { describe, expect, it } from "vitest";
|
||||
import { readMemoryFile } from "./read-file.js";
|
||||
|
||||
async function createDirectorySymlink(target: string, linkPath: string): Promise<boolean> {
|
||||
try {
|
||||
await fs.symlink(target, linkPath, "dir");
|
||||
return true;
|
||||
} catch (err) {
|
||||
const code = (err as NodeJS.ErrnoException).code;
|
||||
if (code === "EPERM" || code === "EACCES") {
|
||||
return false;
|
||||
}
|
||||
throw err;
|
||||
}
|
||||
}
|
||||
|
||||
describe("readMemoryFile", () => {
|
||||
it("returns empty text for missing files under extra path directories", async () => {
|
||||
const tmpRoot = await fs.mkdtemp(path.join(os.tmpdir(), "memory-read-file-"));
|
||||
try {
|
||||
const workspaceDir = path.join(tmpRoot, "workspace");
|
||||
const extraDir = path.join(tmpRoot, "extra");
|
||||
const missingPath = path.join(extraDir, "missing.md");
|
||||
await fs.mkdir(workspaceDir, { recursive: true });
|
||||
await fs.mkdir(extraDir, { recursive: true });
|
||||
|
||||
const result = await readMemoryFile({
|
||||
workspaceDir,
|
||||
extraPaths: [extraDir],
|
||||
relPath: missingPath,
|
||||
});
|
||||
|
||||
expect(result).toEqual({
|
||||
text: "",
|
||||
path: path.relative(workspaceDir, missingPath).replace(/\\/g, "/"),
|
||||
});
|
||||
} finally {
|
||||
await fs.rm(tmpRoot, { recursive: true, force: true });
|
||||
}
|
||||
});
|
||||
|
||||
it("rejects extra path reads through symlinked directory components", async () => {
|
||||
const tmpRoot = await fs.mkdtemp(path.join(os.tmpdir(), "memory-read-file-"));
|
||||
try {
|
||||
const workspaceDir = path.join(tmpRoot, "workspace");
|
||||
const extraDir = path.join(tmpRoot, "extra");
|
||||
const outsideDir = path.join(tmpRoot, "outside");
|
||||
await fs.mkdir(workspaceDir, { recursive: true });
|
||||
await fs.mkdir(extraDir, { recursive: true });
|
||||
await fs.mkdir(outsideDir, { recursive: true });
|
||||
await fs.writeFile(path.join(extraDir, "inside.md"), "inside", "utf-8");
|
||||
await fs.writeFile(path.join(outsideDir, "private.md"), "private", "utf-8");
|
||||
|
||||
const inside = await readMemoryFile({
|
||||
workspaceDir,
|
||||
extraPaths: [extraDir],
|
||||
relPath: path.join(extraDir, "inside.md"),
|
||||
});
|
||||
expect(inside.text).toBe("inside");
|
||||
|
||||
const insideLinkPath = path.join(extraDir, "inside-link");
|
||||
if (!(await createDirectorySymlink(extraDir, insideLinkPath))) {
|
||||
return;
|
||||
}
|
||||
await expect(
|
||||
readMemoryFile({
|
||||
workspaceDir,
|
||||
extraPaths: [extraDir],
|
||||
relPath: path.join(insideLinkPath, "inside.md"),
|
||||
}),
|
||||
).rejects.toThrow("path required");
|
||||
|
||||
const outsideLinkPath = path.join(extraDir, "link");
|
||||
if (!(await createDirectorySymlink(outsideDir, outsideLinkPath))) {
|
||||
return;
|
||||
}
|
||||
|
||||
await expect(
|
||||
readMemoryFile({
|
||||
workspaceDir,
|
||||
extraPaths: [extraDir],
|
||||
relPath: path.join(outsideLinkPath, "private.md"),
|
||||
}),
|
||||
).rejects.toThrow("path required");
|
||||
await expect(
|
||||
readMemoryFile({
|
||||
workspaceDir,
|
||||
extraPaths: [extraDir],
|
||||
relPath: path.join(outsideLinkPath, "missing.md"),
|
||||
}),
|
||||
).rejects.toThrow("path required");
|
||||
} finally {
|
||||
await fs.rm(tmpRoot, { recursive: true, force: true });
|
||||
}
|
||||
});
|
||||
});
|
||||
@@ -7,8 +7,10 @@ import {
|
||||
type OpenClawConfig,
|
||||
} from "./config-utils.js";
|
||||
import {
|
||||
assertNoSymlinkParents,
|
||||
isFileMissingError,
|
||||
isPathInside,
|
||||
isPathInsideWithRealpath,
|
||||
readRegularFile,
|
||||
root,
|
||||
statRegularFile,
|
||||
@@ -20,6 +22,29 @@ import {
|
||||
type MemoryReadResult,
|
||||
} from "./read-file-shared.js";
|
||||
|
||||
async function isAllowedAdditionalDirectoryPath(
|
||||
additionalPath: string,
|
||||
absPath: string,
|
||||
): Promise<boolean> {
|
||||
if (!isPathInside(additionalPath, absPath)) {
|
||||
return false;
|
||||
}
|
||||
try {
|
||||
await assertNoSymlinkParents({ rootDir: additionalPath, targetPath: absPath });
|
||||
} catch {
|
||||
return false;
|
||||
}
|
||||
if (!isPathInsideWithRealpath(additionalPath, absPath)) {
|
||||
try {
|
||||
await fs.lstat(absPath);
|
||||
} catch (err) {
|
||||
return isFileMissingError(err);
|
||||
}
|
||||
return false;
|
||||
}
|
||||
return true;
|
||||
}
|
||||
|
||||
export async function readMemoryFile(params: {
|
||||
workspaceDir: string;
|
||||
extraPaths?: string[];
|
||||
@@ -49,7 +74,7 @@ export async function readMemoryFile(params: {
|
||||
continue;
|
||||
}
|
||||
if (stat.isDirectory()) {
|
||||
if (isPathInside(additionalPath, absPath)) {
|
||||
if (await isAllowedAdditionalDirectoryPath(additionalPath, absPath)) {
|
||||
const candidateStat = await fs.lstat(absPath).catch(() => null);
|
||||
if (candidateStat?.isSymbolicLink()) {
|
||||
continue;
|
||||
|
||||
Reference in New Issue
Block a user