Reject out-of-bound symlink targets - #158
Open
alehlopeh wants to merge 1 commit into
Open
Conversation
jkomyno
added a commit
to ComposioHQ/composio
that referenced
this pull request
Aug 23, 2026
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](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](max-mapper/extract-zip#158) | 2026-08-14 | GHSA-jmr9-qjv8-65gv, this advisory | | [#160](max-mapper/extract-zip#160) | 2026-08-17 | CVE-2026-19693, symlink at the final path component | | [#161](max-mapper/extract-zip#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](max-mapper/extract-zip#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](max-mapper/extract-zip#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
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Reject symlink entries whose resolved target falls outside the extraction directory. This prevents the path traversal described in GHSA-jmr9-qjv8-65gv while preserving symlinks that resolve within the destination.\n\nThe existing symlink fixture now verifies that extraction stops before creating the unsafe link. The AVA suite and both ESLint targets pass locally.