From 673f0452126fb36a4967ebb6ffd9e8686a07f07c Mon Sep 17 00:00:00 2001 From: Oliver Kieselbach Date: Fri, 25 Sep 2026 13:55:43 +0200 Subject: [PATCH] fix(web): platform settings saves send only the changed fields via PATCH global/config (D-285) Each save handler still builds the edited configuration but now sends changedAdminConfigFields(loaded, edited) through lib/adminConfigSave.ts, and nothing at all when nothing changed. This covers the Global Settings, the diagnostics paths, the ops alerts and the vulnerability section. The MCP access policy patches its single field without reading first. A page loaded before another writer (the agent release pipeline, a second operator, a sync job) can no longer revert that writer's fields. Deployed after the backend that serves PATCH global/config. Co-Authored-By: Claude Opus 5.5 (1M context) --- .../app/admin/AdminConfigContext.tsx | 38 ++++++-------- .../components/VulnerabilityDataSection.tsx | 16 +++--- .../settings/components/McpUsersSection.tsx | 11 ++-- .../lib/__tests__/adminConfigSave.test.ts | 52 +++++++++++++++++++ .../lib/adminConfigSave.ts | 36 +++++++++++++ src/Web/autopilot-monitor-web/lib/api.ts | 2 + .../types/adminConfig.ts | 22 ++++++++ 7 files changed, 138 insertions(+), 39 deletions(-) create mode 100644 src/Web/autopilot-monitor-web/lib/__tests__/adminConfigSave.test.ts create mode 100644 src/Web/autopilot-monitor-web/lib/adminConfigSave.ts diff --git a/src/Web/autopilot-monitor-web/app/admin/AdminConfigContext.tsx b/src/Web/autopilot-monitor-web/app/admin/AdminConfigContext.tsx index 2b97b68e..c9488a58 100644 --- a/src/Web/autopilot-monitor-web/app/admin/AdminConfigContext.tsx +++ b/src/Web/autopilot-monitor-web/app/admin/AdminConfigContext.tsx @@ -5,14 +5,14 @@ import { useAuth } from "../../contexts/AuthContext"; import { api } from "@/lib/api"; import { apiErrorText, fetchJson, jsonBody } from "@/lib/apiClient"; import { CONFIG_PATH_PREFIX, invalidateCachedAuthFetch } from "@/lib/cachedAuthFetch"; -import { fromWireAdminConfiguration, toWireAdminConfiguration, type AdminConfiguration, type OpsAlertRule } from "@/types/adminConfig"; +import { fromWireAdminConfiguration, type AdminConfiguration, type OpsAlertRule } from "@/types/adminConfig"; +import { saveAdminConfigChanges } from "@/lib/adminConfigSave"; import type { AdminConfiguration as WireAdminConfiguration, GetAllPreviewNotificationEmailsResponse, GetPreviewWhitelistResponse, TestNotificationChannelRequest, TestWebhookNotificationResponse, - UpdateAdminConfigurationResponse, } from "@/utils/wire-types.generated"; import type { NotificationChannel } from "@/app/settings/types"; @@ -376,14 +376,14 @@ export function AdminConfigProvider({ children }: { children: React.ReactNode }) maxImeMsiDownloadSizeMB, }; - const result = await fetchJson(api.globalConfig.get(), getAccessToken, { - method: "PUT", - body: jsonBody(toWireAdminConfiguration(updatedConfig)), - }); - setAdminConfig(fromWireAdminConfiguration(result.config)); - // selfServiceAppHomingEnabled gates every tenant's appHomingFunnelActive feature flag. - invalidateCachedAuthFetch(CONFIG_PATH_PREFIX); - setSuccessMessage("Admin configuration saved successfully!"); + // Only the fields this form changed go out; the backend writes nothing else (D-285). + const saved = await saveAdminConfigChanges(adminConfig, updatedConfig, getAccessToken); + if (saved) { + setAdminConfig(saved); + // selfServiceAppHomingEnabled gates every tenant's appHomingFunnelActive feature flag. + invalidateCachedAuthFetch(CONFIG_PATH_PREFIX); + } + setSuccessMessage(saved ? "Admin configuration saved successfully!" : "No changes to save."); setTimeout(() => setSuccessMessage(null), 3000); } catch (err) { console.error("Error saving admin configuration:", err); @@ -460,13 +460,10 @@ export function AdminConfigProvider({ children }: { children: React.ReactNode }) diagnosticsGlobalLogPathsJson: JSON.stringify(paths), }; - const result = await fetchJson(api.globalConfig.get(), getAccessToken, { - method: "PUT", - body: jsonBody(toWireAdminConfiguration(updatedConfig)), - }); - setAdminConfig(fromWireAdminConfiguration(result.config)); + const saved = await saveAdminConfigChanges(adminConfig, updatedConfig, getAccessToken); + if (saved) setAdminConfig(saved); setGlobalDiagPaths(paths); - setSuccessMessage("Global diagnostics log paths saved successfully!"); + setSuccessMessage(saved ? "Global diagnostics log paths saved successfully!" : "No changes to save."); setTimeout(() => setSuccessMessage(null), 4000); } catch (err) { setError(apiErrorText(err, "Failed to save diagnostics paths")); @@ -504,18 +501,15 @@ export function AdminConfigProvider({ children }: { children: React.ReactNode }) excessiveEventAutoActionDurationHours: newAutoActionDurationHours, }; - const result = await fetchJson(api.globalConfig.get(), getAccessToken, { - method: "PUT", - body: jsonBody(toWireAdminConfiguration(updatedConfig)), - }); - setAdminConfig(fromWireAdminConfiguration(result.config)); + const saved = await saveAdminConfigChanges(adminConfig, updatedConfig, getAccessToken); + if (saved) setAdminConfig(saved); setOpsAlertRules(rules); setOpsNotificationChannels(channels); setExcessiveEventCountThreshold(newExcessiveThreshold); setExcessiveEventAutoActionMode(newAutoActionMode); setExcessiveEventAutoActionThreshold(newAutoActionThreshold); setExcessiveEventAutoActionDurationHours(newAutoActionDurationHours); - setSuccessMessage("Alert configuration saved successfully!"); + setSuccessMessage(saved ? "Alert configuration saved successfully!" : "No changes to save."); setTimeout(() => setSuccessMessage(null), 4000); } catch (err) { setError(apiErrorText(err, "Failed to save alert configuration")); diff --git a/src/Web/autopilot-monitor-web/app/admin/components/VulnerabilityDataSection.tsx b/src/Web/autopilot-monitor-web/app/admin/components/VulnerabilityDataSection.tsx index 5947bf56..77f1bc4c 100644 --- a/src/Web/autopilot-monitor-web/app/admin/components/VulnerabilityDataSection.tsx +++ b/src/Web/autopilot-monitor-web/app/admin/components/VulnerabilityDataSection.tsx @@ -2,10 +2,11 @@ import { useCallback, useEffect, useState } from "react"; import { api } from "@/lib/api"; -import { apiErrorText, fetchJson, jsonBody } from "@/lib/apiClient"; +import { apiErrorText, fetchJson } from "@/lib/apiClient"; import SaveResetBar from "../../settings/components/SaveResetBar"; import VulnerabilityExposurePanel from "@/components/VulnerabilityExposurePanel"; -import { fromWireAdminConfiguration, toWireAdminConfiguration, type AdminConfiguration } from "@/types/adminConfig"; +import { fromWireAdminConfiguration, type AdminConfiguration } from "@/types/adminConfig"; +import { saveAdminConfigChanges } from "@/lib/adminConfigSave"; import { useCanMutatePlatform } from "@/hooks/useCanMutatePlatform"; import { SectionCardHeader } from "@/components/SectionCardHeader"; import type { @@ -16,7 +17,6 @@ import type { TriggerMsrcSyncResponse, TriggerNvdCacheRefreshResponse, TriggerVulnerabilityDataSyncResponse, - UpdateAdminConfigurationResponse, } from "@/utils/wire-types.generated"; interface VulnerabilityDataSectionProps { @@ -168,12 +168,10 @@ export function VulnerabilityDataSection({ vulnerabilityCorrelationEnabled, }; - const result = await fetchJson(api.globalConfig.get(), getAccessToken, { - method: "PUT", - body: jsonBody(toWireAdminConfiguration(updatedConfig)), - }); - onAdminConfigUpdated(fromWireAdminConfiguration(result.config)); - setSuccessMessage("Vulnerability configuration saved successfully!"); + // Only the fields this section changed go out; the backend writes nothing else (D-285). + const saved = await saveAdminConfigChanges(adminConfig, updatedConfig, getAccessToken); + if (saved) onAdminConfigUpdated(saved); + setSuccessMessage(saved ? "Vulnerability configuration saved successfully!" : "No changes to save."); setTimeout(() => setSuccessMessage(null), 4000); } catch (err) { const message = apiErrorText(err, "Failed to save vulnerability configuration"); diff --git a/src/Web/autopilot-monitor-web/app/settings/components/McpUsersSection.tsx b/src/Web/autopilot-monitor-web/app/settings/components/McpUsersSection.tsx index 1fdde4d0..11e63041 100644 --- a/src/Web/autopilot-monitor-web/app/settings/components/McpUsersSection.tsx +++ b/src/Web/autopilot-monitor-web/app/settings/components/McpUsersSection.tsx @@ -4,6 +4,7 @@ import { useCallback, useEffect, useState } from "react"; import { useAuth } from "../../../contexts/AuthContext"; import { ApiError, apiErrorText, fetchJson, fetchOk, jsonBody } from "@/lib/apiClient"; import { api } from "@/lib/api"; +import { patchAdminConfigFields } from "@/lib/adminConfigSave"; import { useTenantList } from "@/hooks/useTenantList"; import { HOME_TENANT_UNRESOLVED } from "@/lib/identityBinding"; import { isApplicationKey, looksLikeGuid, principalLabel } from "@/utils/principalKeys"; @@ -11,7 +12,6 @@ import { SectionCardHeader } from "@/components/SectionCardHeader"; import { DOCS_PATHS } from "@/lib/docsPaths"; import type { AddMcpUserRequest, - AdminConfiguration, GetMcpUsersResponse, McpUserEntry, PlanTierDefinition as PlanTierDefinitionWire, @@ -111,13 +111,8 @@ export default function McpUsersSection() { setError(null); setSuccessMessage(null); - // Read current global config, update McpAccessPolicy, save back - const config = await fetchJson(api.globalConfig.get(), getAccessToken); - - await fetchOk(api.globalConfig.get(), getAccessToken, { - method: "PUT", - body: jsonBody({ ...config, mcpAccessPolicy: newPolicy }), - }); + // Only this one field is sent; the backend writes nothing else (D-285). + await patchAdminConfigFields({ mcpAccessPolicy: newPolicy }, getAccessToken); setPolicy(newPolicy); setSuccessMessage(`MCP access policy changed to "${POLICY_LABELS[newPolicy]}".`); diff --git a/src/Web/autopilot-monitor-web/lib/__tests__/adminConfigSave.test.ts b/src/Web/autopilot-monitor-web/lib/__tests__/adminConfigSave.test.ts new file mode 100644 index 00000000..44c79325 --- /dev/null +++ b/src/Web/autopilot-monitor-web/lib/__tests__/adminConfigSave.test.ts @@ -0,0 +1,52 @@ +import { describe, expect, it, vi } from "vitest"; +import { saveAdminConfigChanges } from "@/lib/adminConfigSave"; +import { changedAdminConfigFields, type AdminConfiguration } from "@/types/adminConfig"; + +// D-285: a save sends only the fields the page changed. The page's copy of everything else — the +// release pipeline's agent hashes above all — must never travel, or a page loaded before a release +// reverts it (2026-09-25). +const loaded = { + partitionKey: "GlobalConfig", + rowKey: "config", + enforceClientAppBinding: false, + mcpClientRegistrationEnabled: true, + globalRateLimitRequestsPerMinute: 100, + latestAgentV2Version: "2.0.1464", + latestAgentV2ExeSha256: "3cd3a3f1", + platformStatsBlobSasUrl: undefined, + excessiveEventAutoActionMode: undefined, +} as unknown as AdminConfiguration; + +describe("changedAdminConfigFields", () => { + it("returns only the toggled field, never the stale agent hashes", () => { + const edited = { ...loaded, enforceClientAppBinding: true }; + expect(changedAdminConfigFields(loaded, edited)).toEqual({ enforceClientAppBinding: true }); + }); + + it("treats undefined and null as the same unset value", () => { + const edited = { ...loaded, platformStatsBlobSasUrl: null } as unknown as AdminConfiguration; + expect(changedAdminConfigFields(loaded, edited)).toEqual({}); + }); + + it("compares the wire form, so an unset mode equal to the server default is no change", () => { + const edited = { ...loaded, excessiveEventAutoActionMode: "Off" } as AdminConfiguration; + expect(changedAdminConfigFields(loaded, edited)).toEqual({}); + }); + + it("sends a cleared string as its new value", () => { + const withUrl = { ...loaded, platformStatsBlobSasUrl: "https://example.test/sas" } as AdminConfiguration; + const edited = { ...withUrl, platformStatsBlobSasUrl: "" } as AdminConfiguration; + expect(changedAdminConfigFields(withUrl, edited)).toEqual({ platformStatsBlobSasUrl: "" }); + }); +}); + +describe("saveAdminConfigChanges", () => { + it("sends no request when nothing changed", async () => { + const getAccessToken = vi.fn(async () => "token"); + const fetchSpy = vi.spyOn(globalThis, "fetch"); + await expect(saveAdminConfigChanges(loaded, { ...loaded }, getAccessToken)).resolves.toBeNull(); + expect(getAccessToken).not.toHaveBeenCalled(); + expect(fetchSpy).not.toHaveBeenCalled(); + fetchSpy.mockRestore(); + }); +}); diff --git a/src/Web/autopilot-monitor-web/lib/adminConfigSave.ts b/src/Web/autopilot-monitor-web/lib/adminConfigSave.ts new file mode 100644 index 00000000..42aaae20 --- /dev/null +++ b/src/Web/autopilot-monitor-web/lib/adminConfigSave.ts @@ -0,0 +1,36 @@ +import { api } from "@/lib/api"; +import { fetchJson, jsonBody, type GetAccessToken } from "@/lib/apiClient"; +import { changedAdminConfigFields, fromWireAdminConfiguration, type AdminConfiguration } from "@/types/adminConfig"; +import type { + AdminConfiguration as WireAdminConfiguration, + PatchAdminConfigurationRequest, + UpdateAdminConfigurationResponse, +} from "@/utils/wire-types.generated"; + +/** + * Saves the admin configuration by sending only what changed (PATCH global/config, D-285). + * `loaded` is the configuration as the page received it, `edited` the same object with the + * section's edits applied. Returns the stored configuration after the write, or null when nothing + * changed and no request was sent. + */ +export async function saveAdminConfigChanges( + loaded: AdminConfiguration, + edited: AdminConfiguration, + getAccessToken: GetAccessToken, +): Promise { + const fields = changedAdminConfigFields(loaded, edited); + if (Object.keys(fields).length === 0) return null; + return patchAdminConfigFields(fields, getAccessToken); +} + +/** Sends the given fields as they are — for a caller that changes one known field without a loaded page state. */ +export async function patchAdminConfigFields( + fields: Partial, + getAccessToken: GetAccessToken, +): Promise { + const result = await fetchJson(api.globalConfig.update(), getAccessToken, { + method: "PATCH", + body: jsonBody({ fields }), + }); + return fromWireAdminConfiguration(result.config); +} diff --git a/src/Web/autopilot-monitor-web/lib/api.ts b/src/Web/autopilot-monitor-web/lib/api.ts index 8e4f9619..7677874c 100644 --- a/src/Web/autopilot-monitor-web/lib/api.ts +++ b/src/Web/autopilot-monitor-web/lib/api.ts @@ -192,6 +192,8 @@ export const api = { // ── Global Config (global admin) ────────────────────────────────────────── globalConfig: { get: () => `${API_BASE_URL}/api/global/config`, + /** PATCH with only the changed fields (D-285) — there is no full-model PUT. */ + update: () => `${API_BASE_URL}/api/global/config`, /** Sends a test notification to one saved platform (ops) channel. */ testOpsChannel: () => `${API_BASE_URL}/api/global/config/test-ops-channel`, // NOTE: there is deliberately no per-tenant variant — /api/global/config/{tenantId} does diff --git a/src/Web/autopilot-monitor-web/types/adminConfig.ts b/src/Web/autopilot-monitor-web/types/adminConfig.ts index 8642f7a8..ba1624a5 100644 --- a/src/Web/autopilot-monitor-web/types/adminConfig.ts +++ b/src/Web/autopilot-monitor-web/types/adminConfig.ts @@ -31,6 +31,28 @@ export function toWireAdminConfiguration(config: AdminConfiguration): WireAdminC return { ...config, excessiveEventAutoActionMode: config.excessiveEventAutoActionMode ?? "Off" }; } +/** + * The wire fields whose value differs between the configuration as loaded and as edited — the only + * ones a save sends (PATCH global/config, D-285). The backend re-reads the row and writes just these, + * so a page loaded before another writer (the agent release pipeline, a second operator, a sync job) + * can never revert that writer's fields. Compared on the wire form; undefined and null are the same + * unset value. + */ +export function changedAdminConfigFields( + loaded: AdminConfiguration, + edited: AdminConfiguration, +): Partial { + const before = toWireAdminConfiguration(loaded) as unknown as Record; + const after = toWireAdminConfiguration(edited) as unknown as Record; + const changed: Record = {}; + for (const key of Object.keys(after)) { + if (JSON.stringify(after[key] ?? null) !== JSON.stringify(before[key] ?? null)) { + changed[key] = after[key] ?? null; + } + } + return changed as Partial; +} + /** One parsed entry of AdminConfiguration.opsAlertRulesJson (client-side view). */ export interface OpsAlertRule { eventType: string;