Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions .github/codeql-config.yml
Original file line number Diff line number Diff line change
Expand Up @@ -3,3 +3,11 @@ name: "SysManager CodeQL Configuration"
paths-ignore:
- '**/obj/**'
- '**/bin/**'

query-filters:
# Intentional use of CreateFromSignedFile for Authenticode verification —
# no modern .NET replacement exists without P/Invoke. Already suppressed
# with #pragma warning disable SYSLIB0057.
- exclude:
id: cs/call-to-obsolete-method
path: SysManager/SysManager/Services/UpdateService.cs
Comment on lines +11 to +13

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

🧩 Analysis chain

🌐 Web query:

In GitHub CodeQL codeql-config.yml, does query-filters.excludesupport apathfield? Please list the officially supported fields forquery-filters and whether unsupported keys are ignored or rejected.

💡 Result:

In GitHub CodeQL configuration files (codeql-config.yml), the query-filters.exclude block does not support a path field [1][2]. The query-filters mechanism is designed to filter queries based on their metadata properties (such as id, tags, or severity) rather than file paths [1][2]. To exclude specific files or directories from CodeQL analysis, you should use the top-level paths-ignore field in your codeql-config.yml file instead [3][2][4]. Officially supported fields for query-filters The query-filters section supports include and exclude blocks [2]. Within these blocks, you can filter queries using any valid CodeQL query metadata property [1]. Common examples include: - id: The unique identifier of the query [5][1][2]. - tags: The tags associated with the query [2]. - kind: The kind of query (e.g., problem, path-problem, table) [5][2]. - problem.severity: The severity level of the query (e.g., warning, recommendation) [2]. Handling of unsupported keys In GitHub Actions, providing unsupported keys in the codeql-config.yml file typically results in a warning or the key being ignored by the CodeQL action [6]. If you provide an invalid property at the top level of the configuration file or within specific blocks, the action may log a warning indicating that the property is not a valid input or is being ignored [6]. It is recommended to strictly follow the documented schema to ensure your configuration is applied correctly [2].

Citations:


🏁 Script executed:

#!/bin/bash
# Verify whether obsolete-call usage is truly isolated to the intended call site.
rg -n --type=cs 'CreateFromSignedFile|SYSLIB0057|`#pragma` warning disable SYSLIB0057'

Repository: laurentiu021/SystemManager

Length of output: 822


Remove path from query-filters.exclude—it's unsupported and causes the suppression to be global

query-filters.exclude does not support a path field. Unsupported fields are ignored, so the id: cs/call-to-obsolete-method exclusion will suppress this rule across the entire repository, not just UpdateService.cs. Since both UpdateService.cs and SpeedTestService.cs already use local #pragma warning disable SYSLIB0057 pragmas at the call sites (which is the correct approach), remove the ineffective CodeQL config line entirely.

🤖 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 @.github/codeql-config.yml around lines 11 - 13, Remove the unsupported
"path" field from the query-filters.exclude entry so the exclusion is not
applied globally: delete the entire exclude block that contains id:
cs/call-to-obsolete-method (the one attempting to target
SysManager/SysManager/Services/UpdateService.cs), leaving CodeQL to rely on the
local pragmas already present (the existing "`#pragma` warning disable SYSLIB0057"
usages in UpdateService.cs and SpeedTestService.cs) instead of a repository-wide
suppression.

26 changes: 22 additions & 4 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -8,15 +8,35 @@ adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).

## [0.48.33] - 2026-05-18

### Changed
### Fixed
- **CodeQL** — resolved 5 remaining source code alerts:
- Replaced generic `catch (Exception)` in `ViewModelBase.InitializeAsync`
with specific exception types (`InvalidOperationException`,
`UnauthorizedAccessException`, `IOException`, `HttpRequestException`,
`TimeoutException`).
- Converted `UninstallerService.IsUnderTrustedDirectory` foreach+if to
LINQ `.Any()` (cs/linq/missed-where).
- Converted `WindowsFeaturesService.ParseFeatureList` foreach loop to
LINQ `.Select().Where()` pipeline (cs/linq/missed-select).
- Extracted `ProcessManagerViewModel.MatchesFilter` complex condition into
three focused helper methods (cs/complex-condition).
- Replaced `Path.Combine` with `Path.Join` in `UpdateService.DownloadAsync`
(cs/path-combine).
- **CodeQL workflow** — added query filter to suppress `cs/call-to-obsolete-method`
for `UpdateService.VerifyAuthenticode` (intentional use of `CreateFromSignedFile`
— no modern .NET replacement exists without P/Invoke).

