From 04881306d475f690c16c7b56cd6871cfc2d0b7ca Mon Sep 17 00:00:00 2001 From: latenighthackathon Date: Sat, 16 May 2026 22:16:52 +0000 Subject: [PATCH] fix(preflight): use uncached .invalid probe for container DNS check (#3630) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Closes #3630. The container DNS preflight queried `registry.npmjs.org`, whose answer Docker's embedded DNS resolver (and most upstream resolvers) caches aggressively. On a host where outbound UDP/TCP port 53 was blocked at the iptables OUTPUT chain, the cached resolution still made the probe report `✓ Container DNS resolution works` and onboard continued past preflight, even though the sandbox would later fail to pull npm packages or anything else needing fresh DNS. Replace the probe target with a random subdomain of the RFC 6761 reserved `.invalid` TLD. Properties: - Never cached anywhere: each call uses a fresh `nemoclaw-dns-probe-<16-hex>.invalid` name, so the query always round-trips through the upstream resolver. - Always NXDOMAIN: every compliant resolver returns NXDOMAIN immediately for any `.invalid` name (RFC 6761 §6.4). NXDOMAIN proves the resolver was reached even though the name does not resolve, which is the only invariant the probe needs. - Bypasses the cache hit that previously masked host-side egress blocks. The success regex is widened to accept both shapes busybox emits: - `Name:` + `Address:` lines for a real A-record resolution (kept for back-compat with custom probe names that resolve to a real IP). - `Server:` header + `** server can't find : NXDOMAIN` for the `.invalid` probe sent by default. The resolver-identification block (`Server: / Address: :53`) appears in every busybox response, including malformed ones, so the new regex requires either a real `Name:` line OR an NXDOMAIN response body before declaring success. A regression test pins the malformed case so this guard cannot quietly weaken. Tests: `src/lib/onboard/preflight.test.ts` adds the #3630 NXDOMAIN success case, the malformed-response failure case, the random probe-name property, and a pinned-name override seam. 18/18 probeContainerDns tests pass. Existing success/servers_unreachable/ image_pull_failed/no_output/error cases pass unchanged. Signed-off-by: latenighthackathon Signed-off-by: latenighthackathon --- src/lib/onboard/preflight.test.ts | 59 ++++++++++++++++++++++++++++--- src/lib/onboard/preflight.ts | 46 ++++++++++++++++++++++-- 2 files changed, 98 insertions(+), 7 deletions(-) diff --git a/src/lib/onboard/preflight.test.ts b/src/lib/onboard/preflight.test.ts index 44bc99bd031..bc54df21559 100644 --- a/src/lib/onboard/preflight.test.ts +++ b/src/lib/onboard/preflight.test.ts @@ -21,6 +21,7 @@ import { parseDockerStorageDriver, parseDockerUsesContainerdSnapshotter, planHostRemediation, + dnsProbeName, probeContainerDns, } from "../../../dist/lib/onboard/preflight"; @@ -1170,13 +1171,35 @@ describe("probeContainerDns", () => { expect(result.reason).toBe("image_pull_failed"); }); - it("flags resolution_failed for NXDOMAIN-style failures (resolver OK, name unknown)", () => { + it("returns ok on NXDOMAIN — resolver reachable, name does not resolve (#3630)", () => { + // The default probe queries a random `.invalid` subdomain (RFC 6761), + // which any compliant resolver answers with NXDOMAIN. NXDOMAIN proves + // the resolver was reached even though the name doesn't resolve, which + // is the only invariant we need to prove DNS works. Crucially, this + // path does NOT depend on a real A record (e.g., `registry.npmjs.org`) + // being reachable from the container, so a Docker-embedded-DNS cache + // hit cannot mask a host-side egress block. See #3630. const result = probeContainerDns({ outputOverride: "Server:\t\t1.1.1.1\n" + "Address:\t1.1.1.1:53\n" + "\n" + - "** server can't find registry.npmjs.org: NXDOMAIN\n", + "** server can't find nemoclaw-dns-probe-abc123def456.invalid: NXDOMAIN\n", + }); + expect(result.ok).toBe(true); + expect(result.reason).toBeUndefined(); + }); + + it("flags resolution_failed for malformed responses with neither Address nor NXDOMAIN", () => { + // Resolver answered with something we don't recognize (e.g., SERVFAIL, + // unexpected format). Surface as resolution_failed rather than masking + // it as success. + const result = probeContainerDns({ + outputOverride: + "Server:\t\t1.1.1.1\n" + + "Address:\t1.1.1.1:53\n" + + "\n" + + ";; reply from unexpected source: 8.8.8.8#53, expected 1.1.1.1#53\n", }); expect(result.ok).toBe(false); expect(result.reason).toBe("resolution_failed"); @@ -1216,7 +1239,9 @@ describe("probeContainerDns", () => { const script = captured[0][2]; expect(script).toContain("docker run --rm"); expect(script).toContain("busybox:latest"); - expect(script).toContain("registry.npmjs.org"); + // Probe queries a random `.invalid` subdomain (#3630), not a real + // domain — cache-bypass guarantee. Stable prefix is asserted instead. + expect(script).toMatch(/nslookup nemoclaw-dns-probe-[0-9a-f]+\.invalid /); expect(script).toContain("2>&1"); }); @@ -1232,7 +1257,33 @@ describe("probeContainerDns", () => { expect(seen).toEqual(["echo", "OVERRIDDEN"]); }); - it("treats thrown runCapture errors as error reason", () => { + it("uses a random .invalid probe per call to bypass Docker embedded DNS cache (#3630)", () => { + // Before #3630 the probe queried `registry.npmjs.org`, whose answer + // Docker's embedded DNS resolver could serve from cache even when + // host-side egress to UDP/TCP port 53 was fully blocked. The fix + // is a random subdomain of the RFC 6761 reserved `.invalid` TLD so + // the query is never cached anywhere and always exercises real + // upstream DNS reachability. + const name1 = dnsProbeName(); + const name2 = dnsProbeName(); + expect(name1).toMatch(/^nemoclaw-dns-probe-[0-9a-f]{16}\.invalid$/); + expect(name2).toMatch(/^nemoclaw-dns-probe-[0-9a-f]{16}\.invalid$/); + expect(name1).not.toBe(name2); + }); + + it("honors a pinned probeName override for stable test assertions", () => { + let seenScript = ""; + probeContainerDns({ + probeName: "pinned-test.invalid", + runCaptureImpl: (command) => { + seenScript = command[2] ?? ""; + return "Server:\t1.1.1.1\nAddress:\t1.1.1.1:53\n** server can't find pinned-test.invalid: NXDOMAIN\n"; + }, + }); + expect(seenScript).toContain("nslookup pinned-test.invalid"); + }); + + it("treats thrown runCapture errors as error reason", () => { const result = probeContainerDns({ runCaptureImpl: () => { throw new Error("docker daemon unreachable"); diff --git a/src/lib/onboard/preflight.ts b/src/lib/onboard/preflight.ts index 26a3407630f..0823183baa3 100644 --- a/src/lib/onboard/preflight.ts +++ b/src/lib/onboard/preflight.ts @@ -9,6 +9,7 @@ * tests can run without real I/O. */ +import { randomBytes } from "node:crypto"; import fs from "node:fs"; import net from "node:net"; import os from "node:os"; @@ -1166,6 +1167,8 @@ export interface ProbeContainerDnsOpts { outputOverride?: string | null; /** Override runCapture. */ runCaptureImpl?: NullableRunCaptureFn; + /** Override the probe name (test seam; pinned name for stable assertions). */ + probeName?: string; } /** @@ -1176,6 +1179,22 @@ export interface ProbeContainerDnsOpts { */ const PROBE_TIMEOUT_MS = 20_000; +/** + * Random subdomain of the RFC 6761 reserved .invalid TLD. Every compliant + * resolver returns NXDOMAIN immediately for any .invalid name, so the + * probe round-trips through the upstream DNS server without depending on + * any specific A record being reachable from the container. The random + * suffix prevents the answer from being served from Docker's embedded + * DNS cache (or any upstream cache), which is what masked host-side + * egress blocks for the previous `registry.npmjs.org` query in #3630. + * + * Exported so tests can pin the probe-name pattern; production callers + * never need to override this. + */ +export function dnsProbeName(): string { + return `nemoclaw-dns-probe-${randomBytes(8).toString("hex")}.invalid`; +} + /** * Discover the IPv4 gateway address of docker's default bridge network. * Returns null if docker isn't running, the inspect command fails, or the @@ -1245,10 +1264,11 @@ export function probeContainerDns(opts: ProbeContainerDnsOpts = {}): DnsProbeRes // signatures the parser below depends on (`Error response from daemon`, // `no servers could be reached`) to stderr. Every token in the script // is a fixed constant, so no shell injection surface. + const probeName = opts.probeName ?? dnsProbeName(); const command = opts.command ?? [ "sh", "-c", - "docker run --rm --pull=missing busybox:latest nslookup registry.npmjs.org 2>&1", + `docker run --rm --pull=missing busybox:latest nslookup ${probeName} 2>&1`, ]; let output: string | null | undefined = opts.outputOverride; @@ -1285,8 +1305,28 @@ export function probeContainerDns(opts: ProbeContainerDnsOpts = {}): DnsProbeRes }; } - // Success: busybox nslookup prints "Name:" and "Address:" lines. - if (/\bName:\s*registry\.npmjs\.org\b/.test(output) && /\bAddress:\s*\d/.test(output)) { + // Success: the resolver answered the probe. Two shapes count as success: + // 1. busybox nslookup prints "Name:" + "Address:" lines for a normal + // resolution (kept for back-compat with custom probe names that do + // resolve to a real A record). + // 2. busybox nslookup prints "Server: ..." + "** server can't find + // ...: NXDOMAIN" for the .invalid probe we send by default. NXDOMAIN + // proves the resolver was reached even though the name does not + // resolve, which is the only invariant we need to prove DNS works. + // Before #3630 we only accepted the Address shape and used + // `registry.npmjs.org`, so a Docker-embedded-DNS cache hit could + // mask a host-side egress block. The .invalid probe is never cached + // anywhere, so reaching the resolver is genuine round-trip evidence. + // The resolver identification block — every busybox nslookup response + // begins with `Server: ... / Address: :53` — proves only that we + // reached *something* claiming to be a resolver, not that we got an + // answer. Real success requires either an actual `Name:`+`Address:` + // resolution pair OR an NXDOMAIN response body. + const probeAnswered = + /^Server:\s*\S/im.test(output) && + ((/\bName:\s*\S/i.test(output) && /\bAddress:\s*\d/.test(output)) || + /server can't find\b.*NXDOMAIN/i.test(output)); + if (probeAnswered) { return { ok: true }; }