Repository navigation
fix: audit round 3 -- printer safety, slicing correctness, setup - #142
Merged
Merged
Conversation
…-HOME first run README: move "Print something" (step 1 = install OrcaSlicer) above "Try it in 30 seconds", retitle the nav link so no "30 seconds" precedes the first OrcaSlicer mention, and say the sim needs neither a printer nor OrcaSlicer while a real print needs both. Anchors unchanged. Tests (tests/test_first_run_path.py, section 6) lock the empty-HOME contract a newcomer hits, so the walk is deterministic without a printer: - `plate preflight` exits 1 and reports config, orca-slicer, and profiles-dir as three separate checks (human and --json), each with its own next step. - `plate setup --sim` with a non-TTY stdin exits 1 with the headless message naming --printer-ip/--serial/--access-code-file, no traceback. - `plate --sim status` with an empty HOME still exits 0 and reports IDLE. No CLI behaviour changed; no protocol code touched.
The project was credited to "platecli contributors", so the PyPI author field, the wheel/sdist METADATA, and the copyright line carried no real name. Replace it with the actual author and a real contact address. - pyproject.toml: authors and maintainers become Dylan Reed with the project's public contact address. PyPI renders it on the project page, which is the intent -- someone evaluating the project can reach the author without going through GitHub. - LICENSE: copyright holder is now a named person, which is what makes the MIT grant unambiguous. - tests/package_contents_smoke.py: the LICENSE snippet assertion follows. tests/privacy_smoke.py gains a PUBLISHED_AUTHOR_IDENTIFIERS allowlist holding exactly the published contact identity. That gate flags every email address in the tree and in the release archives, so without an entry it would red CI on the one address that is supposed to be there. The allowlist is span-based rather than exact-match so a substring hit -- a local-account-name pattern derived from a contributor's git config can match inside the published address or name -- is exempt too. The exception is deliberately narrow, and unit-verified as such: a personal address, a GitHub noreply address, an arbitrary third-party address, and a lookalike suffix (the published address followed by another domain) all still fail the gate. Only the published address passes. Verified: twine check passes both artifacts; a byte-level scan of all 344 wheel + sdist members finds the published address in 6 metadata/README members and no other address anywhere; privacy_smoke --include-dist reports zero findings against dist/. pytest -m "not live" 1535 passed at 91.1% coverage; ruff check, ruff format, mypy -p bambu_cli, bandit, package_contents_smoke, ci_workflow_smoke, python_compat_smoke clean.
Someone evaluating the project could not tell who wrote it or how well it is tested without reading the CI config. Both are now above the fold. - Tests (1,535 passing) and coverage (91%) badges next to the existing CI and PyPI ones, linked to a new "Tested and hardened" section. They are static shields.io badges -- no coverage service is wired up -- so an HTML comment records the command to re-measure them at release time. - "Who it's for": four concrete audiences (terminal-first makers, homelab and print-farm users, agents/automation, locked-down networks) after the pipeline diagram, so the reader self-selects before the spec details. - "Tested and hardened" section: the measured test and coverage numbers against the 86% CI floor, the OS/Python matrix, the blocking gate list, the mutation-testing scope, and the wheel/sdist install smokes. Notes that the coverage config does not exclude error paths, so the figure is not inflated. - Byline under the nav line, an Author section carrying the contact address, and "MIT (c) 2026 Dylan Reed" on the licence line. Project questions are still routed to issues, discussions and SECURITY.md so the mailbox is for work enquiries rather than support triage. - "Support & expectations" now names the maintainer instead of saying "one person". Numbers are measured on this tree at v0.5.1, not estimated: pytest -m "not live" reports 1535 passed and TOTAL 91.1%.
…ast-fail on auth errors
…inject printer context
… bounds build_safe_opener assembled an OpenerDirector by hand and never added HTTPErrorProcessor. That processor is what routes a non-2xx response into parent.error(), so without it the opener returned 3xx/4xx/5xx bodies as successes: redirects were never followed, HTTPError was never raised, and the redirect hop cap plus the 429/503 retry in polite_open were unreachable code. Any download behind a redirect failed with "Downloaded file is empty", and a 404 was misreported as "HTML page did not contain a direct model file link" with no http_status. Latent since the netsafety split. Raising on the hop cap then exposed a second leak: HTTPRedirectHandler's http_error_302 calls redirect_request before its own fp.read()/fp.close(), so bailing out there left the intermediate response's socket to the collector. redirect_request now closes fp itself. Other fixes: - ams: tray_now 254 (external spool) and 255 (nothing loaded) are distinct states. Treating both as active put a false active filament in status --json and a "play" marker next to the external spool on an idle printer. Both sentinels are now named constants. - cli: the printer_ip hostname check ran _resolve_ip and then a second, timeout-less socket.getaddrinfo, reintroducing the unbounded lookup the timeout exists to prevent. New utils._try_resolve_ip returns None on failure or timeout so the caller can branch without a second lookup; in-flight resolutions are shared per host, so a blackholed resolver parks one thread rather than one per call. - download: the preserved multi-model archive name skipped _sanitize_download_filename, so a long URL path failed os.replace with ENAMETOOLONG into a swallowed except and stranded the archive under its hidden temp name. Windows reserved stems and control characters got through too. The swallowed OSError now logs. - check_layers: relative imports were skipped as "same unit by construction", which only holds at level=1 in a top-level module. From a subpackage, "from ..protocols.ftps import x" crossed units unseen - the exact breach the checker exists to catch. They are now resolved to absolute paths. Verified against an injected violation. - netsafety: _throttle_host clamped its sleep to one interval, so queued callers under-slept and burst past the throttle; the bound is now MAX_RETRY_AFTER_WAIT. - slicer: an oversized slice_info.config now logs when it falls back to the gcode header instead of degrading silently. Tests: new tests/test_netsafety_opener.py drives a live local server rather than calling handlers directly - the existing redirect tests all invoked redirect_request() by hand or asserted handler registration, both of which pass against a chain that can never invoke the handler. Confirmed the core cases fail with the fix reverted. Also covers per-hop SSRF re-validation, which was untestable while redirects were inert. The fd-counting variant of the socket-leak test was written and then removed: wrap_socket detaches the socket and refcounting closes the fd as the exception propagates, so it passed against a deliberately reintroduced leak. The explicit close assertion is what catches that regression; the file now says so. Note: a 404 download now exits 2 (EXIT_NETWORK_ERROR) instead of 3. That is what the handler always intended, but it is visible in --json. 1652 passed, 92.69% coverage; ruff, mypy, bandit, check_layers, gen_schemas and all smokes green.
…resolver, see through from-imports Follow-up to b25607b from a second adversarial pass. Each production change below had a mutation that left the suite green; each now has a test that goes red against that mutation. netsafety: urllib copies every request header onto a redirected request whatever host it names, so an Authorization or Cookie header sent to host A followed a 302 to host B. Nothing in platecli sends credentials today; the redirect handler now drops them on a host change so the first caller that does cannot leak a token, and re-picks the User-Agent for the new host (the policy is per host: honest token for first-party API hosts, browser-shaped elsewhere). utils._try_resolve_ip: a failed Thread.start() left an in-flight entry whose done event was never set, so every later call for that host waited its full timeout and returned None for the rest of the process -- surfacing as a bogus "Invalid printer_ip or hostname". The start is now guarded. The resolver cache also gets a 300 s TTL: the resolved IP is handed to paho and ftplib as a literal (they never re-resolve, contrary to what the old docstring claimed), so a permanent cache pinned a long-lived TUI session to a stale DHCP lease. check_layers: a from-import names a base and a list of names, and a name can be a submodule. Judging the base alone let `from bambu_cli import protocols`, `from .. import protocols` and `from bambu_cli.printables import client` (the sealed module) pass the checker. import_targets now yields base.name for every name; the checker still passes on the real tree. Tests added for the untested lines found by mutation: the throttle sleeping the full backlog rather than one interval, the post-redirect URL re-validation in the downloader, the success half of the hostname check through main(), and the oversized slice_info fallback log. test_jsonio adjusted for the cache tuple. 1670 passed, 92.72% coverage; ruff, ruff format, mypy, bandit, check_layers, gen_schemas and all smokes green.
The reserved-range check refused all of 64:ff9b::/96, breaking every download on DNS64 (IPv6-only) networks. Unwrap the embedded IPv4 instead so public hosts pass and private ones are refused. Also reject fec0::/10, which stdlib ipaddress reports as global. Changelog wording for the FTP path check corrected to defense in depth.
An unrecognised model name (including the A1 mini's own product name, "A1 mini") and a missing machine profile both fell back to the P1P profile, a 256 mm bed, without a warning. Slicing now stops with a config error that names the model and the supported list; "A1 mini" and "X1 Carbon" resolve to A1M and X1C. The process-profile search no longer borrows another model's profile. setup no longer turns an unknown model into P1P: the prompt re-asks and has no default when discovery found none, and non-interactive setup requires --model. config validate and preflight gain a printer-model check.
…limits The 0-350 C nozzle / 0-150 C bed check parsed override values as JSON only, so OrcaSlicer's own comma-list form got through: nozzle_temperature=400,220 produced M109 S400 in the real G-code. Temperature values are now split the way OrcaSlicer reads them, and any value that is not a plain number is refused. Chamber, vitrification and range keys are bounded too. G-code and script settings (filament_start_gcode, machine_start_gcode, post_process, ...) and printer (machine) settings such as printable_area, which OrcaSlicer honoured when sent through --set, are refused outright. slice --list-settings no longer lists the G-code keys.
The retry loop gave every attempt a fresh "published" flag, so a PUBACK that arrived after the timeout made gcode/stop/pause/resume/light publish the same command up to three times and then report sent=false. Retries now only happen while nothing has gone out. A command that was published but not acknowledged raises CommandUnconfirmed: exit 6, failed_step mqtt, sent=true, acknowledged=false, next_command status --json. The TUI's held session reset its once-flag per attempt too and gets the same fix.
--nozzle-temp/--bed-temp defaulted to 220/60 and overwrote every filament, so PETG and ABS printed at PLA temperatures. Removing that exposed why it was there: the process and filament temp profiles kept an inherits reference that OrcaSlicer cannot resolve from a file outside its profile tree, so every inherited value fell back to a generic default (PETG sliced as filament_type PLA at 200 C; acceleration 500 instead of 10000). And the printer's default_bed_type was never applied, so slices targeted the Cool Plate, where Bambu's PETG/ABS profiles set 0 C. Both profiles are now flattened like the machine profile, the temperature flags default to None (the filament profile's value), and curr_bed_type comes from the printer definition. Verified with real OrcaSlicer: PETG, PLA and ABS slice at 255/70, 220/55 and 270/90 C on the Textured PEI plate.
camera_allow_streamer was read with bool(), so "camera_allow_streamer": "false" in a hand-edited config turned ON the Docker streamer that ignores cert_fingerprint. Camera switches now use a strict reader like insecure_tls: JSON booleans and true/false spellings are honoured, null means unset, and an unreadable value falls to the safe side (streamer off, camera_direct_only on) with a warning.
…nter
A refused MQTT connection (wrong access code, rc=5) set the terminal event
and returned: exit 0, no NDJSON, indistinguishable from success. A socket or
TLS error escaped as a raw OSError ("Unexpected error", exit 5), and a broker
that never sent CONNACK kept the monitor waiting forever. All three now raise
PrinterConnectionError: exit 2, failed_step mqtt. The connect wait is bounded
by the printer's MQTT timeout; a report message also counts as connected.
…work error A download the SSRF guard blocked (http://localhost/..., a LAN host) exited 2 with a nested "<urlopen error <urlopen error Could not connect ...>>" and no hint, which reads as a transient failure. The branch meant to catch it matched "Security Error", text that only reached a log line; its two tests injected that text, so they passed while production never did. _get_safe_connection now raises BlockedAddressError when every resolved address was refused. The downloader maps it to exit 5, failed_step validate, a message naming the addresses, --allow-private-ips when that would help (not for multicast), and blocked_addresses in the envelope. The old tests inject the real error; new tests drive the real opener against a local server.
… failure zipfile raises zlib.error (or lzma.LZMAError / EOFError) for damaged compressed data behind intact headers, and none of those is a BadZipFile. They escaped _extract_zip_model, so job and download ended in "Unexpected error" with a traceback (exit 5) and job lost its structured summary. They now map to ValueError like every other bad archive: exit 3, failed_step extract.
--network/slicer/command/upload/scan-timeout accepted -5, 0, nan and inf: with nan or inf a slice could never time out, and negative values failed deep inside a socket call. They now go through a positive_seconds argparse type (exit 5 on a bad value). A bad *_timeout in config.json was passed to float() before any command handler ran, so a typo crashed with a traceback; it is now ignored with a warning, and config validate reports it as a timeouts error.
The print payload always named Metadata/plate_1.gcode, so a pre-sliced 3MF whose only sliced plate was another one failed on the printer, and a 3MF with no sliced plate at all was uploaded and sent to print. job/send now read the sliced plates: plate 1, or the only sliced plate, is printed (with a warning when there are several), and an unsliced 3MF is refused before upload (exit 3). print, job and send take --plate N; the JSON reports plate and a next_command carries a non-default plate. Estimates for a multi-plate 3MF are for the printed plate instead of the last one. Printer-ready test fixtures now write real sliced 3MF packages (with fixed timestamps) instead of placeholder text the new check rightly refuses.
OrcaSlicer writes plate_1.gcode and result.json next to its export. Its --outputdir was the user's output folder, by default the model's own, so every slice left both behind and overwrote same-named files there. OrcaSlicer now writes into a private temp directory inside the output folder (same filesystem); only the 3MF it actually produced is moved to the output path, so the stale-output check still works, and the directory is always removed. The fake OrcaSlicer now writes the same side files as the real one.
With both access_code and access_code_file set, migration removed the inline key without looking at the file. A missing file (or one holding a placeholder) then left the config with no usable access code. A missing file now receives the inline code; an unreadable or placeholder file keeps the inline code and the migration reports an error (exit 1). A valid file still wins over a stale inline copy.
The streamer-path JSON hardcoded docker_container "bambu_camera" regardless of camera_container_name, so it named the wrong container whenever the name was configured.
uv.lock still pinned platecli 0.5.1 after main's dev bump, so uv lock --check failed and every uv run rewrote the lock locally.
upload_file deletes /model/<name> before storing the new file, so re-running job on the same model mid-print pulled the file out from under the running print (what the firmware then does is unverified on hardware). cmd_upload now asks the printer for its current job first and refuses (exit 4) when an active print (PREPARE/RUNNING/PAUSE/SLICING) is using the same file; a status query that fails does not block the upload. Upload unit tests answer the status probe locally instead of attempting a real MQTT connection.
…unverified print sends every file with project_file, which names a plate inside a 3MF; a plain .gcode has none. The community protocol notes list a separate gcode_file command, but the only first-hand report found could not get it to start a custom file. Neither path is confirmed on hardware, so print now warns before sending a plain .gcode and the docs say so. docs/live-printer-smoke.md gains an open-questions list for the hardware checks this work could not run: plain .gcode printing, firmware behaviour on a 3MF sliced for another model, and deleting the file of a running print.
OrcaSlicer refuses relative extrusion unless a layer-change hook resets the extruder (G92 E0). Bambu's X1/P1 profiles include it; the A1 and A1 mini profiles rely on an exemption the OrcaSlicer CLI does not apply, so every A1/A1 mini slice failed (reproduced on the base commit; even the untouched vendor profile fails). The temp machine profile now appends G92 E0 to the layer-change G-code when no reset is present and relative E is on. Verified with real OrcaSlicer: A1 mini, A1, P1S and X1C all slice; the A1 mini output targets its 180 mm bed.
The manual and troubleshooting guide tell users to run plate setup --printer-ip <new-ip>, --access-code-env CODE, --profiles-dir, --orca-slicer or --cert-fingerprint to change one value. None of that worked: partial non-interactive setup failed on missing required values, and the path and pin flags were not setup arguments at all, so they started the interactive wizard and were ignored. With a config present and at least one setup flag given, values not passed are now kept. The address, model, nozzle, access code and pin are inherited only when --serial is unchanged, so a second printer never inherits another printer's pin or code. A new code for the file the printer already uses is a rotation and needs no --force.
…ing match P1S, X1 and X1E have no process profiles of their own and borrow the X1C ones. Discovery took the first file in directory order whose name contained the model code, which with the real OrcaSlicer profiles was "0.20mm Bambu Support W @bbl X1C" -- a support-material profile -- for every standard slice; "X1" also matched X1C/X1E files and "A1" A1 mini ones. A profile now belongs to the model only by an exact "@bbl <model>" name or a compatible_printers entry, candidates are sorted, and the requested quality word wins. A real P1S slice now uses 0.20mm Standard @bbl X1C.
The range check parsed override values with float(), which also accepts 3_5_0, 1e2 and non-ASCII digits. OrcaSlicer reads those differently (3_5_0 and 1e2 slice at 3 C and 1 C), so the checked and the sliced temperature could differ. Temperature tokens must now be plain ASCII decimals, and a bare scalar is checked as the raw text OrcaSlicer receives rather than json's reading of it. Found in review.
send_command decided on the already-expired wait() result, so a PUBACK that arrived during loop_stop() was discarded and an acknowledged command was reported as unacknowledged (exit 6). Both the one-shot and the held-session path now re-check the event before deciding. Still never re-sends. Found in review.
The strict reader honoured the strings "0"/"1" but treated the numbers 0/1 as unreadable, so "camera_allow_streamer": 1 silently stayed off and "camera_direct_only": 0 turned on. Found in review.
The real-profile check named an absolute path under a local home directory, which the privacy smoke rightly rejects. It now asks detect_profiles_dir(), the same lookup plate setup uses, and skips when no install is present.
This was referenced Oct 6, 2026
DLANSAMA
added a commit
that referenced
this pull request
Oct 10, 2026
…history notes (#147) * docs(agents): drop a dead module row, a wrong PR citation and change-history notes - Remove the `printables.py` row from the module table. The module was replaced by the `printables/` package in #103, which already has its own row. - Coverage gate row: drop the "PR #123" citation (that PR was closed unmerged; the 90% floor landed in #142) and the dated measurement snapshot. - Add `contracts` to the rank-10 row of the layer diagram so it matches `RANKS` in scripts/check_layers.py. - State the camera, MQTT and camera-port notes as current facts rather than as changes ("moved here", "was split", "now"), and stop listing completed B.4/B.5 work under known architecture debt. * docs: point stale references at the printables package and the merged PR - netsafety module docstring named `printables.py`, which became the `printables/` package in #103. - The test-backlog snapshot heading cited PR #123, which was closed unmerged; the work landed in #142. Drop the PR reference and keep the date.
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
1 participant
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.
Fixes every finding from the third audit, plus the problems found while fixing them. Each fix is its own commit with a test that failed before it. The branch is built on #140's head with
mainmerged in, so it includes #140 and supersedes it.Printer safety
setupneeds a real model andconfig validatechecks it.--set/--set-filamentcould get around the 0–350 °C nozzle / 0–150 °C bed limits (nozzle_temperature=400,220producedM109 S400in real G-code), and could inject G-code or change machine settings such asprintable_area. All of that is refused now.gcode/stop/pause/resume/lightre-published on a late PUBACK (up to 3×) and then reportedsent: false. A command is now published once. Unacknowledged → exit 6 withsent: true, acknowledged: false.uploadrefuses to replace the file the printer is currently printing.Slicing correctness (verified with real OrcaSlicer)
filament_type = PLAat 200 °C, acceleration 500). The 220/60 temperature defaults overwrote every filament, and the printer's Textured PEI default plate was never applied. PETG/PLA/ABS now slice at 255/70, 220/55 and 270/90 °C. Speeds and time estimates change to Bambu's defaults.Bambu Support Wprocess profile. They now get0.20mm Standard @BBL X1C.plate_1.gcode/result.jsonnext to the model.--plateonprint/job/send.jobprints the sliced plate and refuses an unsliced 3MF before upload.Errors and contracts
status --monitor(04476fd): now fails with exit 2 on a refused, errored or silent connection. It used to exit 0.blocked_addresses), not a retryable network error. The old handler and its tests matched text that production never produced."camera_allow_streamer": "false"no longer enables the unpinned streamer.plate setup --<one value>now updates an existing config, as the docs describe. Pin and code carry over only for the same serial.uv.lockis current with0.6.0.dev0.Needs hardware (documented, not verified)
Listed in
docs/live-printer-smoke.md:.gcodefiles start over LAN (printnow warns)Verification
-W error::ResourceWarning.--check,uv lock --checkand the CI grep guards all pass.--include-diston a fresh build.