mirror of
https://github.com/openclaw/openclaw.git
synced 2026-07-21 10:16:44 +00:00
fix(ui): reject nondecimal cron repeat amounts (#107480)
* fix(ui): reject nondecimal cron repeat amounts * fix(ui): satisfy cron LOC ratchet * fix(ui): validate cron intervals exactly Co-authored-by: xingzhou <zhang.guiping@xydigit.com> * chore(pr): stage cron UI proof Co-authored-by: xingzhou <zhang.guiping@xydigit.com> * chore(pr): remove staged cron UI proof Co-authored-by: xingzhou <zhang.guiping@xydigit.com> * fix(ui): remove unused cron type export Co-authored-by: xingzhou <zhang.guiping@xydigit.com> --------- Co-authored-by: Peter Steinberger <steipete@gmail.com> Co-authored-by: Peter Steinberger <peter@steipete.me>
This commit is contained in:
co-authored by
Peter Steinberger
Peter Steinberger
parent
66604df9b6
commit
0c067fbfa4
@@ -0,0 +1,36 @@
|
||||
const CRON_POSITIVE_DECIMAL_RE = /^(?:\d+(?:\.\d*)?|\.\d+)$/u;
|
||||
|
||||
const CRON_EVERY_UNIT_MS = {
|
||||
minutes: 60_000,
|
||||
hours: 3_600_000,
|
||||
days: 86_400_000,
|
||||
} as const;
|
||||
|
||||
export function parseCronEveryMs(
|
||||
value: string,
|
||||
unit: keyof typeof CRON_EVERY_UNIT_MS,
|
||||
): number | undefined {
|
||||
const trimmed = value.trim();
|
||||
if (!CRON_POSITIVE_DECIMAL_RE.test(trimmed)) {
|
||||
return undefined;
|
||||
}
|
||||
const [wholePart, fractionalPart = ""] = trimmed.split(".");
|
||||
const wholeDigits = (wholePart || "0").replace(/^0+/u, "") || "0";
|
||||
const fractionalDigits = fractionalPart.replace(/0+$/u, "");
|
||||
// Days contain 2^10 milliseconds, the largest base-10 scale any supported
|
||||
// unit can cancel after trailing decimal zeroes are removed.
|
||||
if (wholeDigits.length > String(Number.MAX_SAFE_INTEGER).length || fractionalDigits.length > 10) {
|
||||
return undefined;
|
||||
}
|
||||
|
||||
const scale = 10n ** BigInt(fractionalDigits.length);
|
||||
const decimal = BigInt(wholeDigits) * scale + BigInt(fractionalDigits || "0");
|
||||
const scaledMilliseconds = decimal * BigInt(CRON_EVERY_UNIT_MS[unit]);
|
||||
if (scaledMilliseconds % scale !== 0n) {
|
||||
return undefined;
|
||||
}
|
||||
const milliseconds = scaledMilliseconds / scale;
|
||||
return milliseconds > 0n && milliseconds <= BigInt(Number.MAX_SAFE_INTEGER)
|
||||
? Number(milliseconds)
|
||||
: undefined;
|
||||
}
|
||||
@@ -1582,6 +1582,85 @@ describe("cron controller", () => {
|
||||
expect(errors.deliveryTo).toBe("cron.errors.webhookUrlInvalid");
|
||||
});
|
||||
|
||||
it.each(["0x10", "1e3", "+1", String(Number.MAX_SAFE_INTEGER), "0.000001"])(
|
||||
"rejects invalid recurring amounts before submit: %s",
|
||||
async (everyAmount) => {
|
||||
const request = vi.fn(async (method: string) => {
|
||||
if (method === "cron.add") {
|
||||
return { id: "job-nondecimal" };
|
||||
}
|
||||
if (method === "cron.list") {
|
||||
return { jobs: [] };
|
||||
}
|
||||
if (method === "cron.status") {
|
||||
return { enabled: true, jobs: 0, nextWakeAtMs: null };
|
||||
}
|
||||
return {};
|
||||
});
|
||||
const state = createState({
|
||||
client: { request } as unknown as CronState["client"],
|
||||
cronForm: {
|
||||
...DEFAULT_CRON_FORM,
|
||||
name: "decimal interval",
|
||||
everyAmount,
|
||||
payloadText: "run",
|
||||
deliveryMode: "none",
|
||||
},
|
||||
});
|
||||
|
||||
const saved = await addCronJob(state);
|
||||
|
||||
expect(saved.saved).toBe(false);
|
||||
expect(state.cronFieldErrors.everyAmount).toBe("cron.errors.everyAmountInvalid");
|
||||
expect(request).not.toHaveBeenCalled();
|
||||
},
|
||||
);
|
||||
|
||||
it.each([
|
||||
["1.5", "minutes", 90_000],
|
||||
["4.1", "minutes", 246_000],
|
||||
["0.1", "hours", 360_000],
|
||||
["0.000125", "hours", 450],
|
||||
["0.1", "days", 8_640_000],
|
||||
["0.0009765625", "days", 84_375],
|
||||
] as const)(
|
||||
"converts %s %s to safe integer milliseconds",
|
||||
async (everyAmount, everyUnit, expectedEveryMs) => {
|
||||
const request = vi.fn(async (method: string) => {
|
||||
if (method === "cron.add") {
|
||||
return { id: "job-decimal" };
|
||||
}
|
||||
if (method === "cron.list") {
|
||||
return { jobs: [] };
|
||||
}
|
||||
if (method === "cron.status") {
|
||||
return { enabled: true, jobs: 0, nextWakeAtMs: null };
|
||||
}
|
||||
return {};
|
||||
});
|
||||
const state = createState({
|
||||
client: { request } as unknown as CronState["client"],
|
||||
cronForm: {
|
||||
...DEFAULT_CRON_FORM,
|
||||
name: "decimal interval",
|
||||
everyAmount,
|
||||
everyUnit,
|
||||
payloadText: "run",
|
||||
deliveryMode: "none",
|
||||
},
|
||||
});
|
||||
|
||||
const saved = await addCronJob(state);
|
||||
|
||||
expect(saved.saved).toBe(true);
|
||||
const addCall = findRequestCall(request.mock.calls, "cron.add");
|
||||
expect(requestPayload(addCall).schedule).toEqual({
|
||||
kind: "every",
|
||||
everyMs: expectedEveryMs,
|
||||
});
|
||||
},
|
||||
);
|
||||
|
||||
it("does not require cron expression fields for on-exit schedules", () => {
|
||||
const errors = validateCronForm({
|
||||
...DEFAULT_CRON_FORM,
|
||||
|
||||
@@ -24,6 +24,7 @@ import {
|
||||
isMissingOperatorReadScopeError,
|
||||
} from "../gateway-errors.ts";
|
||||
import { normalizeLowercaseStringOrEmpty, sortUniqueStrings } from "../string-coerce.ts";
|
||||
import { parseCronEveryMs } from "./decimal.ts";
|
||||
import { loadCronFailingCount } from "./scope.ts";
|
||||
|
||||
export { loadCronFailingCount, loadCronScopeStats } from "./scope.ts";
|
||||
@@ -293,8 +294,7 @@ export function validateCronForm(form: CronFormState): CronFieldErrors {
|
||||
errors.scheduleAt = "cron.errors.scheduleAtInvalid";
|
||||
}
|
||||
} else if (form.scheduleKind === "every") {
|
||||
const amount = toNumber(form.everyAmount, 0);
|
||||
if (amount <= 0) {
|
||||
if (parseCronEveryMs(form.everyAmount, form.everyUnit) === undefined) {
|
||||
errors.everyAmount = "cron.errors.everyAmountInvalid";
|
||||
}
|
||||
} else if (form.scheduleKind === "cron") {
|
||||
@@ -834,13 +834,11 @@ function buildCronSchedule(form: CronFormState) {
|
||||
return { kind: "at" as const, at: new Date(ms).toISOString() };
|
||||
}
|
||||
if (form.scheduleKind === "every") {
|
||||
const amount = toNumber(form.everyAmount, 0);
|
||||
if (amount <= 0) {
|
||||
const everyMs = parseCronEveryMs(form.everyAmount, form.everyUnit);
|
||||
if (everyMs === undefined) {
|
||||
throw new Error(t("cron.errors.invalidIntervalAmount"));
|
||||
}
|
||||
const unit = form.everyUnit;
|
||||
const mult = unit === "minutes" ? 60_000 : unit === "hours" ? 3_600_000 : 86_400_000;
|
||||
return { kind: "every" as const, everyMs: amount * mult };
|
||||
return { kind: "every" as const, everyMs };
|
||||
}
|
||||
const expr = form.cronExpr.trim();
|
||||
if (!expr) {
|
||||
|
||||
@@ -618,6 +618,16 @@ describe("cron view editor", () => {
|
||||
expect(onceText).toContain("2026");
|
||||
});
|
||||
|
||||
it("hides the schedule summary for recurring amounts that cannot produce safe milliseconds", () => {
|
||||
for (const everyAmount of ["0x10", "1e3", "+1", String(Number.MAX_SAFE_INTEGER), "0.000001"]) {
|
||||
const container = renderView({
|
||||
createOpen: true,
|
||||
form: { ...DEFAULT_CRON_FORM, scheduleKind: "every", everyAmount },
|
||||
});
|
||||
expect(container.querySelector(".cron-schedule-summary")).toBeNull();
|
||||
}
|
||||
});
|
||||
|
||||
it("renders supported delivery options and normalizes stale announce selection", () => {
|
||||
// systemEvent + main session cannot announce; a stale announce selection
|
||||
// must render as none and the announce option must disappear.
|
||||
|
||||
@@ -26,13 +26,14 @@ import "../../components/web-awesome.ts";
|
||||
import "../../components/web-awesome-popover.ts";
|
||||
import { t } from "../../i18n/index.ts";
|
||||
import { isCronJobActiveFailure, resolveCronJobLastRunStatus } from "../../lib/cron-status.ts";
|
||||
import { parseCronEveryMs } from "../../lib/cron/decimal.ts";
|
||||
import type {
|
||||
CronFieldErrors,
|
||||
CronFieldKey,
|
||||
CronFormState,
|
||||
CronJobsLastStatusFilter,
|
||||
CronJobsScheduleKindFilter,
|
||||
} from "../../lib/cron/index.ts";
|
||||
import type { CronFormState } from "../../lib/cron/index.ts";
|
||||
import { formatRelativeTimestamp, formatMs } from "../../lib/format.ts";
|
||||
import { formatCronSchedule } from "../../lib/presenter.ts";
|
||||
import { normalizeStringEntries, uniqueStrings } from "../../lib/string-coerce.ts";
|
||||
@@ -1231,12 +1232,11 @@ function renderGeneralSection(props: CronProps) {
|
||||
);
|
||||
}
|
||||
|
||||
// Human-readable line under the schedule pills; null while inputs are invalid
|
||||
// so the summary never lies about what would be saved.
|
||||
// Human-readable schedule summary; null while invalid so it never disagrees with the saved value.
|
||||
function describeFormSchedule(form: CronFormState): string | null {
|
||||
if (form.scheduleKind === "every") {
|
||||
const amount = form.everyAmount.trim();
|
||||
if (!amount || !Number.isFinite(Number(amount)) || Number(amount) <= 0) {
|
||||
if (parseCronEveryMs(amount, form.everyUnit) === undefined) {
|
||||
return null;
|
||||
}
|
||||
if (Number(amount) === 1) {
|
||||
|
||||
Reference in New Issue
Block a user