Skip to content

security: add shell-injection prevention and pass NGC_API_KEY to NIM container #219

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

Closed
brianwtaylor wants to merge 1 commit into from
Closed

security: add shell-injection prevention and pass NGC_API_KEY to NIM container #219

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
9 changes: 7 additions & 2 deletions bin/lib/nim.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
//
// NIM container management — pull, start, stop, health-check NIM images.

const { run, runCapture } = require("./runner");
const { run, runCapture, shellQuote } = require("./runner");
const nimImages = require("./nim-images.json");

function containerName(sandboxName) {
Expand Down Expand Up @@ -136,9 +136,14 @@ function startNimContainer(sandboxName, model, port = 8000) {
// Stop any existing container with same name
run(`docker rm -f ${name} 2>/dev/null || true`, { ignoreError: true });

// Pass NGC_API_KEY so the container can pull model weights from NGC.
// Falls back to NVIDIA_API_KEY for environments that only set that.
const ngcKey = process.env.NGC_API_KEY || process.env.NVIDIA_API_KEY || "";
const envFlags = ngcKey ? `-e NGC_API_KEY=${shellQuote(ngcKey)}` : "";

console.log(` Starting NIM container: ${name}`);
run(
`docker run -d --gpus all -p ${port}:8000 --name ${name} --shm-size 16g ${image}`
`docker run -d --gpus all -p ${port}:8000 --name ${name} --shm-size 16g ${envFlags} ${image}`
);
return name;
}
Expand Down
20 changes: 10 additions & 10 deletions bin/lib/onboard.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@

const fs = require("fs");
const path = require("path");
const { ROOT, SCRIPTS, run, runCapture } = require("./runner");
const { ROOT, SCRIPTS, run, runCapture, shellQuote } = require("./runner");
const {
getDefaultOllamaModel,
getLocalProviderBaseUrl,
Expand Down Expand Up @@ -85,10 +85,6 @@ function step(n, total, msg) {
console.log(` ${"─".repeat(50)}`);
}

function shellQuote(value) {
return `'${String(value).replace(/'/g, `'\\''`)}'`;
}

function pythonLiteralJson(value) {
return JSON.stringify(JSON.stringify(value));
}
Expand Down Expand Up @@ -464,9 +460,13 @@ async function createSandbox(gpu) {

console.log(` Creating sandbox '${sandboxName}' (this takes a few minutes on first run)...`);
const chatUiUrl = process.env.CHAT_UI_URL || 'http://127.0.0.1:18789';
const envArgs = [`CHAT_UI_URL=${chatUiUrl}`];
const envArgs = [`CHAT_UI_URL=${shellQuote(chatUiUrl)}`];
if (process.env.NVIDIA_API_KEY) {
envArgs.push(`NVIDIA_API_KEY=${process.env.NVIDIA_API_KEY}`);
envArgs.push(`NVIDIA_API_KEY=${shellQuote(process.env.NVIDIA_API_KEY)}`);
}
const ngcKey = process.env.NGC_API_KEY || process.env.NVIDIA_API_KEY || "";
if (ngcKey) {
envArgs.push(`NGC_API_KEY=${shellQuote(ngcKey)}`);
}

// Run without piping through awk — the pipe masked non-zero exit codes
Expand Down Expand Up @@ -733,7 +733,7 @@ async function setupInference(sandboxName, model, provider) {
{ ignoreError: true }
);
run(
`openshell inference set --no-verify --provider nvidia-nim --model ${model} 2>/dev/null || true`,
`openshell inference set --no-verify --provider nvidia-nim --model "${model}" 2>/dev/null || true`,
{ ignoreError: true }
);
} else if (provider === "vllm-local") {
Expand All @@ -752,7 +752,7 @@ async function setupInference(sandboxName, model, provider) {
{ ignoreError: true }
);
run(
`openshell inference set --no-verify --provider vllm-local --model ${model} 2>/dev/null || true`,
`openshell inference set --no-verify --provider vllm-local --model "${model}" 2>/dev/null || true`,
{ ignoreError: true }
);
} else if (provider === "ollama-local") {
Expand All @@ -772,7 +772,7 @@ async function setupInference(sandboxName, model, provider) {
{ ignoreError: true }
);
run(
`openshell inference set --no-verify --provider ollama-local --model ${model} 2>/dev/null || true`,
`openshell inference set --no-verify --provider ollama-local --model "${model}" 2>/dev/null || true`,
{ ignoreError: true }
);
console.log(` Priming Ollama model: ${model}`);
Expand Down
7 changes: 6 additions & 1 deletion bin/lib/runner.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -58,4 +58,9 @@ function runCapture(cmd, opts = {}) {
}
}

module.exports = { ROOT, SCRIPTS, run, runCapture, runInteractive };
/** Single-quote a value for safe interpolation into shell commands. */
function shellQuote(value) {
return `'${String(value).replace(/'/g, `'\\''`)}'`;
}

module.exports = { ROOT, SCRIPTS, run, runCapture, runInteractive, shellQuote };
86 changes: 86 additions & 0 deletions test/nim.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ const { describe, it } = require("node:test");
const assert = require("node:assert/strict");

const nim = require("../bin/lib/nim");
const { shellQuote } = require("../bin/lib/runner");

describe("nim", () => {
describe("listModels", () => {
Expand Down Expand Up @@ -74,4 +75,89 @@ describe("nim", () => {
assert.equal(st.running, false);
});
});

describe("NGC_API_KEY escaping", () => {
it("single-quotes a normal API key", () => {
const key = "nvapi-abc123DEF456";
const envFlags = key ? `-e NGC_API_KEY=${shellQuote(key)}` : "";
assert.equal(envFlags, "-e NGC_API_KEY='nvapi-abc123DEF456'");
});

it("escapes embedded single quotes in key", () => {
const key = "key'with'quotes";
const envFlags = `-e NGC_API_KEY=${shellQuote(key)}`;
assert.equal(envFlags, "-e NGC_API_KEY='key'\\''with'\\''quotes'");
});

it("produces empty string for empty key", () => {
const key = "";
const envFlags = key ? `-e NGC_API_KEY=${shellQuote(key)}` : "";
assert.equal(envFlags, "");
});

it("blocks shell metacharacters via single quotes", () => {
const key = '$(whoami)"; rm -rf /; echo "';
const envFlags = `-e NGC_API_KEY=${shellQuote(key)}`;
assert.ok(envFlags.startsWith("-e NGC_API_KEY='"));
assert.ok(envFlags.endsWith("'"));
});

it("prefers NGC_API_KEY over NVIDIA_API_KEY in fallback", () => {
const origNgc = process.env.NGC_API_KEY;
const origNvidia = process.env.NVIDIA_API_KEY;
try {
process.env.NGC_API_KEY = "ngc-primary";
process.env.NVIDIA_API_KEY = "nvidia-fallback";
const ngcKey = process.env.NGC_API_KEY || process.env.NVIDIA_API_KEY || "";
assert.equal(ngcKey, "ngc-primary");
} finally {
if (origNgc === undefined) delete process.env.NGC_API_KEY;
else process.env.NGC_API_KEY = origNgc;
if (origNvidia === undefined) delete process.env.NVIDIA_API_KEY;
else process.env.NVIDIA_API_KEY = origNvidia;
}
});

it("falls back to NVIDIA_API_KEY when NGC_API_KEY empty", () => {
const origNgc = process.env.NGC_API_KEY;
const origNvidia = process.env.NVIDIA_API_KEY;
try {
process.env.NGC_API_KEY = "";
process.env.NVIDIA_API_KEY = "nvidia-fallback";
const ngcKey = process.env.NGC_API_KEY || process.env.NVIDIA_API_KEY || "";
assert.equal(ngcKey, "nvidia-fallback");
} finally {
if (origNgc === undefined) delete process.env.NGC_API_KEY;
else process.env.NGC_API_KEY = origNgc;
if (origNvidia === undefined) delete process.env.NVIDIA_API_KEY;
else process.env.NVIDIA_API_KEY = origNvidia;
}
});
});

describe("shellQuote (shared helper)", () => {
it("wraps a simple value in single quotes", () => {
assert.equal(shellQuote("hello"), "'hello'");
});

it("escapes embedded single quotes", () => {
assert.equal(shellQuote("it's"), "'it'\\''s'");
});

it("handles empty string", () => {
assert.equal(shellQuote(""), "''");
});

it("neutralizes shell metacharacters", () => {
const result = shellQuote('$(whoami); rm -rf /');
assert.ok(result.startsWith("'"));
assert.ok(result.endsWith("'"));
assert.ok(result.includes("$(whoami)"));
});

it("coerces non-string values", () => {
assert.equal(shellQuote(12345), "'12345'");
assert.equal(shellQuote(null), "'null'");
});
});
});