Skip to content

fix: dynamic user-path sanitization in LogService and cache FontFamily in MarkdownTextBlock #444

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
laurentiu021 merged 1 commit into from
May 20, 2026
Merged

fix: dynamic user-path sanitization in LogService and cache FontFamily in MarkdownTextBlock #444

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
6 changes: 6 additions & 0 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,12 @@ adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
now uses `long` before clamping to the 0–20 deduction cap.
- **SpeedTestService** — documented pinned Ookla CLI version (`1.2.0`) with
maintenance comment explaining update procedure and Authenticode verification.
- **LogService** — path sanitization regex now dynamically derives the user
profile directory from `Environment.GetFolderPath` instead of assuming a
hardcoded `<drive>:\Users\` pattern; falls back to the generic regex if the
environment variable is unavailable.
- **MarkdownTextBlock** — cached `FontFamily("Consolas")` as a static field to
eliminate per-render allocation in code span formatting.

### Added
- **ServicesViewModelTests** — 20 unit tests covering ApplyFilter logic: category
Expand Down
5 changes: 4 additions & 1 deletion SysManager/SysManager/Helpers/MarkdownTextBlock.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,9 @@ private static void OnMarkdownChanged(DependencyObject d, DependencyPropertyChan
private static readonly Regex InlineFormattingRegex = new(
@"\*\*(.+?)\*\*|`([^`]+)`", RegexOptions.Compiled);

// PERF-007: Cache FontFamily to avoid allocating a new instance per code span render.
private static readonly FontFamily CodeFontFamily = new("Consolas");

/// <summary>
/// Parses a single line for **bold** and `code` inline formatting
/// and appends the resulting Inlines to the TextBlock.
Expand All @@ -111,7 +114,7 @@ private static void AddFormattedText(TextBlock tb, string text)
// `code`
tb.Inlines.Add(new Run(m.Groups[2].Value)
{
FontFamily = new FontFamily("Consolas"),
FontFamily = CodeFontFamily,
Foreground = Application.Current?.TryFindResource("Accent") as Brush ?? Brushes.CornflowerBlue
});
}
Expand Down
24 changes: 20 additions & 4 deletions SysManager/SysManager/Services/LogService.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,10 +16,26 @@ public static class LogService
public static string LogDir { get; } =
Path.Combine(Environment.GetFolderPath(Environment.SpecialFolder.LocalApplicationData), "SysManager", "logs");

// Matches <DriveLetter>:\Users\<username>\ and replaces the username with [user].
private static readonly Regex UserPathRegex = new(
@"(?i)([A-Z]:\\Users\\)[^\\]+",
RegexOptions.Compiled);
// Dynamically build regex from the actual user profile parent directory
// (e.g. C:\Users) so it works even if Windows is installed on a non-standard
// drive or the Users folder has a custom path.
private static readonly Regex UserPathRegex = BuildUserPathRegex();

private static Regex BuildUserPathRegex()
{
var userProfile = Environment.GetFolderPath(Environment.SpecialFolder.UserProfile);
if (!string.IsNullOrEmpty(userProfile))
{
var usersDir = Path.GetDirectoryName(userProfile);
if (!string.IsNullOrEmpty(usersDir))
{
var escaped = Regex.Escape(usersDir + @"\");
return new Regex($@"(?i)({escaped})[^\\]+", RegexOptions.Compiled);
}
}
// Fallback: match any drive letter followed by \Users\<username>
return new Regex(@"(?i)([A-Z]:\\Users\\)[^\\]+", RegexOptions.Compiled);
}
Comment on lines +24 to +38

@coderabbitai coderabbitai Bot May 20, 2026

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 .NET on Windows, what exact values can Environment.GetFolderPath(Environment.SpecialFolder.UserProfile) return for LocalSystem/NetworkService accounts, and can it be non-empty while not pointing to a standard multi-user root like X:\Users\ ?

💡 Result:

In.NET on Windows, Environment.GetFolderPath(Environment.SpecialFolder.UserProfile) returns the “user profile” path for whatever security context the current process is running under, and it can be empty if the corresponding folder does not physically exist. 1) Exact non-empty values for LocalSystem - For the LocalSystem service account, the returned UserProfile is typically: C:\Windows\System32\Config\SystemProfile This matches reports from.NET code running in a service context where Environment.GetFolderPath(UserProfile) yields C:\Windows\system32\config\systemprofile (and similarly used when building AppData paths). [1][2] 2) NetworkService / LocalService - The.NET/environment APIs compute the UserProfile path based on the running account’s profile. Microsoft’s LocalSystem documentation explains that LocalSystem is a special service account used by the SCM and notes the token/SID context, and it contrasts LocalSystem with LocalService/NetworkService as alternatives for services. [3] - However, in the sources retrieved here, I did not find a similarly explicit, authoritative “exact string” mapping for NetworkService specifically (e.g., the exact folder path string) comparable to the LocalSystem/SystemProfile mapping above. 3) Can it be non-empty while not pointing to a standard multi-user root like X:\Users? - Yes, at least for LocalSystem: the returned path is not under X:\Users\…; it is under the system profile directory (C:\Windows\System32\Config\SystemProfile). [1][2] - Empty-string behavior: Environment.GetFolderPath returns an empty string (“”) if the requested special-folder path does not physically exist on the computer. [4] Bottom line - LocalSystem: non-empty UserProfile path is expected to be C:\Windows\System32\Config\SystemProfile, i.e., non-X:\Users and therefore can meet your “non-empty while not pointing to a standard multi-user root” condition. [1][2] - NetworkService: non-empty is possible, but I cannot, from the sourced material gathered here, state the exact value it will return for NetworkService. If you tell me your exact OS version (e.g., Windows Server 2008 R2 vs newer) and whether the service is running under LocalSystem vs NetworkService vs a custom domain/service account, I can try one more targeted lookup to pin down the exact NetworkService UserProfile string for Windows Server +.NET version in your scenario.

