Changes in / [a4fea5e:ed5e7ab] in Klonkt

Files:

• deploy/MULTI-INSTANCE.md (modified) (1 diff)
• scripts/klonkt-migrate-data.sh (modified) (1 diff)
• scripts/klonkt-refresh-updater.sh (modified) (1 diff)
• src/config/database.js (modified) (3 diffs)
• src/services/ActivityPubService.js (modified) (1 diff)
• test/ap-signature-keyid-binding.test.js (added)
• test/feed-state-triggers.test.js (added)

deploy/MULTI-INSTANCE.md

@@ -84 +84 @@
 because the port comes from the same `.env`.
 
-**Rolling back.** The old `klonkt.service` is disabled and masked, not deleted.
-Masked because `disable` alone does not stop `systemctl restart klonkt` from
-starting it again, and a resurrected unit no longer finds its `.env` (that moved
-with the data): it would fall back to the defaults and write a fresh empty
-database into the checkout. To go back, move the data into `/opt/klonkt/storage`,
-restore the relative paths in `.env`, then `systemctl unmask klonkt` and
-`systemctl enable --now klonkt`.
+**The old unit is moved aside, not deleted.** It ends up next to its old place
+as `klonkt.service.retired-<timestamp>`. Stopping and disabling is not enough:
+`systemctl restart klonkt` starts a disabled unit anyway, which is exactly what
+an updater generated before the split does. A resurrected `klonkt.service` no
+longer finds its `.env` (that moved with the data), falls back to the built-in
+defaults, and writes a fresh empty database into the checkout. Masking does not
+work here either, because the unit file sits in `/etc/systemd/system` and
+`systemctl mask` refuses while a real file is there.
+
+**Rolling back.** Move the data into `/opt/klonkt/storage`, restore the relative
+paths in `.env`, move the retired unit file back to
+`/etc/systemd/system/klonkt.service`, then:
+
+```bash
+sudo systemctl daemon-reload
+sudo systemctl enable --now klonkt
+```
 
 ## Adding an instance

scripts/klonkt-migrate-data.sh

@@ -128 +128 @@
 run "systemctl daemon-reload"
 
+step "Retiring $OLD_UNIT"
+# Stopping and disabling is NOT enough: `systemctl restart klonkt` starts a
+# disabled unit anyway, and that is exactly what an updater generated before
+# the split does. A resurrected klonkt.service no longer finds its .env (that
+# moved with the data), falls back to the built-in defaults, and writes a
+# FRESH EMPTY database into the checkout.
+#
+# Masking does not help either: the unit file lives in /etc/systemd/system,
+# the highest-priority directory, and `systemctl mask` refuses when a real
+# file is already there ("File ... already exists"). Verified, not assumed.
+#
+# So the file is moved aside. systemd then no longer knows the unit at all and
+# any restart fails loudly with "Unit klonkt.service not found". The file is
+# kept next to its old place, timestamped, so a rollback is a move back.
+if [ -f "/etc/systemd/system/$OLD_UNIT" ]; then
+  run "systemctl stop $OLD_UNIT 2>/dev/null || true"
+  run "systemctl disable $OLD_UNIT 2>/dev/null || true"
+  RETIRED="/etc/systemd/system/${OLD_UNIT}.retired-$(date +%Y%m%d%H%M%S)"
+  run "mv '/etc/systemd/system/$OLD_UNIT' '$RETIRED'"
+  run "systemctl daemon-reload"
+  say "stopped, disabled and moved aside → $RETIRED"
+  say "roll back by moving that file back and running: systemctl daemon-reload"
+else
+  say "no $OLD_UNIT unit file to retire"
+fi
+
 step "Switching to klonkt@$SLUG"
-if systemctl list-unit-files "$OLD_UNIT" >/dev/null 2>&1; then
-  run "systemctl disable --now $OLD_UNIT"
-  # Disable only removes the autostart link: `systemctl restart klonkt` would
-  # still START it. That is not theoretical — an updater generated before the
-  # split does exactly that, and the resurrected unit finds no .env (it moved
-  # with the data), falls back to the built-in defaults and creates a FRESH
-  # EMPTY database in the checkout. Masking makes any such call fail loudly.
-  # Reversible: systemctl unmask klonkt.
-  run "systemctl mask $OLD_UNIT"
-  say "disabled and masked $OLD_UNIT (unmask to roll back)"
-fi
 run "systemctl enable --now 'klonkt@$SLUG'"
 

