From 9abf014f3502009faf9c73df5ca2cff719e54639 Mon Sep 17 00:00:00 2001
From: Peter Steinberger <steipete@gmail.com>
Date: Mon, 9 Mar 2026 05:59:31 +0000
Subject: [PATCH] fix(skills): pin validated download roots

---
 CHANGELOG.md                               |  1 +
 src/agents/skills-install-download.ts      | 23 ++++++++----
 src/agents/skills-install.download.test.ts | 41 ++++++++++++++++++++++
 3 files changed, 59 insertions(+), 6 deletions(-)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index a58cab6915f..f1b4cbe4a93 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -9,6 +9,7 @@ Docs: https://docs.openclaw.ai
 - Browser/SSRF: block private-network intermediate redirect hops in strict browser navigation flows and fail closed when remote tab-open paths cannot inspect redirect chains. Thanks @zpbrent.
 - MS Teams/authz: keep `groupPolicy: "allowlist"` enforcing sender allowlists even when a team/channel route allowlist is configured, so route matches no longer widen group access to every sender in that route. Thanks @zpbrent.
 - Security/system.run: bind approved `bun` and `deno run` script operands to on-disk file snapshots so post-approval script rewrites are denied before execution.
+- Skills/download installs: pin the validated per-skill tools root before writing downloaded archives, so rebinding the lexical tools path cannot redirect download writes outside the intended tools directory. Thanks @tdjackey.
 
 ## 2026.3.8
 
diff --git a/src/agents/skills-install-download.ts b/src/agents/skills-install-download.ts
index 345fd1a3698..f5c62ceb0e8 100644
--- a/src/agents/skills-install-download.ts
+++ b/src/agents/skills-install-download.ts
@@ -130,22 +130,33 @@ export async function installDownloadSpec(params: {
     filename = "download";
   }
 
+  let canonicalSafeRoot = "";
   let targetDir = "";
   try {
-    targetDir = resolveDownloadTargetDir(entry, spec);
-    await ensureDir(targetDir);
+    await ensureDir(safeRoot);
     await assertCanonicalPathWithinBase({
       baseDir: safeRoot,
-      candidatePath: targetDir,
+      candidatePath: safeRoot,
       boundaryLabel: "skill tools directory",
     });
+    canonicalSafeRoot = await fs.promises.realpath(safeRoot);
+
+    const requestedTargetDir = resolveDownloadTargetDir(entry, spec);
+    await ensureDir(requestedTargetDir);
+    await assertCanonicalPathWithinBase({
+      baseDir: safeRoot,
+      candidatePath: requestedTargetDir,
+      boundaryLabel: "skill tools directory",
+    });
+    const targetRelativePath = path.relative(safeRoot, requestedTargetDir);
+    targetDir = path.join(canonicalSafeRoot, targetRelativePath);
   } catch (err) {
     const message = err instanceof Error ? err.message : String(err);
     return { ok: false, message, stdout: "", stderr: message, code: null };
   }
 
   const archivePath = path.join(targetDir, filename);
-  const archiveRelativePath = path.relative(safeRoot, archivePath);
+  const archiveRelativePath = path.relative(canonicalSafeRoot, archivePath);
   if (
     !archiveRelativePath ||
     archiveRelativePath === ".." ||
@@ -164,7 +175,7 @@ export async function installDownloadSpec(params: {
   try {
     const result = await downloadFile({
       url,
-      rootDir: safeRoot,
+      rootDir: canonicalSafeRoot,
       relativePath: archiveRelativePath,
       timeoutMs,
     });
@@ -198,7 +209,7 @@ export async function installDownloadSpec(params: {
 
   try {
     await assertCanonicalPathWithinBase({
-      baseDir: safeRoot,
+      baseDir: canonicalSafeRoot,
       candidatePath: targetDir,
       boundaryLabel: "skill tools directory",
     });
diff --git a/src/agents/skills-install.download.test.ts b/src/agents/skills-install.download.test.ts
index e030b9cbf76..0c357089678 100644
--- a/src/agents/skills-install.download.test.ts
+++ b/src/agents/skills-install.download.test.ts
@@ -251,6 +251,47 @@ describe("installDownloadSpec extraction safety", () => {
       ),
     ).toBe("hi");
   });
+
+  it.runIf(process.platform !== "win32")(
+    "fails closed when the lexical tools root is rebound before the final copy",
+    async () => {
+      const entry = buildEntry("base-rebind");
+      const safeRoot = resolveSkillToolsRootDir(entry);
+      const outsideRoot = path.join(workspaceDir, "outside-root");
+      await fs.mkdir(outsideRoot, { recursive: true });
+
+      fetchWithSsrFGuardMock.mockResolvedValue({
+        response: new Response(
+          new ReadableStream({
+            async start(controller) {
+              controller.enqueue(new Uint8Array(Buffer.from("payload")));
+              const reboundRoot = `${safeRoot}-rebound`;
+              await fs.rename(safeRoot, reboundRoot);
+              await fs.symlink(outsideRoot, safeRoot);
+              controller.close();
+            },
+          }),
+          { status: 200 },
+        ),
+        release: async () => undefined,
+      });
+
+      const result = await installDownloadSpec({
+        entry,
+        spec: {
+          kind: "download",
+          id: "dl",
+          url: "https://example.invalid/payload.bin",
+          extract: false,
+          targetDir: "runtime",
+        },
+        timeoutMs: 30_000,
+      });
+
+      expect(result.ok).toBe(false);
+      expect(await fileExists(path.join(outsideRoot, "runtime", "payload.bin"))).toBe(false);
+    },
+  );
 });
 
 describe("installDownloadSpec extraction safety (tar.bz2)", () => {