Ergo push notification fixes, with #10's quit-log change

pkgs/ergochat.nix

@@ -1,7 +1,7 @@
 # Concierge needs /v1/ns/passwd (2.18) and /v1/ns/saget and saset (2.19);
 # drop the version bump once nixpkgs ships them. nixpkgs builds Ergo
 # without SQLite, which keeps channel history across restarts.
-{ ergochat, fetchFromGitHub }:
+{ ergochat, fetchFromGitHub, fetchpatch }:
 
 ergochat.overrideAttrs (finalAttrs: old: {
   version = "2.19.1";
@@ -23,5 +23,11 @@ ergochat.overrideAttrs (finalAttrs: old: {
     # Members manage their accounts through concierge, which Ergo's own
     # NickServ and HostServ commands would bypass. Not offered upstream yet.
     ./ergochat-disable-service-commands.patch
+    # Maintenance PINGs and MARKREADs went out at high urgency, which costs
+    # phones battery. Upstream's PR #2471; drop it once a release has it.
+    (fetchpatch {
+      url = "https://github.com/ergochat/ergo/commit/9975d8dc0bff2f51e79b7307b9955da375a945fe.patch";
+      hash = "sha256-fLQisMdGqdD5CBlxhlN5sD2iMTWWlOhWl+rUkRU19Ec=";
+    })
   ];
 })

services/ergo.nix

@@ -155,17 +155,19 @@ in
       webpush = {
         enabled = true;
         subscriber = "https://${domain}";
+        max-subscriptions = 10;
       };
       api = {
         enabled = true;
         listener = "127.0.0.1:8089";
       };
       # nixpkgs' ergo ships no translations for the default path to find.
       languages.enabled = false;
-      # connect-ip lines carry client addresses, which we promise not to keep.
+      # connect-ip lines, and quit lines for failed writes, carry client
+      # addresses, which we promise not to keep.
       logging = [{
         method = "stderr";
-        type = "* -userinput -useroutput -connect-ip";
+        type = "* -userinput -useroutput -connect-ip -quit";
         level = "info";
       }];
     };

tests/e2e/clients/irc-client.py

@@ -9,6 +9,7 @@
 import os
 import socket
 import ssl
+import struct
 
 QUIET_SECONDS = 2
 
@@ -117,6 +118,11 @@ def lines(self):
                 return
             self.buffer += data
 
+    def reset(self):
+        """Drops the connection with a TCP RST, as a phone losing its network can leave it."""
+        self.sock.setsockopt(socket.SOL_SOCKET, socket.SO_LINGER, struct.pack("ii", 1, 0))
+        self.sock.close()
+
     def drain(self, seconds):
         self.sock.settimeout(seconds)
         for _ in self.lines():
@@ -155,6 +161,12 @@ def main():
         metavar="SECONDS",
         help="stay connected after the commands until the server is quiet this long or closes the connection",
     )
+    parser.add_argument(
+        "--close",
+        choices=["quit", "reset"],
+        default="quit",
+        help="leave with QUIT once the server is quiet, or reset the connection once it answers the first PING",
+    )
     parser.add_argument("commands", nargs="*")
     args = parser.parse_args()
     if args.account and not (args.password or args.cert):
@@ -182,6 +194,10 @@ def main():
 
     for command in args.commands:
         conn.send(command)
+    if args.close == "reset":
+        conn.read_until("PONG")
+        conn.reset()
+        return
     conn.drain(args.linger)
     with contextlib.suppress(OSError):
         conn.send("QUIT")

tests/e2e/ergo.nix

