Merge pull request #70 from hotdata-dev/fix/empty-catalog

fix: handle empty catalogs where data directory does not exist

Author: zfarrell

src/path_resolver.rs

@@ -174,13 +174,23 @@ fn parse_file_url(data_path: &str) -> Result<(ObjectStoreUrl, String)> {
 
 /// Parse a local filesystem path into ObjectStoreUrl and key path
 fn parse_local_path(data_path: &str) -> Result<(ObjectStoreUrl, String)> {
+    // Reject paths containing ".." to prevent directory traversal attacks.
+    // std::path::absolute() does NOT normalize ".." components, so a malicious
+    // data_path like "/data/../../../etc/" would pass through unchanged.
+    if data_path.split(['/', '\\']).any(|c| c == "..") {
+        return Err(DuckLakeError::InvalidConfig(
+            "Path contains '..' traversal component".to_string(),
+        ));
+    }
+
     let has_trailing_slash = data_path.ends_with('/') || data_path.ends_with('\\');
 
-    let absolute_path = std::path::PathBuf::from(data_path)
-        .canonicalize()
-        .map_err(|e| {
-            DuckLakeError::InvalidConfig(format!("Failed to resolve path '{}': {}", data_path, e))
-        })?;
+    // Use std::path::absolute() instead of canonicalize() so that the path
+    // does not need to exist on disk yet. This supports empty catalogs where
+    // the data directory has not been created (#57).
+    let absolute_path = std::path::absolute(data_path).map_err(|e| {
+        DuckLakeError::InvalidConfig(format!("Failed to resolve path '{}': {}", data_path, e))
+    })?;
 
     let object_store_url = ObjectStoreUrl::parse("file:///").map_err(|e| {
         DuckLakeError::InvalidConfig(format!("Failed to create ObjectStoreUrl: {}", e))
@@ -523,16 +533,40 @@ mod tests {
 
     #[test]
     fn test_parse_local_path_nonexistent() {
-        let result = parse_object_store_url("/nonexistent/path/that/does/not/exist");
+        // Non-existent paths should now succeed (#57) - the directory does not
+        // need to exist at catalog creation time (e.g., empty catalogs).
+        let (url, path) = parse_object_store_url("/nonexistent/path/that/does/not/exist").unwrap();
+        assert_eq!(url, ObjectStoreUrl::parse("file:///").unwrap());
+        assert_eq!(path, "/nonexistent/path/that/does/not/exist");
+    }
+
+    #[test]
+    fn test_parse_local_path_nonexistent_with_trailing_slash() {
+        let (url, path) = parse_object_store_url("/nonexistent/data/path/").unwrap();
+        assert_eq!(url, ObjectStoreUrl::parse("file:///").unwrap());
+        assert_eq!(path, "/nonexistent/data/path/");
+    }
+
+    #[test]
+    fn test_reject_dotdot_in_local_path() {
+        // Local paths with ".." must be rejected since std::path::absolute()
+        // does not normalize them away.
+        let result = parse_object_store_url("/data/../../../etc/passwd");
         assert!(result.is_err());
+        let err = result.unwrap_err().to_string();
         assert!(
-            result
-                .unwrap_err()
-                .to_string()
-                .contains("Failed to resolve path")
+            err.contains("'..'"),
+            "error should mention '..' traversal: {}",
+            err
         );
     }
 
+    #[test]
+    fn test_reject_dotdot_in_local_path_backslash() {
+        let result = parse_object_store_url("/data\\..\\secret");
+        assert!(result.is_err());
+    }
+
     #[test]
     fn test_join_paths_nested_relative() {
         assert_eq!(