From 3c0e024213359e81ad9d2dbf526964ac339564bb Mon Sep 17 00:00:00 2001
From: Jannis Braun <151788261+TheZwiss@users.noreply.github.com>
Date: Sun, 3 May 2026 23:57:18 +0200
Subject: [PATCH] test(harness): document why peerInstances inserts two rows
 per direction
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

The original peerInstances comment framed the two-row pattern as a
band-aid for getOurOrigin's https://${DOMAIN} default. After
investigating a clean collapse to one row (PUBLIC_ORIGIN override on
each spawned instance), the deeper coupling surfaces:

  - extractDomain() strips port via new URL().hostname, so unique-port
    localhost instances all share hostname '127.0.0.1' and the
    receiver's attribution guard
        extractDomain(user.homeInstance) === extractDomain(fedHeaders.origin)
    becomes ambiguous in any multi-remote configuration.
  - The homeInstance validator regex /^[a-zA-Z0-9._-]+$/ in auth.ts
    rejects ':', so the port cannot be encoded into homeInstance to
    disambiguate.
  - Eliminating the second row would require a production refactor of
    extractDomain (port-preserving), the attribution check (decoupled
    from URL), or the homeInstance validator (allow ':') — all out of
    scope.

So the harness DELIBERATELY keeps DOMAIN as a per-instance human label
('home.test.local' / 'remoteN.test.local') for stable identity, and the
two peer rows per direction (transport URL + getOurOrigin URL) are
structural to localhost-port test reality, not a band-aid. Comment
rewritten to reflect this. PUBLIC_ORIGIN remains available in
production code for reverse-proxy / dev-without-TLS deployments.
---
 packages/server/test/helpers/seedPeer.ts | 44 +++++++++++++++---------
 1 file changed, 28 insertions(+), 16 deletions(-)

diff --git a/packages/server/test/helpers/seedPeer.ts b/packages/server/test/helpers/seedPeer.ts
index 2d559df9..e26e8cc4 100644
--- a/packages/server/test/helpers/seedPeer.ts
+++ b/packages/server/test/helpers/seedPeer.ts
@@ -6,20 +6,34 @@ import type { SpawnedInstance } from './twoInstanceHarness.js';
  * with a single shared HMAC secret. Returns the secret for tests that need to sign
  * raw S2S requests directly.
  *
- * IMPORTANT — dual-origin reality in tests:
+ * Why two rows per direction (transport row + identity row):
  * Production code stores `peer.origin` as a single string and uses it for BOTH
- *   (a) outbound URL: `fetch(${peer.origin}/api/...)`, AND
- *   (b) inbound auth: `WHERE origin = X-Federation-Origin` claim from inbound headers.
- * In production with `DOMAIN=example.com`, both reduce to `https://example.com`.
+ *   (a) outbound URL: `fetch(${peer.origin}/api/...)` — wants the transport URL, AND
+ *   (b) inbound auth: `WHERE origin = X-Federation-Origin` — wants `getOurOrigin()`.
+ * In production with `DOMAIN=example.com`, both reduce to `https://example.com`,
+ * so a single row satisfies both. In a localhost integration harness the two
+ * forms cannot be collapsed:
+ *   - The transport URL is `http://127.0.0.1:<ephemeral-port>` (per-instance unique).
+ *   - `getOurOrigin()` defaults to `https://${DOMAIN}` (= `https://home.test.local`),
+ *     and even with the `PUBLIC_ORIGIN` env override the receiver's attribution
+ *     guard `extractDomain(user.homeInstance) === extractDomain(fedHeaders.origin)`
+ *     forces `homeInstance` to match the URL's hostname. `extractDomain` strips
+ *     the port (`new URL().hostname`), so unique-port localhost instances all
+ *     share the hostname `127.0.0.1` and attribution becomes ambiguous in
+ *     multi-remote setups. And the `homeInstance` validator regex
+ *     (`/^[a-zA-Z0-9._-]+$/` in `auth.ts`) rejects `:` — so we cannot encode
+ *     port into `homeInstance` to disambiguate.
  *
- * In our test harness, the URL is `http://127.0.0.1:<ephemeral>` but `getOurOrigin()`
- * returns `https://${DOMAIN}` (= `https://home.test.local`). These two values are
- * DIFFERENT, so we cannot satisfy both with one `peer.origin` row.
+ * Therefore the harness keeps `DOMAIN` as a per-instance human label
+ * (`home.test.local` / `remoteN.test.local`) for stable identity (federated
+ * usernames, attribution domain), and inserts TWO peer rows per direction:
+ * one keyed by the transport URL (outbound lookup) and one keyed by the
+ * `getOurOrigin()` URL (inbound auth lookup). The schema's UNIQUE(origin)
+ * permits both because the strings differ.
  *
- * Workaround: insert TWO rows per direction — one with the URL form (for outbound
- * lookup on the sender) and one with the DOMAIN-claim form (for inbound auth on
- * the receiver). The schema has a UNIQUE constraint on `origin`, but the two
- * rows have distinct origins so there is no conflict.
+ * Eliminating the second row would require a production refactor of either
+ * `extractDomain` (port-preserving), the attribution check (decouple from
+ * URL), or the `homeInstance` validator (allow `:`). Out of scope here.
  */
 export async function peerInstances(
   a: SpawnedInstance,
@@ -44,14 +58,12 @@ export async function peerInstances(
     }
   };
 
-  // Outbound lookup form (URL) — what users.ts / federation.ts use to find the
-  // hmacSecret given a body-supplied or DB-stored origin URL.
+  // Transport row — outbound URL lookup (`fetch(peer.origin)`).
   await seedOn(a, b.origin, b.domain);
   await seedOn(b, a.origin, a.domain);
 
-  // Inbound auth form (DOMAIN claim) — what the receiver uses to look up the
-  // peer when validating the X-Federation-Origin header from a sender whose
-  // `getOurOrigin()` returns `https://${DOMAIN}`.
+  // Identity row — inbound HMAC `X-Federation-Origin` claim, which equals the
+  // sender's `getOurOrigin()` (= `https://${DOMAIN}` by default).
   await seedOn(a, `https://${b.domain}`, b.domain);
   await seedOn(b, `https://${a.domain}`, a.domain);