From 6479cac77be147cea66f76e9601e33c25409d652 Mon Sep 17 00:00:00 2001 From: gambtho Date: Tue, 22 Sep 2026 13:04:15 -0400 Subject: [PATCH 1/7] [AKS] Add native Azure plugin installation boundary --- .../cli/command_modules/acs/_azure_plugin.py | 168 +++++++ .../acs/tests/latest/test_azure_plugin.py | 455 ++++++++++++++++++ 2 files changed, 623 insertions(+) create mode 100644 src/azure-cli/azure/cli/command_modules/acs/_azure_plugin.py create mode 100644 src/azure-cli/azure/cli/command_modules/acs/tests/latest/test_azure_plugin.py diff --git a/src/azure-cli/azure/cli/command_modules/acs/_azure_plugin.py b/src/azure-cli/azure/cli/command_modules/acs/_azure_plugin.py new file mode 100644 index 00000000000..a65a887860b --- /dev/null +++ b/src/azure-cli/azure/cli/command_modules/acs/_azure_plugin.py @@ -0,0 +1,168 @@ +# -------------------------------------------------------------------------------------------- +# Copyright (c) Microsoft Corporation. All rights reserved. +# Licensed under the MIT License. See License.txt in the project root for license information. +# -------------------------------------------------------------------------------------------- + +"""Native, host-owned installation of the full Azure plugin, after caller consent.""" + +import json +import re +import shutil +import subprocess + +from azure.cli.core.azclierror import ( + ClientRequestError, InvalidArgumentValueError, ResourceNotFoundError, ValidationError, +) + +HOST_IDS = ('claude-code', 'github-copilot', 'codex') +HOST_LABELS = {'claude-code': 'Claude Code', 'github-copilot': 'GitHub Copilot CLI', 'codex': 'Codex CLI'} +_EXECUTABLES = {'claude-code': 'claude', 'github-copilot': 'copilot', 'codex': 'codex'} + + +def discover_hosts() -> list[str]: + """Discover executables without running hosts or inspecting their configuration.""" + return [host for host in HOST_IDS if shutil.which(_EXECUTABLES[host])] + + +def install_plugin(host_id: str) -> bool: + """Install after consent; False means Azure was found and no plugin install ran. + + A marketplace may have been added before an existing installation became visible. + Native inventory can hide stale registrations; consent covers native install-and-enable. + """ + if host_id not in HOST_IDS: + raise InvalidArgumentValueError(f'Unsupported Azure plugin host: {host_id}. Choose from {", ".join(HOST_IDS)}.') + executable = shutil.which(_EXECUTABLES[host_id]) + if not executable: + raise ResourceNotFoundError(_failure(host_id, 'prerequisite check', + f'Install {_EXECUTABLES[host_id]} and make it available on PATH.')) + args = ['plugin', 'list', '--available', '--json'] if host_id == 'codex' else ['plugin', 'list', '--json'] + inventory = _inventory(host_id, [executable, *args], 'plugin inventory') + if _azure_registered(host_id, inventory): + return False + + if host_id == 'github-copilot': + _check_copilot_mcp(executable) + if host_id == 'claude-code': + marketplace = 'claude-plugins-official' + source = 'anthropics/claude-plugins-official' + flags = ['--scope', 'user'] + else: + marketplace = 'azure-skills' + source = 'microsoft/azure-skills' + flags = ['--json'] if host_id == 'codex' else [] + + markets = _inventory(host_id, [executable, 'plugin', 'marketplace', 'list', '--json'], 'marketplace inventory') + if host_id == 'codex': + markets = markets.get('marketplaces') if isinstance(markets, dict) else None + origin = 'root' if host_id == 'codex' else 'source' + if not isinstance(markets, list) or any( + not isinstance(row, dict) or not _text(row.get('name')) or not _text(row.get(origin)) + for row in markets): + raise ValidationError(_failure(host_id, 'marketplace inventory', 'Unrecognized native JSON schema.')) + _check_runtime(host_id) + if not any(row['name'] == marketplace for row in markets): + _run(host_id, [executable, 'plugin', 'marketplace', 'add', source, *flags], 'marketplace add') + # Adding a catalog can expose installed or live plugins hidden from the first inventory. + inventory = _inventory(host_id, [executable, *args], 'plugin inventory') + if _azure_registered(host_id, inventory): + return False + action = 'add' if host_id == 'codex' else 'install' + _run(host_id, [executable, 'plugin', action, f'azure@{marketplace}', *flags], 'plugin install') + return True + + +def _check_runtime(host_id): + node = shutil.which('node') + if not node or not shutil.which('npx'): + raise ResourceNotFoundError(_failure(host_id, 'runtime prerequisite check', + 'Install Node.js 22 or later with node and npx available on PATH.')) + result = _run(host_id, [node, '--version'], 'Node.js version check') + version = re.fullmatch(r'v(\d+)\.\d+\.\d+', result.stdout.strip()) + if not version or int(version[1]) < 22: + raise ValidationError(_failure(host_id, 'runtime prerequisite check', + 'Node.js 22 or later is required for a new Azure plugin installation. ' + f'Reported version: {_diagnostic(result.stdout)}')) + + +def _text(value): + return isinstance(value, str) and bool(value.strip()) + + +def _azure_registered(host_id, inventory): + error = _failure(host_id, 'plugin inventory', 'Unrecognized native JSON schema.') + if host_id == 'codex': + if not isinstance(inventory, dict) or not all( + isinstance(inventory.get(key), list) for key in ('installed', 'available')): + raise ValidationError(error) + for key, installed in (('installed', True), ('available', False)): + for row in inventory[key]: + if (not isinstance(row, dict) or not _text(row.get('name')) or + not _text(row.get('marketplaceName')) or not isinstance(row.get('enabled'), bool)): + raise ValidationError(error) + if (row.get('installed') is not installed or + row.get('pluginId') != f"{row['name']}@{row['marketplaceName']}"): + raise ValidationError(error) + return any(row['name'] == 'azure' for row in inventory['installed']) + + identity = 'id' if host_id == 'claude-code' else 'name' + origin = 'scope' if host_id == 'claude-code' else 'source' + if not isinstance(inventory, list) or any( + not isinstance(row, dict) or not _text(row.get(identity)) or + not _text(row.get(origin)) or not isinstance(row.get('enabled'), bool) + for row in inventory): + raise ValidationError(error) + return any(row[identity].split('@', 1)[0] == 'azure' for row in inventory) + + +def _check_copilot_mcp(executable): + inventory = _inventory('github-copilot', [executable, 'mcp', 'list', '--json'], 'MCP inventory') + servers = inventory.get('mcpServers') if isinstance(inventory, dict) else None + if not isinstance(servers, dict) or any( + not isinstance(row, dict) or not _text(row.get('source')) or not isinstance(row.get('enabled'), bool) + for row in servers.values()): + raise ValidationError(_failure('github-copilot', 'MCP inventory', 'Unrecognized native JSON schema.')) + if 'azure' in servers: + raise ValidationError(_failure('github-copilot', 'MCP inventory', + 'An existing azure MCP server could be shadowed by the Azure plugin. ' + 'Resolve this collision manually before installing.')) + + +def _inventory(host_id, argv, operation): + result = _run(host_id, argv, operation) + # A warning may mean the host skipped unreadable configuration, not an empty inventory. + if result.stderr.strip(): + raise ValidationError(_failure(host_id, operation, _diagnostic(result.stderr))) + try: + return json.loads(result.stdout) + except (ValueError, RecursionError) as ex: + raise ValidationError(_failure(host_id, operation, + f'Invalid native JSON: {_diagnostic(result.stdout)}')) from ex + + +def _diagnostic(value): + if isinstance(value, bytes): + value = value.decode('utf-8', errors='replace') + value = (value or '').strip() + return value[:1500] + (' [truncated]' if len(value) > 1500 else '') + + +def _failure(host_id, operation, detail): + return (f'{HOST_LABELS[host_id]}: {operation} failed. {detail} ' + "Inspect and recover using the host's native plugin commands; " + 'no automatic retry or rollback was attempted.') + + +def _run(host_id, argv, operation, timeout=300): + try: + result = subprocess.run(argv, stdin=subprocess.DEVNULL, capture_output=True, + encoding='utf-8', errors='replace', timeout=timeout, check=False) + except subprocess.TimeoutExpired as ex: + detail = f'timed out after {timeout}s. {_diagnostic(ex.stdout)} {_diagnostic(ex.stderr)}' + raise ClientRequestError(_failure(host_id, operation, detail)) from ex + except OSError as ex: + raise ClientRequestError(_failure(host_id, operation, _diagnostic(str(ex)))) from ex + if result.returncode: + detail = f'exit {result.returncode}. {_diagnostic(result.stdout)} {_diagnostic(result.stderr)}' + raise ClientRequestError(_failure(host_id, operation, detail)) + return result diff --git a/src/azure-cli/azure/cli/command_modules/acs/tests/latest/test_azure_plugin.py b/src/azure-cli/azure/cli/command_modules/acs/tests/latest/test_azure_plugin.py new file mode 100644 index 00000000000..325ab2ec25c --- /dev/null +++ b/src/azure-cli/azure/cli/command_modules/acs/tests/latest/test_azure_plugin.py @@ -0,0 +1,455 @@ +# -------------------------------------------------------------------------------------------- +# Copyright (c) Microsoft Corporation. All rights reserved. +# Licensed under the MIT License. See License.txt in the project root for license information. +# -------------------------------------------------------------------------------------------- + +import copy +import json +import subprocess +import sys +import unittest +from unittest import mock + +from azure.cli.command_modules.acs import _azure_plugin as plugin +from azure.cli.core.azclierror import ( + ClientRequestError, InvalidArgumentValueError, ResourceNotFoundError, ValidationError, +) + +# Claude Code 2.1.267 and Copilot 1.0.86-2 isolated native inventory outputs, +# with only fixture names/paths substituted. No executable plugin content. +CLAUDE_PLUGIN = { + 'id': 'azure@private-market', 'version': '1.0.0', 'scope': 'user', 'enabled': False, + 'installPath': '/fixture/azure', 'installedAt': '2026-09-22T00:00:00.000Z', + 'lastUpdated': '2026-09-22T00:00:00.000Z', +} +COPILOT_PLUGIN = { + 'name': 'azure', 'marketplace': 'private-market', 'version': '1.0.0', + 'enabled': False, 'source': 'installed', +} +# Codex rust-v0.144.3 cli/src/plugin_cmd.rs (JsonPluginListEntry), and +# cli/tests/plugin_cli.rs (plugin_list_json_prints_installed_plugins). +CODEX_PLUGIN = { + 'pluginId': 'azure@private-market', 'name': 'azure', 'marketplaceName': 'private-market', + 'version': '1.0.0', 'installed': True, 'enabled': False, + 'source': {'source': 'local', 'path': '/fixture/plugins/azure'}, + 'marketplaceSource': {'sourceType': 'local', 'source': '/fixture'}, + 'installPolicy': 'AVAILABLE', 'authPolicy': 'ON_USE', +} +CLAUDE_MARKET = { + 'name': 'claude-plugins-official', 'source': 'github', 'repo': 'example/private-fork', + 'ref': 'pinned', 'installLocation': '/fixture/marketplace', +} +COPILOT_MARKET = {'name': 'azure-skills', 'source': 'GitHub: example/private-fork', 'isDefault': False} +# Codex JsonMarketplaceListEntry: source metadata is optional; refs remain host-owned. +CODEX_MARKET = { + 'name': 'azure-skills', 'root': '/fixture/pinned-marketplace', + 'marketplaceSource': {'sourceType': 'git', 'source': 'https://example.com/private-fork.git'}, +} +COPILOT_MCP = {'mcpServers': {'azure': { + 'tools': ['*'], 'command': '/nonexistent/inert', 'args': ['--pinned'], 'source': 'user', 'enabled': True, +}}} + + +class NativeProcessFixture: + """Only exact native argv are accepted; every call is observable.""" + + def __init__(self, responses): + self.responses = responses + self.calls = [] + + def __call__(self, argv, **kwargs): + self.calls.append(tuple(argv)) + if tuple(argv) not in self.responses: + raise AssertionError('Unexpected native command: ' + repr(argv)) + response = self.responses[tuple(argv)] + if callable(response): + response = response() + if isinstance(response, Exception): + raise response + if isinstance(response, subprocess.CompletedProcess): + return response + stdout = response if isinstance(response, str) else json.dumps(response) + return subprocess.CompletedProcess(argv, 0, stdout, '') + + +class AzurePluginNativeTest(unittest.TestCase): + def setUp(self): + patcher = mock.patch.object(plugin.shutil, 'which', side_effect=lambda name: name) + self.which = patcher.start() + self.addCleanup(patcher.stop) + + def install(self, host, responses): + self.process = NativeProcessFixture(responses) + with mock.patch.object(plugin.subprocess, 'run', side_effect=self.process): + return plugin.install_plugin(host) + + def fresh_responses(self, host, markets): + if host == 'claude-code': + return { + ('claude', 'plugin', 'list', '--json'): [], + ('claude', 'plugin', 'marketplace', 'list', '--json'): markets, + ('claude', 'plugin', 'marketplace', 'add', 'anthropics/claude-plugins-official', '--scope', 'user'): '', + ('claude', 'plugin', 'install', 'azure@claude-plugins-official', '--scope', 'user'): 'Installed', + ('node', '--version'): 'v22.0.0\n', + } + if host == 'codex': + return { + ('codex', 'plugin', 'list', '--available', '--json'): {'installed': [], 'available': []}, + ('codex', 'plugin', 'marketplace', 'list', '--json'): {'marketplaces': markets}, + ('codex', 'plugin', 'marketplace', 'add', 'microsoft/azure-skills', '--json'): '{}', + ('codex', 'plugin', 'add', 'azure@azure-skills', '--json'): '{}', + ('node', '--version'): 'v22.0.0\n', + } + return { + ('copilot', 'plugin', 'list', '--json'): [], + ('copilot', 'mcp', 'list', '--json'): {'mcpServers': {}}, + ('copilot', 'plugin', 'marketplace', 'list', '--json'): markets, + ('copilot', 'plugin', 'marketplace', 'add', 'microsoft/azure-skills'): '', + ('copilot', 'plugin', 'install', 'azure@azure-skills'): 'Installed', + ('node', '--version'): 'v22.0.0\n', + } + + def mutations(self): + return [call for call in self.process.calls if 'add' in call or 'install' in call] + + def test_claude_fresh_install_adds_official_marketplace_in_user_scope(self): + self.assertTrue(self.install('claude-code', self.fresh_responses('claude-code', []))) + self.assertEqual(self.mutations(), [ + ('claude', 'plugin', 'marketplace', 'add', 'anthropics/claude-plugins-official', '--scope', 'user'), + ('claude', 'plugin', 'install', 'azure@claude-plugins-official', '--scope', 'user'), + ]) + + def test_copilot_fresh_install_checks_mcp_before_marketplace_add(self): + self.assertTrue(self.install('github-copilot', self.fresh_responses('github-copilot', []))) + self.assertEqual(self.mutations(), [ + ('copilot', 'plugin', 'marketplace', 'add', 'microsoft/azure-skills'), + ('copilot', 'plugin', 'install', 'azure@azure-skills'), + ]) + self.assertLess(self.process.calls.index(('copilot', 'mcp', 'list', '--json')), + self.process.calls.index(self.mutations()[0])) + + def test_codex_fresh_install_adds_marketplace_then_plugin_natively(self): + self.assertTrue(self.install('codex', self.fresh_responses('codex', []))) + self.assertEqual(self.process.calls, [ + ('codex', 'plugin', 'list', '--available', '--json'), + ('codex', 'plugin', 'marketplace', 'list', '--json'), + ('node', '--version'), + ('codex', 'plugin', 'marketplace', 'add', 'microsoft/azure-skills', '--json'), + ('codex', 'plugin', 'list', '--available', '--json'), + ('codex', 'plugin', 'add', 'azure@azure-skills', '--json'), + ]) + + def test_existing_marketplace_keeps_native_source_and_pin(self): + for host, market, command in ( + ('claude-code', CLAUDE_MARKET, + ('claude', 'plugin', 'install', 'azure@claude-plugins-official', '--scope', 'user')), + ('github-copilot', COPILOT_MARKET, ('copilot', 'plugin', 'install', 'azure@azure-skills')), + ('codex', CODEX_MARKET, ('codex', 'plugin', 'add', 'azure@azure-skills', '--json'))): + with self.subTest(host=host): + responses = self.fresh_responses(host, [copy.deepcopy(market)]) + original = copy.deepcopy(responses) + self.assertTrue(self.install(host, responses)) + self.assertEqual(self.mutations(), [command]) + self.assertEqual(responses, original) + + def test_codex_marketplace_without_optional_source_metadata_is_not_readded(self): + market = {'name': 'azure-skills', 'root': '/fixture/marketplace'} + self.assertTrue(self.install('codex', self.fresh_responses('codex', [market]))) + self.assertEqual(self.mutations(), [('codex', 'plugin', 'add', 'azure@azure-skills', '--json')]) + + def test_installed_azure_is_untouched_even_disabled_or_from_another_marketplace(self): + for enabled in (True, False): + for host, command, row in ( + ('claude-code', ('claude', 'plugin', 'list', '--json'), CLAUDE_PLUGIN), + ('github-copilot', ('copilot', 'plugin', 'list', '--json'), COPILOT_PLUGIN), + ('codex', ('codex', 'plugin', 'list', '--available', '--json'), CODEX_PLUGIN)): + with self.subTest(host=host, enabled=enabled): + entry = dict(row, enabled=enabled) + inventory = {'installed': [entry], 'available': []} if host == 'codex' else [entry] + self.which.reset_mock() + self.assertFalse(self.install(host, {command: inventory})) + self.assertEqual(self.process.calls, [command]) + self.assertEqual(self.which.call_args_list, [mock.call(command[0])]) + + def test_claude_azure_in_other_scopes_or_missing_files_is_not_repaired(self): + for scope in ('project', 'local', 'managed'): + row = dict(CLAUDE_PLUGIN, scope=scope, installPath='/missing/azure', projectPath='/fixture/project') + self.assertFalse(self.install('claude-code', {('claude', 'plugin', 'list', '--json'): [row]})) + self.assertEqual(self.mutations(), []) + + def test_copilot_direct_azure_registration_is_preserved(self): + row = dict(COPILOT_PLUGIN, marketplace='', installedFrom='example/private-repo#pinned') + self.assertFalse(self.install('github-copilot', {('copilot', 'plugin', 'list', '--json'): [row]})) + self.assertEqual(self.mutations(), []) + + def test_copilot_azure_mcp_collision_blocks_fresh_install_from_every_source(self): + for source in ('user', 'workspace', 'plugin', 'builtin'): + for enabled in (True, False): + responses = self.fresh_responses('github-copilot', []) + mcp = copy.deepcopy(COPILOT_MCP) + mcp['mcpServers']['azure'].update(source=source, enabled=enabled) + responses[('copilot', 'mcp', 'list', '--json')] = mcp + with self.subTest(source=source, enabled=enabled), self.assertRaisesRegex(ValidationError, 'azure.*MCP'): + self.install('github-copilot', responses) + self.assertEqual(self.mutations(), []) + + def test_codex_valid_absence_allows_native_install_including_hidden_or_stale_states(self): + # Native output cannot distinguish absence from orphaned/uncached registrations. + # Consent covers normal install-and-enable, without reconstructing native settings. + available = dict(CODEX_PLUGIN, installed=False) + for inventory in ({'installed': [], 'available': []}, + {'installed': [], 'available': [available]}, + {'installed': [], 'available': [dict(available, enabled=True)]}): + responses = self.fresh_responses('codex', [CODEX_MARKET]) + responses[('codex', 'plugin', 'list', '--available', '--json')] = inventory + with self.subTest(inventory=inventory): + self.assertTrue(self.install('codex', responses)) + self.assertEqual(self.mutations(), [('codex', 'plugin', 'add', 'azure@azure-skills', '--json')]) + + def test_codex_native_policy_refusal_is_not_bypassed(self): + responses = self.fresh_responses('codex', [CODEX_MARKET]) + responses[('codex', 'plugin', 'list', '--available', '--json')] = { + 'installed': [], 'available': [dict(CODEX_PLUGIN, installed=False, installPolicy='NOT_AVAILABLE')], + } + command = ('codex', 'plugin', 'add', 'azure@azure-skills', '--json') + responses[command] = subprocess.CompletedProcess(command, 1, '', 'installation denied by policy') + with self.assertRaisesRegex(ClientRequestError, 'Codex CLI.*denied by policy'): + self.install('codex', responses) + self.assertEqual(self.mutations(), [command]) + self.assertEqual(self.process.calls[-1], command) + + def test_copilot_live_azure_is_existing_even_when_disabled(self): + for enabled in (False, True): + row = dict(COPILOT_PLUGIN, source='live', enabled=enabled, installedFrom='/fixture/marketplace') + command = ('copilot', 'plugin', 'list', '--json') + self.which.reset_mock() + with self.subTest(enabled=enabled): + self.assertFalse(self.install('github-copilot', {command: [row]})) + self.assertEqual(self.process.calls, [command]) + self.assertEqual(self.which.call_args_list, [mock.call('copilot')]) + + def test_newly_visible_azure_after_marketplace_add_is_not_installed(self): + for host, command, row in ( + ('claude-code', ('claude', 'plugin', 'list', '--json'), CLAUDE_PLUGIN), + ('github-copilot', ('copilot', 'plugin', 'list', '--json'), dict(COPILOT_PLUGIN, source='live')), + ('codex', ('codex', 'plugin', 'list', '--available', '--json'), CODEX_PLUGIN)): + for enabled in (False, True): + responses = self.fresh_responses(host, []) + entry = dict(row, enabled=enabled) + visible = {'installed': [entry], 'available': []} if host == 'codex' else [entry] + responses[command] = iter([responses[command], visible]).__next__ + with self.subTest(host=host, enabled=enabled): + self.assertFalse(self.install(host, responses)) + self.assertEqual(len(self.mutations()), 1) + self.assertEqual(self.mutations()[0][1:4], ('plugin', 'marketplace', 'add')) + self.assertEqual(self.process.calls[-1], command) + self.assertEqual(self.process.calls.count(command), 2) + + def test_inventory_refresh_failure_after_marketplace_add_never_installs_plugin(self): + for host, command in ( + ('claude-code', ('claude', 'plugin', 'list', '--json')), + ('github-copilot', ('copilot', 'plugin', 'list', '--json')), + ('codex', ('codex', 'plugin', 'list', '--available', '--json'))): + for response, error in ( + ('not json', ValidationError), ({}, ValidationError), + (subprocess.CompletedProcess(command, 0, '[]', 'inventory warning'), ValidationError), + (subprocess.CompletedProcess(command, 1, '', 'policy refusal'), ClientRequestError), + (OSError('permission denied'), ClientRequestError), + (subprocess.TimeoutExpired(command, 300), ClientRequestError)): + responses = self.fresh_responses(host, []) + responses[command] = iter([responses[command], response]).__next__ + with self.subTest(host=host, response=response), self.assertRaises(error): + self.install(host, responses) + self.assertEqual(len(self.mutations()), 1) + self.assertEqual(self.mutations()[0][1:4], ('plugin', 'marketplace', 'add')) + self.assertEqual(self.process.calls[-1], command) + self.assertEqual(self.process.calls.count(command), 2) + + def test_invalid_plugin_inventory_never_becomes_absence(self): + for host, command, bad_values in ( + ('claude-code', ('claude', 'plugin', 'list', '--json'), + ('not json', {}, None, [None], [{'name': 'azure'}], [dict(CLAUDE_PLUGIN, enabled='false')], + [dict(CLAUDE_PLUGIN, scope=None)])), + ('github-copilot', ('copilot', 'plugin', 'list', '--json'), + ('help output', {}, None, [{}], [dict(COPILOT_PLUGIN, name=None)], + [dict(COPILOT_PLUGIN, source=None)])), + ('codex', ('codex', 'plugin', 'list', '--available', '--json'), + ('help output', [], {}, {'installed': []}, {'installed': None, 'available': []}, + {'installed': [dict(CODEX_PLUGIN, installed=False)], 'available': []}, + {'installed': [], 'available': [CODEX_PLUGIN]}, + {'installed': [], 'available': [dict(CODEX_PLUGIN, installed=False, pluginId='wrong')]}, + {'installed': [], 'available': [dict(CODEX_PLUGIN, installed=False, enabled=None)]}))): + for bad in bad_values: + with self.subTest(host=host, bad=bad), self.assertRaises(ValidationError): + self.install(host, {command: bad}) + self.assertEqual(self.mutations(), []) + + def test_invalid_marketplace_inventory_never_triggers_add(self): + for host in ('claude-code', 'github-copilot'): + for bad in ({}, None, [None], [{}], [{'name': None}], [{'name': 'new-schema', 'source': {}}]): + with self.subTest(host=host, bad=bad), self.assertRaises(ValidationError): + self.install(host, self.fresh_responses(host, bad)) + self.assertEqual(self.mutations(), []) + + def test_invalid_codex_marketplace_inventory_never_triggers_add(self): + for bad in ([], {}, None, {'marketplaces': None}, {'marketplaces': [None]}, + {'marketplaces': [{}]}, {'marketplaces': [dict(CODEX_MARKET, name=None)]}, + {'marketplaces': [dict(CODEX_MARKET, root=None)]}): + responses = self.fresh_responses('codex', []) + responses[('codex', 'plugin', 'marketplace', 'list', '--json')] = bad + with self.subTest(bad=bad), self.assertRaises(ValidationError): + self.install('codex', responses) + self.assertEqual(self.mutations(), []) + + def test_invalid_mcp_inventory_never_becomes_absence(self): + for bad in ({}, [], {'mcpServers': []}, {'mcpServers': {'other': None}}, + {'mcpServers': {'other': {}}}, {'mcpServers': {'other': {'source': 'user', 'enabled': 'false'}}}): + responses = self.fresh_responses('github-copilot', []) + responses[('copilot', 'mcp', 'list', '--json')] = bad + with self.subTest(bad=bad), self.assertRaises(ValidationError): + self.install('github-copilot', responses) + self.assertEqual(self.mutations(), []) + + def test_inventory_warning_never_becomes_absence(self): + command = ('copilot', 'plugin', 'list', '--json') + response = subprocess.CompletedProcess(command, 0, '[]', 'Configuration could not be loaded') + with self.assertRaisesRegex(ValidationError, 'Configuration could not be loaded'): + self.install('github-copilot', {command: response}) + self.assertEqual(self.mutations(), []) + + def test_command_failure_never_retries_or_falls_through(self): + for host in ('claude-code', 'github-copilot', 'codex'): + responses = self.fresh_responses(host, []) + commands = [command for command in responses if command[0] != 'node'] + for command in commands: + broken = dict(responses) + broken[command] = subprocess.CompletedProcess(command, 2, '', 'unsupported flag or policy refusal') + with self.subTest(host=host, command=command), self.assertRaisesRegex(ClientRequestError, 'policy refusal'): + self.install(host, broken) + self.assertEqual(self.process.calls[-1], command) + self.assertEqual(self.process.calls.count(command), 1) + + def test_fresh_install_requires_node_and_npx_before_any_mutation(self): + for missing in ('node', 'npx'): + for host in ('claude-code', 'github-copilot', 'codex'): + self.which.side_effect = lambda name: None if name == missing else name + with self.subTest(host=host, missing=missing), self.assertRaisesRegex(ResourceNotFoundError, missing): + self.install(host, self.fresh_responses(host, [])) + self.assertEqual(self.mutations(), []) + + def test_node_21_or_unparseable_version_blocks_fresh_install(self): + for version in ('v21.9.0', 'v20.19.0', '22', 'Node v22.0.0', '', 'v22.0.0\nunexpected output'): + responses = self.fresh_responses('claude-code', []) + responses[('node', '--version')] = version + with self.subTest(version=version), self.assertRaisesRegex(ValidationError, 'Claude Code.*Node.js 22'): + self.install('claude-code', responses) + self.assertEqual(self.mutations(), []) + + def test_node_22_and_later_are_checked_once_before_native_mutations(self): + for version in ('v22.0.0\n', 'v24.1.2', 'v25.9.0'): + responses = self.fresh_responses('github-copilot', []) + responses[('node', '--version')] = version + with self.subTest(version=version): + self.assertTrue(self.install('github-copilot', responses)) + self.assertEqual(self.process.calls.count(('node', '--version')), 1) + self.assertLess(self.process.calls.index(('node', '--version')), + self.process.calls.index(self.mutations()[0])) + self.assertIn(mock.call('npx'), self.which.call_args_list) + + def test_node_execution_failure_never_triggers_native_mutation(self): + for error in (OSError('cannot execute node'), subprocess.TimeoutExpired(['node', '--version'], 300), + subprocess.CompletedProcess(['node', '--version'], 1, '', 'node failed')): + responses = self.fresh_responses('claude-code', []) + responses[('node', '--version')] = error + with self.subTest(error=error), self.assertRaisesRegex(ClientRequestError, 'Claude Code.*Node.js'): + self.install('claude-code', responses) + self.assertEqual(self.mutations(), []) + + def test_native_oserror_and_timeout_stop_at_the_failing_operation(self): + for host in ('claude-code', 'github-copilot', 'codex'): + responses = self.fresh_responses(host, []) + for command in (key for key in responses if key[0] != 'node'): + for error in (OSError('permission denied'), subprocess.TimeoutExpired(command, 300)): + broken = dict(responses) + broken[command] = error + with self.subTest(host=host, command=command, error=error), self.assertRaises(ClientRequestError): + self.install(host, broken) + self.assertEqual(self.process.calls[-1], command) + self.assertEqual(self.process.calls.count(command), 1) + + def test_repeated_install_never_reenables_a_disabled_registration(self): + for host, executable, row in (('claude-code', 'claude', CLAUDE_PLUGIN), + ('github-copilot', 'copilot', COPILOT_PLUGIN)): + command = (executable, 'plugin', 'list', '--json') + responses = {command: [copy.deepcopy(row)]} + with self.subTest(host=host): + for _ in range(2): + self.assertFalse(self.install(host, responses)) + self.assertEqual(self.process.calls, [command]) + self.assertEqual(responses[command], [row]) + + def test_unknown_host_fails_before_lookup_or_process(self): + with self.assertRaises(InvalidArgumentValueError): + self.install('other-host', {}) + self.which.assert_not_called() + self.assertEqual(self.process.calls, []) + + def test_missing_host_has_actionable_prerequisite_error(self): + self.which.side_effect = None + self.which.return_value = None + with self.assertRaisesRegex(ResourceNotFoundError, 'Claude Code.*PATH'): + self.install('claude-code', {}) + self.assertEqual(self.process.calls, []) + + +class AzurePluginProcessTest(unittest.TestCase): + def test_child_receives_literal_arguments_and_closed_stdin(self): + argv = [sys.executable, '-c', + 'import json, sys; print(json.dumps([sys.argv[1:], sys.stdin.read()]))', + 'spaces and ; shell | characters', '$(not-a-command)'] + result = plugin._run('claude-code', argv, 'test inventory') + self.assertEqual(json.loads(result.stdout), [argv[3:], '']) + + def test_child_nonzero_keeps_bounded_context(self): + argv = [sys.executable, '-c', + 'import sys; print("useful stdout"); sys.stderr.write("failure detail " + "x" * 10000); sys.exit(7)'] + with self.assertRaises(ClientRequestError) as caught: + plugin._run('github-copilot', argv, 'plugin install') + message = str(caught.exception) + for part in ('GitHub Copilot CLI', 'plugin install', '7', 'useful stdout', 'failure detail', 'native'): + self.assertIn(part, message) + self.assertLess(len(message), 4000) + self.assertIn('truncated', message) + + def test_child_timeout_is_finite_and_keeps_partial_output(self): + argv = [sys.executable, '-c', 'import time; print("waiting", flush=True); time.sleep(30)'] + with self.assertRaises(ClientRequestError) as caught: + plugin._run('codex', argv, 'plugin inventory', timeout=0.2) + message = str(caught.exception) + for part in ('Codex CLI', 'plugin inventory', 'timed out', 'waiting', 'native'): + self.assertIn(part, message) + + def test_oserror_is_concrete_cli_error(self): + with mock.patch.object(plugin.subprocess, 'run', side_effect=OSError('executable permission denied')): + with self.assertRaisesRegex(ClientRequestError, 'Claude Code.*plugin inventory.*permission denied'): + plugin._run('claude-code', ['claude', 'plugin', 'list', '--json'], 'plugin inventory') + + def test_runner_has_no_shell_and_finite_default_timeout(self): + with mock.patch.object(plugin.subprocess, 'run', wraps=subprocess.run) as run: + plugin._run('codex', [sys.executable, '-c', 'print("ok")'], 'test inventory') + kwargs = run.call_args.kwargs + self.assertFalse(kwargs.get('shell', False)) + self.assertEqual(kwargs['stdin'], subprocess.DEVNULL) + self.assertGreater(kwargs['timeout'], 0) + self.assertLessEqual(kwargs['timeout'], 300) + + +class AzurePluginDiscoveryTest(unittest.TestCase): + def test_discovery_only_checks_executable_presence(self): + with mock.patch.object(plugin.shutil, 'which', side_effect=lambda name: '/bin/' + name if name == 'codex' else None), \ + mock.patch('subprocess.run', side_effect=AssertionError('Discovery must not execute a host')): + self.assertEqual(plugin.discover_hosts(), ['codex']) + + def test_discovery_uses_stable_host_order(self): + with mock.patch.object(plugin.shutil, 'which', side_effect=lambda name: '/bin/' + name): + self.assertEqual(plugin.discover_hosts(), ['claude-code', 'github-copilot', 'codex']) From 822f724fc1d029789cf28de53f9838ca93f88fef Mon Sep 17 00:00:00 2001 From: gambtho Date: Tue, 22 Sep 2026 13:15:59 -0400 Subject: [PATCH 2/7] [AKS] Suppress sensitive MCP inventory diagnostics --- .../cli/command_modules/acs/_azure_plugin.py | 31 ++++++++------ .../acs/tests/latest/test_azure_plugin.py | 42 ++++++++++++++++++- 2 files changed, 60 insertions(+), 13 deletions(-) diff --git a/src/azure-cli/azure/cli/command_modules/acs/_azure_plugin.py b/src/azure-cli/azure/cli/command_modules/acs/_azure_plugin.py index a65a887860b..f49def9bb2e 100644 --- a/src/azure-cli/azure/cli/command_modules/acs/_azure_plugin.py +++ b/src/azure-cli/azure/cli/command_modules/acs/_azure_plugin.py @@ -116,7 +116,8 @@ def _azure_registered(host_id, inventory): def _check_copilot_mcp(executable): - inventory = _inventory('github-copilot', [executable, 'mcp', 'list', '--json'], 'MCP inventory') + # MCP command arguments and environment values can contain credentials. + inventory = _inventory('github-copilot', [executable, 'mcp', 'list', '--json'], 'MCP inventory', sensitive=True) servers = inventory.get('mcpServers') if isinstance(inventory, dict) else None if not isinstance(servers, dict) or any( not isinstance(row, dict) or not _text(row.get('source')) or not isinstance(row.get('enabled'), bool) @@ -128,19 +129,22 @@ def _check_copilot_mcp(executable): 'Resolve this collision manually before installing.')) -def _inventory(host_id, argv, operation): - result = _run(host_id, argv, operation) +def _inventory(host_id, argv, operation, *, sensitive=False): + result = _run(host_id, argv, operation, sensitive=sensitive) # A warning may mean the host skipped unreadable configuration, not an empty inventory. if result.stderr.strip(): - raise ValidationError(_failure(host_id, operation, _diagnostic(result.stderr))) + detail = f'Native inventory warning: {_diagnostic(result.stderr, sensitive=sensitive)}' + raise ValidationError(_failure(host_id, operation, detail)) try: return json.loads(result.stdout) except (ValueError, RecursionError) as ex: - raise ValidationError(_failure(host_id, operation, - f'Invalid native JSON: {_diagnostic(result.stdout)}')) from ex + detail = f'Invalid native JSON: {_diagnostic(result.stdout, sensitive=sensitive)}' + raise ValidationError(_failure(host_id, operation, detail)) from (None if sensitive else ex) -def _diagnostic(value): +def _diagnostic(value, *, sensitive=False): + if sensitive: + return '[sensitive inventory output suppressed]' if isinstance(value, bytes): value = value.decode('utf-8', errors='replace') value = (value or '').strip() @@ -153,16 +157,19 @@ def _failure(host_id, operation, detail): 'no automatic retry or rollback was attempted.') -def _run(host_id, argv, operation, timeout=300): +def _run(host_id, argv, operation, timeout=300, *, sensitive=False): try: result = subprocess.run(argv, stdin=subprocess.DEVNULL, capture_output=True, encoding='utf-8', errors='replace', timeout=timeout, check=False) except subprocess.TimeoutExpired as ex: - detail = f'timed out after {timeout}s. {_diagnostic(ex.stdout)} {_diagnostic(ex.stderr)}' - raise ClientRequestError(_failure(host_id, operation, detail)) from ex + detail = (f'timed out after {timeout}s. {_diagnostic(ex.stdout, sensitive=sensitive)} ' + f'{_diagnostic(ex.stderr, sensitive=sensitive)}') + raise ClientRequestError(_failure(host_id, operation, detail)) from (None if sensitive else ex) except OSError as ex: - raise ClientRequestError(_failure(host_id, operation, _diagnostic(str(ex)))) from ex + detail = _diagnostic(str(ex), sensitive=sensitive) + raise ClientRequestError(_failure(host_id, operation, detail)) from (None if sensitive else ex) if result.returncode: - detail = f'exit {result.returncode}. {_diagnostic(result.stdout)} {_diagnostic(result.stderr)}' + detail = (f'exit {result.returncode}. {_diagnostic(result.stdout, sensitive=sensitive)} ' + f'{_diagnostic(result.stderr, sensitive=sensitive)}') raise ClientRequestError(_failure(host_id, operation, detail)) return result diff --git a/src/azure-cli/azure/cli/command_modules/acs/tests/latest/test_azure_plugin.py b/src/azure-cli/azure/cli/command_modules/acs/tests/latest/test_azure_plugin.py index 325ab2ec25c..f643e40c789 100644 --- a/src/azure-cli/azure/cli/command_modules/acs/tests/latest/test_azure_plugin.py +++ b/src/azure-cli/azure/cli/command_modules/acs/tests/latest/test_azure_plugin.py @@ -7,6 +7,7 @@ import json import subprocess import sys +import traceback import unittest from unittest import mock @@ -310,6 +311,44 @@ def test_invalid_mcp_inventory_never_becomes_absence(self): self.install('github-copilot', responses) self.assertEqual(self.mutations(), []) + def test_mcp_inventory_failures_do_not_disclose_credentials(self): + stdout_secret = 'synthetic-mcp-stdout-credential' + stderr_secret = 'synthetic-mcp-stderr-credential' + inventory = copy.deepcopy(COPILOT_MCP) + inventory['mcpServers']['azure']['args'] = ['--token', stdout_secret] + payload = json.dumps(inventory) + real_run = subprocess.run + for case, stdout, stderr, ending, error, context in ( + ('invalid JSON', payload[:-1], '', 'sys.exit(0)', ValidationError, 'Invalid native JSON'), + ('nonzero', payload, stderr_secret, 'sys.exit(7)', ClientRequestError, 'exit 7'), + ('timeout', payload, stderr_secret, 'time.sleep(30)', ClientRequestError, 'timed out after'), + ('warning', payload, stderr_secret, 'sys.exit(0)', ValidationError, 'warning')): + calls = [] + script = (f'import sys, time; print({stdout!r}, flush=True); ' + f'print({stderr!r}, file=sys.stderr, flush=True); {ending}') + + def run(argv, **kwargs): + calls.append(tuple(argv)) + if argv == ['copilot', 'plugin', 'list', '--json']: + return subprocess.CompletedProcess(argv, 0, '[]', '') + if argv == ['copilot', 'mcp', 'list', '--json']: + kwargs['timeout'] = 0.2 if case == 'timeout' else 5 + return real_run([sys.executable, '-c', script], **kwargs) + raise AssertionError('Unexpected native command: ' + repr(argv)) + + with self.subTest(case=case): + with mock.patch.object(plugin.subprocess, 'run', side_effect=run), self.assertRaises(error) as caught: + plugin.install_plugin('github-copilot') + message = str(caught.exception) + for safe_context in ('GitHub Copilot CLI', 'MCP inventory', context, 'native'): + self.assertIn(safe_context, message) + rendered = ''.join(traceback.format_exception(caught.exception)) + for secret in (stdout_secret, stderr_secret): + self.assertNotIn(secret, message) + self.assertNotIn(secret, rendered) + self.assertEqual(calls, [('copilot', 'plugin', 'list', '--json'), + ('copilot', 'mcp', 'list', '--json')]) + def test_inventory_warning_never_becomes_absence(self): command = ('copilot', 'plugin', 'list', '--json') response = subprocess.CompletedProcess(command, 0, '[]', 'Configuration could not be loaded') @@ -324,7 +363,8 @@ def test_command_failure_never_retries_or_falls_through(self): for command in commands: broken = dict(responses) broken[command] = subprocess.CompletedProcess(command, 2, '', 'unsupported flag or policy refusal') - with self.subTest(host=host, command=command), self.assertRaisesRegex(ClientRequestError, 'policy refusal'): + context = 'exit 2' if command[1] == 'mcp' else 'policy refusal' + with self.subTest(host=host, command=command), self.assertRaisesRegex(ClientRequestError, context): self.install(host, broken) self.assertEqual(self.process.calls[-1], command) self.assertEqual(self.process.calls.count(command), 1) From 0bce4910bd465fe492803b528feb32907c427586 Mon Sep 17 00:00:00 2001 From: gambtho Date: Tue, 22 Sep 2026 13:31:48 -0400 Subject: [PATCH 3/7] [AKS] Add consented Azure plugin setup to install-cli --- .../cli/command_modules/acs/_azure_plugin.py | 112 +++++++++ .../azure/cli/command_modules/acs/_help.py | 40 ++- .../azure/cli/command_modules/acs/_params.py | 12 + .../azure/cli/command_modules/acs/custom.py | 5 +- .../acs/tests/latest/test_azure_plugin.py | 235 ++++++++++++++++++ .../acs/tests/latest/test_custom.py | 169 +++++++++++++ 6 files changed, 571 insertions(+), 2 deletions(-) diff --git a/src/azure-cli/azure/cli/command_modules/acs/_azure_plugin.py b/src/azure-cli/azure/cli/command_modules/acs/_azure_plugin.py index f49def9bb2e..2ce163c536c 100644 --- a/src/azure-cli/azure/cli/command_modules/acs/_azure_plugin.py +++ b/src/azure-cli/azure/cli/command_modules/acs/_azure_plugin.py @@ -6,19 +6,131 @@ """Native, host-owned installation of the full Azure plugin, after caller consent.""" import json +import os import re import shutil import subprocess +import sys + +from knack.log import get_logger +from knack.prompting import NoTTYException, prompt, prompt_y_n from azure.cli.core.azclierror import ( ClientRequestError, InvalidArgumentValueError, ResourceNotFoundError, ValidationError, ) +logger = get_logger(__name__) + HOST_IDS = ('claude-code', 'github-copilot', 'codex') HOST_LABELS = {'claude-code': 'Claude Code', 'github-copilot': 'GitHub Copilot CLI', 'codex': 'Codex CLI'} _EXECUTABLES = {'claude-code': 'claude', 'github-copilot': 'copilot', 'codex': 'codex'} +def validate_plugin_options(install_azure_plugin=None, plugin_hosts=None): + """Validate consent and normalize explicit hosts before binary installation.""" + if install_azure_plugin is not True: + if plugin_hosts is not None: + raise InvalidArgumentValueError('--plugin-hosts requires --install-azure-plugin true.') + return None + if not plugin_hosts or any(host not in HOST_IDS for host in plugin_hosts): + raise InvalidArgumentValueError( + '--install-azure-plugin true requires --plugin-hosts with one or more of: ' + ', '.join(HOST_IDS)) + if _under_sudo(): + raise InvalidArgumentValueError( + 'Azure plugin setup cannot run under sudo. Run as the intended host user with writable binary paths.') + return [host for host in HOST_IDS if host in plugin_hosts] + + +def _under_sudo(): + return bool(os.environ.get('SUDO_USER') or os.environ.get('SUDO_UID')) + + +def maybe_install_azure_plugin(cmd, install_azure_plugin=None, plugin_hosts=None): + """Offer optional setup after both binaries succeed; explicit flags are consent.""" + hosts = validate_plugin_options(install_azure_plugin, plugin_hosts) + if install_azure_plugin is False: + return + if install_azure_plugin is None: + if (_under_sudo() or not sys.stdin.isatty() or + cmd.cli_ctx.config.getboolean('core', 'disable_confirm_prompt', fallback=False)): + return + try: + if not prompt_y_n('Set up the full Azure plugin for an AI CLI host?', default='n'): + return + hosts = _select_hosts() + if not hosts: + return + _disclose_setup() + labels = ', '.join(HOST_LABELS[host] for host in hosts) + if not prompt_y_n(f'Authorize native user/global installation and enablement for {labels}?', default='n'): + return + except (NoTTYException, EOFError, KeyboardInterrupt): + return + else: + _disclose_setup() + _install_selected_hosts(hosts, explicit=install_azure_plugin is True) + + +def _install_selected_hosts(hosts, *, explicit): + failures = [] + outcomes = [] + for host in hosts: + try: + installed = install_plugin(host) + except (ClientRequestError, ResourceNotFoundError, ValidationError) as ex: + failures.append(ex) + outcomes.append(f'{HOST_LABELS[host]}: failed. {ex}') + else: + status = ('installed; authentication/activation and hook trust may still be required' if installed else + 'Azure already reported; plugin install skipped (a marketplace may have been added)') + outcome = f'{HOST_LABELS[host]}: {status}.' + outcomes.append(outcome) + print(outcome, file=sys.stderr) + if failures: + summary = '\n'.join(outcomes) + message = ('Azure plugin setup did not complete for all selected hosts. ' + f'kubectl and kubelogin remain installed.\n{summary}\n' + "Inspect and recover using each host's native plugin commands. " + 'No automatic retry or rollback was attempted.') + if explicit: + raise type(failures[0])(message) from None + logger.warning(message) + + +def _select_hosts(): + detected = discover_hosts() + choices = '\n'.join(f' {index}. {HOST_LABELS[host]}' + (' [detected]' if host in detected else '') + for index, host in enumerate(HOST_IDS, 1)) + defaults = ' '.join(str(index) for index, host in enumerate(HOST_IDS, 1) if host in detected) or '0' + while True: + answer = prompt(f'{choices}\nSelect host numbers separated by spaces (replaces defaults); ' + f'0 selects none. Enter keeps [{defaults}]: ').strip() + if not answer: + return detected + if answer == '0': + return [] + numbers = answer.split() + if all(number in ('1', '2', '3') for number in numbers): + return [host for index, host in enumerate(HOST_IDS, 1) if str(index) in numbers] + print('Choose 1, 2 and/or 3 separated by spaces, or 0 for none.', file=sys.stderr) + + +def _disclose_setup(): + # Consent context must remain visible even with --only-show-errors. + print( + 'Azure plugin setup installs the full Azure plugin (skills, MCP configuration and hooks) in native ' + 'user/global scope, not repository scope. Native inventory-reported Azure installations, including ' + 'disabled ones, are skipped. When inventory reports absence, consent authorizes normal native installation ' + 'and enablement, including changes to hidden/stale disable preferences or registrations.\n' + 'New installations require an installed host CLI and Node.js 22+ with npx on PATH. ' + 'The stock MCP runtime uses @azure/mcp@latest and is not pinned by the plugin version. ' + 'Hosts own permissions, marketplace sources/pins and updates; Azure CLI adds no custom updater or bypass. ' + 'Azure authentication, MCP activation, hook trust and sovereign-cloud setup may still be required. ' + 'No prerequisites are installed and no Azure login or resource operations are performed.', + file=sys.stderr, + ) + + def discover_hosts() -> list[str]: """Discover executables without running hosts or inspecting their configuration.""" return [host for host in HOST_IDS if shutil.which(_EXECUTABLES[host])] diff --git a/src/azure-cli/azure/cli/command_modules/acs/_help.py b/src/azure-cli/azure/cli/command_modules/acs/_help.py index be439d0d8d5..cc512e1528f 100644 --- a/src/azure-cli/azure/cli/command_modules/acs/_help.py +++ b/src/azure-cli/azure/cli/command_modules/acs/_help.py @@ -1479,7 +1479,45 @@ helps["aks install-cli"] = """ type: command -short-summary: Download and install kubectl, the Kubernetes command-line tool. Download and install kubelogin, a client-go credential (exec) plugin implementing azure authentication. +short-summary: Download and install kubectl and kubelogin. Optionally set up the full Azure plugin for supported AI CLI hosts. +long-summary: | + Installs kubectl first, then kubelogin. Azure plugin setup runs only after both succeed. + With --install-azure-plugin omitted, noninteractive sessions, core.disable_confirm_prompt=true and sudo + perform no plugin setup or host/runtime probes. Other sessions offer default-No setup, a numbered host + selection with PATH-detected defaults, and final default-No consent. Detection is not consent. + Use --install-azure-plugin false for binaries only. Explicit true requires --plugin-hosts and authorizes + full-plugin setup without prompts, including in automation. Explicit plugin setup under sudo is rejected; + run as the intended host user with writable --install-location and --kubelogin-install-location paths. + + Only Claude Code, GitHub Copilot CLI and Codex CLI are supported, not Pi or VS Code. This installs the full + base Azure plugin (upstream skills, MCP configuration and hooks), not individual skills or the separate + deeper AKS plugin. Installation uses native user/global scope, not repository configuration. Hosts and + Node.js 22+ with node and npx on PATH must already be available for new installations; Azure CLI does not + install prerequisites. The stock MCP runtime uses @azure/mcp@latest, which is not pinned by the plugin + version and may change its runtime requirements. Azure authentication, MCP activation, hook trust and + sovereign-cloud setup may still be needed after installation. No Azure login or resource operations run. + + Azure plugins reported by native inventory, including disabled installations and applicable non-user + scopes, are skipped without reinstalling or enabling them. Inventory can omit hidden/stale registrations + or disable preferences. When valid inventory reports absence, consent authorizes the host's normal native + install-and-enable behavior, including changes to that hidden state. Invalid or unsupported inventory + fails safely; Azure CLI does not reconstruct native configuration. A marketplace may have been added + before an existing plugin becomes visible and plugin installation is skipped. + + Hosts own permissions, trust, marketplace sources/pins and future updates. An existing marketplace + registration is used as configured, without repointing it or bypassing host policy; the recommended + marketplace is added only if absent. There is no custom updater, forced update or per-task plugin reinstall. + --gh-token is used only for kubelogin downloads, not passed to plugin hosts. + Selected hosts are attempted independently, so partial success is possible. Optional interactive failures + warn without failing the binary installation. Explicit setup failures return nonzero; kubectl and kubelogin + remain installed. No automatic retry or rollback occurs. Recover with the host's native plugin commands. +examples: + - name: Install binaries only, without any Azure plugin offer or setup + text: az aks install-cli --install-azure-plugin false + - name: Install binaries and explicitly authorize full Azure plugin setup for Codex CLI + text: az aks install-cli --install-azure-plugin true --plugin-hosts codex + - name: Install binaries and authorize full Azure plugin setup for Claude Code and GitHub Copilot CLI + text: az aks install-cli --install-azure-plugin --plugin-hosts claude-code github-copilot """ helps["aks install-desktop"] = """ diff --git a/src/azure-cli/azure/cli/command_modules/acs/_params.py b/src/azure-cli/azure/cli/command_modules/acs/_params.py index e4aad1e3345..2423e2c35e0 100644 --- a/src/azure-cli/azure/cli/command_modules/acs/_params.py +++ b/src/azure-cli/azure/cli/command_modules/acs/_params.py @@ -7,6 +7,7 @@ import platform from argcomplete.completers import FilesCompleter +from azure.cli.command_modules.acs._azure_plugin import HOST_IDS from azure.cli.command_modules.acs._completers import ( get_k8s_upgrades_completion_list, get_k8s_versions_completion_list) from azure.cli.command_modules.acs._consts import ( @@ -1025,6 +1026,17 @@ def load_arguments(self, _): c.argument('kubelogin_base_src_url', options_list=['--kubelogin-base-src-url', '-l'], help='Base download source URL for kubelogin releases.') c.argument('gh_token', help='GitHub authentication token used when downloading kubelogin binaries from GitHub releases. Supplying a token helps avoid GitHub API rate limits.') + with self.argument_context('aks install-cli', arg_group='Azure Plugin') as c: + c.argument('install_azure_plugin', arg_type=get_three_state_flag(), + help='Consent to full Azure plugin setup in native user/global scope after both binaries install. ' + 'Requires --plugin-hosts when true; false skips setup. If omitted, only interactive sessions ' + 'offer default-No setup, unless confirmation prompts are disabled or running under sudo. ' + 'Valid native inventory absence authorizes normal install-and-enable, including hidden/stale ' + 'disable preferences. See command help for runtime, MCP, hooks and authentication requirements.') + c.argument('plugin_hosts', arg_type=get_enum_type(HOST_IDS), nargs='+', + help='Host CLIs for full Azure plugin setup. Requires --install-azure-plugin true. ' + 'Host CLIs must already be installed. Supported hosts only; not Pi or VS Code.') + with self.argument_context('aks install-desktop') as c: c.argument('version', help='Version of AKS Desktop to install. By default, the latest stable version is installed.') c.argument('gh_token', help='GitHub authentication token used to retrieve AKS Desktop release metadata. ' diff --git a/src/azure-cli/azure/cli/command_modules/acs/custom.py b/src/azure-cli/azure/cli/command_modules/acs/custom.py index 8f026a7fcb2..b1dd9736c7f 100644 --- a/src/azure-cli/azure/cli/command_modules/acs/custom.py +++ b/src/azure-cli/azure/cli/command_modules/acs/custom.py @@ -44,6 +44,7 @@ import colorama import requests import yaml +from azure.cli.command_modules.acs._azure_plugin import maybe_install_azure_plugin, validate_plugin_options from azure.cli.command_modules.acs._client_factory import ( cf_agent_pools, ) @@ -2379,12 +2380,14 @@ def aks_check_acr(cmd, client, resource_group_name, name, acr, node_name=None): # install kubectl & kubelogin def k8s_install_cli(cmd, client_version='latest', install_location=None, base_src_url=None, kubelogin_version='latest', kubelogin_install_location=None, - kubelogin_base_src_url=None, gh_token=None): + kubelogin_base_src_url=None, gh_token=None, install_azure_plugin=None, plugin_hosts=None): + plugin_hosts = validate_plugin_options(install_azure_plugin, plugin_hosts) arch = get_arch_for_cli_binary() k8s_install_kubectl(cmd, client_version, install_location, base_src_url, arch=arch) k8s_install_kubelogin(cmd, kubelogin_version, kubelogin_install_location, kubelogin_base_src_url, arch=arch, gh_token=gh_token) + maybe_install_azure_plugin(cmd, install_azure_plugin, plugin_hosts) _AKS_DESKTOP_RELEASES_API = 'https://api.github.com/repos/Azure/aks-desktop/releases' diff --git a/src/azure-cli/azure/cli/command_modules/acs/tests/latest/test_azure_plugin.py b/src/azure-cli/azure/cli/command_modules/acs/tests/latest/test_azure_plugin.py index f643e40c789..1287e1c8e93 100644 --- a/src/azure-cli/azure/cli/command_modules/acs/tests/latest/test_azure_plugin.py +++ b/src/azure-cli/azure/cli/command_modules/acs/tests/latest/test_azure_plugin.py @@ -4,18 +4,24 @@ # -------------------------------------------------------------------------------------------- import copy +import io import json +import os import subprocess import sys import traceback import unittest from unittest import mock +from knack.prompting import NoTTYException + from azure.cli.command_modules.acs import _azure_plugin as plugin from azure.cli.core.azclierror import ( ClientRequestError, InvalidArgumentValueError, ResourceNotFoundError, ValidationError, ) +REAL_INSTALL_PLUGIN = plugin.install_plugin + # Claude Code 2.1.267 and Copilot 1.0.86-2 isolated native inventory outputs, # with only fixture names/paths substituted. No executable plugin content. CLAUDE_PLUGIN = { @@ -73,6 +79,235 @@ def __call__(self, argv, **kwargs): return subprocess.CompletedProcess(argv, 0, stdout, '') +class AzurePluginOptionsTest(unittest.TestCase): + def setUp(self): + patcher = mock.patch.dict(os.environ, {}, clear=True) + patcher.start() + self.addCleanup(patcher.stop) + + def test_omitted_and_false_without_hosts_are_noop_options(self): + for enabled in (None, False): + with self.subTest(enabled=enabled): + self.assertIsNone(plugin.validate_plugin_options(enabled)) + + def test_explicit_hosts_are_deduplicated_in_supported_order(self): + self.assertEqual(plugin.validate_plugin_options(True, ['codex', 'claude-code', 'codex', 'github-copilot']), + ['claude-code', 'github-copilot', 'codex']) + self.assertEqual(plugin.validate_plugin_options(True, ['codex']), ['codex']) + + def test_invalid_option_relationships_fail_before_any_external_work(self): + cases = [(True, None), (True, []), (True, ['']), (True, ['pi']), (True, ['codex', 'other']), + (None, ['codex']), (False, ['codex']), (None, []), (False, [])] + with mock.patch.object(plugin.shutil, 'which') as which, \ + mock.patch.object(plugin.subprocess, 'run') as run: + for enabled, hosts in cases: + with self.subTest(enabled=enabled, hosts=hosts), self.assertRaises(InvalidArgumentValueError): + plugin.validate_plugin_options(enabled, hosts) + which.assert_not_called() + run.assert_not_called() + + def test_explicit_install_rejects_sudo_but_binary_only_options_remain_valid(self): + for variable, value in (('SUDO_USER', 'someone'), ('SUDO_UID', '0')): + with self.subTest(variable=variable), mock.patch.dict(os.environ, {variable: value}): + with self.assertRaisesRegex(InvalidArgumentValueError, 'sudo'): + plugin.validate_plugin_options(True, ['codex']) + self.assertIsNone(plugin.validate_plugin_options()) + self.assertIsNone(plugin.validate_plugin_options(False)) + + +class AzurePluginConsentTest(unittest.TestCase): + def setUp(self): + self.cmd = mock.Mock() + self.cmd.cli_ctx.config.getboolean.return_value = False + self.prompts = [] + self.answers = iter([]) + self.stderr = io.StringIO() + self.installed = [] + patches = ( + mock.patch.dict(os.environ, {}, clear=True), + mock.patch('sys.stdin.isatty', return_value=True), + mock.patch('sys.stderr', self.stderr), + mock.patch('knack.prompting._input', side_effect=self.answer), + mock.patch.object(plugin.shutil, 'which', side_effect=lambda name: name if name == 'codex' else None), + mock.patch.object(plugin, 'install_plugin', side_effect=self.install), + mock.patch.object(plugin.subprocess, 'run', side_effect=AssertionError('Unexpected native process')), + ) + self.mocks = [patcher.start() for patcher in patches] + for patcher in patches: + self.addCleanup(patcher.stop) + + def answer(self, message): + self.prompts.append(message) + result = next(self.answers) + if isinstance(result, BaseException): + raise result + return result + + def install(self, host): + self.installed.append(host) + self.assertIn('MCP', self.stderr.getvalue()) + return True + + def run_optional(self, answers): + self.answers = iter(answers) + plugin.maybe_install_azure_plugin(self.cmd) + + def test_false_no_tty_disabled_confirmation_and_sudo_do_no_discovery_or_execution(self): + for condition in ('false', 'no-tty', 'disabled', 'sudo-user', 'sudo-uid'): + with self.subTest(condition=condition), mock.patch.object(plugin, 'discover_hosts') as discover: + self.mocks[1].return_value = condition != 'no-tty' + self.cmd.cli_ctx.config.getboolean.return_value = condition == 'disabled' + environment = {'SUDO_USER': 'user'} if condition == 'sudo-user' else {} + if condition == 'sudo-uid': + environment = {'SUDO_UID': '0'} + with mock.patch.dict(os.environ, environment, clear=True): + plugin.maybe_install_azure_plugin(self.cmd, False if condition == 'false' else None) + discover.assert_not_called() + self.assertEqual(self.installed, []) + self.assertEqual(self.prompts, []) + self.mocks[4].assert_not_called() + self.mocks[6].assert_not_called() + + def test_initial_offer_defaults_to_no_without_even_discovering(self): + self.run_optional(['']) + self.assertEqual(self.installed, []) + self.assertEqual(len(self.prompts), 1) + self.assertIn('(y/N)', self.prompts[0]) + self.mocks[4].assert_not_called() + + def test_detected_defaults_require_final_default_no_consent(self): + self.run_optional(['y', '', '']) + self.assertEqual(self.installed, []) + self.assertEqual(len(self.prompts), 3) + self.assertIn('(y/N)', self.prompts[-1]) + self.assertIn('Codex CLI', self.prompts[-1]) + self.assertIn('3', self.prompts[1]) + self.assert_disclosure() + + def assert_disclosure(self): + text = self.stderr.getvalue() + for disclosure in ('full Azure plugin', 'user', 'global', 'MCP', 'hooks', 'Node.js 22', 'npx', + '@azure/mcp@latest', 'not pinned', 'native', 'enable', 'hidden', 'stale', + 'disabled', 'authentication', 'trust', 'sovereign', 'updates'): + self.assertIn(disclosure, text) + + def test_detected_host_installs_only_after_final_consent(self): + def install(host): + self.assertEqual(len(self.prompts), 3) + self.assert_disclosure() + return self.install(host) + self.mocks[5].side_effect = install + self.run_optional(['y', '', 'y']) + self.assertEqual(self.installed, ['codex']) + + def test_numbered_selection_replaces_defaults_and_deduplicates(self): + self.run_optional(['y', '2 1 2', 'y']) + self.assertEqual(self.installed, ['claude-code', 'github-copilot']) + self.assertNotIn('Codex CLI', self.prompts[-1]) + + def test_all_hosts_can_be_deselected_without_final_prompt(self): + self.run_optional(['y', '0']) + self.assertEqual(self.installed, []) + self.assertEqual(len(self.prompts), 2) + + def test_no_detected_hosts_blank_selection_does_not_consent(self): + self.mocks[4].side_effect = lambda name: None + self.run_optional(['y', '']) + self.assertEqual(self.installed, []) + self.assertEqual(len(self.prompts), 2) + + def test_invalid_selection_reprompts_without_installing(self): + self.run_optional(['y', '4', '0 1', '-1', 'codex', '1,2', '2', 'y']) + self.assertEqual(self.installed, ['github-copilot']) + self.assertEqual(len(self.prompts), 8) + + def test_cancellation_eof_or_lost_tty_at_each_prompt_is_optional_noop(self): + for error in (KeyboardInterrupt, EOFError, NoTTYException): + for answers in ([error()], ['y', error()], ['y', '', error()]): + with self.subTest(error=error, stage=len(answers)): + self.run_optional(answers) + self.assertEqual(self.installed, []) + + def test_explicit_hosts_need_no_tty_prompts_or_detection_even_with_confirm_disabled(self): + self.mocks[1].return_value = False + self.cmd.cli_ctx.config.getboolean.return_value = True + with mock.patch.object(plugin, 'discover_hosts') as discover: + plugin.maybe_install_azure_plugin(self.cmd, True, ['codex', 'claude-code', 'codex']) + self.assertEqual(self.installed, ['claude-code', 'codex']) + self.assertEqual(self.prompts, []) + discover.assert_not_called() + self.assert_disclosure() + + def test_expected_failures_continue_hosts_and_preserve_explicit_error_category(self): + for error_type in (ClientRequestError, ResourceNotFoundError, ValidationError): + calls = [] + + def install(host): + calls.append(host) + if host == 'claude-code': + raise error_type('first host failure') + if host == 'codex': + raise ValidationError('last host failure') + return True + + self.mocks[5].side_effect = install + with self.subTest(error=error_type), self.assertRaises(error_type) as caught: + plugin.maybe_install_azure_plugin(self.cmd, True, ['codex', 'github-copilot', 'claude-code']) + message = str(caught.exception) + for part in ('kubectl', 'kubelogin', 'remain installed', 'native plugin commands', 'rollback', + 'Claude Code', 'first host failure', 'Codex CLI', 'last host failure', + 'GitHub Copilot CLI', 'installed'): + self.assertIn(part, message) + self.assertEqual(calls, ['claude-code', 'github-copilot', 'codex']) + + def test_optional_failure_warns_without_failing_and_remaining_host_still_installs(self): + self.mocks[5].side_effect = [ValidationError('bad inventory'), True] + with self.assertLogs('cli.' + plugin.__name__, level='WARNING') as logs: + self.run_optional(['y', '1 3', 'y']) + message = '\n'.join(logs.output) + for part in ('Claude Code', 'bad inventory', 'Codex CLI', 'installed', 'remain installed', 'native'): + self.assertIn(part, message) + self.assertEqual(self.mocks[5].call_args_list, [mock.call('claude-code'), mock.call('codex')]) + + def test_report_distinguishes_install_from_skip_without_claiming_zero_marketplace_changes(self): + self.mocks[5].side_effect = [True, False] + plugin.maybe_install_azure_plugin(self.cmd, True, ['claude-code', 'codex']) + message = self.stderr.getvalue() + self.assertIn('Claude Code: installed', message) + self.assertIn('Codex CLI: Azure already reported; plugin install skipped', message) + self.assertIn('marketplace may have been added', message) + self.assertNotIn('all integrations are ready', message) + + def test_programming_errors_are_not_swallowed_on_optional_or_explicit_paths(self): + self.mocks[5].side_effect = TypeError('programming error') + with self.assertRaisesRegex(TypeError, 'programming error'): + self.run_optional(['y', '1', 'y']) + with self.assertRaisesRegex(TypeError, 'programming error'): + plugin.maybe_install_azure_plugin(self.cmd, True, ['codex']) + + def test_mcp_sensitive_errors_stay_redacted_through_flow_aggregation(self): + secret = 'synthetic-mcp-flow-secret' + # Exercise real Task 1 parsing and redaction inside the new aggregation path. + self.mocks[5].side_effect = REAL_INSTALL_PLUGIN + self.mocks[4].side_effect = lambda name: name + self.mocks[6].side_effect = NativeProcessFixture({ + ('copilot', 'plugin', 'list', '--json'): [], + ('copilot', 'mcp', 'list', '--json'): subprocess.CompletedProcess([], 7, secret, secret), + }) + with self.assertRaises(ClientRequestError) as caught: + plugin.maybe_install_azure_plugin(self.cmd, True, ['github-copilot']) + rendered = ''.join(traceback.format_exception(caught.exception)) + self.assertNotIn(secret, rendered + self.stderr.getvalue()) + self.assertIn('MCP inventory', str(caught.exception)) + self.assertIn('exit 7', str(caught.exception)) + + def test_explicit_sudo_is_rejected_before_any_execution(self): + with mock.patch.dict(os.environ, {'SUDO_UID': '123'}), self.assertRaises(InvalidArgumentValueError): + plugin.maybe_install_azure_plugin(self.cmd, True, ['codex']) + self.assertEqual(self.installed, []) + self.assertEqual(self.prompts, []) + + class AzurePluginNativeTest(unittest.TestCase): def setUp(self): patcher = mock.patch.object(plugin.shutil, 'which', side_effect=lambda name: name) diff --git a/src/azure-cli/azure/cli/command_modules/acs/tests/latest/test_custom.py b/src/azure-cli/azure/cli/command_modules/acs/tests/latest/test_custom.py index 445565898a2..98d4a1a6032 100644 --- a/src/azure-cli/azure/cli/command_modules/acs/tests/latest/test_custom.py +++ b/src/azure-cli/azure/cli/command_modules/acs/tests/latest/test_custom.py @@ -6,8 +6,10 @@ import os import hashlib import io +import json import shutil import subprocess +import sys import tarfile import tempfile import unittest @@ -50,6 +52,7 @@ aks_stop, aks_upgrade, is_monitoring_addon_enabled, + k8s_install_cli, k8s_install_kubectl, k8s_install_kubelogin, merge_kubernetes_configurations, @@ -84,6 +87,172 @@ ) +class AcsInstallCliPluginTest(unittest.TestCase): + def setUp(self): + self.cmd = mock.Mock() + self.calls = mock.Mock() + for name, kwargs in ( + ('get_arch_for_cli_binary', {'return_value': 'arm64'}), + ('k8s_install_kubectl', {}), ('k8s_install_kubelogin', {}), + ('maybe_install_azure_plugin', {'create': True})): + patcher = mock.patch('azure.cli.command_modules.acs.custom.' + name, **kwargs) + self.calls.attach_mock(patcher.start(), name) + self.addCleanup(patcher.stop) + patcher = mock.patch.dict(os.environ, {}, clear=True) + patcher.start() + self.addCleanup(patcher.stop) + + def test_default_binary_arguments_and_plugin_stage_order_are_preserved(self): + k8s_install_cli(self.cmd) + self.assertEqual(self.calls.mock_calls, [ + mock.call.get_arch_for_cli_binary(), + mock.call.k8s_install_kubectl(self.cmd, 'latest', None, None, arch='arm64'), + mock.call.k8s_install_kubelogin(self.cmd, 'latest', None, None, arch='arm64', gh_token=None), + mock.call.maybe_install_azure_plugin(self.cmd, None, None), + ]) + + def test_explicit_hosts_normalize_without_changing_binary_arguments_or_forwarding_token(self): + k8s_install_cli(self.cmd, '1.2.3', '/kubectl', 'https://kubectl.example', + '4.5.6', '/kubelogin', 'https://kubelogin.example', 'binary-only-token', + True, ['codex', 'claude-code', 'codex']) + self.assertEqual(self.calls.mock_calls, [ + mock.call.get_arch_for_cli_binary(), + mock.call.k8s_install_kubectl(self.cmd, '1.2.3', '/kubectl', 'https://kubectl.example', arch='arm64'), + mock.call.k8s_install_kubelogin(self.cmd, '4.5.6', '/kubelogin', 'https://kubelogin.example', + arch='arm64', gh_token='binary-only-token'), + mock.call.maybe_install_azure_plugin(self.cmd, True, ['claude-code', 'codex']), + ]) + + def test_false_is_forwarded_after_both_binaries(self): + k8s_install_cli(self.cmd, install_azure_plugin=False) + self.assertEqual(self.calls.mock_calls[-1], mock.call.maybe_install_azure_plugin(self.cmd, False, None)) + self.assertEqual(len(self.calls.mock_calls), 4) + + def test_invalid_plugin_options_are_rejected_before_binary_side_effects(self): + for enabled, hosts in ((True, None), (True, []), (True, ['pi']), (False, ['codex']), (None, ['codex'])): + with self.subTest(enabled=enabled, hosts=hosts), self.assertRaises(InvalidArgumentValueError): + k8s_install_cli(self.cmd, install_azure_plugin=enabled, plugin_hosts=hosts) + self.assertEqual(self.calls.mock_calls, []) + with mock.patch.dict(os.environ, {'SUDO_USER': 'user'}), self.assertRaises(InvalidArgumentValueError): + k8s_install_cli(self.cmd, install_azure_plugin=True, plugin_hosts=['codex']) + self.assertEqual(self.calls.mock_calls, []) + + def test_either_binary_failure_prevents_plugin_stage(self): + for binary in ('k8s_install_kubectl', 'k8s_install_kubelogin'): + with self.subTest(binary=binary): + self.calls.reset_mock(side_effect=True) + getattr(self.calls, binary).side_effect = ClientRequestError('binary failed') + with self.assertRaisesRegex(ClientRequestError, 'binary failed'): + k8s_install_cli(self.cmd, install_azure_plugin=True, plugin_hosts=['codex']) + self.calls.maybe_install_azure_plugin.assert_not_called() + if binary == 'k8s_install_kubectl': + self.calls.k8s_install_kubelogin.assert_not_called() + + +# A new interpreter keeps import-time cloud/extension paths and CLI global state +# isolated even when this file is run outside the repository's test environment. +_PLUGIN_PARSER_SCRIPT = r''' +import io +import json +import os +import sys +from contextlib import ExitStack, redirect_stdout, redirect_stderr +from unittest import mock + +blocked = [] +def forbidden(*args, **kwargs): + blocked.append(repr(args)) + raise AssertionError('Parser fixture attempted external I/O: ' + repr(args)) + +with ExitStack() as stack: + for target in ('socket.getaddrinfo', 'socket.socket.connect', 'socket.socket.connect_ex', + 'subprocess.Popen', 'os.system', 'requests.sessions.Session.request'): + stack.enter_context(mock.patch(target, side_effect=forbidden)) + # Patch before construction: handle_version_update may query freshness even + # when ordinary CLI version-check configuration is disabled. + stack.enter_context(mock.patch('uuid.getnode', return_value=0x123456789ABC)) + stack.enter_context(mock.patch('azure.cli.core.util.check_connectivity', return_value=False)) + from azure.cli.core import get_default_cli + from azure.cli.core import cloud, extension + from azure.cli.command_modules.acs import custom, _azure_plugin + assert cloud.CLOUD_CONFIG_FILE.startswith(os.environ['AZURE_CONFIG_DIR']) + assert extension.EXTENSIONS_DIR == os.environ['AZURE_EXTENSION_DIR'] + cli = get_default_cli() + assert cli.cloud.name == 'AzureCloud' + assert cli.config.config_dir == os.environ['AZURE_CONFIG_DIR'] + cli.config.set_value('core', 'collect_telemetry', 'no') + cli.config.set_value('core', 'check_version', 'no') + mode, arguments = json.loads(sys.argv[1]) + if mode == 'parser': + handler = stack.enter_context(mock.patch.object(custom, 'k8s_install_cli', autospec=True)) + handler.return_value = None + else: + stack.enter_context(mock.patch.object(custom, 'k8s_install_kubectl')) + stack.enter_context(mock.patch.object(custom, 'k8s_install_kubelogin')) + handler = stack.enter_context(mock.patch.object(_azure_plugin, 'install_plugin', return_value=True)) + output, errors = io.StringIO(), io.StringIO() + with redirect_stdout(output), redirect_stderr(errors): + try: + code = cli.invoke(['aks', 'install-cli', *arguments]) + except SystemExit as ex: + code = ex.code + assert not blocked, blocked + calls = [{key: value for key, value in call.kwargs.items() if key != 'cmd'} + for call in handler.call_args_list] + hosts = [call.args[0] for call in handler.call_args_list] if mode != 'parser' else [] + print(json.dumps({'code': code, 'calls': calls, 'hosts': hosts, 'stderr': errors.getvalue()})) +''' + + +class AcsInstallCliPluginParserTest(unittest.TestCase): + def invoke_isolated(self, arguments, mode='parser'): + with tempfile.TemporaryDirectory() as root: + environment = {key: value for key, value in os.environ.items() + if not key.startswith(('AZURE_', 'ARM_', 'SUDO_', 'XDG_', 'CLAUDE_', 'COPILOT_', 'CODEX_'))} + for key in ('HOME', 'USERPROFILE', 'AZURE_CONFIG_DIR', 'AZURE_EXTENSION_DIR', 'AZURE_EXTENSION_SYS_DIR', + 'XDG_CONFIG_HOME', 'XDG_CACHE_HOME', 'XDG_DATA_HOME', 'CLAUDE_CONFIG_DIR', 'COPILOT_HOME', + 'COPILOT_CACHE_HOME', 'CODEX_HOME'): + environment[key] = os.path.join(root, key.lower()) + os.makedirs(environment[key]) + environment.update(AZURE_CORE_COLLECT_TELEMETRY='no', AZURE_CORE_CHECK_VERSION='no', + PYTHONDONTWRITEBYTECODE='1', PYTHONNOUSERSITE='1', + PYTHONPATH=os.pathsep.join(path for path in sys.path if path)) + result = subprocess.run([sys.executable, '-B', '-c', _PLUGIN_PARSER_SCRIPT, json.dumps([mode, arguments])], + cwd=root, env=environment, stdin=subprocess.DEVNULL, capture_output=True, + text=True, timeout=90, check=False) + self.assertEqual(result.returncode, 0, result.stdout + result.stderr) + return json.loads(result.stdout) + + def test_real_parser_registers_three_states_and_all_host_choices(self): + for arguments, enabled, hosts in ( + ([], None, None), (['--install-azure-plugin', 'false'], False, None), + (['--install-azure-plugin', '--plugin-hosts', 'claude-code', 'github-copilot', 'codex'], + True, ['claude-code', 'github-copilot', 'codex']), + (['--install-azure-plugin', 'true', '--plugin-hosts', 'codex'], True, ['codex'])): + with self.subTest(arguments=arguments): + result = self.invoke_isolated(arguments) + self.assertEqual(result['code'], 0, result['stderr']) + self.assertEqual(len(result['calls']), 1) + self.assertEqual(result['calls'][0]['install_azure_plugin'], enabled) + self.assertEqual(result['calls'][0]['plugin_hosts'], hosts) + + def test_real_parser_rejects_unknown_or_empty_hosts_before_handler(self): + for hosts in (['pi'], ['vscode'], ['aks'], []): + with self.subTest(hosts=hosts): + result = self.invoke_isolated(['--install-azure-plugin', '--plugin-hosts', *hosts]) + self.assertEqual(result['code'], 2) + self.assertEqual(result['calls'], []) + + def test_explicit_disclosure_is_visible_with_only_show_errors(self): + result = self.invoke_isolated(['--install-azure-plugin', '--plugin-hosts', 'codex', '--only-show-errors'], + mode='flow') + self.assertEqual(result['code'], 0, result['stderr']) + self.assertEqual(result['hosts'], ['codex']) + for disclosure in ('full Azure plugin', 'user/global', 'MCP', 'hooks', '@azure/mcp@latest', + 'hidden/stale', 'enablement', 'authentication', 'trust'): + self.assertIn(disclosure, result['stderr']) + + class AcsCustomCommandTest(unittest.TestCase): def setUp(self): self.cli = MockCLI() From a621626d50d2ec4f53c0d5a14a8420b2e2f97a44 Mon Sep 17 00:00:00 2001 From: gambtho Date: Tue, 22 Sep 2026 13:43:25 -0400 Subject: [PATCH 4/7] [AKS] Exercise plugin installation through isolated CLI scenario --- .../tests/latest/test_aks_install_plugin.py | 287 ++++++++++++++++++ 1 file changed, 287 insertions(+) create mode 100644 src/azure-cli/azure/cli/command_modules/acs/tests/latest/test_aks_install_plugin.py diff --git a/src/azure-cli/azure/cli/command_modules/acs/tests/latest/test_aks_install_plugin.py b/src/azure-cli/azure/cli/command_modules/acs/tests/latest/test_aks_install_plugin.py new file mode 100644 index 00000000000..95e2fe30d0a --- /dev/null +++ b/src/azure-cli/azure/cli/command_modules/acs/tests/latest/test_aks_install_plugin.py @@ -0,0 +1,287 @@ +# -------------------------------------------------------------------------------------------- +# Copyright (c) Microsoft Corporation. All rights reserved. +# Licensed under the MIT License. See License.txt in the project root for license information. +# -------------------------------------------------------------------------------------------- + +import io +import json +import os +from pathlib import Path +import stat +import subprocess +import sys +import tempfile +import unittest +from unittest import mock +import zipfile + + +# An owned executable, not a mocked subprocess result. The state follows Codex +# rust-v0.144.3 JsonPluginListEntry/JsonMarketplaceListEntry, not native config files. +_HOST_SCRIPT = r''' +import json +import os +from pathlib import Path +import sys + +root = Path.cwd() +args = sys.argv[1:] +name = Path(sys.argv[0]).name +assert sys.stdin.read() == '', 'Native stdin must be closed' +assert os.environ['CODEX_HOME'] == str(root / 'codex_home') +assert (root / 'installed/kubectl').read_bytes() == b'fixture kubectl\n' +assert (root / 'installed/kubelogin').read_bytes() == b'fixture kubelogin\n' +with (root / 'native.jsonl').open('a') as stream: + stream.write(json.dumps([name, *args]) + '\n') +if name == 'node': + assert args == ['--version'], args + print('v22.0.0') + sys.exit(0) +assert name == 'codex', name +state_path = root / 'codex_home/state.json' +state = json.loads(state_path.read_text()) if state_path.exists() else {'installed': [], 'marketplaces': []} +if args == ['plugin', 'list', '--available', '--json']: + print(json.dumps({'installed': state['installed'], 'available': []})) +elif args == ['plugin', 'marketplace', 'list', '--json']: + print(json.dumps({'marketplaces': state['marketplaces']})) +elif args == ['plugin', 'marketplace', 'add', 'microsoft/azure-skills', '--json']: + assert not state['marketplaces'], 'Marketplace must not be re-added' + state['marketplaces'] = [{ + 'name': 'azure-skills', 'root': str(root / 'codex_home/marketplace'), + 'marketplaceSource': {'sourceType': 'git', 'source': 'https://github.com/microsoft/azure-skills.git'}, + }] + state_path.write_text(json.dumps(state)) + print('{}') +elif args == ['plugin', 'add', 'azure@azure-skills', '--json']: + assert state['marketplaces'], 'Marketplace must be registered first' + # Like native add, another add would reinstall and re-enable this plugin. + state['installed'] = [{ + 'pluginId': 'azure@azure-skills', 'name': 'azure', 'marketplaceName': 'azure-skills', + 'version': '1.0.0', 'installed': True, 'enabled': True, + 'source': {'source': 'local', 'path': str(root / 'codex_home/marketplace/azure')}, + 'marketplaceSource': {'sourceType': 'git', 'source': 'https://github.com/microsoft/azure-skills.git'}, + 'installPolicy': 'AVAILABLE', 'authPolicy': 'ON_USE', + }] + state_path.write_text(json.dumps(state)) + print('{}') +else: + raise AssertionError('Unexpected native command: ' + repr(args)) +''' + + +_ENV_DIRS = ( + 'HOME', 'USERPROFILE', 'AZURE_CONFIG_DIR', 'AZURE_EXTENSION_DIR', 'AZURE_EXTENSION_SYS_DIR', + 'XDG_CONFIG_HOME', 'XDG_CACHE_HOME', 'XDG_DATA_HOME', 'CLAUDE_CONFIG_DIR', + 'COPILOT_HOME', 'COPILOT_CACHE_HOME', 'CODEX_HOME', 'TMPDIR', +) + + +def _run_cli_scenario(mode): + # This function runs only in a new interpreter: no caller's cached Azure + # cloud, extensions, SDK sessions, credentials or version state can leak in. + from contextlib import ExitStack + import socket + + root = Path.cwd() + blocked = [] + writable = [root / 'azure_config_dir', root / 'installed', root / 'tmpdir'] + executables = [str(root / 'bin' / name) for name in ('codex', 'node')] + + def forbidden(event, args): + blocked.append(event) + raise AssertionError('Forbidden fixture I/O: ' + event + ' ' + repr(args)) + + def audit(event, args): + # urllib3's local IPv6 capability probe binds ::1 without sending data. + local_probe = event == 'socket.bind' and args[1] == ('::1', 0) + if event.startswith('socket.') and event != 'socket.__new__' and not local_probe: + forbidden(event, args) + if event in ('os.system', 'os.exec'): + forbidden(event, args) + # Python 3.14 can use posix_spawn underneath the real Popen. + if event in ('subprocess.Popen', 'os.posix_spawn') and args[0] not in executables: + forbidden(event, args) + paths = [] + if event == 'open' and isinstance(args[0], (str, bytes, os.PathLike)) and args[0] != os.devnull: + if args[2] & (os.O_WRONLY | os.O_RDWR | os.O_CREAT | os.O_TRUNC | os.O_APPEND): + paths = [args[0]] + if event in ('os.mkdir', 'os.remove', 'os.rmdir', 'os.chmod', 'os.truncate'): + paths = [args[0]] + if event in ('os.rename', 'os.link', 'os.symlink'): + paths = list(args[:2]) + for path in paths: + if not isinstance(path, (str, bytes, os.PathLike)): + continue + resolved = Path(os.fsdecode(path)) + # shutil.rmtree uses directory-relative unlink/rmdir on Linux. + if event in ('os.remove', 'os.rmdir') and args[1] != -1: + resolved = Path(os.readlink('/proc/self/fd/' + str(args[1]))) / resolved + resolved = resolved.resolve() + if not any(resolved == base or base in resolved.parents for base in writable): + forbidden(event, args) + + sys.addaudithook(audit) + with ExitStack() as stack: + stack.enter_context(mock.patch('uuid.getnode', return_value=0x123456789ABC)) + stack.enter_context(mock.patch('requests.sessions.Session.request', + side_effect=lambda *a, **kw: forbidden('requests', a))) + # handle_version_update checks freshness even with check_version=no. + connectivity = stack.enter_context(mock.patch('azure.cli.core.util.check_connectivity', return_value=False)) + from azure.cli.core import cloud, extension + from azure.cli.core._config import GLOBAL_CONFIG_DIR + from azure.cli.core.mock import DummyCli + + assert GLOBAL_CONFIG_DIR == str(root / 'azure_config_dir') + assert cloud.CLOUD_CONFIG_FILE == str(root / 'azure_config_dir/clouds.config') + assert extension.EXTENSIONS_DIR == str(root / 'azure_extension_dir') + assert extension.EXTENSIONS_SYS_DIR == str(root / 'azure_extension_sys_dir') + assert extension.DEV_EXTENSION_SOURCES == [] + assert 'ARM_CLOUD_METADATA_URL' not in os.environ + assert not any(key.startswith('SUDO_') for key in os.environ) + + if mode == 'guards': + # Prove attempts are counted even when a caller swallows the error. + probes = [ + lambda: socket.getaddrinfo('fixture.invalid', 443), + lambda: subprocess.run(['/forbidden/real-host'], check=False), + lambda: (root / 'codex_home/config.toml').write_text('forbidden'), + ] + with socket.socket() as connection: + probes.append(lambda: connection.connect(('127.0.0.1', 443))) + for probe in probes: + try: + probe() + except AssertionError: + pass + assert blocked == ['socket.getaddrinfo', 'subprocess.Popen', 'open', 'socket.connect'], blocked + # Failure during the actual constructor must propagate, while the + # parent still removes all disposable config and executable files. + connectivity.side_effect = RuntimeError('injected startup failure after four blocked probes') + DummyCli() + raise AssertionError('Startup failure was not propagated') + + cli = DummyCli() + assert connectivity.called, 'Cold startup connectivity was not isolated' + assert cli.cloud.name == 'AzureCloud' + assert cli.config.config_dir == str(root / 'azure_config_dir') + from azure.cli.command_modules.acs import custom + + archive = io.BytesIO() + with zipfile.ZipFile(archive, 'w') as stream: + stream.writestr('bin/linux_amd64/kubelogin', b'fixture kubelogin\n') + payloads = { + 'https://dl.k8s.io/release/v1.2.3/bin/linux/amd64/kubectl': b'fixture kubectl\n', + 'https://github.com/Azure/kubelogin/releases/download/v4.5.6/kubelogin.zip': archive.getvalue(), + } + downloads = [] + + def transport(request, **kwargs): + del kwargs + url = request.full_url + assert url in payloads, url + assert request.get_header('Authorization') is None + downloads.append(url) + return io.BytesIO(payloads[url]) + + stack.enter_context(mock.patch.object(custom, 'urlopen', side_effect=transport)) + stack.enter_context(mock.patch('platform.system', return_value='Linux')) + stack.enter_context(mock.patch('platform.machine', return_value='x86_64')) + arguments = [ + 'aks', 'install-cli', '--client-version', '1.2.3', '--kubelogin-version', '4.5.6', + '--install-location', str(root / 'installed/kubectl'), + '--kubelogin-install-location', str(root / 'installed/kubelogin'), + '--install-azure-plugin', '--plugin-hosts', 'codex', + ] + assert cli.invoke(arguments) == 0 + state_path = root / 'codex_home/state.json' + state = json.loads(state_path.read_text()) + assert state['installed'][0]['pluginId'] == 'azure@azure-skills' + assert state['installed'][0]['enabled'] is True + assert state['marketplaces'][0]['name'] == 'azure-skills' + native_log = root / 'native.jsonl' + calls = [json.loads(line) for line in native_log.read_text().splitlines()] + assert calls == [ + ['codex', 'plugin', 'list', '--available', '--json'], + ['codex', 'plugin', 'marketplace', 'list', '--json'], + ['node', '--version'], + ['codex', 'plugin', 'marketplace', 'add', 'microsoft/azure-skills', '--json'], + ['codex', 'plugin', 'list', '--available', '--json'], + ['codex', 'plugin', 'add', 'azure@azure-skills', '--json'], + ], calls + for name, payload in (('kubectl', b'fixture kubectl\n'), ('kubelogin', b'fixture kubelogin\n')): + binary = root / 'installed' / name + assert binary.read_bytes() == payload + assert stat.S_IMODE(binary.stat().st_mode) & 0o111 == 0o111 + + # Fixture-owned user change, outside CLI execution. A native reinstall + # would turn enabled back on; even rewriting identical state is rejected. + writable.append(root / 'codex_home') + state['installed'][0]['enabled'] = False + state_path.write_text(json.dumps(state)) + writable.pop() + before = (state_path.read_bytes(), state_path.stat().st_mtime_ns) + assert cli.invoke(arguments) == 0 + assert (state_path.read_bytes(), state_path.stat().st_mtime_ns) == before + repeated_calls = [json.loads(line) for line in native_log.read_text().splitlines()] + assert repeated_calls == calls + [['codex', 'plugin', 'list', '--available', '--json']], repeated_calls + assert downloads == list(payloads) * 2, downloads + assert list((root / 'tmpdir').iterdir()) == [] + for directory in _ENV_DIRS: + path = root / directory.lower() + if directory not in ('AZURE_CONFIG_DIR', 'CODEX_HOME'): + assert list(path.iterdir()) == [], str(path) + assert sorted(path.name for path in (root / 'codex_home').iterdir()) == ['state.json'] + assert not blocked, blocked + print('Fresh install and disabled-existing rerun verified; no forbidden I/O') + + +@unittest.skipUnless(sys.platform.startswith('linux'), 'Owned executable and fd guards use Linux') +class AKSInstallPluginScenarioTest(unittest.TestCase): + def invoke_isolated(self, mode='scenario'): + with tempfile.TemporaryDirectory() as sandbox: + root = Path(sandbox) + # Allowlist, rather than inheriting tokens, sudo, ARM metadata, host + # homes, extension dev sources or the user's executable search path. + environment = { + 'PATH': str(root / 'bin'), 'PYTHONPATH': os.pathsep.join(path for path in sys.path if path), + 'PYTHONDONTWRITEBYTECODE': '1', 'PYTHONNOUSERSITE': '1', + 'AZURE_CORE_COLLECT_TELEMETRY': 'no', 'AZURE_CORE_CHECK_VERSION': 'no', + 'LANG': 'C.UTF-8', + } + for key in _ENV_DIRS: + directory = root / key.lower() + directory.mkdir() + environment[key] = str(directory) + environment['TMP'] = environment['TEMP'] = environment['TMPDIR'] + (root / 'azure_config_dir/config').write_text('[cloud]\nname = AzureCloud\n') + (root / 'bin').mkdir() + (root / 'installed').mkdir() + for name in ('codex', 'node', 'npx'): + executable = root / 'bin' / name + executable.write_text('#!' + sys.executable + '\n' + _HOST_SCRIPT) + executable.chmod(0o755) + result = subprocess.run([sys.executable, '-B', str(Path(__file__).resolve()), mode], + cwd=root, env=environment, stdin=subprocess.DEVNULL, + capture_output=True, text=True, timeout=90, check=False) + self.assertFalse(root.exists(), 'Fixture cleanup left host/config files behind') + return result + + def test_install_plugin_real_cli_preserves_disabled_existing_plugin(self): + # Hostile caller state must never reach the child, even when azdev has + # already imported cloud/extension modules and warmed their global paths. + with mock.patch.dict(os.environ, { + 'ARM_CLOUD_METADATA_URL': 'https://fixture.invalid/metadata', 'SUDO_UID': '123', + 'AZURE_EXTENSION_DEV_SOURCES': '/forbidden/extensions', 'CODEX_HOME': '/forbidden/codex', + 'AZURE_CONFIG_DIR': '/forbidden/azure', 'HOME': '/forbidden/home'}): + result = self.invoke_isolated() + self.assertEqual(result.returncode, 0, result.stdout + result.stderr) + self.assertIn('Fresh install and disabled-existing rerun verified; no forbidden I/O', result.stdout) + + def test_install_plugin_fixture_guards_and_constructor_failure_cleanup(self): + result = self.invoke_isolated('guards') + self.assertNotEqual(result.returncode, 0) + self.assertIn('injected startup failure after four blocked probes', result.stderr) + + +if __name__ == '__main__': + _run_cli_scenario(sys.argv[1]) From 56a29a0945f805207f0d563cdb33bee1dc40f567 Mon Sep 17 00:00:00 2001 From: gambtho Date: Tue, 22 Sep 2026 14:05:40 -0400 Subject: [PATCH 5/7] [AKS] Harden native plugin inventory and process cleanup --- .../cli/command_modules/acs/_azure_plugin.py | 103 ++++++- .../tests/latest/test_aks_install_plugin.py | 15 +- .../acs/tests/latest/test_azure_plugin.py | 284 ++++++++++++++++-- .../acs/tests/latest/test_custom.py | 2 +- 4 files changed, 361 insertions(+), 43 deletions(-) diff --git a/src/azure-cli/azure/cli/command_modules/acs/_azure_plugin.py b/src/azure-cli/azure/cli/command_modules/acs/_azure_plugin.py index 2ce163c536c..b444d57555e 100644 --- a/src/azure-cli/azure/cli/command_modules/acs/_azure_plugin.py +++ b/src/azure-cli/azure/cli/command_modules/acs/_azure_plugin.py @@ -9,6 +9,7 @@ import os import re import shutil +import signal import subprocess import sys @@ -77,6 +78,11 @@ def _install_selected_hosts(hosts, *, explicit): for host in hosts: try: installed = install_plugin(host) + except KeyboardInterrupt: + outcomes.append(f'{HOST_LABELS[host]}: interrupted; native state is uncertain.') + # Cancellation must disclose partial state even with --only-show-errors. + print(_setup_summary(outcomes), file=sys.stderr) + raise except (ClientRequestError, ResourceNotFoundError, ValidationError) as ex: failures.append(ex) outcomes.append(f'{HOST_LABELS[host]}: failed. {ex}') @@ -87,16 +93,20 @@ def _install_selected_hosts(hosts, *, explicit): outcomes.append(outcome) print(outcome, file=sys.stderr) if failures: - summary = '\n'.join(outcomes) - message = ('Azure plugin setup did not complete for all selected hosts. ' - f'kubectl and kubelogin remain installed.\n{summary}\n' - "Inspect and recover using each host's native plugin commands. " - 'No automatic retry or rollback was attempted.') + message = _setup_summary(outcomes) if explicit: raise type(failures[0])(message) from None logger.warning(message) +def _setup_summary(outcomes): + summary = '\n'.join(outcomes) + return ('Azure plugin setup did not complete for all selected hosts. ' + f'kubectl and kubelogin remain installed.\n{summary}\n' + "Inspect and recover using each host's native plugin commands. " + 'No automatic retry or rollback was attempted.') + + def _select_hosts(): detected = discover_hosts() choices = '\n'.join(f' {index}. {HOST_LABELS[host]}' + (' [detected]' if host in detected else '') @@ -228,8 +238,7 @@ def _azure_registered(host_id, inventory): def _check_copilot_mcp(executable): - # MCP command arguments and environment values can contain credentials. - inventory = _inventory('github-copilot', [executable, 'mcp', 'list', '--json'], 'MCP inventory', sensitive=True) + inventory = _inventory('github-copilot', [executable, 'mcp', 'list', '--json'], 'MCP inventory') servers = inventory.get('mcpServers') if isinstance(inventory, dict) else None if not isinstance(servers, dict) or any( not isinstance(row, dict) or not _text(row.get('source')) or not isinstance(row.get('enabled'), bool) @@ -241,17 +250,28 @@ def _check_copilot_mcp(executable): 'Resolve this collision manually before installing.')) -def _inventory(host_id, argv, operation, *, sensitive=False): - result = _run(host_id, argv, operation, sensitive=sensitive) +def _inventory(host_id, argv, operation): + # All inventories can carry credentials: Git URLs as well as MCP args/env. + result = _run(host_id, argv, operation, sensitive=True) # A warning may mean the host skipped unreadable configuration, not an empty inventory. if result.stderr.strip(): - detail = f'Native inventory warning: {_diagnostic(result.stderr, sensitive=sensitive)}' + detail = f'Native inventory warning: {_diagnostic(result.stderr, sensitive=True)}' raise ValidationError(_failure(host_id, operation, detail)) try: - return json.loads(result.stdout) - except (ValueError, RecursionError) as ex: - detail = f'Invalid native JSON: {_diagnostic(result.stdout, sensitive=sensitive)}' - raise ValidationError(_failure(host_id, operation, detail)) from (None if sensitive else ex) + return json.loads(result.stdout, object_pairs_hook=_unique_object) + except (ValueError, RecursionError): + detail = 'Invalid native JSON (malformed, too deeply nested or duplicate object members).' + raise ValidationError(_failure(host_id, operation, detail)) from None + + +def _unique_object(pairs): + result = {} + for key, value in pairs: + if key in result: + # Do not put potentially sensitive keys or values in exception chains. + raise ValueError('duplicate object member') + result[key] = value + return result def _diagnostic(value, *, sensitive=False): @@ -269,10 +289,61 @@ def _failure(host_id, operation, detail): 'no automatic retry or rollback was attempted.') +def _stop_process_tree(process): + # Own a POSIX session, or target only the Windows launcher's descendant tree. + # This is not containment for a host that deliberately detaches its children. + uncertain = False + output = None + try: + if sys.platform == 'win32': + taskkill = os.path.join(os.environ['SystemRoot'], 'System32', 'taskkill.exe') + result = subprocess.run([taskkill, '/PID', str(process.pid), '/T', '/F'], + stdin=subprocess.DEVNULL, stdout=subprocess.DEVNULL, stderr=subprocess.DEVNULL, + timeout=5, check=False) + uncertain = result.returncode != 0 + else: + try: + os.killpg(process.pid, signal.SIGKILL) + except ProcessLookupError: + pass + except (OSError, subprocess.TimeoutExpired, KeyError): + uncertain = True + try: + process.kill() + output = process.communicate(timeout=5) + except (OSError, subprocess.TimeoutExpired): + uncertain = True + try: + process.wait(timeout=5) + except (OSError, subprocess.TimeoutExpired): + pass + if uncertain: + print('Native process-tree cleanup could not be confirmed; native state is uncertain. ' + "Inspect the host's running processes and recover using its native plugin commands.", file=sys.stderr) + return output + + def _run(host_id, argv, operation, timeout=300, *, sensitive=False): try: - result = subprocess.run(argv, stdin=subprocess.DEVNULL, capture_output=True, - encoding='utf-8', errors='replace', timeout=timeout, check=False) + windows = sys.platform == 'win32' + process = subprocess.Popen(argv, stdin=subprocess.DEVNULL, stdout=subprocess.PIPE, stderr=subprocess.PIPE, + encoding='utf-8', errors='replace', start_new_session=not windows, + creationflags=subprocess.CREATE_NEW_PROCESS_GROUP if windows else 0) + try: + stdout, stderr = process.communicate(timeout=timeout) + except (subprocess.TimeoutExpired, KeyboardInterrupt) as ex: + output = _stop_process_tree(process) + if isinstance(ex, subprocess.TimeoutExpired) and output is not None: + # Windows reader threads only return partial output after EOF. + ex.stdout, ex.stderr = output + raise + finally: + # POSIX has no pipe-reader threads. On Windows communicate owns and + # closes its pipes; closing them here after failed cleanup can block. + if not windows: + process.stdout.close() + process.stderr.close() + result = subprocess.CompletedProcess(argv, process.returncode, stdout, stderr) except subprocess.TimeoutExpired as ex: detail = (f'timed out after {timeout}s. {_diagnostic(ex.stdout, sensitive=sensitive)} ' f'{_diagnostic(ex.stderr, sensitive=sensitive)}') diff --git a/src/azure-cli/azure/cli/command_modules/acs/tests/latest/test_aks_install_plugin.py b/src/azure-cli/azure/cli/command_modules/acs/tests/latest/test_aks_install_plugin.py index 95e2fe30d0a..4bf5cb584cd 100644 --- a/src/azure-cli/azure/cli/command_modules/acs/tests/latest/test_aks_install_plugin.py +++ b/src/azure-cli/azure/cli/command_modules/acs/tests/latest/test_aks_install_plugin.py @@ -156,8 +156,14 @@ def audit(event, args): assert blocked == ['socket.getaddrinfo', 'subprocess.Popen', 'open', 'socket.connect'], blocked # Failure during the actual constructor must propagate, while the # parent still removes all disposable config and executable files. - connectivity.side_effect = RuntimeError('injected startup failure after four blocked probes') - DummyCli() + failure = RuntimeError('injected active-cloud constructor failure') + with mock.patch.object(cloud, 'get_active_cloud', side_effect=failure): + try: + DummyCli() + except RuntimeError as ex: + assert ex is failure, 'Unexpected constructor failure' + print('Caught intended constructor failure after four blocked probes') + sys.exit(23) raise AssertionError('Startup failure was not propagated') cli = DummyCli() @@ -279,8 +285,9 @@ def test_install_plugin_real_cli_preserves_disabled_existing_plugin(self): def test_install_plugin_fixture_guards_and_constructor_failure_cleanup(self): result = self.invoke_isolated('guards') - self.assertNotEqual(result.returncode, 0) - self.assertIn('injected startup failure after four blocked probes', result.stderr) + self.assertEqual(result.returncode, 23, result.stdout + result.stderr) + self.assertIn('Caught intended constructor failure after four blocked probes', result.stdout) + self.assertNotIn('Startup failure was not propagated', result.stderr) if __name__ == '__main__': diff --git a/src/azure-cli/azure/cli/command_modules/acs/tests/latest/test_azure_plugin.py b/src/azure-cli/azure/cli/command_modules/acs/tests/latest/test_azure_plugin.py index 1287e1c8e93..8c3c1b01e27 100644 --- a/src/azure-cli/azure/cli/command_modules/acs/tests/latest/test_azure_plugin.py +++ b/src/azure-cli/azure/cli/command_modules/acs/tests/latest/test_azure_plugin.py @@ -7,8 +7,12 @@ import io import json import os +from pathlib import Path +import signal import subprocess import sys +import tempfile +import time import traceback import unittest from unittest import mock @@ -71,12 +75,14 @@ def __call__(self, argv, **kwargs): response = self.responses[tuple(argv)] if callable(response): response = response() - if isinstance(response, Exception): + if isinstance(response, BaseException): raise response - if isinstance(response, subprocess.CompletedProcess): - return response - stdout = response if isinstance(response, str) else json.dumps(response) - return subprocess.CompletedProcess(argv, 0, stdout, '') + if not isinstance(response, subprocess.CompletedProcess): + stdout = response if isinstance(response, str) else json.dumps(response) + response = subprocess.CompletedProcess(argv, 0, stdout, '') + process = mock.Mock(returncode=response.returncode) + process.communicate.return_value = (response.stdout, response.stderr) + return process class AzurePluginOptionsTest(unittest.TestCase): @@ -99,7 +105,7 @@ def test_invalid_option_relationships_fail_before_any_external_work(self): cases = [(True, None), (True, []), (True, ['']), (True, ['pi']), (True, ['codex', 'other']), (None, ['codex']), (False, ['codex']), (None, []), (False, [])] with mock.patch.object(plugin.shutil, 'which') as which, \ - mock.patch.object(plugin.subprocess, 'run') as run: + mock.patch.object(plugin.subprocess, 'Popen') as run: for enabled, hosts in cases: with self.subTest(enabled=enabled, hosts=hosts), self.assertRaises(InvalidArgumentValueError): plugin.validate_plugin_options(enabled, hosts) @@ -130,7 +136,7 @@ def setUp(self): mock.patch('knack.prompting._input', side_effect=self.answer), mock.patch.object(plugin.shutil, 'which', side_effect=lambda name: name if name == 'codex' else None), mock.patch.object(plugin, 'install_plugin', side_effect=self.install), - mock.patch.object(plugin.subprocess, 'run', side_effect=AssertionError('Unexpected native process')), + mock.patch.object(plugin.subprocess, 'Popen', side_effect=AssertionError('Unexpected native process')), ) self.mocks = [patcher.start() for patcher in patches] for patcher in patches: @@ -301,6 +307,61 @@ def test_mcp_sensitive_errors_stay_redacted_through_flow_aggregation(self): self.assertIn('MCP inventory', str(caught.exception)) self.assertIn('exit 7', str(caught.exception)) + def test_all_inventory_errors_stay_redacted_through_flow_aggregation(self): + secret = 'synthetic-private-market-credential' + self.mocks[5].side_effect = REAL_INSTALL_PLUGIN + self.mocks[4].side_effect = lambda name: name + command = ('codex', 'plugin', 'list', '--available', '--json') + market = ('codex', 'plugin', 'marketplace', 'list', '--json') + for failing in (command, market): + self.mocks[6].side_effect = NativeProcessFixture({ + command: {'installed': [], 'available': []}, + failing: subprocess.CompletedProcess([], 7, secret, secret), + }) + with self.subTest(failing=failing), self.assertRaises(ClientRequestError) as caught: + plugin.maybe_install_azure_plugin(self.cmd, True, ['codex']) + self.assertNotIn(secret, ''.join(traceback.format_exception(caught.exception)) + self.stderr.getvalue()) + self.assertIn('exit 7', str(caught.exception)) + + def test_cancellation_reports_partial_native_state_even_when_logging_is_quiet(self): + self.mocks[5].side_effect = REAL_INSTALL_PLUGIN + self.mocks[4].side_effect = lambda name: name + process = NativeProcessFixture({ + ('claude', 'plugin', 'list', '--json'): [], + ('claude', 'plugin', 'marketplace', 'list', '--json'): [], + ('node', '--version'): 'v22.0.0', + ('claude', 'plugin', 'marketplace', 'add', 'anthropics/claude-plugins-official', '--scope', 'user'): '', + ('claude', 'plugin', 'install', 'azure@claude-plugins-official', '--scope', 'user'): + subprocess.CompletedProcess([], 1, '', 'native install refused'), + ('copilot', 'plugin', 'list', '--json'): KeyboardInterrupt(), + }) + for explicit in (True, False): + self.mocks[6].side_effect = process + self.stderr.truncate(0) + self.stderr.seek(0) + process.calls.clear() + with self.subTest(explicit=explicit), mock.patch.object(plugin.logger, 'disabled', True): + with self.assertRaises(KeyboardInterrupt): + if explicit: + plugin.maybe_install_azure_plugin(self.cmd, True, list(plugin.HOST_IDS)) + else: + self.run_optional(['y', '1 2 3', 'y']) + message = self.stderr.getvalue() + for part in ('Claude Code', 'native install refused', 'GitHub Copilot CLI: interrupted', + 'native state is uncertain', 'kubectl and kubelogin remain installed', + 'native plugin commands', 'No automatic retry or rollback'): + self.assertIn(part, message) + self.assertEqual(process.calls[-1], ('copilot', 'plugin', 'list', '--json')) + self.assertFalse(any(call[0] == 'codex' for call in process.calls)) + + def test_cancellation_reports_prior_success_without_continuing(self): + self.mocks[5].side_effect = [True, KeyboardInterrupt(), True] + with self.assertRaises(KeyboardInterrupt): + plugin.maybe_install_azure_plugin(self.cmd, True, list(plugin.HOST_IDS)) + self.assertEqual(self.mocks[5].call_args_list, [mock.call('claude-code'), mock.call('github-copilot')]) + self.assertIn('Claude Code: installed', self.stderr.getvalue()) + self.assertIn('GitHub Copilot CLI: interrupted', self.stderr.getvalue()) + def test_explicit_sudo_is_rejected_before_any_execution(self): with mock.patch.dict(os.environ, {'SUDO_UID': '123'}), self.assertRaises(InvalidArgumentValueError): plugin.maybe_install_azure_plugin(self.cmd, True, ['codex']) @@ -316,7 +377,7 @@ def setUp(self): def install(self, host, responses): self.process = NativeProcessFixture(responses) - with mock.patch.object(plugin.subprocess, 'run', side_effect=self.process): + with mock.patch.object(plugin.subprocess, 'Popen', side_effect=self.process): return plugin.install_plugin(host) def fresh_responses(self, host, markets): @@ -552,7 +613,7 @@ def test_mcp_inventory_failures_do_not_disclose_credentials(self): inventory = copy.deepcopy(COPILOT_MCP) inventory['mcpServers']['azure']['args'] = ['--token', stdout_secret] payload = json.dumps(inventory) - real_run = subprocess.run + real_popen = subprocess.Popen for case, stdout, stderr, ending, error, context in ( ('invalid JSON', payload[:-1], '', 'sys.exit(0)', ValidationError, 'Invalid native JSON'), ('nonzero', payload, stderr_secret, 'sys.exit(7)', ClientRequestError, 'exit 7'), @@ -562,17 +623,19 @@ def test_mcp_inventory_failures_do_not_disclose_credentials(self): script = (f'import sys, time; print({stdout!r}, flush=True); ' f'print({stderr!r}, file=sys.stderr, flush=True); {ending}') - def run(argv, **kwargs): + def popen(argv, **kwargs): calls.append(tuple(argv)) if argv == ['copilot', 'plugin', 'list', '--json']: - return subprocess.CompletedProcess(argv, 0, '[]', '') + return NativeProcessFixture({tuple(argv): []})(argv) if argv == ['copilot', 'mcp', 'list', '--json']: - kwargs['timeout'] = 0.2 if case == 'timeout' else 5 - return real_run([sys.executable, '-c', script], **kwargs) + process = real_popen([sys.executable, '-c', script], **kwargs) + communicate = process.communicate + process.communicate = lambda timeout: communicate(timeout=0.2 if case == 'timeout' else timeout) + return process raise AssertionError('Unexpected native command: ' + repr(argv)) with self.subTest(case=case): - with mock.patch.object(plugin.subprocess, 'run', side_effect=run), self.assertRaises(error) as caught: + with mock.patch.object(plugin.subprocess, 'Popen', side_effect=popen), self.assertRaises(error) as caught: plugin.install_plugin('github-copilot') message = str(caught.exception) for safe_context in ('GitHub Copilot CLI', 'MCP inventory', context, 'native'): @@ -584,10 +647,58 @@ def run(argv, **kwargs): self.assertEqual(calls, [('copilot', 'plugin', 'list', '--json'), ('copilot', 'mcp', 'list', '--json')]) + def test_plugin_and_marketplace_failures_do_not_disclose_credentials(self): + secret = 'synthetic-private-git-credential' + payload = '{"source":{"url":"https://user:' + secret + '@example.test/repo.git"}}' + for host, executable, args in ( + ('claude-code', 'claude', ['plugin', 'list', '--json']), + ('github-copilot', 'copilot', ['plugin', 'list', '--json']), + ('codex', 'codex', ['plugin', 'list', '--available', '--json'])): + for command in ((executable, *args), (executable, 'plugin', 'marketplace', 'list', '--json')): + for response, error, context in ( + (payload[:-1], ValidationError, 'Invalid native JSON'), + (subprocess.CompletedProcess([], 0, payload, secret), ValidationError, 'warning'), + (subprocess.CompletedProcess([], 7, payload, secret), ClientRequestError, 'exit 7'), + (subprocess.TimeoutExpired(command, 300, output=payload, stderr=secret), + ClientRequestError, 'timed out'), + (OSError(secret), ClientRequestError, 'suppressed')): + responses = self.fresh_responses(host, []) + responses[command] = response + with self.subTest(host=host, command=command, context=context), self.assertRaises(error) as caught: + self.install(host, responses) + self.assertNotIn(secret, ''.join(traceback.format_exception(caught.exception))) + self.assertIn(context, str(caught.exception)) + self.assertEqual(self.mutations(), []) + + def test_duplicate_json_members_fail_before_native_mutations(self): + cases = ( + ('github-copilot', ('copilot', 'mcp', 'list', '--json'), + '{"mcpServers":{"azure":{"source":"user","enabled":true}},"mcpServers":{}}'), + ('codex', ('codex', 'plugin', 'list', '--available', '--json'), + '{"installed":[{"name":"azure"}],"installed":[],"available":[]}'), + ('codex', ('codex', 'plugin', 'marketplace', 'list', '--json'), + '{"marketplaces":[{"name":"azure-skills"}],"marketplaces":[]}'), + ('claude-code', ('claude', 'plugin', 'list', '--json'), + '[{"id":"azure@private","id":"other@private","scope":"user","enabled":false}]'), + ('github-copilot', ('copilot', 'mcp', 'list', '--json'), + '{"mcpServers":{"other":{"source":"user","enabled":true,' + '"private-key-sentinel":"secret-value-sentinel","private-key-sentinel":"last"}}}'), + ) + for host, command, raw in cases: + responses = self.fresh_responses(host, []) + responses[command] = raw + with self.subTest(host=host, command=command): + with self.assertRaisesRegex(ValidationError, 'duplicate') as caught: + self.install(host, responses) + rendered = ''.join(traceback.format_exception(caught.exception)) + self.assertNotIn('private-key-sentinel', rendered) + self.assertNotIn('secret-value-sentinel', rendered) + self.assertEqual(self.mutations(), []) + def test_inventory_warning_never_becomes_absence(self): command = ('copilot', 'plugin', 'list', '--json') response = subprocess.CompletedProcess(command, 0, '[]', 'Configuration could not be loaded') - with self.assertRaisesRegex(ValidationError, 'Configuration could not be loaded'): + with self.assertRaisesRegex(ValidationError, 'Native inventory warning'): self.install('github-copilot', {command: response}) self.assertEqual(self.mutations(), []) @@ -598,7 +709,7 @@ def test_command_failure_never_retries_or_falls_through(self): for command in commands: broken = dict(responses) broken[command] = subprocess.CompletedProcess(command, 2, '', 'unsupported flag or policy refusal') - context = 'exit 2' if command[1] == 'mcp' else 'policy refusal' + context = 'exit 2' if 'list' in command else 'policy refusal' with self.subTest(host=host, command=command), self.assertRaisesRegex(ClientRequestError, context): self.install(host, broken) self.assertEqual(self.process.calls[-1], command) @@ -704,25 +815,154 @@ def test_child_timeout_is_finite_and_keeps_partial_output(self): for part in ('Codex CLI', 'plugin inventory', 'timed out', 'waiting', 'native'): self.assertIn(part, message) + @unittest.skipUnless(sys.platform.startswith('linux'), 'Real process-group regression uses Linux') + def test_timeout_and_cancellation_stop_owned_descendant_before_it_can_write(self): + import psutil + + communicate = subprocess.Popen.communicate + for cancelled in (False, True): + for sensitive in (False, True): + with self.subTest(cancelled=cancelled, sensitive=sensitive), tempfile.TemporaryDirectory() as directory: + root = Path(directory) + ready, gate, marker = (root / name for name in ('ready', 'gate', 'marker')) + child = ( + 'import os, pathlib, time; ' + f'root = pathlib.Path({directory!r}); ' + '(root / "pid").write_text(str(os.getpid())); (root / "pid").rename(root / "ready"); ' + '\nwhile not (root / "gate").exists(): time.sleep(0.01)\n' + 'time.sleep(0.8); (root / "marker").write_text("continued")' + ) + parent = ( + 'import subprocess, sys, time; ' + f'subprocess.Popen([sys.executable, "-c", {child!r}]); ' + 'print("partial-output-sentinel", flush=True); time.sleep(10)' + ) + owned = [] + + def after_ready(process, *args, **kwargs): + if not owned: + owned.append(psutil.Process(process.pid)) + deadline = time.monotonic() + 5 + while not ready.exists() and time.monotonic() < deadline: + time.sleep(0.01) + self.assertTrue(ready.exists(), 'Descendant did not start') + owned.append(psutil.Process(int(ready.read_text()))) + gate.touch() + if cancelled: + raise KeyboardInterrupt() + return communicate(process, *args, **kwargs) + + try: + error = KeyboardInterrupt if cancelled else ClientRequestError + with mock.patch.object(subprocess.Popen, 'communicate', after_ready): + started = time.monotonic() + with self.assertRaises(error) as caught: + plugin._run('codex', [sys.executable, '-c', parent], 'plugin inventory', + timeout=0.2, sensitive=sensitive) + self.assertLess(time.monotonic() - started, 3, 'Cleanup exceeded bounded deadline') + time.sleep(1) + self.assertFalse(marker.exists(), 'Owned descendant continued after timeout/cancellation') + self.assertTrue(all(not proc.is_running() or proc.status() == psutil.STATUS_ZOMBIE + for proc in owned)) + if not cancelled: + message = ''.join(traceback.format_exception(caught.exception)) + self.assertIn('timed out', message) + if sensitive: + self.assertNotIn('partial-output-sentinel', message) + else: + self.assertIn('partial-output-sentinel', message) + finally: + # Clean up even on the unfixed runner, without killing an unrelated group. + for process in reversed(owned): + try: + process.kill() + except psutil.NoSuchProcess: + pass + psutil.wait_procs(owned, timeout=1) + + def test_windows_timeout_and_cancellation_use_bounded_owned_tree_termination(self): + for failure in (subprocess.TimeoutExpired(['codex'], 0.2), KeyboardInterrupt()): + process = mock.Mock(pid=321) + process.communicate.side_effect = [failure, ('windows partial output', '')] + with self.subTest(failure=type(failure).__name__), \ + mock.patch.object(plugin.sys, 'platform', 'win32'), \ + mock.patch.dict(os.environ, {'SystemRoot': 'C:\\Windows'}), \ + mock.patch.object(subprocess, 'CREATE_NEW_PROCESS_GROUP', 512, create=True), \ + mock.patch.object(subprocess, 'Popen', return_value=process) as popen, \ + mock.patch.object(subprocess, 'run', return_value=subprocess.CompletedProcess([], 0)) as taskkill: + error = KeyboardInterrupt if isinstance(failure, KeyboardInterrupt) else ClientRequestError + with self.assertRaises(error) as caught: + plugin._run('codex', ['codex', 'plugin', 'add', 'azure@azure-skills'], 'plugin install', timeout=0.2) + if not isinstance(failure, KeyboardInterrupt): + self.assertIn('windows partial output', str(caught.exception)) + self.assertEqual(popen.call_args.kwargs['creationflags'], 512) + self.assertFalse(popen.call_args.kwargs.get('shell', False)) + self.assertEqual(taskkill.call_args.args[0], [ + os.path.join('C:\\Windows', 'System32', 'taskkill.exe'), '/PID', '321', '/T', '/F']) + self.assertEqual(taskkill.call_args.kwargs['stdin'], subprocess.DEVNULL) + self.assertGreater(taskkill.call_args.kwargs['timeout'], 0) + self.assertLessEqual(taskkill.call_args.kwargs['timeout'], 5) + self.assertTrue(all(call.kwargs['timeout'] <= 5 for call in process.communicate.call_args_list)) + + def test_posix_cancellation_uses_only_the_owned_group_including_on_macos(self): + for platform in ('linux', 'darwin'): + process = mock.Mock(pid=321) + process.communicate.side_effect = [KeyboardInterrupt(), ('', '')] + with self.subTest(platform=platform), mock.patch.object(plugin.sys, 'platform', platform), \ + mock.patch.object(subprocess, 'Popen', return_value=process) as popen, \ + mock.patch.object(os, 'killpg', create=True) as killpg, \ + mock.patch.object(signal, 'SIGKILL', 9, create=True): + with self.assertRaises(KeyboardInterrupt): + plugin._run('codex', ['codex'], 'plugin install') + self.assertTrue(popen.call_args.kwargs['start_new_session']) + killpg.assert_called_once_with(321, signal.SIGKILL) + process.stdout.close.assert_called_once() + process.stderr.close.assert_called_once() + + def test_unconfirmed_windows_cleanup_is_bounded_redacted_and_preserves_failure(self): + for failure in (subprocess.TimeoutExpired(['codex'], 0.2), KeyboardInterrupt()): + process = mock.Mock(pid=321) + process.communicate.side_effect = [failure, subprocess.TimeoutExpired(['codex'], 5)] + process.wait.side_effect = subprocess.TimeoutExpired(['codex'], 5) + output = io.StringIO() + with self.subTest(failure=type(failure).__name__), \ + mock.patch.object(plugin.sys, 'platform', 'win32'), \ + mock.patch.object(subprocess, 'CREATE_NEW_PROCESS_GROUP', 512, create=True), \ + mock.patch.object(subprocess, 'Popen', return_value=process), \ + mock.patch.object(subprocess, 'run', side_effect=OSError('private-cleanup-sentinel')), \ + mock.patch.dict(os.environ, {'SystemRoot': 'C:\\Windows'}), mock.patch('sys.stderr', output): + error = KeyboardInterrupt if isinstance(failure, KeyboardInterrupt) else ClientRequestError + with self.assertRaises(error) as caught: + plugin._run('codex', ['codex'], 'plugin inventory', timeout=0.2, sensitive=True) + self.assertIn('cleanup could not be confirmed', output.getvalue()) + self.assertNotIn('private-cleanup-sentinel', output.getvalue() + + ''.join(traceback.format_exception(caught.exception))) + process.kill.assert_called_once() + process.wait.assert_called_once_with(timeout=5) + process.stdout.close.assert_not_called() + process.stderr.close.assert_not_called() + def test_oserror_is_concrete_cli_error(self): - with mock.patch.object(plugin.subprocess, 'run', side_effect=OSError('executable permission denied')): + with mock.patch.object(plugin.subprocess, 'Popen', side_effect=OSError('executable permission denied')): with self.assertRaisesRegex(ClientRequestError, 'Claude Code.*plugin inventory.*permission denied'): plugin._run('claude-code', ['claude', 'plugin', 'list', '--json'], 'plugin inventory') def test_runner_has_no_shell_and_finite_default_timeout(self): - with mock.patch.object(plugin.subprocess, 'run', wraps=subprocess.run) as run: + communicate = subprocess.Popen.communicate + with mock.patch.object(subprocess.Popen, 'communicate', autospec=True, side_effect=communicate) as exchange, \ + mock.patch.object(plugin.subprocess, 'Popen', wraps=subprocess.Popen) as popen: plugin._run('codex', [sys.executable, '-c', 'print("ok")'], 'test inventory') - kwargs = run.call_args.kwargs + kwargs = popen.call_args.kwargs self.assertFalse(kwargs.get('shell', False)) self.assertEqual(kwargs['stdin'], subprocess.DEVNULL) - self.assertGreater(kwargs['timeout'], 0) - self.assertLessEqual(kwargs['timeout'], 300) + self.assertGreater(exchange.call_args.kwargs['timeout'], 0) + self.assertLessEqual(exchange.call_args.kwargs['timeout'], 300) class AzurePluginDiscoveryTest(unittest.TestCase): def test_discovery_only_checks_executable_presence(self): with mock.patch.object(plugin.shutil, 'which', side_effect=lambda name: '/bin/' + name if name == 'codex' else None), \ - mock.patch('subprocess.run', side_effect=AssertionError('Discovery must not execute a host')): + mock.patch('subprocess.Popen', side_effect=AssertionError('Discovery must not execute a host')): self.assertEqual(plugin.discover_hosts(), ['codex']) def test_discovery_uses_stable_host_order(self): diff --git a/src/azure-cli/azure/cli/command_modules/acs/tests/latest/test_custom.py b/src/azure-cli/azure/cli/command_modules/acs/tests/latest/test_custom.py index 98d4a1a6032..04ecb4f41eb 100644 --- a/src/azure-cli/azure/cli/command_modules/acs/tests/latest/test_custom.py +++ b/src/azure-cli/azure/cli/command_modules/acs/tests/latest/test_custom.py @@ -119,7 +119,7 @@ def test_explicit_hosts_normalize_without_changing_binary_arguments_or_forwardin mock.call.get_arch_for_cli_binary(), mock.call.k8s_install_kubectl(self.cmd, '1.2.3', '/kubectl', 'https://kubectl.example', arch='arm64'), mock.call.k8s_install_kubelogin(self.cmd, '4.5.6', '/kubelogin', 'https://kubelogin.example', - arch='arm64', gh_token='binary-only-token'), + arch='arm64', gh_token='binary-only-token'), mock.call.maybe_install_azure_plugin(self.cmd, True, ['claude-code', 'codex']), ]) From fa0a0b3689d3083f6fc85cb835965ae01ea25a04 Mon Sep 17 00:00:00 2001 From: gambtho Date: Tue, 22 Sep 2026 14:56:55 -0400 Subject: [PATCH 6/7] [AKS] Fix Windows cleanup in MCP inventory test fixture Intercept only owned-fixture taskkill commands separately from host commands, and register unconditional child cleanup. Cover the original confidentiality test with the Windows branch selected so taskkill cannot hit the Copilot-only Popen guard. --- .../acs/tests/latest/test_azure_plugin.py | 23 ++++++++++++++++++- 1 file changed, 22 insertions(+), 1 deletion(-) diff --git a/src/azure-cli/azure/cli/command_modules/acs/tests/latest/test_azure_plugin.py b/src/azure-cli/azure/cli/command_modules/acs/tests/latest/test_azure_plugin.py index 8c3c1b01e27..03e2966eb58 100644 --- a/src/azure-cli/azure/cli/command_modules/acs/tests/latest/test_azure_plugin.py +++ b/src/azure-cli/azure/cli/command_modules/acs/tests/latest/test_azure_plugin.py @@ -620,6 +620,7 @@ def test_mcp_inventory_failures_do_not_disclose_credentials(self): ('timeout', payload, stderr_secret, 'time.sleep(30)', ClientRequestError, 'timed out after'), ('warning', payload, stderr_secret, 'sys.exit(0)', ValidationError, 'warning')): calls = [] + owned = [] script = (f'import sys, time; print({stdout!r}, flush=True); ' f'print({stderr!r}, file=sys.stderr, flush=True); {ending}') @@ -629,13 +630,26 @@ def popen(argv, **kwargs): return NativeProcessFixture({tuple(argv): []})(argv) if argv == ['copilot', 'mcp', 'list', '--json']: process = real_popen([sys.executable, '-c', script], **kwargs) + owned.append(process) + # Register bound methods before wrapping communicate, even if an assertion fails. + self.addCleanup(process.communicate, timeout=5) + self.addCleanup(process.kill) communicate = process.communicate process.communicate = lambda timeout: communicate(timeout=0.2 if case == 'timeout' else timeout) return process raise AssertionError('Unexpected native command: ' + repr(argv)) + def taskkill(argv, **kwargs): + self.assertEqual(sys.platform, 'win32') + self.assertEqual(len(owned), 1) + self.assertEqual(argv, [os.path.join(os.environ['SystemRoot'], 'System32', 'taskkill.exe'), + '/PID', str(owned[0].pid), '/T', '/F']) + return subprocess.CompletedProcess(argv, 0) + with self.subTest(case=case): - with mock.patch.object(plugin.subprocess, 'Popen', side_effect=popen), self.assertRaises(error) as caught: + with mock.patch.object(plugin.subprocess, 'Popen', side_effect=popen), \ + mock.patch.object(plugin.subprocess, 'run', side_effect=taskkill), \ + self.assertRaises(error) as caught: plugin.install_plugin('github-copilot') message = str(caught.exception) for safe_context in ('GitHub Copilot CLI', 'MCP inventory', context, 'native'): @@ -647,6 +661,13 @@ def popen(argv, **kwargs): self.assertEqual(calls, [('copilot', 'plugin', 'list', '--json'), ('copilot', 'mcp', 'list', '--json')]) + def test_mcp_inventory_failures_with_windows_cleanup_do_not_disclose_credentials(self): + # Exercise the same real-child fixture without requiring a Windows host. + with mock.patch.object(plugin.sys, 'platform', 'win32'), \ + mock.patch.object(subprocess, 'CREATE_NEW_PROCESS_GROUP', 0, create=True), \ + mock.patch.dict(os.environ, {'SystemRoot': 'C:\\Windows'}): + self.test_mcp_inventory_failures_do_not_disclose_credentials() + def test_plugin_and_marketplace_failures_do_not_disclose_credentials(self): secret = 'synthetic-private-git-credential' payload = '{"source":{"url":"https://user:' + secret + '@example.test/repo.git"}}' From 93e2513a300a85af6b767d717da6fb1a531424d4 Mon Sep 17 00:00:00 2001 From: gambtho Date: Thu, 24 Sep 2026 08:23:17 -0400 Subject: [PATCH 7/7] [AKS] Make optional Azure plugin setup nonblocking --- .../cli/command_modules/acs/_azure_plugin.py | 79 +++---- .../azure/cli/command_modules/acs/_help.py | 51 ++--- .../azure/cli/command_modules/acs/_params.py | 13 +- .../tests/latest/test_aks_install_plugin.py | 56 ++++- .../acs/tests/latest/test_azure_plugin.py | 205 ++++++++---------- 5 files changed, 193 insertions(+), 211 deletions(-) diff --git a/src/azure-cli/azure/cli/command_modules/acs/_azure_plugin.py b/src/azure-cli/azure/cli/command_modules/acs/_azure_plugin.py index b444d57555e..144b062348f 100644 --- a/src/azure-cli/azure/cli/command_modules/acs/_azure_plugin.py +++ b/src/azure-cli/azure/cli/command_modules/acs/_azure_plugin.py @@ -14,7 +14,6 @@ import sys from knack.log import get_logger -from knack.prompting import NoTTYException, prompt, prompt_y_n from azure.cli.core.azclierror import ( ClientRequestError, InvalidArgumentValueError, ResourceNotFoundError, ValidationError, @@ -25,6 +24,15 @@ HOST_IDS = ('claude-code', 'github-copilot', 'codex') HOST_LABELS = {'claude-code': 'Claude Code', 'github-copilot': 'GitHub Copilot CLI', 'codex': 'Codex CLI'} _EXECUTABLES = {'claude-code': 'claude', 'github-copilot': 'copilot', 'codex': 'codex'} +_LIFECYCLE_GUIDANCE = { + 'claude-code': 'Update: claude plugin update azure@claude-plugins-official --scope user\n' + 'Remove: claude plugin uninstall azure@claude-plugins-official --scope user', + 'github-copilot': 'Update: copilot plugin update azure@azure-skills\n' + 'Remove: copilot plugin uninstall azure@azure-skills', + 'codex': 'Update (marketplace-wide; affects other installed plugins from azure-skills): ' + 'codex plugin marketplace upgrade azure-skills\n' + 'Remove: codex plugin remove azure@azure-skills', +} def validate_plugin_options(install_azure_plugin=None, plugin_hosts=None): @@ -47,7 +55,7 @@ def _under_sudo(): def maybe_install_azure_plugin(cmd, install_azure_plugin=None, plugin_hosts=None): - """Offer optional setup after both binaries succeed; explicit flags are consent.""" + """Hint after both binaries succeed; only explicit flags authorize setup.""" hosts = validate_plugin_options(install_azure_plugin, plugin_hosts) if install_azure_plugin is False: return @@ -55,24 +63,13 @@ def maybe_install_azure_plugin(cmd, install_azure_plugin=None, plugin_hosts=None if (_under_sudo() or not sys.stdin.isatty() or cmd.cli_ctx.config.getboolean('core', 'disable_confirm_prompt', fallback=False)): return - try: - if not prompt_y_n('Set up the full Azure plugin for an AI CLI host?', default='n'): - return - hosts = _select_hosts() - if not hosts: - return - _disclose_setup() - labels = ', '.join(HOST_LABELS[host] for host in hosts) - if not prompt_y_n(f'Authorize native user/global installation and enablement for {labels}?', default='n'): - return - except (NoTTYException, EOFError, KeyboardInterrupt): - return - else: - _disclose_setup() - _install_selected_hosts(hosts, explicit=install_azure_plugin is True) + logger.warning('For optional Azure plugin setup, use --install-azure-plugin true --plugin-hosts .') + return + _disclose_setup() + _install_selected_hosts(hosts) -def _install_selected_hosts(hosts, *, explicit): +def _install_selected_hosts(hosts): failures = [] outcomes = [] for host in hosts: @@ -92,11 +89,10 @@ def _install_selected_hosts(hosts, *, explicit): outcome = f'{HOST_LABELS[host]}: {status}.' outcomes.append(outcome) print(outcome, file=sys.stderr) + if installed: + print(_LIFECYCLE_GUIDANCE[host], file=sys.stderr) if failures: - message = _setup_summary(outcomes) - if explicit: - raise type(failures[0])(message) from None - logger.warning(message) + raise type(failures[0])(_setup_summary(outcomes)) from None def _setup_summary(outcomes): @@ -107,45 +103,20 @@ def _setup_summary(outcomes): 'No automatic retry or rollback was attempted.') -def _select_hosts(): - detected = discover_hosts() - choices = '\n'.join(f' {index}. {HOST_LABELS[host]}' + (' [detected]' if host in detected else '') - for index, host in enumerate(HOST_IDS, 1)) - defaults = ' '.join(str(index) for index, host in enumerate(HOST_IDS, 1) if host in detected) or '0' - while True: - answer = prompt(f'{choices}\nSelect host numbers separated by spaces (replaces defaults); ' - f'0 selects none. Enter keeps [{defaults}]: ').strip() - if not answer: - return detected - if answer == '0': - return [] - numbers = answer.split() - if all(number in ('1', '2', '3') for number in numbers): - return [host for index, host in enumerate(HOST_IDS, 1) if str(index) in numbers] - print('Choose 1, 2 and/or 3 separated by spaces, or 0 for none.', file=sys.stderr) - - def _disclose_setup(): # Consent context must remain visible even with --only-show-errors. print( - 'Azure plugin setup installs the full Azure plugin (skills, MCP configuration and hooks) in native ' - 'user/global scope, not repository scope. Native inventory-reported Azure installations, including ' - 'disabled ones, are skipped. When inventory reports absence, consent authorizes normal native installation ' - 'and enablement, including changes to hidden/stale disable preferences or registrations.\n' - 'New installations require an installed host CLI and Node.js 22+ with npx on PATH. ' - 'The stock MCP runtime uses @azure/mcp@latest and is not pinned by the plugin version. ' - 'Hosts own permissions, marketplace sources/pins and updates; Azure CLI adds no custom updater or bypass. ' - 'Azure authentication, MCP activation, hook trust and sovereign-cloud setup may still be required. ' - 'No prerequisites are installed and no Azure login or resource operations are performed.', + 'Installing the full Azure plugin (skills, MCP configuration and hooks) in native user/global scope, ' + 'not repository scope. New installs require an installed host CLI and Node.js 22+ with npx on PATH. ' + 'Azure authentication, MCP activation, hook trust and sovereign-cloud setup may still be required.\n' + 'Reported existing Azure plugins, including disabled ones, stay unchanged. Inventory absence authorizes ' + 'native installation and enablement, including changes to hidden/stale preferences or registrations.\n' + '@azure/mcp@latest is not pinned by the plugin version. Host policy, marketplace sources/pins and updates ' + 'remain authoritative. Azure CLI installs no prerequisites and performs no Azure login or resource operations.', file=sys.stderr, ) -def discover_hosts() -> list[str]: - """Discover executables without running hosts or inspecting their configuration.""" - return [host for host in HOST_IDS if shutil.which(_EXECUTABLES[host])] - - def install_plugin(host_id: str) -> bool: """Install after consent; False means Azure was found and no plugin install ran. diff --git a/src/azure-cli/azure/cli/command_modules/acs/_help.py b/src/azure-cli/azure/cli/command_modules/acs/_help.py index cc512e1528f..c778342f10f 100644 --- a/src/azure-cli/azure/cli/command_modules/acs/_help.py +++ b/src/azure-cli/azure/cli/command_modules/acs/_help.py @@ -1481,38 +1481,29 @@ type: command short-summary: Download and install kubectl and kubelogin. Optionally set up the full Azure plugin for supported AI CLI hosts. long-summary: | - Installs kubectl first, then kubelogin. Azure plugin setup runs only after both succeed. - With --install-azure-plugin omitted, noninteractive sessions, core.disable_confirm_prompt=true and sudo - perform no plugin setup or host/runtime probes. Other sessions offer default-No setup, a numbered host - selection with PATH-detected defaults, and final default-No consent. Detection is not consent. - Use --install-azure-plugin false for binaries only. Explicit true requires --plugin-hosts and authorizes - full-plugin setup without prompts, including in automation. Explicit plugin setup under sudo is rejected; - run as the intended host user with writable --install-location and --kubelogin-install-location paths. - - Only Claude Code, GitHub Copilot CLI and Codex CLI are supported, not Pi or VS Code. This installs the full - base Azure plugin (upstream skills, MCP configuration and hooks), not individual skills or the separate - deeper AKS plugin. Installation uses native user/global scope, not repository configuration. Hosts and - Node.js 22+ with node and npx on PATH must already be available for new installations; Azure CLI does not - install prerequisites. The stock MCP runtime uses @azure/mcp@latest, which is not pinned by the plugin - version and may change its runtime requirements. Azure authentication, MCP activation, hook trust and - sovereign-cloud setup may still be needed after installation. No Azure login or resource operations run. - - Azure plugins reported by native inventory, including disabled installations and applicable non-user - scopes, are skipped without reinstalling or enabling them. Inventory can omit hidden/stale registrations - or disable preferences. When valid inventory reports absence, consent authorizes the host's normal native - install-and-enable behavior, including changes to that hidden state. Invalid or unsupported inventory - fails safely; Azure CLI does not reconstruct native configuration. A marketplace may have been added - before an existing plugin becomes visible and plugin installation is skipped. - - Hosts own permissions, trust, marketplace sources/pins and future updates. An existing marketplace - registration is used as configured, without repointing it or bypassing host policy; the recommended - marketplace is added only if absent. There is no custom updater, forced update or per-task plugin reinstall. + The optional full Azure plugin adds skills, MCP configuration and hooks for Claude Code, GitHub Copilot CLI + or Codex CLI in native user/global scope, not repository scope. New installations require an installed host + and Node.js 22+ with node and npx on PATH. Azure authentication, MCP activation, hook trust and sovereign-cloud + setup may still be needed. Azure CLI installs no prerequisites and performs no Azure login or resource operations. + + Installs kubectl first, then kubelogin. Plugin setup runs only after both succeed and requires + --install-azure-plugin true with --plugin-hosts; it never prompts. Omission performs no setup or host/runtime + probes and may show one opt-in hint on TTY stdin. False, non-TTY stdin, sudo, core.disable_confirm_prompt=true + and --only-show-errors suppress that hint. Explicit setup under sudo is rejected; run as the intended host + user with writable --install-location and --kubelogin-install-location paths. + + Reported existing Azure plugins, including disabled ones, stay unchanged. Inventory absence authorizes native + installation and enablement, including changes to hidden/stale disable preferences or registrations. A marketplace + may be added before an existing plugin becomes visible and installation is skipped. Host policy, marketplace + sources/pins and updates remain authoritative; existing sources are not repointed. The MCP runtime uses + @azure/mcp@latest, which is not pinned by the plugin version and may change its requirements. + + Native update/remove commands are shown after new installations; hosts own the plugin lifecycle, with no Azure CLI + updater. Selected hosts are attempted independently. Failures return nonzero but leave kubectl, kubelogin and any + successful plugin installations in place. No automatic retry or rollback occurs; recover with native plugin commands. --gh-token is used only for kubelogin downloads, not passed to plugin hosts. - Selected hosts are attempted independently, so partial success is possible. Optional interactive failures - warn without failing the binary installation. Explicit setup failures return nonzero; kubectl and kubelogin - remain installed. No automatic retry or rollback occurs. Recover with the host's native plugin commands. examples: - - name: Install binaries only, without any Azure plugin offer or setup + - name: Install binaries only, without any Azure plugin hint or setup text: az aks install-cli --install-azure-plugin false - name: Install binaries and explicitly authorize full Azure plugin setup for Codex CLI text: az aks install-cli --install-azure-plugin true --plugin-hosts codex diff --git a/src/azure-cli/azure/cli/command_modules/acs/_params.py b/src/azure-cli/azure/cli/command_modules/acs/_params.py index 2423e2c35e0..008e63d1976 100644 --- a/src/azure-cli/azure/cli/command_modules/acs/_params.py +++ b/src/azure-cli/azure/cli/command_modules/acs/_params.py @@ -1028,14 +1028,13 @@ def load_arguments(self, _): with self.argument_context('aks install-cli', arg_group='Azure Plugin') as c: c.argument('install_azure_plugin', arg_type=get_three_state_flag(), - help='Consent to full Azure plugin setup in native user/global scope after both binaries install. ' - 'Requires --plugin-hosts when true; false skips setup. If omitted, only interactive sessions ' - 'offer default-No setup, unless confirmation prompts are disabled or running under sudo. ' - 'Valid native inventory absence authorizes normal install-and-enable, including hidden/stale ' - 'disable preferences. See command help for runtime, MCP, hooks and authentication requirements.') + help='Install Azure plugin skills, MCP configuration and hooks in native user/global scope. ' + 'True requires --plugin-hosts and authorizes setup without prompts after both binaries install. ' + 'Omission only allows an opt-in hint; false suppresses it. See command help for prerequisites ' + 'and native enablement behavior.') c.argument('plugin_hosts', arg_type=get_enum_type(HOST_IDS), nargs='+', - help='Host CLIs for full Azure plugin setup. Requires --install-azure-plugin true. ' - 'Host CLIs must already be installed. Supported hosts only; not Pi or VS Code.') + help='Installed host CLIs to configure. Requires --install-azure-plugin true; ' + 'new plugins also require Node.js 22+ with npx on PATH.') with self.argument_context('aks install-desktop') as c: c.argument('version', help='Version of AKS Desktop to install. By default, the latest stable version is installed.') diff --git a/src/azure-cli/azure/cli/command_modules/acs/tests/latest/test_aks_install_plugin.py b/src/azure-cli/azure/cli/command_modules/acs/tests/latest/test_aks_install_plugin.py index 4bf5cb584cd..61217ca9596 100644 --- a/src/azure-cli/azure/cli/command_modules/acs/tests/latest/test_aks_install_plugin.py +++ b/src/azure-cli/azure/cli/command_modules/acs/tests/latest/test_aks_install_plugin.py @@ -85,7 +85,7 @@ def _run_cli_scenario(mode): root = Path.cwd() blocked = [] writable = [root / 'azure_config_dir', root / 'installed', root / 'tmpdir'] - executables = [str(root / 'bin' / name) for name in ('codex', 'node')] + executables = [] if mode.startswith('optional') else [str(root / 'bin' / name) for name in ('codex', 'node')] def forbidden(event, args): blocked.append(event) @@ -196,8 +196,23 @@ def transport(request, **kwargs): 'aks', 'install-cli', '--client-version', '1.2.3', '--kubelogin-version', '4.5.6', '--install-location', str(root / 'installed/kubectl'), '--kubelogin-install-location', str(root / 'installed/kubelogin'), - '--install-azure-plugin', '--plugin-hosts', 'codex', ] + if mode.startswith('optional'): + assert sys.stdin.isatty(), 'Regression requires inherited TTY stdin' + if mode == 'optional-quiet': + arguments.append('--only-show-errors') + assert cli.invoke(arguments) == 0 + assert not (root / 'native.jsonl').exists(), 'Optional stage executed a host/runtime' + assert downloads == list(payloads), downloads + for name, payload in (('kubectl', b'fixture kubectl\n'), ('kubelogin', b'fixture kubelogin\n')): + assert (root / 'installed' / name).read_bytes() == payload + for directory in _ENV_DIRS: + if directory != 'AZURE_CONFIG_DIR': + assert list((root / directory.lower()).iterdir()) == [], directory + assert not blocked, blocked + print('Omitted plugin setup returned without input or native I/O') + return + arguments.extend(['--install-azure-plugin', '--plugin-hosts', 'codex']) assert cli.invoke(arguments) == 0 state_path = root / 'codex_home/state.json' state = json.loads(state_path.read_text()) @@ -244,7 +259,10 @@ def transport(request, **kwargs): @unittest.skipUnless(sys.platform.startswith('linux'), 'Owned executable and fd guards use Linux') class AKSInstallPluginScenarioTest(unittest.TestCase): def invoke_isolated(self, mode='scenario'): - with tempfile.TemporaryDirectory() as sandbox: + from contextlib import ExitStack + import pty + + with tempfile.TemporaryDirectory() as sandbox, ExitStack() as stack: root = Path(sandbox) # Allowlist, rather than inheriting tokens, sudo, ARM metadata, host # homes, extension dev sources or the user's executable search path. @@ -266,9 +284,22 @@ def invoke_isolated(self, mode='scenario'): executable = root / 'bin' / name executable.write_text('#!' + sys.executable + '\n' + _HOST_SCRIPT) executable.chmod(0o755) - result = subprocess.run([sys.executable, '-B', str(Path(__file__).resolve()), mode], - cwd=root, env=environment, stdin=subprocess.DEVNULL, - capture_output=True, text=True, timeout=90, check=False) + command = [sys.executable, '-B', str(Path(__file__).resolve()), mode] + stdin = subprocess.DEVNULL + if mode.startswith('optional'): + master, stdin = pty.openpty() + stack.callback(os.close, master) + stack.callback(os.close, stdin) + # A noninteractive shell can still pass a TTY to az. Keep its + # master open without ever sending input; do not mock prompting. + command = ['/bin/bash', '--noprofile', '--norc', '-c', + 'case "$-" in *i*) exit 99;; esac; exec "$@"', 'fixture', *command] + try: + result = subprocess.run(command, cwd=root, env=environment, stdin=stdin, + capture_output=True, text=True, + timeout=30 if mode.startswith('optional') else 90, check=False) + except subprocess.TimeoutExpired as ex: + self.fail('CLI did not exit without input: ' + repr((ex.stdout, ex.stderr))) self.assertFalse(root.exists(), 'Fixture cleanup left host/config files behind') return result @@ -283,6 +314,19 @@ def test_install_plugin_real_cli_preserves_disabled_existing_plugin(self): self.assertEqual(result.returncode, 0, result.stdout + result.stderr) self.assertIn('Fresh install and disabled-existing rerun verified; no forbidden I/O', result.stdout) + def test_install_plugin_omitted_in_noninteractive_shell_with_inherited_tty_never_waits(self): + for mode in ('optional', 'optional-quiet'): + with self.subTest(mode=mode): + result = self.invoke_isolated(mode) + self.assertEqual(result.returncode, 0, result.stdout + result.stderr) + self.assertEqual(result.stdout, 'Omitted plugin setup returned without input or native I/O\n') + if mode == 'optional': + self.assertEqual(result.stderr.count('--install-azure-plugin'), 1) + self.assertEqual(result.stderr.count('--plugin-hosts'), 1) + else: + self.assertEqual(result.stderr, '') + self.assertNotIn('(y/N)', result.stderr) + def test_install_plugin_fixture_guards_and_constructor_failure_cleanup(self): result = self.invoke_isolated('guards') self.assertEqual(result.returncode, 23, result.stdout + result.stderr) diff --git a/src/azure-cli/azure/cli/command_modules/acs/tests/latest/test_azure_plugin.py b/src/azure-cli/azure/cli/command_modules/acs/tests/latest/test_azure_plugin.py index 03e2966eb58..0cbe29db949 100644 --- a/src/azure-cli/azure/cli/command_modules/acs/tests/latest/test_azure_plugin.py +++ b/src/azure-cli/azure/cli/command_modules/acs/tests/latest/test_azure_plugin.py @@ -17,8 +17,6 @@ import unittest from unittest import mock -from knack.prompting import NoTTYException - from azure.cli.command_modules.acs import _azure_plugin as plugin from azure.cli.core.azclierror import ( ClientRequestError, InvalidArgumentValueError, ResourceNotFoundError, ValidationError, @@ -125,42 +123,34 @@ class AzurePluginConsentTest(unittest.TestCase): def setUp(self): self.cmd = mock.Mock() self.cmd.cli_ctx.config.getboolean.return_value = False - self.prompts = [] - self.answers = iter([]) self.stderr = io.StringIO() + self.stdout = io.StringIO() self.installed = [] patches = ( mock.patch.dict(os.environ, {}, clear=True), mock.patch('sys.stdin.isatty', return_value=True), mock.patch('sys.stderr', self.stderr), - mock.patch('knack.prompting._input', side_effect=self.answer), + mock.patch('knack.prompting._input', side_effect=AssertionError('Unexpected prompt')), mock.patch.object(plugin.shutil, 'which', side_effect=lambda name: name if name == 'codex' else None), mock.patch.object(plugin, 'install_plugin', side_effect=self.install), mock.patch.object(plugin.subprocess, 'Popen', side_effect=AssertionError('Unexpected native process')), + mock.patch('sys.stdout', self.stdout), + mock.patch('sys.stdin.read', side_effect=AssertionError('Unexpected stdin read')), + mock.patch('sys.stdin.readline', side_effect=AssertionError('Unexpected stdin read')), + mock.patch('builtins.input', side_effect=AssertionError('Unexpected input')), ) self.mocks = [patcher.start() for patcher in patches] for patcher in patches: self.addCleanup(patcher.stop) - def answer(self, message): - self.prompts.append(message) - result = next(self.answers) - if isinstance(result, BaseException): - raise result - return result - def install(self, host): self.installed.append(host) self.assertIn('MCP', self.stderr.getvalue()) return True - def run_optional(self, answers): - self.answers = iter(answers) - plugin.maybe_install_azure_plugin(self.cmd) - - def test_false_no_tty_disabled_confirmation_and_sudo_do_no_discovery_or_execution(self): + def test_false_no_tty_disabled_confirmation_and_sudo_do_not_hint_or_probe(self): for condition in ('false', 'no-tty', 'disabled', 'sudo-user', 'sudo-uid'): - with self.subTest(condition=condition), mock.patch.object(plugin, 'discover_hosts') as discover: + with self.subTest(condition=condition), self.assertNoLogs('cli.' + plugin.__name__, level='WARNING'): self.mocks[1].return_value = condition != 'no-tty' self.cmd.cli_ctx.config.getboolean.return_value = condition == 'disabled' environment = {'SUDO_USER': 'user'} if condition == 'sudo-user' else {} @@ -168,27 +158,23 @@ def test_false_no_tty_disabled_confirmation_and_sudo_do_no_discovery_or_executio environment = {'SUDO_UID': '0'} with mock.patch.dict(os.environ, environment, clear=True): plugin.maybe_install_azure_plugin(self.cmd, False if condition == 'false' else None) - discover.assert_not_called() self.assertEqual(self.installed, []) - self.assertEqual(self.prompts, []) + self.assertEqual(self.stderr.getvalue(), '') + self.assertEqual(self.stdout.getvalue(), '') self.mocks[4].assert_not_called() self.mocks[6].assert_not_called() - def test_initial_offer_defaults_to_no_without_even_discovering(self): - self.run_optional(['']) + def test_omitted_tty_only_hints_once_without_reading_stdin_or_probing(self): + with self.assertLogs('cli.' + plugin.__name__, level='WARNING') as logs: + plugin.maybe_install_azure_plugin(self.cmd) + self.assertEqual(len(logs.records), 1) + self.assertIn('--install-azure-plugin', logs.output[0]) + self.assertIn('--plugin-hosts', logs.output[0]) self.assertEqual(self.installed, []) - self.assertEqual(len(self.prompts), 1) - self.assertIn('(y/N)', self.prompts[0]) + self.assertEqual(self.stderr.getvalue(), '') + self.assertEqual(self.stdout.getvalue(), '') self.mocks[4].assert_not_called() - - def test_detected_defaults_require_final_default_no_consent(self): - self.run_optional(['y', '', '']) - self.assertEqual(self.installed, []) - self.assertEqual(len(self.prompts), 3) - self.assertIn('(y/N)', self.prompts[-1]) - self.assertIn('Codex CLI', self.prompts[-1]) - self.assertIn('3', self.prompts[1]) - self.assert_disclosure() + self.mocks[6].assert_not_called() def assert_disclosure(self): text = self.stderr.getvalue() @@ -197,51 +183,58 @@ def assert_disclosure(self): 'disabled', 'authentication', 'trust', 'sovereign', 'updates'): self.assertIn(disclosure, text) - def test_detected_host_installs_only_after_final_consent(self): - def install(host): - self.assertEqual(len(self.prompts), 3) - self.assert_disclosure() - return self.install(host) - self.mocks[5].side_effect = install - self.run_optional(['y', '', 'y']) - self.assertEqual(self.installed, ['codex']) - - def test_numbered_selection_replaces_defaults_and_deduplicates(self): - self.run_optional(['y', '2 1 2', 'y']) - self.assertEqual(self.installed, ['claude-code', 'github-copilot']) - self.assertNotIn('Codex CLI', self.prompts[-1]) - - def test_all_hosts_can_be_deselected_without_final_prompt(self): - self.run_optional(['y', '0']) - self.assertEqual(self.installed, []) - self.assertEqual(len(self.prompts), 2) - - def test_no_detected_hosts_blank_selection_does_not_consent(self): - self.mocks[4].side_effect = lambda name: None - self.run_optional(['y', '']) - self.assertEqual(self.installed, []) - self.assertEqual(len(self.prompts), 2) - - def test_invalid_selection_reprompts_without_installing(self): - self.run_optional(['y', '4', '0 1', '-1', 'codex', '1,2', '2', 'y']) - self.assertEqual(self.installed, ['github-copilot']) - self.assertEqual(len(self.prompts), 8) + def test_new_install_prints_only_selected_host_lifecycle_commands(self): + commands = { + 'claude-code': ('claude plugin update azure@claude-plugins-official --scope user', + 'claude plugin uninstall azure@claude-plugins-official --scope user'), + 'github-copilot': ('copilot plugin update azure@azure-skills', + 'copilot plugin uninstall azure@azure-skills'), + 'codex': ('codex plugin marketplace upgrade azure-skills', 'codex plugin remove azure@azure-skills'), + } + for host, expected in commands.items(): + with self.subTest(host=host): + self.stderr.truncate(0) + self.stderr.seek(0) + plugin.maybe_install_azure_plugin(self.cmd, True, [host]) + message = self.stderr.getvalue() + for command in expected: + self.assertIn(command, [line.rsplit(': ', 1)[-1] for line in message.splitlines()]) + self.assertEqual(message.count(command), 1) + self.assertLess(message.index('installed;'), message.index(command)) + for other, excluded in commands.items(): + if other != host: + for command in excluded: + self.assertNotIn(command, message) + if host == 'codex': + self.assertIn('marketplace-wide', message) + self.assertIn('other installed plugins', message) + self.assertNotIn('codex plugin update', message) + self.assertNotIn('codex plugin add', message) + self.assertEqual(self.stdout.getvalue(), '') + self.mocks[6].assert_not_called() - def test_cancellation_eof_or_lost_tty_at_each_prompt_is_optional_noop(self): - for error in (KeyboardInterrupt, EOFError, NoTTYException): - for answers in ([error()], ['y', error()], ['y', '', error()]): - with self.subTest(error=error, stage=len(answers)): - self.run_optional(answers) - self.assertEqual(self.installed, []) + def test_existing_private_source_gets_no_hardcoded_lifecycle_target(self): + self.mocks[5].side_effect = REAL_INSTALL_PLUGIN + self.mocks[4].side_effect = lambda name: name + process = NativeProcessFixture({ + ('claude', 'plugin', 'list', '--json'): [CLAUDE_PLUGIN], + ('copilot', 'plugin', 'list', '--json'): [COPILOT_PLUGIN], + ('codex', 'plugin', 'list', '--available', '--json'): {'installed': [CODEX_PLUGIN], 'available': []}, + }) + self.mocks[6].side_effect = process + plugin.maybe_install_azure_plugin(self.cmd, True, list(plugin.HOST_IDS)) + message = self.stderr.getvalue() + self.assertEqual(message.count('Azure already reported; plugin install skipped'), 3) + self.assertNotIn('azure@', message) + self.assertNotIn('marketplace upgrade', message) + self.assertEqual(len(process.calls), 3) def test_explicit_hosts_need_no_tty_prompts_or_detection_even_with_confirm_disabled(self): self.mocks[1].return_value = False self.cmd.cli_ctx.config.getboolean.return_value = True - with mock.patch.object(plugin, 'discover_hosts') as discover: - plugin.maybe_install_azure_plugin(self.cmd, True, ['codex', 'claude-code', 'codex']) + plugin.maybe_install_azure_plugin(self.cmd, True, ['codex', 'claude-code', 'codex']) self.assertEqual(self.installed, ['claude-code', 'codex']) - self.assertEqual(self.prompts, []) - discover.assert_not_called() + self.mocks[4].assert_not_called() self.assert_disclosure() def test_expected_failures_continue_hosts_and_preserve_explicit_error_category(self): @@ -265,15 +258,11 @@ def install(host): 'GitHub Copilot CLI', 'installed'): self.assertIn(part, message) self.assertEqual(calls, ['claude-code', 'github-copilot', 'codex']) - - def test_optional_failure_warns_without_failing_and_remaining_host_still_installs(self): - self.mocks[5].side_effect = [ValidationError('bad inventory'), True] - with self.assertLogs('cli.' + plugin.__name__, level='WARNING') as logs: - self.run_optional(['y', '1 3', 'y']) - message = '\n'.join(logs.output) - for part in ('Claude Code', 'bad inventory', 'Codex CLI', 'installed', 'remain installed', 'native'): - self.assertIn(part, message) - self.assertEqual(self.mocks[5].call_args_list, [mock.call('claude-code'), mock.call('codex')]) + output = self.stderr.getvalue() + self.assertIn('copilot plugin update azure@azure-skills', output) + self.assertIn('copilot plugin uninstall azure@azure-skills', output) + self.assertNotIn('claude plugin', output) + self.assertNotIn('codex plugin', output) def test_report_distinguishes_install_from_skip_without_claiming_zero_marketplace_changes(self): self.mocks[5].side_effect = [True, False] @@ -283,11 +272,12 @@ def test_report_distinguishes_install_from_skip_without_claiming_zero_marketplac self.assertIn('Codex CLI: Azure already reported; plugin install skipped', message) self.assertIn('marketplace may have been added', message) self.assertNotIn('all integrations are ready', message) + self.assertIn('claude plugin update azure@claude-plugins-official --scope user', message) + self.assertIn('claude plugin uninstall azure@claude-plugins-official --scope user', message) + self.assertNotIn('codex plugin', message) - def test_programming_errors_are_not_swallowed_on_optional_or_explicit_paths(self): + def test_programming_errors_are_not_swallowed(self): self.mocks[5].side_effect = TypeError('programming error') - with self.assertRaisesRegex(TypeError, 'programming error'): - self.run_optional(['y', '1', 'y']) with self.assertRaisesRegex(TypeError, 'programming error'): plugin.maybe_install_azure_plugin(self.cmd, True, ['codex']) @@ -335,24 +325,18 @@ def test_cancellation_reports_partial_native_state_even_when_logging_is_quiet(se subprocess.CompletedProcess([], 1, '', 'native install refused'), ('copilot', 'plugin', 'list', '--json'): KeyboardInterrupt(), }) - for explicit in (True, False): - self.mocks[6].side_effect = process - self.stderr.truncate(0) - self.stderr.seek(0) - process.calls.clear() - with self.subTest(explicit=explicit), mock.patch.object(plugin.logger, 'disabled', True): - with self.assertRaises(KeyboardInterrupt): - if explicit: - plugin.maybe_install_azure_plugin(self.cmd, True, list(plugin.HOST_IDS)) - else: - self.run_optional(['y', '1 2 3', 'y']) - message = self.stderr.getvalue() - for part in ('Claude Code', 'native install refused', 'GitHub Copilot CLI: interrupted', - 'native state is uncertain', 'kubectl and kubelogin remain installed', - 'native plugin commands', 'No automatic retry or rollback'): - self.assertIn(part, message) - self.assertEqual(process.calls[-1], ('copilot', 'plugin', 'list', '--json')) - self.assertFalse(any(call[0] == 'codex' for call in process.calls)) + self.mocks[6].side_effect = process + with mock.patch.object(plugin.logger, 'disabled', True), self.assertRaises(KeyboardInterrupt): + plugin.maybe_install_azure_plugin(self.cmd, True, list(plugin.HOST_IDS)) + message = self.stderr.getvalue() + for part in ('Claude Code', 'native install refused', 'GitHub Copilot CLI: interrupted', + 'native state is uncertain', 'kubectl and kubelogin remain installed', + 'native plugin commands', 'No automatic retry or rollback'): + self.assertIn(part, message) + self.assertEqual(process.calls[-1], ('copilot', 'plugin', 'list', '--json')) + self.assertFalse(any(call[0] == 'codex' for call in process.calls)) + self.assertNotIn('azure@', message) + self.assertNotIn('marketplace upgrade', message) def test_cancellation_reports_prior_success_without_continuing(self): self.mocks[5].side_effect = [True, KeyboardInterrupt(), True] @@ -361,12 +345,16 @@ def test_cancellation_reports_prior_success_without_continuing(self): self.assertEqual(self.mocks[5].call_args_list, [mock.call('claude-code'), mock.call('github-copilot')]) self.assertIn('Claude Code: installed', self.stderr.getvalue()) self.assertIn('GitHub Copilot CLI: interrupted', self.stderr.getvalue()) + self.assertIn('claude plugin update azure@claude-plugins-official --scope user', self.stderr.getvalue()) + self.assertIn('claude plugin uninstall azure@claude-plugins-official --scope user', self.stderr.getvalue()) + self.assertNotIn('copilot plugin', self.stderr.getvalue()) + self.assertNotIn('codex plugin', self.stderr.getvalue()) def test_explicit_sudo_is_rejected_before_any_execution(self): with mock.patch.dict(os.environ, {'SUDO_UID': '123'}), self.assertRaises(InvalidArgumentValueError): plugin.maybe_install_azure_plugin(self.cmd, True, ['codex']) self.assertEqual(self.installed, []) - self.assertEqual(self.prompts, []) + self.mocks[4].assert_not_called() class AzurePluginNativeTest(unittest.TestCase): @@ -978,14 +966,3 @@ def test_runner_has_no_shell_and_finite_default_timeout(self): self.assertEqual(kwargs['stdin'], subprocess.DEVNULL) self.assertGreater(exchange.call_args.kwargs['timeout'], 0) self.assertLessEqual(exchange.call_args.kwargs['timeout'], 300) - - -class AzurePluginDiscoveryTest(unittest.TestCase): - def test_discovery_only_checks_executable_presence(self): - with mock.patch.object(plugin.shutil, 'which', side_effect=lambda name: '/bin/' + name if name == 'codex' else None), \ - mock.patch('subprocess.Popen', side_effect=AssertionError('Discovery must not execute a host')): - self.assertEqual(plugin.discover_hosts(), ['codex']) - - def test_discovery_uses_stable_host_order(self): - with mock.patch.object(plugin.shutil, 'which', side_effect=lambda name: '/bin/' + name): - self.assertEqual(plugin.discover_hosts(), ['claude-code', 'github-copilot', 'codex'])