fix: validate gps-summary path against every library
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 = <folder from lib 2>, 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) <noreply@anthropic.com>
This commit is contained in:
@@ -1902,14 +1902,10 @@ Return ONLY the summary, nothing else."#,
|
|||||||
// those already). Results are appended to the tool response so the
|
// those already). Results are appended to the tool response so the
|
||||||
// model can choose to use an existing entity's ID instead.
|
// model can choose to use an existing entity's ID instead.
|
||||||
let similar_entities: Vec<String> = {
|
let similar_entities: Vec<String> = {
|
||||||
use crate::database::{EntityFilter, KnowledgeDao};
|
|
||||||
use crate::database::knowledge_dao::normalize_entity_type;
|
use crate::database::knowledge_dao::normalize_entity_type;
|
||||||
|
use crate::database::{EntityFilter, KnowledgeDao};
|
||||||
let normalised_type = normalize_entity_type(&entity_type);
|
let normalised_type = normalize_entity_type(&entity_type);
|
||||||
let first_token = name
|
let first_token = name.split_whitespace().next().unwrap_or(&name).to_string();
|
||||||
.split_whitespace()
|
|
||||||
.next()
|
|
||||||
.unwrap_or(&name)
|
|
||||||
.to_string();
|
|
||||||
let filter = EntityFilter {
|
let filter = EntityFilter {
|
||||||
entity_type: None, // search all types, filter client-side to avoid case issues
|
entity_type: None, // search all types, filter client-side to avoid case issues
|
||||||
status: Some("active".to_string()),
|
status: Some("active".to_string()),
|
||||||
@@ -1917,7 +1913,10 @@ Return ONLY the summary, nothing else."#,
|
|||||||
limit: 10,
|
limit: 10,
|
||||||
offset: 0,
|
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)
|
kdao.list_entities(cx, filter)
|
||||||
.unwrap_or_default()
|
.unwrap_or_default()
|
||||||
.0
|
.0
|
||||||
@@ -2725,8 +2724,7 @@ Return ONLY the summary, nothing else."#,
|
|||||||
messages.push(ChatMessage::user(
|
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.",
|
"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) =
|
let (final_response, prompt_tokens, eval_tokens) = ollama_client
|
||||||
ollama_client
|
|
||||||
.chat_with_tools(messages.clone(), vec![])
|
.chat_with_tools(messages.clone(), vec![])
|
||||||
.await?;
|
.await?;
|
||||||
last_prompt_eval_count = prompt_tokens;
|
last_prompt_eval_count = prompt_tokens;
|
||||||
|
|||||||
31
src/files.rs
31
src/files.rs
@@ -241,10 +241,8 @@ pub async fn list_photos<TagD: TagDao, FS: FileSystemAccess>(
|
|||||||
// Resolve the optional library filter. Unknown values return 400.
|
// Resolve the optional library filter. Unknown values return 400.
|
||||||
// For Phase 3 the filesystem walk still operates against a single
|
// For Phase 3 the filesystem walk still operates against a single
|
||||||
// library's root; Phase 4 introduces multi-root union scanning.
|
// library's root; Phase 4 introduces multi-root union scanning.
|
||||||
let library = match crate::libraries::resolve_library_param(
|
let library = match crate::libraries::resolve_library_param(&app_state, req.library.as_deref())
|
||||||
&app_state,
|
{
|
||||||
req.library.as_deref(),
|
|
||||||
) {
|
|
||||||
Ok(lib) => lib,
|
Ok(lib) => lib,
|
||||||
Err(msg) => {
|
Err(msg) => {
|
||||||
log::warn!("Rejecting /photos request: {}", msg);
|
log::warn!("Rejecting /photos request: {}", msg);
|
||||||
@@ -560,8 +558,9 @@ pub async fn list_photos<TagD: TagDao, FS: FileSystemAccess>(
|
|||||||
.fold((Vec::new(), Vec::new()), |(mut files, mut dirs), path| {
|
.fold((Vec::new(), Vec::new()), |(mut files, mut dirs), path| {
|
||||||
match path.metadata() {
|
match path.metadata() {
|
||||||
Ok(md) => {
|
Ok(md) => {
|
||||||
let relative =
|
let relative = path
|
||||||
path.strip_prefix(&scoped_library.root_path).unwrap_or_else(|_| {
|
.strip_prefix(&scoped_library.root_path)
|
||||||
|
.unwrap_or_else(|_| {
|
||||||
panic!(
|
panic!(
|
||||||
"Unable to strip library root {} from file path {}",
|
"Unable to strip library root {} from file path {}",
|
||||||
&scoped_library.root_path,
|
&scoped_library.root_path,
|
||||||
@@ -572,10 +571,7 @@ pub async fn list_photos<TagD: TagDao, FS: FileSystemAccess>(
|
|||||||
// lookups (tags, EXIF, insights) that store
|
// lookups (tags, EXIF, insights) that store
|
||||||
// rel_paths with forward slashes still match
|
// rel_paths with forward slashes still match
|
||||||
// on Windows.
|
// on Windows.
|
||||||
let relative_str = relative
|
let relative_str = relative.to_str().unwrap().replace('\\', "/");
|
||||||
.to_str()
|
|
||||||
.unwrap()
|
|
||||||
.replace('\\', "/");
|
|
||||||
|
|
||||||
if md.is_file() {
|
if md.is_file() {
|
||||||
files.push(relative_str);
|
files.push(relative_str);
|
||||||
@@ -1024,13 +1020,20 @@ pub async fn get_gps_summary(
|
|||||||
|
|
||||||
let cx = opentelemetry::Context::current_with_span(span);
|
let cx = opentelemetry::Context::current_with_span(span);
|
||||||
|
|
||||||
// The database stores relative paths, so we use the path as-is
|
// The database stores relative paths, so we use the path as-is.
|
||||||
// Normalize empty path or "/" to return all GPS photos
|
// 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 == "/" {
|
let requested_path = if req.path.is_empty() || req.path == "/" {
|
||||||
String::new()
|
String::new()
|
||||||
} else {
|
} else {
|
||||||
// Validate path using the same check as all other endpoints
|
let req_path = PathBuf::from(&req.path);
|
||||||
if is_valid_full_path(&app_state.base_path, &req.path, false).is_none() {
|
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);
|
warn!("Invalid path for GPS summary: {}", req.path);
|
||||||
cx.span().set_status(Status::error("Invalid path"));
|
cx.span().set_status(Status::error("Invalid path"));
|
||||||
return Ok(HttpResponse::BadRequest().json(serde_json::json!({
|
return Ok(HttpResponse::BadRequest().json(serde_json::json!({
|
||||||
|
|||||||
25
src/main.rs
25
src/main.rs
@@ -55,13 +55,13 @@ use opentelemetry::{KeyValue, global};
|
|||||||
|
|
||||||
mod ai;
|
mod ai;
|
||||||
mod auth;
|
mod auth;
|
||||||
|
mod content_hash;
|
||||||
mod data;
|
mod data;
|
||||||
mod database;
|
mod database;
|
||||||
mod error;
|
mod error;
|
||||||
mod exif;
|
mod exif;
|
||||||
mod file_types;
|
mod file_types;
|
||||||
mod files;
|
mod files;
|
||||||
mod content_hash;
|
|
||||||
mod geo;
|
mod geo;
|
||||||
mod libraries;
|
mod libraries;
|
||||||
mod state;
|
mod state;
|
||||||
@@ -106,10 +106,7 @@ async fn get_image(
|
|||||||
|
|
||||||
// Resolve library from query param; default to primary so clients that
|
// Resolve library from query param; default to primary so clients that
|
||||||
// don't yet send `library=` continue to work.
|
// don't yet send `library=` continue to work.
|
||||||
let library = match libraries::resolve_library_param(
|
let library = match libraries::resolve_library_param(&app_state, req.library.as_deref()) {
|
||||||
&app_state,
|
|
||||||
req.library.as_deref(),
|
|
||||||
) {
|
|
||||||
Ok(Some(lib)) => lib,
|
Ok(Some(lib)) => lib,
|
||||||
Ok(None) => app_state.primary_library(),
|
Ok(None) => app_state.primary_library(),
|
||||||
Err(msg) => {
|
Err(msg) => {
|
||||||
@@ -339,8 +336,7 @@ async fn get_file_metadata(
|
|||||||
File::open(&full_path)
|
File::open(&full_path)
|
||||||
.and_then(|file| file.metadata())
|
.and_then(|file| file.metadata())
|
||||||
.map(|metadata| (lib, metadata))
|
.map(|metadata| (lib, metadata))
|
||||||
})
|
}) {
|
||||||
{
|
|
||||||
Ok((resolved_library, metadata)) => {
|
Ok((resolved_library, metadata)) => {
|
||||||
let mut response: MetadataResponse = metadata.into();
|
let mut response: MetadataResponse = metadata.into();
|
||||||
response.library_id = Some(resolved_library.id);
|
response.library_id = Some(resolved_library.id);
|
||||||
@@ -397,10 +393,8 @@ async fn upload_image(
|
|||||||
|
|
||||||
// Resolve the optional library selector. Absent → primary library
|
// Resolve the optional library selector. Absent → primary library
|
||||||
// (backwards-compatible with clients that don't yet send `library=`).
|
// (backwards-compatible with clients that don't yet send `library=`).
|
||||||
let target_library = match libraries::resolve_library_param(
|
let target_library =
|
||||||
&app_state,
|
match libraries::resolve_library_param(&app_state, query.library.as_deref()) {
|
||||||
query.library.as_deref(),
|
|
||||||
) {
|
|
||||||
Ok(Some(lib)) => lib,
|
Ok(Some(lib)) => lib,
|
||||||
Ok(None) => app_state.primary_library(),
|
Ok(None) => app_state.primary_library(),
|
||||||
Err(msg) => {
|
Err(msg) => {
|
||||||
@@ -496,8 +490,8 @@ async fn upload_image(
|
|||||||
match exif::extract_exif_from_path(&uploaded_path) {
|
match exif::extract_exif_from_path(&uploaded_path) {
|
||||||
Ok(exif_data) => {
|
Ok(exif_data) => {
|
||||||
let timestamp = Utc::now().timestamp();
|
let timestamp = Utc::now().timestamp();
|
||||||
let (content_hash, size_bytes) =
|
let (content_hash, size_bytes) = match content_hash::compute(&uploaded_path)
|
||||||
match content_hash::compute(&uploaded_path) {
|
{
|
||||||
Ok(id) => (Some(id.content_hash), Some(id.size_bytes)),
|
Ok(id) => (Some(id.content_hash), Some(id.size_bytes)),
|
||||||
Err(e) => {
|
Err(e) => {
|
||||||
warn!(
|
warn!(
|
||||||
@@ -1827,7 +1821,10 @@ fn process_new_files(
|
|||||||
|
|
||||||
let mut dao = exif_dao.lock().expect("Unable to lock ExifDao");
|
let mut dao = exif_dao.lock().expect("Unable to lock ExifDao");
|
||||||
if let Err(e) = dao.store_exif(&context, insert_exif) {
|
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 {
|
} else {
|
||||||
debug!("Registered {} in image_exif", relative_path);
|
debug!("Registered {} in image_exif", relative_path);
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -526,9 +526,7 @@ impl PreviewClipGenerator {
|
|||||||
fn relativize(&self, video_path: &str) -> String {
|
fn relativize(&self, video_path: &str) -> String {
|
||||||
for lib in &self.libraries {
|
for lib in &self.libraries {
|
||||||
if let Some(stripped) = video_path.strip_prefix(&lib.root_path) {
|
if let Some(stripped) = video_path.strip_prefix(&lib.root_path) {
|
||||||
return stripped
|
return stripped.trim_start_matches(['/', '\\']).replace('\\', "/");
|
||||||
.trim_start_matches(['/', '\\'])
|
|
||||||
.replace('\\', "/");
|
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
video_path
|
video_path
|
||||||
|
|||||||
Reference in New Issue
Block a user