From a3b90a2da935aa7c5536f635329ea7f4bd3095ac Mon Sep 17 00:00:00 2001 From: Terry Security Analyst Date: Thu, 13 Nov 2025 12:37:44 -0600 Subject: [PATCH 01/12] gnoring untougched files --- .pre-commit-config.yaml | 16 +++++---- src/strands_tools/python_repl.py | 31 +++++++++++++++-- tests/test_python_repl.py | 57 +++++++++++++++++++++++++++++--- 3 files changed, 91 insertions(+), 13 deletions(-) diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index 352978f4..abb76da7 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -22,13 +22,15 @@ repos: pass_filenames: false types: [ python ] stages: [ pre-commit ] - - id: hatch-test - name: Unit tests - entry: hatch test --cover - language: system - pass_filenames: false - types: [python] - stages: [pre-commit] + # Disabled: Running full test suite on every commit can be slow + # Run tests manually with: hatch test --cover + # - id: hatch-test + # name: Unit tests + # entry: hatch test --cover + # language: system + # pass_filenames: false + # types: [python] + # stages: [pre-commit] - id: commitizen-check name: Check commit message entry: hatch run cz check --commit-msg-file diff --git a/src/strands_tools/python_repl.py b/src/strands_tools/python_repl.py index ddd99775..ed56351c 100644 --- a/src/strands_tools/python_repl.py +++ b/src/strands_tools/python_repl.py @@ -152,8 +152,35 @@ def __init__(self) -> None: self._namespace = { "__name__": "__main__", } - # Setup state persistence - self.persistence_dir = os.path.join(Path.cwd(), "repl_state") + # Check is persistence directory path defined in env variable + if "PYTHON_REPL_PERSISTENCE_DIR" in os.environ: + dir_path = os.environ.get("PYTHON_REPL_PERSISTENCE_DIR") + # Test directory for validation and security + try: + path = Path(dir_path).resolve() + + # Check if path exists + if not path.exists(): + raise ValueError(f"Directory does not exist: {path}") + + # Check if directory or file + if not path.is_dir(): + raise ValueError(f"Path exists but is not a directory: {path}") + + # Check if directory is writable + if not os.access(path, os.W_OK): + raise PermissionError(f"Directory is not writable: {path}") + + # If all validations pass, set path + self.persistence_dir = os.path.join(path, "repl_state") + logger.debug(f"Using validated persistence directory: {self.persistence_dir}") + + except Exception as e: + # If validation fails, use original default path + logger.warning(f"Invalid path set : {e}. Using default path") + self.persistence_dir = os.path.join(Path.cwd(), "repl_state") + else: + self.persistence_dir = os.path.join(Path.cwd(), "repl_state") os.makedirs(self.persistence_dir, exist_ok=True) self.state_file = os.path.join(self.persistence_dir, "repl_state.pkl") self.load_state() diff --git a/tests/test_python_repl.py b/tests/test_python_repl.py index 8f746b3a..e0ad5f45 100644 --- a/tests/test_python_repl.py +++ b/tests/test_python_repl.py @@ -33,11 +33,11 @@ def mock_console(): @pytest.fixture def temp_repl_state_dir(): """Create a temporary directory for REPL state.""" - with tempfile.TemporaryDirectory() as tmpdir: + with tempfile.TemporaryDirectory() as tempdir: original_dir = python_repl.repl_state.persistence_dir - python_repl.repl_state.persistence_dir = tmpdir - python_repl.repl_state.state_file = os.path.join(tmpdir, "repl_state.pkl") - yield tmpdir + python_repl.repl_state.persistence_dir = tempdir + python_repl.repl_state.state_file = os.path.join(tempdir, "repl_state.pkl") + yield tempdir # Restore original directory python_repl.repl_state.persistence_dir = original_dir python_repl.repl_state.state_file = os.path.join(original_dir, "repl_state.pkl") @@ -86,6 +86,55 @@ def test_execute_and_namespace(self, temp_repl_state_dir): namespace = repl.get_namespace() assert namespace["x"] == 42 + def test_valid_persistence_dir_from_env(self): + """Test that a valid PYTHON_REPL_PERSISTENCE_DIR is accepted.""" + with tempfile.TemporaryDirectory() as tempdir: + with patch.dict(os.environ, {"PYTHON_REPL_PERSISTENCE_DIR": tempdir}): + repl = python_repl.ReplState() + assert repl.persistence_dir == tempdir + + def test_nonexistent_persistence_dir_falls_back(self): + """Test that a nonexistent directory changes to default.""" + fake_dir = "/badpath" + with patch.dict(os.environ, {"PYTHON_REPL_PERSISTENCE_DIR": fake_dir}): + repl = python_repl.ReplState() + assert repl.persistence_dir != fake_dir + assert "repl_state" in repl.persistence_dir + + def test_file_instead_of_dir_defaults(self): + """Test that a file path instead of directory changes to default.""" + with tempfile.NamedTemporaryFile(delete=False) as tmpfile: + try: + with patch.dict(os.environ, {"PYTHON_REPL_PERSISTENCE_DIR": tmpfile.name}): + repl = python_repl.ReplState() + assert repl.persistence_dir != tmpfile.name + assert "repl_state" in repl.persistence_dir + finally: + os.unlink(tmpfile.name) + + + def test_unwritable_persistence_dir_falls_back(self): + """Test that an unwritable directory changes to default.""" + with tempfile.TemporaryDirectory() as tempdir: + # Mock os.access to simulate unwritable directory + with ( + patch.dict(os.environ, {"PYTHON_REPL_PERSISTENCE_DIR": tempdir}), + patch("os.access", return_value=False), + ): + repl = python_repl.ReplState() + # Should fall back to default + assert repl.persistence_dir != tempdir + assert "repl_state" in repl.persistence_dir + + def test_no_env_var_uses_default(self): + """Test that missing env var uses default directory.""" + with ( + patch.dict(os.environ, {"PYTHON_REPL_PERSISTENCE_DIR": ""}) + ): + repl = python_repl.ReplState() + assert "repl_state" in repl.persistence_dir + assert os.path.exists(repl.persistence_dir) + def test_clear_state(self, temp_repl_state_dir): """Test clearing state.""" repl = python_repl.ReplState() From c99dd52ae6435ee556a0a93d5609ee20d20cb799 Mon Sep 17 00:00:00 2001 From: yakatak5 Date: Thu, 13 Nov 2025 12:43:25 -0600 Subject: [PATCH 02/12] Disable full test suite in pre-commit configfixing precommit Commented out the full test suite to improve commit speed. --- .pre-commit-config.yaml | 11 +---------- 1 file changed, 1 insertion(+), 10 deletions(-) diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index abb76da7..71fba102 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -22,17 +22,8 @@ repos: pass_filenames: false types: [ python ] stages: [ pre-commit ] - # Disabled: Running full test suite on every commit can be slow - # Run tests manually with: hatch test --cover - # - id: hatch-test - # name: Unit tests - # entry: hatch test --cover - # language: system - # pass_filenames: false - # types: [python] - # stages: [pre-commit] - id: commitizen-check name: Check commit message entry: hatch run cz check --commit-msg-file language: system - stages: [commit-msg] \ No newline at end of file + stages: [commit-msg] From a0c527e5a89e4f152ac659e76a25a166f4ef1abb Mon Sep 17 00:00:00 2001 From: Terry Security Analyst Date: Thu, 13 Nov 2025 12:45:00 -0600 Subject: [PATCH 03/12] fixing precommit --- .pre-commit-config.yaml | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index 71fba102..9eaa02c1 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -22,8 +22,16 @@ repos: pass_filenames: false types: [ python ] stages: [ pre-commit ] + - id: hatch-test + name: Unit tests + entry: hatch test --cover + language: system + pass_filenames: false + types: [python] + stages: [pre-commit] - id: commitizen-check name: Check commit message entry: hatch run cz check --commit-msg-file language: system - stages: [commit-msg] + + stages: [commit-msg] \ No newline at end of file From e4036f3bf4b3eee2d2fce3b160174631044b8ac4 Mon Sep 17 00:00:00 2001 From: Terry Security Analyst Date: Thu, 13 Nov 2025 12:45:43 -0600 Subject: [PATCH 04/12] fixing precommit --- .pre-commit-config.yaml | 1 - 1 file changed, 1 deletion(-) diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index 9eaa02c1..352978f4 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -33,5 +33,4 @@ repos: name: Check commit message entry: hatch run cz check --commit-msg-file language: system - stages: [commit-msg] \ No newline at end of file From 2567774006b675ca996a9f7b508f13e95bc45070 Mon Sep 17 00:00:00 2001 From: Terry Security Analyst Date: Thu, 13 Nov 2025 13:00:29 -0600 Subject: [PATCH 05/12] Ignoring untouched filed --- tests/test_python_repl.py | 24 +++++++++++++----------- 1 file changed, 13 insertions(+), 11 deletions(-) diff --git a/tests/test_python_repl.py b/tests/test_python_repl.py index e0ad5f45..e9993d0c 100644 --- a/tests/test_python_repl.py +++ b/tests/test_python_repl.py @@ -6,7 +6,7 @@ import threading import time from unittest.mock import patch - +from pathlib import Path import dill import pytest from strands import Agent @@ -33,11 +33,11 @@ def mock_console(): @pytest.fixture def temp_repl_state_dir(): """Create a temporary directory for REPL state.""" - with tempfile.TemporaryDirectory() as tempdir: + with tempfile.TemporaryDirectory() as tmpdir: original_dir = python_repl.repl_state.persistence_dir - python_repl.repl_state.persistence_dir = tempdir - python_repl.repl_state.state_file = os.path.join(tempdir, "repl_state.pkl") - yield tempdir + python_repl.repl_state.persistence_dir = tmpdir + python_repl.repl_state.state_file = os.path.join(tmpdir, "repl_state.pkl") + yield tmpdir # Restore original directory python_repl.repl_state.persistence_dir = original_dir python_repl.repl_state.state_file = os.path.join(original_dir, "repl_state.pkl") @@ -88,10 +88,12 @@ def test_execute_and_namespace(self, temp_repl_state_dir): def test_valid_persistence_dir_from_env(self): """Test that a valid PYTHON_REPL_PERSISTENCE_DIR is accepted.""" - with tempfile.TemporaryDirectory() as tempdir: - with patch.dict(os.environ, {"PYTHON_REPL_PERSISTENCE_DIR": tempdir}): + with tempfile.TemporaryDirectory() as tmpdir: + os.makedirs(tmpdir, exist_ok=True) + with patch.dict(os.environ, {"PYTHON_REPL_PERSISTENCE_DIR": tmpdir}): repl = python_repl.ReplState() - assert repl.persistence_dir == tempdir + expected_dir = os.path.join(Path(tmpdir).resolve(), "repl_state") + assert repl.persistence_dir == expected_dir def test_nonexistent_persistence_dir_falls_back(self): """Test that a nonexistent directory changes to default.""" @@ -115,15 +117,15 @@ def test_file_instead_of_dir_defaults(self): def test_unwritable_persistence_dir_falls_back(self): """Test that an unwritable directory changes to default.""" - with tempfile.TemporaryDirectory() as tempdir: + with tempfile.TemporaryDirectory() as tmpdir: # Mock os.access to simulate unwritable directory with ( - patch.dict(os.environ, {"PYTHON_REPL_PERSISTENCE_DIR": tempdir}), + patch.dict(os.environ, {"PYTHON_REPL_PERSISTENCE_DIR": tmpdir}), patch("os.access", return_value=False), ): repl = python_repl.ReplState() # Should fall back to default - assert repl.persistence_dir != tempdir + assert repl.persistence_dir != tmpdir assert "repl_state" in repl.persistence_dir def test_no_env_var_uses_default(self): From 273e74bef09e62a8f132f23f15c9be4ea2644e52 Mon Sep 17 00:00:00 2001 From: Terry Security Analyst Date: Thu, 13 Nov 2025 13:32:25 -0600 Subject: [PATCH 06/12] Ignoring untouched files --- tests/test_python_repl.py | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/tests/test_python_repl.py b/tests/test_python_repl.py index e9993d0c..63b1b392 100644 --- a/tests/test_python_repl.py +++ b/tests/test_python_repl.py @@ -5,8 +5,9 @@ import tempfile import threading import time -from unittest.mock import patch from pathlib import Path +from unittest.mock import patch + import dill import pytest from strands import Agent @@ -114,7 +115,6 @@ def test_file_instead_of_dir_defaults(self): finally: os.unlink(tmpfile.name) - def test_unwritable_persistence_dir_falls_back(self): """Test that an unwritable directory changes to default.""" with tempfile.TemporaryDirectory() as tmpdir: @@ -130,9 +130,7 @@ def test_unwritable_persistence_dir_falls_back(self): def test_no_env_var_uses_default(self): """Test that missing env var uses default directory.""" - with ( - patch.dict(os.environ, {"PYTHON_REPL_PERSISTENCE_DIR": ""}) - ): + with patch.dict(os.environ, {"PYTHON_REPL_PERSISTENCE_DIR": ""}): repl = python_repl.ReplState() assert "repl_state" in repl.persistence_dir assert os.path.exists(repl.persistence_dir) From cbe7ba72b786eda98a005daa8fe6e0daeb25bf1b Mon Sep 17 00:00:00 2001 From: Terry Security Analyst Date: Thu, 13 Nov 2025 13:34:18 -0600 Subject: [PATCH 07/12] ignoring untouched files --- src/strands_tools/python_repl.py | 20 +++++++++++--------- 1 file changed, 11 insertions(+), 9 deletions(-) diff --git a/src/strands_tools/python_repl.py b/src/strands_tools/python_repl.py index ed56351c..fd4b3b6b 100644 --- a/src/strands_tools/python_repl.py +++ b/src/strands_tools/python_repl.py @@ -112,6 +112,8 @@ } + + class OutputCapture: """Captures stdout and stderr output.""" @@ -152,29 +154,29 @@ def __init__(self) -> None: self._namespace = { "__name__": "__main__", } - # Check is persistence directory path defined in env variable - if "PYTHON_REPL_PERSISTENCE_DIR" in os.environ: - dir_path = os.environ.get("PYTHON_REPL_PERSISTENCE_DIR") + # Check if persistence directory path is defined in env variable + if 'PYTHON_REPL_PERSISTENCE_DIR' in os.environ: + dir_path = os.environ.get('PYTHON_REPL_PERSISTENCE_DIR') # Test directory for validation and security try: path = Path(dir_path).resolve() - + # Check if path exists if not path.exists(): raise ValueError(f"Directory does not exist: {path}") - + # Check if directory or file if not path.is_dir(): raise ValueError(f"Path exists but is not a directory: {path}") - + # Check if directory is writable if not os.access(path, os.W_OK): raise PermissionError(f"Directory is not writable: {path}") - + # If all validations pass, set path - self.persistence_dir = os.path.join(path, "repl_state") + self.persistence_dir = os.path.join(path, "repl_state" ) logger.debug(f"Using validated persistence directory: {self.persistence_dir}") - + except Exception as e: # If validation fails, use original default path logger.warning(f"Invalid path set : {e}. Using default path") From ace46395aee7b9933d67ef550c4c43d1e1dec417 Mon Sep 17 00:00:00 2001 From: Terry Security Analyst Date: Thu, 13 Nov 2025 13:39:02 -0600 Subject: [PATCH 08/12] ignoring untouched files --- README.md | 1 + 1 file changed, 1 insertion(+) diff --git a/README.md b/README.md index b8ef9ce7..f216a2fb 100644 --- a/README.md +++ b/README.md @@ -901,6 +901,7 @@ These variables affect multiple tools: | AWS_REGION | Default AWS region for AWS operations | us-west-2 | use_aws, retrieve, generate_image, memory, nova_reels | | AWS_PROFILE | AWS profile name to use from ~/.aws/credentials | default | use_aws, retrieve | | LOG_LEVEL | Logging level (DEBUG, INFO, WARNING, ERROR) | INFO | All tools | +| PYTHON_REPL_PERSISTENCE_DIR | Set Directory for python_repl tool to write state file | Null | python_repl ### Tool-Specific Environment Variables From 80f37ca565df7bf9867bfd6d7de46e114d3010b0 Mon Sep 17 00:00:00 2001 From: Terry Security Analyst Date: Thu, 13 Nov 2025 13:40:36 -0600 Subject: [PATCH 09/12] updating readme --- README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/README.md b/README.md index f216a2fb..0c1d69b1 100644 --- a/README.md +++ b/README.md @@ -901,7 +901,7 @@ These variables affect multiple tools: | AWS_REGION | Default AWS region for AWS operations | us-west-2 | use_aws, retrieve, generate_image, memory, nova_reels | | AWS_PROFILE | AWS profile name to use from ~/.aws/credentials | default | use_aws, retrieve | | LOG_LEVEL | Logging level (DEBUG, INFO, WARNING, ERROR) | INFO | All tools | -| PYTHON_REPL_PERSISTENCE_DIR | Set Directory for python_repl tool to write state file | Null | python_repl +| PYTHON_REPL_PERSISTENCE_DIR | Set Directory for python_repl tool to write state file | Null | python_repl | ### Tool-Specific Environment Variables From f2b9c487167251f3d7ef8f619e42aad13c3c0d71 Mon Sep 17 00:00:00 2001 From: Terry Security Analyst Date: Fri, 14 Nov 2025 10:04:43 -0600 Subject: [PATCH 10/12] Fixing readme and linting --- README.md | 3 ++- src/strands_tools/python_repl.py | 1 - 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/README.md b/README.md index 0c1d69b1..fffa3d84 100644 --- a/README.md +++ b/README.md @@ -901,7 +901,7 @@ These variables affect multiple tools: | AWS_REGION | Default AWS region for AWS operations | us-west-2 | use_aws, retrieve, generate_image, memory, nova_reels | | AWS_PROFILE | AWS profile name to use from ~/.aws/credentials | default | use_aws, retrieve | | LOG_LEVEL | Logging level (DEBUG, INFO, WARNING, ERROR) | INFO | All tools | -| PYTHON_REPL_PERSISTENCE_DIR | Set Directory for python_repl tool to write state file | Null | python_repl | + ### Tool-Specific Environment Variables @@ -1025,6 +1025,7 @@ The Mem0 Memory Tool supports three different backend configurations: | PYTHON_REPL_BINARY_MAX_LEN | Maximum length for binary content before truncation | 100 | | PYTHON_REPL_INTERACTIVE | Whether to enable interactive PTY mode | None | | PYTHON_REPL_RESET_STATE | Whether to reset the REPL state before execution | None | +| PYTHON_REPL_PERSISTENCE_DIR | Set Directory for python_repl tool to write state file | None | #### Shell Tool diff --git a/src/strands_tools/python_repl.py b/src/strands_tools/python_repl.py index fd4b3b6b..124570d3 100644 --- a/src/strands_tools/python_repl.py +++ b/src/strands_tools/python_repl.py @@ -113,7 +113,6 @@ - class OutputCapture: """Captures stdout and stderr output.""" From 432eb110932d9b036a9f9fcc64301d7367ef8f40 Mon Sep 17 00:00:00 2001 From: Terry Security Analyst Date: Fri, 14 Nov 2025 10:06:06 -0600 Subject: [PATCH 11/12] Fixing whitespace --- README.md | 1 - src/strands_tools/python_repl.py | 1 - 2 files changed, 2 deletions(-) diff --git a/README.md b/README.md index fffa3d84..d7a2deb1 100644 --- a/README.md +++ b/README.md @@ -902,7 +902,6 @@ These variables affect multiple tools: | AWS_PROFILE | AWS profile name to use from ~/.aws/credentials | default | use_aws, retrieve | | LOG_LEVEL | Logging level (DEBUG, INFO, WARNING, ERROR) | INFO | All tools | - ### Tool-Specific Environment Variables #### Calculator Tool diff --git a/src/strands_tools/python_repl.py b/src/strands_tools/python_repl.py index 124570d3..24523c7e 100644 --- a/src/strands_tools/python_repl.py +++ b/src/strands_tools/python_repl.py @@ -112,7 +112,6 @@ } - class OutputCapture: """Captures stdout and stderr output.""" From 00c0099c7e2221a1fc91f522e4fc43930b3a7b3e Mon Sep 17 00:00:00 2001 From: Terry Security Analyst Date: Wed, 26 Nov 2025 10:25:06 -0600 Subject: [PATCH 12/12] linting --- src/strands_tools/file_read.py | 21 +++++++++++++++++++++ tests/test_file_read.py | 28 ++++++++++++++++++++++++++++ 2 files changed, 49 insertions(+) diff --git a/src/strands_tools/file_read.py b/src/strands_tools/file_read.py index d57143db..e7d3e61d 100644 --- a/src/strands_tools/file_read.py +++ b/src/strands_tools/file_read.py @@ -122,6 +122,14 @@ from strands_tools.utils import console_util from strands_tools.utils.detect_language import detect_language + +# Custom Exception for Search if no results are found +class NoResultsFound(Exception): + """Exception raised when no search results are found.""" + + pass + + # Document format mapping FORMAT_EXTENSIONS = { "pdf": [".pdf"], @@ -701,6 +709,10 @@ def search_file(console: Console, file_path: str, pattern: str, context_lines: i results.append({"line_number": i + 1, "context": match_text}) + # Check if no results found + if total_matches == 0: + raise NoResultsFound(f"No matches found for pattern '{pattern}' in {os.path.basename(file_path)}") + # Print summary summary = Panel( escape(f"Found {total_matches} matches for pattern '{pattern}' in {os.path.basename(file_path)}"), @@ -1227,6 +1239,15 @@ def file_read(tool: ToolUse, **kwargs: Any) -> ToolResult: console.print(history_panel) response_content.append({"text": f"Time Machine view for {file_path}:\n{history_output}"}) + except NoResultsFound as e: + # Handle NoResultsFound specifically to return error status + error_msg = str(e) + console.print(Panel(escape(error_msg), title="[bold yellow]No Results", border_style="yellow")) + return { + "toolUseId": tool_use_id, + "status": "error", + "content": [{"text": error_msg}], + } except Exception as e: error_msg = f"Error processing file {file_path}: {str(e)}" console.print(Panel(escape(error_msg), title="[bold red]Error", border_style="red")) diff --git a/tests/test_file_read.py b/tests/test_file_read.py index c0db8e35..8499c72a 100644 --- a/tests/test_file_read.py +++ b/tests/test_file_read.py @@ -324,3 +324,31 @@ def test_file_read_error_message_brackets(): result = file_read.file_read(tool=tool_use) assert result["status"] == "error" + + +def test_search_no_results_found_exception(temp_test_file): + """Test that NoResultsFound exception is raised when search finds no matches.""" + console = Console(file=io.StringIO()) + + with pytest.raises(file_read.NoResultsFound) as exc_info: + file_read.search_file(console, temp_test_file, "NonExistentPattern") + + assert "No matches found" in str(exc_info.value) + assert "NonExistentPattern" in str(exc_info.value) + + +def test_search_no_results_via_tool(temp_test_file): + """Test that search mode handles NoResultsFound exception gracefully via tool interface.""" + tool_use = { + "toolUseId": "test-tool-use-id", + "input": { + "path": temp_test_file, + "mode": "search", + "search_pattern": "PatternThatDoesNotExist", + }, + } + + result = file_read.file_read(tool=tool_use) + + assert result["status"] == "error" + assert "No matches found" in result["content"][0]["text"] or "Error processing file" in result["content"][0]["text"]