Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

{doc} Add security guidance for az cmd execution #30069

Open
wants to merge 5 commits into
base: dev
Choose a base branch
from
Open
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
60 changes: 57 additions & 3 deletions doc/cli_subprocess_guidelines.md
Original file line number Diff line number Diff line change
Expand Up @@ -67,12 +67,66 @@ 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`
Besides that, developers 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.
4. if the target cmd is `az`-related, like `az group show --name xxxx`, please see the following "`az`-related cmd execution" section.

### `az`-related command execution

If the target cmd is `az`-related, like `az group show --name xxxx`, instead of using `subprocess`, CLI recommends using internal function call to get the target information, as the following three options (if applicable):

1. SDK operation. If target cmd is an operation from SDK, developers can apply corresponding method from SDK directly. For example:
```python
# code before:
cmd = "az group show --name {}".format(resource_group_name)
import json
import subprocess
output = subprocess.run(cmd, shell=True, check=False, stderr=subprocess.PIPE, stdout=subprocess.PIPE)
resourceGroup = json.loads(output.stdout)
location = resourceGroup['location']

# code after:
from azure.cli.core.commands.client_factory import get_mgmt_service_client
from azure.cli.core.profiles import ResourceType
resource_client = get_mgmt_service_client(cmd.cli_ctx, ResourceType.MGMT_RESOURCE_RESOURCES)
location = resource_client.resource_groups.get(resource_group_name).location
```

2. Atomic cmd from codegen. If this cmd has been migrated through codegenV2, developers can call its atomic cmd. For example:
```python
# code before:
cmd = "az monitor autoscale show --name {} --resource-group {}".format(autoscale_name, resource_group_name)
import json
import subprocess
output = subprocess.run(cmd, shell=True, check=False, stderr=subprocess.PIPE, stdout=subprocess.PIPE)
autoscale_settings = json.loads(output.stdout)

# code after:
from .aaz.latest.monitor.autoscale import Show as AutoScaleShow
autoscale_settings = AutoScaleShow(cli_ctx=instance.cli_ctx)(command_args={
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

command_args is a positional arugment:

It is not the convention to call it as keyword arguments as command_args=.

"resource_group": resource_group_name,
"autoscale_name": autoscale_name
})
```

3. if neither of the two methods above is applicable, developers can apply the central function `run_az_cmd` CLI provided to execute `az`-related cmds. For example:
```python
# code before:
cmd = "az group show --name {}".format(resource_group_name)
import json
import subprocess
output = subprocess.run(cmd, shell=True, check=False, stderr=subprocess.PIPE, stdout=subprocess.PIPE)
resourceGroup = json.loads(output.stdout)
location = resourceGroup['location']

# code after:
cmd = ["az", "group", "show", "--name", resource_group_name]
from azure.cli.core.util import run_az_cmd
resourceGroup = run_az_cmd(cmd).result
location = resourceGroup['location']
```

### Best practices in subprocess use cases

Expand Down Expand Up @@ -109,4 +163,4 @@ When using subprocess module, avoid `shell=True` argument when it comes with cmd


## 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.
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 (or the applicable three methods for `az`-related cmds) , to safeguard CLI modules from command injection attack and for future more accessible security enforcements.
Loading