fix: security hardening, battery health false-critical, Win11 startup detection by laurentiu021 · Pull Request #401 · laurentiu021/SystemManager

laurentiu021 · 2026-05-15T13:48:19Z

Summary

Addresses HIGH and MEDIUM findings from the comprehensive code review.

Security

SEC-H1 — Authenticode verification on update binary (rejects tampered signatures)
SEC-H2 — Uninstall exe path validation against trusted directories
SEC-M1 — TOCTOU fix: update script uses random GUID filename
SEC-M5 — XPath sanitization expanded with all metacharacters

Functionality

FUNC-H1 — Battery health returns -1 (unavailable) instead of 0 when WMI data missing without admin; health score treats this as neutral (100) not critical (10)
FUNC-M1 — StartupService Win11 registry blob: uses bitmask instead of == 3

Files changed (9)

Services: UpdateService.cs, UninstallerService.cs, EventLogService.cs, HealthScoreService.cs, StartupService.cs
Models: BatteryInfo.cs
ViewModels: AboutViewModel.cs, BatteryHealthViewModel.cs
Docs: CHANGELOG.md

Build

0 errors, 0 new warnings.

Summary by CodeRabbit

Bug Fixes
- Enhanced security validation for uninstaller executables and update binaries.
- Improved handling of missing battery capacity data with clearer unavailability messaging in battery health UI.
- Corrected startup entry enabled/disabled detection logic.
- Hardened XPath construction in event log queries against injection.
- Randomized updater script filenames to prevent timing-related vulnerabilities.
Documentation
- Updated changelog with security and behavior fixes for v0.48.23.

… detection

+                (!string.IsNullOrEmpty(pf) && fullPath.StartsWith(pf, StringComparison.OrdinalIgnoreCase)) ||
+                (!string.IsNullOrEmpty(pfx86) && fullPath.StartsWith(pfx86, StringComparison.OrdinalIgnoreCase)) ||
+                (!string.IsNullOrEmpty(winDir) && fullPath.StartsWith(winDir, StringComparison.OrdinalIgnoreCase)) ||
+                fullPath.StartsWith(progData, StringComparison.OrdinalIgnoreCase) ||
+                (!string.IsNullOrEmpty(localAppData) && fullPath.StartsWith(localAppData, StringComparison.OrdinalIgnoreCase));


coderabbitai · 2026-05-15T13:53:37Z

Caution

Review failed

Pull request was closed or merged during review

📝 Walkthrough

Walkthrough

This PR consolidates multiple security and correctness hardening fixes for SysManager 0.48.23. Battery health data now uses -1 to signal unavailable capacity, with downstream health score and UI updates to reflect this. Update binaries are verified via Authenticode signatures with GUID-based script naming to prevent TOCTOU races. Uninstall executables are validated against trusted directories, XPath expressions are further sanitized, and StartupService's enabled-state detection is corrected for Windows 11 registry blobs.

Changes

Security and Correctness Hardening

