Merge branch 'dev' into cfs

.azure-pipelines/templates/variables.yml

@@ -2,4 +2,4 @@ variables:
   ubuntu_pool: 'pool-ubuntu-2004'
   windows_pool: 'pool-windows-2019'
   ubuntu_arm64_pool: 'ubuntu-arm64-2004-pool'
-  macos_pool: 'macOS-12'
+  macos_pool: 'macOS-14'

.github/workflows/RunIssueSentinel.yml

@@ -12,4 +12,6 @@ jobs:
       - name: Run Issue Sentinel
         uses: Azure/issue-sentinel@v1
         with:
-          password: ${{secrets.ISSUE_SENTINEL_PASSWORD}}
+          password: ${{secrets.ISSUE_SENTINEL_PASSWORD}}
+          enable-similar-issues-scanning: true
+          enable-security-issues-scanning: true

azure-pipelines.yml

@@ -140,7 +140,7 @@ jobs:
   displayName: 'Verify src/azure-cli/requirements.*.Darwin.txt'
   condition: succeeded()
   pool:
-    vmImage: 'macOS-12'
+    vmImage: ${{ variables.macos_pool }}
 
   steps:
   - task: UsePythonVersion@0
@@ -641,7 +641,7 @@ jobs:
   dependsOn: BuildHomebrewFormula
   condition: succeeded()
   pool:
-    vmImage: 'macOS-12'
+    vmImage: ${{ variables.macos_pool }}
   steps:
   - task: DownloadPipelineArtifact@1
     displayName: 'Download Metadata'
@@ -685,7 +685,7 @@ jobs:
   # condition: and(succeeded(), in(variables['Build.Reason'], 'IndividualCI', 'BatchedCI', 'Manual', 'Schedule'))
   condition: false
   pool:
-    vmImage: 'macOS-12'
+    vmImage: ${{ variables.macos_pool }}
   steps:
   - task: DownloadPipelineArtifact@1
     displayName: 'Download Metadata'
@@ -923,12 +923,6 @@ jobs:
           pool: ${{ arch.pool }}
 
         # https://wiki.debian.org/DebianReleases
-        Buster ${{ arch.name }}:
-          # 10
-          deb_system: debian
-          distro: buster
-          arch: ${{ arch.value }}
-          pool: ${{ arch.pool }}
         Bullseye ${{ arch.name }}:
           # 11
           deb_system: debian
@@ -989,11 +983,6 @@ jobs:
           distro: noble
           arch: ${{ arch.value }}
           pool: ${{ arch.pool }}
-        Buster ${{ arch.name }}:
-          deb_system: debian
-          distro: buster
-          arch: ${{ arch.value }}
-          pool: ${{ arch.pool }}
         Bullseye ${{ arch.name }}:
           deb_system: debian
           distro: bullseye

doc/cli_subprocess_guidelines.md

