Skip to content

Commit c7b780e

Browse files
committed
fix(onboard): reject an unsafe custom endpoint URL before any mutation
Custom endpoint intake validated only userinfo, query, and fragment components (#9106), so an endpoint URL containing shell metacharacters, percent-encoded control characters, raw control characters, or a non-HTTP(S) value passed intake and reached the SSRF preflight, the endpoint probe, provider registration, session checkpoint writes, and registry writes before any deep layer rejected it — and nothing rejected percent-encoded control characters at all. One composite classification, unsafeEndpointUrlViolation, now owns the rejection rules, and every custom endpoint intake consumes it before mutating state: onboarding intake (interactive and non-interactive), inference set --endpoint-url before DNS resolution, and rebuild resume preflight, which treats a violating recorded value as unknown metadata. The character allowlist matches the container startup-command token set plus "~", so an accepted URL stays inert across every downstream consumer; the sets stay separate because command tokens and endpoint URLs are distinct contracts. Rejection reasons are static and never echo the input. The #9106 class keeps its established message and hint, and the inference set shape check keeps its established message for the classes it already owned. The integration rows prove the QA contract directly: a subprocess onboard with an unsafe URL exits 1 with no probe request and no onboard-session.json or sandboxes.json write under the test HOME. The url-utils fan-in budget moves 29 -> 30 for the one new inference-set importer, the same adjustment #9119 made for this file. Fixes #9301 Signed-off-by: Dongni Yang <dongniy@nvidia.com>
1 parent 588bb6d commit c7b780e

11 files changed

Lines changed: 266 additions & 22 deletions

ci/source-architecture-budget.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@
1717
"src/lib/core/json-types.ts": 36,
1818
"src/lib/core/ports.ts": 89,
1919
"src/lib/core/shell-quote.ts": 28,
20-
"src/lib/core/url-utils.ts": 29,
20+
"src/lib/core/url-utils.ts": 30,
2121
"src/lib/core/wait.ts": 36,
2222
"src/lib/credentials/store.ts": 46,
2323
"src/lib/inference/config.ts": 30,

docs/inference/custom-endpoint-security.mdx

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,12 @@ Custom endpoint onboarding rejects endpoint URLs that contain userinfo, query, o
3636
NemoClaw does not forward those components to the endpoint.
3737
Configure the provider credential separately instead of putting it in the endpoint URL.
3838

39+
Custom endpoint onboarding also rejects an endpoint URL that contains control characters, percent-encoded control characters, spaces, shell metacharacters, or other characters outside the URL-safe ASCII set.
40+
It also rejects an input that is not an absolute HTTP or HTTPS URL.
41+
This rejection happens before any network request, provider registration, registry write, or sandbox and image mutation, so a rejected input changes no NemoClaw state.
42+
The `inference set` command applies the same rejection classes to `--endpoint-url` before DNS resolution.
43+
Sandbox rebuild applies the same rejection classes to recorded custom endpoint metadata and treats a violating value as unknown.
44+
3945
Managed provider defaults that do not provide an explicit custom endpoint through these paths are unaffected.
4046

4147
Custom endpoint onboarding has one narrower operator-controlled exception for corporate inference gateways.

docs/reference/commands.mdx

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4737,7 +4737,7 @@ Set them before running `$$nemoclaw onboard`.
47374737
| `NEMOCLAW_LLAMACPP_RECIPE` | repository-owned managed-inference recipe ID | Selects the exact managed llama.cpp recipe when `NEMOCLAW_PROVIDER=install-llama-cpp`, including a compatible lower-priority profile. When unset, NemoClaw selects the unique highest-priority compatible automatic profile. An unknown recipe, an ambiguous selection, or a stale or incompatible readiness report fails before image, model, or runtime effects. |
47384738
| `NEMOCLAW_MODEL` | model ID | Selects an explicit model for a non-interactive onboarding run. NemoClaw preserves it across a detected provider switch, even when it matches the recorded provider's default. When this variable is unset during such a switch, NemoClaw ignores the `NEMOCLAW_PROVIDER_MODEL` compatibility fallback and uses normal provider model selection. |
47394739
| `NEMOCLAW_TOOL_DISCLOSURE` | `progressive` or `direct` | Selects progressive tool discovery or the prior direct-exposure behavior. Defaults to `progressive`; `--tool-disclosure` takes precedence when both are set. |
4740-
| `NEMOCLAW_ENDPOINT_URL` | URL | Custom endpoint URL. Used together with `NEMOCLAW_PROVIDER=custom` for OpenAI-compatible endpoints or `NEMOCLAW_PROVIDER=anthropicCompatible` for Anthropic-compatible endpoints. Onboarding rejects a URL that contains userinfo, query, or fragment components. |
4740+
| `NEMOCLAW_ENDPOINT_URL` | URL | Custom endpoint URL. Used together with `NEMOCLAW_PROVIDER=custom` for OpenAI-compatible endpoints or `NEMOCLAW_PROVIDER=anthropicCompatible` for Anthropic-compatible endpoints. Onboarding rejects a URL that contains userinfo, query, or fragment components. It also rejects a URL that contains control characters, percent-encoded control characters, spaces, shell metacharacters, or other characters outside the URL-safe ASCII set, and a value that is not an absolute HTTP or HTTPS URL. |
47414741
| `NEMOCLAW_COMPATIBLE_AUTH_MODE` | `none` or unset | Explicitly selects no authentication for an HTTP OpenAI-compatible endpoint using `localhost`, `127.0.0.1`, or `[::1]` and port `8000`, `11434`, or `11435` during non-interactive onboarding. |
47424742
| `NEMOCLAW_TRUSTED_PRIVATE_HOSTS` | comma-separated exact hostnames or IP literals | Allows operator-owned RFC1918, CGNAT, or IPv6 unique local destinations through supported inference, managed MCP, and custom-policy registration paths. Link-local metadata and other reserved ranges remain blocked; DNS resolution and exact address pinning remain active; wildcards are not supported. |
47434743
| `NEMOCLAW_TRUSTED_PRIVATE_INFERENCE_HOSTS` | comma-separated exact hostnames or IP literals | Inference-only compatibility alias. Inference onboarding combines entries from this variable and `NEMOCLAW_TRUSTED_PRIVATE_HOSTS`. |

src/lib/actions/inference-set-endpoint-security.test.ts

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,29 @@ describe("custom inference endpoint DNS pinning", () => {
3838
).rejects.toThrow(/endpoint-url is not allowed:.*private\/internal address/i);
3939
});
4040

