🛡️ Sentinel: [CRITICAL] Fix Zip Slip vulnerability in backup restore #86

google-labs-jules · 2026-01-06T23:06:00Z

This PR fixes a potential "Zip Slip" vulnerability in the backup restoration process.
Previously, ImportExportService.RestoreFromBackupAsync used ZipFile.ExtractToDirectory which, depending on the runtime, might allow malicious zip files with ../ entries to write files outside the intended restore directory.

Changes:

Modified RestoreFromBackupAsync to iterate through zip entries manually.
Added a check !destinationPath.StartsWith(tempDirFullPath) to ensure the resolved path is safe.
Included handling for the partial path traversal edge case by appending Path.DirectorySeparatorChar.
Added a regression test ZipSlipTests.cs.

Verification:
A new test BookLoggerApp.Tests/Security/ZipSlipTests.cs was added. It mocks the necessary dependencies, creates a malicious zip file with a ../../evil.txt entry, and asserts that RestoreFromBackupAsync throws an IOException (or security exception) instead of allowing the write.

PR created automatically by Jules for task 14377759404812732540 started by @Tr1sma

- Replaced `ZipFile.ExtractToDirectory` with manual extraction loop in `ImportExportService.RestoreFromBackupAsync`. - Added explicit path validation to ensure all extracted files reside within the target directory. - Added `BookLoggerApp.Tests/Security/ZipSlipTests.cs` to verify the fix and document the vulnerability vector.

google-labs-jules · 2026-01-06T23:06:01Z

👋 Jules, reporting for duty! I'm here to lend a hand with this pull request.

When you start a review, I'll add a 👀 emoji to each comment to let you know I've read it. I'll focus on feedback directed at me and will do my best to stay out of conversations between you and other bots or reviewers to keep the noise down.

I'll push a commit with your requested changes shortly after. Please note there might be a delay between these steps, but rest assured I'm on the job!

For more direct control, you can switch me to Reactive Mode. When this mode is on, I will only act on comments where you specifically mention me with @jules. You can find this option in the Pull Request section of your global Jules UI settings. You can always switch back!

New to Jules? Learn more at jules.google/docs.

For security, I will only act on instructions from the user who triggered this task.

Copilot

Pull request overview

This PR addresses a critical Zip Slip vulnerability in the backup restoration functionality by replacing the unsafe ZipFile.ExtractToDirectory method with manual path validation during extraction.

Key changes:

Replaced automatic extraction with manual iteration through zip entries with path validation
Added path traversal detection by verifying extracted paths stay within the intended directory
Implemented regression test to verify malicious zip files are rejected

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 5 comments.

File	Description
BookLoggerApp.Infrastructure/Services/ImportExportService.cs	Replaced `ZipFile.ExtractToDirectory` with manual extraction loop that validates each entry's destination path against the target directory to prevent Zip Slip attacks
BookLoggerApp.Tests/Security/ZipSlipTests.cs	Added security regression test with mock implementations to verify that zip files with path traversal entries (../../) are rejected with IOException
.jules/sentinel.md	Added documentation of the vulnerability, learning points, and prevention guidelines for future reference

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-01-09T13:25:49Z

BookLoggerApp.Infrastructure/Services/ImportExportService.cs

+                        var destinationPath = Path.GetFullPath(Path.Combine(tempExtractDir, entry.FullName));
+                        var tempDirFullPath = Path.GetFullPath(tempExtractDir);
+
+                        if (!tempDirFullPath.EndsWith(Path.DirectorySeparatorChar.ToString(), StringComparison.Ordinal))


Using EndsWith with ToString() on Path.DirectorySeparatorChar is unnecessarily verbose. The EndsWith method has an overload that accepts a char directly: tempDirFullPath.EndsWith(Path.DirectorySeparatorChar).

Suggested change

if (!tempDirFullPath.EndsWith(Path.DirectorySeparatorChar.ToString(), StringComparison.Ordinal))

if (!tempDirFullPath.EndsWith(Path.DirectorySeparatorChar))

Copilot · 2026-01-09T13:25:49Z

BookLoggerApp.Tests/Security/ZipSlipTests.cs

+    [Fact]
+    public async Task RestoreFromBackupAsync_ShouldThrowIOException_OnZipSlip()
+    {
+        // Arrange
+        var tempDir = Path.Combine(Path.GetTempPath(), "ZipSlipTests_" + Guid.NewGuid().ToString());
+        Directory.CreateDirectory(tempDir);
+        var zipPath = Path.Combine(tempDir, "malicious.zip");
+
+        try
+        {
+            // 1. Create a malicious zip file
+            // We can't use the standard ZipFile.CreateFromDirectory to easily create ".." entries
+            // because it sanitizes them. We must manipulate the archive directly.
+            using (var fileStream = new FileStream(zipPath, FileMode.Create))
+            using (var archive = new ZipArchive(fileStream, ZipArchiveMode.Create))
+            {
+                // Create an entry with ".." in the name
+                // Note: Windows and some libraries might sanitize this automatically,
+                // but this is the standard way to attempt a creation of such an entry for testing.
+                var entry = archive.CreateEntry("../../evil.txt");
+                using (var entryStream = entry.Open())
+                using (var writer = new StreamWriter(entryStream))
+                {
+                    writer.Write("This is a malicious file.");
+                }
+            }
+
+            // 2. Setup Service
+            var dbName = Guid.NewGuid().ToString();
+            var contextFactory = new TestDbContextFactory(dbName);
+            var fileSystem = new MockFileSystem();
+            var settingsProvider = new MockAppSettingsProvider();
+
+            // Pass the tempDir as appDataPath so backups/restores happen there
+            var service = new ImportExportService(
+                contextFactory,
+                fileSystem,
+                settingsProvider,
+                null,
+                tempDir);
+
+            // Act & Assert
+            // The service should detect the ".." in the entry name and throw an IOException
+            await Assert.ThrowsAsync<IOException>(async () =>
+            {
+                await service.RestoreFromBackupAsync(zipPath);
+            });
+        }
+        finally
+        {
+            if (Directory.Exists(tempDir)) Directory.Delete(tempDir, true);
+        }
+    }


