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.

Sign up for GitHub

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

Jump to bottom

{Mysql} Mitigate subprocess security risk #29991

Draft
wants to merge 2 commits into
base: dev
Choose a base branch
from
Draft

{Mysql} Mitigate subprocess security risk #29991

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
2b28257
mitigate subprocess security risk
AllyW Sep 27, 2024
3b30b0a
fix style
AllyW Sep 27, 2024
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
29 changes: 10 additions & 19 deletions src/azure-cli/azure/cli/command_modules/mysql/_util.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@
import math
import os
import random
import subprocess
import secrets
import string
import yaml
Expand All @@ -23,7 +22,7 @@
from msrestazure.azure_exceptions import CloudError
from azure.cli.core.commands.client_factory import get_subscription_id
from azure.cli.core.commands.progress import IndeterminateProgressBar
from azure.cli.core.util import CLIError
from azure.cli.core.util import CLIError, run_cmd
from azure.core.exceptions import HttpResponseError
from azure.core.paging import ItemPaged
from azure.core.rest import HttpRequest
Expand Down Expand Up @@ -375,19 +374,11 @@ def _resolve_api_version(client, provider_namespace, resource_type, parent_path)

def run_subprocess(command, stdout_show=None):
if stdout_show:
process = subprocess.Popen(command, shell=True)
process = run_cmd(command)
else:
process = subprocess.Popen(command, shell=True, stdout=subprocess.PIPE, stderr=subprocess.PIPE)
process.wait()
process = run_cmd(command, capture_output=True)
if process.returncode:
logger.warning(process.stderr.read().strip().decode('UTF-8'))


def run_subprocess_get_output(command):
commands = command.split()
process = subprocess.Popen(commands, shell=True, stdout=subprocess.PIPE, stderr=subprocess.PIPE)
process.wait()
return process
logger.warning(process.stderr.strip().decode('UTF-8'))


def register_credential_secrets(cmd, database_engine, server, repository):
Expand All @@ -414,7 +405,7 @@ def register_credential_secrets(cmd, database_engine, server, repository):
credential_file = "./temp_app_credential.txt"
with open(credential_file, "w") as f:
f.write(app_json)
run_subprocess('gh secret set {} --repo {} < {}'.format(AZURE_CREDENTIALS, repository, credential_file))
run_subprocess(["gh", "secret", "set", AZURE_CREDENTIALS, "--repo", repository, "<", credential_file])
os.remove(credential_file)


Expand All @@ -423,7 +414,7 @@ def register_connection_secrets(server, database_name, administrator_login,
logger.warning("Added secret %s to github repository", connection_string_name)
connection_string = "Server={}; Port=3306; Database={}; Uid={}; Pwd={}; SslMode=Preferred;".format(
server.fully_qualified_domain_name, database_name, administrator_login, administrator_login_password)
run_subprocess('gh secret set {} --repo {} -b"{}"'.format(connection_string_name, repository, connection_string))
run_subprocess(['gh', 'secret', 'set', connection_string_name, '--repo', repository, '-b', connection_string])


def fill_action_template(cmd, database_engine, server, database_name, administrator_login,
Expand All @@ -433,8 +424,8 @@ def fill_action_template(cmd, database_engine, server, database_name, administra
if not os.path.exists(action_dir):
os.makedirs(action_dir)

process = run_subprocess_get_output("gh secret list --repo {}".format(repository))
github_secrets = process.stdout.read().strip().decode('UTF-8')
process = run_cmd(["gh", "secret", "list", "--repo", repository], capture_output=True)
github_secrets = process.stdout.strip().decode('UTF-8')

if AZURE_CREDENTIALS not in github_secrets:
try:
Expand Down Expand Up @@ -469,8 +460,8 @@ def fill_action_template(cmd, database_engine, server, database_name, administra


def get_git_root_dir():
process = run_subprocess_get_output("git rev-parse --show-toplevel")
return process.stdout.read().strip().decode('UTF-8')
process = run_cmd(["git", "rev-parse", "--show-toplevel"], capture_output=True)
return process.stdout.strip().decode('UTF-8')


def get_user_confirmation(message, yes=False):
Expand Down
18 changes: 9 additions & 9 deletions src/azure-cli/azure/cli/command_modules/mysql/custom.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@
from azure.core.exceptions import ResourceNotFoundError
from azure.cli.core.commands.client_factory import get_subscription_id
from azure.cli.command_modules.mysql.random.generate import generate_username
from azure.cli.core.util import CLIError, sdk_no_wait, user_confirmation
from azure.cli.core.util import CLIError, sdk_no_wait, user_confirmation, run_cmd
from azure.cli.core.local_context import ALL
from azure.mgmt.rdbms import mysql_flexibleservers
from azure.cli.core.azclierror import ClientRequestError, RequiredArgumentMissingError, ArgumentUsageError, InvalidArgumentValueError, ValidationError
Expand All @@ -27,7 +27,7 @@
cf_mysql_firewall_rules
from ._util import resolve_poller, generate_missing_parameters, get_mysql_list_skus_info, generate_password, parse_maintenance_window, \
replace_memory_optimized_tier, build_identity_and_data_encryption, get_identity_and_data_encryption, get_tenant_id, run_subprocess, \
run_subprocess_get_output, fill_action_template, get_git_root_dir, get_single_to_flex_sku_mapping, get_firewall_rules_from_paged_response, \
fill_action_template, get_git_root_dir, get_single_to_flex_sku_mapping, get_firewall_rules_from_paged_response, \
ImportFromStorageProgressHook, OperationProgressBar, GITHUB_ACTION_PATH
from ._network import prepare_mysql_exist_private_dns_zone, prepare_mysql_exist_private_network, prepare_private_network, prepare_private_dns_zone, prepare_public_network
from ._validators import mysql_arguments_validator, mysql_auto_grow_validator, mysql_georedundant_backup_validator, mysql_restore_tier_validator, mysql_accelerated_logs_validator, \
Expand Down Expand Up @@ -223,12 +223,12 @@ def github_actions_setup(cmd, client, resource_group_name, server_name, database

action_path = get_git_root_dir() + GITHUB_ACTION_PATH + action_name + '.yml'
logger.warning("Making git commit for file %s", action_path)
run_subprocess("git add {}".format(action_path))
run_subprocess("git commit -m \"Add github action file\"")
run_subprocess(["git", "add", action_path])
run_subprocess(["git", "commit", "-m", "Add github action file"])

if allow_push:
logger.warning("Pushing the created action file to origin %s branch", branch)
run_subprocess("git push origin {}".format(branch))
run_subprocess(["git", "push", "origin", branch])
else:
logger.warning('You did not set --allow-push parameter. Please push the prepared file %s to your remote repo and run "deploy run" command to activate the workflow.', action_path)

Expand All @@ -237,17 +237,17 @@ def github_actions_run(action_name, branch):

gitcli_check_and_login()
logger.warning("Created an event for %s.yml in branch %s", action_name, branch)
run_subprocess("gh workflow run {}.yml --ref {}".format(action_name, branch))
run_subprocess(["gh", "workflow", "run", action_name + ".yml", "--ref", branch])


def gitcli_check_and_login():
output = run_subprocess_get_output("gh")
output = run_cmd(["gh"], capture_output=True)
if output.returncode:
raise ClientRequestError('Please install "Github CLI" to run this command.')

output = run_subprocess_get_output("gh auth status")
output = run_cmd(["gh", "auth", "status"], capture_output=True)
if output.returncode:
run_subprocess("gh auth login", stdout_show=True)
run_subprocess(["gh", "auth", "login"], stdout_show=True)


# Custom functions for server logs
Expand Down