Skip to content

Commit 3b6bcbf

Browse files
brokemac79brokemac79
authored
fix: make sandbox skills readable in writable sandboxes
Materializes prompt-visible skills into a protected sandbox-readable workspace for rw sandboxes, refreshes Docker/SSH/OpenShell views, and hardens stale or poisoned remote skill copies. Fixes #90410.
1 parent e498d39 commit 3b6bcbf

36 files changed

Lines changed: 1996 additions & 193 deletions

‎docs/gateway/sandboxing.md‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -318,7 +318,7 @@ With the OpenShell backend:
318318
Inbound media is copied into the active sandbox workspace (`media/inbound/*`).
319319

320320
<Note>
321-
**Skills note:** the `read` tool is sandbox-rooted. With `workspaceAccess: "none"`, OpenClaw mirrors eligible skills into the sandbox workspace (`.../skills`) so they can be read. With `"rw"`, workspace skills are readable from `/workspace/skills`.
321+
**Skills note:** the `read` tool is sandbox-rooted. With `workspaceAccess: "none"`, OpenClaw mirrors eligible skills into the sandbox workspace (`.../skills`) so they can be read. With `"rw"`, workspace skills are readable from `/workspace/skills`, and eligible managed, bundled, or plugin skills are materialized into the generated read-only path `/workspace/.openclaw/sandbox-skills/skills`.
322322
</Note>
323323

324324
## Custom bind mounts

‎extensions/openshell/src/backend.ts‎

Lines changed: 175 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,7 @@ import { resolveOpenShellPluginConfig, type ResolvedOpenShellPluginConfig } from
3131
import { createOpenShellFsBridge } from "./fs-bridge.js";
3232
import {
3333
DEFAULT_OPEN_SHELL_MIRROR_EXCLUDE_DIRS,
34+
movePathWithCopyFallback,
3435
replaceDirectoryContents,
3536
stageDirectoryContents,
3637
} from "./mirror.js";
@@ -43,6 +44,48 @@ type PendingExec = {
4344
sshSession: SshSandboxSession;
4445
};
4546