@@ -0,0 +1,112 @@
+# Azure CLI Subprocess Guidelines
+
+In certain CLI modules, there are scenarios that need to call a subprocess to run commands outside CLI, like getting kubectl info in aks, or deployment setup in mysql using `git` and `gh`, through python built-in [subprocess](https://docs.python.org/3/library/subprocess.html) module. Despite its simplicity and versatility, ensuring the security of applying it can be challenging in different platforms under different circumstance, and it is error-prone for developers to neglect security best practices during development.
+
+
+## Insecure Use Of Commands Under Subprocess Module
+
+
+### Dynamic command construction under risk
+
+Assume a script that needs to read in `user_input` from users to execute a `git` related command and runs it through subprocess using shell. In regular cases, users would add a valid git command, like `git --version` or something else, but if undesired input gets logged into this system, like `--version;echo aa`, the appended `;echo aa` would be executed by `subprocess` too, like below: 
+
+```python
+import subprocess
+user_input = input("input a git command to run: ")
+cmd = f"git {user_input}"
+subprocess.run(cmd, shell=True)
+```
+
+```console
+input a git command to run: --version;echo aa
+git version 2.34.1
+aa
+```
+
+This is a simple example for demonstrating the side effects in python's `subprocess` improper usage. And it's common for CLI developers to build and execute commands dynamically from users' input in a more complicated way. When constructing and executing commands in subprocess through `shell=True`, it exposes a big security vulnerability to potential malicious users outside. 
+
+
+## Mitigating Security Vulnerability When Calling Subprocess Commands
+
+There are several aspects of security practices that developers need to have in mindset to safeguard their CLI modules from command injection attacks.
+
+### CLI centralized subprocess executing
+
+Azure CLI provides a centralized function `run_cmd` adapted from official `subprocess.run`, with necessary argument covered and illegal input blocking enforced. 
+
+What developers need to do is:
+1. `from azure.cli.core.util import run_cmd`
+2. replace `subprocess.run` (or `Popen` or `check_call` or `check_output` or `call`) with `run_cmd`.
+3. construct cmd `args` as array, like `[executable, arg0, arg1, arg2, ...]`
+
+`run_cmd` add necessary argument type checks and process the input and output the same way as `subprocess.run`, and block potential risks from commands constructed from user input.
+Below is an example for `run_cmd` use case:
+
+```python
+# code before:
+cmd = f"git commit -m {user_message}"
+import subprocess
+output = subprocess.run(cmd, shell=True, check=False, stderr=subprocess.PIPE, stdout=subprocess.PIPE)
+if output.returncode != 0:
+      raise CLIInternalError('Command execution failed, command is: '{}', error message is: {}'.format(cmd, output.stderr))
+return output.stdout
+```
+
+If `run_cmd` is applied, it would be like:
+```python
+# code after:
+cmd = ["git","commit", "-m", user_message]
+import subprocess
+from azure.cli.core.util import run_cmd
+output = run_cmd(cmd, check=False, capture_output=True)
+if output.returncode != 0:
+      raise CLIInternalError('Command execution failed, command is: '{}', error message is: {}'.format(cmd, output.stderr))
+return output.stdout
+```
+
+All various kinds of `subprocess` Popen calling use cases can be easily adjusted into `run_cmd` with security risks processed and eliminated in this centralized function.
+
+#### Notes
+Besides that, users might need to know some parts of the accessibility in both `run_cmd` and `subprocess`
+1. when calling shell built-in cmds, like `dir` or `echo`, using `shell=True` **in windows platform**, `subprocess` implicitly uses `cmd.exe`, while `run_cmd` asks developers to provide the `cmd.exe` as executable file specifically in the arg list's first item, like `["cmd.exe", "/c", "echo", "abc"]`
+2. if developers want to find an easy way to split their current cmd string into list, **for unix-like platforms**, developers can apply [`shlex.split`](https://docs.python.org/3/library/shlex.html#shlex.split) for quick access. But a prepared cmd statement is still more recommended (for more info about prepared cmd statement, please read below sections). 
+3. if developers want to locate the target command's executable file, a tool developers can use is `shutil.which` that gives the full executable file path in system, like `shutil.which(git)` returns the full `git.exe` path in windows platform `C:\\Program Files\\Git\\cmd\\git.EXE`. 
+4. if the target cmd is `az`-related, like `az group show --name xxxx`, please use internal function call `cli.invoke(["az", "group", "show", "--name", "xxx"])` from a newly created CLI context to get the target information.
+
+
+### Best practices in subprocess use cases
+
+
+The following sections discuss some secure coding conventions that, when implemented, can help protect CLI applications from command injection vulnerabilities when using `subprocess` module.
+
+#### Proper input validation and sanitization
+
+Input from users or external sources should never be trusted when appending them into `subprocess` cmd. Developers better provide expected patterns to validate and sanitize user input before adding that into subprocess' cmd. 
+Below is an example input sanitizer that only allows alphanumeric characters from the input string.  
+
+```python
+import re
+def sanitize_input(user_input):
+    return re.sub(r'[^a-zA-Z0-9]', '', user_input)
+```
+In a real CLI module scenario, developers better use their corresponding sanitization pattern for reducing the chaining commands risks.
+
+#### Use parameterized cmd statement
+
+A parameterized cmd statement is a way to structure a command starting from defining the command structure first and then provide the parameters that should be inserted into the command separately. 
+In this way, the system will treat the first item as the executable file and the left item as args of that executable application.
+
+```python
+# instead of
+cmd = f"git {user_input}"
+# using 
+cmd = ["git", user_input]
+```
+
+#### Avoid use `shell=True` with user_inputs
+
+When using subprocess module, avoid `shell=True` argument when it comes with cmd containing user inputs. When python executes a command with `shell=True`, the system shell will interpret any string passed in and if this string contains user input without proper sanitization, an attacker can inject malicious commands, as demonstrated in the very beginning of this doc.
+
+
+## Summary
+Ensuring the safety of Azure CLI from command injection under subprocess calling requires an in-depth understanding of these vulnerabilities and also proactive measures to counteract potential exploits. CLI developers should apply the security practices before using builtin `subprocess`, and it's recommended to use the centralized function `run_cmd` CLI provided, to safeguard CLI modules from command injection attack and for future more accessible security enforcements.

linter_exclusions.yml

@@ -2754,6 +2754,14 @@ network vnet peering create:
     allow_gateway_transit:
       rule_exclusions:
       - option_length_too_long
+network vnet peering update:
+  parameters:
+    allow_forwarded_traffic:
+      rule_exclusions:
+      - option_length_too_long
+    allow_gateway_transit:
+      rule_exclusions:
+      - option_length_too_long
 network vnet subnet create:
   parameters:
     service_endpoint_policy:

pylintrc

@@ -32,6 +32,7 @@ disable=
     subprocess-run-check,
     super-with-arguments,
     too-many-arguments,
+    too-many-positional-arguments,
     too-many-function-args,
     too-many-lines,
     using-constant-test,

src/azure-cli-core/HISTORY.rst

@@ -3,6 +3,10 @@
 Release History
 ===============
 
+2.65.0
+++++++
+* Minor fixes
+
 2.64.0
 ++++++
 * Minor fixes

src/azure-cli-core/azure/cli/core/__init__.py

@@ -4,7 +4,7 @@
 # --------------------------------------------------------------------------------------------
 # pylint: disable=line-too-long
 
-__version__ = "2.64.0"
+__version__ = "2.65.0"
 
 import os
 import sys

src/azure-cli-core/azure/cli/core/_profile.py

@@ -11,7 +11,7 @@
 from azure.cli.core._session import ACCOUNT
 from azure.cli.core.azclierror import AuthenticationError
 from azure.cli.core.cloud import get_active_cloud, set_cloud_subscription
-from azure.cli.core.util import in_cloud_console, can_launch_browser
+from azure.cli.core.util import in_cloud_console, can_launch_browser, is_github_codespaces
 from knack.log import get_logger
 from knack.util import CLIError
 
@@ -166,6 +166,10 @@ def login(self,
                 logger.info('No web browser is available. Fall back to device code.')
                 use_device_code = True
 
+            if not use_device_code and is_github_codespaces():
+                logger.info('GitHub Codespaces is detected. Fall back to device code.')
+                use_device_code = True
+
             if use_device_code:
                 user_identity = identity.login_with_device_code(scopes=scopes, **kwargs)
             else:

src/azure-cli-core/azure/cli/core/aaz/_arg.py

@@ -6,6 +6,7 @@
 import copy
 
 from azure.cli.core import azclierror
+from azure.cli.core.commands.arm import add_usage, remove_usage, set_usage
 from knack.arguments import CLICommandArgument, CaseInsensitiveList
 from knack.preview import PreviewItem
 from knack.experimental import ExperimentalItem
@@ -626,12 +627,11 @@ def _build_cmd_action(self):
 
 
 class AAZGenericUpdateSetArg(AAZGenericUpdateArg):
-    _example = '--set property1.property2=<value>'
 
     def __init__(
             self, options=('--set',), arg_group='Generic Update',
             help='Update an object by specifying a property path and value to set.'
-                 '  Example: {}'.format(_example),
+                 '  Example: {}'.format(set_usage),
             **kwargs):
         super().__init__(
             options=options,
@@ -653,12 +653,11 @@ class Action(AAZGenericUpdateAction):
 
 
 class AAZGenericUpdateAddArg(AAZGenericUpdateArg):
-    _example = '--add property.listProperty <key=value, string or JSON string>'
 
     def __init__(
             self, options=('--add',), arg_group='Generic Update',
             help='Add an object to a list of objects by specifying a path and key value pairs.'
-                 '  Example: {}'.format(_example),
+                 '  Example: {}'.format(add_usage),
             **kwargs):
         super().__init__(
             options=options,
@@ -680,12 +679,11 @@ class Action(AAZGenericUpdateAction):
 
 
 class AAZGenericUpdateRemoveArg(AAZGenericUpdateArg):
-    _example = '--remove property.list <indexToRemove> OR --remove propertyToRemove'
 
     def __init__(
             self, options=('--remove', ), arg_group='Generic Update',
             help='Remove a property or an element from a list.'
-                 '  Example: {}'.format(_example),
+                 '  Example: {}'.format(remove_usage),
             **kwargs):
         super().__init__(
             options=options,

src/azure-cli-core/azure/cli/core/breaking_change.py

@@ -250,8 +250,10 @@ def register(self, cli_ctx):
         elif self.is_command_group(cli_ctx):
             command_group = cli_ctx.invocation.commands_loader.command_group_table[self.command_name]
             if not command_group:
-                self._register_to_direct_sub_cg_or_command(cli_ctx, self.command_name, self.to_tag(cli_ctx))
-            else:
+                loader = self.find_suitable_command_loader(cli_ctx)
+                command_group = loader.command_group(self.command_name)
+                cli_ctx.invocation.commands_loader.command_group_table[self.command_name] = command_group
+            if command_group:
                 command_group.group_kwargs['deprecate_info'] = \
                     self.appended_status_tag(cli_ctx, command_group.group_kwargs.get('deprecate_info'),
                                              self.to_tag(cli_ctx))
@@ -270,22 +272,11 @@ def appended_status_tag(cli_ctx, old_status_tag, new_status_tag):
             return old_status_tag
         return new_status_tag
 
-    def _register_to_direct_sub_cg_or_command(self, cli_ctx, cg_name, status_tag):
-        for key, command_group in cli_ctx.invocation.commands_loader.command_group_table.items():
-            split_key = key.rsplit(maxsplit=1)
-            # If inpass command group name is empty, all first level command group should be registered to.
-            # Otherwise, we need to find all direct sub command groups.
-            if (not cg_name and len(split_key) == 1) or (len(split_key) == 2 and split_key[0] == cg_name):
-                from azure.cli.core.commands import AzCommandGroup
-                if isinstance(command_group, AzCommandGroup):
-                    command_group.group_kwargs['deprecate_info'] = \
-                        self.appended_status_tag(cli_ctx, command_group.group_kwargs.get('deprecate_info'), status_tag)
-                else:
-                    self._register_to_direct_sub_cg_or_command(cli_ctx, key, status_tag)
-        for key, command in cli_ctx.invocation.commands_loader.command_table.items():
-            # If inpass command group name is empty, all first level command should be registered to.
-            if (not cg_name and ' ' not in key) or key.rsplit(maxsplit=1)[0] == cg_name:
-                command.deprecate_info = self.appended_status_tag(cli_ctx, command.deprecate_info, self.to_tag(cli_ctx))
+    def find_suitable_command_loader(self, cli_ctx):
+        for name, loader in cli_ctx.invocation.commands_loader.cmd_to_loader_map.items():
+            if name.startswith(self.command_name) and loader:
+                return loader[0]
+        return None
 
     def _register_option_deprecate(self, cli_ctx, arguments, option_name):
         for _, argument in arguments.items():

src/azure-cli-core/azure/cli/core/commands/__init__.py

@@ -860,7 +860,7 @@ def _resolve_output_sensitive_data_warning(self, cmd, result):
         from ..credential_helper import distinguish_credential
         from ..telemetry import set_secrets_detected
         try:
-            containing_credential, secret_property_names = distinguish_credential(result)
+            containing_credential, secret_property_names, secret_names = distinguish_credential(result)
             if not containing_credential:
                 set_secrets_detected(False)
                 return
@@ -869,10 +869,10 @@ def _resolve_output_sensitive_data_warning(self, cmd, result):
             if secret_property_names:
                 message = sensitive_data_detailed_warning_message.format(', '.join(secret_property_names))
             logger.warning(message)
-            set_secrets_detected(True, secret_property_names)
-        except Exception:  # pylint: disable=broad-except
+            set_secrets_detected(True, secret_property_names, secret_names)
+        except Exception as ex:  # pylint: disable=broad-except
             # ignore all exceptions, as this is just a warning
-            pass
+            logger.debug('Scan credentials failed with %s', str(ex))
 
     def resolve_confirmation(self, cmd, parsed_args):
         confirm = cmd.confirmation and not parsed_args.__dict__.pop('yes', None) \