From 113f1d46ad7547939d5b221e75e9469364ae2394 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 3 Jul 2026 06:25:00 +0000 Subject: [PATCH] =?UTF-8?q?fix:=20three=20bug=20fixes=20=E2=80=94=20safety?= =?UTF-8?q?=20guardrails=20false=20positives,=20pyyaml=20error=20status,?= =?UTF-8?q?=20set=5Fwaves=20install=20fallback?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Bug #321 (safety-guardrails.py): check_file_safety matched dangerous patterns against the full path, causing false positives when directory names contained security-related words (e.g. secrets-injector/values.yaml was blocked because "secret" appeared in the parent directory name). Fix: match only against os.path.basename(path).lower(). Bug #319 (map_step_runner.py): parse_requirements_index caught both ImportError (missing PyYAML) and yaml.YAMLError in a single except-Exception block, returning status='malformed' for both — misleading users into thinking their spec was broken when PyYAML was simply not installed. Fix: split exception handlers; ImportError now returns status='pyyaml_missing' with an actionable "pip install pyyaml" message. validate_blueprint_contract updated to handle the new status. Bug #320 (map_orchestrator.py): set_waves ImportError fallback only searched source-checkout layout paths (src/mapify_cli/ relative to __file__ parents), missing packages installed via 'uv tool install' or 'pipx install' which land in ~/.local/share/uv/tools/mapify-cli/lib/python3.X/site-packages/. Fix: extend the candidate list to include common installed-package locations; improve error message to guide uv-tool install users. Also: skip test_write_project_mcp_json_permission_error when running as root (root bypasses chmod 0o444 restrictions — the test's OSError cannot be triggered). Add regression tests for all three bugs. Co-Authored-By: Claude Sonnet 4.6 Claude-Session: https://claude.ai/code/session_011dqN4Wq6pEtodBmSTxbyZp --- .claude/hooks/safety-guardrails.py | 13 +- .map/scripts/map_orchestrator.py | 29 ++++- .map/scripts/map_step_runner.py | 35 +++++- .../templates/hooks/safety-guardrails.py | 13 +- .../templates/map/scripts/map_orchestrator.py | 29 ++++- .../templates/map/scripts/map_step_runner.py | 35 +++++- .../hooks/safety-guardrails.py.jinja | 13 +- .../map/scripts/map_orchestrator.py.jinja | 29 ++++- .../map/scripts/map_step_runner.py.jinja | 35 +++++- tests/hooks/test_safety_guardrails.py | 58 +++++++++ tests/test_map_orchestrator.py | 119 ++++++++++++++++++ tests/test_map_step_runner.py | 26 ++++ 12 files changed, 401 insertions(+), 33 deletions(-) diff --git a/.claude/hooks/safety-guardrails.py b/.claude/hooks/safety-guardrails.py index f7d871f5..c904ccaf 100755 --- a/.claude/hooks/safety-guardrails.py +++ b/.claude/hooks/safety-guardrails.py @@ -130,17 +130,22 @@ def check_file_safety(path: str) -> tuple[bool, str]: if is_safe_path(path): return True, "" - # Check dangerous patterns - path_lower = path.lower() + # Check dangerous patterns against the basename only, not the full path. + # Matching the full path causes false positives when a directory name contains + # security-related words (e.g. "secrets-injector", "stackland-secrets-webhook"): + # a legitimate file like "values.yaml" inside such a directory would be blocked + # even though the file itself is not a credential file. The patterns are designed + # to catch files with dangerous *names* — anchoring to the basename is correct. + basename_lower = os.path.basename(path).lower() if DANGEROUS_FILE_PATTERNS == _DEFAULT_DANGEROUS_FILE_PATTERNS and not any( - marker in path_lower for marker in _DEFAULT_DANGEROUS_FILE_MARKERS + marker in basename_lower for marker in _DEFAULT_DANGEROUS_FILE_MARKERS ): return True, "" import re for pattern in DANGEROUS_FILE_PATTERNS: - if re.search(pattern, path_lower, re.IGNORECASE): + if re.search(pattern, basename_lower, re.IGNORECASE): return ( False, f"Blocked: Access to sensitive file pattern '{pattern}' in path: {path}", diff --git a/.map/scripts/map_orchestrator.py b/.map/scripts/map_orchestrator.py index d33bbcd7..744b9987 100755 --- a/.map/scripts/map_orchestrator.py +++ b/.map/scripts/map_orchestrator.py @@ -2230,12 +2230,29 @@ def set_waves(branch: str, blueprint_path: Optional[str] = None) -> dict: from mapify_cli.dependency_graph import DependencyGraph, SubtaskNode except ImportError: # When running as a standalone script, dependency_graph.py may not be - # importable from sys.path. Walk upward and look for src/mapify_cli/. + # importable from sys.path. Try in order: + # 1. Source-checkout layout: src/mapify_cli/ relative to this file or cwd. + # 2. Installed-package layout: uv tool install / pipx venv locations + # (~/.local/share/uv/tools/mapify-cli/... or + # ~/.local/pipx/venvs/mapify-cli/...). import importlib.util - dg_candidates = [Path("src/mapify_cli/dependency_graph.py")] + dg_candidates: list[Path] = [Path("src/mapify_cli/dependency_graph.py")] for parent in Path(__file__).resolve().parents: dg_candidates.append(parent / "src" / "mapify_cli" / "dependency_graph.py") + + # Common installed-package locations (uv tool install, pipx install). + _home = Path.home() + for _tool_dir in [ + _home / ".local" / "share" / "uv" / "tools" / "mapify-cli", + _home / ".local" / "pipx" / "venvs" / "mapify-cli", + ]: + if _tool_dir.exists(): + for _py_dir in sorted(_tool_dir.glob("lib/python3.*"), reverse=True)[:1]: + dg_candidates.append( + _py_dir / "site-packages" / "mapify_cli" / "dependency_graph.py" + ) + loaded = False for candidate in dg_candidates: if candidate.exists(): @@ -2252,7 +2269,13 @@ def set_waves(branch: str, blueprint_path: Optional[str] = None) -> dict: if not loaded: return { "status": "error", - "message": "Cannot import dependency_graph module", + "message": ( + "Cannot import dependency_graph module. " + "If mapify-cli was installed via 'uv tool install', invoke this " + "script with the uv-tool Python interpreter directly: " + "~/.local/share/uv/tools/mapify-cli/bin/python3 " + ".map/scripts/map_orchestrator.py ..." + ), } if blueprint_path is None: diff --git a/.map/scripts/map_step_runner.py b/.map/scripts/map_step_runner.py index dab0a623..50935b29 100755 --- a/.map/scripts/map_step_runner.py +++ b/.map/scripts/map_step_runner.py @@ -2797,18 +2797,22 @@ def parse_requirements_index(spec_text: str) -> dict[str, object]: (open) and ```` (close). Only the fenced ```yaml block between those two sentinels is authoritative; a sentinel-shaped string anywhere else in the prose is ignored. - - Parses the inner YAML via a lazy ``import yaml``; both ``ImportError`` and - ``yaml.YAMLError`` yield ``status='malformed'`` — never an uncaught exception. + - Parses the inner YAML via a lazy ``import yaml``. ``ImportError`` (PyYAML not + installed) yields ``status='pyyaml_missing'`` with an honest message; genuine + ``yaml.YAMLError`` yields ``status='malformed'`` — never an uncaught exception. - Returns:: { 'requirements': [{'id': str, 'kind': str}, ...], - 'status': 'absent' | 'malformed' | 'present_empty' | 'present_nonempty', + 'status': 'absent' | 'pyyaml_missing' | 'malformed' | 'present_empty' | 'present_nonempty', 'warnings': [str, ...], } Status semantics: - ``absent`` — sentinel pair not found in the text. + - ``pyyaml_missing`` — open sentinel found, YAML block present, but PyYAML is + not installed in the running Python environment. This is + an environment problem, NOT a spec formatting problem. - ``malformed`` — open sentinel found but: no close sentinel, no inner yaml fence, YAML parse error, or top-level shape is not ``{requirements: list}``. @@ -2854,9 +2858,23 @@ def parse_requirements_index(spec_text: str) -> dict[str, object]: yaml_text = after_fence[:fence_close] - # Step 4: parse YAML (lazy import; treat ImportError == YAMLError == malformed). + # Step 4: parse YAML (lazy import). + # ImportError means PyYAML is not installed — an environment problem, not a spec + # formatting problem. Return a distinct status so callers can surface an honest + # "install pyyaml" message instead of the misleading "Requirements Index is malformed". try: import yaml # noqa: PLC0415 + except ImportError: + return { + "requirements": [], + "status": "pyyaml_missing", + "warnings": warnings + + [ + "PyYAML is not installed; cannot parse Requirements Index. " + "Run: pip install pyyaml" + ], + } + try: data = yaml.safe_load(yaml_text) except Exception: return {"requirements": [], "status": "malformed", "warnings": warnings} @@ -3497,6 +3515,15 @@ def _constraint_body(c: dict) -> str: errors.append(_fc_msg) else: warnings.append(_fc_msg) + elif _fc_status == "pyyaml_missing": + # PyYAML not installed — environment problem, not a spec formatting problem. + _fc_confidence = "low" + _fc_basis = "PyYAML not installed; cannot parse Requirements Index" + errors.append( + "Forward-coverage: Cannot validate Requirements Index — " + "PyYAML is not installed in this Python environment. " + "Run: pip install pyyaml" + ) elif _fc_status == "malformed": # HC-5 / HC-3: malformed is always a hard error regardless of strict flag. _fc_confidence = "low" diff --git a/src/mapify_cli/templates/hooks/safety-guardrails.py b/src/mapify_cli/templates/hooks/safety-guardrails.py index f7d871f5..c904ccaf 100755 --- a/src/mapify_cli/templates/hooks/safety-guardrails.py +++ b/src/mapify_cli/templates/hooks/safety-guardrails.py @@ -130,17 +130,22 @@ def check_file_safety(path: str) -> tuple[bool, str]: if is_safe_path(path): return True, "" - # Check dangerous patterns - path_lower = path.lower() + # Check dangerous patterns against the basename only, not the full path. + # Matching the full path causes false positives when a directory name contains + # security-related words (e.g. "secrets-injector", "stackland-secrets-webhook"): + # a legitimate file like "values.yaml" inside such a directory would be blocked + # even though the file itself is not a credential file. The patterns are designed + # to catch files with dangerous *names* — anchoring to the basename is correct. + basename_lower = os.path.basename(path).lower() if DANGEROUS_FILE_PATTERNS == _DEFAULT_DANGEROUS_FILE_PATTERNS and not any( - marker in path_lower for marker in _DEFAULT_DANGEROUS_FILE_MARKERS + marker in basename_lower for marker in _DEFAULT_DANGEROUS_FILE_MARKERS ): return True, "" import re for pattern in DANGEROUS_FILE_PATTERNS: - if re.search(pattern, path_lower, re.IGNORECASE): + if re.search(pattern, basename_lower, re.IGNORECASE): return ( False, f"Blocked: Access to sensitive file pattern '{pattern}' in path: {path}", diff --git a/src/mapify_cli/templates/map/scripts/map_orchestrator.py b/src/mapify_cli/templates/map/scripts/map_orchestrator.py index d33bbcd7..744b9987 100755 --- a/src/mapify_cli/templates/map/scripts/map_orchestrator.py +++ b/src/mapify_cli/templates/map/scripts/map_orchestrator.py @@ -2230,12 +2230,29 @@ def set_waves(branch: str, blueprint_path: Optional[str] = None) -> dict: from mapify_cli.dependency_graph import DependencyGraph, SubtaskNode except ImportError: # When running as a standalone script, dependency_graph.py may not be - # importable from sys.path. Walk upward and look for src/mapify_cli/. + # importable from sys.path. Try in order: + # 1. Source-checkout layout: src/mapify_cli/ relative to this file or cwd. + # 2. Installed-package layout: uv tool install / pipx venv locations + # (~/.local/share/uv/tools/mapify-cli/... or + # ~/.local/pipx/venvs/mapify-cli/...). import importlib.util - dg_candidates = [Path("src/mapify_cli/dependency_graph.py")] + dg_candidates: list[Path] = [Path("src/mapify_cli/dependency_graph.py")] for parent in Path(__file__).resolve().parents: dg_candidates.append(parent / "src" / "mapify_cli" / "dependency_graph.py") + + # Common installed-package locations (uv tool install, pipx install). + _home = Path.home() + for _tool_dir in [ + _home / ".local" / "share" / "uv" / "tools" / "mapify-cli", + _home / ".local" / "pipx" / "venvs" / "mapify-cli", + ]: + if _tool_dir.exists(): + for _py_dir in sorted(_tool_dir.glob("lib/python3.*"), reverse=True)[:1]: + dg_candidates.append( + _py_dir / "site-packages" / "mapify_cli" / "dependency_graph.py" + ) + loaded = False for candidate in dg_candidates: if candidate.exists(): @@ -2252,7 +2269,13 @@ def set_waves(branch: str, blueprint_path: Optional[str] = None) -> dict: if not loaded: return { "status": "error", - "message": "Cannot import dependency_graph module", + "message": ( + "Cannot import dependency_graph module. " + "If mapify-cli was installed via 'uv tool install', invoke this " + "script with the uv-tool Python interpreter directly: " + "~/.local/share/uv/tools/mapify-cli/bin/python3 " + ".map/scripts/map_orchestrator.py ..." + ), } if blueprint_path is None: diff --git a/src/mapify_cli/templates/map/scripts/map_step_runner.py b/src/mapify_cli/templates/map/scripts/map_step_runner.py index dab0a623..50935b29 100755 --- a/src/mapify_cli/templates/map/scripts/map_step_runner.py +++ b/src/mapify_cli/templates/map/scripts/map_step_runner.py @@ -2797,18 +2797,22 @@ def parse_requirements_index(spec_text: str) -> dict[str, object]: (open) and ```` (close). Only the fenced ```yaml block between those two sentinels is authoritative; a sentinel-shaped string anywhere else in the prose is ignored. - - Parses the inner YAML via a lazy ``import yaml``; both ``ImportError`` and - ``yaml.YAMLError`` yield ``status='malformed'`` — never an uncaught exception. + - Parses the inner YAML via a lazy ``import yaml``. ``ImportError`` (PyYAML not + installed) yields ``status='pyyaml_missing'`` with an honest message; genuine + ``yaml.YAMLError`` yields ``status='malformed'`` — never an uncaught exception. - Returns:: { 'requirements': [{'id': str, 'kind': str}, ...], - 'status': 'absent' | 'malformed' | 'present_empty' | 'present_nonempty', + 'status': 'absent' | 'pyyaml_missing' | 'malformed' | 'present_empty' | 'present_nonempty', 'warnings': [str, ...], } Status semantics: - ``absent`` — sentinel pair not found in the text. + - ``pyyaml_missing`` — open sentinel found, YAML block present, but PyYAML is + not installed in the running Python environment. This is + an environment problem, NOT a spec formatting problem. - ``malformed`` — open sentinel found but: no close sentinel, no inner yaml fence, YAML parse error, or top-level shape is not ``{requirements: list}``. @@ -2854,9 +2858,23 @@ def parse_requirements_index(spec_text: str) -> dict[str, object]: yaml_text = after_fence[:fence_close] - # Step 4: parse YAML (lazy import; treat ImportError == YAMLError == malformed). + # Step 4: parse YAML (lazy import). + # ImportError means PyYAML is not installed — an environment problem, not a spec + # formatting problem. Return a distinct status so callers can surface an honest + # "install pyyaml" message instead of the misleading "Requirements Index is malformed". try: import yaml # noqa: PLC0415 + except ImportError: + return { + "requirements": [], + "status": "pyyaml_missing", + "warnings": warnings + + [ + "PyYAML is not installed; cannot parse Requirements Index. " + "Run: pip install pyyaml" + ], + } + try: data = yaml.safe_load(yaml_text) except Exception: return {"requirements": [], "status": "malformed", "warnings": warnings} @@ -3497,6 +3515,15 @@ def _constraint_body(c: dict) -> str: errors.append(_fc_msg) else: warnings.append(_fc_msg) + elif _fc_status == "pyyaml_missing": + # PyYAML not installed — environment problem, not a spec formatting problem. + _fc_confidence = "low" + _fc_basis = "PyYAML not installed; cannot parse Requirements Index" + errors.append( + "Forward-coverage: Cannot validate Requirements Index — " + "PyYAML is not installed in this Python environment. " + "Run: pip install pyyaml" + ) elif _fc_status == "malformed": # HC-5 / HC-3: malformed is always a hard error regardless of strict flag. _fc_confidence = "low" diff --git a/src/mapify_cli/templates_src/hooks/safety-guardrails.py.jinja b/src/mapify_cli/templates_src/hooks/safety-guardrails.py.jinja index f7d871f5..c904ccaf 100755 --- a/src/mapify_cli/templates_src/hooks/safety-guardrails.py.jinja +++ b/src/mapify_cli/templates_src/hooks/safety-guardrails.py.jinja @@ -130,17 +130,22 @@ def check_file_safety(path: str) -> tuple[bool, str]: if is_safe_path(path): return True, "" - # Check dangerous patterns - path_lower = path.lower() + # Check dangerous patterns against the basename only, not the full path. + # Matching the full path causes false positives when a directory name contains + # security-related words (e.g. "secrets-injector", "stackland-secrets-webhook"): + # a legitimate file like "values.yaml" inside such a directory would be blocked + # even though the file itself is not a credential file. The patterns are designed + # to catch files with dangerous *names* — anchoring to the basename is correct. + basename_lower = os.path.basename(path).lower() if DANGEROUS_FILE_PATTERNS == _DEFAULT_DANGEROUS_FILE_PATTERNS and not any( - marker in path_lower for marker in _DEFAULT_DANGEROUS_FILE_MARKERS + marker in basename_lower for marker in _DEFAULT_DANGEROUS_FILE_MARKERS ): return True, "" import re for pattern in DANGEROUS_FILE_PATTERNS: - if re.search(pattern, path_lower, re.IGNORECASE): + if re.search(pattern, basename_lower, re.IGNORECASE): return ( False, f"Blocked: Access to sensitive file pattern '{pattern}' in path: {path}", diff --git a/src/mapify_cli/templates_src/map/scripts/map_orchestrator.py.jinja b/src/mapify_cli/templates_src/map/scripts/map_orchestrator.py.jinja index d33bbcd7..744b9987 100755 --- a/src/mapify_cli/templates_src/map/scripts/map_orchestrator.py.jinja +++ b/src/mapify_cli/templates_src/map/scripts/map_orchestrator.py.jinja @@ -2230,12 +2230,29 @@ def set_waves(branch: str, blueprint_path: Optional[str] = None) -> dict: from mapify_cli.dependency_graph import DependencyGraph, SubtaskNode except ImportError: # When running as a standalone script, dependency_graph.py may not be - # importable from sys.path. Walk upward and look for src/mapify_cli/. + # importable from sys.path. Try in order: + # 1. Source-checkout layout: src/mapify_cli/ relative to this file or cwd. + # 2. Installed-package layout: uv tool install / pipx venv locations + # (~/.local/share/uv/tools/mapify-cli/... or + # ~/.local/pipx/venvs/mapify-cli/...). import importlib.util - dg_candidates = [Path("src/mapify_cli/dependency_graph.py")] + dg_candidates: list[Path] = [Path("src/mapify_cli/dependency_graph.py")] for parent in Path(__file__).resolve().parents: dg_candidates.append(parent / "src" / "mapify_cli" / "dependency_graph.py") + + # Common installed-package locations (uv tool install, pipx install). + _home = Path.home() + for _tool_dir in [ + _home / ".local" / "share" / "uv" / "tools" / "mapify-cli", + _home / ".local" / "pipx" / "venvs" / "mapify-cli", + ]: + if _tool_dir.exists(): + for _py_dir in sorted(_tool_dir.glob("lib/python3.*"), reverse=True)[:1]: + dg_candidates.append( + _py_dir / "site-packages" / "mapify_cli" / "dependency_graph.py" + ) + loaded = False for candidate in dg_candidates: if candidate.exists(): @@ -2252,7 +2269,13 @@ def set_waves(branch: str, blueprint_path: Optional[str] = None) -> dict: if not loaded: return { "status": "error", - "message": "Cannot import dependency_graph module", + "message": ( + "Cannot import dependency_graph module. " + "If mapify-cli was installed via 'uv tool install', invoke this " + "script with the uv-tool Python interpreter directly: " + "~/.local/share/uv/tools/mapify-cli/bin/python3 " + ".map/scripts/map_orchestrator.py ..." + ), } if blueprint_path is None: diff --git a/src/mapify_cli/templates_src/map/scripts/map_step_runner.py.jinja b/src/mapify_cli/templates_src/map/scripts/map_step_runner.py.jinja index dab0a623..50935b29 100755 --- a/src/mapify_cli/templates_src/map/scripts/map_step_runner.py.jinja +++ b/src/mapify_cli/templates_src/map/scripts/map_step_runner.py.jinja @@ -2797,18 +2797,22 @@ def parse_requirements_index(spec_text: str) -> dict[str, object]: (open) and ```` (close). Only the fenced ```yaml block between those two sentinels is authoritative; a sentinel-shaped string anywhere else in the prose is ignored. - - Parses the inner YAML via a lazy ``import yaml``; both ``ImportError`` and - ``yaml.YAMLError`` yield ``status='malformed'`` — never an uncaught exception. + - Parses the inner YAML via a lazy ``import yaml``. ``ImportError`` (PyYAML not + installed) yields ``status='pyyaml_missing'`` with an honest message; genuine + ``yaml.YAMLError`` yields ``status='malformed'`` — never an uncaught exception. - Returns:: { 'requirements': [{'id': str, 'kind': str}, ...], - 'status': 'absent' | 'malformed' | 'present_empty' | 'present_nonempty', + 'status': 'absent' | 'pyyaml_missing' | 'malformed' | 'present_empty' | 'present_nonempty', 'warnings': [str, ...], } Status semantics: - ``absent`` — sentinel pair not found in the text. + - ``pyyaml_missing`` — open sentinel found, YAML block present, but PyYAML is + not installed in the running Python environment. This is + an environment problem, NOT a spec formatting problem. - ``malformed`` — open sentinel found but: no close sentinel, no inner yaml fence, YAML parse error, or top-level shape is not ``{requirements: list}``. @@ -2854,9 +2858,23 @@ def parse_requirements_index(spec_text: str) -> dict[str, object]: yaml_text = after_fence[:fence_close] - # Step 4: parse YAML (lazy import; treat ImportError == YAMLError == malformed). + # Step 4: parse YAML (lazy import). + # ImportError means PyYAML is not installed — an environment problem, not a spec + # formatting problem. Return a distinct status so callers can surface an honest + # "install pyyaml" message instead of the misleading "Requirements Index is malformed". try: import yaml # noqa: PLC0415 + except ImportError: + return { + "requirements": [], + "status": "pyyaml_missing", + "warnings": warnings + + [ + "PyYAML is not installed; cannot parse Requirements Index. " + "Run: pip install pyyaml" + ], + } + try: data = yaml.safe_load(yaml_text) except Exception: return {"requirements": [], "status": "malformed", "warnings": warnings} @@ -3497,6 +3515,15 @@ def validate_blueprint_contract( errors.append(_fc_msg) else: warnings.append(_fc_msg) + elif _fc_status == "pyyaml_missing": + # PyYAML not installed — environment problem, not a spec formatting problem. + _fc_confidence = "low" + _fc_basis = "PyYAML not installed; cannot parse Requirements Index" + errors.append( + "Forward-coverage: Cannot validate Requirements Index — " + "PyYAML is not installed in this Python environment. " + "Run: pip install pyyaml" + ) elif _fc_status == "malformed": # HC-5 / HC-3: malformed is always a hard error regardless of strict flag. _fc_confidence = "low" diff --git a/tests/hooks/test_safety_guardrails.py b/tests/hooks/test_safety_guardrails.py index ce9d85c1..df9c436d 100644 --- a/tests/hooks/test_safety_guardrails.py +++ b/tests/hooks/test_safety_guardrails.py @@ -498,6 +498,64 @@ def test_empty_input(self): assert _parse_stdout(result.stdout) == {} +# ============================================================================= +# Regression Tests — Bug #321: Directory Name False Positives +# ============================================================================= + + +class TestDirectoryNameFalsePositives: + """Regression tests for bug #321. + + Files with safe names inside directories with security-related names + (e.g. 'secrets-injector', 'stackland-secrets-webhook') must NOT be blocked. + The pattern check must match against the file basename only, never the full path. + """ + + @pytest.mark.parametrize( + "path", + [ + # Directory contains 'secret' but the FILE is a plain config file. + "deploy/secrets-injector/values.yaml", + "k8s/stackland-secrets-webhook/config.json", + "infra/secrets-manager/deployment.yaml", + # Directory contains 'credential' but the file is not a credential file. + "apps/credential-service/main.py", + # Directory contains 'token' but the file is not a token store. + "services/token-validator/handler.go", + # Directory contains 'key' but the file is not a private key. + "apps/api-key-service/schema.sql", + # Directory contains 'password' but the file is documentation. + "docs/password-policy/README.md", + # Directory contains 'private' but the file is a public module. + "libs/private-utils/public_api.py", + ], + ) + def test_safe_file_in_dangerous_directory_not_blocked(self, path): + """A file with a safe basename must not be blocked even if its parent + directory name matches a dangerous pattern (regression for bug #321).""" + exit_code, stdout, _ = run_hook_file("Read", path) + assert exit_code == 0 + assert _parse_stdout(stdout) == {}, ( + f"Path '{path}' was incorrectly blocked — basename is safe; " + "only the directory name contains a dangerous-looking word" + ) + + @pytest.mark.parametrize( + "path", + [ + # The FILE itself is dangerous — must still be blocked. + "deploy/secrets-injector/secrets.yaml", + "k8s/app/.env", + "infra/deploy/credentials.json", + ], + ) + def test_dangerous_file_in_any_directory_still_blocked(self, path): + """A file with a dangerous basename must be blocked regardless of where it lives.""" + exit_code, stdout, _ = run_hook_file("Read", path) + assert exit_code == 0 + _assert_denied(_parse_stdout(stdout)) + + # ============================================================================= # Performance Tests # ============================================================================= diff --git a/tests/test_map_orchestrator.py b/tests/test_map_orchestrator.py index 96627bfa..84d8d522 100644 --- a/tests/test_map_orchestrator.py +++ b/tests/test_map_orchestrator.py @@ -5617,5 +5617,124 @@ def test_vc4_mixed_plan_current_width1_later_width2_sequential( ) +# ============================================================================ +# Regression Tests — Bug #320: set_waves ImportError fallback for uv tool installs +# ============================================================================ + + +class TestSetWavesImportFallback: + """Regression tests for bug #320. + + Previously the ImportError fallback in set_waves only searched source-checkout + layout paths (src/mapify_cli/dependency_graph.py relative to parents of __file__). + When mapify-cli is installed via 'uv tool install' or 'pipx install', the package + lives in ~/.local/share/uv/tools/mapify-cli/lib/python3.X/site-packages/, so the + fallback silently failed. + + Fix: extend the candidate list to include common installed-package locations. + """ + + def test_set_waves_error_message_mentions_uv_tool_path( + self, branch_dir: str, tmp_path: Path + ) -> None: + """When dependency_graph cannot be found anywhere, the error message must + mention the uv-tool Python path so users know how to fix it (regression #320).""" + from unittest import mock + + # Write a minimal blueprint so we can get past the file-reading step. + bp_dir = tmp_path / ".map" / branch_dir + bp_dir.mkdir(parents=True, exist_ok=True) + blueprint = { + "subtasks": [ + {"id": "ST-001", "dependencies": [], "affected_files": []}, + ] + } + bp_file = bp_dir / "blueprint.json" + bp_file.write_text(json.dumps(blueprint), encoding="utf-8") + + # Simulate a totally missing dependency_graph: make importlib.util.spec_from_file_location + # always return None (as if no candidate file exists), and make Path.home() point to an + # empty tmp dir (so no uv/pipx tool dirs exist to find). + empty_home = tmp_path / "fake_home" + empty_home.mkdir() + + with ( + mock.patch.object( + map_orchestrator.importlib.util, + "spec_from_file_location", + return_value=None, + ) if hasattr(map_orchestrator, "importlib") else mock.patch("importlib.util.spec_from_file_location", return_value=None), + mock.patch("pathlib.Path.home", return_value=empty_home), + mock.patch.dict(sys.modules, {"mapify_cli.dependency_graph": None}), + ): + result = map_orchestrator.set_waves(branch_dir, str(bp_file)) + + assert result["status"] == "error" + msg = result.get("message", "") + assert "uv tool install" in msg or "uv-tool" in msg or "mapify-cli" in msg, ( + f"Error message should guide users to the uv-tool install path. Got: {msg!r}. " + "Bug #320: previously had no guidance for uv tool install users." + ) + + def test_set_waves_fallback_finds_module_in_fake_uv_tool_dir( + self, branch_dir: str, tmp_path: Path, monkeypatch: pytest.MonkeyPatch + ) -> None: + """set_waves must succeed when dependency_graph.py is only available + at the uv-tool install layout path (regression for bug #320). + + We build a fake uv-tool directory tree rooted at a tmp home, plant the + REAL dependency_graph.py there, then simulate the package-level import + failing so the fallback path is exercised. + """ + import shutil + from unittest import mock + + # Build the fake uv-tool installed-package layout. + fake_home = tmp_path / "fake_home" + py_version = f"python{sys.version_info.major}.{sys.version_info.minor}" + site_packages = ( + fake_home + / ".local" / "share" / "uv" / "tools" / "mapify-cli" + / "lib" / py_version / "site-packages" / "mapify_cli" + ) + site_packages.mkdir(parents=True) + + # Copy the real dependency_graph.py into the fake site-packages. + real_dg = ( + Path(__file__).resolve().parents[1] + / "src" / "mapify_cli" / "dependency_graph.py" + ) + shutil.copy(real_dg, site_packages / "dependency_graph.py") + + # Write a blueprint to work with. + bp_dir = tmp_path / ".map" / branch_dir + bp_dir.mkdir(parents=True, exist_ok=True) + blueprint = { + "subtasks": [ + {"id": "ST-001", "dependencies": [], "affected_files": []}, + {"id": "ST-002", "dependencies": ["ST-001"], "affected_files": []}, + ] + } + bp_file = bp_dir / "blueprint.json" + bp_file.write_text(json.dumps(blueprint), encoding="utf-8") + + # Patch Path.home() so the fallback search lands in our fake tree. + monkeypatch.setattr(map_orchestrator.Path, "home", staticmethod(lambda: fake_home)) + + # Force the package-level import to raise ImportError so the fallback runs. + with mock.patch.dict(sys.modules, {"mapify_cli.dependency_graph": None}): + result = map_orchestrator.set_waves(branch_dir, str(bp_file)) + + assert result["status"] == "success", ( + f"set_waves should succeed when dependency_graph is found in the fake " + f"uv-tool install dir. status={result.get('status')!r}, " + f"message={result.get('message')!r}. " + "Bug #320: fallback didn't include uv tool install paths." + ) + waves = result["execution_waves"] + assert waves[0] == ["ST-001"] + assert waves[1] == ["ST-002"] + + if __name__ == "__main__": pytest.main([__file__, "-v"]) diff --git a/tests/test_map_step_runner.py b/tests/test_map_step_runner.py index a589a1a4..4fe053e7 100644 --- a/tests/test_map_step_runner.py +++ b/tests/test_map_step_runner.py @@ -10524,6 +10524,32 @@ def test_st003_vc3_close_sentinel_without_open_yields_absent(self) -> None: assert result["status"] == "absent" assert result["requirements"] == [] + def test_st003_pyyaml_missing_returns_distinct_status(self) -> None: + """Regression for bug #319: ImportError (missing PyYAML) must return + status='pyyaml_missing', NOT status='malformed'. + + Previously both ImportError and yaml.YAMLError were caught by the same + ``except Exception``, causing a misleading 'malformed' verdict when the + environment simply lacked PyYAML. + """ + import sys + from unittest import mock + + spec = _make_spec("requirements:\n - id: AC-1\n kind: acceptance_criterion\n") + # Simulate a missing PyYAML by making 'import yaml' raise ImportError. + with mock.patch.dict(sys.modules, {"yaml": None}): + result = map_step_runner.parse_requirements_index(spec) + + assert result["status"] == "pyyaml_missing", ( + f"Expected status='pyyaml_missing' when PyYAML is unavailable, " + f"got status={result['status']!r}. " + "Bug #319: ImportError was previously mis-classified as 'malformed'." + ) + assert result["requirements"] == [] + assert any("pyyaml" in w.lower() or "pip install" in w.lower() for w in result["warnings"]), ( + "Expected a warning mentioning PyYAML installation" + ) + # ============================================================================ # ST-005: forward-gate 5 outcomes via validate_blueprint_contract