feat(installer): add standalone hosted install and uninstall flow by yiliang114 · Pull Request #3828 · QwenLM/qwen-code

yiliang114 · 2026-05-04T13:35:56Z

Summary

Adds the standalone hosted installation flow for release archives, including Linux/macOS and Windows entrypoints, checksum staging, release-time OSS publishing, and Aliyun latest/VERSION resolution.
Adds a matching standalone uninstall flow so users who installed outside npm can remove the standalone runtime, generated command wrappers, and installer-managed PATH changes without deleting their Qwen Code config by default.
Fixes the hosted Windows PowerShell entrypoint so installs launched from cmd.exe can make qwen available in the same command prompt when a writable directory already on the inherited PATH can host the generated shim.

Validation

Commands run:

npm run test:scripts
npm run package:hosted-installation -- --out-dir dist/installation
bash -n scripts/installation/install-qwen-standalone.sh scripts/installation/uninstall-qwen-standalone.sh
npx prettier --experimental-cli --check .github/workflows/release.yml scripts/release-script-utils.js scripts/verify-installation-release.js scripts/tests/install-script.test.js scripts/installation/INSTALLATION_GUIDE.md
git diff --check

Latest incremental check after the final mirror-probe cleanup commit:

bash -n scripts/installation/install-qwen-standalone.sh
git diff --check -- scripts/installation/install-qwen-standalone.sh

Expected release result:
- Versioned standalone archives and release SHA256SUMS are uploaded to releases/qwen-code/<tag>/.
- Stable releases update the small releases/qwen-code/latest/VERSION pointer after versioned release assets are uploaded and verified; there is no latest/<archive> alias.
- Hosted installer/uninstaller snapshots and hosted SHA256SUMS are uploaded to installation/<tag>/ for audit and rollback; the release workflow does not overwrite the global installation/ entrypoint objects.
- Aliyun latest installs read latest/VERSION, then download the archive and SHA256SUMS from the resolved versioned release directory.
Observed result:
- Installer script test suite passed locally before the final one-line cleanup change.
- Hosted asset staging completed and produced checksums for install and uninstall entrypoints.
- Public OSS global hosted assets were re-synced manually after the latest script changes, then downloaded back and verified successfully against hosted SHA256SUMS.
- Public OSS releases/qwen-code/latest/VERSION was verified to resolve to the current stable release pointer during manual OSS sync validation.
- npm run build && npm run typecheck did not complete locally because build stops on the existing @lydell/node-pty declaration resolution issue under Node v22.22.0.
Quickest reviewer verification path:
1. Run npm run test:scripts.
2. Run hosted asset staging and inspect dist/installation/SHA256SUMS.
3. For a non-dry-run stable release, verify the pointer and resolved versioned assets:
```
BASE=https://qwen-code-assets.oss-cn-hangzhou.aliyuncs.com
TAG=vX.Y.Z
test "$(curl -fsSL "$BASE/releases/qwen-code/latest/VERSION" | tr -d '[:space:]')" = "$TAG"
npm run verify:installation-release -- --base-url "$BASE/releases/qwen-code/$TAG"
```
4. Download hosted installer assets from installation/ and run sha256sum -c SHA256SUMS against them.
5. On Windows, install through the hosted PowerShell entrypoint from cmd.exe, then run qwen -v in the same prompt.
6. Run the hosted standalone uninstaller and verify the standalone install path is removed while user config remains.

Scope / Risk

Main risk or tradeoff: Windows current-prompt availability depends on finding a writable directory that already exists on the parent cmd.exe PATH; when that is not available, the installer prints an explicit one-line PATH fallback.
Release rollout note: hosted installers are fixed OSS entrypoint objects under installation/; release jobs publish versioned hosted snapshots under installation/<tag>/, while the moving release selection for Aliyun is the small latest/VERSION pointer, not duplicated latest archives.
Not covered / not validated: Windows E2E was validated manually against the hosted installer path; repository Windows CI should continue covering Windows script behavior. Local full build is currently blocked by the unrelated @lydell/node-pty typing resolution issue noted above.
Breaking changes / migration notes: No npm install behavior change. Standalone installs now have a dedicated uninstall path because npm uninstall @qwen-code/qwen-code cannot remove standalone files.

Testing Matrix

	🍏	🪟	🐧
npm run	✅	⚠️	⚠️
npx	✅	⚠️	⚠️
Docker	N/A	N/A	N/A
Podman	N/A	N/A	N/A
Seatbelt	N/A	N/A	N/A

Testing matrix notes:

macOS local script checks passed.
Windows hosted installer behavior was manually exercised from cmd.exe.
Windows and Linux should be covered by repository CI for the platform-specific script cases.

Linked Issues / Bugs

Related #3728

wenshao

Requesting changes due to a deterministic script test failure.

— gpt-5.5 via Qwen Code /review

yiliang114 · 2026-05-05T13:11:00Z

@copilot pls review for me

Agent-Logs-Url: https://github.com/QwenLM/qwen-code/sessions/38467aec-15b9-4b76-9139-0b2cfe40477a

wenshao · 2026-05-20T07:20:40Z

+            fi
+            ;;
+        *.tar.gz|*.tgz|*.tar.xz)
+            if ! entries=$(tar -tf "${archive_path}"); then


[Critical] tar -tf does not show symlink targets (both GNU tar and macOS bsdtar omit the -> target portion), so validate_archive_entry_path never sees symlink entries and cannot reject them. Then at line 936, tar -xzf follows symlinks during extraction on Linux (GNU tar default). A crafted archive with a symlink entry (e.g., qwen-code/evil -> /etc) followed by a regular file entry traversing that symlink (e.g., qwen-code/evil/cron.d/backdoor) achieves arbitrary file write before the post-extraction find -type l check at line 948 can detect it. This is a textbook tar-symlink vulnerability (same root cause as CVEs in pip, npm tar, etc.). macOS is safe because bsdtar refuses to extract through symlinks.