Layer / File(s)	Summary
Battery Health Sentinel and Model Behavior `SysManager/SysManager/Models/BatteryInfo.cs`, `SysManager/SysManager.Tests/BatteryInfoEdgeCaseTests.cs`	`BatteryInfo.HealthPercent` and `BatteryInfo.WearPercent` now return `-1` instead of `0` when capacity data is unavailable. Edge-case tests updated to expect `-1` sentinel value.
Battery Health Scoring and Display `SysManager/SysManager/Services/HealthScoreService.cs`, `SysManager/SysManager.Tests/BatteryServiceTests.cs`, `SysManager/SysManager/ViewModels/BatteryHealthViewModel.cs`	`HealthScoreService.ComputeBatteryScore` treats negative health (`-1`) as neutral score (100). `BatteryHealthViewModel.RefreshAsync` conditionally formats summary—numeric health for valid data, "requires elevation" message for `-1`. Service tests updated for sentinel expectations.
Update Integrity Verification and Safe Script Naming `SysManager/SysManager/Services/UpdateService.cs`, `SysManager/SysManager/ViewModels/AboutViewModel.cs`	`UpdateService.VerifyAuthenticode` validates X509 certificate signatures on downloaded binaries. `AboutViewModel.InstallUpdateAsync` invokes verification after SHA256 check and aborts on invalid signatures. Updater script filename uses random GUID to prevent concurrent-execution collisions.
Uninstaller Trusted-Directory Validation `SysManager/SysManager/Services/UninstallerService.cs`	`UninstallLocalAsync` resolves uninstall executable full path and blocks execution if outside trusted directories (Program Files, Windows, ProgramData, LocalApplicationData).
Event Log XPath Metacharacter Sanitization `SysManager/SysManager/Services/EventLogService.cs`	`BuildXPath` expands sanitization to strip backslash, pipe, parentheses, @, wildcard, and angle brackets from provider names before embedding into XPath filter expressions.
StartupService Enabled-State Bitmask Detection `SysManager/SysManager/Services/StartupService.cs`	`ApplyApprovedState` now determines enabled/disabled from least-significant bit of registry blob's first byte, correcting misclassification when Windows 11 disabled markers are encountered.
Release Notes Documentation `CHANGELOG.md`	Changelog entry for version 0.48.23 documents all fixes across update verification, uninstaller validation, XPath sanitization, battery health sentinel behavior, health score neutrality, elevation messaging, and StartupService bitmask correction.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

laurentiu021/SystemManager#274: Both PRs modify AboutViewModel's update installation flow—fix: auto-update now performs in-place self-replace with SHA256 verification #274 introduces SHA256 verification and in-place self-replace updater scripts, while this PR adds Authenticode signature verification and randomized GUID-based script filenames to that same flow.
laurentiu021/SystemManager#361: Both PRs modify UninstallerService.UninstallLocalAsync and EventLogService.BuildXPath, applying additional security hardening to uninstall command validation and XPath expression construction in overlapping code paths.
laurentiu021/SystemManager#376: Both PRs harden UninstallerService.UninstallLocalAsync by tightening validation of trusted uninstall executables—this PR adds full-path trusted-directory checks while the retrieved PR resolves executables to absolute System32 paths to prevent PATH hijacking.

Poem

🐰 With signatures sealed and paths now verified,
Our battery speaks -1 when data's retired,
GUID-named scripts skip TOCTOU's dance,
While trusted roots guard each uninstall chance.
Robustness wins—no false alarms today!

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 33.33% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title accurately summarizes the three main categories of changes: security hardening, battery health false-critical fixes, and Windows 11 startup detection, all of which are prominently featured in the changeset.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

📝 Generate docstrings

Create stacked PR
Commit on current branch

🧪 Generate unit tests (beta)

Create PR with unit tests
Commit unit tests in branch fix/security-functionality-batch

Tip

💬 Introducing Slack Agent: The best way for teams to turn conversations into code.

Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.

Generate code and open pull requests
Plan features and break down work
Investigate incidents and troubleshoot customer tickets together
Automate recurring tasks and respond to alerts with triggers
Summarize progress and report instantly

Built for teams:

Shared memory across your entire org—no repeating context
Per-thread sandboxes to safely plan and execute work
Governance built-in—scoped access, auditability, and budget controls

One agent for your entire SDLC. Right inside Slack.

👉 Get started

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

codecov-commenter · 2026-05-15T13:55:59Z

⚠️ Please install the to ensure uploads and comments are reliably processed by Codecov.

Codecov Report

❌ Patch coverage is 34.04255% with 31 lines in your changes missing coverage. Please review.

Files with missing lines	Patch %	Lines
...sManager/SysManager/Services/UninstallerService.cs	0.00%	15 Missing ⚠️
SysManager/SysManager/Services/UpdateService.cs	0.00%	11 Missing ⚠️
SysManager/SysManager/ViewModels/AboutViewModel.cs	0.00%	4 Missing ⚠️
...sManager/SysManager/Services/HealthScoreService.cs	0.00%	0 Missing and 1 partial ⚠️