@@ -0,0 +1,30 @@
+{ vmTest, pkgs, ... }:
+
+let
+  # Ergo probes every push endpoint it's given, and only over HTTPS to a
+  # public address; the client stands in for a push service at one.
+  pushService = "push.test";
+  pushAddress = "203.0.113.7";
+  certificates = import ./lib/certificates.nix { inherit pkgs; } pushService;
+in
+vmTest {
+  credenza = {
+    security.pki.certificateFiles = [ "${certificates}/minica.pem" ];
+    networking.hosts.${pushAddress} = [ pushService ];
+    networking.interfaces.eth1.ipv4.routes = [{ address = pushAddress; prefixLength = 32; }];
+  };
+
+  client = {
+    networking.interfaces.eth1.ipv4.addresses = [{ address = pushAddress; prefixLength = 32; }];
+    networking.firewall.allowedTCPPorts = [ 443 ];
+    services.nginx = {
+      enable = true;
+      virtualHosts.${pushService} = {
+        onlySSL = true;
+        sslCertificate = "${certificates}/${pushService}/cert.pem";
+        sslCertificateKey = "${certificates}/${pushService}/key.pem";
+        locations."/".return = "201";
+      };
+    };
+  };
+}

tests/e2e/ergo.py

@@ -4,7 +4,7 @@
 
 if TYPE_CHECKING:
     from driver import client, credenza, subtest
-    from helpers import Account, concierge_admin, ergo_api, irc, irc_command, register
+    from helpers import Account, assert_client_address_absent, concierge_admin, ergo_api, irc, irc_command, register
 
 alice: Account = {"account": "alice", "password": "correct-horse"}
 bob: Account = {"account": "bob", "password": "battery-staple"}
@@ -102,3 +102,22 @@
     whois = irc("WHOIS alice", "WHOIS carol", **renewed)
     hosts = dict(re.findall(r" 311 carol (\S+) \S+ (\S+) ", whois))
     assert hosts == {"alice": "weblounge", "carol": "weblounge"}, whois
+
+with subtest("a client that drops its connection mid-burst leaves no address in Ergo's log"):
+    # Fakelag holds the rest of the burst's replies until after the reset, so Ergo's next write fails.
+    irc(*["PING burst"] * 20, close="reset", **renewed)
+    # Carol isn't always-on, so she leaves the network once Ergo gives up on the connection.
+    whois_carol = irc_command("WHOIS carol", **alice)
+    client.wait_until_succeeds(f"{whois_carol} | grep ' 401 alice carol ' >/dev/null", timeout=60)
+    assert_client_address_absent("journalctl -u ergochat")
+
+with subtest("a member may keep push subscriptions on ten devices, and no more"):
+    keys = (
+        "p256dh=BB-oXlVJ4NSC4u0q_xDR-sBNQvDtdOQkBoiAHSGfdjMYGgisDueQhqo_qJPXT9ZdtR6nxu40lmMYe5SfMkDfK6g;"
+        "auth=saK8t84e6IvjXtf1fv0KIg"
+    )
+    devices = [f"https://push.test/device-{n}" for n in range(11)]
+    replies = irc(*[f"WEBPUSH REGISTER {device} {keys}" for device in devices], **alice)
+    registered = re.findall(r" WEBPUSH REGISTER (\S+) ", replies)
+    assert registered == devices[:10], replies
+    assert f"FAIL WEBPUSH MAX_REGISTRATIONS REGISTER {devices[10]} " in replies, replies

tests/e2e/lib/certificates.nix

@@ -0,0 +1,7 @@
+# A test CA and a certificate from it for domain: minica.pem is the CA's,
+# and <domain>/cert.pem and <domain>/key.pem the server's.
+{ pkgs }:
+
+domain: pkgs.runCommand "${domain}-test-certificates" { nativeBuildInputs = [ pkgs.minica ]; } ''
+  mkdir $out && cd $out && minica --domains ${domain}
+''

tests/e2e/lib/mail.nix

@@ -8,9 +8,7 @@ let
   server = "mail";
   # secrets/vm.yaml's smtp-password, which the mailer logs in with.
   password = "vm-smtp-password";
-  certificates = pkgs.runCommand "mail-test-certificates" { nativeBuildInputs = [ pkgs.minica ]; } ''
-    mkdir $out && cd $out && minica --domains ${server}
-  '';
+  certificates = import ./certificates.nix { inherit pkgs; } server;
   logins = username: pkgs.runCommand "mailpit-logins" { nativeBuildInputs = [ pkgs.apacheHttpd ]; } ''
     htpasswd -nbB ${pkgs.lib.escapeShellArgs [ username password ]} > $out
   '';