fix(sandbox): respect registry owner prune policy (#153752)

This commit is contained in:
Vincent Koc 2026-09-20 23:01:09 +08:00 • committed by GitHub
parent 578184cb62
commit 50eb8faa3f
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
6 changed files with 194 additions and 84 deletions

View file

@ -3,6 +3,7 @@
### Fixes
- Codex: restore background memory narratives and isolated text completions on agent-scoped local runtimes with administrator-managed hooks, preserving managed hooks and existing native-account/proxy routing while keeping ordinary hooks and model tools isolated. (#151658)
- Sandboxes: honor each registered runtime owner's pruning policy so a stricter agent cannot evict another agent's containers or browser bridges.
### Changes

View file

@ -316,7 +316,7 @@ async function resolveProvisionedSandboxContext(
resolved,
);
if (cfg.prune.idleHours !== 0 || cfg.prune.maxAgeDays !== 0) {
await (await import("./prune.js")).maybePruneSandboxes(cfg);
await (await import("./prune.js")).maybePruneSandboxes();
}
const {

View file

@ -1,8 +1,8 @@
// Sandbox prune tests cover runtime removal ordering and registry cleanup
// behavior for stale sandbox entries.
import { beforeEach, describe, expect, it, vi } from "vitest";
import type { OpenClawConfig } from "../../config/types.openclaw.js";
import type { SandboxRegistryEntry } from "./registry.js";
import type { SandboxConfig } from "./types.js";
let maybePruneSandboxes: typeof import("./prune.js").maybePruneSandboxes;
let BROWSER_BRIDGES: typeof import("./browser-bridges.js").BROWSER_BRIDGES;
@ -17,6 +17,7 @@ const backendMocks = vi.hoisted(() => ({
}));
const registryMocks = vi.hoisted(() => ({
assertSandboxBrowserRegistryEntryCurrent: vi.fn(),
readBrowserRegistry: vi.fn(),
readRegistry: vi.fn(),
removeBrowserRegistryEntry: vi.fn(),
@ -49,68 +50,49 @@ vi.mock("./docker-backend.js", () => ({
}));
vi.mock("./registry.js", () => ({
assertSandboxBrowserRegistryEntryCurrent: registryMocks.assertSandboxBrowserRegistryEntryCurrent,
readBrowserRegistry: registryMocks.readBrowserRegistry,
readRegistry: registryMocks.readRegistry,
removeBrowserRegistryEntry: registryMocks.removeBrowserRegistryEntry,
removeRegistryEntry: registryMocks.removeRegistryEntry,
removeSandboxRegistryGeneration: (
_kind: string,
entry: SandboxRegistryEntry,
assertCurrent: () => void,
) => {
assertCurrent();
return registryMocks.removeBrowserRegistryEntry(entry.containerName);
},
removeSandboxRegistryRuntime: async (
entry: SandboxRegistryEntry,
removeRuntime: (current: SandboxRegistryEntry) => Promise<void>,
options?: { shouldRemove?: (current: SandboxRegistryEntry) => boolean },
) => {
if (options?.shouldRemove && !options.shouldRemove(entry)) {
return;
}
await removeRuntime(entry);
await registryMocks.removeRegistryEntry(entry.containerName);
},
withSandboxRegistryEntryLock: async (
_entry: SandboxRegistryEntry,
operation: () => Promise<unknown>,
) => operation(),
}));
vi.mock("../../plugin-sdk/browser-bridge.js", () => ({
stopBrowserBridgeServer: bridgeMocks.stopBrowserBridgeServer,
}));
function buildPruneConfig(): SandboxConfig {
function buildPruneConfig(): OpenClawConfig {
return {
mode: "all",
backend: "docker",
scope: "session",
workspaceAccess: "none",
workspaceRoot: "/tmp/openclaw-sandboxes",
dockerTmpfsSource: "configured",
docker: {
image: "openclaw-sandbox:bookworm-slim",
containerPrefix: "openclaw-sbx-",
workdir: "/workspace",
readOnlyRoot: true,
tmpfs: [],
network: "none",
capDrop: ["ALL"],
env: {},
},
ssh: {
command: "ssh",
workspaceRoot: "/tmp/openclaw-sandboxes",
strictHostKeyChecking: true,
updateHostKeys: true,
},
browser: {
enabled: true,
image: "openclaw-sandbox-browser:bookworm-slim",
containerPrefix: "openclaw-sbx-browser-",
network: "none",
cdpPort: 9222,
vncPort: 5900,
noVncPort: 6080,
headless: true,
noVncEnabled: false,
allowHostControl: false,
autoStart: true,
autoStartTimeoutMs: 1_000,
},
tools: {
allow: [],
deny: [],
},
prune: {
idleHours: 1,
maxAgeDays: 0,
agents: {
defaults: {
sandbox: {
mode: "all",
prune: { idleHours: 1, maxAgeDays: 0 },
},
},
},
};
}
@ -119,6 +101,7 @@ describe("maybePruneSandboxes", () => {
beforeEach(async () => {
vi.resetModules();
configMocks.getRuntimeConfig.mockReset();
registryMocks.assertSandboxBrowserRegistryEntryCurrent.mockReset();
backendMocks.getSandboxBackendManager.mockReset().mockReturnValue(backendMocks);
backendMocks.removeRuntime.mockReset();
registryMocks.readBrowserRegistry.mockReset();
@ -135,6 +118,7 @@ describe("maybePruneSandboxes", () => {
{
containerName: "sandbox-1",
backendId: "docker",
sessionKey: "agent:main:main",
createdAtMs: Date.now() - 4 * 60 * 60 * 1000,
lastUsedAtMs: Date.now() - 2 * 60 * 60 * 1000,
image: "openclaw-sandbox:bookworm-slim",
@ -154,6 +138,82 @@ describe("maybePruneSandboxes", () => {
expect(registryMocks.removeRegistryEntry).toHaveBeenCalledWith("sandbox-1");
});
it("uses each registry owner's prune policy for containers and browsers", async () => {
const now = Date.now();
vi.spyOn(Date, "now").mockReturnValue(now);
configMocks.getRuntimeConfig.mockReturnValue({
agents: {
defaults: {
sandbox: { mode: "all", prune: { idleHours: 1, maxAgeDays: 0 } },
},
entries: {
work: { sandbox: { prune: { idleHours: 24, maxAgeDays: 0 } } },
},
},
});
const entry = (containerName: string, agentId: string) => ({
containerName,
backendId: "docker",
sessionKey: `agent:${agentId}:main`,
createdAtMs: now - 4 * 60 * 60 * 1000,
lastUsedAtMs: now - 2 * 60 * 60 * 1000,
image: "openclaw-sandbox:bookworm-slim",
});
registryMocks.readRegistry.mockResolvedValue({
entries: [entry("main-container", "main"), entry("work-container", "work")],
});
registryMocks.readBrowserRegistry.mockResolvedValue({
entries: [
{ ...entry("main-browser", "main"), cdpPort: 9222 },
{ ...entry("work-browser", "work"), cdpPort: 9223 },
],
});
await maybePruneSandboxes();
expect(
backendMocks.removeRuntime.mock.calls.map(([params]) => params.entry.containerName),
).toEqual(["main-container", "main-browser"]);
expect(backendMocks.removeRuntime.mock.calls.map(([params]) => params.agentId)).toEqual([
"main",
"main",
]);
expect(registryMocks.removeRegistryEntry).toHaveBeenCalledExactlyOnceWith("main-container");
expect(registryMocks.removeBrowserRegistryEntry).toHaveBeenCalledExactlyOnceWith(
"main-browser",
);
});
it("uses global prune policy for shared runtimes despite the caller override", async () => {
const now = Date.now();
vi.spyOn(Date, "now").mockReturnValue(now);
configMocks.getRuntimeConfig.mockReturnValue({
agents: {
defaults: { sandbox: { mode: "all", prune: { idleHours: 24, maxAgeDays: 0 } } },
entries: {
main: { sandbox: { prune: { idleHours: 1, maxAgeDays: 0 } } },
},
},
});
registryMocks.readRegistry.mockResolvedValue({
entries: [
{
containerName: "shared-runtime",
backendId: "docker",
sessionKey: "shared",
createdAtMs: now - 4 * 60 * 60 * 1000,
lastUsedAtMs: now - 2 * 60 * 60 * 1000,
image: "openclaw-sandbox:bookworm-slim",
},
],
});
registryMocks.readBrowserRegistry.mockResolvedValue({ entries: [] });
await maybePruneSandboxes();
expect(backendMocks.removeRuntime).not.toHaveBeenCalled();
});
it("keeps the registry entry when runtime removal fails", async () => {
// The registry is the retry source; keep it until the backend confirms the
// runtime was removed.
@ -174,6 +234,7 @@ describe("maybePruneSandboxes", () => {
{
containerName: "openshell-1",
backendId: "openshell",
sessionKey: "agent:main:main",
createdAtMs: Date.now() - 4 * 60 * 60 * 1000,
lastUsedAtMs: Date.now() - 2 * 60 * 60 * 1000,
image: "openclaw",
@ -195,6 +256,7 @@ describe("maybePruneSandboxes", () => {
{
containerName: "sandbox-out-of-range",
backendId: "docker",
sessionKey: "agent:main:main",
createdAtMs: Date.now(),
lastUsedAtMs: Number.MAX_SAFE_INTEGER,
image: "openclaw-sandbox:bookworm-slim",

View file

@ -5,30 +5,43 @@ import { asDateTimestampMs } from "@openclaw/normalization-core/number-coercion"
* Removes stale runtime containers and browser bridges on a best-effort schedule.
*/
import { getRuntimeConfig } from "../../config/config.js";
import type { OpenClawConfig } from "../../config/types.openclaw.js";
import { defaultRuntime } from "../../runtime.js";
import { getSandboxBackendManager, usesSandboxRuntimeReservations } from "./backend.js";
import { stopCachedBrowserBridgesForContainer } from "./browser-bridges.js";
import { resolveSandboxConfigForAgent } from "./config.js";
import { dockerSandboxBackendManager } from "./docker-backend.js";
import {
assertSandboxBrowserRegistryEntryCurrent,
readBrowserRegistry,
readRegistry,
removeBrowserRegistryEntry,
removeSandboxRegistryGeneration,
removeSandboxRegistryRuntime,
withSandboxRegistryEntryLock,
type SandboxBrowserRegistryEntry,
type SandboxRegistryEntry,
} from "./registry.js";
import type { SandboxConfig } from "./types.js";
import { resolveSandboxAgentId } from "./shared.js";
import type { SandboxPruneConfig } from "./types.js";
let lastPruneAtMs = 0;
type PruneableRegistryEntry = Pick<
SandboxRegistryEntry,
"containerName" | "backendId" | "createdAtMs" | "lastUsedAtMs"
"containerName" | "backendId" | "createdAtMs" | "lastUsedAtMs" | "sessionKey"
>;
function shouldPruneSandboxEntry(cfg: SandboxConfig, now: number, entry: PruneableRegistryEntry) {
const idleHours = cfg.prune.idleHours;
const maxAgeDays = cfg.prune.maxAgeDays;
function resolveEntryPruneConfig(config: OpenClawConfig, entry: PruneableRegistryEntry) {
return resolveSandboxConfigForAgent(config, resolveSandboxAgentId(entry.sessionKey)).prune;
}
function shouldPruneSandboxEntry(
prune: SandboxPruneConfig,
now: number,
entry: PruneableRegistryEntry,
) {
const idleHours = prune.idleHours;
const maxAgeDays = prune.maxAgeDays;
if (idleHours === 0 && maxAgeDays === 0) {
return false;
}
@ -45,7 +58,7 @@ function shouldPruneSandboxEntry(cfg: SandboxConfig, now: number, entry: Pruneab
/** Removes expired registry entries and their backing runtime resources. */
async function pruneSandboxRegistryEntries<TEntry extends SandboxRegistryEntry>(params: {
cfg: SandboxConfig;
config: OpenClawConfig;
read: () => Promise<{ entries: TEntry[] }>;
remove: (
entry: TEntry,
@ -53,16 +66,15 @@ async function pruneSandboxRegistryEntries<TEntry extends SandboxRegistryEntry>(
) => Promise<void>;
}) {
const now = Date.now();
if (params.cfg.prune.idleHours === 0 && params.cfg.prune.maxAgeDays === 0) {
return;
}
const registry = await params.read();
for (const entry of registry.entries) {
if (!shouldPruneSandboxEntry(params.cfg, now, entry)) {
if (!shouldPruneSandboxEntry(resolveEntryPruneConfig(params.config, entry), now, entry)) {
continue;
}
try {
await params.remove(entry, (current) => shouldPruneSandboxEntry(params.cfg, now, current));
await params.remove(entry, (current) =>
shouldPruneSandboxEntry(resolveEntryPruneConfig(params.config, current), now, current),
);
} catch (error) {
const message =
error instanceof Error
@ -78,10 +90,9 @@ async function pruneSandboxRegistryEntries<TEntry extends SandboxRegistryEntry>(
}
/** Prunes ordinary sandbox runtime containers from the configured backend manager. */
async function pruneSandboxContainers(cfg: SandboxConfig) {
const config = getRuntimeConfig();
async function pruneSandboxContainers(config: OpenClawConfig) {
await pruneSandboxRegistryEntries<SandboxRegistryEntry>({
cfg,
config,
read: readRegistry,
remove: (entry, shouldRemove) =>
removeSandboxRegistryRuntime(
@ -94,7 +105,11 @@ async function pruneSandboxContainers(cfg: SandboxConfig) {
`Sandbox backend "${backendId}" is unavailable; enable its plugin before removing this runtime.`,
);
}
await manager.removeRuntime({ entry: current, config });
await manager.removeRuntime({
entry: current,
config,
agentId: resolveSandboxAgentId(current.sessionKey),
});
},
{
reserveRuntime: usesSandboxRuntimeReservations(entry.backendId ?? "docker"),
@ -105,8 +120,7 @@ async function pruneSandboxContainers(cfg: SandboxConfig) {
}
/** Prunes browser bridge containers and closes matching in-process bridge servers. */
async function pruneSandboxBrowsers(cfg: SandboxConfig) {
const config = getRuntimeConfig();
async function pruneSandboxBrowsers(config: OpenClawConfig) {
await pruneSandboxRegistryEntries<
SandboxBrowserRegistryEntry & {
backendId?: string;
@ -114,34 +128,51 @@ async function pruneSandboxBrowsers(cfg: SandboxConfig) {
configLabelKind?: string;
}
>({
cfg,
config,
read: readBrowserRegistry,
remove: async (entry) => {
await stopCachedBrowserBridgesForContainer(entry.containerName);
await dockerSandboxBackendManager.removeRuntime({
entry: {
...entry,
backendId: "docker",
runtimeLabel: entry.containerName,
configLabelKind: "Image",
},
config,
remove: async (entry, shouldRemove) => {
await withSandboxRegistryEntryLock({ ...entry, backendId: "docker" }, async () => {
const current = (await readBrowserRegistry()).entries.find(
(candidate) => candidate.containerName === entry.containerName,
);
if (!current || !shouldRemove(current)) {
return;
}
try {
assertSandboxBrowserRegistryEntryCurrent(entry);
} catch {
return;
}
await stopCachedBrowserBridgesForContainer(current.containerName);
await dockerSandboxBackendManager.removeRuntime({
entry: {
...current,
backendId: "docker",
runtimeLabel: current.containerName,
configLabelKind: "Image",
},
config,
agentId: resolveSandboxAgentId(current.sessionKey),
});
removeSandboxRegistryGeneration("browser", current, () =>
assertSandboxBrowserRegistryEntryCurrent(current),
);
});
await removeBrowserRegistryEntry(entry.containerName);
},
});
}
/** Runs sandbox pruning at most once per throttle window. */
export async function maybePruneSandboxes(cfg: SandboxConfig) {
export async function maybePruneSandboxes(config?: OpenClawConfig) {
const now = Date.now();
if (now - lastPruneAtMs < 5 * 60 * 1000) {
return;
}
lastPruneAtMs = now;
try {
await pruneSandboxContainers(cfg);
await pruneSandboxBrowsers(cfg);
const currentConfig = config ?? getRuntimeConfig();
await pruneSandboxContainers(currentConfig);
await pruneSandboxBrowsers(currentConfig);
} catch (error) {
const message =
error instanceof Error

View file

@ -383,7 +383,8 @@ export async function removeSandboxRegistryRuntime(
!current ||
(current.runtimeState !== "removing" && current.runtimeState !== "removing-pending") ||
current.backendId !== removing.backendId ||
current.sessionKey !== removing.sessionKey
current.sessionKey !== removing.sessionKey ||
(options.shouldRemove && !options.shouldRemove(current))
) {
return;
}

View file

@ -44,6 +44,22 @@ function advancePruneTime() {
vi.spyOn(Date, "now").mockReturnValue(pruneTimeMs);
}
function withImmediatePrune(cfg: OpenClawConfig): OpenClawConfig {
return {
...cfg,
agents: {
...cfg.agents,
defaults: {
...cfg.agents?.defaults,
sandbox: {
...cfg.agents?.defaults?.sandbox,
prune: { idleHours: 1, maxAgeDays: 0 },
},
},
},
};
}
beforeEach(() => {
const stateDir = tempDirs.make("sandbox-reservation-");
vi.stubEnv("OPENCLAW_STATE_DIR", stateDir);
@ -269,7 +285,7 @@ describe("durable sandbox runtime generations", () => {
it.each(["recreate", "prune"] as const)(
"fences a legacy %s snapshot after concurrent adoption",
async (operation) => {
const cfg = await seedLegacyRuntime();
await seedLegacyRuntime();
const snapshot = await readRegistry();
const started = createDeferred();
const finish = createDeferred();
@ -290,7 +306,7 @@ describe("durable sandbox runtime generations", () => {
const removing =
operation === "recreate"
? removeSandboxContainer("legacy-runtime")
: maybePruneSandboxes({ ...cfg, prune: { idleHours: 1, maxAgeDays: 0 } });
: maybePruneSandboxes(withImmediatePrune(config));
const creating = expect(resolve()).rejects.toThrow("removed or is being removed");
await started.promise;
try {
@ -419,8 +435,7 @@ describe("durable sandbox runtime generations", () => {
let removing: Promise<void>;
if (operation === "prune") {
advancePruneTime();
const cfg = resolveSandboxConfigForAgent(config, "test");
removing = maybePruneSandboxes({ ...cfg, prune: { idleHours: 1, maxAgeDays: 0 } });
removing = maybePruneSandboxes(withImmediatePrune(config));
} else {
removing = removeSandboxContainer(id);
}