fix(skills): pin validated download roots

This commit is contained in:
Peter Steinberger
2026-03-09 05:59:31 +00:00
parent cf3a479bd1
commit 9abf014f35
3 changed files with 59 additions and 6 deletions

View File

@@ -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

View File

@@ -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",
});

View File

@@ -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)", () => {