NetClean 1.0.0: PS7-only, security hardening, live-tested bug fixes, release infra - #4
Merged
Conversation
…effort step; docs: update changelog and PR template
…to maintain, and easier to read.
…oving safety and performance.
… up into the different phases as much as possible and keeping functions that are used by all in NetClean.psm1.
Restores module caching (removed in the last revision) but redesigned
to avoid the bug that caused it to break PS5.1 CI: instead of
installing to a custom directory and manually wiring it onto
PSModulePath (fragile - a missed $env:GITHUB_ENV write is exactly what
broke the ps51 job before), cache the actual default -Scope CurrentUser
install directory for each PowerShell edition. PowerShell already
searches that path natively, so a cache hit needs zero extra plumbing.
Also switch the cache key from hashFiles('**/*.psd1') (invalidated by
any unrelated manifest edit) to the exact pinned module versions, and
make the Install-Module calls conditional on the pinned version already
being present so a cache hit skips the network round-trip entirely.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
The dual-version support existed for locked-down/managed machines
where a user can't install PS7. NetClean's actual target use (prepping
a personally-owned machine for a conference or CTF) never depends on
that scenario - it's exactly what the tool exists to avoid. Combined
with the security/complexity argument (PS7 runs on actively-patched
modern .NET vs. legacy .NET Framework, and collapsing to one engine
removes an entire class of dual-implementation portability bugs, e.g.
this session's earlier HashSet-constructor divergence), standardizing
on PS7 is the right call, and it halves per-change CI validation time.
- NetClean.psd1: PowerShellVersion 5.1 -> 7.4, add
CompatiblePSEditions = @('Core'). This is the actual enforcement
point - Test-ModuleManifest/Import-Module now refuse to load the
module under Windows PowerShell 5.1.
- ci.yml: remove the windows-powershell-compatibility job. Confirmed
no other job depends on it (publish-coverage-badge and
comment-coverage only need test-and-analyze).
- README/CONTRIBUTING/AGENTS/CHANGELOG: drop dual-version references,
update the PowerShell badge to a single 7.4+ badge.
- NetClean.Coverage.Unit.Tests.ps1: replace the dual-badge and
PS5.1-CI-job assertions with checks that the job is gone and the
manifest enforces the 7.4 floor.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Research this session found a concrete command-injection angle: Remove-WiFiProfilesSafe (NetCleanPhase3.ps1) built netsh.exe's "name=$p" argument by interpolating a Wi-Fi profile name directly, with no validation. Profile *names* (distinct from raw SSID octets) are attacker-influenceable - a hostile profile can be imported with an arbitrary name via a crafted XML - so an embedded double quote could close the argument early and let extra tokens reach netsh's own argument parser. AGENTS.md already states "Validate all file/registry/ network inputs before use"; this closes a real gap against that standard. - Add Test-NetCleanSafeIdentifier (Modules/NetClean.psm1): rejects values containing an embedded double quote, backtick, or shell metacharacters (;&|<>) before they reach any native command. - Apply it in Remove-WiFiProfilesSafe (NetCleanPhase3.ps1) and Export-WiFiProfile's per-profile fallback (NetCleanPhase2.ps1): an unsafe name is skipped and logged as WARN, sibling profiles in the same batch are still processed. - Add tests/Security, a new discoverable test category for this high-privilege tool (registry, firewall, adapters, Wi-Fi credentials): injection-rejection contract tests, an over-blocking regression guard for legitimate unicode/space names, a secret- redaction test proving exported PSK/key material never reaches logs, and a real (non-mocked) ACL test confirming backup directories are actually restricted to current user/Administrators/SYSTEM with inheritance disabled. - Wire tests/Security into Run-NetClean-Coverage.ps1's test discovery. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Completes the whole-suite implementation-agnostic-test audit from earlier this session (findings 1-3, 5-15; see memory for the full list). Every test below now calls the real production function and asserts on genuinely observable output, rather than treating Should -Invoke on a mock as the only or primary check. Highest severity - tautological Launcher tests (never called the real Invoke-NetCleanLauncher at all, calling their own mocks directly instead, so a real regression would go undetected): - NetClean.Launcher.Functional.Tests.ps1: rewrote "Launcher workflow invocation" and "Logging initialization" to call the real Invoke-NetCleanLauncher with script-scoped state, following the working pattern already used by "Post-run power handling". Dropped one test that duplicated existing PerformanceTune-profile coverage. Rewrote the two "verification message" tests to call the real Show-NetCleanSummary with a correctly-shaped Result object (the old mock shape didn't even match what the function reads - Result.Verify.Passed vs. the real Result.Verify.Summary.Passed) and assert on the real printed message text. Logging-order test now records real call order instead of asserting two independent Should -Invoke counts. Should-Invoke-only tests with discarded return values, now asserting on real output: - NetClean.Core.Unit.Tests.ps1: Invoke-NetCleanNativeCapture's test mocked Start-Process, which the real implementation never calls (a dead mock, acknowledged as a stopgap in its own comment) - replaced with a real cmd.exe invocation and assertions on Succeeded/ExitCode/ Output. Set-NetCleanPrivateDirectoryAcl and Start-NetCleanLog tests now check the real resulting ACL via Get-Acl instead of mocking Set-Acl/Set-NetCleanPrivateDirectoryAcl. - NetClean.Phase1.Unit.Tests.ps1: Get-ProtectionInventory's single-collection test now also asserts on the returned inventory. - NetClean.Phase2.Unit.Tests.ps1: backup-directory-creation test now also asserts $result.Phase. - NetClean.Phase3.Unit.Tests.ps1: merged the "-Confirm" test (whose filter didn't actually distinguish the adapter-reset call from any other, since no sibling call passes -Confirm either) into the existing adapter-reset invoke-check instead of a separate, misleadingly-titled test. Wi-Fi union-of-profiles and DNS-skip tests now assert on real returned fields instead of only call arguments. - NetClean.Phase4.Unit.Tests.ps1: the two CollectionSnapshot-reuse tests now use a mock whose return value depends on receiving the correct arguments, so Passed/Failed on the real result proves the wiring works. The dry-run verification-report test now asserts on the real Verify.VerificationReport field. - NetClean.Workflow.Functional.Tests.ps1: the skip-switch, AdvancedRepair, and PerformanceProfile forwarding tests now use mocks that throw unless they receive the expected arguments, so a real end-to-end failure (not just a call-argument inspection) proves each mode/option is correctly forwarded to Invoke-NetCleanPhase3Clean. Left unchanged (reviewed, judged not worth changing): the aggregate "no directories created during dry-run exports" test in NetClean.Safety.Unit.Tests.ps1 - already uses a genuine external- boundary mock and the audit itself flagged it as optional/coarse rather than coupled. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Invoke-NetCleanPhase1Detect crashed on every real run with: "The property 'Name' cannot be found on this object." This session's P5 registry-API rewrite (ca1aebf) swapped Get-RegistryValuesSafe from Get-ItemProperty to the raw Microsoft.Win32.Registry API. For a registry key that exists but has zero direct values - a completely normal shape for many real Windows service keys (e.g. a service whose only content is a Parameters subkey) - the new implementation returns [pscustomobject]$values where $values is an empty ordered hashtable. That's a real PowerShell engine quirk, not a hypothetical: a PSCustomObject with zero properties makes ANY later `.PSObject.Properties.Name -contains 'X'` check throw under Set-StrictMode -Version Latest (confirmed by direct repro - even `.Count` on the empty Properties collection throws). Every caller of Get-RegistryValuesSafe (Get-ServiceRegistrySnapshot, Get-NdisFilterClassEvidence, Get-AdapterRegistryCorrelation, product/ uninstall scanning) uses exactly that idiom to test for a specific value name, so this was reachable from a plain Preview run on any real machine. The old Get-ItemProperty-based implementation never hit this: its returned object always carried PSPath/PSProvider metadata properties, even for a value-less key, so it was never truly empty. Fix: when a key exists with zero registry values, return a pscustomobject carrying one synthetic property instead of a raw empty hashtable cast, restoring the "always non-empty when the key exists" property the rest of the codebase already assumes. Verified against the real registry (Get-ServiceRegistrySnapshot: 927 services, full Invoke-NetCleanPhase1Detect: 58 candidates) with no crash. Added a regression test using a real TestRegistry: container-only key (exists, zero direct values) - confirmed red against the prior implementation, green after the fix. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Same class of bug as the just-fixed Get-RegistryValuesSafe crash: this function read $props.ComponentId/.DriverDesc/.ProviderName/ .NetCfgInstanceId directly (no PSObject.Properties.Name -contains guard), where $props is a per-key-varying registry value bag from Get-RegistryValuesSafe. A real adapter class subkey missing any one of these (ProviderName is commonly absent for basic/inbox NDIS drivers) throws under Set-StrictMode, and this function has no try/catch anywhere, so the throw propagates all the way up through Get-ProtectionInventory - a full workflow crash reachable from the default Detect path. Fix mirrors the existing correct pattern used three lines away in Get-ServiceRegistrySnapshot: guard each optional property with PSObject.Properties.Name -contains before reading it. Added a regression test using real TestRegistry: data - a class subkey with only NetCfgInstanceId set (a valid real-world shape), confirming Get-AdapterRegistryCorrelation no longer throws and correctly reports the missing fields as null. Part of a broader input-validation hardening pass; see memory for the remaining findings still to fix. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Highest-priority finding from the input-validation audit: the ServiceRegistry block in Get-ProtectionEvidence read $svcProps.ImagePath/.DisplayName/.Start/.Type/.Group directly with no existence guard, where $svcProps is a per-key-varying registry value bag from Get-RegistryValuesSafe. Empirically confirmed against this machine's real registry (927 services): 462 lack Group, 82 lack DisplayName, 24 lack ImagePath, 11 lack Start/Type. The whole per-service loop shares one try/catch, so the FIRST service missing one of these properties threw and silently aborted the entire loop - meaning ServiceRegistry evidence collected effectively zero entries on nearly every real run. This defeats detection of installed AV/EDR/VPN services via service-registry evidence, which is core to this tool's protection-preservation guarantees. Fixed by mirroring the existing correct guard pattern from Get-ServiceRegistrySnapshot (PSObject.Properties.Name -contains before reading each optional property). While verifying against the real machine, found and fixed a second, related bug in the same code path: Get-NdisServiceBindingEvidence read $serviceEntry.LinkageValues unguarded, assuming every caller- supplied snapshot entry has that property. Get-ProtectionEvidence's own internally-built snapshot only includes Name/RegistryPath/Values - never LinkageValues - so this threw on every single service on this machine (a snapshot-shape mismatch between two different snapshot constructors, not caught by the earlier audit since each constructor's shape is individually "fixed"). Fixed the same way: guard the property read, falling through to the existing $null handling immediately below. Verified against the real registry (927 real services): all 927 now produce ServiceRegistry evidence, 2391 total evidence entries, no errors. Added regression tests for both using the exact shapes that triggered them. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Two more findings from the input-validation audit, same bug class as the prior two fixes: a dynamically-shaped registry object accessed without an existence guard. - Get-ProtectionEvidence's Uninstall-registry block (NetCleanPhase1.ps1): read $_.DisplayName/.DisplayIcon/.Publisher/ .InstallLocation/.UninstallString directly off a raw Get-ItemProperty result over HKLM:\...\Uninstall\*, whose property set only includes values that actually exist on that key. Confirmed empirically: 27 of 505 real Uninstall subkeys on this machine lack DisplayName entirely. Contained by a per-root try/catch, so it silently truncated enumeration of the remaining entries under that root rather than crashing - still a real evidence-collection gap. Fixed by computing each optional field through a PSObject.Properties.Name -contains guard before use, matching the pattern used elsewhere in this file. - Get-NetworkListProfileSnapshot (NetCleanPhase2.ps1): read $properties.ProfileName unguarded while the sibling Category/Description/Managed fields three lines below were already correctly guarded - an inconsistency within the same function. A NetworkList profile subkey can exist without a ProfileName value (e.g. a transient/incompletely-initialized profile). Verified against the real registry: Get-ProtectionEvidence now collects 478 Uninstall entries (up from a run silently truncated by the first missing-DisplayName key) and 2838 total evidence entries, no errors. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Priority 1 from the second input-validation audit: two exported,
public-surface functions had an uncaught crash confirmed by direct
execution.
- Invoke-NetCleanPhase2Protect (NetCleanPhase2.ps1): the backup
directory create+ACL step was the only step in the whole function
with no try/catch - every other export step already fails soft with
a WARN. But a failed backup directory isn't safe to silently proceed
past like an individual export failure is, so this now throws a
clear, actionable message ("Unable to create or secure the backup
directory '...': ... Check that the path does not already exist as
a file...") instead of the bare "Private data directory does not
exist" exception. Real trigger: -BackupPath resolving to something
that already exists as a file rather than a directory.
- Invoke-NetCleanLauncher (netclean.ps1): both the Start-NetCleanLog
call and the BackupPath New-Item call had no try/catch, and
$ErrorActionPreference='Stop' with no surrounding handler anywhere
in the launcher meant either one crashing terminated the entire
script with a raw stack trace before Detect/Protect/Clean ever ran.
Both now catch, print a clear actionable message via
Write-Information, and return cleanly - matching the existing
Write-Information+return convention already used for the launcher's
other controlled early exits (e.g. "Operation cancelled").
Added regression tests for both: Phase2Protect throws the new clear
message when directory setup fails; the launcher aborts cleanly
(without an uncaught exception, and without ever reaching
Invoke-NetCleanWorkflow) when either Start-NetCleanLog or the backup
New-Item fails.
Remaining from the audit: a locale-dependent netsh parsing gap
(Get-WiFiProfileSnapshot keys off the English "Profile" label) - non-
crashing, lower priority, not yet addressed. See memory for details.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Two new findings from the consolidation/release-readiness audit. - netclean.ps1: Test-NetCleanAdministrator's bare throw, at the very first line of Invoke-NetCleanLauncher, had zero surrounding error handling - the single most likely mistake a new user makes (running non-elevated) produced a raw uncaught-exception stack trace instead of a clean message. Every functional test mocked this check away, so the real non-admin path had never been exercised. Now wrapped in try/catch, printing the existing clear message via Write-Information and returning, matching the same convention already used for the launcher's other controlled early exits. - examples/run-netclean.ps1 had no -Mode parameter at all and never passed -Force, so the scheduled task that examples/register-scheduledtask.ps1 wires up to run it unattended at startup always fell through to interactive -Mode Menu with no console attached - hanging forever or spinning in an infinite re-prompt loop. The advertised "run netclean unattended at startup" capability could not work as shipped. Fixed by requiring an explicit -Mode on the wrapper (forcing a conscious choice rather than a silent default that changes system state) and having register-scheduledtask.ps1 accept and forward -Mode plus -Force (unattended execution has no console to answer the interactive confirmation prompt). Also caught while fixing this: both example scripts launched via 'powershell.exe' (Windows PowerShell), which can no longer load NetClean's module manifest after this session's PS7.4+ requirement - switched both to resolve and use pwsh.exe, with a clear error if it isn't installed. Bundled two related fixes in the same files: run-netclean.ps1 now checks the launched process's actual exit code instead of always printing "run completed" regardless of outcome; restore-wifi- profiles.ps1 now checks $LASTEXITCODE instead of relying on a try/ catch around a native command that doesn't throw on failure, so it no longer reports "Imported" for profiles that were never restored. Added a regression test confirming the launcher aborts cleanly (without an uncaught exception, and without reaching Invoke-NetCleanWorkflow) when Test-NetCleanAdministrator throws. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Two scripts, git-ignoring the actual captured data: - tests/LocalFixtures/Export-NetCleanRealFixture.ps1: captures the exact output of Get-ServiceRegistrySnapshot, Get-NetworkListProfileSnapshot, Get-AdapterRegistryCorrelation, plus raw Uninstall-registry entries, to local JSON files under tests/LocalFixtures/data/ (now git-ignored - this data never leaves the machine it was captured on). - tests/LocalFixtures/NetClean.RealFixture.Tests.ps1: replays a captured fixture through those same functions inside an isolated TestRegistry: hive (same mechanism as tests/System/NetClean.Registry.System.Tests.ps1), never touching the real registry. Skips entirely (not a failure) when no fixture has been captured, so it's safe on any machine including CI. This is the same class of check that found all the crash bugs fixed earlier this session (Get-ServiceRegistrySnapshot/Get-ProtectionEvidence/ Get-NdisServiceBindingEvidence/etc. against real property shapes), now available as a repeatable local tool instead of one-off manual verification. Intentionally excluded from Run-NetClean-Coverage.ps1's discovery - the TestRegistry: provider has real per-write overhead (~0.9s/call measured on this machine), so even a capped sample (30 services, 30 Uninstall entries) takes several minutes; this is meant to be run occasionally as a diagnostic; the fast day-to-day suite is unaffected (confirmed: still discovers exactly 15 files, 481 tests, 95.52% coverage). Deliberately scoped to the registry-reading functions only, not Get-ProtectionEvidence's full evidence collection - that also queries CIM/WFP/INF/ScheduledTasks/Appx directly against the real system regardless of any registry fixture, and takes 20-30+ seconds by design independent of this tool. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Move all accumulated Unreleased entries under a new "## 1.0.0" section and add entries for the fixes made this session that weren't yet documented: the registry zero-value crash, the four unguarded-property findings from the input-validation audit, the fail-closed error handling, the non-admin crash, and the scheduled-task example fix. Leaves a fresh empty Unreleased section for future work.
…indows
Get-WiFiProfileSnapshot decided whether a netsh output line was a Wi-Fi
profile entry by checking whether its label contained the English word
"Profile", and classified Group-Policy-managed profiles by matching the
English section headers "Group policy profiles"/"User profiles". Both
are localized by netsh on non-English Windows installs, so this
silently detected zero profiles - defeating Wi-Fi profile backup and
removal entirely, with no error or warning.
Fixed by treating any "key: value" line in this specific netsh
command's output as a profile entry, regardless of the label text -
there is nothing else in this command's output that has that shape and
isn't a profile name.
The Group-Policy classification is safety-relevant: Phase 3 excludes
IsPolicyManaged profiles from removal
(NetCleanPhase3.ps1:Where-Object { -not $_.IsPolicyManaged }), so
getting it wrong in the "treat as removable" direction on a non-English
system would be a real regression, not just cosmetic. Since section-
header text is still locale-dependent and can't be fully generalized
without a translation table, this fails closed: if no localized header
was recognized at all, every profile found is conservatively treated
as policy-managed (excluded from removal) rather than risking removal
of something that should have been protected. English-locale behavior
is unchanged (verified against this machine's real 35 Wi-Fi profiles -
all still correctly classified as User/not-policy-managed).
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Introduce New-NetCleanEvidenceRecord to replace 7 of the 15 near-identical 15-field pscustomobject literals in Get-ProtectionEvidence (SecurityCenter2 AV/FW, Win32_Service, Win32_SystemDriver, Uninstall registry, NetAdapter, PnpDevice) with a single parameterized constructor. Publisher/CompanyName/ ProductName support per-site fallback overrides since those defaults vary (e.g. Uninstall falls back to Publisher/DisplayName, NetAdapter has no metadata at all). Behavior-preserving: full suite still 483/483 passing at 95.64% coverage, 0 PSScriptAnalyzer findings.
Write-NetCleanLog is defined unconditionally in NetClean.psm1 and dot-sourced before any Phase1-4 function can execute, so `Get-Command Write-NetCleanLog` always resolves - the $canLog guard was tautologically true at every one of its ~90 call sites. Removed the assignment and unwrapped/simplified every `if ($canLog)`, `elseif ($canLog)`, and `if ($canLog -and ...)` site accordingly (elseif/compound conditions simplified in place; plain if-wrapped blocks unwrapped and de-indented). Net -170 lines. Behavior-preserving: full suite still 483/483 passing at 95.64% coverage, 0 PSScriptAnalyzer findings.
…r-ArpCacheSafe Both wrappers were byte-identical except for the target name, native command, and IgnoreExitCode flag. The shared helper covers the actual invoke+log step; each wrapper still owns its own -DryRun short-circuit and ShouldProcess check (kept separate deliberately, since they return slightly different result shapes for those paths). Did not touch Invoke-AdvancedNetworkRepair/Invoke-NetworkPerformanceTune: on inspection they are not byte-identical (Tune's result objects carry extra Profile/Why fields and IgnoreExitCode differs between them), and a shared loop helper would need to route each function's own -WhatIf/-Confirm through a nested ShouldProcess call, which is easy to get subtly wrong. Left as a follow-up rather than risk breaking a documented safety behavior. Full suite still 483/483 passing at 95.64% coverage, 0 PSScriptAnalyzer findings.
The earlier $canLog cleanup (88181b5) only matched the variable-assignment form; NetClean.psm1 (11 sites) and NetCleanPhase4.ps1 (3 sites) had the same tautologically-true check written inline instead: if ($null -ne (Get-Command Write-NetCleanLog -ErrorAction SilentlyContinue)). Same reasoning applies - Write-NetCleanLog is always defined by the time these functions run - so removed these the same way. Also restores NetClean.psm1's original UTF-8 BOM, which the removal script's Set-Content incidentally stripped; no other unintended changes. Full suite still 483/483 passing at 95.62% coverage, 0 PSScriptAnalyzer findings.
Add three private constructors and convert every call site in NetCleanPhase4.ps1 to use them, replacing repeated pscustomobject literals: - New-NetCleanNotApplicableResult: the 6 byte-identical Applicable=false early returns across Test-NetCleanAdapterPostState/CleanupPostState/ ArtifactRemovalPostState. - New-NetCleanVerificationCheck: ~25 Category/Target/Expected/Actual/Passed/Error check objects. VerificationType and Applicable are only emitted when the caller supplies them, since adapter checks never had those fields while WiFiConnection/DnsCache/ArpCache checks always did - preserves each family's original shape rather than forcing one. - New-NetCleanArtifactDecisionCheck: the 2 near-identical artifact-decision blocks in Test-NetCleanArtifactRemovalPostState, parameterized on the two "not verified" detail messages that differ between the NetworkProfileDecisions and CandidateArtifacts call sites. Net -96 lines. Full suite still 483/483 passing at 95.47% coverage, 0 PSScriptAnalyzer findings.
Adds a shared helper for the "read Name from InputObject if the property exists, else null" pattern used throughout the registry/CIM-derived evidence and verification code, and converts ~34 exact-shape call sites across NetClean.psm1, NetCleanPhase1.ps1, NetCleanPhase2.ps1, and NetCleanPhase4.ps1 (both the single-line and multi-line forms). Left ~4 sites untouched where the shape doesn't match: two have a non-null fallback (else -1, else $true) and two return a transformed value (-join ', ') rather than the raw property - folding those into a null-only helper would be a leaky abstraction, per the original consolidation audit's own guidance on this exact case. Full suite still 483/483 passing at 95.68% coverage, 0 PSScriptAnalyzer findings.
Found via live testing on a real Hyper-V VM (never previously tested outside mocks): a real cleanup run correctly deleted a NetworkList\Signatures key, which recursively removed its \Managed and \Unmanaged child keys too - but Phase 3's per-item ledger recorded the children as Skipped/Reason=NotFound (since there was nothing left to individually delete), not Removed=true. Test-NetCleanArtifactRemovalPostState only trusted Removed=true, so it reported those two already-gone children as "not confirmed removed," failing overall verification for a cleanup that had, in fact, fully succeeded. Fix: treat a registry result as confirmed-absent when Removed=true OR Reason='NotFound' (mirroring the same real-state-aware pattern already used by Test-NetCleanCleanupPostState), while still correctly rejecting a NotFound Reason on a Preserve-decision candidate (which is a genuine unexpected-removal failure, not a false alarm). Second, related bug: Invoke-NetCleanPhase4Verify never surfaced ArtifactVerification at all - not in the Verify object, not in the Summary counts, not in the exported JSON report, not in the console/log output - even though its Passed value already silently factored into the overall Passed result. A caller seeing Passed=False with every other count at 0 had no way to learn why. Added ArtifactVerification to the Verify object, ArtifactCheckFailureCount to Summary, detail logging per check, inclusion in the exported report, and an ArtifactCheckFailures count in the console summary line. Full suite: 487/487 passing (4 new tests), 95.66% coverage, 0 PSScriptAnalyzer findings.
…mpty Found via live VM testing: on a system with nothing left to sanitize (e.g. just after a real cleanup run already removed the NetworkList artifacts), Phase 2's Sanitizable-artifact backup step threw "Cannot bind argument to parameter 'Contents' because it is an empty string" instead of writing a valid, empty JSON array. Root cause: `$Data | ConvertTo-Json` produces zero pipeline output when $Data is an empty array - PowerShell's ConvertTo-Json emits nothing at all rather than "[]" when it receives no pipeline input, the same gotcha already hit this session in Export-NetCleanRealFixture.ps1's own tooling. Switched to `ConvertTo-Json -InputObject $Data`, which serializes correctly regardless of element count. This shared helper backs Export-SanitizableNetworkArtifact, Export-ProtectionInventory, and Export-ProtectionRegistryMap, so the fix covers all three. The two other `| ConvertTo-Json` call sites in the codebase (NetCleanPhase2.ps1:806, NetCleanPhase4.ps1:1015) pipe a single ordered hashtable, not a collection, so they were never at risk of this bug. Full suite: 488/488 passing (1 new test), 95.66% coverage, 0 PSScriptAnalyzer findings.
Add .github/workflows/release.yml, triggered by pushing a vX.Y.Z tag (or manual workflow_dispatch): verifies the pushed tag's version matches NetClean.psd1's ModuleVersion, re-runs PSScriptAnalyzer and the full Pester suite as a release gate, then publishes a GitHub Release whose notes are extracted directly from the matching CHANGELOG.md section. Document the release process in CONTRIBUTING.md, and fold this session's consolidation and live-testing bug-fix work into the CHANGELOG's existing 1.0.0 section (no tag has actually been cut yet, so this is still the same not-yet-released version, not a new one). Deliberately does not publish to the PowerShell Gallery - that would need an API key secret and is a separate decision.
scweeks
added
documentation
NetClean Coverage Report
|
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
This branch takes NetClean from its original state to a hardened, PS7-only 1.0.0 candidate. Highlights, in rough chronological order:
PowerShell 7.4+ only. Dropped Windows PowerShell 5.1 support (CompatiblePSEditions = 'Core') — the tool's actual use case (preparing a personally-owned machine for a conference/CTF) never depended on the locked-down managed machines that motivated dual-version support, and standardizing on one runtime removed a class of dual-implementation bugs.
Real, previously-unknown bugs found and fixed by testing against real machines and a dedicated Hyper-V VM rather than stopping at "tests pass": several strict-mode crashes on real registry data shapes unit tests never exercised, a Wi-Fi profile name command-injection gap, locale-dependent Wi-Fi detection silently finding nothing on non-English Windows, a non-admin launch crash, a broken scheduled-task example, and — from live end-to-end Phase 1–4 testing this session — a Phase 4 verification false-failure (with zero visibility into why) and an empty-array JSON export crash in the backup step.
A real tests/Security suite: injection-resistance, secret-redaction, and backup-directory-ACL regression tests.
Code consolidation: shared constructors/helpers replacing ~15 evidence-record duplicates, ~34 optional-property-read duplicates, and Phase 3/4 object-construction duplicates; removed a dead capability check present at ~90 call sites across all 4 phase modules.
Release infrastructure: a tag-triggered GitHub Release workflow that re-validates and re-tests before publishing, with release notes drawn straight from CHANGELOG.md.
Full itemized history is in CHANGELOG.md's 1.0.0 section.
Test plan
done
PSScriptAnalyzer: 0 findings
done
Full Pester suite: 488/488 passing, 95.66% coverage (≥94% gate)
done
All 4 phases (Detect/Protect/Clean/Verify) live-tested for real against a dedicated Hyper-V test VM, including SafeConferencePrep, AdvancedRepair, and PerformanceTune modes, plus the full Invoke-NetCleanWorkflow entry point end-to-end
done
Wi-Fi profile removal previously verified live by the repo owner (not re-tested this session — confirmed no logic changes to that code path, only a mechanical dead-code removal)
not done
Adapter/DNS reconfiguration not live-tested against real hardware (blocked by Hyper-V VM limitations: no virtual Wi-Fi adapter, and the VM's own NIC is always classified as vendor-protected) — known gap, not a regression