41+
it.each([
42+
[
43+
"shell metacharacters",
44+
"http://public.example/v1$(id)",
45+
/endpoint-url must contain only URL-safe ASCII characters\./,
46+
],
47+
[
48+
"percent-encoded control characters",
49+
"http://public.example/v1%0ainjected",
50+
/endpoint-url must not contain percent-encoded control characters\./,
51+
],
52+
] as const)(
53+
"rejects an endpoint URL with %s before DNS validation or any mutation (#9301)",
54+
async (_label, endpointUrl, message) => {
55+
const rewriteUrl = vi.fn(async () => {
56+
throw new Error("unsafe endpoint unexpectedly reached DNS validation");
57+
});
58+
59+
await expect(normalizeCustomEndpointUrl(endpointUrl, rewriteUrl)).rejects.toThrow(message);
60+
expect(rewriteUrl).not.toHaveBeenCalled();
61+
},
62+
);
63+
4164
it("pins validated public HTTP endpoints before they become durable metadata", async () => {
4265
const lookup = vi.fn(async () => [{ address: "93.184.216.34", family: 4 }]);
4366

src/lib/actions/inference-set-route-containment.ts

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ import {
1111
type HttpsPinCredentialProviderType,
1212
isHttpsPinRuntimeEligible,
1313
} from "../inference/https-pin-runtime";
14+
import { unsafeEndpointUrlViolation } from "../core/url-utils";
1415
import { resolveSandboxGatewayName } from "../onboard/gateway-binding";
1516
import { isAllowedOpenShellSandboxBridgeUrl } from "../private-networks";
1617
import { ConfigUrlValidationError } from "../sandbox/config";
@@ -136,14 +137,25 @@ function normalizeCustomEndpointUrlWithoutDns(value: string | null | undefined):
136137
const raw = typeof value === "string" ? value.trim() : "";
137138
if (!raw)
138139
throw new InferenceSetError("endpoint-url is required for custom-compatible metadata.", 2);
140+
let normalized: string;
139141
try {
140-
return normalizeEndpointUrlShape(raw).normalized;
142+
normalized = normalizeEndpointUrlShape(raw).normalized;
141143
} catch {
142144
throw new InferenceSetError(
143145
"endpoint-url must be a valid http(s) URL without userinfo, query, or fragment components.",
144146
2,
145147
);
146148
}
149+
// #9301: reject control characters, percent-encoded control characters,
150+
// spaces, and shell metacharacters before any provider, registry, or
151+
// sandbox mutation, matching onboarding intake. The shape check above owns
152+
// the userinfo, query, fragment, scheme, and parse classes and their
153+
// established message.
154+
const violation = unsafeEndpointUrlViolation(raw);
155+
if (violation) {
156+
throw new InferenceSetError(`endpoint-url ${violation.reason}`, 2);
157+
}
158+
return normalized;
147159
}
148160

149161
export async function normalizeCustomEndpointUrl(

src/lib/actions/sandbox/rebuild-resume-config.test.ts

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -133,6 +133,12 @@ describe("getRebuildEndpointFromRegistry", () => {
133133
expect(
134134
getRebuildEndpointFromRegistry("compatible-endpoint", "https://example.test/v1?x=1"),
135135
).toEqual({ known: false });
136+
expect(
137+
getRebuildEndpointFromRegistry("compatible-endpoint", "https://example.test/v1;id"),
138+
).toEqual({ known: false });
139+
expect(
140+
getRebuildEndpointFromRegistry("compatible-endpoint", "https://example.test/v1%0ax"),
141+
).toEqual({ known: false });
136142
expect(getRebuildEndpointFromRegistry("compatible-endpoint", "http://@example.test/v1")).toEqual(
137143
{ known: false },
138144
);

src/lib/actions/sandbox/rebuild-resume-preflight.ts

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@
22
// SPDX-License-Identifier: Apache-2.0
33

44
import { D, R } from "../../cli/terminal-style";
5-
import { endpointUrlHasUserinfoQueryOrFragment } from "../../core/url-utils";
5+
import { unsafeEndpointUrlViolation } from "../../core/url-utils";
66
import type { InferenceSelection } from "../../inference/selection";
77
import type { RegistryInferenceRoute } from "../../onboard/rebuild-route-handoff";
88
import { isRecoveredProviderCredentialReuseSelectionKey } from "../../onboard/recovered-provider-reuse";
@@ -98,9 +98,9 @@ const SESSION_ONLY_ENDPOINT_PROVIDER_NAMES = new Set(
9898

9999
export function canonicalCustomEndpointUrl(value: string | null | undefined): string | null {
100100
const raw = typeof value === "string" ? value.trim() : "";
101-
// #9106: reject userinfo, query, and fragment components instead of
102-
// stripping them, matching onboarding intake.
103-
if (endpointUrlHasUserinfoQueryOrFragment(raw)) return null;
101+
// #9106/#9301: reject unsafe endpoint metadata instead of stripping or
102+
// forwarding it, matching onboarding intake.
103+
if (unsafeEndpointUrlViolation(raw)) return null;
104104
try {
105105
const url = new URL(raw);
106106
const supportedProtocol = url.protocol === "http:" || url.protocol === "https:";

src/lib/core/url-utils.test.ts

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ import {
1212
normalizeProviderBaseUrl,
1313
parsePolicyPresetEnv,
1414
stripEndpointSuffix,
15+
unsafeEndpointUrlViolation,
1516
} from "./url-utils";
1617

1718
describe("compactText", () => {
@@ -145,6 +146,44 @@ describe("endpointUrlHasUserinfoQueryOrFragment", () => {
145146
});
146147
});
147148

149+
describe("unsafeEndpointUrlViolation", () => {
150+
it.each([
151+
["backtick command substitution", "http://127.0.0.1:8000/v1`whoami`", "unsupported-characters"],
152+
["dollar command substitution", "http://127.0.0.1:8000/v1$(id)", "unsupported-characters"],
153+
["semicolon in the path", "https://example.test/v1;id", "unsupported-characters"],
154+
["pipe in the path", "https://example.test/v1|cat", "unsupported-characters"],
155+
["ampersand in the path", "https://example.test/v1&x", "unsupported-characters"],
156+
["double quote", 'https://example.test/v1"q"', "unsupported-characters"],
157+
["single quote", "https://example.test/v1'q'", "unsupported-characters"],
158+
["interior space", "https://example.test/v 1", "unsupported-characters"],
159+
["encoded newline", "https://example.test/v1%0ainjected", "encoded-control-characters"],
160+
["encoded carriage return uppercase", "https://example.test/v1%0Dx", "encoded-control-characters"],
161+
["encoded NUL", "https://example.test/v1%00x", "encoded-control-characters"],
162+
["raw tab", "https://example.test/v\t1", "control-characters"],
163+
["raw newline", "https://example.test/v\n1", "control-characters"],
164+
["query string", "http://127.0.0.1:8000/v1?param=value", "userinfo-query-fragment"],
165+
["userinfo", "https://user:password@example.test/v1", "userinfo-query-fragment"],
166+
["non-HTTP scheme", "ftp://example.test/v1", "unsupported-protocol"],
167+
["scheme-less host and port", "localhost:8000/v1", "unsupported-protocol"],
168+
["scheme-less host path", "example.test/v1", "invalid-url"],
169+
["non-ASCII host", "https://exämple.test/v1", "unsupported-characters"],
170+
] as const)("rejects %s (#9301)", (_label, input, kind) => {
171+
expect(unsafeEndpointUrlViolation(input)?.kind).toBe(kind);
172+
});
173+
174+
it.each([
175+
["IPv6 loopback with port", "http://[::1]:8000/v1"],
176+
["host with port and deep path", "https://example.test:8443/deep/path-v1"],
177+
["path with URL-legal punctuation", "http://example.test/v1_x.y~z"],
178+
["percent-encoded space in the path", "https://example.test/v1/a%20b"],
179+
["clean origin", "https://proxy.example.com"],
180+
["empty input", ""],
181+
["whitespace input", " "],
182+
] as const)("accepts %s (#9301)", (_label, input) => {
183+
expect(unsafeEndpointUrlViolation(input)).toBeNull();
184+
});
185+
});
186+
148187
describe("isLoopbackHostname", () => {
149188
it.each([
150189
["localhost", true],

src/lib/core/url-utils.ts

Lines changed: 65 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -73,6 +73,71 @@ export function endpointUrlHasUserinfoQueryOrFragment(value: string | null | und
7373
}
7474
}
7575

76+
// Endpoint URL inputs feed provider registration, registry writes, Dockerfile
77+
// ARGs, and container startup commands, so intake accepts only characters that
78+
// stay inert across every downstream consumer. The set matches the
79+
// startup-command token allowlist in onboard/docker-startup-command-env.ts
80+
// plus "~"; the two sets stay separate because command tokens and endpoint
81+
// URLs are distinct contracts.
82+
const ENDPOINT_URL_ALLOWED_CHARACTERS = /^[A-Za-z0-9_./:=,@%+\-[\]~]+$/u;
83+
const PERCENT_ENCODED_CONTROL_CHARACTER = /%(?:[01][0-9a-f]|7f)/i;
84+
85+
export type EndpointUrlViolation = {
86+
kind:
87+
| "userinfo-query-fragment"
88+
| "control-characters"
89+
| "encoded-control-characters"
90+
| "unsupported-characters"
91+
| "invalid-url"
92+
| "unsupported-protocol";
93+
reason: string;
94+
};
95+
96+
/**
97+
* Classify an endpoint URL input that onboarding must reject before any
98+
* network request, provider registration, registry write, or sandbox and
99+
* image mutation (#9301). Returns null for an empty input (emptiness is a
100+
* separate required-input error) and for a safe absolute HTTP(S) URL. The
101+
* reason completes the sentence "Endpoint URL ..." and never echoes the
102+
* input value.
103+
*/
104+
export function unsafeEndpointUrlViolation(
105+
value: string | null | undefined,
106+
): EndpointUrlViolation | null {
107+
const raw = String(value || "").trim();
108+
if (!raw) return null;
109+
if (endpointUrlHasUserinfoQueryOrFragment(raw)) {
110+
return {
111+
kind: "userinfo-query-fragment",
112+
reason: "must not contain userinfo, query, or fragment components.",
113+
};
114+
}
115+
if (/[\p{Cc}\p{Cf}]/u.test(raw)) {
116+
return { kind: "control-characters", reason: "must not contain control characters." };
117+
}
118+
if (PERCENT_ENCODED_CONTROL_CHARACTER.test(raw)) {
119+
return {
120+
kind: "encoded-control-characters",
121+
reason: "must not contain percent-encoded control characters.",
122+
};
123+
}
124+
if (!ENDPOINT_URL_ALLOWED_CHARACTERS.test(raw)) {
125+
return {
126+
kind: "unsupported-characters",
127+
reason: "must contain only URL-safe ASCII characters.",
128+
};
129+
}
130+
try {
131+
const url = new URL(raw);
132+
if (url.protocol !== "http:" && url.protocol !== "https:") {
133+
return { kind: "unsupported-protocol", reason: "must use HTTP or HTTPS." };
134+
}
135+
} catch {
136+
return { kind: "invalid-url", reason: "must be a valid HTTP or HTTPS URL." };
137+
}
138+
return null;
139+
}
140+
76141
/** Return the bounded canonical form of a credential-free HTTP(S) provider endpoint. */
77142
export function canonicalEndpoint(
78143
value: string | null | undefined,

src/lib/onboard/setup-nim-selection.ts

Lines changed: 18 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -3,8 +3,8 @@
33

44
import {
55
canonicalEndpoint,
6-
endpointUrlHasUserinfoQueryOrFragment,
76
normalizeProviderBaseUrl,
7+
unsafeEndpointUrlViolation,
88
} from "../core/url-utils";
99
import { applyCompatibleEndpointContextWindow } from "../inference/compatible-endpoint-context";
1010
import type { TrustedPrivateEndpointCapability } from "../inference/endpoint-ssrf-preflight";
@@ -142,21 +142,24 @@ export async function resolveCompatibleEndpointSelection(args: {
142142
if (navigation === "exit") {
143143
exitOnboardFromPrompt();
144144
}
145-
// #9106: reject instead of silently stripping components that NemoClaw
146-
// cannot forward to the endpoint.
147-
if (endpointUrlHasUserinfoQueryOrFragment(endpointInput)) {
148-
console.error(" Endpoint URL must not contain userinfo, query, or fragment components.");
149-
// canonicalEndpoint returns null unless the stripped base is a
150-
// credential-free http(s) URL, so the hint never echoes userinfo or
151-
// query values.
152-
const strippedBaseUrl = canonicalEndpoint(
153-
normalizeProviderBaseUrl(endpointInput, args.kind),
154-
args.kind,
155-
);
156-
if (strippedBaseUrl) {
157-
console.error(
158-
` NemoClaw does not forward these components to the endpoint. Use: ${strippedBaseUrl}`,
145+
// #9106/#9301: reject unsafe endpoint input here, before any network
146+
// request, provider registration, registry write, or sandbox mutation.
147+
const endpointViolation = unsafeEndpointUrlViolation(endpointInput);
148+
if (endpointViolation) {
149+
console.error(` Endpoint URL ${endpointViolation.reason}`);
150+
if (endpointViolation.kind === "userinfo-query-fragment") {
151+
// canonicalEndpoint returns null unless the stripped base is a
152+
// credential-free http(s) URL, so the hint never echoes userinfo or
153+
// query values.
154+
const strippedBaseUrl = canonicalEndpoint(
155+
normalizeProviderBaseUrl(endpointInput, args.kind),
156+
args.kind,
159157
);
158+
if (strippedBaseUrl) {
159+
console.error(
160+
` NemoClaw does not forward these components to the endpoint. Use: ${strippedBaseUrl}`,
161+
);
162+
}
160163
}
161164
if (args.nonInteractive) {
162165
process.exit(1);

0 commit comments

Comments
 (0)