From 450051e541fa0059c8da308b3fb8a5a4c14fe356 Mon Sep 17 00:00:00 2001
From: Scott Nonnenberg <scott@signal.org>
Date: Mon, 19 Sep 2022 13:42:37 -0700
Subject: [PATCH] Only process signal domain links if they have hash/path/query

---
 ts/test-node/util/sgnlHref_test.ts | 66 ++++++++++++++++++++++++++----
 ts/util/sgnlHref.ts                |  5 ++-
 2 files changed, 61 insertions(+), 10 deletions(-)

diff --git a/ts/test-node/util/sgnlHref_test.ts b/ts/test-node/util/sgnlHref_test.ts
index 5ba29c4fe..bddadac45 100644
--- a/ts/test-node/util/sgnlHref_test.ts
+++ b/ts/test-node/util/sgnlHref_test.ts
@@ -121,16 +121,32 @@ describe('sgnlHref', () => {
     });
 
     it('returns false if the protocol is not "https:"', () => {
-      assert.isFalse(isSignalHttpsLink('sgnl://signal.art', explodingLogger));
       assert.isFalse(
         isSignalHttpsLink(
-          'sgnl://signal.art/addstickers/?pack_id=abc',
+          'sgnl://signal.art/#pack_id=234234&pack_key=342342',
           explodingLogger
         )
       );
       assert.isFalse(
-        isSignalHttpsLink('signal://signal.group', explodingLogger)
+        isSignalHttpsLink(
+          'sgnl://signal.art/addstickers/#pack_id=234234&pack_key=342342',
+          explodingLogger
+        )
       );
+      assert.isFalse(
+        isSignalHttpsLink(
+          'signal://signal.group/#AD234Dq342dSDJWE',
+          explodingLogger
+        )
+      );
+    });
+
+    it('returns false if missing path/hash/query', () => {
+      assert.isFalse(
+        isSignalHttpsLink('https://signal.group/', explodingLogger)
+      );
+      assert.isFalse(isSignalHttpsLink('https://signal.art/', explodingLogger));
+      assert.isFalse(isSignalHttpsLink('https://signal.me/', explodingLogger));
     });
 
     it('returns false if the URL is not a valid Signal URL', () => {
@@ -139,10 +155,39 @@ describe('sgnlHref', () => {
     });
 
     it('returns true if the protocol is "https:"', () => {
-      assert.isTrue(isSignalHttpsLink('https://signal.group', explodingLogger));
-      assert.isTrue(isSignalHttpsLink('https://signal.art', explodingLogger));
-      assert.isTrue(isSignalHttpsLink('HTTPS://signal.art', explodingLogger));
-      assert.isTrue(isSignalHttpsLink('https://signal.me', explodingLogger));
+      assert.isTrue(
+        isSignalHttpsLink(
+          'https://signal.group/#AD234Dq342dSDJWE',
+          explodingLogger
+        )
+      );
+      assert.isTrue(
+        isSignalHttpsLink(
+          'https://signal.group/AD234Dq342dSDJWE',
+          explodingLogger
+        )
+      );
+      assert.isTrue(
+        isSignalHttpsLink(
+          'https://signal.group/?AD234Dq342dSDJWE',
+          explodingLogger
+        )
+      );
+      assert.isTrue(
+        isSignalHttpsLink(
+          'https://signal.art/addstickers/#pack_id=234234&pack_key=342342',
+          explodingLogger
+        )
+      );
+      assert.isTrue(
+        isSignalHttpsLink(
+          'HTTPS://signal.art/addstickers/#pack_id=234234&pack_key=342342',
+          explodingLogger
+        )
+      );
+      assert.isTrue(
+        isSignalHttpsLink('https://signal.me/#p/+32423432', explodingLogger)
+      );
     });
 
     it('returns false if username or password are set', () => {
@@ -153,14 +198,17 @@ describe('sgnlHref', () => {
 
     it('returns false if port is set', () => {
       assert.isFalse(
-        isSignalHttpsLink('https://signal.group:1234', explodingLogger)
+        isSignalHttpsLink(
+          'https://signal.group:1234/#AD234Dq342dSDJWE',
+          explodingLogger
+        )
       );
     });
 
     it('accepts URL objects', () => {
       const invalid = new URL('sgnl://example.com');
       assert.isFalse(isSignalHttpsLink(invalid, explodingLogger));
-      const valid = new URL('https://signal.art');
+      const valid = new URL('https://signal.art/#AD234Dq342dSDJWE');
       assert.isTrue(isSignalHttpsLink(valid, explodingLogger));
     });
   });
diff --git a/ts/util/sgnlHref.ts b/ts/util/sgnlHref.ts
index 3bcf95ce9..0e9efc3d6 100644
--- a/ts/util/sgnlHref.ts
+++ b/ts/util/sgnlHref.ts
@@ -34,6 +34,8 @@ export function isCaptchaHref(
   return Boolean(url?.protocol === 'signalcaptcha:');
 }
 
+// A link to a signal 'action' domain with private data in path/hash/query. We could
+//   open a browser, but it will just link back to us. We will parse it locally instead.
 export function isSignalHttpsLink(
   value: string | URL,
   logger: LoggerType
@@ -45,7 +47,8 @@ export function isSignalHttpsLink(
       !url.password &&
       !url.port &&
       url.protocol === 'https:' &&
-      SIGNAL_HOSTS.has(url.host)
+      SIGNAL_HOSTS.has(url.host) &&
+      (url.hash || url.pathname !== '/' || url.search)
   );
 }