* refactor(providers): unify provider config into core, remove CLI re-exports
Move all ProviderConfig definitions, registry (ALL_PROVIDERS), and
utility functions (buildInstallPlan, resolveBaseUrl, etc.) from
packages/cli/src/auth/ into packages/core/src/providers/ so both
CLI and VSCode can share the same provider system.
- Add core providers module with types, presets, install logic
- Rewrite VSCode AuthMessageHandler to dynamically generate provider
choices from ALL_PROVIDERS instead of hardcoding 3 providers
- Add applyProviderInstallPlanToFile in VSCode settingsWriter using
the ProviderSettingsAdapter abstraction
- Delete 11 CLI re-export wrapper files, update ~20 import sites
- Keep CLI-specific applyProviderInstallPlan (uses LoadedSettings)
and openrouterOAuth.ts (CLI-only OAuth runtime)
Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
* refactor(cli): drop OpenRouter OAuth + /manage-models, simplify /auth
OpenRouter now uses the standard API-key flow under "Third-party Providers"
(issue #4108). The whole OpenRouter OAuth implementation (PKCE, callback
server, model auto-install) and the /manage-models command (only OpenRouter
was wired in; /auth Step 2 already covers model selection) are removed.
/auth is renamed around the "Connect a Provider" mental model:
- Dialog title is now "Connect a Provider"; the OAuth main entry is gone
- handleAuthSelect (mixed close + auth trigger) is split into a single-purpose
closeAuthDialog; legacy wrappers (handleSubscriptionPlanSubmit,
handleApiKeyProviderSubmit, handleCustomApiKeySubmit, ...) are dropped in
favor of the unified handleProviderSubmit
Core: openRouterProvider switches to authMethod='input', uiGroup='third-party',
ships with two recommended free models, and is reordered to the end of the
third-party list to keep DeepSeek as the default highlight.
Net diff: 34 files, +124 / -3835.
Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
* refactor(auth): unify applyProviderInstallPlan in core, drop cli/auth
CLI and vscode now share core's applyProviderInstallPlan instead of keeping
two parallel implementations. The CLI-only env rollback (snapshot
process.env, restore on error) is folded into the core version so vscode
also benefits from it.
CLI ships a LoadedSettingsAdapter that maps LoadedSettings to core's
ProviderSettingsAdapter contract. Backup/restore is layered: write a .orig
file, structuredClone settings + originalSettings, then recomputeMerged()
on restore — same guarantees as before, just routed through the adapter.
Tests for the install logic are migrated to core and rewritten against the
adapter mock (more focused than the previous LoadedSettings/Config mocks).
packages/cli/src/auth/ is gone entirely.
Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
* refactor(providers): drop unused authMethod field from ProviderConfig
Every preset has had authMethod='input' since OpenRouter switched to the
standard API-key flow, making the field a dead dimension. Removing it
cleans up three never-taken branches and aligns the type with reality:
connecting a provider always means entering an API key.
- core: remove ProviderConfig.authMethod; shouldShowStep('apiKey') is
now unconditionally true; drop authMethod from 9 presets
- vscode AuthMessageHandler: drop the OAuth branch in handleAuthInteractive
- vscode WebViewProvider: simplify the apiKey-required guard
- tests: update provider-config.test and custom-provider.test
If a future provider needs a browser-based flow, the field can be
re-introduced; for now the smaller surface is worth more.
Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
* refactor(providers): prefix Alibaba plan presets with alibaba-
Rename coding-plan.{ts,test.ts} → alibaba-coding-plan.{ts,test.ts} and
token-plan.{ts,test.ts} → alibaba-token-plan.{ts,test.ts} so the file
names line up with the existing alibaba-standard preset and make it
obvious at a glance which presets belong to Alibaba ModelStudio.
Export names (codingPlanProvider, tokenPlanProvider, TOKEN_PLAN_*,
CODING_PLAN_*) are unchanged — only the file paths and the two
imports in all-providers.ts / index.ts move.
Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
* fix(vscode): guard ProviderSettingsAdapter against prototype pollution
The dotted-key writer in createFileSettingsAdapter walked through any
segment, including __proto__/constructor/prototype, which would let a
malicious or malformed ProviderInstallPlan reach Object.prototype.
Refuse to write paths containing reserved segments and use
hasOwnProperty when traversing intermediate objects so that inherited
properties cannot redirect the walk.
Addresses CodeQL alert #226 surfaced on PR #4287.
Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
* fix(auth): default Audio modality to off in provider advanced config
In the /auth Custom Provider advanced-config step, "Enable modality"
should default to Image + Video only. Audio was on by default, which
implied the model accepts audio input even though most providers
people configure here don't.
Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
* fix(auth): show base URL default as placeholder, not prefilled value
In Custom Provider Step 2/6 (and on protocol switch), the base URL
input started with the protocol's default URL pre-filled. Users who
wanted a non-default endpoint had to manually clear the field first.
Switch to placeholder semantics: the input starts empty, the default
URL is shown as a hint, and submitting blank falls back to that
default (then writes it back to baseUrl so downstream steps see a
real value).
Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
* refactor(cli): rename /auth description to "Connect an LLM provider"
The old description ("Configure authentication information for login")
implied a Qwen-account login. After the /auth refactor it's really
about picking an LLM provider and entering credentials, so the menu
entry should say that.
Also add 'connect' as an alt-name alongside the existing 'login' so
users can type /connect when 'auth' feels wrong. Keep 'login' for
muscle-memory compatibility.
Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
* i18n(cli): translate "Connect an LLM provider" in all locales
Strict-parity locales (zh, zh-TW) require every built-in command
description to be translated; the renamed /auth description was
falling back to English and breaking the must-translate test.
Add translations for zh / zh-TW (required) and refresh the other
seven locales (en, ru, de, ja, fr, ca, pt) so the old
"Configure authentication information for login" key is removed
everywhere rather than left as a dangling dictionary entry.
Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
* fix(vscode): await applyProviderInstallPlanToFile and grow test coverage
Critical: applyProviderInstallPlanToFile fired the install plan with
`void`, so any rejection (EACCES from persist(), prototype-pollution
guard throw, etc.) was silently swallowed and WebViewProvider proceeded
to disconnect/reconnect the agent as if the write had succeeded.
Make the wrapper `async` and `await` it in the only caller.
Tests added:
- core/install.test: isSameModelIdentity fallback path
(prepend-and-remove-owned with no ownsModel) — verifies models are
matched on id+baseUrl, not just id.
- vscode/AuthMessageHandler.test: happy-path with a fixed-baseUrl
third-party provider, validateApiKey error branch, and BaseUrlOption
picker presentation.
Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
* fix(auth): address PR #4287 review (critical + suggestion)
vscode AuthMessageHandler (Critical):
- Add the missing protocol-selection step so custom-provider users can
pick Anthropic/Gemini instead of being silently locked to OpenAI.
- Validate free-form base URL with the same /^https?:\/\// check the
CLI uses; reject file:/javascript: schemes.
vscode AuthMessageHandler (Suggestion):
- Stop filtering separator entries from the provider QuickPick so
groups (Alibaba Cloud / Third Party / Custom) actually show as
headers instead of a flat list.
- Treat a null authInteractiveHandler as an error: surface an
authError + cancellation notification instead of silently dropping
the user's input.
- Call notifyAuthCancelled when validateApiKey rejects so the
webview state resets and the user can retry.
core/providers/presets/openrouter.ts (Critical):
- Replace the substring includes() in ownsModel with a URL-hostname
match so paths like https://api.example.com/openrouter.ai/v1 stop
being misidentified as OpenRouter models (and getting removed on
re-install).
vscode/services/settingsWriter.ts (Critical):
- stripTrailingCommas() so JSONC files with trailing commas (VSCode's
default style) parse instead of silently returning {} and then
overwriting the entire settings file.
- readSettings() distinguishes ENOENT (return {}) from parse errors
(log + rethrow) so a malformed file never gets clobbered.
- writeSettings() writes through a temp file + fs.renameSync atomic
rename, eliminating the half-written file window on EACCES /
disk-full / crash.
- setValue() refuses to overwrite a scalar at an intermediate path
segment (would have silently destroyed e.g. {"env": "legacy-string"}).
core/providers/install.ts (Suggestion):
- Move settings.backup?.() inside the try block so a backup failure
still triggers the env-rollback path in catch.
cli/config/loadedSettingsAdapter.ts (Suggestion):
- Add the same UNSAFE_KEY_PARTS guard the vscode adapter has, so
__proto__/constructor/prototype segments are rejected before
reaching the underlying setNestedPropertySafe walker. Defense in
depth: not exploitable today but the utility has no built-in guard.
vscode/webview/providers/WebViewProvider.ts (Suggestion):
- Hoist buildInstallPlan / applyProviderInstallPlanToFile to static
imports (both modules already top-level imported); drops two
per-call await import() round-trips.
cli/utils/doctorChecks.ts (Suggestion):
- Whitespace nit before the comma in the qwen-code-core import.
Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
* fix(auth): second round of PR #4287 review fixes
Critical:
- settingsWriter: stripTrailingCommas now uses a char-by-char scanner so
literal ",]" inside a string value is preserved (the previous regex
silently corrupted it).
- install.ts: wrap settings.restore() in try/catch so a restore failure
doesn't mask the original error or skip the env-rollback loop.
- install.ts: snapshot the runtime ModelProvidersConfig before applying
patches and reload it in the catch path, so an in-flight refreshAuth()
failure doesn't leave the live session holding providers that were
never successfully installed.
- AuthMessageHandler: custom-provider Base URL is now a placeholder
instead of a pre-filled value, with the default selected by the
user's chosen protocol (openai/anthropic/gemini). Empty input falls
back to the protocol-appropriate URL, preventing the
pick-Anthropic-but-keep-OpenAI-URL footgun.
Suggestion:
- AuthDialog: replace the isCurrentlyCodingPlan misnomer with a uiGroup
check — resolveMetadataKey returns config.id for *any* provider with
a static models[], so the old guard made DeepSeek/MiniMax/OpenRouter
users land on the Alibaba tab instead of Third-party Providers.
- AuthMessageHandler: guard against modelIds being [] after splitting
comma input (matches the CLI's "Model IDs cannot be empty.").
- WebViewProvider: restore the explanatory comment for the
authState === true success-toast guard that the previous diff
accidentally dropped.
Tests:
- settingsWriter.test: new applyProviderInstallPlanToFile suite covering
happy path, prototype-pollution guard (built via Object.defineProperty
to bypass __proto__ literal semantics), intermediate-scalar rejection,
malformed-file no-clobber, JSONC-with-trailing-commas parsing
(including a string containing ",]"), and the atomic-write tmp-file
cleanup.
- loadedSettingsAdapter.test: new file — forwarding, UNSAFE_KEY_PARTS
rejection, getValue against merged settings, backup/restore round-trip,
cleanupBackup semantics.
- provider-config.test: added findProviderByCredentials and
getAllProviderBaseUrls coverage (preset hits, unknown-key misses,
BaseUrlOption[] preset expansion).
Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
* fix(cli): satisfy strict tsc --build in loadedSettingsAdapter.test
CI's `tsc --build` (with emit) enforced two strict checks that
`tsc --noEmit` had been letting through:
- `noPropertyAccessFromIndexSignature` flagged `file.settings['env']`
reads against `Record<string, unknown>`. Switched the test fixture
shape to a named `SettingsShape` interface with explicit `env` and
`modelProviders` keys (plus an index signature for setValue's
arbitrary writes), so dot access on the known keys is no longer
"through" the index signature.
- Calling optional methods via `adapter.backup?.()` produced TS2722
(`Cannot invoke an object which is possibly 'undefined'`) under the
build flags. createLoadedSettingsAdapter always installs
backup/restore/cleanupBackup, so the tests now assert
`toBeTypeOf('function')` first and then call via non-null assertion,
which both documents the invariant and makes the call typesafe.
- Dropped the `({} as Record<string, unknown>)['polluted']` sanity
check; `expect(setValue).not.toHaveBeenCalled()` already proves the
guard short-circuits before any write reaches LoadedSettings.
Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
* fix(cli): guard mock setValue against prototype pollution in adapter test
CodeQL flagged the mock setValue's recursive property assignment as a prototype-pollution sink. Add UNSAFE_KEY_PARTS check at the top of the mock to align with the real setNestedPropertySafe contract.
Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
* fix(cli): use literal === guards for CodeQL prototype-pollution sanitiser
CodeQL re-flagged the mock setValue write even after the Set.has guard added in 2e6adf8a6d — the scanner only recognises inline literal === comparisons as prototype-pollution sanitisers, not Set lookups.
Reworked the mock to (1) merge the guard into the loop so every current[part] write is preceded by a literal === check against '__proto__'/'constructor'/'prototype', and (2) collapse the dual leaf/branch logic into a single loop body. Runtime behaviour is identical; CodeQL should now treat the write as sanitised.
Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
* fix(auth): third round of PR #4287 review fixes (8 comments)
Critical:
- useAuth: handleProviderSubmit now calls setPendingAuthType at the start
of the try, so handleAuthFailure can record the AuthEvent telemetry on
applyProviderInstallPlan rejection (previously dropped silently because
pendingAuthType was undefined).
- settingsWriter: readQwenSettingsForVSCode wraps readSettings in
try/catch so a malformed settings.json no longer crashes the VSCode
extension on activation; the write paths (writeCodingPlanConfig,
writeModelProvidersConfig) deliberately keep propagating to avoid
silently overwriting a corrupt file with partial data.
Suggestions:
- settingsWriter.setValue: intermediate-segment guard now also rejects
arrays (typeof [] === 'object' previously slipped through and would
let us set string keys on an array). Loop restructured so the
literal-=== prototype-pollution guard runs at every step, satisfying
CodeQL's sanitiser detector on both the leaf and intermediate writes.
- settingsWriter atomic write: SETTINGS_FILE_MODE = 0o600 +
SETTINGS_DIR_MODE = 0o700 + best-effort chmod on existing files. API
keys persisted into env.* are no longer world-readable on multi-user
systems.
- loadedSettingsAdapter: switched its prototype-pollution guard to the
same inline literal === pattern so the two adapters stay symmetric
and CodeQL recognises both as sanitisers (Comment 6 — explicit
'keep in sync' comment + same shape rather than a shared helper that
CodeQL wouldn't trace through).
- AuthMessageHandler: protocol QuickPick now shows 'OpenAI Compatible'
/ 'Anthropic' / 'Gemini' instead of the raw AuthType enum values.
- WebViewProvider: authInteractive log now records only the parsed
hostname, not the full inputs.baseUrl, so credentials embedded in
userinfo or query strings don't leak into extension-host logs.
Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
* test(auth): cover the rollback safety nets in applyProviderInstallPlan + useAuth failure path
Addresses the missing-coverage points in the latest review pass: every deliberately-engineered rollback path in install.ts and the visible side effects of handleAuthFailure now have a regression test, so a future refactor that 'simplifies' these paths can't silently break them.
applyProviderInstallPlan (install.test.ts, +4 cases):
- restores runtime model providers when refreshAuth rejects after
reloadModelProviders ran (asserts the second reloadModelProviders call
receives the pre-install snapshot).
- still rolls back env vars when backup() throws before persist (pins
the 'backup inside try' invariant added in 38a214d0ec).
- continues env rollback even when settings.restore itself throws
(pins the nested try/catch around restore added in 38a214d0ec).
- continues throw + env rollback when the rollback-time
reloadModelProviders itself throws (the original error must still
surface; env vars must still revert).
useAuth (useAuth.test.ts, +1 case):
- surfaces install-plan rejection as an auth error and records
telemetry — refreshAuth throws, the test asserts authError is set,
the dialog reopens, isAuthenticating clears, no success toast is
added, and pendingAuthType is populated (which is what the new
setPendingAuthType call lets handleAuthFailure key the AuthEvent on).
- createSettings now mocks recomputeMerged + forScope.settings so the
loaded-settings-adapter restore() path doesn't emit a noisy stderr.
Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
* fix(auth): fourth round of PR #4287 review fixes
Critical:
- settingsWriter JSONC scanner: \uXXXX is a 6-char escape, not 2.
The previous stripJsonComments / stripTrailingCommas used j+=2 for
every backslash, so a value containing \u0022 would let the embedded
quote terminate the string early — turning a single string value into
multiple top-level keys after the strip passes. That's a parser
differential vs JSON.parse and enables settings.json key injection
(e.g. an attacker-controlled API_KEY string could inject env.NODE_OPTIONS).
Now we branch on text[j+1] === 'u' and skip 6, satisfying both scanners.
- resolveBaseUrl no longer crashes on an empty baseUrl array. The
previous config.baseUrl[0].url threw 'Cannot read undefined.url' on []
and brought down the whole install flow. Falls back to selectedBaseUrl
or '' instead.
- providerMatchesCredentials now resolves function-typed envKey by
calling it with (protocol, baseUrl). The previous typeof-string gate
made the custom provider invisible to findProviderByCredentials —
/doctor and system-info diagnostics couldn't see custom-provider users.
Catches the function call so a misbehaving custom envKey can't crash
the matcher.
Suggestions:
- AuthDialog: defaultMainIndex now also returns 2 for uiGroup === 'custom'
so a custom-provider user lands on the Custom Provider tab instead of
Alibaba ModelStudio.
- install.ts: env-var rollback loop is now wrapped in try/catch matching
the same shape as the settings.restore() and reloadModelProviders
rollbacks. A process.env write throwing (custom property descriptors,
some sandboxes) won't skip the runtime-providers rollback below.
- readSettings: SyntaxError is now wrapped in an actionable Error
('Cannot parse ~/.qwen/settings.json ($name: $message). Standard
JSONC is supported... Please fix or delete $path...') so users facing
a corrupt file get a clear message instead of a bare SyntaxError. The
cause is preserved via Error.cause.
Tests:
- settingsWriter: new \u0022 injection regression — asserts that a
string containing \u0022 stays a single string and no injected key
lands at the top level.
- provider-config: new edge-case suite for resolveBaseUrl with [] and
providerMatchesCredentials with function-typed envKey (matching path,
wrong-key path, function-throws path). Re-imports via the relative
source path so the new behaviour is exercised even before dist/ is
rebuilt.
Not addressed:
- handleProviderSubmit error-path test (Comment 3264567491) was already
added in 7d8b4785ad — same test, same surface (refreshAuth rejection
+ authError set + dialog reopen + isAuthenticating false + no success
toast + pendingAuthType populated).
Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
* fix(vscode): import AuthType as value not type
AuthMessageHandler now references AuthType.USE_OPENAI etc. as enum values (for the protocolLabels map added in cdc17cbba0), but the import was 'import type AuthType' which strips the runtime binding. TS1361 fired in CI's emitting build even though --noEmit was happy locally.
Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
* fix(providers): restore modelscope test + tighten openrouter ownsModel
Two findings from the latest /review pass that survived earlier rounds:
1. modelscope.test.ts was deleted in the move-from-CLI step (60 lines / 4 cases under packages/cli/src/auth/providers/thirdParty/) but never recreated in core's preset test folder. Re-added a 3-case suite (config shape, install plan with per-model metadata for known IDs, graceful fallback for unknown IDs) so the third-party preset coverage is symmetric again. Also exported modelscopeProvider from packages/core/src/providers/index.ts so the public API matches the other presets.
2. openrouter.ts ownsModel previously claimed any model on an openrouter.ai hostname, which would silently delete a user's hand-added entry that happened to route through openrouter.ai under a different envKey (e.g. a personal gateway). Now requires both model.envKey === OPENROUTER_ENV_KEY AND the openrouter.ai hostname match. Existing openrouter.test.ts updated and extended to cover: matching path, envKey mismatch path, host mismatch path, missing/malformed baseUrl.
The remaining findings in that /review were either already addressed in earlier rounds (custom provider visibility / resolveBaseUrl empty array / useAuth telemetry / TS4111 errors — verified 0 locally) or architectural concerns beyond this PR's scope (LoadedSettings.setValue's per-call saveSettings).
Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
* fix(auth): fifth round of PR #4287 review fixes
Critical:
- provider-config.ts providerMatchesCredentials: iterate config.protocolOptions
when resolving a function-typed envKey instead of relying on the default
config.protocol. A custom provider configured under USE_ANTHROPIC or
USE_GEMINI persists an envKey derived from THAT protocol, not from
USE_OPENAI — without iteration the matcher silently misses them and
custom-provider users disappear from /doctor + AppHeader +
systemInfoFields + AuthDialog.defaultMainIndex.
- provider-config.test.ts: the existing test asserting 'returns false for
function-typed envKey' was holding on the old broken behaviour. Flipped
to assert toBe(true) for the matching path, and routed it through the
relative source import so it doesn't run against stale dist.
Suggestions:
- settingsWriter.clearPersistedAuth: now wipes every preset's string envKey
(iterates ALL_PROVIDERS, plus the existing subscription-plan loop kept
for explicitness) and every QWEN_CUSTOM_API_KEY_* key by prefix match.
Previously DeepSeek / MiniMax / Z.AI / IdeaLab / ModelScope / OpenRouter
/ custom keys lingered on disk after clearing auth.
- custom-provider.ts generateCustomEnvKey: the readable-only normalization
collapsed 'api.example.com', 'api-example.com', and 'api_example.com'
into the same env key, so two structurally different custom providers
would overwrite each other's API key. Now appends a 6-hex-char SHA-256
suffix derived from (protocol, baseUrl-with-trailing-slash-stripped).
The trailing-slash invariant from the prior implementation is preserved
(api/v1 and api/v1/ still hash equal). Suffix collision probability at
6 hex chars is ~1/16M per pair — fine for an interactive flow.
Tests:
- provider-config.test.ts: added a 'iterates protocolOptions' case that
configures a custom-style provider, derives the key under
USE_ANTHROPIC, and asserts the matcher finds it.
- custom-provider.test.ts: regex-matches the new readable+hash format
for the deterministic / special-character / empty-string cases, and a
new 'disambiguates structurally distinct URLs that normalize
identically' case that pins down the collision fix
(api.example.com vs api-example.com vs api_example.com all differ).
Not addressed:
- TS1361 'type AuthType' import — already fixed in 8f94b018bd
- modelscope re-export — already fixed in 7228d73d80
Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
* fix(custom-provider): replace polynomial regex with linear char scans
CodeQL alerts 225 + 232 flagged `/_+/g`, `/^_+|_+$/g`, and `/\/+$/` in generateCustomEnvKey as polynomial regex on user input. V8 handles these patterns linearly in practice, but the scanner can't see that and any baseUrl with many '_' or '/' would be flagged as a theoretical worst case.
Replaced both passes with single-pass character scans:
- normalizeEnvSegment: walks the string once, emits alphanumerics verbatim, collapses any non-alphanumeric run to a single '_', then trims leading/trailing underscores via charCodeAt index walks. Equivalent to the prior three regexes but with no quantifier backtracking surface.
- stripTrailingSlashes: walks backwards from the end while charCodeAt === 47, then slices. Equivalent to `replace(/\/+$/, '')`.
All 11 custom-provider tests still pass — output format and invariants (trailing-slash equivalence, hash suffix, protocol/URL disambiguation) are unchanged.
Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
* fix(auth): seventh round of PR #4287 review fixes
Critical:
- i18n: 9 locale files updated to replace orphaned 'Select Authentication
Method' / 'You must select an auth method...' keys with the new
'Connect a Provider' / 'You must connect a provider...' keys the
AuthDialog actually references. Non-English users no longer see the
English fallback for the main heading + exit-prevention warning.
- settingsWriter.writeSettings: renameSync is now wrapped in try/catch
that unlinks the temp file on failure (EPERM/EBUSY on Windows from
watchers/AV would otherwise orphan a secret-bearing .tmp file in
~/.qwen on every failed write).
- settingsWriter.restore(): write to disk FIRST, then update in-memory
data. The previous order left memory clean while disk retained the
failed install's partial state if writeSettings threw. Now matches
the CLI adapter's order.
- AuthMessageHandler custom-provider tests: added 4 cases covering
protocol picker → free-form URL → API key → comma-split model IDs →
advanced config (one happy path), plus the http(s) scheme guard, the
protocol-aware blank-URL fallback, and the whitespace-only model
IDs guard. Previously the entire custom path through
runProviderSetupFlow had zero coverage.
- settingsWriter clearPersistedAuth tests: added cases for the
expanded preset/custom/subscription cleanup (asserts NODE_OPTIONS
survives, every QWEN_CUSTOM_API_KEY_* is wiped, providerMetadata
entries for every preset are gone) plus a no-settings-file no-op.
Suggestions:
- loadedSettingsAdapter.restore(): now checks restoreSettingsFromBackup's
boolean return value and logs an explicit warning when on-disk rollback
fails (EACCES / missing .orig). Previously the failure was silent and
the next CLI restart would read a corrupted file.
- generateCustomEnvKey: hash suffix lengthened from 6 → 12 hex chars
(24 → 48 bits). Brings collision search out of milliseconds-range
enumeration; offline 'pick a URL that collides' attack is no longer
practical at interactive setup time.
- getDefaultBaseUrlForProtocol: new shared helper in core consumed by
both the CLI (useProviderSetupFlow) and VS Code (AuthMessageHandler)
flows. Removes the duplicated DEFAULT_BASE_URLS map; one source of
truth for the OpenAI/Anthropic/Gemini placeholder URLs.
- settingsWriter.clearPersistedAuth: providerMetadata cleanup now
iterates ALL_PROVIDERS with resolveMetadataKey instead of hardcoding
coding-plan/token-plan. Stale metadata for deepseek/minimax/zai/
idealab/modelscope/openrouter no longer lingers after logout.
- resolveMetadataKey: explicit guard against provider ids containing
'.'. A dotted id would split into multiple nested objects under
providerMetadata, silently corrupting the settings tree. Now throws
loudly at registration time.
- customProvider: added explicit ownsModel that prefix-matches against
QWEN_CUSTOM_API_KEY_*. Reinstalling a custom provider under a
different baseUrl now reliably replaces (not accumulates) the old
entries.
Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
* fix(auth): eighth round of PR #4287 review fixes
Suggestions:
- clearPersistedAuth metadata cleanup loop: per-iteration try/catch
around resolveMetadataKey so a future dotted-id provider can't abort
the loop and leave secrets on disk.
- VS Code AuthMessageHandler: removed the hardcoded
|| 'https://api.openai.com/v1' fallback after
getDefaultBaseUrlForProtocol — defaults must live in core. The CLI
flow has no such fallback, and the silent OpenAI default would mask
a new AuthType core hadn't been taught about.
- settingsWriter restore() comment: clarified the deliberate divergence
from the CLI adapter's trade-off (disk-fail-throws here, disk-fail-
logs-and-continues there) so the comment doesn't read 'same order'.
- useAuth handleAuthFailure: closure staleness — setPendingAuthType
queues an async React update, so handleAuthFailure's pendingAuthType
read could see undefined when a synchronous throw beats the next
render. Added an optional protocolForTelemetry argument that the new
handleProviderSubmit passes explicitly; closure fallback kept for
legacy callers. AuthEvent error telemetry is no longer silently
dropped.
- install.ts: track currentStep before each phase (backup → env →
modelProviders → authType → legacyCredentials → modelSelection →
providerState → persist → reloadModelProviders → syncAuthState →
refreshAuth → cleanupBackup) and annotate the rethrown error with
the failing step + authType. Original error preserved via Error.cause
so callers matching on err.code still work.
- custom-provider.ts: stale '6-hex-char' comment updated to 12. Added
a migration note explaining that old 6-char keys persist as harmless
orphan disk state until clear-auth.
- settingsUtils.restoreSettingsFromBackup: was swallowing fs errors
with catch(_e); now logs the underlying cause so the adapter's
on-disk-rollback-failed warning has something specific to point at.
Tests:
- useAuth: new cancelAuthentication case asserts isAuthenticating
clears, externalAuthState clears, dialog opens, authError clears.
- provider-config: new resolveMetadataKey suite — normal id, no-models
→ undefined, dotted id → throws.
- install: new case asserting the rethrown error names the failing
step ('refreshAuth') + authType and preserves the original error
via Error.cause.
Not addressed:
- 6→12 hash backward compat (Comment 3267562667): The 6-char keys are
orphan disk state — never read by applyProviderInstallPlan (the new
model provider entries reference the new 12-char key), so no security
or correctness issue, just disk noise that clears on next sign-out.
Documented in custom-provider.ts. A full clean-up pass would need a
new ProviderSettingsAdapter delete API + a migration scan — better
as its own PR.
- writeSettings renameSync error path test + loadedSettingsAdapter
restore-failure log test (terminal-only findings): adding these
requires fs mocking surgery that's worth its own PR.
Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
* fix(format): four prettier/JSDoc nitpicks from review
All four are Critical-tagged formatter / docs issues caught by the latest /review pass:
- AppHeader.tsx: `AuthType ,` (stray space before comma) → standard newline-after-{ form. Was breaking CI Lint.
- useProviderUpdates.test.ts: same `AuthType ,` pattern → standard form.
- apiPreconnect.ts: double blank line after the closing `}` of the
import block (left behind when getAllProviderBaseUrls was removed
from the old auth/allProviders path) → single blank line.
- types.ts (Suggestion): JSDoc for `modelsEditable` said
"false → skip model step; use models as-is (e.g. Coding Plan)" but
codingPlanProvider actually sets modelsEditable: true (every preset
in the registry does), so the example contradicts the registry.
Dropped the parenthetical.
Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
* test(scripts): raise install-script suite timeout to survive Windows
Windows CI flaked on `standalone release packaging > rejects unexpected dist assets` with a 5000ms timeout. The test shells out to `node scripts/create-standalone-package.js` which produces a tar.gz; observed real runtimes from sibling tests in the same run: 4780ms / 1666ms / 1079ms — the 4.8s case is already at vitest's default 5s limit, so a slightly slower subprocess startup (antivirus inspection, contended runner) tips it over.
Pre-existing test (added 2026-05-11 in cb7059f54d), unrelated to this PR's auth refactor. Bumped the suite-wide testTimeout to 30s in scripts/tests/vitest.config.ts — the tests still complete in seconds when subprocess startup is healthy; the headroom only kicks in to cover Windows-slow variance.
Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
* fix(auth): ninth round of PR #4287 review fixes
Critical:
- WebViewProvider.handleAuthInteractive: roll back bad credentials when the
agent reconnect rejects them. applyProviderInstallPlanToFile commits the
key + calls cleanupBackup before the disconnect/reconnect runs, so the
plan's own rollback can't cover an authState=false outcome. Now snapshot
settings before the write (snapshotSettingsForRollback) and restore it
(restoreSettingsSnapshot) on both the authState!=true branch and the
catch branch. Without this a rejected key persisted and every VS Code
restart retried it. Two new helpers added to settingsWriter; never-throw
snapshot so a malformed pre-state degrades to a no-op restore.
Suggestions:
- AuthMessageHandler: trim the API key before validateApiKey + persistence,
matching the CLI flow (useProviderSetupFlow trims in two places). A key
pasted with trailing whitespace no longer causes silent auth failures or
VS-Code-only validateApiKey rejections.
- install.ts: the annotated rethrow no longer bakes 'step "persist"' into
the user-facing message. Step + authType are now structured properties on
a new exported ProviderInstallError (message stays the underlying error
text, cause preserved). Callers can show a clean message and log
err.step/err.authType to the dev console.
- provider-config.ts: providerMatchesCredentials no longer swallows a throw
from a function-typed envKey — console.warn surfaces the programming
error so a custom provider silently vanishing from /doctor has a trace.
- types.ts: documented that ProviderSettingsAdapter.setValue MAY flush to
disk eagerly (the CLI LoadedSettings adapter does) and that persist() can
be a no-op for such adapters — so future authors don't insert pre-persist
steps assuming atomicity.
- settingsWriter: moved the orphaned stripJsonComments JSDoc off
jsonEscapeLength (the \u-escape helper inserted between the doc and its
function) back onto stripJsonComments itself.
Tests:
- settingsWriter: snapshot/restore round-trip, malformed→null→no-op-restore,
no-file→{} snapshot.
- install: updated the step-annotation test to assert err.step/err.authType
structured properties + clean message instead of the embedded string.
- WebViewProvider.test: settingsWriter mock extended with
applyProviderInstallPlanToFile/snapshotSettingsForRollback/
restoreSettingsSnapshot.
Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
* fix(auth): tenth round of PR #4287 review fixes
Critical (both from the previous round's own changes):
- WebViewProvider.handleAuthInteractive: restoreSettingsSnapshot →
writeSettings can throw (EPERM on Windows renameSync / disk full /
EACCES). Both rollback call sites are now routed through a local
safeRollback() that try/catches and logs, so a rollback failure can
never (a) re-throw out of the else-branch into the outer catch and
trigger a second rollback that skips the error message, nor (b) throw
out of the catch-branch and leave the webview auth dialog hanging with
no feedback.
- provider-config.providerMatchesCredentials: the new envKey-throw
console.warn logged the full baseUrl, which can embed credentials
(https://user:sk-secret@host). Now logs only new URL(baseUrl).hostname
(with an [invalid] fallback) and err.message, matching the
sanitization WebViewProvider already uses.
Tests:
- WebViewProvider.test: new 'credential rollback' describe with three
cases — (1) authState!==true after reconnect → restoreSettingsSnapshot
called with the snapshot, (2) authState===true → restore NOT called,
(3) restore throws (EPERM) → handleAuthInteractive still resolves and
the authError message is still sent. Hoisted mocks extended with
applyProviderInstallPlanToFile / snapshotSettingsForRollback /
restoreSettingsSnapshot refs so the scenario is controllable.
Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
* fix(auth): eleventh round of PR #4287 review fixes
Critical:
- AuthMessageHandler: validation-failure paths (bad URL scheme, invalid
API key, empty model IDs, handler-not-set) no longer call
notifyAuthCancelled after sendToWebView({authError}). The webview's
ProviderSetupForm clears the error on authCancelled, so the two
messages raced and the error flashed away before the user could read
it. authCancelled is now reserved for genuine user dismissals (Escape
on a QuickPick/InputBox); authError already clears the connecting state.
- WebViewProvider: after rolling back rejected credentials, also
disconnect the agent. The reconnect spawned a process holding the bad
key in memory; without disconnect a subsequent chat message hit a
stale-credential error unrelated to the original auth failure. Now
agentManager.disconnect() + agentInitialized=false so the next /auth
reconnects cleanly.
Suggestions:
- install.ts: added a DENY_ENV_KEYS denylist (NODE_OPTIONS, NODE_PATH,
LD_PRELOAD, LD_LIBRARY_PATH, DYLD_INSERT_LIBRARIES, DYLD_LIBRARY_PATH,
PATH, HOME, TMPDIR), checked case-insensitively before writing any
plan.env entry to settings + process.env. Defense in depth: all callers
go through buildInstallPlan with hardcoded keys today, but
ProviderInstallPlan is exported.
- settingsUtils: setNestedPropertySafe AND setNestedPropertyForce now
refuse __proto__/constructor/prototype path segments (inline literal
=== so CodeQL recognises the sanitiser). migrateProviderMetadata feeds
field names from Object.entries on user settings.json, and JSON.parse
keeps __proto__ as an own property — guarding at the utility protects
every caller, not just the adapters.
Already fixed in f31224bac1 (review ran against 9f45a7536b):
- restoreSettingsSnapshot throw masking the original error → safeRollback.
- baseUrl logged verbatim in providerMatchesCredentials → hostname only.
Tests:
- install: NODE_OPTIONS rejected + not leaked to process.env/settings;
case-insensitive Path rejection.
- AuthMessageHandler: validation authError is NOT followed by
authCancelled.
- WebViewProvider: rollback path disconnects the agent + clears
agentInitialized.
- settingsUtils: setNestedPropertySafe/Force refuse __proto__/
constructor/prototype and don't pollute Object.prototype.
Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
* fix(test): use bracket access in settingsUtils prototype-pollution tests
The new setNestedProperty guard tests asserted obj.a.b.c / obj.x.y dot-access on Record<string, unknown>, which trips noPropertyAccessFromIndexSignature (TS4111) under the emitting tsc --build the CI 'Install dependencies' step runs. Local npm run typecheck (--noEmit) had a stale tsbuildinfo and didn't re-check the file. Switched to bracket access (obj['a']['b']['c']) to match the strict option. Behaviour unchanged; 78 settingsUtils tests still pass.
Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
* test(vscode): cover the outer-catch rollback path in handleAuthInteractive
All prior rollback tests exercised the else-branch (authState !== true). The outer catch — reached when applyProviderInstallPlanToFile or doInitializeAgentConnection throws (disk errors, partial writes) — had no coverage, and that's the higher-risk path. New test makes doInitializeAgentConnection reject and asserts (1) restoreSettingsSnapshot called with the snapshot, (2) authError sent containing 'Configuration failed', (3) handleAuthInteractive resolves without throwing. Guards against a regression that drops the safeRollback wrapper in the catch.
Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
* test(providers): make NODE_OPTIONS denylist test env-independent
The test asserted process.env.NODE_OPTIONS toBeUndefined after the rejected plan, but CI sets NODE_OPTIONS (--max-old-space-size=3072 from the build script), so it failed there while passing locally where NODE_OPTIONS is unset. Snapshot the original value and assert the rejected plan left it UNCHANGED (and specifically not the evil --require value) — that's the actual invariant: the denylist throws before mutating process.env.
Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
* fix(vscode): disconnect stale agent in handleAuthInteractive catch block too
The else-branch (authState !== true) disconnected the agent after rollback, but the outer catch only rolled back. If doInitializeAgentConnection partially initializes (agentInitialized=true, agent process spawned) then throws — e.g. a disk error during post-connect setup — the stale-credential agent stayed connected.
Extracted a disconnectStaleAgent() local helper (alongside safeRollback) and called it in both the else-branch and the catch, so the two paths are symmetric. Extended the outer-catch test to spawn a partial agent before the throw and assert disconnect() is called + agentInitialized cleared.
Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
* fix(auth): twelfth round of PR #4287 review fixes (5 suggestions)
All from DeepSeek's pass, all on recent commits:
- settingsUtils: stale comment referenced a non-existent UNSAFE_PATH_SEGMENTS const; the actual guard is pathHasUnsafeSegment(). Fixed both comment sites.
- settingsWriter.snapshotSettingsForRollback: was silently returning null on a readSettings throw (disabling credential rollback with no signal). Now console.warn's the cause so oncall can tie repeated cross-restart auth failures back to a transient unreadable settings file.
- provider-config.providerMatchesCredentials: the envKey-throw warn logged err.message, which a user-defined envKey fn could populate with the API key (new Error(`bad config: ${apiKey}`)). Now logs only err.constructor.name — no message, no URL.
- install.ProviderInstallError: was an interface (erased at compile time → instanceof always false). Converted to a class extending Error so instanceof works at runtime; exported as a value (not type) from the barrel. Construction simplified to new ProviderInstallError(msg, step, authType, { cause }).
- install.DENY_ENV_KEYS: added Windows TMP/TEMP alongside TMPDIR so a crafted plan can't redirect temp-file creation on Windows.
Tests:
- install: assert the thrown error is instanceof ProviderInstallError; new it.each covering TMP/TEMP/tmp rejection (case-insensitive).
Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
* fix(vscode): log error class name not message in snapshotSettingsForRollback
Consistency with the err.constructor.name approach applied in provider-config.providerMatchesCredentials. The risk here is lower (the catch is filesystem errors from readSettings/structuredClone, not user-defined functions), but logging only the class name keeps the security stance uniform across the codebase.
Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
---------
Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
a552df8998