From 6a563e6045a11d9b9d91e903cbfa8b82c1aa1970 Mon Sep 17 00:00:00 2001 From: "Jessica Zhang (NY)" Date: Wed, 3 Jul 2024 14:34:55 -0400 Subject: [PATCH 1/4] Fix CI tests --- .circleci/config.yml | 4 ++-- .../create_integration_test_ami.py | 8 ++++++- .../setup_integration_test_ami.py | 8 ++++++- .../tests/test_ray_aws_launcher.py | 22 ++++++++++++++----- 4 files changed, 33 insertions(+), 9 deletions(-) diff --git a/.circleci/config.yml b/.circleci/config.yml index 8ee8f9c0df9..a9933161a0c 100644 --- a/.circleci/config.yml +++ b/.circleci/config.yml @@ -39,8 +39,8 @@ commands: if [[ -f ~/miniconda3/LICENSE.txt ]] ; then echo "miniconda installed already." else - curl -o Miniconda3-py38_4.8.3-MacOSX-x86_64.sh https://repo.anaconda.com/miniconda/Miniconda3-py38_4.8.3-MacOSX-x86_64.sh - bash ./Miniconda3-py38_4.8.3-MacOSX-x86_64.sh -b + curl -o Miniconda3-py39_24.5.0-0-MacOSX-arm64.sh https://repo.anaconda.com/miniconda/Miniconda3-py39_24.5.0-0-MacOSX-arm64.sh + bash ./Miniconda3-py39_24.5.0-0-MacOSX-arm64.sh -b fi ~/miniconda3/bin/conda init bash - run: diff --git a/plugins/hydra_ray_launcher/integration_test_tools/create_integration_test_ami.py b/plugins/hydra_ray_launcher/integration_test_tools/create_integration_test_ami.py index dc2ab834e75..6352e4ae874 100644 --- a/plugins/hydra_ray_launcher/integration_test_tools/create_integration_test_ami.py +++ b/plugins/hydra_ray_launcher/integration_test_tools/create_integration_test_ami.py @@ -24,7 +24,13 @@ def _run_command(command: str) -> str: print(f"{str(datetime.now())} - Running: {command}") - output = subprocess.getoutput(command) + # Do some basic validation to avoid injection but it is not exhaustive, + # there is still a security risk here! + if ";" in command or "||" in command or "&&" in command or ">" in command: + raise ValueError( + "To avoid possible injection, command cannot contain ; || or &&" + ) + output = subprocess.getoutput(command) # nosec B605 print(f"{str(datetime.now())} - {output}") return output diff --git a/plugins/hydra_ray_launcher/integration_test_tools/setup_integration_test_ami.py b/plugins/hydra_ray_launcher/integration_test_tools/setup_integration_test_ami.py index f114e67b424..c0f861b6129 100644 --- a/plugins/hydra_ray_launcher/integration_test_tools/setup_integration_test_ami.py +++ b/plugins/hydra_ray_launcher/integration_test_tools/setup_integration_test_ami.py @@ -16,7 +16,13 @@ def _run_command(command: str) -> str: print(f"{str( datetime.now() )} - OUT: {command}") - output = subprocess.getoutput(command) + # Do some basic validation to avoid injection but it is not exhaustive, + # there is still a security risk here! + if ";" in command or "||" in command or "&&" in command or ">" in command: + raise ValueError( + "To avoid possible injection, command cannot contain ; || or &&" + ) + output = subprocess.getoutput(command) # nosec B605 print(f"{str( datetime.now() )} - OUT: {output}") return output diff --git a/plugins/hydra_ray_launcher/tests/test_ray_aws_launcher.py b/plugins/hydra_ray_launcher/tests/test_ray_aws_launcher.py index c06aa2c67e5..26cf4c4dcff 100644 --- a/plugins/hydra_ray_launcher/tests/test_ray_aws_launcher.py +++ b/plugins/hydra_ray_launcher/tests/test_ray_aws_launcher.py @@ -11,7 +11,11 @@ import boto3 # type: ignore import pkg_resources -from botocore.exceptions import NoCredentialsError, NoRegionError # type: ignore +from botocore.exceptions import ( # type: ignore + ClientError, + NoCredentialsError, + NoRegionError, +) from hydra.core.plugins import Plugins from hydra.plugins.launcher import Launcher from hydra.test_utils.launcher_common_tests import ( @@ -49,7 +53,7 @@ ec2 = boto3.client("ec2") ec2.describe_regions() aws_not_configured = False -except (NoCredentialsError, NoRegionError): +except (ClientError, NoCredentialsError, NoRegionError): aws_not_configured = True @@ -129,7 +133,13 @@ def run_command(commands: str) -> str: log.info(f"running: {commands}") - output = subprocess.getoutput(commands) + # Do some basic validation to avoid injection but it is not exhaustive, + # there is still a security risk here! + if ";" in commands or "||" in commands or "&&" in commands or ">" in commands: + raise ValueError( + "To avoid possible injection, command cannot contain ; || or &&" + ) + output = subprocess.getoutput(commands) # nosec B605 log.info(f"outputs: {output}") return output @@ -139,7 +149,8 @@ def build_ray_launcher_wheel(tmp_wheel_dir: str) -> str: plugin = "hydra_ray_launcher" os.chdir(Path("plugins") / plugin) log.info(f"Build wheel for {plugin}, save wheel to {tmp_wheel_dir}.") - run_command(f"python setup.py sdist bdist_wheel && cp dist/*.whl {tmp_wheel_dir}") + run_command("python setup.py sdist bdist_wheel") + run_command(f"cp dist/*.whl {tmp_wheel_dir}") log.info("Download all plugin dependency wheels.") run_command(f"pip download . -d {tmp_wheel_dir}") plugin_wheel = run_command("ls dist/*.whl").split("/")[-1] @@ -149,7 +160,8 @@ def build_ray_launcher_wheel(tmp_wheel_dir: str) -> str: def build_core_wheel(tmp_wheel_dir: str) -> str: chdir_hydra_root() - run_command(f"python setup.py sdist bdist_wheel && cp dist/*.whl {tmp_wheel_dir}") + run_command("python setup.py sdist bdist_wheel") + run_command(f"cp dist/*.whl {tmp_wheel_dir}") # download dependency wheel for hydra-core run_command(f"pip download -r requirements/requirements.txt -d {tmp_wheel_dir}") From 9bf7c4070365cf5b28ff933c143b9fb67ee43323 Mon Sep 17 00:00:00 2001 From: jesszzzz Date: Mon, 15 Jul 2024 10:17:39 -0400 Subject: [PATCH 2/4] Remove unnecessary command injection checks in tests Co-authored-by: Omry Yadan --- .../integration_test_tools/create_integration_test_ami.py | 6 ------ 1 file changed, 6 deletions(-) diff --git a/plugins/hydra_ray_launcher/integration_test_tools/create_integration_test_ami.py b/plugins/hydra_ray_launcher/integration_test_tools/create_integration_test_ami.py index 6352e4ae874..1f5bdc67a76 100644 --- a/plugins/hydra_ray_launcher/integration_test_tools/create_integration_test_ami.py +++ b/plugins/hydra_ray_launcher/integration_test_tools/create_integration_test_ami.py @@ -24,12 +24,6 @@ def _run_command(command: str) -> str: print(f"{str(datetime.now())} - Running: {command}") - # Do some basic validation to avoid injection but it is not exhaustive, - # there is still a security risk here! - if ";" in command or "||" in command or "&&" in command or ">" in command: - raise ValueError( - "To avoid possible injection, command cannot contain ; || or &&" - ) output = subprocess.getoutput(command) # nosec B605 print(f"{str(datetime.now())} - {output}") return output From c7fc511780a25f06f7b3fc112f3d12f436876bd2 Mon Sep 17 00:00:00 2001 From: jesszzzz Date: Mon, 15 Jul 2024 10:17:46 -0400 Subject: [PATCH 3/4] Remove unnecessary command injection checks in tests Co-authored-by: Omry Yadan --- .../integration_test_tools/setup_integration_test_ami.py | 6 ------ 1 file changed, 6 deletions(-) diff --git a/plugins/hydra_ray_launcher/integration_test_tools/setup_integration_test_ami.py b/plugins/hydra_ray_launcher/integration_test_tools/setup_integration_test_ami.py index c0f861b6129..a49fccd692a 100644 --- a/plugins/hydra_ray_launcher/integration_test_tools/setup_integration_test_ami.py +++ b/plugins/hydra_ray_launcher/integration_test_tools/setup_integration_test_ami.py @@ -16,12 +16,6 @@ def _run_command(command: str) -> str: print(f"{str( datetime.now() )} - OUT: {command}") - # Do some basic validation to avoid injection but it is not exhaustive, - # there is still a security risk here! - if ";" in command or "||" in command or "&&" in command or ">" in command: - raise ValueError( - "To avoid possible injection, command cannot contain ; || or &&" - ) output = subprocess.getoutput(command) # nosec B605 print(f"{str( datetime.now() )} - OUT: {output}") return output From 7aa56cba0863438ce2951ca12b9c8fff04c3f854 Mon Sep 17 00:00:00 2001 From: jesszzzz Date: Mon, 15 Jul 2024 10:17:54 -0400 Subject: [PATCH 4/4] Remove unnecessary command injection checks in tests Co-authored-by: Omry Yadan --- plugins/hydra_ray_launcher/tests/test_ray_aws_launcher.py | 6 ------ 1 file changed, 6 deletions(-) diff --git a/plugins/hydra_ray_launcher/tests/test_ray_aws_launcher.py b/plugins/hydra_ray_launcher/tests/test_ray_aws_launcher.py index 26cf4c4dcff..8c89a2b86bf 100644 --- a/plugins/hydra_ray_launcher/tests/test_ray_aws_launcher.py +++ b/plugins/hydra_ray_launcher/tests/test_ray_aws_launcher.py @@ -133,12 +133,6 @@ def run_command(commands: str) -> str: log.info(f"running: {commands}") - # Do some basic validation to avoid injection but it is not exhaustive, - # there is still a security risk here! - if ";" in commands or "||" in commands or "&&" in commands or ">" in commands: - raise ValueError( - "To avoid possible injection, command cannot contain ; || or &&" - ) output = subprocess.getoutput(commands) # nosec B605 log.info(f"outputs: {output}") return output