fix: align recipe git clone error handling with git_utils

sagemaker-core/src/sagemaker/core/modules/utils.py

@@ -23,6 +23,7 @@
 from datetime import datetime
 from typing import Literal, Any
 
+from sagemaker.core.git_utils import _redact_credentials_from_url
 from sagemaker.core.shapes import Unassigned
 from sagemaker.core.modules import logger
 
@@ -170,9 +171,16 @@ def _run_clone_command_silent(repo_url, dest_dir):
                 stderr=subprocess.DEVNULL,
             )
         except subprocess.CalledProcessError as e:
-            logger.error(f"Failed to clone repository: {repo_url}")
-            logger.error(f"Error output:\n{e}")
-            raise
+            safe_url = _redact_credentials_from_url(repo_url)
+            error = subprocess.CalledProcessError(
+                e.returncode,
+                ["git", "clone", safe_url, dest_dir],
+                output=e.output,
+                stderr=e.stderr,
+            )
+            logger.error(f"Failed to clone repository: {safe_url}")
+            logger.error(f"Error output:\n{error}")
+            raise error from None
     elif repo_url.startswith("git@") or repo_url.startswith("ssh://"):
         try:
             with tempfile.TemporaryDirectory() as tmp_dir:

sagemaker-core/tests/unit/modules/test_utils_clone.py

@@ -0,0 +1,74 @@
+# Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved.
+#
+# Licensed under the Apache License, Version 2.0 (the "License"). You
+# may not use this file except in compliance with the License. A copy of
+# the License is located at
+#
+#     http://aws.amazon.com/apache2.0/
+#
+# or in the "license" file accompanying this file. This file is
+# distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF
+# ANY KIND, either express or implied. See the License for the specific
+# language governing permissions and limitations under the License.
+"""Unit tests for _run_clone_command_silent in sagemaker.core.modules.utils."""
+
+import logging
+import subprocess
+from unittest.mock import patch
+
+import pytest
+
+from sagemaker.core.modules.utils import _run_clone_command_silent
+
+MODULE = "sagemaker.core.modules.utils"
+DEST = "/tmp/dest"
+
+
+@pytest.mark.parametrize(
+    "repo_url,userinfo",
+    [
+        ("https://oauth2:abc123@example.com/org/repo.git", "oauth2:abc123"),
+        ("https://user:p%40ss@example.com/org/repo.git", "user:p%40ss"),
+    ],
+)
+def test_clone_failure_uses_sanitized_url(repo_url, userinfo, caplog):
+    with patch(
+        f"{MODULE}.subprocess.check_call",
+        side_effect=subprocess.CalledProcessError(128, ["git", "clone", repo_url, DEST]),
+    ):
+        with caplog.at_level(logging.ERROR):
+            with pytest.raises(subprocess.CalledProcessError) as exc_info:
+                _run_clone_command_silent(repo_url, DEST)
+
+    error = exc_info.value
+    expected_url = "https://<credentials-redacted>@example.com/org/repo.git"
+    assert error.returncode == 128
+    assert error.cmd == ["git", "clone", expected_url, DEST]
+    assert error.__suppress_context__
+    assert userinfo not in str(error)
+    assert userinfo not in caplog.text
+    assert expected_url in caplog.text
+
+
+def test_clone_failure_without_userinfo_keeps_url(caplog):
+    repo_url = "https://example.com/org/repo.git"
+    with patch(
+        f"{MODULE}.subprocess.check_call",
+        side_effect=subprocess.CalledProcessError(128, ["git", "clone", repo_url, DEST]),
+    ):
+        with caplog.at_level(logging.ERROR):
+            with pytest.raises(subprocess.CalledProcessError) as exc_info:
+                _run_clone_command_silent(repo_url, DEST)
+
+    assert exc_info.value.cmd == ["git", "clone", repo_url, DEST]
+    assert repo_url in caplog.text
+
+
+def test_clone_success_uses_original_url():
+    repo_url = "https://oauth2:abc123@example.com/org/repo.git"
+    with patch(f"{MODULE}.subprocess.check_call") as mock_check_call:
+        _run_clone_command_silent(repo_url, DEST)
+
+    args, kwargs = mock_check_call.call_args
+    assert args[0] == ["git", "clone", repo_url, DEST]
+    assert kwargs["env"]["GIT_TERMINAL_PROMPT"] == "0"

