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 <andrewf@element.io>

* Convert some config URL strings to URL objects

This allows both parsing and easier crafting of relative URLs.

Signed-off-by: Andrew Ferrazzutti <andrewf@element.io>

* 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 <andrewf@element.io>

changelog.d/412.bugfix

@@ -0,0 +1 @@
+Disallow empty and invalid values for the `widgets.publicUrl` and `generic.urlPrefix` configuration settings.

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:

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}`);

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"
         };

src/Config/Defaults.ts

@@ -143,11 +143,16 @@ function renderSection(doc: YAML.Document, obj: Record<string, unknown>, parentN
         if (keyIsHidden(obj, key)) {
             return;
         }
-        
+
         let newNode: Node;
         if (typeof value === "object" && !Array.isArray(value)) {
             newNode = YAML.createNode({});
             renderSection(doc, value as Record<string, unknown>, 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);
         }

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)
         }

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.`);

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,
             }
         );