diff --git a/src/Agent/AutopilotMonitor.Agent.V2.Core.Tests/Configuration/RemoteConfigServiceCacheHardeningTests.cs b/src/Agent/AutopilotMonitor.Agent.V2.Core.Tests/Configuration/RemoteConfigServiceCacheHardeningTests.cs new file mode 100644 index 00000000..36b276b7 --- /dev/null +++ b/src/Agent/AutopilotMonitor.Agent.V2.Core.Tests/Configuration/RemoteConfigServiceCacheHardeningTests.cs @@ -0,0 +1,239 @@ +using System; +using System.IO; +using System.Net.Http; +using System.Reflection; +using AutopilotMonitor.Agent.V2.Core.Configuration; +using AutopilotMonitor.Agent.V2.Core.Logging; +using AutopilotMonitor.Agent.V2.Core.Monitoring.Transport; +using AutopilotMonitor.Agent.V2.Core.Tests.Harness; +using AutopilotMonitor.Shared.Models; +using Newtonsoft.Json; +using Xunit; + +namespace AutopilotMonitor.Agent.V2.Core.Tests.Configuration +{ + /// + /// Pins the OOBE-MITM cache-hardening contract on + /// (RemoteConfigService.cs:234-311). The on-disk config cache MUST NOT persist — nor, + /// on read, trust — security-sensitive fields: a single MITM during OOBE could otherwise + /// plant attacker-controlled values (UnrestrictedMode / AllowAgentDowngrade / + /// LatestAgentExeSha256 / DeviceBlocked / DeviceKillSignal / UnblockAt) that survive every + /// future cold boot. The strip is defence-in-depth on BOTH paths — CacheConfig (write) + /// and LoadCachedConfig (read) — so both are asserted here. + /// + /// The production cache path is a fixed %ProgramData%\AutopilotMonitor\Config location. + /// To exercise the REAL CacheConfig/LoadCachedConfig disk + serialization code in + /// isolation, the probe subclass exposes the two protected methods and the private + /// _cacheFilePath field is redirected to a per-test via + /// reflection. Nothing about the strip logic itself is faked. + /// + /// + public sealed class RemoteConfigServiceCacheHardeningTests : IDisposable + { + private readonly TempDirectory _tmp = new TempDirectory(); + private readonly AgentLogger _logger; + private readonly string _cachePath; + + public RemoteConfigServiceCacheHardeningTests() + { + _logger = new AgentLogger(Path.Combine(_tmp.Path, "logs"), AgentLogLevel.Info); + _cachePath = Path.Combine(_tmp.Path, "remote-config.json"); + } + + public void Dispose() => _tmp.Dispose(); + + // ── Write path: CacheConfig strips before serializing to disk ──────── + + [Fact] + public void CacheConfig_strips_all_security_sensitive_fields_on_disk() + { + var probe = NewProbe(); + + // A config carrying a full set of attacker-controlled security values, as if a live + // (or MITM'd) fetch returned them. Benign fields are set to non-default markers so the + // round-trip can prove they are NOT collateral-stripped. + var config = new AgentConfigResponse + { + ConfigVersion = 77, + NtpServer = "pool.attacker.example", + UnrestrictedMode = true, + AllowAgentDowngrade = true, + LatestAgentExeSha256 = "deadbeefdeadbeefdeadbeefdeadbeefdeadbeefdeadbeefdeadbeefdeadbeef", + DeviceBlocked = true, + DeviceKillSignal = true, + UnblockAt = new DateTime(2099, 1, 1, 0, 0, 0, DateTimeKind.Utc), + }; + + probe.DoCache(config); + + // Read the file the production code actually wrote and deserialize with the same lib. + var raw = File.ReadAllText(_cachePath); + var persisted = JsonConvert.DeserializeObject(raw); + + Assert.False(persisted.UnrestrictedMode); + Assert.False(persisted.AllowAgentDowngrade); + Assert.Null(persisted.LatestAgentExeSha256); + Assert.False(persisted.DeviceBlocked); + Assert.False(persisted.DeviceKillSignal); + Assert.Null(persisted.UnblockAt); + + // The attacker EXE hash must not appear anywhere in the persisted bytes. + Assert.DoesNotContain("deadbeef", raw); + + // Benign fields survive the strip. + Assert.Equal(77, persisted.ConfigVersion); + Assert.Equal("pool.attacker.example", persisted.NtpServer); + + // The in-memory object is restored after serialization — CacheConfig must not + // permanently mutate the live config it was handed (it is still in use by the agent). + Assert.True(config.UnrestrictedMode); + Assert.True(config.AllowAgentDowngrade); + Assert.Equal("deadbeefdeadbeefdeadbeefdeadbeefdeadbeefdeadbeefdeadbeefdeadbeef", config.LatestAgentExeSha256); + Assert.True(config.DeviceBlocked); + Assert.True(config.DeviceKillSignal); + Assert.Equal(new DateTime(2099, 1, 1, 0, 0, 0, DateTimeKind.Utc), config.UnblockAt); + } + + // ── Read path: LoadCachedConfig strips a planted (attacker) cache file ─ + + [Theory] + [InlineData("UnrestrictedMode")] + [InlineData("AllowAgentDowngrade")] + [InlineData("LatestAgentExeSha256")] + [InlineData("DeviceBlocked")] + [InlineData("DeviceKillSignal")] + [InlineData("UnblockAt")] + public void LoadCachedConfig_strips_planted_security_sensitive_field(string field) + { + var probe = NewProbe(); + + // Simulate an OOBE-MITM (or an older cache format) having persisted a cache file that + // DOES carry attacker values on every sensitive field — written out-of-band, bypassing + // the write-side strip. The read-side strip is the defence-in-depth that must still hold. + var attacker = new AgentConfigResponse + { + ConfigVersion = 5, + UnrestrictedMode = true, + AllowAgentDowngrade = true, + LatestAgentExeSha256 = "0011223344556677889900112233445566778899001122334455667788990011", + DeviceBlocked = true, + DeviceKillSignal = true, + UnblockAt = new DateTime(2099, 1, 1, 0, 0, 0, DateTimeKind.Utc), + }; + File.WriteAllText(_cachePath, JsonConvert.SerializeObject(attacker, Formatting.Indented)); + + var loaded = probe.DoLoad(); + + Assert.NotNull(loaded); + switch (field) + { + case "UnrestrictedMode": + Assert.False(loaded.UnrestrictedMode); + break; + case "AllowAgentDowngrade": + Assert.False(loaded.AllowAgentDowngrade); + break; + case "LatestAgentExeSha256": + Assert.Null(loaded.LatestAgentExeSha256); + break; + case "DeviceBlocked": + Assert.False(loaded.DeviceBlocked); + break; + case "DeviceKillSignal": + Assert.False(loaded.DeviceKillSignal); + break; + case "UnblockAt": + Assert.Null(loaded.UnblockAt); + break; + default: + throw new InvalidOperationException($"unknown field {field}"); + } + + // Benign data from the planted file is still loaded — the strip is surgical. + Assert.Equal(5, loaded.ConfigVersion); + } + + // ── Round-trip: benign fields survive write + read unchanged ───────── + + [Fact] + public void RoundTrip_preserves_non_sensitive_fields() + { + var probe = NewProbe(); + + var config = new AgentConfigResponse + { + ConfigVersion = 26, + UploadIntervalSeconds = 45, + MaxBatchSize = 250, + SelfDestructOnComplete = false, + KeepLogFile = true, + EnableGeoLocation = false, + NtpServer = "time.contoso.com", + LogLevel = "Debug", + DiagnosticsUploadMode = "OnFailure", + // A non-sensitive hash field (ZIP hash) that must NOT be stripped — only the + // EXE hash (LatestAgentExeSha256) is security-sensitive on the cache path. + LatestAgentSha256 = "cafebabecafebabecafebabecafebabecafebabecafebabecafebabecafebabe", + }; + + probe.DoCache(config); + var loaded = probe.DoLoad(); + + Assert.NotNull(loaded); + Assert.Equal(26, loaded.ConfigVersion); + Assert.Equal(45, loaded.UploadIntervalSeconds); + Assert.Equal(250, loaded.MaxBatchSize); + Assert.False(loaded.SelfDestructOnComplete); + Assert.True(loaded.KeepLogFile); + Assert.False(loaded.EnableGeoLocation); + Assert.Equal("time.contoso.com", loaded.NtpServer); + Assert.Equal("Debug", loaded.LogLevel); + Assert.Equal("OnFailure", loaded.DiagnosticsUploadMode); + Assert.Equal("cafebabecafebabecafebabecafebabecafebabecafebabecafebabecafebabe", loaded.LatestAgentSha256); + } + + // ── Helpers ───────────────────────────────────────────────────────── + + private CacheProbe NewProbe() + { + // A throwaway BackendApiClient — the cache-path methods never hit the wire. + var apiClient = new BackendApiClient( + httpClient: new HttpClient(), + baseUrl: "http://localhost", + manufacturer: string.Empty, + model: string.Empty, + serialNumber: string.Empty, + useBootstrapTokenAuth: false, + bootstrapToken: null, + agentVersion: "0.0.0", + logger: _logger); + + var probe = new CacheProbe(apiClient, _logger); + RedirectCachePath(probe, _cachePath); + return probe; + } + + // Redirect the private, constructor-computed %ProgramData% cache path to the per-test temp + // file so the REAL CacheConfig/LoadCachedConfig disk code runs in isolation. + private static void RedirectCachePath(RemoteConfigService svc, string path) + { + var field = typeof(RemoteConfigService).GetField( + "_cacheFilePath", BindingFlags.Instance | BindingFlags.NonPublic); + Assert.NotNull(field); // guards against a rename silently disabling the redirect + field.SetValue(svc, path); + } + + // Minimal subclass: exposes the two protected cache seams so the real strip logic can be + // driven directly, without standing up the retry/fetch machinery. + private sealed class CacheProbe : RemoteConfigService + { + public CacheProbe(BackendApiClient apiClient, AgentLogger logger) + : base(apiClient, "00000000-0000-0000-0000-000000000001", logger) + { + } + + public void DoCache(AgentConfigResponse config) => CacheConfig(config); + public AgentConfigResponse DoLoad() => LoadCachedConfig(); + } + } +} diff --git a/src/Agent/AutopilotMonitor.Agent.V2.Core.Tests/Monitoring/Ime/CmTraceLogParserTests.cs b/src/Agent/AutopilotMonitor.Agent.V2.Core.Tests/Monitoring/Ime/CmTraceLogParserTests.cs new file mode 100644 index 00000000..edbc885b --- /dev/null +++ b/src/Agent/AutopilotMonitor.Agent.V2.Core.Tests/Monitoring/Ime/CmTraceLogParserTests.cs @@ -0,0 +1,93 @@ +using System; +using AutopilotMonitor.Agent.V2.Core.Monitoring.Enrollment.Ime; +using Xunit; + +namespace AutopilotMonitor.Agent.V2.Core.Tests.Monitoring.Ime +{ + /// + /// Hostile-input hardening for + /// (CmTraceLogParser.cs:35-73). The parser runs on every IME log line — including partially + /// flushed, BOM-prefixed, junk-prefixed and pathologically long lines written by Windows while + /// the file is still being appended. It must never throw; it either returns a populated + /// or reports a clean "does not match" (false). These pin CURRENT + /// behaviour. + /// + public sealed class CmTraceLogParserTests + { + // Same well-formed CMTrace line shape as StallProbeActiveInstallFilterTests' helper. + private static string CmTraceLine(string message, string time, string date) => + $""; + + // ── Non-matching / malformed input → clean false, never a throw ─────── + + [Theory] + [InlineData(null)] // null + [InlineData("")] // empty + [InlineData(" ")] // whitespace only + [InlineData("")] // leading BOM breaks the StartsWith gate + [InlineData("garbage prefix ")] // leading junk + public void TryParseLine_returns_false_without_throwing_for_malformed_input(string line) + { + var ok = CmTraceLogParser.TryParseLine(line, out var entry); + + Assert.False(ok); + Assert.Null(entry); + } + + // ── Well-formed line → parses ──────────────────────────────────────── + + [Fact] + public void TryParseLine_parses_well_formed_line() + { + var line = CmTraceLine("EnforcementState: Installing app X", "06:08:04.8834397", "2-8-2026"); + + var ok = CmTraceLogParser.TryParseLine(line, out var entry); + + Assert.True(ok); + Assert.NotNull(entry); + Assert.Equal("EnforcementState: Installing app X", entry.Message); + Assert.Equal("AppEnforce", entry.Component); + Assert.Equal(1, entry.Type); + Assert.NotEqual(default, entry.Timestamp); + } + + // ── Structurally valid but unparseable timestamp → UtcNow fallback ──── + + [Fact] + public void TryParseLine_falls_back_to_utcnow_for_unparseable_timestamp() + { + // time/date satisfy the regex character classes ([\d:.]+ / [\d-]+) so the line MATCHES, + // but "13-45-2026" / "25:99:99" fail every DateTime format → the parser falls back to + // DateTime.UtcNow (CmTraceLogParser.cs:57) instead of throwing. + var before = DateTime.UtcNow; + var line = CmTraceLine("some message", "25:99:99.0000000", "13-45-2026"); + + var ok = CmTraceLogParser.TryParseLine(line, out var entry); + var after = DateTime.UtcNow; + + Assert.True(ok); + Assert.NotNull(entry); + Assert.Equal("some message", entry.Message); + // Don't assert an exact instant — only that the fallback stamped a fresh "now" + // (a successfully-parsed date would land in 2026, far outside this window). + Assert.InRange(entry.Timestamp, before.AddSeconds(-5), after.AddSeconds(5)); + } + + // ── Oversized line → parses, no throw, no truncation of the message ─── + + [Fact] + public void TryParseLine_handles_oversized_line() + { + var hugeMessage = new string('x', 200_000); + var line = CmTraceLine(hugeMessage, "06:08:04.8834397", "2-8-2026"); + + var ok = CmTraceLogParser.TryParseLine(line, out var entry); + + Assert.True(ok); + Assert.NotNull(entry); + Assert.Equal(hugeMessage.Length, entry.Message.Length); + } + } +} diff --git a/src/Agent/AutopilotMonitor.Agent.V2.Core.Tests/Monitoring/Ime/ImeLogPatternReDoSTests.cs b/src/Agent/AutopilotMonitor.Agent.V2.Core.Tests/Monitoring/Ime/ImeLogPatternReDoSTests.cs new file mode 100644 index 00000000..1d05718c --- /dev/null +++ b/src/Agent/AutopilotMonitor.Agent.V2.Core.Tests/Monitoring/Ime/ImeLogPatternReDoSTests.cs @@ -0,0 +1,105 @@ +using System.Collections.Generic; +using AutopilotMonitor.Agent.V2.Core.Logging; +using AutopilotMonitor.Agent.V2.Core.Monitoring.Enrollment.Ime; +using AutopilotMonitor.Agent.V2.Core.Tests.Harness; +using AutopilotMonitor.Shared.Models; +using Xunit; + +namespace AutopilotMonitor.Agent.V2.Core.Tests.Monitoring.Ime +{ + /// + /// Backend-supplied IME log patterns are untrusted regex delivered on the config channel — a + /// bad (or malicious) tenant/global pattern must never crash the tracker. Two defences are + /// pinned here: + /// + /// compile-time: an INVALID pattern is swallowed in + /// (ImeLogTracker.cs:452-487) — the tracker constructs, valid siblings still compile, and the + /// tracker stays usable. + /// match-time: every pattern is compiled with a 1 s MatchTimeout + /// (ImeLogTracker.cs:458); a catastrophic-backtracking pattern + adversarial input raises + /// , which the per-line + /// match loop catches (ImeLogTracker.LogProcessing.cs:177-180 / the ProcessLogMessageForTest + /// seam:482-489) rather than propagating. + /// + /// + public sealed class ImeLogPatternReDoSTests + { + private static ImeLogPattern Pattern(string id, string regex, string action = null) => new ImeLogPattern + { + PatternId = id, + Pattern = regex, + Category = "always", + Action = action, + Enabled = true, + Parameters = new Dictionary(), + }; + + private static ImeLogTracker BuildTracker(TempDirectory tmp, List patterns) => + new ImeLogTracker( + logFolder: tmp.Path, + patterns: patterns, + logger: new AgentLogger(tmp.Path, AgentLogLevel.Info)); + + // ── Compile-time swallow ───────────────────────────────────────────── + + [Theory] + [InlineData("(")] // unbalanced group + [InlineData("[a-")] // unterminated character class + [InlineData("(?")] // unterminated named group + [InlineData("*invalid")] // quantifier with nothing to quantify + [InlineData("\\")] // dangling escape + public void Invalid_backend_pattern_is_swallowed_and_tracker_stays_usable(string badRegex) + { + using var tmp = new TempDirectory(); + + // Construction compiles the patterns; the bad one must not surface as an exception. + using var tracker = BuildTracker(tmp, new List { Pattern("BAD", badRegex) }); + + // And the tracker is still functional — driving a line through the (empty) active-pattern + // set is a no-op, not a throw. + var ex = Record.Exception(() => tracker.ProcessLogMessageForTest("any line here")); + Assert.Null(ex); + } + + [Fact] + public void Valid_patterns_still_compile_when_a_sibling_pattern_is_invalid() + { + using var tmp = new TempDirectory(); + + var fired = false; + using var tracker = BuildTracker(tmp, new List + { + Pattern("BAD", "("), // dropped at compile + Pattern("IME-START", "IME Agent Started", action: "imeStarted"), // survives + }); + tracker.OnImeStarted = () => fired = true; + + tracker.ProcessLogMessageForTest("Info: IME Agent Started (v1.2.3)"); + + // The invalid sibling did not poison the batch — the valid pattern compiled and fired. + Assert.True(fired); + } + + // ── Match-time timeout swallow (ReDoS) ─────────────────────────────── + + [Fact] + public void Catastrophic_backtracking_pattern_hits_match_timeout_and_is_swallowed() + { + using var tmp = new TempDirectory(); + + // Classic exponential-backtracking pattern; the trailing non-matching char forces the + // engine to explore the full 2^n search space, which blows past the 1 s MatchTimeout. + using var tracker = BuildTracker(tmp, new List + { + Pattern("REDOS", "^(a+)+$"), + }); + + var adversarialInput = new string('a', 40) + "!"; + + // The RegexMatchTimeoutException raised inside the match loop must be caught, not + // propagated — the tracker survives a hostile backend pattern + hostile log line. + var ex = Record.Exception(() => tracker.ProcessLogMessageForTest(adversarialInput)); + Assert.Null(ex); + } + } +} diff --git a/src/Agent/AutopilotMonitor.Agent.V2.Core.Tests/Security/CertificateHelperFindMdmCertTests.cs b/src/Agent/AutopilotMonitor.Agent.V2.Core.Tests/Security/CertificateHelperFindMdmCertTests.cs new file mode 100644 index 00000000..6445dc57 --- /dev/null +++ b/src/Agent/AutopilotMonitor.Agent.V2.Core.Tests/Security/CertificateHelperFindMdmCertTests.cs @@ -0,0 +1,71 @@ +using System; +using System.Security.Cryptography; +using System.Security.Cryptography.X509Certificates; +using AutopilotMonitor.Agent.V2.Core.Security; +using Xunit; + +// NOTE: FindMdmCertificate (CertificateHelper.cs:20-101) is NOT unit-testable as written — it +// opens LocalMachine\My and CurrentUser\My X509Store instances directly and performs the +// issuer-match ("CN=Microsoft Intune MDM Device CA", explicitly NOT the MMP-C +// "CN=Microsoft Intune Device Management Device CA"), the ClientAuth EKU (1.3.6.1.5.5.7.3.2) +// filter, and the NotAfter tiebreaker inline inside that store loop. There is no pure seam +// (e.g. an internal selection method over an IEnumerable or an issuer +// predicate) to drive those rules without a real machine cert store. Per the project rule we do +// NOT fake the store. Refactor needed to make the security-critical selection testable: extract a +// `SelectMdmCertificate(IEnumerable)` pure method and unit-test the issuer +// discrimination + NotAfter tiebreaker against it. Until then this file covers only the pure +// helper actually exposed: IsCertificateValid. +namespace AutopilotMonitor.Agent.V2.Core.Tests.Security +{ + /// + /// Covers the pure, store-independent portion of — + /// (CertificateHelper.cs:106-113), the + /// NotBefore/NotAfter validity-window check. See the file-top NOTE for why FindMdmCertificate's + /// selection logic is out of unit-test reach. + /// + public sealed class CertificateHelperFindMdmCertTests + { + /// + /// In-memory self-signed cert with an explicit validity window (days relative to now), + /// so the store is never touched. Mirrors the CreateSelfSignedCert pattern used by + /// MtlsHttpClientFactoryTests. Windows are kept >= 1 day away from "now" so the + /// helper's local-vs-UTC comparison skew cannot flip the expected result. + /// + private static X509Certificate2 CreateCert(int notBeforeDays, int notAfterDays) + { + using (var rsa = RSA.Create(2048)) + { + var req = new CertificateRequest( + "CN=apmon-v2-test", + rsa, + HashAlgorithmName.SHA256, + RSASignaturePadding.Pkcs1); + + return req.CreateSelfSigned( + DateTimeOffset.UtcNow.AddDays(notBeforeDays), + DateTimeOffset.UtcNow.AddDays(notAfterDays)); + } + } + + [Theory] + // Currently within the window → valid. + [InlineData(-1, 1, true)] + [InlineData(-30, 30, true)] + // Expired (NotAfter in the past) → invalid. + [InlineData(-10, -1, false)] + // Not yet valid (NotBefore in the future) → invalid. + [InlineData(1, 10, false)] + public void IsCertificateValid_reflects_validity_window(int notBeforeDays, int notAfterDays, bool expected) + { + using var cert = CreateCert(notBeforeDays, notAfterDays); + + Assert.Equal(expected, CertificateHelper.IsCertificateValid(cert)); + } + + [Fact] + public void IsCertificateValid_returns_false_for_null() + { + Assert.False(CertificateHelper.IsCertificateValid(null)); + } + } +} diff --git a/src/Backend/AutopilotMonitor.Functions.Tests/AppInsightsQueryFunctionTests.cs b/src/Backend/AutopilotMonitor.Functions.Tests/AppInsightsQueryFunctionTests.cs new file mode 100644 index 00000000..95a9261f --- /dev/null +++ b/src/Backend/AutopilotMonitor.Functions.Tests/AppInsightsQueryFunctionTests.cs @@ -0,0 +1,81 @@ +using AutopilotMonitor.Functions.Middleware; +using AutopilotMonitor.Functions.Security; +using Xunit; + +namespace AutopilotMonitor.Functions.Tests; + +/// +/// Tests for the POST /api/global/raw/logs KQL-proxy endpoint served by +/// — an operator tool +/// that forwards an operator-supplied KQL query VERBATIM to the Application Insights REST API. Because +/// the query is passed through un-sandboxed and the App Insights workspace can surface secret-bearing +/// backend traces, the TRUST BOUNDARY for this endpoint is its authorization gate: only a platform +/// Global Admin may call it. These tests pin that gate. +/// +/// SCOPE / WHAT IS AND IS NOT COVERED HERE (deliberate): +/// The function's own in-request behavior — appId-not-configured → 503, empty/whitespace query → +/// 400 BadRequest, App-Insights-error → 502 BadGateway structured error (innererror message + code + +/// SyntaxError/SemanticError hint), happy path → 200 with the raw AI tables JSON — is NOT exercised +/// here. The entrypoint Run sits directly on the abstract HttpRequestData / +/// HttpResponseData pair (whose HttpResponseData.Cookies returns the abstract +/// HttpCookies), reads the body via ReadFromJsonAsync and writes via +/// CreateResponse/WriteAsJsonAsync (all of which need a live worker serializer resolved +/// from FunctionContext.InstanceServices), and calls Application Insights through a PRIVATE +/// STATIC HttpClient + DefaultAzureCredential with no injection seam. Faking that whole +/// pipeline + a live Managed Identity is "more setup than the test is worth" — the exact rationale the +/// sibling function tests cite (see DeviceBlockFunctionTests / GetAllBlockedDevicesFunctionTests, +/// which likewise test the reachable seam instead of the HTTP entrypoint). The authorization gate +/// below IS the security-relevant, compilable seam for this endpoint. +/// +/// The gate is asserted through (the single source +/// of truth the running middleware consults) and . +/// +public class AppInsightsQueryFunctionTests +{ + private const string LogsRoute = "/api/global/raw/logs"; + + // ── Authorization gate: the KQL proxy is Global-Admin-only ── + [Fact] + public void RawLogs_post_is_registered_as_GlobalAdminOnly() + { + var entry = EndpointAccessPolicyCatalog.FindPolicy("POST", LogsRoute); + + Assert.NotNull(entry); + Assert.Equal(EndpointPolicy.GlobalAdminOnly, entry!.Policy); + } + + // ── The raw family (logs + tables) is deliberately kept OFF the read-only Global Reader tier: + // these endpoints can dump secret-bearing rows/traces and would bypass the GlobalReader config + // redaction, so they stay GlobalAdminOnly. Regression guard for that catalog decision. ── + [Theory] + [InlineData("POST", "/api/global/raw/logs")] + [InlineData("GET", "/api/global/raw/tables")] + [InlineData("GET", "/api/global/raw/tables/TenantConfiguration")] + public void Raw_family_endpoints_are_GlobalAdminOnly_not_global_reader(string method, string path) + { + var entry = EndpointAccessPolicyCatalog.FindPolicy(method, path); + + Assert.NotNull(entry); + Assert.Equal(EndpointPolicy.GlobalAdminOnly, entry!.Policy); + // Explicitly NOT the read-only cross-tenant tier. + Assert.NotEqual(EndpointPolicy.GlobalReadOrAdmin, entry.Policy); + } + + // ── A full, un-exempt JWT is required to reach the endpoint (it is not anonymous/device-exempt), + // which — combined with the GlobalAdminOnly policy — is what confines the raw KQL passthrough to + // platform admins. Documents the trust boundary end to end. ── + [Fact] + public void RawLogs_post_requires_jwt_and_is_not_exempt() + { + Assert.False(AuthenticationMiddleware.SkipsJwtValidation("POST", LogsRoute)); + } + + // ── Verb binding: the KQL proxy is POST-only. A GET to the same path is unregistered and therefore + // fail-closed (FindPolicy == null → the middleware denies). Guards against a future GET alias that + // would carry the KQL in the query string (and into logs/URLs). ── + [Fact] + public void RawLogs_get_is_unregistered_and_fails_closed() + { + Assert.Null(EndpointAccessPolicyCatalog.FindPolicy("GET", LogsRoute)); + } +} diff --git a/src/Backend/AutopilotMonitor.Functions.Tests/AuthenticationMiddlewareTests.cs b/src/Backend/AutopilotMonitor.Functions.Tests/AuthenticationMiddlewareTests.cs new file mode 100644 index 00000000..f5b1dcb6 --- /dev/null +++ b/src/Backend/AutopilotMonitor.Functions.Tests/AuthenticationMiddlewareTests.cs @@ -0,0 +1,112 @@ +using AutopilotMonitor.Functions.Middleware; +using Xunit; + +namespace AutopilotMonitor.Functions.Tests; + +/// +/// Tests for via its transport-agnostic +/// SkipsJwtValidation(method, path) seam — the JWT-exemption decision, DERIVED from +/// (never a hand-kept +/// parallel allowlist), so a new anonymous/device route can't drift out of sync and get 401'd before +/// PolicyEnforcementMiddleware honors its policy. +/// +/// SCOPE / WHAT IS AND IS NOT COVERED HERE (deliberate — mirrors the sibling function tests' +/// "more setup than the test is worth" rationale, e.g. DeviceBlockFunctionTests / +/// GetAllBlockedDevicesFunctionTests, and PolicyEnforcementMiddlewareTests which tests its own +/// transport-agnostic DecideAsync seam rather than Invoke): +/// +/// The signature-validation rejects that performs — +/// • alg: none / HS-family blocked by the RS256/PS256 whitelist (ValidAlgorithms), +/// • expired token (ValidateLifetime), +/// • wrong audience (ValidAudiences = EntraId:ClientId + api://EntraId:ClientId), +/// • non-GUID / missing tid pre-reject (before any OIDC-metadata fetch), +/// are NOT unit-tested here. They live INLINE inside Invoke behind two heavy boundaries: +/// (1) a live OIDC ConfigurationManager that fetches tenant signing keys over the network, +/// and (2) the ASP.NET Core HttpContext obtained via FunctionContext.GetHttpContext(). +/// The TokenValidationParameters are built inline (not an extracted seam), so exercising those +/// rejects would require booting the worker HTTP pipeline + outbound OIDC — out of scope for a unit +/// test and untestable without live infra. The SkipsJwtValidation seam below is the smallest +/// validatable one and is exactly the security-relevant fork (exempt route vs. JWT-required route). +/// +public class AuthenticationMiddlewareTests +{ + // ── Exempt: PublicAnonymous + DeviceOrBootstrapAuth routes bypass JWT validation ── + // (The middleware short-circuits to next() for these; JWT is validated later — or not at all — + // by the device/bootstrap auth path inside the function.) + [Theory] + // PublicAnonymous + [InlineData("GET", "/api/health")] + [InlineData("GET", "/api/stats/platform")] + [InlineData("GET", "/api/bootstrap/validate/ABC123")] + [InlineData("POST", "/api/agent/distress")] + [InlineData("GET", "/api/diagnostics/download")] + // DeviceOrBootstrapAuth + [InlineData("POST", "/api/agent/register-session")] + [InlineData("POST", "/api/agent/telemetry")] + [InlineData("GET", "/api/agent/config")] + [InlineData("POST", "/api/agent/upload-url")] + [InlineData("POST", "/api/agent/error")] + [InlineData("POST", "/api/bootstrap/register-session")] + [InlineData("GET", "/api/bootstrap/config")] + [InlineData("POST", "/api/bootstrap/error")] + public void SkipsJwtValidation_public_and_device_routes_are_exempt(string method, string path) + { + Assert.True(AuthenticationMiddleware.SkipsJwtValidation(method, path)); + } + + // ── NOT exempt: AuthenticatedUser and every higher tier require a valid JWT ── + [Theory] + [InlineData("GET", "/api/auth/me")] // AuthenticatedUser + [InlineData("GET", "/api/auth/is-global-admin")] // AuthenticatedUser + [InlineData("POST", "/api/realtime/negotiate")] // AuthenticatedUser + [InlineData("GET", "/api/progress/sessions")] // AuthenticatedUser + [InlineData("GET", "/api/sessions")] // MemberRead + [InlineData("GET", "/api/audit/logs")] // MemberRead + [InlineData("PUT", "/api/config/11111111-1111-1111-1111-111111111111")] // TenantAdminOrGA + [InlineData("GET", "/api/global/sessions")] // GlobalReadOrDelegatedSubset + [InlineData("POST", "/api/global/raw/logs")] // GlobalAdminOnly (KQL proxy) + [InlineData("GET", "/api/health/detailed")] // AuthenticatedUser (sub-route is NOT blanket-exempt) + [InlineData("GET", "/api/health/mcp")] // AuthenticatedUser + public void SkipsJwtValidation_authenticated_and_higher_routes_are_not_exempt(string method, string path) + { + Assert.False(AuthenticationMiddleware.SkipsJwtValidation(method, path)); + } + + // ── Fail-closed: unregistered route/verb requires JWT (FindPolicy == null → not exempt) ── + [Theory] + [InlineData("GET", "/api/does-not-exist")] // no catalog entry + [InlineData("POST", "/api/health")] // health is GET-only; POST is an unexpected verb + [InlineData("DELETE", "/api/agent/telemetry")] // device route but wrong verb + [InlineData("GET", "/api/global/raw/logs")] // logs proxy is POST-only; GET is unregistered + public void SkipsJwtValidation_unregistered_route_or_verb_is_not_exempt(string method, string path) + { + Assert.False(AuthenticationMiddleware.SkipsJwtValidation(method, path)); + } + + // ── Path normalization: the catalog strips the /api/ prefix, so the exemption decision is the + // same with or without it (the middleware passes HttpContext.Request.Path, which carries /api/). ── + [Theory] + [InlineData("health")] + [InlineData("/api/health")] + public void SkipsJwtValidation_normalizes_api_prefix_for_exempt_route(string path) + { + Assert.True(AuthenticationMiddleware.SkipsJwtValidation("GET", path)); + } + + [Theory] + [InlineData("sessions")] + [InlineData("/api/sessions")] + public void SkipsJwtValidation_normalizes_api_prefix_for_protected_route(string path) + { + Assert.False(AuthenticationMiddleware.SkipsJwtValidation("GET", path)); + } + + // ── Method-awareness: a public PATH does not wave through an unexpected VERB ── + [Fact] + public void SkipsJwtValidation_is_method_aware_on_public_path() + { + // GET health is public/exempt; POST to the same path is not registered → JWT required. + Assert.True(AuthenticationMiddleware.SkipsJwtValidation("GET", "/api/health")); + Assert.False(AuthenticationMiddleware.SkipsJwtValidation("POST", "/api/health")); + } +} diff --git a/src/Backend/AutopilotMonitor.Functions.Tests/IngestTelemetryAuthPathTests.cs b/src/Backend/AutopilotMonitor.Functions.Tests/IngestTelemetryAuthPathTests.cs new file mode 100644 index 00000000..e3053300 --- /dev/null +++ b/src/Backend/AutopilotMonitor.Functions.Tests/IngestTelemetryAuthPathTests.cs @@ -0,0 +1,197 @@ +using System.Threading; +using System.Threading.Tasks; +using Azure.Data.Tables; +using AutopilotMonitor.Functions.Functions.Ingest; +using AutopilotMonitor.Functions.Services.Deletion; +using AutopilotMonitor.Shared.Models; +using AutopilotMonitor.Shared.Models.Deletion; +using Microsoft.Extensions.Logging.Abstractions; +using Moq; +using Xunit; + +namespace AutopilotMonitor.Functions.Tests; + +/// +/// Auth / pre-persist gate coverage for . +/// +/// +/// HARNESS NOTE — why these are decision-seam tests, not a live Run() invocation. +/// This suite has no fixture that boots the Functions worker's response serializer, so +/// HttpRequestData.CreateResponse + HttpResponseData.WriteAsJsonAsync (which every +/// Run() exit path calls, incl. the 400/403/410 error paths via WriteErrorAsync) cannot be +/// exercised without inventing runtime infrastructure. This is the same call the sibling function +/// tests make explicitly — see / +/// ("mocking HttpRequestData + the middleware chain +/// is more setup than the test is worth"). So, exactly like , +/// this file pins the decision predicates and mapping helpers Run() branches on at their real +/// seams, not the terminal HTTP status emission. +/// +/// +/// +/// Coverage map to the four requested auth branches: +/// +/// missing X-Tenant-Id → 400: the string.IsNullOrEmpty(tenantIdHeader) gate +/// (Run L100). +/// tenant mismatch (header vs PartitionKey) → 403 / malformed PartitionKey → 400: the real +/// static + the +/// OrdinalIgnoreCase equality Run performs (Run L169-180). +/// session deletion locked → 410: the real throwing +/// (Run L205-213), plus the unique +/// that consumes the guard's row. +/// kill-switch / device-blocked → 200-block: NOT duplicated here. The verdict Run() routes +/// on (IsBlocked/IsKill/UnblockAt) is produced by +/// and is already exhaustively covered at the +/// exact seam Run() calls in (device Block/Kill, version +/// Block/Kill, device-block-short-circuit). Re-instantiating the evaluator here would add zero +/// coverage. +/// +/// +/// +public class IngestTelemetryAuthPathTests +{ + private const string TenantId = "a1b2c3d4-e5f6-7890-abcd-ef1234567890"; + private const string SessionId = "b2c3d4e5-f6a7-8901-bcde-f12345678901"; + + // ============================================================ Missing X-Tenant-Id → 400 + + [Theory] + [InlineData(null, true)] + [InlineData("", true)] + [InlineData(TenantId, false)] + public void MissingTenantHeader_gate_rejects_null_or_empty(string? tenantIdHeader, bool rejected) + { + // Run L100: `if (string.IsNullOrEmpty(tenantIdHeader)) return 400 "X-Tenant-Id header is required"`. + // An absent header surfaces here as null (Run maps Headers.Contains==false → null). + Assert.Equal(rejected, string.IsNullOrEmpty(tenantIdHeader)); + } + + // ============================================================ Tenant / PartitionKey auth decision + + public enum AuthDecision { Accept, MalformedPartitionKey, TenantMismatch } + + public static IEnumerable PartitionKeyAuthCases() + { + // header, partitionKey, expected Run() decision + yield return new object[] { TenantId, $"{TenantId}_{SessionId}", AuthDecision.Accept }; + // OrdinalIgnoreCase equality — same GUID, different casing must NOT be a mismatch (Run L174). + yield return new object[] { TenantId.ToUpperInvariant(), $"{TenantId}_{SessionId}", AuthDecision.Accept }; + // Different tenant GUID in the body → 403. + yield return new object[] { TenantId, $"c3d4e5f6-a7b8-9012-cdef-012345678901_{SessionId}", AuthDecision.TenantMismatch }; + // Unparseable PartitionKey → 400 before the mismatch check even runs (Run L169). + yield return new object[] { TenantId, "no-underscore-here", AuthDecision.MalformedPartitionKey }; + yield return new object[] { TenantId, "_missing-tenant", AuthDecision.MalformedPartitionKey }; + yield return new object[] { TenantId, "too_many_parts_here", AuthDecision.MalformedPartitionKey }; + } + + [Theory] + [MemberData(nameof(PartitionKeyAuthCases))] + public void TenantAuth_reproduces_Run_decision(string header, string partitionKey, AuthDecision expected) + { + // Mirrors Run L169-180 using the real production static + the exact OrdinalIgnoreCase compare. + var parsed = IngestTelemetryFunction.TryParsePartitionKey(partitionKey, out var bodyTenant, out _); + + AuthDecision actual; + if (!parsed) + { + actual = AuthDecision.MalformedPartitionKey; // Run → 400 "Malformed PartitionKey" + } + else if (!string.Equals(bodyTenant, header, StringComparison.OrdinalIgnoreCase)) + { + actual = AuthDecision.TenantMismatch; // Run → 403 "TenantId mismatch..." + } + else + { + actual = AuthDecision.Accept; + } + + Assert.Equal(expected, actual); + } + + // ============================================================ Session deletion locked → 410 + + [Theory] + [InlineData(SessionDeletionState.Preparing)] + [InlineData(SessionDeletionState.Queued)] + [InlineData(SessionDeletionState.Running)] + [InlineData(SessionDeletionState.Poisoned)] + public async Task DeletionLocked_guard_throws_the_exception_Run_maps_to_410(string lockState) + { + // Run L205-213: EnsureWritableAndGetRowAsync throws SessionDeletionLockedException for an + // in-flight cascade → Run responds 410 Gone via WriteSessionLockedAsync. + var guard = NewGuard(out var reader); + var row = new TableEntity(TenantId, SessionId) + { + ["DeletionState"] = lockState, + ["PendingDeletionManifestId"] = "MANIFEST-410", + }; + reader.Setup(r => r.GetSessionRowAsync(TenantId, SessionId, It.IsAny())) + .ReturnsAsync(row); + + var ex = await Assert.ThrowsAsync( + () => guard.EnsureWritableAndGetRowAsync(TenantId, SessionId, callerContext: "V2.IngestTelemetry")); + + Assert.Equal(lockState, ex.CurrentState); + Assert.Equal("MANIFEST-410", ex.ManifestId); + } + + [Fact] + public async Task DeletionUnlocked_guard_returns_row_that_feeds_the_status_prefetch() + { + // Run L205-217: on an unlocked session the guard hands back the loaded row, and Run reads + // Status off it via TryReadSessionStatus to seed the stall-heal prefetch (no second read). + var guard = NewGuard(out var reader); + var row = new TableEntity(TenantId, SessionId) + { + ["DeletionState"] = SessionDeletionState.None, + ["Status"] = "Succeeded", // Sessions stores Status as string (status.ToString()) + }; + reader.Setup(r => r.GetSessionRowAsync(TenantId, SessionId, It.IsAny())) + .ReturnsAsync(row); + + var returned = await guard.EnsureWritableAndGetRowAsync(TenantId, SessionId, callerContext: "V2.IngestTelemetry"); + + Assert.Same(row, returned); + Assert.Equal(SessionStatus.Succeeded, IngestTelemetryFunction.TryReadSessionStatus(returned)); + } + + // ============================================================ TryReadSessionStatus (unique helper) + + [Theory] + [InlineData("InProgress", SessionStatus.InProgress)] + [InlineData("Succeeded", SessionStatus.Succeeded)] + [InlineData("failed", SessionStatus.Failed)] // case-insensitive parse + [InlineData("Stalled", SessionStatus.Stalled)] + public void TryReadSessionStatus_parses_string_status_case_insensitively(string stored, SessionStatus expected) + { + var row = new TableEntity(TenantId, SessionId) { ["Status"] = stored }; + Assert.Equal(expected, IngestTelemetryFunction.TryReadSessionStatus(row)); + } + + [Fact] + public void TryReadSessionStatus_returns_null_for_null_row() + { + Assert.Null(IngestTelemetryFunction.TryReadSessionStatus(null)); + } + + [Fact] + public void TryReadSessionStatus_returns_null_when_status_column_absent() + { + var row = new TableEntity(TenantId, SessionId); // no Status column + Assert.Null(IngestTelemetryFunction.TryReadSessionStatus(row)); + } + + [Fact] + public void TryReadSessionStatus_returns_null_for_unparseable_status() + { + var row = new TableEntity(TenantId, SessionId) { ["Status"] = "NotARealStatus" }; + Assert.Null(IngestTelemetryFunction.TryReadSessionStatus(row)); + } + + // ============================================================ Harness (mirrors SessionDeletionGuardTests) + + private static SessionDeletionGuard NewGuard(out Mock reader) + { + reader = new Mock(); + return new SessionDeletionGuard(reader.Object, NullLogger.Instance); + } +} diff --git a/src/Backend/AutopilotMonitor.Functions.Tests/IngestTelemetryMalformedTests.cs b/src/Backend/AutopilotMonitor.Functions.Tests/IngestTelemetryMalformedTests.cs new file mode 100644 index 00000000..e639fa5e --- /dev/null +++ b/src/Backend/AutopilotMonitor.Functions.Tests/IngestTelemetryMalformedTests.cs @@ -0,0 +1,151 @@ +using System.IO; +using System.Text; +using AutopilotMonitor.Functions.Functions.Ingest; +using AutopilotMonitor.Shared.Models; + +namespace AutopilotMonitor.Functions.Tests; + +/// +/// Adversarial / malformed-body coverage for the ingest parse entrypoint. These exercise the same +/// seam the existing malformed-JSON test uses — +/// — because Run() feeds the raw (already gzip-decompressed) request stream straight into it +/// (Run L142). Anything this helper throws as a JsonException is caught at Run L144-148 and +/// mapped to a controlled 400 "Malformed JSON body"; nothing escapes as an unhandled crash. +/// +/// Body construction mirrors verbatim +/// (MemoryStream over raw bytes → ReadBodyWithSizeCapAsync(stream, maxBytes)). +/// +public class IngestTelemetryMalformedTests +{ + private const string TenantId = "a1b2c3d4-e5f6-7890-abcd-ef1234567890"; + private const string SessionId = "b2c3d4e5-f6a7-8901-bcde-f12345678901"; + + // ============================================================ Deeply-nested JSON (JSON-bomb) + + [Theory] + [InlineData("array")] + [InlineData("object")] + public async Task DeeplyNestedJson_is_a_controlled_JsonException_not_a_crash(string shape) + { + // 6000 levels of nesting — a JSON-bomb-style payload. Newtonsoft 13.x enforces a default + // reader MaxDepth of 64, so the parse fails fast with a JsonReaderException (a JsonException + // subclass) LONG before the nesting can exhaust the stack. Run() catches this at L144 and + // returns a controlled 400 "Malformed JSON body". CURRENT BEHAVIOR — this is safe by design + // (bounded depth), NOT a hardening gap. + const int depth = 6000; + var json = shape == "array" + ? new string('[', depth) + new string(']', depth) + : "[" + string.Concat(Enumerable.Repeat("{\"a\":", depth)) + "1" + new string('}', depth) + "]"; + + var payload = Encoding.UTF8.GetBytes(json); + using var stream = new MemoryStream(payload); + + await Assert.ThrowsAnyAsync( + () => IngestTelemetryFunction.ReadBodyWithSizeCapAsync(stream, maxBytes: 10 * 1024 * 1024)); + } + + // ============================================================ Non-UTF8 / invalid bytes + + [Fact] + public async Task InvalidUtf8Bytes_decode_to_replacement_chars_and_surface_as_JsonException() + { + // Lone UTF-8 continuation bytes (0x80-0xBF) are not valid UTF-8 and are not a byte-order + // mark. The StreamReader's default replacement fallback turns them into U+FFFD (it never + // throws on decode), and the resulting text is not valid JSON → controlled JsonException, + // no crash. CURRENT BEHAVIOR. + var payload = new byte[] { 0x80, 0x81, 0x82, 0x83, 0x84, 0x85 }; + using var stream = new MemoryStream(payload); + + await Assert.ThrowsAnyAsync( + () => IngestTelemetryFunction.ReadBodyWithSizeCapAsync(stream, maxBytes: 1024)); + } + + [Fact] + public async Task Utf8BomPrefixedValidJson_is_stripped_and_parsed() + { + // A legitimate UTF-8 BOM (EF BB BF) in front of a valid empty batch: the StreamReader + // detects and strips the BOM, so the parse succeeds and yields an empty list (which Run + // then rejects downstream at L160 with 400 "No telemetry items provided"). Proves the UTF8 + // decode path handles a real-world byte-order mark without corruption. + var payload = new byte[] { 0xEF, 0xBB, 0xBF } + .Concat(Encoding.UTF8.GetBytes("[]")) + .ToArray(); + using var stream = new MemoryStream(payload); + + var (exceeded, items) = await IngestTelemetryFunction.ReadBodyWithSizeCapAsync(stream, maxBytes: 1024); + + Assert.False(exceeded); + Assert.NotNull(items); + Assert.Empty(items!); + } + + [Fact] + public async Task MultiByteUtf8_string_values_round_trip_through_the_parser() + { + // Contrast to the invalid-byte case: well-formed multi-byte UTF-8 (accents + emoji) inside a + // string value must decode losslessly, confirming the StreamReader(UTF-8) path is correct + // for legitimate non-ASCII payloads. + var json = "[{\"Kind\":\"Signal\",\"PayloadJson\":\"café-\\u00e9-🚀\"}]"; + var payload = Encoding.UTF8.GetBytes(json); + using var stream = new MemoryStream(payload); + + var (exceeded, items) = await IngestTelemetryFunction.ReadBodyWithSizeCapAsync(stream, maxBytes: 1024); + + Assert.False(exceeded); + var item = Assert.Single(items!); + Assert.Equal("Signal", item.Kind); + Assert.Equal("café-é-🚀", item.PayloadJson); + } + + // ============================================================ Content-Type is not inspected + + [Fact] + public async Task WrongContentType_ValidJsonBytes_still_parse_because_seam_reads_raw_stream() + { + // ReadBodyWithSizeCapAsync (and Run() around it) never inspects Content-Type — it reads the + // raw decompressed byte stream. So a body sent with, e.g., text/plain but carrying valid + // JSON bytes parses exactly as an application/json body would. CURRENT BEHAVIOR: parse is + // content-type-agnostic. + var payload = Encoding.UTF8.GetBytes("[{\"Kind\":\"Event\"}]"); + using var stream = new MemoryStream(payload); + + var (exceeded, items) = await IngestTelemetryFunction.ReadBodyWithSizeCapAsync(stream, maxBytes: 1024); + + Assert.False(exceeded); + var item = Assert.Single(items!); + Assert.Equal("Event", item.Kind); + } + + // ============================================================ Adversarial-but-valid shapes + + [Theory] + [InlineData("[]")] // empty array → empty list (Run → 400 "No telemetry items") + [InlineData("[] \n\t ")] // trailing whitespace is ignored by the parser + public async Task EmptyArray_yields_empty_list_not_null(string json) + { + var payload = Encoding.UTF8.GetBytes(json); + using var stream = new MemoryStream(payload); + + var (exceeded, items) = await IngestTelemetryFunction.ReadBodyWithSizeCapAsync(stream, maxBytes: 1024); + + Assert.False(exceeded); + Assert.NotNull(items); + Assert.Empty(items!); + } + + [Theory] + [InlineData("{\"Kind\":\"Event\"}")] // top-level object, not an array + [InlineData("\"just-a-string\"")] // top-level primitive + [InlineData("[{\"Kind\":\"Event\"},")] // truncated array + [InlineData("[{\"Kind\":}]")] // missing value + public async Task StructurallyBrokenOrWrongRootJson_throws_JsonException(string json) + { + // Each is caught at Run L144 and mapped to 400 "Malformed JSON body". A top-level object or + // primitive cannot bind to List and surfaces as a JsonException too. + var payload = Encoding.UTF8.GetBytes(json); + using var stream = new MemoryStream(payload); + + await Assert.ThrowsAnyAsync( + () => IngestTelemetryFunction.ReadBodyWithSizeCapAsync(stream, maxBytes: 1024)); + } +} diff --git a/src/McpServer/autopilot-monitor-mcp/src/__tests__/tools-raw-adversarial.test.ts b/src/McpServer/autopilot-monitor-mcp/src/__tests__/tools-raw-adversarial.test.ts new file mode 100644 index 00000000..ba9d3e86 --- /dev/null +++ b/src/McpServer/autopilot-monitor-mcp/src/__tests__/tools-raw-adversarial.test.ts @@ -0,0 +1,204 @@ +/** + * Adversarial UNIT tests for the Global-Admin raw tools (query_table, query_backend_logs). + * + * The existing tools-raw.test.ts is a live-backend integration suite gated behind + * `describe.skipIf(!AUTOPILOT_API_TOKEN)` — so in CI (no token) it goes green without + * exercising anything. These tests fill that gap at the unit level: fetch is stubbed, so + * they ALWAYS run in CI and prove the request the tool *builds* is safe: + * + * - `tableName` is encodeURIComponent'd → path-traversal is contained to one path segment. + * - `continuation` cross-tool path substitution is rejected by followNextLink's base-path guard. + * - operator-supplied OData `filter` / KQL `query` are forwarded VERBATIM (documenting the + * trust boundary: sanitization + RBAC live on the backend, GlobalAdmin-only). These tests + * pin that forwarding so a regression that silently mangles or drops the payload is caught, + * and so the "no client-side length cap" behavior is explicit rather than accidental. + * + * Harness mirrors tools-delegated.test.ts: build a server for a strict-GA caller, pull the raw + * registered handler, stub global fetch, invoke inside runWithCaller, inspect url + body. + */ +import { describe, it, expect, vi, afterEach } from 'vitest'; +import { McpServer } from '@modelcontextprotocol/sdk/server/mcp.js'; +import { registerTools } from '../tools.js'; +import { runWithCaller } from '../client.js'; + +type ToolHandler = (args: Record, extra: unknown) => Promise<{ + content?: Array<{ type: string; text?: string }>; + isError?: boolean; +}>; + +/** Build a strict-GA server and return a tool's raw handler by name. */ +function gaHandler(name: string): ToolHandler { + const server = new McpServer({ name: 'test', version: '0.0.0' }); + registerTools(server, undefined, undefined, /*ga*/ true, /*strictGa*/ true, /*delegated*/ false); + const registry = (server as unknown as { _registeredTools: Record })._registeredTools; + const tool = registry[name]; + if (!tool) throw new Error(`tool ${name} not registered for strict-GA caller`); + return tool.handler; +} + +interface Captured { url: string; method: string; body: string | undefined; } + +/** Stub global fetch; capture url + method + body, return an empty-OK JSON body. */ +function stubFetchCapture(): { calls: Captured[] } { + const calls: Captured[] = []; + const fn = vi.fn(async (url: string, init?: RequestInit) => { + calls.push({ + url: String(url), + method: (init?.method ?? 'GET').toUpperCase(), + body: typeof init?.body === 'string' ? init.body : undefined, + }); + return { + ok: true, + status: 200, + json: async () => ({ table: 'X', count: 0, entities: [], tables: [], nextLink: null }), + text: async () => '{}', + } as unknown as Response; + }); + vi.stubGlobal('fetch', fn); + return { calls }; +} + +function resultText(r: { content?: Array<{ text?: string }> }): string { + return (r.content ?? []).map((c) => c.text ?? '').join('\n'); +} + +const GA = { token: 'ga-token', isGlobalAdmin: true } as const; +const extra = { signal: new AbortController().signal }; + +afterEach(() => vi.unstubAllGlobals()); + +// ── query_table: table-name path-traversal containment ────────────────────── +describe('query_table — tableName path-traversal is contained by encodeURIComponent', () => { + const traversals: Array<[label: string, tableName: string]> = [ + ['parent-dir slashes', 'Sessions/../../admin/secrets'], + ['leading slash', '/etc/passwd'], + ['encoded slash', 'Sessions..%2F..%2Fadmin'], + ['backslash', 'Sessions\\..\\admin'], + ['dot segments only', '../../../'], + ]; + + it.each(traversals)('%s stays within the /raw/tables/ segment', async (_label, tableName) => { + const handler = gaHandler('query_table'); + const { calls } = stubFetchCapture(); + + await runWithCaller(GA, () => handler({ tableName }, extra)); + + expect(calls).toHaveLength(1); + const url = calls[0].url; + // The request targets the tables endpoint... + expect(url).toContain('/api/global/raw/tables/'); + // ...and the table name is a single, fully-encoded path segment: no raw '/' + // (and no raw '\\') survives between the base and the query string, so the + // caller cannot pivot to a sibling/parent path like /admin/secrets. + const afterBase = url.split('/api/global/raw/tables/')[1] ?? ''; + const segment = afterBase.split('?')[0]; + expect(segment).not.toContain('/'); + expect(segment).not.toContain('\\'); + }); +}); + +// ── query_table: continuation cross-tool path substitution guard ──────────── +describe('query_table — crafted continuation cannot bend the request to another endpoint', () => { + const foreign: Array<[label: string, continuation: string]> = [ + ['other tool path', '/api/global/raw/logs?x=1'], + ['admin path', '/api/admin/secrets'], + ['different table id', '/api/global/raw/tables/OtherTable?pageSize=1'], + ]; + + it.each(foreign)('rejects continuation pointing at %s', async (_label, continuation) => { + const handler = gaHandler('query_table'); + const { calls } = stubFetchCapture(); + + const res = await runWithCaller(GA, () => handler({ tableName: 'Sessions', continuation }, extra)); + + // followNextLink throws on base-path mismatch; toolError turns it into an isError result + // WITHOUT ever issuing the substituted request. + expect(calls).toHaveLength(0); + expect(res.isError).toBe(true); + expect(resultText(res)).toMatch(/does not match|base path/i); + }); + + it('accepts a continuation that matches its OWN base path', async () => { + const handler = gaHandler('query_table'); + const { calls } = stubFetchCapture(); + const ownLink = '/api/global/raw/tables/Sessions?pageSize=200&continuation=abc'; + + await runWithCaller(GA, () => handler({ tableName: 'Sessions', continuation: ownLink }, extra)); + + expect(calls).toHaveLength(1); + expect(calls[0].url).toContain(ownLink); + }); +}); + +// ── query_table: OData filter is forwarded verbatim (trust boundary) ───────── +describe('query_table — OData filter forwarded verbatim and URL-safely encoded', () => { + const filters: Array<[label: string, filter: string]> = [ + ['tenant-scope breakout', "PartitionKey ne '' or true"], + ['quote injection', "Status eq 'x' or 1 eq 1"], + ['url-breaking chars', "Status eq 'a&b=c#d?e'"], + ['unicode', "Name eq 'Ünîçodè-⚡-名前'"], + ]; + + it.each(filters)('%s round-trips through the filter param', async (_label, filter) => { + const handler = gaHandler('query_table'); + const { calls } = stubFetchCapture(); + + await runWithCaller(GA, () => handler({ tableName: 'Sessions', filter }, extra)); + + expect(calls).toHaveLength(1); + const parsed = new URL(calls[0].url, 'https://base.invalid'); + // Encoded on the wire (no structural break), decoded value is exactly what we sent. + expect(parsed.searchParams.get('filter')).toBe(filter); + }); +}); + +// ── query_backend_logs: KQL forwarded verbatim over POST ──────────────────── +describe('query_backend_logs — KQL forwarded verbatim in POST body (backend is the gate)', () => { + const queries: Array<[label: string, query: string]> = [ + ['externaldata exfil attempt', "externaldata(x:string)[@'https://evil/e.csv'] with (format='csv')"], + ['cross-table join', 'traces | join (requests) on operation_Id'], + ['comment/terminator smuggling', "traces | take 1 // ; drop"], + ['unicode + newlines', 'traces\n| where message has "Ünî"\n| take 1'], + ]; + + it.each(queries)('%s reaches /raw/logs unchanged', async (_label, query) => { + const handler = gaHandler('query_backend_logs'); + const { calls } = stubFetchCapture(); + + await runWithCaller(GA, () => handler({ query }, extra)); + + expect(calls).toHaveLength(1); + expect(calls[0].method).toBe('POST'); + expect(calls[0].url).toContain('/api/global/raw/logs'); + const sent = JSON.parse(calls[0].body ?? '{}'); + expect(sent.query).toBe(query); + // (The PT1H timespan default is applied by the zod inputSchema at the MCP framework + // boundary, not inside the raw handler, so it is asserted separately below.) + }); + + // NOTE (hardening gap): there is NO client-side length cap on `query`. This test pins the + // current behavior — an oversized KQL string is forwarded intact — so any future cap is a + // deliberate, tested change rather than a silent one. The real DoS gate is backend RBAC + // (GlobalAdmin-only) + App Insights query limits. + it('forwards an oversized KQL string without client-side truncation', async () => { + const handler = gaHandler('query_backend_logs'); + const { calls } = stubFetchCapture(); + const huge = 'traces | where message contains "' + 'A'.repeat(100_000) + '" | take 1'; + + await runWithCaller(GA, () => handler({ query: huge }, extra)); + + expect(calls).toHaveLength(1); + const sent = JSON.parse(calls[0].body ?? '{}'); + expect(sent.query.length).toBe(huge.length); + }); + + it('honors an explicit timespan', async () => { + const handler = gaHandler('query_backend_logs'); + const { calls } = stubFetchCapture(); + + await runWithCaller(GA, () => handler({ query: 'traces | take 1', timespan: 'P1D' }, extra)); + + const sent = JSON.parse(calls[0].body ?? '{}'); + expect(sent.timespan).toBe('P1D'); + }); +}); diff --git a/src/Web/autopilot-monitor-web/app/go/[code]/__tests__/route.test.ts b/src/Web/autopilot-monitor-web/app/go/[code]/__tests__/route.test.ts new file mode 100644 index 00000000..9d83b6b4 --- /dev/null +++ b/src/Web/autopilot-monitor-web/app/go/[code]/__tests__/route.test.ts @@ -0,0 +1,193 @@ +/** + * Tests for the OOBE bootstrap script route handler: app/go/[code]/route.ts. + * + * This is the web tier's single security-critical server endpoint: it proxies to the + * backend and inlines validated values into a PowerShell script that runs as SYSTEM during + * Windows OOBE. The VALIDATOR (utils/bootstrapValidation) is already well-tested; the HANDLER + * itself (code-format gate, backend-error passthrough, 200-on-error `irm|iex` contract, + * errorScript quote-escaping + length cap, and the "never leak the offending value" rule) was + * not. These tests cover the first line of defense with fetch stubbed. + */ +import { describe, it, expect, vi, beforeEach, afterEach } from "vitest"; +import { GET } from "../route"; + +function makeCtx(code: string) { + return { params: Promise.resolve({ code }) }; +} + +function invoke(code: string) { + return GET(new Request(`https://portal.test/go/${code}`), makeCtx(code)); +} + +function okBackend(body: unknown): Response { + return new Response(JSON.stringify(body), { + status: 200, + headers: { "Content-Type": "application/json" }, + }); +} + +function errBackend(status: number, body: unknown = {}): Response { + return new Response(JSON.stringify(body), { + status, + headers: { "Content-Type": "application/json" }, + }); +} + +/** A bootstrap response that passes validateBootstrapResponse. */ +function validPayload() { + const expiresAt = new Date(Date.now() + 24 * 60 * 60 * 1000).toISOString(); + return { + success: true, + tenantId: "11111111-1111-1111-1111-111111111111", + token: "22222222-2222-2222-2222-222222222222", + agentDownloadUrl: + "https://autopilotmonitor.blob.core.windows.net/agent/agent-v2.zip", + expiresAt, + }; +} + +const fetchMock = vi.fn(); + +beforeEach(() => { + vi.stubGlobal("fetch", fetchMock); + fetchMock.mockReset(); +}); + +afterEach(() => { + vi.unstubAllGlobals(); + vi.restoreAllMocks(); +}); + +// ── Code-format gate: rejected BEFORE any backend call ────────────────────── +describe("code-format validation", () => { + const badCodes: Array<[label: string, code: string]> = [ + ["empty", ""], + ["too short (3)", "abc"], + ["too long (11)", "abcdefghijk"], + ["space", "ab cd"], + ["slash / path traversal", "../etc"], + ["encoded slash", "ab%2Fcd"], + ["ps metachar $", "abc$"], + ["quote", "ab'cd"], + ["dot", "ab.cd"], + ]; + + it.each(badCodes)("rejects %s without hitting the backend", async (_label, code) => { + const res = await invoke(code); + expect(fetchMock).not.toHaveBeenCalled(); + // 200 so `irm | iex` still runs and surfaces the Write-Host error. + expect(res.status).toBe(200); + const body = await res.text(); + expect(body).toContain("ERROR: Invalid bootstrap code format."); + }); + + const goodCodes: Array<[string]> = [["abcd"], ["ABC123"], ["a1b2c3d4e5"]]; + it.each(goodCodes)("accepts well-formed code %s (proceeds to backend)", async (code) => { + fetchMock.mockResolvedValueOnce(errBackend(404, {})); + await invoke(code); + expect(fetchMock).toHaveBeenCalledTimes(1); + expect(String(fetchMock.mock.calls[0][0])).toContain( + `/api/bootstrap/validate/${code}`, + ); + }); +}); + +// ── Backend error passthrough ─────────────────────────────────────────────── +describe("backend validation errors", () => { + it("passes through the backend message", async () => { + fetchMock.mockResolvedValueOnce(errBackend(410, { message: "Code was revoked by admin." })); + const res = await invoke("abcd"); + expect(res.status).toBe(200); + expect(await res.text()).toContain("ERROR: Code was revoked by admin."); + }); + + it("falls back to a default message when the backend body has none", async () => { + fetchMock.mockResolvedValueOnce(errBackend(404, {})); + const res = await invoke("abcd"); + expect(await res.text()).toContain( + "ERROR: Bootstrap code not found, expired, or revoked.", + ); + }); + + it("handles a non-JSON backend error body gracefully", async () => { + fetchMock.mockResolvedValueOnce( + new Response("502", { status: 502 }), + ); + const res = await invoke("abcd"); + expect(res.status).toBe(200); + expect(await res.text()).toContain("ERROR:"); + }); + + it("returns a network-error script when fetch throws", async () => { + fetchMock.mockRejectedValueOnce(new Error("ECONNREFUSED")); + const res = await invoke("abcd"); + expect(res.status).toBe(200); + const body = await res.text(); + expect(body).toContain("Failed to validate bootstrap code"); + // The raw exception text must never be inlined into the script. + expect(body).not.toContain("ECONNREFUSED"); + }); +}); + +// ── errorScript hardening: quote-escaping + length cap ─────────────────────── +describe("errorScript hardening", () => { + it("escapes single quotes for the PowerShell string literal", async () => { + fetchMock.mockResolvedValueOnce(errBackend(400, { message: "it's a 'bad' code" })); + const body = await (await invoke("abcd")).text(); + // Single quotes doubled; the surrounding literal stays intact. + expect(body).toContain("it''s a ''bad'' code"); + }); + + it("caps an oversized backend message (raw length, before escaping)", async () => { + const huge = "X".repeat(300); + fetchMock.mockResolvedValueOnce(errBackend(400, { message: huge })); + const body = await (await invoke("abcd")).text(); + expect(body).toContain("X".repeat(200) + "..."); + expect(body).not.toContain("X".repeat(201)); + }); +}); + +// ── Success path ──────────────────────────────────────────────────────────── +describe("valid bootstrap → generated OOBE script", () => { + it("emits the PS script with the validated values and hardened headers", async () => { + const payload = validPayload(); + fetchMock.mockResolvedValueOnce(okBackend(payload)); + + const res = await invoke("abcd"); + + expect(res.status).toBe(200); + expect(res.headers.get("Content-Type")).toBe("text/plain; charset=utf-8"); + expect(res.headers.get("X-Content-Type-Options")).toBe("nosniff"); + expect(res.headers.get("Cache-Control")).toContain("no-store"); + + const body = await res.text(); + expect(body).toContain("#Requires -RunAsAdministrator"); + expect(body).toContain(`--tenant-id '${payload.tenantId}'`); + expect(body).toContain(`--bootstrap-token '${payload.token}'`); + expect(body).toContain(`$AgentDownloadUrl = '${payload.agentDownloadUrl}'`); + }); + + it("rejects a backend payload that fails validation WITHOUT leaking the offending value", async () => { + // Valid shape/success, but hostile agentDownloadUrl (foreign host + PS injection). + const hostile = { + success: true, + tenantId: "11111111-1111-1111-1111-111111111111", + token: "22222222-2222-2222-2222-222222222222", + agentDownloadUrl: "https://evil.example.com/x.zip'; $(calc); '", + expiresAt: new Date(Date.now() + 3600_000).toISOString(), + }; + const warn = vi.spyOn(console, "warn").mockImplementation(() => {}); + fetchMock.mockResolvedValueOnce(okBackend(hostile)); + + const res = await invoke("abcd"); + const body = await res.text(); + + expect(res.status).toBe(200); + expect(body).toContain("ERROR: Bootstrap response failed validation. Contact support."); + // The generated script must not be produced, and the payload must not appear anywhere. + expect(body).not.toContain("#Requires -RunAsAdministrator"); + expect(body).not.toContain("evil.example.com"); + expect(body).not.toContain("calc"); + warn.mockRestore(); + }); +});