NetBelt 1.3.0: review fixes and live console baud change by gomanizm · Pull Request #4 · gomanizm/NetBelt

gomanizm · 2026-09-11T16:33:25Z

NetBelt 1.3.0: the whole-codebase review fixes, and a console baud change that applies at once.

1.3.0 reached main at the end of August but was never tagged, so users are
still on 1.2.0. This merges everything since and releases it as 1.3.0. The
CHANGELOG folds the unpublished 1.3.0 section into this one, because the
release body carries only the current version's section.

A console baud change applies to the open tab

After speed 115200 on a Cisco console the device switches at once, but
changing the rate in the device tree only updated the tree and the reconnect
copy. The open tab stayed at the old rate and printed garbage until it was
closed and connected again.

pyserial reconfigures an open port when its baudrate is assigned
(SetCommState on the live handle), so the change is applied in place. The
port stays open, the reader and writer threads keep running, and the tab
keeps its scrollback. A rate the adapter refuses leaves the session at its
old rate and says so on the status bar, instead of dropping a connection
that still works. A change made while the port is still opening is applied
once it opens.

Not run on hardware: there is no COM port on the build machine. It was
measured with a loopback port while the reader and writer threads ran
(2000 switches, 2000 lines back in order, no errors), and two independent
inspectors confirmed that reverting the source makes the new tests fail.

Review fixes

Third review (104 findings): all addressed, as described in the
earlier revision of this pull request and in the CHANGELOG.
Fourth review (75 findings from Codex): each was measured in a
separate context before anything changed. 76 were real (one was refuted;
two defects found while refuting it were added, and a few batches
overlapped). Measured severity: 4 high, 46 medium, 28 low, all fixed.
Two of the high ones had been introduced by the third review's own
fixes: the SFTP server could delete or rename a file outside its root
through a drive-relative name (C:name), and an SFTP upload whose
rename reply timed out removed both the final file and the transferred
copy.
Inspection of those fixes: nine inspectors reverted each fix's
source in a scratch worktree and checked that its test fails. 77 of 79
both held and had a test that caught the revert; one left the same
defect reachable from a neighbouring branch, and one test did not fail
when its fix alone was reverted, because a later fix covered the same
symptom. They measured 24 gaps in all, mostly the same defect still
reachable from a neighbouring path. All 24 are closed.
Fifth review, partial: Codex hit its usage limit after one of eight
batches (update and build). Its 6 findings were all real and are fixed;
the high one was a pending-update ZIP deleted by another process during
the startup scan, which stopped the application from starting at all.
The remaining seven batches will go into a later patch release.

A regression found by the full run is fixed here too: build.bat had no
code page handling, so its new Japanese comments were read as CP932, lost
the reader's place, and ran as commands, turning a successful build into
exit code 1. It now settles the code page and reads itself again, the way
updater.bat does.

Checks

Local full suite, before the two runner fixes below: 1846 passed,
2 skipped, 0 failed. The two skips are the symbolic-link privilege and
paramiko's missing Ed25519 generator.
The release workflow, run by hand on this branch without a tag, passed
every step: tests (1847 passed, 1 skipped on the runner), version
check, third-party notices, the exe build, the ZIP and its checksum,
and the release body. Nothing was published.
No dependency, spec or workflow changes against main.

The first of those runs failed, and it caught two tests that could not
pass on the runner. Both were test-only faults and are fixed here:

The runner's TEMP is an 8.3 short path (C:\Users\RUNNER~1\...). The
SFTP server normalises its root with realpath, so two tests that spied
on os.lstat compared the long path against the short one and never
matched.
The runner's locale is English (cp1252). A test that starts NetBelt in
a child process read its output as UTF-8 but let the child write in
the code page, so the product's Japanese log line raised
UnicodeEncodeError before the test could check anything. Running from
source with redirected output on a non-Japanese locale has failed this
way since 1.2.0; the exe has no console output.

Both were reproduced locally before fixing, and reverting the product
fixes they guard still fails them. The same first run also saw pytest
stop at about half way without a summary. That did not reproduce
locally under Python 3.11, a short TEMP, cp1252 output, or the runner's
drive layout, and the second run went past it cleanly, so its cause is
still unknown.

🤖 Generated with Claude Code

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

