diff --git a/python/strands_agents_sops/utils.py b/python/strands_agents_sops/utils.py index 79136c2..b3aac3a 100644 --- a/python/strands_agents_sops/utils.py +++ b/python/strands_agents_sops/utils.py @@ -75,6 +75,18 @@ def load_external_sops(sop_directories: list[Path]) -> list[dict[str, Any]]: description = overview_match.group(1).strip().replace("\n", " ") sop_name = sop_file.stem.removesuffix(".sop") + # sop_name is later joined onto an output directory as a bare + # path segment (e.g. skills.py's `output_path / sop_name`) to + # create per-SOP files/directories. A crafted filename such as + # "...sop.md" derives sop_name == "..", which escapes the + # intended output directory by one level. Reject any name that + # isn't a plain, single path component. + if not sop_name or sop_name in (".", "..") or os.sep in sop_name: + logger.warning( + f"Skipping SOP with unsafe name {sop_name!r} from {sop_file}" + ) + continue + external_sops.append( { "name": sop_name, diff --git a/python/tests/test_external_sop_loading.py b/python/tests/test_external_sop_loading.py index db6c75b..ef2d3ed 100644 --- a/python/tests/test_external_sop_loading.py +++ b/python/tests/test_external_sop_loading.py @@ -97,6 +97,19 @@ def test_handle_nonexistent_directory(self): sops = load_external_sops([nonexistent_path]) assert len(sops) == 0 # Should handle gracefully + def test_reject_path_traversal_sop_name(self): + """A filename like '...sop.md' derives sop_name == '..', which would + escape the intended output directory when later joined onto one + (e.g. skills.py's `output_path / sop_name`). Must be skipped.""" + with tempfile.TemporaryDirectory() as temp_dir: + sop_content = """## Overview +Attempts to masquerade as the parent directory. +""" + (Path(temp_dir) / "...sop.md").write_text(sop_content) + + sops = load_external_sops([Path(temp_dir)]) + assert len(sops) == 0 + class TestMCPIntegration: """Test MCP server integration with external SOPs"""