fix(dns): finish the CNAME chain instead of handing clients a dead end

lib/dns/server.ts

@@ -24,6 +24,7 @@ import {
   setMessageId,
   udpPayloadSize,
   type Message,
+  type Question,
 } from "./wire";
 
 export type ServerStats = {
@@ -164,6 +165,59 @@ export function createDnsServer(options: DnsServerOptions): DnsServer {
     return setMessageId(response, clientId);
   }
 
+  /**
+   * Finish a CNAME chain the client will not finish itself.
+   *
+   * A name pointed at a hostname answers with a CNAME, which is correct for an
+   * authoritative server: its answer gets completed by whichever recursive
+   * resolver asked. This resolver is not in that position. It sets RA=1 and is
+   * used *directly* by stub clients — browsers, curl, and every machine
+   * pointed at the DoH endpoint — and a stub does not chase CNAMEs. It reads
+   * the address out of the answer section, finds none, and reports failure.
+   *
+   * So a bare CNAME reads to all of them as "no such host": `curl` says
+   * "Could not resolve host: seo.rank" for a name that resolves perfectly.
+   *
+   * Best-effort on purpose. If the upstream lookup fails we still return the
+   * CNAME rather than nothing — a resolver that chases well is better than the
+   * one we had, and a resolver that fails closed on an upstream hiccup is
+   * worse.
+   */
+  async function completeCnameChain(message: Message, question: Question): Promise<void> {
+    if (question.type !== TYPE.A && question.type !== TYPE.AAAA) return;
+    // Already has what was asked for — a name pointed at a literal address.
+    if (message.answers.some((r) => r.type === question.type)) return;
+
+    const cname = message.answers.find((r) => r.type === TYPE.CNAME && r.target);
+    if (!cname?.target) return;
+
+    try {
+      const probe = encodeMessage({
+        id: randomId(),
+        flags: { qr: false, opcode: 0, aa: false, tc: false, rd: true, ra: false, z: false, ad: false, cd: false, rcode: 0 },
+        questions: [{ name: cname.target, type: question.type, class: CLASS.IN }],
+      });
+      const resolved = decodeMessage(await forwarder.query(probe));
+      for (const record of resolved.answers) {
+        // Leaves only. Relaying the upstream's own CNAMEs would rebuild the
+        // same dead end one link further along.
+        if (record.type !== question.type || !record.address) continue;
+        message.answers.push({
+          name: cname.target,
+          type: question.type,
+          class: CLASS.IN,
+          // Never outlive the registry's own TTL: the owner can repoint this
+          // name at any moment, and an address cached past that is the one
+          // failure nobody can debug from outside.
+          ttl: Math.min(ttl, record.ttl ?? ttl),
+          address: record.address,
+        });
+      }
+    } catch {
+      // Keep the CNAME. See above.
+    }
+  }
+
   async function moshpitResponse(query: Message, name: string): Promise<{ buffer: Buffer; message: Message } | null> {
     const lookup = await registry.lookup(name);
     if (!lookup?.registered) return null;
@@ -187,6 +241,7 @@ export function createDnsServer(options: DnsServerOptions): DnsServer {
       ttl,
     });
     if (!message) return null;
+    await completeCnameChain(message, query.questions[0]);
     message.additionals = echoOpt(query);
     return { buffer: encodeMessage(message), message };
   }

tests/dns-server.test.mjs

@@ -55,6 +55,7 @@ function stubRegistry(names, options = {}) {
           resolved: entry?.resolved ?? name,
           registered: Boolean(entry),
           aliased: Boolean(entry?.resolved && entry.resolved !== name),
+          target: entry?.target ?? null,
         }),
         { status: 200, headers: { "content-type": "application/json" } },
       );
@@ -286,3 +287,47 @@ test("the registry is asked once for a name, however many clients ask us", async
   await registry.lookup("scrambled.eggs");
   assert.equal(calls, 1, "coalesced in flight, then cached");
 });
+
+// A name pointed at a hostname is the case `seo.rank` hit in production: the
+// answer was a CNAME to dev.profullstack.com and nothing else, and every stub
+// client read that as "no such host".
+test("a name pointed at a hostname answers with the address, not just the CNAME", async () => {
+  const dns = harness({
+    names: { "seo.rank": { target: "dev.profullstack.com" } },
+    zone: { "dev.profullstack.com": CLEARNET_V4 },
+  });
+  const response = decodeMessage(await dns.handle(query("seo.rank")));
+
+  assert.equal(response.flags.rcode, RCODE.NOERROR);
+  // The CNAME still goes out — a `dig` should show where the name points.
+  assert.ok(
+    response.answers.some((r) => r.type === TYPE.CNAME && r.target === "dev.profullstack.com"),
+    "the CNAME is what makes the indirection visible",
+  );
+  // ...but the address has to be there too. This resolver sets RA=1 and talks
+  // to stub clients directly; a stub reads the answer section for an address
+  // and gives up when there is none, so a bare CNAME is a failed lookup.
+  assert.deepEqual(addresses(response), [CLEARNET_V4], "a stub client needs the leaf address");
+  await dns.close();
+});
+
+test("an unreachable upstream still yields the CNAME rather than nothing", async () => {
+  // Chasing is best-effort. Failing closed here would turn one upstream
+  // hiccup into "this name does not exist".
+  const dns = harness({ names: { "seo.rank": { target: "dev.profullstack.com" } }, zone: {} });
+  const response = decodeMessage(await dns.handle(query("seo.rank")));
+
+  assert.equal(response.flags.rcode, RCODE.NOERROR);
+  assert.ok(response.answers.some((r) => r.type === TYPE.CNAME), "the CNAME survives an upstream failure");
+  await dns.close();
+});
+
+test("a name pointed at a literal address does not get a spurious lookup", async () => {
+  // Nothing to chase: the address is already the answer.
+  const dns = harness({ names: { "pinned.rank": { target: "203.0.113.55" } } });
+  const response = decodeMessage(await dns.handle(query("pinned.rank")));
+
+  assert.deepEqual(addresses(response), ["203.0.113.55"]);
+  assert.ok(!response.answers.some((r) => r.type === TYPE.CNAME), "no CNAME when the target is an address");
+  await dns.close();
+});