Skip to content

fix(onboard): run inference curl probes without shell expansion #890

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
OffbeatOps wants to merge 2 commits into
base: main
Choose a base branch
from
Open

fix(onboard): run inference curl probes without shell expansion #890

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
c5658ad
fix(onboard): run inference curl probes without shell expansion
OffbeatOps Mar 25, 2026
7221fa1
merge: incorporate main into fix/onboard-inference-curl-probes
cv Apr 4, 2026
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
11 changes: 9 additions & 2 deletions bin/lib/credentials.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,10 +25,17 @@ function saveCredential(key, value) {
fs.writeFileSync(CREDS_FILE, JSON.stringify(creds, null, 2), { mode: 0o600 });
}

function normalizeSecret(value) {
if (value == null) return null;
return String(value).replace(/\r/g, "").trim();
}

function getCredential(key) {
if (process.env[key]) return process.env[key];
if (process.env[key]) return normalizeSecret(process.env[key]);
const creds = loadCredentials();
return creds[key] || null;
const raw = creds[key];
if (raw == null) return null;
return normalizeSecret(raw);
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai bot Mar 25, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Propagate the cleaned secret into the environment.

getCredential() only returns the normalized value. In non-interactive onboarding, setupInference() still forwards process.env[resolvedCredentialEnv] unchanged to upsertProvider() at bin/lib/onboard.js Line 1957 through Line 1960, so a key with trailing CR/LF can pass the probe here and then still be written to OpenShell with the bad bytes. Please normalize before exporting, or have the provider-setup path consume getCredential() instead of the raw env var.

🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@bin/lib/credentials.js` around lines 28 - 38, getCredential currently returns
a normalized secret but does not propagate that cleaned value back into
process.env, which lets onboarding's setupInference -> upsertProvider still read
the original env var (resolvedCredentialEnv) with CR/LF; update getCredential to
normalize and then assign the cleaned value back into process.env[key] before
returning (use normalizeSecret and loadCredentials as needed), or alternatively
change the provider-setup path (setupInference/upsertProvider) to call
getCredential(resolvedCredentialEnv) instead of reading process.env directly so
the cleaned value is consumed when exporting credentials.

}

function promptSecret(question) {
Expand Down
207 changes: 119 additions & 88 deletions bin/lib/onboard.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -357,6 +357,17 @@ function getCurlTimingArgs() {
return ["--connect-timeout 5", "--max-time 20"];
}

/** Args for `spawnSync("curl", ...)` — avoids shell parsing of secrets; `--http1.1` dodges flaky HTTP/2 paths. */
function getCurlSpawnArgs() {
return ["--connect-timeout", "5", "--max-time", "20", "--http1.1"];
}

function probeDisplayCode(result) {
const httpCode = Number(String(result.stdout || "").trim());
if (result.status !== 0) return result.status;
return Number.isFinite(httpCode) ? httpCode : 0;
}
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai bot Mar 25, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🌐 Web query:

In Node.js child_process.spawnSync, what values do result.status, result.signal, and result.error have when the executable cannot be found or when the child process is terminated by a signal?

💡 Result:

When the executable cannot be found (ENOENT error): - result.status: null - result.signal: null - result.error: Error object with code 'ENOENT' (e.g., Error: spawnSync not_found ENOENT) When the child process is terminated by a signal: - result.status: null - result.signal: the signal name (e.g., 'SIGTERM', 'SIGKILL') - result.error: undefined

Citations:

🏁 Script executed:

cat -n bin/lib/onboard.js | sed -n '360,375p'

Repository: NVIDIA/NemoClaw

Length of output: 1870

🏁 Script executed:

cat -n bin/lib/onboard.js | sed -n '525,545p'

Repository: NVIDIA/NemoClaw

Length of output: 899

🏁 Script executed:

# Search for callers of probeDisplayCode to understand the impact
rg "probeDisplayCode" bin/lib/onboard.js -B 3 -A 3

Repository: NVIDIA/NemoClaw

Length of output: 2136

Handle spawnSync() launch failures before formatting them as HTTP errors.

If curl cannot be started or the child is terminated by a signal, spawnSync() sets result.status to null and reports the failure via result.error (with code 'ENOENT' for missing executable) or result.signal (for signal termination). The probeDisplayCode() helper currently returns null in these cases, so callers end up showing HTTP null with no response body instead of an actionable curl failed to start or curl terminated message.

Check result.error and result.signal in addition to result.status to provide meaningful error messages to users.

Also applies to: 532-536

🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@bin/lib/onboard.js` around lines 365 - 369, probeDisplayCode currently only
checks result.status and returns null for spawnSync launch failures; change it
to first check result.error and result.signal and raise/return a clear failure
instead of null: if result.error exists, throw (or return an error object) with
a message like "curl failed to start: <result.error.code|message>", and if
result.signal exists, throw/return "curl terminated by signal <result.signal>".
Apply the same change to the other identical probe block referenced (the logic
around the second probe at the other occurrence) so callers receive actionable
error information rather than HTTP null.


function buildProviderArgs(action, name, type, credentialEnv, baseUrl) {
const args =
action === "create"
Expand Down Expand Up @@ -518,7 +529,11 @@ function patchStagedDockerfile(dockerfilePath, model, chatUiUrl, buildId = Strin
fs.writeFileSync(dockerfilePath, dockerfile);
}

function summarizeProbeError(body, status) {
function summarizeProbeError(body, status, stderr = "") {
const errTail = stderr ? ` — ${stderr.replace(/\s+/g, " ").trim().slice(0, 280)}` : "";
if (Number.isFinite(status) && status > 0 && status < 100) {
return `curl failed (exit ${status})${errTail}`;
}
if (!body) return `HTTP ${status} with no response body`;
try {
const parsed = JSON.parse(body);
Expand Down Expand Up @@ -562,34 +577,37 @@ function probeOpenAiLikeEndpoint(endpointUrl, model, apiKey) {
for (const probe of probes) {
const bodyFile = path.join(os.tmpdir(), `nemoclaw-probe-${Date.now()}-${Math.random().toString(36).slice(2)}.json`);
try {
const cmd = [
"curl -sS",
...getCurlTimingArgs(),
`-o ${shellQuote(bodyFile)}`,
"-w '%{http_code}'",
"-H 'Content-Type: application/json'",
...(apiKey ? ['-H "Authorization: Bearer $NEMOCLAW_PROBE_API_KEY"'] : []),
`-d ${shellQuote(probe.body)}`,
shellQuote(probe.url),
].join(" ");
const result = spawnSync("bash", ["-c", cmd], {
const args = [
"-sS",
...getCurlSpawnArgs(),
"-o",
bodyFile,
"-w",
"%{http_code}",
"-H",
"Content-Type: application/json",
...(apiKey ? ["-H", `Authorization: Bearer ${apiKey}`] : []),
"-d",
probe.body,
probe.url,
];
const result = spawnSync("curl", args, {
cwd: ROOT,
encoding: "utf8",
env: {
...process.env,
NEMOCLAW_PROBE_API_KEY: apiKey,
},
env: { ...process.env },
});
const body = fs.existsSync(bodyFile) ? fs.readFileSync(bodyFile, "utf8") : "";
const status = Number(String(result.stdout || "").trim());
if (result.status === 0 && status >= 200 && status < 300) {
const httpStatus = Number(String(result.stdout || "").trim());
if (result.status === 0 && httpStatus >= 200 && httpStatus < 300) {
return { ok: true, api: probe.api, label: probe.name };
}
const display = probeDisplayCode(result);
const stderr = String(result.stderr || "");
failures.push({
name: probe.name,
httpStatus: Number.isFinite(status) ? status : 0,
httpStatus: Number.isFinite(httpStatus) ? httpStatus : 0,
curlStatus: result.status || 0,
message: summarizeProbeError(body, status || result.status || 0),
message: summarizeProbeError(body, display, stderr),
});
} finally {
fs.rmSync(bodyFile, { force: true });
Expand All @@ -606,41 +624,47 @@ function probeOpenAiLikeEndpoint(endpointUrl, model, apiKey) {
function probeAnthropicEndpoint(endpointUrl, model, apiKey) {
const bodyFile = path.join(os.tmpdir(), `nemoclaw-anthropic-probe-${Date.now()}-${Math.random().toString(36).slice(2)}.json`);
try {
const cmd = [
"curl -sS",
...getCurlTimingArgs(),
`-o ${shellQuote(bodyFile)}`,
"-w '%{http_code}'",
'-H "x-api-key: $NEMOCLAW_PROBE_API_KEY"',
"-H 'anthropic-version: 2023-06-01'",
"-H 'content-type: application/json'",
`-d ${shellQuote(JSON.stringify({
model,
max_tokens: 16,
messages: [{ role: "user", content: "Reply with exactly: OK" }],
}))}`,
shellQuote(`${String(endpointUrl).replace(/\/+$/, "")}/v1/messages`),
].join(" ");
const result = spawnSync("bash", ["-c", cmd], {
const payload = JSON.stringify({
model,
max_tokens: 16,
messages: [{ role: "user", content: "Reply with exactly: OK" }],
});
const args = [
"-sS",
...getCurlSpawnArgs(),
"-o",
bodyFile,
"-w",
"%{http_code}",
"-H",
`x-api-key: ${apiKey}`,
"-H",
"anthropic-version: 2023-06-01",
"-H",
"content-type: application/json",
"-d",
payload,
`${String(endpointUrl).replace(/\/+$/, "")}/v1/messages`,
];
const result = spawnSync("curl", args, {
cwd: ROOT,
encoding: "utf8",
env: {
...process.env,
NEMOCLAW_PROBE_API_KEY: apiKey,
},
env: { ...process.env },
});
const body = fs.existsSync(bodyFile) ? fs.readFileSync(bodyFile, "utf8") : "";
const status = Number(String(result.stdout || "").trim());
if (result.status === 0 && status >= 200 && status < 300) {
const httpStatus = Number(String(result.stdout || "").trim());
if (result.status === 0 && httpStatus >= 200 && httpStatus < 300) {
return { ok: true, api: "anthropic-messages", label: "Anthropic Messages API" };
}
const display = probeDisplayCode(result);
const stderr = String(result.stderr || "");
return {
ok: false,
message: summarizeProbeError(body, status || result.status || 0),
message: summarizeProbeError(body, display, stderr),
failures: [
{
name: "Anthropic Messages API",
httpStatus: Number.isFinite(status) ? status : 0,
httpStatus: Number.isFinite(httpStatus) ? httpStatus : 0,
curlStatus: result.status || 0,
},
],
Expand Down Expand Up @@ -752,27 +776,30 @@ async function validateCustomAnthropicSelection(label, endpointUrl, model, crede
function fetchNvidiaEndpointModels(apiKey) {
const bodyFile = path.join(os.tmpdir(), `nemoclaw-nvidia-models-${Date.now()}-${Math.random().toString(36).slice(2)}.json`);
try {
const cmd = [
"curl -sS",
...getCurlTimingArgs(),
`-o ${shellQuote(bodyFile)}`,
"-w '%{http_code}'",
"-H 'Content-Type: application/json'",
'-H "Authorization: Bearer $NEMOCLAW_PROBE_API_KEY"',
shellQuote(`${BUILD_ENDPOINT_URL}/models`),
].join(" ");
const result = spawnSync("bash", ["-c", cmd], {
const args = [
"-sS",
...getCurlSpawnArgs(),
"-o",
bodyFile,
"-w",
"%{http_code}",
"-H",
"Content-Type: application/json",
"-H",
`Authorization: Bearer ${apiKey}`,
`${BUILD_ENDPOINT_URL}/models`,
];
const result = spawnSync("curl", args, {
cwd: ROOT,
encoding: "utf8",
env: {
...process.env,
NEMOCLAW_PROBE_API_KEY: apiKey,
},
env: { ...process.env },
});
const body = fs.existsSync(bodyFile) ? fs.readFileSync(bodyFile, "utf8") : "";
const status = Number(String(result.stdout || "").trim());
if (result.status !== 0 || !(status >= 200 && status < 300)) {
return { ok: false, message: summarizeProbeError(body, status || result.status || 0) };
const display = probeDisplayCode(result);
const stderr = String(result.stderr || "");
return { ok: false, message: summarizeProbeError(body, display, stderr) };
}
const parsed = JSON.parse(body);
const ids = Array.isArray(parsed?.data)
Expand Down Expand Up @@ -806,26 +833,27 @@ function validateNvidiaEndpointModel(model, apiKey) {
function fetchOpenAiLikeModels(endpointUrl, apiKey) {
const bodyFile = path.join(os.tmpdir(), `nemoclaw-openai-models-${Date.now()}-${Math.random().toString(36).slice(2)}.json`);
try {
const cmd = [
"curl -sS",
...getCurlTimingArgs(),
`-o ${shellQuote(bodyFile)}`,
"-w '%{http_code}'",
...(apiKey ? ['-H "Authorization: Bearer $NEMOCLAW_PROBE_API_KEY"'] : []),
shellQuote(`${String(endpointUrl).replace(/\/+$/, "")}/models`),
].join(" ");
const result = spawnSync("bash", ["-c", cmd], {
const args = [
"-sS",
...getCurlSpawnArgs(),
"-o",
bodyFile,
"-w",
"%{http_code}",
...(apiKey ? ["-H", `Authorization: Bearer ${apiKey}`] : []),
`${String(endpointUrl).replace(/\/+$/, "")}/models`,
];
const result = spawnSync("curl", args, {
cwd: ROOT,
encoding: "utf8",
env: {
...process.env,
NEMOCLAW_PROBE_API_KEY: apiKey,
},
env: { ...process.env },
});
const body = fs.existsSync(bodyFile) ? fs.readFileSync(bodyFile, "utf8") : "";
const status = Number(String(result.stdout || "").trim());
if (result.status !== 0 || !(status >= 200 && status < 300)) {
return { ok: false, status, message: summarizeProbeError(body, status || result.status || 0) };
const display = probeDisplayCode(result);
const stderr = String(result.stderr || "");
return { ok: false, status, message: summarizeProbeError(body, display, stderr) };
}
const parsed = JSON.parse(body);
const ids = Array.isArray(parsed?.data)
Expand All @@ -842,27 +870,30 @@ function fetchOpenAiLikeModels(endpointUrl, apiKey) {
function fetchAnthropicModels(endpointUrl, apiKey) {
const bodyFile = path.join(os.tmpdir(), `nemoclaw-anthropic-models-${Date.now()}-${Math.random().toString(36).slice(2)}.json`);
try {
const cmd = [
"curl -sS",
...getCurlTimingArgs(),
`-o ${shellQuote(bodyFile)}`,
"-w '%{http_code}'",
'-H "x-api-key: $NEMOCLAW_PROBE_API_KEY"',
"-H 'anthropic-version: 2023-06-01'",
shellQuote(`${String(endpointUrl).replace(/\/+$/, "")}/v1/models`),
].join(" ");
const result = spawnSync("bash", ["-c", cmd], {
const args = [
"-sS",
...getCurlSpawnArgs(),
"-o",
bodyFile,
"-w",
"%{http_code}",
"-H",
`x-api-key: ${apiKey}`,
"-H",
"anthropic-version: 2023-06-01",
`${String(endpointUrl).replace(/\/+$/, "")}/v1/models`,
];
const result = spawnSync("curl", args, {
cwd: ROOT,
encoding: "utf8",
env: {
...process.env,
NEMOCLAW_PROBE_API_KEY: apiKey,
},
env: { ...process.env },
});
const body = fs.existsSync(bodyFile) ? fs.readFileSync(bodyFile, "utf8") : "";
const status = Number(String(result.stdout || "").trim());
if (result.status !== 0 || !(status >= 200 && status < 300)) {
return { ok: false, status, message: summarizeProbeError(body, status || result.status || 0) };
const display = probeDisplayCode(result);
const stderr = String(result.stderr || "");
return { ok: false, status, message: summarizeProbeError(body, display, stderr) };
}
const parsed = JSON.parse(body);
const ids = Array.isArray(parsed?.data)
Expand Down