Fix: use tar -tvf (verbose listing shows -> target) to reject any symlink entries before extraction:

Suggested change

if ! entries=$(tar -tf "${archive_path}"); then

*.tar.gz|*.tgz|*.tar.xz)

if tar -tvf "${archive_path}" 2>/dev/null | grep -q ' -> '; then

log_error "Archive contains symlinks; refusing to install."

return 1

fi

if ! entries=$(tar -tf "${archive_path}"); then

log_error "Failed to inspect archive entries: ${archive_path}"

return 1

fi

;;

— qwen-latest-series-invite-beta-v34 via Qwen Code /review

wenshao · 2026-05-20T07:20:40Z

+      { stdio: 'inherit' },
+    );
+
+    if (result.error) {


[Critical] The result.error branch (ossutil missing from PATH, fork failure, etc.) is never exercised in tests. The test shim always spawns a Node process that exits cleanly (status 0 or 1), so spawnSync never populates result.error. If ossutil is absent in CI, the error propagation behavior (throwing without retry) is unverified.

Add a test where ossutilCommand points to a nonexistent binary and assert the thrown error matches ENOENT without retrying.

— qwen-latest-series-invite-beta-v34 via Qwen Code /review

wenshao · 2026-05-20T07:20:40Z

+  config,
+  { ossutilCommand, ossutilCommandArgs },
+) {
+  for (let attempt = 1; attempt <= MAX_UPLOAD_ATTEMPTS; attempt += 1) {


[Critical] The retry logic only tests two scenarios: success on first attempt, and failure after all 3 attempts. The transient-failure-then-success path (attempt 1 fails, attempt 2 succeeds) — the core value of the retry mechanism — is never tested. A regression in the retry loop (wrong backoff, off-by-one, missing return after success) would go undetected.

Add a test with an ossutil shim that tracks invocation count in a file and exits 1 on the first call, 0 on subsequent calls.

— qwen-latest-series-invite-beta-v34 via Qwen Code /review

wenshao · 2026-05-20T07:20:40Z

            PRERELEASE_FLAG="--prerelease"
          fi

+          mapfile -t release_assets < <(node scripts/verify-installation-release.js --dir dist/standalone --list-release-asset-paths)


[Suggestion] Bash mapfile silently swallows failures from the process substitution <(...), even under set -euo pipefail. If verify-installation-release.js exits non-zero, release_assets is empty and gh release create proceeds with only dist/cli.js — publishing a release with no standalone archives.

Add a guard:

if [[ ${#release_assets[@]} -eq 0 ]]; then echo "::error::Release asset verification produced no assets" exit 1 fi

— qwen-latest-series-invite-beta-v34 via Qwen Code /review

wenshao · 2026-05-20T07:20:40Z

+)
+
+set "EXPECTED_HASH="
+for /f "usebackq tokens=1,2" %%H in ("!CHECKSUM_FILE!") do (


[Suggestion] The for /f loop iterates all lines in SHA256SUMS and keeps the first match (if "!EXPECTED_HASH!"=="" guard). While "first match wins" is safer than "last match wins", the loop doesn't break early. Batch for /f doesn't support break, but you can goto out of the loop after finding the first match to avoid processing remaining lines.

— qwen-latest-series-invite-beta-v34 via Qwen Code /review

wenshao · 2026-05-20T07:20:40Z

+function fetchWithTimeout(fetchImpl, url, options = {}) {
+  return fetchImpl(url, {
+    ...options,
+    signal: AbortSignal.timeout(REMOTE_FETCH_TIMEOUT_MS),


[Suggestion] AbortSignal.timeout(30_000) covers the entire request lifecycle: DNS, TCP, TLS, headers, and body consumption. In fetchSha256, the body is a 50-150MB archive streamed through pipeline(). If the total download exceeds 30s (slow CDN, rate limiting), the abort fires mid-stream and the error is reported as "unavailable release asset" rather than a timeout.

Consider increasing REMOTE_FETCH_TIMEOUT_MS to 300_000 (matching curl --max-time 300 used elsewhere) or applying a separate timeout for body streaming.

— qwen-latest-series-invite-beta-v34 via Qwen Code /review

wenshao · 2026-05-20T07:20:40Z

+        run: |-
+          set -euo pipefail
+
+          hosted_assets=(


[Suggestion] This hardcoded hosted_assets=() array must stay in sync with two other locations: (1) HOSTED_INSTALLATION_ASSETS in build-hosted-installation-assets.js and (2) the for asset in ... loop in the "Verify Aliyun OSS Hosted Installation Assets" step. Adding a new hosted installer to the build script without updating this array means the new file silently never reaches the CDN.

Consider deriving the upload list from the build output (e.g., node scripts/build-hosted-installation-assets.js --list-output-names) instead of hardcoding.

— qwen-latest-series-invite-beta-v34 via Qwen Code /review

wenshao · 2026-05-20T07:20:41Z

+set "QWEN_DOWNLOAD_DEST=%~2"
+rem Prefer curl.exe -# for a hash-mark progress bar (Windows 10+ includes it);
+rem fall back to Invoke-WebRequest (which shows its own progress bar).
+powershell -NoProfile -ExecutionPolicy Bypass -Command "$ErrorActionPreference = 'Stop'; $curl = $env:QWEN_INSTALL_CURL_EXE; if ([string]::IsNullOrEmpty($curl)) { $cmd = Get-Command curl.exe -CommandType Application -ErrorAction SilentlyContinue | Select-Object -First 1; if ($null -ne $cmd) { $curl = $cmd.Source } }; if (-not [string]::IsNullOrEmpty($curl)) { & $curl --connect-timeout 15 --max-time 300 --retry 2 -#fSLo $env:QWEN_DOWNLOAD_DEST $env:QWEN_DOWNLOAD_URL; if ($LASTEXITCODE -ne 0) { throw ('curl.exe download failed (exit code ' + $LASTEXITCODE + ')') }; exit 0 }; try { try { [Net.ServicePointManager]::SecurityProtocol = [Net.SecurityProtocolType]::Tls12 -bor [Net.SecurityProtocolType]::Tls13 } catch { [Net.ServicePointManager]::SecurityProtocol = [Net.SecurityProtocolType]::Tls12 }; Invoke-WebRequest -Uri $env:QWEN_DOWNLOAD_URL -OutFile $env:QWEN_DOWNLOAD_DEST -UseBasicParsing -MaximumRedirection 10 -TimeoutSec 300; exit 0 } catch { [Console]::Error.WriteLine('Download error: ' + $_.Exception.Message); exit 1 }"


[Suggestion] The .bat installer spawns PowerShell as a subprocess ~10-12 times during a typical installation (ValidateRawEnvironmentOptions, ValidateOptions, ValidateVersion, RaceMirrorHead, DownloadFile, VerifyChecksum, CreateTempDir, ValidateArchiveContents, MaybeUpdateUserPath, etc.). Each PowerShell process spawn costs 1-3 seconds, adding an estimated 15-30 seconds of overhead. The .sh equivalent uses native bash builtins for most operations.

Consider consolidating validation into a single PowerShell invocation, or writing a temporary .ps1 that performs all validation/download/checksum in one process.

— qwen-latest-series-invite-beta-v34 via Qwen Code /review

wenshao · 2026-05-20T07:23:04Z

+            fi
+            ;;
+        *.tar.gz|*.tgz|*.tar.xz)
+            if ! entries=$(tar -tf "${archive_path}"); then


[Critical] tar -tf does not show symlink targets (both GNU tar and macOS bsdtar omit the -> target portion), so validate_archive_entry_path never sees symlink entries and cannot reject them. Then at line 936, tar -xzf follows symlinks during extraction on Linux (GNU tar default). A crafted archive with a symlink entry (e.g., qwen-code/evil -> /etc) followed by a regular file entry traversing that symlink (e.g., qwen-code/evil/cron.d/backdoor) achieves arbitrary file write before the post-extraction find -type l check at line 948 can detect it. This is a textbook tar-symlink vulnerability (same root cause as CVEs in pip, npm tar, etc.). macOS is safe because bsdtar refuses to extract through symlinks.

Fix: use tar -tvf (verbose listing shows -> target) to reject any symlink entries before extraction:

Suggested change

if ! entries=$(tar -tf "${archive_path}"); then

*.tar.gz|*.tgz|*.tar.xz)

if tar -tvf "${archive_path}" 2>/dev/null | grep -q ' -> '; then

log_error "Archive contains symlinks; refusing to install."

return 1

fi

if ! entries=$(tar -tf "${archive_path}"); then

log_error "Failed to inspect archive entries: ${archive_path}"

return 1

fi

;;

— qwen-latest-series-invite-beta-v34 via Qwen Code /review

wenshao · 2026-05-20T07:23:06Z

+            --config "${RUNNER_TEMP}/.ossutilconfig" \
+            --prefix "installation/${RELEASE_TAG}" \
+            "${hosted_assets[@]}"
+          node scripts/upload-aliyun-oss-assets.js \


[Critical] This ossutil cp call bypasses the retry wrapper (upload-aliyun-oss-assets.js), unlike every other OSS upload in the workflow. If it fails transiently after gh release create has already succeeded, re-running the workflow hits a duplicate-tag error — the workflow cannot make progress without manual intervention (delete the GitHub release or manually upload VERSION).

Route through upload-aliyun-oss-assets.js or replicate the retry loop inline.

— qwen-latest-series-invite-beta-v34 via Qwen Code /review

wenshao · 2026-05-20T07:23:08Z

+      { stdio: 'inherit' },
+    );
+
+    if (result.error) {


[Critical] The result.error branch (ossutil missing from PATH, fork failure, etc.) is never exercised in tests. The test shim always spawns a Node process that exits cleanly (status 0 or 1), so spawnSync never populates result.error. If ossutil is absent in CI, the error propagation behavior (throwing without retry) is unverified.

Add a test where ossutilCommand points to a nonexistent binary and assert the thrown error matches ENOENT without retrying.

— qwen-latest-series-invite-beta-v34 via Qwen Code /review

wenshao · 2026-05-20T07:23:09Z

+  config,
+  { ossutilCommand, ossutilCommandArgs },
+) {
+  for (let attempt = 1; attempt <= MAX_UPLOAD_ATTEMPTS; attempt += 1) {


[Critical] The retry logic only tests two scenarios: success on first attempt, and failure after all 3 attempts. The transient-failure-then-success path (attempt 1 fails, attempt 2 succeeds) — the core value of the retry mechanism — is never tested. A regression in the retry loop (wrong backoff, off-by-one, missing return after success) would go undetected.

Add a test with an ossutil shim that tracks invocation count in a file and exits 1 on the first call, 0 on subsequent calls.

— qwen-latest-series-invite-beta-v34 via Qwen Code /review

wenshao · 2026-05-20T07:23:11Z

+          set -euo pipefail
+
+          printf '%s\n' "${RELEASE_TAG}" > "${RUNNER_TEMP}/qwen-code-latest-version"
+          ossutil cp "${RUNNER_TEMP}/qwen-code-latest-version" "oss://${ALIYUN_OSS_BUCKET}/releases/qwen-code/latest/VERSION" -c "${RUNNER_TEMP}/.ossutilconfig" -f --acl public-read


[Critical] If this second upload (global installation/ prefix) fails partway through, users get a mix of old archives and new SHA256SUMS (or vice versa), causing checksum verification failures for anyone running irm .../installation/install-qwen-standalone.ps1 | iex. The versioned prefix is safe, but the global alias is not atomic.

Fix: reorder the hosted_assets array so SHA256SUMS is first (not last), ensuring checksums are uploaded before archives. A partial failure then leaves old-but-consistent state (new SHA256SUMS referencing new archives that weren't uploaded yet — old archives still match old SHA256SUMS).

— qwen-latest-series-invite-beta-v34 via Qwen Code /review

wenshao · 2026-05-20T07:23:12Z

            PRERELEASE_FLAG="--prerelease"
          fi

+          mapfile -t release_assets < <(node scripts/verify-installation-release.js --dir dist/standalone --list-release-asset-paths)


[Critical] This ossutil cp call bypasses the retry wrapper (upload-aliyun-oss-assets.js), unlike every other OSS upload in the workflow. If it fails transiently after gh release create has already succeeded, re-running the workflow hits a duplicate-tag error — the workflow cannot make progress without manual intervention (delete the GitHub release or manually upload VERSION).

Route through upload-aliyun-oss-assets.js or replicate the retry loop inline.

— qwen-latest-series-invite-beta-v34 via Qwen Code /review

wenshao · 2026-05-20T07:23:14Z

+)
+
+set "EXPECTED_HASH="
+for /f "usebackq tokens=1,2" %%H in ("!CHECKSUM_FILE!") do (


[Suggestion] The for /f loop iterates all lines in SHA256SUMS and keeps the first match (if "!EXPECTED_HASH!"=="" guard). While "first match wins" is safer than "last match wins", the loop doesn't break early. Batch for /f doesn't support break, but you can goto out of the loop after finding the first match to avoid processing remaining lines.

— qwen-latest-series-invite-beta-v34 via Qwen Code /review

wenshao · 2026-05-20T07:23:16Z

+function fetchWithTimeout(fetchImpl, url, options = {}) {
+  return fetchImpl(url, {
+    ...options,
+    signal: AbortSignal.timeout(REMOTE_FETCH_TIMEOUT_MS),


[Suggestion] AbortSignal.timeout(30_000) covers the entire request lifecycle: DNS, TCP, TLS, headers, and body consumption. In fetchSha256, the body is a 50-150MB archive streamed through pipeline(). If the total download exceeds 30s (slow CDN, rate limiting), the abort fires mid-stream and the error is reported as "unavailable release asset" rather than a timeout.

Consider increasing REMOTE_FETCH_TIMEOUT_MS to 300_000 (matching curl --max-time 300 used elsewhere) or applying a separate timeout for body streaming.

— qwen-latest-series-invite-beta-v34 via Qwen Code /review

wenshao · 2026-05-20T07:23:17Z

+        run: |-
+          set -euo pipefail
+
+          hosted_assets=(


[Critical] If this second upload (global installation/ prefix) fails partway through, users get a mix of old archives and new SHA256SUMS (or vice versa), causing checksum verification failures for anyone running irm .../installation/install-qwen-standalone.ps1 | iex. The versioned prefix is safe, but the global alias is not atomic.

Fix: reorder the hosted_assets array so SHA256SUMS is first (not last), ensuring checksums are uploaded before archives. A partial failure then leaves old-but-consistent state (new SHA256SUMS referencing new archives that weren't uploaded yet — old archives still match old SHA256SUMS).

— qwen-latest-series-invite-beta-v34 via Qwen Code /review

wenshao · 2026-05-20T07:23:19Z

+        return 0
+    fi
+
+    if [[ "${SOURCE}" =~ ^[A-Za-z0-9._-]+$ ]]; then


[Critical] tar -tf does not show symlink targets (both GNU tar and macOS bsdtar omit the -> target portion), so validate_archive_entry_path never sees symlink entries and cannot reject them. Then at line 936, tar -xzf follows symlinks during extraction on Linux (GNU tar default). A crafted archive with a symlink entry (e.g., qwen-code/evil -> /etc) followed by a regular file entry traversing that symlink (e.g., qwen-code/evil/cron.d/backdoor) achieves arbitrary file write before the post-extraction find -type l check at line 948 can detect it. This is a textbook tar-symlink vulnerability (same root cause as CVEs in pip, npm tar, etc.). macOS is safe because bsdtar refuses to extract through symlinks.

Fix: use tar -tvf (verbose listing shows -> target) to reject any symlink entries before extraction:

Suggested change

if [[ "${SOURCE}" =~ ^[A-Za-z0-9._-]+$ ]]; then

*.tar.gz|*.tgz|*.tar.xz)

if tar -tvf "${archive_path}" 2>/dev/null | grep -q ' -> '; then

log_error "Archive contains symlinks; refusing to install."

return 1

fi

if ! entries=$(tar -tf "${archive_path}"); then

log_error "Failed to inspect archive entries: ${archive_path}"

return 1

fi

;;

— qwen-latest-series-invite-beta-v34 via Qwen Code /review

wenshao · 2026-05-20T07:23:21Z

+    # Manifest format is produced by writeManifest in create-standalone-package.js.
+    # Keep these grep checks in sync if that JSON layout changes.
+    grep -Eq '"name"[[:space:]]*:[[:space:]]*"@qwen-code/qwen-code"' "${manifest_path}" 2>/dev/null || return 1
+    grep -Eq '"target"[[:space:]]*:[[:space:]]*"(darwin|linux)-(arm64|x64)"' "${manifest_path}" 2>/dev/null || return 1


[Suggestion] Cross-platform inconsistency: this regex (^[A-Za-z0-9._-]+$) allows digits, underscores, or dashes as the first character, but the .bat equivalent at line 460 requires ^[A-Za-z][A-Za-z0-9._-]*$ (letter first). A source value like 123build or _internal passes on Linux/macOS but fails on Windows.

Align the .bat regex to match, or tighten the .sh to require a letter first.

— qwen-latest-series-invite-beta-v34 via Qwen Code /review

wenshao · 2026-05-20T07:23:23Z

+set "QWEN_DOWNLOAD_DEST=%~2"
+rem Prefer curl.exe -# for a hash-mark progress bar (Windows 10+ includes it);
+rem fall back to Invoke-WebRequest (which shows its own progress bar).
+powershell -NoProfile -ExecutionPolicy Bypass -Command "$ErrorActionPreference = 'Stop'; $curl = $env:QWEN_INSTALL_CURL_EXE; if ([string]::IsNullOrEmpty($curl)) { $cmd = Get-Command curl.exe -CommandType Application -ErrorAction SilentlyContinue | Select-Object -First 1; if ($null -ne $cmd) { $curl = $cmd.Source } }; if (-not [string]::IsNullOrEmpty($curl)) { & $curl --connect-timeout 15 --max-time 300 --retry 2 -#fSLo $env:QWEN_DOWNLOAD_DEST $env:QWEN_DOWNLOAD_URL; if ($LASTEXITCODE -ne 0) { throw ('curl.exe download failed (exit code ' + $LASTEXITCODE + ')') }; exit 0 }; try { try { [Net.ServicePointManager]::SecurityProtocol = [Net.SecurityProtocolType]::Tls12 -bor [Net.SecurityProtocolType]::Tls13 } catch { [Net.ServicePointManager]::SecurityProtocol = [Net.SecurityProtocolType]::Tls12 }; Invoke-WebRequest -Uri $env:QWEN_DOWNLOAD_URL -OutFile $env:QWEN_DOWNLOAD_DEST -UseBasicParsing -MaximumRedirection 10 -TimeoutSec 300; exit 0 } catch { [Console]::Error.WriteLine('Download error: ' + $_.Exception.Message); exit 1 }"


[Suggestion] The .bat installer spawns PowerShell as a subprocess ~10-12 times during a typical installation (ValidateRawEnvironmentOptions, ValidateOptions, ValidateVersion, RaceMirrorHead, DownloadFile, VerifyChecksum, CreateTempDir, ValidateArchiveContents, MaybeUpdateUserPath, etc.). Each PowerShell process spawn costs 1-3 seconds, adding an estimated 15-30 seconds of overhead. The .sh equivalent uses native bash builtins for most operations.

Consider consolidating validation into a single PowerShell invocation, or writing a temporary .ps1 that performs all validation/download/checksum in one process.

— qwen-latest-series-invite-beta-v34 via Qwen Code /review

wenshao · 2026-05-20T07:25:32Z

+        return 0
+    fi
+
+    if [[ "${SOURCE}" =~ ^[A-Za-z0-9._-]+$ ]]; then


[Suggestion] Cross-platform inconsistency: this regex (^[A-Za-z0-9._-]+$) allows digits, underscores, or dashes as the first character, but the .bat equivalent at line 460 requires ^[A-Za-z][A-Za-z0-9._-]*$ (letter first). A source value like 123build or _internal passes on Linux/macOS but fails on Windows.

Align the .bat regex to match, or tighten the .sh to require a letter first.

— qwen-latest-series-invite-beta-v34 via Qwen Code /review

wenshao · 2026-05-20T07:25:33Z

+    # Manifest format is produced by writeManifest in create-standalone-package.js.
+    # Keep these grep checks in sync if that JSON layout changes.
+    grep -Eq '"name"[[:space:]]*:[[:space:]]*"@qwen-code/qwen-code"' "${manifest_path}" 2>/dev/null || return 1
+    grep -Eq '"target"[[:space:]]*:[[:space:]]*"(darwin|linux)-(arm64|x64)"' "${manifest_path}" 2>/dev/null || return 1


[Suggestion] The manifest target regex (darwin|linux)-(arm64|x64) is duplicated across 4 files: install-qwen-standalone.sh, uninstall-qwen-standalone.sh, install-qwen-standalone.bat, and uninstall-qwen-standalone.ps1. When RELEASE_TARGETS in build-standalone-release.js gains a new platform (e.g., linux-riscv64), all four regexes must be updated independently with no build-time check linking them.

Add a comment near RELEASE_TARGETS listing the coupled locations, or add a test asserting the regex patterns match RELEASE_TARGETS.

— qwen-latest-series-invite-beta-v34 via Qwen Code /review

wenshao · 2026-05-20T07:25:36Z

+            --config "${RUNNER_TEMP}/.ossutilconfig" \
+            --prefix "installation/${RELEASE_TAG}" \
+            "${hosted_assets[@]}"
+          node scripts/upload-aliyun-oss-assets.js \


[Critical] If this second upload (global installation/ prefix) fails partway through, users get a mix of old archives and new SHA256SUMS (or vice versa), causing checksum verification failures for anyone running irm .../installation/install-qwen-standalone.ps1 | iex. The versioned prefix is safe, but the global alias is not atomic.

Fix: reorder the hosted_assets array so SHA256SUMS is first (not last), ensuring checksums are uploaded before archives. A partial failure then leaves old-but-consistent state (new SHA256SUMS referencing new archives that weren't uploaded yet — old archives still match old SHA256SUMS).

— qwen-latest-series-invite-beta-v34 via Qwen Code /review

wenshao · 2026-05-20T07:25:37Z

+          set -euo pipefail
+
+          printf '%s\n' "${RELEASE_TAG}" > "${RUNNER_TEMP}/qwen-code-latest-version"
+          ossutil cp "${RUNNER_TEMP}/qwen-code-latest-version" "oss://${ALIYUN_OSS_BUCKET}/releases/qwen-code/latest/VERSION" -c "${RUNNER_TEMP}/.ossutilconfig" -f --acl public-read


[Critical] This ossutil cp call bypasses the retry wrapper (upload-aliyun-oss-assets.js), unlike every other OSS upload in the workflow. If it fails transiently after gh release create has already succeeded, re-running the workflow fails with a duplicate-tag error — the workflow cannot make progress without manual intervention (delete the GitHub release or manually upload VERSION).

Route through upload-aliyun-oss-assets.js or replicate the retry loop inline.

— qwen-latest-series-invite-beta-v34 via Qwen Code /review

…ection [skip ci] 1. The hosted installation asset build now accepts --version and stamps it into the copied .sh/.bat installers so they default to the tagged release version instead of 'latest'. The release workflow passes the version. 2. install-qwen-with-source.bat now validates archive entries before calling Expand-Archive, rejecting paths with '..', leading '/', drive-rooted paths, empty names, or control characters — matching the protection already present in install-qwen-standalone.bat and the .sh installer.

…[skip ci] The SOURCE variable is user-provided and used in path operations but was not included in the :ValidateOptions unsafe-character check. Add it alongside the other validated variables.

wenshao · 2026-05-21T01:08:24Z

+        ) {
+            Add-PathCandidate -Candidates $candidates -Directory $entry
+        }
+    }


[Critical] The PowerShell shim's checksum verification (download SHA256SUMS, parse hash, Get-FileHash comparison, abort-on-mismatch) is the security boundary for irm ... | iex — the primary Windows hosted install entrypoint. However, it is only verified by toContain string assertions in scripts/tests/install-script.test.js (lines ~794-812). No E2E test executes the ps1 against a real or mock HTTP server serving a .bat + SHA256SUMS pair.

The .bat installer's checksum verification IS tested E2E (tampered archive test), but the equivalent ps1 path is not. A regression in the checksum parsing regex ('^([0-9a-fA-F]{64})\\s+\\*?(.+)$'), hash comparison (.ToLowerInvariant()), or cleanup-on-failure logic would silently bypass the security boundary.

Consider adding an itOnWindows E2E test that starts a local HTTPS server, serves a .bat + SHA256SUMS, and runs the ps1 shim — testing both the happy path (checksum match) and failure path (tampered .bat, shim exits non-zero without executing).

— qwen-latest-series-invite-beta-v34 via Qwen Code /review

wenshao · 2026-05-21T01:08:24Z

+        $preferredDirectories += Join-Path $env:USERPROFILE '.bun\bin'
+    }
+
+    foreach ($preferredDirectory in $preferredDirectories) {


[Critical] The HTTPS-only guard ($env:QWEN_INSTALLER_BAT_URL -notmatch '^https://') is the injection-prevention boundary for the hosted entrypoint. It is verified only by toContain('must start with https://') string assertion — no test passes an http:// URL and verifies the shim actually rejects it with exit 1.

If the regex or exit logic regresses, a misconfigured or malicious QWEN_INSTALLER_BAT_URL=http://attacker.com/... would silently download an unencrypted .bat, enabling MITM attacks on the primary Windows install path.

Consider adding an itOnWindows test that sets QWEN_INSTALLER_BAT_URL=http://localhost/... and asserts the ps1 exits with code 1.

— qwen-latest-series-invite-beta-v34 via Qwen Code /review

wenshao · 2026-05-21T01:08:24Z

+
+    if ($removed) {
+        [Environment]::SetEnvironmentVariable('Path', ($kept -join ';'), 'User')
+        Write-Success "Removed $BinDir from user PATH."


[Critical] Remove-UserPathEntry reads the Windows User PATH from the registry, removes the qwen bin directory, and writes back via [Environment]::SetEnvironmentVariable('Path', ..., 'User'). This is a destructive registry modification that is only tested by static toContain assertions — no E2E test sets a real User PATH, runs the uninstaller, and verifies the registry value afterward.

The Unix equivalent IS tested E2E (shell rc PATH removal tests with .zshrc/.bashrc). A regression in Get-NormalizedPath or Test-PathMatches could corrupt the PATH by removing wrong entries — a registry write that is hard for users to diagnose.

Consider adding an itOnWindows test that: (1) sets User PATH to include the qwen bin dir plus other entries, (2) runs the uninstaller, (3) reads back [Environment]::GetEnvironmentVariable('Path', 'User') and asserts only the qwen entry was removed.

— qwen-latest-series-invite-beta-v34 via Qwen Code /review

wenshao · 2026-05-21T01:08:25Z

+
+    if ! mv -f "${wrapper_tmp}" "${INSTALL_BIN_DIR}/qwen"; then
+        rm -rf "${INSTALL_LIB_DIR}" "${wrapper_tmp}"
+        if [[ -e "${old_install_dir}" ]]; then


[Critical] When wrapper creation fails (mv -f "${wrapper_tmp}" at line 1224), the rollback unconditionally destroys the newly-installed directory (rm -rf "${INSTALL_LIB_DIR}") BEFORE attempting to restore the old install from ${old_install_dir}. If the subsequent mv also fails (permissions, disk error, cross-device rename), the user is left with no installation at all. The error message "Failed to create qwen wrapper" does not mention that the old install may still exist at ${old_install_dir}.

This is a data-loss scenario in a dual-failure path. Consider a safer ordering: rename INSTALL_LIB_DIR to a trash name first, attempt the rollback, and only delete the trash if rollback succeeded. If rollback fails, restore the new install so the user isn't empty-handed:

local trash="${INSTALL_LIB_DIR}.trash.$$" mv "${INSTALL_LIB_DIR}" "${trash}" 2>/dev/null || true if [[ -e "${old_install_dir}" ]]; then if mv "${old_install_dir}" "${INSTALL_LIB_DIR}"; then rm -rf "${trash}" else [[ ! -e "${INSTALL_LIB_DIR}" ]] && mv "${trash}" "${INSTALL_LIB_DIR}" 2>/dev/null rm -rf "${trash}" log_error "Failed to restore previous install from ${old_install_dir}." fi fi

— qwen-latest-series-invite-beta-v34 via Qwen Code /review

wenshao · 2026-05-21T01:08:34Z

+        ) {
+            Add-PathCandidate -Candidates $candidates -Directory $entry
+        }
+    }


[Critical] The PowerShell shim's checksum verification (download SHA256SUMS, parse hash, Get-FileHash comparison, abort-on-mismatch) is the security boundary for irm ... | iex — the primary Windows hosted install entrypoint. However, it is only verified by toContain string assertions in scripts/tests/install-script.test.js (lines ~794-812). No E2E test executes the ps1 against a real or mock HTTP server serving a .bat + SHA256SUMS pair.

The .bat installer's checksum verification IS tested E2E (tampered archive test), but the equivalent ps1 path is not. A regression in the checksum parsing regex ('^([0-9a-fA-F]{64})\\s+\\*?(.+)$'), hash comparison (.ToLowerInvariant()), or cleanup-on-failure logic would silently bypass the security boundary.

Consider adding an itOnWindows E2E test that starts a local HTTPS server, serves a .bat + SHA256SUMS, and runs the ps1 shim — testing both the happy path (checksum match) and failure path (tampered .bat, shim exits non-zero without executing).

— qwen-latest-series-invite-beta-v34 via Qwen Code /review

wenshao · 2026-05-21T01:08:35Z

+        $preferredDirectories += Join-Path $env:USERPROFILE '.bun\bin'
+    }
+
+    foreach ($preferredDirectory in $preferredDirectories) {


[Critical] The HTTPS-only guard ($env:QWEN_INSTALLER_BAT_URL -notmatch '^https://') is the injection-prevention boundary for the hosted entrypoint. It is verified only by toContain('must start with https://') string assertion — no test passes an http:// URL and verifies the shim actually rejects it with exit 1.

If the regex or exit logic regresses, a misconfigured or malicious QWEN_INSTALLER_BAT_URL=http://attacker.com/... would silently download an unencrypted .bat, enabling MITM attacks on the primary Windows install path.

Consider adding an itOnWindows test that sets QWEN_INSTALLER_BAT_URL=http://localhost/... and asserts the ps1 exits with code 1.

— qwen-latest-series-invite-beta-v34 via Qwen Code /review

wenshao · 2026-05-21T01:08:37Z

+
+    if ($removed) {
+        [Environment]::SetEnvironmentVariable('Path', ($kept -join ';'), 'User')
+        Write-Success "Removed $BinDir from user PATH."


[Critical] Remove-UserPathEntry reads the Windows User PATH from the registry, removes the qwen bin directory, and writes back via [Environment]::SetEnvironmentVariable('Path', ..., 'User'). This is a destructive registry modification that is only tested by static toContain assertions — no E2E test sets a real User PATH, runs the uninstaller, and verifies the registry value afterward.

The Unix equivalent IS tested E2E (shell rc PATH removal tests with .zshrc/.bashrc). A regression in Get-NormalizedPath or Test-PathMatches could corrupt the PATH by removing wrong entries — a registry write that is hard for users to diagnose.

Consider adding an itOnWindows test that: (1) sets User PATH to include the qwen bin dir plus other entries, (2) runs the uninstaller, (3) reads back [Environment]::GetEnvironmentVariable('Path', 'User') and asserts only the qwen entry was removed.

— qwen-latest-series-invite-beta-v34 via Qwen Code /review

wenshao · 2026-05-21T01:08:38Z

+
+    if ! mv -f "${wrapper_tmp}" "${INSTALL_BIN_DIR}/qwen"; then
+        rm -rf "${INSTALL_LIB_DIR}" "${wrapper_tmp}"
+        if [[ -e "${old_install_dir}" ]]; then


[Critical] When wrapper creation fails (mv -f "${wrapper_tmp}" at line 1224), the rollback unconditionally destroys the newly-installed directory (rm -rf "${INSTALL_LIB_DIR}") BEFORE attempting to restore the old install from ${old_install_dir}. If the subsequent mv also fails (permissions, disk error, cross-device rename), the user is left with no installation at all. The error message "Failed to create qwen wrapper" does not mention that the old install may still exist at ${old_install_dir}.

This is a data-loss scenario in a dual-failure path. Consider a safer ordering: rename INSTALL_LIB_DIR to a trash name first, attempt the rollback, and only delete the trash if rollback succeeded. If rollback fails, restore the new install so the user isn't empty-handed:

local trash="${INSTALL_LIB_DIR}.trash.$$" mv "${INSTALL_LIB_DIR}" "${trash}" 2>/dev/null || true if [[ -e "${old_install_dir}" ]]; then if mv "${old_install_dir}" "${INSTALL_LIB_DIR}"; then rm -rf "${trash}" else [[ ! -e "${INSTALL_LIB_DIR}" ]] && mv "${trash}" "${INSTALL_LIB_DIR}" 2>/dev/null rm -rf "${trash}" log_error "Failed to restore previous install from ${old_install_dir}." fi fi

— qwen-latest-series-invite-beta-v34 via Qwen Code /review

DennisYu07

lgtm

yiliang114 · 2026-05-21T03:40:06Z

Bot Review Critical 评论统一回复

以下是对本 PR 中 bot（gpt-5.5 via /review）标记的所有 Critical 评论的逐一核实。大部分是误报或风险被过度放大。

误报（代码中已处理，bot 未正确识别）

评论	为什么是误报
"hosted 安装脚本未 bake version，默认安装 latest 而非指定版本"	`build-hosted-installation-assets.js` 有明确的 version 替换逻辑（`VERSION="${QWEN_INSTALL_VERSION:-latest}"` → `VERSION="${QWEN_INSTALL_VERSION:-${version}}"`），workflow 传了 `--version "${RELEASE_VERSION}"`
".bat 缺少 Zip Slip 路径遍历防护"	PR 已新增 `:ValidateArchiveContents` 函数，在 `Expand-Archive` 之前做解压前路径遍历检查
"`latest/VERSION` 在 hosted sync 之前发布，导致不一致"	实际 workflow 中 `Publish Aliyun OSS Latest VERSION` 是最后一步（注释明确 "Run last"），在所有上传和验证之后执行
"hosted assets 只上传到版本化路径，全局路径 404"	workflow 有两次 `upload-aliyun-oss-assets.js` 调用：`--prefix "installation/${RELEASE_TAG}"`（版本化）和 `--prefix "installation"`（全局）
"OSS 凭证写入 .ossutilconfig 无清理步骤"	workflow 有 `Cleanup Aliyun OSS Credentials` 步骤（`always()` 条件守护）
"`gh release create` 还在用 hardcoded glob，新平台会遗漏"	已改为 `verify-installation-release.js --list-release-asset-paths` 动态枚举，不再使用 glob
"`sudo install ossutil` 存在脆弱的 sudo 依赖"	实际是 `install -m 0755` 到 `$HOME/.local/bin`，没有 sudo
"upload-aliyun-oss-assets.js 无重试逻辑"	文件中已有 `uploadWithRetry` 实现

风险过度放大（理论可能但实际影响极低）

评论	实际风险评估
tar symlink 攻击（Linux）	archive 由自身 CI 构建，下载经 SHA256 校验。攻击者需同时控制 OSS + 绕过 checksum，威胁模型不成立
HEAD probe 不可靠导致 fallback npm	探测的是自有 OSS endpoint，HEAD 方法支持稳定，非第三方 CDN
全局 `installation/` 上传非原子	6 个小文件（脚本+SHA256SUMS）上传耗时 <1s，不一致窗口几乎为零
`win-arm64` 不在 RELEASE_TARGETS	当前不发布 arm64 archive，检测代码预留了 fallback to npm 路径，用户不会卡死
`parseSha256Sums` 严格解析会打断流水线	解析的是自己生成的 SHA256SUMS（格式完全可控），不是 nodejs.org 的文件
TOCTOU: checksum 验证与执行之间被替换	攻击者需在同一 runner 上同一秒内精确替换文件，GitHub Actions 隔离环境下不现实
`SharedArrayBuffer` 不可用导致上传崩溃	Node 22 + GitHub Actions Linux runner 完全支持，不存在此问题
`ossutil cp` VERSION 绕过重试 wrapper	这是单个 <100 byte 文件写入，失败概率极低，且有前置验证确保 OSS 连通性

可作为 Follow-up 的合理建议（非 blocker）

download_text() 补充 --connect-timeout / --max-time（与 download_file 对齐）
新 .bat 的 endlocal & set PATH 延迟展开语义确认
PowerShell / bat 脚本后续补充 E2E 测试覆盖
checksum 失败时输出 expected vs actual hash 值便于调试

以上均不影响现有安装脚本和发布流程的正确性，不阻塞合并。

pomelo-nwu

LGTM

refer：#3828 (comment)

yiliang114 added 17 commits April 30, 2026 22:04

feat(installer): add standalone archive installation

eb2a9a8

fix(installer): harden standalone archive installs

d2e5b1b

fix(installer): address standalone review findings

d7277a6

chore(installer): clarify review followups

5719595

fix(installer): stabilize standalone script checks

472e51e

chore(installer): remove internal planning docs

47fdfbc

chore(installer): simplify standalone release review fixes

2b40bad

test(installer): add Windows batch install smoke

a4a15e1

test(installer): fix Windows batch smoke quoting

fee13e4

test(installer): preserve Windows cmd quotes

69e22f9

fix(installer): use robust Windows checksum hashing

faa9087

ci: narrow installer debug matrix

66dec5f

fix(installer): address standalone review hardening

1ca86fe

fix(installer): avoid Windows validation parse errors

0eb58a8

fix(installer): simplify Windows option validation

e7e3f90

fix(installer): harden standalone review fixes

fee51d1

feat(installer): publish release installer assets

ec654dd

wenshao reviewed May 4, 2026

View reviewed changes

Comment thread scripts/create-standalone-package.js Outdated

Comment thread scripts/build-installation-assets.js Outdated

Comment thread scripts/create-standalone-package.js Outdated

Comment thread scripts/build-installation-assets.js Outdated

fix(installer): address release asset review feedback

ccf83e9

wenshao requested changes May 5, 2026

View reviewed changes

Comment thread scripts/tests/install-script.test.js

yiliang114 added 2 commits May 5, 2026 19:34

fix(installer): avoid prerelease installer asset links

b078ade

test(installer): isolate standalone dist fixture

668c006

yiliang114 mentioned this pull request May 5, 2026

[Depends on #3828] feat(installer): stage hosted install assets #3853

Merged

feat(installer): add hosted install release alias

a205e6c

yiliang114 marked this pull request as ready for review May 5, 2026 12:52

yiliang114 requested a review from wenshao May 5, 2026 12:53

Copilot started work on behalf of yiliang114 May 5, 2026 13:11 View session

chore: no changes - code review requested

bbaa8ff

Agent-Logs-Url: https://github.com/QwenLM/qwen-code/sessions/38467aec-15b9-4b76-9139-0b2cfe40477a

yiliang114 changed the base branch from codex/installer-standalone to main May 5, 2026 13:21

wenshao requested changes May 20, 2026

View reviewed changes

wenshao reviewed May 20, 2026

View reviewed changes

yiliang114 added 3 commits May 21, 2026 01:37

fix(installer): add SOURCE to PowerShell unsafe-character validation …

cb3f356

…[skip ci] The SOURCE variable is user-provided and used in path operations but was not included in the :ValidateOptions unsafe-character check. Add it alongside the other validated variables.

wenshao previously requested changes May 21, 2026

View reviewed changes

wenshao reviewed May 21, 2026

View reviewed changes

DennisYu07 approved these changes May 21, 2026

View reviewed changes

pomelo-nwu approved these changes May 21, 2026

View reviewed changes

pomelo-nwu merged commit d59c9e7 into main May 21, 2026
15 checks passed

Conversation

yiliang114 commented May 4, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Validation

Scope / Risk

Testing Matrix

Linked Issues / Bugs

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

wenshao left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

yiliang114 commented May 5, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

yiliang114 commented May 4, 2026 •

edited

Loading