From 12e66299e695c454f4239f22eda00ee17704b965 Mon Sep 17 00:00:00 2001 From: Oliver Kieselbach Date: Wed, 9 Sep 2026 11:54:36 +0200 Subject: [PATCH] feat(rules): allow-list folders under the signed-in user's profile (D-228, D-229) Gather rules and admin-configured diagnostics paths could never reach a folder below a user profile. The %LOGGED_ON_USER_PROFILE% token lifts the C:\Users hard block for AppData\Local and AppData\Roaming, but lifting is not admitting: the path then fell into filePrefixes, which is matched literally against the already-expanded path, and the profile name is not known when that list is authored. The exception only ever worked in unrestricted mode, and no agent test passed a non-null userProfilePath. D-228 introduces userProfileFilePrefixes, holding entries relative to the profile root (AppData\Local\RealmJoin). Both guards resolve them against the detected profile and match segment-bounded; entries that are rooted, contain "..", or sit outside AppData\Local / AppData\Roaming are dropped at load, so configuration can only narrow the hard block's exception, never widen it. The allowlist is anchored to the detected profile, so a rule cannot reach another user's AppData. The AppData subdirectory list existed in both guard classes with the GatherRuleGuards copy unused; it is now one list and DiagnosticsPathGuards delegates. filePrefixes additionally gains C:\ProgramData\RealmJoin\choco\logs. D-229 fixes the pre-flight validators, which must only ever reject what the agent rejects: - the portal returned "allowed" on the lifted hard block without consulting the allowlist, so a rule the agent refuses passed the enable gate and then failed silently on the device with a security_warning; - the MCP validator offered logparser the diagnosticsPathPrefixes list that no collector consults (four paths reported allowed that the agent blocks); - the MCP validator did not expand environment variables at all, reporting %ProgramData% targets as blocked although the device reads them. Measured before changing the portal: no rule in any tenant used the token, so nothing that was working gets disabled. Verified: solution build 0 warnings, agent 2897 tests, web 1257 + tsc + eslint 0, MCP 779, guardrails.json validates against its schema, rules/dist regenerates without drift. The six new agent tests were green against the unchanged guard first, which is what proves the exception was inert. Takes effect on devices only after an agent release: guardrails.json is an EmbeddedResource in the agent assembly. Co-Authored-By: Claude Opus 5 (1M context) --- internal | 2 +- rules/guardrails.json | 6 +- rules/schema/guardrails.schema.json | 10 ++ rules/scripts/combine.js | 8 ++ .../Gather/GatherRuleCollectorGuardTests.cs | 119 ++++++++++++++++++ .../Telemetry/Gather/GuardrailsParityTests.cs | 25 ++++ .../Telemetry/Gather/DiagnosticsPathGuards.cs | 47 ++----- .../Telemetry/Gather/GatherRuleGuards.cs | 116 ++++++++++++++++- .../src/__tests__/rule-validation.test.ts | 52 ++++++++ .../src/rule-authoring.generated.ts | 6 +- .../src/rule-validation.ts | 71 +++++++++-- .../utils/__tests__/guardValidation.test.ts | 28 ++++- .../utils/guardValidation.ts | 57 +++++++-- .../utils/guardrails.generated.ts | 9 ++ 14 files changed, 486 insertions(+), 70 deletions(-) diff --git a/internal b/internal index e94af0c3..3b7f771e 160000 --- a/internal +++ b/internal @@ -1 +1 @@ -Subproject commit e94af0c32e06b5efa875980989f945f960e0331e +Subproject commit 3b7f771e6507a6bda32dedcc646fc7e912889e3d diff --git a/rules/guardrails.json b/rules/guardrails.json index f85b2500..b458eb9c 100644 --- a/rules/guardrails.json +++ b/rules/guardrails.json @@ -94,7 +94,11 @@ "C:\\Windows\\Panther", "C:\\Windows\\SetupDiag", "C:\\ProgramData\\Microsoft\\DiagnosticLogCSP", - "C:\\Windows\\SoftwareDistribution\\ReportingEvents.log" + "C:\\Windows\\SoftwareDistribution\\ReportingEvents.log", + "C:\\ProgramData\\RealmJoin\\choco\\logs" + ], + "userProfileFilePrefixes": [ + "AppData\\Local\\RealmJoin" ], "wmiQueryPrefixes": [ "SELECT * FROM Win32_OperatingSystem", diff --git a/rules/schema/guardrails.schema.json b/rules/schema/guardrails.schema.json index 886b80c3..76d7869b 100644 --- a/rules/schema/guardrails.schema.json +++ b/rules/schema/guardrails.schema.json @@ -8,6 +8,16 @@ "$schema": { "type": "string" }, "registryPrefixes": { "$ref": "#/$defs/categorizedList" }, "filePrefixes": { "$ref": "#/$defs/stringList" }, + "userProfileFilePrefixes": { + "description": "Folders under the signed-in user's profile that file collectors and diagnostics paths may read, RELATIVE to the profile root (e.g. \"AppData\\\\Local\\\\Vendor\"). Resolved against %LOGGED_ON_USER_PROFILE% at match time. Entries outside AppData\\Local / AppData\\Roaming are dropped at load: the C:\\Users hard block never admits them, so this list can only ever narrow what the block's exception already permits.", + "type": "array", + "items": { + "allOf": [ + { "$ref": "#/$defs/safeString" }, + { "pattern": "^AppData\\\\(Local|Roaming)(\\\\|$)" } + ] + } + }, "wmiQueryPrefixes": { "$ref": "#/$defs/stringList" }, "eventLogChannels": { "$ref": "#/$defs/categorizedList" }, "blockedEventLogChannels": { "$ref": "#/$defs/stringList" }, diff --git a/rules/scripts/combine.js b/rules/scripts/combine.js index bfac3d8b..e8771e77 100644 --- a/rules/scripts/combine.js +++ b/rules/scripts/combine.js @@ -208,6 +208,14 @@ export const ALLOWED_FILE_PREFIXES: readonly string[] = [ ${flatArray(guardrails.filePrefixes)} ]; +// Folders under the signed-in user's profile, RELATIVE to the profile root. The agent +// resolves %LOGGED_ON_USER_PROFILE% at runtime; the portal validates against a placeholder +// profile. These cannot live in ALLOWED_FILE_PREFIXES — that list is matched literally +// against the already-expanded path, where the user name is unknown at authoring time. +export const USER_PROFILE_FILE_PREFIXES: readonly string[] = [ +${flatArray(guardrails.userProfileFilePrefixes ?? [])} +]; + export const ALLOWED_WMI_QUERY_PREFIXES: readonly string[] = [ ${flatArray(guardrails.wmiQueryPrefixes)} ]; diff --git a/src/Agent/AutopilotMonitor.Agent.V2.Core.Tests/Monitoring/Gather/GatherRuleCollectorGuardTests.cs b/src/Agent/AutopilotMonitor.Agent.V2.Core.Tests/Monitoring/Gather/GatherRuleCollectorGuardTests.cs index f1d44e1a..c38d9f0b 100644 --- a/src/Agent/AutopilotMonitor.Agent.V2.Core.Tests/Monitoring/Gather/GatherRuleCollectorGuardTests.cs +++ b/src/Agent/AutopilotMonitor.Agent.V2.Core.Tests/Monitoring/Gather/GatherRuleCollectorGuardTests.cs @@ -456,5 +456,124 @@ namespace AutopilotMonitor.Agent.V2.Core.Tests.Monitoring.Gather SingleSecurityWarning(); } + + // ------------------------------------------------------------------- + // %LOGGED_ON_USER_PROFILE% exception (userProfilePath overload) + // + // The token lifts the C:\Users hard block for AppData\Local and + // AppData\Roaming — and nothing more. The allowlist still decides, + // which is what the cases below pin down: the exception is a gate the + // allowlist has to open, never an allowance of its own. + // ------------------------------------------------------------------- + + private const string TestProfile = @"C:\Users\TestUser"; + + [Fact] + public void UserProfile_AppDataPath_WithoutAllowlistEntry_IsStillBlocked() + { + // The lifted hard block alone admits nothing: without a matching + // userProfileFilePrefixes entry the path falls through to the + // allowlist and is refused there. + Assert.False(GatherRuleGuards.IsFilePathAllowed( + TestProfile + @"\AppData\Local\UnlistedVendor\app.log", + unrestrictedMode: false, userProfilePath: TestProfile)); + } + + [Fact] + public void UserProfile_OutsideAppData_IsBlockedByTheHardBlock() + { + foreach (var target in new[] { @"\Desktop\secret.txt", @"\Documents\notes.txt", @"\ntuser.dat" }) + { + Assert.False(GatherRuleGuards.IsFilePathAllowed( + TestProfile + target, unrestrictedMode: false, userProfilePath: TestProfile)); + } + } + + [Fact] + public void UserProfile_NullProfile_KeepsTheUsersHardBlock() + { + // No interactive user detected — the exception cannot apply and the + // path stays under the plain C:\Users block. + Assert.False(GatherRuleGuards.IsFilePathAllowed( + @"C:\Users\TestUser\AppData\Local\UnlistedVendor\app.log", + unrestrictedMode: false, userProfilePath: null)); + } + + [Fact] + public void UserProfile_TraversalOutOfAppData_IsBlocked() + { + // Spelling Documents as a traversal out of AppData must not survive + // Path.GetFullPath normalisation. + const string traversal = @"C:\Users\TestUser\AppData\Local\..\..\Documents\secret.txt"; + + Assert.Equal(@"C:\Users\TestUser\Documents\secret.txt", Path.GetFullPath(traversal)); + Assert.False(GatherRuleGuards.IsFilePathAllowed( + traversal, unrestrictedMode: false, userProfilePath: TestProfile)); + } + + [Fact] + public void UserProfile_ForeignProfile_IsBlockedEvenUnderAppData() + { + // The exception is scoped to the DETECTED profile — a rule must not + // reach a second user's AppData by spelling the path out. + Assert.False(GatherRuleGuards.IsFilePathAllowed( + @"C:\Users\OtherUser\AppData\Local\UnlistedVendor\app.log", + unrestrictedMode: false, userProfilePath: TestProfile)); + } + + [Fact] + public void UserProfile_AllowlistedVendorFolder_IsAllowed() + { + // AppData\Local\RealmJoin is on userProfileFilePrefixes: the file itself, a + // subfolder below it, and the folder itself all pass. + foreach (var target in new[] { @"\tray.log", @"\Logs\rj.log", "" }) + { + Assert.True(GatherRuleGuards.IsFilePathAllowed( + TestProfile + @"\AppData\Local\RealmJoin" + target, + unrestrictedMode: false, userProfilePath: TestProfile), + $"expected allowed: ...\\RealmJoin{target}"); + } + } + + [Fact] + public void UserProfile_SiblingOfAnAllowlistedVendorFolder_IsBlocked() + { + // Segment-bounded: "RealmJoin" must not admit "RealmJoinSomethingElse". + Assert.False(GatherRuleGuards.IsFilePathAllowed( + TestProfile + @"\AppData\Local\RealmJoinSomethingElse\tray.log", + unrestrictedMode: false, userProfilePath: TestProfile)); + } + + [Fact] + public void UserProfile_AllowlistedVendorFolder_InAForeignProfile_IsBlocked() + { + // The allowlist is anchored to the DETECTED profile, not to C:\Users\*. + Assert.False(GatherRuleGuards.IsFilePathAllowed( + @"C:\Users\OtherUser\AppData\Local\RealmJoin\tray.log", + unrestrictedMode: false, userProfilePath: TestProfile)); + } + + [Fact] + public void UserProfile_AllowlistedVendorFolder_WithoutADetectedUser_IsBlocked() + { + Assert.False(GatherRuleGuards.IsFilePathAllowed( + @"C:\Users\TestUser\AppData\Local\RealmJoin\tray.log", + unrestrictedMode: false, userProfilePath: null)); + } + + [Fact] + public void UserProfile_AppDataPath_IsAllowedInUnrestrictedMode() + { + // Where the exception does bite today: unrestricted mode returns + // before the allowlist, so the lifted hard block is the whole story. + Assert.True(GatherRuleGuards.IsFilePathAllowed( + TestProfile + @"\AppData\Local\UnlistedVendor\app.log", + unrestrictedMode: true, userProfilePath: TestProfile)); + + // ...but only under AppData — Documents stays blocked even there. + Assert.False(GatherRuleGuards.IsFilePathAllowed( + TestProfile + @"\Documents\secret.txt", + unrestrictedMode: true, userProfilePath: TestProfile)); + } } } diff --git a/src/Agent/AutopilotMonitor.Agent.V2.Core.Tests/Monitoring/Telemetry/Gather/GuardrailsParityTests.cs b/src/Agent/AutopilotMonitor.Agent.V2.Core.Tests/Monitoring/Telemetry/Gather/GuardrailsParityTests.cs index 63a3c3b0..27c90b75 100644 --- a/src/Agent/AutopilotMonitor.Agent.V2.Core.Tests/Monitoring/Telemetry/Gather/GuardrailsParityTests.cs +++ b/src/Agent/AutopilotMonitor.Agent.V2.Core.Tests/Monitoring/Telemetry/Gather/GuardrailsParityTests.cs @@ -56,6 +56,31 @@ namespace AutopilotMonitor.Agent.V2.Core.Tests.Monitoring.Telemetry.Gather LoadEmbeddedGuardrails()["maxCommandLength"]!.Value()); } + [Fact] + public void User_profile_prefixes_are_all_accepted_by_the_loader() + { + // The loader drops entries outside AppData\Local / AppData\Roaming (the hard block + // would never release them anyway). A dropped entry is invisible at runtime — it + // just never matches — so a typo in the JSON must fail here instead. + var json = LoadEmbeddedGuardrails()["userProfileFilePrefixes"]?.ToObject() + ?? new string[0]; + + Assert.Equal( + json.Select(p => p.Trim().TrimEnd('\\')).OrderBy(p => p), + GatherRuleGuards.AllowedUserProfileFilePrefixes.OrderBy(p => p)); + } + + [Fact] + public void User_profile_prefixes_stay_under_the_hard_blocks_exception() + { + Assert.All( + GatherRuleGuards.AllowedUserProfileFilePrefixes, + prefix => Assert.Contains( + GatherRuleGuards.AllowedUserProfileSubdirs, + subdir => prefix.StartsWith(subdir, System.StringComparison.OrdinalIgnoreCase) && + (prefix.Length == subdir.Length || prefix[subdir.Length] == '\\'))); + } + [Fact] public void Blocked_event_log_channels_match_code_constants() { diff --git a/src/Agent/AutopilotMonitor.Agent.V2.Core/Monitoring/Telemetry/Gather/DiagnosticsPathGuards.cs b/src/Agent/AutopilotMonitor.Agent.V2.Core/Monitoring/Telemetry/Gather/DiagnosticsPathGuards.cs index 34e1b445..bf4c5878 100644 --- a/src/Agent/AutopilotMonitor.Agent.V2.Core/Monitoring/Telemetry/Gather/DiagnosticsPathGuards.cs +++ b/src/Agent/AutopilotMonitor.Agent.V2.Core/Monitoring/Telemetry/Gather/DiagnosticsPathGuards.cs @@ -70,12 +70,8 @@ namespace AutopilotMonitor.Agent.V2.Core.Monitoring.Telemetry.Gather @"C:\Windows\System32\config", // SAM, SECURITY, SYSTEM hives }; - // Allowed subdirectories under a user profile (used with %LOGGED_ON_USER_PROFILE% token) - private static readonly string[] AllowedUserProfileSubdirs = new[] - { - @"AppData\Local", - @"AppData\Roaming", - }; + // The allowed user-profile subdirectories and the vendor allowlist under them live in + // GatherRuleGuards, which owns the embedded guardrails.json — one copy for both guards. /// /// Returns true if the given path is allowed for diagnostics collection. @@ -135,7 +131,7 @@ namespace AutopilotMonitor.Agent.V2.Core.Monitoring.Telemetry.Gather (normalizedDir.Length == BlockedUsersPrefix.Length || normalizedDir[BlockedUsersPrefix.Length] == Path.DirectorySeparatorChar)) { - if (!IsUserProfileSubpathAllowed(normalizedDir, userProfilePath)) + if (!GatherRuleGuards.IsUserProfileSubpathAllowed(normalizedDir, userProfilePath)) return false; } @@ -169,7 +165,10 @@ namespace AutopilotMonitor.Agent.V2.Core.Monitoring.Telemetry.Gather } } - return false; + // Vendor folders under the signed-in user's profile — same allowlist the file + // collectors use, since it answers the same question: which folders below a + // profile carry enrollment-relevant logs rather than the user's own data. + return GatherRuleGuards.IsUserProfilePathOnAllowlist(normalizedDir, userProfilePath); } catch { @@ -178,38 +177,6 @@ namespace AutopilotMonitor.Agent.V2.Core.Monitoring.Telemetry.Gather } } - /// - /// Returns true if the normalized path falls under one of the allowed subdirectories - /// of the detected user profile (AppData\Local, AppData\Roaming). - /// Only applies when %LOGGED_ON_USER_PROFILE% was used to resolve the path. - /// - internal static bool IsUserProfileSubpathAllowed(string normalizedPath, string userProfilePath) - { - if (string.IsNullOrEmpty(userProfilePath)) - return false; - - try - { - var normalizedProfile = Path.GetFullPath(userProfilePath); - foreach (var subdir in AllowedUserProfileSubdirs) - { - var allowedPrefix = Path.GetFullPath(Path.Combine(normalizedProfile, subdir)); - if (normalizedPath.StartsWith(allowedPrefix, StringComparison.OrdinalIgnoreCase) && - (normalizedPath.Length == allowedPrefix.Length || - normalizedPath[allowedPrefix.Length] == Path.DirectorySeparatorChar)) - { - return true; - } - } - } - catch - { - // Path normalization failure — deny - } - - return false; - } - /// /// Checks whether any component of the path (directories or file) is a symlink, /// junction, or other reparse point. Returns true if a reparse point is detected. diff --git a/src/Agent/AutopilotMonitor.Agent.V2.Core/Monitoring/Telemetry/Gather/GatherRuleGuards.cs b/src/Agent/AutopilotMonitor.Agent.V2.Core/Monitoring/Telemetry/Gather/GatherRuleGuards.cs index 9a5b3659..5f7618f8 100644 --- a/src/Agent/AutopilotMonitor.Agent.V2.Core/Monitoring/Telemetry/Gather/GatherRuleGuards.cs +++ b/src/Agent/AutopilotMonitor.Agent.V2.Core/Monitoring/Telemetry/Gather/GatherRuleGuards.cs @@ -32,6 +32,16 @@ namespace AutopilotMonitor.Agent.V2.Core.Monitoring.Telemetry.Gather // ----------------------------------------------------------------------- public static readonly IReadOnlyList AllowedRegistryPrefixes; public static readonly IReadOnlyList AllowedFilePrefixes; + + /// + /// Folders under the signed-in user's profile that file collectors and + /// admin-configured diagnostics paths may read, RELATIVE to the profile root + /// (e.g. AppData\Local\Vendor). Resolved against the path + /// %LOGGED_ON_USER_PROFILE% produced, at match time — the profile name is not + /// known when the list is authored, which is why these entries cannot live in + /// (matched literally against the expanded path). + /// + public static readonly IReadOnlyList AllowedUserProfileFilePrefixes; public static readonly IReadOnlyList AllowedWmiQueryPrefixes; public static readonly IReadOnlyCollection AllowedCommands; public static readonly IReadOnlyList AllowedEventLogChannels; @@ -63,8 +73,10 @@ namespace AutopilotMonitor.Agent.V2.Core.Monitoring.Telemetry.Gather Path.GetFullPath(@"C:\Windows\System32\config"), // SAM, SECURITY, SYSTEM hives }; - // Allowed subdirectories under a user profile (used with %LOGGED_ON_USER_PROFILE% token) - private static readonly string[] AllowedUserProfileSubdirs = new[] + // The only subdirectories of a user profile the C:\Users hard block will ever + // release (used with %LOGGED_ON_USER_PROFILE%). Single copy for both guards — + // DiagnosticsPathGuards delegates here. + internal static readonly string[] AllowedUserProfileSubdirs = new[] { @"AppData\Local", @"AppData\Roaming", @@ -118,6 +130,7 @@ namespace AutopilotMonitor.Agent.V2.Core.Monitoring.Telemetry.Gather AllowedRegistryPrefixes = FlattenCategorized(obj, "registryPrefixes", "prefixes"); AllowedFilePrefixes = obj["filePrefixes"]?.ToObject>() ?? new List(); + AllowedUserProfileFilePrefixes = LoadUserProfilePrefixes(obj); AllowedWmiQueryPrefixes = obj["wmiQueryPrefixes"]?.ToObject>() ?? new List(); var commands = FlattenCategorized(obj, "allowedCommands", "commands"); @@ -136,6 +149,7 @@ namespace AutopilotMonitor.Agent.V2.Core.Monitoring.Telemetry.Gather AllowedRegistryPrefixes = new List(); AllowedFilePrefixes = new List(); + AllowedUserProfileFilePrefixes = new List(); AllowedWmiQueryPrefixes = new List(); AllowedCommands = new HashSet(StringComparer.OrdinalIgnoreCase); AllowedEventLogChannels = new List(); @@ -143,6 +157,43 @@ namespace AutopilotMonitor.Agent.V2.Core.Monitoring.Telemetry.Gather ResidualWmiPrefixes = new List(); } + /// + /// Reads userProfileFilePrefixes and drops every entry that is rooted, carries a + /// traversal, or does not sit under one of . + /// The C:\Users hard block releases nothing outside AppData, so such an entry could only + /// ever be dead text — dropping it at load keeps the list honest instead of silently + /// ineffective, and no configuration can widen the hard block's exception. + /// + private static List LoadUserProfilePrefixes(JObject obj) + { + var raw = obj["userProfileFilePrefixes"]?.ToObject>() ?? new List(); + var result = new List(); + + foreach (var entry in raw) + { + if (string.IsNullOrWhiteSpace(entry)) + continue; + + var trimmed = entry.Trim().TrimEnd('\\'); + if (Path.IsPathRooted(trimmed)) + continue; + if (trimmed.IndexOf("..", StringComparison.Ordinal) >= 0) + continue; + + foreach (var subdir in AllowedUserProfileSubdirs) + { + if (trimmed.StartsWith(subdir, StringComparison.OrdinalIgnoreCase) && + (trimmed.Length == subdir.Length || trimmed[subdir.Length] == '\\')) + { + result.Add(trimmed); + break; + } + } + } + + return result; + } + private static void DeriveWmiClassAllowlist( IReadOnlyList entries, out HashSet classes, @@ -265,12 +316,13 @@ namespace AutopilotMonitor.Agent.V2.Core.Monitoring.Telemetry.Gather // C:\Users block always applies (even in unrestricted mode) // Exception: paths under \AppData\Local or AppData\Roaming - // are allowed when the %LOGGED_ON_USER_PROFILE% token was used. + // are allowed when the %LOGGED_ON_USER_PROFILE% token was used. The exception + // only LIFTS the block — in restricted mode the allowlist below still decides. if (normalizedPath.StartsWith(BlockedUsersPrefix, StringComparison.OrdinalIgnoreCase) && (normalizedPath.Length == BlockedUsersPrefix.Length || normalizedPath[BlockedUsersPrefix.Length] == Path.DirectorySeparatorChar)) { - if (!DiagnosticsPathGuards.IsUserProfileSubpathAllowed(normalizedPath, userProfilePath)) + if (!IsUserProfileSubpathAllowed(normalizedPath, userProfilePath)) return false; } @@ -288,9 +340,61 @@ namespace AutopilotMonitor.Agent.V2.Core.Monitoring.Telemetry.Gather if (unrestrictedMode) return true; - return AllowedFilePrefixes.Any(prefix => + if (AllowedFilePrefixes.Any(prefix => normalizedPath.StartsWith(prefix, StringComparison.OrdinalIgnoreCase) && - (normalizedPath.Length == prefix.Length || normalizedPath[prefix.Length] == '\\')); + (normalizedPath.Length == prefix.Length || normalizedPath[prefix.Length] == '\\'))) + { + return true; + } + + // Vendor folders under the signed-in user's profile. Only reachable when the rule + // spelled %LOGGED_ON_USER_PROFILE% and the resolver found an interactive user — + // a null profile denies, so a machine with nobody signed in cannot widen anything. + return IsUserProfilePathOnAllowlist(normalizedPath, userProfilePath); + } + + /// + /// True when the normalized path sits under one of + /// of the detected user profile. This is the C:\Users hard block's only exception; it + /// releases the block and nothing else. A null profile denies. + /// + internal static bool IsUserProfileSubpathAllowed(string normalizedPath, string userProfilePath) + => AllowedUserProfileSubdirs.Any(subdir => + MatchesProfileRelativePrefix(normalizedPath, userProfilePath, subdir)); + + /// + /// True when the normalized path is admitted by an entry of + /// under the detected user profile. + /// + internal static bool IsUserProfilePathOnAllowlist(string normalizedPath, string userProfilePath) + => AllowedUserProfileFilePrefixes.Any(prefix => + MatchesProfileRelativePrefix(normalizedPath, userProfilePath, prefix)); + + /// + /// Segment-bounded match of against + /// <profile>\<relativePrefix>. Both sides go through Path.GetFullPath, so a + /// traversal spelled into either one cannot slip past the boundary check. + /// + private static bool MatchesProfileRelativePrefix( + string normalizedPath, string userProfilePath, string relativePrefix) + { + if (string.IsNullOrEmpty(normalizedPath) || string.IsNullOrEmpty(userProfilePath)) + return false; + + try + { + var allowedPrefix = Path.GetFullPath( + Path.Combine(Path.GetFullPath(userProfilePath), relativePrefix)); + + return normalizedPath.StartsWith(allowedPrefix, StringComparison.OrdinalIgnoreCase) && + (normalizedPath.Length == allowedPrefix.Length || + normalizedPath[allowedPrefix.Length] == Path.DirectorySeparatorChar); + } + catch + { + // Path normalization failure — deny. + return false; + } } /// diff --git a/src/McpServer/autopilot-monitor-mcp/src/__tests__/rule-validation.test.ts b/src/McpServer/autopilot-monitor-mcp/src/__tests__/rule-validation.test.ts index 8e70bafc..597bea83 100644 --- a/src/McpServer/autopilot-monitor-mcp/src/__tests__/rule-validation.test.ts +++ b/src/McpServer/autopilot-monitor-mcp/src/__tests__/rule-validation.test.ts @@ -113,6 +113,58 @@ describe('gather guardrails (agent matching semantics)', () => { expect(errors(validateRuleDraft(bad).findings).some((m) => m.includes('hard-blocked'))).toBe(true); }); + // %LOGGED_ON_USER_PROFILE% releases the C:\Users hard block for AppData\Local and + // AppData\Roaming — and only releases it. The allowlist still decides, which is what + // the agent does (GatherRuleGuards.IsFilePathAllowed) and what the portal shows. + it('accepts a userProfileFilePrefixes folder below %LOGGED_ON_USER_PROFILE%', () => { + const ok = validGather(); + ok.collectorType = 'logparser'; + ok.target = '%LOGGED_ON_USER_PROFILE%\\AppData\\Local\\RealmJoin\\tray*.log'; + ok.parameters = { pattern: '(?SSO)', format: 'text' }; + expect(errors(validateRuleDraft(ok).findings).some((m) => m.includes('guardrails'))).toBe(false); + }); + + it('rejects an unlisted vendor folder below %LOGGED_ON_USER_PROFILE%\\AppData', () => { + const bad = validGather(); + bad.collectorType = 'file'; + bad.target = '%LOGGED_ON_USER_PROFILE%\\AppData\\Local\\SomeApp\\data.json'; + expect(errors(validateRuleDraft(bad).findings).some((m) => m.includes('not under any allowed file prefix'))).toBe(true); + }); + + it('rejects %LOGGED_ON_USER_PROFILE% outside AppData as hard-blocked', () => { + const bad = validGather(); + bad.collectorType = 'file'; + bad.target = '%LOGGED_ON_USER_PROFILE%\\Documents\\secret.txt'; + expect(errors(validateRuleDraft(bad).findings).some((m) => m.includes('hard-blocked'))).toBe(true); + }); + + it('rejects a sibling of a userProfileFilePrefixes folder (segment boundary)', () => { + const bad = validGather(); + bad.collectorType = 'file'; + bad.target = '%LOGGED_ON_USER_PROFILE%\\AppData\\Local\\RealmJoinEvil\\tray.log'; + expect(validateRuleDraft(bad).valid).toBe(false); + }); + + // The agent expands environment variables before guarding, so a target spelled with + // %ProgramData% must not be reported as blocked — there are live rules doing this. + it('accepts an allow-listed path spelled with %ProgramData%', () => { + const ok = validGather(); + ok.collectorType = 'logparser'; + ok.target = '%ProgramData%\\Microsoft\\IntuneManagementExtension\\Logs\\HPReports\\HP Image Assistant.log'; + ok.parameters = { pattern: '(?x)', format: 'text' }; + expect(errors(validateRuleDraft(ok).findings).some((m) => m.includes('guardrails'))).toBe(false); + }); + + // diagnosticsPathPrefixes governs admin-configured diagnostics paths, not collectors: + // the agent guards logparser against filePrefixes alone. + it('rejects a logparser target that only diagnosticsPathPrefixes would allow', () => { + const bad = validGather(); + bad.collectorType = 'logparser'; + bad.target = 'C:\\Install\\Log\\setup.log'; + bad.parameters = { pattern: '(?x)', format: 'text' }; + expect(errors(validateRuleDraft(bad).findings).some((m) => m.includes('not under any allowed file prefix'))).toBe(true); + }); + it('rejects the registry-hive store path (System32\\config) as hard-blocked', () => { const bad = validGather(); bad.collectorType = 'file'; diff --git a/src/McpServer/autopilot-monitor-mcp/src/rule-authoring.generated.ts b/src/McpServer/autopilot-monitor-mcp/src/rule-authoring.generated.ts index c0eb3b05..acaf7d6c 100644 --- a/src/McpServer/autopilot-monitor-mcp/src/rule-authoring.generated.ts +++ b/src/McpServer/autopilot-monitor-mcp/src/rule-authoring.generated.ts @@ -870,7 +870,11 @@ export const RULE_GUARDRAILS = { "C:\\Windows\\Panther", "C:\\Windows\\SetupDiag", "C:\\ProgramData\\Microsoft\\DiagnosticLogCSP", - "C:\\Windows\\SoftwareDistribution\\ReportingEvents.log" + "C:\\Windows\\SoftwareDistribution\\ReportingEvents.log", + "C:\\ProgramData\\RealmJoin\\choco\\logs" + ], + "userProfileFilePrefixes": [ + "AppData\\Local\\RealmJoin" ], "wmiQueryPrefixes": [ "SELECT * FROM Win32_OperatingSystem", diff --git a/src/McpServer/autopilot-monitor-mcp/src/rule-validation.ts b/src/McpServer/autopilot-monitor-mcp/src/rule-validation.ts index c8d886e9..46b6e5f7 100644 --- a/src/McpServer/autopilot-monitor-mcp/src/rule-validation.ts +++ b/src/McpServer/autopilot-monitor-mcp/src/rule-validation.ts @@ -103,6 +103,52 @@ function pathPrefixAllowed(target: string, prefixes: readonly string[]): boolean }); } +// The agent expands environment variables and the custom %LOGGED_ON_USER_PROFILE% token +// BEFORE it guards a path (FileCollector/LogParserCollector -> UserProfileResolver -> +// GatherRuleGuards). Judging the raw spelling instead reported targets as blocked that the +// device happily reads (%ProgramData%\...) and made token targets unjudgeable altogether. +// The profile name is unknown here, so the token resolves to a placeholder profile — the +// same device-independent stand-in the portal validator uses. +const COMMON_ENV_VARS: Record = { + '%ProgramData%': 'C:\\ProgramData', + '%SystemRoot%': 'C:\\Windows', + '%windir%': 'C:\\Windows', + '%SystemDrive%': 'C:', +}; +const USER_PROFILE_TOKEN = '%LOGGED_ON_USER_PROFILE%'; +const USER_PROFILE_PLACEHOLDER = 'C:\\Users\\__AGENT_RESOLVED__'; + +/** The only profile subdirectories the C:\Users hard block ever releases. */ +const ALLOWED_USER_PROFILE_SUBDIRS: readonly string[] = ['AppData\\Local', 'AppData\\Roaming']; + +/** Vendor folders below the profile, relative to its root (GatherRuleGuards.AllowedUserProfileFilePrefixes). */ +const USER_PROFILE_FILE_PREFIXES: readonly string[] = RULE_GUARDRAILS.userProfileFilePrefixes ?? []; + +function escapeRegExp(literal: string): string { + return literal.replace(/[.*+?^${}()|[\]\\]/g, '\\$&'); +} + +function expandPathTokens(target: string): string { + let result = target.replace( + new RegExp(escapeRegExp(USER_PROFILE_TOKEN), 'gi'), USER_PROFILE_PLACEHOLDER); + for (const [envVar, replacement] of Object.entries(COMMON_ENV_VARS)) { + result = result.replace(new RegExp(escapeRegExp(envVar), 'gi'), replacement); + } + return result; +} + +/** True when the expanded path sits in the part of the profile the hard block releases. */ +function isUnderUserProfileException(expanded: string): boolean { + return ALLOWED_USER_PROFILE_SUBDIRS.some( + (subdir) => pathPrefixAllowed(expanded, [`${USER_PROFILE_PLACEHOLDER}\\${subdir}`])); +} + +/** True when a userProfileFilePrefixes entry admits the expanded path. */ +function userProfilePrefixAllowed(expanded: string): boolean { + return USER_PROFILE_FILE_PREFIXES.some( + (prefix) => pathPrefixAllowed(expanded, [`${USER_PROFILE_PLACEHOLDER}\\${prefix}`])); +} + function stripRegistryHive(target: string): string { return target.replace(/^(HKLM|HKEY_LOCAL_MACHINE|HKCU|HKEY_CURRENT_USER)\\/i, ''); } @@ -127,19 +173,28 @@ function checkGatherTarget(collectorType: string, target: string): ValidationFin case 'json': case 'xml': case 'logparser': { - const blocked = HARD_BLOCKED_PATH_PREFIXES.find((p) => pathPrefixAllowed(target, [p])); - if (blocked) { + const expanded = expandPathTokens(target); + + // C:\Users is hard-blocked; %LOGGED_ON_USER_PROFILE% releases it for AppData\Local + // and AppData\Roaming and nothing else. The release is not an allowance — the + // allowlist below still decides, exactly as GatherRuleGuards.IsFilePathAllowed does. + const blocked = HARD_BLOCKED_PATH_PREFIXES.find((p) => pathPrefixAllowed(expanded, [p])); + if (blocked && !isUnderUserProfileException(expanded)) { findings.push({ level: 'error', message: `guardrails: "${target}" is under the hard-blocked path ${blocked} — the agent always refuses this, it cannot be allow-listed.` }); break; } - const allowed = collectorType === 'logparser' - ? [...RULE_GUARDRAILS.filePrefixes, ...RULE_GUARDRAILS.diagnosticsPathPrefixes] - : [...RULE_GUARDRAILS.filePrefixes]; - // logparser targets may carry a glob suffix (*.log) — prefix matching still applies. - if (!pathPrefixAllowed(target, allowed) && !allowed.some((p) => target.toLowerCase().startsWith(p.toLowerCase() + '\\'))) { + // The agent guards EVERY file collector — logparser included — against + // filePrefixes alone (GatherRuleGuards.IsFilePathAllowed). diagnosticsPathPrefixes + // governs admin-configured diagnostics paths, a different surface the collectors + // never consult; offering it here reported logparser targets as allowed that the + // agent refuses on the device (C:\Install\Log, winevt\Logs, WER, ProgramData\AutopilotMonitor). + // logparser targets may carry a glob suffix (*.log) — segment-bounded prefix + // matching covers that, the glob sits in the last segment. + if (!pathPrefixAllowed(expanded, RULE_GUARDRAILS.filePrefixes) && + !userProfilePrefixAllowed(expanded)) { findings.push({ level: 'error', - message: `guardrails: ${collectorType} target "${target}" is not under any allowed file prefix (rule_guardrails → filePrefixes${collectorType === 'logparser' ? ' / diagnosticsPathPrefixes' : ''}).`, + message: `guardrails: ${collectorType} target "${target}" is not under any allowed file prefix (rule_guardrails → filePrefixes, or userProfileFilePrefixes below %LOGGED_ON_USER_PROFILE%).`, }); } break; diff --git a/src/Web/autopilot-monitor-web/utils/__tests__/guardValidation.test.ts b/src/Web/autopilot-monitor-web/utils/__tests__/guardValidation.test.ts index ac3e95be..76b9286e 100644 --- a/src/Web/autopilot-monitor-web/utils/__tests__/guardValidation.test.ts +++ b/src/Web/autopilot-monitor-web/utils/__tests__/guardValidation.test.ts @@ -143,12 +143,32 @@ describe("validateFileTarget", () => { expect(r.allowed).toBe(false); }); - it("allows %LOGGED_ON_USER_PROFILE% with AppData\\Local in file target", () => { + // The token lifts the C:\Users hard block for AppData — it does not admit the path. + // An unlisted vendor folder therefore falls through to the allowlist and is refused, + // exactly as GatherRuleGuards.IsFilePathAllowed does on the device. + it("blocks an unlisted vendor folder under %LOGGED_ON_USER_PROFILE%\\AppData\\Local", () => { const r = validateFileTarget( "%LOGGED_ON_USER_PROFILE%\\AppData\\Local\\SomeApp\\data.json", false, ); + expect(r.allowed).toBe(false); + }); + + it("allows a userProfileFilePrefixes folder under %LOGGED_ON_USER_PROFILE%", () => { + const r = validateFileTarget( + "%LOGGED_ON_USER_PROFILE%\\AppData\\Local\\RealmJoin\\tray.log", + false, + ); expect(r.allowed).toBe(true); + expect(r.reason).toContain("%LOGGED_ON_USER_PROFILE%"); + }); + + it("blocks a sibling of a userProfileFilePrefixes folder (segment-bounded)", () => { + const r = validateFileTarget( + "%LOGGED_ON_USER_PROFILE%\\AppData\\Local\\RealmJoinSomethingElse\\tray.log", + false, + ); + expect(r.allowed).toBe(false); }); it("blocks %LOGGED_ON_USER_PROFILE% with Desktop in file target", () => { @@ -348,12 +368,14 @@ describe("validateDiagnosticsPath", () => { expect(r.reason).toContain("%LOGGED_ON_USER_PROFILE%"); }); - it("allows %LOGGED_ON_USER_PROFILE% with AppData\\Roaming subpath", () => { + it("blocks an unlisted vendor folder under %LOGGED_ON_USER_PROFILE%\\AppData\\Roaming", () => { + // AppData\Roaming is inside the hard block's exception, but the folder is not on + // userProfileFilePrefixes — the allowlist still has the last word. const r = validateDiagnosticsPath( "%LOGGED_ON_USER_PROFILE%\\AppData\\Roaming\\SomeApp\\logs\\*.log", false, ); - expect(r.allowed).toBe(true); + expect(r.allowed).toBe(false); }); it("blocks %LOGGED_ON_USER_PROFILE% with non-AppData subpath", () => { diff --git a/src/Web/autopilot-monitor-web/utils/guardValidation.ts b/src/Web/autopilot-monitor-web/utils/guardValidation.ts index 959db8e2..aec81090 100644 --- a/src/Web/autopilot-monitor-web/utils/guardValidation.ts +++ b/src/Web/autopilot-monitor-web/utils/guardValidation.ts @@ -9,6 +9,7 @@ import { ALLOWED_REGISTRY_PREFIXES, ALLOWED_FILE_PREFIXES, + USER_PROFILE_FILE_PREFIXES, ALLOWED_WMI_QUERY_PREFIXES, ALLOWED_COMMANDS_LIST, ALLOWED_DIAGNOSTICS_PATH_PREFIXES, @@ -198,6 +199,18 @@ function isUserProfileSubpathAllowed(normalizedDir: string): boolean { return false; } +/** + * Returns the userProfileFilePrefixes entry admitting the path, if any. + * Mirrors GatherRuleGuards.IsUserProfilePathOnAllowlist: the entries are relative to + * the profile root, so they are matched against the placeholder profile the token + * expanded to. + */ +function findUserProfilePrefix(normalizedDir: string): string | undefined { + return USER_PROFILE_FILE_PREFIXES.find((prefix) => + matchesPrefix(normalizedDir, `${USER_PROFILE_PLACEHOLDER}\\${prefix}`, "\\") + ); +} + // --------------------------------------------------------------------------- // Validation functions // --------------------------------------------------------------------------- @@ -265,11 +278,14 @@ export function validateFileTarget( normalizedDir = normalizePath(expanded); } - // Hard block: C:\Users always blocked — except allowed subdirs via %LOGGED_ON_USER_PROFILE% token - if (matchesPrefix(normalizedDir, BLOCKED_USERS_PREFIX, "\\")) { - if (isUserProfileSubpathAllowed(normalizedDir)) { - return { allowed: true, reason: "Allowed via %LOGGED_ON_USER_PROFILE% (AppData only)", unrestricted: false }; - } + // Hard block: C:\Users always blocked. The %LOGGED_ON_USER_PROFILE% token LIFTS it for + // AppData\Local and AppData\Roaming — it does not admit the path: the allowlist below + // still has to match, exactly as GatherRuleGuards.IsFilePathAllowed does. Returning + // "allowed" here would show a target as green that the agent refuses on the device. + if ( + matchesPrefix(normalizedDir, BLOCKED_USERS_PREFIX, "\\") && + !isUserProfileSubpathAllowed(normalizedDir) + ) { return { allowed: false, reason: "C:\\Users is always blocked (privacy protection)", unrestricted: false }; } @@ -289,6 +305,15 @@ export function validateFileTarget( } } + const userProfilePrefix = findUserProfilePrefix(normalizedDir); + if (userProfilePrefix) { + return { + allowed: true, + reason: `Matches: %LOGGED_ON_USER_PROFILE%\\${userProfilePrefix}`, + unrestricted: false, + }; + } + return { allowed: false, reason: "Not under any allowed file path prefix", @@ -415,11 +440,14 @@ export function validateDiagnosticsPath( normalizedDir = normalizePath(expanded); } - // Hard block: C:\Users always blocked — except allowed subdirs via %LOGGED_ON_USER_PROFILE% token - if (matchesPrefix(normalizedDir, BLOCKED_USERS_PREFIX, "\\")) { - if (isUserProfileSubpathAllowed(normalizedDir)) { - return { allowed: true, reason: "Allowed via %LOGGED_ON_USER_PROFILE% (AppData only)", unrestricted: false }; - } + // Hard block: C:\Users always blocked. As in validateFileTarget, the + // %LOGGED_ON_USER_PROFILE% token only lifts the block for AppData\Local and + // AppData\Roaming — DiagnosticsPathGuards.IsDiagnosticsPathAllowed then still + // requires an allowlist match below. + if ( + matchesPrefix(normalizedDir, BLOCKED_USERS_PREFIX, "\\") && + !isUserProfileSubpathAllowed(normalizedDir) + ) { return { allowed: false, reason: "C:\\Users is always blocked (privacy protection)", unrestricted: false }; } @@ -439,6 +467,15 @@ export function validateDiagnosticsPath( } } + const userProfilePrefix = findUserProfilePrefix(normalizedDir); + if (userProfilePrefix) { + return { + allowed: true, + reason: `Matches: %LOGGED_ON_USER_PROFILE%\\${userProfilePrefix}`, + unrestricted: false, + }; + } + return { allowed: false, reason: "Not under any allowed diagnostics path prefix", diff --git a/src/Web/autopilot-monitor-web/utils/guardrails.generated.ts b/src/Web/autopilot-monitor-web/utils/guardrails.generated.ts index 0b9416a2..7867b4e4 100644 --- a/src/Web/autopilot-monitor-web/utils/guardrails.generated.ts +++ b/src/Web/autopilot-monitor-web/utils/guardrails.generated.ts @@ -263,6 +263,15 @@ export const ALLOWED_FILE_PREFIXES: readonly string[] = [ "C:\\Windows\\SetupDiag", "C:\\ProgramData\\Microsoft\\DiagnosticLogCSP", "C:\\Windows\\SoftwareDistribution\\ReportingEvents.log", + "C:\\ProgramData\\RealmJoin\\choco\\logs", +]; + +// Folders under the signed-in user's profile, RELATIVE to the profile root. The agent +// resolves %LOGGED_ON_USER_PROFILE% at runtime; the portal validates against a placeholder +// profile. These cannot live in ALLOWED_FILE_PREFIXES — that list is matched literally +// against the already-expanded path, where the user name is unknown at authoring time. +export const USER_PROFILE_FILE_PREFIXES: readonly string[] = [ + "AppData\\Local\\RealmJoin", ]; export const ALLOWED_WMI_QUERY_PREFIXES: readonly string[] = [