p2b/conn and p2b/main both rewrote _quarantine_invalid_devices: conn to rescue groups with no name (and items that are not groups at all), main to drop devices whose name collides with the home tab. Both are wanted, so the two loops are folded into one and the warning text lists whichever reasons applied. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

An inspector checked every entry against the commits behind it. Five said something the code does not do: the reconnect that loses keyboard input is the one started from the device tree, not the Enter-key reconnect; "ホーム" is the home tab's name, not a group's; the raw syslog message is kept only in the JSON export; the firewall check still falls back to matching by name alone where netsh cannot be read; and the updater's backup sweep changed which date it judges, not only which names it matches. Nine user-visible changes had no entry at all. Added: hyphenated MIB names, the shared temporary file that made two concurrent downloads fail, the firewall reason now shown in the panels, the host no longer recorded for a refused SNMP request, the SFTP manager dropped on a connection error, auto commands not starting after a disconnect, the caret counted in cells, and the limits that come with the display cap and the zero-width rule. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

This is the crash that has been taking the whole test suite down at a different place on every run, and the one the FTP stop work was chasing from the wrong end. SNMPManager forwarded its receiver thread's signals with connect(self.trap_received.emit). self.trap_received builds a throwaway pyqtBoundSignal on each access, so .emit is a builtin bound to that temporary, not to the SNMPManager. PyQt cannot see that the real receiver is this QObject, so Qt neither drops the connection when the manager is destroyed nor removes the calls already posted for it. A trap receiver that stops after the manager's last Python reference is gone leaves a queued call behind; the cyclic collector frees the manager; the next processEvents anywhere in the process calls emit on freed memory. Measured: a script that starts and stops a trap receiver, drops the manager and collects dies with 0xC0000409 (fail-fast) every time before this change and never after it. Because the explosion happens wherever the process next pumps events, the crash appeared in the FTP tests, in the SNMP trap tests, and as a silent death, which is why the earlier ftp_server fixes could not make it stop. Also gives the table models a parent. QAbstractItemView.setModel does not take ownership, so a model with no parent lives only as long as its Python reference: the collector could free it while its view was still alive and pointing at it (observed: four SyslogTableModels destroyed with panel and view still alive). tests/test_snmp_trap_limit.py took the panel out of a MainWindow it then let go. That worked only while the model had no parent; now the model belongs to the window, so the fixture keeps the window alive, the way the other window tests already do. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

The two FTP commits explained themselves with a mechanism that does not exist. They said close_all() removing a file descriptor from the list that select is scanning makes CPython read past the end of a shrunk array. It does not: CPython copies the descriptors into a C fd_set while holding the GIL and never touches the Python list during the call. An inspector threw 386 million concurrent removes and appends at a running select and closed a socket under a blocking select 400 times, with no exception and no crash on Python 3.12 and Windows 11. The crash those commits were chasing had another cause entirely, now fixed and covered by tests/test_signal_relay_outlives_receiver.py. The code stays as it is, for a reason that does hold: pyftpdlib assumes every registration and deregistration on an IO loop happens on the thread that polls it, and a stop from the GUI thread broke that assumption. A start-then-stop on its own logged WinError 10038 from the serving thread before the change. Only the explanation is replaced. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

…ng them Both CSV exports were plain UTF-8 with no BOM. Measured on the exported files: the GET/WALK CSV starts with b"OID,Type" and the trap CSV with b"\xe6\x99\x82" (the first byte of the Japanese header), neither carries EF BB BF. A Japanese Windows Excel opening such a file by double-click decodes it as cp932, so the header reads "譎ょ綾,騾∽ｿ｡蜈ｵP,..." instead of "時刻,送信元IP,..." and any Japanese sysName / sysLocation / varbind value the device returned is unreadable. Nothing is lost - the import wizard or any other tool still reads it - but the exports go into tickets and reports, where the receiver just sees garbage. Switch both open() calls to encoding="utf-8-sig". txt and json exports are left as-is: they are not opened with Excel. Existing tests that compared the CSV header row read the file as plain utf-8; updated to utf-8-sig for the new contract (test_snmp_export.py, test_snmp_csv_injection.py). New tests/test_snmp_csv_bom.py asserts the BOM by byte comparison, that the partial-export marker still precedes the header, and that txt/json stay BOM-free. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

