Repository navigation
chore(deps): bump sharp to ^0.35.0 to resolve GHSA-f88m-g3jw-g9cj - #8952
Merged
Merged
Conversation
yiliang114
requested review from
LaZzyMan,
doudouOUC,
tanzhenxin and
wenshao
as code owners
August 11, 2026 16:33
Collaborator
Author
|
This is a pure dependency bump ( Verification approach1. Build + typecheck — the import pattern change ( 2. Unit tests for image-view — the only code touched is 3. Quick smoke test — send an image to the CLI to confirm image processing (resize, metadata) still works end-to-end: No UI verification is needed — there are zero changes to any UI layer (CLI TUI, Web UI, or any rendering component). |
Contributor
Code Coverage Summary
CLI Package - Full Text ReportCore Package - Full Text ReportFor detailed HTML reports, please see the 'coverage-reports-22.x-ubuntu-latest' artifact from the main CI run. |
The published CLI's sharp version was hardcoded in prepare-package.js, which drifted from the workspace dependency on every bump. Read it from packages/core/package.json so the publish pin always matches the declared dependency. Add a test assertion to catch future drift in CI.
…ackage.json range The previous approach read the sharp version from packages/core/package.json (which has ^0.35.0) and stripped the caret, producing 0.35.0. This is the range floor, not the lockfile-resolved version (0.35.3). Read from package-lock.json so the published CLI ships the same version CI tests.
Co-authored-by: Qwen-Coder <[email protected]>
yiliang114
enabled auto-merge
August 12, 2026 08:43
doudouOUC
previously approved these changes
Aug 12, 2026
doudouOUC
left a comment
doudouOUC
left a comment
Collaborator
There was a problem hiding this comment.
已全面复核最新提交 87cd915。此前 review 指出的发布包仍钉在旧版、range floor 与 lockfile 解析版本不一致、读取错误 workspace resolution、缺少明确失败信息、测试无法防止重新硬编码等问题，均已在当前实现和 sentinel fixture 中正确关闭。
我检查了完整 diff、sharp 的全部生产读取点、npm 发布链路、lockfile 的语义变化和剩余 review threads；本地使用 sharp 0.35.3 验证了 packages/core typecheck、package-assets 31 个测试、动态 ESM default export，以及 metadata → extract → resize(lanczos3) → flatten → jpeg 的真实运行链路。相关图像测试共 257 个，256 个通过；唯一失败的权限边界用例已在未修改的 base commit 原样复现，与本 PR 无关。当前未发现新的阻塞或建议项，approve。
chiga0
reviewed
Aug 12, 2026
chiga0
left a comment
chiga0
left a comment
Collaborator
There was a problem hiding this comment.
Code Review Overview (AI Generated)
PR: #8952 chore(deps): bump sharp to ^0.35.0 to resolve GHSA-f88m-g3jw-g9cj
Author: @yiliang114
Type: Dependency bump + build-script change (scripts/prepare-package.js)
Change size: +814 / -248 across 7 files (of which +772/-232 is the regenerated lockfile)
Reviewed at HEAD: 87cd9157 — independent blind review (Phase 1) completed before reading any existing review.
Findings Summary
- Critical: 0
- Major: 0
- Minor: 3 (all in
scripts/prepare-package.js) - Nit: 3 (PR-description accuracy, lockfile side effects)
Key Observations
The dependency bump itself is correct and complete, and I verified every substantive claim against upstream artifacts rather than the PR description:
- Type change is sound.
[email protected]shipsdist/index.d.mtswhich exports bothSharpConstructor(line 1992) andMetadata(line 1220), plusexport default sharp(line 2045).SharpConstructorretainskernel: KernelEnum, sosharp.kernel.lanczos3atimage-view.ts:217still type-checks.packages/coreis"type": "module", so TS resolves theimportcondition — the correct entry. Runtimedist/index.mjsends inexport default Sharp, so(await import('sharp')).defaultis right. - Note for future CJS consumers:
dist/index.d.ctsstill usesexport = sharpand does not exportSharpConstructor. Today onlyimage-view.tsimports sharp types (verified via repo-wide code search), and core is ESM-only, so there is no breakage — but this type is not usable from a CJS-resolved file. - No API breakage from 0.35.0. I read the upstream changelog:
failOnErrorwas removed (the code already uses the newfailOn: 'error'),paletteBitDepthwas dropped from metadata (unused),format.jp2k→format.jp2(unused), and the newlimitInputChannelsdefault of 5 cannot affect a codepath restricted to static PNG/JPEG/WebP.Metadata.formatis non-optionalkeyof FormatEnumin both 0.34.5 and 0.35.3. - The lockfile diff is tightly scoped: 27 version changes (sharp + all
@img/*+ libvips 1.2.4→1.3.2, which carries the actual CVE fix) and 27 additions, with zero removals. - The
prepare-package.jschange is essential, not scope creep. Without it the published manifest would keepsharp: '0.34.5'and the fix would never reach users. Verified against the live registry:npm view @qwen-code/qwen-code@latest optionalDependenciescurrently returns"sharp": "0.34.5".
My three Minor findings are all residual gaps in the new lockfile reader, not defects in the bump.
Cross-Validation
I completed Phase 1 blind, then fetched the 3 qwen-code-ci-bot review rounds (7 findings) and @doudouOUC's approval. Every prior finding was re-verified against HEAD file content rather than trusting "fixed" claims.
| Finding | Other Reviewer | My Independent Assessment |
|---|---|---|
R1-1 (Critical): dist manifest keeps hard-coded sharp: '0.34.5' |
qwen-code-ci-bot | Confirmed + resolved. prepare-package.js:334 now emits sharp: sharpVersion. Registry check confirms the published 0.21.10 really does carry "sharp": "0.34.5", so the finding was genuine. Comment is now obsolete (anchor package.json:159 is unchanged, so GitHub does not grey it out). |
| R1-2: no test cross-checks the generated dist pin | qwen-code-ci-bot | Confirmed + resolved. package-assets.test.js:726-728 now asserts the pin. Obsolete. |
R2-1: pin derived from range floor via .replace(/^\^/,'') |
qwen-code-ci-bot | Confirmed + resolved by moving to the lockfile. Comment is orphaned: its anchor prepare-package.js:332 is now an unrelated @img comment line. |
R3-1: reads hoisted node_modules/sharp rather than core's resolution |
qwen-code-ci-bot | Partially resolved. The two-tier ?? lookup landed exactly as suggested and I verified the precedence is correct for every hoisting scenario. But R3-1's second half — "assert in the test that the pin satisfies core's declared range" — was not implemented, and closing R3-3 removed the fixture that would have enabled it. See Minor-1. Anchor line 327 is now orphaned. |
R3-2: missing guard vs. sibling readClipboardPackageSpecs |
qwen-code-ci-bot | Partially resolved. The TypeError half is fixed (optional chaining + explicit throw). The ENOENT half that R3-2 explicitly called out is still unguarded — fs.readFileSync at line 275 is bare. Folded into Minor-1. Anchor orphaned. |
R3-3: dead packages/core/package.json fixture |
qwen-code-ci-bot | Confirmed + resolved — fixture removed (0 matches at HEAD). Obsolete; anchor 1109 now points at the audio-capture fixture. |
| R3-4: drift guard survives a re-hard-code mutation | qwen-code-ci-bot | Confirmed + resolved. Sentinels 0.0.0-root-fixture / 0.0.0-core-fixture now also pin down the lookup precedence, which is a nice bonus beyond what R3-4 asked for. Obsolete; anchor 1094 now points at the fix itself. |
Blanket approval at 87cd9157 |
@doudouOUC | Largely agree — the bump is safe and all prior blockers really are closed. I do not fully agree that there are no remaining suggestions: the three Minor items below are reachable from the same code and were not raised by anyone. |
| Unique-Mine 1 (Minor): sibling reader validates lockfile↔manifest; this one does not | — | New — prepare-package.js:277 vs. build-standalone-release.js:170 |
| Unique-Mine 2 (Minor): the branch that runs in production is the untested one | — | New — prepare-package.js:279 |
| Unique-Mine 3 (Minor): exact pin structurally preserves #8944's un-remediable condition | — | New — prepare-package.js:334 |
Additional Audit Coverage
Dimensions I checked independently that go beyond the existing findings:
- Data provenance tracing — traced the sharp version end-to-end:
packages/core/package.jsonrange →npm ciresolution → lockfile key →writeDistPackageJson→ publishedoptionalDependencies→ end-user install. Confirmed the only path that reaches users is the dist manifest, so this PR's script change is load-bearing. - Caller/consumer impact — repo-wide code search for sharp consumers. Only
packages/core/src/utils/image-view.tsimports sharp types;fileUtils.tsmerely mentions sharp in a comment. Blast radius of thePreparedImage.sharptype narrowing is one file. - Upstream contract verification — fetched sharp 0.35.3's actual
package.jsonexports map,dist/index.d.mts,dist/index.d.ctsanddist/index.mjsfrom the registry rather than trusting the PR's assertions. - Project convention compliance (AGENTS.md) —
packages/core/src/**is maintainer-only "core infrastructure", but the core change here is 4 added / 10 removed lines and purely a type-level adjustment, well inside Tier 2's small-scope lane. Simplicity-First is respected: the diff removes more than it adds in core. - Sibling code consistency — compared the new reader against
readClipboardPackageSpecs, the repo's established lockfile-version reader. This produced Minor-1. - Test-fixture vs. production-reality diff — compared the fixture lockfile shape against the real lockfile at HEAD. This produced Minor-2.
- Release-pipeline reachability —
.github/workflows/release.ymlrunsnpm ci→npm run bundle→npm run prepare:package, sopackage-lock.jsonandnode_modulesare both guaranteed present at packaging time. This is why Minor-1's ENOENT gap is Minor and not Major.
Nits (no inline comment)
- The PR description mis-characterises the residual
[email protected]. It callsmobilewright"a dev tooling dependency of the mobile-mcp package", butpackages/mobile-mcp/package.jsondeclares"mobilewright": "0.0.45"underdependencies, and the lockfile marksnode_modules/@mobilewright/core/node_modules/sharpasdev: false. Consequentlynpm audit --omit=devwill still report GHSA-f88m-g3jw-g9cj through@qwen-code/mobile-mcp → mobilewright → @mobilewright/core → [email protected], contrary to the Reviewer Test Plan.@qwen-code/mobile-mcpis also a published package (currently0.1.5on npm). This does not affect the published CLI — its dist manifest hasdependencies: {}— so #8944 is genuinely fixed; only the description's framing is wrong. - Side effect worth noting: because the top-level resolution moved to 0.35.3, mobilewright's
^0.34.5no longer dedupes and the lockfile gains a full nested sharp tree (27 new entries incl. all@imgplatform packages). The dev tree now materialises two sharp installations. - Stale engine claim: the description states sharp 0.35 requires
^18.17 || ^20.3 || >=22— that is sharp 0.34's range.[email protected]declaresengines: { node: ">=20.9.0" }. The conclusion (compatible with this project's Node >= 22) is still correct. - Unrelated drift: the regeneration also bumped
semver7.7.3 → 7.8.5 (visible inNOTICES.txt). Benign, but it is not part of the stated scope.
Industry Context
The two-tier lockfile lookup matches how other npm monorepos pin natively-built optional dependencies for publish, and the choice to declare only sharp (letting its own optionalDependencies fan out to @img/sharp-<platform>) is the pattern sharp's own documentation recommends — pinning the platform packages would indeed drift, exactly as the in-code comment says. One upstream change is worth flagging for operators: sharp 0.35.0 removed the install script, so when no prebuilt binary matches, sharp no longer self-heals and simply fails to load. image-view.ts already degrades gracefully into renderer_unavailable, so the CLI stays usable — but installs performed with --no-optional or with --os/--cpu overrides will now silently lose image rendering instead of building from source.
Final Verdict
LGTM with minor suggestions — recommend merge. The bump is correct, the type migration is verified against sharp 0.35.3's actual shipped declarations, the changelog contains no breaking change that touches this codebase's usage, and the prepare-package.js fix is what makes the security fix actually reach users. All seven prior review findings are genuinely closed at 87cd9157 — I verified each against HEAD rather than trusting the author's replies. CI is green (40/41 complete, only the review bot still running); mergeable_state is blocked pending approvals, not checks. The three Minor items are hardening of the new drift guard and can reasonably land as a follow-up rather than blocking this security fix.
This review was generated by QoderWork AI
…d fallback test - Align the lockfile reader with the sibling pattern in build-standalone-release.js: validate the resolved version against packages/core's declared sharp range - Wrap the lockfile read in try/catch so a missing or malformed file surfaces a clear error instead of an opaque ENOENT/SyntaxError - Add a test for the hoisted fallback path (node_modules/sharp) that actually executes in the current production release - Add a comment explaining why sharp is exact-pinned like all other native optional deps in the published manifest
Collaborator
|
Reviewed the whole change and verified the dependency-level claims independently. The bump itself is sound — my notes below are about scope and coverage, not about the upgrade being wrong. Verified ✅The lockfile delta is genuinely minimal. I parsed both lockfiles and diffed the resolved versions rather than reading the 772-line text diff. All 27 version changes are sharp-related: plus two incidental re-resolutions ( The type change is correct against the real 0.35.3 artifact. I unpacked the published tarball: 1.
|
doudouOUC
reviewed
Aug 12, 2026
doudouOUC
left a comment
doudouOUC
left a comment
Collaborator
There was a problem hiding this comment.
Reviewed. Suggestions are inline.
Not explored to full depth (tool budget reached): This PR bumps sharp from ^0.34.5 to ^0.35.0.: 无。所有检查都在预算内完成。.
Not reviewed: verification and reverse audit — neither the verifier nor the reverse auditor was launched with a prompt this skill builds — the posted findings were ruled on, and the misses the rest of the review left were hunted, if at all, without the briefs this skill certifies against.
中文说明
已审查。 建议见行内评论。
未探索到全部深度（达到工具调用预算）：This PR bumps sharp from ^0.34.5 to ^0.35.0.：无。所有检查都在预算内完成。。
未审查：验证与反向审计——验证 agent 与反向审计 agent 都没有用本 skill 构建的 prompt 启动——发布的发现即便被裁定过、评审其余部分遗漏的问题即便被搜寻过，也都缺失了本 skill 用以认证的 brief。
— deepseek-v4-flash via Qwen Code /review (v0.21.10)
Collaborator
Author
|
Closeout from resolve-pr-comments automation: Changed: merged latest main to refresh the exact-head linter-install CI failure. No product code changes beyond the base refresh. Pending: new CI, SDK Java, and automatic review are running; existing review threads still need a separate fix/decision pass. |
Collaborator
Author
|
No code change. The exact-head failed Qwen Code CI run had empty failed-job logs from the API, and this branch was already refreshed to latest base in the previous pass, so I reran the failed jobs and left it pending verification. |
Collaborator
Author
|
The exact-head failed-job logs for run 31633973484 were empty via |
yiliang114
dismissed
a stale review
August 13, 2026 03:15
Dismissed as stale: later commits made the generated dist manifest resolve and validate sharp from the lockfile/core declaration, and exact-head Test passed. Round 7 reviewed the unchanged PR files with no findings.
yiliang114
dismissed
a stale review
August 13, 2026 03:15
Dismissed as stale: commit 343824f replaced exact range equality with semver compatibility validation. The PR files are unchanged at the current head, exact-head Test passed, and round 7 reported no findings.
Collaborator
Author
|
@qwen-code /triage |
ytahdn
approved these changes
Aug 13, 2026
wenshao
approved these changes
Aug 13, 2026
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
6 participants
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.
What this PR does
Bumps the
sharpimage library from^0.34.5to^0.35.0in the two places that pin a version — the core package's production dependency and the root dev dependency — and regenerates the lockfile (resolves[email protected]). The dynamic import in the image rendering path is also updated to match sharp 0.35's new bundled ESM type definitions: the oldexport =workaround for@types/sharpis replaced by a direct.defaultaccess typed asSharpConstructor, with no runtime behavior change.Why it's needed
Reported in #8944: every install of the published CLI pulls
[email protected]through the core package, andnpm auditflags GHSA-f88m-g3jw-g9cj (2 high severity — libvips CVEs inherited by sharp). The advisory is fixed in[email protected], but under 0.x caret semantics^0.34.5never matches0.35.x, so users cannot clear the warning withnpm audit fix— the bump has to land here and ship in a release.Reviewer Test Plan
How to verify
npm audit --omit=devshould not listsharp <0.35.0under@qwen-code/qwen-code/@qwen-code/qwen-code-core(a remaining nested[email protected]under@mobilewright/corein the monorepo dev tree is pre-existing and unrelated to the published CLI).cd packages/core && npx vitest run src/utils/image-view.test.ts src/tools/zoom-image.test.ts src/utils/fileUtils.test.ts src/utils/readManyFiles.test.ts src/utils/pathReader.test.ts— all pass. Two pre-existing permission-boundary failures inread-file.test.ts/zoom-image.test.tsreproduce identically on unmodifiedmainin my environment and are unrelated to this change.node -e "import('sharp').then(m => console.log(m.default.versions.sharp))"prints0.35.3, and the default export remains callable withkernel.lanczos3available. An end-to-end render through the exact production pipeline (dynamic import →metadata()→extract→resize(lanczos3)→flatten→jpeg) on a generated 2000x1500 PNG produces valid JPEG output.[email protected]under@mobilewright/corecomes frompackages/mobile-mcp, which is not in the published CLI's dependency tree at all, so end users never install it.Evidence (Before & After)
N/A (dependency bump, no UI change)
Tested on
Environment (optional)
Unit tests only (
npm run devnot exercised).Risk & Scope
^18.17 || ^20.3 || >=22, which matches this project's Node >= 22 requirement; the only production usage (metadata()+extract/resize/flatten/jpegwithfailOn: 'error',limitInputPixels,lanczos3) is stable across this minor bump.[email protected]pulled bymobilewright(a dev tooling dependency of the mobile-mcp package) and the separatepackages/desktopworkspace pins — neither affects the published CLI audit.Linked Issues
Fixes #8944
中文说明
本 PR 做了什么
将图像库
sharp从^0.34.5升级到^0.35.0，涉及两处版本锁定 —— core 包的生产依赖与根目录的 devDependency —— 并重新生成 lockfile（解析为[email protected]）。同时按 sharp 0.35 自带的 ESM 类型定义调整了图像渲染路径中的动态导入：移除针对@types/sharp的旧export =兼容写法，改为直接使用类型为SharpConstructor的.default，运行时行为不变。为什么需要
来自 #8944 的反馈：已发布 CLI 的每次安装都会经由 core 包引入
[email protected]，npm audit会报告 GHSA-f88m-g3jw-g9cj（2 个高危 —— sharp 继承的 libvips CVE）。该公告在[email protected]修复，但 0.x 的 caret 语义下^0.34.5永远匹配不到0.35.x，用户侧npm audit fix无法消除告警 —— 必须在本仓库升级并随版本发布。评审验证计划
如何验证
npm audit --omit=dev中@qwen-code/qwen-code/@qwen-code/qwen-code-core下不应再出现sharp <0.35.0（monorepo 开发树中@mobilewright/core嵌套的[email protected]为既有状态，与已发布 CLI 无关）。cd packages/core && npx vitest run src/utils/image-view.test.ts src/tools/zoom-image.test.ts src/utils/fileUtils.test.ts src/utils/readManyFiles.test.ts src/utils/pathReader.test.ts—— 全部通过。read-file.test.ts/zoom-image.test.ts中两个权限边界用例的失败在未改动的main上同样复现，属本地环境既有问题，与本变更无关。node -e "import('sharp').then(m => console.log(m.default.versions.sharp))"输出0.35.3，default 导出仍可调用且kernel.lanczos3可用。走与生产完全一致的渲染链路（动态导入 →metadata()→extract→resize(lanczos3)→flatten→jpeg）对生成的 2000x1500 PNG 做端到端渲染，输出正常。@mobilewright/core嵌套的[email protected]来自packages/mobile-mcp，该包完全不在已发布 CLI 的依赖树中，终端用户不会安装到它。证据（前后对比）
N/A（依赖升级，无 UI 变化）
测试环境
环境（可选）
仅单元测试（未跑
npm run dev）。风险与范围
^18.17 || ^20.3 || >=22，与本项目 Node >= 22 的要求一致；生产代码仅使用metadata()+extract/resize/flatten/jpeg（含failOn: 'error'、limitInputPixels、lanczos3），在该 minor 版本间 API 稳定。mobilewright（mobile-mcp 包的开发工具依赖）嵌套引入的[email protected]，以及独立的packages/desktop工作区中的版本锁定 —— 两者均不影响已发布 CLI 的审计结果。关联 Issue
Fixes #8944