Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 26 additions & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

3 changes: 3 additions & 0 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
resolver = "3"
members = [
"server",
"mlm_web_api",
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick | 🔵 Trivial

Consider adding a matching dev-profile entry for mlm_web_api.

Line 5 correctly adds the crate to the workspace. For consistency with other workspace crates, you may also add [profile.dev.package.mlm_web_api] opt-level = 0.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@Cargo.toml` at line 5, Add a matching dev-profile entry for the workspace
crate "mlm_web_api" by adding a [profile.dev.package.mlm_web_api] table with
opt-level = 0 to your Cargo.toml (the same workspace manifest where
"mlm_web_api" is listed) so the dev profile is consistent with the other
workspace crates; ensure the table key exactly matches mlm_web_api.

"mlm_db",
"mlm_parse",
"mlm_mam",
Expand Down Expand Up @@ -29,6 +30,8 @@ opt-level = 0
opt-level = 0
[profile.dev.package.mlm_parse]
opt-level = 0
[profile.dev.package.mlm_web_api]
opt-level = 0
[profile.dev.package.mlm_web_askama]
opt-level = 0

Expand Down
24 changes: 24 additions & 0 deletions mlm_web_api/Cargo.toml
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
[package]
name = "mlm_web_api"
version = "0.1.0"
edition = "2024"

[dependencies]
anyhow = "1.0.100"
axum = { version = "0.8.4", features = ["query"] }
axum-extra = { version = "0.10.1", features = ["form"] }
mime_guess = "2.0.5"
mlm_core = { path = "../mlm_core" }
mlm_db = { path = "../mlm_db" }
mlm_mam = { path = "../mlm_mam" }
native_db = { git = "https://github.com/StirlingMouse/native_db.git", branch = "0.8.x" }
qbit = { git = "https://github.com/StirlingMouse/qbittorrent-webui-api.git" }
Comment on lines +14 to +15
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "== Branch/floating git dependencies in Cargo.toml files =="
rg -n 'git\s*=\s*"[^"]+"|branch\s*=' --glob '**/Cargo.toml' -C1

echo
if [ -f Cargo.lock ]; then
  echo "== Git sources in Cargo.lock (native_db/qbit) =="
  rg -n 'name = "(native_db|qbit)"|source = "git\+' Cargo.lock -C2
else
  echo "Cargo.lock not found."
fi

Repository: StirlingMouse/MLM

Length of output: 4185


🏁 Script executed:

git ls-files | grep -E "^Cargo\.lock$"

Repository: StirlingMouse/MLM

Length of output: 71


Pin git dependencies to immutable commits instead of floating branches.

Lines 13–14 use branch-based and floating git dependencies, which allow upstream movement. Even with Cargo.lock pinning current builds, manifests should explicitly specify rev to prevent unintended drift during lockfile updates and strengthen supply-chain transparency.

Suggested manifest change
-native_db = { git = "https://github.com/StirlingMouse/native_db.git", branch = "0.8.x" }
-qbit = { git = "https://github.com/StirlingMouse/qbittorrent-webui-api.git" }
+native_db = { git = "https://github.com/StirlingMouse/native_db.git", rev = "cddaafe0f6863146fd0879cae88fdb613bc50b47" }
+qbit = { git = "https://github.com/StirlingMouse/qbittorrent-webui-api.git", rev = "1038c6000e749b6b0d55fe2108618e82beeaebfe" }

Note: This pattern also appears in server/Cargo.toml, mlm_core/Cargo.toml, and other manifests.

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
native_db = { git = "https://github.com/StirlingMouse/native_db.git", branch = "0.8.x" }
qbit = { git = "https://github.com/StirlingMouse/qbittorrent-webui-api.git" }
native_db = { git = "https://github.com/StirlingMouse/native_db.git", rev = "cddaafe0f6863146fd0879cae88fdb613bc50b47" }
qbit = { git = "https://github.com/StirlingMouse/qbittorrent-webui-api.git", rev = "1038c6000e749b6b0d55fe2108618e82beeaebfe" }
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@mlm_web_api/Cargo.toml` around lines 13 - 14, The Cargo manifest uses
floating/branch git dependencies (native_db and qbit) which can drift; update
each dependency declaration (e.g., native_db and qbit) to pin an immutable
commit by replacing the branch/omitted revision with a rev = "<commit-sha>"
field (use the specific commit SHA you want to lock to), and remove the branch
field if present; apply the same change pattern to other manifests that list
these git dependencies (e.g., server/Cargo.toml, mlm_core/Cargo.toml) so all git
dependencies use rev pins for reproducible builds and supply-chain safety.

serde = "1.0.136"
serde_json = "1.0.140"
thiserror = "2.0.17"
tokio = { version = "1.45.1", features = ["fs"] }
tokio-util = "0.7"
tower = "0.5.2"
tower-http = { version = "0.6.6", features = ["fs"] }
toml = "0.8.23"
tracing = "0.1.41"
69 changes: 69 additions & 0 deletions mlm_web_api/src/download.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,69 @@
use axum::{
body::Body,
extract::{Path, State},
response::IntoResponse,
};
use mlm_db::Torrent;
use tokio_util::io::ReaderStream;
use tracing::warn;

use crate::error::AppError;
use mlm_core::{
Context, ContextExt,
linker::map_path,
qbittorrent::{self},
};

pub async fn torrent_file(
State(context): State<Context>,
Path((id, filename)): Path<(String, String)>,
) -> Result<impl IntoResponse, AppError> {
let config = context.config().await;
let Some(torrent) = context.db().r_transaction()?.get().primary::<Torrent>(id)? else {
return Err(AppError::NotFound);
};
let Some(path) = (if let (Some(library_path), Some(library_file)) = (
&torrent.library_path,
torrent
.library_files
.iter()
.find(|f| f.to_string_lossy() == filename),
) {
Some(library_path.join(library_file))
} else if let Some((torrent, qbit, qbit_config)) =
qbittorrent::get_torrent(&config, &torrent.id).await?
{
qbit.files(&torrent.hash, None)
.await?
.into_iter()
.find(|f| f.name == filename)
.map(|file| map_path(&qbit_config.path_mapping, &torrent.save_path).join(&file.name))
} else {
None
}) else {
return Err(AppError::NotFound);
};
let file = match tokio::fs::File::open(&path).await {
Ok(file) => file,
Err(err) => {
warn!("Failed opening torrent file {}: {err}", path.display());
return Err(AppError::NotFound);
}
};
let stream = ReaderStream::new(file);
let body = Body::from_stream(stream);
let content_type = mime_guess::from_path(&filename)
.first_or_octet_stream()
.to_string();
let safe_filename = filename.replace(['\r', '\n', '"'], "_");

let headers = [
(axum::http::header::CONTENT_TYPE, content_type),
(
axum::http::header::CONTENT_DISPOSITION,
format!("attachment; filename=\"{}\"", safe_filename),
),
];
Comment on lines +60 to +66
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Incorrect Content-Type for file downloads.

The Content-Type is hardcoded to text/toml; charset=utf-8, but this endpoint serves arbitrary torrent-related files (audio files, etc.), not TOML configuration files. This will cause browsers to misinterpret the file type.

Additionally, the filename in Content-Disposition should be sanitized to prevent header injection with characters like " or newlines.

🐛 Proposed fix
     let headers = [
         (
             axum::http::header::CONTENT_TYPE,
-            "text/toml; charset=utf-8".to_string(),
+            "application/octet-stream".to_string(),
         ),
         (
             axum::http::header::CONTENT_DISPOSITION,
-            format!("attachment; filename=\"{}\"", filename),
+            format!(
+                "attachment; filename=\"{}\"",
+                filename.replace('"', "\\\"").replace('\n', "").replace('\r', "")
+            ),
         ),
     ];
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
let headers = [
(
axum::http::header::CONTENT_TYPE,
"text/toml; charset=utf-8".to_string(),
),
(
axum::http::header::CONTENT_DISPOSITION,
format!("attachment; filename=\"{}\"", filename),
),
];
let headers = [
(
axum::http::header::CONTENT_TYPE,
"application/octet-stream".to_string(),
),
(
axum::http::header::CONTENT_DISPOSITION,
format!(
"attachment; filename=\"{}\"",
filename.replace('"', "\\\"").replace('\n', "").replace('\r', "")
),
),
];
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@mlm_web_api/src/download.rs` around lines 52 - 61, The headers block
incorrectly hardcodes "text/toml; charset=utf-8" and uses an unsanitized
filename; update the code that builds the headers (the `headers` array where
`CONTENT_TYPE` and `CONTENT_DISPOSITION` are set) to: 1) derive the MIME type
from the file name/path using a library such as mime_guess (e.g.
mime_guess::from_path(filename).first_or_octet_stream().to_string()) and use
that value for `CONTENT_TYPE` (include charset only for text types if desired),
and 2) sanitize/escape the `filename` used in `CONTENT_DISPOSITION` to prevent
header injection (strip or replace newlines and quotes and/or use the RFC5987
encoding form `filename*=` with percent-encoding for UTF-8); ensure you fall
back to "application/octet-stream" if no MIME is found.


Ok((headers, body))
}
32 changes: 32 additions & 0 deletions mlm_web_api/src/error.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
use axum::{
http::StatusCode,
response::{IntoResponse, Response},
};

#[derive(Debug, thiserror::Error)]
pub enum AppError {
#[error("Could not query db: {0}")]
Db(#[from] native_db::db_type::Error),
#[error("Meta Error: {0:?}")]
MetaError(#[from] mlm_mam::meta::MetaError),
#[error("Qbit Error: {0:?}")]
QbitError(#[from] qbit::Error),
#[error("Toml Parse Error: {0:?}")]
Toml(#[from] toml::de::Error),
#[error("Error: {0:?}")]
Generic(#[from] anyhow::Error),
#[error("JSON Error: {0}")]
Json(#[from] serde_json::Error),
#[error("Page Not Found")]
NotFound,
}

impl IntoResponse for AppError {
fn into_response(self) -> Response {
let status = match self {
AppError::NotFound => StatusCode::NOT_FOUND,
_ => StatusCode::INTERNAL_SERVER_ERROR,
};
(status, self.to_string()).into_response()
}
Comment on lines +24 to +31
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Consider sanitizing error messages in responses to avoid information leakage.

The current implementation returns self.to_string() as the response body for all errors, which may expose internal details like database error messages, file paths, or stack traces to API clients. This could be a security concern in production.

Consider returning generic messages for 500 errors while logging the actual error server-side.

🛡️ Proposed fix
 impl IntoResponse for AppError {
     fn into_response(self) -> Response {
         let status = match self {
             AppError::NotFound => StatusCode::NOT_FOUND,
             _ => StatusCode::INTERNAL_SERVER_ERROR,
         };
-        (status, self.to_string()).into_response()
+        let message = match &self {
+            AppError::NotFound => "Not Found".to_string(),
+            _ => {
+                tracing::error!("API error: {self:?}");
+                "Internal Server Error".to_string()
+            }
+        };
+        (status, message).into_response()
     }
 }
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@mlm_web_api/src/error.rs` around lines 24 - 31, The into_response
implementation on AppError currently returns self.to_string() which can leak
internal details; change into_response (impl IntoResponse for AppError) to map
AppError::NotFound to StatusCode::NOT_FOUND with a safe user-facing message, but
for all other variants (StatusCode::INTERNAL_SERVER_ERROR) return a generic
error message like "Internal server error" to clients while logging the full
error string/server-side (use your existing logging facility) so internal
details are not sent in the HTTP response.

}
80 changes: 80 additions & 0 deletions mlm_web_api/src/lib.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,80 @@
mod download;
mod error;
mod search;
mod torrent;

use std::path::PathBuf;

use axum::{
Router,
body::Body,
http::{HeaderValue, Request},
middleware::{self, Next},
response::Response,
routing::get,
};
use mlm_core::Context;
use tower::ServiceBuilder;
use tower_http::services::{ServeDir, ServeFile};

use crate::{
download::torrent_file,
search::{search_api, search_api_post},
torrent::torrent_api,
};

pub fn router(context: Context, dioxus_public_path: PathBuf) -> Router {
let dioxus_assets_path = dioxus_public_path.join("assets");
let app = Router::new()
.route("/api/search", get(search_api).post(search_api_post))
.route(
"/api/torrents/{id}",
get(torrent_api).with_state(context.clone()),
)
.route(
"/torrents/{id}/{filename}",
get(torrent_file).with_state(context.clone()),
)
.with_state(context.clone())
.nest_service(
"/assets",
ServiceBuilder::new()
.layer(middleware::from_fn(set_static_cache_control))
.service(ServeDir::new(dioxus_assets_path).fallback(ServeDir::new("server/assets"))),
);

#[cfg(debug_assertions)]
let app = app.nest_service(
"/assets/favicon.png",
ServiceBuilder::new()
.layer(middleware::from_fn(set_static_cache_control))
.service(ServeFile::new("server/assets/favicon_dev.png")),
);

#[cfg(debug_assertions)]
let app = app.nest_service(
"/favicon.ico",
ServiceBuilder::new()
.layer(middleware::from_fn(set_static_cache_control))
.service(ServeFile::new("server/assets/favicon_dev.png")),
);

#[cfg(not(debug_assertions))]
let app = app.nest_service(
"/favicon.ico",
ServiceBuilder::new()
.layer(middleware::from_fn(set_static_cache_control))
.service(ServeFile::new("server/assets/favicon.png")),
);

app
}

async fn set_static_cache_control(request: Request<Body>, next: Next) -> Response {
let mut response = next.run(request).await;
response.headers_mut().insert(
axum::http::header::CACHE_CONTROL,
HeaderValue::from_static("must-revalidate"),
);
response
}
Loading