Skip to content

Commit 737620a

Browse files
PratikRai0101PratikRai0101
authored
Merge branch 'main' into fix/sandbox-off-docker-probe
2 parents 3d6f87e + 230f712 commit 737620a

11 files changed

Lines changed: 963 additions & 97 deletions

‎CHANGELOG.md‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ Docs: https://docs.openclaw.ai
1212

1313
### Fixes
1414

15+
- fix(security): prevent workspace PATH injection via service env and trash helpers. (#73264) Thanks @pgondhi987.
1516
- Active Memory: allow `allowedChatTypes` to include explicit portal/webchat sessions and classify `agent:...:explicit:...` session keys before opaque session ids can shadow the chat type. Fixes #65775. (#66285) Thanks @Lidang-Jiang.
1617
- Active Memory: allow the hidden recall sub-agent to use both `memory_recall` and the legacy `memory_search`/`memory_get` memory tool contract, so bundled `memory-lancedb` recall works without breaking the default `memory-core` path. Fixes #73502. (#73584) Thanks @Takhoffman.
1718
- fix(device-pairing): validate callerScopes against resolved token scopes on repair [AI]. (#72925) Thanks @pgondhi987.
@@ -27,6 +28,7 @@ Docs: https://docs.openclaw.ai
2728
- Acpx/runtime: validate the runtime session mode at the `AcpxRuntime.ensureSession` wrapper boundary so callers that pass anything other than `persistent` or `oneshot` get a clear `ACP_INVALID_RUNTIME_OPTION` error instead of silently round-tripping through the encoded handle as a default `persistent` mode and later throwing `SessionResumeRequiredError`. Investigation context: #73071. (#73548) Thanks @amknight.
2829
- CLI/infer: keep web-search fallback on missing provider API keys, preserve structured validation errors from the selected provider, and let per-request image describe prompts override configured media-entry prompts. (#63263) Thanks @Spolen23.
2930
- WhatsApp/Web: pass explicit Baileys socket timings into every WhatsApp Web socket and expose `web.whatsapp.*` keepalive, connect, and query timeout settings so unstable networks can avoid repeated 408 disconnect and opening-handshake timeout loops. Fixes #56365. (#73580) Thanks @velvet-shark.
31+
- Channels/Telegram: persist native command metadata on target sessions so topic, helper, and ACP-bound slash commands keep their session metadata attached to the routed conversation. (#57548) Thanks @GaosCode.
3032

3133
## 2026.4.27
3234

‎extensions/browser/src/browser/trash.test.ts‎

Lines changed: 177 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -8,36 +8,197 @@ vi.mock("../process/exec.js", () => ({
88
runExec,
99
}));
1010

11+
function mockTrashContainer(...suffixes: string[]) {
12+
let call = 0;
13+
return vi.spyOn(fs, "mkdtempSync").mockImplementation((prefix) => {
14+
const suffix = suffixes[call] ?? "secure";
15+
call += 1;
16+
return `${prefix}${suffix}`;
17+
});
18+
}
19+
1120
describe("browser trash", () => {
1221
beforeEach(() => {
1322
vi.restoreAllMocks();
1423
runExec.mockReset();
1524
vi.spyOn(Date, "now").mockReturnValue(123);
1625
vi.spyOn(os, "homedir").mockReturnValue("/home/test");
26+
vi.spyOn(os, "tmpdir").mockReturnValue("/tmp");
27+
vi.spyOn(fs, "lstatSync").mockReturnValue({
28+
isDirectory: () => true,
29+
isSymbolicLink: () => false,
30+
} as fs.Stats);
31+
vi.spyOn(fs.realpathSync, "native").mockImplementation((candidate) => String(candidate));
1732
});
1833

19-
it("returns the target path when trash exits successfully", async () => {
34+
it("moves paths to a reserved user trash container without invoking a PATH-resolved command", async () => {
2035
const { movePathToTrash } = await import("./trash.js");
21-
runExec.mockResolvedValue(undefined);
22-
const mkdirSync = vi.spyOn(fs, "mkdirSync");
23-
const renameSync = vi.spyOn(fs, "renameSync");
36+
const mkdirSync = vi.spyOn(fs, "mkdirSync").mockImplementation(() => undefined);
37+
const mkdtempSync = mockTrashContainer("secure");
38+
const renameSync = vi.spyOn(fs, "renameSync").mockImplementation(() => undefined);
39+
const cpSync = vi.spyOn(fs, "cpSync");
40+
const rmSync = vi.spyOn(fs, "rmSync");
2441

25-
await expect(movePathToTrash("/tmp/demo")).resolves.toBe("/tmp/demo");
26-
expect(runExec).toHaveBeenCalledWith("trash", ["/tmp/demo"], { timeoutMs: 10_000 });
27-
expect(mkdirSync).not.toHaveBeenCalled();
28-
expect(renameSync).not.toHaveBeenCalled();
42+
await expect(movePathToTrash("/tmp/demo")).resolves.toBe(
43+
"/home/test/.Trash/demo-123-secure/demo",
44+
);
45+
expect(runExec).not.toHaveBeenCalled();
46+
expect(mkdirSync).toHaveBeenCalledWith("/home/test/.Trash", {
47+
recursive: true,
48+
mode: 0o700,
49+
});
50+
expect(mkdtempSync).toHaveBeenCalledWith("/home/test/.Trash/demo-123-");
51+
expect(renameSync).toHaveBeenCalledWith("/tmp/demo", "/home/test/.Trash/demo-123-secure/demo");
52+
expect(cpSync).not.toHaveBeenCalled();
53+
expect(rmSync).not.toHaveBeenCalled();
2954
});
3055

31-
it("falls back to rename when trash exits non-zero", async () => {
56+
it("uses the resolved trash directory for reserved destinations", async () => {
3257
const { movePathToTrash } = await import("./trash.js");
33-
runExec.mockRejectedValue(new Error("permission denied"));
34-
const mkdirSync = vi.spyOn(fs, "mkdirSync").mockImplementation(() => undefined);
35-
const existsSync = vi.spyOn(fs, "existsSync").mockReturnValue(false);
58+
vi.spyOn(fs, "mkdirSync").mockImplementation(() => undefined);
59+
vi.spyOn(fs.realpathSync, "native").mockImplementation((candidate) => {
60+
const value = String(candidate);
61+
if (value === "/home/test") {
62+
return "/real/home/test";
63+
}
64+
if (value === "/home/test/.Trash") {
65+
return "/real/home/test/.Trash";
66+
}
67+
return value;
68+
});
69+
const mkdtempSync = mockTrashContainer("secure");
3670
const renameSync = vi.spyOn(fs, "renameSync").mockImplementation(() => undefined);
3771

38-
await expect(movePathToTrash("/tmp/demo")).resolves.toBe("/home/test/.Trash/demo-123");
39-
expect(mkdirSync).toHaveBeenCalledWith("/home/test/.Trash", { recursive: true });
40-
expect(existsSync).toHaveBeenCalledWith("/home/test/.Trash/demo-123");
41-
expect(renameSync).toHaveBeenCalledWith("/tmp/demo", "/home/test/.Trash/demo-123");
72+
await expect(movePathToTrash("/tmp/demo")).resolves.toBe(
73+
"/real/home/test/.Trash/demo-123-secure/demo",
74+
);
75+
expect(mkdtempSync).toHaveBeenCalledWith("/real/home/test/.Trash/demo-123-");
76+
expect(renameSync).toHaveBeenCalledWith(
77+
"/tmp/demo",
78+
"/real/home/test/.Trash/demo-123-secure/demo",
79+
);
80+
});
81+
82+
it("refuses to trash filesystem roots", async () => {
83+
const { movePathToTrash } = await import("./trash.js");
84+
85+
await expect(movePathToTrash("/")).rejects.toThrow("Refusing to trash root path");
86+
});
87+
88+
it("refuses to trash paths outside allowed roots", async () => {
89+
const { movePathToTrash } = await import("./trash.js");
90+
91+
await expect(movePathToTrash("/etc/openclaw-demo")).rejects.toThrow(
92+
"Refusing to trash path outside allowed roots",
93+
);
94+
});
95+
96+
it("refuses to use a symlinked trash directory", async () => {
97+
const { movePathToTrash } = await import("./trash.js");
98+
vi.spyOn(fs, "mkdirSync").mockImplementation(() => undefined);
99+
vi.spyOn(fs, "lstatSync").mockReturnValue({
100+
isDirectory: () => true,
101+
isSymbolicLink: () => true,
102+
} as fs.Stats);
103+
104+
await expect(movePathToTrash("/tmp/demo")).rejects.toThrow(
105+
"Refusing to use non-directory/symlink trash directory",
106+
);
107+
});
108+
109+
it("falls back to copy and remove when rename crosses filesystems", async () => {
110+
const { movePathToTrash } = await import("./trash.js");
111+
const exdev = Object.assign(new Error("cross-device"), { code: "EXDEV" });
112+
vi.spyOn(fs, "mkdirSync").mockImplementation(() => undefined);
113+
mockTrashContainer("secure");
114+
vi.spyOn(fs, "renameSync").mockImplementation(() => {
115+
throw exdev;
116+
});
117+
const cpSync = vi.spyOn(fs, "cpSync").mockImplementation(() => undefined);
118+
const rmSync = vi.spyOn(fs, "rmSync").mockImplementation(() => undefined);
119+
120+
await expect(movePathToTrash("/tmp/demo")).resolves.toBe(
121+
"/home/test/.Trash/demo-123-secure/demo",
122+
);
123+
expect(cpSync).toHaveBeenCalledWith("/tmp/demo", "/home/test/.Trash/demo-123-secure/demo", {
124+
recursive: true,
125+
force: false,
126+
errorOnExist: true,
127+
});
128+
expect(rmSync).toHaveBeenCalledWith("/tmp/demo", { recursive: true, force: false });
129+
});
130+
131+
it("retries copy fallback when the copy destination is created concurrently", async () => {
132+
const { movePathToTrash } = await import("./trash.js");
133+
const exdev = Object.assign(new Error("cross-device"), { code: "EXDEV" });
134+
const copyCollision = Object.assign(new Error("copy exists"), {
135+
code: "ERR_FS_CP_EEXIST",
136+
});
137+
vi.spyOn(fs, "mkdirSync").mockImplementation(() => undefined);
138+
mockTrashContainer("first", "second");
139+
vi.spyOn(fs, "renameSync").mockImplementation(() => {
140+
throw exdev;
141+
});
142+
const cpSync = vi
143+
.spyOn(fs, "cpSync")
144+
.mockImplementationOnce(() => {
145+
throw copyCollision;
146+
})
147+
.mockImplementation(() => undefined);
148+
const rmSync = vi.spyOn(fs, "rmSync").mockImplementation(() => undefined);
149+
150+
await expect(movePathToTrash("/tmp/demo")).resolves.toBe(
151+
"/home/test/.Trash/demo-123-second/demo",
152+
);
153+
expect(cpSync).toHaveBeenNthCalledWith(
154+
1,
155+
"/tmp/demo",
156+
"/home/test/.Trash/demo-123-first/demo",
157+
{
158+
recursive: true,
159+
force: false,
160+
errorOnExist: true,
161+
},
162+
);
163+
expect(cpSync).toHaveBeenNthCalledWith(
164+
2,
165+
"/tmp/demo",
166+
"/home/test/.Trash/demo-123-second/demo",
167+
{
168+
recursive: true,
169+
force: false,
170+
errorOnExist: true,
171+
},
172+
);
173+
expect(rmSync).toHaveBeenCalledTimes(1);
174+
expect(Date.now).toHaveBeenCalledTimes(1);
175+
});
176+
177+
it("retries with the same timestamp when the destination is created concurrently", async () => {
178+
const { movePathToTrash } = await import("./trash.js");
179+
const collision = Object.assign(new Error("exists"), { code: "EEXIST" });
180+
vi.spyOn(fs, "mkdirSync").mockImplementation(() => undefined);
181+
mockTrashContainer("first", "second");
182+
const renameSync = vi
183+
.spyOn(fs, "renameSync")
184+
.mockImplementationOnce(() => {
185+
throw collision;
186+
})
187+
.mockImplementation(() => undefined);
188+
189+
await expect(movePathToTrash("/tmp/demo")).resolves.toBe(
190+
"/home/test/.Trash/demo-123-second/demo",
191+
);
192+
expect(renameSync).toHaveBeenNthCalledWith(
193+
1,
194+
"/tmp/demo",
195+
"/home/test/.Trash/demo-123-first/demo",
196+
);
197+
expect(renameSync).toHaveBeenNthCalledWith(
198+
2,
199+
"/tmp/demo",
200+
"/home/test/.Trash/demo-123-second/demo",
201+
);
202+
expect(Date.now).toHaveBeenCalledTimes(1);
42203
});
43204
});

‎extensions/browser/src/browser/trash.ts‎

Lines changed: 132 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -1,22 +1,141 @@
11
import fs from "node:fs";
22
import os from "node:os";
33
import path from "node:path";
4-
import { generateSecureToken } from "../infra/secure-random.js";
5-
import { runExec } from "../process/exec.js";
64

7-
export async function movePathToTrash(targetPath: string): Promise<string> {
5+
const TRASH_DESTINATION_COLLISION_CODES = new Set(["EEXIST", "ENOTEMPTY", "ERR_FS_CP_EEXIST"]);
6+
const TRASH_DESTINATION_RETRY_LIMIT = 4;
7+
8+
function getFsErrorCode(error: unknown): string | undefined {
9+
if (!error || typeof error !== "object" || !("code" in error)) {
10+
return undefined;
11+
}
12+
const code = (error as NodeJS.ErrnoException).code;
13+
return typeof code === "string" ? code : undefined;
14+
}
15+
16+
function isTrashDestinationCollision(error: unknown): boolean {
17+
const code = getFsErrorCode(error);
18+
return Boolean(code && TRASH_DESTINATION_COLLISION_CODES.has(code));
19+
}
20+
21+
function isSameOrChildPath(candidate: string, parent: string): boolean {
22+
return candidate === parent || candidate.startsWith(`${parent}${path.sep}`);
23+
}
24+
25+
function resolveAllowedTrashRoots(): string[] {
26+
const roots = [os.homedir(), os.tmpdir()].map((root) => {
27+
try {
28+
return path.resolve(fs.realpathSync.native(root));
29+
} catch {
30+
return path.resolve(root);
31+
}
32+
});
33+
return [...new Set(roots)];
34+
}
35+
36+
function assertAllowedTrashTarget(targetPath: string): void {
37+
let resolvedTargetPath = path.resolve(targetPath);
838
try {
9-
await runExec("trash", [targetPath], { timeoutMs: 10_000 });
10-
return targetPath;
39+
resolvedTargetPath = path.resolve(fs.realpathSync.native(targetPath));
1140
} catch {
12-
const trashDir = path.join(os.homedir(), ".Trash");
13-
fs.mkdirSync(trashDir, { recursive: true });
14-
const base = path.basename(targetPath);
15-
let dest = path.join(trashDir, `${base}-${Date.now()}`);
16-
if (fs.existsSync(dest)) {
17-
dest = path.join(trashDir, `${base}-${Date.now()}-${generateSecureToken(6)}`);
18-
}
41+
// The subsequent move will surface missing or inaccessible targets.
42+
}
43+
const isAllowed = resolveAllowedTrashRoots().some(
44+
(root) => resolvedTargetPath !== root && isSameOrChildPath(resolvedTargetPath, root),
45+
);
46+
if (!isAllowed) {
47+
throw new Error(`Refusing to trash path outside allowed roots: ${targetPath}`);
48+
}
49+
}
50+
51+
function resolveTrashDir(): string {
52+
const homeDir = os.homedir();
53+
const trashDir = path.join(homeDir, ".Trash");
54+
fs.mkdirSync(trashDir, { recursive: true, mode: 0o700 });
55+
const trashDirStat = fs.lstatSync(trashDir);
56+
if (!trashDirStat.isDirectory() || trashDirStat.isSymbolicLink()) {
57+
throw new Error(`Refusing to use non-directory/symlink trash directory: ${trashDir}`);
58+
}
59+
const realHome = path.resolve(fs.realpathSync.native(homeDir));
60+
const resolvedTrashDir = path.resolve(fs.realpathSync.native(trashDir));
61+
if (resolvedTrashDir === realHome || !isSameOrChildPath(resolvedTrashDir, realHome)) {
62+
throw new Error(`Trash directory escaped home directory: ${trashDir}`);
63+
}
64+
return resolvedTrashDir;
65+
}
66+
67+
function trashBaseName(targetPath: string): string {
68+
const resolvedTargetPath = path.resolve(targetPath);
69+
if (resolvedTargetPath === path.parse(resolvedTargetPath).root) {
70+
throw new Error(`Refusing to trash root path: ${targetPath}`);
71+
}
72+
const base = path.basename(resolvedTargetPath).replace(/[\\/]+/g, "");
73+
if (!base) {
74+
throw new Error(`Unable to derive safe trash basename for: ${targetPath}`);
75+
}
76+
return base;
77+
}
78+
79+
function resolveContainedPath(root: string, leaf: string): string {
80+
const resolvedRoot = path.resolve(root);
81+
const resolvedPath = path.resolve(resolvedRoot, leaf);
82+
if (!isSameOrChildPath(resolvedPath, resolvedRoot) || resolvedPath === resolvedRoot) {
83+
throw new Error(`Trash destination escaped trash directory: ${resolvedPath}`);
84+
}
85+
return resolvedPath;
86+
}
87+
88+
function reserveTrashDestination(trashDir: string, base: string, timestamp: number): string {
89+
const containerPrefix = resolveContainedPath(trashDir, `${base}-${timestamp}-`);
90+
const container = fs.mkdtempSync(containerPrefix);
91+
const resolvedContainer = path.resolve(container);
92+
const resolvedTrashDir = path.resolve(trashDir);
93+
if (
94+
resolvedContainer === resolvedTrashDir ||
95+
!isSameOrChildPath(resolvedContainer, resolvedTrashDir)
96+
) {
97+
throw new Error(`Trash destination escaped trash directory: ${container}`);
98+
}
99+
return resolveContainedPath(container, base);
100+
}
101+
102+
function movePathToDestination(targetPath: string, dest: string): boolean {
103+
try {
19104
fs.renameSync(targetPath, dest);
20-
return dest;
105+
return true;
106+
} catch (error) {
107+
if (getFsErrorCode(error) !== "EXDEV") {
108+
if (isTrashDestinationCollision(error)) {
109+
return false;
110+
}
111+
throw error;
112+
}
21113
}
114+
115+
try {
116+
fs.cpSync(targetPath, dest, { recursive: true, force: false, errorOnExist: true });
117+
fs.rmSync(targetPath, { recursive: true, force: false });
118+
return true;
119+
} catch (error) {
120+
if (isTrashDestinationCollision(error)) {
121+
return false;
122+
}
123+
throw error;
124+
}
125+
}
126+
127+
export async function movePathToTrash(targetPath: string): Promise<string> {
128+
// Avoid resolving external trash helpers through the service PATH during cleanup.
129+
const base = trashBaseName(targetPath);
130+
assertAllowedTrashTarget(targetPath);
131+
const trashDir = resolveTrashDir();
132+
const timestamp = Date.now();
133+
for (let attempt = 0; attempt < TRASH_DESTINATION_RETRY_LIMIT; attempt += 1) {
134+
const dest = reserveTrashDestination(trashDir, base, timestamp);
135+
if (movePathToDestination(targetPath, dest)) {
136+
return dest;
137+
}
138+
}
139+
140+
throw new Error(`Unable to choose a unique trash destination for ${targetPath}`);
22141
}

0 commit comments

Comments
 (0)