fix(server): add validation for cron expression (#815)

Author: maslow

server/package-lock.json

@@ -42,6 +42,7 @@
     "@prisma/client": "^4.9.0",
     "class-validator": "^0.14.0",
     "compression": "^1.7.4",
+    "cron-validate": "^1.4.5",
     "database-proxy": "^0.8.2",
     "dotenv": "^16.0.3",
     "fast-json-patch": "^3.1.1",

server/package.json

@@ -30,42 +30,47 @@ export class CronJobService {
     const batchApi = this.clusterService.makeBatchV1Api(region)
     const name = `cron-${trigger.id}`
     const command = await this.getTriggerCommand(trigger)
-    const res = await batchApi.createNamespacedCronJob(ns, {
-      metadata: {
-        name,
-        labels: {
-          appid,
-          id: trigger.id,
+    const res = await batchApi
+      .createNamespacedCronJob(ns, {
+        metadata: {
+          name,
+          labels: {
+            appid,
+            id: trigger.id,
+          },
         },
-      },
-      spec: {
-        schedule: trigger.cron,
-        successfulJobsHistoryLimit: 1,
-        failedJobsHistoryLimit: 1,
-        concurrencyPolicy: 'Allow',
-        startingDeadlineSeconds: 60,
-        jobTemplate: {
-          spec: {
-            activeDeadlineSeconds: 60,
-            template: {
-              spec: {
-                restartPolicy: 'Never',
-                terminationGracePeriodSeconds: 30,
-                automountServiceAccountToken: false,
-                containers: [
-                  {
-                    name: name,
-                    image: 'curlimages/curl:7.87.0',
-                    command: ['sh', '-c', command],
-                    imagePullPolicy: 'IfNotPresent',
-                  },
-                ],
+        spec: {
+          schedule: trigger.cron,
+          successfulJobsHistoryLimit: 1,
+          failedJobsHistoryLimit: 1,
+          concurrencyPolicy: 'Allow',
+          startingDeadlineSeconds: 60,
+          jobTemplate: {
+            spec: {
+              activeDeadlineSeconds: 60,
+              template: {
+                spec: {
+                  restartPolicy: 'Never',
+                  terminationGracePeriodSeconds: 30,
+                  automountServiceAccountToken: false,
+                  containers: [
+                    {
+                      name: name,
+                      image: 'curlimages/curl:7.87.0',
+                      command: ['sh', '-c', command],
+                      imagePullPolicy: 'IfNotPresent',
+                    },
+                  ],
+                },
               },
             },
           },
         },
-      },
-    })
+      })
+      .catch((err) => {
+        this.logger.error(`create cronjob ${name} failed:`, err)
+        return null
+      })
 
     this.logger.debug(`create cronjob ${name} success`)
     return res.body
@@ -77,7 +82,10 @@ export class CronJobService {
     const region = await this.regionService.findByAppId(appid)
     const batchApi = this.clusterService.makeBatchV1Api(region)
     const name = `cron-${trigger.id}`
-    const res = await batchApi.readNamespacedCronJob(name, ns)
+    const res = await batchApi.readNamespacedCronJob(name, ns).catch((err) => {
+      this.logger.error(`read cronjob ${name} failed:`, err)
+      return null
+    })
     return res.body
   }
 
@@ -87,7 +95,12 @@ export class CronJobService {
     const region = await this.regionService.findByAppId(appid)
     const batchApi = this.clusterService.makeBatchV1Api(region)
     const name = `cron-${trigger.id}`
-    const res = await batchApi.deleteNamespacedCronJob(name, ns)
+    const res = await batchApi
+      .deleteNamespacedCronJob(name, ns)
+      .catch((err) => {
+        this.logger.error(`delete cronjob ${name} failed:`, err)
+        return null
+      })
     return res.body
   }
 

server/src/trigger/cron-job.service.ts

@@ -1,12 +1,11 @@
 import { Injectable, Logger } from '@nestjs/common'
-import { RegionService } from 'src/region/region.service'
-import * as assert from 'node:assert'
 import { Cron, CronExpression } from '@nestjs/schedule'
 import { times } from 'lodash'
 import { TASK_LOCK_INIT_TIME } from 'src/constants'
 import { SystemDatabase } from 'src/database/system-database'
 import { CronJobService } from './cron-job.service'
 import { CronTrigger, TriggerPhase, TriggerState } from '@prisma/client'
+import * as assert from 'node:assert'
 
 @Injectable()
 export class TriggerTaskService {
@@ -125,7 +124,7 @@ export class TriggerTaskService {
 
     // delete cron job
     const ret = await this.cronService.delete(doc)
-    if (ret.status !== 'Success') return
+    if (ret?.status !== 'Success') return
 
     this.logger.debug('cron job deleted:', doc._id)
 

server/src/trigger/trigger-task.service.ts

@@ -38,6 +38,12 @@ export class TriggerController {
   @UseGuards(JwtAuthGuard, ApplicationAuthGuard)
   @Post()
   async create(@Param('appid') appid: string, @Body() dto: CreateTriggerDto) {
+    // check cron expression
+    const valid = this.triggerService.isValidCronExpression(dto.cron)
+    if (!valid) {
+      return ResponseUtil.error('Invalid cron expression')
+    }
+
     const res = await this.triggerService.create(appid, dto)
     return ResponseUtil.ok(res)
   }