mirror of
https://github.com/openclaw/openclaw.git
synced 2026-08-12 21:53:00 -06:00
fix(cron): reject blank delivery targets
Reject whitespace-only cron delivery target strings before cron input normalization can trim and drop them, so bad delivery targets return INVALID_REQUEST instead of behaving as omitted fields. Keep explicit null update clears for delivery, failure destination, and completion destination fields. Co-authored-by: gaozixiang1 <gaozixiang1@xiaomi.com> Co-authored-by: Lanzhi <lizhan3@xiaomi.com>
This commit is contained in:
@@ -132,6 +132,56 @@ describe("cron protocol validators", () => {
|
||||
).toBe(true);
|
||||
});
|
||||
|
||||
it("rejects blank cron delivery target strings", () => {
|
||||
expect(
|
||||
validateCronAddParams({
|
||||
...minimalAddParams,
|
||||
delivery: {
|
||||
mode: "announce",
|
||||
channel: "telegram",
|
||||
to: " ",
|
||||
},
|
||||
}),
|
||||
).toBe(false);
|
||||
|
||||
expect(
|
||||
validateCronUpdateParams({
|
||||
id: "job-1",
|
||||
patch: {
|
||||
delivery: {
|
||||
channel: "\t",
|
||||
},
|
||||
},
|
||||
}),
|
||||
).toBe(false);
|
||||
|
||||
expect(
|
||||
validateCronUpdateParams({
|
||||
id: "job-1",
|
||||
patch: {
|
||||
delivery: {
|
||||
failureDestination: {
|
||||
channel: null,
|
||||
to: " ",
|
||||
},
|
||||
},
|
||||
},
|
||||
}),
|
||||
).toBe(false);
|
||||
|
||||
expect(
|
||||
validateCronUpdateParams({
|
||||
id: "job-1",
|
||||
patch: {
|
||||
failureAlert: {
|
||||
channel: "last",
|
||||
to: "\n\t",
|
||||
},
|
||||
},
|
||||
}),
|
||||
).toBe(false);
|
||||
});
|
||||
|
||||
it("accepts remove params for id and jobId selectors", () => {
|
||||
expect(validateCronRemoveParams({ id: "job-1" })).toBe(true);
|
||||
expect(validateCronRemoveParams({ jobId: "job-2" })).toBe(true);
|
||||
|
||||
@@ -75,6 +75,8 @@ const CronDeliveryStatusSchema = Type.Union([
|
||||
Type.Literal("unknown"),
|
||||
Type.Literal("not-requested"),
|
||||
]);
|
||||
const NonBlankString = Type.String({ minLength: 1, pattern: "\\S" });
|
||||
const CronAnnounceChannelSchema = Type.Union([Type.Literal("last"), NonBlankString]);
|
||||
const CronFailoverReasonSchema = Type.Union([
|
||||
Type.Literal("auth"),
|
||||
Type.Literal("auth_permanent"),
|
||||
@@ -215,8 +217,8 @@ export const CronPayloadPatchSchema = Type.Union([
|
||||
export const CronFailureAlertSchema = Type.Object(
|
||||
{
|
||||
after: Type.Optional(Type.Integer({ minimum: 1 })),
|
||||
channel: Type.Optional(Type.Union([Type.Literal("last"), NonEmptyString])),
|
||||
to: Type.Optional(Type.String()),
|
||||
channel: Type.Optional(CronAnnounceChannelSchema),
|
||||
to: Type.Optional(NonBlankString),
|
||||
cooldownMs: Type.Optional(Type.Integer({ minimum: 0 })),
|
||||
includeSkipped: Type.Optional(Type.Boolean()),
|
||||
mode: Type.Optional(Type.Union([Type.Literal("announce"), Type.Literal("webhook")])),
|
||||
@@ -227,8 +229,8 @@ export const CronFailureAlertSchema = Type.Object(
|
||||
|
||||
export const CronFailureDestinationSchema = Type.Object(
|
||||
{
|
||||
channel: Type.Optional(Type.Union([Type.Literal("last"), NonEmptyString])),
|
||||
to: Type.Optional(Type.String()),
|
||||
channel: Type.Optional(CronAnnounceChannelSchema),
|
||||
to: Type.Optional(NonBlankString),
|
||||
accountId: Type.Optional(NonEmptyString),
|
||||
mode: Type.Optional(Type.Union([Type.Literal("announce"), Type.Literal("webhook")])),
|
||||
},
|
||||
@@ -237,8 +239,8 @@ export const CronFailureDestinationSchema = Type.Object(
|
||||
|
||||
const CronFailureDestinationPatchSchema = Type.Object(
|
||||
{
|
||||
channel: Type.Optional(Type.Union([Type.Literal("last"), NonEmptyString, Type.Null()])),
|
||||
to: Type.Optional(Type.Union([Type.String(), Type.Null()])),
|
||||
channel: Type.Optional(Type.Union([CronAnnounceChannelSchema, Type.Null()])),
|
||||
to: Type.Optional(Type.Union([NonBlankString, Type.Null()])),
|
||||
accountId: Type.Optional(Type.Union([NonEmptyString, Type.Null()])),
|
||||
mode: Type.Optional(
|
||||
Type.Union([Type.Literal("announce"), Type.Literal("webhook"), Type.Null()]),
|
||||
@@ -250,13 +252,13 @@ const CronFailureDestinationPatchSchema = Type.Object(
|
||||
export const CronCompletionDestinationSchema = Type.Object(
|
||||
{
|
||||
mode: Type.Literal("webhook"),
|
||||
to: NonEmptyString,
|
||||
to: NonBlankString,
|
||||
},
|
||||
{ additionalProperties: false },
|
||||
);
|
||||
|
||||
const CronDeliverySharedProperties = {
|
||||
channel: Type.Optional(Type.Union([Type.Literal("last"), NonEmptyString])),
|
||||
channel: Type.Optional(CronAnnounceChannelSchema),
|
||||
threadId: Type.Optional(Type.Union([Type.String(), Type.Number()])),
|
||||
accountId: Type.Optional(NonEmptyString),
|
||||
bestEffort: Type.Optional(Type.Boolean()),
|
||||
@@ -264,7 +266,7 @@ const CronDeliverySharedProperties = {
|
||||
};
|
||||
|
||||
const CronDeliveryPatchSharedProperties = {
|
||||
channel: Type.Optional(Type.Union([Type.Literal("last"), NonEmptyString, Type.Null()])),
|
||||
channel: Type.Optional(Type.Union([CronAnnounceChannelSchema, Type.Null()])),
|
||||
threadId: Type.Optional(Type.Union([Type.String(), Type.Number(), Type.Null()])),
|
||||
accountId: Type.Optional(Type.Union([NonEmptyString, Type.Null()])),
|
||||
bestEffort: Type.Optional(Type.Boolean()),
|
||||
@@ -275,7 +277,7 @@ const CronDeliveryNoopSchema = Type.Object(
|
||||
{
|
||||
mode: Type.Literal("none"),
|
||||
...CronDeliverySharedProperties,
|
||||
to: Type.Optional(Type.String()),
|
||||
to: Type.Optional(NonBlankString),
|
||||
},
|
||||
{ additionalProperties: false },
|
||||
);
|
||||
@@ -285,7 +287,7 @@ const CronDeliveryAnnounceSchema = Type.Object(
|
||||
mode: Type.Literal("announce"),
|
||||
...CronDeliverySharedProperties,
|
||||
completionDestination: Type.Optional(CronCompletionDestinationSchema),
|
||||
to: Type.Optional(Type.String()),
|
||||
to: Type.Optional(NonBlankString),
|
||||
},
|
||||
{ additionalProperties: false },
|
||||
);
|
||||
@@ -294,7 +296,7 @@ const CronDeliveryWebhookSchema = Type.Object(
|
||||
{
|
||||
mode: Type.Literal("webhook"),
|
||||
...CronDeliverySharedProperties,
|
||||
to: NonEmptyString,
|
||||
to: NonBlankString,
|
||||
},
|
||||
{ additionalProperties: false },
|
||||
);
|
||||
@@ -314,7 +316,7 @@ export const CronDeliveryPatchSchema = Type.Object(
|
||||
completionDestination: Type.Optional(
|
||||
Type.Union([CronCompletionDestinationSchema, Type.Null()]),
|
||||
),
|
||||
to: Type.Optional(Type.Union([Type.String(), Type.Null()])),
|
||||
to: Type.Optional(Type.Union([NonBlankString, Type.Null()])),
|
||||
},
|
||||
{ additionalProperties: false },
|
||||
);
|
||||
|
||||
@@ -633,6 +633,34 @@ describe("cron tool", () => {
|
||||
expect(params?.failureAlert).toBe(false);
|
||||
});
|
||||
|
||||
it.each([
|
||||
["delivery.channel", { channel: " ", to: "chat-1" }],
|
||||
["delivery.to", { mode: "announce", channel: "telegram", to: " \t" }],
|
||||
[
|
||||
"delivery.failureDestination.to",
|
||||
{ mode: "announce", failureDestination: { mode: "announce", to: " " } },
|
||||
],
|
||||
[
|
||||
"delivery.completionDestination.to",
|
||||
{ mode: "announce", completionDestination: { mode: "webhook", to: "\n" } },
|
||||
],
|
||||
])("rejects blank cron.add %s before gateway normalization", async (field, delivery) => {
|
||||
const tool = createTestCronTool();
|
||||
|
||||
await expect(
|
||||
tool.execute("call-blank-delivery-add", {
|
||||
action: "add",
|
||||
job: {
|
||||
name: "reminder",
|
||||
schedule: { at: new Date(123).toISOString() },
|
||||
payload: { kind: "agentTurn", message: "hello" },
|
||||
delivery,
|
||||
},
|
||||
}),
|
||||
).rejects.toThrow(`${field} must be a non-empty string`);
|
||||
expect(callGatewayMock).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("recovers flattened add params for failureAlert and payload extras", async () => {
|
||||
const tool = createTestCronTool();
|
||||
await tool.execute("call-flat-add-extras", {
|
||||
@@ -1336,6 +1364,55 @@ describe("cron tool", () => {
|
||||
expect(params?.patch?.enabled).toBe(false);
|
||||
});
|
||||
|
||||
it.each([
|
||||
["delivery.channel", { channel: " " }],
|
||||
["delivery.to", { to: " " }],
|
||||
["delivery.failureDestination.to", { failureDestination: { to: " " } }],
|
||||
["delivery.completionDestination.to", { completionDestination: { mode: "webhook", to: " " } }],
|
||||
])("rejects blank cron.update %s before gateway normalization", async (field, delivery) => {
|
||||
const tool = createTestCronTool();
|
||||
|
||||
await expect(
|
||||
tool.execute("call-blank-delivery-update", {
|
||||
action: "update",
|
||||
id: "job-blank-delivery",
|
||||
patch: { delivery },
|
||||
}),
|
||||
).rejects.toThrow(`${field} must be a non-empty string`);
|
||||
expect(callGatewayMock).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("passes nullable cron.update delivery clears through to the gateway", async () => {
|
||||
const tool = createTestCronTool();
|
||||
await tool.execute("call-null-delivery-update", {
|
||||
action: "update",
|
||||
id: "job-clear-delivery",
|
||||
patch: {
|
||||
delivery: {
|
||||
channel: null,
|
||||
to: null,
|
||||
failureDestination: null,
|
||||
completionDestination: null,
|
||||
},
|
||||
},
|
||||
});
|
||||
|
||||
const params = expectSingleGatewayCallMethod("cron.update") as
|
||||
| { id?: string; patch?: { delivery?: unknown } }
|
||||
| undefined;
|
||||
expect(params).toEqual({
|
||||
id: "job-clear-delivery",
|
||||
patch: {
|
||||
delivery: {
|
||||
channel: null,
|
||||
to: null,
|
||||
failureDestination: null,
|
||||
completionDestination: null,
|
||||
},
|
||||
},
|
||||
});
|
||||
});
|
||||
|
||||
it("recovers additional flat patch params for update action", async () => {
|
||||
callGatewayMock.mockResolvedValueOnce({ ok: true });
|
||||
|
||||
|
||||
@@ -2,6 +2,7 @@ import { normalizeLowercaseStringOrEmpty } from "@openclaw/normalization-core/st
|
||||
import { Type, type TSchema } from "typebox";
|
||||
import { getRuntimeConfig } from "../../config/config.js";
|
||||
import { resolveCronCreationDelivery } from "../../cron/delivery-context.js";
|
||||
import { assertCronDeliveryInputNonBlankFields } from "../../cron/delivery-target-validation.js";
|
||||
import { normalizeCronJobCreate, normalizeCronJobPatch } from "../../cron/normalize.js";
|
||||
import type { CronDelivery } from "../../cron/types.js";
|
||||
import { normalizeHttpWebhookUrl } from "../../cron/webhook-url.js";
|
||||
@@ -651,6 +652,7 @@ Use jobId canonical; id accepted compat. contextMessages (0-10) adds previous me
|
||||
throw new Error("job required");
|
||||
}
|
||||
const canonicalJob = canonicalizeCronToolObject(params.job as Record<string, unknown>);
|
||||
assertCronDeliveryInputNonBlankFields(canonicalJob.delivery);
|
||||
const job =
|
||||
normalizeCronJobCreate(canonicalJob, {
|
||||
sessionContext: { sessionKey: opts?.agentSessionKey },
|
||||
@@ -767,6 +769,7 @@ Use jobId canonical; id accepted compat. contextMessages (0-10) adds previous me
|
||||
const canonicalPatch = canonicalizeCronToolObject(
|
||||
params.patch as Record<string, unknown>,
|
||||
);
|
||||
assertCronDeliveryInputNonBlankFields(canonicalPatch.delivery);
|
||||
const patch = normalizeCronJobPatch(canonicalPatch) ?? canonicalPatch;
|
||||
if (recoveredFlatPatch && isEmptyRecoveredCronPatch(patch)) {
|
||||
throw new Error("patch required");
|
||||
|
||||
@@ -0,0 +1,36 @@
|
||||
function assertNonBlankStringField(field: string, value: unknown) {
|
||||
if (value === undefined || value === null || typeof value !== "string") {
|
||||
return;
|
||||
}
|
||||
if (value.trim()) {
|
||||
return;
|
||||
}
|
||||
throw new Error(`${field} must be a non-empty string`);
|
||||
}
|
||||
|
||||
export function assertCronDeliveryInputNonBlankFields(delivery: unknown, fieldPrefix = "delivery") {
|
||||
if (!delivery || typeof delivery !== "object") {
|
||||
return;
|
||||
}
|
||||
const deliveryRecord = delivery as {
|
||||
channel?: unknown;
|
||||
to?: unknown;
|
||||
failureDestination?: unknown;
|
||||
completionDestination?: unknown;
|
||||
};
|
||||
assertNonBlankStringField(`${fieldPrefix}.channel`, deliveryRecord.channel);
|
||||
assertNonBlankStringField(`${fieldPrefix}.to`, deliveryRecord.to);
|
||||
|
||||
const failureDestination = deliveryRecord.failureDestination;
|
||||
if (failureDestination && typeof failureDestination === "object") {
|
||||
const failureRecord = failureDestination as { channel?: unknown; to?: unknown };
|
||||
assertNonBlankStringField(`${fieldPrefix}.failureDestination.channel`, failureRecord.channel);
|
||||
assertNonBlankStringField(`${fieldPrefix}.failureDestination.to`, failureRecord.to);
|
||||
}
|
||||
|
||||
const completionDestination = deliveryRecord.completionDestination;
|
||||
if (completionDestination && typeof completionDestination === "object") {
|
||||
const completionRecord = completionDestination as { to?: unknown };
|
||||
assertNonBlankStringField(`${fieldPrefix}.completionDestination.to`, completionRecord.to);
|
||||
}
|
||||
}
|
||||
@@ -14,6 +14,7 @@ import {
|
||||
} from "../../../packages/gateway-protocol/src/index.js";
|
||||
import type { OpenClawConfig } from "../../config/types.openclaw.js";
|
||||
import { resolveCronDeliveryPreviews } from "../../cron/delivery-preview.js";
|
||||
import { assertCronDeliveryInputNonBlankFields } from "../../cron/delivery-target-validation.js";
|
||||
import { normalizeCronJobCreate, normalizeCronJobPatch } from "../../cron/normalize.js";
|
||||
import {
|
||||
isInvalidCronRunLogJobIdError,
|
||||
@@ -364,6 +365,7 @@ export const cronHandlers: GatewayRequestHandlers = {
|
||||
: undefined;
|
||||
let normalized: unknown;
|
||||
try {
|
||||
assertCronDeliveryInputNonBlankFields((params as { delivery?: unknown } | null)?.delivery);
|
||||
normalized =
|
||||
normalizeCronJobCreate(params, {
|
||||
sessionContext: { sessionKey },
|
||||
@@ -441,7 +443,13 @@ export const cronHandlers: GatewayRequestHandlers = {
|
||||
"cron.update": async ({ params, respond, context }) => {
|
||||
let normalizedPatch: ReturnType<typeof normalizeCronJobPatch>;
|
||||
try {
|
||||
normalizedPatch = normalizeCronJobPatch((params as { patch?: unknown } | null)?.patch);
|
||||
const rawPatch = (params as { patch?: unknown } | null)?.patch;
|
||||
assertCronDeliveryInputNonBlankFields(
|
||||
rawPatch && typeof rawPatch === "object"
|
||||
? (rawPatch as { delivery?: unknown }).delivery
|
||||
: undefined,
|
||||
);
|
||||
normalizedPatch = normalizeCronJobPatch(rawPatch);
|
||||
} catch (err) {
|
||||
respond(
|
||||
false,
|
||||
|
||||
@@ -507,6 +507,48 @@ describe("cron method validation", () => {
|
||||
expectCronSuccess(respond);
|
||||
});
|
||||
|
||||
it("rejects blank announce delivery fields before normalization", async () => {
|
||||
const { context, respond } = await invokeCronAdd(
|
||||
agentTurnCronParams({
|
||||
name: "blank delivery target",
|
||||
delivery: {
|
||||
mode: "announce",
|
||||
channel: "telegram",
|
||||
to: " ",
|
||||
},
|
||||
}),
|
||||
);
|
||||
|
||||
expect(context.cron.add).not.toHaveBeenCalled();
|
||||
expectResponseError(respond, {
|
||||
code: "INVALID_REQUEST",
|
||||
messageIncludes: "delivery.to must be a non-empty string",
|
||||
});
|
||||
});
|
||||
|
||||
it("rejects blank failure destination fields before normalization", async () => {
|
||||
const { context, respond } = await invokeCronAdd(
|
||||
agentTurnCronParams({
|
||||
name: "blank failure target",
|
||||
delivery: {
|
||||
mode: "announce",
|
||||
channel: "telegram",
|
||||
to: "telegram:123",
|
||||
failureDestination: {
|
||||
mode: "announce",
|
||||
channel: " ",
|
||||
},
|
||||
},
|
||||
}),
|
||||
);
|
||||
|
||||
expect(context.cron.add).not.toHaveBeenCalled();
|
||||
expectResponseError(respond, {
|
||||
code: "INVALID_REQUEST",
|
||||
messageIncludes: "delivery.failureDestination.channel must be a non-empty string",
|
||||
});
|
||||
});
|
||||
|
||||
it("rejects announce targets prefixed for a different explicit delivery channel", async () => {
|
||||
setRuntimeConfig(telegramSlackConfig());
|
||||
|
||||
@@ -602,6 +644,53 @@ describe("cron method validation", () => {
|
||||
expect(clearDelivery.completionDestination).toBeNull();
|
||||
});
|
||||
|
||||
it("rejects blank delivery target patches before normalization", async () => {
|
||||
const { context, respond } = await invokeCronUpdate(
|
||||
{
|
||||
id: "cron-1",
|
||||
patch: {
|
||||
delivery: {
|
||||
to: "\t",
|
||||
},
|
||||
},
|
||||
},
|
||||
createCronJob({
|
||||
delivery: { mode: "announce", channel: "telegram", to: "telegram:123" },
|
||||
}),
|
||||
);
|
||||
|
||||
expect(context.cron.update).not.toHaveBeenCalled();
|
||||
expectResponseError(respond, {
|
||||
code: "INVALID_REQUEST",
|
||||
messageIncludes: "delivery.to must be a non-empty string",
|
||||
});
|
||||
});
|
||||
|
||||
it("rejects blank completion destination patches before normalization", async () => {
|
||||
const { context, respond } = await invokeCronUpdate(
|
||||
{
|
||||
id: "cron-1",
|
||||
patch: {
|
||||
delivery: {
|
||||
completionDestination: {
|
||||
mode: "webhook",
|
||||
to: " ",
|
||||
},
|
||||
},
|
||||
},
|
||||
},
|
||||
createCronJob({
|
||||
delivery: { mode: "announce" },
|
||||
}),
|
||||
);
|
||||
|
||||
expect(context.cron.update).not.toHaveBeenCalled();
|
||||
expectResponseError(respond, {
|
||||
code: "INVALID_REQUEST",
|
||||
messageIncludes: "delivery.completionDestination.to must be a non-empty string",
|
||||
});
|
||||
});
|
||||
|
||||
it("accepts nullable delivery target clears on update", async () => {
|
||||
const { context, respond } = await invokeCronUpdate(
|
||||
{
|
||||
|
||||
Reference in New Issue
Block a user