diff --git a/src/Backend/AutopilotMonitor.Functions.Tests/TableKeySanitizerTests.cs b/src/Backend/AutopilotMonitor.Functions.Tests/TableKeySanitizerTests.cs new file mode 100644 index 00000000..194579fd --- /dev/null +++ b/src/Backend/AutopilotMonitor.Functions.Tests/TableKeySanitizerTests.cs @@ -0,0 +1,82 @@ +using System.Text.RegularExpressions; +using AutopilotMonitor.Functions.Helpers; + +namespace AutopilotMonitor.Functions.Tests; + +public class TableKeySanitizerTests +{ + [Fact] + public void Sanitize_RemovesControlCharacters() + { + // C0 and C1 controls: Table Storage rejects them in keys, and a point read carries the key + // in its URL, where the HTTP layer refuses them with "400 Invalid URL". + var input = $"a{(char)0}b\tc\r\nd{(char)0x7F}e{(char)0x85}f"; + Assert.Equal("abcdef", TableKeySanitizer.Sanitize(input)); + } + + [Fact] + public void Sanitize_ReplacesTheCharactersTableStorageRejectsInKeys() + { + Assert.Equal("a_b_c_d_e", TableKeySanitizer.Sanitize("a/b\\c#d?e")); + } + + [Fact] + public void Sanitize_NulPaddedInventoryTriple_YieldsTheCleanKey() + { + var padded = "contoso\0\0\0\0\0:widget\0\0\0\0\0:1.0"; + Assert.Equal("contoso:widget:1.0", TableKeySanitizer.Sanitize(padded)); + } + + [Theory] + [InlineData("contoso:widget:1.0")] + [InlineData(":microsoft edge update:1.3.265.7")] + [InlineData("müller gmbh:übersicht:2.3")] + [InlineData("cve_cpe:2.3:a:contoso:widget:*:*:*:*:*:*:*:*")] + public void Sanitize_KeyWithoutControlOrIllegalCharacters_IsUnchanged(string key) + { + // Existing rows must stay reachable: the rule only changes keys that could never be read. + Assert.Equal(key, TableKeySanitizer.Sanitize(key)); + } + + [Fact] + public void Sanitize_IsIdempotent() + { + var once = TableKeySanitizer.Sanitize("a/b\0#c?\td"); + Assert.Equal(once, TableKeySanitizer.Sanitize(once)); + } + + [Fact] + public void RemoveControlCharacters_KeepsKeyIllegalCharacters() + { + Assert.Equal("a/b#c", TableKeySanitizer.RemoveControlCharacters("a/b\0#\u0001c")); + } + + [Fact] + public void No_key_builder_keeps_its_own_copy_of_the_rule() + { + // The rule existed ten times, and only one copy removed control characters. A new inline + // copy would map the same text to a different key than every other writer of that table. + var functionsDir = Path.Combine(RepoRoot(), "src", "Backend", "AutopilotMonitor.Functions"); + var inlineCopy = new Regex(@"Replace\(\s*[""']#[""']\s*,\s*[""']_[""']\s*\)|static\s+string\s+SanitizeTableKey\s*\("); + var offenders = Directory + .EnumerateFiles(functionsDir, "*.cs", SearchOption.AllDirectories) + .Where(f => !f.Contains($"{Path.DirectorySeparatorChar}bin{Path.DirectorySeparatorChar}") + && !f.Contains($"{Path.DirectorySeparatorChar}obj{Path.DirectorySeparatorChar}")) + .Where(f => inlineCopy.IsMatch(File.ReadAllText(f))) + .Select(f => Path.GetRelativePath(functionsDir, f)) + .ToList(); + + Assert.True(offenders.Count == 0, + "Build table keys from free text through TableKeySanitizer.Sanitize, not a local copy: " + + string.Join(", ", offenders)); + } + + private static string RepoRoot() + { + var dir = new DirectoryInfo(AppContext.BaseDirectory); + while (dir != null && !File.Exists(Path.Combine(dir.FullName, "AutopilotMonitor.sln"))) + dir = dir.Parent; + Assert.NotNull(dir); + return dir!.FullName; + } +} diff --git a/src/Backend/AutopilotMonitor.Functions.Tests/TableStorageInventoryCasTests.cs b/src/Backend/AutopilotMonitor.Functions.Tests/TableStorageInventoryCasTests.cs index 8ec020d3..ac8a4972 100644 --- a/src/Backend/AutopilotMonitor.Functions.Tests/TableStorageInventoryCasTests.cs +++ b/src/Backend/AutopilotMonitor.Functions.Tests/TableStorageInventoryCasTests.cs @@ -70,6 +70,36 @@ public class TableStorageInventoryCasTests harness.SoftwareInventory.Verify(t => t.UpdateEntityAsync(It.IsAny(), It.IsAny(), It.IsAny(), It.IsAny()), Times.Never); } + [Fact] + public async Task Increment_with_nul_padded_triple_reads_and_writes_the_clean_key() + { + // Installers that pad REG_SZ values with NULs used to put "%00" into the point-read URL, + // which the HTTP layer rejects ("400 Invalid URL") before Table Storage sees it. + var harness = new Harness(); + string? readRowKey = null; + harness.SoftwareInventory.Setup(t => + t.GetEntityAsync(TenantId, It.IsAny(), It.IsAny>(), It.IsAny())) + .Callback, CancellationToken>((_, rk, _, _) => readRowKey = rk) + .ThrowsAsync(new RequestFailedException(404, "ResourceNotFound")); + + TableEntity? addedEntity = null; + harness.SoftwareInventory.Setup(t => t.AddEntityAsync(It.IsAny(), It.IsAny())) + .Returns((e, _) => + { + addedEntity = e; + return Task.FromResult(new Mock().Object); + }); + + var item = new SoftwareInventoryItem + { + NormalizedVendor = "contoso\0\0\0", NormalizedName = "widget\0\0", NormalizedVersion = "1.0", + }; + await harness.Sut.IncrementSoftwareInventoryEntryAsync(TenantId, item, SessionId); + + Assert.Equal("contoso:widget:1.0", readRowKey); + Assert.Equal("contoso:widget:1.0", addedEntity!.RowKey); + } + [Fact] public async Task Increment_does_etag_cas_plus_one_when_row_exists() { diff --git a/src/Backend/AutopilotMonitor.Functions.Tests/VulnerabilityCorrelationServiceContributionsTests.cs b/src/Backend/AutopilotMonitor.Functions.Tests/VulnerabilityCorrelationServiceContributionsTests.cs index 3e05ff0d..777b44eb 100644 --- a/src/Backend/AutopilotMonitor.Functions.Tests/VulnerabilityCorrelationServiceContributionsTests.cs +++ b/src/Backend/AutopilotMonitor.Functions.Tests/VulnerabilityCorrelationServiceContributionsTests.cs @@ -1,10 +1,14 @@ using System; using System.Collections.Generic; using System.Threading; +using System.Text.Json; using System.Threading.Tasks; +using Azure; +using AutopilotMonitor.Functions.Services; using AutopilotMonitor.Functions.Services.Vulnerability; using AutopilotMonitor.Shared.DataAccess; using AutopilotMonitor.Shared.Models.Deletion; +using Microsoft.Extensions.Caching.Memory; using Microsoft.Extensions.Logging.Abstractions; using Moq; using Xunit; @@ -13,10 +17,9 @@ namespace AutopilotMonitor.Functions.Tests; /// /// Behaviour tests for the rewritten inventory-write path -/// (). The -/// background Task.Run in CorrelateAsync just forwards to this method, so testing -/// it directly is equivalent to testing the rewritten flow end-to-end without the timing -/// uncertainty of a fire-and-forget Task. +/// (). +/// CorrelateAsync awaits this method, so testing it directly covers the write flow +/// without the NVD side of the correlation. /// public class VulnerabilityCorrelationServiceContributionsTests { @@ -158,18 +161,19 @@ public class VulnerabilityCorrelationServiceContributionsTests } [Fact] - public async Task ApplyInventoryContributionsAsync_swallows_side_row_helper_exceptions_for_non_fatal_logging() + public async Task ApplyInventoryContributionsAsync_swallows_side_row_helper_exceptions_and_reports_one_ops_event() { // Helpers are fail-loud (per memory feedback_storage_helpers_fail_soft) but the caller - // intentionally logs+swallows the side-row CAS exhaustion so the primary VulnerabilityReport - // stays stored. Drift is the documented trade-off (§17.3 "Side-row write before counter writes"). + // logs+swallows the side-row CAS exhaustion so the primary VulnerabilityReport stays + // stored. The operator still learns about it: one ops event, flagged sideRowFailed. var vulnRepo = new Mock(); vulnRepo .Setup(r => r.UpsertSessionInventoryContributionsAsync( TenantId, SessionId, It.IsAny>(), It.IsAny())) .ThrowsAsync(new InvalidOperationException("SessionInventoryContributions ETag-CAS exhausted ...")); - var service = NewService(vulnRepo); + var (opsEvents, saved) = OpsRig(); + var service = NewService(vulnRepo, opsEvents); var raw = new List> { new Dictionary { ["normalizedPublisher"] = "v1", ["normalizedName"] = "n1", ["normalizedVersion"] = "1.0" }, @@ -181,15 +185,19 @@ public class VulnerabilityCorrelationServiceContributionsTests vulnRepo.Verify(r => r.IncrementSoftwareInventoryEntryAsync( It.IsAny(), It.IsAny(), It.IsAny(), It.IsAny()), Times.Never); + var evt = Assert.Single(saved); + Assert.Equal(OpsEventTypes.SoftwareInventoryCounterWriteFailed, evt.EventType); + using var details = JsonDocument.Parse(evt.Details!); + Assert.True(details.RootElement.GetProperty("sideRowFailed").GetBoolean()); } [Fact] - public async Task ApplyInventoryContributionsAsync_continues_after_per_counter_failure_and_audits_each() + public async Task ApplyInventoryContributionsAsync_continues_after_per_counter_failure_and_reports_one_ops_event_per_run() { // Codex Round 2: a single Increment failure must NOT abort the rest of the loop — // the side-row already promises the new key set, so subsequent runs would see an empty - // delta and never retry the under-counted keys. Per Plan §17.3 + §17.8, each failure - // emits an inventory_counter_write_failed audit + we keep processing the rest. + // delta and never retry the under-counted keys (Plan §17.3). The failures of the run are + // reported as ONE ops event — never per key, never in the tenant audit trail. var vulnRepo = new Mock(); vulnRepo .Setup(r => r.UpsertSessionInventoryContributionsAsync( @@ -209,19 +217,18 @@ public class VulnerabilityCorrelationServiceContributionsTests }, }); - // Increment of (v1,n1,1.0) succeeds, (v2,n2,2.0) THROWS, (v3,n3,3.0) succeeds. + // Increments of (v2,n2) and (v3,n3) THROW, (v1,n1) succeeds; the decrement throws too. vulnRepo.Setup(r => r.IncrementSoftwareInventoryEntryAsync( - TenantId, It.Is(i => i.NormalizedName == "n2"), + TenantId, It.Is(i => i.NormalizedName == "n2" || i.NormalizedName == "n3"), SessionId, It.IsAny())) - .ThrowsAsync(new InvalidOperationException("SoftwareInventory increment ETag-CAS exhausted")); + .ThrowsAsync(new RequestFailedException(400, + "Service request failed.\nStatus: 400 (Bad Request)\n\nContent:\nBad Request - Invalid URL")); + vulnRepo.Setup(r => r.DecrementSoftwareInventoryEntryAsync( + TenantId, "vOld", "nOld", "0.9", It.IsAny())) + .ThrowsAsync(new InvalidOperationException("Decrement ETag-CAS exhausted")); - var maintenanceRepo = new Mock(); - maintenanceRepo.Setup(m => m.LogAuditEntryAsync( - It.IsAny(), It.IsAny(), It.IsAny(), - It.IsAny(), It.IsAny(), It.IsAny?>())) - .ReturnsAsync(true); - - var service = NewService(vulnRepo, maintenanceRepo); + var (opsEvents, saved) = OpsRig(); + var service = NewService(vulnRepo, opsEvents); var raw = new List> { new Dictionary { ["normalizedPublisher"] = "v1", ["normalizedName"] = "n1", ["normalizedVersion"] = "1.0" }, @@ -232,38 +239,33 @@ public class VulnerabilityCorrelationServiceContributionsTests // Should NOT throw. await service.ApplyInventoryContributionsAsync(TenantId, SessionId, raw, cpeMappings: null); - // All three increments attempted. + // Every counter write attempted. vulnRepo.Verify(r => r.IncrementSoftwareInventoryEntryAsync( - TenantId, It.Is(i => i.NormalizedName == "n1"), - SessionId, It.IsAny()), Times.Once); - vulnRepo.Verify(r => r.IncrementSoftwareInventoryEntryAsync( - TenantId, It.Is(i => i.NormalizedName == "n2"), - SessionId, It.IsAny()), Times.Once); - vulnRepo.Verify(r => r.IncrementSoftwareInventoryEntryAsync( - TenantId, It.Is(i => i.NormalizedName == "n3"), - SessionId, It.IsAny()), Times.Once); - // Decrement still runs after the loop continues past the failed Increment. + TenantId, It.IsAny(), SessionId, It.IsAny()), Times.Exactly(3)); vulnRepo.Verify(r => r.DecrementSoftwareInventoryEntryAsync( TenantId, "vOld", "nOld", "0.9", It.IsAny()), Times.Once); - // Audit emitted exactly once for the failed (v2,n2,2.0) Increment. - maintenanceRepo.Verify(m => m.LogAuditEntryAsync( - TenantId, - "inventory_counter_write_failed", - "Session", - SessionId, - "system", - It.Is?>(d => - d != null - && d["operation"] == "increment" - && d["vendor"] == "v2" - && d["name"] == "n2" - && d["version"] == "2.0")), - Times.Once); + var evt = Assert.Single(saved); + Assert.Equal(OpsEventTypes.SoftwareInventoryCounterWriteFailed, evt.EventType); + Assert.Equal(OpsEventCategory.Maintenance, evt.Category); + Assert.Equal(OpsEventSeverity.Warning, evt.Severity); + Assert.Equal(TenantId, evt.TenantId); + using var details = JsonDocument.Parse(evt.Details!); + var root = details.RootElement; + Assert.Equal(SessionId, root.GetProperty("sessionId").GetString()); + Assert.Equal(2, root.GetProperty("failedIncrements").GetInt32()); + Assert.Equal(1, root.GetProperty("failedDecrements").GetInt32()); + Assert.False(root.GetProperty("sideRowFailed").GetBoolean()); + Assert.Equal( + new[] { "v2:n2:2.0", "v3:n3:3.0", "vOld:nOld:0.9" }, + root.GetProperty("sampleKeys").EnumerateArray().Select(e => e.GetString()).ToArray()); + // The storage failure is described by status only — never the raw response body. + Assert.Equal("RequestFailedException 400", root.GetProperty("error").GetString()); + Assert.DoesNotContain("HTML", evt.Message, StringComparison.OrdinalIgnoreCase); } [Fact] - public async Task ApplyInventoryContributionsAsync_audits_decrement_failures_too() + public async Task ApplyInventoryContributionsAsync_writes_no_ops_event_when_every_counter_write_succeeds() { var vulnRepo = new Mock(); vulnRepo @@ -271,24 +273,15 @@ public class VulnerabilityCorrelationServiceContributionsTests TenantId, SessionId, It.IsAny>(), It.IsAny())) .ReturnsAsync(new InventoryContributionsDelta { - AddedItems = new List(), - RemovedKeys = new List + AddedItems = new List { - new DeletionDecrementKey { Vendor = "vDead", Name = "nDead", Version = "0.5" }, + new SoftwareInventoryItem { NormalizedVendor = "v1", NormalizedName = "n1", NormalizedVersion = "1.0" }, }, + RemovedKeys = new List(), }); - vulnRepo.Setup(r => r.DecrementSoftwareInventoryEntryAsync( - TenantId, "vDead", "nDead", "0.5", It.IsAny())) - .ThrowsAsync(new InvalidOperationException("Decrement ETag-CAS exhausted")); - - var maintenanceRepo = new Mock(); - maintenanceRepo.Setup(m => m.LogAuditEntryAsync( - It.IsAny(), It.IsAny(), It.IsAny(), - It.IsAny(), It.IsAny(), It.IsAny?>())) - .ReturnsAsync(true); - - var service = NewService(vulnRepo, maintenanceRepo); + var (opsEvents, saved) = OpsRig(); + var service = NewService(vulnRepo, opsEvents); var raw = new List> { new Dictionary { ["normalizedPublisher"] = "v1", ["normalizedName"] = "n1", ["normalizedVersion"] = "1.0" }, @@ -296,23 +289,34 @@ public class VulnerabilityCorrelationServiceContributionsTests await service.ApplyInventoryContributionsAsync(TenantId, SessionId, raw, cpeMappings: null); - maintenanceRepo.Verify(m => m.LogAuditEntryAsync( - TenantId, - "inventory_counter_write_failed", - "Session", - SessionId, - "system", - It.Is?>(d => - d != null - && d["operation"] == "decrement" - && d["vendor"] == "vDead" - && d["name"] == "nDead" - && d["version"] == "0.5")), - Times.Once); + Assert.Empty(saved); + } + + [Fact] + public void DescribeCounterFailure_keeps_the_first_line_of_a_non_storage_message() + { + Assert.Equal("InvalidOperationException: first line", + VulnerabilityCorrelationService.DescribeCounterFailure(new InvalidOperationException("first line\nsecond line"))); + Assert.Equal("RequestFailedException 412 ConditionNotMet", + VulnerabilityCorrelationService.DescribeCounterFailure(new RequestFailedException(412, "body", "ConditionNotMet", null))); + } + + private static (OpsEventService Service, List Saved) OpsRig() + { + var saved = new List(); + var opsRepo = new Mock(); + opsRepo.Setup(r => r.SaveOpsEventAsync(It.IsAny())) + .Callback(e => { lock (saved) saved.Add(e); }) + .Returns(Task.CompletedTask); + var adminConfig = new Mock( + Mock.Of(), NullLogger.Instance, + new MemoryCache(new MemoryCacheOptions())) { CallBase = false }; + var alertDispatch = TestNotifications.InertOpsAlertDispatch(adminConfig.Object); + return (new OpsEventService(opsRepo.Object, NullLogger.Instance, alertDispatch), saved); } private static VulnerabilityCorrelationService NewService( - Mock vulnRepo, Mock? maintenanceRepo = null) + Mock vulnRepo, OpsEventService? opsEvents = null) { return new VulnerabilityCorrelationService( vulnRepo.Object, @@ -323,6 +327,6 @@ public class VulnerabilityCorrelationServiceContributionsTests adminConfig: null!, telemetryClient: null, logger: NullLogger.Instance, - maintenanceRepo: maintenanceRepo?.Object); + opsEvents: opsEvents); } } diff --git a/src/Backend/AutopilotMonitor.Functions.Tests/VulnerabilityInventoryLoaderTests.cs b/src/Backend/AutopilotMonitor.Functions.Tests/VulnerabilityInventoryLoaderTests.cs index f4d105a1..b77447f3 100644 --- a/src/Backend/AutopilotMonitor.Functions.Tests/VulnerabilityInventoryLoaderTests.cs +++ b/src/Backend/AutopilotMonitor.Functions.Tests/VulnerabilityInventoryLoaderTests.cs @@ -96,6 +96,43 @@ public class VulnerabilityInventoryLoaderTests Assert.Single(result); } + [Fact] + public async Task Loader_strips_control_characters_from_inventory_text() + { + // Installers can pad REG_SZ values with NULs. The vendor/name/version triple becomes a + // table key, so the loader hands out text without control characters; other values pass. + var evt = new EnrollmentEvent + { + TenantId = TenantId, + SessionId = SessionId, + EventType = Constants.EventTypes.SoftwareInventoryAnalysis, + Data = new Dictionary + { + ["triggered_at"] = "shutdown", + ["inventory"] = new List + { + new Dictionary + { + ["displayName"] = "Contoso Widget\0\0", + ["normalizedName"] = "contoso widget\0\0\0\0\0", + ["normalizedPublisher"] = "contoso \0\0\0", + ["normalizedVersion"] = "1.0", + ["isWindowsInstaller"] = true, + }, + }, + }, + }; + + var result = await BuildLoader(new List { evt }).LoadShutdownInventoryAsync(TenantId, SessionId); + + var item = Assert.Single(result); + Assert.Equal("Contoso Widget", item["displayName"]); + Assert.Equal("contoso widget", item["normalizedName"]); + Assert.Equal("contoso", item["normalizedPublisher"]); + Assert.Equal("1.0", item["normalizedVersion"]); + Assert.Equal(true, item["isWindowsInstaller"]); + } + // ------------------------------------------------------------------------- // Phase filtering — guards against cross-WG-phase contamination // ------------------------------------------------------------------------- diff --git a/src/Backend/AutopilotMonitor.Functions/DataAccess/TableStorage/TableHardwareRejectionNotificationTracker.cs b/src/Backend/AutopilotMonitor.Functions/DataAccess/TableStorage/TableHardwareRejectionNotificationTracker.cs index 9fad7533..283c7a23 100644 --- a/src/Backend/AutopilotMonitor.Functions/DataAccess/TableStorage/TableHardwareRejectionNotificationTracker.cs +++ b/src/Backend/AutopilotMonitor.Functions/DataAccess/TableStorage/TableHardwareRejectionNotificationTracker.cs @@ -1,4 +1,5 @@ using System.Text.Json; +using AutopilotMonitor.Functions.Helpers; using AutopilotMonitor.Functions.Services; using AutopilotMonitor.Shared; using AutopilotMonitor.Shared.DataAccess; @@ -431,9 +432,9 @@ namespace AutopilotMonitor.Functions.DataAccess.TableStorage { // Paths are a closed vocabulary plus rule ids (ANALYZE-...), statuses are enum names and // the group marker "*" - none contain table-key-hostile characters, but sanitize anyway. - var k = SanitizeTableKey((kind ?? string.Empty).Trim().ToLowerInvariant()); - var p = SanitizeTableKey((verdictPath ?? string.Empty).Trim().ToLowerInvariant()); - var s = SanitizeTableKey((status ?? string.Empty).Trim().ToLowerInvariant()); + var k = TableKeySanitizer.Sanitize((kind ?? string.Empty).Trim().ToLowerInvariant()); + var p = TableKeySanitizer.Sanitize((verdictPath ?? string.Empty).Trim().ToLowerInvariant()); + var s = TableKeySanitizer.Sanitize((status ?? string.Empty).Trim().ToLowerInvariant()); return $"{VerdictCalibrationRowKeyPrefix}{k}|{p}|{s}"; } @@ -579,25 +580,15 @@ namespace AutopilotMonitor.Functions.DataAccess.TableStorage /// internal static string BuildAppVersionRegressionRowKey(string? source, string appName, string version) { - var app = SanitizeTableKey((appName ?? string.Empty).Trim().ToLowerInvariant()); - var ver = SanitizeTableKey((version ?? string.Empty).Trim().ToLowerInvariant()); + var app = TableKeySanitizer.Sanitize((appName ?? string.Empty).Trim().ToLowerInvariant()); + var ver = TableKeySanitizer.Sanitize((version ?? string.Empty).Trim().ToLowerInvariant()); // IME keys predate channels and keep their shape; other channels prefix the app part. var channel = AppInstallSources.Normalize(source); if (channel != AppInstallSources.Ime) - app = $"{SanitizeTableKey(channel)}:{app}"; + app = $"{TableKeySanitizer.Sanitize(channel)}:{app}"; return $"{AppVersionRegressionRowKeyPrefix}{app}|{ver}"; } - /// Replace characters not allowed in Azure Table Storage keys (/, \, #, ?). - private static string SanitizeTableKey(string key) - { - return key - .Replace("/", "_") - .Replace("\\", "_") - .Replace("#", "_") - .Replace("?", "_"); - } - // internal static: entity builder + mapper are pinned by round-trip unit tests // (table-storage-serialization rule — every property must survive Store→Map). internal static TableEntity BuildAppVersionRegressionEntity(string tenantId, AppVersionRegressionAlert alert) diff --git a/src/Backend/AutopilotMonitor.Functions/DataAccess/TableStorage/TableVulnerabilityRepository.cs b/src/Backend/AutopilotMonitor.Functions/DataAccess/TableStorage/TableVulnerabilityRepository.cs index 83bf612c..1211f95b 100644 --- a/src/Backend/AutopilotMonitor.Functions/DataAccess/TableStorage/TableVulnerabilityRepository.cs +++ b/src/Backend/AutopilotMonitor.Functions/DataAccess/TableStorage/TableVulnerabilityRepository.cs @@ -48,10 +48,6 @@ namespace AutopilotMonitor.Functions.DataAccess.TableStorage // --- Software Inventory --- - public Task UpsertSoftwareInventoryAsync(string tenantId, List> inventoryItems, - string sessionId, Dictionary? cpeMappings = null) - => _storage.UpsertSoftwareInventoryAsync(tenantId, inventoryItems, sessionId, cpeMappings); - public Task>> GetSoftwareInventoryAsync(string tenantId) => _storage.GetSoftwareInventoryAsync(tenantId); diff --git a/src/Backend/AutopilotMonitor.Functions/Functions/Vulnerability/AutoResolveCpeMappingFunction.cs b/src/Backend/AutopilotMonitor.Functions/Functions/Vulnerability/AutoResolveCpeMappingFunction.cs index b6006983..f7c57232 100644 --- a/src/Backend/AutopilotMonitor.Functions/Functions/Vulnerability/AutoResolveCpeMappingFunction.cs +++ b/src/Backend/AutopilotMonitor.Functions/Functions/Vulnerability/AutoResolveCpeMappingFunction.cs @@ -121,9 +121,8 @@ namespace AutopilotMonitor.Functions.Functions.Vulnerability item.SoftwareName); // Save as custom mapping - var rowKey = $"{(mapping.NormalizedVendor ?? "unknown")}:{mapping.NormalizedProduct}" - .ToLowerInvariant() - .Replace("/", "_").Replace("\\", "_").Replace("#", "_").Replace("?", "_"); + var rowKey = TableKeySanitizer.Sanitize( + $"{(mapping.NormalizedVendor ?? "unknown")}:{mapping.NormalizedProduct}".ToLowerInvariant()); var properties = new Dictionary { diff --git a/src/Backend/AutopilotMonitor.Functions/Functions/Vulnerability/DeleteCustomCpeMappingFunction.cs b/src/Backend/AutopilotMonitor.Functions/Functions/Vulnerability/DeleteCustomCpeMappingFunction.cs index 39212094..4604a17c 100644 --- a/src/Backend/AutopilotMonitor.Functions/Functions/Vulnerability/DeleteCustomCpeMappingFunction.cs +++ b/src/Backend/AutopilotMonitor.Functions/Functions/Vulnerability/DeleteCustomCpeMappingFunction.cs @@ -49,8 +49,8 @@ namespace AutopilotMonitor.Functions.Functions.Vulnerability return await req.BadRequestAsync("normalizedProduct is required"); } - var rowKey = $"{(request.NormalizedVendor ?? "unknown")}:{request.NormalizedProduct}".ToLowerInvariant(); - rowKey = rowKey.Replace("/", "_").Replace("\\", "_").Replace("#", "_").Replace("?", "_"); + var rowKey = TableKeySanitizer.Sanitize( + $"{(request.NormalizedVendor ?? "unknown")}:{request.NormalizedProduct}".ToLowerInvariant()); // Check if mapping exists before deleting (repository silently ignores 404) var existingMappings = await _vulnRepo.GetCpeMappingsByPartitionAsync("cpe_map_custom"); diff --git a/src/Backend/AutopilotMonitor.Functions/Functions/Vulnerability/IgnoreSoftwareFunction.cs b/src/Backend/AutopilotMonitor.Functions/Functions/Vulnerability/IgnoreSoftwareFunction.cs index 996d9cf5..db197632 100644 --- a/src/Backend/AutopilotMonitor.Functions/Functions/Vulnerability/IgnoreSoftwareFunction.cs +++ b/src/Backend/AutopilotMonitor.Functions/Functions/Vulnerability/IgnoreSoftwareFunction.cs @@ -85,9 +85,8 @@ namespace AutopilotMonitor.Functions.Functions.Vulnerability { if (string.IsNullOrWhiteSpace(item.SoftwareName)) continue; - var rowKey = $"{(item.Publisher ?? "unknown")}:{item.SoftwareName}" - .ToLowerInvariant() - .Replace("/", "_").Replace("\\", "_").Replace("#", "_").Replace("?", "_"); + var rowKey = TableKeySanitizer.Sanitize( + $"{(item.Publisher ?? "unknown")}:{item.SoftwareName}".ToLowerInvariant()); var properties = new Dictionary { @@ -131,9 +130,8 @@ namespace AutopilotMonitor.Functions.Functions.Vulnerability return await req.BadRequestAsync("softwareName is required"); } - var rowKey = $"{(request.Publisher ?? "unknown")}:{request.SoftwareName}" - .ToLowerInvariant() - .Replace("/", "_").Replace("\\", "_").Replace("#", "_").Replace("?", "_"); + var rowKey = TableKeySanitizer.Sanitize( + $"{(request.Publisher ?? "unknown")}:{request.SoftwareName}".ToLowerInvariant()); await _vulnRepo.DeleteCpeMappingAsync(IgnorePartition, rowKey); diff --git a/src/Backend/AutopilotMonitor.Functions/Functions/Vulnerability/SaveCustomCpeMappingFunction.cs b/src/Backend/AutopilotMonitor.Functions/Functions/Vulnerability/SaveCustomCpeMappingFunction.cs index dd7b48e9..5334979c 100644 --- a/src/Backend/AutopilotMonitor.Functions/Functions/Vulnerability/SaveCustomCpeMappingFunction.cs +++ b/src/Backend/AutopilotMonitor.Functions/Functions/Vulnerability/SaveCustomCpeMappingFunction.cs @@ -56,9 +56,8 @@ namespace AutopilotMonitor.Functions.Functions.Vulnerability mapping.CpeUri = CpeUriNormalizer.Normalize(mapping.CpeUri); mapping.CpeProduct = CpeUriNormalizer.Normalize(mapping.CpeProduct); - var rowKey = $"{(mapping.NormalizedVendor ?? "unknown")}:{mapping.NormalizedProduct}".ToLowerInvariant(); - // Sanitize RowKey (Table Storage doesn't allow /, \, #, ?) - rowKey = rowKey.Replace("/", "_").Replace("\\", "_").Replace("#", "_").Replace("?", "_"); + var rowKey = TableKeySanitizer.Sanitize( + $"{(mapping.NormalizedVendor ?? "unknown")}:{mapping.NormalizedProduct}".ToLowerInvariant()); var properties = new Dictionary { diff --git a/src/Backend/AutopilotMonitor.Functions/Helpers/EndpointNormalizer.cs b/src/Backend/AutopilotMonitor.Functions/Helpers/EndpointNormalizer.cs index cd3c555c..bd2abe09 100644 --- a/src/Backend/AutopilotMonitor.Functions/Helpers/EndpointNormalizer.cs +++ b/src/Backend/AutopilotMonitor.Functions/Helpers/EndpointNormalizer.cs @@ -32,7 +32,7 @@ public static class EndpointNormalizer // Replace GUIDs with {id} normalized = GuidPattern.Replace(normalized, "{id}"); - return ReplaceIllegalKeyChars(normalized).ToLowerInvariant(); + return TableKeySanitizer.Sanitize(normalized).ToLowerInvariant(); } /// @@ -53,11 +53,7 @@ public static class EndpointNormalizer // recognizes; the regex pass then drops any remaining control character or separator. var cleaned = toolName.Replace("\r", string.Empty).Replace("\n", string.Empty); cleaned = ControlChars.Replace(cleaned, string.Empty); - cleaned = ReplaceIllegalKeyChars(cleaned).Trim(); + cleaned = TableKeySanitizer.Sanitize(cleaned).Trim(); return cleaned.Length <= MaxToolNameLength ? cleaned : cleaned[..MaxToolNameLength]; } - - /// Replaces the characters Azure Table Storage rejects in keys (/, \, #, ?). - private static string ReplaceIllegalKeyChars(string value) - => value.Replace('/', '_').Replace('\\', '_').Replace('#', '_').Replace('?', '_'); } diff --git a/src/Backend/AutopilotMonitor.Functions/Helpers/TableKeySanitizer.cs b/src/Backend/AutopilotMonitor.Functions/Helpers/TableKeySanitizer.cs new file mode 100644 index 00000000..9d0a8309 --- /dev/null +++ b/src/Backend/AutopilotMonitor.Functions/Helpers/TableKeySanitizer.cs @@ -0,0 +1,45 @@ +using System.Text; + +namespace AutopilotMonitor.Functions.Helpers; + +/// +/// The one rule for turning free text (device inventory, app names, admin input) into an Azure +/// Table Storage key. Every builder of a key from free text uses it, so the same text always maps +/// to the same key. +/// +/// Control characters (U+0000–U+001F, U+007F–U+009F) are removed: Table Storage rejects them in +/// keys, and a point read, update or delete carries the key in its request URL, which the HTTP layer +/// in front of the service refuses outright ("400 Invalid URL"). The characters Table Storage +/// rejects in keys (/, \, #, ?) become '_'. Text without control characters keeps its key byte for +/// byte, so existing rows stay reachable. +/// +/// +public static class TableKeySanitizer +{ + public static string Sanitize(string key) + { + var sb = new StringBuilder(key.Length); + foreach (var c in key) + { + if (char.IsControl(c)) + continue; + sb.Append(c is '/' or '\\' or '#' or '?' ? '_' : c); + } + return sb.ToString(); + } + + /// + /// Removes control characters only — for text that later becomes part of a key or an identity + /// compared against one (the software inventory triple). + /// + public static string RemoveControlCharacters(string text) + { + var sb = new StringBuilder(text.Length); + foreach (var c in text) + { + if (!char.IsControl(c)) + sb.Append(c); + } + return sb.ToString(); + } +} diff --git a/src/Backend/AutopilotMonitor.Functions/Services/OpsEventService.cs b/src/Backend/AutopilotMonitor.Functions/Services/OpsEventService.cs index 97fffc1a..bfd41e54 100644 --- a/src/Backend/AutopilotMonitor.Functions/Services/OpsEventService.cs +++ b/src/Backend/AutopilotMonitor.Functions/Services/OpsEventService.cs @@ -171,6 +171,23 @@ namespace AutopilotMonitor.Functions.Services }); } + /// + /// One vulnerability-correlation run could not write every SoftwareInventory counter, or + /// its per-session contributions side-row (: then no + /// counter was touched). One event per run, never per key. The counters stay off by these + /// amounts (accepted drift, Plan §17), so a repeating event points at a systematic cause. + /// is the exception type plus a short reason — never a response body. + /// + public Task RecordSoftwareInventoryCounterWriteFailedAsync( + string tenantId, string sessionId, int failedIncrements, int failedDecrements, bool sideRowFailed, + string error, IReadOnlyList sampleKeys) + => WriteAsync(OpsEventCategory.Maintenance, OpsEventTypes.SoftwareInventoryCounterWriteFailed, OpsEventSeverity.Warning, + sideRowFailed + ? $"Software inventory contributions not written for session {sessionId}: {error}" + : $"{failedIncrements + failedDecrements} software inventory counter writes failed for session {sessionId}: {error}", + tenantId, "System.VulnerabilityCorrelation", + new { sessionId, failedIncrements, failedDecrements, sideRowFailed, error, sampleKeys }); + // ── Cascade-Delete Maintenance (Plan §5 PR6 / §16 R14) ───────────────── // Event types dispatched by SessionDeletionMaintenanceFunction: Started, LongRunning, // LongRunningSevere, BudgetExceeded, SkippedLocked, Failed, Completed, FanoutSkipped diff --git a/src/Backend/AutopilotMonitor.Functions/Services/TableStorageService.Inventory.cs b/src/Backend/AutopilotMonitor.Functions/Services/TableStorageService.Inventory.cs index cc0969b4..79fac3ff 100644 --- a/src/Backend/AutopilotMonitor.Functions/Services/TableStorageService.Inventory.cs +++ b/src/Backend/AutopilotMonitor.Functions/Services/TableStorageService.Inventory.cs @@ -5,6 +5,7 @@ using System.Threading; using System.Threading.Tasks; using Azure; using Azure.Data.Tables; +using AutopilotMonitor.Functions.Helpers; using AutopilotMonitor.Functions.Security; using AutopilotMonitor.Shared; using AutopilotMonitor.Shared.Models.Deletion; @@ -450,15 +451,13 @@ namespace AutopilotMonitor.Functions.Services // ============================================================ Helpers ==== /// - /// SoftwareInventory RowKey format — preserves the same shape used by the legacy - /// UpsertSoftwareInventoryAsync path so existing rows remain reachable by the - /// new helpers. SanitizeTableKey lives in TableStorageService.Rules.cs as a - /// private static; partial classes share scope. + /// SoftwareInventory RowKey format: vendor:name:version through + /// , capped at 512 characters. /// private static string BuildSoftwareInventoryRowKey(string vendor, string name, string version) { var raw = $"{vendor ?? string.Empty}:{name ?? string.Empty}:{version ?? string.Empty}"; - var sanitized = SanitizeTableKey(raw); + var sanitized = TableKeySanitizer.Sanitize(raw); return sanitized.Length > 512 ? sanitized.Substring(0, 512) : sanitized; } } diff --git a/src/Backend/AutopilotMonitor.Functions/Services/TableStorageService.Metrics.cs b/src/Backend/AutopilotMonitor.Functions/Services/TableStorageService.Metrics.cs index fcc20ef2..d3b491a1 100644 --- a/src/Backend/AutopilotMonitor.Functions/Services/TableStorageService.Metrics.cs +++ b/src/Backend/AutopilotMonitor.Functions/Services/TableStorageService.Metrics.cs @@ -249,7 +249,7 @@ namespace AutopilotMonitor.Functions.Services internal static string BuildAppInstallSummaryRowKey(AppInstallSummary summary) { var source = AppInstallSources.Normalize(summary.Source); - return SanitizeTableKey(source == AppInstallSources.Ime + return TableKeySanitizer.Sanitize(source == AppInstallSources.Ime ? $"{summary.SessionId}_{summary.AppName}" : $"{summary.SessionId}_{source}|{summary.InstallScope}|{summary.AppId}"); } diff --git a/src/Backend/AutopilotMonitor.Functions/Services/TableStorageService.Rules.cs b/src/Backend/AutopilotMonitor.Functions/Services/TableStorageService.Rules.cs index ba4731d0..5a678f4d 100644 --- a/src/Backend/AutopilotMonitor.Functions/Services/TableStorageService.Rules.cs +++ b/src/Backend/AutopilotMonitor.Functions/Services/TableStorageService.Rules.cs @@ -1,6 +1,7 @@ using System.Text.Json; using Azure.Data.Tables; using AutopilotMonitor.Functions.DataAccess.TableStorage; +using AutopilotMonitor.Functions.Helpers; using AutopilotMonitor.Functions.Security; using AutopilotMonitor.Functions.Services.Caching; using AutopilotMonitor.Functions.Services.Vulnerability; @@ -1044,104 +1045,6 @@ namespace AutopilotMonitor.Functions.Services // ===== SOFTWARE INVENTORY METHODS ===== - /// - /// Upserts software inventory entries for a tenant. - /// PK = tenantId, RK = {normalizedVendor}:{normalizedProduct}:{normalizedVersion} (sanitized). - /// If an entry already exists, increments SessionCount and updates LastSeenAt/LastSessionId. - /// - public async Task UpsertSoftwareInventoryAsync(string tenantId, List> inventoryItems, string sessionId, Dictionary? cpeMappings = null) - { - SecurityValidator.EnsureValidGuid(tenantId, nameof(tenantId)); - - try - { - var tableClient = _tableServiceClient.GetTableClient(Constants.TableNames.SoftwareInventory); - - // Read all existing entities for this tenant to merge SessionCount - var existingEntities = new Dictionary(StringComparer.OrdinalIgnoreCase); - var existingQuery = tableClient.QueryAsync(filter: $"PartitionKey eq '{tenantId}'"); - await foreach (var entity in existingQuery) - { - existingEntities[entity.RowKey] = entity; - } - - // Build the batch of entities to upsert (dictionary to deduplicate by RowKey) - var entitiesToUpsert = new Dictionary(StringComparer.OrdinalIgnoreCase); - - foreach (var item in inventoryItems) - { - var normalizedName = item.ContainsKey("normalizedName") ? item["normalizedName"]?.ToString() ?? "" : ""; - var normalizedVendor = item.ContainsKey("normalizedPublisher") ? item["normalizedPublisher"]?.ToString() ?? "" : ""; - var normalizedVersion = item.ContainsKey("normalizedVersion") ? item["normalizedVersion"]?.ToString() ?? "" : ""; - var displayName = item.ContainsKey("displayName") ? item["displayName"]?.ToString() ?? "" : ""; - var publisher = item.ContainsKey("publisher") ? item["publisher"]?.ToString() ?? "" : ""; - var registrySource = item.ContainsKey("registrySource") ? item["registrySource"]?.ToString() ?? "" : ""; - var normalizationConfidence = item.ContainsKey("normalizationConfidence") ? item["normalizationConfidence"]?.ToString() ?? "" : ""; - - var rowKey = SanitizeTableKey($"{normalizedVendor}:{normalizedName}:{normalizedVersion}"); - if (string.IsNullOrWhiteSpace(rowKey) || rowKey == "::") - continue; - - // Truncate RowKey to Table Storage limit (1KB) - if (rowKey.Length > 512) - rowKey = rowKey.Substring(0, 512); - - var now = DateTime.UtcNow; - TableEntity entity; - - if (existingEntities.TryGetValue(rowKey, out var existing)) - { - // Update existing: increment SessionCount, update LastSeenAt - existing["SessionCount"] = (existing.GetInt32("SessionCount") ?? 0) + 1; - existing["LastSeenAt"] = now.ToString("o"); - existing["LastSessionId"] = sessionId; - - // Update CpeUri if we have a mapping and current is empty - if (cpeMappings != null && cpeMappings.TryGetValue(normalizedName, out var cpeUri) && !string.IsNullOrEmpty(cpeUri)) - { - existing["CpeUri"] = cpeUri; - } - - entity = existing; - } - else - { - // Create new entry - entity = new TableEntity(tenantId, rowKey) - { - ["DisplayName"] = displayName, - ["NormalizedName"] = normalizedName, - ["NormalizedVendor"] = normalizedVendor, - ["NormalizedVersion"] = normalizedVersion, - ["Publisher"] = publisher, - ["RegistrySource"] = registrySource, - ["NormalizationConfidence"] = normalizationConfidence, - ["FirstSeenAt"] = now.ToString("o"), - ["LastSeenAt"] = now.ToString("o"), - ["FirstSessionId"] = sessionId, - ["LastSessionId"] = sessionId, - ["SessionCount"] = 1, - ["CpeUri"] = cpeMappings != null && cpeMappings.TryGetValue(normalizedName, out var cpeUri) && !string.IsNullOrEmpty(cpeUri) ? cpeUri : "" - }; - } - - entitiesToUpsert[rowKey] = entity; - } - - // Batch write (byte-aware, all same PK) - var upsertList = entitiesToUpsert.Values.ToList(); - foreach (var batch in TableTransactionBatcher.Split(upsertList, TableTransactionActionType.UpsertReplace)) - await tableClient.SubmitTransactionAsync(batch); - - _logger.LogInformation("Upserted {Count} software inventory entries for tenant {TenantId}", upsertList.Count, tenantId); - } - catch (Exception ex) - { - _logger.LogWarning(ex, "Failed to upsert software inventory for tenant {TenantId}", tenantId); - throw; - } - } - /// /// Gets all software inventory entries for a tenant. /// PK = tenantId. Returns all rows. @@ -1185,19 +1088,6 @@ namespace AutopilotMonitor.Functions.Services } } - /// - /// Sanitize a string for use as an Azure Table Storage key. - /// Replaces /, \, #, ? with underscores. - /// - private static string SanitizeTableKey(string key) - { - return key - .Replace("/", "_") - .Replace("\\", "_") - .Replace("#", "_") - .Replace("?", "_"); - } - // ===== CPE MAPPING SEED IMPORT ===== /// @@ -1231,7 +1121,7 @@ namespace AutopilotMonitor.Functions.Services var entities = new List(); foreach (var mapping in seed.Mappings) { - var rowKey = SanitizeTableKey( + var rowKey = TableKeySanitizer.Sanitize( $"{(mapping.NormalizedVendor ?? "unknown")}:{(mapping.NormalizedProduct ?? "unknown")}".ToLowerInvariant()); if (rowKey.Length > 512) @@ -1291,7 +1181,7 @@ namespace AutopilotMonitor.Functions.Services var entities = new List(); foreach (var mapping in seed.Mappings) { - var rowKey = SanitizeTableKey( + var rowKey = TableKeySanitizer.Sanitize( $"{(mapping.NormalizedVendor ?? "unknown")}:{(mapping.NormalizedProduct ?? "unknown")}".ToLowerInvariant()); if (rowKey.Length > 512) diff --git a/src/Backend/AutopilotMonitor.Functions/Services/Vulnerability/VulnerabilityCorrelationService.cs b/src/Backend/AutopilotMonitor.Functions/Services/Vulnerability/VulnerabilityCorrelationService.cs index 4f71e8ce..fc5195ca 100644 --- a/src/Backend/AutopilotMonitor.Functions/Services/Vulnerability/VulnerabilityCorrelationService.cs +++ b/src/Backend/AutopilotMonitor.Functions/Services/Vulnerability/VulnerabilityCorrelationService.cs @@ -6,6 +6,7 @@ using System.Text.Json.Serialization; using System.Threading; using System.Threading.Tasks; using AutopilotMonitor.Functions.DataAccess.TableStorage; +using AutopilotMonitor.Functions.Helpers; using AutopilotMonitor.Shared.DataAccess; using AutopilotMonitor.Shared.Models.Deletion; using Microsoft.ApplicationInsights; @@ -17,7 +18,7 @@ namespace AutopilotMonitor.Functions.Services.Vulnerability { private readonly IVulnerabilityRepository _vulnRepo; private readonly ISessionRepository _sessionRepo; - private readonly IMaintenanceRepository? _maintenanceRepo; + private readonly OpsEventService? _opsEvents; private readonly NvdApiClient _nvdApi; private readonly KevDataService _kevService; private readonly MsrcCorrelationService _msrcCorrelation; @@ -48,12 +49,12 @@ namespace AutopilotMonitor.Functions.Services.Vulnerability AdminConfigurationService adminConfig, TelemetryClient? telemetryClient, ILogger logger, - IMaintenanceRepository? maintenanceRepo = null, + OpsEventService? opsEvents = null, EpssApiClient? epssApi = null) { _vulnRepo = vulnRepo; _sessionRepo = sessionRepo; - _maintenanceRepo = maintenanceRepo; + _opsEvents = opsEvents; _nvdApi = nvdApi; _kevService = kevService; _msrcCorrelation = msrcCorrelation; @@ -267,13 +268,13 @@ namespace AutopilotMonitor.Functions.Services.Vulnerability return ra.CompareTo(rb); }); - // Persist software inventory via the contributions side-row (Plan §17). - // Fire-and-forget so the report still gets stored if the side-row write hits a - // transient storage failure — the report is the primary product, inventory is the - // secondary tenant-aggregate signal. PR3 wires SessionDeletionGuard above this - // layer (at VulnerabilityCorrelateHandler) so a session locked by a cascade never - // reaches this code path; PR2's side-row write proceeds unguarded. - _ = Task.Run(() => ApplyInventoryContributionsAsync(tenantId, sessionId, inventoryItems, cpeMappingsDict)); + // Persist software inventory via the contributions side-row (Plan §17). Awaited inside + // the invocation: a detached task can be cut off by a scale-in after the side-row has + // already promised the key set, leaving the counters short with no trace. The method + // never throws, so the report — the primary product — is stored regardless. PR3 wires + // SessionDeletionGuard above this layer (at VulnerabilityCorrelateHandler) so a session + // locked by a cascade never reaches this code path. + await ApplyInventoryContributionsAsync(tenantId, sessionId, inventoryItems, cpeMappingsDict); // Summary log for production monitoring _logger.LogInformation( @@ -536,9 +537,8 @@ namespace AutopilotMonitor.Functions.Services.Vulnerability { try { - var rowKey = $"{(mapping.NormalizedVendor ?? "unknown")}:{mapping.NormalizedProduct}" - .ToLowerInvariant() - .Replace("/", "_").Replace("\\", "_").Replace("#", "_").Replace("?", "_"); + var rowKey = TableKeySanitizer.Sanitize( + $"{(mapping.NormalizedVendor ?? "unknown")}:{mapping.NormalizedProduct}".ToLowerInvariant()); var properties = new Dictionary { @@ -613,7 +613,7 @@ namespace AutopilotMonitor.Functions.Services.Vulnerability try { // 1. Try cache first - var cacheKey = SanitizeTableKey($"cve_{cpeUri}"); + var cacheKey = TableKeySanitizer.Sanitize($"cve_{cpeUri}"); var cachedResult = await GetCachedCvesAsync(cacheKey); if (cachedResult == null || cachedResult.IsHardExpired) @@ -1076,13 +1076,13 @@ namespace AutopilotMonitor.Functions.Services.Vulnerability /// +1 per added (with full metadata for first-insert) and -1 per removed /// (clamp ≥ 0). /// - /// Per Plan §17.3 "Side-row write before counter writes" + §17.8: individual counter - /// failures must NOT abort the rest of the loop, because the side-row already promises - /// the new key set — if we stopped on first failure, the next correlation run would see - /// an empty delta and never retry the under-counted keys. Each counter call is therefore - /// guarded individually; failures emit an inventory_counter_write_failed audit - /// entry (when the maintenance repo is wired) and continue processing. The wrapping - /// catch handles only the side-row CAS exhaustion + the parser path. + /// Per Plan §17.3 "Side-row write before counter writes": individual counter failures must + /// NOT abort the rest of the loop, because the side-row already promises the new key set — + /// if we stopped on first failure, the next correlation run would see an empty delta and + /// never retry the under-counted keys. Each counter call is therefore guarded individually; + /// the failures of a run are counted and reported as ONE + /// SoftwareInventoryCounterWriteFailed ops event — an operator signal, never an entry + /// in the tenant's audit trail. Never throws. /// /// internal async Task ApplyInventoryContributionsAsync( @@ -1102,9 +1102,21 @@ namespace AutopilotMonitor.Functions.Services.Vulnerability _logger.LogWarning(ex, "Failed to upsert software inventory contributions side-row (non-fatal). tenant={TenantId} session={SessionId}", tenantId, sessionId); + await ReportCounterFailuresAsync(tenantId, sessionId, 0, 0, sideRowFailed: true, ex, Array.Empty()); return; } + var failedIncrements = 0; + var failedDecrements = 0; + Exception? firstError = null; + var sampleKeys = new List(); + void Record(Exception ex, string vendor, string name, string version) + { + firstError ??= ex; + if (sampleKeys.Count < MaxCounterFailureSamples) + sampleKeys.Add($"{vendor}:{name}:{version}"); + } + foreach (var added in delta.AddedItems) { try @@ -1116,9 +1128,8 @@ namespace AutopilotMonitor.Functions.Services.Vulnerability _logger.LogWarning(ex, "SoftwareInventory increment failed (non-fatal — counter drift accepted). tenant={TenantId} session={SessionId} vendor={Vendor} name={Name} version={Version}", tenantId, sessionId, added.NormalizedVendor, added.NormalizedName, added.NormalizedVersion); - await TryAuditInventoryCounterFailureAsync( - tenantId, sessionId, "increment", - added.NormalizedVendor, added.NormalizedName, added.NormalizedVersion, ex); + failedIncrements++; + Record(ex, added.NormalizedVendor, added.NormalizedName, added.NormalizedVersion); } } @@ -1134,46 +1145,43 @@ namespace AutopilotMonitor.Functions.Services.Vulnerability _logger.LogWarning(ex, "SoftwareInventory decrement failed (non-fatal — counter drift accepted). tenant={TenantId} session={SessionId} vendor={Vendor} name={Name} version={Version}", tenantId, sessionId, removed.Vendor, removed.Name, removed.Version); - await TryAuditInventoryCounterFailureAsync( - tenantId, sessionId, "decrement", - removed.Vendor, removed.Name, removed.Version, ex); + failedDecrements++; + Record(ex, removed.Vendor, removed.Name, removed.Version); } } + + if (firstError != null) + await ReportCounterFailuresAsync(tenantId, sessionId, failedIncrements, failedDecrements, sideRowFailed: false, firstError, sampleKeys); } + private const int MaxCounterFailureSamples = 5; + + private Task ReportCounterFailuresAsync( + string tenantId, string sessionId, int failedIncrements, int failedDecrements, bool sideRowFailed, + Exception firstError, IReadOnlyList sampleKeys) + => _opsEvents?.RecordSoftwareInventoryCounterWriteFailedAsync( + tenantId, sessionId, failedIncrements, failedDecrements, sideRowFailed, + DescribeCounterFailure(firstError), sampleKeys) + ?? Task.CompletedTask; + /// - /// Emits the inventory_counter_write_failed audit entry on a single counter - /// failure (Plan §17.8). Non-fatal — audit emission itself must not break the loop. + /// Exception type plus a short reason for the ops event. A storage failure is described by + /// status and error code only — its message embeds the raw response body and headers. /// - private async Task TryAuditInventoryCounterFailureAsync( - string tenantId, string sessionId, string operation, - string vendor, string name, string version, Exception failureCause) + internal static string DescribeCounterFailure(Exception ex) { - if (_maintenanceRepo == null) return; - try + if (ex is Azure.RequestFailedException rfe) { - await _maintenanceRepo.LogAuditEntryAsync( - tenantId, - action: "inventory_counter_write_failed", - entityType: "Session", - entityId: sessionId, - performedBy: "system", - details: new Dictionary(StringComparer.Ordinal) - { - ["operation"] = operation, - ["vendor"] = vendor, - ["name"] = name, - ["version"] = version, - ["exceptionType"] = failureCause.GetType().Name, - ["exceptionMessage"] = failureCause.Message, - }); - } - catch (Exception ex) - { - _logger.LogDebug(ex, - "Failed to emit inventory_counter_write_failed audit entry (suppressed). tenant={TenantId} session={SessionId}", - tenantId, sessionId); + return string.IsNullOrEmpty(rfe.ErrorCode) + ? $"RequestFailedException {rfe.Status}" + : $"RequestFailedException {rfe.Status} {rfe.ErrorCode}"; } + + var message = ex.Message; + var lineEnd = message.IndexOfAny(new[] { '\r', '\n' }); + if (lineEnd >= 0) message = message.Substring(0, lineEnd); + if (message.Length > 200) message = message.Substring(0, 200); + return $"{ex.GetType().Name}: {message}"; } /// @@ -1226,18 +1234,6 @@ namespace AutopilotMonitor.Functions.Services.Vulnerability return items; } - /// - /// Replace characters not allowed in Azure Table Storage keys (/, \, #, ?). - /// - private static string SanitizeTableKey(string key) - { - return key - .Replace("/", "_") - .Replace("\\", "_") - .Replace("#", "_") - .Replace("?", "_"); - } - // ----------------------------------------------------------------------- // CVE Cache Models // ----------------------------------------------------------------------- diff --git a/src/Backend/AutopilotMonitor.Functions/Services/Vulnerability/VulnerabilityInventoryLoader.cs b/src/Backend/AutopilotMonitor.Functions/Services/Vulnerability/VulnerabilityInventoryLoader.cs index 8a6e5214..a6337a9d 100644 --- a/src/Backend/AutopilotMonitor.Functions/Services/Vulnerability/VulnerabilityInventoryLoader.cs +++ b/src/Backend/AutopilotMonitor.Functions/Services/Vulnerability/VulnerabilityInventoryLoader.cs @@ -3,6 +3,7 @@ using System.Collections.Generic; using System.Linq; using System.Threading; using System.Threading.Tasks; +using AutopilotMonitor.Functions.Helpers; using AutopilotMonitor.Shared; using AutopilotMonitor.Shared.DataAccess; using Microsoft.Extensions.Logging; @@ -70,13 +71,9 @@ namespace AutopilotMonitor.Functions.Services.Vulnerability { foreach (var entry in nativeList) { - if (entry is Dictionary dict) + if (entry is IDictionary dict) { - items.Add(dict); - } - else if (entry is IDictionary idict) - { - items.Add(new Dictionary(idict)); + items.Add(WithoutControlCharacters(dict)); } } } @@ -86,7 +83,7 @@ namespace AutopilotMonitor.Functions.Services.Vulnerability { if (item is Newtonsoft.Json.Linq.JObject obj) { - items.Add(obj.ToObject>() ?? new Dictionary()); + items.Add(WithoutControlCharacters(obj.ToObject>() ?? new Dictionary())); } } } @@ -104,6 +101,26 @@ namespace AutopilotMonitor.Functions.Services.Vulnerability return items; } + /// + /// Inventory text comes from the device registry, where some installers pad REG_SZ values + /// with NULs. The vendor/name/version triple built from it becomes a table key and a + /// side-row identity, and neither may carry a control character — so every text value + /// leaves the loader without them, trimmed. The source dictionary is not mutated. + /// + internal static Dictionary WithoutControlCharacters(IDictionary item) + { + var clean = item is Dictionary typed + ? new Dictionary(typed.Count, typed.Comparer) + : new Dictionary(item.Count); + foreach (var (key, value) in item) + { + clean[key] = value is string text + ? TableKeySanitizer.RemoveControlCharacters(text).Trim() + : value; + } + return clean; + } + /// /// Checks whether an event's whiteglove_part tag matches the requested phase. /// Numeric parsing is lenient because EnrollmentEvent.Data values can come diff --git a/src/McpServer/autopilot-monitor-mcp/src/generated/wire-vocabularies.generated.ts b/src/McpServer/autopilot-monitor-mcp/src/generated/wire-vocabularies.generated.ts index a79f29b5..2a0a70d2 100644 --- a/src/McpServer/autopilot-monitor-mcp/src/generated/wire-vocabularies.generated.ts +++ b/src/McpServer/autopilot-monitor-mcp/src/generated/wire-vocabularies.generated.ts @@ -86,6 +86,7 @@ export const OPS_EVENT_TYPES = [ "CriticalTableBackupSkippedLocked", "BackupRowRestored", "VerdictCalibrationDrift", + "SoftwareInventoryCounterWriteFailed", "DeviceBlocked", "VersionBlocked", "SessionTenantConflict", diff --git a/src/Shared/AutopilotMonitor.Shared/DataAccess/IVulnerabilityRepository.cs b/src/Shared/AutopilotMonitor.Shared/DataAccess/IVulnerabilityRepository.cs index e604abe5..718d2165 100644 --- a/src/Shared/AutopilotMonitor.Shared/DataAccess/IVulnerabilityRepository.cs +++ b/src/Shared/AutopilotMonitor.Shared/DataAccess/IVulnerabilityRepository.cs @@ -23,8 +23,6 @@ namespace AutopilotMonitor.Shared.DataAccess Task DeleteVulnerabilityReportAsync(string tenantId, string sessionId); // --- Software Inventory --- - Task UpsertSoftwareInventoryAsync(string tenantId, List> inventoryItems, - string sessionId, Dictionary? cpeMappings = null); Task>> GetSoftwareInventoryAsync(string tenantId); /// /// Full cross-tenant scan of SoftwareInventory. projects the diff --git a/src/Shared/AutopilotMonitor.Shared/DataAccess/OpsEventTypes.cs b/src/Shared/AutopilotMonitor.Shared/DataAccess/OpsEventTypes.cs index 3458919e..f472fa20 100644 --- a/src/Shared/AutopilotMonitor.Shared/DataAccess/OpsEventTypes.cs +++ b/src/Shared/AutopilotMonitor.Shared/DataAccess/OpsEventTypes.cs @@ -54,6 +54,12 @@ namespace AutopilotMonitor.Shared.DataAccess public const string CriticalTableBackupSkippedLocked = "CriticalTableBackupSkippedLocked"; public const string BackupRowRestored = "BackupRowRestored"; public const string VerdictCalibrationDrift = "VerdictCalibrationDrift"; + /// + /// A vulnerability-correlation run could not write every SoftwareInventory counter, or the + /// per-session contributions side-row. Warning; one event per run with failure counts and + /// up to five sample keys, never one per key and never in the tenant audit trail. + /// + public const string SoftwareInventoryCounterWriteFailed = "SoftwareInventoryCounterWriteFailed"; // ── Security ── Blocks, ownership conflicts, certificate expiry, capacity + poison-queue alarms. public const string DeviceBlocked = "DeviceBlocked"; @@ -178,7 +184,7 @@ namespace AutopilotMonitor.Shared.DataAccess public static readonly IReadOnlyList All = new[] { ConsentFlowStarted, ConsentFlowSuccess, ConsentFlowFailed, ConsentRedirectUriMismatch, AppHomingFlipped, AppHomingFlippedWithEntraRoles, - MaintenanceCompleted, MaintenanceFailed, MaintenanceLongRunning, MaintenanceStarted, MaintenanceSkippedLocked, SessionSweepCompleted, SessionSweepFailed, OpsEventCleanup, OrphanEventsCleaned, SessionDeletionMaintenanceStarted, SessionDeletionMaintenanceBudgetExceeded, SessionDeletionMaintenanceSkippedLocked, SessionDeletionMaintenanceLongRunning, SessionDeletionMaintenanceLongRunningSevere, SessionDeletionMaintenanceFailed, SessionDeletionStrandedQueued, SessionDeletionPoisoned, SessionDeletionMaintenanceCompleted, SessionDeletionMaintenanceFanoutSkipped, CriticalTableBackupCompleted, CriticalTableBackupPartial, CriticalTableBackupFailed, CriticalTableBackupSkippedLocked, BackupRowRestored, VerdictCalibrationDrift, + MaintenanceCompleted, MaintenanceFailed, MaintenanceLongRunning, MaintenanceStarted, MaintenanceSkippedLocked, SessionSweepCompleted, SessionSweepFailed, OpsEventCleanup, OrphanEventsCleaned, SessionDeletionMaintenanceStarted, SessionDeletionMaintenanceBudgetExceeded, SessionDeletionMaintenanceSkippedLocked, SessionDeletionMaintenanceLongRunning, SessionDeletionMaintenanceLongRunningSevere, SessionDeletionMaintenanceFailed, SessionDeletionStrandedQueued, SessionDeletionPoisoned, SessionDeletionMaintenanceCompleted, SessionDeletionMaintenanceFanoutSkipped, CriticalTableBackupCompleted, CriticalTableBackupPartial, CriticalTableBackupFailed, CriticalTableBackupSkippedLocked, BackupRowRestored, VerdictCalibrationDrift, SoftwareInventoryCounterWriteFailed, DeviceBlocked, VersionBlocked, SessionTenantConflict, SessionOwnerMismatch, KillSignalDelivered, EmbeddedCertExpiringSoon, EmbeddedCertExpiringUrgent, EmbeddedCertExpired, EmbeddedCertBundleEmpty, SignalRConnectionsHigh, SignalRConnectionsCritical, SignalRMessagesHigh, SignalRMessagesCritical, PoisonQueueBacklogHigh, PoisonQueueBacklogCritical, ExcessiveSessionEventsAutoActioned, PrivilegedRouteDenied, McpServicePrincipalFirstSeen, OffboardingFeedbackReceived, FeedbackReceived, TenantSignup, SessionReportSubmitted, DiagFilesReportSubmitted, RuleSubmissionReceived, TenantOffboarded, TenantOffboardingFailed, TenantAutoApproved, WelcomeEmailSent, WelcomeEmailSkipped, WelcomeEmailFailed, FarewellEmailSent, FarewellEmailSkipped, FarewellEmailFailed, TenantTrialStarted, TenantTrialExpiring, TenantTrialExpired, TenantPlanDowngraded, TenantRetentionGraceExpiring, TenantRetentionGraceEnded, RuleFrequencyRegression, AppVersionDurationRegression, CollectLogsQuickConfigEnabled, DiagnosticsUploadEnabled, DiagnosticsUploadDisabled, SessionActionQueued, SessionTimeouts, AgentEmergencyBreak, AgentBinaryIntegrityMismatch, CmTraceTimeSkewRegression, ExcessiveSessionEvents, NewImeVersionDetected, ImePatternDriftSuspected, BlobStorageMissing, BlobStorageUnreachable, TelemetryItemsRejected, diff --git a/src/Web/autopilot-monitor-web/app/admin/components/OpsAlertRulesSection.tsx b/src/Web/autopilot-monitor-web/app/admin/components/OpsAlertRulesSection.tsx index 8358b4c4..83f52c4f 100644 --- a/src/Web/autopilot-monitor-web/app/admin/components/OpsAlertRulesSection.tsx +++ b/src/Web/autopilot-monitor-web/app/admin/components/OpsAlertRulesSection.tsx @@ -31,6 +31,10 @@ const OPS_EVENT_TYPES: Record = { // fallthrough decides ≥20% of classifier verdicts. Warning-tier, once per episode // (tracker-deduped). Dual-register per memory feedback_ops_event_types_dual_register. "VerdictCalibrationDrift", + // A vulnerability-correlation run could not write every SoftwareInventory counter (or the + // per-session side-row). Warning, one per run with counts — replaced the per-key tenant audit + // entry inventory_counter_write_failed. + "SoftwareInventoryCounterWriteFailed", "MaintenanceCompleted", "MaintenanceFailed", // Soft watchdog (Warning): a maintenance run completed but exceeded the 10min threshold and is diff --git a/src/Web/autopilot-monitor-web/utils/shared-manifests.generated.ts b/src/Web/autopilot-monitor-web/utils/shared-manifests.generated.ts index 4be1a9bf..812b4032 100644 --- a/src/Web/autopilot-monitor-web/utils/shared-manifests.generated.ts +++ b/src/Web/autopilot-monitor-web/utils/shared-manifests.generated.ts @@ -620,6 +620,7 @@ export const SHARED_MANIFEST = { "CriticalTableBackupSkippedLocked", "BackupRowRestored", "VerdictCalibrationDrift", + "SoftwareInventoryCounterWriteFailed", "DeviceBlocked", "VersionBlocked", "SessionTenantConflict", diff --git a/src/Web/autopilot-monitor-web/utils/shared-manifests.json b/src/Web/autopilot-monitor-web/utils/shared-manifests.json index 00823d5b..9857818a 100644 --- a/src/Web/autopilot-monitor-web/utils/shared-manifests.json +++ b/src/Web/autopilot-monitor-web/utils/shared-manifests.json @@ -616,6 +616,7 @@ "CriticalTableBackupSkippedLocked", "BackupRowRestored", "VerdictCalibrationDrift", + "SoftwareInventoryCounterWriteFailed", "DeviceBlocked", "VersionBlocked", "SessionTenantConflict",