fix(agents): add strict format validation to sessions_spawn for agentId
Implements a strict format validation for the agentId parameter in sessions_spawn to fully resolve the ghost workspace creation bug reported in #31311. This fix introduces a regex format gate at the entry point to immediately reject malformed agentId strings. This prevents error messages (e.g., 'Agent not found: xyz') or path traversals from being mangled by normalizeAgentId into seemingly valid IDs (e.g., 'agent-not-found--xyz'), which was the root cause of the bug. The validation is placed before normalization and does not interfere with existing workflows, including delegating to agents that are allowlisted but not globally configured. New, non-redundant tests are added to sessions-spawn.allowlist.test.ts to cover format validation and ensure no regressions in allowlist behavior. Fixes #31311
This commit is contained in:
@@ -223,4 +223,124 @@ describe("openclaw-tools: subagents (sessions_spawn allowlist)", () => {
|
||||
expect(details.error).toContain('sandbox="require"');
|
||||
expect(callGatewayMock).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
// ---------------------------------------------------------------------------
|
||||
// agentId format validation (#31311)
|
||||
// ---------------------------------------------------------------------------
|
||||
|
||||
it("rejects error-message-like strings as agentId (#31311)", async () => {
|
||||
setSessionsSpawnConfigOverride({
|
||||
session: { mainKey: "main", scope: "per-sender" },
|
||||
agents: {
|
||||
list: [{ id: "main", subagents: { allowAgents: ["*"] } }, { id: "research" }],
|
||||
},
|
||||
});
|
||||
const tool = await getSessionsSpawnTool({
|
||||
agentSessionKey: "main",
|
||||
agentChannel: "whatsapp",
|
||||
});
|
||||
const result = await tool.execute("call-err-msg", {
|
||||
task: "do thing",
|
||||
agentId: "Agent not found: xyz",
|
||||
});
|
||||
const details = result.details as { status?: string; error?: string };
|
||||
expect(details.status).toBe("error");
|
||||
expect(details.error).toContain("Invalid agentId");
|
||||
expect(details.error).toContain("agents_list");
|
||||
expect(callGatewayMock).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("rejects agentId containing path separators (#31311)", async () => {
|
||||
setSessionsSpawnConfigOverride({
|
||||
session: { mainKey: "main", scope: "per-sender" },
|
||||
agents: {
|
||||
list: [{ id: "main", subagents: { allowAgents: ["*"] } }],
|
||||
},
|
||||
});
|
||||
const tool = await getSessionsSpawnTool({
|
||||
agentSessionKey: "main",
|
||||
agentChannel: "whatsapp",
|
||||
});
|
||||
const result = await tool.execute("call-path", {
|
||||
task: "do thing",
|
||||
agentId: "../../../etc/passwd",
|
||||
});
|
||||
const details = result.details as { status?: string; error?: string };
|
||||
expect(details.status).toBe("error");
|
||||
expect(details.error).toContain("Invalid agentId");
|
||||
expect(callGatewayMock).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("rejects agentId exceeding 64 characters (#31311)", async () => {
|
||||
setSessionsSpawnConfigOverride({
|
||||
session: { mainKey: "main", scope: "per-sender" },
|
||||
agents: {
|
||||
list: [{ id: "main", subagents: { allowAgents: ["*"] } }],
|
||||
},
|
||||
});
|
||||
const tool = await getSessionsSpawnTool({
|
||||
agentSessionKey: "main",
|
||||
agentChannel: "whatsapp",
|
||||
});
|
||||
const result = await tool.execute("call-long", {
|
||||
task: "do thing",
|
||||
agentId: "a".repeat(65),
|
||||
});
|
||||
const details = result.details as { status?: string; error?: string };
|
||||
expect(details.status).toBe("error");
|
||||
expect(details.error).toContain("Invalid agentId");
|
||||
expect(callGatewayMock).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("accepts well-formed agentId with hyphens and underscores (#31311)", async () => {
|
||||
setSessionsSpawnConfigOverride({
|
||||
session: { mainKey: "main", scope: "per-sender" },
|
||||
agents: {
|
||||
list: [{ id: "main", subagents: { allowAgents: ["*"] } }, { id: "my-research_agent01" }],
|
||||
},
|
||||
});
|
||||
callGatewayMock.mockImplementation(async () => ({
|
||||
runId: "run-1",
|
||||
status: "accepted",
|
||||
acceptedAt: 1000,
|
||||
}));
|
||||
const tool = await getSessionsSpawnTool({
|
||||
agentSessionKey: "main",
|
||||
agentChannel: "whatsapp",
|
||||
});
|
||||
const result = await tool.execute("call-valid", {
|
||||
task: "do thing",
|
||||
agentId: "my-research_agent01",
|
||||
});
|
||||
const details = result.details as { status?: string };
|
||||
expect(details.status).toBe("accepted");
|
||||
});
|
||||
|
||||
it("allows allowlisted-but-unconfigured agentId (#31311)", async () => {
|
||||
setSessionsSpawnConfigOverride({
|
||||
session: { mainKey: "main", scope: "per-sender" },
|
||||
agents: {
|
||||
list: [
|
||||
{ id: "main", subagents: { allowAgents: ["research"] } },
|
||||
// "research" is NOT in agents.list — only in allowAgents
|
||||
],
|
||||
},
|
||||
});
|
||||
callGatewayMock.mockImplementation(async () => ({
|
||||
runId: "run-1",
|
||||
status: "accepted",
|
||||
acceptedAt: 1000,
|
||||
}));
|
||||
const tool = await getSessionsSpawnTool({
|
||||
agentSessionKey: "main",
|
||||
agentChannel: "whatsapp",
|
||||
});
|
||||
const result = await tool.execute("call-unconfigured", {
|
||||
task: "do thing",
|
||||
agentId: "research",
|
||||
});
|
||||
const details = result.details as { status?: string };
|
||||
// Must pass: "research" is in allowAgents even though not in agents.list
|
||||
expect(details.status).toBe("accepted");
|
||||
});
|
||||
});
|
||||
|
||||
@@ -31,6 +31,16 @@ export type SpawnSubagentMode = (typeof SUBAGENT_SPAWN_MODES)[number];
|
||||
export const SUBAGENT_SPAWN_SANDBOX_MODES = ["inherit", "require"] as const;
|
||||
export type SpawnSubagentSandboxMode = (typeof SUBAGENT_SPAWN_SANDBOX_MODES)[number];
|
||||
|
||||
/**
|
||||
* Strict format gate for user-supplied agentId values.
|
||||
* Rejects error-message-like strings, path traversals, and other
|
||||
* values that would be silently mangled by {@link normalizeAgentId}
|
||||
* and could create ghost workspace directories.
|
||||
*
|
||||
* Must stay in sync with the canonical `VALID_ID_RE` in session-key.ts.
|
||||
*/
|
||||
const STRICT_AGENT_ID_RE = /^[a-z0-9][a-z0-9_-]{0,63}$/i;
|
||||
|
||||
export function decodeStrictBase64(value: string, maxDecodedBytes: number): Buffer | null {
|
||||
const maxEncodedBytes = Math.ceil(maxDecodedBytes / 3) * 4;
|
||||
if (value.length > maxEncodedBytes * 2) {
|
||||
@@ -248,7 +258,19 @@ export async function spawnSubagentDirect(
|
||||
): Promise<SpawnSubagentResult> {
|
||||
const task = params.task;
|
||||
const label = params.label?.trim() || "";
|
||||
const requestedAgentId = params.agentId;
|
||||
const requestedAgentId = params.agentId?.trim();
|
||||
|
||||
// Reject malformed agentId before normalizeAgentId can mangle it.
|
||||
// Without this gate, error-message strings like "Agent not found: xyz" pass
|
||||
// through normalizeAgentId and become "agent-not-found--xyz", which later
|
||||
// creates ghost workspace directories and triggers cascading cron loops (#31311).
|
||||
if (requestedAgentId && !STRICT_AGENT_ID_RE.test(requestedAgentId)) {
|
||||
return {
|
||||
status: "error",
|
||||
error: `Invalid agentId "${requestedAgentId}". Agent IDs must match [a-z0-9][a-z0-9_-]{0,63}. Use agents_list to discover valid targets.`,
|
||||
};
|
||||
}
|
||||
|
||||
const modelOverride = params.model;
|
||||
const thinkingOverrideRaw = params.thinking;
|
||||
const requestThreadBinding = params.thread === true;
|
||||
|
||||
Reference in New Issue
Block a user