[PR #5803] [MERGED] fix(artifacts): admit ZIP archives after unclaimed leading bytes #7073

Closed
opened 2026-11-01 21:45:10 +00:00 by bsakaguchi · 0 comments
Owner

📋 Pull Request Information

Original PR: https://github.com/morluto/rea/pull/5803
Author: @kaoru0822-kitauji
Created: 10/21/2026
Status: ✅ Merged
Merged: 10/21/2026
Merged by: @kaoru0822-kitauji

Base: main ← Head: fix/zip-executable-prefix


📝 Commits (3)

  • 41948cc fix(artifacts): admit ZIP archives after unclaimed leading bytes
  • 93e5576 fix(artifacts): share the root ZIP family across recognition paths
  • 63ba251 Merge remote-tracking branch 'origin/main' into fix/zip-executable-prefix

📊 Changes

7 files changed (+405 additions, -18 deletions)

View changed files

📝 src/application/BinaryTargetResolver.ts (+51 -8)
📝 src/artifacts/inventory/classify.ts (+17 -8)
📝 src/domain/binaryTarget.ts (+16 -0)
📝 src/domain/zipPackageFormat.ts (+86 -1)
➕ tests/acceptance/artifactZipLeadingBytes.test.ts (+74 -0)
📝 tests/boundary/filesystem/artifactComparison.test.ts (+1 -1)
📝 tests/boundary/filesystem/zipDetection.test.ts (+160 -0)

📄 Description

What broke: inspect-artifact and extract-artifact refused a valid ZIP behind a script or self-extractor stub as "unsupported binary format", although Info-ZIP, bsdtar and Python read it (#5732).
Fix: target resolution and root inventory now locate a trailing single-disk ZIP from its end record, central directory and first local header. This applies only when no other format claims the leading bytes; executable-led files keep their executable route.
Risk: low to medium. It is additive for previously refused files. Classification stays unchanged for files any existing format recognizes, and Zip64, split and empty archives still fail closed.

Summary

Fixes #5732. A ZIP preceded by inert leading bytes, such as a #!/bin/sh stub or a self-extractor, was refused before inventory or extraction because ZIP recognition only checked the first four bytes. Both prefix styles in common use now inventory and extract with the same verified member bytes as the plain archive:

  • member offsets that include the leading bytes (zip -A, or Python's zipfile appending to the stub);
  • raw concatenation, where offsets were written before the stub was prepended.

Problem and expected behavior

Trigger: a 17-byte shell stub followed by a ZIP. Before this change, on main d672cc239 (macOS arm64), inspect-artifact returned target_unavailable / "Cannot open artifact: unsupported binary format." for both prefix styles, while the plain ZIP passed.

Expected: a file whose leading bytes are claimed by no other format, and which ends in a structurally consistent ZIP, is a ZIP archive. A file whose leading bytes begin an executable header keeps the executable classification. For example, /bin/ls with an appended ZIP stays mach-o-universal.

Change and scope

  • src/domain/zipPackageFormat.ts now owns ZIP recognition at both ends of a file: hasZipSignature for the start and the new locateTrailingZipArchive for a trailing archive. The locator takes an injected positional read and is bounded to the last 22 + 65,535 bytes, one 46-byte central-directory header and one 4-byte local-header signature. It requires:

    • an end record whose comment ends exactly at end of file;
    • a single disk with at least one entry and no Zip64 markers;
    • a central-directory header at the derived position;
    • a local header at the first entry's offset, using an offset shift that resolves both prefix styles.
  • src/domain/binaryTarget.ts adds claimsExecutableHeader, which recognizes the ELF, MZ, Mach-O and FAT families that parseExecutableHeader interprets, whether or not the header is valid. Both callers use it, so they make the same decision.

  • BinaryTargetResolver consults the locator only after native header detection fails and only when no executable family claims the leading bytes. Root inventory classification (classify.ts) consults it only when the bytes classify as file or unknown and no executable family claims them.

  • ZIP_NON_ENTRY_LIMITATION now names leading bytes before the first entry, such as a self-extractor stub, as non-entry bytes that are not occurrences.

  • Not included:

    • Zip64, split, and empty trailing archives, which stay refused;
    • trailing data after the end record;
    • nested members, whose classification still uses their own leading bytes;
    • executable-led archives such as PE or ELF self-extractors, which keep their executable route.

    Admitting any of these would need a separate decision.

  • Scope decision: not applicable. This is format recognition within the existing ZIP reader boundary. The reader (@zip.js/zip.js 2.15.0) already reads both prefix styles, as verified below.

Contract and boundary impact

  • Semantic owner and earliest changed stage: ZIP recognition in src/domain/zipPackageFormat.ts, consumed by target resolution and root inventory classification.
  • Contribution scope fit and linked maintainer decision: not applicable (ordinary fix).
  • CLI and MCP/tool-catalog contract: no schema change. Previously refused files with unclaimed leading bytes and a trailing ZIP now return zip artifact inventories and extractions.
  • Provider, bridge, target-format, or platform compatibility: classification is unchanged for every file another format recognizes. Executable-led files with appended archives keep their executable route.
  • Evidence, artifact, provenance, or reconstruction contract: root identity and digest cover the whole file, leading bytes included. Members keep their content identities. The ZIP limitation now names leading bytes as non-entry bytes.
  • Target selection, authority, upstream behavior, and cleanup details: no new process, write or network behavior. Extraction still uses the hardened ZIP reader and output tree.
  • Generated metadata, package, or installation impact: none.

Expected effects:

  • Read existing files or captured data.
  • Launch or attach to a process.
  • Inject instrumentation or run scripts/code.
  • Change process memory, registers, files, or configuration.
  • Contact network services.
  • Recover protected code or work around protections.
  • Documentation or usability only; no target interaction.
  • Not sure yet; investigation or maintainer guidance needed.

Evidence and regression coverage

  • Tests added or updated:
    • tests/acceptance/artifactZipLeadingBytes.test.ts: the public CLI inspect-artifact and extract-artifact on a script-stubbed ZIP. It checks zip root format, verified members, the non-entry limitation, and the extracted bytes. This fails on the base revision with target_unavailable.
    • tests/boundary/filesystem/zipDetection.test.ts:
      • Both prefix styles classify as zip through root classification and parseBinaryTarget, with member identities equal to the plain archive's.
      • ELF-, MZ- and Mach-O-led files with an appended ZIP stay off the ZIP route.
      • A trailing ZIP is not located with bytes after the end record, a corrupt central-directory header, a Zip64 directory offset, or an empty archive record.
    • tests/boundary/filesystem/artifactComparison.test.ts: updated for the widened limitation text.
  • Base reproduction (executed, macOS arm64, main d672cc239): plain.zip passed; prefixed.zip (offsets include the stub) and concat.zip (raw concatenation) failed with "unsupported binary format".
  • After the change (executed):
    • All three inventory a.txt, d, and d/b.txt as verified, and extract members with identical digests.
    • /bin/ls + ZIP stays mach-o-universal.
    • A corrupted central directory, trailing junk, and a text file ending in an end-record-shaped tail are still refused.
  • Reader check (executed): @zip.js/zip.js 2.15.0 reads both prefix styles. For raw concatenation it rebases entry offsets itself (a.txt at offset 17).
  • User-visible CLI output (inspect-artifact on the concatenated file): root format zip; occurrences ., a.txt, d, d/b.txt, all verified.
  • Performance (executed, warm, 200 iterations): locateTrailingZipArchive takes 0.112 ms per call on a 50 MB unrecognized random file and 0.023 ms on the prefixed ZIP. It runs only for files that no other format claims; the cold-cache cost of one ≤64 KiB tail read was not measured separately.
  • Remaining proof gaps:
    • Zip64 trailing archives are not located.
    • Real PE self-extractors were not exercised; the MZ-led synthetic case covers the routing rule.
    • Linux and Windows hosts were not run locally; CI covers them.

For evidence-bearing changes:

  • Observed, derived, and inferred claims remain distinguishable.
  • Artifact identity, source provenance, and failed attempts remain preserved.
  • Unsupported, incomplete, unavailable, or uncertain outcomes remain visible.

Validation performed

  • npm run test:focused -- tests/acceptance/artifactZipLeadingBytes.test.ts tests/boundary/filesystem/zipDetection.test.ts tests/boundary/filesystem/artifactComparison.test.ts tests/acceptance/artifactZipCurrentDirectory.test.ts tests/acceptance/artifactZipExtractionPaths.test.ts tests/acceptance/pyinstallerArchive.test.ts tests/acceptance/godotPck.test.ts src/artifacts/ZipArtifactReader.test.ts src/artifacts/apple/machoContainer.test.ts: 76 passed.

  • npm run test:focused -- every test file that references parseBinaryTarget, classifyAndHashRoot, BinaryTargetResolver or parseExecutableHeader (46 files): 822 passed, 1 skipped.

  • npm run check:fast: passed (typecheck, lint).

  • npm run format:check: passed.

  • CLI reproduction and non-regression checks above: passed.

  • Review follow-up (93e55760e): the root ZIP family mapping (.app → app-pack) now lives once in zipRootPackageFormat and is shared by both resolver routes and root inventory; a stub-prefixed root Example.app is app-pack from both. zipDetection + CLI acceptance: 25 passed; check:fast, format:check passed. The first CI run failed only peResources (30 s timeout, also seen on main on 2026-10-20); it passed locally in 9 s.

Compatibility, safety, and release

  • Breaking changes or migration steps: none. Only previously refused inputs change outcome.
  • Real Hopper/Ghidra, browser, or OS coverage: macOS arm64 only, locally. No provider is involved.
  • Package or release metadata impact: none.
  • Security, privacy, process, or containment review: the leading bytes are never executed or extracted. Extraction keeps the existing path-containment and output-tree checks, and the locator's reads are bounded.

Author readiness

Human owner: @kaoru0822-kitauji

  • I reviewed the final diff and relevant existing code, understand the change and its edge cases, and take responsibility for the complete PR, including AI-assisted work.
  • I checked the evidence above, confirmed it supports the current change, and recorded meaningful validation gaps.
  • I declare this PR ready for initial maintainer review.

Before merge

  • This PR addresses a concrete problem or an agreed enhancement.
  • Material scope and authority questions are resolved, and any required maintainer scope decision covers the final diff.
  • The PR has one focused outcome and the title follows type(scope): outcome.
  • Related issue is linked, or the reason for not linking one is stated above.
  • Tests cover changed observable behavior and meaningful failure paths.
  • Owning docs, contracts, and generated metadata are updated where needed.
  • User-visible CLI/MCP changes include representative output.
  • I checked the final diff for secrets, unrelated cleanup, and unsupported claims.

🤖 Generated with Claude Code


🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.

## 📋 Pull Request Information **Original PR:** https://github.com/morluto/rea/pull/5803 **Author:** [@kaoru0822-kitauji](https://github.com/kaoru0822-kitauji) **Created:** 10/21/2026 **Status:** ✅ Merged **Merged:** 10/21/2026 **Merged by:** [@kaoru0822-kitauji](https://github.com/kaoru0822-kitauji) **Base:** `main` ← **Head:** `fix/zip-executable-prefix` --- ### 📝 Commits (3) - [`41948cc`](https://github.com/morluto/rea/commit/41948cc6a5f2612aa488e1455a1f2ee4b9a1632f) fix(artifacts): admit ZIP archives after unclaimed leading bytes - [`93e5576`](https://github.com/morluto/rea/commit/93e55760ef7a6daa23e630afcfc22c36f4f391ed) fix(artifacts): share the root ZIP family across recognition paths - [`63ba251`](https://github.com/morluto/rea/commit/63ba2518ed672173877d60e6d0cb68350c0f8a8c) Merge remote-tracking branch 'origin/main' into fix/zip-executable-prefix ### 📊 Changes **7 files changed** (+405 additions, -18 deletions) <details> <summary>View changed files</summary> 📝 `src/application/BinaryTargetResolver.ts` (+51 -8) 📝 `src/artifacts/inventory/classify.ts` (+17 -8) 📝 `src/domain/binaryTarget.ts` (+16 -0) 📝 `src/domain/zipPackageFormat.ts` (+86 -1) ➕ `tests/acceptance/artifactZipLeadingBytes.test.ts` (+74 -0) 📝 `tests/boundary/filesystem/artifactComparison.test.ts` (+1 -1) 📝 `tests/boundary/filesystem/zipDetection.test.ts` (+160 -0) </details> ### 📄 Description **What broke:** `inspect-artifact` and `extract-artifact` refused a valid ZIP behind a script or self-extractor stub as "unsupported binary format", although Info-ZIP, bsdtar and Python read it (#5732). **Fix:** target resolution and root inventory now locate a trailing single-disk ZIP from its end record, central directory and first local header. This applies only when no other format claims the leading bytes; executable-led files keep their executable route. **Risk:** low to medium. It is additive for previously refused files. Classification stays unchanged for files any existing format recognizes, and Zip64, split and empty archives still fail closed. ## Summary Fixes #5732. A ZIP preceded by inert leading bytes, such as a `#!/bin/sh` stub or a self-extractor, was refused before inventory or extraction because ZIP recognition only checked the first four bytes. Both prefix styles in common use now inventory and extract with the same verified member bytes as the plain archive: - member offsets that include the leading bytes (`zip -A`, or Python's `zipfile` appending to the stub); - raw concatenation, where offsets were written before the stub was prepended. ## Problem and expected behavior Trigger: a 17-byte shell stub followed by a ZIP. Before this change, on main `d672cc239` (macOS arm64), `inspect-artifact` returned `target_unavailable` / "Cannot open artifact: unsupported binary format." for both prefix styles, while the plain ZIP passed. Expected: a file whose leading bytes are claimed by no other format, and which ends in a structurally consistent ZIP, is a ZIP archive. A file whose leading bytes begin an executable header keeps the executable classification. For example, `/bin/ls` with an appended ZIP stays `mach-o-universal`. ## Change and scope - `src/domain/zipPackageFormat.ts` now owns ZIP recognition at both ends of a file: `hasZipSignature` for the start and the new `locateTrailingZipArchive` for a trailing archive. The locator takes an injected positional read and is bounded to the last 22 + 65,535 bytes, one 46-byte central-directory header and one 4-byte local-header signature. It requires: - an end record whose comment ends exactly at end of file; - a single disk with at least one entry and no Zip64 markers; - a central-directory header at the derived position; - a local header at the first entry's offset, using an offset shift that resolves both prefix styles. - `src/domain/binaryTarget.ts` adds `claimsExecutableHeader`, which recognizes the ELF, MZ, Mach-O and FAT families that `parseExecutableHeader` interprets, whether or not the header is valid. Both callers use it, so they make the same decision. - `BinaryTargetResolver` consults the locator only after native header detection fails and only when no executable family claims the leading bytes. Root inventory classification (`classify.ts`) consults it only when the bytes classify as `file` or `unknown` and no executable family claims them. - `ZIP_NON_ENTRY_LIMITATION` now names leading bytes before the first entry, such as a self-extractor stub, as non-entry bytes that are not occurrences. - Not included: - Zip64, split, and empty trailing archives, which stay refused; - trailing data after the end record; - nested members, whose classification still uses their own leading bytes; - executable-led archives such as PE or ELF self-extractors, which keep their executable route. Admitting any of these would need a separate decision. - Scope decision: not applicable. This is format recognition within the existing ZIP reader boundary. The reader (`@zip.js/zip.js` 2.15.0) already reads both prefix styles, as verified below. ## Contract and boundary impact - Semantic owner and earliest changed stage: ZIP recognition in `src/domain/zipPackageFormat.ts`, consumed by target resolution and root inventory classification. - Contribution scope fit and linked maintainer decision: not applicable (ordinary fix). - CLI and MCP/tool-catalog contract: no schema change. Previously refused files with unclaimed leading bytes and a trailing ZIP now return `zip` artifact inventories and extractions. - Provider, bridge, target-format, or platform compatibility: classification is unchanged for every file another format recognizes. Executable-led files with appended archives keep their executable route. - Evidence, artifact, provenance, or reconstruction contract: root identity and digest cover the whole file, leading bytes included. Members keep their content identities. The ZIP limitation now names leading bytes as non-entry bytes. - Target selection, authority, upstream behavior, and cleanup details: no new process, write or network behavior. Extraction still uses the hardened ZIP reader and output tree. - Generated metadata, package, or installation impact: none. Expected effects: - [x] Read existing files or captured data. - [ ] Launch or attach to a process. - [ ] Inject instrumentation or run scripts/code. - [ ] Change process memory, registers, files, or configuration. - [ ] Contact network services. - [ ] Recover protected code or work around protections. - [ ] Documentation or usability only; no target interaction. - [ ] Not sure yet; investigation or maintainer guidance needed. ## Evidence and regression coverage - Tests added or updated: - `tests/acceptance/artifactZipLeadingBytes.test.ts`: the public CLI `inspect-artifact` and `extract-artifact` on a script-stubbed ZIP. It checks `zip` root format, verified members, the non-entry limitation, and the extracted bytes. This fails on the base revision with `target_unavailable`. - `tests/boundary/filesystem/zipDetection.test.ts`: - Both prefix styles classify as `zip` through root classification and `parseBinaryTarget`, with member identities equal to the plain archive's. - ELF-, MZ- and Mach-O-led files with an appended ZIP stay off the ZIP route. - A trailing ZIP is not located with bytes after the end record, a corrupt central-directory header, a Zip64 directory offset, or an empty archive record. - `tests/boundary/filesystem/artifactComparison.test.ts`: updated for the widened limitation text. - Base reproduction (executed, macOS arm64, main `d672cc239`): `plain.zip` passed; `prefixed.zip` (offsets include the stub) and `concat.zip` (raw concatenation) failed with "unsupported binary format". - After the change (executed): - All three inventory `a.txt`, `d`, and `d/b.txt` as `verified`, and extract members with identical digests. - `/bin/ls` + ZIP stays `mach-o-universal`. - A corrupted central directory, trailing junk, and a text file ending in an end-record-shaped tail are still refused. - Reader check (executed): `@zip.js/zip.js` 2.15.0 reads both prefix styles. For raw concatenation it rebases entry offsets itself (`a.txt` at offset 17). - User-visible CLI output (`inspect-artifact` on the concatenated file): root format `zip`; occurrences `.`, `a.txt`, `d`, `d/b.txt`, all `verified`. - Performance (executed, warm, 200 iterations): `locateTrailingZipArchive` takes 0.112 ms per call on a 50 MB unrecognized random file and 0.023 ms on the prefixed ZIP. It runs only for files that no other format claims; the cold-cache cost of one ≤64 KiB tail read was not measured separately. - Remaining proof gaps: - Zip64 trailing archives are not located. - Real PE self-extractors were not exercised; the MZ-led synthetic case covers the routing rule. - Linux and Windows hosts were not run locally; CI covers them. For evidence-bearing changes: - [x] Observed, derived, and inferred claims remain distinguishable. - [x] Artifact identity, source provenance, and failed attempts remain preserved. - [x] Unsupported, incomplete, unavailable, or uncertain outcomes remain visible. ## Validation performed - `npm run test:focused -- tests/acceptance/artifactZipLeadingBytes.test.ts tests/boundary/filesystem/zipDetection.test.ts tests/boundary/filesystem/artifactComparison.test.ts tests/acceptance/artifactZipCurrentDirectory.test.ts tests/acceptance/artifactZipExtractionPaths.test.ts tests/acceptance/pyinstallerArchive.test.ts tests/acceptance/godotPck.test.ts src/artifacts/ZipArtifactReader.test.ts src/artifacts/apple/machoContainer.test.ts`: 76 passed. - `npm run test:focused --` every test file that references `parseBinaryTarget`, `classifyAndHashRoot`, `BinaryTargetResolver` or `parseExecutableHeader` (46 files): 822 passed, 1 skipped. - `npm run check:fast`: passed (typecheck, lint). - `npm run format:check`: passed. - CLI reproduction and non-regression checks above: passed. - Review follow-up (93e55760e): the root ZIP family mapping (`.app` → `app-pack`) now lives once in `zipRootPackageFormat` and is shared by both resolver routes and root inventory; a stub-prefixed root `Example.app` is `app-pack` from both. `zipDetection` + CLI acceptance: 25 passed; `check:fast`, `format:check` passed. The first CI run failed only `peResources` (30 s timeout, also seen on main on 2026-10-20); it passed locally in 9 s. ## Compatibility, safety, and release - Breaking changes or migration steps: none. Only previously refused inputs change outcome. - Real Hopper/Ghidra, browser, or OS coverage: macOS arm64 only, locally. No provider is involved. - Package or release metadata impact: none. - Security, privacy, process, or containment review: the leading bytes are never executed or extracted. Extraction keeps the existing path-containment and output-tree checks, and the locator's reads are bounded. ## Author readiness Human owner: @kaoru0822-kitauji - [x] I reviewed the final diff and relevant existing code, understand the change and its edge cases, and take responsibility for the complete PR, including AI-assisted work. - [x] I checked the evidence above, confirmed it supports the current change, and recorded meaningful validation gaps. - [x] I declare this PR ready for initial maintainer review. ## Before merge - [ ] This PR addresses a concrete problem or an agreed enhancement. - [ ] Material scope and authority questions are resolved, and any required maintainer scope decision covers the final diff. - [ ] The PR has one focused outcome and the title follows `type(scope): outcome`. - [ ] Related issue is linked, or the reason for not linking one is stated above. - [ ] Tests cover changed observable behavior and meaningful failure paths. - [ ] Owning docs, contracts, and generated metadata are updated where needed. - [ ] User-visible CLI/MCP changes include representative output. - [ ] I checked the final diff for secrets, unrelated cleanup, and unsupported claims. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>
bsakaguchi 2026-11-01 21:45:10 +00:00
Sign in to join this conversation.
No milestone
No project
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference
morluto/rea#7073
No description provided.