From d4f701c8718a108026a4a51ae71fbeb6d2241a85 Mon Sep 17 00:00:00 2001 From: Andrew Ferrazzutti Date: Wed, 13 Jul 2022 08:39:23 -0400 Subject: [PATCH] Disallow some empty config URL settings (#412) * Disallow some empty config URL settings If these settings are meant to be unspecified, then their entire parent sections (`widgets` & `generic`) should be unspecified. Signed-off-by: Andrew Ferrazzutti * Convert some config URL strings to URL objects This allows both parsing and easier crafting of relative URLs. Signed-off-by: Andrew Ferrazzutti * Dump parsed URLs to default config Also implement getters to return stringified URLs, instead of having to store a URL's string representation directly. Signed-off-by: Andrew Ferrazzutti --- changelog.d/412.bugfix | 1 + config.sample.yml | 2 +- src/Bridge.ts | 2 +- src/Config/Config.ts | 34 ++++++++++++++++++++++-------- src/Config/Defaults.ts | 7 +++++- src/Connections/GenericHook.ts | 4 ++-- src/Connections/SetupConnection.ts | 2 +- src/Widgets/SetupWidget.ts | 2 +- 8 files changed, 38 insertions(+), 16 deletions(-) create mode 100644 changelog.d/412.bugfix diff --git a/changelog.d/412.bugfix b/changelog.d/412.bugfix new file mode 100644 index 00000000..c34a4796 --- /dev/null +++ b/changelog.d/412.bugfix @@ -0,0 +1 @@ +Disallow empty and invalid values for the `widgets.publicUrl` and `generic.urlPrefix` configuration settings. diff --git a/config.sample.yml b/config.sample.yml index 304e6d3d..9d965209 100644 --- a/config.sample.yml +++ b/config.sample.yml @@ -144,7 +144,7 @@ widgets: - fec0::/10 roomSetupWidget: addOnInvite: false - publicUrl: http://example.com/widgetapi/v1/static + publicUrl: http://example.com/widgetapi/v1/static/ branding: widgetTitle: Hookshot Configuration permissions: diff --git a/src/Bridge.ts b/src/Bridge.ts index ad650083..e97327d7 100644 --- a/src/Bridge.ts +++ b/src/Bridge.ts @@ -1183,7 +1183,7 @@ export class Bridge { return this.as.botClient.inviteUser(adminRoom.userId, newConnection.roomId); }); this.adminRooms.set(roomId, adminRoom); - if (this.config.widgets?.addToAdminRooms && this.config.widgets.publicUrl) { + if (this.config.widgets?.addToAdminRooms) { await SetupWidget.SetupAdminRoomConfigWidget(roomId, this.as.botIntent, this.config.widgets); } log.debug(`Set up ${roomId} as an admin room for ${adminRoom.userId}`); diff --git a/src/Config/Config.ts b/src/Config/Config.ts index fe352af4..835bb934 100644 --- a/src/Config/Config.ts +++ b/src/Config/Config.ts @@ -13,6 +13,10 @@ import { GITHUB_CLOUD_URL } from "../Github/GithubInstance"; const log = new LogWrapper("Config"); +function makePrefixedUrl(urlString: string): URL { + return new URL(urlString.endsWith("/") ? urlString : urlString + "/"); +} + export const ValidLogLevelStrings = [ LogLevel.ERROR.toString(), LogLevel.WARN.toString(), @@ -261,18 +265,24 @@ export interface BridgeGenericWebhooksConfigYAML { export class BridgeConfigGenericWebhooks { public readonly enabled: boolean; - public readonly urlPrefix: string; + + @hideKey() + public readonly parsedUrlPrefix: URL; + public readonly urlPrefix: () => string; + public readonly userIdPrefix?: string; public readonly allowJsTransformationFunctions?: boolean; public readonly waitForComplete?: boolean; public readonly enableHttpGet: boolean; constructor(yaml: BridgeGenericWebhooksConfigYAML) { - if (typeof yaml.urlPrefix !== "string") { - throw new ConfigError("generic.urlPrefix", "is not defined or not a string"); - } this.enabled = yaml.enabled || false; this.enableHttpGet = yaml.enableHttpGet || false; - this.urlPrefix = yaml.urlPrefix; + try { + this.parsedUrlPrefix = makePrefixedUrl(yaml.urlPrefix); + this.urlPrefix = () => { return this.parsedUrlPrefix.href; } + } catch (err) { + throw new ConfigError("generic.urlPrefix", "is not defined or not a valid URL"); + } this.userIdPrefix = yaml.userIdPrefix; this.allowJsTransformationFunctions = yaml.allowJsTransformationFunctions; this.waitForComplete = yaml.waitForComplete; @@ -305,7 +315,11 @@ interface BridgeWidgetConfigYAML { export class BridgeWidgetConfig { public readonly addToAdminRooms: boolean; - public readonly publicUrl: string; + + @hideKey() + public readonly parsedPublicUrl: URL; + public readonly publicUrl: () => string; + public readonly roomSetupWidget?: { addOnInvite?: boolean; }; @@ -323,10 +337,12 @@ export class BridgeWidgetConfig { if (yaml.disallowedIpRanges !== undefined && (!Array.isArray(yaml.disallowedIpRanges) || !yaml.disallowedIpRanges.every(s => typeof s === "string"))) { throw new ConfigError("widgets.disallowedIpRanges", "must be a string array"); } - if (typeof yaml.publicUrl !== "string") { - throw new ConfigError("widgets.publicUrl", "is not defined or not a string"); + try { + this.parsedPublicUrl = makePrefixedUrl(yaml.publicUrl) + this.publicUrl = () => { return this.parsedPublicUrl.href; } + } catch (err) { + throw new ConfigError("widgets.publicUrl", "is not defined or not a valid URL"); } - this.publicUrl = yaml.publicUrl; this.branding = yaml.branding || { widgetTitle: "Hookshot Configuration" }; diff --git a/src/Config/Defaults.ts b/src/Config/Defaults.ts index 46bbaec7..5b8d8d77 100644 --- a/src/Config/Defaults.ts +++ b/src/Config/Defaults.ts @@ -143,11 +143,16 @@ function renderSection(doc: YAML.Document, obj: Record, parentN if (keyIsHidden(obj, key)) { return; } - + let newNode: Node; if (typeof value === "object" && !Array.isArray(value)) { newNode = YAML.createNode({}); renderSection(doc, value as Record, newNode as YAMLSeq); + } else if (typeof value === "function") { + if (value.length !== 0) { + throw Error("Only zero-argument functions are allowed as config values"); + } + newNode = YAML.createNode(value()); } else { newNode = YAML.createNode(value); } diff --git a/src/Connections/GenericHook.ts b/src/Connections/GenericHook.ts index 4a3c54c1..9f5902a7 100644 --- a/src/Connections/GenericHook.ts +++ b/src/Connections/GenericHook.ts @@ -27,7 +27,7 @@ export interface GenericHookSecrets { /** * The public URL for the webhook. */ - url: string; + url: URL; /** * The hookId of the webhook. */ @@ -404,7 +404,7 @@ export class GenericHookConnection extends BaseConnection implements IConnection name: this.state.name, }, ...(showSecrets ? { secrets: { - url: `${this.config.urlPrefix}${this.config.urlPrefix.endsWith('/') ? '' : '/'}${this.hookId}`, + url: new URL(this.hookId, this.config.parsedUrlPrefix), hookId: this.hookId, } as GenericHookSecrets} : undefined) } diff --git a/src/Connections/SetupConnection.ts b/src/Connections/SetupConnection.ts index bbf62c9f..a66204e5 100644 --- a/src/Connections/SetupConnection.ts +++ b/src/Connections/SetupConnection.ts @@ -139,7 +139,7 @@ export class SetupConnection extends CommandConnection { throw new CommandError("Bad webhook name", "A webhook name must be between 3-64 characters."); } const c = await GenericHookConnection.provisionConnection(this.roomId, userId, {name}, this.provisionOpts); - const url = `${this.config.generic.urlPrefix}${this.config.generic.urlPrefix.endsWith('/') ? '' : '/'}${c.connection.hookId}`; + const url = new URL(c.connection.hookId, this.config.generic.parsedUrlPrefix); const adminRoom = await this.getOrCreateAdminRoom(userId); await adminRoom.sendNotice(`You have bridged a webhook. Please configure your webhook source to use ${url}.`); return this.as.botClient.sendNotice(this.roomId, `Room configured to bridge webhooks. See admin room for secret url.`); diff --git a/src/Widgets/SetupWidget.ts b/src/Widgets/SetupWidget.ts index b2398b51..a9d83eb0 100644 --- a/src/Widgets/SetupWidget.ts +++ b/src/Widgets/SetupWidget.ts @@ -54,7 +54,7 @@ export class SetupWidget { "id": stateKey, "name": config.branding.widgetTitle, "type": "m.custom", - "url": `${config?.publicUrl}/#/?kind=${kind}&roomId=$matrix_room_id&widgetId=$matrix_widget_id`, + "url": new URL(`#/?kind=${kind}&roomId=$matrix_room_id&widgetId=$matrix_widget_id`, config.parsedPublicUrl).href, "waitForIframeLoad": true, } );