Update the signal integration to use DBus

The initial implementation was just calling signal-cli directly
using `subprocess.run`.

Going with DBus makes it easier to shield signal-cli from the
rest of the system. It also makes sure the signal-cli daemon is
running in the background and receiving messages. This is important
when a recipient does the "Reset secure connection" from the app. We
must receive their new keys, otherwise our future messages will
appear as "bad encrypted message" for them.

README.md

@@ -136,8 +136,7 @@ Healthchecks reads configuration from the following environment variables:
 | PUSHOVER_SUBSCRIPTION_URL | `None`
 | REMOTE_USER_HEADER | `None` | See [External Authentication](#external-authentication) for details.
 | SHELL_ENABLED | `"False"`
-| SIGNAL_CLI_USERNAME | `None`
-| SIGNAL_CLI_CMD | `signal-cli` | Path to the signal-cli executable
+| SIGNAL_CLI_ENABLED | `"False"`
 | SLACK_CLIENT_ID | `None`
 | SLACK_CLIENT_SECRET | `None`
 | TELEGRAM_BOT_NAME | `"ExampleBot"`
@@ -412,34 +411,14 @@ To enable the Pushover integration, you will need to:
 ### Signal
 
 Healthchecks uses [signal-cli](https://github.com/AsamK/signal-cli) to send Signal
-notifications. It requires the `signal-cli` program to be installed and available on
-the local machine.
-
-To send notifications, healthchecks executes "signal-cli send" calls.
-It does not handle phone number registration and verification. You must do that
-manually, before using the integration.
+notifications. Healthcecks interacts with signal-cli over DBus.
 
 To enable the Signal integration:
 
-* Download and install signal-cli in your preferred location
-  (for example, in `/srv/signal-cli-0.7.2/`).
-* Register and verify phone number, or [link it](https://github.com/AsamK/signal-cli/wiki/Linking-other-devices-(Provisioning))
-  to an existing registration.
-* Test your signal-cli configuration by sending a message manually from command line.
-* Put the sender phone number in the `SIGNAL_CLI_USERNAME` environment variable.
-  Example: `SIGNAL_CLI_USERNAME=+123456789`.
-* If `signal-cli` is not in the system path, specify its path in `SIGNAL_CLI_CMD`.
-  Example: `SIGNAL_CLI_CMD=/srv/signal-cli-0.7.2/bin/signal-cli`
-
-It is possible to use a separate system user for running signal-cli:
-
-* Create a separate system user, (for example, "signal-user").
-* Configure signal-cli while logged in as signal-user.
-* Change `SIGNAL_CLI_CMD` to run signal-cli through sudo:
-  `sudo -u signal-user /srv/signal-cli-0.7.2/bin/signal-cli`.
-* Configure sudo to not require password. For example, if healthchecks
-  runs under the www-data system user, the sudoers rule would be:
-  `www-data ALL=(signal-user) NOPASSWD: /srv/signal-cli-0.7.2/bin/signal-cli`.
+* Set up and configure signal-cli to listen on DBus system bus ([instructions](https://github.com/AsamK/signal-cli/wiki/DBus-service)).
+  Make sure you can send test messages from command line, using the `dbus-send`
+  example given in the signal-cli instructions.
+* Set the `SIGNAL_CLI_ENABLED` environment variable to `True`.
 
 
 ### Telegram

hc/api/tests/test_notify_signal.py

@@ -28,24 +28,22 @@ class NotifySignalTestCase(BaseTestCase):
         self.channel.save()
         self.channel.checks.add(self.check)
 
-    @patch("hc.api.transports.subprocess.run")
-    def test_it_works(self, mock_run):
-        mock_run.return_value.returncode = 0
-
+    @patch("hc.api.transports.dbus")
+    @patch("hc.api.transports.Signal.get_service")
+    def test_it_works(self, mock_get_service, mock_dbus):
         self.channel.notify(self.check)
 
         n = Notification.objects.get()
         self.assertEqual(n.error, "")
 
-        self.assertTrue(mock_run.called)
-        args, kwargs = mock_run.call_args
-        cmd = " ".join(args[0])
-
-        self.assertIn("-u +987654321", cmd)
-        self.assertIn("send +123456789", cmd)
+        self.assertTrue(mock_get_service.called)
+        args, kwargs = mock_get_service.return_value.sendMessage.call_args
+        self.assertIn("is DOWN", args[0])
+        self.assertEqual(args[2], ["+123456789"])
 
-    @patch("hc.api.transports.subprocess.run")
-    def test_it_obeys_down_flag(self, mock_run):
+    @patch("hc.api.transports.dbus")
+    @patch("hc.api.transports.Signal.get_service")
+    def test_it_obeys_down_flag(self, mock_get_service, mock_dbus):
         payload = {"value": "+123456789", "up": True, "down": False}
         self.channel.value = json.dumps(payload)
         self.channel.save()
@@ -54,35 +52,35 @@ class NotifySignalTestCase(BaseTestCase):
 
         # This channel should not notify on "down" events:
         self.assertEqual(Notification.objects.count(), 0)
-        self.assertFalse(mock_run.called)
 
-    @patch("hc.api.transports.subprocess.run")
-    def test_it_requires_signal_cli_username(self, mock_run):
+        self.assertFalse(mock_get_service.called)
 
-        with override_settings(SIGNAL_CLI_USERNAME=None):
+    @patch("hc.api.transports.dbus")
+    @patch("hc.api.transports.Signal.get_service")
+    def test_it_requires_signal_cli_enabled(self, mock_get_service, mock_dbus):
+        with override_settings(SIGNAL_CLI_ENABLED=False):
             self.channel.notify(self.check)
 
         n = Notification.objects.get()
         self.assertEqual(n.error, "Signal notifications are not enabled")
 
-        self.assertFalse(mock_run.called)
+        self.assertFalse(mock_get_service.called)
 
-    @patch("hc.api.transports.subprocess.run")
-    def test_it_does_not_escape_special_characters(self, mock_run):
+    @patch("hc.api.transports.dbus")
+    @patch("hc.api.transports.Signal.get_service")
+    def test_it_does_not_escape_special_characters(self, mock_get_service, mock_dbus):
         self.check.name = "Foo & Bar"
         self.check.save()
 
-        mock_run.return_value.returncode = 0
         self.channel.notify(self.check)
 
-        self.assertTrue(mock_run.called)
-        args, kwargs = mock_run.call_args
-        cmd = " ".join(args[0])
-
-        self.assertIn("Foo & Bar", cmd)
+        args, kwargs = mock_get_service.return_value.sendMessage.call_args
+        self.assertIn("Foo & Bar", args[0])
 
     @override_settings(SECRET_KEY="test-secret")
-    def test_it_obeys_rate_limit(self):
+    @patch("hc.api.transports.dbus")
+    @patch("hc.api.transports.Signal.get_service")
+    def test_it_obeys_rate_limit(self, mock_get_service, mock_dbus):
         # "2862..." is sha1("+123456789test-secret")
         obj = TokenBucket(value="signal-2862991ccaa15c8856e7ee0abaf3448fb3c292e0")
         obj.tokens = 0
@@ -91,3 +89,5 @@ class NotifySignalTestCase(BaseTestCase):
         self.channel.notify(self.check)
         n = Notification.objects.first()
         self.assertEqual(n.error, "Rate limit exceeded")
+
+        self.assertFalse(mock_get_service.called)

hc/api/transports.py

@@ -6,7 +6,6 @@ from django.utils import timezone
 from django.utils.html import escape
 import json
 import requests
-import subprocess
 from urllib.parse import quote, urlencode
 
 from hc.accounts.models import Profile
@@ -19,6 +18,12 @@ except ImportError:
     # Enforce
     settings.APPRISE_ENABLED = False
 
+try:
+    import dbus
+except ImportError:
+    # Enforce
+    settings.SIGNAL_CLI_ENABLED = False
+
 
 def tmpl(template_name, **ctx):
     template_path = "integrations/%s" % template_name
@@ -669,8 +674,13 @@ class Signal(Transport):
         else:
             return not self.channel.signal_notify_up
 
+    def get_service(self):
+        bus = dbus.SystemBus()
+        signal_object = bus.get_object("org.asamk.Signal", "/org/asamk/Signal")
+        return dbus.Interface(signal_object, "org.asamk.Signal")
+
     def notify(self, check):
-        if not settings.SIGNAL_CLI_USERNAME:
+        if not settings.SIGNAL_CLI_ENABLED:
             return "Signal notifications are not enabled"
 
         from hc.api.models import TokenBucket
@@ -680,14 +690,10 @@ class Signal(Transport):
 
         text = tmpl("signal_message.html", check=check, site_name=settings.SITE_NAME)
 
-        args = settings.SIGNAL_CLI_CMD.split()
-        args.extend(["-u", settings.SIGNAL_CLI_USERNAME])
-        args.extend(["send", self.channel.phone_number])
-        args.extend(["-m", text])
-
-        # Need a high timeout because sending the first message to a new
-        # recipient sometimes takes 20+ seconds
-        result = subprocess.run(args, timeout=30)
+        try:
+            self.get_service().sendMessage(text, [], [self.channel.phone_number])
+        except dbus.exceptions.DBusException as e:
+            if "NotFoundException" in str(e):
+                return "Recipient not found"
 
-        if result.returncode != 0:
-            return "signal-cli returned exit code %d" % result.returncode
+            return "signal-cli call failed"

hc/front/tests/test_add_signal.py

@@ -45,8 +45,8 @@ class AddSignalTestCase(BaseTestCase):
         self.assertFalse(c.signal_notify_down)
         self.assertFalse(c.signal_notify_up)
 
-    @override_settings(SIGNAL_CLI_USERNAME=None)
-    def test_it_handles_unset_sender_username(self):
+    @override_settings(SIGNAL_CLI_ENABLED=False)
+    def test_it_handles_disabled_integration(self):
         self.client.login(username="[email protected]", password="password")
         r = self.client.get(self.url)
         self.assertEqual(r.status_code, 404)

hc/front/views.py

@@ -299,7 +299,7 @@ def index(request):
         "enable_pushbullet": settings.PUSHBULLET_CLIENT_ID is not None,
         "enable_pushover": settings.PUSHOVER_API_TOKEN is not None,
         "enable_shell": settings.SHELL_ENABLED is True,
-        "enable_signal": settings.SIGNAL_CLI_USERNAME is not None,
+        "enable_signal": settings.SIGNAL_CLI_ENABLED is True,
         "enable_slack_btn": settings.SLACK_CLIENT_ID is not None,
         "enable_sms": settings.TWILIO_AUTH is not None,
         "enable_telegram": settings.TELEGRAM_TOKEN is not None,
@@ -763,7 +763,7 @@ def channels(request, code):
         "enable_pushbullet": settings.PUSHBULLET_CLIENT_ID is not None,
         "enable_pushover": settings.PUSHOVER_API_TOKEN is not None,
         "enable_shell": settings.SHELL_ENABLED is True,
-        "enable_signal": settings.SIGNAL_CLI_USERNAME is not None,
+        "enable_signal": settings.SIGNAL_CLI_ENABLED is True,
         "enable_slack_btn": settings.SLACK_CLIENT_ID is not None,
         "enable_sms": settings.TWILIO_AUTH is not None,
         "enable_telegram": settings.TELEGRAM_TOKEN is not None,
@@ -1628,7 +1628,7 @@ def add_whatsapp(request, code):
     return render(request, "integrations/add_whatsapp.html", ctx)
 
 
-@require_setting("SIGNAL_CLI_USERNAME")
+@require_setting("SIGNAL_CLI_ENABLED")
 @login_required
 def add_signal(request, code):
     project = _get_rw_project_for_user(request, code)

hc/settings.py

@@ -231,8 +231,7 @@ LINENOTIFY_CLIENT_ID = os.getenv("LINENOTIFY_CLIENT_ID")
 LINENOTIFY_CLIENT_SECRET = os.getenv("LINENOTIFY_CLIENT_SECRET")
 
 # Signal
-SIGNAL_CLI_USERNAME = os.getenv("SIGNAL_CLI_USERNAME")
-SIGNAL_CLI_CMD = os.getenv("SIGNAL_CLI_CMD", "signal-cli")
+SIGNAL_CLI_ENABLED = envbool("SIGNAL_CLI_ENABLED", "False")
 
 if os.path.exists(os.path.join(BASE_DIR, "hc/local_settings.py")):
     from .local_settings import *