mirror of
https://github.com/openclaw/openclaw.git
synced 2026-10-03 09:39:25 +00:00
fix(sandbox): sessions fail with EACCES after skills refresh on read-only installs (#162304)
* fix(sandbox): keep synced skill copies removable on read-only installs Sandbox skill sync copied bundled skills with fs.cp, which keeps source modes. From a read-only install every copied directory became 0555, so the next full re-sync (skills version bump or Gateway restart) failed with EACCES unlinking skills/<skill>/SKILL.md, and library-pinned sessions failed every turn. Copied directories are now made owner-writable (file modes unchanged), and removal repairs legacy read-only trees inside the synced child before one retry. The repair stays inside the skills root and never follows symlinks. The Claude CLI skills-plugin copy fallback uses the same helper. * fix(sandbox): classify skill removal errors without a type assertion
This commit is contained in:
parent
55fe1b4889
commit
be93e85955
4 changed files with 135 additions and 1 deletions
|
|
@ -6,6 +6,7 @@ import fs from "node:fs/promises";
|
|||
import path from "node:path";
|
||||
import { normalizeLowercaseStringOrEmpty } from "@openclaw/normalization-core/string-coerce";
|
||||
import { resolvePreferredOpenClawTmpDir } from "../../infra/tmp-openclaw-dir.js";
|
||||
import { ensureWritableSkillDirectories } from "../../skills/loading/skill-directory-modes.js";
|
||||
import type { SkillSnapshot } from "../../skills/types.js";
|
||||
import { cliBackendLog } from "./log.js";
|
||||
|
||||
|
|
@ -43,6 +44,10 @@ async function linkOrCopySkillDir(params: { sourceDir: string; targetDir: string
|
|||
force: true,
|
||||
verbatimSymlinks: true,
|
||||
});
|
||||
await ensureWritableSkillDirectories(
|
||||
path.dirname(params.targetDir),
|
||||
path.basename(params.targetDir),
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
|
|
|
|||
53
src/skills/loading/skill-directory-modes.ts
Normal file
53
src/skills/loading/skill-directory-modes.ts
Normal file
|
|
@ -0,0 +1,53 @@
|
|||
import fs from "node:fs";
|
||||
import path from "node:path";
|
||||
import { root } from "@openclaw/fs-safe/root";
|
||||
import { openRootFileSync } from "../../infra/boundary-file-read.js";
|
||||
|
||||
export async function ensureWritableSkillDirectories(
|
||||
skillsDir: string,
|
||||
relativePath: string,
|
||||
): Promise<void> {
|
||||
if (process.platform === "win32") {
|
||||
return;
|
||||
}
|
||||
const fixedRoot = path.join(
|
||||
await fs.promises.realpath(path.dirname(skillsDir)),
|
||||
path.basename(skillsDir),
|
||||
);
|
||||
const boundary = await root(fixedRoot);
|
||||
if (boundary.rootReal !== fixedRoot) {
|
||||
throw new Error("Skill directory root must not be a symbolic link");
|
||||
}
|
||||
const visit = async (relativeDir: string): Promise<void> => {
|
||||
const scopedPath = `.${path.sep}${relativeDir}`;
|
||||
const stat = await boundary.stat(scopedPath);
|
||||
if (stat.isSymbolicLink || !stat.isDirectory) {
|
||||
return;
|
||||
}
|
||||
const opened = openRootFileSync({
|
||||
absolutePath: path.join(fixedRoot, relativeDir),
|
||||
rootPath: fixedRoot,
|
||||
rootRealPath: fixedRoot,
|
||||
boundaryLabel: "skills directory",
|
||||
allowedType: "directory",
|
||||
symlinks: "reject",
|
||||
});
|
||||
if (!opened.ok) {
|
||||
throw new Error("Could not open skill directory safely", { cause: opened.error });
|
||||
}
|
||||
try {
|
||||
// Sealed release copies (including legacy sandboxes) need writable directories, not files.
|
||||
if ((opened.stat.mode & 0o700) !== 0o700) {
|
||||
fs.fchmodSync(opened.fd, opened.stat.mode | 0o700);
|
||||
}
|
||||
} finally {
|
||||
fs.closeSync(opened.fd);
|
||||
}
|
||||
for (const child of await boundary.list(scopedPath, { withFileTypes: true })) {
|
||||
if (child.isDirectory && !child.isSymbolicLink) {
|
||||
await visit(path.join(relativeDir, child.name));
|
||||
}
|
||||
}
|
||||
};
|
||||
await visit(relativePath);
|
||||
}
|
||||
|
|
@ -4,6 +4,7 @@ import os from "node:os";
|
|||
import path from "node:path";
|
||||
import { afterAll, beforeAll, describe, expect, it, vi } from "vitest";
|
||||
import type { OpenClawConfig } from "../../config/types.openclaw.js";
|
||||
import { hasErrnoCode } from "../../infra/errno.js";
|
||||
import { withEnvAsync } from "../../test-utils/env.js";
|
||||
import { bumpSkillsSnapshotVersion, getSkillsSnapshotVersion } from "../runtime/refresh-state.js";
|
||||
import { resolveReusableWorkspaceSkillSnapshot } from "../runtime/session-snapshot.js";
|
||||
|
|
@ -478,6 +479,68 @@ describe("syncWorkspaceSkills", () => {
|
|||
);
|
||||
});
|
||||
|
||||
it
|
||||
.runIf(process.platform !== "win32" && process.getuid?.() !== 0)
|
||||
.each(["copied source", "legacy destination"])(
|
||||
"refreshes read-only skill trees from a %s",
|
||||
async (scenario) => {
|
||||
const sourceWorkspace = await createCaseDir("readonly-source");
|
||||
const targetWorkspace = await createCaseDir("readonly-target");
|
||||
const outsideDir = await createCaseDir("readonly-outside");
|
||||
const sourceSkill = path.join(sourceWorkspace, ".bundled", "sealed");
|
||||
const targetSkill = path.join(targetWorkspace, "skills", "sealed");
|
||||
await writeSkill({ dir: sourceSkill, name: "sealed", description: "Sealed release skill" });
|
||||
await fs.mkdir(path.join(sourceSkill, "scripts"));
|
||||
await fs.writeFile(path.join(sourceSkill, "scripts", "run.sh"), "#!/bin/sh\necho sealed\n");
|
||||
await fs.chmod(path.join(sourceSkill, "SKILL.md"), 0o444);
|
||||
await fs.chmod(path.join(sourceSkill, "scripts", "run.sh"), 0o555);
|
||||
const directories = [sourceSkill, path.join(sourceSkill, "scripts"), outsideDir];
|
||||
try {
|
||||
for (const directory of directories) {
|
||||
await fs.chmod(directory, 0o555);
|
||||
}
|
||||
if (scenario === "legacy destination") {
|
||||
await fs.cp(sourceSkill, targetSkill, { recursive: true });
|
||||
await fs.chmod(targetSkill, 0o755);
|
||||
await fs.symlink(outsideDir, path.join(targetSkill, "outside"), "dir");
|
||||
await fs.writeFile(path.join(targetSkill, "stale.txt"), "old copy");
|
||||
await fs.chmod(targetSkill, 0o555);
|
||||
} else {
|
||||
await syncSourceSkillsToTarget(sourceWorkspace, targetWorkspace);
|
||||
bumpSkillsSnapshotVersion({ workspaceDir: sourceWorkspace });
|
||||
}
|
||||
|
||||
await syncSourceSkillsToTarget(sourceWorkspace, targetWorkspace);
|
||||
|
||||
for (const relative of ["", "scripts"]) {
|
||||
expect((await fs.stat(path.join(targetSkill, relative))).mode & 0o700).toBe(0o700);
|
||||
expect((await fs.stat(path.join(sourceSkill, relative))).mode & 0o777).toBe(0o555);
|
||||
}
|
||||
expect(await fs.readFile(path.join(targetSkill, "SKILL.md"), "utf8")).toContain(
|
||||
"Sealed release skill",
|
||||
);
|
||||
expect(await fs.readFile(path.join(targetSkill, "scripts", "run.sh"), "utf8")).toBe(
|
||||
"#!/bin/sh\necho sealed\n",
|
||||
);
|
||||
expect((await fs.stat(path.join(targetSkill, "SKILL.md"))).mode & 0o777).toBe(0o444);
|
||||
expect((await fs.stat(path.join(targetSkill, "scripts", "run.sh"))).mode & 0o777).toBe(
|
||||
0o555,
|
||||
);
|
||||
expect((await fs.stat(outsideDir)).mode & 0o777).toBe(0o555);
|
||||
expect(await pathExists(path.join(targetSkill, "stale.txt"))).toBe(false);
|
||||
expect(await pathExists(path.join(targetSkill, "outside"))).toBe(false);
|
||||
} finally {
|
||||
for (const directory of [...directories, targetSkill, path.join(targetSkill, "scripts")]) {
|
||||
await fs.chmod(directory, 0o700).catch((error: unknown) => {
|
||||
if (!hasErrnoCode(error, "ENOENT")) {
|
||||
throw error;
|
||||
}
|
||||
});
|
||||
}
|
||||
}
|
||||
},
|
||||
);
|
||||
|
||||
it("does not publish a manifest when a refreshed copy fails", async () => {
|
||||
const sourceWorkspace = await createCaseDir("source");
|
||||
const targetWorkspace = await createCaseDir("target");
|
||||
|
|
|
|||
|
|
@ -6,6 +6,7 @@ import { resolveSandboxPath } from "../../agents/sandbox-paths.js";
|
|||
import { canonicalizePath } from "../../agents/utils/paths.js";
|
||||
import type { OpenClawConfig } from "../../config/types.openclaw.js";
|
||||
import { sha256Hex } from "../../infra/crypto-digest.js";
|
||||
import { hasErrnoCode } from "../../infra/errno.js";
|
||||
import { tryReadJson, writeJson } from "../../infra/json-files.js";
|
||||
import { pruneMapToMaxSize } from "../../infra/map-size.js";
|
||||
import { createSubsystemLogger } from "../../logging/subsystem.js";
|
||||
|
|
@ -22,6 +23,7 @@ import type {
|
|||
SkillUsagePath,
|
||||
} from "../types.js";
|
||||
import { resolveSkillKey } from "./frontmatter.js";
|
||||
import { ensureWritableSkillDirectories } from "./skill-directory-modes.js";
|
||||
import { shouldSyncSkillPath } from "./skill-paths.js";
|
||||
import { resolveSkillTelemetrySource } from "./source.js";
|
||||
import { prepareWorkspaceSkills } from "./workspace-skill-loader.js";
|
||||
|
|
@ -279,7 +281,17 @@ export async function syncWorkspaceSkills(params: {
|
|||
);
|
||||
for (const child of await fsp.readdir(targetSkillsDir)) {
|
||||
if (!preservedDestinations.has(child)) {
|
||||
await fsp.rm(path.join(targetSkillsDir, child), { recursive: true, force: true });
|
||||
const childPath = path.join(targetSkillsDir, child);
|
||||
try {
|
||||
await fsp.rm(childPath, { recursive: true, force: true });
|
||||
} catch (error) {
|
||||
const permissionDenied = hasErrnoCode(error, "EACCES") || hasErrnoCode(error, "EPERM");
|
||||
if (process.platform === "win32" || !permissionDenied) {
|
||||
throw error;
|
||||
}
|
||||
await ensureWritableSkillDirectories(targetSkillsDir, child);
|
||||
await fsp.rm(childPath, { recursive: true, force: true });
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
|
|
@ -313,6 +325,7 @@ export async function syncWorkspaceSkills(params: {
|
|||
force: true,
|
||||
filter: shouldSyncSkillPath,
|
||||
});
|
||||
await ensureWritableSkillDirectories(targetSkillsDir, path.basename(destinationPath));
|
||||
}
|
||||
} catch (error) {
|
||||
if (entry.skill.source === "openclaw-library") {
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue