cleanup

apps/frontend/src/pages/moderation/technical-review/rules.vue

@@ -434,26 +434,62 @@ const RULE_EDITOR_OPTIONS: Partial<Ace.EditorOptions> = {
 	useSoftTabs: true,
 }
 
-const TEST_TRACES: Labrinth.TechReview.Internal.TestDelphiRuleTrace[] = [
+const TEST_INPUTS: Labrinth.TechReview.Internal.RuleInput[] = [
 	{
-		key: 'known-safe:obfuscated-bootstrap',
+		schema_version: 1,
-		issue_type: 'OBFUSCATED_NAMES',
+		trace: {
-		severity: 'high',
+			key: 'known-safe:obfuscated-bootstrap',
-		jar: 'META-INF/jars/embedded.jar',
+			issue_type: 'OBFUSCATED_NAMES',
-		file_path: 'com/example/Bootstrap.class',
+			severity: 'high',
-		data: {
+			jar: 'META-INF/jars/embedded.jar',
-			confidence: 0.97,
+			file_path: 'com/example/Bootstrap.class',
-			symbol_count: 42,
+			data: {
+				confidence: 0.97,
+				symbol_count: 42,
+			},
+		},
+		scan: {
+			delphi_version: 17,
+		},
+		artifact: {
+			size: 412_892,
+			hashes: {
+				sha1: '0123456789abcdef',
+				sha512: 'fedcba9876543210',
+			},
+		},
+		scope: {
+			project_id: 'example-project',
+			version_id: 'example-version',
+			file_id: 'example-file',
 		},
 	},
 	{
-		key: 'network/known-telemetry-host',
+		schema_version: 1,
-		issue_type: 'SUSPICIOUS_NETWORK_ACCESS',
+		trace: {
-		severity: 'medium',
+			key: 'network/known-telemetry-host',
-		jar: null,
+			issue_type: 'SUSPICIOUS_NETWORK_ACCESS',
-		file_path: 'com/example/Telemetry.class',
+			severity: 'medium',
-		data: {
+			jar: null,
-			host: 'telemetry.example.com',
+			file_path: 'com/example/Telemetry.class',
+			data: {
+				host: 'telemetry.example.com',
+			},
+		},
+		scan: {
+			delphi_version: 18,
+		},
+		artifact: {
+			size: 98_304,
+			hashes: {
+				sha1: 'abcdef0123456789',
+				sha512: '0123456789abcdef',
+			},
+		},
+		scope: {
+			project_id: 'telemetry-project',
+			version_id: 'telemetry-version',
+			file_id: 'telemetry-file',
 		},
 	},
 ]
@@ -511,7 +547,7 @@ const ruleOutputSchemaText = computed(() =>
 	ruleSchema.value ? formatRuleSchema(ruleSchema.value.output, ruleSchema.value.components) : '',
 )
 const previewExamples = computed(() =>
-	TEST_TRACES.map((original, index) => {
+	TEST_INPUTS.map(({ trace: original }, index) => {
 		const effect = ruleTestEffects.value[index] ?? null
 		const effectiveSeverity = effect?.severity ?? original.severity
 		let summary: string
@@ -665,7 +701,7 @@ async function testRule() {
 	try {
 		const response = await client.labrinth.tech_review_internal.testRule({
 			rule,
-			traces: TEST_TRACES,
+			inputs: TEST_INPUTS,
 		})
 		if (requestId !== ruleTestRequestId) return
 

apps/labrinth/src/routes/internal/moderation/tech_review/rules.rs

@@ -1,14 +1,11 @@
-use std::collections::{BTreeMap, HashMap};
-
 use actix_web::{HttpRequest, delete, get, post, put, web};
 use chrono::{DateTime, Utc};
 use eyre::eyre;
 use serde::{Deserialize, Serialize};
+use validator::Validate;
 use xredis::RedisPool;
 
-use super::rules_scan::{
+use super::rules_scan::RuleInput;
-    RuleArtifact, RuleInput, RuleScan, RuleScope, RuleTrace,
-};
 use crate::{
     auth::check_is_moderator_from_headers,
     database::{
@@ -25,13 +22,9 @@ use crate::{
     },
     queue::session::AuthQueue,
     routes::ApiError,
-    util::error::Context,
+    util::{error::Context, validate::validation_errors_to_string},
 };
 
-const MAX_RULE_NAME_LENGTH: usize = 256;
-const MAX_RULE_EXPRESSION_LENGTH: usize = 65_536;
-const MAX_RULE_TEST_TRACES: usize = 10;
-
 pub fn config(cfg: &mut actix_web::web::ServiceConfig) {
     cfg.service(get_rules)
         .service(test_rule)
@@ -75,28 +68,22 @@ pub struct DelphiRuleAffectedDetail {
     pub hidden: bool,
 }
 
-#[derive(Debug, Deserialize, utoipa::ToSchema)]
+#[derive(Debug, Deserialize, Validate, utoipa::ToSchema)]
 pub struct WriteDelphiRule {
+    #[validate(length(min = 1, max = 256))]
     pub name: String,
+    #[validate(length(min = 1, max = 65536))]
     pub rule: String,
     #[serde(default)]
     pub priority: i32,
 }
 
-#[derive(Debug, Deserialize, utoipa::ToSchema)]
+#[derive(Deserialize, Validate, utoipa::ToSchema)]
 pub struct TestDelphiRule {
+    #[validate(length(min = 1, max = 65536))]
     pub rule: String,
-    pub traces: Vec<TestDelphiRuleTrace>,
+    #[validate(length(max = 10))]
-}
+    pub inputs: Vec<RuleInput>,
-
-#[derive(Debug, Deserialize, Serialize, utoipa::ToSchema)]
-pub struct TestDelphiRuleTrace {
-    pub key: String,
-    pub issue_type: String,
-    pub severity: DelphiSeverity,
-    pub jar: Option<String>,
-    pub file_path: String,
-    pub data: HashMap<String, serde_json::Value>,
 }
 
 #[derive(Debug, Serialize, utoipa::ToSchema)]
@@ -120,36 +107,24 @@ struct ValidatedRule {
 }
 
 impl WriteDelphiRule {
-    fn validate(self) -> Result<ValidatedRule, ApiError> {
+    async fn validate(mut self) -> Result<ValidatedRule, ApiError> {
-        let name = self.name.trim().to_string();
+        self.name = self.name.trim().to_string();
-        if name.is_empty() {
+        self.rule = self.rule.trim().to_string();
-            return Err(ApiError::Request(eyre!("rule name cannot be empty")));
+        Validate::validate(&self).map_err(|error| {
-        }
+            ApiError::Validation(validation_errors_to_string(error, None))
-        if name.chars().count() > MAX_RULE_NAME_LENGTH {
-            return Err(ApiError::Request(eyre!(
-                "rule name cannot exceed {MAX_RULE_NAME_LENGTH} characters"
-            )));
-        }
-
-        let rule = self.rule.trim().to_string();
-        if rule.is_empty() {
-            return Err(ApiError::Request(eyre!(
-                "rule expression cannot be empty"
-            )));
-        }
-        if rule.len() > MAX_RULE_EXPRESSION_LENGTH {
-            return Err(ApiError::Request(eyre!(
-                "rule expression cannot exceed {MAX_RULE_EXPRESSION_LENGTH} bytes"
-            )));
-        }
-
-        cel::Program::compile(&rule).map_err(|error| {
-            ApiError::Request(eyre!("invalid cel expression: {error}"))
         })?;
 
+        let expression = self.rule.clone();
+        tokio::task::spawn_blocking(move || cel::Program::compile(&expression))
+            .await
+            .wrap_internal_err("failed to join cel compilation task")?
+            .map_err(|error| {
+                ApiError::Request(eyre!("invalid cel expression: {error}"))
+            })?;
+
         Ok(ValidatedRule {
-            name,
+            name: self.name,
-            rule,
+            rule: self.rule,
             priority: self.priority,
         })
     }
@@ -180,35 +155,27 @@ pub async fn test_rule(
     )
     .await?;
 
-    let request = body.into_inner();
+    let mut request = body.into_inner();
-    let rule = request.rule.trim();
+    request.rule = request.rule.trim().to_string();
-    if rule.is_empty() {
+    request.validate().map_err(|error| {
-        return Err(ApiError::Request(eyre!(
+        ApiError::Validation(validation_errors_to_string(error, None))
-            "rule expression cannot be empty"
-        )));
-    }
-    if rule.len() > MAX_RULE_EXPRESSION_LENGTH {
-        return Err(ApiError::Request(eyre!(
-            "rule expression cannot exceed {MAX_RULE_EXPRESSION_LENGTH} bytes"
-        )));
-    }
-    if request.traces.len() > MAX_RULE_TEST_TRACES {
-        return Err(ApiError::Request(eyre!(
-            "cannot test more than {MAX_RULE_TEST_TRACES} traces at once"
-        )));
-    }
-
-    let program = cel::Program::compile(rule).map_err(|error| {
-        ApiError::Request(eyre!("invalid cel expression: {error}"))
     })?;
-    let mut effects = Vec::with_capacity(request.traces.len());
 
-    for (index, trace) in request.traces.iter().enumerate() {
+    let rule = request.rule;
-        let input = test_rule_input(trace);
+    let program =
+        tokio::task::spawn_blocking(move || cel::Program::compile(&rule))
+            .await
+            .wrap_internal_err("failed to join cel compilation task")?
+            .map_err(|error| {
+                ApiError::Request(eyre!("invalid cel expression: {error}"))
+            })?;
+    let mut effects = Vec::with_capacity(request.inputs.len());
+
+    for (index, input) in request.inputs.iter().enumerate() {
         let effect = super::rules_scan::evaluate_rule(&program, input)
             .map_err(|error| {
                 ApiError::Request(eyre!(
-                    "failed to evaluate test trace {index}: {error}"
+                    "failed to evaluate test input {index}: {error}"
                 ))
             })?;
         effects.push(effect);
@@ -217,33 +184,6 @@ pub async fn test_rule(
     Ok(web::Json(TestDelphiRuleResponse { effects }))
 }
 
-fn test_rule_input(trace: &TestDelphiRuleTrace) -> RuleInput {
-    RuleInput {
-        schema_version: 1,
-        trace: RuleTrace {
-            key: trace.key.clone(),
-            issue_type: trace.issue_type.clone(),
-            severity: trace.severity,
-            jar: trace.jar.clone(),
-            file_path: trace.file_path.clone(),
-            data: trace.data.clone(),
-        },
-        scan: RuleScan { delphi_version: 17 },
-        artifact: RuleArtifact {
-            size: Some(412_892),
-            hashes: BTreeMap::from([
-                ("sha1".to_string(), "0123456789abcdef".to_string()),
-                ("sha512".to_string(), "fedcba9876543210".to_string()),
-            ]),
-        },
-        scope: RuleScope {
-            project_id: Some("example-project".to_string()),
-            version_id: Some("example-version".to_string()),
-            file_id: Some("example-file".to_string()),
-        },
-    }
-}
-
 /// List all Delphi rules that are not pending deletion.
 #[utoipa::path(
 	context_path = "/moderation/tech-review",
@@ -345,26 +285,7 @@ pub async fn get_rules(
 
     let mut response = Vec::<DelphiRule>::new();
     for rule in rules {
-        if response
+        let affected_detail = if let (
-            .last()
-            .is_none_or(|existing| existing.id != rule.id)
-        {
-            response.push(DelphiRule {
-                id: rule.id,
-                name: rule.name,
-                rule: rule.rule,
-                priority: rule.priority,
-                revision: rule.revision,
-                created_at: rule.created_at,
-                updated_at: rule.updated_at,
-                created_by: rule.created_by,
-                updated_by: rule.updated_by,
-                affected_details_count: rule.affected_details_count,
-                affected_details: Vec::new(),
-            });
-        }
-
-        if let (
             Some(detail_id),
             Some(issue_id),
             Some(issue_type),
@@ -381,28 +302,49 @@ pub async fn get_rules(
             rule.original_severity,
             rule.hidden,
         ) {
-            response
+            Some(DelphiRuleAffectedDetail {
-                .last_mut()
+                detail_id,
-                .expect("a delphi rule was inserted above")
+                issue_id,
-                .affected_details
+                project_id: rule.project_id.map(ProjectId::from),
-                .push(DelphiRuleAffectedDetail {
+                project_name: rule.project_name,
-                    detail_id,
+                project_icon_url: rule.project_icon_url,
-                    issue_id,
+                version_id: rule.version_id.map(VersionId::from),
-                    project_id: rule.project_id.map(ProjectId::from),
+                version_name: rule.version_name,
-                    project_name: rule.project_name,
+                version_number: rule.version_number,
-                    project_icon_url: rule.project_icon_url,
+                issue_type,
-                    version_id: rule.version_id.map(VersionId::from),
+                key,
-                    version_name: rule.version_name,
+                jar: rule.jar,
-                    version_number: rule.version_number,
+                file_path,
-                    issue_type,
+                original_severity,
-                    key,
+                severity: rule.effect_severity,
-                    jar: rule.jar,
+                hidden,
-                    file_path,
+            })
-                    original_severity,
+        } else {
-                    severity: rule.effect_severity,
+            None
-                    hidden,
+        };
-                });
+
+        if let Some(existing) = response.last_mut()
+            && existing.id == rule.id
+        {
+            if let Some(affected_detail) = affected_detail {
+                existing.affected_details.push(affected_detail);
+            }
+            continue;
         }
+
+        response.push(DelphiRule {
+            id: rule.id,
+            name: rule.name,
+            rule: rule.rule,
+            priority: rule.priority,
+            revision: rule.revision,
+            created_at: rule.created_at,
+            updated_at: rule.updated_at,
+            created_by: rule.created_by,
+            updated_by: rule.updated_by,
+            affected_details_count: rule.affected_details_count,
+            affected_details: affected_detail.into_iter().collect(),
+        });
     }
 
     Ok(web::Json(response))
@@ -519,7 +461,7 @@ pub async fn create_rule(
         Scopes::PROJECT_WRITE,
     )
     .await?;
-    let rule = body.into_inner().validate()?;
+    let rule = body.into_inner().validate().await?;
     let user_id = user.id.0 as i64;
 
     let rule = sqlx::query!(
@@ -601,7 +543,7 @@ pub async fn update_rule(
     )
     .await?;
     let (id,) = path.into_inner();
-    let rule = body.into_inner().validate()?;
+    let rule = body.into_inner().validate().await?;
     let user_id = user.id.0 as i64;
 
     let rule = sqlx::query!(

apps/labrinth/src/routes/internal/moderation/tech_review/rules_scan.rs

@@ -5,7 +5,7 @@ use ariadne::ids::base62_impl::to_base62;
 use bytes::Bytes;
 use eyre::{Context as _, Result, eyre};
 use futures_util::{StreamExt, TryStreamExt};
-use serde::Serialize;
+use serde::{Deserialize, Serialize};
 use sqlx::types::Json;
 use tokio::sync::mpsc;
 use tokio_stream::wrappers::UnboundedReceiverStream;
@@ -52,7 +52,7 @@ struct RuleScanErrorEvent<'a> {
     message: &'a str,
 }
 
-#[derive(Serialize, utoipa::ToSchema)]
+#[derive(Deserialize, Serialize, utoipa::ToSchema)]
 pub struct RuleInput {
     pub schema_version: u32,
     pub trace: RuleTrace,
@@ -61,7 +61,7 @@ pub struct RuleInput {
     pub scope: RuleScope,
 }
 
-#[derive(Serialize, utoipa::ToSchema)]
+#[derive(Deserialize, Serialize, utoipa::ToSchema)]
 pub struct RuleTrace {
     pub key: String,
     pub issue_type: String,
@@ -71,18 +71,18 @@ pub struct RuleTrace {
     pub data: HashMap<String, serde_json::Value>,
 }
 
-#[derive(Serialize, utoipa::ToSchema)]
+#[derive(Deserialize, Serialize, utoipa::ToSchema)]
 pub struct RuleScan {
     pub delphi_version: i32,
 }
 
-#[derive(Serialize, utoipa::ToSchema)]
+#[derive(Deserialize, Serialize, utoipa::ToSchema)]
 pub struct RuleArtifact {
     pub size: Option<i32>,
     pub hashes: BTreeMap<String, String>,
 }
 
-#[derive(Serialize, utoipa::ToSchema)]
+#[derive(Deserialize, Serialize, utoipa::ToSchema)]
 pub struct RuleScope {
     pub project_id: Option<String>,
     pub version_id: Option<String>,
@@ -238,9 +238,9 @@ pub async fn get_detail_rule_input(
             hashes: detail.hashes.0,
         },
         scope: RuleScope {
-            project_id: detail.project_id.map(to_public_id),
+            project_id: detail.project_id.map(|id| to_base62(id as u64)),
-            version_id: detail.version_id.map(to_public_id),
+            version_id: detail.version_id.map(|id| to_base62(id as u64)),
-            file_id: detail.file_id.map(to_public_id),
+            file_id: detail.file_id.map(|id| to_base62(id as u64)),
         },
     }))
 }
@@ -316,27 +316,28 @@ pub async fn scan_rules(
     actix_web::rt::spawn(async move {
         match run_scan(transaction, &sender).await {
             Ok(summary) => {
-                send_event(
+                let event = RuleScanEvent {
-                    &sender,
+                    phase: "complete",
-                    "complete",
+                    revision: summary.revision,
-                    &RuleScanEvent {
+                    scanned: summary.scanned,
-                        phase: "complete",
+                    total: summary.total,
-                        revision: summary.revision,
+                    effects: summary.effects,
-                        scanned: summary.scanned,
+                };
-                        total: summary.total,
+                if let Ok(data) = serde_json::to_string(&event) {
-                        effects: summary.effects,
+                    let _ = sender.send(Bytes::from(format!(
-                    },
+                        "event: complete\ndata: {data}\n\n"
-                );
+                    )));
+                }
             }
             Err(error) => {
                 tracing::error!(error = ?error, "delphi rule scan failed");
-                send_event(
+                let message = error.to_string();
-                    &sender,
+                let event = RuleScanErrorEvent { message: &message };
-                    "failed",
+                if let Ok(data) = serde_json::to_string(&event) {
-                    &RuleScanErrorEvent {
+                    let _ = sender.send(Bytes::from(format!(
-                        message: &error.to_string(),
+                        "event: failed\ndata: {data}\n\n"
-                    },
+                    )));
-                );
+                }
             }
         }
     });
@@ -422,7 +423,17 @@ async fn run_scan(
 
     let mut effects = Vec::new();
     let mut scanned = 0;
-    send_progress(sender, "scanning", revision, 0, total, 0);
+    let event = RuleScanEvent {
+        phase: "scanning",
+        revision,
+        scanned: 0,
+        total,
+        effects: 0,
+    };
+    if let Ok(data) = serde_json::to_string(&event) {
+        let _ = sender
+            .send(Bytes::from(format!("event: progress\ndata: {data}\n\n")));
+    }
 
     while let Some(detail) = details
         .try_next()
@@ -448,9 +459,9 @@ async fn run_scan(
                 hashes: detail.hashes.0,
             },
             scope: RuleScope {
-                project_id: detail.project_id.map(to_public_id),
+                project_id: detail.project_id.map(|id| to_base62(id as u64)),
-                version_id: detail.version_id.map(to_public_id),
+                version_id: detail.version_id.map(|id| to_base62(id as u64)),
-                file_id: detail.file_id.map(to_public_id),
+                file_id: detail.file_id.map(|id| to_base62(id as u64)),
             },
         };
 
@@ -474,20 +485,34 @@ async fn run_scan(
 
         scanned += 1;
         if scanned % PROGRESS_INTERVAL == 0 || scanned == total {
-            send_progress(
+            let event = RuleScanEvent {
-                sender,
+                phase: "scanning",
-                "scanning",
                 revision,
                 scanned,
                 total,
-                effects.len(),
+                effects: effects.len(),
-            );
+            };
+            if let Ok(data) = serde_json::to_string(&event) {
+                let _ = sender.send(Bytes::from(format!(
+                    "event: progress\ndata: {data}\n\n"
+                )));
+            }
             tokio::task::yield_now().await;
         }
     }
     drop(details);
 
-    send_progress(sender, "publishing", revision, total, total, effects.len());
+    let event = RuleScanEvent {
+        phase: "publishing",
+        revision,
+        scanned: total,
+        total,
+        effects: effects.len(),
+    };
+    if let Ok(data) = serde_json::to_string(&event) {
+        let _ = sender
+            .send(Bytes::from(format!("event: progress\ndata: {data}\n\n")));
+    }
 
     insert_materialized_effects(revision, &effects, &mut transaction).await?;
 
@@ -656,9 +681,9 @@ pub(crate) async fn materialize_current_rule_effects(
                 hashes: detail.hashes.0,
             },
             scope: RuleScope {
-                project_id: detail.project_id.map(to_public_id),
+                project_id: detail.project_id.map(|id| to_base62(id as u64)),
-                version_id: detail.version_id.map(to_public_id),
+                version_id: detail.version_id.map(|id| to_base62(id as u64)),
-                file_id: detail.file_id.map(to_public_id),
+                file_id: detail.file_id.map(|id| to_base62(id as u64)),
             },
         };
 
@@ -687,7 +712,7 @@ pub(crate) async fn materialize_current_rule_effects(
 async fn fetch_compiled_rules(
     transaction: &mut PgTransaction<'_>,
 ) -> Result<Vec<CompiledRule>> {
-    sqlx::query!(
+    let rules = sqlx::query!(
         r#"
         SELECT id AS "id!: DelphiRuleId", rule
         FROM delphi_rules
@@ -697,18 +722,28 @@ async fn fetch_compiled_rules(
     )
     .fetch_all(&mut *transaction)
     .await
-    .wrap_err("failed to fetch delphi rules")?
+    .wrap_err("failed to fetch delphi rules")?;
-    .into_iter()
+
-    .map(|rule| {
+    tokio::task::spawn_blocking(move || {
-        let program = cel::Program::compile(&rule.rule).map_err(|error| {
+        rules
-            eyre!("failed to compile delphi rule {}: {error}", rule.id.0)
+            .into_iter()
-        })?;
+            .map(|rule| {
-        Ok(CompiledRule {
+                let program =
-            id: rule.id,
+                    cel::Program::compile(&rule.rule).map_err(|error| {
-            program,
+                        eyre!(
-        })
+                            "failed to compile delphi rule {}: {error}",
+                            rule.id.0
+                        )
+                    })?;
+                Ok(CompiledRule {
+                    id: rule.id,
+                    program,
+                })
+            })
+            .collect()
     })
-    .collect()
+    .await
+    .wrap_err("failed to join cel compilation task")?
 }
 
 async fn insert_materialized_effects(
@@ -790,40 +825,3 @@ pub(super) fn evaluate_rule(
             .wrap_err("cel expression returned an invalid rule effect"),
     }
 }
-
-fn to_public_id(id: i64) -> String {
-    to_base62(id as u64)
-}
-
-fn send_progress(
-    sender: &mpsc::UnboundedSender<Bytes>,
-    phase: &'static str,
-    revision: i64,
-    scanned: usize,
-    total: usize,
-    effects: usize,
-) {
-    send_event(
-        sender,
-        "progress",
-        &RuleScanEvent {
-            phase,
-            revision,
-            scanned,
-            total,
-            effects,
-        },
-    );
-}
-
-fn send_event(
-    sender: &mpsc::UnboundedSender<Bytes>,
-    event: &str,
-    data: &impl Serialize,
-) {
-    let Ok(data) = serde_json::to_string(data) else {
-        return;
-    };
-    let _ =
-        sender.send(Bytes::from(format!("event: {event}\ndata: {data}\n\n")));
-}

packages/api-client/src/modules/labrinth/types.ts

@@ -2288,18 +2288,9 @@ export namespace Labrinth {
 				priority: number
 			}
 
-			export type TestDelphiRuleTrace = {
-				key: string
-				issue_type: string
-				severity: DelphiSeverity
-				jar: string | null
-				file_path: string
-				data: Record<string, unknown>
-			}
-
 			export type TestDelphiRuleRequest = {
 				rule: string
-				traces: TestDelphiRuleTrace[]
+				inputs: RuleInput[]
 			}
 
 			export type DelphiRuleEffect = {