diff --git a/.cargo/config.toml b/.cargo/config.toml index abd0872..a91a31b 100644 --- a/.cargo/config.toml +++ b/.cargo/config.toml @@ -6,6 +6,17 @@ rustflags = [ "-Clto", "-Zvirtual-function-elimination", "-Zlocation-detail=none", + + # Configuration for these lints should be placed in `.clippy.toml` at the crate root. + "-Dwarnings", +] + +[target.wasm32-unknown-unknown] +rustflags = [ + "-C", + "link-args=-z stack-size=268435456", + "-C", + "target-feature=+atomics,+bulk-memory,+mutable-globals", ] diff --git a/.deny.toml b/.deny.toml index 5613325..bc98780 100644 --- a/.deny.toml +++ b/.deny.toml @@ -1,74 +1,13 @@ -# This template contains all of the possible sections and their default values +# https://embarkstudios.github.io/cargo-deny/checks/bans/cfg.html +[bans] +multiple-versions = "allow" # TODO +wildcards = "allow" +skip = [] -# Note that all fields that take a lint level have these possible values: -# * deny - An error will be produced and the check will fail -# * warn - A warning will be produced, but the check will not fail -# * allow - No warning or error will be produced, though in some cases a note -# will be - -# The values provided in this template are the default values that will be used -# when any section or field is not specified in your own configuration - -# Root options - -# The graph table configures how the dependency graph is constructed and thus -# which crates the checks are performed against -[graph] -# If 1 or more target triples (and optionally, target_features) are specified, -# only the specified targets will be checked when running `cargo deny check`. -# This means, if a particular package is only ever used as a target specific -# dependency, such as, for example, the `nix` crate only being used via the -# `target_family = "unix"` configuration, that only having windows targets in -# this list would mean the nix crate, as well as any of its exclusive -# dependencies not shared by any other crates, would be ignored, as the target -# list here is effectively saying which targets you are building for. -targets = [ - # The triple can be any string, but only the target triples built in to - # rustc (as of 1.40) can be checked against actual config expressions - #"x86_64-unknown-linux-musl", - # You can also specify which target_features you promise are enabled for a - # particular target. target_features are currently not validated against - # the actual valid features supported by the target architecture. - #{ triple = "wasm32-unknown-unknown", features = ["atomics"] }, -] -# When creating the dependency graph used as the source of truth when checks are -# executed, this field can be used to prune crates from the graph, removing them -# from the view of cargo-deny. This is an extremely heavy hammer, as if a crate -# is pruned from the graph, all of its dependencies will also be pruned unless -# they are connected to another crate in the graph that hasn't been pruned, -# so it should be used with care. The identifiers are [Package ID Specifications] -# (https://doc.rust-lang.org/cargo/reference/pkgid-spec.html) -#exclude = [] -# If true, metadata will be collected with `--all-features`. Note that this can't -# be toggled off if true, if you want to conditionally enable `--all-features` it -# is recommended to pass `--all-features` on the cmd line instead -all-features = false -# If true, metadata will be collected with `--no-default-features`. The same -# caveat with `all-features` applies -no-default-features = false -# If set, these feature will be enabled when collecting metadata. If `--features` -# is specified on the cmd line they will take precedence over this option. -#features = [] - -# The output table provides options for how/if diagnostics are outputted -[output] -# When outputting inclusion graphs in diagnostics that include features, this -# option can be used to specify the depth at which feature edges will be added. -# This option is included since the graphs can be quite large and the addition -# of features from the crate(s) to all of the graph roots can be far too verbose. -# This option can be overridden via `--feature-depth` on the cmd line -feature-depth = 1 - -# This section is considered when running `cargo deny check advisories` -# More documentation for the advisories section can be found here: # https://embarkstudios.github.io/cargo-deny/checks/advisories/cfg.html [advisories] -# The path where the advisory databases are cloned/fetched into -#db-path = "$CARGO_HOME/advisory-dbs" -# The url(s) of the advisory databases to use -#db-urls = ["https://github.com/rustsec/advisory-db"] -# A list of advisory IDs to ignore. Note that ignored advisories will still -# output a note when they are encountered. +yanked = "deny" +unmaintained = "none" ignore = [ # Dioxus pulls a whole bunch of GTK3 dependencies that are all deprecated and # marked insecure. Unfortunately, there doesn't seem to be a GTK4 migration @@ -82,25 +21,12 @@ ignore = [ { id = "RUSTSEC-2024-0418", reason = "Used by Dioxus and there is no alternative!" }, { id = "RUSTSEC-2024-0419", reason = "Used by Dioxus and there is no alternative!" }, { id = "RUSTSEC-2024-0420", reason = "Used by Dioxus and there is no alternative!" }, - - #"RUSTSEC-0000-0000", - #{ id = "RUSTSEC-0000-0000", reason = "you can specify a reason the advisory is ignored" }, - #"a-crate-that-is-yanked@0.1.1", # you can also ignore yanked crate versions if you wish - #{ crate = "a-crate-that-is-yanked@0.1.1", reason = "you can specify why you are ignoring the yanked crate" }, ] -# If this is true, then cargo deny will use the git executable to fetch advisory database. -# If this is false, then it uses a built-in git library. -# Setting this to true can be helpful if you have special authentication requirements that cargo-deny does not support. -# See Git Authentication for more information about setting up git authentication. -#git-fetch-with-cli = true -# This section is considered when running `cargo deny check licenses` -# More documentation for the licenses section can be found here: # https://embarkstudios.github.io/cargo-deny/checks/licenses/cfg.html [licenses] -# List of explicitly allowed licenses -# See https://spdx.org/licenses/ for list of possible licenses -# [possible values: any SPDX 3.11 short identifier (+ optional exception)]. +unused-allowed-license = "deny" +private.ignore = true allow = [ "Apache-2.0 WITH LLVM-exception", "Apache-2.0", @@ -112,147 +38,9 @@ allow = [ "Unicode-3.0", "BSD-2-Clause", ] -# The confidence threshold for detecting a license from license text. -# The higher the value, the more closely the license text must be to the -# canonical license text of a valid SPDX license file. -# [possible values: any between 0.0 and 1.0]. -confidence-threshold = 0.8 -# Allow 1 or more licenses on a per-crate basis, so that particular licenses -# aren't accepted for every possible crate as with the normal allow list -exceptions = [ - # Each entry is the crate and version constraint, and its specific allow - # list - #{ allow = ["Zlib"], crate = "adler32" }, -] -# Some crates don't have (easily) machine readable licensing information, -# adding a clarification entry for it allows you to manually specify the -# licensing information -#[[licenses.clarify]] -# The package spec the clarification applies to -#crate = "ring" -# The SPDX expression for the license requirements of the crate -#expression = "MIT AND ISC AND OpenSSL" -# One or more files in the crate's source used as the "source of truth" for -# the license expression. If the contents match, the clarification will be used -# when running the license check, otherwise the clarification will be ignored -# and the crate will be checked normally, which may produce warnings or errors -# depending on the rest of your configuration -#license-files = [ -# Each entry is a crate relative path, and the (opaque) hash of its contents -#{ path = "LICENSE", hash = 0xbd0eed23 } -#] - -[licenses.private] -# If true, ignores workspace crates that aren't published, or are only -# published to private registries. -# To see how to mark a crate as unpublished (to the official registry), -# visit https://doc.rust-lang.org/cargo/reference/manifest.html#the-publish-field. -ignore = false -# One or more private registries that you might publish crates to, if a crate -# is only published to private registries, and ignore is true, the crate will -# not have its license(s) checked -registries = [ - #"https://sekretz.com/registry -] - -# This section is considered when running `cargo deny check bans`. -# More documentation about the 'bans' section can be found here: -# https://embarkstudios.github.io/cargo-deny/checks/bans/cfg.html -[bans] -# Lint level for when multiple versions of the same crate are detected -multiple-versions = "warn" -# Lint level for when a crate version requirement is `*` -wildcards = "allow" -# The graph highlighting used when creating dotgraphs for crates -# with multiple versions -# * lowest-version - The path to the lowest versioned duplicate is highlighted -# * simplest-path - The path to the version with the fewest edges is highlighted -# * all - Both lowest-version and simplest-path are used -highlight = "all" -# The default lint level for `default` features for crates that are members of -# the workspace that is being checked. This can be overridden by allowing/denying -# `default` on a crate-by-crate basis if desired. -workspace-default-features = "allow" -# The default lint level for `default` features for external crates that are not -# members of the workspace. This can be overridden by allowing/denying `default` -# on a crate-by-crate basis if desired. -external-default-features = "allow" -# List of crates that are allowed. Use with care! -allow = [ - #"ansi_term@0.11.0", - #{ crate = "ansi_term@0.11.0", reason = "you can specify a reason it is allowed" }, -] -# If true, workspace members are automatically allowed even when using deny-by-default -# This is useful for organizations that want to deny all external dependencies by default -# but allow their own workspace crates without having to explicitly list them -allow-workspace = false -# List of crates to deny -deny = [ - #"ansi_term@0.11.0", - #{ crate = "ansi_term@0.11.0", reason = "you can specify a reason it is banned" }, - # Wrapper crates can optionally be specified to allow the crate when it - # is a direct dependency of the otherwise banned crate - #{ crate = "ansi_term@0.11.0", wrappers = ["this-crate-directly-depends-on-ansi_term"] }, -] - -# List of features to allow/deny -# Each entry the name of a crate and a version range. If version is -# not specified, all versions will be matched. -#[[bans.features]] -#crate = "reqwest" -# Features to not allow -#deny = ["json"] -# Features to allow -#allow = [ -# "rustls", -# "__rustls", -# "__tls", -# "hyper-rustls", -# "rustls", -# "rustls-pemfile", -# "rustls-tls-webpki-roots", -# "tokio-rustls", -# "webpki-roots", -#] -# If true, the allowed features must exactly match the enabled feature set. If -# this is set there is no point setting `deny` -#exact = true - -# Certain crates/versions that will be skipped when doing duplicate detection. -skip = [ - #"ansi_term@0.11.0", - #{ crate = "ansi_term@0.11.0", reason = "you can specify a reason why it can't be updated/removed" }, -] -# Similarly to `skip` allows you to skip certain crates during duplicate -# detection. Unlike skip, it also includes the entire tree of transitive -# dependencies starting at the specified crate, up to a certain depth, which is -# by default infinite. -skip-tree = [ - #"ansi_term@0.11.0", # will be skipped along with _all_ of its direct and transitive dependencies - #{ crate = "ansi_term@0.11.0", depth = 20 }, -] - -# This section is considered when running `cargo deny check sources`. -# More documentation about the 'sources' section can be found here: -# https://embarkstudios.github.io/cargo-deny/checks/sources/cfg.html +# [sources] -# Lint level for what to happen when a crate from a crate registry that is not -# in the allow list is encountered -unknown-registry = "warn" -# Lint level for what to happen when a crate from a git repository that is not -# in the allow list is encountered -unknown-git = "warn" -# List of URLs for allowed crate registries. Defaults to the crates.io index -# if not specified. If it is specified but empty, no registries are allowed. -allow-registry = ["https://github.com/rust-lang/crates.io-index"] -# List of URLs for allowed Git repositories +unknown-registry = "deny" +unknown-git = "deny" allow-git = [] - -[sources.allow-org] -# github.com organizations to allow git sources for -github = [] -# gitlab.com organizations to allow git sources for -gitlab = [] -# bitbucket.org organizations to allow git sources for -bitbucket = [] diff --git a/crates/pinakes-core/src/integrity.rs b/crates/pinakes-core/src/integrity.rs index 625bbe2..ff4bf9b 100644 --- a/crates/pinakes-core/src/integrity.rs +++ b/crates/pinakes-core/src/integrity.rs @@ -392,7 +392,13 @@ pub async fn cleanup_orphaned_thumbnails( if thumbnail_dir.exists() { let entries = std::fs::read_dir(thumbnail_dir)?; - for entry in entries.flatten() { + for entry in entries.filter_map(|e| { + e.map_err(|err| { + warn!(error = %err, "failed to read thumbnail directory entry"); + err + }) + .ok() + }) { let path = entry.path(); if let Some(stem) = path.file_stem().and_then(|s| s.to_str()) && !known_ids.contains(stem) diff --git a/crates/pinakes-core/src/scan.rs b/crates/pinakes-core/src/scan.rs index ce43896..31bab6e 100644 --- a/crates/pinakes-core/src/scan.rs +++ b/crates/pinakes-core/src/scan.rs @@ -271,7 +271,9 @@ pub async fn scan_directory_with_options( if let Some(p) = progress { p.record_error(msg.clone()); } - errors.push(msg); + if errors.len() < MAX_STORED_ERRORS { + errors.push(msg); + } }, } } diff --git a/crates/pinakes-core/src/storage/postgres.rs b/crates/pinakes-core/src/storage/postgres.rs index f9d2a43..76d84cd 100644 --- a/crates/pinakes-core/src/storage/postgres.rs +++ b/crates/pinakes-core/src/storage/postgres.rs @@ -3721,8 +3721,20 @@ impl StorageBackend for PostgresBackend { .map(|p| p.to_string_lossy().to_string()); let track_index = subtitle .track_index - .map(|i| i32::try_from(i).unwrap_or(i32::MAX)); - let offset_ms = i32::try_from(subtitle.offset_ms).unwrap_or(i32::MAX); + .map(|i| { + i32::try_from(i).map_err(|_| { + PinakesError::InvalidOperation(format!( + "subtitle track_index {i} exceeds i32 range" + )) + }) + }) + .transpose()?; + let offset_ms = i32::try_from(subtitle.offset_ms).map_err(|_| { + PinakesError::InvalidOperation(format!( + "subtitle offset_ms {} exceeds i32 range", + subtitle.offset_ms + )) + })?; client .execute( "INSERT INTO subtitles (id, media_id, language, format, file_path, \ @@ -3809,7 +3821,11 @@ impl StorageBackend for PostgresBackend { .get() .await .map_err(|e| PinakesError::Database(format!("pool error: {e}")))?; - let offset = i32::try_from(offset_ms).unwrap_or(i32::MAX); + let offset = i32::try_from(offset_ms).map_err(|_| { + PinakesError::InvalidOperation(format!( + "subtitle offset_ms {offset_ms} exceeds i32 range" + )) + })?; client .execute("UPDATE subtitles SET offset_ms = $1 WHERE id = $2", &[ &offset, &id, diff --git a/crates/pinakes-core/src/thumbnail.rs b/crates/pinakes-core/src/thumbnail.rs index 7e3b799..3c79b25 100644 --- a/crates/pinakes-core/src/thumbnail.rs +++ b/crates/pinakes-core/src/thumbnail.rs @@ -28,9 +28,10 @@ impl TempFileGuard { impl Drop for TempFileGuard { fn drop(&mut self) { if self.0.exists() - && let Err(e) = std::fs::remove_file(&self.0) { - warn!("failed to clean up temp file {}: {e}", self.0.display()); - } + && let Err(e) = std::fs::remove_file(&self.0) + { + warn!("failed to clean up temp file {}: {e}", self.0.display()); + } } } diff --git a/crates/pinakes-plugin-api/src/validation.rs b/crates/pinakes-plugin-api/src/validation.rs index b7bb445..7b717fc 100644 --- a/crates/pinakes-plugin-api/src/validation.rs +++ b/crates/pinakes-plugin-api/src/validation.rs @@ -134,7 +134,10 @@ impl SchemaValidator { } /// Recursively validate a [`UiElement`] subtree. - pub fn validate_element(element: &UiElement, errors: &mut Vec) { + pub(crate) fn validate_element( + element: &UiElement, + errors: &mut Vec, + ) { match element { UiElement::Container { children, .. } | UiElement::Grid { children, .. } diff --git a/crates/pinakes-server/src/dto/media.rs b/crates/pinakes-server/src/dto/media.rs index e404776..4c16a17 100644 --- a/crates/pinakes-server/src/dto/media.rs +++ b/crates/pinakes-server/src/dto/media.rs @@ -16,22 +16,23 @@ pub fn relativize_path(full_path: &Path, roots: &[PathBuf]) -> String { let mut best: Option<&PathBuf> = None; for root in roots { if full_path.starts_with(root) { - let is_longer = best - .is_none_or(|b| root.components().count() > b.components().count()); + let is_longer = + best.is_none_or(|b| root.components().count() > b.components().count()); if is_longer { best = Some(root); } } } if let Some(root) = best - && let Ok(rel) = full_path.strip_prefix(root) { - // Normalise to forward slashes on all platforms. - return rel - .components() - .map(|c| c.as_os_str().to_string_lossy()) - .collect::>() - .join("/"); - } + && let Ok(rel) = full_path.strip_prefix(root) + { + // Normalise to forward slashes on all platforms. + return rel + .components() + .map(|c| c.as_os_str().to_string_lossy()) + .collect::>() + .join("/"); + } full_path.to_string_lossy().into_owned() } diff --git a/crates/pinakes-server/src/dto/search.rs b/crates/pinakes-server/src/dto/search.rs index eb9260e..dfe2576 100644 --- a/crates/pinakes-server/src/dto/search.rs +++ b/crates/pinakes-server/src/dto/search.rs @@ -1,7 +1,14 @@ +use pinakes_core::model::Pagination; use serde::{Deserialize, Serialize}; use super::media::MediaResponse; +/// Maximum offset accepted from clients. Prevents pathologically large OFFSET +/// values that cause expensive sequential scans in the database. +pub const MAX_OFFSET: u64 = 10_000_000; +/// Maximum page size accepted from most listing endpoints. +pub const MAX_LIMIT: u64 = 1000; + #[derive(Debug, Deserialize)] pub struct SearchParams { pub q: String, @@ -10,6 +17,17 @@ pub struct SearchParams { pub limit: Option, } +impl SearchParams { + #[must_use] + pub fn to_pagination(&self) -> Pagination { + Pagination::new( + self.offset.unwrap_or(0).min(MAX_OFFSET), + self.limit.unwrap_or(50).min(MAX_LIMIT), + None, + ) + } +} + #[derive(Debug, Serialize)] pub struct SearchResponse { pub items: Vec, @@ -25,6 +43,17 @@ pub struct SearchRequestBody { pub limit: Option, } +impl SearchRequestBody { + #[must_use] + pub fn to_pagination(&self) -> Pagination { + Pagination::new( + self.offset.unwrap_or(0).min(MAX_OFFSET), + self.limit.unwrap_or(50).min(MAX_LIMIT), + None, + ) + } +} + // Pagination #[derive(Debug, Deserialize)] pub struct PaginationParams { @@ -32,3 +61,14 @@ pub struct PaginationParams { pub limit: Option, pub sort: Option, } + +impl PaginationParams { + #[must_use] + pub fn to_pagination(&self) -> Pagination { + Pagination::new( + self.offset.unwrap_or(0).min(MAX_OFFSET), + self.limit.unwrap_or(50).min(MAX_LIMIT), + self.sort.clone(), + ) + } +} diff --git a/crates/pinakes-server/src/routes/audit.rs b/crates/pinakes-server/src/routes/audit.rs index 191c93f..7a32067 100644 --- a/crates/pinakes-server/src/routes/audit.rs +++ b/crates/pinakes-server/src/routes/audit.rs @@ -2,7 +2,6 @@ use axum::{ Json, extract::{Query, State}, }; -use pinakes_core::model::Pagination; use crate::{ dto::{AuditEntryResponse, PaginationParams}, @@ -14,11 +13,7 @@ pub async fn list_audit( State(state): State, Query(params): Query, ) -> Result>, ApiError> { - let pagination = Pagination::new( - params.offset.unwrap_or(0), - params.limit.unwrap_or(50).min(1000), - None, - ); + let pagination = params.to_pagination(); let entries = state.storage.list_audit_entries(None, &pagination).await?; Ok(Json( entries.into_iter().map(AuditEntryResponse::from).collect(), diff --git a/crates/pinakes-server/src/routes/backup.rs b/crates/pinakes-server/src/routes/backup.rs index dcca2b7..4af2b18 100644 --- a/crates/pinakes-server/src/routes/backup.rs +++ b/crates/pinakes-server/src/routes/backup.rs @@ -31,7 +31,9 @@ pub async fn create_backup( let bytes = tokio::fs::read(&backup_path) .await .map_err(|e| ApiError(pinakes_core::error::PinakesError::Io(e)))?; - let _ = tokio::fs::remove_dir_all(&backup_dir).await; + if let Err(e) = tokio::fs::remove_dir_all(&backup_dir).await { + tracing::warn!(path = %backup_dir.display(), error = %e, "failed to clean up backup temp dir"); + } let disposition = format!("attachment; filename=\"{filename}\""); Ok( diff --git a/crates/pinakes-server/src/routes/books.rs b/crates/pinakes-server/src/routes/books.rs index 9e3a0bc..b8a1758 100644 --- a/crates/pinakes-server/src/routes/books.rs +++ b/crates/pinakes-server/src/routes/books.rs @@ -22,7 +22,7 @@ use uuid::Uuid; use crate::{ auth::resolve_user_id, - dto::MediaResponse, + dto::{MAX_OFFSET, MediaResponse}, error::ApiError, state::AppState, }; @@ -177,7 +177,7 @@ pub async fn list_books( Query(query): Query, ) -> Result { let pagination = Pagination { - offset: query.offset, + offset: query.offset.min(MAX_OFFSET), limit: query.limit.min(1000), sort: None, }; diff --git a/crates/pinakes-server/src/routes/database.rs b/crates/pinakes-server/src/routes/database.rs index 6338382..4c71cde 100644 --- a/crates/pinakes-server/src/routes/database.rs +++ b/crates/pinakes-server/src/routes/database.rs @@ -26,6 +26,8 @@ pub async fn vacuum_database( pub async fn clear_database( State(state): State, ) -> Result, ApiError> { + tracing::error!("clear_database: all data is being wiped by admin request"); state.storage.clear_all_data().await?; + tracing::error!("clear_database: all data wiped successfully"); Ok(Json(serde_json::json!({"status": "ok"}))) } diff --git a/crates/pinakes-server/src/routes/enrichment.rs b/crates/pinakes-server/src/routes/enrichment.rs index 0229d55..5b93b3f 100644 --- a/crates/pinakes-server/src/routes/enrichment.rs +++ b/crates/pinakes-server/src/routes/enrichment.rs @@ -42,6 +42,13 @@ pub async fn batch_enrich( State(state): State, Json(req): Json, // Reuse: has media_ids field ) -> Result, ApiError> { + if req.media_ids.is_empty() || req.media_ids.len() > 1000 { + return Err(ApiError( + pinakes_core::error::PinakesError::InvalidOperation( + "media_ids must contain 1-1000 items".into(), + ), + )); + } let media_ids: Vec = req.media_ids.into_iter().map(MediaId).collect(); let job_id = state diff --git a/crates/pinakes-server/src/routes/media.rs b/crates/pinakes-server/src/routes/media.rs index 358db29..fd26f23 100644 --- a/crates/pinakes-server/src/routes/media.rs +++ b/crates/pinakes-server/src/routes/media.rs @@ -2,10 +2,7 @@ use axum::{ Json, extract::{Path, Query, State}, }; -use pinakes_core::{ - model::{MediaId, Pagination}, - storage::DynStorageBackend, -}; +use pinakes_core::{model::MediaId, storage::DynStorageBackend}; use uuid::Uuid; use crate::{ @@ -40,6 +37,24 @@ use crate::{ state::AppState, }; +/// Validates that a destination path is absolute and within a configured root. +fn validate_destination_path( + destination: &std::path::Path, + roots: &[std::path::PathBuf], +) -> Result<(), ApiError> { + if !destination.is_absolute() { + return Err(ApiError::bad_request( + "destination must be an absolute path", + )); + } + if !roots.iter().any(|root| destination.starts_with(root)) { + return Err(ApiError::bad_request( + "destination must be within a configured library root", + )); + } + Ok(()) +} + /// Apply tags and add to collection after a successful import. /// Shared logic used by `import_with_options`, `batch_import`, and /// `import_directory_endpoint`. @@ -114,11 +129,7 @@ pub async fn list_media( State(state): State, Query(params): Query, ) -> Result>, ApiError> { - let pagination = Pagination::new( - params.offset.unwrap_or(0), - params.limit.unwrap_or(50).min(1000), - params.sort, - ); + let pagination = params.to_pagination(); let items = state.storage.list_media(&pagination).await?; let roots = state.config.read().await.directories.roots.clone(); Ok(Json( @@ -387,6 +398,12 @@ pub async fn import_with_options( State(state): State, Json(req): Json, ) -> Result, ApiError> { + if req.tag_ids.as_ref().is_some_and(|v| v.len() > 100) { + return Err(ApiError::bad_request("tag_ids must not exceed 100 items")); + } + if req.new_tags.as_ref().is_some_and(|v| v.len() > 100) { + return Err(ApiError::bad_request("new_tags must not exceed 100 items")); + } let result = pinakes_core::import::import_file( &state.storage, &req.path, @@ -415,6 +432,12 @@ pub async fn batch_import( State(state): State, Json(req): Json, ) -> Result, ApiError> { + if req.tag_ids.as_ref().is_some_and(|v| v.len() > 100) { + return Err(ApiError::bad_request("tag_ids must not exceed 100 items")); + } + if req.new_tags.as_ref().is_some_and(|v| v.len() > 100) { + return Err(ApiError::bad_request("new_tags must not exceed 100 items")); + } if req.paths.len() > 10_000 { return Err(ApiError( pinakes_core::error::PinakesError::InvalidOperation( @@ -776,19 +799,17 @@ pub async fn batch_delete( let media_ids: Vec = req.media_ids.iter().map(|id| MediaId(*id)).collect(); - // Record audit entries BEFORE delete to avoid FK constraint violation. - // Use None for media_id since they'll be deleted; include ID in details. - for id in &media_ids { - if let Err(e) = pinakes_core::audit::record_action( - &state.storage, - None, - pinakes_core::model::AuditAction::Deleted, - Some(format!("batch delete: media_id={}", id.0)), - ) - .await - { - tracing::warn!(error = %e, "failed to record audit entry"); - } + // Record a single audit entry before delete to avoid FK constraint + // violations. One entry for the whole batch is sufficient. + if let Err(e) = pinakes_core::audit::record_action( + &state.storage, + None, + pinakes_core::model::AuditAction::Deleted, + Some(format!("batch delete: {} items", media_ids.len())), + ) + .await + { + tracing::warn!(error = %e, "failed to record audit entry"); } match state.storage.batch_delete_media(&media_ids).await { @@ -924,6 +945,15 @@ pub async fn rename_media( Path(id): Path, Json(req): Json, ) -> Result, ApiError> { + let name_len = req.new_name.chars().count(); + if name_len == 0 || name_len > 255 { + return Err(ApiError::bad_request("new_name must be 1-255 characters")); + } + if req.new_name.contains('\0') || req.new_name.contains('/') { + return Err(ApiError::bad_request( + "new_name must not contain null bytes or path separators", + )); + } let media_id = MediaId(id); // Perform the rename @@ -967,6 +997,8 @@ pub async fn move_media_endpoint( Path(id): Path, Json(req): Json, ) -> Result, ApiError> { + let roots = state.config.read().await.directories.roots.clone(); + validate_destination_path(&req.destination, &roots)?; let media_id = MediaId(id); // Perform the move @@ -1020,6 +1052,8 @@ pub async fn batch_move_media( ), )); } + let roots = state.config.read().await.directories.roots.clone(); + validate_destination_path(&req.destination, &roots)?; let media_ids: Vec = req.media_ids.iter().map(|id| MediaId(*id)).collect(); @@ -1030,26 +1064,35 @@ pub async fn batch_move_media( .await { Ok(results) => { - // Record sync changes for each moved item + // Record sync changes for each moved item. Derive the new path from + // the destination and old filename to avoid N extra get_media calls. for (media_id, old_path) in &results { - if let Ok(item) = state.storage.get_media(*media_id).await { - let change = pinakes_core::sync::SyncLogEntry { - id: uuid::Uuid::now_v7(), - sequence: 0, - change_type: pinakes_core::sync::SyncChangeType::Moved, - media_id: Some(*media_id), - path: item.path.to_string_lossy().to_string(), - content_hash: Some(item.content_hash.clone()), - file_size: Some(item.file_size), - metadata_json: Some( - serde_json::json!({ "old_path": old_path }).to_string(), - ), - changed_by_device: None, - timestamp: chrono::Utc::now(), - }; - if let Err(e) = state.storage.record_sync_change(&change).await { - tracing::warn!(error = %e, "failed to record sync change"); - } + let Some(file_name) = + std::path::Path::new(old_path.as_str()).file_name() + else { + tracing::warn!( + old_path = %old_path, + "skipping sync log entry: no filename in old_path" + ); + continue; + }; + let new_path = req.destination.join(file_name); + let change = pinakes_core::sync::SyncLogEntry { + id: uuid::Uuid::now_v7(), + sequence: 0, + change_type: pinakes_core::sync::SyncChangeType::Moved, + media_id: Some(*media_id), + path: new_path.to_string_lossy().to_string(), + content_hash: None, + file_size: None, + metadata_json: Some( + serde_json::json!({ "old_path": old_path }).to_string(), + ), + changed_by_device: None, + timestamp: chrono::Utc::now(), + }; + if let Err(e) = state.storage.record_sync_change(&change).await { + tracing::warn!(error = %e, "failed to record sync change"); } } @@ -1164,11 +1207,7 @@ pub async fn list_trash( State(state): State, Query(params): Query, ) -> Result, ApiError> { - let pagination = Pagination::new( - params.offset.unwrap_or(0), - params.limit.unwrap_or(50).min(1000), - params.sort, - ); + let pagination = params.to_pagination(); let items = state.storage.list_trash(&pagination).await?; let count = state.storage.count_trash().await?; diff --git a/crates/pinakes-server/src/routes/photos.rs b/crates/pinakes-server/src/routes/photos.rs index 4119774..c36b463 100644 --- a/crates/pinakes-server/src/routes/photos.rs +++ b/crates/pinakes-server/src/routes/photos.rs @@ -152,6 +152,14 @@ pub async fn get_map_photos( State(state): State, Query(query): Query, ) -> Result { + let valid_lat = |v: f64| v.is_finite() && (-90.0..=90.0).contains(&v); + let valid_lon = |v: f64| v.is_finite() && (-180.0..=180.0).contains(&v); + if !valid_lat(query.lat1) || !valid_lat(query.lat2) { + return Err(ApiError::bad_request("latitude must be in [-90, 90]")); + } + if !valid_lon(query.lon1) || !valid_lon(query.lon2) { + return Err(ApiError::bad_request("longitude must be in [-180, 180]")); + } // Validate bounding box let min_lat = query.lat1.min(query.lat2); let max_lat = query.lat1.max(query.lat2); diff --git a/crates/pinakes-server/src/routes/saved_searches.rs b/crates/pinakes-server/src/routes/saved_searches.rs index c5e6e23..ed103ab 100644 --- a/crates/pinakes-server/src/routes/saved_searches.rs +++ b/crates/pinakes-server/src/routes/saved_searches.rs @@ -22,10 +22,43 @@ pub struct SavedSearchResponse { pub created_at: chrono::DateTime, } +const VALID_SORT_ORDERS: &[&str] = &[ + "date_asc", + "date_desc", + "name_asc", + "name_desc", + "size_asc", + "size_desc", +]; + pub async fn create_saved_search( State(state): State, Json(req): Json, ) -> Result, ApiError> { + let name_len = req.name.chars().count(); + if name_len == 0 || name_len > 255 { + return Err(ApiError( + pinakes_core::error::PinakesError::InvalidOperation( + "name must be 1-255 characters".into(), + ), + )); + } + if req.query.is_empty() || req.query.len() > 2048 { + return Err(ApiError( + pinakes_core::error::PinakesError::InvalidOperation( + "query must be 1-2048 bytes".into(), + ), + )); + } + if let Some(ref sort) = req.sort_order + && !VALID_SORT_ORDERS.contains(&sort.as_str()) { + return Err(ApiError( + pinakes_core::error::PinakesError::InvalidOperation(format!( + "sort_order must be one of: {}", + VALID_SORT_ORDERS.join(", ") + )), + )); + } let id = uuid::Uuid::now_v7(); state .storage diff --git a/crates/pinakes-server/src/routes/search.rs b/crates/pinakes-server/src/routes/search.rs index 7f0e6b1..eacec6e 100644 --- a/crates/pinakes-server/src/routes/search.rs +++ b/crates/pinakes-server/src/routes/search.rs @@ -2,10 +2,7 @@ use axum::{ Json, extract::{Query, State}, }; -use pinakes_core::{ - model::Pagination, - search::{SearchRequest, SortOrder, parse_search_query}, -}; +use pinakes_core::search::{SearchRequest, SortOrder, parse_search_query}; use crate::{ dto::{MediaResponse, SearchParams, SearchRequestBody, SearchResponse}, @@ -43,11 +40,7 @@ pub async fn search( let request = SearchRequest { query, sort, - pagination: Pagination::new( - params.offset.unwrap_or(0), - params.limit.unwrap_or(50).min(1000), - None, - ), + pagination: params.to_pagination(), }; let results = state.storage.search(&request).await?; @@ -81,11 +74,7 @@ pub async fn search_post( let request = SearchRequest { query, sort, - pagination: Pagination::new( - body.offset.unwrap_or(0), - body.limit.unwrap_or(50).min(1000), - None, - ), + pagination: body.to_pagination(), }; let results = state.storage.search(&request).await?; diff --git a/crates/pinakes-server/src/routes/shares.rs b/crates/pinakes-server/src/routes/shares.rs index 76fea3c..39d00d9 100644 --- a/crates/pinakes-server/src/routes/shares.rs +++ b/crates/pinakes-server/src/routes/shares.rs @@ -207,11 +207,7 @@ pub async fn list_outgoing( Query(params): Query, ) -> ApiResult>> { let user_id = resolve_user_id(&state.storage, &username).await?; - let pagination = Pagination { - offset: params.offset.unwrap_or(0), - limit: params.limit.unwrap_or(50).min(1000), - sort: params.sort, - }; + let pagination = params.to_pagination(); let shares = state .storage @@ -230,11 +226,7 @@ pub async fn list_incoming( Query(params): Query, ) -> ApiResult>> { let user_id = resolve_user_id(&state.storage, &username).await?; - let pagination = Pagination { - offset: params.offset.unwrap_or(0), - limit: params.limit.unwrap_or(50).min(1000), - sort: params.sort, - }; + let pagination = params.to_pagination(); let shares = state .storage @@ -406,6 +398,9 @@ pub async fn batch_delete( Extension(username): Extension, Json(req): Json, ) -> ApiResult> { + if req.share_ids.is_empty() || req.share_ids.len() > 100 { + return Err(ApiError::bad_request("share_ids must contain 1-100 items")); + } let user_id = resolve_user_id(&state.storage, &username).await?; let share_ids: Vec = req.share_ids.into_iter().map(ShareId).collect(); @@ -624,11 +619,7 @@ pub async fn get_activity( )); } - let pagination = Pagination { - offset: params.offset.unwrap_or(0), - limit: params.limit.unwrap_or(50).min(1000), - sort: params.sort, - }; + let pagination = params.to_pagination(); let activity = state .storage diff --git a/crates/pinakes-server/src/routes/social.rs b/crates/pinakes-server/src/routes/social.rs index f5bc17a..116146b 100644 --- a/crates/pinakes-server/src/routes/social.rs +++ b/crates/pinakes-server/src/routes/social.rs @@ -40,6 +40,17 @@ pub async fn rate_media( ), )); } + if req + .review_text + .as_ref() + .is_some_and(|t| t.chars().count() > 10_000) + { + return Err(ApiError( + pinakes_core::error::PinakesError::InvalidOperation( + "review_text must not exceed 10000 characters".into(), + ), + )); + } let user_id = resolve_user_id(&state.storage, &username).await?; let rating = state .storage @@ -139,6 +150,13 @@ pub async fn create_share_link( Extension(username): Extension, Json(req): Json, ) -> Result, ApiError> { + if req.password.as_ref().is_some_and(|p| p.len() > 1024) { + return Err(ApiError( + pinakes_core::error::PinakesError::InvalidOperation( + "password must not exceed 1024 bytes".into(), + ), + )); + } let user_id = resolve_user_id(&state.storage, &username).await?; let token = uuid::Uuid::now_v7().to_string().replace('-', ""); let password_hash = match req.password.as_ref() { @@ -178,6 +196,13 @@ pub async fn access_shared_media( Path(token): Path, Query(query): Query, ) -> Result, ApiError> { + if query.password.as_ref().is_some_and(|p| p.len() > 1024) { + return Err(ApiError( + pinakes_core::error::PinakesError::InvalidOperation( + "password must not exceed 1024 bytes".into(), + ), + )); + } let link = state.storage.get_share_link(&token).await?; // Check expiration if let Some(expires) = link.expires_at diff --git a/crates/pinakes-server/src/routes/subtitles.rs b/crates/pinakes-server/src/routes/subtitles.rs index 3e94770..b8be6ca 100644 --- a/crates/pinakes-server/src/routes/subtitles.rs +++ b/crates/pinakes-server/src/routes/subtitles.rs @@ -47,6 +47,13 @@ pub async fn add_subtitle( ), )); } + if req + .language + .as_ref() + .is_some_and(|l| l.is_empty() || l.len() > 64) + { + return Err(ApiError::bad_request("language must be 1-64 bytes")); + } let subtitle = Subtitle { id: Uuid::now_v7(), media_id: MediaId(id), diff --git a/crates/pinakes-server/src/routes/transcode.rs b/crates/pinakes-server/src/routes/transcode.rs index b98ad4f..c57becb 100644 --- a/crates/pinakes-server/src/routes/transcode.rs +++ b/crates/pinakes-server/src/routes/transcode.rs @@ -16,6 +16,9 @@ pub async fn start_transcode( Path(id): Path, Json(req): Json, ) -> Result, ApiError> { + if req.profile.is_empty() || req.profile.len() > 255 { + return Err(ApiError::bad_request("profile must be 1-255 bytes")); + } let job_id = state .job_queue .submit(pinakes_core::jobs::JobKind::Transcode { diff --git a/crates/pinakes-server/src/routes/users.rs b/crates/pinakes-server/src/routes/users.rs index 65dfbd2..e97e8a5 100644 --- a/crates/pinakes-server/src/routes/users.rs +++ b/crates/pinakes-server/src/routes/users.rs @@ -161,12 +161,28 @@ pub async fn get_user_libraries( )) } +fn validate_root_path(path: &str) -> Result<(), ApiError> { + if path.is_empty() || path.len() > 4096 { + return Err(ApiError::bad_request("root_path must be 1-4096 bytes")); + } + if !path.starts_with('/') { + return Err(ApiError::bad_request("root_path must be an absolute path")); + } + if path.split('/').any(|segment| segment == "..") { + return Err(ApiError::bad_request( + "root_path must not contain '..' traversal components", + )); + } + Ok(()) +} + /// Grant library access to a user (admin only) pub async fn grant_library_access( State(state): State, Path(id): Path, Json(req): Json, ) -> Result, ApiError> { + validate_root_path(&req.root_path)?; let user_id: UserId = id.parse::().map(UserId::from).map_err(|_| { ApiError(pinakes_core::error::PinakesError::InvalidOperation( @@ -191,6 +207,7 @@ pub async fn revoke_library_access( Path(id): Path, Json(req): Json, ) -> Result, ApiError> { + validate_root_path(&req.root_path)?; let user_id: UserId = id.parse::().map(UserId::from).map_err(|_| { ApiError(pinakes_core::error::PinakesError::InvalidOperation( diff --git a/crates/pinakes-ui/src/components/markdown_viewer.rs b/crates/pinakes-ui/src/components/markdown_viewer.rs index ec3454f..35f20f2 100644 --- a/crates/pinakes-ui/src/components/markdown_viewer.rs +++ b/crates/pinakes-ui/src/components/markdown_viewer.rs @@ -235,13 +235,24 @@ fn render_markdown(text: &str) -> String { /// Convert wikilinks [[target]] and [[target|display]] to styled HTML links. /// Uses a special URL scheme that can be intercepted by click handlers. +/// +/// # Panics +/// +/// Never panics because the regex patterns are hardcoded and syntactically +/// valid. +#[expect(clippy::expect_used)] fn convert_wikilinks(text: &str) -> String { use regex::Regex; // Match embeds ![[target]] first, convert to a placeholder image/embed span - let embed_re = Regex::new(r"!\[\[([^\]|]+)(?:\|([^\]]+))?\]\]").unwrap(); + let embed_re = Regex::new(r"!\[\[([^\]|]+)(?:\|([^\]]+))?\]\]") + .expect("invalid regex pattern for wikilink embeds"); let text = embed_re.replace_all(text, |caps: ®ex::Captures| { - let target = caps.get(1).unwrap().as_str().trim(); + let target = caps + .get(1) + .expect("capture group 1 always exists for wikilink embeds") + .as_str() + .trim(); let alt = caps.get(2).map(|m| m.as_str().trim()).unwrap_or(target); format!( " String { }); // Match wikilinks [[target]] or [[target|display]] - let wikilink_re = Regex::new(r"\[\[([^\]|]+)(?:\|([^\]]+))?\]\]").unwrap(); + let wikilink_re = Regex::new(r"\[\[([^\]|]+)(?:\|([^\]]+))?\]\]") + .expect("invalid regex pattern for wikilinks"); let text = wikilink_re.replace_all(&text, |caps: ®ex::Captures| { - let target = caps.get(1).unwrap().as_str().trim(); + let target = caps + .get(1) + .expect("capture group 1 always exists for wikilinks") + .as_str() + .trim(); let display = caps.get(2).map(|m| m.as_str().trim()).unwrap_or(target); // Create a styled link that uses a special pseudo-protocol scheme // This makes it easier to intercept clicks via JavaScript