From ba81316c042cc990568cf22fd0e0ad7645d87e76 Mon Sep 17 00:00:00 2001 From: Sev7eNup <79143581+Sev7eNup@users.noreply.github.com> Date: Sat, 24 Oct 2026 17:36:43 +0200 Subject: [PATCH] fix: validate runtime settings before atomic replacement Match configuration keys case-insensitively and merge disjoint case-variant sections when reading runtime overrides. Validate serialized candidates with the production JSON configuration parser before changing the original or backups. Add regression tests for mixed casing and duplicate flattened keys. Fixes #611 --- .../Configuration/RuntimeOverridesWriter.cs | 54 ++++++++++-- .../RuntimeOverridesWriterTests.cs | 83 +++++++++++++++++++ 2 files changed, 129 insertions(+), 8 deletions(-) diff --git a/src/NodePilot.Api/Configuration/RuntimeOverridesWriter.cs b/src/NodePilot.Api/Configuration/RuntimeOverridesWriter.cs index 7d71296e..d27ac28b 100644 --- a/src/NodePilot.Api/Configuration/RuntimeOverridesWriter.cs +++ b/src/NodePilot.Api/Configuration/RuntimeOverridesWriter.cs @@ -2,6 +2,7 @@ using System.Text; using System.Text.Json; using System.Text.Json.Nodes; +using Microsoft.Extensions.Configuration.Json; using NodePilot.Core.Interfaces; namespace NodePilot.Api.Configuration; @@ -127,11 +128,11 @@ private static bool ReencryptValues(JsonNode node, ISecretProtector protector, C internal static void ReplaceWithoutBackup(string path, JsonObject root) { + var content = SerializeAndValidate(root); var temporary = path + ".migration." + Guid.NewGuid().ToString("N") + ".tmp"; try { - File.WriteAllText(temporary, root.ToJsonString(new JsonSerializerOptions { WriteIndented = true }), - new UTF8Encoding(encoderShouldEmitUTF8Identifier: false)); + File.WriteAllBytes(temporary, content); try { File.Replace(temporary, path, destinationBackupFileName: null); } catch (Exception ex) when (ex is IOException or PlatformNotSupportedException or UnauthorizedAccessException) { @@ -147,13 +148,40 @@ internal static void ReplaceWithoutBackup(string path, JsonObject root) /// public JsonObject ReadOrEmpty() { - if (!File.Exists(_path)) return new JsonObject(); + var nodeOptions = new JsonNodeOptions { PropertyNameCaseInsensitive = true }; + if (!File.Exists(_path)) return new JsonObject(nodeOptions); var raw = File.ReadAllText(_path); - if (string.IsNullOrWhiteSpace(raw)) return new JsonObject(); - var parsed = JsonNode.Parse(raw) as JsonObject + if (string.IsNullOrWhiteSpace(raw)) return new JsonObject(nodeOptions); + var parsed = JsonNode.Parse(raw, documentOptions: + new JsonDocumentOptions { CommentHandling = JsonCommentHandling.Skip, AllowTrailingCommas = true }) as JsonObject ?? throw new InvalidOperationException( $"Runtime overrides file '{_path}' must contain a JSON object at the root."); - return parsed; + return NormalizeConfigurationKeys(parsed).AsObject(); + } + + private static JsonNode NormalizeConfigurationKeys(JsonNode node) + { + if (node is JsonObject source) + { + var result = new JsonObject(new JsonNodeOptions { PropertyNameCaseInsensitive = true }); + MergeConfigurationObject(result, source); + return result; + } + if (node is JsonArray array) + return new JsonArray(array.Select(child => child is null ? null : NormalizeConfigurationKeys(child)).ToArray()); + return node.DeepClone(); + } + + private static void MergeConfigurationObject(JsonObject target, JsonObject source) + { + foreach (var (key, value) in source) + { + // IConfiguration permits case-variant sections when their leaf keys are disjoint. + if (target[key] is JsonObject existing && value is JsonObject nested) + MergeConfigurationObject(existing, nested); + else + target.Add(key, value is null ? null : NormalizeConfigurationKeys(value)); + } } /// @@ -463,9 +491,9 @@ private void EnsureDirectoryExists() private void WriteAtomic(JsonObject root) { + var content = SerializeAndValidate(root); var tmp = _path + ".tmp"; - var json = root.ToJsonString(new JsonSerializerOptions { WriteIndented = true }); - File.WriteAllText(tmp, json, new UTF8Encoding(encoderShouldEmitUTF8Identifier: false)); + File.WriteAllBytes(tmp, content); if (!File.Exists(_path)) { @@ -497,6 +525,16 @@ private void WriteAtomic(JsonObject root) RotateBackups(); } + private static byte[] SerializeAndValidate(JsonObject root) + { + var content = Encoding.UTF8.GetBytes(root.ToJsonString(new JsonSerializerOptions { WriteIndented = true })); + // The encrypting production provider uses this parser before decrypting leaf values. + using var provider = new JsonConfigurationProvider(new JsonConfigurationSource()); + using var stream = new MemoryStream(content, writable: false); + provider.Load(stream); + return content; + } + private void RotateBackups() { var dir = Path.GetDirectoryName(_path); diff --git a/tests/NodePilot.Api.Tests/Configuration/RuntimeOverridesWriterTests.cs b/tests/NodePilot.Api.Tests/Configuration/RuntimeOverridesWriterTests.cs index cec48ede..164dbc3a 100644 --- a/tests/NodePilot.Api.Tests/Configuration/RuntimeOverridesWriterTests.cs +++ b/tests/NodePilot.Api.Tests/Configuration/RuntimeOverridesWriterTests.cs @@ -1,4 +1,5 @@ using System.Text.Json.Nodes; +using Microsoft.Extensions.Configuration; using FluentAssertions; using Microsoft.Extensions.Logging.Abstractions; using NodePilot.Api.Configuration; @@ -33,6 +34,88 @@ private RuntimeOverridesWriter NewWriter(string? subPath = null) return new RuntimeOverridesWriter(path, NullLogger.Instance); } + [Fact] + public void TryUpdateSectionAtomic_LowerCaseSection_RemainsLoadable() + { + var writer = NewWriter(); + File.WriteAllText(writer.OverridesPath, """{"stats":{"RefreshIntervalMinutes":5,"WindowDays":7}}"""); + var result = writer.TryUpdateSectionAtomic("Stats", writer.ComputeSectionEtag("Stats"), + new JsonObject { ["RefreshIntervalMinutes"] = 10, ["WindowDays"] = 7 }, + null, "admin", DateTimeOffset.UtcNow); + + result.Success.Should().BeTrue(); + var config = new ConfigurationBuilder().AddJsonFile(writer.OverridesPath).Build(); + using var disposable = (IDisposable)config; + config["Stats:RefreshIntervalMinutes"].Should().Be("10"); + config["Stats:WindowDays"].Should().Be("7"); + writer.ReadOrEmpty().Count(pair => pair.Key.Equals("Stats", StringComparison.OrdinalIgnoreCase)).Should().Be(1); + } + + [Fact] + public void TryUpdateSectionAtomic_MixedCaseNestedPath_PreservesSiblingAndReadsExistingValue() + { + var writer = NewWriter(); + File.WriteAllText(writer.OverridesPath, + """{"retention":{"executions":{"MaxAgeDays":7},"auditlog":{"MaxAgeDays":90}}}"""); + (writer.ReadSection("Retention:Executions")?["MaxAgeDays"]?.GetValue()).Should().Be(7); + + var result = writer.TryUpdateSectionAtomic("Retention:Executions", writer.ComputeSectionEtag("Retention:Executions"), + new JsonObject { ["MaxAgeDays"] = 14 }, null, "admin", DateTimeOffset.UtcNow); + + result.Success.Should().BeTrue(); + var config = new ConfigurationBuilder().AddJsonFile(writer.OverridesPath).Build(); + using var disposable = (IDisposable)config; + config["Retention:Executions:MaxAgeDays"].Should().Be("14"); + config["Retention:AuditLog:MaxAgeDays"].Should().Be("90"); + } + + [Theory] + [InlineData(false)] + [InlineData(true)] + public void Write_DuplicateFlattenedKeys_RejectsCandidateAndPreservesOriginal(bool replace) + { + var writer = NewWriter(); + const string original = """{"Stats":{"WindowDays":7}}"""; + File.WriteAllText(writer.OverridesPath, original); + var invalid = new JsonObject + { + ["Stats"] = new JsonObject { ["WindowDays"] = 14 }, + ["stats:windowdays"] = 30 + }; + + var act = () => + { + if (replace) writer.ReplaceAll(invalid); + else writer.MutateAndWrite(root => root["stats:windowdays"] = 30); + }; + + act.Should().Throw(); + File.ReadAllText(writer.OverridesPath).Should().Be(original); + Directory.GetFiles(_tempDir, "*.bak.*").Should().BeEmpty(); + Directory.GetFiles(_tempDir, "*.tmp").Should().BeEmpty(); + } + + [Fact] + public void TryUpdateSectionAtomic_DisjointCaseVariantSections_MergesTheirExistingValues() + { + var writer = NewWriter(); + File.WriteAllText(writer.OverridesPath, + """{"retention":{"executions":{"MaxAgeDays":7}},"Retention":{"auditlog":{"MaxAgeDays":90}}}"""); + var before = new ConfigurationBuilder().AddJsonFile(writer.OverridesPath).Build(); + using var beforeDisposable = (IDisposable)before; + before["Retention:Executions:MaxAgeDays"].Should().Be("7"); + before["Retention:AuditLog:MaxAgeDays"].Should().Be("90"); + + var result = writer.TryUpdateSectionAtomic("Retention:Executions", writer.ComputeSectionEtag("Retention:Executions"), + new JsonObject { ["MaxAgeDays"] = 14 }, null, "admin", DateTimeOffset.UtcNow); + + result.Success.Should().BeTrue(); + var after = new ConfigurationBuilder().AddJsonFile(writer.OverridesPath).Build(); + using var afterDisposable = (IDisposable)after; + after["Retention:Executions:MaxAgeDays"].Should().Be("14"); + after["Retention:AuditLog:MaxAgeDays"].Should().Be("90"); + } + [Fact] public void ReadOrEmpty_FileMissing_ReturnsEmptyObject() {