Fix slipery zip extraction

[ildyria]

Summary by CodeRabbit

• Bug Fixes

  • Improved uploaded filename validation to reject unsafe names, path traversal, directory paths, and invalid values.
  • Strengthened ZIP extraction safeguards to keep archive contents within the permitted extraction location.
  • Improved handling of invalid extraction destinations, preventing unsafe or misconfigured archive extraction.
  • Added protection against archive entries resolving outside the designated extraction directory.

• Tests

  • Added coverage for valid and invalid filenames, including traversal attempts, directory paths, empty values, and non-string input.

[coderabbitai]

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

• @coderabbitai resume to resume automatic reviews.
• @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

• ▶️ Resume reviews
• 🔍 Trigger review

📝 Walkthrough

Walkthrough

The changes add filename-specific request validation. ZIP extraction now tracks a canonical extraction root and constrains destinations and entry paths to that root.

Changes

Security validation

Layer / File(s)	Summary
Filename validation contract and request wiring app/Rules/FilenameRule.php, app/Http/Requests/Photo/UploadPhotoRequest.php, tests/Unit/Rules/FilenameRuleTest.php	FilenameRule rejects non-string values, directory components, separators, traversal patterns, and the standalone . name. The request applies the rule, and unit tests cover accepted and rejected inputs.
Safe ZIP destination validation app/Services/Zip/SafeZipExtractor.php	SafeZipExtractor validates extraction destinations against the configured base directory and returns normalized relative entry paths.
Canonical extraction-root tracking app/Jobs/ExtractZip.php, tests/Unit/Jobs/ExtractZipTest.php	ExtractZip resolves and stores the canonical extraction root, requires it when creating SafeZipExtractor, and passes it during inspection and extraction. Tests initialize the root before direct validation calls.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Poem

A rabbit checks each filename bright,
And keeps each ZIP path in its right.
Roots are fixed, unsafe trails fade,
Tests hop through the paths we made.
“Safe extraction!” the rabbit brays.

🚥 Pre-merge checks | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 47.06% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[coderabbitai]

Actionable comments posted: 2

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 7b89acce-5f74-480a-a81a-0313bb5b04ff

📥 Commits

Reviewing files that changed from the base of the PR and between e6fc163 and 2d8a995.

📒 Files selected for processing (4)

• app/Http/Requests/Photo/UploadPhotoRequest.php
• app/Jobs/ExtractZip.php
• app/Rules/FilenameRule.php
• tests/Unit/Rules/FilenameRuleTest.php

[coderabbitai]

Actionable comments posted: 1

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 635e74c3-329a-4969-82fb-98944bd918eb

📥 Commits

Reviewing files that changed from the base of the PR and between 2d8a995 and 3d937e6.

📒 Files selected for processing (4)

• app/Jobs/ExtractZip.php
• app/Rules/FilenameRule.php
• app/Services/Zip/SafeZipExtractor.php
• tests/Unit/Rules/FilenameRuleTest.php

🚧 Files skipped from review as they are similar to previous changes (2)

• app/Rules/FilenameRule.php
• tests/Unit/Rules/FilenameRuleTest.php

[coderabbitai]

🧹 Nitpick comments (1)

tests/Unit/Jobs/ExtractZipTest.php (1)

71-78: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick win

Cover extraction-time destination enforcement.

callValidateZip() only invokes validate_zip(). The destination constraint is exercised when extraction writes through SafeZipExtractor. Add a focused test for an allowed destination and a destination outside the canonical extraction root, or add a dedicated SafeZipExtractor unit test.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: f8eb2650-2737-4887-8518-aff779b94577

📥 Commits

Reviewing files that changed from the base of the PR and between 0fbfe5c and ad5abad.

📒 Files selected for processing (1)

• tests/Unit/Jobs/ExtractZipTest.php

[coderabbitai]

Actionable comments posted: 2

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 3d3b85c9-a429-4960-aea0-bb22d99cacdd

📥 Commits

Reviewing files that changed from the base of the PR and between ad5abad and d2667eb.

📒 Files selected for processing (1)

• app/Rules/FilenameRule.php

[codecov]

Codecov Report

❌ Patch coverage is 92.10526% with 3 lines in your changes missing coverage. Please review.
✅ Project coverage is 91.02%. Comparing base (e6fc163) to head (f916f6b).
⚠️ Report is 1 commits behind head on master.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[Alpastx]

the updated PR looks good. The pre-create realpath issue is addressed, and FilenameRule + base-dir containment in SafeZipExtractor close the reported path.