Repository navigation
feat: add privacy toggles for telemetry, ads, and tracking - #481
Conversation
📝 WalkthroughWalkthroughThis PR implements the Privacy Toggles feature by introducing a registry-backed toggle system with a data model, core service for Windows privacy management, view model for UI state orchestration, XAML UI with category filtering and commands, and replacing the WIP placeholder in the main window navigation. All toggles support instant registry apply, state detection on load, and bulk operations. ChangesPrivacy Toggles Implementation
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related issues
Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@SysManager/SysManager/Services/PrivacyService.cs`:
- Around line 66-77: ApplyAll promises to log per-toggle errors and continue,
but because ApplyToggle only handles certain exception types, unexpected
exceptions can abort the loop; to fix, wrap each ApplyToggle(toggle) call inside
a broad try/catch in ApplyAll (after ArgumentNullException.ThrowIfNull) that
catches Exception, logs the error with context (including the toggle identifier)
and continues to the next item; alternatively, ensure ApplyToggle(PrivacyToggle
toggle) itself catches all exceptions, logs them, and returns without throwing —
reference ApplyAll(IEnumerable<PrivacyToggle> toggles) and
ApplyToggle(PrivacyToggle toggle) when making the change.
In `@SysManager/SysManager/ViewModels/PrivacyViewModel.cs`:
- Around line 92-97: ApplyAll currently just reapplies current states; change it
so it first sets every PrivacyToggle.IsEnabled = true on each entry in the
Toggles collection, then call _service.ApplyAll(Toggles); update StatusMessage
and Log.Information accordingly (e.g., "All {Count} toggles enabled and
applied") to reflect that protections were turned on; touch the ApplyAll method
and the Toggles iteration (use a simple foreach over Toggles to set IsEnabled)
before invoking _service.ApplyAll.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 68048afb-051b-4998-a964-38262db87732
📒 Files selected for processing (8)
CHANGELOG.mdSysManager/SysManager/Models/PrivacyToggle.csSysManager/SysManager/ServiceRegistration.csSysManager/SysManager/Services/PrivacyService.csSysManager/SysManager/ViewModels/MainWindowViewModel.csSysManager/SysManager/ViewModels/PrivacyViewModel.csSysManager/SysManager/Views/PrivacyView.xamlSysManager/SysManager/Views/PrivacyView.xaml.cs
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Analyze (csharp)
- GitHub Check: Build & unit tests
🔇 Additional comments (8)
CHANGELOG.md (1)
9-12: LGTM!Also applies to: 14-15, 21-21, 26-27, 29-31
SysManager/SysManager/Models/PrivacyToggle.cs (1)
14-38: LGTM!SysManager/SysManager/Services/PrivacyService.cs (1)
22-64: LGTM!Also applies to: 82-145, 147-280
SysManager/SysManager/ServiceRegistration.cs (1)
56-56: LGTM!Also applies to: 88-88
SysManager/SysManager/ViewModels/PrivacyViewModel.cs (1)
31-89: LGTM!Also applies to: 99-142
SysManager/SysManager/ViewModels/MainWindowViewModel.cs (1)
47-47: LGTM!Also applies to: 73-73, 146-146, 194-194, 228-228, 404-404, 569-569
SysManager/SysManager/Views/PrivacyView.xaml.cs (1)
9-12: LGTM!SysManager/SysManager/Views/PrivacyView.xaml (1)
57-69: ⚡ Quick winAction: Paste the review comment to rewrite. Provide the full contents of the
<review_comment>...</review_comment>block (including any diff snippets and file/line references) so I can rewrite it in the required format.
| /// <summary> | ||
| /// Applies all toggles in sequence. Errors on individual toggles are | ||
| /// logged but do not stop the batch. | ||
| /// </summary> | ||
| public void ApplyAll(IEnumerable<PrivacyToggle> toggles) | ||
| { | ||
| ArgumentNullException.ThrowIfNull(toggles); | ||
|
|
||
| foreach (var toggle in toggles) | ||
| { | ||
| ApplyToggle(toggle); | ||
| } |
There was a problem hiding this comment.
ApplyAll can still abort despite the “continue on error” contract.
Line 67 says per-toggle failures should not stop the batch, but currently only security/access exceptions are handled in ApplyToggle. Other exceptions can still terminate the loop.
Proposed fix
public void ApplyAll(IEnumerable<PrivacyToggle> toggles)
{
ArgumentNullException.ThrowIfNull(toggles);
foreach (var toggle in toggles)
{
- ApplyToggle(toggle);
+ try
+ {
+ ApplyToggle(toggle);
+ }
+ catch (Exception ex)
+ {
+ Log.Error(ex, "Failed applying privacy toggle {Name}; continuing batch", toggle.Name);
+ }
}
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| /// <summary> | |
| /// Applies all toggles in sequence. Errors on individual toggles are | |
| /// logged but do not stop the batch. | |
| /// </summary> | |
| public void ApplyAll(IEnumerable<PrivacyToggle> toggles) | |
| { | |
| ArgumentNullException.ThrowIfNull(toggles); | |
| foreach (var toggle in toggles) | |
| { | |
| ApplyToggle(toggle); | |
| } | |
| /// <summary> | |
| /// Applies all toggles in sequence. Errors on individual toggles are | |
| /// logged but do not stop the batch. | |
| /// </summary> | |
| public void ApplyAll(IEnumerable<PrivacyToggle> toggles) | |
| { | |
| ArgumentNullException.ThrowIfNull(toggles); | |
| foreach (var toggle in toggles) | |
| { | |
| try | |
| { | |
| ApplyToggle(toggle); | |
| } | |
| catch (Exception ex) | |
| { | |
| Log.Error(ex, "Failed applying privacy toggle {Name}; continuing batch", toggle.Name); | |
| } | |
| } | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@SysManager/SysManager/Services/PrivacyService.cs` around lines 66 - 77,
ApplyAll promises to log per-toggle errors and continue, but because ApplyToggle
only handles certain exception types, unexpected exceptions can abort the loop;
to fix, wrap each ApplyToggle(toggle) call inside a broad try/catch in ApplyAll
(after ArgumentNullException.ThrowIfNull) that catches Exception, logs the error
with context (including the toggle identifier) and continues to the next item;
alternatively, ensure ApplyToggle(PrivacyToggle toggle) itself catches all
exceptions, logs them, and returns without throwing — reference
ApplyAll(IEnumerable<PrivacyToggle> toggles) and ApplyToggle(PrivacyToggle
toggle) when making the change.
| private void ApplyAll() | ||
| { | ||
| _service.ApplyAll(Toggles); | ||
| StatusMessage = $"All {Toggles.Count} toggles applied."; | ||
| Log.Information("Privacy: applied all {Count} toggles", Toggles.Count); | ||
| } |
There was a problem hiding this comment.
ApplyAll currently does not turn protections on.
ApplyAll() applies existing states only; it never flips all PrivacyToggle.IsEnabled values to true, so it can be a no-op instead of “enable all protections”.
Suggested fix
[RelayCommand]
private void ApplyAll()
{
+ _suppressApply = true;
+ try
+ {
+ foreach (var toggle in Toggles)
+ toggle.IsEnabled = true;
+ }
+ finally
+ {
+ _suppressApply = false;
+ }
+
_service.ApplyAll(Toggles);
+ UpdateStatus();
StatusMessage = $"All {Toggles.Count} toggles applied.";
Log.Information("Privacy: applied all {Count} toggles", Toggles.Count);
}🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@SysManager/SysManager/ViewModels/PrivacyViewModel.cs` around lines 92 - 97,
ApplyAll currently just reapplies current states; change it so it first sets
every PrivacyToggle.IsEnabled = true on each entry in the Toggles collection,
then call _service.ApplyAll(Toggles); update StatusMessage and Log.Information
accordingly (e.g., "All {Count} toggles enabled and applied") to reflect that
protections were turned on; touch the ApplyAll method and the Toggles iteration
(use a simple foreach over Toggles to set IsEnabled) before invoking
_service.ApplyAll.
#481) ## Summary Fully implements the App Blocker tab, replacing the WIP placeholder. Blocks applications from executing using the Image File Execution Options (IFEO) registry mechanism. ## Changes - **New**: \AppBlockerService\ — uses IFEO Debugger key to prevent app execution. Block, unblock, check status, enumerate all blocked apps. Fully reversible. - **New**: \AppBlockerViewModel\ — block by name or browse, unblock selected, refresh list, select/deselect all. Admin privilege detection. - **New**: \BlockedApp\ model — executable name, full path, blocked timestamp, selection state. - **New**: \AppBlockerView.xaml\ — text input + browse, toolbar, DataGrid. - **Updated**: \MainWindowViewModel\ — replaced \WipAppBlocker\ placeholder with real \AppBlockerViewModel\. - **Tests**: 5 unit tests for ViewModel and Model. ## Safety - Confirmation dialog before block and unblock - Admin privilege check with clear status message - Fully reversible — unblock removes the IFEO Debugger key - Only blocks apps marked by SysManager (checks for specific debugger path) Closes #378 Co-authored-by: laurentiu021 <laurentiu021@users.noreply.github.com>
* Initial state * feat: add privacy toggles for telemetry, ads, and tracking --------- Co-authored-by: laurentiu021 <laurentiu021@users.noreply.github.com>
Summary
New Privacy tab with 12 one-click toggles for Windows privacy settings.
Categories:
Behavior:
Closes #9
Test plan