…/sticky The permission column built only nine rwx characters and the chmod dialog seeded its input with mode & 0o777, so a special bit was invisible in both places. Pressing OK without editing anything then sent a mode with the bit cleared: paramiko's SFTPClient.chmod assigns attr.st_mode = mode verbatim, so the device really loses it. Measured before the change: mode=0o41777 shown as 'drwxrwxrwx', default '777' -> change_permissions(.., 0o777) mode=0o104755 shown as '-rwxr-xr-x', default '755' -> change_permissions(.., 0o755) mode=0o102755 shown as '-rwxr-xr-x', default '755' -> change_permissions(.., 0o755) _format_permissions now overlays the bits on the execute columns the way ls does (s/S, s/S, t/T) and the dialog seeds four octal digits from mode & 0o7777, so an untouched OK round-trips. A three digit entry keeps its old meaning (special bits cleared) and the prompt now says so. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

…" in cleartext is_encrypted() judged a password encrypted from the "DPAPI:" prefix alone. A user whose device password really begins with "DPAPI:" therefore got it written to config.json verbatim: measured on disk as 'DPAPI:<plaintext>' while a control password was stored as ciphertext. Reloading also counted it as undecryptable, so the "passwords could not be decrypted" notice appeared on every start, and the FTP/SFTP servers refused to start with such a password. Require the payload after the prefix to be valid base64 (validate=True). Ciphertext this app produces is always base64, so foreign ciphertext from another Windows account is still recognised and never re-encrypted - the contract in tests/test_password_reencrypt.py is unchanged. decrypt() now goes through the same predicate. The remaining limit (a plaintext that happens to be "DPAPI:" + valid base64) is stated in the docstring. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

… exceeded Neither wait is branched on: SNMPPanel.wait_for_background_work() drops the QThread.wait() return value, and SNMPManager.cancel_operation() only prints a warning. The bound is really exceeded - a GET or WALK against a host that does not answer takes about 6.1 s with the default pysnmp timeout/retries, measured against a closed port on 127.0.0.1. What the user sees is only a slow close: up to 5 s, up to about 10 s when a MIB load and a GET/WALK are both outstanding. It is not a crash. Measured over 20+ runs with threads still running at close (5.1 / 5.3 / 5.6 / 8 s): every process exited 0, with no "QThread: Destroyed while thread is still running" on stderr, because PyQt6 keeps its reference to a running QThread even after the owning panel is destroyed (destroyed flag False for the running thread, True for the window). Documentation only - no behaviour change. Shortening the close would mean setting an explicit UdpTransportTarget timeout/retries on the GET/WALK path, not touching the waits; noted in the docstring. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

Uploads start one thread each and transfer in whatever order those threads win _sftp_lock, which threading.Lock does not hand out FIFO. Two uploads to the same remote_path can therefore land newest-first and leave the older content on the device, and both report the same "アップロード完了" so the operator cannot tell. Measured: 30 natural back-to-back pairs all landed in call order; the reversal needed the first thread's pre-lock local stat held until the second transfer finished (then final content = OLD with two identical completion messages). With the default confirm_overwrite=True the second drop raises an overwrite dialog, during which the first thread always takes the lock. Guaranteeing order means a per-connection queue.Queue plus a single worker thread, and listing/download/delete share the same lock, so the change is wide. Documented as a known limit instead. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

The 切断 button in the device list was enabled and visible but its clicked signal had no receiver: measured receivers(btn_disconnect.clicked) = 0 while btn_connect and btn_add each had 1. Selecting a connected device and pressing it left the session in self.connections with is_connected True and the status bar unchanged, so the user is told nothing while the session stays up. Connect it to a handler that runs the same cleanup as closing the tab (macro cleanup, SFTP drop, disconnect). The tab stays open, so the reconnect notice appears there as with any other disconnect. tests/test_disconnect_button.py Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

