Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
59 changes: 55 additions & 4 deletions src/lib/onboard/preflight.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ import {
parseDockerStorageDriver,
parseDockerUsesContainerdSnapshotter,
planHostRemediation,
dnsProbeName,
probeContainerDns,
} from "../../../dist/lib/onboard/preflight";

Expand Down Expand Up @@ -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");
Expand Down Expand Up @@ -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");
});

Expand All @@ -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");
Expand Down
46 changes: 43 additions & 3 deletions src/lib/onboard/preflight.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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";
Expand Down Expand Up @@ -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;
}

/**
Expand All @@ -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
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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: <ip>: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 };
}

Expand Down
Loading