sagemaker-train/src/sagemaker/train/utils.py

@@ -24,6 +24,7 @@
 from datetime import datetime
 from typing import Literal, Any
 
+from sagemaker.core.git_utils import _redact_credentials_from_url
 from sagemaker.core.helper.session_helper import Session
 from sagemaker.core.shapes import Unassigned
 from sagemaker.core.utils.utils import logger
@@ -217,9 +218,16 @@ def _run_clone_command_silent(repo_url, dest_dir):
                 stderr=subprocess.DEVNULL,
             )
         except subprocess.CalledProcessError as e:
-            logger.error(f"Failed to clone repository: {repo_url}")
-            logger.error(f"Error output:\n{e}")
-            raise
+            safe_url = _redact_credentials_from_url(repo_url)
+            error = subprocess.CalledProcessError(
+                e.returncode,
+                ["git", "clone", safe_url, dest_dir],
+                output=e.output,
+                stderr=e.stderr,
+            )
+            logger.error(f"Failed to clone repository: {safe_url}")
+            logger.error(f"Error output:\n{error}")
+            raise error from None
     elif repo_url.startswith("git@") or repo_url.startswith("ssh://"):
         try:
             with tempfile.TemporaryDirectory() as tmp_dir:

sagemaker-train/tests/unit/train/test_utils_clone.py

@@ -0,0 +1,74 @@
+# Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved.
+#
+# Licensed under the Apache License, Version 2.0 (the "License"). You
+# may not use this file except in compliance with the License. A copy of
+# the License is located at
+#
+#     http://aws.amazon.com/apache2.0/
+#
+# or in the "license" file accompanying this file. This file is
+# distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF
+# ANY KIND, either express or implied. See the License for the specific
+# language governing permissions and limitations under the License.
+"""Unit tests for _run_clone_command_silent in sagemaker.train.utils."""
+
+import logging
+import subprocess
+from unittest.mock import patch
+
+import pytest
+
+from sagemaker.train.utils import _run_clone_command_silent
+
+MODULE = "sagemaker.train.utils"
+DEST = "/tmp/dest"
+
+
+@pytest.mark.parametrize(
+    "repo_url,userinfo",
+    [
+        ("https://oauth2:abc123@example.com/org/repo.git", "oauth2:abc123"),
+        ("https://user:p%40ss@example.com/org/repo.git", "user:p%40ss"),
+    ],
+)
+def test_clone_failure_uses_sanitized_url(repo_url, userinfo, caplog):
+    with patch(
+        f"{MODULE}.subprocess.check_call",
+        side_effect=subprocess.CalledProcessError(128, ["git", "clone", repo_url, DEST]),
+    ):
+        with caplog.at_level(logging.ERROR):
+            with pytest.raises(subprocess.CalledProcessError) as exc_info:
+                _run_clone_command_silent(repo_url, DEST)
+
+    error = exc_info.value
+    expected_url = "https://<credentials-redacted>@example.com/org/repo.git"
+    assert error.returncode == 128
+    assert error.cmd == ["git", "clone", expected_url, DEST]
+    assert error.__suppress_context__
+    assert userinfo not in str(error)
+    assert userinfo not in caplog.text
+    assert expected_url in caplog.text
+
+
+def test_clone_failure_without_userinfo_keeps_url(caplog):
+    repo_url = "https://example.com/org/repo.git"
+    with patch(
+        f"{MODULE}.subprocess.check_call",
+        side_effect=subprocess.CalledProcessError(128, ["git", "clone", repo_url, DEST]),
+    ):
+        with caplog.at_level(logging.ERROR):
+            with pytest.raises(subprocess.CalledProcessError) as exc_info:
+                _run_clone_command_silent(repo_url, DEST)
+
+    assert exc_info.value.cmd == ["git", "clone", repo_url, DEST]
+    assert repo_url in caplog.text
+
+
+def test_clone_success_uses_original_url():
+    repo_url = "https://oauth2:abc123@example.com/org/repo.git"
+    with patch(f"{MODULE}.subprocess.check_call") as mock_check_call:
+        _run_clone_command_silent(repo_url, DEST)
+
+    args, kwargs = mock_check_call.call_args
+    assert args[0] == ["git", "clone", repo_url, DEST]
+    assert kwargs["env"]["GIT_TERMINAL_PROMPT"] == "0"