scripts/klonkt-refresh-updater.sh

@@ -77 +77 @@
 echo "klonkt-update rewritten: branch ${BRANCH}, code ${KLONKT_DIR}, instances under ${DATA_ROOT}"
 
-# On a split install the old single unit must not be startable. `disable` alone
-# does not stop `systemctl restart klonkt` from starting it, and a resurrected
-# klonkt.service has no .env (it moved with the data): it falls back to the
-# defaults and writes a fresh empty database into the checkout.
+# On a split install the old single unit must not be startable at all.
+# `disable` is not enough (restart starts a disabled unit anyway) and `mask`
+# refuses while the real file sits in /etc/systemd/system, the highest-priority
+# directory. Moving the file aside is what actually works: systemd stops
+# knowing the unit, so any restart fails loudly instead of quietly starting a
+# second process that writes an empty database into the checkout.
 SPLIT=0
 for d in "${DATA_ROOT}"/*/; do [ -f "$d/.env" ] && SPLIT=1 && break; done
-if [ "$SPLIT" = 1 ] && systemctl list-unit-files klonkt.service >/dev/null 2>&1; then
-  if ! systemctl is-enabled klonkt.service 2>/dev/null | grep -q masked; then
-    systemctl stop klonkt.service 2>/dev/null || true
-    systemctl disable klonkt.service 2>/dev/null || true
-    systemctl mask klonkt.service
-    echo "retired klonkt.service: stopped, disabled and masked (unmask to roll back)"
+if [ "$SPLIT" = 1 ] && [ -f /etc/systemd/system/klonkt.service ]; then
+  systemctl stop klonkt.service 2>/dev/null || true
+  systemctl disable klonkt.service 2>/dev/null || true
+  RETIRED="/etc/systemd/system/klonkt.service.retired-$(date +%Y%m%d%H%M%S)"
+  if mv /etc/systemd/system/klonkt.service "$RETIRED"; then
+    systemctl daemon-reload
+    echo "retired klonkt.service → $RETIRED (move it back + daemon-reload to roll back)"
+  else
+    echo "WARNING: could not move /etc/systemd/system/klonkt.service aside."
+    echo "         Until you do, any 'systemctl restart klonkt' starts a second"
+    echo "         process that writes an empty database into ${KLONKT_DIR}."
   fi
 fi

src/config/database.js

@@ -499 +499 @@
     );
     CREATE INDEX IF NOT EXISTS idx_ap_timeline_slug ON ap_timeline(slug, published);
+    -- canonicalReactionUri herleidt een permalink naar het object-id door op (slug, url)
+    -- te zoeken. Zonder deze index viel dat terug op idx_ap_timeline_slug, dus een scan
+    -- van elke rij van die slug. Dat gebeurt PER REACTIE in getInteractions, en de
+    -- reactie-migratie erft het in haar re-key-join, die synchroon vóór listen draait:
+    -- de opstartkosten waren reacties maal tijdlijnrijen.
+    CREATE INDEX IF NOT EXISTS idx_ap_timeline_url ON ap_timeline(slug, url);
     CREATE TABLE IF NOT EXISTS ap_blocks (
       id INTEGER PRIMARY KEY AUTOINCREMENT,
@@ -850 +856 @@
     }
     // slug + object_uri verschillen per bron; de rest is voor alle vier gelijk.
-    const zet = (naam, gebeurtenis, tabel, slug, uri, kind, extra = '') => `
+    const zet = (naam, gebeurtenis, tabel, slug, uri, kind, extra = '', wanneer = '') => `
       DROP TRIGGER IF EXISTS ${naam};
-      CREATE TRIGGER ${naam} AFTER ${gebeurtenis} ON ${tabel} BEGIN
+      CREATE TRIGGER ${naam} AFTER ${gebeurtenis} ON ${tabel}${wanneer ? ` WHEN ${wanneer}` : ''} BEGIN
         UPDATE ap_feed_rev SET n = n + 1;
         INSERT INTO ap_feed_state (slug, object_uri, rev, kind)
@@ -874 +880 @@
       // ap_interactions draagt geen slug: die hangt aan de POST. Vandaar de join,
       // en vandaar dat deze drie niet in de gewone vorm passen.
-      zet('trg_feed_ia_ins', 'INSERT', 'ap_interactions', '', '', '', `${joinPosts('NEW.object_uri', 'new')} WHERE p.id = NEW.post_id`),
-      zet('trg_feed_ia_upd', 'UPDATE OF content, media_json, quote_json, embed_json', 'ap_interactions', '', '', '', `${joinPosts('NEW.object_uri', 'updated')} WHERE p.id = NEW.post_id`),
-      zet('trg_feed_ia_del', 'DELETE', 'ap_interactions', '', '', '', `${joinPosts('OLD.object_uri', 'deleted')} WHERE p.id = OLD.post_id`),
+      //
+      // De WHEN op kind='reply' is nodig omdat deze tabel ook likes en announces
+      // draagt, en die schrijven object_uri = '' (zie recordInteraction). Zonder de
+      // WHEN bumpte elke inkomende like de rev, werd elke wachter gewekt en kreeg
+      // die de hele collectie opnieuw terwijl er niets aan veranderd was: precies de
+      // kosten die de 304 moest wegnemen. Bovendien belandde er dan een rij op de
+      // lege string in ap_feed_state, die feedChangesSince vervolgens uitdeelt.
+      // De oude cursor filterde hier wel op kind; bij ap_timeline is dit ook gedaan
+      // (de UPDATE OF sluit liked/boosted uit) en één tabel verder vergeten.
+      zet('trg_feed_ia_ins', 'INSERT', 'ap_interactions', '', '', '', `${joinPosts('NEW.object_uri', 'new')} WHERE p.id = NEW.post_id`, "NEW.kind = 'reply'"),
+      zet('trg_feed_ia_upd', 'UPDATE OF content, media_json, quote_json, embed_json', 'ap_interactions', '', '', '', `${joinPosts('NEW.object_uri', 'updated')} WHERE p.id = NEW.post_id`, "NEW.kind = 'reply'"),
+      zet('trg_feed_ia_del', 'DELETE', 'ap_interactions', '', '', '', `${joinPosts('OLD.object_uri', 'deleted')} WHERE p.id = OLD.post_id`, "OLD.kind = 'reply'"),
     ].join('\n'));
   } catch (e) {

src/services/ActivityPubService.js

@@ -1435 +1435 @@
   const pem = actor && actor.publicKey && actor.publicKey.publicKeyPem;
   if (!pem) return null;
+  // Bind the key to the actor it speaks for. Without this we hand back whatever
+  // `id` the fetched document claims, so anyone could host a document carrying a
+  // VICTIM's id next to their OWN public key, sign with their own private half,
+  // and be believed: the victim's server is never contacted. The caller decides on
+  // `verified.id`, so the identity has to come from where the key was FETCHED,
+  // never from what the document says about itself.
+  // Adds conditions only, and there is no exemption list on purpose: an
+  // "unless it's a known peer" escape hatch is exactly the door this closes.
+  // Note this does not narrow what we accept in practice, since the line above
+  // already requires the embedded publicKey object (an array or a bare URI
+  // reference never worked here).
+  const key = actor.publicKey;
+  try {
+    if (new URL(p.keyId).host !== new URL(actor.id).host) return null;   // same origin as the key
+    if (key.id && key.id !== p.keyId) return null;                       // this key, not a neighbour's
+    if (key.owner && key.owner !== actor.id) return null;                // and it belongs to this actor
+  } catch { return null; }                                               // unparseable id or keyId
   const hs = (p.headers || '(request-target) host date').split(/\s+/);
   // Behind a reverse proxy the raw Host header is the backend bind (e.g. localhost:3000, when