Skip to content

docs: refresh the v3 plan, flag decompress for removal - #662

Merged
Kamirus merged 1 commit into
mainfrom
kamil/next-major-refresh-decompress
Aug 5, 2026
Merged

docs: refresh the v3 plan, flag decompress for removal#662
Kamirus merged 1 commit into
mainfrom
kamil/next-major-refresh-decompress

Conversation

@Kamirus

@Kamirus Kamirus commented Aug 4, 2026

Copy link
Copy Markdown
Collaborator

decompress is a critical, unfixable advisory in the CLI's dependency tree, and the plan doc said dropping vessel would take it with it. Neither half of that was right, so both are corrected here along with the parts of the plan that #642 made stale.

decompress has to go, and vessel won't take it

cli/package.json pins decompress 4.2.1. Both advisories against it cover <=4.2.1, so the newest release is still affected and there is nothing to upgrade to:

$ npm audit
critical  decompress
  Decompress: Archive extraction can create files and links outside of the target directory
  decompress: Arbitrary File Write via Archive Extraction (Zip Slip)
  fixAvailable: false

GHSA-mp2f-45pm-3cg9, GHSA-h39j-r5qq-r9mm. Removing the dependency is the only remedy, and there are two call sites — the toolchain extractor (toolchain-utils.ts:64, for moc/wasmtime/pocket-ic/lintoko archives) and installFromGithub (vessel.ts:167, for repo = "..." deps). The second one is the exposed surface: those archives come from arbitrary user-specified repos, which is precisely the zip-slip threat model.

Filed in TODO.md rather than here, because replacing an extractor breaks nothing and should not wait for a major.

installFromGithub must move, not be deleted

The vessel item listed installFromGithub among the things v3 deletes. It lives in cli/vessel.ts for historical reasons only — cli/commands/install/install-dep.ts:23 and cli/commands/add.ts:104 call it for ordinary GitHub deps, which mops supports and which the lockfile-v4 track plans to improve with commit-SHA pinning. Deleting it would break git deps; it moves into cli/commands/install/ instead.

This is also why dropping vessel does not resolve the advisory on its own: that decompress call site outlives vessel.

pic migration is history now

#642 shipped the @dfinity/pic switch, so the long spike narrative is replaced by current state and the three follow-up issues it produced (#651 vendor bundling, #652 @icp-sdk/core 5.x, #655 log hook). Only removing the legacy pic-ic client is still v3 work, tracked in #653. Provenance notes are kept in condensed form, since the reason the fork existed is worth not relearning.

Docs only

No code changes. Nothing here is an argument for cutting v3 sooner — the decompress work deliberately lands outside it.

`decompress` has two critical advisories that cover every published
version including the 4.2.1 we pin, so `npm audit` offers no fix and
removing the dependency is the only remedy. Recorded in TODO.md with both
call sites, because it is non-breaking and should not wait for v3.

Two corrections while there:

- `installFromGithub` cannot be deleted with vessel. It sits in
  vessel.ts for historical reasons, but `install-dep.ts` and `add.ts`
  call it for ordinary `repo = "..."` deps, so it has to move instead.
  Which also means dropping vessel does not remove `decompress` — that
  call site survives, alongside the toolchain extractor.
- The pic migration shipped in 2.x (#642), so its spike narrative is
  replaced by current state plus the follow-up issues (#651, #652,
  #655). Only the legacy `pic-ic` removal is still v3 work.
@github-actions

github-actions Bot commented Aug 4, 2026

Copy link
Copy Markdown
Contributor

Cursor AI review

👍 APPROVE — looks safe to merge

Category Assessment Details
Summary Docs-only refresh of NEXT-MAJOR.md / TODO.md: corrects that installFromGithub must move (not die with vessel), flags replacing vulnerable decompress, and condenses the shipped #642 pic narrative into follow-up issue pointers.
Code Quality No code changes; planning text matches real call sites (install-dep.ts:23, add.ts:104, vessel.ts:167/188, toolchain-utils.ts:64).
Consistency Aligns with existing split (TODO.md = non-breaking, NEXT-MAJOR.md = v3); lockfile-v4 note’s “installFromGithub must survive vessel deletion” stays consistent with the correction.
Security No runtime/auth/integrity changes. Verified decompress@4.2.1 still in cli/package.json and used at the two cited sites; TODO correctly treats replacement as 2.x work, not v3-gated.
Tests Docs-only; no behavior to cover.
Maintainability ⚠️ #652 bullet says “see the corrected note above” after this PR deleted that note (NEXT-MAJOR.md:70); substance (“belongs with dfx-replica removal”) remains, so harmless dangling cross-ref only.

Verdict

Decision: APPROVE
Risk: Very Low
Reason: Planning-doc updates only; cited call sites and dependency facts check out against the checkout, with no production code or release-pipeline impact.


Generated for commit f3adfab

@automation-sa-sre automation-sa-sre left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Automated approval: the AI review verdict for f3adfab is APPROVE. See the "Cursor AI review" comment for details.

@Kamirus
Kamirus merged commit 5218b9a into main Aug 5, 2026
30 checks passed
@Kamirus
Kamirus deleted the kamil/next-major-refresh-decompress branch August 5, 2026 07:45
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