adding warning if pat detects in git url while creation of job (#40337)

* adding warning if pat detects in git url while creation of job

* adding test cases

* applying black formatter

Author: jayesh-tanna

Diff for: sdk/ml/azure-ai-ml/CHANGELOG.md

@@ -4,6 +4,7 @@
 
 ### Bugs Fixed
  - Fix for compute Instance, disableLocalAuth property should be depend on ssh public access enabled.
+ - Removing Git-related properties from job properties if a PAT token is detected in the repository URL.
 
 ## 1.26.1 (2025-03-27)
 

Diff for: sdk/ml/azure-ai-ml/azure/ai/ml/operations/_job_operations.py

@@ -65,6 +65,7 @@
     WorkspaceDiscoveryUrlKey,
 )
 from azure.ai.ml.constants._compute import ComputeType
+from azure.ai.ml.constants._common import GitProperties
 from azure.ai.ml.constants._job.pipeline import PipelineConstants
 from azure.ai.ml.entities import Compute, Job, PipelineJob, ServiceInstance, ValidationResult
 from azure.ai.ml.entities._assets._artifacts.code import Code
@@ -105,7 +106,12 @@
 from ._component_operations import ComponentOperations
 from ._compute_operations import ComputeOperations
 from ._dataset_dataplane_operations import DatasetDataplaneOperations
