Final audit sweep: firewall fail-open + iSCSI/netdiag/session fixes (v1.121.15)#83
Merged
Merged
Conversation
…v1.121.15)
Fixes from the final consolidated audit of the remaining modules.
15-RDP.ps1:
- RDP/WinRM 'restrict to subnet' now ALSO disables RackStack's own custom RDP
('RDP Inbound TCP/UDP 3389') and WinRM ('WinRM HTTP/HTTPS Inbound') firewall
rules, which have no DisplayGroup and RemoteAddress=Any. The DisplayGroup-only
disable missed them, so 3389/5985/5986 stayed reachable from ANY address while
the tool reported the host as subnet-locked -- a silent security fail-open.
- PowerShell-Remoting reports each firewall group's ACTUAL enabled state instead
of a hardcoded green list (the Enable calls are -EA SilentlyContinue).
10-iSCSI.ps1:
- The [M] Manual option in Connect-to-iSCSI-Targets is matched before the nav
check (Test-NavigationCommand consumed 'M' as home and exited the menu -- 2nd
instance of the class).
- iSCSI Auto-Claim status reads the iSCSI key of the settings hashtable instead
of the hashtable's (always-true) truthiness.
- The connect dry-run undo matches the target portal via the session's connection
(TargetAddress), not InitiatorPortalAddress (the local NIC IP) which matched
nothing -- the undo now actually disconnects.
58-NetworkDiagnostics.ps1:
- Throughput benchmark uses a case-distinct Mbps variable; $writeMbps collided
with $writeMBps (PS is case-insensitive), so the MB/s column showed the 8x Mbps.
55-QoLFeatures.ps1:
- The session-state file that Restore-SessionState reads is now written
atomically (tmp -> rename), matching the snapshot file.
Tests: Section 202 (10 asserts). Structural 5366/5366; Pester 312/312; PSSA 0.
This completes an end-to-end adversarial audit of the whole codebase. Version -> 1.121.15.
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
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 from the final consolidated audit of the remaining modules (Firewall/RDP, iSCSI, network-diagnostics, session/QoL). This closes out an end-to-end adversarial audit of the whole codebase. 0 finder findings were refuted.
RDP/WinRM subnet restriction fail-open (
15, HIGH + MED)"Restrict RDP (or WinRM) to a subnet" disabled only the built-in
Remote Desktop/Windows Remote Managementfirewall groups — but RackStack's own Firewall-Template rules (RDP Inbound TCP/UDP 3389,WinRM HTTP/HTTPS Inbound) are created with no DisplayGroup andRemoteAddress=Any, so they were missed and stayed enabled. Since Windows OR-combines inbound Allow rules, 3389/5985/5986 remained reachable from any address while the tool reported the host as subnet-locked. Both paths now disable those custom rules too (and re-enable them on undo).PowerShell-Remoting honest firewall report (
15, LOW)Reported every firewall group as "enabled" regardless of the
-EA SilentlyContinueresult; now reports each group's actual state.iSCSI (
10)Test-NavigationCommandconsumedMas home and exited the menu — 2nd instance of the class). (MED)iSCSIkey of the settings hashtable instead of the hashtable's always-true truthiness. (MED)TargetAddress, notInitiatorPortalAddress(the local NIC IP, which matched nothing). (LOW)Network throughput 8× inflated (
58, MED)$writeMbpscollided with$writeMBps(PowerShell is case-insensitive), so the MB/s column displayed the Mbps value. Renamed the Mbps var to be case-distinct.Session state atomic write (
55, LOW)The file
Restore-SessionStateactually reads is now written tmp→rename (matching the snapshot file), so a crash mid-save can't leave a truncated file that's discarded on next resume.Verification