Codeql fixes [azure-ai-ml] (#39536)

* change hash function

* codeql fixes

* correct uuid formation

* roll back one change

* hash change

* fix lint issue

* fix unit tests

* update recordings

* reverse recording

* reverse hash_dict change

* add object hash change

* pass function_scoped to true

* update recordings

* update recordings

* update recordings

* updates

* update failed test recording

* update failed test recording

Author: PratibhaShrivastav18

sdk/ml/azure-ai-ml/assets.json

@@ -2,5 +2,5 @@
   "AssetsRepo": "Azure/azure-sdk-assets",
   "AssetsRepoPrefixPath": "python",
   "TagPrefix": "python/ml/azure-ai-ml",
-  "Tag": "python/ml/azure-ai-ml_53a7a4a178"
+  "Tag": "python/ml/azure-ai-ml_a27e606230"
 }

sdk/ml/azure-ai-ml/azure/ai/ml/_utils/_asset_utils.py

@@ -5,9 +5,9 @@
 # pylint: disable=protected-access,too-many-lines
 
 import hashlib
+import json
 import logging
 import os
-import json
 import uuid
 import warnings
 from concurrent.futures import ThreadPoolExecutor, as_completed
@@ -16,17 +16,7 @@
 from os import PathLike
 from pathlib import Path
 from platform import system
-from typing import (
-    TYPE_CHECKING,
-    Any,
-    Dict,
-    Iterable,
-    List,
-    Optional,
-    Tuple,
-    Union,
-    cast,
-)
+from typing import TYPE_CHECKING, Any, Dict, Iterable, List, Optional, Tuple, Union, cast
 
 from colorama import Fore
 from tqdm import TqdmWarning, tqdm
@@ -67,11 +57,7 @@
 from azure.ai.ml._restclient.v2023_04_01.models import PendingUploadRequestDto
 from azure.ai.ml._utils._pathspec import GitWildMatchPattern, normalize_file
 from azure.ai.ml._utils.utils import convert_windows_path_to_unix, retry, snake_to_camel
-from azure.ai.ml.constants._common import (
-    MAX_AUTOINCREMENT_ATTEMPTS,
-    DefaultOpenEncoding,
-    OrderString,
-)
+from azure.ai.ml.constants._common import MAX_AUTOINCREMENT_ATTEMPTS, DefaultOpenEncoding, OrderString
 from azure.ai.ml.entities._assets.asset import Asset
 from azure.ai.ml.exceptions import (
     AssetPathException,
@@ -92,7 +78,7 @@
         ModelOperations,
     )
 
-hash_type = type(hashlib.md5())  # nosec
+hash_type = type(hashlib.sha256())  # nosec
 
 module_logger = logging.getLogger(__name__)
 
@@ -335,7 +321,7 @@ def _build_metadata_dict(name: str, version: str) -> Dict[str, str]:
 
 
 def get_object_hash(path: Union[str, os.PathLike], ignore_file: IgnoreFile = IgnoreFile()) -> str:
-    _hash = hashlib.md5(b"Initialize for october 2021 AML CLI version")  # nosec
+    _hash = hashlib.sha256(b"Initialize for october 2021 AML CLI version")  # nosec
     if Path(path).is_dir():
         object_hash = _get_dir_hash(directory=path, _hash=_hash, ignore_file=ignore_file)
     else:

sdk/ml/azure-ai-ml/azure/ai/ml/_utils/utils.py

