From df640e368484e99737f4b24beb3a8201f9d04684 Mon Sep 17 00:00:00 2001 From: Oliver Kieselbach Date: Fri, 25 Sep 2026 13:51:53 +0200 Subject: [PATCH] fix(backend): admin configuration is written only in its changed columns; PATCH global/config replaces the full-model PUT (D-285) On 2026-09-25 a Global Settings save from a page loaded before an agent release wrote the whole configuration back and reverted the release's LatestAgentV2* hashes. After that, every 2.0.1465 agent reported IntegrityCheckFailed. The flaw was structural: every writer (portal PUT, plan tiers, three vulnerability-sync stamp paths) replaced the whole row from the copy it held, usually the 5-minute cache. - IConfigRepository.UpdateAdminConfigurationAsync(mutate, updatedBy) is the one write path. It reads the row fresh with its ETag and applies the mutation to a copy materialized from that read. It computes the changed columns from the converter projections and writes only those onto the freshly read raw entity; a null value drops the column. The replace is conditional, and a conflict triggers a re-read and retry. A no-op writes nothing. Pipeline-owned and unknown columns survive. The full-model SaveAdminConfigurationAsync / SaveConfigurationAsync are removed, and the first row is created with AddEntity. - PATCH global/config with { fields } (PatchAdminConfigurationRequest) replaces PUT/POST. Its field gate is derived from the persisted columns and rejects server stamps, LatestAgentV2* / LatestBootstrapV2ScriptVersion, PlanTierDefinitionsJson, the sync stamps and redacted placeholders. The route catalog lists PATCH. - Plan tiers, TriggerMsrcSync, TriggerVulnerabilityDataSync, VulnerabilityDataSyncFunction and VulnerabilityCacheRefreshService.StampAsync each hand in a mutation. - Tests: AdminConfigurationChangedFieldsTests (stale-page toggle keeps the release hashes, concurrent pipeline write re-read and kept, no-op, rejection, null drop, first row, gate); existing tests adapted; the request guard allows the admin field map. - Regenerated shared manifest and wire types (web utils + MCP). The web still sends PUT until the web commit that follows the backend deploy. - The internal pointer is bumped to D-285. Co-Authored-By: Claude Opus 5.5 (1M context) --- internal | 2 +- .../AdminConfigurationChangedFieldsTests.cs | 276 ++++++++++++++++++ .../AdminConfigurationRateLimitSyncTests.cs | 27 +- .../ConfigBackupHookTests.cs | 19 +- .../TypedRequestGuardTests.cs | 14 +- .../TableStorage/TableConfigRepository.cs | 132 ++++++++- .../Config/PatchAdminConfigurationFunction.cs | 223 ++++++++++++++ .../Config/PlanManagementFunction.cs | 7 +- .../UpdateAdminConfigurationFunction.cs | 118 -------- .../Vulnerability/TriggerMsrcSyncFunction.cs | 10 +- .../TriggerVulnerabilityDataSyncFunction.cs | 10 +- .../VulnerabilityDataSyncFunction.cs | 20 +- .../Security/EndpointAccessPolicyCatalog.cs | 4 +- .../Services/AdminConfigurationService.cs | 44 +-- .../TableStorageService.Maintenance.cs | 2 +- .../VulnerabilityCacheRefreshService.cs | 6 +- .../src/generated/wire-types.generated.ts | 7 +- .../AdminConfigurationUpdateResult.cs | 26 ++ .../DataAccess/IConfigRepository.cs | 19 +- .../Models/ConfigApiModels.cs | 12 +- .../utils/shared-manifests.json | 19 +- .../utils/wire-types.generated.ts | 7 +- 22 files changed, 780 insertions(+), 224 deletions(-) create mode 100644 src/Backend/AutopilotMonitor.Functions.Tests/AdminConfigurationChangedFieldsTests.cs create mode 100644 src/Backend/AutopilotMonitor.Functions/Functions/Config/PatchAdminConfigurationFunction.cs delete mode 100644 src/Backend/AutopilotMonitor.Functions/Functions/Config/UpdateAdminConfigurationFunction.cs create mode 100644 src/Shared/AutopilotMonitor.Shared/DataAccess/AdminConfigurationUpdateResult.cs diff --git a/internal b/internal index f0bf0585..3f7411b4 160000 --- a/internal +++ b/internal @@ -1 +1 @@ -Subproject commit f0bf0585a96fb2debd9a2687bbf348ec9f64721a +Subproject commit 3f7411b45467d09ce160cc82a3f20b83b2824d22 diff --git a/src/Backend/AutopilotMonitor.Functions.Tests/AdminConfigurationChangedFieldsTests.cs b/src/Backend/AutopilotMonitor.Functions.Tests/AdminConfigurationChangedFieldsTests.cs new file mode 100644 index 00000000..26176dea --- /dev/null +++ b/src/Backend/AutopilotMonitor.Functions.Tests/AdminConfigurationChangedFieldsTests.cs @@ -0,0 +1,276 @@ +using System; +using System.Collections.Generic; +using System.Linq; +using System.Threading; +using System.Threading.Tasks; +using AutopilotMonitor.Functions.DataAccess.TableStorage; +using AutopilotMonitor.Functions.Functions.Config; +using AutopilotMonitor.Functions.Services; +using AutopilotMonitor.Shared; +using AutopilotMonitor.Shared.DataAccess; +using AutopilotMonitor.Shared.Models; +using Azure; +using Azure.Data.Tables; +using Microsoft.Extensions.Logging.Abstractions; +using Moq; +using Newtonsoft.Json.Linq; +using Xunit; + +namespace AutopilotMonitor.Functions.Tests; + +/// +/// D-285: the admin configuration row is changed only through the changed-columns write — fresh +/// read, mutation on a copy, only the columns whose value changed are laid onto the freshly read +/// row, conditional on its ETag. Pins the 2026-09-25 incident: a Global Settings page loaded before +/// an agent release saved one toggle and reverted the release's hashes with the full-model PUT. +/// +public class AdminConfigurationChangedFieldsTests +{ + private const string ReleaseVersion = "2.0.1465"; + private const string ReleaseZipSha = "9f0b5403da7e49202274fdf8f38675bc5cfea85f7d881e85d5fe4bd71308250e"; + private const string ReleaseExeSha = "503dd02b3c3690acfe20c44dfce82c367ca1e56dcedaba3ecc91ff6ce893d726"; + + /// + /// A one-row table that behaves like storage: reads hand out copies with the current ETag, + /// a replace with a stale ETag fails 412, a create on an existing row fails 409. + /// + private sealed class FakeAdminTable + { + private int _version = 1; + public TableEntity? Current; + public int Writes; + public TableUpdateMode? LastMode; + /// Runs once, right before the first replace is checked — a concurrent writer. + public Action? BeforeFirstReplace; + public TableConfigRepository Sut { get; } + + public FakeAdminTable(TableEntity? current) + { + Current = current; + var table = new Mock(); + table.Setup(c => c.GetEntityAsync( + It.IsAny(), It.IsAny(), It.IsAny>(), It.IsAny())) + .Returns(() => Current == null + ? throw new RequestFailedException(404, "Not Found", "ResourceNotFound", null) + : Task.FromResult(Response.FromValue(Copy(Current, _version), Mock.Of()))); + table.Setup(c => c.UpdateEntityAsync( + It.IsAny(), It.IsAny(), It.IsAny(), It.IsAny())) + .Returns((e, ifMatch, mode, _) => + { + if (BeforeFirstReplace != null) + { + var concurrent = BeforeFirstReplace; + BeforeFirstReplace = null; + concurrent(this); + } + if (ifMatch != Etag(_version)) + throw new RequestFailedException(412, "Precondition Failed", "UpdateConditionNotSatisfied", null); + Current = Copy(e, _version); + _version++; + Writes++; + LastMode = mode; + return Task.FromResult(Mock.Of()); + }); + table.Setup(c => c.AddEntityAsync(It.IsAny(), It.IsAny())) + .Returns((e, _) => + { + if (Current != null) + throw new RequestFailedException(409, "Conflict", "EntityAlreadyExists", null); + Current = Copy(e, _version); + Writes++; + return Task.FromResult(Mock.Of()); + }); + + var serviceClient = new Mock(); + serviceClient.Setup(c => c.GetTableClient(It.IsAny())).Returns(table.Object); + Sut = new TableConfigRepository( + new TableStorageService(serviceClient.Object, NullLogger.Instance), + new Mock().Object, + NullLogger.Instance); + } + + /// A write by someone else (the release pipeline's Merge), bumping the ETag. + public void ExternalMerge(string column, object value) + { + Current![column] = value; + _version++; + } + + private static ETag Etag(int version) => new($"W/\"v{version}\""); + + private static TableEntity Copy(TableEntity source, int version) + => new(source.Where(kv => kv.Key != "odata.etag").ToDictionary(kv => kv.Key, kv => kv.Value)) { ETag = Etag(version) }; + } + + /// The stored row right after the agent release: the pipeline's hashes plus a column this build does not know. + private static TableEntity ReleasedRow() + { + var row = TableConfigRepository.ConvertToAdminTableEntity(new AdminConfiguration + { + UpdatedBy = "ga@operator.example", + GlobalRateLimitRequestsPerMinute = 100, + NvdApiKey = "stored-key", + LatestAgentV2Version = ReleaseVersion, + LatestAgentV2Sha256 = ReleaseZipSha, + LatestAgentV2ExeSha256 = ReleaseExeSha, + }); + row["ColumnOfANewerBuild"] = "keep-me"; + return row; + } + + [Fact] + public async Task Stale_page_toggle_writes_only_that_field_and_keeps_the_release_hashes() + { + var table = new FakeAdminTable(ReleasedRow()); + // The page was loaded before the release; it sends only what the operator changed. + var fields = JObject.Parse("{ \"enforceClientAppBinding\": true }"); + Assert.Null(PatchAdminConfigurationFunction.CheckFields(fields)); + + var result = await table.Sut.UpdateAdminConfigurationAsync( + draft => PatchAdminConfigurationFunction.ApplyFields(draft, fields), "ga@operator.example"); + + Assert.Null(result.Error); + Assert.Equal(new[] { "EnforceClientAppBinding" }, result.ChangedColumns); + var row = table.Current!; + Assert.True(row.GetBoolean("EnforceClientAppBinding")); + Assert.Equal(ReleaseVersion, row.GetString("LatestAgentV2Version")); + Assert.Equal(ReleaseZipSha, row.GetString("LatestAgentV2Sha256")); + Assert.Equal(ReleaseExeSha, row.GetString("LatestAgentV2ExeSha256")); + Assert.Equal("keep-me", row.GetString("ColumnOfANewerBuild")); + Assert.Equal("stored-key", row.GetString("NvdApiKey")); + Assert.Equal(TableUpdateMode.Replace, table.LastMode); + Assert.Equal(1, table.Writes); + } + + [Fact] + public async Task A_concurrent_pipeline_write_is_reread_and_kept() + { + var table = new FakeAdminTable(ReleasedRow()) + { + // The release pipeline lands between the backend's read and its write. + BeforeFirstReplace = t => t.ExternalMerge("LatestAgentV2Version", "2.0.1466"), + }; + + var result = await table.Sut.UpdateAdminConfigurationAsync( + c => { c.GlobalRateLimitRequestsPerMinute = 250; return null; }, "System (test)"); + + Assert.Null(result.Error); + Assert.Equal("2.0.1466", table.Current!.GetString("LatestAgentV2Version")); + Assert.Equal(250, table.Current.GetInt32("GlobalRateLimitRequestsPerMinute")); + Assert.Equal(1, table.Writes); + } + + [Fact] + public async Task A_mutation_that_changes_nothing_writes_nothing() + { + var table = new FakeAdminTable(ReleasedRow()); + + var result = await table.Sut.UpdateAdminConfigurationAsync( + c => { c.GlobalRateLimitRequestsPerMinute = 100; return null; }, "ga@operator.example"); + + Assert.Null(result.Error); + Assert.Empty(result.ChangedColumns); + Assert.Equal(0, table.Writes); + } + + [Fact] + public async Task A_rejected_mutation_writes_nothing() + { + var table = new FakeAdminTable(ReleasedRow()); + + var result = await table.Sut.UpdateAdminConfigurationAsync( + c => { c.GlobalRateLimitRequestsPerMinute = 999; return "nope"; }, "ga@operator.example"); + + Assert.Equal("nope", result.Error); + Assert.Equal(0, table.Writes); + Assert.Equal(100, table.Current!.GetInt32("GlobalRateLimitRequestsPerMinute")); + } + + [Fact] + public async Task A_null_value_drops_its_column() + { + var table = new FakeAdminTable(ReleasedRow()); + + var result = await table.Sut.UpdateAdminConfigurationAsync( + c => { c.NvdApiKey = null!; return null; }, "ga@operator.example"); // what a JSON null in a patch does + + Assert.Equal(new[] { "NvdApiKey" }, result.ChangedColumns); + Assert.False(table.Current!.ContainsKey("NvdApiKey")); + Assert.Equal(ReleaseExeSha, table.Current.GetString("LatestAgentV2ExeSha256")); + } + + [Fact] + public async Task Without_a_row_the_first_change_creates_it() + { + var table = new FakeAdminTable(current: null); + + var result = await table.Sut.UpdateAdminConfigurationAsync( + c => { c.EnforceClientAppBinding = true; return null; }, "ga@operator.example"); + + Assert.Null(result.Error); + Assert.True(table.Current!.GetBoolean("EnforceClientAppBinding")); + Assert.Equal(1, table.Writes); + } + + [Fact] + public async Task Default_creation_never_replaces_an_existing_row() + { + var table = new FakeAdminTable(ReleasedRow()); + + var created = await table.Sut.CreateAdminConfigurationIfMissingAsync(AdminConfiguration.CreateDefault()); + + Assert.False(created); + Assert.Equal(ReleaseVersion, table.Current!.GetString("LatestAgentV2Version")); + } + + [Theory] + [InlineData("latestAgentV2Version")] + [InlineData("LatestAgentV2Sha256")] + [InlineData("latestAgentV2ExeSha256")] + [InlineData("latestBootstrapV2ScriptVersion")] + [InlineData("updatedBy")] + [InlineData("lastUpdated")] + [InlineData("planTierDefinitionsJson")] + [InlineData("msrcLastSyncUtc")] + public void The_gate_rejects_pipeline_stamp_and_flow_owned_fields(string field) + { + var error = PatchAdminConfigurationFunction.CheckFields(new JObject { [field] = "x" }); + Assert.NotNull(error); + Assert.Contains("not writable", error); + } + + [Fact] + public void The_gate_rejects_unknown_fields_and_redacted_placeholders() + { + Assert.Contains("Unknown field", PatchAdminConfigurationFunction.CheckFields(new JObject { ["noSuchField"] = 1 })); + Assert.Contains("placeholder", PatchAdminConfigurationFunction.CheckFields( + new JObject { ["nvdApiKey"] = Constants.RedactedSecretPlaceholder })); + } + + [Fact] + public void Patchable_fields_are_the_persisted_settings() + { + var patchable = PatchAdminConfigurationFunction.PatchableFields; + Assert.Contains("EnforceClientAppBinding", patchable); + Assert.Contains("McpClientRegistrationEnabled", patchable); + Assert.Contains("McpAccessPolicy", patchable); + Assert.Contains("NvdApiKey", patchable); + Assert.Contains("OpsNotificationChannelsJson", patchable); + Assert.DoesNotContain("LatestAgentV2ExeSha256", patchable); + Assert.DoesNotContain("PartitionKey", patchable); + Assert.DoesNotContain("UpdatedBy", patchable); + } + + [Fact] + public void Applying_validates_the_result_without_echoing_values() + { + var draft = AdminConfiguration.CreateDefault(); + Assert.Contains("at least 1", PatchAdminConfigurationFunction.ApplyFields( + draft, JObject.Parse("{ \"globalRateLimitRequestsPerMinute\": 0 }"))); + + var typeError = PatchAdminConfigurationFunction.ApplyFields( + AdminConfiguration.CreateDefault(), JObject.Parse("{ \"collectorIdleTimeoutMinutes\": \"https://secret.example/sas?sig=abc\" }")); + Assert.NotNull(typeError); + Assert.DoesNotContain("secret.example", typeError); + } +} diff --git a/src/Backend/AutopilotMonitor.Functions.Tests/AdminConfigurationRateLimitSyncTests.cs b/src/Backend/AutopilotMonitor.Functions.Tests/AdminConfigurationRateLimitSyncTests.cs index f73c98a2..9a71bab8 100644 --- a/src/Backend/AutopilotMonitor.Functions.Tests/AdminConfigurationRateLimitSyncTests.cs +++ b/src/Backend/AutopilotMonitor.Functions.Tests/AdminConfigurationRateLimitSyncTests.cs @@ -14,7 +14,7 @@ namespace AutopilotMonitor.Functions.Tests; /// for the device path and UserRateLimitMiddleware for the user path). These tests pin the /// removal of the former background sync job: /// -/// Saving the admin config must NOT enumerate or write ANY tenant configuration. +/// Changing the admin config must NOT enumerate or write ANY tenant configuration. /// /// /// Historical context: the sync used to copy GlobalRateLimitRequestsPerMinute into every @@ -28,26 +28,33 @@ namespace AutopilotMonitor.Functions.Tests; public class AdminConfigurationRateLimitSyncTests { [Fact] - public async Task SaveConfigurationAsync_DoesNotEnumerateOrWriteTenantConfigs() + public async Task UpdateAsync_DoesNotEnumerateOrWriteTenantConfigs() { var repo = new Mock(MockBehavior.Strict); - repo.Setup(r => r.SaveAdminConfigurationAsync(It.IsAny())) - .ReturnsAsync(true); + repo.Setup(r => r.UpdateAdminConfigurationAsync( + It.IsAny>(), It.IsAny(), It.IsAny())) + .ReturnsAsync(new AdminConfigurationUpdateResult + { + Before = new AdminConfiguration(), + After = new AdminConfiguration(), + ChangedColumns = new[] { "GlobalRateLimitRequestsPerMinute", "UserRateLimitRequestsPerMinute" }, + }); var sut = new AdminConfigurationService( repo.Object, NullLogger.Instance, new MemoryCache(new MemoryCacheOptions())); - await sut.SaveConfigurationAsync(new AdminConfiguration + await sut.UpdateAsync(c => { - UpdatedBy = "global-admin@contoso.com", - GlobalRateLimitRequestsPerMinute = 200, - UserRateLimitRequestsPerMinute = 240, - }); + c.GlobalRateLimitRequestsPerMinute = 200; + c.UserRateLimitRequestsPerMinute = 240; + return null; + }, "global-admin@contoso.com"); // The admin config row is written... - repo.Verify(r => r.SaveAdminConfigurationAsync(It.IsAny()), Times.Once); + repo.Verify(r => r.UpdateAdminConfigurationAsync( + It.IsAny>(), "global-admin@contoso.com", It.IsAny()), Times.Once); // ...but NO tenant configuration is enumerated or mutated (sync removed). repo.Verify(r => r.GetAllTenantConfigurationsAsync(), Times.Never); repo.Verify(r => r.SaveTenantConfigurationAsync(It.IsAny()), Times.Never); diff --git a/src/Backend/AutopilotMonitor.Functions.Tests/ConfigBackupHookTests.cs b/src/Backend/AutopilotMonitor.Functions.Tests/ConfigBackupHookTests.cs index fdbf0942..8ff02423 100644 --- a/src/Backend/AutopilotMonitor.Functions.Tests/ConfigBackupHookTests.cs +++ b/src/Backend/AutopilotMonitor.Functions.Tests/ConfigBackupHookTests.cs @@ -56,6 +56,13 @@ public class ConfigBackupHookTests Upserts.Add(e); return Task.FromResult(Mock.Of()); }); + Table.Setup(c => c.UpdateEntityAsync( + It.IsAny(), It.IsAny(), It.IsAny(), It.IsAny())) + .Returns((e, _, _, _) => + { + Upserts.Add(e); + return Task.FromResult(Mock.Of()); + }); var serviceClient = new Mock(); serviceClient.Setup(c => c.GetTableClient(It.IsAny())).Returns(Table.Object); @@ -225,15 +232,11 @@ public class ConfigBackupHookTests .Callback((e, _) => snapshot = e) .Returns(Task.CompletedTask); - var incoming = new AdminConfiguration - { - UpdatedBy = "ga@operator.example", - GlobalRateLimitRequestsPerMinute = 200, - }; + var result = await harness.Sut.UpdateAdminConfigurationAsync( + c => { c.GlobalRateLimitRequestsPerMinute = 200; return null; }, "ga@operator.example"); - var saved = await harness.Sut.SaveAdminConfigurationAsync(incoming); - - Assert.True(saved); + Assert.Null(result.Error); + Assert.Equal(new[] { "GlobalRateLimitRequestsPerMinute" }, result.ChangedColumns); Assert.NotNull(snapshot); Assert.Equal("GlobalConfig", snapshot!.PartitionKey); Assert.Equal("admin-config", snapshot.Source); diff --git a/src/Backend/AutopilotMonitor.Functions.Tests/TypedRequestGuardTests.cs b/src/Backend/AutopilotMonitor.Functions.Tests/TypedRequestGuardTests.cs index 1413b6c5..a8b36247 100644 --- a/src/Backend/AutopilotMonitor.Functions.Tests/TypedRequestGuardTests.cs +++ b/src/Backend/AutopilotMonitor.Functions.Tests/TypedRequestGuardTests.cs @@ -61,7 +61,7 @@ public class TypedRequestGuardTests private static readonly Dictionary NewtonsoftBaseline = new(StringComparer.OrdinalIgnoreCase) { ["Functions/Config/PatchTenantConfigurationFieldsFunction.cs"] = 1, - ["Functions/Config/UpdateAdminConfigurationFunction.cs"] = 1, + ["Functions/Config/PatchAdminConfigurationFunction.cs"] = 1, ["Functions/Config/UpdateTenantConfigurationFunction.cs"] = 1, ["Functions/Rules/AnalyzeRulesFunction.cs"] = 2, ["Functions/Rules/DryRunAnalyzeRuleFunction.cs"] = 1, @@ -157,13 +157,21 @@ public class TypedRequestGuardTests var isObjectMap = slot.IsGenericType && slot.GetGenericTypeDefinition() == typeof(Dictionary<,>) && slot.GetGenericArguments()[1] == typeof(object); if (slot == typeof(object)) offenders.Add($"{t.Name}.{p.Name}: object slot (TypeScript gets unknown)"); - else if (isObjectMap && !(t == typeof(PatchTenantConfigurationFieldsRequest) && p.Name == nameof(PatchTenantConfigurationFieldsRequest.Fields))) - offenders.Add($"{t.Name}.{p.Name}: untyped map (only the tenant-config field patch carries one by design)"); + else if (isObjectMap && !IsFieldPatchMap(t, p)) + offenders.Add($"{t.Name}.{p.Name}: untyped map (only the config field patches carry one by design)"); } } Assert.True(offenders.Count == 0, "Request DTO shape violations:\n " + string.Join("\n ", offenders)); } + /// + /// The two field patches (tenant and admin configuration) carry a { field: value } map on purpose: + /// each value takes its field's own JSON type and the server gates the keys against the model. + /// + private static bool IsFieldPatchMap(Type type, PropertyInfo property) + => (type == typeof(PatchTenantConfigurationFieldsRequest) && property.Name == nameof(PatchTenantConfigurationFieldsRequest.Fields)) + || (type == typeof(PatchAdminConfigurationRequest) && property.Name == nameof(PatchAdminConfigurationRequest.Fields)); + private static IEnumerable<(string Relative, string Text)> HandlerSources() { var functionsRoot = Path.Combine(FindRepoRoot(), "src", "Backend", "AutopilotMonitor.Functions"); diff --git a/src/Backend/AutopilotMonitor.Functions/DataAccess/TableStorage/TableConfigRepository.cs b/src/Backend/AutopilotMonitor.Functions/DataAccess/TableStorage/TableConfigRepository.cs index 20450c55..e89920ea 100644 --- a/src/Backend/AutopilotMonitor.Functions/DataAccess/TableStorage/TableConfigRepository.cs +++ b/src/Backend/AutopilotMonitor.Functions/DataAccess/TableStorage/TableConfigRepository.cs @@ -327,23 +327,126 @@ namespace AutopilotMonitor.Functions.DataAccess.TableStorage } } - public async Task SaveAdminConfigurationAsync(AdminConfiguration config) - { - await TrySnapshotBeforeSaveAsync( - _adminConfigTableClient, "GlobalConfig", "config", - ConvertFromAdminTableEntity, config, AdminBackupNoiseProperties, - config.UpdatedBy, "admin-config", reason: null); + internal const int AdminUpdateMaxAttempts = 3; + /// Server stamps: written on every accepted change, never counted as one. + private static readonly HashSet AdminStampColumns = new(StringComparer.Ordinal) + { + "PartitionKey", "RowKey", "LastUpdated", "UpdatedBy", + }; + + /// + /// The one write path of the admin row (D-285). Each attempt reads the raw row fresh with + /// its ETag and materializes it twice — before and the draft the mutation edits — + /// so the copy is exact for every persisted column. The changed columns are computed from + /// the two converter projections and laid onto the freshly read raw entity (a null value + /// drops the column); the conditional replace then writes exactly those plus the stamps. + /// Every other column keeps its stored value: the release pipeline's LatestAgentV2*, + /// columns of a newer build, retired columns. The former full-model replace wrote every + /// column from whatever copy the caller held, so a portal page loaded before an agent + /// release reverted the release's hashes on save (2026-09-25). + /// + public async Task UpdateAdminConfigurationAsync( + Func mutate, string updatedBy, string? source = null) + { + if (mutate == null) throw new ArgumentNullException(nameof(mutate)); + + for (var attempt = 1; attempt <= AdminUpdateMaxAttempts; attempt++) + { + TableEntity? stored = null; + try + { + stored = (await _adminConfigTableClient.GetEntityAsync("GlobalConfig", "config")).Value; + } + catch (RequestFailedException ex) when (ex.Status == 404) + { + // No row yet: the first accepted change creates it from the defaults. + } + + var before = stored != null ? ConvertFromAdminTableEntity(stored) : AdminConfiguration.CreateDefault(); + var draft = stored != null ? ConvertFromAdminTableEntity(stored) : AdminConfiguration.CreateDefault(); + + var error = mutate(draft); + if (error != null) + return new AdminConfigurationUpdateResult { Error = error, Before = before, After = before }; + + var beforeColumns = ConvertToAdminTableEntity(before); + var draftColumns = ConvertToAdminTableEntity(draft); + var changed = ChangedAdminColumns(beforeColumns, draftColumns); + if (changed.Count == 0) + return new AdminConfigurationUpdateResult { Before = before, After = before }; + + draft.UpdatedBy = updatedBy; + draft.LastUpdated = DateTime.UtcNow; + + await TrySnapshotBeforeSaveAsync( + _adminConfigTableClient, "GlobalConfig", "config", + ConvertFromAdminTableEntity, draft, AdminBackupNoiseProperties, + updatedBy, source ?? "admin-config", reason: null); + + try + { + if (stored == null) + { + await _adminConfigTableClient.AddEntityAsync(ConvertToAdminTableEntity(draft)); + } + else + { + foreach (var column in changed) + { + var value = draftColumns[column]; + if (value == null) stored.Remove(column); + else stored[column] = value; + } + stored["LastUpdated"] = draft.LastUpdated; + stored["UpdatedBy"] = draft.UpdatedBy; + await _adminConfigTableClient.UpdateEntityAsync(stored, stored.ETag, TableUpdateMode.Replace); + } + + return new AdminConfigurationUpdateResult { Before = before, After = draft, ChangedColumns = changed }; + } + catch (RequestFailedException ex) when (ex.Status == 412 || ex.Status == 409) + { + // Someone wrote (or created) the row since this attempt's read: re-read and + // re-apply the mutation onto their state instead of overwriting it. + _logger.LogInformation( + "Admin configuration changed concurrently (attempt {Attempt}/{Max}) — re-reading", + attempt, AdminUpdateMaxAttempts); + await Task.Delay(TimeSpan.FromMilliseconds(150)); + } + } + + throw new InvalidOperationException( + $"Admin configuration update lost {AdminUpdateMaxAttempts} conditional-write races — retry shortly."); + } + + /// + /// Columns whose projected value differs between the two converter outputs. Both sides come + /// from , so value types match and plain equality holds. + /// + internal static List ChangedAdminColumns(TableEntity before, TableEntity after) + { + var changed = new List(); + foreach (var column in after.Keys.Union(before.Keys, StringComparer.Ordinal)) + { + if (AdminStampColumns.Contains(column)) continue; + before.TryGetValue(column, out var b); + after.TryGetValue(column, out var a); + if (!Equals(b, a)) changed.Add(column); + } + return changed; + } + + public async Task CreateAdminConfigurationIfMissingAsync(AdminConfiguration config) + { try { - var entity = ConvertToAdminTableEntity(config); - await _adminConfigTableClient.UpsertEntityAsync(entity, TableUpdateMode.Replace); + await _adminConfigTableClient.AddEntityAsync(ConvertToAdminTableEntity(config)); return true; } - catch (Exception ex) + catch (RequestFailedException ex) when (ex.Status == 409) { - _logger.LogError(ex, "Error saving admin configuration"); - return false; + return false; // A row exists — never replace it with defaults. } } @@ -931,10 +1034,11 @@ namespace AutopilotMonitor.Functions.DataAccess.TableStorage { "OpsAlertSlackEnabled", config.OpsAlertSlackEnabled }, { "OpsAlertSlackWebhookUrl", config.OpsAlertSlackWebhookUrl ?? string.Empty }, { "OpsNotificationChannelsJson", config.OpsNotificationChannelsJson ?? string.Empty }, - // Per-line agent binary integrity (written by build scripts via Merge). + // Per-line agent binary integrity: written by the release pipeline via Merge. + // The backend never changes them (the PATCH gate rejects them), so the + // changed-columns write (UpdateAdminConfigurationAsync) never touches them. // V2 is the only wired line; future V3 = add field set here. Retired columns - // (V1-suffix and the even older unsuffixed "LatestAgent*") are never written, - // so the next Save evicts them implicitly on overwrite. + // (V1-suffix and the even older unsuffixed "LatestAgent*") are not projected. { "AllowAgentDowngrade", config.AllowAgentDowngrade }, { "LatestAgentV2Version", config.LatestAgentV2Version ?? string.Empty }, { "LatestAgentV2Sha256", config.LatestAgentV2Sha256 ?? string.Empty }, diff --git a/src/Backend/AutopilotMonitor.Functions/Functions/Config/PatchAdminConfigurationFunction.cs b/src/Backend/AutopilotMonitor.Functions/Functions/Config/PatchAdminConfigurationFunction.cs new file mode 100644 index 00000000..d5a42c1c --- /dev/null +++ b/src/Backend/AutopilotMonitor.Functions/Functions/Config/PatchAdminConfigurationFunction.cs @@ -0,0 +1,223 @@ +using System; +using System.Collections.Generic; +using System.Linq; +using System.Net; +using System.Reflection; +using System.Threading.Tasks; +using AutopilotMonitor.Functions.DataAccess.TableStorage; +using AutopilotMonitor.Functions.Helpers; +using AutopilotMonitor.Functions.Services; +using AutopilotMonitor.Shared; +using AutopilotMonitor.Shared.DataAccess; +using AutopilotMonitor.Shared.Models; +using AutopilotMonitor.Shared.Models.Notifications; +using Microsoft.Azure.Functions.Worker; +using Microsoft.Azure.Functions.Worker.Http; +using Microsoft.Extensions.Logging; +using Newtonsoft.Json; +using Newtonsoft.Json.Linq; + +namespace AutopilotMonitor.Functions.Functions.Config +{ + /// + /// PATCH global/config — the portal's only write to the platform configuration (D-285). The body + /// names just the fields the operator changed; the repository re-reads the row, applies them and + /// writes only the columns whose value actually changed. It replaced a full-model PUT that wrote + /// every column from the page's copy, so a page loaded before an agent release reverted the + /// release's hashes on save (2026-09-25). + /// + public class PatchAdminConfigurationFunction + { + private const int MaxBodyBytes = 1_048_576; + + private readonly ILogger _logger; + private readonly AdminConfigurationService _adminConfigService; + private readonly IMaintenanceRepository _maintenanceRepo; + + public PatchAdminConfigurationFunction( + ILogger logger, + AdminConfigurationService adminConfigService, + IMaintenanceRepository maintenanceRepo) + { + _logger = logger; + _adminConfigService = adminConfigService; + _maintenanceRepo = maintenanceRepo; + } + + /// + /// Fields no patch may write: the server stamps, the columns only the agent release pipeline + /// writes (build-agent.yml / publish-scripts.yml), and the ones owned by their own flow + /// (plan-tier endpoint, vulnerability-sync bookkeeping). + /// + internal static readonly HashSet DeniedFields = new(StringComparer.OrdinalIgnoreCase) + { + "LastUpdated", "UpdatedBy", + "LatestAgentV2Version", "LatestAgentV2Sha256", "LatestAgentV2ExeSha256", "LatestBootstrapV2ScriptVersion", + "PlanTierDefinitionsJson", + "VulnerabilityDataLastSyncUtc", "MsrcLastSyncUtc", "NvdCacheLastRefreshUtc", "EpssLastSyncUtc", + }; + + /// + /// Every persisted, writable AdminConfiguration property except . + /// Derived from the entity converter, so a new column is patchable the moment it is persisted + /// and a model property that is never stored cannot be "saved" into nothing. + /// + internal static readonly IReadOnlyCollection PatchableFields = BuildPatchableFields(); + + private static HashSet BuildPatchableFields() + { + var writable = new HashSet( + typeof(AdminConfiguration) + .GetProperties(BindingFlags.Public | BindingFlags.Instance) + .Where(p => p.CanWrite && p.GetIndexParameters().Length == 0) + .Select(p => p.Name), + StringComparer.OrdinalIgnoreCase); + + return new HashSet( + TableConfigRepository.ConvertToAdminTableEntity(AdminConfiguration.CreateDefault()).Keys + .Where(c => c != "PartitionKey" && c != "RowKey" && writable.Contains(c) && !DeniedFields.Contains(c)), + StringComparer.OrdinalIgnoreCase); + } + + [Function("PatchAdminConfiguration")] + public async Task Run( + [HttpTrigger(AuthorizationLevel.Anonymous, "patch", Route = "global/config")] HttpRequestData req) + { + try + { + // Authentication + GlobalAdminOnly authorization enforced by PolicyEnforcementMiddleware + string userIdentifier = TenantHelper.GetUserIdentifier(req); + + // Newtonsoft on purpose: the field map is applied with JsonConvert.PopulateObject, like the + // tenant field patch (TypedRequestGuardTests baseline). + var read = await req.ReadNewtonsoftAsync(MaxBodyBytes); + if (read.Error != null) return read.Error; + var request = read.Value!; + if (request.Fields == null || request.Fields.Count == 0) + { + return await req.BadRequestAsync( + "Body must be { \"fields\": { : , ... } } with at least one field."); + } + + var fields = JObject.FromObject(request.Fields); + var gateError = CheckFields(fields); + if (gateError != null) return await req.BadRequestAsync(gateError); + + var result = await _adminConfigService.UpdateAsync( + draft => ApplyFields(draft, fields), userIdentifier, "portal"); + if (result.Error != null) return await req.BadRequestAsync(result.Error); + + _logger.LogInformation( + "PatchAdminConfiguration by {User}: {Count} field(s) sent, changed: {Columns}", + userIdentifier, fields.Count, string.Join(", ", result.ChangedColumns)); + + if (result.ChangedColumns.Count > 0) + { + await _maintenanceRepo.LogAuditEntryAsync( + Constants.AuditGlobalTenantId, + "UPDATE", + "AdminConfiguration", + "GlobalConfig", + userIdentifier, + ConfigDiffHelper.GetChanges(result.Before, result.After)); + } + + var response = req.CreateResponse(HttpStatusCode.OK); + await response.WriteAsJsonAsync(new UpdateAdminConfigurationResponse + { + Success = true, + Message = result.ChangedColumns.Count > 0 + ? "Admin configuration updated successfully" + : "No changes — nothing was written", + Config = result.After, + }); + return response; + } + catch (Exception ex) + { + return await req.InternalServerErrorAsync(_logger, ex, "PatchAdminConfiguration"); + } + } + + /// + /// The field gate, before any storage work: every key must be a patchable field, named in the + /// error when it is not so the caller can correct itself. + /// + internal static string? CheckFields(JObject fields) + { + foreach (var prop in fields.Properties()) + { + if (DeniedFields.Contains(prop.Name)) + { + return $"Field \"{prop.Name}\" is not writable here (server stamp, owned by the agent release " + + "pipeline, or written by its own flow)."; + } + if (!PatchableFields.Contains(prop.Name)) + return $"Unknown field \"{prop.Name}\"."; + + // The ops channel list carries per-channel redacted destinations that are restored below; + // anywhere else the placeholder would overwrite a real secret with the sentinel. + if (!prop.Name.Equals(nameof(AdminConfiguration.OpsNotificationChannelsJson), StringComparison.OrdinalIgnoreCase) + && prop.Value is JValue { Type: JTokenType.String, Value: string text } + && text.Contains(Constants.RedactedSecretPlaceholder, StringComparison.Ordinal)) + { + return $"Field \"{prop.Name}\" carries the \"{Constants.RedactedSecretPlaceholder}\" placeholder — " + + "a redacted read must never be written back."; + } + } + return null; + } + + /// + /// Applies the gated fields onto the repository's fresh draft and validates the result. JSON + /// null clears a nullable field; camelCase keys bind case-insensitively to the model. + /// + internal static string? ApplyFields(AdminConfiguration draft, JObject fields) + { + var storedChannels = draft.OpsNotificationChannelsJson; + try + { + JsonConvert.PopulateObject(fields.ToString(), draft, new JsonSerializerSettings + { + MissingMemberHandling = MissingMemberHandling.Error, + }); + } + catch (JsonException ex) + { + // Never ex.Message: Newtonsoft quotes the offending VALUE, which may be a secret. + var path = (ex as JsonSerializationException)?.Path ?? (ex as JsonReaderException)?.Path; + return string.IsNullOrEmpty(path) + ? "A value has the wrong JSON type for its field." + : $"The value for \"{path}\" has the wrong JSON type."; + } + + // Rate limits must be positive: a zero/negative value would throttle every request + // (RateLimitService clamps as a last resort, but reject at the edge for a clear error). + var rateLimitError = + draft.GlobalRateLimitRequestsPerMinute < 1 ? "Global Device API Rate Limit" : + draft.UserRateLimitRequestsPerMinute < 1 ? "MCP & Integrations API Rate Limit" : + draft.PortalUserRateLimitRequestsPerMinute < 1 ? "Portal User API Rate Limit" : + draft.GlobalAdminRateLimitRequestsPerMinute < 1 ? "Global Admin API Rate Limit" : + null; + if (rateLimitError != null) + return $"{rateLimitError} must be at least 1 request per minute."; + + if (fields.Properties().Any(p => p.Name.Equals( + nameof(AdminConfiguration.OpsNotificationChannelsJson), StringComparison.OrdinalIgnoreCase))) + { + // A GlobalReader is served destinations as the ***REDACTED*** sentinel; persisting it as a + // chat ID or webhook URL would silently break every alert, so restore per channel id. + draft.OpsNotificationChannelsJson = NotificationChannel.RestoreRedactedList( + draft.OpsNotificationChannelsJson, storedChannels); + + // Same structural gate the tenant channel list passes. No GA gate on Telegram here: + // this endpoint is GlobalAdminOnly by policy. + var channelsError = TenantConfigValidation.ValidateNotificationChannels(draft.OpsNotificationChannelsJson); + if (channelsError != null) + return $"Invalid ops notification channels: {channelsError}"; + } + + return null; + } + } +} diff --git a/src/Backend/AutopilotMonitor.Functions/Functions/Config/PlanManagementFunction.cs b/src/Backend/AutopilotMonitor.Functions/Functions/Config/PlanManagementFunction.cs index 2afc44d0..e3ba4984 100644 --- a/src/Backend/AutopilotMonitor.Functions/Functions/Config/PlanManagementFunction.cs +++ b/src/Backend/AutopilotMonitor.Functions/Functions/Config/PlanManagementFunction.cs @@ -602,9 +602,10 @@ namespace AutopilotMonitor.Functions.Functions.Config foreach (var tier in body.Tiers) tier.Name = tier.Name.ToLowerInvariant(); - var config = await _adminConfigService.GetConfigurationAsync(); - config.PlanTierDefinitionsJson = JsonSerializer.Serialize(body.Tiers); - await _adminConfigService.SaveConfigurationAsync(config); + var tiersJson = JsonSerializer.Serialize(body.Tiers); + await _adminConfigService.UpdateAsync( + c => { c.PlanTierDefinitionsJson = tiersJson; return null; }, + TenantHelper.GetUserIdentifier(req), "plan-tiers"); var response = req.CreateResponse(HttpStatusCode.OK); await response.WriteAsJsonAsync(new PlanTierDefinitionsResponse { Tiers = body.Tiers, Catalog = BuildUsagePlanCatalog() }); diff --git a/src/Backend/AutopilotMonitor.Functions/Functions/Config/UpdateAdminConfigurationFunction.cs b/src/Backend/AutopilotMonitor.Functions/Functions/Config/UpdateAdminConfigurationFunction.cs deleted file mode 100644 index d58e1586..00000000 --- a/src/Backend/AutopilotMonitor.Functions/Functions/Config/UpdateAdminConfigurationFunction.cs +++ /dev/null @@ -1,118 +0,0 @@ -using System; -using System.IO; -using System.Net; -using System.Threading.Tasks; -using System.Collections.Generic; -using AutopilotMonitor.Functions.Helpers; -using AutopilotMonitor.Functions.Services; -using AutopilotMonitor.Shared.DataAccess; -using AutopilotMonitor.Shared.Models; -using AutopilotMonitor.Shared.Models.Notifications; -using Microsoft.Azure.Functions.Worker; -using Microsoft.Azure.Functions.Worker.Http; -using Microsoft.Extensions.Logging; - -namespace AutopilotMonitor.Functions.Functions.Config -{ - public class UpdateAdminConfigurationFunction - { - private readonly ILogger _logger; - private readonly AdminConfigurationService _adminConfigService; - private readonly IMaintenanceRepository _maintenanceRepo; - - public UpdateAdminConfigurationFunction( - ILogger logger, - AdminConfigurationService adminConfigService, - IMaintenanceRepository maintenanceRepo) - { - _logger = logger; - _adminConfigService = adminConfigService; - _maintenanceRepo = maintenanceRepo; - } - - [Function("UpdateAdminConfiguration")] - public async Task Run( - [HttpTrigger(AuthorizationLevel.Anonymous, "put", "post", Route = "global/config")] HttpRequestData req) - { - try - { - // Authentication + GlobalAdminOnly authorization enforced by PolicyEnforcementMiddleware - string userIdentifier = TenantHelper.GetUserIdentifier(req); - - _logger.LogInformation($"UpdateAdminConfiguration by Global Admin user {userIdentifier}"); - - // Parse request body - // Newtonsoft on purpose: same configuration stack as the tenant PUT — TypedRequestGuardTests baseline. - var read = await req.ReadNewtonsoftAsync(1_048_576); - if (read.Error != null) return read.Error; - var config = read.Value!; - - // Rate limits must be positive: a zero/negative value would throttle every request - // (RateLimitService clamps as a last resort, but reject at the edge for a clear error). - var rateLimitError = - config.GlobalRateLimitRequestsPerMinute < 1 ? "Global Device API Rate Limit" : - config.UserRateLimitRequestsPerMinute < 1 ? "MCP & Integrations API Rate Limit" : - config.PortalUserRateLimitRequestsPerMinute < 1 ? "Portal User API Rate Limit" : - config.GlobalAdminRateLimitRequestsPerMinute < 1 ? "Global Admin API Rate Limit" : - null; - if (rateLimitError != null) - { - return await req.BadRequestAsync($"{rateLimitError} must be at least 1 request per minute."); - } - - // Set the actual user identifier for audit logging - config.UpdatedBy = userIdentifier; - - // Ensure PartitionKey and RowKey are correct - config.PartitionKey = "GlobalConfig"; - config.RowKey = "config"; - - // Load existing config for diff before saving - var existingConfig = await _adminConfigService.GetConfigurationAsync(); - - // Ops channels: a GlobalReader is served a redacted copy (destinations replaced - // with the ***REDACTED*** sentinel). Only a GA can reach this endpoint, so this is - // defense-in-depth — but persisting the literal sentinel as a chat ID or webhook - // URL would silently break every alert, so restore per channel id first. - config.OpsNotificationChannelsJson = NotificationChannel.RestoreRedactedList( - config.OpsNotificationChannelsJson, existingConfig.OpsNotificationChannelsJson); - - // Same structural gate the tenant channel list passes (ids, provider types, - // SSRF-checked URLs / Telegram chat IDs, header and signing-secret shape). No GA - // gate on Telegram here: this endpoint is GlobalAdminOnly by policy. - var channelsError = TenantConfigValidation.ValidateNotificationChannels(config.OpsNotificationChannelsJson); - if (channelsError != null) - { - return await req.BadRequestAsync($"Invalid ops notification channels: {channelsError}"); - } - - var changes = ConfigDiffHelper.GetChanges(existingConfig, config); - - // Save configuration - await _adminConfigService.SaveConfigurationAsync(config); - - await _maintenanceRepo.LogAuditEntryAsync( - AutopilotMonitor.Shared.Constants.AuditGlobalTenantId, - "UPDATE", - "AdminConfiguration", - "GlobalConfig", - userIdentifier, - changes.Count > 0 ? changes : null - ); - - var response = req.CreateResponse(HttpStatusCode.OK); - await response.WriteAsJsonAsync(new UpdateAdminConfigurationResponse - { - Success = true, - Message = "Admin configuration updated successfully", - Config = config - }); - return response; - } - catch (Exception ex) - { - return await req.InternalServerErrorAsync(_logger, ex, "UpdateAdminConfiguration"); - } - } - } -} diff --git a/src/Backend/AutopilotMonitor.Functions/Functions/Vulnerability/TriggerMsrcSyncFunction.cs b/src/Backend/AutopilotMonitor.Functions/Functions/Vulnerability/TriggerMsrcSyncFunction.cs index 2daf5076..686ca52c 100644 --- a/src/Backend/AutopilotMonitor.Functions/Functions/Vulnerability/TriggerMsrcSyncFunction.cs +++ b/src/Backend/AutopilotMonitor.Functions/Functions/Vulnerability/TriggerMsrcSyncFunction.cs @@ -49,12 +49,10 @@ namespace AutopilotMonitor.Functions.Functions.Vulnerability if (string.IsNullOrEmpty(msrc.Error)) { - var config = await _adminConfigService.GetConfigurationAsync(); - if (config != null) - { - config.MsrcLastSyncUtc = DateTime.UtcNow.ToString("o"); - await _adminConfigService.SaveConfigurationAsync(config); - } + var syncedAt = DateTime.UtcNow.ToString("o"); + await _adminConfigService.UpdateAsync( + c => { c.MsrcLastSyncUtc = syncedAt; return null; }, + "System (MSRC sync)", "vuln-sync"); } var durationMs = (int)(DateTime.UtcNow - startedAt).TotalMilliseconds; diff --git a/src/Backend/AutopilotMonitor.Functions/Functions/Vulnerability/TriggerVulnerabilityDataSyncFunction.cs b/src/Backend/AutopilotMonitor.Functions/Functions/Vulnerability/TriggerVulnerabilityDataSyncFunction.cs index db5c0f50..87137940 100644 --- a/src/Backend/AutopilotMonitor.Functions/Functions/Vulnerability/TriggerVulnerabilityDataSyncFunction.cs +++ b/src/Backend/AutopilotMonitor.Functions/Functions/Vulnerability/TriggerVulnerabilityDataSyncFunction.cs @@ -97,12 +97,10 @@ namespace AutopilotMonitor.Functions.Functions.Vulnerability } // Update sync timestamp - var config = await _adminConfigService.GetConfigurationAsync(); - if (config != null) - { - config.VulnerabilityDataLastSyncUtc = DateTime.UtcNow.ToString("o"); - await _adminConfigService.SaveConfigurationAsync(config); - } + var syncedAt = DateTime.UtcNow.ToString("o"); + await _adminConfigService.UpdateAsync( + c => { c.VulnerabilityDataLastSyncUtc = syncedAt; return null; }, + "System (vulnerability sync)", "vuln-sync"); var response = req.CreateResponse(HttpStatusCode.OK); await response.WriteAsJsonAsync(new TriggerVulnerabilityDataSyncResponse diff --git a/src/Backend/AutopilotMonitor.Functions/Functions/Vulnerability/VulnerabilityDataSyncFunction.cs b/src/Backend/AutopilotMonitor.Functions/Functions/Vulnerability/VulnerabilityDataSyncFunction.cs index 5c7afa3b..a326fa6a 100644 --- a/src/Backend/AutopilotMonitor.Functions/Functions/Vulnerability/VulnerabilityDataSyncFunction.cs +++ b/src/Backend/AutopilotMonitor.Functions/Functions/Vulnerability/VulnerabilityDataSyncFunction.cs @@ -117,22 +117,14 @@ namespace AutopilotMonitor.Functions.Functions.Vulnerability // "Last KEV/MSRC Sync" accurately. (NVD/EPSS stamps are written by the service.) try { - var config = await _adminConfigService.GetConfigurationAsync(); - if (config != null) + if (kevSyncedAt != null || msrcSyncedAt != null) { - bool dirty = false; - if (kevSyncedAt != null) + await _adminConfigService.UpdateAsync(c => { - config.VulnerabilityDataLastSyncUtc = kevSyncedAt; - dirty = true; - } - if (msrcSyncedAt != null) - { - config.MsrcLastSyncUtc = msrcSyncedAt; - dirty = true; - } - if (dirty) - await _adminConfigService.SaveConfigurationAsync(config); + if (kevSyncedAt != null) c.VulnerabilityDataLastSyncUtc = kevSyncedAt; + if (msrcSyncedAt != null) c.MsrcLastSyncUtc = msrcSyncedAt; + return null; + }, "System (vulnerability sync)", "vuln-sync"); } } catch (Exception ex) diff --git a/src/Backend/AutopilotMonitor.Functions/Security/EndpointAccessPolicyCatalog.cs b/src/Backend/AutopilotMonitor.Functions/Security/EndpointAccessPolicyCatalog.cs index 1d0c346e..b52f60cb 100644 --- a/src/Backend/AutopilotMonitor.Functions/Security/EndpointAccessPolicyCatalog.cs +++ b/src/Backend/AutopilotMonitor.Functions/Security/EndpointAccessPolicyCatalog.cs @@ -479,8 +479,8 @@ public static class EndpointAccessPolicyCatalog // ?tenantId= path — never the no-tenantId aggregate (which the delegated-rescue cannot satisfy). new("GET", "global/metrics/summary", EndpointPolicy.GlobalReadOrAdmin, TenantScoping.QueryParam), new("GET", "global/config", EndpointPolicy.GlobalReadOrAdmin), - new("PUT", "global/config", EndpointPolicy.GlobalAdminOnly), - new("POST", "global/config", EndpointPolicy.GlobalAdminOnly), + // Changed fields only (D-285); the former full-model PUT/POST is gone. + new("PATCH", "global/config", EndpointPolicy.GlobalAdminOnly), new("POST", "global/config/test-ops-channel", EndpointPolicy.GlobalAdminOnly), new("GET", "config/all", EndpointPolicy.GlobalReadOrDelegatedSubset), // Transactional field-level config writes + snapshot list/revert (MCP + web Settings). diff --git a/src/Backend/AutopilotMonitor.Functions/Services/AdminConfigurationService.cs b/src/Backend/AutopilotMonitor.Functions/Services/AdminConfigurationService.cs index 09bb1d22..27433dc1 100644 --- a/src/Backend/AutopilotMonitor.Functions/Services/AdminConfigurationService.cs +++ b/src/Backend/AutopilotMonitor.Functions/Services/AdminConfigurationService.cs @@ -78,8 +78,8 @@ namespace AutopilotMonitor.Functions.Services try { - // Save the default configuration via repository - await _configRepo.SaveAdminConfigurationAsync(defaultConfig); + // Create-only: a concurrent first write (or a row this read missed) is never replaced. + await _configRepo.CreateAdminConfigurationIfMissingAsync(defaultConfig); _cache.Set(CacheKey, defaultConfig, CacheDuration); @@ -191,38 +191,24 @@ namespace AutopilotMonitor.Functions.Services } /// - /// Saves global admin configuration and syncs rate limit to all tenant configurations + /// Changes the admin configuration through the repository's changed-columns write (D-285): + /// fresh read, on a copy, only the columns it changed are written. + /// Never pass a configuration object around to save it — a cached or page-loaded copy is + /// exactly what used to revert other writers' changes. Invalidates this instance's cache + /// after a write; other instances catch up within the 5-minute TTL. /// - public async Task SaveConfigurationAsync(AdminConfiguration config) + public virtual async Task UpdateAsync( + Func mutate, string updatedBy, string? source = null) { - if (config == null) + var result = await _configRepo.UpdateAdminConfigurationAsync(mutate, updatedBy, source); + if (result.Error == null && result.ChangedColumns.Count > 0) { - throw new ArgumentException("Configuration is required"); - } - - try - { - config.LastUpdated = DateTime.UtcNow; - - await _configRepo.SaveAdminConfigurationAsync(config); - - // Invalidate cache _cache.Remove(CacheKey); - - _logger.LogInformation($"Admin configuration saved by {config.UpdatedBy}"); - - // NOTE: Rate limits are no longer mirrored into every tenant row. The effective - // per-tenant limit is computed at read time as `tenantOverride ?? global` - // (see SecurityValidator for the device path and UserRateLimitMiddleware for the - // user path). This removes the former background sync job — and the clobbering - // foot-gun where a per-tenant edit to the base field was overwritten on the next - // global save. - } - catch (Exception ex) - { - _logger.LogError(ex, "Error saving admin configuration"); - throw; + _logger.LogInformation( + "Admin configuration changed by {UpdatedBy}: {Columns}", + updatedBy, string.Join(", ", result.ChangedColumns)); } + return result; } /// diff --git a/src/Backend/AutopilotMonitor.Functions/Services/TableStorageService.Maintenance.cs b/src/Backend/AutopilotMonitor.Functions/Services/TableStorageService.Maintenance.cs index d29dd85e..12e3ec0e 100644 --- a/src/Backend/AutopilotMonitor.Functions/Services/TableStorageService.Maintenance.cs +++ b/src/Backend/AutopilotMonitor.Functions/Services/TableStorageService.Maintenance.cs @@ -281,7 +281,7 @@ namespace AutopilotMonitor.Functions.Services // Tenants come from TenantConfiguration (1 row per tenant — cheap). // PLUS the synthetic global-tenant partition (Constants.AuditGlobalTenantId) // where platform-action audits are written from TenantOffboardFunction, - // VersionBlockFunction, UpdateAdminConfigurationFunction, … TenantConfiguration + // VersionBlockFunction, PatchAdminConfigurationFunction, … TenantConfiguration // has no row for it because it's a virtual partition, so it'd be // silently skipped without this explicit add. var tenantIds = new HashSet(StringComparer.OrdinalIgnoreCase) diff --git a/src/Backend/AutopilotMonitor.Functions/Services/Vulnerability/VulnerabilityCacheRefreshService.cs b/src/Backend/AutopilotMonitor.Functions/Services/Vulnerability/VulnerabilityCacheRefreshService.cs index 51e63d2b..1a565f44 100644 --- a/src/Backend/AutopilotMonitor.Functions/Services/Vulnerability/VulnerabilityCacheRefreshService.cs +++ b/src/Backend/AutopilotMonitor.Functions/Services/Vulnerability/VulnerabilityCacheRefreshService.cs @@ -314,10 +314,8 @@ namespace AutopilotMonitor.Functions.Services.Vulnerability { try { - var config = await _adminConfigService.GetConfigurationAsync(); - if (config == null) return; - set(config); - await _adminConfigService.SaveConfigurationAsync(config); + await _adminConfigService.UpdateAsync( + c => { set(c); return null; }, "System (vulnerability sync)", "vuln-sync"); } catch (Exception ex) { diff --git a/src/McpServer/autopilot-monitor-mcp/src/generated/wire-types.generated.ts b/src/McpServer/autopilot-monitor-mcp/src/generated/wire-types.generated.ts index 7a35c591..07ef0ec1 100644 --- a/src/McpServer/autopilot-monitor-mcp/src/generated/wire-types.generated.ts +++ b/src/McpServer/autopilot-monitor-mcp/src/generated/wire-types.generated.ts @@ -3029,6 +3029,11 @@ export interface OpsEventListResponse { nextLink?: string; } +/** Body of PATCH global/config: { "fields": { : , ... } } with at least one field, keyed by the AdminConfiguration wire name. Only the fields sent are written; values take the field's own JSON type. */ +export interface PatchAdminConfigurationRequest { + fields?: Record; +} + /** Body of PATCH config/{tenantId}/fields: { "fields": { : , ... }, "reason"?: "..." } with at least one field. Values take the field's own JSON type (TenantConfiguration). */ export interface PatchTenantConfigurationFieldsRequest { fields?: Record; @@ -5291,7 +5296,7 @@ export interface UnmatchedSoftwareItem { normalizedVersion?: string; } -/** Response of PUT global/config: acknowledgement plus the stored admin configuration. */ +/** Response of PATCH global/config: acknowledgement plus the stored admin configuration. */ export interface UpdateAdminConfigurationResponse { success: boolean; message: string; diff --git a/src/Shared/AutopilotMonitor.Shared/DataAccess/AdminConfigurationUpdateResult.cs b/src/Shared/AutopilotMonitor.Shared/DataAccess/AdminConfigurationUpdateResult.cs new file mode 100644 index 00000000..44c0d9ff --- /dev/null +++ b/src/Shared/AutopilotMonitor.Shared/DataAccess/AdminConfigurationUpdateResult.cs @@ -0,0 +1,26 @@ +using System; +using System.Collections.Generic; +using AutopilotMonitor.Shared.Models; + +namespace AutopilotMonitor.Shared.DataAccess +{ + /// + /// Outcome of : either the mutation + /// rejected the change ( set, nothing written) or it was accepted, in which case + /// names the columns actually written (empty = no-op, no write). + /// + public sealed class AdminConfigurationUpdateResult + { + /// The mutation's rejection message; null when the change was accepted. + public string? Error { get; set; } + + /// The stored configuration the mutation started from (fresh read). + public AdminConfiguration Before { get; set; } = default!; + + /// The configuration after the mutation — what the row holds now when accepted. + public AdminConfiguration After { get; set; } = default!; + + /// Columns written, server stamps excluded. Empty when the mutation changed nothing. + public IReadOnlyCollection ChangedColumns { get; set; } = Array.Empty(); + } +} diff --git a/src/Shared/AutopilotMonitor.Shared/DataAccess/IConfigRepository.cs b/src/Shared/AutopilotMonitor.Shared/DataAccess/IConfigRepository.cs index 12235450..4294f82c 100644 --- a/src/Shared/AutopilotMonitor.Shared/DataAccess/IConfigRepository.cs +++ b/src/Shared/AutopilotMonitor.Shared/DataAccess/IConfigRepository.cs @@ -66,7 +66,24 @@ namespace AutopilotMonitor.Shared.DataAccess // --- Admin Configuration --- Task GetAdminConfigurationAsync(); - Task SaveAdminConfigurationAsync(AdminConfiguration config); + + /// + /// The one write path of the admin configuration row. It reads the row fresh, runs + /// on a copy and writes ONLY the columns whose value the mutation + /// changed, conditionally on the row's ETag (re-read and retry on a concurrent write). + /// Every other column keeps its stored value, including the ones only the agent release + /// pipeline writes (LatestAgentV2*) and columns this build does not know. A mutation + /// that changes nothing writes nothing. returns an error message + /// to reject the change (nothing is written) or null to accept it. Storage failures throw. + /// + Task UpdateAdminConfigurationAsync( + Func mutate, string updatedBy, string? source = null); + + /// + /// Creates the row from when none exists yet; never replaces an + /// existing row. True when this call created it. + /// + Task CreateAdminConfigurationIfMissingAsync(AdminConfiguration config); // --- Preview Whitelist --- Task IsInPreviewWhitelistAsync(string tenantId); diff --git a/src/Shared/AutopilotMonitor.Shared/Models/ConfigApiModels.cs b/src/Shared/AutopilotMonitor.Shared/Models/ConfigApiModels.cs index 0e32f8c3..ee77d384 100644 --- a/src/Shared/AutopilotMonitor.Shared/Models/ConfigApiModels.cs +++ b/src/Shared/AutopilotMonitor.Shared/Models/ConfigApiModels.cs @@ -320,7 +320,7 @@ namespace AutopilotMonitor.Shared.Models } /// - /// Response of PUT global/config: acknowledgement plus the stored admin configuration. + /// Response of PATCH global/config: acknowledgement plus the stored admin configuration. /// // Declaration order == wire order. public class UpdateAdminConfigurationResponse : IApiResponse @@ -360,6 +360,16 @@ namespace AutopilotMonitor.Shared.Models public string? Reason { get; set; } } + /// + /// Body of PATCH global/config: { "fields": { <fieldName>: <value>, ... } } with at least one + /// field, keyed by the AdminConfiguration wire name. Only the fields sent are written; values take + /// the field's own JSON type. + /// + public class PatchAdminConfigurationRequest : IApiRequest + { + public Dictionary? Fields { get; set; } + } + /// Body of POST config/{tenantId}/revert. public class RevertTenantConfigurationRequest : IApiRequest { diff --git a/src/Web/autopilot-monitor-web/utils/shared-manifests.json b/src/Web/autopilot-monitor-web/utils/shared-manifests.json index a8aad587..fa9f547d 100644 --- a/src/Web/autopilot-monitor-web/utils/shared-manifests.json +++ b/src/Web/autopilot-monitor-web/utils/shared-manifests.json @@ -14239,6 +14239,23 @@ } ] }, + "PatchAdminConfigurationRequest": { + "kind": "object", + "doc": "Body of PATCH global/config: { \"fields\": { : , ... } } with at least one field, keyed by the AdminConfiguration wire name. Only the fields sent are written; values take the field's own JSON type.", + "fields": [ + { + "name": "fields", + "type": { + "kind": "map", + "value": { + "kind": "primitive", + "name": "unknown" + } + }, + "optional": true + } + ] + }, "PatchTenantConfigurationFieldsRequest": { "kind": "object", "doc": "Body of PATCH config/{tenantId}/fields: { \"fields\": { : , ... }, \"reason\"?: \"...\" } with at least one field. Values take the field's own JSON type (TenantConfiguration).", @@ -24121,7 +24138,7 @@ }, "UpdateAdminConfigurationResponse": { "kind": "object", - "doc": "Response of PUT global/config: acknowledgement plus the stored admin configuration.", + "doc": "Response of PATCH global/config: acknowledgement plus the stored admin configuration.", "fields": [ { "name": "success", diff --git a/src/Web/autopilot-monitor-web/utils/wire-types.generated.ts b/src/Web/autopilot-monitor-web/utils/wire-types.generated.ts index 92ea1f21..12172d23 100644 --- a/src/Web/autopilot-monitor-web/utils/wire-types.generated.ts +++ b/src/Web/autopilot-monitor-web/utils/wire-types.generated.ts @@ -3029,6 +3029,11 @@ export interface OpsEventListResponse { nextLink?: string; } +/** Body of PATCH global/config: { "fields": { : , ... } } with at least one field, keyed by the AdminConfiguration wire name. Only the fields sent are written; values take the field's own JSON type. */ +export interface PatchAdminConfigurationRequest { + fields?: Record; +} + /** Body of PATCH config/{tenantId}/fields: { "fields": { : , ... }, "reason"?: "..." } with at least one field. Values take the field's own JSON type (TenantConfiguration). */ export interface PatchTenantConfigurationFieldsRequest { fields?: Record; @@ -5291,7 +5296,7 @@ export interface UnmatchedSoftwareItem { normalizedVersion?: string; } -/** Response of PUT global/config: acknowledgement plus the stored admin configuration. */ +/** Response of PATCH global/config: acknowledgement plus the stored admin configuration. */ export interface UpdateAdminConfigurationResponse { success: boolean; message: string;