ESC[1J sent from the bottom-right cell erases every visible cell, but only ED 2 / ED 0-from-home handed the screen to the history, so the wiped lines vanished from copy and from the "save log" output (toPlainText). Measured on a 3x10 screen: after "one/two/three" plus ESC[3;10H ESC[1J the text was ['', '', ''] and history was [] (ED 2 in the same state kept ['one', 'two']). Record the screen in that case too, and hand the history a copy of the row: _record_screen used to append the live list, which _erase_line then blanked in place, so the recorded bottom row came back empty. The erase itself still runs through the mode == 1 branch, so a row longer than the screen keeps the part right of the cursor as before. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

…cklog max_traps only trims after the GUI has taken a trap; the receiver thread emits one full dict per trap through a queued connection, so while the GUI is blocked that queue grows unbounded. Measured: 50,000 traps delivered during a 30 s GUI block cost about 2.3 KB each, RSS 104.2 MB -> 220.2 MB (+116.1 MB), all of it released once the GUI drained them (19.62 s, back to 1000 rows). The other half of the concern does not hold: reception cannot outrun the display. pysnmp decode runs at about 1,670 traps/s while the GUI takes them at about 2,550/s, so with an external sender at 3,000/s the backlog stayed at 3 or below for the whole run and the 1000-row cap did the work; excess datagrams are dropped by the kernel UDP buffer, not queued. Documentation only - batching behind a deque + QTimer would change the trap path for a window that only opens when the GUI is already frozen for tens of seconds, so the limit is written down instead. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

update_settings.github_token was the only credential left out of _encrypt_passwords/_decrypt_passwords. Measured: after set_github_token(), config.json held {'github_token': 'ghp_...'} verbatim, and a corrupted config copied that cleartext into backup_* as well. Device, FTP and SFTP passwords are all DPAPI-protected; the token is the same kind of secret. Encrypt it on save and decrypt it on load, leaving an already-encrypted value untouched so a config carried to another Windows account is not double-wrapped (same rule as device passwords). get_github_token() now returns None for a value it cannot decrypt and says so, instead of putting ciphertext in the Authorization header for a silent 401. The GITHUB_TOKEN environment variable still wins over the file. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

ESC[4J and ESC[3K (and any larger value) fell through to the "erase everything" branch, so a device that sends an out-of-range parameter wiped the visible screen or line. Measured before the fix: "KEEP" + ESC[4J left text ['', '', ''], and so did ESC[3K and ESC[9K. XTerm ctlseqs defines 0-3 for ED and 0-2 for EL, so the rest is now dropped the same way an unknown final byte is. ED 3 keeps its existing early return (NetBelt does not erase the scrollback). Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

transfer_started/progress/complete were declared with pyqtSignal int arguments, which PyQt maps to a 32-bit C++ int. Measured: emitting a 5GiB total (5368709120) delivered 1073741824 to the slot, so the history row showed "1.0GB" and reached "100%" at 1GiB; a 3GiB progress value arrived negative and the rate column showed "-1073741824000000.0B/s". No exception was raised, so the rounding was silent. Pass the byte counts as object, the way sftp_manager.py already does. Panel code keeps its plain integer arithmetic. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

Dragging the splitter handle to the right edge leaves tool_tabs at width 0 while isHidden() stays False. Measured: sizes [262, 930, 0], tool width 0, isHidden False. From there "ツールエリア表示/非表示" first falls to the hide branch, so pressing it twice ends back at width 0, and picking a tool from the View menu only changes the selected tab. The zero is then stored in ui_layout.splitter_sizes and carried into the next launch, so the area cannot be recovered from the menu at all. The device list already treats "no width" as hidden; the tool area did not. Treat width < 40 as hidden and restore a default width, in both _toggle_tool_area and _select_tool_tab. tests/test_tool_area_hiding.py Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

…s first discarded Entering the discard state threw away the whole pending buffer, including a trailing unpaired IAC. Measured: feeding an SB larger than MAX_PENDING_BYTES that ends on IAC, then a chunk starting with SE, left _discarding_sb=True forever - output stayed b'' and even an IAC DO went unanswered, so the session goes silent with no recovery short of reconnecting. The already covered case (the split happening on a later chunk) was handled. Carry over only the unpaired IAC, counted by _find_sb_end so a body ending in an escaped IAC IAC is not carried and the next SE is not mistaken for the terminator. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

…no local echo Reviewers keep rediscovering that WILL ECHO is answered with DONT while the terminal never echoes locally, so a device that honours DONT ECHO would show nothing while the user types. Measured against a local listener: the client answers ff fe 01 ff fe 03 ff fc 18 ff fc 00 and the widget stays empty. Not changed: the refuse-everything policy is what telnetlib does and is loop-safe without negotiation state, the devices this tool targets echo from the pty or line side, and answering DO ECHO cannot be verified against real hardware from here. State the limit and what a fix would require instead. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

check_for_updates picked the first asset whose name contained "windows" and ended with ".zip". A release that carries another windows-named zip ahead of the portable build (symbols, an installer, a docs bundle) hands that one to the downloader instead. When such a zip ships its own <name>.zip.sha256, the checksum gate passes too, so the user downloads and installs something that is not the application, and updater.bat then reports whatever it finds in it. Measured on this branch before the change: with assets ordered [NetBelt-v99.0.0-Windows-Symbols.zip, its .sha256, NetBelt-v99.0.0-Windows-Portable.zip, its .sha256], download_url came back as the Symbols asset URL ('https://api.example.com/assets/9' != '.../1'). Now the canonical CI name (NetBelt-v<version>-Windows-Portable.zip, see .github/workflows/build-release.yml:90) is matched first; the old heuristic stays as a fallback so releases published under an older naming scheme are still offered. No test was weakened or skipped. The updater.bat half of the same review item (a zip without NetBelt.exe reported as success when an old exe was already installed) no longer reproduces: b9b2bac stages the new exe as NetBelt.exe.new and checks for that staged file, so the old exe no longer satisfies the check. A regression test with an old exe in place is added to lock that in - it passes as written, which is the evidence that the behaviour is fixed. Tests: tests/test_update_checksum.py, tests/test_updater_script.py. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

…wline send_command passes keystrokes through verbatim, so Enter reaches the device as 0x0d alone: measured on the wire as b'sh\r' for s, h, Enter, and send_text('a\nb\r\n') as b'a\rb\r'. RFC 854 wants CR LF or CR NUL, so a strict NVT server would not commit the line. Not changed: rewriting CR to CR LF alters every keystroke sent to every device, and on a device that also treats LF as a newline it adds a blank line per Enter - a visible regression traded for a case no reachable device exhibits (IOS and netkit telnetd commit on a bare CR). Record the limit until it can be checked against real hardware. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

Entering the alternate screen blanked it for all three modes, so an application that leaves with ESC[?47l and comes back with ESC[?47h found an empty screen until it repainted. XTerm ctlseqs only clears on entry for 1049; 47 keeps the content and 1047 clears on exit. Measured before the fix: "ALT" on the alt screen, ESC[?47l, ESC[?47h -> ['', '', ''] (xterm shows ALT again). Clear is now driven by the mode: 1049 on entry, 1047 on exit, 47 never. The home-cursor move follows the entry clear, so 47/1047 leave the cursor where the sequence found it, as xterm does. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

DetachableTabBar kept the index recorded on mouse press, but a movable QTabBar calls moveTab while the pointer travels sideways. Measured with synthetic drags on the real tab bar: pressing tab 0 "Syslog", dragging horizontally reorders to SNMP/FTP/Syslog while _press_index stays 0, and pulling down then detaches SNMP - a tool the user never touched - into its own window. On a bare bar: pressed "A", detach callback got index 0 which by then was "B". Follow tabMoved so the recorded index tracks the pressed tab. tests/test_tool_detach.py Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

text=True alone decodes with the locale codepage (CP932 on a Japanese Windows), so a single Japanese line from the child raised UnicodeDecodeError before the test could look at the exit code it exists to check (measured: 'cp932' codec can't decode byte 0x88 in position 53, inside a worktree whose environment set UTF-8 output). The child's output encoding is pinned and decoded as UTF-8. The assertions are unchanged. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

A candidate ZIP can disappear mid-scan (a second NetBelt instance, or another installation cleaning up). os.path.getmtime then raised FileNotFoundError, which escaped MainWindow.__init__ and main(), so app.exec() was never reached and the app did not start at all. Wrap one candidate's evaluation in try/except OSError and skip that candidate, and guard the _check_pending_updates() call on startup so an update-check failure can never stop the application from starting. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

…be written The .sha256 / .version sidecars were written after os.replace had already moved the ZIP to its final name, and a write failure was only printed (the .version failure was swallowed entirely). download_update still returned the ZIP path, so the dialog showed "download complete" while every apply was rejected by is_verified_update. Re-downloading the same version also overwrote an already verified ZIP before failing, breaking an update that had been usable. Write both sidecars next to the .part file first, and only rename all three once they are on disk. If anything fails, discard the leftovers and return None so DownloadThread reports a failed download. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

cancel_check was only consulted inside the chunk loop. Cancelling while the checksum file was being fetched (a separate request, up to 30s) left the download running to completion: the verified ZIP, .sha256 and .version were published and offered as a pending update on the next start. abort() only closes the body response, so it does not shorten that window. Check for cancellation after the checksum has been verified and again just before the files are renamed to their final names, discarding the partial files in both cases. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

Two updates pointed at the same install folder shared the fixed staging name NetBelt.exe.new. Measured: whichever renamed first installed the *other* run's exe and still reported success with exit 0, while the second run found its target gone and told the user "NetBelt.exe is still the old version" -- which was false. The work folder in TEMP is already claimed exclusively with md, so borrow its name as a per-run stamp for the staging file. Nothing else changes; the bundled non-exe files are still copied in place, and that remaining limitation is recorded next to the rename. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

The restart said "started" unconditionally and then deleted the ZIP and its .sha256/.version sidecars. Measured: Windows printed "not compatible with the version of Windows you're running" and the very next line said the app had started, exit 0, retry material gone. start does not reset errorlevel on success, which is why its return value was ignored (the v1.1.0 false-failure bug). Measured here: levelling errorlevel to 0 immediately before start makes both work -- a broken exe yields 216, a good one 0, and a carried-over 9 no longer leaks in. So level, then judge; on failure say so plainly, keep the ZIP and sidecars for a retry, and wait instead of closing. The exit code and the completion banner are unchanged: the files were in fact replaced, and the caller has already quit, so nothing reads the code. That limitation is recorded next to the banner. Contract change in tests/test_updater_script.py: - test_the_restart_is_not_judged_by_errorlevel becomes test_the_restart_levels_errorlevel_before_judging_it: reading errorlevel after start is now required, levelling before it is what gets pinned. - test_a_stray_failure_before_the_restart_is_not_reported_as_one now injects its failure before the levelling line and ships a real executable in the ZIP; injecting after the levelling would step over the defence and a non-executable payload cannot tell a carried-over failure from a real launch failure. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

verify_before_apply checked the ZIP at a well-known path, then only the path string was passed to updater.bat, which reopens it about three seconds later. Another process running as the same user could replace the file in that window, so bytes that never matched the .sha256 were expanded, installed and started while the updater still reported success. VersionManager.stage_for_apply copies the ZIP (with its sidecars) to an unpredictable name, verifies that copy, and returns its path; both apply paths now hand that path to updater.bat, so the bytes that were checked are the bytes that get expanded. The remaining limit - a process that can write to the update folder can also enumerate it - is documented on the method; closing it entirely needs updater.bat to verify the hash it is given. Contract change: the ZIP path given to updater.bat is now the staged copy, not the file the caller passed. Two assertions in tests/test_updater_script.py that pinned the original path were narrowed to the property they were written for (the ZIP argument is quoted and stays in the same folder). Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

After `speed 115200` on a Cisco console the device switches immediately, but changing the rate in the device tree only updated the tree and the reconnect copy. The open connection stayed at the old rate, the screen turned to garbage, and the only way out was to close the tab and connect again. pyserial reconfigures an open port when its baudrate is assigned (SetCommState on the live handle), so the change is now applied in place: the port stays open, the reader and writer threads keep running, and the tab and its scrollback are untouched. A port that refuses the rate is not reported through error_occurred, because MainWindow treats that as a dead connection and drops a session that still works. The status bar says what happened instead. Measured with a loopback port while the reader and writer threads ran: 2000 switches, 2000 lines back in order, no errors. There is no COM port on this machine, so the Win32 SetCommState path itself was not run on hardware. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

…d one Two gaps the inspectors measured in the live baud change: - A change made while connect() was still inside serial.Serial() only updated the stored value. The port then opened at the old rate while the status bar said the change was made. MainWindow registers the connection before the connect thread starts, so the tree can reach it in that window. connect() now remembers the rate it opened with and applies the current one if they differ; a lock keeps set_baudrate and the hand-over of the opened port in order. - pyserial 3.5 writes the new rate into the port before SetCommState and leaves it there when the call fails, so the port named a rate it was not running at. The old rate is assigned back on failure. The comparison is between the rate used to open and the rate wanted now, so a port whose rate never changed is not read at all. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

1.3.0 reached main but was never tagged or released, so users are on 1.2.0. The release body only carries this version's section, so keeping the two sections apart would hide the earlier 1.3.0 changes, including the firewall behaviour change, from everyone updating. The earlier section now sits under this one with its headings one level down, and the intro points to it. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

Two tests failed on the GitHub runner and passed locally. The runner's TEMP is an 8.3 short path (C:\Users\RUNNER~1\...), and the server normalises its root with realpath, so os.lstat receives the long form while the tests compared against the short one. The spy never matched and the injected failure never fired. Reproduced locally by pointing TEMP at a short name: exactly these two failed. The comparisons now use the long form as well: the link itself is built from the resolved parent plus its name, since resolving the whole path would land on the target. Test-only change; the assertions are unchanged in meaning. Checked with both TEMP forms: the suite passes, and reverting either product fix (LSTAT resolving the link, READDIR dropping an entry silently) still fails the corresponding tests. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

The test decodes the child's stdout and stderr as UTF-8 but let the child write in the locale code page. On an English Windows such as the GitHub runner (cp1252), the product's Japanese print in _start_background_mib_loading raised UnicodeEncodeError, so the child exited 1 before the test could see whether closing right after opening crashes. Reproduced locally with PYTHONIOENCODING=cp1252; passes with the child pinned to UTF-8, in both encodings. Test-only change; the exit-code check is unchanged. The Japanese prints have been there since 1.2.0, so running NetBelt from source with its output redirected on a non-Japanese locale is an existing limitation, not a regression. The frozen exe has no console output. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

gomanizm and others added 30 commits September 12, 2026 06:47

merge p2b/snmp: close the inspector's gaps in the medium fixes

2e2f0cd

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

merge p2b/termcore: close the inspector's gaps in the medium fixes

c3ce74c

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

merge p2b/termui: close the inspector's gaps in the medium fixes

0835761

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

merge p2b/conn: close the inspector's gaps in the medium fixes

878b54d

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

merge p2b/srv: close the inspector's gaps in the medium fixes

88e7c28

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

merge p2b/sftpc: close the inspector's gaps in the medium fixes

5f172d5

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

merge p2b/release: close the inspector's gaps in the medium fixes

ab7ebc3

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

merge p2b/ftp: close the inspector's gaps in the medium fixes

19aefab

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

gomanizm and others added 28 commits September 15, 2026 03:41

merge: gapsyslog gap fixes

cf91398

merge: gapbuild gap fixes

0d543c1

merge: gapscreen gap fixes

0a35d37

merge: gapdispose gap fixes

e119ff0

merge: gaptermui gap fixes

68d63a6

merge: gapmainwin gap fixes

fe8cf39

merge: gapportcheck gap fixes

8aff5ae

merge: gapmib gap fixes

9e82734

merge: gapmenus gap fixes

71a6ea5

merge: gapsnmpui gap fixes

b0e508c

merge: gaptestflaky gap fixes

4dcf7c0

docs(changelog): record the terminal gaps the inspectors found

48ecf1a

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

docs(changelog): record the connection, SFTP, SNMP and server gaps

d5f2b6f

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

merge: round-5 updater fixes

df5b239

docs(changelog): record the six findings of the fifth review

1ba3c0c

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

docs(changelog): note that a console baud change now applies at once

f202008

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

gomanizm changed the title ~~Fix every finding of the whole-codebase review (v1.3.1)~~ NetBelt 1.3.0: review fixes and live console baud change Sep 17, 2026

gomanizm merged commit 01fa8a2 into main Sep 17, 2026
1 check passed

Navigation Menu

NetBelt 1.3.0: review fixes and live console baud change - #4

NetBelt 1.3.0: review fixes and live console baud change#4
gomanizm merged 373 commits into
mainfrom
fix/v1.3.1-review3

gomanizm commented Sep 11, 2026 •

edited

Loading

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

gomanizm commented Sep 11, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

A console baud change applies to the open tab

Review fixes

Checks

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

gomanizm commented Sep 11, 2026 •

edited

Loading