Fix/backup restore with invalid settings (#1793)

* Fix typo in setting validation error message

* Convert value to internal type before validating it

* Only update setting in case value is valid

* Ignore settings validation errors on backup restore

* Remove potential not privacy safe value from logs
This commit is contained in:
schroda
2025-11-26 03:56:57 +01:00
committed by GitHub
parent b58a716daa
commit aa8d27f679
4 changed files with 30 additions and 11 deletions

View File

@@ -35,7 +35,7 @@ object SettingsBackupSettingsHandlerGenerator {
appendLine( appendLine(
KotlinFileGeneratorHelper.createImports( KotlinFileGeneratorHelper.createImports(
listOf( listOf(
"suwayomi.tachidesk.graphql.mutations.SettingsMutation", "suwayomi.tachidesk.server.settings.SettingsUpdater",
"suwayomi.tachidesk.manga.impl.backup.BackupFlags", "suwayomi.tachidesk.manga.impl.backup.BackupFlags",
"suwayomi.tachidesk.manga.impl.backup.proto.models.BackupServerSettings", "suwayomi.tachidesk.manga.impl.backup.proto.models.BackupServerSettings",
"suwayomi.tachidesk.server.serverConfig", "suwayomi.tachidesk.server.serverConfig",
@@ -77,7 +77,7 @@ object SettingsBackupSettingsHandlerGenerator {
appendLine("fun restore(backupServerSettings: BackupServerSettings?) {".addIndentation(indentation)) appendLine("fun restore(backupServerSettings: BackupServerSettings?) {".addIndentation(indentation))
appendLine("if (backupServerSettings == null) { return }".addIndentation(contentIndentation)) appendLine("if (backupServerSettings == null) { return }".addIndentation(contentIndentation))
appendLine() appendLine()
appendLine("SettingsMutation().updateSettings(".addIndentation(contentIndentation)) appendLine("SettingsUpdater.updateAll(".addIndentation(contentIndentation))
appendLine("backupServerSettings.copy(".addIndentation(indentation * 3)) appendLine("backupServerSettings.copy(".addIndentation(indentation * 3))
val deprecatedSettings = settings.filter { it.typeInfo.restoreLegacy != null } val deprecatedSettings = settings.filter { it.typeInfo.restoreLegacy != null }

View File

@@ -109,7 +109,7 @@ open class SettingDelegate<T : Any>(
val error = validate(initialValue) val error = validate(initialValue)
if (error != null) { if (error != null) {
KotlinLogging.logger { }.warn { KotlinLogging.logger { }.warn {
"Invalid config value ($initialValue) for $moduleName.$propertyName: $error. Using default value: $defaultValue" "Invalid config value for $moduleName.$propertyName: $error. Using default value: $defaultValue"
} }
stateFlow.value = toValidValue?.let { it(initialValue) } ?: defaultValue stateFlow.value = toValidValue?.let { it(initialValue) } ?: defaultValue
@@ -406,8 +406,8 @@ class DisableableDoubleSetting(
validator = { value -> validator = { value ->
when { when {
value == 0.0 -> null value == 0.0 -> null
min != null && value < min -> "Value must 0.0 or be at least $min" min != null && value < min -> "Value must be 0.0 or be at least $min"
max != null && value > max -> "Value must 0.0 or not exceed $max" max != null && value > max -> "Value must be 0.0 or not exceed $max"
else -> null else -> null
} }
}, },

View File

@@ -9,6 +9,8 @@ import kotlin.reflect.KProperty1
import kotlin.reflect.full.memberProperties import kotlin.reflect.full.memberProperties
object SettingsUpdater { object SettingsUpdater {
private val logger = KotlinLogging.logger { }
private fun updateSetting( private fun updateSetting(
name: String, name: String,
value: Any, value: Any,
@@ -30,12 +32,21 @@ object SettingsUpdater {
?.convertToInternalType ?.convertToInternalType
?.invoke(value) ?: value ?.invoke(value) ?: value
val validationError = SettingsValidator.validate(name, maybeConvertedValue)
val isValid = validationError == null
if (!isValid) {
logger.warn { "Invalid value for setting $name: $validationError. Ignoring update." }
return
}
// Normal update - MigratedConfigValue handles deprecated mappings automatically // Normal update - MigratedConfigValue handles deprecated mappings automatically
@Suppress("UNCHECKED_CAST") @Suppress("UNCHECKED_CAST")
(stateFlow as MutableStateFlow<Any>).value = maybeConvertedValue (stateFlow as MutableStateFlow<Any>).value = maybeConvertedValue
} }
} catch (e: Exception) { } catch (e: Exception) {
KotlinLogging.logger { }.error(e) { "Failed to update setting $name due to" } logger.error(e) { "Failed to update setting $name due to" }
} }
} }

View File

@@ -3,24 +3,32 @@ package suwayomi.tachidesk.server.settings
import suwayomi.tachidesk.graphql.types.Settings import suwayomi.tachidesk.graphql.types.Settings
object SettingsValidator { object SettingsValidator {
private fun validateSingle( fun validate(
name: String, name: String,
value: Any?, value: Any?,
): String? { ): String? {
val metadata = SettingsRegistry.get(name) ?: return null val metadata = SettingsRegistry.get(name) ?: return null
return metadata.validator?.invoke(value)
val maybeConvertedValue =
if (value != null) {
metadata.typeInfo.convertToInternalType?.invoke(value) ?: value
} else {
value
} }
private fun validateAll( return metadata.validator?.invoke(maybeConvertedValue)
}
fun validate(
values: Map<String, Any?>, values: Map<String, Any?>,
ignoreNull: Boolean?, ignoreNull: Boolean?,
): List<String> = ): List<String> =
values values
.filterValues { value -> ignoreNull == false || value != null } .filterValues { value -> ignoreNull == false || value != null }
.mapNotNull { (name, value) -> validateSingle(name, value)?.let { error -> "$name: $error" } } .mapNotNull { (name, value) -> validate(name, value)?.let { error -> "$name: $error" } }
fun validate( fun validate(
settings: Settings, settings: Settings,
ignoreNull: Boolean = false, ignoreNull: Boolean = false,
): List<String> = validateAll(settings.asMap(), ignoreNull) ): List<String> = validate(settings.asMap(), ignoreNull)
} }