diff --git a/.github/skills/add-policy/github-managed-settings.md b/.github/skills/add-policy/github-managed-settings.md index 3b9ea8f9791..76433c17149 100644 --- a/.github/skills/add-policy/github-managed-settings.md +++ b/.github/skills/add-policy/github-managed-settings.md @@ -79,7 +79,7 @@ the VS Code bag is **flattened** to dot-paths — e.g. the schema's nested |------------------------|----------------|----------------------------------------| | `permissions.disableBypassPermissionsMode` | string enum `"disable"` | most-restrictive-wins (sticky once set) | | `enabledPlugins` | `{ "PLUGIN@MARKETPLACE": boolean }` | deny-wins (false beats true; enterprise denials immutable) | -| `extraKnownMarketplaces` | `{ name: { source } }`, source `github` \| `git` \| `directory` | most-restrictive-wins (higher layer is the complete allowlist) | +| `extraKnownMarketplaces` | `{ name: { source, autoUpdate? } }`, source `github` \| `git` \| `directory` | most-restrictive-wins (higher layer is the complete allowlist); explicit `autoUpdate` overrides the client's global plugin auto-update setting for that marketplace | | `strictKnownMarketplaces` | array of source descriptors | most-restrictive-wins (empty array = lockdown) | > **Current schema ↔ runtime divergence** (treat `managed-settings-schema.json` as the diff --git a/build/lib/policies/policyData.jsonc b/build/lib/policies/policyData.jsonc index 61e8def658d..258ac8ccc42 100644 --- a/build/lib/policies/policyData.jsonc +++ b/build/lib/policies/policyData.jsonc @@ -605,7 +605,7 @@ "localization": { "description": { "key": "chat.plugins.extraMarketplaces.policy", - "value": "Additional plugin marketplaces to query. Keys are marketplace names; values are GitHub shorthand (`owner/repo[#ref]`) or Git URIs (`{url}[#ref]`)." + "value": "Additional plugin marketplaces to query. Keys are marketplace names; values are GitHub shorthand (`owner/repo[#ref]`) or Git URIs (`{url}[#ref]`), optionally with an enterprise-managed auto-update override." } }, "type": "object", diff --git a/src/vs/base/common/managedSettings.ts b/src/vs/base/common/managedSettings.ts index 1d5205a0b99..73bccbc621d 100644 --- a/src/vs/base/common/managedSettings.ts +++ b/src/vs/base/common/managedSettings.ts @@ -8,8 +8,15 @@ * name (used as `displayLabel`) and the original `source` discriminator. */ export type IExtraKnownMarketplaceEntry = - | { readonly name: string; readonly source: { readonly source: 'github'; readonly repo: string; readonly ref?: string } } - | { readonly name: string; readonly source: { readonly source: 'git'; readonly url: string; readonly ref?: string } }; + | { readonly name: string; readonly autoUpdate?: boolean; readonly source: { readonly source: 'github'; readonly repo: string; readonly ref?: string } } + | { readonly name: string; readonly autoUpdate?: boolean; readonly source: { readonly source: 'git'; readonly url: string; readonly ref?: string } }; + +export interface IExtraKnownMarketplaceConfigValue { + readonly source: string; + readonly autoUpdate: boolean; +} + +export type ExtraKnownMarketplacesConfigDict = Record; /** * A single entry in the enterprise-managed `strictKnownMarketplaces` allowlist @@ -32,10 +39,13 @@ export interface IStrictMarketplaceSource { /** * Converts an {@link IExtraKnownMarketplaceEntry} array into the - * `{ [name]: url-or-shorthand }` dict stored on the `chat.plugins.extraMarketplaces` + * policy dict stored on the `chat.plugins.extraMarketplaces` * setting (and carried as the canonical JSON value of the `extraKnownMarketplaces` * managed setting across both the server endpoint and native MDM delivery). * + * Entries without `autoUpdate` retain the legacy source string. Entries with an + * explicit override use a JSON-encoded {@link IExtraKnownMarketplaceConfigValue}. + * * Plain-string entries (allowed by the policy schema but unnamed) are stored with * the value used as both key and value so they survive the round-trip intact. * @@ -43,11 +53,11 @@ export interface IStrictMarketplaceSource { * so `__proto__` / `constructor` / `prototype` keys are skipped to avoid prototype pollution * (mirroring the guard in the managed-settings normalizer's string-map encoder). */ -export function extraKnownMarketplacesToConfigDict(entries: readonly (string | IExtraKnownMarketplaceEntry)[] | undefined): Record | undefined { +export function extraKnownMarketplacesToConfigDict(entries: readonly (string | IExtraKnownMarketplaceEntry)[] | undefined): ExtraKnownMarketplacesConfigDict | undefined { if (!entries?.length) { return undefined; } - const obj: Record = {}; + const obj: ExtraKnownMarketplacesConfigDict = {}; for (const entry of entries) { if (typeof entry === 'string') { if (isUnsafeMarketplaceKey(entry)) { @@ -60,7 +70,8 @@ export function extraKnownMarketplacesToConfigDict(entries: readonly (string | I } const s = entry.source; const base = s.source === 'github' ? s.repo : s.url; - obj[entry.name] = s.ref ? `${base}#${s.ref}` : base; + const source = s.ref ? `${base}#${s.ref}` : base; + obj[entry.name] = entry.autoUpdate === undefined ? source : JSON.stringify({ source, autoUpdate: entry.autoUpdate } satisfies IExtraKnownMarketplaceConfigValue); } } return obj; diff --git a/src/vs/platform/policy/common/copilotManagedSettings.ts b/src/vs/platform/policy/common/copilotManagedSettings.ts index 61db1834e64..fa5f220358f 100644 --- a/src/vs/platform/policy/common/copilotManagedSettings.ts +++ b/src/vs/platform/policy/common/copilotManagedSettings.ts @@ -5,7 +5,7 @@ import { Event } from '../../../base/common/event.js'; import { IPolicyData } from '../../../base/common/defaultAccount.js'; -import { IExtraKnownMarketplaceEntry, extraKnownMarketplacesToConfigDict } from '../../../base/common/managedSettings.js'; +import { ExtraKnownMarketplacesConfigDict, IExtraKnownMarketplaceEntry, extraKnownMarketplacesToConfigDict } from '../../../base/common/managedSettings.js'; import { IManagedSettingPolicyDefinition, IManagedSettingsPolicyDefinitions, ManagedSettingValue, ManagedSettingsData } from '../../../base/common/policy.js'; import { IStringDictionary } from '../../../base/common/collections.js'; import { isEmptyObject, isObject, isString } from '../../../base/common/types.js'; @@ -437,11 +437,11 @@ function encodeArray(value: unknown): unknown[] | undefined { } /** - * Encode the schema's `{ [id]: { source } }` marketplace map into the canonical - * `{ [name]: url-or-shorthand }` dict; drops malformed entries (with an optional warning) and omits + * Encode the schema's `{ [id]: { source, autoUpdate? } }` marketplace map into the canonical + * policy dict; drops malformed entries (with an optional warning) and omits * the key when there are none. */ -function encodeExtraMarketplaces(value: unknown, onWarn?: (msg: string) => void): Record | undefined { +function encodeExtraMarketplaces(value: unknown, onWarn?: (msg: string) => void): ExtraKnownMarketplacesConfigDict | undefined { return extraKnownMarketplacesToConfigDict(normalizeExtraKnownMarketplaces(value, onWarn)); } @@ -530,8 +530,8 @@ function withNestedManagedKeyDeleted(obj: Record, dottedKey: st * - Structured settings (declared in {@link STRUCTURED_MANAGED_SETTINGS}) are carried as canonical * JSON strings under a single key each — the same shape an admin authors via native MDM. * `PolicyConfiguration` parses the JSON back into the object-typed setting on read. - * `extraKnownMarketplaces` is normalized from the schema's `{ [id]: { source } }` map to the - * `{ [name]: url-or-shorthand }` dict. + * `extraKnownMarketplaces` is normalized from the schema's `{ [id]: { source, autoUpdate? } }` + * map to the policy-backed marketplace dict. * * Malformed marketplace entries are dropped (with an optional warning via {@link onWarn}) rather * than throwing, so a bad enterprise settings file degrades gracefully instead of blocking startup. @@ -559,7 +559,7 @@ export function normalizeManagedSettings(parsed: Record, onWarn } /** - * Normalize the schema's `{ [id]: { source } }` marketplace map into an + * Normalize the schema's `{ [id]: { source, autoUpdate? } }` marketplace map into an * {@link IExtraKnownMarketplaceEntry} array, preserving the marketplace `name`, * source discriminator, and any `ref`. Malformed or off-spec entries are dropped * (with an optional warning via {@link onWarn}). @@ -575,12 +575,17 @@ function normalizeExtraKnownMarketplaces(value: unknown, onWarn?: (msg: string) onWarn?.(`Skipping malformed extraKnownMarketplaces entry "${name}": expected { source: { source, repo|url } }`); continue; } - const src = (entry as Record).source as { source?: string; repo?: string; url?: string; ref?: string }; + const rawEntry = entry as Record; + const src = rawEntry.source as { source?: string; repo?: string; url?: string; ref?: string }; + const autoUpdate = typeof rawEntry.autoUpdate === 'boolean' ? rawEntry.autoUpdate : undefined; + if (rawEntry.autoUpdate !== undefined && autoUpdate === undefined) { + onWarn?.(`Ignoring invalid autoUpdate for extraKnownMarketplaces entry "${name}": expected boolean`); + } let normalized: IExtraKnownMarketplaceEntry | undefined; if (src.source === 'github' && isString(src.repo)) { - normalized = { name, source: { source: 'github', repo: src.repo, ...(src.ref ? { ref: src.ref } : {}) } }; + normalized = { name, ...(autoUpdate === undefined ? {} : { autoUpdate }), source: { source: 'github', repo: src.repo, ...(src.ref ? { ref: src.ref } : {}) } }; } else if (src.source === 'git' && isString(src.url)) { - normalized = { name, source: { source: 'git', url: src.url, ...(src.ref ? { ref: src.ref } : {}) } }; + normalized = { name, ...(autoUpdate === undefined ? {} : { autoUpdate }), source: { source: 'git', url: src.url, ...(src.ref ? { ref: src.ref } : {}) } }; } else if (src.source === 'github' || src.source === 'git') { onWarn?.(`Skipping extraKnownMarketplaces entry "${name}": source "${src.source}" requires ${src.source === 'github' ? '"repo"' : '"url"'}`); } else { diff --git a/src/vs/platform/policy/test/common/fileManagedSettingsService.test.ts b/src/vs/platform/policy/test/common/fileManagedSettingsService.test.ts index ab52531dff1..c519bd4723d 100644 --- a/src/vs/platform/policy/test/common/fileManagedSettingsService.test.ts +++ b/src/vs/platform/policy/test/common/fileManagedSettingsService.test.ts @@ -65,15 +65,29 @@ suite('normalizeManagedSettings', () => { test('normalizes extraKnownMarketplaces from schema format to config dict', () => { const result = normalizeManagedSettings({ [COPILOT_EXTRA_MARKETPLACES_KEY]: { - 'a': { source: { source: 'github', repo: 'github/agent-skills' } }, - 'b': { source: { source: 'git', url: 'https://example.com/repo.git', ref: 'v1' } }, + 'a': { source: { source: 'github', repo: 'github/agent-skills' }, autoUpdate: true }, + 'b': { source: { source: 'git', url: 'https://example.com/repo.git', ref: 'v1' }, autoUpdate: false }, + 'c': { source: { source: 'github', repo: 'github/copilot-plugins' } }, } }); assert.deepStrictEqual(result, { - [COPILOT_EXTRA_MARKETPLACES_KEY]: '{"a":"github/agent-skills","b":"https://example.com/repo.git#v1"}', + [COPILOT_EXTRA_MARKETPLACES_KEY]: '{"a":"{\\"source\\":\\"github/agent-skills\\",\\"autoUpdate\\":true}","b":"{\\"source\\":\\"https://example.com/repo.git#v1\\",\\"autoUpdate\\":false}","c":"github/copilot-plugins"}', }); }); + test('ignores non-boolean marketplace autoUpdate with warning', () => { + const warnings: string[] = []; + const result = normalizeManagedSettings({ + [COPILOT_EXTRA_MARKETPLACES_KEY]: { + 'a': { source: { source: 'github', repo: 'github/agent-skills' }, autoUpdate: 'yes' }, + } + }, msg => warnings.push(msg)); + assert.deepStrictEqual(result, { + [COPILOT_EXTRA_MARKETPLACES_KEY]: '{"a":"github/agent-skills"}', + }); + assert.deepStrictEqual(warnings, ['Ignoring invalid autoUpdate for extraKnownMarketplaces entry "a": expected boolean']); + }); + test('drops malformed marketplace entries with warning', () => { const warnings: string[] = []; const result = normalizeManagedSettings({ diff --git a/src/vs/workbench/contrib/chat/browser/chat.shared.contribution.ts b/src/vs/workbench/contrib/chat/browser/chat.shared.contribution.ts index d9dc9079259..434c15d386b 100644 --- a/src/vs/workbench/contrib/chat/browser/chat.shared.contribution.ts +++ b/src/vs/workbench/contrib/chat/browser/chat.shared.contribution.ts @@ -1279,21 +1279,19 @@ configurationRegistry.registerConfiguration({ // Policy-only delivery slot for enterprise-managed marketplace entries (via the // `ChatExtraMarketplaces` policy). Consumers union this with `chat.plugins.marketplaces`. // - // Stored as a `{ [name]: url-or-shorthand }` object so that: + // Stored as a named string map. Explicit update overrides are JSON-encoded + // inside the value string so the Settings Editor can use its inline object renderer. + // This ensures: // - The Settings Editor (ComplexObject renderer) can display entries inline when // managed by policy, rather than only showing "Edit in settings.json". // - Marketplace names are preserved for `enabledPlugins["plugin@"]` resolution. // - // `additionalProperties: { type: ['string'] }` uses the single-element array form of - // JSON Schema's `type` keyword (equivalent to `type: 'string'`) to trigger VS Code's - // ComplexObject renderer, which shows key-value rows inline and hides the - // "Edit in settings.json" link when the value is managed by policy. type: 'object', additionalProperties: { type: ['string'] as ['string'] }, default: {}, scope: ConfigurationScope.APPLICATION, included: false, - markdownDescription: nls.localize('chat.plugins.extraMarketplaces', "Enterprise-managed additional plugin marketplaces. Unioned with {0}.", `\`#${ChatConfiguration.PluginMarketplaces}#\``), + markdownDescription: nls.localize('chat.plugins.extraMarketplaces', "Enterprise-managed additional plugin marketplaces. Unioned with {0}. An entry's `autoUpdate` value overrides {1} for plugins from that marketplace.", `\`#${ChatConfiguration.PluginMarketplaces}#\``, '`#extensions.autoUpdate#`'), policy: { name: 'ChatExtraMarketplaces', category: PolicyCategory.InteractiveSession, @@ -1305,7 +1303,7 @@ configurationRegistry.registerConfiguration({ localization: { description: { key: 'chat.plugins.extraMarketplaces.policy', - value: nls.localize('chat.plugins.extraMarketplaces.policy', "Additional plugin marketplaces to query. Keys are marketplace names; values are GitHub shorthand (`owner/repo[#ref]`) or Git URIs (`{url}[#ref]`)."), + value: nls.localize('chat.plugins.extraMarketplaces.policy', "Additional plugin marketplaces to query. Keys are marketplace names; values are GitHub shorthand (`owner/repo[#ref]`) or Git URIs (`{url}[#ref]`), optionally with an enterprise-managed auto-update override."), } }, }, diff --git a/src/vs/workbench/contrib/chat/browser/pluginAutoUpdate.ts b/src/vs/workbench/contrib/chat/browser/pluginAutoUpdate.ts index 8cfa7a9eaba..28c761987f1 100644 --- a/src/vs/workbench/contrib/chat/browser/pluginAutoUpdate.ts +++ b/src/vs/workbench/contrib/chat/browser/pluginAutoUpdate.ts @@ -8,7 +8,6 @@ import { Disposable } from '../../../../base/common/lifecycle.js'; import { autorun } from '../../../../base/common/observable.js'; import { ILogService } from '../../../../platform/log/common/log.js'; import { IWorkbenchContribution } from '../../../common/contributions.js'; -import { IExtensionsWorkbenchService } from '../../extensions/common/extensions.js'; import { IPluginInstallService } from '../common/plugins/pluginInstallService.js'; import { IPluginMarketplaceService } from '../common/plugins/pluginMarketplaceService.js'; @@ -17,21 +16,16 @@ import { IPluginMarketplaceService } from '../common/plugins/pluginMarketplaceSe * {@link IPluginMarketplaceService} with the plugin update *action* exposed * by {@link IPluginInstallService}. * - * The marketplace service flips `hasUpdatesAvailable` to `true` roughly once - * a day when at least one cloned plugin repository has upstream changes. + * The marketplace service reports canonical marketplace IDs roughly once + * a day when cloned plugin repositories have upstream changes. * Without this contribution, that signal was never consumed and plugins * were never auto-updated (see microsoft/vscode#308563). * - * When the signal becomes `true` and `extensions.autoUpdate` is `on`, we - * silently update all installed plugins. When auto-update is `off`, plugins - * are not auto-updated. (`getAutoUpdateValue()` normalizes the setting to - * `'on' | 'off'`, migrating any legacy stored values such as `false`.) + * Only plugins from the reported marketplaces are updated. The marketplace + * service applies managed per-marketplace policy before reporting updates. * - * The flag is cleared after every attempt — including failures — so the - * next periodic check's `false → true` transition can always re-trigger the - * autorun. `updateAllPlugins` already clears it on success; clearing again - * in `finally` is a no-op on the success path and handles the partial- - * failure path where the install service leaves the flag at `true`. + * Processed marketplace IDs are acknowledged after every attempt, including + * failures. IDs reported while an update is running remain queued. */ export class PluginAutoUpdate extends Disposable implements IWorkbenchContribution { static readonly ID = 'workbench.contrib.pluginAutoUpdate'; @@ -41,40 +35,32 @@ export class PluginAutoUpdate extends Disposable implements IWorkbenchContributi constructor( @IPluginMarketplaceService private readonly _pluginMarketplaceService: IPluginMarketplaceService, @IPluginInstallService private readonly _pluginInstallService: IPluginInstallService, - @IExtensionsWorkbenchService private readonly _extensionsWorkbenchService: IExtensionsWorkbenchService, @ILogService private readonly _logService: ILogService, ) { super(); this._register(autorun(reader => { - if (!this._pluginMarketplaceService.hasUpdatesAvailable.read(reader)) { + const marketplaceIds = this._pluginMarketplaceService.marketplacesWithUpdates.read(reader); + if (marketplaceIds.size === 0) { return; } - void this._triggerAutoUpdate(); + void this._triggerAutoUpdate(marketplaceIds); })); } - private async _triggerAutoUpdate(): Promise { + private async _triggerAutoUpdate(marketplaceIds: ReadonlySet): Promise { if (this._updateInFlight) { return; } - const autoUpdate = this._extensionsWorkbenchService.getAutoUpdateValue(); - if (autoUpdate === 'off') { - return; - } - this._updateInFlight = true; try { - await this._pluginInstallService.updateAllPlugins({ silent: true }, CancellationToken.None); + await this._pluginInstallService.updateAllPlugins({ silent: true, automatic: true, marketplaceIds }, CancellationToken.None); } catch (err) { this._logService.error('[PluginAutoUpdate] Failed to auto-update plugins:', err); } finally { this._updateInFlight = false; - // Ensure the flag is cleared even on partial failure so the next - // periodic check can re-arm the autorun via a `false → true` - // transition. - this._pluginMarketplaceService.clearUpdatesAvailable(); + this._pluginMarketplaceService.clearUpdatesAvailable(marketplaceIds); } } } diff --git a/src/vs/workbench/contrib/chat/browser/pluginInstallService.ts b/src/vs/workbench/contrib/chat/browser/pluginInstallService.ts index 6e856b17650..c6d5db4abd7 100644 --- a/src/vs/workbench/contrib/chat/browser/pluginInstallService.ts +++ b/src/vs/workbench/contrib/chat/browser/pluginInstallService.ts @@ -355,6 +355,14 @@ export class PluginInstallService implements IPluginInstallService { } async updatePlugin(plugin: IMarketplacePlugin, silent?: boolean): Promise { + if (this._pluginMarketplaceService.isStrictMarketplacePolicyActive() && !this._pluginMarketplaceService.isMarketplaceTrusted(plugin.marketplaceReference)) { + this._notificationService.notify({ + severity: Severity.Warning, + message: localize('strictMarketplaceBlockedUpdate', "Updates from '{0}' are blocked by your organization's policy.", plugin.marketplaceReference.displayLabel), + }); + return false; + } + const kind = plugin.sourceDescriptor.kind; if (kind === PluginSourceKind.Npm || kind === PluginSourceKind.Pip) { @@ -371,7 +379,11 @@ export class PluginInstallService implements IPluginInstallService { } async updateAllPlugins(options: IUpdateAllPluginsOptions, token: CancellationToken): Promise { - const installed = this._pluginMarketplaceService.installedPlugins.get(); + const allInstalled = this._pluginMarketplaceService.installedPlugins.get(); + const installed = allInstalled.filter(entry => + (!options.marketplaceIds || options.marketplaceIds.has(entry.plugin.marketplaceReference.canonicalId)) + && (!options.automatic || this._pluginMarketplaceService.isMarketplaceAutoUpdateEnabled(entry.plugin.marketplaceReference)) + ); if (installed.length === 0) { return { updatedNames: [], failedNames: [] }; } @@ -393,6 +405,10 @@ export class PluginInstallService implements IPluginInstallService { continue; } seenMarketplaces.add(ref.canonicalId); + if (this._pluginMarketplaceService.isStrictMarketplacePolicyActive() && !this._pluginMarketplaceService.isMarketplaceTrusted(ref)) { + failedNames.push(ref.displayLabel); + continue; + } gitTasks.push((async () => { if (token.isCancellationRequested) { return; @@ -419,7 +435,8 @@ export class PluginInstallService implements IPluginInstallService { // 2. Re-fetch marketplace data *after* pulling so we see any // updated plugin descriptors (new versions, refs, etc.). - const marketplacePlugins = await this._pluginMarketplaceService.fetchMarketplacePlugins(token); + const marketplaceIds = new Set(installed.map(entry => entry.plugin.marketplaceReference.canonicalId)); + const marketplacePlugins = await this._pluginMarketplaceService.fetchMarketplacePlugins(token, marketplaceIds); const marketplaceByKey = new Map(); for (const mp of marketplacePlugins) { marketplaceByKey.set(`${mp.marketplaceReference.canonicalId}::${mp.name}`, mp); @@ -513,13 +530,17 @@ export class PluginInstallService implements IPluginInstallService { }, }); } else if (updatedNames.length > 0) { - this._pluginMarketplaceService.clearUpdatesAvailable(); + if (!options.automatic) { + this._pluginMarketplaceService.clearUpdatesAvailable(options.marketplaceIds); + } this._notificationService.notify({ severity: Severity.Info, message: localize('updateAllSuccess', "Updated plugins: {0}", updatedNames.join(', ')), }); } else if (!token.isCancellationRequested) { - this._pluginMarketplaceService.clearUpdatesAvailable(); + if (!options.automatic) { + this._pluginMarketplaceService.clearUpdatesAvailable(options.marketplaceIds); + } } return { updatedNames, failedNames }; diff --git a/src/vs/workbench/contrib/chat/common/plugins/AGENTS_PLUGINS.md b/src/vs/workbench/contrib/chat/common/plugins/AGENTS_PLUGINS.md index 414dc2716ca..bc8432d8c3d 100644 --- a/src/vs/workbench/contrib/chat/common/plugins/AGENTS_PLUGINS.md +++ b/src/vs/workbench/contrib/chat/common/plugins/AGENTS_PLUGINS.md @@ -131,7 +131,7 @@ Manages the catalog of available and installed plugins: - **Fetch** — reads `chat.plugins.marketplaces` config (GitHub shorthand, Git URLs, or file URIs), fetches `marketplace.json` from each, and returns parsed `IMarketplacePlugin` entries. - **Installed storage** — persists installed plugins in application-scoped storage (`chat.plugins.installed.v1`). Each entry tracks `{ pluginUri, plugin, enabled }`. - **Trust** — marketplace canonical IDs must be explicitly trusted before install proceeds (`chat.plugins.trustedMarketplaces.v1`). -- **Auto-update** — checks for upstream changes approximately every 24 hours when `extensions.autoUpdate` is enabled; sets `hasUpdatesAvailable` observable. +- **Auto-update** — checks eligible installed marketplaces approximately every 24 hours and reports their canonical IDs through `marketplacesWithUpdates`. Managed `extraKnownMarketplaces..autoUpdate` values override `extensions.autoUpdate` for that marketplace; undefined entries inherit the global setting. Checks and updates are restricted to enabled marketplaces and still enforce `strictKnownMarketplaces`. - **GitHub caching** — caches raw GitHub API responses with an 8-hour TTL to avoid repeated fetches. ### Marketplace Definition Files diff --git a/src/vs/workbench/contrib/chat/common/plugins/marketplaceReference.ts b/src/vs/workbench/contrib/chat/common/plugins/marketplaceReference.ts index b1661cf786d..78a99fab76e 100644 --- a/src/vs/workbench/contrib/chat/common/plugins/marketplaceReference.ts +++ b/src/vs/workbench/contrib/chat/common/plugins/marketplaceReference.ts @@ -4,6 +4,7 @@ *--------------------------------------------------------------------------------------------*/ import { URI } from '../../../../../base/common/uri.js'; +import { ExtraKnownMarketplacesConfigDict, IExtraKnownMarketplaceConfigValue } from '../../../../../base/common/managedSettings.js'; import { IConfigurationService } from '../../../../../platform/configuration/common/configuration.js'; import { ChatConfiguration } from '../constants.js'; @@ -25,6 +26,7 @@ export interface IMarketplaceReference { readonly ref?: string; readonly githubRepo?: string; readonly localRepositoryUri?: URI; + readonly autoUpdate?: boolean; } /** @@ -53,12 +55,18 @@ export function readConfiguredMarketplaces(configurationService: IConfigurationS // `ChatExtraMarketplaces` is stored as `{ [name]: url-or-shorthand }` when delivered by // policy. Convert each entry to the nested IExtraMarketplaceObjectEntry shape so that // parseMarketplaceReferences can set displayLabel = name (critical for enabledPlugins keys). - const extraObj = configurationService.getValue>(ChatConfiguration.ExtraMarketplaces) ?? {}; - const extraValues: IExtraMarketplaceObjectEntry[] = Object.entries(extraObj).map(([name, src]) => { + const extraObj = configurationService.getValue(ChatConfiguration.ExtraMarketplaces) ?? {}; + const extraValues: IExtraMarketplaceObjectEntry[] = Object.entries(extraObj).flatMap(([name, value]) => { + if (typeof value !== 'string') { + return []; + } + const encoded = parseExtraMarketplaceConfigValue(value); + const src = encoded?.source ?? value; + const autoUpdate = encoded?.autoUpdate; const isGithubShorthand = _githubShorthandRe.test(src); - return isGithubShorthand - ? { name, source: { source: 'github' as const, repo: src } } - : { name, source: { source: 'git' as const, url: src } }; + return [isGithubShorthand + ? { name, autoUpdate, source: { source: 'github' as const, repo: src } } + : { name, autoUpdate, source: { source: 'git' as const, url: src } }]; }); return { @@ -68,6 +76,20 @@ export function readConfiguredMarketplaces(configurationService: IConfigurationS }; } +function parseExtraMarketplaceConfigValue(value: string): IExtraKnownMarketplaceConfigValue | undefined { + try { + const parsed = JSON.parse(value); + return parsed + && typeof parsed === 'object' + && typeof parsed.source === 'string' + && typeof parsed.autoUpdate === 'boolean' + ? parsed + : undefined; + } catch { + return undefined; + } +} + export function parseMarketplaceReferences(values: readonly unknown[]): IMarketplaceReference[] { const byCanonicalId = new Map(); @@ -78,8 +100,13 @@ export function parseMarketplaceReferences(values: readonly unknown[]): IMarketp } else if (value && typeof value === 'object') { parsed = parseMarketplaceObjectEntry(value as IExtraMarketplaceObjectEntry); } - if (parsed && !byCanonicalId.has(parsed.canonicalId)) { - byCanonicalId.set(parsed.canonicalId, parsed); + if (parsed) { + const existing = byCanonicalId.get(parsed.canonicalId); + if (!existing) { + byCanonicalId.set(parsed.canonicalId, parsed); + } else if (parsed.autoUpdate !== undefined) { + byCanonicalId.set(parsed.canonicalId, { ...existing, autoUpdate: parsed.autoUpdate }); + } } } @@ -101,6 +128,7 @@ export interface IExtraMarketplaceObjectEntry { readonly repo?: string; readonly url?: string; readonly ref?: string; + readonly autoUpdate?: boolean; } export function parseMarketplaceObjectEntry(entry: IExtraMarketplaceObjectEntry): IMarketplaceReference | undefined { @@ -132,6 +160,9 @@ export function parseMarketplaceObjectEntry(entry: IExtraMarketplaceObjectEntry) if (parsed && typeof entry.name === 'string' && entry.name.length > 0) { parsed = { ...parsed, displayLabel: entry.name }; } + if (parsed && typeof entry.autoUpdate === 'boolean') { + parsed = { ...parsed, autoUpdate: entry.autoUpdate }; + } return parsed; } diff --git a/src/vs/workbench/contrib/chat/common/plugins/pluginInstallService.ts b/src/vs/workbench/contrib/chat/common/plugins/pluginInstallService.ts index 7458cdc42cc..e3f3fe39b32 100644 --- a/src/vs/workbench/contrib/chat/common/plugins/pluginInstallService.ts +++ b/src/vs/workbench/contrib/chat/common/plugins/pluginInstallService.ts @@ -24,6 +24,12 @@ export interface IUpdateAllPluginsOptions { * updated, and error notifications are shown on failure. */ readonly silent?: boolean; + + /** Restricts updates to plugins installed from these canonical marketplace IDs. */ + readonly marketplaceIds?: ReadonlySet; + + /** Rechecks marketplace automatic-update policy before updating. */ + readonly automatic?: boolean; } export interface IUpdateAllPluginsResult { diff --git a/src/vs/workbench/contrib/chat/common/plugins/pluginMarketplaceService.ts b/src/vs/workbench/contrib/chat/common/plugins/pluginMarketplaceService.ts index e687f082e4a..d4c9113e0d0 100644 --- a/src/vs/workbench/contrib/chat/common/plugins/pluginMarketplaceService.ts +++ b/src/vs/workbench/contrib/chat/common/plugins/pluginMarketplaceService.ts @@ -149,12 +149,8 @@ export interface IPluginMarketplaceService { readonly onDidChangeMarketplaces: Event; /** Installed marketplace plugins, backed by storage. */ readonly installedPlugins: IObservable; - /** - * Observable that is `true` when at least one cloned marketplace - * repository has upstream changes available. Checked periodically - * (approximately once per day) when `extensions.autoUpdate` is enabled. - */ - readonly hasUpdatesAvailable: IObservable; + /** Canonical IDs of marketplaces with updates detected by the periodic check. */ + readonly marketplacesWithUpdates: IObservable>; /** * Observable snapshot of the last {@link fetchMarketplacePlugins} result. * Empty until the first fetch completes. Views should use this for @@ -167,9 +163,9 @@ export interface IPluginMarketplaceService { * may be added over time; consumers should not assume a specific source. */ readonly recommendedPlugins: IObservable>; - /** Resets {@link hasUpdatesAvailable} to `false`. */ - clearUpdatesAvailable(): void; - fetchMarketplacePlugins(token: CancellationToken): Promise; + /** Clears all reported marketplaces, or only the provided canonical IDs. */ + clearUpdatesAvailable(marketplaceIds?: ReadonlySet): void; + fetchMarketplacePlugins(token: CancellationToken, marketplaceIds?: ReadonlySet): Promise; getMarketplacePluginMetadata(pluginUri: URI): IMarketplacePlugin | undefined; addInstalledPlugin(pluginUri: URI, plugin: IMarketplacePlugin): void; removeInstalledPlugin(pluginUri: URI): void; @@ -181,6 +177,8 @@ export interface IPluginMarketplaceService { * configured. When active, blocked marketplaces cannot be trusted by the user. */ isStrictMarketplacePolicyActive(): boolean; + /** Returns the effective automatic-update policy for a marketplace. */ + isMarketplaceAutoUpdateEnabled(ref: IMarketplaceReference): boolean; /** Records that the user trusts the given marketplace, persisted permanently. */ trustMarketplace(ref: IMarketplaceReference): void; /** @@ -301,13 +299,13 @@ export class PluginMarketplaceService extends Disposable implements IPluginMarke private readonly _pluginMetadata = new Map(); private readonly _trustedMarketplacesStore: ObservableMemento; private readonly _lastFetchedPluginsStore: ObservableMemento; - private readonly _hasUpdatesAvailable = observableValue('hasUpdatesAvailable', false); + private readonly _marketplacesWithUpdates = observableValue>('marketplacesWithUpdates', new Set()); private _updateCheckTimer: ReturnType | undefined; readonly onDidChangeMarketplaces: Event; readonly installedPlugins: IObservable; - readonly hasUpdatesAvailable: IObservable = this._hasUpdatesAvailable; + readonly marketplacesWithUpdates: IObservable> = this._marketplacesWithUpdates; readonly lastFetchedPlugins: IObservable; readonly recommendedPlugins: IObservable>; @@ -391,12 +389,16 @@ export class PluginMarketplaceService extends Disposable implements IPluginMarke ); this._register(runWhenGlobalIdle(() => { - // Schedule periodic update checks when auto-update is enabled. this._scheduleUpdateCheck(); this._register(Event.filter( _configurationService.onDidChangeConfiguration, - e => e.affectsConfiguration(AutoUpdateConfigurationKey), - )(() => this._scheduleUpdateCheck())); + e => e.affectsConfiguration(AutoUpdateConfigurationKey) + || e.affectsConfiguration(ChatConfiguration.ExtraMarketplaces) + || e.affectsConfiguration(ChatConfiguration.StrictMarketplaces), + )(() => { + this.clearUpdatesAvailable(); + this._scheduleUpdateCheck(); + })); })); // Hydrate plugin metadata for installed entries that are not yet in @@ -420,11 +422,16 @@ export class PluginMarketplaceService extends Disposable implements IPluginMarke super.dispose(); } - clearUpdatesAvailable(): void { - this._hasUpdatesAvailable.set(false, undefined); + clearUpdatesAvailable(marketplaceIds?: ReadonlySet): void { + if (!marketplaceIds) { + this._marketplacesWithUpdates.set(new Set(), undefined); + return; + } + const remaining = new Set([...this._marketplacesWithUpdates.get()].filter(id => !marketplaceIds.has(id))); + this._marketplacesWithUpdates.set(remaining, undefined); } - async fetchMarketplacePlugins(token: CancellationToken): Promise { + async fetchMarketplacePlugins(token: CancellationToken, marketplaceIds?: ReadonlySet): Promise { if (!this._configurationService.getValue(ChatConfiguration.PluginsEnabled)) { return []; } @@ -456,8 +463,12 @@ export class PluginMarketplaceService extends Disposable implements IPluginMarke } } + const refsToFetch = allRefs.filter(ref => + (!marketplaceIds || marketplaceIds.has(ref.canonicalId)) + && this._isMarketplaceAllowedByStrictPolicy(ref) + ); const results = await Promise.all( - allRefs.map(ref => { + refsToFetch.map(ref => { if (ref.kind === MarketplaceReferenceKind.GitHubShorthand && ref.githubRepo) { return this._fetchFromGitHubRepo(ref, ref.githubRepo, token); } @@ -465,7 +476,10 @@ export class PluginMarketplaceService extends Disposable implements IPluginMarke }) ); const plugins = results.flat(); - this._lastFetchedPluginsStore.set({ plugins, fetchedAt: Date.now() }, undefined); + const storedPlugins = marketplaceIds + ? [...this.lastFetchedPlugins.get().filter(plugin => !marketplaceIds.has(plugin.marketplaceReference.canonicalId)), ...plugins] + : plugins; + this._lastFetchedPluginsStore.set({ plugins: storedPlugins, fetchedAt: Date.now() }, undefined); return plugins; } @@ -646,6 +660,16 @@ export class PluginMarketplaceService extends Disposable implements IPluginMarke return getStrictKnownMarketplaces(this._configurationService.getValue(ChatConfiguration.StrictMarketplaces)) !== undefined; } + isMarketplaceAutoUpdateEnabled(ref: IMarketplaceReference): boolean { + const { extraValues } = readConfiguredMarketplaces(this._configurationService); + const managedRef = parseMarketplaceReferences(extraValues).find(candidate => candidate.canonicalId === ref.canonicalId); + return managedRef?.autoUpdate ?? this._extensionsWorkbenchService.getAutoUpdateValue() !== 'off'; + } + + private _isMarketplaceAllowedByStrictPolicy(ref: IMarketplaceReference): boolean { + return !this.isStrictMarketplacePolicyActive() || this.isMarketplaceTrusted(ref); + } + // --- Plugin metadata hydration ----------------------------------------------- /** @@ -760,8 +784,12 @@ export class PluginMarketplaceService extends Disposable implements IPluginMarke // --- Periodic update check ------------------------------------------------ - private _isAutoUpdateEnabled(): boolean { - return this._extensionsWorkbenchService.getAutoUpdateValue() !== 'off'; + private _hasAutoUpdateEnabledMarketplace(): boolean { + if (this._extensionsWorkbenchService.getAutoUpdateValue() !== 'off') { + return true; + } + const { extraValues } = readConfiguredMarketplaces(this._configurationService); + return parseMarketplaceReferences(extraValues).some(ref => ref.autoUpdate === true); } /** @@ -774,7 +802,7 @@ export class PluginMarketplaceService extends Disposable implements IPluginMarke this._updateCheckTimer = undefined; } - if (!this._isAutoUpdateEnabled()) { + if (!this._hasAutoUpdateEnabledMarketplace()) { return; } @@ -799,11 +827,13 @@ export class PluginMarketplaceService extends Disposable implements IPluginMarke } const seenMarketplaces = new Set(); - let hasUpdates = false; + const marketplacesWithUpdates = new Set(); for (const entry of installed) { const ref = entry.plugin.marketplaceReference; - if (seenMarketplaces.has(ref.canonicalId)) { + if (seenMarketplaces.has(ref.canonicalId) + || !this.isMarketplaceAutoUpdateEnabled(ref) + || !this._isMarketplaceAllowedByStrictPolicy(ref)) { continue; } seenMarketplaces.add(ref.canonicalId); @@ -811,15 +841,14 @@ export class PluginMarketplaceService extends Disposable implements IPluginMarke try { const behind = await this._pluginRepositoryService.fetchRepository(ref); if (behind) { - hasUpdates = true; - break; + marketplacesWithUpdates.add(ref.canonicalId); } } catch (err) { this._logService.debug(`[PluginMarketplaceService] Update check failed for ${ref.displayLabel}:`, err); } } - this._hasUpdatesAvailable.set(hasUpdates, undefined); + this._marketplacesWithUpdates.set(marketplacesWithUpdates, undefined); this._storageService.store( PLUGIN_UPDATE_LAST_CHECK_STORAGE_KEY, Date.now(), @@ -830,7 +859,7 @@ export class PluginMarketplaceService extends Disposable implements IPluginMarke this._logService.debug('[PluginMarketplaceService] Periodic update check failed:', err); } finally { // Reschedule for the next check - if (this._isAutoUpdateEnabled()) { + if (this._hasAutoUpdateEnabledMarketplace()) { this._updateCheckTimer = setTimeout(() => this._runUpdateCheck(), PLUGIN_UPDATE_CHECK_INTERVAL_MS); } } diff --git a/src/vs/workbench/contrib/chat/test/browser/plugins/pluginAutoUpdate.test.ts b/src/vs/workbench/contrib/chat/test/browser/plugins/pluginAutoUpdate.test.ts index 9ef9d37b329..c88cff13d66 100644 --- a/src/vs/workbench/contrib/chat/test/browser/plugins/pluginAutoUpdate.test.ts +++ b/src/vs/workbench/contrib/chat/test/browser/plugins/pluginAutoUpdate.test.ts @@ -9,7 +9,6 @@ import { observableValue } from '../../../../../../base/common/observable.js'; import { ensureNoDisposablesAreLeakedInTestSuite } from '../../../../../../base/test/common/utils.js'; import { TestInstantiationService } from '../../../../../../platform/instantiation/test/common/instantiationServiceMock.js'; import { ILogService, NullLogService } from '../../../../../../platform/log/common/log.js'; -import { AutoUpdateConfigurationValue, IExtensionsWorkbenchService } from '../../../../extensions/common/extensions.js'; import { PluginAutoUpdate } from '../../../browser/pluginAutoUpdate.js'; import { IPluginInstallService, IUpdateAllPluginsOptions, IUpdateAllPluginsResult } from '../../../common/plugins/pluginInstallService.js'; import { IPluginMarketplaceService } from '../../../common/plugins/pluginMarketplaceService.js'; @@ -18,28 +17,29 @@ suite('PluginAutoUpdate', () => { const store = ensureNoDisposablesAreLeakedInTestSuite(); interface MockState { - hasUpdatesAvailable: ReturnType>; + marketplacesWithUpdates: ReturnType>>; updateAllCalls: IUpdateAllPluginsOptions[]; updateAllImpl: () => Promise; - clearUpdatesAvailableCalls: number; + clearUpdatesAvailableCalls: ReadonlySet[]; } - function createContribution(autoUpdate: AutoUpdateConfigurationValue, stateOverrides?: Partial): { contribution: PluginAutoUpdate; state: MockState } { + function createContribution(stateOverrides?: Partial): { contribution: PluginAutoUpdate; state: MockState } { const instantiationService = store.add(new TestInstantiationService()); const state: MockState = { - hasUpdatesAvailable: observableValue('test.hasUpdatesAvailable', false), + marketplacesWithUpdates: observableValue>('test.marketplacesWithUpdates', new Set()), updateAllCalls: [], updateAllImpl: async () => ({ updatedNames: [], failedNames: [] }), - clearUpdatesAvailableCalls: 0, + clearUpdatesAvailableCalls: [], ...stateOverrides, }; instantiationService.stub(IPluginMarketplaceService, { - hasUpdatesAvailable: state.hasUpdatesAvailable, - clearUpdatesAvailable: () => { - state.clearUpdatesAvailableCalls++; - state.hasUpdatesAvailable.set(false, undefined); + marketplacesWithUpdates: state.marketplacesWithUpdates, + clearUpdatesAvailable: marketplaceIds => { + state.clearUpdatesAvailableCalls.push(marketplaceIds ?? new Set()); + const remaining = new Set([...state.marketplacesWithUpdates.get()].filter(id => !marketplaceIds?.has(id))); + state.marketplacesWithUpdates.set(remaining, undefined); }, } as Partial as IPluginMarketplaceService); @@ -50,10 +50,6 @@ suite('PluginAutoUpdate', () => { }, } as Partial as IPluginInstallService); - instantiationService.stub(IExtensionsWorkbenchService, { - getAutoUpdateValue: () => autoUpdate, - } as Partial as IExtensionsWorkbenchService); - instantiationService.stub(ILogService, new NullLogService()); const contribution = store.add(instantiationService.createInstance(PluginAutoUpdate)); @@ -66,93 +62,79 @@ suite('PluginAutoUpdate', () => { } test('does not trigger update on construction', async () => { - const { state } = createContribution('on'); + const { state } = createContribution(); await flushMicrotasks(); assert.deepStrictEqual(state.updateAllCalls, []); }); - test('triggers silent updateAllPlugins when hasUpdatesAvailable becomes true', async () => { - const { state } = createContribution('on'); + test('triggers a targeted silent update when a marketplace reports updates', async () => { + const { state } = createContribution(); - state.hasUpdatesAvailable.set(true, undefined); + state.marketplacesWithUpdates.set(new Set(['github:microsoft/plugins']), undefined); await flushMicrotasks(); - assert.deepStrictEqual(state.updateAllCalls, [{ silent: true }]); + assert.deepStrictEqual(state.updateAllCalls.map(call => ({ + silent: call.silent, + automatic: call.automatic, + marketplaceIds: [...call.marketplaceIds ?? []], + })), [{ silent: true, automatic: true, marketplaceIds: ['github:microsoft/plugins'] }]); }); - test('does not trigger update when extensions.autoUpdate is off', async () => { - const { state } = createContribution('off'); - - state.hasUpdatesAvailable.set(true, undefined); - await flushMicrotasks(); - - assert.deepStrictEqual(state.updateAllCalls, []); - }); - - test('triggers update when extensions.autoUpdate is on', async () => { - const { state } = createContribution('on'); - - state.hasUpdatesAvailable.set(true, undefined); - await flushMicrotasks(); - - assert.deepStrictEqual(state.updateAllCalls, [{ silent: true }]); - }); - - test('does not run a second update concurrently with one in flight', async () => { + test('queues a marketplace reported while another update is in flight', async () => { let resolveUpdate!: () => void; const pendingUpdate = new Promise(resolve => { resolveUpdate = () => resolve({ updatedNames: [], failedNames: [] }); }); - const { state } = createContribution('on', { - updateAllImpl: () => pendingUpdate, + let updateCount = 0; + const { state } = createContribution({ + updateAllImpl: () => updateCount++ === 0 ? pendingUpdate : Promise.resolve({ updatedNames: [], failedNames: [] }), }); - state.hasUpdatesAvailable.set(true, undefined); + state.marketplacesWithUpdates.set(new Set(['a']), undefined); await flushMicrotasks(); - // While the first update is still pending, simulate a redundant signal - // (e.g. another periodic check firing). Observable de-dupes equal - // values, so toggle false→true to force the autorun to re-run. - state.hasUpdatesAvailable.set(false, undefined); - state.hasUpdatesAvailable.set(true, undefined); + state.marketplacesWithUpdates.set(new Set(['a', 'b']), undefined); await flushMicrotasks(); assert.strictEqual(state.updateAllCalls.length, 1, 'should not start a second concurrent update'); resolveUpdate(); await pendingUpdate; + await flushMicrotasks(); + await flushMicrotasks(); + assert.deepStrictEqual(state.updateAllCalls.map(call => [...call.marketplaceIds ?? []]), [['a'], ['b']]); }); test('continues running on subsequent cycles after the previous update finished', async () => { - const { state } = createContribution('on'); + const { state } = createContribution(); - state.hasUpdatesAvailable.set(true, undefined); + state.marketplacesWithUpdates.set(new Set(['a']), undefined); await flushMicrotasks(); assert.strictEqual(state.updateAllCalls.length, 1); // Simulate `updateAllPlugins` clearing the flag, then the next // periodic check finding updates again. - state.hasUpdatesAvailable.set(false, undefined); + state.marketplacesWithUpdates.set(new Set(), undefined); await flushMicrotasks(); - state.hasUpdatesAvailable.set(true, undefined); + state.marketplacesWithUpdates.set(new Set(['a']), undefined); await flushMicrotasks(); assert.strictEqual(state.updateAllCalls.length, 2); }); test('swallows errors from updateAllPlugins', async () => { - const { state } = createContribution('on', { + const { state } = createContribution({ updateAllImpl: async () => { throw new Error('boom'); }, }); - state.hasUpdatesAvailable.set(true, undefined); + state.marketplacesWithUpdates.set(new Set(['a']), undefined); // Wait long enough for the rejected promise to settle. await flushMicrotasks(); await flushMicrotasks(); assert.strictEqual(state.updateAllCalls.length, 1); // A subsequent cycle should still work after the failure. - state.hasUpdatesAvailable.set(false, undefined); - state.hasUpdatesAvailable.set(true, undefined); + state.marketplacesWithUpdates.set(new Set(), undefined); + state.marketplacesWithUpdates.set(new Set(['a']), undefined); await flushMicrotasks(); await flushMicrotasks(); assert.strictEqual(state.updateAllCalls.length, 2); @@ -163,36 +145,37 @@ suite('PluginAutoUpdate', () => { // path in `PluginInstallService.updateAllPlugins`). Without our own // clear in `finally`, the observable would stay stuck at `true` and // the next periodic check's `set(true)` would not notify subscribers. - const { state } = createContribution('on', { + const { state } = createContribution({ updateAllImpl: async () => ({ updatedNames: [], failedNames: ['plugin-a'] }), }); - state.hasUpdatesAvailable.set(true, undefined); + state.marketplacesWithUpdates.set(new Set(['a']), undefined); await flushMicrotasks(); await flushMicrotasks(); assert.strictEqual(state.updateAllCalls.length, 1); - assert.strictEqual(state.clearUpdatesAvailableCalls, 1); - assert.strictEqual(state.hasUpdatesAvailable.get(), false); + assert.strictEqual(state.clearUpdatesAvailableCalls.length, 1); + assert.deepStrictEqual([...state.clearUpdatesAvailableCalls[0]], ['a']); + assert.strictEqual(state.marketplacesWithUpdates.get().size, 0); // The next periodic check finds updates again; the cleared flag lets // the autorun re-fire via a clean `false → true` transition. - state.hasUpdatesAvailable.set(true, undefined); + state.marketplacesWithUpdates.set(new Set(['a']), undefined); await flushMicrotasks(); await flushMicrotasks(); assert.strictEqual(state.updateAllCalls.length, 2); }); test('clears the flag even when updateAllPlugins throws', async () => { - const { state } = createContribution('on', { + const { state } = createContribution({ updateAllImpl: async () => { throw new Error('boom'); }, }); - state.hasUpdatesAvailable.set(true, undefined); + state.marketplacesWithUpdates.set(new Set(['a']), undefined); await flushMicrotasks(); await flushMicrotasks(); - assert.strictEqual(state.clearUpdatesAvailableCalls, 1); - assert.strictEqual(state.hasUpdatesAvailable.get(), false); + assert.strictEqual(state.clearUpdatesAvailableCalls.length, 1); + assert.strictEqual(state.marketplacesWithUpdates.get().size, 0); }); }); diff --git a/src/vs/workbench/contrib/chat/test/browser/plugins/pluginInstallService.test.ts b/src/vs/workbench/contrib/chat/test/browser/plugins/pluginInstallService.test.ts index 5f9b224dce4..337ea4986f5 100644 --- a/src/vs/workbench/contrib/chat/test/browser/plugins/pluginInstallService.test.ts +++ b/src/vs/workbench/contrib/chat/test/browser/plugins/pluginInstallService.test.ts @@ -4,7 +4,9 @@ *--------------------------------------------------------------------------------------------*/ import assert from 'assert'; +import { CancellationToken } from '../../../../../../base/common/cancellation.js'; import { isCancellationError } from '../../../../../../base/common/errors.js'; +import { observableValue } from '../../../../../../base/common/observable.js'; import { URI } from '../../../../../../base/common/uri.js'; import { ensureNoDisposablesAreLeakedInTestSuite } from '../../../../../../base/test/common/utils.js'; import { IConfigurationService } from '../../../../../../platform/configuration/common/configuration.js'; @@ -20,7 +22,7 @@ import { ITerminalService } from '../../../../terminal/browser/terminal.js'; import { PluginInstallService } from '../../../browser/pluginInstallService.js'; import { IAgentPluginRepositoryService, IEnsureRepositoryOptions, IPullRepositoryOptions } from '../../../common/plugins/agentPluginRepositoryService.js'; import { ChatConfiguration } from '../../../common/constants.js'; -import { IMarketplacePlugin, IMarketplaceReference, IPluginMarketplaceService, IPluginSourceDescriptor, MarketplaceType, parseMarketplaceReference, PluginSourceKind } from '../../../common/plugins/pluginMarketplaceService.js'; +import { IMarketplaceInstalledPlugin, IMarketplacePlugin, IMarketplaceReference, IPluginMarketplaceService, IPluginSourceDescriptor, MarketplaceType, parseMarketplaceReference, PluginSourceKind } from '../../../common/plugins/pluginMarketplaceService.js'; import { IPluginSource } from '../../../common/plugins/pluginSource.js'; suite('PluginInstallService', () => { @@ -71,6 +73,11 @@ suite('PluginInstallService', () => { marketplaceTrusted: boolean; /** Whether the strict-marketplace enterprise policy is active */ strictMarketplacePolicyActive?: boolean; + installedPlugins: IMarketplaceInstalledPlugin[]; + fetchedMarketplacePlugins: IMarketplacePlugin[]; + fetchMarketplaceCalls: string[][]; + autoUpdateByMarketplace: Map; + clearUpdatesAvailableCalls: number; /** Canonical IDs that were trusted via trustMarketplace() */ trustedMarketplaces: string[]; /** Plugins returned by readPluginsFromDirectory */ @@ -113,6 +120,11 @@ suite('PluginInstallService', () => { updatePluginSourceCalls: [], marketplaceTrusted: true, strictMarketplacePolicyActive: false, + installedPlugins: [], + fetchedMarketplacePlugins: [], + fetchMarketplaceCalls: [], + autoUpdateByMarketplace: new Map(), + clearUpdatesAvailableCalls: 0, trustedMarketplaces: [], readPluginsResult: [], singlePluginManifestResult: undefined, @@ -145,8 +157,9 @@ suite('PluginInstallService', () => { // INotificationService instantiationService.stub(INotificationService, { - notify: (notification: { severity: number; message: string }) => { + notify: (notification: { severity: number; message: string; actions?: { primary?: readonly { dispose(): void }[] } }) => { state.notifications.push({ severity: notification.severity, message: notification.message }); + notification.actions?.primary?.forEach(action => action.dispose()); return undefined; }, } as unknown as INotificationService); @@ -285,11 +298,18 @@ suite('PluginInstallService', () => { // IPluginMarketplaceService instantiationService.stub(IPluginMarketplaceService, { + installedPlugins: observableValue('test.installedPlugins', state.installedPlugins), addInstalledPlugin: (uri: URI, plugin: IMarketplacePlugin) => { state.addedPlugins.push({ uri: uri.toString(), plugin }); }, isMarketplaceTrusted: () => state.marketplaceTrusted, isStrictMarketplacePolicyActive: () => state.strictMarketplacePolicyActive ?? false, + isMarketplaceAutoUpdateEnabled: (ref: IMarketplaceReference) => state.autoUpdateByMarketplace.get(ref.canonicalId) ?? true, + fetchMarketplacePlugins: async (_token: CancellationToken, marketplaceIds?: ReadonlySet) => { + state.fetchMarketplaceCalls.push([...marketplaceIds ?? []]); + return state.fetchedMarketplacePlugins.filter(plugin => !marketplaceIds || marketplaceIds.has(plugin.marketplaceReference.canonicalId)); + }, + clearUpdatesAvailable: () => state.clearUpdatesAvailableCalls++, trustMarketplace: (ref: IMarketplaceReference) => { state.trustedMarketplaces.push(ref.canonicalId); }, @@ -747,6 +767,28 @@ suite('PluginInstallService', () => { assert.strictEqual(state.updatePluginSourceCalls.length, 1); }); + test('blocks direct updates when the strict marketplace policy disallows the source', async () => { + const { service, state } = createService({ + strictMarketplacePolicyActive: true, + marketplaceTrusted: false, + }); + const plugin = createPlugin({ + sourceDescriptor: { kind: PluginSourceKind.GitHub, repo: 'owner/repo' }, + }); + + const updated = await service.updatePlugin(plugin); + + assert.deepStrictEqual({ + updated, + updateCalls: state.updatePluginSourceCalls.length, + notifications: state.notifications.map(notification => notification.message), + }, { + updated: false, + updateCalls: 0, + notifications: ['Updates from \'microsoft/vscode\' are blocked by your organization\'s policy.'], + }); + }); + test('re-installs for npm plugin updates', async () => { const { service, state } = createService({ ensurePluginSourceResult: URI.file('/cache/agentPlugins/npm/my-pkg'), @@ -813,6 +855,72 @@ suite('PluginInstallService', () => { }); }); + suite('updateAllPlugins', () => { + + function installedPlugin(name: string, marketplace: string): IMarketplaceInstalledPlugin { + const marketplaceReference = makeMarketplaceRef(marketplace); + const plugin = createPlugin({ + name, + marketplace, + marketplaceReference, + source: `plugins/${name}`, + sourceDescriptor: { kind: PluginSourceKind.RelativePath, path: `plugins/${name}` }, + }); + return { pluginUri: URI.file(`/plugins/${name}`), plugin }; + } + + test('updates only the targeted marketplace', async () => { + const first = installedPlugin('first', 'microsoft/first'); + const second = installedPlugin('second', 'microsoft/second'); + const { service, state } = createService({ installedPlugins: [first, second] }); + + await service.updateAllPlugins({ + silent: true, + automatic: true, + marketplaceIds: new Set([first.plugin.marketplaceReference.canonicalId]), + }, CancellationToken.None); + + assert.deepStrictEqual({ + pulled: state.pullRepositoryCalls.map(call => call.marketplace.canonicalId), + fetched: state.fetchMarketplaceCalls, + }, { + pulled: [first.plugin.marketplaceReference.canonicalId], + fetched: [[first.plugin.marketplaceReference.canonicalId]], + }); + }); + + test('rechecks managed auto-update policy before an automatic update', async () => { + const installed = installedPlugin('blocked', 'microsoft/blocked'); + const { service, state } = createService({ + installedPlugins: [installed], + autoUpdateByMarketplace: new Map([[installed.plugin.marketplaceReference.canonicalId, false]]), + }); + + await service.updateAllPlugins({ + silent: true, + automatic: true, + marketplaceIds: new Set([installed.plugin.marketplaceReference.canonicalId]), + }, CancellationToken.None); + + assert.deepStrictEqual(state.pullRepositoryCalls, []); + assert.deepStrictEqual(state.fetchMarketplaceCalls, []); + }); + + test('blocks updates when the strict marketplace policy disallows the source', async () => { + const installed = installedPlugin('blocked', 'microsoft/blocked'); + const { service, state } = createService({ + installedPlugins: [installed], + strictMarketplacePolicyActive: true, + marketplaceTrusted: false, + }); + + const result = await service.updateAllPlugins({ silent: true }, CancellationToken.None); + + assert.deepStrictEqual(result.failedNames, [installed.plugin.marketplaceReference.displayLabel]); + assert.deepStrictEqual(state.pullRepositoryCalls, []); + }); + }); + // ========================================================================= // installPlugin — marketplace trust // ========================================================================= diff --git a/src/vs/workbench/contrib/chat/test/common/plugins/pluginMarketplaceService.test.ts b/src/vs/workbench/contrib/chat/test/common/plugins/pluginMarketplaceService.test.ts index 67e18e76be7..37f073e74b0 100644 --- a/src/vs/workbench/contrib/chat/test/common/plugins/pluginMarketplaceService.test.ts +++ b/src/vs/workbench/contrib/chat/test/common/plugins/pluginMarketplaceService.test.ts @@ -22,7 +22,7 @@ import { IRequestService } from '../../../../../../platform/request/common/reque import { IStorageService, InMemoryStorageService, StorageScope, StorageTarget } from '../../../../../../platform/storage/common/storage.js'; import { IWorkspaceTrustManagementService } from '../../../../../../platform/workspace/common/workspaceTrust.js'; import { IEnvironmentService } from '../../../../../../platform/environment/common/environment.js'; -import { IExtensionsWorkbenchService } from '../../../../extensions/common/extensions.js'; +import { AutoUpdateConfigurationValue, IExtensionsWorkbenchService } from '../../../../extensions/common/extensions.js'; import { ChatConfiguration } from '../../../common/constants.js'; import { IAgentPluginRepositoryService } from '../../../common/plugins/agentPluginRepositoryService.js'; import { IMarketplacePlugin, IMarketplaceReference, IPluginSourceDescriptor, MarketplaceReferenceKind, MarketplaceType, PluginMarketplaceService, PluginSourceKind, extraKnownMarketplacesToConfigDict, getPluginSourceLabel, parseMarketplaceReference, parseMarketplaceReferences, parsePluginSource, readConfiguredMarketplaces } from '../../../common/plugins/pluginMarketplaceService.js'; @@ -180,9 +180,10 @@ suite('PluginMarketplaceService', () => { test('readConfiguredMarketplaces converts policy dict to named marketplace entries', () => { const configService = new TestConfigurationService({ [ChatConfiguration.ExtraMarketplaces]: { - 'acme-internal': 'https://plugins.internal.acme.com', - 'acme-public': 'https://copilot-plugins.acme.io', + 'acme-internal': '{"source":"https://plugins.internal.acme.com","autoUpdate":true}', + 'acme-public': '{"source":"https://copilot-plugins.acme.io","autoUpdate":false}', 'vscode-team-kit': 'microsoft/vscode-team-kit', + 'invalid': null, }, }); const { extraValues, effectiveValues } = readConfiguredMarketplaces(configService as unknown as IConfigurationService); @@ -191,6 +192,7 @@ suite('PluginMarketplaceService', () => { assert.deepStrictEqual(refs.map(r => r.displayLabel), ['acme-internal', 'acme-public', 'vscode-team-kit']); assert.strictEqual(refs[0].kind, MarketplaceReferenceKind.GitUri); assert.strictEqual(refs[2].kind, MarketplaceReferenceKind.GitHubShorthand); + assert.deepStrictEqual(refs.map(r => r.autoUpdate), [true, false, undefined]); // Effective values union user + extra assert.strictEqual(effectiveValues.length, extraValues.length); }); @@ -207,6 +209,31 @@ suite('PluginMarketplaceService', () => { assert.deepStrictEqual(dict, { 'vscode-team-kit': 'microsoft/vscode-team-kit' }); }); + test('extraKnownMarketplacesToConfigDict: preserves explicit autoUpdate values', () => { + const dict = extraKnownMarketplacesToConfigDict([ + { name: 'always', autoUpdate: true, source: { source: 'github', repo: 'microsoft/always' } }, + { name: 'never', autoUpdate: false, source: { source: 'github', repo: 'microsoft/never' } }, + { name: 'default', source: { source: 'github', repo: 'microsoft/default' } }, + ]); + assert.deepStrictEqual(dict, { + always: '{"source":"microsoft/always","autoUpdate":true}', + never: '{"source":"microsoft/never","autoUpdate":false}', + default: 'microsoft/default', + }); + }); + + test('managed autoUpdate survives a duplicate user marketplace reference', () => { + const configService = new TestConfigurationService({ + [ChatConfiguration.PluginMarketplaces]: ['microsoft/plugins'], + [ChatConfiguration.ExtraMarketplaces]: { + managed: '{"source":"microsoft/plugins","autoUpdate":true}', + }, + }); + const refs = parseMarketplaceReferences(readConfiguredMarketplaces(configService as unknown as IConfigurationService).effectiveValues); + assert.strictEqual(refs.length, 1); + assert.strictEqual(refs[0].autoUpdate, true); + }); + test('extraKnownMarketplacesToConfigDict: github source with ref appends #ref', () => { const dict = extraKnownMarketplacesToConfigDict([ { name: 'team-kit-beta', source: { source: 'github', repo: 'microsoft/vscode-team-kit', ref: 'beta' } }, @@ -498,11 +525,12 @@ suite('PluginMarketplaceService - getMarketplacePluginMetadata', () => { const marketplaceRef = parseMarketplaceReference('microsoft/plugins')!; - function createService(): PluginMarketplaceService { + function createService(autoUpdate: AutoUpdateConfigurationValue = 'on', extraMarketplaces: Record = {}): PluginMarketplaceService { const instantiationService = store.add(new TestInstantiationService()); instantiationService.stub(IConfigurationService, new TestConfigurationService({ [ChatConfiguration.PluginMarketplaces]: ['microsoft/plugins'], + [ChatConfiguration.ExtraMarketplaces]: extraMarketplaces, [ChatConfiguration.PluginsEnabled]: true, })); instantiationService.stub(IEnvironmentService, { cacheHome: URI.file('/cache') } as Partial as IEnvironmentService); @@ -520,7 +548,7 @@ suite('PluginMarketplaceService - getMarketplacePluginMetadata', () => { onDidChangeTrust: Event.None, } as Partial as IWorkspaceTrustManagementService); instantiationService.stub(IExtensionsWorkbenchService, { - getAutoUpdateValue: () => 'on', + getAutoUpdateValue: () => autoUpdate, } as Partial as IExtensionsWorkbenchService); return store.add(instantiationService.createInstance(PluginMarketplaceService)); @@ -557,6 +585,26 @@ suite('PluginMarketplaceService - getMarketplacePluginMetadata', () => { const result = service.getMarketplacePluginMetadata(URI.file('/any/path')); assert.strictEqual(result, undefined); }); + + test('managed marketplace autoUpdate overrides the global setting by canonical identity', () => { + const service = createService('off', { + always: '{"source":"microsoft/always","autoUpdate":true}', + never: '{"source":"microsoft/never","autoUpdate":false}', + inherited: 'microsoft/inherited', + }); + + assert.deepStrictEqual({ + always: service.isMarketplaceAutoUpdateEnabled(parseMarketplaceReference('https://github.com/microsoft/always.git')!), + never: service.isMarketplaceAutoUpdateEnabled(parseMarketplaceReference('microsoft/never')!), + inherited: service.isMarketplaceAutoUpdateEnabled(parseMarketplaceReference('microsoft/inherited')!), + unmanaged: service.isMarketplaceAutoUpdateEnabled(parseMarketplaceReference('microsoft/unmanaged')!), + }, { + always: true, + never: false, + inherited: false, + unmanaged: false, + }); + }); }); suite('PluginMarketplaceService - installed plugins lifecycle', () => {