Repository navigation
fix: audit round 4 — at-most-once printer commands, pin precedence, heater limits, terminal-safe output - #153
Merged
Merged
Conversation
…take effect upload_file bound sleep=time.sleep when the module was imported, so tests that patch time.sleep still slept through real backoff delays (about 48 s per run across three upload tests).
…models, clean up on Ctrl-C - A password holding /, ? or # (user:pa/ss@host) parses as host 'user' and port 'pa', so redaction found no userinfo and the secret was echoed in the JSON envelope and in http.client's 'nonnumeric port' error. Validation now refuses a non-numeric port without echoing it, and the redactor scrubs through the last @ when the port is invalid. - An unbalanced IPv6 bracket made urlparse raise out of validation and the redactor; it is now an invalid-URL refusal (exit 5), not a traceback. - A URL that names a model file is no longer refused for text/plain: ASCII STL, OBJ, STEP and G-code are text, and GitHub raw serves them that way. - Raw Unicode in the URL path or query is percent-encoded before the request line is built, instead of failing with an ASCII codec error. - Ctrl-C mid-download or mid-extraction removes the .part file and the 0-byte placeholder before propagating. - A Printables 'ok' response whose link is not a string is a contract change, not a success that crashes the downloader.
Rich and Textual stop markup injection but pass ESC through, so a printer file name or MQTT field holding an OSC 52 sequence can write the clipboard. display_safe replaces C0/C1 controls, DEL and bidi-format characters.
…t starts paho's network loop reconnected after a dropped connection and resent the unacknowledged QoS-1 PUBLISH, so a command that reached the printer just before the drop ran twice while send_command reported success. Automatic reconnect is off; the existing retry loop owns reconnection and never resends a published command. A print start that was published but never acknowledged now reports printed: null with sent: true on the raised error, so the --json envelope and the job summary carry it (they read the exception, not the recorded detail). Malformed reports (deep nesting, over-long integers, non-object JSON) are dropped by decode_report instead of raising out of on_message and killing paho's network thread.
With both set, MQTT and the camera took the insecure branch and skipped the pin. The pin is now checked first everywhere, and snapshot's SSL error path keys on the pin too. Under --sim, snapshot refuses the Docker streamer. A failed FTPS data-channel wrap closes the raw socket for errors raised before ssl takes ownership of the descriptor.
…ng, full-report upload guard plate gcode sent M104/M109/M140/M190 at any temperature; values above the slicer limits (350/150 C) or not plain decimals are refused before sending. --ams-mapping accepted Unicode digits that int() reads. The active-print upload guard now requires a complete status report, so a delta-first report without gcode_file cannot read as 'not printing this file'. The FTP listing is shown through display_safe.
…ted names Printer, MQTT, mDNS and prompt text reached the terminal with ESC sequences and bidi overrides intact. TUI fields, interactive prompts (markup off), the setup wizard's mDNS list and kept-file notes go through display_safe. Remote names with control or bidi characters are refused for upload, and download renaming replaces them so a downloaded file can still be uploaded.
…good output on failure An unmapped numeric --quality must resolve to a process profile of that height (the error lists available heights) instead of slicing Standard. Filament profile ranking is exact stem, then shortest, then name, and is logged. Profile globs are escaped. An overflowing temperature override is a validation error. A failed OrcaSlicer run no longer replaces earlier output.
…viations Wrong-typed config values (non-string access_code, cert_fingerprint or printer_ip; non-object config.json) give a config error or a clean fallback instead of a traceback. camera_port is validated and normalized; preflight gains cert-fingerprint and camera-port checks. Timeouts are capped at 24 h. allow_abbrev is off so --conf cannot stand in for the literal --confirm gate.
… stray doctor file emit_json maps non-finite floats to null and dumps with allow_nan=False, and sets the emitted flag only after serialization succeeds. AMS parsing handles OverflowError and non-finite values and skips non-list trays. doctor creates its temporary file only right before writing it.
One ordered plan any session can follow, with the open decisions, the release and launch steps, and the three safety gaps the audit left open (pre-sliced 3MF heater limits, TUI AMS slot, lone mDNS result). Findings judged not worth fixing now are one line each in docs/plan/inbox.md. docs/plans/ stays as the finished design records.
- status --monitor hung after a dropped connection: it relied on paho's auto-reconnect, which the at-most-once fix turned off. The monitor opts back in (its only publish is a QoS-0 pushall, never resent), with a real-paho test against a broker that drops once. Client IDs are never empty, since paho's fallback for a broker rejecting one is a reconnect. - plate gcode: a T word reset the heater context, so M104 T0 S400 passed. - Upload guard: an active print whose report names no file now blocks. - status, status --monitor: human lines go through display_safe. - int() of 1e999 from a report raised OverflowError in the TUI and the monitor's event builder (a terminal event was then never emitted); every int() of outside data now catches it. - decode_report drops reports nested past 32 levels; json.loads accepts a few thousand, the recursive emit/redact walks do not. - Redaction and validation: user:12/pw@host parses as host:port with the password in the path; it is redacted and refused. Host-less forms (https:///user:pw@host) are redacted. Windows drive paths are untouched. - Inbox: chamber/autotemp limits, comment false positive, multi-delimiter passwords, files' exit code for a non-string pin, doctor's json.dump.
…tree files The new redaction tests held literal user:pw@host and address-shaped strings, which the privacy smoke rejects in CI. Locally it passed because, inside .claude/worktrees/, every absolute path contains .claude and the excluded-dir check skipped every file. It now matches on the repo-relative path (318 files scanned instead of 0) and flags the old test file.
plate gcode refused M104 S400, but a downloaded or third-party sliced 3MF carrying the same line went straight to the SD card through upload, job and send. cmd_upload (and job's printer-ready dry run) now scan every .gcode member of a 3MF, or a plain .gcode file, with the same check, moved to bambu_cli/gcode_limits.py (rank 10). Only lines whose command is a heater command are parsed, and comments are dropped, so slicer prose does not trip it; Bambu's own start G-code forms (S<n>, S<n> H<n>) pass. A member that cannot be decompressed fails closed. 2M lines scan in about 0.5 s. Plan: S-1 done.
plate go and the TUI read the loaded filament once, when the material is chosen, and fed that slot at print time. Slicing can take minutes; a spool swapped in between was fed at the temperatures sliced for the old one. Start print now reads the AMS again and refuses, with nothing sent, when the material or slot changed or the read fails. Upload only is not checked. The confirm summary gains a Feed row and a Temperatures row with the hottest nozzle and bed targets in the file's G-code.
Discovery is unauthenticated and the access code goes to the configured address, yet a lone result was taken without a question and its address was never checked. A lone result is now shown and confirmed; no falls back to manual entry. Unspecified and multicast addresses are skipped. A public or loopback address is flagged and needs an explicit yes, also when it is picked from several.
upload now reads the file to check its heater targets, and these tests mocked os.path.exists for a test.gcode that was never on disk, so the scan failed closed. They now run in a temp dir holding a real one.
- every archive entry is scanned (infolist): open(name) returns only the last entry of a duplicated name, so a hot first copy went unread; - CR and CRLF end a line as LF does; a CR-only file was one long line whose heater commands were never seen; - files are read in 1 MiB chunks and a line over 1 MiB is refused, so a member with no line breaks cannot exhaust memory; - a 3MF that cannot be opened is refused instead of uploaded unchecked; - job --dry-run of a ZIP scans its .gcode/.3mf member too; - the wizard's preview widens its label column for Temperatures. Tests that uploaded placeholder bytes as a .3mf now use a real one (tests/fake_3mf.py). Docs carry the measured counts: 2217 passing, 93.5% over 9446 statements.
Merged
DLANSAMA
added a commit
that referenced
this pull request
Nov 6, 2026
* chore: release 0.6.0 Set the version to 0.6.0 and move the Unreleased changelog entries under 0.6.0 (2026-11-06). Refresh the README test and coverage figures to measured values (2,217 tests, 93.5%, 90% floor). Retire plan box D-1 (PR #153 merged) and fold L-3 into L-2. * docs(plan): note the unpushed coverage tests in the inbox
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Round 4 of the security and robustness audit. Every fix has a test that failed before it. Two independent reviews of the first wave found eight more problems, and a third review of the safety-gap commits found six; all are fixed here.
Printer safety
gcode/stop/lightthat reached the printer just before the connection died ran twice while reporting success. Reconnect is off for commands (shown with real paho against a local broker that hangs up on PUBLISH: 3 deliveries before, 1 after).status --monitorkeeps auto-reconnect; it only publishes a QoS-0 pushall. Client IDs are never empty, so nothing relies on paho's reconnect-based fallback for a broker that rejects them.printed: null,sent: trueon the raised error, soprint --jsonand thejobsummary carry it (both read the exception, not the side-channel record).job_errorschema:printedis now nullable.M104/M109> 350 °C,M140/M190> 150 °C, or a non-plain number are refused before sending, includingM104 T0 S400.upload,job(and its--dry-run, ZIP members included) andsendscan every G-code entry of a 3MF, or a plain.gcode, before upload. CR/LF/CRLF all end a line, duplicate entry names are all read, reading is bounded (a line over 1 MiB is refused), and anything that cannot be opened or read is refused. Checked against Bambu Studio's own start G-code forms (M104 S220,M109 S25 H140,M104 T1 S230): none are refused.plate goand the TUI): the slot was captured when the material was chosen, minutes before printing. A changed or unreadable AMS now stops the print with nothing sent. The confirm summary gains Feed (AMS0·2 (PETG)/ External spool) and Temperatures rows.0.0.0.0/multicast answers are skipped; a public or loopback address needs an explicit yes.--ams-mappingis ASCII digits only. The upload guard waits for a complete report and blocks when the printer is printing a file it does not name.--quality 0.28needs a 0.28 mm profile; a failed run no longer replaces earlier output; deterministic filament choice.Security
cert_fingerprintalways wins overinsecure_tls(MQTT, camera, snapshot).status/monitor/TUI, mDNS names, prompts) has control and bidi characters replaced (bambu_cli/textsafe.py); remote names with them are refused for upload.1e999) are dropped instead of killing paho's network thread./ ? #are redacted, including the digit-prefixed form that parses ashost:port, which is also refused (it would send the password as a path to hostuser).--confcan no longer stand in for--confirm.Robustness
JSON never carries NaN/Infinity; wrong-typed config values give config errors; timeouts are capped at 24 h; Ctrl-C cleans partial downloads; non-ASCII URLs work;
text/plainmodel files are accepted when the URL names one; the FTPS data socket is closed when its TLS wrap fails.Plan
docs/MASTER-PLAN.mdanddocs/plan/inbox.mdare new;docs/plan/done.mdrecords S-1 to S-3, the three safety gaps this branch closes. Lesser findings are inbox lines.Verification
ruff check, ruff format, mypy, bandit, check_layers, gen_schemas --check, and the ci_workflow, syntax, cli_help, privacy, python_compat and agent_cli smokes all pass locally. The full suite passes with
-W error::ResourceWarning: 2217 passed, 93.5% coverage. The Windows and macOS legs and pip-audit only run in CI. Nothing was run against a real printer.