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[] = [