Skip to main content

max / audiofiles

Add sibling name conflict checks, cross-VFS move rejection, symlink-safe walkdir VFS rename and move now check for sibling name conflicts before applying. Move rejects cross-VFS transfers. Mirror walkdir uses symlink_metadata to avoid following symlinks (prevents infinite loops and path escapes). Sanitize preserves dot-prefixed names (.hidden) while still blocking . and .. traversal. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Author: Max J. <87768334+MaxJMath@users.noreply.github.com> · 2026-04-26 19:38 UTC
Commit: feca3a65234497182634294602f4bb9f6292f08f
Parent: f19a9ec
2 files changed, +74 insertions, -14 deletions
M crates/audiofiles-core/src/vfs.rs +53 -5
@@ -190,6 +190,36 @@ fn check_root_name_conflict(
190 190 Ok(())
191 191 }
192 192
193 + /// Check for sibling name conflicts, handling both root (NULL parent) and
194 + /// non-root cases. Excludes `exclude_id` so a node doesn't conflict with itself
195 + /// (needed for rename).
196 + fn check_sibling_name_conflict(
197 + db: &Database,
198 + vfs_id: VfsId,
199 + parent_id: Option<NodeId>,
200 + name: &str,
201 + exclude_id: NodeId,
202 + ) -> Result<()> {
203 + let count: i64 = match parent_id {
204 + None => db.conn().query_row(
205 + "SELECT COUNT(*) FROM vfs_nodes \
206 + WHERE vfs_id = ?1 AND parent_id IS NULL AND name = ?2 AND id != ?3",
207 + rusqlite::params![vfs_id, name, exclude_id],
208 + |row| row.get(0),
209 + )?,
210 + Some(pid) => db.conn().query_row(
211 + "SELECT COUNT(*) FROM vfs_nodes \
212 + WHERE parent_id = ?1 AND name = ?2 AND id != ?3",
213 + rusqlite::params![pid, name, exclude_id],
214 + |row| row.get(0),
215 + )?,
216 + };
217 + if count > 0 {
218 + return Err(CoreError::NameConflict(name.to_string()));
219 + }
220 + Ok(())
221 + }
222 +
193 223 /// Create a directory node under the given parent (or at root if `None`). Returns the new node ID.
194 224 #[instrument(skip_all)]
195 225 pub fn create_directory(
@@ -284,10 +314,13 @@ pub fn get_node(db: &Database, id: NodeId) -> Result<VfsNode> {
284 314 })
285 315 }
286 316
287 - /// Rename a VFS node. Returns `NodeNotFound` if the ID doesn't exist.
317 + /// Rename a VFS node. Returns `NodeNotFound` if the ID doesn't exist,
318 + /// `NameConflict` if a sibling with the same name already exists.
288 319 #[instrument(skip_all)]
289 320 pub fn rename_node(db: &Database, id: NodeId, new_name: &str) -> Result<()> {
290 321 validate_node_name(new_name)?;
322 + let node = get_node(db, id)?;
323 + check_sibling_name_conflict(db, node.vfs_id, node.parent_id, new_name, id)?;
291 324 let changed = db.conn().execute(
292 325 "UPDATE vfs_nodes SET name = ?1 WHERE id = ?2",
293 326 rusqlite::params![new_name, id],
@@ -300,10 +333,22 @@ pub fn rename_node(db: &Database, id: NodeId, new_name: &str) -> Result<()> {
300 333
301 334 /// Move a VFS node to a new parent directory (or root if `None`).
302 335 ///
303 - /// Returns an error if the move would create a circular parent reference
304 - /// (e.g. moving a node to be a child of one of its own descendants).
336 + /// Returns an error if the move would create a circular parent reference,
337 + /// cross a VFS boundary, or conflict with an existing sibling name.
305 338 #[instrument(skip_all)]
306 339 pub fn move_node(db: &Database, id: NodeId, new_parent_id: Option<NodeId>) -> Result<()> {
340 + let node = get_node(db, id)?;
341 +
342 + // Reject cross-VFS moves.
343 + if let Some(parent) = new_parent_id {
344 + let parent_node = get_node(db, parent)?;
345 + if parent_node.vfs_id != node.vfs_id {
346 + return Err(CoreError::Internal(
347 + "cannot move a node to a different VFS".to_string(),
348 + ));
349 + }
350 + }
351 +
307 352 // Check for circular reference: walk from new_parent_id up to root.
308 353 // If we encounter `id` along the way, the move would create a cycle.
309 354 if let Some(parent) = new_parent_id {
@@ -314,11 +359,14 @@ pub fn move_node(db: &Database, id: NodeId, new_parent_id: Option<NodeId>) -> Re
314 359 "move would create a circular parent reference".to_string(),
315 360 ));
316 361 }
317 - let node = get_node(db, cur_id)?;
318 - current = node.parent_id;
362 + let cur_node = get_node(db, cur_id)?;
363 + current = cur_node.parent_id;
319 364 }
320 365 }
321 366
367 + // Check for name conflicts at the destination.
368 + check_sibling_name_conflict(db, node.vfs_id, new_parent_id, &node.name, id)?;
369 +
322 370 let changed = db.conn().execute(
323 371 "UPDATE vfs_nodes SET parent_id = ?1 WHERE id = ?2",
324 372 rusqlite::params![new_parent_id, id],
M crates/audiofiles-core/src/vfs_mirror.rs +21 -9
@@ -159,14 +159,24 @@ fn walkdir(root: &Path) -> std::io::Result<Vec<PathBuf>> {
159 159 }
160 160
161 161 fn walkdir_inner(dir: &Path, out: &mut Vec<PathBuf>) -> std::io::Result<()> {
162 - if !dir.is_dir() {
162 + // Use symlink_metadata to avoid following symlinks (prevents infinite loops
163 + // and escaping the mirror root via directory symlinks).
164 + let meta = match std::fs::symlink_metadata(dir) {
165 + Ok(m) => m,
166 + Err(_) => return Ok(()),
167 + };
168 + if !meta.is_dir() {
163 169 return Ok(());
164 170 }
165 171 for entry in std::fs::read_dir(dir)? {
166 172 let entry = entry?;
167 173 let path = entry.path();
168 174 out.push(path.clone());
169 - if path.is_dir() {
175 + let entry_meta = match std::fs::symlink_metadata(&path) {
176 + Ok(m) => m,
177 + Err(_) => continue,
178 + };
179 + if entry_meta.is_dir() {
170 180 walkdir_inner(&path, out)?;
171 181 }
172 182 }
@@ -183,12 +193,12 @@ fn sanitize_path(path: &str) -> String {
183 193 }
184 194
185 195 /// Sanitize a single path component.
196 + /// Replaces null bytes, rejects `.` and `..` to prevent traversal.
197 + /// Preserves leading dots on other names (e.g. `.hidden` stays `.hidden`).
186 198 fn sanitize_component(name: &str) -> String {
187 - let mut s = name.replace('\0', "_");
188 - // Trim leading/trailing dots that could create hidden files or . / ..
189 - s = s.trim_matches('.').to_string();
190 - if s.is_empty() {
191 - s = "_".to_string();
199 + let s = name.replace('\0', "_");
200 + if s == "." || s == ".." || s.is_empty() {
201 + return "_".to_string();
192 202 }
193 203 s
194 204 }
@@ -355,9 +365,11 @@ mod tests {
355 365 fn sanitize_path_handles_special_chars() {
356 366 assert_eq!(sanitize_path("Library/Drums"), "Library/Drums");
357 367 assert_eq!(sanitize_path("a\0b/c"), "a_b/c");
358 - // Leading/trailing dots stripped per component
368 + // Only . and .. are replaced to prevent traversal; other dot-prefixed names preserved
359 369 assert_eq!(sanitize_component(".."), "_");
360 - assert_eq!(sanitize_component(".hidden"), "hidden");
370 + assert_eq!(sanitize_component("."), "_");
371 + assert_eq!(sanitize_component(".hidden"), ".hidden");
361 372 assert_eq!(sanitize_component(""), "_");
373 + assert_eq!(sanitize_component("normal"), "normal");
362 374 }
363 375 }