fix(core): reject ZIP entries not listed in MANIFEST (closes #1)#3
Conversation
verifyManifestHashes only walked manifest.files, so any extra file appended to a signed ZIP was silently accepted as part of the authenticated package. After signing, an attacker could re-zip the archive with additional files (e.g. postinstall.sh) and verifyPackage / verifyFromAuthor still returned valid: true. Add a bidirectional check that fails closed when a ZIP entry is not listed in the manifest (MANIFEST.json itself excepted). Apply in both verifyPackage and verifyZipFromAuthor. Introduce VerifyErrorCode.UNEXPECTED_FILE and a regression test reproducing the PoC from issue #1. Fixes #1 Made-with: Cursor -e Signed-off-by: Emre Tinaztepe <emre@binalyze.com>
There was a problem hiding this comment.
Pull request overview
Fixes a ZIP verification bypass where unsigned extra ZIP entries could be appended after signing and still pass verification, by failing closed on any ZIP entry not listed in MANIFEST.json.
Changes:
- Added
VerifyErrorCode.UNEXPECTED_FILEand surfaced it through verification results. - Implemented
findUnexpectedFiles()and wired it into bothverifyPackage()andverifyZipFromAuthor(). - Added a regression test reproducing the PoC (extra unsigned file appended to a signed ZIP).
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| packages/core/src/types.ts | Adds UNEXPECTED_FILE error code for unexpected ZIP entries. |
| packages/core/src/verify.ts | Rejects ZIP entries not present in manifest.files and centralizes file-failure reason formatting. |
| packages/core/test/verify.spec.ts | Adds a regression test ensuring appended unsigned files are rejected. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| } | ||
|
|
||
| const fileResults = await verifyManifestHashes(manifest, files); | ||
| const extras = findUnexpectedFiles(manifest, files); | ||
| const fileResults = [...(await verifyManifestHashes(manifest, files)), ...extras]; | ||
| const hasFailedFile = fileResults.some((f) => !f.valid); |
There was a problem hiding this comment.
verifyZipFromAuthor() now rejects ZIP entries not present in manifest.files, but there’s no regression test exercising this code path (only verifyPackage() is covered). Add a test that calls verifyFromAuthor(tamperedZip, { fetch: mockFetchWithKeys([...]), resolveTxt: false }) and asserts it returns valid: false with VerifyErrorCode.UNEXPECTED_FILE and includes the unexpected entry in details.files.
This ensures the security fix can’t regress in the author-verification entry point.
There was a problem hiding this comment.
Good catch — added in 1ea22de. New test rejects ZIP with extra unsigned file via verifyFromAuthor (UNEXPECTED_FILE) exercises verifyFromAuthor with the same tampered-ZIP scenario and asserts code: UNEXPECTED_FILE and that evil.sh shows up in details.files as a failed entry.
Address Copilot review feedback on #3: the fix is applied in both verifyPackage and verifyZipFromAuthor, but only verifyPackage was covered by a regression test. Add a test exercising verifyFromAuthor with an injected unsigned file to ensure the author-verification entry point cannot regress. Made-with: Cursor -e Signed-off-by: Emre Tinaztepe <emre@binalyze.com>
…neous-files Made-with: Cursor # Conflicts: # packages/core/test/verify.spec.ts -e Signed-off-by: Emre Tinaztepe <emre@binalyze.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Summary
Closes #1 — ZIP verification bypass via unsigned extra files (CWE-349).
verifyManifestHashesonly walkedmanifest.files, so any extra file appended to a signed ZIP was silently accepted as part of the authenticated package. The reporter's PoC (re-zip a signed package with an injectedevil.sh) still returnedvalid: true.Changes
VerifyErrorCode.UNEXPECTED_FILE.findUnexpectedFileshelper that fails closed for any ZIP entry not inmanifest.files(excludingMANIFEST.jsonitself).verifyPackageandverifyZipFromAuthorso the fix covers both verification entry points.failedFileReason.rejects ZIP with extra unsigned file not in manifest(reproduces the PoC from issue ZIP verification bypass — unsigned extra files accepted as valid package content #1).Test plan
pnpm --filter @binalyze/notar test— 122 passedpnpm --filter @binalyze/notar typecheck— cleanpnpm lint— 0 warnings / 0 errorsNotes
Patch release will follow once #2 is also merged.
Made with Cursor