From bf63425805bed9248ba17568c07e382c8e205887 Mon Sep 17 00:00:00 2001 From: whyour Date: Sun, 27 Sep 2026 16:45:09 +0800 Subject: [PATCH] fix: isolate invalid cron rules during scheduler recovery --- back/schedule/addCron.ts | 21 +------ back/services/cron.ts | 26 +++++++-- back/shared/cronSchedule.ts | 27 +++++++++ back/validation/schedule.ts | 15 +---- docs/releases/2.22.0.md | 4 +- package.json | 1 + pnpm-lock.yaml | 3 + .../back/cron-schedule-compatibility.test.cjs | 41 ++++++++++++++ test/back/cron-scheduler-routing.test.cjs | 51 +++++++++++++++++ test/back/legacy-cron-recovery.test.cjs | 4 +- test/back/scheduler-reconciliation.test.cjs | 56 ++++++++++++++++++- version.yaml | 3 +- 12 files changed, 211 insertions(+), 41 deletions(-) create mode 100644 back/shared/cronSchedule.ts create mode 100644 test/back/cron-schedule-compatibility.test.cjs create mode 100644 test/back/cron-scheduler-routing.test.cjs diff --git a/back/schedule/addCron.ts b/back/schedule/addCron.ts index 7df2d9a1..bcf7200b 100644 --- a/back/schedule/addCron.ts +++ b/back/schedule/addCron.ts @@ -1,30 +1,15 @@ import { ServerUnaryCall, sendUnaryData, status } from '@grpc/grpc-js'; import { AddCronRequest, AddCronResponse } from '../protos/cron'; import nodeSchedule from 'node-schedule'; -import CronExpressionParser from 'cron-parser'; +import { isValidCronSchedule } from '../shared/cronSchedule'; import { scheduleStacks } from './data'; import { runCron } from '../shared/runCron'; import Logger from '../loaders/logger'; import { tf } from '../shared/i18n'; -/** - * 预校验 cron 表达式，检测 node-schedule 会拒绝但 cron-parser 会接受的 pattern。 - * node-schedule 对 bare /N（字段以 / 开头，如前无星号/数字前缀的 /6）返回 null， - * 提前拦截避免走 scheduleJob 后才发现无效。 - */ +// Validate the entire batch before replacing any existing jobs. const isValidCronField = (cron: string): boolean => { - // 检测 bare /N 模式：字段以 / 开头如 "/6"，或空格后紧跟 "/6" - // node-schedule 会对这种字段返回 null - if (/\s\/\d/.test(cron) || /^\/\d/.test(cron)) { - return false; - } - // 日期和星期字段的 ? 是合法通配符。解析完整表达式，避免将单独的 - // ? 等无效规则放行后，在替换恢复快照时清除已有任务。 - try { - return CronExpressionParser.parse(cron).hasNext(); - } catch { - return false; - } + return isValidCronSchedule(cron); }; const addCron = ( diff --git a/back/services/cron.ts b/back/services/cron.ts index 558c29c7..b259ad01 100644 --- a/back/services/cron.ts +++ b/back/services/cron.ts @@ -1,4 +1,5 @@ import { randomUUID } from 'crypto'; +import { getInvalidCronSchedules } from '../shared/cronSchedule'; import { withSchedulerMutation, schedulerRegistrationError, @@ -46,10 +47,13 @@ export default class CronService { private isNodeCron(cron: Crontab) { const { schedule, extra_schedules } = cron; - if (Number(schedule?.split(/ +/).length) > 5 || extra_schedules?.length) { - return true; - } - return false; + // System crontab only receives portable numeric five-field expressions. + // Extended syntax, macros and legacy shorthand belong to node-schedule. + return ( + schedule?.trim().split(/\s+/).length !== 5 || + /[^\d\s*,/\-]/.test(schedule || '') || + Boolean(extra_schedules?.length) + ); } private get schedulerMode(): 'system' | 'node' { @@ -1075,6 +1079,20 @@ export default class CronService { public async autosave_crontab(requireScheduler = false) { return withSchedulerMutation(async () => { const tabs = await this.crontabs(); + // A bad persisted rule must not block startup or all other schedules. + // Keep the DB row editable; omit it from both runtime scheduler snapshots. + tabs.data = tabs.data.filter((doc) => { + if (doc.isDisabled === 1) return true; + const invalidSchedules = getInvalidCronSchedules(doc); + if (invalidSchedules.length === 0) return true; + this.logger.warn( + '[crontab] Skipping task with invalid schedule (id=%s, name=%s): %s', + doc.id, + doc.name || '', + JSON.stringify(invalidSchedules), + ); + return false; + }); const regularCrons = tabs.data .filter( (x) => diff --git a/back/shared/cronSchedule.ts b/back/shared/cronSchedule.ts new file mode 100644 index 00000000..69d4f2aa --- /dev/null +++ b/back/shared/cronSchedule.ts @@ -0,0 +1,27 @@ +import cronParser from 'cron-parser-v4'; +import { ScheduleType } from '../interface/schedule'; + +// Keep validation aligned with node-schedule's locked cron-parser version. +// cron-parser 5 accepts syntax (e.g. H) that the scheduler cannot execute. +export function isValidCronSchedule(schedule: unknown): schedule is string { + if (typeof schedule !== 'string' || !schedule.trim()) return false; + try { + return cronParser.parseExpression(schedule).hasNext(); + } catch { + return false; + } +} + +export function getInvalidCronSchedules(cron: { + schedule?: string; + extra_schedules?: Array<{ schedule: string }>; +}): unknown[] { + if ( + cron.schedule?.startsWith(ScheduleType.ONCE) || + cron.schedule?.startsWith(ScheduleType.BOOT) + ) { + return []; + } + return [cron.schedule, ...(cron.extra_schedules || []).map((x) => x.schedule)] + .filter((schedule) => !isValidCronSchedule(schedule)); +} diff --git a/back/validation/schedule.ts b/back/validation/schedule.ts index c948b461..c6422c00 100644 --- a/back/validation/schedule.ts +++ b/back/validation/schedule.ts @@ -1,5 +1,5 @@ import { Joi } from 'celebrate'; -import CronExpressionParser from 'cron-parser'; +import { isValidCronSchedule } from '../shared/cronSchedule'; import { ScheduleType } from '../interface/schedule'; import path from 'path'; import config from '../config'; @@ -12,18 +12,7 @@ const validateSchedule = (value: string, helpers: any) => { return value; } - // 检测裸 /N 模式：cron-parser 会接受，但 node-schedule 会返回 null - // 提前拦截，避免任务入库后调度器注册失败 - if (/\s\/\d/.test(value) || /^\/\d/.test(value)) { - return helpers.error('any.invalid'); - } - try { - if (CronExpressionParser.parse(value).hasNext()) { - return value; - } - } catch (e) { - return helpers.error('any.invalid'); - } + if (isValidCronSchedule(value)) return value; return helpers.error('any.invalid'); }; diff --git a/docs/releases/2.22.0.md b/docs/releases/2.22.0.md index dfde248f..d33dd8f0 100644 --- a/docs/releases/2.22.0.md +++ b/docs/releases/2.22.0.md @@ -4,7 +4,9 @@ - 修复历史任务中合法的 `?` 定时表达式被拒绝，导致启动恢复失败、面板健康检查返回 503 的问题（#3078）。主规则和附加规则均保留兼容性，无效规则仍在替换现有任务前拒绝。 - 修复 `ql check` 等 Shell 诊断输出以 `--` 开头时的 `printf: --: invalid option`，补全调度注册失败日志中的错误详情。 -- 已使用真实调度任务验证恢复流程；本地回归 237 项，234 通过、3 项平台相关跳过，后端构建通过。 +- 启动恢复跳过无效主规则或附加规则的任务，记录任务 ID、名称和错误规则；数据库原记录保留可编辑，其他任务与面板正常启动。服务连接故障仍返回 503。 +- 校验使用与实际调度器一致的解析器，避免接受 `H` 等无法执行的语法；五字段扩展规则、宏和历史简写交给 Node 调度器，避免系统 crontab 拒绝整个任务快照。 +- 已对 250 多个表达式样本与真实调度库做一致性校验，验证无效任务隔离和两种调度模式；本地回归 240 项，237 通过、3 项平台相关跳过，后端构建通过。 ## 新增功能 diff --git a/package.json b/package.json index 005fe177..25d356ce 100644 --- a/package.json +++ b/package.json @@ -90,6 +90,7 @@ "compression": "^1.7.4", "cors": "^2.8.5", "cron-parser": "^5.4.0", + "cron-parser-v4": "npm:cron-parser@4.9.0", "cross-spawn": "^7.0.6", "dayjs": "^1.11.13", "dotenv": "^16.4.6", diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index c4247703..e8089d99 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -42,6 +42,9 @@ dependencies: cron-parser: specifier: ^5.4.0 version: 5.4.0 + cron-parser-v4: + specifier: npm:cron-parser@4.9.0 + version: /cron-parser@4.9.0 cross-spawn: specifier: ^7.0.6 version: 7.0.6 diff --git a/test/back/cron-schedule-compatibility.test.cjs b/test/back/cron-schedule-compatibility.test.cjs new file mode 100644 index 00000000..0e74f044 --- /dev/null +++ b/test/back/cron-schedule-compatibility.test.cjs @@ -0,0 +1,41 @@ +const test = require('node:test'); +const assert = require('node:assert/strict'); +const nodeSchedule = require('node-schedule'); +const { isValidCronSchedule } = require('../../back/shared/cronSchedule'); + +test('validation agrees with the actual scheduler across legacy cron syntax', () => { + const candidates = new Set([ + '*', '0', '?', '0 0', '0 0 *', '0 0 * *', + '@yearly', '@annually', '@monthly', '@weekly', '@daily', '@midnight', + '@hourly', '@secondly', '@minutely', '@weekdays', '@weekends', '@reboot', + '0 0 1 1 * 2027', '0 0 0 L * *', '0 0 0 * * 5L', + '0 0 0 * * MON#2', '0 0 12 LW * *', '0 0 12 15W * *', + '0 0 0 ? * MON', '0 0/30 * * * ?', '0 /5 * * * *', + ]); + const fields = [ + ['*', '?', '/5', '*/5', '0/5', '0', '01', '1-5', '1,3', '5-1', 'H', 'H/5', 'H(0-10)', 'L'], + ['*', '?', '/5', '*/5', '0/5', '0', '01', '1-5', '1,3', '5-1', 'H', 'H/5', 'H(0-10)', 'L'], + ['*', '?', '/2', '*/2', '0/2', '0', '01', '1-5', '1,3', '23-2', 'H', 'L'], + ['*', '?', '/2', '*/2', '1/2', '1', '01', '1-5', '1,3', 'L', 'L-1', 'LW', '15W', 'H'], + ['*', '?', '/2', '*/2', '1/2', '1', '01', '1-5', '1,3', 'JAN', 'jan', 'JAN-MAR', 'DEC-FEB', 'H'], + ['*', '?', '/2', '*/2', '0/2', '0', '7', '01', '1-5', '1,3', 'MON', 'mon', 'MON-FRI', 'FRI-MON', '5L', 'L', 'MON#2', '1#5', 'H'], + ]; + fields.forEach((options, index) => options.forEach((field) => { + const values = ['0', '0', '0', '*', '*', '*']; + values[index] = field; + candidates.add(values.join(' ')); + })); + for (const value of [...candidates]) { + if (value.split(' ').length === 6) { + candidates.add(` ${value} `); + candidates.add(value.split(' ').slice(1).join(' ')); + } + } + for (const value of candidates) { + const job = nodeSchedule.scheduleJob(value, () => {}); + const accepted = Boolean(job); + job?.cancel(); + assert.equal(isValidCronSchedule(value), accepted, value); + } + assert.ok(candidates.size > 250); +}); diff --git a/test/back/cron-scheduler-routing.test.cjs b/test/back/cron-scheduler-routing.test.cjs new file mode 100644 index 00000000..6f241697 --- /dev/null +++ b/test/back/cron-scheduler-routing.test.cjs @@ -0,0 +1,51 @@ +const test = require('node:test'); +const assert = require('node:assert/strict'); +const fs = require('node:fs'); +const ts = require('typescript'); +const { ScheduleType } = require('../../back/interface/schedule'); + +test('system and node modes route extended cron once and keep portable rules in crontab', async () => { + const source = fs.readFileSync('back/services/cron.ts', 'utf8'); + const begin = source.indexOf(' private isNodeCron('); + const end = source.indexOf(' private async getLogName', begin); + const saveBegin = source.indexOf(' private async setCrontab('); + const saveEnd = source.indexOf(' public importCrontab(', saveBegin); + const js = ts.transpileModule(`class Fixture {${source.slice(begin, end)}${source.slice(saveBegin, saveEnd)}}; module.exports = Fixture;`, { + compilerOptions: { target: ts.ScriptTarget.ES2020 }, + }).outputText; + for (const mode of ['system', 'node']) { + const module = { exports: {} }; + let file = '', installs = 0; + new Function('module', 'process', 'execSync', 'ScheduleType', 'config', 'writeFileWithLock', 'CrontabModel', js)( + module, { env: { QL_SCHEDULER: mode } }, () => { installs++; }, ScheduleType, + { crontabFile: '/unused' }, async (_, data) => { file = data; }, { update: async () => {} }, + ); + const service = new module.exports(); + service.makeCommand = (row) => `task ${row.id}.js`; + const rows = [ + { id: 'plain', schedule: '*/5 0-23 * * 1,3' }, + { id: 'spaces', schedule: ' 0\t0 * * * ' }, + { id: 'seconds', schedule: '0 0 * * * *' }, + { id: 'question', schedule: '0 0 * * ?' }, + { id: 'last', schedule: '0 0 L * *' }, + { id: 'nth', schedule: '0 0 * * 1#2' }, + { id: 'named', schedule: '0 0 * JAN MON' }, + { id: 'macro', schedule: '@daily' }, + { id: 'short', schedule: '*' }, + { id: 'extra', schedule: '* * * * *', extra_schedules: [{ schedule: '0 * * * *' }] }, + { id: 'once', schedule: '@once' }, + { id: 'boot', schedule: '@boot' }, + ]; + for (const row of rows) { + const special = ['once', 'boot'].includes(row.id); + const portable = ['plain', 'spaces'].includes(row.id); + assert.equal(service.shouldUseCronClient(row), !special && (mode === 'node' || !portable), `${mode}: ${row.id}`); + } + await service.setCrontab({ data: rows, total: rows.length }); + assert.equal(installs, mode === 'system' ? 1 : 0); + for (const id of ['seconds', 'question', 'last', 'nth', 'named', 'macro', 'short', 'extra', 'once', 'boot']) { + const line = file.split('\n').find((line) => line.endsWith(`task ${id}.js`)); + assert.ok(line.startsWith('# '), `${mode}: ${id} cannot also run from system crontab`); + } + } +}); diff --git a/test/back/legacy-cron-recovery.test.cjs b/test/back/legacy-cron-recovery.test.cjs index c25d2a8d..ef6b2f66 100644 --- a/test/back/legacy-cron-recovery.test.cjs +++ b/test/back/legacy-cron-recovery.test.cjs @@ -22,7 +22,7 @@ test('API accepts legacy question-mark schedules but rejects malformed cron', () for (const schedule of [...legacySchedules, '@once', '@boot']) { assert.equal(scheduleSchema.validate(schedule).error, undefined, schedule); } - for (const schedule of ['?', '0 /5 * * * ?', '0 70 * * * ?', 'not a cron']) { + for (const schedule of ['@bogus', '0 /5 * * * ?', '0 70 * * * ?', 'not a cron']) { assert.ok(scheduleSchema.validate(schedule).error, schedule); } }); @@ -57,7 +57,7 @@ test('legacy main and extra schedules restore real jobs and healthy readiness', assert.ok(jobs.every((job) => job.nextInvocation())); } const previousJobs = [...stacks.values()].flat(); - for (const schedule of ['?', '0 /5 * * * ?', '0 70 * * * ?']) { + for (const schedule of ['@bogus', '0 /5 * * * ?', '0 70 * * * ?']) { for (const invalid of [ { ...crons[0], schedule }, { ...crons[0], extra_schedules: [{ schedule }] }, diff --git a/test/back/scheduler-reconciliation.test.cjs b/test/back/scheduler-reconciliation.test.cjs index 1380c623..effeaea1 100644 --- a/test/back/scheduler-reconciliation.test.cjs +++ b/test/back/scheduler-reconciliation.test.cjs @@ -5,6 +5,7 @@ const ts = require('typescript'); const load = require('../helpers/load-security-module.cjs'); const { SchedulerReadiness } = require('../../back/shared/schedulerReadiness'); const { AddCronRequest } = require('../../back/protos/cron'); +const { getInvalidCronSchedules } = require('../../back/shared/cronSchedule'); function fixture(client) { const source = fs.readFileSync('back/services/cron.ts', 'utf8'); @@ -15,11 +16,12 @@ function fixture(client) { { compilerOptions: { target: ts.ScriptTarget.ES2020 } }, ).outputText; const module = { exports: {} }; - new Function('module', 'isDemoEnv', 'cronClient', 'withSchedulerMutation', js)( + new Function('module', 'isDemoEnv', 'cronClient', 'withSchedulerMutation', 'getInvalidCronSchedules', js)( module, () => false, client, (fn) => fn(), + getInvalidCronSchedules, ); const service = new module.exports(); service.setCrontab = async () => {}; @@ -101,7 +103,7 @@ test('invalid replacement leaves the previous schedule intact', async () => { { request: { replace: true, - crons: [{ id: 'bad', schedule: '?', extra_schedules: [] }], + crons: [{ id: 'bad', schedule: 'not a cron', extra_schedules: [] }], }, }, (err) => (err ? reject(err) : resolve()), @@ -111,6 +113,56 @@ test('invalid replacement leaves the previous schedule intact', async () => { assert.deepEqual([...stacks.keys()], ['old']); }); +test('invalid persisted schedules cannot block recovery, valid jobs or HTTP health', async (t) => { + const stacks = new Map(); + const warnings = []; + const { addCron } = load('back/schedule/addCron.ts', { + './data': { scheduleStacks: stacks }, + '../shared/runCron': {}, + '../loaders/logger': { info() {}, warn() {} }, + '../shared/i18n': { tf: require('node:util').format }, + }); + const client = { addCron: (crons, replace) => new Promise((resolve, reject) => { + addCron({ request: { crons, replace } }, (error) => error ? reject(error) : resolve()); + }) }; + const service = fixture(client); + let snapshot; + service.setCrontab = async (tabs) => { snapshot = tabs.data; }; + service.logger.warn = (...args) => warnings.push(require('node:util').format(...args)); + let rows = [ + { id: 'good', name: 'valid', schedule: '0 0/30 * * * ?', isDisabled: 0 }, + { id: 'bad-main', name: 'bad main', schedule: 'not a cron', isDisabled: 0 }, + { id: 'bad-extra', name: 'bad extra', schedule: '* * * * *', extra_schedules: [{ schedule: '0 70 * * * ?' }], isDisabled: 0 }, + { id: 'unsupported', name: 'unsupported', schedule: 'H * * * *', isDisabled: 0 }, + { id: 'disabled', name: 'disabled', schedule: 'bad', isDisabled: 1 }, + ]; + service.crontabs = async () => ({ data: rows }); + const state = new SchedulerReadiness(async () => {}, 60000); + t.after(() => { + clearTimeout(state.retry); + for (const jobs of stacks.values()) for (const job of jobs) job.cancel(); + }); + state.configure(() => service.autosave_crontab(true)); + assert.equal(await state.recover(), true); + assert.deepEqual([...stacks.keys()], ['good']); + assert.deepEqual(snapshot.map((x) => x.id), ['good', 'disabled']); + assert.equal(rows.length, 5, 'persisted rows are preserved for editing'); + assert.equal(rows[1].isDisabled, 0); + assert.equal(warnings.length, 3); + assert.match(warnings.join('\n'), /bad-main.*bad main.*not a cron/); + assert.match(warnings.join('\n'), /bad-extra.*0 70/); + const { HealthService } = load('back/services/health.ts', { + typedi: { Service: () => (x) => x }, './http': {}, + '../schedule/client': { readiness: state }, '../loaders/logger': { error() {} }, + }); + const health = new HealthService({ getServer: () => ({}) }); + assert.equal((await health.check()).status, 'ok'); + rows = rows.filter((x) => x.id !== 'good'); + assert.equal(await state.recover(), true, 'even all-invalid snapshots can recover'); + assert.equal(stacks.size, 0); + assert.equal((await health.check()).status, 'ok'); +}); + test('pre-RPC channel failures invalidate readiness and return 503 without executing or replaying writes', async () => { for (const method of ['addCron', 'delCron']) { let invalidations = 0, diff --git a/version.yaml b/version.yaml index f477d7ef..45328a27 100644 --- a/version.yaml +++ b/version.yaml @@ -1,8 +1,9 @@ version: 2.22.0 changeLogLink: https://github.com/whyour/qinglong/blob/master/docs/releases/2.22.0.md -publishTime: 2026-09-27 1615 +publishTime: 2026-09-27 1643 changeLog: | 修复更新后含 ? 的历史定时规则导致面板 503，以及 ql check 的 printf 报错（#3078） + 无效任务不再阻止面板启动；统一调度表达式校验并修复扩展规则的调度器分配 1. 新增独立远程 CLI，覆盖任务、订阅、环境变量、配置、脚本等 OpenAPI 2. 新增可选面板内部 TypeScript 工具，默认保留 Shell 入口 3. 仪表盘支持查看今日成功和失败任务明细