From bf4a8a1b43b1606b7c21586c098c561525cd98ab Mon Sep 17 00:00:00 2001 From: Cameron Date: Sat, 18 Apr 2026 16:38:28 -0400 Subject: [PATCH] fix: validate gps-summary path against every library MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The /photos/gps-summary handler validated the incoming path against the primary library's root with new_file=false, which requires the path to exist on disk. For a viewer opened on a file from a non-primary library, tapping the GPS link produced activePath = , the primary-only check failed, and the server 400'd — so the map came up empty. Validation here is purely a traversal guard (the DAO does a prefix LIKE against rel_path), so we now accept the path as long as any configured library can resolve it without escaping its root. Also applies cargo fmt drift on files touched this session. Co-Authored-By: Claude Opus 4.7 (1M context) --- src/ai/insight_generator.rs | 20 ++++++------- src/files.rs | 31 ++++++++++--------- src/main.rs | 59 ++++++++++++++++++------------------- src/video/actors.rs | 4 +-- 4 files changed, 55 insertions(+), 59 deletions(-) diff --git a/src/ai/insight_generator.rs b/src/ai/insight_generator.rs index 07881c6..0e2ab94 100644 --- a/src/ai/insight_generator.rs +++ b/src/ai/insight_generator.rs @@ -1902,14 +1902,10 @@ Return ONLY the summary, nothing else."#, // those already). Results are appended to the tool response so the // model can choose to use an existing entity's ID instead. let similar_entities: Vec = { - use crate::database::{EntityFilter, KnowledgeDao}; use crate::database::knowledge_dao::normalize_entity_type; + use crate::database::{EntityFilter, KnowledgeDao}; let normalised_type = normalize_entity_type(&entity_type); - let first_token = name - .split_whitespace() - .next() - .unwrap_or(&name) - .to_string(); + let first_token = name.split_whitespace().next().unwrap_or(&name).to_string(); let filter = EntityFilter { entity_type: None, // search all types, filter client-side to avoid case issues status: Some("active".to_string()), @@ -1917,7 +1913,10 @@ Return ONLY the summary, nothing else."#, limit: 10, offset: 0, }; - let mut kdao = self.knowledge_dao.lock().expect("Unable to lock KnowledgeDao"); + let mut kdao = self + .knowledge_dao + .lock() + .expect("Unable to lock KnowledgeDao"); kdao.list_entities(cx, filter) .unwrap_or_default() .0 @@ -2725,10 +2724,9 @@ Return ONLY the summary, nothing else."#, messages.push(ChatMessage::user( "Based on the context gathered, please write the final photo insight: a title and a detailed personal summary. Write in first person as Cameron.", )); - let (final_response, prompt_tokens, eval_tokens) = - ollama_client - .chat_with_tools(messages.clone(), vec![]) - .await?; + let (final_response, prompt_tokens, eval_tokens) = ollama_client + .chat_with_tools(messages.clone(), vec![]) + .await?; last_prompt_eval_count = prompt_tokens; last_eval_count = eval_tokens; final_content = final_response.content.clone(); diff --git a/src/files.rs b/src/files.rs index 50d5c93..d83325e 100644 --- a/src/files.rs +++ b/src/files.rs @@ -241,10 +241,8 @@ pub async fn list_photos( // Resolve the optional library filter. Unknown values return 400. // For Phase 3 the filesystem walk still operates against a single // library's root; Phase 4 introduces multi-root union scanning. - let library = match crate::libraries::resolve_library_param( - &app_state, - req.library.as_deref(), - ) { + let library = match crate::libraries::resolve_library_param(&app_state, req.library.as_deref()) + { Ok(lib) => lib, Err(msg) => { log::warn!("Rejecting /photos request: {}", msg); @@ -560,8 +558,9 @@ pub async fn list_photos( .fold((Vec::new(), Vec::new()), |(mut files, mut dirs), path| { match path.metadata() { Ok(md) => { - let relative = - path.strip_prefix(&scoped_library.root_path).unwrap_or_else(|_| { + let relative = path + .strip_prefix(&scoped_library.root_path) + .unwrap_or_else(|_| { panic!( "Unable to strip library root {} from file path {}", &scoped_library.root_path, @@ -572,10 +571,7 @@ pub async fn list_photos( // lookups (tags, EXIF, insights) that store // rel_paths with forward slashes still match // on Windows. - let relative_str = relative - .to_str() - .unwrap() - .replace('\\', "/"); + let relative_str = relative.to_str().unwrap().replace('\\', "/"); if md.is_file() { files.push(relative_str); @@ -1024,13 +1020,20 @@ pub async fn get_gps_summary( let cx = opentelemetry::Context::current_with_span(span); - // The database stores relative paths, so we use the path as-is - // Normalize empty path or "/" to return all GPS photos + // The database stores relative paths, so we use the path as-is. + // Normalize empty path or "/" to return all GPS photos. Validation + // is purely a traversal guard — the path need not exist on disk + // under any particular library, because the DAO just does a prefix + // match against image_exif.rel_path (which is library-agnostic for + // this summary query). let requested_path = if req.path.is_empty() || req.path == "/" { String::new() } else { - // Validate path using the same check as all other endpoints - if is_valid_full_path(&app_state.base_path, &req.path, false).is_none() { + let req_path = PathBuf::from(&req.path); + let validated = app_state.libraries.iter().any(|lib| { + is_valid_full_path(&PathBuf::from(&lib.root_path), &req_path, true).is_some() + }); + if !validated { warn!("Invalid path for GPS summary: {}", req.path); cx.span().set_status(Status::error("Invalid path")); return Ok(HttpResponse::BadRequest().json(serde_json::json!({ diff --git a/src/main.rs b/src/main.rs index af20c4e..53e96cf 100644 --- a/src/main.rs +++ b/src/main.rs @@ -55,13 +55,13 @@ use opentelemetry::{KeyValue, global}; mod ai; mod auth; +mod content_hash; mod data; mod database; mod error; mod exif; mod file_types; mod files; -mod content_hash; mod geo; mod libraries; mod state; @@ -106,10 +106,7 @@ async fn get_image( // Resolve library from query param; default to primary so clients that // don't yet send `library=` continue to work. - let library = match libraries::resolve_library_param( - &app_state, - req.library.as_deref(), - ) { + let library = match libraries::resolve_library_param(&app_state, req.library.as_deref()) { Ok(Some(lib)) => lib, Ok(None) => app_state.primary_library(), Err(msg) => { @@ -339,8 +336,7 @@ async fn get_file_metadata( File::open(&full_path) .and_then(|file| file.metadata()) .map(|metadata| (lib, metadata)) - }) - { + }) { Ok((resolved_library, metadata)) => { let mut response: MetadataResponse = metadata.into(); response.library_id = Some(resolved_library.id); @@ -397,17 +393,15 @@ async fn upload_image( // Resolve the optional library selector. Absent → primary library // (backwards-compatible with clients that don't yet send `library=`). - let target_library = match libraries::resolve_library_param( - &app_state, - query.library.as_deref(), - ) { - Ok(Some(lib)) => lib, - Ok(None) => app_state.primary_library(), - Err(msg) => { - span.set_status(Status::error(msg.clone())); - return HttpResponse::BadRequest().body(msg); - } - }; + let target_library = + match libraries::resolve_library_param(&app_state, query.library.as_deref()) { + Ok(Some(lib)) => lib, + Ok(None) => app_state.primary_library(), + Err(msg) => { + span.set_status(Status::error(msg.clone())); + return HttpResponse::BadRequest().body(msg); + } + }; let mut file_content: BytesMut = BytesMut::new(); let mut file_name: Option = None; @@ -496,18 +490,18 @@ async fn upload_image( match exif::extract_exif_from_path(&uploaded_path) { Ok(exif_data) => { let timestamp = Utc::now().timestamp(); - let (content_hash, size_bytes) = - match content_hash::compute(&uploaded_path) { - Ok(id) => (Some(id.content_hash), Some(id.size_bytes)), - Err(e) => { - warn!( - "Failed to hash uploaded {}: {:?}", - uploaded_path.display(), - e - ); - (None, None) - } - }; + let (content_hash, size_bytes) = match content_hash::compute(&uploaded_path) + { + Ok(id) => (Some(id.content_hash), Some(id.size_bytes)), + Err(e) => { + warn!( + "Failed to hash uploaded {}: {:?}", + uploaded_path.display(), + e + ); + (None, None) + } + }; let insert_exif = InsertImageExif { library_id: target_library.id, file_path: relative_path.clone(), @@ -1827,7 +1821,10 @@ fn process_new_files( let mut dao = exif_dao.lock().expect("Unable to lock ExifDao"); if let Err(e) = dao.store_exif(&context, insert_exif) { - error!("Failed to register {} in image_exif: {:?}", relative_path, e); + error!( + "Failed to register {} in image_exif: {:?}", + relative_path, e + ); } else { debug!("Registered {} in image_exif", relative_path); } diff --git a/src/video/actors.rs b/src/video/actors.rs index 8af2482..284c8e3 100644 --- a/src/video/actors.rs +++ b/src/video/actors.rs @@ -526,9 +526,7 @@ impl PreviewClipGenerator { fn relativize(&self, video_path: &str) -> String { for lib in &self.libraries { if let Some(stripped) = video_path.strip_prefix(&lib.root_path) { - return stripped - .trim_start_matches(['/', '\\']) - .replace('\\', "/"); + return stripped.trim_start_matches(['/', '\\']).replace('\\', "/"); } } video_path