Skip to content
Merged
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
38 changes: 38 additions & 0 deletions core/src/common/utils.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,3 +3,41 @@
// Note: Device ID management has been moved to device::manager for better
// module organization. Import from there instead:
// use crate::device::manager::{get_current_device_id, set_current_device_id};

/// Strip Windows extended path prefixes produced by `std::fs::canonicalize()`.
///
/// On Windows, `canonicalize()` returns paths like `\\?\C:\...` (local) or
/// `\\?\UNC\server\share\...` (network). These prefixes break `starts_with()`
/// matching throughout the codebase and must be normalized.
///
/// - `\\?\UNC\server\share\...` → `\\server\share\...`
/// - `\\?\C:\...` → `C:\...`
/// - All other paths are returned unchanged.
#[cfg(windows)]
pub fn strip_windows_extended_prefix(path: std::path::PathBuf) -> std::path::PathBuf {
if let Some(s) = path.to_str() {
if s.starts_with(r"\\?\UNC\") {
// \\?\UNC\server\share\... → \\server\share\...
std::path::PathBuf::from(format!(r"\\{}", &s[8..]))
} else if let Some(stripped) = s.strip_prefix(r"\\?\") {
// Only strip \\?\ when followed by a drive letter (e.g. C:\).
// Leave volume GUIDs (\\?\Volume{...}\) and other verbatim
// forms untouched — they are invalid without the prefix.
if stripped.as_bytes().get(1) == Some(&b':') {
std::path::PathBuf::from(stripped)
} else {
path
}
} else {
Comment on lines +22 to +31
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Restrict \\?\ stripping to canonical drive-letter paths only.

On Line 21, the generic strip_prefix(r"\\?\") branch also rewrites non-drive verbatim paths (for example \\?\Volume{...}), which can produce invalid relative-like paths. Please gate this branch to drive-letter forms (C:\...) and leave other verbatim forms unchanged.

Proposed fix
-		} else if let Some(stripped) = s.strip_prefix(r"\\?\") {
-			std::path::PathBuf::from(stripped)
+		} else if let Some(stripped) = s.strip_prefix(r"\\?\") {
+			// Only normalize canonical local-drive form (e.g. C:\...)
+			if stripped.len() >= 3
+				&& stripped.as_bytes()[1] == b':'
+				&& stripped.as_bytes()[2] == b'\\'
+			{
+				std::path::PathBuf::from(stripped)
+			} else {
+				path
+			}
 		} else {
 			path
 		}
📝 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
} else if let Some(stripped) = s.strip_prefix(r"\\?\") {
std::path::PathBuf::from(stripped)
} else {
} else if let Some(stripped) = s.strip_prefix(r"\\?\") {
// Only normalize canonical local-drive form (e.g. C:\...)
if stripped.len() >= 3
&& stripped.as_bytes()[1] == b':'
&& stripped.as_bytes()[2] == b'\\'
{
std::path::PathBuf::from(stripped)
} else {
path
}
} else {
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@core/src/common/utils.rs` around lines 21 - 23, The branch that
unconditionally strips the verbatim prefix using s.strip_prefix(r"\\?\") should
only apply to drive-letter paths; update the condition around the existing
s.strip_prefix usage so it additionally checks the remainder begins with a
drive-letter form (e.g. matches r"^[A-Za-z]:\\", or equivalently check
stripped.get(0..2) ends with ':' and stripped.get(2..3) == "\\"), and only then
construct PathBuf::from(stripped); leave other verbatim forms (like volume
GUIDs) untouched so they fall through to the existing else branch.

path
}
} else {
path
}
}

/// No-op on non-Windows platforms.
#[cfg(not(windows))]
pub fn strip_windows_extended_prefix(path: std::path::PathBuf) -> std::path::PathBuf {
path
}
24 changes: 24 additions & 0 deletions core/src/location/manager.rs
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,30 @@ impl LocationManager {
job_policies: Option<String>,
volume_manager: &crate::volume::VolumeManager,
) -> LocationResult<(Uuid, String)> {
// Canonicalize local physical paths to absolute form before storing.
// Relative paths break the watcher, volume resolution, and indexer.
// Only for local device — remote paths can't be resolved locally.
let sd_path = if sd_path.is_local() {
if let crate::domain::addressing::SdPath::Physical { device_slug, path } = sd_path {
let canonical = tokio::fs::canonicalize(&path).await.map_err(|e| {
LocationError::InvalidPath(format!(
"Failed to resolve path {}: {}",
path.display(),
e
))
})?;
let canonical = crate::common::utils::strip_windows_extended_prefix(canonical);
crate::domain::addressing::SdPath::Physical {
device_slug,
path: canonical,
}
} else {
sd_path
}
} else {
sd_path
};

info!("Adding location: {}", sd_path);

// Validate the path based on type
Expand Down
25 changes: 25 additions & 0 deletions core/src/ops/locations/add/action.rs
Original file line number Diff line number Diff line change
Expand Up @@ -102,6 +102,31 @@ impl LibraryAction for LocationAddAction {
.await
.map_err(|e| ActionError::Internal(e.to_string()))?;

// Register the new location with the filesystem watcher so changes
// (creates, deletes, renames) are detected in real-time.
// Without this, the watcher only learns about locations at startup.
if let Some(local_path) = self.input.path.as_local_path() {
if let Some(fs_watcher) = context.get_fs_watcher().await {
use crate::ops::indexing::handlers::LocationMeta;
use crate::ops::indexing::RuleToggles;

let root_path = tokio::fs::canonicalize(local_path)
.await
.unwrap_or_else(|_| local_path.to_path_buf());
let root_path = crate::common::utils::strip_windows_extended_prefix(root_path);

let meta = LocationMeta {
id: location_id,
library_id: library.id(),
root_path,
rule_toggles: RuleToggles::default(),
};
if let Err(e) = fs_watcher.watch_location(meta).await {
tracing::warn!("Failed to register location with watcher: {}", e);
}
Comment thread
coderabbitai[bot] marked this conversation as resolved.
}
}

// Parse the job ID from the string returned by add_location
let job_id = if !job_id_string.is_empty() {
Some(
Expand Down
13 changes: 12 additions & 1 deletion core/src/ops/locations/remove/action.rs
Original file line number Diff line number Diff line change
Expand Up @@ -43,13 +43,24 @@ impl LibraryAction for LocationRemoveAction {
library: std::sync::Arc<crate::library::Library>,
context: Arc<CoreContext>,
) -> Result<Self::Output, ActionError> {
// Remove the location
// Remove the location from DB
let location_manager = LocationManager::new(context.events.as_ref().clone());
location_manager
.remove_location(&library, self.input.location_id)
.await
.map_err(|e| ActionError::Internal(e.to_string()))?;

// Unwatch the location from the filesystem watcher
if let Some(watcher) = context.get_fs_watcher().await {
if let Err(e) = watcher.unwatch_location(self.input.location_id).await {
tracing::warn!(
"Failed to unwatch location {}: {}",
self.input.location_id,
e
);
}
}

Ok(LocationRemoveOutput::new(self.input.location_id, None))
}

Expand Down
13 changes: 1 addition & 12 deletions core/src/volume/fs/refs.rs
Original file line number Diff line number Diff line change
Expand Up @@ -293,18 +293,7 @@ impl super::FilesystemHandler for RefsHandler {
}

fn contains_path(&self, volume: &Volume, path: &std::path::Path) -> bool {
// Strip Windows extended path prefix (\\?\) produced by canonicalize()
let normalized_path = if let Some(path_str) = path.to_str() {
if path_str.starts_with("\\\\?\\UNC\\") {
PathBuf::from(format!("\\\\{}", &path_str[8..]))
} else if let Some(stripped) = path_str.strip_prefix("\\\\?\\") {
PathBuf::from(stripped)
} else {
path.to_path_buf()
}
} else {
path.to_path_buf()
};
let normalized_path = crate::common::utils::strip_windows_extended_prefix(path.to_path_buf());

if normalized_path.starts_with(&volume.mount_point) {
return true;
Expand Down