fastskill_core/storage/
zip.rs

1//! ZIP package handling for skill distribution
2
3use crate::core::service::ServiceError;
4use crate::security::path::normalize_path;
5use std::io;
6use std::path::Path;
7
8pub struct ZipHandler;
9
10impl ZipHandler {
11    pub fn new() -> Result<Self, ServiceError> {
12        Ok(Self)
13    }
14
15    /// Validate ZIP package structure
16    pub async fn validate_package(&self, _zip_path: &Path) -> Result<(), ServiceError> {
17        Ok(())
18    }
19
20    /// Safely extract a ZIP file to a destination directory
21    ///
22    /// This function protects against ZIP slip attacks by:
23    /// - Normalizing the output path for each entry
24    /// - Verifying that the normalized path stays within the extraction directory
25    /// - Rejecting entries that would create files outside the extraction root
26    /// - Rejecting symlink entries
27    ///
28    /// # Arguments
29    /// * `zip_path` - Path to the ZIP file to extract
30    /// * `dest_dir` - Destination directory for extraction (must exist)
31    ///
32    /// # Errors
33    /// Returns `ServiceError::Io` for I/O errors
34    /// Returns `ServiceError::Validation` if path traversal is detected
35    pub fn extract_to_dir(&self, zip_path: &Path, dest_dir: &Path) -> Result<(), ServiceError> {
36        let file = std::fs::File::open(zip_path).map_err(ServiceError::Io)?;
37        let mut archive = zip::ZipArchive::new(file)
38            .map_err(|e| ServiceError::Validation(format!("Invalid ZIP file: {}", e)))?;
39
40        // Canonicalize the destination directory for reliable path comparison
41        let dest_canonical = dest_dir.canonicalize().map_err(|e| {
42            ServiceError::Io(io::Error::new(
43                e.kind(),
44                format!("Failed to canonicalize destination: {}", e),
45            ))
46        })?;
47
48        for i in 0..archive.len() {
49            let mut file = archive.by_index(i).map_err(|e| {
50                ServiceError::Validation(format!("Failed to read ZIP entry: {}", e))
51            })?;
52
53            let entry_name = file.name().to_string();
54
55            // Reject symlink entries
56            #[cfg(unix)]
57            if matches!(file.unix_mode(), Some(mode) if (mode & 0o170000) == 0o120000) {
58                return Err(ServiceError::Validation(format!(
59                    "Symlink entry rejected for security: {}",
60                    entry_name
61                )));
62            }
63
64            // Normalize the entry name to resolve . and .. components before any I/O
65            let normalized_entry_name = normalize_path(Path::new(&entry_name));
66            if normalized_entry_name.as_os_str().is_empty() {
67                return Err(ServiceError::Validation(format!(
68                    "Path traversal attempt detected in ZIP entry: '{}'",
69                    entry_name
70                )));
71            }
72
73            // Build the output path using the normalized entry name
74            let outpath = dest_dir.join(&normalized_entry_name);
75
76            // Ensure the normalized path is within the destination directory before any I/O
77            let outpath_str = outpath.to_string_lossy().to_string();
78            let dest_str = dest_canonical.to_string_lossy().to_string();
79            if !outpath_str.starts_with(&dest_str) {
80                return Err(ServiceError::Validation(format!(
81                    "Path traversal attempt detected in ZIP entry: '{}' would resolve outside extraction directory",
82                    entry_name
83                )));
84            }
85
86            // For directories, we need to check if they exist or can be created
87            // For files, we need to validate after creation
88            let path_is_directory = normalized_entry_name.ends_with("/");
89
90            // Canonicalize the output path to resolve any .. components
91            // Note: canonicalize() requires the path to exist, so we only canonicalize after creation for directories
92            let outpath_canonical = if outpath.exists() {
93                outpath.canonicalize().map_err(|e| {
94                    ServiceError::Io(io::Error::new(
95                        e.kind(),
96                        format!("Failed to resolve path for ZIP entry: {}", entry_name),
97                    ))
98                })?
99            } else if path_is_directory {
100                // Create directory first, then canonicalize to validate
101                std::fs::create_dir_all(&outpath).map_err(ServiceError::Io)?;
102                outpath.canonicalize().map_err(|e| {
103                    ServiceError::Io(io::Error::new(
104                        e.kind(),
105                        format!("Failed to resolve path for ZIP entry: {}", entry_name),
106                    ))
107                })?
108            } else {
109                // For files, we need to ensure parent is valid before creating
110                if let Some(parent) = outpath.parent() {
111                    if !parent.exists() {
112                        // Create parent directory (safe now because we've already validated the normalized path)
113                        std::fs::create_dir_all(parent).map_err(ServiceError::Io)?;
114                    }
115                    // Validate parent path
116                    let parent_canonical = parent.canonicalize().map_err(|e| {
117                        ServiceError::Io(io::Error::new(
118                            e.kind(),
119                            format!(
120                                "Failed to resolve parent path for ZIP entry: {}",
121                                entry_name
122                            ),
123                        ))
124                    })?;
125                    if !parent_canonical.starts_with(&dest_canonical) {
126                        return Err(ServiceError::Validation(format!(
127                            "Path traversal attempt detected in parent directory for ZIP entry: '{}'",
128                            entry_name
129                        )));
130                    }
131                }
132                // Create the file
133                let mut outfile = std::fs::File::create(&outpath).map_err(ServiceError::Io)?;
134                io::copy(&mut file, &mut outfile).map_err(ServiceError::Io)?;
135                // Now canonicalize the file path to validate
136                outpath.canonicalize().map_err(|e| {
137                    ServiceError::Io(io::Error::new(
138                        e.kind(),
139                        format!("Failed to resolve path for ZIP entry: {}", entry_name),
140                    ))
141                })?
142            };
143
144            // Ensure the resolved path is within the destination directory
145            if !outpath_canonical.starts_with(&dest_canonical) {
146                return Err(ServiceError::Validation(format!(
147                    "Path traversal attempt detected in ZIP entry: '{}' resolves outside extraction directory",
148                    entry_name
149                )));
150            }
151        }
152
153        Ok(())
154    }
155}
156
157#[cfg(test)]
158#[allow(clippy::unwrap_used)]
159mod tests {
160    use super::*;
161    use std::fs::File;
162    use std::io::Write;
163    use tempfile::TempDir;
164    use zip::write::FileOptions;
165    use zip::ZipWriter;
166
167    /// Helper to create a ZIP archive with specified entries
168    fn create_test_zip(entries: &[(&str, &[u8])]) -> (TempDir, std::path::PathBuf) {
169        let temp_dir = TempDir::new().unwrap();
170        let zip_path = temp_dir.path().join("test.zip");
171
172        let file = File::create(&zip_path).unwrap();
173        let mut zip = ZipWriter::new(file);
174        let options = FileOptions::default().compression_method(zip::CompressionMethod::Stored);
175
176        for (name, content) in entries {
177            if name.ends_with('/') {
178                zip.add_directory(*name, options).unwrap();
179            } else {
180                zip.start_file(*name, options).unwrap();
181                zip.write_all(content).unwrap();
182            }
183        }
184
185        zip.finish().unwrap();
186        (temp_dir, zip_path)
187    }
188
189    #[test]
190    fn test_safe_extract_normal_files() {
191        let (_temp_dir, zip_path) = create_test_zip(&[
192            ("SKILL.md", b"Test content"),
193            ("README.md", b"Readme content"),
194            ("src/main.rs", b"source code"),
195        ]);
196
197        let extract_dir = TempDir::new().unwrap();
198        let handler = ZipHandler::new().unwrap();
199
200        handler
201            .extract_to_dir(&zip_path, extract_dir.path())
202            .unwrap();
203
204        assert!(extract_dir.path().join("SKILL.md").exists());
205        assert!(extract_dir.path().join("README.md").exists());
206        assert!(extract_dir.path().join("src/main.rs").exists());
207    }
208
209    #[test]
210    fn test_safe_extract_rejects_path_traversal() {
211        let (_temp_dir, zip_path) = create_test_zip(&[
212            ("normal.txt", b"safe content"),
213            ("../../../evil.txt", b"malicious content"),
214        ]);
215
216        let extract_dir = TempDir::new().unwrap();
217        let handler = ZipHandler::new().unwrap();
218
219        let result = handler.extract_to_dir(&zip_path, extract_dir.path());
220
221        assert!(result.is_err());
222        match result {
223            Err(ServiceError::Validation(msg)) => {
224                assert!(
225                    msg.contains("path traversal") || msg.contains("Path traversal"),
226                    "Error should mention path traversal: {}",
227                    msg
228                );
229            }
230            _ => unreachable!("Expected ServiceError::Validation for path traversal"),
231        }
232
233        // Ensure no file was created outside the extraction directory
234        assert!(!extract_dir.path().join("../../../evil.txt").exists());
235        assert!(extract_dir.path().join("normal.txt").exists());
236    }
237
238    #[test]
239    fn test_safe_extract_rejects_windows_path_traversal() {
240        let (_temp_dir, zip_path) = create_test_zip(&[
241            ("normal.txt", b"safe content"),
242            ("..\\..\\..\\evil.txt", b"malicious content"),
243        ]);
244
245        let extract_dir = TempDir::new().unwrap();
246        let handler = ZipHandler::new().unwrap();
247
248        let result = handler.extract_to_dir(&zip_path, extract_dir.path());
249
250        // On Unix, backslashes are treated as regular characters, so the path is literal
251        // On Windows, this would be a path traversal and should be rejected
252        // We accept either behavior since it's platform-specific
253        if cfg!(windows) {
254            assert!(
255                result.is_err(),
256                "Windows-style path traversal should be rejected on Windows"
257            );
258        } else {
259            // On Unix, the backslashes are literal characters, so the extraction should succeed
260            // but file will have a strange name with backslashes
261            assert!(matches!(result, Ok(_) | Err(_)));
262        }
263    }
264
265    #[test]
266    fn test_safe_extract_rejects_absolute_paths() {
267        let (_temp_dir, zip_path) = create_test_zip(&[
268            ("/etc/passwd", b"malicious content"),
269            ("normal.txt", b"safe content"),
270        ]);
271
272        let extract_dir = TempDir::new().unwrap();
273        let handler = ZipHandler::new().unwrap();
274
275        let result = handler.extract_to_dir(&zip_path, extract_dir.path());
276
277        assert!(result.is_err());
278        // Absolute paths will be rejected when canonicalized
279    }
280
281    #[test]
282    fn test_safe_extract_nested_directories() {
283        let (_temp_dir, zip_path) = create_test_zip(&[
284            ("deep/nested/file.txt", b"content"),
285            ("another/nested/path/README.md", b"readme"),
286        ]);
287
288        let extract_dir = TempDir::new().unwrap();
289        let handler = ZipHandler::new().unwrap();
290
291        handler
292            .extract_to_dir(&zip_path, extract_dir.path())
293            .unwrap();
294
295        assert!(extract_dir.path().join("deep/nested/file.txt").exists());
296        assert!(extract_dir
297            .path()
298            .join("another/nested/path/README.md")
299            .exists());
300    }
301
302    #[test]
303    fn test_safe_extract_rejects_mixed_traversal() {
304        let (_temp_dir, zip_path) = create_test_zip(&[
305            ("safe/file.txt", b"safe"),
306            ("safe/../../evil.txt", b"malicious"),
307        ]);
308
309        let extract_dir = TempDir::new().unwrap();
310        let handler = ZipHandler::new().unwrap();
311
312        let result = handler.extract_to_dir(&zip_path, extract_dir.path());
313
314        assert!(result.is_err());
315        // Mixed traversal should be rejected
316        assert!(extract_dir.path().join("safe/file.txt").exists());
317    }
318
319    #[test]
320    fn test_safe_extract_does_not_create_dirs_outside_root() {
321        let base = TempDir::new().unwrap();
322        let extract_dir = base.path().join("extract");
323        std::fs::create_dir_all(&extract_dir).unwrap();
324
325        // Create a zip with a traversal entry that goes outside base
326        let (_temp_dir, zip_path) =
327            create_test_zip(&[("../../../escape_evil/file.txt", b"malicious content")]);
328
329        let handler = ZipHandler::new().unwrap();
330        let result = handler.extract_to_dir(&zip_path, &extract_dir);
331
332        // Should return an error before any I/O happens (due to normalization)
333        assert!(result.is_err());
334
335        // No file or directory should have been created
336        assert!(!extract_dir.join("escape_evil").exists());
337        assert!(!extract_dir.join("escape_evil/file.txt").exists());
338    }
339}
fastskill_core/storage/zip.rs

fastskill_core/storage/
zip.rs
