Skip to content

fix: use execFile in status and logs to prevent shell interpolation of sandbox name #281

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
areporeporepo wants to merge 1 commit into from
Closed

fix: use execFile in status and logs to prevent shell interpolation of sandbox name #281

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
12 changes: 7 additions & 5 deletions nemoclaw/src/commands/logs.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,12 +5,12 @@
* `openclaw nemoclaw logs` — stream or tail blueprint execution and sandbox logs.
*/

import { exec, spawn } from "node:child_process";
import { execFile, spawn } from "node:child_process";
import { promisify } from "node:util";
import type { PluginLogger, NemoClawConfig } from "../index.js";
import { loadState } from "../blueprint/state.js";

const execAsync = promisify(exec);
const execFileAsync = promisify(execFile);

export interface LogsOptions {
follow: boolean;
Expand Down Expand Up @@ -61,9 +61,11 @@ export async function cliLogs(opts: LogsOptions): Promise<void> {

async function isSandboxRunning(sandboxName: string): Promise<boolean> {
try {
const { stdout } = await execAsync(`openshell sandbox get ${sandboxName} --json`, {
timeout: 5000,
});
const { stdout } = await execFileAsync(
"openshell",
["sandbox", "get", sandboxName, "--json"],
{ timeout: 5000 },
);
const parsed = JSON.parse(stdout) as { state?: string };
return parsed.state === "running";
} catch {
Expand Down
52 changes: 28 additions & 24 deletions nemoclaw/src/commands/status.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ vi.mock("node:fs", () => ({

// Mock node:child_process — controls openshell command results
vi.mock("node:child_process", () => ({
exec: vi.fn(),
execFile: vi.fn(),
}));

// Mock state loader — controls plugin state
Expand All @@ -26,7 +26,7 @@ vi.mock("../blueprint/state.js", () => ({

// Import after mocks are set up
const { existsSync } = await import("node:fs");
const { exec } = await import("node:child_process");
const { execFile } = await import("node:child_process");
const { loadState } = await import("../blueprint/state.js");
const { cliStatus } = await import("./status.js");

Expand Down Expand Up @@ -82,18 +82,20 @@ function captureLogger(): { lines: string[]; logger: PluginLogger } {
}

/**
* Make the exec mock resolve with the given stdout, or reject if error is set.
* Routes by command substring so sandbox and inference calls can differ.
* Make the execFile mock resolve with the given stdout, or reject if error is set.
* Routes by argument substring so sandbox and inference calls can differ.
*/
function mockExec(responses: Record<string, string | Error>): void {
vi.mocked(exec).mockImplementation(((
cmd: string,
function mockExecFile(responses: Record<string, string | Error>): void {
vi.mocked(execFile).mockImplementation(((
_file: string,
args: string[],
_opts: unknown,
callback?: (err: Error | null, result: { stdout: string; stderr: string }) => void,
) => {
// promisify(exec)(cmd, opts) calls exec(cmd, opts, callback)
// promisify(execFile)(file, args, opts) calls execFile(file, args, opts, callback)
const joined = args.join(" ");
for (const [substring, response] of Object.entries(responses)) {
if (cmd.includes(substring)) {
if (joined.includes(substring)) {
if (response instanceof Error) {
callback?.(response, { stdout: "", stderr: response.message });
} else {
Expand All @@ -103,8 +105,8 @@ function mockExec(responses: Record<string, string | Error>): void {
}
}
// Default: command not found
callback?.(new Error(`command not found: ${cmd}`), { stdout: "", stderr: "" });
}) as typeof exec);
callback?.(new Error(`command not found: ${joined}`), { stdout: "", stderr: "" });
}) as typeof execFile);
}

// ---------------------------------------------------------------------------
Expand All @@ -115,7 +117,7 @@ beforeEach(() => {
vi.resetAllMocks();
vi.mocked(existsSync).mockReturnValue(false);
vi.mocked(loadState).mockReturnValue(blankState());
mockExec({});
mockExecFile({});
});

describe("cliStatus", () => {
Expand Down Expand Up @@ -162,7 +164,7 @@ describe("cliStatus", () => {
// =========================================================================
describe("host — sandbox running, inference configured", () => {
beforeEach(() => {
mockExec({
mockExecFile({
"sandbox status": JSON.stringify({ state: "running", uptime: "2h 14m" }),
"inference get": JSON.stringify({
provider: "nvidia",
Expand Down Expand Up @@ -216,7 +218,7 @@ describe("cliStatus", () => {
// =========================================================================
describe("host — sandbox running, no inference", () => {
beforeEach(() => {
mockExec({
mockExecFile({
"sandbox status": JSON.stringify({ state: "running", uptime: "45m 12s" }),
"inference get": new Error("no inference configured"),
});
Expand Down Expand Up @@ -291,7 +293,7 @@ describe("cliStatus", () => {

await cliStatus({ json: false, logger, pluginConfig: defaultConfig });

expect(exec).not.toHaveBeenCalled();
expect(execFile).not.toHaveBeenCalled();
});

it("JSON output has insideSandbox: true everywhere", async () => {
Expand Down Expand Up @@ -367,7 +369,7 @@ describe("cliStatus", () => {
...blankState(),
sandboxName: "custom-sandbox",
});
mockExec({
mockExecFile({
"sandbox status": JSON.stringify({ state: "running", uptime: "1m" }),
"inference get": new Error("not configured"),
});
Expand All @@ -378,26 +380,28 @@ describe("cliStatus", () => {
const output = lines.join("\n");
expect(output).toContain("Name: custom-sandbox");

// Verify the exec call used the custom sandbox name
expect(exec).toHaveBeenCalledWith(
expect.stringContaining("custom-sandbox"),
// Verify the execFile call used the custom sandbox name
expect(execFile).toHaveBeenCalledWith(
"openshell",
expect.arrayContaining(["custom-sandbox"]),
expect.anything(),
expect.anything(),
);
});

it("defaults sandbox name to 'openclaw' when state has none", async () => {
mockExec({
mockExecFile({
"sandbox status": new Error("not found"),
"inference get": new Error("not configured"),
});

const { lines, logger } = captureLogger();
await cliStatus({ json: true, logger, pluginConfig: defaultConfig });

// Verify exec was called with default name
expect(exec).toHaveBeenCalledWith(
expect.stringContaining("openclaw"),
// Verify execFile was called with default name
expect(execFile).toHaveBeenCalledWith(
"openshell",
expect.arrayContaining(["openclaw"]),
expect.anything(),
expect.anything(),
);
Expand Down Expand Up @@ -428,7 +432,7 @@ describe("cliStatus", () => {
});

it("handles sandbox running but with missing uptime field", async () => {
mockExec({
mockExecFile({
"sandbox status": JSON.stringify({ state: "running" }),
"inference get": new Error("not configured"),
});
Expand Down
20 changes: 12 additions & 8 deletions nemoclaw/src/commands/status.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,13 +1,13 @@
// SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved.
// SPDX-License-Identifier: Apache-2.0

import { exec } from "node:child_process";
import { execFile } from "node:child_process";
import { existsSync } from "node:fs";
import { promisify } from "node:util";
import type { PluginLogger, NemoClawConfig } from "../index.js";
import { loadState } from "../blueprint/state.js";

const execAsync = promisify(exec);
const execFileAsync = promisify(execFile);

/**
* Detect whether the plugin is running inside an OpenShell sandbox.
Expand Down Expand Up @@ -128,9 +128,11 @@ async function getSandboxStatus(sandboxName: string, insideSandbox: boolean): Pr
return { name: sandboxName, running: false, uptime: null, insideSandbox: true };
}
try {
const { stdout } = await execAsync(`openshell sandbox status ${sandboxName} --json`, {
timeout: 5000,
});
const { stdout } = await execFileAsync(
"openshell",
["sandbox", "status", sandboxName, "--json"],
{ timeout: 5000 },
);
const parsed = JSON.parse(stdout) as SandboxStatusResponse;
return {
name: sandboxName,
Expand Down Expand Up @@ -162,9 +164,11 @@ async function getInferenceStatus(insideSandbox: boolean): Promise<InferenceStat
return { configured: false, provider: null, model: null, endpoint: null, insideSandbox: true };
}
try {
const { stdout } = await execAsync("openshell inference get --json", {
timeout: 5000,
});
const { stdout } = await execFileAsync(
"openshell",
["inference", "get", "--json"],
{ timeout: 5000 },
);
const parsed = JSON.parse(stdout) as InferenceStatusResponse;
return {
configured: true,
Expand Down