Remove dead Windows 7 conditions and inline CheckKeyConsistency in PfxFormatTests

[Copilot]

Description

Two places in PfxFormatTests.cs had an always-false condition OperatingSystem.IsWindows() && !PlatformDetection.IsWindows guarding followup = null. Since these can never be true, the followup variable was always CheckKeyConsistency and never modified.

• Removed both dead if blocks and their associated Windows 7 comment blocks
• Eliminated the intermediate Action<X509Certificate2> followup variable
• Passed CheckKeyConsistency directly to ReadMultiPfx calls

Before:

Action<X509Certificate2> followup = CheckKeyConsistency;

// For unknown reasons, CheckKeyConsistency on this test fails
// on Windows 7 with an Access Denied in all variations for
// Collections, and in invertCertOrder: true for Single.
//
// Obviously this hit some sort of weird corner case in the Win7
// loader, but it's not important to the test.

if (OperatingSystem.IsWindows() &&
    !PlatformDetection.IsWindows)
{
    followup = null;
}

ReadMultiPfx(pfxBytes, pw, first, expectedOrder, followup);

After:

ReadMultiPfx(pfxBytes, pw, first, expectedOrder, CheckKeyConsistency);

[dotnet-policy-service]

Tagging subscribers to this area: @bartonjs, @vcsjones, @dotnet/area-system-security
See info in area-owners.md if you want to be subscribed.

[vcsjones]

This was just incomplete cleanup done in #122907.

[Copilot]

Pull request overview

Cleans up dead/unreachable Windows 7-specific code paths in PfxFormatTests by removing an always-false condition and simplifying the callback wiring used by ReadMultiPfx.

Changes:

• Removed the dead OperatingSystem.IsWindows() && !PlatformDetection.IsWindows branches (and associated Windows 7 comments).
• Eliminated the intermediate followup delegate variable.
• Passed CheckKeyConsistency directly into ReadMultiPfx calls.

[stephentoub]

@copilot please invoke the code-review skill and post the analysis/comments as a comment on this PR

[Copilot]

@copilot please invoke the code-review skill and post the analysis/comments as a comment on this PR

🤖 Copilot Code Review

Holistic Assessment

Motivation: This PR cleans up dead code left behind by PR #122907 (removal of PlatformDetection.IsWindows7). The condition OperatingSystem.IsWindows() && !PlatformDetection.IsWindows is logically contradictory — it checks "is Windows AND is not Windows" — so it's always false, making the guarded followup = null assignments unreachable. The cleanup is justified.

Approach: The fix correctly removes the dead if blocks, their associated stale Windows 7 comments, and the now-unnecessary intermediate followup variable, passing CheckKeyConsistency directly to ReadMultiPfx. This is the simplest correct approach.

Summary: ✅ LGTM. The change is minimal, correct, and behavior-preserving. No functional impact — the removed code paths were unreachable.

---

Detailed Findings

✅ Dead code removal is correct

The condition OperatingSystem.IsWindows() && !PlatformDetection.IsWindows can never be true — both check the same OS. This condition was likely !PlatformDetection.IsWindows8xOrLater (or similar) before PR #122907 replaced the old Win7-era properties with PlatformDetection.IsWindows, making the condition contradictory. The two removed blocks in TwoCerts_TwoKeys_ManySafeContentsValues and TwoCerts_TwoKeys_ManySafeContentsValues_UnencryptedAuthSafes_NoMac are identical in structure and equally dead.

✅ Remaining followup pattern is unaffected

The followup variable at line 350 in OneCert_TwoKeys_FirstWins branches on !correctKeyFirst (a test parameter), which is a legitimate runtime condition — not dead code. This PR correctly leaves it untouched.

✅ No other instances of this dead pattern exist

Searched the file for other bare !PlatformDetection.IsWindows (without version qualifier) references — none remain. The s_loaderFailsKeysEarly field uses !PlatformDetection.IsWindows10Version1607OrGreater, which is a meaningful check for pre-RS1 Windows versions and is still correct.

💡 Stale Windows 7 comments in field declarations (out of scope)

Lines 18, 21-22 reference "Windows 7-8.1 support" and "Windows 7" in comments for s_digestAlgorithm and s_windowsPbe. These comments are somewhat stale since Windows 7 is no longer supported, but they explain the historical reason for the algorithm choices (SHA-1, 3DES) and the variables are still actively used. This is a follow-up consideration, not a blocker for this PR.