Citations:

🏁 Script executed:

cat -n SysManager/SysManager/Services/LogService.cs | head -100

Repository: laurentiu021/SystemManager

Length of output: 3009

🏁 Script executed:

rg "UserPathRegex|BuildUserPathRegex" SysManager/SysManager/Services/LogService.cs -A 3 -B 2

Repository: laurentiu021/SystemManager

Length of output: 593

🏁 Script executed:

rg "Regex\.Replace" SysManager/ -A 1 -B 1 -t cs

Repository: laurentiu021/SystemManager

Length of output: 708

🌐 Web query:

.NET Regex.Replace named capture group replacement syntax ${groupname} C# documentation

💡 Result:

In.NET, the syntax to reference a named capture group within a replacement string is ${name} [1][2]. When using the Regex.Replace method, you can include the substring matched by a named capturing group (defined in your pattern as (?...)) by placing the group name inside the ${} substitution element [1][2]. Key behaviors of this syntax include: - If the name corresponds to a valid named capturing group defined in the regular expression,.NET substitutes the last substring matched by that group [1][2]. - If the name consists only of digits and does not match a named group,.NET interprets it as a numbered backreference (e.g., ${1}) [1][2]. - If the name is neither a valid named group nor a valid numbered group, it is treated as a literal string [1][2]. Example usage: string input = "First Second"; string pattern = @"(?First) (?Second)"; string replacement = "${second} ${first}"; string result = Regex.Replace(input, pattern, replacement); // Result: "Second First" [3]

Citations:

Fallback pattern is bypassed when UserProfile is non-empty but non-standard—user paths can leak in logs.

When the service runs under LocalSystem (or other non-interactive accounts), Environment.GetFolderPath(Environment.SpecialFolder.UserProfile) returns a non-empty path like C:\Windows\System32\Config\SystemProfile. This causes an early return on line 33 with a pattern that only matches paths under that system directory, never reaching the fallback on line 37. If logs contain paths like C:\Users\username\..., they won't be redacted.

Combine both patterns using alternation and return a single regex:

Suggested fix 
 private static Regex BuildUserPathRegex()
 {
     var userProfile = Environment.GetFolderPath(Environment.SpecialFolder.UserProfile);
+    string pattern = @"(?i)(?<prefix>[A-Z]:\\Users\\)[^\\]+";
+
     if (!string.IsNullOrEmpty(userProfile))
     {
         var usersDir = Path.GetDirectoryName(userProfile);
         if (!string.IsNullOrEmpty(usersDir))
         {
             var escaped = Regex.Escape(usersDir + @"\");
-            return new Regex($@"(?i)({escaped})[^\\]+", RegexOptions.Compiled);
+            pattern = $@"(?i)(?<prefix>{escaped}|[A-Z]:\\Users\\)[^\\]+";
         }
     }
-    // Fallback: match any drive letter followed by \Users\<username>
-    return new Regex(@"(?i)([A-Z]:\\Users\\)[^\\]+", RegexOptions.Compiled);
+
+    return new Regex(pattern, RegexOptions.Compiled);
 }

Also update line 68:

-        return UserPathRegex.Replace(path, "$1[user]");
+        return UserPathRegex.Replace(path, "${prefix}[user]");
📝 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
private static Regex BuildUserPathRegex()
{
var userProfile = Environment.GetFolderPath(Environment.SpecialFolder.UserProfile);
if (!string.IsNullOrEmpty(userProfile))
{
var usersDir = Path.GetDirectoryName(userProfile);
if (!string.IsNullOrEmpty(usersDir))
{
var escaped = Regex.Escape(usersDir + @"\");
return new Regex($@"(?i)({escaped})[^\\]+", RegexOptions.Compiled);
}
}
// Fallback: match any drive letter followed by \Users\<username>
return new Regex(@"(?i)([A-Z]:\\Users\\)[^\\]+", RegexOptions.Compiled);
}
private static Regex BuildUserPathRegex()
{
var userProfile = Environment.GetFolderPath(Environment.SpecialFolder.UserProfile);
string pattern = @"(?i)(?<prefix>[A-Z]:\\Users\\)[^\\]+";
if (!string.IsNullOrEmpty(userProfile))
{
var usersDir = Path.GetDirectoryName(userProfile);
if (!string.IsNullOrEmpty(usersDir))
{
var escaped = Regex.Escape(usersDir + @"\");
pattern = $@"(?i)(?<prefix>{escaped}|[A-Z]:\\Users\\)[^\\]+";
}
}
return new Regex(pattern, RegexOptions.Compiled);
}
🤖 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/LogService.cs` around lines 24 - 38,
BuildUserPathRegex currently returns a regex that only matches the discovered
userProfile directory (which can be a system profile) and so misses normal
C:\Users\username paths; change BuildUserPathRegex to build and return a single
Regex that alternates the escaped usersDir-based pattern and the fallback
drive-based pattern (e.g. "(?:<escapedUsersDir>[^\\]+|[A-Z]:\\Users\\[^\\]+)")
with RegexOptions.Compiled and case-insensitive; ensure you escape the usersDir
+ "\" correctly before inserting into the alternation so both patterns are
matched by the single returned Regex used elsewhere (function
BuildUserPathRegex).


public static void Init()
{
Expand Down
Loading