tju-yxq opened a new issue, #1419:
URL: https://github.com/apache/rocketmq-dashboard/issues/1419

   ## Bug Report
   
   ### Before Creating the Bug Report
   
   - [x] I found a bug, not just asking a question, which should be created in 
[GitHub Discussions](https://github.com/apache/rocketmq/discussions).
   - [x] I have searched the [GitHub 
Issues](https://github.com/apache/rocketmq/issues) and [GitHub 
Discussions](https://github.com/apache/rocketmq/discussions) of this repository 
and believe that this is not a duplicate.
   - [x] I have confirmed that this bug belongs to the current repository, not 
other repositories of RocketMQ.
   
   ### Runtime platform environment
   
   OS: Ubuntu 20.04 / Any OS running RocketMQ Studio
   
   ### RocketMQ version
   
   branch: rocketmq-studio
   version: 5.3.2+
   Git commit id: f727341
   
   ### JDK Version
   
   OpenJDK 21
   
   ### Describe the Bug
   
   `AlertService` does not validate the `operator` field of alert rules before 
injecting it into the Prometheus query expression. The `expression()` method 
builds the PromQL by direct string concatenation:
   
   ```java
   private String expression(AlertRuleVO rule) {
       String metric = hasText(rule.getMetric()) ? rule.getMetric() : 
"rocketmq_consumer_lag_messages";
       String operator = hasText(rule.getOperator()) ? rule.getOperator() : ">";
       return metric + labelSelector(rule) + " " + operator + " " + 
formatThreshold(rule.getThreshold());
   }
   ```
   
   If an admin user enters an operator like `> 0) or 
rocketmq_broker_disk_ratio(`, the generated PromQL becomes:
   ```
   rocketmq_consumer_lag_messages{cluster="..."} > 0) or 
rocketmq_broker_disk_ratio( 1000
   ```
   
   This produces malformed PromQL that can cause Prometheus rule loading to 
fail, or in a more adversarial scenario, evaluate unintended expressions. The 
`metric` field has the same issue - it is also injected without validation.
   
   Additionally, the `duration` field (e.g., `5m`) is not validated against 
Prometheus duration syntax, so an invalid value like `5xyz` would produce a 
rules file that Prometheus rejects.
   
   ### Steps to Reproduce
   
   1. Create an alert rule with operator set to `> 0) or vector(999` and 
threshold `100`.
   2. Export the Prometheus rules YAML.
   3. Load the YAML into Prometheus.
   4. Prometheus fails to parse the rules due to malformed PromQL.
   
   ### What Did You Expect to See?
   
   The operator should be validated against a whitelist of valid comparison 
operators (`>`, `>=`, `<`, `<=`, `==`, `!=`) before being used in the 
expression. The metric name should be validated to only contain valid 
Prometheus metric name characters (`[a-zA-Z_:][a-zA-Z0-9_:]*`). The duration 
should be validated against Prometheus duration format.
   
   ### What Did You See Instead?
   
   Arbitrary text is accepted and injected directly into the PromQL expression, 
which can break Prometheus rule loading.
   
   ### Additional Context
   
   **Affected file**: 
`server/src/main/java/org/apache/rocketmq/studio/ops/alert/AlertService.java`, 
methods `expression()`, `alertName()`, and `duration()`.
   
   **Fix approach**:
   
   1. Validate the operator against a whitelist:
   ```java
   private static final Set<String> VALID_OPERATORS = Set.of(">", ">=", "<", 
"<=", "==", "!=");
   
   private String operator(AlertRuleVO rule) {
       String op = hasText(rule.getOperator()) ? rule.getOperator().trim() : 
">";
       return VALID_OPERATORS.contains(op) ? op : ">";
   }
   ```
   
   2. Validate the metric name with a regex:
   ```java
   private static final Pattern METRIC_NAME_PATTERN = 
Pattern.compile("^[a-zA-Z_:][a-zA-Z0-9_:]*$");
   
   private String metric(AlertRuleVO rule) {
       String metric = hasText(rule.getMetric()) ? rule.getMetric().trim() : 
"rocketmq_consumer_lag_messages";
       return METRIC_NAME_PATTERN.matcher(metric).matches() ? metric : 
"rocketmq_consumer_lag_messages";
   }
   ```
   
   3. Validate the duration against Prometheus duration format:
   ```java
   private static final Pattern DURATION_PATTERN = 
Pattern.compile("^\\d+(ms|s|m|h|d|w|y)$");
   
   private String duration(AlertRuleVO rule) {
       String dur = hasText(rule.getDuration()) ? rule.getDuration().trim() : 
"5m";
       return DURATION_PATTERN.matcher(dur).matches() ? dur : "5m";
   }
   ```
   
   This adds approximately 25 lines (three validation methods + constants) 
without removing any existing logic.
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to