api/cannon: Re-fix local IP check with better error handling

packages/api/src/app-router.ts

@@ -53,6 +53,9 @@ const PROM_BUNDLE_OPTS: promBundle.Opts = {
   },
 };
 
+const isTest =
+  process.env.NODE_ENV === "test" || process.env.NODE_ENV === "development";
+
 export default async function makeApp(params: CliArgs) {
   const {
     httpPrefix,
@@ -122,15 +125,12 @@ export default async function makeApp(params: CliArgs) {
     recordCatalystObjectStoreId,
     secondaryRecordObjectStoreId,
     supportAddr,
-    verifyUrls: true,
+    skipUrlVerification: isTest,
     queue,
   });
   await webhookCannon.start();
 
-  if (
-    process.env.NODE_ENV === "test" ||
-    process.env.NODE_ENV === "development"
-  ) {
+  if (isTest) {
     await setupTestTus();
   } else if (vodObjectStoreId) {
     await setupTus(vodObjectStoreId);

packages/api/src/store/webhook-table.ts

@@ -49,12 +49,14 @@ export default class WebhookTable extends Table<DBWebhook> {
   }
 
   async updateStatus(id: string, status: DBWebhook["status"]) {
+    const statusStr = JSON.stringify(status);
     const res = await this.db.query(
-      `UPDATE ${
-        this.name
-      } SET data = jsonb_set(data, '{status}', case when data->'status' is null then '{}' else data->'status' end || '${JSON.stringify(
-        status
-      )}') WHERE id = '${id}'`
+      sql``
+        .append(`UPDATE ${this.name} `) // table name can't be parameterized, append a raw string
+        .append(
+          sql`SET data = jsonb_set(data, '{status}', case when data->'status' is null then '{}' else data->'status' end || ${statusStr}) `
+        )
+        .append(sql`WHERE id = ${id}`)
     );
 
     if (res.rowCount < 1) {

packages/api/src/webhooks/cannon.ts

@@ -1,17 +1,17 @@
 import { ConsumeMessage } from "amqplib";
-import { promises as dns } from "dns";
+import { isIP } from "net";
+import dns from "dns";
 import isLocalIP from "is-local-ip";
 import { Response } from "node-fetch";
 import { v4 as uuid } from "uuid";
 import { parse as parseUrl } from "url";
-import { DB } from "../store/db";
 import { DBSession } from "../store/session-table";
 import messages from "../store/messages";
 import Queue from "../store/queue";
 import { DBWebhook } from "../store/webhook-table";
 import { fetchWithTimeout, RequestInitWithTimeout, sleep } from "../util";
 import logger from "../logger";
-import { sign, sendgridEmail, pathJoin } from "../controllers/helpers";
+import { sign, sendgridEmail } from "../controllers/helpers";
 import { taskScheduler } from "../task/scheduler";
 import { generateUniquePlaybackId } from "../controllers/generate-keys";
 import { createAsset, primaryStorageExperiment } from "../controllers/asset";
@@ -42,7 +42,7 @@ function isRuntimeError(err: any): boolean {
 
 export default class WebhookCannon {
   running: boolean;
-  verifyUrls: boolean;
+  skipUrlVerification: boolean;
   frontendDomain: string;
   sendgridTemplateId: string;
   sendgridApiKey: string;
@@ -51,7 +51,7 @@ export default class WebhookCannon {
   secondaryVodObjectStoreId: string;
   recordCatalystObjectStoreId: string;
   secondaryRecordObjectStoreId: string;
-  resolver: any;
+  resolver: dns.promises.Resolver;
   queue: Queue;
   constructor({
     frontendDomain,
@@ -62,11 +62,11 @@ export default class WebhookCannon {
     secondaryVodObjectStoreId,
     recordCatalystObjectStoreId,
     secondaryRecordObjectStoreId,
-    verifyUrls,
+    skipUrlVerification,
     queue,
   }) {
     this.running = true;
-    this.verifyUrls = verifyUrls;
+    this.skipUrlVerification = skipUrlVerification;
     this.frontendDomain = frontendDomain;
     this.sendgridTemplateId = sendgridTemplateId;
     this.sendgridApiKey = sendgridApiKey;
@@ -75,7 +75,7 @@ export default class WebhookCannon {
     this.secondaryVodObjectStoreId = secondaryVodObjectStoreId;
     this.recordCatalystObjectStoreId = recordCatalystObjectStoreId;
     this.secondaryRecordObjectStoreId = secondaryRecordObjectStoreId;
-    this.resolver = new dns.Resolver();
+    this.resolver = new dns.promises.Resolver();
     this.queue = queue;
     // this.start();
   }
@@ -208,8 +208,7 @@ export default class WebhookCannon {
       return;
     }
     try {
-      // TODO Activate URL Verification
-      await this._fireHook(trigger, false);
+      await this._fireHook(trigger);
     } catch (err) {
       console.log("_fireHook error", err);
       await this.retry(trigger, null, err);
@@ -223,10 +222,6 @@ export default class WebhookCannon {
     this.running = false;
   }
 
-  disableUrlVerify() {
-    this.verifyUrls = false;
-  }
-
   public calcBackoff = (lastInterval?: number): number => {
     if (!lastInterval || lastInterval < 1000) {
       return 5000;
@@ -328,7 +323,7 @@ export default class WebhookCannon {
     );
   }
 
-  async _fireHook(trigger: messages.WebhookTrigger, verifyUrl = true) {
+  async _fireHook(trigger: messages.WebhookTrigger) {
     const { event, webhook, stream, user } = trigger;
     if (!event || !webhook || !user) {
       console.error(
@@ -338,34 +333,15 @@ export default class WebhookCannon {
       return;
     }
     console.log(`trying webhook ${webhook.name}: ${webhook.url}`);
-    let ips, urlObj, isLocal;
-    if (verifyUrl) {
-      try {
-        urlObj = parseUrl(webhook.url);
-        if (urlObj.host) {
-          ips = await this.resolver.resolve4(urlObj.hostname);
-        }
-      } catch (e) {
-        console.error("error: ", e);
-        throw e;
-      }
-    }
 
-    // This is mainly useful for local testing
-    if (user.admin || verifyUrl === false) {
-      isLocal = false;
-    } else {
-      try {
-        if (ips && ips.length) {
-          isLocal = isLocalIP(ips[0]);
-        } else {
-          isLocal = true;
-        }
-      } catch (e) {
-        console.error("isLocal Error", isLocal, e);
-        throw e;
-      }
-    }
+    const { ips, isLocal } = await this.checkIsLocalIp(
+      webhook.url,
+      user.admin
+    ).catch((e) => {
+      console.error("error checking if is local IP: ", e);
+      return { ips: [], isLocal: false };
+    });
+
     if (isLocal) {
       // don't fire this webhook.
       console.log(
@@ -464,6 +440,36 @@ export default class WebhookCannon {
     }
   }
 
+  private async checkIsLocalIp(url: string, isAdmin: boolean) {
+    if (isAdmin || this.skipUrlVerification) {
+      // this is mainly useful for local testing
+      return { ips: [], isLocal: false };
+    }
+
+    const emptyIfNotFound = (err) => {
+      if ([dns.NODATA, dns.NOTFOUND, dns.BADFAMILY].includes(err.code)) {
+        return [] as string[];
+      }
+      throw err;
+    };
+
+    const { hostname } = parseUrl(url);
+    if (["localhost", "ip6-localhost", "ip6-loopback"].includes(hostname)) {
+      // dns.resolve functions do not take /etc/hosts into account, so we need to handle these separately
+      return { ips: ["127.0.0.1"], isLocal: true };
+    }
+
+    const ips = isIP(hostname)
+      ? [hostname]
+      : await Promise.all([
+          this.resolver.resolve4(hostname).catch(emptyIfNotFound),
+          this.resolver.resolve6(hostname).catch(emptyIfNotFound),
+        ]).then((ipsArrs) => ipsArrs.flat());
+
+    const isLocal = ips.some(isLocalIP);
+    return { ips, isLocal };
+  }
+
   async storeTriggerStatus(
     webhook: DBWebhook,
     triggerTime: number,