Skip to content

Commit e225dfb

Browse files
yimojyimoj
authored
fix(cli): include OpenShell audit events in logs (#2590)
## Summary Make `nemoclaw <sandbox> logs` include OpenShell audit events so policy denials show up alongside the existing OpenClaw gateway log output. ## Related Issue Fixes #2512 ## Changes - Enable sandbox audit logs before reading logs. - Read both the in-sandbox `/tmp/gateway.log` source and `openshell logs <sandbox> --source all`. - In `--follow` mode, keep the remaining log source running if one source exits. - Add CLI regression coverage for both log sources and follow-mode source shutdown. ## Type of Change - [x] Code change (feature, bug fix, or refactor) - [ ] Code change with doc updates - [ ] Doc only (prose changes, no code sample modifications) - [ ] Doc only (includes code sample changes) ## Verification - [ ] `npx prek run --all-files` passes - [ ] `npm test` passes - [x] Tests added or updated for new or changed behavior - [x] No secrets, API keys, or credentials committed - [ ] Docs updated for user-facing behavior changes - [ ] `make docs` builds without warnings (doc changes only) - [ ] Doc pages follow the [style guide](https://github.com/NVIDIA/NemoClaw/blob/main/docs/CONTRIBUTING.md) (doc changes only) - [ ] New doc pages include SPDX header and frontmatter (new pages only) Additional verification: - `npm run build:cli` - `npx vitest run test/cli.test.ts -t logs` - `npm run typecheck:cli` - Pre-push hooks passed during `git push --force-with-lease` - E2E repro confirmed denied `curl https://example.com` appears in `nemoclaw repro-2512-e2e logs --follow` ## AI Disclosure - [x] AI-assisted — tool: OpenAI Codex --- Signed-off-by: Yimo Jiang <yimoj@nvidia.com> <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * `logs --follow` now streams gateway and sandbox logs concurrently and derives CLI exit status from child streams. * **Improvements** * Non-follow log collection probes gateway availability before gathering logs, with a configurable timeout and clearer “unavailable” messaging. * Per-command timeouts honored and follow lifecycle handles signals with a timed forced-exit fallback. * **Bug Fixes** * Still collects logs and emits warnings when enabling sandbox audit logging fails or times out. * **Tests / Chores** * Expanded tests for multi-source streaming, probes/timeouts, signal handling, and minor test/e2e updates. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Signed-off-by: Yimo Jiang <yimoj@nvidia.com>
1 parent b4ef3db commit e225dfb

8 files changed

Lines changed: 601 additions & 139 deletions

File tree

‎src/lib/openshell.test.ts‎

Lines changed: 58 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,10 @@ function stubSpawnSync(spec: SpawnResultSpec): OpenshellSpawnSync {
3939
return () => makeSpawnResult(spec);
4040
}
4141

42+
function timeoutError(): Error {
43+
return Object.assign(new Error("spawnSync openshell ETIMEDOUT"), { code: "ETIMEDOUT" });
44+
}
45+
4246
function exitWithCode(code: number): never {
4347
throw new Error(`exit:${code}`);
4448
}
@@ -94,6 +98,60 @@ describe("openshell helpers", () => {
9498
expect(result.status).toBe(0);
9599
});
96100

101+
it("passes timeout options through to OpenShell spawn calls", () => {
102+
const timeouts: Array<number | undefined> = [];
103+
const spawnSyncImpl: OpenshellSpawnSync = (_command, _args, options) => {
104+
timeouts.push(options.timeout);
105+
return makeSpawnResult({ status: 0, stdout: "ok\n", stderr: "" });
106+
};
107+
108+
runOpenshellCommand("openshell", ["status"], { timeout: 4321, spawnSyncImpl });
109+
captureOpenshellCommand("openshell", ["status"], { timeout: 9876, spawnSyncImpl });
110+
111+
expect(timeouts).toEqual([4321, 9876]);
112+
});
113+
114+
it("returns ignored run timeouts so callers can fall back", () => {
115+
const result = runOpenshellCommand("openshell", ["status"], {
116+
ignoreError: true,
117+
timeout: 1,
118+
spawnSyncImpl: stubSpawnSync({
119+
status: null,
120+
stdout: "",
121+
stderr: "",
122+
error: timeoutError(),
123+
signal: "SIGTERM",
124+
}),
125+
exit: exitWithCode,
126+
});
127+
128+
expect(result.status).toBeNull();
129+
expect(result.signal).toBe("SIGTERM");
130+
expect(result.error?.message).toContain("ETIMEDOUT");
131+
});
132+
133+
it("returns ignored capture timeouts with timeout metadata", () => {
134+
const result = captureOpenshellCommand("openshell", ["sandbox", "list"], {
135+
ignoreError: true,
136+
timeout: 1,
137+
spawnSyncImpl: stubSpawnSync({
138+
status: null,
139+
stdout: "partial\n",
140+
stderr: "timeout detail\n",
141+
error: timeoutError(),
142+
signal: "SIGTERM",
143+
}),
144+
exit: exitWithCode,
145+
});
146+
147+
expect(result).toEqual({
148+
status: null,
149+
output: "partial",
150+
error: expect.objectContaining({ message: expect.stringContaining("ETIMEDOUT") }),
151+
signal: "SIGTERM",
152+
});
153+
});
154+
97155
it("uses the injected exit handler on failure", () => {
98156
expect(() =>
99157
runOpenshellCommand("openshell", ["status"], {

‎src/lib/openshell.ts‎

Lines changed: 23 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -17,23 +17,24 @@ export type OpenshellSpawnSync = (
1717
interface OpenshellSpawnOptions {
1818
cwd?: string;
1919
env?: NodeJS.ProcessEnv;
20+
timeout?: number;
21+
ignoreError?: boolean;
2022
spawnSyncImpl?: OpenshellSpawnSync;
2123
errorLine?: (message: string) => void;
2224
exit?: (code: number) => never;
2325
}
2426

2527
export interface RunOpenshellOptions extends OpenshellSpawnOptions {
2628
stdio?: SpawnSyncOptions["stdio"];
27-
ignoreError?: boolean;
2829
}
2930

30-
export interface CaptureOpenshellOptions extends OpenshellSpawnOptions {
31-
ignoreError?: boolean;
32-
}
31+
export interface CaptureOpenshellOptions extends OpenshellSpawnOptions {}
3332

3433
export interface CaptureOpenshellResult {
35-
status: number;
34+
status: number | null;
3635
output: string;
36+
error?: Error;
37+
signal?: NodeJS.Signals | null;
3738
}
3839

3940
// eslint-disable-next-line no-control-regex
@@ -76,6 +77,10 @@ function handleSpawnError(
7677
return (opts.exit ?? ((code) => process.exit(code)))(1);
7778
}
7879

80+
function isIgnoredTimeout(error: Error, opts: OpenshellSpawnOptions): boolean {
81+
return opts.ignoreError === true && (error as NodeJS.ErrnoException).code === "ETIMEDOUT";
82+
}
83+
7984
export function runOpenshellCommand(
8085
binary: string,
8186
args: string[],
@@ -87,8 +92,12 @@ export function runOpenshellCommand(
8792
env: { ...process.env, ...opts.env },
8893
encoding: "utf-8",
8994
stdio: opts.stdio ?? "inherit",
95+
timeout: opts.timeout,
9096
});
9197
if (result.error) {
98+
if (isIgnoredTimeout(result.error, opts)) {
99+
return result;
100+
}
92101
return handleSpawnError(binary, args, result.error, opts);
93102
}
94103
if (result.status !== 0 && !opts.ignoreError) {
@@ -111,8 +120,17 @@ export function captureOpenshellCommand(
111120
env: { ...process.env, ...opts.env },
112121
encoding: "utf-8",
113122
stdio: ["ignore", "pipe", "pipe"],
123+
timeout: opts.timeout,
114124
});
115125
if (result.error) {
126+
if (isIgnoredTimeout(result.error, opts)) {
127+
return {
128+
status: result.status,
129+
output: `${result.stdout || ""}${opts.ignoreError ? "" : result.stderr || ""}`.trim(),
130+
error: result.error,
131+
signal: result.signal,
132+
};
133+
}
116134
return handleSpawnError(binary, args, result.error, opts);
117135
}
118136
return {

‎src/nemoclaw.ts‎

Lines changed: 188 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
// SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved.
22
// SPDX-License-Identifier: Apache-2.0
33

4-
const { execFileSync, spawnSync } = require("child_process");
4+
const { execFileSync, spawn, spawnSync } = require("child_process");
55
const path = require("path");
66
const fs = require("fs");
77
const os = require("os");
@@ -113,6 +113,8 @@ type SpawnLikeResult = {
113113
stdout?: string;
114114
stderr?: string;
115115
output?: string;
116+
error?: Error;
117+
signal?: NodeJS.Signals | null;
116118
};
117119

118120
type SandboxCommandResult = {
@@ -131,6 +133,8 @@ const REMOTE_UNINSTALL_URL = buildVersionedUninstallUrl(getVersion());
131133
let OPENSHELL_BIN: string | null = null;
132134
const NEMOCLAW_GATEWAY_NAME = "nemoclaw";
133135
const DASHBOARD_FORWARD_PORT = String(DASHBOARD_PORT);
136+
const DEFAULT_LOGS_PROBE_TIMEOUT_MS = 5000;
137+
const LOGS_PROBE_TIMEOUT_ENV = "NEMOCLAW_LOGS_PROBE_TIMEOUT_MS";
134138

135139
function getOpenshellBinary(): string {
136140
if (!OPENSHELL_BIN) {
@@ -1904,8 +1908,14 @@ async function sandboxStatus(sandboxName: string) {
19041908
}
19051909

19061910
function sandboxLogs(sandboxName: string, follow: boolean) {
1907-
const args = buildSandboxLogsArgs(sandboxName, follow);
1911+
if (follow) {
1912+
streamSandboxFollowLogs(sandboxName);
1913+
return;
1914+
}
19081915

1916+
enableSandboxAuditLogs(sandboxName);
1917+
runOpenclawGatewayLogs(sandboxName, false);
1918+
const args = buildSandboxLogsArgs(sandboxName, false);
19091919
const result = runOpenshell(args, {
19101920
stdio: "inherit",
19111921
ignoreError: true,
@@ -1916,7 +1926,174 @@ function sandboxLogs(sandboxName: string, follow: boolean) {
19161926
exitWithSpawnResult(result);
19171927
}
19181928

1919-
function buildSandboxLogsArgs(sandboxName: string, follow: boolean): string[] {
1929+
function getLogsProbeTimeoutMs(): number {
1930+
const rawValue = process.env[LOGS_PROBE_TIMEOUT_ENV];
1931+
if (!rawValue) {
1932+
return DEFAULT_LOGS_PROBE_TIMEOUT_MS;
1933+
}
1934+
const parsed = Number(rawValue);
1935+
const timeoutMs = Number.isFinite(parsed) ? Math.floor(parsed) : Number.NaN;
1936+
return timeoutMs > 0 ? timeoutMs : DEFAULT_LOGS_PROBE_TIMEOUT_MS;
1937+
}
1938+
1939+
function describeLogProbeResult(result: SpawnLikeResult): string {
1940+
if (result.error) {
1941+
return result.error.message;
1942+
}
1943+
if (result.signal) {
1944+
return `signal ${result.signal}`;
1945+
}
1946+
return `exit ${result.status ?? "unknown"}`;
1947+
}
1948+
1949+
function runOpenclawGatewayLogs(sandboxName: string, follow: boolean): SpawnLikeResult {
1950+
const args = buildSandboxOpenclawGatewayLogsArgs(sandboxName, follow);
1951+
const result = runOpenshell(args, {
1952+
stdio: "inherit",
1953+
ignoreError: true,
1954+
timeout: getLogsProbeTimeoutMs(),
1955+
});
1956+
if (result.status !== 0) {
1957+
console.error(
1958+
` OpenClaw log source unavailable (${describeLogProbeResult(result)}): ` +
1959+
`openshell ${args.join(" ")}`,
1960+
);
1961+
}
1962+
return result;
1963+
}
1964+
1965+
function streamSandboxFollowLogs(sandboxName: string): void {
1966+
const openclawArgs = buildSandboxOpenclawGatewayLogsArgs(sandboxName, true);
1967+
const openshellArgs = buildSandboxLogsArgs(sandboxName, true);
1968+
const spawnOptions = {
1969+
cwd: ROOT,
1970+
env: process.env,
1971+
stdio: "inherit" as const,
1972+
};
1973+
const sources: Array<{
1974+
label: string;
1975+
args: string[];
1976+
child: import("node:child_process").ChildProcess;
1977+
done: boolean;
1978+
}> = [];
1979+
let exiting = false;
1980+
let completedSources = 0;
1981+
let finalStatus = 0;
1982+
let requestedExitCode: number | null = null;
1983+
let forcedExitTimer: NodeJS.Timeout | null = null;
1984+
// Guard against early exit: a source spawned before enableSandboxAuditLogs
1985+
// can fire its exit event during the blocking spawnSync call, before the
1986+
// second source is registered. Without this flag, maybeExit would see
1987+
// completedSources === sources.length === 1 and exit prematurely.
1988+
let setupComplete = false;
1989+
1990+
const stopChildren = (signal: NodeJS.Signals) => {
1991+
for (const { child } of sources) {
1992+
if (!child.killed && child.exitCode === null && child.signalCode === null) {
1993+
child.kill(signal);
1994+
}
1995+
}
1996+
};
1997+
const maybeExit = () => {
1998+
if (!setupComplete || completedSources !== sources.length) {
1999+
return;
2000+
}
2001+
if (forcedExitTimer) {
2002+
clearTimeout(forcedExitTimer);
2003+
forcedExitTimer = null;
2004+
}
2005+
process.exit(requestedExitCode ?? finalStatus);
2006+
};
2007+
const exitFromSignal = (signal: NodeJS.Signals | null): number => {
2008+
if (!signal) return 1;
2009+
const signalNumber = os.constants.signals[signal];
2010+
return signalNumber ? 128 + signalNumber : 1;
2011+
};
2012+
const markSourceDone = (
2013+
source: (typeof sources)[number],
2014+
status: number,
2015+
detail: string | null = null,
2016+
) => {
2017+
if (source.done) return;
2018+
source.done = true;
2019+
completedSources += 1;
2020+
if (status !== 0 && finalStatus === 0) {
2021+
finalStatus = status;
2022+
}
2023+
if (completedSources < sources.length && !exiting) {
2024+
const suffix = detail || `exit ${status}`;
2025+
console.error(` ${source.label} stopped (${suffix}); continuing with remaining log source.`);
2026+
}
2027+
maybeExit();
2028+
};
2029+
const requestExitAfterSignal = (signal: NodeJS.Signals, exitCode: number) => {
2030+
if (requestedExitCode !== null) return;
2031+
exiting = true;
2032+
requestedExitCode = exitCode;
2033+
stopChildren(signal);
2034+
forcedExitTimer = setTimeout(() => process.exit(exitCode), 2000);
2035+
forcedExitTimer.unref?.();
2036+
maybeExit();
2037+
};
2038+
2039+
process.once("SIGINT", () => {
2040+
requestExitAfterSignal("SIGINT", 130);
2041+
});
2042+
process.once("SIGTERM", () => {
2043+
requestExitAfterSignal("SIGTERM", 143);
2044+
});
2045+
2046+
const addSource = (label: string, args: string[]) => {
2047+
const source = { label, args, child: spawn(getOpenshellBinary(), args, spawnOptions), done: false };
2048+
sources.push(source);
2049+
source.child.on("error", (error: Error) => {
2050+
markSourceDone(source, 1, error.message);
2051+
});
2052+
source.child.on("exit", (code: number | null, signal: NodeJS.Signals | null) => {
2053+
markSourceDone(source, code ?? exitFromSignal(signal), signal ? `signal ${signal}` : null);
2054+
});
2055+
};
2056+
2057+
addSource("OpenClaw log source", openclawArgs);
2058+
enableSandboxAuditLogs(sandboxName);
2059+
addSource("OpenShell log source", openshellArgs);
2060+
setupComplete = true;
2061+
maybeExit();
2062+
}
2063+
2064+
function enableSandboxAuditLogs(sandboxName: string) {
2065+
const args = buildEnableSandboxAuditLogsArgs(sandboxName);
2066+
const result = runOpenshell(args, {
2067+
stdio: ["ignore", "ignore", "pipe"],
2068+
ignoreError: true,
2069+
timeout: getLogsProbeTimeoutMs(),
2070+
});
2071+
if (result.status !== 0) {
2072+
warnSandboxAuditLogsUnavailable(sandboxName, args, result);
2073+
}
2074+
}
2075+
2076+
function warnSandboxAuditLogsUnavailable(
2077+
sandboxName: string,
2078+
args: string[],
2079+
result: SpawnLikeResult,
2080+
): void {
2081+
const stderr = String(result.stderr || "").trim();
2082+
console.error(
2083+
` Warning: failed to enable OpenShell audit logs for sandbox '${sandboxName}' ` +
2084+
`(${describeLogProbeResult(result)}): openshell ${args.join(" ")}`,
2085+
);
2086+
if (stderr) {
2087+
console.error(` ${stderr}`);
2088+
}
2089+
console.error(" Policy denial events may be missing from OpenShell logs.");
2090+
}
2091+
2092+
function buildEnableSandboxAuditLogsArgs(sandboxName: string): string[] {
2093+
return ["settings", "set", sandboxName, "--key", "ocsf_json_enabled", "--value", "true"];
2094+
}
2095+
2096+
function buildSandboxOpenclawGatewayLogsArgs(sandboxName: string, follow: boolean): string[] {
19202097
const args = ["sandbox", "exec", "-n", sandboxName, "--", "tail", "-n", "200"];
19212098
if (follow) {
19222099
args.push("-f");
@@ -1925,6 +2102,14 @@ function buildSandboxLogsArgs(sandboxName: string, follow: boolean): string[] {
19252102
return args;
19262103
}
19272104

2105+
function buildSandboxLogsArgs(sandboxName: string, follow: boolean): string[] {
2106+
const args = ["logs", sandboxName, "-n", "200", "--source", "all"];
2107+
if (follow) {
2108+
args.push("--tail");
2109+
}
2110+
return args;
2111+
}
2112+
19282113
/**
19292114
* Handle `nemoclaw <sandbox> policy-add [flags]`. Supports three mutually
19302115
* exclusive modes: interactive preset picker (default), `--from-file <path>`

0 commit comments

Comments
 (0)