From 5e5c94fc4549ad29db18d465b32afe54ae7387aa Mon Sep 17 00:00:00 2001 From: m4bard <304653687+m4bard@users.noreply.github.com> Date: Wed, 16 Sep 2026 06:06:13 -0500 Subject: [PATCH] Keep stored secrets when a save carries the redaction sentinel GET /configuration/startupconfig and GET /settings replace each secret with ApiResponseRedactor.RedactedValue for any caller the redaction gate does not exempt. The settings screen holds that document and posts it back unchanged when the operator saves, so the literal sentinel was written over the stored value and the real secret was lost. SaveProwlarrImportSettingsAsync and the ProwlarrApiKeyEncrypted branch of SaveApplicationSettingsAsync already compare the incoming value against the sentinel and keep what is stored. This applies the same check to the fields that never got it: ApiKey and SslCertPassword on the startup config, and WebhookUrl, DiscordBotToken and the per-webhook Url on the settings row. Only the sentinel is special-cased. Blank still clears the value on these paths, so an operator can still remove a key or a webhook URL, and the tests cover that alongside a plain rotation to a new value. Co-Authored-By: Claude Sonnet 5 --- .../Core/ConfigurationService.cs | 65 +++++- .../Core/ConfigurationServiceTests.cs | 189 ++++++++++++++++++ 2 files changed, 253 insertions(+), 1 deletion(-) diff --git a/listenarr.application/Configuration/Core/ConfigurationService.cs b/listenarr.application/Configuration/Core/ConfigurationService.cs index 7adefede2..999de5283 100644 --- a/listenarr.application/Configuration/Core/ConfigurationService.cs +++ b/listenarr.application/Configuration/Core/ConfigurationService.cs @@ -164,6 +164,41 @@ public async Task SaveApplicationSettingsAsync(ApplicationSettings settings) settings.EnabledNotificationTriggers = existing.EnabledNotificationTriggers; if (settings.Webhooks == null) settings.Webhooks = existing.Webhooks; + + // The other fields RedactApplicationSettings covers need the + // same sentinel check as ProwlarrApiKeyEncrypted above. A + // redaction-gated caller is handed RedactedValue for each of + // them by the GET and posts it back verbatim, so without this + // the sentinel overwrites the stored secret. Blank is left + // alone here for the same reason as above: clearing a webhook + // URL or a bot token has to stay possible. + if (string.Equals(settings.WebhookUrl, ApiResponseRedactor.RedactedValue, StringComparison.Ordinal)) + { + settings.WebhookUrl = existing.WebhookUrl; + } + + if (string.Equals(settings.DiscordBotToken, ApiResponseRedactor.RedactedValue, StringComparison.Ordinal)) + { + settings.DiscordBotToken = existing.DiscordBotToken; + } + + if (settings.Webhooks != null && existing.Webhooks != null) + { + foreach (var webhook in settings.Webhooks) + { + if (!string.Equals(webhook.Url, ApiResponseRedactor.RedactedValue, StringComparison.Ordinal)) + { + continue; + } + + var storedWebhook = existing.Webhooks + .FirstOrDefault(candidate => string.Equals(candidate.Id, webhook.Id, StringComparison.Ordinal)); + if (storedWebhook != null) + { + webhook.Url = storedWebhook.Url; + } + } + } } if (!string.IsNullOrWhiteSpace(settings.OutputPath) @@ -357,6 +392,35 @@ public async Task SaveStartupConfigAsync(StartupConfig config) { try { + var currentConfig = startupConfigService.GetConfig(); + + // The GET that populates the settings screen replaces each + // secret with ApiResponseRedactor.RedactedValue for callers the + // redaction gate does not exempt, and the screen posts that same + // document straight back when the operator saves. Treat the + // sentinel as "unchanged" and keep what is already stored, the + // way SaveApplicationSettingsAsync and + // SaveProwlarrImportSettingsAsync already do for the Prowlarr + // key. Without this the literal sentinel lands in config.json + // and the real value is gone. + // + // Only the sentinel is special-cased. A blank or absent field + // still means what it has always meant here, which is clear the + // value, so an operator who empties the API key box can still + // do so. + if (config != null && currentConfig != null) + { + if (string.Equals(config.ApiKey, ApiResponseRedactor.RedactedValue, StringComparison.Ordinal)) + { + config.ApiKey = currentConfig.ApiKey; + } + + if (string.Equals(config.SslCertPassword, ApiResponseRedactor.RedactedValue, StringComparison.Ordinal)) + { + config.SslCertPassword = currentConfig.SslCertPassword; + } + } + // Defense-in-depth backstop against the auth-enable lockout. // SaveApplicationSettingsAsync's throw-on-failure (above) only // covers the case where admin credentials were *supplied* but @@ -378,7 +442,6 @@ public async Task SaveStartupConfigAsync(StartupConfig config) // management path, not here. if (config != null && config.IsAuthenticationEnabled()) { - var currentConfig = startupConfigService.GetConfig(); var wasAuthEnabled = currentConfig?.IsAuthenticationEnabled() == true; if (!wasAuthEnabled) { diff --git a/tests/Features/Application/Configuration/Core/ConfigurationServiceTests.cs b/tests/Features/Application/Configuration/Core/ConfigurationServiceTests.cs index ef292849a..f2e5a91aa 100644 --- a/tests/Features/Application/Configuration/Core/ConfigurationServiceTests.cs +++ b/tests/Features/Application/Configuration/Core/ConfigurationServiceTests.cs @@ -394,6 +394,195 @@ await svc.SaveStartupConfigAsync(new StartupConfig currentConfigEnabled.Verify(s => s.SaveAsync(It.IsAny()), Times.Once); } + [Fact] + public async Task SaveStartupConfig_RedactedSecrets_KeepStoredValues() + { + // GET /configuration/startupconfig hands a redaction-gated caller + // ApiResponseRedactor.RedactedValue in place of the API key and the + // SSL certificate password, and the settings screen posts that same + // document back on save. The sentinel must be read as "unchanged", + // not written to config.json on top of the real secret. + var stored = new StartupConfig + { + AuthenticationRequired = "false", + ApiKey = "stored-api-key", + SslCertPassword = "stored-cert-password", + Port = 5000, + }; + + var startupConfigService = new Mock(); + startupConfigService.Setup(s => s.GetConfig()).Returns(stored); + StartupConfig? saved = null; + startupConfigService.Setup(s => s.SaveAsync(It.IsAny())) + .Callback(c => saved = c) + .Returns(Task.CompletedTask); + + Init(b => b.WithSingleton(startupConfigService.Object)); + var svc = _provider.GetRequiredService(); + + // Build the incoming payload the way the API actually produces it, + // so the test exercises the real redaction sentinel rather than a + // hand-written copy of it. + var incoming = ApiResponseRedactor.RedactStartupConfig(stored); + Assert.Equal(ApiResponseRedactor.RedactedValue, incoming.ApiKey); + Assert.Equal(ApiResponseRedactor.RedactedValue, incoming.SslCertPassword); + + await svc.SaveStartupConfigAsync(incoming); + + Assert.NotNull(saved); + Assert.Equal("stored-api-key", saved!.ApiKey); + Assert.Equal("stored-cert-password", saved.SslCertPassword); + } + + [Fact] + public async Task SaveStartupConfig_NewSecrets_ReplaceStoredValues() + { + // Control for the test above. The sentinel check must not degrade + // into "never update these fields": an operator who types a new API + // key or a new certificate password still has to be able to save it. + var stored = new StartupConfig + { + AuthenticationRequired = "false", + ApiKey = "stored-api-key", + SslCertPassword = "stored-cert-password", + }; + + var startupConfigService = new Mock(); + startupConfigService.Setup(s => s.GetConfig()).Returns(stored); + StartupConfig? saved = null; + startupConfigService.Setup(s => s.SaveAsync(It.IsAny())) + .Callback(c => saved = c) + .Returns(Task.CompletedTask); + + Init(b => b.WithSingleton(startupConfigService.Object)); + var svc = _provider.GetRequiredService(); + + await svc.SaveStartupConfigAsync(new StartupConfig + { + AuthenticationRequired = "false", + ApiKey = "rotated-api-key", + SslCertPassword = "rotated-cert-password", + }); + + Assert.NotNull(saved); + Assert.Equal("rotated-api-key", saved!.ApiKey); + Assert.Equal("rotated-cert-password", saved.SslCertPassword); + } + + [Fact] + public async Task SaveStartupConfig_BlankOrAbsentSecrets_AreWrittenThroughUnchanged() + { + // Second control. Only the sentinel is special-cased; blank keeps + // the meaning it has always had on this path, which is clear the + // value. Preserving on blank as well would take away the operator's + // way of removing an API key or a certificate password. + var stored = new StartupConfig + { + AuthenticationRequired = "false", + ApiKey = "stored-api-key", + SslCertPassword = "stored-cert-password", + }; + + var startupConfigService = new Mock(); + startupConfigService.Setup(s => s.GetConfig()).Returns(stored); + var savedConfigs = new List(); + startupConfigService.Setup(s => s.SaveAsync(It.IsAny())) + .Callback(savedConfigs.Add) + .Returns(Task.CompletedTask); + + Init(b => b.WithSingleton(startupConfigService.Object)); + var svc = _provider.GetRequiredService(); + + await svc.SaveStartupConfigAsync(new StartupConfig + { + AuthenticationRequired = "false", + ApiKey = string.Empty, + SslCertPassword = string.Empty, + }); + await svc.SaveStartupConfigAsync(new StartupConfig + { + AuthenticationRequired = "false", + ApiKey = null, + SslCertPassword = null, + }); + + Assert.Equal(2, savedConfigs.Count); + Assert.Equal(string.Empty, savedConfigs[0].ApiKey); + Assert.Equal(string.Empty, savedConfigs[0].SslCertPassword); + Assert.Null(savedConfigs[1].ApiKey); + Assert.Null(savedConfigs[1].SslCertPassword); + } + + [Fact] + public async Task SaveApplicationSettings_RedactedSecrets_KeepStoredValues() + { + // Same round trip on the settings row. ProwlarrApiKeyEncrypted was + // already guarded; the rest of what RedactApplicationSettings covers + // was not, so the sentinel reached the database. + var svc = _provider.GetRequiredService(); + + var seed = await svc.GetApplicationSettingsAsync(); + seed.WebhookUrl = "https://example.test/global-hook"; + seed.DiscordBotToken = "stored-discord-token"; + seed.Webhooks = + [ + new() { Id = "webhook-1", Name = "First", Url = "https://example.test/one", Type = "Zapier" }, + new() { Id = "webhook-2", Name = "Second", Url = "https://example.test/two", Type = "Slack" }, + ]; + await svc.SaveApplicationSettingsAsync(seed); + + var current = await svc.GetApplicationSettingsAsync(); + var incoming = ApiResponseRedactor.RedactApplicationSettings(current); + Assert.Equal(ApiResponseRedactor.RedactedValue, incoming.WebhookUrl); + Assert.Equal(ApiResponseRedactor.RedactedValue, incoming.DiscordBotToken); + Assert.All(incoming.Webhooks!, w => Assert.Equal(ApiResponseRedactor.RedactedValue, w.Url)); + + await svc.SaveApplicationSettingsAsync(incoming); + + var stored = await _applicationSettingsRepository.GetAsync(); + Assert.NotNull(stored); + Assert.Equal("https://example.test/global-hook", stored!.WebhookUrl); + Assert.Equal("stored-discord-token", stored.DiscordBotToken); + Assert.NotNull(stored.Webhooks); + Assert.Equal( + "https://example.test/one", + stored.Webhooks!.Single(w => w.Id == "webhook-1").Url); + Assert.Equal( + "https://example.test/two", + stored.Webhooks!.Single(w => w.Id == "webhook-2").Url); + } + + [Fact] + public async Task SaveApplicationSettings_NewSecrets_ReplaceStoredValues() + { + // Control for the test above, on the settings row this time. + var svc = _provider.GetRequiredService(); + + var seed = await svc.GetApplicationSettingsAsync(); + seed.WebhookUrl = "https://example.test/global-hook"; + seed.DiscordBotToken = "stored-discord-token"; + seed.Webhooks = + [ + new() { Id = "webhook-1", Name = "First", Url = "https://example.test/one", Type = "Zapier" }, + ]; + await svc.SaveApplicationSettingsAsync(seed); + + var incoming = await svc.GetApplicationSettingsAsync(); + incoming.WebhookUrl = "https://example.test/changed-hook"; + incoming.DiscordBotToken = "rotated-discord-token"; + incoming.Webhooks!.Single(w => w.Id == "webhook-1").Url = "https://example.test/changed-one"; + + await svc.SaveApplicationSettingsAsync(incoming); + + var stored = await _applicationSettingsRepository.GetAsync(); + Assert.NotNull(stored); + Assert.Equal("https://example.test/changed-hook", stored!.WebhookUrl); + Assert.Equal("rotated-discord-token", stored.DiscordBotToken); + Assert.Equal( + "https://example.test/changed-one", + stored.Webhooks!.Single(w => w.Id == "webhook-1").Url); + } + [Fact] public async Task SaveStartupConfig_SkipsAdminCheck_WhenAuthDisabled() {