Skip to content

Add details to zip safety errors#636

Merged
ErisDS merged 1 commit into
mainfrom
fix/zip-existing-error-details
May 14, 2026
Merged

Add details to zip safety errors#636
ErisDS merged 1 commit into
mainfrom
fix/zip-existing-error-details

Conversation

@ErisDS
Copy link
Copy Markdown
Member

@ErisDS ErisDS commented May 14, 2026

Summary

  • Add machine-readable codes and human-readable context to existing zip symlink and filename safety errors.
  • Include entry name and byte details in errorDetails where available.
  • Update tests to assert the richer Ghost error shape.

Testing

  • pnpm --filter @tryghost/zip test
  • pnpm exec oxfmt -c .oxfmtrc.json --check \"packages/zip/**/*.{js,json,md}\"

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented May 14, 2026

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 5693c563-710f-4928-9d5e-4536e11677be

📥 Commits

Reviewing files that changed from the base of the PR and between 6a18dff and 219a3a9.

📒 Files selected for processing (2)
  • packages/zip/lib/extract.js
  • packages/zip/test/zip.test.js

Walkthrough

This PR enhances error reporting for ZIP extraction validation failures. Two validation functions (throwOnSymlinks and throwOnLargeFilenames) now throw UnsupportedMediaTypeError with structured error details: explicit context/code fields and errorDetails containing relevant metadata (entry name, observed/limit byte counts). Corresponding test assertions are updated from simple message regex matching to comprehensive validation of the complete error object properties, ensuring the enriched error structure is properly reported.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly summarizes the main change: adding enhanced error details (codes, context, entry names, byte information) to existing zip safety error handling for symlinks and filenames.
Description check ✅ Passed The description is directly related to the changeset, detailing the addition of machine-readable codes, human-readable context, and entry/byte details to zip safety errors, matching the implemented changes.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/zip-existing-error-details

Warning

There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure.

🔧 ESLint

If the error stems from missing dependencies, add them to the package.json file. For unrecoverable errors (e.g., due to private dependencies), disable the tool in the CodeRabbit configuration.

ESLint skipped: no ESLint configuration detected in root package.json. To enable, add eslint to devDependencies.


Comment @coderabbitai help to get the list of available commands and usage tips.

@codecov-commenter
Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 100.00%. Comparing base (6a18dff) to head (219a3a9).

Additional details and impacted files
@@             Coverage Diff             @@
##             main      #636      +/-   ##
===========================================
+ Coverage   98.12%   100.00%   +1.87%     
===========================================
  Files          84         2      -82     
  Lines        2770        75    -2695     
  Branches      510        12     -498     
===========================================
- Hits         2718        75    -2643     
+ Misses         11         0      -11     
+ Partials       41         0      -41     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 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.

@ErisDS ErisDS merged commit 4c827db into main May 14, 2026
4 checks passed
@ErisDS ErisDS deleted the fix/zip-existing-error-details branch May 14, 2026 11:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants