Skip to content

Commit 8262422

Browse files
rsliterprekshivyas
andauthored
refactor(inference): centralize vLLM SSH policy (#9520)
## Summary Centralizes the strict base SSH arguments in one serving-owned vLLM policy module. Station preparation, persisted Station bindings, and managed discovery now append their endpoint-specific arguments to the same unchanged policy. ## Related Issue Closes #9519 ## Changes - Add `src/lib/inference/serving/vllm-ssh-transport-policy.ts` as the single owner of the current strict SSH transport arguments. - Keep known-hosts, host-key alias, host, user, port, target, and remote command arguments with their current consumers. - Replace the two-implementation parity test with one exact policy contract and consumer-composition coverage. - Leave the three different Python host probes unchanged and out of scope. ## 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: - [ ] Docs updated for user-facing behavior changes - [x] Docs not applicable — justification: Internal ownership refactor with no CLI, configuration, default, workflow, or supported behavior change. - [x] Sensitive paths changed (security, policy, credentials, preflight, onboarding, inference, runner, sandbox, or messaging) - [x] Sensitive-path review completed or maintainer-approved waiver recorded — reviewer/approval link/justification: Issue #9519 is maintainer-authorized. The option values and order are unchanged, and exact policy plus consumer tests cover the affected SSH boundaries. - [ ] Non-success, skipped, or missing CI check accepted by maintainer — check name, approval link, and follow-up issue: ## Documentation Writer Review - [x] Documentation writer subagent reviewed the completed changes - Result: `no-docs-needed` - Evidence: PR #9520 centralizes the unchanged internal vLLM SSH transport policy. Commit `666ec8afa` adds only Oxfmt line wrapping and changes no CLI, configuration, default, Python probe, workflow, or supported behavior. - Agent: Codex Desktop <!-- docs-review-head-sha: 666ec8a --> <!-- docs-review-agents-blob-sha: 993bdd8 --> ## DGX Station Hardware Evidence - [ ] Tested on DGX Station - Tested commit: - Station profile/scenario: - Result: Not applicable. `scripts/prepare-dgx-station-host.sh` is unchanged. - Supporting evidence: ## 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 — `npx vitest run --project cli src/lib/inference/serving/vllm-ssh-transport-policy.test.ts src/lib/inference/vllm-station-ssh-binding.test.ts src/lib/inference/serving/managed-cluster-discovery.test.ts` (75 passed); `npx vitest run --project installer-integration test/install-station-pair-preparation.test.ts` (80 passed); `npm run build:cli` passed; `npm run checks:repository` passed. - [ ] Applicable broad gate passed — Not applicable for this focused internal ownership refactor. - [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) --- Signed-off-by: Rebecca Sliter <571084+rsliter@users.noreply.github.com> <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit - **Security** - Strengthened SSH connections with strict host-key verification, public-key authentication, timeouts, and disabled forwarding and proxy features. - Standardized secure SSH behavior across managed cluster and Station preparation workflows. - Improved protection against untrusted hosts by enforcing pinned known-host configuration. - **Reliability** - Station preparation and connectivity checks now use consistent, noninteractive SSH settings. - **Tests** - Added coverage for secure SSH options, endpoint pinning, and preparation workflows. - Updated onboarding test fixtures to reflect the current agent configuration format. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Signed-off-by: Rebecca Sliter <571084+rsliter@users.noreply.github.com> Signed-off-by: Prekshi Vyas <prekshiv@nvidia.com> Co-authored-by: Rebecca Sliter <571084+rsliter@users.noreply.github.com> Co-authored-by: Prekshi Vyas <prekshiv@nvidia.com>
1 parent 8221777 commit 8262422

8 files changed

Lines changed: 171 additions & 153 deletions

scripts/prepare-dual-dgx-station.mts

Lines changed: 15 additions & 68 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@ import {
1515
stationKnownHostsDigest,
1616
writeDualStationSshBinding,
1717
} from "../src/lib/inference/vllm-station-ssh-binding.ts";
18+
import { strictVllmSshTransportArgs } from "../src/lib/inference/serving/vllm-ssh-transport-policy.ts";
1819
import {
1920
type DualStationPreparationDeps,
2021
type DualStationResumeState,
@@ -378,64 +379,6 @@ function runStreamingCommand(
378379
return result.status ?? 1;
379380
}
380381

381-
export function strictStationPrepSshTransportArgs(): string[] {
382-
return [
383-
"-T",
384-
"-o",
385-
"BatchMode=yes",
386-
"-o",
387-
"StrictHostKeyChecking=yes",
388-
"-o",
389-
"VerifyHostKeyDNS=no",
390-
"-o",
391-
"NoHostAuthenticationForLocalhost=no",
392-
"-o",
393-
"NumberOfPasswordPrompts=0",
394-
"-o",
395-
"PasswordAuthentication=no",
396-
"-o",
397-
"KbdInteractiveAuthentication=no",
398-
"-o",
399-
"PreferredAuthentications=publickey",
400-
"-o",
401-
"ConnectTimeout=5",
402-
"-o",
403-
"ConnectionAttempts=1",
404-
"-o",
405-
"ServerAliveInterval=5",
406-
"-o",
407-
"ServerAliveCountMax=1",
408-
"-o",
409-
"ClearAllForwardings=yes",
410-
"-o",
411-
"ForwardAgent=no",
412-
"-o",
413-
"ForwardX11=no",
414-
"-o",
415-
"ForwardX11Trusted=no",
416-
"-o",
417-
"Tunnel=no",
418-
"-o",
419-
"UpdateHostKeys=no",
420-
"-o",
421-
"ControlMaster=no",
422-
"-o",
423-
"ControlPath=none",
424-
"-o",
425-
"PermitLocalCommand=no",
426-
"-o",
427-
"RemoteCommand=none",
428-
"-o",
429-
"ProxyCommand=none",
430-
"-o",
431-
"ProxyJump=none",
432-
"-o",
433-
"KnownHostsCommand=none",
434-
"-o",
435-
"LogLevel=ERROR",
436-
];
437-
}
438-
439382
function parseSshConfig(stdout: string): SshConfig {
440383
const values = new Map<string, string[]>();
441384
for (const rawLine of stdout.split(/\r?\n/)) {
@@ -567,11 +510,7 @@ function knownHostEvidence(
567510

568511
export function inspectPretrustedSshTarget(target: string): PretrustedSshTarget | null {
569512
validateStationPeerTarget(target);
570-
const configResult = runCommand(
571-
"ssh",
572-
["-G", ...strictStationPrepSshTransportArgs(), "--", target],
573-
"",
574-
);
513+
const configResult = runCommand("ssh", ["-G", ...strictVllmSshTransportArgs(), "--", target], "");
575514
if (!commandSucceeded(configResult, true)) return null;
576515
const config = parseSshConfig(configResult.stdout);
577516
assertStrictSshConfig(config);
@@ -936,13 +875,13 @@ function assertHelperFile(helperPath: string): Buffer {
936875
}
937876
}
938877

939-
function sshArgs(
878+
export function stationPrepSshArgs(
940879
binding: PretrustedSshTarget,
941880
pinnedKnownHostsPath: string,
942881
remoteCommand: string,
943882
): string[] {
944883
return [
945-
...strictStationPrepSshTransportArgs(),
884+
...strictVllmSshTransportArgs(),
946885
"-o",
947886
`UserKnownHostsFile=${pinnedKnownHostsPath}`,
948887
"-o",
@@ -997,7 +936,7 @@ function createRuntimeDeps(options: CliOptions): {
997936
parseHostResult(
998937
runCommand(
999938
"ssh",
1000-
sshArgs(binding, pinnedKnownHosts(binding), "python3 -"),
939+
stationPrepSshArgs(binding, pinnedKnownHosts(binding), "python3 -"),
1001940
STATION_DISCOVERY_PROBE,
1002941
),
1003942
"Peer Station identity probe",
@@ -1014,7 +953,11 @@ function createRuntimeDeps(options: CliOptions): {
1014953
return connectivityMatches(
1015954
runCommand(
1016955
"ssh",
1017-
sshArgs(binding, pinnedKnownHosts(binding), ["python3", "-", ...args].join(" ")),
956+
stationPrepSshArgs(
957+
binding,
958+
pinnedKnownHosts(binding),
959+
["python3", "-", ...args].join(" "),
960+
),
1018961
CONNECTIVITY_PROBE,
1019962
),
1020963
requests,
@@ -1023,7 +966,11 @@ function createRuntimeDeps(options: CliOptions): {
1023966
runRemoteHelper: (binding, mode) => {
1024967
return runStreamingCommand(
1025968
"ssh",
1026-
sshArgs(binding, pinnedKnownHosts(binding), buildRemoteHelperCommand(helperSha256, mode)),
969+
stationPrepSshArgs(
970+
binding,
971+
pinnedKnownHosts(binding),
972+
buildRemoteHelperCommand(helperSha256, mode),
973+
),
1027974
helperBytes.toString("utf8"),
1028975
);
1029976
},

src/lib/inference/serving/managed-cluster-ssh-binding.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ export type {
66
QualifiedStationSshIdentity as QualifiedManagedVllmSshIdentity,
77
WriteDualStationSshBindingOptions as WriteManagedVllmSshBindingOptions,
88
} from "../vllm-station-ssh-binding.js";
9+
export { strictVllmSshTransportArgs as strictManagedVllmSshTransportArgs } from "./vllm-ssh-transport-policy.js";
910
/**
1011
* Cardinality-neutral names for the pinned SSH transport shared by managed
1112
* vLLM clusters. The legacy Station implementation remains the compatibility
@@ -23,6 +24,5 @@ export {
2324
loadDualStationSshBindingForStatePath as loadManagedVllmSshBindingForStatePath,
2425
loadDualStationSshBindingHandoff as loadManagedVllmSshBindingHandoff,
2526
stationKnownHostsDigest as managedVllmKnownHostsDigest,
26-
strictStationSshTransportArgs as strictManagedVllmSshTransportArgs,
2727
writeDualStationSshBinding as writeManagedVllmSshBinding,
2828
} from "../vllm-station-ssh-binding.js";
Lines changed: 66 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,66 @@
1+
// SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved.
2+
// SPDX-License-Identifier: Apache-2.0
3+
4+
import { describe, expect, it } from "vitest";
5+
6+
import { strictVllmSshTransportArgs } from "./vllm-ssh-transport-policy";
7+
8+
describe("strict vLLM SSH transport policy", () => {
9+
it("returns the exact noninteractive transport boundary (#9519)", () => {
10+
expect(strictVllmSshTransportArgs()).toEqual([
11+
"-T",
12+
"-o",
13+
"BatchMode=yes",
14+
"-o",
15+
"StrictHostKeyChecking=yes",
16+
"-o",
17+
"VerifyHostKeyDNS=no",
18+
"-o",
19+
"NoHostAuthenticationForLocalhost=no",
20+
"-o",
21+
"NumberOfPasswordPrompts=0",
22+
"-o",
23+
"PasswordAuthentication=no",
24+
"-o",
25+
"KbdInteractiveAuthentication=no",
26+
"-o",
27+
"PreferredAuthentications=publickey",
28+
"-o",
29+
"ConnectTimeout=5",
30+
"-o",
31+
"ConnectionAttempts=1",
32+
"-o",
33+
"ServerAliveInterval=5",
34+
"-o",
35+
"ServerAliveCountMax=1",
36+
"-o",
37+
"ClearAllForwardings=yes",
38+
"-o",
39+
"ForwardAgent=no",
40+
"-o",
41+
"ForwardX11=no",
42+
"-o",
43+
"ForwardX11Trusted=no",
44+
"-o",
45+
"Tunnel=no",
46+
"-o",
47+
"UpdateHostKeys=no",
48+
"-o",
49+
"ControlMaster=no",
50+
"-o",
51+
"ControlPath=none",
52+
"-o",
53+
"PermitLocalCommand=no",
54+
"-o",
55+
"RemoteCommand=none",
56+
"-o",
57+
"ProxyCommand=none",
58+
"-o",
59+
"ProxyJump=none",
60+
"-o",
61+
"KnownHostsCommand=none",
62+
"-o",
63+
"LogLevel=ERROR",
64+
]);
65+
});
66+
});
Lines changed: 60 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,60 @@
1+
// SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved.
2+
// SPDX-License-Identifier: Apache-2.0
3+
4+
export function strictVllmSshTransportArgs(): string[] {
5+
return [
6+
"-T",
7+
"-o",
8+
"BatchMode=yes",
9+
"-o",
10+
"StrictHostKeyChecking=yes",
11+
"-o",
12+
"VerifyHostKeyDNS=no",
13+
"-o",
14+
"NoHostAuthenticationForLocalhost=no",
15+
"-o",
16+
"NumberOfPasswordPrompts=0",
17+
"-o",
18+
"PasswordAuthentication=no",
19+
"-o",
20+
"KbdInteractiveAuthentication=no",
21+
"-o",
22+
"PreferredAuthentications=publickey",
23+
"-o",
24+
"ConnectTimeout=5",
25+
"-o",
26+
"ConnectionAttempts=1",
27+
"-o",
28+
"ServerAliveInterval=5",
29+
"-o",
30+
"ServerAliveCountMax=1",
31+
"-o",
32+
"ClearAllForwardings=yes",
33+
"-o",
34+
"ForwardAgent=no",
35+
"-o",
36+
"ForwardX11=no",
37+
"-o",
38+
"ForwardX11Trusted=no",
39+
"-o",
40+
"Tunnel=no",
41+
"-o",
42+
"UpdateHostKeys=no",
43+
"-o",
44+
"ControlMaster=no",
45+
"-o",
46+
"ControlPath=none",
47+
"-o",
48+
"PermitLocalCommand=no",
49+
"-o",
50+
"RemoteCommand=none",
51+
"-o",
52+
"ProxyCommand=none",
53+
"-o",
54+
"ProxyJump=none",
55+
"-o",
56+
"KnownHostsCommand=none",
57+
"-o",
58+
"LogLevel=ERROR",
59+
];
60+
}

src/lib/inference/vllm-station-ssh-binding.test.ts

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -21,9 +21,9 @@ import {
2121
loadDualStationSshBindingHandoff,
2222
type QualifiedStationSshIdentity,
2323
stationKnownHostsDigest,
24-
strictStationSshTransportArgs,
2524
writeDualStationSshBinding,
2625
} from "./vllm-station-ssh-binding";
26+
import { strictVllmSshTransportArgs } from "./serving/vllm-ssh-transport-policy";
2727

2828
const PEER_TARGET = "station@10.10.0.2";
2929
const PEER_HOST = "10.10.0.2";
@@ -162,8 +162,8 @@ exit "${"${NEMOCLAW_TEST_DOCKER_EXIT:-0}"}"
162162
const binding = writeBinding();
163163
const args = dualStationPinnedSshArgs(binding);
164164

165-
expect(args.slice(0, strictStationSshTransportArgs().length)).toEqual(
166-
strictStationSshTransportArgs(),
165+
expect(args.slice(0, strictVllmSshTransportArgs().length)).toEqual(
166+
strictVllmSshTransportArgs(),
167167
);
168168
expect(args).toEqual(
169169
expect.arrayContaining([

src/lib/inference/vllm-station-ssh-binding.ts

Lines changed: 3 additions & 59 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,8 @@ import fs from "node:fs";
66
import net from "node:net";
77
import path from "node:path";
88

9+
import { strictVllmSshTransportArgs } from "./serving/vllm-ssh-transport-policy.ts";
10+
911
export const NEMOCLAW_DGX_STATION_SSH_BINDING_ENV = "NEMOCLAW_DGX_STATION_SSH_BINDING";
1012

1113
const BINDING_SCHEMA_VERSION = 2;
@@ -287,72 +289,14 @@ function shellQuote(value: string): string {
287289
return `'${value.replaceAll("'", `'"'"'`)}'`;
288290
}
289291

290-
export function strictStationSshTransportArgs(): string[] {
291-
return [
292-
"-T",
293-
"-o",
294-
"BatchMode=yes",
295-
"-o",
296-
"StrictHostKeyChecking=yes",
297-
"-o",
298-
"VerifyHostKeyDNS=no",
299-
"-o",
300-
"NoHostAuthenticationForLocalhost=no",
301-
"-o",
302-
"NumberOfPasswordPrompts=0",
303-
"-o",
304-
"PasswordAuthentication=no",
305-
"-o",
306-
"KbdInteractiveAuthentication=no",
307-
"-o",
308-
"PreferredAuthentications=publickey",
309-
"-o",
310-
"ConnectTimeout=5",
311-
"-o",
312-
"ConnectionAttempts=1",
313-
"-o",
314-
"ServerAliveInterval=5",
315-
"-o",
316-
"ServerAliveCountMax=1",
317-
"-o",
318-
"ClearAllForwardings=yes",
319-
"-o",
320-
"ForwardAgent=no",
321-
"-o",
322-
"ForwardX11=no",
323-
"-o",
324-
"ForwardX11Trusted=no",
325-
"-o",
326-
"Tunnel=no",
327-
"-o",
328-
"UpdateHostKeys=no",
329-
"-o",
330-
"ControlMaster=no",
331-
"-o",
332-
"ControlPath=none",
333-
"-o",
334-
"PermitLocalCommand=no",
335-
"-o",
336-
"RemoteCommand=none",
337-
"-o",
338-
"ProxyCommand=none",
339-
"-o",
340-
"ProxyJump=none",
341-
"-o",
342-
"KnownHostsCommand=none",
343-
"-o",
344-
"LogLevel=ERROR",
345-
];
346-
}
347-
348292
type PinnedStationEndpoint = Pick<
349293
DualStationSshBinding,
350294
"knownHostsFile" | "lookupHost" | "port" | "resolvedHost" | "sshUser"
351295
>;
352296

353297
function pinnedOptionArgs(binding: PinnedStationEndpoint): string[] {
354298
return [
355-
...strictStationSshTransportArgs(),
299+
...strictVllmSshTransportArgs(),
356300
"-o",
357301
`UserKnownHostsFile=${binding.knownHostsFile}`,
358302
"-o",

src/lib/onboard/dockerfile-patch.test.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -707,7 +707,7 @@ describe("dockerfile patch helpers", () => {
707707
{ channelId: "telegram", active: true },
708708
],
709709
agentRender: [
710-
{ agent: "openclaw", channelId: "discord", target: "config", path: ["discord"] },
710+
{ agent: "openclaw", channelId: "discord", target: "openclaw.json", path: ["discord"] },
711711
],
712712
});
713713
const tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), "nemoclaw-onboard-dockerfile-plan-"));

0 commit comments

Comments
 (0)