mirror of https://github.com/openclaw/openclaw.git
fix(security): restrict leaf subagent control scope
This commit is contained in:
parent
3ba6491659
commit
ecdbd8aa52
|
|
@ -138,6 +138,7 @@ Docs: https://docs.openclaw.ai
|
|||
- Docs/Changelog: correct the contributor credit for the bundled Control UI global-install fix to @LarytheLord. (#40420) Thanks @velvet-shark.
|
||||
- Telegram/media downloads: time out only stalled body reads so polling recovers from hung file downloads without aborting slow downloads that are still streaming data. (#40098) thanks @tysoncung.
|
||||
- Docker/runtime image: prune dev dependencies, strip build-only dist metadata for smaller Docker images. (#40307) Thanks @vincentkoc.
|
||||
- Subagents/sandboxing: restrict leaf subagents to their own spawned runs and remove leaf `subagents` control access so sandboxed leaf workers can no longer steer sibling sessions. Thanks @tdjackey.
|
||||
- Gateway/restart timeout recovery: exit non-zero when restart-triggered shutdown drains time out so launchd/systemd restart the gateway instead of treating the failed restart as a clean stop. Landed from contributor PR #40380 by @dsantoreis. Thanks @dsantoreis.
|
||||
- Gateway/config restart guard: validate config before service start/restart and keep post-SIGUSR1 startup failures from crashing the gateway process, reducing invalid-config restart loops and macOS permission loss. Landed from contributor PR #38699 by @lml2468. Thanks @lml2468.
|
||||
- Gateway/launchd respawn detection: treat `XPC_SERVICE_NAME` as a launchd supervision hint so macOS restarts exit cleanly under launchd instead of attempting detached self-respawn. Landed from contributor PR #20555 by @dimat. Thanks @dimat.
|
||||
|
|
|
|||
|
|
@ -0,0 +1,152 @@
|
|||
import fs from "node:fs";
|
||||
import os from "node:os";
|
||||
import path from "node:path";
|
||||
import { beforeEach, describe, expect, it } from "vitest";
|
||||
import {
|
||||
callGatewayMock,
|
||||
resetSubagentsConfigOverride,
|
||||
setSubagentsConfigOverride,
|
||||
} from "./openclaw-tools.subagents.test-harness.js";
|
||||
import { addSubagentRunForTests, resetSubagentRegistryForTests } from "./subagent-registry.js";
|
||||
import "./test-helpers/fast-core-tools.js";
|
||||
import { createPerSenderSessionConfig } from "./test-helpers/session-config.js";
|
||||
import { createSubagentsTool } from "./tools/subagents-tool.js";
|
||||
|
||||
function writeStore(storePath: string, store: Record<string, unknown>) {
|
||||
fs.mkdirSync(path.dirname(storePath), { recursive: true });
|
||||
fs.writeFileSync(storePath, JSON.stringify(store, null, 2), "utf-8");
|
||||
}
|
||||
|
||||
describe("openclaw-tools: subagents scope isolation", () => {
|
||||
let storePath = "";
|
||||
|
||||
beforeEach(() => {
|
||||
resetSubagentRegistryForTests();
|
||||
resetSubagentsConfigOverride();
|
||||
callGatewayMock.mockReset();
|
||||
storePath = path.join(
|
||||
os.tmpdir(),
|
||||
`openclaw-subagents-scope-${Date.now()}-${Math.random().toString(16).slice(2)}.json`,
|
||||
);
|
||||
setSubagentsConfigOverride({
|
||||
session: createPerSenderSessionConfig({ store: storePath }),
|
||||
});
|
||||
writeStore(storePath, {});
|
||||
});
|
||||
|
||||
it("leaf subagents do not inherit parent sibling control scope", async () => {
|
||||
const leafKey = "agent:main:subagent:leaf";
|
||||
const siblingKey = "agent:main:subagent:unsandboxed";
|
||||
|
||||
writeStore(storePath, {
|
||||
[leafKey]: {
|
||||
sessionId: "leaf-session",
|
||||
updatedAt: Date.now(),
|
||||
spawnedBy: "agent:main:main",
|
||||
},
|
||||
[siblingKey]: {
|
||||
sessionId: "sibling-session",
|
||||
updatedAt: Date.now(),
|
||||
spawnedBy: "agent:main:main",
|
||||
},
|
||||
});
|
||||
|
||||
addSubagentRunForTests({
|
||||
runId: "run-leaf",
|
||||
childSessionKey: leafKey,
|
||||
requesterSessionKey: "agent:main:main",
|
||||
requesterDisplayKey: "main",
|
||||
task: "sandboxed leaf",
|
||||
cleanup: "keep",
|
||||
createdAt: Date.now() - 30_000,
|
||||
startedAt: Date.now() - 30_000,
|
||||
});
|
||||
addSubagentRunForTests({
|
||||
runId: "run-sibling",
|
||||
childSessionKey: siblingKey,
|
||||
requesterSessionKey: "agent:main:main",
|
||||
requesterDisplayKey: "main",
|
||||
task: "unsandboxed sibling",
|
||||
cleanup: "keep",
|
||||
createdAt: Date.now() - 20_000,
|
||||
startedAt: Date.now() - 20_000,
|
||||
});
|
||||
|
||||
const tool = createSubagentsTool({ agentSessionKey: leafKey });
|
||||
const result = await tool.execute("call-leaf-list", { action: "list" });
|
||||
|
||||
expect(result.details).toMatchObject({
|
||||
status: "ok",
|
||||
requesterSessionKey: leafKey,
|
||||
callerSessionKey: leafKey,
|
||||
callerIsSubagent: true,
|
||||
total: 0,
|
||||
active: [],
|
||||
recent: [],
|
||||
});
|
||||
expect(callGatewayMock).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("orchestrator subagents still see children they spawned", async () => {
|
||||
const orchestratorKey = "agent:main:subagent:orchestrator";
|
||||
const workerKey = `${orchestratorKey}:subagent:worker`;
|
||||
const siblingKey = "agent:main:subagent:sibling";
|
||||
|
||||
writeStore(storePath, {
|
||||
[orchestratorKey]: {
|
||||
sessionId: "orchestrator-session",
|
||||
updatedAt: Date.now(),
|
||||
spawnedBy: "agent:main:main",
|
||||
},
|
||||
[workerKey]: {
|
||||
sessionId: "worker-session",
|
||||
updatedAt: Date.now(),
|
||||
spawnedBy: orchestratorKey,
|
||||
},
|
||||
[siblingKey]: {
|
||||
sessionId: "sibling-session",
|
||||
updatedAt: Date.now(),
|
||||
spawnedBy: "agent:main:main",
|
||||
},
|
||||
});
|
||||
|
||||
addSubagentRunForTests({
|
||||
runId: "run-worker",
|
||||
childSessionKey: workerKey,
|
||||
requesterSessionKey: orchestratorKey,
|
||||
requesterDisplayKey: orchestratorKey,
|
||||
task: "worker child",
|
||||
cleanup: "keep",
|
||||
createdAt: Date.now() - 30_000,
|
||||
startedAt: Date.now() - 30_000,
|
||||
});
|
||||
addSubagentRunForTests({
|
||||
runId: "run-sibling",
|
||||
childSessionKey: siblingKey,
|
||||
requesterSessionKey: "agent:main:main",
|
||||
requesterDisplayKey: "main",
|
||||
task: "sibling of orchestrator",
|
||||
cleanup: "keep",
|
||||
createdAt: Date.now() - 20_000,
|
||||
startedAt: Date.now() - 20_000,
|
||||
});
|
||||
|
||||
const tool = createSubagentsTool({ agentSessionKey: orchestratorKey });
|
||||
const result = await tool.execute("call-orchestrator-list", { action: "list" });
|
||||
const details = result.details as {
|
||||
status?: string;
|
||||
requesterSessionKey?: string;
|
||||
total?: number;
|
||||
active?: Array<{ sessionKey?: string }>;
|
||||
};
|
||||
|
||||
expect(details.status).toBe("ok");
|
||||
expect(details.requesterSessionKey).toBe(orchestratorKey);
|
||||
expect(details.total).toBe(1);
|
||||
expect(details.active).toEqual([
|
||||
expect.objectContaining({
|
||||
sessionKey: workerKey,
|
||||
}),
|
||||
]);
|
||||
});
|
||||
});
|
||||
|
|
@ -6,6 +6,11 @@ import { addSubagentRunForTests, resetSubagentRegistryForTests } from "./subagen
|
|||
import { createPerSenderSessionConfig } from "./test-helpers/session-config.js";
|
||||
import { createSessionsSpawnTool } from "./tools/sessions-spawn-tool.js";
|
||||
|
||||
vi.mock("@mariozechner/pi-ai/oauth", () => ({
|
||||
getOAuthApiKey: () => undefined,
|
||||
getOAuthProviders: () => [],
|
||||
}));
|
||||
|
||||
const callGatewayMock = vi.fn();
|
||||
|
||||
vi.mock("../gateway/call.js", () => ({
|
||||
|
|
|
|||
|
|
@ -5,6 +5,11 @@ export type LoadedConfig = ReturnType<(typeof import("../config/config.js"))["lo
|
|||
|
||||
export const callGatewayMock: MockFn = vi.fn();
|
||||
|
||||
vi.mock("@mariozechner/pi-ai/oauth", () => ({
|
||||
getOAuthApiKey: () => undefined,
|
||||
getOAuthProviders: () => [],
|
||||
}));
|
||||
|
||||
const defaultConfig: LoadedConfig = {
|
||||
session: {
|
||||
mainKey: "main",
|
||||
|
|
|
|||
|
|
@ -144,9 +144,9 @@ describe("resolveSubagentToolPolicy depth awareness", () => {
|
|||
expect(isToolAllowedByPolicyName("sessions_spawn", policy)).toBe(false);
|
||||
});
|
||||
|
||||
it("depth-2 leaf allows subagents (for visibility)", () => {
|
||||
it("depth-2 leaf denies subagents", () => {
|
||||
const policy = resolveSubagentToolPolicy(baseCfg, 2);
|
||||
expect(isToolAllowedByPolicyName("subagents", policy)).toBe(true);
|
||||
expect(isToolAllowedByPolicyName("subagents", policy)).toBe(false);
|
||||
});
|
||||
|
||||
it("depth-2 leaf denies sessions_list and sessions_history", () => {
|
||||
|
|
|
|||
|
|
@ -64,15 +64,20 @@ const SUBAGENT_TOOL_DENY_ALWAYS = [
|
|||
* Additional tools denied for leaf sub-agents (depth >= maxSpawnDepth).
|
||||
* These are tools that only make sense for orchestrator sub-agents that can spawn children.
|
||||
*/
|
||||
const SUBAGENT_TOOL_DENY_LEAF = ["sessions_list", "sessions_history", "sessions_spawn"];
|
||||
const SUBAGENT_TOOL_DENY_LEAF = [
|
||||
"subagents",
|
||||
"sessions_list",
|
||||
"sessions_history",
|
||||
"sessions_spawn",
|
||||
];
|
||||
|
||||
/**
|
||||
* Build the deny list for a sub-agent at a given depth.
|
||||
*
|
||||
* - Depth 1 with maxSpawnDepth >= 2 (orchestrator): allowed to use sessions_spawn,
|
||||
* subagents, sessions_list, sessions_history so it can manage its children.
|
||||
* - Depth >= maxSpawnDepth (leaf): denied sessions_spawn and
|
||||
* session management tools. Still allowed subagents (for list/status visibility).
|
||||
* - Depth >= maxSpawnDepth (leaf): denied subagents, sessions_spawn, and
|
||||
* session management tools.
|
||||
*/
|
||||
function resolveSubagentDenyList(depth: number, maxSpawnDepth: number): string[] {
|
||||
const isLeaf = depth >= Math.max(1, Math.floor(maxSpawnDepth));
|
||||
|
|
|
|||
|
|
@ -7,7 +7,6 @@ import {
|
|||
sortSubagentRuns,
|
||||
type SubagentTargetResolution,
|
||||
} from "../../auto-reply/reply/subagents-utils.js";
|
||||
import { DEFAULT_SUBAGENT_MAX_SPAWN_DEPTH } from "../../config/agent-limits.js";
|
||||
import { loadConfig } from "../../config/config.js";
|
||||
import type { SessionEntry } from "../../config/sessions.js";
|
||||
import { loadSessionStore, resolveStorePath, updateSessionStore } from "../../config/sessions.js";
|
||||
|
|
@ -28,7 +27,6 @@ import { INTERNAL_MESSAGE_CHANNEL } from "../../utils/message-channel.js";
|
|||
import { AGENT_LANE_SUBAGENT } from "../lanes.js";
|
||||
import { abortEmbeddedPiRun } from "../pi-embedded.js";
|
||||
import { optionalStringEnum } from "../schema/typebox.js";
|
||||
import { getSubagentDepthFromSessionStore } from "../subagent-depth.js";
|
||||
import {
|
||||
clearSubagentRunSteerRestart,
|
||||
countPendingDescendantRuns,
|
||||
|
|
@ -204,36 +202,28 @@ function resolveRequesterKey(params: {
|
|||
};
|
||||
}
|
||||
|
||||
// Check if this sub-agent can spawn children (orchestrator).
|
||||
// If so, it should see its own children, not its parent's children.
|
||||
const callerDepth = getSubagentDepthFromSessionStore(callerSessionKey, { cfg: params.cfg });
|
||||
const maxSpawnDepth =
|
||||
params.cfg.agents?.defaults?.subagents?.maxSpawnDepth ?? DEFAULT_SUBAGENT_MAX_SPAWN_DEPTH;
|
||||
if (callerDepth < maxSpawnDepth) {
|
||||
// Orchestrator sub-agent: use its own session key as requester
|
||||
// so it sees children it spawned.
|
||||
return {
|
||||
requesterSessionKey: callerSessionKey,
|
||||
callerSessionKey,
|
||||
callerIsSubagent: true,
|
||||
};
|
||||
}
|
||||
|
||||
// Leaf sub-agent: walk up to its parent so it can see sibling runs.
|
||||
const cache = new Map<string, Record<string, SessionEntry>>();
|
||||
const callerEntry = resolveSessionEntryForKey({
|
||||
cfg: params.cfg,
|
||||
key: callerSessionKey,
|
||||
cache,
|
||||
}).entry;
|
||||
const spawnedBy = typeof callerEntry?.spawnedBy === "string" ? callerEntry.spawnedBy.trim() : "";
|
||||
return {
|
||||
requesterSessionKey: spawnedBy || callerSessionKey,
|
||||
// Subagents can only control runs spawned from their own session key.
|
||||
// Announce routing still uses SubagentRunRecord.requesterSessionKey elsewhere.
|
||||
requesterSessionKey: callerSessionKey,
|
||||
callerSessionKey,
|
||||
callerIsSubagent: true,
|
||||
};
|
||||
}
|
||||
|
||||
function ensureSubagentControlsOwnDescendants(params: {
|
||||
requester: ResolvedRequesterKey;
|
||||
entry: SubagentRunRecord;
|
||||
}) {
|
||||
if (!params.requester.callerIsSubagent) {
|
||||
return undefined;
|
||||
}
|
||||
if (params.entry.requesterSessionKey === params.requester.callerSessionKey) {
|
||||
return undefined;
|
||||
}
|
||||
return "Subagents can only control runs spawned from their own session.";
|
||||
}
|
||||
|
||||
async function killSubagentRun(params: {
|
||||
cfg: ReturnType<typeof loadConfig>;
|
||||
entry: SubagentRunRecord;
|
||||
|
|
@ -499,6 +489,20 @@ export function createSubagentsTool(opts?: { agentSessionKey?: string }): AnyAge
|
|||
error: resolved.error ?? "Unknown subagent target.",
|
||||
});
|
||||
}
|
||||
const ownershipError = ensureSubagentControlsOwnDescendants({
|
||||
requester,
|
||||
entry: resolved.entry,
|
||||
});
|
||||
if (ownershipError) {
|
||||
return jsonResult({
|
||||
status: "forbidden",
|
||||
action: "kill",
|
||||
target,
|
||||
runId: resolved.entry.runId,
|
||||
sessionKey: resolved.entry.childSessionKey,
|
||||
error: ownershipError,
|
||||
});
|
||||
}
|
||||
const killCache = new Map<string, Record<string, SessionEntry>>();
|
||||
const stopResult = await killSubagentRun({
|
||||
cfg,
|
||||
|
|
@ -568,6 +572,20 @@ export function createSubagentsTool(opts?: { agentSessionKey?: string }): AnyAge
|
|||
error: resolved.error ?? "Unknown subagent target.",
|
||||
});
|
||||
}
|
||||
const ownershipError = ensureSubagentControlsOwnDescendants({
|
||||
requester,
|
||||
entry: resolved.entry,
|
||||
});
|
||||
if (ownershipError) {
|
||||
return jsonResult({
|
||||
status: "forbidden",
|
||||
action: "steer",
|
||||
target,
|
||||
runId: resolved.entry.runId,
|
||||
sessionKey: resolved.entry.childSessionKey,
|
||||
error: ownershipError,
|
||||
});
|
||||
}
|
||||
if (resolved.entry.endedAt) {
|
||||
return jsonResult({
|
||||
status: "done",
|
||||
|
|
|
|||
Loading…
Reference in New Issue