From 8925c7086c8c4c5c8b9fb7f495fd334898bc65f5 Mon Sep 17 00:00:00 2001 From: Julfried Date: Tue, 17 Dec 2024 14:41:34 +0100 Subject: [PATCH 1/8] fix: resolve Windows path handling in _find_config * Replace Path.match("/") with Path.anchor comparison * Fix infinite loop in _studio.py path traversal --- src/sagemaker/_studio.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/sagemaker/_studio.py b/src/sagemaker/_studio.py index a23fae87e9..6ea5c4c875 100644 --- a/src/sagemaker/_studio.py +++ b/src/sagemaker/_studio.py @@ -65,7 +65,9 @@ def _find_config(working_dir=None): wd = Path(working_dir) if working_dir else Path.cwd() path = None - while path is None and not wd.match("/"): + + root = Path(wd.anchor) # Properly get the root of the current working directory for both Windows and Unix-like systems + while path is None and wd != root: candidate = wd / STUDIO_PROJECT_CONFIG if Path.exists(candidate): path = candidate From 62a489eade891725265c8c9990d46d27cea12e79 Mon Sep 17 00:00:00 2001 From: Julfried Date: Tue, 17 Dec 2024 17:00:05 +0100 Subject: [PATCH 2/8] test: Add tests for the new root path exploration --- tests/unit/sagemaker/test_studio.py | 59 ++++++++++++++++++++++++++++- 1 file changed, 58 insertions(+), 1 deletion(-) diff --git a/tests/unit/sagemaker/test_studio.py b/tests/unit/sagemaker/test_studio.py index 47528e1f36..bc57e4917f 100644 --- a/tests/unit/sagemaker/test_studio.py +++ b/tests/unit/sagemaker/test_studio.py @@ -12,7 +12,8 @@ # language governing permissions and limitations under the License. # language governing permissions and limitations under the License. from __future__ import absolute_import - +import os +from pathlib import Path from sagemaker._studio import ( _append_project_tags, _find_config, @@ -20,6 +21,62 @@ _parse_tags, ) +def test_find_config_cross_platform(tmpdir): + """Test _find_config works correctly across different platforms.""" + # Create a completely separate directory for isolated tests + import tempfile + with tempfile.TemporaryDirectory() as isolated_root: + # Setup test directory structure for positive tests + config = tmpdir.join(".sagemaker-code-config") + config.write('{"sagemakerProjectId": "proj-1234"}') + + # Test 1: Direct parent directory + working_dir = tmpdir.mkdir("sub") + found_path = _find_config(working_dir) + assert found_path == config + + # Test 2: Deeply nested directories + nested_dir = tmpdir.mkdir("deep").mkdir("nested").mkdir("path") + found_path = _find_config(nested_dir) + assert found_path == config + + # Test 3: Start from root directory + import os + root_dir = os.path.abspath(os.sep) + found_path = _find_config(root_dir) + assert found_path is None + + # Test 4: No config file in path - using truly isolated directory + isolated_path = Path(isolated_root) / "nested" / "path" + isolated_path.mkdir(parents=True) + found_path = _find_config(isolated_path) + assert found_path is None + +def test_find_config_path_separators(tmpdir): + """Test _find_config handles different path separator styles. + + Tests: + 1. Forward slashes + 2. Backslashes + 3. Mixed separators + """ + # Setup + config = tmpdir.join(".sagemaker-code-config") + config.write('{"sagemakerProjectId": "proj-1234"}') + base_path = str(tmpdir) + + # Test different path separator styles + paths = [ + os.path.join(base_path, "dir1", "dir2"), # OS native + "/".join([base_path, "dir1", "dir2"]), # Forward slashes + "\\".join([base_path, "dir1", "dir2"]), # Backslashes + base_path + "/dir1\\dir2" # Mixed + ] + + for path in paths: + os.makedirs(path, exist_ok=True) + found_path = _find_config(path) + assert found_path == config def test_find_config(tmpdir): path = tmpdir.join(".sagemaker-code-config") From efbdc135c76b0516e943d9cef31f455e2a1380d1 Mon Sep 17 00:00:00 2001 From: Julfried <51880314+Julfried@users.noreply.github.com> Date: Sat, 1 Feb 2025 10:54:53 +0100 Subject: [PATCH 3/8] Fix formatting style --- src/sagemaker/_studio.py | 6 ++++-- tests/unit/sagemaker/test_studio.py | 25 +++++++++++++++---------- 2 files changed, 19 insertions(+), 12 deletions(-) diff --git a/src/sagemaker/_studio.py b/src/sagemaker/_studio.py index 6ea5c4c875..ce73de3179 100644 --- a/src/sagemaker/_studio.py +++ b/src/sagemaker/_studio.py @@ -65,8 +65,10 @@ def _find_config(working_dir=None): wd = Path(working_dir) if working_dir else Path.cwd() path = None - - root = Path(wd.anchor) # Properly get the root of the current working directory for both Windows and Unix-like systems + + root = Path( + wd.anchor + ) # Properly get the root of the current working directory for both Windows and Unix-like systems while path is None and wd != root: candidate = wd / STUDIO_PROJECT_CONFIG if Path.exists(candidate): diff --git a/tests/unit/sagemaker/test_studio.py b/tests/unit/sagemaker/test_studio.py index bc57e4917f..6a09e590a3 100644 --- a/tests/unit/sagemaker/test_studio.py +++ b/tests/unit/sagemaker/test_studio.py @@ -21,40 +21,44 @@ _parse_tags, ) + def test_find_config_cross_platform(tmpdir): """Test _find_config works correctly across different platforms.""" # Create a completely separate directory for isolated tests import tempfile + with tempfile.TemporaryDirectory() as isolated_root: # Setup test directory structure for positive tests config = tmpdir.join(".sagemaker-code-config") config.write('{"sagemakerProjectId": "proj-1234"}') - + # Test 1: Direct parent directory working_dir = tmpdir.mkdir("sub") found_path = _find_config(working_dir) assert found_path == config - + # Test 2: Deeply nested directories nested_dir = tmpdir.mkdir("deep").mkdir("nested").mkdir("path") found_path = _find_config(nested_dir) assert found_path == config - + # Test 3: Start from root directory import os + root_dir = os.path.abspath(os.sep) found_path = _find_config(root_dir) assert found_path is None - + # Test 4: No config file in path - using truly isolated directory isolated_path = Path(isolated_root) / "nested" / "path" isolated_path.mkdir(parents=True) found_path = _find_config(isolated_path) assert found_path is None + def test_find_config_path_separators(tmpdir): """Test _find_config handles different path separator styles. - + Tests: 1. Forward slashes 2. Backslashes @@ -64,20 +68,21 @@ def test_find_config_path_separators(tmpdir): config = tmpdir.join(".sagemaker-code-config") config.write('{"sagemakerProjectId": "proj-1234"}') base_path = str(tmpdir) - + # Test different path separator styles paths = [ os.path.join(base_path, "dir1", "dir2"), # OS native - "/".join([base_path, "dir1", "dir2"]), # Forward slashes - "\\".join([base_path, "dir1", "dir2"]), # Backslashes - base_path + "/dir1\\dir2" # Mixed + "/".join([base_path, "dir1", "dir2"]), # Forward slashes + "\\".join([base_path, "dir1", "dir2"]), # Backslashes + base_path + "/dir1\\dir2", # Mixed ] - + for path in paths: os.makedirs(path, exist_ok=True) found_path = _find_config(path) assert found_path == config + def test_find_config(tmpdir): path = tmpdir.join(".sagemaker-code-config") path.write('{"sagemakerProjectId": "proj-1234"}') From 6fc188cedde112234cfff9c1f796f2bd108a1093 Mon Sep 17 00:00:00 2001 From: Julfried Date: Tue, 4 Mar 2025 08:28:46 +0100 Subject: [PATCH 4/8] Fixed line to long --- src/sagemaker/_studio.py | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/src/sagemaker/_studio.py b/src/sagemaker/_studio.py index ce73de3179..99f543e3dd 100644 --- a/src/sagemaker/_studio.py +++ b/src/sagemaker/_studio.py @@ -65,10 +65,9 @@ def _find_config(working_dir=None): wd = Path(working_dir) if working_dir else Path.cwd() path = None - - root = Path( - wd.anchor - ) # Properly get the root of the current working directory for both Windows and Unix-like systems + + # Get the root of the current working directory for both Windows and Unix-like systems + root = Path(wd.anchor) while path is None and wd != root: candidate = wd / STUDIO_PROJECT_CONFIG if Path.exists(candidate): From ebaf50792d7370412d770fd5457b963932889ee8 Mon Sep 17 00:00:00 2001 From: Julfried Date: Wed, 5 Mar 2025 09:01:11 +0100 Subject: [PATCH 5/8] Fix docstyle by running black manually --- src/sagemaker/_studio.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/sagemaker/_studio.py b/src/sagemaker/_studio.py index 99f543e3dd..22f1c94c5f 100644 --- a/src/sagemaker/_studio.py +++ b/src/sagemaker/_studio.py @@ -65,7 +65,7 @@ def _find_config(working_dir=None): wd = Path(working_dir) if working_dir else Path.cwd() path = None - + # Get the root of the current working directory for both Windows and Unix-like systems root = Path(wd.anchor) while path is None and wd != root: From 9079baa91c56a3e8657f39488718b667d93f1742 Mon Sep 17 00:00:00 2001 From: Julfried Date: Thu, 6 Mar 2025 22:05:42 +0100 Subject: [PATCH 6/8] Fix testcase with \\ when running on non-windows machines --- tests/unit/sagemaker/test_studio.py | 19 +++++++++++-------- 1 file changed, 11 insertions(+), 8 deletions(-) diff --git a/tests/unit/sagemaker/test_studio.py b/tests/unit/sagemaker/test_studio.py index 6a09e590a3..6e4eb88b96 100644 --- a/tests/unit/sagemaker/test_studio.py +++ b/tests/unit/sagemaker/test_studio.py @@ -13,6 +13,7 @@ # language governing permissions and limitations under the License. from __future__ import absolute_import import os +import platform from pathlib import Path from sagemaker._studio import ( _append_project_tags, @@ -69,20 +70,22 @@ def test_find_config_path_separators(tmpdir): config.write('{"sagemakerProjectId": "proj-1234"}') base_path = str(tmpdir) - # Test different path separator styles - paths = [ - os.path.join(base_path, "dir1", "dir2"), # OS native - "/".join([base_path, "dir1", "dir2"]), # Forward slashes - "\\".join([base_path, "dir1", "dir2"]), # Backslashes - base_path + "/dir1\\dir2", # Mixed - ] + # Always include the OS native path and forward slashes (which are equivalent on all OS) + paths = [os.path.join(base_path, "dir1", "dir2"), + "/".join([base_path, "dir1", "dir2"])] + + # Only on Windows add the backslashes and mixed separator test cases. + if os.name == "nt": + paths.extend([ + "\\".join([base_path, "dir1", "dir2"]), + base_path + "/dir1\\dir2", + ]) for path in paths: os.makedirs(path, exist_ok=True) found_path = _find_config(path) assert found_path == config - def test_find_config(tmpdir): path = tmpdir.join(".sagemaker-code-config") path.write('{"sagemakerProjectId": "proj-1234"}') From 74d82cd952e2ae899f51c57005e5c1082bedcbfa Mon Sep 17 00:00:00 2001 From: Julfried Date: Thu, 6 Mar 2025 22:11:48 +0100 Subject: [PATCH 7/8] Fix formatting style --- tests/unit/sagemaker/test_studio.py | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/tests/unit/sagemaker/test_studio.py b/tests/unit/sagemaker/test_studio.py index 6e4eb88b96..be08d03b53 100644 --- a/tests/unit/sagemaker/test_studio.py +++ b/tests/unit/sagemaker/test_studio.py @@ -71,21 +71,18 @@ def test_find_config_path_separators(tmpdir): base_path = str(tmpdir) # Always include the OS native path and forward slashes (which are equivalent on all OS) - paths = [os.path.join(base_path, "dir1", "dir2"), - "/".join([base_path, "dir1", "dir2"])] + paths = [os.path.join(base_path, "dir1", "dir2"), "/".join([base_path, "dir1", "dir2"])] # Only on Windows add the backslashes and mixed separator test cases. if os.name == "nt": - paths.extend([ - "\\".join([base_path, "dir1", "dir2"]), - base_path + "/dir1\\dir2", - ]) + paths.extend(["\\".join([base_path, "dir1", "dir2"]), base_path + "/dir1\\dir2"]) for path in paths: os.makedirs(path, exist_ok=True) found_path = _find_config(path) assert found_path == config + def test_find_config(tmpdir): path = tmpdir.join(".sagemaker-code-config") path.write('{"sagemakerProjectId": "proj-1234"}') From 24952326b96e46f6d206e249841fbdc94613801c Mon Sep 17 00:00:00 2001 From: Julfried Date: Thu, 6 Mar 2025 22:12:25 +0100 Subject: [PATCH 8/8] cleanup unused import --- tests/unit/sagemaker/test_studio.py | 1 - 1 file changed, 1 deletion(-) diff --git a/tests/unit/sagemaker/test_studio.py b/tests/unit/sagemaker/test_studio.py index be08d03b53..81302894ab 100644 --- a/tests/unit/sagemaker/test_studio.py +++ b/tests/unit/sagemaker/test_studio.py @@ -13,7 +13,6 @@ # language governing permissions and limitations under the License. from __future__ import absolute_import import os -import platform from pathlib import Path from sagemaker._studio import ( _append_project_tags,