@@ -788,9 +788,9 @@ def hash_dict(items: Dict[str, Any], keys_to_omit: Optional[Iterable[str]] = Non
     items = pydash.omit(items, keys_to_omit)
     # serialize dict with order so same dict will have same content
     serialized_component_interface = json.dumps(items, sort_keys=True)
-    object_hash = hashlib.md5()  # nosec
+    object_hash = hashlib.sha256()
     object_hash.update(serialized_component_interface.encode("utf-8"))
-    return str(UUID(object_hash.hexdigest()))
+    return str(UUID(object_hash.hexdigest()[:32]))
 
 
 def convert_identity_dict(

sdk/ml/azure-ai-ml/azure/ai/ml/constants/_component.py

@@ -147,4 +147,4 @@ class IOConstants:
     GROUP_TYPE_NAME = "group"
     # Note: ([a-zA-Z_]+[a-zA-Z0-9_]*) is a valid single key,
     # so a valid pipeline key is: ^{single_key}([.]{single_key})*$
-    VALID_KEY_PATTERN = r"^([a-zA-Z_]+[a-zA-Z0-9_]*)([.]([a-zA-Z_]+[a-zA-Z0-9_]*))*$"
+    VALID_KEY_PATTERN = r"^[a-zA-Z_][a-zA-Z0-9_]*(\.[a-zA-Z_][a-zA-Z0-9_]*)*$"

sdk/ml/azure-ai-ml/azure/ai/ml/entities/_assets/_artifacts/model.py

@@ -24,7 +24,7 @@
 from azure.ai.ml.entities._assets import Artifact
 from azure.ai.ml.entities._assets.intellectual_property import IntellectualProperty
 from azure.ai.ml.entities._system_data import SystemData
-from azure.ai.ml.entities._util import get_md5_string, load_from_dict
+from azure.ai.ml.entities._util import get_sha256_string, load_from_dict
 
 from .artifact import ArtifactStorageInfo
 
@@ -103,7 +103,7 @@ def __init__(
         if self._is_anonymous and self.path:
             _ignore_file = get_ignore_file(self.path)
             _upload_hash = get_object_hash(self.path, _ignore_file)
-            self.name = get_md5_string(_upload_hash)
+            self.name = get_sha256_string(_upload_hash)
 
     @classmethod
     def _load(

sdk/ml/azure-ai-ml/azure/ai/ml/entities/_assets/environment.py

@@ -26,7 +26,7 @@
 from azure.ai.ml.entities._assets.intellectual_property import IntellectualProperty
 from azure.ai.ml.entities._mixins import LocalizableMixin
 from azure.ai.ml.entities._system_data import SystemData
-from azure.ai.ml.entities._util import get_md5_string, load_from_dict
+from azure.ai.ml.entities._util import get_sha256_string, load_from_dict
 from azure.ai.ml.exceptions import ErrorCategory, ErrorTarget, ValidationErrorType, ValidationException
 
 
@@ -374,20 +374,20 @@ def _generate_anonymous_name_version(
     ) -> None:
         hash_str = ""
         if source == "image":
-            hash_str = hash_str.join(get_md5_string(self.image))
+            hash_str = hash_str.join(get_sha256_string(self.image))
             if inference_config:
-                hash_str = hash_str.join(get_md5_string(yaml.dump(inference_config, sort_keys=True)))
+                hash_str = hash_str.join(get_sha256_string(yaml.dump(inference_config, sort_keys=True)))
             if conda_file:
-                hash_str = hash_str.join(get_md5_string(conda_file))
+                hash_str = hash_str.join(get_sha256_string(conda_file))
         if source == "build":
             if self.build is not None and not self.build.dockerfile_path:
-                hash_str = hash_str.join(get_md5_string(self._upload_hash))
+                hash_str = hash_str.join(get_sha256_string(self._upload_hash))
             else:
                 if self.build is not None:
-                    hash_str = hash_str.join(get_md5_string(self._upload_hash)).join(
-                        get_md5_string(self.build.dockerfile_path)
+                    hash_str = hash_str.join(get_sha256_string(self._upload_hash)).join(
+                        get_sha256_string(self.build.dockerfile_path)
                     )
-        version_hash = get_md5_string(hash_str)
+        version_hash = get_sha256_string(hash_str)
         self.version = version_hash
         self.name = ANONYMOUS_ENV_NAME
 

sdk/ml/azure-ai-ml/azure/ai/ml/entities/_util.py

@@ -218,17 +218,17 @@ def decorate_validation_error(schema: Any, pretty_error: str, additional_message
     return f"Validation for {schema.__name__} failed:\n\n {pretty_error} \n\n {additional_message}"
 
 
-def get_md5_string(text: Optional[str]) -> str:
-    """Get md5 string for a given text.
+def get_sha256_string(text: Optional[str]) -> str:
+    """Get sha256 string for a given text.
 
-    :param text: The text to get md5 string for.
+    :param text: The text to get sha256 string for.
     :type text: str
-    :return: The md5 string.
+    :return: The sha256 string.
     :rtype: str
     """
     try:
         if text is not None:
-            return hashlib.md5(text.encode("utf8")).hexdigest()  # nosec
+            return hashlib.sha256(text.encode("utf8")).hexdigest()  # nosec
         return ""
     except Exception as ex:
         raise ex

sdk/ml/azure-ai-ml/tests/command_job/unittests/test_command_job_schema.py

@@ -127,7 +127,10 @@ def test_anonymous_assets(self):
         assert internal_representation.environment.name != envName
         assert internal_representation.environment.name == ANONYMOUS_ENV_NAME
         assert internal_representation.environment._is_anonymous
-        assert internal_representation.environment.version == "7edecdc2427764b4606a68e5f1a95fe3"
+        assert (
+            internal_representation.environment.version
+            == "9a8126427ef71af0cdc4a56218a9b24e764524f68383d5c4b645448376a03f6b"
+        )
 
         assert internal_representation.inputs["test1"].path == input_path
         # Validate default dataset is mounted

sdk/ml/azure-ai-ml/tests/component/unittests/test_flow_component.py

@@ -49,7 +49,7 @@ def test_component_load_from_dag(self):
                 "is_anonymous": False,
                 "is_archived": False,
                 "properties": {
-                    "client_component_hash": "19278001-3d52-0e43-dc43-4082128d8243",
+                    "client_component_hash": "a32d6c66-9253-d3b2-d068-b6a409ab6770",
                 },
                 "tags": {},
             },
@@ -115,7 +115,7 @@ def test_component_load_from_run(self):
                 "description": "A run of the basic flow",
                 "is_anonymous": False,
                 "is_archived": False,
-                "properties": {"client_component_hash": "bc6d5b98-1aef-0d5a-96ff-5803f1a906c8"},
+                "properties": {"client_component_hash": "f060820c-7fb3-56a8-790c-ae2969a7b544"},
                 "tags": {},
             },
         }
@@ -199,7 +199,7 @@ def test_flow_component_entity(self):
                 "is_anonymous": False,
                 "is_archived": False,
                 # note that this won't take effect actually
-                "properties": {"client_component_hash": "0eec0297-5f6d-e333-780d-c76871ad9c57"},
+                "properties": {"client_component_hash": "07cdc416-c6ee-0beb-3d1b-4e5a5c4b44ec"},
                 "tags": {},
             },
         }

sdk/ml/azure-ai-ml/tests/conftest.py

@@ -630,15 +630,19 @@ def pipeline_samples_e2e_registered_eval_components(client: MLClient) -> Compone
 
 @pytest.fixture
 def mock_code_hash(request, mocker: MockFixture) -> None:
+    fake_uuid = "00000000000000000000000000000000"
+
     def generate_hash(*args, **kwargs):
-        return str(uuid.uuid4())
+        real_uuid = str(uuid.uuid4())
+        add_general_string_sanitizer(value=fake_uuid, target=real_uuid, function_scoped=True)
+        return real_uuid
 
     if "disable_mock_code_hash" not in request.keywords and is_live_and_not_recording():
         mocker.patch("azure.ai.ml._artifacts._artifact_utilities.get_object_hash", side_effect=generate_hash)
     elif not is_live():
         mocker.patch(
             "azure.ai.ml._artifacts._artifact_utilities.get_object_hash",
-            return_value="00000000000000000000000000000000",
+            return_value=fake_uuid,
         )
 
 
@@ -659,7 +663,7 @@ def mock_anon_component_version(mocker: MockFixture):
 
     def generate_name_version(*args, **kwargs):
         real_uuid = str(uuid.uuid4())
-        add_general_string_sanitizer(value=fake_uuid, target=real_uuid)
+        add_general_string_sanitizer(value=fake_uuid, target=real_uuid, function_scoped=True)
         return ANONYMOUS_COMPONENT_NAME, real_uuid
 
     def fake_name_version(*args, **kwargs):
@@ -683,7 +687,7 @@ def mock_asset_name(mocker: MockFixture):
 
     def generate_uuid(*args, **kwargs):
         real_uuid = str(uuid.uuid4())
-        add_general_string_sanitizer(value=fake_uuid, target=real_uuid)
+        add_general_string_sanitizer(value=fake_uuid, target=real_uuid, function_scoped=True)
         return real_uuid
 
     if is_live():
@@ -727,7 +731,7 @@ def generate_component_hash(*args, **kwargs):
     """Normalize component dict with sanitized value and return hash."""
     dict_hash = hash_dict(*args, **kwargs)
     normalized_dict_hash = normalized_hash_dict(*args, **kwargs)
-    add_general_string_sanitizer(value=normalized_dict_hash, target=dict_hash)
+    add_general_string_sanitizer(value=normalized_dict_hash, target=dict_hash, function_scoped=True)
     return dict_hash
 
 
@@ -837,7 +841,7 @@ def mock_job_name_generator(mocker: MockFixture):
 
     def generate_and_sanitize_job_name(*args, **kwargs):
         real_job_name = generate_job_name()
-        add_general_string_sanitizer(value=fake_job_name, target=real_job_name)
+        add_general_string_sanitizer(value=fake_job_name, target=real_job_name, function_scoped=True)
         return real_job_name
 
     if is_live():

sdk/ml/azure-ai-ml/tests/dsl/e2etests/test_dsl_pipeline.py

@@ -174,7 +174,7 @@ def pipeline(job_in_number, job_in_other_number, job_in_path):
         job = client.jobs.create_or_update(pipeline)
         # check required fields in job dict
         job_dict = job._to_dict()
-        expected_keys = ["status", "properties", "tags", "creation_context"]
+        expected_keys = ["status", "tags", "creation_context"]
         for k in expected_keys:
             assert k in job_dict.keys(), f"failed to get {k} in {job_dict}"
 

sdk/ml/azure-ai-ml/tests/environment/unittests/test_env_entity.py

@@ -127,8 +127,8 @@ def test_anonymous_environment_version_changes_with_inference_config(self):
 
         assert env_no_inference_config.name == env_no_inference_config.name == ANONYMOUS_ENV_NAME
         assert env_no_inference_config.version != env_with_inference_config.version
-        assert env_no_inference_config.version == "25484232c6e5bfb2c6e1dc5d38fb9fa5"
-        assert env_with_inference_config.version == "b181e0fef2ad76757e2f78317c10cb2b"
+        assert env_no_inference_config.version == "09c18f036e80ca941e58b12313012d8e6f23cdc3ccc35b8c97f890ccbaedbf40"
+        assert env_with_inference_config.version == "e060479364110e178fa0338de64bcd5cdf2dc186ee80b89641d842fc974c6949"
 
     def test_ipp_environment(self) -> None:
         # test through deserializing REST instead of using real IP assets