Skip to content

Commit d34dab5

Browse files
udsy19prekshivyasjyaunches
authored
fix(inference): validate every runtime adapter port through one shared path (#9545)
## Summary The Bedrock Runtime adapter was the only one of the three local runtime adapters that never validated its configured host port, so `NEMOCLAW_BEDROCK_RUNTIME_ADAPTER_PORT` could place it on the Ollama inference port, the Ollama auth proxy port, a port inside the dashboard allocation range, or any other configured service port. Its two siblings reject all of those before spawning anything. After this change all three adapters reject the same collisions and produce the same named errors. ## Related Issue Fixes #9544 ## Changes `src/lib/core/ports.ts` - Deleted `validateBedrockRuntimeAdapterPort`, `validateOpenRouterRuntimeAdapterPort` and `validateHttpsPinRuntimeAdapterPort`. - Added one `CURRENT_RUNTIME_PORT_CONFIGURATION` next to the catalog, and `validateRuntimeAdapterPort(ownerEnvVar, port, options = CURRENT_RUNTIME_PORT_CONFIGURATION)`, which calls `validateServicePort(ownerEnvVar, port, options, ownerEnvVar)`. - `validateLlamaCppPortReservation`'s bottom call now takes that same object instead of rebuilding the ten fields a second time. `src/lib/inference/bedrock-runtime-adapter.ts`, `openrouter-runtime-adapter-lifecycle.ts`, `https-pin-runtime-adapter.ts` - Deleted all three `validateAdapterPortConfiguration()` helpers and the eight now-dead port imports in each of the two sibling files. - Each ensure path opens with a direct `validateRuntimeAdapterPort("<its own env var>", <its own port>)`. `src/lib/core/ports.test.ts` - The three near-identical describes collapse into one `validateRuntimeAdapterPort` describe: an `ADAPTER_OWNERS` table of the three owner/default/label triples, a `SHARED_CONFLICTS` table, and a generated matrix crossing each owner against the shared conflicts *and* the other two adapters' defaults. Same assertions as before, one copy. Plus one case proving the no-options overload validates against the live configuration. `src/lib/inference/runtime-adapter-port-collisions.test.ts` - Added in `f1f589858` by @prekshivyas. It covers all three public ensure paths symmetrically, so it answers the boundary-test request without reintroducing the per-adapter asymmetry that a Bedrock-only ensure test would have created. Each case asserts the named validation error and that no adapter process is spawned and no pid, token, or state file is written. Production is net **+42 / −94 = −52 lines** and is unchanged by the follow-up commit. Including the new boundary-test file, the complete PR is **+191 / −141 = +50 lines** across the repo. An earlier revision of this description quoted the repo-wide figure from before that file existed. ## Type of Change - [x] Code change (feature, bug fix, or refactor) - [ ] Code change with doc updates - [ ] Doc only (prose changes, no code sample modifications) - [ ] Doc only (includes code sample changes) ## Quality Gates - [x] Tests added or updated for changed behavior - [ ] Existing tests cover changed behavior — justification: - [ ] Tests not applicable — justification: - [x] Sensitive paths changed (security, policy, credentials, preflight, onboarding, inference, runner, sandbox, or messaging) - [ ] Sensitive-path review completed or maintainer-approved waiver recorded — reviewer/approval link/justification: - [ ] Non-success, skipped, or missing CI check accepted by maintainer — check name, approval link, and follow-up issue: ## Verification - [x] PR description includes a `Signed-off-by:` line and every commit appears as `Verified` in GitHub - [x] Normal `pre-commit`, `commit-msg`, and `pre-push` hooks passed, or `npm run validate:pr` passed after refreshing `origin/main` when hooks were skipped or unavailable - [x] Targeted behavior tests pass for the current change set, or tests are marked not applicable above — command/result or justification: `npx vitest run --project cli src/lib/core/ports.test.ts src/lib/inference/bedrock-runtime-adapter.test.ts` — 73 passed (73). Without the change the nine new cases fail; with it they pass. `npm run validate:pr` passed, including `Codebase growth guardrails` and `TypeScript (CLI)`. - [ ] Applicable broad gate passed — `npm test` for broad runtime/test-harness changes; `npm run check` for repo-wide validation/coverage changes — command/result: - [x] Quality Gates section completed with required justifications or waivers - [x] No secrets, API keys, or credentials committed - [ ] `npm run docs` builds without warnings (doc changes only) - [ ] Doc pages follow the [style guide](https://github.com/NVIDIA/NemoClaw/blob/main/docs/CONTRIBUTING.md) (doc changes only) - [ ] New doc pages include SPDX header and frontmatter (new pages only) --- **Note on overlap with #9537:** #9537 restructures the same file and converges on the same shape — it rewrites `validateLlamaCppPortReservation` into a direct `validateServicePort(...)` call. This revision no longer merges cleanly with it: exactly one file and one region conflict, the doc comment above `validateLlamaCppPortReservation`. I resolved it in a scratch worktree and the port tests still pass there. Happy to rebase on #9537 the moment it lands, or to reorder if you would rather take this one first. Signed-off-by: Udaya Tejas <udayatejas2004@gmail.com> <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Bug Fixes** * Improved port validation for Bedrock, OpenRouter, and HTTPS pin runtime adapters. * Prevented runtime adapters from using ports reserved by other services or adapters. * Added safeguards against overlapping and conflicting port assignments. * Ensured adapter startup checks reflect the current service configuration. * Prevented conflicting adapters from starting or creating incomplete runtime state. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Signed-off-by: Udaya Tejas <udayatejas2004@gmail.com> Signed-off-by: Prekshi Vyas <prekshiv@nvidia.com> Signed-off-by: Julie Yaunches <jyaunches@nvidia.com> Co-authored-by: Prekshi Vyas <prekshiv@nvidia.com> Co-authored-by: Julie Yaunches <jyaunches@nvidia.com>
1 parent 722ff55 commit d34dab5

6 files changed

Lines changed: 191 additions & 141 deletions

src/lib/core/ports.test.ts

Lines changed: 32 additions & 47 deletions
Original file line numberDiff line numberDiff line change
@@ -6,9 +6,8 @@ import { afterEach, beforeEach, describe, expect, it } from "vitest";
66
import {
77
parseGatewayPort,
88
parsePort,
9-
validateHttpsPinRuntimeAdapterPort,
109
validateLlamaCppPortReservation,
11-
validateOpenRouterRuntimeAdapterPort,
10+
validateRuntimeAdapterPort,
1211
} from "./ports";
1312

1413
const GATEWAY_VALIDATION_OPTIONS = {
@@ -181,68 +180,54 @@ describe("validateLlamaCppPortReservation", () => {
181180
});
182181
});
183182

