Bundle 7 issue fixes: hashable Finding, distinct ADM ids, unified PWD-002 guidance, SigAge warning, partial-write logging, tz-aware scan_time, IP filter#24
Open
TiltedLunar123 wants to merge 7 commits into
Conversation
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.
Summary
Closes seven open GitHub issues with small, isolated fixes. Each commit is its own logical unit so individual fixes can be reverted in isolation if needed.
Fixes
Findingis now hashable via__hash__matching__eq__, so equal findings can be deduplicated throughset()or used as dict keys.ADM-001for the enumeration-failure path,ADM-002for the success-case member-count finding incheck_local_admins.PWD-002title and description now both reference 14 characters (CIS recommendation). Borderline lengths 12-13 are now flagged WARNING instead of passing.check_antivirusnow emits anAV-003WARNINGfinding whenGet-MpComputerStatusreturns a non-numericSigAge, instead of silently dropping the result.winrecon.cliis wrapped in its own try/except. Success message only appears when every requested report lands; partial and total failures get their own log lines. Logic was extracted intowrite_reports()for direct unit testing.system_info.scan_timeis now produced bydatetime.now(timezone.utc).isoformat(timespec="seconds"), matching the JSON schema'sformat: date-timedeclaration._is_uninteresting_ipnow also filters IPv6 link-local (fe80:) and IPv4 APIPA (169.254.) addresses out ofsystem_info.ip_addresses.Test plan
python -m pytest -qpasses (143 tests, up from 122)python -m ruff check .cleanTestFindingHash(3): hashability, hash-equality, set-deduplicationTestAdminCheckIds(2): success path uses ADM-002, failure path keeps ADM-001TestPasswordPolicyMessaging(3): short/borderline/compliant length casesTestAntivirusSigAge(3): unparseable, empty, current valuesTestPartialReportFailureLogging(4): partial failure, full success, total failure, json-onlyTestSystemInfoScanTime(1): RFC 3339 with UTC offsetTestUninterestingIpFilter(5): loopback v4/v6, APIPA, link-local v6, real addressesNotes
ADM-001->ADM-002for the success-case finding (consumers parsing JSON by check_id should update if they rely on the old behavior; this was a bug per Duplicate check_id ADM-001 used for both success and failure findings #4).winrecon.cli.write_reportsis now a public-ish helper. It is not exported through__init__.py.