-from ._job_ops_helper import get_git_properties, get_job_output_uris_from_dataplane, stream_logs_until_completion
+from ._job_ops_helper import (
+    get_git_properties,
+    get_job_output_uris_from_dataplane,
+    has_pat_token,
+    stream_logs_until_completion,
+)
 from ._local_job_invoker import is_local_run, start_run_if_local
 from ._model_dataplane_operations import ModelDataplaneOperations
 from ._operation_orchestrator import (
@@ -681,7 +687,18 @@ def create_or_update(
         # their job, the request will fail since the git props will be repopulated.
         # MFE does not allow existing properties to be updated, only for new props to be added
         if not any(prop_name in job.properties for prop_name in git_props):
-            job.properties = {**job.properties, **git_props}
+            repo_url = git_props.get(GitProperties.PROP_MLFLOW_GIT_REPO_URL)
+
+            if has_pat_token(repo_url):
+                git_props.pop(GitProperties.PROP_MLFLOW_GIT_REPO_URL)
+                git_props.pop(GitProperties.PROP_MLFLOW_GIT_BRANCH)
+                git_props.pop(GitProperties.PROP_MLFLOW_GIT_COMMIT)
+                git_props.pop(GitProperties.PROP_DIRTY)
+                module_logger.warning("Git properties are removed because the repository URL contains a secret.")
+
+            if git_props:
+                job.properties = {**job.properties, **git_props}
+
         rest_job_resource = to_rest_job_object(job)
 
         # Make a copy of self._kwargs instead of contaminate the original one

Diff for: sdk/ml/azure-ai-ml/azure/ai/ml/operations/_job_ops_helper.py

@@ -435,6 +435,22 @@ def _run_git_cmd(args: Iterable[str]) -> Optional[str]:
     return properties
 
 
+def has_pat_token(url: Optional[str]) -> bool:
+    """Check if the given repository URL contains a PAT token.
+
+    :param url: Repository URL to check.
+    :type url: Optional[str]
+    :return: True if PAT token is detected, False otherwise.
+    :rtype: bool
+    """
+    if url is None:
+        return False
+
+    # Matches both "https://dev.azure.com/mypattoken@..." and "https://[email protected]/..."
+    pat_regex = r"https://(?:[^/@]+/)?([^/@]+)@"
+    return re.search(pat_regex, url) is not None
+
+
 def get_job_output_uris_from_dataplane(
     job_name: Optional[str],
     run_operations: RunOperations,

Diff for: sdk/ml/azure-ai-ml/tests/job_common/unittests/test_job_operations.py

@@ -1,36 +1,25 @@
 import json
 import os
-from typing import Callable
 from unittest.mock import Mock, patch
 
 import jwt
 import pytest
-import yaml
-from msrest import Deserializer
 from pytest_mock import MockFixture
 
-from azure.ai.ml import MLClient, load_job
+from azure.ai.ml import load_job
 from azure.ai.ml._azure_environments import _get_aml_resource_id_from_metadata, _resource_to_scopes
 from azure.ai.ml._restclient.v2023_04_01_preview import models
 from azure.ai.ml._scope_dependent_operations import OperationConfig, OperationScope
-from azure.ai.ml.constants._common import AZUREML_PRIVATE_FEATURES_ENV_VAR, AzureMLResourceType
+from azure.ai.ml.constants._common import AZUREML_PRIVATE_FEATURES_ENV_VAR, AzureMLResourceType, GitProperties
 from azure.ai.ml.entities._builders import Command
-from azure.ai.ml.entities._job.automl.automl_job import AutoMLJob
-from azure.ai.ml.entities._job.automl.training_settings import TrainingSettings
 from azure.ai.ml.entities._job.job import Job
-from azure.ai.ml.entities._job.sweep.sweep_job import SweepJob
-from azure.ai.ml.exceptions import ValidationException
 from azure.ai.ml.operations import DatastoreOperations, EnvironmentOperations, JobOperations, WorkspaceOperations
 from azure.ai.ml.operations._code_operations import CodeOperations
 from azure.ai.ml.operations._job_ops_helper import get_git_properties
-from azure.ai.ml.operations._run_history_constants import RunHistoryConstants
 from azure.ai.ml.operations._run_operations import RunOperations
 from azure.core.credentials import AccessToken
-from azure.core.exceptions import HttpResponseError
 from azure.identity import DefaultAzureCredential
 
-from .test_vcr_utils import before_record_cb, vcr_header_filters
-
 
 @pytest.fixture
 def mock_datastore_operation(
@@ -298,6 +287,64 @@ def test_job_create_skip_validation(self, mock_method, mock_job_operation: JobOp
             mock_job_operation.create_or_update(job=job)
             mock_thing.assert_called_once()
 
+    @patch("azure.ai.ml.operations._job_operations.get_git_properties")
+    @patch.object(Job, "_from_rest_object")
+    def test_create_or_update_removes_git_props_if_pat_in_repo_url(
+        self, mock_method, mock_get_git_properties, mock_job_operation: JobOperations
+    ) -> None:
+        mock_method.return_value = Command(component=None)
+
+        mock_get_git_properties.return_value = {
+            GitProperties.PROP_MLFLOW_GIT_REPO_URL: "https://example@mock-repo-url",
+            GitProperties.PROP_MLFLOW_GIT_BRANCH: "mock-branch",
+            GitProperties.PROP_MLFLOW_GIT_COMMIT: "mock-commit",
+            GitProperties.PROP_DIRTY: "True",
+        }
+
+        job = load_job("./tests/test_configs/command_job/simple_train_test.yml")
+        with patch.object(JobOperations, "_validate") as mock_thing, patch.object(
+            JobOperations, "_resolve_arm_id_or_upload_dependencies"
+        ):
+            mock_job_operation.create_or_update(job=job)
+            mock_get_git_properties.assert_called_once()
+            assert (
+                GitProperties.PROP_MLFLOW_GIT_REPO_URL not in job.properties
+            ), "repoURL key should not exist in job.properties"
+            assert (
+                GitProperties.PROP_MLFLOW_GIT_BRANCH not in job.properties
+            ), "branch key should not exist in job.properties"
+            assert (
+                GitProperties.PROP_MLFLOW_GIT_COMMIT not in job.properties
+            ), "commit key should not exist in job.properties"
+            assert GitProperties.PROP_DIRTY not in job.properties, "dirty key should not exist in job.properties"
+
+    @patch("azure.ai.ml.operations._job_operations.get_git_properties")
+    @patch.object(Job, "_from_rest_object")
+    def test_create_or_update_includes_git_props_if_no_pat_in_repo_url(
+        self, mock_method, mock_get_git_properties, mock_job_operation: JobOperations
+    ) -> None:
+        mock_method.return_value = Command(component=None)
+
+        mock_get_git_properties.return_value = {
+            GitProperties.PROP_MLFLOW_GIT_REPO_URL: "https://mock-repo-url",
+            GitProperties.PROP_MLFLOW_GIT_BRANCH: "mock-branch",
+            GitProperties.PROP_MLFLOW_GIT_COMMIT: "mock-commit",
+            GitProperties.PROP_DIRTY: "True",
+        }
+
+        job = load_job("./tests/test_configs/command_job/simple_train_test.yml")
+        with patch.object(JobOperations, "_validate") as mock_thing, patch.object(
+            JobOperations, "_resolve_arm_id_or_upload_dependencies"
+        ):
+            mock_job_operation.create_or_update(job=job)
+            mock_get_git_properties.assert_called_once()
+            assert (
+                GitProperties.PROP_MLFLOW_GIT_REPO_URL in job.properties
+            ), "repoURL key should exist in job.properties"
+            assert GitProperties.PROP_MLFLOW_GIT_BRANCH in job.properties, "branch key should exist in job.properties"
+            assert GitProperties.PROP_MLFLOW_GIT_COMMIT in job.properties, "commit key should exist in job.properties"
+            assert GitProperties.PROP_DIRTY in job.properties, "dirty key should exist in job.properties"
+
     def test_download_with_none(self, mock_job_operation: JobOperations) -> None:
         with pytest.raises(Exception) as ex:
             mock_job_operation.download(None)

Diff for: sdk/ml/azure-ai-ml/tests/job_common/unittests/test_job_ops_helper.py

@@ -12,6 +12,7 @@
 from azure.ai.ml._scope_dependent_operations import OperationScope
 from azure.ai.ml.operations._job_ops_helper import (
     _get_sorted_filtered_logs,
+    has_pat_token,
     _incremental_print,
     list_logs,
     stream_logs_until_completion,
@@ -64,6 +65,16 @@ def mock_run_operations(mock_workspace_scope: OperationScope, mock_aml_services_
     yield RunOperations(mock_workspace_scope, mock_aml_services_run_history)
 
 
+@pytest.mark.unittest
+@pytest.mark.training_experiences_test
+class TestJobOpsHelper:
+    def test_has_pat_token(self) -> None:
+        assert has_pat_token("https://dev.azure.com/mypattoken@company_machineName/project-name/_git/repo_name")
+        assert has_pat_token("https://[email protected]/<organization>/<project>/_git/<repo>")
+        assert not has_pat_token("https://dev.azure.com/organization/project/_apis/pipelines/1/runs")
+        assert not has_pat_token("https://learn.microsoft.com/en-us/ai/?tabs=developer")
+
+
 @pytest.mark.skip("TODO 1907352: Relies on a missing VCR.py recording + test suite needs to be reworked")
 @pytest.mark.unittest
 @pytest.mark.training_experiences_test