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
50 changes: 34 additions & 16 deletions nemoclaw/src/blueprint/runner.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -71,9 +71,19 @@ vi.mock("execa", () => ({
execa: (...args: unknown[]) => mockExeca(...args),
}));

vi.mock("./ssrf.js", () => ({
validateEndpointUrl: vi.fn(async (url: string) => ({ url, pinnedUrl: url })),
}));
vi.mock("./ssrf.js", async (importOriginal) => {
const actual = await importOriginal<typeof import("./ssrf.js")>();
return {
...actual,
validateEndpointUrl: vi.fn(async (url: string) => ({
url,
pinnedUrl: url,
protocol: url.startsWith("http:") ? "http:" : "https:",
hostname: new URL(url).hostname,
dnsResolved: false,
})),
};
});

const { validateEndpointUrl } = await import("./ssrf.js");
const mockedValidateEndpoint = vi.mocked(validateEndpointUrl);
Expand Down Expand Up @@ -553,17 +563,6 @@ describe("runner", () => {
expect(plan.dry_run).toBe(true);
});

it("validates and applies endpoint URL override", async () => {
captureStdout();
mockExeca.mockResolvedValue({ exitCode: 0 });

const plan = await actionPlan("default", minimalBlueprint(), {
endpointUrl: "https://override.example.com/v1",
});
expect(plan.inference.endpoint).toBe("https://override.example.com/v1");
expect(mockedValidateEndpoint).toHaveBeenCalledWith("https://override.example.com/v1");
});

