mirror of
https://github.com/openclaw/openclaw.git
synced 2026-05-06 12:10:42 +00:00
fix: reject invalid cron announce delivery config (#69015)
* test(gateway): cover invalid cron announce delivery config * fix(gateway): reject invalid cron announce delivery config * style(gateway): use toSorted for configured channels * fix: reject invalid cron announce delivery config (#69015) * fix: preserve default-agent cron delivery validation (#69015)
This commit is contained in:
@@ -18,6 +18,7 @@ Docs: https://docs.openclaw.ai
|
||||
- Browser/user-profile: let existing-session `profile="user"` tool calls auto-route to a connected browser node or use explicit `target="node"`, while still honoring explicit `target="host"` pinning. (#48677)
|
||||
- Discord/slash commands: tolerate partial Discord channel metadata in slash-command and model-picker flows so partial channel objects no longer crash when channel names, topics, or thread parent metadata are unavailable. (#68953) Thanks @dutifulbob.
|
||||
- BlueBubbles: consolidate outbound HTTP through a typed `BlueBubblesClient` that resolves the SSRF policy once at construction so image attachments stop getting blocked on localhost and reactions stop getting blocked on private-IP BB deployments. Fixes #34749 and #59722. (#68234) Thanks @omarshahine.
|
||||
- Cron/gateway: reject ambiguous announce delivery config at add/update time so invalid multi-channel or target-id provider settings fail early instead of persisting broken cron jobs. (#69015) Thanks @obviyus.
|
||||
|
||||
## 2026.4.19-beta.2
|
||||
|
||||
|
||||
@@ -1,13 +1,19 @@
|
||||
import { resolveDefaultAgentId } from "../../agents/agent-scope.js";
|
||||
import { listPotentialConfiguredChannelIds } from "../../channels/config-presence.js";
|
||||
import { loadConfig } from "../../config/config.js";
|
||||
import type { OpenClawConfig } from "../../config/types.openclaw.js";
|
||||
import { normalizeCronJobCreate, normalizeCronJobPatch } from "../../cron/normalize.js";
|
||||
import {
|
||||
readCronRunLogEntriesPage,
|
||||
readCronRunLogEntriesPageAll,
|
||||
resolveCronRunLogPath,
|
||||
} from "../../cron/run-log.js";
|
||||
import { applyJobPatch } from "../../cron/service/jobs.js";
|
||||
import { isInvalidCronSessionTargetIdError } from "../../cron/session-target.js";
|
||||
import type { CronJobCreate, CronJobPatch } from "../../cron/types.js";
|
||||
import type { CronDelivery, CronJob, CronJobCreate, CronJobPatch } from "../../cron/types.js";
|
||||
import { validateScheduleTimestamp } from "../../cron/validate-timestamp.js";
|
||||
import { formatErrorMessage } from "../../infra/errors.js";
|
||||
import { normalizeMessageChannel } from "../../utils/message-channel.js";
|
||||
import {
|
||||
ErrorCodes,
|
||||
errorShape,
|
||||
@@ -23,6 +29,84 @@ import {
|
||||
} from "../protocol/index.js";
|
||||
import type { GatewayRequestHandlers } from "./types.js";
|
||||
|
||||
function assertConfiguredAnnounceChannel(params: {
|
||||
cfg: OpenClawConfig;
|
||||
channel?: string;
|
||||
field: "delivery.channel" | "delivery.failureDestination.channel";
|
||||
}) {
|
||||
if (params.channel === "last") {
|
||||
return;
|
||||
}
|
||||
|
||||
const configuredChannels = listPotentialConfiguredChannelIds(params.cfg, process.env, {
|
||||
includePersistedAuthState: false,
|
||||
}).toSorted();
|
||||
const normalizedChannel = normalizeMessageChannel(params.channel);
|
||||
if (!normalizedChannel) {
|
||||
if (configuredChannels.length <= 1) {
|
||||
return;
|
||||
}
|
||||
throw new Error(
|
||||
`${params.field} is required when multiple channels are configured: ${configuredChannels.join(", ")}`,
|
||||
);
|
||||
}
|
||||
|
||||
if (configuredChannels.length === 0) {
|
||||
return;
|
||||
}
|
||||
|
||||
if (configuredChannels.includes(normalizedChannel)) {
|
||||
return;
|
||||
}
|
||||
|
||||
throw new Error(`${params.field} must be one of: ${configuredChannels.join(", ")}`);
|
||||
}
|
||||
|
||||
function assertValidCronAnnounceDelivery(params: { cfg: OpenClawConfig; delivery?: CronDelivery }) {
|
||||
if (params.delivery?.mode === "announce") {
|
||||
assertConfiguredAnnounceChannel({
|
||||
cfg: params.cfg,
|
||||
channel: params.delivery.channel,
|
||||
field: "delivery.channel",
|
||||
});
|
||||
}
|
||||
|
||||
const failureDestination = params.delivery?.failureDestination;
|
||||
if (failureDestination && (failureDestination.mode ?? "announce") === "announce") {
|
||||
assertConfiguredAnnounceChannel({
|
||||
cfg: params.cfg,
|
||||
channel: failureDestination.channel,
|
||||
field: "delivery.failureDestination.channel",
|
||||
});
|
||||
}
|
||||
}
|
||||
|
||||
function assertValidCronCreateDelivery(cfg: OpenClawConfig, jobCreate: CronJobCreate) {
|
||||
assertValidCronAnnounceDelivery({
|
||||
cfg,
|
||||
delivery: jobCreate.delivery,
|
||||
});
|
||||
}
|
||||
|
||||
function assertValidCronUpdateDelivery(params: {
|
||||
cfg: OpenClawConfig;
|
||||
currentJob: CronJob | undefined;
|
||||
patch: CronJobPatch;
|
||||
}) {
|
||||
if (!params.currentJob || !("delivery" in params.patch)) {
|
||||
return;
|
||||
}
|
||||
|
||||
const nextJob = structuredClone(params.currentJob);
|
||||
applyJobPatch(nextJob, params.patch, {
|
||||
defaultAgentId: resolveDefaultAgentId(params.cfg),
|
||||
});
|
||||
assertValidCronAnnounceDelivery({
|
||||
cfg: params.cfg,
|
||||
delivery: nextJob.delivery,
|
||||
});
|
||||
}
|
||||
|
||||
export const cronHandlers: GatewayRequestHandlers = {
|
||||
wake: ({ params, respond, context }) => {
|
||||
if (!validateWakeParams(params)) {
|
||||
@@ -124,6 +208,7 @@ export const cronHandlers: GatewayRequestHandlers = {
|
||||
return;
|
||||
}
|
||||
const jobCreate = normalized as unknown as CronJobCreate;
|
||||
const cfg = loadConfig();
|
||||
const timestampValidation = validateScheduleTimestamp(jobCreate.schedule);
|
||||
if (!timestampValidation.ok) {
|
||||
respond(
|
||||
@@ -133,6 +218,19 @@ export const cronHandlers: GatewayRequestHandlers = {
|
||||
);
|
||||
return;
|
||||
}
|
||||
try {
|
||||
assertValidCronCreateDelivery(cfg, jobCreate);
|
||||
} catch (err) {
|
||||
respond(
|
||||
false,
|
||||
undefined,
|
||||
errorShape(
|
||||
ErrorCodes.INVALID_REQUEST,
|
||||
`invalid cron.add params: ${formatErrorMessage(err)}`,
|
||||
),
|
||||
);
|
||||
return;
|
||||
}
|
||||
const job = await context.cron.add(jobCreate);
|
||||
context.logGateway.info("cron: job created", { jobId: job.id, schedule: jobCreate.schedule });
|
||||
respond(true, job, undefined);
|
||||
@@ -182,6 +280,7 @@ export const cronHandlers: GatewayRequestHandlers = {
|
||||
return;
|
||||
}
|
||||
const patch = p.patch as unknown as CronJobPatch;
|
||||
const cfg = loadConfig();
|
||||
if (patch.schedule) {
|
||||
const timestampValidation = validateScheduleTimestamp(patch.schedule);
|
||||
if (!timestampValidation.ok) {
|
||||
@@ -193,6 +292,23 @@ export const cronHandlers: GatewayRequestHandlers = {
|
||||
return;
|
||||
}
|
||||
}
|
||||
try {
|
||||
assertValidCronUpdateDelivery({
|
||||
cfg,
|
||||
currentJob: context.cron.getJob(jobId),
|
||||
patch,
|
||||
});
|
||||
} catch (err) {
|
||||
respond(
|
||||
false,
|
||||
undefined,
|
||||
errorShape(
|
||||
ErrorCodes.INVALID_REQUEST,
|
||||
`invalid cron.update params: ${formatErrorMessage(err)}`,
|
||||
),
|
||||
);
|
||||
return;
|
||||
}
|
||||
const job = await context.cron.update(jobId, patch);
|
||||
context.logGateway.info("cron: job updated", { jobId });
|
||||
respond(true, job, undefined);
|
||||
|
||||
@@ -528,6 +528,198 @@ describe("gateway server cron", () => {
|
||||
}
|
||||
});
|
||||
|
||||
test("keeps delivery updates valid for main jobs owned by an explicit default agent", async () => {
|
||||
const { prevSkipCron } = await setupCronTestRun({
|
||||
tempPrefix: "openclaw-gw-cron-main-default-agent-delivery-",
|
||||
cronEnabled: false,
|
||||
});
|
||||
|
||||
await writeCronConfig({
|
||||
session: {
|
||||
mainKey: "main",
|
||||
},
|
||||
agents: {
|
||||
list: [{ id: "ops", default: true }],
|
||||
},
|
||||
channels: {
|
||||
telegram: {
|
||||
botToken: "telegram-token",
|
||||
},
|
||||
},
|
||||
});
|
||||
|
||||
const { server, ws } = await startServerWithClient();
|
||||
await connectOk(ws);
|
||||
|
||||
try {
|
||||
const addRes = await rpcReq(ws, "cron.add", {
|
||||
name: "main default agent",
|
||||
enabled: true,
|
||||
schedule: { kind: "every", everyMs: 60_000 },
|
||||
sessionTarget: "main",
|
||||
agentId: "ops",
|
||||
wakeMode: "next-heartbeat",
|
||||
payload: { kind: "systemEvent", text: "hello" },
|
||||
});
|
||||
expect(addRes.ok).toBe(true);
|
||||
const jobIdValue = (addRes.payload as { id?: unknown } | null)?.id;
|
||||
const jobId = typeof jobIdValue === "string" ? jobIdValue : "";
|
||||
expect(jobId.length > 0).toBe(true);
|
||||
|
||||
const updateRes = await rpcReq(ws, "cron.update", {
|
||||
id: jobId,
|
||||
patch: {
|
||||
delivery: { mode: "announce", channel: "telegram", to: "19098680" },
|
||||
},
|
||||
});
|
||||
|
||||
expect(updateRes.ok).toBe(true);
|
||||
const updated = updateRes.payload as { delivery?: unknown } | undefined;
|
||||
expect(updated?.delivery).toBeUndefined();
|
||||
} finally {
|
||||
await cleanupCronTestRun({ ws, server, prevSkipCron });
|
||||
}
|
||||
});
|
||||
|
||||
test("rejects ambiguous announce delivery on add when multiple channels are configured", async () => {
|
||||
const { prevSkipCron } = await setupCronTestRun({
|
||||
tempPrefix: "openclaw-gw-cron-ambiguous-delivery-add-",
|
||||
cronEnabled: false,
|
||||
});
|
||||
|
||||
await writeCronConfig({
|
||||
session: {
|
||||
mainKey: "main",
|
||||
},
|
||||
channels: {
|
||||
telegram: {
|
||||
botToken: "telegram-token",
|
||||
},
|
||||
slack: {
|
||||
botToken: "xoxb-slack-token",
|
||||
appToken: "xapp-slack-token",
|
||||
},
|
||||
},
|
||||
});
|
||||
|
||||
const { server, ws } = await startServerWithClient();
|
||||
await connectOk(ws);
|
||||
|
||||
try {
|
||||
const addRes = await rpcReq(ws, "cron.add", {
|
||||
name: "ambiguous announce add",
|
||||
enabled: true,
|
||||
schedule: { kind: "every", everyMs: 60_000 },
|
||||
sessionTarget: "isolated",
|
||||
wakeMode: "next-heartbeat",
|
||||
payload: { kind: "agentTurn", message: "hello" },
|
||||
delivery: { mode: "announce" },
|
||||
});
|
||||
|
||||
expect(addRes.ok).toBe(false);
|
||||
expect(addRes.error?.message).toContain("delivery.channel is required");
|
||||
} finally {
|
||||
await cleanupCronTestRun({ ws, server, prevSkipCron });
|
||||
}
|
||||
});
|
||||
|
||||
test("rejects ambiguous announce delivery on update when multiple channels are configured", async () => {
|
||||
const { prevSkipCron } = await setupCronTestRun({
|
||||
tempPrefix: "openclaw-gw-cron-ambiguous-delivery-update-",
|
||||
cronEnabled: false,
|
||||
});
|
||||
|
||||
await writeCronConfig({
|
||||
session: {
|
||||
mainKey: "main",
|
||||
},
|
||||
channels: {
|
||||
telegram: {
|
||||
botToken: "telegram-token",
|
||||
},
|
||||
slack: {
|
||||
botToken: "xoxb-slack-token",
|
||||
appToken: "xapp-slack-token",
|
||||
},
|
||||
},
|
||||
});
|
||||
|
||||
const { server, ws } = await startServerWithClient();
|
||||
await connectOk(ws);
|
||||
|
||||
try {
|
||||
const addRes = await rpcReq(ws, "cron.add", {
|
||||
name: "ambiguous announce update",
|
||||
enabled: true,
|
||||
schedule: { kind: "every", everyMs: 60_000 },
|
||||
sessionTarget: "isolated",
|
||||
wakeMode: "next-heartbeat",
|
||||
payload: { kind: "agentTurn", message: "hello" },
|
||||
delivery: { mode: "none" },
|
||||
});
|
||||
expect(addRes.ok).toBe(true);
|
||||
const jobIdValue = (addRes.payload as { id?: unknown } | null)?.id;
|
||||
const jobId = typeof jobIdValue === "string" ? jobIdValue : "";
|
||||
expect(jobId.length > 0).toBe(true);
|
||||
|
||||
const updateRes = await rpcReq(ws, "cron.update", {
|
||||
id: jobId,
|
||||
patch: {
|
||||
delivery: { mode: "announce" },
|
||||
},
|
||||
});
|
||||
|
||||
expect(updateRes.ok).toBe(false);
|
||||
expect(updateRes.error?.message).toContain("delivery.channel is required");
|
||||
} finally {
|
||||
await cleanupCronTestRun({ ws, server, prevSkipCron });
|
||||
}
|
||||
});
|
||||
|
||||
test("rejects target ids mistakenly supplied as delivery.channel providers", async () => {
|
||||
const { prevSkipCron } = await setupCronTestRun({
|
||||
tempPrefix: "openclaw-gw-cron-invalid-delivery-provider-",
|
||||
cronEnabled: false,
|
||||
});
|
||||
|
||||
await writeCronConfig({
|
||||
session: {
|
||||
mainKey: "main",
|
||||
},
|
||||
channels: {
|
||||
slack: {
|
||||
botToken: "xoxb-slack-token",
|
||||
appToken: "xapp-slack-token",
|
||||
},
|
||||
},
|
||||
});
|
||||
|
||||
const { server, ws } = await startServerWithClient();
|
||||
await connectOk(ws);
|
||||
|
||||
try {
|
||||
const addRes = await rpcReq(ws, "cron.add", {
|
||||
name: "invalid delivery provider",
|
||||
enabled: true,
|
||||
schedule: { kind: "every", everyMs: 60_000 },
|
||||
sessionTarget: "isolated",
|
||||
wakeMode: "next-heartbeat",
|
||||
payload: { kind: "agentTurn", message: "hello" },
|
||||
delivery: {
|
||||
mode: "announce",
|
||||
channel: "C0AT2Q238MQ",
|
||||
to: "C0AT2Q238MQ",
|
||||
},
|
||||
});
|
||||
|
||||
expect(addRes.ok).toBe(false);
|
||||
expect(addRes.error?.message).toContain("delivery.channel");
|
||||
expect(addRes.error?.message).toContain("slack");
|
||||
} finally {
|
||||
await cleanupCronTestRun({ ws, server, prevSkipCron });
|
||||
}
|
||||
});
|
||||
|
||||
test("writes cron run history and auto-runs due jobs", async () => {
|
||||
const { prevSkipCron } = await setupCronTestRun({
|
||||
tempPrefix: "openclaw-gw-cron-log-",
|
||||
|
||||
Reference in New Issue
Block a user