184-
describe("validateOpenRouterRuntimeAdapterPort", () => {
185-
const ENV_KEY = "NEMOCLAW_OPENROUTER_RUNTIME_ADAPTER_PORT";
183+
describe("validateRuntimeAdapterPort", () => {
184+
const ADAPTER_OWNERS = [
185+
["NEMOCLAW_BEDROCK_RUNTIME_ADAPTER_PORT", 11436, "Bedrock Runtime adapter"],
186+
["NEMOCLAW_OPENROUTER_RUNTIME_ADAPTER_PORT", 11437, "OpenRouter Runtime adapter"],
187+
["NEMOCLAW_HTTPS_PIN_RUNTIME_ADAPTER_PORT", 11438, "HTTPS Pin Runtime adapter"],
188+
] as const;
186189

187-
it("allows the default OpenRouter Runtime adapter port", () => {
188-
expect(() =>
189-
validateOpenRouterRuntimeAdapterPort(ENV_KEY, 11437, GATEWAY_VALIDATION_OPTIONS),
190-
).not.toThrow();
191-
});
192-
193-
it.each([
190+
const SHARED_CONFLICTS = [
194191
[8080, "NEMOCLAW_GATEWAY_PORT"],
195192
[8000, "vLLM / NIM inference"],
196193
[11434, "Ollama inference"],
197194
[11435, "Ollama auth proxy"],
198-
[11436, "Bedrock Runtime adapter"],
199-
[11438, "HTTPS Pin Runtime adapter"],
200195
[18790, "18789-18799"],
201-
])("rejects OpenRouter adapter overlap with %s", (port, expectedMessage) => {
202-
expect(() =>
203-
validateOpenRouterRuntimeAdapterPort(ENV_KEY, port, GATEWAY_VALIDATION_OPTIONS),
204-
).toThrow(expectedMessage);
205-
});
196+
] as const;
206197

207-
it("rejects OpenRouter adapter overlap with configured service ports", () => {
198+
it.each(ADAPTER_OWNERS)("allows the default port owned by %s", (ownerEnvVar, defaultPort) => {
208199
expect(() =>
209-
validateOpenRouterRuntimeAdapterPort(ENV_KEY, 19001, {
210-
...GATEWAY_VALIDATION_OPTIONS,
211-
vllmPort: 19001,
212-
}),
213-
).toThrow("NEMOCLAW_VLLM_PORT");
214-
});
215-
});
216-
217-
describe("validateHttpsPinRuntimeAdapterPort", () => {
218-
const ENV_KEY = "NEMOCLAW_HTTPS_PIN_RUNTIME_ADAPTER_PORT";
219-
220-
it("allows the default HTTPS Pin Runtime adapter port", () => {
221-
expect(() =>
222-
validateHttpsPinRuntimeAdapterPort(ENV_KEY, 11438, GATEWAY_VALIDATION_OPTIONS),
200+
validateRuntimeAdapterPort(ownerEnvVar, defaultPort, GATEWAY_VALIDATION_OPTIONS),
223201
).not.toThrow();
224202
});
225203

226-
it.each([
227-
[8080, "NEMOCLAW_GATEWAY_PORT"],
228-
[8000, "vLLM / NIM inference"],
229-
[11434, "Ollama inference"],
230-
[11435, "Ollama auth proxy"],
231-
[11436, "Bedrock Runtime adapter"],
232-
[11437, "OpenRouter Runtime adapter"],
233-
[18790, "18789-18799"],
234-
])("rejects HTTPS Pin adapter overlap with %s", (port, expectedMessage) => {
235-
expect(() =>
236-
validateHttpsPinRuntimeAdapterPort(ENV_KEY, port, GATEWAY_VALIDATION_OPTIONS),
237-
).toThrow(expectedMessage);
204+
it.each(
205+
ADAPTER_OWNERS.flatMap(([ownerEnvVar, defaultPort]) =>
206+
[
207+
...SHARED_CONFLICTS,
208+
...ADAPTER_OWNERS.filter(([, otherPort]) => otherPort !== defaultPort).map(
209+
([, otherPort, otherLabel]) => [otherPort, otherLabel] as const,
210+
),
211+
].map(([port, expectedMessage]) => [ownerEnvVar, port, expectedMessage] as const),
212+
),
213+
)("rejects %s on %d because it overlaps %s", (ownerEnvVar, port, expectedMessage) => {
214+
expect(() => validateRuntimeAdapterPort(ownerEnvVar, port, GATEWAY_VALIDATION_OPTIONS)).toThrow(
215+
expectedMessage,
216+
);
238217
});
239218

240-
it("rejects HTTPS Pin adapter overlap with configured service ports", () => {
219+
it.each(ADAPTER_OWNERS)("rejects %s on a configured non-default service port", (ownerEnvVar) => {
241220
expect(() =>
242-
validateHttpsPinRuntimeAdapterPort(ENV_KEY, 19001, {
221+
validateRuntimeAdapterPort(ownerEnvVar, 19001, {
243222
...GATEWAY_VALIDATION_OPTIONS,
244223
vllmPort: 19001,
245224
}),
246225
).toThrow("NEMOCLAW_VLLM_PORT");
247226
});
227+
228+
it("validates against the live port configuration when no options are injected", () => {
229+
expect(() =>
230+
validateRuntimeAdapterPort("NEMOCLAW_BEDROCK_RUNTIME_ADAPTER_PORT", 11434),
231+
).toThrow("Ollama inference");
232+
});
248233
});

src/lib/core/ports.ts

Lines changed: 30 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -233,21 +233,6 @@ export function parseGatewayPort(
233233
return port;
234234
}
235235

236-
export function validateOpenRouterRuntimeAdapterPort(
237-
envVar: string,
238-
port: number,
239-
options: RuntimeAdapterPortValidationOptions,
240-
): void {
241-
validateServicePort(envVar, port, options, "NEMOCLAW_OPENROUTER_RUNTIME_ADAPTER_PORT");
242-
}
243-
244-
export function validateHttpsPinRuntimeAdapterPort(
245-
envVar: string,
246-
port: number,
247-
options: RuntimeAdapterPortValidationOptions,
248-
): void {
249-
validateServicePort(envVar, port, options, "NEMOCLAW_HTTPS_PIN_RUNTIME_ADAPTER_PORT");
250-
}
251236
/** OpenShell gateway port (default 8080, override via NEMOCLAW_GATEWAY_PORT). */
252237
export const GATEWAY_PORT = parseGatewayPort("NEMOCLAW_GATEWAY_PORT", DEFAULT_GATEWAY_PORT, {
253238
dashboardPort: DASHBOARD_PORT,
@@ -261,6 +246,35 @@ export const GATEWAY_PORT = parseGatewayPort("NEMOCLAW_GATEWAY_PORT", DEFAULT_GA
261246
httpsPinRuntimeAdapterPort: HTTPS_PIN_RUNTIME_ADAPTER_PORT,
262247
});
263248

249+
/** The live host-port configuration every runtime adapter is validated against. */
250+
const CURRENT_RUNTIME_PORT_CONFIGURATION: RuntimeAdapterPortValidationOptions = {
251+
gatewayPort: GATEWAY_PORT,
252+
dashboardPort: DASHBOARD_PORT,
253+
dashboardRangeStart: DASHBOARD_PORT_RANGE_START,
254+
dashboardRangeEnd: DASHBOARD_PORT_RANGE_END,
255+
vllmPort: VLLM_PORT,
256+
ollamaPort: OLLAMA_PORT,
257+
ollamaProxyPort: OLLAMA_PROXY_PORT,
258+
bedrockRuntimeAdapterPort: BEDROCK_RUNTIME_ADAPTER_PORT,
259+
openrouterRuntimeAdapterPort: OPENROUTER_RUNTIME_ADAPTER_PORT,
260+
httpsPinRuntimeAdapterPort: HTTPS_PIN_RUNTIME_ADAPTER_PORT,
261+
};
262+
263+
/**
264+
* Reject a runtime adapter port that overlaps the dashboard allocation range, a
265+
* reserved service default, or another configured service port. `ownerEnvVar`
266+
* names the adapter being validated: it is both the variable reported in the
267+
* error and the catalog entry excluded from the self-conflict check. Tests
268+
* inject `options`; production callers use the live configuration above.
269+
*/
270+
export function validateRuntimeAdapterPort(
271+
ownerEnvVar: string,
272+
port: number,
273+
options: RuntimeAdapterPortValidationOptions = CURRENT_RUNTIME_PORT_CONFIGURATION,
274+
): void {
275+
validateServicePort(ownerEnvVar, port, options, ownerEnvVar);
276+
}
277+
264278
/** Reject every configurable service collision with fixed llama.cpp attachment port 8081. */
265279
export function validateLlamaCppPortReservation(
266280
options: RuntimeAdapterPortValidationOptions,
@@ -275,15 +289,4 @@ export function validateLlamaCppPortReservation(
275289
}
276290
}
277291

278-
validateLlamaCppPortReservation({
279-
gatewayPort: GATEWAY_PORT,
280-
dashboardPort: DASHBOARD_PORT,
281-
dashboardRangeStart: DASHBOARD_PORT_RANGE_START,
282-
dashboardRangeEnd: DASHBOARD_PORT_RANGE_END,
283-
vllmPort: VLLM_PORT,
284-
ollamaPort: OLLAMA_PORT,
285-
ollamaProxyPort: OLLAMA_PROXY_PORT,
286-
bedrockRuntimeAdapterPort: BEDROCK_RUNTIME_ADAPTER_PORT,
287-
openrouterRuntimeAdapterPort: OPENROUTER_RUNTIME_ADAPTER_PORT,
288-
httpsPinRuntimeAdapterPort: HTTPS_PIN_RUNTIME_ADAPTER_PORT,
289-
});
292+
validateLlamaCppPortReservation(CURRENT_RUNTIME_PORT_CONFIGURATION);

src/lib/inference/bedrock-runtime-adapter.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@ import path from "node:path";
77

88
import { BedrockRuntimeClient } from "@aws-sdk/client-bedrock-runtime";
99

10-
import { BEDROCK_RUNTIME_ADAPTER_PORT } from "../core/ports";
10+
import { BEDROCK_RUNTIME_ADAPTER_PORT, validateRuntimeAdapterPort } from "../core/ports";
1111
import { compactText, isLoopbackRemoteAddress } from "../core/url-utils";
1212
import { run, runCapture, SCRIPTS } from "../runner";
1313
import { buildSubprocessEnv } from "../subprocess-env";
@@ -379,6 +379,7 @@ export async function ensureBedrockRuntimeAdapter(options: {
379379
token: string;
380380
region: string;
381381
}> {
382+
validateRuntimeAdapterPort("NEMOCLAW_BEDROCK_RUNTIME_ADAPTER_PORT", BEDROCK_RUNTIME_ADAPTER_PORT);
382383
const region = resolveBedrockRuntimeRegion(options.classification);
383384
const endpointUrl = options.classification.endpointUrl;
384385
const compatibleCredential = options.compatibleCredential || null;

src/lib/inference/https-pin-runtime-adapter.ts

Lines changed: 5 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -42,19 +42,7 @@ import http from "node:http";
4242
import { BlockList, isIP } from "node:net";
4343
import path from "node:path";
4444

45-
import {
46-
BEDROCK_RUNTIME_ADAPTER_PORT,
47-
DASHBOARD_PORT,
48-
DASHBOARD_PORT_RANGE_END,
49-
DASHBOARD_PORT_RANGE_START,
50-
GATEWAY_PORT,
51-
HTTPS_PIN_RUNTIME_ADAPTER_PORT,
52-
OLLAMA_PORT,
53-
OLLAMA_PROXY_PORT,
54-
OPENROUTER_RUNTIME_ADAPTER_PORT,
55-
VLLM_PORT,
56-
validateHttpsPinRuntimeAdapterPort,
57-
} from "../core/ports";
45+
import { HTTPS_PIN_RUNTIME_ADAPTER_PORT, validateRuntimeAdapterPort } from "../core/ports";
5846
import { retryUntilAsync } from "../core/retry";
5947
import { getVersion } from "../core/version";
6048
import { ROOT, run, runCapture } from "../runner";
@@ -1467,25 +1455,6 @@ async function withAdapterLock<T>(operation: () => Promise<T>): Promise<T> {
14671455
throw new Error("HTTPS Pin Runtime adapter startup is already in progress");
14681456
}
14691457

1470-
function validateAdapterPortConfiguration(): void {
1471-
validateHttpsPinRuntimeAdapterPort(
1472-
"NEMOCLAW_HTTPS_PIN_RUNTIME_ADAPTER_PORT",
1473-
HTTPS_PIN_RUNTIME_ADAPTER_PORT,
1474-
{
1475-
dashboardPort: DASHBOARD_PORT,
1476-
dashboardRangeStart: DASHBOARD_PORT_RANGE_START,
1477-
dashboardRangeEnd: DASHBOARD_PORT_RANGE_END,
1478-
gatewayPort: GATEWAY_PORT,
1479-
vllmPort: VLLM_PORT,
1480-
ollamaPort: OLLAMA_PORT,
1481-
ollamaProxyPort: OLLAMA_PROXY_PORT,
1482-
bedrockRuntimeAdapterPort: BEDROCK_RUNTIME_ADAPTER_PORT,
1483-
openrouterRuntimeAdapterPort: OPENROUTER_RUNTIME_ADAPTER_PORT,
1484-
httpsPinRuntimeAdapterPort: HTTPS_PIN_RUNTIME_ADAPTER_PORT,
1485-
},
1486-
);
1487-
}
1488-
14891458
async function findReusableAdapterControlToken(
14901459
priorToken: string | null,
14911460
allowedSourceCidrs: readonly string[] = ["127.0.0.1/32"],
@@ -1523,7 +1492,10 @@ async function ensureAdapterProcessLocked(options: {
15231492
routeId: string;
15241493
allowedSourceCidrs: string[];
15251494
}): Promise<string> {
1526-
validateAdapterPortConfiguration();
1495+
validateRuntimeAdapterPort(
1496+
"NEMOCLAW_HTTPS_PIN_RUNTIME_ADAPTER_PORT",
1497+
HTTPS_PIN_RUNTIME_ADAPTER_PORT,
1498+
);
15271499
const priorToken = readLocalAdapterTextFile(TOKEN_PATH);
15281500
// The authenticated, build-bound health response is stronger identity
15291501
// evidence than a PID file. Reuse the live adapter even if its PID metadata

src/lib/inference/openrouter-runtime-adapter-lifecycle.ts

Lines changed: 5 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -5,19 +5,7 @@ import fs from "node:fs";
55
import http from "node:http";
66
import path from "node:path";
77

8-
import {
9-
BEDROCK_RUNTIME_ADAPTER_PORT,
10-
DASHBOARD_PORT,
11-
DASHBOARD_PORT_RANGE_END,
12-
DASHBOARD_PORT_RANGE_START,
13-
GATEWAY_PORT,
14-
HTTPS_PIN_RUNTIME_ADAPTER_PORT,
15-
OLLAMA_PORT,
16-
OLLAMA_PROXY_PORT,
17-
OPENROUTER_RUNTIME_ADAPTER_PORT,
18-
VLLM_PORT,
19-
validateOpenRouterRuntimeAdapterPort,
20-
} from "../core/ports";
8+
import { OPENROUTER_RUNTIME_ADAPTER_PORT, validateRuntimeAdapterPort } from "../core/ports";
219
import { run, runCapture } from "../runner";
2210
import { buildSubprocessEnv } from "../subprocess-env";
2311
import {
@@ -209,25 +197,6 @@ function adapterRoute(): AdapterRoute {
209197
};
210198
}
211199

212-
function validateAdapterPortConfiguration(): void {
213-
validateOpenRouterRuntimeAdapterPort(
214-
"NEMOCLAW_OPENROUTER_RUNTIME_ADAPTER_PORT",
215-
OPENROUTER_RUNTIME_ADAPTER_PORT,
216-
{
217-
dashboardPort: DASHBOARD_PORT,
218-
dashboardRangeStart: DASHBOARD_PORT_RANGE_START,
219-
dashboardRangeEnd: DASHBOARD_PORT_RANGE_END,
220-
gatewayPort: GATEWAY_PORT,
221-
vllmPort: VLLM_PORT,
222-
ollamaPort: OLLAMA_PORT,
223-
ollamaProxyPort: OLLAMA_PROXY_PORT,
224-
bedrockRuntimeAdapterPort: BEDROCK_RUNTIME_ADAPTER_PORT,
225-
openrouterRuntimeAdapterPort: OPENROUTER_RUNTIME_ADAPTER_PORT,
226-
httpsPinRuntimeAdapterPort: HTTPS_PIN_RUNTIME_ADAPTER_PORT,
227-
},
228-
);
229-
}
230-
231200
function resolveAuthorizationHash(
232201
authorizationToken: string | null | undefined,
233202
priorState: JsonObject | null,
@@ -247,7 +216,10 @@ function resolveAuthorizationHash(
247216
async function ensureOpenRouterRuntimeAdapterLocked(
248217
options: EnsureOpenRouterRuntimeAdapterOptions = {},
249218
): Promise<AdapterRoute> {
250-
validateAdapterPortConfiguration();
219+
validateRuntimeAdapterPort(
220+
"NEMOCLAW_OPENROUTER_RUNTIME_ADAPTER_PORT",
221+
OPENROUTER_RUNTIME_ADAPTER_PORT,
222+
);
251223
const upstreamBaseUrl = OPENROUTER_ENDPOINT_URL;
252224
const configHash = adapterConfigHash(upstreamBaseUrl);
253225
const priorState = readLocalAdapterJsonFile(STATE_PATH);

0 commit comments

Comments
 (0)