📢 Thoughts on this report? Let us know!

…nfigureAwait(false) (#424) ## Summary **Batch 5+6** of the QA bug fix series (combined — related changes). ### Issue #401 — Hardcoded launcher paths - Added RiotLogDirs helper that scans all fixed drives for Riot Games/League of Legends log directories - Follows the same pattern as SteamRoots and SteamShaderCacheDirs - Other launcher caches (Epic, Battle.net, GOG, EA) correctly use %LOCALAPPDATA%/%PROGRAMDATA% — no changes needed ### Issue #402 — Unbounded icon cache - Added MaxCacheSize property (default 500, configurable) to IconExtractorService - When cache exceeds the limit, half the entries are evicted ### Issue #403 — Missing ConfigureAwait(false) - PerformanceService: all async calls now use ConfigureAwait(false) - UninstallerService: RunProcessAsync call uses ConfigureAwait(false) - WingetService: all RunProcessAsync calls use ConfigureAwait(false) ### Testing - Build: 0 errors Closes #401, Closes #402, Closes #403 Co-authored-by: laurentiu021 <laurentiu021@users.noreply.github.com>

Adds CHANGELOG entries for all 9 releases from the QA bug fix session: - **v0.28.16** — Dispose lifecycle (#395, #410) - **v0.28.17** — CTS disposal + bare catch (#396, #413) - **v0.28.18** — Input validation + null checks (#397, #398) - **v0.28.19** — JSON error handling (#400) - **v0.28.20** — Drive scanning + cache eviction + ConfigureAwait (#401, #402, #403) - **v0.28.21** — Audit logging + error messages (#405, #407) - **v0.28.22** — SHA256 verification (#408, #409) - **v0.28.23** — Service timeout + snapshot persist + traceroute DNS (#414, #415, #416) - **v0.28.24** — Accessibility (#411) 18 bugs fixed in total. Co-authored-by: laurentiu021 <laurentiu021@users.noreply.github.com>

… detection (#401) * fix: security hardening, battery health false-critical, Win11 startup detection * test: update battery tests for -1 (unavailable) return value --------- Co-authored-by: laurentiu021 <laurentiu021@users.noreply.github.com>

fix: security hardening, battery health false-critical, Win11 startup…

e7c69c0

… detection

github-advanced-security AI found potential problems May 15, 2026

View reviewed changes

test: update battery tests for -1 (unavailable) return value

b9e048d

laurentiu021 merged commit f359da7 into main May 15, 2026
3 of 4 checks passed

laurentiu021 deleted the fix/security-functionality-batch branch May 15, 2026 13:57

coderabbitai Bot mentioned this pull request May 18, 2026

fix: resolve 5 CodeQL source alerts and suppress 1 intentional obsolete call #428

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

fix: security hardening, battery health false-critical, Win11 startup detection#401

fix: security hardening, battery health false-critical, Win11 startup detection#401
laurentiu021 merged 2 commits into
mainfrom
fix/security-functionality-batch

laurentiu021 commented May 15, 2026 •

edited by coderabbitai Bot

Loading

Uh oh!

Uh oh!

coderabbitai Bot commented May 15, 2026 •

edited

Loading

Review failed

Walkthrough

Changes

Estimated code review effort

Possibly related PRs

Poem

❌ Failed checks (1 warning)

Uh oh!

codecov-commenter commented May 15, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Uh oh!

Conversation

laurentiu021 commented May 15, 2026 • edited by coderabbitai Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Security

Functionality

Files changed (9)

Build

Summary by CodeRabbit

Uh oh!

Uh oh!

coderabbitai Bot commented May 15, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Review failed

Walkthrough

Changes

Estimated code review effort

Possibly related PRs

Poem

❌ Failed checks (1 warning)

Uh oh!

codecov-commenter commented May 15, 2026

Codecov Report

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

laurentiu021 commented May 15, 2026 •

edited by coderabbitai Bot

Loading

coderabbitai Bot commented May 15, 2026 •

edited

Loading