From 3d201c35615d6b32d0660451a836aa0c0a72aa61 Mon Sep 17 00:00:00 2001 From: DLANSAMA <258674612+DLANSAMA@users.noreply.github.com> Date: Sat, 7 Nov 2026 16:19:23 +0000 Subject: [PATCH 1/2] fix: keep test doubles out of the shipped package and refresh stale quality numbers patched_logger imported unittest.mock inside bambu_cli; it moves into the one test that used it. namespace_get and _host_of keep their defensive behaviour with docstrings that describe the real inputs. The CI grep now rejects unittest, pytest and mock imports in bambu_cli/, and tests/test_production_purity.py checks the same on the AST. Roadmap, backlog and README coverage and test counts re-measured. --- .github/workflows/ci.yml | 2 +- CHANGELOG.md | 4 ++ CONTRIBUTING.md | 3 +- README.md | 4 +- bambu_cli/argutils.py | 2 +- bambu_cli/logging_utils.py | 21 ++------- bambu_cli/netsafety.py | 5 +-- docs/quality-roadmap.md | 22 +++++----- docs/test-backlog.md | 8 ++-- tests/test_primitives_helpers.py | 16 ++++++- tests/test_production_purity.py | 75 ++++++++++++++++++++++++++++++++ 11 files changed, 120 insertions(+), 42 deletions(-) create mode 100644 tests/test_production_purity.py diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 0e4f2b5..b9a4a11 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -210,7 +210,7 @@ jobs: run: uvx mypy@2.3.0 -p bambu_cli - name: no test-awareness in production code (blocking) run: | - if grep -rnE 'isinstance\([^)]*\bMock\b|["'"'"']unittest["'"'"'] in sys\.modules' bambu_cli/; then + if grep -rnE 'isinstance\([^)]*\bMock\b|["'"'"']unittest["'"'"'] in sys\.modules|^\s*(import|from)\s+(unittest|pytest|mock)\b' bambu_cli/; then echo "::error::Test-awareness branch found in production code (see docs/test-backlog.md ground rules)." exit 1 fi diff --git a/CHANGELOG.md b/CHANGELOG.md index 38b673b..9113eb2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,10 @@ Format follows [Keep a Changelog](https://keepachangelog.com/en/1.1.0/); version ## [Unreleased] +### Changed + +- `bambu_cli.logging_utils.patched_logger` is removed: it imported `unittest.mock` in the shipped package. `set_logger` / `reset_logger` still swap the logger backend. CI now rejects any `unittest`, `pytest` or `mock` import in `bambu_cli/`. + ## [0.6.0] - 2026-11-06 ### Printer safety diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index b861b66..dd56030 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -54,7 +54,8 @@ that CI's clean checkout does not have — expected locally, not a failure to ch - **Mutation baseline** (safety modules): `./scripts/run_mutation_baseline.sh` — [docs/mutation-baseline.md](docs/mutation-baseline.md). Nightly CI only (when `main` has changed), not every PR. - CI enforces a "no test-awareness in production code" rule: production modules must not - branch on `Mock` / `unittest in sys.modules`. Prefer injecting collaborators over + branch on `Mock` / `unittest in sys.modules` or import `unittest` / `pytest` / `mock` + (`tests/test_production_purity.py` checks the same on the AST). Prefer injecting collaborators over patching module globals. Domain code raises `BambuError` / `abort`; `sys.exit` only in `bambu_cli/cli.py`. diff --git a/README.md b/README.md index bbb7bbb..4f064ec 100644 --- a/README.md +++ b/README.md @@ -13,7 +13,7 @@ -[![Tests](https://img.shields.io/badge/tests-2217%20passing-brightgreen)](#tested-and-hardened) +[![Tests](https://img.shields.io/badge/tests-2226%20passing-brightgreen)](#tested-and-hardened) [![Coverage](https://img.shields.io/badge/coverage-93%25-brightgreen)](#tested-and-hardened) [Install](#install) · [Print something](#print-something) · [Try it without a printer](#try-it-in-30-seconds) · [Quality](#tested-and-hardened) · [User guide](https://github.com/DLANSAMA/platecli/blob/main/docs/manual.md) · [Troubleshooting](https://github.com/DLANSAMA/platecli/blob/main/docs/troubleshooting.md) · [For AI agents](#built-for-ai-agents) @@ -160,7 +160,7 @@ The numbers in the badges are measured on this repo, not aspirational: | | | |---|---| | **Tests** | 2,217 passing (`pytest -m "not live"`, v0.6.0) — unit, contract, property-based (Hypothesis), and Textual pilot tests | -| **Coverage** | **93.5%** line+branch across the package; CI fails the build below **90%** | +| **Coverage** | **93.6%** line+branch across the package; CI fails the build below **90%** | | **Platforms** | Linux, macOS, and Windows in CI, on Python 3.10 / 3.12 / 3.14 | | **Blocking gates** | `ruff`, `ruff format`, `mypy` (whole package, `check_untyped_defs`), `bandit`, `pip-audit`, plus layer-boundary and JSON-schema-drift checks | | **Mutation testing** | Nightly whenever `main` has changed, scoped to the safety-critical modules (download validation, SSRF guards, slicer options, job payload) | diff --git a/bambu_cli/argutils.py b/bambu_cli/argutils.py index e338232..32c2e31 100644 --- a/bambu_cli/argutils.py +++ b/bambu_cli/argutils.py @@ -16,7 +16,7 @@ def namespace_get(args, name, default=None): - """Read argparse.Namespace values without treating MagicMock attributes as set.""" + """Read a value from an argparse.Namespace; ``default`` for anything without a ``__dict__``.""" try: return vars(args).get(name, default) except TypeError: diff --git a/bambu_cli/logging_utils.py b/bambu_cli/logging_utils.py index 5b93616..7515518 100644 --- a/bambu_cli/logging_utils.py +++ b/bambu_cli/logging_utils.py @@ -1,7 +1,7 @@ """Process-wide logger used by production modules. -``LoggerProxy`` always delegates to a replaceable backend so tests can install -a ``MagicMock`` (via ``set_logger`` / the ``mock_bambu_logger`` fixture) without +``LoggerProxy`` always delegates to a replaceable backend. A caller (a test, an +embedding application) installs its own logger with ``set_logger`` without patching ``bambu_cli.bambu.logger`` or each consumer module's import binding. """ @@ -9,7 +9,6 @@ import logging import sys -from contextlib import contextmanager from typing import Any _BACKEND: Any = logging.getLogger("bambu") @@ -31,7 +30,7 @@ def __getattr__(self, name: str) -> Any: def set_logger(backend: Any) -> None: - """Replace the process logger backend (tests pass a ``MagicMock``).""" + """Replace the process logger backend with any object that has the logging methods.""" global _BACKEND _BACKEND = backend @@ -42,20 +41,6 @@ def reset_logger() -> None: _BACKEND = logging.getLogger("bambu") -@contextmanager -def patched_logger(backend: Any | None = None): - """Context manager that installs ``backend`` (default: ``MagicMock``) for the block.""" - from unittest.mock import MagicMock - - mock = backend if backend is not None else MagicMock() - prev = _BACKEND - set_logger(mock) - try: - yield mock - finally: - set_logger(prev) - - def safe_log_error(message: Any, **kwargs: Any) -> None: """Log an error without ever letting the logging layer abort the process. diff --git a/bambu_cli/netsafety.py b/bambu_cli/netsafety.py index 9c14232..2ad211a 100644 --- a/bambu_cli/netsafety.py +++ b/bambu_cli/netsafety.py @@ -313,9 +313,8 @@ def _default_user_agent() -> str: def _host_of(url) -> str: """Best-effort lowercase hostname. Never raises. - Defensive on purpose: callers pass ``req.full_url``, and the test suite - patches ``urllib.request.Request`` with a MagicMock whose ``full_url`` is - not a string. ``urlparse`` on such a value raises ``TypeError``. Returning + Defensive on purpose: callers pass ``req.full_url``, which is not always a + string, and ``urlparse`` on a non-string raises ``TypeError``. Returning "" means "no host policy, no throttling", which is the safe degradation. """ try: diff --git a/docs/quality-roadmap.md b/docs/quality-roadmap.md index 084db87..b4cb25f 100644 --- a/docs/quality-roadmap.md +++ b/docs/quality-roadmap.md @@ -15,13 +15,13 @@ Historical baseline (do not read as current), from the audit + full `pytest --cov=bambu_cli` on 2026-07-08: **368 tests**, **78%** line coverage (1105 / 4973 stmts missed), **130** `sys.exit` sites in `bambu_cli/`, **7** `@mockable` sites (def + 6 uses), **1** `BambuError` raise in production. -The "Baseline" column below is that snapshot. **Current measured (2026-08-13, -PR #119): 1499 passed / 1 live deselected, 90.99% branch coverage over 8368 -statements (local Linux).** Full CI matrix: Linux 90.99% / Windows 90.68% (still -the binding leg) / macOS passing, on the 3.10 / 3.12 / 3.14 matrix (Python 3.9 -was dropped in #115). The prior `5b08720` matrix (run `31044588411`, 2026-08-05) -measured 88.8–89.3%; CI runs a clean checkout, so its numbers differ slightly -from a local run — do not reconcile one to the other. +The "Baseline" column below is that snapshot. The current local measurement is in +the "Scoreboard (current)" table below. The last full CI-matrix measurement +(2026-08-13, PR #119) was Linux 90.99% / Windows 90.68% (the binding leg) / +macOS passing, on the 3.10 / 3.12 / 3.14 matrix (Python 3.9 was dropped in +#115). CI runs a clean checkout, so its numbers differ slightly from a local +run — do not reconcile one to the other. Windows has not been re-measured +since, so raising the CI floor waits on a fresh matrix reading. | Area | Baseline | Gate to A | Gate to A+ | Primary evidence | |------|----------|-----------|------------|------------------| @@ -60,15 +60,15 @@ items are **accepted 1.0 residuals** in [SECURITY.md](../SECURITY.md), not open | Correctness / bugs | **A+** | No known dead flags; property tests on filenames/SSRF/ZIP/temps in `tests/test_properties_safety.py` | | Typing | **A** | Full-package mypy + `check_untyped_defs`. A+ (`mypy --strict`) is **1521** errors — not this release | | Error model | **A+** | `sys.exit` only in `cli.py`; `emit_json_error` is `NoReturn`; domain uses `abort` / `BambuError` | -| Tests | **A+** | **2217** non-live tests passing (2026-11-05, local Linux; 1 skipped, 1 live deselected). A column cleared. Measured **93.5%** branch coverage over 9446 statements; A+ threshold met | +| Tests | **A+** | **2226** non-live tests passing (2026-11-07, local Linux; 1 skipped, 1 live deselected). A column cleared. Measured **93.6%** branch coverage over 9436 statements; A+ threshold met | | CI / release | **A** | `--cov-fail-under=90` (floor raised 86→90 with comfortable margin). A+ is fail-under 92 | | Docs / governance | **A+** | Roadmap + backlog match this measurement; stability policy + support matrix in `docs/api.md`; docs-consistency pins floor / % / count | | Product polish | **A−** | Support matrix + hermetic Orca stub; still **Beta / pre-1.0** (no Production/Stable, no v1.0.0 tag). A+ is the §5 checklist | -**Overall:** **A** — none below A−. **Not A+ across the board.** Remaining: coverage 91.0→92, CI floor 92, Typing strict, Product `v1.0.0` tag. +**Overall:** **A** — none below A−. **Not A+ across the board.** Remaining: CI floor 92 (needs a fresh Windows reading and more `ftps.py` / `netsafety.py` coverage first, see the per-module targets in §3.1), Typing strict, Product `v1.0.0` tag. -**Coverage floor history:** 79 (honest post-Phase-1 gate) → **81** (2026-07-09) → **83** (2026-07-26) → **86** (2026-08-13) → **90** (2026-09-16; measured 92.8%). -Measured branch total is **92.8%** on local Linux; the floor is set +**Coverage floor history:** 79 (honest post-Phase-1 gate) → **81** (2026-07-09) → **83** (2026-07-26) → **86** (2026-08-13) → **90** (2026-09-16; measured 92.8% then, 93.6% on 2026-11-07). +Measured branch total is **93.6%** on local Linux (2026-11-07); the floor is set at the multi-OS minimum so the matrix does not flake while still denying points of silent rot vs the old 79 gate. diff --git a/docs/test-backlog.md b/docs/test-backlog.md index bc89a6d..50b4c78 100644 --- a/docs/test-backlog.md +++ b/docs/test-backlog.md @@ -6,15 +6,15 @@ This file is a short **remaining-gaps** list only. Refresh after each phase or audit. Do not treat historical “≥98% coverage” claims as current — see the snapshot below. -## Snapshot (2026-09-16; measured on Linux) +## Snapshot (2026-11-07; measured on Linux) | Metric | Current (honest) | A+ / 1.0 target | |--------|------------------|-----------------| -| Non-live tests collected | **2217** passing (measured 2026-11-05 on Linux; 1 skipped, 1 live test deselected) | ≥550 with zero known flakes ✅ size | -| Line/branch coverage (CI) | **92.8%** branch coverage over 8396 statements (local Linux, 2026-09-16; Windows / macOS passing on CI); prior CI matrix on `5b08720` was 88.8–89.3%; **floor 90** | **≥92%** total; optional module floors | +| Non-live tests collected | **2226** passing (measured 2026-11-07 on Linux; 1 skipped, 1 live test deselected) | ≥550 with zero known flakes ✅ size | +| Line/branch coverage (CI) | **93.6%** branch coverage over 9436 statements (local Linux, 2026-11-07; Windows / macOS were passing on the last CI matrix); prior CI matrix on `5b08720` was 88.8–89.3%; **floor 90** | **≥92%** total; optional module floors | | Typing | Full package mypy + `check_untyped_defs` | keep; optional full `strict` later | | Error model | `sys.exit` only in `cli.py` | keep | -| `@mockable` / test-awareness | **0** (CI greps) | keep | +| `@mockable` / test-awareness | **0** (CI greps plus `tests/test_production_purity.py`, which also rejects `unittest` / `pytest` / `Mock` imports) | keep | | JSON schemas | **27** files under `docs/schemas/`, **generated** from `bambu_cli/contracts/`; coverage is derived from `build_parser()`, so a new subcommand cannot ship schema-less | monitor goldens; field-level api.md ↔ schema sync | | Mutation baseline | Pure safety modules; score **50.7%** measured 2026-08-04, CI floor raised 40 → **48** | the C.4 re-run happened and **disproved** the prediction: `slicer/output.py` stayed at 21.8% while its line coverage went 79.8% → 92.7%. See [mutation-baseline.md](mutation-baseline.md); raising that row needs a production refactor, not more tests | | Live printer | Documented opt-in harness | manual pre-release (optional scheduled lab) | diff --git a/tests/test_primitives_helpers.py b/tests/test_primitives_helpers.py index 7689ae5..c887645 100644 --- a/tests/test_primitives_helpers.py +++ b/tests/test_primitives_helpers.py @@ -3,16 +3,30 @@ from __future__ import annotations from argparse import Namespace +from contextlib import contextmanager from types import SimpleNamespace from bambu_cli.argutils import exit_code_from_system_exit, namespace_get, setup_args_provided from bambu_cli.constants import EXIT_COMMAND_ERROR, EXIT_SUCCESS from bambu_cli.errors import BambuError from bambu_cli.job.support import _exit_code_from_error, _last_error_for -from bambu_cli.logging_utils import patched_logger, reset_logger, set_logger +from bambu_cli.logging_utils import reset_logger, set_logger from bambu_cli.paths import display_path, exception_for_message, expand_path, path_for_message +@contextmanager +def patched_logger(backend): + """Install ``backend`` as the process logger for the block, then restore it.""" + from bambu_cli import logging_utils + + prev = logging_utils._BACKEND + set_logger(backend) + try: + yield backend + finally: + set_logger(prev) + + def test_namespace_get_reads_vars_and_falls_back_on_typeerror(): assert namespace_get(Namespace(serial="SN"), "serial") == "SN" assert namespace_get(Namespace(), "missing", "d") == "d" diff --git a/tests/test_production_purity.py b/tests/test_production_purity.py new file mode 100644 index 0000000..778fe1d --- /dev/null +++ b/tests/test_production_purity.py @@ -0,0 +1,75 @@ +"""Production code must not know it is being tested. + +The CI grep catches the two classic shapes (``isinstance(..., Mock)`` and +``"unittest" in sys.modules``). This test closes the rest on the AST, so a +docstring or comment may still *mention* a test double but code cannot import +or name one: no import of ``unittest`` / ``pytest`` / ``mock``, no use of a +``Mock`` class, and no ``sys.modules`` probe for a test framework. +""" + +from __future__ import annotations + +import ast +from pathlib import Path + +import pytest + +PKG = Path(__file__).resolve().parent.parent / "bambu_cli" + +TEST_MODULES = {"unittest", "pytest", "mock", "_pytest"} +TEST_NAMES = {"Mock", "MagicMock", "AsyncMock", "NonCallableMock", "PropertyMock", "patch", "create_autospec"} + + +def _is_modules_probe(node: ast.Constant) -> bool: + parent = getattr(node, "_parent", None) + return isinstance(parent, (ast.Compare, ast.Subscript)) + + +def _scan(source: str) -> list[str]: + tree = ast.parse(source) + for parent in ast.walk(tree): + for child in ast.iter_child_nodes(parent): + child._parent = parent # type: ignore[attr-defined] + found: list[str] = [] + for node in ast.walk(tree): + if isinstance(node, ast.Import): + found += [f"import {a.name}" for a in node.names if a.name.split(".")[0] in TEST_MODULES] + elif isinstance(node, ast.ImportFrom): + if node.module and node.module.split(".")[0] in TEST_MODULES: + found.append(f"from {node.module} import ...") + elif isinstance(node, ast.Name) and node.id in TEST_NAMES: + found.append(f"name {node.id}") + elif isinstance(node, ast.Attribute) and node.attr in TEST_NAMES: + found.append(f"attribute {node.attr}") + elif isinstance(node, ast.Constant) and node.value in TEST_MODULES and _is_modules_probe(node): + found.append(f"probe for {node.value!r}") + return found + + +def test_no_production_module_references_a_test_framework(): + bad = {} + for path in sorted(PKG.rglob("*.py")): + hits = _scan(path.read_text(encoding="utf-8")) + if hits: + bad[str(path.relative_to(PKG.parent))] = hits + assert not bad, f"test-awareness in production code: {bad}" + + +@pytest.mark.parametrize( + "source", + [ + "import unittest", + "from unittest.mock import MagicMock", + "import pytest", + "x = MagicMock()", + "import unittest.mock as m\nm.patch('a')", + "import sys\nif 'unittest' in sys.modules: pass", + "import sys\nsys.modules['pytest']", + ], +) +def test_scanner_flags_the_shapes_it_exists_to_catch(source): + assert _scan(source) + + +def test_scanner_ignores_docstrings_and_comments(): + assert not _scan('"""Tests may install a MagicMock here."""\n# pytest capture\nx = 1\n') From 8ee948004cd903f2eab0002445a98231e30596a9 Mon Sep 17 00:00:00 2001 From: DLANSAMA <258674612+DLANSAMA@users.noreply.github.com> Date: Sat, 7 Nov 2026 16:34:40 +0000 Subject: [PATCH 2/2] test: cover the remaining ftps and netsafety edge paths Default-argument handling in ImplicitFTPS.connect, cleanup when a socket or file will not close, the unprotected data channel, the redirect handler's header rules, the HTTPS handler's optional context and the throttle table's size cap. Both modules reach 100% branch coverage. Roadmap, backlog and README numbers re-measured: 2239 tests, 93.8%. --- README.md | 4 +- docs/quality-roadmap.md | 8 +- docs/test-backlog.md | 4 +- tests/test_transport_edges.py | 272 ++++++++++++++++++++++++++++++++++ 4 files changed, 280 insertions(+), 8 deletions(-) create mode 100644 tests/test_transport_edges.py diff --git a/README.md b/README.md index 4f064ec..cfa6e0a 100644 --- a/README.md +++ b/README.md @@ -13,7 +13,7 @@ -[![Tests](https://img.shields.io/badge/tests-2226%20passing-brightgreen)](#tested-and-hardened) +[![Tests](https://img.shields.io/badge/tests-2239%20passing-brightgreen)](#tested-and-hardened) [![Coverage](https://img.shields.io/badge/coverage-93%25-brightgreen)](#tested-and-hardened) [Install](#install) · [Print something](#print-something) · [Try it without a printer](#try-it-in-30-seconds) · [Quality](#tested-and-hardened) · [User guide](https://github.com/DLANSAMA/platecli/blob/main/docs/manual.md) · [Troubleshooting](https://github.com/DLANSAMA/platecli/blob/main/docs/troubleshooting.md) · [For AI agents](#built-for-ai-agents) @@ -160,7 +160,7 @@ The numbers in the badges are measured on this repo, not aspirational: | | | |---|---| | **Tests** | 2,217 passing (`pytest -m "not live"`, v0.6.0) — unit, contract, property-based (Hypothesis), and Textual pilot tests | -| **Coverage** | **93.6%** line+branch across the package; CI fails the build below **90%** | +| **Coverage** | **93.8%** line+branch across the package; CI fails the build below **90%** | | **Platforms** | Linux, macOS, and Windows in CI, on Python 3.10 / 3.12 / 3.14 | | **Blocking gates** | `ruff`, `ruff format`, `mypy` (whole package, `check_untyped_defs`), `bandit`, `pip-audit`, plus layer-boundary and JSON-schema-drift checks | | **Mutation testing** | Nightly whenever `main` has changed, scoped to the safety-critical modules (download validation, SSRF guards, slicer options, job payload) | diff --git a/docs/quality-roadmap.md b/docs/quality-roadmap.md index b4cb25f..f7a4617 100644 --- a/docs/quality-roadmap.md +++ b/docs/quality-roadmap.md @@ -60,15 +60,15 @@ items are **accepted 1.0 residuals** in [SECURITY.md](../SECURITY.md), not open | Correctness / bugs | **A+** | No known dead flags; property tests on filenames/SSRF/ZIP/temps in `tests/test_properties_safety.py` | | Typing | **A** | Full-package mypy + `check_untyped_defs`. A+ (`mypy --strict`) is **1521** errors — not this release | | Error model | **A+** | `sys.exit` only in `cli.py`; `emit_json_error` is `NoReturn`; domain uses `abort` / `BambuError` | -| Tests | **A+** | **2226** non-live tests passing (2026-11-07, local Linux; 1 skipped, 1 live deselected). A column cleared. Measured **93.6%** branch coverage over 9436 statements; A+ threshold met | +| Tests | **A+** | **2239** non-live tests passing (2026-11-07, local Linux; 1 skipped, 1 live deselected). A column cleared. Measured **93.8%** branch coverage over 9436 statements; A+ threshold met. `protocols/ftps.py` and `netsafety.py` are at 100% (`tests/test_transport_edges.py`) | | CI / release | **A** | `--cov-fail-under=90` (floor raised 86→90 with comfortable margin). A+ is fail-under 92 | | Docs / governance | **A+** | Roadmap + backlog match this measurement; stability policy + support matrix in `docs/api.md`; docs-consistency pins floor / % / count | | Product polish | **A−** | Support matrix + hermetic Orca stub; still **Beta / pre-1.0** (no Production/Stable, no v1.0.0 tag). A+ is the §5 checklist | -**Overall:** **A** — none below A−. **Not A+ across the board.** Remaining: CI floor 92 (needs a fresh Windows reading and more `ftps.py` / `netsafety.py` coverage first, see the per-module targets in §3.1), Typing strict, Product `v1.0.0` tag. +**Overall:** **A** — none below A−. **Not A+ across the board.** Remaining: CI floor 92 (needs a fresh Windows reading first; the `ftps.py` / `netsafety.py` per-module targets in §3.1 are now met), Typing strict, Product `v1.0.0` tag. -**Coverage floor history:** 79 (honest post-Phase-1 gate) → **81** (2026-07-09) → **83** (2026-07-26) → **86** (2026-08-13) → **90** (2026-09-16; measured 92.8% then, 93.6% on 2026-11-07). -Measured branch total is **93.6%** on local Linux (2026-11-07); the floor is set +**Coverage floor history:** 79 (honest post-Phase-1 gate) → **81** (2026-07-09) → **83** (2026-07-26) → **86** (2026-08-13) → **90** (2026-09-16; measured 92.8% then, 93.8% on 2026-11-07). +Measured branch total is **93.8%** on local Linux (2026-11-07); the floor is set at the multi-OS minimum so the matrix does not flake while still denying points of silent rot vs the old 79 gate. diff --git a/docs/test-backlog.md b/docs/test-backlog.md index 50b4c78..c497c20 100644 --- a/docs/test-backlog.md +++ b/docs/test-backlog.md @@ -10,8 +10,8 @@ Do not treat historical “≥98% coverage” claims as current — see the snap | Metric | Current (honest) | A+ / 1.0 target | |--------|------------------|-----------------| -| Non-live tests collected | **2226** passing (measured 2026-11-07 on Linux; 1 skipped, 1 live test deselected) | ≥550 with zero known flakes ✅ size | -| Line/branch coverage (CI) | **93.6%** branch coverage over 9436 statements (local Linux, 2026-11-07; Windows / macOS were passing on the last CI matrix); prior CI matrix on `5b08720` was 88.8–89.3%; **floor 90** | **≥92%** total; optional module floors | +| Non-live tests collected | **2239** passing (measured 2026-11-07 on Linux; 1 skipped, 1 live test deselected) | ≥550 with zero known flakes ✅ size | +| Line/branch coverage (CI) | **93.8%** branch coverage over 9436 statements (local Linux, 2026-11-07; Windows / macOS were passing on the last CI matrix); prior CI matrix on `5b08720` was 88.8–89.3%; **floor 90** | **≥92%** total; optional module floors | | Typing | Full package mypy + `check_untyped_defs` | keep; optional full `strict` later | | Error model | `sys.exit` only in `cli.py` | keep | | `@mockable` / test-awareness | **0** (CI greps plus `tests/test_production_purity.py`, which also rejects `unittest` / `pytest` / `Mock` imports) | keep | diff --git a/tests/test_transport_edges.py b/tests/test_transport_edges.py new file mode 100644 index 0000000..537de40 --- /dev/null +++ b/tests/test_transport_edges.py @@ -0,0 +1,272 @@ +"""Edge paths in the FTPS transport and the SSRF layer that the main suites skip. + +Roadmap section 3.1 sets per-module targets for ``protocols/ftps.py`` and +``netsafety.py``. These tests cover what was left: default-argument handling in +``ImplicitFTPS.connect``, cleanup when a socket or file will not close, the +non-TLS data-channel branch, the redirect handler's header rules, the HTTPS +handler's optional context, and the per-host throttle table's size cap. Nothing +here opens a socket. +""" + +from __future__ import annotations + +import ftplib +import ipaddress +import socket +import ssl +import types +import urllib.error +import urllib.request +from unittest.mock import Mock + +import pytest + +from bambu_cli import netsafety +from bambu_cli.protocols import ftps + +_DER = b"\x30\x82edge-case-der" + + +# --------------------------------------------------------------------------- +# protocols/ftps.py +# --------------------------------------------------------------------------- +def test_sim_ftp_voidcmd_acknowledges(): + assert ftps._SimFtp().voidcmd("NOOP") == "200 OK" + + +class _FakeTlsSock: + """A wrapped socket that records closes and can refuse to close.""" + + def __init__(self, *, close_error=None, file=None): + self.family = socket.AF_INET + self.closed = False + self._close_error = close_error + self._file = file + + def getpeercert(self, binary_form=False): + return _DER + + def makefile(self, mode, encoding=None): + return self._file + + def close(self): + self.closed = True + if self._close_error: + raise self._close_error + + +class _FakeContext: + def __init__(self, tls): + self._tls = tls + self.check_hostname = None + self.verify_mode = None + + def load_default_certs(self): + pass + + def wrap_socket(self, sock, server_hostname=None): + return self._tls + + +def _plain_printer(): + return types.SimpleNamespace(cert_fingerprint=None, insecure_tls=True) + + +def test_connect_keeps_preset_host_port_and_timeout_when_given_defaults(): + """host='', port<=0 and the -999 timeout sentinel mean 'keep what is set'.""" + seen = {} + + def create_connection(address, timeout): + seen["address"] = address + seen["timeout"] = timeout + return _FakeTlsSock() + + tls = _FakeTlsSock(file=Mock()) + ftp = ftps.ImplicitFTPS() + ftp.printer = _plain_printer() + ftp.host = "printer.local" + ftp.port = 990 + ftp.getresp = lambda: "220 hi" # type: ignore[method-assign] + + welcome = ftp.connect("", 0, create_connection=create_connection, ssl_context_cls=lambda proto: _FakeContext(tls)) + + assert welcome == "220 hi" + assert seen["address"] == ("printer.local", 990) + assert seen["timeout"] == ftp.timeout + + +def test_connect_failure_survives_files_and_sockets_that_will_not_close(): + """The handshake error is what the caller sees, not a secondary close error.""" + bad_file = Mock() + bad_file.close.side_effect = OSError("file close failed") + tls = _FakeTlsSock(close_error=OSError("socket close failed"), file=bad_file) + + ftp = ftps.ImplicitFTPS() + ftp.printer = _plain_printer() + + def refuse_greeting(): + raise ftplib.error_temp("421 try later") + + ftp.getresp = refuse_greeting # type: ignore[method-assign] + + with pytest.raises(ftplib.error_temp, match="421"): + ftp.connect( + "printer.local", + 990, + 5, + create_connection=lambda address, timeout: _FakeTlsSock(), + ssl_context_cls=lambda proto: _FakeContext(tls), + ) + + bad_file.close.assert_called_once() + assert tls.closed is True + + +def _data_channel_client(printer): + client = ftps.ImplicitFTPS.__new__(ftps.ImplicitFTPS) + client._prot_p = True + client.host = "printer.local" + client.printer = printer + client.sock = Mock(spec=ssl.SSLSocket) + client.sock.session = None + return client + + +def test_ntransfercmd_returns_the_raw_channel_when_the_session_is_not_protected(monkeypatch): + client = _data_channel_client(_plain_printer()) + client._prot_p = False + raw = Mock() + monkeypatch.setattr(ftplib.FTP, "ntransfercmd", lambda self, cmd, rest=None: (raw, 7)) + + conn, size = client.ntransfercmd("RETR x") + + assert conn is raw and size == 7 + client.sock.context.wrap_socket.assert_not_called() + + +def test_ntransfercmd_wrap_failure_closes_the_data_socket_and_reraises(monkeypatch): + client = _data_channel_client(_plain_printer()) + raw = Mock() + raw.close.side_effect = OSError("already detached") + client.sock.context.wrap_socket.side_effect = ssl.SSLError("handshake failed") + monkeypatch.setattr(ftplib.FTP, "ntransfercmd", lambda self, cmd, rest=None: (raw, 7)) + + with pytest.raises(ssl.SSLError, match="handshake failed"): + client.ntransfercmd("STOR x") + + raw.close.assert_called_once() + + +def test_ntransfercmd_pin_mismatch_reraises_even_if_the_socket_will_not_close(monkeypatch): + client = _data_channel_client(types.SimpleNamespace(cert_fingerprint="ab" * 32, insecure_tls=False)) + wrapped = Mock() + wrapped.getpeercert.return_value = _DER + wrapped.close.side_effect = OSError("close failed") + client.sock.context.wrap_socket.return_value = wrapped + monkeypatch.setattr(ftplib.FTP, "ntransfercmd", lambda self, cmd, rest=None: (Mock(), 7)) + + with pytest.raises(ssl.SSLError, match="fingerprint mismatch"): + client.ntransfercmd("STOR x") + + wrapped.close.assert_called_once() + + +# --------------------------------------------------------------------------- +# netsafety.py +# --------------------------------------------------------------------------- +@pytest.fixture(autouse=True) +def _clean_netsafety_state(): + netsafety._dns_cache.clear() + netsafety._last_request_at.clear() + yield + netsafety._dns_cache.clear() + netsafety._last_request_at.clear() + + +def test_allow_private_accepts_ipv6_loopback_and_unique_local(): + for text in ("::1", "fd00::1"): + assert netsafety._is_safe_ip(ipaddress.ip_address(text), allow_private=True) is True + assert netsafety._is_safe_ip(ipaddress.ip_address(text), allow_private=False) is False + + +def test_a_refused_address_is_reported_once_even_if_dns_returns_it_twice(monkeypatch): + entry = (socket.AF_INET, socket.SOCK_STREAM, 6, "", ("127.0.0.1", 443)) + monkeypatch.setattr(netsafety.socket, "getaddrinfo", lambda *a, **k: [entry, entry]) + + with pytest.raises(netsafety.BlockedAddressError) as excinfo: + netsafety._get_safe_connection("loop.example", 443, 5, None) + + assert str(excinfo.value).count("127.0.0.1") == 1 + + +class _ClosingRaises: + def close(self): + raise OSError("close failed") + + +def test_redirect_limit_error_is_not_masked_by_a_response_that_will_not_close(): + handler = netsafety.SafeHTTPRedirectHandler() + req = urllib.request.Request("https://example.com/start") + req._bambu_redirect_hops = netsafety.MAX_DOWNLOAD_REDIRECT_HOPS + + with pytest.raises(urllib.error.URLError, match="Too many redirects"): + handler.redirect_request(req, _ClosingRaises(), 302, "Found", {}, "https://example.com/next") + + +def test_cross_host_redirect_drops_credentials_and_adds_no_user_agent_that_was_never_set(): + handler = netsafety.SafeHTTPRedirectHandler() + req = urllib.request.Request("https://a.example/start", headers={"Authorization": "Bearer secret"}) + + new_req = handler.redirect_request(req, None, 302, "Found", {}, "https://b.example/next") + + assert new_req is not None + assert not new_req.has_header("Authorization") + assert not new_req.has_header("User-agent") + + +class _RecordingHttpsHandler(netsafety.SafeHTTPSHandler): + def do_open(self, http_class, req, **kwargs): + self.seen = kwargs + return None + + +def test_https_handler_passes_context_and_hostname_check_only_when_it_has_them(): + handler = _RecordingHttpsHandler() + for attr in ("_context", "_check_hostname"): + handler.__dict__.pop(attr, None) + req = urllib.request.Request("https://example.com/") + + handler.https_open(req) + assert handler.seen == {} + + ctx = ssl.create_default_context() + handler._context = ctx + handler.https_open(req) + assert handler.seen == {"context": ctx} + + handler._check_hostname = True + handler.https_open(req) + assert handler.seen == {"context": ctx, "check_hostname": True} + + +def _fill_throttle_table(age_seconds): + import time + + stamp = time.monotonic() - age_seconds + netsafety._last_request_at.update({f"host{i}.example": stamp for i in range(1000)}) + + +def test_throttle_table_drops_stale_hosts_when_it_reaches_the_cap(): + _fill_throttle_table(age_seconds=120) + + netsafety._throttle_host("fresh.example", sleep=lambda s: None) + + assert list(netsafety._last_request_at) == ["fresh.example"] + + +def test_throttle_table_is_cleared_when_every_host_is_recent(): + _fill_throttle_table(age_seconds=1) + + netsafety._throttle_host("fresh.example", sleep=lambda s: None) + + assert list(netsafety._last_request_at) == ["fresh.example"]