fix: isolate invalid cron rules during scheduler recovery

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);
+});

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`);
+    }
+  }
+});

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 }] },

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,