From c0edb7686ba7bf921462a36c9df827dfcb947c65 Mon Sep 17 00:00:00 2001 From: Aniruddha Adak Date: Tue, 22 Sep 2026 15:19:00 +0530 Subject: [PATCH] fix(service): validate cron schedule before create_cron_job API call create_cron_job forwarded any schedule string to the Render API, so typos like everyday surfaced as opaque downstream errors. Validate the 5-field expression locally (ranges, wildcards, ranges, steps, lists, JAN-DEC and SUN-SAT names) and return a clear tool error naming the bad field. Fixes #31 --- pkg/service/tools.go | 4 + pkg/service/tools_test.go | 31 ++++++++ pkg/validate/params.go | 151 ++++++++++++++++++++++++++++++++++++ pkg/validate/params_test.go | 39 ++++++++++ 4 files changed, 225 insertions(+) diff --git a/pkg/service/tools.go b/pkg/service/tools.go index 0fa095f..0bb961a 100644 --- a/pkg/service/tools.go +++ b/pkg/service/tools.go @@ -619,6 +619,10 @@ func createValidatedCronJobRequest(ctx context.Context, request mcp.CallToolRequ return nil, err } + if err := validate.CronSchedule(schedule); err != nil { + return nil, err + } + cronJobDetailsPOST := client.CronJobDetailsPOST{ Runtime: client.ServiceRuntime(runtime), Schedule: schedule, diff --git a/pkg/service/tools_test.go b/pkg/service/tools_test.go index 45fa647..08e64d4 100644 --- a/pkg/service/tools_test.go +++ b/pkg/service/tools_test.go @@ -430,6 +430,37 @@ func TestCreateServiceRuntimeValidation(t *testing.T) { } } +func TestCreateCronJobScheduleValidation(t *testing.T) { + ctx := createTestContext(t, "own-123") + + valid := map[string]any{ + "name": "test-service", + "runtime": "node", + "buildCommand": "npm install", + "startCommand": "npm start", + "schedule": "*/15 * * * *", + } + request := mcp.CallToolRequest{} + request.Params.Arguments = valid + _, err := createValidatedCronJobRequest(ctx, request) + require.NoError(t, err) + + for _, schedule := range []string{"every day", "99 99 * * *", "0 0 * *", "*/0 * * * *"} { + args := map[string]any{ + "name": "test-service", + "runtime": "node", + "buildCommand": "npm install", + "startCommand": "npm start", + "schedule": schedule, + } + request := mcp.CallToolRequest{} + request.Params.Arguments = args + _, err := createValidatedCronJobRequest(ctx, request) + require.Error(t, err) + assert.Contains(t, err.Error(), "invalid schedule expression") + } +} + func TestCreateCronJobTool(t *testing.T) { ownerId := "own-123456" cronJobName := "test-cron-job" diff --git a/pkg/validate/params.go b/pkg/validate/params.go index 64f3b3c..e2189b8 100644 --- a/pkg/validate/params.go +++ b/pkg/validate/params.go @@ -4,6 +4,8 @@ import ( "errors" "fmt" "slices" + "strconv" + "strings" "github.com/mark3labs/mcp-go/mcp" "github.com/render-oss/render-mcp-server/pkg/client" @@ -176,3 +178,152 @@ func PostgresDiskSizeGb(diskSizeGb int) error { } return fmt.Errorf("diskSizeGb can be 0 for the free plan, otherwise it must be either 1, or a multiple of 5") } + +// cronFieldBounds describes the allowed numeric range for one cron field. +type cronFieldBounds struct { + min int + max int +} + +// CronSchedule validates a 5-field standard cron expression (minute hour +// day-of-month month day-of-week) as accepted by create_cron_job. It supports +// wildcards, single values, ranges, steps, and comma-separated lists, plus +// JAN-DEC month names and SUN-SAT day-of-week names. Day-of-week accepts 0-7 +// (both 0 and 7 mean Sunday). +func CronSchedule(schedule string) error { + const formatHint = "expected 5 fields: minute (0-59) hour (0-23) day of month (1-31) month (1-12) day of week (0-6, Sunday=0)" + + fields := strings.Fields(schedule) + if len(fields) != 5 { + return fmt.Errorf("invalid schedule expression %q: %s", schedule, formatHint) + } + + bounds := []cronFieldBounds{ + {min: 0, max: 59}, + {min: 0, max: 23}, + {min: 1, max: 31}, + {min: 1, max: 12}, + {min: 0, max: 7}, + } + + for i, field := range fields { + if err := cronField(field, bounds[i], i == 3, i == 4); err != nil { + return fmt.Errorf("invalid schedule expression %q: field %d (%q): %w", schedule, i+1, field, err) + } + } + return nil +} + +func cronField(field string, bounds cronFieldBounds, allowMonthNames, allowDowNames bool) error { + if field == "" { + return fmt.Errorf("empty field") + } + for _, item := range strings.Split(field, ",") { + if err := cronItem(item, bounds, allowMonthNames, allowDowNames); err != nil { + return err + } + } + return nil +} + +func cronItem(item string, bounds cronFieldBounds, allowMonthNames, allowDowNames bool) error { + if item == "" { + return fmt.Errorf("empty list entry") + } + base := item + if slash := strings.Index(item, "/"); slash >= 0 { + base = item[:slash] + stepStr := item[slash+1:] + if strings.Contains(stepStr, "/") || stepStr == "" { + return fmt.Errorf("invalid step in %q", item) + } + step, err := strconv.Atoi(stepStr) + if err != nil || step < 1 { + return fmt.Errorf("invalid step in %q: step must be a positive integer", item) + } + if base == "" { + return fmt.Errorf("invalid step in %q: missing base before '/'", item) + } + } + if base == "*" { + return nil + } + if strings.Contains(base, "-") { + parts := strings.Split(base, "-") + if len(parts) != 2 { + return fmt.Errorf("invalid range in %q", item) + } + lo, err := cronValue(parts[0], bounds, allowMonthNames, allowDowNames) + if err != nil { + return err + } + hi, err := cronValue(parts[1], bounds, allowMonthNames, allowDowNames) + if err != nil { + return err + } + if lo > hi { + return fmt.Errorf("invalid range in %q: start must not exceed end", item) + } + return nil + } + _, err := cronValue(base, bounds, allowMonthNames, allowDowNames) + return err +} + +func cronValue(token string, bounds cronFieldBounds, allowMonthNames, allowDowNames bool) (int, error) { + if token == "" { + return 0, fmt.Errorf("empty value") + } + if v, ok := cronNameValue(token, allowMonthNames, allowDowNames); ok { + if v < bounds.min || v > bounds.max { + return 0, fmt.Errorf("value %q out of range (%d-%d)", token, bounds.min, bounds.max) + } + return v, nil + } + if allowMonthNames || allowDowNames { + lower := strings.ToLower(token) + if isAlpha(lower) { + return 0, fmt.Errorf("unknown name %q", token) + } + } + v, err := strconv.Atoi(token) + if err != nil { + return 0, fmt.Errorf("invalid value %q: must be an integer, a range, a step, a list, or *", token) + } + if v < bounds.min || v > bounds.max { + return 0, fmt.Errorf("value %q out of range (%d-%d)", token, bounds.min, bounds.max) + } + return v, nil +} + +func cronNameValue(token string, allowMonthNames, allowDowNames bool) (int, bool) { + lower := strings.ToLower(token) + if allowMonthNames { + if v, ok := map[string]int{ + "jan": 1, "feb": 2, "mar": 3, "apr": 4, "may": 5, "jun": 6, + "jul": 7, "aug": 8, "sep": 9, "oct": 10, "nov": 11, "dec": 12, + }[lower]; ok { + return v, true + } + } + if allowDowNames { + if v, ok := map[string]int{ + "sun": 0, "mon": 1, "tue": 2, "wed": 3, "thu": 4, "fri": 5, "sat": 6, + }[lower]; ok { + return v, true + } + } + return 0, false +} + +func isAlpha(s string) bool { + if s == "" { + return false + } + for _, r := range s { + if (r < 'a' || r > 'z') && (r < 'A' || r > 'Z') { + return false + } + } + return true +} diff --git a/pkg/validate/params_test.go b/pkg/validate/params_test.go index afda803..3489ec2 100644 --- a/pkg/validate/params_test.go +++ b/pkg/validate/params_test.go @@ -89,3 +89,42 @@ func TestPostgresPlan(t *testing.T) { assert.Contains(t, err.Error(), "invalid Postgres plan") }) } + +func TestCronSchedule(t *testing.T) { + for _, schedule := range []string{ + "0 0 * * *", + "*/15 * * * *", + "0 9 * * 1-5", + "0 0 1 * *", + "30 2 1 JAN *", + "0 12 * * MON-FRI", + "5/15 0 * * *", + "0,30 9-17 * * *", + "0 0 * * * ", + } { + t.Run("valid/"+schedule, func(t *testing.T) { + assert.NoError(t, validate.CronSchedule(schedule)) + }) + } + + for _, schedule := range []string{ + "", + "every day", + "0 0 * *", + "0 0 * * * *", + "99 99 * * *", + "60 0 * * *", + "0 24 * * *", + "0 0 0 * *", + "0 0 * 13 *", + "0 0 * * 8", + "*/0 * * * *", + "5-2 * * * *", + } { + t.Run("invalid/"+schedule, func(t *testing.T) { + err := validate.CronSchedule(schedule) + require.Error(t, err) + assert.Contains(t, err.Error(), "invalid schedule expression") + }) + } +}