api: Enable local IP verification on webhooks (#2118)

* api: Enable URL verification on webhooks * api: Simplify the isLocal checks * api: Fix log * api: Skip URL verification on tests * api: Resolve both IPv4 and IPv6 * api: Consider local if any record is local Otherwise you could create a domain that resolves both to a public and a private IP and we would allow it to passthrough.
livepeer · Apr 2, 2024 · 2c11962 · 2c11962
1 parent a8e5672
commit 2c11962
Show file tree

Hide file tree

Showing 2 changed files with 23 additions and 38 deletions.
diff --git a/packages/api/src/app-router.ts b/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);

diff --git a/packages/api/src/webhooks/cannon.ts b/packages/api/src/webhooks/cannon.ts
@@ -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.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;
@@ -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,24 @@ 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 {
+    let ips: string[];
+    let isLocal = false;
+    // These conditions are mainly useful for local testing
+    if (!user.admin && !this.skipUrlVerification) {
       try {
-        if (ips && ips.length) {
-          isLocal = isLocalIP(ips[0]);
-        } else {
-          isLocal = true;
-        }
+        const urlObj = parseUrl(webhook.url);
+        ips = await Promise.all([
+          this.resolver.resolve4(urlObj.hostname),
+          this.resolver.resolve6(urlObj.hostname),
+        ]).then((ipsArrs) => ipsArrs.flat());
+        isLocal = ips.some(isLocalIP);
       } catch (e) {
-        console.error("isLocal Error", isLocal, e);
+        console.error("error checking if is local IP: ", e);
         throw e;
       }
     }
+
     if (isLocal) {
       // don't fire this webhook.
       console.log(