## [0.48.32] - 2026-05-18

### Fixed
- **ConsoleViewModel** — buffer trimming now uses clear-and-rebuild when
removing more than 25% of lines, reducing worst-case from O(n×excess)
to O(n) (CQ-LOW: ConsoleViewModel O(n²)).
- **LogsViewModel** — event log entries are now dispatched to the UI thread
in batches of 50 instead of one-at-a-time, reducing dispatcher overhead
by ~98% when loading large event logs (CQ-LOW: LogsViewModel batch dispatch).

## [0.48.32] - 2026-05-18
## [0.48.31] - 2026-05-18

### Fixed
- **FormatSize duplication** — extracted shared `FormatHelper.FormatSize` method;
Expand All @@ -26,8 +46,6 @@ adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
`SystemHealthViewModel` (chkdsk) now use `PowerShellRunner.OemEncoding`
instead of duplicating the encoding resolution logic inline.

## [0.48.31] - 2026-05-18

### Changed
- **Test parallelism** — enabled `parallelizeTestCollections` in xunit.runner.json
so pure-logic unit tests run concurrently, reducing CI test time. Tests that
Expand Down
9 changes: 3 additions & 6 deletions SysManager/SysManager/Services/UninstallerService.cs
Original file line number Diff line number Diff line change
Expand Up @@ -402,11 +402,8 @@ private static bool IsUnderTrustedDirectory(string fullPath)
Environment.GetFolderPath(Environment.SpecialFolder.LocalApplicationData)
};

foreach (var dir in trustedDirs)
{
if (!string.IsNullOrEmpty(dir) && fullPath.StartsWith(dir, StringComparison.OrdinalIgnoreCase))
return true;
}
return false;
return trustedDirs.Any(dir =>
!string.IsNullOrEmpty(dir) &&
fullPath.StartsWith(dir, StringComparison.OrdinalIgnoreCase));
Comment on lines +405 to +407

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical | ⚡ Quick win

Harden trusted-directory matching to avoid prefix bypass.

This check trusts paths that only start with a trusted prefix, which can be bypassed by sibling paths (for example, C:\Program Files Evil\...). Since this guards execution of registry-sourced uninstall commands, it should enforce a directory-boundary match.

