From 6aa13c843ea06c211aad85c9d14c30530dce7b21 Mon Sep 17 00:00:00 2001 From: Aaron Erickson Date: Sun, 29 Mar 2026 19:25:44 -0700 Subject: [PATCH 1/5] feat(onboard): use OpenShell providers for messaging credentials Create OpenShell providers for Discord, Slack, and Telegram tokens during onboard instead of passing them as raw environment variables into the sandbox. The L7 proxy rewrites Authorization headers (Bearer/Bot) with real secrets at egress, so sandbox processes never see the actual token values. - Discord and Slack tokens flow through provider/placeholder system - Telegram provider is created for credential storage but host-side bridge still reads from host env (Telegram uses URL-path auth that the proxy can't rewrite yet) - Raw env var injection removed for Discord and Slack --- bin/lib/onboard.js | 47 ++++++++++++++++++++++++++++++++-------------- 1 file changed, 33 insertions(+), 14 deletions(-) diff --git a/bin/lib/onboard.js b/bin/lib/onboard.js index 53069e339..e38610a6d 100644 --- a/bin/lib/onboard.js +++ b/bin/lib/onboard.js @@ -1778,27 +1778,46 @@ async function createSandbox(gpu, model, provider, preferredInferenceApi = null, ]; // --gpu is intentionally omitted. See comment in startGateway(). - 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"; - patchStagedDockerfile(stagedDockerfile, model, chatUiUrl, String(Date.now()), provider, preferredInferenceApi); - // Only pass non-sensitive env vars to the sandbox. NVIDIA_API_KEY is NOT - // needed inside the sandbox — inference is proxied through the OpenShell - // gateway which injects the stored credential server-side. The gateway - // also strips any Authorization headers sent by the sandbox client. - // See: crates/openshell-sandbox/src/proxy.rs (header stripping), - // crates/openshell-router/src/backend.rs (server-side auth injection). - const envArgs = [formatEnvAssignment("CHAT_UI_URL", chatUiUrl)]; - const sandboxEnv = { ...process.env }; - delete sandboxEnv.NVIDIA_API_KEY; + // Create OpenShell providers for messaging credentials so they flow through + // the provider/placeholder system instead of raw env vars. The L7 proxy + // rewrites Authorization headers (Bearer/Bot) with real secrets at egress. + // Telegram provider is created for credential storage but the host-side bridge + // still reads from host env — Telegram uses URL-path auth (/bot{TOKEN}/) which + // the proxy can't rewrite yet. + const messagingProviders = []; const discordToken = getCredential("DISCORD_BOT_TOKEN") || process.env.DISCORD_BOT_TOKEN; if (discordToken) { - sandboxEnv.DISCORD_BOT_TOKEN = discordToken; + upsertProvider("discord-bridge", "generic", "DISCORD_BOT_TOKEN", null, { DISCORD_BOT_TOKEN: discordToken }); + messagingProviders.push("discord-bridge"); } const slackToken = getCredential("SLACK_BOT_TOKEN") || process.env.SLACK_BOT_TOKEN; if (slackToken) { - sandboxEnv.SLACK_BOT_TOKEN = slackToken; + upsertProvider("slack-bridge", "generic", "SLACK_BOT_TOKEN", null, { SLACK_BOT_TOKEN: slackToken }); + messagingProviders.push("slack-bridge"); + } + const telegramToken = getCredential("TELEGRAM_BOT_TOKEN") || process.env.TELEGRAM_BOT_TOKEN; + if (telegramToken) { + upsertProvider("telegram-bridge", "generic", "TELEGRAM_BOT_TOKEN", null, { TELEGRAM_BOT_TOKEN: telegramToken }); + messagingProviders.push("telegram-bridge"); + } + for (const p of messagingProviders) { + createArgs.push("--provider", p); } + 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"; + patchStagedDockerfile(stagedDockerfile, model, chatUiUrl, String(Date.now()), provider, preferredInferenceApi); + // Only pass non-sensitive env vars to the sandbox. Credentials flow through + // OpenShell providers — the gateway injects them as placeholders and the L7 + // proxy rewrites Authorization headers with real secrets at egress. + // See: crates/openshell-sandbox/src/secrets.rs (placeholder rewriting), + // crates/openshell-router/src/backend.rs (inference auth injection). + const envArgs = [formatEnvAssignment("CHAT_UI_URL", chatUiUrl)]; + const sandboxEnv = { ...process.env }; + delete sandboxEnv.NVIDIA_API_KEY; + delete sandboxEnv.DISCORD_BOT_TOKEN; + delete sandboxEnv.SLACK_BOT_TOKEN; + // Run without piping through awk — the pipe masked non-zero exit codes // from openshell because bash returns the status of the last pipeline // command (awk, always 0) unless pipefail is set. Removing the pipe From d8985440e386baa6fa2bbc6991b866fbb2e7364e Mon Sep 17 00:00:00 2001 From: Aaron Erickson Date: Sun, 29 Mar 2026 19:43:14 -0700 Subject: [PATCH 2/5] fix(security): verify messaging providers exist after sandbox creation Add verifyProviderExists() check post-sandbox-creation to confirm messaging credential providers are actually in the gateway. Warns with remediation steps if a provider is missing. --- bin/lib/onboard.js | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/bin/lib/onboard.js b/bin/lib/onboard.js index f97aa6d50..3c9b6fdfa 100644 --- a/bin/lib/onboard.js +++ b/bin/lib/onboard.js @@ -477,6 +477,11 @@ function upsertProvider(name, type, credentialEnv, baseUrl, env = {}) { } } +function verifyProviderExists(name) { + const output = runCaptureOpenshell(["provider", "get", name], { ignoreError: true }); + return Boolean(output && !output.includes("not found")); +} + function verifyInferenceRoute(_provider, _model) { const output = runCaptureOpenshell(["inference", "get"], { ignoreError: true }); if (!output || /Gateway inference:\s*[\r\n]+\s*Not configured/i.test(output)) { @@ -1902,6 +1907,15 @@ async function createSandbox(gpu, model, provider, preferredInferenceApi = null, console.log(" Setting up sandbox DNS proxy..."); run(`bash "${path.join(SCRIPTS, "setup-dns-proxy.sh")}" ${GATEWAY_NAME} "${sandboxName}" 2>&1 || true`, { ignoreError: true }); + // Verify messaging providers are attached to the sandbox + for (const p of messagingProviders) { + if (!verifyProviderExists(p)) { + console.error(` ⚠ Messaging provider '${p}' was not found in the gateway.`); + console.error(` The credential may not be available inside the sandbox.`); + console.error(` To fix: openshell provider create --name ${p} --type generic --credential `); + } + } + console.log(` ✓ Sandbox '${sandboxName}' created`); return sandboxName; } From 499053d059cdbac7093868286121bbd6c86fed3c Mon Sep 17 00:00:00 2001 From: Aaron Erickson Date: Sun, 29 Mar 2026 19:49:56 -0700 Subject: [PATCH 3/5] fix(security): test provider creation and sandbox attachment for messaging tokens Verify that messaging credentials flow through the provider system: - Provider create commands issued with correct credential key names - Sandbox create includes --provider flags for all detected tokens - Real token values never appear in sandbox create command or env --- test/onboard.test.js | 114 +++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 114 insertions(+) diff --git a/test/onboard.test.js b/test/onboard.test.js index 16b7e5453..480e93ecb 100644 --- a/test/onboard.test.js +++ b/test/onboard.test.js @@ -1029,6 +1029,120 @@ const { createSandbox } = require(${onboardPath}); assert.doesNotMatch(createCommand.command, /SLACK_BOT_TOKEN=/); }); + it("creates providers for messaging tokens and attaches them to the sandbox", async () => { + const repoRoot = path.join(import.meta.dirname, ".."); + const tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), "nemoclaw-onboard-messaging-providers-")); + const fakeBin = path.join(tmpDir, "bin"); + const scriptPath = path.join(tmpDir, "messaging-provider-check.js"); + const onboardPath = JSON.stringify(path.join(repoRoot, "bin", "lib", "onboard.js")); + const runnerPath = JSON.stringify(path.join(repoRoot, "bin", "lib", "runner.js")); + const registryPath = JSON.stringify(path.join(repoRoot, "bin", "lib", "registry.js")); + const preflightPath = JSON.stringify(path.join(repoRoot, "bin", "lib", "preflight.js")); + const credentialsPath = JSON.stringify(path.join(repoRoot, "bin", "lib", "credentials.js")); + + fs.mkdirSync(fakeBin, { recursive: true }); + fs.writeFileSync(path.join(fakeBin, "openshell"), "#!/usr/bin/env bash\nexit 0\n", { mode: 0o755 }); + + const script = String.raw` +const runner = require(${runnerPath}); +const registry = require(${registryPath}); +const preflight = require(${preflightPath}); +const credentials = require(${credentialsPath}); +const childProcess = require("node:child_process"); +const { EventEmitter } = require("node:events"); + +const commands = []; +runner.run = (command, opts = {}) => { + commands.push({ command, env: opts.env || null }); + return { status: 0 }; +}; +runner.runCapture = (command) => { + if (command.includes("'sandbox' 'get' 'my-assistant'")) return ""; + if (command.includes("'sandbox' 'list'")) return "my-assistant Ready"; + if (command.includes("'provider' 'get'")) return "Provider: discord-bridge"; + return ""; +}; +registry.registerSandbox = () => true; +registry.removeSandbox = () => true; +preflight.checkPortAvailable = async () => ({ ok: true }); +credentials.prompt = async () => ""; + +childProcess.spawn = (...args) => { + const child = new EventEmitter(); + child.stdout = new EventEmitter(); + child.stderr = new EventEmitter(); + commands.push({ command: args[1][1], env: args[2]?.env || null }); + process.nextTick(() => { + child.stdout.emit("data", Buffer.from("Created sandbox: my-assistant\n")); + child.emit("close", 0); + }); + return child; +}; + +const { createSandbox } = require(${onboardPath}); + +(async () => { + process.env.OPENSHELL_GATEWAY = "nemoclaw"; + process.env.DISCORD_BOT_TOKEN = "test-discord-token-value"; + process.env.SLACK_BOT_TOKEN = "xoxb-test-slack-token-value"; + process.env.TELEGRAM_BOT_TOKEN = "123456:ABC-test-telegram-token"; + const sandboxName = await createSandbox(null, "gpt-5.4"); + console.log(JSON.stringify({ sandboxName, commands })); +})().catch((error) => { + console.error(error); + process.exit(1); +}); +`; + fs.writeFileSync(scriptPath, script); + + const result = spawnSync(process.execPath, [scriptPath], { + cwd: repoRoot, + encoding: "utf-8", + env: { + ...process.env, + HOME: tmpDir, + PATH: `${fakeBin}:${process.env.PATH || ""}`, + NEMOCLAW_NON_INTERACTIVE: "1", + }, + }); + + assert.equal(result.status, 0, result.stderr); + const payloadLine = result.stdout + .trim() + .split("\n") + .slice() + .reverse() + .find((line) => line.startsWith("{") && line.endsWith("}")); + assert.ok(payloadLine, `expected JSON payload in stdout:\n${result.stdout}`); + const payload = JSON.parse(payloadLine); + + // Verify providers were created with the right credential keys + const providerCommands = payload.commands.filter((e) => e.command.includes("'provider' 'create'")); + const discordProvider = providerCommands.find((e) => e.command.includes("discord-bridge")); + assert.ok(discordProvider, "expected discord-bridge provider create command"); + assert.match(discordProvider.command, /'--credential' 'DISCORD_BOT_TOKEN'/); + + const slackProvider = providerCommands.find((e) => e.command.includes("slack-bridge")); + assert.ok(slackProvider, "expected slack-bridge provider create command"); + assert.match(slackProvider.command, /'--credential' 'SLACK_BOT_TOKEN'/); + + const telegramProvider = providerCommands.find((e) => e.command.includes("telegram-bridge")); + assert.ok(telegramProvider, "expected telegram-bridge provider create command"); + assert.match(telegramProvider.command, /'--credential' 'TELEGRAM_BOT_TOKEN'/); + + // Verify sandbox create includes --provider flags for all three + const createCommand = payload.commands.find((e) => e.command.includes("'sandbox' 'create'")); + assert.ok(createCommand, "expected sandbox create command"); + assert.match(createCommand.command, /'--provider' 'discord-bridge'/); + assert.match(createCommand.command, /'--provider' 'slack-bridge'/); + assert.match(createCommand.command, /'--provider' 'telegram-bridge'/); + + // Verify real token values are NOT in the sandbox create command or env + assert.doesNotMatch(createCommand.command, /test-discord-token-value/); + assert.doesNotMatch(createCommand.command, /xoxb-test-slack-token-value/); + assert.doesNotMatch(createCommand.command, /123456:ABC-test-telegram-token/); + }); + it("continues once the sandbox is Ready even if the create stream never closes", async () => { const repoRoot = path.join(import.meta.dirname, ".."); const tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), "nemoclaw-onboard-create-ready-")); From 9d4d5e1a1bcae3d44b3ab396fd1e1f23165e6035 Mon Sep 17 00:00:00 2001 From: Aaron Erickson Date: Sun, 29 Mar 2026 19:56:52 -0700 Subject: [PATCH 4/5] fix(security): address CodeRabbit review findings - Use exit code instead of string matching in verifyProviderExists() - Use hydrateCredentialEnv() for Telegram so host-side bridge can find the token from stored credentials - Replace individual delete statements with a blocklist that covers all credential env vars (NVIDIA, OpenAI, Anthropic, Gemini, compatible endpoints, Discord, Slack, Telegram) --- bin/lib/onboard.js | 24 +++++++++++++++++------- 1 file changed, 17 insertions(+), 7 deletions(-) diff --git a/bin/lib/onboard.js b/bin/lib/onboard.js index 3c9b6fdfa..16ad2e440 100644 --- a/bin/lib/onboard.js +++ b/bin/lib/onboard.js @@ -478,8 +478,8 @@ function upsertProvider(name, type, credentialEnv, baseUrl, env = {}) { } function verifyProviderExists(name) { - const output = runCaptureOpenshell(["provider", "get", name], { ignoreError: true }); - return Boolean(output && !output.includes("not found")); + const result = runOpenshell(["provider", "get", name], { ignoreError: true }); + return result.status === 0; } function verifyInferenceRoute(_provider, _model) { @@ -1800,7 +1800,7 @@ async function createSandbox(gpu, model, provider, preferredInferenceApi = null, upsertProvider("slack-bridge", "generic", "SLACK_BOT_TOKEN", null, { SLACK_BOT_TOKEN: slackToken }); messagingProviders.push("slack-bridge"); } - const telegramToken = getCredential("TELEGRAM_BOT_TOKEN") || process.env.TELEGRAM_BOT_TOKEN; + const telegramToken = hydrateCredentialEnv("TELEGRAM_BOT_TOKEN") || process.env.TELEGRAM_BOT_TOKEN; if (telegramToken) { upsertProvider("telegram-bridge", "generic", "TELEGRAM_BOT_TOKEN", null, { TELEGRAM_BOT_TOKEN: telegramToken }); messagingProviders.push("telegram-bridge"); @@ -1818,10 +1818,20 @@ async function createSandbox(gpu, model, provider, preferredInferenceApi = null, // See: crates/openshell-sandbox/src/secrets.rs (placeholder rewriting), // crates/openshell-router/src/backend.rs (inference auth injection). const envArgs = [formatEnvAssignment("CHAT_UI_URL", chatUiUrl)]; - const sandboxEnv = { ...process.env }; - delete sandboxEnv.NVIDIA_API_KEY; - delete sandboxEnv.DISCORD_BOT_TOKEN; - delete sandboxEnv.SLACK_BOT_TOKEN; + const blockedSandboxEnvNames = new Set([ + "NVIDIA_API_KEY", + "OPENAI_API_KEY", + "ANTHROPIC_API_KEY", + "GEMINI_API_KEY", + "COMPATIBLE_API_KEY", + "COMPATIBLE_ANTHROPIC_API_KEY", + "DISCORD_BOT_TOKEN", + "SLACK_BOT_TOKEN", + "TELEGRAM_BOT_TOKEN", + ]); + const sandboxEnv = Object.fromEntries( + Object.entries(process.env).filter(([name]) => !blockedSandboxEnvNames.has(name)) + ); // Run without piping through awk — the pipe masked non-zero exit codes // from openshell because bash returns the status of the last pipeline From 752fc94878c5147152b14f63658064668655a9c4 Mon Sep 17 00:00:00 2001 From: Aaron Erickson Date: Sun, 29 Mar 2026 20:41:08 -0700 Subject: [PATCH 5/5] fix(security): update credential-exposure test for blocklist pattern The test pattern-matched on the old `{ ...process.env }` spread. Update to verify the blocklist approach that strips all credential env vars from sandboxEnv. --- test/credential-exposure.test.js | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/test/credential-exposure.test.js b/test/credential-exposure.test.js index 08f880a9d..43e947158 100644 --- a/test/credential-exposure.test.js +++ b/test/credential-exposure.test.js @@ -76,7 +76,12 @@ describe("credential exposure in process arguments", () => { it("onboard.js does not embed sandbox secrets in the sandbox create command line", () => { const src = fs.readFileSync(ONBOARD_JS, "utf-8"); - expect(src).toMatch(/const sandboxEnv = \{ \.\.\.process\.env \};/); + // sandboxEnv must be built with a blocklist that strips all credential env vars + expect(src).toMatch(/blockedSandboxEnvNames/); + expect(src).toMatch(/NVIDIA_API_KEY/); + expect(src).toMatch(/DISCORD_BOT_TOKEN/); + expect(src).toMatch(/SLACK_BOT_TOKEN/); + expect(src).toMatch(/TELEGRAM_BOT_TOKEN/); expect(src).toMatch(/streamSandboxCreate\(createCommand, sandboxEnv(?:, \{)?/); expect(src).not.toMatch(/envArgs\.push\(formatEnvAssignment\("NVIDIA_API_KEY"/); expect(src).not.toMatch(/envArgs\.push\(formatEnvAssignment\("DISCORD_BOT_TOKEN"/);