diff --git a/bambu_cli/commands/doctor.py b/bambu_cli/commands/doctor.py index d180426..b9a2b39 100644 --- a/bambu_cli/commands/doctor.py +++ b/bambu_cli/commands/doctor.py @@ -66,7 +66,6 @@ def _offer_pin_fingerprint( def cmd_doctor(args, ctx=None): """Health-check: auto-discover printer capabilities and verify configuration.""" - from bambu_cli.protocols.ftps import get_ftp from bambu_cli.protocols.mqtt import probe_cert_fingerprint from bambu_cli.utils import emit_json @@ -161,7 +160,7 @@ def shown_ip(): logger.info(f" [3/3] Verifying FTPS connectivity to {shown_ip()}:990...") try: - with get_ftp(printer, timeout=net_timeout): + with printer.get_ftp_client(timeout=net_timeout): logger.info(" ✅ FTPS connection established.") except Exception as e: message = f"FTPS connection failed: {e}" diff --git a/bambu_cli/config.py b/bambu_cli/config.py index 17de7cd..9a5b7a7 100644 --- a/bambu_cli/config.py +++ b/bambu_cli/config.py @@ -480,11 +480,9 @@ def load_username(): def _expected_fingerprint(): """Return the normalized (lowercase, separator-free) pinned SHA-256, or None.""" from bambu_cli.context import current_config + from bambu_cli.tlspin import normalize_fingerprint - fp = current_config().get("cert_fingerprint") - if not fp: - return None - return fp.lower().replace(":", "").replace(" ", "") + return normalize_fingerprint(current_config().get("cert_fingerprint")) def fingerprint_sha256(der_cert): diff --git a/bambu_cli/context.py b/bambu_cli/context.py index 505e7d7..aabccb4 100644 --- a/bambu_cli/context.py +++ b/bambu_cli/context.py @@ -72,13 +72,10 @@ def _coerce_insecure_tls(value: Any) -> bool: def _normalize_fingerprint(fp: str | None) -> str | None: - """Normalize a pinned SHA-256 fingerprint (lowercase, separator-free). + """Normalize a pinned SHA-256 fingerprint (lowercase, separator-free).""" + from bambu_cli.tlspin import normalize_fingerprint - Mirrors ``bambu_cli.config._expected_fingerprint``. - """ - if not fp: - return None - return fp.lower().replace(":", "").replace(" ", "") + return normalize_fingerprint(fp) @dataclass @@ -190,6 +187,7 @@ def printer(self) -> BambuPrinter: from bambu_cli.config import load_access_code from bambu_cli.printer import BambuPrinter + from bambu_cli.tlspin import normalize_fingerprint access_code = "" if self.simulation else load_access_code() self._printer = BambuPrinter( @@ -197,7 +195,7 @@ def printer(self) -> BambuPrinter: serial=self.settings.serial, access_code=access_code, insecure_tls=self.settings.insecure_tls, - cert_fingerprint=_normalize_fingerprint(self.settings.cert_fingerprint), + cert_fingerprint=normalize_fingerprint(self.settings.cert_fingerprint), simulation_mode=self.simulation, ) return self._printer diff --git a/bambu_cli/jsonio.py b/bambu_cli/jsonio.py index 3b3c8e6..4d83e1f 100644 --- a/bambu_cli/jsonio.py +++ b/bambu_cli/jsonio.py @@ -5,10 +5,9 @@ before they reach a log line or a JSON envelope, without importing from the CLI entrypoint. These helpers never terminate the process. -Note: ``bambu_cli.utils`` carries its own credential-redaction pass applied -uniformly across every emitted JSON payload; the ``redact_url_credentials`` -here is the eager, single-value variant callers use when building the strings -that go into those payloads and log messages. +This is the single redactor: command code calls it when building log lines +and payload fields, and ``bambu_cli.utils.emit_json`` runs the same function +over every emitted string. """ from urllib.parse import urlparse, urlunparse diff --git a/bambu_cli/printer.py b/bambu_cli/printer.py index b784830..516a61d 100644 --- a/bambu_cli/printer.py +++ b/bambu_cli/printer.py @@ -67,15 +67,12 @@ def get_ftp_client(self, timeout: Optional[float] = None): """Context manager to get a connected FTP client.""" if timeout is None: timeout = self.ftps_timeout - # We can implement pooling here in the future client = ftps_protocol._create_raw_ftp(self, timeout=timeout) try: yield client finally: - try: - client.quit() - except _FTP_SSL_ERRORS: - pass + # Bambu's FTPS control channel can hang in quit(); close() tears + # the socket down without waiting for the 221 reply. try: client.close() except _FTP_SSL_ERRORS: @@ -288,7 +285,8 @@ def get_printer(*, access_code_loader=None) -> BambuPrinter: inject a fake instead of patching module globals. """ from bambu_cli.config import load_access_code - from bambu_cli.context import _normalize_fingerprint, current_settings, current_simulation + from bambu_cli.context import current_settings, current_simulation + from bambu_cli.tlspin import normalize_fingerprint _load = access_code_loader if access_code_loader is not None else load_access_code settings = current_settings() @@ -300,6 +298,6 @@ def get_printer(*, access_code_loader=None) -> BambuPrinter: # require credentials (load_access_code exits when unconfigured). access_code="" if simulation_mode else _load(), insecure_tls=settings.insecure_tls, - cert_fingerprint=_normalize_fingerprint(settings.cert_fingerprint), + cert_fingerprint=normalize_fingerprint(settings.cert_fingerprint), simulation_mode=simulation_mode, ) diff --git a/bambu_cli/protocols/ftps.py b/bambu_cli/protocols/ftps.py index 391af2d..9bd9f41 100644 --- a/bambu_cli/protocols/ftps.py +++ b/bambu_cli/protocols/ftps.py @@ -1,25 +1,13 @@ -import atexit import ftplib import os import socket import ssl -import threading from typing import Any from bambu_cli.utils import _resolve_ip _SIM_FTP_FILES = {"simulated_file.3mf": 1000} -# Module-level so mypy accepts these in `except` clauses (star-unpack of -# ftplib.all_errors inline is rejected as non-exception-type). -_FTP_ERRORS: tuple[type[BaseException], ...] = ftplib.all_errors -_FTP_SSL_ERRORS: tuple[type[BaseException], ...] = ftplib.all_errors + (ssl.SSLError,) -_FTP_POOL_ERRORS: tuple[type[BaseException], ...] = ftplib.all_errors + ( - ssl.SSLError, - OSError, - AttributeError, -) - class _SimFtp: """Small FTPS stand-in for --sim without importing test-only mocks.""" @@ -56,7 +44,6 @@ def delete(self, path): _SIM_FTP_FILES.pop(os.path.basename(path), None) def voidcmd(self, cmd): - """Pool health-check (NOOP) used by ConnectionManager.get_ftp.""" return "200 OK" def quit(self): @@ -159,86 +146,6 @@ def ntransfercmd(self, cmd, rest=None): return conn, size -class PooledFTPWrapper: - def __init__(self, ftp, manager): - self._ftp = ftp - self._manager = manager - - def __getattr__(self, name): - return getattr(self._ftp, name) - - def __enter__(self): - self._manager._ftp_usage_lock.acquire() - return self - - def __exit__(self, exc_type, exc_val, exc_tb): - try: - if exc_type is not None: - with self._manager._lock: - if self._manager._ftp_client is self._ftp: - self._manager._ftp_client = None - try: - self._ftp.close() - except Exception: - pass - finally: - self._manager._ftp_usage_lock.release() - - -class ConnectionManager: - """Manages reusable MQTT and FTPS connections to reduce socket churn.""" - - def __init__(self): - self._mqtt_client = None - self._ftp_client = None - self._lock = threading.Lock() - self._ftp_usage_lock = threading.Lock() - - def get_ftp(self, printer, timeout=60): - with self._lock: - client = self._ftp_client - if client is not None: - try: - with self._ftp_usage_lock: - client.voidcmd("NOOP") - return PooledFTPWrapper(client, self) - except _FTP_POOL_ERRORS: - with self._lock: - if self._ftp_client is client and client is not None: - try: - client.close() - except Exception: - pass - self._ftp_client = None - - ftp = _create_raw_ftp(printer, timeout=timeout) - with self._lock: - self._ftp_client = ftp - return PooledFTPWrapper(ftp, self) - - def close_all(self): - self.clear() - - def clear(self): - with self._lock: - if self._mqtt_client is not None: - try: - self._mqtt_client.disconnect() - except Exception: - pass - self._mqtt_client = None - if self._ftp_client is not None: - try: - self._ftp_client.close() - except Exception: - pass - self._ftp_client = None - - -connection_manager = ConnectionManager() -atexit.register(connection_manager.close_all) - - def _create_raw_ftp(printer, timeout=60): """Connect to printer's FTPS server.""" if printer.simulation_mode: @@ -255,13 +162,3 @@ def _create_raw_ftp(printer, timeout=60): ftp.login("bblp", printer.access_code) ftp.prot_p() return ftp - - -def get_ftp(printer, timeout=60): - """Borrow a pooled FTPS client for *printer*. - - ``printer`` is required: this module must not reach up to - ``bambu_cli.printer`` for an ambient one (see scripts/check_layers.py). - Callers own the lookup. - """ - return connection_manager.get_ftp(printer, timeout=timeout) diff --git a/bambu_cli/protocols/mqtt.py b/bambu_cli/protocols/mqtt.py index eab95cb..61ae365 100644 --- a/bambu_cli/protocols/mqtt.py +++ b/bambu_cli/protocols/mqtt.py @@ -96,7 +96,6 @@ def probe_cert_fingerprint(host, port=990, timeout=5): def create_mqtt_client(printer, client_id=""): - global _TRUSTED_CERT_FILE if printer.simulation_mode: return _SimMqttClient() @@ -641,31 +640,6 @@ def on_message(client, userdata, msg): pass -import base64 - -_TRUSTED_CERT_FILE = None - -# probe_cert_fingerprint is defined above - - -def _get_and_verify_cert_pem(host, port, expected_fingerprint, timeout=5): - ctx = ssl.SSLContext(ssl.PROTOCOL_TLS_CLIENT) - ctx.check_hostname = False - ctx.verify_mode = ssl.CERT_NONE - with socket.create_connection((host, port), timeout) as raw, ctx.wrap_socket(raw, server_hostname=host) as tls: - der = tls.getpeercert(binary_form=True) - from bambu_cli.tlspin import verify_cert_fingerprint - - verify_cert_fingerprint(der, expected_fingerprint) - assert der is not None # verify_cert_fingerprint raises on a missing cert - pem = "-----BEGIN CERTIFICATE-----\n" - b64 = base64.b64encode(der).decode("ascii") - for i in range(0, len(b64), 64): - pem += b64[i : i + 64] + "\n" - pem += "-----END CERTIFICATE-----\n" - return pem - - def _printer_error_hex(code: object) -> Optional[str]: """Render a printer error code as the hex form Bambu documents (e.g. 0x0500C010). diff --git a/bambu_cli/tlspin.py b/bambu_cli/tlspin.py index 8ef2143..0aecef5 100644 --- a/bambu_cli/tlspin.py +++ b/bambu_cli/tlspin.py @@ -10,9 +10,8 @@ * No pin configured where a pin is required, a mismatched pin, or an unobtainable peer cert all raise; none of them return normally. * The compare is constant-time (``hmac.compare_digest``) on normalized hex. -* Inputs are normalized identically to :func:`bambu_cli.config._expected_fingerprint` - and :func:`bambu_cli.context._normalize_fingerprint`: lowercased with ``:`` and - ASCII spaces stripped. Accepted pin formats therefore include +* Inputs are normalized by :func:`normalize_fingerprint`: lowercased with ``:`` + and ASCII spaces stripped. Accepted pin formats therefore include ``AA:BB:CC...`` (colon-separated, any case), ``aa bb cc ...`` (space-separated), and the bare 64-hex-char digest. diff --git a/bambu_cli/utils.py b/bambu_cli/utils.py index 868955f..1a6c322 100644 --- a/bambu_cli/utils.py +++ b/bambu_cli/utils.py @@ -60,26 +60,10 @@ def _ensure_parent_dir(path): def _redact_url_credentials(url): - # Performance optimization: Fast-path for non-strings and strings that clearly - # cannot contain credentials (missing '@'). This avoids the overhead of - # lazy importing urllib and running the relatively expensive urlparse - # on every string value in large JSON responses. - if not isinstance(url, str) or "@" not in url: - return url + """Strip URL userinfo. Delegates to ``jsonio.redact_url_credentials``.""" + from bambu_cli.jsonio import redact_url_credentials - from urllib.parse import urlparse, urlunparse - - try: - parsed = urlparse(url) - if parsed.username or parsed.password: - netloc = f"***@{parsed.hostname}" - if parsed.port: - netloc += f":{parsed.port}" - parsed = parsed._replace(netloc=netloc) - return urlunparse(parsed) - except Exception: - pass - return url + return redact_url_credentials(url) _HOME_DIR = os.path.expanduser("~") diff --git a/tests/bambu_test_base.py b/tests/bambu_test_base.py index 6c00deb..da68ff2 100644 --- a/tests/bambu_test_base.py +++ b/tests/bambu_test_base.py @@ -122,7 +122,6 @@ def cleanup_mock_config(): from bambu_cli.cli import setup_logging from bambu_cli.commands import cmd_stop, cmd_light from bambu_cli.config import load_config - from bambu_cli.protocols.ftps import get_ftp from bambu_cli.protocols.mqtt import create_mqtt_client, execute_print_command import ssl import urllib.error @@ -179,7 +178,6 @@ def _setup_slice_proc(mock_proc, returncode=0, stdout=b"", stderr=b""): "mock_config_path", "bambu", "cmd_stop", - "get_ftp", "load_config", "create_mqtt_client", "cmd_light", diff --git a/tests/json_contract_base.py b/tests/json_contract_base.py index 6e00001..35f718d 100644 --- a/tests/json_contract_base.py +++ b/tests/json_contract_base.py @@ -50,8 +50,6 @@ def assert_shape(payload, spec, path="$"): - "enum": iterable of allowed values for this exact node. - "items": subspec applied to every element when type is list. """ - assert isinstance(payload, dict) or "type" in spec or True, path - if "type" in spec: expected_type = spec["type"] assert isinstance(payload, expected_type), ( @@ -99,7 +97,6 @@ def base_error_spec(command=None, require_failed_step=True): return {"type": dict, "required": required} - # --------------------------------------------------------------------------- # Harness # --------------------------------------------------------------------------- @@ -142,5 +139,3 @@ def make_ready_file(tmp_path, name="ready.3mf", content="simulated 3mf content") path = tmp_path / name path.write_text(content, encoding="utf-8") return path - - diff --git a/tests/test_audit_fixes_pr4.py b/tests/test_audit_fixes_pr4.py index 015b1fb..007b7da 100644 --- a/tests/test_audit_fixes_pr4.py +++ b/tests/test_audit_fixes_pr4.py @@ -56,6 +56,19 @@ def test_redact_preserves_existing_schemeless_and_full_url_behavior(): assert redact_url_credentials("no-at-sign") == "no-at-sign" +def test_emit_json_uses_jsonio_redactor(capsys): + """emit_json must strip userinfo, not the weaker ***@ placeholder.""" + from bambu_cli import utils + + at = "@" + utils._JSON_EMITTED = False + utils.emit_json({"source": "https://user:pass" + at + "host.com/x.stl"}) + payload = capsys.readouterr().out + assert "user:pass" not in payload + assert "***@" not in payload + assert "https://host.com/x.stl" in payload + + # --------------------------------------------------------------------------- # utils._display_path — home-prefix separator boundary # --------------------------------------------------------------------------- diff --git a/tests/test_audit_fixes_pr4_integration.py b/tests/test_audit_fixes_pr4_integration.py index 09395c6..d236d03 100644 --- a/tests/test_audit_fixes_pr4_integration.py +++ b/tests/test_audit_fixes_pr4_integration.py @@ -26,7 +26,7 @@ class TestDoctorCameraCapability(unittest.TestCase): @patch("bambu_cli.printer.BambuPrinter.get_version", return_value=[]) @patch("bambu_cli.protocols.mqtt.probe_cert_fingerprint", return_value=None) @patch("bambu_cli.protocols.mqtt.get_status") - @patch("bambu_cli.protocols.ftps.get_ftp") + @patch("bambu_cli.printer.BambuPrinter.get_ftp_client") @patch("bambu_cli.logging_utils._BACKEND") @patch("builtins.open") def _run_doctor_json( @@ -113,13 +113,9 @@ def test_direct_write_oserror_becomes_file_error(self, _mock_logger): fake_ctx = MagicMock() fake_ctx.printer.return_value = fake_printer - with patch.object( - camera, "_write_snapshot_atomic", side_effect=OSError("No space left on device") - ): + with patch.object(camera, "_write_snapshot_atomic", side_effect=OSError("No space left on device")): with self.assertRaises(BambuError) as cm: - camera._cmd_snapshot( - args, ctx=fake_ctx, grab_frame=lambda printer: b"\xff\xd8jpegbytes" - ) + camera._cmd_snapshot(args, ctx=fake_ctx, grab_frame=lambda printer: b"\xff\xd8jpegbytes") # A file write failure exits EXIT_FILE_ERROR, not the generic command # error the uncaught OSError would have produced. self.assertEqual(cm.exception.exit_code, EXIT_FILE_ERROR) diff --git a/tests/test_bambu_cli_regressions.py b/tests/test_bambu_cli_regressions.py index e9965e9..7a744a9 100644 --- a/tests/test_bambu_cli_regressions.py +++ b/tests/test_bambu_cli_regressions.py @@ -7,7 +7,7 @@ of headless GL / thumbnail noise (`_benign_rc` in bambu_cli/slicer/output.py), but must still FAIL on a genuine error ("nothing to be sliced", no .3mf). (b) FTPS teardown must use close(), never the hanging quit() - (bambu_cli/protocols/ftps.py ConnectionManager.clear / PooledFTPWrapper.__exit__). + (bambu_cli/printer.py BambuPrinter.get_ftp_client). (c) The download success path must be able to resolve `_record_download_success` (it was a NameError) -- bambu_cli/download/downloader.py `_cmd_download`. (d) snapshot must prefer the direct camera grab and NOT shell out to Docker when @@ -267,23 +267,26 @@ def voidcmd(self, *a, **k): self.calls.append("voidcmd") -def test_b_connection_manager_clear_uses_close_not_quit(): - mgr = ftps.ConnectionManager() +def test_b_get_ftp_client_teardown_uses_close_not_quit(): + from tests.bambu_test_base import _test_printer + + printer = _test_printer() fake = _RecordingFtp() - mgr._ftp_client = fake - mgr.clear() - assert "close" in fake.calls, "clear() must close the FTP connection" - assert "quit" not in fake.calls, "clear() must NOT call the hanging quit()" - assert mgr._ftp_client is None + with patch.object(ftps, "_create_raw_ftp", return_value=fake): + with printer.get_ftp_client(timeout=5): + pass + assert "close" in fake.calls, "get_ftp_client must close the FTP connection" + assert "quit" not in fake.calls, "get_ftp_client must NOT call the hanging quit()" + +def test_b_get_ftp_client_teardown_uses_close_not_quit_on_error(): + from tests.bambu_test_base import _test_printer -def test_b_pooled_wrapper_exit_on_error_uses_close_not_quit(): - mgr = ftps.ConnectionManager() + printer = _test_printer() fake = _RecordingFtp() - mgr._ftp_client = fake - wrapper = ftps.PooledFTPWrapper(fake, mgr) - wrapper.__enter__() # acquires usage lock - wrapper.__exit__(RuntimeError, RuntimeError("boom"), None) + with patch.object(ftps, "_create_raw_ftp", return_value=fake), pytest.raises(RuntimeError): + with printer.get_ftp_client(timeout=5): + raise RuntimeError("boom") assert "close" in fake.calls, "__exit__ on error must close the FTP connection" assert "quit" not in fake.calls, "__exit__ must NOT call the hanging quit()" diff --git a/tests/test_cmd_files.py b/tests/test_cmd_files.py index ddd6387..5557efb 100644 --- a/tests/test_cmd_files.py +++ b/tests/test_cmd_files.py @@ -2,6 +2,7 @@ from tests.bambu_test_base import * # noqa: F401,F403 + class TestBambuCmdFiles(unittest.TestCase): def _printer_with_ftp(self, mock_get_printer, mock_get_ftp): printer = _test_printer() @@ -93,8 +94,9 @@ def test_cmd_files_get_ftp_error(self, mock_exit, mock_logger, mock_get_printer) mock_get_ftp.assert_called_once() mock_logger.error.assert_called_with("Error listing files: Failed to list files via printer API") + class TestBambuCmdDelete(unittest.TestCase): - @patch("bambu_cli.protocols.ftps.get_ftp") + @patch("bambu_cli.printer.BambuPrinter.get_ftp_client") @patch("bambu_cli.logging_utils._BACKEND") @patch("sys.exit") def test_cmd_delete_no_confirm(self, mock_exit, mock_logger, mock_get_ftp): diff --git a/tests/test_doctor_and_safety.py b/tests/test_doctor_and_safety.py index c2349ff..368b1dc 100644 --- a/tests/test_doctor_and_safety.py +++ b/tests/test_doctor_and_safety.py @@ -40,7 +40,7 @@ def test_cmd_doctor_mqtt_fail(self, mock_logger, mock_exit, mock_get_status, moc @patch("bambu_cli.commands.doctor.load_config") @patch("bambu_cli.protocols.mqtt.get_status") - @patch("bambu_cli.protocols.ftps.get_ftp") + @patch("bambu_cli.printer.BambuPrinter.get_ftp_client") @patch("sys.exit") @patch("bambu_cli.logging_utils._BACKEND") def test_cmd_doctor_ftps_fail(self, mock_logger, mock_exit, mock_get_ftp, mock_get_status, mock_load): @@ -59,7 +59,7 @@ def test_cmd_doctor_ftps_fail(self, mock_logger, mock_exit, mock_get_ftp, mock_g mock_logger.error.assert_any_call(" ❌ FTPS connection failed: FTPS Fail") @patch("bambu_cli.protocols.mqtt.get_status") - @patch("bambu_cli.protocols.ftps.get_ftp") + @patch("bambu_cli.printer.BambuPrinter.get_ftp_client") @patch("bambu_cli.logging_utils._BACKEND") @patch("builtins.open") def test_cmd_doctor_success(self, mock_file_open, mock_logger, mock_get_ftp, mock_get_status): @@ -93,14 +93,16 @@ def custom_open(file, *args, **kwargs): and call[0][1] == "w" for call in mock_file_open.call_args_list ) - self.assertTrue(any_caps_open, "Expected a randomly generated printer_capabilities_*.json to be opened for writing") + self.assertTrue( + any_caps_open, "Expected a randomly generated printer_capabilities_*.json to be opened for writing" + ) @patch("bambu_cli.protocols.mqtt.get_status") - @patch("bambu_cli.protocols.ftps.get_ftp") + @patch("bambu_cli.printer.BambuPrinter.get_ftp_client") @patch("bambu_cli.logging_utils._BACKEND") @patch("builtins.open") def test_cmd_doctor_honours_network_timeout(self, mock_file_open, mock_logger, mock_get_ftp, mock_get_status): - """--network-timeout is forwarded to printer.status() and get_ftp().""" + """--network-timeout is forwarded to printer.status() and get_ftp_client().""" from bambu_cli.commands import cmd_doctor import io @@ -126,6 +128,8 @@ def custom_open(file, *args, **kwargs): call_kwargs = mock_get_status.call_args self.assertEqual(call_kwargs.kwargs.get("timeout") or call_kwargs[1].get("timeout"), 3.0) self.assertEqual(call_kwargs.kwargs.get("retries", call_kwargs[1].get("retries")), 0) + ftp_kwargs = mock_get_ftp.call_args + self.assertEqual(ftp_kwargs.kwargs.get("timeout") or ftp_kwargs[1].get("timeout"), 3.0) class TestOfferPinFingerprint(unittest.TestCase): @@ -253,37 +257,26 @@ def test_simulation_mode_upload(self, mock_commands_logger, mock_ftps_logger): args.dry_run = False args.sim = True - from bambu_cli.protocols.ftps import connection_manager - - connection_manager.clear() - with settings_ctx(simulation=True): + # Use a real file (not mock_open) so _SimFtp's fp.tell()/seek()-based + # size bookkeeping — and upload_file's post-transfer size + # verification against it — reflect actual byte counts. + local_path = os.path.join(os.getcwd(), "test.3mf") + with open(local_path, "wb") as f: + f.write(b"x" * 1024) try: - # Use a real file (not mock_open) so _SimFtp's fp.tell()/seek()-based - # size bookkeeping — and upload_file's post-transfer size - # verification against it — reflect actual byte counts. - local_path = os.path.join(os.getcwd(), "test.3mf") - with open(local_path, "wb") as f: - f.write(b"x" * 1024) - try: - cmd_upload(args) - finally: - os.unlink(local_path) - - self.assertTrue( - any( - "Connecting to simulated FTPS server" in call[0][0] - for call in mock_ftps_logger.info.call_args_list - ) - ) - self.assertTrue( - any( - "Uploaded test.3mf to printer" in call[0][0] - for call in mock_commands_logger.info.call_args_list - ) - ) + cmd_upload(args) finally: - connection_manager.clear() + os.unlink(local_path) + + self.assertTrue( + any( + "Connecting to simulated FTPS server" in call[0][0] for call in mock_ftps_logger.info.call_args_list + ) + ) + self.assertTrue( + any("Uploaded test.3mf to printer" in call[0][0] for call in mock_commands_logger.info.call_args_list) + ) class TestBambuSecurity(unittest.TestCase): @@ -371,7 +364,7 @@ def _logged(mock_logger): @patch("bambu_cli.protocols.mqtt.probe_cert_fingerprint", return_value=None) @patch("bambu_cli.protocols.mqtt.get_status") - @patch("bambu_cli.protocols.ftps.get_ftp") + @patch("bambu_cli.printer.BambuPrinter.get_ftp_client") @patch("bambu_cli.logging_utils._BACKEND") def test_cmd_doctor_human_output_redacts_printer_ip_by_default( self, mock_logger, mock_get_ftp, mock_get_status, mock_probe @@ -399,7 +392,7 @@ def test_cmd_doctor_human_output_redacts_printer_ip_by_default( @patch("bambu_cli.protocols.mqtt.probe_cert_fingerprint", return_value=None) @patch("bambu_cli.protocols.mqtt.get_status") - @patch("bambu_cli.protocols.ftps.get_ftp") + @patch("bambu_cli.printer.BambuPrinter.get_ftp_client") @patch("bambu_cli.logging_utils._BACKEND") def test_cmd_doctor_human_output_shows_printer_ip_with_verbose( self, mock_logger, mock_get_ftp, mock_get_status, mock_probe @@ -431,7 +424,7 @@ def test_cmd_doctor_human_output_shows_printer_ip_with_verbose( @patch("bambu_cli.commands.doctor._expected_fingerprint", return_value="ab" * 32) @patch("bambu_cli.protocols.mqtt.probe_cert_fingerprint", return_value="ab" * 32) @patch("bambu_cli.protocols.mqtt.get_status") - @patch("bambu_cli.protocols.ftps.get_ftp") + @patch("bambu_cli.printer.BambuPrinter.get_ftp_client") @patch("bambu_cli.logging_utils._BACKEND") def test_cmd_doctor_hides_fingerprint_when_already_pinned( self, mock_logger, mock_get_ftp, mock_get_status, mock_probe, mock_expected @@ -460,7 +453,7 @@ def test_cmd_doctor_hides_fingerprint_when_already_pinned( @patch("bambu_cli.commands.doctor._expected_fingerprint", return_value=None) @patch("bambu_cli.protocols.mqtt.probe_cert_fingerprint", return_value="cd" * 32) @patch("bambu_cli.protocols.mqtt.get_status") - @patch("bambu_cli.protocols.ftps.get_ftp") + @patch("bambu_cli.printer.BambuPrinter.get_ftp_client") @patch("bambu_cli.logging_utils._BACKEND") def test_cmd_doctor_shows_fingerprint_when_not_pinned( self, mock_logger, mock_get_ftp, mock_get_status, mock_probe, mock_expected, mock_offer diff --git a/tests/test_mqtt_print_and_setup.py b/tests/test_mqtt_print_and_setup.py index 2ff7eac..c1532d6 100644 --- a/tests/test_mqtt_print_and_setup.py +++ b/tests/test_mqtt_print_and_setup.py @@ -450,26 +450,6 @@ def test_setup_placeholder_ip(): wizard_mod._cmd_setup_noninteractive(args) -def test_get_and_verify_cert_pem_mismatch(): - der = b"\x01\x02" - raw = MagicMock() - tls = MagicMock() - tls.getpeercert.return_value = der - tls.__enter__ = lambda s: tls - tls.__exit__ = lambda *a: False - raw_cm = MagicMock() - raw_cm.__enter__ = lambda s: raw - raw_cm.__exit__ = lambda *a: False - ctx = MagicMock() - ctx.wrap_socket.return_value = tls - with ( - patch("bambu_cli.protocols.mqtt.socket.create_connection", return_value=raw_cm), - patch("ssl.SSLContext", return_value=ctx), - pytest.raises(ssl.SSLError), - ): - mqtt_mod._get_and_verify_cert_pem("h", 990, "00" * 32, timeout=1) - - def test_send_command_on_connect_fail_rc(): printer = _test_printer(simulation_mode=False) client = MagicMock() @@ -513,6 +493,7 @@ def loop_start(): # message goes nowhere and command_accepted.wait() times out — a real breakage. client.loop_start.assert_called() from unittest.mock import MagicMock as _MagicMock + assert not isinstance(client.on_message, _MagicMock), ( "execute_print_command must assign a real handler to client.on_message; " "a MagicMock default means the MQTT accept path is unwired" @@ -765,15 +746,11 @@ def loop_start(): ): mqtt_mod.execute_print_command(printer, "{}", "x.3mf", dry_run=False, command_timeout=1) # The print-start payload must be published exactly once despite two connects. - request_publishes = [ - c for c in client.publish.call_args_list if c.args and str(c.args[0]).endswith("/request") - ] + request_publishes = [c for c in client.publish.call_args_list if c.args and str(c.args[0]).endswith("/request")] assert len(request_publishes) == 1 # But the report subscription must be (re)established on BOTH connects, or an # ack after a mid-window reconnect would be invisible and time the print out. - report_subscribes = [ - c for c in client.subscribe.call_args_list if c.args and str(c.args[0]).endswith("/report") - ] + report_subscribes = [c for c in client.subscribe.call_args_list if c.args and str(c.args[0]).endswith("/report")] assert len(report_subscribes) == 2 @@ -793,9 +770,7 @@ def loop_start(): patch.object(mqtt_mod, "_mqtt_connect"), ): assert mqtt_mod.send_command(printer, "{}", timeout=1, retries=0) is True - request_publishes = [ - c for c in client.publish.call_args_list if c.args and str(c.args[0]).endswith("/request") - ] + request_publishes = [c for c in client.publish.call_args_list if c.args and str(c.args[0]).endswith("/request")] assert len(request_publishes) == 1 # And publishes at QoS 1 so on_publish reflects a broker PUBACK, not a bare # local socket write. diff --git a/tests/test_netsafety_handlers.py b/tests/test_netsafety_handlers.py index 9061a51..d98870b 100644 --- a/tests/test_netsafety_handlers.py +++ b/tests/test_netsafety_handlers.py @@ -1,13 +1,10 @@ """Behavior tests salvaged from former coverage-padding modules. -These assert observable outcomes for netsafety handlers, MQTT cert PEM, -and preflight edge cases. +These assert observable outcomes for netsafety handlers and preflight edge cases. """ from __future__ import annotations -import base64 -import hashlib import urllib.request from unittest.mock import MagicMock, patch @@ -92,28 +89,6 @@ def _boom(name, *a, **k): mqtt_mod.mqtt = prev -def test_get_and_verify_cert_pem_success(): - der = b"cert-bytes-for-pin" - expected = hashlib.sha256(der).hexdigest() - raw = MagicMock() - tls = MagicMock() - tls.getpeercert.return_value = der - tls.__enter__ = lambda s: tls - tls.__exit__ = lambda *a: False - raw_cm = MagicMock() - raw_cm.__enter__ = lambda s: raw - raw_cm.__exit__ = lambda *a: False - ctx = MagicMock() - ctx.wrap_socket.return_value = tls - with ( - patch("bambu_cli.protocols.mqtt.socket.create_connection", return_value=raw_cm), - patch("ssl.SSLContext", return_value=ctx), - ): - pem = mqtt_mod._get_and_verify_cert_pem("host", 990, expected, timeout=1) - assert "BEGIN CERTIFICATE" in pem - assert base64.b64encode(der).decode("ascii")[:20] in pem.replace("\n", "") - - def test_preflight_permission_win32_skips(monkeypatch): monkeypatch.setattr(preflight_mod.sys, "platform", "win32") assert preflight_mod._file_permission_check("/tmp/x", "access_code") is None diff --git a/tests/test_protocol_clients.py b/tests/test_protocol_clients.py index 241a0d5..16514fc 100644 --- a/tests/test_protocol_clients.py +++ b/tests/test_protocol_clients.py @@ -136,50 +136,37 @@ def side_effect_connect(host, port, keepalive): class TestGetFtp(unittest.TestCase): - def setUp(self): - from bambu_cli.protocols.ftps import connection_manager - - connection_manager.clear() - self.addCleanup(connection_manager.clear) - @patch("bambu_cli.protocols.ftps.ImplicitFTPS") - def test_get_ftp_success(self, mock_implicit_ftps): - # Setup mocks + def test_create_raw_ftp_success(self, mock_implicit_ftps): + from bambu_cli.protocols.ftps import _create_raw_ftp + mock_ftp_instance = MagicMock() mock_implicit_ftps.return_value = mock_ftp_instance printer = _test_printer(ip="192.168.1.100", access_code="mock_access_code") - # get_ftp/_create_raw_ftp now take the printer object - result = get_ftp(printer) + result = _create_raw_ftp(printer) - # Assertions mock_implicit_ftps.assert_called_once() mock_ftp_instance.connect.assert_called_once_with("192.168.1.100", 990, timeout=60) - mock_ftp_instance_login = mock_ftp_instance.login - mock_ftp_instance_login.assert_called_once_with("bblp", "mock_access_code") + mock_ftp_instance.login.assert_called_once_with("bblp", "mock_access_code") mock_ftp_instance.prot_p.assert_called_once() - - from bambu_cli.protocols.ftps import PooledFTPWrapper - - self.assertIsInstance(result, PooledFTPWrapper) - self.assertEqual(result._ftp, mock_ftp_instance) + self.assertIs(result, mock_ftp_instance) @patch("bambu_cli.protocols.ftps.ImplicitFTPS") - def test_get_ftp_connect_failure(self, mock_implicit_ftps): - # Setup mock to raise an exception on connect + def test_create_raw_ftp_connect_failure(self, mock_implicit_ftps): + from bambu_cli.protocols.ftps import _create_raw_ftp + mock_ftp_instance = MagicMock() mock_implicit_ftps.return_value = mock_ftp_instance mock_ftp_instance.connect.side_effect = OSError("Connection Refused") printer = _test_printer(ip="192.168.1.100", access_code="mock_access_code") - # Call the function and assert it raises with self.assertRaises(Exception) as context: - get_ftp(printer) + _create_raw_ftp(printer) self.assertEqual(str(context.exception), "Connection Refused") mock_implicit_ftps.assert_called_once() mock_ftp_instance.connect.assert_called_once_with("192.168.1.100", 990, timeout=60) - # Ensure it doesn't try to login if connect fails mock_ftp_instance.login.assert_not_called() mock_ftp_instance.prot_p.assert_not_called() diff --git a/tests/test_sim_transport_setup.py b/tests/test_sim_transport_setup.py index f07cbd3..717d567 100644 --- a/tests/test_sim_transport_setup.py +++ b/tests/test_sim_transport_setup.py @@ -148,18 +148,10 @@ def test_noncolliding_path_creates_sibling(tmp_path): def test_get_ftp_simulation(): - ftps_mod.connection_manager.clear() printer = _test_printer(simulation_mode=True) - client = ftps_mod.get_ftp(printer, timeout=5) - assert client is not None - with client: - pass - # Reuse pooled sim client (exercises voidcmd health check). - client2 = ftps_mod.get_ftp(printer, timeout=5) - assert client2 is not None - with client2: - pass - ftps_mod.connection_manager.clear() + with printer.get_ftp_client(timeout=5) as client: + assert client is not None + assert "simulated_file.3mf" in client.nlst() # --- netsafety extras --------------------------------------------------------- @@ -303,17 +295,6 @@ def connect(*a, **k): assert result is None -def test_connection_manager_clear(): - mgr = ftps_mod.ConnectionManager() - fake = MagicMock() - mgr._ftp_client = fake - mgr.clear() - fake.close.assert_called() - assert mgr._ftp_client is None - # close_all should not raise when empty - mgr.close_all() - - def test_common_looks_like_placeholder(): from bambu_cli.setup_cmd import common as common diff --git a/tests/test_tlspin.py b/tests/test_tlspin.py index 38a0814..e2ad654 100644 --- a/tests/test_tlspin.py +++ b/tests/test_tlspin.py @@ -146,3 +146,14 @@ def test_no_peer_cert_uses_exc_factory(): ) def test_normalize_fingerprint(raw, expected): assert normalize_fingerprint(raw) == expected + + +def test_config_and_context_delegate_to_tlspin(): + from bambu_cli.config import _expected_fingerprint + from bambu_cli.context import _normalize_fingerprint + from tests.bambu_test_base import config_ctx + + raw = "AA:BB:CC:DD" + assert _normalize_fingerprint(raw) == normalize_fingerprint(raw) + with config_ctx({"cert_fingerprint": raw}): + assert _expected_fingerprint() == normalize_fingerprint(raw)