Proposed fix
-        return trustedDirs.Any(dir =>
-            !string.IsNullOrEmpty(dir) &&
-            fullPath.StartsWith(dir, StringComparison.OrdinalIgnoreCase));
+        var normalizedPath = System.IO.Path.GetFullPath(fullPath)
+            .TrimEnd(System.IO.Path.DirectorySeparatorChar, System.IO.Path.AltDirectorySeparatorChar);
+
+        return trustedDirs.Any(dir =>
+        {
+            if (string.IsNullOrWhiteSpace(dir)) return false;
+            var normalizedDir = System.IO.Path.GetFullPath(dir)
+                .TrimEnd(System.IO.Path.DirectorySeparatorChar, System.IO.Path.AltDirectorySeparatorChar);
+
+            return normalizedPath.Equals(normalizedDir, StringComparison.OrdinalIgnoreCase)
+                || normalizedPath.StartsWith(
+                    normalizedDir + System.IO.Path.DirectorySeparatorChar,
+                    StringComparison.OrdinalIgnoreCase)
+                || normalizedPath.StartsWith(
+                    normalizedDir + System.IO.Path.AltDirectorySeparatorChar,
+                    StringComparison.OrdinalIgnoreCase);
+        });
🤖 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/UninstallerService.cs` around lines 405 - 407,
The current trusted-directory check in UninstallerService (the lambda returning
trustedDirs.Any(...)) uses StartsWith and can be bypassed by sibling paths;
change it to compare normalized full paths with directory-boundary checks by
normalizing both fullPath and each trusted dir via Path.GetFullPath, ensuring
each trusted dir has a trailing directory separator (e.g., append
Path.DirectorySeparatorChar if missing) and then use
fullPathNormalized.StartsWith(trustedDirNormalized,
StringComparison.OrdinalIgnoreCase) so only paths under that directory (or the
directory itself) match; update the Any(...) predicate to perform these
normalizations and the separator-aware comparison.

}
}
2 changes: 1 addition & 1 deletion SysManager/SysManager/Services/UpdateService.cs
Original file line number Diff line number Diff line change
Expand Up @@ -141,7 +141,7 @@ public async Task<IReadOnlyList<ReleaseInfo>> GetRecentAsync(int count = 10, Can
Environment.GetFolderPath(Environment.SpecialFolder.LocalApplicationData),
"SysManager", "updates");
Directory.CreateDirectory(dir);
var target = Path.Combine(dir, $"SysManager-{rel.Version}.exe");
var target = Path.Join(dir, $"SysManager-{rel.Version}.exe");

// SEC-M2: Skip re-download only if we have a cached hash that matches.
// File size alone is insufficient — an attacker could replace the binary
Expand Down
50 changes: 25 additions & 25 deletions SysManager/SysManager/Services/WindowsFeaturesService.cs
Original file line number Diff line number Diff line change
Expand Up @@ -114,32 +114,32 @@ void Collect(PowerShellLine l)
/// <summary>Parses the pipe-delimited feature list output.</summary>
internal static List<WindowsFeature> ParseFeatureList(List<string> lines)
{
var features = new List<WindowsFeature>();

foreach (var line in lines.Where(l => !string.IsNullOrWhiteSpace(l)))
{
var parts = line.Split('|', 2);
if (parts.Length < 2) continue;

var name = parts[0].Trim();
var state = parts[1].Trim();

if (string.IsNullOrWhiteSpace(name)) continue;

var isEnabled = state.Equals("Enabled", StringComparison.OrdinalIgnoreCase);

features.Add(new WindowsFeature
return lines
.Where(l => !string.IsNullOrWhiteSpace(l))
.Select(line =>
{
Name = name,
DisplayName = HumanizeName(name),
IsEnabled = isEnabled,
Category = WindowsFeature.CategorizeFeature(name)
});
}

return features.OrderBy(f => f.Category)
.ThenBy(f => f.DisplayName)
.ToList();
var parts = line.Split('|', 2);
if (parts.Length < 2) return null;

var name = parts[0].Trim();
var state = parts[1].Trim();

if (string.IsNullOrWhiteSpace(name)) return null;

var isEnabled = state.Equals("Enabled", StringComparison.OrdinalIgnoreCase);

return new WindowsFeature
{
Name = name,
DisplayName = HumanizeName(name),
IsEnabled = isEnabled,
Category = WindowsFeature.CategorizeFeature(name)
};
})
.Where(f => f is not null)
.OrderBy(f => f!.Category)
.ThenBy(f => f!.DisplayName)
.ToList()!;
}

/// <summary>
Expand Down
11 changes: 9 additions & 2 deletions SysManager/SysManager/ViewModels/ProcessManagerViewModel.cs
Original file line number Diff line number Diff line change
Expand Up @@ -160,9 +160,16 @@ private void ApplyFilter()
private static string FormatSize(long bytes) => Helpers.FormatHelper.FormatSize(bytes);

private static bool MatchesFilter(ProcessEntry p, string filter) =>
p.Name.Contains(filter, StringComparison.OrdinalIgnoreCase) ||
MatchesName(p, filter) || MatchesDescription(p, filter) || MatchesPid(p, filter);

private static bool MatchesName(ProcessEntry p, string filter) =>
p.Name.Contains(filter, StringComparison.OrdinalIgnoreCase);

private static bool MatchesDescription(ProcessEntry p, string filter) =>
(p.Description?.Contains(filter, StringComparison.OrdinalIgnoreCase) ?? false) ||
(p.PlainDescription?.Contains(filter, StringComparison.OrdinalIgnoreCase) ?? false) ||
(p.Category?.Contains(filter, StringComparison.OrdinalIgnoreCase) ?? false) ||
(p.Category?.Contains(filter, StringComparison.OrdinalIgnoreCase) ?? false);

private static bool MatchesPid(ProcessEntry p, string filter) =>
p.Pid.ToString().Contains(filter);
}
20 changes: 18 additions & 2 deletions SysManager/SysManager/ViewModels/ViewModelBase.cs
Original file line number Diff line number Diff line change
Expand Up @@ -31,9 +31,25 @@ protected static async void InitializeAsync(Func<Task> asyncAction, [System.Runt
{
// Expected during shutdown — no action needed.
}
catch (Exception ex)
catch (InvalidOperationException ex)
{
Log.Error(ex, "Unhandled exception in async initialization of {Caller}", callerName);
Log.Error(ex, "Invalid operation in async initialization of {Caller}", callerName);
}
catch (UnauthorizedAccessException ex)
{
Log.Error(ex, "Access denied in async initialization of {Caller}", callerName);
}
catch (System.IO.IOException ex)
{
Log.Error(ex, "I/O error in async initialization of {Caller}", callerName);
}
catch (System.Net.Http.HttpRequestException ex)
{
Log.Error(ex, "Network error in async initialization of {Caller}", callerName);
}
catch (TimeoutException ex)
{
Log.Error(ex, "Timeout in async initialization of {Caller}", callerName);
}
Comment on lines +34 to 53

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Unhandled exception gap reintroduced in InitializeAsync

Line 34 onward narrows catches, but without a final fallback, async void can crash the app for unexpected exceptions. This breaks the safety guarantee documented for InitializeAsync.

Suggested patch
         catch (TimeoutException ex)
         {
             Log.Error(ex, "Timeout in async initialization of {Caller}", callerName);
         }
+        catch (Exception ex)
+        {
+            Log.Error(ex, "Unexpected error in async initialization of {Caller}", callerName);
+        }
📝 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.

Suggested change
catch (InvalidOperationException ex)
{
Log.Error(ex, "Unhandled exception in async initialization of {Caller}", callerName);
Log.Error(ex, "Invalid operation in async initialization of {Caller}", callerName);
}
catch (UnauthorizedAccessException ex)
{
Log.Error(ex, "Access denied in async initialization of {Caller}", callerName);
}
catch (System.IO.IOException ex)
{
Log.Error(ex, "I/O error in async initialization of {Caller}", callerName);
}
catch (System.Net.Http.HttpRequestException ex)
{
Log.Error(ex, "Network error in async initialization of {Caller}", callerName);
}
catch (TimeoutException ex)
{
Log.Error(ex, "Timeout in async initialization of {Caller}", callerName);
}
catch (InvalidOperationException ex)
{
Log.Error(ex, "Invalid operation in async initialization of {Caller}", callerName);
}
catch (UnauthorizedAccessException ex)
{
Log.Error(ex, "Access denied in async initialization of {Caller}", callerName);
}
catch (System.IO.IOException ex)
{
Log.Error(ex, "I/O error in async initialization of {Caller}", callerName);
}
catch (System.Net.Http.HttpRequestException ex)
{
Log.Error(ex, "Network error in async initialization of {Caller}", callerName);
}
catch (TimeoutException ex)
{
Log.Error(ex, "Timeout in async initialization of {Caller}", callerName);
}
catch (Exception ex)
{
Log.Error(ex, "Unexpected error in async initialization of {Caller}", callerName);
}
🤖 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/ViewModelBase.cs` around lines 34 - 53, The
narrow exception handlers in ViewModelBase.InitializeAsync leave unhandled
exceptions from its async void path; add a final catch (Exception ex) at the end
of the try/catch chain inside InitializeAsync to log the unexpected exception
(using Log.Error with context including callerName) and swallow or handle it to
preserve the documented safety guarantee for async void initialization; ensure
the new catch comes after the existing specific catches in the InitializeAsync
method of ViewModelBase so no unexpected exception can escape.

}

Expand Down
Loading