Defensively coerce legacy config values during migration, and pass in error to fastify.log correctly

Author: nmfretz

apps/backend/src/modules/config/migration.ts

@@ -1,14 +1,56 @@
 // This module is responsible for migrating the JSON config from the previous app (umbel-config.json) to this app's equivalent settings.json
+// It will run on first boot after the update and coerce the values to the new schema if needed.
 import path from 'node:path'
 import fse from 'fs-extra'
+import {ZodError} from 'zod'
 
 import {writeWithBackup} from './fs-helpers.js'
 import {APP_STATE_DIR} from '../../lib/paths.js'
-import {settingsSchema, defaultValues, type SettingsSchema} from '#settings'
+import {settingsSchema, defaultValues, settingsMetadata, type SettingsSchema, type Option} from '#settings'
 
 const LEGACY_CONFIG_PATH = path.join(APP_STATE_DIR, 'bitcoin-config.json')
 const NEW_CONFIG_PATH = path.join(APP_STATE_DIR, 'settings.json')
 
+// Normalise anything “boolean‑like” that may appear in the legacy file.
+const toBool = (v: unknown): boolean => {
+	if (typeof v === 'boolean') return v
+	if (typeof v === 'number') return v === 1
+	if (typeof v === 'string') {
+		const s = v.trim().toLowerCase()
+		if (s === 'true') return true
+		if (s === 'false') return false
+		if (s === '1') return true
+		if (s === '0') return false
+	}
+	return false
+}
+
+// Coerce legacy string / number / boolean mixes into the correct runtime types expected by the new schema, using `settingsMetadata` as the single source of truth.
+// Legacy values should already be stored as the correct types; however, this is a defensive measure in case users have manually edited the file or there are edge cases we aren't aware of in the old app.
+function coerceFromMetadata(raw: Record<string, unknown>) {
+	const out: Record<string, unknown> = {}
+
+	for (const [key, value] of Object.entries(raw)) {
+		// ignore any unknown keys
+		if (!(key in settingsMetadata)) continue
+
+		const meta = settingsMetadata[key as keyof typeof settingsMetadata] as Option
+
+		switch (meta.kind) {
+			case 'number':
+				out[key] = typeof value === 'string' ? Number(value) : value
+				break
+			case 'toggle':
+				out[key] = toBool(value)
+				break
+			default:
+				// 'multi' & 'select' need no coercion here
+				out[key] = value
+		}
+	}
+	return out
+}
+
 // Mapping of legacy config options to modern config options.
 // Options with values that are 1-to-1 with the modern config options are included. Options that require logic to translate are handled below.
 // These are listed in the same order as the old DEFAULT_ADVANCED_SETTINGS from the legacy app for ease of comparison
@@ -50,35 +92,51 @@ export async function migrateLegacyConfig(): Promise<SettingsSchema | undefined>
 	// If the old bitcoin-config.json doesn't exist, we don't need to migrate
 	if (!(await fse.pathExists(LEGACY_CONFIG_PATH))) return
 
-	const legacyConfig = await fse.readJson(LEGACY_CONFIG_PATH)
+	const legacyConfig = await fse.readJson(LEGACY_CONFIG_PATH).catch((err) => {
+		// throw a nicely formatted error message that is logged by the Fastify server logger
+		const msg =
+			err instanceof SyntaxError
+				? '[migration] Invalid JSON in legacy bitcoin-config.json'
+				: '[migration] Unable to read legacy bitcoin-config.json'
+		throw new Error(`${msg}: ${err.message}`)
+	})
 
 	// First we translate the config options that are not 1-to-1
 	const translated: Partial<SettingsSchema> = {
 		// the legacy config split up the outgoing connections into separate keys for clearnet, tor, and i2p
 		onlynet: [
-			...(legacyConfig.clearnet ? ['clearnet'] : []),
-			...(legacyConfig.tor ? ['tor'] : []),
-			...(legacyConfig.i2p ? ['i2p'] : []),
+			...(toBool(legacyConfig.clearnet) ? ['clearnet'] : []),
+			...(toBool(legacyConfig.tor) ? ['tor'] : []),
+			...(toBool(legacyConfig.i2p) ? ['i2p'] : []),
 		],
 		// the legacy config had a single incomingConnections key that was a boolean for whether to allow incoming connections on ALL networks
-		listen: legacyConfig.incomingConnections ? ['clearnet', 'tor', 'i2p'] : [],
+		listen: toBool(legacyConfig.incomingConnections) ? ['clearnet', 'tor', 'i2p'] : [],
 		// only use the pruneSizeGB value if prune is enabled because pruning could be disabled in the old app even with a pruneSizeGB value set
-		prune: legacyConfig.prune?.enabled ? legacyConfig.prune.pruneSizeGB : 0,
+		prune: toBool(legacyConfig.prune?.enabled) ? Number(legacyConfig.prune.pruneSizeGB) : 0,
 	}
 
 	// Then we translate the config options that are 1-to-1
 	for (const [legacyKey, modernKey] of Object.entries(LEGACY_TO_MODERN_MAP)) {
 		if (legacyKey in legacyConfig) translated[modernKey] = legacyConfig[legacyKey]
 	}
 
-	// merge with current defaults and validate to fail fast here if there are any issues
-	// This is done again in ensureConfig
-	const validated = settingsSchema.parse({...defaultValues, ...translated})
+	// defensively coerce the values to the new schema
+	const coerced = coerceFromMetadata(translated)
 
-	await writeWithBackup(NEW_CONFIG_PATH, JSON.stringify(validated, null, 2) + '\n')
-	await fse.move(LEGACY_CONFIG_PATH, `${LEGACY_CONFIG_PATH}.bak`, {overwrite: false})
-
-	return validated
+	try {
+		const validated = settingsSchema.parse({...defaultValues, ...coerced})
+		await writeWithBackup(NEW_CONFIG_PATH, JSON.stringify(validated, null, 2) + '\n')
+		await fse.move(LEGACY_CONFIG_PATH, `${LEGACY_CONFIG_PATH}.bak`, {overwrite: false})
+		return validated
+	} catch (error) {
+		if (error instanceof ZodError) {
+			// This should be extremely rare because we defensively coerce the values to the new schema
+			// but could still happen if, for example, the user manually edits the chain to an invalid value like "mainnet"
+			const summary = error.errors.map((e) => `${e.path.join('.')}: ${e.message}`).join('; ')
+			throw new Error(`[migration] Validation of legacy config failed – ${summary}`)
+		}
+		throw new Error('[migration] Unexpected error during migration of legacy config', {cause: error as Error})
+	}
 }
 
 // Previous app's bitcoin-config.json for reference:

apps/backend/src/server.ts

@@ -15,9 +15,9 @@ const __dirname = path.dirname(fileURLToPath(import.meta.url))
 await ensureDirs()
 
 // Start bitcoind without blocking server start
-bootBitcoind().catch((err) => {
-	bitcoind.setLastError(err as Error) // record for /status
-	app.log.error('Bitcoind bootstrap failed:', err)
+bootBitcoind().catch((error) => {
+	bitcoind.setLastError(error as Error) // record for /status
+	app.log.error(error, 'Bitcoind bootstrap failed.')
 })
 
 // Create the HTTP server and register the routes