mirror of
https://github.com/openclaw/openclaw.git
synced 2026-05-06 05:50:43 +00:00
fix: allow cron self-removal in isolated runs (#73028)
This commit is contained in:
@@ -357,6 +357,7 @@ Docs: https://docs.openclaw.ai
|
||||
- Discord/gateway: count failed health-monitor restart attempts toward cooldown and hourly caps, and evict stale account lifecycle state during channel reloads so repeated Discord gateway recovery cannot loop on old status. Fixes #38596. (#40413) Thanks @jellyAI-dev and @vashquez.
|
||||
- TTS/BlueBubbles: pre-transcode synthesized MP3 audio to opus-in-CAF (mono, 24 kHz — validated against macOS 15.x Messages.app's native voice-memo CAF descriptor) on macOS hosts before handing the file to BlueBubbles, so iMessage renders the result as a native voice-memo bubble with proper duration and waveform UI instead of a plain file attachment. Adds an opt-in `tts.voice.preferAudioFileFormat` channel capability and a magic-byte sniff for the CAF container so the host-local-media validator (which uses `file-type` and didn't recognize CAF natively) can verify the pre-transcoded buffer. Channels that don't opt in are unaffected. (#72586) Fixes #72506. Thanks @omarshahine.
|
||||
- Feishu: retry WebSocket startup failures with monitor-owned backoff while preserving SDK-local heartbeat defaults, so persistent-connection startup failures no longer leave the monitor hung. Fixes #68766; related #42354 and #55532. Thanks @alex-xuweilong, @120106835, @sirfengyu, and @tianhaocui.
|
||||
- Cron: normalize isolated job tool allowlists before granting the narrow self-removal cron tool path, keeping scheduled jobs aligned with shared tool policy normalization. (#73028) Thanks @jalehman.
|
||||
|
||||
## 2026.4.26
|
||||
|
||||
|
||||
@@ -89,6 +89,8 @@ export function createOpenClawTools(
|
||||
allowMediaInvokeCommands?: boolean;
|
||||
/** Explicit agent ID override for cron/hook sessions. */
|
||||
requesterAgentIdOverride?: string;
|
||||
/** Restrict the cron tool to self-removing this active cron job. */
|
||||
cronSelfRemoveOnlyJobId?: string;
|
||||
/** Require explicit message targets (no implicit last-route sends). */
|
||||
requireExplicitMessageTarget?: boolean;
|
||||
/** If true, omit the message tool from the tool list. */
|
||||
@@ -247,6 +249,9 @@ export function createOpenClawTools(
|
||||
accountId: options?.agentAccountId,
|
||||
threadId: options?.currentThreadTs ?? options?.agentThreadId,
|
||||
},
|
||||
...(options?.cronSelfRemoveOnlyJobId
|
||||
? { selfRemoveOnlyJobId: options.cronSelfRemoveOnlyJobId }
|
||||
: {}),
|
||||
}),
|
||||
]),
|
||||
...(!embedded && messageTool ? [messageTool] : []),
|
||||
|
||||
@@ -302,4 +302,26 @@ describe("createOpenClawTools cron context wiring", () => {
|
||||
},
|
||||
});
|
||||
});
|
||||
|
||||
it("passes self-remove scope into the cron tool", async () => {
|
||||
const { createOpenClawTools } = await import("./openclaw-tools.js");
|
||||
|
||||
createOpenClawTools({
|
||||
agentSessionKey: "agent:main:cron:job-current",
|
||||
cronSelfRemoveOnlyJobId: "job-current",
|
||||
disableMessageTool: true,
|
||||
disablePluginTools: true,
|
||||
});
|
||||
|
||||
expect(mocks.createCronToolOptions).toHaveBeenCalledWith({
|
||||
agentSessionKey: "agent:main:cron:job-current",
|
||||
currentDeliveryContext: {
|
||||
channel: undefined,
|
||||
to: undefined,
|
||||
accountId: undefined,
|
||||
threadId: undefined,
|
||||
},
|
||||
selfRemoveOnlyJobId: "job-current",
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
@@ -56,6 +56,7 @@ type FinalEffectiveToolPolicyParams = {
|
||||
senderUsername?: string | null;
|
||||
senderE164?: string | null;
|
||||
senderIsOwner?: boolean;
|
||||
ownerOnlyToolAllowlist?: string[];
|
||||
warn: (message: string) => void;
|
||||
};
|
||||
|
||||
@@ -152,6 +153,7 @@ export function applyFinalEffectiveToolPolicy(
|
||||
const ownerFiltered = applyOwnerOnlyToolPolicy(
|
||||
params.bundledTools,
|
||||
params.senderIsOwner === true,
|
||||
params.ownerOnlyToolAllowlist,
|
||||
);
|
||||
// Suppress unavailable-core-tool warnings on every step of this pass.
|
||||
// `applyToolPolicyPipeline` infers `coreToolNames` from the `tools` array
|
||||
|
||||
@@ -43,6 +43,7 @@ function makeForwardingCase(internalEvents: AgentInternalEvent[]) {
|
||||
runId: "forward-attempt-params",
|
||||
params: {
|
||||
toolsAllow: ["exec", "read"],
|
||||
ownerOnlyToolAllowlist: ["cron"],
|
||||
bootstrapContextMode: "lightweight",
|
||||
bootstrapContextRunKind: "cron",
|
||||
disableMessageTool: true,
|
||||
@@ -52,6 +53,7 @@ function makeForwardingCase(internalEvents: AgentInternalEvent[]) {
|
||||
},
|
||||
expected: {
|
||||
toolsAllow: ["exec", "read"],
|
||||
ownerOnlyToolAllowlist: ["cron"],
|
||||
bootstrapContextMode: "lightweight",
|
||||
bootstrapContextRunKind: "cron",
|
||||
disableMessageTool: true,
|
||||
|
||||
@@ -988,7 +988,9 @@ export async function runEmbeddedPiAgent(
|
||||
silentExpected: params.silentExpected,
|
||||
bootstrapContextMode: params.bootstrapContextMode,
|
||||
bootstrapContextRunKind: params.bootstrapContextRunKind,
|
||||
jobId: params.jobId,
|
||||
toolsAllow: params.toolsAllow,
|
||||
ownerOnlyToolAllowlist: params.ownerOnlyToolAllowlist,
|
||||
disableMessageTool: params.disableMessageTool,
|
||||
forceMessageTool: params.forceMessageTool,
|
||||
requireExplicitMessageTarget: params.requireExplicitMessageTarget,
|
||||
|
||||
@@ -49,6 +49,18 @@ describe("runEmbeddedAttempt memory flush tool forwarding", () => {
|
||||
}
|
||||
});
|
||||
|
||||
it("forwards cron job id into tool creation so self-removal can be scoped", () => {
|
||||
expect(
|
||||
buildEmbeddedAttemptToolRunContext({
|
||||
trigger: "cron",
|
||||
jobId: "job-current",
|
||||
}),
|
||||
).toMatchObject({
|
||||
trigger: "cron",
|
||||
jobId: "job-current",
|
||||
});
|
||||
});
|
||||
|
||||
it("activates the memory flush append-only write wrapper", async () => {
|
||||
const workspaceDir = await fs.mkdtemp(path.join(os.tmpdir(), "openclaw-attempt-memory-flush-"));
|
||||
const memoryFile = path.join(workspaceDir, MEMORY_RELATIVE_PATH);
|
||||
|
||||
@@ -72,6 +72,14 @@ describe("applyEmbeddedAttemptToolsAllow", () => {
|
||||
applyEmbeddedAttemptToolsAllow(tools, ["exec", "read"]).map((tool) => tool.name),
|
||||
).toEqual(["exec", "read"]);
|
||||
});
|
||||
|
||||
it("normalizes explicit toolsAllow entries before filtering", () => {
|
||||
const tools = [{ name: "cron" }, { name: "read" }, { name: "message" }];
|
||||
|
||||
expect(
|
||||
applyEmbeddedAttemptToolsAllow(tools, [" cron ", "READ"]).map((tool) => tool.name),
|
||||
).toEqual(["cron", "read"]);
|
||||
});
|
||||
});
|
||||
|
||||
describe("normalizeMessagesForLlmBoundary", () => {
|
||||
|
||||
@@ -6,15 +6,18 @@ import type { EmbeddedRunTrigger } from "./params.js";
|
||||
|
||||
export function buildEmbeddedAttemptToolRunContext(params: {
|
||||
trigger?: EmbeddedRunTrigger;
|
||||
jobId?: string;
|
||||
memoryFlushWritePath?: string;
|
||||
trace?: DiagnosticTraceContext;
|
||||
}): {
|
||||
trigger?: EmbeddedRunTrigger;
|
||||
jobId?: string;
|
||||
memoryFlushWritePath?: string;
|
||||
trace?: DiagnosticTraceContext;
|
||||
} {
|
||||
return {
|
||||
trigger: params.trigger,
|
||||
jobId: params.jobId,
|
||||
memoryFlushWritePath: params.memoryFlushWritePath,
|
||||
...(params.trace ? { trace: freezeDiagnosticTraceContext(params.trace) } : {}),
|
||||
};
|
||||
|
||||
@@ -157,6 +157,7 @@ import {
|
||||
collectExplicitToolAllowlistSources,
|
||||
} from "../../tool-allowlist-guard.js";
|
||||
import { UNKNOWN_TOOL_THRESHOLD } from "../../tool-loop-detection.js";
|
||||
import { normalizeToolName } from "../../tool-policy.js";
|
||||
import { shouldAllowProviderOwnedThinkingReplay } from "../../transcript-policy.js";
|
||||
import { normalizeUsage, type NormalizedUsage } from "../../usage.js";
|
||||
import { DEFAULT_BOOTSTRAP_FILENAME } from "../../workspace.js";
|
||||
@@ -480,8 +481,8 @@ export function applyEmbeddedAttemptToolsAllow<T extends { name: string }>(
|
||||
if (!toolsAllow || toolsAllow.length === 0) {
|
||||
return tools;
|
||||
}
|
||||
const allowSet = new Set(toolsAllow);
|
||||
return tools.filter((tool) => allowSet.has(tool.name));
|
||||
const allowSet = new Set(toolsAllow.map((name) => normalizeToolName(name)));
|
||||
return tools.filter((tool) => allowSet.has(normalizeToolName(tool.name)));
|
||||
}
|
||||
|
||||
export function normalizeMessagesForLlmBoundary(messages: AgentMessage[]): AgentMessage[] {
|
||||
@@ -718,6 +719,7 @@ export async function runEmbeddedAttempt(
|
||||
senderUsername: params.senderUsername,
|
||||
senderE164: params.senderE164,
|
||||
senderIsOwner: params.senderIsOwner,
|
||||
ownerOnlyToolAllowlist: params.ownerOnlyToolAllowlist,
|
||||
allowGatewaySubagentBinding: params.allowGatewaySubagentBinding,
|
||||
sessionKey: sandboxSessionKey,
|
||||
sessionId: params.sessionId,
|
||||
@@ -929,6 +931,7 @@ export async function runEmbeddedAttempt(
|
||||
senderUsername: params.senderUsername,
|
||||
senderE164: params.senderE164,
|
||||
senderIsOwner: params.senderIsOwner,
|
||||
ownerOnlyToolAllowlist: params.ownerOnlyToolAllowlist,
|
||||
warn: (message) => log.warn(message),
|
||||
});
|
||||
const effectiveTools = [...tools, ...filteredBundledTools];
|
||||
|
||||
@@ -60,6 +60,11 @@ export type RunEmbeddedPiAgentParams = {
|
||||
senderE164?: string | null;
|
||||
/** Whether the sender is an owner (required for owner-only tools). */
|
||||
senderIsOwner?: boolean;
|
||||
/**
|
||||
* Additional owner-only tools authorized by a server-side runtime grant.
|
||||
* This must stay narrow; it does not make the sender an owner.
|
||||
*/
|
||||
ownerOnlyToolAllowlist?: string[];
|
||||
/** Current channel ID for auto-threading (Slack). */
|
||||
currentChannelId?: string;
|
||||
/** Current thread timestamp for auto-threading (Slack). */
|
||||
|
||||
@@ -17,6 +17,8 @@ import { createHostSandboxFsBridge } from "./test-helpers/host-sandbox-fs-bridge
|
||||
import { expectReadWriteEditTools } from "./test-helpers/pi-tools-fs-helpers.js";
|
||||
import { createPiToolsSandboxContext } from "./test-helpers/pi-tools-sandbox-context.js";
|
||||
import { providerAliasCases } from "./test-helpers/provider-alias-cases.js";
|
||||
import { buildEmptyExplicitToolAllowlistError } from "./tool-allowlist-guard.js";
|
||||
import { normalizeToolName } from "./tool-policy.js";
|
||||
|
||||
const tinyPngBuffer = Buffer.from(
|
||||
"iVBORw0KGgoAAAANSUhEUgAAAAEAAAABCAQAAAC1HAwCAAAAC0lEQVR42mP8/x8AAwMCAO2f7z8AAAAASUVORK5CYII=",
|
||||
@@ -84,6 +86,11 @@ function expectNoSubagentControlTools(tools: ReturnType<typeof createOpenClawCod
|
||||
expect(names.has("subagents")).toBe(false);
|
||||
}
|
||||
|
||||
function applyRuntimeToolsAllow<T extends { name: string }>(tools: T[], toolsAllow: string[]) {
|
||||
const allowSet = new Set(toolsAllow.map((name) => normalizeToolName(name)));
|
||||
return tools.filter((tool) => allowSet.has(normalizeToolName(tool.name)));
|
||||
}
|
||||
|
||||
describe("createOpenClawCodingTools", () => {
|
||||
const testConfig: OpenClawConfig = {};
|
||||
|
||||
@@ -106,6 +113,56 @@ describe("createOpenClawCodingTools", () => {
|
||||
);
|
||||
});
|
||||
|
||||
it("exposes only an explicitly authorized owner-only tool to non-owner sessions", () => {
|
||||
const tools = createOpenClawCodingTools({
|
||||
config: testConfig,
|
||||
senderIsOwner: false,
|
||||
ownerOnlyToolAllowlist: ["cron"],
|
||||
});
|
||||
const names = new Set(tools.map((tool) => tool.name));
|
||||
|
||||
expect(names.has("cron")).toBe(true);
|
||||
expect(names.has("gateway")).toBe(false);
|
||||
expect(names.has("nodes")).toBe(false);
|
||||
});
|
||||
|
||||
it("resolves isolated cron runtime toolsAllow after the cron owner-only grant", () => {
|
||||
const withoutGrant = applyRuntimeToolsAllow(
|
||||
createOpenClawCodingTools({
|
||||
config: testConfig,
|
||||
senderIsOwner: false,
|
||||
}),
|
||||
["cron"],
|
||||
);
|
||||
const errorWithoutGrant = buildEmptyExplicitToolAllowlistError({
|
||||
sources: [{ label: "runtime toolsAllow", entries: ["cron"] }],
|
||||
callableToolNames: withoutGrant.map((tool) => tool.name),
|
||||
toolsEnabled: true,
|
||||
});
|
||||
|
||||
expect(errorWithoutGrant?.message).toContain(
|
||||
"No callable tools remain after resolving explicit tool allowlist (runtime toolsAllow: cron); no registered tools matched.",
|
||||
);
|
||||
|
||||
const withGrant = applyRuntimeToolsAllow(
|
||||
createOpenClawCodingTools({
|
||||
config: testConfig,
|
||||
senderIsOwner: false,
|
||||
ownerOnlyToolAllowlist: ["cron"],
|
||||
}),
|
||||
["cron"],
|
||||
);
|
||||
|
||||
expect(withGrant.map((tool) => tool.name)).toEqual(["cron"]);
|
||||
expect(
|
||||
buildEmptyExplicitToolAllowlistError({
|
||||
sources: [{ label: "runtime toolsAllow", entries: ["cron"] }],
|
||||
callableToolNames: withGrant.map((tool) => tool.name),
|
||||
toolsEnabled: true,
|
||||
}),
|
||||
).toBeNull();
|
||||
});
|
||||
|
||||
it("preserves action enums in normalized schemas", () => {
|
||||
const defaultTools = createOpenClawCodingTools({ config: testConfig, senderIsOwner: true });
|
||||
const toolNames = ["canvas", "nodes", "cron", "gateway", "message"];
|
||||
|
||||
67
src/agents/pi-tools.cron-scope.test.ts
Normal file
67
src/agents/pi-tools.cron-scope.test.ts
Normal file
@@ -0,0 +1,67 @@
|
||||
import { beforeEach, describe, expect, it, vi } from "vitest";
|
||||
import type { AnyAgentTool } from "./tools/common.js";
|
||||
|
||||
const mocks = vi.hoisted(() => {
|
||||
const stubTool = (name: string, ownerOnly = false) =>
|
||||
({
|
||||
name,
|
||||
label: name,
|
||||
displaySummary: name,
|
||||
description: name,
|
||||
ownerOnly,
|
||||
parameters: { type: "object", properties: {} },
|
||||
execute: vi.fn(),
|
||||
}) satisfies AnyAgentTool;
|
||||
|
||||
return {
|
||||
createOpenClawToolsOptions: vi.fn(),
|
||||
stubTool,
|
||||
};
|
||||
});
|
||||
|
||||
vi.mock("./openclaw-tools.js", () => ({
|
||||
createOpenClawTools: (options: unknown) => {
|
||||
mocks.createOpenClawToolsOptions(options);
|
||||
return [mocks.stubTool("cron", true)];
|
||||
},
|
||||
}));
|
||||
|
||||
import "./test-helpers/fast-bash-tools.js";
|
||||
import "./test-helpers/fast-coding-tools.js";
|
||||
import { createOpenClawCodingTools } from "./pi-tools.js";
|
||||
|
||||
describe("createOpenClawCodingTools cron scope", () => {
|
||||
beforeEach(() => {
|
||||
mocks.createOpenClawToolsOptions.mockClear();
|
||||
});
|
||||
|
||||
it("scopes the cron owner-only runtime grant to self-removal", () => {
|
||||
const tools = createOpenClawCodingTools({
|
||||
trigger: "cron",
|
||||
jobId: "job-current",
|
||||
senderIsOwner: false,
|
||||
ownerOnlyToolAllowlist: ["cron"],
|
||||
});
|
||||
|
||||
expect(tools.map((tool) => tool.name)).toContain("cron");
|
||||
expect(mocks.createOpenClawToolsOptions).toHaveBeenCalledWith(
|
||||
expect.objectContaining({
|
||||
cronSelfRemoveOnlyJobId: "job-current",
|
||||
}),
|
||||
);
|
||||
});
|
||||
|
||||
it("does not scope ordinary owner cron sessions", () => {
|
||||
createOpenClawCodingTools({
|
||||
trigger: "cron",
|
||||
jobId: "job-current",
|
||||
senderIsOwner: true,
|
||||
});
|
||||
|
||||
expect(mocks.createOpenClawToolsOptions).toHaveBeenCalledWith(
|
||||
expect.not.objectContaining({
|
||||
cronSelfRemoveOnlyJobId: expect.any(String),
|
||||
}),
|
||||
);
|
||||
});
|
||||
});
|
||||
@@ -66,6 +66,7 @@ import {
|
||||
applyOwnerOnlyToolPolicy,
|
||||
collectExplicitAllowlist,
|
||||
mergeAlsoAllowPolicy,
|
||||
normalizeToolName,
|
||||
resolveToolProfilePolicy,
|
||||
} from "./tool-policy.js";
|
||||
import { resolveWorkspaceRoot } from "./workspace-dir.js";
|
||||
@@ -273,6 +274,8 @@ export function createOpenClawCodingTools(options?: {
|
||||
trace?: DiagnosticTraceContext;
|
||||
/** What initiated this run (for trigger-specific tool restrictions). */
|
||||
trigger?: string;
|
||||
/** Stable cron job identifier populated for cron-triggered runs. */
|
||||
jobId?: string;
|
||||
/** Relative workspace path that memory-triggered writes may append to. */
|
||||
memoryFlushWritePath?: string;
|
||||
agentDir?: string;
|
||||
@@ -340,6 +343,11 @@ export function createOpenClawCodingTools(options?: {
|
||||
forceMessageTool?: boolean;
|
||||
/** Whether the sender is an owner (required for owner-only tools). */
|
||||
senderIsOwner?: boolean;
|
||||
/**
|
||||
* Additional owner-only tools authorized by a server-side runtime grant.
|
||||
* Keep this narrowly scoped; it is not a replacement for sender ownership.
|
||||
*/
|
||||
ownerOnlyToolAllowlist?: string[];
|
||||
/** Callback invoked when sessions_yield tool is called. */
|
||||
onYield?: (message: string) => Promise<void> | void;
|
||||
}): AnyAgentTool[] {
|
||||
@@ -350,6 +358,12 @@ export function createOpenClawCodingTools(options?: {
|
||||
throw new Error("memoryFlushWritePath required for memory-triggered tool runs");
|
||||
}
|
||||
const memoryFlushWritePath = isMemoryFlushRun ? options.memoryFlushWritePath : undefined;
|
||||
const cronSelfRemoveOnlyJobId =
|
||||
options?.trigger === "cron" &&
|
||||
options.jobId?.trim() &&
|
||||
options.ownerOnlyToolAllowlist?.some((toolName) => normalizeToolName(toolName) === "cron")
|
||||
? options.jobId.trim()
|
||||
: undefined;
|
||||
const {
|
||||
agentId,
|
||||
globalPolicy,
|
||||
@@ -626,6 +640,7 @@ export function createOpenClawCodingTools(options?: {
|
||||
modelHasVision: options?.modelHasVision,
|
||||
requireExplicitMessageTarget: options?.requireExplicitMessageTarget,
|
||||
disableMessageTool: options?.disableMessageTool,
|
||||
...(cronSelfRemoveOnlyJobId ? { cronSelfRemoveOnlyJobId } : {}),
|
||||
requesterAgentIdOverride: agentId,
|
||||
requesterSenderId: options?.senderId,
|
||||
senderIsOwner: options?.senderIsOwner,
|
||||
@@ -670,7 +685,11 @@ export function createOpenClawCodingTools(options?: {
|
||||
});
|
||||
// Security: treat unknown/undefined as unauthorized (opt-in, not opt-out)
|
||||
const senderIsOwner = options?.senderIsOwner === true;
|
||||
const toolsByAuthorization = applyOwnerOnlyToolPolicy(toolsForModelProvider, senderIsOwner);
|
||||
const toolsByAuthorization = applyOwnerOnlyToolPolicy(
|
||||
toolsForModelProvider,
|
||||
senderIsOwner,
|
||||
options?.ownerOnlyToolAllowlist,
|
||||
);
|
||||
const subagentFiltered = applyToolPolicyPipeline({
|
||||
tools: toolsByAuthorization,
|
||||
toolMeta: (tool) => getPluginToolMeta(tool),
|
||||
|
||||
@@ -32,6 +32,11 @@ function createOwnerPolicyTools() {
|
||||
ownerOnly: true,
|
||||
execute: async () => ({ content: [], details: {} }) as any,
|
||||
},
|
||||
{
|
||||
name: "nodes",
|
||||
ownerOnly: true,
|
||||
execute: async () => ({ content: [], details: {} }) as any,
|
||||
},
|
||||
] as unknown as AnyAgentTool[];
|
||||
}
|
||||
|
||||
@@ -107,7 +112,20 @@ describe("tool-policy", () => {
|
||||
it("keeps owner-only tools for the owner sender", async () => {
|
||||
const tools = createOwnerPolicyTools();
|
||||
const filtered = applyOwnerOnlyToolPolicy(tools, true);
|
||||
expect(filtered.map((t) => t.name)).toEqual(["read", "cron", "gateway"]);
|
||||
expect(filtered.map((t) => t.name)).toEqual(["read", "cron", "gateway", "nodes"]);
|
||||
});
|
||||
|
||||
it("keeps only explicitly authorized owner-only tools for non-owner senders", async () => {
|
||||
const tools = createOwnerPolicyTools();
|
||||
const filtered = applyOwnerOnlyToolPolicy(tools, false, ["cron"]);
|
||||
expect(filtered.map((t) => t.name)).toEqual(["read", "cron"]);
|
||||
|
||||
await expect(
|
||||
filtered.find((tool) => tool.name === "cron")?.execute?.("call_1", {}),
|
||||
).resolves.toEqual({
|
||||
content: [],
|
||||
details: {},
|
||||
});
|
||||
});
|
||||
|
||||
it("honors ownerOnly metadata for custom tool names", async () => {
|
||||
|
||||
@@ -19,8 +19,8 @@ export type { ToolProfileId } from "./tool-policy-shared.js";
|
||||
export type OwnerOnlyToolApprovalClass = "control_plane" | "exec_capable" | "interactive";
|
||||
|
||||
// Keep tool-policy browser-safe: do not import tools/common at runtime.
|
||||
function wrapOwnerOnlyToolExecution(tool: AnyAgentTool, senderIsOwner: boolean): AnyAgentTool {
|
||||
if (tool.ownerOnly !== true || senderIsOwner || !tool.execute) {
|
||||
function wrapOwnerOnlyToolExecution(tool: AnyAgentTool, authorized: boolean): AnyAgentTool {
|
||||
if (tool.ownerOnly !== true || authorized || !tool.execute) {
|
||||
return tool;
|
||||
}
|
||||
return {
|
||||
@@ -51,17 +51,30 @@ function isOwnerOnlyTool(tool: AnyAgentTool) {
|
||||
return tool.ownerOnly === true || isOwnerOnlyToolName(tool.name);
|
||||
}
|
||||
|
||||
export function applyOwnerOnlyToolPolicy(tools: AnyAgentTool[], senderIsOwner: boolean) {
|
||||
/**
|
||||
* Filters owner-only tools unless the sender is an owner or a server-side
|
||||
* runtime grant authorizes a specific owner-only tool for this run.
|
||||
*/
|
||||
export function applyOwnerOnlyToolPolicy(
|
||||
tools: AnyAgentTool[],
|
||||
senderIsOwner: boolean,
|
||||
ownerOnlyToolAllowlist?: string[],
|
||||
) {
|
||||
const allowedOwnerOnlyTools = new Set(
|
||||
ownerOnlyToolAllowlist?.map((name) => normalizeToolName(name)) ?? [],
|
||||
);
|
||||
const isAuthorized = (tool: AnyAgentTool) =>
|
||||
senderIsOwner || allowedOwnerOnlyTools.has(normalizeToolName(tool.name));
|
||||
const withGuard = tools.map((tool) => {
|
||||
if (!isOwnerOnlyTool(tool)) {
|
||||
return tool;
|
||||
}
|
||||
return wrapOwnerOnlyToolExecution(tool, senderIsOwner);
|
||||
return wrapOwnerOnlyToolExecution(tool, isAuthorized(tool));
|
||||
});
|
||||
if (senderIsOwner) {
|
||||
return withGuard;
|
||||
}
|
||||
return withGuard.filter((tool) => !isOwnerOnlyTool(tool));
|
||||
return withGuard.filter((tool) => !isOwnerOnlyTool(tool) || isAuthorized(tool));
|
||||
}
|
||||
|
||||
export type ToolPolicyLike = {
|
||||
|
||||
@@ -173,6 +173,43 @@ describe("cron tool", () => {
|
||||
expect(tool.ownerOnly).toBe(true);
|
||||
});
|
||||
|
||||
it("allows scoped isolated cron runs to remove the current job", async () => {
|
||||
const tool = createTestCronTool({ selfRemoveOnlyJobId: "job-current" });
|
||||
|
||||
await tool.execute("call-self-remove", {
|
||||
action: "remove",
|
||||
jobId: "job-current",
|
||||
});
|
||||
|
||||
const params = expectSingleGatewayCallMethod("cron.remove");
|
||||
expect(params).toEqual({ id: "job-current" });
|
||||
});
|
||||
|
||||
it("denies scoped isolated cron runs from removing another job", async () => {
|
||||
const tool = createTestCronTool({ selfRemoveOnlyJobId: "job-current" });
|
||||
|
||||
await expect(
|
||||
tool.execute("call-remove-other", {
|
||||
action: "remove",
|
||||
jobId: "job-other",
|
||||
}),
|
||||
).rejects.toThrow("Cron tool is restricted to removing the current cron job.");
|
||||
|
||||
expect(callGatewayMock).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("denies scoped isolated cron runs from using non-remove cron actions", async () => {
|
||||
const tool = createTestCronTool({ selfRemoveOnlyJobId: "job-current" });
|
||||
|
||||
await expect(
|
||||
tool.execute("call-list", {
|
||||
action: "list",
|
||||
}),
|
||||
).rejects.toThrow("Cron tool is restricted to removing the current cron job.");
|
||||
|
||||
expect(callGatewayMock).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("documents deferred follow-up guidance in the tool description", () => {
|
||||
const tool = createTestCronTool();
|
||||
expect(tool.description).toContain(
|
||||
|
||||
@@ -314,6 +314,8 @@ export const CronToolSchema = Type.Object(
|
||||
type CronToolOptions = {
|
||||
agentSessionKey?: string;
|
||||
currentDeliveryContext?: DeliveryContext;
|
||||
/** Restrict this cron tool instance to removing only this active cron job. */
|
||||
selfRemoveOnlyJobId?: string;
|
||||
};
|
||||
|
||||
type GatewayToolCaller = typeof callGatewayTool;
|
||||
@@ -343,6 +345,29 @@ function truncateText(input: string, maxLen: number) {
|
||||
return `${truncated}...`;
|
||||
}
|
||||
|
||||
function readCronJobIdParam(params: Record<string, unknown>) {
|
||||
return readStringParam(params, "jobId") ?? readStringParam(params, "id");
|
||||
}
|
||||
|
||||
function assertCronSelfRemoveScope(
|
||||
opts: CronToolOptions | undefined,
|
||||
action: string,
|
||||
params: Record<string, unknown>,
|
||||
) {
|
||||
const selfRemoveOnlyJobId = opts?.selfRemoveOnlyJobId?.trim();
|
||||
if (!selfRemoveOnlyJobId) {
|
||||
return;
|
||||
}
|
||||
if (action !== "remove") {
|
||||
throw new Error("Cron tool is restricted to removing the current cron job.");
|
||||
}
|
||||
const id = readCronJobIdParam(params);
|
||||
if (id && id === selfRemoveOnlyJobId) {
|
||||
return;
|
||||
}
|
||||
throw new Error("Cron tool is restricted to removing the current cron job.");
|
||||
}
|
||||
|
||||
function extractMessageText(message: ChatMessage): { role: string; text: string } | null {
|
||||
const role = typeof message.role === "string" ? message.role : "";
|
||||
if (role !== "user" && role !== "assistant") {
|
||||
@@ -616,6 +641,7 @@ Use jobId as the canonical identifier; id is accepted for compatibility. Use con
|
||||
execute: async (_toolCallId, args) => {
|
||||
const params = args as Record<string, unknown>;
|
||||
const action = readStringParam(params, "action", { required: true });
|
||||
assertCronSelfRemoveScope(opts, action, params);
|
||||
const gatewayOpts: GatewayCallOptions = {
|
||||
...readGatewayCallOptions(params),
|
||||
timeoutMs:
|
||||
@@ -745,7 +771,7 @@ Use jobId as the canonical identifier; id is accepted for compatibility. Use con
|
||||
return jsonResult(await callGateway("cron.add", gatewayOpts, job));
|
||||
}
|
||||
case "update": {
|
||||
const id = readStringParam(params, "jobId") ?? readStringParam(params, "id");
|
||||
const id = readCronJobIdParam(params);
|
||||
if (!id) {
|
||||
throw new Error("jobId required (id accepted for backward compatibility)");
|
||||
}
|
||||
@@ -780,14 +806,14 @@ Use jobId as the canonical identifier; id is accepted for compatibility. Use con
|
||||
);
|
||||
}
|
||||
case "remove": {
|
||||
const id = readStringParam(params, "jobId") ?? readStringParam(params, "id");
|
||||
const id = readCronJobIdParam(params);
|
||||
if (!id) {
|
||||
throw new Error("jobId required (id accepted for backward compatibility)");
|
||||
}
|
||||
return jsonResult(await callGateway("cron.remove", gatewayOpts, { id }));
|
||||
}
|
||||
case "run": {
|
||||
const id = readStringParam(params, "jobId") ?? readStringParam(params, "id");
|
||||
const id = readCronJobIdParam(params);
|
||||
if (!id) {
|
||||
throw new Error("jobId required (id accepted for backward compatibility)");
|
||||
}
|
||||
@@ -796,7 +822,7 @@ Use jobId as the canonical identifier; id is accepted for compatibility. Use con
|
||||
return jsonResult(await callGateway("cron.run", gatewayOpts, { id, mode: runMode }));
|
||||
}
|
||||
case "runs": {
|
||||
const id = readStringParam(params, "jobId") ?? readStringParam(params, "id");
|
||||
const id = readCronJobIdParam(params);
|
||||
if (!id) {
|
||||
throw new Error("jobId required (id accepted for backward compatibility)");
|
||||
}
|
||||
|
||||
@@ -1,4 +1,5 @@
|
||||
import type { SkillSnapshot } from "../../agents/skills.js";
|
||||
import { normalizeToolList } from "../../agents/tool-policy.js";
|
||||
import type { ThinkLevel, VerboseLevel } from "../../auto-reply/thinking.js";
|
||||
import type { AgentDefaultsConfig } from "../../config/types.agent-defaults.js";
|
||||
import type { OpenClawConfig } from "../../config/types.openclaw.js";
|
||||
@@ -47,6 +48,13 @@ async function loadCronSubagentRegistryRuntime() {
|
||||
return await cronSubagentRegistryRuntimePromise;
|
||||
}
|
||||
|
||||
function resolveCronOwnerOnlyToolAllowlist(toolsAllow: string[] | undefined): string[] | undefined {
|
||||
if (!normalizeToolList(toolsAllow).includes("cron")) {
|
||||
return undefined;
|
||||
}
|
||||
return ["cron"];
|
||||
}
|
||||
|
||||
export type CronExecutionResult = {
|
||||
runResult: CronPromptRunResult;
|
||||
fallbackProvider: string;
|
||||
@@ -172,6 +180,9 @@ export function createCronPromptExecutor(params: {
|
||||
cleanupBundleMcpOnRunEnd: params.job.sessionTarget === "isolated",
|
||||
allowGatewaySubagentBinding: true,
|
||||
senderIsOwner: false,
|
||||
ownerOnlyToolAllowlist: resolveCronOwnerOnlyToolAllowlist(
|
||||
params.agentPayload?.toolsAllow,
|
||||
),
|
||||
messageChannel: params.messageChannel,
|
||||
agentAccountId: params.resolvedDelivery.accountId,
|
||||
messageTo: params.resolvedDelivery.to,
|
||||
|
||||
@@ -29,6 +29,22 @@ function makeParams() {
|
||||
};
|
||||
}
|
||||
|
||||
function makeParamsWithToolsAllow(toolsAllow: string[]) {
|
||||
const params = makeParams();
|
||||
const job = params.job as Record<string, unknown>;
|
||||
return {
|
||||
...params,
|
||||
job: {
|
||||
...job,
|
||||
payload: {
|
||||
kind: "agentTurn",
|
||||
message: "check owner tools",
|
||||
toolsAllow,
|
||||
},
|
||||
} as never,
|
||||
};
|
||||
}
|
||||
|
||||
describe("runCronIsolatedAgentTurn owner auth", () => {
|
||||
let previousFastTestEnv: string | undefined;
|
||||
|
||||
@@ -68,4 +84,48 @@ describe("runCronIsolatedAgentTurn owner auth", () => {
|
||||
expect(senderIsOwner).toBe(false);
|
||||
},
|
||||
);
|
||||
|
||||
it(
|
||||
"authorizes the exact isolated cron toolsAllow=cron self-removal path",
|
||||
{ timeout: RUN_OWNER_AUTH_TIMEOUT_MS },
|
||||
async () => {
|
||||
await runCronIsolatedAgentTurn(makeParamsWithToolsAllow(["cron"]));
|
||||
|
||||
expect(runEmbeddedPiAgentMock).toHaveBeenCalledTimes(1);
|
||||
const call = runEmbeddedPiAgentMock.mock.calls[0]?.[0];
|
||||
expect(call?.senderIsOwner).toBe(false);
|
||||
expect(call?.jobId).toBe("owner-auth");
|
||||
expect(call?.ownerOnlyToolAllowlist).toEqual(["cron"]);
|
||||
expect(call?.toolsAllow).toEqual(["cron"]);
|
||||
},
|
||||
);
|
||||
|
||||
it(
|
||||
"normalizes toolsAllow before authorizing isolated cron self-removal",
|
||||
{ timeout: RUN_OWNER_AUTH_TIMEOUT_MS },
|
||||
async () => {
|
||||
await runCronIsolatedAgentTurn(makeParamsWithToolsAllow([" CRON "]));
|
||||
|
||||
expect(runEmbeddedPiAgentMock).toHaveBeenCalledTimes(1);
|
||||
const call = runEmbeddedPiAgentMock.mock.calls[0]?.[0];
|
||||
expect(call?.senderIsOwner).toBe(false);
|
||||
expect(call?.jobId).toBe("owner-auth");
|
||||
expect(call?.ownerOnlyToolAllowlist).toEqual(["cron"]);
|
||||
expect(call?.toolsAllow).toEqual([" CRON "]);
|
||||
},
|
||||
);
|
||||
|
||||
it(
|
||||
"does not authorize cron when isolated cron toolsAllow omits cron",
|
||||
{ timeout: RUN_OWNER_AUTH_TIMEOUT_MS },
|
||||
async () => {
|
||||
await runCronIsolatedAgentTurn(makeParamsWithToolsAllow(["maniple__check_idle_workers"]));
|
||||
|
||||
expect(runEmbeddedPiAgentMock).toHaveBeenCalledTimes(1);
|
||||
const call = runEmbeddedPiAgentMock.mock.calls[0]?.[0];
|
||||
expect(call?.senderIsOwner).toBe(false);
|
||||
expect(call?.ownerOnlyToolAllowlist).toBeUndefined();
|
||||
expect(call?.toolsAllow).toEqual(["maniple__check_idle_workers"]);
|
||||
},
|
||||
);
|
||||
});
|
||||
|
||||
@@ -1020,6 +1020,115 @@ describe("cron service timer regressions", () => {
|
||||
expect(jobs.find((job) => job.id === second.id)?.state.lastStatus).toBe("ok");
|
||||
});
|
||||
|
||||
it("finalizes a successful isolated job that removes itself during execution", async () => {
|
||||
const store = timerRegressionFixtures.makeStorePath();
|
||||
const dueAt = Date.parse("2026-02-06T10:05:01.000Z");
|
||||
const selfRemovingJob = createDueIsolatedJob({
|
||||
id: "self-removing-success",
|
||||
nowMs: dueAt,
|
||||
nextRunAtMs: dueAt,
|
||||
});
|
||||
selfRemovingJob.delivery = {
|
||||
mode: "announce",
|
||||
channel: "telegram",
|
||||
to: "chat-123",
|
||||
};
|
||||
await writeCronJobs(store.storePath, [selfRemovingJob]);
|
||||
|
||||
const events: CronEvent[] = [];
|
||||
const log = {
|
||||
...noopLogger,
|
||||
warn: vi.fn(),
|
||||
info: vi.fn(),
|
||||
};
|
||||
const state = createCronServiceState({
|
||||
cronEnabled: true,
|
||||
storePath: store.storePath,
|
||||
log,
|
||||
nowMs: () => dueAt,
|
||||
enqueueSystemEvent: vi.fn(),
|
||||
requestHeartbeatNow: vi.fn(),
|
||||
onEvent: (evt) => {
|
||||
events.push(evt);
|
||||
},
|
||||
runIsolatedAgentJob: vi.fn(async (params: { job: { id: string } }) => {
|
||||
await fs.writeFile(store.storePath, JSON.stringify({ version: 1, jobs: [] }), "utf-8");
|
||||
return {
|
||||
status: "ok" as const,
|
||||
summary: `finished ${params.job.id}`,
|
||||
delivered: true,
|
||||
};
|
||||
}),
|
||||
});
|
||||
|
||||
await onTimer(state);
|
||||
|
||||
expect(state.store?.jobs).toEqual([]);
|
||||
expect(log.warn).not.toHaveBeenCalledWith(
|
||||
expect.anything(),
|
||||
"cron: applyOutcomeToStoredJob — job not found after forceReload, result discarded",
|
||||
);
|
||||
expect(log.info).toHaveBeenCalledWith(
|
||||
{ jobId: selfRemovingJob.id },
|
||||
"cron: finalized successful run after job was removed during execution",
|
||||
);
|
||||
expect(events).toContainEqual(
|
||||
expect.objectContaining({
|
||||
jobId: selfRemovingJob.id,
|
||||
action: "finished",
|
||||
status: "ok",
|
||||
summary: `finished ${selfRemovingJob.id}`,
|
||||
delivered: true,
|
||||
deliveryStatus: "delivered",
|
||||
}),
|
||||
);
|
||||
});
|
||||
|
||||
it("keeps missing-job discard semantics for failed isolated outcomes", async () => {
|
||||
const store = timerRegressionFixtures.makeStorePath();
|
||||
const dueAt = Date.parse("2026-02-06T10:05:01.000Z");
|
||||
const failedJob = createDueIsolatedJob({
|
||||
id: "self-removing-failure",
|
||||
nowMs: dueAt,
|
||||
nextRunAtMs: dueAt,
|
||||
});
|
||||
await writeCronJobs(store.storePath, [failedJob]);
|
||||
|
||||
const events: CronEvent[] = [];
|
||||
const log = {
|
||||
...noopLogger,
|
||||
warn: vi.fn(),
|
||||
};
|
||||
const state = createCronServiceState({
|
||||
cronEnabled: true,
|
||||
storePath: store.storePath,
|
||||
log,
|
||||
nowMs: () => dueAt,
|
||||
enqueueSystemEvent: vi.fn(),
|
||||
requestHeartbeatNow: vi.fn(),
|
||||
onEvent: (evt) => {
|
||||
events.push(evt);
|
||||
},
|
||||
runIsolatedAgentJob: vi.fn(async () => {
|
||||
await fs.writeFile(store.storePath, JSON.stringify({ version: 1, jobs: [] }), "utf-8");
|
||||
return { status: "error" as const, error: "agent failed after removal" };
|
||||
}),
|
||||
});
|
||||
|
||||
await onTimer(state);
|
||||
|
||||
expect(state.store?.jobs).toEqual([]);
|
||||
expect(log.warn).toHaveBeenCalledWith(
|
||||
{ jobId: failedJob.id },
|
||||
"cron: applyOutcomeToStoredJob — job not found after forceReload, result discarded",
|
||||
);
|
||||
expect(
|
||||
events.some(
|
||||
(evt) => evt.jobId === failedJob.id && evt.action === "finished" && evt.status === "error",
|
||||
),
|
||||
).toBe(false);
|
||||
});
|
||||
|
||||
it("outer cron timeout fires at configured timeoutSeconds, not at 1/3 (#29774)", async () => {
|
||||
vi.useFakeTimers();
|
||||
try {
|
||||
|
||||
@@ -69,6 +69,7 @@ type ResolvedFailureAlert = {
|
||||
type TimedCronRunOutcome = CronRunOutcome &
|
||||
CronRunTelemetry & {
|
||||
jobId: string;
|
||||
job: CronJob;
|
||||
taskRunId?: string;
|
||||
delivered?: boolean;
|
||||
deliveryAttempted?: boolean;
|
||||
@@ -651,6 +652,21 @@ function applyOutcomeToStoredJob(state: CronServiceState, result: TimedCronRunOu
|
||||
const jobs = store.jobs;
|
||||
const job = jobs.find((entry) => entry.id === result.jobId);
|
||||
if (!job) {
|
||||
if (result.status === "ok") {
|
||||
applyJobResult(state, result.job, {
|
||||
status: result.status,
|
||||
error: result.error,
|
||||
delivered: result.delivered,
|
||||
startedAt: result.startedAt,
|
||||
endedAt: result.endedAt,
|
||||
});
|
||||
emitJobFinished(state, result.job, result, result.startedAt);
|
||||
state.deps.log.info(
|
||||
{ jobId: result.jobId },
|
||||
"cron: finalized successful run after job was removed during execution",
|
||||
);
|
||||
return;
|
||||
}
|
||||
state.deps.log.warn(
|
||||
{ jobId: result.jobId },
|
||||
"cron: applyOutcomeToStoredJob — job not found after forceReload, result discarded",
|
||||
@@ -811,6 +827,7 @@ export async function onTimer(state: CronServiceState) {
|
||||
const result = await executeJobCoreWithTimeout(state, job);
|
||||
return {
|
||||
jobId: id,
|
||||
job,
|
||||
taskRunId,
|
||||
...result,
|
||||
startedAt,
|
||||
@@ -824,6 +841,7 @@ export async function onTimer(state: CronServiceState) {
|
||||
);
|
||||
return {
|
||||
jobId: id,
|
||||
job,
|
||||
taskRunId,
|
||||
status: "error",
|
||||
error: errorText,
|
||||
@@ -1119,6 +1137,7 @@ async function runStartupCatchupCandidate(
|
||||
const result = await executeJobCoreWithTimeout(state, candidate.job);
|
||||
return {
|
||||
jobId: candidate.jobId,
|
||||
job: candidate.job,
|
||||
taskRunId,
|
||||
status: result.status,
|
||||
error: result.error,
|
||||
@@ -1135,6 +1154,7 @@ async function runStartupCatchupCandidate(
|
||||
} catch (err) {
|
||||
return {
|
||||
jobId: candidate.jobId,
|
||||
job: candidate.job,
|
||||
taskRunId,
|
||||
status: "error",
|
||||
error: normalizeCronRunErrorText(err),
|
||||
|
||||
Reference in New Issue
Block a user