diff --git a/CHANGELOG.md b/CHANGELOG.md index 82ef340..1f0ac4e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,55 @@ Format follows [Keep a Changelog](https://keepachangelog.com/en/1.1.0/); version ### Printer safety +- **A printer command is sent at most once.** When the connection dropped + after a command reached the printer but before its acknowledgement came + back, the MQTT client reconnected by itself and resent it: a `gcode` move, + `stop` or `light` ran twice while the command reported success. Automatic + reconnect is off; the command is reported as sent but unconfirmed + (`sent: true`, `acknowledged: false`) and is never resent. +- **An unacknowledged print start is no longer reported as "not printed".** + When the print command was published but the printer never confirmed it, + `print` and `job` said `printed: false`, which invites a retry and a second + print. They now report `printed: null` with `sent: true`, say the printer + may already be printing, and point at `plate status`. The fields are on the + `--json` error envelope too (before, it carried only "Command failed"). +- **Raw G-code is held to the heater limits.** `plate gcode "M104 S400"` went + straight to the printer. `M104`/`M109` above 350 °C and `M140`/`M190` above + 150 °C are now refused before anything is sent, as is a temperature that + is not a plain number (`S4e2`, `S0x190`, non-ASCII digits), including after + a tool number (`M104 T0 S400`). +- **Pre-sliced files are held to the heater limits too.** A downloaded or + third-party sliced 3MF (or `.gcode`) went to the printer with whatever + temperatures it carried. `upload`, `job` and `send` (and `job --dry-run`) now + read every plate's G-code and refuse a nozzle above 350 °C or a bed above + 150 °C, naming the plate and line. Every line ending (LF, CR, CRLF) and + every archive entry counts, duplicate names included; G-code or a 3MF that + cannot be read is refused, and so is a line over 1 MiB. +- **Guided mode re-reads the AMS before it starts a print.** `plate go` and the + TUI read the loaded filament when the material is chosen; slicing can take + minutes, and a spool swapped in that time was fed at the temperatures sliced + for the old one. Start print now reads the AMS again and refuses, with + nothing sent, if the material or slot changed or cannot be read (upload only + is unaffected). The summary before confirming gains a Feed row (`AMS0·2 + (PETG)` or External spool) and a Temperatures row with the hottest nozzle + and bed targets in the file's G-code. +- **Setup asks before using the one printer mDNS found.** Discovery is + unauthenticated and the access code goes to the configured address, yet a + lone result was taken without a question. It is now shown and confirmed + (no falls back to typing the address). An address no printer can have + (`0.0.0.0`, multicast) is skipped, and one a LAN printer would not have + (a public address, this computer) is flagged and needs an explicit yes. +- `--ams-mapping` accepts ASCII digits only; `٣` and other Unicode digits that + `int()` reads were passed through as slot numbers. +- The upload guard that refuses to overwrite the file being printed now waits + for a full status report, and a print whose report names no file blocks the + upload too: a partial report without `gcode_file` read as "not printing this + file" and let the upload through. +- `slice --quality 0.28` (a height with no named profile) now needs a 0.28 mm + process profile and lists the available heights when there is none; it used + to slice the Standard profile without saying so. A failed OrcaSlicer run no + longer replaces an earlier good output file. + - **A1 and A1 mini can be sliced.** Every A1/A1 mini slice failed inside OrcaSlicer ("Relative extruder addressing requires resetting the extruder position at each layer"): Bambu's A1 profiles omit the per-layer `G92 E0` @@ -73,6 +122,30 @@ Format follows [Keep a Changelog](https://keepachangelog.com/en/1.1.0/); version ### Fixed +- `download` accepts URLs with non-ASCII paths (pasted from a browser), and + ASCII STL/OBJ/G-code served as `text/plain` when the URL names a model file. + Ctrl-C during a download or ZIP extraction removes the partial file. A + malformed IPv6 host is a clean "Invalid URL" instead of a traceback. +- Printables: a download link that is not a string is reported as an API + change instead of crashing. +- Config values of the wrong type (a number for `access_code`, + `cert_fingerprint` or `printer_ip`, a non-object `config.json`) give a + config error or fall back cleanly instead of a traceback; `camera_port` is + validated, and `preflight` reports `cert-fingerprint` and `camera-port` + checks. Timeouts are capped at 24 hours. +- Option abbreviations are off: `--conf` no longer silently means `--confirm`. +- JSON output never contains `NaN` or `Infinity` (invalid JSON): non-finite + numbers from a printer report become `null`. Over-long AMS numbers no longer + crash `status`. +- `doctor` no longer leaves an empty temporary file behind when its checks + stop early; `snapshot --sim` refuses the Docker streamer instead of trying it. +- Slicing: filament profile choice is deterministic (exact name, then + shortest) and logged; profile directories with `[` or `*` in the path work; + an absurd temperature override is a validation error, not a traceback. +- Interactive mode: prompts no longer interpret `[...]` as markup, and a failed + upload or print says so before returning to the menu. +- A failed TLS data-channel setup closes its socket instead of leaking it. + - `status --monitor` exited 0 with no output when the printer refused the connection (wrong access code), so a script could not tell it from success. It now fails with exit 2 (`failed_step: mqtt`) on a refused connection, on a @@ -140,6 +213,25 @@ Format follows [Keep a Changelog](https://keepachangelog.com/en/1.1.0/); version ### Security +- **`insecure_tls` no longer switches off a configured certificate pin.** + With both `cert_fingerprint` and `insecure_tls` set, MQTT and the camera + skipped the pin and accepted any certificate. A pin is now always checked. +- **Untrusted names cannot drive the terminal.** File names from the printer's + FTP listing, MQTT report fields (`gcode_file`, AMS tray types, error text) + in `status`, `status --monitor` and the TUI, mDNS names and prompt text could carry ESC sequences (clear the screen, + write the clipboard with OSC 52) or bidi overrides that make one name read + as another. They are shown with control and bidi characters replaced, and + names with them are refused for upload (download renaming replaces them). +- A hostile or corrupt MQTT report (deeply nested JSON, an over-long integer, + non-object JSON, `1e999`) is ignored instead of killing the client's network + thread, which left a held session reporting stale state, or stopping + `status --monitor` from ever seeing the end of the print. +- A URL password holding `/`, `?` or `#` (`user:pa/ss@host`, `user:12/ss@host`) + is now redacted and the URL refused: the parser read it as a host and port, + so the password was echoed in errors and JSON, and with a digit prefix the + request went to host `user` with the password in its path. `https:///user:pw@host` + and similar host-less forms are redacted too. + - **`"camera_allow_streamer": "false"` no longer enables the unpinned camera streamer.** The key was read with `bool()`, so any non-empty string — including `"false"` — turned on the Docker streamer that ignores diff --git a/bambu_cli/ams.py b/bambu_cli/ams.py index 3775fba..e8999a2 100644 --- a/bambu_cli/ams.py +++ b/bambu_cli/ams.py @@ -6,6 +6,8 @@ filament is loaded where and to build a correct ``--ams-mapping`` argument. """ +import math + # Bambu firmware reports two sentinel values in ``tray_now`` that are not AMS # slot indices. They are distinct states and must not be conflated: 254 means # the printer is feeding from the external spool (vt_tray), 255 means nothing @@ -17,15 +19,22 @@ def _to_int(value, default=None): try: return int(value) - except (TypeError, ValueError): + except (TypeError, ValueError, OverflowError): return default def _to_float(value, default=None): try: - return float(value) - except (TypeError, ValueError): + result = float(value) + except (TypeError, ValueError, OverflowError): return default + # NaN/Infinity from a spoofed report are unknown, not a reading. + return result if math.isfinite(result) else default + + +def _tray_type(raw): + """Filament type as a non-empty string, else None (any other JSON type is junk).""" + return raw if isinstance(raw, str) and raw else None def _normalize_color(raw): @@ -94,7 +103,7 @@ def parse_ams(status): # _to_int returns `default` for missing AND unparseable input, so the # default alone covers every case -- no None check needed after it. vt_slot = _to_int(vt_tray_raw.get("id"), default=EXTERNAL_SPOOL_TRAY_ID) - vt_type = vt_tray_raw.get("tray_type") or None + vt_type = _tray_type(vt_tray_raw.get("tray_type")) vt_color = _normalize_color(vt_tray_raw.get("tray_color")) vt_remain = _to_int(vt_tray_raw.get("remain")) # 254 and 255 are BOTH sentinels, but they do not mean the same thing: @@ -130,11 +139,12 @@ def parse_ams(status): continue unit_id = _to_int(unit_raw.get("id"), default=0) or 0 trays = [] - for tray_raw in unit_raw.get("tray") or []: + tray_list = unit_raw.get("tray") + for tray_raw in tray_list if isinstance(tray_list, list) else []: if not isinstance(tray_raw, dict): continue slot = _to_int(tray_raw.get("id"), default=0) or 0 - ftype = tray_raw.get("tray_type") or None + ftype = _tray_type(tray_raw.get("tray_type")) absolute = unit_id * 4 + slot trays.append( { diff --git a/bambu_cli/cliparse.py b/bambu_cli/cliparse.py index 2eff89b..9aa163e 100644 --- a/bambu_cli/cliparse.py +++ b/bambu_cli/cliparse.py @@ -32,12 +32,16 @@ def positive_seconds(value): """ import math + from bambu_cli.constants import MAX_TIMEOUT_SECONDS + try: seconds = float(value) - except (TypeError, ValueError): + except (TypeError, ValueError, OverflowError): raise argparse.ArgumentTypeError(f"must be a positive number of seconds (got {value!r})") from None if not math.isfinite(seconds) or seconds <= 0: raise argparse.ArgumentTypeError(f"must be a positive number of seconds (got {value!r})") + if seconds > MAX_TIMEOUT_SECONDS: + raise argparse.ArgumentTypeError(f"must be at most {MAX_TIMEOUT_SECONDS} seconds (got {value!r})") return seconds @@ -45,7 +49,7 @@ def positive_int(value): """argparse type for a 1-based index such as a plate number.""" try: number = int(value) - except (TypeError, ValueError): + except (TypeError, ValueError, OverflowError): raise argparse.ArgumentTypeError(f"must be a positive whole number (got {value!r})") from None if number < 1: raise argparse.ArgumentTypeError(f"must be a positive whole number (got {value!r})") @@ -106,6 +110,9 @@ class JsonArgumentParser(argparse.ArgumentParser): def __init__(self, *args, **kwargs): kwargs.setdefault("conflict_handler", "resolve") + # An abbreviated --conf must not reach the --confirm gate: agent policies + # that look for the literal flag would be bypassed. + kwargs.setdefault("allow_abbrev", False) super().__init__(*args, **kwargs) def error(self, message): diff --git a/bambu_cli/commands/doctor.py b/bambu_cli/commands/doctor.py index f90fd81..9299270 100644 --- a/bambu_cli/commands/doctor.py +++ b/bambu_cli/commands/doctor.py @@ -88,17 +88,16 @@ def emit_doctor_failure(failed_step, exit_code, error, extra=None): emit_json(payload) cap_path = _namespace_get(args, "output") - if not cap_path: - fd, cap_path = tempfile.mkstemp(prefix="printer_capabilities_", suffix=".json") - os.close(fd) - cap_path = _expand_path(cap_path) - if cap_path.startswith("-"): + if cap_path: + cap_path = _expand_path(cap_path) + if cap_path and cap_path.startswith("-"): message = f"Invalid output path: {_path_for_message(cap_path)}" logger.error(message) emit_doctor_failure("validate", EXIT_FILE_ERROR, message) abort("", exit_code=EXIT_FILE_ERROR) try: - _ensure_parent_dir(cap_path) + if cap_path: + _ensure_parent_dir(cap_path) except BambuError as exc: emit_doctor_failure( "validate", @@ -223,6 +222,10 @@ def shown_ip(): }, } + if not cap_path: + # Created only now so a run that fails earlier leaves nothing behind. + fd, cap_path = tempfile.mkstemp(prefix="printer_capabilities_", suffix=".json") + os.close(fd) try: with open(cap_path, "w", encoding="utf-8") as f: json.dump(capabilities, f, indent=2) diff --git a/bambu_cli/commands/files.py b/bambu_cli/commands/files.py index 12409e5..f6ccb1f 100644 --- a/bambu_cli/commands/files.py +++ b/bambu_cli/commands/files.py @@ -15,11 +15,13 @@ ) from bambu_cli.errors import abort from bambu_cli.fsutil import _portable_basename +from bambu_cli.gcode_limits import file_heater_problem from bambu_cli.logging_utils import logger from bambu_cli.paths import exception_for_message as _exception_for_message from bambu_cli.paths import expand_path as _expand_path from bambu_cli.paths import path_for_message as _path_for_message from bambu_cli.slicer import _directory_input_message, _is_directory_input +from bambu_cli.textsafe import display_safe from bambu_cli.utils import emit_json # gcode_state values for which the printer is using its current file. @@ -38,20 +40,30 @@ def _print_name_key(name): def _printing_this_file(printer, filename): """True when the printer reports an active print of ``filename``. + ``None`` when it reports an active print without naming the file: deltas + can add up to a "complete" report with neither ``gcode_file`` nor + ``subtask_name``, and "not this file" cannot be told apart from that. + ``upload_file`` deletes the remote file before storing the new one, so an upload with the name of the running print pulls the file out from under the printer (what the firmware does then is unverified). Best effort: a status query that fails returns False and the upload goes ahead. + + ``require_complete=True``: mid-print the printer streams deltas that carry + neither ``gcode_state`` nor the file name, and the first message to arrive + is often one of them; reading only that let the guard pass while printing. """ try: - data = printer.status(retries=0, require_complete=False) + data = printer.status(retries=0, require_complete=True) except Exception as exc: # noqa: BLE001 -- a probe must never block the upload logger.debug(f"Could not check the printer's current job before upload: {exc}") return False if not isinstance(data, dict) or str(data.get("gcode_state") or "").upper() not in _ACTIVE_GCODE_STATES: return False - target = _print_name_key(filename) - return target in {_print_name_key(data.get("subtask_name")), _print_name_key(data.get("gcode_file"))} + names = {_print_name_key(data.get("subtask_name")), _print_name_key(data.get("gcode_file"))} - {""} + if not names: + return None + return _print_name_key(filename) in names def cmd_upload(args, ctx=None): @@ -118,6 +130,17 @@ def cmd_upload(args, ctx=None): failed_step="validate", extra={"file": filepath, "remote_name": filename, "bytes": filesize}, ) + heater = file_heater_problem(filepath) + if heater: + # A pre-sliced 3MF from a download or another slicer carries G-code + # platecli never generated; hold it to the limits slicing obeys. + message = f"Refusing to upload {filename}: {heater}" + abort( + message, + exit_code=EXIT_FILE_ERROR, + failed_step="validate", + extra={"file": filepath, "remote_name": filename}, + ) if getattr(args, "dry_run", False): logger.info(f"🔍 Dry Run: Validating printer connectivity for {filename}...") printer = ctx.printer() @@ -166,9 +189,15 @@ def cmd_upload(args, ctx=None): return filename printer = ctx.printer() - if _printing_this_file(printer, filename): + printing = _printing_this_file(printer, filename) + if printing is not False: + what = ( + f"is printing {filename}" + if printing + else f"is printing but did not say which file, so it may be {filename}" + ) abort( - f"The printer is printing {filename} right now; uploading would replace that file mid-print. " + f"The printer {what} right now; uploading would replace that file mid-print. " "Wait for the print to finish, or upload under a different name.", exit_code=EXIT_PRINTER_ERROR, failed_step="upload", @@ -273,7 +302,9 @@ def cmd_files(args, ctx=None): return logger.info("📁 Files on printer:") for f in files: - logger.info(f" {f}") + # Names come from the printer's FTP listing; never let one write + # terminal control sequences (OSC 52 clipboard, screen clears). + logger.info(f" {display_safe(f)}") except Exception as e: message = f"Error listing files: {e}" abort( diff --git a/bambu_cli/commands/gcode.py b/bambu_cli/commands/gcode.py index 9da1728..6d31f04 100644 --- a/bambu_cli/commands/gcode.py +++ b/bambu_cli/commands/gcode.py @@ -8,6 +8,7 @@ from bambu_cli.contracts import Gcode from bambu_cli.download.naming import _has_command_injection_chars from bambu_cli.errors import CommandUnconfirmed, abort +from bambu_cli.gcode_limits import heater_problem as _heater_problem from bambu_cli.logging_utils import logger from bambu_cli.utils import emit_json, get_sequence_id @@ -27,6 +28,14 @@ def cmd_gcode(args, ctx=None): failed_step="validate", extra={"gcode": gcode, "sent": False}, ) + problem = _heater_problem(gcode) + if problem: + abort( + f"Invalid G-code: {problem}", + exit_code=EXIT_COMMAND_ERROR, + failed_step="validate", + extra={"gcode": gcode, "sent": False}, + ) if not args.confirm: logger.warning("⚠️ This will SEND raw G-code to the printer. Add --confirm to proceed.") diff --git a/bambu_cli/commands/snapshot.py b/bambu_cli/commands/snapshot.py index 5809914..5c3a889 100644 --- a/bambu_cli/commands/snapshot.py +++ b/bambu_cli/commands/snapshot.py @@ -245,7 +245,7 @@ def cmd_snapshot( message = f"Camera TLS certificate does not match pinned fingerprint: {_exc}" emit_json_error(args, "snapshot", EXIT_NETWORK_ERROR, message, failed_step="grab", output=outpath) except ssl.SSLError as _exc: - if not printer.insecure_tls and printer.cert_fingerprint: + if printer.cert_fingerprint: message = ( "Camera TLS error with a cert pin configured " f"(refusing to fall back to the unverified Docker streamer): {_exc}" @@ -290,6 +290,12 @@ def cmd_snapshot( message = _streamer_refused_message(ctx.settings, _fallback_reason) emit_json_error(args, "snapshot", EXIT_NETWORK_ERROR, message, failed_step="grab", output=outpath) + if ctx.simulation: + # The simulated printer has no access code; the streamer would read the + # real one and contact the real printer. + message = "Simulation mode has no camera: the Docker streamer is not available under --sim." + emit_json_error(args, "snapshot", EXIT_NETWORK_ERROR, message, failed_step="grab", output=outpath) + streamer_url = ctx.settings.camera_stream_url camera_image = ctx.settings.camera_image diff --git a/bambu_cli/commands/status.py b/bambu_cli/commands/status.py index 5a1e4b3..7aecf03 100644 --- a/bambu_cli/commands/status.py +++ b/bambu_cli/commands/status.py @@ -7,6 +7,7 @@ from bambu_cli.contracts import PrinterState, Status from bambu_cli.errors import PrinterConnectionError from bambu_cli.logging_utils import logger +from bambu_cli.textsafe import display_safe from bambu_cli.utils import emit_json _PRINTER_STATE_FIELDS = {f.name for f in dataclasses.fields(PrinterState)} @@ -28,6 +29,11 @@ def _status_payload(data, ams): return payload +def _show(line): + """Log one human status line; every value in it came from the printer.""" + logger.info(display_safe(line)) + + def cmd_status(args, ctx=None): """Query and display the printer's current status.""" from bambu_cli.ams import parse_ams @@ -60,7 +66,7 @@ def cmd_status(args, ctx=None): total_layers = data.get("total_layer_num", 0) try: remaining = int(data.get("mc_remaining_time", 0)) - except (TypeError, ValueError): + except (TypeError, ValueError, OverflowError): remaining = 0 filename = data.get("gcode_file", "") bed_temp = data.get("bed_temper", "?") @@ -71,32 +77,32 @@ def cmd_status(args, ctx=None): wifi = str(data.get("wifi_signal", "?")).replace("dBm", "") logger.info("🖨️ Bambu Printer Status") - logger.info(f" State: {state}") + _show(f" State: {state}") if state == "RUNNING": hrs, mins = divmod(remaining, 60) - logger.info(f" File: {filename}") - logger.info(f" Progress: {pct}% | Layer {layer}/{total_layers}") - logger.info(f" Time left: {hrs}h {mins}m") - logger.info(f" Bed: {bed_temp}°C / {bed_target}°C") - logger.info(f" Nozzle: {nozzle_temp}°C / {nozzle_target}°C") - logger.info(f" Fan: {fan} | WiFi: {wifi}dBm") + _show(f" File: {filename}") + _show(f" Progress: {pct}% | Layer {layer}/{total_layers}") + _show(f" Time left: {hrs}h {mins}m") + _show(f" Bed: {bed_temp}°C / {bed_target}°C") + _show(f" Nozzle: {nozzle_temp}°C / {nozzle_target}°C") + _show(f" Fan: {fan} | WiFi: {wifi}dBm") if ams: if ams.get("units"): logger.info(" AMS:") for unit in ams["units"]: - logger.info(f" Unit {unit['id']} (humidity {unit['humidity']}, {unit['temp']}°C)") + _show(f" Unit {unit['id']} (humidity {unit['humidity']}, {unit['temp']}°C)") for tray in unit["trays"]: marker = "▶ " if tray["active"] else " " if tray["empty"]: - logger.info(f" {marker}Slot {tray['slot']}: empty") + _show(f" {marker}Slot {tray['slot']}: empty") else: color = f" #{tray['color']}" if tray["color"] else "" remain = f" | {tray['remain']}%" if tray["remain"] is not None else "" - logger.info(f" {marker}Slot {tray['slot']}: {tray['type']}{color}{remain}") + _show(f" {marker}Slot {tray['slot']}: {tray['type']}{color}{remain}") ext_tray = ams.get("external_tray") if ext_tray and not ext_tray.get("empty"): marker = "▶ " if ext_tray.get("active") else " " color = f" #{ext_tray['color']}" if ext_tray.get("color") else "" remain = f" | {ext_tray['remain']}%" if ext_tray.get("remain") is not None else "" - logger.info(f" External Spool: {marker}{ext_tray.get('type')}{color}{remain}") + _show(f" External Spool: {marker}{ext_tray.get('type')}{color}{remain}") diff --git a/bambu_cli/config.py b/bambu_cli/config.py index 98b8b62..33a2e5d 100644 --- a/bambu_cli/config.py +++ b/bambu_cli/config.py @@ -40,13 +40,16 @@ def read_config_json(path): command reject the exact file every other command loads fine. Mirrors ``load_config``'s ``utf-8-sig`` read. Raises ``FileNotFoundError``, - ``OSError``, or ``json.JSONDecodeError`` to the caller — it does not enforce + ``OSError``, or ``ValueError`` (bad JSON, bad UTF-8, or a non-object document) to the caller — it does not enforce permissions or apply the config; use ``load_config`` for that. """ from bambu_cli.paths import expand_path as _expand_path with open(_expand_path(path), encoding="utf-8-sig") as f: - return json.load(f) + data = json.load(f) + if not isinstance(data, dict): + raise ValueError("config.json must contain a JSON object") + return data def load_config(exit_on_fail=True): @@ -456,22 +459,28 @@ def load_access_code(): # old inline-wins precedence meant a user who rotated the code in the file # kept authenticating with a stale inline copy, silently. Warn loudly so the # conflict is visible, then defer to the file branch below. - if "access_code" in cfg and not cfg.get("access_code_file"): + # A non-string access_code_file (null, a number) counts as not set. + code_file = cfg.get("access_code_file") if isinstance(cfg.get("access_code_file"), str) else None + if "access_code" in cfg and not code_file: _warn_inline_access_code_once() - access_code = str(cfg["access_code"]).strip() + if not isinstance(cfg["access_code"], str): + # str(None) is "None", which would be sent to the printer as the password. + logger.error(f"config 'access_code' must be a string, not {type(cfg['access_code']).__name__}.") + abort("", exit_code=EXIT_CONFIG_ERROR) + access_code = cfg["access_code"].strip() problem = _access_code_value_problem(access_code) if problem: logger.error(problem) abort("", exit_code=EXIT_CONFIG_ERROR) return access_code - if "access_code_file" in cfg: + if code_file is not None: if "access_code" in cfg: logger.warning( "⚠️ config.json has BOTH an inline access_code and an access_code_file; " "using the file and ignoring the stale inline value " "(run: plate setup --migrate-access-code to remove the inline key)." ) - path = _expand_path(cfg["access_code_file"]) + path = _expand_path(code_file) _enforce_secret_file_permissions(path, _display_path(path)) try: # utf-8-sig: a hand-created secret file may carry a BOM that .strip() @@ -533,14 +542,18 @@ def config_timeout_problem(value): """Why ``value`` is not a usable timeout (positive, finite seconds), or None.""" import math + from bambu_cli.constants import MAX_TIMEOUT_SECONDS + if isinstance(value, bool): return f"{value!r} is not a number of seconds" try: seconds = float(value) - except (TypeError, ValueError): + except (TypeError, ValueError, OverflowError): return f"{value!r} is not a number of seconds" if not math.isfinite(seconds) or seconds <= 0: return f"{value!r} must be a positive number of seconds" + if seconds > MAX_TIMEOUT_SECONDS: + return f"{value!r} is above the {MAX_TIMEOUT_SECONDS}-second limit" return None diff --git a/bambu_cli/constants.py b/bambu_cli/constants.py index cbe77f9..1c6e0d5 100644 --- a/bambu_cli/constants.py +++ b/bambu_cli/constants.py @@ -70,6 +70,8 @@ def __getattr__(name: str) -> str: DOWNLOAD_TIMEOUT = 60.0 SLICER_TIMEOUT = 120.0 COMMAND_TIMEOUT = 5.0 +# Ceiling for user-supplied timeouts: larger values overflow socket/lock calls (Windows caps them far lower). +MAX_TIMEOUT_SECONDS = 86400 PRINT_ACK_TIMEOUT = 10.0 UPLOAD_TIMEOUT = 300.0 diff --git a/bambu_cli/context.py b/bambu_cli/context.py index bd7a5de..aaf8d40 100644 --- a/bambu_cli/context.py +++ b/bambu_cli/context.py @@ -68,6 +68,15 @@ def _coerce_config_bool(key: str, value: Any, *, default: bool, unreadable: bool return unreadable +def insecure_tls_reading(value: Any) -> bool | None: + """How ``insecure_tls`` is read: True/False for a clear value, None when unreadable.""" + if value is True or value is False: + return bool(value) + if isinstance(value, str) and value.strip().lower() in _TRUE_WORDS | _FALSE_WORDS: + return value.strip().lower() in _TRUE_WORDS + return None + + def _coerce_insecure_tls(value: Any) -> bool: """Strictly resolve the ``insecure_tls`` opt-out, failing CLOSED. @@ -77,10 +86,9 @@ def _coerce_insecure_tls(value: Any) -> bool: believes it is off. So only a clear true enables it; anything unreadable keeps validation on (and warns). """ - if value is True or value is False: - return bool(value) - if isinstance(value, str) and value.strip().lower() in _TRUE_WORDS | _FALSE_WORDS: - return value.strip().lower() in _TRUE_WORDS + reading = insecure_tls_reading(value) + if reading is not None: + return reading from bambu_cli.logging_utils import logger logger.warning( @@ -91,6 +99,27 @@ def _coerce_insecure_tls(value: Any) -> bool: return False +def camera_port_problem(value: Any) -> str | None: + """Why ``value`` is not a usable ``camera_port`` (docker ``-p`` string or 1-65535 int), or None.""" + if isinstance(value, bool) or not isinstance(value, (int, str)): + return f"{value!r} must be a port number (1-65535) or a docker port spec such as 127.0.0.1:1985:1984" + if isinstance(value, int) and not 1 <= value <= 65535: + return f"{value!r} is outside 1-65535" + return None + + +def normalize_camera_port(value: Any, default: str) -> str: + """A docker ``-p`` spec for ``value``; an int becomes a loopback-only mapping to the streamer's port 1984. + + An unusable value gives ``default`` rather than failing the whole config load. + """ + if camera_port_problem(value): + return default + if isinstance(value, int): + return f"127.0.0.1:{value}:1984" + return str(value) + + def _normalize_fingerprint(fp: str | None) -> str | None: """Normalize a pinned SHA-256 fingerprint (lowercase, separator-free).""" from bambu_cli.tlspin import normalize_fingerprint @@ -157,12 +186,13 @@ def from_config(cls, cfg: dict[str, Any]) -> Settings: except ImportError: # pragma: no cover -- config is always importable printer_model = str(raw_model).strip().upper() nozzle_size = str(cfg.get("nozzle", cfg.get("nozzle_size", "0.4"))) - camera_port = cfg.get("camera_port", default_camera_port) + camera_port = normalize_camera_port(cfg.get("camera_port", default_camera_port), default_camera_port) host_port = _stream_host_port(camera_port) camera_stream_url = cfg.get("camera_stream_url", f"http://localhost:{host_port}/api/frame.jpeg?src=p1s") return cls( - printer_ip=cfg.get("printer_ip", "0.0.0.0"), + # A non-string address keeps the placeholder; `config validate` reports it. + printer_ip=cfg["printer_ip"] if isinstance(cfg.get("printer_ip"), str) else "0.0.0.0", serial=cfg.get("serial", "UNKNOWN"), username=cfg.get("username", "bblp"), mqtt_port=cfg.get("mqtt_port", 8883), diff --git a/bambu_cli/contracts/models.py b/bambu_cli/contracts/models.py index ac2b808..e698c6e 100644 --- a/bambu_cli/contracts/models.py +++ b/bambu_cli/contracts/models.py @@ -534,6 +534,9 @@ class JobError(Contract): "remote_name", "workdir", "next_command", + # null when the print command was sent but never acknowledged: + # the printer may be printing, so neither true nor false is known. + "printed", } ) diff --git a/bambu_cli/download/downloader.py b/bambu_cli/download/downloader.py index 320adf6..d31219e 100644 --- a/bambu_cli/download/downloader.py +++ b/bambu_cli/download/downloader.py @@ -28,6 +28,7 @@ _sanitize_download_filename, ) from bambu_cli.download.validation import ( + _iri_to_uri, _known_unsupported_download_extension, _normalize_url_input, _reject_oversized_download, @@ -214,7 +215,7 @@ def _cleanup_reserved(): outpath = os.path.join(outdir, filename) outpath = _reserve(outpath) filename = _portable_basename(outpath) - req = urllib.request.Request(url, headers={**headers, "User-Agent": user_agent_for_url(url)}) + req = urllib.request.Request(_iri_to_uri(url), headers={**headers, "User-Agent": user_agent_for_url(url)}) with polite_open(safe_opener, req, timeout=DOWNLOAD_TIMEOUT) as resp: final_url = _response_url(resp) if final_url and final_url != url: @@ -717,3 +718,9 @@ def _cleanup_reserved(): ) safe_log_error(message) abort("", exit_code=EXIT_NETWORK_ERROR) + except BaseException: + # Ctrl-C (KeyboardInterrupt) skips every handler above. Still remove the + # partial transfer and the 0-byte placeholders, then let it propagate. + _remove_partial_file(partial_path) + _cleanup_reserved() + raise diff --git a/bambu_cli/download/extract.py b/bambu_cli/download/extract.py index 48c1d76..572140c 100644 --- a/bambu_cli/download/extract.py +++ b/bambu_cli/download/extract.py @@ -122,7 +122,9 @@ def _cleanup_reservation(): if extracted > max_bytes: raise ValueError(_archive_member_exceeded_limit_message(member_filename, max_bytes)) dst.write(chunk) - except Exception: + except BaseException: + # BaseException, not Exception: a Ctrl-C mid-extraction must not + # leave a .part file or a 0-byte placeholder behind either. _cleanup_reservation() raise size = os.path.getsize(partial_path) diff --git a/bambu_cli/download/naming.py b/bambu_cli/download/naming.py index d0e37c7..38a9711 100644 --- a/bambu_cli/download/naming.py +++ b/bambu_cli/download/naming.py @@ -19,6 +19,7 @@ from bambu_cli.fsutil import _has_command_injection_chars, _portable_basename from bambu_cli.jsonio import redact_url_credentials as _redact_url_credentials from bambu_cli.logging_utils import logger +from bambu_cli.textsafe import has_unsafe_display_chars def _name_for_message(value): @@ -100,7 +101,11 @@ def _sanitize_download_filename(filename): each of which the printer-side check then rejected. """ filename = _portable_basename(filename) - filename = re.sub(r'[\x00-\x1f\x7f<>:"/\\|?*]', "_", filename).strip(" .") + # C0/C1 controls, DEL and bidi-format characters too: _safe_remote_name + # refuses them, and this repairer's output must always pass that check. + filename = re.sub( + r'[\x00-\x1f\x7f-\x9f<>:"/\\|?*\u200e\u200f\u2028\u2029\u202a-\u202e\u2066-\u2069]', "_", filename + ).strip(" .") if filename in (".", "..") or not filename: return "model.stl" if _reserved_device_stem(filename): @@ -154,7 +159,9 @@ def _safe_remote_name(filename): return None if filename != _portable_basename(filename): return None - if _has_command_injection_chars(filename): + if _has_command_injection_chars(filename) or has_unsafe_display_chars(filename): + # Beyond CR/LF/NUL: ESC, C1 and bidi controls in a name end up in + # terminal output (logs, `plate files`) and can disguise the name. return None if any(c in filename for c in '<>:"/\\|?*\x7f'): return None diff --git a/bambu_cli/download/validation.py b/bambu_cli/download/validation.py index e1bf233..44c77d2 100644 --- a/bambu_cli/download/validation.py +++ b/bambu_cli/download/validation.py @@ -2,7 +2,7 @@ import os import re -from urllib.parse import unquote, urlparse +from urllib.parse import quote, unquote, urlparse, urlsplit, urlunsplit from bambu_cli.argutils import namespace_get as _namespace_get from bambu_cli.constants import ( @@ -18,13 +18,57 @@ from bambu_cli.errors import BambuError, abort from bambu_cli.fsutil import _portable_basename from bambu_cli.jsonio import looks_like_schemeless_credential_url as _looks_like_schemeless_credential_url +from bambu_cli.jsonio import looks_like_split_userinfo as _looks_like_split_userinfo from bambu_cli.jsonio import redact_url_credentials as _redact_url_credentials from bambu_cli.logging_utils import safe_log_error from bambu_cli.paths import expand_path as _expand_path +_SCHEME_PREFIX = re.compile(r"^[A-Za-z][A-Za-z0-9+.-]*://") + + +def _safe_urlparse(value): + """``urlparse`` that returns None instead of raising on a malformed URL. + + ``urlparse`` raises ``ValueError`` for an unbalanced IPv6 bracket such as + ``http://[::1/x.stl``; callers report that as an invalid URL rather than + letting it escape as an unexpected error with a traceback. + """ + try: + return urlparse(value) + except ValueError: + return None + + +# RFC 3986 reserved and unreserved characters, plus "%" so existing escapes +# survive: quoting with this safe set only touches non-ASCII and unsafe bytes. +_URI_SAFE = "!#$%&'()*+,/:;=?@[]~" + + +def _iri_to_uri(url): + """Percent-encode non-ASCII in the path, query and fragment. + + A URL pasted from a browser address bar can hold raw Unicode + (``/files/модель.stl``); http.client sends the request line as ASCII and + would fail with an opaque ``'ascii' codec can't encode`` error. + """ + if url.isascii(): + return url + parts = urlsplit(url) + return urlunsplit( + ( + parts.scheme, + parts.netloc, + quote(parts.path, safe=_URI_SAFE), + quote(parts.query, safe=_URI_SAFE), + quote(parts.fragment, safe=_URI_SAFE), + ) + ) + def _looks_like_url(value): - parsed = urlparse(value) + parsed = _safe_urlparse(value) + if parsed is None: + return bool(_SCHEME_PREFIX.match(value)) return bool(parsed.scheme and "://" in value) @@ -48,8 +92,8 @@ def _normalize_url_input(value): def _is_http_url(value): if any(ord(c) < 32 or ord(c) == 127 or c.isspace() for c in (value or "")): return False - parsed = urlparse(value) - return parsed.scheme.lower() in ("http", "https") and bool(parsed.netloc) + parsed = _safe_urlparse(value) + return parsed is not None and parsed.scheme.lower() in ("http", "https") and bool(parsed.netloc) def _validate_http_url_or_exit(value): @@ -57,7 +101,11 @@ def _validate_http_url_or_exit(value): message = "Invalid URL: contains whitespace or control characters" safe_log_error(message) abort(message, exit_code=EXIT_COMMAND_ERROR, failed_step="validate") - parsed = urlparse(value) + parsed = _safe_urlparse(value) + if parsed is None: + message = "Invalid URL: malformed host" + safe_log_error(message) + abort(message, exit_code=EXIT_COMMAND_ERROR, failed_step="validate") if parsed.scheme.lower() not in ("http", "https"): message = f"Invalid URL scheme: {parsed.scheme or 'none'}" safe_log_error(message) @@ -66,10 +114,22 @@ def _validate_http_url_or_exit(value): message = "Invalid URL: missing host" safe_log_error(message) abort(message, exit_code=EXIT_COMMAND_ERROR, failed_step="validate") - if parsed.username is not None or parsed.password is not None: + if parsed.username is not None or parsed.password is not None or _looks_like_split_userinfo(value, parsed): + # The second form is user:1234/pw@host: it parses as host "user", so the + # request would carry the password in its path to whatever "user" resolves to. message = "Invalid URL: embedded credentials are not supported" safe_log_error(message) abort(message, exit_code=EXIT_COMMAND_ERROR, failed_step="validate") + try: + _ = parsed.port + except ValueError: + # Usually a password holding "/", "?" or "#" (user:pa/ss@host): the + # parser then reads the password as the port. The message must not + # echo it, and http.client's own "nonnumeric port: ''" must + # never be reached. + message = "Invalid URL: the port is not a number (a password in the URL is not supported)" + safe_log_error(message) + abort(message, exit_code=EXIT_COMMAND_ERROR, failed_step="validate") def _validate_download_url_or_exit(args, source_url, normalized_source, url, failed_step, label): @@ -138,10 +198,23 @@ def _known_unsupported_content_type(content_type): return None +def _names_a_model_file(url): + """True when the URL path ends in a supported model or print extension.""" + parsed = _safe_urlparse(url or "") + if parsed is None: + return False + return _file_extension(_portable_basename(unquote(parsed.path))) in DOWNLOADABLE_EXTENSIONS + + def _reject_unsupported_content_type(args, source_url, normalized_source, url, content_type): media_type = _known_unsupported_content_type(content_type) if not media_type: return + # ASCII STL, OBJ, STEP and G-code are text, and many hosts (GitHub raw among + # them) serve every file as text/plain. When the URL itself names a model + # file, text/plain says nothing about whether it is one. + if media_type == "text/plain" and _names_a_model_file(url): + return message = f"Download URL returned unsupported content type '{media_type}', not a model file." abort( message, @@ -160,7 +233,7 @@ def _max_download_mb_error(args): max_download_mb = _namespace_get(args, "max_download_mb", DEFAULT_MAX_DOWNLOAD_MB) try: max_download_mb = int(max_download_mb) - except (TypeError, ValueError): + except (TypeError, ValueError, OverflowError): max_download_mb = 0 if max_download_mb <= 0: return "--max-download-mb must be a positive integer" diff --git a/bambu_cli/gcode_limits.py b/bambu_cli/gcode_limits.py new file mode 100644 index 0000000..212b020 --- /dev/null +++ b/bambu_cli/gcode_limits.py @@ -0,0 +1,225 @@ +"""Hold G-code heater commands to the same limits slice overrides obey. + +``plate gcode`` sends one line straight to the printer, and a pre-sliced 3MF +(downloaded, or exported by another slicer) carries G-code that platecli never +generated. Both must stay under the 350 °C nozzle / 150 °C bed ceilings that +``slice --set`` already enforces: a typo such as ``M104 S2200`` must not reach +the heater because it came in by another path. + +Rank 10: stdlib and ``constants`` only. +""" + +import re +import zipfile +import zlib + +from bambu_cli.constants import MAX_BED_TEMP_C, MAX_NOZZLE_TEMP_C + +__all__ = [ + "MAX_LINE_BYTES", + "file_heater_problem", + "heater_problem", + "peak_heater_targets", + "zip_member_heater_problem", +] + +# One G-code word: a letter and the numeric text right after it. +_WORD = re.compile(r"([A-Za-z])[ \t]*([-+]?[0-9.]*)", re.ASCII) +_PLAIN_TEMPERATURE = re.compile(r"\+?[0-9]+(?:\.[0-9]+)?", re.ASCII) +# Heater commands and the limit their S (and Marlin's R) temperature must stay under. +_HEATERS = { + 104: ("nozzle", MAX_NOZZLE_TEMP_C), + 109: ("nozzle", MAX_NOZZLE_TEMP_C), + 140: ("bed", MAX_BED_TEMP_C), + 190: ("bed", MAX_BED_TEMP_C), +} +# A file line whose command is a heater command (after an optional line number). +# Firmware runs the first word of a line, so only those lines can set a heater. +_HEATER_LINE = re.compile(rb"[ \t]*(?:[Nn][0-9]+[ \t]*)?[Mm][ \t]*0*1(?:04|09|40|90)(?![0-9])") + + +def heater_problem(gcode): + """Why ``gcode`` would set a heater past the limits, or None. + + Deliberately conservative: comments are scanned too, and a temperature + that is not a plain decimal (``S2e3``, ``S0x3E8``, ``S250T0``) is refused + because firmware parsers read those forms differently. + """ + if not gcode.isascii(): + return "G-code must be plain ASCII." + heater = None + for match in _WORD.finditer(gcode): + letter, value = match.group(1).upper(), match.group(2) + # T is a parameter here, not a new command: "M104 T0 S3000" heats tool 0. + if letter in ("G", "M"): + heater = None + if letter == "M" and _PLAIN_TEMPERATURE.fullmatch(value): + heater = _HEATERS.get(int(float(value))) + continue + if heater is None or letter not in ("S", "R"): + continue + label, limit = heater + end = match.end() + if not _PLAIN_TEMPERATURE.fullmatch(value) or (end < len(gcode) and gcode[end].isalpha()): + token = gcode[match.start() :].split(None, 1)[0].split(";", 1)[0] + return f"Unreadable {label} temperature '{token}'; write it as a plain number such as {letter}220." + if float(value) > limit: + return f"{label.capitalize()} temperature {value} °C is above the {limit} °C limit." + return None + + +# Firmware ends a line at LF, CR or CRLF. Files are read in bounded chunks so a +# member with no line breaks cannot pull gigabytes into memory. +_EOL = re.compile(rb"\r\n|\r|\n") +_CHUNK = 1 << 20 +MAX_LINE_BYTES = 1 << 20 +_READ_ERRORS = (OSError, EOFError, RuntimeError, NotImplementedError, zipfile.BadZipFile, zlib.error) + + +class _LineTooLong(ValueError): + pass + + +def _lines(fh): + """Every line of the binary stream ``fh``; a line over MAX_LINE_BYTES raises _LineTooLong.""" + pending = b"" + count = 0 + while True: + chunk = fh.read(_CHUNK) + if not chunk: + break + data = pending + chunk + # Hold back a trailing CR: the LF that would make it CRLF may start the next chunk. + held = b"\r" if data.endswith(b"\r") else b"" + parts = _EOL.split(data[:-1] if held else data) + pending = parts.pop() + held + count += len(parts) + yield from parts + if len(pending) > MAX_LINE_BYTES: + raise _LineTooLong(count + 1) + if pending: + yield from _EOL.split(pending) + + +def _heater_codes(fh): + """``(line number, code)`` for every heater-command line: the part before ``;``. + + Comments in a sliced file are prose ("; set nozzle temperature"), not + parameters; only the code before ";" reaches the firmware. + """ + for number, raw in enumerate(_lines(fh), 1): + if _HEATER_LINE.match(raw): + yield number, raw.split(b";", 1)[0].decode("utf-8", errors="replace").strip() + + +def _lines_problem(fh, member): + try: + for number, code in _heater_codes(fh): + problem = heater_problem(code) + if problem: + return f"{member} line {number}: {problem}" + except _LineTooLong as exc: + return f"{member} line {exc.args[0]} is over {MAX_LINE_BYTES >> 20} MiB, too long to check" + return None + + +def _gcode_members(archive): + # infolist, not namelist: a name can appear twice, and open(name) returns + # only the last entry with it, so the first would never be read. + return [info for info in archive.infolist() if info.filename.lower().endswith(".gcode")] + + +def _archive_problem(source): + try: + archive = zipfile.ZipFile(source) + except _READ_ERRORS as exc: + return f"could not open it as a 3MF to check its temperatures ({type(exc).__name__})" + with archive: + for info in _gcode_members(archive): + try: + with archive.open(info) as fh: + problem = _lines_problem(fh, info.filename) + except _READ_ERRORS as exc: + return f"could not read {info.filename} to check its temperatures ({type(exc).__name__})" + if problem: + return problem + return None + + +def file_heater_problem(path): + """Why the G-code in ``path`` (a .gcode file or a sliced 3MF) breaks the limits, or None. + + Every ``.gcode`` entry of a 3MF is checked, not only the plate about to be + printed: the plate can be chosen later, and a file on the printer can be + started from its own screen. Anything that cannot be read is refused, since + it cannot be checked. + """ + name = str(path).lower() + if name.endswith(".gcode"): + try: + with open(path, "rb") as fh: + return _lines_problem(fh, _basename(path)) + except OSError as exc: + return f"could not read it to check its temperatures ({type(exc).__name__})" + if name.endswith(".3mf"): + return _archive_problem(path) + return None + + +def zip_member_heater_problem(archive_path, info): + """``file_heater_problem`` for the ZIP entry ``info`` without extracting it (job --dry-run).""" + name = info.filename.lower() + if not name.endswith((".gcode", ".3mf")): + return None + try: + with zipfile.ZipFile(archive_path) as outer, outer.open(info) as fh: + if name.endswith(".gcode"): + return _lines_problem(fh, _basename(info.filename)) + return _archive_problem(fh) + except _READ_ERRORS as exc: + return f"could not read {info.filename} to check its temperatures ({type(exc).__name__})" + + +def _targets(code): + heater = None + for match in _WORD.finditer(code): + letter, value = match.group(1).upper(), match.group(2) + if letter in ("G", "M"): + heater = None + if letter == "M" and _PLAIN_TEMPERATURE.fullmatch(value): + heater = _HEATERS.get(int(float(value))) + elif heater is not None and letter in ("S", "R") and _PLAIN_TEMPERATURE.fullmatch(value): + yield heater[0], float(value) + + +def _peaks_in(fh, peaks): + for _number, code in _heater_codes(fh): + for label, value in _targets(code): + peaks[label] = max(value, peaks.get(label, value)) + + +def peak_heater_targets(path): + """The highest nozzle and bed targets ``path``'s G-code sets: ``{"nozzle": 220.0, ...}``. + + For a confirmation screen, so the person approving a print sees what the + file will actually heat to. ``{}`` when nothing is set; None when the + G-code cannot be read. + """ + peaks: dict[str, float] = {} + name = str(path).lower() + try: + if name.endswith(".gcode"): + with open(path, "rb") as fh: + _peaks_in(fh, peaks) + elif name.endswith(".3mf"): + with zipfile.ZipFile(path) as archive: + for info in _gcode_members(archive): + with archive.open(info) as fh: + _peaks_in(fh, peaks) + except (*_READ_ERRORS, ValueError): + return None + return peaks + + +def _basename(path): + return str(path).replace("\\", "/").rsplit("/", 1)[-1] diff --git a/bambu_cli/interactive/core.py b/bambu_cli/interactive/core.py index fa78307..8f309bf 100644 --- a/bambu_cli/interactive/core.py +++ b/bambu_cli/interactive/core.py @@ -34,6 +34,7 @@ from bambu_cli.interactive.presets import parse_args_or_abort, preset_to_job_args from bambu_cli.paths import expand_path as _expand_path from bambu_cli.slicer.estimate import format_estimate, read_3mf_estimate +from bambu_cli.textsafe import display_safe MATERIAL_CHOICES = ["PLA", "PETG", "ABS", "TPU"] MATERIAL_GUIDANCE = { @@ -502,9 +503,11 @@ def run_prepare_pipeline(steps: Any, state: WizardState, workdir: str) -> str: def preview_rows(state: WizardState, model_path: str) -> list[tuple[str, str]]: """Return the ``(label, value)`` rows of the pre-print preview. - Pure formatting over the state plus the current settings — both front-ends - render the same four rows, including the pre-sliced caveat that stops us - implying the chosen material was applied to a file we did not slice. + Formatting over the state plus the current settings — both front-ends + render the same rows, including the pre-sliced caveat that stops us + implying the chosen material was applied to a file we did not slice. Feed + and Temperatures say what the print will actually do (which spool, how hot + the G-code heats), so the person confirming sees it before anything moves. """ from bambu_cli.config import MODEL_MAPPING from bambu_cli.context import current_settings @@ -516,7 +519,7 @@ def preview_rows(state: WizardState, model_path: str) -> list[tuple[str, str]]: full_name = model_info.get("full_name", settings.printer_model or "printer") quality_layer = QUALITY_LAYER_HEIGHTS.get(state.quality, "") - name = _portable_basename(model_path) + name = display_safe(_portable_basename(model_path)) ext = _file_extension(model_path) if ext == ".gcode": @@ -541,10 +544,11 @@ def preview_rows(state: WizardState, model_path: str) -> list[tuple[str, str]]: ("Model", name), ("Printer", f"{full_name}, {settings.nozzle_size}mm nozzle"), ("Material", material_line), + ("Feed", _feed_line(state)), + ("Temperatures", _temperatures_line(state.printable_path or model_path)), ("Estimate", estimate_line), ] - # Only when something was actually overridden: the wizard never sets any, so - # its preview keeps exactly the four lines it has always printed. + # Only when something was actually overridden. if state.sliced and not state.overrides.is_empty(): rows.append(("Overrides", state.overrides.summary())) return rows @@ -555,6 +559,69 @@ def preview_rows(state: WizardState, model_path: str) -> list[tuple[str, str]]: # --------------------------------------------------------------------------- +def ams_slot_label(slot: int) -> str: + """``AMS0·2`` for absolute tray 2: the label the TUI's AMS panel shows.""" + return f"AMS{slot // 4}·{slot % 4}" + + +def ams_feed_slot(state: WizardState) -> int | None: + """The absolute AMS tray the print will feed from, or None for the external spool. + + The AMS feeds only when detection found a firm active slot and the user kept + the detected material: then that slot holds exactly what was sliced for. + """ + if ( + state.detected_ams_material is not None + and state.detected_ams_slot is not None + and state.material == state.detected_ams_material + ): + return state.detected_ams_slot + return None + + +def ams_change_problem(detector: Callable[..., Any], args: argparse.Namespace, state: WizardState) -> str | None: + """Re-read the AMS right before a print that feeds from it; why not to start, or None. + + The slot is detected when the material is chosen, and slicing can take + minutes. A spool swapped or reloaded in between would otherwise be fed at + the temperatures sliced for the old filament. A read that fails cannot + confirm the slot, so it also stops the print (upload only still works). + """ + slot = ams_feed_slot(state) + if slot is None: + return None + material, now_slot = detect_ams_material(detector, args) + if material == state.detected_ams_material and now_slot == slot: + return None + was = f"{state.detected_ams_material} in {ams_slot_label(slot)}" + if material is None or now_slot is None: + return ( + f"Could not confirm that {was} is still the loaded filament, so the print was not started. " + "Check the AMS and try again, or upload only." + ) + return ( + f"The AMS changed since this file was prepared: it had {was}, now {material} in " + f"{ams_slot_label(now_slot)}. The print was not started; prepare the file again for the loaded filament." + ) + + +def _feed_line(state: WizardState) -> str: + slot = ams_feed_slot(state) + if slot is None: + return "External spool" + return f"{ams_slot_label(slot)} ({state.detected_ams_material})" + + +def _temperatures_line(path: str) -> str: + from bambu_cli.gcode_limits import peak_heater_targets + + peaks = peak_heater_targets(path) + if peaks is None: + return "could not read the G-code" + parts = [f"{label} up to {peaks[label]:g} °C" for label in ("nozzle", "bed") if label in peaks] + return " · ".join(parts) if parts else "not set in the G-code" + + def build_job_namespace(state: WizardState, args: argparse.Namespace, *, confirm: bool) -> argparse.Namespace: """Build the ``job`` namespace for the prepared printable file. @@ -581,13 +648,10 @@ def build_job_namespace(state: WizardState, args: argparse.Namespace, *, confirm # refuses to let firmware pick a default tray), so we supply the detected # active slot. If detection failed, the user picked a different material, or # we have no firm active slot, keep the conservative external-spool default. - if ( - state.detected_ams_material is not None - and state.detected_ams_slot is not None - and state.material == state.detected_ams_material - ): + feed_slot = ams_feed_slot(state) + if feed_slot is not None: job_ns.use_ams = True - job_ns.ams_mapping = str(state.detected_ams_slot) + job_ns.ams_mapping = str(feed_slot) # Carry global request flags (sim / verbose) through so the same simulated # printer path runs end to end. if bool(getattr(args, "sim", False)): @@ -655,6 +719,7 @@ def decline_message(state: WizardState) -> str: """ kept = preserve_printable(state) if kept: + kept = display_safe(kept) label = "File kept at" if not state.sliced else "Sliced file kept at" return f"Nothing sent. {label} {kept}" return "Nothing sent." diff --git a/bambu_cli/interactive/prompts.py b/bambu_cli/interactive/prompts.py index d9f8256..80eb095 100644 --- a/bambu_cli/interactive/prompts.py +++ b/bambu_cli/interactive/prompts.py @@ -85,7 +85,9 @@ def confirm( return CANCELLED def print(self, message: str = "") -> None: # pragma: no cover -- interactive prompt - self.console.print(message) + # Markup off: the text carries file names and errors, and "[remix]" or + # "x[/b]" must print literally rather than vanish or raise MarkupError. + self.console.print(message, markup=False, emoji=False) def is_cancelled(value: object) -> bool: diff --git a/bambu_cli/interactive/session.py b/bambu_cli/interactive/session.py index 9a20ecf..979a2d9 100644 --- a/bambu_cli/interactive/session.py +++ b/bambu_cli/interactive/session.py @@ -34,8 +34,9 @@ EXIT_COMMAND_ERROR, EXIT_CONFIG_ERROR, EXIT_FILE_ERROR, + EXIT_PRINTER_ERROR, ) -from bambu_cli.errors import abort +from bambu_cli.errors import BambuError, abort from bambu_cli.interactive.core import MATERIAL_CHOICES as _MATERIAL_CHOICES from bambu_cli.interactive.core import MATERIAL_GUIDANCE as _MATERIAL_GUIDANCE from bambu_cli.interactive.core import QUALITY_CHOICES as _QUALITY_CHOICES @@ -43,6 +44,7 @@ from bambu_cli.interactive.core import SUPPORTED_SOURCE_EXTS as _SUPPORTED_SOURCE_EXTS # noqa: F401 from bambu_cli.interactive.core import GoSteps as GoSteps from bambu_cli.interactive.core import WizardState as WizardState +from bambu_cli.interactive.core import ams_change_problem as _ams_change_problem from bambu_cli.interactive.core import build_job_namespace as _build_job_namespace from bambu_cli.interactive.core import cleanup_workdir as _cleanup_workdir from bambu_cli.interactive.core import decline_message as _decline_message @@ -57,6 +59,7 @@ from bambu_cli.interactive.core import under_workdir as _under_workdir # noqa: F401 from bambu_cli.interactive.core import validate_source as _validate_source from bambu_cli.interactive.prompts import Prompts, is_cancelled +from bambu_cli.textsafe import display_safe __all__ = [ "GoDeps", @@ -173,7 +176,7 @@ def _step_source(args: argparse.Namespace, deps: GoDeps, state: WizardState) -> if source is not None: state.source = source return - prompts.print(error or "Invalid source.") + prompts.print(display_safe(error) or "Invalid source.") if attempt < _MAX_URL_ATTEMPTS - 1: prompts.print(f"Try again ({_MAX_URL_ATTEMPTS - attempt - 1} attempts left).") @@ -269,8 +272,10 @@ def _step_prepare(args: argparse.Namespace, deps: GoDeps, state: WizardState) -> def _render_preview(prompts: Any, state: WizardState, model_path: str) -> None: prompts.print("") - for label, value in _preview_rows(state, model_path): - prompts.print(f"{label:<11}{value}") + rows = _preview_rows(state, model_path) + width = max(len(label) for label, _ in rows) + 2 + for label, value in rows: + prompts.print(f"{label:<{width}}{value}") prompts.print("") @@ -279,12 +284,27 @@ def _render_preview(prompts: Any, state: WizardState, model_path: str) -> None: # --------------------------------------------------------------------------- +def _kept_note(state: WizardState) -> str: + return _decline_message(state).replace("Nothing sent. ", "", 1).replace("Nothing sent.", "") + + def _run_print(args: argparse.Namespace, deps: GoDeps, state: WizardState, *, confirm: bool) -> None: """Drive cmd_job on the sliced/printer-ready file (upload, optionally print).""" + if confirm: + problem = _ams_change_problem(deps.get_steps().get_ams_material(), args, state) + if problem is not None: + deps.get_prompts().print(f"{problem} {_kept_note(state)}".strip()) + abort(problem, exit_code=EXIT_PRINTER_ERROR, failed_step="ams", logged=True) job_ns = _build_job_namespace(state, args, confirm=confirm) utils._LAST_ERROR_PAYLOAD = None utils._LAST_DOWNLOAD_PAYLOAD = None - deps.get_steps().get_job()(job_ns) + try: + deps.get_steps().get_job()(job_ns) + except BambuError: + # The slice succeeded; losing it to a flaky upload would waste the + # minutes it took, so keep it exactly as a decline does. + deps.get_prompts().print(f"Upload or print failed. {_kept_note(state)}".strip()) + raise def _step_confirm_print(args: argparse.Namespace, deps: GoDeps, state: WizardState) -> None: diff --git a/bambu_cli/job/orchestrate.py b/bambu_cli/job/orchestrate.py index a5b64e8..5583a67 100644 --- a/bambu_cli/job/orchestrate.py +++ b/bambu_cli/job/orchestrate.py @@ -37,6 +37,7 @@ _validate_http_url_or_exit, ) from bambu_cli.errors import BambuError +from bambu_cli.gcode_limits import file_heater_problem, zip_member_heater_problem from bambu_cli.job.payload import _parse_print_options, _print_next_command from bambu_cli.job.predict import ( _ext_would_slice, @@ -334,6 +335,16 @@ def _run_job(ctx, args, steps=None): EXIT_FILE_ERROR, f"Refusing to dry-run an empty ZIP member: {member_filename}", ) + # The real run's upload refuses a hot member; the dry run must say so too. + heater = zip_member_heater_problem(source_path, info) + if heater: + _job_fail( + args, + summary, + "validate", + EXIT_FILE_ERROR, + f"Refusing to upload {os.path.basename(member_filename)}: {heater}", + ) logger.info( "🔍 Dry Run: ZIP archive contains a supported file; skipping extraction, slicing, upload, and print." ) @@ -488,6 +499,16 @@ def _run_job(ctx, args, steps=None): EXIT_FILE_ERROR, f"Refusing to dry-run an empty printer-ready file: {_path_for_message(source_path)}", ) + heater = file_heater_problem(source_path) + if heater: + # The real run's upload refuses this file; the dry run must say so. + _job_fail( + args, + summary, + "validate", + EXIT_FILE_ERROR, + f"Refusing to upload {os.path.basename(source_path)}: {heater}", + ) logger.info("🔍 Dry Run: printer-ready file is valid; skipping upload and print.") summary["status"] = "dry_run_local_skipped" summary["printable_path"] = source_path @@ -571,6 +592,10 @@ def _run_job(ctx, args, steps=None): ) except BambuError as exc: detail = _last_error_for("print", ctx, exc) + if detail and detail.get("sent") is True: + # The print command went out unacknowledged: it may be printing, + # so "printed: false" would invite a retry and a second print. + summary["printed"] = None summary["next_command"] = ["status", "--json"] summary["recovery_hint"] = ( "Upload succeeded but print start was not confirmed. Check printer status before retrying." diff --git a/bambu_cli/job/payload.py b/bambu_cli/job/payload.py index f9740cb..0f25dae 100644 --- a/bambu_cli/job/payload.py +++ b/bambu_cli/job/payload.py @@ -3,6 +3,7 @@ from __future__ import annotations import json +import re from urllib.parse import quote from bambu_cli.argutils import namespace_get as _namespace_get @@ -77,15 +78,14 @@ def _parse_print_options(args): return None, None if not use_ams: return None, "--ams-mapping requires --use-ams" - try: - clean_mapping = raw_mapping.strip("[]") - mapping = [int(x.strip()) for x in clean_mapping.split(",")] - except ValueError: - return None, "Invalid AMS mapping format. Use comma-separated integers like '0' or '0,1,2'" - if not mapping: - return None, "Invalid AMS mapping format. Use comma-separated integers like '0' or '0,1,2'" - if any(slot < 0 for slot in mapping): + parts = [x.strip() for x in str(raw_mapping).strip("[]").split(",")] + if any(re.fullmatch(r"-[0-9]+", part) for part in parts): return None, "Invalid AMS mapping format. Slot indexes must be zero or positive integers like '0' or '0,1,2'" + # ASCII digits only: int() alone reads "1_0" as 10 and a fullwidth "1" as 1, + # so a typo would silently select a different tray (and material). + if not all(re.fullmatch(r"[0-9]+", part) for part in parts): + return None, "Invalid AMS mapping format. Use comma-separated integers like '0' or '0,1,2'" + mapping = [int(part) for part in parts] if any(slot > MAX_AMS_SLOT_INDEX for slot in mapping): return ( None, diff --git a/bambu_cli/jsonio.py b/bambu_cli/jsonio.py index 4d83e1f..0a341ab 100644 --- a/bambu_cli/jsonio.py +++ b/bambu_cli/jsonio.py @@ -10,11 +10,13 @@ over every emitted string. """ +import re from urllib.parse import urlparse, urlunparse __all__ = [ "json_mode_requested", "looks_like_schemeless_credential_url", + "looks_like_split_userinfo", "redact_url_credentials", ] @@ -38,12 +40,84 @@ def looks_like_schemeless_credential_url(value): return False +def _strip_through_last_at(text): + """Conservative scrub for URL text the parser cannot read. + + Drops everything between the scheme separator and the last ``@``, which is + where a browser-style reader would put the userinfo. Over-redacting an + already-invalid URL loses nothing; echoing a password does. + """ + sep = text.find("://") + start = sep + 3 if sep >= 0 else (2 if text.startswith("//") else 0) + at = text.rfind("@") + if at < start: + return text + return text[:start] + text[at + 1 :] + + +def _port_is_invalid(parsed): + try: + _ = parsed.port + except ValueError: + return True + return False + + +_AUTHORITY_END = re.compile(r"[/?#]") + + +def _after_netloc(text, parsed): + start = text.find("//") + at = text.find(parsed.netloc, start + 2 if start >= 0 else 0) + return text[at + len(parsed.netloc) :] if at >= 0 else "" + + +def looks_like_split_userinfo(text, parsed=None): + """True for ``user:12/pw@host``: a password holding ``/``, ``?`` or ``#``. + + The parser ends the authority at the first of those, so when the text + before it is a valid port (digits, or nothing) the URL parses cleanly as + host ``user`` and port ``12``, with ``pw@host`` in the path, query or + fragment. A segment that starts with ``@`` (``/@scope/pkg``) is a path. + """ + if parsed is None: + try: + parsed = urlparse(text) + except ValueError: + return False + if not parsed.netloc or "@" in parsed.netloc or ":" not in parsed.netloc: + return False + rest = _after_netloc(text, parsed) + if not rest or rest[0] not in "/?#": + return False + segment = _AUTHORITY_END.split(rest[1:], maxsplit=1)[0] + return "@" in segment and not segment.startswith("@") + + def redact_url_credentials(value): """Return URL text with any userinfo removed before logging or JSON output.""" text = str(value or "") if "@" not in text: return value - parsed = urlparse(text) + try: + parsed = urlparse(text) + except ValueError: + # e.g. an unbalanced IPv6 bracket after the userinfo: user:pw@[x/a.stl + return _strip_through_last_at(text) + after_netloc = _after_netloc(text, parsed) if parsed.netloc else "" + if parsed.netloc and _port_is_invalid(parsed) and "@" in after_netloc: + # A password holding "/", "?" or "#" (user:pa/ss@host) makes the parser + # read "user" as the host and the password as the port, so there is no + # userinfo to strip and the secret would be echoed as-is. + return _strip_through_last_at(text) + if looks_like_split_userinfo(text, parsed): + # The same with a password that starts with digits: user:1234/pw@host. + return _strip_through_last_at(text) + if len(parsed.scheme) > 1 and not parsed.netloc and text[len(parsed.scheme) + 1 :].startswith(("/", "\\")): + # https:///user:pw@host, https:/user:pw@host, https:\\user:pw@host: no + # authority for the parser, but a lenient reader still finds userinfo. + # (A one-letter "scheme" is a Windows drive: C:\\Users\\a@b\\x.stl.) + return _strip_through_last_at(text) if "://" not in text and looks_like_schemeless_credential_url(text): redacted = redact_url_credentials(f"https://{text}") prefix = "https://" diff --git a/bambu_cli/netsafety.py b/bambu_cli/netsafety.py index 5ef3cf2..9c14232 100644 --- a/bambu_cli/netsafety.py +++ b/bambu_cli/netsafety.py @@ -382,7 +382,7 @@ def _retry_after_seconds(err) -> float: raw = headers.get("Retry-After") if headers else None try: seconds = float(str(raw).strip()) - except (TypeError, ValueError): + except (TypeError, ValueError, OverflowError): return MIN_HOST_REQUEST_INTERVAL return max(0.0, min(seconds, MAX_RETRY_AFTER_WAIT)) diff --git a/bambu_cli/printables/client.py b/bambu_cli/printables/client.py index 7f3bf7b..bfbd7af 100644 --- a/bambu_cli/printables/client.py +++ b/bambu_cli/printables/client.py @@ -218,9 +218,12 @@ def get_download_link(file_id, model_id, file_type, file_name, headers, opener): raise PrintablesContractChanged("Printables returned an unexpected getDownloadLink payload.") if link_payload.get("ok"): - link = (link_payload.get("output") or {}).get("link") - if link: + output = link_payload.get("output") or {} + link = output.get("link") if isinstance(output, dict) else None + if isinstance(link, str) and link: return link, file_name + if link: + raise PrintablesContractChanged("Printables returned a download link that is not a string.") raise PrintablesContractChanged("Printables reported success but returned no download link.") errs = link_payload.get("errors") or [] diff --git a/bambu_cli/printer.py b/bambu_cli/printer.py index e97e230..d1646e1 100644 --- a/bambu_cli/printer.py +++ b/bambu_cli/printer.py @@ -143,9 +143,12 @@ def upload_file( timeout: float | None = None, progress_callback=None, on_resume=None, - sleep=time.sleep, + sleep=None, ) -> bool: """Upload a file via FTPS as a verified state machine: (fresh|probe) -> (resume|restart) -> transfer -> verify.""" + # Resolve the default at call time, not definition time, so a patched + # time.sleep takes effect (a default bound at import costs real seconds). + sleep = sleep or time.sleep if _has_command_injection_chars(remote_path): logger.error(f"Upload failed: unsafe remote path {remote_path!r}") return False diff --git a/bambu_cli/protocols/camera.py b/bambu_cli/protocols/camera.py index ca3951c..0bfef75 100644 --- a/bambu_cli/protocols/camera.py +++ b/bambu_cli/protocols/camera.py @@ -77,12 +77,14 @@ def _recv_exact(sock_, n): tls = ctx.wrap_socket(sock, server_hostname=printer.ip) tls.settimeout(timeout) - if not printer.insecure_tls and printer.cert_fingerprint: + # A configured pin is enforced even when insecure_tls is set, as on + # MQTT and FTPS: the user named the exact certificate to trust. + if printer.cert_fingerprint: from bambu_cli.tlspin import verify_cert_fingerprint der = tls.getpeercert(binary_form=True) verify_cert_fingerprint(der, printer.cert_fingerprint, exc_factory=_CameraPinMismatch) - elif not printer.insecure_tls and not printer.cert_fingerprint: + elif not printer.insecure_tls: raise ssl.SSLError( "No cert_fingerprint pinned for camera connection; run 'plate setup' to pin one, or set insecure_tls to bypass (not recommended)" ) diff --git a/bambu_cli/protocols/ftps.py b/bambu_cli/protocols/ftps.py index a47d3a0..6cb9172 100644 --- a/bambu_cli/protocols/ftps.py +++ b/bambu_cli/protocols/ftps.py @@ -120,7 +120,17 @@ def ntransfercmd(self, cmd, rest=None): secure = getattr(self, "_secure_data", False) or getattr(self, "_prot_p", False) if secure and isinstance(self.sock, ssl.SSLSocket): session = self.sock.session - conn = self.sock.context.wrap_socket(conn, server_hostname=self.host, session=session) + try: + conn = self.sock.context.wrap_socket(conn, server_hostname=self.host, session=session) + except BaseException: + # ssl closes the descriptor itself when the handshake fails, but + # not for errors raised before it takes ownership (a socket-type + # or hostname check). Closing a socket ssl already detached is a no-op. + try: + conn.close() + except OSError: + pass + raise printer = self.printer pin = printer.cert_fingerprint if printer is not None else None if pin: diff --git a/bambu_cli/protocols/mqtt_cmd.py b/bambu_cli/protocols/mqtt_cmd.py index 037302b..822766d 100644 --- a/bambu_cli/protocols/mqtt_cmd.py +++ b/bambu_cli/protocols/mqtt_cmd.py @@ -5,6 +5,7 @@ import json import ssl import threading +from collections.abc import Iterable from bambu_cli.errors import PrinterStatusIncomplete from bambu_cli.logging_utils import logger @@ -14,6 +15,49 @@ TERMINAL_GCODE_STATES = frozenset({"FINISH", "FAILED", "STOP", "IDLE"}) +def decode_report(msg): + """Parse an MQTT payload into a dict, or None for anything else. + + Runs on paho's network thread, where an escaping exception kills the loop + for good (a held session then never recovers). Besides malformed JSON this + covers what a spoofed or broken report can make ``json.loads`` raise: a + RecursionError for deeply nested arrays and a ValueError for an integer + past the digit limit. Non-object JSON (``5``, ``[1]``) is not a report. + """ + try: + data = json.loads(msg.payload.decode("utf-8")) + except (ValueError, RecursionError, AttributeError, TypeError) as exc: + logger.debug(f"MQTT decode error: {type(exc).__name__}") + return None + if not isinstance(data, dict): + return None + if _nested_deeper_than(data, _MAX_REPORT_DEPTH): + # json.loads accepts a few thousand levels; the recursive walks that + # redact and emit JSON do not. Real reports are about six deep. + logger.debug("MQTT report dropped: nested too deeply") + return None + return data + + +_MAX_REPORT_DEPTH = 32 + + +def _nested_deeper_than(value, limit): + stack = [(value, 1)] + while stack: + item, depth = stack.pop() + if isinstance(item, dict): + children: Iterable[object] = item.values() + elif isinstance(item, list): + children = item + else: + continue + if depth > limit: + return True + stack.extend((child, depth + 1) for child in children) + return False + + def _client_factory(client_factory): if client_factory is not None: return client_factory @@ -233,12 +277,8 @@ def on_connect( status_received.set() def on_message(client, userdata, msg, status_received=status_received): - try: - data = json.loads(msg.payload.decode("utf-8")) - except (json.JSONDecodeError, UnicodeDecodeError) as e: - logger.debug(f"MQTT decode error: {e}") - return - if not isinstance(data, dict) or not isinstance(data.get("print"), dict): + data = decode_report(msg) + if data is None or not isinstance(data.get("print"), dict): return with merged_lock: merged.update(data["print"]) @@ -323,9 +363,8 @@ def on_connect(client, userdata, flags, rc, properties=None): received.set() def on_message(client, userdata, msg): - try: - data = json.loads(msg.payload.decode("utf-8")) - except (json.JSONDecodeError, UnicodeDecodeError): + data = decode_report(msg) + if data is None: return info = data.get("info") if isinstance(info, dict) and info.get("command") == "get_version" and "module" in info: diff --git a/bambu_cli/protocols/mqtt_monitor.py b/bambu_cli/protocols/mqtt_monitor.py index 477b378..c1236b5 100644 --- a/bambu_cli/protocols/mqtt_monitor.py +++ b/bambu_cli/protocols/mqtt_monitor.py @@ -18,6 +18,7 @@ _connect, _sleep, ) +from bambu_cli.textsafe import display_safe from bambu_cli.utils import get_sequence_id @@ -27,7 +28,7 @@ def _status_event(p, event): def _int(value, default=0): try: return int(value) - except (TypeError, ValueError): + except (TypeError, ValueError, OverflowError): return default return StatusEvent( @@ -74,7 +75,9 @@ def monitor_status(args, printer): terminal_states = TERMINAL_GCODE_STATES received_terminal = threading.Event() show_progress_bar = not json_mode and sys.stdout.isatty() - client = _client_factory(None)(printer) + # The monitor keeps paho's auto-reconnect: it runs for the length of a + # print, and its only publish is a QoS-0 pushall, which paho never resends. + client = _client_factory(None)(printer, reconnect=True) userdata: dict = {} client.user_data_set(userdata) @@ -126,16 +129,20 @@ def on_message(client, userdata, msg): ) progress.start() userdata["progress"] = progress - userdata["task_id"] = progress.add_task(f"State: {state}", total=100, completed=pct) + userdata["task_id"] = progress.add_task( + display_safe(f"State: {state}"), total=100, completed=pct + ) except ImportError: userdata["progress"] = None if userdata.get("progress"): - userdata["progress"].update(userdata["task_id"], completed=pct, description=f"State: {state}") + userdata["progress"].update( + userdata["task_id"], completed=pct, description=display_safe(f"State: {state}") + ) elif json_mode: emit_json_line(_status_event(p, "update")) else: - logger.info(f"⏳ Status: State={state}, Progress={pct}%") + logger.info(display_safe(f"⏳ Status: State={state}, Progress={pct}%")) last_state[0] = state last_pct[0] = pct @@ -144,7 +151,7 @@ def on_message(client, userdata, msg): if json_mode: emit_json_line(_status_event(p, "terminal")) else: - logger.info(f"🏁 Reached terminal state: {state}") + logger.info(display_safe(f"🏁 Reached terminal state: {state}")) received_terminal.set() except (UnicodeDecodeError, json.JSONDecodeError) as e: logger.debug(f"MQTT decode error: {e}") diff --git a/bambu_cli/protocols/mqtt_print.py b/bambu_cli/protocols/mqtt_print.py index f426fec..8ddead7 100644 --- a/bambu_cli/protocols/mqtt_print.py +++ b/bambu_cli/protocols/mqtt_print.py @@ -2,13 +2,12 @@ from __future__ import annotations -import json import threading from bambu_cli.config import get_command_timeout from bambu_cli.errors import BambuError, abort from bambu_cli.logging_utils import logger -from bambu_cli.protocols.mqtt_cmd import _client_factory, _connect, status_is_complete +from bambu_cli.protocols.mqtt_cmd import _client_factory, _connect, decode_report, status_is_complete def _printer_error_hex(code: object) -> str | None: @@ -18,6 +17,28 @@ def _printer_error_hex(code: object) -> str | None: return f"0x{code & 0xFFFFFFFF:08X}" +def _abort_unconfirmed(basename, detail, exit_code): + """The print command went out but its acknowledgement never came. + + The printer may well be printing, so the outcome is unknown, not "not + printed": an agent reading ``printed: false`` would retry and start a second + print. Report ``printed: null`` with ``sent: true`` and point at status. + """ + from bambu_cli.utils import record_error_detail + + message = ( + f"The print command for {basename} was sent, but the printer did not acknowledge it {detail}. " + "It may already be printing; check `plate status` before printing again." + ) + logger.error(message) + fields = {"file": basename, "printed": None, "sent": True, "acknowledged": False} + next_command = ["status", "--json"] + record_error_detail("print", exit_code, message, failed_step="print", next_command=next_command, **fields) + # The exception carries the fields too: the CLI builds the --json envelope + # and the job summary from the raised error, not from the recorded detail. + abort(message, exit_code=exit_code, failed_step="print", next_command=next_command, extra=fields, logged=True) + + def execute_print_command( printer, payload, @@ -111,8 +132,8 @@ def on_connect(client, userdata, flags, rc, properties=None): def on_message(client, userdata, msg): try: - data = json.loads(msg.payload.decode("utf-8")) - if "print" in data: + data = decode_report(msg) + if data is not None and isinstance(data.get("print"), dict): p = data["print"] is_ack = p.get("command") == "project_file" if is_ack: @@ -127,8 +148,6 @@ def on_message(client, userdata, msg): if isinstance(result, str) and result.strip().lower() not in ("", "success"): reject_reason[0] = str(p.get("reason") or result) command_accepted.set() - except (UnicodeDecodeError, json.JSONDecodeError) as e: - logger.debug(f"MQTT decode error: {e}") except Exception as e: logger.warning(f"MQTT message handling error: {e}") @@ -143,6 +162,8 @@ def on_message(client, userdata, msg): client.loop_start() accepted = command_accepted.wait(print_ack_timeout) if not accepted: + if published[0]: + _abort_unconfirmed(basename, f"within {print_ack_timeout:g}s", EXIT_TIMEOUT) message = f"Timed out waiting for printer to acknowledge print start for {basename}" logger.error(message) record_error_detail("print", EXIT_TIMEOUT, message, failed_step="print", file=basename, printed=False) @@ -150,6 +171,8 @@ def on_message(client, userdata, msg): except BambuError: raise except Exception as e: + if published[0]: + _abort_unconfirmed(basename, f"({e})", EXIT_NETWORK_ERROR) message = f"Error: {e}" logger.error(message) record_error_detail("print", EXIT_NETWORK_ERROR, message, failed_step="print", file=basename, printed=False) diff --git a/bambu_cli/protocols/mqtt_session.py b/bambu_cli/protocols/mqtt_session.py index 6c55a04..847cd54 100644 --- a/bambu_cli/protocols/mqtt_session.py +++ b/bambu_cli/protocols/mqtt_session.py @@ -21,7 +21,7 @@ from bambu_cli.errors import PrinterStatusIncomplete from bambu_cli.logging_utils import logger -from bambu_cli.protocols.mqtt_cmd import _REQUIRED_STATUS_KEYS, status_is_complete +from bambu_cli.protocols.mqtt_cmd import _REQUIRED_STATUS_KEYS, decode_report, status_is_complete from bambu_cli.utils import get_sequence_id ClientFactory = Callable[[Any], Any] @@ -140,12 +140,8 @@ def _on_disconnect( self._live = False def _on_message(self, client: Any, userdata: Any, msg: Any) -> None: - try: - data = json.loads(msg.payload.decode("utf-8")) - except (json.JSONDecodeError, UnicodeDecodeError, AttributeError) as exc: - logger.debug(f"MQTT decode error: {exc}") - return - if not isinstance(data, dict): + data = decode_report(msg) + if data is None: return info = data.get("info") if isinstance(info, dict) and info.get("command") == "get_version" and "module" in info: diff --git a/bambu_cli/protocols/mqtt_tls.py b/bambu_cli/protocols/mqtt_tls.py index 3906ee4..17dada3 100644 --- a/bambu_cli/protocols/mqtt_tls.py +++ b/bambu_cli/protocols/mqtt_tls.py @@ -10,6 +10,7 @@ from __future__ import annotations +import secrets import socket import ssl @@ -125,23 +126,43 @@ def probe_cert_fingerprint(host, port=990, timeout=5): return fingerprint_sha256(tls.getpeercert(binary_form=True)) -def create_mqtt_client(printer, client_id=""): +def _paho_client(client_id="", *, reconnect=False): + """A paho client that, by default, never reconnects on its own. + + paho keeps an unacknowledged QoS-1 PUBLISH and resends it (DUP set) when + its network loop reconnects after a drop, so a command that reached the + printer just before the connection died would run twice. Command callers + own reconnection: one-shot commands retry with a fresh client only while + nothing was published, and ``MqttSession.ensure_connected`` builds a new + client (with an empty in-flight queue) after a drop. ``reconnect=True`` is + for the status monitor, which publishes only an idempotent QoS-0 pushall. + + The client ID is never empty: paho's fallback for a broker that rejects an + empty ID is a reconnect, which ``reconnect=False`` also switches off. + """ + client_id = client_id or f"plate-{secrets.token_hex(8)}" + try: + return mqtt.Client(mqtt.CallbackAPIVersion.VERSION2, client_id, reconnect_on_failure=reconnect) + except AttributeError: + return mqtt.Client(client_id, reconnect_on_failure=reconnect) + + +def create_mqtt_client(printer, client_id="", *, reconnect=False): if printer.simulation_mode: return _SimMqttClient() _require_mqtt() - try: - client = mqtt.Client(mqtt.CallbackAPIVersion.VERSION2, client_id) - except AttributeError: - client = mqtt.Client(client_id) + client = _paho_client(client_id, reconnect=reconnect) client.username_pw_set("bblp", printer.access_code) - if printer.insecure_tls: - client.tls_set(cert_reqs=ssl.CERT_NONE) - client.tls_insecure_set(True) - elif printer.cert_fingerprint: + # A configured pin wins over insecure_tls: the user named the exact + # certificate to trust, and FTPS already enforces it in that case. + if printer.cert_fingerprint: client.tls_set_context(pinning_ssl_context(printer.cert_fingerprint)) client.tls_insecure_set(True) + elif printer.insecure_tls: + client.tls_set(cert_reqs=ssl.CERT_NONE) + client.tls_insecure_set(True) else: client.tls_set(cert_reqs=ssl.CERT_REQUIRED) return client @@ -154,7 +175,7 @@ def _mqtt_port(printer): return 8883 try: port = int(raw) - except (TypeError, ValueError): + except (TypeError, ValueError, OverflowError): return 8883 if port < 1 or port > 65535: return 8883 diff --git a/bambu_cli/setup_cmd/config_cmd.py b/bambu_cli/setup_cmd/config_cmd.py index 441bb21..ba01278 100644 --- a/bambu_cli/setup_cmd/config_cmd.py +++ b/bambu_cli/setup_cmd/config_cmd.py @@ -25,6 +25,8 @@ "serial", "printer-model", "timeouts", + "cert-fingerprint", + "camera-port", "access-code", "access-code-inline-conflict", "access-code-permissions", @@ -63,7 +65,7 @@ def _cmd_config_show(args): # utf-8-sig via the canonical reader: a Windows-editor BOM must not make # `config show` reject the exact file every other command loads fine. config = read_config_json(config_path) - except (OSError, json.JSONDecodeError) as exc: + except (OSError, ValueError) as exc: message = f"Could not read config: {_exception_for_message(exc)}" emit_json_error( args, diff --git a/bambu_cli/setup_cmd/migrate.py b/bambu_cli/setup_cmd/migrate.py index 6739063..0e36cf9 100644 --- a/bambu_cli/setup_cmd/migrate.py +++ b/bambu_cli/setup_cmd/migrate.py @@ -1,6 +1,5 @@ """Migrate an inline access_code from config.json into a separate secret file.""" -import json import os from bambu_cli.argutils import namespace_get as _namespace_get @@ -186,7 +185,7 @@ def _cmd_migrate_access_code(args): logger.error(message) _setup_json_error(args, message) abort("", exit_code=EXIT_CONFIG_ERROR) - except (OSError, json.JSONDecodeError) as exc: + except (OSError, ValueError) as exc: message = f"Could not migrate access code: {_exception_for_message(exc)}" logger.error(message) _setup_json_error(args, message) diff --git a/bambu_cli/setup_cmd/preflight.py b/bambu_cli/setup_cmd/preflight.py index bc075d3..c0c9a83 100644 --- a/bambu_cli/setup_cmd/preflight.py +++ b/bambu_cli/setup_cmd/preflight.py @@ -9,6 +9,7 @@ from bambu_cli.config import _access_code_value_problem, load_config from bambu_cli.constants import EXIT_CONFIG_ERROR, EXIT_SUCCESS +from bambu_cli.context import insecure_tls_reading from bambu_cli.errors import abort from bambu_cli.logging_utils import logger @@ -93,6 +94,27 @@ def _timeouts_check(cfg): return _preflight_result("ok", "timeouts", "Configured timeouts are valid.") +def _cert_fingerprint_check(cfg): + """Error when a present ``cert_fingerprint`` cannot work as a pin.""" + from bambu_cli.tlspin import fingerprint_config_problem + + problem = fingerprint_config_problem(cfg.get("cert_fingerprint")) + if problem: + return _preflight_result("error", "cert-fingerprint", f"config.json {problem}.") + return _preflight_result("ok", "cert-fingerprint", "Certificate pin is valid or not set.") + + +def _camera_port_check(cfg): + """Error when ``camera_port`` is neither a docker port spec nor a 1-65535 integer.""" + from bambu_cli.context import camera_port_problem + + if "camera_port" in cfg: + problem = camera_port_problem(cfg["camera_port"]) + if problem: + return _preflight_result("error", "camera-port", f"config.json 'camera_port': {problem}.") + return _preflight_result("ok", "camera-port", "camera_port is valid or not set.") + + def collect_preflight_checks(): """Collect local install/config checks without contacting the printer.""" from bambu_cli.context import current_config, current_settings @@ -133,19 +155,30 @@ def collect_preflight_checks(): if config_permissions: checks.append(config_permissions) if "insecure_tls" in cfg and not isinstance(cfg["insecure_tls"], bool): - # A non-boolean insecure_tls (e.g. the string "false") is ignored and - # TLS validation stays ON (fail-closed); flag it so the user is not - # confused about whether they disabled it. + # Say what the runtime actually does, using the same reading it uses. + reading = insecure_tls_reading(cfg["insecure_tls"]) + if reading is None: + outcome = "it is ignored and TLS certificate validation stays ENABLED" + elif reading: + outcome = "it is honoured as true and TLS certificate validation is DISABLED" + else: + outcome = "it is read as false and TLS certificate validation stays ENABLED" checks.append( _preflight_result( "warning", "insecure-tls", f"config.json 'insecure_tls' is not a JSON boolean ({cfg['insecure_tls']!r}); " - "it is ignored and TLS certificate validation stays ENABLED. Use true/false.", + f"{outcome}. Use true/false.", ) ) printer_ip = cfg.get("printer_ip") - if _looks_like_placeholder(printer_ip, {"0.0.0.0", "192.168.0.XXX", "PRINTER_IP"}): + if printer_ip is not None and not isinstance(printer_ip, str): + checks.append( + _preflight_result( + "error", "printer-ip", "config.json 'printer_ip' must be a string address or hostname." + ) + ) + elif _looks_like_placeholder(printer_ip, {"0.0.0.0", "192.168.0.XXX", "PRINTER_IP"}): checks.append( _preflight_result("error", "printer-ip", "Config must contain a real printer_ip or hostname.") ) @@ -160,10 +193,14 @@ def collect_preflight_checks(): checks.append(_printer_model_check(cfg)) checks.append(_timeouts_check(cfg)) + checks.append(_cert_fingerprint_check(cfg)) + checks.append(_camera_port_check(cfg)) access_code = cfg.get("access_code") has_inline_code = bool(access_code) access_file = cfg.get("access_code_file") + if access_file is not None and not isinstance(access_file, str): + access_file = None # not a path: treated as unset, as at runtime if access_file: expanded = _expand_path(access_file) if os.path.exists(expanded): @@ -202,6 +239,12 @@ def collect_preflight_checks(): "error", "access-code", f"Access code file not found: {_display_path(access_file)}" ) ) + elif "access_code" in cfg and not isinstance(access_code, str): + checks.append( + _preflight_result( + "error", "access-code", "config.json 'access_code' must be a string (or use access_code_file)." + ) + ) elif has_inline_code and _access_code_value_problem(access_code): checks.append(_preflight_result("error", "access-code", _access_code_value_problem(access_code))) elif has_inline_code: diff --git a/bambu_cli/setup_cmd/wizard.py b/bambu_cli/setup_cmd/wizard.py index ad5d354..57986e4 100644 --- a/bambu_cli/setup_cmd/wizard.py +++ b/bambu_cli/setup_cmd/wizard.py @@ -1,5 +1,6 @@ """The `setup` command: guided/non-interactive config creation and mDNS discovery.""" +import ipaddress import os import re import socket @@ -31,6 +32,7 @@ _validate_setup_access_code_file, _write_setup_config, ) +from bambu_cli.textsafe import display_safe from bambu_cli.utils import emit_json @@ -40,7 +42,7 @@ def _service_info_address(info): if callable(parsed_addresses): try: addresses = list(parsed_addresses()) - except (TypeError, ValueError): + except (TypeError, ValueError, OverflowError): addresses = [] if addresses: return addresses[0] @@ -57,6 +59,66 @@ def _service_info_address(info): raise ValueError("service did not advertise a usable IP address") +def _discovered_address_problem(ip): + """``(usable, why)`` for an address a printer on this network would not have, or None. + + mDNS is unauthenticated: anything on the LAN can answer, and the access + code is sent to whatever address is configured. An address no printer can + have (unspecified, multicast) is unusable; one a printer here would not + have (this computer, the public internet) needs the user's explicit yes. + """ + try: + addr = ipaddress.ip_address(str(ip).split("%", 1)[0]) + except ValueError: + return False, f"{display_safe(str(ip))} is not an IP address" + if addr.is_unspecified or addr.is_multicast: + return False, f"{ip} cannot be a printer's address" + if addr.is_loopback: + return True, f"{ip} is this computer, not a printer" + if addr.is_private or addr.is_link_local: + return None + return True, f"{ip} is not a local network address" + + +def _choose_discovered(args, discovered): + """The discovered printer the user confirms, or None to enter one by hand. + + A lone result is shown and confirmed rather than taken: on a network with + a spoofed answer it is the only thing between the access code and a + stranger. Choosing one of several by number is the confirmation, unless + its address is flagged. + """ + several = len(discovered) > 1 + logger.info("\nSelect a printer:" if several else "\nFound one printer:") + for i, d in enumerate(discovered): + prefix = f" [{i}] " if several else " " + logger.info(f"{prefix}{display_safe(d['name'])} ({d['ip']})") + flagged = _discovered_address_problem(d["ip"]) + if flagged: + logger.warning(f" ⚠️ {flagged[1]}") + if several: + choice = _prompt_text("Choice: ", args) + try: + idx = int(choice) + except ValueError: + logger.error(f"Invalid input: '{display_safe(choice)}'. Enter a number.") + abort("", exit_code=EXIT_COMMAND_ERROR) + if idx < 0 or idx >= len(discovered): + logger.error(f"Invalid selection: {display_safe(choice)}. Must be 0-{len(discovered) - 1}.") + abort("", exit_code=EXIT_COMMAND_ERROR) + selected = discovered[idx] + else: + selected = discovered[0] + flagged = _discovered_address_problem(selected["ip"]) + if flagged: + answer = _prompt_text(f"{flagged[1]}. Use it anyway? [y/N]: ", args).strip().lower() + return selected if answer in ("y", "yes") else None + if several: + return selected + answer = _prompt_text("Use this printer? [Y/n]: ", args).strip().lower() + return selected if answer in ("", "y", "yes") else None + + def _parse_mdns_printer_identity(name): """Return (serial, model) from a Bambu mDNS service name. @@ -371,16 +433,20 @@ def add_service(self, zc, type_, name): try: ip = _service_info_address(info) except ValueError as e: - logger.warning(f"⚠️ Skipping {name}: {e}") + logger.warning(f"⚠️ Skipping {display_safe(name)}: {e}") + return + unusable = _discovered_address_problem(ip) + if unusable and not unusable[0]: + logger.warning(f"⚠️ Skipping {display_safe(name)}: {unusable[1]}") return service_key = (name, ip) with discovery_lock: if service_key in seen_services: - logger.debug(f"Ignoring duplicate mDNS service: {name} at {ip}") + logger.debug(f"Ignoring duplicate mDNS service: {display_safe(name)} at {ip}") return seen_services.add(service_key) discovered.append({"name": name, "ip": ip, "info": info}) - logger.info(f" ✨ Found: {name} at {ip}") + logger.info(f" ✨ Found: {display_safe(name)} at {ip}") def update_service(self, zc, type_, name): pass @@ -413,7 +479,15 @@ def remove_service(self, zc, type_, name): except Exception: pass - if use_manual: + selected = None + if not use_manual: + if not discovered: + logger.error("No printers found. Ensure printer is on the same network.") + abort("", exit_code=EXIT_NETWORK_ERROR) + selected = _choose_discovered(args, discovered) + use_manual = selected is None + + if selected is None: ip = _prompt_text("Enter Printer IP Address (e.g. 192.168.1.50): ", args) if not ip: logger.error("IP Address is required.") @@ -423,31 +497,9 @@ def remove_service(self, zc, type_, name): logger.error("Serial Number is required.") abort("", exit_code=EXIT_CONFIG_ERROR) detected_model = None + logger.info("\nConfiguring manual printer...") else: - if not discovered: - logger.error("No printers found. Ensure printer is on the same network.") - abort("", exit_code=EXIT_NETWORK_ERROR) - - # Simple selection if multiple - if len(discovered) > 1: - logger.info("\nSelect a printer:") - for i, d in enumerate(discovered): - logger.info(f" [{i}] {d['name']} ({d['ip']})") - choice = _prompt_text("Choice: ", args) - try: - idx = int(choice) - if idx < 0 or idx >= len(discovered): - logger.error(f"Invalid selection: {choice}. Must be 0-{len(discovered) - 1}.") - abort("", exit_code=EXIT_COMMAND_ERROR) - selected = discovered[idx] - except ValueError: - logger.error(f"Invalid input: '{choice}'. Enter a number.") - abort("", exit_code=EXIT_COMMAND_ERROR) - else: - selected = discovered[0] - ip = selected["ip"] - if not use_manual: # Extract serial/model from names like # "BBLP-P1P-SN123._bblp._tcp.local." without saving "P1P-SN123" # as the MQTT serial. @@ -457,9 +509,7 @@ def remove_service(self, zc, type_, name): f"⚠️ Printer discovered via unauthenticated mDNS. Verify that the reported IP " f"({selected['ip']}) belongs to your actual printer to protect your access code!" ) - logger.info(f"\nConfiguring {selected['name']}...") - else: - logger.info("\nConfiguring manual printer...") + logger.info(f"\nConfiguring {display_safe(selected['name'])}...") access_code = _prompt_interactive_access_code(args) # Guided prompt for model & nozzle diff --git a/bambu_cli/slicer/cmd.py b/bambu_cli/slicer/cmd.py index af61b13..1b5c2c0 100644 --- a/bambu_cli/slicer/cmd.py +++ b/bambu_cli/slicer/cmd.py @@ -28,6 +28,7 @@ from bambu_cli.slicer.orca import _build_orcaslicer_cmd, _run_orcaslicer from bambu_cli.slicer.output import _finalize_slice, _output_snapshot from bambu_cli.slicer.profiles import ( + _available_layer_heights, _create_temp_machine, _create_temp_profiles, _default_bed_type, @@ -39,6 +40,22 @@ from bambu_cli.utils import _ensure_output_dir, emit_json_error +def _produced_is_accepted(result, produced: str) -> bool: + """True when the work-dir file is a result ``_finalize_slice`` would accept.""" + from bambu_cli.slicer.output import SliceOutcome, _classify_slice_result, _is_valid_sliced_3mf + + size = os.path.getsize(produced) + outcome = _classify_slice_result( + returncode=result.returncode if result is not None else -1, + stdout_stderr=((result.stdout or "") + (result.stderr or "")) if result is not None else "", + fresh=True, + file_exists=True, + file_size=size, + is_valid_3mf=_is_valid_sliced_3mf(produced) if size > 0 else False, + ) + return outcome in (SliceOutcome.SUCCESS, SliceOutcome.BENIGN_GL_WARNING) + + def _list_settings(args: argparse.Namespace, settings) -> str: """Discovery: dump every settable OrcaSlicer process/filament key + example. @@ -180,6 +197,9 @@ def cmd_slice( ) layer = quality_map.get(requested_quality, f"0.20mm Standard @BBL {model_code}") process_file = f"{layer}.json" + # An unmapped layer height must resolve to a profile of that height; the + # Standard file existing must not satisfy it. + numeric_quality = requested_quality not in quality_map and str(requested_quality).startswith("0.") outdir = _expand_path(args.output) if args.output else os.path.dirname(os.path.abspath(source_filepath)) if outdir.startswith("-"): @@ -216,16 +236,23 @@ def cmd_slice( requested_filament = str(requested_filament_name).lower() if os.path.isdir(filament_dir): - files = os.listdir(filament_dir) - for f in files: - if requested_filament in f.lower() and "@base" in f: - filament = os.path.join(filament_dir, f) - break - if not filament: - for f in files: - if requested_filament in f.lower(): - filament = os.path.join(filament_dir, f) - break + files = sorted(f for f in os.listdir(filament_dir) if requested_filament in f.lower()) + base_files = [f for f in files if "@base" in f] + candidates = base_files or files + exact_stems = {requested_filament, f"bambu {requested_filament}"} + + def _filament_rank(name: str) -> tuple[bool, int, str]: + stem = name[: -len(".json")] if name.endswith(".json") else name + stem = stem.lower().replace(" @base", "").strip() + return (stem not in exact_stems, len(name), name) + + if candidates: + best = min(candidates, key=_filament_rank) + filament = os.path.join(filament_dir, best) + if len(candidates) > 1: + logger.info( + f"Filament '{requested_filament_name}' matched {len(candidates)} profiles; using {best}" + ) if not filament or not os.path.exists(filament): logger.warning( @@ -277,7 +304,7 @@ def cmd_slice( detected_profiles_dir=detected_profiles, ) - if not os.path.exists(process): + if numeric_quality or not os.path.exists(process): compatible_printer = f"{full_model_name} {settings.nozzle_size} nozzle" discovered_process = _discover_process_profile( args.quality, @@ -285,7 +312,21 @@ def cmd_slice( model_code=model_code, compatible_printer=compatible_printer, profiles_dir=settings.profiles_dir, + fallback=not numeric_quality, ) + if not discovered_process and numeric_quality: + heights = _available_layer_heights(settings.profiles_dir, model_code, compatible_printer) + emit_json_error( + args, + "slice", + EXIT_COMMAND_ERROR, + f"No {requested_quality} mm process profile for {full_model_name}. " + f"Available layer heights: {', '.join(heights) if heights else 'none found'}.", + failed_step="validate", + file=filepath, + quality=str(requested_quality), + available_layer_heights=heights, + ) if not discovered_process: hint, detected_profiles = _profiles_dir_diagnostic(settings.profiles_dir) if hint: @@ -302,6 +343,8 @@ def cmd_slice( ) assert discovered_process is not None # for type checkers; emit_json_error is NoReturn above process = discovered_process + if numeric_quality: + layer = os.path.basename(process)[: -len(".json")] for path, name in [(machine, "machine"), (filament, "filament")]: if not os.path.exists(path): @@ -432,8 +475,14 @@ def cmd_slice( produced = os.path.join(work_dir, outfile) if os.path.isfile(produced): # Only a file OrcaSlicer actually wrote replaces outpath, so - # _finalize_slice's freshness check still catches a stale one. - os.replace(produced, outpath) + # _finalize_slice's freshness check still catches a stale one. A + # failed run must not overwrite an earlier good output; with none + # to protect, the file moves anyway so _finalize_slice can report + # exactly what was wrong with it. + if os.path.exists(outpath) and not _produced_is_accepted(result, produced): + os.unlink(produced) + else: + os.replace(produced, outpath) finally: if work_dir is not None: shutil.rmtree(work_dir, ignore_errors=True) diff --git a/bambu_cli/slicer/options.py b/bambu_cli/slicer/options.py index 5ab9865..8fe6229 100644 --- a/bambu_cli/slicer/options.py +++ b/bambu_cli/slicer/options.py @@ -62,7 +62,7 @@ def _known_setting_keys(profiles_dir: str, kind: str) -> dict[str, Any]: command for the exact same directories (static over execution lifetime). """ result: dict[str, Any] = {} - for path in sorted(glob.glob(os.path.join(profiles_dir, kind, "*.json"))): + for path in sorted(glob.glob(os.path.join(glob.escape(os.path.join(profiles_dir, kind)), "*.json"))): try: with open(path, encoding="utf-8") as f: data = json.load(f) @@ -226,7 +226,10 @@ def _temperature_values(value: Any) -> list[float] | None: if isinstance(item, bool): return None if isinstance(item, (int, float)): - out.append(float(item)) + try: + out.append(float(item)) + except OverflowError: + return None continue if not isinstance(item, str): return None @@ -234,7 +237,10 @@ def _temperature_values(value: Any) -> list[float] | None: token = token.strip() if not _PLAIN_NUMBER_RE.fullmatch(token): return None - out.append(float(token)) + try: + out.append(float(token)) + except OverflowError: + return None return out or None diff --git a/bambu_cli/slicer/profiles.py b/bambu_cli/slicer/profiles.py index 38c4c19..2e3e4a2 100644 --- a/bambu_cli/slicer/profiles.py +++ b/bambu_cli/slicer/profiles.py @@ -93,9 +93,13 @@ def _discover_process_profile( model_code: str = "P1P", compatible_printer: str | None = None, profiles_dir: str | None = None, + fallback: bool = True, ) -> str | None: """Find the process profile for ``quality_arg`` on this printer. + With ``fallback=False`` a layer height with no profile returns ``None`` + instead of borrowing the 0.20 mm standard one. + A file belongs to the model when its name ends in ``@BBL `` exactly (``"A1"`` must not match ``"... @BBL A1M"``), or when its ``compatible_printers`` lists ``compatible_printer`` -- how the P1S, X1 and @@ -112,6 +116,12 @@ def _discover_process_profile( profiles_dir = current_settings().profiles_dir requested = quality_map.get(quality_arg, f"0.20mm Standard @BBL {model_code}") layer_height = quality_arg if quality_arg.startswith("0.") else requested.split(" ")[0] + if not fallback: + try: + # "0.1" must mean 0.10mm, not the prefix of 0.12mm. + layer_height = f"{float(quality_arg):.2f}mm" + except ValueError: + pass requested_parts = requested.split(" @BBL ")[0].split(" ", 1) quality_word = requested_parts[1] if len(requested_parts) > 1 else None proc_dir = os.path.join(profiles_dir, "process") @@ -137,6 +147,8 @@ def choose(prefix: str, word: str | None) -> str | None: if process_file: logger.debug(f"Profile auto-discovered: {process_file}") return os.path.join(proc_dir, process_file) + if not fallback: + return None process_file = choose("0.20mm", "Standard") if process_file: logger.warning(f"⚠️ Requested quality not found, using: {process_file}") @@ -147,6 +159,25 @@ def choose(prefix: str, word: str | None) -> str | None: return None +def _available_layer_heights(profiles_dir: str, model_code: str, compatible_printer: str | None) -> list[str]: + """Layer heights (e.g. ``0.12``) that have a process profile for this printer.""" + proc_dir = os.path.join(profiles_dir, "process") + if not os.path.isdir(proc_dir): + return [] + heights: set[str] = set() + for name in os.listdir(proc_dir): + if not name.endswith(".json") or "nozzle" in name: + continue + match = re.match(r"(\d+\.\d+)mm", name) + if not match: + continue + if name[: -len(".json")].endswith(f"@BBL {model_code}") or ( + compatible_printer and _process_profile_compatible(os.path.join(proc_dir, name), compatible_printer) + ): + heights.add(match.group(1)) + return sorted(heights) + + def _load_flattened_profile(path: str) -> dict: """Load an OrcaSlicer profile with its ``inherits`` chain merged in. diff --git a/bambu_cli/textsafe.py b/bambu_cli/textsafe.py new file mode 100644 index 0000000..bcdce93 --- /dev/null +++ b/bambu_cli/textsafe.py @@ -0,0 +1,44 @@ +"""Make untrusted text safe to show in a terminal. + +Names that come from outside the process (the printer's FTP listing, MQTT +report fields such as ``gcode_file`` or an AMS ``tray_type``, mDNS instance +names, Printables model names) can carry terminal control sequences. Rich and +Textual stop *markup* injection when text is wrapped in ``Text()``, but they +pass ESC through, so ``\\x1b]52;c;...`` can write the user's clipboard and +``\\x1b[2J`` can clear the screen. Bidi overrides can make ``evil.gcode`` read +as something else on a confirmation line. + +Rank 10: stdlib only, no knowledge of config, transport or commands. +""" + +__all__ = ["display_safe", "has_unsafe_display_chars"] + +# Bidi embedding/override/isolate controls, the implicit marks, and the Unicode +# line and paragraph separators (which some terminals treat as newlines). +_UNSAFE_FORMAT_CHARS = frozenset( + chr(code) for code in (0x200E, 0x200F, 0x2028, 0x2029, *range(0x202A, 0x202F), *range(0x2066, 0x206A)) +) + + +def _is_unsafe(char: str) -> bool: + code = ord(char) + # C0 controls (including ESC, CR, LF, TAB), DEL, and C1 controls (0x9b is + # an 8-bit CSI that some terminals honour). + return code < 0x20 or 0x7F <= code <= 0x9F or char in _UNSAFE_FORMAT_CHARS + + +def has_unsafe_display_chars(value) -> bool: + """True if *value* holds a control or bidi-format character.""" + return any(_is_unsafe(c) for c in str(value if value is not None else "")) + + +def display_safe(value, replacement: str = "?") -> str: + """Return *value* as text with every control or bidi-format character replaced. + + ``None`` becomes the empty string. Printable text, including non-ASCII + letters, is returned unchanged. + """ + text = str(value if value is not None else "") + if not has_unsafe_display_chars(text): + return text + return "".join(replacement if _is_unsafe(c) else c for c in text) diff --git a/bambu_cli/tlspin.py b/bambu_cli/tlspin.py index 8d5b0b1..2050039 100644 --- a/bambu_cli/tlspin.py +++ b/bambu_cli/tlspin.py @@ -29,6 +29,8 @@ import ssl from collections.abc import Callable +from bambu_cli.errors import ConfigError + # A normalized SHA-256 fingerprint: exactly 64 lowercase hex chars, no separators. _HEX64_RE = re.compile(r"\A[0-9a-f]{64}\Z") @@ -37,13 +39,38 @@ def normalize_fingerprint(fp: str | None) -> str | None: """Normalize a SHA-256 fingerprint to lowercase, separator-free hex. Strips ``:`` and ASCII spaces and lowercases, so ``AA:BB``, ``aa bb`` and - ``aabb`` compare equal. Returns ``None`` for ``None``/empty input. + ``aabb`` compare equal. Returns ``None`` for ``None``/empty input. A + non-string (a hand-edited ``"cert_fingerprint": 5``) is a config error, not + an ``AttributeError`` traceback. """ + if fp is None: + return None + if not isinstance(fp, str): + raise ConfigError(f"config 'cert_fingerprint' must be a string, not {type(fp).__name__}") if not fp: return None return fp.lower().replace(":", "").replace(" ", "") +def fingerprint_config_problem(value: object) -> str | None: + """Why a configured ``cert_fingerprint`` cannot work as a pin, or None. + + ``None`` (key absent/null) is "no pin" and fine; a present value that is blank, + not a string, or not 64 hex chars would otherwise silently mean "no pin" or + fail only at connect time. + """ + if value is None: + return None + if not isinstance(value, str): + return f"'cert_fingerprint' must be a string, not {type(value).__name__}" + normalized = normalize_fingerprint(value) + if not normalized: + return "'cert_fingerprint' is blank, which means no pin; remove it or set the printer's SHA-256" + if not _HEX64_RE.match(normalized): + return "'cert_fingerprint' is not a 64-hex-character SHA-256" + return None + + def fingerprint_sha256(der_cert: bytes | None) -> str | None: """Hex SHA-256 of a DER-encoded certificate, or None if no cert.""" if not der_cert: diff --git a/bambu_cli/tui/screens/confirm.py b/bambu_cli/tui/screens/confirm.py index f6423db..45b019c 100644 --- a/bambu_cli/tui/screens/confirm.py +++ b/bambu_cli/tui/screens/confirm.py @@ -43,7 +43,7 @@ from textual.widgets import Button, Footer, Static from bambu_cli.errors import BambuError -from bambu_cli.interactive.core import build_job_namespace, cleanup_workdir, decline_message +from bambu_cli.interactive.core import ams_change_problem, build_job_namespace, cleanup_workdir, decline_message # Outcome kinds handed back to the prepare screen. PRINTED = "printed" @@ -164,6 +164,9 @@ def _job_worker(self, confirm: bool) -> None: # worker prints; a future one that does must not rely on stdout. buffer = io.StringIO() try: + problem = ams_change_problem(self._deps.get_ams_detector(), self._args, self._state) if confirm else None + if problem is not None: + raise BambuError(problem) job_ns = build_job_namespace(self._state, self._args, confirm=confirm) with contextlib.redirect_stdout(buffer), contextlib.redirect_stderr(buffer): self._deps.get_steps().get_job()(job_ns) diff --git a/bambu_cli/tui/services.py b/bambu_cli/tui/services.py index 348306e..48322a6 100644 --- a/bambu_cli/tui/services.py +++ b/bambu_cli/tui/services.py @@ -13,6 +13,8 @@ from dataclasses import dataclass, field from typing import Any +from bambu_cli.textsafe import display_safe + @dataclass class StatusSnapshot: @@ -108,7 +110,7 @@ def status_lines(snapshot: StatusSnapshot) -> list[tuple[str, str]]: strings. On a failed snapshot returns a single explanatory row. """ if not snapshot.ok: - return [("State", snapshot.error or "Printer unreachable.")] + return [("State", display_safe(snapshot.error) or "Printer unreachable.")] raw = snapshot.raw @@ -122,7 +124,7 @@ def _temp(cur_key: str, target_key: str) -> str: return f"{cur}°C" rows: list[tuple[str, str]] = [ - ("State", str(raw.get("gcode_state", "UNKNOWN"))), + ("State", display_safe(raw.get("gcode_state", "UNKNOWN"))), ("Progress", f"{raw.get('mc_percent', 0)}%"), ("Nozzle", _temp("nozzle_temper", "nozzle_target_temper")), ("Bed", _temp("bed_temper", "bed_target_temper")), @@ -132,9 +134,9 @@ def _temp(cur_key: str, target_key: str) -> str: if layer is not None or total_layer is not None: rows.append(("Layer", f"{layer or 0} / {total_layer or 0}")) if raw.get("wifi_signal"): - rows.append(("Wi-Fi", str(raw.get("wifi_signal")))) + rows.append(("Wi-Fi", display_safe(raw.get("wifi_signal")))) if raw.get("gcode_file"): - rows.append(("File", str(raw.get("gcode_file")))) + rows.append(("File", display_safe(raw.get("gcode_file")))) return rows @@ -160,7 +162,7 @@ def ams_tray_rows(snapshot: StatusSnapshot) -> list[dict[str, Any]]: "unit": unit_id, "slot": slot, "label": f"AMS{unit_id}·{slot}", - "type": ftype, + "type": display_safe(ftype), "color": tray.get("color"), "remain": remain_label, "active": bool(tray.get("active")), @@ -330,17 +332,17 @@ def is_terminal(self, snapshot: StatusSnapshot) -> bool: def job_progress_lines(snapshot: StatusSnapshot) -> list[tuple[str, str]]: """Return ``(label, value)`` rows for the job-progress widget (pure).""" if not snapshot.ok: - return [("Status", snapshot.error or "Printer unreachable.")] + return [("Status", display_safe(snapshot.error) or "Printer unreachable.")] raw = snapshot.raw rows: list[tuple[str, str]] = [ - ("State", str(raw.get("gcode_state", "UNKNOWN"))), + ("State", display_safe(raw.get("gcode_state", "UNKNOWN"))), ("Progress", f"{raw.get('mc_percent', 0)}%"), ("Layer", f"{raw.get('layer_num') or 0} / {raw.get('total_layer_num') or 0}"), ("Remaining", format_remaining(raw.get("mc_remaining_time"))), ] if raw.get("gcode_file"): - rows.append(("File", str(raw.get("gcode_file")))) + rows.append(("File", display_safe(raw.get("gcode_file")))) return rows @@ -348,7 +350,7 @@ def format_remaining(minutes: Any) -> str: """Render ``mc_remaining_time`` (minutes) as ``1h 05m`` / ``42m`` / ``—``.""" try: total = int(minutes) - except (TypeError, ValueError): + except (TypeError, ValueError, OverflowError): return "—" if total < 0: return "—" @@ -364,6 +366,6 @@ def progress_percent(snapshot: StatusSnapshot) -> int: return 0 try: value = int(snapshot.raw.get("mc_percent", 0)) - except (TypeError, ValueError): + except (TypeError, ValueError, OverflowError): return 0 return max(0, min(100, value)) diff --git a/bambu_cli/utils.py b/bambu_cli/utils.py index 89226f7..ca8903c 100644 --- a/bambu_cli/utils.py +++ b/bambu_cli/utils.py @@ -1,4 +1,5 @@ import json +import math import os from typing import NoReturn @@ -96,11 +97,13 @@ def _display_path(path): def _compact_all_strings(val): if isinstance(val, dict): return {k: _compact_all_strings(v) for k, v in val.items()} - if isinstance(val, list): + if isinstance(val, (list, tuple)): return [_compact_all_strings(v) for v in val] if isinstance(val, str): redacted = _redact_url_credentials(val) return redacted if redacted != val else _display_path(val) + if isinstance(val, float) and not math.isfinite(val): + return None return val @@ -117,10 +120,13 @@ def _json_display_paths(value): else: result[key] = _json_display_paths(item) return result - if isinstance(value, list): + if isinstance(value, (list, tuple)): return [_json_display_paths(item) for item in value] if isinstance(value, str): return _redact_url_credentials(value) + if isinstance(value, float) and not math.isfinite(value): + # NaN/Infinity are not valid JSON; a spoofed report must not emit them. + return None return value @@ -140,8 +146,11 @@ def _as_payload(data): def emit_json(data): global _JSON_EMITTED + # Serialize before marking the envelope emitted: if dumping ever raised, the + # caller must still be able to print an error envelope. + text = json.dumps(_json_display_paths(_as_payload(data)), indent=2, allow_nan=False) _JSON_EMITTED = True - print(json.dumps(_json_display_paths(_as_payload(data)), indent=2)) + print(text) def emit_json_line(data): @@ -152,8 +161,9 @@ def emit_json_line(data): document at the end. """ global _JSON_EMITTED + text = json.dumps(_json_display_paths(_as_payload(data)), separators=(",", ":"), allow_nan=False) _JSON_EMITTED = True - print(json.dumps(_json_display_paths(_as_payload(data)), separators=(",", ":")), flush=True) + print(text, flush=True) def _namespace_get(args, key, default=None): diff --git a/docs/MASTER-PLAN.md b/docs/MASTER-PLAN.md new file mode 100644 index 0000000..cca3157 --- /dev/null +++ b/docs/MASTER-PLAN.md @@ -0,0 +1,35 @@ +# platecli — the plan + +The work we chose, in order. **No task from the user? Take the top 🤖 box under *Next* that isn't claimed or blocked.** Everything else noticed goes in `docs/plan/inbox.md`. Rules: the `master-plan` skill. Limits: 300 lines; a box is at most 5 lines. + +`docs/plans/` holds finished design records (TUI, guided mode, the A+ and post-audit gameplans); `docs/quality-roadmap.md` is the frozen quality scoreboard. Neither is a to-do list. + +## Goals + +`plate` lets a person or an agent go from a model file or Printables link to a print on a Bambu Lab printer over the LAN, safely: nothing moves or heats without `--confirm`, every command speaks `--json`, and no secret reaches a log. Engineering is at an honest A−; the 2026-09-01 audit froze quality work until there are outside users, so launch steps come first. + +## Waiting on you 🧑 + +- [ ] **D-1 · Merge the audit-round4 PR (#153)** · P1 · 🧑 + Branch `fix/audit-round4`: at-most-once MQTT delivery, pin precedence, heater limits for raw and pre-sliced G-code, AMS re-read before a guided print, confirmed mDNS results, terminal-escape hardening, config value checks. Needs the 6 CI checks green. +- [ ] **D-2 · Dependabot PRs #150 (actions) and #152 (python deps)** · P2 · 🧑 + Open since 2026-10-26 and 2026-11-02. *Done when:* merged or closed. +- [ ] **D-3 · Keep `"bambu"` in the PyPI keywords?** · P2 · 🧑 + Unruled since 2026-07-25; recommendation was keep (nominative search term, not a name). +- [ ] **L-1 · Post to r/BambuLab** · P1 · both · after L-2 + 🤖 rewrites the held copy to lead with `plate tui`/`plate go`; 🧑 posts (Tue–Thu, 10–11 EDT). +- [ ] **L-2 · Cut 0.6.0** · P1 · both · after D-1 + `[Unreleased]` holds the A1 slicing fix and two audit waves. 🤖 prepares the release PR per `docs/releasing.md`; 🧑 approves the tag push (it publishes to PyPI). + +## Next + +- [ ] **L-3 · Check that 0.5.1 installs from PyPI** · P2 · 🤖 + Never verified after the 2026-08-28 publish. Clean venv, `pip install platecli==0.5.1`, `plate --version`, `plate --sim status`. + *Done when:* the result is recorded in `docs/plan/done.md`. + +## Later + +- [ ] **L-4 · Re-record the hero GIF with `plate job`** · P2 · both + The README hero shows `status`/`slice`, not the command the pitch rests on. Recording starts a real print: 🧑 says yes and is at the printer. + +**Highest ids given:** D-3 · L-4 · S-3 · retired: S-1, S-2, S-3 diff --git a/docs/plan/done.md b/docs/plan/done.md new file mode 100644 index 0000000..10f19d5 --- /dev/null +++ b/docs/plan/done.md @@ -0,0 +1,7 @@ +# Done + +One line per finished plan box, newest first: `- date · id · what happened · commit`. + +- 2026-11-05 · S-3 · setup shows a lone mDNS result and asks (no → manual entry); unspecified/multicast addresses are skipped, public/loopback ones need an explicit yes, also when picked from several · this commit +- 2026-11-05 · S-2 · real (the slot was read once, at material choice): `plate go` and the TUI re-read the AMS on Start print and refuse with nothing sent if it changed or can't be read; confirm summary gains Feed and Temperatures rows; pilot + wizard tests · 8cae64c +- 2026-11-05 · S-1 · `upload`/`job`/`send` scan every `.gcode` member of a 3MF (and plain `.gcode`) for M104/M109/M140/M190 over 350/150 °C before upload; `job --dry-run` reports it too; unreadable members fail closed; 0.5 s for 2M lines · ef01a1d diff --git a/docs/plan/inbox.md b/docs/plan/inbox.md new file mode 100644 index 0000000..28d3361 --- /dev/null +++ b/docs/plan/inbox.md @@ -0,0 +1,23 @@ +# Inbox + +Noticed but not chosen, one line each, oldest first: `- date · where · what · (source)`. Triage promotes, merges or closes lines (see the `master-plan` skill). Nobody needs to read this to start work. + +- 2026-07-25 · bambu_cli/config.py · flatpak OrcaSlicer profile-dir paths are inferred from the app-tree layout, never checked against a real Flathub install · (launch notes) +- 2026-07-26 · tests/test_docs_consistency.py · measured coverage % has no drift guard; only the test count and the CI floor are pinned · (launch notes) +- 2026-07-26 · download · `plate download ` end to end was last exercised before the UA/throttle rewrite; may be stale, re-check · (launch notes) +- 2026-11-05 · bambu_cli/slicer · no bounds on speed/acceleration overrides (OrcaSlicer rejects bad values itself) · (audit round 4) +- 2026-11-05 · bambu_cli/job · no upper cap on `--copies` · (audit round 4) +- 2026-11-05 · bambu_cli/protocols/camera.py · each socket read has a 12 s timeout but the frame loop has no overall deadline · (audit round 4) +- 2026-11-05 · bambu_cli/download/extract.py · a ZIP's whole central directory is held in memory (zipfile reads it before any size check can run) · (audit round 4) +- 2026-11-05 · bambu_cli/jsonio.py · userinfo inside prose (not a URL on its own) is not redacted · (audit round 4) +- 2026-11-05 · bambu_cli/commands/snapshot.py · the Docker streamer's urlopen honours HTTP(S)_PROXY from the environment · (audit round 4) +- 2026-11-05 · bambu_cli/setup_cmd/config_cmd.py · human-mode `config show` can print camera_stream_url with credentials · (audit round 4) +- 2026-11-05 · bambu_cli/commands/files.py · `cmd_files` builds its printer with `get_printer()` instead of `ctx.printer()` like the other commands · (audit round 4) +- 2026-11-05 · bambu_cli/tlspin.py · a blank `cert_fingerprint` means "no pin" at connect time; `config validate`/`preflight` flag it, but commands that skip those checks run unpinned without a warning · (audit round 4) +- 2026-11-05 · bambu_cli/commands/gcode.py · `M141`/`M191` (chamber) and Marlin's `M104 B` (autotemp max) are not held to a limit; there is no chamber constant to check against · (review of the round-4 fixes) +- 2026-11-05 · bambu_cli/commands/gcode.py · `M104 S220 ; Set nozzle` is refused ("Unreadable nozzle temperature 'Set'") because comments are scanned; safe direction, but a false refusal · (review of the round-4 fixes) +- 2026-11-05 · bambu_cli/jsonio.py · a password with two or more delimiters after a digit prefix (`user:12/pa/ss@host`) still parses as a path and is not redacted · (review of the round-4 fixes) +- 2026-11-05 · bambu_cli/commands/files.py · `"cert_fingerprint": 5` makes `files` fail as `failed_step: ftps`, exit 2, where `status` gives the config error, exit 1 · (review of the round-4 fixes) +- 2026-11-05 · bambu_cli/commands/doctor.py · the capabilities file is written with `json.dump` defaults, so a NaN in a report would make it invalid JSON (unverified) · (review of the round-4 fixes) +- 2026-11-05 · bambu_cli/gcode_limits.py · the pre-sliced scan has no cap on total decompressed size: a highly compressible 3MF can take minutes to scan (memory stays bounded) · (review of the S-1 fix) +- 2026-11-05 · bambu_cli/gcode_limits.py · whether Bambu firmware ends a G-code line at a bare CR is unconfirmed; the scanner treats CR as a break, the safe reading · (review of the S-1 fix) diff --git a/docs/quality-roadmap.md b/docs/quality-roadmap.md index 70e5b6e..084db87 100644 --- a/docs/quality-roadmap.md +++ b/docs/quality-roadmap.md @@ -60,7 +60,7 @@ 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+** | **1847** non-live tests passing (2026-09-22, local Linux; 1 live deselected). A column cleared. Measured **92.8%** branch coverage over 8396 statements; A+ threshold met | +| 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 | | 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 | diff --git a/docs/schemas/job_error.json b/docs/schemas/job_error.json index e1de0d2..ba74902 100644 --- a/docs/schemas/job_error.json +++ b/docs/schemas/job_error.json @@ -78,7 +78,10 @@ ] }, "printed": { - "type": "boolean" + "type": [ + "boolean", + "null" + ] }, "uploaded": { "type": "boolean" diff --git a/docs/test-backlog.md b/docs/test-backlog.md index 42e0c86..bc89a6d 100644 --- a/docs/test-backlog.md +++ b/docs/test-backlog.md @@ -10,7 +10,7 @@ Do not treat historical “≥98% coverage” claims as current — see the snap | Metric | Current (honest) | A+ / 1.0 target | |--------|------------------|-----------------| -| Non-live tests collected | **1847** passing (measured 2026-09-22 on Linux; 1 live test deselected) | ≥550 with zero known flakes ✅ size | +| 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 | | Typing | Full package mypy + `check_untyped_defs` | keep; optional full `strict` later | | Error model | `sys.exit` only in `cli.py` | keep | diff --git a/scripts/check_layers.py b/scripts/check_layers.py index c069f05..fd993be 100644 --- a/scripts/check_layers.py +++ b/scripts/check_layers.py @@ -42,6 +42,8 @@ "jsonio": 10, "tlspin": 10, "fsutil": 10, + "textsafe": 10, + "gcode_limits": 10, # Typed --json payload shapes. Pure data: stdlib only, imports nothing from # the package, and generates docs/schemas/. Any layer may build one. "contracts": 10, diff --git a/tests/contracts/test_schema_validation.py b/tests/contracts/test_schema_validation.py index d705e23..e840f18 100644 --- a/tests/contracts/test_schema_validation.py +++ b/tests/contracts/test_schema_validation.py @@ -15,6 +15,7 @@ from bambu_cli.cli import main # noqa: E402 from bambu_cli import utils # noqa: E402 from bambu_cli.constants import VERSION # noqa: E402 +from tests.fake_3mf import minimal_3mf pytestmark = pytest.mark.contract @@ -355,7 +356,7 @@ def test_upload_matches_schema(monkeypatch, tmp_path, capsys): config_path = tmp_path / "config" / "cfg.json" _write_valid_config(config_path) model = tmp_path / "probe.gcode.3mf" - model.write_bytes(b"probe payload") + model.write_bytes(minimal_3mf()) monkeypatch.setattr(sys, "argv", ["plate", "--sim", "upload", str(model), "--json"]) monkeypatch.setattr("bambu_cli.config.CONFIG_PATH", str(config_path)) monkeypatch.setattr("bambu_cli.cli.setup_logging", lambda *a, **k: None) diff --git a/tests/fake_3mf.py b/tests/fake_3mf.py new file mode 100644 index 0000000..2be6dac --- /dev/null +++ b/tests/fake_3mf.py @@ -0,0 +1,23 @@ +"""A small, valid 3MF for tests that only need a file upload accepts. + +``upload`` opens every 3MF to check its heater targets and refuses one it +cannot open, so placeholder bytes such as ``b"PK\\x03\\x04 sliced"`` no longer +stand in for a sliced file. +""" + +from __future__ import annotations + +import io +import zipfile + +_STAMP = (2026, 1, 1, 0, 0, 0) + + +def minimal_3mf(pad: int = 0) -> bytes: + """A valid 3MF with no G-code; ``pad`` adds that many stored bytes.""" + buf = io.BytesIO() + with zipfile.ZipFile(buf, "w") as zf: + zf.writestr(zipfile.ZipInfo("[Content_Types].xml", date_time=_STAMP), "") + if pad: + zf.writestr(zipfile.ZipInfo("Metadata/pad.bin", date_time=_STAMP), b"x" * pad) + return buf.getvalue() diff --git a/tests/privacy_smoke.py b/tests/privacy_smoke.py index 19ba337..e679592 100644 --- a/tests/privacy_smoke.py +++ b/tests/privacy_smoke.py @@ -167,7 +167,10 @@ def iter_files(include_dist=False): continue rel = raw_rel.decode("utf-8", errors="replace") path = ROOT / rel - if any(part in excluded_dirs for part in path.parts): + # Match on the repo-relative path: in a worktree under + # .claude/worktrees/ every absolute path contains ".claude", and + # matching on it skipped every file, so the smoke passed scanning nothing. + if any(part in excluded_dirs for part in Path(rel).parts): continue if path.is_symlink() or not path.is_file(): continue diff --git a/tests/test_camera_capture.py b/tests/test_camera_capture.py index c890435..ad7ac6b 100644 --- a/tests/test_camera_capture.py +++ b/tests/test_camera_capture.py @@ -81,6 +81,23 @@ def test_grab_camera_frame_direct_insecure(self): # (not the bare socket) must be closed or the fd leaks. mock_tls.close.assert_called_once() + def test_pin_is_enforced_even_with_insecure_tls(self): + """insecure_tls used to skip a configured camera pin entirely.""" + from bambu_cli.protocols.camera import _CameraPinMismatch, _grab_camera_frame_direct + + create_connection, ssl_factory, mock_sock, mock_tls, mock_ctx = self._mock_net() + mock_tls.getpeercert.return_value = b"attacker_cert" + printer = _test_printer( + ip="192.168.1.100", + access_code="my_secret_code", + insecure_tls=True, + cert_fingerprint=hashlib.sha256(b"real_cert").hexdigest(), + ) + + with self.assertRaises(_CameraPinMismatch): + _grab_camera_frame_direct(printer, create_connection=create_connection, ssl_context_factory=ssl_factory) + mock_tls.sendall.assert_not_called() + def test_grab_camera_frame_direct_with_pin(self): from bambu_cli.protocols.camera import _grab_camera_frame_direct diff --git a/tests/test_cli_entry.py b/tests/test_cli_entry.py index 6be0fca..6fc30b1 100644 --- a/tests/test_cli_entry.py +++ b/tests/test_cli_entry.py @@ -189,7 +189,7 @@ def fake_browser(zc, type_, listener): from bambu_cli.commands import cmd_setup mock_getpass.return_value = "12345678" - self.input_mock_obj.side_effect = ["P1P", "", "n"] + self.input_mock_obj.side_effect = ["", "P1P", "", "n"] # use this printer? (default yes) first tmpdir = tempfile.mkdtemp() cfg_path = _os.path.join(tmpdir, "config.json") diff --git a/tests/test_cli_envelopes.py b/tests/test_cli_envelopes.py index 37db6cd..6a3e4b6 100644 --- a/tests/test_cli_envelopes.py +++ b/tests/test_cli_envelopes.py @@ -10,6 +10,7 @@ from contextlib import redirect_stdout, redirect_stderr from bambu_cli.errors import BambuError +from tests.fake_3mf import minimal_3mf # --------------------------------------------------------------------------- @@ -137,7 +138,7 @@ def test_dry_run_surfaces_ssl_pin_reason(self, mock_get_printer): tmpdir = tempfile.mkdtemp() fpath = os.path.join(tmpdir, "x.gcode.3mf") with open(fpath, "wb") as fh: - fh.write(b"PK\x03\x04dummy") + fh.write(minimal_3mf()) args = argparse.Namespace(file=fpath, dry_run=True, json=True, verbose=False) diff --git a/tests/test_cliparse_no_abbrev.py b/tests/test_cliparse_no_abbrev.py new file mode 100644 index 0000000..ac609ad --- /dev/null +++ b/tests/test_cliparse_no_abbrev.py @@ -0,0 +1,69 @@ +"""Option abbreviations are off, so the literal ``--confirm`` gate cannot be bypassed by ``--conf``.""" + +from __future__ import annotations + +import json +from unittest.mock import patch + +import pytest + +from bambu_cli import cli +from bambu_cli.cliparse import JsonArgumentParser, build_parser +from bambu_cli.constants import EXIT_COMMAND_ERROR + + +def _subparsers(parser): + import argparse + + for action in parser._actions: + if isinstance(action, argparse._SubParsersAction): + return action.choices + return {} + + +def test_every_parser_is_json_parser_without_abbrev(): + parser = build_parser() + assert isinstance(parser, JsonArgumentParser) and parser.allow_abbrev is False + subs = _subparsers(parser) + assert subs + for name, sub in subs.items(): + assert isinstance(sub, JsonArgumentParser), name + assert sub.allow_abbrev is False, name + for nested in _subparsers(sub).values(): + assert nested.allow_abbrev is False + + +@pytest.mark.parametrize( + "argv", + [ + ["stop", "--conf"], + ["print", "x.3mf", "--con"], + ["gcode", "G28", "--conf"], + ["delete", "x.3mf", "--conf"], + ], +) +@pytest.mark.parametrize("json_mode", [False, True]) +def test_abbreviated_confirm_is_a_usage_error(argv, json_mode, capsys): + argv = argv + (["--json"] if json_mode else []) + with ( + patch("sys.argv", ["plate", *argv]), + patch("bambu_cli.commands.cmd_stop") as stop, + patch("bambu_cli.commands.cmd_print") as pr, + patch("bambu_cli.commands.cmd_gcode") as gc, + patch("bambu_cli.commands.cmd_delete") as de, + pytest.raises(SystemExit) as exc, + ): + cli.main() + assert exc.value.code == EXIT_COMMAND_ERROR + for m in (stop, pr, gc, de): + m.assert_not_called() + out = capsys.readouterr() + if json_mode: + payload = json.loads(out.out) + assert payload["failed_step"] == "parse" + assert payload["exit_code"] == EXIT_COMMAND_ERROR + + +def test_full_confirm_still_parses(): + args = build_parser().parse_args(["stop", "--confirm"]) + assert args.confirm is True diff --git a/tests/test_cmd_upload.py b/tests/test_cmd_upload.py index fd8a10f..314aa7b 100644 --- a/tests/test_cmd_upload.py +++ b/tests/test_cmd_upload.py @@ -7,6 +7,15 @@ class TestBambuCmdUploadEdgeCases(unittest.TestCase): + def _cwd_with_test_gcode(self): + """Run in a temp dir holding a real test.gcode: upload reads it to check its temperatures.""" + workdir = tempfile.mkdtemp() + self.addCleanup(shutil.rmtree, workdir, True) + with open(os.path.join(workdir, "test.gcode"), "w", encoding="utf-8") as fh: + fh.write("G28\nM104 S220\nM140 S60\n") + self.addCleanup(os.chdir, os.getcwd()) + os.chdir(workdir) + @patch("bambu_cli.logging_utils._BACKEND") @patch("sys.exit") def test_cmd_upload_invalid_filepath(self, mock_exit, mock_logger): @@ -42,6 +51,8 @@ def test_cmd_upload_file_not_found(self, mock_exit, mock_logger, mock_exists): def test_cmd_upload_dry_run_success(self, mock_logger, mock_get_printer, mock_getsize, mock_exists): from bambu_cli.commands import cmd_upload + self._cwd_with_test_gcode() + mock_exists.return_value = True mock_getsize.return_value = 1024 args = MagicMock() @@ -70,6 +81,8 @@ def test_cmd_upload_dry_run_success(self, mock_logger, mock_get_printer, mock_ge def test_cmd_upload_dry_run_fail(self, mock_exit, mock_logger, mock_get_printer, mock_getsize, mock_exists): from bambu_cli.commands import cmd_upload + self._cwd_with_test_gcode() + mock_exists.return_value = True mock_getsize.return_value = 1024 args = MagicMock() @@ -197,6 +210,11 @@ def test_cmd_upload_retry_success( mock_exists.return_value = True mock_getsize.return_value = 2048 + # open() is mocked and test.3mf is not on disk; the heater scan is + # covered by test_presliced_heater_limits.py, so pass it here. + heater_scan = patch("bambu_cli.commands.files.file_heater_problem", return_value=None) + heater_scan.start() + self.addCleanup(heater_scan.stop) mock_ftp = MagicMock() # Fail once, then succeed diff --git a/tests/test_config_round4_values.py b/tests/test_config_round4_values.py new file mode 100644 index 0000000..f4e996f --- /dev/null +++ b/tests/test_config_round4_values.py @@ -0,0 +1,275 @@ +"""Audit round 4: timeout ceiling, wrongly typed config values, unreadable config files, mDNS names.""" + +from __future__ import annotations + +import argparse +import io +from unittest.mock import patch + +import pytest + +from bambu_cli import config +from bambu_cli.cliparse import positive_seconds +from bambu_cli.constants import EXIT_CONFIG_ERROR, MAX_TIMEOUT_SECONDS +from bambu_cli.context import Settings +from bambu_cli.errors import BambuError +from bambu_cli.setup_cmd import collect_preflight_checks +from bambu_cli.tlspin import normalize_fingerprint +from tests.bambu_test_base import config_ctx + +GOOD_PIN = "aa" * 32 + + +# --- C1 ---------------------------------------------------------------------------------- + + +def test_max_timeout_constant(): + assert MAX_TIMEOUT_SECONDS == 86400 + + +@pytest.mark.parametrize("value", [1e10, 86401, "86401"]) +def test_config_timeout_over_limit_rejected(value): + problem = config.config_timeout_problem(value) + assert problem and "86400" in problem + + +def test_config_timeout_at_limit_ok(): + assert config.config_timeout_problem(86400) is None + + +@pytest.mark.parametrize("value", ["1e10", "86401"]) +def test_positive_seconds_over_limit_rejected(value): + with pytest.raises(argparse.ArgumentTypeError, match="86400"): + positive_seconds(value) + + +def test_positive_seconds_at_limit_ok(): + assert positive_seconds("86400") == 86400.0 + + +# --- C3 ---------------------------------------------------------------------------------- + + +def _preflight(cfg): + with ( + patch("bambu_cli.setup_cmd.preflight.load_config", return_value=cfg), + patch("bambu_cli.setup_cmd.preflight._config_path", return_value="/tmp/config.json"), + patch("bambu_cli.setup_cmd.preflight._display_path", side_effect=lambda p: p), + patch("bambu_cli.slicer.cmd._slicer_executable_problem", return_value=None), + patch("os.path.isdir", return_value=True), + patch("os.path.exists", return_value=True), + patch("shutil.which", return_value=None), + ): + return collect_preflight_checks() + + +def _check(checks, name): + return [c for c in checks if c["name"] == name] + + +BASE = {"printer_ip": "10.1.2.3", "serial": "S1", "access_code": "12345678"} + + +@pytest.mark.parametrize("value", ["yes", "true", "1", "on"]) +def test_preflight_insecure_tls_truthy_string_says_disabled(value): + assert Settings.from_config({"insecure_tls": value}).insecure_tls is True + (tls,) = _check(_preflight({**BASE, "insecure_tls": value}), "insecure-tls") + assert "DISABLED" in tls["message"] + assert "stays ENABLED" not in tls["message"] + + +def test_preflight_insecure_tls_unreadable_says_enabled(): + (tls,) = _check(_preflight({**BASE, "insecure_tls": 7}), "insecure-tls") + assert "stays ENABLED" in tls["message"] + + +# --- C4 ---------------------------------------------------------------------------------- + + +@pytest.mark.parametrize("value", [None, ["x"], 12345678, True]) +def test_non_string_inline_access_code_is_a_config_error(value): + with config_ctx({"access_code": value}), pytest.raises(BambuError) as exc: + config.load_access_code() + assert exc.value.exit_code == EXIT_CONFIG_ERROR + + +@pytest.mark.parametrize("value", [None, 5, ["a"]]) +def test_non_string_access_code_file_is_treated_as_unset(value): + with config_ctx({"access_code_file": value}), pytest.raises(BambuError) as exc: + config.load_access_code() + assert exc.value.exit_code == EXIT_CONFIG_ERROR + + +def test_null_access_code_file_falls_back_to_inline(): + with config_ctx({"access_code_file": None, "access_code": "REALCODE1"}): + assert config.load_access_code() == "REALCODE1" + + +@pytest.mark.parametrize("value", [5, True, ["aa"], {"a": 1}]) +def test_non_string_pin_is_a_config_error(value): + with pytest.raises(BambuError) as exc: + normalize_fingerprint(value) + assert exc.value.exit_code == EXIT_CONFIG_ERROR + + +def test_expected_fingerprint_non_string_is_a_config_error(): + with config_ctx({"cert_fingerprint": 5}), pytest.raises(BambuError) as exc: + config._expected_fingerprint() + assert exc.value.exit_code == EXIT_CONFIG_ERROR + + +def test_normalize_fingerprint_still_accepts_none_and_strings(): + assert normalize_fingerprint(None) is None + assert normalize_fingerprint("AA:BB") == "aabb" + + +@pytest.mark.parametrize("pin", [5, ["a"], "", " "]) +def test_validate_reports_bad_pin(pin): + checks = _preflight({**BASE, "cert_fingerprint": pin}) + (pin_check,) = _check(checks, "cert-fingerprint") + assert pin_check["status"] == "error" + + +def test_validate_good_pin_is_ok(): + (pin_check,) = _check(_preflight({**BASE, "cert_fingerprint": GOOD_PIN}), "cert-fingerprint") + assert pin_check["status"] == "ok" + + +def test_int_camera_port_is_accepted(): + settings = Settings.from_config({"camera_port": 1985}) + assert settings.camera_port.split(":")[-2] == "1985" + assert "1985" in settings.camera_stream_url + + +@pytest.mark.parametrize("value", [0, 70000, -1, True, ["x"], 1.5]) +def test_bad_camera_port_does_not_drop_the_config(value): + settings = Settings.from_config({"camera_port": value, "printer_ip": "10.0.0.5"}) + assert settings.printer_ip == "10.0.0.5" + (check,) = _check(_preflight({**BASE, "camera_port": value}), "camera-port") + assert check["status"] == "error" + + +def test_preflight_does_not_blame_json_for_int_port(): + checks = _preflight({**BASE, "camera_port": 1985}) + assert [c for c in checks if c["name"] == "camera-port"][0]["status"] == "ok" + + +@pytest.mark.parametrize("value", [["a"], 5, None, True]) +def test_non_string_printer_ip(value): + settings = Settings.from_config({"printer_ip": value}) + assert isinstance(settings.printer_ip, str) + (check,) = _check(_preflight({**BASE, "printer_ip": value}), "printer-ip") + assert check["status"] == "error" + + +def test_preflight_non_string_access_fields_do_not_crash(): + checks = _preflight({**BASE, "access_code": None, "access_code_file": ["x"]}) + assert _check(checks, "access-code")[0]["status"] == "error" + checks = _preflight({**BASE, "access_code": ["x"]}) + assert _check(checks, "access-code")[0]["status"] == "error" + + +# --- C5 ---------------------------------------------------------------------------------- + + +def _cfg_file(tmp_path, data: bytes): + path = tmp_path / "config.json" + path.write_bytes(data) + return str(path) + + +@pytest.mark.parametrize("data", [b"\xff\xfe", b"[]", b"3", b'"x"']) +def test_read_config_json_rejects_bad_files_as_valueerror(tmp_path, data): + with pytest.raises(ValueError): + config.read_config_json(_cfg_file(tmp_path, data)) + + +@pytest.mark.parametrize("data", [b"\xff\xfe", b"[]"]) +def test_config_show_reports_config_error(tmp_path, data): + from bambu_cli.setup_cmd.config_cmd import _cmd_config_show + + path = _cfg_file(tmp_path, data) + args = argparse.Namespace(cmd="config", action="show", json=False) + with ( + patch("bambu_cli.setup_cmd.config_cmd._config_path", return_value=path), + patch("bambu_cli.setup_cmd.config_cmd.logger"), + patch("sys.stdout", new_callable=io.StringIO) as out, + pytest.raises(BambuError) as exc, + ): + _cmd_config_show(args) + assert exc.value.exit_code == EXIT_CONFIG_ERROR + assert '"config"' not in out.getvalue() + + +@pytest.mark.parametrize("data", [b"\xff\xfe", b"[]"]) +def test_migrate_reports_config_error(tmp_path, data): + from bambu_cli.setup_cmd.migrate import _cmd_migrate_access_code + + path = _cfg_file(tmp_path, data) + args = argparse.Namespace(json=False, access_code_file=None) + with ( + patch("bambu_cli.setup_cmd.migrate._config_path", return_value=path), + patch("bambu_cli.setup_cmd.migrate.logger"), + pytest.raises(BambuError) as exc, + ): + _cmd_migrate_access_code(args) + assert exc.value.exit_code == EXIT_CONFIG_ERROR + + +# --- C6 ---------------------------------------------------------------------------------- + + +def test_discovery_log_lines_strip_control_chars(tmp_path, monkeypatch): + import builtins + import sys + from unittest.mock import MagicMock + + from bambu_cli.setup_cmd import common as common_mod + from bambu_cli.setup_cmd import wizard as wizard_mod + from tests.test_wizard_guided import create_mock_zeroconf, mock_service_browser + + cfg = tmp_path / "config.json" + answers = iter(["0", "87654321", "", "", ""]) + + def fake_prompt(msg, args=None, default=None): + return next(answers, default or "") + + monkeypatch.setattr(sys.stdin, "isatty", lambda: True) + monkeypatch.setattr(sys.stdout, "isatty", lambda: True) + args = argparse.Namespace( + json=False, migrate_access_code=False, scan_timeout=0.01, printer_ip=None, serial=None, access_code=None, + access_code_file=None, access_code_env=None, model=None, nozzle=None, orca_slicer=None, profiles_dir=None, + cert_fingerprint=None, insecure_tls=False, config=str(cfg), + ) + services = [ + ("BBLP-P1P-01P00A123._bblp\nERROR: spoofed\x1b[2J._tcp.local.", "10.0.0.50"), + ("BBLP-A1-03000A111._bblp._tcp.local.", "10.0.0.70"), + ] + log = MagicMock() + real_import = builtins.__import__ + + def guarded(name, *a, **k): + if name == "zeroconf": + zc = MagicMock() + zc.Zeroconf = create_mock_zeroconf(services) + zc.ServiceBrowser = mock_service_browser(services) + return zc + return real_import(name, *a, **k) + + with ( + patch.object(common_mod, "_config_path", return_value=str(cfg)), + patch.object(wizard_mod, "_config_path", return_value=str(cfg)), + patch.object(wizard_mod, "logger", log), + patch.object(wizard_mod, "_prompt_text", side_effect=fake_prompt), + patch.object(wizard_mod, "_prompt_secret", side_effect=fake_prompt), + patch.object(wizard_mod, "_prompt_access_code_file_path", return_value=None), + patch("bambu_cli.protocols.mqtt.probe_cert_fingerprint", return_value="aa:bb:cc"), + patch("builtins.__import__", side_effect=guarded), + ): + wizard_mod._cmd_setup(args) + + logged = [str(c.args[0]) for c in log.method_calls if c.args] + joined = "\n".join(logged) + assert "spoofed" in joined # the name is still shown, just defanged + assert "\x1b" not in joined + assert not any("\nERROR: spoofed" in line for line in logged) diff --git a/tests/test_doctor_and_safety.py b/tests/test_doctor_and_safety.py index 7cc5061..bc08141 100644 --- a/tests/test_doctor_and_safety.py +++ b/tests/test_doctor_and_safety.py @@ -1,5 +1,6 @@ from tests.bambu_test_base import * # noqa: F401,F403 from bambu_cli.errors import BambuError +from tests.fake_3mf import minimal_3mf class TestBambuDoctor(unittest.TestCase): @@ -273,7 +274,7 @@ def test_simulation_mode_upload(self, mock_commands_logger, mock_ftps_logger): # 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) + f.write(minimal_3mf(pad=1024)) try: cmd_upload(args) finally: diff --git a/tests/test_doctor_no_stray_capabilities_file.py b/tests/test_doctor_no_stray_capabilities_file.py new file mode 100644 index 0000000..a52b88e --- /dev/null +++ b/tests/test_doctor_no_stray_capabilities_file.py @@ -0,0 +1,20 @@ +"""A doctor run that fails early must not leave an empty capabilities file behind.""" + +import glob +import tempfile +from types import SimpleNamespace +from unittest.mock import patch + +import pytest + +from bambu_cli.errors import BambuError, ConfigError + + +def test_failed_doctor_leaves_no_temp_file(tmp_path, monkeypatch): + from bambu_cli.commands import cmd_doctor + + monkeypatch.setattr(tempfile, "tempdir", str(tmp_path)) + with patch("bambu_cli.commands.doctor.load_config", side_effect=ConfigError("nope")): + with pytest.raises(BambuError): + cmd_doctor(SimpleNamespace(json=False, output=None)) + assert glob.glob(str(tmp_path / "printer_capabilities_*")) == [] diff --git a/tests/test_download_url_edge_cases.py b/tests/test_download_url_edge_cases.py new file mode 100644 index 0000000..0cd9246 --- /dev/null +++ b/tests/test_download_url_edge_cases.py @@ -0,0 +1,200 @@ +"""Download URL edge cases found in the 2026-11 audit. + +Each test failed before its fix: + +* a password holding "/", "?" or "#" was read as the port, so redaction found + no userinfo and the secret was echoed in the JSON error envelope; +* an unbalanced IPv6 bracket raised ValueError out of urlparse and surfaced as + "Unexpected error" with a traceback; +* a model URL served as text/plain (GitHub raw serves everything that way) was + refused although ASCII STL, OBJ, STEP and G-code are text; +* a URL with raw Unicode in the path failed with an opaque ASCII codec error; +* Ctrl-C mid-download left the .part file and the 0-byte placeholder behind. + +Literal ``user:pass@host`` strings are assembled from pieces because +tests/privacy_smoke.py rejects them in the repo. +""" + +from __future__ import annotations + +import json +import types +from unittest.mock import MagicMock + +import pytest + +from bambu_cli.constants import EXIT_COMMAND_ERROR, EXIT_FILE_ERROR +from bambu_cli.download import validation as V +from bambu_cli.errors import BambuError +from bambu_cli.jsonio import redact_url_credentials + +AT = "@" +SECRET = "S3CRETPW" + + +@pytest.mark.parametrize("sep", ["/", "?", "#"]) +def test_password_with_a_delimiter_is_redacted(sep): + url = f"https://user:{SECRET}{sep}rest{AT}host.example/x.stl" + redacted = redact_url_credentials(url) + assert SECRET not in redacted + assert "rest" not in redacted + assert redacted == "https://host.example/x.stl" + + +@pytest.mark.parametrize("sep", ["/", "?", "#"]) +def test_password_with_a_delimiter_is_refused_without_echoing_it(sep): + url = f"https://user:{SECRET}{sep}rest{AT}host.example/x.stl" + args = types.SimpleNamespace(json=True) + with pytest.raises(BambuError) as excinfo: + V._validate_download_url_or_exit(args, url, url, url, "validate", "Invalid URL") + assert excinfo.value.exit_code == EXIT_COMMAND_ERROR + emitted = json.dumps(excinfo.value.to_error_payload("download")) + assert SECRET not in emitted + assert "embedded credentials are not supported" in emitted + + +def test_an_at_sign_in_an_ordinary_path_is_left_alone(): + url = "https://host.example/@maker/model.stl" + assert redact_url_credentials(url) == url + + +@pytest.mark.parametrize( + "url", + ["http://[::1/x.stl", f"https://user:pw{AT}[x/a.stl"], +) +def test_unbalanced_ipv6_bracket_is_an_invalid_url_not_a_crash(url): + assert V._looks_like_url(url) is True + assert V._is_http_url(url) is False + with pytest.raises(BambuError) as excinfo: + V._validate_http_url_or_exit(url) + assert excinfo.value.exit_code == EXIT_COMMAND_ERROR + assert "pw" not in redact_url_credentials(url) + + +@pytest.mark.parametrize( + "url", + [ + "https://raw.githubusercontent.com/u/r/main/model.stl", + "https://host.example/files/part.OBJ", + "https://host.example/a/print.gcode?token=1", + "https://host.example/a/b%20c.step", + ], +) +def test_text_plain_is_accepted_when_the_url_names_a_model_file(url): + V._reject_unsupported_content_type(types.SimpleNamespace(json=False), url, None, url, "text/plain; charset=utf-8") + + +@pytest.mark.parametrize("url", ["https://host.example/download?id=4", "https://host.example/readme.txt"]) +def test_text_plain_is_still_refused_without_a_model_extension(url): + with pytest.raises(BambuError) as excinfo: + V._reject_unsupported_content_type(types.SimpleNamespace(json=False), url, None, url, "text/plain") + assert excinfo.value.exit_code == EXIT_FILE_ERROR + + +def test_images_are_still_refused_even_with_a_model_extension(): + url = "https://host.example/model.stl" + with pytest.raises(BambuError): + V._reject_unsupported_content_type(types.SimpleNamespace(json=False), url, None, url, "image/png") + + +def test_non_ascii_path_and_query_are_percent_encoded(): + assert V._iri_to_uri("http://h.example/files/модель.stl?n=ф#x") == ( + "http://h.example/files/%D0%BC%D0%BE%D0%B4%D0%B5%D0%BB%D1%8C.stl?n=%D1%84#x" + ) + + +def test_ascii_urls_and_existing_escapes_are_untouched(): + for url in ("http://h.example/a%20b.stl?x=1&y=2#f", "https://h.example/~u/[1].stl"): + assert V._iri_to_uri(url) == url + + +def test_ctrl_c_mid_download_leaves_nothing_behind(tmp_path): + from bambu_cli import download + + url = "https://example.com/model.stl" + reads = {"n": 0} + + def read(n): + reads["n"] += 1 + if reads["n"] > 2: + raise KeyboardInterrupt + return b"x" * n + + mock_resp = MagicMock() + mock_resp.read.side_effect = read + mock_resp.getheader.return_value = None + mock_resp.geturl.return_value = url + mock_opener = MagicMock() + mock_opener.open.return_value.__enter__.return_value = mock_resp + args = types.SimpleNamespace( + url=url, output=str(tmp_path), name=None, max_download_mb=10, json=False, progress=False + ) + + with pytest.raises(KeyboardInterrupt): + download._cmd_download(args, opener_factory=lambda: mock_opener) + + assert reads["n"] > 2, "the interrupt never fired mid-transfer" + assert list(tmp_path.iterdir()) == [], "Ctrl-C left a partial or placeholder file" + + +# A password that starts with digits and holds "/", "?" or "#" parses cleanly as +# host:port, so the port check above does not see it. +_SPLIT_USERINFO = [ + "https://bob:1234/" + SECRET + AT + "files.invalid/x.stl", + "https://bob:123?" + SECRET + AT + "files.invalid/x.stl", + "https://bob:80#" + SECRET + AT + "files.invalid/x.stl", + "https://bob:/" + SECRET + AT + "files.invalid/x.stl", +] + + +@pytest.mark.parametrize("url", _SPLIT_USERINFO) +def test_digit_prefixed_password_with_a_delimiter_is_redacted(url): + assert SECRET not in redact_url_credentials(url) + + +@pytest.mark.parametrize("url", _SPLIT_USERINFO) +def test_digit_prefixed_password_with_a_delimiter_is_refused(url): + with pytest.raises(BambuError) as excinfo: + V._validate_http_url_or_exit(url) + assert excinfo.value.exit_code == EXIT_COMMAND_ERROR + assert SECRET not in str(excinfo.value) + + +@pytest.mark.parametrize( + "url", + [ + "https:///bob:" + SECRET + AT + "files.invalid/x.stl", + "https:/bob:" + SECRET + AT + "h/x", + "https:\\\\bob:" + SECRET + AT + "h/x", + ], +) +def test_userinfo_without_a_parsed_host_is_redacted(url): + assert SECRET not in redact_url_credentials(url) + + +@pytest.mark.parametrize( + "text", + [ + "https://host:8443/" + AT + "scope/pkg.stl", + "https://medium.com/" + AT + "user/model.stl", + "https://host:8080/download?user=a" + AT + "b.com", + "C:\\Users\\bob" + AT + "corp\\model.stl", + "0.20mm Standard " + AT + "BBL X1C", + "/home/u/a" + AT + "b.stl", + ], +) +def test_ordinary_text_with_an_at_sign_is_left_alone(text): + assert redact_url_credentials(text) == text + + +@pytest.mark.parametrize( + "url", ["https://host:8443/" + AT + "scope/pkg.stl", "https://host:8080/download?user=a" + AT + "b.com"] +) +def test_urls_with_a_port_and_an_at_sign_in_the_path_still_validate(url): + V._validate_http_url_or_exit(url) + + +def test_a_non_numeric_port_without_credentials_is_still_named(): + with pytest.raises(BambuError) as excinfo: + V._validate_http_url_or_exit("https://host.example:abc/x.stl") + assert "port is not a number" in str(excinfo.value) diff --git a/tests/test_gcode_heater_limits.py b/tests/test_gcode_heater_limits.py new file mode 100644 index 0000000..9fd16dd --- /dev/null +++ b/tests/test_gcode_heater_limits.py @@ -0,0 +1,77 @@ +"""Raw G-code obeys the same heater ceilings as slice overrides. + +Before the 2026-11 audit, ``plate gcode "M104 S2200" --confirm`` went straight +to the printer: the only checks were non-empty and no CR/LF/NUL, while every +slice-time temperature was held to 350 °C nozzle / 150 °C bed. +""" + +from __future__ import annotations + +import argparse + +import pytest + +from bambu_cli.commands.gcode import _heater_problem, cmd_gcode +from bambu_cli.constants import EXIT_COMMAND_ERROR +from bambu_cli.errors import BambuError + + +class _RecordingPrinter: + def __init__(self): + self.sent = [] + + def send_command(self, payload): + self.sent.append(payload) + return True + + +class _Ctx: + def __init__(self): + self.printer_obj = _RecordingPrinter() + + def printer(self): + return self.printer_obj + + +@pytest.mark.parametrize( + "gcode", + [ + "M104 S2200", + "m140 s500", + "M104S9999", + "M109 R400", + "M190 S151", + "M140 S150.5", + "M0104 S999", + "M104.0 S999", + "M104 S200 ; S999", # comments are scanned too: conservative by design + "M104 S2e3", # exponent: strtod-style parsers read 2000 + "M104 S0x3E8", # hex: strtod-style parsers read 1000 + "M104 S250T0", + "M104 S-5", + "M104 S٩٩٩", # Arabic-Indic digits + "M104 T0 S3000", # a tool number before the temperature + "M109 T1 R9999", + "M140 T0 S200", + "M190 T0 S900", + "M104 S200 T1 S9999", + ], +) +def test_out_of_range_or_unreadable_heater_values_are_refused_before_sending(gcode): + assert _heater_problem(gcode) is not None + ctx = _Ctx() + with pytest.raises(BambuError) as excinfo: + cmd_gcode(argparse.Namespace(code=gcode, confirm=True, json=False), ctx=ctx) + assert excinfo.value.exit_code == EXIT_COMMAND_ERROR + assert ctx.printer_obj.sent == [] + + +@pytest.mark.parametrize( + "gcode", + ["M104 S220", "M104 S350", "M190 S60", "M140 S150", "M109 S0", "G28", "M106 S255", "M104 S200 M106 S255", "M141 S40", "M104 T0 S220", "T1"], +) +def test_ordinary_g_code_still_passes(gcode): + assert _heater_problem(gcode) is None + ctx = _Ctx() + cmd_gcode(argparse.Namespace(code=gcode, confirm=True, json=False), ctx=ctx) + assert len(ctx.printer_obj.sent) == 1 diff --git a/tests/test_interactive_core.py b/tests/test_interactive_core.py index a9f5a9f..727da6b 100644 --- a/tests/test_interactive_core.py +++ b/tests/test_interactive_core.py @@ -455,7 +455,8 @@ def test_preview_rows_gain_an_overrides_line_only_when_set(tmp_path): _install_ready_settings(tmp_path) sliced = _sliced_3mf(tmp_path) plain = core.WizardState(printable_path=sliced, sliced=True) - assert [label for label, _ in core.preview_rows(plain, sliced)] == ["Model", "Printer", "Material", "Estimate"] + labels = [label for label, _ in core.preview_rows(plain, sliced)] + assert labels == ["Model", "Printer", "Material", "Feed", "Temperatures", "Estimate"] tuned = core.WizardState(printable_path=sliced, sliced=True) tuned.overrides = core.SliceOverrides(fields={"walls": 5}) @@ -466,3 +467,89 @@ def test_preview_rows_gain_an_overrides_line_only_when_set(tmp_path): presliced = core.WizardState(printable_path=sliced, sliced=False) presliced.overrides = core.SliceOverrides(fields={"walls": 5}) assert "Overrides" not in dict(core.preview_rows(presliced, sliced)) + + +# --------------------------------------------------------------------------- +# Feed and Temperatures rows; the AMS re-read before printing (2026-11-05 audit, S-2) +# --------------------------------------------------------------------------- + + +def _with_plate_gcode(path, gcode): + with zipfile.ZipFile(path, "a") as zf: + zf.writestr("Metadata/plate_1.gcode", gcode) + return path + + +def test_feed_row_names_the_ams_slot_only_when_it_feeds(tmp_path): + _install_ready_settings(tmp_path) + sliced = _sliced_3mf(tmp_path) + fed = core.WizardState( + material="PETG", detected_ams_material="PETG", detected_ams_slot=6, printable_path=sliced, sliced=True + ) + assert dict(core.preview_rows(fed, sliced))["Feed"] == "AMS1·2 (PETG)" + changed = core.WizardState( + material="ABS", detected_ams_material="PETG", detected_ams_slot=6, printable_path=sliced, sliced=True + ) + assert dict(core.preview_rows(changed, sliced))["Feed"] == "External spool" + no_slot = core.WizardState(material="PETG", detected_ams_material="PETG", printable_path=sliced, sliced=True) + assert dict(core.preview_rows(no_slot, sliced))["Feed"] == "External spool" + + +def test_temperatures_row_shows_the_hottest_targets_in_the_gcode(tmp_path): + _install_ready_settings(tmp_path) + sliced = _with_plate_gcode( + _sliced_3mf(tmp_path), + "M140 S55 ; bed\nM104 S215\nM190 S60\nM109 S220 ; wait\nM104 S0\n; M104 S999 in a comment\n", + ) + state = core.WizardState(printable_path=sliced, sliced=True) + assert dict(core.preview_rows(state, sliced))["Temperatures"] == "nozzle up to 220 °C · bed up to 60 °C" + + +def test_temperatures_row_for_a_plain_gcode_file_and_for_none(tmp_path): + _install_ready_settings(tmp_path) + gcode = tmp_path / "part.gcode" + gcode.write_text("G28\nM104 T1 S230\n", encoding="utf-8") + state = core.WizardState(printable_path=str(gcode), sliced=False) + assert dict(core.preview_rows(state, str(gcode)))["Temperatures"] == "nozzle up to 230 °C" + sliced = _sliced_3mf(tmp_path) + state = core.WizardState(printable_path=sliced, sliced=True) + assert dict(core.preview_rows(state, sliced))["Temperatures"] == "not set in the G-code" + + +def _reader(material, slot): + def detector(args, on_active_slot=None): + if slot is not None: + on_active_slot(slot) + return material + + return detector + + +def _fed_state(): + return core.WizardState(material="PETG", detected_ams_material="PETG", detected_ams_slot=2) + + +def test_ams_change_problem_is_none_when_nothing_changed(): + assert core.ams_change_problem(_reader("PETG", 2), argparse.Namespace(), _fed_state()) is None + + +def test_ams_change_problem_does_not_read_when_not_feeding_from_the_ams(): + def must_not_run(*_a): + raise AssertionError("read the AMS for an external-spool print") + + state = core.WizardState(material="ABS", detected_ams_material="PETG", detected_ams_slot=2) + assert core.ams_change_problem(must_not_run, argparse.Namespace(), state) is None + + +@pytest.mark.parametrize( + ("material", "slot", "expected"), + [ + ("PLA", 2, "it had PETG in AMS0·2, now PLA in AMS0·2"), + ("PETG", 3, "now PETG in AMS0·3"), + (None, None, "Could not confirm that PETG in AMS0·2"), + ], +) +def test_ams_change_problem_names_what_changed(material, slot, expected): + problem = core.ams_change_problem(_reader(material, slot), argparse.Namespace(), _fed_state()) + assert problem is not None and expected in problem + assert "not started" in problem diff --git a/tests/test_interactive_markup_and_keep.py b/tests/test_interactive_markup_and_keep.py new file mode 100644 index 0000000..ad503e5 --- /dev/null +++ b/tests/test_interactive_markup_and_keep.py @@ -0,0 +1,66 @@ +"""plate go: untrusted text is printed literally, and a failed job keeps the sliced file.""" + +from __future__ import annotations + +import io +import os + +import pytest +from rich.console import Console + +from bambu_cli.errors import BambuError +from bambu_cli.interactive import core +from bambu_cli.interactive.core import WizardState +from bambu_cli.interactive.prompts import Prompts +from bambu_cli.interactive.session import GoDeps, GoSteps, cmd_go +from tests.test_interactive_session import ( # noqa: F401 + Recorder, + ScriptedPrompts, + _args, + _install_ready_settings, + _make_stl, + _reset_context, + _sliced_3mf, + _tty, +) + + +def _printed(text: str) -> str: + buf = io.StringIO() + Prompts(console=Console(file=buf, force_terminal=False, width=200)).print(text) + return buf.getvalue() + + +def test_print_keeps_bracketed_names(): + assert "Benchy [remix].stl" in _printed("Benchy [remix].stl") + + +def test_print_does_not_raise_on_stray_closing_tag(): + assert "x[/b]" in _printed("File not found: x[/b]") + + +def test_preview_name_is_display_safe(tmp_path): + _install_ready_settings(tmp_path) + state = WizardState(printable_path=str(tmp_path / "a.gcode"), sliced=False) + rows = dict(core.preview_rows(state, str(tmp_path / "evil\x1b]52;c;AA==\x07.gcode"))) + assert "\x1b" not in rows["Model"] + + +def test_job_failure_keeps_sliced_file(tmp_path): + _install_ready_settings(tmp_path) + stl = _make_stl(tmp_path) + sliced = _sliced_3mf(tmp_path, "keep.gcode.3mf") + job = Recorder(raises=BambuError("printer offline")) + steps = GoSteps(download=Recorder(return_value=stl), slice=Recorder(return_value=sliced), job=job) + prompts = ScriptedPrompts(["https://example.com/cube.stl", True, "PLA", "standard", False, True]) + cwd = tmp_path / "cwd" + cwd.mkdir() + old = os.getcwd() + os.chdir(cwd) + try: + with pytest.raises(BambuError): + cmd_go(_args(), GoDeps(prompts=prompts, steps=steps)) + finally: + os.chdir(old) + assert any("keep.gcode.3mf" in m and "kept" in m.lower() for m in prompts.printed) + assert os.path.exists(sliced) diff --git a/tests/test_interactive_session.py b/tests/test_interactive_session.py index e36c270..5a6e81c 100644 --- a/tests/test_interactive_session.py +++ b/tests/test_interactive_session.py @@ -887,3 +887,86 @@ def test_help_epilog_advertises_go(capsys): out = capsys.readouterr().out assert "plate go" in out assert "walks you through printing from a URL" in out + + +# --------------------------------------------------------------------------- +# The AMS is read again right before the print starts (2026-11-05 audit, S-2) +# --------------------------------------------------------------------------- + + +def _ams_readings(*readings): + """An ams_material seam reporting each ``(material, slot)`` in turn, then the last again.""" + calls = {"n": 0} + + def detector(args, on_active_slot=None): + material, slot = readings[min(calls["n"], len(readings) - 1)] + calls["n"] += 1 + if slot is not None: + on_active_slot(slot) + return material + + detector.calls = calls + return detector + + +def _go_with_ams(tmp_path, detector, start=True): + _install_ready_settings(tmp_path) + stl = _make_stl(tmp_path) + job = Recorder(return_value=None) + steps = GoSteps( + download=Recorder(return_value=stl), + slice=Recorder(return_value=_sliced_3mf(tmp_path)), + job=job, + ams_material=detector, + ) + answers = [stl, True, "PETG", "standard", False, start] + ([] if start else [True]) + prompts = ScriptedPrompts(answers) + return job, prompts, lambda: cmd_go(_args(), GoDeps(prompts=prompts, steps=steps)) + + +def test_print_is_refused_when_the_ams_changed_after_prepare(tmp_path): + job, prompts, run = _go_with_ams(tmp_path, _ams_readings(("PETG", 1), ("PLA", 1))) + with pytest.raises(BambuError) as ei: + run() + assert ei.value.exit_code == 4 + assert job.calls == [], "the print was sent although the AMS no longer matched" + assert "it had PETG in AMS0·1, now PLA in AMS0·1" in str(ei.value) + assert any("The print was not started" in m and "kept at" in m for m in prompts.printed) + + +def test_print_is_refused_when_the_ams_cannot_be_read_again(tmp_path): + job, _prompts, run = _go_with_ams(tmp_path, _ams_readings(("PETG", 1), (None, None))) + with pytest.raises(BambuError) as ei: + run() + assert job.calls == [] + assert "Could not confirm that PETG in AMS0·1" in str(ei.value) + + +def test_print_goes_ahead_when_the_ams_is_unchanged(tmp_path): + detector = _ams_readings(("PETG", 1)) + job, _prompts, run = _go_with_ams(tmp_path, detector) + run() + assert detector.calls["n"] == 2 + assert job.calls[0].confirm is True + assert job.calls[0].ams_mapping == "1" + + +def test_upload_only_skips_the_second_ams_read(tmp_path): + detector = _ams_readings(("PETG", 1), (None, None)) + job, _prompts, run = _go_with_ams(tmp_path, detector, start=False) + run() + assert detector.calls["n"] == 1 + assert job.calls[0].confirm is False + + +def test_preview_labels_never_run_into_their_values(tmp_path): + """"Temperatures" is longer than the old fixed 11-column label field.""" + _install_ready_settings(tmp_path) + stl = _make_stl(tmp_path) + steps = GoSteps(download=Recorder(return_value=stl), slice=Recorder(return_value=_sliced_3mf(tmp_path)), job=Recorder()) + prompts = ScriptedPrompts([stl, True, "PLA", "standard", False, False, False]) + cmd_go(_args(), GoDeps(prompts=prompts, steps=steps)) + rows = [m for m in prompts.printed if m.startswith(("Model", "Feed", "Temperatures", "Estimate"))] + assert len(rows) == 4 + assert all(line.split(" ", 1)[0] in ("Model", "Feed", "Temperatures", "Estimate") for line in rows) + assert len({len(line) - len(line.split(" ", 1)[1].lstrip()) for line in rows}) == 1, "values are not aligned" diff --git a/tests/test_job.py b/tests/test_job.py index d50ae91..ee4b095 100644 --- a/tests/test_job.py +++ b/tests/test_job.py @@ -46,6 +46,7 @@ def emit(self, record): from bambu_cli.job import JobSteps, _run_job # noqa: E402 from bambu_cli.errors import BambuError from tests.json_contract_base import sliced_3mf_bytes +from tests.fake_3mf import minimal_3mf def _json_path_field(path): @@ -534,7 +535,7 @@ def test_dry_run_local_zip_member_slice_matches_predicate(tmp_path, member, woul """ zpath = tmp_path / "bundle.zip" with zipfile.ZipFile(zpath, "w") as zf: - zf.writestr(member, b"content") + zf.writestr(member, minimal_3mf() if member.endswith(".3mf") else b"content") args = _parse(["job", str(zpath), "--dry-run", "--json"]) _run_job(_ctx(), args, default_steps()) payload = _read_json(capsys) @@ -1179,7 +1180,7 @@ def test_zip_printer_ready_member_copies_ignored_flagged(tmp_path, capsys): copies_ignored, matching the non-ZIP printer-ready branch.""" zpath = tmp_path / "bundle.zip" with zipfile.ZipFile(zpath, "w") as zf: - zf.writestr("plate.gcode.3mf", b"x" * 20) + zf.writestr("plate.gcode.3mf", minimal_3mf()) args = _parse(["job", str(zpath), "--copies", "3", "--dry-run", "--json"]) with _capture_bambu_warnings() as records: _run_job(_ctx(), args, default_steps()) @@ -1245,3 +1246,31 @@ def _probe_denies_in_tmp(*a, **kw): assert getattr(excinfo.value, "exit_code", getattr(excinfo.value, "code", None)) == EXIT_FILE_ERROR payload = _read_json(capsys) assert payload["failed_step"] == "validate" + + +def test_unacknowledged_print_start_leaves_printed_unknown(tmp_path, capsys): + """A sent-but-unacknowledged print is "printed: null", never "false". + + "false" tells an agent to run the print again; the printer may already be + printing the first one. + """ + from bambu_cli.constants import EXIT_TIMEOUT + from bambu_cli.protocols import mqtt_print + + ready = tmp_path / "model.3mf" + ready.write_bytes(sliced_3mf_bytes()) + args = _parse(["job", str(ready), "--confirm", "--json"]) + + def unacknowledged_print(_args): + mqtt_print._abort_unconfirmed("model.3mf", "within 30s", EXIT_TIMEOUT) + + steps = default_steps(upload=fake_upload("model.3mf"), print_=unacknowledged_print) + with pytest.raises((SystemExit, BambuError)): + _run_job(_ctx(), args, steps) + payload = _read_json(capsys) + assert payload["failed_step"] == "print" + assert payload["uploaded"] is True + assert payload["printed"] is None + assert payload["print_error"]["sent"] is True + assert "check `plate status` before printing again" in payload["error"] + assert payload["next_command"] == ["status", "--json"] diff --git a/tests/test_monitor_connection_errors.py b/tests/test_monitor_connection_errors.py index 6f89f9c..ea08c00 100644 --- a/tests/test_monitor_connection_errors.py +++ b/tests/test_monitor_connection_errors.py @@ -55,7 +55,7 @@ def _printer(timeout=0.2): def _run(client, *, connect=lambda p, c: None, timeout=0.2): with ( - mock.patch.object(mqtt_monitor, "_client_factory", lambda f: lambda printer: client), + mock.patch.object(mqtt_monitor, "_client_factory", lambda f: lambda printer, **_kw: client), mock.patch.object(mqtt_monitor, "_connect", connect), ): return mqtt_monitor.monitor_status(argparse.Namespace(json=True), _printer(timeout)) diff --git a/tests/test_mqtt_command_delivery.py b/tests/test_mqtt_command_delivery.py new file mode 100644 index 0000000..7a35ad2 --- /dev/null +++ b/tests/test_mqtt_command_delivery.py @@ -0,0 +1,379 @@ +"""A printer command goes out at most once, and an unacknowledged one says so. + +Found in the 2026-11 audit; each test failed before its fix: + +* paho's network loop reconnected after a drop and resent the unacknowledged + QoS-1 PUBLISH (DUP set), so a command that reached the printer just before + the connection died ran twice — while ``send_command`` reported success; +* a print start whose acknowledgement never came was reported as + ``printed: false`` although the command had been published, inviting a + retry that starts a second print; +* a malformed report (deeply nested JSON, an over-long integer, non-object + JSON) raised out of ``on_message`` and killed paho's network thread. +""" + +from __future__ import annotations + +import importlib +import socket +import sys +import threading +import types + +import pytest + +from bambu_cli import utils +from bambu_cli.constants import EXIT_NETWORK_ERROR, EXIT_TIMEOUT +from bambu_cli.errors import BambuError, CommandUnconfirmed +from bambu_cli.printer import BambuPrinter +from bambu_cli.protocols import mqtt_cmd, mqtt_print, mqtt_tls +from bambu_cli.protocols.mqtt_cmd import decode_report +from bambu_cli.protocols.mqtt_session import MqttSession + +# --- a broker that drops the connection instead of sending PUBACK ----------- + + +def _read_packet(conn): + header = conn.recv(1) + if not header: + return None + multiplier, length = 1, 0 + while True: + byte = conn.recv(1)[0] + length += (byte & 127) * multiplier + multiplier *= 128 + if not byte & 128: + break + body = b"" + while len(body) < length: + chunk = conn.recv(length - len(body)) + if not chunk: + return None + body += chunk + return header[0], body + + +class _DroppingBroker: + """Accepts CONNECT, records every PUBLISH, and hangs up instead of a PUBACK.""" + + def __init__(self): + self.connections = 0 + self.publishes = 0 + self._server = socket.socket() + self._server.setsockopt(socket.SOL_SOCKET, socket.SO_REUSEADDR, 1) + self._server.bind(("127.0.0.1", 0)) + self._server.listen(5) + self._server.settimeout(0.2) + self.port = self._server.getsockname()[1] + self._stop = threading.Event() + self._thread = threading.Thread(target=self._serve, daemon=True) + self._thread.start() + + def _serve(self): + while not self._stop.is_set(): + try: + conn, _ = self._server.accept() + except OSError: + continue + self.connections += 1 + with conn: + conn.settimeout(2) + try: + while True: + packet = _read_packet(conn) + if packet is None: + break + kind = packet[0] >> 4 + if kind == 1: # CONNECT -> CONNACK + conn.sendall(b"\x20\x02\x00\x00") + elif kind == 3: # PUBLISH -> hang up, no PUBACK + self.publishes += 1 + break + elif kind == 14: # DISCONNECT + break + except OSError: + pass + + def close(self): + self._stop.set() + self._thread.join(timeout=5) + self._server.close() + + +@pytest.fixture +def real_paho(monkeypatch): + """The real paho client module for one test. + + tests/bambu_test_base.py replaces paho with a MagicMock for the whole run; + the redelivery bug lives inside paho's own reconnect loop, so only the real + library can show it. The stub is put back before the test body runs. + """ + is_paho = lambda name: name == "paho" or name.startswith("paho.") # noqa: E731 + stub = {name: sys.modules.pop(name) for name in list(sys.modules) if is_paho(name)} + try: + real = importlib.import_module("paho.mqtt.client") + finally: + for name in [name for name in sys.modules if is_paho(name)]: + del sys.modules[name] + sys.modules.update(stub) + monkeypatch.setattr(mqtt_tls, "mqtt", real) + return real + + +@pytest.fixture +def broker(): + b = _DroppingBroker() + yield b + b.close() + + +def _printer(port, timeout=3.0): + printer = BambuPrinter("127.0.0.1", "SER", "code") + printer.mqtt_port = port + printer.mqtt_timeout = timeout + return printer + + +def test_a_dropped_command_is_not_resent_by_the_client(real_paho, broker): + """Real paho against a local broker: one PUBLISH, reported as unconfirmed.""" + printer = _printer(broker.port) + with pytest.raises(CommandUnconfirmed) as excinfo: + mqtt_cmd.send_command( + printer, + '{"print": {"command": "gcode_line", "param": "G91 G1 Z10"}}', + retries=2, + client_factory=lambda _printer, client_id="": mqtt_tls._paho_client(client_id), + sleep=lambda _s: None, + ) + assert excinfo.value.extra == {"sent": True, "acknowledged": False} + assert broker.publishes == 1, "the command reached the printer more than once" + assert broker.connections == 1, "the client reconnected on its own after the drop" + + +# --- print start: published but never acknowledged --------------------------- + + +class _SilentPrinterClient: + """Connects and accepts the publish; the printer never answers.""" + + def __init__(self, fail_after_publish=None): + self.published = [] + self._fail_after_publish = fail_after_publish + + def user_data_set(self, _data): + pass + + def connect(self, *_args, **_kwargs): + pass + + def subscribe(self, _topic): + pass + + def publish(self, _topic, payload, qos=0): + self.published.append(payload) + + def loop_start(self): + self.on_connect(self, None, None, 0) + if self._fail_after_publish is not None: + raise self._fail_after_publish + + def loop_stop(self): + pass + + def disconnect(self): + pass + + +def _run_print(client): + printer = BambuPrinter("127.0.0.1", "SER", "code") + utils._LAST_ERROR_PAYLOAD = None + with pytest.raises(BambuError) as excinfo: + mqtt_print.execute_print_command( + printer, + '{"print": {"command": "project_file"}}', + "cube.3mf", + # The ack wait is command_timeout + 5 s; keep the test fast. + command_timeout=-4.9, + client_factory=lambda _printer, _client_id="": client, + ) + return excinfo.value, utils._LAST_ERROR_PAYLOAD + + +def test_unacknowledged_print_start_is_reported_as_sent_not_as_not_printed(): + client = _SilentPrinterClient() + exc, payload = _run_print(client) + assert len(client.published) == 1 + assert exc.exit_code == EXIT_TIMEOUT + assert payload["printed"] is None + assert payload["sent"] is True + assert payload["acknowledged"] is False + assert payload["next_command"] == ["status", "--json"] + assert "check `plate status` before printing again" in payload["error"] + + +def test_a_network_error_after_publishing_is_also_unconfirmed(): + client = _SilentPrinterClient(fail_after_publish=OSError("connection reset")) + exc, payload = _run_print(client) + assert len(client.published) == 1 + assert exc.exit_code == EXIT_NETWORK_ERROR + assert payload["printed"] is None + assert payload["sent"] is True + + +def test_nothing_published_still_reports_not_printed(): + class _NeverConnects(_SilentPrinterClient): + def loop_start(self): + pass + + client = _NeverConnects() + exc, payload = _run_print(client) + assert client.published == [] + assert exc.exit_code == EXIT_TIMEOUT + assert payload["printed"] is False + assert "sent" not in payload + + +# --- hostile reports never escape the network thread ------------------------- + +HOSTILE_PAYLOADS = [ + b"[" * 200_000, # RecursionError in json.loads + b"1" * 5_000, # ValueError: integer string past the digit limit + b"5", + b"[1]", + b'"text"', + b"null", + b"\xff\xfe", + b"{not json", +] + + +@pytest.mark.parametrize("payload", HOSTILE_PAYLOADS) +def test_decode_report_returns_none_for_anything_but_an_object(payload): + assert decode_report(types.SimpleNamespace(payload=payload)) is None + + +def test_decode_report_passes_a_report_through(): + msg = types.SimpleNamespace(payload=b'{"print": {"gcode_state": "IDLE"}}') + assert decode_report(msg) == {"print": {"gcode_state": "IDLE"}} + + +@pytest.mark.parametrize("payload", HOSTILE_PAYLOADS) +def test_held_session_survives_a_hostile_report(payload): + session = MqttSession(BambuPrinter("127.0.0.1", "SER", "code")) + session._on_message(None, None, types.SimpleNamespace(payload=payload)) + session._on_message(None, None, types.SimpleNamespace(payload=b'{"print": {"gcode_state": "RUNNING"}}')) + assert session._snapshot() == {"gcode_state": "RUNNING"} + + +def test_the_raised_error_carries_the_unconfirmed_fields(): + """The --json envelope is built from the exception, so it must say so too.""" + exc, _payload = _run_print(_SilentPrinterClient()) + envelope = exc.to_error_payload("print") + assert envelope["printed"] is None + assert envelope["sent"] is True + assert envelope["acknowledged"] is False + assert envelope["next_command"] == ["status", "--json"] + assert "was sent, but the printer did not acknowledge it" in envelope["error"] + assert exc.logged, "the line is already logged; the CLI must not print it twice" + + +# --- the status monitor still rides out a dropped connection --------------------- + + +def _publish_packet(topic, payload): + body = len(topic).to_bytes(2, "big") + topic + payload + length, encoded = len(body), b"" + while True: + byte, length = length % 128, length // 128 + encoded += bytes([byte | (128 if length else 0)]) + if not length: + return b"\x30" + encoded + body + + +class _DropOnceBroker: + """First connection: a RUNNING report, then a hang-up. Later ones: FINISH.""" + + def __init__(self): + self.connections = 0 + self.client_ids = [] + self._server = socket.socket() + self._server.setsockopt(socket.SOL_SOCKET, socket.SO_REUSEADDR, 1) + self._server.bind(("127.0.0.1", 0)) + self._server.listen(5) + self._server.settimeout(0.2) + self.port = self._server.getsockname()[1] + self._stop = threading.Event() + self._thread = threading.Thread(target=self._serve, daemon=True) + self._thread.start() + + def _report(self, state, pct): + return _publish_packet(b"device/SER/report", f'{{"print": {{"gcode_state": "{state}", "mc_percent": {pct}}}}}'.encode()) + + def _serve(self): + while not self._stop.is_set(): + try: + conn, _ = self._server.accept() + except OSError: + continue + self.connections += 1 + first = self.connections == 1 + with conn: + conn.settimeout(3) + try: + while True: + packet = _read_packet(conn) + if packet is None: + break + kind, body = packet[0] >> 4, packet[1] + if kind == 1: # CONNECT: protocol name, level, flags, keepalive, then the client id + id_len = int.from_bytes(body[10:12], "big") + self.client_ids.append(body[12 : 12 + id_len].decode()) + conn.sendall(b"\x20\x02\x00\x00") + elif kind == 8: # SUBSCRIBE -> SUBACK, then a report + conn.sendall(bytes([0x90, 3, body[0], body[1], 0])) + conn.sendall(self._report("RUNNING", 10) if first else self._report("FINISH", 100)) + if first: + break # hang up mid-print + elif kind == 14: + break + except OSError: + pass + + def close(self): + self._stop.set() + self._thread.join(timeout=5) + self._server.close() + + +def test_status_monitor_reconnects_after_a_drop(real_paho, monkeypatch, capsys): + """The monitor relied on paho's auto-reconnect; turning it off for commands hung it.""" + from bambu_cli.protocols import mqtt as mqtt_facade + from bambu_cli.protocols.mqtt_monitor import monitor_status + + broker = _DropOnceBroker() + monkeypatch.setattr( + mqtt_facade, + "create_mqtt_client", + lambda _printer, client_id="", reconnect=False: mqtt_tls._paho_client(client_id, reconnect=reconnect), + ) + done = [] + worker = threading.Thread( + target=lambda: (monitor_status(types.SimpleNamespace(json=True), _printer(broker.port)), done.append(True)), + daemon=True, + ) + try: + worker.start() + worker.join(15) + finally: + broker.close() + assert done, "the monitor never saw FINISH after the connection dropped" + assert broker.connections >= 2 + assert all(cid.startswith("plate-") for cid in broker.client_ids), broker.client_ids + assert '"terminal"' in capsys.readouterr().out + + +def test_decode_report_drops_reports_nested_past_what_the_json_walkers_handle(): + deep = b'{"a":' * 5000 + b"1" + b"}" * 5000 + assert decode_report(types.SimpleNamespace(payload=deep)) is None + real = b'{"print": {"ams": {"ams": [{"tray": [{"id": "0", "tray_type": "PLA"}]}]}}}' + assert decode_report(types.SimpleNamespace(payload=real))["print"]["ams"]["ams"][0]["tray"][0]["id"] == "0" diff --git a/tests/test_output_hostile_report_values.py b/tests/test_output_hostile_report_values.py new file mode 100644 index 0000000..0431b84 --- /dev/null +++ b/tests/test_output_hostile_report_values.py @@ -0,0 +1,134 @@ +"""Hostile MQTT report values must neither crash parsing nor produce invalid JSON.""" + +import json +from types import SimpleNamespace +from unittest.mock import MagicMock + +import pytest + +from bambu_cli.ams import parse_ams + +_BAD = [ + float("inf"), + float("-inf"), + float("nan"), + 1e999, + 10**400, + "x", + [], + {}, + None, + [1], + {"a": 1}, +] + + +def _reject(name): + raise ValueError(f"invalid JSON constant {name}") + + +def test_status_json_is_strict_json(capsys): + from bambu_cli.ams import parse_ams + from bambu_cli.commands.status import _status_payload + from bambu_cli.utils import emit_json + + data = json.loads('{"nozzle_temper": NaN, "bed_temper": Infinity, "extra": [-Infinity, {"a": NaN}]}') + emit_json(_status_payload(data, parse_ams(data))) + out = json.loads(capsys.readouterr().out, parse_constant=_reject) + assert out["printer"]["nozzle_temper"] is None + assert out["printer"]["bed_temper"] is None + assert out["printer"]["extra"] == [None, {"a": None}] + + +@pytest.mark.parametrize("bad", _BAD, ids=repr) +def test_parse_ams_tolerates_any_value(bad): + status = { + "ams": { + "tray_now": bad, + "ams": [ + {"id": bad, "humidity": bad, "temp": bad, "tray": bad}, + {"id": 0, "humidity": bad, "tray": [{"id": bad, "remain": bad, "tray_type": bad, "tray_color": bad}]}, + ], + }, + "vt_tray": {"id": bad, "remain": bad, "tray_type": "PLA"}, + "tray_now": bad, + } + result = parse_ams(status) + assert result is None or isinstance(result["units"], list) + json.dumps(result, allow_nan=False) + + +@pytest.mark.parametrize("bad", _BAD, ids=repr) +def test_human_status_tolerates_any_value(bad): + from bambu_cli.commands.status import cmd_status + + data = { + "gcode_state": "RUNNING", + "mc_remaining_time": bad, + "mc_percent": bad, + "layer_num": bad, + "ams": {"tray_now": bad, "ams": [{"id": 0, "tray": [{"id": 0, "remain": bad, "tray_type": "PLA"}]}]}, + } + printer = MagicMock() + printer.status.return_value = data + ctx = SimpleNamespace(printer=lambda: printer) + cmd_status(SimpleNamespace(json=False, monitor=False), ctx) + + +_ESC_NAME = "\x1b]52;c;ZXZpbA==\x07\x1b[2Jevil.3mf" + + +def test_human_status_does_not_pass_terminal_escapes(caplog): + """gcode_file and tray_type come from the printer; ESC must not reach the terminal.""" + from bambu_cli.commands.status import cmd_status + + data = { + "gcode_state": "RUNNING", + "gcode_file": _ESC_NAME, + "wifi_signal": "-40dBm\x1b[2J", + "ams": {"tray_now": "0", "ams": [{"id": 0, "tray": [{"id": 0, "remain": 50, "tray_type": "PLA\x1b[2J"}]}]}, + "vt_tray": {"id": 254, "tray_type": "PETG\u202e"}, + } + printer = MagicMock() + printer.status.return_value = data + with caplog.at_level("INFO", logger="bambu"): + cmd_status(SimpleNamespace(json=False, monitor=False), SimpleNamespace(printer=lambda: printer)) + assert "evil.3mf" in caplog.text + assert "\x1b" not in caplog.text and "\u202e" not in caplog.text + + +def test_monitor_status_lines_do_not_pass_terminal_escapes(caplog): + from bambu_cli.protocols import mqtt_monitor + + client = MagicMock() + + def loop_start(): + userdata = {} + client.on_connect(client, userdata, None, 0) + report = {"print": {"gcode_state": "RUNNING\x1b[2J", "mc_percent": 50}} + client.on_message(client, userdata, SimpleNamespace(payload=json.dumps(report).encode())) + client.on_message(client, userdata, SimpleNamespace(payload=b'{"print": {"gcode_state": "FINISH"}}')) + + client.loop_start.side_effect = loop_start + printer = SimpleNamespace(simulation_mode=False, serial="SER", mqtt_timeout=1) + with ( + caplog.at_level("INFO", logger="bambu"), + pytest.MonkeyPatch.context() as mp, + ): + mp.setattr(mqtt_monitor, "_client_factory", lambda _f: lambda _printer, **_kw: client) + mp.setattr(mqtt_monitor, "_connect", lambda _printer, _client: None) + mp.setattr("sys.stdout.isatty", lambda: False) + mqtt_monitor.monitor_status(SimpleNamespace(json=False), printer) + assert "RUNNING" in caplog.text and "FINISH" in caplog.text + assert "\x1b" not in caplog.text + + +@pytest.mark.parametrize("bad", [1e999, float("-inf"), float("nan")], ids=repr) +def test_tui_and_monitor_numbers_tolerate_non_finite_values(bad): + from bambu_cli.protocols.mqtt_monitor import _status_event + from bambu_cli.tui.services import format_remaining, progress_percent + + assert format_remaining(bad) == "—" + assert progress_percent(SimpleNamespace(ok=True, raw={"mc_percent": bad})) == 0 + event = _status_event({"gcode_state": "FINISH", "mc_percent": bad, "mc_remaining_time": bad}, "terminal") + assert event["mc_percent"] == 0 and event["mc_remaining_time"] == 0 diff --git a/tests/test_presliced_heater_limits.py b/tests/test_presliced_heater_limits.py new file mode 100644 index 0000000..1796b24 --- /dev/null +++ b/tests/test_presliced_heater_limits.py @@ -0,0 +1,211 @@ +"""Pre-sliced G-code is held to the heater limits before it reaches the printer. + +``plate gcode`` refused ``M104 S400`` while a downloaded 3MF carrying the same +line went straight to the SD card through ``upload``/``job``/``send``: platecli +never generated that G-code, so nothing had checked it. +""" + +from __future__ import annotations + +import argparse +import io +import zipfile + +import pytest + +from bambu_cli.commands.files import cmd_upload +from bambu_cli.constants import EXIT_FILE_ERROR +from bambu_cli.errors import BambuError +from bambu_cli.gcode_limits import file_heater_problem + +_STAMP = (2026, 1, 1, 0, 0, 0) + + +class _Printer: + simulation_mode = False + + def __init__(self): + self.uploads = [] + + def status(self, timeout=None, retries=2, *, require_complete=True): + return {"gcode_state": "IDLE"} + + def upload_file(self, local, remote, **kwargs): + self.uploads.append(remote) + return True + + +def _three_mf(path, members): + buf = io.BytesIO() + with zipfile.ZipFile(buf, "w", zipfile.ZIP_DEFLATED) as zf: + zf.writestr(zipfile.ZipInfo("[Content_Types].xml", date_time=_STAMP), "") + for name, text in members.items(): + zf.writestr(zipfile.ZipInfo(name, date_time=_STAMP), text) + path.write_bytes(buf.getvalue()) + return path + + +def _upload(path, printer, dry_run=False): + args = argparse.Namespace(file=str(path), dry_run=dry_run, json=False, progress=False) + return cmd_upload(args, ctx=argparse.Namespace(printer=lambda: printer)) + + +BAMBU_LIKE = ( + "; HEADER_BLOCK_START\n; nozzle_temperature = 220\n; HEADER_BLOCK_END\n" + "M140 S55 ; set bed temperature\n" + "M104 S220 ; Set nozzle\n" + "M109 S25 H140\n" + "M190 S55\n" + "G28\n" + "M104 T1 S230\n" + "M106 P1 S255\n" + "M104 S0\n" +) + + +@pytest.mark.parametrize( + "line", + ["M104 S400", "M109 S2200 ; typo", "M140 S200", "M190 T0 S900", " m104 s351", "N12 M104 S999", "M104 S2e3"], +) +def test_a_hot_plate_in_a_3mf_is_refused_before_upload(tmp_path, line): + path = _three_mf(tmp_path / "model.gcode.3mf", {"Metadata/plate_1.gcode": BAMBU_LIKE + line + "\n"}) + printer = _Printer() + with pytest.raises(BambuError) as excinfo: + _upload(path, printer) + assert excinfo.value.exit_code == EXIT_FILE_ERROR + assert "Metadata/plate_1.gcode line 12" in str(excinfo.value) + assert printer.uploads == [] + + +def test_every_plate_is_checked_not_only_the_first(tmp_path): + path = _three_mf( + tmp_path / "two.3mf", + {"Metadata/plate_1.gcode": BAMBU_LIKE, "Metadata/plate_2.gcode": "G28\nM140 S300\n"}, + ) + assert "Metadata/plate_2.gcode line 2" in file_heater_problem(path) + + +def test_a_plain_gcode_file_is_checked(tmp_path): + path = tmp_path / "hot.gcode" + path.write_text("G28\nM104 S500\n") + printer = _Printer() + with pytest.raises(BambuError) as excinfo: + _upload(path, printer, dry_run=True) + assert "hot.gcode line 2" in str(excinfo.value) + assert printer.uploads == [] + + +def test_ordinary_sliced_output_uploads(tmp_path): + path = _three_mf(tmp_path / "model.gcode.3mf", {"Metadata/plate_1.gcode": BAMBU_LIKE}) + printer = _Printer() + assert _upload(path, printer) == "model.gcode.3mf" + assert printer.uploads == ["/model/model.gcode.3mf"] + + +def test_a_member_that_cannot_be_read_fails_closed(tmp_path): + path = _three_mf(tmp_path / "broken.3mf", {"Metadata/plate_1.gcode": "M104 S220\n" * 2000}) + data = bytearray(path.read_bytes()) + start = data.index(b"Metadata/plate_1.gcode") + len(b"Metadata/plate_1.gcode") + data[start : start + 40] = b"\xff" * 40 # corrupt the deflate stream right after the local header + path.write_bytes(bytes(data)) + problem = file_heater_problem(path) + assert problem and "could not read Metadata/plate_1.gcode" in problem + + +def test_job_dry_run_reports_the_refusal_too(tmp_path, capsys): + import json + + from bambu_cli.cli import build_parser + from bambu_cli.context import RuntimeContext + from bambu_cli.job import JobSteps, _run_job + + path = _three_mf(tmp_path / "model.3mf", {"Metadata/plate_1.gcode": "G28\nM104 S400\n"}) + args = build_parser().parse_args(["job", str(path), "--dry-run", "--json"]) + with pytest.raises(BambuError): + _run_job(RuntimeContext(), args, JobSteps()) + payload = json.loads(capsys.readouterr().out) + assert payload["failed_step"] == "validate" + assert "above the 350 °C limit" in payload["error"] + + +# --- found by the independent review of the S-1 fix -------------------------- + + +def test_a_duplicate_member_name_cannot_hide_a_hot_plate(tmp_path): + """ZipFile.open(name) returns the LAST entry of a name; an unzip that takes the first would run it.""" + path = tmp_path / "dup.3mf" + with zipfile.ZipFile(path, "w") as zf, pytest.warns(UserWarning, match="Duplicate name"): + zf.writestr(zipfile.ZipInfo("Metadata/plate_1.gcode", date_time=_STAMP), "M104 S400\n") + zf.writestr(zipfile.ZipInfo("Metadata/plate_1.gcode", date_time=_STAMP), "M104 S220\n") + assert "above the 350 °C limit" in file_heater_problem(path) + from bambu_cli.gcode_limits import peak_heater_targets + + assert peak_heater_targets(path) == {"nozzle": 400.0} + + +@pytest.mark.parametrize("eol", ["\r", "\r\n", "\n"]) +def test_every_line_ending_is_a_line_break(tmp_path, eol): + path = tmp_path / "part.gcode" + path.write_bytes(eol.join(["G28", "M104 S220", "M104 S400", ""]).encode()) + assert file_heater_problem(path) == "part.gcode line 3: Nozzle temperature 400 °C is above the 350 °C limit." + + +def test_a_crlf_split_across_read_chunks_is_one_line_break(tmp_path, monkeypatch): + from bambu_cli import gcode_limits + + monkeypatch.setattr(gcode_limits, "_CHUNK", 4) # "G28\r" | "\nM10" ... + path = tmp_path / "part.gcode" + path.write_bytes(b"G28\r\nM104 S220\r\nM104 S400\r\n") + assert file_heater_problem(path).startswith("part.gcode line 3:") + + +def test_a_line_too_long_to_check_is_refused_without_reading_it_whole(tmp_path, monkeypatch): + from bambu_cli import gcode_limits + + monkeypatch.setattr(gcode_limits, "MAX_LINE_BYTES", 64) + monkeypatch.setattr(gcode_limits, "_CHUNK", 16) + path = _three_mf(tmp_path / "bomb.3mf", {"Metadata/plate_1.gcode": "G28\n" + "M" * 10_000}) + reads = [] + real_open = zipfile.ZipFile.open + + def counting_open(self, *a, **kw): + fh = real_open(self, *a, **kw) + real_read = fh.read + fh.read = lambda n=-1: reads.append(n) or real_read(n) + return fh + + monkeypatch.setattr(zipfile.ZipFile, "open", counting_open) + assert "Metadata/plate_1.gcode line 2 is over" in file_heater_problem(path) + assert all(n == 16 for n in reads), "a read was not bounded" + assert gcode_limits.peak_heater_targets(path) is None + + +def test_a_3mf_that_cannot_be_opened_is_refused(tmp_path): + path = tmp_path / "model.3mf" + path.write_bytes(b"not a zip at all") + printer = _Printer() + with pytest.raises(BambuError) as excinfo: + _upload(path, printer) + assert "could not open it as a 3MF" in str(excinfo.value) + assert printer.uploads == [] + + +@pytest.mark.parametrize("member", ["hot.gcode", "hot.gcode.3mf"]) +def test_job_dry_run_of_a_zip_checks_the_member_too(tmp_path, capsys, member): + import json + + from bambu_cli.cli import build_parser + from bambu_cli.context import RuntimeContext + from bambu_cli.job import JobSteps, _run_job + + hot = "G28\nM140 S200\n" + inner = _three_mf(tmp_path / "inner.3mf", {"Metadata/plate_1.gcode": hot}).read_bytes() + archive = tmp_path / "bundle.zip" + with zipfile.ZipFile(archive, "w") as zf: + zf.writestr(member, inner if member.endswith(".3mf") else hot) + args = build_parser().parse_args(["job", str(archive), "--dry-run", "--json"]) + with pytest.raises(BambuError): + _run_job(RuntimeContext(), args, JobSteps()) + payload = json.loads(capsys.readouterr().out) + assert payload["failed_step"] == "validate" + assert "above the 150 °C limit" in payload["error"] diff --git a/tests/test_printables_adapter.py b/tests/test_printables_adapter.py index a8ec5ba..439ccea 100644 --- a/tests/test_printables_adapter.py +++ b/tests/test_printables_adapter.py @@ -238,6 +238,21 @@ def test_ok_link_with_no_url_is_a_contract_change(_log): assert result.ok is False assert result.reason == "printables_contract_changed" +@pytest.mark.parametrize( + "output", + [{"link": 123}, {"link": ["https://files.example/a.stl"]}, {"link": {"url": "x"}}, ["https://files.example/a.stl"]], +) +@patch("bambu_cli.logging_utils._BACKEND") +def test_ok_link_that_is_not_a_string_is_a_contract_change(_log, output): + """A non-string link used to come back as success and crash the downloader.""" + adapter, _ = _adapter( + _model(stls=[{"name": "part1.stl", "fileSize": 1024, "id": "file_123"}]), + {"data": {"getDownloadLink": {"ok": True, "output": output}}}, + ) + result = adapter.resolve(MODEL_URL) + assert result.ok is False + assert result.reason == "printables_contract_changed" + # --- containment: the reason this package exists ----------------------------- # Every payload here is something Printables could plausibly start returning. diff --git a/tests/test_properties_safety.py b/tests/test_properties_safety.py index c95629c..fa6d2e5 100644 --- a/tests/test_properties_safety.py +++ b/tests/test_properties_safety.py @@ -426,3 +426,10 @@ def test_prop_print_payload_ams_only_when_use_ams(basename: str, use_ams: bool, assert data["print"]["url"].startswith("file:///sdcard/model/") assert "\n" not in data["print"]["url"] assert "\r" not in data["print"]["url"] + +@pytest.mark.parametrize("raw", ["1_0", "1", "+1", "0x1", "1e1", "1,,2", "١"]) +def test_ams_mapping_requires_ascii_digits(raw: str) -> None: + """int() alone reads "1_0" as slot 10 and a fullwidth digit as 1: a typo picked another tray.""" + mapping, err = job_payload._parse_print_options(argparse.Namespace(use_ams=True, ams_mapping=raw)) + assert mapping is None + assert err is not None and "mapping" in err.lower() diff --git a/tests/test_protocol_clients.py b/tests/test_protocol_clients.py index b99b0f0..afe0deb 100644 --- a/tests/test_protocol_clients.py +++ b/tests/test_protocol_clients.py @@ -224,7 +224,7 @@ def test_create_mqtt_client_secure(self, mock_mqtt_client): client = create_mqtt_client(printer, "test_client") # Use ANY for the version argument to avoid identity mismatches with module-level mocks - mock_mqtt_client.assert_called_once_with(ANY, "test_client") + mock_mqtt_client.assert_called_once_with(ANY, "test_client", reconnect_on_failure=False) mock_client_instance.username_pw_set.assert_called_once_with("bblp", "mock_access_code") mock_client_instance.tls_set.assert_called_once_with(cert_reqs=ssl.CERT_REQUIRED) mock_client_instance.tls_insecure_set.assert_not_called() @@ -238,13 +238,31 @@ def test_create_mqtt_client_insecure(self, mock_mqtt_client): printer = _test_printer(access_code="mock_access_code", insecure_tls=True) client = create_mqtt_client(printer) - mock_mqtt_client.assert_called_once_with(ANY, "") + mock_mqtt_client.assert_called_once_with(ANY, ANY, reconnect_on_failure=False) + client_id = mock_mqtt_client.call_args.args[1] + assert client_id.startswith("plate-") and len(client_id) <= 23 mock_client_instance.username_pw_set.assert_called_once_with("bblp", "mock_access_code") mock_client_instance.tls_set.assert_called_once_with(cert_reqs=ssl.CERT_NONE) mock_client_instance.tls_insecure_set.assert_called_once_with(True) self.assertEqual(client, mock_client_instance) + @patch("bambu_cli.protocols.mqtt_tls.pinning_ssl_context") + @patch("bambu_cli.protocols.mqtt_tls.mqtt.Client") + def test_pin_wins_over_insecure_tls(self, mock_mqtt_client, mock_pinning): + """insecure_tls plus a pin used to skip the pin on MQTT while FTPS enforced it.""" + mock_client_instance = MagicMock() + mock_mqtt_client.return_value = mock_client_instance + pin = "ab" * 32 + + printer = _test_printer(access_code="mock_access_code", insecure_tls=True, cert_fingerprint=pin) + create_mqtt_client(printer) + + mock_pinning.assert_called_once_with(pin) + mock_client_instance.tls_set_context.assert_called_once_with(mock_pinning.return_value) + mock_client_instance.tls_set.assert_not_called() + + class TestMqttConnectTimeout(unittest.TestCase): def test_mqtt_connect_sets_client_timeout_without_mutating_socket_default(self): import socket as socket_mod diff --git a/tests/test_remote_name_control_chars.py b/tests/test_remote_name_control_chars.py new file mode 100644 index 0000000..e4ff976 --- /dev/null +++ b/tests/test_remote_name_control_chars.py @@ -0,0 +1,37 @@ +"""Printer-side names refuse every control and bidi character, and repair agrees. + +``_safe_remote_name`` used to reject only CR/LF/NUL, so ESC, C1 controls and +bidi overrides could reach the SD card and every later terminal line that +shows the name. ``_sanitize_download_filename`` must keep producing names the +check accepts, or a file that downloads could not be uploaded. +""" + +import pytest + +from bambu_cli.download.naming import _safe_remote_name, _sanitize_download_filename + +UNSAFE = [ + "ok\x1b[2J.3mf", + "we ird\x1b[31m.gcode", + "csi\x9b31m.stl", + "abc‮gcode.3mf", + "iso⁦late.stl", + "tab\there.stl", +] + + +@pytest.mark.parametrize("name", UNSAFE) +def test_control_and_bidi_names_are_refused(name): + assert _safe_remote_name(name) is None + + +@pytest.mark.parametrize("name", UNSAFE) +def test_repair_output_is_always_accepted(name): + repaired = _sanitize_download_filename(name) + assert _safe_remote_name(repaired) == repaired + + +@pytest.mark.parametrize("name", ["Benchy [remix].stl", "модель.stl", "cube v2.3mf"]) +def test_ordinary_names_are_unchanged(name): + assert _safe_remote_name(name) == name + assert _sanitize_download_filename(name) == name diff --git a/tests/test_setup_lone_mdns.py b/tests/test_setup_lone_mdns.py new file mode 100644 index 0000000..7c368ba --- /dev/null +++ b/tests/test_setup_lone_mdns.py @@ -0,0 +1,165 @@ +"""Setup confirms what mDNS found (2026-11-05 audit, S-3). + +mDNS is unauthenticated, and the access code goes to whatever address is +configured. Before this fix a lone result was taken without asking and its +address was never looked at, so one spoofed answer on the LAN (``0.0.0.0``, a +public address) became the printer. Each test failed before the fix. +""" + +from __future__ import annotations + +import argparse +import builtins +import json +import sys +from unittest.mock import MagicMock, patch + +import pytest + +from bambu_cli.errors import BambuError +from bambu_cli.setup_cmd import common as common_mod +from bambu_cli.setup_cmd import wizard as wizard_mod +from tests.test_wizard_guided import create_mock_zeroconf, mock_service_browser + +P1P = "BBLP-P1P-01P00A123._bblp._tcp.local." +A1 = "BBLP-A1-03000A111._bblp._tcp.local." + + +def _run_setup(tmp_path, monkeypatch, services, answers): + """Guided setup against fake mDNS results; returns (config or None, prompts, log lines, error).""" + cfg = tmp_path / "config.json" + pending = list(answers) + asked = [] + + def fake_prompt(msg, args=None, default=None): + asked.append(msg) + return pending.pop(0) if pending else (default or "") + + monkeypatch.setattr(sys.stdin, "isatty", lambda: True) + monkeypatch.setattr(sys.stdout, "isatty", lambda: True) + args = argparse.Namespace( + json=False, + migrate_access_code=False, + scan_timeout=0.01, + printer_ip=None, + serial=None, + access_code=None, + access_code_file=None, + access_code_env=None, + model=None, + nozzle=None, + orca_slicer=None, + profiles_dir=None, + cert_fingerprint=None, + insecure_tls=False, + config=str(cfg), + ) + log = MagicMock() + real_import = builtins.__import__ + + def guarded(name, *a, **k): + if name == "zeroconf": + zc = MagicMock() + zc.Zeroconf = create_mock_zeroconf(services) + zc.ServiceBrowser = mock_service_browser(services) + return zc + return real_import(name, *a, **k) + + error = None + with ( + patch.object(common_mod, "_config_path", return_value=str(cfg)), + patch.object(wizard_mod, "_config_path", return_value=str(cfg)), + patch.object(wizard_mod, "logger", log), + patch.object(wizard_mod, "_prompt_text", side_effect=fake_prompt), + patch.object(wizard_mod, "_prompt_secret", side_effect=fake_prompt), + patch.object(wizard_mod, "_prompt_access_code_file_path", return_value=None), + patch("bambu_cli.protocols.mqtt.probe_cert_fingerprint", return_value="aa:bb:cc"), + patch("builtins.__import__", side_effect=guarded), + ): + try: + wizard_mod._cmd_setup(args) + except BambuError as exc: + error = exc + data = json.loads(cfg.read_text(encoding="utf-8")) if cfg.is_file() else None + logged = [str(c.args[0]) for c in log.method_calls if c.args] + return data, asked, logged, error + + +def test_a_lone_result_is_shown_and_asked_about(tmp_path, monkeypatch): + data, asked, logged, _ = _run_setup(tmp_path, monkeypatch, [(P1P, "192.168.1.50")], ["", "87654321"]) + assert asked[0] == "Use this printer? [Y/n]: " + assert any("192.168.1.50" in line and "BBLP-P1P" in line for line in logged) + assert data["printer_ip"] == "192.168.1.50" + + +def test_declining_a_lone_result_falls_back_to_manual_entry(tmp_path, monkeypatch): + answers = ["n", "192.168.1.77", "01P00AMANUAL", "87654321", "P1S"] + data, asked, _, _ = _run_setup(tmp_path, monkeypatch, [(P1P, "192.168.1.50")], answers) + assert asked[1].startswith("Enter Printer IP Address") + assert data["printer_ip"] == "192.168.1.77" + assert data["serial"] == "01P00AMANUAL" + + +@pytest.mark.parametrize(("ip", "why"), [("8.8.8.8", "not a local network address"), ("127.0.0.1", "this computer")]) +def test_a_flagged_address_needs_an_explicit_yes(tmp_path, monkeypatch, ip, why): + answers = ["", "192.168.1.77", "01P00AMANUAL", "87654321", "P1S"] + data, asked, logged, _ = _run_setup(tmp_path, monkeypatch, [(P1P, ip)], answers) + assert asked[0].startswith(ip) and why in asked[0] + assert asked[0].endswith("Use it anyway? [y/N]: ") + assert any(why in line for line in logged), "the address was not pointed out in the listing" + assert data["printer_ip"] == "192.168.1.77", "Enter (the default) must not accept a flagged address" + + +def test_a_flagged_address_can_still_be_chosen_on_purpose(tmp_path, monkeypatch): + data, _, _, _ = _run_setup(tmp_path, monkeypatch, [(P1P, "8.8.8.8")], ["yes", "87654321"]) + assert data["printer_ip"] == "8.8.8.8" + + +@pytest.mark.parametrize("ip", ["0.0.0.0", "224.0.0.251", "::"]) +def test_an_unusable_address_is_dropped(tmp_path, monkeypatch, ip): + data, asked, logged, error = _run_setup(tmp_path, monkeypatch, [(P1P, ip)], []) + assert data is None + assert error is not None and error.exit_code == 2 # no printers found + assert asked == [] + assert any("Skipping" in line and "cannot be a printer's address" in line for line in logged) + + +def test_choosing_a_flagged_one_of_several_asks_again(tmp_path, monkeypatch): + services = [(P1P, "192.168.1.50"), (A1, "8.8.4.4")] + answers = ["1", "n", "192.168.1.77", "01P00AMANUAL", "87654321", "P1S"] + data, asked, _, _ = _run_setup(tmp_path, monkeypatch, services, answers) + assert asked[:2] == ["Choice: ", "8.8.4.4 is not a local network address. Use it anyway? [y/N]: "] + assert data["printer_ip"] == "192.168.1.77" + + +def test_choosing_a_local_one_of_several_needs_no_second_question(tmp_path, monkeypatch): + services = [(P1P, "192.168.1.50"), (A1, "8.8.4.4")] + data, asked, _, _ = _run_setup(tmp_path, monkeypatch, services, ["0", "87654321"]) + assert asked[0] == "Choice: " + assert not any("Use" in question for question in asked) + assert data["printer_ip"] == "192.168.1.50" + + +@pytest.mark.parametrize( + ("ip", "expected"), + [ + ("192.168.1.5", None), + ("10.1.2.3", None), + ("172.20.0.9", None), + ("169.254.10.1", None), + ("fe80::1%eth0", None), + ("fd00::5", None), + ("8.8.8.8", True), + ("127.0.0.1", True), + ("::1", True), + ("0.0.0.0", False), + ("224.0.0.251", False), + ("not-an-ip", False), + ], +) +def test_discovered_address_problem(ip, expected): + problem = wizard_mod._discovered_address_problem(ip) + if expected is None: + assert problem is None + else: + assert problem is not None and problem[0] is expected diff --git a/tests/test_slice_round4_slicer.py b/tests/test_slice_round4_slicer.py new file mode 100644 index 0000000..0e88355 --- /dev/null +++ b/tests/test_slice_round4_slicer.py @@ -0,0 +1,105 @@ +"""Slicer audit round 4: numeric quality, overflow, filament choice, glob escape, failed-run output.""" + +from __future__ import annotations + +import argparse +import json +import os +import pathlib + +import pytest + +from bambu_cli.errors import BambuError +from bambu_cli.slicer import cmd_slice +from bambu_cli.slicer import options as slicer_options +from tests.bambu_test_base import settings_ctx +from tests.fakes.orca_stub import build_profiles_dir, make_orca_launcher, write_stl +from tests.test_slice_stub_integration import _slice_args, orca_env # noqa: F401 + + +def _write(path, payload): + os.makedirs(os.path.dirname(path), exist_ok=True) + with open(path, "w", encoding="utf-8") as fh: + json.dump(payload, fh) + + +# --- S1 ---------------------------------------------------------------------- + + +@pytest.mark.parametrize("quality", ["0.08", "0.10", "0.14", "0.18"]) +def test_unmapped_numeric_quality_without_profile_errors(orca_env, quality): # noqa: F811 + args = _slice_args(orca_env.model, orca_env.outdir, quality=quality) + with pytest.raises(BambuError) as exc: + orca_env("success", args=args) + assert quality in str(exc.value) + assert "0.20" in str(exc.value) # lists the available heights + assert not os.path.exists(orca_env.outpath) + + +def test_unmapped_numeric_quality_uses_matching_profile(orca_env): # noqa: F811 + _write( + os.path.join(orca_env.profiles, "process", "0.08mm Extra Fine @BBL P1P.json"), + {"type": "process", "name": "0.08mm Extra Fine @BBL P1P", "layer_height": "0.08"}, + ) + dump = os.path.join(os.path.dirname(orca_env.profiles), "dump.json") + args = _slice_args(orca_env.model, orca_env.outdir, quality="0.08") + orca_env("success", args=args, ORCA_STUB_PROFILE_DUMP=dump) + assert "0.08" in pathlib.Path(dump).read_text(encoding="utf-8") + + +# --- S2 ---------------------------------------------------------------------- + + +def test_huge_integer_temperature_is_a_validation_error(): + huge = "1" + "0" * 400 + assert slicer_options._temperature_values(f"[{huge}]") is None + args = argparse.Namespace(set_filament=[f"nozzle_temperature=[{huge}]"], set=[], settings_json=None) + problem = slicer_options._override_safety_problem(args) + assert problem and "nozzle_temperature" in problem + + +# --- S3 ---------------------------------------------------------------------- + + +def test_filament_choice_ignores_directory_order(orca_env, monkeypatch, caplog): # noqa: F811 + import logging + + fdir = os.path.join(orca_env.profiles, "filament") + for name in ("Bambu ASA-Aero @base.json", "Bambu ASA @base.json", "Bambu Support For ASA @base.json"): + _write(os.path.join(fdir, name), {"type": "filament", "name": name[:-5], "filament_type": ["ASA"]}) + real = os.listdir + + def reverse_first(path): + names = real(path) + return sorted(names) if os.path.basename(path) != "filament" else sorted(names, reverse=True) + + monkeypatch.setattr(os, "listdir", reverse_first) + args = _slice_args(orca_env.model, orca_env.outdir, filament="ASA") + with caplog.at_level(logging.INFO, logger="bambu"): + orca_env("success", args=args) + text = "\n".join(r.getMessage() for r in caplog.records) + assert "Bambu ASA," in text and "ASA-Aero," not in text + assert "matched" in text + + +# --- S4 ---------------------------------------------------------------------- + + +def test_known_keys_found_in_bracketed_profiles_dir(tmp_path): + base = tmp_path / "Orca [beta]" / "profiles" / "BBL" + _write(str(base / "machine" / "m.json"), {"min_layer_height": "0.07"}) + slicer_options._known_setting_keys.cache_clear() + assert "min_layer_height" in slicer_options._known_setting_keys(str(base), "machine") + + +# --- S5 ---------------------------------------------------------------------- + + +def test_failed_run_does_not_clobber_previous_output(orca_env): # noqa: F811 + before = b"previous good output" + with open(orca_env.outpath, "wb") as fh: + fh.write(before) + with pytest.raises(BambuError): + orca_env("fail_real_gl") + assert pathlib.Path(orca_env.outpath).read_bytes() == before + assert not [n for n in os.listdir(orca_env.outdir) if n.startswith(".plate-slice-")] diff --git a/tests/test_snapshot_sim_no_streamer.py b/tests/test_snapshot_sim_no_streamer.py new file mode 100644 index 0000000..006b796 --- /dev/null +++ b/tests/test_snapshot_sim_no_streamer.py @@ -0,0 +1,33 @@ +"""--sim must never reach the Docker streamer or read the real access code.""" + +from types import SimpleNamespace +from unittest.mock import MagicMock + +import pytest + +from bambu_cli.errors import BambuError +from tests.bambu_test_base import settings_ctx + + +def test_sim_refuses_streamer_without_touching_real_code(tmp_path): + from bambu_cli.commands import cmd_snapshot + from bambu_cli.context import get_current + + loader, run, urlopen = MagicMock(), MagicMock(), MagicMock() + args = SimpleNamespace(output=str(tmp_path / "s.jpg"), json=False, allow_camera_streamer=True, unique=False) + with settings_ctx(simulation=True, camera_allow_streamer=True): + ctx = get_current() + with pytest.raises(BambuError) as cm: + cmd_snapshot( + args, + ctx, + grab_frame=lambda printer: None, + which=lambda name: "/usr/bin/docker", + subprocess_run=run, + access_code_loader=loader, + urlopen=urlopen, + ) + assert cm.value.exit_code == 2 + loader.assert_not_called() + run.assert_not_called() + urlopen.assert_not_called() diff --git a/tests/test_textsafe.py b/tests/test_textsafe.py new file mode 100644 index 0000000..adf2861 --- /dev/null +++ b/tests/test_textsafe.py @@ -0,0 +1,33 @@ +"""display_safe strips terminal control and bidi characters from untrusted names.""" + +import pytest + +from bambu_cli.textsafe import display_safe, has_unsafe_display_chars + + +@pytest.mark.parametrize( + "raw, shown", + [ + ("\x1b]52;c;ZWNobw==\x07x.gcode", "?]52;c;ZWNobw==?x.gcode"), + ("\x1b[2Jbenchy.3mf", "?[2Jbenchy.3mf"), + ("a\nb\rc\td", "a?b?c?d"), + ("csi\x9b31m", "csi?31m"), + ("del\x7f", "del?"), + ("abc\u202egcode.exe", "abc?gcode.exe"), + ("line\u2028sep", "line?sep"), + ], +) +def test_control_and_bidi_characters_are_replaced(raw, shown): + assert has_unsafe_display_chars(raw) + assert display_safe(raw) == shown + + +@pytest.mark.parametrize("text", ["Benchy [remix].stl", "модель.stl", "部件 v2.3mf", "", "plain"]) +def test_printable_text_is_unchanged(text): + assert not has_unsafe_display_chars(text) + assert display_safe(text) == text + + +def test_none_and_non_strings(): + assert display_safe(None) == "" + assert display_safe(42) == "42" diff --git a/tests/test_tls_pinning.py b/tests/test_tls_pinning.py index cc4e161..dbfa833 100644 --- a/tests/test_tls_pinning.py +++ b/tests/test_tls_pinning.py @@ -216,3 +216,26 @@ def test_ftps_data_channel_malformed_pin_closes_socket(): ftp.ntransfercmd("STOR /model/x.3mf") data_tls.close.assert_called_once() + +def test_ftps_data_channel_wrap_failure_closes_raw_socket(): + """An error from wrap_socket itself must not leak the raw data socket.""" + ftp = ftps_mod.ImplicitFTPS() + ftp.printer = _test_printer(cert_fingerprint=_FP, insecure_tls=False) + ftp.host = "192.168.1.1" + ftp._prot_p = True + + control_tls = MagicMock(spec=ssl.SSLSocket) + control_tls.session = object() + control_ctx = MagicMock() + control_ctx.wrap_socket.side_effect = ValueError("check_hostname requires server_hostname") + control_tls.context = control_ctx + ftp.sock = control_tls + + data_raw = MagicMock() + with ( + patch.object(ftps_mod.ftplib.FTP, "ntransfercmd", return_value=(data_raw, 100)), + pytest.raises(ValueError), + ): + ftp.ntransfercmd("STOR /model/x.3mf") + + data_raw.close.assert_called_once() diff --git a/tests/test_tui_confirm.py b/tests/test_tui_confirm.py index a2af3e5..c4ea5f5 100644 --- a/tests/test_tui_confirm.py +++ b/tests/test_tui_confirm.py @@ -418,3 +418,92 @@ def test_confirm_summary_survives_a_markup_shaped_value(): text = renderable_text(_summary_table(rows)) # raises MarkupError against str cells assert "a[/b]c.gcode" in text assert "[0.98]" in text + + +# --------------------------------------------------------------------------- +# The AMS is read again when "Start print" is pressed (2026-11-05 audit, S-2): +# the slot captured while choosing the material can be minutes old by then. + + +def _changing_detector(*readings): + """A detector that reports each ``(material, slot)`` in turn, then repeats the last.""" + calls = {"n": 0} + + def detector(args, on_active_slot=None): + material, slot = readings[min(calls["n"], len(readings) - 1)] + calls["n"] += 1 + if slot is not None: + on_active_slot(slot) + return material + + detector.calls = calls + return detector + + +@pytest.mark.parametrize( + ("second", "expected"), + [ + (("PLA", 2), "now PLA in AMS0·2"), + (("PETG", 5), "now PETG in AMS1·1"), + ((None, None), "Could not confirm that PETG in AMS0·2"), + ], +) +async def test_start_print_is_refused_when_the_ams_changed(tmp_path, second, expected): + _install_ready_settings(tmp_path) + stl = _make_stl(tmp_path) + job = Recorder() + steps = GoSteps(download=Recorder(), slice=_slicer_into_workdir(tmp_path), job=job) + detector = _changing_detector(("PETG", 2), second) + + app = PlateApp(_args(), _deps(steps, ams_detector=detector)) + async with app.run_test() as pilot: + await _settle(pilot) + screen = await _prepare_to_preview(pilot, stl) + modal = await _open_modal(pilot, screen) + assert "AMS0·2 (PETG)" in _text(modal.query_one("#confirm-summary", Static)) + await _press_button(pilot, modal, "#confirm-print") + status = _text(modal.query_one("#confirm-status", Static)) + assert isinstance(pilot.app.screen, ConfirmModal) + assert modal.query_one("#confirm-upload").disabled is False + await _press_button(pilot, modal, "#confirm-cancel") + + assert job.calls == [], "the print was sent although the AMS no longer matched" + assert expected in status + assert "not started" in status + + +async def test_start_print_goes_ahead_when_the_ams_is_unchanged(tmp_path): + _install_ready_settings(tmp_path) + stl = _make_stl(tmp_path) + job = Recorder() + steps = GoSteps(download=Recorder(), slice=_slicer_into_workdir(tmp_path), job=job) + detector = _changing_detector(("PETG", 2)) + + app = PlateApp(_args(), _deps(steps, ams_detector=detector)) + async with app.run_test() as pilot: + await _settle(pilot) + screen = await _prepare_to_preview(pilot, stl) + modal = await _open_modal(pilot, screen) + await _press_button(pilot, modal, "#confirm-print") + + assert detector.calls["n"] == 2, "the AMS was not read again before printing" + assert [ns.confirm for ns in job.calls] == [True] + assert job.calls[0].ams_mapping == "2" + + +async def test_upload_only_does_not_read_the_ams_again(tmp_path): + _install_ready_settings(tmp_path) + stl = _make_stl(tmp_path) + job = Recorder() + steps = GoSteps(download=Recorder(), slice=_slicer_into_workdir(tmp_path), job=job) + detector = _changing_detector(("PETG", 2), (None, None)) + + app = PlateApp(_args(), _deps(steps, ams_detector=detector)) + async with app.run_test() as pilot: + await _settle(pilot) + screen = await _prepare_to_preview(pilot, stl) + modal = await _open_modal(pilot, screen) + await _press_button(pilot, modal, "#confirm-upload") + + assert detector.calls["n"] == 1 + assert [ns.confirm for ns in job.calls] == [False] diff --git a/tests/test_tui_terminal_escapes.py b/tests/test_tui_terminal_escapes.py new file mode 100644 index 0000000..e61d5cb --- /dev/null +++ b/tests/test_tui_terminal_escapes.py @@ -0,0 +1,35 @@ +"""Printer-supplied strings must not reach the TUI with terminal control characters.""" + +from __future__ import annotations + +from bambu_cli.tui.services import StatusSnapshot, ams_tray_rows, job_progress_lines, status_lines + +OSC52 = "\x1b]52;c;ZWNobw==\x07x.gcode" + + +def _snap() -> StatusSnapshot: + raw = {"gcode_state": "RUN\x1b[2J", "gcode_file": OSC52, "mc_percent": 5} + ams = {"units": [{"id": 0, "trays": [{"slot": 0, "type": "PLA\x1b]52;c;AA==\x07", "remain": 50}]}]} + return StatusSnapshot(ok=True, raw=raw, ams=ams) + + +def _flat(rows) -> str: + return "".join(str(v) for row in rows for v in (row.values() if isinstance(row, dict) else row)) + + +def test_status_rows_strip_escapes(): + assert "\x1b" not in _flat(status_lines(_snap())) + + +def test_job_rows_strip_escapes(): + assert "\x1b" not in _flat(job_progress_lines(_snap())) + + +def test_ams_rows_strip_escapes(): + assert "\x1b" not in _flat(ams_tray_rows(_snap())) + + +def test_error_text_strips_escapes(): + snap = StatusSnapshot(ok=False, error="boom\x1b]52;c;AA==\x07") + assert "\x1b" not in _flat(status_lines(snap)) + assert "\x1b" not in _flat(job_progress_lines(snap)) diff --git a/tests/test_upload_active_print_guard.py b/tests/test_upload_active_print_guard.py index 4151e77..d437799 100644 --- a/tests/test_upload_active_print_guard.py +++ b/tests/test_upload_active_print_guard.py @@ -15,6 +15,7 @@ from bambu_cli.constants import EXIT_PRINTER_ERROR from bambu_cli.errors import BambuError +from tests.fake_3mf import minimal_3mf class _Printer: @@ -39,7 +40,7 @@ def _upload(tmp_path, printer, name="model.3mf"): from bambu_cli.commands.files import cmd_upload path = tmp_path / name - path.write_bytes(b"PK\x03\x04 sliced") + path.write_bytes(minimal_3mf()) args = argparse.Namespace(file=str(path), dry_run=False, json=False, progress=False) return cmd_upload(args, ctx=argparse.Namespace(printer=lambda: printer)) @@ -51,6 +52,9 @@ def _upload(tmp_path, printer, name="model.3mf"): {"gcode_state": "PAUSE", "subtask_name": "model"}, {"gcode_state": "PREPARE", "gcode_file": "/sdcard/model/model.3mf"}, {"gcode_state": "RUNNING", "gcode_file": "model.3mf"}, + # Printing, but the report names no file: it may be this one. + {"gcode_state": "RUNNING", "mc_percent": 4, "bed_temper": 60, "nozzle_temper": 220}, + {"gcode_state": "PAUSE", "subtask_name": "", "gcode_file": None}, ], ) def test_refuses_to_replace_the_file_being_printed(tmp_path, status): @@ -81,3 +85,43 @@ def test_status_failure_does_not_block_the_upload(tmp_path): printer = _Printer(status_error=OSError("mqtt down")) assert _upload(tmp_path, printer) == "model.3mf" assert printer.uploads == ["/model/model.3mf"] + + +class _DeltaFirstPrinter(_Printer): + """Mid-print the first report is usually a delta with no state or file name.""" + + def status(self, timeout=None, retries=2, *, require_complete=True): + if not require_complete: + return {"mc_percent": 4, "bed_temper": 60.5} + return { + "gcode_state": "RUNNING", + "subtask_name": "model.3mf", + "mc_percent": 4, + "bed_temper": 60.5, + "nozzle_temper": 220, + } + + +def test_a_delta_first_report_does_not_let_the_upload_through(tmp_path): + printer = _DeltaFirstPrinter() + with pytest.raises(BambuError) as caught: + _upload(tmp_path, printer) + assert caught.value.exit_code == EXIT_PRINTER_ERROR + assert printer.uploads == [] + + +def test_file_listing_does_not_pass_terminal_escapes(capsys, monkeypatch): + from bambu_cli import printer as printer_mod + from bambu_cli.commands.files import cmd_files + + class _ListingPrinter: + simulation_mode = False + + def list_files(self, *args, **kwargs): + return ["\x1b]52;c;ZWNobw==\x07evil.3mf", "\x1b[2Jcube.3mf"] + + # cmd_files builds its printer through get_printer(), as tests/test_cmd_files.py patches it. + monkeypatch.setattr(printer_mod, "get_printer", lambda *a, **k: _ListingPrinter()) + cmd_files(argparse.Namespace(json=False, path=None), ctx=argparse.Namespace()) + captured = capsys.readouterr() + assert "\x1b" not in captured.out + captured.err diff --git a/tests/test_wizard_guided.py b/tests/test_wizard_guided.py index 58064e7..1825831 100644 --- a/tests/test_wizard_guided.py +++ b/tests/test_wizard_guided.py @@ -184,6 +184,7 @@ def test_guided_setup_mdns_one_printer(tmp_path, monkeypatch): cfg = tmp_path / "config.json" answers = iter( [ + "", # use this printer? (default yes) "87654321", # access code "", # confirm model "", # confirm nozzle