47+
const MATERIALIZED_SKILLS_REMOTE_PARTS = [".openclaw", "sandbox-skills"] as const;
48+
const ENSURE_REMOTE_REAL_DIRECTORY_SCRIPT = [
49+
"set -e",
50+
'target="$1"',
51+
'root="${2:-$1}"',
52+
'case "$target" in /*) ;; *) echo "remote directory must be absolute: $target" >&2; exit 1 ;; esac',
53+
'case "$root" in /*) ;; *) echo "remote root must be absolute: $root" >&2; exit 1 ;; esac',
54+
'target="${target%/}"',
55+
'root="${root%/}"',
56+
'[ -n "$target" ] || target="/"',
57+
'[ -n "$root" ] || root="/"',
58+
'case "$target/" in "$root"/*|"$root/") ;; *) echo "remote directory must stay under root: $target" >&2; exit 1 ;; esac',
59+
'old_ifs="$IFS"',
60+
'IFS="/"',
61+
"set -- ${target#/} ${root#/}",
62+
'IFS="$old_ifs"',
63+
"for part do",
64+
' [ -n "$part" ] || continue',
65+
' case "$part" in "."|"..") echo "unsafe remote directory component: $part" >&2; exit 1 ;; esac',
66+
"done",
67+
'if [ -L "$root" ]; then echo "unsafe remote root symlink: $root" >&2; exit 1; fi',
68+
'mkdir -p -- "$root"',
69+
'canonical_root="$(cd "$root" && pwd -P)"',
70+
'relative="${target#"$root"}"',
71+
'relative="${relative#/}"',
72+
'current="$canonical_root"',
73+
'IFS="/"',
74+
"set -- $relative",
75+
'IFS="$old_ifs"',
76+
"for part do",
77+
' [ -n "$part" ] || continue',
78+
' if [ "$current" = "/" ]; then next="/$part"; else next="$current/$part"; fi',
79+
' if [ -L "$next" ]; then echo "unsafe remote directory symlink: $next" >&2; exit 1; fi',
80+
' if [ -e "$next" ]; then',
81+
' if [ ! -d "$next" ]; then echo "unsafe remote directory component: $next" >&2; exit 1; fi',
82+
" else",
83+
' mkdir -- "$next"',
84+
" fi",
85+
' current="$next"',
86+
"done",
87+
].join("\n");
88+
4689
export function buildOpenShellSshExecEnv(): NodeJS.ProcessEnv {
4790
return sanitizeEnvVars(process.env).allowed;
4891
}
@@ -221,7 +264,10 @@ class OpenShellSandboxBackendImpl {
221264
if (this.params.execContext.config.mode === "mirror") {
222265
await this.syncWorkspaceToRemote();
223266
} else {
224-
await this.maybeSeedRemoteWorkspace();
267+
const seeded = await this.maybeSeedRemoteWorkspace();
268+
if (!seeded) {
269+
await this.syncSkillsWorkspaceToRemote();
270+
}
225271
}
226272
const sshSession = await createOpenShellSshSession({
227273
context: this.params.execContext,
@@ -257,7 +303,10 @@ class OpenShellSandboxBackendImpl {
257303
params: SandboxBackendCommandParams,
258304
): Promise<SandboxBackendCommandResult> {
259305
await this.ensureSandboxExists();
260-
await this.maybeSeedRemoteWorkspace();
306+
const seeded = await this.maybeSeedRemoteWorkspace();
307+
if (!seeded) {
308+
await this.syncSkillsWorkspaceToRemote();
309+
}
261310
return await this.runRemoteShellScriptInternal(params);
262311
}
263312

@@ -410,6 +459,31 @@ class OpenShellSandboxBackendImpl {
410459
this.params.remoteAgentWorkspaceDir,
411460
);
412461
}
462+
await this.syncSkillsWorkspaceToRemote();
463+
}
464+
465+
private async syncSkillsWorkspaceToRemote(): Promise<void> {
466+
if (
467+
this.params.createParams.cfg.workspaceAccess !== "rw" ||
468+
!this.params.createParams.skillsWorkspaceDir
469+
) {
470+
return;
471+
}
472+
const remoteSkillsWorkspaceDir = resolveRemoteMaterializedSkillsWorkspaceDir(
473+
this.params.remoteWorkspaceDir,
474+
);
475+
await this.runRemoteShellScriptInternal({
476+
script: `${ENSURE_REMOTE_REAL_DIRECTORY_SCRIPT}\nfind "$1" -mindepth 1 -maxdepth 1 -exec rm -rf -- {} +`,
477+
args: [remoteSkillsWorkspaceDir, this.params.remoteWorkspaceDir],
478+
});
479+
const stats = await fs.lstat(this.params.createParams.skillsWorkspaceDir).catch(() => null);
480+
if (!stats?.isDirectory() || stats.isSymbolicLink()) {
481+
return;
482+
}
483+
await this.uploadPathToRemote(
484+
this.params.createParams.skillsWorkspaceDir,
485+
remoteSkillsWorkspaceDir,
486+
);
413487
}
414488

415489
private async syncWorkspaceFromRemote(): Promise<void> {
@@ -430,13 +504,25 @@ class OpenShellSandboxBackendImpl {
430504
if (result.code !== 0) {
431505
throw new Error(result.stderr.trim() || "openshell sandbox download failed");
432506
}
433-
await replaceDirectoryContents({
434-
sourceDir: tmpDir,
435-
targetDir: this.params.createParams.workspaceDir,
436-
// Never sync trusted host hook directories or repository metadata from
437-
// the remote sandbox.
438-
excludeDirs: DEFAULT_OPEN_SHELL_MIRROR_EXCLUDE_DIRS,
507+
await removeMaterializedSkillsFromDownloadedWorkspace(tmpDir);
508+
const preservedSandboxSkills = await moveMaterializedSkillsShadowAside({
509+
workspaceDir: this.params.createParams.workspaceDir,
510+
tmpDir,
439511
});
512+
try {
513+
await replaceDirectoryContents({
514+
sourceDir: tmpDir,
515+
targetDir: this.params.createParams.workspaceDir,
516+
// Never sync trusted host hook directories or repository metadata from
517+
// the remote sandbox.
518+
excludeDirs: DEFAULT_OPEN_SHELL_MIRROR_EXCLUDE_DIRS,
519+
});
520+
} finally {
521+
await restoreMaterializedSkillsShadow({
522+
workspaceDir: this.params.createParams.workspaceDir,
523+
preserved: preservedSandboxSkills,
524+
});
525+
}
440526
},
441527
);
442528
}
@@ -470,13 +556,14 @@ class OpenShellSandboxBackendImpl {
470556
);
471557
}
472558

473-
private async maybeSeedRemoteWorkspace(): Promise<void> {
559+
private async maybeSeedRemoteWorkspace(): Promise<boolean> {
474560
if (!this.remoteSeedPending) {
475-
return;
561+
return false;
476562
}
477563
this.remoteSeedPending = false;
478564
try {
479565
await this.syncWorkspaceToRemote();
566+
return true;
480567
} catch (error) {
481568
this.remoteSeedPending = true;
482569
throw error;
@@ -508,6 +595,84 @@ export function buildOpenShellSandboxName(scopeKey: string): string {
508595
return `openclaw-${safe || "session"}-${hash.toString(16).slice(0, 8)}`;
509596
}
510597

598+
function resolveRemoteMaterializedSkillsWorkspaceDir(remoteWorkspaceDir: string): string {
599+
const root = remoteWorkspaceDir.replace(/\\/g, "/").replace(/\/+$/, "") || "/";
600+
return path.posix.join(root, ...MATERIALIZED_SKILLS_REMOTE_PARTS);
601+
}
602+
603+
async function removeMaterializedSkillsFromDownloadedWorkspace(tmpDir: string): Promise<void> {
604+
let cursor = tmpDir;
605+
for (const [index, part] of MATERIALIZED_SKILLS_REMOTE_PARTS.entries()) {
606+
const next = path.join(cursor, part);
607+
const stats = await fs.lstat(next).catch(() => null);
608+
if (!stats) {
609+
return;
610+
}
611+
if (index === MATERIALIZED_SKILLS_REMOTE_PARTS.length - 1) {
612+
await fs.rm(next, { recursive: true, force: true });
613+
return;
614+
}
615+
if (stats.isSymbolicLink() || !stats.isDirectory()) {
616+
await fs.rm(next, { recursive: true, force: true });
617+
return;
618+
}
619+
cursor = next;
620+
}
621+
}
622+
623+
async function moveMaterializedSkillsShadowAside(params: {
624+
workspaceDir: string;
625+
tmpDir: string;
626+
}): Promise<{ preservedPath: string; preserveRoot: string } | undefined> {
627+
const shadowPath = path.join(params.workspaceDir, ...MATERIALIZED_SKILLS_REMOTE_PARTS);
628+
const parentStats = await fs.lstat(path.dirname(shadowPath)).catch(() => null);
629+
if (!parentStats?.isDirectory() || parentStats.isSymbolicLink()) {
630+
return undefined;
631+
}
632+
const shadowStats = await fs.lstat(shadowPath).catch(() => null);
633+
if (!shadowStats || shadowStats.isSymbolicLink()) {
634+
return undefined;
635+
}
636+
const preserveRoot = await fs.mkdtemp(
637+
path.join(path.dirname(params.tmpDir), "openclaw-openshell-preserve-"),
638+
);
639+
const preservedPath = path.join(preserveRoot, "sandbox-skills");
640+
await movePathWithCopyFallback({ from: shadowPath, to: preservedPath });
641+
return { preservedPath, preserveRoot };
642+
}
643+
644+
async function restoreMaterializedSkillsShadow(params: {
645+
workspaceDir: string;
646+
preserved?: { preservedPath: string; preserveRoot: string };
647+
}): Promise<void> {
648+
if (!params.preserved) {
649+
return;
650+
}
651+
let restored = false;
652+
try {
653+
const shadowPath = path.join(params.workspaceDir, ...MATERIALIZED_SKILLS_REMOTE_PARTS);
654+
const parentPath = path.dirname(shadowPath);
655+
const parentStats = await fs.lstat(parentPath).catch(() => null);
656+
if (parentStats?.isSymbolicLink()) {
657+
throw new Error(`Refusing to restore sandbox skills through symlink parent: ${parentPath}`);
658+
}
659+
if (parentStats && !parentStats.isDirectory()) {
660+
await fs.rm(parentPath, { recursive: true, force: true });
661+
}
662+
await fs.mkdir(parentPath, { recursive: true });
663+
await fs.rm(shadowPath, { recursive: true, force: true });
664+
await movePathWithCopyFallback({
665+
from: params.preserved.preservedPath,
666+
to: shadowPath,
667+
});
668+
restored = true;
669+
} finally {
670+
if (restored) {
671+
await fs.rm(params.preserved.preserveRoot, { recursive: true, force: true });
672+
}
673+
}
674+
}
675+
511676
function resolveOpenShellTmpRoot(): string {
512677
return path.resolve(resolvePreferredOpenClawTmpDir());
513678
}

0 commit comments

Comments
 (0)