From b92fc10e54b5d71cf136555bbc3230a2e71e4185 Mon Sep 17 00:00:00 2001 From: aecsocket <43144841+aecsocket@users.noreply.github.com> Date: Mon, 27 Jul 2026 17:47:17 +0100 Subject: [PATCH] cleanup --- .../moderation/technical-review/rules.vue | 72 ++++-- .../internal/moderation/tech_review/rules.rs | 230 +++++++----------- .../moderation/tech_review/rules_scan.rs | 176 +++++++------- .../api-client/src/modules/labrinth/types.ts | 11 +- 4 files changed, 228 insertions(+), 261 deletions(-) diff --git a/apps/frontend/src/pages/moderation/technical-review/rules.vue b/apps/frontend/src/pages/moderation/technical-review/rules.vue index dc13669e94..4c18acc654 100644 --- a/apps/frontend/src/pages/moderation/technical-review/rules.vue +++ b/apps/frontend/src/pages/moderation/technical-review/rules.vue @@ -434,26 +434,62 @@ const RULE_EDITOR_OPTIONS: Partial = { useSoftTabs: true, } -const TEST_TRACES: Labrinth.TechReview.Internal.TestDelphiRuleTrace[] = [ +const TEST_INPUTS: Labrinth.TechReview.Internal.RuleInput[] = [ { - key: 'known-safe:obfuscated-bootstrap', - issue_type: 'OBFUSCATED_NAMES', - severity: 'high', - jar: 'META-INF/jars/embedded.jar', - file_path: 'com/example/Bootstrap.class', - data: { - confidence: 0.97, - symbol_count: 42, + schema_version: 1, + trace: { + key: 'known-safe:obfuscated-bootstrap', + issue_type: 'OBFUSCATED_NAMES', + severity: 'high', + jar: 'META-INF/jars/embedded.jar', + file_path: 'com/example/Bootstrap.class', + 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', - issue_type: 'SUSPICIOUS_NETWORK_ACCESS', - severity: 'medium', - jar: null, - file_path: 'com/example/Telemetry.class', - data: { - host: 'telemetry.example.com', + schema_version: 1, + trace: { + key: 'network/known-telemetry-host', + issue_type: 'SUSPICIOUS_NETWORK_ACCESS', + severity: 'medium', + jar: null, + 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 diff --git a/apps/labrinth/src/routes/internal/moderation/tech_review/rules.rs b/apps/labrinth/src/routes/internal/moderation/tech_review/rules.rs index 10a7c0d6fa..7a4e454938 100644 --- a/apps/labrinth/src/routes/internal/moderation/tech_review/rules.rs +++ b/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::{ - RuleArtifact, RuleInput, RuleScan, RuleScope, RuleTrace, -}; +use super::rules_scan::RuleInput; 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, -} - -#[derive(Debug, Deserialize, Serialize, utoipa::ToSchema)] -pub struct TestDelphiRuleTrace { - pub key: String, - pub issue_type: String, - pub severity: DelphiSeverity, - pub jar: Option, - pub file_path: String, - pub data: HashMap, + #[validate(length(max = 10))] + pub inputs: Vec, } #[derive(Debug, Serialize, utoipa::ToSchema)] @@ -120,36 +107,24 @@ struct ValidatedRule { } impl WriteDelphiRule { - fn validate(self) -> Result { - let name = self.name.trim().to_string(); - if name.is_empty() { - return Err(ApiError::Request(eyre!("rule name cannot be empty"))); - } - 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}")) + async fn validate(mut self) -> Result { + self.name = self.name.trim().to_string(); + self.rule = self.rule.trim().to_string(); + Validate::validate(&self).map_err(|error| { + ApiError::Validation(validation_errors_to_string(error, None)) })?; + 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, - rule, + name: self.name, + rule: self.rule, priority: self.priority, }) } @@ -180,35 +155,27 @@ pub async fn test_rule( ) .await?; - let request = body.into_inner(); - let rule = request.rule.trim(); - 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" - ))); - } - 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 request = body.into_inner(); + request.rule = request.rule.trim().to_string(); + request.validate().map_err(|error| { + ApiError::Validation(validation_errors_to_string(error, None)) })?; - let mut effects = Vec::with_capacity(request.traces.len()); - for (index, trace) in request.traces.iter().enumerate() { - let input = test_rule_input(trace); + let rule = request.rule; + 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::::new(); for rule in rules { - if response - .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 ( + let affected_detail = 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 - .last_mut() - .expect("a delphi rule was inserted above") - .affected_details - .push(DelphiRuleAffectedDetail { - detail_id, - issue_id, - project_id: rule.project_id.map(ProjectId::from), - project_name: rule.project_name, - project_icon_url: rule.project_icon_url, - version_id: rule.version_id.map(VersionId::from), - version_name: rule.version_name, - version_number: rule.version_number, - issue_type, - key, - jar: rule.jar, - file_path, - original_severity, - severity: rule.effect_severity, - hidden, - }); + Some(DelphiRuleAffectedDetail { + detail_id, + issue_id, + project_id: rule.project_id.map(ProjectId::from), + project_name: rule.project_name, + project_icon_url: rule.project_icon_url, + version_id: rule.version_id.map(VersionId::from), + version_name: rule.version_name, + version_number: rule.version_number, + issue_type, + key, + jar: rule.jar, + file_path, + original_severity, + severity: rule.effect_severity, + hidden, + }) + } else { + None + }; + + 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!( diff --git a/apps/labrinth/src/routes/internal/moderation/tech_review/rules_scan.rs b/apps/labrinth/src/routes/internal/moderation/tech_review/rules_scan.rs index de618826c2..f701c5e141 100644 --- a/apps/labrinth/src/routes/internal/moderation/tech_review/rules_scan.rs +++ b/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, } -#[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, pub hashes: BTreeMap, } -#[derive(Serialize, utoipa::ToSchema)] +#[derive(Deserialize, Serialize, utoipa::ToSchema)] pub struct RuleScope { pub project_id: Option, pub version_id: Option, @@ -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), - version_id: detail.version_id.map(to_public_id), - file_id: detail.file_id.map(to_public_id), + project_id: detail.project_id.map(|id| to_base62(id as u64)), + version_id: detail.version_id.map(|id| to_base62(id as u64)), + 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( - &sender, - "complete", - &RuleScanEvent { - phase: "complete", - revision: summary.revision, - scanned: summary.scanned, - total: summary.total, - effects: summary.effects, - }, - ); + let event = RuleScanEvent { + phase: "complete", + revision: summary.revision, + scanned: summary.scanned, + total: summary.total, + effects: summary.effects, + }; + if let Ok(data) = serde_json::to_string(&event) { + let _ = sender.send(Bytes::from(format!( + "event: complete\ndata: {data}\n\n" + ))); + } } Err(error) => { tracing::error!(error = ?error, "delphi rule scan failed"); - send_event( - &sender, - "failed", - &RuleScanErrorEvent { - message: &error.to_string(), - }, - ); + let message = error.to_string(); + let event = RuleScanErrorEvent { message: &message }; + if let Ok(data) = serde_json::to_string(&event) { + let _ = sender.send(Bytes::from(format!( + "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), - version_id: detail.version_id.map(to_public_id), - file_id: detail.file_id.map(to_public_id), + project_id: detail.project_id.map(|id| to_base62(id as u64)), + version_id: detail.version_id.map(|id| to_base62(id as u64)), + 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( - sender, - "scanning", + let event = RuleScanEvent { + phase: "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), - version_id: detail.version_id.map(to_public_id), - file_id: detail.file_id.map(to_public_id), + project_id: detail.project_id.map(|id| to_base62(id as u64)), + version_id: detail.version_id.map(|id| to_base62(id as u64)), + 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> { - 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")? - .into_iter() - .map(|rule| { - let program = cel::Program::compile(&rule.rule).map_err(|error| { - eyre!("failed to compile delphi rule {}: {error}", rule.id.0) - })?; - Ok(CompiledRule { - id: rule.id, - program, - }) + .wrap_err("failed to fetch delphi rules")?; + + tokio::task::spawn_blocking(move || { + rules + .into_iter() + .map(|rule| { + let program = + cel::Program::compile(&rule.rule).map_err(|error| { + 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, - 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, - 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"))); -} diff --git a/packages/api-client/src/modules/labrinth/types.ts b/packages/api-client/src/modules/labrinth/types.ts index e6169e2061..75c84cf450 100644 --- a/packages/api-client/src/modules/labrinth/types.ts +++ b/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 - } - export type TestDelphiRuleRequest = { rule: string - traces: TestDelphiRuleTrace[] + inputs: RuleInput[] } export type DelphiRuleEffect = {