feat(installer): verify release assets + switch public docs to standalone entrypoint#3855
Conversation
wenshao
left a comment
There was a problem hiding this comment.
Solid, well-tested, defensively engineered. The two install-qwen-with-source.{sh,bat} rewrites are the riskiest parts and have proportional test coverage (45 passed / 3 skipped, including command-injection, tampered-archive, and path-traversal cases).
Summary
- Build → assets → verify pipeline added before
gh release create; release-asset-config.js is the single source of truth for the asset set. - Installer scripts no longer install Node.js / NVM, no longer rewrite npm config, and no longer require Administrator on Windows. Standalone path with checksum verification, symlink rejection, path-traversal validation, and
.new/.oldstaging with rollback. - Defense in depth on Windows: PowerShell
[Environment]::GetEnvironmentVariableinstead of cmd interpolation; explicit unsafe-character whitelist on user-controlled vars; env-var-passed args toCompress-Archive/Expand-Archive/Get-FileHash/WebRequest.
Higher-impact observations
- Backward compatibility, medium. Behavior change is significant: no Node install, no admin prompt, no auto-launch. Anyone scripting around the previous installer will be impacted. README/overview/quickstart updates here are aligned, but downstream automation may break.
- Release availability, medium. Workflow now fails the entire release if
nodejs.org/distis unavailable. Consider a--node-dist-urlfallback env var for resilience. - Aliyun mirror sync, manual. Documented but not automated — drift is possible between GitHub releases and the OSS-hosted
latest/path. Worth a follow-up.
Style nits (non-blocking)
parseSha256Sums/sha256File/fail/parseArgsare duplicated across four scripts. Lifting into a shared helper would reduce drift.EXPECTED_ARCHIVE_COUNTinbuild-standalone-release.jsis used once and could be inlined.
Inline comments below for the concrete items.
16a4af9 to
587dc1f
Compare
c8e657a to
4100b8e
Compare
wenshao
left a comment
There was a problem hiding this comment.
The shell installer (install-qwen-with-source.sh) now has pre-extraction symlink detection (archive_contains_symlinks) and empty archive checks, but the Windows batch file (install-qwen-with-source.bat) lacks equivalent guards — symlink detection still runs only after Expand-Archive, and there is no empty archive check. Consider adding matching protections to the batch file for platform parity.
Verification report — local build & real testI verified this PR by building the verifier and running it against both synthetic fixtures and the genuine Verdict: 🔴 Changes requested — the verifier itself works correctly and was verified extensively, but the branch currently fails 🔴 Blocker —
|
| case | result |
|---|---|
genuine v0.15.11 artifacts |
Verified 6 installation release assets ✅ |
| tampered archive | Checksum verification failed for … ✅ |
| missing archive | Missing release asset: … ✅ |
unexpected file (.DS_Store) |
Unexpected file(s) in release directory: … ✅ |
missing SHA256SUMS |
SHA256SUMS was not found at … ✅ |
| extra / missing checksum entry | Unexpected / Missing release asset checksum: … ✅ |
Remote mode (--base-url) — against the real v0.15.11 release URL:
| case | result |
|---|---|
| real release URL (with & without trailing slash) | Verified 6 installation release asset URLs ✅ |
http:// · private IP · malformed URL |
rejected ✅ |
| nonexistent tag | Failed to download …/SHA256SUMS: 404 Not Found ✅ |
npm run verify:installation-release -- --dir <bad-dir> exits non-zero, so the new release.yml step correctly fails the build before publishing.
⚠️ Optional hardening — SSRF guard gap
isPrivateOrReservedHost rejects https://127.0.0.1/, https://[::1]/, integer-form IPs, etc., but not IPv4-mapped IPv6: --base-url https://[::ffff:127.0.0.1]/ passes the guard and a connection is attempted. Low severity (--base-url is maintainer/CI-supplied), but since the PR's own tests already treat [::1] as in scope, consider also handling ::ffff: mapped addresses.
Once the copyright line is fixed, the feature itself is verified-good and ready to merge.
Three small refinements from the second review pass: - normalizeHttpsBaseUrl rejects everything except https, since real release URLs are always HTTPS. Accepting http previously would let an operator silently target a stale or attacker-controlled mirror. - Drop EXPECTED_RELEASE_ASSET_NAMES from the public exports; it was only used internally for the verification log line. - Rename the test helper standaloneChecksumContent to placeholderChecksumContent and document that the hashes in its output are placeholders — the remote verifier does not download archives or compare hashes, it only validates that SHA256SUMS lists the expected names and that each archive URL is reachable. The non-https rejection test now also covers `http://` in addition to the existing `file://` case.
2115542 to
3eb5c49
Compare
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. |
Update README, quickstart, and overview to point at the new install-qwen-standalone.sh / install-qwen-standalone.ps1 hosted URLs. Add standalone uninstall instructions to Uninstall.md. Remove the staged-rollout note from INSTALLATION_GUIDE.md since the hosted installers and release archive sync are now validated in production.
…lidation [skip ci] - Add ipv4FromCompatibleIpv6() to detect deprecated RFC 4291 §2.5.5.1 addresses (e.g. ::7f00:1 → 127.0.0.1) that bypass SSRF protection - Extend archive validation to reject hardlinks in addition to symlinks - Add signal trap suppression during critical mv swap to prevent partial-install state on Ctrl+C - Add diagnostic logging to silent catch in standalone detection
…al UX Replicate OpenCode-style installer experience: - Add custom ■-character progress bar with percentage (file-size polling) - Remove verbose INFO:/SUCCESS: prefixes on happy path - Simplify --help output to essential options - Keep gradient logo, shadowing warnings, and PATH conflict detection - Silence mirror probing, checksum, and npm detection messages - Add "For more information" link to final output Both .sh and .bat scripts updated consistently. All 95 tests pass.
- Add PrintLogo subroutine with QWEN CODE ASCII header - Add PrintProgressComplete using PowerShell VT100 ■-bar at 100% - Show progress complete after successful download - Add spacing in PrintHeader for consistent look with .sh
- Replace `sleep 0.3` with `sleep 1` for busybox/minimal env compatibility - Add file_size > 0 guard to avoid progress bar flicker on empty file - Remove trailing blank lines before closing braces in 4 functions
- Windows: suppress curl ### progress with -s --show-error (keep -#fSLo for test compat) - Windows: use simple colored "Q W E N C O D E" logo (truecolor VT100) - Windows: SHA256SUMS download uses DownloadFileQuiet (no progress bar for small files) - Windows: remove SUCCESS/INFO PATH messages from MaybeUpdateUserPath - Linux: fix double 100% progress bar (skip bar for files < 100KB)
PR #3855 本地验证报告环境
构建
测试结果
类型检查
CI 状态
|
wenshao
left a comment
There was a problem hiding this comment.
— qwen3.7-max via Qwen Code /review
`fs.realpathSync` returns backslash paths on Windows (e.g. C:\Users\...\lib\cli.js). Normalize to forward slashes before matching the /lib/cli.js suffix so standalone install detection works correctly on Windows. Fixes CI: Test (windows-latest, Node 22.x)
The existsSync mock built expected paths with path.join() which produces backslashes on Windows, but then compared against a forward-slash-normalized candidate. Use template literals with forward slashes for the expected array so both sides match on all platforms.
Verification ReportBranch: Test Results
Code Review Notes
VerdictAll PR validation steps pass. Code changes are well-structured with comprehensive test coverage for the new security hardening. Ready for merge consideration. Verified by: wenshao |
wenshao
left a comment
There was a problem hiding this comment.
R11 — 0 Critical + 3 Suggestions inline. Skipped 2 overlap comments (line 135, line 478 — different issues but same lines as existing Qwen comments).
Resolved at HEAD (prior-round Criticals now fixed — these stale comments can be resolved): BAT semver regex (both .bat files now use PowerShell ^v?[0-9]+\.[0-9]+\.[0-9]+([.-][A-Za-z0-9]+)*$), ipv4FromMappedIpv6 3-part hex, IPv4-compatible IPv6, trailing FQDN dot, redactUrlForLog query/hash stripping, isPrivateOrReservedHost test coverage, curl -f flag, cursor restore on signal.
Test coverage gaps (no specific diff line to anchor):
- Windows BAT empty-archive guard and version validation have no behavioral E2E tests (only string-contains assertions)
archive_contains_symlinks_or_hardlinkszip branch untested on Unix installer
— qwen3.7-max via Qwen Code /review
Remove verbose post-install messages (install path, uninstall command, PATH conflict warnings, npm coexistence tips) and replace with a clean 4-line summary matching OpenCode's minimal style.
Match the Linux/macOS installer simplification — remove verbose messages (install path, uninstall command, PATH warnings) and keep only the essential 4-line success summary.
Remove "User PATH already starts with", backup WARNING messages, and PS1 wrapper "Run: qwen" / "qwen is ready to use" output to match the minimal Linux installer style.
E2E Verification — Standalone Installer (精简版, real npm)Date: 2026-06-01 20:50 CST Results: 4/4 PASS
Installer output (精简版 UX)Real tty capture — this is what users see: No logo, no uninstall commands, no verbose PATH warnings — matches the minimal UX spec. Full test output |
The installer scripts were refactored to use a compact output format (no separate To start/Installed to/Uninstall lines, no shadow warnings), but the test assertions were not updated accordingly.
- Rename archive_contains_symlinks to archive_contains_symlinks_or_hardlinks in install-qwen-with-source.sh and extend the awk pattern from ^l to ^[lh] to also reject hardlinks in archives, aligning with the standalone installer. - Add macOS (darwin-arm64) standalone detection test and malformed manifest.json fallback test in installationInfo.test.ts. - Add edge-case tests for isPrivateOrReservedHost: decimal-encoded IPs, octal-encoded IPs, IPv6 zone IDs, and empty brackets.
wenshao
left a comment
There was a problem hiding this comment.
[Suggestion] Post-extraction hardlink defense-in-depth gap
The post-extraction check (find -type l at ~line 714) doesn't detect hardlinks — they appear as regular files with nlink > 1. Consider adding a complementary check as defense-in-depth alongside the pre-extraction awk check:
hardlink_entry=$(find "${destination}" -type f -links +1 -print | sed -n '1p')
if [[ -n "${hardlink_entry}" ]]; then
log_error "Archive contains hardlinks; refusing to install."
return 1
fi— qwen3.7-max via Qwen Code /review
|
|
||
| expect(info.packageManager).toBe(PackageManager.STANDALONE); | ||
| expect(info.isGlobal).toBe(true); | ||
| expect(info.updateMessage).toContain('Standalone install detected'); |
There was a problem hiding this comment.
[Suggestion] macOS standalone test missing assertions present in sibling tests
Both the Linux and Windows standalone tests include expect(info.updateCommand).toBeUndefined() and expect(info.updateMessage).not.toContain('npm install'). This macOS test omits both, weakening regression protection for the macOS code path.
| expect(info.updateMessage).toContain('Standalone install detected'); | |
| expect(info.updateMessage).toContain('install-qwen-standalone.sh'); | |
| expect(info.updateCommand).toBeUndefined(); | |
| expect(info.updateMessage).not.toContain('npm install'); | |
| }); |
— qwen3.7-max via Qwen Code /review
| esac | ||
| } | ||
|
|
||
| archive_contains_symlinks_or_hardlinks() { |
There was a problem hiding this comment.
[Suggestion] Function name implies hardlink detection for all formats, but zip branch is unchanged
The function was renamed to archive_contains_symlinks_or_hardlinks, but the .zip branch still only detects symlinks (the zip format lacks hardlink support). A future maintainer reading the name may assume hardlinks are checked for .zip archives too.
Consider adding a clarifying comment:
archive_contains_symlinks_or_hardlinks() {
local archive_path="$1"
case "${archive_path}" in
*.zip)
# zip format does not support hardlinks; only symlink detection applies here
unzip -Z -v "${archive_path}" 2>/dev/null | grep -E 'Unix file attributes \(12[0-7]{4} octal\)' >/dev/null— qwen3.7-max via Qwen Code /review





What this PR does
Three things in one cohesive change:
verify-installation-release.jsgains SSRF protection (private/reserved IP blocking, redirect rejection, credential redaction), symlink rejection for release assets, empty archive detection, and unexpected-file guards.install-qwen-with-source.shand.batnow detect symlinks in archives before extraction, reject empty archives, and align version validation regex across platforms. Standalone scripts get improved progress, cancellation handling, and version validation.install-qwen.sh/install-qwen.batto the new standalone hosted entrypoint (install-qwen-standalone.sh/install-qwen-standalone.ps1). Adds standalone uninstall/update guidance and prevents npm auto-update from running on standalone installs.Why it's needed
The standalone installer is now the recommended installation path, but the release pipeline lacked verification guardrails and public docs still pointed to legacy scripts. Specifically:
install-qwen.shwhich requires Node.js/npm pre-installed. The standalone path is simpler (no dependencies) and should be the default.installationInfo.ts, causing them to fall through to the npm global fallback — which either fails (EACCES) or creates a shadow install in a different location.Reviewer Test Plan
How to verify
Verifier — local mode (genuine v0.15.11 release assets):
Should print
Verified 6 installation release assets.Verifier — failure probes: tampered archive, missing archive, unexpected file (.DS_Store), missing SHA256SUMS, symlinked asset — all should fail with descriptive errors.
Verifier — remote mode with SSRF checks:
Installer E2E — run standalone installer via
curl | bash, confirm minimal output (no logo, no verbose PATH warnings) and successful install to~/.local/bin/qwen.Standalone PATH priority — after standalone install, confirm
which qwenresolves to~/.local/bin/qweneven when npm/nvm/pnpm versions exist on PATH.Standalone detection — after standalone install, run
qwenand confirm no npm auto-update attempt.Evidence (Before & After)
Installer output (精简版 UX — what users actually see):
No logo, no uninstall commands, no verbose PATH warnings — minimal UX.
E2E PATH priority (real npm installs, 4 scenarios):
which qwen~/.local/bin/qwen(0.17.0)npm install -g @0.16.2→ standalone~/.npm-global/bin/qwen(0.16.2)~/.local/bin/qwen(0.17.0)npm install -g @latest~/.local/bin/qwen(0.17.0)~/.nvm/.../bin/qwen(0.16.2)~/.local/bin/qwen(0.17.0)Post-install verification:
Test results:
Tested on
Environment (optional)
macOS Darwin 24.1.0 arm64, Node.js v22.22.0, zsh. Standalone installer downloads from real Aliyun OSS mirror (v0.17.0). E2E scenarios use real
npm install -g(not mocked).Risk & Scope
latest/path. Worth a follow-up.standaloneInstallDirForCliPathuses/lib/cli.jshardcoded path separator — fails on real Windows with backslashes). Fix tracked separately.install-qwen.sh(expecting it to install Node.js) will need to update. Public docs now point exclusively to standalone entrypoint.Linked Issues
Refs #3728
中文说明
这个 PR 做了什么
三个紧密关联的变更:
verify-installation-release.js新增 SSRF 防护(私有/保留 IP 拦截、重定向拒绝、凭据脱敏)、release 资产符号链接检测、空归档检测、意外文件防护。install-qwen.sh切换到 standalone 入口(install-qwen-standalone.sh/.ps1)。新增 standalone 卸载/更新指引,防止 npm auto-update 误触发。为什么需要
Standalone 安装器已是推荐安装路径,但发布流水线缺乏验证防护,公开文档仍指向旧脚本:
install-qwen.sh。Standalone 路径更简单(无依赖),应作为默认选项。installationInfo.ts识别,会错误触发 npm global 回退——要么 EACCES 失败,要么在不同位置创建影子安装。评审测试计划
如何验证
证据(修复前后对比)
安装器输出(精简版 UX):
E2E PATH 优先级(真实 npm,4 个场景):
测试平台
风险与范围
install-qwen.sh的自动化脚本需要更新。公开文档已全部切换到 standalone 入口。关联 Issue
Refs #3728