fix: harden Settings Watchdog against malformed baseline + surface admin requirement#1152
Merged
Merged
Conversation
…min requirement - A settings-baseline.json that parses as JSON but omits its Values map deserialized to a null map, which then threw a NullReferenceException in DetectChanges (escaping LoadBaseline's JsonException/IOException catch) and crashed the Check-now/Refresh command. LoadBaseline now normalizes a null Values to an empty map, and DetectChanges null-guards its baseline argument (defense in depth at the pure trust boundary). - The view never bound IsElevated, so a non-elevated user restoring a machine-wide (HKLM) setting only learned it failed after the fact. Added the standard not-elevated admin banner with a Run-as-administrator button bound to the VM's existing RelaunchAsAdminCommand. - Extracted ISettingsWatchdogService so the ViewModel's confirm-gate logic is unit-testable with a substituted service (no real registry/baseline file). Tests: DetectChanges null-baseline regression; VM SaveBaseline/RestoreSelected confirm-gate (declined -> no write), first-time save skips overwrite-confirm, RestoreSelected CanExecute false with no restorable drift.
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.
What
Three robustness fixes for the Settings Watchdog tab (#335), all surfaced by an exhaustive multi-agent audit of the recently-shipped features.
Fixes
settings-baseline.jsonthat's valid JSON but omits theValuesproperty deserialized to aBaselineSnapshotwithValues == null;DetectChangesthen NRE'd, and the catch inLoadBaselineonly coversJsonException/IOException, so it escaped and crashed the Check-now/Refresh command. Fix:LoadBaselinenormalizes a nullValuesto an empty map, andDetectChangesnull-guards its baseline argument (defense in depth at the pure trust boundary).IsElevated, although the VM already computed it and exposedRelaunchAsAdminCommand. Restoring a machine-wide (HKLM) setting needs admin, so a non-elevated user only learned it failed after the restore. Fix: added the standard not-elevated admin banner (matching CleanupView/TweaksHubView) with a Run-as-administrator button.ISettingsWatchdogServiceso the VM's confirm-gate logic is unit-testable with a substituted service (no real registry/baseline file) — the establishedIAppBlockerService/IPowerShellRunnerpattern.Tests
DetectChanges_NullBaseline_ReturnsEmpty_DoesNotThrow— the crash regression.SettingsWatchdogViewModelTests): SaveBaseline declined → no write; first-time save skips overwrite-confirm; RestoreSelected declined → no write; RestoreSelected restores each restorable drift; CanExecute false with no restorable drift.Docs
CHANGELOG 1.51.1, version bump to 1.51.1. README/ARCHITECTURE unchanged (feature behavior is the same; this is internal robustness + an admin affordance).
Part of the post-feature audit remediation (Batch 1 of 6).