Skip to content
45 changes: 29 additions & 16 deletions src/lib/actions/sandbox/rebuild.ts
Original file line number Diff line number Diff line change
Expand Up @@ -23,10 +23,12 @@ const hermesProviderAuth = require("../../hermes-provider-auth") as {
baseUrl?: string,
) => void;
};
const { LOCAL_INFERENCE_PROVIDERS, REMOTE_PROVIDER_CONFIG } = require("../../onboard/providers") as {
LOCAL_INFERENCE_PROVIDERS: string[];
REMOTE_PROVIDER_CONFIG: Record<string, { providerName: string; credentialEnv: string | null }>;
};
const { LOCAL_INFERENCE_PROVIDERS, REMOTE_PROVIDER_CONFIG, providerExistsInGateway } =
require("../../onboard/providers") as {
LOCAL_INFERENCE_PROVIDERS: string[];
REMOTE_PROVIDER_CONFIG: Record<string, { providerName: string; credentialEnv: string | null }>;
providerExistsInGateway: (name: string, runOpenshellFn: typeof runOpenshell) => boolean;
};

import {
detectOpenShellStateRpcPreflightIssue,
Expand Down Expand Up @@ -370,18 +372,29 @@ export async function rebuildSandbox(
`Preflight credential check: ${rebuildCredentialEnv} → ${credentialValue ? "present" : "MISSING"}`,
);
if (!credentialValue) {
console.error("");
console.error(` ${_RD}Rebuild preflight failed:${R} provider credential not found.`);
console.error(` The non-interactive recreate step requires ${rebuildCredentialEnv},`);
console.error(" but it is not set in the environment.");
console.error("");
console.error(" To fix, do one of:");
console.error(` export ${rebuildCredentialEnv}=<your-key>`);
console.error(` ${CLI_NAME} onboard # re-enter the key interactively`);
console.error("");
console.error(" Sandbox is untouched — no data was lost.");
bail(`Missing credential: ${rebuildCredentialEnv}`);
return;
// When the inference provider is already registered in the OpenShell
// gateway, the recreate step does not need a host env value — the
// gateway is the source of truth for the secret. Skip the env-only
// preflight in that case so flows like `channels add` + rebuild keep
// working when the user has logged out of the original shell.
if (rebuildProvider && providerExistsInGateway(rebuildProvider, runOpenshell)) {
log(
`Preflight credential check: provider '${rebuildProvider}' registered in gateway — skipping env check for ${rebuildCredentialEnv}`,
);
} else {
console.error("");
console.error(` ${_RD}Rebuild preflight failed:${R} provider credential not found.`);
console.error(` The non-interactive recreate step requires ${rebuildCredentialEnv},`);
console.error(" but it is not set in the environment.");
console.error("");
console.error(" To fix, do one of:");
console.error(` export ${rebuildCredentialEnv}=<your-key>`);
console.error(` ${CLI_NAME} onboard # re-enter the key interactively`);
console.error("");
console.error(" Sandbox is untouched — no data was lost.");
bail(`Missing credential: ${rebuildCredentialEnv}`);
return;
}
}
} else {
// No credentialEnv in session — local inference (Ollama/vLLM) or
Expand Down
21 changes: 10 additions & 11 deletions src/lib/onboard.ts
Original file line number Diff line number Diff line change
Expand Up @@ -919,9 +919,7 @@ function upsertMessagingProviders(
if (mutated) persistMigratedLegacyKeys();
return upserted;
}
function providerExistsInGateway(name: string) {
return onboardProviders.providerExistsInGateway(name, runOpenshell);
}
const providerExistsInGateway = (name: string) => onboardProviders.providerExistsInGateway(name, runOpenshell);

// Tri-state probe factory for messaging-conflict backfill. An upfront liveness
// check is necessary because `openshell provider get` exits non-zero for both
Expand Down Expand Up @@ -4431,16 +4429,17 @@ async function setupNim(
}
if (isNonInteractive()) {
const resolvedNvidiaKey = resolveProviderCredential("NVIDIA_API_KEY");
if (!resolvedNvidiaKey) {
if (resolvedNvidiaKey) {
const keyError = validateNvidiaApiKeyValue(resolvedNvidiaKey);
if (keyError) {
console.error(keyError);
console.error(` Get a key from ${REMOTE_PROVIDER_CONFIG.build.helpUrl}`);
process.exit(1);
}
} else if (!providerExistsInGateway(provider)) {
logMissingNvidiaApiKeyHelp(REMOTE_PROVIDER_CONFIG.build.helpUrl);
process.exit(1);
}
const keyError = validateNvidiaApiKeyValue(resolvedNvidiaKey);
if (keyError) {
console.error(keyError);
console.error(` Get a key from ${REMOTE_PROVIDER_CONFIG.build.helpUrl}`);
process.exit(1);
}
} else {
await ensureApiKey();
}
Expand Down Expand Up @@ -4503,7 +4502,7 @@ async function setupNim(
break;
}
if (isNonInteractive()) {
if (!resolveProviderCredential(selectedCredentialEnv)) {
if (!resolveProviderCredential(selectedCredentialEnv) && !providerExistsInGateway(provider)) {
console.error(
` ${selectedCredentialEnv} (or NEMOCLAW_PROVIDER_KEY) is required for ${remoteConfig.label} in non-interactive mode.`,
);
Expand Down
65 changes: 65 additions & 0 deletions src/lib/onboard/providers.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -168,6 +168,71 @@ describe("onboard provider helpers", () => {
expect(commands[1]).toMatch(/--config OPENAI_BASE_URL=https:\/\/integrate\.api\.nvidia\.com\/v1/);
});

it("omits --credential from the update args when the env value is empty", () => {
const commands: string[] = [];
const result = upsertProvider(
"nvidia-prod",
"openai",
"NVIDIA_API_KEY",
"https://integrate.api.nvidia.com/v1",
{},
(command) => {
commands.push(command.join(" "));
return { status: 0, stdout: "", stderr: "" };
},
);

expect(result).toEqual({ ok: true });
expect(commands).toHaveLength(2);
expect(commands[0]).toMatch(/provider get/);
expect(commands[1]).toMatch(/^provider update nvidia-prod /);
// OpenShell CLI rejects `--credential KEY` when the host env is empty;
// dropping the flag turns the call into a no-op merge that succeeds.
expect(commands[1]).not.toMatch(/--credential/);
expect(commands[1]).toMatch(/OPENAI_BASE_URL=https:\/\/integrate\.api\.nvidia\.com\/v1/);
});

it("keeps --credential on the create path even when env is empty", () => {
// create cannot omit credentials — OpenShell rejects empty credential
// maps on creation. The caller is responsible for staging a value.
const commands: string[] = [];
upsertProvider(
"fresh-provider",
"generic",
"FRESH_TOKEN",
null,
{},
(command) => {
commands.push(command.join(" "));
if (command.includes("get")) return { status: 1, stdout: "", stderr: "" };
return { status: 0, stdout: "", stderr: "" };
},
);

expect(commands).toHaveLength(2);
expect(commands[1]).toMatch(/^provider create --name fresh-provider /);
expect(commands[1]).toMatch(/--credential FRESH_TOKEN/);
});

it("keeps --credential on the update path when a value is staged in env", () => {
const commands: string[] = [];
upsertProvider(
"nvidia-prod",
"openai",
"NVIDIA_API_KEY",
null,
{ NVIDIA_API_KEY: "nvapi-staged" },
(command) => {
commands.push(command.join(" "));
return { status: 0, stdout: "", stderr: "" };
},
);

expect(commands).toHaveLength(2);
expect(commands[1]).toMatch(/^provider update nvidia-prod /);
expect(commands[1]).toMatch(/--credential NVIDIA_API_KEY/);
});

it("returns redacted error details when create or update fails", () => {
const result = upsertProvider("bad-provider", "generic", "SOME_KEY", null, {}, (command) => {
if (command.includes("get")) return { status: 1, stdout: "", stderr: "" };
Expand Down
26 changes: 22 additions & 4 deletions src/lib/onboard/providers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -234,13 +234,23 @@ function getRequestedModelHint(nonInteractive) {
* @param {string} type - Provider type (e.g. "openai", "anthropic", "generic").
* @param {string} credentialEnv - Credential environment variable name.
* @param {string|null} baseUrl - Optional base URL for API-compatible endpoints.
* @param {{ includeCredential?: boolean }} [opts] - When `includeCredential` is
* false, the `--credential` flag is omitted from the args. Used on the
* `provider update` path when the host env does not carry the credential and
* the gateway already holds it (no rotation needed). OpenShell's CLI rejects
* `--credential KEY` when the local env var is empty, so passing the flag
* would fail before reaching the gateway.
* @returns {string[]} Argument array for runOpenshell().
*/
function buildProviderArgs(action, name, type, credentialEnv, baseUrl) {
function buildProviderArgs(action, name, type, credentialEnv, baseUrl, opts = {}) {
const { includeCredential = true } = opts;
const args =
action === "create"
? ["provider", "create", "--name", name, "--type", type, "--credential", credentialEnv]
: ["provider", "update", name, "--credential", credentialEnv];
? ["provider", "create", "--name", name, "--type", type]
: ["provider", "update", name];
if (includeCredential) {
args.push("--credential", credentialEnv);
}
if (baseUrl && type === "openai") {
args.push("--config", `OPENAI_BASE_URL=${baseUrl}`);
} else if (baseUrl && type === "anthropic") {
Expand Down Expand Up @@ -303,7 +313,15 @@ function upsertProvider(name, type, credentialEnv, baseUrl, env, _runOpenshell,
}
}
const action = exists && !options.replaceExisting ? "update" : "create";
const args = buildProviderArgs(action, name, type, credentialEnv, baseUrl);
// On the update path, the OpenShell CLI's `--credential KEY` form reads the
// value from the host env and aborts when empty. If the caller did not stage
// a credential value (rebuild after `channels add` with the original env
// unset), drop the flag so `provider update` becomes a no-op merge — the
// gateway already holds the secret.
const credentialValueAvailable =
!!credentialEnv && typeof env[credentialEnv] === "string" && env[credentialEnv].length > 0;
const includeCredential = action === "create" || credentialValueAvailable;
const args = buildProviderArgs(action, name, type, credentialEnv, baseUrl, { includeCredential });
const runOpts = { ignoreError: true, env, stdio: ["ignore", "pipe", "pipe"] };
const result = _runOpenshell(args, runOpts);
if (result.status !== 0) {
Expand Down
30 changes: 30 additions & 0 deletions test/e2e/test-channels-add-remove.sh
Original file line number Diff line number Diff line change
Expand Up @@ -323,6 +323,16 @@ section "Phase 3: channels add telegram + rebuild"
# the operator decides to add a channel and exports the token first.
export TELEGRAM_BOT_TOKEN="$TELEGRAM_TOKEN"

# Gateway-credential reuse gate. Before the fix, the rebuild preflight
# aborted with "provider credential not found" when NVIDIA_API_KEY was unset
# in the host env even though the inference provider was already registered
# in the OpenShell gateway. Drop the key from the env around `channels add`
# + rebuild so the post-add rebuild has to reuse the gateway-stored
# credential instead of demanding it back on the host.
NVIDIA_API_KEY_BACKUP="${NVIDIA_API_KEY:-}"
unset NVIDIA_API_KEY
info "NVIDIA_API_KEY unset for gateway-credential-reuse gate; gateway must hold the credential"

if nemoclaw "$SANDBOX_NAME" channels add telegram >/tmp/nc-add.log 2>&1; then
add_rc=0
else
Expand All @@ -342,9 +352,29 @@ if run_rebuild_with_live_log /tmp/nc-rebuild-add.log; then
else
fail "C3b: rebuild (post-add) failed"
tail -100 /tmp/nc-rebuild-add.log 2>/dev/null || true
# Restore env before bailing so later phases (and operators rerunning
# the script interactively) still see the original key.
if [ -n "$NVIDIA_API_KEY_BACKUP" ]; then
export NVIDIA_API_KEY="$NVIDIA_API_KEY_BACKUP"
fi
print_summary
fi

# Gateway-credential reuse assertion: the rebuild must not have aborted with
# the "provider credential not found" error.
if grep -q "provider credential not found" /tmp/nc-rebuild-add.log; then
fail "C3c: REGRESSION — rebuild aborted on missing NVIDIA_API_KEY despite gateway-registered credential"
else
pass "C3c: rebuild reused gateway-stored credential without NVIDIA_API_KEY"
fi

# Restore for the remaining phases — `channels remove` + rebuild should
# work in the normal env-present case too.
if [ -n "$NVIDIA_API_KEY_BACKUP" ]; then
export NVIDIA_API_KEY="$NVIDIA_API_KEY_BACKUP"
fi
unset NVIDIA_API_KEY_BACKUP

# ══════════════════════════════════════════════════════════════════
# Phase 4: Post-add assertions (Test 2 acceptance, regression #3437)
# ══════════════════════════════════════════════════════════════════
Expand Down
46 changes: 45 additions & 1 deletion test/e2e/test-hermes-discord-e2e.sh
Original file line number Diff line number Diff line change
Expand Up @@ -580,7 +580,51 @@ else
dump_hermes_discord_diagnostics
fi

section "Phase 8: Cleanup"
section "Phase 8: Gateway-stored credential rebuild"

# Rebuild with NVIDIA_API_KEY unset so the preflight is forced to reuse the
# gateway-stored inference credential. Catches the Hermes regression that
# motivated the gateway-aware credential check in setupNim + rebuild.

# Phase 7's fake Discord Gateway leaves a root-owned scratch dir at
# $REPO/.tmp/fake-discord.* via its Docker bind-mount. `nemoclaw rebuild`
# recreates the sandbox by copying $REPO into a build context, which hits
# EACCES on those files because the runner uid cannot read root-owned
# bytes. Tear the container down and `sudo rm` the scratch before the
# rebuild so the build context copy succeeds.
if [ -n "${FAKE_DISCORD_GATEWAY_CONTAINER:-}" ]; then
docker rm -f "$FAKE_DISCORD_GATEWAY_CONTAINER" >/dev/null 2>&1 || true
fi
if [ -d "$REPO/.tmp" ]; then
sudo rm -rf "$REPO/.tmp"/fake-discord.* 2>/dev/null || rm -rf "$REPO/.tmp"/fake-discord.* 2>/dev/null || true
fi

NVIDIA_API_KEY_BACKUP="${NVIDIA_API_KEY:-}"
unset NVIDIA_API_KEY
info "NVIDIA_API_KEY unset; gateway must hold the inference credential"

HERMES_REBUILD_LOG="/tmp/nc-hermes-rebuild-noenv.log"
if nemoclaw "$SANDBOX_NAME" rebuild --yes >"$HERMES_REBUILD_LOG" 2>&1; then
rebuild_rc=0
else
rebuild_rc=$?
fi

if [ "$rebuild_rc" -ne 0 ]; then
fail "Hermes rebuild failed with NVIDIA_API_KEY unset (rc=${rebuild_rc})"
tail -80 "$HERMES_REBUILD_LOG" 2>/dev/null || true
elif grep -q "provider credential not found" "$HERMES_REBUILD_LOG"; then
fail "REGRESSION — rebuild aborted on missing NVIDIA_API_KEY despite gateway-registered credential"
else
pass "Hermes rebuild reused gateway-stored credential without NVIDIA_API_KEY"
fi

if [ -n "$NVIDIA_API_KEY_BACKUP" ]; then
export NVIDIA_API_KEY="$NVIDIA_API_KEY_BACKUP"
fi
unset NVIDIA_API_KEY_BACKUP

section "Phase 9: Cleanup"

if [[ "${NEMOCLAW_E2E_KEEP_SANDBOX:-}" != "1" ]]; then
nemoclaw "$SANDBOX_NAME" destroy --yes 2>&1 | tail -3 || true
Expand Down
8 changes: 8 additions & 0 deletions test/onboard-selection.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4783,6 +4783,14 @@ const { setupNim, __setNonInteractive } = onboardModule.exports;
const runnerPath = JSON.stringify(path.join(repoRoot, "dist", "lib", "runner.js"));

fs.mkdirSync(fakeBin, { recursive: true });
// Fake openshell: report the inference provider as absent so the
// gateway-credential-reuse fallback does NOT swallow the missing-key
// error path under test.
fs.writeFileSync(
path.join(fakeBin, "openshell"),
`#!${process.execPath}\nprocess.exit(1);\n`,
{ mode: 0o755 },
);

const script = String.raw`
const fs = require("fs");
Expand Down
Loading
Loading