mirror of
https://github.com/ComposioHQ/composio.git
synced 2026-09-22 11:46:35 +08:00
Alberto Schiabel
f926058147
Addresses the `pnpm audit --prod` warning that has been riding along on recent CLI PRs: **`extract-zip` unvalidated symlink path traversal**, [GHSA-jmr9-qjv8-65gv](https://github.com/advisories/GHSA-jmr9-qjv8-65gv) / CVE-2026-56876, high. ## There is no version to upgrade to `pnpm audit` renders "Patched versions >=2.0.2", which reads like a bump would fix it. It would not: - `npm view extract-zip versions` ends at **2.0.1** — no 2.0.2 was ever published - the advisory itself records `"first_patched_version": null` So `pnpm audit --fix --prod` cannot resolve this, and neither can a version constraint. ## What the flaw actually is I reproduced it rather than working from the summary. `extract-zip` 2.0.1 **does** block writes that traverse a symlink — an archive with `link -> ../../ESCAPED` plus `link/pwned.txt` is refused with `Out of bound path ... while processing file link/pwned.txt`, and nothing lands outside the target. What it does not do is validate the symlink itself. An archive whose only entry is `escape-abs -> /tmp/ABSOLUTE-TARGET` extracts cleanly and plants that dangling symlink in the output directory. Per the advisory: *"Depending on how extract-zip is used, an attacker could read or write to arbitrary files."* Every call site here extracts an archive and then reads or copies files back out of the tree, which is exactly the shape that turns a planted symlink into an arbitrary-path read. ## Why not swap the library `#4183` moved tar extraction to `Bun.Archive`, so that was the obvious candidate. It does not work: **`Bun.Archive` cannot read zip archives at all.** A benign zip fails with `ReadError`, same as the malicious ones — those "refusals" were read failures, not security. Verified before ruling it out. ## Why not fix it upstream Because it already is, three times over, and none of it has moved: | PR | Date | Targets | | --- | --- | --- | | [max-mapper/extract-zip#158](https://github.com/max-mapper/extract-zip/pull/158) | 2026-08-14 | GHSA-jmr9-qjv8-65gv, this advisory | | [#160](https://github.com/max-mapper/extract-zip/pull/160) | 2026-08-17 | CVE-2026-19693, symlink at the final path component | | [#161](https://github.com/max-mapper/extract-zip/pull/161) | 2026-08-20 | CVE-2026-56876 again, and bumps the version to 2.0.2 | All three are mergeable with zero review comments. The repository's last commit to `master` was **August 2021**; 2.0.1 shipped June 2020. [Issue #159](https://github.com/max-mapper/extract-zip/issues/159), a downstream consumer asking whether a patched release is planned, has three replies — every one from another stranded consumer, none from a maintainer. A fourth PR would not be a contribution. And our implementation is the wrong shape to upstream regardless: refusing *every* symlink is right for an application whose archives never carry one, but a breaking change for a general-purpose library, where the correct fix is validating that a link's resolved target stays inside the extraction root — which is what #158 and #161 already do. Upstream even tracks "do not extract symlinks" as a separate [feature request](https://github.com/max-mapper/extract-zip/issues/140), open since 2023. So this is a deliberate local workaround on a timeline we control. The helper's doc comment records the tracking PRs and the condition for deleting it: a published release that actually carries the fix. ## The fix No archive either package extracts — release binaries, companion assets, skills, sidecar bundles — legitimately contains a symlink. So the entry type is refused outright, via `extract-zip`'s own `onEntry` hook, which runs **before** the entry is written. A rejected archive plants nothing. All four call sites now go through it: | package | call site | | --- | --- | | `cli` | `services/upgrade-binary.ts` | | `cli` | `services/run-companion-modules.ts` | | `cli` | `effects/install-skill.ts` | | `cli-local-tools` | `src/bundled-binaries.ts` | Each package carries its own copy of the helper rather than sharing one. `run-companion-modules.ts` is bundled as a standalone companion module and imports nothing from `@composio/cli-local-tools`; routing it through that package to share ~15 lines would pull the local-tools tree into those bundles. ## Verification Committed three fixture archives (827 bytes total) and tested against real zips, not mocks: - benign archive extracts, contents intact - `symlink-absolute.zip` (`-> /tmp/ABSOLUTE-TARGET`) refused, output directory left empty - `symlink-relative.zip` (`-> ../../outside`) refused, output directory left empty - `isSymlinkZipEntry` unit-tested across symlink, regular file, directory, and Windows-authored entries with no Unix mode **Mutation-checked:** neutering the predicate makes exactly the three symlink tests fail and leaves the other four green, so the suite genuinely detects the regression rather than passing by construction. Suites: `@composio/cli` 123 files / 1263 passed, `@composio/cli-local-tools` 7 files / 20 passed. Typecheck clean on both. `pnpm validate:changesets` passes; note lands in `ts/packages/cli/CHANGELOG.md`. `extract-zip` stays as the dependency — it is still the only working zip reader here, and with symlink entries refused the advisory's vector no longer applies to this codebase. https://claude.ai/code/session_01JkwtxPHfobZxzvAqe53x62