it("SSRF-validates the blueprint-defined endpoint even without --endpoint-url override", async () => {
captureStdout();
mockExeca.mockResolvedValue({ exitCode: 0 });
Expand Down Expand Up @@ -1081,9 +1080,28 @@ describe("runner", () => {

it("validates and applies endpoint URL override", async () => {
await actionApply("default", minimalBlueprint(), {
endpointUrl: "https://override.example.com/v1",
endpointUrl: "https://93.184.216.34/v1",
});
expect(mockedValidateEndpoint).toHaveBeenCalledWith("https://93.184.216.34/v1");
});

it("fails closed before provider creation for DNS-backed HTTPS endpoint overrides", async () => {
mockedValidateEndpoint.mockResolvedValueOnce({
url: "https://override.example.com/v1",
pinnedUrl: "https://93.184.216.34/v1",
protocol: "https:",
hostname: "override.example.com",
dnsResolved: true,
});
expect(mockedValidateEndpoint).toHaveBeenCalledWith("https://override.example.com/v1");

await expect(
actionApply("default", minimalBlueprint(), {
endpointUrl: "https://override.example.com/v1",
}),
).rejects.toThrow(/DNS-backed HTTPS endpoint/);
expect(
mockExeca.mock.calls.some((c) => Array.isArray(c[1]) && c[1].includes("provider")),
).toBe(false);
});

it("passes --timeout when timeout_secs is set in profile", async () => {
Expand Down
11 changes: 3 additions & 8 deletions nemoclaw/src/blueprint/runner.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ import { join, sep } from "node:path";
import { execa } from "execa";
import YAML from "yaml";

import { validateEndpointUrl } from "./ssrf.js";
import { safeEndpointUrlForDownstream, validateEndpointUrl } from "./ssrf.js";
import { buildSubprocessEnv } from "../lib/subprocess-env.js";
import { DASHBOARD_PORT } from "../lib/ports.js";

Expand Down Expand Up @@ -432,18 +432,13 @@ async function resolveRunConfig(
let inferenceCfg = { ...inferenceProfiles[profile] };
if (endpointUrl) {
const validated = await validateEndpointUrl(endpointUrl);
// Use DNS-pinned URL for HTTP (full SSRF/rebinding protection). For HTTPS,
// keep the original hostname — TLS certificate validation prevents rebinding
// since the attacker cannot present a valid cert for the target.
const safe = endpointUrl.startsWith("https:") ? validated.url : validated.pinnedUrl;
inferenceCfg = { ...inferenceCfg, endpoint: safe };
inferenceCfg = { ...inferenceCfg, endpoint: safeEndpointUrlForDownstream(validated) };
}

// Validate the final endpoint (whether from CLI override or blueprint profile)
if (inferenceCfg.endpoint) {
const validated = await validateEndpointUrl(inferenceCfg.endpoint);
const safe = inferenceCfg.endpoint.startsWith("https:") ? validated.url : validated.pinnedUrl;
inferenceCfg = { ...inferenceCfg, endpoint: safe };
inferenceCfg = { ...inferenceCfg, endpoint: safeEndpointUrlForDownstream(validated) };
}

const sandboxCfg = blueprint.components?.sandbox ?? {};
Expand Down
36 changes: 30 additions & 6 deletions nemoclaw/src/blueprint/ssrf.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,9 @@ vi.mock("node:dns", () => ({
promises: { lookup: (...args: unknown[]) => mockLookup(...(args as [string, { all: true }])) },
}));

const { isPrivateIp, validateEndpointUrl } = await import("./ssrf.js");
const { isPrivateIp, safeEndpointUrlForDownstream, validateEndpointUrl } = await import(
"./ssrf.js"
);

// ── isPrivateIp ─────────────────────────────────────────────────

Expand Down Expand Up @@ -340,10 +342,29 @@ describe("validateEndpointUrl – DNS pinning", () => {
expect(result.url).toBe("http://attacker.com:8080/v1");
});

it("pins HTTPS URL to resolved IP", async () => {
it("pins HTTPS URL to resolved IP metadata without marking it downstream-safe", async () => {
mockPublicDns();
const result = await validateEndpointUrl("https://api.example.com/v1");
expect(result.pinnedUrl).toBe("https://93.184.216.34/v1");
expect(result).toMatchObject({
protocol: "https:",
hostname: "api.example.com",
resolvedAddress: "93.184.216.34",
resolvedFamily: 4,
dnsResolved: true,
});
expect(() => safeEndpointUrlForDownstream(result)).toThrow(/DNS-backed HTTPS endpoint/);
});

it("keeps HTTPS IP-literal endpoints downstream-safe", async () => {
const result = await validateEndpointUrl("https://93.184.216.34/v1");
expect(safeEndpointUrlForDownstream(result)).toBe("https://93.184.216.34/v1");
});

it("returns pinned HTTP endpoints for downstream use", async () => {
mockPublicDns();
const result = await validateEndpointUrl("http://api.example.com/v1");
expect(safeEndpointUrlForDownstream(result)).toBe("http://93.184.216.34/v1");
});

it("pins IPv6 address with brackets", async () => {
Expand All @@ -369,25 +390,28 @@ describe("validateEndpointUrl – URL parsing edge cases", () => {
);
});

it("allows URL with query parameters", async () => {
it("parses URL with query parameters but does not mark DNS-backed HTTPS downstream-safe", async () => {
mockPublicDns();
const url = "https://api.example.com/v1?key=abc&model=gpt";
const result = await validateEndpointUrl(url);
expect(result.url).toBe(url);
expect(() => safeEndpointUrlForDownstream(result)).toThrow(/DNS-backed HTTPS endpoint/);
});

it("allows URL with fragment", async () => {
it("parses URL with fragment but does not mark DNS-backed HTTPS downstream-safe", async () => {
mockPublicDns();
const url = "https://api.example.com/v1#section";
const result = await validateEndpointUrl(url);
expect(result.url).toBe(url);
expect(() => safeEndpointUrlForDownstream(result)).toThrow(/DNS-backed HTTPS endpoint/);
});

it("allows URL with userinfo/basic auth", async () => {
it("parses URL with userinfo/basic auth but does not mark DNS-backed HTTPS downstream-safe", async () => {
mockPublicDns();
// URL parser extracts hostname correctly even with userinfo
// URL parser extracts hostname correctly even with userinfo.
const url = "https://user:pass@api.example.com/v1";
const result = await validateEndpointUrl(url);
expect(result.url).toBe(url);
expect(() => safeEndpointUrlForDownstream(result)).toThrow(/DNS-backed HTTPS endpoint/);
});
});
41 changes: 36 additions & 5 deletions nemoclaw/src/blueprint/ssrf.ts
Original file line number Diff line number Diff line change
Expand Up @@ -24,13 +24,21 @@ function hostnameForDnsLookup(hostname: string): string {
* DNS rebinding TOCTOU attacks where an attacker returns a public IP at
* validation time and a private IP at connection time.
*
* Callers should use `pinnedUrl` for HTTP endpoints (full protection) and `url`
* for HTTPS endpoints (TLS certificate validation prevents rebinding since the
* attacker cannot present a valid cert for the rebinding target).
* Callers should use `safeEndpointUrlForDownstream` before passing the endpoint
* to a downstream provider. DNS-backed HTTP endpoints are rewritten to
* `pinnedUrl`; DNS-backed HTTPS endpoints currently fail closed because the
* downstream provider would otherwise perform a second DNS lookup while NemoClaw
* cannot pin the TCP peer and preserve TLS SNI/Host across the OpenShell runtime
* boundary.
*/
export interface ValidatedEndpoint {
url: string;
pinnedUrl: string;
protocol: "http:" | "https:";
hostname: string;
resolvedAddress?: string;
resolvedFamily?: number;
dnsResolved: boolean;
}

export async function validateEndpointUrl(url: string): Promise<ValidatedEndpoint> {
Expand Down Expand Up @@ -60,8 +68,9 @@ export async function validateEndpointUrl(url: string): Promise<ValidatedEndpoin
}

const lookupHostname = hostnameForDnsLookup(hostname);
const protocol = parsed.protocol as "http:" | "https:";
if (isIP(lookupHostname)) {
return { url, pinnedUrl: url };
return { url, pinnedUrl: url, protocol, hostname, dnsResolved: false };
}

let addresses: Array<{ address: string; family: number }>;
Expand Down Expand Up @@ -89,5 +98,27 @@ export async function validateEndpointUrl(url: string): Promise<ValidatedEndpoin
const first = addresses[0];
pinned.hostname = first.family === 6 ? `[${first.address}]` : first.address;

return { url, pinnedUrl: pinned.toString() };
return {
url,
pinnedUrl: pinned.toString(),
protocol,
hostname,
resolvedAddress: first.address,
resolvedFamily: first.family,
dnsResolved: true,
};
}

export function safeEndpointUrlForDownstream(validated: ValidatedEndpoint): string {
if (validated.protocol === "https:" && validated.dnsResolved) {
throw new Error(
`DNS-backed HTTPS endpoint '${validated.hostname}' is not supported yet because ` +
"NemoClaw cannot guarantee the downstream provider connects to the same IP " +
"that passed SSRF validation across the OpenShell runtime boundary. " +
"Use an HTTPS IP-literal endpoint, an HTTP endpoint that can be DNS-pinned, " +
"or wait for the runtime-aware HTTPS pinning transport.",
);
}

return validated.protocol === "http:" ? validated.pinnedUrl : validated.url;
}
Loading
Loading