Fix gallery creation validation and validators returning incorrect er… (#263)

* Fix gallery creation validation and validators returning incorrect errors

* Remove docker image

* Add URL validation for pack files

* Remove unneeded dependencies

.env

@@ -37,4 +37,6 @@ VERSION_INDEX_INTERVAL=1800
 GITHUB_CLIENT_ID=none
 GITHUB_CLIENT_SECRET=none
 
-RATE_LIMIT_IGNORE_IPS='[]'
+RATE_LIMIT_IGNORE_IPS='[]'
+
+WHITELISTED_MODPACK_DOMAINS='["cdn.modrinth.com", "edge.forgecdn.net", "github.com", "raw.githubusercontent.com"]'

.github/workflows/docker-compile.yml

@@ -20,9 +20,7 @@ jobs:
         id: docker_meta
         uses: crazy-max/ghaction-docker-meta@v1
         with:
-          images: |
+          images: ghcr.io/modrinth/labrinth
-            ghcr.io/modrinth/labrinth
-            docker.io/modrinth/labrinth
       -
         name: Set up QEMU
         uses: docker/setup-qemu-action@v1
@@ -45,13 +43,6 @@ jobs:
           registry: ghcr.io
           username: ${{ github.actor }}
           password: ${{ secrets.GITHUB_TOKEN }}
-      -
-        name: Login to DockerHub
-        uses: docker/login-action@v1
-        if: github.event_name != 'pull_request'
-        with:
-          username: ${{ secrets.DOCKER_USERNAME }}
-          password: ${{ secrets.DOCKER_PASSWORD }}
       -
         name: Build and push
         uses: docker/build-push-action@v2

Cargo.toml

@@ -13,7 +13,7 @@ path = "src/main.rs"
 
 [dependencies]
 actix-web = "3.3.2"
-actix-rt = "1.1.0"
+actix-rt = "1.1.1"
 actix-files = "0.5.0"
 actix-multipart = "0.3.0"
 actix-cors = "0.5.4"
@@ -39,6 +39,7 @@ zip = "0.5.12"
 
 validator = { version = "0.13", features = ["derive"] }
 regex = "1.5.4"
+url = "2.2.2"
 
 gumdrop = "0.8.0"
 dotenv = "0.15"
@@ -57,7 +58,4 @@ sqlx = { version = "0.4.2", features = ["runtime-actix-rustls", "postgres", "chr
 sentry = { version = "0.22.0", features = ["log"] }
 sentry-actix = "0.22.0"
 
-actix-web-prom = {git = "https://github.com/nlopes/actix-web-prom", branch = "master"}
-prometheus = "0.12.0"
-
 bytes = "0.5.6"

src/health/mod.rs

@@ -1,5 +1,3 @@
-pub mod pod;
-pub mod scheduler;
 pub mod status;
 
 use lazy_static::lazy_static;

src/health/pod.rs

@@ -1,33 +0,0 @@
-use std::sync::{Arc, RwLock};
-
-#[derive(Clone, Debug)]
-pub struct PodInfo {
-    pub pod_name: String,
-    pub node_name: String,
-    pod_id: Arc<RwLock<Option<String>>>,
-}
-
-impl PodInfo {
-    pub fn new() -> Self {
-        Self {
-            pod_name: dotenv::var("POD_NAME").unwrap_or_else(|_| "DEV".to_string()),
-            node_name: dotenv::var("NODE_NAME").unwrap_or_else(|_| "self-hosted".to_string()),
-            pod_id: Arc::new(RwLock::new(None)),
-        }
-    }
-    pub fn get_id(&self) -> String {
-        {
-            let lock = self.pod_id.read().unwrap();
-            if lock.is_some() {
-                return lock.clone().unwrap();
-            }
-        }
-        let mut lock = self.pod_id.write().unwrap();
-        let id = self.generate_id();
-        lock.replace(id.clone());
-        id
-    }
-    fn generate_id(&self) -> String {
-        base64::encode(format!("{}-{}", self.node_name, self.pod_name))
-    }
-}

src/health/scheduler.rs

@@ -1,136 +0,0 @@
-use crate::scheduler::Scheduler;
-use sqlx::{Pool, Postgres};
-
-use actix_web::dev::{Service, ServiceRequest, ServiceResponse, Transform};
-use actix_web::Error;
-use prometheus::{opts, IntGaugeVec};
-
-use futures::future::{ok, Ready};
-use std::future::Future;
-use std::pin::Pin;
-use std::task::{Context, Poll};
-
-use crate::health::pod::PodInfo;
-use actix_web::http::{HeaderName, HeaderValue};
-use actix_web_prom::PrometheusMetrics;
-
-pub struct HealthCounters {
-    pod: PodInfo,
-    idle_db_conn: IntGaugeVec,
-    opened_db_conn: IntGaugeVec,
-    current_requests: IntGaugeVec,
-}
-impl HealthCounters {
-    pub fn new() -> Self {
-        let idle_opts = opts!("idle_db_conn", "Amount of idle connections").namespace("api");
-        let opened_opts = opts!("open_db_conn", "Amount of open connections").namespace("api");
-        let current_opts = opts!("current_requests", "Currently open requests").namespace("api");
-        Self {
-            pod: PodInfo::new(),
-            idle_db_conn: IntGaugeVec::new(idle_opts, &[]).unwrap(),
-            opened_db_conn: IntGaugeVec::new(opened_opts, &[]).unwrap(),
-            current_requests: IntGaugeVec::new(current_opts, &["endpoint", "method"]).unwrap(),
-        }
-    }
-    pub fn register(&self, builder: &mut PrometheusMetrics) {
-        builder
-            .registry
-            .register(Box::new(self.opened_db_conn.clone()))
-            .unwrap();
-        builder
-            .registry
-            .register(Box::new(self.idle_db_conn.clone()))
-            .unwrap();
-        builder
-            .registry
-            .register(Box::new(self.current_requests.clone()))
-            .unwrap();
-    }
-    pub fn schedule(&self, pool: Pool<Postgres>, scheduler: &mut Scheduler) {
-        let this = self.clone();
-        scheduler.run(std::time::Duration::from_secs(5), move || {
-            let idle = pool.num_idle();
-            let total = pool.size();
-            this.idle_db_conn.with_label_values(&[]).set(idle as i64);
-            this.opened_db_conn.with_label_values(&[]).set(total as i64);
-            async move {
-                ok::<i32, i32>(1).await.unwrap();
-            }
-        });
-    }
-}
-
-impl Clone for HealthCounters {
-    fn clone(&self) -> Self {
-        Self {
-            pod: self.pod.clone(),
-            idle_db_conn: self.idle_db_conn.clone(),
-            opened_db_conn: self.opened_db_conn.clone(),
-            current_requests: self.current_requests.clone(),
-        }
-    }
-}
-
-impl<S, B> Transform<S> for HealthCounters
-where
-    S: Service<Request = ServiceRequest, Response = ServiceResponse<B>, Error = Error>,
-    S::Future: 'static,
-    B: 'static,
-{
-    type Request = ServiceRequest;
-    type Response = ServiceResponse<B>;
-    type Error = Error;
-    type Transform = MonitoringMiddleware<S>;
-    type InitError = ();
-    type Future = Ready<Result<Self::Transform, Self::InitError>>;
-
-    fn new_transform(&self, service: S) -> Self::Future {
-        ok(MonitoringMiddleware {
-            service,
-            counters: self.clone(),
-        })
-    }
-}
-
-pub struct MonitoringMiddleware<S> {
-    service: S,
-    counters: HealthCounters,
-}
-
-impl<S, B> Service for MonitoringMiddleware<S>
-where
-    S: Service<Request = ServiceRequest, Response = ServiceResponse<B>, Error = Error>,
-    S::Future: 'static,
-    B: 'static,
-{
-    type Request = ServiceRequest;
-    type Response = ServiceResponse<B>;
-    type Error = Error;
-    type Future = Pin<Box<dyn Future<Output = Result<Self::Response, Self::Error>>>>;
-
-    fn poll_ready(&mut self, cx: &mut Context<'_>) -> Poll<Result<(), Self::Error>> {
-        self.service.poll_ready(cx)
-    }
-
-    fn call(&mut self, req: ServiceRequest) -> Self::Future {
-        // The request has started.
-        let pattern_or_path = req.match_pattern().unwrap_or_else(|| "unknown".to_string());
-        let counter = self
-            .counters
-            .current_requests
-            .with_label_values(&[&*pattern_or_path, req.method().as_str()]);
-        counter.inc();
-        let pod = self.counters.pod.clone();
-        let fut = self.service.call(req);
-        Box::pin(async move {
-            let mut res: Self::Response = fut.await?;
-            // The request finished, remove a counter
-            counter.dec();
-            res.headers_mut().insert(
-                HeaderName::from_static("x-server"),
-                HeaderValue::from_str(&*pod.get_id()).unwrap(),
-            );
-            Ok(res)
-        })
-    }
-}

src/main.rs

@@ -1,11 +1,9 @@
 use crate::file_hosting::S3Host;
-use crate::health::scheduler::HealthCounters;
 use crate::util::env::{parse_strings_from_var, parse_var};
 use actix_cors::Cors;
 use actix_ratelimit::errors::ARError;
 use actix_ratelimit::{MemoryStore, MemoryStoreActor, RateLimiter};
 use actix_web::{http, web, App, HttpServer};
-use actix_web_prom::PrometheusMetricsBuilder;
 use env_logger::Env;
 use gumdrop::Options;
 use log::{error, info, warn};
@@ -242,22 +240,11 @@ async fn main() -> std::io::Result<()> {
 
     let store = MemoryStore::new();
 
-    // Get prometheus service
-    let mut prometheus = PrometheusMetricsBuilder::new("api")
-        .endpoint("/metrics")
-        .build()
-        .unwrap();
-    // Get custom service
-    let health = HealthCounters::new();
-    health.register(&mut prometheus);
-    health.schedule(pool.clone(), &mut scheduler);
     info!("Starting Actix HTTP server!");
 
     // Init App
     HttpServer::new(move || {
         App::new()
-            .wrap(prometheus.clone())
-            .wrap(health.clone())
             .wrap(
                 Cors::default()
                     .allowed_methods(["GET", "POST", "DELETE", "PATCH", "PUT"])
@@ -343,6 +330,11 @@ fn check_env_vars() -> bool {
         failed |= true;
     }
 
+    if parse_strings_from_var("WHITELISTED_MODPACK_DOMAINS").is_none() {
+        warn!("Variable `WHITELISTED_MODPACK_DOMAINS` missing in dotenv or not a json array of strings");
+        failed |= true;
+    }
+
     failed |= check_var::<String>("SITE_URL");
     failed |= check_var::<String>("CDN_URL");
     failed |= check_var::<String>("DATABASE_URL");

src/routes/project_creation.rs

@@ -196,10 +196,10 @@ pub struct NewGalleryItem {
     pub item: String,
     /// Whether the gallery item should show in search or not
     pub featured: bool,
-    #[validate(url, length(min = 1, max = 2048))]
+    #[validate(length(min = 1, max = 2048))]
     /// The title of the gallery item
     pub title: Option<String>,
-    #[validate(url, length(min = 1, max = 2048))]
+    #[validate(length(min = 1, max = 2048))]
     /// The description of the gallery item
     pub description: Option<String>,
 }

src/validate/mod.rs

@@ -67,7 +67,7 @@ pub async fn validate_file(
     game_versions: Vec<GameVersion>,
     all_game_versions: Vec<crate::database::models::categories::GameVersion>,
 ) -> Result<ValidationResult, ValidationError> {
-    Ok(actix_web::web::block(move || {
+    let res = actix_web::web::block(move || {
         let reader = std::io::Cursor::new(data);
         let mut zip = zip::ZipArchive::new(reader)?;
 
@@ -103,8 +103,15 @@ pub async fn validate_file(
             Ok(ValidationResult::Pass)
         }
     })
-    .await
+    .await;
-    .map_err(|_| ValidationError::BlockingError)?)
+
+    match res {
+        Ok(x) => Ok(x),
+        Err(err) => match err {
+            actix_web::error::BlockingError::Canceled => Err(ValidationError::BlockingError),
+            actix_web::error::BlockingError::Error(err) => Err(err),
+        },
+    }
 }
 
 fn game_version_supported(

src/validate/pack.rs

@@ -1,4 +1,5 @@
 use crate::models::projects::SideType;
+use crate::util::env::parse_strings_from_var;
 use crate::util::validate::validation_errors_to_string;
 use crate::validate::{SupportedGameVersions, ValidationError, ValidationResult};
 use serde::{Deserialize, Serialize};
@@ -33,8 +34,18 @@ pub struct PackFile<'a> {
 
 fn validate_download_url(values: &Vec<&str>) -> Result<(), validator::ValidationError> {
     for value in values {
-        if !validator::validate_url(*value) {
+        let domains = parse_strings_from_var("WHITELISTED_MODPACK_DOMAINS").unwrap_or_default();
-            return Err(validator::ValidationError::new("invalid URL"));
+        if !domains.contains(
+            &url::Url::parse(value)
+                .ok()
+                .ok_or_else(|| validator::ValidationError::new("invalid URL"))?
+                .domain()
+                .ok_or_else(|| validator::ValidationError::new("invalid URL"))?
+                .to_string(),
+        ) {
+            return Err(validator::ValidationError::new(
+                "File download source is not from allowed sources",
+            ));
         }
     }
 
@@ -42,10 +53,21 @@ fn validate_download_url(values: &Vec<&str>) -> Result<(), validator::Validation
 }
 
 #[derive(Serialize, Deserialize, Eq, PartialEq, Hash)]
-#[serde(rename_all = "camelCase")]
+#[serde(rename_all = "camelCase", from = "String")]
 pub enum FileHash {
     Sha1,
     Sha512,
+    Unknown(String),
+}
+
+impl From<String> for FileHash {
+    fn from(s: String) -> Self {
+        return match s.as_str() {
+            "sha1" => FileHash::Sha1,
+            "sha512" => FileHash::Sha512,
+            _ => FileHash::Unknown(s),
+        };
+    }
 }
 
 #[derive(Serialize, Deserialize, Eq, PartialEq, Hash)]
@@ -122,6 +144,14 @@ impl super::Validator for PackValidator {
             ));
         }
 
+        for file in pack.files {
+            if file.hashes.get(&FileHash::Sha1).is_none() {
+                return Err(ValidationError::InvalidInputError(
+                    "All pack files must provide a SHA1 hash!".into(),
+                ));
+            }
+        }
+
         Ok(ValidationResult::Pass)
     }
 }