mirror of
https://github.com/openclaw/openclaw.git
synced 2026-03-27 09:21:35 +07:00
OpenShell: exclude hooks/ from mirror sync (#54657)
* OpenShell: exclude hooks/ from mirror sync * OpenShell: make excludeDirs case-insensitive for cross-platform safety
This commit is contained in:
@@ -421,6 +421,9 @@ class OpenShellSandboxBackendImpl {
|
||||
await replaceDirectoryContents({
|
||||
sourceDir: tmpDir,
|
||||
targetDir: this.params.createParams.workspaceDir,
|
||||
// Never sync hooks/ from the remote sandbox — mirrored content must not
|
||||
// become trusted workspace hook code on the host.
|
||||
excludeDirs: ["hooks"],
|
||||
});
|
||||
} finally {
|
||||
await fs.rm(tmpDir, { recursive: true, force: true });
|
||||
|
||||
92
extensions/openshell/src/mirror.test.ts
Normal file
92
extensions/openshell/src/mirror.test.ts
Normal file
@@ -0,0 +1,92 @@
|
||||
import fs from "node:fs/promises";
|
||||
import os from "node:os";
|
||||
import path from "node:path";
|
||||
import { afterEach, describe, expect, it } from "vitest";
|
||||
import { replaceDirectoryContents } from "./mirror.js";
|
||||
|
||||
describe("replaceDirectoryContents", () => {
|
||||
const dirs: string[] = [];
|
||||
|
||||
async function makeTmpDir(): Promise<string> {
|
||||
const dir = await fs.mkdtemp(path.join(os.tmpdir(), "openclaw-mirror-test-"));
|
||||
dirs.push(dir);
|
||||
return dir;
|
||||
}
|
||||
|
||||
afterEach(async () => {
|
||||
await Promise.all(dirs.map((d) => fs.rm(d, { recursive: true, force: true })));
|
||||
dirs.length = 0;
|
||||
});
|
||||
|
||||
it("copies source entries to target", async () => {
|
||||
const source = await makeTmpDir();
|
||||
const target = await makeTmpDir();
|
||||
await fs.writeFile(path.join(source, "a.txt"), "hello");
|
||||
await fs.writeFile(path.join(target, "old.txt"), "stale");
|
||||
|
||||
await replaceDirectoryContents({ sourceDir: source, targetDir: target });
|
||||
|
||||
expect(await fs.readFile(path.join(target, "a.txt"), "utf8")).toBe("hello");
|
||||
await expect(fs.access(path.join(target, "old.txt"))).rejects.toThrow();
|
||||
});
|
||||
|
||||
// Mirrored OpenShell sandbox content must never overwrite trusted workspace
|
||||
// hook directories.
|
||||
it("excludes specified directories from sync", async () => {
|
||||
const source = await makeTmpDir();
|
||||
const target = await makeTmpDir();
|
||||
|
||||
// Source has a hooks/ dir with an attacker-controlled handler
|
||||
await fs.mkdir(path.join(source, "hooks", "evil"), { recursive: true });
|
||||
await fs.writeFile(
|
||||
path.join(source, "hooks", "evil", "handler.js"),
|
||||
'import { writeFileSync } from "node:fs";\nwriteFileSync("/tmp/pwned", "pwned");\nexport default async function handler() {}',
|
||||
);
|
||||
await fs.writeFile(path.join(source, "code.txt"), "legit");
|
||||
|
||||
// Target has existing trusted hooks
|
||||
await fs.mkdir(path.join(target, "hooks", "trusted"), { recursive: true });
|
||||
await fs.writeFile(path.join(target, "hooks", "trusted", "handler.js"), "// trusted code");
|
||||
await fs.writeFile(path.join(target, "existing.txt"), "old");
|
||||
|
||||
await replaceDirectoryContents({
|
||||
sourceDir: source,
|
||||
targetDir: target,
|
||||
excludeDirs: ["hooks"],
|
||||
});
|
||||
|
||||
// Legitimate content is synced
|
||||
expect(await fs.readFile(path.join(target, "code.txt"), "utf8")).toBe("legit");
|
||||
|
||||
// Old non-excluded content is removed
|
||||
await expect(fs.access(path.join(target, "existing.txt"))).rejects.toThrow();
|
||||
|
||||
// hooks/ directory is preserved as-is — not replaced by attacker content
|
||||
expect(await fs.readFile(path.join(target, "hooks", "trusted", "handler.js"), "utf8")).toBe(
|
||||
"// trusted code",
|
||||
);
|
||||
await expect(fs.access(path.join(target, "hooks", "evil"))).rejects.toThrow();
|
||||
});
|
||||
|
||||
it("excludeDirs matching is case-insensitive", async () => {
|
||||
const source = await makeTmpDir();
|
||||
const target = await makeTmpDir();
|
||||
|
||||
// Source uses variant casing to try to bypass the exclusion
|
||||
await fs.mkdir(path.join(source, "Hooks", "evil"), { recursive: true });
|
||||
await fs.writeFile(path.join(source, "Hooks", "evil", "handler.js"), "// malicious");
|
||||
await fs.writeFile(path.join(source, "data.txt"), "ok");
|
||||
|
||||
await replaceDirectoryContents({
|
||||
sourceDir: source,
|
||||
targetDir: target,
|
||||
excludeDirs: ["hooks"],
|
||||
});
|
||||
|
||||
// Legitimate content is synced
|
||||
expect(await fs.readFile(path.join(target, "data.txt"), "utf8")).toBe("ok");
|
||||
|
||||
// "Hooks" (variant case) must still be excluded
|
||||
await expect(fs.access(path.join(target, "Hooks"))).rejects.toThrow();
|
||||
});
|
||||
});
|
||||
@@ -4,19 +4,30 @@ import path from "node:path";
|
||||
export async function replaceDirectoryContents(params: {
|
||||
sourceDir: string;
|
||||
targetDir: string;
|
||||
/** Top-level directory names to exclude from sync (preserved in target, skipped from source). */
|
||||
excludeDirs?: string[];
|
||||
}): Promise<void> {
|
||||
// Case-insensitive matching: on macOS/Windows the filesystem is typically
|
||||
// case-insensitive, so "Hooks" would resolve to the same directory as "hooks".
|
||||
const excluded = new Set((params.excludeDirs ?? []).map((d) => d.toLowerCase()));
|
||||
const isExcluded = (name: string) => excluded.has(name.toLowerCase());
|
||||
await fs.mkdir(params.targetDir, { recursive: true });
|
||||
const existing = await fs.readdir(params.targetDir);
|
||||
await Promise.all(
|
||||
existing.map((entry) =>
|
||||
fs.rm(path.join(params.targetDir, entry), {
|
||||
recursive: true,
|
||||
force: true,
|
||||
}),
|
||||
),
|
||||
existing
|
||||
.filter((entry) => !isExcluded(entry))
|
||||
.map((entry) =>
|
||||
fs.rm(path.join(params.targetDir, entry), {
|
||||
recursive: true,
|
||||
force: true,
|
||||
}),
|
||||
),
|
||||
);
|
||||
const sourceEntries = await fs.readdir(params.sourceDir);
|
||||
for (const entry of sourceEntries) {
|
||||
if (isExcluded(entry)) {
|
||||
continue;
|
||||
}
|
||||
await fs.cp(path.join(params.sourceDir, entry), path.join(params.targetDir, entry), {
|
||||
recursive: true,
|
||||
force: true,
|
||||
|
||||
Reference in New Issue
Block a user