From 1ebc2675b1027f161f37cfebcf74c310cc4a6cbd Mon Sep 17 00:00:00 2001 From: Oliver Kieselbach Date: Mon, 10 Aug 2026 13:41:14 +0200 Subject: [PATCH] gather-rules: WQL property projections, portal enable-gating, author from JWT - WMI guard accepts SELECT FROM for any class already allowed via SELECT * (strict data subset); same algorithm in agent, portal validator, and MCP validate_rule (incl. trim fix); guardrails.json unchanged - Portal refuses to enable custom rules with not-allowed targets: create lands disabled, invalid edit force-disables, "Blocked on devices" badge, GitHub-issue hint for allowlist requests; agent stays the enforcement boundary - Rule Author is stamped from the creator's token (display name > UPN) at create and preserved through every update, incl. cross-tenant GA edits - bumpVersion keeps the segment count ("1.0.0" -> "1.1.0") Customer feedback: SELECT BatteryStatus FROM Win32_Battery + on_change was impossible because SELECT * re-emits on every charge-percent fluctuation. Co-Authored-By: Claude Fable 5 --- docs/log.md | 4 +- docs/rules/gather-rule-guardrails.md | 15 +++- .../Gather/GatherRuleCollectorGuardTests.cs | 43 ++++++++++ .../Telemetry/Gather/GatherRuleGuards.cs | 53 ++++++++++++- .../GatherRuleAuthorStampingTests.cs | 51 ++++++++++++ .../GatherRuleUpdatePartialMergeTests.cs | 29 +++++++ .../Functions/Rules/GatherRulesFunction.cs | 4 + .../Helpers/TenantHelper.cs | 78 ++++++++++++------- .../Services/GatherRuleService.cs | 5 ++ .../src/__tests__/rule-validation.test.ts | 31 ++++++++ .../src/rule-validation.ts | 44 +++++++++-- .../__tests__/targetBlocked.test.ts | 35 +++++++++ .../components/GatherRuleCard.tsx | 37 ++++++++- .../components/GatherRuleFormFields.tsx | 15 ++++ .../app/gather-rules/page.tsx | 26 +++++-- .../app/gather-rules/types.ts | 21 ++++- .../lib/__tests__/rulePageHelpers.test.ts | 23 ++++++ .../lib/rulePageHelpers.ts | 16 ++-- .../utils/__tests__/guardValidation.test.ts | 26 +++++++ .../utils/guardValidation.ts | 35 ++++++++- 20 files changed, 530 insertions(+), 61 deletions(-) create mode 100644 src/Backend/AutopilotMonitor.Functions.Tests/GatherRuleAuthorStampingTests.cs create mode 100644 src/Web/autopilot-monitor-web/app/gather-rules/__tests__/targetBlocked.test.ts create mode 100644 src/Web/autopilot-monitor-web/lib/__tests__/rulePageHelpers.test.ts diff --git a/docs/log.md b/docs/log.md index 6cae1fa5..0f8e66f5 100644 --- a/docs/log.md +++ b/docs/log.md @@ -1,6 +1,8 @@ # Log -## 2026-08-08 +## 2026-08-10 + +* **Update**: `rules/gather-rule-guardrails.md` — WMI matcher generalized from literal prefix matching to shape parsing: `SELECT <* | property-list> FROM ` is allowed when the class is derived from the existing `SELECT * FROM ` entries (a projection is a strict subset; motivated by `SELECT BatteryStatus FROM Win32_Battery` + `on_change` battery polling that `SELECT *` defeats via fluctuating charge %). Same algorithm mirrored agent/portal/MCP; guardrails.json unchanged. Portal now also gates enabling on the client-side validation (invalid-target custom rules create disabled, cannot be toggled on, force-disable on invalid edit, "Blocked on devices" badge) — UX only, the agent guard stays the boundary. Gather-rule Author is now stamped from the creator's JWT (display name → UPN) at create and immutable on update. * **New**: `agent/continue-anyway-observation.md` — Continue-Anyway observation mode (`EnableEspContinueAnywayObservation`, ConfigVersion 37, operator-set): Device-phase ESP terminal failures on Continue-Anyway profiles are defanged into a 60-min observation window (Classic/Unknown, non-pre-prov only); real-user desktop + Hello gate complete the session as Succeeded + `EspSoftFailure`/`CompletionSource` (amber "with issues" badge + detail banner), category recovery resolves clean, expiry un-defangs to the original `esp_terminal_failure`. Registered in the Agent index section. Related backend fix (no doc): the auto-analyze enqueue now carries a 30 s `visibilityTimeout` so rule evaluation no longer races the agent's terminal flush (ANALYZE-ESP-004's `{{appName}}` placeholder). diff --git a/docs/rules/gather-rule-guardrails.md b/docs/rules/gather-rule-guardrails.md index e37cd310..173841f2 100644 --- a/docs/rules/gather-rule-guardrails.md +++ b/docs/rules/gather-rule-guardrails.md @@ -7,14 +7,14 @@ description: >- guard rather than trusting rule.Target. resource: src/Agent/AutopilotMonitor.Agent.V2.Core/Monitoring/Telemetry/Gather tags: [gather-rules, security, guardrails, agent] -timestamp: 2026-07-22 +timestamp: 2026-08-10 --- # Concept A gather rule is authored by a **tenant administrator** through `POST /api/rules/gather` and executed by the agent as **SYSTEM** on an enrolling device. The backend does not validate `collectorType`, `target`, or `parameters` at all — it only enforces scope/emit-mode fields, a 1 MB body cap, and that the rule lands in the caller's own tenant partition. Everything that decides *what a rule may reach* is therefore enforced **on the agent**, in `GatherRuleGuards`. -This is the security boundary. The portal's `ValidationIndicator` is a convenience: it does not block submit, and the API is reachable directly. +This is the security boundary. The portal's `ValidationIndicator` is a convenience — the API stays reachable directly and the backend stays validation-free — but since 2026-08 the portal additionally **gates enabling** on the same client-side validation: a custom rule whose target fails validation cannot be toggled on, is created `enabled=false`, and an edit that makes the target invalid force-disables the rule (`targetBlocked` in `app/gather-rules/types.ts`). An enabled rule with an invalid target shows a "Blocked on devices" badge. All of this is UX, not security — the agent guard remains the enforcement point. # Schema @@ -27,7 +27,7 @@ item in `AutopilotMonitor.Agent.V2.Core.csproj`) and regenerated into the portal | --- | --- | --- | | `IsRegistryPathAllowed` | `registryPrefixes` | prefix, boundary `\` or end | | `IsFilePathAllowed` | `filePrefixes` | `Path.GetFullPath` first, then prefix + boundary | -| `IsWmiQueryAllowed` | `wmiQueryPrefixes` | prefix, boundary whitespace or end | +| `IsWmiQueryAllowed` | `wmiQueryPrefixes` | `SELECT <* \| property-list> FROM `; class = case-insensitive equality against classes derived from the `SELECT * FROM ` entries, boundary whitespace or end (trailing `WHERE` allowed). Non-canonical entries (none today) fall back to literal prefix + boundary | | `IsCommandAllowed` | `allowedCommands` | **exact** match, trimmed — never prefix | | `IsEventLogChannelAllowed` | `eventLogChannels` | prefix, boundary `/` or end | @@ -71,6 +71,15 @@ Two collectors were added to this contract on 2026-07-22 after shipping without remote config. The operator using it is already a local admin, so it relaxes the allowlist exactly the way unrestricted mode does — hard blocks still apply. +**WMI property projections (2026-08):** `SELECT BatteryStatus FROM Win32_Battery` is +allowed because `SELECT * FROM Win32_Battery` is — a projection exposes a strict subset +of the allowed class. The matcher parses the query shape instead of literal-prefix +matching; the allowed-class set is derived at load time from the existing +`wmiQueryPrefixes` entries, so `rules/guardrails.json` needs no new key. The same +algorithm is mirrored in the portal (`utils/guardValidation.ts`) and the MCP server +(`rule-validation.ts`); agents older than the change still block projections with a +`security_warning` until they self-update. + # Citations * `src/Agent/AutopilotMonitor.Agent.V2.Core/Monitoring/Telemetry/Gather/GatherRuleGuards.cs` 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 c4065c07..f1d44e1a 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 @@ -308,9 +308,52 @@ namespace AutopilotMonitor.Agent.V2.Core.Tests.Monitoring.Gather [InlineData("SELECT * FROM Win32_UserAccount")] [InlineData("SELECT * FROM Win32_ShadowCopy")] [InlineData("SELECT * FROM Win32_OperatingSystemExtra")] // prefix spoofing + [InlineData("SELECT Name FROM Win32_Process")] // projection of a disallowed class + [InlineData("SELECT BatteryStatus FROM Win32_BatteryX")] // class spoofing via projection public void WmiCollector_OffAllowlistQuery_IsBlockedBeforeTheQueryRuns(string query) => AssertBlocked(new WmiCollector().Execute(Rule("wmi", query), Context()), query); + // ------------------------------------------------------------------- + // WMI query matcher: "SELECT <* | property-list> FROM " + // + // Guard-level (no WMI touched): a property projection of an allowed class + // exposes a strict subset of the already-allowed SELECT *, so it is admitted. + // ------------------------------------------------------------------- + + [Theory] + [InlineData("SELECT * FROM Win32_Battery")] // legacy form unchanged + [InlineData("SELECT * FROM Win32_BIOS WHERE PrimaryBIOS = TRUE")] // legacy + WHERE + [InlineData("SELECT BatteryStatus FROM Win32_Battery")] + [InlineData("SELECT BatteryStatus, EstimatedChargeRemaining FROM Win32_Battery")] + [InlineData("select batterystatus from win32_battery")] // case-insensitive + [InlineData("SELECT\tBatteryStatus\tFROM\tWin32_Battery")] // tabs as separators + [InlineData(" SELECT BatteryStatus FROM Win32_Battery ")] // surrounding whitespace + [InlineData("SELECT BatteryStatus,EstimatedChargeRemaining FROM Win32_Battery")] // no space after comma + [InlineData("SELECT Status FROM Win32_Battery WHERE BatteryStatus = 1")] // projection + WHERE + public void WmiGuard_ProjectionOfAllowedClass_IsAllowed(string query) + => Assert.True(GatherRuleGuards.IsWmiQueryAllowed(query)); + + [Theory] + [InlineData("SELECT Name FROM Win32_Process")] // class not on the allowlist + [InlineData("SELECT BatteryStatus FROM Win32_BatteryX")] // class spoofing + [InlineData("SELECT * FROM Win32_BatteryX")] + [InlineData("SELECT *, Name FROM Win32_BIOS")] // star mixed into a property list + [InlineData("SELECT FROM Win32_BIOS")] // empty property list + [InlineData("SELECT BatteryStatus FROM")] // missing class + [InlineData("SELECT BatteryStatus FROM Win32_Battery;DROP")] // non-whitespace boundary after class + [InlineData("SELECT BatteryStatus, FROM Win32_Battery")] // trailing comma + [InlineData("")] + public void WmiGuard_MalformedOrDisallowedQuery_IsBlocked(string query) + => Assert.False(GatherRuleGuards.IsWmiQueryAllowed(query)); + + [Fact] + public void WmiGuard_NullQuery_IsBlocked() + => Assert.False(GatherRuleGuards.IsWmiQueryAllowed(null)); + + [Fact] + public void WmiGuard_UnrestrictedMode_AllowsAnyQuery() + => Assert.True(GatherRuleGuards.IsWmiQueryAllowed("SELECT Name FROM Win32_Process", unrestrictedMode: true)); + [Fact] public void JsonCollector_UserProfilePath_IsBlockedBeforeTheFileIsOpened() { 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 a178947b..263834c0 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 @@ -3,6 +3,7 @@ using System.Collections.Generic; using System.IO; using System.Linq; using System.Reflection; +using System.Text.RegularExpressions; using Newtonsoft.Json.Linq; namespace AutopilotMonitor.Agent.V2.Core.Monitoring.Telemetry.Gather @@ -35,6 +36,24 @@ namespace AutopilotMonitor.Agent.V2.Core.Monitoring.Telemetry.Gather public static readonly IReadOnlyCollection AllowedCommands; public static readonly IReadOnlyList AllowedEventLogChannels; + // Derived from AllowedWmiQueryPrefixes: entries of the canonical form + // "SELECT * FROM " contribute their class name; any other entry + // (none today) stays on the legacy literal-prefix path. + private static readonly HashSet AllowedWmiClasses; + private static readonly List ResidualWmiPrefixes; + + // Canonical allowlist entry: exactly "SELECT * FROM ". + private static readonly Regex CanonicalWmiEntryRegex = new Regex( + @"^SELECT\s+\*\s+FROM\s+([A-Za-z_][A-Za-z0-9_]*)$", + RegexOptions.IgnoreCase | RegexOptions.Compiled); + + // WQL shape "SELECT <* | property-list> FROM ", class token bounded + // by whitespace or end of string (a trailing WHERE clause is permitted, + // "Win32_BIOSX" cannot pass as "Win32_BIOS"). + private static readonly Regex WmiQueryRegex = new Regex( + @"^SELECT\s+(?:\*|[A-Za-z_][A-Za-z0-9_]*(?:\s*,\s*[A-Za-z_][A-Za-z0-9_]*)*)\s+FROM\s+([A-Za-z_][A-Za-z0-9_]*)(?=\s|$)", + RegexOptions.IgnoreCase | RegexOptions.Compiled); + // Hard blocks: never allowed, even in unrestricted mode private static readonly string BlockedUsersPrefix = Path.GetFullPath(@"C:\Users"); private static readonly string[] AdditionalHardBlockedPathPrefixes = new[] @@ -101,6 +120,7 @@ namespace AutopilotMonitor.Agent.V2.Core.Monitoring.Telemetry.Gather AllowedCommands = new HashSet(commands, StringComparer.OrdinalIgnoreCase); AllowedEventLogChannels = FlattenCategorized(obj, "eventLogChannels", "channels"); + DeriveWmiClassAllowlist(AllowedWmiQueryPrefixes, out AllowedWmiClasses, out ResidualWmiPrefixes); return; } } @@ -115,6 +135,26 @@ namespace AutopilotMonitor.Agent.V2.Core.Monitoring.Telemetry.Gather AllowedWmiQueryPrefixes = new List(); AllowedCommands = new HashSet(StringComparer.OrdinalIgnoreCase); AllowedEventLogChannels = new List(); + AllowedWmiClasses = new HashSet(StringComparer.OrdinalIgnoreCase); + ResidualWmiPrefixes = new List(); + } + + private static void DeriveWmiClassAllowlist( + IReadOnlyList entries, + out HashSet classes, + out List residual) + { + classes = new HashSet(StringComparer.OrdinalIgnoreCase); + residual = new List(); + + foreach (var entry in entries) + { + var match = CanonicalWmiEntryRegex.Match(entry.Trim()); + if (match.Success) + classes.Add(match.Groups[1].Value); + else + residual.Add(entry); + } } private static string LoadEmbeddedGuardrails() @@ -250,8 +290,9 @@ namespace AutopilotMonitor.Agent.V2.Core.Monitoring.Telemetry.Gather } /// - /// Returns true if the WMI query (trimmed) matches an allowed prefix - /// with boundary matching (next char must be whitespace or end of string). + /// Returns true if the WMI query (trimmed) is "SELECT <* | property-list> FROM <class>" + /// (optionally followed by a WHERE clause) against a class on the allowlist. + /// A property projection exposes a strict subset of the allowed "SELECT *". /// public static bool IsWmiQueryAllowed(string query) => IsWmiQueryAllowed(query, unrestrictedMode: false); @@ -269,7 +310,13 @@ namespace AutopilotMonitor.Agent.V2.Core.Monitoring.Telemetry.Gather return true; var trimmed = query.Trim(); - return AllowedWmiQueryPrefixes.Any(prefix => + + var match = WmiQueryRegex.Match(trimmed); + if (match.Success && AllowedWmiClasses.Contains(match.Groups[1].Value)) + return true; + + // Non-canonical allowlist entries keep the legacy literal-prefix match. + return ResidualWmiPrefixes.Any(prefix => trimmed.StartsWith(prefix, StringComparison.OrdinalIgnoreCase) && (trimmed.Length == prefix.Length || char.IsWhiteSpace(trimmed[prefix.Length]))); } diff --git a/src/Backend/AutopilotMonitor.Functions.Tests/GatherRuleAuthorStampingTests.cs b/src/Backend/AutopilotMonitor.Functions.Tests/GatherRuleAuthorStampingTests.cs new file mode 100644 index 00000000..d2ee9f05 --- /dev/null +++ b/src/Backend/AutopilotMonitor.Functions.Tests/GatherRuleAuthorStampingTests.cs @@ -0,0 +1,51 @@ +using System.Security.Claims; +using AutopilotMonitor.Functions.Helpers; + +namespace AutopilotMonitor.Functions.Tests; + +/// +/// Pins the claim precedence behind gather-rule Author stamping +/// (): display name +/// first, then UPN/email/preferred_username; null for anonymous/app-only callers +/// so CreateGatherRule falls back to the model default "Autopilot Monitor". +/// +public class GatherRuleAuthorStampingTests +{ + private static ClaimsPrincipal Principal(params (string Type, string Value)[] claims) + => new(new ClaimsIdentity(claims.Select(c => new Claim(c.Type, c.Value)), "TestAuth")); + + [Fact] + public void DisplayName_wins_over_upn() + { + var user = Principal(("name", "Alice Admin"), ("upn", "alice@contoso.com")); + Assert.Equal("Alice Admin", TenantHelper.GetUserDisplayName(user)); + } + + [Fact] + public void Upn_used_when_no_name_claim() + { + var user = Principal(("upn", "alice@contoso.com"), ("preferred_username", "alice@contoso.onmicrosoft.com")); + Assert.Equal("alice@contoso.com", TenantHelper.GetUserDisplayName(user)); + } + + [Fact] + public void PreferredUsername_is_the_last_fallback() + { + var user = Principal(("preferred_username", "alice@contoso.com")); + Assert.Equal("alice@contoso.com", TenantHelper.GetUserDisplayName(user)); + } + + [Fact] + public void Null_for_principal_without_identifying_claims() + { + Assert.Null(TenantHelper.GetUserDisplayName(Principal(("tid", "a1b2c3d4-e5f6-7890-abcd-ef1234567890")))); + } + + [Fact] + public void Null_for_unauthenticated_or_missing_principal() + { + Assert.Null(TenantHelper.GetUserDisplayName((ClaimsPrincipal?)null)); + // ClaimsIdentity without an authentication type => IsAuthenticated == false. + Assert.Null(TenantHelper.GetUserDisplayName(new ClaimsPrincipal(new ClaimsIdentity(new[] { new Claim("name", "Ghost") })))); + } +} diff --git a/src/Backend/AutopilotMonitor.Functions.Tests/GatherRuleUpdatePartialMergeTests.cs b/src/Backend/AutopilotMonitor.Functions.Tests/GatherRuleUpdatePartialMergeTests.cs index 062a3819..d93975ae 100644 --- a/src/Backend/AutopilotMonitor.Functions.Tests/GatherRuleUpdatePartialMergeTests.cs +++ b/src/Backend/AutopilotMonitor.Functions.Tests/GatherRuleUpdatePartialMergeTests.cs @@ -33,6 +33,7 @@ public class GatherRuleUpdatePartialMergeTests Enabled = true, IsBuiltIn = false, IsCommunity = false, + Author = "Alice Admin", CreatedAt = new DateTime(2026, 1, 15, 12, 0, 0, DateTimeKind.Utc), ActiveFromPhase = "AccountSetup", EmitMode = "on_change", @@ -77,6 +78,7 @@ public class GatherRuleUpdatePartialMergeTests Assert.Equal("AccountSetup", written.ActiveFromPhase); Assert.Equal("on_change", written.EmitMode); Assert.Equal(existing.CreatedAt, written.CreatedAt); + Assert.Equal("Alice Admin", written.Author); } [Fact] @@ -126,6 +128,33 @@ public class GatherRuleUpdatePartialMergeTests Assert.Equal(existing.CreatedAt, written.CreatedAt); } + [Fact] + public async Task FullPayload_preserves_original_Author_even_when_payload_carries_one() + { + var existing = ExistingCustomRule(); + var (service, _, stored) = BuildService(existing); + + var fullPayload = new GatherRule + { + RuleId = RuleId, + Title = "Collect BIOS Config v2", + CollectorType = "registry", + Target = "HKLM\\SOFTWARE\\RealmJoin\\Custom\\BIOS2", + Trigger = "interval", + OutputEventType = "gather_bios_config", + Enabled = true, + // A hand-crafted PUT could try to rewrite attribution — Author is + // stamped at create time and immutable afterwards. + Author = "Spoofed Author", + }; + + var ok = await service.UpdateRuleAsync(TenantId, fullPayload); + + Assert.True(ok); + var written = Assert.Single(stored); + Assert.Equal("Alice Admin", written.Author); + } + [Fact] public async Task BuiltInRule_toggle_still_goes_through_RuleState_path() { diff --git a/src/Backend/AutopilotMonitor.Functions/Functions/Rules/GatherRulesFunction.cs b/src/Backend/AutopilotMonitor.Functions/Functions/Rules/GatherRulesFunction.cs index 7109d5b2..6683bf30 100644 --- a/src/Backend/AutopilotMonitor.Functions/Functions/Rules/GatherRulesFunction.cs +++ b/src/Backend/AutopilotMonitor.Functions/Functions/Rules/GatherRulesFunction.cs @@ -72,6 +72,10 @@ namespace AutopilotMonitor.Functions.Functions.Rules return badRequest; } + // Author is stamped from the creator's token, never from the payload + // (anti-spoof), and stays immutable through every later update. + rule.Author = TenantHelper.GetUserDisplayName(req) ?? "Autopilot Monitor"; + try { var success = await _ruleService.CreateRuleAsync(tenantId, rule); diff --git a/src/Backend/AutopilotMonitor.Functions/Helpers/TenantHelper.cs b/src/Backend/AutopilotMonitor.Functions/Helpers/TenantHelper.cs index 1621b006..fbedf95a 100644 --- a/src/Backend/AutopilotMonitor.Functions/Helpers/TenantHelper.cs +++ b/src/Backend/AutopilotMonitor.Functions/Helpers/TenantHelper.cs @@ -19,27 +19,33 @@ public static class TenantHelper /// The HTTP request /// The tenant ID from the JWT token /// Thrown when user is not authenticated or tenant ID is missing - public static string GetTenantId(HttpRequestData req) + /// + /// Resolves the authenticated ClaimsPrincipal for the request, or null. + /// Azure Functions Isolated Worker: FunctionContext.Items first (set by + /// AuthenticationMiddleware, more reliable than httpContext.User), then + /// the HTTP context as fallback. + /// + private static ClaimsPrincipal? GetAuthenticatedPrincipal(HttpRequestData req) { - // Azure Functions Isolated Worker: Try FunctionContext.Items first - // This is set by AuthenticationMiddleware and is more reliable than httpContext.User - ClaimsPrincipal? user = null; - if (req.FunctionContext.Items.TryGetValue("ClaimsPrincipal", out var principalObj) && principalObj is ClaimsPrincipal principal) { - user = principal; + return principal; } - else + + var httpContext = req.FunctionContext.GetHttpContext(); + if (httpContext?.User?.Identity?.IsAuthenticated == true) { - // Fallback to HTTP context (may not work reliably in isolated worker) - var httpContext = req.FunctionContext.GetHttpContext(); - if (httpContext?.User?.Identity?.IsAuthenticated == true) - { - user = httpContext.User; - } + return httpContext.User; } + return null; + } + + public static string GetTenantId(HttpRequestData req) + { + var user = GetAuthenticatedPrincipal(req); + if (user?.Identity?.IsAuthenticated != true) { throw new UnauthorizedAccessException("User is not authenticated. JWT token required."); @@ -67,23 +73,7 @@ public static class TenantHelper /// User email or name, or "Anonymous" if not authenticated public static string GetUserIdentifier(HttpRequestData req) { - // Azure Functions Isolated Worker: Try FunctionContext.Items first - ClaimsPrincipal? user = null; - - if (req.FunctionContext.Items.TryGetValue("ClaimsPrincipal", out var principalObj) - && principalObj is ClaimsPrincipal principal) - { - user = principal; - } - else - { - // Fallback to HTTP context - var httpContext = req.FunctionContext.GetHttpContext(); - if (httpContext?.User?.Identity?.IsAuthenticated == true) - { - user = httpContext.User; - } - } + var user = GetAuthenticatedPrincipal(req); if (user?.Identity?.IsAuthenticated != true) { @@ -100,4 +90,32 @@ public static class TenantHelper user.FindFirst("name")?.Value ?? "Unknown"; } + + /// + /// Gets the caller's human-readable name for attribution (e.g. rule Author): + /// display name first, then UPN/email/preferred_username. Returns null when + /// the request carries no user identity (anonymous or app-only token) so the + /// caller can choose its own fallback. + /// + public static string? GetUserDisplayName(HttpRequestData req) + => GetUserDisplayName(GetAuthenticatedPrincipal(req)); + + /// + /// Claim-precedence core of , + /// separated so the precedence is unit-testable without an HttpRequestData. + /// + public static string? GetUserDisplayName(ClaimsPrincipal? user) + { + if (user?.Identity?.IsAuthenticated != true) + { + return null; + } + + return user.FindFirst("name")?.Value ?? + user.FindFirst(ClaimTypes.Name)?.Value ?? + user.FindFirst("upn")?.Value ?? + user.FindFirst(ClaimTypes.Upn)?.Value ?? + user.FindFirst(ClaimTypes.Email)?.Value ?? + user.FindFirst("preferred_username")?.Value; + } } diff --git a/src/Backend/AutopilotMonitor.Functions/Services/GatherRuleService.cs b/src/Backend/AutopilotMonitor.Functions/Services/GatherRuleService.cs index b6fc7af4..91f46b4f 100644 --- a/src/Backend/AutopilotMonitor.Functions/Services/GatherRuleService.cs +++ b/src/Backend/AutopilotMonitor.Functions/Services/GatherRuleService.cs @@ -214,7 +214,12 @@ namespace AutopilotMonitor.Functions.Services rule.IsBuiltIn = false; rule.IsCommunity = false; if (existing != null) + { rule.CreatedAt = existing.CreatedAt; + // Author is stamped from the creator's token at create time and is + // immutable afterwards — updates (incl. cross-tenant GA edits) keep it. + rule.Author = existing.Author; + } rule.UpdatedAt = DateTime.UtcNow; return await _ruleRepo.StoreGatherRuleAsync(rule, tenantId); } 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 743cec74..c09a1b2d 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 @@ -137,6 +137,37 @@ describe('gather guardrails (agent matching semantics)', () => { expect(validateRuleDraft(spoof).valid).toBe(false); }); + it('WMI: property projection of an allowed class is admitted', () => { + const single = validGather(); + single.collectorType = 'wmi'; + single.target = 'SELECT BatteryStatus FROM Win32_Battery'; + expect(validateRuleDraft(single).valid).toBe(true); + + const multi = validGather(); + multi.collectorType = 'wmi'; + multi.target = 'SELECT BatteryStatus, EstimatedChargeRemaining FROM Win32_Battery WHERE BatteryStatus = 1'; + expect(validateRuleDraft(multi).valid).toBe(true); + }); + + it('WMI: projection of a disallowed class and star-in-list are rejected', () => { + const badClass = validGather(); + badClass.collectorType = 'wmi'; + badClass.target = 'SELECT Name FROM Win32_Process'; + expect(validateRuleDraft(badClass).valid).toBe(false); + + const starMix = validGather(); + starMix.collectorType = 'wmi'; + starMix.target = 'SELECT *, Name FROM Win32_BIOS'; + expect(validateRuleDraft(starMix).valid).toBe(false); + }); + + it('WMI: leading whitespace is trimmed like the agent does (no false positive)', () => { + const padded = validGather(); + padded.collectorType = 'wmi'; + padded.target = ' SELECT * FROM Win32_BIOS'; + expect(validateRuleDraft(padded).valid).toBe(true); + }); + it('commands match exactly — extra arguments are rejected', () => { const ok = validGather(); ok.collectorType = 'command_allowlisted'; diff --git a/src/McpServer/autopilot-monitor-mcp/src/rule-validation.ts b/src/McpServer/autopilot-monitor-mcp/src/rule-validation.ts index 0984a040..c930b0d9 100644 --- a/src/McpServer/autopilot-monitor-mcp/src/rule-validation.ts +++ b/src/McpServer/autopilot-monitor-mcp/src/rule-validation.ts @@ -67,6 +67,25 @@ const flatRegistryPrefixes = RULE_GUARDRAILS.registryPrefixes.flatMap((g) => [.. const flatCommands = RULE_GUARDRAILS.allowedCommands.flatMap((g) => [...g.commands]); const flatEventLogChannels = RULE_GUARDRAILS.eventLogChannels.flatMap((g) => [...g.channels]); +// Canonical WMI allowlist entry: exactly "SELECT * FROM ". Such entries +// contribute their class name; any other entry (none today) stays on the legacy +// literal-prefix path. Mirrors GatherRuleGuards.cs. +const CANONICAL_WMI_ENTRY_REGEX = /^SELECT\s+\*\s+FROM\s+([A-Za-z_][A-Za-z0-9_]*)$/i; + +// WQL shape "SELECT <* | property-list> FROM ", class token bounded by +// whitespace or end of string (trailing WHERE permitted, Win32_BIOSX cannot +// pass as Win32_BIOS). +const WMI_QUERY_REGEX = + /^SELECT\s+(?:\*|[A-Za-z_][A-Za-z0-9_]*(?:\s*,\s*[A-Za-z_][A-Za-z0-9_]*)*)\s+FROM\s+([A-Za-z_][A-Za-z0-9_]*)(?=\s|$)/i; + +const allowedWmiClasses = new Set(); +const residualWmiPrefixes: string[] = []; +for (const entry of RULE_GUARDRAILS.wmiQueryPrefixes) { + const m = CANONICAL_WMI_ENTRY_REGEX.exec(entry.trim()); + if (m) allowedWmiClasses.add(m[1].toLowerCase()); + else residualWmiPrefixes.push(entry); +} + /** Hard blocks enforced in agent CODE (not liftable via guardrails.json). Keep in * sync with GatherRuleGuards.cs — additions there are fail-safe (agent still blocks), * missing ones here just cost a pre-flight warning. */ @@ -121,15 +140,24 @@ function checkGatherTarget(collectorType: string, target: string): ValidationFin break; } case 'wmi': { - const t = target.toLowerCase(); - const ok = RULE_GUARDRAILS.wmiQueryPrefixes.some((p) => { - const pl = p.toLowerCase(); - // Whitespace-bounded: the query is the prefix or continues with whitespace - // (mirrors the agent's boundary check, prevents Win32_BIOSX spoofing). - return t === pl || (t.startsWith(pl) && /\s/.test(t.charAt(pl.length))); - }); + // Trim like the agent does — an untrimmed target must not be a false positive. + const t = target.trim().toLowerCase(); + const m = WMI_QUERY_REGEX.exec(t); + let ok = m !== null && allowedWmiClasses.has(m[1]); if (!ok) { - findings.push({ level: 'error', message: `guardrails: WMI query "${target}" does not start with any allowed query prefix (rule_guardrails → wmiQueryPrefixes).` }); + // Non-canonical allowlist entries keep the legacy literal-prefix match. + ok = residualWmiPrefixes.some((p) => { + const pl = p.toLowerCase(); + return t === pl || (t.startsWith(pl) && /\s/.test(t.charAt(pl.length))); + }); + } + if (!ok) { + findings.push({ + level: 'error', + message: + `guardrails: WMI query "${target}" is not allowed — use "SELECT FROM " against a class from rule_guardrails → wmiQueryPrefixes. ` + + 'Allowing additional classes requires a contribution to rules/guardrails.json in the product repository.', + }); } break; } diff --git a/src/Web/autopilot-monitor-web/app/gather-rules/__tests__/targetBlocked.test.ts b/src/Web/autopilot-monitor-web/app/gather-rules/__tests__/targetBlocked.test.ts new file mode 100644 index 00000000..19497960 --- /dev/null +++ b/src/Web/autopilot-monitor-web/app/gather-rules/__tests__/targetBlocked.test.ts @@ -0,0 +1,35 @@ +import { describe, it, expect } from "vitest"; +import { targetBlocked } from "../types"; + +// The enable gate for gather rules: a CUSTOM rule whose target fails guardrail +// validation must not be enableable — the agent would block it on every device. + +const custom = { isBuiltIn: false, isCommunity: false }; + +describe("targetBlocked", () => { + it("blocks a custom rule with a disallowed WMI query", () => { + expect(targetBlocked({ ...custom, collectorType: "wmi", target: "SELECT * FROM Win32_Process" }, false)).toBe(true); + }); + + it("does not block an allowed projection query", () => { + expect(targetBlocked({ ...custom, collectorType: "wmi", target: "SELECT BatteryStatus FROM Win32_Battery" }, false)).toBe(false); + }); + + it("never blocks built-in or community rules", () => { + expect(targetBlocked({ isBuiltIn: true, isCommunity: false, collectorType: "wmi", target: "SELECT * FROM Win32_Process" }, false)).toBe(false); + expect(targetBlocked({ isBuiltIn: false, isCommunity: true, collectorType: "wmi", target: "SELECT * FROM Win32_Process" }, false)).toBe(false); + }); + + it("respects unrestricted mode", () => { + expect(targetBlocked({ ...custom, collectorType: "wmi", target: "SELECT * FROM Win32_Process" }, true)).toBe(false); + }); + + it("does not block an empty target (required-field validation owns that)", () => { + expect(targetBlocked({ ...custom, collectorType: "wmi", target: "" }, false)).toBe(false); + }); + + it("blocks disallowed targets of other collector types too", () => { + expect(targetBlocked({ ...custom, collectorType: "file", target: "C:\\Users\\alice\\secret.txt" }, false)).toBe(true); + expect(targetBlocked({ ...custom, collectorType: "eventlog", target: "Security" }, false)).toBe(true); + }); +}); diff --git a/src/Web/autopilot-monitor-web/app/gather-rules/components/GatherRuleCard.tsx b/src/Web/autopilot-monitor-web/app/gather-rules/components/GatherRuleCard.tsx index 86f39e40..1f5544e4 100644 --- a/src/Web/autopilot-monitor-web/app/gather-rules/components/GatherRuleCard.tsx +++ b/src/Web/autopilot-monitor-web/app/gather-rules/components/GatherRuleCard.tsx @@ -68,6 +68,11 @@ export function GatherRuleCard({ [rule.collectorType, rule.target, unrestrictedMode] ); + // Custom rule whose target the agent would block on every device (security_warning, + // no data): the portal refuses to enable it; disabling always stays possible. + const isBlockedTarget = canEdit && targetValidation !== null && !targetValidation.allowed; + const enableBlocked = !rule.enabled && isBlockedTarget; + return (
+ {/* Enabled rule the agent blocks on every device — visible without expanding */} + {rule.enabled && isBlockedTarget && ( + + Blocked on devices + + )} + {/* Expand/Collapse Arrow */} + {isBlockedTarget && ( +

+ This target isn't on the collection allowlist. Think it should be?{" "} + + Request an addition via a GitHub issue + + . +

+ )}
{rule.parameters && Object.keys(rule.parameters).length > 0 && (
diff --git a/src/Web/autopilot-monitor-web/app/gather-rules/components/GatherRuleFormFields.tsx b/src/Web/autopilot-monitor-web/app/gather-rules/components/GatherRuleFormFields.tsx index a15f02f0..acbc75ca 100644 --- a/src/Web/autopilot-monitor-web/app/gather-rules/components/GatherRuleFormFields.tsx +++ b/src/Web/autopilot-monitor-web/app/gather-rules/components/GatherRuleFormFields.tsx @@ -145,6 +145,21 @@ export function GatherRuleFormFields({ form, setForm, showRuleId, unrestrictedMo

{TARGET_HINTS[form.collectorType] || "Registry path, WMI class, event log name, file path, or command depending on collector type"}

+ {targetValidation && !targetValidation.allowed && ( +

+ This target isn't on the collection allowlist — the rule can be saved but not enabled. + Think it should be allowed?{" "} + + Request an addition via a GitHub issue + + . +

+ )} {/* Registry: optional Value Name */} diff --git a/src/Web/autopilot-monitor-web/app/gather-rules/page.tsx b/src/Web/autopilot-monitor-web/app/gather-rules/page.tsx index 4f1ee5b8..7c26261d 100644 --- a/src/Web/autopilot-monitor-web/app/gather-rules/page.tsx +++ b/src/Web/autopilot-monitor-web/app/gather-rules/page.tsx @@ -14,7 +14,7 @@ import { FormJsonToggle, JsonModeToggleButtons } from "@/components/rules/FormJs import { useAuthenticatedFetch, useNotificationMessages, useGlobalAdminScope } from "@/hooks"; import { GlobalAdminBanner, globalAdminSubtitle } from "@/components/GlobalAdminBanner"; import { TenantScopeSelector } from "@/components/TenantScopeSelector"; -import { GatherRule, NewRuleForm, EMPTY_FORM, PHASE_TRIGGERS, buildScopeFields, validateScopeSelection, withDerivedScopeMode } from "./types"; +import { GatherRule, NewRuleForm, EMPTY_FORM, PHASE_TRIGGERS, buildScopeFields, targetBlocked, validateScopeSelection, withDerivedScopeMode } from "./types"; import { GatherRuleFormFields } from "./components/GatherRuleFormFields"; import { GatherRuleCard } from "./components/GatherRuleCard"; import { DOCS_URL } from "@/utils/config"; @@ -125,6 +125,12 @@ export default function GatherRulesPage() { }, [effectiveTenantId, getAccessToken]); const handleToggleRule = async (rule: GatherRule) => { + // Defense in depth next to the disabled toggle button: never enable a rule the + // agent would block on every device anyway. Disabling always stays possible. + if (!rule.enabled && targetBlocked(rule, unrestrictedMode)) { + showError(`Rule "${rule.title}" cannot be enabled — its target is not allowed by the collection guardrails.`); + return; + } setTogglingRule(rule.ruleId); const result = await mutate( gatherRuleWriteUrl(rule.ruleId), @@ -214,6 +220,9 @@ export default function GatherRulesPage() { } setCreating(true); + // Without an explicit enabled the backend model defaults to true — a rule the agent + // would block on every device must land disabled instead. + const blocked = targetBlocked(form, unrestrictedMode); const payload = { ruleId: form.ruleId, title: form.title, @@ -229,6 +238,7 @@ export default function GatherRulesPage() { ...buildScopeFields(form), outputEventType: form.outputEventType, outputSeverity: form.outputSeverity, + enabled: !blocked, }; const result = await mutate( @@ -241,7 +251,9 @@ export default function GatherRulesPage() { ); if (result !== null) { trackEvent("rule_created", { ruleType: "gather" }); - showSuccess(`Rule "${form.title}" created`); + showSuccess(blocked + ? `Rule "${form.title}" created (disabled — its target is not allowed by the collection guardrails)` + : `Rule "${form.title}" created`); setShowCreateForm(false); setNewRule({ ...EMPTY_FORM }); setJsonModeCreate(false); @@ -306,6 +318,9 @@ export default function GatherRulesPage() { } setSaving(true); + // An edit that makes the target invalid force-disables the rule — the agent would + // block it on every device, so leaving it enabled only produces security warnings. + const forcedOff = rule.enabled && targetBlocked(form, unrestrictedMode); const payload = { title: form.title, description: form.description, @@ -321,8 +336,7 @@ export default function GatherRulesPage() { outputEventType: form.outputEventType, outputSeverity: form.outputSeverity, version: bumpVersion(rule.version), - enabled: rule.enabled, - author: rule.author, + enabled: rule.enabled && !forcedOff, createdAt: rule.createdAt, // Same wipe class as the toggle bug: the full-replace PUT must carry tags or they vanish. tags: rule.tags ?? [], @@ -340,7 +354,9 @@ export default function GatherRulesPage() { ); if (result !== null) { trackEvent("rule_modified", { ruleType: "gather" }); - showSuccess(`Rule "${form.title}" saved`); + showSuccess(forcedOff + ? `Rule "${form.title}" saved and disabled — its target is not allowed by the collection guardrails` + : `Rule "${form.title}" saved`); setEditingRuleId(null); setJsonModeEdit(false); setJsonError(null); diff --git a/src/Web/autopilot-monitor-web/app/gather-rules/types.ts b/src/Web/autopilot-monitor-web/app/gather-rules/types.ts index cf398f84..a929e164 100644 --- a/src/Web/autopilot-monitor-web/app/gather-rules/types.ts +++ b/src/Web/autopilot-monitor-web/app/gather-rules/types.ts @@ -1,3 +1,5 @@ +import { validateGatherRuleTarget } from "@/utils/guardValidation"; + export interface GatherRule { ruleId: string; title: string; @@ -126,7 +128,7 @@ export const TARGET_PLACEHOLDERS: Record = { export const TARGET_HINTS: Record = { registry: "Full registry path including hive (HKLM, HKCU). The agent reads values from this key.", eventlog: "Event log name — supports operational/analytic logs like Microsoft-Windows-Shell-Core/Operational.", - wmi: "Full WQL query (SELECT * FROM ...). Must use an allowed WMI class.", + wmi: "Full WQL query — SELECT * or a comma-separated property list from an allowed WMI class, e.g. SELECT BatteryStatus FROM Win32_Battery. Pairs well with Emit Mode \"On change\": polling a single property only emits when that property changes.", file: "File path. Environment variables like %ProgramData% are supported. Must be within allowed directories.", command_allowlisted: "Exact command string from the agent's allowlist. Custom commands are not permitted.", logparser: "Path to a log file. Supports wildcards (* and ?) in the filename, e.g. AppWorkload-*.log. Environment variables are expanded.", @@ -203,6 +205,23 @@ export function supportsEmitMode(form: Pick & Partial>, + unrestrictedMode: boolean +): boolean { + if (rule.isBuiltIn || rule.isCommunity) return false; + if (!rule.target?.trim()) return false; + // null = collector type the validator doesn't know — no verdict, don't gate. + const result = validateGatherRuleTarget(rule.collectorType, rule.target, unrestrictedMode); + return result !== null && !result.allowed; +} + /** * Rejects a scope mode that was selected but left unfilled. Without this the payload * silently sends null (= unrestricted) and the rule runs everywhere while the form still diff --git a/src/Web/autopilot-monitor-web/lib/__tests__/rulePageHelpers.test.ts b/src/Web/autopilot-monitor-web/lib/__tests__/rulePageHelpers.test.ts new file mode 100644 index 00000000..cc56ea9b --- /dev/null +++ b/src/Web/autopilot-monitor-web/lib/__tests__/rulePageHelpers.test.ts @@ -0,0 +1,23 @@ +import { describe, it, expect } from "vitest"; +import { bumpVersion } from "../rulePageHelpers"; + +describe("bumpVersion", () => { + it.each([ + ["1.0", "1.1"], + ["1.9", "1.10"], + ["1.0.0", "1.1.0"], // three segments stay three segments + ["2.3.7", "2.4.0"], // patch resets on a minor bump + ["1.0.0.5", "1.1.0.0"], + ])("bumps %s to %s preserving the segment count", (input, expected) => { + expect(bumpVersion(input)).toBe(expected); + }); + + it("falls back to 1.1 for empty or missing input", () => { + expect(bumpVersion("")).toBe("1.1"); + expect(bumpVersion(undefined as unknown as string)).toBe("1.1"); + }); + + it("treats a non-numeric minor as 0", () => { + expect(bumpVersion("1.x")).toBe("1.1"); + }); +}); diff --git a/src/Web/autopilot-monitor-web/lib/rulePageHelpers.ts b/src/Web/autopilot-monitor-web/lib/rulePageHelpers.ts index 1c91016d..f05343af 100644 --- a/src/Web/autopilot-monitor-web/lib/rulePageHelpers.ts +++ b/src/Web/autopilot-monitor-web/lib/rulePageHelpers.ts @@ -25,10 +25,16 @@ export function stripInternalFields; } -/** Bump a semver-style version string: "1.0" -> "1.1", "1.9" -> "1.10", etc. */ +/** + * Bump a semver-style version string's minor segment, preserving the segment + * count and zeroing anything after the minor: "1.0" -> "1.1", "1.9" -> "1.10", + * "1.0.0" -> "1.1.0", "2.3.7" -> "2.4.0". + */ export function bumpVersion(v: string): string { - const parts = (v ?? "1.0").split("."); - const major = parts[0] ?? "1"; - const minor = parseInt(parts[1] ?? "0", 10); - return `${major}.${minor + 1}`; + const parts = (v || "1.0").split("."); + const major = parts[0] || "1"; + const parsedMinor = parseInt(parts[1] ?? "0", 10); + const minor = Number.isNaN(parsedMinor) ? 0 : parsedMinor; + const trailingZeros = parts.slice(2).map(() => "0"); + return [major, `${minor + 1}`, ...trailingZeros].join("."); } 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 088199dd..b22f8117 100644 --- a/src/Web/autopilot-monitor-web/utils/__tests__/guardValidation.test.ts +++ b/src/Web/autopilot-monitor-web/utils/__tests__/guardValidation.test.ts @@ -196,6 +196,32 @@ describe("validateWmiTarget", () => { const r = validateWmiTarget("", false); expect(r.allowed).toBe(false); }); + + // Property projections of an allowed class expose a strict subset of the + // already-allowed SELECT * and are therefore admitted. + it.each([ + "SELECT BatteryStatus FROM Win32_Battery", + "SELECT BatteryStatus, EstimatedChargeRemaining FROM Win32_Battery", + "SELECT BatteryStatus,EstimatedChargeRemaining FROM Win32_Battery", + "select batterystatus from win32_battery", + "SELECT\tBatteryStatus\tFROM\tWin32_Battery", + " SELECT BatteryStatus FROM Win32_Battery ", + "SELECT Status FROM Win32_Battery WHERE BatteryStatus = 1", + ])("allows projection of an allowed class: %s", (query) => { + expect(validateWmiTarget(query, false).allowed).toBe(true); + }); + + it.each([ + "SELECT Name FROM Win32_Process", // class not on the allowlist + "SELECT BatteryStatus FROM Win32_BatteryX", // class spoofing + "SELECT *, Name FROM Win32_BIOS", // star mixed into a property list + "SELECT FROM Win32_BIOS", // empty property list + "SELECT BatteryStatus FROM", // missing class + "SELECT BatteryStatus FROM Win32_Battery;DROP", // non-whitespace boundary + "SELECT BatteryStatus, FROM Win32_Battery", // trailing comma + ])("rejects malformed or disallowed projection: %s", (query) => { + expect(validateWmiTarget(query, false).allowed).toBe(false); + }); }); // --------------------------------------------------------------------------- diff --git a/src/Web/autopilot-monitor-web/utils/guardValidation.ts b/src/Web/autopilot-monitor-web/utils/guardValidation.ts index 04f6eb13..0d80794a 100644 --- a/src/Web/autopilot-monitor-web/utils/guardValidation.ts +++ b/src/Web/autopilot-monitor-web/utils/guardValidation.ts @@ -39,6 +39,29 @@ const ALLOWED_COMMANDS_SET = new Set( ALLOWED_COMMANDS_LIST.map((c) => c.toLowerCase()) ); +// Canonical WMI allowlist entry: exactly "SELECT * FROM ". +// Such entries contribute their class name; any other entry (none today) +// stays on the legacy literal-prefix path. Mirrors the agent's +// GatherRuleGuards derivation. +const CANONICAL_WMI_ENTRY_REGEX = /^SELECT\s+\*\s+FROM\s+([A-Za-z_][A-Za-z0-9_]*)$/i; + +// WQL shape "SELECT <* | property-list> FROM ", class token bounded by +// whitespace or end of string (a trailing WHERE clause is permitted, +// "Win32_BIOSX" cannot pass as "Win32_BIOS"). +const WMI_QUERY_REGEX = + /^SELECT\s+(?:\*|[A-Za-z_][A-Za-z0-9_]*(?:\s*,\s*[A-Za-z_][A-Za-z0-9_]*)*)\s+FROM\s+([A-Za-z_][A-Za-z0-9_]*)(?=\s|$)/i; + +const ALLOWED_WMI_CLASSES = new Set(); +const RESIDUAL_WMI_PREFIXES: string[] = []; +for (const entry of ALLOWED_WMI_QUERY_PREFIXES) { + const match = CANONICAL_WMI_ENTRY_REGEX.exec(entry.trim()); + if (match) { + ALLOWED_WMI_CLASSES.add(match[1].toLowerCase()); + } else { + RESIDUAL_WMI_PREFIXES.push(entry); + } +} + const BLOCKED_USERS_PREFIX = BLOCKED_FILE_PREFIXES[0] || "C:\\Users"; // --------------------------------------------------------------------------- @@ -267,7 +290,15 @@ export function validateWmiTarget( return { allowed: true, reason: "All WMI queries allowed in unrestricted mode", unrestricted: true }; } - for (const prefix of ALLOWED_WMI_QUERY_PREFIXES) { + // "SELECT <* | property-list> FROM " against an allowed class — a + // property projection exposes a strict subset of the already-allowed SELECT *. + const match = WMI_QUERY_REGEX.exec(trimmed); + if (match && ALLOWED_WMI_CLASSES.has(match[1].toLowerCase())) { + return { allowed: true, reason: `Class ${match[1]} is on the WMI allowlist`, unrestricted: false }; + } + + // Non-canonical allowlist entries keep the legacy literal-prefix match. + for (const prefix of RESIDUAL_WMI_PREFIXES) { if (trimmed.length >= prefix.length) { const candidate = trimmed.substring(0, prefix.length); if (candidate.toLowerCase() === prefix.toLowerCase()) { @@ -280,7 +311,7 @@ export function validateWmiTarget( return { allowed: false, - reason: "Not a recognized WMI query prefix", + reason: "Not an allowed WMI query — use SELECT FROM ", unrestricted: false, }; }