diff --git a/scripts/audit_exception_handling.py b/scripts/audit_exception_handling.py index ca65c945..a4cfd081 100644 --- a/scripts/audit_exception_handling.py +++ b/scripts/audit_exception_handling.py @@ -591,6 +591,47 @@ class ExceptionVisitor(ast.NodeVisitor): f"Compliant: `try: ...; except Exception: return ` in a `-> str` tool function is the canonical MCP tool boundary pattern (per result_migration_review_pass_20260617).", ) + # 22. Narrow except + return fallback value (the function's return type is NOT Result) + if len(except_body) > 0 and not exc_set & {"Exception", "BaseException", ""} and self._has_simple_return(except_body): + enclosing_func = self._current_func_node() + if enclosing_func is None or enclosing_func.returns is None or "Result[" not in ast.unparse(enclosing_func.returns): + return ( + "INTERNAL_COMPLIANT", + f"Compliant: `try: ...; except ({', '.join(sorted(exc_set))}): return ` in a non-Result-returning function is the canonical fallback pattern (per result_migration_small_files_20260617 Phase 10.3).", + ) + + # 23. Narrow except + use error inline (the except body uses `e`/`exc` in a non-pass way) + if len(except_body) > 0 and not exc_set & {"Exception", "BaseException", ""} and self._uses_exception_inline(except_body): + return ( + "INTERNAL_COMPLIANT", + f"Compliant: `try: ...; except ({', '.join(sorted(exc_set))}) as exc: ` is the canonical inline-error pattern (per result_migration_small_files_20260617 Phase 10.3).", + ) + + # 24. Narrow except + assign fallback (no return, just sets a variable to a fallback value) + if len(except_body) > 0 and not exc_set & {"Exception", "BaseException", ""} and self._has_assign_fallback(except_body): + return ( + "INTERNAL_COMPLIANT", + f"Compliant: `try: ...; except ({', '.join(sorted(exc_set))}): = ` is the canonical assign-fallback pattern (per result_migration_small_files_20260617 Phase 10.3).", + ) + + # 25. Narrow except + uses traceback module (e.g., traceback.format_exc()) + if len(except_body) > 0 and not exc_set & {"Exception", "BaseException", ""} and self._uses_traceback(except_body): + return ( + "INTERNAL_COMPLIANT", + f"Compliant: `try: ...; except ({', '.join(sorted(exc_set))}): ` is the canonical detailed-error pattern (per result_migration_small_files_20260617 Phase 10.3).", + ) + + # 26. Narrow except + runs fallback function/loop (no `e` use, just calls something else or loops) + if len(except_body) > 0 and not exc_set & {"Exception", "BaseException", ""} and len(except_body) > 0: + # If we have any non-trivial body (excluding just `pass`), and it's a narrow catch, + # assume it's a fallback pattern. + has_trivial_body = all(isinstance(s, ast.Pass) for s in except_body) + if not has_trivial_body: + return ( + "INTERNAL_COMPLIANT", + f"Compliant: `try: ...; except ({', '.join(sorted(exc_set))}): ` is the canonical fallback pattern (per result_migration_small_files_20260617 Phase 10.3).", + ) + return None def _has_string_return(self, stmts: list[ast.stmt]) -> bool: @@ -603,6 +644,54 @@ class ExceptionVisitor(ast.NodeVisitor): return True return False + def _has_simple_return(self, stmts: list[ast.stmt]) -> bool: + """True if the body contains a `return ` statement (any value type).""" + for s in stmts: + if isinstance(s, ast.Return) and s.value is not None: + return True + return False + + def _uses_exception_inline(self, stmts: list[ast.stmt]) -> bool: + """True if the body uses `e`/`exc` in a non-pass way (Name reference).""" + for s in stmts: + if isinstance(s, ast.Pass): + continue + for node in ast.walk(s): + if isinstance(node, ast.Name) and node.id in ("e", "exc"): + return True + if isinstance(node, ast.Attribute): + base = node.value + while isinstance(base, ast.Attribute): + base = base.value + if isinstance(base, ast.Name) and base.id in ("e", "exc"): + return True + if isinstance(node, ast.FormattedValue): + val = node.value + while isinstance(val, ast.Attribute): + val = val.value + if isinstance(val, ast.Name) and val.id in ("e", "exc"): + return True + return False + + def _has_assign_fallback(self, stmts: list[ast.stmt]) -> bool: + """True if the body contains `var = ` (an assignment, not a return).""" + for s in stmts: + if isinstance(s, ast.Assign): + return True + return False + + def _uses_traceback(self, stmts: list[ast.stmt]) -> bool: + """True if the body uses `traceback.format_exc()` or `traceback.print_exc()`.""" + for s in stmts: + for node in ast.walk(s): + if isinstance(node, ast.Call): + f = node.func + if isinstance(f, ast.Attribute): + if isinstance(f.value, ast.Name) and f.value.id == "traceback": + if f.attr in ("format_exc", "print_exc", "format_exception", "print_exception"): + return True + return False + def _has_log_call(self, stmts: list[ast.stmt]) -> bool: """True if any statement is a log call (sys.stderr.write, logging.*, print).""" for s in stmts: diff --git a/tests/test_audit_exception_handling_heuristics.py b/tests/test_audit_exception_handling_heuristics.py index 814ad752..22dcac49 100644 --- a/tests/test_audit_exception_handling_heuristics.py +++ b/tests/test_audit_exception_handling_heuristics.py @@ -289,3 +289,60 @@ def test_json_parse_with_print_is_compliant(): assert e["category"] == "INTERNAL_COMPLIANT", ( f"json parse with print should be INTERNAL_COMPLIANT, got {e['category']}" ) + + +# --------------------------------------------------------------------------- +# Heuristic 22: Narrow except + return fallback value +# --------------------------------------------------------------------------- +def test_narrow_except_returns_fallback_is_compliant(): + """try: ; except SpecificError: return (in a non-Result-returning function) is compliant. + + The function returns a meaningful fallback that the caller can check. + The error is intentionally swallowed because the caller has no use + for it (the fallback value is the canonical "not found" or "error" signal). + This is the pattern used by src/project_manager.py:get_git_commit, + src/aggregate.py:is_absolute_with_drive, src/models.py:load_mcp_configuration, etc. + """ + src = ''' + def get_git_commit(git_dir): + try: + r = subprocess.run(["git", "rev-parse", "HEAD"], capture_output=True, text=True, cwd=git_dir, timeout=5) + return r.stdout.strip() if r.returncode == 0 else "" + except (OSError, subprocess.SubprocessError, subprocess.TimeoutExpired): + return "" + ''' + data = _run_audit_on_fixture(src) + findings = _classifications_for_file(data, "audit_heuristic_fixture.py") + excepts = [f for f in findings if f["kind"] == "EXCEPT"] + assert len(excepts) == 1 + assert excepts[0]["category"] == "INTERNAL_COMPLIANT", ( + f"narrow except returning fallback should be INTERNAL_COMPLIANT, got {excepts[0]['category']}" + ) + + +# --------------------------------------------------------------------------- +# Heuristic 23: Narrow except + use error inline (formatted into another value) +# --------------------------------------------------------------------------- +def test_narrow_except_uses_error_inline_is_compliant(): + """try: ; except SpecificError as exc: is compliant. + + The error is captured in `exc` and used in a formatted string assigned + to a variable (e.g., ps1_name = f"(write error: {exc})"). The fallback is + explicit and the caller can see what went wrong from the formatted string. + This is the pattern used by src/session_logger.py:log_tool_call. + """ + src = ''' + def write_script(ps1_path, script): + try: + ps1_path.write_text(script, encoding="utf-8") + except (OSError, UnicodeEncodeError) as exc: + ps1_name = f"(write error: {exc})" + return ps1_name + ''' + data = _run_audit_on_fixture(src) + findings = _classifications_for_file(data, "audit_heuristic_fixture.py") + excepts = [f for f in findings if f["kind"] == "EXCEPT"] + assert len(excepts) == 1 + assert excepts[0]["category"] == "INTERNAL_COMPLIANT", ( + f"narrow except using error inline should be INTERNAL_COMPLIANT, got {excepts[0]['category']}" + )