The test only verifies one malicious pattern (../../evil.txt). Consider adding test cases for other path traversal patterns to ensure comprehensive coverage: absolute paths (/etc/passwd), Windows paths (C:\evil.txt), mixed separators (../../evil.txt), URL-encoded patterns (%2e%2e%2fevil.txt), and a positive test case with a legitimate zip file that should extract successfully.

Copilot · 2026-01-09T13:25:50Z

BookLoggerApp.Infrastructure/Services/ImportExportService.cs

+                            tempDirFullPath += Path.DirectorySeparatorChar;
+                        }
+
+                        if (!destinationPath.StartsWith(tempDirFullPath, StringComparison.OrdinalIgnoreCase))


The path validation uses OrdinalIgnoreCase comparison, which could allow case-based bypass attacks on case-sensitive file systems (Linux, macOS with APFS). For security-critical path validation, use Ordinal (case-sensitive) comparison to ensure consistent behavior across all platforms.

Suggested change

if (!destinationPath.StartsWith(tempDirFullPath, StringComparison.OrdinalIgnoreCase))

if (!destinationPath.StartsWith(tempDirFullPath, StringComparison.Ordinal))

Copilot · 2026-01-09T13:25:50Z

BookLoggerApp.Tests/Security/ZipSlipTests.cs

+public class MockFileSystem : IFileSystem
+{
+    // Minimal implementation for the test
+    public bool FileExists(string path) => true;
+    public bool DirectoryExists(string path) => true;
+    public void CreateDirectory(string path) { }
+    public void CopyFile(string source, string dest, bool overwrite) { }
+    public string CombinePath(params string[] paths) => Path.Combine(paths);
+}


The MockFileSystem is missing several methods required by the IFileSystem interface: ReadAllTextAsync, WriteAllTextAsync, ReadAllBytesAsync, WriteAllBytesAsync, DeleteFile, and OpenWrite. While these may not be called during this specific test, incomplete interface implementations can cause runtime errors if the code path changes. Implement all interface methods or use a mocking framework like Moq.

Copilot · 2026-01-09T13:25:50Z

BookLoggerApp.Tests/Security/ZipSlipTests.cs

+    public async Task RestoreFromBackupAsync_ShouldThrowIOException_OnZipSlip()
+    {
+        // Arrange
+        var tempDir = Path.Combine(Path.GetTempPath(), "ZipSlipTests_" + Guid.NewGuid().ToString());


Redundant call to 'ToString' on a String object.

Suggested change

var tempDir = Path.Combine(Path.GetTempPath(), "ZipSlipTests_" + Guid.NewGuid().ToString());

var tempDir = Path.Combine(Path.GetTempPath(), "ZipSlipTests_" + Guid.NewGuid());

Tr1sma marked this pull request as ready for review January 9, 2026 13:19

Copilot AI review requested due to automatic review settings January 9, 2026 13:19

Tr1sma merged commit b27e340 into main Jan 9, 2026
1 check failed

Tr1sma deleted the sentinel/fix-zip-slip-14377759404812732540 branch January 9, 2026 13:19

Copilot started reviewing on behalf of Tr1sma January 9, 2026 13:19 View session

Copilot AI reviewed Jan 9, 2026

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

🛡️ Sentinel: [CRITICAL] Fix Zip Slip vulnerability in backup restore #86

🛡️ Sentinel: [CRITICAL] Fix Zip Slip vulnerability in backup restore #86

Uh oh!

google-labs-jules bot commented Jan 6, 2026

Uh oh!

google-labs-jules bot commented Jan 6, 2026

Uh oh!

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Jan 9, 2026

Uh oh!

Copilot AI Jan 9, 2026

Uh oh!

Copilot AI Jan 9, 2026

Uh oh!

Copilot AI Jan 9, 2026

Uh oh!

Copilot AI Jan 9, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

	if (!tempDirFullPath.EndsWith(Path.DirectorySeparatorChar.ToString(), StringComparison.Ordinal))
	if (!tempDirFullPath.EndsWith(Path.DirectorySeparatorChar))

	if (!destinationPath.StartsWith(tempDirFullPath, StringComparison.OrdinalIgnoreCase))
	if (!destinationPath.StartsWith(tempDirFullPath, StringComparison.Ordinal))

	var tempDir = Path.Combine(Path.GetTempPath(), "ZipSlipTests_" + Guid.NewGuid().ToString());
	var tempDir = Path.Combine(Path.GetTempPath(), "ZipSlipTests_" + Guid.NewGuid());

🛡️ Sentinel: [CRITICAL] Fix Zip Slip vulnerability in backup restore #86

🛡️ Sentinel: [CRITICAL] Fix Zip Slip vulnerability in backup restore #86

Uh oh!

Conversation

google-labs-jules bot commented Jan 6, 2026

Uh oh!

google-labs-jules bot commented Jan 6, 2026

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Jan 9, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 9, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 9, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 9, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 9, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants