feat(extensions): support archive install sources - #4909
Conversation
6e17c47 to
f2e4fff
Compare
kkhomej33-netizen
changed the title
f2e4fff to
52a55bd
Compare
wenshao
left a comment
wenshao
left a comment
There was a problem hiding this comment.
Test coverage gap: downloadFile was substantially rewritten (HTTP transport selection, 307/308 redirect support, includeGitHubToken option, relative redirect resolution) but has no direct unit tests. Existing tests reach it only indirectly with statusCode: 200 mock responses — the HTTP transport branch, redirect codes, includeGitHubToken: false path, and relative URL resolution are all unreachable from current tests. Consider adding direct tests for these new code paths.
— qwen3.7-max via Qwen Code /review
| } finally { | ||
| if (tempDir) { | ||
| await fs.promises.rm(tempDir, { recursive: true, force: true }); | ||
| } |
There was a problem hiding this comment.
[Critical] checkForExtensionUpdate has no branch for the new archive-url install type. The guard at line 251 (installMetadata.type !== 'git' && installMetadata.type !== 'github-release') returns NOT_UPDATABLE for archive-url extensions. The documentation added by this PR states "Archive URLs can be updated later", but update detection will never fire for them.
Add an archive-url branch that re-downloads the archive to a temp directory, loads the config, and compares versions — similar to the local-archive branch above. Also update the guard to include installMetadata.type !== 'archive-url'.
— qwen3.7-max via Qwen Code /review
There was a problem hiding this comment.
Addressed in a352e09: added an archive-url update branch that downloads the archive into a temp directory, loads the extracted config, compares versions, and cleans up the temp directory. Added UPDATE_AVAILABLE and UP_TO_DATE tests for archive-url update checks.
| return; | ||
| } | ||
| const redirectUrl = new URL(res.headers.location, url).toString(); | ||
| downloadFile(redirectUrl, dest, options).then(resolve).catch(reject); |
There was a problem hiding this comment.
[Critical] downloadFile forwards the Authorization: token <GITHUB_TOKEN> header to all redirect targets without checking if the redirect host differs from the original. This leaks the GitHub PAT to arbitrary third-party servers on cross-host redirects. The PR also expanded redirect codes (307/308) and added http: transport, widening the leakage surface.
The codebase already implements the correct pattern in npm.ts (fetchNpmJson / downloadNpmFile):
const originalHost = new URL(url).host;
const redirectHost = new URL(redirectUrl).host;
const redirectOptions = redirectHost === originalHost
? options
: { ...options, includeGitHubToken: false };
downloadFile(redirectUrl, dest, redirectOptions).then(resolve).catch(reject);Also consider adding a redirectCount parameter to prevent unbounded recursion on redirect loops.
— qwen3.7-max via Qwen Code /review
There was a problem hiding this comment.
Addressed in a352e09: cross-host redirects now continue with includeGitHubToken: false, same-host redirects preserve the existing options, and redirect recursion is capped. I also removed HTTP archive downloads and restricted archive URL installs to HTTPS only, which resolves the CodeQL alert.
| `Failed to download archive from ${redactUrlCredentials(installMetadata.source)}: ${redactUrlCredentials(getErrorMessage(error))}`, | ||
| ); | ||
| } | ||
| } |
There was a problem hiding this comment.
[Suggestion] The catch block wraps all errors — including post-download extraction failures — as "Failed to download archive from ...". When download succeeds but extraction fails (corrupt archive, missing manifest), the error prefix is misleading.
Consider separating download from extraction errors:
try {
await downloadFile(installMetadata.source, downloadedAssetPath, {
includeGitHubToken: false,
});
} catch (error) {
throw new Error(
`Failed to download archive from ${redactUrlCredentials(installMetadata.source)}: ${getErrorMessage(error)}`,
);
}
await extractArchiveFile(downloadedAssetPath, destination);
await fs.promises.unlink(downloadedAssetPath);— qwen3.7-max via Qwen Code /review
There was a problem hiding this comment.
Addressed in a352e09: downloadFromArchiveUrl now wraps only download failures with the download-specific prefix. Extraction and manifest validation errors are left as extraction/archive errors so corrupt archives report the clearer message.
| ); | ||
| }); | ||
| }); | ||
|
|
There was a problem hiding this comment.
[Suggestion] flattenSingleExtensionDirectory has three guard branches that are never tested: (1) entries.length > 2 → no flatten, (2) no directory among entries → no flatten, (3) lonely dir without a supported manifest → no flatten. Only the flattening-happens path is exercised indirectly.
Consider adding tests for each guard:
- Archive with 3+ top-level entries (files stay in place)
- Archive with only files, no directory (no flatten)
- Archive with one dir + one file, but dir has no manifest (no flatten)
— qwen3.7-max via Qwen Code /review
There was a problem hiding this comment.
Addressed in a352e09: added coverage for the flatten guards using extractArchiveFile behavior: multiple top-level entries, files-only archives, and a top-level directory without a supported manifest.
9203c34 to
a352e09
Compare
|
Updated in a352e09 to address the review feedback: added archive-url update detection with tests, added direct coverage for 307/308 redirects, relative redirects, no-token archive downloads, cross-host redirect token stripping, redirect loop limits, and flatten guard behavior. Also split archive download errors from extraction/manifest errors and restricted archive URL installs to HTTPS to satisfy CodeQL. |
LaZzyMan
left a comment
LaZzyMan
left a comment
There was a problem hiding this comment.
Review summary
Overall this is a well-built PR — it reuses the existing extraction/validation/flatten pipeline, the refactor is clean, test coverage is broad, and there's a nice security touch (remote downloads deliberately omit the GitHub token). I'm requesting changes for one functional bug: remote archive (archive-url) extensions are advertised as updatable, but the update path never runs — and it's invisible to CI because no test exercises it.
🔴 Blocking
Remote archive (archive-url) updates never run — and the CLI falsely reports "already up to date"
The PR wires up the archive-url install path, and on the update side it:
- adds
archive-urlto the autoUpdate allowlist ininstall.ts, - documents in
introduction.mdthat "Archive URLs can be updated later…" and lists "an archive URL" under "you can explicitly update to the latest version", - states in the description that "Remote archive installs should be updatable by replaying the stored URL."
But checkForExtensionUpdate has no archive-url branch. An archive-url install is none of local/npm/git/github-release, so it hits the fall-through and returns NOT_UPDATABLE:
// github.ts — checkForExtensionUpdate
if (
!installMetadata ||
installMetadata.originSource === 'Claude' ||
(installMetadata.type !== 'git' && installMetadata.type !== 'github-release')
) {
return ExtensionUpdateState.NOT_UPDATABLE;
}Both update entry points gate strictly on UPDATE_AVAILABLE, so NOT_UPDATABLE means the update is never performed:
- Manual
qwen extensions update <name>—update.ts:61:if (updateState !== ExtensionUpdateState.UPDATE_AVAILABLE) { …"already up to date"; return; }→ never re-downloads, and the "up to date" message is actively misleading. - autoUpdate —
useExtensionUpdates.ts:266:status !== ExtensionUpdateState.UPDATE_AVAILABLE → continue→ silently skipped (even thoughinstall.tslet the user set--auto-updateon it). update --all—updateAllUpdatableExtensionsfilters onUPDATE_AVAILABLE→ skipped.
The strongest tell that this is an oversight rather than intent: the PR does add archive update support for local archives (the new isSupportedArchivePath → extract-to-tmp → loadExtensionConfig block inside the local branch of checkForExtensionUpdate), but the remote archive case was left out.
Suggested fix: add an archive-url branch to checkForExtensionUpdate mirroring the local-archive logic — downloadFromArchiveUrl into a tmp dir, loadExtensionConfig, compare versions, return the real state, clean up in finally — plus a test covering the archive-url update path so it can't regress silently.
🟡 Security (non-blocking, but worth a reply)
- Plaintext HTTP install of executable extension content.
isSupportedArchiveUrlallowshttp:, anddownloadFilegained an http transport (it was https-only before). Installing an extension pulls in executable content (commands/skills/agents), so a plaintext fetch is MITM-tamperable. Consider restricting archive-url tohttps, or at least warning onhttp. - Redirect handling.
downloadFilefollows 301/302/307/308 with no hop limit (a redirect loop can hang it) and can downgrade https→http or follow into internal addresses. Lower risk here since archive-url carries no token, but it compounds with (1). A max-redirect cap would be good.
✅ What looks good
- Remote download passes
includeGitHubToken: false— avoids leaking the GitHub token to arbitrary third-party hosts. Nice and easy to miss. downloadFileredirect fixes: 307/308 support, relativeLocationresolution vianew URL(location, url), and an explicit missing-Locationcheck.- Clean refactor extracting
flattenSingleExtensionDirectory+hasSupportedExtensionSourceManifest, reused by both the GitHub-release and archive paths. - Clear error messages ("missing qwen-extension.json…", "Make sure it is a valid .zip or .tar.gz file").
- zip-slip is covered by the libraries (extract-zip 2.0.1's "Out of bound path" check, node-tar's default
..stripping); tmpdir cleanup viafinallyis correct; install-source precedence (local path > archive-url > git) is right. - Broad test coverage across install / extensionManager / github / marketplace.
🟢 Nits
archive-urlis matched before git inparseInstallSource(you flagged this under Risk) — fine in practice, git remotes rarely end in an archive suffix.downloadFromArchiveUrlparsesnew URL(installMetadata.source)twice; could cache it.- The flatten path for archives (archive written into
destination, then extracted into the same dir, then flattened) has no real integration test —extensionManagertests mockextractArchiveFile, andgithub.testuses a root-level manifest, so the single-top-level-dir flatten case for archives is untested.
Thanks for the PR — solid work overall; the only real blocker is the remote-archive update gap.
| const redirectOptions = | ||
| redirectHost === parsedUrl.host | ||
| ? options | ||
| : { ...options, includeGitHubToken: false }; |
There was a problem hiding this comment.
[Suggestion] The redirect response body is not consumed before the recursive downloadFile call. In Node.js, the response body must be consumed (e.g., res.resume()) to release the socket back to the connection pool. Without this, each redirect leaks a connection — particularly impactful for 307/308 redirects which typically include a body.
| : { ...options, includeGitHubToken: false }; | |
| res.resume(); | |
| const redirectUrl = new URL(res.headers.location, url).toString(); |
— qwen3.7-plus via Qwen Code /review
There was a problem hiding this comment.
Addressed in d0ff5d0: added res.resume() before following redirects so redirect response bodies are consumed and sockets can be released.
| } | ||
| } | ||
| } | ||
| await flattenSingleExtensionDirectory(destination); |
There was a problem hiding this comment.
[Suggestion] downloadFromArchiveUrl uses the new extractArchiveFile helper (which wraps extraction + flatten + manifest assertion with user-friendly error messages), but downloadFromGitHubRelease still calls extractFile, flattenSingleExtensionDirectory, and assertExtractedArchiveContainsExtensionSource separately. This duplicates the three-step pattern and means a corrupt release archive gets a raw extraction error instead of the friendlier "Make sure it is a valid .zip or .tar.gz file" message.
Consider replacing these three lines with:
| await flattenSingleExtensionDirectory(destination); | |
| await extractArchiveFile(downloadedAssetPath, destination); |
— qwen3.7-plus via Qwen Code /review
There was a problem hiding this comment.
Addressed in d0ff5d0: downloadFromGitHubRelease now uses extractArchiveFile so release downloads share the same extraction, flattening, manifest validation, and friendly extraction error handling as archive-url installs.
| async function downloadFile( | ||
| url: string, | ||
| dest: string, | ||
| options: { includeGitHubToken?: boolean } = { includeGitHubToken: true }, |
There was a problem hiding this comment.
[Suggestion] downloadFile defaults to includeGitHubToken: true, meaning any future caller that omits the options argument will silently send the GITHUB_TOKEN to whatever host the URL points at. Today there are only two callers and the non-GitHub one passes { includeGitHubToken: false } explicitly, so this is not a current bug — but the default is backwards for a function that may send a secret. A function that transmits credentials should default to not sending them.
Consider inverting the default:
| options: { includeGitHubToken?: boolean } = { includeGitHubToken: true }, | |
| options: { includeGitHubToken?: boolean } = { includeGitHubToken: false }, |
and having downloadFromGitHubRelease pass { includeGitHubToken: true } explicitly.
— qwen3.7-plus via Qwen Code /review
There was a problem hiding this comment.
Addressed in d0ff5d0: downloadFile now defaults includeGitHubToken to false, and downloadFromGitHubRelease opts in explicitly with includeGitHubToken: true.
a352e09 to
d0ff5d0
Compare
|
Thanks for the detailed review. The blocking archive-url update gap is addressed in the current head: checkForExtensionUpdate now has an archive-url branch that downloads the stored URL into a temp directory, loads the extracted config, compares versions, and cleans up in finally. The focused coverage includes UPDATE_AVAILABLE and UP_TO_DATE archive-url update cases. I also addressed the security suggestions by restricting archive URLs to HTTPS and capping redirects; the latest d0ff5d0 update additionally consumes redirect bodies, defaults downloadFile to no token, and has GitHub release downloads opt into token use explicitly. |
| async function downloadFile( | ||
| url: string, | ||
| dest: string, | ||
| options: { includeGitHubToken?: boolean } = { includeGitHubToken: false }, |
There was a problem hiding this comment.
[Suggestion] The default was correctly flipped to includeGitHubToken: false, but the guard at line 542 still reads options.includeGitHubToken !== false. This means undefined still includes the token — if a future caller passes {} or an options object without this field, the token is silently sent, contradicting the new safe default.
Consider changing the guard to options.includeGitHubToken === true so the token is only sent by explicit opt-in, matching the intent of the default change.
(Current callers all pass the option explicitly so this is not exploitable today — it's defense-in-depth for the next person who touches this function.)
— qwen3.7-plus via Qwen Code /review
There was a problem hiding this comment.
Addressed in 824403d: changed the guard to options.includeGitHubToken === true, so token use is now explicit opt-in even if a future caller passes {}. I kept this as a focused one-line fix because downloadFile remains private and the existing entry-point tests already cover the two current explicit modes: GitHub release opts in and archive-url opts out.
d0ff5d0 to
824403d
Compare
| const entries = await fs.promises.readdir(destination, { | ||
| withFileTypes: true, | ||
| }); | ||
| if (entries.length > 2) { |
There was a problem hiding this comment.
[Critical] flattenSingleExtensionDirectory uses entries.length > 2 as the flatten threshold, but this depends on an undocumented invariant: whether the downloaded archive file is co-located inside destination during readdir.
- Download callers (
downloadFromGitHubReleaseline 399,downloadFromArchiveUrlline 439): the archive file sits indestination, so a wrapper dir + one sibling file (README, LICENSE) produces 3 entries → flatten skipped →assertExtractedArchiveContainsExtensionSourcefails with misleading "missing qwen-extension.json" error. - Local callers (
extensionManager.ts:936,checkForExtensionUpdateline 215): archive is NOT in the temp dir, so the same archive produces 2 entries → flatten proceeds correctly.
The same archive produces different behavior (and a confusing error) depending on the install path. A future refactor that moves archive downloads to a separate temp file would also silently break flatten for all download callers.
| if (entries.length > 2) { | |
| // Filter out the downloaded archive file if present, so the threshold | |
| // is consistent regardless of whether the archive is co-located. | |
| const archiveExtensions = ['.tar.gz', '.zip']; | |
| const extractedEntries = entries.filter( | |
| (e) => !archiveExtensions.some((ext) => e.name.endsWith(ext)), | |
| ); | |
| if (extractedEntries.length > 1) { | |
| return; | |
| } | |
| const lonelyDir = extractedEntries.find((entry) => entry.isDirectory()); |
— qwen3.7-max via Qwen Code /review
| let tempDir: string | undefined; | ||
| try { | ||
| let extensionDir = installMetadata.source; | ||
| if (isSupportedArchivePath(installMetadata.source)) { |
There was a problem hiding this comment.
[Suggestion] The type === 'local' + isSupportedArchivePath() branch in checkForExtensionUpdate is a new code path with no test coverage. The existing local update tests only exercise plain directory sources (/path/to/source), never archive paths (.zip/.tar.gz).
A regression in this branch (e.g., temp dir not cleaned up in finally, wrong directory passed to loadExtensionConfig) would go undetected.
Suggested test: create a local .zip archive with a valid extension manifest, create an extension with installMetadata: { type: 'local', source: archivePath }, and assert UPDATE_AVAILABLE / UP_TO_DATE / NOT_UPDATABLE (corrupt archive) results.
— qwen3.7-max via Qwen Code /review
| if (installMetadata?.type === 'npm') { | ||
| return checkNpmUpdate(installMetadata); | ||
| } | ||
| if (installMetadata?.type === 'archive-url') { |
There was a problem hiding this comment.
[Suggestion] checkForExtensionUpdate for archive-url downloads and extracts the entire archive just to compare latestConfig.version !== extension.version. This is called from checkAllExtensionsForUpdates for every archive-url extension, with no caching mechanism (ETag, Last-Modified, content hash).
With multiple archive-url extensions, every update check (including the auto-update poll on session start) triggers a full download + extract + temp-dir cleanup cycle per extension. This wastes bandwidth and can saturate slow connections.
Consider storing the archive's Content-Length + Last-Modified in installMetadata at install time and sending conditional HEAD requests before downloading. Even a simpler size-based short-circuit would help.
— qwen3.7-max via Qwen Code /review
| } | ||
|
|
||
| throw new Error( | ||
| `Extension archive is missing ${EXTENSIONS_CONFIG_FILENAME}. ` + |
There was a problem hiding this comment.
[Suggestion] The error message says "Extension archive is missing ${EXTENSIONS_CONFIG_FILENAME}." (i.e., "missing qwen-extension.json"), but hasSupportedExtensionSourceManifest actually accepts four manifest types: qwen-extension.json, gemini-extension.json, .claude-plugin/marketplace.json, .claude-plugin/plugin.json.
An archive containing only gemini-extension.json would receive an error claiming qwen-extension.json is missing, which is misleading.
| `Extension archive is missing ${EXTENSIONS_CONFIG_FILENAME}. ` + | |
| throw new Error( | |
| 'Extension archive does not contain a recognized extension manifest ' + | |
| '(expected qwen-extension.json, gemini-extension.json, or .claude-plugin/). ' + | |
| 'The archive must contain a complete extension at its root, or a single top-level extension directory.', | |
| ); |
— qwen3.7-max via Qwen Code /review
| false, | ||
| ); | ||
| expect( | ||
| isSupportedArchiveUrl('https://example.com/extension.tar.gz'), |
There was a problem hiding this comment.
[Suggestion] All extractArchiveFile tests use .zip format only. The .tar.gz extraction path through the new extractArchiveFile wrapper (which adds flattenSingleExtensionDirectory + assertExtractedArchiveContainsExtensionSource on top of extractFile) is not tested end-to-end.
A .tar.gz archive with a single top-level directory could fail to flatten or validate correctly without detection. The legacy extractFile has a .tar.gz test, but the wrapper's combined pipeline is untested.
Add at least one extractArchiveFile test using a .tar.gz archive with a wrapped extension directory.
— qwen3.7-max via Qwen Code /review
There was a problem hiding this comment.
Addressed in c8cd23b: added an extractArchiveFile .tar.gz test with a single wrapped extension directory, covering extraction plus flattening and manifest validation for the tar path.
| localSourcePath = tempDir; | ||
| } else if (installMetadata.type === 'archive-url') { | ||
| tempDir = await ExtensionStorage.createTmpDir(); | ||
| await downloadFromArchiveUrl(installMetadata, tempDir); |
There was a problem hiding this comment.
[Suggestion] installExtension error paths for archive types are not tested. The happy paths for archive-url and local archive installs are covered, but when downloadFromArchiveUrl or extractArchiveFile throws, the temp dir cleanup (tempDir removal in the outer error handler) is unverified.
Add tests where mockDownloadFromArchiveUrl.mockRejectedValue(...) and mockExtractArchiveFile.mockRejectedValue(...), then assert installExtension propagates the error and the temp dir is cleaned up.
— qwen3.7-max via Qwen Code /review
There was a problem hiding this comment.
Addressed in c8cd23b: fixed installExtension to clean up temp dirs when archive-url download or local archive extraction fails before the existing finally block, and added tests that assert the errors propagate and the temp dirs are removed.
c8cd23b to
34028e9
Compare
| } | ||
| return extension; | ||
| } catch (error) { | ||
| if (tempDir) { |
There was a problem hiding this comment.
Bug (medium confidence): Moving tempDir cleanup before the fallback recovery at line 1149-1157 creates a regression for git and github-release install types.
When an installation fails after a successful git clone (e.g., during convertGeminiOrClaudeExtension, loadExtensionConfig, or copyExtension), the catch block now deletes tempDir before the fallback loadExtensionConfig runs. For git/github-release installs, localSourcePath === tempDir, so the fallback tries to read from a deleted directory and silently fails.
In the original code, tempDir was declared inside the try block and therefore inaccessible in the catch block. The finally block (inside the try) handled cleanup only on the success path, leaving localSourcePath intact for the fallback recovery.
Impact: The partial-install recovery mechanism is broken for git-based installs. A user whose install fails mid-copy will no longer get a best-effort recovery of the extension config.
Fix: Move the early cleanup after the fallback, or scope it to archive types only:
} catch (error) {
if (!newExtensionConfig && localSourcePath) {
try {
newExtensionConfig = this.loadExtensionConfig({
extensionDir: localSourcePath,
workspaceDir: currentDir,
});
} catch {
// Ignore error
}
}
if (tempDir) {
await fs.promises.rm(tempDir, { recursive: true, force: true });
}
// ... rest of catch block
}Or limit early cleanup to archive types where localSourcePath === tempDir and the fallback was never viable:
if (tempDir && (installMetadata.type === 'archive-url' || isSupportedArchivePath(installMetadata.source))) {
await fs.promises.rm(tempDir, { recursive: true, force: true });
}There was a problem hiding this comment.
Addressed in 935f065 by moving the catch-side tempDir cleanup after the best-effort loadExtensionConfig fallback. One nuance I noticed while checking this: failures after entering the inner install try already run the existing inner finally before the outer catch, so this change specifically keeps the new catch-side cleanup from preempting the fallback for paths that reach the outer catch before that finally. The archive early-failure cleanup behavior remains covered by the existing tests.
34028e9 to
935f065
Compare
|
|
||
| await extractArchiveFile(downloadedAssetPath, destination); | ||
|
|
||
| await fs.promises.unlink(downloadedAssetPath); |
There was a problem hiding this comment.
[Suggestion] downloadFromGitHubRelease still wraps all errors — including post-download extraction and manifest validation failures — under a single Failed to download release from ... prefix (lines 399-408). The sister function downloadFromArchiveUrl was refactored to separate download errors from extraction errors (addressed in a352e09), but the same treatment was not applied here.
When a GitHub release archive downloads successfully but fails extraction (corrupt archive) or manifest validation (missing qwen-extension.json), the user sees Failed to download release from owner/repo: Extension archive could not be extracted... — the "Failed to download" prefix is misleading since the download succeeded.
Consider narrowing the try/catch to wrap only the download step:
try {
await downloadFile(archiveUrl, downloadedAssetPath, {
includeGitHubToken: true,
});
} catch (error) {
throw new Error(
`Failed to download release from ${redactUrlCredentials(installMetadata.source)}: ${redactUrlCredentials(getErrorMessage(error))}`,
);
}
await extractArchiveFile(downloadedAssetPath, destination);
await fs.promises.unlink(downloadedAssetPath);This matches the pattern already used in downloadFromArchiveUrl and gives users clearer error attribution.
— qwen3-coder-plus via Qwen Code /review
There was a problem hiding this comment.
Addressed in b2bc0ac: narrowed downloadFromGitHubRelease error wrapping so only the archive download step gets the "Failed to download release" prefix. Extraction and manifest validation errors now surface directly, matching downloadFromArchiveUrl.
Local real-run verification report (maintainer, macOS)Verified this PR end-to-end with a real built CLI ( Environment
Static & unit checks
Note: the PR text says package-level CLI typecheck still fails on pre-existing serve/acp errors — in my tree E2E matrix (real CLI + real archives + real HTTPS)
Notes (non-blocking)
ConclusionThe feature does what it says against real archives and a real HTTPS origin: both formats, both locations, single-dir flattening (without clobbering legitimate subdirectories), update replay for local and remote, and precise, actionable errors for the two failure modes. The detection is correctly conservative — it requires 中文版（Chinese version）本地真实运行验证报告（维护者,macOS）用真实构建的 CLI( 环境
静态与单元检查
说明:PR 文本称 package 级 CLI typecheck 仍因既有 serve/acp 错误失败——但在我的工作树里 E2E 矩阵(真实 CLI + 真实归档 + 真实 HTTPS）
备注（非阻塞）
结论该特性在真实归档和真实 HTTPS 源上确实做到了所声称的:两种格式、两种位置、单目录展平(且不会破坏合法子目录)、本地与远程的更新重放、两种失败模式的精确可操作错误。识别逻辑足够保守——要求 |
935f065 to
b2bc0ac
Compare
| ); | ||
| } | ||
| try { | ||
| await extractFile(archivePath, destination); |
There was a problem hiding this comment.
[Critical] extractArchiveFile calls extractFile which uses tar.x({ file, cwd: dest }) without restricting symlink creation. A malicious .tar.gz archive can include a SymbolicLink entry (e.g., symlink -> /) followed by a File entry at symlink/etc/cron.d/backdoor. The tar library's CHECKPATH validates entry.path syntactically but does not resolve the effective path through symlinks, so the file write follows the symlink to an arbitrary location outside dest.
While extractFile itself is unchanged, this PR significantly expands its attack surface: the new archive-url install type (downloadFromArchiveUrl) and the local-archive install path both accept user-provided archives through this code. Previously, extractFile was only called with GitHub release archives from trusted sources.
Consider adding a filter to reject symlinks and hardlinks during extraction:
| await extractFile(archivePath, destination); | |
| export async function extractFile(file: string, dest: string): Promise<void> { | |
| if (file.endsWith('.tar.gz')) { | |
| await tar.x({ | |
| file, | |
| cwd: dest, | |
| filter: (_path, entry) => entry.type !== 'SymbolicLink' && entry.type !== 'Link', | |
| }); | |
| } else if (file.endsWith('.zip')) { | |
| await extract(file, { dir: dest }); | |
| } else { | |
| throw new Error(`Unsupported file extension for extraction: ${file}`); | |
| } | |
| } |
— qwen3.7-max via Qwen Code /review
There was a problem hiding this comment.
Addressed in 2a93bf1: tar extraction now filters out SymbolicLink and Link entries, with a regression test covering symlink entries being skipped during extraction.
| const latestConfig = extensionManager.loadExtensionConfig({ | ||
| extensionDir: tempDir, | ||
| }); | ||
| if (!latestConfig) { |
There was a problem hiding this comment.
[Suggestion] checkForExtensionUpdate calls loadExtensionConfig directly on the extracted archive content, but loadExtensionConfig only looks for qwen-extension.json. During install (extensionManager.ts:948), convertGeminiOrClaudeExtension is called first to convert gemini-extension.json or .claude-plugin/* manifests into qwen-extension.json. Both the local + archive branch (line 217) and this archive-url branch skip the conversion step.
Archives that contain only a gemini or claude manifest will pass extractArchiveFile (which accepts all 4 manifest types) but then fail in loadExtensionConfig, causing the update check to return NOT_UPDATABLE (local) or ERROR (archive-url) instead of the correct state.
Add a convertGeminiOrClaudeExtension call before loadExtensionConfig, mirroring the install flow:
| if (!latestConfig) { | |
| await downloadFromArchiveUrl(installMetadata, tempDir); | |
| const { extensionDir } = await convertGeminiOrClaudeExtension( | |
| tempDir, | |
| installMetadata.pluginName, | |
| ); | |
| const latestConfig = extensionManager.loadExtensionConfig({ | |
| extensionDir, | |
| }); |
— qwen3.7-max via Qwen Code /review
There was a problem hiding this comment.
Addressed in 2a93bf1: moved the Gemini/Claude conversion helper into a shared extension-converter module and reused it from archive update checks before loadExtensionConfig. Added coverage for both local archive and archive-url Gemini archives being converted before update comparison, including cleanup of conversion temp dirs.
DragonnZhang
left a comment
DragonnZhang
left a comment
There was a problem hiding this comment.
Review: feat(extensions): support archive install sources
Summary
Well-structured PR adding archive (zip/tar.gz) install support for extensions. Good security practices overall: HTTPS-only enforcement for archive URLs, GitHub token stripping on cross-host redirects, and comprehensive test coverage.
Finding
[Bug] downloadFile missing stream error handlers — packages/core/src/extension/github.ts
The downloadFile function pipes the HTTP response to a file write stream but attaches no error handler to either stream. If the network connection drops mid-download or the write stream encounters an error (disk full, permission denied), the error event on the stream is unhandled. This will either crash the process via uncaughtException or cause the Promise to hang indefinitely, blocking the entire extension install/update flow.
const file = fs.createWriteStream(dest);
res.pipe(file);
file.on('finish', () => file.close(resolve as () => void));
// Missing: res.on('error', reject); file.on('error', reject);Suggested fix:
const file = fs.createWriteStream(dest);
res.on('error', reject);
file.on('error', reject);
res.pipe(file);
file.on('finish', () => file.close(resolve as () => void));No other high-confidence issues found. The redirect loop protection, cross-host token stripping, archive validation, and temp-dir cleanup all look correct.
b2bc0ac to
2a93bf1
Compare
|
Follow-up for DragonnZhang’s latest review: addressed the downloadFile stream error-handler finding in 2a93bf1 by attaching error handlers to both the HTTP response stream and the file write stream. Added coverage for an archive URL response stream error rejecting with a clear download error. |
qqqys
left a comment
qqqys
left a comment
There was a problem hiding this comment.
Critical blockers from the earlier archive-install review are resolved at head 2f43e1b. I rechecked the current diff, review history, comments, and checks: archive-url update replay, local archive update, co-located archive flattening, redirect/token handling, and archive extraction hardening are covered, and CI is green. No new critical issues found.
6965680
2f43e1b to
6965680
Compare
|
Rebased onto latest Conflict resolution notes:
Verification after conflict resolution:
GitHub now reports the branch as mergeable again; CI is re-running on the rebased head. |
|
@qwen-code /triage |
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
LGTM, looks ready to ship. ✅
| return; | ||
| } | ||
| res.resume(); | ||
| const redirectUrl = new URL(res.headers.location, url).toString(); |
There was a problem hiding this comment.
[Critical] new URL(res.headers.location, url) can throw a synchronous TypeError on malformed Location headers (e.g., https://[::1). Throws inside https.get response callbacks are NOT caught by .on('error', reject) — they become process-level uncaught exceptions that crash the application. A misconfigured or adversarial archive server can crash the entire Node.js process.
| const redirectUrl = new URL(res.headers.location, url).toString(); | |
| res.resume(); | |
| try { | |
| const redirectUrl = new URL(res.headers.location, url).toString(); | |
| const redirectHost = new URL(redirectUrl).host; | |
| const redirectOptions = | |
| redirectHost === parsedUrl.host | |
| ? options | |
| : { ...options, includeGitHubToken: false }; | |
| downloadFile(redirectUrl, dest, redirectOptions, redirectCount + 1) | |
| .then(resolve) | |
| .catch(reject); | |
| } catch (err) { | |
| reject(new Error(`Invalid redirect URL: ${getErrorMessage(err)}`)); | |
| } |
— qwen3.7-max via Qwen Code /review
| } | ||
|
|
||
| async function downloadFile(url: string, dest: string): Promise<void> { | ||
| async function downloadFile( |
There was a problem hiding this comment.
[Suggestion] downloadFile has no request timeout and no response body size limit. The sibling fetchUrl in marketplace.ts already implements both MARKETPLACE_FETCH_TIMEOUT_MS (10s wall-clock deadline + socket-idle timeout) and MARKETPLACE_MAX_BODY_BYTES (10 MB streaming cap). A slow or malicious archive server can hold the connection open indefinitely or fill disk without bound — especially in the automatic checkForExtensionUpdate path which runs without user interaction.
Consider adding a req.setTimeout() + absolute deadline and a byte counter on the data events, mirroring the fetchUrl pattern.
— qwen3.7-max via Qwen Code /review
| ); | ||
| } | ||
| const file = fs.createWriteStream(dest); | ||
| res.on('error', reject); |
There was a problem hiding this comment.
[Suggestion] The non-200 error path (line 595) does not call res.resume() before rejecting. The redirect path above correctly drains the response body so the socket can be returned to the connection pool. Without draining on error, repeated non-200 responses (e.g., 404 for removed archives) leak sockets and can stall subsequent downloads.
| res.on('error', reject); | |
| if (res.statusCode !== 200) { | |
| res.resume(); | |
| return reject( | |
| new Error(`Request failed with status code ${res.statusCode}`), | |
| ); | |
| } |
— qwen3.7-max via Qwen Code /review
| const configFilePath = path.join(extensionDir, EXTENSIONS_CONFIG_FILENAME); | ||
| if (fs.existsSync(configFilePath)) { | ||
| newExtensionDir = extensionDir; | ||
| } else if (isGeminiExtensionConfig(extensionDir)) { |
There was a problem hiding this comment.
[Suggestion] Dead code: newExtensionDir is initialized to extensionDir on line 25, and this if branch reassigns it to the exact same value. The fs.existsSync check and the entire branch can be removed with no behavioral change — the QwenCode config case is handled by the default initialization.
| } else if (isGeminiExtensionConfig(extensionDir)) { | |
| let newExtensionDir = extensionDir; | |
| let originSource: ExtensionOriginSource = 'QwenCode'; | |
| if (isGeminiExtensionConfig(extensionDir)) { |
— qwen3.7-max via Qwen Code /review
|
|
||
| function isZipSymlinkEntry(externalFileAttributes: number): boolean { | ||
| const mode = externalFileAttributes >>> 16; | ||
| return (mode & ZIP_FILE_TYPE_MASK) === ZIP_SYMBOLIC_LINK_TYPE; |
There was a problem hiding this comment.
[Suggestion] isZipSymlinkEntry filters symlink entries (0xa000) but not hard link entries (0x8000). The tar extraction path explicitly rejects both SymbolicLink and Link types. A crafted zip carrying hard-link entries would bypass the filter, creating aliased files inside the extraction directory. Consider adding a hard-link check for parity:
| return (mode & ZIP_FILE_TYPE_MASK) === ZIP_SYMBOLIC_LINK_TYPE; | |
| const ZIP_HARD_LINK_TYPE = 0x8000; | |
| function isZipLinkEntry(externalFileAttributes: number): boolean { | |
| const mode = externalFileAttributes >>> 16; | |
| return (mode & ZIP_FILE_TYPE_MASK) === ZIP_SYMBOLIC_LINK_TYPE || | |
| (mode & ZIP_FILE_TYPE_MASK) === ZIP_HARD_LINK_TYPE; | |
| } |
— qwen3.7-max via Qwen Code /review
6965680 to
def8e0a
Compare
|
Addressed the latest review in
Verification:
|
|
@qwen-code /triage |
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
LGTM, looks ready to ship. ✅
| res.statusCode === 308 | ||
| ) { | ||
| if (!res.headers.location) { | ||
| reject(new Error('Redirect response missing location header')); |
There was a problem hiding this comment.
[Suggestion] The redirect-without-location branch uses bare reject() instead of fail(). Every other error path in this function uses fail(), which sets settled = true and calls cleanup() to clear the hardDeadline timer. Using reject() bypasses both: the 120-second setTimeout leaks, keeping the Node.js event loop alive for up to 2 minutes after the caller has moved on.
| reject(new Error('Redirect response missing location header')); | |
| fail(new Error('Redirect response missing location header')); |
— qwen3.7-max via Qwen Code /review
| installMetadata.releaseTag = result.version; | ||
| localSourcePath = tempDir; | ||
| } else if ( | ||
| installMetadata.type === 'local' && |
There was a problem hiding this comment.
[Suggestion] Temp directory leak for local archive Gemini/Claude installs. When installing from a local .zip/.tar.gz that is a Gemini or Claude extension, convertGeminiOrClaudeExtension (called later at line ~1121) creates a separate temp directory for the converted output, and localSourcePath is reassigned to it. The finally block at line 1308 skips cleanup of this converted dir because installMetadata.type !== 'local' is false for local installs. Contrast with checkForExtensionUpdate (github.ts:226-246) which correctly tracks convertedDir separately and cleans it explicitly.
Consider tracking the converter's output directory separately (mirroring the convertedDir pattern) and cleaning it in both finally and catch blocks.
— qwen3.7-max via Qwen Code /review
| cwd: dest, | ||
| filter: (_path, entry) => | ||
| !('type' in entry) || | ||
| (entry.type !== 'SymbolicLink' && entry.type !== 'Link'), |
There was a problem hiding this comment.
[Suggestion] Asymmetric symlink/link handling between archive formats. The tar filter silently drops SymbolicLink and Link entries (returns false), while the zip onEntry handler throws an error that aborts extraction entirely. An extension archive containing symlinks installs successfully from .tar.gz (with the symlinked files silently missing) but fails outright from .zip.
Either both should throw (matching the zip behavior) or both should skip with a warning. Silent dropping in tar is the more dangerous path — the extension installs but may fail at runtime with cryptic "file not found" errors.
— qwen3.7-max via Qwen Code /review
| return undefined; | ||
| } | ||
|
|
||
| function hasSupportedExtensionSourceManifest(rootPath: string): boolean { |
There was a problem hiding this comment.
[Suggestion] hasSupportedExtensionSourceManifest hardcodes the same four manifest filenames that convertGeminiOrClaudeExtension (in extension-converter.ts) checks independently via EXTENSIONS_CONFIG_FILENAME, isGeminiExtensionConfig, and .claude-plugin/plugin.json. These two lists live in different files with no shared constant or compile-time coupling.
If a new third-party manifest format is added to the converter (e.g., Cursor or Windsurf), archives containing that format will be rejected at extraction time with a misleading "missing a supported extension manifest" error, before the converter ever sees them.
Consider exporting a shared SUPPORTED_EXTENSION_MANIFESTS constant consumed by both functions.
— qwen3.7-max via Qwen Code /review
| ); | ||
| } | ||
| }); | ||
| res.on('error', fail); |
There was a problem hiding this comment.
[Suggestion] res.on('error', fail) does not call file.destroy(). When the response stream errors mid-pipe, fail settles the Promise but the write stream's file descriptor remains open until GC. Compare with the byte-limit path (lines 644-645) which correctly calls both res.destroy() and file.destroy().
| res.on('error', fail); | |
| res.on('error', (err) => { file.destroy(); fail(err); }); |
— qwen3.7-max via Qwen Code /review
| withFileTypes: true, | ||
| }) | ||
| ).filter((entry) => entry.name !== archiveNameToIgnore); | ||
| if (entries.length > 2) { |
There was a problem hiding this comment.
[Suggestion] entries.length > 2 is more permissive than the old code it replaces. The original downloadFromGitHubRelease used entries.length === 2 where entries included the archive file — meaning exactly 1 directory and nothing else. After filtering out the archive, the new code allows up to 2 non-archive entries (1 directory + 1 sibling file), which the old code would not have flattened.
This could cause a rename collision if the directory contains a child with the same name as the sibling root file. Either tighten to entries.length > 1 to match the old single-directory-only semantics, or add a rename-collision check before each fs.promises.rename.
— qwen3.7-max via Qwen Code /review
|
Updated at What changed:
Additional tests added for:
Verification:
Note: |
|
@wenshao Updated at |
|
@qwen-code /triage |
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
LGTM, looks ready to ship. ✅
✅ Maintainer verification — archive installs work end-to-end on Linux (incl. the security hardening)I ran an independent real-CLI verification on Linux (the PR marks Linux Bottom line: every happy path, error path, and the security hardening behave correctly on the bundled CLI. CI is already green on all three OSes. One small UX note (the Method
Results
Evidence
Security (the [Critical] from review — mitigated by the The head adds A/B vs base (
|
| Base | PR | |
|---|---|---|
local root.tar.gz |
Configuration file not found at …/root.tar.gz/qwen-extension.json (treats archive as a dir, no extraction) |
extracted + installed |
| archive URL | Failed to clone Git repository from https://…/demo-remote.tar.gz (treated as git) |
installed as archive-url |
Confirms the behavior is genuinely new, not pre-existing.
Unit tests / CI
- PR's listed suites, run locally on Linux: 5 files / 165 tests pass (
github.test.ts,marketplace.test.ts,extensionManager.test.ts,install.test.ts,dev.test.js). - CI:
Testgreen on macOS, Ubuntu, and Windows; Lint + CodeQL green.MERGEABLE(no conflicts). The block is the openCHANGES_REQUESTEDreview, not CI.
One UX note (non-blocking)
Remote archive URLs are intentionally HTTPS-only (isSupportedArchiveUrl requires protocol === 'https:'). A plain http://…/x.tar.gz therefore falls through to the git path and surfaces a misleading Failed to clone Git repository from http://…/x.tar.gz rather than a clear "archive URLs must use https". Functionally correct and a sound security default; consider a clearer message (or at least documenting https-only) so users who paste an http:// archive aren't sent debugging a phantom git error.
🇨🇳 中文版（点击展开）
✅ 维护者验证 —— 归档安装在 Linux 上端到端可用（含安全加固）
我在 Linux 上做了独立的真实 CLI 验证（PR 标 Linux qwen extensions install 流程——包括 tmux 下的交互式授权确认——针对真实的 .zip / .tar.gz 归档与本地 HTTPS 服务器，并专门复测了 review 里提的 [Critical] 符号链接 向量。
结论： 所有正常路径、错误路径与安全加固在打包后的 CLI 上行为正确；CI 在三平台已全绿。下面有一个小的 UX 备注（http:// → git 回退）。
方法
| 组件 | 说明 |
|---|---|
| PR 构建 | head 6406760b8 → npm ci + npm run bundle → 真实 dist/cli.js |
| Base 构建 | 61dcf865d（PR 父提交），用于 A/B |
| 归档 | 真实 tar/zip：根目录含 manifest、单顶层目录（展平）、损坏、无 manifest、符号链接（tar+zip）、路径穿越 |
| 远程 | 自签名本地 HTTPS 服务器，提供可替换的 .tar.gz（NODE_TLS_REJECT_UNAUTHORIZED=0），用于 URL 安装 + 更新重放 |
| 驱动 | 隔离 QWEN_HOME；矩阵用直接 CLI + 一次真实 tmux 交互授权 |
结果
| # | 场景 | 结果 |
|---|---|---|
| 1 | 本地 .tar.gz，manifest 在根 |
✅ 安装 · metadata type: "local" |
| 2 | 本地 .zip，manifest 在根 |
✅ 安装 · type: "local" |
| 3 | manifest 在单个顶层目录内 | ✅ 安装并展平（qwen-extension.json 落到扩展根目录） |
| 4 | 远程 HTTPS 归档 URL | ✅ 安装 · metadata type: "archive-url" · 走授权流程 |
| 5 | 更新重放（把服务文件从 1.0.0 换成 2.0.0） |
✅ extensions update 重新下载所存 URL → 1.0.0 → 2.0.0；HTTPS 日志显示第二次拉取 |
| 6 | 损坏归档 | ✅ Extension archive could not be extracted…（未安装） |
| 7 | 无受支持 manifest 的归档 | ✅ Extension archive is missing a supported extension manifest. Expected one of: …（未安装） |
| 8 | .tar.gz 中的符号链接 |
✅ 拒绝：Tar archive contains unsupported link entry: ./pwn_link |
| 9 | .zip 中的符号链接 |
✅ 拒绝：Zip archive contains unsupported symbolic link entry: pwn_link |
| 10 | 路径穿越（../../../../tmp/PWNED.txt） |
✅ 被中和——payload 从未逃逸（/tmp/...PWNED.txt 未创建；条目被丢弃）；安装目录只剩 manifest |
| 11 | 交互式授权（tmux） | ✅ 真实 Do you want to continue? [Y/n]: → 输入 y → installed successfully and enabled |
安全（review 里的 [Critical]，由 harden 提交 6406760b8 修复）
evil-symlink.tar.gz → Tar archive contains unsupported link entry: ./pwn_link
evil-symlink.zip → Zip archive contains unsupported symbolic link entry: pwn_link
evil-traversal.tar.gz → 安装（manifest 合法），但 ../ 条目被丢弃；/tmp/...PWNED.txt 未创建
head 新增 assertTarArchiveHasNoLinks()（预扫描 tar 条目，遇 SymbolicLink/Link 抛错）与 zip onEntry 符号链接检查；node-tar 默认会剥离 ..。三种向量我都在运行时确认被中和。加固还把下载上限设为 100 MB、超时 120 s，并在跨主机重定向时剥离 GitHub token。
A/B 对比 base（61dcf865d）
| Base | PR | |
|---|---|---|
本地 root.tar.gz |
Configuration file not found at …/root.tar.gz/qwen-extension.json（把归档当目录，不解压） |
解压并安装 |
| 归档 URL | Failed to clone Git repository from https://…/demo-remote.tar.gz（当成 git） |
作为 archive-url 安装 |
证明此行为是新增的，而非既有。
单测 / CI
- PR 列出的套件，本地 Linux 运行：5 文件 / 165 测试通过（
github.test.ts、marketplace.test.ts、extensionManager.test.ts、install.test.ts、dev.test.js）。 - CI：
Test在 macOS、Ubuntu、Windows 全绿；Lint + CodeQL 绿。MERGEABLE（无冲突）。阻塞来自尚未消除的CHANGES_REQUESTED，而非 CI。
一个 UX 备注（不阻塞）
远程归档 URL 是仅 HTTPS（isSupportedArchiveUrl 要求 protocol === 'https:'）。因此 http://…/x.tar.gz 会回退到 git 路径，报出有误导性的 Failed to clone Git repository from http://…/x.tar.gz，而不是清晰的"归档 URL 必须用 https"。功能正确、也是稳妥的安全默认；建议给个更清晰的提示（或至少文档化 https-only），免得用户粘贴 http:// 归档时去排查并不存在的 git 错误。
Verified locally against real archive fixtures + a self-signed HTTPS server under an isolated QWEN_HOME; no external services or real credentials. Malicious-archive fixtures (symlink / path-traversal) were synthetic and confined to the test dir.
| } | ||
|
|
||
| return ( | ||
| parsedUrl.protocol === 'https:' && |
7 participants
What this PR does
Adds support for installing extensions from local
.zipand.tar.gzarchives, plus remote archive URLs that end in.zipor.tar.gz. Archive installs reuse the existing extension extraction, validation, conversion, consent, copy, metadata, and update paths. Local archives stay aslocalinstall metadata, while remote archives use a newarchive-urlinstall metadata type so updates can replay the stored URL.This also makes the existing Windows-only dev launcher test path assertions accept Windows path separators, which unblocked the Windows CI job.
Why it is needed
Some extension authors distribute self-contained archives instead of git repositories or npm packages. Letting the install command consume those archives directly makes the CLI install path more flexible while keeping the extension structure and validation requirements the same.
Reviewer Test Plan
How to verify
Install a local
.zipor.tar.gzarchive that containsqwen-extension.jsonat the archive root, or inside a single top-level directory, and confirm it installs and writes local install metadata. Install a remote archive URL ending in.zipor.tar.gzand confirm it installs through the same consent and copy flow, storesarchive-urlmetadata, and can be checked for updates by replaying that URL. Try a corrupt archive or an archive without a supported extension manifest and confirm the CLI reports a specific archive extraction or missing-manifest error.Evidence (Before & After)
Before:
qwen extensions installtreated HTTP(S) archive URLs as git sources, and local archive files were copied as files instead of being extracted as extension packages.After: supported local archives and archive URLs are extracted, flattened when they contain a single top-level extension directory, validated for a compatible extension manifest, and installed through the normal extension manager flow.
Tested on
Environment (optional)
Node.js >=22 with focused unit tests.
Commands run:
Risk & Scope
.zipor.tar.gz.npm run preflightwas not run; package-level CLI typecheck still fails on existing serve/channel/acp/web-template workspace errors outside this change.Linked Issues
Fixes #4910
中文说明
What this PR does
支持从本地
.zip/.tar.gz归档安装扩展，也支持从以.zip或.tar.gz结尾的远程归档 URL 安装扩展。归档安装复用现有扩展解压、校验、转换、授权确认、复制、metadata 写入和更新路径。本地归档仍保存为local安装 metadata；远程归档使用新的archive-url安装 metadata 类型，这样更新时可以重放保存的 URL。同时让现有 Windows-only dev launcher 测试里的路径断言兼容 Windows 路径分隔符，用来解除 Windows CI job 的阻塞。
Why it is needed
有些扩展作者会发布自包含归档，而不是 git 仓库或 npm 包。让 install 命令直接消费这些归档，可以让 CLI 安装路径更灵活，同时保持扩展结构和校验要求不变。
Reviewer Test Plan
How to verify
安装一个本地
.zip或.tar.gz归档，归档根目录或单个顶层目录中包含qwen-extension.json，确认它能安装并写入 local 安装 metadata。安装一个以.zip或.tar.gz结尾的远程归档 URL，确认它经过同一套确认和复制流程，保存archive-urlmetadata，并能通过重放该 URL 检查更新。尝试损坏归档或缺少兼容 manifest 的归档，确认 CLI 输出明确的解压失败或缺少 manifest 错误。Evidence (Before & After)
Before：
qwen extensions install会把 HTTP(S) 归档 URL 当作 git source，本地归档文件也会作为普通文件复制，而不是作为扩展包解压。After：支持的本地归档和归档 URL 会被解压；如果包含单个顶层扩展目录会自动展平；随后校验兼容扩展 manifest，并通过正常 extension manager 流程安装。
Tested on
macOS 已本地测试；Windows 依赖 CI；Linux 未在本地测试。
Environment (optional)
Node.js >=22，运行了聚焦单元测试。
Risk & Scope
主要风险或权衡：当 URL path 以
.zip或.tar.gz结尾时，archive URL 会优先于普通 HTTP(S) git source 被识别。未验证或范围外：没有运行完整
npm run preflight；package-level CLI typecheck 仍会因为本次改动外已有的 serve/channel/acp/web-template workspace 错误失败。破坏性变更或迁移说明：无。
Linked Issues
Fixes #4910