fix: guard remote sandbox skill roots (#85591)

This commit is contained in:
Peter Steinberger
2026-05-24 00:15:47 +01:00
parent a3526789a4
commit a8f68877a5
2 changed files with 117 additions and 3 deletions

View File

@@ -97,7 +97,7 @@ class RemoteShellSandboxFsBridge implements SandboxFsBridge {
signal?: AbortSignal;
}): Promise<void> {
const target = this.resolveTarget(params);
this.ensureWritable(target, "write files");
await this.ensureRemoteWritable(target, "write files", params.signal);
const pinned = await this.resolvePinnedParent({
containerPath: target.containerPath,
action: "write files",
@@ -126,7 +126,7 @@ class RemoteShellSandboxFsBridge implements SandboxFsBridge {
async mkdirp(params: { filePath: string; cwd?: string; signal?: AbortSignal }): Promise<void> {
const target = this.resolveTarget(params);
this.ensureWritable(target, "create directories");
await this.ensureRemoteWritable(target, "create directories", params.signal);
const relativePath = path.posix.relative(target.mountRootPath, target.containerPath);
if (relativePathEscapesContainerRoot(relativePath)) {
throw new Error(
@@ -147,7 +147,7 @@ class RemoteShellSandboxFsBridge implements SandboxFsBridge {
signal?: AbortSignal;
}): Promise<void> {
const target = this.resolveTarget(params);
this.ensureWritable(target, "remove files");
await this.ensureRemoteWritable(target, "remove files", params.signal);
const exists = await this.remotePathExists(target.containerPath, params.signal);
if (!exists) {
if (params.force === false) {
@@ -182,6 +182,8 @@ class RemoteShellSandboxFsBridge implements SandboxFsBridge {
signal?: AbortSignal;
}): Promise<void> {
const { from, to } = this.resolveRenameTargets(params);
await this.ensureRemoteWritable(from, "rename files", params.signal);
await this.ensureRemoteWritable(to, "rename files", params.signal);
const fromPinned = await this.resolvePinnedParent({
containerPath: from.containerPath,
action: "rename files",
@@ -384,6 +386,44 @@ class RemoteShellSandboxFsBridge implements SandboxFsBridge {
}
}
private async ensureRemoteWritable(
target: ResolvedRemotePath,
action: string,
signal?: AbortSignal,
): Promise<void> {
this.ensureWritable(target, action);
const protectedRoot = this.findRemoteProtectedSkillRoot(target.containerPath);
if (protectedRoot && (await this.remotePathExists(protectedRoot, signal))) {
throw new Error(`Sandbox path is read-only; cannot ${action}: ${target.containerPath}`);
}
}
private findRemoteProtectedSkillRoot(containerPath: string): string | null {
const roots = this.getRemoteProtectedSkillRoots().toSorted((a, b) => b.length - a.length);
for (const root of roots) {
if (isPathInsideContainerRoot(root, containerPath)) {
return root;
}
}
return null;
}
private getRemoteProtectedSkillRoots(): string[] {
const workspaceContainerRoot = normalizeContainerPath(this.runtime.remoteWorkspaceDir);
const agentContainerRoot = normalizeContainerPath(this.runtime.remoteAgentWorkspaceDir);
const roots = [
path.posix.join(workspaceContainerRoot, "skills"),
path.posix.join(workspaceContainerRoot, ".agents", "skills"),
];
if (path.resolve(this.sandbox.agentWorkspaceDir) !== path.resolve(this.sandbox.workspaceDir)) {
roots.push(
path.posix.join(agentContainerRoot, "skills"),
path.posix.join(agentContainerRoot, ".agents", "skills"),
);
}
return roots;
}
private async remotePathExists(containerPath: string, signal?: AbortSignal): Promise<boolean> {
const result = await this.runRemoteScript({
script: 'if [ -e "$1" ] || [ -L "$1" ]; then printf "1\\n"; else printf "0\\n"; fi',

View File

@@ -97,6 +97,80 @@ describe("workspace skills bridge mount policy", () => {
},
);
it.runIf(process.platform !== "win32")(
"rejects remote bridge writes under remote-only skill roots",
async () => {
await withTempDir("openclaw-skills-remote-only-", async (stateDir) => {
const workspaceDir = path.join(stateDir, "workspace");
const remoteWorkspaceDir = path.join(stateDir, "remote-workspace");
await fs.mkdir(workspaceDir, { recursive: true });
await fs.mkdir(path.join(remoteWorkspaceDir, "skills", "demo"), { recursive: true });
const canonicalWorkspaceDir = await fs.realpath(workspaceDir);
const canonicalRemoteWorkspaceDir = await fs.realpath(remoteWorkspaceDir);
const bridge = createRemoteShellSandboxFsBridge({
sandbox: createSandbox({
workspaceDir: canonicalWorkspaceDir,
agentWorkspaceDir: canonicalWorkspaceDir,
}),
runtime: {
remoteWorkspaceDir: canonicalRemoteWorkspaceDir,
remoteAgentWorkspaceDir: canonicalRemoteWorkspaceDir,
runRemoteShellScript: async (command) => {
const result = command.script.includes('python3 /dev/fd/3 "$@" 3<<')
? spawnSync(
"python3",
["-c", SANDBOX_PINNED_MUTATION_PYTHON, ...(command.args ?? [])],
{
input: command.stdin,
encoding: "buffer",
stdio: ["pipe", "pipe", "pipe"],
},
)
: spawnSync(
"sh",
["-c", command.script, "openclaw-test", ...(command.args ?? [])],
{
input: command.stdin,
encoding: "buffer",
stdio: ["pipe", "pipe", "pipe"],
},
);
const stdout = Buffer.isBuffer(result.stdout)
? result.stdout
: Buffer.from(result.stdout ?? []);
const stderr = Buffer.isBuffer(result.stderr)
? result.stderr
: Buffer.from(result.stderr ?? []);
const code = result.status ?? (result.signal ? 128 : 1);
if (result.error) {
throw result.error;
}
if (code !== 0 && !command.allowFailure) {
throw Object.assign(
new Error(stderr.toString("utf8").trim() || `shell exited with code ${code}`),
{ code, stdout, stderr },
);
}
return { stdout, stderr, code };
},
},
});
await expect(
bridge.writeFile({
filePath: "skills/demo/SKILL.md",
cwd: canonicalRemoteWorkspaceDir,
data: "# Demo\n",
}),
).rejects.toThrow(/read-only/);
await expect(
fs.stat(path.join(canonicalRemoteWorkspaceDir, "skills", "demo", "SKILL.md")),
).rejects.toMatchObject({ code: "ENOENT" });
});
},
);
it.runIf(process.platform !== "win32")(
"rejects remote bridge mkdirp under skill roots from container cwd",
async () => {