diff --git a/src/vs/platform/agentPlugins/common/pluginParsers.ts b/src/vs/platform/agentPlugins/common/pluginParsers.ts index 3c8407d330784..1d6d8b4448d98 100644 --- a/src/vs/platform/agentPlugins/common/pluginParsers.ts +++ b/src/vs/platform/agentPlugins/common/pluginParsers.ts @@ -187,16 +187,18 @@ export function parseComponentPathConfig(raw: unknown): IComponentPathConfig { /** * Resolves the directories to scan for a given component type, combining * the default directory with any custom paths from the manifest config. - * Paths that resolve outside the plugin root are silently ignored. + * Paths that resolve outside the boundary are silently ignored. + * @param boundaryUri The outermost directory that resolved paths must stay within. Defaults to {@link pluginUri}. */ -export function resolveComponentDirs(pluginUri: URI, defaultDir: string, config: IComponentPathConfig): readonly URI[] { +export function resolveComponentDirs(pluginUri: URI, defaultDir: string, config: IComponentPathConfig, boundaryUri?: URI): readonly URI[] { + const boundary = (boundaryUri && isEqualOrParent(pluginUri, boundaryUri)) ? boundaryUri : pluginUri; const dirs: URI[] = []; if (!config.exclusive) { dirs.push(joinPath(pluginUri, defaultDir)); } for (const p of config.paths) { const resolved = normalizePath(joinPath(pluginUri, p)); - if (isEqualOrParent(resolved, pluginUri)) { + if (isEqualOrParent(resolved, boundary)) { dirs.push(resolved); } } @@ -811,6 +813,7 @@ export async function parsePlugin( fileService: IFileService, workspaceRoot: URI | undefined, userHome: string, + boundaryUri?: URI, ): Promise { const formatConfig = await detectPluginFormat(pluginUri, fileService); @@ -819,10 +822,10 @@ export async function parsePlugin( const manifest = (manifestJson && typeof manifestJson === 'object') ? manifestJson as Record : undefined; // Resolve component directories from manifest - const hookDirs = resolveComponentDirs(pluginUri, formatConfig.hookConfigPath, parseComponentPathConfig(manifest?.['hooks'])); - const mcpDirs = resolveComponentDirs(pluginUri, '.mcp.json', parseComponentPathConfig(manifest?.['mcpServers'])); - const skillDirs = resolveComponentDirs(pluginUri, 'skills', parseComponentPathConfig(manifest?.['skills'])); - const agentDirs = resolveComponentDirs(pluginUri, 'agents', parseComponentPathConfig(manifest?.['agents'])); + const hookDirs = resolveComponentDirs(pluginUri, formatConfig.hookConfigPath, parseComponentPathConfig(manifest?.['hooks']), boundaryUri); + const mcpDirs = resolveComponentDirs(pluginUri, '.mcp.json', parseComponentPathConfig(manifest?.['mcpServers']), boundaryUri); + const skillDirs = resolveComponentDirs(pluginUri, 'skills', parseComponentPathConfig(manifest?.['skills']), boundaryUri); + const agentDirs = resolveComponentDirs(pluginUri, 'agents', parseComponentPathConfig(manifest?.['agents']), boundaryUri); // Handle embedded MCP servers in manifest let embeddedMcp: IMcpServerDefinition[] = []; diff --git a/src/vs/platform/agentPlugins/test/common/pluginParsers.test.ts b/src/vs/platform/agentPlugins/test/common/pluginParsers.test.ts index afc3121f922f1..9d9a69923ed63 100644 --- a/src/vs/platform/agentPlugins/test/common/pluginParsers.test.ts +++ b/src/vs/platform/agentPlugins/test/common/pluginParsers.test.ts @@ -103,6 +103,26 @@ suite('pluginParsers', () => { // Should only have the default dir, the traversal path is rejected assert.strictEqual(dirs.length, 1); }); + + test('allows paths that escape plugin root but stay within boundaryUri', () => { + const boundaryUri = URI.file('/workspace'); + const dirs = resolveComponentDirs(pluginUri, 'skills', { paths: ['../shared-skills'], exclusive: false }, boundaryUri); + assert.strictEqual(dirs.length, 2); + assert.ok(dirs[1].path.endsWith('/shared-skills')); + }); + + test('rejects paths that escape boundaryUri', () => { + const boundaryUri = URI.file('/workspace'); + const dirs = resolveComponentDirs(pluginUri, 'skills', { paths: ['../../outside'], exclusive: false }, boundaryUri); + assert.strictEqual(dirs.length, 1); + }); + + test('falls back to pluginUri when boundaryUri is not an ancestor of pluginUri', () => { + const boundaryUri = URI.file('/unrelated/directory'); + const dirs = resolveComponentDirs(pluginUri, 'skills', { paths: ['custom'], exclusive: false }, boundaryUri); + assert.strictEqual(dirs.length, 2); + assert.ok(dirs[1].path.endsWith('/custom')); + }); }); // ---- normalizeMcpServerConfiguration -------------------------------- diff --git a/src/vs/workbench/contrib/chat/common/plugins/agentPluginServiceImpl.ts b/src/vs/workbench/contrib/chat/common/plugins/agentPluginServiceImpl.ts index 3c3af139a2a3b..23fb44746437e 100644 --- a/src/vs/workbench/contrib/chat/common/plugins/agentPluginServiceImpl.ts +++ b/src/vs/workbench/contrib/chat/common/plugins/agentPluginServiceImpl.ts @@ -147,6 +147,8 @@ type PluginEntry = IAgentPlugin; interface IPluginSource { readonly uri: URI; readonly fromMarketplace: IMarketplacePlugin | undefined; + /** Repository root that serves as the boundary for component path resolution. */ + readonly repositoryUri?: URI; /** Called when remove is invoked on the plugin */ remove(): void; } @@ -203,7 +205,7 @@ export abstract class AbstractAgentPluginDiscovery extends Disposable implements if (!seenPluginUris.has(key)) { seenPluginUris.add(key); const format = await detectPluginFormat(source.uri, this._fileService); - plugins.push(this._toPlugin(source.uri, format, source.fromMarketplace, () => source.remove())); + plugins.push(this._toPlugin(source.uri, format, source.fromMarketplace, source.repositoryUri, () => source.remove())); } } @@ -222,7 +224,7 @@ export abstract class AbstractAgentPluginDiscovery extends Disposable implements } } - private _toPlugin(uri: URI, format: IPluginFormatConfig, fromMarketplace: IMarketplacePlugin | undefined, removeCallback: () => void): IAgentPlugin { + private _toPlugin(uri: URI, format: IPluginFormatConfig, fromMarketplace: IMarketplacePlugin | undefined, repositoryUri: URI | undefined, removeCallback: () => void): IAgentPlugin { const key = uri.toString(); const existing = this._pluginEntries.get(key); if (existing) { @@ -258,7 +260,7 @@ export abstract class AbstractAgentPluginDiscovery extends Disposable implements } const paths = parseComponentPathConfig(section); - const dirs = resolveComponentDirs(uri, defaultPath, paths); + const dirs = resolveComponentDirs(uri, defaultPath, paths, repositoryUri); for (const d of dirs) { const watcher = this._fileService.createWatcher(d, { recursive: false, excludes: [] }); reader.store.add(watcher); @@ -632,9 +634,12 @@ export class MarketplaceAgentPluginDiscovery extends AbstractAgentPluginDiscover continue; } + const repositoryUri = this._pluginRepositoryService.getRepositoryUri(entry.plugin.marketplaceReference, entry.plugin.marketplaceType); + sources.push({ uri: stat.resource, fromMarketplace: entry.plugin, + repositoryUri, remove: () => { this._enablementModel.remove(stat.resource.toString()); this._pluginMarketplaceService.removeInstalledPlugin(entry.pluginUri);