From 4015543f66de1a276ffbdcb3f61fb86f8cd8a040 Mon Sep 17 00:00:00 2001 From: Gnuxie <50846879+Gnuxie@users.noreply.github.com> Date: Wed, 9 Oct 2024 11:38:24 +0100 Subject: [PATCH] Filesystem config improvements (#604) * Rename read to configRead as it should have always been. * Got a way to extract non-default values. Now let's try unknown configuration values. * Show unknown property paths with a warning. Now we just need to make this scrap available in commands. * Remove the old Mjolnir horrible RUNTIME client. * Make the path that is used to load the config available. * Warn when `--draupnir-config` isn't used. * Introduce configMeta so that we can log meta on process.exit later. * Only show non-default config values when draupnir is exiting. to reduce noise. * Get consistent with logging. So it turns out that mps4bot-sdk is using a different instance of the bot-sdk module than Draupnir, i think. Since we used to tell MPS's logger to use the bot-sdk's `LogService`, but the `setLogger` that was used was obviously inconsistent with Draupnir's. Obviously the bot-sdk should be a peer dependency in the bot-sdk to prevent this happening in future. --- src/DraupnirBotMode.ts | 4 - src/config.ts | 224 ++++++++++++++++++---- src/index.ts | 11 +- test/integration/fixtures.ts | 3 +- test/integration/manualLaunchScript.ts | 2 +- test/integration/mjolnirSetupUtils.ts | 2 - test/unit/config/unknownPropertiesTest.ts | 41 ++++ 7 files changed, 233 insertions(+), 54 deletions(-) create mode 100644 test/unit/config/unknownPropertiesTest.ts diff --git a/src/DraupnirBotMode.ts b/src/DraupnirBotMode.ts index 5288c81..66ff32c 100644 --- a/src/DraupnirBotMode.ts +++ b/src/DraupnirBotMode.ts @@ -11,7 +11,6 @@ import { StandardClientsInRoomMap, DefaultEventDecoder, - setGlobalLoggerProvider, RoomStateBackingStore, ClientsInRoomMap, Task, @@ -21,7 +20,6 @@ import { ConfigRecoverableError, } from "matrix-protection-suite"; import { - BotSDKLogServiceLogger, ClientCapabilityFactory, MatrixSendClient, RoomStateManagerFactory, @@ -51,8 +49,6 @@ import { ResultError } from "@gnuxie/typescript-result"; import { SafeModeCause, SafeModeReason } from "./safemode/SafeModeCause"; import { SafeModeBootOption } from "./safemode/BootOption"; -setGlobalLoggerProvider(new BotSDKLogServiceLogger()); - const log = new Logger("DraupnirBotMode"); export function constructWebAPIs(draupnir: Draupnir): WebAPIs { diff --git a/src/config.ts b/src/config.ts index 61c29e2..b3b91fa 100644 --- a/src/config.ts +++ b/src/config.ts @@ -10,10 +10,35 @@ import * as fs from "fs"; import { load } from "js-yaml"; -import { MatrixClient, LogService } from "matrix-bot-sdk"; +import { LogService, RichConsoleLogger } from "matrix-bot-sdk"; import Config from "config"; import path from "path"; import { SafeModeBootOption } from "./safemode/BootOption"; +import { Logger, setGlobalLoggerProvider } from "matrix-protection-suite"; + +LogService.setLogger(new RichConsoleLogger()); +setGlobalLoggerProvider(new RichConsoleLogger()); +const log = new Logger("Draupnir config"); + +/** + * The version of the configuration that has been explicitly provided, + * and does not contain default values. Secrets are marked with "REDACTED". + */ +export function getNonDefaultConfigProperties( + config: IConfig +): Record { + const nonDefault = Config.util.diffDeep(defaultConfig, config); + if ("accessToken" in nonDefault) { + nonDefault.accessToken = "REDACTED"; + } + if ( + "pantalaimon" in nonDefault && + typeof nonDefault.pantalaimon === "object" + ) { + nonDefault.pantalaimon.password = "REDACTED"; + } + return nonDefault; +} /** * The configuration, as read from production.yaml @@ -64,9 +89,11 @@ export interface IConfig { * should be printed to our managementRoom. */ displayReports: boolean; - admin?: { - enableMakeRoomAdminCommand?: boolean; - }; + admin?: + | { + enableMakeRoomAdminCommand?: boolean; + } + | undefined; commands: { allowNoPrefix: boolean; additionalPrefixes: string[]; @@ -103,15 +130,17 @@ export interface IConfig { unhealthyStatus: number; }; // If specified, attempt to upload any crash statistics to sentry. - sentry?: { - dsn: string; + sentry?: + | { + dsn: string; - // Frequency of performance monitoring. - // - // A number in [0.0, 1.0], where 0.0 means "don't bother with tracing" - // and 1.0 means "trace performance at every opportunity". - tracesSampleRate: number; - }; + // Frequency of performance monitoring. + // + // A number in [0.0, 1.0], where 0.0 means "don't bother with tracing" + // and 1.0 means "trace performance at every opportunity". + tracesSampleRate: number; + } + | undefined; }; web: { enabled: boolean; @@ -130,13 +159,19 @@ export interface IConfig { // This can not be used with Pantalaimon. experimentalRustCrypto: boolean; - /** - * Config options only set at runtime. Try to avoid using the objects - * here as much as possible. - */ - RUNTIME: { - client?: MatrixClient; - }; + configMeta: + | { + /** + * The path that the configuration file was loaded from. + */ + configPath: string; + + isDraupnirConfigOptionUsed: boolean; + + isAccessTokenPathOptionUsed: boolean; + isPasswordPathOptionUsed: boolean; + } + | undefined; } const defaultConfig: IConfig = { @@ -204,7 +239,9 @@ const defaultConfig: IConfig = { healthyStatus: 200, unhealthyStatus: 418, }, + sentry: undefined, }, + admin: undefined, web: { enabled: false, port: 8080, @@ -217,37 +254,95 @@ const defaultConfig: IConfig = { enabled: false, }, experimentalRustCrypto: false, - - // Needed to make the interface happy. - RUNTIME: {}, + configMeta: undefined, }; export function getDefaultConfig(): IConfig { return Config.util.cloneDeep(defaultConfig); } +function logNonDefaultConfiguration(config: IConfig): void { + log.info( + "non-default configuration properties:", + JSON.stringify(getNonDefaultConfigProperties(config), null, 2) + ); +} + +function logConfigMeta(config: IConfig): void { + log.info("Configuration meta:", JSON.stringify(config.configMeta, null, 2)); +} + +function getConfigPath(): { + isDraupnirPath: boolean; + path: string; +} { + const draupnirPath = getCommandLineOption(process.argv, "--draupnir-config"); + if (draupnirPath) { + return { isDraupnirPath: true, path: draupnirPath }; + } + const mjolnirPath = getCommandLineOption(process.argv, "--mjolnir-config"); + if (mjolnirPath) { + return { isDraupnirPath: false, path: mjolnirPath }; + } + const path = Config.util.getConfigSources().at(-1)?.name; + if (path === undefined) { + throw new TypeError("No configuration path has been found for Draupnir"); + } + return { isDraupnirPath: false, path }; +} + +function getConfigMeta(): NonNullable { + const { isDraupnirPath, path } = getConfigPath(); + return { + configPath: path, + isDraupnirConfigOptionUsed: isDraupnirPath, + isAccessTokenPathOptionUsed: isCommandLineOptionPresent( + process.argv, + "--access-token-path" + ), + isPasswordPathOptionUsed: isCommandLineOptionPresent( + process.argv, + "--pantalaimon-password-path" + ), + }; +} + /** * @returns The users's raw config, deep copied over the `defaultConfig`. */ function readConfigSource(): IConfig { - const explicitConfigPath = getCommandLineOption( - process.argv, - "--draupnir-config" - ); - if (explicitConfigPath !== undefined) { - const content = fs.readFileSync(explicitConfigPath, "utf8"); + const configMeta = getConfigMeta(); + const config = (() => { + const content = fs.readFileSync(configMeta.configPath, "utf8"); const parsed = load(content); - return Config.util.extendDeep({}, defaultConfig, parsed); - } else { - return Config.util.extendDeep( - {}, - defaultConfig, - Config.util.toObject() - ) as IConfig; + return Config.util.extendDeep({}, defaultConfig, parsed, { + configMeta: configMeta, + }) as IConfig; + })(); + logConfigMeta(config); + if (!configMeta.isDraupnirConfigOptionUsed) { + log.warn( + "DEPRECATED", + "Starting Draupnir without the --draupnir-config option is deprecated. Please provide Draupnir's configuration explicitly with --draupnir-config.", + "config path used:", + config.configMeta?.configPath + ); } + const unknownProperties = getUnknownConfigPropertyPaths(config); + if (unknownProperties.length > 0) { + log.warn( + "There are unknown configuration properties, possibly a result of typos:", + unknownProperties + ); + } + process.on("exit", () => { + logNonDefaultConfiguration(config); + logConfigMeta(config); + }); + return config; } -export function read(): IConfig { +export function configRead(): IConfig { const config = readConfigSource(); const explicitAccessTokenPath = getCommandLineOption( process.argv, @@ -290,7 +385,7 @@ export function getProvisionedMjolnirConfig(managementRoomId: string): IConfig { "backgroundDelayMS", "safeMode", ]; - const configTemplate = read(); // we use the standard bot config as a template for every provisioned draupnir. + const configTemplate = configRead(); // we use the standard bot config as a template for every provisioned draupnir. const unusedKeys = Object.keys(configTemplate).filter( (key) => !allowedKeys.includes(key) ); @@ -391,3 +486,58 @@ function getCommandLineOption( // No value was provided, or the next argument is another option throw new Error(`No value provided for ${optionName}`); } + +type UnknownPropertyPaths = string[]; + +export function getUnknownPropertiesHelper( + rawConfig: unknown, + rawDefaults: unknown, + currentPathProperties: string[] +): UnknownPropertyPaths { + const unknownProperties: UnknownPropertyPaths = []; + if ( + typeof rawConfig !== "object" || + rawConfig === null || + Array.isArray(rawConfig) + ) { + return unknownProperties; + } + if (rawDefaults === undefined || rawDefaults == null) { + // the top level property should have been defined, these could be and + // probably are custom properties. + return unknownProperties; + } + if (typeof rawDefaults !== "object") { + throw new TypeError("default and normal config are out of sync"); + } + const defaultConfig = rawDefaults as Record; + const config = rawConfig as Record; + for (const key of Object.keys(config)) { + if (!(key in defaultConfig)) { + unknownProperties.push("/" + [...currentPathProperties, key].join("/")); + } else { + const unknownSubProperties = getUnknownPropertiesHelper( + config[key], + defaultConfig[key] as Record, + [...currentPathProperties, key] + ); + unknownProperties.push(...unknownSubProperties); + } + } + return unknownProperties; +} + +/** + * Return a list of JSON paths to properties in the given config object that are not present in the default config. + * This is used to detect typos in the config file. + */ +export function getUnknownConfigPropertyPaths(config: unknown): string[] { + if (typeof config !== "object" || config === null) { + return []; + } + return getUnknownPropertiesHelper( + config, + defaultConfig as unknown as Record, + [] + ); +} diff --git a/src/index.ts b/src/index.ts index 7b4903d..94f07fc 100644 --- a/src/index.ts +++ b/src/index.ts @@ -15,12 +15,11 @@ import { LogService, MatrixClient, PantalaimonClient, - RichConsoleLogger, SimpleFsStorageProvider, RustSdkCryptoStorageProvider, } from "matrix-bot-sdk"; import { StoreType } from "@matrix-org/matrix-sdk-crypto-nodejs"; -import { read as configRead } from "./config"; +import { configRead as configRead } from "./config"; import { initializeSentry, patchMatrixClient } from "./utils"; import { DraupnirBotModeToggle } from "./DraupnirBotMode"; import { SafeMatrixEmitterWrapper } from "matrix-protection-suite-for-matrix-bot-sdk"; @@ -30,9 +29,6 @@ import { SqliteRoomStateBackingStore } from "./backingstore/better-sqlite3/Sqlit void (async function () { const config = configRead(); - config.RUNTIME = {}; - - LogService.setLogger(new RichConsoleLogger()); LogService.setLevel(LogLevel.fromString(config.logLevel, LogLevel.DEBUG)); LogService.info("index", "Starting bot..."); @@ -48,6 +44,7 @@ void (async function () { } let bot: DraupnirBotModeToggle | null = null; + let client: MatrixClient; try { const storagePath = path.isAbsolute(config.dataPath) ? config.dataPath @@ -56,7 +53,6 @@ void (async function () { path.join(storagePath, "bot.json") ); - let client: MatrixClient; if (config.pantalaimon.use && !config.experimentalRustCrypto) { const pantalaimon = new PantalaimonClient(config.homeserverUrl, storage); client = await pantalaimon.createClientWithCredentials( @@ -88,7 +84,6 @@ void (async function () { ); } patchMatrixClient(); - config.RUNTIME.client = client; const eventDecoder = DefaultEventDecoder; const store = config.roomStateBackingStore.enabled ? new SqliteRoomStateBackingStore( @@ -115,7 +110,7 @@ void (async function () { throw err; } try { - await config.RUNTIME.client.start(); + await client.start(); await bot.encryptionInitialized(); healthz.isHealthy = true; } catch (err) { diff --git a/test/integration/fixtures.ts b/test/integration/fixtures.ts index a3c94a7..969c15e 100644 --- a/test/integration/fixtures.ts +++ b/test/integration/fixtures.ts @@ -12,7 +12,7 @@ import { MJOLNIR_PROTECTED_ROOMS_EVENT_TYPE, MJOLNIR_WATCHED_POLICY_ROOMS_EVENT_TYPE, } from "matrix-protection-suite"; -import { read as configRead } from "../../src/config"; +import { configRead } from "../../src/config"; import { patchMatrixClient } from "../../src/utils"; import { DraupnirTestContext, @@ -52,7 +52,6 @@ export const mochaHooks = { if (draupnirMatrixClient === null) { throw new TypeError(`setup code is broken`); } - config.RUNTIME.client = draupnirMatrixClient; await draupnirClient()?.start(); await this.toggle.encryptionInitialized(); console.log("mochaHooks.beforeEach DONE"); diff --git a/test/integration/manualLaunchScript.ts b/test/integration/manualLaunchScript.ts index 313ddfc..f94a373 100644 --- a/test/integration/manualLaunchScript.ts +++ b/test/integration/manualLaunchScript.ts @@ -13,7 +13,7 @@ */ import { draupnirClient, makeBotModeToggle } from "./mjolnirSetupUtils"; -import { read as configRead } from "../../src/config"; +import { configRead } from "../../src/config"; import { SqliteRoomStateBackingStore } from "../../src/backingstore/better-sqlite3/SqliteRoomStateBackingStore"; import path from "path"; import { DefaultEventDecoder } from "matrix-protection-suite"; diff --git a/test/integration/mjolnirSetupUtils.ts b/test/integration/mjolnirSetupUtils.ts index ad6801c..2bec3e0 100644 --- a/test/integration/mjolnirSetupUtils.ts +++ b/test/integration/mjolnirSetupUtils.ts @@ -14,7 +14,6 @@ import { MemoryStorageProvider, LogService, LogLevel, - RichConsoleLogger, } from "matrix-bot-sdk"; import { overrideRatelimitForUser, registerUser } from "./clientHelper"; import { initializeSentry, patchMatrixClient } from "../../src/utils"; @@ -137,7 +136,6 @@ export async function makeBotModeToggle( } = {} ): Promise { await configureMjolnir(config); - LogService.setLogger(new RichConsoleLogger()); LogService.setLevel(LogLevel.fromString(config.logLevel, LogLevel.DEBUG)); LogService.info("test/mjolnirSetupUtils", "Starting bot..."); const pantalaimon = new PantalaimonClient( diff --git a/test/unit/config/unknownPropertiesTest.ts b/test/unit/config/unknownPropertiesTest.ts new file mode 100644 index 0000000..4820c42 --- /dev/null +++ b/test/unit/config/unknownPropertiesTest.ts @@ -0,0 +1,41 @@ +// SPDX-FileCopyrightText: 2024 Gnuxie +// +// SPDX-License-Identifier: AFL-3.0 + +import expect from "expect"; +import { + getNonDefaultConfigProperties, + getUnknownConfigPropertyPaths, +} from "../../../src/config"; +import { IConfig } from "../../../src/config"; + +describe("Test unknown properties detection", () => { + it("Should detect when there are typos in the config", function () { + const config = { + pantalaimon: { + use: true, + passweird: "my password hehe", + }, + }; + const unknownProperties = getUnknownConfigPropertyPaths(config); + expect(unknownProperties.length).toBe(1); + expect(unknownProperties[0]).toBe("/pantalaimon/passweird"); + }); +}); + +describe("Test non-default values detection", () => { + it("Should detect when there are non-default values in the config", function () { + const config = { + pantalaimon: { + use: true, + password: "my password hehe", + }, + }; + const differentProperties = getNonDefaultConfigProperties( + config as IConfig + ) as unknown as IConfig; + expect(Object.entries(differentProperties).length).toBe(1); + expect(differentProperties.pantalaimon.password).toBe("REDACTED"); + expect(differentProperties.pantalaimon.use).toBe(true); + }); +});