chore: validate alert payload before saving#9872
chore: validate alert payload before saving#9872abhishekhugetech wants to merge 119 commits intomainfrom
Conversation
…iven columnName till the very end to return the origin column with given name
… for origin fields
…lert_payload_before_saving
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
|
This turned out to be pretty big, and too many touching parts. I am wondering if we should break this down. |
| } | ||
| } | ||
| return nil | ||
| } |
There was a problem hiding this comment.
ClickHouse validation uses different variable patterns than runtime
High Severity
The validateClickHouseQuery function only handles Go template syntax ({{.key}}), but runtime code in parser.go first replaces {{key}} (without dot), [[key]], and $key patterns via string replacement before Go template execution. Queries using {{start_timestamp}} (the common pattern without the dot) will fail validation with "failed to execute clickhouse query template" because Go templates interpret {{start_timestamp}} as a function call rather than a variable access. This rejects valid queries that work correctly at runtime.
| } | ||
| if strings.TrimSpace(q.On) == "" { | ||
| return errors.NewInvalidInputf(errors.CodeInvalidInput, "on is required") | ||
| } |
There was a problem hiding this comment.
Cross join validation incorrectly requires ON clause
Medium Severity
The QueryBuilderJoin.Validate() method requires the On field to be non-empty for all join types. However, JoinTypeCross (cross join) is a supported join type that in SQL doesn't require an ON clause - it produces a Cartesian product. Valid cross joins without an On clause would fail validation with "on is required" error.
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
Bugbot Autofix is OFF. To automatically fix reported issues with Cloud Agents, enable Autofix in the Cursor dashboard.
| return errors.NewInvalidInputf(errors.CodeInvalidInput, "on is required") | ||
| } | ||
| return nil | ||
| } |
There was a problem hiding this comment.
Join query references not validated against existing queries
Medium Severity
The QueryBuilderJoin.Validate() method validates that Left.Name and Right.Name are non-empty, but does not validate that these names reference queries that actually exist in the composite query. In contrast, QueryBuilderTraceOperator.ValidateTraceOperator() receives the queries list and validates all referenced queries exist. A join referencing non-existent queries like "X" and "Y" would pass validation but fail at query execution time with a confusing error instead of a clear validation message.
Additional Locations (1)
|
Will be addressed as part of https://github.com/SigNoz/engineering-pod/issues/3199 |
2 participants
📄 Summary
We're breaking the PR into smaller PRs for limiting the surface area for review and reducing blast radius.
✅ Changes
🏷️ Required: Add Relevant Labels
ex:
frontendbackenddevopsbugenhancementuitest👥 Reviewers
🧪 How to Test
🔍 Related Issues
Closes #
📸 Screenshots / Screen Recording (if applicable / mandatory for UI related changes)
📋 Checklist
👀 Notes for Reviewers
clickhouse-sql-parserwhich doesn't complexly validates the syntax of CH queries, need to find some more robust approach to validate the CH queryNote
Strengthens backend validation and error handling for alerts/rules and queries.
CompositeQuery.Validate()with unit checks, PromQL and ClickHouse SQL syntax validation, and "no all-disabled" enforcement; new helpers invalidation.gowith testsFunction.Validate,FunctionName.Validate,JoinType.Validate,QueryBuilderJoin.Validate,QueryBuilderFormula.Validate); exportGetQueryIdentifierBadRequeston invalid inputs; add stricterRuleCondition/PostableRulevalidations (alert/rule type, version, seasonality, selectedQuery)*model.ApiErrordirectlyAssignReservedVarsV3withAssignReservedVarsand update all call sites (parser, threshold rules); pre-render CH templates before parsingconverter(Unit.Validate,IsValidUnit)Written by Cursor Bugbot for commit b8da56b. This will update automatically on new commits. Configure here.